Skip to content

feat(materialize): add retained CAS materialization handles#751

Merged
flyingrobots merged 5 commits into
mainfrom
v19-cas-materialization-handles
Jul 16, 2026
Merged

feat(materialize): add retained CAS materialization handles#751
flyingrobots merged 5 commits into
mainfrom
v19-cas-materialization-handles

Conversation

@flyingrobots

@flyingrobots flyingrobots commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

  • define exact lane materialization coordinates, retained handles, and eight independently addressable root bundles
  • add a storage-neutral materialization store port backed by git-cas pages, ordered bundles, CacheSet retention, and retention witnesses
  • decode warm materialization handles with one bounded canonical bundle pass, rejecting missing, duplicate, unknown, wrong-kind, and excess members
  • wire materialization storage through repository and memory providers without exposing Git or git-cas through the public package surface
  • prove exact resume and transitive Git reachability with a fresh-adapter integration test and immediate prune dry run

Scope boundary

Refs #738
Refs #734

This foundation does not close #738. RuntimeHost still retains a complete WarpState; streaming reducer output, bounded handle-backed reads, constrained-memory proof, and normal-path resident-state removal remain follow-up slices.

Review

  • self-review found and fixed locale-sensitive frontier ordering before publication
  • Graft structural review found additive kernel modules with no removed symbols or signature changes
  • CodeRabbit findings fixed: SHA-1/SHA-256 page-handle metadata, canonical root-name ownership, and tuple-derived root entries
  • package publication validator remains clean with no root-surface additions

Validation

  • npm run lint
  • npm run lint:sludge
  • npm run typecheck
  • npm run typecheck:policy
  • npm run typecheck:surface
  • focused materialization matrix: 61 passed
  • final stable pre-push matrix: 6,985 passed, 2 skipped
  • full coverage matrix: 7,081 passed, 2 skipped; 92.78% lines
  • new materialization domain and adapter files: 100% statements, branches, functions, and lines
  • real Git/git-cas integration verifies the materialization bundle, all root bundles, and child pages are not prunable with git prune -n --expire=now

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds immutable materialization identities, a Git CAS-backed materialization store, runtime service wiring, explicit crypto dependencies, comprehensive tests, and a higher Vitest coverage threshold.

Changes

Materialization storage

