diff --git a/apps/api/src/opendirector_api/localization.py b/apps/api/src/opendirector_api/localization.py index 0586213f..0b1965bd 100644 --- a/apps/api/src/opendirector_api/localization.py +++ b/apps/api/src/opendirector_api/localization.py @@ -63,6 +63,7 @@ "capability_unsupported": "This capability is not supported yet.", "operation_rejected": "The operation was rejected. Reload the item and try again.", "storage_busy": "Project storage is busy. Try again shortly.", + "project_open_elsewhere": "This project is open in another window. Close it there and try again.", "local_only": "Only this computer can access the project service.", "cross_origin": "File access from another site is not allowed.", "invalid_local_request": "This is not a valid local app request.", diff --git a/packages/core/src/opendirector/infrastructure/local_projects.py b/packages/core/src/opendirector/infrastructure/local_projects.py index bac7d0ca..2e515e9d 100644 --- a/packages/core/src/opendirector/infrastructure/local_projects.py +++ b/packages/core/src/opendirector/infrastructure/local_projects.py @@ -123,7 +123,7 @@ def _open(self, db, project: dict, session: str): identifier(session) project_id = project["id"] if db.execute("SELECT 1 FROM sessions WHERE project_id=? AND id<>? AND expires>?", (project_id, session, time.time())).fetchone(): - raise ProjectError("该项目正在其他窗口中打开,请先关闭后再试。", 409, code="storage_busy") + raise ProjectError("该项目正在其他窗口中打开,请先关闭后再试。", 409, code="project_open_elsewhere") lease = db.execute("SELECT path FROM sessions WHERE id=? AND project_id=? AND expires>?", (session, project_id, time.time())).fetchone() if lease and lease["path"] != project["path"]: raise ProjectError("项目目录已在应用外改名或移动,请复制未保存内容并刷新页面。", 409) diff --git a/scripts/run-browser-suite.mjs b/scripts/run-browser-suite.mjs index 70e56877..b64bb957 100644 --- a/scripts/run-browser-suite.mjs +++ b/scripts/run-browser-suite.mjs @@ -33,6 +33,7 @@ const workflows = [ name: "project list overflow", script: "tests/browser/project_list_overflow.mjs", }, + { name: "session lease", script: "tests/browser/session_lease.mjs" }, { name: "mp4 export", script: "tests/browser/mp4_export.mjs" }, { name: "editor save", script: "tests/browser/editor_save.mjs" }, { name: "canvas crop", script: "tests/browser/canvas_crop.mjs" }, diff --git a/src/backend/api.ts b/src/backend/api.ts index d83cf6f8..4d63e7dd 100644 --- a/src/backend/api.ts +++ b/src/backend/api.ts @@ -5,6 +5,75 @@ import { tNow } from "../i18n/messages.ts"; export const sessionId = crypto.randomUUID(); +const SESSION_KEY = "opendirector.session.v1"; +const lockName = (id: string) => `opendirector.session.${id}`; + +interface SessionLocks { + request(name: string, callback: () => Promise): Promise; + query(): Promise<{ held?: { name?: string }[] }>; +} + +/** Release this tab's previous page lease after a reload or crash. + * + * Each page load uses a new session. When the previous page could not send its + * release (crash, killed process, lost keepalive), its lease would otherwise + * block the reloaded tab as "another window" until it expires. The previous + * session is released only when no live page still holds its Web Lock, so a + * duplicated tab, which copies sessionStorage, never releases a live window. + * Without Web Locks nothing is released and the lease simply expires. */ +export async function releaseStaleSession( + previous: string | null, + current: string, + locks: SessionLocks | undefined, + release: (id: string) => Promise, + pause = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)), +) { + if (!locks) return false; + // Hold this page's lock for its lifetime; the browser drops it on unload. + void locks.request(lockName(current), () => new Promise(() => {})).catch(() => {}); + if (!previous || previous === current) return false; + for (let attempt = 0; attempt < 8; attempt++) { + // A reloading page may still be tearing down; look again briefly. + if (attempt) await pause(250); + const state = await locks.query().catch(() => null); + if (!state) return false; + if (!state.held?.some((lock) => lock.name === lockName(previous))) { + await release(previous).catch(() => {}); + return true; + } + } + return false; +} + +function tabStorage(): Storage | null { + try { + return globalThis.sessionStorage ?? null; + } catch { + return null; + } +} + +const previousSession = tabStorage()?.getItem(SESSION_KEY) ?? null; +/** This page replaced an earlier page of the same tab (reload or reconnect). */ +export const reloadedTab = previousSession !== null; +try { + tabStorage()?.setItem(SESSION_KEY, sessionId); +} catch { + // Private modes may refuse storage; the lease then just expires. +} + +/** Requests wait until a stale lease from this tab's previous page is gone. */ +const sessionReady = releaseStaleSession( + previousSession, + sessionId, + (globalThis.navigator as { locks?: SessionLocks } | undefined)?.locks, + (id) => + fetch(`/api/creation/sessions/${id}`, { + method: "DELETE", + headers: { "X-OpenDirector-Request": "1" }, + }), +).catch(() => false); + export class ApiError extends Error { status: number | null; code: string; @@ -41,6 +110,7 @@ export async function request( path: string, options: RequestInit = {}, ): Promise { + await sessionReady; let response: Response; try { response = await fetch(`/api${path}`, { diff --git a/src/backend/creationClient.ts b/src/backend/creationClient.ts index 6bfa53af..232bd37e 100644 --- a/src/backend/creationClient.ts +++ b/src/backend/creationClient.ts @@ -16,6 +16,7 @@ import { ApiError, headers, json, + reloadedTab, request, responseError, storageRequest, @@ -160,13 +161,16 @@ export class CreationClient { this.install(canvas ? canvas.snapshot : (opened as ProjectSnapshot)); break; } catch (error) { - // The old page's keepalive release can arrive just after a reload's - // first open. Genuine concurrent windows retain their lease protection. + // A storage lock timeout is retried. After a reload, the old page's + // keepalive release can also arrive just after this first open, so a + // reloaded tab briefly waits for its own lease too. A fresh window + // refused by another live window (`project_open_elsewhere`) fails at once. if ( attempt >= 7 || !(error instanceof ApiError) || error.status !== 409 || - error.code !== "storage_busy" + (error.code !== "storage_busy" && + !(reloadedTab && error.code === "project_open_elsewhere")) ) throw error; await new Promise((resolve) => diff --git a/tests/backendWorkspace.test.ts b/tests/backendWorkspace.test.ts index 819dfc5c..b9b729b5 100644 --- a/tests/backendWorkspace.test.ts +++ b/tests/backendWorkspace.test.ts @@ -660,7 +660,7 @@ test("a long read that lost its lease fails instead of adopting it", async (t) = (options.method ?? "GET") === "PUT" ) return Response.json( - { detail: "工程正在其他窗口中打开", code: "storage_busy" }, + { detail: "工程正在其他窗口中打开", code: "project_open_elsewhere" }, { status: 409 }, ); return plain(url, options); diff --git a/tests/browser/manifest.json b/tests/browser/manifest.json index a6e8c1ab..343f7569 100644 --- a/tests/browser/manifest.json +++ b/tests/browser/manifest.json @@ -340,6 +340,13 @@ "production" ] }, + { + "file": "session_lease.mjs", + "lanes": [ + "gate" + ], + "note": "Reload after a lost lease release reopens at once; a second window is refused with project_open_elsewhere (#277)." + }, { "file": "project_title_enter.mjs", "previewSafe": true, diff --git a/tests/browser/session_lease.mjs b/tests/browser/session_lease.mjs new file mode 100644 index 00000000..f1777268 --- /dev/null +++ b/tests/browser/session_lease.mjs @@ -0,0 +1,77 @@ +/** A tab reload after a lost lease release reopens its own project at once, + * while a second live window is still refused with an explicit message. */ +import assert from "node:assert/strict"; +import { launchBrowser } from "./engine.mjs"; + +const base = process.env.CREATION_WEB_URL ?? "http://127.0.0.1:5179"; +const api = process.env.CREATION_API_URL ?? "http://127.0.0.1:8019"; +const headers = { "X-OpenDirector-Request": "1", "Content-Type": "application/json" }; +async function call(path, body) { + const response = await fetch(api + path, { headers, ...(body ? { method: "POST", body: JSON.stringify(body) } : {}) }); + assert.ok(response.ok, await response.clone().text()); + return response.json(); +} +assert.equal((await call("/api/creation-test-health")).isolated, true); +const saved = { "zh-CN": "已保存到本机", "en-US": "Saved on this device" }; + +const browser = await launchBrowser(); +try { + for (const locale of ["en-US", "zh-CN"]) { + const id = crypto.randomUUID(); + await call(`/api/creation/projects/${id}/commands`, { projectId: id, commandId: crypto.randomUUID(), + type: "project.create", payload: { name: `Session lease ${locale} ${id.slice(0, 8)}` } }); + const url = `${base}/ai-tool/ai-canvas/${id}`; + const context = await browser.newContext({ locale, viewport: { width: 1280, height: 800 } }); + await context.addInitScript((value) => { + localStorage.setItem("opendirector.modelSetup.v1", "dismissed"); + localStorage.setItem("opendirector.locale.v1", value); + }, locale); + const page = await context.newPage(); + page.setDefaultTimeout(20000); + const openStatuses = []; + page.on("console", (m) => { if (process.env.DEBUG && m.type() === "error") console.log("console:", m.text()); }); + page.on("response", (response) => { + if (process.env.DEBUG && response.url().includes("/api/creation/") && !response.url().includes("/assets/")) console.log(response.request().method(), response.status(), response.url().slice(-60)); + if (response.url().includes("/open-canvas")) openStatuses.push(response.status()); + }); + await page.goto(url); + await page.getByText(saved[locale], { exact: true }).waitFor({ state: "attached" }); + + // A lost pagehide release (crash, killed process): the old page's DELETE + // never leaves it. Only this page's fetch is replaced; the reload is fresh. + const stale = await page.evaluate(() => { + const original = window.fetch; + window.fetch = (input, init) => + String(input).includes("/creation/sessions/") && init?.method === "DELETE" + ? Promise.reject(new TypeError("dropped")) + : original(input, init); + return sessionStorage.getItem("opendirector.session.v1"); + }); + assert.ok(stale, "the tab records its session"); + if (process.env.DEBUG) console.log("--- reload"); + const started = Date.now(); + await page.reload(); + // The old page's lease was still live, so the reload itself released it. + assert.notEqual(await page.evaluate(() => sessionStorage.getItem("opendirector.session.v1")), stale); + await page.getByText(saved[locale], { exact: true }).waitFor({ state: "attached" }); + const elapsed = Date.now() - started; + assert.ok(elapsed < 15000, `reload reopened after ${elapsed} ms`); + assert.deepEqual(openStatuses.filter((status) => status === 409), [], `${locale}: reload must not be refused`); + + // A genuinely separate live window still cannot open the same project. + const other = await context.newPage(); + await other.addInitScript(() => sessionStorage.clear()); + const refused = other.waitForResponse((response) => response.url().includes("/open-canvas")); + await other.goto(url); + const response = await refused; + assert.equal(response.status(), 409); + const body = await response.json(); + assert.equal(body.code, "project_open_elsewhere"); + if (locale === "en-US") assert.match(body.detail, /another window/); + else assert.match(body.detail, /其他窗口/); + console.log(`PASS ${locale}: reload reopened in ${elapsed} ms; second window refused: ${body.detail}`); + await context.close(); + } +} finally { + await browser.close(); +} diff --git a/tests/sessionLease.test.ts b/tests/sessionLease.test.ts new file mode 100644 index 00000000..581b842f --- /dev/null +++ b/tests/sessionLease.test.ts @@ -0,0 +1,67 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { releaseStaleSession } from "../src/backend/api.ts"; + +function locks(held: string[]) { + const requested: string[] = []; + return { + requested, + request: async (name: string) => { + requested.push(name); + }, + query: async () => ({ held: held.map((name) => ({ name })) }), + }; +} +const noPause = async () => {}; + +test("a reloaded tab releases its previous page's unreleased lease", async () => { + const released: string[] = []; + const browser = locks([]); + assert.equal( + await releaseStaleSession("old", "new", browser, async (id) => released.push(id), noPause), + true, + ); + assert.deepEqual(released, ["old"]); + assert.deepEqual(browser.requested, ["opendirector.session.new"]); +}); + +test("a duplicated tab never releases the lease of a live window", async () => { + const released: string[] = []; + const browser = locks(["opendirector.session.old"]); + assert.equal( + await releaseStaleSession("old", "new", browser, async (id) => released.push(id), noPause), + false, + ); + assert.deepEqual(released, []); +}); + +test("a first load or a browser without Web Locks releases nothing", async () => { + const released: string[] = []; + const release = async (id: string) => released.push(id); + assert.equal(await releaseStaleSession(null, "new", locks([]), release, noPause), false); + assert.equal(await releaseStaleSession("old", "new", undefined, release, noPause), false); + assert.deepEqual(released, []); +}); + +test("a failed release does not block the page", async () => { + assert.equal( + await releaseStaleSession("old", "new", locks([]), async () => { + throw new Error("offline"); + }, noPause), + true, + ); +}); + +test("a previous page that is still unloading is released once its lock is gone", async () => { + const released: string[] = []; + let checks = 0; + const browser = { + request: async () => {}, + query: async () => ({ held: ++checks < 3 ? [{ name: "opendirector.session.old" }] : [] }), + }; + assert.equal( + await releaseStaleSession("old", "new", browser, async (id) => released.push(id), noPause), + true, + ); + assert.deepEqual(released, ["old"]); +}); diff --git a/tests/test_creation_storage.py b/tests/test_creation_storage.py index 7dab2b13..f54fccae 100644 --- a/tests/test_creation_storage.py +++ b/tests/test_creation_storage.py @@ -193,8 +193,10 @@ def test_leases_expiry_external_moves_and_closed_rename_guards(projects): created = create(projects) session, other = str(uuid4()), str(uuid4()) projects.open(created.projectId, session) - with pytest.raises(ProjectError, match='其他窗口'): + with pytest.raises(ProjectError, match='其他窗口') as busy: projects.open(created.projectId, other) + # Another window is not a lock timeout: it must not share the retryable code. + assert busy.value.code == 'project_open_elsewhere' with pytest.raises(ProjectError, match='关闭'): projects.execute(rename(created)) (projects.root / 'Example').rename(projects.root / 'Moved outside app') diff --git a/tests/test_localization.py b/tests/test_localization.py index 121f8c53..deafa035 100644 --- a/tests/test_localization.py +++ b/tests/test_localization.py @@ -94,3 +94,10 @@ def test_streaming_and_background_jobs_use_the_same_language_rule() -> None: ) == "The item changed or is busy. Reload it and try again." ) + + +def test_another_window_is_not_reported_as_busy_storage() -> None: + chinese = "该项目正在其他窗口中打开,请先关闭后再试。" + assert localized_error(request("zh-CN"), chinese=chinese, status=409, code="project_open_elsewhere") == chinese + english = localized_error(request("en-US"), chinese=chinese, status=409, code="project_open_elsewhere") + assert "another window" in english and "busy" not in english