feat(cas): M2 Boomerang — restore, CLI, integration tests#2
feat(cas): M2 Boomerang — restore, CLI, integration tests#2flyingrobots merged 10 commits intomainfrom
Conversation
Add CasService.restore() with per-chunk SHA-256 integrity verification and encrypted restore support. Add readTree() to GitPersistencePort for parsing Git trees via ls-tree. Wrap stream errors as STREAM_ERROR with chunksWritten metadata. Add git-cas CLI (bin/git-cas.js) with store, tree, and restore subcommands using commander. All three README CLI promises now work. Add 59 Docker-gated integration tests exercising real Git bare repos (JSON + CBOR codecs, plaintext + encrypted, fuzz around chunk boundaries). Hard gate prevents accidental host execution. Fix readBlob Uint8Array→Buffer normalisation for codec compatibility. Update CHANGELOG, README, and ROADMAP.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds file restoration and integrity verification to the Git-backed CAS, standardizes stream error wrapping, introduces a CLI ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CLI
participant CAS as CasService
participant Persist as GitPersistenceAdapter
participant Codec as Codec(JSON/CBOR)
participant Crypto as CryptoAdapter
Client->>CLI: user runs git-cas restore / store / tree
CLI->>CAS: invoke API (store/restore/createTree)
alt store (streaming)
CAS->>Codec: encode chunks
loop per chunk
CAS->>Persist: writeBlob(chunk)
Persist-->>CAS: oid
end
CAS-->>CLI: manifest / tree OID
else restore
CAS->>Persist: readBlob(chunkOid)
Persist-->>CAS: chunk Buffer
CAS->>Codec: decode chunk
Codec-->>CAS: raw bytes
CAS->>CAS: verify digest per chunk
alt encrypted
CAS->>Crypto: decrypt(allChunks, key)
Crypto-->>CAS: decrypted bytes
end
CAS-->>CLI: { bytesWritten, buffer } (or write to disk)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review please |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@ROADMAP.md`:
- Around line 114-117: The roadmap uses two conflicting CLI signatures for the
tree command: `git cas tree --slug <slug>` and `git cas tree --manifest <path>`;
pick one canonical flag (either --slug or --manifest) and update every
occurrence so the Contracts section and Task 2.5 use the same interface; search
for the command string `git cas tree` and replace all instances to the chosen
form, and ensure the associated bullets (Output, Exit 0/1) describe the argument
name consistently (e.g., if you choose --manifest, change the header to `git cas
tree --manifest <path>` and any mentions of slug to manifest).
In `@src/domain/services/CasService.js`:
- Around line 195-225: The restore path allows returning ciphertext when
manifest.encryption?.encrypted is true but encryptionKey is missing; modify the
logic in CasService (around manifest.encryption handling in the restore/decrypt
flow) to fail fast by checking manifest.encryption?.encrypted and throwing a
clear CasError (or similar) if encryptionKey is not provided; keep the existing
_validateKey call for non-null keys and only call this.decrypt({ buffer, key:
encryptionKey, meta: manifest.encryption }) when the key exists, otherwise throw
an error indicating a missing decryption key.
In `@src/infrastructure/adapters/GitPersistenceAdapter.js`:
- Around line 64-90: The parsing logic in GitPersistenceAdapter.js that reads
the variable output and splits on '\n' with trim() is unsafe for filenames
containing leading/trailing spaces; change the git invocation to use "git
ls-tree -z" and update the parser in the method that currently processes output
to split on '\0' (NUL) instead of '\n', remove the global trim() call, and still
validate each entry by finding the '\t' separator and the meta parts
(mode/type/oid) before returning the object; keep the CasError throws for
malformed entries but ensure they include the original rawEntry for debugging.
In `@test/unit/domain/services/CasService.stream-error.test.js`:
- Around line 155-171: The fuzz test currently only asserts inside the catch
block so it will pass silently if service.store(...) does not throw; update the
test for `fuzz: randomized failure points` so it fails when no error is thrown
by either wrapping the call in an `await
expect(service.store({...})).rejects.toThrow()` pattern or by adding an explicit
fail/assertion immediately after the await call (e.g., call fail('expected
error') or expect.hasAssertions()) — target the invocation of service.store({
source: failingSource(failAfter), slug: `fuzz-${i}`, filename: 'fuzz.bin' }) and
keep the existing checks for instanceof CasError and
err.code/err.meta.chunksWritten in the rejection handler.
🧹 Nitpick comments (1)
ROADMAP.md (1)
109-112: Clarifystore --treestdout format.“Prints manifest JSON to stdout (tree OID if
--treeflag is passed)” is ambiguous about whether both are emitted or only the tree OID. Consider stating the exact format or that both are printed (and how).
- Add MISSING_KEY guard in restore() for encrypted manifests without a key - Switch readTree() to git ls-tree -z for NUL-delimited filename safety - Fix fuzz tests to fail explicitly when store() doesn't throw - Align ROADMAP tree CLI signature (--manifest, not --slug) - Clarify store --tree stdout format in ROADMAP contracts
Runs lint and unit tests on Node 22 natively, then a Docker matrix across Node/Bun/Deno for both unit and integration tests.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/domain/services/CasService.js`:
- Around line 62-69: The catch block in CasService.js currently uses a
single-line if without braces (if (err instanceof CasError) throw err;) which
triggers a lint error; change it to use braces (if (err instanceof CasError) {
throw err; }) within the same catch scope so the existing throw of new
CasError(`Stream error during store: ${err.message}`, 'STREAM_ERROR', {
chunksWritten: manifestData.chunks.length, originalError: err }) remains
unchanged and linting passes.
🧹 Nitpick comments (1)
src/domain/services/CasService.js (1)
194-235: Consider extracting chunk verification to reduce cyclomatic complexity.Static analysis reports complexity of 11 (max 10). The redundant check at Line 226 (
&& encryptionKey) can also be simplified since ifmanifest.encryption?.encryptedis true without a key, Line 199-204 would have already thrown.♻️ Suggested refactor
+ /** + * Reads and verifies all chunks from a manifest. + * `@private` + */ + async _readAndVerifyChunks(manifest) { + const chunks = []; + for (const chunk of manifest.chunks) { + const blob = await this.persistence.readBlob(chunk.blob); + const digest = this._sha256(blob); + if (digest !== chunk.digest) { + throw new CasError( + `Chunk ${chunk.index} integrity check failed`, + 'INTEGRITY_ERROR', + { chunkIndex: chunk.index, expected: chunk.digest, actual: digest }, + ); + } + chunks.push(blob); + } + return chunks; + } + async restore({ manifest, encryptionKey }) { if (encryptionKey) { this._validateKey(encryptionKey); } if (manifest.encryption?.encrypted && !encryptionKey) { throw new CasError( 'Encryption key required to restore encrypted content', 'MISSING_KEY', ); } if (manifest.chunks.length === 0) { return { buffer: Buffer.alloc(0), bytesWritten: 0 }; } - const chunks = []; - for (const chunk of manifest.chunks) { - const blob = await this.persistence.readBlob(chunk.blob); - const digest = this._sha256(blob); - if (digest !== chunk.digest) { - throw new CasError( - `Chunk ${chunk.index} integrity check failed`, - 'INTEGRITY_ERROR', - { chunkIndex: chunk.index, expected: chunk.digest, actual: digest }, - ); - } - chunks.push(blob); - } + const chunks = await this._readAndVerifyChunks(manifest); let buffer = Buffer.concat(chunks); - if (manifest.encryption?.encrypted && encryptionKey) { + if (manifest.encryption?.encrypted) { buffer = this.decrypt({ buffer, key: encryptionKey, meta: manifest.encryption, }); } return { buffer, bytesWritten: buffer.length }; }
Fix linting violations including unused imports, prefer-template, curly brace enforcement, and no-new warnings. Extract _readAndVerifyChunks helper in CasService to reduce function complexity.
The MISSING_KEY error is already thrown earlier when encrypted content has no key, making the && encryptionKey check unreachable and redundant.
Zod 3.25.x re-exports z as a namespace object which Bun cannot resolve as a named import. Switch to default import which works across Node, Bun, and Deno.
cbor-extract and esbuild require the node binary to run their install/postinstall scripts during deno install.
Summary
CasService.restore()(domain) andContentAddressableStore.restoreFile()(facade).ls-treeonGitPersistencePort/GitPersistenceAdapter. ThrowsTREE_PARSE_ERRORon malformed output.store()now surface asCasError('STREAM_ERROR')with{ chunksWritten }metadata. No manifest returned on partial store.bin/git-cas.jswithstore,tree, andrestoresubcommands. Supports--key-filefor encryption,--cwdfor working directory,--treeflag on store. All three README CLI promises now work.readBlob()now normalisesUint8Arrayfrom plumbing intoBufferfor codec/crypto compatibility.await, expanded CLI docs), ROADMAP (added CLI tasks 2.5/2.6).Test plan
npm test— 314 unit tests pass locallynpm run test:integration:node— 59 integration tests pass in Dockernpm run test:node— full unit suite passes in Node Dockernpx git-cas --help— CLI loads and prints usageGIT_STUNTS_DOCKER=1Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests