From 3c8637117e817bb3148dc7716b254821feee3c3f Mon Sep 17 00:00:00 2001 From: Glenn Gore Date: Mon, 31 Aug 2026 08:23:40 +0200 Subject: [PATCH 1/2] fix(inbox): adopt over a relay nobody is on record choosing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #148 removed the hardcoded demo mediator and backfilled the agent's relay for wallets that had none. It did not fire, and the reason was in the same file: `setSettings` merged the *defaulted* settings and wrote them back. const current = await getSettings(); // mediatorDid: DEFAULT_… await put(SETTINGS_KEY, { ...current, ...patch }); Under the old default that meant any unrelated write — turning on the passkey lock, toggling TSP preference, saving a step-up default — persisted the demo mediator DID into IndexedDB as though it had been chosen. So wallets carry a stored inbox nobody picked, and `inboxToAdopt` declining to move "an inbox that is already set" declined to move exactly the ones the migration existed for. The rule was right; the value alone was never enough to go on. `mediatorDidSource` records who put the relay there. `operator` is a person typing into Setup → Message routing and is never overridden. `agent` is onboarding or the boot backfill, and is left alone too — the inbox is an address others route to, so it must not chase the active VTA. Absent means nobody is on record, which is the truth for every record written before now, and those adopt the agent's relay once. Self-limiting without a version counter: after one boot every record has a source. The cost, stated rather than buried: an operator who hand-set a mediator before provenance existed has it adopted over, once. With nothing deployed, the alternative is wallets stuck on a relay in someone else's deployment, and the person affected is the one who knows how to set it again. `setSettings` now merges onto the STORED record. That write-back is the root cause and it was not specific to the mediator — every derived default became a persisted value that later code could no longer tell apart from a choice. Also: a boot that adopts nothing because no onboarded agent advertises a relay now says so, instead of looking like it did its job. Signed-off-by: Glenn Gore --- packages/extension/src/background.ts | 22 +++++- packages/extension/src/config.ts | 70 ++++++++++++++++--- packages/extension/src/offscreen.ts | 8 ++- packages/extension/src/setup-pane.tsx | 4 +- .../extension/tests/wallet-inbox.test.mts | 28 ++++++-- 5 files changed, 109 insertions(+), 23 deletions(-) diff --git a/packages/extension/src/background.ts b/packages/extension/src/background.ts index 29500a1..1a8401e 100644 --- a/packages/extension/src/background.ts +++ b/packages/extension/src/background.ts @@ -480,10 +480,26 @@ async function startInboundListener(): Promise { // it applies the same rule onboarding now applies, at the one place that // runs on every boot. `inboxToAdopt` declines when an inbox is already set, // so this never moves an address in use. - const adopt = inboxToAdopt((await getSettings()).mediatorDid, await readAgentMediatorDid()); + const settings = await getSettings(); + const adopt = inboxToAdopt( + { did: settings.mediatorDid, source: settings.mediatorDidSource }, + await readAgentMediatorDid(), + ); if (adopt) { - await setSettings({ mediatorDid: adopt }); - console.info("[pnm inbound] inbox mediator backfilled from agent:", adopt); + await setSettings({ mediatorDid: adopt, mediatorDidSource: "agent" }); + console.info("[pnm inbound] inbox mediator adopted from agent:", adopt); + } else if (!settings.mediatorDidSource && vtaDids.length > 0) { + // Nothing adopted and nothing on record choosing what is there: the + // persisted connection carries no mediator to adopt. Said out loud, + // because the alternative is a wallet that silently keeps whatever relay + // it had and a boot that looks like it did its job. Refreshing transports + // (Setup → the agent's transports) re-resolves the DID document and fills + // the connection in. + console.warn( + "[pnm inbound] inbox relay is unattributed and no onboarded agent " + + "advertises one to adopt — refresh the agent's transports, or set a " + + "relay under Setup → Message routing.", + ); } // Seed _lastActiveVtaDid too — otherwise the first chrome.storage // onChanged callback would see _lastActiveVtaDid=null and emit a diff --git a/packages/extension/src/config.ts b/packages/extension/src/config.ts index d53d12a..31591bc 100644 --- a/packages/extension/src/config.ts +++ b/packages/extension/src/config.ts @@ -19,6 +19,10 @@ import { IndexedDBKVStore } from "@openvtc/pnm-core"; +/** Who put the inbox mediator there. Absent on records written before this + * existed — treated as "nobody is on record", which is the truth. */ +export type InboxSource = "agent" | "operator"; + export interface WalletSettings { /** * The wallet's inbox: the mediator an RP or executor pushes to in order to @@ -35,6 +39,12 @@ export interface WalletSettings { * it can only appear to work. (R5 — config absence is the restrictive case.) */ mediatorDid?: string; + + /** Provenance for `mediatorDid`: `agent` when onboarding (or the boot + * backfill) adopted the agent's advertised relay, `operator` when a person + * typed it into Setup → Message routing. Read by `inboxToAdopt`; see the + * note there for why the value alone was not enough to go on. */ + mediatorDidSource?: InboxSource; /** Optional default VTA DID prefilled into the step-up flow. */ defaultStepUpVtaDid?: string; /** Optional default VTA mediator DID prefilled into the step-up flow. */ @@ -124,27 +134,61 @@ export interface WalletSettings { * mediator cannot push to a wallet; an inbox invented here would be a * relay nobody was ever asked about. Unset is the honest state and the * self-test reports it. - * - Already set → leave it. Either an operator chose it deliberately (they - * run more than one relay), or a previous onboarding adopted it — and the - * inbox is an *address* other parties already route to, so a second - * onboarding silently moving it would strand everyone who knows this - * wallet. Changing it stays a deliberate act with its own confirmation. - * - Otherwise → adopt the agent's. + * - Set, and someone is on record choosing it → leave it. Either an operator + * picked it (they run more than one relay) or an earlier onboarding adopted + * it — and the inbox is an *address* other parties already route to, so + * moving it silently would strand everyone who knows this wallet. + * - Otherwise → adopt the agent's. That covers an unset inbox and, once, the + * records written before provenance existed. + * + * **Why `source` had to exist.** The first cut of this keyed on "is anything + * set?", which read as sufficient and was not. `setSettings` merged the + * *defaulted* view of the settings and wrote it back, so under the old + * hardcoded default any unrelated write — turning on the passkey lock, + * toggling TSP preference — persisted the demo mediator DID into IndexedDB as + * though it had been chosen. Wallets therefore carry a stored inbox nobody + * picked, indistinguishable by value from a deliberate one, and the migration + * that was supposed to rescue them declined to touch it. (That write-back is + * fixed in `setSettings` below; this handles the records it already made.) + * + * The cost is stated rather than hidden: an operator who hand-set a mediator + * before provenance existed has it adopted over, once. With nothing deployed + * and the alternative being wallets stuck on a relay in someone else's + * deployment, that is the right side to err on — and the person affected is + * exactly the person who knows how to set it again. */ export function inboxToAdopt( - current: string | undefined, + current: { did?: string | undefined; source?: InboxSource | undefined }, advertised: string | undefined, ): string | undefined { if (!advertised) return undefined; - if (current) return undefined; + // A person chose this relay. Never overridden. + if (current.source === "operator") return undefined; + // Already adopted from an agent. Left alone even when the active agent + // changes: the inbox is an address others route to, and chasing the active + // VTA would move it out from under them. + if (current.did && current.source === "agent") return undefined; + // Either nothing is set, or something is set that no one recorded choosing — + // which is every record written before provenance existed. Adopt. return advertised; } const SETTINGS_KEY = "pnm/settings/v1"; /** Read the current settings, falling back to defaults for unset fields. */ +/** The record as it is actually stored — no defaults applied. + * + * Separate from `getSettings` because the two have genuinely different jobs, + * and conflating them is what produced the inbox defect: `setSettings` merged + * the *defaulted* view and wrote it back, so every read-modify-write turned + * derived defaults into persisted values that later code could no longer tell + * apart from choices. A write must merge onto what is on disk. */ +async function storedSettings(): Promise> { + return (await new IndexedDBKVStore().get>(SETTINGS_KEY)) ?? {}; +} + export async function getSettings(): Promise { - const s = await new IndexedDBKVStore().get>(SETTINGS_KEY); + const s = await storedSettings(); // `encryptHolderSecret` defaults to FALSE until the popup-driven // WebAuthn-enrol path lands — see the field's docblock for the // architectural constraint (offscreen + WebAuthn don't mix). @@ -154,6 +198,9 @@ export async function getSettings(): Promise { typeof s?.encryptHolderSecret === "boolean" ? s.encryptHolderSecret : false; return { ...(s?.mediatorDid ? { mediatorDid: s.mediatorDid } : {}), + ...(s?.mediatorDidSource === "agent" || s?.mediatorDidSource === "operator" + ? { mediatorDidSource: s.mediatorDidSource } + : {}), ...(s?.defaultStepUpVtaDid ? { defaultStepUpVtaDid: s.defaultStepUpVtaDid } : {}), ...(s?.defaultStepUpVtaMediatorDid ? { defaultStepUpVtaMediatorDid: s.defaultStepUpVtaMediatorDid } @@ -176,6 +223,7 @@ export async function getSettings(): Promise { /** Merge a partial update into the stored settings. */ export async function setSettings(patch: Partial): Promise { - const current = await getSettings(); - await new IndexedDBKVStore().put(SETTINGS_KEY, { ...current, ...patch }); + // Merged onto the STORED record, not the defaulted one. See `storedSettings`. + const stored = await storedSettings(); + await new IndexedDBKVStore().put(SETTINGS_KEY, { ...stored, ...patch }); } diff --git a/packages/extension/src/offscreen.ts b/packages/extension/src/offscreen.ts index ad831cc..f4900b0 100644 --- a/packages/extension/src/offscreen.ts +++ b/packages/extension/src/offscreen.ts @@ -1829,9 +1829,13 @@ async function doOnboardConnect(params: OnboardConnectParams): Promise { - assert.equal(inboxToAdopt(undefined, AGENT), AGENT); + assert.equal(inboxToAdopt({}, AGENT), AGENT); }); -test("leaves an inbox the operator already chose", () => { +test("leaves an inbox the operator chose", () => { // The operator running two relays picked this one on purpose. - assert.equal(inboxToAdopt(OTHER, AGENT), undefined); + assert.equal(inboxToAdopt({ did: OTHER, source: "operator" }, AGENT), undefined); }); test("a second onboarding does not move an address others already route to", () => { - assert.equal(inboxToAdopt(AGENT, OTHER), undefined); + assert.equal(inboxToAdopt({ did: AGENT, source: "agent" }, OTHER), undefined); }); test("an agent advertising no mediator leaves the inbox unset, not invented", () => { // Unset is reported by the self-test as "nothing can reach this wallet". // Substituting anything here is what caused the original defect. - assert.equal(inboxToAdopt(undefined, undefined), undefined); - assert.equal(inboxToAdopt("", undefined), undefined); + assert.equal(inboxToAdopt({}, undefined), undefined); + assert.equal(inboxToAdopt({ did: "" }, undefined), undefined); +}); + +test("adopts over a stored inbox nobody is on record choosing", () => { + // The case that defeated the first migration. `setSettings` merged the + // DEFAULTED settings and wrote them back, so any unrelated write — the + // passkey lock, the TSP toggle — persisted the old hardcoded demo mediator + // as though it had been picked. By value it is indistinguishable from a + // deliberate choice; by provenance it is not. + const DEMO = "did:webvh:QmDemoRelay:demo.example:mediator"; + assert.equal(inboxToAdopt({ did: DEMO }, AGENT), AGENT); +}); + +test("the adoption happens once, not on every boot", () => { + // Stamped `agent` on the way in, so the next boot leaves it alone even + // though the active agent may since have changed. + assert.equal(inboxToAdopt({ did: AGENT, source: "agent" }, AGENT), undefined); }); // ─── No mediator may be baked into the source again ─── From 754b5aa25757dd6b0473af9cabbaf31f2f5ff259 Mon Sep 17 00:00:00 2001 From: Glenn Gore Date: Mon, 31 Aug 2026 08:27:52 +0200 Subject: [PATCH 2/2] fix(inbox): follow the agent when it moves its relay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inbox is not an address this wallet owns. A v4 holder is a `did:key`, which carries no service endpoint, and the wallet publishes its mediator to nobody — there is no discovery path. So an executor pushing to this wallet can only hand the message to a mediator it already knows, its own, and the wallet hears it only if it is listening there. "Wherever my agent's relay is" is what the inbox means. Which makes the previous commit's rule half right. Pinning an agent-sourced inbox forever is a wallet that goes dark the day its operator redeploys the mediator, with every check still green — the same failure this thread started with, one deployment later. `followAgentInbox` re-resolves the agent's DID document and moves an `agent`-sourced inbox when the advertised relay has changed, then re-opens the sessions on it. An `operator`-sourced inbox never moves; that override is the whole reason provenance exists. On `onStartup` and `onInstalled`, not per worker spin-up: MV3 respawns the worker on almost any event and a DID-document fetch on each would be a lot of network for a value that changes when someone redeploys. The blank-filling backfill still runs every spin-up — it reads the persisted connection and costs nothing. A document that cannot be read is not evidence the relay moved, so a failed resolve keeps what we have and says so. Known limitation, unchanged by this and worth its own issue: one inbox setting against a multi-VTA wallet. Two agents on different mediators cannot both reach it. Signed-off-by: Glenn Gore --- packages/extension/src/background.ts | 73 ++++++++++++++++++- packages/extension/src/config.ts | 9 ++- .../extension/tests/wallet-inbox.test.mts | 10 ++- 3 files changed, 83 insertions(+), 9 deletions(-) diff --git a/packages/extension/src/background.ts b/packages/extension/src/background.ts index 1a8401e..466ed96 100644 --- a/packages/extension/src/background.ts +++ b/packages/extension/src/background.ts @@ -204,7 +204,10 @@ import { // it constantly and a missed event would otherwise persist until the next one. chrome.permissions.onAdded.addListener(() => void syncProviderRegistration()); chrome.permissions.onRemoved.addListener(() => void syncProviderRegistration()); -chrome.runtime.onStartup.addListener(() => void syncProviderRegistration()); +chrome.runtime.onStartup.addListener(() => { + void syncProviderRegistration(); + void followAgentInbox(); +}); void syncProviderRegistration().then((matches) => { console.info( matches.length > 0 @@ -216,6 +219,9 @@ void syncProviderRegistration().then((matches) => { chrome.runtime.onInstalled.addListener((details) => { console.info("[pnm] extension installed:", details.reason); void ensurePushWake(); + // An update is the other moment worth one DID-document read: it is when a + // deployment's pieces tend to move together. + void followAgentInbox(); // Fresh install: open setup in a tab rather than leaving the user to find // it. The order of the steps there is load-bearing — the agent's address @@ -1399,6 +1405,71 @@ async function handleWalletLockState( })) as RuntimeWalletLockStateResponse; } +/** + * Follow the agent if it has moved its relay. + * + * Distinct from the adopt-when-blank backfill in `startInboundListener`, and + * for a reason that only shows up when you ask how inbound actually arrives: + * **a v4 holder is a `did:key`, which carries no service endpoint, and the + * wallet publishes its inbox to nobody.** There is no discovery path. So an + * executor pushing to this wallet can only hand the message to a mediator it + * already knows — its own — and the wallet hears it only if it is listening + * there. The inbox is not an independent address the wallet owns; it is + * "wherever my agent's relay is", and a wallet pinned to yesterday's mediator + * goes dark while every check still reports green. + * + * So an `agent`-sourced inbox FOLLOWS the agent's DID document. An + * `operator`-sourced one never moves: someone running more than one relay + * chose it, and this is exactly the override that has to survive. + * + * Run on browser startup and on update, not per worker spin-up. MV3 respawns + * the worker on almost any event, and a DID-document fetch on each of those + * would be a lot of network for a value that changes when an operator + * redeploys a mediator. The blank/unattributed backfill still runs every + * spin-up — it reads the persisted connection and costs nothing. + */ +async function followAgentInbox(): Promise { + const settings = await getSettings(); + if (settings.mediatorDidSource === "operator") return; // pinned, deliberately + + const vtaDid = await readActiveVtaDid(); + if (!vtaDid) return; + + let live: string | undefined; + try { + const resp = await handleRefreshVtaTransports({ + type: RUNTIME_REFRESH_VTA_TRANSPORTS, + vtaDid, + }); + if (!resp.ok) throw new Error(resp.error); + live = resp.result.mediatorDid; + } catch (e) { + // A DID document we could not read says nothing about where the relay is, + // so it must not be read as "it moved to nowhere". Keep what we have. + console.warn("[pnm inbound] could not re-resolve the agent's relay:", e); + return; + } + + if (!live) { + console.warn( + "[pnm inbound] the agent advertises no DIDComm relay — nothing can be " + + "pushed to this wallet through it.", + ); + return; + } + if (live === settings.mediatorDid) return; + + await setSettings({ mediatorDid: live, mediatorDidSource: "agent" }); + console.info( + "[pnm inbound] the agent moved its relay:", + settings.mediatorDid ?? "(none)", + "→", + live, + ); + // Re-open on the relay that can actually reach us. + await startInboundListener(); +} + async function handleRefreshVtaTransports( req: RuntimeRefreshVtaTransportsRequest, ): Promise { diff --git a/packages/extension/src/config.ts b/packages/extension/src/config.ts index 31591bc..b3687c5 100644 --- a/packages/extension/src/config.ts +++ b/packages/extension/src/config.ts @@ -134,10 +134,11 @@ export interface WalletSettings { * mediator cannot push to a wallet; an inbox invented here would be a * relay nobody was ever asked about. Unset is the honest state and the * self-test reports it. - * - Set, and someone is on record choosing it → leave it. Either an operator - * picked it (they run more than one relay) or an earlier onboarding adopted - * it — and the inbox is an *address* other parties already route to, so - * moving it silently would strand everyone who knows this wallet. + * - Set, and someone is on record choosing it → leave it here. An operator + * pin is final; an `agent`-sourced one is not frozen either, but moving it + * is the job of `followAgentInbox` in `background.ts`, which re-resolves + * the agent's DID document rather than guessing from a cached connection. + * This function only ever fills a blank. * - Otherwise → adopt the agent's. That covers an unset inbox and, once, the * records written before provenance existed. * diff --git a/packages/extension/tests/wallet-inbox.test.mts b/packages/extension/tests/wallet-inbox.test.mts index babbbb3..fbf8c1e 100644 --- a/packages/extension/tests/wallet-inbox.test.mts +++ b/packages/extension/tests/wallet-inbox.test.mts @@ -48,10 +48,12 @@ test("adopts over a stored inbox nobody is on record choosing", () => { assert.equal(inboxToAdopt({ did: DEMO }, AGENT), AGENT); }); -test("the adoption happens once, not on every boot", () => { - // Stamped `agent` on the way in, so the next boot leaves it alone even - // though the active agent may since have changed. - assert.equal(inboxToAdopt({ did: AGENT, source: "agent" }, AGENT), undefined); +test("the blank-filling adoption happens once, not on every boot", () => { + // Stamped `agent` on the way in, so the per-spin-up backfill leaves it + // alone. Moving an agent-sourced inbox when the agent moves its relay is + // `followAgentInbox`'s job — it re-resolves the DID document, where this + // function only ever reads a cached connection. + assert.equal(inboxToAdopt({ did: AGENT, source: "agent" }, OTHER), undefined); }); // ─── No mediator may be baked into the source again ───