From 82ab74d1e85c62a322b0c0ac23eb94c6981c0c19 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sun, 6 Sep 2026 15:46:16 +0800 Subject: [PATCH 1/3] fix(conductor): let the fleet read surface run without prompting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit conductor-design §3 specifies that the fleet READ verbs — fleet_list, fleet_find, fleet_summary, fleet_recall, fleet_tasks, machine_map — "run silently". They did not. `isSafeTool` knew the memory and blackboard mounts but not the fleet, so in guarded mode every one of them raised an approval prompt. Found by driving a real conductor rather than reading the code: asking it to resolve a session reference produced an approval request for `fleet_find`, which is a read. An assistant that asks permission to look something up is not an assistant, and this is the first friction anyone meets in the new conductor pane (#325). The daemon already put these verbs in the provider's `allowedTools`, so the SDK warned it would auto-approve them — but codeoid's own gate is consulted independently and did not recognise them, which is why the two disagreed. Follows the established shape exactly: match the namespace prefix, then require the suffix to be a known read verb. Never a bare prefix match — an over-broad match here is a prompt bypass, which is why a look-alike segment (`mcp__evil_codeoid_fleet__…`) is asserted to gain nothing. The read list is `FLEET_READ_TOOLS` from the shared protocol package, so the send half cannot leak in by someone editing one of two copies. Send verbs remain hard-gated BEFORE this function is consulted (`isFleetSendTool` in Session#shouldAutoApprove, checked ahead of any mode logic); this is defence in depth, not the fence. Verified live, both directions. With the fix, fleet_list / fleet_find / machine_map go straight to `executing` with zero prompts; fleet_spawn still lands in `waiting_confirmation` and waits for the owner. The R3 invariant is intact. Also lists the bare (non-`mcp__`) namespacing so a mounted fleet (#245) does not silently regress to prompting on every read. 10 tool-safety tests; 2452 daemon tests; typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) --- src/daemon/providers/tool-safety.ts | 24 ++++++++++++++++++ src/tests/tool-safety.test.ts | 39 +++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/src/daemon/providers/tool-safety.ts b/src/daemon/providers/tool-safety.ts index cbe57b5..54f15e1 100644 --- a/src/daemon/providers/tool-safety.ts +++ b/src/daemon/providers/tool-safety.ts @@ -8,6 +8,7 @@ import { BLACKBOARD_MCP_SERVER_NAME } from "../blackboard/mcp-http.js"; import { MEMORY_MCP_SERVER_NAME } from "../memory/mcp-http.js"; import { MEMORY_TOOL_NAMES } from "../memory/tools.js"; +import { FLEET_READ_TOOLS, FLEET_TOOL_PREFIX } from "../../protocol/types.js"; /** Built-in read-only tools that never require confirmation. */ const SAFE_TOOLS = new Set(["Read", "Grep", "Glob"]); @@ -24,6 +25,18 @@ const BLACKBOARD_TOOL_PREFIXES = [ `${BLACKBOARD_MCP_SERVER_NAME}__`, ] as const; +/** + * Same two namespacing conventions, for the conductor's fleet mount. + * + * The bare form is unused today — the fleet server is a Claude in-process MCP + * object — but is listed so a mounted fleet (#245) does not silently regress to + * prompting on every read. + */ +const FLEET_TOOL_PREFIXES = [ + FLEET_TOOL_PREFIX, // `mcp__codeoid_fleet__` — Claude in-process MCP + FLEET_TOOL_PREFIX.replace(/^mcp__/, ""), // bare mount +] as const; + /** * Blackboard tools that may run unprompted. * @@ -60,6 +73,17 @@ export function isSafeTool(name: string): boolean { return (BLACKBOARD_SAFE_TOOLS as readonly string[]).includes(name.slice(prefix.length)); } } + // Fleet READS only. The send-class verbs are absent by construction — + // `FLEET_READ_TOOLS` is the read half of the shared vocabulary, so a verb + // added to the send half can never leak in here by editing one list. They are + // additionally hard-gated before this function is ever consulted + // (`isFleetSendTool` in Session#shouldAutoApprove), which is the invariant; + // this is defence in depth, not the fence. + for (const prefix of FLEET_TOOL_PREFIXES) { + if (name.startsWith(prefix)) { + return (FLEET_READ_TOOLS as readonly string[]).includes(name.slice(prefix.length)); + } + } return false; } diff --git a/src/tests/tool-safety.test.ts b/src/tests/tool-safety.test.ts index 4bb8746..0a8f46b 100644 --- a/src/tests/tool-safety.test.ts +++ b/src/tests/tool-safety.test.ts @@ -1,4 +1,5 @@ import { describe, test, expect } from "bun:test"; +import { FLEET_READ_TOOLS, FLEET_SEND_TOOLS } from "../protocol/types.js"; import { isElicitationTool, isSafeTool } from "../daemon/providers/tool-safety.js"; import { MEMORY_TOOL_NAMES } from "../daemon/memory/tools.js"; @@ -50,3 +51,41 @@ describe("isElicitationTool", () => { } }); }); + +describe("isSafeTool — the conductor's fleet mount", () => { + // The fleet READ surface is specified to run silently (conductor-design §3): + // "fleet_list / fleet_find / fleet_summary / fleet_recall / fleet_tasks / + // machine_map run silently". They did not — `isSafeTool` knew the memory and + // blackboard mounts but not the fleet, so in guarded mode every `fleet_find` + // raised an approval prompt. An assistant that asks permission to look + // something up is not an assistant. + + test("every read verb runs unprompted, under both namespacings", () => { + for (const verb of FLEET_READ_TOOLS) { + expect(isSafeTool(`mcp__codeoid_fleet__${verb}`)).toBe(true); + expect(isSafeTool(`codeoid_fleet__${verb}`)).toBe(true); + } + }); + + test("NO send-class verb is ever safe", () => { + // The one that must never regress. These are hard-gated earlier too + // (isFleetSendTool, before any mode logic), so this is the second fence. + for (const verb of FLEET_SEND_TOOLS) { + expect(isSafeTool(`mcp__codeoid_fleet__${verb}`)).toBe(false); + expect(isSafeTool(`codeoid_fleet__${verb}`)).toBe(false); + } + }); + + test("an unknown verb on the fleet prefix prompts rather than auto-approving", () => { + expect(isSafeTool("mcp__codeoid_fleet__fleet_detonate")).toBe(false); + expect(isSafeTool("mcp__codeoid_fleet__")).toBe(false); + }); + + test("a look-alike server segment does not inherit fleet safety", () => { + // Matching on the server segment alone would let these through; the prefix + // must match exactly, then the suffix must be a known read verb. + expect(isSafeTool("mcp__evil_codeoid_fleet__fleet_find")).toBe(false); + expect(isSafeTool("x_codeoid_fleet__fleet_find")).toBe(false); + expect(isSafeTool("mcp__codeoid_fleet_x__fleet_find")).toBe(false); + }); +}); From 37fd8a0155ee235845a7678a216b6609ac2e910b Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sun, 6 Sep 2026 15:49:57 +0800 Subject: [PATCH 2/3] =?UTF-8?q?feat(web):=20Conductor=20=E2=87=84=20Sessio?= =?UTF-8?q?ns=20toggle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes P5.1 (docs/conductor-frontends-design.md §3.A). Two co-equal top-level homes with one control in the status bar. §3 is the constraint the whole feature rests on: the conductor is a LENS over the same sessions, never a wall, and there must be no state a user can get stuck in. So this is a navigation preference, not a mode — switching home changes which session you are looking at and nothing else, and every session stays reachable from the list in both homes. **Sessions stays the default, even once a conductor exists.** Silently relocating someone's home the first time they spawn a conductor is exactly the "trapped in an orchestrated mode" feeling §3 exists to prevent, and a user who wants the conductor is one click away — then remembered. This settles the "default home" question left open in §13. The resolution rules live in `lib/home.ts` as pure functions, since which session a home lands on is the decision worth testing: - Conductor home focuses the conductor, or does NOTHING when there is none — a normal state, not an error. `null` means "leave focus alone", deliberately distinct from "focus nothing", so re-selecting a home you are already on does not reset scroll for nothing. - Sessions home acts only when you are actually sitting on the conductor. Otherwise you are already somewhere in Sessions and moving you would be the surprise this design avoids. - It returns you to the session you came from, falling back to the first ordinary session when that one was destroyed. Workers are never a landing target, on either path. They are disposable and die with their task, so landing on one is landing somewhere about to disappear — and a worker can legitimately be the last thing you looked at, having drilled into it from the fleet rail. A test caught that: the remembered-session path originally excluded only conductors. Acting on the choice is an effect rather than click handling, so the two stay consistent when the population changes underneath — the conductor being created while Conductor home is already selected, for instance. The preference persists via the existing layout store, validated on read rather than cast: a stored value from a future build must fall back, not select a home that does not exist. The remembered session is deliberately NOT persisted — it is a within-visit convenience, and an id from days ago is likelier to name a destroyed session than to be useful. 11 home tests; 210 web tests; typecheck, lint and build clean. Co-Authored-By: Claude Opus 5 (1M context) --- web/src/components/HomeToggle.tsx | 109 ++++++++++++++++++++++++++++++ web/src/components/StatusBar.tsx | 5 ++ web/src/lib/home.test.ts | 83 +++++++++++++++++++++++ web/src/lib/home.ts | 82 ++++++++++++++++++++++ web/src/state/layout.ts | 21 ++++++ 5 files changed, 300 insertions(+) create mode 100644 web/src/components/HomeToggle.tsx create mode 100644 web/src/lib/home.test.ts create mode 100644 web/src/lib/home.ts diff --git a/web/src/components/HomeToggle.tsx b/web/src/components/HomeToggle.tsx new file mode 100644 index 0000000..5f04ec2 --- /dev/null +++ b/web/src/components/HomeToggle.tsx @@ -0,0 +1,109 @@ +/** + * The Conductor ⇄ Sessions toggle (conductor-frontends-design §3.A). + * + * Two co-equal homes, one control. Neither is modal — this changes which + * session you are looking at and nothing else, and every session stays + * reachable from the list in both homes. That is §3's load-bearing constraint: + * the conductor is a lens over the same sessions, never a wall, and there must + * be no state a user can get stuck in. + * + * The resolution rules live in `lib/home.ts`; this is the control plus the one + * effect that acts on the choice. + */ + +import { Component, createEffect, createMemo, createSignal } from "solid-js"; + +import { findConductor, homeTarget, type Home } from "../lib/home"; +import { activeHome, setHome } from "../state/layout"; +import { focusedSessionId, focusSession, sessionList } from "../state/sessions"; + +/** + * The last ordinary session focused before switching to the conductor, so + * switching back returns you to your work rather than an arbitrary first row. + * + * Module-level, not persisted: it is a within-visit convenience, and a + * remembered id from days ago is more likely to name a destroyed session than + * to be useful. + */ +const [lastSessionId, setLastSessionId] = createSignal(null); + +const HomeToggle: Component = () => { + const conductor = createMemo(() => findConductor(sessionList())); + + // Track where the user was in Sessions, so Conductor → Sessions can return + // them there. Recorded on every focus change that is not the conductor. + createEffect(() => { + const id = focusedSessionId(); + if (!id) return; + const s = sessionList().find((x) => x.id === id); + if (s && s.role === undefined) setLastSessionId(id); + }); + + // Acting on the choice is an effect rather than click handling, so the two + // stay consistent when the population changes underneath — e.g. the conductor + // is created while Conductor home is already selected. + createEffect(() => { + const target = homeTarget( + sessionList(), + activeHome(), + focusedSessionId() ?? null, + lastSessionId(), + ); + if (target) focusSession(target); + }); + + return ( +
+ + +
+ ); +}; + +const HomeButton: Component<{ + home: Home; + label: string; + title: string; + muted?: boolean; +}> = (props) => { + const active = () => activeHome() === props.home; + return ( + + ); +}; + +/** Reset the remembered session — for tests. */ +export function _resetHomeMemoryForTest(): void { + setLastSessionId(null); +} + +export default HomeToggle; diff --git a/web/src/components/StatusBar.tsx b/web/src/components/StatusBar.tsx index 8cfcc6e..990b768 100644 --- a/web/src/components/StatusBar.tsx +++ b/web/src/components/StatusBar.tsx @@ -10,6 +10,8 @@ import { Component, Show } from "solid-js"; +import HomeToggle from "./HomeToggle"; + import { ctxWindowColorClass, elapsedSince, @@ -44,6 +46,9 @@ const StatusBar: Component = () => { codeoid + {/* Two co-equal homes, always both available (§3.A) — never a mode. */} + + diff --git a/web/src/lib/home.test.ts b/web/src/lib/home.test.ts new file mode 100644 index 0000000..565cb50 --- /dev/null +++ b/web/src/lib/home.test.ts @@ -0,0 +1,83 @@ +import { describe, it, expect } from "vitest"; + +import { DEFAULT_HOME, findConductor, homeTarget, isHome } from "./home"; +import type { SessionInfo } from "../protocol/types"; + +const s = (id: string, role?: "conductor" | "worker"): SessionInfo => + ({ id, name: id, ...(role ? { role } : {}) }) as SessionInfo; + +const CONDUCTOR = s("cond", "conductor"); +const WORKER = s("worker-scout-a", "worker"); +const WORK = s("api"); +const OTHER = s("web"); + +describe("isHome / DEFAULT_HOME", () => { + it("defaults to Sessions even once a conductor exists", () => { + // Silently relocating someone's home the first time they spawn a conductor + // is the "trapped in an orchestrated mode" feeling §3 exists to prevent. + expect(DEFAULT_HOME).toBe("sessions"); + }); + + it("rejects anything that is not a home, so stored junk falls back", () => { + expect(isHome("conductor")).toBe(true); + expect(isHome("sessions")).toBe(true); + expect(isHome("fleet")).toBe(false); + expect(isHome(undefined)).toBe(false); + expect(isHome(null)).toBe(false); + }); +}); + +describe("findConductor", () => { + it("finds it, and reports null rather than guessing when absent", () => { + expect(findConductor([WORK, CONDUCTOR, WORKER])?.id).toBe("cond"); + expect(findConductor([WORK, WORKER])).toBeNull(); + }); +}); + +describe("homeTarget — Conductor home", () => { + const all = [WORK, CONDUCTOR, WORKER]; + + it("focuses the conductor", () => { + expect(homeTarget(all, "conductor", "api", null)).toBe("cond"); + }); + + it("leaves focus alone when already on the conductor", () => { + // null means "don't touch it" — re-focusing would reset scroll for nothing. + expect(homeTarget(all, "conductor", "cond", null)).toBeNull(); + }); + + it("leaves focus alone when no conductor exists yet", () => { + // A normal state, not an error: the toggle offers to create one. + expect(homeTarget([WORK, OTHER], "conductor", "api", null)).toBeNull(); + }); +}); + +describe("homeTarget — Sessions home", () => { + const all = [WORK, OTHER, CONDUCTOR, WORKER]; + + it("returns to the session you came from", () => { + expect(homeTarget(all, "sessions", "cond", "web")).toBe("web"); + }); + + it("does nothing when you are not on the conductor", () => { + // You are already somewhere in Sessions; moving you would be the surprise + // this design avoids. + expect(homeTarget(all, "sessions", "api", "web")).toBeNull(); + }); + + it("falls back to an ordinary session when the remembered one is gone", () => { + expect(homeTarget(all, "sessions", "cond", "destroyed")).toBe("api"); + expect(homeTarget(all, "sessions", "cond", null)).toBe("api"); + }); + + it("never falls back onto a worker", () => { + // Workers are disposable and die with their task — landing on one is + // landing somewhere that is about to disappear. + expect(homeTarget([CONDUCTOR, WORKER], "sessions", "cond", null)).toBeNull(); + expect(homeTarget([CONDUCTOR, WORKER], "sessions", "cond", "worker-scout-a")).toBeNull(); + }); + + it("leaves focus alone when the conductor is the only session", () => { + expect(homeTarget([CONDUCTOR], "sessions", "cond", null)).toBeNull(); + }); +}); diff --git a/web/src/lib/home.ts b/web/src/lib/home.ts new file mode 100644 index 0000000..4730180 --- /dev/null +++ b/web/src/lib/home.ts @@ -0,0 +1,82 @@ +/** + * The two top-level homes — Conductor and Sessions (conductor-frontends-design + * §3.A) — and the rule for what each one focuses. + * + * §3 is the constraint the whole feature rests on: the conductor is a LENS over + * the same sessions, never a wall. So this is a navigation preference, not a + * mode — switching home changes which session you are looking at and nothing + * else. Every session stays reachable from the list in both homes, and there is + * no state a user can get stuck in. + * + * Pure functions: which session a home resolves to is the decision worth + * testing, and it needs no reactive root. + */ + +import type { SessionInfo } from "../protocol/types"; + +export type Home = "sessions" | "conductor"; + +/** + * Default home for a user who has never chosen. + * + * Sessions, deliberately — even once a conductor exists. Silently relocating + * someone's home the first time they spawn a conductor is exactly the "trapped + * in an orchestrated mode" feeling §3 exists to prevent, and a user who wants + * the conductor is one click (and one remembered preference) away. + */ +export const DEFAULT_HOME: Home = "sessions"; + +export function isHome(v: unknown): v is Home { + return v === "sessions" || v === "conductor"; +} + +/** The tenant's conductor, or null when none has been created yet. */ +export function findConductor(sessions: readonly SessionInfo[]): SessionInfo | null { + return sessions.find((s) => s.role === "conductor") ?? null; +} + +/** + * Which session a home should focus. + * + * Returns null to mean "leave the focus alone" — a distinct outcome from "focus + * nothing", and the right answer whenever the home has no better candidate than + * whatever the user is already reading. + * + * `lastSessionId` is the session the user was on before switching to the + * conductor, so switching back returns them to their work rather than to an + * arbitrary first row. It is ignored when that session has since been + * destroyed. + */ +export function homeTarget( + sessions: readonly SessionInfo[], + home: Home, + currentId: string | null, + lastSessionId: string | null, +): string | null { + if (home === "conductor") { + const conductor = findConductor(sessions); + // No conductor yet is a normal state, not an error: the toggle offers to + // create one, and until then the current session stays put. + return conductor && conductor.id !== currentId ? conductor.id : null; + } + + // Sessions home. Only act when the user is actually sitting on the conductor + // — otherwise they are already somewhere in Sessions and moving them would be + // the surprise this design is trying to avoid. + const current = sessions.find((s) => s.id === currentId) ?? null; + if (current?.role !== "conductor") return null; + + // Ordinary sessions only, on BOTH paths. Workers are disposable and die with + // their task, so landing on one is landing somewhere about to disappear — + // and a worker can legitimately be the last thing you looked at, having + // drilled into it from the fleet rail. + const isOrdinary = (s: SessionInfo): boolean => s.role === undefined; + + const remembered = lastSessionId + ? (sessions.find((s) => s.id === lastSessionId) ?? null) + : null; + if (remembered && isOrdinary(remembered)) return remembered.id; + + // Nothing remembered, or it was destroyed, or it was a worker. + return sessions.find(isOrdinary)?.id ?? null; +} diff --git a/web/src/state/layout.ts b/web/src/state/layout.ts index 116ea1d..8e32329 100644 --- a/web/src/state/layout.ts +++ b/web/src/state/layout.ts @@ -8,6 +8,8 @@ import { batch, createEffect, createSignal } from "solid-js"; +import { DEFAULT_HOME, isHome, type Home } from "../lib/home"; + const STORAGE_KEY = "codeoid.layout.v1"; interface LayoutState { @@ -16,6 +18,12 @@ interface LayoutState { rightPanePx: number; /** Session header collapse — when true, only a 1-line summary shows. */ headerCollapsed: boolean; + /** + * Which top-level home the user last chose (§3.A). A navigation preference, + * not a mode — see lib/home.ts. Persisted so the toggle is remembered rather + * than re-decided on every reload. + */ + home: Home; } const DEFAULTS: LayoutState = { @@ -23,6 +31,7 @@ const DEFAULTS: LayoutState = { leftSidebarCollapsed: false, rightPanePx: 576, // 36rem-ish headerCollapsed: false, + home: DEFAULT_HOME, }; const LIMITS = { @@ -56,6 +65,9 @@ function load(): LayoutState { typeof parsed.headerCollapsed === "boolean" ? parsed.headerCollapsed : DEFAULTS.headerCollapsed, + // Validated rather than cast: a stored value from a future build (or a + // hand-edited one) must fall back, not select a home that does not exist. + home: isHome(parsed.home) ? parsed.home : DEFAULTS.home, }; } catch { return DEFAULTS; @@ -72,6 +84,7 @@ const [rightPanePx, setRightPanePx] = createSignal(initial.rightPanePx); const [headerCollapsed, setHeaderCollapsedSig] = createSignal( initial.headerCollapsed, ); +const [home, setHomeSig] = createSignal(initial.home); /** Effective width for the left sidebar accounting for collapse. */ export function leftSidebarEffectivePx(): number { @@ -83,6 +96,13 @@ export const isLeftCollapsed = leftSidebarCollapsed; export const rightWidth = rightPanePx; export const isHeaderCollapsed = headerCollapsed; +/** The user's chosen top-level home (§3.A). */ +export const activeHome = home; + +export function setHome(next: Home): void { + setHomeSig(next); +} + // ── Mobile / narrow-viewport (Telegram Mini App) ────────────────────────── // Reactive viewport-width breakpoint. Below 768px the 3-pane grid is too @@ -144,6 +164,7 @@ createEffect(() => { leftSidebarCollapsed: leftSidebarCollapsed(), rightPanePx: rightPanePx(), headerCollapsed: headerCollapsed(), + home: home(), }; if (typeof localStorage === "undefined") return; if (persistTimer !== null) clearTimeout(persistTimer); From abfa776dfe5dde3e36cf639b0fbb24843e7e51ea Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sun, 6 Sep 2026 23:21:06 +0800 Subject: [PATCH 3/3] refactor(web): name the ordinary-session predicate, keep its fail-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on #329 (Oracle, both comments). The feedback asked for an explicit role taxonomy instead of `s.role === undefined`, suggesting `role !== "conductor" && role !== "worker"`. Taking the first half and declining the second. **Taken: name it.** The check was an inline lambda in `home.ts` and a duplicated inline test in `HomeToggle.tsx`. Both now call an exported `isOrdinarySession`, so the taxonomy has a name and one definition. **Declined: the negative form**, because it inverts the safety property. `SessionInfo.role` is documented as "Absent = normal session", and the protocol deliberately anticipates roles this client has not heard of — `session.create` types its role as an open string precisely "so a future role from a newer client still type-checks on the wire". The two forms therefore differ exactly when a new role appears: role === undefined → an unknown role is NOT ordinary (excluded) role !== "conductor" && ... → an unknown role IS ordinary (included) The negative form reads as more explicit and is the more dangerous of the two: it silently opts every future session kind into being a focus target. Workers are excluded here because they vanish with their task; inheriting that risk for kinds we know nothing about is the wrong default. An unknown role now stays excluded until somebody adds it here deliberately. Pinned by two tests — one on the predicate, one through `homeTarget` — that a session with an unrecognised role is never a landing target. 14 home tests; 213 web tests; typecheck, lint and build clean. Co-Authored-By: Claude Opus 5 (1M context) --- web/src/components/HomeToggle.tsx | 4 +-- web/src/lib/home.test.ts | 36 ++++++++++++++++++++++++- web/src/lib/home.ts | 44 ++++++++++++++++++++++++------- 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/web/src/components/HomeToggle.tsx b/web/src/components/HomeToggle.tsx index 5f04ec2..2b8afe8 100644 --- a/web/src/components/HomeToggle.tsx +++ b/web/src/components/HomeToggle.tsx @@ -13,7 +13,7 @@ import { Component, createEffect, createMemo, createSignal } from "solid-js"; -import { findConductor, homeTarget, type Home } from "../lib/home"; +import { findConductor, homeTarget, isOrdinarySession, type Home } from "../lib/home"; import { activeHome, setHome } from "../state/layout"; import { focusedSessionId, focusSession, sessionList } from "../state/sessions"; @@ -36,7 +36,7 @@ const HomeToggle: Component = () => { const id = focusedSessionId(); if (!id) return; const s = sessionList().find((x) => x.id === id); - if (s && s.role === undefined) setLastSessionId(id); + if (s && isOrdinarySession(s)) setLastSessionId(id); }); // Acting on the choice is an effect rather than click handling, so the two diff --git a/web/src/lib/home.test.ts b/web/src/lib/home.test.ts index 565cb50..67c6f46 100644 --- a/web/src/lib/home.test.ts +++ b/web/src/lib/home.test.ts @@ -1,6 +1,12 @@ import { describe, it, expect } from "vitest"; -import { DEFAULT_HOME, findConductor, homeTarget, isHome } from "./home"; +import { + DEFAULT_HOME, + findConductor, + homeTarget, + isHome, + isOrdinarySession, +} from "./home"; import type { SessionInfo } from "../protocol/types"; const s = (id: string, role?: "conductor" | "worker"): SessionInfo => @@ -34,6 +40,26 @@ describe("findConductor", () => { }); }); +describe("isOrdinarySession", () => { + it("accepts only a session with no role", () => { + expect(isOrdinarySession(WORK)).toBe(true); + expect(isOrdinarySession(CONDUCTOR)).toBe(false); + expect(isOrdinarySession(WORKER)).toBe(false); + }); + + it("EXCLUDES a role this client has never heard of", () => { + // The fail-safe, and the reason this is `role === undefined` rather than + // `role !== "conductor" && role !== "worker"`. The protocol deliberately + // allows roles a client does not know (session.create types role as an open + // string "so a future role from a newer client still type-checks"), and the + // negative form would silently opt every future kind into being a landing + // target. Workers are excluded because they vanish; inheriting that risk + // for kinds we know nothing about is the wrong default. + const future = { id: "x", name: "x", role: "sandbox" } as unknown as SessionInfo; + expect(isOrdinarySession(future)).toBe(false); + }); +}); + describe("homeTarget — Conductor home", () => { const all = [WORK, CONDUCTOR, WORKER]; @@ -80,4 +106,12 @@ describe("homeTarget — Sessions home", () => { it("leaves focus alone when the conductor is the only session", () => { expect(homeTarget([CONDUCTOR], "sessions", "cond", null)).toBeNull(); }); + + it("never lands on an unknown future role, remembered or not", () => { + // Same fail-safe as isOrdinarySession, asserted through the real entry + // point: a new session kind must not become a landing target for free. + const future = { id: "fut", name: "fut", role: "sandbox" } as unknown as SessionInfo; + expect(homeTarget([CONDUCTOR, future], "sessions", "cond", null)).toBeNull(); + expect(homeTarget([CONDUCTOR, future], "sessions", "cond", "fut")).toBeNull(); + }); }); diff --git a/web/src/lib/home.ts b/web/src/lib/home.ts index 4730180..fc66794 100644 --- a/web/src/lib/home.ts +++ b/web/src/lib/home.ts @@ -35,6 +35,34 @@ export function findConductor(sessions: readonly SessionInfo[]): SessionInfo | n return sessions.find((s) => s.role === "conductor") ?? null; } +/** + * An ordinary coding session — one you own and drive, as opposed to the + * conductor or a disposable dispatch worker. The only kind this module will + * ever move focus TO. + * + * Tested as "has no role" rather than "is not conductor and not worker", and + * the difference is a fail-safe, not a style choice. `SessionInfo.role` is + * documented as *"Absent = normal session"*, and the protocol deliberately + * anticipates roles this client has not heard of — `session.create` types its + * role as an open string precisely "so a future role from a newer client still + * type-checks on the wire". + * + * So the two forms differ exactly when a new role appears: + * + * role === undefined → an unknown role is NOT ordinary (excluded) + * role !== "conductor" && ... → an unknown role IS ordinary (included) + * + * The second reads as more explicit and is the more dangerous of the two: it + * silently opts every future session kind into being a landing target. Since + * the whole reason workers are excluded is "do not send someone to a session + * that is about to disappear", inheriting that risk for kinds we know nothing + * about is the wrong default. An unknown role stays excluded until somebody + * deliberately adds it here. + */ +export function isOrdinarySession(s: SessionInfo): boolean { + return s.role === undefined; +} + /** * Which session a home should focus. * @@ -66,17 +94,15 @@ export function homeTarget( const current = sessions.find((s) => s.id === currentId) ?? null; if (current?.role !== "conductor") return null; - // Ordinary sessions only, on BOTH paths. Workers are disposable and die with - // their task, so landing on one is landing somewhere about to disappear — - // and a worker can legitimately be the last thing you looked at, having - // drilled into it from the fleet rail. - const isOrdinary = (s: SessionInfo): boolean => s.role === undefined; - + // Ordinary sessions only, on BOTH paths — see isOrdinarySession. Workers are + // disposable and die with their task, so landing on one is landing somewhere + // about to disappear, and a worker can legitimately be the last thing you + // looked at, having drilled into it from the fleet rail. const remembered = lastSessionId ? (sessions.find((s) => s.id === lastSessionId) ?? null) : null; - if (remembered && isOrdinary(remembered)) return remembered.id; + if (remembered && isOrdinarySession(remembered)) return remembered.id; - // Nothing remembered, or it was destroyed, or it was a worker. - return sessions.find(isOrdinary)?.id ?? null; + // Nothing remembered, or it was destroyed, or it was not an ordinary session. + return sessions.find(isOrdinarySession)?.id ?? null; }