diff --git a/.fork/customizations.yaml b/.fork/customizations.yaml index dcecbc01a89..c234484aa5d 100644 --- a/.fork/customizations.yaml +++ b/.fork/customizations.yaml @@ -370,6 +370,89 @@ - apps/web/src/__fork_guards__/sidebarV2Rain.test.ts - apps/web/src/__fork_guards__/sidebarV2CardRows.test.ts +- id: sidebar-v2-dev-server-pulse + intent: > + The thread card's branch/worktree mark pulses working-green to foreground + while the port scanner attributes a listening server to that thread, so + the row whose checkout the running server is actually serving can be + picked out of the sidebar at a glance. The row's tooltip names what the + mark can only signal — `localhost:` for local threads, `port ` + for remote ones, plus a `(+n)` overflow — so the pulse is never the only + way to learn the port. The host split is load-bearing: a remote thread's + listener is on the remote host, and unlike v1's Globe (a button routed + through openDiscoveredPort to the right environment, where "localhost" is + loose phrasing around a correct action) this label is inert text, so it + must not name a machine the port is not on. + + Attribution is the port scanner's existing terminal→thread mapping and + nothing more: a listener counts for a row only when the scanner's + subprocess inspection traced it to one of the thread's own T3 terminals + (`useThreadDiscoveredPorts`, `port.terminal.threadId`). A server started in + an external shell arrives with `terminal: null` — the scanner knows its pid + but not its cwd — and must light up nothing rather than a guessed row. The + fuller feature (resolving the pid's cwd server-side and matching it against + thread.worktreePath) is deliberately out of scope: it needs an apps/server + + contracts change, recorded here so a future pass knows the seam. + + Second known limitation, broader than the first: when the scanner's + process probe fails (`lsof` absent, its 5s timeout blown, any + ProcessRunError), PortScanner falls back to probing COMMON_DEV_PORTS and + every entry it yields carries `pid: null, terminal: null` — attribution is + impossible in that mode, so no row can ever pulse, and nothing surfaces + the degradation; it is indistinguishable from "no servers running". + Relatedly, the lsof output is truncated at 1MiB, so on a host with very + many listeners a port can drop out of one snapshot and return the next, + which this mark would render as a flicker. Both are scanner behaviours, + not introduced here — recorded because this is the first UI that turns + them into peripheral-vision motion. + + Naming is deliberately restrained for the same reason: the scanner keeps + every listening TCP socket — no port range, no process-name filter, no + HTTP probe — so a debugger port, an ssh tunnel, or a database all light + the mark. The accessible text therefore says "Server listening on port N" + (what is actually known), never "dev server running", matching upstream's + own restraint ("Open localhost:5173"). + + Cost: a card row subscribes to its thread's environment discovered-servers + stream, which retains the server-side port scanner (3s lsof poll, + reference-counted) while subscribed. Stated precisely, because v2 is an + opt-in replacement for v1, not a neighbour: enabling v2 adds this standing + cost outright for its users — a v2 session with the sidebar open runs the + lsof poll for the whole session even if the preview pane is never opened, + where before this change nothing retained the scanner. It is the same + cost every v1 user already pays (Sidebar.tsx calls the identical hook per + row, unconditionally), which is the precedent for accepting it — not + evidence that nothing changed. Subscriptions key on thread.environmentId, + so a sidebar listing threads across M environments holds M streams and + every one of those hosts runs the 3s poll for the session — worth naming + because it leaves the machine. Slim (settled/snoozed) rows never draw the + card meta, so they pass null and skip the subscription entirely rather + than retaining the scanner for rows that cannot pulse. + + Green is `--sidebar-v2-status-working` — one hue for "alive" across the + card — and the far pole is `--foreground`, not literal white, so light + mode pulses toward ink instead of vanishing. A full breath is 8s (4s + alternate): ambient status, not an alert — notification-shaped motion in + this app means "blocked on you", which a healthy dev server is the + opposite of. Both marks are currentColor, so animating `color` on the svg + is the entire mechanism; `color` does not composite, so each live row + repaints its 12px glyph every frame — accepted deliberately, same as the + pixel rain, with content-visibility sparing offscreen rows. The state also + rides in `sr-only` text after the branch name (identity before transient + state) because the motion is invisible to a screen reader, and + `prefers-reduced-motion` drops the animation onto a static working-green + mark rather than dropping the state. + tier: 4 + files: + - apps/web/src/custom/SidebarV2ThreadCardMeta.tsx + - apps/web/src/theme.custom.css + shadows: [] + watch: + - apps/web/src/components/SidebarV2.tsx + - apps/web/src/portDiscoveryState.ts + verify: + - apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts + - id: sidebar-v2-project-grouping intent: > Sidebar V2's active cards can be grouped under project headers, from a diff --git a/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts b/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts new file mode 100644 index 00000000000..e5c30f9fe9f --- /dev/null +++ b/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts @@ -0,0 +1,186 @@ +// @effect-diagnostics nodeBuiltinImport:off +/** + * Fork guard — see `.fork/README.md` §4b and + * `.fork/customizations.yaml#sidebar-v2-dev-server-pulse`. + * + * The thread card's branch/worktree mark pulses working-green to foreground + * while the port scanner attributes a listening server to one of the thread's + * own T3 terminals. Three pieces have to stay joined — the fenced subscription + * in upstream's `SidebarV2.tsx`, the gate in the fork-owned meta component, + * and the animation in the Tier 1 stylesheet — and a sync can drop any one + * while the other two keep compiling. + * + * Assertion style, learned the hard way (PR #32 review replayed the first + * revision of this file against mutated sources and all eight assertions + * passed while the feature was dead, inverted, or always-on): + * + * - Source assertions run against the extracted `fork:begin`/`fork:end` + * hunks, never the whole file, and pin the *expressions* — the subscription + * input gate and the derived port — not the existence of a call. The + * `readMainHunk` precedent is `forkClerkLaunchResilience.test.ts`. + * - The meta component is asserted on its rendered output, so the attribute + * gate, the `> svg` DOM contract the stylesheet depends on, and the + * accessible text are pinned as outcomes a formatting pass cannot shake. + * - CSS is read through `cssRules.ts`, so declaration order and whitespace + * are free to change while scoping and content are not. + */ + +import * as NodeFS from "node:fs"; +import * as NodeURL from "node:url"; +import { createElement } from "react"; +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vite-plus/test"; + +import { SidebarV2ThreadCardMeta } from "../custom/SidebarV2ThreadCardMeta"; +import { cssRules } from "./cssRules"; + +function readSibling(relativePath: string): string { + return NodeFS.readFileSync(NodeURL.fileURLToPath(new URL(relativePath, import.meta.url)), "utf8"); +} + +const sidebarV2 = readSibling("../components/SidebarV2.tsx"); +const theme = readSibling("../theme.custom.css"); + +/** Every fenced hunk for this customization, concatenated, so assertions can + only match shipped code — never prose, comments elsewhere, or the manifest + quoting itself. */ +function readPulseHunks(): string { + const begin = "fork:begin sidebar-v2-dev-server-pulse"; + const end = "fork:end sidebar-v2-dev-server-pulse"; + const hunks: string[] = []; + let cursor = 0; + for (;;) { + const start = sidebarV2.indexOf(begin, cursor); + if (start === -1) break; + const stop = sidebarV2.indexOf(end, start); + if (stop === -1) throw new Error("unterminated sidebar-v2-dev-server-pulse hunk"); + hunks.push(sidebarV2.slice(start, stop)); + cursor = stop + end.length; + } + if (hunks.length === 0) throw new Error("no sidebar-v2-dev-server-pulse hunks in SidebarV2.tsx"); + return hunks.join("\n"); +} + +function renderMeta(over: { + readonly branch?: string | null; + readonly hasWorktree?: boolean; + readonly devServerPort?: number | null; +}): string { + return renderToStaticMarkup( + createElement(SidebarV2ThreadCardMeta, { + projectTitle: null, + branch: over.branch ?? null, + hasWorktree: over.hasWorktree ?? false, + devServerPort: over.devServerPort ?? null, + prSlot: null, + prUnknown: false, + insertions: null, + deletions: null, + modelLabel: null, + isRemote: false, + }), + ); +} + +describe("fork guard: sidebar-v2-dev-server-pulse", () => { + it("subscribes card rows to the thread's own attribution, and slim rows to nothing", () => { + const hunk = readPulseHunks(); + // The full input expressions, not the call's existence. `threadId: null` + // (dead feature), a dropped variant gate (slim rows retaining the + // scanner), or a hardcoded id all fail here, where "the call exists" + // passed for every one of them. + expect(hunk).toContain('environmentId: variant === "card" ? thread.environmentId : null,'); + expect(hunk).toContain('threadId: variant === "card" ? thread.id : null,'); + // The derived values, pinned as expressions: `= null` (never pulses) and + // `= 5173` (always pulses) both fail. + expect(hunk).toContain("const devServerPort = devServerPorts[0]?.port ?? null;"); + expect(hunk).toContain("devServerPort={devServerPort}"); + // The tooltip names what the mark can only signal — port plus overflow + // count — and receives it through the same fenced seam. Both host + // branches are pinned: a remote thread's listener is on the remote host, + // so "localhost" there would name the wrong machine (round-2 review #1), + // while a local thread keeps the copy-pasteable form. + expect(hunk).toContain("isRemote ? `port ${devServerPort}` : `localhost:${devServerPort}`"); + expect(hunk).toContain("devServerLabel={devServerLabel}"); + expect(hunk).toContain("{devServerLabel ? ("); + }); + + it("marks the slot only while a port is attributed, absent rather than false", () => { + // Rendered output, not source text: the stylesheet keys on attribute + // *presence*, and `data-…={false}` would still serialize an attribute and + // pulse every row. React omits `undefined` entirely, so the off state + // must render with no attribute at all. + const live = renderMeta({ branch: "fork/x", hasWorktree: true, devServerPort: 5173 }); + expect(live).toContain('data-fork-dev-server-live=""'); + const off = renderMeta({ branch: "fork/x", hasWorktree: true, devServerPort: null }); + expect(off).not.toContain("data-fork-dev-server-live"); + }); + + it("keeps the mark a direct child of the attributed slot, as `> svg` requires", () => { + // The stylesheet's `[data-fork-dev-server-live] > svg` is a DOM-structure + // contract; wrapping the icon (a tooltip trigger, a span) kills the pulse + // with every source-text assertion green. Pinned on the rendered tree: + // the svg follows the attribute with at most sr-only text between, for + // both marks the slot can draw. + const directChild = + /data-fork-dev-server-live="">(?:[^<]*<\/span>)* { + // "Server listening on port N", never "dev server": the scanner keeps + // every listening TCP socket (debuggers, tunnels, databases), so naming + // the port is the whole truthful claim. Ordered after the branch name so + // a screen reader hears what the row is before its transient state. + const live = renderMeta({ branch: "fork/x", hasWorktree: true, devServerPort: 5173 }); + expect(live).toContain("Server listening on port 5173"); + expect(live).not.toMatch(/dev server/iu); + expect(live.indexOf("Server listening on port 5173")).toBeGreaterThan(live.indexOf("fork/x")); + // A thread with neither branch nor worktree draws no slot: no attribute, + // no orphaned announcement. + const slotless = renderMeta({ branch: null, hasWorktree: false, devServerPort: 5173 }); + expect(slotless).not.toContain("data-fork-dev-server-live"); + expect(slotless).not.toContain("Server listening"); + }); + + it("animates the mark from the working green, scoped to the fork marker", () => { + const rules = cssRules(theme); + const selector = ':root[data-fork="noahhendrickson-t3code"] [data-fork-dev-server-live] > svg'; + // Declaration content, order-free: the first revision pinned `color:` one + // line above `animation:` and broke on a behaviour-identical swap. + const base = rules.find((rule) => rule.selector === selector && rule.atRules.length === 0); + expect(base).toBeDefined(); + expect(base?.body).toContain("animation: sidebar-v2-dev-server-pulse"); + // The static declaration is what reduced-motion falls back onto. + expect(base?.body).toContain("color: var(--sidebar-v2-status-working)"); + + const keyframeRules = rules.filter((rule) => + rule.atRules.some((at) => at === "@keyframes sidebar-v2-dev-server-pulse"), + ); + const from = keyframeRules.find((rule) => rule.selector === "from"); + const to = keyframeRules.find((rule) => rule.selector === "to"); + // Working green to foreground — not literal white, so light mode pulses + // toward ink instead of vanishing into the panel. + expect(from?.body).toContain("color: var(--sidebar-v2-status-working)"); + expect(to?.body).toContain("color: var(--foreground)"); + }); + + it("drops the motion, not the state, under prefers-reduced-motion", () => { + const rules = cssRules(theme); + const reduced = rules.find( + (rule) => + rule.selector === + ':root[data-fork="noahhendrickson-t3code"] [data-fork-dev-server-live] > svg' && + rule.atRules.some((at) => at.includes("prefers-reduced-motion: reduce")), + ); + // `animation: none` leaves the base rule's static green standing, so + // reduced-motion users keep the indicator and lose only the breathing. + expect(reduced).toBeDefined(); + expect(reduced?.body).toContain("animation: none"); + }); +}); diff --git a/apps/web/src/components/SidebarV2.tsx b/apps/web/src/components/SidebarV2.tsx index c6535dbd495..eec49f7c89d 100644 --- a/apps/web/src/components/SidebarV2.tsx +++ b/apps/web/src/components/SidebarV2.tsx @@ -28,6 +28,12 @@ import { Trash2Icon, Undo2Icon, } from "lucide-react"; +/* fork:begin sidebar-v2-dev-server-pulse — see .fork/customizations.yaml#sidebar-v2-dev-server-pulse */ +// A statement of its own rather than a name in upstream's list: the phosphor +// guard parses that list's braces, and a fence comment inside them reads as a +// binding. Out here the fence survives and the parser stays honest. +import { Globe2Icon } from "lucide-react"; +/* fork:end sidebar-v2-dev-server-pulse */ import { memo, useCallback, @@ -83,6 +89,9 @@ import { useHandleNewThread } from "../hooks/useHandleNewThread"; import { useScrollGutterWidth } from "~/custom/useScrollGutterWidth"; /* fork:end fork-sidebar-chrome */ import { openCommandPalette } from "../commandPaletteBus"; +/* fork:begin sidebar-v2-dev-server-pulse — see .fork/customizations.yaml#sidebar-v2-dev-server-pulse */ +import { useThreadDiscoveredPorts } from "../portDiscoveryState"; +/* fork:end sidebar-v2-dev-server-pulse */ import { /* fork:begin sidebar-v2-project-grouping — see .fork/customizations.yaml#sidebar-v2-project-grouping */ resolveThreadActionProjectRef, @@ -256,6 +265,9 @@ function SidebarV2ThreadTooltip({ modelInstanceId, modelLabel, branchMismatch, + /* fork:begin sidebar-v2-dev-server-pulse — see .fork/customizations.yaml#sidebar-v2-dev-server-pulse */ + devServerLabel, + /* fork:end sidebar-v2-dev-server-pulse */ }: { thread: SidebarThreadSummary; projectTitle: string | null; @@ -268,6 +280,12 @@ function SidebarV2ThreadTooltip({ threadBranch: string; currentBranch: string; } | null; + /* fork:begin sidebar-v2-dev-server-pulse — see .fork/customizations.yaml#sidebar-v2-dev-server-pulse */ + /** `localhost:` (+n) while the scanner attributes a listener to this + thread's terminals, or null. Names what the card's pulsing mark can only + signal — the next reader should not assume the boolean was all we had. */ + devServerLabel: string | null; + /* fork:end sidebar-v2-dev-server-pulse */ }) { return ( {thread.branch} ) : null} + {/* fork:begin sidebar-v2-dev-server-pulse — see .fork/customizations.yaml#sidebar-v2-dev-server-pulse */} + {devServerLabel ? ( +
+ +
{devServerLabel}
+
+ ) : null} + {/* fork:end sidebar-v2-dev-server-pulse */} {branchMismatch ? (
@@ -555,6 +581,39 @@ const SidebarV2Row = memo(function SidebarV2Row(props: { const isRemote = props.currentEnvironmentId !== null && thread.environmentId !== props.currentEnvironmentId; + /* fork:begin sidebar-v2-dev-server-pulse — see .fork/customizations.yaml#sidebar-v2-dev-server-pulse */ + // Attribution is the port scanner's existing terminal→thread mapping — the + // same per-row subscription upstream's v1 row already makes (Sidebar.tsx). + // A listener counts for this row only when it was spawned inside one of the + // thread's own T3 terminals; a server started in an external shell has + // `terminal: null` (the scanner knows its pid but not its cwd) and lights + // nothing rather than a guessed row. + // + // Slim rows never draw the card meta, so they pass null and skip the + // subscription instead of retaining the scanner for a row that cannot + // pulse. Card rows subscribe per thread.environmentId — a list spanning M + // environments holds M discovered-servers streams, the multiplier v1's row + // already carries. + const devServerPorts = useThreadDiscoveredPorts({ + environmentId: variant === "card" ? thread.environmentId : null, + threadId: variant === "card" ? thread.id : null, + }); + const devServerPort = devServerPorts[0]?.port ?? null; + // The tooltip names what the mark can only signal, and only what is true + // from where the user sits. The port is the whole claim — the scanner keeps + // every listening TCP socket, so "dev server" would overclaim — and for a + // remote thread the listener is on the remote host, so "localhost" would + // name the wrong machine. v1 says localhost loosely because its Globe is a + // button routed through openDiscoveredPort to the right environment; this + // label is inert text and carries no such correction. + const devServerLabel = + devServerPort === null + ? null + : `${isRemote ? `port ${devServerPort}` : `localhost:${devServerPort}`}${ + devServerPorts.length > 1 ? ` (+${devServerPorts.length - 1})` : "" + }`; + /* fork:end sidebar-v2-dev-server-pulse */ + const detailsTooltip = ( ); @@ -1071,6 +1133,9 @@ const SidebarV2Row = memo(function SidebarV2Row(props: { // the row is not treating as one. // fork:end sidebar-v2-card-rows hasWorktree={thread.worktreePath !== null} + /* fork:begin sidebar-v2-dev-server-pulse — see .fork/customizations.yaml#sidebar-v2-dev-server-pulse */ + devServerPort={devServerPort} + /* fork:end sidebar-v2-dev-server-pulse */ prSlot={prBadge} prUnknown={prUnknown} insertions={diff?.insertions ?? null} diff --git a/apps/web/src/custom/SidebarV2ThreadCardMeta.tsx b/apps/web/src/custom/SidebarV2ThreadCardMeta.tsx index d73e729dc8f..bea8eacaa2b 100644 --- a/apps/web/src/custom/SidebarV2ThreadCardMeta.tsx +++ b/apps/web/src/custom/SidebarV2ThreadCardMeta.tsx @@ -41,6 +41,16 @@ export interface SidebarV2ThreadCardMetaProps { project's checkout. Swaps the branch mark for the worktree one — see the render site for why it replaces rather than joins. */ readonly hasWorktree?: boolean; + /** The first port the scanner attributes to one of this thread's own T3 + terminals, or null — see `sidebar-v2-dev-server-pulse`. Non-null pulses + the branch/worktree mark so the row whose checkout the running server is + serving can be picked out at a glance. A port rather than a boolean + because the accessible text names it: the scanner keeps every listening + TCP socket — no port range, no process filter, no HTTP probe — so + "Server listening on port N" is what it actually knows, where "dev + server running" would overclaim (a debugger, an ssh tunnel, and a + database all count). */ + readonly devServerPort?: number | null; /** Pre-built `#123` badge, or null when the thread has no pull request. */ readonly prSlot: ReactNode; /** The row's VCS query has not answered yet, so `prSlot` being null means @@ -154,8 +164,24 @@ export function SidebarV2ThreadCardMeta(props: SidebarV2ThreadCardMetaProps) { The distinction is invisible to a screen reader either way — both marks are decorative — so the worktree case carries it in text. */} + {/* The dev-server pulse rides this same slot rather than adding a + glyph of its own: the question it answers — "which checkout is + the running server serving?" — is a property of the mark that + already names the checkout. The attribute lands on the slot and + the stylesheet animates the mark inside it (`> svg`, so the + branch text stays legible while the glyph carries the signal). A + thread with neither branch nor worktree never draws the slot and + so cannot pulse — such a thread runs in the project checkout, + which is not the ambiguity this exists to resolve. The animation + is decorative motion, so the state also rides in text for screen + readers — after the branch name, identity before transient state + — and survives `prefers-reduced-motion` as a static + working-green mark. */} {props.hasWorktree || props.branch ? ( - + {props.hasWorktree ? ( <> Worktree @@ -167,6 +193,9 @@ export function SidebarV2ThreadCardMeta(props: SidebarV2ThreadCardMetaProps) { {props.branch ? ( {props.branch} ) : null} + {props.devServerPort != null ? ( + {`Server listening on port ${props.devServerPort}`} + ) : null} ) : null} diff --git a/apps/web/src/theme.custom.css b/apps/web/src/theme.custom.css index 8d7bf7853a3..58a7f7936c1 100644 --- a/apps/web/src/theme.custom.css +++ b/apps/web/src/theme.custom.css @@ -7,8 +7,9 @@ * in this file can leak into an unmarked, pure-upstream build. * * `@keyframes` are the one exception: they are global by definition and cannot - * be scoped to a selector. The fork-owned ones are prefixed `sidebar-v2-rain-*` - * so the names cannot collide with upstream's. + * be scoped to a selector. The fork-owned ones are prefixed `sidebar-v2-` + * (`sidebar-v2-rain-*`, `sidebar-v2-dev-server-pulse`) so the names cannot + * collide with upstream's. * * `@font-face` is the second exception, for the same reason: a face * *registration* is global — there is no selector to hang it off. What matters @@ -585,6 +586,54 @@ --sidebar-v2-status-failed: #ff6b60; } +/* --------------------------------------------------------------------------- + Dev-server pulse — see `.fork/customizations.yaml#sidebar-v2-dev-server-pulse`. + + The thread card's branch/worktree mark breathes from working-green to the + theme's foreground while the port scanner attributes a listening dev server + to one of the thread's own terminals. Green is the same + `--sidebar-v2-status-working` the trailing mark uses, so "alive" keeps one + hue across the card; the far pole is `--foreground` rather than literal + white so the light theme pulses toward ink instead of vanishing into the + panel. + + The mark is `currentColor` throughout (both fork WorktreeIcon and Lucide's + GitBranchIcon), so animating `color` on the svg is the whole mechanism — + the duotone's 20% inner nodes ride along for free. `> svg` rather than a + bare descendant: the slot also carries text (`sr-only` labels, the branch + name) and only the glyph is the signal. + + 4s and `alternate`, so a full breath is 8s — slow enough to read as ambient + status rather than an alert; every notification-shaped pulse in this app is + something blocked on you, and a healthy dev server is the opposite. Under + `prefers-reduced-motion` the animation drops and the `color` declaration + beneath it keeps the mark statically green, so the state survives without + the motion. + + `color` does not composite, so each live row repaints its 12px glyph every + frame for as long as the listener runs — same deal the pixel rain already + accepts, and at this size a deliberate yes rather than an accident. Rows + offscreen pay nothing: `content-visibility: auto` on the li skips them. */ +:root[data-fork="noahhendrickson-t3code"] [data-fork-dev-server-live] > svg { + color: var(--sidebar-v2-status-working); + animation: sidebar-v2-dev-server-pulse 4s ease-in-out infinite alternate; +} + +@media (prefers-reduced-motion: reduce) { + :root[data-fork="noahhendrickson-t3code"] [data-fork-dev-server-live] > svg { + animation: none; + } +} + +@keyframes sidebar-v2-dev-server-pulse { + from { + color: var(--sidebar-v2-status-working); + } + to { + color: var(--foreground); + } +} + /* Pixel rain: the "working" indicator ported from the phanttom Ghostty fork (macos/Sources/Features/Terminal/Sidebar/SidebarView.swift, PixelSparkleView). Three columns of drops fall through a 5-row grid, each column with its own