From e916ecd95ea6c1d500bc2a4a901d7320580b1e92 Mon Sep 17 00:00:00 2001 From: Blomios Date: Thu, 25 Jun 2026 16:23:40 +0200 Subject: [PATCH] =?UTF-8?q?fix(layout):=20auto-r=C3=A9parer=20l'onglet=20a?= =?UTF-8?q?ctif=20=20=20stale?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le backend renvoie l'id actif autoritaire et le frontend l'adopte pour éviter de rejouer un layout disparu après overwrite externe. Co-Authored-By: Claude Opus 4.8 --- crates/app-tauri/src/commands.rs | 34 +++---- crates/app-tauri/src/dto.rs | 19 +++- crates/application/src/layout/management.rs | 33 +++++-- crates/application/src/layout/mod.rs | 2 +- crates/application/src/layout/store.rs | 68 ++++++++++++++ crates/application/src/layout/usecases.rs | 10 ++- crates/application/src/lib.rs | 7 +- crates/application/tests/layout_usecases.rs | 43 +++++++-- frontend/src/adapters/layout.ts | 4 +- frontend/src/adapters/mock/index.ts | 8 +- frontend/src/adapters/mock/layout.test.ts | 15 +++- .../src/features/layout/LayoutTabs.test.tsx | 88 ++++++++++++++++++- frontend/src/features/layout/useLayouts.ts | 82 +++++++++++++++-- frontend/src/ports/index.ts | 10 ++- 14 files changed, 372 insertions(+), 51 deletions(-) diff --git a/crates/app-tauri/src/commands.rs b/crates/app-tauri/src/commands.rs index 61d2aac..4f84154 100644 --- a/crates/app-tauri/src/commands.rs +++ b/crates/app-tauri/src/commands.rs @@ -19,12 +19,11 @@ use application::{ ListResumableAgentsInput, ListSkillsInput, LiveSessions, LoadLayoutInput, McpRuntime, MutateLayoutInput, OpenProjectInput, ReadAgentContextInput, ReadConversationPageInput, ReadMemoryIndexInput, ReadProjectContextInput, RecallMemoryInput, ReconcileLayoutsInput, - ReconcileLiveStateInput, - RenameLayoutInput, ResolveAgentPermissionsInput, ResolveMemoryLinksInput, - RotateConversationLogInput, SetActiveLayoutInput, SnapshotRunningAgentsInput, - StopLiveAgentInput, SyncAgentWithTemplateInput, UnassignSkillFromAgentInput, - UpdateAgentContextInput, UpdateAgentPermissionsInput, UpdateMemoryInput, - UpdateProjectContextInput, UpdateProjectPermissionsInput, UpdateSkillInput, + ReconcileLiveStateInput, RenameLayoutInput, ResolveAgentPermissionsInput, + ResolveMemoryLinksInput, RotateConversationLogInput, SetActiveLayoutInput, + SnapshotRunningAgentsInput, StopLiveAgentInput, SyncAgentWithTemplateInput, + UnassignSkillFromAgentInput, UpdateAgentContextInput, UpdateAgentPermissionsInput, + UpdateMemoryInput, UpdateProjectContextInput, UpdateProjectPermissionsInput, UpdateSkillInput, }; use domain::ports::PtyHandle; @@ -49,13 +48,13 @@ use crate::dto::{ ReadConversationPageRequestDto, ReattachChatDto, ReattachResultDto, RecallMemoryRequestDto, RenameLayoutRequestDto, ReplyChunk, ResizeTerminalRequestDto, ResolveAgentPermissionsRequestDto, ResumableAgentListDto, SaveEmbedderProfileRequestDto, - SaveProfileRequestDto, SetActiveLayoutRequestDto, SkillDto, SkillListDto, - StopLiveAgentRequestDto, StopLiveAgentResponseDto, SyncAgentWithTemplateRequestDto, - SyncResultDto, TemplateDto, TemplateListDto, TerminalClosedDto, TerminalSessionDto, - TurnPageDto, UnassignSkillRequestDto, UpdateAgentContextRequestDto, - UpdateAgentPermissionsRequestDto, UpdateMemoryRequestDto, UpdateProjectContextRequestDto, - UpdateProjectPermissionsRequestDto, UpdateSkillRequestDto, UpdateTemplateRequestDto, - WriteTerminalRequestDto, + SaveProfileRequestDto, SetActiveLayoutRequestDto, SetActiveLayoutResultDto, SkillDto, + SkillListDto, StopLiveAgentRequestDto, StopLiveAgentResponseDto, + SyncAgentWithTemplateRequestDto, SyncResultDto, TemplateDto, TemplateListDto, + TerminalClosedDto, TerminalSessionDto, TurnPageDto, UnassignSkillRequestDto, + UpdateAgentContextRequestDto, UpdateAgentPermissionsRequestDto, UpdateMemoryRequestDto, + UpdateProjectContextRequestDto, UpdateProjectPermissionsRequestDto, UpdateSkillRequestDto, + UpdateTemplateRequestDto, WriteTerminalRequestDto, }; use crate::pty::{PtyBridge, PtyChunk}; use crate::state::AppState; @@ -678,14 +677,16 @@ pub async fn delete_layout( /// `set_active_layout` — switch the active named layout of a project. /// +/// Self-healing: a stale `layout_id` does not error; the active layout is left +/// unchanged and the actually-active id is returned (authoritative). +/// /// # Errors -/// Returns an [`ErrorDto`] (`INVALID` for malformed id, `NOT_FOUND` if the -/// project or layout is unknown). +/// Returns an [`ErrorDto`] (`INVALID` for malformed id) or a persistence error. #[tauri::command] pub async fn set_active_layout( request: SetActiveLayoutRequestDto, state: State<'_, AppState>, -) -> Result<(), ErrorDto> { +) -> Result { let project_id = parse_project_id(&request.project_id)?; let layout_id = parse_layout_id(&request.layout_id)?; state @@ -695,6 +696,7 @@ pub async fn set_active_layout( layout_id, }) .await + .map(SetActiveLayoutResultDto::from) .map_err(ErrorDto::from) } diff --git a/crates/app-tauri/src/dto.rs b/crates/app-tauri/src/dto.rs index 27adf9c..9dc9693 100644 --- a/crates/app-tauri/src/dto.rs +++ b/crates/app-tauri/src/dto.rs @@ -390,7 +390,7 @@ pub fn parse_session_id(raw: &str) -> Result { use application::{ CreateLayoutOutput, DeleteLayoutOutput, LayoutInfo, LayoutOperation, ListLayoutsOutput, - LoadLayoutOutput, MutateLayoutOutput, + LoadLayoutOutput, MutateLayoutOutput, SetActiveLayoutOutput, }; use domain::{AgentId, Direction, LayoutId, LayoutTree, NodeId}; @@ -700,6 +700,23 @@ pub struct DeleteLayoutRequestDto { pub layout_id: String, } +/// Response DTO for `set_active_layout`. +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct SetActiveLayoutResultDto { + /// The layout actually activated — equals the requested id when valid, else + /// the unchanged current active id (self-healing fallback). Authoritative. + pub active_id: String, +} + +impl From for SetActiveLayoutResultDto { + fn from(out: SetActiveLayoutOutput) -> Self { + Self { + active_id: out.active_id.to_string(), + } + } +} + /// Request DTO for `set_active_layout`. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase")] diff --git a/crates/application/src/layout/management.rs b/crates/application/src/layout/management.rs index dd8d7ea..d5bc67c 100644 --- a/crates/application/src/layout/management.rs +++ b/crates/application/src/layout/management.rs @@ -297,6 +297,15 @@ pub struct SetActiveLayoutInput { pub layout_id: LayoutId, } +/// Output of [`SetActiveLayout::execute`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SetActiveLayoutOutput { + /// The id of the layout that was **actually** made active. Equals the + /// requested id when it exists; otherwise the unchanged current active id + /// (self-healing fallback). Authoritative for the frontend (invariant I4). + pub active_id: LayoutId, +} + /// Switches the active layout of a project. pub struct SetActiveLayout { store: Arc, @@ -317,20 +326,30 @@ impl SetActiveLayout { /// Sets the active layout. /// + /// A stale requested id (e.g. an `activeId` left over after git overwrote + /// `layouts.json`) must **not** freeze the workspace: instead of hard- + /// erroring, it degrades silently to the current active layout (invariant + /// I3). The returned [`SetActiveLayoutOutput::active_id`] is the id that was + /// actually activated and is authoritative for the frontend (invariant I4). + /// /// # Errors - /// [`AppError::NotFound`] if the project or layout is unknown. - pub async fn execute(&self, input: SetActiveLayoutInput) -> Result<(), AppError> { + /// - [`AppError::FileSystem`] on persistence failure, + /// - [`AppError::Store`] on registry I/O failure. + pub async fn execute( + &self, + input: SetActiveLayoutInput, + ) -> Result { let project = self.store.load_project(input.project_id).await?; let mut doc = resolve_doc(self.fs.as_ref(), &project).await?; - if doc.find(input.layout_id).is_none() { - return Err(AppError::NotFound(format!("layout {}", input.layout_id))); - } - doc.active_id = input.layout_id; + // Self-heal: keep the requested id when valid, else fall back to the + // (always-valid, I2) current active id rather than erroring. + let active_id = doc.resolve_existing_id(Some(input.layout_id)); + doc.active_id = active_id; persist_doc(self.fs.as_ref(), &project, &doc).await?; self.events.publish(DomainEvent::LayoutChanged { project_id: input.project_id, }); - Ok(()) + Ok(SetActiveLayoutOutput { active_id }) } } diff --git a/crates/application/src/layout/mod.rs b/crates/application/src/layout/mod.rs index 7a1b5eb..6908d42 100644 --- a/crates/application/src/layout/mod.rs +++ b/crates/application/src/layout/mod.rs @@ -29,7 +29,7 @@ mod usecases; pub use management::{ CreateLayout, CreateLayoutInput, CreateLayoutOutput, DeleteLayout, DeleteLayoutInput, DeleteLayoutOutput, LayoutInfo, ListLayouts, ListLayoutsInput, ListLayoutsOutput, RenameLayout, - RenameLayoutInput, SetActiveLayout, SetActiveLayoutInput, + RenameLayoutInput, SetActiveLayout, SetActiveLayoutInput, SetActiveLayoutOutput, }; pub use reconcile::{ReconcileLayouts, ReconcileLayoutsInput, ReconcileLayoutsOutput}; pub use snapshot::{ diff --git a/crates/application/src/layout/store.rs b/crates/application/src/layout/store.rs index e0527bc..d203b34 100644 --- a/crates/application/src/layout/store.rs +++ b/crates/application/src/layout/store.rs @@ -77,6 +77,22 @@ impl LayoutsDoc { id.unwrap_or(self.active_id) } + /// Resolves a requested view id to a **valid** layout id, never failing. + /// + /// Returns `id` when it is present in `layouts`; otherwise falls back to + /// `active_id` (guaranteed valid by invariant I2 / [`Self::is_valid`]). + /// A `None` request (the current view) also resolves to `active_id`. This + /// is the self-healing path: a stale `active_id` written by an external tool + /// (e.g. git overwriting `layouts.json`) degrades silently instead of + /// hard-erroring and freezing the workspace (invariant I3). + #[must_use] + pub fn resolve_existing_id(&self, id: Option) -> LayoutId { + match id { + Some(id) if self.find(id).is_some() => id, + _ => self.active_id, + } + } + /// Finds a layout by id. #[must_use] pub fn find(&self, id: LayoutId) -> Option<&NamedLayout> { @@ -177,3 +193,55 @@ pub async fn resolve_doc(fs: &dyn FileSystem, project: &Project) -> Result (LayoutsDoc, LayoutId, LayoutId) { + let active = LayoutId::new_random(); + let other = LayoutId::new_random(); + let doc = LayoutsDoc { + version: LAYOUTS_VERSION, + active_id: active, + layouts: vec![ + NamedLayout { + id: active, + name: "Default".to_owned(), + kind: LayoutKind::Terminal, + tree: default_tree(), + }, + NamedLayout { + id: other, + name: "Second".to_owned(), + kind: LayoutKind::Terminal, + tree: default_tree(), + }, + ], + }; + (doc, active, other) + } + + #[test] + fn resolve_existing_id_returns_present_id() { + let (doc, _active, other) = two_layout_doc(); + // A present, non-active id is returned verbatim. + assert_eq!(doc.resolve_existing_id(Some(other)), other); + } + + #[test] + fn resolve_existing_id_falls_back_to_active_when_absent() { + let (doc, active, _other) = two_layout_doc(); + // A stale id absent from `layouts` degrades to the valid active id. + let stale = LayoutId::new_random(); + assert_eq!(doc.resolve_existing_id(Some(stale)), active); + } + + #[test] + fn resolve_existing_id_none_resolves_to_active() { + let (doc, active, _other) = two_layout_doc(); + // The "current view" request resolves to the active id. + assert_eq!(doc.resolve_existing_id(None), active); + } +} diff --git a/crates/application/src/layout/usecases.rs b/crates/application/src/layout/usecases.rs index 794edef..93db87f 100644 --- a/crates/application/src/layout/usecases.rs +++ b/crates/application/src/layout/usecases.rs @@ -169,7 +169,10 @@ impl LoadLayout { pub async fn execute(&self, input: LoadLayoutInput) -> Result { let project = self.store.load_project(input.project_id).await?; let doc = resolve_doc(self.fs.as_ref(), &project).await?; - let id = doc.resolve_id(input.layout_id); + // Self-healing (I3): a stale requested id degrades to the valid active + // id instead of hard-erroring. `find` is now infallible in practice; + // the `ok_or` is kept as a purely defensive guard. + let id = doc.resolve_existing_id(input.layout_id); let named = doc .find(id) .ok_or_else(|| AppError::NotFound(format!("layout {id}")))?; @@ -229,7 +232,10 @@ impl MutateLayout { pub async fn execute(&self, input: MutateLayoutInput) -> Result { let project = self.store.load_project(input.project_id).await?; let mut doc = resolve_doc(self.fs.as_ref(), &project).await?; - let id = doc.resolve_id(input.layout_id); + // Self-healing (I3): a stale requested id degrades to the valid active + // id instead of hard-erroring. `find_mut` is now infallible in + // practice; the `ok_or` is kept as a purely defensive guard. + let id = doc.resolve_existing_id(input.layout_id); let named = doc .find_mut(id) diff --git a/crates/application/src/lib.rs b/crates/application/src/lib.rs index 1c37995..0e56e1e 100644 --- a/crates/application/src/lib.rs +++ b/crates/application/src/lib.rs @@ -31,8 +31,7 @@ pub mod window; pub mod workstate; pub use agent::{ - drain_with_readiness, drain_with_readiness_outcome, - reference_profile_id, reference_profiles, + drain_with_readiness, drain_with_readiness_outcome, reference_profile_id, reference_profiles, selectable_reference_profiles, send_blocking, AgentResumer, ChangeAgentProfile, ChangeAgentProfileInput, ChangeAgentProfileOutput, ConfigureProfiles, ConfigureProfilesInput, ConfigureProfilesOutput, CreateAgentFromScratch, CreateAgentInput, CreateAgentOutput, @@ -73,8 +72,8 @@ pub use layout::{ ListLayoutsInput, ListLayoutsOutput, LoadLayout, LoadLayoutInput, LoadLayoutOutput, MutateLayout, MutateLayoutInput, MutateLayoutOutput, NamedLayout, ReconcileLayouts, ReconcileLayoutsInput, ReconcileLayoutsOutput, RenameLayout, RenameLayoutInput, - SetActiveLayout, SetActiveLayoutInput, SnapshotRunningAgents, SnapshotRunningAgentsInput, - SnapshotRunningAgentsOutput, LAYOUTS_FILE, + SetActiveLayout, SetActiveLayoutInput, SetActiveLayoutOutput, SnapshotRunningAgents, + SnapshotRunningAgentsInput, SnapshotRunningAgentsOutput, LAYOUTS_FILE, }; pub use memory::{ CreateMemory, CreateMemoryInput, CreateMemoryOutput, DeleteMemory, DeleteMemoryInput, diff --git a/crates/application/tests/layout_usecases.rs b/crates/application/tests/layout_usecases.rs index e3668bd..a5996f6 100644 --- a/crates/application/tests/layout_usecases.rs +++ b/crates/application/tests/layout_usecases.rs @@ -338,21 +338,24 @@ async fn load_tolerates_corrupt_json_with_default() { } #[tokio::test] -async fn load_unknown_layout_id_is_not_found() { +async fn load_unknown_layout_id_self_heals_to_active() { let store = FakeStore::default(); let fs = FakeFs::default(); let id = register_project(&store, pid(5)).await; seed_layouts(&fs, lid(1), &single_leaf(nid(1))); + // A stale requested id (e.g. left over after git overwrote layouts.json) + // must NOT hard-error and freeze the workspace; it degrades silently to the + // valid active layout (invariant I3), whose id is authoritative (I4). let load = LoadLayout::new(Arc::new(store), Arc::new(fs)); - let err = load + let out = load .execute(LoadLayoutInput { project_id: id, layout_id: Some(lid(999)), }) .await - .expect_err("unknown layout id rejected"); - assert_eq!(err.code(), "NOT_FOUND", "got {err:?}"); + .expect("stale layout id must self-heal, not error"); + assert_eq!(out.layout_id, lid(1)); } #[tokio::test] @@ -937,13 +940,15 @@ async fn set_active_layout_switches_and_load_follows() { .unwrap(); // Switch back to the Default layout. - SetActiveLayout::new(Arc::new(store.clone()), Arc::new(fs.clone()), Arc::new(bus)) + let set = SetActiveLayout::new(Arc::new(store.clone()), Arc::new(fs.clone()), Arc::new(bus)) .execute(SetActiveLayoutInput { project_id: pid(35), layout_id: lid(1), }) .await .unwrap(); + // Output echoes the actually-activated id (I4). + assert_eq!(set.active_id, lid(1)); let loaded = LoadLayout::new(Arc::new(store), Arc::new(fs)) .execute(LoadLayoutInput { @@ -955,3 +960,31 @@ async fn set_active_layout_switches_and_load_follows() { assert_eq!(loaded.layout_id, lid(1)); assert_ne!(loaded.layout_id, created.layout_id); } + +#[tokio::test] +async fn set_active_layout_stale_id_self_heals_to_current_active() { + // Seeded env: a single "Default" layout, active = lid(1). + let (store, fs, bus) = mgmt_env(pid(36)).await; + + // Targeting a stale id (e.g. an activeId left over after git overwrote + // layouts.json) must NOT error and must NOT freeze the workspace (I3): + // it degrades to the current active id and returns it (I4). + let out = SetActiveLayout::new(Arc::new(store.clone()), Arc::new(fs.clone()), Arc::new(bus)) + .execute(SetActiveLayoutInput { + project_id: pid(36), + layout_id: lid(999), + }) + .await + .expect("stale set_active must self-heal, not error"); + assert_eq!(out.active_id, lid(1)); + + // And a subsequent load of the current view resolves to that valid id. + let loaded = LoadLayout::new(Arc::new(store), Arc::new(fs)) + .execute(LoadLayoutInput { + project_id: pid(36), + layout_id: None, + }) + .await + .unwrap(); + assert_eq!(loaded.layout_id, lid(1)); +} diff --git a/frontend/src/adapters/layout.ts b/frontend/src/adapters/layout.ts index d223bf4..8e022f6 100644 --- a/frontend/src/adapters/layout.ts +++ b/frontend/src/adapters/layout.ts @@ -48,8 +48,8 @@ export class TauriLayoutGateway implements LayoutGateway { }); } - setActiveLayout(projectId: string, layoutId: string): Promise { - return invoke("set_active_layout", { + setActiveLayout(projectId: string, layoutId: string): Promise<{ activeId: string }> { + return invoke<{ activeId: string }>("set_active_layout", { request: { projectId, layoutId }, }); } diff --git a/frontend/src/adapters/mock/index.ts b/frontend/src/adapters/mock/index.ts index 4229da3..a520224 100644 --- a/frontend/src/adapters/mock/index.ts +++ b/frontend/src/adapters/mock/index.ts @@ -846,11 +846,17 @@ export class MockLayoutGateway implements LayoutGateway { return { activeId: ps.activeId }; } - async setActiveLayout(projectId: string, layoutId: string): Promise { + async setActiveLayout( + projectId: string, + layoutId: string, + ): Promise<{ activeId: string }> { const ps = this.getProjectLayouts(projectId); + // Self-heal (I4): only adopt the requested id when it exists; otherwise keep + // the current active id. Either way the returned id is authoritative. if (ps.layouts.some((l) => l.id === layoutId)) { ps.activeId = layoutId; } + return { activeId: ps.activeId }; } } diff --git a/frontend/src/adapters/mock/layout.test.ts b/frontend/src/adapters/mock/layout.test.ts index 4ff1aea..385ed52 100644 --- a/frontend/src/adapters/mock/layout.test.ts +++ b/frontend/src/adapters/mock/layout.test.ts @@ -117,14 +117,25 @@ describe("MockLayoutGateway", () => { expect(activeId).toBe(newActive); }); - it("setActiveLayout switches the active layout", async () => { + it("setActiveLayout switches the active layout and returns the authoritative id", async () => { const gw = new MockLayoutGateway(); const { layoutId } = await gw.createLayout("p1", "Alt"); - await gw.setActiveLayout("p1", layoutId); + const result = await gw.setActiveLayout("p1", layoutId); + expect(result.activeId).toBe(layoutId); const { activeId } = await gw.listLayouts("p1"); expect(activeId).toBe(layoutId); }); + it("setActiveLayout self-heals: a stale id keeps (and returns) the current active id", async () => { + const gw = new MockLayoutGateway(); + const { activeId: initial } = await gw.listLayouts("p1"); + const result = await gw.setActiveLayout("p1", "ghost-layout-id"); + // The requested id does not exist → active id is unchanged and returned. + expect(result.activeId).toBe(initial); + const { activeId } = await gw.listLayouts("p1"); + expect(activeId).toBe(initial); + }); + it("loadLayout with a specific layoutId loads that layout's tree", async () => { const gw = new MockLayoutGateway(); const { layoutId } = await gw.createLayout("p1", "Named"); diff --git a/frontend/src/features/layout/LayoutTabs.test.tsx b/frontend/src/features/layout/LayoutTabs.test.tsx index ecec884..caada22 100644 --- a/frontend/src/features/layout/LayoutTabs.test.tsx +++ b/frontend/src/features/layout/LayoutTabs.test.tsx @@ -6,7 +6,12 @@ import { describe, it, expect, vi } from "vitest"; import { render, screen, waitFor, fireEvent, act } from "@testing-library/react"; import type { Gateways } from "@/ports"; -import { MockLayoutGateway, MockAgentGateway, MockTerminalGateway } from "@/adapters/mock"; +import { + MockLayoutGateway, + MockAgentGateway, + MockTerminalGateway, + MockSystemGateway, +} from "@/adapters/mock"; import { DIProvider } from "@/app/di"; import { LayoutTabs } from "./LayoutTabs"; @@ -14,11 +19,13 @@ function renderTabs( layout: MockLayoutGateway, projectId = "p1", onActiveLayoutChange = vi.fn(), + system?: MockSystemGateway, ) { const gateways = { layout, terminal: new MockTerminalGateway(), agent: new MockAgentGateway(), + ...(system ? { system } : {}), } as unknown as Gateways; return render( @@ -133,4 +140,83 @@ describe("LayoutTabs", () => { expect(screen.queryByText("Second")).toBeNull(); }); }); + + // ── Active-layout self-heal (feature/layouts-active-self-heal) ────────────── + + it("switching tabs adopts the backend's authoritative active id", async () => { + const layout = new MockLayoutGateway(); + const { layoutId: betaId } = await layout.createLayout("p1", "Beta"); + const onActiveLayoutChange = vi.fn(); + renderTabs(layout, "p1", onActiveLayoutChange); + + await waitFor(() => expect(screen.getByText("Beta")).toBeTruthy()); + + await act(async () => { + fireEvent.click(screen.getByText("Beta")); + }); + + await waitFor(() => { + expect(onActiveLayoutChange).toHaveBeenCalledWith( + expect.objectContaining({ id: betaId, name: "Beta" }), + ); + }); + }); + + it("on layoutChanged it re-lists and purges a vanished active id (self-heal)", async () => { + const layout = new MockLayoutGateway(); + const system = new MockSystemGateway(); + const { layoutId: betaId } = await layout.createLayout("p1", "Beta"); + const onActiveLayoutChange = vi.fn(); + renderTabs(layout, "p1", onActiveLayoutChange, system); + + await waitFor(() => expect(screen.getByText("Beta")).toBeTruthy()); + + // Make Beta the active layout from the UI. + await act(async () => { + fireEvent.click(screen.getByText("Beta")); + }); + await waitFor(() => + expect(onActiveLayoutChange).toHaveBeenCalledWith( + expect.objectContaining({ id: betaId }), + ), + ); + + // Simulate an EXTERNAL overwrite of layouts.json: Beta is removed and the + // backend active id falls back to Default — done straight on the gateway, + // bypassing the hook, so the hook still locally remembers the (now stale) id. + const { layouts } = await layout.listLayouts("p1"); + const defaultId = layouts.find((l) => l.name === "Default")!.id; + await layout.deleteLayout("p1", betaId); + + onActiveLayoutChange.mockClear(); + // The backend notifies the change → the hook re-lists and purges the stale id. + await act(async () => { + system.emit({ type: "layoutChanged", projectId: "p1" }); + }); + + await waitFor(() => { + expect(screen.queryByText("Beta")).toBeNull(); + expect(onActiveLayoutChange).toHaveBeenCalledWith( + expect.objectContaining({ id: defaultId, name: "Default" }), + ); + }); + }); + + it("ignores layoutChanged events for another project", async () => { + const layout = new MockLayoutGateway(); + const system = new MockSystemGateway(); + await layout.createLayout("p1", "Beta"); + renderTabs(layout, "p1", vi.fn(), system); + + await waitFor(() => expect(screen.getByText("Beta")).toBeTruthy()); + + // An event for a different project must not trigger a re-list/purge here. + await act(async () => { + system.emit({ type: "layoutChanged", projectId: "other-project" }); + }); + + // Both tabs are still present (no spurious reconciliation). + expect(screen.getByText("Beta")).toBeTruthy(); + expect(screen.getByText("Default")).toBeTruthy(); + }); }); diff --git a/frontend/src/features/layout/useLayouts.ts b/frontend/src/features/layout/useLayouts.ts index c575d8f..e910351 100644 --- a/frontend/src/features/layout/useLayouts.ts +++ b/frontend/src/features/layout/useLayouts.ts @@ -36,13 +36,42 @@ function describe(e: unknown): string { return String(e); } +/** True when `e` is the backend `NOT_FOUND` refusal (a {@link GatewayError}). */ +function isNotFound(e: unknown): boolean { + return ( + typeof e === "object" && + e !== null && + "code" in e && + (e as { code: unknown }).code === "NOT_FOUND" + ); +} + export function useLayouts(projectId: string | null): LayoutsViewModel { - const { layout: gateway } = useGateways(); + const { layout: gateway, system } = useGateways(); const [layouts, setLayouts] = useState([]); const [activeId, setActiveId] = useState(null); const [busy, setBusy] = useState(false); const [error, setError] = useState(null); + // Re-list the layouts and reconcile the locally-remembered active id against + // the backend's authoritative set (invariants I3/I4). Any local active id that + // is absent from the freshly-listed layouts is *purged* — we never replay a + // vanished id (the bug after git overwrites `layouts.json`); we fall back to + // the active id the backend reports. The shared self-heal used on mount, on + // `layoutChanged`, and after a NOT_FOUND from an explicit op (rename/delete). + const refresh = useCallback(async () => { + if (!projectId || !gateway) return; + try { + const { layouts: list, activeId: aid } = await gateway.listLayouts(projectId); + setLayouts(list); + setActiveId((prev) => + prev && list.some((l) => l.id === prev) ? prev : aid, + ); + } catch (e) { + setError(describe(e)); + } + }, [gateway, projectId]); + // Load the list on mount and whenever projectId changes. useEffect(() => { if (!projectId || !gateway) { @@ -71,14 +100,40 @@ export function useLayouts(projectId: string | null): LayoutsViewModel { }; }, [gateway, projectId]); + // Re-sync on external layout changes (e.g. git overwriting `layouts.json`, + // reboot reconciliation): the backend emits `layoutChanged`. We re-list and + // purge any stale local active id so the tab bar never replays a vanished id. + useEffect(() => { + if (!projectId || !system) return; + let cancelled = false; + let unsubscribe: (() => void) | undefined; + void system + .onDomainEvent((event) => { + if (event.type === "layoutChanged" && event.projectId === projectId) { + void refresh(); + } + }) + .then((un) => { + if (cancelled) un(); + else unsubscribe = un; + }); + return () => { + cancelled = true; + unsubscribe?.(); + }; + }, [system, projectId, refresh]); + const setActive = useCallback( async (layoutId: string) => { if (!projectId || !gateway) return; setBusy(true); setError(null); try { - await gateway.setActiveLayout(projectId, layoutId); - setActiveId(layoutId); + // I4: the backend's returned id is authoritative — adopt it instead of + // re-imposing the requested id (which may be stale). Code defensively in + // case an older backend still returns void: fall back to the requested id. + const result = await gateway.setActiveLayout(projectId, layoutId); + setActiveId(result?.activeId ?? layoutId); } catch (e) { setError(describe(e)); } finally { @@ -119,12 +174,19 @@ export function useLayouts(projectId: string | null): LayoutsViewModel { prev.map((l) => (l.id === layoutId ? { ...l, name } : l)), ); } catch (e) { - setError(describe(e)); + // NOT_FOUND on an explicit op is legitimate (the layout vanished, e.g. + // an external overwrite) and must NOT freeze the UI: re-list and adopt + // the backend's active id instead of surfacing a hard error. + if (isNotFound(e)) { + await refresh(); + } else { + setError(describe(e)); + } } finally { setBusy(false); } }, - [gateway, projectId], + [gateway, projectId, refresh], ); const deleteLayout = useCallback( @@ -141,12 +203,18 @@ export function useLayouts(projectId: string | null): LayoutsViewModel { setLayouts((prev) => prev.filter((l) => l.id !== layoutId)); setActiveId(newActiveId); } catch (e) { - setError(describe(e)); + // NOT_FOUND means the layout was already gone (e.g. external overwrite): + // treat it as non-fatal — re-list and adopt the backend's active id. + if (isNotFound(e)) { + await refresh(); + } else { + setError(describe(e)); + } } finally { setBusy(false); } }, - [gateway, projectId, layouts.length], + [gateway, projectId, layouts.length, refresh], ); return { layouts, activeId, busy, error, setActive, create, rename, deleteLayout }; diff --git a/frontend/src/ports/index.ts b/frontend/src/ports/index.ts index 13ae836..d13068c 100644 --- a/frontend/src/ports/index.ts +++ b/frontend/src/ports/index.ts @@ -375,8 +375,14 @@ export interface LayoutGateway { renameLayout(projectId: string, layoutId: string, name: string): Promise; /** Deletes a layout; returns the new active layout id. */ deleteLayout(projectId: string, layoutId: string): Promise<{ activeId: string }>; - /** Sets the active layout for a project. */ - setActiveLayout(projectId: string, layoutId: string): Promise; + /** + * Sets the active layout for a project. Returns the layout id the backend + * *actually* activated — authoritative (invariant I4): it equals the requested + * id when valid, else the unchanged current active id (self-healing fallback + * when the requested id was stale, e.g. after an external overwrite of + * `layouts.json`). Callers must adopt this id rather than the one they asked for. + */ + setActiveLayout(projectId: string, layoutId: string): Promise<{ activeId: string }>; } /** Git: status/commit/checkout/… (L8). */