From 94df80bbd67a51e938b0e88abd9e1f83e9f50976 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 23:16:37 -0700 Subject: [PATCH] test(dashboard): restore the Planning project-switch invariant at its new home (last lane red) (#2982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clears the **last red test in the dashboard lane** (measured on `main` at `41af5e5dbd`: `1 failed | 11180 passed`) and restores a leak-class invariant that has had no assertion since FN-8619. ## What was wrong FN-8619 moved Planning out of `MainContent` (its branch returns `null`) into `PlanningKeepAlive`, mounted by `App`. `MainContent.planning-project-remount.test.tsx` kept asserting there, so it could only fail — and nothing asserted the invariant at the new location. **Deleting the project id from `App.tsx:1956`'s key, or loosening the gate at `:1954`, failed no test.** That invariant is not cosmetic. Before the project-keyed host, Planning kept a running plan's stream, selected session and sidebar list from the **previous** project, and persisted its session under the **new** project's storage key. ## I withdrew this test once, and that was my mistake I wrote it earlier, mutated each guard, saw it stay green both times, concluded it was vacuous, and handed the problem back. **That reasoning was wrong.** App defends this invariant **twice, independently**: 1. the `planningEverOpenedProjectId === currentProject.id` gate, which unmounts the host for a project that never opened Planning; and 2. the project id inside the host's `key`, which forces a remount instead of reconciling A's live instance under B. Either alone upholds the contract. So a single-guard mutation *should* leave a correct test green — that is defence in depth working, not a hole. The probe that settles it is breaking **both**: | mutation | result | |---|---| | both guards intact | **pass** | | key only (drop project id) | pass | | gate only (`!== null`) | pass | | **both** | **fail** | I had mutated guards, not the invariant, and mistook redundancy for vacuity. Worth recording because it is the opposite error from the one I have been making all day: I have caught four genuinely vacuous guards by mutation, and that success made "survived a mutation" read as "proves nothing" when the honest reading was "the system has a second defence." ## The assertion, and why it is `gone OR different node` An earlier draft asserted the subtree must be **absent** after the switch. Wrong: `planningViewActive` stays true, so the latch re-arms for the new project and a fresh host is expected. Both outcomes satisfy the real contract — *project A's instance is not reused* — which is what `subtreeForA.isConnected === false` pins. ## Scope - Planning case moves to `App.test.tsx`, which owns both halves of the guarantee. - `MainContent.planning-project-remount.test.tsx` keeps its **Chat** and **Missions** cases — those still render from MainContent — and gains a note saying where Planning went, so it is not re-added there. **Verified:** `App.test.tsx` 143/143, `MainContent.planning-project-remount` 2/2, `tsc -p tsconfig.app.json` 0 errors, lint clean, FNXC gate exit 0. Test-only; no product code touched, no changeset. Co-authored-by: Claude Opus 5 (1M context) --- .../app/components/__tests__/App.test.tsx | 58 +++++++++++++++++++ ...nContent.planning-project-remount.test.tsx | 28 ++++----- 2 files changed, 69 insertions(+), 17 deletions(-) diff --git a/packages/dashboard/app/components/__tests__/App.test.tsx b/packages/dashboard/app/components/__tests__/App.test.tsx index 1cf22cab30..9d6da80474 100644 --- a/packages/dashboard/app/components/__tests__/App.test.tsx +++ b/packages/dashboard/app/components/__tests__/App.test.tsx @@ -3235,6 +3235,64 @@ describe("App Planning Mode", () => { expect(screen.getByTestId("planning-view")).toBeTruthy(); }); + /* + FNXC:ProjectSwitchModalReset 2026-07-30-23:45: + A PROJECT SWITCH MUST NOT LEAVE THE PREVIOUS PROJECT'S PLANNING SUBTREE MOUNTED. + + Relocated from `MainContent.planning-project-remount.test.tsx`. FN-8619 moved Planning out of + MainContent into `PlanningKeepAlive`, mounted by App — so the old file could only fail, and the + invariant it guarded had no assertion anywhere. The product was already correct; only the coverage + was left behind. + + What is at stake is a cross-project leak, not layout: before the project-keyed host, Planning kept a + running plan's stream, selected session and sidebar list from the PREVIOUS project, and persisted + its session under the NEW project's storage key. + + WHY THE ASSERTION IS "gone OR a different node", and why single-guard mutations do NOT break it. + App defends this twice, independently: + + 1. the `planningEverOpenedProjectId === currentProject.id` gate (App.tsx), which unmounts the + host for a project that never opened Planning; and + 2. the project id inside the host's `key`, which forces a remount rather than reconciling the + live instance under the new project. + + Either alone upholds the invariant, so breaking one leaves this green — correctly. MEASURED: + breaking BOTH fails it. An earlier draft of mine asserted the subtree must be ABSENT, which is + wrong: `planningViewActive` stays true across the switch, so the latch re-arms and a fresh host is + expected. Both outcomes satisfy "project A's instance is not reused", which is the actual contract. + */ + it("never leaves the previous project's Planning subtree mounted after a switch", async () => { + localStorage.setItem("kb-dashboard-view-mode", "project"); + const projectA = { ...DEFAULT_PROJECT, id: "proj_switch_a", name: "Project A" }; + const projectB = { ...DEFAULT_PROJECT, id: "proj_switch_b", name: "Project B" }; + mockCurrentProjectState.currentProject = projectA; + vi.mocked(fetchSettings).mockResolvedValueOnce({ + ...defaultSettings, + experimentalFeatures: { ...defaultSettings.experimentalFeatures, leftSidebarNav: true }, + }); + + const { rerender } = render(); + + await screen.findByTestId("sidebar-nav-planning"); + fireEvent.click(screen.getByTestId("sidebar-nav-planning")); + await waitFor(() => { + expect(screen.getByTestId("planning-keep-alive")).toBeTruthy(); + }); + + /* Control: capture A's live subtree, so "not this node" below is a real statement. */ + const subtreeForA = screen.getByTestId("planning-keep-alive"); + + mockCurrentProjectState.currentProject = projectB; + rerender(); + + await waitFor(() => { + const current = screen.queryByTestId("planning-keep-alive"); + expect(current === null || current !== subtreeForA).toBe(true); + }); + /* The load-bearing half: A's instance is off the page either way. */ + expect(subtreeForA.isConnected).toBe(false); + }); + it("renders planning embedded view with correct initial state", async () => { localStorage.setItem(taskViewStorageKey(), "planning"); diff --git a/packages/dashboard/app/components/dashboard/__tests__/MainContent.planning-project-remount.test.tsx b/packages/dashboard/app/components/dashboard/__tests__/MainContent.planning-project-remount.test.tsx index add8e1d5dc..fb45cc6443 100644 --- a/packages/dashboard/app/components/dashboard/__tests__/MainContent.planning-project-remount.test.tsx +++ b/packages/dashboard/app/components/dashboard/__tests__/MainContent.planning-project-remount.test.tsx @@ -98,25 +98,19 @@ function mainContentProps(overrides: Partial = {}): MainConten } describe("MainContent planning project remount", () => { - it("remounts embedded Planning when the active project changes", () => { - const { rerender } = render(); + /* + FNXC:ProjectSwitchModalReset 2026-07-30-23:40: + THE PLANNING CASE MOVED TO App.test.tsx — it is not gone, and must not be re-added here. - expect(screen.getByLabelText("Planning project")).toHaveTextContent("project-1"); - expect(planningMounts).toEqual(["project-1"]); - - rerender( - , - ); - - // A fresh mount for the new project — not a prop update on the old instance. - expect(screen.getByLabelText("Planning project")).toHaveTextContent("project-2"); - expect(planningMounts).toEqual(["project-1", "project-2"]); - }); + It rendered MainContent and counted Planning mounts per project. FN-8619 moved Planning out of + MainContent entirely (its branch returns `null`; see MainContent.tsx) into `PlanningKeepAlive`, + which App mounts as a sibling. Asserting it here could only fail, and it did — this was the last + red test in the dashboard lane. + App owns BOTH halves of the guarantee, which is why the replacement lives there: the + `planningEverOpenedProjectId === currentProject.id` gate and the project id inside the host's + `key`. Chat and Missions still render from MainContent, so their cases stay. + */ it("remounts embedded Chat when the active project changes", () => { const { rerender } = render(); expect(chatMounts).toEqual(["project-1"]);