Skip to content

Commit 052f4d4

Browse files
FUDCoclaude
andauthored
fix(ocap-kernel): regenerate incarnationId on resetStorage=true (#950)
## Summary - Drops \`'incarnationId'\` from \`Kernel.#resetKernelState\`'s except-list so a state wipe forces a fresh incarnation. - Adds an e2e regression for the \`resetStorage=true\` path (the existing #944 test exercises only the fresh-DB path). ## Why The peer-restart detection added in #948 only works if the sender signals a new incarnation when its state has been wiped. The except-list in \`#resetKernelState\` was preserving \`incarnationId\` across \`resetStorage=true\` resets, so the sender came back signalling the same incarnation, the receiver's handshake said "no restart", \`highestReceivedSeq\` stayed put, and the sender's fresh \`seq=1\` messages were dropped as duplicates. The browser/extension kernel-worker URL carries \`?reset-storage=true\` and hits this branch on every offscreen-page reload, so in production this meant every plugin reload that re-registered a service silently lost its first message. Repro on metamask-extension chip/agentmask: full reset → first reload registers cleanly → second reload's PMS \`registerServiceByRef\` arrives at the matcher with seq=1, matcher logs \`12D3KooW:: ignoring duplicate message seq=1 (highestReceived=4)\` and the message disappears. Confirmed empirically by dumping the matcher's KV after the second-reload symptom: \`peerIncarnation.<ext-peer-id>\` was equal to the incarnation the extension was *currently* sending — exactly the case \`#handleIncarnationChange\` exits early on. The \`provideIncarnationId\` doc comment already advertised the post-fix behaviour ("regenerated when storage is cleared (when state is actually lost)"); the code just wasn't doing it. ## Test plan - [x] \`yarn workspace @MetaMask/ocap-kernel test\` (2341 passing locally) - [x] Production repro on chip/agentmask: full reset, first reload registers, second reload registers again instead of being dropped - [ ] CI green, including the new e2e \`detects restart when sender keeps DB but uses resetStorage=true\` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes how `resetStorage=true` resets persistent kernel state, which affects remote-communication handshake/dedup behavior across restarts; mistakes could cause message loss or unexpected peer identity changes. > > **Overview** > Fixes the `resetStorage=true` reset path to **regenerate `incarnationId`** while still preserving network identity keys (`keySeed`, `peerId`, `ocapURLKey`), so peers can reliably detect a state wipe and clear stale seq-dedup/c-list state. > > Adds an e2e regression test covering the browser/extension-style restart where the sender keeps the same DB file but starts with `resetStorage=true`, ensuring `seq=1` messages are no longer silently dropped after reloads, and documents the fix in the `CHANGELOG`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c5cb8ea. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ec3d701 commit 052f4d4

3 files changed

Lines changed: 90 additions & 3 deletions

File tree

packages/kernel-node-runtime/test/e2e/remote-comms.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,79 @@ describe.sequential('Remote Communications E2E', () => {
11181118
},
11191119
NETWORK_TIMEOUT * 3,
11201120
);
1121+
1122+
it(
1123+
'detects restart when sender keeps DB but uses resetStorage=true',
1124+
async () => {
1125+
// Companion to the test above. The fresh-DB path naturally regenerates
1126+
// incarnationId (nothing in KV to read). The `resetStorage=true` path
1127+
// wipes the KV but used to be coded with an except-list that preserved
1128+
// incarnationId — so the sender came back signaling the same
1129+
// incarnation, the matcher's handshake said "no restart", and the
1130+
// sender's fresh seq=1 was dropped against persisted dedup state.
1131+
// The browser/extension hits this path on every kernel-worker boot
1132+
// (the offscreen page launches the worker with ?reset-storage=true),
1133+
// so without this regression a plugin reload silently loses messages.
1134+
const k2Mnemonic =
1135+
'legal winner thank year wave sausage worth useful legal winner thank yellow';
1136+
const opts = {
1137+
relays: testRelays,
1138+
reconnectionBaseDelayMs: 100,
1139+
reconnectionMaxDelayMs: 500,
1140+
handshakeTimeoutMs: 5_000,
1141+
writeTimeoutMs: 5_000,
1142+
ackTimeoutMs: 2_000,
1143+
maxRetryAttempts: 4,
1144+
};
1145+
1146+
await kernel1.initRemoteComms({ ...opts });
1147+
await kernel2.initRemoteComms({ ...opts, mnemonic: k2Mnemonic });
1148+
const aliceURL = await launchVatAndGetURL(
1149+
kernel1,
1150+
makeRemoteVatConfig('Alice'),
1151+
);
1152+
await launchVatAndGetURL(kernel2, makeRemoteVatConfig('Bob'));
1153+
const bobRef = getVatRootRef(kernel2, kernelStore2, 'Bob');
1154+
1155+
const phase1 = await sendRemoteMessage(
1156+
kernel2,
1157+
bobRef,
1158+
aliceURL,
1159+
'hello',
1160+
['Bob-before-reset'],
1161+
);
1162+
expect(phase1).toContain('vat Alice got "hello" from Bob-before-reset');
1163+
1164+
// Same delay rationale as in the fresh-DB test above.
1165+
await delay(150);
1166+
1167+
// Restart only the sender, KEEPING its DB but passing
1168+
// resetStorage=true. This is the path the browser kernel-worker
1169+
// takes on every offscreen-page reload. Identity keys (keySeed,
1170+
// peerId, ocapURLKey) must survive so the peer id stays stable;
1171+
// incarnationId must NOT survive so the matcher's handshake can
1172+
// detect the wipe.
1173+
await stopWithTimeout(async () => kernel2.stop(), 3000, 'kernel2.stop');
1174+
// eslint-disable-next-line require-atomic-updates
1175+
kernel2 = await restartKernel(dbFilename2, true, testRelays, opts);
1176+
await launchVatAndGetURL(kernel2, makeRemoteVatConfig('Bob'));
1177+
// eslint-disable-next-line require-atomic-updates
1178+
kernelStore2 = makeKernelStore(
1179+
await makeSQLKernelDatabase({ dbFilename: dbFilename2 }),
1180+
);
1181+
const newBobRef = getVatRootRef(kernel2, kernelStore2, 'Bob');
1182+
1183+
const phase2 = await sendRemoteMessage(
1184+
kernel2,
1185+
newBobRef,
1186+
aliceURL,
1187+
'hello',
1188+
['Bob-after-reset'],
1189+
);
1190+
expect(phase2).toContain('vat Alice got "hello" from Bob-after-reset');
1191+
},
1192+
NETWORK_TIMEOUT * 3,
1193+
);
11211194
});
11221195

