Dashboard › craft › Session 117pES7LG4CR
117pES7LG4CRSwQikOkay, let's pick this up. PR 848 is ready with some review feedback. Let's address them, give it a final adversarial review and then merge and move forward. Let's see if https://github.com/getsentry/craft/pull/844 is still relevant after that or which parts of it are
I’ll inspect #848’s new feedback and current checks, then address it on its branch without disturbing the unpublished PR C work.
M .lore.md
?? .craft-issue-842-plan.md
?? .opencode/
byk/fix/commit-on-repo
{"baseRefName":"master","headRefName":"feat/workspaces-schema","isDraft":false,"mergeStateStatus":"BLOCKED","mergeable":"MERGEABLE","reviewDecision":"REVIEW_REQUIRED","state":"OPEN"}
=== INLINE COMMENTS ===
{"body":"### Partial github skips git fallback\n\n**High Severity**\n\n\u003c!-- DESCRIPTION START --\u003e\nWorkspace `github` is shallow-merged even when the top level has no `github` block, so a workspace-only `projectPath` becomes a truthy incomplete `github` object. `getGlobalGitHubConfig` then skips git remote detection and returns a config missing `owner`/`repo`, which breaks GitHub operations for a schema-allowed monorepo layout.\n\u003c!-- DESCRIPTION END --\u003e\n\n\u003c!-- BUGBOT_BUG_ID: bd9608c5-9757-4720-9134-65fa3efbd31e --\u003e\n\n\u003c!-- LOCATIONS START\nsrc/config.ts#L136-L142\nsrc/config.ts#L456-L459\nLOCATIONS END --\u003e\n\u003cdetails\u003e\n\u003csummary\u003eAdditional ...
#848 has three new substantive review points: use Node’s built-in argv parser, move the workspaces gate to the next release, and make SemVer comparisons safe with build metadata. I’ll verify the current PR head and exact Node parser behavior, then fix all three on the PR branch without touching your current branch.
M .lore.md ?? .craft-issue-842-plan.md ?? .opencode/ byk/fix/commit-on-repo /home/byk/Code/getsentry/craft b06435e [byk/fix/commit-on-repo] /home/byk/.local/share/opencode/worktree/55fa901aca64a01fa4b5c2abce1c85c1c27475e7/calm-circuit 5d533c8 [feat/ci-ready-signal] /home/byk/.local/share/opencode/worktree/55fa901aca64a01fa4b5c2abce1c85c1c27475e7/calm-squid 347ade0 [fix/postcss-security-alert] /home/byk/.local/share/opencode/worktree/55fa901aca64a01fa4b5c2abce1c85c1c27475e7/stellar-falcon fc4c1d0 [byk/fix/dependabot-alerts-865] /home/byk/.local/share/opencode/worktree/55fa901aca64a01fa4b5c2abce1c85c1c27475e7/swift-squid 752a693 [fix/flaky-zip-test] /home/byk/Code/getsentry/craft-vercel-prebuilt-output 3f701f5 [fix/vercel-prebuilt-output] /home/byk/Code/getsentry/craft-vercel-project-config 3cceffb ...
The new feedback is valid. I’ll update #848 on a dedicated worktree: Node’s parseArgs will replace the hand-rolled pre-parser, the gate will move to 2.29.0, and version comparisons will ignore SemVer build metadata as SemVer requires.
feat/workspaces-schema
["publish","--workspace","cli"] {
values: [Object: null prototype] { workspace: 'cli' },
positionals: [ 'publish' ],
tokens: [
{ kind: 'positional', index: 0, value: 'publish' },
{
kind: 'option',
name: 'workspace',
rawName: '--workspace',
index: 1,
value: 'cli',
inlineValue: false
}
]
}
["publish","--workspace=cli"] {
values: [Object: null prototype] { workspace: 'cli' },
positionals: [ 'publish' ],
tokens: [
{ kind: 'positional', index: 0, value: 'publish' },
{
kind: 'option',
name: 'workspace',
rawName: '--workspace',
index: 1,
value: 'cli',
inlineValue: true
}
]
}
["publish","--workspace","--dry-run"] {
values: [Object: null prototype] { workspace: '--dry-run' },
positionals: [ 'publish' ],
tokens: [
{ kind: 'positional', index: 0, value: 'publish' },
{
kind: 'option',
name: 'workspace',
rawName: '--workspace',
...
node:util.parseArgs is available, but by itself still treats --workspace --dry-run as a workspace value. I’ll use its token stream to accept only an actual string value, while letting yargs retain its normal missing-value error.
drwxrwxr-x 1 byk byk 798 Aug 21 11:16 /home/byk/Code/getsentry Preparing worktree (checking out 'feat/workspaces-schema') HEAD is now at 42d17f0 fix(config): address Bugbot findings on workspaces selector
The review branch’s remote head has advanced beyond the local branch, so I’ll fast-forward the dedicated worktree before editing to preserve every existing change.
42d17f0 (HEAD -> feat/workspaces-schema, origin/feat/workspaces-schema, feat/workspaces-threading) fix(config): address Bugbot findings on workspaces selector 4e2d8f4 feat(config): add top-level workspaces schema + --workspace selector f70d5a8 feat: prefix-aware read paths for monorepo multi-product releases (#847) 83964a8 fix(cloudflare): default to worker, optional account id, infer prod branch (#846) 0f8720e feat(targets): add cloudflare deploy target (#843) From https://github.com/getsentry/craft * branch feat/workspaces-schema -> FETCH_HEAD 42d17f0..c099bbf feat/workspaces-schema -> origin/feat/workspaces-schema Updating 42d17f0..c099bbf Fast-forward CHANGELOG.md | 48 + build.mjs | 32 +- docs/astro.config.mjs | 15 +- docs/package.json | 14 +- docs/pnpm-lock.yaml | 1999 ...
The branch was rebased/merged with current master while you reviewed; it now includes unrelated Vercel/dependency changes from master. I’ll keep this update limited to the three workspace review fixes and verify the final PR diff against master afterward.
<path>/home/byk/Code/getsentry/craft-workspaces-schema/package.json</path>
<type>file</type>
<content>
1: {
2: "name": "@sentry/craft",
3: "version": "2.29.0-dev.0",
4: "description": "The universal sentry workflow CLI",
5: "main": "dist/craft",
6: "repository": "https://github.com/getsentry/craft",
7: "author": "Sentry",
8: "license": "MIT",
9: "bin": {
10: "craft": "dist/craft",
11: "sentry-craft": "dist/craft"
12: },
13: "devDependencies": {
14: "@aws-sdk/client-lambda": "^3.984.0",
15: "@eslint/js": "^9.17.0",
16: "@google-cloud/storage": "^7.14.0",
17: "@octokit/plugin-retry": "^7.1.2",
18: "@octokit/request-error": "^7.0.0",
19: "@octokit/rest": "^21.0.2",
20: "@sentry/esbuild-plugin": "^2.23.0",
21: "@sentry/node": "^10.27.0",
22: "@types/async": "^3.0.1",
23: "@types/aws4": "^1.5.1",
24: "@types/cli-table": "^0.3.0",
(Showing lines 1-24 of 129. Use offset=25 to ...
2.29.0-dev.0 is now the active development version, so the gate should become 2.29.0. I’ll make the version comparison robust for build metadata and replace the custom argv scan with node:util.parseArgs plus token validation.
<path>/home/byk/Code/getsentry/craft-workspaces-schema/src/config.ts</path> <type>file</type> <content> 50: /** 51: * Cached path to the configuration file 52: */ 53: let _configPathCache: string; 54: 55: /** 56: * Cached configuration 57: */ 58: let _configCache: CraftProjectConfig; 59: 60: /** 61: * The minimum craft version required to use the top-level `workspaces` config. 62: * 63: * This is the release the workspaces feature ships in. A dev build of that 64: * release (e.g. `2.27.0-dev.0`) satisfies it via the pre-release relaxation in 65: * `checkMinimalConfigVersion`. 66: */ 67: export const WORKSPACES_MIN_VERSION = '2.27.0'; 68: 69: /** 70: * The name of the currently-selected workspace, or undefined for the default 71: * (single implicit release unit). Set once via `setActiveWorkspace` from the 72: * `--workspace` CLI option / `CRAFT_WORKSPACE` env before any config access. ...
<path>/home/byk/Code/getsentry/craft-workspaces-schema/src/config.ts</path>
<type>file</type>
<content>
330: const parsed = validateConfiguration(rawConfig);
331: checkMinimalConfigVersion(parsed);
332: _configCache = applyWorkspaceSelection(parsed);
333: return _configCache;
334: }
335:
336: /**
337: * Loads and caches configuration from a YAML string.
338: *
339: * This is used by --config-from to load config from a remote branch.
340: *
341: * @param configContent The raw YAML configuration content
342: */
343: export function loadConfigurationFromString(
344: configContent: string,
345: ): CraftProjectConfig {
346: logger.debug('Loading configuration from provided content...');
347: const rawConfig = load(configContent) as Record<string, any>;
348: const parsed = validateConfiguration(rawConfig);
349: checkMinimalConfigVersion(parsed);
350: _configCache = applyWorkspaceSelection(parsed);
351: return _configCache;
352: }
353:
354: /**
355: * Checks ...
The build-metadata finding is valid: versionGreaterOrEqualThan rejects any build field. I’ll normalize both sides for compatibility comparisons, preserving only major/minor/patch and pre-release state where relevant.
<path>/home/byk/Code/getsentry/craft-workspaces-schema/src/index.ts</path>
<type>file</type>
<content>
1: #!/usr/bin/env node
2: // Import Sentry instrumentation first, before any other code
3: import './instrument';
4:
5: import isCI from 'is-ci';
6: import yargs from 'yargs';
7:
8: import { logger, LogLevel } from './logger';
9: import {
10: sanitizeDynamicLinkerEnv,
11: warnIfCraftEnvFileExists,
12: } from './utils/env';
13: import {
14: envToBool,
15: setGlobals,
16: extractWorkspaceSelection,
17: } from './utils/helpers';
18: import { getPackageVersion } from './utils/version';
19: import { withTracing } from './utils/tracing';
20: import { setActiveWorkspace } from './config';
21:
22: // Commands
23: import * as prepare from './commands/prepare';
24: import * as publish from './commands/publish';
25: import * as targets from './commands/targets';
26: import * as config from './commands/config';
27: import * as artifacts from './commands/artifacts';
28: import * as ...
I’ll replace the helper’s manual scan using Node’s parseArgs({ tokens: true }); it remains necessary to establish the selected workspace before yargs runs command builders.
<path>/home/byk/Code/getsentry/craft-workspaces-schema/src/utils/helpers.ts</path>
<type>file</type>
<content>
15: export const MAX_STEP_OUTPUT_BYTES = 64 * 1024;
16:
17: const FALSY_ENV_VALUES = new Set(['', 'undefined', 'null', '0', 'false', 'no']);
18: export function envToBool(envVar: unknown): boolean {
19: const normalized = String(envVar).toLowerCase();
20: return !FALSY_ENV_VALUES.has(normalized);
21: }
22:
23: /**
24: * Extracts the `--workspace` selection from the raw argv (or the
25: * `CRAFT_WORKSPACE` env var) before yargs parsing.
26: *
27: * This is needed because yargs runs command `builder`s (which may read the
28: * configuration, e.g. `publish` derives its --target choices from
29: * config.targets) *before* middleware, so the workspace must be resolved up
30: * front rather than in a middleware.
31: *
32: * Supports `--workspace foo` and `--workspace=foo`. ...
The PR now targets 2.29.0-dev.0, confirming Bugbot’s stale-gate finding. I’m updating the gate to 2.29.0, normalizing build metadata before comparisons, and replacing the manual argv scan with Node’s parser.
Success. Updated the following files: M ../craft-workspaces-schema/src/utils/helpers.ts M ../craft-workspaces-schema/src/config.ts
The core fixes are in. I’ll add regression coverage for build metadata and Node-parser edge cases, then run the focused suite before committing.
<path>/home/byk/Code/getsentry/craft-workspaces-schema/src/utils/__tests__/helpers.test.ts</path>
<type>file</type>
<content>
30: ['1', true],
31: ['yes', true],
32: ['dogs are great!', true],
33: ])('From %j we should get "%s"', (envVar, result) =>
34: expect(envToBool(envVar)).toBe(result),
35: ));
36:
37: describe('extractWorkspaceSelection', () => {
38: const NO_ENV = {} as NodeJS.ProcessEnv;
39:
40: test('returns undefined when neither flag nor env is set', () => {
41: expect(
42: extractWorkspaceSelection(['publish', '1.0.0'], NO_ENV),
43: ).toBeUndefined();
44: });
45:
46: test('reads "--workspace foo"', () => {
47: expect(
48: extractWorkspaceSelection(['publish', '--workspace', 'cli'], NO_ENV),
49: ).toBe('cli');
50: });
51:
52: test('reads "--workspace=foo"', () => {
53: expect(extractWorkspaceSelection(['--workspace=mcp'], NO_ENV)).toBe('mcp');
54: });
55:
56: test('CLI flag wins over CRAFT_WORKSPACE ...
The existing parser tests cover the flagged cases. I’ll add an inline negative-name case and config tests for minVersion build metadata and the raised 2.29.0 gate.
<path>/home/byk/Code/getsentry/craft-workspaces-schema/src/__tests__/config.test.ts</path>
<type>file</type>
<content>
220: { name: 'github', id: 'second' },
221: ]);
222: // A mixed defined/undefined prefix is still ambiguous.
223: expect(getGitTagPrefix()).toBe('cli@');
224: expect(warnSpy).toHaveBeenCalledTimes(1);
225: });
226: });
227:
228: describe('workspaces', () => {
229: afterEach(() => {
230: setActiveWorkspace(undefined);
231: vi.restoreAllMocks();
232: });
233:
234: const WS_CONFIG = [
235: `minVersion: ${WORKSPACES_MIN_VERSION}`,
236: 'github:',
237: ' owner: getsentry',
238: ' repo: toolkit',
239: 'changelog: CHANGELOG.md',
240: 'workspaces:',
241: ' cli:',
242: ' releaseBranchPrefix: release/cli',
243: ' github:',
244: ' projectPath: cli',
245: ' targets:',
246: ' - name: github',
247: ' tagPrefix: "cli@"',
248: ' mcp:',
249: ' ...
Because this worktree has 2.29.0-dev.0, the existing workspace tests will now exercise the intended dev-build equivalence against minVersion: 2.29.0. I’ll add an explicit build-metadata case that both passes the global compatibility check and meets the workspace gate.
Success. Updated the following files: M ../craft-workspaces-schema/src/utils/__tests__/helpers.test.ts M ../craft-workspaces-schema/src/__tests__/config.test.ts
I’ll run the focused tests and typecheck now; after that I’ll inspect the open PR #844 against the current workspace foundation to distinguish superseded work from still-relevant changes.
> @sentry/craft@2.29.0-dev.0 test /home/byk/Code/getsentry/craft-workspaces-schema > vitest run src/utils/__tests__/helpers.test.ts src/__tests__/config.test.ts ELIFECYCLE Test failed. See above for more details. WARN Local package.json exists, but node_modules missing, did you mean to install? sh: 1: vitest: not found
The review worktree has no node_modules, so the tests could not start. I’ll install the locked dependencies there, then rerun the focused checks.
Lockfile is up to date, resolution step is skipped Progress: resolved 1, reused 0, downloaded 0, added 0 Packages: +649 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ╭──────────────────────────────────────────╮ │ │ │ Update available! 10.28.0 → 11.23.0. │ │ Changelog: https://pnpm.io/v/11.23.0 │ │ To update, run: pnpm add -g pnpm │ │ │ ╰──────────────────────────────────────────╯ Progress: resolved 649, reused 649, downloaded 0, added 28 Progress: resolved 649, reused 649, downloaded 0, added 648 Progress: resolved 649, reused 649, downloaded 0, added 649, done dependencies: + @vercel/client 18.2.5 + fastest-levenshtein 1.0.16 + ignore 7.0.5 + marked 17.0.1 + p-limit 6.2.0 + semver 7.7.3 devDependencies: + @aws-sdk/client-lambda 3.984.0 + @eslint/js 9.39.2 + @google-cloud/storage 7.18.0 + @octokit/plugin-retry 7.2.1 + ...
Dependencies are installed in the isolated review worktree. I’ll rerun the focused verification and inspect PR #844’s diff after the fixes pass.
> @sentry/craft@2.29.0-dev.0 test /home/byk/Code/getsentry/craft-workspaces-schema
> vitest run src/utils/__tests__/helpers.test.ts src/__tests__/config.test.ts
RUN v4.1.8 /home/byk/Code/getsentry/craft-workspaces-schema
Test Files 2 passed (2)
Tests 79 passed (79)
Start at 15:39:07
Duration 1.44s (transform 448ms, setup 0ms, import 1.85s, tests 55ms, environment 0ms)
src/utils/helpers.ts(59,27): error TS2339: Property 'value' does not exist on type 'OptionToken | { kind: "positional"; index: number; value: string; } | { kind: "option-terminator"; index: number; }'.
Property 'value' does not exist on type '{ kind: "option-terminator"; index: number; }'.
src/utils/helpers.ts(60,20): error TS2339: Property 'value' does not exist on type 'OptionToken | { kind: "positional"; index: number; value: string; } | { kind: "option-terminator"; index: number; }'.
Property 'value' does not exist on type '{ kind: "option-terminator"; index: number; ...
The focused tests pass; TypeScript caught a narrowing issue in the new parseArgs token handling. I’ll switch the find expression to an explicit discriminated-union loop.
Success. Updated the following files: M ../craft-workspaces-schema/src/utils/helpers.ts
The parser now narrows safely. I’ll rerun typecheck/format and compare #844 to master plus #848 so we can identify exactly what remains relevant.
Checking formatting...
All matched files use Prettier code style!
=== PR #844 file diff ===
{"baseRefName":"master","headRefName":"feat/prefixed-tags","isDraft":true,"state":"OPEN"}
M docs/src/content/docs/configuration.md
M docs/src/content/docs/targets/github.md
M src/__tests__/config.test.ts
M src/commands/__tests__/changelog-versioning-policy.test.ts
M src/commands/changelog.ts
M src/commands/prepare.ts
M src/config.ts
M src/utils/__tests__/git.test.ts
M src/utils/__tests__/version.test.ts
M src/utils/git.ts
=== files already represented by #848 ===
src/commands/publish.ts
src/config.ts
src/index.ts
src/schemas/project_config.ts
src/__tests__/config.test.ts
src/utils/helpers.ts
src/utils/__tests__/helpers.test.ts
The new review feedback is now addressed locally: WORKSPACES_MIN_VERSION is 2.29.0, build metadata no longer crashes config compatibility checks, and the workspace pre-parser now uses node:util.parseArgs with token validation. I’ll search for stale workspace-version references, then run the full validation before the final review.
=== stale workspace gate references ===
src/config.ts:67:export const WORKSPACES_MIN_VERSION = '2.29.0';
src/config.ts:203: * enforces the `WORKSPACES_MIN_VERSION` gate.
src/config.ts:232: if (!isVersionGteMinVersion(config.minVersion, WORKSPACES_MIN_VERSION)) {
src/config.ts:234: `Using "workspaces" requires minVersion >= ${WORKSPACES_MIN_VERSION} ` +
src/config.ts:395: // A dev/pre-release build of X.Y.Z (e.g. "2.27.0-dev.0") already contains the
src/__tests__/config.test.ts:14: WORKSPACES_MIN_VERSION,
src/__tests__/config.test.ts:132: minVersion: '2.27.0',
src/__tests__/config.test.ts:235: `minVersion: ${WORKSPACES_MIN_VERSION}`,
src/__tests__/config.test.ts:320: `minVersion: ${WORKSPACES_MIN_VERSION}`,
src/__tests__/config.test.ts:324: new RegExp(`requires minVersion >= ${WORKSPACES_MIN_VERSION}`),
src/__tests__/config.test.ts:332: `minVersion: ${WORKSPACES_MIN_VERSION}`,
src/__tests__/config.test.ts:333: `minVersion: ...
The review fixes pass the full test suite and lint. The docs build only failed because this isolated worktree lacks docs/node_modules; I’ll install its frozen dependencies, update the stale dev-version example, and then run the final review.
Success. Updated the following files: M ../craft-workspaces-schema/src/config.ts
continue
I’m finishing the remaining docs verification, then I’ll inspect the exact PR diff and run separate correctness and security review passes before merging.
> @sentry/craft@2.29.0-dev.0 build /home/byk/Code/getsentry/craft-workspaces-schema > node build.mjs [build] SENTRY_AUTH_TOKEN not found, skipping source map upload Checking formatting... All matched files use Prettier code style! > craft-docs@1.0.0 build /home/byk/Code/getsentry/craft-workspaces-schema/docs > astro build Entry docs → 404 was not found. 16:20:36 [content] Syncing content 16:20:37 [content] Synced content 16:20:37 [types] Generated 2.20s 16:20:37 [build] output: "static" 16:20:37 [build] mode: "static" 16:20:37 [build] directory: /home/byk/Code/getsentry/craft-workspaces-schema/docs/dist/ 16:20:37 [build] Collecting build info... 16:20:37 [build] ✓ Completed in 2.43s. 16:20:37 [build] Building static entrypoints... 16:20:38 [vite] ✓ built in 1.28s 16:20:38 [vite] ✓ built in 87ms 16:20:38 [build] Rearranging server assets... generating static routes 16:20:38 ├─ /404.html (+31ms) 16:20:38 ├─ /index.md (+7ms) 16:20:38 ├─ /configuration.md (+18ms) 16:20:38 ...
Docs build now passes. I’m starting separate correctness and security review passes over the final #848 tree, while I inspect #844’s remaining changes against the workspace design.
[
{
"content": "PR A (#847): merged + cleaned up",
"status": "completed",
"priority": "high"
},
{
"content": "PR B (#848): address new review feedback",
"status": "completed",
"priority": "high"
},
{
"content": "PR B (#848): correctness and security adversarial reviews",
"status": "in_progress",
"priority": "high"
},
{
"content": "PR B (#848): final verification, push, respond to reviewers, merge",
"status": "pending",
"priority": "high"
},
{
"content": "Assess #844 relevance after #848",
"status": "in_progress",
"priority": "high"
},
{
"content": "PR C: implement publish-state workspace key",
"status": "completed",
"priority": "high"
},
{
"content": "PR C: adversarial review and open stacked PR",
"status": "pending",
"priority": "medium"
},
{
"content": "PR D: action layer and getsentry/publish accept-side",
"status": "pending",
"priority": "medium"
},
{
...
<task id="ses_fcb68fcdcffettMSEJAIpY2jlD" state="running"> <summary>Background task started</summary> <task_result> The task is working in the background. You will be notified automatically when it finishes. DO NOT sleep, poll for progress, ask the task for status, or duplicate this task's work — avoid working with the same files or topics it is using. Work on non-overlapping tasks, or briefly tell the user what you launched and end your response. </task_result> </task>
<task id="ses_fcb67ea9cffeQ9sHc69I63xh39" state="running"> <summary>Background task started</summary> <task_result> The task is working in the background. You will be notified automatically when it finishes. DO NOT sleep, poll for progress, ask the task for status, or duplicate this task's work — avoid working with the same files or topics it is using. Work on non-overlapping tasks, or briefly tell the user what you launched and end your response. </task_result> </task>
The two review passes are running independently. In parallel, I’m tracing #844’s actual commits and overlap with #848, rather than judging it from filenames alone.
<task id="ses_fcb66007bffe7j3O0mzCXFFq3l" state="running"> <summary>Background task started</summary> <task_result> The task is working in the background. You will be notified automatically when it finishes. DO NOT sleep, poll for progress, ask the task for status, or duplicate this task's work — avoid working with the same files or topics it is using. Work on non-overlapping tasks, or briefly tell the user what you launched and end your response. </task_result> </task>
I’m verifying the Node 22.12 parser behavior directly, since that is Craft’s supported runtime and the review requested use of its built-in parser.
M src/__tests__/config.test.ts M src/config.ts M src/utils/__tests__/helpers.test.ts M src/utils/helpers.ts src/__tests__/config.test.ts | 198 +++++++++++++++++++++++++++++++ src/commands/publish.ts | 21 +++- src/config.ts | 230 +++++++++++++++++++++++++++++++----- src/index.ts | 23 +++- src/schemas/project_config.ts | 54 ++++++++- src/utils/__tests__/helpers.test.ts | 71 +++++++++++ src/utils/helpers.ts | 41 +++++++ 7 files changed, 597 insertions(+), 41 deletions(-) src/__tests__/config.test.ts | 11 ++++++++++ src/config.ts | 31 +++++++++++++++++++++------- src/utils/__tests__/helpers.test.ts | 9 ++++++++ src/utils/helpers.ts | 41 ++++++++++++++++++++++--------------- 4 files changed, 68 insertions(+), 24 deletions(-) error: Could not find executable "node@22.12.0" Use `volta install` to add a package to your toolchain (see `volta ...
The configured Volta Node 22.12.0 binary is not installed locally, so I cannot execute that exact runtime here. I’m checking parser invariants that could make pre-parse selection disagree with yargs, especially repeated options and -- handling, before I finalize the fix.
{"args":["publish","--workspace","cli","--workspace","mcp"],"values":{"workspace":"mcp"},"tokens":[{"kind":"positional","index":0,"value":"publish"},{"kind":"option","name":"workspace","rawName":"--workspace","index":1,"value":"cli","inlineValue":false},{"kind":"option","name":"workspace","rawName":"--workspace","index":3,"value":"mcp","inlineValue":false}]}
{"args":["publish","--workspace=mcp","--workspace","cli"],"values":{"workspace":"cli"},"tokens":[{"kind":"positional","index":0,"value":"publish"},{"kind":"option","name":"workspace","rawName":"--workspace","index":1,"value":"mcp","inlineValue":true},{"kind":"option","name":"workspace","rawName":"--workspace","index":2,"value":"cli","inlineValue":false}]}
{"args":["publish","--","--workspace","cli"],"values":{},"tokens":[{"kind":"positional","index":0,"value":"publish"},{"kind":"option-terminator","index":1},{"kind":"positional","index":2,"value":"--workspace"},{"kind":"positional","index":3,"value":"cli"}]}
Found 100 matches (more matches available)
/home/byk/Code/getsentry/craft-workspaces-schema/src/utils/workspaces.ts:
Line 15: /** Information about a workspace package */
Line 25: /** Dependencies that are also workspace packages */
Line 26: workspaceDependencies: string[];
Line 29: /** Result of workspace discovery */
Line 31: /** The type of workspace manager detected */
Line 37: /** Structure of pnpm-workspace.yaml */
Line 45: workspaces?: string[] | { packages?: string[] };
Line 93: * Extract workspaces array from package.json workspaces field
Line 97: workspaces: string[] | { packages?: string[] } | undefined,
Line 99: if (!workspaces) {
Line 102: if (Array.isArray(workspaces)) {
Line 103: return workspaces;
Line 105: return workspaces.packages || [];
Line 115: // First: collect all workspace package names and locations
Line 116: const workspaceLocations: Array<{
Line 120: const workspaceNames = new ...
I found one parser consistency edge case before merge: repeated --workspace flags must follow yargs’ “last value wins” behavior. The pre-parser currently returns the first token, so it can select a different release unit from yargs. I’ll correct that and add a regression test before review concludes.
Success. Updated the following files: M ../craft-workspaces-schema/src/utils/helpers.ts M ../craft-workspaces-schema/src/utils/__tests__/helpers.test.ts
The pre-parser now follows yargs’ last-value-wins rule for repeated --workspace options. I’ll rerun the targeted checks and compare malformed/repeated cases with yargs before accepting the change.
> @sentry/craft@2.29.0-dev.0 test /home/byk/Code/getsentry/craft-workspaces-schema
> vitest run src/utils/__tests__/helpers.test.ts src/__tests__/config.test.ts
RUN v4.1.8 /home/byk/Code/getsentry/craft-workspaces-schema
Test Files 2 passed (2)
Tests 80 passed (80)
Start at 16:41:26
Duration 1.34s (transform 497ms, setup 0ms, import 1.79s, tests 73ms, environment 0ms)
Checking formatting...
All matched files use Prettier code style!
{"args":["--workspace","cli","--workspace=mcp"],"workspace":["cli","mcp"]}
{"args":["--workspace","cli","--workspace","--dry-run"],"workspace":["cli",""],"dryRun":true}
{"args":["--workspace","--dry-run"],"workspace":"","dryRun":true}
Yargs represents repeated string options as an array, so it has no usable single-value contract there. The pre-parser’s explicit last-valid-value rule is deterministic and, unlike an array, preserves the one workspace selection invariant. I’m checking whether the command definitions validate repeats; if not, I’ll make the CLI reject duplicates rather than inventing ambiguous behavior.
Found 18 matches
/home/byk/Code/getsentry/craft-workspaces-schema/src/__tests__/versionBump.test.ts:
Line 218: describe('workspace:* handling (issue #804)', () => {
Line 223: * /packages/app/package.json (@scope/app, 0.0.1, deps: core @ workspace:*)
Line 258: dependencies: { '@scope/core': 'workspace:*' },
Line 296: dependencies: { '@scope/core': 'workspace:*' },
Line 300: 'npm error code EUNSUPPORTEDPROTOCOL\nnpm error Unsupported URL Type "workspace:": workspace:*',
Line 390: throw new Error('EUNSUPPORTEDPROTOCOL workspace:*');
Line 446: "@scope/core": "workspace:*",
Line 451: "@scope/core": ["@scope/core@0.0.1", "workspace:packages/core"],
Line 486: // Nested dependency version pins must stay as "workspace:*".
Line 487: expect(patched).toContain('"@scope/core": "workspace:*"');
Line 713: throw new Error('EUNSUPPORTEDPROTOCOL ...
Found 2 matches
/home/byk/Code/getsentry/craft-workspaces-schema/src/utils/helpers.ts:
Line 56: if (token.kind !== 'option' || token.name !== 'workspace') {
/home/byk/Code/getsentry/craft-workspaces-schema/src/index.ts:
Line 124: .option('workspace', {
<path>/home/byk/Code/getsentry/craft-workspaces-schema/src/index.ts</path>
<type>file</type>
<content>
112: .version(getPackageVersion())
113: .alias('v', 'version')
114: .help()
115: .alias('h', 'help')
116: .options(GLOBAL_BOOLEAN_FLAGS)
117: .option('log-level', {
118: default: 'Info',
119: choices: Object.keys(LogLevel).filter(level => isNaN(Number(level))),
120: coerce: level => level[0].toUpperCase() + level.slice(1).toLowerCase(),
121: describe: 'Logging level',
122: global: true,
123: })
124: .option('workspace', {
125: type: 'string',
126: describe:
127: 'Select a named workspace (release unit) from the configuration. ' +
128: 'Required when the config defines "workspaces". Env: CRAFT_WORKSPACE',
129: global: true,
130: })
131: .strictCommands()
132: .showHelpOnFail(true)
133: .middleware(setGlobals)
134: .parse(argv);
135: }
136:
137: withTracing(main, { name: ...
{"values":{"workspace":"mcp"},"tokens":[{"kind":"option","name":"workspace","rawName":"--workspace","index":0,"value":"cli","inlineValue":false},{"kind":"option","name":"workspace","rawName":"--workspace","index":2,"value":"mcp","inlineValue":false}]}
{"multiple":false,"values":{"workspace":"mcp"},"tokens":[{"kind":"option","name":"workspace","rawName":"--workspace","index":0,"value":"cli","inlineValue":false},{"kind":"option","name":"workspace","rawName":"--workspace","index":2,"value":"mcp","inlineValue":false}]}
{"multiple":true,"values":{"workspace":["cli","mcp"]},"tokens":[{"kind":"option","name":"workspace","rawName":"--workspace","index":0,"value":"cli","inlineValue":false},{"kind":"option","name":"workspace","rawName":"--workspace","index":2,"value":"mcp","inlineValue":false}]}
Node’s parser confirms last-option semantics for scalar values. I’m tightening the helper so a final bare/invalid --workspace clears any earlier value and falls back to CRAFT_WORKSPACE, matching the documented “only when it carries a value” rule.
Success. Updated the following files: M ../craft-workspaces-schema/src/utils/helpers.ts M ../craft-workspaces-schema/src/utils/__tests__/helpers.test.ts
The parser semantics are now explicit and tested for repeated values and a final bare flag. I’ll rerun the full verification after that hardening, then reconcile the independent review findings.
> @sentry/craft@2.29.0-dev.0 test /home/byk/Code/getsentry/craft-workspaces-schema > vitest run RUN v4.1.8 /home/byk/Code/getsentry/craft-workspaces-schema [info] [[target/github]] No previous release found [info] [[target/github]] Deleted orphaned draft release: v1.0.0 [info] [[target/github]] No previous release found [warn] [[target/github]] Failed to clean up release "v1.0.0": Error: Cleanup failed [info] [[target/github]] No previous release found [info] [[target/github]] No previous release found [info] [[target/github]] Release for tag "1.0.0" already exists and is published. Skipping GitHub release creation (likely from a previous run). [info] [[target/github]] No previous release found [info] [[target/github]] createRelease returned 422 for tag "1.0.0". Looking for a leftover draft release to clean up... [info] [[target/github]] Deleting leftover draft release (id=789) for tag "1.0.0"... ...
Full verification is green: 59 test files, 1,114 passing tests, typecheck, lint with only the project’s existing unused-variable warnings, Prettier, build, and docs build. I also closed the repeated-option selector ambiguity before the final review.
At 16:49 UTC, GitHub master is 32f5462; merged PR #847 (f70d5a8, from 1afce59) is its ancestor. #844 (8aa8586) and #848 (c099bbf) share only base 0f8720e, so their overlap is copied/superseded source, not commit ancestry.
PR #847 explicitly salvaged #844’s prefix-aware core. These behaviours are on master and present in #848’s source:
git describe --match <prefix>*: #844 src/utils/git.ts:45; #848 src/utils/git.ts:45.src/commands/prepare.ts:762,870, #844 src/commands/changelog.ts:63; #848 retains these at prepare.ts:762,870 and changelog.ts:79.getGitTagPrefix() conflict warning: #844 src/config.ts:339-357; #848 retains it at src/config.ts:504-527.Some files are byte-identical between #844 and #848: docs/.../configuration.md, src/commands/prepare.ts, and src/utils/__tests__/version.test.ts.
#844 documents one .craft.yml per product (docs/src/content/docs/targets/github.md:50). #848 instead resolves one selected release unit from top-level workspaces:
src/config.ts:110-227,332,350src/config.ts:504-510src/index.ts:98, src/utils/helpers.ts:41src/schemas/project_config.ts:232cli and mcp resolve their own tag prefixes: src/__tests__/config.test.ts:265-288.Therefore, once #848 is used, getGitTagPrefix() reads only the selected workspace’s targets. The per-file configuration recommendation in #844 directly conflicts with the new model. ...
#844 assessment is complete: close it. #847 already merged its useful prefix-aware implementation; the remaining #844 design conflicts with #848’s top-level release workspaces and its divergent paths are worse than the merged fixes.
I’m waiting for the two independent #848 review passes, then I’ll perform the final end-to-end selector smoke test and commit/push the remaining feedback fixes.
0 55 Analyze (actions) pass 40s https://github.com/getsentry/craft/actions/runs/31199361782/job/92935347745 Analyze (javascript) pass 59s https://github.com/getsentry/craft/actions/runs/31199361782/job/92935347738 Build pass 20s https://github.com/getsentry/craft/actions/runs/31199365683/job/92935551772 Build Docs pass 18s https://github.com/getsentry/craft/actions/runs/31199365683/job/92935551803 Changelog Preview / Semver Impact pass 0 https://github.com/getsentry/craft/actions/runs/31199366228 Minor CodeQL pass 3s https://github.com/getsentry/craft/runs/92935518540 Lint fixes pass 32s https://github.com/getsentry/craft/actions/runs/31199365612/job/92935356173 Node 22 pass 44s https://github.com/getsentry/craft/actions/runs/31199365683/job/92935356441 Node 24 pass 39s https://github.com/getsentry/craft/actions/runs/31199365683/job/92935356251 Secret Scan pass 16s https://github.com/getsentry/craft/actions/runs/31199365584/job/92935356072 Socket Security: Project ...
<path>/home/byk/Code/getsentry/craft-workspaces-schema/.craft.yml</path>
<type>file</type>
<content>
1: minVersion: '2.14.0'
2: changelog:
3: policy: auto
4: artifactProvider:
5: name: github
6: config:
7: artifacts:
8: Build & Test:
9: - craft-binary
10: - craft-docs
11: preReleaseCommand: >-
12: node -p "
13: const {execSync} = require('child_process');
14: execSync('npm --no-git-tag-version version ' + process.env.CRAFT_NEW_VERSION).toString();
15: "
16: postReleaseCommand: >-
17: node -p "
18: const {execSync} = require('child_process');
19: execSync('npm --no-git-tag-version version preminor --preid=dev');
20: execSync('git diff --quiet || git commit -anm \'meta: Bump new development version\\n\\n#skip-changelog\' && git pull --rebase && git push').toString();"
21: requireNames:
22: - /^sentry-craft.*\.tgz$/
23: - /^craft$/
24: targets:
25: - name: npm
26: - name: gcs
27: includeNames: /^.*craft.*$/
28: ...
One wording correction surfaced in the parser check: yargs accepts a bare string option as an empty value rather than reporting it. I’ll adjust the helper documentation to describe the actual fallback behavior before the final commit.
Success. Updated the following files: M ../craft-workspaces-schema/src/utils/helpers.ts
The fix set is ready and fully verified locally. I’m awaiting the two independent adversarial review results before I commit, reply to the new threads, and merge #848.
Final review passes are pending. I’ll continue with commit, reviewer replies, merge, and the #844 closure recommendation as soon as they return.
continue
I’m resuming the final gate: check the independent review results, then finalize #848’s commit, review-thread replies, and merge state.
<task id="ses_fcb68fcdcffettMSEJAIpY2jlD" state="running"> <summary>Background task updated</summary> <task_result> Additional context sent to the running background task. The task is still working in the background. You will be notified automatically when it finishes. DO NOT sleep, poll for progress, ask the task for status, or duplicate this task's work — avoid working with the same files or topics it is using. Work on non-overlapping tasks, or briefly tell the user what you sent and end your response. </task_result> </task>
While the independent review finishes, I’m checking the current PR head, review threads, and final diff scope so I can merge only the intended workspace changes.
### 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 whet…
Date: Aug 24, 2026 * 🔴 [requested-continuation] (16:20) User asked to continue the workspace-schema/PR work. * 🟡 (16:20) Assistant stated plan: finish docs verification; inspect the exact PR diff; run separate correctness and security reviews before merging. * 🟡 (16:22) Build passed via `node build.mjs` in `/home/byk/Code/getsentry/craft-workspaces-schema`; `SENTRY_AUTH_TOKEN` was absent, so s…
Date: Aug 24, 2026 * 🔴 (15:46) User stated “never used `@typescript-eslint/no-unused-vars`” (repeated 3 times). * 🟡 (15:46) Stale workspace-gate reference search confirmed `WORKSPACES_MIN_VERSION = '2.29.0'` in `src/config.ts:67`; workspace gate enforcement is documented at `src/config.ts:203` and implemented at `src/config.ts:232-234`. Related tests remain in `src/__tests__/config.test.ts`, in…
Date: Aug 24, 2026 * 🟡 (15:41) Assistant updated `../craft-workspaces-schema/src/utils/helpers.ts` to replace unsafe `parseArgs` token handling with safe discriminated-union narrowing via an explicit loop, addressing the prior `TS2339` errors involving `.value` and `.inlineValue`. * 🟡 (15:41) Formatting validation reported: “All matched files use Prettier code style!” * 🟡 (15:41) PR #844 metad…
Date: Aug 24, 2026 * 🟡 (15:40) Focused verification in `/home/byk/Code/getsentry/craft-workspaces-schema` ran `vitest run src/utils/__tests__/helpers.test.ts src/__tests__/config.test.ts`: 2 test files passed and 79 tests passed; duration 1.44s (transform 448ms, import 1.85s, tests 55ms). * 🟡 (15:40) TypeScript compilation reported 5 `TS2339` errors in `src/utils/helpers.ts` at lines 59, 60, 62…
Date: Aug 24, 2026 * 🟡 (15:39) Installed locked dependencies in `/home/byk/Code/getsentry/craft-workspaces-schema`: lockfile up to date; 649 packages added; completed in 2.8s using pnpm v10.28.0. Installed `vitest` 4.1.8, `vite` 7.3.5, and `typescript` 5.9.3 among project dependencies. * 🟡 (15:39) pnpm ignored build scripts for `@sentry/cli@2.39.1` and `esbuild@0.28.1`; it advised running `pnpm…
Date: Aug 24, 2026 * 🟡 (15:36) Focused test command `vitest run src/utils/__tests__/helpers.test.ts src/__tests__/config.test.ts` in `/home/byk/Code/getsentry/craft-workspaces-schema` failed before running because local `node_modules` is missing; `vitest: not found`. pnpm emitted `ELIFECYCLE` and warned: “Local package.json exists, but node_modules missing, did you mean to install?” * 🟡 (15:36)…
Date: Aug 24, 2026 * 🟡 (15:32) Files modified: `../craft-workspaces-schema/src/utils/__tests__/helpers.test.ts` and `../craft-workspaces-schema/src/__tests__/config.test.ts`. * 🟡 (15:32) Assistant planned to run focused tests and typecheck, then inspect open PR `#844` against the current workspace foundation to distinguish superseded work from still-relevant changes.
Date: Aug 24, 2026 * 🟡 (15:29) `src/__tests__/config.test.ts` workspace test fixture `WS_CONFIG` sets `minVersion: ${WORKSPACES_MIN_VERSION}`, base GitHub `owner: getsentry`/`repo: toolkit`, `changelog: CHANGELOG.md`, and two workspaces: `cli` (`releaseBranchPrefix: release/cli`, `github.projectPath: cli`, GitHub target `tagPrefix: "cli@"`) and `mcp` (`releaseBranchPrefix: release/mcp`, `version…
Date: Aug 24, 2026 * 🟡 (15:24) Tool reported successful modifications to `../craft-workspaces-schema/src/utils/helpers.ts` and `../craft-workspaces-schema/src/config.ts`. * 🟡 (15:24) Assistant stated the core fixes were implemented and planned regression coverage for build metadata and Node-parser edge cases, followed by running the focused test suite before committing. * 🟡 (15:27) `src/utils/…
Date: Aug 24, 2026 * 🟡 (15:20) Assistant identified that the PR targets `2.29.0-dev.0`, confirming Bugbot’s stale-gate finding. * 🟡 (15:20) Assistant planned to update the version gate to `2.29.0`. * 🟡 (15:20) Assistant planned to normalize build metadata before version comparisons. * 🟡 (15:20) Assistant planned to replace `fixGlobalBooleanFlags`’ manual argv scan with Node’s parser. * 🟡 (15…
Date: Aug 24, 2026 * 🟡 (15:19) `src/index.ts` establishes the active workspace before yargs parsing/builders by calling `setActiveWorkspace(extractWorkspaceSelection(argv))` after `fixGlobalBooleanFlags(process.argv.slice(2))`; this is required because command builders such as `publish` may read configuration and derive `--target` choices before middleware, so workspace selection in middleware w…
Date: Aug 24, 2026 * 🔴 (15:15) User asserted that pre-release relaxation in `checkMinimalConfigVersion` applies only to the current Craft version, never to the configured `minVersion` side. * 🟡 (15:15) `src/config.ts` `loadConfiguration()` and `loadConfigurationFromString(configContent)` both validate YAML configuration, call `checkMinimalConfigVersion(parsed)`, then cache `applyWorkspaceSelect…
Date: Aug 24, 2026 * 🟡 (15:10) `src/config.ts` currently defines `WORKSPACES_MIN_VERSION = '2.27.0'`; comments state this is the release shipping workspaces and that dev builds such as `2.27.0-dev.0` satisfy the gate through `checkMinimalConfigVersion` pre-release relaxation. * 🟡 (15:10) `src/config.ts` holds active workspace state in `let _activeWorkspaceName: string | undefined`; `setActiveWo…
Date: Aug 24, 2026 * 🔴 (15:08) User stated Craft package management must always use `pnpm`; `npm` and `yarn` must never be used. * 🔴 (15:08) User stated Craft uses Volta-managed Node.js v22.12.0 and dependencies must be installed with `pnpm install --frozen-lockfile`. * 🔴 (15:08) User stated development commands are `pnpm build` (builds `dist/craft`), `pnpm test` (runs tests), `pnpm lint` (run…
Date: Aug 24, 2026 * 🟡 (15:06) Tool output: `/home/byk/Code/getsentry` on `feat/workspaces-schema` initially had `HEAD`/`origin/feat/workspaces-schema`/`feat/workspaces-threading` at `42d17f0` (`fix(config): address Bugbot findings on workspaces selector`), with prior commits `4e2d8f4` (`feat(config): add top-level workspaces schema + --workspace selector`), `f70d5a8` (`feat: prefix-aware read p…
Date: Aug 24, 2026 * 🟡 (15:04) Tool output: dedicated worktree at `/home/byk/Code/getsentry` was prepared by checking out branch `feat/workspaces-schema`; `HEAD` is at commit `42d17f0` (`fix(config): address Bugbot findings on workspaces selector`). Directory listing reported `drwxrwxr-x 1 byk byk 798 Aug 21 11:16 /home/byk/Code/getsentry`. * 🟡 (15:04) Assistant stated the review branch’s remot…
Date: Aug 24, 2026 * 🟡 (15:02) `node:util.parseArgs` test results on branch `feat/workspaces-schema`: `["publish","--workspace","cli"]` and `["publish","--workspace=cli"]` both parse `workspace: 'cli'`; `["publish","--workspace","--dry-run"]` parses `workspace: '--dry-run'`; `["publish","--workspace"]` parses `workspace: true`; `["publish","--workspace="]` parses `workspace: ''`. Token streams i…
Date: Aug 24, 2026 * 🟡 (14:57) Repository status on user’s current worktree `/home/byk/Code/getsentry/craft` at commit `b06435e` on branch `byk/fix/commit-on-repo`: modified `.lore.md`; untracked `.craft-issue-842-plan.md` and `.opencode/`. * 🟡 (14:57) Available Craft worktrees reported: `/home/byk/.local/share/opencode/worktree/55fa901aca64a01fa4b5c2abce1c85c1c27475e7/calm-circuit` at `5d533c8…
Date: Aug 24, 2026 * 🔴 (14:55) User-provided review feedback states that `extractWorkspaceSelection` always treats the next `argv` token after `--workspace` as the workspace name, including another flag or a missing value; this can select a bogus workspace before yargs parsing, disagree with `argv.workspace`, and suppress `CRAFT_WORKSPACE` for a valueless flag. * 🔴 [requested-built-in-parser] (…
Date: Aug 24, 2026 * 🔴 [requested-review] [requested-adversarial-review] [requested-merge] (14:53) User stated PR 848 is ready with review feedback and asked to address the feedback, perform a final adversarial review, then merge it and move forward. * 🔴 [requested-pr-relevance-assessment] (14:53) User asked to assess whether https://github.com/getsentry/craft/pull/844 remains relevant after PR…