Add revalidated Runtime settlement plans - #796
Conversation
|
Warning Review limit reached
Next review available in: 12 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 Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds immutable runtime settlement previews and revalidated settlement execution, with serialized mutations, settlement runtime contracts, canonical receipts, public exports, and integration, unit, type-check, and API-boundary tests. ChangesRuntime settlement
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Runtime
participant RuntimeSettlement
participant SourceRuntime
participant TargetRuntime
Client->>Runtime: previewSettlement(source, target)
Runtime->>RuntimeSettlement: prepare settlement plan
RuntimeSettlement->>SourceRuntime: capture source snapshot and digest
RuntimeSettlement->>TargetRuntime: capture target basis
RuntimeSettlement-->>Runtime: immutable SettlementPreview
Client->>Runtime: settle(preview.plan)
Runtime->>RuntimeSettlement: revalidate bound plan
RuntimeSettlement->>SourceRuntime: runExclusive promotion
SourceRuntime-->>RuntimeSettlement: promotion result
RuntimeSettlement-->>Runtime: SettlementReceipt
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/application/RuntimeMutationGate.ts`:
- Around line 4-21: Add a concise documentation comment to
RuntimeMutationGate.run warning that invoking run directly or transitively on
the same gate from within an active operation causes a deadlock, and instruct
callers to avoid reentrant use. Keep the existing serialization behavior
unchanged.
In `@src/application/RuntimeSettlement.ts`:
- Around line 394-405: Update outcomeFields to return a runtime-frozen
SettlementOutcomeFields object, matching the Object.freeze behavior of
createSettlementFields while preserving all existing field values.
In `@src/domain/api/SettlementIdentityRuntime.ts`:
- Around line 52-59: Update flattenFrontier to sort writerId values with a
deterministic ordinal/codepoint comparison instead of localeCompare(),
preserving the existing ascending order and flattened [writerId, patchSha]
output used by the frontier digest functions.
In `@src/domain/api/SettlementPreview.ts`:
- Around line 53-70: Extract the duplicated freezeLaneReference helper into a
shared module alongside ReceiptSupport.ts, preserving its validation, error
message/code, and frozen return value. Remove the local helper from
src/domain/api/SettlementPreview.ts lines 53-70 and import the shared helper;
remove the duplicate from src/domain/api/SettlementReceipt.ts lines 74-91 and
import the same helper.
In `@test/unit/application/RuntimeSettlement.preview.test.ts`:
- Line 1: Extract the duplicated settlement setup into a shared test-support
harness, moving settlementHarness, freezeSnapshot, bindTargetLane,
bindSourceLane, createLane, and unavailableCoordinate from
RuntimeSettlement.preview.test.ts and replacing the inline
SettlementSourceRuntime, snapshot, evidence, and bindSettlementPlan setup in
RuntimeSettlement.test.ts. Import and reuse this harness in both suites,
parameterizing only the plural/conflict outcome while preserving each test’s
existing 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: 5a5255a6-4c80-4eb1-84a8-849435d7069f
📒 Files selected for processing (29)
docs/topics/reference.mdindex.tssrc/application/Runtime.tssrc/application/RuntimeLaneAdapter.tssrc/application/RuntimeMutationGate.tssrc/application/RuntimeSettlement.tssrc/application/RuntimeSettlementOptions.tssrc/application/RuntimeSettlementValidation.tssrc/application/RuntimeStrandLaneBinding.tssrc/domain/api/DraftSettlementRuntime.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/LaneRuntime.tssrc/domain/api/LaneSettlementRuntime.tssrc/domain/api/PublicReceipt.tssrc/domain/api/SettlementIdentityRuntime.tssrc/domain/api/SettlementOutcomeRuntime.tssrc/domain/api/SettlementPlanRuntime.tssrc/domain/api/SettlementPreview.tssrc/domain/api/SettlementReceipt.tssrc/domain/settlement/SettlementPlan.tstest/integration/application/Runtime.settlement.integration.test.tstest/type-check/v19-consumer.tstest/unit/application/Runtime.test.tstest/unit/application/RuntimeLaneAdapter.test.tstest/unit/application/RuntimeMutationGate.test.tstest/unit/application/RuntimeSettlement.preview.test.tstest/unit/application/RuntimeSettlement.test.tstest/unit/domain/settlement/SettlementPlan.test.tstest/unit/scripts/v19-public-api-boundary.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: test-node (22)
- GitHub Check: v19 base/head performance
- GitHub Check: coverage-threshold
- GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (4)
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/PublicReceipt.tssrc/application/RuntimeSettlementOptions.tssrc/domain/api/SettlementIdentityRuntime.tssrc/domain/api/LaneSettlementRuntime.tssrc/domain/api/SettlementPreview.tssrc/application/RuntimeMutationGate.tssrc/domain/api/SettlementPlanRuntime.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/LaneRuntime.tssrc/application/RuntimeStrandLaneBinding.tssrc/domain/api/SettlementReceipt.tssrc/application/RuntimeSettlementValidation.tssrc/domain/api/SettlementOutcomeRuntime.tssrc/application/RuntimeSettlement.tssrc/domain/settlement/SettlementPlan.tssrc/domain/api/DraftSettlementRuntime.tssrc/application/Runtime.tssrc/application/RuntimeLaneAdapter.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/PublicReceipt.tssrc/domain/api/SettlementIdentityRuntime.tssrc/domain/api/LaneSettlementRuntime.tssrc/domain/api/SettlementPreview.tssrc/domain/api/SettlementPlanRuntime.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/LaneRuntime.tssrc/domain/api/SettlementReceipt.tssrc/domain/api/SettlementOutcomeRuntime.tssrc/domain/settlement/SettlementPlan.tssrc/domain/api/DraftSettlementRuntime.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/PublicReceipt.tssrc/domain/api/SettlementIdentityRuntime.tssrc/domain/api/LaneSettlementRuntime.tssrc/domain/api/SettlementPreview.tssrc/domain/api/SettlementPlanRuntime.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/LaneRuntime.tssrc/domain/api/SettlementReceipt.tssrc/domain/api/SettlementOutcomeRuntime.tssrc/domain/settlement/SettlementPlan.tssrc/domain/api/DraftSettlementRuntime.ts
**/*.{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:
src/domain/api/PublicReceipt.tssrc/application/RuntimeSettlementOptions.tstest/unit/application/RuntimeMutationGate.test.tssrc/domain/api/SettlementIdentityRuntime.tstest/unit/application/RuntimeLaneAdapter.test.tsindex.tssrc/domain/api/LaneSettlementRuntime.tssrc/domain/api/SettlementPreview.tssrc/application/RuntimeMutationGate.tssrc/domain/api/SettlementPlanRuntime.tstest/unit/scripts/v19-public-api-boundary.test.tssrc/domain/api/DraftTimelineRuntime.tstest/unit/application/RuntimeSettlement.test.tstest/unit/domain/settlement/SettlementPlan.test.tssrc/domain/api/LaneRuntime.tssrc/application/RuntimeStrandLaneBinding.tstest/type-check/v19-consumer.tssrc/domain/api/SettlementReceipt.tssrc/application/RuntimeSettlementValidation.tstest/integration/application/Runtime.settlement.integration.test.tstest/unit/application/Runtime.test.tstest/unit/application/RuntimeSettlement.preview.test.tssrc/domain/api/SettlementOutcomeRuntime.tssrc/application/RuntimeSettlement.tssrc/domain/settlement/SettlementPlan.tssrc/domain/api/DraftSettlementRuntime.tssrc/application/Runtime.tssrc/application/RuntimeLaneAdapter.ts
🔇 Additional comments (30)
index.ts (1)
14-18: LGTM!Also applies to: 41-43
docs/topics/reference.md (1)
42-73: LGTM!test/integration/application/Runtime.settlement.integration.test.ts (1)
20-239: LGTM!test/type-check/v19-consumer.ts (1)
15-21: LGTM!Also applies to: 55-64, 113-113
test/unit/application/Runtime.test.ts (1)
42-49: LGTM!Also applies to: 210-210
test/unit/application/RuntimeLaneAdapter.test.ts (1)
138-189: LGTM!test/unit/application/RuntimeMutationGate.test.ts (1)
1-43: LGTM!test/unit/application/RuntimeSettlement.preview.test.ts (1)
1-157: LGTM!test/unit/application/RuntimeSettlement.test.ts (1)
1-14: LGTM!Also applies to: 96-132
test/unit/domain/settlement/SettlementPlan.test.ts (1)
7-20: LGTM!Also applies to: 31-34, 56-80
test/unit/scripts/v19-public-api-boundary.test.ts (1)
31-34: LGTM!src/application/RuntimeSettlementOptions.ts (1)
1-6: LGTM!src/domain/api/LaneSettlementRuntime.ts (1)
1-45: LGTM!src/domain/api/SettlementOutcomeRuntime.ts (1)
1-81: LGTM!src/domain/api/SettlementPlanRuntime.ts (1)
21-53: LGTM!src/domain/api/DraftTimelineRuntime.ts (1)
23-23: LGTM!Also applies to: 297-299
src/domain/api/DraftSettlementRuntime.ts (2)
34-39: 🎯 Functional Correctness | ⚡ Quick winClarify discarded
prepareOpticBasis()result before readingcoordinate().
await state.runtime.prepareOpticBasis();discards its return value and is immediately followed by a separatestate.runtime.coordinate()call to gettargetCoordinate, which feedstargetFrontierRefused for stale/common-basis comparisons. Every other "prepare" call in this file/DraftTimelineRuntime.tsuses its returned basis. Please confirm this is intentional cache/ref warm-up (and not a spot where the returned basis was meant to be used directly), since an incorrect assumption here would undermine the stale-basis detection this PR is meant to guarantee.
73-157: LGTM!src/application/RuntimeSettlementValidation.ts (1)
1-105: LGTM!src/application/RuntimeSettlement.ts (1)
60-393: LGTM!src/domain/api/LaneRuntime.ts (1)
4-10: LGTM!Also applies to: 23-23
src/domain/settlement/SettlementPlan.ts (1)
1-14: LGTM!Also applies to: 17-77
src/domain/api/SettlementPreview.ts (1)
1-52: LGTM!src/domain/api/SettlementReceipt.ts (1)
1-73: LGTM!src/domain/api/PublicReceipt.ts (1)
2-6: LGTM!src/application/RuntimeLaneAdapter.ts (1)
34-35: LGTM!Also applies to: 47-71, 73-100, 102-104, 106-128, 141-167, 169-194
src/application/Runtime.ts (3)
46-46: 🚀 Performance & ScalabilityConfirm the Runtime-wide single mutation gate is intentional.
#mutationsis oneRuntimeMutationGateinstance shared by every worldline lane opened vialane()and every strand lane forked from them, so all writes and settlement promotions across the entire Runtime now serialize through a single queue rather than per-lane. This is likely correct given git's single-working-tree write constraints, but it's worth confirming this doesn't unexpectedly throttle unrelated concurrent lanes under load.Also applies to: 55-55, 82-90
108-117: 🩺 Stability & AvailabilityVerify
previewSettlement/settleare safe against concurrentclose().Unlike
lane()andfork(), these methods don't wrap themselves inthis.#activity.run(...)/.acquire()at the top level.settle()'s actual work is tracked indirectly viarunExclusive's nestedactivity.run(mutations.run(...)), butpreviewSettlement'sprepareSettlementinternals live inRuntimeSettlement.ts, which isn't in this review batch — please confirm none of its steps perform I/O outside the granular, activity-tracked capture calls, soRuntime.close()can't complete while a preview/settle call is still doing untracked work.
2-27: LGTM!src/application/RuntimeStrandLaneBinding.ts (1)
1-57: LGTM!
Release Preflight
If this PR is from a |
1 participant
Adds immutable Runtime-issued settlement plans with exact Lane/frontier/law/policy binding, stale-basis revalidation, canonical receipts, and serialized promotion.
Validated by the full pre-push M9 firewall and coverage (7,350 tests; 92.97% lines).
Closes #793