fix(layout): auto-réparer l'onglet actif
stale
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
<noreply@anthropic.com>
This commit is contained in:
@ -48,8 +48,8 @@ export class TauriLayoutGateway implements LayoutGateway {
|
||||
});
|
||||
}
|
||||
|
||||
setActiveLayout(projectId: string, layoutId: string): Promise<void> {
|
||||
return invoke<void>("set_active_layout", {
|
||||
setActiveLayout(projectId: string, layoutId: string): Promise<{ activeId: string }> {
|
||||
return invoke<{ activeId: string }>("set_active_layout", {
|
||||
request: { projectId, layoutId },
|
||||
});
|
||||
}
|
||||
|
||||
@ -846,11 +846,17 @@ export class MockLayoutGateway implements LayoutGateway {
|
||||
return { activeId: ps.activeId };
|
||||
}
|
||||
|
||||
async setActiveLayout(projectId: string, layoutId: string): Promise<void> {
|
||||
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 };
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -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");
|
||||
|
||||
@ -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(
|
||||
<DIProvider gateways={gateways}>
|
||||
@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@ -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<LayoutInfo[]>([]);
|
||||
const [activeId, setActiveId] = useState<string | null>(null);
|
||||
const [busy, setBusy] = useState(false);
|
||||
const [error, setError] = useState<string | null>(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 };
|
||||
|
||||
@ -375,8 +375,14 @@ export interface LayoutGateway {
|
||||
renameLayout(projectId: string, layoutId: string, name: string): Promise<void>;
|
||||
/** 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<void>;
|
||||
/**
|
||||
* 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). */
|
||||
|
||||
Reference in New Issue
Block a user