Skip to content

Preserve coherent Git object sessions#95

Merged
flyingrobots merged 3 commits into
mainfrom
perf/session-coherence
Jul 19, 2026
Merged

Preserve coherent Git object sessions#95
flyingrobots merged 3 commits into
mainfrom
perf/session-coherence

Conversation

@flyingrobots

Copy link
Copy Markdown
Member

Summary

  • preserve cat-file --batch-command across successful loose blob, tree, and scoped bulk writes;
  • preserve mktree --batch -z across loose writes, while retaining the mandatory post-pack retirement boundary;
  • add SHA-1/SHA-256 real-Git regression coverage, explicit lifecycle/error-path tests, and a measured witness.

Closes #94.

Root cause

The adapter treated every successful object-database mutation as if it invalidated every typed Git process. Git's lookup behavior is more precise: cat-file refreshes packed object state on a later lookup, while mktree uses quick lookup and cannot discover a pack created after its object-database snapshot. The previous blanket retirement policy therefore converted persistent protocols back into process-per-object execution.

Behavioral boundary

This is one internal session-coherence correction. Individual writeBlob() calls remain one-shot hash-object; scoped fast-import still checkpoints and closes before OIDs escape; retention, refs, object identity, lifecycle, and the public API are unchanged.

[cite: src/infrastructure/adapters/GitPersistenceAdapter.js#93-175@9208871802a596cbc508be725353532110d40198]

The real-Git test forces two packs above fastimport.unpackLimit, exercises both object formats, and proves one persistent reader plus the required replacement mktree generation.

[cite: test/integration/git-object-session-coherence.test.js#24-60@9208871802a596cbc508be725353532110d40198]

Measured impact

On the audited temporary 128-node git-warp retained-materialization fixture:

Metric 6.5.2 Candidate Delta
Git child processes 558 401 -28.1%
cat-file processes 80 4 -95.0%
mktree processes 82 1 -98.8%
Median materialization time 14.24 s 12.72 s -10.7%
Median process CPU 15.74 s 13.83 s -12.1%

Process topology is the high-confidence result. Timings are local three-run medians, not a cross-platform guarantee. The committed integration count is the durable regression gate.

Verification

  • pnpm release:verify: 14/14 steps, 6,826 observed tests;
  • Node/Bun/Deno integration: 198/198 each;
  • serial Bats runtime cases: 3/3;
  • public Deno type compatibility: pass;
  • npm pack and JSR publish dry-runs: pass;
  • git diff --check origin/main...HEAD: pass;
  • Graft implementation export review: semver impact none.

The host pnpm test:platforms command encountered Homebrew moreutils' incompatible parallel before executing tests. The exact Bats file passed serially across Node, Bun, and Deno; the witness records this harness limitation explicitly.

Full evidence: docs/design/0053-git-object-session-coherence/witness/verification.md.

@coderabbitai

coderabbitai Bot commented Jul 19, 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: 60924873-25ef-4965-93bb-509d913b17ed

📥 Commits

Reviewing files that changed from the base of the PR and between fb3d3c2 and 7adeabe.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • docs/design/0053-git-object-session-coherence/git-object-session-coherence.md
  • docs/design/0053-git-object-session-coherence/witness/measurements.json
  • docs/design/0053-git-object-session-coherence/witness/verification.md
  • docs/design/README.md
  • src/infrastructure/adapters/GitPersistenceAdapter.js
  • test/integration/git-object-session-coherence.test.js
  • test/unit/infrastructure/adapters/GitPersistenceAdapter.sessionCoherence.test.js
  • test/unit/infrastructure/adapters/GitPersistenceAdapter.sessions.test.js
📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: test-docker (node)
  • GitHub Check: test-docker (bun)
  • GitHub Check: test-docker (deno)
🧰 Additional context used
📓 Path-based instructions (2)
docs/design/**

📄 CodeRabbit inference engine (AGENTS.md)

Use docs/design/ directory for durable design contracts and proof plans

Files:

  • docs/design/0053-git-object-session-coherence/witness/measurements.json
  • docs/design/README.md
  • docs/design/0053-git-object-session-coherence/witness/verification.md
  • docs/design/0053-git-object-session-coherence/git-object-session-coherence.md
CHANGELOG.md

📄 CodeRabbit inference engine (AGENTS.md)

Use CHANGELOG.md to record the historical truth of merged behavior

Files:

  • CHANGELOG.md
🧠 Learnings (3)
📚 Learning: 2026-03-30T19:53:48.000Z
Learnt from: flyingrobots
Repo: git-stunts/git-cas PR: 29
File: docs/design/TR-010-planning-index-consistency-review.md:121-131
Timestamp: 2026-03-30T19:53:48.000Z
Learning: In this repo, CHANGELOG.md entries should be user-facing and descriptive (bullet points) rather than cycle-ID headings (e.g., use a phrase like “Planning-index consistency review” instead of a “TR-010 — …” heading). When searching the changelog, don’t rely on grepping for cycle IDs (e.g., “TR-010”) because it may cause false negatives; search for the descriptive keywords/phrases from the bullet entries instead.

Applied to files:

  • CHANGELOG.md
📚 Learning: 2026-02-28T19:21:13.982Z
Learnt from: flyingrobots
Repo: git-stunts/git-cas PR: 15
File: test/unit/domain/services/CasService.envelope.test.js:326-330
Timestamp: 2026-02-28T19:21:13.982Z
Learning: In fuzz tests for cryptographic operations, use a seeded PRNG (e.g., xorshift32) for control-flow variables such as plaintext size selection and recipient index selection to ensure reproducibility. Continue to use randomBytes() for cryptographic keys and nonces to preserve realistic randomness and avoid security anti-patterns. This guideline applies to test files that perform fuzz testing of crypto logic; implement a consistent seed setup (e.g., fixed seed in test initialization) and document the rationale to enable deterministic replays across runs.

Applied to files:

  • test/unit/infrastructure/adapters/GitPersistenceAdapter.sessionCoherence.test.js
  • test/integration/git-object-session-coherence.test.js
  • test/unit/infrastructure/adapters/GitPersistenceAdapter.sessions.test.js
📚 Learning: 2026-07-13T17:00:46.222Z
Learnt from: flyingrobots
Repo: git-stunts/git-cas PR: 65
File: src/domain/helpers/isCanonicalCollectionKey.js:21-35
Timestamp: 2026-07-13T17:00:46.222Z
Learning: In git-stunts/git-cas, the codebase is intended to run on multiple JavaScript runtimes (Node, Bun, and Deno), not just Node. During code review, don’t suggest replacing existing portable helper logic with newer Node-only built-ins (e.g., Node 20+ APIs like `String.prototype.isWellFormed()`) unless the change preserves Bun/Deno compatibility (via portable/polyfill implementations or safe runtime gating). If a file includes manual implementations (such as `src/domain/helpers/isCanonicalCollectionKey.js`), treat them as intentional to avoid requiring a higher host built-in baseline.

Applied to files:

  • src/infrastructure/adapters/GitPersistenceAdapter.js
🪛 ast-grep (0.44.1)
test/integration/git-object-session-coherence.test.js

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

(detect-child-process)

🔇 Additional comments (9)
docs/design/0053-git-object-session-coherence/git-object-session-coherence.md (1)

1-234: LGTM!

docs/design/0053-git-object-session-coherence/witness/measurements.json (1)

1-95: LGTM!

docs/design/0053-git-object-session-coherence/witness/verification.md (1)

1-104: LGTM!

docs/design/README.md (1)

14-14: LGTM!

CHANGELOG.md (1)

10-16: LGTM!

src/infrastructure/adapters/GitPersistenceAdapter.js (1)

93-103: LGTM!

Also applies to: 113-152, 160-176, 543-556

test/unit/infrastructure/adapters/GitPersistenceAdapter.sessionCoherence.test.js (1)

1-76: LGTM!

test/integration/git-object-session-coherence.test.js (1)

1-99: LGTM!

test/unit/infrastructure/adapters/GitPersistenceAdapter.sessions.test.js (1)

359-424: LGTM!

Also applies to: 466-488, 506-507, 541-541


📝 Walkthrough

Summary by CodeRabbit

  • Performance

    • Improved Git object write performance by reusing active sessions across compatible operations.
    • Bulk writes retain read sessions while safely refreshing tree-writing sessions when needed.
  • Bug Fixes

    • Improved session lifecycle handling after loose object, tree, and bulk writes.
    • Added coverage across SHA-1 and SHA-256 repositories, including packed-object scenarios.
  • Documentation

    • Added design and verification documentation describing session coherence behavior and measured results.

Walkthrough

Git object session retirement is now selective: cat-file and safe mktree sessions remain active after compatible writes, while bulk writes still retire mktree. Design documentation, measurement witnesses, unit tests, and real-Git integration tests record the behavior.

Changes

Git object session coherence

Layer / File(s) Summary
Coherence contract and verification evidence
docs/design/0053-git-object-session-coherence/..., CHANGELOG.md, docs/design/README.md
Documents the preservation and retirement matrix, implementation scope, verification results, measurements, and release boundaries.
Selective session retirement
src/infrastructure/adapters/GitPersistenceAdapter.js
Removes blanket retirement, preserves cat-file across successful writes, preserves mktree for loose writes, and retires mktree after bulk writes.
Session lifecycle and real-Git validation
test/unit/infrastructure/adapters/..., test/integration/git-object-session-coherence.test.js
Verifies session closure behavior, retirement barriers, error handling, pack creation, and SHA-1/SHA-256 repositories.

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

Sequence Diagram(s)

sequenceDiagram
  participant Adapter as GitPersistenceAdapter
  participant CatFile as cat-file session
  participant Mktree as mktree session
  participant FastImport as fast-import session
  Adapter->>CatFile: write object and preserve reader
  Adapter->>Mktree: perform loose write and preserve writer
  Adapter->>FastImport: execute scoped bulk write
  FastImport-->>Adapter: checkpoint and return object IDs
  Adapter->>Mktree: retire after possible pack creation
Loading

Possibly related PRs

Suggested reviewers: git-stunts

Poem

I’m a rabbit with sessions kept warm,
While Git writes objects through every storm.
Cat-file stays, mktree knows when to flee,
Bulk packs retire it responsibly.
Tests hop through SHA formats bright—
Coherent processes, snug for the night.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 summarizes the main change to preserve coherent Git object sessions.
Description check ✅ Passed The description includes the change summary, rationale, proof, validation, and release impact details, so it is mostly complete.
Linked Issues check ✅ Passed The PR implements the session-retirement matrix, adds SHA-1/SHA-256 tests, and includes the witness/docs required by #94.
Out of Scope Changes check ✅ Passed The added docs, tests, and changelog entries all support the session-coherence fix and stay within the linked issue scope.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: lockfile failed supply-chain policy check. Run pnpm install locally to update the lockfile.


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

@flyingrobots

Copy link
Copy Markdown
Member Author

Self-code review

Severity Count Disposition
Critical 0 None
High 0 None
Medium 0 None
Low 0 None
Markdown/style 1 Fixed in 7adeabe9 before PR creation

Reviewed the clean branch against freshly fetched origin/main, including every changed hunk, lifecycle/error aggregation, concurrent retirement barriers, SHA-1/SHA-256 packed-object behavior, package contents, public declarations, and witness claims.

The only finding was two Markdown hard-break trailing spaces in the witness header. git diff --check origin/main...HEAD caught them; they were fixed in a new commit without amending history. The restarted review has no unresolved findings.

Graft reports no exported API changes at implementation commit 92088718. Its whole-branch export scan treats top-level fields in the documentation-only witness JSON as exports; npm pack and public type gates confirm that file is outside the package/API surface.

Verification: pnpm release:verify passed 14/14 steps with 6,826 observed tests, the serial Node/Bun/Deno Bats matrix passed 3/3, and the worktree is clean.

@codex please confirm the session-coherence boundary and the documented Graft JSON false positive.

@flyingrobots
flyingrobots merged commit 7bdcbf1 into main Jul 19, 2026
6 checks passed
@flyingrobots
flyingrobots deleted the perf/session-coherence branch July 19, 2026 16: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.

perf: preserve Git object sessions across coherent writes

1 participant