Expose causal entity occurrences and substrate allocation - #838
Expose causal entity occurrences and substrate allocation#838flyingrobots wants to merge 56 commits into
Conversation
Creating a node with properties previously required node.add followed by
property.set: two patches, and the payload patch records a self-read
because setProperty adds its subject to the observed operands. Neither
patch can carry a complete entity, so an entity's cone was never a
singleton and its footprint was an under-approximation.
intent.entity.add({ subject, properties }) states one entity and its
complete initial payload as one fact. PatchBuilder.addEntity lowers it to
a single patch of NodeAdd + NodePropSet that reads nothing and writes
exactly one fresh id. The NodeAdd in that same patch is what brings the
node into existence, so the payload depends on nothing preceding the
patch, and the syntactic footprint is exact by construction rather than a
lower bound.
The shape is enforced, not merely offered: an id that already exists in
the patch or the graph is rejected (E_PATCH_ENTITY_EXISTS), and an entity
with no payload is rejected (E_PATCH_ENTITY_EMPTY), so the empty shell
filled by later property writes is not representable.
intentFromPatch recovers the shape from persisted operations. Any other
multi-operation patch still fails hydration rather than being
reinterpreted, including a payload naming a different node and a property
that precedes the node it belongs to.
Reference: docs/READINGS_AND_OPTICS.md sections 4, 8, and 11 (the
write-path affordances the substrate should provide).
PatchBuilder.ts sat one line under the 500 LOC source-size ceiling, so
this makes room honestly rather than relaxing the gate: node and edge
content attachment now share one staging helper instead of duplicating
the asset-storage precondition, and effect id validation moved to
PatchBuilderValidation.
Additive public API on every surface, library and CLI. Recommend a minor
version bump to 19.1.0.
Review found the implementation sound but the claims stronger than the evidence. This tightens the evidence and retracts the overclaims. Hydration was matching on operation shape alone. A leading NodeAdd followed by same-subject property writes is exactly what a legacy PropSet sequence looks like, and that sequence carries the self-read entity capture exists to eliminate, so shape-only recognition could promote a self-reading patch into a dependency-pure claim. Recognition now also requires the patch to declare the footprint: no reads, and exactly one write naming the created subject. A patch that resembles the shape without declaring the footprint falls through and fails hydration. Duplicate property keys were silently collapsed by last-write-wins assignment, so re-lowering a hydrated intent would not reproduce the patch it came from. Repeated keys now fail hydration. Property maps are built with a null prototype at both boundaries, so a caller- or patch-supplied `__proto__`, `constructor`, or `prototype` key stays ordinary data. Payload keys are ordered canonically at both ends, so two payloads differing only in construction order lower to identical operations. Three claims were wrong and are corrected in the API docs, the CHANGELOG, and the CLI documentation: - "complete payload" — the substrate enforces a non-empty payload. Which fields make an entity complete is an application schema concern it cannot know. - "the cone is a singleton" — true of the creation. `property.set` and `node.remove` remain public, so an immutable-entity lifetime is a law an application adopts, not one a constructor imposes. - "rejects an id that already exists in the graph" — measured false on the lane write path. The guard reads the builder's snapshot, which is the host's cached state, and that is null until something materializes. A new integration test records the actual behaviour: a writer with no materialized basis re-creates the same id without complaint, and two writers from the same frontier are both admitted and merged, giving that entity a two-patch cone. Collision-resistant ids are the application's responsibility, and the guard is now described as refusing only ids the builder can see. Adds end-to-end coverage the unit tests could not give: write through a real Runtime lane, close it, reopen from disk, hydrate the persisted patch back into the intent that wrote it, and prove `patchesFor` and `materializeSlice` return exactly the creation evidence and rebuild the entity from it, while sibling captures stay outside the cone. Also records measured release impact rather than asserting a version. `entity.add` is a new member of the Intent union, and although IntentKind is not exported by name it is structurally reachable, so an exhaustive consumer switch stops compiling with TS2345. The version choice is left to policy. Brings docs/READINGS_AND_OPTICS.md onto the branch, which the code, CHANGELOG, and CLI documentation all reference. The copy in the fix/default-checkpoint-policy worktree is untouched and still uncommitted.
The release impact was recorded as measured but unresolved. Resolving it: this ships as a major. entity.add adds a member to the Intent discriminated union. IntentKind is not exported by name, but it is structurally reachable through Intent['kind'], so a consumer that switches exhaustively over intent kinds stops compiling with TS2345. The runtime surface is purely additive and this repository's own consumer contract still compiles, but a measured compile break for a real consumer pattern is a breaking change, and the convention here is to say so rather than to reclassify it. Adds the migration note: take the new case, or stop treating the union as closed. BREAKING CHANGE: `entity.add` widens the public `Intent` discriminated union. Consumers performing exhaustive `switch` checks over `Intent['kind']` or `Intent['descriptor']` will fail to compile until they handle the new `entity.add` member.
The uniqueness guard was documented as a condition a writer might fail: "a writer that has not materialized has no basis to check." On the Runtime lane path there is no such condition. Runtime exposes lane, fork, strand, settle and close, none of which materialize, so the builder's basis is always null; and one intent lowers to one patch with validation ahead of the node add, so nothing precedes the entity in its own patch either. Both arms of the guard are unreachable there, which means E_PATCH_ENTITY_EXISTS never fires on the only write path the CLI, the MCP boundary and Runtime consumers have. docs/topics/cli.md carried the sharpest instance: it documented a failure mode for `git warp write --lane` that cannot occur. The concurrency test asserted the frontier case without creating it. Both writers opened and closed sequentially, so it passed for the same reason as the no-basis test beside it, isolating one mechanism while naming two. It now walks reachability outwards from the tightest case: one writer re-creating an id on one lane, two writers holding a shared frontier open simultaneously, and a writer opening only after the first creation is durable. All three are admitted, and the second still proves the merge and the two-dot cone. No behaviour changes. 7,336 unit tests pass.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds ChangesEntity capture and occurrence flow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Release Preflight
If this PR is from a |
Code Lawyer self-audit findings
No reviewer threads exist on this Draft PR; these are self-discovered. I am continuing the audit and will handle the queue one defect at a time with RED → GREEN → VERIFY → COMMIT. @codex Please provide a second opinion, especially on the strand settlement identity drift and counter-exhaustion boundary. |
Code Lawyer self-audit addendum
Local reproduction result: {"a":{"lane":"lane-a","subject":"entry:7772697465723a31"},"b":{"lane":"lane-b","subject":"entry:7772697465723a31"},"idsDistinct":true,"relation":"same"}The fix must scope occurrence identity and causal comparison to the worldline instead of asserting cross-worldline dot equality. @codex second opinion requested. |
Code Lawyer self-audit addendum
The committed behavioral fix will not be amended. I will restore the enforced source budget in a separate mechanical commit, then repush both commits. @codex second opinion requested. |
Code Lawyer self-audit addendum
These join the already published queue. @codex second opinion requested. |
Code Lawyer Activity Summary
Every item followed an isolated RED → GREEN → VERIFY → COMMIT cycle. No PR-originated review threads existed, so no thread-resolution mutation was applicable. Local exact-head evidence:
Hosted checks are running against that exact head. |
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: 6
🤖 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/Intent.ts`:
- Around line 196-232: Update entityIdentity to determine hasSubject and
hasNamespace from whether fields.subject and fields.namespace are not undefined,
rather than using key-presence checks. Preserve the exactly-one-identity
validation and existing non-empty string validation so an explicitly undefined
optional identity is treated as absent.
In `@src/domain/api/WriteReceipt.ts`:
- Around line 52-67: The admitted-outcome check in validateOccurrence must
explicitly accept only derived and plural, rather than treating every
non-obstruction outcome as admitted; update src/domain/api/WriteReceipt.ts lines
52-67 accordingly. Add a test in test/unit/domain/ReceiptOutcome.test.ts lines
120-133 covering an entity.add receipt with a conflict outcome, asserting the
intended behavior when the occurrence is present and absent.
In `@src/domain/api/WriteRuntime.ts`:
- Around line 192-205: Update the publication validation in the entity-write
flow around the leading NodeAdd check to hydrate the complete patch as an
entity.add capture before calling createEntityOccurrence. Reject patches that do
not hydrate as entity.add, and when a subject was supplied require the recovered
subject to match it; use WarpError code E_WRITE_ENTITY_OCCURRENCE for all
mismatches while preserving the existing occurrence creation path for valid
publications.
In `@src/domain/crdt/Dot.ts`:
- Around line 86-87: Update the validation error in Dot’s counter check to say
“counter must be a positive safe integer,” and update the corresponding message
assertions in Dot tests to match the new wording.
In `@src/domain/services/PatchBuilderEntity.ts`:
- Around line 126-135: Validate properties in the entity payload handling before
calling Object.entries: accept only plain objects or null-prototype records, and
reject strings, arrays, class instances, and other non-record values with the
existing PatchError pattern. Update the surrounding logic in PatchBuilderEntity
to perform this boundary validation while preserving the empty-record check and
deterministic key sorting for valid payloads.
In `@test/unit/cli/v19-entity-intent.test.ts`:
- Around line 10-15: Move the test named “documents the JSON and TypeScript
occurrence surfaces separately” out of the intent-parsing suite into a dedicated
documentation test file or suite. Keep its existing docs/topics/cli.md
assertions there, and leave the intentFromValue/intentFromText tests focused
only on intent parsing 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 Plus
Run ID: 70c61036-5767-4881-a064-392796bf1d2d
📒 Files selected for processing (37)
CHANGELOG.mdbin/cli/v19/V19DomainInput.tsbin/presenters/V19ReadingReceipt.tsdocs/READINGS_AND_OPTICS.mddocs/topics/cli.mddocs/topics/reference.mdindex.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/EntityOccurrenceRuntime.tssrc/domain/api/Intent.tssrc/domain/api/IntentBuilders.tssrc/domain/api/IntentRuntime.tssrc/domain/api/WriteReceipt.tssrc/domain/api/WriteRuntime.tssrc/domain/crdt/Dot.tssrc/domain/crdt/VersionVector.tssrc/domain/services/PatchBuilder.tssrc/domain/services/PatchBuilderContent.tssrc/domain/services/PatchBuilderEntity.tssrc/domain/services/PatchBuilderValidation.tstest/integration/application/Runtime.entityCapture.concurrent.test.tstest/integration/application/Runtime.entityCapture.integration.test.tstest/integration/application/Runtime.entityOccurrence.integration.test.tstest/type-check/v19-subpaths.tstest/unit/cli/v19-entity-intent.test.tstest/unit/domain/EntityOccurrence.test.tstest/unit/domain/Intent.entity.test.tstest/unit/domain/IntentRuntime.entity.test.tstest/unit/domain/ReceiptOutcome.test.tstest/unit/domain/WriteRuntime.test.tstest/unit/domain/crdt/Dot.test.tstest/unit/domain/crdt/VersionVector.test.tstest/unit/domain/services/PatchBuilder.entity.test.tstest/unit/scripts/entity-capture-type-assertion-ratchet.test.tstest/unit/scripts/v19-public-api-boundary.test.tsvitest.config.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not use direct imports fromsrc/infrastructure/**insrc/domain/**orsrc/ports/**; depend on a port instead.
Do not use direct Node built-ins insrc/domain/**orsrc/ports/**; use a port instead.
Files:
index.tstest/unit/scripts/v19-public-api-boundary.test.tstest/unit/domain/crdt/VersionVector.test.tstest/unit/domain/crdt/Dot.test.tsbin/presenters/V19ReadingReceipt.tssrc/domain/api/DraftTimelineRuntime.tsvitest.config.tssrc/domain/crdt/Dot.tstest/unit/domain/ReceiptOutcome.test.tssrc/domain/crdt/VersionVector.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/EntityOccurrenceRuntime.tstest/unit/domain/EntityOccurrence.test.tstest/unit/cli/v19-entity-intent.test.tstest/integration/application/Runtime.entityCapture.integration.test.tssrc/domain/services/PatchBuilderValidation.tssrc/domain/services/PatchBuilderContent.tstest/unit/domain/WriteRuntime.test.tssrc/domain/api/WriteRuntime.tstest/unit/domain/IntentRuntime.entity.test.tssrc/domain/api/IntentBuilders.tstest/unit/domain/services/PatchBuilder.entity.test.tssrc/domain/services/PatchBuilderEntity.tstest/type-check/v19-subpaths.tstest/unit/domain/Intent.entity.test.tssrc/domain/services/PatchBuilder.tstest/integration/application/Runtime.entityOccurrence.integration.test.tssrc/domain/api/IntentRuntime.tssrc/domain/api/Intent.tstest/unit/scripts/entity-capture-type-assertion-ratchet.test.tssrc/domain/api/WriteReceipt.tsbin/cli/v19/V19DomainInput.tstest/integration/application/Runtime.entityCapture.concurrent.test.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,js,jsx}: Do not introduceany,as any,as unknown as,unknown(outside adapters),Record<string, unknown>(outside adapters),*Likeplaceholder types,JSON.parse/JSON.stringify(outside adapters),fetch(outside adapters),process.env(outside adapters),@ts-ignore, orz.any()in core code; use validated boundary models and ports instead.
Use constructor-injected ports for external capabilities; do not rely on ambient dependencies for I/O, clocks, persistence, or entropy.
Do not createutils.ts,helpers.ts,misc.ts, orcommon.ts; name files after the actual concept they model.
Prefer one file per class, type, or object; if a file accumulates peer concepts, split it.
Keep helper corridors, fake shape trust, transitional duplication, and compile-time theater out of the codebase; runtime-honest TypeScript must reflect actual behavior.
No enum usage; prefer runtime-backed domain forms and unions.
Do not use boolean trap parameters; prefer named option objects or separate methods.
Avoid magic strings or numbers when a named constant should exist.
Keep domain bytes asUint8Array;Bufferbelongs in infrastructure adapters.
Files:
src/domain/api/DraftTimelineRuntime.tssrc/domain/crdt/Dot.tssrc/domain/crdt/VersionVector.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/EntityOccurrenceRuntime.tssrc/domain/services/PatchBuilderValidation.tssrc/domain/services/PatchBuilderContent.tssrc/domain/api/WriteRuntime.tssrc/domain/api/IntentBuilders.tssrc/domain/services/PatchBuilderEntity.tssrc/domain/services/PatchBuilder.tssrc/domain/api/IntentRuntime.tssrc/domain/api/Intent.tssrc/domain/api/WriteReceipt.ts
src/domain/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/domain/**/*.{ts,tsx,js,jsx}: Insrc/domain/**, do not useDate.now(),new Date(),Date(),performance.now(),Math.random(),crypto.randomUUID(),crypto.getRandomValues(),setTimeout,setInterval, rawnew Error(...)/new TypeError(...), or direct imports from Node built-ins; time, entropy, and external capabilities must enter through ports or parameters, and domain errors should extendWarpError.
Construct domain objects only in core when doing so establishes validated runtime truth; do not build infrastructure adapters, host APIs, persistence implementations, wall clocks, or entropy sources inside core.
Prefer discriminated unions and explicit result types instead of boolean-flag bags, and model expected failures as return values rather than exceptions.
src/domain/must not import host APIs or Node-specific globals; hexagonal architecture boundaries are mandatory.
Domain code must not use the wall clock directly; time must enter through a port or parameter.
Files:
src/domain/api/DraftTimelineRuntime.tssrc/domain/crdt/Dot.tssrc/domain/crdt/VersionVector.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/EntityOccurrenceRuntime.tssrc/domain/services/PatchBuilderValidation.tssrc/domain/services/PatchBuilderContent.tssrc/domain/api/WriteRuntime.tssrc/domain/api/IntentBuilders.tssrc/domain/services/PatchBuilderEntity.tssrc/domain/services/PatchBuilder.tssrc/domain/api/IntentRuntime.tssrc/domain/api/Intent.tssrc/domain/api/WriteReceipt.ts
src/domain/**/!(*.test).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit domain concepts with validated constructors,
Object.freeze, andinstanceofdispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.
Files:
src/domain/api/DraftTimelineRuntime.tssrc/domain/crdt/Dot.tssrc/domain/crdt/VersionVector.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/EntityOccurrenceRuntime.tssrc/domain/services/PatchBuilderValidation.tssrc/domain/services/PatchBuilderContent.tssrc/domain/api/WriteRuntime.tssrc/domain/api/IntentBuilders.tssrc/domain/services/PatchBuilderEntity.tssrc/domain/services/PatchBuilder.tssrc/domain/api/IntentRuntime.tssrc/domain/api/Intent.tssrc/domain/api/WriteReceipt.ts
🧠 Learnings (1)
📚 Learning: 2026-03-08T19:50:17.519Z
Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 65
File: CHANGELOG.md:88-88
Timestamp: 2026-03-08T19:50:17.519Z
Learning: Follow the Keep a Changelog convention for CHANGELOG.md. Allow duplicate subheadings across versions (e.g., '### Added', '### Fixed'). Configure markdownlint MD024 with {"siblings_only": true} to avoid cross-version false positives.
Applied to files:
CHANGELOG.md
🪛 LanguageTool
docs/READINGS_AND_OPTICS.md
[style] ~284-~284: Consider an alternative for the overused word “exactly”.
Context: ...has no history,"_ and that ambiguity is exactly what let the Think census (§12) go unno...
(EXACTLY_PRECISELY)
🔇 Additional comments (36)
CHANGELOG.md (1)
10-110: LGTM!src/domain/api/Intent.ts (2)
5-11: LGTM!Also applies to: 39-45, 59-66, 103-109, 141-143
234-258: LGTM!src/domain/api/IntentBuilders.ts (1)
2-4: LGTM!Also applies to: 14-17, 32-35
bin/cli/v19/V19DomainInput.ts (1)
59-67: LGTM!Also applies to: 103-131
test/unit/cli/v19-entity-intent.test.ts (1)
17-81: LGTM!test/unit/domain/Intent.entity.test.ts (1)
1-154: LGTM!test/type-check/v19-subpaths.ts (1)
9-10: LGTM!Also applies to: 47-50, 62-66, 91-98
src/domain/api/EntityOccurrence.ts (1)
1-63: LGTM!src/domain/api/EntityOccurrenceRuntime.ts (1)
1-125: LGTM!src/domain/api/WriteReceipt.ts (1)
8-9: LGTM!Also applies to: 18-18, 29-29, 44-49, 69-79
test/unit/domain/ReceiptOutcome.test.ts (1)
5-12: LGTM!Also applies to: 135-204, 250-259
docs/topics/cli.md (1)
42-87: LGTM!index.ts (1)
46-49: LGTM!docs/topics/reference.md (1)
39-62: LGTM!test/unit/scripts/v19-public-api-boundary.test.ts (1)
14-15: LGTM!test/integration/application/Runtime.entityCapture.integration.test.ts (1)
1-117: LGTM!test/integration/application/Runtime.entityCapture.concurrent.test.ts (1)
1-145: LGTM!test/unit/scripts/entity-capture-type-assertion-ratchet.test.ts (1)
1-40: LGTM!vitest.config.ts (1)
33-33: LGTM!src/domain/services/PatchBuilderEntity.ts (1)
65-99: LGTM!src/domain/services/PatchBuilder.ts (1)
24-40: LGTM!Also applies to: 255-259, 313-318, 347-352
src/domain/api/IntentRuntime.ts (1)
3-3: LGTM!Also applies to: 16-16, 33-146, 232-239
src/domain/api/DraftTimelineRuntime.ts (1)
328-346: LGTM!test/unit/domain/IntentRuntime.entity.test.ts (1)
1-198: LGTM!test/unit/domain/services/PatchBuilder.entity.test.ts (1)
1-190: LGTM!bin/presenters/V19ReadingReceipt.ts (1)
51-56: LGTM!test/integration/application/Runtime.entityOccurrence.integration.test.ts (1)
1-148: LGTM!test/unit/domain/EntityOccurrence.test.ts (1)
1-169: LGTM!src/domain/crdt/Dot.ts (1)
69-76: LGTM!src/domain/crdt/VersionVector.ts (1)
39-41: LGTM!Also applies to: 95-97, 124-124
src/domain/services/PatchBuilderContent.ts (1)
6-6: LGTM!Also applies to: 27-30, 41-56
docs/READINGS_AND_OPTICS.md (1)
1-503: LGTM!test/unit/domain/crdt/Dot.test.ts (1)
291-294: LGTM!test/unit/domain/crdt/VersionVector.test.ts (1)
64-72: LGTM!Also applies to: 359-360
src/domain/services/PatchBuilderValidation.ts (1)
27-29: 🗄️ Data Integrity & IntegrationNo change needed:
@warp/effect:is documented as the reserved node ID prefix, and existing regression coverage exercisesemitEffectunder that namespace.
|
@codex second opinion requested on one additional closure-gate finding.
The production path is untouched by PR #838; this is deterministic closure-test infrastructure. |
Code Lawyer Activity Summary — continuationAudited and published head: Issues 1–25 and their commits remain recorded in the first Activity Summary. This continuation closes every additional finding discovered during the renewed deep audit and closure gates.
Closure evidence
The technical queue is closed. The merge gate remains subject to a fresh exact-head CI/review/thread census and the required two approvals. |
Release Preflight
If this PR is from a |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
bin/cli/v19/V19DomainInput.ts (1)
13-21: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve
__proto__payloads through the CLI parser.Zod 3.24.1 records skip an own
__proto__key while assembling object output. If the CLI accepts entity JSON with bothsafeand__proto__,entityIntentFromreceives onlysafe, and the persisted capture differs from the submitted capture. Use a boundary decoder that preserves own keys in null-prototype objects, or reject these keys before parsing. Add raw-JSON regression cases for top-level and nested__proto__values.🤖 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 `@bin/cli/v19/V19DomainInput.ts` around lines 13 - 21, Update JSON_INPUT_SCHEMA parsing to preserve own __proto__ keys in both top-level and nested objects, preferably by decoding object records into null-prototype objects before entityIntentFrom receives them; alternatively reject __proto__ keys at the CLI boundary. Add raw-JSON regression coverage for top-level and nested __proto__ values, ensuring persisted captures match submitted JSON.src/domain/types/PropValue.ts (1)
174-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the duplicated record-equality body.
propValueRecordsEqualandentityCapturePayloadsEqualinsrc/domain/types/EntityCapturePayload.ts(lines 16-36) contain byte-identical logic. Both define payload identity semantics thatrequirePublishedEntityPayloadrelies on for authenticity checks. If one copy changes, the two identity rules diverge silently.Let
entityCapturePayloadsEqualdelegate to the record comparison exported from this module.🤖 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/PropValue.ts` around lines 174 - 194, Export the existing propValueRecordsEqual function from PropValue.ts so it can be reused, then update entityCapturePayloadsEqual in EntityCapturePayload.ts to delegate to that shared record-comparison implementation. Remove the duplicated equality logic while preserving the current identity semantics used by requirePublishedEntityPayload.
🤖 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 `@bin/cli/v19/V19DomainInput.ts`:
- Line 11: Remove the duplicate JsonInput type alias in V19DomainInput.ts,
keeping exactly one declaration of JsonInput so the module type-checks
successfully.
In `@src/domain/api/EntityOccurrence.ts`:
- Around line 68-79: Remove the self-comparison `issued.subject ===
occurrence.subject` from `EntityOccurrence.requireReceiptBinding`, since
`#requireIssued` returns the same occurrence and the check cannot validate a
receipt binding. Keep the existing evidence, intent, worldline, receipt-writer,
and return behavior unchanged.
In `@src/domain/api/EvidenceRuntime.ts`:
- Around line 201-209: Update isCanonicalRetentionEvidence to validate policy,
reachability, and rootKind using the existing RetentionEvidence field-validation
logic before accepting the object, while preserving the current frozen, key-set,
and witness checks. Add a regression test covering a frozen object with
RetentionEvidence.prototype, a valid witness, and an invalid retention field,
ensuring it is rejected and normal construction/validation proceeds.
In `@test/unit/cli/v19-entity-intent.test.ts`:
- Around line 56-75: Add a test in the entity.add intent cases that calls
intentFromValue with only kind and a non-empty properties record, omitting both
subject and namespace. Assert that entityIntentFrom rejects this no-identity
descriptor, while leaving the existing empty-subject and supplied/allocated
identity tests unchanged.
---
Outside diff comments:
In `@bin/cli/v19/V19DomainInput.ts`:
- Around line 13-21: Update JSON_INPUT_SCHEMA parsing to preserve own __proto__
keys in both top-level and nested objects, preferably by decoding object records
into null-prototype objects before entityIntentFrom receives them; alternatively
reject __proto__ keys at the CLI boundary. Add raw-JSON regression coverage for
top-level and nested __proto__ values, ensuring persisted captures match
submitted JSON.
In `@src/domain/types/PropValue.ts`:
- Around line 174-194: Export the existing propValueRecordsEqual function from
PropValue.ts so it can be reused, then update entityCapturePayloadsEqual in
EntityCapturePayload.ts to delegate to that shared record-comparison
implementation. Remove the duplicated equality logic while preserving the
current identity semantics used by requirePublishedEntityPayload.
🪄 Autofix
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 Plus
Run ID: 982263ae-5c67-488a-9309-5fc7366f305c
📒 Files selected for processing (42)
CHANGELOG.mdbin/cli/v19/V19DomainInput.tsbin/presenters/V19ReadingReceipt.tsdocs/READINGS_AND_OPTICS.mddocs/topics/cli.mdsrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/EntityOccurrenceRuntime.tssrc/domain/api/EvidenceRuntime.tssrc/domain/api/Intent.tssrc/domain/api/IntentRuntime.tssrc/domain/api/WriteReceipt.tssrc/domain/api/WriteRuntime.tssrc/domain/crdt/Dot.tssrc/domain/crdt/VersionVector.tssrc/domain/services/PatchBuilder.tssrc/domain/services/PatchBuilderContent.tssrc/domain/services/PatchBuilderEntity.tssrc/domain/services/PatchBuilderPropertyRuntime.tssrc/domain/services/PatchBuilderValidation.tssrc/domain/types/EntityCapturePayload.tssrc/domain/types/PropValue.tstest/integration/application/Runtime.entityCapture.concurrent.test.tstest/type-check/v19-subpaths.tstest/unit/cli/v19-entity-intent.test.tstest/unit/domain/EntityOccurrence.test.tstest/unit/domain/EvidenceRuntime.test.tstest/unit/domain/Intent.entity.test.tstest/unit/domain/IntentRuntime.entity.test.tstest/unit/domain/ReceiptOutcome.test.tstest/unit/domain/WriteRuntime.test.tstest/unit/domain/crdt/Dot.test.tstest/unit/domain/crdt/VersionVector.test.tstest/unit/domain/services/PatchBuilder.commit.test.tstest/unit/domain/services/PatchBuilder.entity.test.tstest/unit/domain/types/EntityCapturePayload.test.tstest/unit/infrastructure/adapters/WesleyDotCodecAdapter.test.tstest/unit/scripts/cli-entity-documentation.test.tstest/unit/scripts/entity-capture-doctrine.test.tstest/unit/scripts/entity-capture-type-assertion-ratchet.test.tstest/unit/scripts/v18-to-v19-finalization.test.tsvitest.config.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: preflight
- GitHub Check: v19 base/head performance
- GitHub Check: test-node (22)
- GitHub Check: coverage-threshold
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not use direct imports fromsrc/infrastructure/**insrc/domain/**orsrc/ports/**; depend on a port instead.
Do not use direct Node built-ins insrc/domain/**orsrc/ports/**; use a port instead.
Files:
test/unit/domain/EvidenceRuntime.test.tstest/integration/application/Runtime.entityCapture.concurrent.test.tstest/unit/scripts/entity-capture-doctrine.test.tstest/unit/infrastructure/adapters/WesleyDotCodecAdapter.test.tstest/unit/cli/v19-entity-intent.test.tstest/unit/domain/Intent.entity.test.tssrc/domain/crdt/Dot.tstest/type-check/v19-subpaths.tsbin/presenters/V19ReadingReceipt.tssrc/domain/types/PropValue.tstest/unit/domain/services/PatchBuilder.commit.test.tssrc/domain/api/EntityOccurrenceRuntime.tstest/unit/domain/EntityOccurrence.test.tsvitest.config.tstest/unit/domain/crdt/Dot.test.tssrc/domain/api/WriteReceipt.tssrc/domain/api/IntentRuntime.tstest/unit/domain/IntentRuntime.entity.test.tstest/unit/domain/ReceiptOutcome.test.tstest/unit/scripts/v18-to-v19-finalization.test.tssrc/domain/api/EvidenceRuntime.tssrc/domain/api/Intent.tssrc/domain/types/EntityCapturePayload.tssrc/domain/services/PatchBuilderContent.tstest/unit/domain/WriteRuntime.test.tssrc/domain/api/DraftTimelineRuntime.tstest/unit/domain/types/EntityCapturePayload.test.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/WriteRuntime.tssrc/domain/services/PatchBuilderEntity.tstest/unit/domain/services/PatchBuilder.entity.test.tstest/unit/scripts/cli-entity-documentation.test.tstest/unit/scripts/entity-capture-type-assertion-ratchet.test.tssrc/domain/services/PatchBuilderValidation.tssrc/domain/crdt/VersionVector.tsbin/cli/v19/V19DomainInput.tssrc/domain/services/PatchBuilderPropertyRuntime.tstest/unit/domain/crdt/VersionVector.test.tssrc/domain/services/PatchBuilder.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,js,jsx}: Do not introduceany,as any,as unknown as,unknown(outside adapters),Record<string, unknown>(outside adapters),*Likeplaceholder types,JSON.parse/JSON.stringify(outside adapters),fetch(outside adapters),process.env(outside adapters),@ts-ignore, orz.any()in core code; use validated boundary models and ports instead.
Use constructor-injected ports for external capabilities; do not rely on ambient dependencies for I/O, clocks, persistence, or entropy.
Do not createutils.ts,helpers.ts,misc.ts, orcommon.ts; name files after the actual concept they model.
Prefer one file per class, type, or object; if a file accumulates peer concepts, split it.
Keep helper corridors, fake shape trust, transitional duplication, and compile-time theater out of the codebase; runtime-honest TypeScript must reflect actual behavior.
No enum usage; prefer runtime-backed domain forms and unions.
Do not use boolean trap parameters; prefer named option objects or separate methods.
Avoid magic strings or numbers when a named constant should exist.
Keep domain bytes asUint8Array;Bufferbelongs in infrastructure adapters.
Files:
src/domain/crdt/Dot.tssrc/domain/types/PropValue.tssrc/domain/api/EntityOccurrenceRuntime.tssrc/domain/api/WriteReceipt.tssrc/domain/api/IntentRuntime.tssrc/domain/api/EvidenceRuntime.tssrc/domain/api/Intent.tssrc/domain/types/EntityCapturePayload.tssrc/domain/services/PatchBuilderContent.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/WriteRuntime.tssrc/domain/services/PatchBuilderEntity.tssrc/domain/services/PatchBuilderValidation.tssrc/domain/crdt/VersionVector.tssrc/domain/services/PatchBuilderPropertyRuntime.tssrc/domain/services/PatchBuilder.ts
src/domain/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/domain/**/*.{ts,tsx,js,jsx}: Insrc/domain/**, do not useDate.now(),new Date(),Date(),performance.now(),Math.random(),crypto.randomUUID(),crypto.getRandomValues(),setTimeout,setInterval, rawnew Error(...)/new TypeError(...), or direct imports from Node built-ins; time, entropy, and external capabilities must enter through ports or parameters, and domain errors should extendWarpError.
Construct domain objects only in core when doing so establishes validated runtime truth; do not build infrastructure adapters, host APIs, persistence implementations, wall clocks, or entropy sources inside core.
Prefer discriminated unions and explicit result types instead of boolean-flag bags, and model expected failures as return values rather than exceptions.
src/domain/must not import host APIs or Node-specific globals; hexagonal architecture boundaries are mandatory.
Domain code must not use the wall clock directly; time must enter through a port or parameter.
Files:
src/domain/crdt/Dot.tssrc/domain/types/PropValue.tssrc/domain/api/EntityOccurrenceRuntime.tssrc/domain/api/WriteReceipt.tssrc/domain/api/IntentRuntime.tssrc/domain/api/EvidenceRuntime.tssrc/domain/api/Intent.tssrc/domain/types/EntityCapturePayload.tssrc/domain/services/PatchBuilderContent.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/WriteRuntime.tssrc/domain/services/PatchBuilderEntity.tssrc/domain/services/PatchBuilderValidation.tssrc/domain/crdt/VersionVector.tssrc/domain/services/PatchBuilderPropertyRuntime.tssrc/domain/services/PatchBuilder.ts
src/domain/**/!(*.test).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit domain concepts with validated constructors,
Object.freeze, andinstanceofdispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.
Files:
src/domain/crdt/Dot.tssrc/domain/types/PropValue.tssrc/domain/api/EntityOccurrenceRuntime.tssrc/domain/api/WriteReceipt.tssrc/domain/api/IntentRuntime.tssrc/domain/api/EvidenceRuntime.tssrc/domain/api/Intent.tssrc/domain/types/EntityCapturePayload.tssrc/domain/services/PatchBuilderContent.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/WriteRuntime.tssrc/domain/services/PatchBuilderEntity.tssrc/domain/services/PatchBuilderValidation.tssrc/domain/crdt/VersionVector.tssrc/domain/services/PatchBuilderPropertyRuntime.tssrc/domain/services/PatchBuilder.ts
🧠 Learnings (1)
📚 Learning: 2026-03-08T19:50:17.519Z
Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 65
File: CHANGELOG.md:88-88
Timestamp: 2026-03-08T19:50:17.519Z
Learning: Follow the Keep a Changelog convention for CHANGELOG.md. Allow duplicate subheadings across versions (e.g., '### Added', '### Fixed'). Configure markdownlint MD024 with {"siblings_only": true} to avoid cross-version false positives.
Applied to files:
CHANGELOG.md
🔇 Additional comments (45)
src/domain/services/PatchBuilderPropertyRuntime.ts (2)
100-144: 🩺 Stability & AvailabilityVerify post-commit protection after content staging.
attachNodeContentandattachEdgeContentappend operations and asset handles afterawait stageContentAttachment(...). IfPatchBuilder.commit()completes while staging is pending, these methods can mutate a committed patch. Serialize pending attachment work incommit(), or check the lifecycle again before lowering the attachment.#!/bin/bash set -euo pipefail ast-grep outline src/domain/services/PatchBuilder.ts --items all rg -n -C 12 '\b(attachContent|attachNodeContent|attachEdgeContent|commit|_assertNotCommitted|_contentAssets|_ops)\b' \ src/domain/services/PatchBuilder.ts \ src/domain/services/PatchBuilderPropertyRuntime.ts
1-98: LGTM!Also applies to: 119-126, 146-222
src/domain/services/PatchBuilderContent.ts (1)
27-31: LGTM!Also applies to: 48-58, 60-112
src/domain/services/PatchBuilderValidation.ts (1)
17-31: LGTM!Also applies to: 66-99, 114-157
test/unit/domain/services/PatchBuilder.commit.test.ts (1)
11-11: LGTM!Also applies to: 86-86, 112-118, 179-179
src/domain/api/EntityOccurrence.ts (1)
1-27: LGTM!Also applies to: 37-65, 81-120, 122-178, 180-224
src/domain/api/EntityOccurrenceRuntime.ts (1)
4-7: LGTM!Also applies to: 27-53
src/domain/api/WriteReceipt.ts (1)
49-55: LGTM!Also applies to: 57-77, 79-90
src/domain/api/WriteRuntime.ts (1)
26-26: LGTM!Also applies to: 190-215, 217-229, 247-254
bin/presenters/V19ReadingReceipt.ts (1)
15-17: LGTM!Also applies to: 46-56, 70-75, 95-95, 106-125, 179-198
test/unit/domain/EntityOccurrence.test.ts (1)
83-87: LGTM!Also applies to: 98-111, 113-176
test/unit/domain/ReceiptOutcome.test.ts (1)
9-31: LGTM!Also applies to: 136-196, 215-215, 271-302, 353-398
test/unit/domain/WriteRuntime.test.ts (2)
345-359: 📐 Maintainability & Code Quality | 💤 Low valueConfirm the declared type of
Patch.writesmatches thestring[] | undefinedparameter.Line 348 defaults
writestopatch.writes, and line 363 assignspatch.writes?.map(...)into the same parameter type. IfPatchdeclareswritesasreadonly string[] | undefined, the default assignment does not type-check. Widen the parameter toreadonly string[] | undefinedin that case.#!/bin/bash set -euo pipefail fd -t f 'Patch.ts' src/domain/types --exec ast-grep outline {} --items all rg -n -C 3 '\bwrites\b' src/domain/types/Patch.ts
57-98: LGTM!Also applies to: 100-157, 159-206, 361-365, 380-385, 397-408
vitest.config.ts (1)
12-13: LGTM!Also applies to: 25-25
test/unit/scripts/cli-entity-documentation.test.ts (1)
8-8: LGTM!test/unit/scripts/entity-capture-doctrine.test.ts (1)
1-44: LGTM!test/unit/scripts/entity-capture-type-assertion-ratchet.test.ts (1)
10-11: LGTM!Also applies to: 24-76, 78-108, 122-129
test/unit/scripts/v18-to-v19-finalization.test.ts (1)
25-25: LGTM!Also applies to: 115-166
src/domain/crdt/Dot.ts (1)
60-60: LGTM!Also applies to: 71-89, 127-127, 150-157
src/domain/crdt/VersionVector.ts (1)
95-97: LGTM!Also applies to: 124-124, 141-158, 175-177, 228-230
src/domain/api/EvidenceRuntime.ts (1)
32-32: LGTM!Also applies to: 261-303
test/unit/domain/crdt/Dot.test.ts (1)
2-3: LGTM!Also applies to: 35-60, 187-202, 315-320
test/unit/domain/crdt/VersionVector.test.ts (1)
63-73: LGTM!Also applies to: 324-340, 375-381
test/unit/infrastructure/adapters/WesleyDotCodecAdapter.test.ts (1)
16-28: LGTM!Also applies to: 65-67
test/unit/domain/EvidenceRuntime.test.ts (1)
1-84: LGTM!CHANGELOG.md (1)
10-148: LGTM!docs/READINGS_AND_OPTICS.md (1)
75-90: LGTM!Also applies to: 197-221, 255-270, 468-469
src/domain/api/Intent.ts (1)
24-28: LGTM!Also applies to: 201-223, 224-241, 254-279
bin/cli/v19/V19DomainInput.ts (1)
24-65: LGTM!Also applies to: 71-215
docs/topics/cli.md (1)
42-90: LGTM!test/type-check/v19-subpaths.ts (1)
51-60: LGTM!test/unit/cli/v19-entity-intent.test.ts (1)
3-54: LGTM!Also applies to: 77-86
test/unit/domain/Intent.entity.test.ts (1)
8-8: LGTM!Also applies to: 23-27, 47-52, 76-80, 101-150, 176-181
test/integration/application/Runtime.entityCapture.concurrent.test.ts (1)
91-91: LGTM!Also applies to: 105-111
src/domain/types/EntityCapturePayload.ts (1)
16-36: LGTM!src/domain/types/PropValue.ts (1)
126-130: LGTM!Also applies to: 151-165
test/unit/domain/types/EntityCapturePayload.test.ts (1)
27-32: LGTM!Also applies to: 57-72
src/domain/api/IntentRuntime.ts (1)
51-65: LGTM!Also applies to: 87-117, 119-140, 142-146, 161-170, 182-182
src/domain/services/PatchBuilder.ts (1)
79-123: LGTM!Also applies to: 134-140, 153-175, 185-232, 262-262, 272-298, 302-339, 370-373, 388-406
src/domain/services/PatchBuilderEntity.ts (1)
2-11: LGTM!Also applies to: 105-131, 168-186
src/domain/api/DraftTimelineRuntime.ts (2)
110-110: LGTM!Also applies to: 135-144, 161-163
304-313: 🗄️ Data Integrity & IntegrationVerify the stored draft intent after the
intentFromPatchround trip.The change summary states that
writeDraftIntentnow derives the stored intent withintentFromPatch(publication.patch)instead of appending the submitted intent. ThewriteDraftIntentbody is not included in this review context, so I cannot confirm the effect.The recovery path loses the allocation mode.
entityIntentForinsrc/domain/api/IntentRuntime.tsalways returnsIntent.addEntity({ subject, properties }); there is noaddEntityAutorecovery branch. A caller that submitsIntent.addEntityAuto({ namespace, properties })therefore gets an explicit-subjectentity.addintent stored in the draft. The stored subject carries the original writer's dot.If any code replays persisted draft intents, confirm that replaying a recovered
addEntityreproduces the intended subject and does not reuse a foreign writer's allocated id.#!/bin/bash # Description: Inspect writeDraftIntent and any replay path for persisted draft intents. set -euo pipefail ast-grep outline src/domain/api/DraftTimelineRuntime.ts --items all echo '--- writeDraftIntent body ---' ast-grep run --lang typescript --pattern 'async function writeDraftIntent($$$) { $$$ }' src/domain/api/DraftTimelineRuntime.ts echo '--- persisted draft entry shape and consumers ---' rg -nP -C6 '\bWarpDraftPatchEntry\b' --type=ts echo '--- consumers of persisted draft intents ---' rg -nP -C6 '\bintentFromPatch\s*\(' --type=ts echo '--- any addEntityAuto recovery branch ---' rg -nP -C4 '\baddEntityAuto\b' --type=tstest/unit/domain/IntentRuntime.entity.test.ts (1)
4-4: LGTM!Also applies to: 13-22, 32-38, 53-61, 76-121, 123-176, 178-208, 220-228, 243-252
test/unit/domain/services/PatchBuilder.entity.test.ts (1)
19-41: LGTM!Also applies to: 43-43, 57-102, 147-151
Code Lawyer Activity Summary — exact-head continuationCumulative audit history: issues 1–25 and issues 26–38. This table closes every finding raised by the CodeRabbit review of
Exact-head validation
No unresolved local defect remains. Merge eligibility is pending the exact-head hosted check and review census. |
Release Preflight
If this PR is from a |
Code Lawyer merge-gate closure
MERGE GATE: LOCKED. Code and validation are clean; review authority is not. Obtain two current approvals and an exact-head CodeRabbit disposition after cooldown, then rerun the thread/review/check census. No merge was attempted. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/domain/services/PatchBuilderPropertyRuntime.ts (1)
109-115: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject a committed builder before asset staging.
Lines 115 and 143 check mutability only after
stageContentAttachmentcompletes. A post-commit call can create an unreferenced staged asset before it receivesE_PATCH_ALREADY_COMMITTED.Call
assertMutable()before staging. Keep the existing post-awaitcheck to reject a commit that occurs during staging. Add a regression test that confirmsassetStorage.stage()is not called when the builder is already committed.Also applies to: 137-143
🤖 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/PatchBuilderPropertyRuntime.ts` around lines 109 - 115, In the attachment-building flow of PatchBuilderPropertyRuntime, call this.#options.assertMutable() before each stageContentAttachment invocation so an already committed builder fails before staging; retain the existing post-await assertions to catch commits occurring during staging. Add a regression test verifying assetStorage.stage() is not called when the builder is already committed.test/unit/cli/v19-entity-intent.test.ts (1)
20-30: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the complete JSON conversion payload.
The test passes if
intentFromTextpreserves onlykindand dropssubjectorproperties. Assert the full descriptor, includingsubject: 'entry:1'andproperties: { count: 1 }.Proposed assertion
- ).kind - ).toBe('entity.add'); + ).descriptor + ).toEqual({ + kind: 'entity.add', + subject: 'entry:1', + properties: { count: 1 }, + });🤖 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/cli/v19-entity-intent.test.ts` around lines 20 - 30, Update the test case “accepts the same entity capture as JSON text” to assert the complete result returned by intentFromText, including kind, subject: 'entry:1', and properties: { count: 1 }, rather than checking kind alone.src/domain/services/PatchBuilder.ts (1)
273-285: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winConstrain
emitEffectpayload type withCanonicalJsonValue.
canonicalStringifyaccepts arbitraryunknown, accepts shared references/diamonds, and silently omits function/symbol object values. Let callers compile unsupported payloads that silently round-trip differently or store wrong JSON. Usepayload?: Readonly<CanonicalJsonValue>or the exact repository payload type.🤖 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 273 - 285, Update the emitEffect method’s payload parameter to use Readonly<CanonicalJsonValue> (or the repository’s equivalent canonical JSON payload type) instead of the unconstrained generic T. Preserve the existing optional-payload handling and canonicalStringify call while preventing unsupported values, shared references, and non-JSON values from compiling as effect payloads.Source: Coding guidelines
🤖 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/MachineLocalPathPolicy.ts`:
- Around line 9-13: Update WINDOWS_HOME_PATTERN so the Windows “Users” directory
segment matches case-insensitively, while preserving the existing path
boundaries and username rules. Add a regression case covering a lowercase
variant such as C:\users\alice\repo through
MachineLocalPathPolicy.containsMachineLocalPath.
In `@test/unit/scripts/entity-capture-type-assertion-ratchet.test.ts`:
- Line 52: Replace the string-based assertion around occurrence in
entity-capture-type-assertion-ratchet with a TypeScript AST-based check that
resolves EntityOccurrence.ts and detects equality expressions comparing
issued.subject and occurrence.subject in either operand order, regardless of
whitespace or formatting. Keep the test failing whenever that semantic authority
check is present.
---
Outside diff comments:
In `@src/domain/services/PatchBuilder.ts`:
- Around line 273-285: Update the emitEffect method’s payload parameter to use
Readonly<CanonicalJsonValue> (or the repository’s equivalent canonical JSON
payload type) instead of the unconstrained generic T. Preserve the existing
optional-payload handling and canonicalStringify call while preventing
unsupported values, shared references, and non-JSON values from compiling as
effect payloads.
In `@src/domain/services/PatchBuilderPropertyRuntime.ts`:
- Around line 109-115: In the attachment-building flow of
PatchBuilderPropertyRuntime, call this.#options.assertMutable() before each
stageContentAttachment invocation so an already committed builder fails before
staging; retain the existing post-await assertions to catch commits occurring
during staging. Add a regression test verifying assetStorage.stage() is not
called when the builder is already committed.
In `@test/unit/cli/v19-entity-intent.test.ts`:
- Around line 20-30: Update the test case “accepts the same entity capture as
JSON text” to assert the complete result returned by intentFromText, including
kind, subject: 'entry:1', and properties: { count: 1 }, rather than checking
kind alone.
🪄 Autofix
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 Plus
Run ID: acbeed80-98cc-40e9-91bd-a8b961133ac2
📒 Files selected for processing (18)
.github/workflows/ci.ymlAGENTS.mdCHANGELOG.mdpackage.jsonscripts/MachineLocalPathPolicy.tsscripts/check-machine-local-paths.tssrc/domain/api/EntityOccurrence.tssrc/domain/api/EvidenceRuntime.tssrc/domain/api/RetentionEvidence.tssrc/domain/services/PatchBuilder.tssrc/domain/services/PatchBuilderPropertyRuntime.tstest/fixtures/generated-sdk/README.mdtest/unit/cli/v19-entity-intent.test.tstest/unit/domain/EvidenceRuntime.test.tstest/unit/domain/services/PatchBuilder.commit.test.tstest/unit/scripts/entity-capture-type-assertion-ratchet.test.tstest/unit/scripts/machine-local-path-policy.test.tstest/unit/scripts/wesley-ci-install-source.test.ts
💤 Files with no reviewable changes (1)
- src/domain/api/EntityOccurrence.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: test-deno
- GitHub Check: test-node (22)
- GitHub Check: coverage-threshold
- GitHub Check: type-firewall-generated-sdk
- GitHub Check: v19 base/head performance
- GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: In TypeScript, rejectany,as any,as unknown as,*Likeplaceholder types, and@ts-ignore; do not useunknownoutside infrastructure adapters or type assertions generally.
Prefer discriminated unions and explicit result types over boolean-flag bags; expected failures should be returned rather than represented as exceptions.
Do not construct infrastructure adapters, host APIs, persistence implementations, wall clocks, or entropy sources inside core code; use constructor-injected ports for external capabilities.
Do not use puddle-assembly object construction such as incremental property assignment with conditional fields; define explicit validated concepts instead.
When a boundary shape is unclear, define a port or transport DTO rather than inventing a fake domain model.
Prefer one file per class, type, or object; split files when peer concepts accumulate.
Validate inputs at boundaries and constructors; encoding and decoding belong in adapters, codec ports, or explicitly named boundary reader modules.
Preferinstanceofdispatch over tag switching.
Useinterfaceonly for ports; represent domain concepts as classes.
Do not use boolean trap parameters; use named option objects or separate methods.
Do not use magic strings or numbers when a named constant should exist.
Keep domain bytes asUint8Array; keepBufferin infrastructure adapters.
Keep source files at or below 500 LOC, test files at or below 800 LOC, and bin/scripts at or below 300 LOC.
Files:
scripts/MachineLocalPathPolicy.tstest/unit/scripts/wesley-ci-install-source.test.tstest/unit/cli/v19-entity-intent.test.tsscripts/check-machine-local-paths.tstest/unit/domain/EvidenceRuntime.test.tstest/unit/domain/services/PatchBuilder.commit.test.tstest/unit/scripts/machine-local-path-policy.test.tssrc/domain/api/RetentionEvidence.tssrc/domain/api/EvidenceRuntime.tstest/unit/scripts/entity-capture-type-assertion-ratchet.test.tssrc/domain/services/PatchBuilderPropertyRuntime.tssrc/domain/services/PatchBuilder.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run an SSJS scorecard for every slice; until automated, verify runtime-backed concepts, boundary validation, owning-type behavior, no message parsing for significant branching, no ambient time or entropy, and no fake shape trust or cast-cosplay.
Files:
scripts/MachineLocalPathPolicy.tstest/unit/scripts/wesley-ci-install-source.test.tstest/unit/cli/v19-entity-intent.test.tsscripts/check-machine-local-paths.tstest/unit/domain/EvidenceRuntime.test.tstest/unit/domain/services/PatchBuilder.commit.test.tstest/unit/scripts/machine-local-path-policy.test.tssrc/domain/api/RetentionEvidence.tssrc/domain/api/EvidenceRuntime.tstest/unit/scripts/entity-capture-type-assertion-ratchet.test.tssrc/domain/services/PatchBuilderPropertyRuntime.tssrc/domain/services/PatchBuilder.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Never amend, rebase, force git operations, or use destructive cleanup/history-rewrite commands; create new commits instead.
Do not publish machine-local absolute paths; use repository-relative paths,~,$HOME, or explicit placeholders.
At the end of a turn, stage only files written in that turn and commit those edits; do not leave self-authored changes staged and uncommitted.
Use GitHub Issues as the live work tracker and prefer precise issues for discoveries; do not rely on local backlog or deleted historical planning files.
Every open issue must have exactly one label from each live axis: type, priority, status, and area; use milestones for release targeting rather than release labels.
End every turn with the specified compact progress report, including goalpost, progress, branch divergence, and pull-request status.
Read the anti-sludge policy and systems-style TypeScript documentation before design-level changes.
Do not leave helper corridors, fake shape trust, or transitional duplication at the end of a slice.
Runnpm run test:local,npm run test:coverage,npm run lint, andnpm run typecheckas appropriate, and fix or explicitly surface every encountered error or warning.
Follow.github/RELEASE.mdand current release tooling; do not maintain a separate prose release dashboard.
Every planned versioned release must have a thesis in its GitHub Milestone description or linked tracking issue before implementation is marked active.
Releases require matching versions inpackage.json,package-lock.json,jsr.json, and private workspace package metadata.
Update changelog, README latest-release information, architecture posture, topic documentation, and operator documentation when a release diff changes their truth.
Files:
scripts/MachineLocalPathPolicy.tspackage.jsontest/fixtures/generated-sdk/README.mdAGENTS.mdtest/unit/scripts/wesley-ci-install-source.test.tstest/unit/cli/v19-entity-intent.test.tsscripts/check-machine-local-paths.tstest/unit/domain/EvidenceRuntime.test.tstest/unit/domain/services/PatchBuilder.commit.test.tstest/unit/scripts/machine-local-path-policy.test.tssrc/domain/api/RetentionEvidence.tssrc/domain/api/EvidenceRuntime.tstest/unit/scripts/entity-capture-type-assertion-ratchet.test.tssrc/domain/services/PatchBuilderPropertyRuntime.tsCHANGELOG.mdsrc/domain/services/PatchBuilder.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
For every refactor slice, touched code must reach 100% test coverage before completion.
Files:
test/unit/scripts/wesley-ci-install-source.test.tstest/unit/cli/v19-entity-intent.test.tstest/unit/domain/EvidenceRuntime.test.tstest/unit/domain/services/PatchBuilder.commit.test.tstest/unit/scripts/machine-local-path-policy.test.tstest/unit/scripts/entity-capture-type-assertion-ratchet.test.ts
src/domain/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/domain/**/*.ts: Domain code must not useDate.now(), date constructors,performance.now(), random or crypto entropy APIs, timers, rawError/TypeError, host APIs, Node globals,Buffer, or infrastructure imports; inject time and capabilities through ports or parameters and extendWarpErrorfor domain errors.
Use runtime-backed domain concepts: named classes with validated constructors,Object.freeze, andinstanceofdispatch; domain objects should establish validated runtime truth.
Files:
src/domain/api/RetentionEvidence.tssrc/domain/api/EvidenceRuntime.tssrc/domain/services/PatchBuilderPropertyRuntime.tssrc/domain/services/PatchBuilder.ts
🧠 Learnings (2)
📚 Learning: 2026-03-04T12:08:30.347Z
Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 63
File: package.json:126-126
Timestamp: 2026-03-04T12:08:30.347Z
Learning: Vitest 4 removes vite-node as a dependency and rewrites its pool system. Do not flag the absence of vite-node in package.json or lockfiles as an error for Vitest 4 projects. Use this as a general guideline: if a project uses Vitest 4, missing vite-node is expected and correct; only flag issues if there is evidence the project is not using Vitest 4 or if vite-node is explicitly required by the project.
Applied to files:
package.json
📚 Learning: 2026-03-08T19:50:17.519Z
Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 65
File: CHANGELOG.md:88-88
Timestamp: 2026-03-08T19:50:17.519Z
Learning: Follow the Keep a Changelog convention for CHANGELOG.md. Allow duplicate subheadings across versions (e.g., '### Added', '### Fixed'). Configure markdownlint MD024 with {"siblings_only": true} to avoid cross-version false positives.
Applied to files:
CHANGELOG.md
🪛 ast-grep (0.45.0)
scripts/MachineLocalPathPolicy.ts
[warning] 10-13: Do not use variable for regular expressions
Context: new RegExp(
[...POSIX_HOME_PATTERN, ...DARWIN_TEMP_PATTERN, WINDOWS_HOME_PATTERN].join('|'),
'u'
)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
scripts/check-machine-local-paths.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFileSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
test/unit/scripts/machine-local-path-policy.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFileSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🔇 Additional comments (15)
test/unit/domain/EvidenceRuntime.test.ts (1)
85-103: LGTM!src/domain/api/EvidenceRuntime.ts (1)
209-209: LGTM!src/domain/api/RetentionEvidence.ts (1)
32-94: LGTM!CHANGELOG.md (1)
10-159: LGTM!test/unit/scripts/wesley-ci-install-source.test.ts (1)
1-34: LGTM!test/fixtures/generated-sdk/README.md (1)
14-16: LGTM!test/unit/domain/services/PatchBuilder.commit.test.ts (1)
157-170: 🩺 Stability & AvailabilityNo change needed.
Promise.withResolversis supported by the declared Node.js and TypeScript targets, andvi.waitForis not the barrier here; the mockedstageresolvesstaged.promisedeterministically.> Likely an incorrect or invalid review comment.package.json (1)
77-77: LGTM!Also applies to: 91-91
scripts/check-machine-local-paths.ts (1)
1-44: LGTM!.github/workflows/ci.yml (1)
130-132: LGTM!AGENTS.md (1)
109-112: LGTM!test/unit/cli/v19-entity-intent.test.ts (1)
6-18: LGTM!Also applies to: 32-44, 46-95, 96-96
src/domain/services/PatchBuilder.ts (3)
17-31: LGTM!Also applies to: 79-96, 106-123, 137-143, 154-162, 186-193, 202-222, 231-231, 263-263, 289-297, 303-340, 389-404, 407-407
371-374: 🗄️ Data Integrity & IntegrationNo mutable
contentAssetsexposure.
PatchBuilderPropertyRuntime.contentAssetsreturns a copy, socommitWithEvidencedoes not pass the property runtime’s mutable collection intocommitPatch.
163-176: 🗄️ Data Integrity & IntegrationNo change needed.
addEntityAutoreceives the allocated dot and advances the sameVersionVectoronce before returning it.
| expect(runtime).not.toMatch(/\bWeakMap\b/); | ||
| expect(occurrence).not.toContain('readonly #compare'); | ||
| expect(occurrence).not.toContain('readonly #relationTo'); | ||
| expect(occurrence).not.toContain('issued.subject === occurrence.subject'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'entity-capture-type-assertion-ratchet|payload|scripts' || true
echo "== target file =="
if [ -f test/unit/scripts/entity-capture-type-assertion-ratchet.test.ts ]; then
wc -l test/unit/scripts/entity-capture-type-assertion-ratchet.test.ts
cat -n test/unit/scripts/entity-capture-type-assertion-ratchet.test.ts | sed -n '1,140p'
fi
echo "== related source names =="
fd -i 'entity|capture|assertion|ratchet|payload' . -t f | sed -n '1,120p'
echo "== search source-text checks =="
rg -n "issued\.subject \=\=|=|occurrence\.subject|not\.toContain|payload" test src --glob '*.{ts,tsx}' || trueRepository: git-stunts/git-warp
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate target file exactly =="
git ls-files | grep -F 'test/unit/scripts/entity-capture-type-assertion-ratchet.test.ts' || true
echo "== files by basename =="
fd -a 'entity-capture' . 2>/dev/null | sed 's#^\./##' | head -120 || true
fd -a 'capture-type-assertion' . 2>/dev/null | sed 's#^\./##' | head -120 || true
echo "== exact search for substring =="
rg -n --fixed-strings 'issued.subject === occurrence.subject' . || true
echo "== exact search for occurrence subject == issued subject =="
rg -n --fixed-strings 'occurrence.subject === issued.subject' . || true
echo "== exact search for noContainedOccurrenceRatchet =="
rg -n --fixed-strings 'noContainedOccurrence' . || trueRepository: git-stunts/git-warp
Length of output: 806
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target test first lines =="
sed -n '1,90p' test/unit/scripts/entity-capture-type-assertion-ratchet.test.ts
echo "== repo files in test unit scripts =="
git ls-files test/unit/scripts | sed -n '1,120p'
echo "== repo files matching type assertion ratchet exactly =="
for f in $(git ls-files | grep 'entity-capture-type-assertion-ratchet'); do
printf '\n--- %s ---\n' "$f"
wc -l "$f"
sed -n '1,120p' "$f"
done
echo "== all tracked files named exactly target =="
find . -path './.git' -prune -o -name 'entity-capture-type-assertion-ratchet.test.ts' -type f -print
echo "== source text around target expression in scripts =="
rg -n "not\.toContain|toContain|noContainedOccurrence|Occurrence|Type Assertion|Ratchet" test/unit/scripts -S || trueRepository: git-stunts/git-warp
Length of output: 49318
Make the occurrence authority check syntax-independent.
not.toContain('issued.subject === occurrence.subject') misses issued.subject===occurrence.subject, other whitespace variants, and the reversed operand order. Resolve EntityOccurrence.ts as a TypeScript source file and reject the equality expression between issued.subject and occurrence.subject with semantic/AST matching before merge.
🤖 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/entity-capture-type-assertion-ratchet.test.ts` at line 52,
Replace the string-based assertion around occurrence in
entity-capture-type-assertion-ratchet with a TypeScript AST-based check that
resolves EntityOccurrence.ts and detects equality expressions comparing
issued.subject and occurrence.subject in either operand order, regardless of
whitespace or formatting. Keep the test failing whenever that semantic authority
check is present.
Source: Coding guidelines
Release Preflight
If this PR is from a |
Path-publication guardrail self-audit
The corrective slices will operate on exact Git representations, report only safe object/path identifiers, and preserve the existing working-inventory lint as defense in depth. Cc: @codex |
Activity Summary — path-publication guardrails
Published head: c1efd1d Validation:
The unrelated occurrence-authority AST ratchet remains outside this guardrail slice and unresolved. |
Release Preflight
If this PR is from a |
1 participant
Summary
Validation
Closes #837