Skip to content

feat(config): top-level workspaces schema + --workspace selector - #848

Open
BYK wants to merge 3 commits into
masterfrom
feat/workspaces-schema
Open

feat(config): top-level workspaces schema + --workspace selector#848
BYK wants to merge 3 commits into
masterfrom
feat/workspaces-schema

Conversation

@BYK

@BYK BYK commented Jul 22, 2026

Copy link
Copy Markdown
Member

Summary

PR B of the workspaces redesign (design doc: .opencode/plans/workspaces-redesign-design.md, tracking #842). Adds a first-class, target-agnostic top-level workspaces: config key plus a --workspace selector so a single .craft.yml can describe multiple independently-versioned release units (e.g. cli@, mcp@).

This PR is the schema + resolver + selector groundwork only. It is fully backward-compatible and inert when no workspaces key is present — the full suite (1078 tests) passes unchanged. Threading the selection through prepare/publish and the publish-issue action layer comes in follow-up PRs (C/D).

What's in it

  • Schema (schemas/project_config.ts): a top-level workspaces map. Each workspace mirrors the release-relevant subset of the top-level config and inherits unspecified fields from the top level. The workspace github block is partial so it can override just projectPath while inheriting owner/repo.
  • Resolver (config.ts): getConfiguration() returns a resolved view — base config merged with the selected workspace's overrides — so getGitTagPrefix, getGlobalGitHubConfig, getVersioningPolicy, providers, and expandWorkspaceTargets(config.targets) all become workspace-aware with no per-accessor changes.
  • Selector (index.ts): global --workspace <name> (env CRAFT_WORKSPACE). Applied before yargs parsing because command builders (notably publish, which derives --target choices from config.targets) run before middleware. Verified end-to-end.
  • Validation: unknown/absent selection, or omitting it when workspaces are defined, fails fast with a helpful message listing available workspaces. Gated behind minVersion >= 2.27.0.
  • checkMinimalConfigVersion dev-build relaxation: a pre-release build of X.Y.Z (e.g. 2.27.0-dev.0) now satisfies a minVersion up to X.Y.Z, so features can be dogfooded from a dev build before their release is cut. Released builds are unaffected; a genuinely-too-new minVersion (e.g. 2.28.0) is still correctly rejected.

Note: this "workspace" (a release unit) is distinct from the npm target's existing workspaces: true field (npm package discovery within one target). No schema clash — one is top-level, one is a target field. Docs (PR E) will disambiguate.

Review — critical bug caught & fixed

Adversarial review found a CRITICAL bug (C1): the publish builder calls getConfiguration() at parse time, which runs before the --workspace middleware — so with a workspaces config it threw "select a workspace" before the selection was applied, breaking craft publish for the very feature this ships. Fixed by resolving the workspace from raw argv/env before .parse(), plus a graceful fallback in the publish builder. Verified end-to-end with a built binary:

  • craft targets --workspace cli → resolves the cli workspace's targets ✓
  • CRAFT_WORKSPACE=cli and --workspace=mcp forms both work ✓
  • no selection → friendly error listing cli, mcp
  • unknown workspace → Unknown workspace "nope". Available workspaces: cli, mcp.

Testing

pnpm test (1078 passed, 1 skipped), tsc, pnpm lint (0 errors), prettier --check all clean. New tests cover schema parsing, the resolver merge (overrides win / base inherited / partial github), all error paths, the minVersion gate, cache re-resolution on selection change, and the publish-builder contract (regression for C1).

Part of #842.

🤖 Generated with opencode

Introduces native, target-agnostic "workspaces" so a single .craft.yml can
define multiple independently-versioned release units (e.g. cli@, mcp@). This
is the schema + resolver + selector groundwork; it is fully backward compatible
and inert when no `workspaces` key is present.

- schema: add a top-level `workspaces` map. Each workspace mirrors the
  release-relevant subset of the top-level config (targets, github,
  releaseBranchPrefix, changelog, versioning, providers, ...) and inherits
  unspecified fields from the top level. A workspace's `github` block is partial
  so it can override just `projectPath` while inheriting owner/repo.
- resolver: `getConfiguration()` returns a resolved VIEW — the base config
  merged with the selected workspace's overrides — so all existing accessors
  (getGitTagPrefix, getGlobalGitHubConfig, getVersioningPolicy, providers,
  expandWorkspaceTargets) become workspace-aware without individual changes.
- selector: global `--workspace <name>` option (env `CRAFT_WORKSPACE`). The
  selection is applied BEFORE yargs parsing because command builders (notably
  `publish`, which derives its --target choices from config.targets) run before
  middleware; the `publish` builder also falls back gracefully if the config
  can't be resolved at parse time.
- validation: selecting an unknown/absent workspace, or omitting the selection
  when workspaces are defined, fails fast with a helpful message. Gated behind
  minVersion >= 2.27.0.
- checkMinimalConfigVersion: a dev/pre-release build of X.Y.Z (e.g.
  2.27.0-dev.0) now satisfies a minVersion of up to X.Y.Z, so a feature can be
  dogfooded from a dev build before its release is cut. Released builds are
  unaffected.

NOTE: this "workspace" (a release unit) is distinct from the npm target's
existing `workspaces: true` field (npm package discovery within one target).

Part of #842. Follow-ups will thread the selection through prepare/publish and
the publish-issue action layer.
Comment thread src/config.ts
Comment thread src/index.ts Outdated
- resolver: a workspace `github` override that only sets projectPath (with no
  top-level github) no longer produces a truthy-but-incomplete github object.
  The merged github is adopted only when it has both owner and repo; otherwise
  it is left unset so getGlobalGitHubConfig() still falls back to git-remote
  detection. (Bugbot High)
- extractWorkspaceSelection: no longer greedily consumes the token after a bare
  `--workspace` (e.g. `--workspace --dry-run`) as the value, and no longer
  suppresses CRAFT_WORKSPACE when the flag has no value. Moved to utils/helpers
  so it is unit-testable without importing the CLI entrypoint (which has import
  side effects), and added coverage. (Bugbot Medium)
- index: removed the redundant post-parse workspace middleware. The pre-parse
  extraction is the single source of truth; the middleware re-applied the parsed
  (possibly empty) --workspace value and clobbered a valid CRAFT_WORKSPACE
  fallback for a bare flag.

Adversarial review verdict: solid/merge. Verified end-to-end with the built
binary. Part of #842.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c099bbf. Configure here.

Comment thread src/config.ts
* release (e.g. `2.27.0-dev.0`) satisfies it via the pre-release relaxation in
* `checkMinimalConfigVersion`.
*/
export const WORKSPACES_MIN_VERSION = '2.27.0';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale workspaces minVersion gate

High Severity

WORKSPACES_MIN_VERSION is 2.27.0, but 2.27.x and 2.28.0 already shipped without top-level workspaces support. A config that sets minVersion: 2.27.0 (or 2.28.0) and defines workspaces still passes those older binaries' version check; Zod then strips the unknown key, so craft can run against the wrong release unit. The gate needs the version that will first ship this feature (currently heading to 2.29.0).

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c099bbf. Configure here.

Comment thread src/config.ts
Comment on lines 519 to 527
logger.warn(
'Multiple "github" targets with different "tagPrefix" values found. ' +
`Using "${firstPrefix}". For independently-versioned products in a ` +
'monorepo, use a separate .craft.yml per product, each with a single ' +
'"github" target and its own "tagPrefix".',
'monorepo, use a top-level "workspaces" entry per product (or a ' +
'separate .craft.yml), each with a single "github" target and its own ' +
'"tagPrefix".',
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The isVersionGteMinVersion function can crash if config.minVersion in .craft.yml contains SemVer build metadata (e.g., 2.27.0+build123), as it's not stripped before comparison.
Severity: MEDIUM

Suggested Fix

Modify isVersionGteMinVersion and checkMinimalConfigVersion to strip build metadata from all SemVer objects before passing them to versionGreaterOrEqualThan. This can be done by creating a new SemVer object with the build property set to undefined regardless of whether the version is a pre-release or not. This will make the version comparison logic robust against versions with build metadata from any source.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/config.ts#L519-L527

Potential issue: The `versionGreaterOrEqualThan` function throws an exception if either
of the versions being compared contains SemVer build metadata (e.g., `+build123`). The
`checkMinimalConfigVersion` function only strips this metadata from pre-release
versions, not stable ones. A more critical issue exists in `isVersionGteMinVersion`,
which is called during workspace selection. If a user specifies a `minVersion` in their
`.craft.yml` configuration file with build metadata (e.g., `minVersion:
"2.27.0+linux"`), the application will crash because the metadata is not stripped before
being passed to `versionGreaterOrEqualThan` for comparison. This leads to an unhandled
exception when validating the workspace configuration.

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant