From 36c696a26cb8819856a4bcd5e68cf13331f8842a Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 00:08:25 +1100 Subject: [PATCH 1/2] fix(navigation): run the visit deadline only while a presenter is attached MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The navigation controller armed a pending visit's 15 s deadline the moment the visit was created. The startup `home` visit is created inside `createServices()` before React renders, so whenever no presenter exists for 15 s — today, the key-setup screen left idle; soon, a first-run flow that keeps `ConnectedApp` hidden for minutes — the visit is already `failed`/`timeout` when the first presenter mounts, and the user's first frame is "This destination couldn’t open … kept for retry". The deadline exists to catch a presenter that never finishes; while no presenter exists its expiry carries no information. The controller's host object gains `attach()`, returning a detach function, and a pending visit's timer is armed only while at least one presenter is attached: `start()` arms at once when one is, `attach()` arms the active pending visit when the first presenter arrives, and the last detach clears it. `useAppNavigation` attaches in a mount-time layout effect, so the startup visit's deadline runs from the moment `ConnectedApp` mounts. `provideNavigation` already spreads the controller's host methods into `services.navigationHost`, so no FOUNDATION file changes and no constant is lengthened: every visit opened while the app is presenting still has exactly its 15 s. Preserved: a visit a mounted presenter never finishes still times out 15 s after that presenter attached (no retry, no loop); ingress/deep-link failures (`fail()`) and `resolve()`'s "keep the original deadline" are untouched; a StrictMode attach→detach→attach restarts the deadline at mount; `attach()` after `dispose()` arms nothing. Tests: `controller.test.ts` — `setup()` attaches a presenter by default, as the app does, so the existing timeout pins hold; a new case shows the startup visit outliving 60 s with no presenter, the deadline starting on attach, standing while a second presenter remains, pausing when the last leaves, and a later presenter measured from its own start. `service.test.ts` attaches in its request()-driven setup. The App-level proof through the real first-run gate (`src/app/first-run-navigation.test.tsx`) lands with the Welcome flow later in this stack. docs/plugin-architecture.md says the deadline runs only while a presenter is attached. Stack: PR 01 of the onboarding stack, based on main; nothing depends on it except 07 (Welcome), and it depends on nothing. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- docs/plugin-architecture.md | 4 ++- src/app/navigation.ts | 5 +++ src/features/navigation/controller.test.ts | 40 ++++++++++++++++++++- src/features/navigation/controller.ts | 41 +++++++++++++++++----- src/features/navigation/service.test.ts | 2 ++ 5 files changed, 82 insertions(+), 10 deletions(-) diff --git a/docs/plugin-architecture.md b/docs/plugin-architecture.md index 59daab8201..fcf1659fab 100644 --- a/docs/plugin-architecture.md +++ b/docs/plugin-architecture.md @@ -1219,7 +1219,9 @@ no inline-renderer or panel contract extension is needed. The host provides `ctx.navigation` to plugins declaring `navigation` in `inject`. `open(target)` returns a presentation result, not merely an accepted address: `opened`, `failed` (with a reason), `superseded`, or `cancelled`. The host owns one -history driver, toolbar/keyboard traversal, and a 15-second attempt deadline. +history driver, toolbar/keyboard traversal, and a 15-second attempt deadline +that runs only while a presenter is attached (the first-run gate hides the only +one, so the startup visit's deadline starts when the gate lifts). A visit has stable identity; retrying/reclicking preserves that visit and Forward, while a new destination truncates the forward branch. Leaving aborts the old attempt, and late completion cannot acknowledge a replacement attempt. diff --git a/src/app/navigation.ts b/src/app/navigation.ts index c281aa2ea0..df3fd76d85 100644 --- a/src/app/navigation.ts +++ b/src/app/navigation.ts @@ -43,6 +43,11 @@ export function useAppNavigation( services.plugins.snapshot, ); + // This is the presenter the visit deadline measures. Behind the first-run gate + // none exists, so the startup visit's deadline runs from this mount, not from + // createServices(); otherwise a new user's first frame was the timeout notice. + useLayoutEffect(() => services.navigationHost.attach(), [services]); + const startup = plugins.configuration.status; const target = state.entry.target; const settings = target.kind === "settings"; diff --git a/src/features/navigation/controller.test.ts b/src/features/navigation/controller.test.ts index addd67ce0e..022aea400d 100644 --- a/src/features/navigation/controller.test.ts +++ b/src/features/navigation/controller.test.ts @@ -14,10 +14,12 @@ const page: OpenTarget = { pageId: "board", }; const cleanups: (() => void)[] = []; -function setup(capacity = 100) { +function setup(capacity = 100, presenter = true) { const history = createMemoryHistory(homeTarget, capacity); const host = createNavigationController(history); cleanups.push(host.dispose); + // The deadline measures a presenter; the app's attaches on mount. + if (presenter) cleanups.push(host.attach()); return { host, navigation: host.navigation, history }; } afterEach(() => { @@ -216,6 +218,42 @@ describe("navigation attempts over one history driver", () => { }); }); +it("runs the attempt deadline only while a presenter is attached", async () => { + vi.useFakeTimers(); + const { host, navigation: nav } = setup(100, false); + // The startup visit exists before any presenter; its deadline is not running. + const { attempt } = nav.snapshot(); + expect(nav.snapshot()).toMatchObject({ status: "opening", attempt }); + await vi.advanceTimersByTimeAsync(60_000); + expect(nav.snapshot()).toMatchObject({ status: "opening", attempt }); + // The deadline starts when a presenter attaches and stops when the last leaves. + const first = host.attach(); + await vi.advanceTimersByTimeAsync(14_999); + const second = host.attach(); + first(); + await vi.advanceTimersByTimeAsync(1); + // One presenter remains, so the running deadline stands. + expect(nav.snapshot()).toMatchObject({ + status: "failed", + reason: "timeout", + attempt, + }); + const pending = nav.retry(); + await vi.advanceTimersByTimeAsync(14_999); + second(); + await vi.advanceTimersByTimeAsync(60_000); + expect(nav.snapshot().status).toBe("opening"); + // A presenter attaching later is measured from its own start. + cleanups.push(host.attach()); + await vi.advanceTimersByTimeAsync(14_999); + expect(nav.snapshot().status).toBe("opening"); + await vi.advanceTimersByTimeAsync(1); + await expect(pending).resolves.toEqual({ + status: "failed", + reason: "timeout", + }); +}); + it("default resolution preserves visit, promise, signal and deadline while revoking the old completion token", async () => { vi.useFakeTimers(); const { host, navigation: nav } = setup(); diff --git a/src/features/navigation/controller.ts b/src/features/navigation/controller.ts index 8b7ee78fbc..9966671d98 100644 --- a/src/features/navigation/controller.ts +++ b/src/features/navigation/controller.ts @@ -78,11 +78,14 @@ export function createNavigationController( promise: Promise; resolve: (result: OpenResult) => void; pending: boolean; - timer?: ReturnType; + timer?: ReturnType | undefined; }; let disposed = false; let active: Operation; let snapshot: NavigationSnapshot; + // The deadline measures a presenter. None may exist yet (the first-run gate + // hides the app for minutes), so it runs only while one is attached. + let presenters = 0; let ingressRetry: (() => Promise) | undefined; const listeners = new Set<() => void>(); // Commit state and capture each caller's result before invoking external code. @@ -123,6 +126,16 @@ export function createNavigationController( clearTimeout(operation.timer); operation.resolve(result); } + function arm(operation: Operation) { + if (!presenters || !operation.pending || operation.timer) return; + operation.timer = setTimeout( + () => + transaction(() => { + finish(operation, { status: "failed", reason: "timeout" }); + }), + timeoutMs, + ); + } function finish(operation: Operation, result: OpenResult) { if (operation !== active || !operation.pending) return; settle(operation, result); @@ -174,13 +187,7 @@ export function createNavigationController( finish(operation, { status: "failed", reason: "invalid-target" }); return promise; } - operation.timer = setTimeout( - () => - transaction(() => { - finish(operation, { status: "failed", reason: "timeout" }); - }), - timeoutMs, - ); + arm(operation); emit(); return promise; } @@ -295,6 +302,24 @@ export function createNavigationController( }); return { navigation, + /** + * Host-only: a presenter now exists. The pending visit's deadline starts (or + * restarts) here and stops when the last presenter detaches, so an attempt + * nobody could present never times out, while one a presenter stalls on does. + */ + attach() { + presenters++; + arm(active); + let attached = true; + return () => { + if (!attached) return; + attached = false; + presenters--; + if (presenters) return; + clearTimeout(active.timer); + active.timer = undefined; + }; + }, /** Normalize a pending presentation, not a new intent; never reset its deadline. */ resolve(attempt: OpenAttempt, input: OpenTarget) { return transaction(() => { diff --git a/src/features/navigation/service.test.ts b/src/features/navigation/service.test.ts index 68510fed87..86d383336b 100644 --- a/src/features/navigation/service.test.ts +++ b/src/features/navigation/service.test.ts @@ -24,6 +24,8 @@ function setup(retain = false) { const ctx = new Context(); contexts.push(ctx); const host = provideNavigation(ctx); + // The visit deadline runs only while a presenter is attached, as the app's is. + host.attach(); let live = true; const listeners = new Set<() => void>(); const owner: PresentationOwner = { From b9ea4754f4b950068d04255a0565ed7d9ef53130 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 01:16:05 +1100 Subject: [PATCH 2/2] fix(navigation): arm the deadline in the ThreadPanel regression test and name the identity gate The ThreadPanel deadline regression test never attached a presenter, so the presenter guard left its deadline unarmed and the test no longer protected the 15-second contract. Attach one, as the service tests do. The presenter is held back by IdentitySetup, not the first-run CommunityLanding branch, so name that gate in the docs and comments. Co-authored-by: Jitter <2b2e6415e748c35180847a37aadae06a11e30dddc76b784e7fd9354c4eb42e7a@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- docs/plugin-architecture.md | 5 +++-- src/app/navigation.ts | 7 ++++--- src/features/messages/ThreadPanel.test.tsx | 2 ++ src/features/navigation/controller.ts | 5 +++-- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/plugin-architecture.md b/docs/plugin-architecture.md index fcf1659fab..311401ef58 100644 --- a/docs/plugin-architecture.md +++ b/docs/plugin-architecture.md @@ -1220,8 +1220,9 @@ The host provides `ctx.navigation` to plugins declaring `navigation` in `inject` `open(target)` returns a presentation result, not merely an accepted address: `opened`, `failed` (with a reason), `superseded`, or `cancelled`. The host owns one history driver, toolbar/keyboard traversal, and a 15-second attempt deadline -that runs only while a presenter is attached (the first-run gate hides the only -one, so the startup visit's deadline starts when the gate lifts). +that runs only while a presenter is attached (the identity/key-setup gate, +`IdentitySetup`, hides the only one, so the startup visit's deadline starts when +that gate lifts). A visit has stable identity; retrying/reclicking preserves that visit and Forward, while a new destination truncates the forward branch. Leaving aborts the old attempt, and late completion cannot acknowledge a replacement attempt. diff --git a/src/app/navigation.ts b/src/app/navigation.ts index df3fd76d85..30cd0becba 100644 --- a/src/app/navigation.ts +++ b/src/app/navigation.ts @@ -43,9 +43,10 @@ export function useAppNavigation( services.plugins.snapshot, ); - // This is the presenter the visit deadline measures. Behind the first-run gate - // none exists, so the startup visit's deadline runs from this mount, not from - // createServices(); otherwise a new user's first frame was the timeout notice. + // This is the presenter the visit deadline measures. Behind the identity and + // key-setup gate (IdentitySetup) none exists, so the startup visit's deadline + // runs from this mount, not from createServices(); otherwise a new user's first + // frame was the timeout notice. useLayoutEffect(() => services.navigationHost.attach(), [services]); const startup = plugins.configuration.status; diff --git a/src/features/messages/ThreadPanel.test.tsx b/src/features/messages/ThreadPanel.test.tsx index a3741ef75e..bfdbbcb028 100644 --- a/src/features/messages/ThreadPanel.test.tsx +++ b/src/features/messages/ThreadPanel.test.tsx @@ -1210,6 +1210,8 @@ it.each(["ready", "unavailable", "error"] as const)( it("a presented ordinary thread survives the real navigation deadline while history is pending", async () => { vi.useFakeTimers(); const controller = createNavigationController(createMemoryHistory()); + // The visit deadline runs only while a presenter is attached, as the app's is. + controller.attach(); let release = () => {}; try { const navigation = ordinaryNavigation(); diff --git a/src/features/navigation/controller.ts b/src/features/navigation/controller.ts index 9966671d98..5492279bfb 100644 --- a/src/features/navigation/controller.ts +++ b/src/features/navigation/controller.ts @@ -83,8 +83,9 @@ export function createNavigationController( let disposed = false; let active: Operation; let snapshot: NavigationSnapshot; - // The deadline measures a presenter. None may exist yet (the first-run gate - // hides the app for minutes), so it runs only while one is attached. + // The deadline measures a presenter. None may exist yet (the identity and + // key-setup gate, IdentitySetup, hides the app for minutes), so it runs only + // while one is attached. let presenters = 0; let ingressRetry: (() => Promise) | undefined; const listeners = new Set<() => void>();