Skip to content

fix(storage): retain trie roots through git-cas#753

Merged
flyingrobots merged 1 commit into
mainfrom
v19-retained-materialization-roots
Jul 16, 2026
Merged

fix(storage): retain trie roots through git-cas#753
flyingrobots merged 1 commit into
mainfrom
v19-retained-materialization-roots

Conversation

@flyingrobots

Copy link
Copy Markdown
Member

Summary

  • replace the native GitTrieStoreAdapter raw blob/tree implementation with GitCasTrieStoreAdapter
  • store trie leaves as bounded git-cas pages wrapped by bundles and branches as deterministic bundles of child bundle handles
  • inject the repository-scoped git-cas facade into production trie storage
  • prove nested trie roots survive materialization retention, fresh-adapter reopen, and git prune -n --expire=now
  • add AST and shell gates that reject raw Git object writers in production
  • delete the obsolete native-Git plumbing mocks and replace them with focused git-cas contract tests

Root cause

The session-backed materializer emitted raw Git blob/tree OIDs from hash-object and mktree. Those objects bypassed git-cas retention and could not lawfully populate MaterializationRoots, so successful materialization could leave its trie graph unreachable and prunable.

This change makes every successful trie root a git-cas bundle handle. A retained materialization bundle can now establish transitive reachability for branch bundles, leaf bundles, and leaf pages.

Impact

This is an internal storage-composition change. The v19 package root and public subpath declarations are unchanged. Deep internal GitTrieStoreAdapter imports are intentionally removed.

Legacy raw-OID roots are rejected as corrupt roots rather than silently interpreted under the new format. The ordinary warm-resume path is not wired in this PR, so #738 remains open.

Refs #738

Validation

  • npm run lint
  • npm run typecheck
  • npm run typecheck:policy
  • npm run typecheck:consumer
  • npm run typecheck:surface
  • npm run test:local (6,970 passed, 2 skipped)
  • npx vitest run test/integration --maxWorkers=1 (94 passed before the final empty-branch case; that case also passed independently)
  • focused adapter/materialization suite (25 passed)
  • npm run test:coverage:ci (7,075 passed, 2 skipped; 92.78% lines)
  • npm pack --dry-run --ignore-scripts
  • full pre-push IRONCLAD M9 gate, including links and documentation topology
  • Graft structural review and export-surface review

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4ec5caa4-0535-4d11-a37f-c0860b2fd9e8

📥 Commits

Reviewing files that changed from the base of the PR and between 57fe806 and eb3b7c9.

📒 Files selected for processing (18)
  • CHANGELOG.md
  • scripts/check-git-cas-invariants.sh
  • scripts/source-size-gate.ts
  • src/domain/errors/TrieStoreError.ts
  • src/domain/orset/trie/TrieStorePort.ts
  • src/infrastructure/adapters/GitCasRepositoryAdapter.ts
  • src/infrastructure/adapters/GitCasTrieStoreAdapter.ts
  • src/infrastructure/adapters/GitTrieStoreAdapter.ts
  • src/infrastructure/adapters/trieNibbleName.ts
  • test/helpers/InMemoryGitCasFacade.ts
  • test/integration/domain/orset/trie/TrieCursor.flush.integration.test.ts
  • test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts
  • test/integration/infrastructure/adapters/GitCasTrieStoreAdapter.integration.test.ts
  • test/integration/infrastructure/adapters/GitTrieStoreAdapter.integration.test.ts
  • test/unit/infrastructure/adapters/GitCasRepositoryAdapter.test.ts
  • test/unit/infrastructure/adapters/GitCasTrieStoreAdapter.test.ts
  • test/unit/infrastructure/adapters/GitTrieStoreAdapter.test.ts
  • test/unit/scripts/storage-ownership-boundary.test.ts
💤 Files with no reviewable changes (4)
  • test/integration/infrastructure/adapters/GitTrieStoreAdapter.integration.test.ts
  • src/infrastructure/adapters/GitTrieStoreAdapter.ts
  • scripts/source-size-gate.ts
  • test/unit/infrastructure/adapters/GitTrieStoreAdapter.test.ts
📜 Recent 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 (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/infrastructure/adapters/GitCasRepositoryAdapter.test.ts
  • src/domain/errors/TrieStoreError.ts
  • test/helpers/InMemoryGitCasFacade.ts
  • src/infrastructure/adapters/GitCasRepositoryAdapter.ts
  • test/integration/infrastructure/adapters/GitCasTrieStoreAdapter.integration.test.ts
  • test/unit/infrastructure/adapters/GitCasTrieStoreAdapter.test.ts
  • src/domain/orset/trie/TrieStorePort.ts
  • test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts
  • src/infrastructure/adapters/trieNibbleName.ts
  • test/unit/scripts/storage-ownership-boundary.test.ts
  • test/integration/domain/orset/trie/TrieCursor.flush.integration.test.ts
  • src/infrastructure/adapters/GitCasTrieStoreAdapter.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/errors/TrieStoreError.ts
  • src/infrastructure/adapters/GitCasRepositoryAdapter.ts
  • src/domain/orset/trie/TrieStorePort.ts
  • src/infrastructure/adapters/trieNibbleName.ts
  • src/infrastructure/adapters/GitCasTrieStoreAdapter.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/errors/TrieStoreError.ts
  • src/domain/orset/trie/TrieStorePort.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/errors/TrieStoreError.ts
  • src/domain/orset/trie/TrieStorePort.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)


