Scope retained materializations with git-cas acquisitions by flyingrobots · Pull Request #756 · git-stunts/git-warp

flyingrobots · 2026-07-17T10:04:29Z

Summary

Upgrade @git-stunts/git-cas to 6.3.0.
Replace retained-materialization findExact() reads with a storage-neutral acquireExact() / release() lifecycle.
Add a handle-first live-resolution path that returns exact retained roots without opening the legacy state cache, replaying patches, opening a state session, hashing full state, or building a whole-state projection.
Hold acquisitions through exact compatibility-state reconstruction and release them on every success and failure path.
Suppress legacy snapshot publication on a handle-first miss, then reacquire and validate the newly retained generation before returning it.

Issue

Refs #738.
Refs #734.
Provides downstream adoption evidence for git-cas #69.

This does not complete #738. RuntimeHost and checkpoint creation still consume the full-state compatibility result. The next slice migrates those consumers to the new handle-first contract.

Test plan

Final pre-push IRONCLAD firewall at 05ff83981: all static gates and 7,020 unit tests passed; 2 skipped.
Clean Node 22 container: 597 Vitest files passed, 1 skipped; 7,199 tests passed, 2 skipped; all 110 BATS tests passed.
Focused real-Git integration: replacement plus expired reflogs and git prune --expire=now keeps generation A readable while acquired; after release, A is prunable while generation B remains retained.
Follow-up review fixes: all 121 tests across the eight affected suites pass, including coordinate, state-hash, and bundle identity drift plus exactly-once release failures.
npm run lint, typechecks, surface validation, Markdown/docs gates, policy gates, and quarantine-graduate checks pass.
Graft structural/API review reports no breaking export changes.
Independent Code Lawyer review found one low-severity compatibility-flow caption ambiguity; 05ff83981 resolves it and all documentation gates pass.

Storage invariants

A retained handle is decoded while its git-cas cache acquisition is active. The acquisition pins the observed generation across concurrent replacement or eviction until the caller releases it. Failure cleanup is best-effort and cannot replace the primary materialization error; release failure after a successful operation remains visible.

Cold materialization must reacquire the retained generation and match its coordinate, state hash, and bundle before returning it. Any mismatch fails closed and releases the mismatched acquisition exactly once.

Compatibility note

git-cas stamps its package version into asset manifests. The dependency bump therefore intentionally advances one v17 migration golden asset handle without changing the represented payload.

ADR checks

This PR does not implement ADR 2 without satisfying ADR 3.
This PR does not change persisted op formats; no ADR 3 readiness issue is required.
This PR does not change wire compatibility; canonical-only wire-op policy remains unchanged.
This PR does not change schema constants; patch and checkpoint namespaces remain distinct.

flyingrobots · 2026-07-17T10:04:42Z

@codex Self-Code Review complete.

Severity	Count	Result
Critical	0	None found
High	0	None found
Medium	0	None found
Low	0	None found

Reviewed origin/main..f03fa1dce after a clean-worktree precheck and fresh git fetch. Graft reports no breaking package-surface change, and an independent Code Lawyer found no concrete correctness, lifecycle, race, or test-honesty defect.

Residual risks are explicit: callers must eventually invoke release(); materialize() remains the full-state compatibility path; only the real-Git integration test proves ref-backed reachability under pruning. Please confirm the acquisition/release and generation-replacement reasoning.

coderabbitai · 2026-07-17T10:05:07Z

📝 Walkthrough

Walkthrough

The PR replaces exact materialization lookups with lifecycle-managed acquisitions, adds coordinate-first live resolution for retained and replayed materializations, updates cleanup and snapshot-publication behavior, upgrades git-cas, and expands tests and documentation.

Changes

Live materialization acquisition

