refactor(storage): delegate immutable payloads to git-cas - #749
Merged
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (130)
📝 WalkthroughWalkthroughThis PR replaces Git blob/OID-based persistence with an opaque asset/bundle handle storage substrate backed by git-cas, introducing StorageHandle/AssetHandle/BundleHandle and StorageRetentionWitness, plus dedicated semantic storage ports (checkpoint, index, patch journal, audit log, strand, intent) and their git-cas adapters. Runtime/graph/patch/audit/strand/checkpoint services, migration scripts, CI policy, documentation, and the entire test suite are rewired onto this new substrate, replacing OID/content-OID fields with handles throughout. Estimated code review effort: 5 (Critical) | ~150 minutes ChangesStorage substrate migration
Sequence Diagram(s)sequenceDiagram
participant Writer
participant PatchBuilder
participant AssetStoragePort
participant PatchJournalPort
participant GitCasPatchJournalAdapter
Writer->>PatchBuilder: attachContent(bytes)
PatchBuilder->>AssetStoragePort: stage(bytes)
AssetStoragePort-->>PatchBuilder: StagedAsset(handle)
PatchBuilder->>PatchBuilder: commitWithEvidence()
PatchBuilder->>PatchJournalPort: appendPatch(patch, attachments)
PatchJournalPort->>GitCasPatchJournalAdapter: publish commit
GitCasPatchJournalAdapter-->>PatchBuilder: PatchCommitResult(sha, retention)
sequenceDiagram
participant CheckpointController
participant CheckpointStorePort
participant CborCheckpointStoreAdapter
participant AssetStoragePort
CheckpointController->>CheckpointStorePort: publishCheckpoint(record)
CheckpointStorePort->>CborCheckpointStoreAdapter: write state tree + commit
CborCheckpointStoreAdapter->>AssetStoragePort: resolve legacy pointers
CborCheckpointStoreAdapter-->>CheckpointController: checkpointSha
CheckpointController->>CheckpointStorePort: loadBasis(checkpointSha)
CheckpointStorePort-->>CheckpointController: indexShardHandles
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
flyingrobots
commented
Jul 15, 2026
flyingrobots
left a comment
flyingrobots
left a comment
Member
Author
There was a problem hiding this comment.
Self-review and code-lawyer pass complete.
Blocking findings: none.
Verified invariants:
src/domainandsrc/portscontain no git-cas/plumbing imports or raw blob/tree capabilities; the AST mutation test enforces both static imports and import types.- Patch publication stages one immutable patch asset, includes every deterministically deduplicated attachment in an ordered bundle, publishes that bundle through the causal writer ref, and returns the git-cas retention witness.
- Current-format integrity and encryption failures propagate; only recognized missing/legacy conditions enter explicit policy-gated compatibility paths.
- Strand legacy blobs are distinguished from publication commits by object type, and trust retries cannot silently publish a signature over a stale predecessor.
- The real-repository test checks the content asset, patch asset, and publication tree against immediate prune output.
- The canonical release command
npm pack --dry-runpasses and includesdist/index.js/dist/index.d.tswhile excluding retired browser and legacy roots.
Residual, non-blocking work is intentionally not claimed here: chronological patch/intent/trust scans still collect identifier chains before oldest-first emission, and checkpoint/index materialization remains the #738/#739 bounded-memory and warm-cache work. Publication conflicts may leave staged assets in git-cas orphan grace, which is the intended reclaimable lifecycle rather than a permanent WARP-owned anchor.
Release Preflight
If this PR is from a |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 60
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/domain/warp/WarpCoreRuntimeProduct.ts (1)
42-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDuplicated method-binding table across
WarpCoreRuntimeProduct.tsandWarpGraphRuntimeProduct.ts. Both functions independently re-list the same ~40 runtime method bindings (this PR had to edit both in lockstep for the content-handle rename and the two new evidence-carrying patch methods), so a future addition to one list but not the other would silently diverge.
src/domain/warp/WarpCoreRuntimeProduct.ts#L42-L148: buildbuildWarpCoreRuntimeSurfaceby spreadingbuildWarpGraphRuntimeSurface(runtime)and layering only the Core-only extras (materialize*,fork,createWormhole,_effectPipeline,_crypto) on top, instead of re-listing every shared method.src/domain/warp/WarpGraphRuntimeProduct.ts#L13-L93: keep as the single source of truth for the shared binding table thatWarpCoreRuntimeProduct.tscomposes over.🤖 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 `@src/domain/warp/WarpCoreRuntimeProduct.ts` around lines 42 - 148, Refactor buildWarpCoreRuntimeSurface in src/domain/warp/WarpCoreRuntimeProduct.ts (lines 42-148) to spread buildWarpGraphRuntimeSurface(runtime) and retain only Core-specific additions: materialize* methods, fork, createWormhole, _effectPipeline, and _crypto. Keep src/domain/warp/WarpGraphRuntimeProduct.ts (lines 13-93) unchanged as the single source of truth for shared runtime bindings.src/domain/services/audit/AuditReceiptService.ts (1)
431-442: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not degrade permanently on non-conflict retry failures.
This catch also handles transient storage, encoding, and crypto failures from
_commitInner(). Those failures currently disable every future audit publication; degrade only when the retry specifically exhausts CAS conflicts.Proposed correction
- } catch { - // Second failure → degraded mode + } catch (error) { + if (!(error instanceof AuditError) + || error.code !== AuditError.E_AUDIT_CAS_FAILED) { + throw error; + } + // Second CAS conflict → degraded mode this._degraded = true;🤖 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 `@src/domain/services/audit/AuditReceiptService.ts` around lines 431 - 442, Update the retry handling around _commitInner in AuditReceiptService so _degraded is set and AuditError.E_AUDIT_DEGRADED is thrown only when the retry fails because CAS conflicts are exhausted. Propagate transient storage, encoding, and crypto errors unchanged instead of entering permanent degraded mode; distinguish the conflict failure using the existing _commitInner error signal or conflict error symbol.src/domain/utils/streamUtils.ts (1)
33-40: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winsrc/domain/utils/streamUtils.ts:15-18 — Require a callable
Symbol.asyncIteratorbefore narrowing.isAsyncIterable()accepts objects like{ [Symbol.asyncIterator]: 1 }, soisStreamingInput()andnormalizeToAsyncIterable()can treat invalid values as async iterables and fail at iteration time. Checktypeof value[Symbol.asyncIterator] === 'function'.🤖 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 `@src/domain/utils/streamUtils.ts` around lines 33 - 40, Update isAsyncIterable and the async-iterable check used by isStreamingInput and normalizeToAsyncIterable to require typeof value[Symbol.asyncIterator] === 'function' before narrowing. Preserve the existing exclusion for Uint8Array and string values, and ensure invalid non-callable asyncIterator properties are rejected.Source: Coding guidelines
src/ports/IndexStorePort.ts (2)
31-60: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDistinguish index aggregates from shard assets across the contract.
The same
AssetHandleruntime type currently represents both a Git tree aggregate and individual byte blobs, allowing handles to be used with the wrong operation.
src/ports/IndexStorePort.ts#L31-L60: return/acceptBundleHandleor a dedicatedIndexHandlefor the aggregate; retainAssetHandlefor shards.src/infrastructure/adapters/CborIndexStoreAdapter.ts#L107-L158: wrap tree and blob OIDs using their corresponding distinct handle concepts.test/unit/ports/IndexStorePort.test.ts#L17-L35: use separate aggregate and shard fixture handle types.test/unit/infrastructure/CborIndexStoreAdapter.test.ts#L68-L121: assert the aggregate handle separately from returned shard asset handles.As per coding guidelines, “Use explicit domain concepts with validated constructors” and runtime-backed nouns.
🤖 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 `@src/ports/IndexStorePort.ts` around lines 31 - 60, Distinguish aggregate index handles from individual shard asset handles throughout the index-store contract and tests. In src/ports/IndexStorePort.ts lines 31-60, use BundleHandle or a dedicated IndexHandle for tree inputs and outputs while retaining AssetHandle for shard blobs. In src/infrastructure/adapters/CborIndexStoreAdapter.ts lines 107-158, wrap tree and blob OIDs with their corresponding validated handle types. Update fixtures and assertions in test/unit/ports/IndexStorePort.test.ts lines 17-35 and test/unit/infrastructure/CborIndexStoreAdapter.test.ts lines 68-121 to use and verify separate aggregate and shard handle types.Source: Coding guidelines
31-36: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSeparate aggregate index handles from shard asset handles.
writeShardsreturns a tree aggregate asAssetHandle, whileopenSharduses the same type for byte assets. Callers can therefore pass an index tree toopenShardor a shard blob toscanShards, producing provider-level lookup failures. ReturnBundleHandleor a dedicated validatedIndexHandlefor the aggregate and reserveAssetHandlefor shards.The OID-oriented documentation also contradicts the opaque-handle contract.
As per coding guidelines, “Use explicit domain concepts with validated constructors” and keep domain objects as “runtime-backed nouns.”
Also applies to: 39-60
🤖 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 `@src/ports/IndexStorePort.ts` around lines 31 - 36, Update the index-handle contract around writeShards and its related methods to use a distinct validated aggregate handle type, such as BundleHandle or a dedicated IndexHandle, while retaining AssetHandle only for shard byte assets. Align the method signatures and callers so aggregate handles cannot be passed to openShard or shard handles to scanShards, and revise the writeShards documentation to describe the opaque validated handle rather than an OID.Source: Coding guidelines
src/infrastructure/adapters/GitTrustChainAdapter.ts (1)
168-183: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire an exact legacy tree before falling back to raw
record.cbor.MANIFEST_NOT_FOUNDalone doesn’t prove the commit is legacy; a malformed current-format asset tree can reach this fallback and returnnull. After the compatibility-policy gate, validate the legacy layout explicitly or fail closed.🤖 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 `@src/infrastructure/adapters/GitTrustChainAdapter.ts` around lines 168 - 183, Update _readRecordIdFromCommit so the raw record.cbor fallback runs only after confirming the asset tree matches the exact legacy trust-tree layout, not merely after rethrowUnlessLegacyTrustTree and _requireLegacyTrustRecordPolicy. Validate the tree entries and required legacy structure before reading RECORD_BLOB_NAME; otherwise fail closed instead of returning null for malformed current-format trees.
🤖 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/api/Evidence.ts`:
- Around line 13-25: Replace the exported RetentionEvidence shape in
src/domain/api/Evidence.ts, lines 13-25, with a frozen validated runtime-backed
domain object using the established constructor pattern and instanceof dispatch.
Update src/domain/api/EvidenceRuntime.ts, lines 197-235, to construct that
object through its validated constructor, validating policy, reachability, and
rootKind rather than copying arbitrary values; retain Evidence’s public API
behavior while preventing invalid retention claims.
In `@src/domain/capabilities/QueryCapability.ts`:
- Around line 111-113: Update the JSDoc comments above getContentHandle and
getEdgeContentHandle to refer to the attached content blob handle, not an OID.
Keep the method signatures and behavior unchanged while consistently describing
the handle as opaque.
In `@src/domain/services/audit/AuditChainVerifier.ts`:
- Around line 167-170: The readHead failure path in AuditChainVerifier must
return an explicit ERROR result with an AUDIT_HEAD_UNAVAILABLE diagnostic
instead of returning the successful empty-chain result; preserve the valid-empty
result only when readHead actually returns null. Update
src/domain/services/audit/AuditChainVerifier.ts lines 167-170 and adjust
test/unit/domain/services/audit/AuditChainVerifier.test.ts lines 163-172 to
assert the error behavior.
In `@src/domain/services/audit/AuditReceiptService.ts`:
- Line 297: Update the JSDoc for AuditReceiptService.commit to document that it
returns a PublishedAuditRecord or null, replacing the outdated SHA return
description. Keep the method signature and implementation unchanged.
- Around line 392-405: Replace the `errorCode(error) === 'PUBLICATION_CONFLICT'`
check in `AuditReceiptService` with the typed publication-conflict contract
exposed by `AuditLogPort.append`. Define or reuse a dedicated
`PublicationConflictError` or result type at the port boundary, update adapters
to emit it, and branch on that type/result while preserving the existing
retry-once and degradation behavior.
In `@src/domain/services/controllers/CheckpointController.ts`:
- Around line 266-267: Update the checkpoint-loading error handling around
loadCheckpoint in CheckpointController so only the store’s typed missing-object
error is converted to null. Remove message-substring checks for “missing” or
“not found,” and rethrow all decoding, integrity, and other failures unchanged.
- Line 74: Update _readPatch and _hasSchema1Patches so legacy patch metadata is
inspected before applying the schema 2|3 Patch type; remove the narrowing cast
that prevents detecting schema 1 or missing schema values, and use decoded
commit/storage metadata directly or introduce a dedicated legacy-read result
while preserving normal Patch handling for current schemas.
In `@src/domain/services/controllers/IntentController.ts`:
- Around line 88-107: Align the queued-intent lookup with the storage key used
by queueIntent: update getWriterIntents to scan the queued channel using the
strand identifier stored as ownerId, or consistently rename the API and
parameter to represent that identifier. Ensure both publish and read paths use
the same key.
In `@src/domain/services/controllers/PatchController.ts`:
- Around line 78-80: Update the _auditService.commit type in PatchController to
return the concrete runtime-backed audit publication result type used by the
audit port/service instead of Promise<unknown>. Reuse the existing result type
definition from that port or service and preserve the commit method’s
Promise-based contract.
In `@src/domain/services/index/BitmapIndexReader.ts`:
- Around line 331-339: Update _loadShardBuffer to normalize any value caught
from openShard into a genuine Error before assigning it to ShardLoadError.cause.
Preserve existing Error instances and wrap non-Error throws with an appropriate
message, replacing the unsafe cause as Error assertion while keeping the
existing shardPath and oid metadata unchanged.
In `@src/domain/services/index/LogicalIndexReader.ts`:
- Around line 84-101: Update loadFromHandles so _resetState() runs only after
all shard handles have decoded successfully; perform decoding first, then reset
and process the decoded items. Preserve the existing index when
indexStore.decodeShard() rejects, while retaining the current successful reload
behavior.
In `@src/domain/services/index/PropertyIndexReader.ts`:
- Around line 61-64: Update the JSDoc for PropertyIndexReader.setupHandles to
describe configuring AssetHandle mappings rather than OID mappings, while
leaving the method behavior unchanged.
In `@src/domain/services/optic/CheckpointContentAnchorFact.ts`:
- Around line 18-30: Adopt AssetHandle as the runtime-backed type throughout the
content-anchor pipeline: in
src/domain/services/optic/CheckpointContentAnchorFact.ts lines 18-30, update the
property, constructor option, and validation to use AssetHandle; in lines 47-55,
retain the AssetHandle internally and convert it to its raw value only when
creating transport data; in src/domain/services/optic/CheckpointFactResolver.ts
lines 98-111, rename the resolver to resolveContentHandle() and make its return
type and implementation AssetHandle | null.
In `@src/domain/services/optic/CheckpointShardFactReader.ts`:
- Around line 63-74: Update the checkpoint shard read paths around the
liveness-root and index-handle lookups to return the absent fallback only when
both entries are missing. Treat one-sided presence and any handle whose string
value differs from oid as a bounded-basis integrity failure before constructing
LogicalIndexReader, while preserving the existing read-failure handling. Add
tests covering missing-entry and mismatched-handle cases for both affected
paths.
In `@src/domain/services/optic/CheckpointTailBasisVerifier.ts`:
- Around line 41-43: Update CheckpointTailBasisVerifier.verify around
requireIndexShards to validate every referenced index shard handle through
source._indexStore before returning success. Ensure dangling or unretained
handles cause verification to fail, while preserving the existing schema and
shard-handle checks.
In `@src/domain/services/PatchBuilder.ts`:
- Around line 289-297: Track pending attachment staging across both
attachment-writing methods that await storeContentAttachmentPayload: increment
the pending-operation counter before the await and decrement it in a finally
block. Update commitWithEvidence() to reject or await while pending attachment
operations remain, preventing commit completion before resumed attachments are
recorded.
In `@src/domain/services/PatchBuilderContent.ts`:
- Around line 12-16: The domain ContentInput contract must remain host-neutral:
remove ReadableStream from ContentInput in
src/domain/services/PatchBuilderContent.ts (lines 12-16), keeping
AsyncIterable<Uint8Array>, Uint8Array, and string. Move ReadableStream detection
and conversion out of src/domain/utils/streamUtils.ts (lines 33-40) into the
adapter or controller boundary so domain code receives an AsyncIterable before
invoking PatchBuilderContent.
In `@src/domain/services/PatchCommitter.ts`:
- Around line 47-52: Update the class-level commit-flow documentation for
PatchCommitter to reflect that publication is delegated to PatchJournalPort
rather than directly persisting blobs, trees, commits, and refs. Preserve the
documented CAS check, Lamport resolution, patch construction, and callback steps
where they still match the implementation.
- Line 45: The patch publication result is currently a service-local shape alias
rather than a validated domain object. In
src/domain/services/PatchCommitter.ts:45, replace PatchCommitResult with a
dedicated frozen domain class in its own file; update
src/domain/capabilities/PatchCapability.ts:11 to import that class from its
domain module, and src/domain/capabilities/PatchCapability.ts:50-53 to return
the dedicated publication-result object.
- Around line 151-157: Update the append error handling in PatchCommitter’s
catch paths, including rethrowPublicationConflict, so head inspection via
readRef occurs only for an explicit PUBLICATION_CONFLICT; rethrow all other
publication or storage failures unchanged without diagnostic reads, preserving
their original errors.
In `@src/domain/services/state/checkpointLoad.ts`:
- Around line 53-60: Update loadCheckpoint at the checkpointStore.loadCheckpoint
boundary to validate checkpoint.schema against the current supported schema
before constructing LoadedCheckpoint. Reject non-current or retired schemas,
while preserving loading for the current schema so all callers, including
materializeAt, enforce the migration boundary.
In `@src/domain/services/state/StateReaderContext.ts`:
- Line 23: Replace the locally defined ContentMeta type in StateReaderContext.ts
with the canonical ContentMeta from src/domain/types/ContentMeta.ts, importing
or re-exporting it as appropriate and removing the duplicate structural
definition. Keep existing consumers using the canonical contract.
In `@src/domain/services/strand/StrandDescriptorStore.ts`:
- Around line 201-211: Update writeDescriptor to validate descriptor.strandId
through the same buildRef path used by readDescriptor, hasDescriptor, and
deleteDescriptor before publishing to _strandStore. Preserve the existing
descriptor serialization and attachments, and use the validated strand
reference/id for the publish request.
In `@src/domain/services/strand/StrandPatchService.ts`:
- Around line 194-223: Keep asset handles runtime-backed through the queued
patch flow: update the relevant StrandPatchService method to accept and forward
readonly AssetHandle[] directly, removing string conversion and reconstruction;
serialization must remain only at the intent-store codec boundary. In
test/unit/domain/services/strand/StrandService.test.ts lines 606-619, update
assertions to expect AssetHandle instances rather than serialized strings.
In `@src/domain/services/sync/syncPatchLoader.ts`:
- Around line 92-96: Make PatchJournalPort mandatory across the patch-loading
and sync APIs: update loadPatchFromCommit and loadPatchRange in
src/domain/services/sync/syncPatchLoader.ts to require and forward patchJournal,
and update the sync response contract in
src/domain/services/sync/syncRequestResponse.ts to require it whenever patches
may be returned.
In `@src/domain/services/sync/syncRequestResponse.ts`:
- Around line 232-238: The ancestry check in the sync request/response flow
should use an explicitly declared port capability rather than casting
persistence to an ad hoc optional intersection. Update the applicable
persistence contract or inject a named ancestry port exposing isAncestor, then
call that declared capability from the pre-check while preserving the existing
divergence behavior.
In `@src/domain/trust/TrustRecordService.ts`:
- Around line 23-30: Replace the exported TrustRetryTip type in
TrustRecordService with a dedicated frozen TrustRetryTip class in
TrustRetryTip.ts, including a validated constructor and runtime identity via
instanceof. Update RetryOptions and the retry/resign flow to accept and pass
TrustRetryTip instances while preserving the existing tipSha and recordId
values.
- Around line 100-102: Update the error handling in the TrustRecordService retry
paths around isRetryableConflict to narrow caught errors to TrustError before
invoking the classifier, while preserving runtime instanceof dispatch and
rethrowing non-retryable errors. Remove the explicit unknown type introduced in
the domain code, including the additional occurrence noted in the comment.
In `@src/domain/types/CoordinateComparison.ts`:
- Line 174: Update the attach_node_content transfer operation in
CoordinateComparison to type contentHandle as the validated
ContentAttachmentHandle (or canonical storage-handle type) rather than string,
including the corresponding definition at the adjacent operation. Keep string
conversion confined to the adapter boundary.
In `@src/domain/utils/RefLayout.ts`:
- Around line 315-322: Update buildIntentRef to validate the channel argument at
runtime, accepting only "admitted" or "queued" and throwing WarpError for
unsupported values before constructing the reference. Preserve the existing
graphName and ownerId validation and reference format.
In `@src/domain/WarpCore.ts`:
- Around line 45-48: Update the JSDoc descriptions for getContentHandle and
getEdgeContentHandle to describe their return values as opaque content handles
rather than content blob OIDs, without changing the accessor declarations or
runtime behavior.
In `@src/domain/WarpWorldline.ts`:
- Around line 112-121: Extract the repeated null-check-and-throw logic from
commitWithEvidence and patchDraftWithEvidence into a private helper such as
requirePort, parameterized by the optional port, error message, and error code.
Update these methods and the existing commit, createDraft, patchDraft,
previewDraftJoin, and prepareOpticBasis guards to use the helper while
preserving each method’s current error details and behavior.
In `@src/infrastructure/adapters/CborCheckpointStoreAdapter.ts`:
- Around line 281-282: Update the decoding logic near the adapter’s
frontier-loading method to decode into an untrusted value rather than
Record<string, string>, validate that the result is a non-null plain record
whose keys and values are strings, and only then construct the Map. Reject
malformed values such as null, numbers, arrays, or non-string entries before
returning the causal basis.
In `@src/infrastructure/adapters/CborPatchJournalAdapter.ts`:
- Around line 132-145: Update the traversal validation after the loop in the
method containing getNodeInfo and decodePatch so any traversal that stops before
reaching fromSha is rejected, including when a non-patch commit leaves current
non-null. Preserve successful completion only when fromSha is actually reached,
and raise the existing SyncError with its current divergence context otherwise.
In `@src/infrastructure/adapters/GitCasAssetStorageAdapter.ts`:
- Around line 110-119: Move the compatibility-policy check in
GitCasAssetStorageAdapter.#openLegacyBlob before calling `#readLegacyCandidate`,
preserving the existing error for disabled legacyContentBlobReads and only
reading/returning bytes when enabled. In
test/unit/infrastructure/adapters/GitCasAssetStorageAdapter.test.ts lines
107-110, update the current-only policy test to assert legacyReader.readBlob was
not called.
- Around line 62-67: Update GitCasAssetStorageAdapter.stage() when constructing
putOptions to preserve AssetWriteOptions.mime and AssetWriteOptions.expectedSize
by passing them to AssetPutOptions if supported by `@git-stunts/git-cas`;
otherwise remove these fields from AssetStoragePort so its contract matches the
adapter’s capabilities.
In `@src/infrastructure/adapters/GitCasAuditLogAdapter.ts`:
- Around line 104-127: Update the legacy fallback in readEntry() and
`#readLegacyReceiptTree`() to consult the injected compatibility policy before
reading receipt.cbor. When legacy reads are disabled, reject the fallback path
instead of calling `#readLegacyReceiptTree`; preserve the existing legacy behavior
when the policy allows it.
- Around line 100-103: Update `#readReceiptRoot` to read the adopted handle
through the adapter’s `#assets.open`(...) method instead of
this.#cas.assets.open(...), preserving collectBytes and enabling the
contentEncryption restore path for encrypted receipts.
In `@src/infrastructure/adapters/GitCasIntentStoreAdapter.ts`:
- Around line 108-123: Update collectIntentHandles to validate node.parents
before following node.parents[0], rejecting commits with more than one parent
instead of silently ignoring additional branches. Preserve the existing
traversal for zero- or one-parent commits and fail closed using the established
assertion/error pattern.
In `@src/infrastructure/adapters/GitCasStrandStoreAdapter.ts`:
- Around line 52-60: The GitCasStrandStoreAdapter.readDescriptor method must
gate blob-backed descriptor reads through the injected substrate compatibility
policy, rejecting them when legacy strand reads are disabled while preserving
the enabled behavior. Update
src/infrastructure/adapters/GitCasStrandStoreAdapter.ts lines 52-60 to inject
and consult that policy, and update
test/unit/infrastructure/adapters/GitCasStrandStoreAdapter.test.ts lines 70-80
with cases covering both disabled rejection and explicitly enabled
compatibility.
- Around line 119-125: Update GitCasStrandStoreAdapter.deleteDescriptor to
capture the revision or head returned by reading ref, then use a revision-aware
conditional delete through StrandHistory so deletion occurs only if the ref
still matches that observed head; return false when the ref is absent or the
conditional delete detects a concurrent replacement.
In `@src/infrastructure/adapters/GitRecursiveTreeOidReaderAdapter.ts`:
- Line 303: Move the TreeEntryProbeResult type contract out of
GitRecursiveTreeOidReaderAdapter.ts into the shared port/domain module used by
both adapters, then update GitRecursiveTreeOidReaderAdapter and
GitTimelineHistoryAdapter imports to reference that shared definition.
In `@src/infrastructure/adapters/GitTrustChainAdapter.ts`:
- Around line 317-343: Move adaptGitCasRetentionWitness out of the try/catch
surrounding cas.publications.commit in the publication flow. Catch only commit
errors, preserve direct rethrow behavior for non-PUBLICATION_CONFLICT errors
through _rethrowPublicationConflict, then adapt publication.witness and build
the frozen result after the catch so post-commit adaptation failures are not
rewritten as conflicts.
In `@src/infrastructure/adapters/GraphModelMigrationDryRunRequestJsonAdapter.ts`:
- Around line 147-155: Update the contentHandle selection in the
GraphModelMigrationContentSource construction to fall back to contentOid only
when the contentHandle key is absent, not when it is explicitly null; preserve
null so readRequiredString validates and rejects it, and add a regression test
covering contentHandle: null together with a valid contentOid.
In `@src/infrastructure/adapters/TrailerCommitMessageCodecAdapter.ts`:
- Around line 343-348: Update the decoded result construction in the trailer
commit message adapter to omit patchOid when spreading legacy parsed data, while
retaining patchHandle created from patchHandle or patchOid. Ensure the returned
object matches PatchCommitMessage at runtime and add a regression assertion
verifying legacy decoding has no patchOid property.
In `@test/conformance/fixtures/V17CheckpointShardIndexFixture.ts`:
- Around line 33-39: Rename V17CheckpointShardIndexFixture methods
nodeLivenessShardOid and propertyShardOid to nodeLivenessShardHandle and
propertyShardHandle, updating all callers. In
test/conformance/fixtures/V17CheckpointTargetShardFixture.ts lines 12-19, rename
the constructor option shardOid to shardHandle and update every call site
consistently.
In `@test/conformance/v18FirstUseOpticsHonesty.test.ts`:
- Around line 201-206: Update the test for prepareOpticBasisImplementation to
assert separately that the preparation implementation delegates to the verifier
and that CheckpointTailBasisVerifier performs the bounded
_checkpointStore.loadBasis read; avoid concatenating both source files before
checking the delegation assertion.
In `@test/helpers/InMemoryAuditLogAdapter.ts`:
- Around line 19-21: Update the read operations listWriterIds and readEntry to
honor `#readFailure` before performing storage reads. Add and reuse a private
guard for the configured error, ensuring failReadsWith consistently causes every
read operation to throw while preserving normal behavior when no failure is
configured.
In `@test/helpers/InMemoryBlobStorageAdapter.ts`:
- Around line 23-45: Update InMemoryBlobStorageAdapter.stage to compare
bytes.byteLength with options.expectedSize after collecting the source and
before computing the hash or storing the bytes; reject the asset when the sizes
differ, while preserving the existing staging behavior for matching sizes.
In `@test/helpers/InMemoryGitCasFacade.ts`:
- Around line 138-155: Update the bundle creation logic around descriptorOid in
the InMemoryGitCasFacade method to encode the joined descriptor text once, reuse
those encoded bytes when calling writeBlob, and set StagedBundle’s
descriptorBytes from the encoded byte length rather than the string length.
In `@test/helpers/MemoryRuntimeHost.ts`:
- Around line 28-40: Require createMemoryRuntimeStorage to accept
InMemoryGraphAdapter, or an explicit structural contract matching the adapter’s
supported capabilities, instead of CorePersistence with a cast; update its
callers and cache typing accordingly. In
test/helpers/MemoryRuntimeStorageAdapter.ts:96-122, remove
withFixtureObjectTypeProbe’s fallback proxy, or replace it with complete
tracking of blob, tree, and commit objects for every creation API.
In `@test/helpers/MockIndexStorage.ts`:
- Around line 25-39: Update MockIndexStorage.writeShards to consume the supplied
WarpStream and persist its shard records plus handle mapping under the newly
returned AssetHandle. Make scanShards and readShardHandles retrieve the stored
data for that handle instead of always returning empty values, while preserving
empty results for unknown handles.
In `@test/unit/domain/services/PatchBuilder.content.test.ts`:
- Around line 57-73: Update the test around createPatchBuilder and attachContent
to use gated producer and storage doubles instead of RecordingAssetStorage;
block the producer after its first chunk, assert stage() has been entered before
releasing the remaining chunks, then verify the existing content, expectedSize,
and _content.size assertions.
In `@test/unit/domain/services/PropertyIndex.test.ts`:
- Around line 97-109: Update the test using PropertyIndexBuilder and yieldShards
to encode corresponding shard.entries through the codec, then compare the
resulting byte arrays rather than only comparing PropertyShard objects. Preserve
the equivalent operation-order setup and verify each corresponding shard
produces identical encoded bytes.
In `@test/unit/domain/services/WarpMessageCodec.test.ts`:
- Around line 153-184: Update the malformed-locator coverage in the test
“rejects malformed graph, writer, lamport, hash, and handles” by replacing or
adding a locator assertion that uses an invalid legacy OID or AssetHandle value
and expects the codec to reject it. Keep the existing graph, writer, lamport,
and state-hash validation cases intact.
In `@test/unit/domain/warp/Writer.test.ts`:
- Around line 364-367: Remove the unused persistence.writeBlob,
persistence.writeTree, and persistence.commitNodeWithTree mock setups from this
test block and the corresponding block around the second setup location. Keep
persistence.updateRef only if it is exercised by the tested flow, and leave
RecordingPatchJournal.appendPatch() behavior unchanged.
In `@test/unit/domain/WarpCore.blobAutoConstruct.test.ts`:
- Around line 26-29: Strengthen the wiring assertions in the test by replacing
the `.toBeInstanceOf(...)` checks for `_checkpointStore` and `_indexStore` with
exact identity checks against `services.checkpoints` and `services.indexes`,
matching the existing `_assetStorage` assertion. Keep the test focused on
verifying reuse of the instances returned by `createRuntimeStorageServices`.
In `@test/unit/infrastructure/adapters/GitCasAuditLogAdapter.test.ts`:
- Around line 66-84: Update the legacy compatibility test around log.readEntry
to first assert that legacy storage is rejected when
V17_SUBSTRATE_MIGRATION_COMPATIBILITY_POLICY is unset. Then enable that policy
explicitly and retain the existing successful read assertions, ensuring the
fixture no longer relies on an implicit compatibility default.
In `@test/unit/infrastructure/adapters/GitTrustChainAdapter.test.ts`:
- Around line 124-128: Update the missing-ref tests for
GitTrustChainAdapter.readTip and readRecords to reject with a not-found-specific
Git error shape instead of a generic Error, and add assertions that unrelated
rev-parse failures propagate rather than becoming null or empty results. Keep
the existing missing-reference outcomes unchanged.
---
Outside diff comments:
In `@src/domain/services/audit/AuditReceiptService.ts`:
- Around line 431-442: Update the retry handling around _commitInner in
AuditReceiptService so _degraded is set and AuditError.E_AUDIT_DEGRADED is
thrown only when the retry fails because CAS conflicts are exhausted. Propagate
transient storage, encoding, and crypto errors unchanged instead of entering
permanent degraded mode; distinguish the conflict failure using the existing
_commitInner error signal or conflict error symbol.
In `@src/domain/utils/streamUtils.ts`:
- Around line 33-40: Update isAsyncIterable and the async-iterable check used by
isStreamingInput and normalizeToAsyncIterable to require typeof
value[Symbol.asyncIterator] === 'function' before narrowing. Preserve the
existing exclusion for Uint8Array and string values, and ensure invalid
non-callable asyncIterator properties are rejected.
In `@src/domain/warp/WarpCoreRuntimeProduct.ts`:
- Around line 42-148: Refactor buildWarpCoreRuntimeSurface in
src/domain/warp/WarpCoreRuntimeProduct.ts (lines 42-148) to spread
buildWarpGraphRuntimeSurface(runtime) and retain only Core-specific additions:
materialize* methods, fork, createWormhole, _effectPipeline, and _crypto. Keep
src/domain/warp/WarpGraphRuntimeProduct.ts (lines 13-93) unchanged as the single
source of truth for shared runtime bindings.
In `@src/infrastructure/adapters/GitTrustChainAdapter.ts`:
- Around line 168-183: Update _readRecordIdFromCommit so the raw record.cbor
fallback runs only after confirming the asset tree matches the exact legacy
trust-tree layout, not merely after rethrowUnlessLegacyTrustTree and
_requireLegacyTrustRecordPolicy. Validate the tree entries and required legacy
structure before reading RECORD_BLOB_NAME; otherwise fail closed instead of
returning null for malformed current-format trees.
In `@src/ports/IndexStorePort.ts`:
- Around line 31-60: Distinguish aggregate index handles from individual shard
asset handles throughout the index-store contract and tests. In
src/ports/IndexStorePort.ts lines 31-60, use BundleHandle or a dedicated
IndexHandle for tree inputs and outputs while retaining AssetHandle for shard
blobs. In src/infrastructure/adapters/CborIndexStoreAdapter.ts lines 107-158,
wrap tree and blob OIDs with their corresponding validated handle types. Update
fixtures and assertions in test/unit/ports/IndexStorePort.test.ts lines 17-35
and test/unit/infrastructure/CborIndexStoreAdapter.test.ts lines 68-121 to use
and verify separate aggregate and shard handle types.
- Around line 31-36: Update the index-handle contract around writeShards and its
related methods to use a distinct validated aggregate handle type, such as
BundleHandle or a dedicated IndexHandle, while retaining AssetHandle only for
shard byte assets. Align the method signatures and callers so aggregate handles
cannot be passed to openShard or shard handles to scanShards, and revise the
writeShards documentation to describe the opaque validated handle rather than an
OID.
🪄 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: 81183cca-d0df-438a-af35-3c13f465c6f2
📒 Files selected for processing (294)
README.mdbin/cli/commands/trust.tsbin/cli/commands/verify-audit.tsdocs/migrations/v19/README.mddocs/topics/README.mddocs/topics/api/README.mddocs/topics/getting-started.mdpolicy/quarantines/0025B-boundary.jsonscripts/hooks/pre-pushscripts/migrations/v17.0.0/checkpoint-schema-upgrade.tsscripts/run-stable-unit-tests.tsscripts/source-size-gate.tsscripts/source-version-name-policy.tsscripts/upgrade-v16-to-v17.tsscripts/v18.0.0/migrations/graph-model/GraphModelMigrationSourceInventoryCollector.tsscripts/v18.0.0/migrations/graph-model/V17RestoredPublicReadLegacyReadingBuilder.tssrc/domain/RuntimeHost.tssrc/domain/WarpApp.tssrc/domain/WarpCore.tssrc/domain/WarpGraph.tssrc/domain/WarpWorldline.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/Evidence.tssrc/domain/api/EvidenceRuntime.tssrc/domain/api/TimelineRuntime.tssrc/domain/api/WriteRuntime.tssrc/domain/capabilities/PatchCapability.tssrc/domain/capabilities/PatchCollector.tssrc/domain/capabilities/QueryCapability.tssrc/domain/capabilities/StrandCapability.tssrc/domain/errors/EncryptionError.tssrc/domain/graph/ContentAttachmentHandle.tssrc/domain/graph/ContentAttachmentOid.tssrc/domain/graph/ContentAttachmentPayload.tssrc/domain/graph/ContentAttachmentWriteIntent.tssrc/domain/graph/publicGraphSubstrate.tssrc/domain/migrations/GraphModelMigrationContentSource.tssrc/domain/services/ContentAttachmentProjection.tssrc/domain/services/CoordinateFactExport.tssrc/domain/services/MaterializedViewHelpers.tssrc/domain/services/MaterializedViewService.tssrc/domain/services/PatchBuilder.tssrc/domain/services/PatchBuilderContent.tssrc/domain/services/PatchCommitter.tssrc/domain/services/WormholeService.tssrc/domain/services/audit/AuditChainVerifier.tssrc/domain/services/audit/AuditReceiptService.tssrc/domain/services/audit/AuditVerifierService.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/ComparisonEngine.tssrc/domain/services/controllers/ComparisonSelectorSupport.tssrc/domain/services/controllers/ForkController.tssrc/domain/services/controllers/IntentController.tssrc/domain/services/controllers/MaterializeCheckpointStrategy.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeStrategyRuntime.tssrc/domain/services/controllers/PatchController.tssrc/domain/services/controllers/PatchDiscovery.tssrc/domain/services/controllers/ProvenanceController.tssrc/domain/services/controllers/QueryContent.tssrc/domain/services/controllers/QueryController.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/services/controllers/StrandController.tssrc/domain/services/controllers/SyncControllerTypes.tssrc/domain/services/controllers/detachedOpen.tssrc/domain/services/index/BitmapIndexReader.tssrc/domain/services/index/LogicalIndexReader.tssrc/domain/services/index/PropertyIndexReader.tssrc/domain/services/optic/CheckpointBasisFactTypes.tssrc/domain/services/optic/CheckpointContentAnchorFact.tssrc/domain/services/optic/CheckpointFactResolver.tssrc/domain/services/optic/CheckpointNeighborhoodPageReader.tssrc/domain/services/optic/CheckpointPatchFactStream.tssrc/domain/services/optic/CheckpointShardFactReader.tssrc/domain/services/optic/CheckpointTailBasisLoader.tssrc/domain/services/optic/CheckpointTailBasisVerifier.tssrc/domain/services/optic/CheckpointTailOpticSource.tssrc/domain/services/optic/CoordinateCheckpointTailOpticSource.tssrc/domain/services/state/StateReaderContext.tssrc/domain/services/state/checkpointCreate.tssrc/domain/services/state/checkpointHelpers.tssrc/domain/services/state/checkpointLoad.tssrc/domain/services/strand/StrandCoordinator.tssrc/domain/services/strand/StrandDescriptorStore.tssrc/domain/services/strand/StrandIntentService.tssrc/domain/services/strand/StrandPatchService.tssrc/domain/services/strand/createStrandCoordinator.tssrc/domain/services/strand/descriptorNormalization.tssrc/domain/services/strand/strandTypes.tssrc/domain/services/sync/syncPatchLoader.tssrc/domain/services/sync/syncRequestResponse.tssrc/domain/services/transfer/transferOps.tssrc/domain/storage/AssetHandle.tssrc/domain/storage/BundleHandle.tssrc/domain/storage/StorageHandle.tssrc/domain/storage/StorageRetentionWitness.tssrc/domain/stream/Sink.tssrc/domain/trust/TrustRecordService.tssrc/domain/types/ContentMeta.tssrc/domain/types/CoordinateComparison.tssrc/domain/types/StrandDescriptor.tssrc/domain/types/WarpIntentDescriptor.tssrc/domain/types/WarpPersistence.tssrc/domain/utils/RefLayout.tssrc/domain/utils/parseStrandBlob.tssrc/domain/utils/streamUtils.tssrc/domain/warp/RuntimeHostBoot.tssrc/domain/warp/RuntimeHostProduct.tssrc/domain/warp/RuntimePatchCollector.tssrc/domain/warp/WarpCoreRuntimeProduct.tssrc/domain/warp/WarpGraphRuntimeProduct.tssrc/domain/warp/Writer.tssrc/infrastructure/adapters/CasBlobAdapter.tssrc/infrastructure/adapters/CasPayloadPointer.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.tssrc/infrastructure/adapters/CborIndexStoreAdapter.tssrc/infrastructure/adapters/CborPatchJournalAdapter.tssrc/infrastructure/adapters/GitCasAssetStorageAdapter.tssrc/infrastructure/adapters/GitCasAuditLogAdapter.tssrc/infrastructure/adapters/GitCasIntentStoreAdapter.tssrc/infrastructure/adapters/GitCasRepositoryAdapter.tssrc/infrastructure/adapters/GitCasRetentionWitnessAdapter.tssrc/infrastructure/adapters/GitCasStrandStoreAdapter.tssrc/infrastructure/adapters/GitRecursiveTreeOidReaderAdapter.tssrc/infrastructure/adapters/GitTimelineHistoryAdapter.tssrc/infrastructure/adapters/GitTrustChainAdapter.tssrc/infrastructure/adapters/GraphModelMigrationDryRunRequestJsonAdapter.tssrc/infrastructure/adapters/TrailerCommitMessageCodecAdapter.tssrc/ports/AssetStoragePort.tssrc/ports/AuditLogPort.tssrc/ports/BlobPort.tssrc/ports/BlobStoragePort.tssrc/ports/CheckpointStorePort.tssrc/ports/CommitMessageCodecPort.tssrc/ports/CommitPort.tssrc/ports/GraphPersistencePort.tssrc/ports/IndexStoragePort.tssrc/ports/IndexStorePort.tssrc/ports/IntentStorePort.tssrc/ports/PatchJournalPort.tssrc/ports/RuntimeStorageProviderPort.tssrc/ports/StrandStorePort.tssrc/ports/TreeEntryProbePort.tssrc/ports/TreePort.tssrc/ports/TrustChainPort.tssrc/ports/WarpKernelPort.tstest/benchmark/logicalIndex.benchmark.tstest/conformance/comparisonLiveCoordinateSeam.test.tstest/conformance/fixtures/V17CheckpointBasisArtifactFixture.tstest/conformance/fixtures/V17CheckpointIndexTreeFixture.tstest/conformance/fixtures/V17CheckpointPayloadBlobFixture.tstest/conformance/fixtures/V17CheckpointShardIndexFixture.tstest/conformance/fixtures/V17CheckpointTargetShardFixture.tstest/conformance/v17CheckpointTailOpticReadBasis.test.tstest/conformance/v18FirstUseOpticsHonesty.test.tstest/helpers/FixturePatchJournal.tstest/helpers/InMemoryAuditLogAdapter.tstest/helpers/InMemoryBlobStorageAdapter.tstest/helpers/InMemoryCheckpointStore.tstest/helpers/InMemoryGitCasFacade.tstest/helpers/InMemoryGraphAdapter.tstest/helpers/MemoryRuntimeHost.tstest/helpers/MemoryRuntimeStorageAdapter.tstest/helpers/MockBlobPort.tstest/helpers/MockIndexStorage.tstest/helpers/MockTreePort.tstest/helpers/MockTrustChainPort.tstest/helpers/WarpGraphMockPersistence.tstest/helpers/storageRetention.tstest/integration/api/content-attachment.test.tstest/unit/cli/verify-audit.test.tstest/unit/domain/DraftTimelineRuntime.test.tstest/unit/domain/WarpApp.delegation.test.tstest/unit/domain/WarpCore.blobAutoConstruct.test.tstest/unit/domain/WarpCore.content.test.tstest/unit/domain/WarpCore.stateSessionAutoConstruct.test.tstest/unit/domain/WarpGraph.audit.test.tstest/unit/domain/WarpGraph.checkpoint.test.tstest/unit/domain/WarpGraph.content.test.tstest/unit/domain/WarpGraph.coverageGaps.test.tstest/unit/domain/WarpGraph.encryption.test.tstest/unit/domain/WarpGraph.strands.compare.test.tstest/unit/domain/WarpGraph.strands.test.tstest/unit/domain/WarpGraph.versionVector.test.tstest/unit/domain/WarpGraph.writerInvalidation.test.tstest/unit/domain/WarpWorldline.capabilities.test.tstest/unit/domain/WarpWorldline.test.tstest/unit/domain/graph/ContentAttachmentPayload.test.tstest/unit/domain/graph/ContentAttachmentRecord.test.tstest/unit/domain/graph/ContentAttachmentWriteIntent.test.tstest/unit/domain/graph/GraphOpAlgebra.test.tstest/unit/domain/migrations/DryRunGraphModelMigrationPlanner.test.tstest/unit/domain/migrations/GraphModelMigrationConstructorGuards.test.tstest/unit/domain/migrations/GraphModelMigrationOperationLowering.test.tstest/unit/domain/migrations/GraphModelMigrationSourceInventory.test.tstest/unit/domain/runtimeProductExecutableSurface.test.tstest/unit/domain/runtimeReadingBasisErrors.test.tstest/unit/domain/services/AuditReceiptService.coverage.test.tstest/unit/domain/services/AuditReceiptService.test.tstest/unit/domain/services/AuditVerifierService.bench.tstest/unit/domain/services/AuditVerifierService.test.tstest/unit/domain/services/BitmapIndexReader.chunked.test.tstest/unit/domain/services/BitmapIndexReader.test.tstest/unit/domain/services/CheckpointService.anchors.test.tstest/unit/domain/services/CheckpointService.edgeCases.test.tstest/unit/domain/services/CheckpointService.test.tstest/unit/domain/services/ContentAttachmentProjection.test.tstest/unit/domain/services/LogicalIndexReader.test.tstest/unit/domain/services/MaterializedViewService.test.tstest/unit/domain/services/MessageCodecModules.test.tstest/unit/domain/services/PatchBuilder.cas.test.tstest/unit/domain/services/PatchBuilder.commit.test.tstest/unit/domain/services/PatchBuilder.content.test.tstest/unit/domain/services/PatchBuilder.contentPersistence.test.tstest/unit/domain/services/PatchBuilder.test.tstest/unit/domain/services/PatchBuilderContentWriteIntent.test.tstest/unit/domain/services/PatchBuilderTestHarness.tstest/unit/domain/services/PatchCommitter.visibility.test.tstest/unit/domain/services/PropertyIndex.test.tstest/unit/domain/services/StateReaderPropertyProjection.test.tstest/unit/domain/services/StateSerializer.test.tstest/unit/domain/services/SyncProtocol.divergence.test.tstest/unit/domain/services/SyncProtocol.stateCoherence.test.tstest/unit/domain/services/SyncProtocol.test.tstest/unit/domain/services/TreeConstruction.determinism.test.tstest/unit/domain/services/VisibleStateTransferPlanner.test.tstest/unit/domain/services/WarpMessageCodec.test.tstest/unit/domain/services/WarpMessageCodec.v3.test.tstest/unit/domain/services/WormholeService.test.tstest/unit/domain/services/audit/AuditChainVerifier.test.tstest/unit/domain/services/controllers/CheckpointController.snapshotCache.test.tstest/unit/domain/services/controllers/CheckpointController.test.tstest/unit/domain/services/controllers/ComparisonController.test.tstest/unit/domain/services/controllers/ForkController.test.tstest/unit/domain/services/controllers/IntentController.test.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tstest/unit/domain/services/controllers/MaterializePatchStreamReducer.test.tstest/unit/domain/services/controllers/PatchController.test.tstest/unit/domain/services/controllers/ProvenanceController.test.tstest/unit/domain/services/controllers/QueryContentProjectionReads.test.tstest/unit/domain/services/controllers/QueryController.test.tstest/unit/domain/services/controllers/StrandController.host-interface.test.tstest/unit/domain/services/optic/CheckpointFactResolver.test.tstest/unit/domain/services/optic/CheckpointPatchFactStream.test.tstest/unit/domain/services/optic/CheckpointShardFactReader.manifest.test.tstest/unit/domain/services/optic/CheckpointTailBasisVerifier.test.tstest/unit/domain/services/optic/CheckpointTailReadIdentityBuilder.test.tstest/unit/domain/services/optic/CoordinateCheckpointTailOpticSource.test.tstest/unit/domain/services/optic/TraversalOptic.test.tstest/unit/domain/services/optic/WorldlineOptic.test.tstest/unit/domain/services/pathKeyedTreeMaps.test.tstest/unit/domain/services/strand/StrandService.test.tstest/unit/domain/services/sync/SyncResponsePagingMetrics.test.tstest/unit/domain/storage/StoragePrimitives.test.tstest/unit/domain/strandAndRuntimeSeams.test.tstest/unit/domain/trust/TrustRecordService.test.tstest/unit/domain/warp/RuntimeHostPortResolvers.test.tstest/unit/domain/warp/Writer.test.tstest/unit/domain/warp/readPatchBlob.test.tstest/unit/infrastructure/CborIndexStoreAdapter.test.tstest/unit/infrastructure/adapters/CasBlobAdapter.test.tstest/unit/infrastructure/adapters/CasPayloadPointer.test.tstest/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.tstest/unit/infrastructure/adapters/CborPatchJournalAdapter.test.tstest/unit/infrastructure/adapters/GitCasAssetStorageAdapter.test.tstest/unit/infrastructure/adapters/GitCasAuditLogAdapter.test.tstest/unit/infrastructure/adapters/GitCasIntentStoreAdapter.test.tstest/unit/infrastructure/adapters/GitCasRepositoryAdapter.test.tstest/unit/infrastructure/adapters/GitCasStrandStoreAdapter.test.tstest/unit/infrastructure/adapters/GitGraphAdapter.coverage.test.tstest/unit/infrastructure/adapters/GitTrustChainAdapter.test.tstest/unit/infrastructure/adapters/GraphModelMigrationDryRunRequestJsonAdapter.test.tstest/unit/infrastructure/adapters/InMemoryBlobStorageAdapter.test.tstest/unit/infrastructure/adapters/InMemoryGraphAdapter.objectType.test.tstest/unit/infrastructure/adapters/TrailerCommitMessageCodecAdapter.test.tstest/unit/ports/BlobPort.test.tstest/unit/ports/BlobStoragePort.test.tstest/unit/ports/CheckpointStorePort.test.tstest/unit/ports/CommitPort.test.tstest/unit/ports/GraphPersistencePort.test.tstest/unit/ports/IndexStorePort.test.tstest/unit/ports/PatchJournalPort.test.tstest/unit/ports/TreePort.test.tstest/unit/ports/WasmVerifiedAdmissionPort.test.tstest/unit/scripts/checkpoint-schema-upgrade.test.tstest/unit/scripts/op-hydration-boundary-ratchet.test.tstest/unit/scripts/pre-push-hook.test.tstest/unit/scripts/source-size-inventory-command.test.tstest/unit/scripts/source-version-name-policy.test.tstest/unit/scripts/storage-ownership-boundary.test.tstest/unit/scripts/v16-to-v17-upgrade.test.tstest/unit/scripts/v18-content-property-closeout-audit.test.tstest/unit/scripts/v18-v17-public-read-legacy-reading-builder.test.ts
💤 Files with no reviewable changes (23)
- src/ports/BlobStoragePort.ts
- src/ports/IndexStoragePort.ts
- src/ports/BlobPort.ts
- src/domain/services/controllers/detachedOpen.ts
- test/conformance/fixtures/V17CheckpointBasisArtifactFixture.ts
- test/unit/domain/services/CheckpointService.anchors.test.ts
- src/ports/TreePort.ts
- src/domain/capabilities/PatchCollector.ts
- src/domain/graph/ContentAttachmentOid.ts
- test/unit/scripts/source-size-inventory-command.test.ts
- src/infrastructure/adapters/CasBlobAdapter.ts
- test/unit/domain/services/MessageCodecModules.test.ts
- src/infrastructure/adapters/CasPayloadPointer.ts
- src/ports/TreeEntryProbePort.ts
- src/domain/services/controllers/MaterializeStrategyRuntime.ts
- src/ports/CommitPort.ts
- src/domain/warp/RuntimePatchCollector.ts
- test/conformance/v17CheckpointTailOpticReadBasis.test.ts
- src/domain/services/controllers/SyncControllerTypes.ts
- test/unit/scripts/v18-content-property-closeout-audit.test.ts
- test/unit/domain/services/PatchBuilder.test.ts
- scripts/source-size-gate.ts
- test/conformance/fixtures/V17CheckpointPayloadBlobFixture.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (40)
src/domain/api/Evidence.ts (1)
13-25: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Model retention evidence as one validated runtime-backed concept.
The shape-only declaration and permissive freezer jointly allow invalid retention claims to enter otherwise frozen evidence.
src/domain/api/Evidence.ts#L13-L25: replace the ad hoc exported shape with a frozen, validated runtime-backed domain object.src/domain/api/EvidenceRuntime.ts#L197-L235: construct that object and validatepolicy,reachability, androotKindinstead of copying arbitrary values.As per coding guidelines, domain objects must use validated constructors,
Object.freeze, andinstanceofdispatch rather than ad hoc shape bags.📍 Affects 2 files
src/domain/api/Evidence.ts#L13-L25(this comment)src/domain/api/EvidenceRuntime.ts#L197-L235🤖 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 `@src/domain/api/Evidence.ts` around lines 13 - 25, Replace the exported RetentionEvidence shape in src/domain/api/Evidence.ts, lines 13-25, with a frozen validated runtime-backed domain object using the established constructor pattern and instanceof dispatch. Update src/domain/api/EvidenceRuntime.ts, lines 197-235, to construct that object through its validated constructor, validating policy, reachability, and rootKind rather than copying arbitrary values; retain Evidence’s public API behavior while preventing invalid retention claims.Source: Coding guidelines
src/domain/capabilities/QueryCapability.ts (1)
111-113: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale JSDoc still references "OID" after the handle rename.
Lines 111 and 120 still say
"Return the content blob OID attached to..."even though the methods were renamed togetContentHandle/getEdgeContentHandle. Update the comments to describe the opaque handle rather than an OID, to avoid confusing consumers of this abstract contract about the new storage semantics.📝 Suggested doc fix
- /** Return the content blob OID attached to a node, if any. */ + /** Return the content asset handle attached to a node, if any. */ abstract getContentHandle(_nodeId: string): Promise<string | null>; ... - /** Return the content blob OID attached to an edge, if any. */ + /** Return the content asset handle attached to an edge, if any. */ abstract getEdgeContentHandle(_from: string, _to: string, _label: string): Promise<string | null>;Also applies to: 120-122
🤖 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 `@src/domain/capabilities/QueryCapability.ts` around lines 111 - 113, Update the JSDoc comments above getContentHandle and getEdgeContentHandle to refer to the attached content blob handle, not an OID. Keep the method signatures and behavior unchanged while consistently describing the handle as opaque.src/domain/services/audit/AuditChainVerifier.ts (1)
167-170: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Audit storage failures must not produce successful verification.
The verifier currently treats
readHead()failure as a valid empty chain, and the new test locks in that false-positive behavior.
src/domain/services/audit/AuditChainVerifier.ts#L167-L170: return an explicitERRORresult with anAUDIT_HEAD_UNAVAILABLEdiagnostic.test/unit/domain/services/audit/AuditChainVerifier.test.ts#L163-L172: expect that error while retaining the valid-empty result only for an actualnullhead.📍 Affects 2 files
src/domain/services/audit/AuditChainVerifier.ts#L167-L170(this comment)test/unit/domain/services/audit/AuditChainVerifier.test.ts#L163-L172🤖 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 `@src/domain/services/audit/AuditChainVerifier.ts` around lines 167 - 170, The readHead failure path in AuditChainVerifier must return an explicit ERROR result with an AUDIT_HEAD_UNAVAILABLE diagnostic instead of returning the successful empty-chain result; preserve the valid-empty result only when readHead actually returns null. Update src/domain/services/audit/AuditChainVerifier.ts lines 167-170 and adjust test/unit/domain/services/audit/AuditChainVerifier.test.ts lines 163-172 to assert the error behavior.Source: Coding guidelines
src/domain/services/audit/AuditReceiptService.ts (2)
297-297: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the documented return contract.
The JSDoc still promises a SHA, but
commit()now returns aPublishedAuditRecordcarrying retention evidence.Proposed correction
- * `@returns` The audit commit SHA, or null on failure + * `@returns` The published audit record and retention evidence, or null on failure📝 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.* `@returns` The published audit record and retention evidence, or null on failure🤖 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 `@src/domain/services/audit/AuditReceiptService.ts` at line 297, Update the JSDoc for AuditReceiptService.commit to document that it returns a PublishedAuditRecord or null, replacing the outdated SHA return description. Keep the method signature and implementation unchanged.
392-405: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail printf 'Repo root: '; pwd printf '\n--- Files of interest ---\n' git ls-files | rg 'src/(domain|ports|infrastructure).*(AuditReceiptService|AuditLogPort|WarpError|errorCode|publication|conflict)' printf '\n--- Search symbols ---\n' rg -n "errorCode\\(|PUBLICATION_CONFLICT|class WarpError|extends WarpError|AuditLogPort|publication conflict|publication-conflict|publicationConflict" srcRepository: git-stunts/git-warp
Length of output: 8420
🏁 Script executed:
echo waitingRepository: git-stunts/git-warp
Length of output: 165
🏁 Script executed:
set -euo pipefail rg -n "errorCode\\(|PUBLICATION_CONFLICT|class WarpError|extends WarpError|AuditLogPort|publicationConflict|publication conflict" src || trueRepository: git-stunts/git-warp
Length of output: 7378
🏁 Script executed:
set -euo pipefail printf 'FILES\n' git ls-files | rg 'src/domain/services/audit/AuditReceiptService.ts|src/.*/AuditLogPort|WarpError' printf '\nMATCHES\n' rg -n "errorCode\\(|PUBLICATION_CONFLICT|extends WarpError|class WarpError|AuditLogPort" src/domain src/ports src/infrastructureRepository: git-stunts/git-warp
Length of output: 7614
🏁 Script executed:
set -euo pipefail printf '\n--- AuditLogPort outline ---\n' ast-grep outline src/ports/AuditLogPort.ts --view expanded || true printf '\n--- WarpError outline ---\n' ast-grep outline src/domain/errors/WarpError.ts --view expanded || true printf '\n--- AuditReceiptService outline ---\n' ast-grep outline src/domain/services/audit/AuditReceiptService.ts --view expanded || true printf '\n--- GitCasAuditLogAdapter outline ---\n' ast-grep outline src/infrastructure/adapters/GitCasAuditLogAdapter.ts --view expanded || trueRepository: git-stunts/git-warp
Length of output: 4683
🏁 Script executed:
set -euo pipefail for f in \ src/ports/AuditLogPort.ts \ src/domain/errors/WarpError.ts \ src/domain/services/audit/AuditReceiptService.ts \ src/infrastructure/adapters/GitCasAuditLogAdapter.ts do printf '\n===== %s (%s lines) =====\n' "$f" "$(wc -l < "$f")" nl -ba "$f" | sed -n '1,260p' doneRepository: git-stunts/git-warp
Length of output: 247
🏁 Script executed:
set -euo pipefail printf '\n===== src/ports/AuditLogPort.ts =====\n' nl -ba src/ports/AuditLogPort.ts | sed -n '1,260p' printf '\n===== src/domain/errors/WarpError.ts =====\n' nl -ba src/domain/errors/WarpError.ts | sed -n '1,220p' printf '\n===== src/domain/services/audit/AuditReceiptService.ts =====\n' nl -ba src/domain/services/audit/AuditReceiptService.ts | sed -n '360,470p' printf '\n===== src/infrastructure/adapters/GitCasAuditLogAdapter.ts =====\n' nl -ba src/infrastructure/adapters/GitCasAuditLogAdapter.ts | sed -n '1,260p'Repository: git-stunts/git-warp
Length of output: 236
🏁 Script executed:
set -euo pipefail rg -n "PUBLICATION_CONFLICT|MANIFEST_NOT_FOUND|errorCode\\(|throw new|class .*Error|extends WarpError|publish|append|write" src/infrastructure/adapters/GitCasAuditLogAdapter.ts nl -ba src/infrastructure/adapters/GitCasAuditLogAdapter.ts | sed -n '1,240p'Repository: git-stunts/git-warp
Length of output: 793
🏁 Script executed:
set -euo pipefail printf '\n===== src/ports/AuditLogPort.ts =====\n' nl -ba src/ports/AuditLogPort.ts | sed -n '1,260p' printf '\n===== publication-conflict related declarations =====\n' rg -n "PUBLICATION_CONFLICT|publication conflict|publicationConflict|AuditLogPort|PublishedAuditRecord|errorCode\\(" src/ports src/domain src/infrastructureRepository: git-stunts/git-warp
Length of output: 236
🏁 Script executed:
python3 - <<'PY' from pathlib import Path files = [ "src/ports/AuditLogPort.ts", "src/domain/errors/WarpError.ts", "src/domain/services/audit/AuditReceiptService.ts", "src/infrastructure/adapters/GitCasAuditLogAdapter.ts", ] for f in files: print(f"\n===== {f} =====") lines = Path(f).read_text().splitlines() for i, line in enumerate(lines, 1): if f.endswith("AuditLogPort.ts") or f.endswith("WarpError.ts"): # small files, print all print(f"{i:4}: {line}") elif f.endswith("AuditReceiptService.ts"): if 360 <= i <= 470: print(f"{i:4}: {line}") elif f.endswith("GitCasAuditLogAdapter.ts"): if 55 <= i <= 170: print(f"{i:4}: {line}") PYRepository: git-stunts/git-warp
Length of output: 10483
🏁 Script executed:
echo waitingRepository: git-stunts/git-warp
Length of output: 165
Use a typed publication-conflict signal at the
AuditLogPortboundary.
AuditReceiptServicestill branches onerrorCode(error), so any thrown object with a stringcodecan be treated as a conflict. Add a dedicatedPublicationConflictError/result type toAuditLogPort.appendand have the adapters emit that contract instead of string matching.🤖 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 `@src/domain/services/audit/AuditReceiptService.ts` around lines 392 - 405, Replace the `errorCode(error) === 'PUBLICATION_CONFLICT'` check in `AuditReceiptService` with the typed publication-conflict contract exposed by `AuditLogPort.append`. Define or reuse a dedicated `PublicationConflictError` or result type at the port boundary, update adapters to emit it, and branch on that type/result while preserving the existing retry-once and degradation behavior.Source: Coding guidelines
src/domain/services/controllers/CheckpointController.ts (1)
266-267: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not classify checkpoint failures by message substring.
The surrounding catch converts any error containing “missing” or “not found” into “no checkpoint.” With the semantic store now owning decoding and integrity checks, a current-format corruption error can be silently discarded. Only a typed missing-object error should return
null; all integrity and decoding failures must propagate.🤖 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 `@src/domain/services/controllers/CheckpointController.ts` around lines 266 - 267, Update the checkpoint-loading error handling around loadCheckpoint in CheckpointController so only the store’s typed missing-object error is converted to null. Remove message-substring checks for “missing” or “not found,” and rethrow all decoding, integrity, and other failures unchanged.src/domain/services/controllers/IntentController.ts (1)
88-107: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Find callers of getWriterIntents and queueIntent to confirm whether writerId/strandId are conflated. rg -nP -C4 '\.getWriterIntents\s*\(' --type=ts rg -nP -C4 '\.queueIntent\s*\(' --type=ts src/domain rg -nP -C4 'getWriterIntents' test/unit/domain/services/controllers/IntentController.test.ts 2>/dev/nullRepository: git-stunts/git-warp
Length of output: 157
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the relevant controller and store files. git ls-files 'src/domain/services/controllers/IntentController.ts' 'src/**/IntentStore*' 'src/**/intent*' 'src/**/Intent*' | sed 's#^`#FILE` #' echo echo '--- outline: IntentController.ts ---' ast-grep outline src/domain/services/controllers/IntentController.ts --view expanded || true echo echo '--- locate buildIntentRef and intent store refs ---' rg -n "buildIntentRef|scan\\(|publish\\(" src/domain src/ports src/infrastructure --glob '!**/*.test.*' || true echo echo '--- callers of getWriterIntents / queueIntent ---' rg -n "getWriterIntents\\s*\\(" src test --glob '!**/*.test.*' || true rg -n "queueIntent\\s*\\(" src test --glob '!**/*.test.*' || trueRepository: git-stunts/git-warp
Length of output: 4185
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the controller implementation around the relevant methods. sed -n '1,180p' src/domain/services/controllers/IntentController.ts echo echo '--- possible store implementation files ---' fd -a 'IntentStore' src fd -a 'intent' src | sed -n '1,120p'Repository: git-stunts/git-warp
Length of output: 4557
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the controller implementation around the relevant methods. sed -n '1,180p' src/domain/services/controllers/IntentController.ts echo echo '--- possible store implementation files ---' fd -a 'IntentStore' src || true fd -a 'intent' src | sed -n '1,120p' || trueRepository: git-stunts/git-warp
Length of output: 4557
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path p = Path('src/domain/services/controllers/IntentController.ts') print(p.exists(), p) PYRepository: git-stunts/git-warp
Length of output: 214
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- IntentStorePort.ts ---' sed -n '1,220p' src/ports/IntentStorePort.ts echo echo '--- GitCasIntentStoreAdapter.ts ---' sed -n '1,220p' src/infrastructure/adapters/GitCasIntentStoreAdapter.ts echo echo '--- IntentCapability.ts ---' sed -n '1,220p' src/domain/capabilities/IntentCapability.ts echo echo '--- StrandIntentService.ts ---' sed -n '1,220p' src/domain/services/strand/StrandIntentService.ts echo echo '--- all getWriterIntents references ---' rg -n "getWriterIntents" src test --glob '!**/*.test.*' || trueRepository: git-stunts/git-warp
Length of output: 17301
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- buildIntentRef in RefLayout.ts ---' sed -n '300,360p' src/domain/utils/RefLayout.ts echo echo '--- search for queued/admitted intent semantics ---' rg -n "queued|admitted|writerId|strandId|ownerId" src/domain src/infrastructure src/ports --glob '!**/*.test.*' | sed -n '1,220p'Repository: git-stunts/git-warp
Length of output: 22588
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- writerId / strandId usage context ---' rg -n "strandId|writerId|_writerId|writer" src/domain src/infrastructure src/ports --glob '!**/*.test.*' | sed -n '1,260p' echo echo '--- IntentRuntime / RuntimeHost wrappers ---' sed -n '600,680p' src/domain/RuntimeHost.tsRepository: git-stunts/git-warp
Length of output: 33842
Queued intents are indexed by strand, but read back by writer id.
queueIntentpublishes underownerId: strandId, whilegetWriterIntentsscansqueuedwithwriterId; that misses queued intents whenever the two ids differ. Use the same key on both paths, or rename the API to match the stored owner.🤖 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 `@src/domain/services/controllers/IntentController.ts` around lines 88 - 107, Align the queued-intent lookup with the storage key used by queueIntent: update getWriterIntents to scan the queued channel using the strand identifier stored as ownerId, or consistently rename the API and parameter to represent that identifier. Ensure both publish and read paths use the same key.src/domain/services/controllers/PatchController.ts (1)
78-80: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve the concrete audit publication result type.
Promise<unknown>erases the retention-evidence contract at this domain boundary. Use the actual runtime-backed result type returned by the audit port/service.As per coding guidelines, “Do not introduce
unknown(outside adapters) ... in core code; use validated boundary models and ports instead.”🤖 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 `@src/domain/services/controllers/PatchController.ts` around lines 78 - 80, Update the _auditService.commit type in PatchController to return the concrete runtime-backed audit publication result type used by the audit port/service instead of Promise<unknown>. Reuse the existing result type definition from that port or service and preserve the commit method’s Promise-based contract.Source: Coding guidelines
src/domain/services/index/LogicalIndexReader.ts (1)
84-101: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Preserve the current index when shard decoding fails.
Line 87 clears the reader before
Promise.allcompletes. If anydecodeShard()rejects, the failed reload also destroys the previously loaded index.Proposed fix
- this._resetState(); if (this._indexStore === null) { throw new IndexError( 'LogicalIndexReader: loadFromHandles() requires an indexStore', { code: 'E_INDEX_NO_STORE' }, ); } @@ })), ); + this._resetState(); this._processDecoded(decoded);📝 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.async loadFromHandles( shardHandles: Readonly<Record<string, AssetHandle>>, ): Promise<this> { if (this._indexStore === null) { throw new IndexError( 'LogicalIndexReader: loadFromHandles() requires an indexStore', { code: 'E_INDEX_NO_STORE' }, ); } const indexStore = this._indexStore; const decoded: DecodedItem[] = await Promise.all( Object.entries(shardHandles).map(async ([path, handle]) => ({ path, data: await indexStore.decodeShard(handle), })), ); this._resetState(); this._processDecoded(decoded);🤖 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 `@src/domain/services/index/LogicalIndexReader.ts` around lines 84 - 101, Update loadFromHandles so _resetState() runs only after all shard handles have decoded successfully; perform decoding first, then reset and process the decoded items. Preserve the existing index when indexStore.decodeShard() rejects, while retaining the current successful reload behavior.src/domain/services/optic/CheckpointShardFactReader.ts (1)
63-74: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail closed when manifest roots and handle records disagree.
These paths treat a one-sided missing entry as “no fact” and never verify
handle.toString() === oid. A corrupt basis can therefore hide live nodes/properties or read a different shard while reporting the manifest identity.Return the absent fallback only when both entries are absent; otherwise raise a bounded-basis integrity failure. Add missing-entry and mismatched-handle tests.
Also applies to: 84-98
🤖 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 `@src/domain/services/optic/CheckpointShardFactReader.ts` around lines 63 - 74, Update the checkpoint shard read paths around the liveness-root and index-handle lookups to return the absent fallback only when both entries are missing. Treat one-sided presence and any handle whose string value differs from oid as a bounded-basis integrity failure before constructing LogicalIndexReader, while preserving the existing read-failure handling. Add tests covering missing-entry and mismatched-handle cases for both affected paths.src/domain/services/optic/CheckpointTailBasisVerifier.ts (2)
41-43: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Verify that index shard handles are actually readable.
A non-empty handle map does not establish a usable bounded basis. Dangling or unretained shard handles currently pass
verify()and fail only during the subsequent optic read. Validate every referenced shard through_indexStorebefore returning success.Also applies to: 55-61
🤖 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 `@src/domain/services/optic/CheckpointTailBasisVerifier.ts` around lines 41 - 43, Update CheckpointTailBasisVerifier.verify around requireIndexShards to validate every referenced index shard handle through source._indexStore before returning success. Ensure dangling or unretained handles cause verification to fail, while preserving the existing schema and shard-handle checks.
64-68: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the explicit
unknownboundary from domain code.Inline the
QueryErrornarrowing in thecatchblock or route failures through a validated domain error model.Proposed simplification
} catch (error) { - rethrowBasisFailure(source.graphName, error); + if (error instanceof QueryError && error.code === 'E_OPTIC_NO_BOUNDED_BASIS') { + throw error; + } + throwNoBoundedBasis(source.graphName, 'checkpoint-basis-unavailable'); } } - -function rethrowBasisFailure(graphName: string, error: unknown): never { - if (error instanceof QueryError && error.code === 'E_OPTIC_NO_BOUNDED_BASIS') { - throw error; - } - throwNoBoundedBasis(graphName, 'checkpoint-basis-unavailable'); -}As per coding guidelines,
src/**core code must not introduceunknown; use validated boundary models instead.📝 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.Source: Coding guidelines
src/domain/services/PatchBuilder.ts (1)
289-297: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Block commits while attachment staging is in flight.
Both methods validate only before awaiting
assetStorage.stage(). A concurrentcommit()can publish existing operations and mark the builder committed; the resumed attachment then mutates_opsand_contentAssets, leaving its content absent from the commit and its staged asset unretained.Track pending attachment operations and make
commitWithEvidence()reject or await them, decrementing the counter infinally.Also applies to: 327-335
🤖 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 `@src/domain/services/PatchBuilder.ts` around lines 289 - 297, Track pending attachment staging across both attachment-writing methods that await storeContentAttachmentPayload: increment the pending-operation counter before the await and decrement it in a finally block. Update commitWithEvidence() to reject or await while pending attachment operations remain, preventing commit completion before resumed attachments are recorded.src/domain/services/PatchBuilderContent.ts (1)
12-16: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Keep
ReadableStreamout of domain contracts.Core should accept boundary-neutral
AsyncIterable<Uint8Array>and convert host streams before entering the domain.
src/domain/services/PatchBuilderContent.ts#L12-L16: removeReadableStreamfromContentInput.src/domain/utils/streamUtils.ts#L33-L40: moveReadableStreamdetection/conversion into an adapter or controller boundary.As per coding guidelines, “
src/domain/must not import host APIs or Node-specific globals; hexagonal architecture boundaries are mandatory.”📍 Affects 2 files
src/domain/services/PatchBuilderContent.ts#L12-L16(this comment)src/domain/utils/streamUtils.ts#L33-L40🤖 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 `@src/domain/services/PatchBuilderContent.ts` around lines 12 - 16, The domain ContentInput contract must remain host-neutral: remove ReadableStream from ContentInput in src/domain/services/PatchBuilderContent.ts (lines 12-16), keeping AsyncIterable<Uint8Array>, Uint8Array, and string. Move ReadableStream detection and conversion out of src/domain/utils/streamUtils.ts (lines 33-40) into the adapter or controller boundary so domain code receives an AsyncIterable before invoking PatchBuilderContent.Source: Coding guidelines
src/domain/services/PatchCommitter.ts (3)
45-45: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Model patch publication evidence as a dedicated domain object.
The public result currently originates as a service-local shape alias, coupling the capability contract to
PatchCommitter.
src/domain/services/PatchCommitter.ts#L45-L45: replace the intersection alias with a validated, frozen domain class in its own file.src/domain/capabilities/PatchCapability.ts#L11-L11: import that domain concept instead of importing from a service.src/domain/capabilities/PatchCapability.ts#L50-L53: return the dedicated publication-result concept.As per coding guidelines, “domain objects should be runtime-backed nouns, not ad hoc shape bags” and “prefer one file per class, type, or object.”
📍 Affects 2 files
src/domain/services/PatchCommitter.ts#L45-L45(this comment)src/domain/capabilities/PatchCapability.ts#L11-L11src/domain/capabilities/PatchCapability.ts#L50-L53🤖 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 `@src/domain/services/PatchCommitter.ts` at line 45, The patch publication result is currently a service-local shape alias rather than a validated domain object. In src/domain/services/PatchCommitter.ts:45, replace PatchCommitResult with a dedicated frozen domain class in its own file; update src/domain/capabilities/PatchCapability.ts:11 to import that class from its domain module, and src/domain/capabilities/PatchCapability.ts:50-53 to return the dedicated publication-result object.Source: Coding guidelines
47-52: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the obsolete commit-flow documentation.
The comment still describes direct blob, tree, commit, and ref writes, while the implementation now delegates publication to
PatchJournalPort.🤖 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 `@src/domain/services/PatchCommitter.ts` around lines 47 - 52, Update the class-level commit-flow documentation for PatchCommitter to reflect that publication is delegated to PatchJournalPort rather than directly persisting blobs, trees, commits, and refs. Preserve the documented CAS check, Lamport resolution, patch construction, and callback steps where they still match the implementation.
151-157: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Preserve non-conflict publication failures.
Every append failure currently performs
readRef(). A generic storage outage can therefore be reclassified asWRITER_CAS_CONFLICTwhen the head changed, or masked if this diagnostic read also fails. Only inspect the head for an explicitPUBLICATION_CONFLICT.Proposed fix
async function rethrowPublicationConflict( persistence: WarpKernelPort, writerRef: string, expectedSha: string | null, error: unknown, ): Promise<never> { + if (errorCode(error) !== 'PUBLICATION_CONFLICT') { + throw error; + } const actualSha = await persistence.readRef(writerRef); - if (actualSha !== expectedSha || errorCode(error) === 'PUBLICATION_CONFLICT') { - throw buildWriterCasConflict(expectedSha, actualSha); - } - throw error; + throw buildWriterCasConflict(expectedSha, actualSha); }Also applies to: 173-183
🤖 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 `@src/domain/services/PatchCommitter.ts` around lines 151 - 157, Update the append error handling in PatchCommitter’s catch paths, including rethrowPublicationConflict, so head inspection via readRef occurs only for an explicit PUBLICATION_CONFLICT; rethrow all other publication or storage failures unchanged without diagnostic reads, preserving their original errors.src/domain/services/state/checkpointLoad.ts (1)
53-60: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Restore schema validation at the central checkpoint-loading boundary.
loadCheckpointnow accepts every schema returned by the store. Callers such asmaterializeAt(checkpointSha)can therefore load a retired checkpoint that is not the validated graph head. Reject non-current schemas here so every loading path enforces the migration boundary.🤖 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 `@src/domain/services/state/checkpointLoad.ts` around lines 53 - 60, Update loadCheckpoint at the checkpointStore.loadCheckpoint boundary to validate checkpoint.schema against the current supported schema before constructing LoadedCheckpoint. Reject non-current or retired schemas, while preserving loading for the current schema so all callers, including materializeAt, enforce the migration boundary.src/domain/services/state/StateReaderContext.ts (1)
23-23: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the canonical
ContentMetatype.Import or re-export
src/domain/types/ContentMeta.tsinstead of maintaining a second structural definition here. Otherwise these two content contracts can silently diverge.As per coding guidelines, “Prefer one file per class, type, or object; if a file accumulates peer concepts, split it.”
🤖 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 `@src/domain/services/state/StateReaderContext.ts` at line 23, Replace the locally defined ContentMeta type in StateReaderContext.ts with the canonical ContentMeta from src/domain/types/ContentMeta.ts, importing or re-exporting it as appropriate and removing the duplicate structural definition. Keep existing consumers using the canonical contract.Source: Coding guidelines
src/domain/services/strand/StrandDescriptorStore.ts (1)
201-211: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
writeDescriptorskips the strand-id validation applied elsewhere.
readDescriptor,hasDescriptor, anddeleteDescriptorall callbuildRef(strandId)(which runsvalidateWriterId) before delegating to_strandStore.writeDescriptorpublishesdescriptor.strandIddirectly with no equivalent validation, so an invalid strand id can be persisted here but rejected everywhere else.♻️ Proposed fix
async writeDescriptor(descriptor: StrandDescriptor): Promise<void> { + this.buildRef(descriptor.strandId); const attachments = descriptor.intentQueue.intents.flatMap((intent) => intent.contentAssetHandles.map((handle) => new AssetHandle(handle)) );📝 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.async writeDescriptor(descriptor: StrandDescriptor): Promise<void> { this.buildRef(descriptor.strandId); const attachments = descriptor.intentQueue.intents.flatMap((intent) => intent.contentAssetHandles.map((handle) => new AssetHandle(handle)) ); await this._graph._strandStore.publishDescriptor({ graphName: this._graph._graphName, strandId: descriptor.strandId, descriptor: textEncode(JSON.stringify(descriptor)), // nosemgrep: ts-no-json-stringify-in-core -- 0025B attachments, }); }🤖 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 `@src/domain/services/strand/StrandDescriptorStore.ts` around lines 201 - 211, Update writeDescriptor to validate descriptor.strandId through the same buildRef path used by readDescriptor, hasDescriptor, and deleteDescriptor before publishing to _strandStore. Preserve the existing descriptor serialization and attachments, and use the validated strand reference/id for the publish request.src/domain/services/strand/StrandPatchService.ts (1)
194-223: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep opaque asset handles runtime-backed throughout queued intents.
The core flow converts
AssetHandlevalues to raw strings and reconstructs them only during commitment, weakening validation and contradicting the migration objective.
src/domain/services/strand/StrandPatchService.ts#L194-L223: accept and forwardreadonly AssetHandle[]; serialize only at the intent-store codec boundary.test/unit/domain/services/strand/StrandService.test.ts#L606-L619: assertAssetHandleinstances rather than serialized strings.As per coding guidelines, domain objects must be runtime-backed nouns and runtime-honest TypeScript must reflect actual behavior.
📍 Affects 2 files
src/domain/services/strand/StrandPatchService.ts#L194-L223(this comment)test/unit/domain/services/strand/StrandService.test.ts#L606-L619🤖 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 `@src/domain/services/strand/StrandPatchService.ts` around lines 194 - 223, Keep asset handles runtime-backed through the queued patch flow: update the relevant StrandPatchService method to accept and forward readonly AssetHandle[] directly, removing string conversion and reconstruction; serialization must remain only at the intent-store codec boundary. In test/unit/domain/services/strand/StrandService.test.ts lines 606-619, update assertions to expect AssetHandle instances rather than serialized strings.Source: Coding guidelines
src/domain/services/sync/syncPatchLoader.ts (1)
92-96: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make
PatchJournalPortmandatory across patch-loading and sync APIs.Persistence no longer has blob capability, but these public contracts still allow the only payload reader to be omitted.
src/domain/services/sync/syncPatchLoader.ts#L92-L96: requirepatchJournalforloadPatchFromCommit.src/domain/services/sync/syncPatchLoader.ts#L148-L154: require and forward it forloadPatchRange.src/domain/services/sync/syncRequestResponse.ts#L201-L207: require it whenever sync can return patches.📍 Affects 2 files
src/domain/services/sync/syncPatchLoader.ts#L92-L96(this comment)src/domain/services/sync/syncPatchLoader.ts#L148-L154src/domain/services/sync/syncRequestResponse.ts#L201-L207🤖 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 `@src/domain/services/sync/syncPatchLoader.ts` around lines 92 - 96, Make PatchJournalPort mandatory across the patch-loading and sync APIs: update loadPatchFromCommit and loadPatchRange in src/domain/services/sync/syncPatchLoader.ts to require and forward patchJournal, and update the sync response contract in src/domain/services/sync/syncRequestResponse.ts to require it whenever patches may be returned.src/domain/services/sync/syncRequestResponse.ts (1)
232-238: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Model ancestry checks as an explicit port capability.
The intersection assertion fabricates an undeclared optional capability. Inject a named ancestry port or add the capability to the applicable persistence contract instead.
As per coding guidelines, “Keep helper corridors, fake shape trust, transitional duplication, and compile-time theater out of the codebase; runtime-honest TypeScript must reflect actual behavior.”
🤖 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 `@src/domain/services/sync/syncRequestResponse.ts` around lines 232 - 238, The ancestry check in the sync request/response flow should use an explicitly declared port capability rather than casting persistence to an ad hoc optional intersection. Update the applicable persistence contract or inject a named ancestry port exposing isAncestor, then call that declared capability from the pre-check while preserving the existing divergence behavior.Source: Coding guidelines
src/domain/trust/TrustRecordService.ts (2)
23-30: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Make
TrustRetryTipa runtime-backed domain object.The exported readonly shape has no validated constructor or runtime identity. Move it to
TrustRetryTip.tsas a frozen class and pass class instances toresign.As per coding guidelines, domain concepts must use validated constructors,
Object.freeze, andinstanceof, and each class or type should preferably have its own file.🤖 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 `@src/domain/trust/TrustRecordService.ts` around lines 23 - 30, Replace the exported TrustRetryTip type in TrustRecordService with a dedicated frozen TrustRetryTip class in TrustRetryTip.ts, including a validated constructor and runtime identity via instanceof. Update RetryOptions and the retry/resign flow to accept and pass TrustRetryTip instances while preserving the existing tipSha and recordId values.Source: Coding guidelines
100-102: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Narrow to
TrustErrorbefore calling the classifier.This removes the newly introduced explicit
unknownfrom domain code while preserving runtimeinstanceofdispatch.Proposed correction
- if (!isRetryableConflict(err, resign)) { + if (!(err instanceof TrustError) || !isRetryableConflict(err, resign)) { throw err; } function isRetryableConflict( - error: unknown, + error: TrustError, resign: RetryOptions['resign'], -): error is TrustError { - return error instanceof TrustError - && (error.code === 'E_TRUST_CAS_CONFLICT' +): boolean { + return error.code === 'E_TRUST_CAS_CONFLICT' || (error.code === 'E_TRUST_PREV_MISMATCH' && resign !== null)); }As per coding guidelines,
src/**must not introduceunknownoutside adapters.Also applies to: 147-153
🤖 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 `@src/domain/trust/TrustRecordService.ts` around lines 100 - 102, Update the error handling in the TrustRecordService retry paths around isRetryableConflict to narrow caught errors to TrustError before invoking the classifier, while preserving runtime instanceof dispatch and rethrowing non-retryable errors. Remove the explicit unknown type introduced in the domain code, including the additional occurrence noted in the comment.Source: Coding guidelines
src/domain/types/CoordinateComparison.ts (1)
174-174: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve the opaque handle type in transfer operations.
Typing
contentHandleasstringpermits unvalidated values and loses the runtime-backed storage identity introduced by this migration. UseContentAttachmentHandle(or the canonical storage-handle noun) and stringify only in an adapter.As per coding guidelines, domain objects must use explicit concepts with validated constructors and remain runtime-backed nouns rather than ad hoc shape bags.
Also applies to: 176-176
🤖 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 `@src/domain/types/CoordinateComparison.ts` at line 174, Update the attach_node_content transfer operation in CoordinateComparison to type contentHandle as the validated ContentAttachmentHandle (or canonical storage-handle type) rather than string, including the corresponding definition at the adjacent operation. Keep string conversion confined to the adapter boundary.Source: Coding guidelines
src/domain/utils/RefLayout.ts (1)
315-322: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the intent channel at runtime.
The union is compile-time-only. Invalid JS or cast input currently creates arbitrary
intents/<channel>refs that normal admitted/queued readers cannot discover. Reject unsupported channels withWarpError.As per coding guidelines, runtime-honest TypeScript must reflect actual behavior.
🤖 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 `@src/domain/utils/RefLayout.ts` around lines 315 - 322, Update buildIntentRef to validate the channel argument at runtime, accepting only "admitted" or "queued" and throwing WarpError for unsupported values before constructing the reference. Preserve the existing graphName and ownerId validation and reference format.Source: Coding guidelines
src/domain/WarpCore.ts (1)
45-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Stale "OID" wording in docstrings for renamed handle accessors.
The JSDoc above
getContentHandle(Line 46) andgetEdgeContentHandle(Line 58) still say "content blob OID," but the whole point of this PR is that these are now opaque handles, not raw Git OIDs. Leaving this wording risks callers/maintainers treating the returned string as a parsable OID again.📝 Proposed docstring fix
/** - * Returns the content blob OID attached to a node. + * Returns the opaque content handle attached to a node. */ declare readonly getContentHandle: WarpCoreRuntimeSurface['getContentHandle'];/** - * Returns the content blob OID attached to an edge. + * Returns the opaque content handle attached to an edge. */ declare readonly getEdgeContentHandle: WarpCoreRuntimeSurface['getEdgeContentHandle'];Also applies to: 57-60
🤖 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 `@src/domain/WarpCore.ts` around lines 45 - 48, Update the JSDoc descriptions for getContentHandle and getEdgeContentHandle to describe their return values as opaque content handles rather than content blob OIDs, without changing the accessor declarations or runtime behavior.src/domain/WarpWorldline.ts (1)
112-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Consider extracting the repeated "require optional port or throw" guard.
commitWithEvidenceandpatchDraftWithEvidencerepeat the same null-check-and-throw shape already used bycommit/createDraft/patchDraft/previewDraftJoin/prepareOpticBasis. A small private helper (e.g.requirePort(port, message, code)) would remove this duplication, which this diff extends to a sixth occurrence.♻️ Proposed helper
+ private requirePort<T>(port: T | null, message: string, code: string): T { + if (port === null) { + throw new WarpError(message, code); + } + return port; + } + async commitWithEvidence(build: WarpWorldlinePatchBuild): Promise<PatchCommitResult> { - if (this._commitPatchWithEvidence === null) { - throw new WarpError( - 'WarpWorldline was not opened with storage evidence support', - 'E_WARP_WORLDLINE_STORAGE_EVIDENCE_UNAVAILABLE', - ); - } - return await this._commitPatchWithEvidence(build); + const commitPatchWithEvidence = this.requirePort( + this._commitPatchWithEvidence, + 'WarpWorldline was not opened with storage evidence support', + 'E_WARP_WORLDLINE_STORAGE_EVIDENCE_UNAVAILABLE', + ); + return await commitPatchWithEvidence(build); }Also applies to: 140-152
🤖 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 `@src/domain/WarpWorldline.ts` around lines 112 - 121, Extract the repeated null-check-and-throw logic from commitWithEvidence and patchDraftWithEvidence into a private helper such as requirePort, parameterized by the optional port, error message, and error code. Update these methods and the existing commit, createDraft, patchDraft, previewDraftJoin, and prepareOpticBasis guards to use the helper while preserving each method’s current error details and behavior.src/infrastructure/adapters/GraphModelMigrationDryRunRequestJsonAdapter.ts (1)
147-155: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not downgrade a present
contentHandle: nulltocontentOid.At Line [148],
??treats an explicitly suppliednullhandle as absent, so malformed current-format input can bypass validation when a legacy OID is also present. SelectcontentOidonly whencontentHandleis absent, and add a regression test for this combination.Proposed fix
- const contentHandle = content['contentHandle'] ?? content['contentOid']; + const contentHandle = Object.prototype.hasOwnProperty.call(content, 'contentHandle') + ? content['contentHandle'] + : content['contentOid'];📝 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.rejectUnknownKeys(content, ['legacyContentKey', 'contentHandle', 'contentOid'], label); const contentHandle = Object.prototype.hasOwnProperty.call(content, 'contentHandle') ? content['contentHandle'] : content['contentOid']; return new GraphModelMigrationContentSource({ legacyContentKey: readRequiredString(content, `${label}.legacyContentKey`, 'legacyContentKey'), contentHandle: readRequiredString( { contentHandle }, `${label}.contentHandle`, 'contentHandle', ),🤖 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 `@src/infrastructure/adapters/GraphModelMigrationDryRunRequestJsonAdapter.ts` around lines 147 - 155, Update the contentHandle selection in the GraphModelMigrationContentSource construction to fall back to contentOid only when the contentHandle key is absent, not when it is explicitly null; preserve null so readRequiredString validates and rejects it, and add a regression test covering contentHandle: null together with a valid contentOid.test/conformance/fixtures/V17CheckpointShardIndexFixture.ts (1)
33-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Stale "Oid" naming left over from the handle rename. Both fixtures now type/return
AssetHandlebut keep pre-rename "Oid" identifiers, which is misleading in a PR specifically about replacing OID identifiers with opaque handles.
test/conformance/fixtures/V17CheckpointShardIndexFixture.ts#L33-L39: renamenodeLivenessShardOid/propertyShardOidtonodeLivenessShardHandle/propertyShardHandle.test/conformance/fixtures/V17CheckpointTargetShardFixture.ts#L12-L19: rename the constructor optionshardOidtoshardHandle(and update call sites).📍 Affects 2 files
test/conformance/fixtures/V17CheckpointShardIndexFixture.ts#L33-L39(this comment)test/conformance/fixtures/V17CheckpointTargetShardFixture.ts#L12-L19🤖 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/conformance/fixtures/V17CheckpointShardIndexFixture.ts` around lines 33 - 39, Rename V17CheckpointShardIndexFixture methods nodeLivenessShardOid and propertyShardOid to nodeLivenessShardHandle and propertyShardHandle, updating all callers. In test/conformance/fixtures/V17CheckpointTargetShardFixture.ts lines 12-19, rename the constructor option shardOid to shardHandle and update every call site consistently.test/helpers/InMemoryAuditLogAdapter.ts (1)
19-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Apply
failReadsWithto every read operation.
listWriterIdsandreadEntryignore the configured failure, so audit traversal cannot accurately simulate storage read failures. Check#readFailurein both methods, ideally through one private guard.Also applies to: 53-59, 94-99
🤖 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/InMemoryAuditLogAdapter.ts` around lines 19 - 21, Update the read operations listWriterIds and readEntry to honor `#readFailure` before performing storage reads. Add and reuse a private guard for the configured error, ensuring failReadsWith consistently causes every read operation to throw while preserving normal behavior when no failure is configured.test/helpers/InMemoryBlobStorageAdapter.ts (1)
23-45: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject assets whose size differs from
expectedSize.
stagecurrently ignoresoptions.expectedSize, allowing truncated or oversized streams to pass tests. Validate the collected byte length before storing or returning the staged asset.🤖 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/InMemoryBlobStorageAdapter.ts` around lines 23 - 45, Update InMemoryBlobStorageAdapter.stage to compare bytes.byteLength with options.expectedSize after collecting the source and before computing the hash or storing the bytes; reject the asset when the sizes differ, while preserving the existing staging behavior for matching sizes.test/helpers/InMemoryGitCasFacade.ts (1)
138-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n -C3 '\bdescriptorBytes\b|new StagedBundle|putOrdered' .Repository: git-stunts/git-warp
Length of output: 6196
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the helper around the reported lines. sed -n '128,160p' test/helpers/InMemoryGitCasFacade.ts # Find the StagedBundle type/definition and any docs for descriptorBytes. rg -n -C3 'descriptorBytes|class StagedBundle|interface StagedBundle|type StagedBundle' src test node_modules 2>/dev/null || trueRepository: git-stunts/git-warp
Length of output: 19711
Measure
descriptorBytesfrom the encoded descriptor.lines.join('\n').lengthcounts UTF-16 code units, so non-ASCII bundle members underreport the blob size. Encode once and use the byte length fordescriptorBytes.🤖 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/InMemoryGitCasFacade.ts` around lines 138 - 155, Update the bundle creation logic around descriptorOid in the InMemoryGitCasFacade method to encode the joined descriptor text once, reuse those encoded bytes when calling writeBlob, and set StagedBundle’s descriptorBytes from the encoded byte length rather than the string length.test/helpers/MemoryRuntimeHost.ts (1)
28-40: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Require a runtime-honest persistence contract instead of casting and emulating it.
createMemoryRuntimeStorageaccepts a broader type than the adapter supports, andwithFixtureObjectTypeProbecompensates with incomplete Git object classification.
test/helpers/MemoryRuntimeHost.ts#L28-L40: requireInMemoryGraphAdapteror an explicit structural capability instead of asserting arbitraryCorePersistence.test/helpers/MemoryRuntimeStorageAdapter.ts#L96-L122: remove the fallback proxy, or implement accurate blob, tree, and commit tracking across every creation API.📍 Affects 2 files
test/helpers/MemoryRuntimeHost.ts#L28-L40(this comment)test/helpers/MemoryRuntimeStorageAdapter.ts#L96-L122🤖 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/MemoryRuntimeHost.ts` around lines 28 - 40, Require createMemoryRuntimeStorage to accept InMemoryGraphAdapter, or an explicit structural contract matching the adapter’s supported capabilities, instead of CorePersistence with a cast; update its callers and cache typing accordingly. In test/helpers/MemoryRuntimeStorageAdapter.ts:96-122, remove withFixtureObjectTypeProbe’s fallback proxy, or replace it with complete tracking of blob, tree, and commit objects for every creation API.test/helpers/MockIndexStorage.ts (1)
25-39: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Persist the supplied shards instead of returning an empty index.
writeShardsignores its stream, whilescanShardsandreadShardHandlesalways expose no entries. Tests using the semantic write path therefore silently lose all shards and cannot validate index round trips. Consume the stream and retain both its shard records and handle mapping under the returned index handle.🤖 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/MockIndexStorage.ts` around lines 25 - 39, Update MockIndexStorage.writeShards to consume the supplied WarpStream and persist its shard records plus handle mapping under the newly returned AssetHandle. Make scanShards and readShardHandles retrieve the stored data for that handle instead of always returning empty values, while preserving empty results for unknown handles.test/unit/domain/services/PatchBuilder.content.test.ts (1)
57-73: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Make this test prove staging starts before producer completion.
RecordingAssetStoragecollects the entire iterable, so this passes even ifPatchBuilderbuffers the complete stream before callingstage(). Use a gated producer/storage double to assert thatstage()is entered before the remaining chunks are released.🤖 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/unit/domain/services/PatchBuilder.content.test.ts` around lines 57 - 73, Update the test around createPatchBuilder and attachContent to use gated producer and storage doubles instead of RecordingAssetStorage; block the producer after its first chunk, assert stage() has been entered before releasing the remaining chunks, then verify the existing content, expectedSize, and _content.size assertions.test/unit/domain/services/WarpMessageCodec.test.ts (1)
153-184: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Exercise the malformed locator validation named by this test.
Every locator here is valid:
new AssetHandle('asset:patch')andLEGACY_OID. Add an invalid legacy OID or handle case so locator-validation regressions cannot pass unnoticed.🤖 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/unit/domain/services/WarpMessageCodec.test.ts` around lines 153 - 184, Update the malformed-locator coverage in the test “rejects malformed graph, writer, lamport, hash, and handles” by replacing or adding a locator assertion that uses an invalid legacy OID or AssetHandle value and expects the codec to reject it. Keep the existing graph, writer, lamport, and state-hash validation cases intact.test/unit/domain/warp/Writer.test.ts (1)
364-367: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Confirm PatchBuilder no longer calls persistence.writeBlob/writeTree/commitNodeWithTree # when a patchJournal is provided. rg -n 'writeBlob|writeTree|commitNodeWithTree' src/domain/services/PatchBuilder.tsRepository: git-stunts/git-warp
Length of output: 157
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Files ==\n' git ls-files 'test/unit/domain/warp/Writer.test.ts' 'test/unit/domain/warp/*' 'test/unit/**/PatchBuilderTestHarness.ts' 'src/**/PatchBuilder.ts' 'src/**/Writer.ts' | sed -n '1,200p' printf '\n== Outline: Writer.test.ts ==\n' ast-grep outline test/unit/domain/warp/Writer.test.ts --view expanded | sed -n '1,220p' printf '\n== Search RecordingPatchJournal / compareAndSwapRef ==\n' rg -n 'RecordingPatchJournal|compareAndSwapRef|appendPatch|writeBlob|writeTree|commitNodeWithTree|updateRef' test/unit src | sed -n '1,220p'Repository: git-stunts/git-warp
Length of output: 24699
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Writer.test.ts lines 320-430 ==\n' sed -n '320,430p' test/unit/domain/warp/Writer.test.ts | cat -n printf '\n== PatchBuilderTestHarness-related files ==\n' fd -a 'PatchBuilderTestHarness.ts|WriterFixtureJournal|RecordingPatchJournal' test/unit src printf '\n== Outline: any matching harness file ==\n' for f in $(fd -a 'PatchBuilderTestHarness.ts' test/unit src); do echo "--- $f ---" ast-grep outline "$f" --view expanded | sed -n '1,260p' doneRepository: git-stunts/git-warp
Length of output: 6711
🏁 Script executed:
#!/bin/bash set -euo pipefail # Read the specific test section with line numbers and inspect the helper harness. sed -n '1,260p' test/unit/domain/warp/Writer.test.ts | cat -n printf '\n== Search for the journal fixture implementation ==\n' rg -n 'class RecordingPatchJournal|class WriterFixtureJournal|appendPatch\(|compareAndSwapRef\(' test/unit srcRepository: git-stunts/git-warp
Length of output: 15320
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show the exact test region around the flagged mocks. nl -ba test/unit/domain/warp/Writer.test.ts | sed -n '350,405p'Repository: git-stunts/git-warp
Length of output: 197
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '112,150p' test/unit/domain/services/PatchBuilderTestHarness.ts | cat -nRepository: git-stunts/git-warp
Length of output: 1547
Drop the unused persistence mocks here and at 393-396.
RecordingPatchJournal.appendPatch()only records the request and returns a fixed published patch, sowriteBlob,writeTree, andcommitNodeWithTreeare dead setup on this path.🤖 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/unit/domain/warp/Writer.test.ts` around lines 364 - 367, Remove the unused persistence.writeBlob, persistence.writeTree, and persistence.commitNodeWithTree mock setups from this test block and the corresponding block around the second setup location. Keep persistence.updateRef only if it is exercised by the tested flow, and leave RecordingPatchJournal.appendPatch() behavior unchanged.test/unit/domain/WarpCore.blobAutoConstruct.test.ts (1)
26-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Weak identity assertion for
_checkpointStore/_indexStore.
_assetStorageis asserted with.toBe(services.content)(exact same instance), but_checkpointStore/_indexStoreonly assert.toBeInstanceOf(...). This wouldn't catch a bug where the runtime constructs a fresh adapter instead of reusing the one returned bycreateRuntimeStorageServices, undermining the test's stated purpose of verifying wiring from the storage provider.♻️ Proposed fix
- expect(runtime._checkpointStore).toBeInstanceOf(services.checkpoints.constructor); - expect(runtime._indexStore).toBeInstanceOf(services.indexes.constructor); + expect(runtime._checkpointStore).toBe(services.checkpoints); + expect(runtime._indexStore).toBe(services.indexes);📝 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.expect(runtime._assetStorage).toBe(services.content); expect(runtime._checkpointStore).toBe(services.checkpoints); expect(runtime._indexStore).toBe(services.indexes); });🤖 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/unit/domain/WarpCore.blobAutoConstruct.test.ts` around lines 26 - 29, Strengthen the wiring assertions in the test by replacing the `.toBeInstanceOf(...)` checks for `_checkpointStore` and `_indexStore` with exact identity checks against `services.checkpoints` and `services.indexes`, matching the existing `_assetStorage` assertion. Keep the test focused on verifying reuse of the instances returned by `createRuntimeStorageServices`.
There was a problem hiding this comment.
Actionable comments posted: 37
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
test/unit/scripts/storage-ownership-boundary.test.ts (1)
74-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate AST-walking logic between
forbiddenReferencesandforbiddenDomainStorageReferences.Both functions build a
ts.createSourceFile, run a near-identicalvisit/forEachChildtraversal, and differ only in which identifier/module sets they check and in violation-message wording. Consider extracting a single parameterized walker (taking identifier set, module set, and message formatter) to avoid two functions drifting out of sync — this would also retroactively giveforbiddenReferencestheimport type/dynamic-import()detection that onlyimportedModuleNamecurrently provides.🤖 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/unit/scripts/storage-ownership-boundary.test.ts` around lines 74 - 154, Consolidate the duplicated AST traversal in forbiddenReferences and forbiddenDomainStorageReferences into one parameterized walker that accepts the relevant identifier set, module set, and violation-message formatting. Reuse importedModuleName so both checks cover regular imports/exports, import type, and dynamic import() consistently, while preserving each function’s existing violation messages and return behavior.src/domain/trust/TrustRecordService.ts (1)
63-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the extracted
prevMismatch()helper instead of duplicating the error construction.
appendRecord's inline prev-mismatch throw (Lines 67-72) constructs the exact sameTrustErrorshape as the newprevMismatch()helper (Lines 162-167). Consolidating avoids the two copies drifting apart later.♻️ Proposed fix
if (record.prev !== currentTipRecordId) { - throw new TrustError( - `Prev-link mismatch: record.prev=${String(record.prev)}, chain tip=${String(currentTipRecordId)}`, - { code: 'E_TRUST_PREV_MISMATCH' }, - ); + throw prevMismatch(record.prev, currentTipRecordId); }Also applies to: 162-167
🤖 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 `@src/domain/trust/TrustRecordService.ts` around lines 63 - 72, Update the prev-link consistency check in appendRecord to call the existing prevMismatch() helper instead of constructing TrustError inline, passing the record’s prev value and currentTipRecordId so the shared error shape remains centralized.src/domain/services/MaterializedViewService.ts (1)
59-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale
persistIndexTree()references after removal.Both comments still describe
_encodeShardsToTreeas existing "for callers that persist viapersistIndexTree()", butpersistIndexTreewas removed from this class entirely. The comments should describe the actual current caller/consumer (or be removed) so they don't point at a nonexistent method.📝 Suggested comment fix
- // P5-LEGACY: encode shards for callers that persist via persistIndexTree(). + // P5-LEGACY: encode shards for callers still persisting the raw tree directly; + // migrate to IndexStorePort.writeShards() once available. const tree = this._encodeShardsToTree(shards); ... /** * Encodes IndexShard instances to a tree of CBOR bytes. * - * P5-LEGACY: This exists to support callers that persist via - * persistIndexTree(tree, persistence). Will be removed when callers - * migrate to IndexStorePort.writeShards(). + * P5-LEGACY: This exists to support callers that still persist the raw + * tree directly. Will be removed when callers migrate to + * IndexStorePort.writeShards(). */Also applies to: 121-127
🤖 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 `@src/domain/services/MaterializedViewService.ts` around lines 59 - 61, Update the P5-LEGACY comments around _encodeShardsToTree at both locations to remove the obsolete persistIndexTree() reference and describe the current caller or consumer instead; if no accurate consumer remains, remove the comments.test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts (1)
160-167: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead fixture fields left over after narrowing
MaterializePersistence/MaterializeDeps.
MaterializeController.tsnarrowedMaterializePersistenceto justreadRefand replacedcommitMessageCodecwithcheckpointStoreinMaterializeDeps, but two test fixtures still carry the old, now-unused shape. This doesn't break anything (excess properties on non-literal variables/superset class implementations are structurally fine), but it obscures the real contract for future readers.
test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts#L160-L167: dropreadTreeOids/showNode/readBlobfrom thepersistenceobject and drop the unusedcommitMessageCodec: DEFAULT_COMMIT_MESSAGE_CODECfield fromdeps.test/unit/domain/services/controllers/MaterializePatchStreamReducer.test.ts#L209-L225: dropshowNode/readTreeOids/readBlobfromTestPersistence, keeping onlyreadRef.🤖 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/unit/domain/services/controllers/MaterializeController.stateSession.test.ts` around lines 160 - 167, Remove the obsolete fixture fields to match the narrowed contracts: in test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts:160-167, keep only readRef in persistence and remove commitMessageCodec from deps; in test/unit/domain/services/controllers/MaterializePatchStreamReducer.test.ts:209-225, update TestPersistence to keep only readRef and remove showNode, readTreeOids, and readBlob.src/domain/services/optic/CheckpointShardFactReader.ts (1)
166-187: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDuplicate failure-translation logic — reuse
rethrowLogicalShardReadFailure.
readShardAsset's catch block (lines 172-186) re-implements the same "if Error, translate viacheckpointLogicalShardReadFailure, else rethrow" logic already centralized inrethrowLogicalShardReadFailure(lines 140-151). Delegating avoids the two implementations drifting apart.♻️ Proposed refactor
async function readShardAsset(options: { readonly graphName: string; readonly indexStore: CheckpointTailOpticSource['_indexStore']; readonly path: string; readonly handle: AssetHandle; }): Promise<Uint8Array> { try { return await collectAsyncIterable(options.indexStore.openShard(options.handle)); } catch (error) { - const failure = error instanceof Error - ? checkpointLogicalShardReadFailure(error, { - graphName: options.graphName, - path: options.path, - oid: options.handle.toString(), - }) - : null; - if (failure !== null) { - throw failure; - } - throw error; + return rethrowLogicalShardReadFailure(error, { + graphName: options.graphName, + path: options.path, + oid: options.handle.toString(), + }); } }🤖 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 `@src/domain/services/optic/CheckpointShardFactReader.ts` around lines 166 - 187, Update readShardAsset to delegate caught errors to the existing rethrowLogicalShardReadFailure helper, passing the graph name, path, object identifier, and original error context required by that helper. Remove the duplicated checkpointLogicalShardReadFailure translation and preserve propagation of non-Error failures through the centralized logic.src/ports/IndexStorePort.ts (1)
29-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocstrings still describe OID semantics after the handle migration.
The
writeShards,readShardHandles, anddecodeSharddocstrings still say "tree OID", "path-to-OID mapping", and use an(oid)example, but the signatures now useAssetHandle. Update the comments to match the opaque-handle contract.🤖 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 `@src/ports/IndexStorePort.ts` around lines 29 - 74, Update the docstrings for writeShards, readShardHandles, and decodeShard to use opaque AssetHandle terminology instead of OID-specific wording. Replace “tree OID,” “path-to-OID mapping,” and the decodeShard example’s “oid” parameter description with handle-based language, while preserving the existing behavior and type details.
🤖 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 `@scripts/migrations/v17.0.0/checkpoint-schema-upgrade.ts`:
- Around line 126-131: Rename the root-tree OID parameter and field throughout
the checkpoint migration flow from indexOid to a name representing the
checkpoint root tree, including the getCommitTree(previousCheckpointSha) call
and loadRetiredCheckpointPayload/readTreeOids handling. Update all references
consistently while preserving the existing tree partitioning behavior.
In `@src/domain/api/EvidenceRuntime.ts`:
- Around line 197-213: Update freezeRetentionEvidenceEntries to validate
entry.policy, entry.reachability, and entry.rootKind with the existing
runtime-backed constructors or explicit union validators before freezing and
returning each retention evidence object. Preserve the validated witness
handling and ensure invalid runtime values throw the appropriate domain error
instead of being copied into the frozen entry.
In `@src/domain/capabilities/QueryCapability.ts`:
- Around line 111-112: Update the doc comments for getContentHandle and
getEdgeContentHandle to describe returning an opaque content handle, not a
content blob OID, while preserving the existing “if any” nullable behavior.
In `@src/domain/capabilities/StrandCapability.ts`:
- Line 20: Move PatchCommitResult into a dedicated domain-owned type module,
then update StrandCapability and StrandController to import it from that module
instead of PatchCommitter.ts. Affected sites:
src/domain/capabilities/StrandCapability.ts:20-20 and
src/domain/services/controllers/StrandController.ts:21-21; both require the
import change.
In `@src/domain/services/audit/AuditChainVerifier.ts`:
- Around line 165-170: Update the readHead error handling in the audit-chain
verification method of AuditChainVerifier so genuine storage, integrity, or
decryption failures are not returned as the initial VALID result. Preserve null
only for a successfully read absent head, and record an error on result or
propagate the caught failure before returning.
In `@src/domain/services/audit/AuditReceiptService.ts`:
- Around line 382-405: Update the AuditLogPort append contract and its adapters
to model publication conflicts explicitly as a discriminated result (or a
runtime-backed WarpError subtype), then have the publication flow around
_retryAfterCasConflict dispatch on that typed contract instead of calling
errorCode(error) or matching PUBLICATION_CONFLICT. Preserve retry-once and
degradation behavior, and ensure any domain error introduced extends WarpError
without adding unknown outside adapter boundaries.
In `@src/domain/services/controllers/CheckpointController.ts`:
- Around line 350-353: Update the legacy-schema check in the _readPatch flow to
use the validated decoded.schema value directly, removing the optional-shape
cast and undefined-as-legacy fallback. If Patch does not expose schema, add a
runtime-backed schema accessor to its validated domain API, then preserve legacy
handling only when decoded.schema equals 1.
- Around line 266-267: Update the checkpoint-loading flow around resolveHead()
and loadCheckpoint() so only a missing head returns null. Once resolveHead()
yields a non-empty handle, remove or narrow the catch that swallows
loadCheckpoint() failures, allowing missing or corrupt checkpoint asset errors
to propagate instead of triggering reconstruction.
In `@src/domain/services/controllers/PatchController.ts`:
- Around line 78-80: Update the _auditService.commit type in PatchController to
expose the concrete audit contract: use Promise<void> when commit returns no
value, or define and use an explicit witness/result type when it does. Remove
Promise<unknown> while preserving the existing nullable audit-service shape.
In `@src/domain/services/controllers/ReadGraphHost.ts`:
- Around line 40-44: Rename the PatchBlobReadHost type to reflect that
_readPatch returns a decoded Patch rather than blob data. Update the
ProvenanceReadHost alias and every reference to the old type name, preserving
the existing members and behavior.
In `@src/domain/services/index/LogicalIndexReader.ts`:
- Around line 81-103: Update the docstring for
LogicalIndexReader.loadFromHandles to describe loading shards through opaque
AssetHandle values and indexStore.decodeShard, using handle-based,
codec-agnostic wording instead of OID→blob storage.
In `@src/domain/services/index/PropertyIndexReader.ts`:
- Around line 61-63: Update the docstring for setupHandles to replace the stale
“OID mappings” terminology with wording that describes configuring opaque
AssetHandle values for lazy loading; keep the documentation aligned with the
handle-based API and remove the OID reference.
In `@src/domain/services/state/checkpointLoad.ts`:
- Around line 49-53: Update loadCheckpoint and the semantic store contract so
checkpoint loading validates both graphName and checkpointSha, rejecting or
ignoring handles owned by another graph before materializing state. Preserve
ownership through materializeIncremental rather than discarding graphName, and
apply the same graph-bound validation to the related checkpoint-loading paths.
In `@src/domain/WarpCore.ts`:
- Around line 45-48: Update the JSDoc for getContentHandle and
getEdgeContentHandle to replace the stale “content blob OID” terminology with
the current opaque-handle terminology, leaving the declarations unchanged.
In `@src/infrastructure/adapters/CborCheckpointStoreAdapter.ts`:
- Around line 84-126: Update publishCheckpoint in
src/infrastructure/adapters/CborCheckpointStoreAdapter.ts:84-126 to stage
checkpoint artifacts and their aggregate through the git-cas asset, bundle, and
publication APIs, retaining returned asset handles instead of using raw Git
blob/tree OIDs. Update the asset-handle construction at
src/infrastructure/adapters/CborCheckpointStoreAdapter.ts:206-224 to return
genuine persisted handles, and stage index shard bytes in asset storage while
preserving their handles at
src/infrastructure/adapters/CborCheckpointStoreAdapter.ts:333-347. Adjust
test/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.ts:110-120 to
verify shards open through the asset-backed storage port rather than raw Git
history.
- Around line 274-283: Validate the decoded value in _readFrontier before
calling Object.entries or constructing the Map, rejecting null, arrays,
non-objects, and objects with any non-string values. Add or reuse an
isStringRecord type guard so the decoded data is runtime-validated and
TypeScript accurately narrows it to Record<string, string>; preserve the
existing Map<string, string> result for valid frontiers.
In `@src/infrastructure/adapters/CborPatchJournalAdapter.ts`:
- Around line 202-204: Extract the null/undefined dependency validation from
CborPatchJournalAdapter’s requireDependency into a shared concept-named module
for git-cas adapters, preserving the existing WarpError message and
E_INVALID_DEPENDENCY code. Remove the local duplicate and update both
CborPatchJournalAdapter and CborCheckpointStoreAdapter to import and reuse the
shared guard.
- Around line 185-199: Both adapters duplicate the async-iterable-to-Uint8Array
buffering logic. Create one concept-named shared AssetByteCollector module, move
the collectBytes implementation there, and replace the local helpers with
imports in src/infrastructure/adapters/CborPatchJournalAdapter.ts lines 185-199
and src/infrastructure/adapters/GitCasStrandStoreAdapter.ts lines 222-236;
update each call site to use the shared implementation.
In `@src/infrastructure/adapters/GitCasAssetStorageAdapter.ts`:
- Around line 110-120: Update `#openLegacyBlob` to validate
compatibilityPolicy.legacyContentBlobReads before calling `#readLegacyCandidate`,
throwing the existing E_LEGACY_SUBSTRATE_DISABLED PersistenceError immediately
when disabled; only read the legacy candidate and return singleChunk(bytes) when
the policy permits access.
In `@src/infrastructure/adapters/GitCasAuditLogAdapter.ts`:
- Around line 131-158: The duplicated helper implementations in
src/infrastructure/adapters/GitCasAuditLogAdapter.ts lines 131-158 and
src/infrastructure/adapters/GitTrustChainAdapter.ts lines 361-388 should be
centralized. Create a shared infrastructure module exporting collectBytes,
errorCode, and the MANIFEST_NOT_FOUND legacy-tree rethrow guard, then import and
use those exports in both adapters while removing their local copies.
In `@src/infrastructure/adapters/GitCasStrandStoreAdapter.ts`:
- Around line 166-207: Extend CommitMessageCodecPort with a strand-descriptor
message kind and use it in GitCasStrandStoreAdapter instead of the local
encodeDescriptorMessage, decodeDescriptorMessage, and requireTrailer helpers.
Implement the corresponding serialization and parsing in
TrailerCommitMessageCodecAdapter, preserving graphName, strandId, and
descriptorHandle validation through the shared codec.
In `@src/infrastructure/adapters/GitRecursiveTreeOidReaderAdapter.ts`:
- Line 303: Move the exported TreeEntryProbeResult type out of
GitRecursiveTreeOidReaderAdapter and into TreeEntryProbePort.ts or an
appropriately named domain type module beside the port contract. Update the
adapter and all consumers to import it from that contract-owned location,
preserving the existing TreeEntryFound and TreeEntryMissing union.
In `@src/infrastructure/adapters/GraphModelMigrationDryRunRequestJsonAdapter.ts`:
- Around line 147-155: Update the content-source parsing around contentHandle in
GraphModelMigrationDryRunRequestJsonAdapter to detect when both contentHandle
and contentOid are supplied and require their values to be equal, rejecting
conflicting aliases before coalescing them. Preserve the existing fallback
behavior when only one alias is present and continue passing the resolved value
to readRequiredString.
In `@src/infrastructure/adapters/TrailerCommitMessageCodecAdapter.ts`:
- Around line 343-348: Update the normalized decode return in the adapter to
omit the legacy patchOid field before spreading parsed.data, while still
constructing patchHandle from patchHandle or patchOid. Ensure the returned
PatchCommitMessage contains only the handle-based representation at runtime.
In `@test/helpers/InMemoryAuditLogAdapter.ts`:
- Around line 19-25: Update the InMemoryAuditLogAdapter read methods so the
failure configured by failReadsWith is checked and propagated by readHead,
readEntry, and listWriterIds. Preserve their existing successful-read behavior
when no read failure is configured.
In `@test/helpers/InMemoryBlobStorageAdapter.ts`:
- Around line 86-89: Update retrieveStream to defer `#requireBytes` errors until
the returned AsyncIterable is consumed, matching open’s async-generator
behavior. Preserve token conversion and single-chunk output, but move byte
lookup into an async generator or equivalent lazy iteration path so missing
handles produce a rejected consumption promise rather than a synchronous
call-time throw.
In `@test/helpers/InMemoryCheckpointStore.ts`:
- Around line 78-84: Update InMemoryCheckpointStore.publishCoverage to return
the generated 40-character suffix directly. Remove the unused graphName-derived
prefix and resulting slice, while preserving the sequence increment and
hexadecimal padding behavior.
In `@test/helpers/InMemoryGitCasFacade.ts`:
- Around line 158-175: Add a concise explanatory comment in `#publish` immediately
before the compareAndSwapRef call inside the current !== request.ref.expected
branch, documenting that the deliberately failing CAS delegates conflict
detection and error behavior to `#history`. Do not alter the existing control flow
or CAS arguments.
In `@test/helpers/InMemoryGraphAdapter.ts`:
- Around line 203-216: Update InMemoryGraphAdapter.readObjectType to return
Promise<'blob' | 'tree' | 'commit'> instead of Promise<string>, preserving its
existing return values and missing-object error behavior. Ensure any related
duck-typed interfaces or call sites, including withFixtureObjectTypeProbe, use
the narrowed result type without weakening it back to string.
In `@test/helpers/MemoryRuntimeStorageAdapter.ts`:
- Around line 38-42: Update the GitCasAssetStorageAdapter initialization in the
MemoryRuntimeStorageAdapter constructor to pass the probed `#history` as
legacyReader instead of raw options.history, keeping `#cas` configured with the
same probed history.
In `@test/helpers/WarpGraphMockPersistence.ts`:
- Around line 151-174: Rename the local FixturePatchJournal class to a distinct
name such as PopulatedPersistencePatchJournal, and update all references to this
class within the file, including instantiation and type usage. Leave the shared
FixturePatchJournal helper unchanged.
In `@test/unit/domain/services/PatchBuilderTestHarness.ts`:
- Around line 27-28: Update the publication/write tracking around
WRITTEN_PATCHES and the helper that accepts the call index so each publication’s
Patch is preserved and retrieved by its corresponding index, rather than
overwriting the latest WeakMap entry; alternatively remove the unused index
consistently if publications are intentionally single-valued. Ensure
multi-publication tests decode the patch written for the requested publication.
In `@test/unit/domain/services/pathKeyedTreeMaps.test.ts`:
- Around line 20-28: Remove the vacuous Object.prototype assertion in the
partitionShardHandles test and replace it with an assertion that directly
verifies Object.prototype itself was not mutated. Keep the existing
Object.hasOwn checks and handle-value assertions unchanged.
In `@test/unit/domain/services/strand/StrandService.test.ts`:
- Around line 321-405: Update the shared patchJournal fake’s appendPatch
implementation to honor request.expectedHead by validating the current targetRef
before writing and rejecting stale-head updates. Return the complete
PublishedPatch shape, including sha, bundleHandle, stagedPatch, and retention,
and replace local partial mocks with this shared fake. Add assertions that
service results preserve the returned retention evidence.
In `@test/unit/domain/WarpGraph.encryption.test.ts`:
- Around line 29-35: Strengthen the encryption tests around
runtime.materialize(), reopening, and corrupted encryption material: verify the
persisted git-cas asset content is not plaintext, then reopen or read it with an
incompatible key and with corrupted encryption data and assert both paths reject
with EncryptionError. Preserve the existing successful encrypted read assertions
for the valid key.
In `@test/unit/infrastructure/adapters/GitTrustChainAdapter.test.ts`:
- Around line 145-160: Update the readRecords test around adapter.readRecords to
make traversal order observable: have the mocked tree handles produce distinct
record IDs for newest and oldest commits, then assert the collected records
exactly match the expected oldest-then-newest sequence rather than only checking
length and uniform IDs.
In `@test/unit/infrastructure/adapters/TrailerCommitMessageCodecAdapter.test.ts`:
- Around line 45-64: Add a sibling round-trip test in the
“TrailerCommitMessageCodecAdapter storage routing” suite using
createGitCasPatchStorage. Assert the encoded trailer includes eg-patch-handle
and omits eg-patch-oid, then verify decodePatch restores the AssetHandle and
current git-cas-asset storage route.
---
Outside diff comments:
In `@src/domain/services/MaterializedViewService.ts`:
- Around line 59-61: Update the P5-LEGACY comments around _encodeShardsToTree at
both locations to remove the obsolete persistIndexTree() reference and describe
the current caller or consumer instead; if no accurate consumer remains, remove
the comments.
In `@src/domain/services/optic/CheckpointShardFactReader.ts`:
- Around line 166-187: Update readShardAsset to delegate caught errors to the
existing rethrowLogicalShardReadFailure helper, passing the graph name, path,
object identifier, and original error context required by that helper. Remove
the duplicated checkpointLogicalShardReadFailure translation and preserve
propagation of non-Error failures through the centralized logic.
In `@src/domain/trust/TrustRecordService.ts`:
- Around line 63-72: Update the prev-link consistency check in appendRecord to
call the existing prevMismatch() helper instead of constructing TrustError
inline, passing the record’s prev value and currentTipRecordId so the shared
error shape remains centralized.
In `@src/ports/IndexStorePort.ts`:
- Around line 29-74: Update the docstrings for writeShards, readShardHandles,
and decodeShard to use opaque AssetHandle terminology instead of OID-specific
wording. Replace “tree OID,” “path-to-OID mapping,” and the decodeShard
example’s “oid” parameter description with handle-based language, while
preserving the existing behavior and type details.
In
`@test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts`:
- Around line 160-167: Remove the obsolete fixture fields to match the narrowed
contracts: in
test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts:160-167,
keep only readRef in persistence and remove commitMessageCodec from deps; in
test/unit/domain/services/controllers/MaterializePatchStreamReducer.test.ts:209-225,
update TestPersistence to keep only readRef and remove showNode, readTreeOids,
and readBlob.
In `@test/unit/scripts/storage-ownership-boundary.test.ts`:
- Around line 74-154: Consolidate the duplicated AST traversal in
forbiddenReferences and forbiddenDomainStorageReferences into one parameterized
walker that accepts the relevant identifier set, module set, and
violation-message formatting. Reuse importedModuleName so both checks cover
regular imports/exports, import type, and dynamic import() consistently, while
preserving each function’s existing violation messages and return behavior.
🪄 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: 81183cca-d0df-438a-af35-3c13f465c6f2
📒 Files selected for processing (294)
README.mdbin/cli/commands/trust.tsbin/cli/commands/verify-audit.tsdocs/migrations/v19/README.mddocs/topics/README.mddocs/topics/api/README.mddocs/topics/getting-started.mdpolicy/quarantines/0025B-boundary.jsonscripts/hooks/pre-pushscripts/migrations/v17.0.0/checkpoint-schema-upgrade.tsscripts/run-stable-unit-tests.tsscripts/source-size-gate.tsscripts/source-version-name-policy.tsscripts/upgrade-v16-to-v17.tsscripts/v18.0.0/migrations/graph-model/GraphModelMigrationSourceInventoryCollector.tsscripts/v18.0.0/migrations/graph-model/V17RestoredPublicReadLegacyReadingBuilder.tssrc/domain/RuntimeHost.tssrc/domain/WarpApp.tssrc/domain/WarpCore.tssrc/domain/WarpGraph.tssrc/domain/WarpWorldline.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/Evidence.tssrc/domain/api/EvidenceRuntime.tssrc/domain/api/TimelineRuntime.tssrc/domain/api/WriteRuntime.tssrc/domain/capabilities/PatchCapability.tssrc/domain/capabilities/PatchCollector.tssrc/domain/capabilities/QueryCapability.tssrc/domain/capabilities/StrandCapability.tssrc/domain/errors/EncryptionError.tssrc/domain/graph/ContentAttachmentHandle.tssrc/domain/graph/ContentAttachmentOid.tssrc/domain/graph/ContentAttachmentPayload.tssrc/domain/graph/ContentAttachmentWriteIntent.tssrc/domain/graph/publicGraphSubstrate.tssrc/domain/migrations/GraphModelMigrationContentSource.tssrc/domain/services/ContentAttachmentProjection.tssrc/domain/services/CoordinateFactExport.tssrc/domain/services/MaterializedViewHelpers.tssrc/domain/services/MaterializedViewService.tssrc/domain/services/PatchBuilder.tssrc/domain/services/PatchBuilderContent.tssrc/domain/services/PatchCommitter.tssrc/domain/services/WormholeService.tssrc/domain/services/audit/AuditChainVerifier.tssrc/domain/services/audit/AuditReceiptService.tssrc/domain/services/audit/AuditVerifierService.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/ComparisonEngine.tssrc/domain/services/controllers/ComparisonSelectorSupport.tssrc/domain/services/controllers/ForkController.tssrc/domain/services/controllers/IntentController.tssrc/domain/services/controllers/MaterializeCheckpointStrategy.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeStrategyRuntime.tssrc/domain/services/controllers/PatchController.tssrc/domain/services/controllers/PatchDiscovery.tssrc/domain/services/controllers/ProvenanceController.tssrc/domain/services/controllers/QueryContent.tssrc/domain/services/controllers/QueryController.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/services/controllers/StrandController.tssrc/domain/services/controllers/SyncControllerTypes.tssrc/domain/services/controllers/detachedOpen.tssrc/domain/services/index/BitmapIndexReader.tssrc/domain/services/index/LogicalIndexReader.tssrc/domain/services/index/PropertyIndexReader.tssrc/domain/services/optic/CheckpointBasisFactTypes.tssrc/domain/services/optic/CheckpointContentAnchorFact.tssrc/domain/services/optic/CheckpointFactResolver.tssrc/domain/services/optic/CheckpointNeighborhoodPageReader.tssrc/domain/services/optic/CheckpointPatchFactStream.tssrc/domain/services/optic/CheckpointShardFactReader.tssrc/domain/services/optic/CheckpointTailBasisLoader.tssrc/domain/services/optic/CheckpointTailBasisVerifier.tssrc/domain/services/optic/CheckpointTailOpticSource.tssrc/domain/services/optic/CoordinateCheckpointTailOpticSource.tssrc/domain/services/state/StateReaderContext.tssrc/domain/services/state/checkpointCreate.tssrc/domain/services/state/checkpointHelpers.tssrc/domain/services/state/checkpointLoad.tssrc/domain/services/strand/StrandCoordinator.tssrc/domain/services/strand/StrandDescriptorStore.tssrc/domain/services/strand/StrandIntentService.tssrc/domain/services/strand/StrandPatchService.tssrc/domain/services/strand/createStrandCoordinator.tssrc/domain/services/strand/descriptorNormalization.tssrc/domain/services/strand/strandTypes.tssrc/domain/services/sync/syncPatchLoader.tssrc/domain/services/sync/syncRequestResponse.tssrc/domain/services/transfer/transferOps.tssrc/domain/storage/AssetHandle.tssrc/domain/storage/BundleHandle.tssrc/domain/storage/StorageHandle.tssrc/domain/storage/StorageRetentionWitness.tssrc/domain/stream/Sink.tssrc/domain/trust/TrustRecordService.tssrc/domain/types/ContentMeta.tssrc/domain/types/CoordinateComparison.tssrc/domain/types/StrandDescriptor.tssrc/domain/types/WarpIntentDescriptor.tssrc/domain/types/WarpPersistence.tssrc/domain/utils/RefLayout.tssrc/domain/utils/parseStrandBlob.tssrc/domain/utils/streamUtils.tssrc/domain/warp/RuntimeHostBoot.tssrc/domain/warp/RuntimeHostProduct.tssrc/domain/warp/RuntimePatchCollector.tssrc/domain/warp/WarpCoreRuntimeProduct.tssrc/domain/warp/WarpGraphRuntimeProduct.tssrc/domain/warp/Writer.tssrc/infrastructure/adapters/CasBlobAdapter.tssrc/infrastructure/adapters/CasPayloadPointer.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.tssrc/infrastructure/adapters/CborIndexStoreAdapter.tssrc/infrastructure/adapters/CborPatchJournalAdapter.tssrc/infrastructure/adapters/GitCasAssetStorageAdapter.tssrc/infrastructure/adapters/GitCasAuditLogAdapter.tssrc/infrastructure/adapters/GitCasIntentStoreAdapter.tssrc/infrastructure/adapters/GitCasRepositoryAdapter.tssrc/infrastructure/adapters/GitCasRetentionWitnessAdapter.tssrc/infrastructure/adapters/GitCasStrandStoreAdapter.tssrc/infrastructure/adapters/GitRecursiveTreeOidReaderAdapter.tssrc/infrastructure/adapters/GitTimelineHistoryAdapter.tssrc/infrastructure/adapters/GitTrustChainAdapter.tssrc/infrastructure/adapters/GraphModelMigrationDryRunRequestJsonAdapter.tssrc/infrastructure/adapters/TrailerCommitMessageCodecAdapter.tssrc/ports/AssetStoragePort.tssrc/ports/AuditLogPort.tssrc/ports/BlobPort.tssrc/ports/BlobStoragePort.tssrc/ports/CheckpointStorePort.tssrc/ports/CommitMessageCodecPort.tssrc/ports/CommitPort.tssrc/ports/GraphPersistencePort.tssrc/ports/IndexStoragePort.tssrc/ports/IndexStorePort.tssrc/ports/IntentStorePort.tssrc/ports/PatchJournalPort.tssrc/ports/RuntimeStorageProviderPort.tssrc/ports/StrandStorePort.tssrc/ports/TreeEntryProbePort.tssrc/ports/TreePort.tssrc/ports/TrustChainPort.tssrc/ports/WarpKernelPort.tstest/benchmark/logicalIndex.benchmark.tstest/conformance/comparisonLiveCoordinateSeam.test.tstest/conformance/fixtures/V17CheckpointBasisArtifactFixture.tstest/conformance/fixtures/V17CheckpointIndexTreeFixture.tstest/conformance/fixtures/V17CheckpointPayloadBlobFixture.tstest/conformance/fixtures/V17CheckpointShardIndexFixture.tstest/conformance/fixtures/V17CheckpointTargetShardFixture.tstest/conformance/v17CheckpointTailOpticReadBasis.test.tstest/conformance/v18FirstUseOpticsHonesty.test.tstest/helpers/FixturePatchJournal.tstest/helpers/InMemoryAuditLogAdapter.tstest/helpers/InMemoryBlobStorageAdapter.tstest/helpers/InMemoryCheckpointStore.tstest/helpers/InMemoryGitCasFacade.tstest/helpers/InMemoryGraphAdapter.tstest/helpers/MemoryRuntimeHost.tstest/helpers/MemoryRuntimeStorageAdapter.tstest/helpers/MockBlobPort.tstest/helpers/MockIndexStorage.tstest/helpers/MockTreePort.tstest/helpers/MockTrustChainPort.tstest/helpers/WarpGraphMockPersistence.tstest/helpers/storageRetention.tstest/integration/api/content-attachment.test.tstest/unit/cli/verify-audit.test.tstest/unit/domain/DraftTimelineRuntime.test.tstest/unit/domain/WarpApp.delegation.test.tstest/unit/domain/WarpCore.blobAutoConstruct.test.tstest/unit/domain/WarpCore.content.test.tstest/unit/domain/WarpCore.stateSessionAutoConstruct.test.tstest/unit/domain/WarpGraph.audit.test.tstest/unit/domain/WarpGraph.checkpoint.test.tstest/unit/domain/WarpGraph.content.test.tstest/unit/domain/WarpGraph.coverageGaps.test.tstest/unit/domain/WarpGraph.encryption.test.tstest/unit/domain/WarpGraph.strands.compare.test.tstest/unit/domain/WarpGraph.strands.test.tstest/unit/domain/WarpGraph.versionVector.test.tstest/unit/domain/WarpGraph.writerInvalidation.test.tstest/unit/domain/WarpWorldline.capabilities.test.tstest/unit/domain/WarpWorldline.test.tstest/unit/domain/graph/ContentAttachmentPayload.test.tstest/unit/domain/graph/ContentAttachmentRecord.test.tstest/unit/domain/graph/ContentAttachmentWriteIntent.test.tstest/unit/domain/graph/GraphOpAlgebra.test.tstest/unit/domain/migrations/DryRunGraphModelMigrationPlanner.test.tstest/unit/domain/migrations/GraphModelMigrationConstructorGuards.test.tstest/unit/domain/migrations/GraphModelMigrationOperationLowering.test.tstest/unit/domain/migrations/GraphModelMigrationSourceInventory.test.tstest/unit/domain/runtimeProductExecutableSurface.test.tstest/unit/domain/runtimeReadingBasisErrors.test.tstest/unit/domain/services/AuditReceiptService.coverage.test.tstest/unit/domain/services/AuditReceiptService.test.tstest/unit/domain/services/AuditVerifierService.bench.tstest/unit/domain/services/AuditVerifierService.test.tstest/unit/domain/services/BitmapIndexReader.chunked.test.tstest/unit/domain/services/BitmapIndexReader.test.tstest/unit/domain/services/CheckpointService.anchors.test.tstest/unit/domain/services/CheckpointService.edgeCases.test.tstest/unit/domain/services/CheckpointService.test.tstest/unit/domain/services/ContentAttachmentProjection.test.tstest/unit/domain/services/LogicalIndexReader.test.tstest/unit/domain/services/MaterializedViewService.test.tstest/unit/domain/services/MessageCodecModules.test.tstest/unit/domain/services/PatchBuilder.cas.test.tstest/unit/domain/services/PatchBuilder.commit.test.tstest/unit/domain/services/PatchBuilder.content.test.tstest/unit/domain/services/PatchBuilder.contentPersistence.test.tstest/unit/domain/services/PatchBuilder.test.tstest/unit/domain/services/PatchBuilderContentWriteIntent.test.tstest/unit/domain/services/PatchBuilderTestHarness.tstest/unit/domain/services/PatchCommitter.visibility.test.tstest/unit/domain/services/PropertyIndex.test.tstest/unit/domain/services/StateReaderPropertyProjection.test.tstest/unit/domain/services/StateSerializer.test.tstest/unit/domain/services/SyncProtocol.divergence.test.tstest/unit/domain/services/SyncProtocol.stateCoherence.test.tstest/unit/domain/services/SyncProtocol.test.tstest/unit/domain/services/TreeConstruction.determinism.test.tstest/unit/domain/services/VisibleStateTransferPlanner.test.tstest/unit/domain/services/WarpMessageCodec.test.tstest/unit/domain/services/WarpMessageCodec.v3.test.tstest/unit/domain/services/WormholeService.test.tstest/unit/domain/services/audit/AuditChainVerifier.test.tstest/unit/domain/services/controllers/CheckpointController.snapshotCache.test.tstest/unit/domain/services/controllers/CheckpointController.test.tstest/unit/domain/services/controllers/ComparisonController.test.tstest/unit/domain/services/controllers/ForkController.test.tstest/unit/domain/services/controllers/IntentController.test.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tstest/unit/domain/services/controllers/MaterializePatchStreamReducer.test.tstest/unit/domain/services/controllers/PatchController.test.tstest/unit/domain/services/controllers/ProvenanceController.test.tstest/unit/domain/services/controllers/QueryContentProjectionReads.test.tstest/unit/domain/services/controllers/QueryController.test.tstest/unit/domain/services/controllers/StrandController.host-interface.test.tstest/unit/domain/services/optic/CheckpointFactResolver.test.tstest/unit/domain/services/optic/CheckpointPatchFactStream.test.tstest/unit/domain/services/optic/CheckpointShardFactReader.manifest.test.tstest/unit/domain/services/optic/CheckpointTailBasisVerifier.test.tstest/unit/domain/services/optic/CheckpointTailReadIdentityBuilder.test.tstest/unit/domain/services/optic/CoordinateCheckpointTailOpticSource.test.tstest/unit/domain/services/optic/TraversalOptic.test.tstest/unit/domain/services/optic/WorldlineOptic.test.tstest/unit/domain/services/pathKeyedTreeMaps.test.tstest/unit/domain/services/strand/StrandService.test.tstest/unit/domain/services/sync/SyncResponsePagingMetrics.test.tstest/unit/domain/storage/StoragePrimitives.test.tstest/unit/domain/strandAndRuntimeSeams.test.tstest/unit/domain/trust/TrustRecordService.test.tstest/unit/domain/warp/RuntimeHostPortResolvers.test.tstest/unit/domain/warp/Writer.test.tstest/unit/domain/warp/readPatchBlob.test.tstest/unit/infrastructure/CborIndexStoreAdapter.test.tstest/unit/infrastructure/adapters/CasBlobAdapter.test.tstest/unit/infrastructure/adapters/CasPayloadPointer.test.tstest/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.tstest/unit/infrastructure/adapters/CborPatchJournalAdapter.test.tstest/unit/infrastructure/adapters/GitCasAssetStorageAdapter.test.tstest/unit/infrastructure/adapters/GitCasAuditLogAdapter.test.tstest/unit/infrastructure/adapters/GitCasIntentStoreAdapter.test.tstest/unit/infrastructure/adapters/GitCasRepositoryAdapter.test.tstest/unit/infrastructure/adapters/GitCasStrandStoreAdapter.test.tstest/unit/infrastructure/adapters/GitGraphAdapter.coverage.test.tstest/unit/infrastructure/adapters/GitTrustChainAdapter.test.tstest/unit/infrastructure/adapters/GraphModelMigrationDryRunRequestJsonAdapter.test.tstest/unit/infrastructure/adapters/InMemoryBlobStorageAdapter.test.tstest/unit/infrastructure/adapters/InMemoryGraphAdapter.objectType.test.tstest/unit/infrastructure/adapters/TrailerCommitMessageCodecAdapter.test.tstest/unit/ports/BlobPort.test.tstest/unit/ports/BlobStoragePort.test.tstest/unit/ports/CheckpointStorePort.test.tstest/unit/ports/CommitPort.test.tstest/unit/ports/GraphPersistencePort.test.tstest/unit/ports/IndexStorePort.test.tstest/unit/ports/PatchJournalPort.test.tstest/unit/ports/TreePort.test.tstest/unit/ports/WasmVerifiedAdmissionPort.test.tstest/unit/scripts/checkpoint-schema-upgrade.test.tstest/unit/scripts/op-hydration-boundary-ratchet.test.tstest/unit/scripts/pre-push-hook.test.tstest/unit/scripts/source-size-inventory-command.test.tstest/unit/scripts/source-version-name-policy.test.tstest/unit/scripts/storage-ownership-boundary.test.tstest/unit/scripts/v16-to-v17-upgrade.test.tstest/unit/scripts/v18-content-property-closeout-audit.test.tstest/unit/scripts/v18-v17-public-read-legacy-reading-builder.test.ts
💤 Files with no reviewable changes (23)
- src/ports/IndexStoragePort.ts
- test/unit/scripts/v18-content-property-closeout-audit.test.ts
- src/ports/TreePort.ts
- src/infrastructure/adapters/CasBlobAdapter.ts
- src/ports/BlobPort.ts
- test/unit/domain/services/CheckpointService.anchors.test.ts
- src/domain/graph/ContentAttachmentOid.ts
- src/ports/BlobStoragePort.ts
- test/unit/domain/services/PatchBuilder.test.ts
- src/ports/TreeEntryProbePort.ts
- test/unit/scripts/source-size-inventory-command.test.ts
- test/conformance/fixtures/V17CheckpointBasisArtifactFixture.ts
- src/domain/capabilities/PatchCollector.ts
- test/conformance/fixtures/V17CheckpointPayloadBlobFixture.ts
- src/infrastructure/adapters/CasPayloadPointer.ts
- scripts/source-size-gate.ts
- src/domain/services/controllers/SyncControllerTypes.ts
- test/conformance/v17CheckpointTailOpticReadBasis.test.ts
- test/unit/domain/services/MessageCodecModules.test.ts
- src/domain/warp/RuntimePatchCollector.ts
- src/ports/CommitPort.ts
- src/domain/services/controllers/MaterializeStrategyRuntime.ts
- src/domain/services/controllers/detachedOpen.ts
Stage checkpoint and index artifacts through git-cas assets and ordered bundles, then publish retained checkpoint references atomically. Preserve legacy checkpoint reads behind an explicit adapter and migrate v5 storage to the v19 bundle envelope with graph and integrity validation.
Assert path-keyed materialization maps retain ordinary prototypes and replace the remaining blob-oriented comments with storage-neutral asset terminology.
Release Preflight
If this PR is from a |
Update node and edge content integration coverage to expect the policy-first compatibility error for naked legacy object handles. The adapter-level migration-policy suite continues to cover missing legacy objects.
Release Preflight
If this PR is from a |
1 similar comment
Release Preflight
If this PR is from a |
This was referenced Jul 16, 2026
This was referenced Jul 25, 2026
This was referenced Jul 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AssetHandleandBundleHandlevalues plus staged and retained storage evidence.Issue
Closes #737
Refs #712
Narrows #528
Follow-up materialization and operational work remains in #738, #739, #741, and #742.
Root cause and impact
git-warp previously owned low-level blob/tree sequencing and passed naked storage identifiers through domain ports. That made retention implicit, allowed referenced content to become prunable, and duplicated responsibilities owned by git-cas. Current writes now stage immutable assets, publish aggregate bundles through causal refs, and return explicit retention witnesses. Legacy reads remain policy-gated and current-format integrity failures are not masked as legacy data.
Checkpoint and index persistence now follows the same ownership rule. Current checkpoints stage assets, assemble an ordered bundle, and atomically publish a retained bundle handle. Reads validate graph identity, bundle membership, asset sizes, and bounded-basis handles. The migration path preserves parents and compare-and-swap expectations while converting legacy v5 raw Git layouts into the v19 git-cas bundle envelope.
This PR fixes object ownership and reachability. It does not claim that full-history reads or materialization are now bounded: backward scans that collect identifiers and checkpoint/index materialization remain explicit work for #738 and #739.
Test plan
npm run lintnpm run typechecknpm run typecheck:surfacenpm run typecheck:policynpm run typecheck:consumernpm run lint:ratchetnpm run lint:semgrepnpm run lint:contaminationGIT_WARP_QUARANTINE_BASE=origin/main npm run lint:quarantine-graduatenpm run test:coverage:ci -- --maxWorkers=2(574 files: 573 passed, 1 skipped; 7,015 tests passed, 2 skipped; 92.70% line coverage)npm run test:integration(19 files; 99 tests)git prune -n --expire=nowADR checks
Review notes
The aggregate Graft structural review did not complete after 26 minutes. Narrow symbol queries completed but reported zero cache hits and misattributed one class provenance commit; Git history identifies
d74acf45a8b10cd6ee35d6a3f2e65df2d278f724as the actual introduction. That performance and cache behavior is relevant evidence for #738 and #739, not a correctness claim for this PR.