Conversation
sirtimid
added this pull request to stack #1107
September 15, 2026 22:37
This was referenced Sep 15, 2026
sirtimid
force-pushed
the
sirtimid/refuse-out-of-loop-savepoint
branch
from
September 16, 2026 16:29
7b2e0e6 to
1d6b159
Compare
sirtimid
force-pushed
the
sirtimid/refuse-out-of-loop-savepoint
branch
from
September 23, 2026 16:57
1d6b159 to
ee493f4
Compare
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
sirtimid
force-pushed
the
sirtimid/refuse-out-of-loop-savepoint
branch
2 times, most recently
from
September 23, 2026 21:47
80ef039 to
d3fe3d4
Compare
sirtimid
force-pushed
the
sirtimid/refuse-out-of-loop-savepoint
branch
from
September 23, 2026 22:26
d3fe3d4 to
86739ec
Compare
sirtimid
force-pushed
the
sirtimid/refuse-out-of-loop-savepoint
branch
from
September 24, 2026 13:18
86739ec to
eff9659
Compare
The enforcement half of stopping the loop before a direct write. A savepoint taken outside the crank machinery is the outermost one on the connection, and so the transaction's commit point: opened while the loop runs, it either nests inside whatever crank is open or opens a transaction the next crank nests inside, and either way somebody else decides whether this caller's writes survive. Nothing does that any more — after inbound remote messages and peer incarnation changes moved onto the run queue, no caller of `createSavepoint`, `rollbackSavepoint` or `releaseSavepoint` is left outside `store/methods/crank.ts`. This is what keeps it that way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sirtimid
force-pushed
the
sirtimid/refuse-out-of-loop-savepoint
branch
from
September 24, 2026 15:10
eff9659 to
5b9afe4
Compare
This branch has not been deployed
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
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.
The enforcement half of stopping the run loop before a direct write.
A savepoint taken outside the crank machinery is the outermost one on the connection, and so the transaction's commit point. Opened while the loop runs, it either nests inside whatever crank is open or opens a transaction the next crank nests inside — and either way somebody else decides whether this caller's writes survive. Ryan's #1039 claim 1 has all three orderings against real SQLite.
Nothing does that any more. After inbound remote messages (#1103) and peer incarnation changes (#1104) moved onto the run queue, no caller of
createSavepoint,rollbackSavepointorreleaseSavepointis left outsidestore/methods/crank.ts. This is what keeps it that way.Changes
KernelStore.createSavepointrefuses a savepoint taken while the run loop is running. The crank machinery's own savepoints are unaffected — they are taken by the run loop, which is what the refusal is defined against.RunLoopStatus'sstoppedstate, added in feat(ocap-kernel): stop the run loop before the control plane writes #1105, is what the check reads, so the control plane's own direct writes still work: it stops the loop first, and only then is a savepoint legal again.Testing
KernelQueue.run-loop-stop.test.tsgains the refusal cases alongside #1105's stop/resume ones.@metamask/ocap-kernel'sKernelQueue,KernelQueue.run-loop-stop,store/indexandget-statussuites are green (102 tests), and the package type-checks.Not done here, and the reason for the warning above: the
code-reviewer/silent-failure-hunter/pr-test-analyzerround, the mutation checks, and the full-package and@ocap/kernel-testruns. Every other PR in this split had all of them.🤖 Generated with Claude Code
Note
Medium Risk
Changes when kernel store transactions may be opened relative to the run loop; incorrect savepoint timing could affect persistence, though the change fails fast instead of allowing ambiguous commits.
Overview
KernelStore.createSavepointnow fails while the run loop is active, so embedders cannot open the outermost DB savepoint (the real commit boundary) during a crank. That nesting let the run loop decide whether direct writes survived, which conflicted with stopping the loop for control-plane storage updates (#1105).KernelQueue.runtoggles the guard via newsetRunLoopRunning: settruewhen the loop starts and cleared infinally, so savepoints are allowed again once the loop is idle or stopped between cranks. Crank-internal savepoints (createCrankSavepoint) are unchanged.Integration tests in
KernelQueue.run-loop-stop.test.tscover refusal during a running loop and success afterstopRunLoop. The changelog documents the behavior change.Reviewed by Cursor Bugbot for commit 5b9afe4. Bugbot is set up for automated code reviews on this repo. Configure here.