Layer / File(s)	Summary
Acquisition contracts `src/ports/MaterializationStorePort.ts`, `src/domain/materialization/LiveMaterializationResolution.ts`, `src/domain/services/controllers/MaterializeStrategyRuntime.ts`, `test/unit/domain/materialization/MaterializationIdentity.test.ts`	Exact requests now return acquisitions with timestamps and `release()`, while live resolution objects validate source-specific state and replay counts.
CAS acquisition adapter `src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts`, `test/helpers/`, `test/unit/infrastructure/adapters/`, `test/integration/infrastructure/adapters/`, `package.json`, `test/unit/scripts/`	Exact cache access uses `CacheSet.acquire()`, propagates retention evidence, tracks active acquisitions, and releases handles across successful and failed resolution paths.
Coordinate-first resolution `src/domain/services/controllers/MaterializeLiveStrategy.ts`, `src/domain/services/controllers/MaterializeController.ts`, `src/domain/services/controllers/MaterializationWorkspaceCleanup.ts`	Live resolution handles empty frontiers, retained hits, replayed misses, coordinate validation, conditional snapshot publication, and acquisition cleanup.
Resolution validation and documentation `test/unit/domain/services/controllers/`, `test/integration/api/`, `docs/topics/cas-first-memoized-materialization.md`	Tests cover retained generations, release failures, replay failures, mismatched handles, and the documented compatibility limitation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant MaterializeController
  participant MaterializeLiveStrategy
  participant MaterializationStore
  participant GitCasMaterializationStoreAdapter
  Caller->>MaterializeController: resolveLiveMaterialization()
  MaterializeController->>MaterializeLiveStrategy: resolveMaterialization()
  MaterializeLiveStrategy->>MaterializationStore: acquireExact(coordinate)
  MaterializationStore->>GitCasMaterializationStoreAdapter: acquireExact(coordinate)
  GitCasMaterializationStoreAdapter-->>MaterializeLiveStrategy: acquisition or miss
  MaterializeLiveStrategy-->>MaterializeController: resolution with release()
  MaterializeController-->>Caller: live materialization resolution

Possibly related issues

v6.3.0: Bounded scoped cache acquisitions git-cas#69 — This PR adopts the scoped CacheSet.acquire() and release lifecycle.
v19 emergency: delegate all CAS and cache ownership to git-cas #734 — This PR advances acquisition-based retained materialization ownership.

Possibly related PRs

git-stunts/git-warp#751 — Provides the retained CAS materialization handle foundation used here.
git-stunts/git-warp#754 — Shares the acquireExact retained-handle resume flow.

Poem

A rabbit held a cached state,
Then released it at the gate.
Warm roots stayed and cold patches flew,
Each handle knew what it must do.
“Coordinate first!” the bunny cried—
And hopped away with state inside.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.90% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately highlights the main change: retaining materializations via git-cas acquisitions.
Description check	✅ Passed	The description follows the template with Summary, Issue refs, Test plan, and completed ADR checks.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 3

🤖 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 `@src/domain/services/controllers/MaterializeController.ts`:
- Around line 376-380: Update the acquisition lifecycle in _buildResult() so the
success-path acquisition.release() runs after the catch that handles operation
failures, preventing a release rejection from being passed to
releaseAcquisitionAfterFailure() and retried. Preserve the existing result
return and failure cleanup behavior.

In `@src/domain/services/controllers/MaterializeLiveStrategy.ts`:
- Around line 355-364: Update materializedResolution to validate the complete
acquired handle: retain the existing bundle comparison and also compare the
handle’s coordinate and stateHash against result.materialization. Throw the same
resolution error when any identifying field differs, ensuring the returned
acquisition represents the reported materialization.

In `@src/domain/services/controllers/MaterializeStrategyRuntime.ts`:
- Around line 28-33: Replace the structural LiveMaterializationResolution type
with a frozen runtime-backed class or factory that validates construction,
especially the allowed source values and their corresponding
materialization/release requirements. Preserve the existing materialization,
source, replayedPatchCount, and release API while ensuring every created
resolution is immutable and cannot represent an invalid combination.

🪄 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: 02dc1ff5-7ff3-4d34-966a-3b97eb8e593c

📥 Commits

Reviewing files that changed from the base of the PR and between ea69ac0 and f03fa1d.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (17)

docs/topics/cas-first-memoized-materialization.md
package.json
src/domain/services/controllers/MaterializationWorkspaceCleanup.ts
src/domain/services/controllers/MaterializeController.ts
src/domain/services/controllers/MaterializeLiveStrategy.ts
src/domain/services/controllers/MaterializeStrategyRuntime.ts
src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
src/ports/MaterializationStorePort.ts
test/helpers/InMemoryGitCasFacade.ts
test/helpers/InMemoryMaterializationStore.ts
test/integration/api/materialization.retainedResume.test.ts
test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts
test/unit/domain/services/controllers/MaterializationWorkspaceCleanup.test.ts
test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts
test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts
test/unit/infrastructure/adapters/GitCasMaterializationWorkspace.test.ts
test/unit/scripts/v18-v17-public-read-legacy-reading-builder.test.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (3)

