From 6dae341f9bc1378abde7a9022108caded2d22226 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 26 Aug 2026 14:16:47 +0000 Subject: [PATCH 1/2] feat: propagate release workspaces --- .github/workflows/release.yml | 9 ++ action.yml | 35 +++++- src/__tests__/action.test.ts | 144 +++++++++++++++++++++++++ src/commands/__tests__/publish.test.ts | 47 ++++++++ src/commands/publish.ts | 33 +++++- 5 files changed, 263 insertions(+), 5 deletions(-) create mode 100644 src/__tests__/action.test.ts diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4e6c11ecb..323e8ba8d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,6 +12,9 @@ on: force: description: Force a release even when there are release-blockers (optional) required: false + workspace: + description: Named Craft release workspace to prepare and publish + required: false # For external repos to call this workflow workflow_call: @@ -51,6 +54,10 @@ on: type: string required: false default: '.' + workspace: + description: Named Craft release workspace to prepare and publish + type: string + required: false craft_config_from_merge_target: description: Use the craft config from the merge target branch type: string @@ -122,6 +129,7 @@ jobs: with: version: ${{ github.event.inputs.version }} force: ${{ github.event.inputs.force }} + workspace: ${{ github.event.inputs.workspace }} # For external repos: use published action - name: Prepare release @@ -139,4 +147,5 @@ jobs: git_user_name: ${{ inputs.git_user_name }} git_user_email: ${{ inputs.git_user_email }} path: ${{ inputs.path }} + workspace: ${{ inputs.workspace }} craft_config_from_merge_target: ${{ inputs.craft_config_from_merge_target }} diff --git a/action.yml b/action.yml index 8fa93907e..6344db2fe 100644 --- a/action.yml +++ b/action.yml @@ -31,6 +31,9 @@ inputs: description: The path that Craft will run inside required: false default: '.' + workspace: + description: Named Craft release workspace to prepare and publish + required: false craft_config_from_merge_target: description: Use the craft config from the merge target branch required: false @@ -68,6 +71,16 @@ outputs: runs: using: 'composite' steps: + - name: Validate workspace + shell: bash + env: + WORKSPACE: ${{ inputs.workspace }} + run: | + if [[ -n "$WORKSPACE" ]] && node -e 'process.exit(/[\p{Cc}\p{Cf}\p{Zl}\p{Zp}]/u.test(process.env.WORKSPACE) ? 0 : 1)'; then + echo "::error::Workspace names cannot contain Unicode control, format, or separator characters." + exit 1 + fi + - id: killswitch name: Check release blockers shell: bash @@ -162,6 +175,7 @@ runs: CRAFT_CONFIG_FROM_MERGE_TARGET: ${{ inputs.craft_config_from_merge_target }} MERGE_TARGET: ${{ inputs.merge_target }} VERSION: ${{ inputs.version }} + WORKSPACE: ${{ inputs.workspace }} working-directory: ${{ inputs.path }} run: | # Ensure we have origin/HEAD set @@ -172,6 +186,9 @@ runs: if [[ "$CRAFT_CONFIG_FROM_MERGE_TARGET" == 'true' && -n "$MERGE_TARGET" ]]; then CRAFT_ARGS=(--config-from "$MERGE_TARGET") fi + if [[ -n "$WORKSPACE" ]]; then + CRAFT_ARGS+=("--workspace=$WORKSPACE") + fi # Version is optional - if not provided, Craft uses versioning.policy from config VERSION_ARG=() @@ -187,8 +204,13 @@ runs: working-directory: ${{ inputs.path }} env: CRAFT_LOG_LEVEL: Warn + WORKSPACE: ${{ inputs.workspace }} run: | - targets=$(craft targets | jq -r '.[]|" - [ ] \(.)"') + CRAFT_ARGS=() + if [[ -n "$WORKSPACE" ]]; then + CRAFT_ARGS=("--workspace=$WORKSPACE") + fi + targets=$(craft targets "${CRAFT_ARGS[@]}" | jq -r '.[]|" - [ ] \(.)"') # https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings echo "targets<> "$GITHUB_OUTPUT" @@ -211,6 +233,7 @@ runs: SUBDIRECTORY: ${{ inputs.path != '.' && format('/{0}', inputs.path) || '' }} MERGE_TARGET: ${{ inputs.merge_target || '(default)' }} PUBLISH_REPO: ${{ inputs.publish_repo || format('{0}/publish', github.repository_owner) }} + WORKSPACE: ${{ inputs.workspace }} run: | # Resolve "self" to the current repository if [[ "$PUBLISH_REPO" == "self" ]]; then @@ -221,7 +244,6 @@ runs: echo "::error::Craft did not output a version. This is unexpected." exit 1 fi - # Read changelog from file to avoid E2BIG. # Produced by craft >= 2.22.0; older versions don't produce the file # and the publish issue will be created without a changelog section. @@ -238,7 +260,14 @@ runs: CHANGELOG="${CHANGELOG:0:$MAX_CHANGELOG_CHARS}"$'\n\n---\n*Changelog truncated for issue body.*' fi - title="publish: ${GITHUB_REPOSITORY}${SUBDIRECTORY}@${RESOLVED_VERSION}" + workspace_title="" + version_separator="@" + if [[ -n "$WORKSPACE" ]]; then + workspace_json=$(jq -Rn --arg workspace "$WORKSPACE" '$workspace') + workspace_title=" [workspace: ${workspace_json}]" + version_separator=" @" + fi + title="publish: ${GITHUB_REPOSITORY}${SUBDIRECTORY}${workspace_title}${version_separator}${RESOLVED_VERSION}" # Check if issue already exists by listing all open issues and filtering by exact title match. # We avoid GitHub search API to bypass indexing delays and query syntax edge cases. diff --git a/src/__tests__/action.test.ts b/src/__tests__/action.test.ts new file mode 100644 index 000000000..742264627 --- /dev/null +++ b/src/__tests__/action.test.ts @@ -0,0 +1,144 @@ +import { + chmodSync, + mkdtempSync, + mkdirSync, + readFileSync, + rmSync, + writeFileSync, +} from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { spawnSync } from 'child_process'; + +import { load } from 'js-yaml'; +import { afterEach, expect, test } from 'vitest'; + +interface ActionStep { + env?: Record; + name?: string; + run?: string; +} + +function getActionSteps(): ActionStep[] { + const action = load( + readFileSync(join(__dirname, '../../action.yml'), 'utf8'), + ) as { + runs?: { steps?: ActionStep[] }; + }; + return action.runs?.steps || []; +} + +function getActionStep(name: string): ActionStep { + const step = getActionSteps().find(step => step.name === name); + if (!step?.run) { + throw new Error(`Missing ${name} action step`); + } + return step; +} + +const tempDirectories: string[] = []; + +function createActionEnvironment() { + const directory = mkdtempSync(join(tmpdir(), 'craft-action-test-')); + tempDirectories.push(directory); + const binDirectory = join(directory, 'bin'); + const craftCalls = join(directory, 'craft-calls'); + const gitCalls = join(directory, 'git-calls'); + const output = join(directory, 'github-output'); + mkdirSync(binDirectory); + writeFileSync(craftCalls, ''); + writeFileSync(gitCalls, ''); + writeFileSync(output, ''); + writeFileSync( + join(binDirectory, 'craft'), + '#!/usr/bin/env bash\nprintf "%s\\n" "$*" >> "$CRAFT_CALLS"\nif [[ "$1" == "targets" ]]; then\n printf \'["github"]\'\nfi\n', + ); + writeFileSync( + join(binDirectory, 'git'), + '#!/usr/bin/env bash\nprintf "%s\\n" "$*" >> "$GIT_CALLS"\n', + ); + chmodSync(join(binDirectory, 'craft'), 0o755); + chmodSync(join(binDirectory, 'git'), 0o755); + + return { binDirectory, craftCalls, directory, gitCalls, output }; +} + +function runActionStep( + stepName: string, + workspace: string, + environment: ReturnType, +) { + return spawnSync('bash', ['-e', '-c', getActionStep(stepName).run!], { + cwd: environment.directory, + env: { + ...process.env, + CRAFT_CALLS: environment.craftCalls, + CRAFT_CONFIG_FROM_MERGE_TARGET: '', + GITHUB_OUTPUT: environment.output, + GIT_CALLS: environment.gitCalls, + MERGE_TARGET: '', + PATH: `${environment.binDirectory}:${process.env.PATH}`, + VERSION: '', + WORKSPACE: workspace, + }, + }); +} + +afterEach(() => { + for (const directory of tempDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } +}); + +test('passes option-looking workspace names inline to Craft', () => { + const environment = createActionEnvironment(); + const workspace = '--config-from=untrusted'; + + expect(runActionStep('Craft Prepare', workspace, environment).status).toBe(0); + expect( + runActionStep('Read Craft Targets', workspace, environment).status, + ).toBe(0); + + expect(readFileSync(environment.craftCalls, 'utf8')).toBe( + 'prepare --workspace=--config-from=untrusted\ntargets --workspace=--config-from=untrusted\n', + ); +}); + +test('forwards workspace input to every Craft command', () => { + expect(getActionStep('Craft Prepare').env?.WORKSPACE).toBe( + '${{ inputs.workspace }}', + ); + expect(getActionStep('Read Craft Targets').env?.WORKSPACE).toBe( + '${{ inputs.workspace }}', + ); +}); + +test.each([ + ['control', 'cli\tnext'], + ['format', 'cli\u202enext'], + ['line separator', 'cli\u2028next'], + ['paragraph separator', 'cli\u2029next'], +])( + 'rejects %s characters before every action side effect', + (_name, workspace) => { + const environment = createActionEnvironment(); + + expect(getActionSteps()[0]?.name).toBe('Validate workspace'); + expect( + runActionStep('Validate workspace', workspace, environment).status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); + }, +); + +test.each(['', 'cli-\u65e5\u672c\u8a9e'])( + 'accepts safe workspace input %j', + workspace => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', workspace, environment).status, + ).toBe(0); + }, +); diff --git a/src/commands/__tests__/publish.test.ts b/src/commands/__tests__/publish.test.ts index f2bd73497..081927495 100644 --- a/src/commands/__tests__/publish.test.ts +++ b/src/commands/__tests__/publish.test.ts @@ -2,11 +2,13 @@ import { vi, describe, test, expect, beforeEach, type Mock } from 'vitest'; import { join as pathJoin } from 'path'; import { spawnProcess, hasExecutable } from '../../utils/system'; import { + getPublishStateGitHubConfig, runPostReleaseCommand, handleReleaseBranch, MergeConflictError, PushError, } from '../publish'; +import { getPublishStateFilename } from '../../utils/publishState'; import type { SimpleGit } from 'simple-git'; vi.mock('../../utils/system'); @@ -162,6 +164,51 @@ describe('runPostReleaseCommand', () => { }); }); +describe('getPublishStateGitHubConfig', () => { + test('uses the controller checkout repository only for state identity', () => { + const resolvedWorkspaceGithub = { + owner: 'release-owner', + repo: 'release-repo', + projectPath: 'packages/cli', + }; + const stateGithub = getPublishStateGitHubConfig( + resolvedWorkspaceGithub, + 'getsentry/toolkit', + ); + + expect(stateGithub).toEqual({ owner: 'getsentry', repo: 'toolkit' }); + expect( + getPublishStateFilename( + '1.2.3', + stateGithub, + '/github/workspace/__repo__/packages/cli', + 'cli', + ), + ).toBe( + getPublishStateFilename( + '1.2.3', + { owner: 'getsentry', repo: 'toolkit' }, + '/github/workspace/__repo__/packages/cli', + 'cli', + ), + ); + }); + + test('keeps the resolved GitHub configuration without controller state identity', () => { + const githubConfig = { owner: 'release-owner', repo: 'release-repo' }; + + expect(getPublishStateGitHubConfig(githubConfig, undefined)).toBe( + githubConfig, + ); + }); + + test('rejects malformed controller state repository values', () => { + expect(() => + getPublishStateGitHubConfig(null, 'getsentry/toolkit/extra'), + ).toThrow('CRAFT_PUBLISH_STATE_GITHUB_REPO'); + }); +}); + describe('handleReleaseBranch', () => { /** * Creates a mock SimpleGit instance where each method returns diff --git a/src/commands/publish.ts b/src/commands/publish.ts index 30bb83484..7158a9e1c 100644 --- a/src/commands/publish.ts +++ b/src/commands/publish.ts @@ -18,7 +18,10 @@ import { getActiveWorkspace, } from '../config'; import { formatTable, logger } from '../logger'; -import { TargetConfig } from '../schemas/project_config'; +import { + type GitHubGlobalConfig, + TargetConfig, +} from '../schemas/project_config'; import { getAllTargetNames, getTargetByName, SpecialTarget } from '../targets'; import { BaseTarget } from '../targets/base'; import { @@ -166,6 +169,32 @@ export interface PublishState { }; } +/** + * The Publish controller prepopulates a secure state file using the issue's + * checkout repository. That can differ from a workspace's release GitHub + * configuration, so this override is deliberately limited to state identity. + */ +export function getPublishStateGitHubConfig( + githubConfig: GitHubGlobalConfig | null, + stateRepository: string | undefined = process.env + .CRAFT_PUBLISH_STATE_GITHUB_REPO, +): GitHubGlobalConfig | null { + if (!stateRepository) { + return githubConfig; + } + + const match = stateRepository.match( + /^(?[A-Za-z0-9_.-]+)\/(?[A-Za-z0-9_.-]+)$/, + ); + if (!match?.groups) { + throw new ConfigurationError( + 'CRAFT_PUBLISH_STATE_GITHUB_REPO must be a GitHub owner/repository pair.', + ); + } + + return { owner: match.groups.owner, repo: match.groups.repo }; +} + /** * Checks that the passed version is a valid version string * @@ -696,7 +725,7 @@ export async function publishMain(argv: PublishOptions): Promise { } const publishStateFile = getPublishStatePath( newVersion, - publishStateGithubConfig, + getPublishStateGitHubConfig(publishStateGithubConfig), process.cwd(), getActiveWorkspace(), ); From 48620564f86d8724982cddb6506c040235fe8954 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 2 Sep 2026 16:20:57 +0000 Subject: [PATCH 2/2] feat: support compact release workspaces --- action.yml | 18 +-- docs/src/content/docs/targets/github.md | 36 ++++- src/__tests__/action.test.ts | 140 +++++++++++++++--- src/__tests__/config.test.ts | 136 ++++++++++++----- src/commands/__tests__/publish.test.ts | 27 ++++ src/commands/publish.ts | 18 ++- src/commands/workspace.ts | 13 ++ .../workspace_cmds/__tests__/list.test.ts | 31 ++++ src/commands/workspace_cmds/list.ts | 9 ++ src/config.ts | 48 ++++-- src/index.ts | 2 + src/schemas/project_config.ts | 65 +++++--- 12 files changed, 438 insertions(+), 105 deletions(-) create mode 100644 src/commands/workspace.ts create mode 100644 src/commands/workspace_cmds/__tests__/list.test.ts create mode 100644 src/commands/workspace_cmds/list.ts diff --git a/action.yml b/action.yml index 6344db2fe..94f3158cc 100644 --- a/action.yml +++ b/action.yml @@ -74,10 +74,15 @@ runs: - name: Validate workspace shell: bash env: + PATH_INPUT: ${{ inputs.path }} WORKSPACE: ${{ inputs.workspace }} run: | - if [[ -n "$WORKSPACE" ]] && node -e 'process.exit(/[\p{Cc}\p{Cf}\p{Zl}\p{Zp}]/u.test(process.env.WORKSPACE) ? 0 : 1)'; then - echo "::error::Workspace names cannot contain Unicode control, format, or separator characters." + if [[ -n "$WORKSPACE" && "$PATH_INPUT" != '.' ]]; then + echo "::error::The path and workspace inputs cannot be used together." + exit 1 + fi + if [[ -n "$WORKSPACE" ]] && ! node -e 'process.exit(/^(?!\.{1,2}$)(?!__proto__$)(?!-)[A-Za-z0-9_.-]+$/.test(process.env.WORKSPACE) ? 0 : 1)'; then + echo "::error::Workspace names must use only ASCII letters, digits, periods, underscores, and hyphens." exit 1 fi @@ -260,14 +265,7 @@ runs: CHANGELOG="${CHANGELOG:0:$MAX_CHANGELOG_CHARS}"$'\n\n---\n*Changelog truncated for issue body.*' fi - workspace_title="" - version_separator="@" - if [[ -n "$WORKSPACE" ]]; then - workspace_json=$(jq -Rn --arg workspace "$WORKSPACE" '$workspace') - workspace_title=" [workspace: ${workspace_json}]" - version_separator=" @" - fi - title="publish: ${GITHUB_REPOSITORY}${SUBDIRECTORY}${workspace_title}${version_separator}${RESOLVED_VERSION}" + title="publish: ${GITHUB_REPOSITORY}${SUBDIRECTORY}${WORKSPACE:+/$WORKSPACE}@${RESOLVED_VERSION}" # Check if issue already exists by listing all open issues and filtering by exact title match. # We avoid GitHub search API to bypass indexing delays and query syntax edge cases. diff --git a/docs/src/content/docs/targets/github.md b/docs/src/content/docs/targets/github.md index 9f522f2b8..e64bcbac5 100644 --- a/docs/src/content/docs/targets/github.md +++ b/docs/src/content/docs/targets/github.md @@ -73,12 +73,40 @@ targets: Releasing `1.2.3` for each product then produces the tags `cli@1.2.3` / `mcp@1.2.3` on release branches `release/cli/1.2.3` / `release/mcp/1.2.3` — no collisions. -:::note[Coming soon: first-class workspaces] -A single, target-agnostic top-level `workspaces:` model (with an explicit `--workspace` selector) is planned so that all of a repo's products can be managed from one `.craft.yml`. It will supersede the per-file convention above. Track progress in [getsentry/craft#842](https://github.com/getsentry/craft/issues/842). -::: +## Release Workspaces + +Use top-level `workspaces:` to define independently versioned release units in +one repository. Select one explicitly with `--workspace ` or +`CRAFT_WORKSPACE`: + +```yaml +minVersion: 2.29.0 +github: + owner: getsentry + repo: toolkit +workspaces: + cli: + releaseBranchPrefix: release/cli + targets: + - name: github + tagPrefix: "cli@" + mcp: + releaseBranchPrefix: release/mcp + targets: + - name: github + tagPrefix: "mcp@" +``` + +Workspace names use ASCII letters, digits, periods, underscores, and hyphens. +They are release units, not npm package workspaces. A workspace cannot use +`github.projectPath`, and a workflow cannot provide both a `path` and a +workspace. This keeps each publish request unambiguous. + +The `craft workspace list` command prints the exact configured workspace names +as a JSON array for automation. :::caution -Declaring **multiple** `github` targets with **different** `tagPrefix` values in a *single* config is currently ambiguous: Craft uses the first prefix for read-path operations and logs a warning. Until workspaces land, use a separate `.craft.yml` per product. +Declaring **multiple** `github` targets with **different** `tagPrefix` values in a single release unit is ambiguous: Craft uses the first prefix for read-path operations and logs a warning. Use separate release workspaces for independent products. ::: :::note[Known limitation: the GitHub "Latest" badge is repo-wide] diff --git a/src/__tests__/action.test.ts b/src/__tests__/action.test.ts index 742264627..8ca535afc 100644 --- a/src/__tests__/action.test.ts +++ b/src/__tests__/action.test.ts @@ -43,10 +43,12 @@ function createActionEnvironment() { tempDirectories.push(directory); const binDirectory = join(directory, 'bin'); const craftCalls = join(directory, 'craft-calls'); + const ghTitles = join(directory, 'gh-titles'); const gitCalls = join(directory, 'git-calls'); const output = join(directory, 'github-output'); mkdirSync(binDirectory); writeFileSync(craftCalls, ''); + writeFileSync(ghTitles, ''); writeFileSync(gitCalls, ''); writeFileSync(output, ''); writeFileSync( @@ -57,16 +59,69 @@ function createActionEnvironment() { join(binDirectory, 'git'), '#!/usr/bin/env bash\nprintf "%s\\n" "$*" >> "$GIT_CALLS"\n', ); + writeFileSync( + join(binDirectory, 'gh'), + `#!/usr/bin/env bash +if [[ "$*" == *"issue list"* ]]; then + printf '[]' + exit 0 +fi +if [[ "$*" == *"issue create"* ]]; then + while [[ $# -gt 0 ]]; do + if [[ "$1" == '--title' ]]; then + printf '%s\\n' "$2" >> "$GH_TITLES" + break + fi + shift + done + printf 'https://github.com/getsentry/publish/issues/1\\n' +fi +`, + ); chmodSync(join(binDirectory, 'craft'), 0o755); + chmodSync(join(binDirectory, 'gh'), 0o755); chmodSync(join(binDirectory, 'git'), 0o755); - return { binDirectory, craftCalls, directory, gitCalls, output }; + return { binDirectory, craftCalls, directory, ghTitles, gitCalls, output }; +} + +function runRequestPublish( + workspace: string, + environment: ReturnType, +) { + return spawnSync( + 'bash', + ['-e', '-c', getActionStep('Request publish').run!], + { + cwd: environment.directory, + env: { + ...process.env, + CHANGELOG_FILE: '', + GITHUB_ACTOR: 'byk', + GITHUB_OUTPUT: environment.output, + GITHUB_REPOSITORY: 'getsentry/toolkit', + GH_TITLES: environment.ghTitles, + MERGE_TARGET: '(default)', + PATH: `${environment.binDirectory}:${process.env.PATH}`, + PUBLISH_REPO: 'getsentry/publish', + RELEASE_BRANCH: 'release/1.2.3', + RELEASE_PREVIOUS_TAG: '1.2.2', + RELEASE_SHA: 'abc123', + RESOLVED_VERSION: '1.2.3', + SUBDIRECTORY: '', + TARGETS: ' - [ ] github', + WORKSPACE: workspace, + }, + }, + ); } function runActionStep( stepName: string, workspace: string, environment: ReturnType, + pathInput = '.', + locale = 'C', ) { return spawnSync('bash', ['-e', '-c', getActionStep(stepName).run!], { cwd: environment.directory, @@ -76,8 +131,10 @@ function runActionStep( CRAFT_CONFIG_FROM_MERGE_TARGET: '', GITHUB_OUTPUT: environment.output, GIT_CALLS: environment.gitCalls, + LC_ALL: locale, MERGE_TARGET: '', PATH: `${environment.binDirectory}:${process.env.PATH}`, + PATH_INPUT: pathInput, VERSION: '', WORKSPACE: workspace, }, @@ -90,21 +147,10 @@ afterEach(() => { } }); -test('passes option-looking workspace names inline to Craft', () => { - const environment = createActionEnvironment(); - const workspace = '--config-from=untrusted'; - - expect(runActionStep('Craft Prepare', workspace, environment).status).toBe(0); - expect( - runActionStep('Read Craft Targets', workspace, environment).status, - ).toBe(0); - - expect(readFileSync(environment.craftCalls, 'utf8')).toBe( - 'prepare --workspace=--config-from=untrusted\ntargets --workspace=--config-from=untrusted\n', - ); -}); - test('forwards workspace input to every Craft command', () => { + expect(getActionStep('Validate workspace').env?.PATH_INPUT).toBe( + '${{ inputs.path }}', + ); expect(getActionStep('Craft Prepare').env?.WORKSPACE).toBe( '${{ inputs.workspace }}', ); @@ -118,6 +164,7 @@ test.each([ ['format', 'cli\u202enext'], ['line separator', 'cli\u2028next'], ['paragraph separator', 'cli\u2029next'], + ['non-ASCII', 'cli-é'], ])( 'rejects %s characters before every action side effect', (_name, workspace) => { @@ -132,13 +179,70 @@ test.each([ }, ); -test.each(['', 'cli-\u65e5\u672c\u8a9e'])( - 'accepts safe workspace input %j', +test('rejects non-ASCII workspace input in a UTF-8 locale', () => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', 'cli-é', environment, '.', 'en_US.utf8') + .status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); +}); + +test.each(['', 'cli-v2'])('accepts safe workspace input %j', workspace => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', workspace, environment).status, + ).toBe(0); +}); + +test('rejects a path and workspace together before every action side effect', () => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', 'cli', environment, 'packages/cli') + .status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); +}); + +test('rejects workspace names outside the compact title grammar', () => { + const environment = createActionEnvironment(); + + expect( + runActionStep('Validate workspace', 'cli/v2', environment).status, + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); +}); + +test.each(['.', '..', '__proto__', '-foo', '--config'])( + 'rejects unsafe workspace name %j', workspace => { const environment = createActionEnvironment(); expect( runActionStep('Validate workspace', workspace, environment).status, - ).toBe(0); + ).toBe(1); + expect(readFileSync(environment.gitCalls, 'utf8')).toBe(''); + expect(readFileSync(environment.craftCalls, 'utf8')).toBe(''); }, ); + +test('uses the compact workspace path in publish request titles', () => { + const rootEnvironment = createActionEnvironment(); + const workspaceEnvironment = createActionEnvironment(); + + expect(runRequestPublish('', rootEnvironment).status).toBe(0); + expect(runRequestPublish('cli', workspaceEnvironment).status).toBe(0); + + expect(readFileSync(rootEnvironment.ghTitles, 'utf8')).toBe( + 'publish: getsentry/toolkit@1.2.3\n', + ); + expect(readFileSync(workspaceEnvironment.ghTitles, 'utf8')).toBe( + 'publish: getsentry/toolkit/cli@1.2.3\n', + ); +}); diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index 90e145bfe..72e0785f9 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -1,4 +1,7 @@ -import { describe, test, expect, vi, afterEach } from 'vitest'; +import { describe, test, expect, vi, afterEach, beforeEach } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; /** * Tests of our ability to read craft config files. (This is NOT general test * configuration). @@ -11,6 +14,7 @@ import { setActiveWorkspace, getActiveWorkspace, getVersioningPolicy, + getWorkspaceNames, WORKSPACES_MIN_VERSION, } from '../config'; import { CraftProjectConfigSchema } from '../schemas/project_config'; @@ -134,7 +138,6 @@ describe('noMerge config', () => { workspaces: { cli: { releaseBranchPrefix: 'release/cli', - github: { projectPath: 'cli' }, targets: [{ name: 'github', tagPrefix: 'cli@' }], }, mcp: { @@ -146,16 +149,74 @@ describe('noMerge config', () => { expect(validateConfiguration(data)).toEqual(data); }); - test('allows a workspace github override without owner/repo', () => { + test('allows a workspace github owner/repo override', () => { const data = { workspaces: { - cli: { github: { projectPath: 'cli' } }, + cli: { github: { owner: 'getsentry', repo: 'toolkit' } }, }, }; - // Workspace github is partial; owner/repo are inherited, not required here. + // Workspace github is partial; owner/repo are not required together here. expect(() => validateConfiguration(data)).not.toThrow(); }); + + test('allows legacy workspace names', () => { + expect(() => + validateConfiguration({ workspaces: { 'cli/v2': {} } }), + ).not.toThrow(); + }); + + test.each(['.', '..'])('rejects traversal workspace name %j', name => { + expect(() => validateConfiguration({ workspaces: { [name]: {} } })).toThrow( + 'Workspace names cannot be "." or "..".', + ); + }); + + test('rejects the __proto__ workspace key', () => { + expect(() => + loadConfigurationFromString( + [ + `minVersion: ${WORKSPACES_MIN_VERSION}`, + 'workspaces:', + ' __proto__: {}', + ].join('\n'), + ), + ).toThrow('Workspace name "__proto__" is not supported.'); + }); + + test('rejects workspace github.projectPath', () => { + expect(() => + validateConfiguration({ + workspaces: { cli: { github: { projectPath: 'cli' } } }, + }), + ).toThrow('Workspace github.projectPath is not supported.'); + }); + + test('rejects a base github.projectPath when workspaces are configured', () => { + expect(() => + validateConfiguration({ + github: { + owner: 'getsentry', + repo: 'toolkit', + projectPath: 'packages/cli', + }, + workspaces: { cli: {} }, + }), + ).toThrow('Workspace configurations cannot use github.projectPath.'); + }); + + test('allows github.projectPath with an empty workspace map', () => { + expect(() => + validateConfiguration({ + github: { + owner: 'getsentry', + repo: 'toolkit', + projectPath: 'packages/cli', + }, + workspaces: {}, + }), + ).not.toThrow(); + }); }); describe('getGitTagPrefix', () => { @@ -226,7 +287,18 @@ describe('getGitTagPrefix', () => { }); describe('workspaces', () => { + let originalCwd: string; + const temporaryDirectories: string[] = []; + + beforeEach(() => { + originalCwd = process.cwd(); + }); + afterEach(() => { + process.chdir(originalCwd); + for (const directory of temporaryDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } setActiveWorkspace(undefined); vi.restoreAllMocks(); }); @@ -240,8 +312,6 @@ describe('workspaces', () => { 'workspaces:', ' cli:', ' releaseBranchPrefix: release/cli', - ' github:', - ' projectPath: cli', ' targets:', ' - name: github', ' tagPrefix: "cli@"', @@ -269,12 +339,7 @@ describe('workspaces', () => { // Overridden by the workspace. expect(config.releaseBranchPrefix).toBe('release/cli'); expect(getGitTagPrefix()).toBe('cli@'); - // github is shallow-merged: owner/repo inherited, projectPath overridden. - expect(config.github).toEqual({ - owner: 'getsentry', - repo: 'toolkit', - projectPath: 'cli', - }); + expect(config.github).toEqual({ owner: 'getsentry', repo: 'toolkit' }); // Inherited from the top level. expect(config.changelog).toBe('CHANGELOG.md'); // `workspaces` is stripped from the resolved config. @@ -287,7 +352,6 @@ describe('workspaces', () => { expect(config.releaseBranchPrefix).toBe('release/mcp'); expect(getGitTagPrefix()).toBe('mcp@'); expect(getVersioningPolicy()).toBe('calver'); - // mcp did not override github.projectPath, so it inherits base github only. expect(config.github).toEqual({ owner: 'getsentry', repo: 'toolkit' }); }); @@ -298,6 +362,25 @@ describe('workspaces', () => { ); }); + test('lists raw workspace names without requiring a selection', () => { + setActiveWorkspace(undefined); + const directory = mkdtempSync(join(tmpdir(), 'craft-workspaces-')); + temporaryDirectories.push(directory); + const configPath = join(directory, '.craft.yml'); + writeFileSync(configPath, WS_CONFIG); + process.chdir(directory); + + expect(getWorkspaceNames()).toEqual(['cli', 'mcp']); + writeFileSync( + configPath, + ['minVersion: 2.14.0', 'workspaces:', ' cli: {}'].join('\n'), + ); + + expect(() => getWorkspaceNames()).toThrow( + `requires minVersion >= ${WORKSPACES_MIN_VERSION}`, + ); + }); + test('errors on an unknown workspace name', () => { setActiveWorkspace('nope'); expect(() => loadConfigurationFromString(WS_CONFIG)).toThrow( @@ -366,28 +449,7 @@ describe('workspaces', () => { expect(config.targets).toEqual([{ name: 'github', tagPrefix: 'cli@' }]); }); - test('does not produce an incomplete github when base has none', () => { - // A workspace that sets only github.projectPath, with NO top-level github, - // must NOT yield a truthy-but-incomplete github object (missing owner/repo) - // — that would make getGlobalGitHubConfig skip its git-remote fallback. - setActiveWorkspace('cli'); - const config = loadConfigurationFromString( - [ - `minVersion: ${WORKSPACES_MIN_VERSION}`, - 'workspaces:', - ' cli:', - ' github:', - ' projectPath: cli', - ' targets:', - ' - name: github', - ' tagPrefix: "cli@"', - ].join('\n'), - ); - // Incomplete github is dropped so git-remote detection can still run. - expect(config.github).toBeUndefined(); - }); - - test('keeps github when workspace override completes owner/repo', () => { + test('shallow-merges a workspace github owner/repo override', () => { setActiveWorkspace('cli'); const config = loadConfigurationFromString( [ @@ -397,7 +459,6 @@ describe('workspaces', () => { ' github:', ' owner: getsentry', ' repo: toolkit', - ' projectPath: cli', ' targets:', ' - name: github', ].join('\n'), @@ -405,7 +466,6 @@ describe('workspaces', () => { expect(config.github).toEqual({ owner: 'getsentry', repo: 'toolkit', - projectPath: 'cli', }); }); }); diff --git a/src/commands/__tests__/publish.test.ts b/src/commands/__tests__/publish.test.ts index 081927495..3e866f72d 100644 --- a/src/commands/__tests__/publish.test.ts +++ b/src/commands/__tests__/publish.test.ts @@ -3,6 +3,7 @@ import { join as pathJoin } from 'path'; import { spawnProcess, hasExecutable } from '../../utils/system'; import { getPublishStateGitHubConfig, + getRevisionBranchName, runPostReleaseCommand, handleReleaseBranch, MergeConflictError, @@ -209,6 +210,32 @@ describe('getPublishStateGitHubConfig', () => { }); }); +describe('getRevisionBranchName', () => { + test('returns the named ref for a revision when available', async () => { + const git = { + raw: vi.fn().mockResolvedValue('release/1.2.3\n'), + } as unknown as SimpleGit; + + await expect(getRevisionBranchName(git, 'abc123')).resolves.toBe( + 'release/1.2.3', + ); + expect(git.raw).toHaveBeenCalledWith( + 'name-rev', + '--name-only', + '--no-undefined', + 'abc123', + ); + }); + + test('allows a detached CI-approved revision', async () => { + const git = { + raw: vi.fn().mockRejectedValue(new Error('Could not get ref name')), + } as unknown as SimpleGit; + + await expect(getRevisionBranchName(git, 'abc123')).resolves.toBe(''); + }); +}); + describe('handleReleaseBranch', () => { /** * Creates a mock SimpleGit instance where each method returns diff --git a/src/commands/publish.ts b/src/commands/publish.ts index 7158a9e1c..dad43f31a 100644 --- a/src/commands/publish.ts +++ b/src/commands/publish.ts @@ -631,9 +631,7 @@ export async function publishMain(argv: PublishOptions): Promise { let branchName; if (rev) { logger.debug(`Trying to get branch name for provided revision: "${rev}"`); - branchName = ( - await git.raw('name-rev', '--name-only', '--no-undefined', rev) - ).trim(); + branchName = await getRevisionBranchName(git, rev); checkoutTarget = branchName || rev; logger.debug('Checking out revision', checkoutTarget); await git.checkout(checkoutTarget); @@ -932,6 +930,20 @@ export async function publishMain(argv: PublishOptions): Promise { await runPostReleaseCommand(newVersion, config.postReleaseCommand); } +export async function getRevisionBranchName( + git: SimpleGit, + revision: string, +): Promise { + try { + return ( + await git.raw('name-rev', '--name-only', '--no-undefined', revision) + ).trim(); + } catch { + // A CI-approved SHA can be checked out detached without a named ref. + return ''; + } +} + export const handler = async (args: { [argName: string]: any; }): Promise => { diff --git a/src/commands/workspace.ts b/src/commands/workspace.ts new file mode 100644 index 000000000..3fd3ba892 --- /dev/null +++ b/src/commands/workspace.ts @@ -0,0 +1,13 @@ +import { Argv, CommandBuilder } from 'yargs'; + +import * as list from './workspace_cmds/list'; + +export const command = ['workspace ']; +export const description = 'Manage release workspaces'; + +export const builder: CommandBuilder = (yargs: Argv) => + yargs.demandCommand().command(list); + +export const handler = (): void => { + /* pass */ +}; diff --git a/src/commands/workspace_cmds/__tests__/list.test.ts b/src/commands/workspace_cmds/__tests__/list.test.ts new file mode 100644 index 000000000..e979cb180 --- /dev/null +++ b/src/commands/workspace_cmds/__tests__/list.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, test, vi } from 'vitest'; + +vi.mock('../../../config', () => ({ + getWorkspaceNames: vi.fn(), +})); +vi.mock('../../../utils/strings', () => ({ + formatJson: vi.fn(value => JSON.stringify(value)), +})); + +import { getWorkspaceNames } from '../../../config'; +import { handler } from '../list'; + +describe('workspace list command', () => { + test('prints exact configured workspace names', () => { + vi.mocked(getWorkspaceNames).mockReturnValue(['cli', 'mcp.v2']); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + + handler(); + + expect(log).toHaveBeenCalledWith('["cli","mcp.v2"]'); + }); + + test('prints an empty array when no workspaces are configured', () => { + vi.mocked(getWorkspaceNames).mockReturnValue([]); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + + handler(); + + expect(log).toHaveBeenCalledWith('[]'); + }); +}); diff --git a/src/commands/workspace_cmds/list.ts b/src/commands/workspace_cmds/list.ts new file mode 100644 index 000000000..05816ab6f --- /dev/null +++ b/src/commands/workspace_cmds/list.ts @@ -0,0 +1,9 @@ +import { getWorkspaceNames } from '../../config'; +import { formatJson } from '../../utils/strings'; + +export const command = ['list']; +export const description = 'List defined release workspaces as a JSON array'; + +export function handler(): void { + console.log(formatJson(getWorkspaceNames())); +} diff --git a/src/config.ts b/src/config.ts index c323addf9..cebbdc8d0 100644 --- a/src/config.ts +++ b/src/config.ts @@ -103,8 +103,8 @@ export function getActiveWorkspace(): string | undefined { * top-level value (shallow override; a workspace either declares a field or * inherits it wholesale — we do not deep-merge arrays/objects, to keep * behavior predictable). - * - `github` is shallow-merged (owner/repo/projectPath) so a workspace can - * override just `projectPath` while inheriting owner/repo. + * - `github` is shallow-merged (owner/repo) so a workspace can override either + * value while inheriting the other. * - `minVersion` and `workspaces` themselves are stripped from the result. */ function resolveWorkspaceConfig( @@ -135,8 +135,8 @@ function resolveWorkspaceConfig( continue; } if (key === 'github') { - // Shallow-merge github so a workspace can override a single field - // (e.g. just projectPath) while inheriting owner/repo from the base. + // Shallow-merge github so a workspace can override owner or repo while + // inheriting the other value from the base configuration. const mergedGithub = { ...(base.github as GitHubGlobalConfig | undefined), ...(value as Partial), @@ -144,9 +144,7 @@ function resolveWorkspaceConfig( // Only adopt the merged github if it is complete (has owner + repo). // Otherwise leave `github` unset so getGlobalGitHubConfig() can still // fall back to git-remote detection instead of seeing a truthy-but- - // incomplete object and skipping the fallback. (A workspace that only - // sets projectPath without a base github relies on git detection for - // owner/repo, exactly like a top-level config with no github block.) + // incomplete object and skipping the fallback. if (mergedGithub.owner && mergedGithub.repo) { resolved.github = mergedGithub as GitHubGlobalConfig; } else { @@ -184,6 +182,15 @@ function isVersionGteMinVersion( ); } +function checkWorkspacesMinVersion(config: CraftProjectConfig): void { + if (!isVersionGteMinVersion(config.minVersion, WORKSPACES_MIN_VERSION)) { + throw new ConfigurationError( + `Using "workspaces" requires minVersion >= ${WORKSPACES_MIN_VERSION} ` + + 'in the configuration file.', + ); + } +} + /** * SemVer build metadata does not affect precedence, but the comparison helper * intentionally rejects versions carrying it. Strip it before compatibility @@ -229,12 +236,7 @@ function applyWorkspaceSelection( } // Gate the feature behind minVersion, mirroring auto-versioning. - if (!isVersionGteMinVersion(config.minVersion, WORKSPACES_MIN_VERSION)) { - throw new ConfigurationError( - `Using "workspaces" requires minVersion >= ${WORKSPACES_MIN_VERSION} ` + - 'in the configuration file.', - ); - } + checkWorkspacesMinVersion(config); return resolveWorkspaceConfig(config, _activeWorkspaceName); } @@ -363,6 +365,26 @@ export function loadConfigurationFromString( return _configCache; } +/** + * Lists workspace names from the raw validated configuration without applying a + * selection. This allows external controllers to discover valid names before + * choosing a workspace. + */ +export function getWorkspaceNames(): string[] { + const configPath = getConfigFilePath(); + const rawConfig = load(readFileSync(configPath, 'utf-8')) as Record< + string, + any + >; + const parsed = validateConfiguration(rawConfig); + checkMinimalConfigVersion(parsed); + const workspaceNames = Object.keys(parsed.workspaces || {}); + if (workspaceNames.length > 0) { + checkWorkspacesMinVersion(parsed); + } + return workspaceNames; +} + /** * Checks that the current "craft" version is compatible with the configuration * diff --git a/src/index.ts b/src/index.ts index ca25418b8..b7db51a1a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -26,6 +26,7 @@ import * as targets from './commands/targets'; import * as config from './commands/config'; import * as artifacts from './commands/artifacts'; import * as changelog from './commands/changelog'; +import * as workspace from './commands/workspace'; function printVersion(): void { if (!process.argv.includes('-v') && !process.argv.includes('--version')) { @@ -108,6 +109,7 @@ async function main(): Promise { .command(config) .command(artifacts) .command(changelog) + .command(workspace) .demandCommand() .version(getPackageVersion()) .alias('v', 'version') diff --git a/src/schemas/project_config.ts b/src/schemas/project_config.ts index e8f02b7f6..e68a313b5 100644 --- a/src/schemas/project_config.ts +++ b/src/schemas/project_config.ts @@ -202,34 +202,61 @@ const releaseUnitFields = { * A workspace mirrors the release-relevant subset of the top-level config; * every field is optional and inherits the top-level value when omitted. The * `github` block is *partial* (all fields optional) so a workspace can override - * just `projectPath` (or `owner`/`repo`) while inheriting the rest from the - * top-level `github`. + * `owner` and/or `repo` while inheriting the rest from the top-level `github`. */ export const WorkspaceSchema = z.object({ ...releaseUnitFields, - github: GitHubGlobalConfigSchema.partial().optional(), + github: GitHubGlobalConfigSchema.partial() + .refine(github => github.projectPath === undefined, { + message: 'Workspace github.projectPath is not supported.', + }) + .optional(), }); export type Workspace = z.infer; +const WorkspaceNameSchema = z + .string() + // Assigning this key to a regular object mutates its prototype instead of + // preserving an own workspace entry. + .refine(name => name !== '__proto__', { + message: 'Workspace name "__proto__" is not supported.', + }) + .refine(name => name !== '.' && name !== '..', { + message: 'Workspace names cannot be "." or "..".', + }); + /** * Craft project-specific configuration */ -export const CraftProjectConfigSchema = z.object({ - ...releaseUnitFields, - minVersion: z - .string() - .regex(/^\d+\.\d+\.\d+.*$/) - .optional(), - /** - * Named, independently-versioned release units within a single repository. - * - * When present, a release run must select one via `--workspace ` (or - * `CRAFT_WORKSPACE`). The selected workspace's fields override the top-level - * ones. When absent, craft behaves exactly as before (the top-level config is - * the single implicit release unit) — fully backward compatible. - */ - workspaces: z.record(z.string(), WorkspaceSchema).optional(), -}); +export const CraftProjectConfigSchema = z + .object({ + ...releaseUnitFields, + minVersion: z + .string() + .regex(/^\d+\.\d+\.\d+.*$/) + .optional(), + /** + * Named, independently-versioned release units within a single repository. + * + * When present, a release run must select one via `--workspace ` (or + * `CRAFT_WORKSPACE`). The selected workspace's fields override the top-level + * ones. When absent, craft behaves exactly as before (the top-level config is + * the single implicit release unit) — fully backward compatible. + */ + workspaces: z.record(WorkspaceNameSchema, WorkspaceSchema).optional(), + }) + .superRefine((config, context) => { + if ( + Object.keys(config.workspaces || {}).length > 0 && + config.github?.projectPath !== undefined + ) { + context.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Workspace configurations cannot use github.projectPath.', + path: ['github', 'projectPath'], + }); + } + }); export type CraftProjectConfig = z.infer;