11231196
describe('Promise Rejection on Remote Give-Up', () => {

packages/ocap-kernel/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3737
- Persist the peer's last-observed incarnation and compare it on every successful handshake; on a detected restart, clear the peer's c-list contributions and reject the promises it was deciding before the new incarnation reuses any erefs
3838
- Accept liveslots-allocated durable, virtual, and faceted vrefs (e.g. `o+d10/1`, `o+v3/4:0`) in `isVRef` / `insistERef` / `EndpointMessageStruct` validation ([#949](https://github.com/MetaMask/ocap-kernel/pull/949))
3939
- Previously the regex only matched plain `[op][+-]N`, so any vat using `defineDurableKind` failed outgoing-send validation and persisted-slot reads
40+
- Regenerate `incarnationId` when `resetStorage=true` clears the rest of kernel state, completing the #948 peer-restart detection on browser/extension kernel reloads ([#950](https://github.com/MetaMask/ocap-kernel/pull/950))
41+
- The previous except-list preserved `incarnationId` across `resetStorage` wipes, so a restarted sender signalled the same incarnation it had before the wipe and the matching receiver's handshake decided "no restart" — leaving stale `highestReceivedSeq` in place and silently dropping the sender's fresh `seq=1` messages
4042

4143
## [0.7.0]
4244

packages/ocap-kernel/src/Kernel.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -704,8 +704,18 @@ export class Kernel {
704704
/**
705705
* Reset the kernel state.
706706
*
707+
* Identity keys (`keySeed`, `peerId`, `ocapURLKey`) are preserved so the
708+
* kernel keeps the same network identity across the reset. The
709+
* `incarnationId` is deliberately *not* preserved: its whole purpose is
710+
* to signal to remote peers that local state has been wiped so they
711+
* clear any seq-dedup / c-list bookkeeping for this peer. Preserving it
712+
* across a state wipe defeats the peer-restart detection introduced in
713+
* #948.
714+
*
707715
* @param options - Options for the reset.
708-
* @param options.resetIdentity - If true, also clears identity keys (keySeed, peerId, ocapURLKey, incarnationId).
716+
* @param options.resetIdentity - If true, also clears identity keys
717+
* (keySeed, peerId, ocapURLKey). Used when recovering from a
718+
* mnemonic, which is regenerating identity from scratch.
709719
*/
710720
#resetKernelState({
711721
resetIdentity = false,
@@ -714,9 +724,11 @@ export class Kernel {
714724
// Full reset including identity - used when recovering from mnemonic
715725
this.#kernelStore.reset();
716726
} else {
717-
// Preserve identity keys so network address survives restart
727+
// Preserve identity keys so network address survives restart.
728+
// `incarnationId` is deliberately omitted: it must be regenerated
729+
// so remote peers can detect the state loss via the handshake.
718730
this.#kernelStore.reset({
719-
except: ['keySeed', 'peerId', 'ocapURLKey', 'incarnationId'],
731+
except: ['keySeed', 'peerId', 'ocapURLKey'],
720732
});
721733
}
722734
}

0 commit comments

Comments
 (0)