Distillation

Current State

• Date: Aug 24, 2026, 15:36. Work is in progress on Craft PR #848 (feat/workspaces-schema), in the dedicated worktree /home/byk/Code/getsentry/craft-workspaces-schema (earlier preparation also referenced /home/byk/Code/getsentry).
• The user requested that PR #848 review feedback be addressed, followed by a final adversarial review and merge. Afterward, assess whether PR #844 (https://github.com/getsentry/craft/pull/844) remains relevant or which pieces remain relevant.
• PR #848’s latest remote head is merge commit c099bbf97c27330e69d2d329dac817bd3f95e0b0 (Merge branch 'master' into feat/workspaces-schema), based on e5a7f97ad03f76d5265e0ac7b5deb38cfef19206. It includes unrelated current-master Vercel/dependency changes; final review must constrain evaluation to the PR’s intended workspace changes relative to master.
• Core review fixes were implemented by 15:24 in:
  • src/utils/helpers.ts
  • src/config.ts
• Regression-test edits were made by 15:32 in:
  • src/utils/__tests__/helpers.test.ts
  • src/__tests__/config.test.ts
• Current blocker: focused tests cannot run because this worktree has no dependencies installed. At 15:36, vitest run src/utils/__tests__/helpers.test.ts src/__tests__/config.test.ts failed before test execution with:
  • vitest: not found
  • pnpm ELIFECYCLE
  • warning: Local package.json exists, but node_modules missing, did you mean to install?
• Next action: run pnpm install --frozen-lockfile in /home/byk/Code/getsentry/craft-workspaces-schema, then rerun focused tests, typecheck/build/lint as appropriate, inspect diffs/adversarially review PR #848, commit/push/merge if clean, then assess PR #844.
• Do not disturb the user’s active unpublished work in /home/byk/Code/getsentry/craft: branch byk/fix/commit-on-repo at b06435e, with modified .lore.md and untracked .craft-issue-842-plan.md and .opencode/.

Key Decisions

• Use pnpm only. User explicitly requires that Craft package management always use pnpm; never use npm or yarn. Dependency installation must be pnpm install --frozen-lockfile.
• Workspace gate version: change WORKSPACES_MIN_VERSION from '2.27.0' to '2.29.0'. Rationale: top-level workspaces support did not ship in 2.27.x or 2.28.0; feature is targeting 2.29.0. The active package development version is 2.29.0-dev.0.
• Pre-release compatibility policy: user clarified at 15:15 that prerelease relaxation in checkMinimalConfigVersion applies only to the current Craft version, never to the configured minVersion side. Thus 2.29.0-dev.0 may satisfy a configured stable 2.29.0, but a configured prerelease must not be artificially promoted/relaxed.
• SemVer build metadata: normalize/ignore SemVer build metadata before compatibility comparisons, because build metadata does not affect SemVer precedence. Preserve major/minor/patch and prerelease state as appropriate. This prevents crashes for values such as minVersion: "2.27.0+linux".
• Use Node’s built-in parser, but do not blindly trust its value semantics. User requested Node’s built-in argument parser. node:util.parseArgs({ tokens: true }) should replace hand-rolled argv scanning where practical, with token validation for --workspace.
  • Rejected approach: use parseArgs values alone. On Node v24.18.0, it parses ["publish","--workspace","--dry-run"] as workspace: '--dry-run', which is invalid for desired semantics.
  • Chosen behavior: inspect parser token stream and accept only a real non-flag string workspace value; preserve yargs’s normal missing-value error behavior. --workspace --dry-run, trailing --workspace, and --workspace= must not create a bogus selection or suppress CRAFT_WORKSPACE.
• Early workspace selection must remain pre-yargs. src/index.ts must establish active workspace before yargs parsing/builders because command builders (e.g. publish) can read configuration and derive --target choices before middleware runs. Middleware-based selection was rejected as too late.
• GitHub config merge fallback: retain merged github only if it has both owner and repo; otherwise delete/unset it so getGlobalGitHubConfig() can detect from git remote. This was already fixed in commit 42d17f0.
• Dry-run architecture: all new destructive operations must use centralized dry-run-aware wrappers:
  • Git: getGitClient() / createGitClient(directory) from src/utils/git.ts
  • GitHub: getGitHubClient() from src/utils/githubApi.ts
  • Files: safeFs from src/utils/dryRun.ts
  • Other processes: safeExec() / safeExecSync() from src/utils/dryRun.ts
  • Direct isDryRun() remains suitable only for tools with native dry-run flags, mock data, and UX optimizations such as skip-sleep.
