From dc40fbff2294636cf4239bec0b8b68d8fa96e21a Mon Sep 17 00:00:00 2001 From: noah Date: Tue, 28 Jul 2026 11:28:51 -0400 Subject: [PATCH 1/3] feat(sidebar-v2): pulse the thread card's branch mark while its dev server is live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The branch/worktree mark on a thread card now breathes from working-green to foreground while the port scanner attributes a listening dev server to one of the thread's own T3 terminals, so the row whose checkout the running server is serving can be picked out of the sidebar at a glance. Frontend-only by design: attribution is the scanner's existing terminal→thread mapping (useThreadDiscoveredPorts). An externally launched server arrives with terminal: null — the scanner knows its pid but not its cwd — and lights up nothing rather than a guessed row. The pid→cwd server half is recorded as an explicit non-goal in the manifest entry. The state rides in sr-only text for screen readers, and prefers-reduced-motion drops the animation onto a static green mark rather than dropping the indicator. Guarded by sidebarV2DevServerPulse.test.ts; manifest entry sidebar-v2-dev-server-pulse carries the intent. Co-Authored-By: Claude Fable 5 --- .fork/customizations.yaml | 51 ++++++++++++ .../sidebarV2DevServerPulse.test.ts | 79 +++++++++++++++++++ apps/web/src/components/SidebarV2.tsx | 19 +++++ .../src/custom/SidebarV2ThreadCardMeta.tsx | 22 +++++- apps/web/src/theme.custom.css | 48 ++++++++++- 5 files changed, 216 insertions(+), 3 deletions(-) create mode 100644 apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts diff --git a/.fork/customizations.yaml b/.fork/customizations.yaml index 417742ef8fa..67bd3beb788 100644 --- a/.fork/customizations.yaml +++ b/.fork/customizations.yaml @@ -370,6 +370,57 @@ - 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 a dev server attributed to that thread is listening, so the row whose + checkout the running server is actually serving can be picked out of the + sidebar at a glance. + + 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. + + The pulse rides the existing branch/worktree mark rather than adding a + glyph: the mark already answers "which checkout is this row on", and the + pulse answers "…and it is the one being served". A thread with neither + branch nor worktree draws no slot and cannot pulse; it runs in the project + checkout, which is not the ambiguity the signal exists to resolve. + + The row subscribes to the environment's discovered-servers stream, which + retains the server-side port scanner (3s lsof poll, reference-counted) + while sidebar rows are mounted — previously it ran only while the preview + panel was open. That standing cost is the accepted price of an ambient + indicator; the subscription atom is keyed per environment, so N rows share + one stream. + + 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. The state also rides in `sr-only` text ("Dev + server running") 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..a669b38a6e2 --- /dev/null +++ b/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts @@ -0,0 +1,79 @@ +// @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 dev server to one of the + * thread's own T3 terminals. Three pieces have to stay joined for that to + * happen — the row's fenced subscription in upstream's `SidebarV2.tsx`, the + * attribute gate in the fork-owned meta component, and the animation in the + * Tier 1 stylesheet — and a sync can silently drop any one of them while the + * other two keep compiling. Each assertion below pins one seam. + */ + +import * as NodeFS from "node:fs"; +import * as NodeURL from "node:url"; +import { describe, expect, it } from "vite-plus/test"; + +function readSibling(relativePath: string): string { + return NodeFS.readFileSync(NodeURL.fileURLToPath(new URL(relativePath, import.meta.url)), "utf8"); +} + +const sidebarV2 = readSibling("../components/SidebarV2.tsx"); +const meta = readSibling("../custom/SidebarV2ThreadCardMeta.tsx"); +const theme = readSibling("../theme.custom.css"); + +describe("fork guard: sidebar-v2-dev-server-pulse", () => { + it("joins the row to the port scanner's terminal→thread attribution", () => { + // The one data source this feature is allowed: a listener the scanner's + // subprocess inspection traced to one of the thread's own terminals. + // Losing the hook call leaves the prop permanently false, which renders as + // "no dev server anywhere" rather than as a bug. + expect(sidebarV2).toContain("useThreadDiscoveredPorts({"); + expect(sidebarV2).toContain("devServerLive={devServerLive}"); + // The filter lives in upstream's `portDiscoveryState.ts` + // (`port.terminal?.threadId`), which is on this customization's watch + // list. An externally launched server has `terminal: null` and must light + // nothing — that behaviour is the hook's, so it is watched, not asserted. + }); + + it("gates the pulse attribute on the prop, absent rather than false", () => { + // The stylesheet keys on attribute *presence*. `data-…={false}` would + // still serialize an attribute and pulse every row, so the off state must + // be `undefined`, which React omits entirely. + expect(meta).toContain('data-fork-dev-server-live={props.devServerLive ? "" : undefined}'); + }); + + it("carries the state in text, because the motion is decorative", () => { + // Both marks are aria-hidden and the animation is invisible to a screen + // reader; the `sr-only` label is the whole of the accessible signal. + expect(meta).toMatch(/sr-only">Dev server running { + // Same hue as the trailing mark's "working": one green for "alive" across + // the card. `> svg` keeps the branch text out of the animation. + expect(theme).toContain( + ':root[data-fork="noahhendrickson-t3code"] [data-fork-dev-server-live] > svg', + ); + expect(theme).toMatch( + /\[data-fork-dev-server-live\] > svg \{\n {2}color: var\(--sidebar-v2-status-working\);\n {2}animation: sidebar-v2-dev-server-pulse/u, + ); + // Foreground, not literal white: light mode pulses toward ink instead of + // vanishing into the panel. + expect(theme).toMatch( + /@keyframes sidebar-v2-dev-server-pulse \{\n {2}from \{\n {4}color: var\(--sidebar-v2-status-working\);\n {2}\}\n {2}to \{\n {4}color: var\(--foreground\);/u, + ); + }); + + it("drops the motion, not the state, under prefers-reduced-motion", () => { + // `animation: none` inside the media query leaves the static `color` + // declaration standing, so reduced-motion users get a steady green mark + // rather than losing the indicator. + const reduced = theme.match( + /@media \(prefers-reduced-motion: reduce\) \{[^}]*\[data-fork-dev-server-live\] > svg \{\n {4}animation: none;/u, + ); + expect(reduced).not.toBeNull(); + }); +}); diff --git a/apps/web/src/components/SidebarV2.tsx b/apps/web/src/components/SidebarV2.tsx index c6535dbd495..f0be1904b4a 100644 --- a/apps/web/src/components/SidebarV2.tsx +++ b/apps/web/src/components/SidebarV2.tsx @@ -83,6 +83,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, @@ -555,6 +558,19 @@ 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: a + // listener counts for this row only when it was spawned inside one of the + // thread's own T3 terminals. A dev server started in an external shell has + // `terminal: null` and lights up nothing — the scanner knows its pid but not + // its cwd, so claiming a row for it would be a guess. + const devServerPorts = useThreadDiscoveredPorts({ + environmentId: thread.environmentId, + threadId: thread.id, + }); + const devServerLive = devServerPorts.length > 0; + /* fork:end sidebar-v2-dev-server-pulse */ + const detailsTooltip = ( + + {props.devServerLive ? Dev server running : null} {props.hasWorktree ? ( <> Worktree diff --git a/apps/web/src/theme.custom.css b/apps/web/src/theme.custom.css index e81b28a5b81..d9cc7ab2cfa 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 @@ -560,6 +561,49 @@ --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. */ +: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 From ee9f88bb0184b9e05b9012f928604528bb4aa4d6 Mon Sep 17 00:00:00 2001 From: noah Date: Tue, 28 Jul 2026 14:31:51 -0400 Subject: [PATCH 2/3] =?UTF-8?q?fix(sidebar-v2):=20address=20PR=20#32=20rev?= =?UTF-8?q?iew=20=E2=80=94=20honest=20guard,=20honest=20manifest,=20named?= =?UTF-8?q?=20port?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 (PR #32). The two blocking items and the decided ones: Guard rewritten so it can actually fail (review #1, #6a). Source assertions now run against the extracted fork:begin/end hunks and pin the subscription's input expressions and the derived port — the reviewer's mutation table (threadId: null, devServerPort = null, = 5173) now fails the suite, replayed and confirmed. The meta component is asserted on rendered output (attribute presence/absence, sr-only copy and ordering, and the `> svg` direct-child contract the stylesheet depends on — wrapping the icon now fails). CSS is read through cssRules.ts, so a behaviour-identical declaration swap passes where it used to fail. Manifest cost paragraph corrected (review #2): upstream's v1 row already makes this exact per-row subscription unconditionally, so v2 gaining it is parity, not a new class of cost. The per-environment multiplier (M environments → M streams, each host polling) is now disclosed instead of hidden behind "N rows share one stream". The lsof-fallback degradation (probe failure → terminal: null everywhere → pulse permanently off, silently) and the 1MiB truncation flicker are recorded as known limitations next to the external-shell one (review #5). The port is named, "dev server" dropped (review #3, #4): the row tooltip now carries `localhost:` (+n overflow), v1's exact copy, and the accessible text says "Server listening on port N" — what the scanner actually knows — placed after the branch name so identity precedes transient state (review #6c). Slim rows no longer subscribe (Cursor review): the hook input is gated on variant === "card", so settled/snoozed rows pass null and skip the subscription instead of retaining the scanner for rows that cannot pulse. The repaint cost of animating a non-compositing property is now a stated yes in the stylesheet rather than an accident (review #6d). Globe2Icon imports as its own fenced statement because the phosphor guard parses upstream's import braces and a fence inside them reads as a binding. Co-Authored-By: Claude Fable 5 --- .fork/customizations.yaml | 59 ++++-- .../sidebarV2DevServerPulse.test.ts | 194 ++++++++++++++---- apps/web/src/components/SidebarV2.tsx | 60 +++++- .../src/custom/SidebarV2ThreadCardMeta.tsx | 39 ++-- apps/web/src/theme.custom.css | 7 +- 5 files changed, 272 insertions(+), 87 deletions(-) diff --git a/.fork/customizations.yaml b/.fork/customizations.yaml index 67bd3beb788..b53b2f0d0e6 100644 --- a/.fork/customizations.yaml +++ b/.fork/customizations.yaml @@ -373,9 +373,12 @@ - id: sidebar-v2-dev-server-pulse intent: > The thread card's branch/worktree mark pulses working-green to foreground - while a dev server attributed to that thread is listening, so the row whose - checkout the running server is actually serving can be picked out of the - sidebar at a glance. + 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:` plus a `(+n)` overflow, the same + copy as v1's Globe affordance — so the pulse is never the only way to + learn the port. Attribution is the port scanner's existing terminal→thread mapping and nothing more: a listener counts for a row only when the scanner's @@ -387,18 +390,37 @@ thread.worktreePath) is deliberately out of scope: it needs an apps/server + contracts change, recorded here so a future pass knows the seam. - The pulse rides the existing branch/worktree mark rather than adding a - glyph: the mark already answers "which checkout is this row on", and the - pulse answers "…and it is the one being served". A thread with neither - branch nor worktree draws no slot and cannot pulse; it runs in the project - checkout, which is not the ambiguity the signal exists to resolve. - - The row subscribes to the environment's discovered-servers stream, which - retains the server-side port scanner (3s lsof poll, reference-counted) - while sidebar rows are mounted — previously it ran only while the preview - panel was open. That standing cost is the accepted price of an ambient - indicator; the subscription atom is keyed per environment, so N rows share - one stream. + 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. This is the same standing cost + upstream's v1 row already pays — Sidebar.tsx calls the identical hook per + row, unconditionally — so v2 gaining it restores parity, it does not add a + new class of cost. What is per-list rather than per-row: 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 — v1's multiplier too, but 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 @@ -406,8 +428,11 @@ 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. The state also rides in `sr-only` text ("Dev - server running") because the motion is invisible to a screen reader, and + 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 diff --git a/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts b/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts index a669b38a6e2..1f213b5d536 100644 --- a/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts +++ b/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts @@ -4,76 +4,180 @@ * `.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 dev server to one of the - * thread's own T3 terminals. Three pieces have to stay joined for that to - * happen — the row's fenced subscription in upstream's `SidebarV2.tsx`, the - * attribute gate in the fork-owned meta component, and the animation in the - * Tier 1 stylesheet — and a sync can silently drop any one of them while the - * other two keep compiling. Each assertion below pins one seam. + * 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 meta = readSibling("../custom/SidebarV2ThreadCardMeta.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("joins the row to the port scanner's terminal→thread attribution", () => { - // The one data source this feature is allowed: a listener the scanner's - // subprocess inspection traced to one of the thread's own terminals. - // Losing the hook call leaves the prop permanently false, which renders as - // "no dev server anywhere" rather than as a bug. - expect(sidebarV2).toContain("useThreadDiscoveredPorts({"); - expect(sidebarV2).toContain("devServerLive={devServerLive}"); - // The filter lives in upstream's `portDiscoveryState.ts` - // (`port.terminal?.threadId`), which is on this customization's watch - // list. An externally launched server has `terminal: null` and must light - // nothing — that behaviour is the hook's, so it is watched, not asserted. + 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. + expect(hunk).toContain("`localhost:${devServerPort}${"); + expect(hunk).toContain("devServerLabel={devServerLabel}"); + expect(hunk).toContain("{devServerLabel ? ("); }); - it("gates the pulse attribute on the prop, absent rather than false", () => { - // The stylesheet keys on attribute *presence*. `data-…={false}` would - // still serialize an attribute and pulse every row, so the off state must - // be `undefined`, which React omits entirely. - expect(meta).toContain('data-fork-dev-server-live={props.devServerLive ? "" : undefined}'); + 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("carries the state in text, because the motion is decorative", () => { - // Both marks are aria-hidden and the animation is invisible to a screen - // reader; the `sr-only` label is the whole of the accessible signal. - expect(meta).toMatch(/sr-only">Dev server running 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", () => { - // Same hue as the trailing mark's "working": one green for "alive" across - // the card. `> svg` keeps the branch text out of the animation. - expect(theme).toContain( - ':root[data-fork="noahhendrickson-t3code"] [data-fork-dev-server-live] > svg', - ); - expect(theme).toMatch( - /\[data-fork-dev-server-live\] > svg \{\n {2}color: var\(--sidebar-v2-status-working\);\n {2}animation: sidebar-v2-dev-server-pulse/u, - ); - // Foreground, not literal white: light mode pulses toward ink instead of - // vanishing into the panel. - expect(theme).toMatch( - /@keyframes sidebar-v2-dev-server-pulse \{\n {2}from \{\n {4}color: var\(--sidebar-v2-status-working\);\n {2}\}\n {2}to \{\n {4}color: var\(--foreground\);/u, + 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", () => { - // `animation: none` inside the media query leaves the static `color` - // declaration standing, so reduced-motion users get a steady green mark - // rather than losing the indicator. - const reduced = theme.match( - /@media \(prefers-reduced-motion: reduce\) \{[^}]*\[data-fork-dev-server-live\] > svg \{\n {4}animation: none;/u, + 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")), ); - expect(reduced).not.toBeNull(); + // `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 f0be1904b4a..107b007a5ea 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, @@ -259,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; @@ -271,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 ? (
@@ -559,16 +582,32 @@ const SidebarV2Row = memo(function SidebarV2Row(props: { 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: a - // listener counts for this row only when it was spawned inside one of the - // thread's own T3 terminals. A dev server started in an external shell has - // `terminal: null` and lights up nothing — the scanner knows its pid but not - // its cwd, so claiming a row for it would be a guess. + // 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: thread.environmentId, - threadId: thread.id, + environmentId: variant === "card" ? thread.environmentId : null, + threadId: variant === "card" ? thread.id : null, }); - const devServerLive = devServerPorts.length > 0; + const devServerPort = devServerPorts[0]?.port ?? null; + // The tooltip names what the mark can only signal. Same copy as v1's Globe + // affordance: the port and nothing more — the scanner keeps every listening + // TCP socket, so claiming "dev server" here would overclaim. + const devServerLabel = + devServerPort === null + ? null + : `localhost:${devServerPort}${ + devServerPorts.length > 1 ? ` (+${devServerPorts.length - 1})` : "" + }`; /* fork:end sidebar-v2-dev-server-pulse */ const detailsTooltip = ( @@ -581,6 +620,9 @@ const SidebarV2Row = memo(function SidebarV2Row(props: { modelInstanceId={modelInstanceId} modelLabel={modelLabel} branchMismatch={branchMismatch} + /* fork:begin sidebar-v2-dev-server-pulse — see .fork/customizations.yaml#sidebar-v2-dev-server-pulse */ + devServerLabel={devServerLabel} + /* fork:end sidebar-v2-dev-server-pulse */ /> ); @@ -1088,7 +1130,7 @@ const SidebarV2Row = memo(function SidebarV2Row(props: { // 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 */ - devServerLive={devServerLive} + devServerPort={devServerPort} /* fork:end sidebar-v2-dev-server-pulse */ prSlot={prBadge} prUnknown={prUnknown} diff --git a/apps/web/src/custom/SidebarV2ThreadCardMeta.tsx b/apps/web/src/custom/SidebarV2ThreadCardMeta.tsx index 34fe8344004..bea8eacaa2b 100644 --- a/apps/web/src/custom/SidebarV2ThreadCardMeta.tsx +++ b/apps/web/src/custom/SidebarV2ThreadCardMeta.tsx @@ -41,11 +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; - /** True while the port scanner sees a listener spawned inside one of this - thread's own T3 terminals — see `sidebar-v2-dev-server-pulse`. Pulses the - branch/worktree mark so the row whose checkout the running dev server is - actually serving can be picked out at a glance. */ - readonly devServerLive?: 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 @@ -161,21 +166,22 @@ export function SidebarV2ThreadCardMeta(props: SidebarV2ThreadCardMetaProps) { 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 dev server serving?" — is a property of the mark that + 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, 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 and - survives `prefers-reduced-motion` as a static working-green mark. */} + 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.devServerLive ? Dev server running : null} {props.hasWorktree ? ( <> Worktree @@ -187,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 d9cc7ab2cfa..22a84dcd09e 100644 --- a/apps/web/src/theme.custom.css +++ b/apps/web/src/theme.custom.css @@ -583,7 +583,12 @@ 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. */ + 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; From 3bcc2acd7e816bc888ec2bd2888abfb61dc7f51b Mon Sep 17 00:00:00 2001 From: noah Date: Tue, 28 Jul 2026 16:09:39 -0400 Subject: [PATCH 3/3] =?UTF-8?q?fix(sidebar-v2):=20address=20PR=20#32=20rou?= =?UTF-8?q?nd-2=20review=20=E2=80=94=20honest=20host,=20honest=20cost=20se?= =?UTF-8?q?ntence?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 finding 1: the tooltip label said localhost: for every thread, but a remote thread's listener is on the remote host — v1 gets away with the word because its Globe is a button routed through openDiscoveredPort to the right environment, while this label is inert text. Remote rows now say `port ` (isRemote was already in scope); local rows keep the copy-pasteable localhost form. The guard pins both branches of the ternary inside the extracted hunk, mutation-checked: hardcoding localhost back fails the suite. Round-2 finding 2: the manifest's cost paragraph said "parity, not a new class of cost", which leaned on v1 as if v2 rendered beside it. v2 is an opt-in replacement, so its users gain the standing 3s lsof poll outright — the paragraph now says exactly that, keeping v1 as the precedent for accepting the cost rather than as evidence nothing changed. Co-Authored-By: Claude Fable 5 --- .fork/customizations.yaml | 33 +++++++++++-------- .../sidebarV2DevServerPulse.test.ts | 7 ++-- apps/web/src/components/SidebarV2.tsx | 12 ++++--- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/.fork/customizations.yaml b/.fork/customizations.yaml index b53b2f0d0e6..bd7e956c0c0 100644 --- a/.fork/customizations.yaml +++ b/.fork/customizations.yaml @@ -376,9 +376,13 @@ 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:` plus a `(+n)` overflow, the same - copy as v1's Globe affordance — so the pulse is never the only way to - learn the port. + 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 @@ -411,16 +415,19 @@ 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. This is the same standing cost - upstream's v1 row already pays — Sidebar.tsx calls the identical hook per - row, unconditionally — so v2 gaining it restores parity, it does not add a - new class of cost. What is per-list rather than per-row: 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 — v1's multiplier too, but 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. + 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 diff --git a/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts b/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts index 1f213b5d536..e5c30f9fe9f 100644 --- a/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts +++ b/apps/web/src/__fork_guards__/sidebarV2DevServerPulse.test.ts @@ -96,8 +96,11 @@ describe("fork guard: sidebar-v2-dev-server-pulse", () => { 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. - expect(hunk).toContain("`localhost:${devServerPort}${"); + // 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 ? ("); }); diff --git a/apps/web/src/components/SidebarV2.tsx b/apps/web/src/components/SidebarV2.tsx index 107b007a5ea..eec49f7c89d 100644 --- a/apps/web/src/components/SidebarV2.tsx +++ b/apps/web/src/components/SidebarV2.tsx @@ -599,13 +599,17 @@ const SidebarV2Row = memo(function SidebarV2Row(props: { threadId: variant === "card" ? thread.id : null, }); const devServerPort = devServerPorts[0]?.port ?? null; - // The tooltip names what the mark can only signal. Same copy as v1's Globe - // affordance: the port and nothing more — the scanner keeps every listening - // TCP socket, so claiming "dev server" here would overclaim. + // 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 - : `localhost:${devServerPort}${ + : `${isRemote ? `port ${devServerPort}` : `localhost:${devServerPort}`}${ devServerPorts.length > 1 ? ` (+${devServerPorts.length - 1})` : "" }`; /* fork:end sidebar-v2-dev-server-pulse */