fix: fence shared chrome overrides by route

2026-09-23 01:29:21 +09:00
parent 9857bf04f7
commit bd3cc65e19
15 changed files with 248 additions and 27 deletions
@@ -6,7 +6,7 @@
    "dev": "deno run -A npm:vite@7.2.7 dev",
    "dev:backend": "cd ../.. && cargo run -p yoi-workspace-server --bin yoi-server -- serve --listen 127.0.0.1:8787",
    "check": "deno run -A npm:@sveltejs/kit@2.49.4 sync && deno run -A npm:svelte-check@4.3.4 --tsconfig ./tsconfig.json",
-    "test": "deno test --allow-read=src,test,tests,../../docs --allow-env=LOG,VSCODE_TEXTMATE_DEBUG,NODE_ENV tests/workspace-model.test.ts tests/repository-api.test.ts tests/workspace-catalog.test.ts tests/profile-api.test.ts tests/skill-api.test.ts tests/tooltip-contract.test.ts tests/bevel-contract.test.ts tests/bevel-line-contract.test.ts tests/typography-contract.test.ts src/lib/workspace/auth/model.test.ts tests/auth-api.test.ts src/lib/workspace/api/http.test.ts src/lib/workspace/api/workers.test.ts src/lib/workspace/header/breadcrumb-model.test.ts src/lib/workspace/console/chat-submit.test.ts src/lib/workspace/console/composer-delivery.test.ts test/composer-history.test.ts tests/composer-paste.test.ts src/lib/workspace/console/composer-command.test.ts src/lib/workspace/console/composer-draft.test.ts src/lib/workspace/console/composer-completion.test.ts test/console/ansi.test.ts src/lib/workspace/console/model.test.ts src/lib/workspace/console/run-status.test.ts src/lib/workspace/companion/api.test.ts tests/workdir-api.test.ts src/lib/workspace/console/tasks.test.ts test/ticket-detail-route-reuse.test.ts test/objective-pages.test.ts test/repositories/ui.test.ts src/lib/workspace/console/worker-console.ui.test.ts src/lib/workspace/settings/model.test.ts tests/runtime-connection.test.ts tests/runtime-management.test.ts tests/runtime-management-source.test.ts src/lib/workspace/sidebar/override-stack.test.ts src/lib/workspace/sidebar/workers.test.ts src/lib/workspace/sidebar/workspace-switcher.test.ts src/lib/workspace/sidebar/worker-subscription.test.ts src/lib/workspace/sidebar/worker-workdir-meta.test.ts src/lib/workspace/sidebar/worker-launch.test.ts test/sidebar/worker-actions.test.ts src/lib/workspace/tickets/merge-request-resources.test.ts src/lib/workspace/tickets/ticket-panel.test.ts test/merge-request-status.test.ts test/config-source/decodal-grammar.test.ts test/config-source/editor-state.test.ts test/config-source/fixed-schema-wrapper.test.ts test/config-source/toolchain.test.ts test/config-source/wasm-parity.test.ts test/repository-access/api.test.ts test/repository-access/loader.test.ts test/repository-access/ui.test.ts test/repository-ssh-connection-ui.test.ts",
+    "test": "deno test --allow-read=src,test,tests,../../docs --allow-env=LOG,VSCODE_TEXTMATE_DEBUG,NODE_ENV tests/workspace-model.test.ts tests/repository-api.test.ts tests/workspace-catalog.test.ts tests/profile-api.test.ts tests/skill-api.test.ts tests/tooltip-contract.test.ts tests/bevel-contract.test.ts tests/bevel-line-contract.test.ts tests/typography-contract.test.ts src/lib/workspace/auth/model.test.ts tests/auth-api.test.ts src/lib/workspace/api/http.test.ts src/lib/workspace/api/workers.test.ts src/lib/workspace/header/breadcrumb-model.test.ts src/lib/workspace/console/chat-submit.test.ts src/lib/workspace/console/composer-delivery.test.ts test/composer-history.test.ts tests/composer-paste.test.ts src/lib/workspace/console/composer-command.test.ts src/lib/workspace/console/composer-draft.test.ts src/lib/workspace/console/composer-completion.test.ts test/console/ansi.test.ts src/lib/workspace/console/model.test.ts src/lib/workspace/console/run-status.test.ts src/lib/workspace/companion/api.test.ts tests/workdir-api.test.ts src/lib/workspace/console/tasks.test.ts test/ticket-detail-route-reuse.test.ts test/objective-pages.test.ts test/repositories/ui.test.ts src/lib/workspace/console/worker-console.ui.test.ts src/lib/workspace/settings/model.test.ts tests/runtime-connection.test.ts tests/runtime-management.test.ts tests/runtime-management-source.test.ts src/lib/workspace/sidebar/override-stack.test.ts src/lib/workspace/sidebar/route-ownership.test.ts src/lib/workspace/sidebar/workers.test.ts src/lib/workspace/sidebar/workspace-switcher.test.ts src/lib/workspace/sidebar/worker-subscription.test.ts src/lib/workspace/sidebar/worker-workdir-meta.test.ts src/lib/workspace/sidebar/worker-launch.test.ts test/sidebar/worker-actions.test.ts src/lib/workspace/tickets/merge-request-resources.test.ts src/lib/workspace/tickets/ticket-panel.test.ts test/merge-request-status.test.ts test/config-source/decodal-grammar.test.ts test/config-source/editor-state.test.ts test/config-source/fixed-schema-wrapper.test.ts test/config-source/toolchain.test.ts test/config-source/wasm-parity.test.ts test/repository-access/api.test.ts test/repository-access/loader.test.ts test/repository-access/ui.test.ts test/repository-ssh-connection-ui.test.ts",
    "test:component": "deno run -A npm:vitest@3.2.4 run --config vitest.config.ts",
    "build": "deno run -A npm:vite@7.2.7 build",
    "preview": "deno run -A npm:vite@7.2.7 preview",
