Read retained properties through git-cas 6.5#762
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe PR adds exact retained node-property reads using bounded, collision-safe shards; stages trie and property artifacts through git-cas workspaces; migrates materialization descriptors and cache keys to schema v3; and adds lifecycle, validation, pruning, and integration coverage. Retained property read flow
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Release Preflight
If this PR is from a |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/helpers/MockIndexStorage.ts (1)
48-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn
nullonly for own shard-map entries.A missing path such as
constructorortoStringresolves throughObject.prototype, sodecodeShardAtattempts to decode a non-AssetHandleinstead of reporting a missing shard. CheckObject.hasOwn()before reading the map.Proposed fix
override async readShardHandle( indexHandle: BundleHandle, path: string, ): Promise<AssetHandle | null> { - return (this.#indexes.get(indexHandle.toString()) ?? {})[path] ?? null; + const shards = this.#indexes.get(indexHandle.toString()); + return shards !== undefined && Object.hasOwn(shards, path) + ? shards[path] ?? null + : null; }🤖 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/MockIndexStorage.ts` around lines 48 - 90, Update readShardHandle in MockIndexStorage to return an AssetHandle only when the requested path is an own property of the shard map, using Object.hasOwn before accessing the entry; otherwise return null. This ensures decodeShardAt preserves missing-shard behavior for inherited names such as constructor and toString.
🤖 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/topics/cas-first-memoized-materialization.md`:
- Around line 216-222: Update the staged-shard lifecycle description in the
documentation to state that staged pages and bundles are retained immediately as
active workspace roots. Remove the outdated Git unreachable-object grace-period
limitation, concurrent-pruning warning, and git-cas issue 75 reference, while
preserving the surrounding CAS ownership and checkpointing guidance.
In `@package.json`:
- Line 120: Update the Unreleased entry in CHANGELOG.md to state that
`@git-stunts/git-cas` was upgraded to ^6.5.0, matching the dependency version
declared in package.json.
In `@src/domain/services/controllers/MaterializeSessionBridge.ts`:
- Around line 229-238: Update the property-shard encoding/staging flow around
store.writeShards and WarpStream.from<IndexShard> to enforce
MATERIALIZATION_PROPERTY_SHARD_READ_LIMITS before promotion. Apply the
retained-read CBOR structural limits during encoding, while preserving the
existing shard count, byte, and staging constraints.
In `@src/domain/services/index/PropertyIndexBuilder.ts`:
- Around line 22-33: Validate options.schemaVersion in the PropertyIndexBuilder
constructor before assigning _schemaVersion, restricting it to the supported
schema version values and rejecting unsupported runtime numbers. Use a validated
domain representation or constructor for the schema version, while preserving
the default version of 1 when no value is provided.
In `@src/domain/services/index/PropertyIndexReader.ts`:
- Line 19: Remove the duplicate IndexedPropertyBag declaration and update the
remaining alias to use a readonly string index signature, preserving PropValue
as the value type to reflect that decoded bags are frozen.
In `@src/infrastructure/adapters/CborIndexShardWriter.ts`:
- Around line 128-138: Update validatedStaging to verify that value is a
non-null object before accessing stagePage or stageOrderedBundle, while
continuing to accept undefined and valid staging ports. Throw the existing
IndexError with code E_INDEX_INVALID_STORAGE for null, primitives, or objects
missing either operation.
In `@src/infrastructure/adapters/CborIndexStoreAdapter.ts`:
- Around line 275-279: Update the stream-processing loop around
appendBoundedShardChunk to track every yielded chunk, including empty ones, with
a separate chunk-count limit before the empty-chunk early return. Ensure
requireShardChunkCount receives the total number of yielded chunks so an
indefinite stream of empty chunks cannot bypass the availability guard, while
preserving the existing byte-limit accounting.
In `@src/ports/IndexStorePort.ts`:
- Around line 8-21: Update IndexShardWriteOptions to a discriminated union
requiring maxShardBytes whenever memberStorage is 'page', while preserving the
valid 'asset' configuration. Replace the optional fields in
IndexShardDecodeOptions with one optional nested structure-limits object whose
internal fields are all required, matching the adapters’ accepted configurations
and rejecting partial limits at compile time.
In `@test/helpers/InMemoryMaterializationStore.ts`:
- Around line 39-49: Update InMemoryMaterializationStore’s stagePage() and
stageOrderedBundle() methods to check the released state before allocating
handles, and reject staging after the workspace has been released, matching the
production workspace mutability guard. Preserve the existing handle generation
and successful staging behavior when the workspace remains active.
In `@test/unit/scripts/v18-v17-fixture-wet-run-harness.test.ts`:
- Around line 95-105: Update the determinism test around
runV17GoldenGraphFixtureWetRun to create two independent temporary target
directories, execute the harness separately for each, and format each returned
result once with formatV17GoldenGraphFixtureWetRunReport. Compare the two
reports for equality and ensure neither report contains its corresponding target
directory; remove the current same-result double-formatting assertion.
---
Outside diff comments:
In `@test/helpers/MockIndexStorage.ts`:
- Around line 48-90: Update readShardHandle in MockIndexStorage to return an
AssetHandle only when the requested path is an own property of the shard map,
using Object.hasOwn before accessing the entry; otherwise return null. This
ensures decodeShardAt preserves missing-shard behavior for inherited names such
as constructor and toString.
🪄 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: f96e1988-2d7b-4cc8-9fc7-40f58d1c3761
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (65)
CHANGELOG.mddocs/topics/cas-first-memoized-materialization.mdpackage.jsonsrc/domain/RuntimeHost.tssrc/domain/materialization/MaterializationPropertyProfile.tssrc/domain/materialization/MaterializationRoot.tssrc/domain/materialization/MaterializationRoots.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/orset/route/RouteKey.tssrc/domain/orset/session/StateSession.tssrc/domain/orset/trie/TrieFlusher.tssrc/domain/orset/trie/TrieStorePort.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeDeps.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/domain/services/controllers/QueryReads.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/services/index/PropertyIndexBuilder.tssrc/domain/services/index/PropertyIndexReader.tssrc/domain/warp/RuntimeHostBoot.tssrc/infrastructure/adapters/BoundedCborValidation.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.tssrc/infrastructure/adapters/CborIndexShardWriter.tssrc/infrastructure/adapters/CborIndexStoreAdapter.tssrc/infrastructure/adapters/GitCasMaterializationBundle.tssrc/infrastructure/adapters/GitCasMaterializationDescriptor.tssrc/infrastructure/adapters/GitCasMaterializationLease.tssrc/infrastructure/adapters/GitCasMaterializationStoreAdapter.tssrc/infrastructure/adapters/GitCasMaterializationWorkspace.tssrc/infrastructure/adapters/GitCasRepositoryAdapter.tssrc/infrastructure/adapters/GitCasTrieStoreAdapter.tssrc/infrastructure/adapters/IndexShardEncodeTransform.tssrc/infrastructure/adapters/IndexShardLimitValidation.tssrc/infrastructure/adapters/PropertyShardEncodedSizeGuard.tssrc/ports/ArtifactStagingPort.tssrc/ports/IndexStorePort.tssrc/ports/MaterializationReadPort.tssrc/ports/MaterializationStorePort.tssrc/ports/MaterializationWorkspacePort.tstest/helpers/InMemoryGitCasFacade.tstest/helpers/InMemoryMaterializationStore.tstest/helpers/MockIndexStorage.tstest/integration/api/materialization.retainedResume.test.tstest/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.tstest/unit/domain/materialization/MaterializationIdentity.test.tstest/unit/domain/materialization/TrieMaterializationReader.test.tstest/unit/domain/orset/session/StateSession.test.tstest/unit/domain/services/PropertyIndex.test.tstest/unit/domain/services/controllers/MaterializationWorkspaceCleanup.test.tstest/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.tstest/unit/domain/services/controllers/MaterializeController.propertyRootRetention.test.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tstest/unit/domain/services/controllers/QueryController.test.tstest/unit/infrastructure/CborIndexStoreAdapter.test.tstest/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.tstest/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.lifecycle.test.tstest/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.tstest/unit/infrastructure/adapters/GitCasMaterializationWorkspace.test.tstest/unit/infrastructure/adapters/GitCasRepositoryAdapter.test.tstest/unit/infrastructure/adapters/GitCasTrieStoreAdapter.test.tstest/unit/ports/IndexStorePort.test.tstest/unit/scripts/v18-v17-fixture-wet-run-harness.test.tstest/unit/scripts/v18-v17-public-read-legacy-reading-builder.test.tsvitest.config.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: preflight
- GitHub Check: coverage-threshold
- GitHub Check: test-node (22)
🧰 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 fromsrc/infrastructure/**insrc/domain/**orsrc/ports/**; depend on a port instead.
Do not use direct Node built-ins insrc/domain/**orsrc/ports/**; use a port instead.
Files:
test/unit/scripts/v18-v17-public-read-legacy-reading-builder.test.tsvitest.config.tssrc/domain/orset/route/RouteKey.tstest/unit/infrastructure/adapters/GitCasRepositoryAdapter.test.tssrc/ports/MaterializationReadPort.tstest/unit/scripts/v18-v17-fixture-wet-run-harness.test.tstest/unit/domain/services/controllers/MaterializationWorkspaceCleanup.test.tssrc/domain/services/controllers/MaterializeDeps.tssrc/infrastructure/adapters/GitCasMaterializationLease.tssrc/ports/ArtifactStagingPort.tssrc/domain/warp/RuntimeHostBoot.tstest/unit/domain/materialization/MaterializationIdentity.test.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.tssrc/infrastructure/adapters/PropertyShardEncodedSizeGuard.tssrc/ports/MaterializationWorkspacePort.tssrc/domain/materialization/MaterializationPropertyProfile.tssrc/infrastructure/adapters/IndexShardLimitValidation.tssrc/domain/materialization/MaterializationRoot.tstest/helpers/MockIndexStorage.tssrc/ports/MaterializationStorePort.tssrc/infrastructure/adapters/BoundedCborValidation.tssrc/infrastructure/adapters/GitCasRepositoryAdapter.tssrc/infrastructure/adapters/GitCasMaterializationDescriptor.tstest/integration/api/materialization.retainedResume.test.tssrc/domain/services/controllers/ReadGraphHost.tstest/unit/ports/IndexStorePort.test.tssrc/domain/services/index/PropertyIndexBuilder.tssrc/infrastructure/adapters/GitCasMaterializationBundle.tssrc/domain/orset/session/StateSession.tstest/unit/infrastructure/adapters/GitCasTrieStoreAdapter.test.tssrc/domain/orset/trie/TrieStorePort.tstest/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.tstest/unit/domain/services/controllers/QueryController.test.tstest/unit/domain/orset/session/StateSession.test.tssrc/domain/services/controllers/QueryReads.tssrc/domain/orset/trie/TrieFlusher.tssrc/domain/materialization/MaterializationRoots.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/infrastructure/adapters/IndexShardEncodeTransform.tssrc/ports/IndexStorePort.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tssrc/domain/materialization/TrieMaterializationReader.tstest/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.lifecycle.test.tstest/unit/domain/services/PropertyIndex.test.tstest/helpers/InMemoryMaterializationStore.tssrc/domain/RuntimeHost.tssrc/infrastructure/adapters/CborIndexShardWriter.tstest/unit/domain/services/controllers/MaterializeController.propertyRootRetention.test.tssrc/domain/services/controllers/MaterializeController.tstest/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.tssrc/domain/services/controllers/MaterializeSessionBridge.tstest/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.tstest/unit/domain/materialization/TrieMaterializationReader.test.tssrc/domain/services/index/PropertyIndexReader.tstest/unit/infrastructure/adapters/GitCasMaterializationWorkspace.test.tssrc/infrastructure/adapters/GitCasTrieStoreAdapter.tssrc/infrastructure/adapters/GitCasMaterializationWorkspace.tstest/helpers/InMemoryGitCasFacade.tstest/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.tstest/unit/infrastructure/CborIndexStoreAdapter.test.tssrc/infrastructure/adapters/CborIndexStoreAdapter.tssrc/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,js,jsx}: Do not introduceany,as any,as unknown as,unknown(outside adapters),Record<string, unknown>(outside adapters),*Likeplaceholder types,JSON.parse/JSON.stringify(outside adapters),fetch(outside adapters),process.env(outside adapters),@ts-ignore, orz.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 createutils.ts,helpers.ts,misc.ts, orcommon.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 asUint8Array;Bufferbelongs in infrastructure adapters.
Files:
src/domain/orset/route/RouteKey.tssrc/ports/MaterializationReadPort.tssrc/domain/services/controllers/MaterializeDeps.tssrc/infrastructure/adapters/GitCasMaterializationLease.tssrc/ports/ArtifactStagingPort.tssrc/domain/warp/RuntimeHostBoot.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.tssrc/infrastructure/adapters/PropertyShardEncodedSizeGuard.tssrc/ports/MaterializationWorkspacePort.tssrc/domain/materialization/MaterializationPropertyProfile.tssrc/infrastructure/adapters/IndexShardLimitValidation.tssrc/domain/materialization/MaterializationRoot.tssrc/ports/MaterializationStorePort.tssrc/infrastructure/adapters/BoundedCborValidation.tssrc/infrastructure/adapters/GitCasRepositoryAdapter.tssrc/infrastructure/adapters/GitCasMaterializationDescriptor.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/services/index/PropertyIndexBuilder.tssrc/infrastructure/adapters/GitCasMaterializationBundle.tssrc/domain/orset/session/StateSession.tssrc/domain/orset/trie/TrieStorePort.tssrc/domain/services/controllers/QueryReads.tssrc/domain/orset/trie/TrieFlusher.tssrc/domain/materialization/MaterializationRoots.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/infrastructure/adapters/IndexShardEncodeTransform.tssrc/ports/IndexStorePort.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/RuntimeHost.tssrc/infrastructure/adapters/CborIndexShardWriter.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/domain/services/index/PropertyIndexReader.tssrc/infrastructure/adapters/GitCasTrieStoreAdapter.tssrc/infrastructure/adapters/GitCasMaterializationWorkspace.tssrc/infrastructure/adapters/CborIndexStoreAdapter.tssrc/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
src/domain/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/domain/**/*.{ts,tsx,js,jsx}: Insrc/domain/**, do not useDate.now(),new Date(),Date(),performance.now(),Math.random(),crypto.randomUUID(),crypto.getRandomValues(),setTimeout,setInterval, rawnew 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 extendWarpError.
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/orset/route/RouteKey.tssrc/domain/services/controllers/MaterializeDeps.tssrc/domain/warp/RuntimeHostBoot.tssrc/domain/materialization/MaterializationPropertyProfile.tssrc/domain/materialization/MaterializationRoot.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/services/index/PropertyIndexBuilder.tssrc/domain/orset/session/StateSession.tssrc/domain/orset/trie/TrieStorePort.tssrc/domain/services/controllers/QueryReads.tssrc/domain/orset/trie/TrieFlusher.tssrc/domain/materialization/MaterializationRoots.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/RuntimeHost.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/domain/services/index/PropertyIndexReader.ts
src/domain/**/!(*.test).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit domain concepts with validated constructors,
Object.freeze, andinstanceofdispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.
Files:
src/domain/orset/route/RouteKey.tssrc/domain/services/controllers/MaterializeDeps.tssrc/domain/warp/RuntimeHostBoot.tssrc/domain/materialization/MaterializationPropertyProfile.tssrc/domain/materialization/MaterializationRoot.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/services/index/PropertyIndexBuilder.tssrc/domain/orset/session/StateSession.tssrc/domain/orset/trie/TrieStorePort.tssrc/domain/services/controllers/QueryReads.tssrc/domain/orset/trie/TrieFlusher.tssrc/domain/materialization/MaterializationRoots.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/RuntimeHost.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/domain/services/index/PropertyIndexReader.ts
src/ports/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
In
src/ports/**, do not import Node built-ins orsrc/infrastructure/**; ports must stay abstract and depend only on boundary-safe types.
Files:
src/ports/MaterializationReadPort.tssrc/ports/ArtifactStagingPort.tssrc/ports/MaterializationWorkspacePort.tssrc/ports/MaterializationStorePort.tssrc/ports/IndexStorePort.ts
🧠 Learnings (2)
📚 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
📚 Learning: 2026-03-08T19:50:17.519Z
Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 65
File: CHANGELOG.md:88-88
Timestamp: 2026-03-08T19:50:17.519Z
Learning: Follow the Keep a Changelog convention for CHANGELOG.md. Allow duplicate subheadings across versions (e.g., '### Added', '### Fixed'). Configure markdownlint MD024 with {"siblings_only": true} to avoid cross-version false positives.
Applied to files:
CHANGELOG.md
🪛 ast-grep (0.44.1)
test/integration/api/materialization.retainedResume.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)
[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)
[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/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)
🔇 Additional comments (65)
src/infrastructure/adapters/CborCheckpointStoreAdapter.ts (1)
51-51: LGTM!Also applies to: 268-268
test/helpers/InMemoryGitCasFacade.ts (1)
19-31: LGTM!Also applies to: 56-76, 86-90, 106-110, 122-124, 147-156, 345-349, 442-543, 587-639
test/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.ts (1)
258-259: LGTM!Also applies to: 280-282, 304-306
test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.lifecycle.test.ts (1)
1-186: LGTM!test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts (1)
17-17: LGTM!Also applies to: 30-30, 73-110, 119-136, 148-165, 180-187, 198-198, 326-326, 393-397, 486-497, 527-540, 590-602, 673-692, 705-705
test/unit/infrastructure/adapters/GitCasMaterializationWorkspace.test.ts (1)
1-108: LGTM!Also applies to: 121-136, 149-160
test/unit/infrastructure/adapters/GitCasRepositoryAdapter.test.ts (1)
133-133: LGTM!test/unit/ports/IndexStorePort.test.ts (1)
14-17: LGTM!Also applies to: 29-54
CHANGELOG.md (1)
75-89: LGTM!docs/topics/cas-first-memoized-materialization.md (1)
105-135: LGTM!Also applies to: 198-215
test/helpers/InMemoryMaterializationStore.ts (1)
21-21: LGTM!Also applies to: 160-173
test/integration/api/materialization.retainedResume.test.ts (1)
53-53: LGTM!Also applies to: 114-160, 236-246
test/unit/domain/orset/session/StateSession.test.ts (1)
466-472: LGTM!Also applies to: 484-490
test/unit/domain/services/controllers/MaterializationWorkspaceCleanup.test.ts (1)
66-72: LGTM!test/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.ts (1)
156-234: LGTM!Also applies to: 237-245, 255-264, 274-309, 341-364
test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts (1)
8-8: LGTM!Also applies to: 29-29, 59-81, 534-569
test/unit/domain/services/controllers/QueryController.test.ts (1)
122-122: LGTM!Also applies to: 233-264
test/unit/infrastructure/CborIndexStoreAdapter.test.ts (1)
2-5: LGTM!Also applies to: 15-36, 57-60, 120-174, 185-190, 193-216, 218-284, 304-324, 326-339, 341-363, 365-391, 393-472, 474-490, 505-546, 567-572
test/unit/infrastructure/adapters/GitCasTrieStoreAdapter.test.ts (1)
2-6: LGTM!Also applies to: 60-80, 223-224
test/helpers/MockIndexStorage.ts (1)
19-19: LGTM!test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts (1)
132-160: LGTM!Also applies to: 163-186
test/unit/domain/materialization/MaterializationIdentity.test.ts (1)
119-128: LGTM!Also applies to: 175-180
test/unit/domain/materialization/TrieMaterializationReader.test.ts (1)
1-17: LGTM!Also applies to: 50-142
test/unit/domain/services/PropertyIndex.test.ts (1)
5-9: LGTM!Also applies to: 38-41, 94-171
test/unit/domain/services/controllers/MaterializeController.propertyRootRetention.test.ts (1)
1-200: LGTM!src/domain/orset/route/RouteKey.ts (1)
103-104: LGTM!test/unit/scripts/v18-v17-public-read-legacy-reading-builder.test.ts (1)
52-52: LGTM!vitest.config.ts (1)
33-33: LGTM!src/domain/RuntimeHost.ts (1)
71-71: LGTM!Also applies to: 260-261, 408-408, 430-447
src/domain/materialization/MaterializationRoot.ts (1)
31-39: LGTM!src/domain/materialization/MaterializationRoots.ts (1)
68-72: LGTM!src/domain/services/controllers/MaterializeController.ts (1)
39-39: LGTM!Also applies to: 168-170, 279-285, 348-358, 394-396, 432-434
src/domain/services/controllers/MaterializeLiveStrategy.ts (1)
18-18: LGTM!Also applies to: 90-108, 390-434
src/domain/services/controllers/QueryReads.ts (1)
136-155: LGTM!Also applies to: 171-174
src/domain/warp/RuntimeHostBoot.ts (1)
346-363: LGTM!src/ports/MaterializationStorePort.ts (1)
25-29: LGTM!src/domain/materialization/MaterializationPropertyProfile.ts (1)
1-43: LGTM!src/domain/services/index/PropertyIndexBuilder.ts (1)
35-57: LGTM!src/infrastructure/adapters/IndexShardLimitValidation.ts (1)
1-50: LGTM!src/domain/services/controllers/MaterializeSessionBridge.ts (2)
9-9: LGTM!Also applies to: 27-35, 55-56, 74-130, 189-228, 239-267
317-319: 🗄️ Data Integrity & IntegrationNo issue: this path returns a live
WarpState.projectFrameToStatecan passframe.prop,frame.observedFrontier, andframe.edgeBirthEventthrough directly;ReducerSessionFrameisn’t mutated after projection, andWarpStateintentionally stores live collections by reference.> Likely an incorrect or invalid review comment.src/domain/materialization/TrieMaterializationReader.ts (1)
2-16: LGTM!Also applies to: 25-31, 40-42, 62-88, 134-144
src/domain/services/controllers/MaterializeDeps.ts (1)
5-5: LGTM!Also applies to: 26-26
src/domain/services/index/PropertyIndexReader.ts (1)
20-266: LGTM!Also applies to: 313-313, 324-324, 374-377
src/infrastructure/adapters/PropertyShardEncodedSizeGuard.ts (1)
1-98: LGTM!src/infrastructure/adapters/IndexShardEncodeTransform.ts (1)
10-14: LGTM!Also applies to: 28-36, 72-78, 95-122
src/infrastructure/adapters/BoundedCborValidation.ts (1)
1-202: LGTM!src/infrastructure/adapters/CborIndexStoreAdapter.ts (1)
1-9: LGTM!Also applies to: 21-42, 70-84, 111-114, 141-158, 183-269, 299-370
src/infrastructure/adapters/CborIndexShardWriter.ts (1)
1-127: LGTM!Also applies to: 142-200
src/ports/ArtifactStagingPort.ts (1)
1-24: LGTM!src/ports/IndexStorePort.ts (1)
50-57: LGTM!Also applies to: 80-85, 101-115
src/domain/orset/trie/TrieFlusher.ts (1)
14-14: LGTM!Also applies to: 24-24, 70-75, 176-176, 195-195
src/domain/services/controllers/ReadGraphHost.ts (1)
11-11: LGTM!Also applies to: 33-35
src/ports/MaterializationReadPort.ts (1)
2-13: LGTM!src/domain/orset/session/StateSession.ts (1)
104-109: LGTM!Also applies to: 444-446
src/domain/orset/trie/TrieStorePort.ts (1)
2-20: LGTM!Also applies to: 82-82, 91-91
src/infrastructure/adapters/GitCasMaterializationBundle.ts (1)
1-137: LGTM!src/infrastructure/adapters/GitCasMaterializationDescriptor.ts (1)
12-12: LGTM!Also applies to: 124-135
src/infrastructure/adapters/GitCasMaterializationLease.ts (1)
1-75: LGTM!src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts (2)
8-13: LGTM!Also applies to: 25-28, 37-63, 72-77, 100-207, 209-232, 238-318, 344-368, 380-385, 396-431, 442-448
233-237: 🩺 Stability & AvailabilityKeep the existing lease on a miss.
nullfrom#openLeasemeans no replacement was opened, so retiring#currentLeasehere would drop the last valid acquisition unnecessarily.> Likely an incorrect or invalid review comment.src/infrastructure/adapters/GitCasMaterializationWorkspace.ts (1)
3-143: LGTM!Also applies to: 159-170, 182-268
src/infrastructure/adapters/GitCasRepositoryAdapter.ts (1)
49-53: LGTM!src/infrastructure/adapters/GitCasTrieStoreAdapter.ts (1)
4-10: LGTM!Also applies to: 31-31, 54-56, 83-83, 95-123, 134-134, 159-164
src/ports/MaterializationWorkspacePort.ts (1)
5-10: LGTM!Also applies to: 23-23
Release Preflight
If this PR is from a |
Self-code review: Code Lawyer@codex please confirm that the review resolutions and remaining risk assessment below are accurate.
All ten inline findings and the outside-diff Structural review with Graft found three branch-level internal breaks: the intentionally changed Validation on
Residual scope is explicit rather than hidden: this PR accelerates exact retained reads, but compatible-predecessor suffix replay remains follow-up work under #738/#739. |
There was a problem hiding this comment.
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/CborIndexStoreAdapter.ts (1)
152-173: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAllow
scanShards()to read page-backed members.
writeShards()now acceptsmemberStorage: 'page', but Line 160 rejects those members throughrequireAssetMember. Scanning a page-backed bundle therefore throws instead of returning its shards. Route recognized members throughreadMemberBytes()asdecodeShardAt()already does.Proposed fix
- const handle = requireAssetMember( - member.path, - member.handle.kind, - member.handle.toString(), - ); const shard = tryClassifyPath(member.path); if (shard === null) { continue; } - const bytes = await collectAsyncIterable(adapter._assets.open(handle)); + const bytes = await readMemberBytes({ + member, + path: member.path, + maxBytes: undefined, + cas: adapter._cas, + assets: adapter._assets, + }); const data = adapter._codec.decode(bytes);🤖 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/CborIndexStoreAdapter.ts` around lines 152 - 173, Update scanShards() to read recognized bundle members through readMemberBytes() instead of requiring requireAssetMember() and opening assets directly, so both asset-backed and page-backed members are supported. Preserve the existing path uniqueness check, classification filtering, decoding, and shard yielding behavior.
🤖 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.
Outside diff comments:
In `@src/infrastructure/adapters/CborIndexStoreAdapter.ts`:
- Around line 152-173: Update scanShards() to read recognized bundle members
through readMemberBytes() instead of requiring requireAssetMember() and opening
assets directly, so both asset-backed and page-backed members are supported.
Preserve the existing path uniqueness check, classification filtering, decoding,
and shard yielding behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 34b04ac9-e0d0-4cef-ad16-6c9e98e6f3e1
📒 Files selected for processing (23)
CHANGELOG.mddocs/topics/cas-first-memoized-materialization.mdsrc/domain/materialization/MaterializationPropertyProfile.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/domain/services/index/PropertyIndexBuilder.tssrc/domain/services/index/PropertyIndexReader.tssrc/infrastructure/adapters/CborIndexShardWriter.tssrc/infrastructure/adapters/CborIndexStoreAdapter.tssrc/infrastructure/adapters/GitCasMaterializationLease.tssrc/infrastructure/adapters/IndexShardEncodeTransform.tssrc/infrastructure/adapters/IndexShardLimitValidation.tssrc/ports/IndexStorePort.tstest/helpers/InMemoryMaterializationStore.tstest/helpers/MockIndexStorage.tstest/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.tstest/unit/domain/materialization/TrieMaterializationReader.test.tstest/unit/domain/services/PropertyIndex.test.tstest/unit/helpers/StorageTestDoubles.test.tstest/unit/infrastructure/CborIndexStoreAdapter.test.tstest/unit/infrastructure/adapters/GitCasMaterializationLease.test.tstest/unit/ports/IndexStorePort.test.tstest/unit/scripts/v18-v17-fixture-wet-run-harness.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 (5)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not use direct imports fromsrc/infrastructure/**insrc/domain/**orsrc/ports/**; depend on a port instead.
Do not use direct Node built-ins insrc/domain/**orsrc/ports/**; use a port instead.
Files:
test/unit/infrastructure/adapters/GitCasMaterializationLease.test.tstest/unit/helpers/StorageTestDoubles.test.tssrc/infrastructure/adapters/GitCasMaterializationLease.tstest/unit/scripts/v18-v17-fixture-wet-run-harness.test.tstest/unit/domain/materialization/TrieMaterializationReader.test.tssrc/infrastructure/adapters/IndexShardLimitValidation.tssrc/infrastructure/adapters/IndexShardEncodeTransform.tstest/unit/domain/services/PropertyIndex.test.tstest/helpers/MockIndexStorage.tssrc/domain/services/index/PropertyIndexBuilder.tssrc/domain/materialization/MaterializationPropertyProfile.tssrc/ports/IndexStorePort.tssrc/domain/materialization/TrieMaterializationReader.tstest/unit/ports/IndexStorePort.test.tssrc/domain/services/index/PropertyIndexReader.tstest/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.tstest/helpers/InMemoryMaterializationStore.tssrc/domain/services/controllers/MaterializeSessionBridge.tstest/unit/infrastructure/CborIndexStoreAdapter.test.tssrc/infrastructure/adapters/CborIndexShardWriter.tssrc/infrastructure/adapters/CborIndexStoreAdapter.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,js,jsx}: Do not introduceany,as any,as unknown as,unknown(outside adapters),Record<string, unknown>(outside adapters),*Likeplaceholder types,JSON.parse/JSON.stringify(outside adapters),fetch(outside adapters),process.env(outside adapters),@ts-ignore, orz.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 createutils.ts,helpers.ts,misc.ts, orcommon.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 asUint8Array;Bufferbelongs in infrastructure adapters.
Files:
src/infrastructure/adapters/GitCasMaterializationLease.tssrc/infrastructure/adapters/IndexShardLimitValidation.tssrc/infrastructure/adapters/IndexShardEncodeTransform.tssrc/domain/services/index/PropertyIndexBuilder.tssrc/domain/materialization/MaterializationPropertyProfile.tssrc/ports/IndexStorePort.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/services/index/PropertyIndexReader.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/infrastructure/adapters/CborIndexShardWriter.tssrc/infrastructure/adapters/CborIndexStoreAdapter.ts
src/domain/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/domain/**/*.{ts,tsx,js,jsx}: Insrc/domain/**, do not useDate.now(),new Date(),Date(),performance.now(),Math.random(),crypto.randomUUID(),crypto.getRandomValues(),setTimeout,setInterval, rawnew 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 extendWarpError.
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/index/PropertyIndexBuilder.tssrc/domain/materialization/MaterializationPropertyProfile.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/services/index/PropertyIndexReader.tssrc/domain/services/controllers/MaterializeSessionBridge.ts
src/domain/**/!(*.test).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit domain concepts with validated constructors,
Object.freeze, andinstanceofdispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.
Files:
src/domain/services/index/PropertyIndexBuilder.tssrc/domain/materialization/MaterializationPropertyProfile.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/services/index/PropertyIndexReader.tssrc/domain/services/controllers/MaterializeSessionBridge.ts
src/ports/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
In
src/ports/**, do not import Node built-ins orsrc/infrastructure/**; ports must stay abstract and depend only on boundary-safe types.
Files:
src/ports/IndexStorePort.ts
🧠 Learnings (1)
📚 Learning: 2026-03-08T19:50:17.519Z
Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 65
File: CHANGELOG.md:88-88
Timestamp: 2026-03-08T19:50:17.519Z
Learning: Follow the Keep a Changelog convention for CHANGELOG.md. Allow duplicate subheadings across versions (e.g., '### Added', '### Fixed'). Configure markdownlint MD024 with {"siblings_only": true} to avoid cross-version false positives.
Applied to files:
CHANGELOG.md
🪛 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)
[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)
🔇 Additional comments (24)
CHANGELOG.md (1)
53-53: LGTM!Also applies to: 75-89
docs/topics/cas-first-memoized-materialization.md (1)
105-135: LGTM!Also applies to: 198-220
test/helpers/MockIndexStorage.ts (1)
19-19: LGTM!Also applies to: 48-56, 84-94
test/unit/infrastructure/adapters/GitCasMaterializationLease.test.ts (1)
8-24: LGTM!Also applies to: 26-37, 39-50, 53-61
test/unit/ports/IndexStorePort.test.ts (1)
7-10: LGTM!Also applies to: 23-46, 57-70
test/unit/scripts/v18-v17-fixture-wet-run-harness.test.ts (1)
95-111: LGTM!test/unit/domain/materialization/TrieMaterializationReader.test.ts (1)
1-17: LGTM!Also applies to: 50-92, 94-112, 114-125, 127-135, 137-147
test/unit/domain/services/PropertyIndex.test.ts (1)
167-180: LGTM!test/unit/helpers/StorageTestDoubles.test.ts (1)
1-29: LGTM!test/unit/infrastructure/CborIndexStoreAdapter.test.ts (1)
7-7: LGTM!Also applies to: 27-27, 108-118, 248-252, 263-267, 307-332, 352-394, 412-416, 435-448, 469-502, 514-533, 624-654
test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts (1)
85-85: LGTM!Also applies to: 125-130
src/domain/materialization/TrieMaterializationReader.ts (1)
15-15: LGTM!Also applies to: 25-42, 63-88, 134-144
src/domain/services/controllers/MaterializeSessionBridge.ts (1)
32-32: LGTM!Also applies to: 55-56, 74-130, 191-269, 318-320
src/domain/materialization/MaterializationPropertyProfile.ts (1)
10-18: LGTM!src/domain/services/index/PropertyIndexBuilder.ts (1)
12-14: LGTM!Also applies to: 25-42, 56-60, 69-88
src/domain/services/index/PropertyIndexReader.ts (2)
308-324: LGTM!Also applies to: 373-374
19-19: 🎯 Functional CorrectnessNo change needed for
IndexedPropertyBag. It is declared only once here, so there’s no duplicate-identifier error.> Likely an incorrect or invalid review comment.src/infrastructure/adapters/IndexShardEncodeTransform.ts (1)
11-19: LGTM!Also applies to: 33-43, 57-102, 105-130
src/infrastructure/adapters/IndexShardLimitValidation.ts (1)
2-81: LGTM!src/infrastructure/adapters/CborIndexShardWriter.ts (1)
7-17: LGTM!Also applies to: 23-29, 51-73, 118-179
src/infrastructure/adapters/CborIndexStoreAdapter.ts (1)
210-243: LGTM!Also applies to: 269-329
src/ports/IndexStorePort.ts (1)
8-42: LGTM!Also applies to: 75-78, 102-106, 132-136
test/helpers/InMemoryMaterializationStore.ts (1)
21-68: LGTM!Also applies to: 170-179
src/infrastructure/adapters/GitCasMaterializationLease.ts (1)
8-71: LGTM!
Release Preflight
If this PR is from a |
1 participant
Summary
RuntimeHost.close()Issue
Refs #738.
Refs #739.
Refs #742.
This slice does not close those issues: incremental materialization still needs compatible-predecessor resume, and whole-state compatibility paths still exist.
Test plan
npm run typechecknpm run lintnpm run lint:mdnpm run typecheck:surfacenpm run test:coverage:ci: 586 files passed, 1 skipped; 7,222 tests passed, 2 skipped; 92.90% line coverageADR checks