fix(chat): keep proof decision out of actor closure#101611
Conversation
999987d to
b2e92cf
Compare
|
Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 3:43 AM ET / 07:43 UTC. Summary PR surface: Other +7. Total +7 across 1 file. Reproducibility: yes. source-level: current Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the narrow Swift fix after exact-head checks settle, keeping the lock scoped to proof decision, cache mutation, and SQLite commit while outbox notifications run after unlock. Do we have a high-confidence way to reproduce the issue? Yes, source-level: current Is this the best way to solve the issue? Yes. Splitting the private helper is the narrowest maintainable fix I found because it removes the actor-isolated closure capture without adding public API, config, or duplicate runtime paths. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3b418bdef6e5. Label changesLabel justifications:
Evidence reviewedPR surface: Other +7. Total +7 across 1 file. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (73 earlier review cycles; latest 8 shown)
|
0630e03 to
f9a7658
Compare
deca650 to
88d4c8e
Compare
|
@clawsweeper re-review The PR body has been refreshed with current-head proof for |
|
🦞🧹 I asked ClawSweeper to review this item again. |
88d4c8e to
96a300c
Compare
d3201ef to
9595fed
Compare
|
@clawsweeper re-review Current PR head is
GitHub checks for this head are also passing, including |
|
🦞🧹 I asked ClawSweeper to review this item again. |
9595fed to
09837a5
Compare
|
@clawsweeper re-review Current PR head is Local proof after this refresh:
The refresh is rebase-only over the existing one-file fix. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
09837a5 to
35f198b
Compare
|
@clawsweeper re-review Current PR head is Local proof after this refresh:
The refresh is rebase-only over the existing one-line actor-isolation fix. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Current PR head is Local proof after this refresh:
This refresh is rebase-only over the existing one-file actor-isolation fix. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Current PR head is Local proof after this refresh:
The refresh is rebase-only over the existing one-file actor-isolation fix. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Current PR head is Local proof after this refresh:
The refresh is rebase-only over the existing one-file actor-isolation fix. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Current PR head is Local proof after this refresh:
The refresh is rebase-only over the existing one-file actor-isolation fix. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Current PR head is Local proof after this refresh:
The refresh is rebase-only over the existing one-file actor-isolation fix. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Current PR head is Local proof on the same head:
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Maintainer review and fixup complete on exact head What changed:
Best-fix judgment: yes. This preserves the original canonical-proof/SQLite cancellation invariant without moving database ownership, changing storage behavior, or adding compatibility paths. Proof:
Known gap: no separate manual simulator scenario was added because this is a compiler-concurrency refactor with unchanged cancellation behavior; the existing cache/outbox regression suite ran in |
|
Merged via squash.
|
What Problem This Solves
Swift 6.2 strict concurrency rejects
OpenClawChatSQLiteTranscriptCache.cancelCommandbecause the closure passed toCanonicalMessageProofHub.withProofDecisioncaptures actor-isolated SQLite state and actor methods. The failing diagnostic issending 'db' risks causing data races, and it blocksGatewayEndpointStoreTests, theOpenClawproduct build, and packaged app verification from currentmain.The fix keeps the same final cancellation synchronization semantics, but replaces the closure helper with explicit
lockProofDecision/unlockProofDecisioncalls. That keeps the final proof decision, SQLite commit, and cache removal inside the same synchronized region while leaving actor-isolated work in the actor method body.Summary
Evidence
Validated on the rebased head
ed57e6a07e4fe1d452db9f78ec18a56725eed1f5, based on currentmain9614129c2bfb877411780b3568c3a0bc7db0d099:swift test --package-path apps/macos --filter ChatTranscriptCacheIdentityTests(6 tests passed)swift test --package-path apps/macos --filter WebChatSwiftUISmokeTests(2 tests passed)swift test --package-path apps/shared/OpenClawKit --filter ChatTranscriptCacheStoreTests(39 tests passed)swift test --package-path apps/macos --filter GatewayEndpointStoreTests(42 tests passed)swift build --package-path apps/macos --product OpenClawgit diff --checkThis refresh is rebase-only over the existing one-file actor-isolation fix.