Skip to content

fix: retain bounded materialization roots#755

Merged
flyingrobots merged 2 commits into
mainfrom
v19-paged-materialization-roots
Jul 16, 2026
Merged

fix: retain bounded materialization roots#755
flyingrobots merged 2 commits into
mainfrom
v19-paged-materialization-roots

Conversation

@flyingrobots

@flyingrobots flyingrobots commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

  • bound StateSession dirty-page residency by periodically flushing immutable trie pages and requiring a pinned git-cas workspace witness before rebasing
  • make terminal close two-phase: final pages remain dirty until the coordinate-keyed materialization returns an anchored retention witness
  • renew temporary workspace leases during long projection/promotion, serialize cleanup behind in-flight work, and use unique operation keys
  • resolve live coordinates independently of the legacy snapshot cache, retain exact materialization roots, and fix deep trie ancestor rebinding after reopened inserts
  • reject bounded compaction until resumable subtree compaction exists

Issue

Refs #738
Refs #734

This is a partial implementation of #738. It bounds mutated dirty pages, not total process memory. Full ORSet projection and bounded observer execution remain unfinished, and #738 remains incomplete and open.

Storage invariants

Periodic flushes cannot forget dirty pages before git-cas confirms anchored workspace retention. Terminal flushes cannot be accepted before final materialization retention. [cite: src/domain/orset/session/StateSession.ts#198-270@960159a02404fae2dfffaa13f4ac34ab72e76f09] [cite: src/domain/orset/session/StateSession.ts#411-466@960159a02404fae2dfffaa13f4ac34ab72e76f09]

Workspace checkpoint, lease renewal, promotion, and release are serialized; release waits for promotion, while renewal remains active during long final retention. [cite: src/infrastructure/adapters/GitCasMaterializationWorkspace.ts#95-245@960159a02404fae2dfffaa13f4ac34ab72e76f09]

Workspace keys are operation-unique, while final materializations remain coordinate-keyed through the existing git-cas CacheSet. [cite: src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts#90-126@960159a02404fae2dfffaa13f4ac34ab72e76f09]

The controller promotes final roots before accepting the prepared session and always releases its temporary workspace. [cite: src/domain/services/controllers/MaterializeController.ts#264-349@960159a02404fae2dfffaa13f4ac34ab72e76f09] [cite: src/domain/services/controllers/MaterializeSessionBridge.ts#49-114@960159a02404fae2dfffaa13f4ac34ab72e76f09]

Failure-unwind cleanup cannot replace the original operation failure, even if cleanup diagnostics themselves fail; successful operations still surface release failures. [cite: src/domain/services/controllers/MaterializationWorkspaceCleanup.ts#4-24@3ecb818dba0060c61ff6bbd9fe24a3e0f7625644] [cite: src/domain/services/controllers/MaterializeController.ts#289-294@3ecb818dba0060c61ff6bbd9fe24a3e0f7625644] [cite: src/domain/services/controllers/MaterializeSessionBridge.ts#111-113@3ecb818dba0060c61ff6bbd9fe24a3e0f7625644]

A recorded lease-renewal failure remains fail-closed for checkpoint/promotion, while release still removes the temporary cache root. [cite: src/infrastructure/adapters/GitCasMaterializationWorkspace.ts#174-182@3ecb818dba0060c61ff6bbd9fe24a3e0f7625644]

Live materialization captures its frontier even when the legacy snapshot cache is disabled. [cite: src/domain/services/controllers/MaterializeLiveStrategy.ts#30-76@960159a02404fae2dfffaa13f4ac34ab72e76f09]

Reopened deep trie writes propagate changed descendant handles through every ancestor. [cite: src/domain/orset/trie/TrieCursor.ts#561-595@960159a02404fae2dfffaa13f4ac34ab72e76f09]

Performance

On a fresh tiny repository, the safe path improved from approximately 7.44s to 2.39s by eliminating the redundant terminal workspace publication/removal. Existing query and traversal integrations again pass their original 10s hook limit.

Test plan

  • npm run test:coverage:ci: 580 files passed, 1 skipped; 7,116 tests passed, 2 skipped; 92.79% lines
  • full integration corpus before review follow-up: 21 files / 98 tests passed
  • focused real-Git adapter and retained-resume integration: 4/4 passed
  • focused review-regression suite: 44/44 passed
  • pre-push Ironclad firewall: all static, documentation, type, policy, public-surface, CAS-ownership, and stable-unit shards passed
  • ESLint, Semgrep, anti-sludge, Markdown, source-size, source/test typecheck, consumer typecheck, and publication-surface checks passed
  • independent Code Lawyer: APPROVE after adversarial failure-precedence review

ADR checks

  • This PR does not implement ADR 2 without satisfying ADR 3
  • Persisted op formats are unchanged; no ADR 3 readiness link is required
  • Wire compatibility is unchanged; canonical-only wire rejection is unaffected
  • Schema constants are unchanged; patch and checkpoint namespaces remain distinct

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

State sessions now support bounded dirty-page checkpointing through renewable CAS-backed workspaces. Materialization reduction carries explicit coordinates, accepts final retention witnesses, and releases workspaces on failure or empty results. Trie rebasing and ancestor rebinding preserve reopened checkpoint state.

Changes

Bounded session checkpointing

Layer / File(s) Summary
Session checkpointing and trie rebasing
src/domain/orset/session/StateSession.ts, src/domain/orset/shadow/ShadowTrieORSet.ts, src/domain/orset/trie/TrieCursor.ts
Sessions track dirty pages, checkpoint roots, validate witnesses, and use two-phase close; trie cursors support rebasing and ancestor rebinding.

CAS workspace retention

Layer / File(s) Summary
Workspace retention adapter
src/ports/MaterializationWorkspacePort.ts, src/ports/MaterializationStorePort.ts, src/infrastructure/adapters/GitCasMaterializationWorkspace.ts, src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
Materialization workspaces retain ordered Git-CAS bundles with expiring pinned cache entries, lease renewal, promotion, and release cleanup.

Materialization orchestration

Layer / File(s) Summary
Coordinate-aware reduction flow
src/domain/services/controllers/*, src/domain/warp/RuntimeHostBoot.ts
Reduction APIs now receive coordinates, open workspaces for state sessions, propagate retention callbacks, and release unused or failed workspaces.

Validation and regression coverage

Layer / File(s) Summary
Workspace, session, and materialization tests
test/helpers/*, test/unit/domain/orset/*, test/unit/domain/services/controllers/*, test/unit/infrastructure/adapters/*, test/integration/*, test/unit/domain/WarpGraph*, test/unit/domain/warp/*
Tests cover checkpointing, lease renewal, pruning, witness validation, cleanup, frontier streaming, and updated materialization paths.
Deterministic migration tests and changelog
test/unit/scripts/v18-graph-model-migration-command-cli.test.ts, CHANGELOG.md
Migration tests use a fixed scratch head, and unreleased notes describe bounded workspace retention and trie fixes.

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

Possibly related PRs

Sequence Diagram(s)

sequenceDiagram
  participant MaterializeStrategy
  participant MaterializeController
  participant MaterializeSessionBridge
  participant MaterializationWorkspace
  participant StateSession
  MaterializeStrategy->>MaterializeController: reduce patches with coordinate
  MaterializeController->>MaterializeSessionBridge: reduceSessionBackedState
  MaterializeSessionBridge->>MaterializationWorkspace: open workspace
  MaterializeSessionBridge->>StateSession: open with workspace
  StateSession->>MaterializationWorkspace: checkpoint dirty roots
  StateSession-->>MaterializeSessionBridge: prepared close roots
  MaterializeController->>MaterializationWorkspace: accept final retention
  MaterializationWorkspace-->>MaterializeController: retained materialization
Loading

Poem

A rabbit hops through pages bright,
Pinning roots through day and night.
Branches rebind, old leaves stay,
Leases renew along the way.
Failed workspaces softly flee—
CAS keeps every root debris-free.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The changes advance #738 with workspace retention and bounded residency, but they do not implement the issue's full RuntimeHost materialization-handle replacement. Either mark this as a partial step in the issue or add the missing RuntimeHost/cache replacement, bounded reads, and retained-handle resume pieces required by #738.
Docstring Coverage ⚠️ Warning Docstring coverage is 2.38% 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 diff is focused on materialization/workspace retention, trie rebasing, and supporting tests; no clearly unrelated feature work stands out.
Title check ✅ Passed The title is concise and directly reflects the main change: retaining bounded materialization roots.
Description check ✅ Passed The description includes Summary, Issue, Test plan, and ADR checks, and is detailed enough for the template.
✨ 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: 4

Caution

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

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

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

Duplicated StorageRetentionWitness construction for workspace-checkpoint doubles. Both sites hand-roll the same "cache-set"/"pinned" witness shape for a workspace checkpoint; extracting one shared factory would remove the drift risk if the witness contract changes.

  • test/helpers/InMemoryMaterializationStore.ts#L114-129: export workspaceRetentionWitness (or a parameterized buildCacheSetRetentionWitness(handle, { policy, namespace, generation })) so it can be imported by other test doubles instead of being reimplemented.
  • test/unit/domain/orset/session/StateSession.test.ts#L454-482: replace RecordingWorkspace.checkpoint's inline StorageRetentionWitness/StorageRetentionRoot construction with the shared exported helper from test/helpers/InMemoryMaterializationStore.ts.
🤖 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/InMemoryMaterializationStore.ts` at line 1, Extract the shared
workspace checkpoint StorageRetentionWitness construction from
InMemoryMaterializationStore into an exported helper, such as
workspaceRetentionWitness or a parameterized factory preserving handle, policy,
namespace, and generation. Update RecordingWorkspace.checkpoint in
StateSession.test.ts to use this helper instead of constructing
StorageRetentionWitness and StorageRetentionRoot inline, keeping the existing
witness values unchanged.
🤖 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 266-299: Update _buildResult so
params.reduced.workspace?.release() cannot replace an exception raised by the
try block; preserve and rethrow the original operation error while isolating any
release failure, and retain release behavior when the main operation succeeds.

In `@src/domain/services/controllers/MaterializeSessionBridge.ts`:
- Around line 108-112: Update the catch block in MaterializeSessionBridge to
preserve the original caught error when workspace.release() fails: attempt
cleanup without allowing its exception to replace raw, then rethrow raw. Apply
the same error-preserving cleanup pattern to the other shared call sites
referenced by the consolidated comment.

In `@src/infrastructure/adapters/GitCasMaterializationWorkspace.ts`:
- Around line 104-183: Update release() to perform best-effort cleanup without
throwing lease-health errors: preserve the release and cache-removal work, but
prevent `#assertLeaseHealthy`() failures from escaping during cleanup. Keep lease
health available through the existing state/error mechanisms rather than masking
the original failure.

In `@test/unit/domain/orset/session/StateSession.test.ts`:
- Around line 454-482: The RecordingWorkspace.checkpoint implementation
duplicates the retention-witness construction already provided by
workspaceRetentionWitness in InMemoryMaterializationStore. Reuse that shared
helper for the returned StorageRetentionWitness, supplying the test checkpoint
handle and generation values as needed, while preserving the existing checkpoint
recording and test-specific identifiers.

---

Outside diff comments:
In `@test/helpers/InMemoryMaterializationStore.ts`:
- Line 1: Extract the shared workspace checkpoint StorageRetentionWitness
construction from InMemoryMaterializationStore into an exported helper, such as
workspaceRetentionWitness or a parameterized factory preserving handle, policy,
namespace, and generation. Update RecordingWorkspace.checkpoint in
StateSession.test.ts to use this helper instead of constructing
StorageRetentionWitness and StorageRetentionRoot inline, keeping the existing
witness values unchanged.
🪄 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: 21ec0d15-31d3-4442-a29a-5377d6a9243a

📥 Commits

Reviewing files that changed from the base of the PR and between 71a97c4 and 960159a.

📒 Files selected for processing (31)
  • CHANGELOG.md
  • src/domain/orset/session/StateSession.ts
  • src/domain/orset/shadow/ShadowTrieORSet.ts
  • src/domain/orset/trie/TrieCursor.ts
  • src/domain/services/controllers/MaterializeController.ts
  • src/domain/services/controllers/MaterializeCoordinateStrategy.ts
  • src/domain/services/controllers/MaterializeLiveStrategy.ts
  • src/domain/services/controllers/MaterializeSessionBridge.ts
  • src/domain/services/controllers/MaterializeStrategyRuntime.ts
  • src/domain/warp/RuntimeHostBoot.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
  • src/infrastructure/adapters/GitCasMaterializationWorkspace.ts
  • src/ports/MaterializationStorePort.ts
  • src/ports/MaterializationWorkspacePort.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/WarpGraph.autoCheckpoint.test.ts
  • test/unit/domain/WarpGraph.patchCount.test.ts
  • test/unit/domain/WarpGraph.test.ts
  • test/unit/domain/WarpGraph.versionVector.test.ts
  • test/unit/domain/orset/session/StateSession.test.ts
  • test/unit/domain/orset/trie/TrieCursor.test.ts
  • test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts
  • test/unit/domain/services/controllers/MaterializeController.test.ts
  • test/unit/domain/services/controllers/MaterializePatchStreamReducer.test.ts
  • test/unit/domain/warp/hydrateCheckpointIndex.regression.test.ts
  • test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts
  • test/unit/infrastructure/adapters/GitCasMaterializationWorkspace.test.ts
  • test/unit/scripts/v18-graph-model-migration-command-cli.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: test-node (22)
  • GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (5)
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/MaterializationWorkspacePort.ts
  • src/domain/orset/shadow/ShadowTrieORSet.ts
  • src/domain/services/controllers/MaterializeCoordinateStrategy.ts
  • src/domain/services/controllers/MaterializeStrategyRuntime.ts
  • src/ports/MaterializationStorePort.ts
  • src/domain/orset/trie/TrieCursor.ts
  • src/domain/services/controllers/MaterializeSessionBridge.ts
  • src/infrastructure/adapters/GitCasMaterializationWorkspace.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
  • src/domain/services/controllers/MaterializeLiveStrategy.ts
  • src/domain/warp/RuntimeHostBoot.ts
  • src/domain/orset/session/StateSession.ts
  • src/domain/services/controllers/MaterializeController.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/MaterializationWorkspacePort.ts
  • src/ports/MaterializationStorePort.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/ports/MaterializationWorkspacePort.ts
  • src/domain/orset/shadow/ShadowTrieORSet.ts
  • src/domain/services/controllers/MaterializeCoordinateStrategy.ts
  • test/unit/domain/services/controllers/MaterializeController.test.ts
  • test/unit/domain/services/controllers/MaterializePatchStreamReducer.test.ts
  • test/unit/domain/WarpGraph.patchCount.test.ts
  • test/unit/domain/WarpGraph.versionVector.test.ts
  • test/unit/domain/WarpGraph.autoCheckpoint.test.ts
  • test/integration/api/materialization.retainedResume.test.ts
  • src/domain/services/controllers/MaterializeStrategyRuntime.ts
  • test/unit/infrastructure/adapters/GitCasMaterializationWorkspace.test.ts
  • test/unit/infrastructure/adapters/GitCasMaterializationStoreAdapter.test.ts
  • test/helpers/InMemoryMaterializationStore.ts
  • src/ports/MaterializationStorePort.ts
  • test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts
  • test/unit/scripts/v18-graph-model-migration-command-cli.test.ts
  • src/domain/orset/trie/TrieCursor.ts
  • src/domain/services/controllers/MaterializeSessionBridge.ts
  • src/infrastructure/adapters/GitCasMaterializationWorkspace.ts
  • test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts
  • test/unit/domain/orset/session/StateSession.test.ts
  • src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts
  • test/unit/domain/orset/trie/TrieCursor.test.ts
  • src/domain/services/controllers/MaterializeLiveStrategy.ts
  • test/unit/domain/warp/hydrateCheckpointIndex.regression.test.ts
  • test/helpers/InMemoryGitCasFacade.ts
  • src/domain/warp/RuntimeHostBoot.ts
  • src/domain/orset/session/StateSession.ts
  • test/unit/domain/WarpGraph.test.ts
  • src/domain/services/controllers/MaterializeController.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/orset/shadow/ShadowTrieORSet.ts
  • src/domain/services/controllers/MaterializeCoordinateStrategy.ts
  • src/domain/services/controllers/MaterializeStrategyRuntime.ts
  • src/domain/orset/trie/TrieCursor.ts
  • src/domain/services/controllers/MaterializeSessionBridge.ts
  • src/domain/services/controllers/MaterializeLiveStrategy.ts
  • src/domain/warp/RuntimeHostBoot.ts
  • src/domain/orset/session/StateSession.ts
  • src/domain/services/controllers/MaterializeController.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/orset/shadow/ShadowTrieORSet.ts
  • src/domain/services/controllers/MaterializeCoordinateStrategy.ts
  • src/domain/services/controllers/MaterializeStrategyRuntime.ts
  • src/domain/orset/trie/TrieCursor.ts
  • src/domain/services/controllers/MaterializeSessionBridge.ts
  • src/domain/services/controllers/MaterializeLiveStrategy.ts
  • src/domain/warp/RuntimeHostBoot.ts
  • src/domain/orset/session/StateSession.ts
  • src/domain/services/controllers/MaterializeController.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/unit/infrastructure/adapters/GitCasMaterializationWorkspace.test.ts

[warning] 353-353: 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)

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)


[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)

🔇 Additional comments (36)
test/helpers/InMemoryGitCasFacade.ts (1)

51-51: LGTM!

Also applies to: 105-107, 295-295, 323-323, 341-352

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

16-20: LGTM!

Also applies to: 87-154, 164-170, 181-187, 298-348

test/unit/domain/orset/trie/TrieCursor.test.ts (1)

120-125: LGTM!

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

17-17: LGTM!

Also applies to: 77-123, 519-519

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

1-492: LGTM!

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

634-638: LGTM!

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

137-148: LGTM!

test/unit/domain/warp/hydrateCheckpointIndex.regression.test.ts (1)

42-42: LGTM!

Also applies to: 61-70

test/unit/scripts/v18-graph-model-migration-command-cli.test.ts (1)

28-28: LGTM!

Also applies to: 75-81, 114-114, 151-151, 187-187

test/helpers/InMemoryMaterializationStore.ts (1)

15-129: LGTM!

test/integration/api/materialization.retainedResume.test.ts (1)

9-12: LGTM!

Also applies to: 84-95, 115-144

test/unit/domain/orset/session/StateSession.test.ts (1)

132-361: LGTM! Thorough coverage of bounded checkpointing, terminal-flush gating on a witnessed retention, null-witness handling, prepareClose failure recovery, and bounded-compaction rejection — all consistent with the StateSession contract shown in context.

test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts (1)

107-199: LGTM! Release-on-failure/empty/legacy-cache-disabled coverage is well aligned with MaterializeController._buildResult's finally { workspace?.release() } and _resolveMaterialization promotion path.

Also applies to: 202-367, 404-404, 440-440

CHANGELOG.md (1)

53-92: LGTM! Changelog entries accurately describe the workspace retention/checkpoint-gating/ancestor-rebinding behavior exercised by the accompanying tests.

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

387-392: LGTM!

Also applies to: 425-430, 526-529

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

138-163: LGTM!

test/unit/domain/services/controllers/MaterializeController.test.ts (1)

81-113: LGTM!

Also applies to: 245-251

test/unit/domain/services/controllers/MaterializePatchStreamReducer.test.ts (1)

49-49: LGTM!

Also applies to: 174-194

src/domain/orset/session/StateSession.ts (3)

6-73: LGTM!

Also applies to: 84-122, 157-215, 223-464


216-221: 🗄️ Data Integrity & Integration

close() isn’t used on the workspace-backed path here. This flow goes through prepareClose() and accept(witness), so the retention-loss concern doesn’t apply.

			> Likely an incorrect or invalid review comment.

154-176: 🩺 Stability & Availability

No concurrent flush path here. The current call sites await each StateSession mutation sequentially, so overlapping #flushAndRetain() calls aren’t reachable on this path.

			> Likely an incorrect or invalid review comment.
src/domain/orset/shadow/ShadowTrieORSet.ts (1)

79-96: LGTM!

src/domain/orset/trie/TrieCursor.ts (2)

594-594: Correct ancestor-rebind fix.

Adding the explicit #rebindParentBranch(args.parentPath, args.nibble) after the recursive descend fixes the previously-missing propagation of dirty markers up through intermediate branch levels when inserting into an already-existing branch chain — without it, only the immediate leaf-level parent got rebinded, leaving ancestor branches pointing at stale child OIDs after a flush.


70-70: LGTM!

Also applies to: 197-216

src/ports/MaterializationStorePort.ts (1)

3-18: LGTM!

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

23-28: LGTM!

Also applies to: 39-45, 90-107

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

341-350: LGTM!

Scoping pageCache per session-open (rather than sharing one instance across all opens) is a reasonable correctness-oriented choice given PageCache is mutable session state.

src/ports/MaterializationWorkspacePort.ts (1)

1-31: LGTM!

src/infrastructure/adapters/GitCasMaterializationWorkspace.ts (2)

1-102: LGTM!

Also applies to: 185-372


248-268: 🗄️ Data Integrity & Integration

parseRoot() already produces a valid bundle member input. BundleMemberInput accepts ApplicationHandleInput, and ApplicationHandleInput includes BundleHandleInput, which accepts string.

			> Likely an incorrect or invalid review comment.
src/domain/services/controllers/MaterializeController.ts (2)

44-68: LGTM!

Also applies to: 111-122


301-465: LGTM!

src/domain/services/controllers/MaterializeCoordinateStrategy.ts (1)

37-37: LGTM!

Also applies to: 65-66, 145-145

src/domain/services/controllers/MaterializeLiveStrategy.ts (1)

32-64: LGTM!

Also applies to: 131-131, 175-186, 259-259

src/domain/services/controllers/MaterializeSessionBridge.ts (1)

6-9: LGTM!

Also applies to: 30-32, 39-107, 114-126

src/domain/services/controllers/MaterializeStrategyRuntime.ts (1)

10-13: LGTM!

Also applies to: 48-81

Comment thread src/domain/services/controllers/MaterializeController.ts
Comment thread src/domain/services/controllers/MaterializeSessionBridge.ts
Comment thread src/infrastructure/adapters/GitCasMaterializationWorkspace.ts
Comment thread test/unit/domain/orset/session/StateSession.test.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.

@flyingrobots
flyingrobots merged commit ea69ac0 into main Jul 16, 2026
18 checks passed
@flyingrobots
flyingrobots deleted the v19-paged-materialization-roots branch July 16, 2026 21:37
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.

1 participant