[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 (14)
src/domain/errors/TrieStoreError.ts (1)

14-17: LGTM!

src/domain/orset/trie/TrieStorePort.ts (1)

58-85: LGTM!

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

1-243: LGTM!

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

2-4: LGTM!

Also applies to: 18-18, 38-45

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

1-206: LGTM!

scripts/check-git-cas-invariants.sh (1)

2-5: LGTM!

Also applies to: 19-35

test/unit/scripts/storage-ownership-boundary.test.ts (1)

27-39: LGTM!

Also applies to: 146-170, 246-268

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

28-28: LGTM!

Also applies to: 47-50, 107-107

test/helpers/InMemoryGitCasFacade.ts (1)

49-49: LGTM!

Also applies to: 78-78, 243-253

test/integration/domain/orset/trie/TrieCursor.flush.integration.test.ts (1)

3-27: LGTM!

Also applies to: 42-49, 61-61, 122-122

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

15-15: LGTM!

Also applies to: 40-41, 54-68, 126-169

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

1-85: LGTM!

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

12-12: LGTM!

Also applies to: 165-165

CHANGELOG.md (1)

55-58: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Trie storage now uses bounded content-addressable pages and bundle handles, improving storage safety and retention.
    • Added support for reading and writing trie leaves and branches through the new storage format.
  • Bug Fixes

    • Improved validation and error reporting for malformed trie roots, branch paths, and storage failures.
  • Tests

    • Added comprehensive coverage for trie persistence, round trips, pruning, malformed data, and storage ownership safeguards.
  • Documentation

    • Updated terminology to refer to opaque root handles and generalized backing storage.

Walkthrough

Shadow-trie leaf and branch persistence moved from raw Git blobs and trees to bounded git-cas pages and bundle handles. Runtime wiring, trie contracts, tests, error terminology, and raw Git write ownership checks were updated.

Changes

CAS-backed trie storage

Layer / File(s) Summary
CAS trie adapter and contract
src/infrastructure/adapters/GitCasTrieStoreAdapter.ts, src/domain/orset/trie/TrieStorePort.ts, src/domain/errors/TrieStoreError.ts, src/infrastructure/adapters/trieNibbleName.ts
Leaf data is stored in bounded pages and branches in ordered bundles; handle, member, nibble, and error validation are implemented.
Runtime wiring and integration fixtures
src/infrastructure/adapters/GitCasRepositoryAdapter.ts, test/helpers/InMemoryGitCasFacade.ts, test/integration/...
Runtime and integration harnesses construct the CAS trie adapter and expose bundle member lookup.
CAS trie validation
test/unit/infrastructure/adapters/GitCasTrieStoreAdapter.test.ts, test/integration/infrastructure/adapters/GitCasTrieStoreAdapter.integration.test.ts, test/integration/infrastructure/adapters/GitCasMaterializationStoreAdapter.integration.test.ts, test/integration/domain/orset/trie/TrieCursor.flush.integration.test.ts, test/unit/infrastructure/adapters/GitCasRepositoryAdapter.test.ts
Tests cover round trips, deterministic ordering, malformed roots and members, typed failures, retention, flushing, and service selection.
Raw Git ownership enforcement
scripts/check-git-cas-invariants.sh, test/unit/scripts/storage-ownership-boundary.test.ts, scripts/source-size-gate.ts, CHANGELOG.md
Checks now reject raw Git object writers and document the CAS ownership boundary; the removed adapter test relaxation is deleted.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

Possibly related PRs

Sequence Diagram(s)

sequenceDiagram
  participant TrieCursor
  participant GitCasTrieStoreAdapter
  participant GitCasFacade
  participant ContentAddressableStore
  TrieCursor->>GitCasTrieStoreAdapter: Flush trie leaf or branch
  GitCasTrieStoreAdapter->>ContentAddressableStore: Store bounded leaf page
  GitCasTrieStoreAdapter->>GitCasFacade: Create ordered bundle
  GitCasFacade-->>GitCasTrieStoreAdapter: Return bundle handle
  TrieCursor->>GitCasTrieStoreAdapter: Read trie root
  GitCasTrieStoreAdapter->>GitCasFacade: Resolve bundle members
  GitCasTrieStoreAdapter->>ContentAddressableStore: Read leaf page
  ContentAddressableStore-->>TrieCursor: Return trie data
Loading

Poem

A rabbit hops where Git trees grew,
Now CAS bundles carry data through.
Pages stay bounded, handles gleam,
Branches sort neatly in the stream.
Raw writers knock, but gates say “no!”
The trie now blooms in CAS’s glow.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.70% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately reflects the trie-storage migration to git-cas-backed roots.
Description check ✅ Passed Mostly matches the template with summary, issue reference, test plan, and validation, but the required ADR checklist section is omitted.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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.

@flyingrobots

Copy link
Copy Markdown
Member Author

Code Lawyer activity summary

Item Severity Evidence Outcome
Exact diff self-audit P1 origin/main...eb3b7c992 reviewed for determinism, retention, errors, and ownership Clean; no post-PR code issues found
Graft structural review P2 18 files; internal adapter replacement; zero remaining references to GitTrieStoreAdapter Intentional deep-module change; package declaration surface unchanged
Review-thread audit P1 Thread-aware GraphQL fetch 0 unresolved threads
CodeRabbit P1 Approved; no actionable comments Passed
Local verification P1 IRONCLAD pre-push, 6,970 unit tests, focused integration, 92.78% coverage Passed
CI matrix P1 Node 22, Bun, Deno, coverage, type firewall, docs, links, release preflight Passed
Issue posture P1 PR references #738 without a closing keyword #738 remains open for warm resume and bounded observer work

No force, rebase, amend, skipped threshold, or draft-PR operation was used.

@flyingrobots
flyingrobots merged commit c15a506 into main Jul 16, 2026
17 checks passed
@flyingrobots
flyingrobots deleted the v19-retained-materialization-roots branch July 16, 2026 14:33
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