GitHub Check: coverage-threshold
GitHub Check: test-node (22)
GitHub Check: preflight

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Do not use direct imports from src/infrastructure/** in src/domain/** or src/ports/**; depend on a port instead.
Do not use direct Node built-ins in src/domain/** or src/ports/**; use a port instead.

Files:

test/unit/infrastructure/adapters/GitCasMaterializationWorkspace.test.ts
test/unit/domain/services/controllers/MaterializationWorkspaceCleanup.test.ts
test/integration/api/materialization.retainedResume.test.ts
src/ports/MaterializationStorePort.ts
src/domain/services/controllers/MaterializationWorkspaceCleanup.ts
test/unit/scripts/v18-v17-public-read-legacy-reading-builder.test.ts
test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts
test/helpers/InMemoryMaterializationStore.ts
test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts
src/domain/services/controllers/MaterializeController.ts
src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
test/helpers/InMemoryGitCasFacade.ts
src/domain/services/controllers/MaterializeStrategyRuntime.ts
src/domain/services/controllers/MaterializeLiveStrategy.ts
test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts

src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx,js,jsx}: Do not introduce any, as any, as unknown as, unknown (outside adapters), Record<string, unknown> (outside adapters), *Like placeholder types, JSON.parse/JSON.stringify (outside adapters), fetch (outside adapters), process.env (outside adapters), @ts-ignore, or z.any() in core code; use validated boundary models and ports instead.
Use constructor-injected ports for external capabilities; do not rely on ambient dependencies for I/O, clocks, persistence, or entropy.
Do not create utils.ts, helpers.ts, misc.ts, or common.ts; name files after the actual concept they model.
Prefer one file per class, type, or object; if a file accumulates peer concepts, split it.
Keep helper corridors, fake shape trust, transitional duplication, and compile-time theater out of the codebase; runtime-honest TypeScript must reflect actual behavior.
No enum usage; prefer runtime-backed domain forms and unions.
Do not use boolean trap parameters; prefer named option objects or separate methods.
Avoid magic strings or numbers when a named constant should exist.
Keep domain bytes as Uint8Array; Buffer belongs in infrastructure adapters.

Files:

src/ports/MaterializationStorePort.ts
src/domain/services/controllers/MaterializationWorkspaceCleanup.ts
src/domain/services/controllers/MaterializeController.ts
src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
src/domain/services/controllers/MaterializeStrategyRuntime.ts
src/domain/services/controllers/MaterializeLiveStrategy.ts

src/ports/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

In src/ports/**, do not import Node built-ins or src/infrastructure/**; ports must stay abstract and depend only on boundary-safe types.

Files:

src/ports/MaterializationStorePort.ts

src/domain/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/domain/**/*.{ts,tsx,js,jsx}: In src/domain/**, do not use Date.now(), new Date(), Date(), performance.now(), Math.random(), crypto.randomUUID(), crypto.getRandomValues(), setTimeout, setInterval, raw new Error(...)/new TypeError(...), or direct imports from Node built-ins; time, entropy, and external capabilities must enter through ports or parameters, and domain errors should extend WarpError.
Construct domain objects only in core when doing so establishes validated runtime truth; do not build infrastructure adapters, host APIs, persistence implementations, wall clocks, or entropy sources inside core.
Prefer discriminated unions and explicit result types instead of boolean-flag bags, and model expected failures as return values rather than exceptions.
src/domain/ must not import host APIs or Node-specific globals; hexagonal architecture boundaries are mandatory.
Domain code must not use the wall clock directly; time must enter through a port or parameter.

Files:

src/domain/services/controllers/MaterializationWorkspaceCleanup.ts
src/domain/services/controllers/MaterializeController.ts
src/domain/services/controllers/MaterializeStrategyRuntime.ts
src/domain/services/controllers/MaterializeLiveStrategy.ts

src/domain/**/!(*.test).{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use explicit domain concepts with validated constructors, Object.freeze, and instanceof dispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.

Files:

src/domain/services/controllers/MaterializationWorkspaceCleanup.ts
src/domain/services/controllers/MaterializeController.ts
src/domain/services/controllers/MaterializeStrategyRuntime.ts
src/domain/services/controllers/MaterializeLiveStrategy.ts

🧠 Learnings (1)

📚 Learning: 2026-03-04T12:08:30.347Z

Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 63
File: package.json:126-126
Timestamp: 2026-03-04T12:08:30.347Z
Learning: Vitest 4 removes vite-node as a dependency and rewrites its pool system. Do not flag the absence of vite-node in package.json or lockfiles as an error for Vitest 4 projects. Use this as a general guideline: if a project uses Vitest 4, missing vite-node is expected and correct; only flag issues if there is evidence the project is not using Vitest 4 or if vite-node is explicitly required by the project.

Applied to files:

package.json

🪛 ast-grep (0.44.1)

test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts

[warning] 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 { execFile } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').