From 06480b9c7cc7c1c6e248a9daf676679192b2d784 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 5 Oct 2026 16:59:13 +0900 Subject: [PATCH] Fix Subject behavior editor isolation across reused routes Reset editor state for each loader identity and bind saves to their captured Subject and Workspace. Fence late success, error, and finally updates using the loader generation, including navigation back to the same Subject. Validation: 14 Memory component tests (5 new route/workspace/in-flight isolation cases); frontend check with zero diagnostics; production Memory browser shell test; cargo check; cargo fmt --all -- --check; git diff --check HEAD. Restored implementation full Rust and frontend suites were independently rerun before this Web-only fix. --- .../memory/[subjectId]/+page.svelte | 44 +++- .../memory/memory-page.browser.spec.ts | 203 ++++++++++++++++++ 2 files changed, 240 insertions(+), 7 deletions(-) diff --git a/web/workspace/src/routes/w/[workspaceId]/memory/[subjectId]/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/memory/[subjectId]/+page.svelte index 29be97da4..39b323d78 100644 --- a/web/workspace/src/routes/w/[workspaceId]/memory/[subjectId]/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/memory/[subjectId]/+page.svelte @@ -18,27 +18,57 @@ let behaviorError = $state(null); let behaviorSaveConfirmed = $state(false); + let routeGeneration = 0; + let subjectWorkspaceId = untrack(() => data.workspaceId); + + // SvelteKit reuses this page across Subject and Workspace routes. A new + // loader snapshot owns a new editor and fences all previous mutations. + $effect(() => { + const workspaceId = data.workspaceId; + const subjectId = data.subjectId; + const loadedSubject = data.subject.data; + untrack(() => { + routeGeneration += 1; + subjectWorkspaceId = workspaceId; + subject = loadedSubject?.id === subjectId ? loadedSubject : null; + behaviorDraft = subject?.behavior_md ?? ''; + editingBehavior = false; + behaviorSaving = false; + behaviorError = null; + behaviorSaveConfirmed = false; + }); + }); + async function saveBehavior(): Promise { - if (!subject || behaviorSaving) return; + if (!subject || behaviorSaving || subject.id !== data.subjectId || subjectWorkspaceId !== data.workspaceId) return; + const generation = routeGeneration; + const workspaceId = subjectWorkspaceId; + const subjectId = subject.id; + const request = { + expected_behavior_revision: subject.behavior_revision, + behavior_md: behaviorDraft, + }; + const isCurrent = () => generation === routeGeneration && + data.workspaceId === workspaceId && data.subjectId === subjectId; behaviorSaving = true; behaviorError = null; behaviorSaveConfirmed = false; try { - subject = await updateSubjektivSubjectBehavior(fetch, data.workspaceId, data.subjectId, { - expected_behavior_revision: subject.behavior_revision, - behavior_md: behaviorDraft, - }); - behaviorDraft = subject.behavior_md; + const updated = await updateSubjektivSubjectBehavior(fetch, workspaceId, subjectId, request); + if (!isCurrent()) return; + subject = updated; + behaviorDraft = updated.behavior_md; editingBehavior = false; behaviorSaveConfirmed = true; } catch (error) { + if (!isCurrent()) return; behaviorError = error instanceof SubjektivSubjectCreateError ? (error.status === 401 || error.status === 403 ? 'You do not have permission to edit this Subject.' : error.message) : 'The save outcome is unknown. Reload the Subject before retrying.'; } finally { - behaviorSaving = false; + if (isCurrent()) behaviorSaving = false; } } diff --git a/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts b/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts index 0aafd1da3..ccd3f9c24 100644 --- a/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts +++ b/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts @@ -374,6 +374,209 @@ test("edits and clears user-managed behavior with CAS while distinguishing save ); }); +function subjectPageData(record = subject(), workspaceId = "workspace-1") { + return { + workspaceId, + subjectId: record.id, + subject: result(record), + surface: result({ + subject_id: record.id, + availability: "ungenerated" as const, + }), + memories: result({ items: [], has_more: false }), + }; +} + +test.each([ + ["subject-2", "workspace-1"], + ["subject-1", "workspace-2"], +])( + "resets reused Subject editor for %s in %s and saves only its own behavior", + async (id, workspaceId) => { + const nextSubject = { ...subject(), id, behavior_md: "Only B behavior." }; + const fetchMock = vi.fn(async (_path: string, init?: RequestInit) => { + const request = JSON.parse(String(init?.body)); + return Response.json({ + ...nextSubject, + behavior_md: request.behavior_md, + behavior_revision: request.expected_behavior_revision + 1, + }); + }); + vi.stubGlobal("fetch", fetchMock); + const view = render(SubjectPage, { data: subjectPageData() } as never); + await fireEvent.click(screen.getByRole("button", { name: "Edit" })); + await fireEvent.input( + screen.getByRole("textbox", { name: "Subject behavior" }), + { + target: { value: "Unsaved A draft" }, + }, + ); + + await view.rerender( + { data: subjectPageData(nextSubject, workspaceId) } as never, + ); + expect(screen.queryByRole("textbox", { name: "Subject behavior" })) + .toBeNull(); + expect( + screen.getByRole("article", { name: "User-managed Subject behavior" }) + .textContent, + ) + .toContain("Only B behavior."); + await fireEvent.click(screen.getByRole("button", { name: "Edit" })); + expect( + (screen.getByRole("textbox", { + name: "Subject behavior", + }) as HTMLTextAreaElement).value, + ) + .toBe("Only B behavior."); + await fireEvent.click( + screen.getByRole("button", { name: "Save behavior" }), + ); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + const [path, init] = fetchMock.mock.calls[0] as unknown as [ + string, + RequestInit, + ]; + expect(path).toBe( + `/api/w/${workspaceId}/subjektiv/subjects/${id}/behavior`, + ); + expect(JSON.parse(String(init.body))).toEqual({ + expected_behavior_revision: 2, + behavior_md: "Only B behavior.", + }); + await waitFor(() => + expect(screen.getByText(/storage confirmed at revision 3/)).not.toBeNull() + ); + }, +); + +test.each(["success", "failure"])( + "fences late %s from a previous Subject without clearing the new save state", + async (outcome) => { + const pending: Array<(response: Response) => void> = []; + const fetchMock = vi.fn(() => + new Promise((resolve) => pending.push(resolve)) + ); + vi.stubGlobal("fetch", fetchMock); + const view = render(SubjectPage, { data: subjectPageData() } as never); + await fireEvent.click(screen.getByRole("button", { name: "Edit" })); + await fireEvent.click( + screen.getByRole("button", { name: "Save behavior" }), + ); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + + const nextSubject = { + ...subject(), + id: "subject-2", + behavior_md: "Only B behavior.", + }; + await view.rerender({ data: subjectPageData(nextSubject) } as never); + await fireEvent.click(screen.getByRole("button", { name: "Edit" })); + await fireEvent.input( + screen.getByRole("textbox", { name: "Subject behavior" }), + { + target: { value: "Updated B behavior." }, + }, + ); + await fireEvent.click( + screen.getByRole("button", { name: "Save behavior" }), + ); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + pending[0]( + outcome === "success" + ? Response.json({ + ...subject(), + behavior_md: "Late A response", + behavior_revision: 3, + }) + : Response.json({ + error: "Forbidden", + message: "A denied", + diagnostics: [], + }, { status: 403 }), + ); + // Drain the previous response through its success/catch/finally branches. + await new Promise((resolve) => setTimeout(resolve, 0)); + expect( + screen.getByRole("button", { name: "Saving…" }).hasAttribute("disabled"), + ).toBe(true); + expect( + (screen.getByRole("textbox", { + name: "Subject behavior", + }) as HTMLTextAreaElement).value, + ) + .toBe("Updated B behavior."); + expect(screen.queryByRole("alert")).toBeNull(); + expect(screen.queryByText(/storage confirmed/)).toBeNull(); + pending[1]( + Response.json({ + ...nextSubject, + behavior_md: "Updated B behavior.", + behavior_revision: 3, + }), + ); + await waitFor(() => + expect(screen.getByText(/storage confirmed at revision 3/)).not.toBeNull() + ); + expect( + screen.getByRole("article", { name: "User-managed Subject behavior" }) + .textContent, + ) + .toContain("Updated B behavior."); + }, +); + +test("a refreshed loader snapshot fences old saves even after returning to the same Subject", async () => { + let resolveRequest!: (response: Response) => void; + vi.stubGlobal( + "fetch", + vi.fn(() => + new Promise((resolve) => { + resolveRequest = resolve; + }) + ), + ); + const view = render(SubjectPage, { data: subjectPageData() } as never); + await fireEvent.click(screen.getByRole("button", { name: "Edit" })); + await fireEvent.click(screen.getByRole("button", { name: "Save behavior" })); + await view.rerender( + { data: subjectPageData({ ...subject(), id: "subject-2" }) } as never, + ); + const fresh = { + ...subject(), + behavior_md: "Fresh A loader", + behavior_revision: 4, + }; + await view.rerender({ data: subjectPageData(fresh) } as never); + resolveRequest( + Response.json({ + ...subject(), + behavior_md: "Stale A response", + behavior_revision: 3, + }), + ); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect( + screen.getByRole("article", { name: "User-managed Subject behavior" }) + .textContent, + ) + .toContain("Fresh A loader"); + expect(screen.queryByText(/storage confirmed/)).toBeNull(); + + await view.rerender( + { + data: { + ...subjectPageData(), + subject: result(null, "Subject unavailable"), + }, + } as never, + ); + expect(screen.queryByRole("button", { name: "Edit" })).toBeNull(); + expect( + screen.queryByRole("article", { name: "User-managed Subject behavior" }), + ).toBeNull(); +}); + test("distinguishes ready-empty, stale, failed, unavailable, and request error surfaces", async () => { const base = { workspaceId: "workspace-1",