From 50d9dd40ec49b7d22f1708dfd1a67c64b30981a3 Mon Sep 17 00:00:00 2001 From: jesse-merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Mon, 10 Aug 2026 19:41:44 +1000 Subject: [PATCH] fix(wizard): retain QR owner across prompts --- src/wizard/session.test.ts | 31 +++++++++++++++++++++++ src/wizard/session.ts | 50 +++++++++++++++++++++++++++++--------- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/src/wizard/session.test.ts b/src/wizard/session.test.ts index e482bb04574c..980e32fed0fa 100644 --- a/src/wizard/session.test.ts +++ b/src/wizard/session.test.ts @@ -242,6 +242,37 @@ describe("WizardSession", () => { ); }); + test("retains external QR ownership across a subsequent prompt until its owner settles", async () => { + let settleOwner!: () => void; + const owner = new Promise((resolve) => { + settleOwner = resolve; + }); + const session = createQrSession(async (prompter) => { + await presentQr(prompter, owner); + await prompter.text({ message: "Device label" }); + }); + + const prompt = await session.next(); + if (!prompt.step) { + throw new Error("expected QR step"); + } + await session.answer(prompt.step.id, true); + + const next = await session.next(); + expect(next.step).toMatchObject({ type: "text", message: "Device label" }); + expect(session.hasExternalQrPresentationOwner()).toBe(true); + expect(session.hasOwnedQrPresentation()).toBe(true); + + settleOwner(); + await vi.waitFor(() => expect(session.hasExternalQrPresentationOwner()).toBe(false)); + expect(session.hasOwnedQrPresentation()).toBe(false); + if (!next.step) { + throw new Error("expected text step"); + } + await session.answer(next.step.id, "Work laptop"); + await session.whenSettled(); + }); + test("skips a QR step whose owner settled before its first presentation", async () => { const session = createQrSession(async (prompter) => { await presentQr(prompter, Promise.resolve()); diff --git a/src/wizard/session.ts b/src/wizard/session.ts index 6b7b43fd4ae0..54220fbd8272 100644 --- a/src/wizard/session.ts +++ b/src/wizard/session.ts @@ -333,8 +333,9 @@ export class WizardSession { private stepDeferred: Deferred | null = null; private pendingTerminalResolution = false; private cancellationLocked = false; - private qrPresentationOwned = false; - private qrPresentationHasExternalOwner = false; + private ownedQrStepIds = new Set(); + private externalQrOwnerStepIds = new Set(); + private settledExternalQrOwnerStepIds = new Set(); private readonly onQrPresentationOwnerSettled: ((stepId: string) => void) | undefined; private settled = false; private pendingExternalUrl: string | undefined; @@ -373,7 +374,7 @@ export class WizardSession { } async next(): Promise { - if (this.currentStep?.type === "qr" && this.qrPresentationHasExternalOwner) { + if (this.currentStep?.type === "qr" && this.externalQrOwnerStepIds.has(this.currentStep.id)) { // Give an already-settled owner callback one microtask to retire the QR before this poll // snapshots it. Otherwise a poll in the same turn can replay expired credential bytes. await Promise.resolve(); @@ -397,7 +398,7 @@ export class WizardSession { this.stepDeferred = createDeferred(); } const step = await this.stepDeferred.promise; - if (step?.type === "qr" && this.qrPresentationHasExternalOwner) { + if (step?.type === "qr" && this.externalQrOwnerStepIds.has(step.id)) { // The owner may settle while the first consumer wakes; let its continuation // publish the next state before returning a QR that is already unusable. await Promise.resolve(); @@ -472,6 +473,9 @@ export class WizardSession { if (this.currentStep?.qrDataUrl) { delete this.currentStep.qrDataUrl; } + if (!this.externalQrOwnerStepIds.has(stepId)) { + this.ownedQrStepIds.delete(stepId); + } this.currentStep = null; pending.deferred.resolve(normalizedValue); return undefined; @@ -491,6 +495,12 @@ export class WizardSession { dismissStep(stepId: string, result: { value: unknown } | { error: unknown }): boolean { // Owner settlement matters even after the client acknowledged the QR; the host may still // be enforcing the credential deadline while the runner applies the truthful owner result. + this.settledExternalQrOwnerStepIds.add(stepId); + if (this.currentStep && this.currentStep.id !== stepId) { + this.ownedQrStepIds.delete(stepId); + this.externalQrOwnerStepIds.delete(stepId); + this.settledExternalQrOwnerStepIds.delete(stepId); + } this.onQrPresentationOwnerSettled?.(stepId); const pending = this.answerDeferred.get(stepId); if (!pending) { @@ -537,6 +547,9 @@ export class WizardSession { this.progressSteps = []; this.deliveredProgressStepIds.clear(); this.dismissedStepIds.clear(); + this.ownedQrStepIds.clear(); + this.externalQrOwnerStepIds.clear(); + this.settledExternalQrOwnerStepIds.clear(); this.resolveStep(null); return true; } @@ -552,17 +565,17 @@ export class WizardSession { /** Keep a wizard eviction-protected while its QR-owned operation is still in flight. */ hasOwnedQrPresentation(): boolean { - return this.qrPresentationOwned && this.status === "running" && !this.settled; + return this.ownedQrStepIds.size > 0 && this.status === "running" && !this.settled; } /** True when a producer promise, rather than the acknowledgement, owns QR completion. */ hasExternalQrPresentationOwner(): boolean { - return this.hasOwnedQrPresentation() && this.qrPresentationHasExternalOwner; + return this.externalQrOwnerStepIds.size > 0 && this.status === "running" && !this.settled; } /** Retire an expired credential while its external owner finishes or rejects the operation. */ expireOwnedQrPresentation(stepId: string): boolean { - if (!this.hasExternalQrPresentationOwner()) { + if (!this.externalQrOwnerStepIds.has(stepId) || !this.hasExternalQrPresentationOwner()) { return false; } const pending = this.answerDeferred.get(stepId); @@ -652,6 +665,9 @@ export class WizardSession { } } finally { this.settled = true; + this.ownedQrStepIds.clear(); + this.externalQrOwnerStepIds.clear(); + this.settledExternalQrOwnerStepIds.clear(); if (this.expiryTimer) { clearTimeout(this.expiryTimer); } @@ -667,11 +683,21 @@ export class WizardSession { if (this.status !== "running") { throw new Error("wizard: session not running"); } - // A later interactive step proves the QR-owned operation finished. Release the - // eviction guard there; a runner stalled between steps remains protected by its QR timer. - this.qrPresentationOwned = Boolean(step.qrDataUrl); - this.qrPresentationHasExternalOwner = - this.qrPresentationOwned && qrPresentationHasExternalOwner; + // Once a settled QR owner emits its next step, it can no longer mutate state behind that + // step. Retire only those owners; unresolved owners stay protected across later prompts. + for (const stepId of this.settledExternalQrOwnerStepIds) { + this.ownedQrStepIds.delete(stepId); + this.externalQrOwnerStepIds.delete(stepId); + } + this.settledExternalQrOwnerStepIds.clear(); + // Acknowledgement can let the producer emit another prompt before its QR owner settles. + // Keep ownership keyed to that QR so later prompts cannot drop its eviction guard. + if (step.type === "qr") { + this.ownedQrStepIds.add(step.id); + if (qrPresentationHasExternalOwner) { + this.externalQrOwnerStepIds.add(step.id); + } + } this.pushStep(step); const deferred = createDeferred(); this.answerDeferred.set(step.id, { deferred, text: step.type === "text", validate });