From 93f2bc528182d7e7349d8707487f037f343b72a4 Mon Sep 17 00:00:00 2001 From: Yuto Nishida Date: Wed, 12 Aug 2026 18:34:35 -0700 Subject: [PATCH] [papyrus] Scope session-tab optimism per workstream; plain-shell UI Fixes the bug where renaming a plain shell made its tab appear in every workstream. Optimistic tab overrides are now keyed per workstream (pendingByNode) and merged through mergePendingTabs, so a lingering override can no longer fabricate a phantom tab in another workstream. Also wires the plain-shell UI: the kind token, a close (x) affordance backed by the DELETE route, and a shell-aware disconnected screen. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/components/NewSessionMenu.test.tsx | 4 +- .../client/src/components/NewSessionMenu.tsx | 6 +- .../papyrus/client/src/components/Sidebar.tsx | 109 +++++++++++++----- .../src/components/sessionOptimism.test.ts | 47 +++++++- .../client/src/components/sessionOptimism.ts | 28 +++++ 5 files changed, 159 insertions(+), 35 deletions(-) diff --git a/experimental/papyrus/client/src/components/NewSessionMenu.test.tsx b/experimental/papyrus/client/src/components/NewSessionMenu.test.tsx index 92b066a..f755aab 100644 --- a/experimental/papyrus/client/src/components/NewSessionMenu.test.tsx +++ b/experimental/papyrus/client/src/components/NewSessionMenu.test.tsx @@ -53,10 +53,10 @@ describe("NewSessionMenu", () => { expect(onClose).toHaveBeenCalled(); }); - test("picking Plain shell fires onPick('shell') then onClose", () => { + test("picking Plain shell fires onPick('plain-shell') then onClose", () => { const { onPick, onClose } = renderMenu(); fireEvent.click(screen.getByText("Plain shell")); - expect(onPick).toHaveBeenCalledWith("shell"); + expect(onPick).toHaveBeenCalledWith("plain-shell"); expect(onClose).toHaveBeenCalled(); }); }); diff --git a/experimental/papyrus/client/src/components/NewSessionMenu.tsx b/experimental/papyrus/client/src/components/NewSessionMenu.tsx index 1952461..7ba3d41 100644 --- a/experimental/papyrus/client/src/components/NewSessionMenu.tsx +++ b/experimental/papyrus/client/src/components/NewSessionMenu.tsx @@ -27,7 +27,7 @@ const iconMap: Record = { }; // One row in the picker. `variant` is the string POSTed to the server; the agent -// variant(s) map to the durable "claude-code" session, "shell" to the ephemeral +// variant(s) map to the durable "claude-code" session, "plain-shell" to a durable // `silverwood spawn` login shell. interface VariantItem { variant: string; @@ -83,9 +83,9 @@ export function NewSessionMenu({ open, anchor, agents, onClose, onPick }: NewSes color: a.color, })), { - variant: "shell", + variant: "plain-shell", label: "Plain shell", - description: "Ephemeral login shell (silverwood spawn)", + description: "Login shell (silverwood spawn)", icon: "terminal", }, ]; diff --git a/experimental/papyrus/client/src/components/Sidebar.tsx b/experimental/papyrus/client/src/components/Sidebar.tsx index be2fe55..1d450c9 100644 --- a/experimental/papyrus/client/src/components/Sidebar.tsx +++ b/experimental/papyrus/client/src/components/Sidebar.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useRef } from "react"; +import { useState, useEffect, useRef, useCallback } from "react"; import { motion, AnimatePresence } from "framer-motion"; import { X, @@ -24,7 +24,7 @@ import { } from "lucide-react"; import { useStore, SessionTab } from "../stores/useStore"; import { AgentIcon } from "./AgentIcon"; -import { type PendingOptimism, shouldDropOptimism } from "./sessionOptimism"; +import { type PendingOptimism, shouldDropOptimism, mergePendingTabs } from "./sessionOptimism"; import { Terminal } from "./Terminal"; import { NewSessionMenu } from "./NewSessionMenu"; import { useResizablePane } from "./useResizablePane"; @@ -86,10 +86,31 @@ export function Sidebar() { const [editingSessionId, setEditingSessionId] = useState(null); const [editSessionName, setEditSessionName] = useState(""); // Optimism: an action's server-confirmed result, shown until the ~1s reconcile - // reflects it (mount-from-response). { sessionId -> partial tab override }. - // Optimistic tab overrides, each stamped with the reconcile generation it was set at (so a - // `connected` optimism can be retired even if the server never echoes it — the fast-fail race). - const [pending, setPending] = useState>({}); + // reflects it (mount-from-response). Each override is stamped with the reconcile + // generation it was set at (so a `connected` optimism can be retired even if the + // server never echoes it — the fast-fail race). **Scoped per workstream** (node id + // → sessionId → override): an override only applies to the workstream it was made + // in, so switching workstreams can never surface a phantom tab in another one. + const [pendingByNode, setPendingByNode] = useState< + Record> + >({}); + const pending = selectedNodeId ? (pendingByNode[selectedNodeId] ?? {}) : {}; + // Update the selected workstream's optimism bucket (no-op with nothing selected). + // Keeps the historic `setPending((prev) => next)` shape at every call site. + const setPending = useCallback( + ( + updater: ( + prev: Record, + ) => Record, + ) => { + if (!selectedNodeId) return; + setPendingByNode((byNode) => ({ + ...byNode, + [selectedNodeId]: updater(byNode[selectedNodeId] ?? {}), + })); + }, + [selectedNodeId], + ); const reconcileSeqRef = useRef(0); const [connectError, setConnectError] = useState(null); const [busy, setBusy] = useState(false); @@ -98,25 +119,9 @@ export function Sidebar() { const storeTabs: SessionTab[] = session?.tabs ?? []; - // The tab list: the server projection, with local optimism merged over it. - const tabs: SessionTab[] = (() => { - const byId = new Map(storeTabs.map((t) => [t.sessionId, { ...t }])); - for (const [sid, { ov }] of Object.entries(pending)) { - const ex = byId.get(sid); - if (ex) byId.set(sid, { ...ex, ...ov }); - else - byId.set(sid, { - sessionId: sid, - name: "claude", - createdAt: new Date().toISOString(), - kind: "claude-code", - connected: true, - lock: null, - ...ov, - }); - } - return [...byId.values()]; - })(); + // The tab list: the server projection, with THIS workstream's optimism merged over + // it (`pending` is already scoped to the selected node — see `pendingByNode`). + const tabs: SessionTab[] = mergePendingTabs(storeTabs, pending); // Reset node-level edit buffers when the selected workstream changes. useEffect(() => { @@ -210,7 +215,7 @@ export function Sidebar() { } }; - // Start a fresh session of `variant` ("claude-code" | "shell") in this workstream. + // Start a fresh session of `variant` ("claude-code" | "plain-shell") in this workstream. const startSession = async (variant: string) => { setMenuAnchor(null); if (!selectedNodeId) return; @@ -227,11 +232,11 @@ export function Sidebar() { setConnectError(data.error || "Failed to start session"); return; } - const shell = variant === "shell"; + const shell = variant === "plain-shell"; setPending((p) => ({ ...p, [data.sessionId]: { - ov: { connected: true, ...(shell ? { name: "shell", kind: "shell" } : {}) }, + ov: { connected: true, ...(shell ? { name: "shell", kind: "plain-shell" } : {}) }, seq: reconcileSeqRef.current, }, })); @@ -259,6 +264,26 @@ export function Sidebar() { } }; + // Remove a session for good: kill any live PTY and delete its silverwood record. + // This is how a plain shell is closed (doctor can't retire a conversation-less + // shell). The tab vanishes on the next reconcile; drop its optimism now. + const removeSession = async (tab: SessionTab) => { + if (!selectedNodeId) return; + setPending((p) => { + const n = { ...p }; + delete n[tab.sessionId]; + return n; + }); + if (editingSessionId === tab.sessionId) setEditingSessionId(null); + try { + await fetch(`/api/sessions/${selectedNodeId}/sessions/${tab.sessionId}`, { + method: "DELETE", + }); + } catch { + // The reconcile loop will reflect the real state. + } + }; + // Doctor + conditionally delete a session that failed to resume. The server runs // `silverwood session doctor` and only removes the session if no conversation exists // on disk; a removed tab vanishes on the next reconcile. @@ -541,6 +566,19 @@ export function Sidebar() { > + {/* Close: only plain shells (a shell has no doctor-based removal). */} + {t.kind === "plain-shell" && ( + + )} ); })} @@ -645,7 +683,9 @@ export function Sidebar() {