• Project development commands: use pnpm build, pnpm test, pnpm lint, and pnpm fix; manual validation is pnpm build && ./dist/craft.

Technical Changes

• PR #848 branch/history
  • Branch: feat/workspaces-schema
  • Earlier local/remote head: 42d17f0 (fix(config): address Bugbot findings on workspaces selector)
  • After fetch at 15:06, fast-forwarded 42d17f0..c099bbf.
  • Current head: c099bbf (Merge branch 'master' into feat/workspaces-schema).
  • The fast-forward brought 23 changed files, 2,865 insertions, and 1,303 deletions, including unrelated master content: docs/src/content/docs/targets/vercel.md, src/targets/__tests__/vercel.test.ts, and src/targets/vercel.ts.
  • Relevant earlier commits: 4e2d8f4 (feat(config): add top-level workspaces schema + --workspace selector), f70d5a8 (#847), 83964a8 (#846), 0f8720e (#843). Merged-master history includes e5a7f97 (#866), 8cc5425 (#865), f9e4663 (#863), and dfa07f2.
• Previously fixed in commit 42d17f0:
  • Fixed incomplete GitHub configuration created by shallow workspace merge (src/config.ts#L136-L142, src/config.ts#L456-L459): workspace-only github.projectPath with no top-level GitHub config could create a truthy but incomplete object, make getGlobalGitHubConfig() skip remote detection, and yield no owner/repo.
  • Added regression test: does not produce an incomplete github when base has none.
  • Moved/fixed extractWorkspaceSelection in src/utils/helpers.ts: valueless/bare flags and --workspace --dry-run do not treat flags as workspace names and do not suppress CRAFT_WORKSPACE; redundant post-parse middleware that clobbered pre-parse selection was removed.
• New PR-review fixes implemented by 15:24:
  • src/utils/helpers.ts
    • Reworked custom argv scanning toward node:util.parseArgs with token validation.
    • extractWorkspaceSelection(argv, env = process.env) remains the single precedence authority for CLI --workspace vs. CRAFT_WORKSPACE.
    • Required semantics:
      • accepts --workspace foo and --workspace=foo;
      • CLI supplied value overrides CRAFT_WORKSPACE;
      • a bare/valueless --workspace, --workspace --dry-run, trailing --workspace, and empty --workspace= fall back to environment or undefined;
      • do not consume a following --prefixed option as workspace name;
      • leave invalid missing-value behavior available for yargs.
    • Also planned/reworked fixGlobalBooleanFlags(argv) to use Node parser facilities while preserving GLOBAL_BOOLEAN_FLAGS behavior for standalone --no-input/--dry-run flags and explicit --flag=no.
  • src/config.ts
    • Raised WORKSPACES_MIN_VERSION from '2.27.0' to '2.29.0'.
    • Made version comparisons robust to SemVer build metadata, addressing failure in versionGreaterOrEqualThan at src/config.ts#L519-L527.
    • checkMinimalConfigVersion should still relax only a prerelease current version by stripping pre/build; configured minVersion prerelease semantics must remain intact.
• Regression coverage added/being added:
  • src/utils/__tests__/helpers.test.ts
    • Existing coverage already includes no flag/env, --workspace foo, --workspace=foo, CLI-over-env, env-only, --workspace --dry-run, trailing bare --workspace, and --workspace=.
    • Planned/added an inline negative-name edge case to ensure a flag-like value is not selected improperly.
  • src/__tests__/config.test.ts
    • Existing WS_CONFIG uses minVersion: ${WORKSPACES_MIN_VERSION}, base GitHub owner: getsentry, repo: toolkit, changelog: CHANGELOG.md, and cli/mcp workspace fixtures.
    • Existing tests cover backward compatibility, workspace merge/inheritance, workspace selection errors, old min-version rejection, cache reselection, publish-builder target exposure, incomplete GitHub fallback, and valid workspace GitHub owner/repo overrides.
    • Planned/added explicit build-metadata regression so both global compatibility and the workspace gate accept appropriate minVersion values with build metadata.
    • Because package version is 2.29.0-dev.0, workspace fixture tests now also verify development-build equivalence against stable required version 2.29.0.
• Relevant current code behavior
  • Active workspace state is _activeWorkspaceName: string | undefined; setActiveWorkspace(name) invalidates _configCache and _globalGitHubConfigCache.
  • applyWorkspaceSelection(config):
    • considers workspaces present only if there is at least one key;
    • no workspaces/no selection: returns config unchanged;
    • selection without workspaces: throws ConfigurationError;
    • workspaces/no selection: requires --workspace <name> or CRAFT_WORKSPACE, lists available choices;
    • selection with workspaces: requires config.minVersion >= WORKSPACES_MIN_VERSION, then resolves workspace config.
  • resolveWorkspaceConfig(base, workspaceName) strips workspaces, shallow-overrides workspace fields, shallow-merges github, preserves only complete owner/repo GitHub config, otherwise removes github to permit remote fallback.
  • loadConfiguration() and loadConfigurationFromString() validate YAML, call checkMinimalConfigVersion(parsed), then cache applyWorkspaceSelection(parsed).
  • checkMinimalConfigVersion has an existing debug-message typo: skpipping.
  • isVersionGteMinVersion returns false when min version is absent or unparsable.
• Environment/repository conventions
  • Craft is TypeScript; package @sentry/craft, version 2.29.0-dev.0; bins craft and sentry-craft map to dist/craft.
  • User says Volta manages Node.js v22.12.0; an initial shell reported Node v24.18.0, so use Volta/project-managed tooling for validation.
  • Main branch: master; CI tests Node 20 and 22.
  • Tests: Vitest, generally src/__tests__/*.test.ts; mocks use vi.fn(), vi.mock(), vi.spyOn().
  • Formatting: Prettier 3.x, single quotes/no arrow parentheses via .prettierrc.yml.
  • Linting: ESLint 9 flat config, eslint.config.mjs, typescript-eslint; _-prefixed unused variables allowed.
  • Important paths include src/__mocks__/, src/__tests__/, src/artifact_providers/, src/commands/, src/schemas/, src/status_providers/, src/targets/, src/types/, src/utils/, src/config.ts, src/index.ts, src/logger.ts, and dist/craft.
  • .craft.yml is root configuration; schema is in src/schemas/.
  • ESLint forbids direct simple-git imports and direct new Octokit(). Wrapper modules needing raw simple-git require: // eslint-disable-next-line no-restricted-imports -- This is the wrapper module.

Session Timeline

• Aug 24, 2026, 14:53: User said PR #848 was ready with feedback; requested fixes, final adversarial review, merge, and follow-on assessment of PR #844 relevance. Assistant committed to isolate PR work from unpublished “PR C” work.
• 14:55: Review feedback identified:
  • bogus workspace selection from naïve --workspace scanning;
  • request to use Node built-in argument parser;
  • stale workspace feature gate (2.27.x/2.28.0 did not ship feature; target 2.29.0);
  • potential SemVer build-metadata crash;
  • prior GitHub merge bug already addressed by 42d17f0.
• 14:57: Inspected repository/worktrees. User worktree was byk/fix/commit-on-repo at b06435e; PR #848 was feat/workspaces-schema at c099bbf; shell Node was v24.18.0; then-known package version output was 2.27.0-dev.0.
• 15:02: Tested node:util.parseArgs; discovered it incorrectly assigns --dry-run as a space-separated --workspace value. Chose token-stream validation.
• 15:04–15:06: Prepared dedicated review worktree, fetched/fast-forwarded branch from 42d17f0 to c099bbf; noted branch merge includes unrelated current-master changes.
• 15:08: User supplied authoritative Craft conventions: pnpm-only, Volta Node v22.12.0, frozen lockfile install, standard commands, TypeScript/test/lint/dry-run rules, package version 2.29.0-dev.0, and project architecture.
• 15:10–15:20: Inspected config/index/helper implementation. Confirmed old gate '2.27.0'; verified pre-yargs selection is necessary; identified concrete plan to use parseArgs tokens, raise gate to 2.29.0, and normalize build metadata.
• 15:15: User clarified prerelease relaxation applies solely to the current Craft version, not configured minVersion.
• 15:24: Modified src/utils/helpers.ts and src/config.ts implementing core review fixes.
• 15:27–15:32: Reviewed existing test coverage and modified src/utils/__tests__/helpers.test.ts plus src/__tests__/config.test.ts for parser/build-metadata/version-gate regressions.
• 15:36: Focused Vitest command failed because node_modules is absent. Must install with pnpm install --frozen-lockfile before validation and onward PR/merge work.