feat: add scoped staging workspaces - #76
Conversation
📝 WalkthroughWalkthroughAdds scoped staging workspaces with RootSet-backed retention, checkpointing, renewal, promotion, release, namespace-bounded inspection, expiry sweeping, repository diagnostics, public types, documentation, and unit/integration coverage. ChangesScoped staging workspaces
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Self-Code Review — Final Posture
Reviewed the complete Review corrections now include:
Verification evidence:
Residual risks are documented rather than hidden: the unavoidable object-write-to-ref-update grace window, cumulative RootSet construction cost at very large target counts, and the still-pending downstream git-warp ten-second performance gate. @codex please confirm this final evidence posture and flag any remaining retention-safety concern. |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/0049-scoped-staging-workspaces/witness/verification.md`:
- Around line 59-76: Reconcile the verification summary around the release
verifier results: make the “13 executed steps” count match the table by adding
the two missing result rows or correcting the count, and clarify whether the
reported 6,454 observed tests excludes the three example checks. Update the
surrounding verification wording consistently without changing unrelated
results.
In `@index.d.ts`:
- Around line 1376-1382: Make the workspaces property required in the
RepositoryDoctor dependency type, while keeping its inspectRecord method and
return type unchanged. Align the declaration with RepositoryDoctor’s
unconditional use of this.workspaces.inspectRecord and ensure callers must
provide the adapter.
In `@src/domain/services/RepositoryDoctor.js`:
- Around line 409-433: Update workspaceUsage so healthy is false whenever
report.issue is non-null, while preserving the existing invalid-posture check.
Ensure records are healthy only when posture is not invalid and no inspection
issue is present.
In `@src/domain/services/StagingWorkspace.js`:
- Around line 264-277: Bound iteration in `#resolveTargets` by enforcing the
configured maximum checkpoint count while consuming handles, before calling
`#resolveTarget` for each entry. Stop or throw once the limit is reached,
including for repeated duplicates and non-terminating iterables, while
preserving deduplication, sorting, and existing invalid-iterable handling.
In `@src/domain/services/StagingWorkspaceRegistry.js`:
- Around line 88-110: The inspect/sweep flow only scans the first limit refs,
preventing later expired workspaces from being reached. Update
StagingWorkspaceRegistry.inspect and its sweep caller to support continuation
pagination (return and accept a cursor) or an explicit bounded internal scan,
while preserving the limit-sized result; add coverage where an active first page
is followed by an expired workspace.
In `@test/unit/domain/services/StagingWorkspacePromotion.test.js`:
- Around line 192-217: Update the test “publishes before releasing and rejects
handles outside the workspace” to cover both stated behaviors: make the mocked
publications.commit callback assert the workspace generation still exists when
invoked, and add a separate promoteToPublication attempt using an unstaged
handle that expects the appropriate rejection. Preserve the existing successful
publication assertions and released-workspace check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ceeb9e2-3b8f-48dd-bd23-2c92f8a37b4e
📒 Files selected for processing (22)
CHANGELOG.mdREADME.mddocs/API.mddocs/design/0049-scoped-staging-workspaces/scoped-staging-workspaces.mddocs/design/0049-scoped-staging-workspaces/witness/verification.mddocs/design/README.mddocs/releases/v6.4.0.mdindex.d.tsindex.jssrc/domain/errors/Codes.jssrc/domain/services/RepositoryDoctor.jssrc/domain/services/StagingWorkspace.jssrc/domain/services/StagingWorkspaceRegistry.jssrc/domain/services/WorkspaceDescriptorCodec.jssrc/domain/value-objects/WorkspaceRef.jstest/integration/staging-workspace.test.jstest/unit/domain/services/RepositoryDoctor.test.jstest/unit/domain/services/StagingWorkspace.test.jstest/unit/domain/services/StagingWorkspacePromotion.test.jstest/unit/domain/services/WorkspaceDescriptorCodec.test.jstest/unit/domain/value-objects/WorkspaceRef.test.jstest/unit/facade/ContentAddressableStore.application-storage.test.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
CHANGELOG.md
📄 CodeRabbit inference engine (AGENTS.md)
Use
CHANGELOG.mdto record the historical truth of merged behavior
Files:
CHANGELOG.md
docs/design/**
📄 CodeRabbit inference engine (AGENTS.md)
Use
docs/design/directory for durable design contracts and proof plans
Files:
docs/design/README.mddocs/design/0049-scoped-staging-workspaces/witness/verification.mddocs/design/0049-scoped-staging-workspaces/scoped-staging-workspaces.md
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Use
README.mdas the public front door, core value prop, and quick start documentation
Files:
README.md
🧠 Learnings (3)
📚 Learning: 2026-02-28T19:21:13.982Z
Learnt from: flyingrobots
Repo: git-stunts/git-cas PR: 15
File: test/unit/domain/services/CasService.envelope.test.js:326-330
Timestamp: 2026-02-28T19:21:13.982Z
Learning: In fuzz tests for cryptographic operations, use a seeded PRNG (e.g., xorshift32) for control-flow variables such as plaintext size selection and recipient index selection to ensure reproducibility. Continue to use randomBytes() for cryptographic keys and nonces to preserve realistic randomness and avoid security anti-patterns. This guideline applies to test files that perform fuzz testing of crypto logic; implement a consistent seed setup (e.g., fixed seed in test initialization) and document the rationale to enable deterministic replays across runs.
Applied to files:
test/unit/domain/services/WorkspaceDescriptorCodec.test.jstest/unit/domain/value-objects/WorkspaceRef.test.jstest/integration/staging-workspace.test.jstest/unit/facade/ContentAddressableStore.application-storage.test.jstest/unit/domain/services/StagingWorkspacePromotion.test.jstest/unit/domain/services/StagingWorkspace.test.jstest/unit/domain/services/RepositoryDoctor.test.js
📚 Learning: 2026-03-30T19:53:48.000Z
Learnt from: flyingrobots
Repo: git-stunts/git-cas PR: 29
File: docs/design/TR-010-planning-index-consistency-review.md:121-131
Timestamp: 2026-03-30T19:53:48.000Z
Learning: In this repo, CHANGELOG.md entries should be user-facing and descriptive (bullet points) rather than cycle-ID headings (e.g., use a phrase like “Planning-index consistency review” instead of a “TR-010 — …” heading). When searching the changelog, don’t rely on grepping for cycle IDs (e.g., “TR-010”) because it may cause false negatives; search for the descriptive keywords/phrases from the bullet entries instead.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-07-13T17:00:46.222Z
Learnt from: flyingrobots
Repo: git-stunts/git-cas PR: 65
File: src/domain/helpers/isCanonicalCollectionKey.js:21-35
Timestamp: 2026-07-13T17:00:46.222Z
Learning: In git-stunts/git-cas, the codebase is intended to run on multiple JavaScript runtimes (Node, Bun, and Deno), not just Node. During code review, don’t suggest replacing existing portable helper logic with newer Node-only built-ins (e.g., Node 20+ APIs like `String.prototype.isWellFormed()`) unless the change preserves Bun/Deno compatibility (via portable/polyfill implementations or safe runtime gating). If a file includes manual implementations (such as `src/domain/helpers/isCanonicalCollectionKey.js`), treat them as intentional to avoid requiring a higher host built-in baseline.
Applied to files:
src/domain/value-objects/WorkspaceRef.jssrc/domain/errors/Codes.jssrc/domain/services/WorkspaceDescriptorCodec.jssrc/domain/services/StagingWorkspaceRegistry.jssrc/domain/services/RepositoryDoctor.jssrc/domain/services/StagingWorkspace.js
🪛 ast-grep (0.44.1)
test/integration/staging-workspace.test.js
[warning] 8-8: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🪛 LanguageTool
docs/design/0049-scoped-staging-workspaces/scoped-staging-workspaces.md
[style] ~487-~487: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...registry and scoped staging wrappers. - Add cache/publication promotion and failure...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~488-~488: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...on promotion and failure-order tests. - Add bounded inspection, expiry sweep, and r...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 OpenGrep (1.25.0)
src/domain/value-objects/WorkspaceRef.js
[ERROR] 112-112: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (23)
CHANGELOG.md (1)
10-26: LGTM!README.md (1)
105-105: LGTM!Also applies to: 143-153
docs/API.md (1)
19-30: LGTM!Also applies to: 1243-1366, 1914-1922, 1960-1965, 3327-3335
docs/design/0049-scoped-staging-workspaces/scoped-staging-workspaces.md (1)
1-614: LGTM!docs/design/0049-scoped-staging-workspaces/witness/verification.md (1)
1-58: LGTM!Also applies to: 78-94
docs/design/README.md (1)
14-15: LGTM!docs/releases/v6.4.0.md (1)
1-114: LGTM!src/domain/errors/Codes.js (1)
103-111: LGTM!src/domain/value-objects/WorkspaceRef.js (2)
1-42: LGTM!Also applies to: 50-89, 108-151
43-49: 🔒 Security & PrivacyLiteral
+cannot appear in collection namespacesCollection namespaces are limited to lowercase ASCII components (
[a-z0-9.-]), so the/→+mapping here stays injective for valid inputs.> Likely an incorrect or invalid review comment.src/domain/services/WorkspaceDescriptorCodec.js (1)
1-144: LGTM!index.d.ts (1)
1244-1269: LGTM!Also applies to: 1350-1361, 1500-1627, 1681-1681
src/domain/services/StagingWorkspace.js (1)
1-263: LGTM!Also applies to: 278-499
src/domain/services/RepositoryDoctor.js (1)
7-7: LGTM!Also applies to: 21-27, 135-149, 184-190, 479-488, 528-537, 580-586, 623-634, 698-709
test/unit/domain/services/RepositoryDoctor.test.js (1)
71-90: LGTM!Also applies to: 91-128, 291-319, 525-563, 565-595
test/unit/domain/services/WorkspaceDescriptorCodec.test.js (1)
1-65: LGTM!src/domain/services/StagingWorkspaceRegistry.js (1)
1-87: LGTM!Also applies to: 160-489
index.js (1)
19-20: LGTM!Also applies to: 74-74, 197-201, 250-251, 326-326, 350-360, 426-430
test/integration/staging-workspace.test.js (1)
1-228: LGTM!test/unit/domain/services/StagingWorkspace.test.js (1)
1-334: LGTM!test/unit/domain/services/StagingWorkspacePromotion.test.js (1)
1-191: LGTM!test/unit/domain/value-objects/WorkspaceRef.test.js (1)
1-56: LGTM!test/unit/facade/ContentAddressableStore.application-storage.test.js (1)
15-15: LGTM!Also applies to: 43-51, 69-69, 92-106
Summary
cas.workspaces.open(),inspect(), andsweep()as the public API for scoped multi-object stagingWhy
Multi-object operations currently need either application-managed refs or temporary cache publications. That leaks storage ownership and imposes a major performance cost on downstream consumers such as git-warp. A staging workspace gives callers an explicit lease whose immutable targets survive aggressive pruning until exact-generation release.
Safety Invariants
Linked Issue
Design / Proof
docs/design/0049-scoped-staging-workspaces/scoped-staging-workspaces.mddocs/design/0049-scoped-staging-workspaces/witness/verification.md29bcf03e9e7289852812a956938e2f80bed7233aValidation
npm run release:verify -- --skip-jsr pnpm exec vitest run test/unit/docs --reporter=dot git diff --checkJSR publication verification was intentionally skipped because this is the feature PR, not the release-preparation PR.
Release Impact