{tabLabel(activeTab)}

- Disconnected — connecting resumes this Claude Code conversation. + {activeTab.kind === "plain-shell" + ? "Disconnected — connecting opens a new shell in the checkout." + : "Disconnected — connecting resumes this Claude Code conversation."}

{activeTab.lock && !activeTab.lock.mine ? ( @@ -690,6 +730,17 @@ export function Sidebar() { )} + {activeTab.kind === "plain-shell" && ( + + )} {connectError &&

{connectError}

} diff --git a/experimental/papyrus/client/src/components/sessionOptimism.test.ts b/experimental/papyrus/client/src/components/sessionOptimism.test.ts index 32556c2..7cdcd8e 100644 --- a/experimental/papyrus/client/src/components/sessionOptimism.test.ts +++ b/experimental/papyrus/client/src/components/sessionOptimism.test.ts @@ -1,5 +1,9 @@ import { describe, test, expect } from "bun:test"; -import { type PendingOptimism, shouldDropOptimism } from "./sessionOptimism"; +import { + type PendingOptimism, + shouldDropOptimism, + mergePendingTabs, +} from "./sessionOptimism"; import type { SessionTab } from "../stores/useStore"; const tab = (over: Partial = {}): SessionTab => ({ @@ -37,3 +41,44 @@ describe("shouldDropOptimism", () => { expect(shouldDropOptimism(opt({ connected: true }, 5), undefined, 99)).toBe(false); }); }); + +describe("mergePendingTabs", () => { + test("override on an existing tab is merged over it", () => { + const tabs = mergePendingTabs([tab({ sessionId: "s1", name: "old" })], { + s1: opt({ name: "new" }), + }); + expect(tabs).toHaveLength(1); + expect(tabs[0].name).toBe("new"); + }); + + test("override with no matching tab fabricates a tab (just-started session)", () => { + const tabs = mergePendingTabs([], { + s9: opt({ connected: true, name: "shell", kind: "plain-shell" }), + }); + expect(tabs).toHaveLength(1); + expect(tabs[0]).toMatchObject({ sessionId: "s9", name: "shell", kind: "plain-shell" }); + }); + + // The bug this fixes: a rename override made in workstream A must not surface a + // phantom tab in workstream B. Callers pass only the selected node's bucket, so + // building B's tabs sees an empty `pending` and fabricates nothing. + test("optimism scoped per workstream does not leak across workstreams", () => { + const pendingByNode: Record> = { + A: { shSid: opt({ name: "my shell", kind: "plain-shell" }) }, + }; + + // Workstream A (which owns the shell) shows the renamed tab. + const tabsA = mergePendingTabs( + [tab({ sessionId: "shSid", name: "shell", kind: "plain-shell" })], + pendingByNode["A"] ?? {}, + ); + expect(tabsA.find((t) => t.sessionId === "shSid")?.name).toBe("my shell"); + + // Workstream B, selected next, gets an empty bucket → no phantom shell tab. + const tabsB = mergePendingTabs( + [tab({ sessionId: "bTab", name: "claude" })], + pendingByNode["B"] ?? {}, + ); + expect(tabsB.map((t) => t.sessionId)).toEqual(["bTab"]); + }); +}); diff --git a/experimental/papyrus/client/src/components/sessionOptimism.ts b/experimental/papyrus/client/src/components/sessionOptimism.ts index 8cd23d7..88c45a3 100644 --- a/experimental/papyrus/client/src/components/sessionOptimism.ts +++ b/experimental/papyrus/client/src/components/sessionOptimism.ts @@ -30,3 +30,31 @@ export function shouldDropOptimism( const backstop = ov.connected !== undefined && seqNow - seq >= 2; return confirmed || backstop; } + +// The tab list a workstream shows: its server projection (`storeTabs`) with local +// optimism merged over it. `pending` must be ONLY the selected workstream's overrides +// (sessionId → override) — callers scope it per node id — because the `else` branch +// fabricates a tab for an override whose id isn't in `storeTabs` (so a just-started +// session shows immediately). A cross-workstream override would otherwise materialize +// as a phantom tab here; scoping `pending` per workstream prevents that. +export function mergePendingTabs( + storeTabs: SessionTab[], + pending: Record, +): SessionTab[] { + const byId = new Map(storeTabs.map((t) => [t.sessionId, { ...t }])); + for (const [sid, { ov }] of Object.entries(pending)) { + const ex = byId.get(sid); + if (ex) byId.set(sid, { ...ex, ...ov }); + else + byId.set(sid, { + sessionId: sid, + name: "claude", + createdAt: new Date().toISOString(), + kind: "claude-code", + connected: true, + lock: null, + ...ov, + }); + } + return [...byId.values()]; +} -- 2.54.0