feat: audit receipt chain (M3.T1.SHADOW-LEDGER, v10.9.0)#26
feat: audit receipt chain (M3.T1.SHADOW-LEDGER, v10.9.0)#26flyingrobots merged 4 commits intomainfrom
Conversation
Complete specification for persistent, chained, tamper-evident audit receipts stored as Git commits. 47 byte-level vector tests covering canonical JSON, opsDigest, CBOR encoding, trailers, string escaping, OID consistency, negative fixtures, and chain break detection.
…HADOW-LEDGER, v10.9.0)
📝 WalkthroughWalkthroughAdds an optional per-commit audit receipts feature: canonical receipt construction, opsDigest computation, CBOR-encoded receipts persisted as Git objects, CAS-protected audit refs per (graphName, writerId), compare-and-swap ref APIs in adapters/ports, WarpGraph audit plumbing, and extensive spec, docs, and tests. Changes
Sequence Diagram(s)sequenceDiagram
actor App
participant WG as WarpGraph
participant ARS as AuditReceiptService
participant Codec as AuditMessageCodec
participant Persist as GitGraphAdapter
participant Refs as RefPort
App->>WG: submit patch (audit enabled)
WG->>WG: joinPatch(tickReceipt=true)
WG->>ARS: commit(tickReceipt)
activate ARS
ARS->>ARS: computeOpsDigest(ops)
ARS->>ARS: buildReceiptRecord & encode CBOR
ARS->>Persist: writeBlob(cbor)
Persist-->>ARS: blobOid
ARS->>Persist: writeTree(entries)
Persist-->>ARS: treeOid
ARS->>Persist: commitNode(tree, message, parents)
Persist-->>ARS: commitOid
ARS->>Refs: compareAndSwapRef(auditRef, commitOid, prevOid)
alt CAS Success
Refs-->>ARS: ok
ARS->>ARS: updateStats(committed++)
else CAS Conflict
Refs-->>ARS: error
ARS->>Refs: readRef(auditRef)
ARS->>ARS: _retryAfterCasConflict()
ARS->>ARS: possibly degrade/skip
end
ARS-->>WG: commitOid or null
deactivate ARS
WG-->>App: commit acknowledged
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
Release Preflight
If you tag this commit as |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/domain/WarpGraph.js (1)
335-347:⚠️ Potential issue | 🟡 MinorValidate the new
auditoption type.
Other options are type-checked; add a boolean guard forauditto avoid accidental truthy values enabling audit.✅ Suggested check
// Validate autoMaterialize if (autoMaterialize !== undefined && typeof autoMaterialize !== 'boolean') { throw new Error('autoMaterialize must be a boolean'); } + + // Validate audit + if (audit !== undefined && typeof audit !== 'boolean') { + throw new Error('audit must be a boolean'); + }
🤖 Fix all issues with AI agents
In `@docs/specs/AUDIT_RECEIPT.md`:
- Around line 79-88: Several fenced code blocks (e.g., the snippet showing
refs/warp/<graphName>/audit/<writerId> and similar examples) are missing
language identifiers; update each triple-backtick fence in AUDIT_RECEIPT.md to
include an appropriate language tag (use `text` for plain paths/refs and `json`
for JSON examples) so markdownlint MD040 is satisfied, and apply this
consistently across all code fences like the
refs/warp/<graphName>/audit/<writerId> example and any request/response JSON
blocks.
In `@src/domain/services/AuditReceiptService.js`:
- Around line 318-345: _commitInner is using tickReceipt.writer for the receipt
and audit message but the instance writer identity is stored in this._writerId;
add a guard at the top of _commitInner to verify tickReceipt.writer ===
this._writerId and throw or reject if they differ to prevent cross-writer
attribution. Locate the _commitInner function and check the tickReceipt
destructuring (tickReceipt.writer) and the subsequent buildReceiptRecord and
audit message construction; validate the writer, and if mismatched, log a clear
error and abort the commit (or convert to a rejected Promise) before computing
opsDigest or building the receipt so that receipts and audit refs always use the
instance writer identity.
In `@src/infrastructure/adapters/GitGraphAdapter.js`:
- Around line 530-541: The compareAndSwapRef implementation currently calls
_executeWithRetry which causes CAS mismatch errors to be retried; modify
compareAndSwapRef in GitGraphAdapter so CAS failures are not retried: either
call this.plumbing.execute(...) directly instead of this._executeWithRetry(...)
for the ['update-ref', ref, newOid, oldArg] operation, or call
this._executeWithRetry with a custom shouldRetry that inspects the command
stderr and returns false when it contains the CAS-mismatch text (e.g., phrases
like "cannot lock ref" combined with "but expected" or "is at <oid> but expected
<oid>") to prevent retrying real CAS conflicts. Ensure you still validate
ref/oid as before and preserve the existing args and behavior for non-CAS
errors.
🧹 Nitpick comments (9)
src/domain/utils/RefLayout.js (1)
294-313: LGTM — clean addition following the established builder pattern.The implementation correctly mirrors
buildWriterRef(validate both params, return interpolated path).Minor: the module-level JSDoc (Lines 7–12) enumerates all ref layouts but doesn't include the new
refs/warp/<graph>/audit/<writer_id>path. Consider adding it for discoverability.src/domain/services/AuditReceiptService.js (3)
70-78: Minor:TextEncoderis instantiated twice per call.Two
new TextEncoder()allocations percomputeOpsDigestcall. Since this is called once per data commit (not a hot loop), the overhead is negligible, but if you want to tighten it:♻️ Optional: hoist TextEncoder
+const encoder = new TextEncoder(); + export async function computeOpsDigest(ops, crypto) { const json = canonicalOpsJson(ops); - const prefix = new TextEncoder().encode(OPS_DIGEST_PREFIX); - const payload = new TextEncoder().encode(json); + const prefix = encoder.encode(OPS_DIGEST_PREFIX); + const payload = encoder.encode(json); const combined = new Uint8Array(prefix.length + payload.length);
326-328:_tickCounteris written but never read.
this._tickCounter = lamporton line 328 is a dead write — no code in this class reads_tickCounter. If this is a placeholder for future use, a brief comment would clarify intent; otherwise it can be removed.
249-263: Silent swallow ofinit()errors may hide persistent infra issues.If
readRefthrows due to a transient error (e.g., Git lock contention), the service silently starts from genesis, which could create a forked chain that diverges from the real tip. Consider logging the error before falling back, so operators have visibility into unexpected cold-start behavior.♻️ Suggested improvement
} catch { - // If we can't read the ref, start fresh + // If we can't read the ref, start fresh — log so operators see unexpected cold starts + this._logger?.warn('[warp:audit]', { + code: 'AUDIT_INIT_READ_FAILED', + writerId: this._writerId, + ref: this._auditRef, + }); this._prevAuditCommit = null; this._expectedOldRef = null; }test/unit/domain/services/AuditReceiptService.bench.js (2)
40-47: Benchmark won't detect O(n²) with a fixed 2-op input.The file's purpose is to "sanity-check that core operations are not O(n²)", but both benchmarks use constant-size inputs. To catch quadratic scaling you'd need parametric runs (e.g., 10, 100, 1000 ops) and compare throughput. As a smoke-test for baseline latency these are fine, though.
14-21:testCryptostub is duplicated across test files.This exact stub appears in
AuditReceiptService.coverage.test.js,AuditReceiptService.test.js, and here. Consider extracting it into a shared test helper (e.g.,test/helpers/testCrypto.js) to reduce duplication.docs/GUIDE.md (1)
1655-1717: LGTM — Appendix I provides a clear, concise overview of the audit feature.Good coverage of enablement, recorded fields, Git structure, chain integrity, and resilience semantics. The link to the full spec is helpful.
One minor nit on Line 1700: the trailer names are listed as
data-commit,graph, etc., while the actual Git trailer keys include theeg-prefix (eg-data-commit,eg-graph, …). Consider clarifying this is shorthand or using the full keys to help readers correlate with raw commit messages.src/domain/services/WarpMessageCodec.js (1)
5-16: Module JSDoc still says "three types" — now there are four.The facade JSDoc mentions "the three types of WARP commit messages" (Patch, Checkpoint, Anchor) and lists three sub-modules. With the addition of Audit, this should be updated to four types and include
AuditMessageCodecin the module list.📝 Suggested doc update
- * This module provides backward-compatible access to the three types of + * This module provides backward-compatible access to the four types of * WARP (Write-Ahead Reference Protocol) commit messages: * - Patch: Contains graph mutations from a single writer * - Checkpoint: Contains a snapshot of materialized graph state * - Anchor: Marks a merge point in the WARP DAG + * - Audit: Records tamper-evident audit receipts for data commits * * Implementation is split across focused sub-modules: * - {`@link` module:domain/services/PatchMessageCodec} * - {`@link` module:domain/services/CheckpointMessageCodec} * - {`@link` module:domain/services/AnchorMessageCodec} + * - {`@link` module:domain/services/AuditMessageCodec} * - {`@link` module:domain/services/MessageSchemaDetector}src/domain/services/AuditMessageCodec.js (1)
66-118: Harden decode validation to match encoder/spec.
Currently decode checks presence only; validate graph/writer IDs, OIDs, and digest format to avoid accepting malformed audit messages.🔧 Suggested update
export function decodeAuditMessage(message) { const codec = getCodec(); const decoded = codec.decode(message); + if (decoded.title !== MESSAGE_TITLES.audit) { + throw new Error( + `Invalid audit message: title must be '${MESSAGE_TITLES.audit}', got '${decoded.title}'` + ); + } const { trailers } = decoded; // Check for duplicate trailers (strict decode) const keys = Object.keys(trailers); @@ const graph = trailers[TRAILER_KEYS.graph]; if (!graph) { throw new Error('Invalid audit message: missing required trailer eg-graph'); } + validateGraphName(graph); const writer = trailers[TRAILER_KEYS.writer]; if (!writer) { throw new Error('Invalid audit message: missing required trailer eg-writer'); } + validateWriterId(writer); const dataCommit = trailers[TRAILER_KEYS.dataCommit]; if (!dataCommit) { throw new Error('Invalid audit message: missing required trailer eg-data-commit'); } + validateOid(dataCommit, 'dataCommit'); const opsDigest = trailers[TRAILER_KEYS.opsDigest]; if (!opsDigest) { throw new Error('Invalid audit message: missing required trailer eg-ops-digest'); } + validateSha256(opsDigest, 'opsDigest'); const schemaStr = trailers[TRAILER_KEYS.schema]; if (!schemaStr) { throw new Error('Invalid audit message: missing required trailer eg-schema'); } - const schema = parseInt(schemaStr, 10); + if (!/^\d+$/.test(schemaStr)) { + throw new Error( + `Invalid audit message: eg-schema must be a positive integer, got '${schemaStr}'` + ); + } + const schema = Number(schemaStr); if (!Number.isInteger(schema) || schema < 1) { throw new Error(`Invalid audit message: eg-schema must be a positive integer, got '${schemaStr}'`); }
| ``` | ||
| refs/warp/<graphName>/audit/<writerId> | ||
| ``` | ||
|
|
||
| Points to the latest audit commit for the given writer. Updated via compare-and-swap (CAS), mirroring the pattern used by `refs/warp/<graphName>/writers/<writerId>`. | ||
|
|
||
| Example: | ||
| ``` | ||
| refs/warp/events/audit/alice -> a1b2c3d4... | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
markdownlint flags multiple fences without a language. Please annotate them (e.g., text, json) consistently across the doc.
🧾 Example fix (apply similarly to other fences)
-```
-refs/warp/<graphName>/audit/<writerId>
-```
+```text
+refs/warp/<graphName>/audit/<writerId>
+```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| refs/warp/<graphName>/audit/<writerId> | |
| ``` | |
| Points to the latest audit commit for the given writer. Updated via compare-and-swap (CAS), mirroring the pattern used by `refs/warp/<graphName>/writers/<writerId>`. | |
| Example: | |
| ``` | |
| refs/warp/events/audit/alice -> a1b2c3d4... | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 79-79: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 86-86: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/specs/AUDIT_RECEIPT.md` around lines 79 - 88, Several fenced code blocks
(e.g., the snippet showing refs/warp/<graphName>/audit/<writerId> and similar
examples) are missing language identifiers; update each triple-backtick fence in
AUDIT_RECEIPT.md to include an appropriate language tag (use `text` for plain
paths/refs and `json` for JSON examples) so markdownlint MD040 is satisfied, and
apply this consistently across all code fences like the
refs/warp/<graphName>/audit/<writerId> example and any request/response JSON
blocks.
…hardening, JSDoc - Validate `audit` option type (boolean guard, matching autoMaterialize pattern) - Guard cross-writer attribution in AuditReceiptService._commitInner - Don't retry CAS failures in GitGraphAdapter.compareAndSwapRef - Harden decodeAuditMessage with OID/SHA-256/schema format validation - Log AUDIT_INIT_READ_FAILED on init() cold-start errors - Remove dead _tickCounter write - Hoist TextEncoder to module-level constant - Update WarpMessageCodec JSDoc to four message types - Add audit ref to RefLayout JSDoc - Fix GUIDE.md trailer names to include eg- prefix
|
@coderabbitai review please |
|
This is a substantial feature addition implementing tamper-evident audit receipt chains. I can see: Core additions:
Integration:
Test coverage:
Documentation:
I'll perform a thorough review now. ✅ Actions performedReview triggered.
|
Release Preflight
If you tag this commit as |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/domain/services/AuditMessageCodec.js`:
- Around line 71-79: The duplicate-trailer check in AuditMessageCodec.js is
ineffective because Object.keys(trailers) can never show duplicates after
codec.decode() has already merged them; move the duplicate-detection logic
earlier into the raw parsing layer (e.g., inside TrailerCodec.decode() or the
function that parses raw trailer text) so you detect duplicate trailer lines
before they are reduced to an object. Locate where TrailerCodec.decode parses
the raw trailer lines and add a Set-based check as you iterate over each parsed
trailer line (use the same error message format: "Duplicate trailer rejected:
<key>") instead of relying on the post-decode trailers object in
AuditMessageCodec.decode().
In `@test/unit/domain/WarpGraph.audit.test.js`:
- Around line 163-198: The test currently silently returns when auditSha is
falsy, masking failures; instead assert that the audit ref exists before
proceeding (e.g., replace the early return with an explicit expectation such as
expect(auditSha).toBeTruthy() or throw a clear Error) so the test fails if the
audit commit wasn't created; locate the variable auditSha in the 'audit commit
tree contains receipt.cbor with correct receipt data' test and add the assertion
immediately after reading the ref (before using persistence._commits and
readTree).
🧹 Nitpick comments (1)
src/domain/services/AuditReceiptService.js (1)
411-425: Dangling Git objects created on CAS conflict retry path.When CAS fails at line 413, the commit created at line 405 becomes unreferenced. The retry in
_retryAfterCasConflictcalls_commitInneragain which creates another commit. If that also fails, two orphaned commits exist. This is inherent to the create-then-CAS pattern and is benign (Git GC handles unreachable objects), but worth noting in case the repo doesn't run GC regularly.
| // Check for duplicate trailers (strict decode) | ||
| const keys = Object.keys(trailers); | ||
| const seen = new Set(); | ||
| for (const key of keys) { | ||
| if (seen.has(key)) { | ||
| throw new Error(`Duplicate trailer rejected: ${key}`); | ||
| } | ||
| seen.add(key); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate and examine the AuditMessageCodec.js file
find . -name "AuditMessageCodec.js" -type f 2>/dev/nullRepository: git-stunts/git-warp
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Get more context around lines 71-79
cat -n src/domain/services/AuditMessageCodec.js | sed -n '60,85p'Repository: git-stunts/git-warp
Length of output: 1119
🏁 Script executed:
#!/bin/bash
# Look for TrailerCodec implementation
find . -name "*TrailerCodec*" -type f 2>/dev/null | grep -v node_modulesRepository: git-stunts/git-warp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Search for how trailers are decoded and any duplicate handling
rg -n "trailers" src/domain/services/AuditMessageCodec.js -B2 -A2Repository: git-stunts/git-warp
Length of output: 1868
🏁 Script executed:
#!/bin/bash
# Find where getCodec is defined and what codec library is being used
rg -n "getCodec" src/domain/services/AuditMessageCodec.js -B2 -A2Repository: git-stunts/git-warp
Length of output: 524
🏁 Script executed:
#!/bin/bash
# Check imports at the top of the file
head -20 src/domain/services/AuditMessageCodec.jsRepository: git-stunts/git-warp
Length of output: 680
🏁 Script executed:
#!/bin/bash
# Search for the getCodec function definition
rg -n "function getCodec\|const getCodec\|export.*getCodec" --type=js -g '!node_modules/**'Repository: git-stunts/git-warp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Look for any codec-related imports or dependencies
rg -n "import.*codec\|require.*codec" --type=js -g '!node_modules/**' -g '!*test*' | head -30Repository: git-stunts/git-warp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Examine the MessageCodecInternal.js file to see what codec is used
cat -n src/domain/services/MessageCodecInternal.js | head -100Repository: git-stunts/git-warp
Length of output: 3764
🏁 Script executed:
#!/bin/bash
# Look for getCodec function in MessageCodecInternal.js
rg -n "getCodec" src/domain/services/MessageCodecInternal.js -A10Repository: git-stunts/git-warp
Length of output: 440
🏁 Script executed:
#!/bin/bash
# Search for any imports of a trailer codec or message codec library
rg -n "import\|require" src/domain/services/MessageCodecInternal.js | head -20Repository: git-stunts/git-warp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Search for tests related to AuditMessageCodec
find . -name "*test*" -o -name "*.test.js" -o -name "*.spec.js" | xargs grep -l "AuditMessageCodec\|decodeAuditMessage" 2>/dev/null | head -5Repository: git-stunts/git-warp
Length of output: 116
🏁 Script executed:
#!/bin/bash
# Look for any test files related to message codecs
find . -name "*test*" -type f -path "*/domain/services/*" 2>/dev/null | head -10Repository: git-stunts/git-warp
Length of output: 649
🏁 Script executed:
#!/bin/bash
# Search for duplicate trailer related tests or handling
rg -n "duplicate" --type=js -g '!node_modules/**' -C3Repository: git-stunts/git-warp
Length of output: 14667
🏁 Script executed:
#!/bin/bash
# Check if package.json or lock file shows the trailer-codec version
find . -name "package.json" -o -name "package-lock.json" | xargs grep -A2 "trailer-codec" 2>/dev/null | head -20Repository: git-stunts/git-warp
Length of output: 1010
🏁 Script executed:
#!/bin/bash
# Examine the actual test for AuditMessageCodec to see duplicate handling
cat -n test/unit/domain/services/AuditMessageCodec.test.jsRepository: git-stunts/git-warp
Length of output: 4148
🏁 Script executed:
#!/bin/bash
# Look at the audit-receipt-vectors test more closely around the duplicate test
sed -n '650,670p' test/unit/specs/audit-receipt-vectors.test.jsRepository: git-stunts/git-warp
Length of output: 677
🏁 Script executed:
#!/bin/bash
# Check the checkDuplicateTrailers function implementation
sed -n '180,210p' test/unit/specs/audit-receipt-vectors.test.jsRepository: git-stunts/git-warp
Length of output: 1060
🏁 Script executed:
#!/bin/bash
# Final check: search for any tests that call decodeAuditMessage with duplicate trailers in raw message
rg -n "decodeAuditMessage" test/ -A10 | grep -A10 "duplicate\|eg-kind.*eg-kind" | head -30Repository: git-stunts/git-warp
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if there are any other places in the codebase that do duplicate detection on raw message text
rg -n "split.*\n\|lines.*split" src/ --type=js -A5 | grep -B5 -A5 "duplicate"Repository: git-stunts/git-warp
Length of output: 254
Duplicate trailer check on Object.keys() is ineffective.
Object.keys(trailers) on a plain JS object cannot yield duplicate keys—the JS engine collapses them during parsing. If the raw message contains duplicate trailer lines, codec.decode() will have already merged them (last-wins) before this code executes. This loop will never throw.
For duplicate detection to protect against tampered messages, it must happen during raw message parsing, not after codec.decode() has already converted the message to an object. Consider moving this check into the TrailerCodec.decode() layer or parsing the raw message text directly.
🤖 Prompt for AI Agents
In `@src/domain/services/AuditMessageCodec.js` around lines 71 - 79, The
duplicate-trailer check in AuditMessageCodec.js is ineffective because
Object.keys(trailers) can never show duplicates after codec.decode() has already
merged them; move the duplicate-detection logic earlier into the raw parsing
layer (e.g., inside TrailerCodec.decode() or the function that parses raw
trailer text) so you detect duplicate trailer lines before they are reduced to
an object. Locate where TrailerCodec.decode parses the raw trailer lines and add
a Set-based check as you iterate over each parsed trailer line (use the same
error message format: "Duplicate trailer rejected: <key>") instead of relying on
the post-decode trailers object in AuditMessageCodec.decode().
| it('audit commit tree contains receipt.cbor with correct receipt data', async () => { | ||
| const persistence = new InMemoryGraphAdapter(); | ||
| const graph = await WarpGraph.open({ | ||
| persistence, | ||
| graphName: 'events', | ||
| writerId: 'alice', | ||
| audit: true, | ||
| }); | ||
|
|
||
| // Materialize first so eager path is available | ||
| await graph.materialize(); | ||
|
|
||
| const patch = await graph.createPatch(); | ||
| patch.addNode('user:eve'); | ||
| await patch.commit(); | ||
|
|
||
| const auditSha = await persistence.readRef('refs/warp/events/audit/alice'); | ||
| if (!auditSha) { | ||
| // Skip if eager path wasn't available | ||
| return; | ||
| } | ||
|
|
||
| const commit = persistence._commits.get(auditSha); | ||
| expect(commit).toBeTruthy(); | ||
| const tree = await persistence.readTree(/** @type {{ treeOid: string }} */ (commit).treeOid); | ||
| expect(tree).toHaveProperty('receipt.cbor'); | ||
|
|
||
| // Decode and verify | ||
| const { decode } = await import('../../../src/infrastructure/codecs/CborCodec.js'); | ||
| const receipt = /** @type {Record<string, unknown>} */ (decode(tree['receipt.cbor'])); | ||
| expect(receipt.version).toBe(1); | ||
| expect(receipt.graphName).toBe('events'); | ||
| expect(receipt.writerId).toBe('alice'); | ||
| expect(typeof receipt.timestamp).toBe('number'); | ||
| expect(Number.isInteger(receipt.timestamp)).toBe(true); | ||
| }); |
There was a problem hiding this comment.
Silent early return masks test failures if audit ref is missing.
Lines 180-183: if auditSha is null, the test returns without any assertion and passes silently. If the audit feature regresses, this test won't catch it. Since this test calls materialize() first (line 173), the audit commit should reliably be created.
🔧 Suggested fix: fail explicitly instead of skipping
const auditSha = await persistence.readRef('refs/warp/events/audit/alice');
- if (!auditSha) {
- // Skip if eager path wasn't available
- return;
- }
+ expect(auditSha).toBeTruthy();🤖 Prompt for AI Agents
In `@test/unit/domain/WarpGraph.audit.test.js` around lines 163 - 198, The test
currently silently returns when auditSha is falsy, masking failures; instead
assert that the audit ref exists before proceeding (e.g., replace the early
return with an explicit expectation such as expect(auditSha).toBeTruthy() or
throw a clear Error) so the test fails if the audit commit wasn't created;
locate the variable auditSha in the 'audit commit tree contains receipt.cbor
with correct receipt data' test and add the assertion immediately after reading
the ref (before using persistence._commits and readTree).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/domain/services/AuditReceiptService.js`:
- Around line 453-469: The catch in _retryAfterCasConflict currently treats any
error from this._commitInner(tickReceipt) as a CAS conflict and sets
this._degraded; instead narrow the catch to only handle the CAS-specific failure
thrown by _commitInner (the one that throws 'CAS failed during retry' when
this._retrying is true). Change the catch to inspect the error (e.g.,
error.message or a specific error class) and only set this._degraded, log
AUDIT_DEGRADED_ACTIVE, and throw the degraded Error when the error indicates the
CAS retry failure; for all other errors rethrow them so they bubble up to
commit() and are handled by the existing failure counting logic. Ensure
references to _retryAfterCasConflict, _commitInner, and the 'CAS failed during
retry' sentinel are used to locate the change.
🧹 Nitpick comments (1)
test/unit/domain/services/AuditReceiptService.test.js (1)
379-425: CAS degradation test relies on prototype chain delegation — consider making the delegation explicit.
Object.create(persistence)silently inheritscommitNodeWithTreefrom the prototype, which makes the test work but isn't obvious to readers. The CAS retry test at Line 334 explicitly binds forwarded methods, while this test (Line 384) only binds a subset. IfInMemoryGraphAdapterever changes method dispatch (e.g., uses private fields), this pattern silently breaks.Consider binding
commitNodeWithTreeexplicitly like the other forwarded methods for consistency and resilience:Suggested addition
failingPersistence.readRef = persistence.readRef.bind(persistence); + failingPersistence.commitNodeWithTree = persistence.commitNodeWithTree.bind(persistence);
| // Rebuild and retry (with guard against recursive retry) | ||
| this._retrying = true; | ||
| try { | ||
| const result = await this._commitInner(tickReceipt); | ||
| return result; | ||
| } catch { | ||
| // Second failure → degraded mode | ||
| this._degraded = true; | ||
| this._logger?.warn('[warp:audit]', { | ||
| code: 'AUDIT_DEGRADED_ACTIVE', | ||
| writerId: this._writerId, | ||
| reason: 'second CAS failure', | ||
| }); | ||
| throw new Error('Audit service degraded after second CAS failure'); | ||
| } finally { | ||
| this._retrying = false; | ||
| } |
There was a problem hiding this comment.
Overly broad catch in _retryAfterCasConflict degrades the service on any failure, not just CAS conflicts.
If _commitInner fails during retry for a transient reason (e.g., writeBlob disk error), the catch at Line 458 permanently degrades the service. Only a second CAS failure should trigger degradation — the _retrying flag inside _commitInner (Line 419-422) already throws a specific 'CAS failed during retry' error for that case, but the catch here doesn't discriminate.
Consider narrowing the catch to only degrade on the CAS-specific error:
Suggested fix
try {
const result = await this._commitInner(tickReceipt);
return result;
- } catch {
+ } catch (err) {
+ // Only degrade on second CAS failure; re-throw other errors
+ if (!(err instanceof Error) || !err.message.includes('CAS failed during retry')) {
+ throw err;
+ }
// Second failure → degraded mode
this._degraded = true;A non-CAS failure during retry would then bubble up normally to commit()'s outer catch, incrementing _failed without permanently degrading the service.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Rebuild and retry (with guard against recursive retry) | |
| this._retrying = true; | |
| try { | |
| const result = await this._commitInner(tickReceipt); | |
| return result; | |
| } catch { | |
| // Second failure → degraded mode | |
| this._degraded = true; | |
| this._logger?.warn('[warp:audit]', { | |
| code: 'AUDIT_DEGRADED_ACTIVE', | |
| writerId: this._writerId, | |
| reason: 'second CAS failure', | |
| }); | |
| throw new Error('Audit service degraded after second CAS failure'); | |
| } finally { | |
| this._retrying = false; | |
| } | |
| // Rebuild and retry (with guard against recursive retry) | |
| this._retrying = true; | |
| try { | |
| const result = await this._commitInner(tickReceipt); | |
| return result; | |
| } catch (err) { | |
| // Only degrade on second CAS failure; re-throw other errors | |
| if (!(err instanceof Error) || !err.message.includes('CAS failed during retry')) { | |
| throw err; | |
| } | |
| // Second failure → degraded mode | |
| this._degraded = true; | |
| this._logger?.warn('[warp:audit]', { | |
| code: 'AUDIT_DEGRADED_ACTIVE', | |
| writerId: this._writerId, | |
| reason: 'second CAS failure', | |
| }); | |
| throw new Error('Audit service degraded after second CAS failure'); | |
| } finally { | |
| this._retrying = false; | |
| } |
🤖 Prompt for AI Agents
In `@src/domain/services/AuditReceiptService.js` around lines 453 - 469, The catch
in _retryAfterCasConflict currently treats any error from
this._commitInner(tickReceipt) as a CAS conflict and sets this._degraded;
instead narrow the catch to only handle the CAS-specific failure thrown by
_commitInner (the one that throws 'CAS failed during retry' when this._retrying
is true). Change the catch to inspect the error (e.g., error.message or a
specific error class) and only set this._degraded, log AUDIT_DEGRADED_ACTIVE,
and throw the degraded Error when the error indicates the CAS retry failure; for
all other errors rethrow them so they bubble up to commit() and are handled by
the existing failure counting logic. Ensure references to
_retryAfterCasConflict, _commitInner, and the 'CAS failed during retry' sentinel
are used to locate the change.
1 participant
Summary
AuditReceiptService— persistent, chained, tamper-evident audit receipts. Whenaudit: trueis passed toWarpGraph.open(), each data commit produces a corresponding audit commit recording per-operation outcomes (applied/superseded/redundant). Receipts are stored as CBOR blobs in Git commits underrefs/warp/<graph>/audit/<writer>, linked via content-addressed parents for tamper-evidence.AuditMessageCodec— encode/decode audit commit messages with 6 trailers in lex order.compareAndSwapRef()— atomic ref update onRefPort, implemented in bothGitGraphAdapterandInMemoryGraphAdapter.timestampfield changed from ISO-8601 string to POSIX millisecond integer indocs/specs/AUDIT_RECEIPT.md. All golden vector CBOR hex regenerated.Test plan
npm run lint— cleannpm run test:local— 169 files, 3438 tests passnpx tsc --noEmit— cleannode scripts/ts-policy-check.js— cleanSummary by CodeRabbit
New Features
Documentation
Tests
Chores