From 5ba9b80d4dbe8adf4b5f69daeb52b056ddf9fbed Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Mon, 1 Jun 2026 18:09:03 +0200 Subject: [PATCH] feat(frontend): keep ?workspace= off workspace-agnostic pages Instance-level pages (/workers, /service_logs, /instance_groups, /concurrency_groups) don't depend on the active workspace, so seeding ?workspace= there is noise. Add a workspaceAgnosticRoute predicate: on these routes the layout strips the param for a clean URL, but first adopts any explicit ?workspace= into the store so a shared deep link still propagates the workspace to subsequent scoped pages. State is preserved by the store + storage, not the URL, so navigation across these pages keeps the workspace. Verified headless: agnostic pages carry no param, a deep-linked param is adopted (storage updated) then stripped, and scoped pages still seed. Co-Authored-By: Claude Opus 4.8 (1M context) --- frontend/src/lib/workspaceParam.test.ts | 32 ++++++++++++++++++- frontend/src/lib/workspaceParam.ts | 28 ++++++++++++++-- .../src/routes/(root)/(logged)/+layout.svelte | 22 ++++++++++--- 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/frontend/src/lib/workspaceParam.test.ts b/frontend/src/lib/workspaceParam.test.ts index 8f12e775e8..c41c20ec70 100644 --- a/frontend/src/lib/workspaceParam.test.ts +++ b/frontend/src/lib/workspaceParam.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { workspaceParamAllowed } from './workspaceParam' +import { workspaceParamAllowed, workspaceAgnosticRoute } from './workspaceParam' describe('workspaceParamAllowed', () => { it('allows regular workspace-scoped app routes', () => { @@ -36,3 +36,33 @@ describe('workspaceParamAllowed', () => { expect(workspaceParamAllowed('/scripts/get/f/team/oauth-helper')).toBe(true) }) }) + +describe('workspaceAgnosticRoute', () => { + it('flags instance-level routes as workspace-agnostic', () => { + expect(workspaceAgnosticRoute('/workers')).toBe(true) + expect(workspaceAgnosticRoute('/service_logs')).toBe(true) + expect(workspaceAgnosticRoute('/instance_groups')).toBe(true) + expect(workspaceAgnosticRoute('/concurrency_groups')).toBe(true) + expect(workspaceAgnosticRoute('/workers/sub/path')).toBe(true) + }) + + it('treats workspace-scoped routes as not agnostic', () => { + expect(workspaceAgnosticRoute('/')).toBe(false) + expect(workspaceAgnosticRoute('/runs')).toBe(false) + expect(workspaceAgnosticRoute('/audit_logs')).toBe(false) + expect(workspaceAgnosticRoute('/variables')).toBe(false) + expect(workspaceAgnosticRoute('/scripts/get/u/me/s')).toBe(false) + }) + + it('does not match on a mid-path or prefix-collision segment', () => { + expect(workspaceAgnosticRoute('/workers_extra')).toBe(false) + expect(workspaceAgnosticRoute('/apps/get/u/me/workers')).toBe(false) + }) + + it('agnostic routes are still within the allowed (non-hard-excluded) set', () => { + // They must pass workspaceParamAllowed so the param can be read/adopted + // before being stripped. + expect(workspaceParamAllowed('/workers')).toBe(true) + expect(workspaceParamAllowed('/service_logs')).toBe(true) + }) +}) diff --git a/frontend/src/lib/workspaceParam.ts b/frontend/src/lib/workspaceParam.ts index b17f7f43e4..6b02012ff8 100644 --- a/frontend/src/lib/workspaceParam.ts +++ b/frontend/src/lib/workspaceParam.ts @@ -11,10 +11,32 @@ const WORKSPACE_PARAM_EXCLUDED_PREFIXES = ['/user/', '/oauth/'] /** - * Whether the `?workspace=` query param should be kept in sync with the active - * workspace store on the given route. Drives both directions of the sync in the - * logged layout: URL → store on navigation, and store → URL via replaceState. + * Whether the `?workspace=` query param may appear at all on the given route. + * Hard exclusion: drives both directions of the sync in the logged layout — when + * false the param is never read into the store nor written to the URL. */ export function workspaceParamAllowed(pathname: string): boolean { return !WORKSPACE_PARAM_EXCLUDED_PREFIXES.some((prefix) => pathname.startsWith(prefix)) } + +// Routes whose content does not depend on the active workspace (instance-level +// pages). The param is meaningless here, so the URL is kept clean: an explicit +// `?workspace=` is still adopted into the store — so a shared link propagates the +// workspace to subsequent workspace-scoped pages — and then stripped. The active +// workspace is preserved by the store + storage, not by the URL on these routes. +const WORKSPACE_AGNOSTIC_PREFIXES = [ + '/workers', + '/service_logs', + '/instance_groups', + '/concurrency_groups' +] + +/** + * Whether the given route is workspace-agnostic (instance-level). On these + * routes the logged layout strips `?workspace=` rather than seeding it. + */ +export function workspaceAgnosticRoute(pathname: string): boolean { + return WORKSPACE_AGNOSTIC_PREFIXES.some( + (prefix) => pathname === prefix || pathname.startsWith(prefix + '/') + ) +} diff --git a/frontend/src/routes/(root)/(logged)/+layout.svelte b/frontend/src/routes/(root)/(logged)/+layout.svelte index 46cc676d29..095188dab1 100644 --- a/frontend/src/routes/(root)/(logged)/+layout.svelte +++ b/frontend/src/routes/(root)/(logged)/+layout.svelte @@ -38,7 +38,7 @@ import CenteredModal from '$lib/components/CenteredModal.svelte' import { afterNavigate, beforeNavigate, replaceState } from '$app/navigation' import { goto } from '$lib/navigation' - import { workspaceParamAllowed } from '$lib/workspaceParam' + import { workspaceParamAllowed, workspaceAgnosticRoute } from '$lib/workspaceParam' import UserSettings from '$lib/components/UserSettings.svelte' import SuperadminSettings from '$lib/components/SuperadminSettings.svelte' import WindmillIcon from '$lib/components/icons/WindmillIcon.svelte' @@ -137,10 +137,24 @@ // which also prevents a ping-pong with the URL → store sync in onQueryChange. function syncWorkspaceToUrl() { if (!BROWSER || !routerReady) return - const ws = $workspaceStore - if (!ws || !workspaceParamAllowed(page.url.pathname)) return - if (page.url.searchParams.get('workspace') === ws) return + const path = page.url.pathname + if (!workspaceParamAllowed(path)) return + const param = page.url.searchParams.get('workspace') try { + if (workspaceAgnosticRoute(path)) { + // Workspace-agnostic page: adopt an explicit ?workspace= into the store + // (so it propagates to later scoped pages), then strip it for a clean + // URL. The store/storage keep the workspace; the URL need not. + if (!param) return + if (param !== $workspaceStore) $workspaceStore = param + const url = new URL(page.url) + url.searchParams.delete('workspace') + replaceState(url, page.state) + return + } + // Workspace-scoped page: reflect the active workspace into the URL. + const ws = $workspaceStore + if (!ws || param === ws) return const url = new URL(page.url) url.searchParams.set('workspace', ws) replaceState(url, page.state)