@@ -179,6 +179,13 @@ Deno.test("design lab demonstrates nested sidebar slots without flattening level
    workspaceLayout.includes("content={sidebarContent}"),
    "Workspace design-lab layout must expose its child override slot",
  );
+  assert(
+    workspaceLayout.includes(
+      "ownsRoutePath(designLabBasePath, page.url.pathname)",
+    ) &&
+      workspaceLayout.includes("active={ownsCurrentRoute}"),
+    "Workspace design-lab chrome must unregister outside its owned route",
+  );
  assert(
    !workspaceLayout.includes("<GlobalNavSections"),
    "Workspace design-lab layout must not flatten Global navigation",
@@ -200,6 +207,11 @@ Deno.test("design lab demonstrates nested sidebar slots without flattening level
    settingsLayout.includes("controller={parentSidebarController}"),
    "Settings design-lab layout must register into its parent slot",
  );
+  assert(
+    settingsLayout.includes("ownsRoutePath(`${designLabBasePath}/settings`") &&
+      settingsLayout.includes("active={ownsCurrentRoute}"),
+    "Settings design-lab chrome must unregister outside its owned route",
+  );
  assert(
    !settingsLayout.includes("<WorkspaceSidebar"),
    "Settings design-lab layout must not duplicate WorkspaceSidebar",
@@ -1011,6 +1023,9 @@ Deno.test("Account UI owns browser passkey session state without workspace autho
  const sidebarOverride = await Deno.readTextFile(
    new URL("../sidebar/SidebarOverride.svelte", import.meta.url),
  );
+  const headerOverride = await Deno.readTextFile(
+    new URL("../header/HeaderOverride.svelte", import.meta.url),
+  );
  const sidebar = await Deno.readTextFile(
    new URL("../sidebar/WorkspaceSidebar.svelte", import.meta.url),
  );
@@ -1104,10 +1119,20 @@ Deno.test("Account UI owns browser passkey session state without workspace autho
    "Sidebar styles should define their layer order before component rules so base link styles do not win by import order",
  );
  assert(
+    sidebarOverride.includes("if (!active) return") &&
      sidebarOverride.includes("controller.registerSidebar(sidebar)") &&
+      workspaceLayout.includes("ownsRoutePath(workspaceRoute(workspaceId)") &&
+      workspaceLayout.includes("active={ownsCurrentRoute}") &&
      rootLayout.includes("createOverrideStack<SidebarSnippet>") &&
      rootLayout.includes("registerSidebar: sidebarOverrides.register"),
-    "SidebarOverride should register a nested sidebar whose cleanup restores the parent override",
+    "Sidebar overrides should register only while their route owns the shared chrome slot",
+  );
+  assert(
+    headerOverride.includes("if (!active) return") &&
+      headerOverride.includes("controller.registerContent(content)") &&
+      rootLayout.includes("createOverrideStack<HeaderSnippet>") &&
+      rootLayout.includes("registerContent: headerOverrides.register"),
+    "Header overrides should share the route-scoped LIFO registration contract",
  );
  assert(
    rootLayoutLoad.includes("export const load") &&
@@ -2,13 +2,17 @@
  import type { Snippet } from 'svelte';
  import { getHeaderController } from './context';

-  let { content }: { content: Snippet<[]> } = $props();
+  let {
+    content,
+    active = true,
+  }: {
+    content: Snippet<[]>;
+    active?: boolean;
+  } = $props();
  const controller = getHeaderController();

  $effect(() => {
-    controller.content = content;
-    return () => {
-      if (controller.content === content) controller.content = null;
-    };
+    if (!active) return;
+    return controller.registerContent(content);
  });
 </script>
@@ -3,9 +3,9 @@ import type { Snippet } from "svelte";

 const HEADER_CONTEXT_KEY = Symbol("workspace-header");

-export type HeaderContent = Snippet<[]> | null;
+export type HeaderSnippet = Snippet<[]>;
 export type HeaderController = {
-  content: HeaderContent;
+  registerContent(content: HeaderSnippet): () => void;
 };

 export function provideHeaderController(controller: HeaderController): void {
@@ -8,10 +8,18 @@
  type Props = {
    sidebar: SidebarSnippet;
    controller?: SidebarController;
+    active?: boolean;
  };

  const inheritedController = getSidebarController();
-  const { sidebar, controller = inheritedController }: Props = $props();
+  const {
+    sidebar,
+    controller = inheritedController,
+    active = true,
+  }: Props = $props();

-  $effect(() => controller.registerSidebar(sidebar));
+  $effect(() => {
+    if (!active) return;
+    return controller.registerSidebar(sidebar);
+  });
 </script>
@@ -2,6 +2,7 @@
  import type { Snippet } from 'svelte';
  import './sidebar.css';
  import { workspaceRoute } from '$lib/workspace/api/http';
+  import { ownsRoutePath } from './route-ownership';
  import ObjectivesNavSection from './ObjectivesNavSection.svelte';
  import MemoryNavSection from './MemoryNavSection.svelte';
  import MergeRequestsNavSection from './MergeRequestsNavSection.svelte';
@@ -50,11 +51,11 @@
          </a>
          <a
            class="workspace-sidebar-shortcut"
-            class:active={currentPath.startsWith(workspaceSettingsHref)}
+            class:active={ownsRoutePath(workspaceSettingsHref, currentPath)}
            href={workspaceSettingsHref}
            aria-label="Workspace settings"
            title="Workspace settings"
-            aria-current={currentPath.startsWith(workspaceSettingsHref) ? 'page' : undefined}
+            aria-current={ownsRoutePath(workspaceSettingsHref, currentPath) ? 'page' : undefined}
          >
            <svg viewBox="0 0 24 24" aria-hidden="true">
              <path d="M4 5h16M4 12h16M4 19h16"></path>
@@ -0,0 +1,78 @@
+// @vitest-environment happy-dom
+
+import { cleanup, render, waitFor } from "@testing-library/svelte";
+import { afterEach, expect, test, vi } from "vitest";
+import type { Snippet } from "svelte";
+import HeaderOverride from "../header/HeaderOverride.svelte";
+import SidebarOverride from "./SidebarOverride.svelte";
+
+const mocks = vi.hoisted(() => {
+  const disposeHeader = vi.fn();
+  const disposeSidebar = vi.fn();
+  return {
+    disposeHeader,
+    disposeSidebar,
+    registerHeader: vi.fn(() => disposeHeader),
+    registerSidebar: vi.fn(() => disposeSidebar),
+  };
+});
+
+vi.mock("./context", () => ({
+  getSidebarController: () => ({
+    registerSidebar: mocks.registerSidebar,
+  }),
+}));
+
+vi.mock("../header/context", () => ({
+  getHeaderController: () => ({
+    registerContent: mocks.registerHeader,
+  }),
+}));
+
+const firstSnippet = (() => {}) as Snippet<[]>;
+
+afterEach(() => {
+  cleanup();
+  mocks.disposeHeader.mockClear();
+  mocks.disposeSidebar.mockClear();
+  mocks.registerHeader.mockClear();
+  mocks.registerHeader.mockReturnValue(mocks.disposeHeader);
+  mocks.registerSidebar.mockClear();
+  mocks.registerSidebar.mockReturnValue(mocks.disposeSidebar);
+});
+
+test("sidebar override registers only while its route is active", async () => {
+  const view = render(SidebarOverride, {
+    sidebar: firstSnippet,
+    active: false,
+  });
+
+  expect(mocks.registerSidebar).not.toHaveBeenCalled();
+
+  await view.rerender({ sidebar: firstSnippet, active: true });
+  await waitFor(() => {
+    expect(mocks.registerSidebar).toHaveBeenCalledOnce();
+    expect(mocks.registerSidebar).toHaveBeenCalledWith(firstSnippet);
+  });
+
+  await view.rerender({ sidebar: firstSnippet, active: false });
+  await waitFor(() => expect(mocks.disposeSidebar).toHaveBeenCalledOnce());
+});
+
+test("header override registers only while its route is active", async () => {
+  const view = render(HeaderOverride, {
+    content: firstSnippet,
+    active: false,
+  });
+
+  expect(mocks.registerHeader).not.toHaveBeenCalled();
+
+  await view.rerender({ content: firstSnippet, active: true });
+  await waitFor(() => {
+    expect(mocks.registerHeader).toHaveBeenCalledOnce();
+    expect(mocks.registerHeader).toHaveBeenCalledWith(firstSnippet);
+  });
+
+  await view.rerender({ content: firstSnippet, active: false });
+  await waitFor(() => expect(mocks.disposeHeader).toHaveBeenCalledOnce());
+});
@@ -0,0 +1,48 @@
+declare const Deno: {
+  test(name: string, fn: () => void): void;
+};
+
+import { ownsRoutePath } from "./route-ownership.ts";
+
+function assertEquals(actual: unknown, expected: unknown): void {
+  if (!Object.is(actual, expected)) {
+    throw new Error(`Expected ${String(expected)}, got ${String(actual)}`);
+  }
+}
+
+Deno.test("route ownership includes only the base route and its descendants", () => {
+  assertEquals(
+    ownsRoutePath(
+      "/design-lab/workspace-web-ux",
+      "/design-lab/workspace-web-ux",
+    ),
+    true,
+  );
+  assertEquals(
+    ownsRoutePath(
+      "/design-lab/workspace-web-ux",
+      "/design-lab/workspace-web-ux/settings",
+    ),
+    true,
+  );
+  assertEquals(
+    ownsRoutePath(
+      "/design-lab/workspace-web-ux",
+      "/w/0197a949-4b6b-7f2a-9d9a-1f87e3a4c5b6/workers",
+    ),
+    false,
+  );
+});
+
+Deno.test("route ownership rejects prefix collisions and empty authorities", () => {
+  assertEquals(ownsRoutePath("/w/alpha", "/w/alphabet/workers"), false);
+  assertEquals(
+    ownsRoutePath(
+      "/design-lab/workspace-web-ux/settings",
+      "/design-lab/workspace-web-ux/settings-preview",
+    ),
+    false,
+  );
+  assertEquals(ownsRoutePath("", "/w/alpha"), false);
+  assertEquals(ownsRoutePath("/w/alpha", ""), false);
+});
@@ -0,0 +1,4 @@
+export function ownsRoutePath(basePath: string, currentPath: string): boolean {
+  if (!basePath || !currentPath) return false;
+  return currentPath === basePath || currentPath.startsWith(`${basePath}/`);
+}
@@ -3,7 +3,11 @@
  import { setContext } from 'svelte';
  import WorkspaceAlerts from '$lib/workspace/alerts/WorkspaceAlerts.svelte';
  import Bevel from '$lib/workspace/ui/Bevel.svelte';
-  import { provideHeaderController, type HeaderController } from '$lib/workspace/header/context';
+  import {
+    provideHeaderController,
+    type HeaderController,
+    type HeaderSnippet,
+  } from '$lib/workspace/header/context';
  import GlobalSidebar from '$lib/workspace/sidebar/GlobalSidebar.svelte';
  import SidebarFrame from '$lib/workspace/sidebar/SidebarFrame.svelte';
  import { SIDEBAR_CONTEXT, type SidebarController, type SidebarSnippet } from '$lib/workspace/sidebar/context';
@@ -16,14 +20,19 @@
  const sidebarOverrides = createOverrideStack<SidebarSnippet>((activeSidebar) => {
    sidebar = activeSidebar;
  });
-  const headerController = $state<HeaderController>({ content: null });
+  let header = $state<HeaderSnippet | null>(null);
+  const headerOverrides = createOverrideStack<HeaderSnippet>((activeHeader) => {
+    header = activeHeader;
+  });
  let sidebarFolded = $state(false);

  function toggleSidebar() {
    sidebarFolded = !sidebarFolded;
  }

-  provideHeaderController(headerController);
+  provideHeaderController({
+    registerContent: headerOverrides.register,
+  } satisfies HeaderController);
  setContext<SidebarController>(SIDEBAR_CONTEXT, {
    registerSidebar: sidebarOverrides.register,
  });
@@ -43,7 +52,7 @@
  <Bevel as="div" class="app-shell__topbar-bevel" top={false} right={false} left={false} fill>
    <header class="app-shell__topbar">
      <div class="app-shell__topbar-location">
-        {#if headerController.content}{@render headerController.content()}{/if}
+        {#if header}{@render header()}{/if}
      </div>
      <nav class="app-shell__topbar-actions" aria-label="Global navigation">
        <button
@@ -10,6 +10,7 @@
    type SidebarSnippet,
  } from '$lib/workspace/sidebar/context';
  import { createOverrideStack } from '$lib/workspace/sidebar/override-stack';
+  import { ownsRoutePath } from '$lib/workspace/sidebar/route-ownership';
  import WorkspaceSidebarFixture from './WorkspaceSidebarFixture.svelte';
  import { designLabBasePath } from './workspace-navigation';
  import './showroom.css';
@@ -26,7 +27,12 @@
    registerSidebar: sidebarContentOverrides.register,
  });

-  const inSettings = $derived(page.url.pathname.startsWith(`${designLabBasePath}/settings`));
+  const ownsCurrentRoute = $derived(
+    ownsRoutePath(designLabBasePath, page.url.pathname),
+  );
+  const inSettings = $derived(
+    ownsRoutePath(`${designLabBasePath}/settings`, page.url.pathname),
+  );
 </script>

 {#snippet designLabHeader()}
@@ -45,7 +51,11 @@
  <WorkspaceSidebarFixture currentPath={page.url.pathname} content={sidebarContent} />
 {/snippet}

-<HeaderOverride content={designLabHeader} />
-<SidebarOverride controller={parentSidebarController} sidebar={workspaceSidebar} />
+<HeaderOverride content={designLabHeader} active={ownsCurrentRoute} />
+<SidebarOverride
+  controller={parentSidebarController}
+  sidebar={workspaceSidebar}
+  active={ownsCurrentRoute}
+/>

 {@render children()}
@@ -1,5 +1,6 @@
 <script lang="ts">
  import type { Snippet } from 'svelte';
+  import { ownsRoutePath } from '$lib/workspace/sidebar/route-ownership';
  import { designLabBasePath, workspaceNavigation, workspaceWorkers } from './workspace-navigation';

  let {
@@ -32,7 +33,7 @@
      </a>
      <a
        class="workspace-sidebar-shortcut"
-        class:active={currentPath.startsWith(settingsPath)}
+        class:active={ownsRoutePath(settingsPath, currentPath)}
        href={settingsPath}
        aria-label="Workspace settings"
        title="Workspace settings"
@@ -9,7 +9,9 @@
    type SidebarSnippet,
  } from '$lib/workspace/sidebar/context';
  import { createOverrideStack } from '$lib/workspace/sidebar/override-stack';
+  import { ownsRoutePath } from '$lib/workspace/sidebar/route-ownership';
  import SettingsSidebarFixture from './SettingsSidebarFixture.svelte';
+  import { designLabBasePath } from '../workspace-navigation';
  import type { LayoutProps } from './$types';

  let { children }: LayoutProps = $props();
@@ -22,12 +24,20 @@
  setContext<SidebarController>(SIDEBAR_CONTEXT, {
    registerSidebar: sidebarContentOverrides.register,
  });
+
+  const ownsCurrentRoute = $derived(
+    ownsRoutePath(`${designLabBasePath}/settings`, page.url.pathname),
+  );
 </script>

 {#snippet settingsSidebar()}
  <SettingsSidebarFixture currentPath={page.url.pathname} content={sidebarContent} />
 {/snippet}

-<SidebarOverride controller={parentSidebarController} sidebar={settingsSidebar} />
+<SidebarOverride
+  controller={parentSidebarController}
+  sidebar={settingsSidebar}
+  active={ownsCurrentRoute}
+/>

 {@render children()}
@@ -11,6 +11,8 @@
    type SidebarSnippet,
  } from '$lib/workspace/sidebar/context';
  import { createOverrideStack } from '$lib/workspace/sidebar/override-stack';
+  import { ownsRoutePath } from '$lib/workspace/sidebar/route-ownership';
+  import { workspaceRoute } from '$lib/workspace/api/http';
  import { disposeWorkspaceMultiplexer } from '$lib/workspace/multiplexer';
  import { disposeWorkspaceWorkersStore } from '$lib/workspace/sidebar/worker-subscription';
  import WorkspaceSidebar from '$lib/workspace/sidebar/WorkspaceSidebar.svelte';
@@ -30,6 +32,11 @@
    registerSidebar: sidebarContentOverrides.register,
  });

+  const workspaceId = $derived(data.workspace?.workspace_id ?? page.params.workspaceId ?? '');
+  const ownsCurrentRoute = $derived(
+    workspaceId !== '' && ownsRoutePath(workspaceRoute(workspaceId), page.url.pathname),
+  );
+
  $effect(() => {
    const workspaceId = data.workspace?.workspace_id;
    if (!workspaceId) return;
@@ -57,7 +64,11 @@
  />
 {/snippet}

-<HeaderOverride content={workspaceHeader} />
-<SidebarOverride controller={parentSidebarController} sidebar={workspaceSidebar} />
+<HeaderOverride content={workspaceHeader} active={ownsCurrentRoute} />
+<SidebarOverride
+  controller={parentSidebarController}
+  sidebar={workspaceSidebar}
+  active={ownsCurrentRoute}
+/>

 {@render children()}
@@ -10,10 +10,12 @@
    type SidebarSnippet,
  } from '$lib/workspace/sidebar/context';
  import { createOverrideStack } from '$lib/workspace/sidebar/override-stack';
+  import { ownsRoutePath } from '$lib/workspace/sidebar/route-ownership';
+  import { workspaceRoute } from '$lib/workspace/api/http';
  import '$lib/workspace/styles/settings.css';
  import type { LayoutProps } from './$types';

-  let { children }: LayoutProps = $props();
+  let { data, children }: LayoutProps = $props();
  const parentSidebarController = getSidebarController();
  let sidebarContent = $state<SidebarSnippet | null>(null);
  const sidebarContentOverrides = createOverrideStack<SidebarSnippet>((activeContent) => {
@@ -23,6 +25,12 @@
  setContext<SidebarController>(SIDEBAR_CONTEXT, {
    registerSidebar: sidebarContentOverrides.register,
  });
+
+  const workspaceId = $derived(data.workspace?.workspace_id ?? page.params.workspaceId ?? '');
+  const ownsCurrentRoute = $derived(
+    workspaceId !== '' &&
+      ownsRoutePath(workspaceRoute(workspaceId, '/settings'), page.url.pathname),
+  );
 </script>

 {#snippet settingsSidebar()}
@@ -33,7 +41,11 @@
  />
 {/snippet}

-<SidebarOverride controller={parentSidebarController} sidebar={settingsSidebar} />
+<SidebarOverride
+  controller={parentSidebarController}
+  sidebar={settingsSidebar}
+  active={ownsCurrentRoute}
+/>

 <section class="settings-page">
  {@render children()}