From 1bdca942869cbd32bf917231c7ecfe7be99e6449 Mon Sep 17 00:00:00 2001 From: Miya Date: Mon, 8 Jun 2026 14:39:36 +0200 Subject: [PATCH 1/4] chore(persona): write local skill files for terminal-renderer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User asked for the persona's skills to be LOCAL files (not phantom remote package references like the autonomous-actor pattern). This commit: 1. Creates six substantive markdown skill files under .agentworkforce/workforce/personas/__assets/terminal-renderer/ — each capturing a discrete domain of the persona's expertise: - xterm-internals.md (105 lines) — parser pipeline, renderer trade-offs, addon discipline, focus mode, viewport vs scrollback, font measurement timing - pty-broker-pipeline.md (172 lines) — chunk path from broker IPC to term.write, rAF coalescing, snapshot-vs-replay race, SIGWINCH bounce, trim cap accounting, predictive echo discipline - ansi-vt-sequences.md (152 lines) — sequence taxonomy, cursor movement, DECSET modes (?1004 / ?1049 / ?2026), TUI redraw patterns and their failure modes - bug-class-triage.md (104 lines) — symptom-to-bug-class lookup table built from the PR #158 catalogue; use BEFORE reading source - lifecycle-decoupling.md (249 lines) — runtime registry pattern, token-based mount ownership, parked host, clearOnDataIf identity check, dispose ordering - fix-discipline.md (152 lines) — one-write invariant, read before guessing, minimal-diff scope, instrument-don't-guess after two failed fixes, AGENTS.md test requirement 2. Updates terminal-renderer.json skills array to reference each file via __assets/terminal-renderer/.md local path. Description for each skill summarizes the file content for quick orientation. Matches the existing .agentworkforce local asset pattern (slack-comms uses __assets/slack-comms/slack-comms.md for its claudeMd). 934 lines of structured operating knowledge extracted from the PR #158 fix cycle so future investigations don't have to re-derive it. --- .../terminal-renderer/ansi-vt-sequences.md | 152 +++++++++++ .../terminal-renderer/bug-class-triage.md | 104 ++++++++ .../terminal-renderer/fix-discipline.md | 152 +++++++++++ .../terminal-renderer/lifecycle-decoupling.md | 249 ++++++++++++++++++ .../terminal-renderer/pty-broker-pipeline.md | 172 ++++++++++++ .../terminal-renderer/xterm-internals.md | 105 ++++++++ .../workforce/personas/terminal-renderer.json | 33 ++- 7 files changed, 966 insertions(+), 1 deletion(-) create mode 100644 .agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md create mode 100644 .agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md create mode 100644 .agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md create mode 100644 .agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md create mode 100644 .agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md create mode 100644 .agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md new file mode 100644 index 00000000..fce97f68 --- /dev/null +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md @@ -0,0 +1,152 @@ +# ansi-vt-escape-sequences + +The byte-level command vocabulary TUIs use to drive the terminal. When you can read the chunks, you can name the bug. + +## Sequence taxonomy + +All escape sequences start with `\x1b` (ESC, often shown as `\e` in logs). Then: + +- `\e[ ...` — **CSI** (Control Sequence Introducer). The bulk of cursor / color / mode commands. +- `\e]...` — **OSC** (Operating System Command). Window title, hyperlinks, cursor color. +- `\eP...` — **DCS** (Device Control String). Sixel graphics, terminfo queries. +- `\eN ...`, `\eO ...` — **SS2 / SS3**. Single-shift to alternate character sets. +- `\e( B`, `\e( 0` — **Character set designation**. `B` = US-ASCII, `0` = DEC special graphics. +- `\eM`, `\eD`, `\eE` — **Cursor controls outside CSI**. Reverse index, index, next line. + +CSI subforms: +- `\e[ ` — standard +- `\e[ ? ` — private (DECSET / DECRST family) +- `\e[ > ` — secondary device attributes +- `\e[ ! ...` — soft terminal reset family + +## Cursor movement + +| Sequence | Meaning | +|---|---| +| `\e[A`, `\e[A` | Cursor up 1 or n rows | +| `\e[B`, `\e[B` | Cursor down n rows | +| `\e[C`, `\e[C` | Cursor right n columns | +| `\e[D`, `\e[D` | Cursor left n columns | +| `\e[;H` | Absolute position (1-indexed) | +| `\e[H` | Position (1,1) — home | +| `\e[s` / `\e[u` | Save / restore cursor | +| `\e[6n` | Query cursor position (terminal responds `\e[;R`) | + +**Cursor up beyond row 1 pegs at row 1.** It does NOT scroll history into view. Cursor up from row 1 = cursor stays at row 1. + +**Absolute positioning** is what TUIs use for in-place redraws. The TUI's notion of "the card is at rows 5-15" depends on the row count never changing under it. If pear's viewport scrolls or buffer trims a row off, the TUI's `\e[5;1H` lands at a different visible row than it expected → stacked redraws. + +## Line / screen clearing + +| Sequence | Meaning | +|---|---| +| `\e[K` | Clear from cursor to end of line | +| `\e[1K` | Clear from start of line to cursor | +| `\e[2K` | Clear entire line | +| `\e[J` | Clear from cursor to end of screen | +| `\e[1J` | Clear from start of screen to cursor | +| `\e[2J` | Clear entire screen | +| `\e[3J` | Clear scrollback (xterm extension) | + +`\e[2J` does NOT reset the cursor. Many TUIs follow it with `\e[H` for the clear-and-home idiom. + +## Scroll region + +`\e[;r` — set the scroll region to rows `top` through `bottom` (1-indexed). All subsequent scrolling is contained within this region. Used by TUIs to keep a status bar at the bottom while content scrolls above it. + +## DECSET / DECRST modes (private CSI ?) + +The modes that change behavior xterm-wide. Format: `\e[?h` to set, `\e[?l` to reset. + +| Mode | Effect | Bug class | +|---|---|---| +| `?25` | Cursor visibility (h = show, l = hide) | Hidden cursor on tab return = TUI forgot to re-show after losing focus | +| `?1000` | X10 mouse tracking | Mouse clicks emit as bytes to the PTY | +| `?1006` | SGR mouse encoding (modern) | Same | +| `?1004` | **Focus event reporting** | TUI receives `\e[I` / `\e[O` on focusin / focusout — **stacked redraws on `term.focus()` calls** | +| `?1049` | Alt-screen with state save | vim/htop entry/exit. Main buffer untouched. | +| `?2004` | Bracketed paste | Pasted text wrapped in `\e[200~` / `\e[201~` so TUI distinguishes from typed input | +| `?2026` | **Synchronized output mode** | TUI wraps a multi-write frame in begin/end markers; terminal defers paint until end marker. Avoids tearing on multi-chunk redraws. | +| `?7` | Auto-wrap mode | Off = cursor pegs at right edge, doesn't auto-wrap | + +**?1004 + programmatic focus = stacked redraws.** This is the bug class behind "+1 card per tab switch back" when a TUI like Claude Code's Ink renderer is running. Fix is not to suppress the mode; fix is not to call `term.focus()` on every visibility change in the host. + +**?1049 vs main buffer**: alt-screen is a clean canvas. The TUI draws once, exits, and the main buffer is exactly as it was. vim and htop are the canonical users. If you see "the TUI's output is in scrollback after I exited it", the TUI didn't use alt-screen — it drew directly into the main buffer. This is the failure mode behind Claude Code's tool-card history. + +**?2026 sync mode**: codex-1's TUI uses this. Each redraw frame is bracketed: + +``` +\e[?2026h ← begin sync +\e[H ← home cursor + +\e[?2026l ← end sync, atomic paint +``` + +xterm.js supports it in recent versions. If you don't see card stacking on codex-1 but do on Claude Code, sync mode is one of the differences. + +## OSC (Operating System Command) + +Terminated by `\x07` (BEL) or `\e\\` (ST). + +| Sequence | Meaning | +|---|---| +| `\e]0;\x07` | Set window title | +| `\e]8;;<url>\e\\<text>\e]8;;\e\\` | Hyperlink (clickable text) | +| `\e]10;<color>\x07` / `\e]11;...` | Set fg / bg color | +| `\e]52;c;<base64>\x07` | OSC 52 clipboard write — security-relevant | + +## DCS (Device Control String) + +Terminated by `\e\\` (ST). + +Most common: sixel graphics `\eP<params>q<data>\e\\`. Not all xterm builds support it. Image addon required. + +## Character sets + +`\e( B` = US-ASCII (default). `\e( 0` = DEC special graphics (box-drawing). Some legacy TUIs switch to the special set to draw `┌─┐│` style boxes; if you see weird non-ASCII characters in scrollback after a TUI exits without restoring `\e( B`, the set wasn't restored. + +## TUI redraw patterns + +Three flavors, each with a different failure mode: + +### 1. Redraw-in-place via cursor positioning + +The TUI emits a card, then later moves cursor back up and rewrites the same lines. Example: Claude Code's tool cards. + +- **Works when**: the cursor's row in the buffer hasn't moved (no intervening scroll, no buffer trim). +- **Fails when**: viewport scrolled OR buffer trimmed OR the TUI's notion of row count is stale (no SIGWINCH on resize). +- **Symptom**: stacked duplicate cards in scrollback. + +### 2. Alt-screen full-redraw + +The TUI enters `?1049h`, draws everything, exits on close. Example: vim, htop, less. + +- **Works**: always cleanly. Main buffer preserved. +- **Fails**: rarely. Alt-screen state can leak if the TUI is killed without sending `?1049l` (resize the terminal, scrollback shows alt-screen contents instead of main). + +### 3. Synchronized output (?2026) + +The TUI brackets each frame between `?2026h` / `?2026l`. Example: codex-1's TUI. + +- **Works**: tear-free atomic frames. +- **Fails**: xterm versions without ?2026 support ignore the brackets and the frame content streams in normally — typically still works because the cursor-positioning model is the same as pattern 1. + +## How to read a chunk preview + +When debugging a chunk stream, replace escapes with readable markers: + +```ts +function previewChunk(chunk: string): string { + return chunk.slice(0, 80) + .replace(/\n/g, '\\n') + .replace(/\r/g, '\\r') + .replace(/\x1b/g, '\\e') +} +``` + +A chunk like `"\\e[H\\e[5;1H\\e[K"` reads as: "home cursor, move to row 5 col 1, clear to end of line". Now you can match it against the bug class catalogue. + +## Companion reading + +- `pty-broker-streaming-pipeline.md` — how chunks arrive +- `renderer-bug-class-triage.md` — symptoms ↔ specific sequences diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md new file mode 100644 index 00000000..a72e02ab --- /dev/null +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md @@ -0,0 +1,104 @@ +# renderer-bug-class-triage + +The lookup table from observed symptom to specific code path. Built from the PR #158 catalogue of fixes and the bugs found while shipping it. Use this **before** reading source — it disambiguates which file to open. + +## Triage axis 1: timing + +| Timing pattern | Implies bug class | +|---|---| +| Immediately on first attach | Snapshot vs replay race, font-settle race | +| Only after N tab switches | Side-effect of visibility change OR listener leak | +| Only after window cmd-tab | Window-focus handler programmatic focus | +| Only at first redraw of a TUI | Initial-mount focus cascade, no SIGWINCH bounce | +| Only under heavy streaming | Trim cap accounting, predictive echo backpressure | +| After resize | Spurious SIGWINCH, font measure / metric drift | +| After theme change | WebGL glyph atlas not invalidated | +| Only on display:none return | Stale WebGL frame, no `term.refresh()` | + +## Triage axis 2: symptom shape + +### Duplicate text + +| Specific shape | Most likely cause | +|---|---| +| Exact byte-for-byte duplicate immediately after attach | **Snapshot-vs-replay race.** The snapshot contains chunks 1-50; the buffer also contains chunks 1-50; the subscriber sliced from 0 and replayed them. Fix: `flushPtyChunksNow(key)` + `writtenChunks = buffer.length` before subscribe. | +| Duplicate only after N tab switches; multiplier increases with N | **Listener leak.** Each tab switch added a new subscriber. Look for `subscribePtyBuffer` calls without paired `unsub`. | +| +1 identical Claude Code tool-card per tab switch back | **Focus events to a `?1004` TUI.** The visibility-effect fires `term.focus()` → focusin → `\e[I` → TUI redraws. Drop the auto-focus on visibility change. | +| +1 card per cmd-tab back to pear | **Window-focus handler** fires the same `term.focus()`. Drop it for the same reason. | +| ~5 cards on first agent spawn | **Initial-mount focus cascade.** Multiple setTimeouts firing `focusTerminal` at 0/50/150/300ms. Each emits `\e[I`. Reduce to one. | +| Identical message twice in chat under load | **Optimistic-vs-canonical chat dedup gap.** User's optimistic local-UUID record and the broker's canonical event_id record both survived id-based dedup. Fix: scope `local: true` flag + check content-window match for the canonical-of-optimistic case. | +| Same agent message twice in chat under load | **Cross-stream broker replay.** Same logical message delivered via `relay_inbound` AND `reconcileMessages` snapshot with mismatched event_ids. Fix: `isDuplicateAgentEcho` content-window guard with strict project equality. | + +### Ghosting / scroll trail + +| Specific shape | Most likely cause | +|---|---| +| Trail when sliding split pages | **Canvas under CSS transform.** `translateX` animation over a WebGL canvas produces ghost composites. Use `display: none/block` instead, paired with `runtime.refreshOnShow()`. | +| Smear during pane drag | **Backdrop-filter blur stacked over hot path.** Remove blur from the hot-path surfaces (terminal/chat). Keep blur only on transient dialogs. | +| Stale frame on tab return | **WebGL stale paint.** `display: none` skipped GPU paints; on return the canvas shows the last paint. Call `term.refresh(0, term.rows - 1)` on visibility flip. | + +### Smeared / drifting glyphs + +| Specific shape | Cause | +|---|---| +| Glyphs slightly misaligned on initial mount; fix themselves at next resize | **Font measurement racing font load.** `term.open()` measured fallback font; `document.fonts.load()` resolved later. Fix: `await document.fonts.load('13px "JetBrains Mono"')` before fit + refresh. | +| Glyphs paint with previous colors after theme change | **WebGL glyph atlas stale.** Theme swap didn't invalidate the texture atlas. Call `webglAddon.clearTextureAtlas()` (where supported) or accept stale glyphs until next major repaint. | +| Single frame of mis-sized glyphs on initial mount | **Font-settle race against WebGL load.** `loadWebgl` scheduled before `awaitFontSettle` resolved. Cosmetic, not load-bearing. Resolves on next refit. | + +### Cursor drift / TUI breaks mid-stream + +| Shape | Cause | +|---|---| +| Cursor lands in the wrong row after a CSI cursor-up | **TUI's expected row count differs from xterm's actual row count.** Resize wasn't propagated as SIGWINCH; TUI thinks it's 60 rows tall when it's actually 30. Fix: bounce SIGWINCH on initial attach. | +| Card appears at wrong location | Same root cause — absolute positioning landed at a stale row. | +| Card content sliced or partial | Predictive-echo write order race OR a `\e[K` after the cursor-up cleared too much OR alt-screen state leaked. | + +### Visible duplicates in chat + +| Shape | Cause | +|---|---| +| User sees their own message twice within ~10s | **Optimistic-vs-canonical.** `addHumanMessage` appended the local record; `relay_inbound` then appended the canonical without realizing it was the echo. Fix: `local: true` flag scoping + canonical-of-optimistic match. | +| User sees their identical "ok" send disappear when they double-send | **Over-eager echo dedup.** The guard collapsed two distinct user messages thinking the second was a canonical echo. Fix: drop content-window check from `addHumanMessage`; only apply on the canonical reception side. | +| Two distinct agents in the same channel can't send identical content | **Agent dedup false-positive on `from` not strict enough.** The guard didn't differentiate sender. Fix: include `from` in the content-window predicate. | + +### Visible duplicates in terminal (NOT chunk-level) + +| Shape | Cause | +|---|---| +| Same multi-line tool card stacked N times in scrollback | Cursor-positioning redraw failed (see "TUI breaks mid-stream"). Not a chunk-write bug — instrument to confirm: each chunk arrives once, each writeChunks fires once. Then look at the TUI's redraw pattern. | +| Streaming output renders twice per chunk | **Predictive echo write path doubled with direct write.** `writeChunks` should EITHER call `predictiveEcho.onServerOutput(chunk)` OR `liveTerm.write(chunk)`, never both. | + +### Blank canvas / black screen + +| Shape | Cause | +|---|---| +| Terminal pane is blank on first attach | `term.open()` was called with a zero-size container. Defer init to `requestAnimationFrame` and retry until `hasLayout(container)` returns true. | +| Split-page terminal blank when initially hidden | Same — the page mounted with `display: none`, zero-size, init bailed, never retried. Schedule `initIfReady` rAF retry on no-layout. | +| Terminal blanks after WebGL context loss | The `onContextLoss` handler dropped WebGL but didn't fall back to DOM. Set `suggestedRenderer = 'dom'` and dispose the failed addon. | + +## How to use this table + +1. Read the symptom literally. "+1 card per tab switch back" not "duplication." +2. Find the matching row above. +3. Open the named file/code path BEFORE forming a fix hypothesis. +4. If the symptom doesn't match any row → instrument before guessing (see `fix-discipline-and-instrumentation.md`). + +## When the symptom is reported vaguely + +The single most useful question to ask the user: + +> Does this happen on first attach, or only after a specific action (tab switch, cmd-tab, resize, theme change)? + +This disambiguates 80% of the bug-class space in one question. + +If still ambiguous, the second question: + +> When you scroll back, are the duplicates exact byte-for-byte (suggesting chunk replay) or do they look like rendered cards that should have been redrawn in place (suggesting cursor positioning failure)? + +Now you can pick the right file to open. + +## Companion reading + +- `pty-broker-streaming-pipeline.md` — the chunk path +- `ansi-vt-escape-sequences.md` — what cursor positioning sequences look like +- `fix-discipline-and-instrumentation.md` — what to do when the table doesn't match diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md new file mode 100644 index 00000000..bc0c6a1e --- /dev/null +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md @@ -0,0 +1,152 @@ +# fix-discipline-and-instrumentation + +The operating discipline that keeps renderer fixes from being speculative or regressing. Read this before writing any fix. + +## The one-write invariant + +> Each PTY chunk that arrives from the broker is written to `term.write` exactly once. + +When duplication appears, count the writes. The bug is in exactly one of three places: + +1. **Upstream** — the chunk arrives at `appendPtyChunk` twice. Broker re-emit, double subscribe, or main-process worker-stream replay. Not a renderer bug. +2. **Pipeline** — the chunk arrives once but the listener fires twice. Listener leak in the registry (two `subscribePtyBuffer` without paired unsub). +3. **TUI re-emit** — the chunk arrives once, fires once, but the TUI emits the same content again later (cursor positioning failed, redraw stacked). Not a chunk dup; an interpretation bug. + +You **must** know which before writing a fix. Add instrumentation if you can't tell from reading. + +## Read before guessing + +For any reported renderer bug, before writing a fix: + +1. Open the file the user named. +2. Trace from the producer (`broker:pty-chunk` IPC arrival) to the consumer (`term.write`) and identify every place a chunk could be duplicated, dropped, or interpreted wrong. +3. Name the specific step that produces the observed symptom. +4. If you can't name the step, you don't have a fix — you have a guess. + +Examples of "naming the step": + +- ✅ "Line 234 of predictive-echo.js writes the chunk before line 236 awaits the model write. The model write is the only thing that suspends, so if `onResize` fires between them, the model is sized for post-resize cols while the chunk was written assuming pre-resize cols. Mis-attributes column counts." +- ❌ "It might be a race in predictive echo." + +The first is a fix hypothesis you can confirm or reject. The second is a paragraph of vague speculation that wastes review time. + +## Pattern-match symptom to bug class + +Before opening any file, use `renderer-bug-class-triage.md` to map the symptom to a candidate file. "+1 stacked card per tab switch" is not a triage prompt — it IS the bug class signal. Open the visibility-effect code path; don't read the chunk pipeline. + +When multiple classes fit, plan the cheapest disambiguation experiment first. Often: a temporary `console.log` at the chunk-write site counting writes per chunk. Five minutes of instrumentation beats two hours of speculative fixes. + +## Minimal-diff scope + +A 5-line targeted change in the right file beats a 50-line refactor across three files. If a fix requires touching unrelated abstractions, the bug class is probably different from what you think — re-check the triage. + +Specifically: + +- Don't refactor architecture beyond the minimum the fix needs. +- Don't introduce new abstractions "for future flexibility." The next renderer bug will be different and you'll have built the wrong abstraction. +- Don't add config knobs ("disable virtualization fallback", "fall back to DOM renderer if WebGL fails twice"). Each knob adds a new failure mode. +- Don't paper over a duplication symptom by adding a dedupe layer downstream. Find where the duplicate is introduced upstream and remove it there. + +## Instrument-don't-guess after two failed fixes + +After two consecutive fixes for the same symptom fail, the third action is **temporary instrumentation**, never another speculative fix. + +The instrumentation must: + +1. Capture **literal runtime values**, not assumptions. If you think a listener is firing twice, log the listener-call count + a preview of the chunk per call. +2. Be **gated behind a localStorage flag** so production renderers don't pay the cost: + +```ts +let __diagChecked = false +let __diagEnabled = false +export function diagEnabled(): boolean { + if (__diagChecked) return __diagEnabled + __diagChecked = true + try { + __diagEnabled = localStorage.getItem('PEAR_DIAG_PTY') === '1' + } catch { + __diagEnabled = false + } + return __diagEnabled +} + +// at the call site: +if (diagEnabled()) { + console.log(`[diag:pty-append] key=${key} bytes=${chunk.length} preview="${preview}"`) +} +``` + +3. Be **reverted in the same PR cycle** as the root-cause fix. Never leave diagnostics live in production. The localStorage gate makes this safe-by-default, but the code still adds noise to the file. + +Enable for capture: + +``` +localStorage.setItem('PEAR_DIAG_PTY', '1') +location.reload() +// ... reproduce the bug +// copy console output +localStorage.removeItem('PEAR_DIAG_PTY') +``` + +## Defensive fixes name the UX trade-off + +If a fix accepts a UX cost to suppress a bug: + +- ✅ "Dropped auto-focus on visibility change. UX cost: one extra click after tab switch to focus the terminal. Worth it because the focus event was producing a stacked card per switch on Claude Code's TUI." +- ❌ Silent commit message: "fix: tab switch issue." + +Name the trade-off in the commit body so the operator can decide whether to accept it. Defensive fixes are legitimate but they're explicit choices. + +## When AGENTS.md requires regression tests + +Pear's `AGENTS.md`: + +> Add regression tests when touching broker start, event streaming, PTY buffering, spawned personas, or integration notifications. Include duplicate/replay cases, not just the happy path. + +This supersedes any "manual test plans are the contract" default. The PTY buffering layer is **unit-testable** with vitest against the `pty-buffer-store` (no DOM needed). The agent-store reconciler is unit-testable. The runtime registry is unit-testable with `happy-dom` + an xterm mock. + +What the manual test plan IS for: visual rendering / paint timing / GPU compositing — things genuinely not automatable in a headless runner. That carve-out is the anti-goal, not "skip all tests." + +A regression test for a duplication bug should: + +1. Set up the exact pre-condition the bug needed (e.g. buffer state, subscribed listeners). +2. Trigger the action that produced the dup. +3. Assert the post-state is correct (count of records, listener-fire count, etc.). + +If you can't write that test, the bug isn't fully understood yet. + +## What "battle-tested" means for a renderer fix + +A renderer fix is proven only when: + +1. The failure-class regression test runs RED without the fix (so you know the test isn't vacuous). +2. The test runs GREEN with the fix. +3. Heavy-load behavior (stress-test harness) doesn't regress on a related metric. +4. The manual-test repro the operator can run no longer surfaces the bug. + +"It worked once on my machine" is not battle-tested. "It passes vitest" is necessary but not sufficient — vitest can't catch frame timing or GPU compositing bugs. + +## Don't recommend a fix you wouldn't sign your name to + +The output of a renderer-bug investigation is: + +- **Diagnosis**: file:line + specific code-path + escape-sequence behavior that produces the symptom. +- **Fix**: file:line + minimal-diff scope. +- **Gates**: typecheck, build, vitest pass; manual test plan with concrete repro steps. +- **UX trade-off** (if any) named explicitly. +- **What couldn't be validated** without running the app. + +If you can't fill in all five, you don't have a fix yet. Say so. Propose the smallest diagnostic experiment that would let you fill them in. + +## Antipatterns + +- **Retry loops without root-cause**: "If the broker IPC fails, retry 3 times" hides the actual bug. +- **Polling fallbacks**: "If the event doesn't arrive, poll every 5s." Now you have two delivery mechanisms drifting apart. +- **Catch-all `try { } catch { /* ignore */ }`**: swallows the next bug that surfaces in the same code path. +- **Adding a SIGWINCH bounce "to be safe"**: each fake resize is a TUI redraw cost. Don't add bounces speculatively — only when you've confirmed the TUI keys initial paint off a winsize change. +- **"Should be fine"**: every renderer bug landed in main was "should be fine" at some point. + +## Companion reading + +- `renderer-bug-class-triage.md` — what to consult before opening any file +- `pty-broker-streaming-pipeline.md` + `xterm-internals-and-renderers.md` — the systems the discipline applies to diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md new file mode 100644 index 00000000..8f6fb1a9 --- /dev/null +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md @@ -0,0 +1,249 @@ +# react-lifecycle-decoupling-and-token-ownership + +The architecture pattern that takes "duplicate text on tab switch" off the table as an entire bug class. xterm + WebGL + PTY subscription must NOT be torn down and rebuilt on React component lifecycle changes. + +## The bad shape (what NOT to do) + +```tsx +function TerminalView({ agentName }: { agentName: string }) { + const containerRef = useRef<HTMLDivElement>(null) + + useEffect(() => { + const term = new Terminal() + term.loadAddon(new WebglAddon()) + term.open(containerRef.current!) + + const unsub = subscribePtyBuffer(agentName, (chunks) => { + chunks.forEach(c => term.write(c)) + }) + + return () => { + unsub() + term.dispose() + } + }, [agentName]) + + return <div ref={containerRef} /> +} +``` + +What goes wrong: + +- Every tab switch triggers unmount → cleanup → mount of a new instance. +- The new instance calls `attachTerminal` → broker returns snapshot. +- Snapshot is written; subscriber catches up against the buffer. +- Between snapshot capture and subscriber attach, new chunks landed. They get replayed on top of the snapshot → **duplicate text**. +- WebGL canvas is created + destroyed each cycle → GPU resource churn. +- Predictive echo state lost → predictions don't reconcile. + +## The right shape + +A module-level runtime registry keyed by agent. React mounts and detaches DOM hosts; the runtime itself survives across React lifecycle changes. Disposal happens only when the agent is fully released. + +```ts +// terminal-runtime-registry.ts + +interface TerminalRuntime { + readonly key: string + readonly term: Terminal + readonly host: HTMLDivElement + mount(container: HTMLElement): symbol // returns token + detach(token: symbol): void // takes token, stale = no-op + dispose(): void // only on agent release + isMounted(): boolean + setOnData(handler: ((data: string) => void) | null): void + clearOnDataIf(handler: (data: string) => void): void // identity-checked + refreshOnShow(): void // for display: none return + fitAndSync(): { rows: number; cols: number } | null + setInputSrttGetter(fn: () => number | null): void +} + +const runtimes = new Map<string, TerminalRuntime>() + +export function acquireTerminalRuntime(opts: {...}): TerminalRuntime { + const key = getAgentKey(opts.projectId, opts.agentName) + const existing = runtimes.get(key) + if (existing) { + existing.setTheme(opts.theme) + existing.setTerminalMode(opts.terminalMode) + return existing // SAME instance + } + const runtime = createRuntime(key, opts) + runtimes.set(key, runtime) + return runtime +} + +export function disposeTerminalRuntime(key: string): void { + const runtime = runtimes.get(key) + if (!runtime) return + runtimes.delete(key) + runtime.dispose() +} +``` + +The React side becomes a thin shim: + +```tsx +function TerminalView({ agentName }: { agentName: string }) { + const containerRef = useRef<HTMLDivElement>(null) + + useEffect(() => { + const runtime = acquireTerminalRuntime({ agentName, ... }) + const token = runtime.mount(containerRef.current!) + + return () => { + runtime.detach(token) // host parks; runtime stays alive + } + }, [agentName]) + + return <div ref={containerRef} className="h-full w-full" /> +} +``` + +Tab switch → unmount → `detach()` → host moves to parked container (`document.body` invisible div). Tab switch back → new effect → mount → reparent host into new container. xterm + WebGL + buffer subscription all survive. No re-attach. No snapshot replay. **The bug class is gone.** + +## Token-based mount ownership + +Required because of React's cross-tree commit ordering. Consider this scenario: + +1. User clicks split mode while tab mode is active. +2. React commits the new (split) tree. +3. The order can be: + - Naive expectation: `componentA.cleanup → componentB.mount` + - Actual React 18 behavior in some cases: `componentB.mount → componentA.cleanup` + +If B's mount runs first into container B, then A's cleanup runs and naively parks the host, B's container is now empty — its mount silently lost. + +The token pattern fixes this without needing to predict React's order: + +```ts +let currentToken: symbol | null = null +let lastMountedContainer: HTMLElement | null = null + +return { + mount(container: HTMLElement): symbol { + if (disposed || !term) return Symbol('disposed') + if (host.parentElement !== container) { + container.appendChild(host) // silently reparent + } + const token = Symbol('mount') + currentToken = token + lastMountedContainer = container + void initIfReady(container) + return token + }, + detach(token: symbol): void { + if (disposed) return + if (token !== currentToken) return // stale token, no-op + currentToken = null + cancelPendingInit() + const park = getParkedContainer() + if (host.parentElement !== park) { + park.appendChild(host) + } + }, +} +``` + +Walkthrough of the cross-tree case: + +1. `tokenA = runtime.mount(A)` → currentToken = tokenA. +2. `tokenB = runtime.mount(B)` → host moves from A to B. currentToken = tokenB. +3. A's cleanup runs: `runtime.detach(tokenA)`. `tokenA !== currentToken` → return. **Host stays in B.** +4. B's cleanup eventually runs: `runtime.detach(tokenB)`. Match → park. + +Both standard order and cross-tree order are handled by the same code path. **No `lastMountedContainer` heuristic needed in the detach guard.** (Earlier versions of pear had both `mounted` and `lastMountedContainer` checks; they interacted destructively. The token model subsumes them.) + +## Parked host container + +A singleton `<div>` appended to `document.body` with `display: none` (or `position: absolute; left: -9999px`). The runtime parks its host there between mounts. + +```ts +let parkedContainer: HTMLDivElement | null = null +function getParkedContainer(): HTMLDivElement { + if (parkedContainer && parkedContainer.isConnected) return parkedContainer + const div = document.createElement('div') + div.setAttribute('data-pear-terminal-park', 'true') + div.style.cssText = 'position:absolute;left:-9999px;width:0;height:0;overflow:hidden' + document.body.appendChild(div) + parkedContainer = div + return div +} +``` + +While parked, the WebGL canvas is invisible but xterm continues to process incoming chunks (the buffer subscription is alive). When the host is reparented into a visible container, the canvas paints from the current buffer state. Call `term.refresh(0, term.rows - 1)` once on un-park (via `runtime.refreshOnShow()`) so the WebGL renderer flushes a frame. + +## clearOnDataIf — companion identity-checked clear for onData + +The runtime exposes a single `onData` handler slot: + +```ts +let onDataHandler: ((data: string) => void) | null = null +term.onData((data) => onDataHandler?.(data)) + +return { + setOnData(handler) { onDataHandler = handler }, + clearOnDataIf(handler) { + if (onDataHandler === handler) onDataHandler = null + }, +} +``` + +The hook captures its own handler ref: + +```ts +const onDataHandler = (data: string) => sendInput(data) +runtime.setOnData(onDataHandler) +// ... +return () => { + runtime.clearOnDataIf(onDataHandler) // only clears if it's still ours +} +``` + +The identity check protects against the same cross-tree case the token model protects the host: if a newer hook has already installed its own handler, the older cleanup is a no-op. Without this, the older cleanup would `setOnData(null)` and silently kill input forwarding for the live mount. + +## Disposal contract + +`runtime.dispose()` is called ONLY when the agent itself is released (closed, removed). Never on React unmount. The store drives disposal: + +```ts +// somewhere in agent-store reaction to `agent_released` broker event: +disposeTerminalRuntime(getAgentKey(projectId, agentName)) +``` + +Inside dispose, order matters: + +```ts +dispose(): void { + if (disposed) return + cancelPendingInit() // 1. Cancel any pending rAF + clearPtyBuffer(key) // 2. Drain buffer + notify with [] tail + disposed = true // 3. Flip flag (after subscribers settled) + currentToken = null + unsubBuffer?.() + unsubBuffer = null + disposePredictiveEcho?.() + predictiveEcho = null + try { webglAddon?.dispose() } catch {} + webglAddon = null + try { term?.dispose() } catch {} + term = null + if (host.parentElement) host.parentElement.removeChild(host) +} +``` + +`clearPtyBuffer(key)` before `disposed = true` so the synchronously-fired empty-tail notification still sees a live runtime (subscribers can check `disposed` at the top of their handler to short-circuit). After `disposed = true`, queued rAFs from `pty-buffer-store` will fire but the handler returns early — safe. + +## Antipatterns + +- **Reference counting per acquire/release**: tempting but unused in practice. The hook never calls a paired `release` (because tab switches don't conceptually "release" the agent). Disposal goes through the store-driven path. If you ship `refCount` it grows monotonically and `releaseTerminalRuntime` becomes dead code. Drop it. + +- **`refuse second concurrent mount` guards** without the token model: produces the bug they're trying to prevent. With the token model, a second mount silently reparents and the older detach is a no-op. Both safe. + +- **`display: none` over the WebGL canvas during animation**: the canvas keeps a backing texture, the compositor re-presents it during transform animations → ghost trails. Use `display: none/block` (paired with `term.refresh()`) or `opacity` fade. Never `transform`. + +## Companion reading + +- `xterm-internals-and-renderers.md` — what survives in the runtime +- `pty-broker-streaming-pipeline.md` — the subscription lifecycle the runtime owns +- `renderer-bug-class-triage.md` — symptoms this pattern eliminates diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md new file mode 100644 index 00000000..040d4ce2 --- /dev/null +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md @@ -0,0 +1,172 @@ +# pty-broker-streaming-pipeline + +The end-to-end PTY byte path from the broker IPC into `term.write`, the races between snapshot and stream, and the discipline that prevents the duplicate-text bug class. + +## Pipeline shape + +``` +PTY child process + ↓ raw bytes +broker (main process) + ↓ IPC (broker:pty-chunk) +pear renderer + ↓ appendPtyChunk(key, chunk) +pty-buffer-store + ↓ rAF flush +runtime.writeChunks(tail) + ↓ predictiveEcho.onServerOutput OR liveTerm.write directly +xterm.js +``` + +Each arrow is a place where the wrong invariant can cost you a "duplicate text" production bug. + +## rAF coalescing (the why) + +Bytes arrive at sub-frame granularity. Without coalescing, every chunk triggers a synchronous `term.write()` call and, for components subscribed to the buffer for previews, a React state update. Under heavy streaming, this pegs the renderer at <10 FPS during agent output. + +The store stages incoming chunks per agent key: + +```ts +const pending = new Map<string, string[]>() +const pendingFrames = new Map<string, number>() + +function appendPtyChunk(key: string, chunk: string): void { + const queue = pending.get(key) + if (queue) queue.push(chunk) + else pending.set(key, [chunk]) + if (pendingFrames.has(key)) return + pendingFrames.set(key, requestAnimationFrame(() => flushPending(key))) +} +``` + +Subscribers see at most one notification per animation frame, with the new tail. The full buffer accumulates separately (capped at 10k chunks). + +## Tail-only listener semantics + +Notifications carry **only the new chunks since last flush**, not the full buffer history: + +```ts +type Listener = (newChunks: string[]) => void +``` + +This sidesteps the trim race: if a subscriber held a `writtenChunks` offset and the buffer trims 50 entries off the head, the subscriber slicing `buffer.slice(writtenChunks)` would slice past the new tail's end and drop the freshly-added chunks. Tail-only listeners can't hit that bug — they receive the literal new chunks and trust the producer to handle accounting. + +Cost: a subscriber that subscribes mid-stream must explicitly catch up against the canonical buffer: + +```ts +unsubBuffer = subscribePtyBuffer(key, writeChunks) +const buffered = getPtyChunks(key) +writeChunks(buffered.slice(writtenChunks)) // initial catch-up +``` + +## Snapshot-vs-replay race + +`broker.attachTerminal()` returns a snapshot of the screen state at some T₁ (the broker's view of the PTY's current visible buffer). Between T₁ and the moment pear writes the snapshot, more chunks arrive into the local buffer. Without care: + +1. Write snapshot (state through T₁). +2. Set `writtenChunks = 0`. +3. Subscribe; subscriber replays `buffer.slice(0)` — the entire buffer, including the T₁ chunks already covered by the snapshot. + +Result: duplicate text. + +Fix: synchronously drain pending into the canonical buffer BEFORE capturing the baseline: + +```ts +term.write(result.snapshot.screen) +flushPtyChunksNow(key) // synchronous drain +writtenChunks = getPtyChunks(key).length +``` + +`flushPtyChunksNow` cancels any pending rAF and flushes immediately: + +```ts +export function flushPtyChunksNow(key: string): void { + const handle = pendingFrames.get(key) + if (handle !== undefined) { + cancelAnimationFrame(handle) + pendingFrames.delete(key) + } + if (pending.has(key)) flushPending(key) +} +``` + +## SIGWINCH semantics + +When the PTY is resized, the running TUI receives SIGWINCH and redraws. This is normal. The bug is **spurious** SIGWINCH: + +- A `ResizeObserver` fires for every dragged pixel during a layout drag. Without debouncing + same-size-skip, every pixel produces an IPC `resizePty` → SIGWINCH → TUI redraw. The TUI's redraw output spams the chunk stream, the user sees visible judder. + +Discipline: + +```ts +let lastSentRows = -1, lastSentCols = -1 +function fitAndSync(): void { + const size = fitAddon.fit() + if (!size) return + if (size.rows !== lastSentRows || size.cols !== lastSentCols) { + lastSentRows = size.rows + lastSentCols = size.cols + pear.broker.resizePty(projectId, agentName, size.rows, size.cols) + } +} +``` + +Plus: debounce the ResizeObserver (75ms trailing), skip zero-size entries (allotment drags + `display:none` produce them). + +## The SIGWINCH bounce (initial-attach case) + +Ink-based TUIs (Claude Code) lock in their row/col count from initial state and only recompute on a winsize **change**, not initial value. If pear attaches and tells the broker `rows=42` and the TUI's notion is `rows=42`, no change happens — but the TUI was started with a default size and never matched the actual terminal. Subsequent redraws use the stale row count. + +Fix: 200ms after `attachAndSeed()` completes, fire a one-pixel bounce: + +```ts +await broker.resizePty(projectId, name, rows - 1, cols) +// re-read in case user resized during the await +const currentRows = liveTerm.rows +const currentCols = liveTerm.cols +await broker.resizePty(projectId, name, currentRows, currentCols) +``` + +The intermediate `rows - 1` is NOT recorded in `lastSentRows/Cols` — the cache check would otherwise skip the bounce. The TUI gets one fake winsize change and recomputes its layout. + +## Trim cap accounting + +The buffer caps at `MAX_PTY_BUFFER_CHUNKS = 10_000`. When `combined.length > MAX`, trim the oldest entries: + +```ts +const trimmed = combined.length > MAX ? combined.slice(combined.length - MAX) : combined +``` + +A subscriber that holds `writtenChunks = N` against the pre-trim buffer will, on the next listener notification, attempt to read at index N which is past the trimmed array's head. This is exactly why **tail-only listener semantics** are correct: the listener doesn't slice, it receives the tail explicitly. The buffer accumulation and subscriber accounting are decoupled. + +If a subscriber needs to recover the full history (e.g. re-attach), they should: + +```ts +const canonical = getPtyChunks(key) +// canonical is the post-trim canonical buffer; just replay from index 0 +``` + +…and reset their own `writtenChunks` to `canonical.length` after replay. + +## Channel separation: pty-chunk vs broker:event + +PTY chunks ride a dedicated IPC channel (`broker:pty-chunk` in pear), not the general `broker:event` stream. The reason: structured-clone cost. Every `broker:event` is wrapped in a metadata envelope (kind, projectId, etc.) which Electron's IPC has to deep-clone. For per-keystroke streaming, that overhead pegs the main process. + +`broker:pty-chunk` is `(projectId, name, chunk)` — three primitives, minimal cloning. If you find yourself routing PTY data through `broker:event` for any reason, that's the regression that costs you smooth typing. + +## Predictive echo + +Optional layer between the chunk arrival and `liveTerm.write`. Engine intercepts user input, optimistically renders predicted glyphs to the live terminal, and reconciles when authoritative server output arrives. Adaptive on measured echo latency (`getInputSrtt`), so on a fast local link it stays dormant (invisible). + +Key invariants: + +- Lives in a **headless xterm clone** (separate Terminal instance with `scrollback: 0`) that tracks confirmed cursor state. Don't read cursor position from the LIVE terminal to make prediction decisions — the live terminal contains optimistic glyphs and lies. +- Construction takes a `write` callback. The engine writes erase sequences (`\x1b[K`) for predicted glyphs and re-renders on reconciliation through this callback. Don't bypass it — direct `liveTerm.write()` plus engine writes will double-write. +- `onResize(cols, rows)` MUST be called when the live terminal resizes; otherwise the headless model's column counting drifts from reality and prediction lands at the wrong column. +- The `getInputSrtt` callback is captured at engine construction. If pear's React lifecycle creates a new `inputSrttRef` per hook mount but the engine survives across mounts, the engine reads a frozen ref forever. Trampoline through a runtime-owned slot and expose `setInputSrttGetter()` for re-binding. + +## Companion reading + +- `xterm-internals-and-renderers.md` — what consumes the chunks +- `ansi-vt-escape-sequences.md` — what the chunks contain +- `renderer-bug-class-triage.md` — pattern-match symptoms to specific pipeline bugs diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md new file mode 100644 index 00000000..9e7a0426 --- /dev/null +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md @@ -0,0 +1,105 @@ +# xterm-internals-and-renderers + +The xterm.js parser pipeline, renderer trade-offs, addon discipline, and the bits of internal state that matter when something paints wrong. + +## Parser pipeline + +Bytes from `term.write(data)` flow through: + +``` +Parser → InputHandler → Buffer → Renderer +``` + +- **Parser**: state machine over the VT/ANSI byte stream. Recognizes CSI / OSC / DCS / SS3 / ESC sequences and decodes their parameters. +- **InputHandler**: dispatches parsed commands to buffer mutations (cursor movement, text writes, mode toggles). +- **Buffer**: the in-memory grid. Two buffers exist — `buffer.normal` (main, with scrollback) and `buffer.alternate` (alt-screen, no scrollback, used by full-screen TUIs like vim/htop). +- **Renderer**: paints the buffer's visible viewport to the DOM (DOM/Canvas/WebGL). + +Behavioral consequences: + +- Cursor movement (`CSI <r>;<c> H`) operates on the buffer grid, not on what's visible. Move up 60 lines from row 1 → cursor pegs at row 1, can't go negative. +- Alt-screen entry/exit (DECSET ?1049 h/l) swaps buffer references. State save/restore happens at the same time. Tools like vim look perfect on exit because the main buffer is exactly as you left it. +- Scrollback trim (when `scrollback: N` is exceeded) drops oldest lines off the top of `buffer.normal`. The cursor's absolute row position adjusts accordingly. TUIs using absolute positioning (cursor up by literal row count from current) break when their target row has been trimmed. + +## Renderer trade-offs + +xterm.js ships three renderer modes. Pick by failure profile, not by feature list. + +- **DOM renderer**: one DOM element per cell. Slowest under heavy streaming. Most compatible. Falls back to it when the Canvas/WebGL renderers crash. Diagnostics that bypass the renderer (extension dev tools inspecting cell contents) work cleanly. +- **Canvas renderer**: 2D canvas blits per row. Mid-tier perf. Fewer addons interact with it well. +- **WebGL renderer**: glyph atlas, GPU-accelerated. Fastest by an order of magnitude under heavy streaming. Failure modes: context loss (browser releases the GL context under memory pressure → entire renderer goes black), addon construction throw, glyph cache invalidation issues after theme change or font reload. + +Discipline: + +- **Load WebGL deferred** (next `requestAnimationFrame` after `term.open()`), so the terminal opens in DOM mode first and upgrades on next frame. Half-initialized WebGL is much worse than DOM rendering. +- **Persist DOM fallback decision**: on construction throw or `onContextLoss`, set a module-level `suggestedRenderer = 'dom'` and have subsequent runtimes skip WebGL for the session. Don't try to re-init WebGL on the same page once it has failed. +- **Theme swap with WebGL**: the glyph atlas caches with previous colors. Call `webgl.clearTextureAtlas()` (if available in your xterm version) after `term.options.theme = newTheme`. Otherwise glyphs paint stale until next refresh. + +## Addon lifecycle + +Each addon should be loaded **once** per Terminal instance: + +```ts +term.loadAddon(addon) +``` + +If you call `loadAddon` again with the same addon, behavior is undefined — most versions stack instances and leak GPU resources (WebGL) or duplicate event handlers. Pattern that avoids the bug: + +```ts +let webglAddon: WebglAddon | null = null +function loadWebgl(term: Terminal): void { + if (webglAddon) return // single-load discipline + try { + const addon = new WebglAddon() + addon.onContextLoss(() => { + addon.dispose() + suggestedRenderer = 'dom' + }) + term.loadAddon(addon) + webglAddon = addon + } catch { + suggestedRenderer = 'dom' + } +} +``` + +## Focus mode (DECSET ?1004) + +When a TUI emits `CSI ? 1004 h`, xterm starts reporting textarea focus changes back to the PTY as `\x1b[I` (focus-in) and `\x1b[O` (focus-out). Every programmatic `term.focus()` call fires `focusin` on the underlying textarea, which then emits `\x1b[I`. TUIs that redraw on focus (Claude Code's Ink-based tool cards) stack a new card per focus event. + +Diagnostic: if your symptom is "+1 stacked card per tab switch back" and your visibility effect calls `term.focus()`, this is your bug. Drop the auto-focus on visibility change; let `pointerdown` handle user-initiated focus. + +## Viewport vs scrollback + +Two coordinate systems to keep separate: + +- **Viewport**: the rows currently visible on screen. `term.refresh(0, term.rows - 1)` repaints viewport rows from buffer state. +- **Scrollback**: the entire buffer, including rows that have scrolled off the top. Indexed by `buffer.active.baseY` (cumulative). The visible viewport's top row is at `buffer.active.viewportY`. + +`scrollToBottom()` moves the viewport so its bottom equals `baseY + rows - 1`. It does NOT move the cursor. The cursor stays wherever it was in the buffer, possibly off-screen. + +Consequence: streaming output naturally pins the viewport at the bottom (auto-scroll). If the user is scrolled up reading history, you should NOT call `scrollToBottom()` on every chunk write — that yanks them away from history. Capture `wasPinned = isViewportPinnedToBottom(term)` BEFORE writing chunks, and only call `scrollToBottom()` if `wasPinned`. + +## Font measurement timing + +xterm measures cell width by inspecting the loaded font at `term.open()` time. If the configured font (e.g. JetBrains Mono) is still loading when `open()` is called, xterm measures the fallback font instead and locks in the wrong cell width. Glyphs look slightly smeared / misaligned until the next resize triggers re-measurement. + +Fix: + +```ts +await document.fonts.load('13px "JetBrains Mono"') +fitAddon.fit() +term.refresh(0, term.rows - 1) +``` + +Cap with a timeout so a missing font doesn't hang the open path forever. + +## Cursor blink + +The cursor blink lives on its own timer, separate from any render frame. `cursorBlink: true` + `cursorStyle: 'bar'` interact differently from `cursorStyle: 'block'` in some xterm versions — the bar is a CSS animation, the block is a paint timer. Don't assume behavior is identical across configurations. + +## Companion reading + +- `react-lifecycle-decoupling-and-token-ownership.md` — what owns the Terminal across React mounts +- `pty-broker-streaming-pipeline.md` — what feeds `term.write` and how it fails +- `ansi-vt-escape-sequences.md` — the sequence reference TUIs emit diff --git a/.agentworkforce/workforce/personas/terminal-renderer.json b/.agentworkforce/workforce/personas/terminal-renderer.json index 2101815a..2a763045 100644 --- a/.agentworkforce/workforce/personas/terminal-renderer.json +++ b/.agentworkforce/workforce/personas/terminal-renderer.json @@ -14,7 +14,38 @@ "performance" ], "description": "Specialist in rendering terminal views pixel-perfectly and at low latency in Electron / web renderers. Deep knowledge of xterm.js internals (DOM/Canvas/WebGL renderers, parser, addons, viewport vs scrollback, alt-screen buffer, cursor positioning, reflow on resize), the PTY pipeline (broker IPC, chunk batching, snapshot vs replay races, SIGWINCH semantics, predictive-echo reconciliation), ANSI/VT escape sequences (CSI/OSC/DCS, cursor movement, line clearing, DECSET modes including ?1004 focus events and ?1049 alt-screen, scroll regions, color rendering), font metrics and cell-grid stability (document.fonts load timing, JetBrains Mono / Fira Code measurement), GPU compositing pitfalls (canvas under transform/display:none, backdrop-filter stacking, will-change discipline), and the common bug classes: duplicate text on re-attach, scroll trail / ghost frames on hide/show, cell-width drift after font load, TUI redraw stacks from focus event reports, viewport-pin loss across refits, chunk loss at trim caps, listener leaks from re-subscription without unsubscribe. Diagnoses production rendering bugs by reading the diff + the relevant escape-sequence behavior, not by guessing. Engages instrumentation only after a clear failed-hypothesis chain. Resists architectural drift — fixes are minimal and targeted, with explanations of why the simpler approach was rejected.", - "skills": [], + "skills": [ + { + "id": "xterm-internals-and-renderers", + "source": "__assets/terminal-renderer/xterm-internals.md", + "description": "The xterm.js parser pipeline (Parser → InputHandler → Buffer → Renderer), DOM/Canvas/WebGL renderer trade-offs (failure profiles, deferred load, context-loss demotion), addon single-load discipline (multi-load stacks and leaks GPU resources), alt-screen vs main buffer (DECSET ?1049), DECSET ?1004 focus mode (the source of programmatic-focus-induced TUI redraws), viewport vs scrollback distinction (refresh vs scrollToBottom semantics), and font measurement timing (document.fonts.load before fit + refresh)." + }, + { + "id": "pty-broker-streaming-pipeline", + "source": "__assets/terminal-renderer/pty-broker-pipeline.md", + "description": "The end-to-end byte path from broker IPC to term.write: rAF coalescing per agent key (appendPtyChunk → pending → flushPending), tail-only listener semantics (sidesteps the trim race), the snapshot-vs-replay race during attach (flushPtyChunksNow + writtenChunks baseline), SIGWINCH semantics (debounced cache + bounce for Ink TUIs), 10k trim cap accounting, separation of broker:pty-chunk vs broker:event channels (structured-clone cost), and predictive-echo discipline (headless model, single write callback, getInputSrtt trampolining)." + }, + { + "id": "ansi-vt-escape-sequences", + "source": "__assets/terminal-renderer/ansi-vt-sequences.md", + "description": "Sequence taxonomy (CSI / OSC / DCS / character sets), cursor movement (CSI A/B/C/D, CSI H, save/restore), line+screen clearing (CSI J/K), scroll regions (CSI r), DECSET/DECRST modes (?25 cursor, ?1004 focus events, ?1049 alt screen, ?2004 bracketed paste, ?2026 synchronized output, ?1000/?1006 mouse), OSC for window title and hyperlinks, DCS for sixel, and the three TUI redraw patterns (cursor-positioning in place, alt-screen full-redraw, synchronized output) with their respective failure modes. Includes a chunk-preview helper for reading raw sequences from instrumentation." + }, + { + "id": "renderer-bug-class-triage", + "source": "__assets/terminal-renderer/bug-class-triage.md", + "description": "The symptom-to-bug-class lookup table built from the PR #158 catalogue. Triage axes: timing (immediate / N-tab-switches / after-resize / after-theme-change) and shape (duplicate text / ghosting / smeared glyphs / cursor drift / chat duplicates / blank canvas). For each row: most-likely cause + the file to open first. Use BEFORE reading source. Includes the two single-most-useful disambiguating questions to ask when a symptom is reported vaguely." + }, + { + "id": "react-lifecycle-decoupling-and-token-ownership", + "source": "__assets/terminal-renderer/lifecycle-decoupling.md", + "description": "The architecture pattern that eliminates 'duplicate text on tab switch' as a bug class: module-level runtime registry keyed by agent, React mounts/detaches DOM hosts only, runtime survives lifecycle changes, disposal goes through store-driven path on agent release. Token-based mount ownership (mount returns symbol token; detach takes token; stale tokens no-op) handles cross-tree React commit ordering and Strict Mode double-invocations. Companion patterns: clearOnDataIf identity-checked handler clear, parked host singleton container, dispose ordering (clearPtyBuffer before disposed flag flip)." + }, + { + "id": "fix-discipline-and-instrumentation", + "source": "__assets/terminal-renderer/fix-discipline.md", + "description": "The operating discipline that keeps renderer fixes from being speculative: the one-write-per-chunk invariant (count writes from broker arrival to term.write), read before guessing (name the specific step that produces the symptom or you don't have a fix), pattern-match symptom to bug class before opening any file, minimal-diff scope, instrument-don't-guess after two failed fixes (localStorage-gated diagnostic, reverted same cycle as the root-cause fix), defensive fixes name the UX trade-off explicitly, AGENTS.md regression-test requirement supersedes 'manual tests only' anti-goal, and 'battle-tested' definition (failure-class test runs RED without fix, GREEN with it)." + } + ], "inputs": { "TASK_DESCRIPTION": { "description": "The terminal-rendering problem to investigate or implement. Should include: (1) the symptom the user observes (duplicate text in scrollback, scroll trails, ghost frames, smeared glyphs, cursor misalignment, blank canvas on tab switch, etc.), (2) the trigger pattern (on first attach, only after N tab switches, only with specific TUI like Claude Code, only after resize, etc.), (3) the relevant files (renderer hook, runtime registry, buffer store, broker IPC), (4) the current hypotheses or fixes already attempted, (5) build constraints (xterm version, addons in use, renderer type DOM/Canvas/WebGL, Electron version). If insufficient, ask before reading code — a wrong hypothesis wastes more time than the question.", From dd35e3752f72324b82b2bc19f0ee73254af7b0ca Mon Sep 17 00:00:00 2001 From: Miya <khaliqgant+miya@gmail.com> Date: Mon, 8 Jun 2026 15:14:08 +0200 Subject: [PATCH 2/4] docs(persona): address review feedback on terminal-renderer skill files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multiple bot reviewers (coderabbit, cubic, gemini) flagged the same high-confidence issue: 'Companion reading' sections in each skill file referenced the original long ID names (xterm-internals-and-renderers.md, pty-broker-streaming-pipeline.md, etc.) rather than the actual short filenames (xterm-internals.md, pty-broker-pipeline.md). Cross-doc links were dead. Fixed: - All 6 'Companion reading' sections now reference the actual filenames - Two inline references (bug-class-triage.md, fix-discipline.md) - Character set designation sequences: \e(B / \e(0 (no space between '(' and final byte — gemini correctly noted my readability spaces were technically inaccurate) - 'one-pixel bounce' → 'one-row bounce' for the SIGWINCH bounce (it's a terminal-row resize, not a pixel resize) - 'Move up 60 lines' clarified to distinguish absolute (CSI <r>;<c> H) vs relative (CSI <n> A) cursor movement - Language tags on the untyped fenced blocks the bots flagged specifically (MD040 in xterm-internals, pty-broker-pipeline, ansi-vt-sequences, fix-discipline) H1 titles in each file (e.g. # xterm-internals-and-renderers) preserved — they're descriptive section names, not file references, and serve as the canonical skill IDs which the persona JSON also uses. --- .../__assets/terminal-renderer/ansi-vt-sequences.md | 10 +++++----- .../__assets/terminal-renderer/bug-class-triage.md | 8 ++++---- .../__assets/terminal-renderer/fix-discipline.md | 8 ++++---- .../__assets/terminal-renderer/lifecycle-decoupling.md | 6 +++--- .../__assets/terminal-renderer/pty-broker-pipeline.md | 10 +++++----- .../__assets/terminal-renderer/xterm-internals.md | 10 +++++----- 6 files changed, 26 insertions(+), 26 deletions(-) diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md index fce97f68..5e560eeb 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md @@ -10,7 +10,7 @@ All escape sequences start with `\x1b` (ESC, often shown as `\e` in logs). Then: - `\e]...` — **OSC** (Operating System Command). Window title, hyperlinks, cursor color. - `\eP...` — **DCS** (Device Control String). Sixel graphics, terminfo queries. - `\eN ...`, `\eO ...` — **SS2 / SS3**. Single-shift to alternate character sets. -- `\e( B`, `\e( 0` — **Character set designation**. `B` = US-ASCII, `0` = DEC special graphics. +- `\e(B`, `\e(0` — **Character set designation** (no space between `(` and the final byte). `B` = US-ASCII, `0` = DEC special graphics. - `\eM`, `\eD`, `\eE` — **Cursor controls outside CSI**. Reverse index, index, next line. CSI subforms: @@ -75,7 +75,7 @@ The modes that change behavior xterm-wide. Format: `\e[?<n>h` to set, `\e[?<n>l` **?2026 sync mode**: codex-1's TUI uses this. Each redraw frame is bracketed: -``` +```text \e[?2026h ← begin sync \e[H ← home cursor <card content> @@ -103,7 +103,7 @@ Most common: sixel graphics `\eP<params>q<data>\e\\`. Not all xterm builds suppo ## Character sets -`\e( B` = US-ASCII (default). `\e( 0` = DEC special graphics (box-drawing). Some legacy TUIs switch to the special set to draw `┌─┐│` style boxes; if you see weird non-ASCII characters in scrollback after a TUI exits without restoring `\e( B`, the set wasn't restored. +`\e(B` = US-ASCII (default). `\e(0` = DEC special graphics (box-drawing). Some legacy TUIs switch to the special set to draw `┌─┐│` style boxes; if you see weird non-ASCII characters in scrollback after a TUI exits without restoring `\e(B`, the set wasn't restored. ## TUI redraw patterns @@ -148,5 +148,5 @@ A chunk like `"\\e[H\\e[5;1H\\e[K"` reads as: "home cursor, move to row 5 col 1, ## Companion reading -- `pty-broker-streaming-pipeline.md` — how chunks arrive -- `renderer-bug-class-triage.md` — symptoms ↔ specific sequences +- `pty-broker-pipeline.md` — how chunks arrive +- `bug-class-triage.md` — symptoms ↔ specific sequences diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md index a72e02ab..e5d5287b 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md @@ -81,7 +81,7 @@ The lookup table from observed symptom to specific code path. Built from the PR 1. Read the symptom literally. "+1 card per tab switch back" not "duplication." 2. Find the matching row above. 3. Open the named file/code path BEFORE forming a fix hypothesis. -4. If the symptom doesn't match any row → instrument before guessing (see `fix-discipline-and-instrumentation.md`). +4. If the symptom doesn't match any row → instrument before guessing (see `fix-discipline.md`). ## When the symptom is reported vaguely @@ -99,6 +99,6 @@ Now you can pick the right file to open. ## Companion reading -- `pty-broker-streaming-pipeline.md` — the chunk path -- `ansi-vt-escape-sequences.md` — what cursor positioning sequences look like -- `fix-discipline-and-instrumentation.md` — what to do when the table doesn't match +- `pty-broker-pipeline.md` — the chunk path +- `ansi-vt-sequences.md` — what cursor positioning sequences look like +- `fix-discipline.md` — what to do when the table doesn't match diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md index bc0c6a1e..aa347542 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md @@ -32,7 +32,7 @@ The first is a fix hypothesis you can confirm or reject. The second is a paragra ## Pattern-match symptom to bug class -Before opening any file, use `renderer-bug-class-triage.md` to map the symptom to a candidate file. "+1 stacked card per tab switch" is not a triage prompt — it IS the bug class signal. Open the visibility-effect code path; don't read the chunk pipeline. +Before opening any file, use `bug-class-triage.md` to map the symptom to a candidate file. "+1 stacked card per tab switch" is not a triage prompt — it IS the bug class signal. Open the visibility-effect code path; don't read the chunk pipeline. When multiple classes fit, plan the cheapest disambiguation experiment first. Often: a temporary `console.log` at the chunk-write site counting writes per chunk. Five minutes of instrumentation beats two hours of speculative fixes. @@ -80,7 +80,7 @@ if (diagEnabled()) { Enable for capture: -``` +```js localStorage.setItem('PEAR_DIAG_PTY', '1') location.reload() // ... reproduce the bug @@ -148,5 +148,5 @@ If you can't fill in all five, you don't have a fix yet. Say so. Propose the sma ## Companion reading -- `renderer-bug-class-triage.md` — what to consult before opening any file -- `pty-broker-streaming-pipeline.md` + `xterm-internals-and-renderers.md` — the systems the discipline applies to +- `bug-class-triage.md` — what to consult before opening any file +- `pty-broker-pipeline.md` + `xterm-internals.md` — the systems the discipline applies to diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md index 8f6fb1a9..39cc33bd 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md @@ -244,6 +244,6 @@ dispose(): void { ## Companion reading -- `xterm-internals-and-renderers.md` — what survives in the runtime -- `pty-broker-streaming-pipeline.md` — the subscription lifecycle the runtime owns -- `renderer-bug-class-triage.md` — symptoms this pattern eliminates +- `xterm-internals.md` — what survives in the runtime +- `pty-broker-pipeline.md` — the subscription lifecycle the runtime owns +- `bug-class-triage.md` — symptoms this pattern eliminates diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md index 040d4ce2..49fd2f13 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md @@ -4,7 +4,7 @@ The end-to-end PTY byte path from the broker IPC into `term.write`, the races be ## Pipeline shape -``` +```text PTY child process ↓ raw bytes broker (main process) @@ -117,7 +117,7 @@ Plus: debounce the ResizeObserver (75ms trailing), skip zero-size entries (allot Ink-based TUIs (Claude Code) lock in their row/col count from initial state and only recompute on a winsize **change**, not initial value. If pear attaches and tells the broker `rows=42` and the TUI's notion is `rows=42`, no change happens — but the TUI was started with a default size and never matched the actual terminal. Subsequent redraws use the stale row count. -Fix: 200ms after `attachAndSeed()` completes, fire a one-pixel bounce: +Fix: 200ms after `attachAndSeed()` completes, fire a one-row bounce (resize by one terminal row, not one pixel): ```ts await broker.resizePty(projectId, name, rows - 1, cols) @@ -167,6 +167,6 @@ Key invariants: ## Companion reading -- `xterm-internals-and-renderers.md` — what consumes the chunks -- `ansi-vt-escape-sequences.md` — what the chunks contain -- `renderer-bug-class-triage.md` — pattern-match symptoms to specific pipeline bugs +- `xterm-internals.md` — what consumes the chunks +- `ansi-vt-sequences.md` — what the chunks contain +- `bug-class-triage.md` — pattern-match symptoms to specific pipeline bugs diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md index 9e7a0426..c6116587 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md @@ -6,7 +6,7 @@ The xterm.js parser pipeline, renderer trade-offs, addon discipline, and the bit Bytes from `term.write(data)` flow through: -``` +```text Parser → InputHandler → Buffer → Renderer ``` @@ -17,7 +17,7 @@ Parser → InputHandler → Buffer → Renderer Behavioral consequences: -- Cursor movement (`CSI <r>;<c> H`) operates on the buffer grid, not on what's visible. Move up 60 lines from row 1 → cursor pegs at row 1, can't go negative. +- Cursor movement operates on the buffer grid, not on what's visible. Absolute positioning (`CSI <r>;<c> H`) and relative movement (`CSI <n> A` for up N rows) both peg at the buffer edges — `CSI 60 A` from row 1 keeps cursor at row 1, can't go negative. - Alt-screen entry/exit (DECSET ?1049 h/l) swaps buffer references. State save/restore happens at the same time. Tools like vim look perfect on exit because the main buffer is exactly as you left it. - Scrollback trim (when `scrollback: N` is exceeded) drops oldest lines off the top of `buffer.normal`. The cursor's absolute row position adjusts accordingly. TUIs using absolute positioning (cursor up by literal row count from current) break when their target row has been trimmed. @@ -100,6 +100,6 @@ The cursor blink lives on its own timer, separate from any render frame. `cursor ## Companion reading -- `react-lifecycle-decoupling-and-token-ownership.md` — what owns the Terminal across React mounts -- `pty-broker-streaming-pipeline.md` — what feeds `term.write` and how it fails -- `ansi-vt-escape-sequences.md` — the sequence reference TUIs emit +- `lifecycle-decoupling.md` — what owns the Terminal across React mounts +- `pty-broker-pipeline.md` — what feeds `term.write` and how it fails +- `ansi-vt-sequences.md` — the sequence reference TUIs emit From 7897b076b1e042ad8203d916cfacd2a0760fe891 Mon Sep 17 00:00:00 2001 From: Khaliq <khaliqgant@gmail.com> Date: Mon, 8 Jun 2026 21:26:39 +0200 Subject: [PATCH 3/4] fix(persona): repoint terminal-renderer local skills to repo-root-relative paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 6 skills[] sources used paths relative to the personas dir (`__assets/terminal-renderer/*.md`), but the AgentWorkforce local-skill provider (persona-kit skills.js `localProvider`) resolves a relative `source` against `repoRoot = process.cwd()` (cli.js:1242/1346/1376) — the project root. So they resolved to `<root>/__assets/...`, which does not exist, and the `cp` install silently failed: the skills never loaded. Fix: use the full repo-root-relative path `.agentworkforce/workforce/personas/__assets/terminal-renderer/*.md`, matching the supported local-skill form documented in the provider (e.g. `.agentworkforce/workforce/skills/<name>.md`). All 6 now resolve to real files; JSON validates; skill files unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --- .../workforce/personas/terminal-renderer.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.agentworkforce/workforce/personas/terminal-renderer.json b/.agentworkforce/workforce/personas/terminal-renderer.json index 2a763045..784672c8 100644 --- a/.agentworkforce/workforce/personas/terminal-renderer.json +++ b/.agentworkforce/workforce/personas/terminal-renderer.json @@ -17,32 +17,32 @@ "skills": [ { "id": "xterm-internals-and-renderers", - "source": "__assets/terminal-renderer/xterm-internals.md", + "source": ".agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md", "description": "The xterm.js parser pipeline (Parser → InputHandler → Buffer → Renderer), DOM/Canvas/WebGL renderer trade-offs (failure profiles, deferred load, context-loss demotion), addon single-load discipline (multi-load stacks and leaks GPU resources), alt-screen vs main buffer (DECSET ?1049), DECSET ?1004 focus mode (the source of programmatic-focus-induced TUI redraws), viewport vs scrollback distinction (refresh vs scrollToBottom semantics), and font measurement timing (document.fonts.load before fit + refresh)." }, { "id": "pty-broker-streaming-pipeline", - "source": "__assets/terminal-renderer/pty-broker-pipeline.md", + "source": ".agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md", "description": "The end-to-end byte path from broker IPC to term.write: rAF coalescing per agent key (appendPtyChunk → pending → flushPending), tail-only listener semantics (sidesteps the trim race), the snapshot-vs-replay race during attach (flushPtyChunksNow + writtenChunks baseline), SIGWINCH semantics (debounced cache + bounce for Ink TUIs), 10k trim cap accounting, separation of broker:pty-chunk vs broker:event channels (structured-clone cost), and predictive-echo discipline (headless model, single write callback, getInputSrtt trampolining)." }, { "id": "ansi-vt-escape-sequences", - "source": "__assets/terminal-renderer/ansi-vt-sequences.md", + "source": ".agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md", "description": "Sequence taxonomy (CSI / OSC / DCS / character sets), cursor movement (CSI A/B/C/D, CSI H, save/restore), line+screen clearing (CSI J/K), scroll regions (CSI r), DECSET/DECRST modes (?25 cursor, ?1004 focus events, ?1049 alt screen, ?2004 bracketed paste, ?2026 synchronized output, ?1000/?1006 mouse), OSC for window title and hyperlinks, DCS for sixel, and the three TUI redraw patterns (cursor-positioning in place, alt-screen full-redraw, synchronized output) with their respective failure modes. Includes a chunk-preview helper for reading raw sequences from instrumentation." }, { "id": "renderer-bug-class-triage", - "source": "__assets/terminal-renderer/bug-class-triage.md", + "source": ".agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md", "description": "The symptom-to-bug-class lookup table built from the PR #158 catalogue. Triage axes: timing (immediate / N-tab-switches / after-resize / after-theme-change) and shape (duplicate text / ghosting / smeared glyphs / cursor drift / chat duplicates / blank canvas). For each row: most-likely cause + the file to open first. Use BEFORE reading source. Includes the two single-most-useful disambiguating questions to ask when a symptom is reported vaguely." }, { "id": "react-lifecycle-decoupling-and-token-ownership", - "source": "__assets/terminal-renderer/lifecycle-decoupling.md", + "source": ".agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md", "description": "The architecture pattern that eliminates 'duplicate text on tab switch' as a bug class: module-level runtime registry keyed by agent, React mounts/detaches DOM hosts only, runtime survives lifecycle changes, disposal goes through store-driven path on agent release. Token-based mount ownership (mount returns symbol token; detach takes token; stale tokens no-op) handles cross-tree React commit ordering and Strict Mode double-invocations. Companion patterns: clearOnDataIf identity-checked handler clear, parked host singleton container, dispose ordering (clearPtyBuffer before disposed flag flip)." }, { "id": "fix-discipline-and-instrumentation", - "source": "__assets/terminal-renderer/fix-discipline.md", + "source": ".agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md", "description": "The operating discipline that keeps renderer fixes from being speculative: the one-write-per-chunk invariant (count writes from broker arrival to term.write), read before guessing (name the specific step that produces the symptom or you don't have a fix), pattern-match symptom to bug class before opening any file, minimal-diff scope, instrument-don't-guess after two failed fixes (localStorage-gated diagnostic, reverted same cycle as the root-cause fix), defensive fixes name the UX trade-off explicitly, AGENTS.md regression-test requirement supersedes 'manual tests only' anti-goal, and 'battle-tested' definition (failure-class test runs RED without fix, GREEN with it)." } ], From 57308ef3a702ddbbbebb0826cb2c0d8710c59459 Mon Sep 17 00:00:00 2001 From: Khaliq <khaliqgant@gmail.com> Date: Mon, 8 Jun 2026 21:32:57 +0200 Subject: [PATCH 4/4] fix(persona): add SKILL frontmatter to terminal-renderer skill files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Claude skill is only valid/discoverable with a YAML frontmatter block (name + description). The 6 terminal-renderer skill .md files started straight at the H1 with no frontmatter, so they would not register as skills once installed. Add `name` + a one-line "Use when…" `description` to each, matching the workforce personas/skills/<name>/SKILL.md shape. `name` is set to each file's basename stem (ansi-vt-sequences, bug-class-triage, fix-discipline, lifecycle-decoupling, pty-broker-pipeline, xterm-internals) — the install dir the local-skill provider derives from the source basename, which a Claude skill's frontmatter name must match. Files stay where they are; the earlier path fix is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --- .../personas/__assets/terminal-renderer/ansi-vt-sequences.md | 5 +++++ .../personas/__assets/terminal-renderer/bug-class-triage.md | 5 +++++ .../personas/__assets/terminal-renderer/fix-discipline.md | 5 +++++ .../__assets/terminal-renderer/lifecycle-decoupling.md | 5 +++++ .../__assets/terminal-renderer/pty-broker-pipeline.md | 5 +++++ .../personas/__assets/terminal-renderer/xterm-internals.md | 5 +++++ 6 files changed, 30 insertions(+) diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md index 5e560eeb..ea250a1b 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/ansi-vt-sequences.md @@ -1,3 +1,8 @@ +--- +name: ansi-vt-sequences +description: Use when reading raw escape sequences from instrumentation or reasoning about cursor/clear/scroll-region/DECSET behavior and the three TUI redraw patterns (cursor-positioning in place, alt-screen full-redraw, synchronized output) behind a terminal rendering bug. +--- + # ansi-vt-escape-sequences The byte-level command vocabulary TUIs use to drive the terminal. When you can read the chunks, you can name the bug. diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md index e5d5287b..4292c103 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/bug-class-triage.md @@ -1,3 +1,8 @@ +--- +name: bug-class-triage +description: Use BEFORE reading source when a terminal rendering symptom is reported — maps the symptom (duplicate text, ghosting, smeared glyphs, cursor drift, blank canvas) by timing and shape to the most-likely cause and the file to open first. +--- + # renderer-bug-class-triage The lookup table from observed symptom to specific code path. Built from the PR #158 catalogue of fixes and the bugs found while shipping it. Use this **before** reading source — it disambiguates which file to open. diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md index aa347542..aaae1f9c 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/fix-discipline.md @@ -1,3 +1,8 @@ +--- +name: fix-discipline +description: Use when fixing any terminal renderer bug — the one-write-per-chunk invariant, read-before-guessing, minimal-diff scope, instrument-don't-guess after two failed fixes, and the battle-tested/regression-test bar that keep fixes from being speculative. +--- + # fix-discipline-and-instrumentation The operating discipline that keeps renderer fixes from being speculative or regressing. Read this before writing any fix. diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md index 39cc33bd..022ca0fe 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/lifecycle-decoupling.md @@ -1,3 +1,8 @@ +--- +name: lifecycle-decoupling +description: Use when terminal duplicate-text-on-tab-switch or mount/dispose races appear — the module-level runtime registry plus token-based mount ownership pattern that decouples xterm/WebGL/PTY from the React component lifecycle. +--- + # react-lifecycle-decoupling-and-token-ownership The architecture pattern that takes "duplicate text on tab switch" off the table as an entire bug class. xterm + WebGL + PTY subscription must NOT be torn down and rebuilt on React component lifecycle changes. diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md index 49fd2f13..ec84f83e 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/pty-broker-pipeline.md @@ -1,3 +1,8 @@ +--- +name: pty-broker-pipeline +description: Use when diagnosing the PTY byte path from broker IPC to term.write — rAF chunk coalescing, the snapshot-vs-replay race on attach, SIGWINCH/resize redraws, trim-cap chunk loss, and predictive-echo reconciliation. +--- + # pty-broker-streaming-pipeline The end-to-end PTY byte path from the broker IPC into `term.write`, the races between snapshot and stream, and the discipline that prevents the duplicate-text bug class. diff --git a/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md b/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md index c6116587..4aa50604 100644 --- a/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md +++ b/.agentworkforce/workforce/personas/__assets/terminal-renderer/xterm-internals.md @@ -1,3 +1,8 @@ +--- +name: xterm-internals +description: Use when a terminal rendering bug implicates xterm.js internals — the parser pipeline, DOM/Canvas/WebGL renderer choice, addon lifecycle, alt-screen vs main buffer, focus mode (DECSET ?1004), viewport vs scrollback, or font-measurement timing. +--- + # xterm-internals-and-renderers The xterm.js parser pipeline, renderer trade-offs, addon discipline, and the bits of internal state that matter when something paints wrong.