Layer / File(s) Summary
Materialization identity and storage contracts
src/domain/materialization/*, src/ports/MaterializationStorePort.ts, src/ports/RuntimeStorageProviderPort.ts
Defines immutable coordinates, roots, handles, retention requests, and runtime storage contracts.
Git CAS materialization adapter
src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
Stores descriptors and root bundles in Git CAS, supports exact coordinate lookup, and validates cached materialization integrity.
Runtime materialization wiring
src/infrastructure/adapters/GitCasRepositoryAdapter.ts
Exposes CAS cache/page capabilities and constructs the materialization store in runtime services.
In-memory storage support
test/helpers/InMemoryGitCasFacade.ts, test/helpers/MemoryRuntimeStorageAdapter.ts
Adds page/cache behavior and wires materialization storage into test services.
Materialization validation
test/unit/domain/materialization/*, test/unit/infrastructure/adapters/*, test/integration/infrastructure/adapters/*
Adds identity, adapter, persistence, pruning, malformed-state, and runtime-wiring coverage.
Explicit crypto wiring
bin/cli/commands/*, scripts/*, src/domain/warp/RuntimeHostBoot.ts, test/unit/*, test/integration/*, vitest.config.ts
Passes crypto implementations into runtime storage creation and raises the line coverage threshold.

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

Sequence Diagram(s)

sequenceDiagram
  participant RuntimeStorageProvider
  participant GitCasMaterializationStoreAdapter
  participant GitCasFacade
  RuntimeStorageProvider->>GitCasMaterializationStoreAdapter: retain coordinate, roots, and state hash
  GitCasMaterializationStoreAdapter->>GitCasFacade: write descriptor and root bundle
  GitCasFacade-->>GitCasMaterializationStoreAdapter: return cache result and retention witness
  RuntimeStorageProvider->>GitCasMaterializationStoreAdapter: findExact coordinate
  GitCasMaterializationStoreAdapter->>GitCasFacade: read cache entry and bundle members
  GitCasFacade-->>GitCasMaterializationStoreAdapter: return materialization handle
Loading

Possibly related issues

Possibly related PRs

Poem

A rabbit found roots in a CAS-bound tree,
With frozen coordinates, tidy as can be.
Crypto hopped in, shared from the start,
Handles held bundles close to the heart.
Tests watched pages, refs, and trails—
While coverage climbed on carrot rails.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR adds materialization primitives and storage, but it does not replace RuntimeHost._cachedState or add bounded reads and streaming reduction required by #738. Implement the RuntimeHost swap to MaterializationHandle, bounded support/page reads, and streaming reduction or diagnostic gating, then re-evaluate against #738.
Docstring Coverage ⚠️ Warning Docstring coverage is 4.92% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The added tests, helpers, and coverage tweak all support the materialization work, with no clearly unrelated code changes.
Title check ✅ Passed The title clearly summarizes the main change: retained CAS materialization handles.
Description check ✅ Passed The PR includes a clear summary, issue refs, and validation details, though the template sections aren’t followed exactly.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@github-actions

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@test/helpers/InMemoryGitCasFacade.ts`:
- Line 263: Update the PageHandle construction in InMemoryGitCasFacade to derive
hashAlgorithm from the returned oid length, matching the logic used by other
handle constructors. Remove the hard-coded sha1 value so SHA-256 OIDs produce
matching page-handle metadata.
🪄 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: 3ba08ecc-c6f8-4b46-a0fa-39590a0d523a

📥 Commits

Reviewing files that changed from the base of the PR and between 494d62c and 3b55d1a.

📒 Files selected for processing (25)
  • bin/cli/commands/doctor/stateCacheCapability.ts
  • bin/cli/commands/trust.ts
  • bin/cli/commands/verify-audit.ts
  • scripts/migrations/v17.0.0/openCheckpointMigrationStore.ts
  • scripts/v18.0.0/migrations/graph-model/V17RestoredPublicReadLegacyReadingBuilder.ts
  • src/domain/materialization/MaterializationCoordinate.ts
  • src/domain/materialization/MaterializationHandle.ts
  • src/domain/materialization/MaterializationRoots.ts
  • src/domain/warp/RuntimeHostBoot.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
  • src/infrastructure/adapters/GitCasRepositoryAdapter.ts
  • src/ports/MaterializationStorePort.ts
  • src/ports/RuntimeStorageProviderPort.ts
  • test/conformance/v18FirstUseOpticsHonesty.test.ts
  • test/helpers/InMemoryGitCasFacade.ts
  • test/helpers/MemoryRuntimeStorageAdapter.ts
  • test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts
  • test/integration/infrastructure/adapters/GitCasWarpStateCacheAdapter.retention.integration.test.ts
  • test/unit/domain/WarpCore.blobAutoConstruct.test.ts
  • test/unit/domain/WarpGraph.audit.test.ts
  • test/unit/domain/materialization/MaterializationIdentity.test.ts
  • test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts
  • test/unit/infrastructure/adapters/GitCasRepositoryAdapter.test.ts
  • test/unit/scripts/v16-to-v17-upgrade.test.ts
  • vitest.config.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/domain/WarpCore.blobAutoConstruct.test.ts
  • src/ports/MaterializationStorePort.ts
  • test/conformance/v18FirstUseOpticsHonesty.test.ts
  • src/domain/warp/RuntimeHostBoot.ts
  • test/unit/domain/WarpGraph.audit.test.ts
  • test/unit/scripts/v16-to-v17-upgrade.test.ts
  • vitest.config.ts
  • bin/cli/commands/trust.ts
  • scripts/migrations/v17.0.0/openCheckpointMigrationStore.ts
  • bin/cli/commands/verify-audit.ts
  • scripts/v18.0.0/migrations/graph-model/V17RestoredPublicReadLegacyReadingBuilder.ts
  • bin/cli/commands/doctor/stateCacheCapability.ts
  • test/integration/infrastructure/adapters/GitCasWarpStateCacheAdapter.retention.integration.test.ts
  • test/helpers/MemoryRuntimeStorageAdapter.ts
  • src/domain/materialization/MaterializationCoordinate.ts
  • test/unit/domain/materialization/MaterializationIdentity.test.ts
  • test/unit/infrastructure/adapters/GitCasRepositoryAdapter.test.ts
  • src/ports/RuntimeStorageProviderPort.ts
  • test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts
  • src/domain/materialization/MaterializationRoots.ts
  • src/domain/materialization/MaterializationHandle.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
  • src/infrastructure/adapters/GitCasRepositoryAdapter.ts
  • test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts
  • test/helpers/InMemoryGitCasFacade.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/warp/RuntimeHostBoot.ts
  • src/domain/materialization/MaterializationCoordinate.ts
  • src/ports/RuntimeStorageProviderPort.ts
  • src/domain/materialization/MaterializationRoots.ts
  • src/domain/materialization/MaterializationHandle.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
  • src/infrastructure/adapters/GitCasRepositoryAdapter.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/ports/RuntimeStorageProviderPort.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/warp/RuntimeHostBoot.ts
  • src/domain/materialization/MaterializationCoordinate.ts
  • src/domain/materialization/MaterializationRoots.ts
  • src/domain/materialization/MaterializationHandle.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/warp/RuntimeHostBoot.ts
  • src/domain/materialization/MaterializationCoordinate.ts
  • src/domain/materialization/MaterializationRoots.ts
  • src/domain/materialization/MaterializationHandle.ts
🪛 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').

(detect-child-process-typescript)

test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts

[warning] 291-291: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${field} dependency, 'u')
Note: [CWE-1333] Inefficient Regular Expression Complexity

(regexp-from-variable)

🔇 Additional comments (25)
bin/cli/commands/doctor/stateCacheCapability.ts (1)

5-5: LGTM!

Also applies to: 22-22

bin/cli/commands/trust.ts (1)

58-65: LGTM!

bin/cli/commands/verify-audit.ts (1)

4-4: LGTM!

Also applies to: 56-56

scripts/migrations/v17.0.0/openCheckpointMigrationStore.ts (1)

3-3: LGTM!

Also applies to: 21-21

test/unit/scripts/v16-to-v17-upgrade.test.ts (1)

75-75: LGTM!

vitest.config.ts (1)

33-33: LGTM!

scripts/v18.0.0/migrations/graph-model/V17RestoredPublicReadLegacyReadingBuilder.ts (1)

20-20: LGTM!

Also applies to: 151-151

src/domain/warp/RuntimeHostBoot.ts (1)

290-290: LGTM!

test/integration/infrastructure/adapters/GitCasWarpStateCacheAdapter.retention.integration.test.ts (1)

12-12: LGTM!

Also applies to: 99-99

test/unit/domain/WarpCore.blobAutoConstruct.test.ts (1)

5-5: LGTM!

Also applies to: 17-17

test/unit/domain/WarpGraph.audit.test.ts (1)

14-14: LGTM!

Also applies to: 166-166

src/ports/MaterializationStorePort.ts (1)

1-18: LGTM!

src/ports/RuntimeStorageProviderPort.ts (1)

7-11: LGTM!

Also applies to: 17-23, 25-36

src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts (1)

1-78: LGTM!

Also applies to: 80-130, 132-225, 227-372

src/infrastructure/adapters/GitCasRepositoryAdapter.ts (1)

19-21: LGTM!

Also applies to: 38-54, 91-107, 187-196

test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts (1)

1-173: LGTM!

test/helpers/MemoryRuntimeStorageAdapter.ts (1)

11-11: LGTM!

Also applies to: 53-102

test/conformance/v18FirstUseOpticsHonesty.test.ts (1)

46-98: LGTM!

test/unit/infrastructure/adapters/GitCasRepositoryAdapter.test.ts (1)

11-11: LGTM!

Also applies to: 130-131, 144-144, 163-163

src/domain/materialization/MaterializationCoordinate.ts (1)

1-42: LGTM!

Also applies to: 44-83

src/domain/materialization/MaterializationRoots.ts (1)

1-61: LGTM!

Also applies to: 63-85

src/domain/materialization/MaterializationHandle.ts (1)

1-44: LGTM!

Also applies to: 46-70

test/unit/domain/materialization/MaterializationIdentity.test.ts (1)

1-239: LGTM!

test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts (1)

1-514: LGTM!

test/helpers/InMemoryGitCasFacade.ts (1)

4-108: LGTM!

Also applies to: 243-262, 264-335, 444-482

Comment thread test/helpers/InMemoryGitCasFacade.ts Outdated
@github-actions

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts (1)

129-143: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract 'evictable' retention policy into a named constant.

CACHE_NAMESPACE and MAX_DESCRIPTOR_BYTES are already named constants at the top of the file, but the retention policy literal 'evictable' passed to cache.put is inlined. As per coding guidelines, "Avoid magic strings or numbers when a named constant should exist."

♻️ Suggested fix
+const RETENTION_POLICY = 'evictable';
+
 async `#retainBundle`(
   bundle: StagedBundle,
   coordinate: MaterializationCoordinate,
 ): Promise<StorageRetentionWitness> {
   const cache = await this.#cas.caches.open({ namespace: CACHE_NAMESPACE });
   const cacheKey = await this.#cacheKey(coordinate);
-  const stored = await cache.put(cacheKey, bundle.handle, { retention: 'evictable' });
+  const stored = await cache.put(cacheKey, bundle.handle, { retention: RETENTION_POLICY });
🤖 Prompt for 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.

In `@src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts` around
lines 129 - 143, Introduce a named constant for the `'evictable'` retention
policy alongside the existing constants, then update `#retainBundle` to pass that
constant in the cache.put options instead of the inline string.

Source: Coding guidelines

🤖 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/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts`:
- Around line 30-40: Make MaterializationRoots.ts the single source of truth by
exporting the canonical MATERIALIZATION_ROOT_NAMES tuple and deriving
MaterializationRootName from its element type. In
GitCasMaterializationStoreAdapter.ts, import and reuse that exported list,
removing the local duplicate declaration while keeping
MATERIALIZATION_MEMBER_COUNT derived from the imported list.

---

Outside diff comments:
In `@src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts`:
- Around line 129-143: Introduce a named constant for the `'evictable'`
retention policy alongside the existing constants, then update `#retainBundle` to
pass that constant in the cache.put options instead of the inline string.
🪄 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: 22ad1c0d-918b-4aa5-a274-3e771b00933b

📥 Commits

Reviewing files that changed from the base of the PR and between 3b55d1a and 701f4c7.

📒 Files selected for processing (4)
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
  • src/infrastructure/adapters/GitCasRepositoryAdapter.ts
  • test/helpers/InMemoryGitCasFacade.ts
  • test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: test-node (22)
  • GitHub Check: coverage-threshold
  • GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (2)
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/infrastructure/adapters/GitCasRepositoryAdapter.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
**/*.{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:

  • src/infrastructure/adapters/GitCasRepositoryAdapter.ts
  • test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
  • test/helpers/InMemoryGitCasFacade.ts
🔇 Additional comments (10)
test/helpers/InMemoryGitCasFacade.ts (2)

242-249: Derive the page hash algorithm from the returned OID.

#putPage hard-codes sha1, while the same facade derives the algorithm from OID length for assets and bundles. A SHA-256 repository can return a 64-character OID, producing inconsistent PageHandle metadata and breaking handle round-tripping. Use oid.length === 64 ? 'sha256' : 'sha1'.

Also applies to: 251-322


47-106: LGTM!

test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts (2)

159-166: LGTM!

Also applies to: 528-541


167-170: 🎯 Functional Correctness

Assert the count-specific error for “too many members.”

Because the added member uses roots/unknown, this case can also fail through unknown-root validation. Verify that the test asserts the adapter’s exact materialization bundle has too many members error; otherwise it does not specifically cover the member-count guard.

src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts (4)

70-127: LGTM!


145-213: LGTM! The switch to a single iterateMembers pass with member-count bounding, duplicate/unknown-member rejection, and lane/coordinate cross-checks in #resolveHit is solid.


286-383: LGTM! Root reconstruction correctly enforces exact membership (no missing/extra/duplicate roots) via requireRoot and parseRootName's strict allowlist.


44-50: 🗄️ Data Integrity & Integration

Confirm git-cas capability types don't reach the public package surface. Both GitCasMaterializationFacade and the GitCasFacade fields derived from it are built directly on @git-stunts/git-cas capability types (BundleCapability, PageCapability, CacheSet), and both are exported from their adapter files — the shared root cause is that no boundary currently prevents these git-cas-shaped types from being re-exported publicly, which would contradict the PR's stated goal of not exposing Git/git-cas through the public surface.

  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts#L44-L50: confirm GitCasMaterializationFacade (and its git-cas-derived members) is not re-exported from any public barrel/index.
  • src/infrastructure/adapters/GitCasRepositoryAdapter.ts#L48-L49: confirm GitCasFacade.caches/.pages (sourced from the type above) are likewise confined to internal wiring and not part of the public API.
src/infrastructure/adapters/GitCasRepositoryAdapter.ts (2)

91-107: LGTM! materializations wiring follows the same request-driven construction pattern as the other stores in createRuntimeStorageServices, and _createMaterializationStore correctly threads the request-scoped codec/crypto ports plus timelineName as the lane identifier.

Also applies to: 187-197


47-47: 🗄️ Data Integrity & Integration

No remaining bundles.getMember usages
The narrower bundles type is fine; nothing in the TypeScript codebase still calls bundles.getMember.

			> Likely an incorrect or invalid review comment.

Comment thread src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts Outdated
@github-actions

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

1 similar comment
@github-actions

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
test/helpers/InMemoryGitCasFacade.ts (1)

289-300: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Don't return an anchored cache witness without persisting the root. #openCache fabricates a RetentionWitness for refs/cas/caches/${namespace}, but it only updates #cacheEntries; nothing writes that cache root through #history. Tests can therefore observe anchored reachability without any real Git reachability, which weakens prune-protection and reachability coverage. Persist the namespace root in history, or make this helper explicitly metadata-only and avoid reporting anchored.

🤖 Prompt for 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.

In `@test/helpers/InMemoryGitCasFacade.ts` around lines 289 - 300, Update
`#openCache` so the cache root for refs/cas/caches/${namespace} is persisted
through `#history` before returning the RetentionWitness with anchored
reachability. Ensure the persisted history entry uses the same namespace,
generation, and root path represented by the witness; otherwise make the helper
metadata-only and change the witness reachability from anchored.

Source: MCP tools

🤖 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/materialization/MaterializationRoots.ts`:
- Around line 4-15: Update the entries() implementation in MaterializationRoots
to derive its root names from MATERIALIZATION_ROOT_NAMES instead of hardcoding
them. Centralize the name-to-handle mapping and iterate the canonical tuple so
member counts, parsing, and bundle writing remain synchronized when roots are
added.

---

Outside diff comments:
In `@test/helpers/InMemoryGitCasFacade.ts`:
- Around line 289-300: Update `#openCache` so the cache root for
refs/cas/caches/${namespace} is persisted through `#history` before returning the
RetentionWitness with anchored reachability. Ensure the persisted history entry
uses the same namespace, generation, and root path represented by the witness;
otherwise make the helper metadata-only and change the witness reachability from
anchored.
🪄 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: c7e37dc1-c5bd-4dca-bf90-2c78551d8f5b

📥 Commits

Reviewing files that changed from the base of the PR and between 701f4c7 and a66466b.

📒 Files selected for processing (5)
  • src/domain/materialization/MaterializationRoots.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
  • test/helpers/InMemoryGitCasFacade.ts
  • test/unit/domain/materialization/MaterializationIdentity.test.ts
  • test/unit/helpers/InMemoryGitCasFacade.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: coverage-threshold
  • GitHub Check: test-node (22)
  • GitHub Check: test-bun
  • GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/helpers/InMemoryGitCasFacade.test.ts
  • test/unit/domain/materialization/MaterializationIdentity.test.ts
  • src/domain/materialization/MaterializationRoots.ts
  • test/helpers/InMemoryGitCasFacade.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.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/domain/materialization/MaterializationRoots.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.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/materialization/MaterializationRoots.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/materialization/MaterializationRoots.ts
🔇 Additional comments (11)
src/domain/materialization/MaterializationRoots.ts (2)

15-26: LGTM!

Also applies to: 28-50, 66-71, 78-83, 85-89, 91-93


73-76: 📐 Maintainability & Code Quality

No change needed: MATERIALIZATION_ROOT_NAMES is already readonly. defineRootNames yields a readonly [...] tuple at the call site, so .push(...) is already a type error; Readonly<Names> is unnecessary.

			> Likely an incorrect or invalid review comment.
src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts (1)

1-33: LGTM!

Also applies to: 34-60, 61-119, 120-135, 137-174, 176-187, 189-205, 208-233, 234-290, 291-355, 356-375, 376-438

test/unit/domain/materialization/MaterializationIdentity.test.ts (1)

4-7: LGTM!

Also applies to: 110-111

test/unit/helpers/InMemoryGitCasFacade.test.ts (1)

1-22: LGTM!

test/helpers/InMemoryGitCasFacade.ts (6)

4-19: LGTM!


50-63: LGTM!

Also applies to: 80-86


96-107: LGTM!


242-260: LGTM!

Also applies to: 262-276


434-450: LGTM!


283-323: 🗄️ Data Integrity & Integration

Model the cache fake’s full contract The helper drops expiresAt, expectedHandle, and logicalBytes, so tests won’t catch TTL, CAS, or size-accounting bugs.

Comment thread src/domain/materialization/MaterializationRoots.ts
@github-actions

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace RuntimeHost full state with CAS-backed materialization handles

1 participant