Problem
#1012 gave each crank two savepoints, crank (outer) and delivery (inner), and the correctness of the whole scheme rests on an invariant the types can't express:
crank is created first, delivery second, and only delivery is ever rolled back.
createCrankSavepoint(name: string) and rollbackCrank(savepoint: string) in packages/ocap-kernel/src/store/methods/crank.ts both take bare string. Today the invariant is held by a comment in KernelQueue.#runLoop and one test assertion.
Why a string literal union is the wrong fix
There are two typo classes, and a 'crank' | 'delivery' union only catches the harmless one:
- Misspelling (
'deivery') — rollbackCrank finds no match and hits Fail`no such savepoint as ...` . Loud. Kills the run loop, corrupts nothing.
- Swapped creation order (
delivery created first) — delivery becomes ordinal 0 → t0 → rolling it back rolls back the outermost savepoint, _spStack empties, rollbackIfNeeded() fires, and the transaction is discarded. Silent, and precisely the hazard the two-savepoint split exists to prevent.
A union type accepts the swapped order happily. The load-bearing invariant is ordinal, not nominal.
KernelStore is also public API (exported from packages/ocap-kernel/src/index.ts), so narrowing the parameter is a breaking change that pushes a KernelQueue policy into a store method that legitimately supports arbitrary nesting depth.
Proposal
Remove the parameter instead. Have startCrank() create both savepoints itself and expose rollbackDelivery():
Fallback if that's too invasive: two module-level constants in KernelQueue.ts. Same narrow protection as a union, without the breaking API change.
Related, same file
ctx.savepoints: string[] lives on StoreContext, shared by every method module, with the index↔tN correspondence held by convention across three functions. Only crank.ts touches it (plus the initializer in store/index.ts), so it could move into the getCrankMethods closure and off StoreContext entirely. The truncation idioms ctx.savepoints.length = ordinal and = 0 do meaning-bearing work unnamed — and getting one of them wrong is exactly the bug #1012 fixed.
Only cost: crank.test.ts pokes context.savepoints directly and would need to drive through the public methods, which is arguably an improvement.
Surfaced by review agents on #1012.
Problem
#1012 gave each crank two savepoints,
crank(outer) anddelivery(inner), and the correctness of the whole scheme rests on an invariant the types can't express:createCrankSavepoint(name: string)androllbackCrank(savepoint: string)inpackages/ocap-kernel/src/store/methods/crank.tsboth take barestring. Today the invariant is held by a comment inKernelQueue.#runLoopand one test assertion.Why a string literal union is the wrong fix
There are two typo classes, and a
'crank' | 'delivery'union only catches the harmless one:'deivery') —rollbackCrankfinds no match and hitsFail`no such savepoint as ...`. Loud. Kills the run loop, corrupts nothing.deliverycreated first) —deliverybecomes ordinal 0 →t0→ rolling it back rolls back the outermost savepoint,_spStackempties,rollbackIfNeeded()fires, and the transaction is discarded. Silent, and precisely the hazard the two-savepoint split exists to prevent.A union type accepts the swapped order happily. The load-bearing invariant is ordinal, not nominal.
KernelStoreis also public API (exported frompackages/ocap-kernel/src/index.ts), so narrowing the parameter is a breaking change that pushes aKernelQueuepolicy into a store method that legitimately supports arbitrary nesting depth.Proposal
Remove the parameter instead. Have
startCrank()create both savepoints itself and exposerollbackDelivery():deliveryis rolled back" becomes unrepresentable-otherwiseendCrankreleasest0by position, socrankmust stay first — stops being a commentFallback if that's too invasive: two module-level constants in
KernelQueue.ts. Same narrow protection as a union, without the breaking API change.Related, same file
ctx.savepoints: string[]lives onStoreContext, shared by every method module, with the index↔tNcorrespondence held by convention across three functions. Onlycrank.tstouches it (plus the initializer instore/index.ts), so it could move into thegetCrankMethodsclosure and offStoreContextentirely. The truncation idiomsctx.savepoints.length = ordinaland= 0do meaning-bearing work unnamed — and getting one of them wrong is exactly the bug #1012 fixed.Only cost:
crank.test.tspokescontext.savepointsdirectly and would need to drive through the public methods, which is arguably an improvement.Surfaced by review agents on #1012.