From 92bcd1568d59159dc3b0e7275d197dda136820eb Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 10 Jun 2026 23:04:10 -0700 Subject: [PATCH] fix(dashboard): address profile-unification review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings (dev review on PR #44007): 1. HIGH — stale page state on profile switch: pages load data on mount and didn't consume the profile scope, so a page opened under profile A kept showing A's state while writes silently targeted the newly selected B. Fixed structurally: ProfileKeyedRoutes wraps the routed page tree and keys it by the selected profile, remounting every page (fresh state + refetch) on switch. ChatPage keeps its own remount (channel keyed on scopedProfile). 2. HIGH — /api/model/auxiliary read was unscoped while /api/model/set wrote scoped (Models page could show default's aux pins while editing worker's). Endpoint now takes profile + _profile_scope, added to PROFILE_SCOPED_PREFIXES, HTTPException re-raise so ghost profiles 404 instead of 500. Regression test asserts read/write symmetry with differing worker/default aux config. 3. MEDIUM — tools post-setup spawned unscoped from the profile-aware drawer. Now spawns 'hermes -p tools post-setup ' (same mechanism as hub installs); drawer threads its profile prop. Most hooks install machine-level artifacts where the scope is inert, but hooks reading config/env now see the drawer's HERMES_HOME. 4. LOW — ty warnings: env Optional asserts before subscript/membership, fastapi import replaced with web_server.HTTPException re-use. 298 tests green across the four affected suites; tsc -b + vite build green; aux scoping E2E-verified with real imports. --- hermes_cli/web_server.py | 28 +++++- .../test_web_server_profile_unification.py | 94 ++++++++++++++++++- web/src/App.tsx | 40 +++++--- web/src/components/ToolsetConfigDrawer.tsx | 2 +- web/src/lib/api.ts | 5 +- 5 files changed, 149 insertions(+), 20 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index a782765adb1..83a59e794c9 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -2735,7 +2735,7 @@ def get_recommended_default_model(provider: str = ""): @app.get("/api/model/auxiliary") -def get_auxiliary_models(): +def get_auxiliary_models(profile: Optional[str] = None): """Return current auxiliary task assignments. Shape: @@ -2746,9 +2746,14 @@ def get_auxiliary_models(): ], "main": {"provider": "openrouter", "model": "anthropic/claude-opus-4.7"}, } + + ``profile`` scopes the read — without it, the Models page would show + the dashboard profile's auxiliary pins while /api/model/set wrote the + selected profile's (read/write asymmetry). """ try: - cfg = load_config() + with _profile_scope(profile): + cfg = load_config() aux_cfg = cfg.get("auxiliary", {}) if not isinstance(aux_cfg, dict): aux_cfg = {} @@ -2773,6 +2778,8 @@ def get_auxiliary_models(): main = {"provider": "", "model": str(model_cfg) if model_cfg else ""} return {"tasks": tasks, "main": main} + except HTTPException: + raise except Exception: _log.exception("GET /api/model/auxiliary failed") raise HTTPException(status_code=500, detail="Failed to read auxiliary config") @@ -8790,10 +8797,13 @@ async def save_toolset_env(name: str, body: ToolsetEnvUpdate, profile: Optional[ class ToolsetPostSetup(BaseModel): key: str + profile: Optional[str] = None @app.post("/api/tools/toolsets/{name}/post-setup") -async def run_toolset_post_setup(name: str, body: ToolsetPostSetup): +async def run_toolset_post_setup( + name: str, body: ToolsetPostSetup, profile: Optional[str] = None +): """Spawn a provider's post-setup install hook as a background action. Post-setup hooks (npm install for browser/Camofox, pip install for @@ -8803,6 +8813,12 @@ async def run_toolset_post_setup(name: str, body: ToolsetPostSetup): ``GET /api/actions/tools-post-setup/status``. The ``key`` is validated against the declared post-setup allowlist before spawning. Returns 400 for unknown toolset or post-setup key. + + ``profile`` spawns the hook as ``hermes -p tools post-setup``. + Most hooks install machine-level artifacts (repo node_modules, shared + pip packages) where the scope is inert, but hooks that read config or + write per-profile state must see the same HERMES_HOME the rest of the + drawer's writes targeted — so the scope is threaded for consistency. """ from hermes_cli.tools_config import ( _get_effective_configurable_toolsets, @@ -8820,8 +8836,12 @@ async def run_toolset_post_setup(name: str, body: ToolsetPostSetup): try: proc = _spawn_hermes_action( - ["tools", "post-setup", body.key], "tools-post-setup" + _profile_cli_args(profile or body.profile) + + ["tools", "post-setup", body.key], + "tools-post-setup", ) + except HTTPException: + raise except Exception as exc: _log.exception("Failed to spawn tools post-setup") raise HTTPException( diff --git a/tests/hermes_cli/test_web_server_profile_unification.py b/tests/hermes_cli/test_web_server_profile_unification.py index 4fb467c9b08..9c67a588bbf 100644 --- a/tests/hermes_cli/test_web_server_profile_unification.py +++ b/tests/hermes_cli/test_web_server_profile_unification.py @@ -196,6 +196,93 @@ class TestProfileScopedModel: if isinstance(default_model, dict): assert default_model.get("default") != "test/model-1" + def test_auxiliary_read_scoped_matches_write_target( + self, client, isolated_profiles + ): + """Reads and writes must scope symmetrically: an aux pin written to + the worker profile must show up ONLY in the worker-scoped read. + (Regression: /api/model/auxiliary used to read unscoped while + /api/model/set wrote scoped — the Models page displayed the + dashboard profile's pins while editing the selected profile's.)""" + (isolated_profiles["worker_beta"] / "config.yaml").write_text( + "auxiliary:\n vision:\n provider: openrouter\n" + " model: worker/vision-pin\n", + encoding="utf-8", + ) + resp = client.get("/api/model/auxiliary", params={"profile": "worker_beta"}) + assert resp.status_code == 200 + vision = next(t for t in resp.json()["tasks"] if t["task"] == "vision") + assert vision["model"] == "worker/vision-pin" + + # Unscoped read = the dashboard's own profile, which has no pin. + resp = client.get("/api/model/auxiliary") + assert resp.status_code == 200 + vision = next(t for t in resp.json()["tasks"] if t["task"] == "vision") + assert vision["model"] != "worker/vision-pin" + + def test_auxiliary_unknown_profile_404(self, client, isolated_profiles): + resp = client.get("/api/model/auxiliary", params={"profile": "ghost"}) + assert resp.status_code == 404 + + +class TestProfileScopedPostSetup: + def test_post_setup_spawns_with_profile_flag( + self, client, isolated_profiles, monkeypatch + ): + """Post-setup runs in a -p scoped subprocess so hooks that read + config / write per-profile state see the same HERMES_HOME the rest + of the drawer's writes targeted.""" + import hermes_cli.web_server as web_server + + calls = [] + + class _FakeProc: + pid = 777 + + monkeypatch.setattr( + web_server, + "_spawn_hermes_action", + lambda subcommand, name: calls.append(list(subcommand)) or _FakeProc(), + ) + monkeypatch.setattr( + "hermes_cli.tools_config.valid_post_setup_keys", + lambda: {"agent_browser"}, + ) + resp = client.post( + "/api/tools/toolsets/browser/post-setup", + json={"key": "agent_browser", "profile": "worker_beta"}, + ) + assert resp.status_code == 200 + assert calls == [ + ["-p", "worker_beta", "tools", "post-setup", "agent_browser"] + ] + + def test_post_setup_without_profile_keeps_legacy_argv( + self, client, isolated_profiles, monkeypatch + ): + import hermes_cli.web_server as web_server + + calls = [] + + class _FakeProc: + pid = 777 + + monkeypatch.setattr( + web_server, + "_spawn_hermes_action", + lambda subcommand, name: calls.append(list(subcommand)) or _FakeProc(), + ) + monkeypatch.setattr( + "hermes_cli.tools_config.valid_post_setup_keys", + lambda: {"agent_browser"}, + ) + resp = client.post( + "/api/tools/toolsets/browser/post-setup", + json={"key": "agent_browser"}, + ) + assert resp.status_code == 200 + assert calls == [["tools", "post-setup", "agent_browser"]] + class TestProfileScopedChatPty: def test_chat_argv_scopes_hermes_home(self, isolated_profiles, monkeypatch): @@ -207,6 +294,7 @@ class TestProfileScopedChatPty: raising=False, ) argv, cwd, env = web_server._resolve_chat_argv(profile="worker_beta") + assert env is not None assert env["HERMES_HOME"] == str(isolated_profiles["worker_beta"]) # Scoped chat must NOT attach to the dashboard's in-memory gateway. assert "HERMES_TUI_GATEWAY_URL" not in env @@ -220,10 +308,10 @@ class TestProfileScopedChatPty: raising=False, ) argv, cwd, env = web_server._resolve_chat_argv() + assert env is not None assert env.get("HERMES_HOME") != str(isolated_profiles["worker_beta"]) def test_chat_argv_unknown_profile_raises(self, isolated_profiles, monkeypatch): - from fastapi import HTTPException import hermes_cli.web_server as web_server monkeypatch.setattr( @@ -231,6 +319,8 @@ class TestProfileScopedChatPty: lambda root, tui_dev=False: (["cat"], None), raising=False, ) - with pytest.raises(HTTPException) as exc: + # Reuse the HTTPException class web_server itself raises — avoids a + # direct fastapi import (unresolvable in the ty lint environment). + with pytest.raises(web_server.HTTPException) as exc: web_server._resolve_chat_argv(profile="ghost") assert exc.value.status_code == 404 diff --git a/web/src/App.tsx b/web/src/App.tsx index 601261e3d82..d3c976358d5 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -65,6 +65,7 @@ import { useSidebarStatus } from "@/hooks/useSidebarStatus"; import { AuthWidget } from "@/components/AuthWidget"; import { PageHeaderProvider } from "@/contexts/PageHeaderProvider"; import { ProfileProvider } from "@/contexts/ProfileProvider"; +import { useProfileScope } from "@/contexts/useProfileScope"; import { ProfileSwitcher } from "@/components/ProfileSwitcher"; import { ProfileScopeBanner } from "@/components/ProfileScopeBanner"; import { useSystemActions } from "@/contexts/useSystemActions"; @@ -734,17 +735,19 @@ export default function App() { "min-h-0 flex flex-1 flex-col", )} > - - {routes.map(({ key, path, element }) => ( - - ))} - - } - /> - + + + {routes.map(({ key, path, element }) => ( + + ))} + + } + /> + + {embeddedChat && !chatOverriddenByPlugin && @@ -786,6 +789,21 @@ export default function App() { ); } +/** + * Remounts the entire routed page tree when the global management profile + * changes. Pages load their data on mount; without this, a page opened + * under profile A would keep showing A's state while writes (via the + * fetchJSON ?profile= injection) silently targeted the newly selected + * profile B — the exact stale-target footgun the switcher exists to kill. + * Keying by profile resets every page's local state so it refetches under + * the new scope. The persistent ChatPage host below handles its own + * remount (channel keyed on scopedProfile). + */ +function ProfileKeyedRoutes({ children }: { children: ReactNode }) { + const { profile } = useProfileScope(); + return
{children}
; +} + function SidebarNavLink({ closeMobile, collapsed, diff --git a/web/src/components/ToolsetConfigDrawer.tsx b/web/src/components/ToolsetConfigDrawer.tsx index 5bbcba61866..792393c9285 100644 --- a/web/src/components/ToolsetConfigDrawer.tsx +++ b/web/src/components/ToolsetConfigDrawer.tsx @@ -198,7 +198,7 @@ export function ToolsetConfigDrawer({ toolset, profile, onClose, onChanged }: Pr setPostSetupLog([]); setPostSetupKey(provider.post_setup); try { - await api.runToolsetPostSetup(toolset.name, provider.post_setup); + await api.runToolsetPostSetup(toolset.name, provider.post_setup, profile); // Bump the trigger so the poll effect (re)starts tailing the log. setPostSetupTrigger((n) => n + 1); } catch (e) { diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 888f1b3d99b..e6754afeefe 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -70,6 +70,7 @@ const PROFILE_SCOPED_PREFIXES = [ "/api/mcp", "/api/model/info", "/api/model/set", + "/api/model/auxiliary", ]; function withManagementProfile(url: string): string { @@ -636,13 +637,13 @@ export const api = { body: JSON.stringify({ env, profile: profile || undefined }), }, ), - runToolsetPostSetup: (name: string, key: string) => + runToolsetPostSetup: (name: string, key: string, profile?: string) => fetchJSON( `/api/tools/toolsets/${encodeURIComponent(name)}/post-setup`, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ key }), + body: JSON.stringify({ key, profile: profile || undefined }), }, ),