-
Notifications
You must be signed in to change notification settings - Fork 2
feat(web): Conductor ⇄ Sessions toggle #329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
82ab74d
37fd8a0
abfa776
e45e99f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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, isOrdinarySession, 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<string | null>(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 && isOrdinarySession(s)) 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. | ||
|
saucam marked this conversation as resolved.
|
||
| createEffect(() => { | ||
| const target = homeTarget( | ||
| sessionList(), | ||
| activeHome(), | ||
| focusedSessionId() ?? null, | ||
| lastSessionId(), | ||
| ); | ||
| if (target) focusSession(target); | ||
| }); | ||
|
|
||
| return ( | ||
| <div | ||
| class="flex items-center rounded border border-border bg-bg p-0.5" | ||
| role="group" | ||
| aria-label="Home" | ||
| > | ||
| <HomeButton home="sessions" label="Sessions" title="Your sessions — the classic list and cockpit" /> | ||
| <HomeButton | ||
| home="conductor" | ||
| label="Conductor" | ||
| title={ | ||
| conductor() | ||
| ? "The conductor — chat to it and watch the fleet" | ||
| : "No conductor yet — create one to route work across your sessions" | ||
| } | ||
| // Shown but inert-looking when there is none: the toggle should still | ||
| // say the feature exists rather than hiding it until it is used. | ||
| muted={!conductor()} | ||
| /> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| const HomeButton: Component<{ | ||
| home: Home; | ||
| label: string; | ||
| title: string; | ||
| muted?: boolean; | ||
| }> = (props) => { | ||
| const active = () => activeHome() === props.home; | ||
| return ( | ||
| <button | ||
| type="button" | ||
| onClick={() => setHome(props.home)} | ||
| title={props.title} | ||
| aria-pressed={active()} | ||
| class={`rounded px-2 py-0.5 text-[11px] font-medium transition ${ | ||
| active() | ||
| ? "bg-accent/15 text-accent" | ||
| : props.muted | ||
| ? "text-fg-faint hover:text-fg-muted" | ||
| : "text-fg-muted hover:text-fg" | ||
| }`} | ||
| > | ||
| {props.label} | ||
| </button> | ||
| ); | ||
| }; | ||
|
|
||
| /** Reset the remembered session — for tests. */ | ||
| export function _resetHomeMemoryForTest(): void { | ||
| setLastSessionId(null); | ||
| } | ||
|
|
||
| export default HomeToggle; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| import { describe, it, expect } from "vitest"; | ||
|
|
||
| import { | ||
| DEFAULT_HOME, | ||
| findConductor, | ||
| homeTarget, | ||
| isHome, | ||
| isOrdinarySession, | ||
| } 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("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]; | ||
|
|
||
| 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(); | ||
| }); | ||
|
|
||
| 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(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| /** | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cross-file issue: Ensure centralized resolution logic is used by Fleet Rail The PR introduces Affected files: Recommendation: Audit the Fleet Rail component (and other entry points) to ensure they delegate navigation resolution to |
||
| * 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; | ||
| } | ||
|
|
||
| /** | ||
| * 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. | ||
| * | ||
| * 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 — 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 && isOrdinarySession(remembered)) return remembered.id; | ||
|
|
||
| // Nothing remembered, or it was destroyed, or it was not an ordinary session. | ||
| return sessions.find(isOrdinarySession)?.id ?? null; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cross-file issue: Verify framework compliance (React vs SolidJS patterns)
The provided Organization Standards explicitly mandate 'Framework: React 18+ / Next.js 14+'. However, the File Review Summary for
HomeToggle.tsxstates that the component follows 'SolidJS patterns'. If thewebapplication is a standard React codebase, this suggests potential architectural drift or the introduction of patterns that violate the established stack standards.Affected files:
web/src/components/HomeToggle.tsxRecommendation: Confirm whether 'SolidJS patterns' refers to the SolidJS framework or 'solid' design principles. If SolidJS framework dependencies or patterns (e.g., signals, fine-grained reactivity) are being introduced into a React app, evaluate if this is a deliberate architectural pivot or a mistake.