From 4ef7b03651f0d80bfe71ca723b169c30feb5c321 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 20 Oct 2026 22:35:38 +0900 Subject: [PATCH] fix: stop background session checks and loading flicker --- web/workspace/src/lib/workspace/api/http.ts | 8 - .../lib/workspace/auth/SessionBoundary.svelte | 7 +- .../workspace/auth/browser-session.svelte.ts | 5 + .../auth/page-session.browser.spec.ts | 101 ++++++++ .../auth/session-boundary.browser.spec.ts | 34 ++- .../auth/session-state.browser.spec.ts | 33 ++- .../src/lib/workspace/auth/session-state.ts | 59 ++--- web/workspace/src/routes/+page.svelte | 12 +- web/workspace/src/routes/account/+page.svelte | 2 +- .../src/routes/dashboard/+page.svelte | 8 +- .../src/routes/entry.browser.spec.ts | 38 +-- .../src/routes/sidebar-fold.browser.spec.ts | 5 +- .../auth-browser/entry-shell.browser.test.ts | 243 ++++++------------ .../test/auth-browser/session-navigation.ts | 62 +++-- 14 files changed, 334 insertions(+), 283 deletions(-) create mode 100644 web/workspace/src/lib/workspace/auth/page-session.browser.spec.ts diff --git a/web/workspace/src/lib/workspace/api/http.ts b/web/workspace/src/lib/workspace/api/http.ts index 02dc0f39e..27773b1bc 100644 --- a/web/workspace/src/lib/workspace/api/http.ts +++ b/web/workspace/src/lib/workspace/api/http.ts @@ -1,7 +1,3 @@ -import { - browserSession, - recheckAfterUnauthorized, -} from "#lib/workspace/auth/session-state.ts"; import { SKILL_API_LIMITS } from "#lib/generated/skill-api.ts"; import type { SkillCatalogResponse, @@ -106,11 +102,7 @@ export async function loadJson( ): Promise> { let stage: "request" | "decode" | "validate" = "request"; try { - const authenticationGeneration = browserSession.authenticationGeneration; const response = await fetchFn(path, init); - if (response.status === 401) { - recheckAfterUnauthorized(authenticationGeneration); - } if (!response.ok) { if (policy) { await response.body?.cancel(); diff --git a/web/workspace/src/lib/workspace/auth/SessionBoundary.svelte b/web/workspace/src/lib/workspace/auth/SessionBoundary.svelte index e910913e6..943d782e7 100644 --- a/web/workspace/src/lib/workspace/auth/SessionBoundary.svelte +++ b/web/workspace/src/lib/workspace/auth/SessionBoundary.svelte @@ -1,7 +1,7 @@ diff --git a/web/workspace/src/lib/workspace/auth/browser-session.svelte.ts b/web/workspace/src/lib/workspace/auth/browser-session.svelte.ts index d69840eb2..9adc8e0cc 100644 --- a/web/workspace/src/lib/workspace/auth/browser-session.svelte.ts +++ b/web/workspace/src/lib/workspace/auth/browser-session.svelte.ts @@ -61,6 +61,11 @@ export class BrowserAuth { return this.session.kind === "ready" ? this.session.actor : null; } + async ensure(): Promise { + if (this.busy || this.disposed) return; + await browserSession.ensure(); + } + async refresh(): Promise { if (this.busy || this.disposed) return; await browserSession.refresh(); diff --git a/web/workspace/src/lib/workspace/auth/page-session.browser.spec.ts b/web/workspace/src/lib/workspace/auth/page-session.browser.spec.ts new file mode 100644 index 000000000..456b5d3eb --- /dev/null +++ b/web/workspace/src/lib/workspace/auth/page-session.browser.spec.ts @@ -0,0 +1,101 @@ +// @vitest-environment happy-dom +import { afterEach, expect, test, vi } from "vitest"; +import { browserSession, observeBrowserSession } from "./session-state.ts"; +import { requirePageSession } from "./page-session.ts"; +import { loadJson } from "../api/http.ts"; +import { authUser, whoamiPayload } from "./api.test-fixtures.ts"; + +const actor = { ...authUser, auth_method: "browser_session" }; +const target = new URL("http://localhost/w/team/tickets?q=mine#entry"); +afterEach(() => { + browserSession.invalidate(); + localStorage.clear(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +function fixture() { + const fetcher = vi.fn(async () => Response.json(whoamiPayload(actor))); + vi.stubGlobal("fetch", fetcher); + return fetcher; +} + +test("focus, visibility and pageshow never recheck an admitted session", async () => { + const fetcher = fixture(); + await requirePageSession(target); + const admitted = browserSession.snapshot; + for (let i = 0; i < 3; i++) { + window.dispatchEvent(new Event("focus")); + window.dispatchEvent(new Event("pageshow")); + document.dispatchEvent(new Event("visibilitychange")); + } + await Promise.resolve(); + expect(fetcher).toHaveBeenCalledTimes(1); + expect(browserSession.snapshot).toBe(admitted); +}); + +test("concurrent API 401 responses return errors without checking or changing the session", async () => { + const fetcher = fixture(); + await requirePageSession(target); + const admitted = browserSession.snapshot; + const unauthorized = vi.fn(async () => + new Response("denied", { status: 401 }) + ); + const results = await Promise.all([ + loadJson(unauthorized, "/api/w/team/workspace"), + loadJson(unauthorized, "/api/w/team/repositories"), + ]); + for (const result of results) { + expect(result).toEqual({ data: null, error: "denied", status: 401 }); + } + expect(fetcher).toHaveBeenCalledTimes(1); + expect(browserSession.snapshot).toBe(admitted); +}); + +for (const kind of ["anonymous", "expired"] as const) { + test(`the next protected page access checks again and redirects ${kind}`, async () => { + const fetcher = fixture(); + await requirePageSession(target); + await browserSession.ensure(); + expect(fetcher).toHaveBeenCalledTimes(1); + fetcher.mockResolvedValueOnce( + Response.json({ actor: null, session: { kind } }), + ); + await expect(requirePageSession(target)).rejects.toMatchObject({ + status: 307, + location: kind === "anonymous" + ? "/" + : "/login?returnTo=%2Fw%2Fteam%2Ftickets%3Fq%3Dmine%23entry", + }); + expect(fetcher).toHaveBeenCalledTimes(2); + }); +} + +test("storage notifications fence the cache without background requests or loading", async () => { + const fetcher = fixture(); + observeBrowserSession(); + await requirePageSession(target); + const admitted = browserSession.snapshot; + const key = "yoi.browser-auth-generation"; + const newValue = JSON.stringify({ + id: "other-tab", + reason: "authentication", + }); + localStorage.setItem(key, newValue); + window.dispatchEvent(new StorageEvent("storage", { key, newValue })); + await Promise.resolve(); + expect(fetcher).toHaveBeenCalledTimes(1); + expect(browserSession.snapshot).toBe(admitted); + await requirePageSession(target); + expect(fetcher).toHaveBeenCalledTimes(2); +}); + +test("a failed page check reports unknown instead of admitting or redirecting", async () => { + const fetcher = fixture(); + await requirePageSession(target); + fetcher.mockResolvedValueOnce(new Response(null, { status: 503 })); + await expect(requirePageSession(target)).rejects.toMatchObject({ + status: 503, + }); + expect(browserSession.snapshot.kind).toBe("error"); +}); diff --git a/web/workspace/src/lib/workspace/auth/session-boundary.browser.spec.ts b/web/workspace/src/lib/workspace/auth/session-boundary.browser.spec.ts index 19bf093ef..80530ea4d 100644 --- a/web/workspace/src/lib/workspace/auth/session-boundary.browser.spec.ts +++ b/web/workspace/src/lib/workspace/auth/session-boundary.browser.spec.ts @@ -9,13 +9,15 @@ import { goto } from "$app/navigation"; const page = vi.hoisted(() => ({ url: new URL("http://localhost:3000/w/team/tickets?q=mine#entry"), })); -vi.mock("$app/state", () => ({ page })); +const navigation = vi.hoisted(() => ({ to: null as { url: URL } | null })); +vi.mock("$app/state", () => ({ page, navigating: navigation })); vi.mock( "$app/navigation", () => ({ goto: vi.fn(async () => {}), invalidate: vi.fn(async () => {}) }), ); afterEach(() => { cleanup(); + navigation.to = null; browserSession.invalidate(); vi.unstubAllGlobals(); vi.restoreAllMocks(); @@ -54,7 +56,23 @@ test("active expired state removes private content and preserves browser query a ); }); -test("unknown state hides and inerts admitted content instead of declaring anonymous", async () => { +test("navigation admission does not redirect to the old page return URL", async () => { + await admitted(); + navigation.to = { url: new URL("http://localhost/dashboard") }; + vi.stubGlobal( + "fetch", + vi.fn(async () => + Response.json({ actor: null, session: { kind: "expired" } }) + ), + ); + await browserSession.refresh(); + await waitFor(() => + expect(screen.queryByText("Private fragment")).toBeNull() + ); + expect(goto).not.toHaveBeenCalled(); +}); + +test("page access keeps admitted content visible while checking, but a failed check is unknown", async () => { await admitted(); let release!: (response: Response) => void; vi.stubGlobal( @@ -66,9 +84,9 @@ test("unknown state hides and inerts admitted content instead of declaring anony ), ); const checking = browserSession.refresh(); - await screen.findByText("Loading session…"); - expect(screen.getByText("Private fragment").closest("[hidden][inert]")) - .toBeTruthy(); + expect(screen.queryByText("Loading session…")).toBeNull(); + expect(screen.getByText("Private fragment").closest("[hidden]")) + .toBeNull(); release(new Response(null, { status: 503 })); await checking; await screen.findByText( @@ -99,7 +117,7 @@ test("another tab changing actor cannot reveal content loaded for the old actor" expect(screen.queryByText("Private fragment")).toBeNull(); }); -test("a child mount session recheck does not unmount and restart its own loader", async () => { +test("a child mount reuses the page session without another request or remount", async () => { window.history.replaceState({}, "", "/w/team/tickets?q=mine#entry"); const fetcher = vi.fn(async () => Response.json(whoamiPayload(actor))); vi.stubGlobal("fetch", fetcher); @@ -109,7 +127,7 @@ test("a child mount session recheck does not unmount and restart its own loader" render: () => "

Mounted session reader

", setup: () => { ++mounts; - void browserSession.refresh(); + void browserSession.ensure(); }, })); render(SessionBoundary, { children: checkingChild }); @@ -117,5 +135,5 @@ test("a child mount session recheck does not unmount and restart its own loader" expect(screen.getByText("Mounted session reader").closest("[hidden]")) .toBeNull(); expect(mounts).toBe(1); - expect(fetcher).toHaveBeenCalledTimes(2); + expect(fetcher).toHaveBeenCalledTimes(1); }); diff --git a/web/workspace/src/lib/workspace/auth/session-state.browser.spec.ts b/web/workspace/src/lib/workspace/auth/session-state.browser.spec.ts index 5ef8ddf6a..238024265 100644 --- a/web/workspace/src/lib/workspace/auth/session-state.browser.spec.ts +++ b/web/workspace/src/lib/workspace/auth/session-state.browser.spec.ts @@ -79,19 +79,48 @@ test("storage generation fences responses even before the other tab storage even expect(session.snapshot).toEqual({ kind: "ready", value: authenticated() }); }); -test("active deadline rechecks the server and reports failures as unknown, never inferred expiry", async () => { +test("deadline does not recheck; the next page check reads the server", async () => { vi.useFakeTimers(); const read = vi.fn<() => Promise>().mockResolvedValueOnce( authenticated(new Date(Date.now() + 5_000).toISOString()), ).mockRejectedValueOnce(new Error("network")).mockResolvedValue(expired); const session = state(read); await session.refresh(); - await vi.advanceTimersByTimeAsync(5_000); + const admitted = session.snapshot; + await vi.advanceTimersByTimeAsync(60_000); + expect(read).toHaveBeenCalledTimes(1); + expect(session.snapshot).toBe(admitted); + await session.refresh(); expect(session.snapshot).toEqual({ kind: "error" }); await session.refresh(); expect(session.snapshot).toEqual({ kind: "ready", value: expired }); }); +test("mounted readers reuse the page check and page access still refreshes", async () => { + const read = vi.fn(async () => authenticated()); + const session = state(read); + await session.refresh(); + await session.ensure(); + await session.ensure(); + expect(read).toHaveBeenCalledTimes(1); + await session.refresh(); + expect(read).toHaveBeenCalledTimes(2); +}); + +test("an admitted page remains ready while its next page check is pending", async () => { + const next = gate(); + const read = vi.fn<() => Promise>() + .mockResolvedValueOnce(authenticated()).mockReturnValueOnce(next.promise); + const session = state(read); + await session.refresh(); + const previous = session.snapshot; + const checking = session.refresh(); + expect(session.snapshot).toBe(previous); + next.resolve(expired); + await checking; + expect(session.snapshot).toEqual({ kind: "ready", value: expired }); +}); + test("a challenge failure is not session expiry; recheck may still be authenticated or anonymous", async () => { for (const value of [authenticated(), anonymous]) { const session = state(async () => value); diff --git a/web/workspace/src/lib/workspace/auth/session-state.ts b/web/workspace/src/lib/workspace/auth/session-state.ts index 2ff372940..202398363 100644 --- a/web/workspace/src/lib/workspace/auth/session-state.ts +++ b/web/workspace/src/lib/workspace/auth/session-state.ts @@ -15,7 +15,7 @@ export class SessionState { private generation = 0; private pending: Promise | null = null; private listeners = new Set<(state: SessionSnapshot) => void>(); - private timer: ReturnType | undefined; + private stale = true; get authenticationGeneration(): number { return this.generation; @@ -31,34 +31,29 @@ export class SessionState { private publish(snapshot: SessionSnapshot): void { this.snapshot = snapshot; - clearTimeout(this.timer); - if ( - snapshot.kind === "ready" && - snapshot.value.session.kind === "authenticated" - ) { - const expiry = snapshot.value.session.expires_at; - if (expiry !== null) { - // Timerは再照会の契機。local clockだけで期限切れと断定しない。 - const remaining = Date.parse(expiry) - Date.now(); - this.timer = setTimeout(() => { - void this.refresh(); - }, Math.min(2_147_483_647, Math.max(1_000, remaining))); - } - } for (const listener of this.listeners) listener(snapshot); } - invalidate(): void { + invalidate(preserveSnapshot = false): void { ++this.generation; this.pending = null; - this.publish({ kind: "loading" }); + this.stale = true; + if (!preserveSnapshot) this.publish({ kind: "loading" }); + } + + // Page load owns freshness. Components mounted after it reuse that result. + async ensure(): Promise { + if (this.pending) return this.pending; + if (!this.stale && this.snapshot.kind === "ready") return this.snapshot; + return this.refresh(); } async refresh(): Promise { if (this.pending) return this.pending; const generation = this.generation; const stamp = storageStamp(); - this.publish({ kind: "loading" }); + // Keep the admitted page visible while a new page access is checked. + if (this.snapshot.kind !== "ready") this.publish({ kind: "loading" }); const pending = (async () => { let result: SessionSnapshot; try { @@ -70,13 +65,16 @@ export class SessionState { // 新世代が既に確定していれば再取得すら不要。古い完了で確定結果を巻き戻さない。 if (generation !== this.generation) { if (this.pending) return await this.pending; - if (this.snapshot.kind !== "loading") return this.snapshot; + if (!this.stale && this.snapshot.kind !== "loading") { + return this.snapshot; + } } // 別tabの操作がstorage eventより先に完了していても古い応答を採用しない。 if (generation === this.generation) this.invalidate(); return await this.refresh(); } this.pending = null; + this.stale = false; this.publish(result); return result; })(); @@ -85,7 +83,6 @@ export class SessionState { } dispose(): void { - clearTimeout(this.timer); this.listeners.clear(); ++this.generation; } @@ -120,34 +117,18 @@ function clearReturnTo(): void { } let installed = false; -export function recheckAfterUnauthorized( - authenticationGeneration: number, -): void { - if ( - installed && - authenticationGeneration === browserSession.authenticationGeneration - ) void browserSession.refresh(); -} export function observeBrowserSession(): void { if (installed || typeof window === "undefined") return; installed = true; - const refresh = () => { - void browserSession.refresh(); - }; - window.addEventListener("focus", refresh); - window.addEventListener("pageshow", refresh); - document.addEventListener("visibilitychange", () => { - if (!document.hidden) refresh(); - }); window.addEventListener("storage", (event) => { if (event.key !== GENERATION_KEY && event.key !== null) return; - browserSession.invalidate(); + // Fence old responses, but leave this page alone until the next page access. + browserSession.invalidate(true); try { if ( event.key === null || JSON.parse(event.newValue ?? "null")?.reason === "logout" ) clearReturnTo(); - } catch { /* 不明な通知も権限ではなく再照会だけ。 */ } - refresh(); + } catch { /* 不明な通知でも次のページアクセス時に確認する。 */ } }); } diff --git a/web/workspace/src/routes/+page.svelte b/web/workspace/src/routes/+page.svelte index 83363a428..cbc762ad4 100644 --- a/web/workspace/src/routes/+page.svelte +++ b/web/workspace/src/routes/+page.svelte @@ -1,7 +1,6 @@ diff --git a/web/workspace/src/routes/account/+page.svelte b/web/workspace/src/routes/account/+page.svelte index 09dacabe6..d86a4f0ba 100644 --- a/web/workspace/src/routes/account/+page.svelte +++ b/web/workspace/src/routes/account/+page.svelte @@ -19,7 +19,7 @@ const confirmed = await auth.run('Logging out…', 'Logged out.', logout, 'logout'); if (confirmed && !auth.actor) await goto('/', { replaceState: true }); } - onMount(() => { void refresh(); return () => auth.dispose(); }); + onMount(() => { void auth.ensure(); return () => auth.dispose(); }); Account · Yoi diff --git a/web/workspace/src/routes/dashboard/+page.svelte b/web/workspace/src/routes/dashboard/+page.svelte index c131f33c1..ff722afb9 100644 --- a/web/workspace/src/routes/dashboard/+page.svelte +++ b/web/workspace/src/routes/dashboard/+page.svelte @@ -1,7 +1,6 @@ diff --git a/web/workspace/src/routes/entry.browser.spec.ts b/web/workspace/src/routes/entry.browser.spec.ts index cc0b06ad1..6fc1a06c7 100644 --- a/web/workspace/src/routes/entry.browser.spec.ts +++ b/web/workspace/src/routes/entry.browser.spec.ts @@ -181,7 +181,7 @@ for (const admin of [false, true]) { else {expect(screen.queryByRole("link", { name: "Hosting settings" })) .toBeNull();} state.signedIn = false; - await fireEvent(window, new Event(SESSION_CHANGED_EVENT)); + await browserSession.refresh(); await screen.findByRole("link", { name: "Log in" }); expect(screen.queryByRole("link", { name: "Hosting settings" })).toBeNull(); }); @@ -233,7 +233,7 @@ test("dashboard removes private content on session change and ignores old catalo render(Dashboard); await screen.findByText("Loading Workspaces…"); state.signedIn = false; - await fireEvent(window, new Event(SESSION_CHANGED_EVENT)); + await browserSession.refresh(); await screen.findByText("Log in to select or create a Workspace."); state.catalogGate.resolve(Response.json([createdWorkspace])); expect(screen.queryByText("New Workspace")).toBeNull(); @@ -246,6 +246,7 @@ test("dashboard does not contact catalog for anonymous or unknown session", asyn expect(workspaceCalls()).toEqual([]); view.unmount(); state.whoamiStatus = 503; + browserSession.invalidate(); render(Dashboard); await screen.findByRole("alert"); expect(workspaceCalls()).toEqual([]); @@ -268,7 +269,7 @@ const creationBodies = (fetcher: ReturnType["fetcher"]) => [, init], ) => JSON.parse(String(init!.body))); -test("same actor focus retains creation disclosure, repository fields, errors and exact retry key through unknown session", async () => { +test("focus retains creation inputs, errors and retry key without a session request", async () => { const { state, fetcher } = boundary(); state.signedIn = true; state.createStatus = 503; @@ -288,16 +289,15 @@ test("same actor focus retains creation disclosure, repository fields, errors an ); await submitCreation(); await screen.findByRole("alert"); - state.sessionGate = deferred(); + const sessionCalls = () => + fetcher.mock.calls.filter(([path]) => path === "/api/auth/whoami").length; + const count = sessionCalls(); await fireEvent(window, new Event("focus")); - await screen.findByText("Loading session…"); - expect(screen.queryByLabelText("Workspace display name")).toBeNull(); - state.sessionGate.resolve( - Response.json({ - session: { kind: "authenticated", expires_at: "2099-01-01T00:00:00Z" }, - actor: { ...authUser, auth_method: "browser_session" }, - }), - ); + await fireEvent(window, new Event("pageshow")); + await fireEvent(document, new Event("visibilitychange")); + await fireEvent(window, new Event(SESSION_CHANGED_EVENT)); + expect(sessionCalls()).toBe(count); + expect(screen.queryByText("Loading session…")).toBeNull(); expect( (await screen.findByLabelText("Workspace display name") as HTMLInputElement) .value, @@ -315,7 +315,7 @@ test("same actor focus retains creation disclosure, repository fields, errors an expect(creationBodies(fetcher)[0]).toEqual(creationBodies(fetcher)[1]); }); -test("pending create remains single across focus and a success during unknown session becomes an explicit confirmed-result link", async () => { +test("pending create remains single during an explicit authentication change and retains its result", async () => { const { state, fetcher } = boundary(); state.signedIn = true; state.creationGate = deferred(); @@ -324,7 +324,8 @@ test("pending create remains single across focus and a success during unknown se await submitCreation(); await screen.findByRole("button", { name: "Creating…" }); state.sessionGate = deferred(); - await fireEvent(window, new Event("focus")); + browserSession.invalidate(); + const checking = browserSession.refresh(); await screen.findByText("Loading session…"); const response = Response.json(creationResponse()); state.creationGate.resolve(response); @@ -336,6 +337,7 @@ test("pending create remains single across focus and a success during unknown se actor: { ...authUser, auth_method: "browser_session" }, }), ); + await checking; expect( (await screen.findByRole("link", { name: "Open created Workspace" })) .getAttribute("href"), @@ -351,7 +353,7 @@ test("same actor session error retains draft privately until retry; confirmed di render(Dashboard); await openCreation(); state.whoamiStatus = 503; - await fireEvent(window, new Event(SESSION_CHANGED_EVENT)); + await browserSession.refresh(); await screen.findByRole("alert"); expect(screen.queryByLabelText("Workspace display name")).toBeNull(); state.whoamiStatus = 200; @@ -361,7 +363,7 @@ test("same actor session error retains draft privately until retry; confirmed di .value, ).toBe("Retained draft"); state.accountId = "different-account"; - await fireEvent(window, new Event(SESSION_CHANGED_EVENT)); + await browserSession.refresh(); await screen.findByText("No accessible Workspaces"); await fireEvent.click( screen.getByText("Create Workspace", { selector: "summary" }), @@ -380,14 +382,14 @@ test("late creation result after confirmed logout or actor change never navigate await submitCreation(); await screen.findByRole("button", { name: "Creating…" }); state.signedIn = false; - await fireEvent(window, new Event(SESSION_CHANGED_EVENT)); + await browserSession.refresh(); await screen.findByRole("link", { name: "Log in" }); const response = Response.json(creationResponse()); state.creationGate.resolve(response); await waitFor(() => expect(response.bodyUsed).toBe(true)); expect(goto).not.toHaveBeenCalled(); state.signedIn = true; - await fireEvent(window, new Event(SESSION_CHANGED_EVENT)); + await browserSession.refresh(); await screen.findByText("No accessible Workspaces"); expect(screen.queryByRole("link", { name: "Open created Workspace" })) .toBeNull(); diff --git a/web/workspace/src/routes/sidebar-fold.browser.spec.ts b/web/workspace/src/routes/sidebar-fold.browser.spec.ts index bf05c26e4..d6f68ba3c 100644 --- a/web/workspace/src/routes/sidebar-fold.browser.spec.ts +++ b/web/workspace/src/routes/sidebar-fold.browser.spec.ts @@ -19,7 +19,10 @@ import Layout from "./+layout.svelte"; vi.mock( "$app/state", - () => ({ page: { url: new URL("https://example.test/w/example") } }), + () => ({ + page: { url: new URL("https://example.test/w/example") }, + navigating: { to: null }, + }), ); const storageKey = "yoi.sidebar.mode.v1"; const legacyKey = "yoi.sidebar.folded.v1"; diff --git a/web/workspace/test/auth-browser/entry-shell.browser.test.ts b/web/workspace/test/auth-browser/entry-shell.browser.test.ts index 659ae56c0..ee5bf98ac 100644 --- a/web/workspace/test/auth-browser/entry-shell.browser.test.ts +++ b/web/workspace/test/auth-browser/entry-shell.browser.test.ts @@ -12,6 +12,27 @@ const root = resolve(import.meta.dirname!, "../../../.."); const origin = "http://127.0.0.1:15411"; const ready = (page: Page, name: string) => page.getByRole("link", { name, exact: true }).waitFor(); +async function focusWithoutSessionCheck(page: Page) { + let checks = 0; + const observe = (request: import("playwright").Request) => { + if (new URL(request.url()).pathname === "/api/auth/whoami") checks++; + }; + page.on("request", observe); + try { + await page.evaluate(async () => { + window.dispatchEvent(new Event("focus")); + window.dispatchEvent(new Event("pageshow")); + document.dispatchEvent(new Event("visibilitychange")); + await new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())) + ); + }); + assertEquals(checks, 0); + assertEquals(await page.getByText("Loading session…").count(), 0); + } finally { + page.off("request", observe); + } +} Deno.test("public entry isolates Workspace authority and authenticated dashboard supports explicit selection", async (t) => { const processes = await startOwnedProcesses( [ @@ -299,23 +320,17 @@ Deno.test("public entry isolates Workspace authority and authenticated dashboard ); assertEquals(retainedCounts, [0, 0, 0, 0]); assertEquals(await page.locator(".workspace-alert").count(), 0); - let release!: () => void; - const recheck = new Promise((resolve) => { - release = resolve; - }); - await page.route("**/api/auth/whoami", async (route) => { - await recheck; - await route.fulfill({ - status: 503, - body: "synthetic unavailable session", - }); - }); - await page.evaluate(() => - window.dispatchEvent(new Event("focus")) + await page.route( + "**/api/auth/whoami", + (route) => + route.fulfill({ + status: 503, + body: "synthetic unavailable session", + }), ); - await page.getByText("Loading session…").waitFor(); + await focusWithoutSessionCheck(page); assertEquals(await page.locator(".workspace-alert").count(), 0); - release(); + await page.reload(); await page.getByText(/Login status is unknown/).waitFor(); assertEquals(await page.locator(".workspace-alert").count(), 0); assertEquals( @@ -355,7 +370,7 @@ Deno.test("public entry isolates Workspace authority and authenticated dashboard ); } await step( - "session retry and focus/session change remove stale actor/admin links", + "session retry and page access refresh actor/admin links", async (page) => { await page.goto(`${origin}/?fixture=session-retry`); await page.getByRole("button", { name: "Retry session" }).click(); @@ -364,9 +379,8 @@ Deno.test("public entry isolates Workspace authority and authenticated dashboard await ready(page, "Hosting settings"); await ready(page, "Open dashboard"); await page.request.post(`${origin}/api/auth/logout`); - await page.evaluate(() => - window.dispatchEvent(new Event("yoi:browser-session-changed")) - ); + await focusWithoutSessionCheck(page); + await page.reload(); await ready(page, "Log in"); assertEquals( await page.getByRole("link", { name: "Hosting settings" }) @@ -513,13 +527,7 @@ Deno.test("public entry isolates Workspace authority and authenticated dashboard exact: true, }).click(); await submitted; - const checked = page.waitForResponse((response) => - new URL(response.url()).pathname === "/api/auth/whoami" - ); - await page.evaluate(() => - window.dispatchEvent(new Event("focus")) - ); - await checked; + await focusWithoutSessionCheck(page); await page.getByRole("button", { name: "Creating…", exact: true }) .waitFor(); assertEquals( @@ -541,150 +549,55 @@ Deno.test("public entry isolates Workspace authority and authenticated dashboard }, ); await step( - "success during unknown session is retained for explicit same-actor recovery", + "SPA page access checks once without hiding the admitted page", async (page) => { - let releaseCreate!: () => void; - const createGate = new Promise((resolve) => { - releaseCreate = resolve; + let checks = 0; + page.on("request", (request) => { + if (new URL(request.url()).pathname === "/api/auth/whoami") { + checks++; + } }); + await page.goto(origin + "/dashboard?fixture=account"); + await ready(page, "Account"); + assertEquals(checks, 1); + let release!: () => void; let started!: () => void; - const submitted = new Promise((resolve) => { + const held = new Promise((resolve) => { + release = resolve; + }); + const requested = new Promise((resolve) => { started = resolve; }); - await page.route("**/api/workspaces", async (route) => { - if (route.request().method() !== "POST") { - return route.continue(); - } - started(); - await createGate; - await route.fulfill({ json: creationResponse() }); - }); - const document = await page.goto( - `${origin}/dashboard?fixture=account#workspace-create-title`, - ); - await page.getByLabel("Workspace display name").fill( - "Confirmed result during recheck", - ); - await page.getByRole("button", { - name: "Create Workspace", - exact: true, - }).click(); - await submitted; - let releaseSession!: () => void; - const sessionGate = new Promise((resolve) => { - releaseSession = resolve; - }); await page.route("**/api/auth/whoami", async (route) => { - await sessionGate; - await route.fulfill({ - json: { - actor: { - user_id: "fixture-user", - account_id: "fixture-account", - handle: "alice", - display_name: "Alice", - auth_method: "browser_session", - }, - session: { - kind: "authenticated", - expires_at: "2099-01-01T00:00:00Z", - }, - }, - }); + started(); + await held; + await route.continue(); }); - await page.evaluate(() => - window.dispatchEvent(new Event("focus")) - ); - await page.getByRole("status").filter({ - hasText: "Loading session…", - }) - .waitFor(); - const completed = page.waitForResponse((response) => - response.request().method() === "POST" && - new URL(response.url()).pathname === "/api/workspaces" - ); - releaseCreate(); - await (await completed).finished(); - // HTTP完了後のbrowser task/render境界まで進め、session不明中には遷移しないことを観測する。 - await page.evaluate(() => - new Promise((resolve) => - requestAnimationFrame(() => resolve()) - ) - ); - assertEquals(new URL(page.url()).pathname, "/dashboard"); - // 確認中のsubtreeは副作用の結果を保持したままhidden/inertになる。 - assertEquals( - await page.getByLabel("Workspace display name").isVisible(), - false, - ); - releaseSession(); - const link = page.getByRole("link", { - name: "Open created Workspace", - exact: true, - }); - await link.waitFor(); - assertEquals(await link.getAttribute("href"), "/w/home-empty"); - assertEquals( + try { + await page.getByRole("link", { name: "Account", exact: true }) + .click(); + await requested; + assertEquals( + await page.getByRole("heading", { + name: "Dashboard", + exact: true, + }).isVisible(), + true, + ); + assertEquals( + await page.getByText("Loading session…").count(), + 0, + ); + release(); await page.getByRole("button", { - name: "Create Workspace", + name: "Add passkey to alice", exact: true, - }).count(), - 0, - ); - const folder = - `${root}/target/web-ux/t767-validation/entry-browser/confirmed-${colorScheme}-${width}`; - await Deno.mkdir(folder, { recursive: true }); - const bytes = await page.screenshot({ - path: folder + "/viewport.png", - fullPage: true, - }); - const hash = Array.from( - new Uint8Array( - await crypto.subtle.digest("SHA-256", Uint8Array.from(bytes)), - ), - ).map((v) => v.toString(16).padStart(2, "0")).join(""); - const source = await new Deno.Command("git", { - cwd: root, - args: ["rev-parse", "HEAD"], - stdout: "piped", - }).output(); - const dirty = await new Deno.Command("git", { - cwd: root, - args: ["status", "--porcelain"], - stdout: "piped", - }).output(); - await Deno.writeTextFile( - folder + "/accessibility.txt", - await page.locator("body").ariaSnapshot(), - ); - await Deno.writeTextFile( - folder + "/review-context.json", - JSON.stringify( - { - scenario: - "production-browser confirmed result after session recheck", - boundary: "synthetic HTTP; not real Server authentication", - source: { - commitSha: new TextDecoder().decode(source.stdout).trim(), - dirty: - new TextDecoder().decode(dirty.stdout).trim() !== "", - }, - document: { - path: new URL(page.url()).pathname, - status: document?.status(), - }, - colorScheme, - viewport: { width, height: 900 }, - screenshot: { path: "viewport.png", sha256: hash }, - visibleAlerts: await page.getByRole("alert") - .allTextContents(), - accessibility: "accessibility.txt", - expectedHttpErrors: [], - }, - null, - 2, - ), - ); + }).waitFor(); + assertEquals(new URL(page.url()).pathname, "/account"); + assertEquals(checks, 2); + } finally { + release(); + } }, ); for (const mode of ["creation-denied", "creation-error"]) { @@ -708,13 +621,7 @@ Deno.test("public entry isolates Workspace authority and authenticated dashboard exact: true, }).click(); await page.getByRole("alert").waitFor(); - const rechecked = page.waitForResponse((response) => - new URL(response.url()).pathname === "/api/auth/whoami" - ); - await page.evaluate(() => - window.dispatchEvent(new Event("focus")) - ); - await rechecked; + await focusWithoutSessionCheck(page); await page.getByRole("button", { name: "Retry creation", exact: true, diff --git a/web/workspace/test/auth-browser/session-navigation.ts b/web/workspace/test/auth-browser/session-navigation.ts index dc6a125ed..3da93f637 100644 --- a/web/workspace/test/auth-browser/session-navigation.ts +++ b/web/workspace/test/auth-browser/session-navigation.ts @@ -230,7 +230,7 @@ export async function sessionNavigation( for (const active of [false, true]) { await step( `real HTTP Max-Age cookie disappears and ${ - active ? "active expiry" : "reload" + active ? "next page access after expiry" : "reload" } returns through login`, async ({ page, go }) => { const target = "/account?tab=keys#after-cookie-expiry"; @@ -247,11 +247,14 @@ export async function sessionNavigation( assert(cookie?.httpOnly); assert(cookie.expires > Date.now() / 1000); if (active) { - // whoamiのexpires_atを製品timerへ渡す。テスト側clockの前進やcookie削除はしない。 + // 明示的な操作で期限を取得する。期限の経過だけでは再確認しない。 const refreshed = page.waitForResponse((r) => new URL(r.url()).pathname === "/api/auth/whoami" ); - await focus(page); + await page.getByRole("button", { + name: "Refresh session", + exact: true, + }).click(); const wire = await (await refreshed).json(); assertEquals(wire.session.kind, "authenticated"); assertEquals( @@ -283,7 +286,17 @@ export async function sessionNavigation( c.name === "auth-fixture" ), ); - if (!active) await page.reload(); + if (active) { + await at(page, target); + assertEquals(await page.getByText("Loading session…").count(), 0); + await page.getByRole("link", { name: "Dashboard / Workspaces" }) + .click(); + await at(page, loginTarget("/dashboard")); + await login(page); + await at(page, "/dashboard"); + return; + } + await page.reload(); await at(page, loginTarget(target)); await page.reload(); await login(page); @@ -296,7 +309,7 @@ export async function sessionNavigation( ); } await step( - "concurrent API 401 and repeated session checks cannot overwrite the original expired return URL", + "concurrent API 401 leaves the page alone; the next page access detects expiry", async ({ page, go }) => { const target = "/w/private/tickets?view=thread#concurrent-expiry"; let release!: () => void; @@ -322,16 +335,15 @@ export async function sessionNavigation( await requests; await control(page, "expire"); release(); - // focus等の外部契機なしに、同時401がwhoami再照会へ接続すること。 - await at(page, loginTarget(target)); + // API failure stays on this page; only the next page access checks whoami. + await page.getByRole("alert").filter({ + hasText: /request failed with HTTP 401/, + }).waitFor(); assert(statuses.length >= 2); - for (let i = 0; i < 2; i++) { - const response = page.waitForResponse((r) => - new URL(r.url()).pathname === "/api/auth/whoami" - ); - await focus(page); - await (await response).finished(); - } + await at(page, target); + await focus(page); + assertEquals(await page.getByText("Loading session…").count(), 0); + await page.reload(); await at(page, loginTarget(target)); await page.unroute("**/api/w/private/**"); await login(page); @@ -375,7 +387,8 @@ export async function sessionNavigation( const observed = page.waitForRequest((r) => new URL(r.url()).pathname === "/api/auth/whoami" ); - await focus(page); + await page.getByRole("button", { name: "Refresh session", exact: true }) + .click(); await observed; await requested; // 他tabの本物の製品logoutでgeneration/storage eventも通す。 @@ -462,9 +475,9 @@ export async function sessionNavigation( assertEquals(new URL(page.url()).searchParams.has("returnTo"), false); }, ); - for (const event of ["pageshow", "visibilitychange"] as const) { + for (const event of ["focus", "pageshow", "visibilitychange"] as const) { await step( - `${event} rechecks active session and preserves expired return URL`, + `${event} leaves the page visible; reload detects expiry`, async ({ page, go }) => { const target = "/account?tab=keys#lifecycle"; await go(target, "account"); @@ -473,6 +486,10 @@ export async function sessionNavigation( exact: true, }).waitFor(); await control(page, "expire"); + let checks = 0; + page.on("request", (request) => { + if (new URL(request.url()).pathname === "/api/auth/whoami") checks++; + }); await page.evaluate((event) => { if (event === "visibilitychange") { if (document.hidden) { @@ -481,6 +498,15 @@ export async function sessionNavigation( document.dispatchEvent(new Event(event)); } else window.dispatchEvent(new Event(event)); }, event); + await page.evaluate(() => + new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())) + ) + ); + assertEquals(checks, 0); + await at(page, target); + assertEquals(await page.getByText("Loading session…").count(), 0); + await page.reload(); await at(page, loginTarget(target)); }, ); @@ -557,7 +583,7 @@ export async function sessionNavigation( } }, ); - for (const status of [403, 404]) { + for (const status of [401, 403, 404]) { await step( `post-login resource ${status} remains a resource failure, never a relogin loop`, async ({ page, go }) => {