Distillation

Date: Jul 22, 2026

• 🟡 (10:55) [tool:result] Git status for feat/workspaces-schema (getsentry/craft): branch up to date with origin/master; changes are UNSTAGED/uncommitted in working tree (not yet committed) for src/tests/config.test.ts, src/config.ts, src/index.ts, src/schemas/project_config.ts; untracked files .craft-issue-842-plan.md and .opencode/.
• 🟢 (10:55) Assistant noted the reviewed changes are uncommitted working-tree state, treated as the intended "PR B".
• 🟡 (10:55) Diff shows config.test.ts adds new imports from ../config: setActiveWorkspace, getActiveWorkspace, getVersioningPolicy, WORKSPACES_MIN_VERSION.
• 🟡 (10:55) New test "parses configuration with workspaces" validates a config with top-level minVersion: '2.27.0', github: {owner: getsentry, repo: toolkit}, and workspaces.cli (releaseBranchPrefix: release/cli, github.projectPath: cli, targets: [{name: github, tagPrefix: 'cli@'}]) and workspaces.mcp (targets: [{name: github, tagPrefix: 'mcp@'}]); expects validateConfiguration(data) to equal data unchanged.
• 🟡 (10:55) New test "allows a workspace github override without owner/repo" validates {workspaces: {cli: {github: {projectPath: 'cli'}}}} does not throw, since workspace github override is partial and owner/repo are inherited from base.
• 🟡 (10:55) Diff also adds tests: "errors on an unknown workspace name" and "setActiveWorkspace re-resolves against a new selection"; plus a comment confirming "workspaces" key is stripped from the resolved config; plus a gate test constructing "belowGate" by replacing WS_CONFIG's minVersion to a value below WORKSPACES_MIN_VERSION.
• 🟡 (10:55) config.ts diff adds: setActiveWorkspace()/getActiveWorkspace() functions setting/reading module-level _activeWorkspaceName; resolveWorkspaceConfig() throws Unknown workspace "${workspaceName}". with available workspace names listed; applyWorkspaceSelection() throws --workspace "${_activeWorkspaceName}" was given but no "workspaces"... when workspace selected but none configured; a workspaces-gate check using isVersionGteMinVersion(config.minVersion, WORKSPACES_MIN_VERSION); merge logic special-cases key === 'github' (shallow-merge) inside Object.entries(workspace) loop.
• 🔴 (10:55) Code comment in src/config.ts confirms design intent for checkMinimalConfigVersion: dev/pre-release relaxation (e.g. "2.27.0-dev.0" treated as satisfying minVersion up to "2.27.0") is applied ONLY to the CURRENT craft version side via effectiveCurrentVersion (currentVersion.pre stripped of pre/build when present) — the configured minVersion side is never relaxed/stripped. Rationale stated: without this, a local dev build would reject any config whose minVersion targets the very release that build is heading toward, blocking dogfooding of new features pre-release.
• 🟡 (10:55) requiresMinVersion() was refactored to delegate to a shared isVersionGteMinVersion(config.minVersion, requiredVersion) helper (previously had its own inline parseVersion/compare logic) — behavior-preserving refactor per diff.
• 🟡 (10:55) [tool:result] Master (origin/master) CraftProjectConfigSchema field order: github, targets, preReleaseCommand, postReleaseCommand, releaseBranchPrefix, changelog, changelogPolicy, minVersion, requireNames, statusProvider, artifactProvider, versioning, noMerge.
• 🟡 (10:55) Assistant's schema comparison finding: new branch extracts a shared releaseUnitFields object (github, targets, preReleaseCommand, postReleaseCommand, releaseBranchPrefix, changelog, changelogPolicy, requireNames, statusProvider, artifactProvider, versioning, noMerge) spread into CraftProjectConfigSchema, then adds minVersion + workspaces; net effect is minVersion moved from position 8 (master) to after noMerge — assistant noted Zod z.object field order does not affect validation, and confirmed github appears exactly once (only via releaseUnitFields spread, not re-added at top level) — no duplication/drop.
• 🟡 (10:55) [tool:result] grep of getConfiguration() usages across commands: publish.ts:60 (getConfiguration().targets used to compute allowedTargetNames) and publish.ts:243,563 (in handler); publish.ts:265,679 use getGlobalGitHubConfig(); prepare.ts:704,819 (getConfiguration in handler, not builder), also uses getGitTagPrefix()/getVersioningPolicy(); targets.ts:10 (getConfiguration in handler function, imports getConfiguration + expandWorkspaceTargets); changelog.ts uses getGitTagPrefix()/getVersioningPolicy(); commands/config.ts uses getConfiguration()+getGlobalGitHubConfig() in handler.
• 🟡 (10:55) [tool:result] publish.ts full builder shown: export const builder: CommandBuilder = (yargs: Argv) => { const definedTargets = getConfiguration().targets || []; ... } computing allowedTargetNames for the --target/-t option choices (also has options: rev/-r, merge-target/-m, remote (default origin), no-merge (default false), keep-branch (default false), keep-downloads (default false)).
• 🔴 [investigated-caching-bug] (10:56) Assistant empirically verified yargs (v18.0.0, installed via pnpm) execution order using a minimal ESM test harness run from within the craft repo dir: confirmed order is BUILDER → MIDDLEWARE → HANDLER (tool result: ["BUILDER","MIDDLEWARE(workspace=cli)","HANDLER"]).
• 🔴 (10:56) CRITICAL BUG CONFIRMED: publish.ts builder (line 60) calls getConfiguration() at parse time, BEFORE the setActiveWorkspace middleware runs (middleware order in src/index.ts is .middleware(setGlobals) then .middleware(argv => setActiveWorkspace(argv.workspace))). For a config that defines workspaces, this causes getConfiguration()→applyWorkspaceSelection() to throw "This configuration defines workspaces; select one with --workspace" even when the user DID pass --workspace cli on the command line, because _activeWorkspaceName is still undefined at builder time. Result: craft publish is unusable whenever workspaces is defined in config, regardless of --workspace flag.
• 🟡 (10:56) [tool:result] Confirmed via separate test harness: builder throw propagates and aborts parsing/execution (tool result: "CAUGHT: BUILDER THREW (workspaces defined, none selected)") — no middleware/handler runs after.
• 🟡 (10:56-10:57) Assistant verified no earlier getConfiguration() call exists before the publish builder runs: printVersion() (src/index.ts:26) and warnIfCraftEnvFileExists() (src/utils/env.ts:97) were checked — warnIfCraftEnvFileExists calls getConfigFileDir() (via findConfigFile()), NOT getConfiguration(), and getConfigFileDir() does not call validateConfiguration/getConfiguration either. So publish.ts:60 is the first call site to touch config resolution for that command.
• 🟡 (10:57) [tool:result] targets.ts's getConfiguration() call (line 10) is inside the async handler() function, which runs AFTER middleware — not affected by the caching-order bug. It additionally calls expandWorkspaceTargets(definedTargets) (comment: "Expand workspace targets (e.g., npm workspaces)") before computing allowedTargetNames from getAllTargetNames().
• 🟡 (10:57) [tool:result] prepare.ts's builder (lines 84+) does NOT call getConfiguration() — its only getConfiguration() calls (lines 704, 819) are deep inside handler logic (after middleware), confirmed not affected by the parse-time caching bug. The scope of the CRITICAL bug is limited to publish.ts only among the reviewed command builders.
• 🟡 (10:57) Assistant flagged a SECONDARY (lower-severity) issue in publish.ts: even ignoring the throw, the builder's allowedTargetNames (used for --target/-t CLI choices validation) is computed from the BASE (unresolved) config's targets before workspace resolution — so --target choices would reflect base targets rather than the selected workspace's targets. Currently moot because the primary throw blocks execution first, but would resurface as a real bug if the throw is fixed without also fixing choice computation to be workspace-aware.
• 🟡 (10:57) [tool:result] SemVer type (src/utils/versions.ts or similar) has fields including optional pre?: string (line 52) and build. versionGreaterOrEqualThan(v1, v2) (lines 81-108): if !v1.pre && v2.pre → v1 wins; if v1.pre && !v2.pre → v2 wins; if both have pre, pre values differ, and both are purely numeric (/^\d+$/) → numeric string comparison; otherwise (line 100-101) if v1.build || v2.build || v1.pre || v2.pre → THROWS (cannot compare non-numeric differing pre-release/build metadata). This confirms area D's throw-path concern: stripping pre/build from the CURRENT version side (effectiveCurrentVersion) before comparison avoids hitting this throw when currentVersion is a dev pre-release being compared against a numeric-only minVersion.