diff --git a/.gitignore b/.gitignore index d94a2b91302..1874a068079 100644 --- a/.gitignore +++ b/.gitignore @@ -100,11 +100,9 @@ docs/reference/react-performance-audit.md validation-screenshots/ .stably-browser -# PR verification evidence screenshots are regenerated on demand (notes/artifacts -# manually, pr-evidence/ by the ORCA_CAPTURE_EVIDENCE e2e run) and must not be -# committed. -notes/artifacts/ -pr-evidence/ +# Local scratch notes and PR evidence screenshots (not part of the product). +/notes/ +/pr-evidence/ # Playwright test-results/ @@ -125,5 +123,5 @@ src/renderer/src/i18n/locales/.ko-catalog-cache.json src/renderer/src/i18n/locales/.ja-catalog-cache.json src/renderer/src/i18n/locales/.es-catalog-cache.json -# Bench result JSONs are working artifacts; headline numbers live in notes/terminal-performance-initiative.md +# Bench result JSONs are working artifacts tools/benchmarks/results/terminal-pipeline-*.json diff --git a/mobile/src/components/BottomDrawer.tsx b/mobile/src/components/BottomDrawer.tsx index 4e24e8c2e06..eb23e53a554 100644 --- a/mobile/src/components/BottomDrawer.tsx +++ b/mobile/src/components/BottomDrawer.tsx @@ -24,6 +24,7 @@ import Animated, { } from 'react-native-reanimated' import { colors, spacing } from '../theme/mobile-theme' import { resolveBottomDrawerMounted } from './bottom-drawer-mount-state' +import { useInsideBottomDrawerModalHost } from './bottom-drawer-modal-host' import { useResponsiveLayout } from '../layout/responsive-layout' const DISMISS_THRESHOLD = 80 @@ -107,6 +108,7 @@ function MountedBottomDrawer({ // center it horizontally. Vertical bottom-anchoring (and all the drag/keyboard // transforms below) is unchanged, so phone behavior stays identical. const { isWideLayout, modalMaxWidth } = useResponsiveLayout() + const insideModalHost = useInsideBottomDrawerModalHost() useEffect(() => { if (visible) { @@ -265,104 +267,114 @@ function MountedBottomDrawer({ return { opacity: progress.value * dragFade } }) - // Why: rendering through a full-screen Modal lifts the sheet into its own - // native window so it always covers the viewport — even when the drawer is - // mounted deep inside a ScrollView, where a plain absolute overlay anchors to - // the scrolled content and clips the sheet. The Modal stays mounted (visible) - // for the whole life of MountedBottomDrawer so the reanimated exit animation - // runs before the parent unmounts us; show/hide is driven by `progress`, so - // animationType stays "none". onRequestClose handles the Android back button. + // Why: the sheet renders through a full-screen native window (its own Modal + // below, or the shared BottomDrawerModalHost) so it always covers the viewport + // — even when mounted deep inside a ScrollView, where a plain absolute overlay + // anchors to the scrolled content and clips the sheet. Show/hide is driven by + // `progress` (animationType "none") so the reanimated exit animation runs before + // the parent unmounts us. + const overlay = ( + + + + + + + + + {!contentScrollable ? ( + <> + + + + + + {children} + + ) : dragContentToDismiss ? ( + <> + + + + + + + + + + {children} + + + + + + ) : ( + <> + + + + + + + {children} + + + )} + + + + + + ) + + // Why: inside a BottomDrawerModalHost the host owns the single native Modal; + // rendering our own would stack modals and reintroduce the iOS present/dismiss + // race the host exists to avoid. The host handles the Android back button. + if (insideModalHost) { + return overlay + } + return ( - - - - - - - - - {!contentScrollable ? ( - <> - - - - - - {children} - - ) : dragContentToDismiss ? ( - <> - - - - - - - - - - {children} - - - - - - ) : ( - <> - - - - - - - {children} - - - )} - - - - - + {overlay} ) } diff --git a/mobile/src/components/NewWorktreeModal.tsx b/mobile/src/components/NewWorktreeModal.tsx index 75b18fcd1fb..09a0914d4aa 100644 --- a/mobile/src/components/NewWorktreeModal.tsx +++ b/mobile/src/components/NewWorktreeModal.tsx @@ -15,6 +15,7 @@ import type { RpcClient } from '../transport/rpc-client' import type { RpcResponse, RpcSuccess } from '../transport/types' import { colors, spacing, radii, typography } from '../theme/mobile-theme' import { BottomDrawer, BOTTOM_DRAWER_HIDE_DURATION_MS } from './BottomDrawer' +import { BottomDrawerModalHost } from './bottom-drawer-modal-host' import { PickerListDrawer } from './PickerListDrawer' import { MobileAgentIcon } from './MobileAgentIcon' import { getSuggestedCreatureName } from './worktree-name-suggestion' @@ -814,7 +815,23 @@ function NewWorktreeModalContent({ } return ( - <> + // Why: hosting the form and every picker in one persistent native Modal makes + // form → repo/agent transitions in-window view swaps, avoiding the iOS + // dismiss-then-present race that left the dropdowns unresponsive. Native back + // closes the flow from the form, routes the trust prompt through its in-flight + // guard, and otherwise returns to the form from a picker. + { + if (drawerView === 'form') { + onClose() + } else if (drawerView === 'trust') { + closeSetupTrust() + } else { + transitionDrawer('form') + } + }} + > Create Workspace @@ -1036,7 +1053,7 @@ function NewWorktreeModalContent({ {/* Why: list drawers stay outside the form's ScrollView, and the transition - state prevents overlapping native modals from swallowing iOS taps. */} + state lets each hosted overlay finish hiding before the next appears. */} - + ) } diff --git a/mobile/src/components/bottom-drawer-modal-host.tsx b/mobile/src/components/bottom-drawer-modal-host.tsx new file mode 100644 index 00000000000..b31a31136e0 --- /dev/null +++ b/mobile/src/components/bottom-drawer-modal-host.tsx @@ -0,0 +1,40 @@ +import { createContext, useContext, type ReactNode } from 'react' +import { Modal } from 'react-native' + +const BottomDrawerModalHostContext = createContext(false) + +/** True when a BottomDrawer is rendered inside a shared BottomDrawerModalHost and + * must therefore skip its own native Modal (the host owns the single Modal). */ +export function useInsideBottomDrawerModalHost(): boolean { + return useContext(BottomDrawerModalHostContext) +} + +type Props = { + visible: boolean + onRequestClose: () => void + children: ReactNode +} + +// Why: iOS cannot reliably dismiss one native modal and present another in the same +// beat. Flows that swap between sibling drawer modals (e.g. the Create Workspace form +// → its repository/agent pickers) dropped the incoming modal, leaving the sheet dead +// to taps. Hosting every drawer in ONE persistent native Modal makes those swaps +// in-window view changes instead, so no present/dismiss race can eat the transition. +export function BottomDrawerModalHost({ visible, onRequestClose, children }: Props) { + if (!visible) { + return null + } + return ( + + + {children} + + + ) +} diff --git a/mobile/src/source-control/mobile-pr-create.ts b/mobile/src/source-control/mobile-pr-create.ts index 675df20bedb..b9a3817f4d2 100644 --- a/mobile/src/source-control/mobile-pr-create.ts +++ b/mobile/src/source-control/mobile-pr-create.ts @@ -66,6 +66,8 @@ export function getMobilePrCreateBlockMessage(prefill: MobilePrPrefill): string return `A ${copy.reviewLabel} already exists for this branch.` case 'fork_head_unsupported': return `Creating a ${copy.reviewLabel} from this fork is not supported.` + case 'base_not_on_remote': + return `Push the base branch before creating a ${copy.reviewLabel}.` case 'needs_push': case null: case undefined: diff --git a/notes/garble-fuzz-divergences.md b/notes/garble-fuzz-divergences.md deleted file mode 100644 index dde77ac2450..00000000000 --- a/notes/garble-fuzz-divergences.md +++ /dev/null @@ -1,296 +0,0 @@ -# Garble differential fuzz — divergence log - -Findings from the HeadlessEmulator-vs-renderer-twin differential fuzz -(`src/main/daemon/headless-emulator-fidelity.fuzz.test.ts`). Each divergence is -a case where restoring a hidden terminal from its main-side snapshot -(`serialize → replay`, exactly as `applyMainBufferSnapshot` does on reveal) -produces a screen that differs from an always-visible renderer terminal fed the -same bytes. Any such diff is a user-visible garble on reveal. - -## Method - -- Corpus: seeded agent-TUI byte streams (`buildAgentTuiStreamOps`), 3 pane - sizes, PTY-style random chunk splitting. -- Differential: production `HeadlessEmulator` snapshot replayed into a fresh - renderer-parity terminal, compared cell-by-cell (text, per-cell style, - cursor, modes, scrollback) against an always-visible renderer-parity twin. -- Parity confirmed: `createRendererParityTerminal` mirrors the renderer pane's - buffer-affecting options exactly — `scrollback: 5000`, `allowProposedApi`, - `vtExtensions.kittyKeyboard`, `Unicode11Addon`, Orca ZWJ provider (verified - against `buildDefaultTerminalOptions` in - `src/renderer/src/lib/pane-manager/pane-terminal-options.ts` and - `pane-dom-creation.ts`). Render-only options (`minimumContrastRatio`, - `drawBoldTextInBrightColors`, font, cursor, scrollbar) do not alter stored - cell attributes, so their omission is not a source of false diffs. - `windowsMode` is unset in both (matches renderer). Addon versions: - `@xterm/addon-serialize` / `@xterm/headless` / `@xterm/addon-unicode11` all - `*-beta.287` (headless `6.1.0-beta.287`). -- Scan: seeds 1..2000. Every divergence is either the known serialize-wrap bug - (predicate `bufferHasSerializeHostileWrappedRow`, tolerated + counted) or is - listed below. - -## Inventory - -| bug | found by | seeds | classification | -| --- | --- | --- | --- | -| A — serialize wrap null-cell | fidelity (suite 1) | 31, 157, 171, 207, 423, 426, 502, 801, 815, 826, 865, 881, 923, 977, 1004, 1119, 1142, 1238, 1241, 1318, 1351, 1374, 1532, 1601, 1657, 1728, 1770 (27 in 1..2000) | (a) real serialize bug, pre-documented + pinned — STILL OPEN | -| B — SGR bold loss (`1;22`) | fidelity (suite 1) | 435, 770, 1321 | (a) real serialize bug — FIXED by the addon patch (intensity-group SGR reorder, config/patches) | -| C — cursor off-by-one at right margin | fidelity (suite 1) | 454, 1696 | (a) real serialize bug — FIXED Orca-side (absolute-cursor epilogue, serializeWithAbsoluteCursor) | -| D — DECSC saved-cursor lost across reveal | reconciliation (suite 2) | seed 3 | (a) real snapshot limitation — FIXED (snapshot re-saves the DECSC register, readSavedCursorRegister) | -| E — snapshot boundary mid-escape-sequence | reconciliation (suite 2) | seed 4 (+~24% of corpus) | (a) real snapshot limitation — FIXED (pendingEscapeTailAnsi carried out-of-band, terminal-partial-escape-tail.ts) | - -Status update (fix/snapshot-decsc-midescape): B/C/D/E repros are UNSKIPPED and -their corpus tolerances removed — only Bug A remains tolerated + counted. Bug D -carries position only (saved SGR/charset are not re-established — the synthetic -ESC 7 saves the serializer's final pen). Bug E's pending tail is a separate -snapshot field written LAST by restorers because any later ESC (e.g. the -post-replay reset) would abort the dangling sequence; its bytes are already -counted by the snapshot seq, so tail-slice arithmetic is unchanged. - -All five bug classes are reproduced by dedicated minimal `test.skip` repros so -they cannot silently regress, AND each is tolerated + counted by its suite's -corpus loop so deep mode surfaces only genuinely NEW divergences: - -- Suite 1 (fidelity): Bug A via `bufferHasSerializeHostileWrappedRow`, Bug B via - `snapshotHasSelfCancellingBoldReset` (matches the `1;22` in the serialized - snapshot), Bug C via `isMarginWrapPendingCursorOffByOne` (cursor x-1 with a - full-width content row). Green at the default 300 and at `FUZZ_ITERATIONS=2000`. -- Suite 2 (reconciliation): Bug E via `prefixEndsMidSequence`. Bug D and the - Bug-C cursor cascade are kept out of the corpus by an append-only racing tail - (no DECSC/cursor motion) and pinned only as standalone repros. Green at the - default 200 and at `FUZZ_ITERATIONS=1000`. - -Each tolerance has a `< max(3, ITERATIONS*0.5)` guard so a predicate that starts -tripping on most seeds fails the suite instead of silently swallowing it. - -Seed 113 (called out in the handoff as a "DECSC/DECRC detour writing colored -text mid-line") does not diverge on the current harness. It is a `savedCursor -Detour` op seed; DECSC/DECRC SGR carry is correctly preserved by both the -emulator and the serializer here. It was most likely an earlier observation -folded into Bug C (the DECRC cases 1696 also involve `\x1b7`/`\x1b8`), or a -transient during harness construction. No live divergence at 113. - ---- - -## Bug A — SerializeAddon drops null cells at a soft-wrap boundary - -**Classification: (a) real `@xterm/addon-serialize` bug.** Pre-existing; found -and minimized by the prior agent, pinned by two `test.skip` repros in the fuzz -suite (V1 seed 31, V2 seed 157). Full mechanism documented in -`bufferHasSerializeHostileWrappedRow` and the suite's headline comment. - -- **V1 (cell loss):** a wrapped continuation row starting with a NULL cell - passes the addon's wrap-validity ternary, gets skipped with `CUF` which clamps - at the right margin, overwriting the previous row's last cell and shifting the - tail left by one. `cols=20: 'ABCDEFGHIJKLMNOPQRSTUVWXYZ12\r\n' then '\x1b[1A\x1b[1K'`. -- **V2 (stray `-` filler):** a wrapped pair whose source row is entirely null - takes the forced-wrap "magic" path; cleanup emits `ESC[0C` (param 0 → 1) so - the ECH erase lands one cell right and the first filler `-` survives. - -**Impact:** any snapshot consumer (hidden reveal, parked-tab reveal, sleep/wake, -mobile subscribe replay) paints lost/shifted characters or stray `-` fillers -when a TUI erases inside a soft-wrapped line. Tolerated + counted by the suite; -unskip the repros when upstream fixes or a local serialize post-processor lands. - ---- - -## Bug B — SerializeAddon loses BOLD when serializing a dim→bold-only transition - -**Classification: (a) real `@xterm/addon-serialize` bug.** New finding. - -**Seeds:** 435, 770 (alt-screen), 1321 (minimal, 2 ops). - -**Minimal repro (isolated, no fuzz corpus needed), cols=20:** - -``` -live bytes: "\x1b[2mA\x1b[22m\x1b[1mB" -SerializeAddon → : "\x1b[2mA\x1b[1;22mB" -live cell B: bold=1 dim=0 (style flags 100000) -restored cell B: bold=0 dim=0 (style flags 000000) ← BOLD LOST -``` - -**Mechanism:** cell A is dim, cell B is bold-only. The serializer diffs the pen -from A (dim on) to B (bold on, dim off). To clear dim it appends SGR 22 — but in -xterm/ECMA-48 **SGR 22 resets *both* bold and dim** (`normalIntensity`). So the -emitted `\x1b[1;22m` sets bold then immediately clears it: the restored cell is -neither dim nor bold. Verified directly: writing `\x1b[1;22mX` yields `bold=0`. -(`\x1b[1;2m` — the same-cell dim+bold case — round-trips fine, so the bug is -specific to a dim-cell → bold-only-cell attribute transition.) - -**Why it garbles a real pane:** agent TUIs routinely draw a dim body line then a -bold status/spinner line (Claude Code, Codex). On the live screen the status -line is bold; after a hide→reveal snapshot restore it renders normal-weight. -The seed-1321 live row `⠦ bash: pnpm typecheck` is bold live, non-bold restored. - -**Repro test:** `headless-emulator-fidelity.fuzz.test.ts` → -`it.skip('preserves bold when serializing a dim cell followed by a bold-only cell')`. -Tolerated + counted in the corpus via `snapshotHasSelfCancellingBoldReset`. - ---- - -## Bug C — SerializeAddon cursor restore is off-by-one when the last content row fills the right margin - -**Classification: (a) real `@xterm/addon-serialize` bug.** New finding. - -**Seeds:** 454 (minimal, plain CUP), 1696 (DECSC/DECRC + wide CJK). - -**Minimal repro (isolated, pure serializer replay), cols=10:** - -``` -live bytes: "0123456789\x1b[3;5H" (fill row 0 to the margin, CUP to r3c5) -SerializeAddon → : "0123456789\x1b[2B\x1b[6D" -live cursor: { x: 4, y: 2 } -restored cursor: { x: 3, y: 2 } ← ONE COLUMN SHORT -``` - -Control (`"012\x1b[3;5H"` — row 0 not full) serializes to `"012\x1b[2B\x1b[1C"` -and round-trips the cursor exactly, isolating the trigger to a full-width final -content row. - -**Mechanism:** after emitting a row filled to exactly `cols`, xterm is left in -the *wrap-pending* state (cursor visually on the last column, logically "one -past"). The serializer computes its final cursor-restore as relative -`CUD`/`CUB` moves from that ambiguous position; the horizontal delta is computed -one column short, so the restored cursor lands at `x-1`. Reproduced with pure -`serializeAddon.serialize()` replay into a fresh terminal — **no Orca preamble -or normalization involved**, confirming it is upstream, not Orca's snapshot -path. - -**Why it garbles a real pane:** the cursor is where the next keystroke echoes -and where the block/bar cursor is drawn. On reveal of a TUI whose bottom line -reached the right edge (wide status lines, long prompts), the cursor sits one -cell left of where the live pane had it — visible as a mispositioned prompt -caret or spinner, and subsequent input can overwrite the wrong cell. - -**Repro test:** `headless-emulator-fidelity.fuzz.test.ts` → -`it.skip('restores the cursor exactly when the last content row fills the right margin')`. -Tolerated + counted in the corpus via `isMarginWrapPendingCursorOffByOne`. - ---- - -## Bug D — snapshot does not preserve the DECSC saved-cursor register across a hide/reveal boundary - -**Classification: (a) real bug — a structural snapshot limitation.** New -finding, surfaced by the reveal-reconciliation fuzz (suite 2), not the fidelity -fuzz. - -**Minimal repro (cols=20):** - -``` -hidden bytes: "AB\x1b7\x1b[4;10HCD" (write AB, DECSC saves cursor at r0c2, - move to r3c9, write CD) -tail bytes: "\x1b8X" (DECRC restores the saved cursor, write X) - -live (always visible): rows ["ABX", " CD"] cursor { x: 3, y: 0 } -reveal (snapshot+tail): rows ["XB", " CD"] cursor { x: 1, y: 0 } - ^^ 'X' overwrote 'A' — DECRC landed at home, not r0c2 -snapshotAnsi: "AB\r\n\r\n\r\n\x1b[9CCD" (no saved-cursor state at all) -``` - -**Mechanism:** the snapshot is a serialized *screen* (SerializeAddon) plus a few -rehydrated modes. The VT100 DECSC/DECRC saved-cursor register (also `CSI s` / -`CSI u`) is runtime state that never appears in the serialized buffer, so it -cannot survive a snapshot. When a hidden TUI runs `\x1b7` (or `\x1b[s`) before -the reveal seq and the racing tail (or any post-reveal output) runs `\x1b8` (or -`\x1b[u`), the restore targets the fresh terminal's default saved position -(home) instead of where the TUI saved it — the next writes land at the wrong -cell and overwrite live content. - -**Why it garbles a real pane:** DECSC/DECRC is common in shell prompts and -status-line redraws (save cursor, jump to a corner to paint a clock/token -counter, restore). If the save happens while the pane is hidden and the restore -fires on reveal, the restored paint clobbers the wrong cells. Found by suite-2 -seed 3 (a `savedCursorDetour` op whose `\x1b7` fell in the hidden prefix and -whose `\x1b8` fell in the racing tail after chunk-splitting). - -**Handling:** suite 2 keeps its racing tail append-only (no DECSC/DECRC, cursor -motion, scroll regions, or alt frames) so the seq-reconciliation byte-stitch is -tested in isolation from this and the other terminal-state-loss garbles. Bug D -is instead pinned as a standalone repro, -`hidden-reveal-reconciliation.fuzz.test.ts` → -`it.skip('preserves the DECSC saved-cursor register across a hide/reveal …')`. - -**Fix (applied):** the snapshot epilogue re-establishes the register with -`CUP(saved) + ESC 7 + CUP(actual)` composed in `serializeWithAbsoluteCursor`, -reading the active buffer's core register via `readSavedCursorRegister` -(alt screen yields its own register). Position-only: saved SGR/charset are not -carried. - ---- - -## Bug E — snapshot boundary mid-escape-sequence drops the partial sequence - -**Classification: (a) real bug — a structural snapshot limitation.** New -finding, surfaced by the reveal-reconciliation fuzz (suite 2). - -**Minimal repro (cols=20):** - -``` -hidden prefix: "AB\x1b[3" (write AB, then ESC [ 3 — no final byte yet) -tail bytes: "mCD" ('m' completes ESC[3m = italic, then CD) - -live (always visible): rows ["ABCD"] (ESC[3m parsed atomically, CD italic) -reveal (snapshot+tail): rows ["ABmCD"] ← 'm' became a literal character -snapshotAnsi: "AB" (the partial ESC[3 is in the parser, gone) -``` - -**Mechanism:** a PTY read (one delivery record) can split an escape sequence. -If the pane is revealed while the emulator's parser sits mid-`ESC[…`, the -serialized SCREEN cannot carry the partial sequence (it lives in the parser -state machine, not the buffer). The racing tail supplies the sequence's -remaining bytes, but with the prefix gone the terminal parses them as literal -text. Reproduced end-to-end against the real `HeadlessEmulator.getSnapshot`. - -**Why it garbles a real pane:** any TUI whose output is heavy with escape -sequences (all of them) can have a read boundary fall mid-escape; if a reveal -lands in that window the continuation renders as stray literal bytes (a rogue -`m`, `H`, digits) injected into the visible text. - -**Reachability:** requires the reveal/snapshot to fire in the gap between the two -halves of a split escape. `main` writes each PTY read to the emulator and -records it as one delivery unit (`session.ts emitSubprocessOutput`), and the -snapshot is taken synchronously at a drain — so the window is a single delivered -record that ended mid-escape. Narrow but real. - -**Handling:** suite 2 tolerates + counts scenarios whose hidden prefix ends -mid-escape-sequence (`prefixEndsMidSequence`), the same way suite 1 tolerates the -serialize wrap bug — it fired on ~24% of the corpus, confirming the class is -common. Pinned by `hidden-reveal-reconciliation.fuzz.test.ts` → -`it.skip('completes an escape sequence split across the hide/reveal boundary')`. - -**Fix (applied):** the emulator tracks the unparsed trailing partial escape at -ingest (`terminal-partial-escape-tail.ts`, committed post-parse like the mouse -mirror) and ships it as `TerminalSnapshot.pendingEscapeTailAnsi`; restorers -write it LAST, after their post-replay resets, so the racing tail's -continuation completes it exactly as live. Snapshot seq already counted those -ingested bytes, so reconcile slicing is unchanged. - ---- - -## Known-legitimate normalization (NOT bugs) - -- **OSC 8 hyperlink underline** — classification (c). xterm marks OSC-8 link - cells underlined; SerializeAddon never re-emits OSC 8. Production restores the - link ranges out-of-band via `snapshot.oscLinks` - (`collectHeadlessOscLinkRanges`), so byte replay keeps the text but drops the - underline by design. Pinned by the passing - `it('drops OSC 8 underline from byte replay but preserves the range …')`. -- **P256→P16 color mode** — classification (c). SerializeAddon re-emits palette - indices 0–15 written as `38;5;N` using classic SGR 30–37/90–97, so a restored - cell reports `CM_P16` where live reported `CM_P256`. Both resolve through the - same 16 theme slots — no visual difference. Canonicalized by - `canonicalColorMode` in the parity fixture. - ---- - -## Corpus vs deep mode - -- **Suite 1** (`headless-emulator-fidelity.fuzz.test.ts`): default - `FUZZ_ITERATIONS=300` (~17s). `FUZZ_ITERATIONS=2000` (~113s) is green — Bugs A, - B, and C are each tolerated + counted by a predicate, so the corpus fails only - on a genuinely new divergence. -- **Suite 2** (`hidden-reveal-reconciliation.fuzz.test.ts`): default - `FUZZ_ITERATIONS=200` (~5s). `FUZZ_ITERATIONS=1000` is green — the racing tail - is append-only, so the only tolerated class is Bug E (`prefixEndsMidSequence`). -- Combined default runtime is ~19s (well under the 60s gate). -- `FUZZ_SEED=`: re-run exactly one seed for a repro (both suites). diff --git a/notes/orca-performance-branch-guide.md b/notes/orca-performance-branch-guide.md deleted file mode 100644 index 870c3813911..00000000000 --- a/notes/orca-performance-branch-guide.md +++ /dev/null @@ -1,532 +0,0 @@ -# orca-performance Branch Guide - -Agent-facing map of every optimization on this branch: what it does, why it exists, -where it lives, and the invariants you must not break when adding to it. The -chronological evidence trail (benchmarks, retractions, A/B protocols) is in -`notes/terminal-performance-initiative.md`; this doc is the _current-state_ view. - -**Context**: Orca's terminal was ~300× slower than Terminal.app under agent load -(DSR-under-load p50 134ms, p99 292ms on v1.4.91; agent-TUI throughput 2.0 MB/s). -As of v1.4.122-rc.1.perf: p50 13.3ms / p99 18.7ms, zero timeouts, throughput -11.8–15.5 MB/s — beats VS Code on 5 of 6 metrics. Goal line still open: 4.5ms -(10× Terminal.app). - -## The pipeline - -``` -shell → pty → daemon (persistence, headless model) → unix socket - → main (ipc/pty.ts: batching, delivery gate, flow control, snapshots) - → IPC → renderer (pty-dispatcher → pty-connection → output scheduler → xterm) -``` - -Main is on the hot path for every byte (unlike VS Code's ptyHost→renderer -MessagePort). The daemon owns sessions so they survive app restarts; it also runs -a headless xterm emulator per pty — the _model_ — which is the source of truth -for screen contents. The renderer terminal is a _view_ that can be discarded and -rebuilt from model snapshots. - -## Optimization inventory - -### 1. Renderer parse-path fixes (the original 16× on agent TUIs) - -- **Parse-clocked scheduler drains** (`pane-terminal-output-scheduler.ts`): drain - cadence follows xterm's actual parse completion instead of fixed timers, so the - queue never outruns the parser. -- **Windowed retained-tail redraw**: TUI repaints (erase-down + redraw) only - re-process a bounded window instead of the full retained tail. Guarded by - differential fuzz `retained-tail-redraw-window.equivalence.test.ts`. -- **Throttled wait-blocked check** (`orca-runtime.ts`): the per-chunk agent - wait-detection (two 256KB waitText builds + multi-pattern scans) now runs at - 50ms cadence with trailing edge + keyword pre-filter. Was ~85% of main's - per-chunk cost. - -### 2. term-speed-2 chain (model/view contract — the architecture) - -Revived from ~38 never-merged branches; kill-switched, default ON. Docs: -`docs/reference/terminal-model-view-contract.md`. - -- **Hidden view parking**: hidden tabs tear down their xterm view entirely - (memory: parked panes cost ~0). -- **Hidden delivery gate** (main): renderer-bound bytes for hidden ptys are - dropped at main — hidden panes receive nothing. Reveal rebuilds the view from a - model snapshot + live chunks after the snapshot's seq. -- **Side-effect authority**: main extracts side-effect facts (bell, title, cwd) - from the model so parked panes stay live in the UI without a view. -- **Model query authority**: main answers terminal queries (DSR/CPR, DA1, OSC - colors) deterministically from the model for hidden panes. -- **Seq/ordered-delivery bookkeeping**: every chunk carries a seq; reveal - reconciliation drops duplicates already covered by the snapshot baseline. - -### 3. Batching & scheduling cadence - -- **Batch windows 8ms → 2ms** in both `daemon-stream-data-batcher.ts` - (`STREAM_DATA_BATCH_INTERVAL_MS`) and `ipc/pty.ts` (`PTY_BATCH_INTERVAL_MS`). - At 9% utilization there is no queue — latency was literally the sum of fixed - batch windows. This one change took dev DSR-load 19→8ms. -- **MessageChannel zero-delay drains** (`pane-terminal-output-scheduler.ts`): - Chromium clamps nested `setTimeout(0)` to ~4ms; posted messages are macrotasks - without the clamp, preserving cooperative yield (input/paint still serviced). - Vitest keeps the timer path (fake timers can't advance channel posts). -- **Input write coalescing** (from main, #7205): renderer input writes coalesce - instead of queuing macrotask-per-keystroke. - -### 4. Backpressure (the correctness spine — read before touching delivery) - -Three cooperating layers, innermost first: - -- **ACK at parse-drain** (`deliverPtyDataWithDeferredAck`, scheduler - `ackCredit`): the renderer credits a chunk when xterm has _parsed_ it (or the - chunk is legitimately discarded), not when IPC delivered it. - **INVARIANT: every delivered chunk credits exactly once — parsed or - discarded.** Every scheduler/pty-connection discard path (backlog replacement, - disposed terminal, reconcile drop, split remainders) must fire the credit. -- **Cumulative ACKs + solicited resync** (`terminal-pty-ack-gate.ts`, - `applyCumulativeAck` in pty.ts): ACKs carry monotonic per-pty processed totals - (TCP-style); main max-merges, so lost ACKs self-heal. Data arriving for a - fully-gated pty triggers a resync probe instead of a timeout reset. The only - timer is a hygiene warn that mutates nothing. Main's 512KB per-pty in-flight - gate + 2MB pendingData cap sit on top. -- **Renderer-pull delivery watchdog** (`terminal-delivery-watchdog.ts`, - `pty:reportRendererDeliveryState` in pty.ts): recovers the field-confirmed - wedge where every main→renderer PUSH channel dies while invoke stays alive - (v1.4.121-rc.0 snapshot; electron#37067 class) — a state the push-ridden - resync probe can never reach. The 15s heartbeat costs one Map upsert per - received chunk and does no IPC while output flows; mutation stays - verified-state-only (the timer decides when to REPORT; the write-off derives - entirely from the renderer's cumulative received totals, never wall-clock, - and a received-but-unparsed window is never written off). Heal = re-attach - push listeners + pull restore markers through the modelRestoreNeeded router. - E2e blackhole harness: `__terminalDeliveryWatchdog`, - `terminal-push-delivery-loss-recovery.spec.ts`. -- **Stale-visibility proof for the hidden gate** (`stale-document-visibility.ts` - and the `shouldWritePtyOutputForeground` fallthrough in pty-connection.ts): - recovers the field-confirmed wedge where macOS occlusion tracking pins - `document.visibilityState` at `'hidden'` after display sleep and never fires - another visibilitychange (v1.4.124-rc.2.perf snapshot: 78MB hidden-gate - dropped across 2 pane-level-visible ptys, transport healthy). Real user - input (keydown/pointerdown/window focus) while the document claims hidden is - a physical contradiction — it latches an override, runs each pane's existing - visibilitychange resync (gate unhide + hidden-output restore), and a genuine - visibilitychange hands authority back. No timers; recovery is purely - event-proven, and the failure bias is safe (a wrong override only restores - pre-gate delivery cost, never drops bytes). Hot-path cost: zero when - visible (same single comparison); one property read per user-interaction - event. E2e: `terminal-stuck-occlusion-recovery.spec.ts` (pins both the - freeze repro and the keystroke recovery, plus the - `hiddenDeliveryGatedVisiblePtyCount` field discriminator). -- **One-paste freeze report** (`terminal-freeze-report.ts`, prod-installed - `await window.__orcaTerminalFreezeReport()`): a single DevTools command that - returns renderer state (visibilityState + stale override, pty:data listener - count, watchdog totals), main's snapshot with a per-pty delivery table - (sent/acked/pending, hidden vs visible-set membership, last send/ACK ages, - window focus flags, power suspend/resume ages, app version), and bounded - breadcrumb rings from BOTH processes (`pty-delivery-diagnostics.ts` shared - ring: 100 entries, same-kind coalescing) recording gate marks, visibility - trust changes, watchdog stalls/heals, restore markers, heal write-offs, - renderer lifecycle resets. Pty ids are redacted to their `@@` suffix — - daemon session ids embed worktree paths. Recording happens only on rare - transitions; the table/report is built only when read. This exists so a - field freeze report never needs a follow-up ask. -- **Hidden/parked exit teardown completeness** (pty-connection.ts kept-exit - guard + `terminal-parked-tab-watchers.ts` exit sidecar): two invariants that - keep a split pane's death near the hidden/park boundary from stranding state - (field incident: a closed setup-split leaf persisted in `root` with no - binding and remounted as a permanently blank pane, unreachable by - dead-session reconcile — it skips ptyId-null panes by design). - (1) The "keep a fresh split whose newborn PTY died" branch is **gated on - `isVisibleRef`** — hidden panes' bytes are gate-withheld, so "no output" - proves nothing there; a hidden newborn death must `closePane`, or the kept - pane becomes a binding-less ghost. (2) A PTY exit that lands **while parked** - reaches ONLY the parked watcher's exit sidecar (hosts' `onPtyExit` needs a - mounted TerminalPane), so the sidecar itself collapses the dead leaf out of - the stored layout via `detachTerminalLayoutLeaf` — a stale binding left - behind reattaches on reveal and the daemon re-creates the exited session id - as a fresh shell (silent pane resurrection). E2e: - `terminal-pane-close-layout-consistency.spec.ts` sweeps close/exit at every - lifecycle phase and asserts leaves(root) == bindings == live panes. -- **Producer flow control** (protocol v19 `pausePty`/`resumePty`, 256KB pause / - 32KB resume watermarks, keyed off **pendingData only** — never renderer - counters; kill switch `PRODUCER_FLOW_CONTROL_ENABLED`, ipc/pty.ts): when main's - buffer grows, the _shell_ blocks. For main-hosted ptys pause is synchronous - (drops impossible); for daemon ptys the pause notify has ~20-30ms socket - latency, so wire-speed bursts can still cross the 2MB cap (known follow-up: - daemon-side self-pacing watermark). -- **Shallow stream-socket write gate + per-session fairness** - (`daemon-stream-data-batcher.ts`, 128KB gate / 64KB safe-split slices / - 4KB small-session bypass / 32MB write-through valve; kill switch - `ORCA_DAEMON_SHALLOW_SOCKET_GATE=0`): the stream socket is one FIFO for - every session — bytes already written can never be overtaken, so a deep - user-space buffer buries a visible pane's echo behind other panes' bulk - (measured 192MB / 6+s under 12 flooding hidden agents). Bulk writes stop at - the gate and hold in the batcher, where the interactive flushSession path - and the deterministic small-session bypass still jump them; socket `drain` - refills. This layer alone bounds echo latency by the shallow depth. -- **Background keep-tail stream thinning + daemon fact authority** - (`daemon-stream-keep-tail-drop.ts` 1MB cap / 512KB keep-tail, - `daemon-background-transient-facts.ts`; kill switch - `ORCA_DAEMON_BACKGROUND_STREAM_DROP=0`): hidden-gated ptys are exempt from - pendingData flow control (main drops their bytes after ingestion), which - let N background agents run unbounded ahead of main. Main mirrors the - hidden-delivery gate to the daemon via the wire-tolerated - `setSessionBackground` notification (introduced in v19; authoritative - thinning requires v20 snapshots, so preserved v19 sessions are explicitly - unthinned; older daemons swallow it) — but a live remote view subscriber - (mobile/web) vetoes backgrounding (`hasRemoteTerminalViewSubscriber`). - Backgrounded sessions' queued output is keep-tail dropped (oldest bytes - replaced by an in-order `dataGap` event; reply-eliciting query bytes are - salvaged so hidden programs never hang on DSR/DA replies); producers are NEVER paused, - so reveal stays instant with zero catch-up. Un-background neither discards - nor force-flushes the queued tail — restore paths read MAIN's model - (hidden-output recovery buffer), so a discarded tail loses a finished - program's last output forever (caught by the ACK-backpressure e2e), and a - 16-pane force-flush dumps ~12MB onto the socket ahead of the reveal's own - bytes; the ordered drain loop delivers it within the budget below. Two - aggregate bounds make that budget real: (1) a GLOBAL background keep - budget (~2MB): per-session keep-tails shrink (512KB → floor 64KB) as more - backgrounded sessions hold queued data, and tighten retroactively when the - count grows — without this, N sub-cap sessions queue N×cap and a worktree - switch waits seconds behind the aggregate (measured 9MB → 2.5s hidden - restore vs the 1.5s budget, probe-verified drain overlap); (2) a - kernel-flush refill sentinel: a held flush pass arms one ~90B empty data - event whose write callback re-flushes when the kernel accepts the - in-flight bytes — without it, held bulk advances one gate-depth per - 'drain' (user-space empty) per event-loop turn (~8MB/s ceiling on a busy - daemon). NOTE: an empty `socket.write('')`'s callback fires immediately - even with megabytes buffered (verified) — the sentinel must be a real - protocol no-op line. Notifications are - structurally lossless: while backgrounded, the DAEMON runs the same shared - scanners main uses (`terminal-output-side-effects.ts`: bell / OSC 133 - command-finished / pr-link / DECSET 2031) over every byte BEFORE drop - decisions and relays facts as in-order `transientFact` events; ordered - `sessionBackgroundMarker` events hand scan authority back and forth - (main suppresses just those four scanners in between), and the emulator's - `partialEscapeTailAnsi` seeds each side's fresh scanner carry so a - sequence split across the handoff neither phantom-fires nor goes missing. - Titles/agent-status stay main-side (they converge from the kept tail and - fuse with synthetic spinner frames). On `dataGap`, main resets its - cross-chunk parse carries, drops the mobile headless mirror (rebuilds from - tail/seeds), and sends the model-restore-needed marker so any - renderer-side buffer heals from the snapshot. Visible ptys are never - touched by this layer. Backlog observability: - `ORCA_DAEMON_STREAM_BACKLOG_FILE=` JSONL - (`daemon-stream-backlog-probe.ts`; events incl. `backgroundKeepTailDrop`, - `setSessionBackground`, `mainBackgroundSync`, `heldWriteThrough`). - Causation A/B (`bench:multi-workspace-typing`): realistic steady rates - (8×192KB/s) don't reproduce even fix-off; burst rates on a loaded machine - do (8×512KB/s + 12 CPU spinners: fix-off p50 293ms/p90 647ms → fix-on p50 - 29ms); extreme 12×1MB/s: fix-off p50 6,146ms → 29ms. - -### 5. Flood resilience (why bulk output can't wedge or lie anymore) - -- **Restore-loop cut** (pty-connection.ts): the hidden-output-restore loop - abandons immediately when a foreground pane's live-chunk queue overflows - (3-iteration hard cap), and a 2s flood-suppression window stops main's own - backpressure drops (`droppedOutput`/`modelRestoreNeeded`) from re-arming - restore — bytes write through, ONE deferred repaint heals after the flood. - This killed a positive feedback loop (restore starves ACKs → main drops → - drop re-arms restore) that caused multi-second renderer stalls. -- **Query survival**: if the 2MB cap ever drops bulk output, embedded terminal - queries are extracted (`terminal-reply-query-extraction.ts`) and answered by - _synthesizing replies on the input path_ (CPR from live buffer, DA1 canned, - OSC via direct responder) — probes and TUIs never hang on a dropped reply. -- Drops are downstream of the model: the daemon ingests every byte, so the - post-flood repaint restores complete, correct content. - -### 6. Wake/sleep recovery - -- powerMonitor resume → `system:resumed` IPC → renderer wake recovery (fixes - WebGL-latch blank-after-sleep that DOM focus/visibilitychange missed). -- Cumulative ACKs make the historical "lost ACKs across suspend pin the global - window forever" wedge (BMW user bug) structurally impossible. - -### 7. Snapshot fidelity (the garble fixes — all fuzz-pinned) - -Reveal-from-snapshot multiplied exposure of serializer defects ~1000×. Five bugs -found by differential fuzzing; four fixed, one tolerated: - -- **B: SGR intensity ordering** — upstream `@xterm/addon-serialize` emitted - `1;22` (22 clears the bold 1 just set). Patched via pnpm patch - (`config/patches/@xterm__addon-serialize@*.patch`): clear-before-set for the - bold/dim group (+2 sibling bare-22 defects). -- **C: cursor off-by-one at wrap-pending margin** — bypassed entirely: - `serializeWithAbsoluteCursor` (`terminal-serialize-absolute-cursor.ts`) - appends absolute CUP from the source terminal's authoritative cursor - (skipped when wrap-pending, where CUP would corrupt). -- **D: DECSC saved-cursor register not serialized** — snapshot appends - `CUP(saved) + ESC 7 + CUP(actual)` when a register exists. -- **E: snapshot mid-escape-sequence** (fired on 24% of fuzz corpus) — - `terminal-partial-escape-tail.ts` is a fold-safe VT-parser-state scanner; the - unparsed tail ships as `TerminalSnapshot.pendingEscapeTailAnsi` and is written - LAST on restore so continuation bytes complete the sequence. Seq accounting - unchanged (the tail is a suffix of bytes ≤ snapshot seq). -- **A (tolerated)**: upstream wrap-null-cell serialize defect — fenced by - `bufferHasSerializeHostileWrappedRow`, the only remaining tolerance. - -## Correctness infrastructure (run these before merging delivery/restore changes) - -- `headless-emulator-fidelity.fuzz.test.ts` — differential: HeadlessEmulator vs - reference xterm, seeded TUI streams. `FUZZ_ITERATIONS=2000` for deep, - `FUZZ_SEED=n` to replay. -- `hidden-reveal-reconciliation.fuzz.test.ts` — property tests: random - hide/reveal boundaries × snapshot seq × racing chunks must equal an - always-visible reference. -- `terminal-snapshot-serialize-roundtrip.test.ts` — the garble repros (unskipped - = regression alarms). -- e2e: `terminal-hidden-view-parking` (incl. 25-cycle park/reveal drift test — - byte-identical vs control), `terminal-parked-memory`, - `terminal-sleep-wake-restore`. -- Scheduler credit-invariant + ack-gate deferred-credit + restore-flood tests - (pane-manager / terminal-pane suites). - -## Benchmarking protocol (hard-won rules) - -- Rig: `tools/benchmarks/terminal-pipeline-bench.mjs` — DSR idle + DSR under - 1MB/s agent-TUI load + DSR-fenced throughput on 4 fixtures. Run _inside_ the - terminal under test. -- Multi-workspace typing rig: `pnpm bench:multi-workspace-typing -- --panes 12 ---rate-kbps 1024 --keys 32 --cadence-ms 250 [--cpu-workers 8] --label ` - — real keystrokes (CDP) into a visible pane while N hidden-worktree panes - replay paced agent-TUI streams through real daemon ptys; decomposes each key - into input-half (keydown→pty, sidecar timestamps) and echo-half - (pty→screen). JSON in `tools/benchmarks/results/`. Noise band at 4×256KB/s: - p50 10-15ms, p90 ≤50ms. The latency signature lives in echo-half; renderer - timer drift staying ~15ms while echo-half grows means the backlog is - upstream of the renderer (daemon socket / main ingest). -- **Bench at 10MB** (`--size-mb 10`). The ACK-at-parse bug shipped because dev - benches used 3MB and never tripped the cap. -- Load-controlled A/B only: alternate builds within one session; dev carries ~2× - day-to-day variance. Never conclude from runs while agents/builds hammer the - machine (two false convictions came from this). -- Never set `ORCA_E2E_USER_DATA_DIR` for benches (arms the e2e ACK gate → hang). -- Packaged builds are truth; dev has ~2× overhead. - -## Release mechanics - -- Perf RCs: `release-cut.yml` workflow_dispatch, `kind=rc ref=orca-performance -version_suffix=perf` → tags like `v1.4.122-rc.1.perf`. The suffix sorts above - its base rc.N but below rc.N+1 (never hijacks the RC channel). The rc counter - (`release-rc-history.mjs`), telemetry identity classifier, and build guard are - all suffix-aware — a suffixed rc classifies as `rc`. -- cmd/ctrl-click "Check for Updates" fetches the latest perf-tagged release - (PR #7278; merged here) — perf-line users self-update after one manual install. - -## Syncing with main: MERGE, never rebase - -`orca-performance` is a long-lived, shared, continuously-pushed integration -branch — RCs are cut from it and agents branch off it. **Always -`git merge origin/main`; never rebase** (rebasing rewrites pushed history and -strands every RC tag, fix branch, and worktree based on the old commits). -Conflict pattern, established over ~6 syncs: - -1. **Our structure wins; main's semantics graft in.** This branch deliberately - restructures terminal code (shared scanners, single-policy handlers, - model/view split). When main adds a feature inside code we've restructured, - keep our shape and port their new behavior into it. Example: main inlined an - OSC 133 parser to add `onCommandStarted` (133;C); we kept the shared - `createOsc133CommandFinishedScanner` (main's side-effect tracker must parse - byte-identically) and added 133;C support to the shared scanner instead. -2. Preserve the invariants in **Guardrails** below through every resolution — - especially chunk-credit, `pendingData`-keyed flow control, and the - single `handleCommandFinished` policy (byte path AND sideEffect-fact path - route through it). -3. After resolving: `pnpm typecheck`, the terminal-pane + ipc/pty + daemon - suites, and both fuzz suites. Commit the merge with a message stating what - was kept from each side; push. If the push races a moved remote, merge the - remote tip — never `pull --rebase` a merge. -4. If a sync lands anything on the delivery/restore path, re-run a 10MB bench - before the next RC cut. - -## Known limits / next levers (in rough priority order) - -1. Daemon self-pacing: daemon-hosted ptys can cross the 2MB cap for ~20-30ms at - wire speed before `pausePty` bites. Fix: daemon enforces its own watermark - locally (VS Code does this server-side for remotes). -2. Cadence floor to the 4.5ms goal: xterm's 12ms parse slices and remaining - drain cadence dominate the 13.3ms prod p50. -3. utilityProcess router endgame: take main off the per-byte hot path - (VS Code's ptyHost→renderer MessagePort shape). -4. SerializeAddon full-buffer stalls at 50k-row scrollbacks (#5096 follow-up). -5. Peel PRs to main: throughput fixes → batch+MessageChannel → flow control → - term-speed-2 last. PR #7214 is the integration overview; #7260 (wake/ACK) - is open against main separately. - -## Guardrails for future agents - -- The chunk-credit invariant (§4) is the load-bearing one. If you add ANY path - that receives, defers, drops, or splits pty data in the renderer, prove it - credits exactly once. The credit-invariant unit tests are the gate. -- Flow control keys off `pendingData` only. Do not couple it to renderer - counters; the two layers compose because they are independent. -- Snapshot changes must keep seq semantics: a snapshot covers _exactly_ bytes - ≤ its seq (Bug E made this true; don't regress it). Chunks after restore are - reconciled by seq — off-by-N re-triggers duplicate-drop garble. -- Hidden panes must receive nothing (delivery gate) but side-effects and query - replies must stay live via the model. If you add a new query type, wire it - through model authority AND the drop-path synthesis. -- Never add timeout-based recovery that mutates counters (user requirement — - design decision from #7260). Deterministic resync or nothing; hygiene timers - may only log. -- Any change on the delivery/restore path: run both fuzz suites, the roundtrip - tests, the chain e2e trio, AND a 10MB bench before calling it done. - -## Audit 1 - -Completed 2026-07-10 against `orca-performance`. Scope: the daemon → main → -renderer terminal path, with particular attention to hidden delivery, parking, -stream thinning, snapshot fidelity, ACK/backpressure semantics, wake/reattach, -mobile/remote composition, SSH routing, and teardown. The audit treated model -state and user scrollback as correctness requirements, not expendable memory. - -### Findings and fixes - -1. **Hidden-gate handoff owners could undo one another.** A parked watcher and - an unmounting/remounting pane shared one boolean hidden mark; likewise a - retiring pane could report `visible=false` after its replacement had already - reported `visible=true`. Hidden claims are now reference-counted and - visibility is counted per owner (`pty-renderer-delivery-claims.ts`). Eager - pre-mount buffers no longer hold raw-byte delivery interest: they are not a - side-effect consumer, and model-backed hidden output can restore from a - snapshot. This removes the ownership and eager-interest races formerly - listed in Known limits item 6 without weakening parked side effects. The - remaining transient-visibility concern was checked separately: bind and - reconnect reports read `TerminalPane`'s synchronously refreshed - `isVisible && isWorktreeActive` ref, the global effect uses the same - expression, and owner counting prevents a retiring pane from overriding its - replacement. No hidden-worktree `visible=true` report site remains. - -2. **A natural/synthetic daemon exit could overtake final output.** When a - shallow-gated socket had queued data, `daemon-server.ts` wrote the exit event - directly. The final bytes could therefore arrive after `exit`. Exit is now an - ordered control event in `DaemonStreamDataBatcher`; both natural and - synthetic exits flush through the same FIFO. A deep-socket regression test - pins final-data-before-exit ordering. - -3. **Keep-tail thinning could permanently reduce scrollback to the retained - tail.** On `dataGap`, main discarded its headless model, then rebuilt it from - later tail bytes even though the daemon still owned the complete model. Live - daemon snapshots now carry `outputSequence`; the provider exposes an - authoritative `getBufferSnapshot`, accepts the requested scrollback depth, - and main requires that provider snapshot after a gap. Reconciliation starts - in the pre-snapshot absolute sequence domain and runtime sequence accounting - advances across dropped bytes. The daemon remains the source of complete - scrollback instead of making a transport optimization destructive. If that - authoritative RPC is temporarily unavailable, main now returns no snapshot - and lets the renderer retry; it never paints main's known-incomplete tail as - a full recovery. - -4. **Query-salvage copies corrupted the absolute sequence domain.** DSR/DA - bytes salvaged from a dropped region are copies of bytes already counted by - the daemon, not new output. Stream events now distinguish delivered text - from `sequenceChars`; salvaged query data advances by zero while the gap - advances by the original characters. Main can still parse/deliver the query - copy without shifting every later snapshot baseline. - -5. **“Parse-deferred” ACKs were submission-deferred, not parse-deferred.** ACK - credit fired when bytes entered `terminal.write`, before xterm's callback. - Split scheduler chunks also attached `onParsed` to the first slice. ACK - credit is now owned by `pane-terminal-output-ack-credit.ts`, fires after the - final xterm parse callback, and is released exactly once on throw, discard, - or terminal disposal. Submitted-but-unparsed credit is retained until parse - or disposal, so main's flow-control window measures parser work rather than - renderer submission. - -6. **Hidden restore ignored configured scrollback.** The renderer always asked - for 5,000 rows, so users configured for 10k–50k silently lost older history - on a hide/reveal rebuild. Restore now reads the pane's xterm scrollback - option and clamps it through the shared 0–50,000 policy - (`terminal-hidden-restore-scrollback.ts`). - -7. **Active alternate-screen snapshots discarded the normal shell buffer.** - SerializeAddon emits `normal buffer + ?1049h + alternate buffer`; the old - normalization sliced away everything before the last `?1049h`. A restored - TUI looked correct until it exited alternate mode, then returned to empty - history. Snapshots now carry the normal buffer separately in - `scrollbackAnsi`. Fresh reattach and mobile/remote snapshot streams compose - both buffers; an already-alt renderer exits alt, clears/rebuilds the normal - buffer, then re-enters and rebuilds alt. History replay also composes both - buffers, including legacy empty-field compatibility. - - Deep fuzz then found a second two-buffer issue: normal-buffer serialization - can leave its SGR pen active while the separately serialized alternate body - assumes default SGR. The rehydrate boundary now emits `SGR 0` before - `?1049h`, preventing a shell color from tinting restored TUI cells. The - regression proves the TUI is visible immediately and `?1049l` returns to the - original shell history. - -8. **Daemon provider wrappers forwarded only part of the recovery contract.** - A preserved current/legacy daemon could emit `dataGap` through a provider - wrapper, but `DegradedDaemonPtyProvider` omitted `getBufferSnapshot`, while - the ordinary multi-version `DaemonPtyRouter` omitted background hints, gap - events, snapshots, and explicit `sequenceChars`. Both wrappers now route the - complete contract to the provider that owns the session, including requested - 50,000-row recovery and zero-advance query-salvage events. - -9. **Sequence-safe recovery was added without advancing the daemon protocol.** - An already-running v19 daemon could accept background-thinning hints but - could not return the new `outputSequence`, making any resulting gap - impossible to reconcile safely. The authoritative snapshot contract is now - protocol v20. Preserved v19 sessions remain live but are explicitly marked - unthinned; their stale background hint is cleared on the ordered control - socket before `createOrAttach`, while fresh v20 sessions retain keep-tail - performance and full-model recovery. - -### Static audit conclusions - -- Model query authority still captures ownership synchronously at ingestion; - seed/hydration/snapshot writes remain reply-silent, remote view subscribers - retain view authority, and replies use the provider input path (including - daemon shell-ready queuing and SSH routing). -- Hidden/visibility/interest/background-sync/provider-snapshot state is cleared - by the centralized PTY teardown path. Parked watcher timers, byte sidecars, - fact consumers, exit subscriptions, hidden claims, and runtime-title slots - dispose on reveal/exit/worktree shutdown. -- Remote-runtime and SSH PTYs remain excluded from cold parking. SSH hidden - panes still have a main-owned headless model, so mounted hidden-gate restore - is valid; live remote viewers veto daemon background thinning. The new - two-buffer payload is recomposed before mobile/remote snapshot frames. -- Wake recovery keeps its focus/visibility/system-resume listener symmetry and - cancels its settled animation frame on cleanup. No timeout was added that - mutates ACK or delivery counters. -- The one documented upstream SerializeAddon null-cell/wrapped-row defect - remains tolerated. Deep reveal fuzz now uses the same narrow hostile-row - predicate as fidelity fuzz, rather than misclassifying that known serializer - defect as sequence-reconciliation loss. - -### Validation evidence - -- Focused ownership/connection/dispatcher tests: 422 passed. -- Daemon server/batcher/order tests passed, including deep queued-socket exit. -- Broad main/daemon/runtime/RPC/SSH run: 1,854 passed, 5 skipped. Three stale - mocks were updated to assert the new explicit `sequenceChars` argument; the - production behavior was already correct. -- Broad renderer terminal/pane/scheduler/runtime-stream run: 1,934 passed. -- Final restore/roundtrip/history/adapter/runtime/scheduler sweep: 1,315 passed. -- Post-protocol completion sweep: 1,106 affected main/daemon tests passed; - 484 renderer/restore tests passed with 2 expected skips. -- Scheduler throughput harness passed with `ORCA_TERMINAL_PERF_BENCH=1`. -- Required hidden-view parking, parked-memory, and sleep/wake E2E trio: - 7 passed on a fresh v20 Electron build, including byte-identical output - across 25 park/reveal cycles. -- `FUZZ_ITERATIONS=2000` headless-emulator fidelity: passed (120.19s). -- `FUZZ_ITERATIONS=2000` hidden reveal reconciliation: passed (69.77s). It - reproducibly found the SGR boundary bug at seed 16 and the known upstream - wrapped-null-cell case at seed 1221 before the final green run. -- Snapshot roundtrip, retained-tail equivalence, ACK gate, PTY connection, and - remote incomplete-escape regression suites passed. -- Fullscreen real-app headful flow: a real shell wrote normal history, entered - a TUI while its worktree was hidden, restored on reveal, then exited with - `?1049l` back to the original history. The BrowserWindow was fullscreen; - no click/focus occurred before evidence; the restored frame settled for - 1.5s before capture. Measurements: window 1710×1073 at DPR 2; xterm 133×63; - `fitAddon.proposeDimensions()` 133×63; cell width 8px; screen-to-xterm gap - 11px (the scrollbar/remainder, with grid and proposed dimensions equal). - Artifacts: `.tmp/terminal-audit-headful/fullscreen-alt-restore.png` and - `.tmp/terminal-audit-headful/fullscreen-alt-restore-metrics.json`. -- Visible, non-E2E current-build Orca 10MB agent-TUI bench: 10.14 MB/s - (986ms for 10.0MB), DSR idle p50/p90/p99 0.64/6.47/57.82ms, - DSR-under-load 6.72/9.86/13.87ms, zero timeouts. The pane was 115×39, - reported app version 1.4.131-rc.2, and ran on a v20 daemon/session. Result: - `tools/benchmarks/results/terminal-pipeline-audit-v20-20260710-2026-07-10T10-54-45-990Z.json`. -- `pnpm typecheck`, oxlint on every touched TypeScript file, - `pnpm check:max-lines-ratchet`, `git diff --check`, and the E2E production - build all passed. No max-lines bypass was added. diff --git a/notes/orca-serve-terminal-persistence.md b/notes/orca-serve-terminal-persistence.md deleted file mode 100644 index d96488572d4..00000000000 --- a/notes/orca-serve-terminal-persistence.md +++ /dev/null @@ -1,152 +0,0 @@ -# Orca Serve Terminal Persistence - -## Problem - -`orca serve` exposes terminal tabs to paired web clients through the runtime -`session.tabs` API, but the host-side terminal tab registry was process-local. -When the host published an empty session-tabs snapshot for a worktree, the web -client bootstrapped a new terminal, giving the user a fresh shell instead of -the previously running host session. - -The browser must remain stateless for terminal identity. Browser storage is -intentionally sanitized because remote handles become stale after a new pairing -or host restart. - -## Goals - -- Keep the host runtime as the source of truth for paired web terminal tabs. -- Persist `orca serve` terminal tab, leaf, and PTY/session bindings in the - existing workspace session model. -- Hydrate headless `session.tabs` snapshots from host persistence before a web - client decides a worktree has no terminals. -- Mirror the SSH persistence model where it applies, while keeping SSH relay - leases and local serve persistence behind their own provider checks. -- Preserve split-pane identity by routing all activation and attachment through - parent tab id plus leaf id. - -## Non-Goals - -- Do not persist remote handles in browser local storage. -- Do not make browser panes supported in headless `orca serve`. -- Do not redesign the terminal daemon or SSH relay. -- Do not treat a persisted PTY id alone as proof that a live process belongs to - a pane. - -## Design - -### 1. Persist Runtime-Owned Serve Spawns - -The runtime PTY spawn path accepts a main-only `persistHostSessionBinding` flag. -Headless serve sets it when creating session-tab terminals. The PTY handler -then calls `Store.persistPtyBinding` only after validating `worktreeId`, -`tabId`, and stable `leafId`. - -This keeps unrelated renderer-local PTY spawns from writing workspace-session -terminal bindings. - -### 2. Use Stable Session IDs - -Serve-created terminals pass `tabId`, `leafId`, optional `sessionId`, and -`persistHostSessionBinding` into `ptyController.spawn`. - -If a pending hydrated terminal has a persisted PTY/session id, activation -passes that id back to the provider. New serve-owned local sessions use a -nonnumeric `serve-${uuid}` id so they cannot collide with older numeric PTY ids -after restart. - -### 3. Hydrate Headless Snapshots - -Before `list`, `listAll`, subscribe initial emission, activation, close, or -move returns an empty headless state, the runtime hydrates -`mobileSessionTabsByWorktree` from `workspaceSession.tabsByWorktree` and -`terminalLayoutsByTabId`. - -Hydration preserves: - -- parent terminal tab id -- stable leaf id -- title fields -- active tab and active leaf -- split layout and `ptyIdsByLeafId` -- persisted PTY/session id - -Legacy terminal tabs without layout entries are still hydrated using a -deterministic stable leaf id derived from the parent tab id. - -### 4. Materialize Pending Tabs On Activation - -Hydrated terminal surfaces with no live trusted handle are exposed as -`pending-handle`. Activating one in headless serve materializes the exact -parent tab and leaf on the server, then returns a ready terminal surface. - -Explicit leaf activation is exact. If `leafId` is provided and the requested -leaf is missing, the runtime returns `tab_not_found` instead of falling back to -a sibling. - -### 5. Require Trusted PTY Identity - -For headless-hydrated persisted tabs, a live PTY is safe to expose only when -the runtime record already matches the same worktree, parent tab id, and pane -key. A process-list entry with the same PTY id but no pane identity stays -pending, preventing stale or numeric id collisions from attaching the wrong -terminal. - -Renderer-published authoritative session snapshots retain their existing -worktree-only daemon PTY adoption path. - -### 6. Close And Move Without A Renderer - -Headless close and move have no-renderer mutation paths: - -- close hydrates and refreshes first, removes the persisted terminal tab and - layout, updates active pointers, emits a new snapshot, and kills every live - trusted leaf under the closed parent tab -- move updates in-memory and persisted tab order without changing PTY bindings - -This prevents closed or reordered tabs from reverting on reconnect. - -### 7. Renderer Guards - -The web client still drops remote terminal identity from browser storage. It -mirrors the host snapshot and activates pending host mirrors through -`session.tabs.activate`. - -Bootstrap of a default web terminal is allowed only for a fresh empty snapshot -for the active worktree when no local terminal state already exists. Stale empty -snapshots and fresh empty snapshots racing with staged local terminals do not -create duplicates. - -### 8. SSH Parity - -Runtime-owned SSH spawns record remote PTY leases with target-local relay PTY -ids while workspace-session PTY bindings keep app-facing ids. Lease writes are -deferred until after binding persistence succeeds for persisted runtime-owned -spawns, so a failed binding save cannot leave durable SSH lease metadata for a -tab/leaf that was not saved. - -## Edge Cases Covered - -- Browser reload or WebSocket reconnect mirrors host-owned terminal tabs. -- Pending headless terminal activation reuses persisted tab and leaf identity. -- Split panes attach and activate the requested leaf only. -- Removed split leaves fail fast even when siblings remain. -- Legacy tabs without layouts hydrate instead of appearing empty. -- Stale empty snapshots do not bootstrap duplicate terminals. -- Fresh empty snapshots do not bootstrap when local terminal state already - exists. -- Numeric PTY id collisions remain pending until a safe reattach/spawn occurs. -- SSH reattach failure after binding persistence failure leaves no stale lease. - -## Verification Plan - -- Unit-test runtime hydration, activation, close, move, split-leaf exactness, - stale PTY id handling, and legacy no-layout tabs. -- Unit-test PTY persistence gates, local session-id reattach behavior, SSH - lease parity, and persistence failure cleanup. -- Unit-test renderer snapshot bootstrap guards and remote runtime PTY transport - pending-mirror behavior. -- Run adjacent session-tab RPC and web-runtime session tests. -- Run typecheck, lint, and `git diff --check`. -- Launch the Electron dev app from this worktree with an isolated profile and - verify CDP attachment, app identity, store availability, visible boot, and - zero console errors. diff --git a/notes/terminal-performance-initiative.md b/notes/terminal-performance-initiative.md deleted file mode 100644 index 2ac56bbf9e4..00000000000 --- a/notes/terminal-performance-initiative.md +++ /dev/null @@ -1,583 +0,0 @@ -# Terminal Performance Initiative - -Working plan for the `orca-performance` branch. Goal: make Orca's terminal as -performant as the architecture allows, with every claim backed by a number. -Started 2026-07-02. - -## Why (user-reported, from the team meeting) - -1. Typing in the terminal is sometimes laggy — occasionally seconds of delay. -2. Users say the terminal is slower than iTerm (unclear if typing or scrolling). -3. Scrolling in Claude Code / OpenCode is slow. -4. Idle memory is high (1–2 GB). -5. Battery usage is high. - -Goals: legit performance complaints ≤ 1/week; sampled P90 typing/scrolling -latency down significantly; lower memory with 0–1 agents. - -## Ground truth (verified against source, 2026-07-02) - -Research corpus: xterm.js 6 / VS Code / Ghostty internals study (verified -file:line claims) — see the archived digest and the "xterm.js vs Ghostty" -deep-dive. The Orca-specific findings below were re-verified against this -repo's code: - -- **Electron main sits on every terminal byte's path** (daemon → main → - renderer). VS Code ships the same xterm.js but bypasses main entirely: its - ptyHost is a UtilityProcess with a direct MessagePort to each renderer. -- **The PTY producer is never paused.** `acknowledgeDataEvent` is a no-op in - both `LocalPtyProvider` and `DaemonPtyAdapter`. Only main→renderer delivery - is watermarked (512 KB, `src/main/ipc/pty.ts:1374`); main's own buffer can - grow toward a 512 MB cap under flood. VS Code pauses the actual pty at 100k - unacked chars (kernel backpressure blocks the shell). -- Renderer terminals share one thread with the entire React app; xterm.js - parses in 12 ms slices at a documented 5–35 MB/s ceiling. -- Renderer scrollback default is 5,000 rows (`src/shared/terminal-scrollback-policy.ts`), - 5× VS Code's default; 12 B/cell plus per-line JS objects; O(all lines) - reflow on column resize. -- Latency physics: Ghostty ~4 ms median keypress latency, VS Code ~31 ms - (same-library reference), native class 5–10 ms. Realistic target: beat - VS Code, close on iTerm2, eliminate the stall/jank class entirely (P99 - dominates perception). - -## Current state - -Branch `orca-performance` (long-lived testing line, from main @ `8e8a08ac7`): - -1. `tools/benchmarks/terminal-pipeline-bench.mjs` — cross-terminal rig - (see Benchmark protocol below). -2. Merge of PR #7153 = #7150 (freeze/memory: backlog caps, wedge guards, - probe-certified replay release) + #7139 (cooperative drain: paced backlog - draining keeps typing responsive under floods). Post-merge on this base: - `pnpm typecheck` clean, 626 targeted tests green (scheduler, guards, - pty/pty-connection/pty-transport suites). #7153 itself is a disposable - testing PR; #7139 and #7150 land separately on main. - -## Workstreams - -### 1. Baseline benchmarks (now; human-in-terminal required) - -Run the rig in each terminal on the same machine — Orca pane, iTerm2, Ghostty, -Terminal.app, VS Code (T3Code if available): - -``` -node tools/benchmarks/terminal-pipeline-bench.mjs --label - -node tools/benchmarks/terminal-pipeline-bench.mjs report -``` - -These numbers answer "are we actually slower than iTerm, and where," and are -the before/after for everything below. - -### 2. Validate #7153 on orca-performance (this week, extended testing) - -Watch for: typing responsiveness under agent floods, bounded memory, -skip-notice + snapshot repaint on overflow, no permanent input loss. When -validated, land #7139 and #7150 as separate PRs on main. - -### 3. Revive term-speed-2 (the headline structural work) - -History: nwparker's ~38-branch chain (+20k lines) implementing the terminal -model/view contract — hidden view parking, hidden delivery gate, side-effect -authority in main, model query authority, skip-grammar deletion — all -kill-switched, documented in -`origin/nwparker/term-speed-2-architecture-docs:docs/reference/terminal-model-view-contract.md`. -It shipped only in v1.4.78-rc.1, a deliberate personal-testing build; it was -never rejected and never reached main. Directly targets complaints 3–5 -(hidden panes stop receiving bytes and unmount their xterm + WebGL atlases). - -Merge scout (2026-07-02, chain tip into orca-performance): 144 files, 34 -conflicted, 115 hunks. Hotspots: `pty-connection.ts` (31), `pty.ts` (16), -`daemon-pty-adapter.ts` (6), `orca-runtime.ts` (5). -`pane-terminal-output-scheduler.ts` does NOT conflict — #7139/#7150 and the -chain touch different layers; runtime interaction (drain pacing × hidden -gate) still needs deliberate testing. - -Execution: dedicated focused session; resolve on `revive/term-speed-2` off -orca-performance; keep both sides' kill switches; validate with typecheck + -the contract tests listed in the model-view-contract doc + #7153's suites; -merge back to orca-performance for extended testing. Estimated ~1 day of -careful resolution + validation. - -### 4. Remaining stall-bug fixes (parallel, independently shippable) - -The "seconds of delay" class = discrete thread-blocking events, not -steady-state latency: - -- PR #7105 (open): skip synchronous cold-restore replay for live daemon - sessions in doSpawn. -- `SerializeAddon.serialize()` audit: ~1.2 s renderer block at 50k scrollback - rows (#5096 follow-up, never done). Call sites include the mobile snapshot - path (`pty-connection.ts:2861`) and sleep/hibernate serialization. -- #2836 frozen-terminal leads: replay-guard latch, codex-stale gate, uncapped - buffers (repro harness exists). -- Checkpoint-RPC main-thread scrub (measured ~2–10 ms bursts per hot 5 s - tick; small, part of the same program). - -### 5. Producer-side PTY flow control - -Ack-driven pause/resume of the actual PTY through the daemon protocol -(node-pty supports it), watermarks per the xterm.js flow-control guide -(≤500 KB). Converts flood-induced buffered lag into shell blocking — the -correct physics. Sequence after #7139 lands (interacts with its drain pacing). - -Design (2026-07-03, implement after the term-speed-2 revival merges — -same files): - -- Signal source: main already tracks per-pty pending + in-flight - (`pendingData`, `rendererInFlightCharsByPty` in `ipc/pty.ts`). When a - pty's pending exceeds HIGH (256 KB), main asks the producer to pause; - below LOW (32 KB), resume. -- Producer side: two new protocol notifications (`pausePty`/`resumePty`, - protocol vNext, version-gated like `supportsIncrementalCheckpoints`); - daemon `Session` calls node-pty `pause()`/`resume()` — stops reading the - pty fd, kernel buffer fills, the shell blocks on write: true kernel - backpressure, identical physics to VS Code's 100k/5k design. - `LocalPtyProvider` calls pause/resume directly. -- Safety invariants: (1) failsafe auto-resume after 5 s regardless of - watermark, so a lost resume can never wedge a shell; (2) resume on - detach/exit/kill/daemon-reconnect; (3) pause must not suppress the - interactive-echo bypass — with the pipeline fixed (11.5 MB/s dev), the - HIGH watermark is only reachable during genuine floods where echo is - already queued; (4) PTY reads never stop for model/tail ingestion - (term-speed-2 invariant #1) — pause gates the fd read, so daemon-side - emulator state pauses with it, which is correct (state = what was read). -- Tests: watermark transition unit tests, lost-resume failsafe, kill/exit - cleanup, plus an e2e pressure scenario asserting bounded main memory and - a blocked producer (`yes` exits promptly on SIGINT while paused). - -### 6. Extend the measurement rig - -- True keypress→pixel latency: Typometer manual protocol (the DSR probe stops - at the parser reply, before paint). -- Idle memory + battery: per-process RSS breakdown + `powermetrics` sampling - at 0/1/5 agents (goal-3 metric). -- FPS under flood; event-loop-delay probes (`monitorEventLoopDelay`) in - main/daemon/renderer behind a debug flag for pipeline attribution. - -### 7. utilityProcess terminal router (structural endgame; gated on data) - -An Electron UtilityProcess owns the daemon socket and hands each renderer a -MessagePort — VS Code's topology while keeping Orca's detached daemon (warm -reattach). Takes main off the terminal data path entirely; daemon-side -history persistence falls out naturally. Prototype only after baselines show -how much tail latency lives in the main hop. - -### 8. Production P90 telemetry - -Sampled keypress→echo latency + long-task/stall counts from real users; -defines the success criterion and becomes the permanent regression gate. -Design after the local rig stabilizes so the metrics match. - -## Benchmark protocol - -`tools/benchmarks/terminal-pipeline-bench.mjs` measures, from inside any -terminal: - -- **DSR idle latency** — ESC[6n round trips (p50/p90/p99); replies come only - after the parser reaches the query, so it proxies the input pipeline - without keystroke injection. -- **Fenced throughput** — 4 deterministic fixtures (`ascii-log`, `cjk-emoji`, - `agent-tui` — Claude-Code-shaped transcript + DEC-2026 status repaints — - and labeled-pathological `styles-stress`), each run ended by a DSR fence so - xterm.js-class ingest queues can't flatter the result. -- **DSR under load** — latency sampled during a paced 1 MB/s agent-TUI - stream: "typing while the agent works," quantified. - -Rules: same machine, AC power, comparable window size, no tmux/screen, hands -off the keyboard during runs. Never compare numbers across machines. - -## Sequencing - -``` -now: [1] baselines [2] #7153 testing (parallel) -next: [3] term-speed-2 revival (dedicated session) -parallel: [4] stall fixes, [6] rig extensions -after 2/3: [5] flow control -gated: [7] utility router [8] telemetry -``` - -BMW-group crash work remains the team's priority gate above all of this -(#7150's wedge guards overlap it); this plan runs measurement and revival -prep in parallel without displacing it. - -## Findings log - -### 2026-07-02 — baseline + decomposition (results committed in tools/benchmarks/results/) - -Same machine, unattended serial runs (Orca 1.4.91 prod, Terminal.app, Ghostty -1.3.1; iTerm2 not installed, VS Code pending): - -| metric | Orca prod | Terminal.app | Ghostty | -|---|---|---|---| -| DSR idle p50/p99 (ms) | 0.69 / 22.7 | 0.35 / 0.68 | 0.19 / 0.72 | -| DSR under 1 MB/s agent load p50/p99 (ms) | **134 / 292** | 0.45 / 7.9 | 0.21 / 6.1 | -| agent-tui fenced throughput | **2.0 MB/s** | 37 | 78 | -| ascii-log fenced throughput | 13 MB/s | 39 | 93 | - -Decomposition of the 51× agent-tui gap — both pipeline ends are fast: - -- Bare `@xterm/headless` (114×85, scrollback 5000): agent-tui **103 MB/s** - (`terminal-headless-parse-bench.mjs`). The xterm parser is not the problem. -- Daemon `Session` ingest (emulator + pending-output recording + fanout): - agent-tui **103 MB/s** (`session-ingest-throughput.bench.test.ts`, - `ORCA_TERMINAL_PERF_BENCH=1`). The daemon is not the problem. - -Conclusions: (1) idle latency is fine — the extra process hop costs ~0.5 ms, -so the utilityProcess router is deprioritized by data; (2) the crisis is -queueing between daemon egress and renderer parse completion — main -per-chunk processing, the 512 KB delivery/ACK pacing (ACKs fire after -renderer write callbacks, so renderer slowness throttles delivery -multiplicatively), and renderer per-chunk layers above xterm; (3) the -agent-TUI shape (DEC-2026 frames + erase/repaint) is 6.5× worse than plain -text inside Orca while being equal-cost everywhere else — profile it in the -renderer first (task #9). - -### 2026-07-02 — dev-build check of #7139/#7150 (confounded; directional only) - -Dev build of orca-performance (282-col window, 3MB fixtures, dev-mode -overhead): DSR idle p50 0.64 ms (unchanged), **DSR under load p50 161 ms** — -the cooperative-drain branch does not move the under-load class. In -hindsight this is structural: DSR replies are ordered within the output -stream, so the metric measures output-queue depth; #7139 paces draining to -protect input-send responsiveness but cannot reorder the queue. Implications: -(1) the 134 ms-class number is fixed only by shrinking the queue (producer -flow control) or raising drain rate (the 51× throughput hunt); (2) #7153's -own wins (freeze class, bounded memory, input-loss guards) must be validated -with freeze scenarios and real typing, not DSR. Also learned: dev-mode runs -are ~2× slower across the board and fences need `--dsr-timeout-ms` headroom. - -### 2026-07-02 — 51× loss attributed: scheduler fixed-nap drip (task #9) - -The renderer output scheduler (`pane-terminal-output-scheduler.ts`) drained -at most 2×16KB per tick, then slept 4ms (high-priority) / 16ms (background) -regardless of parse speed. Isolation bench (fake timers, instant-parse -terminal — `pane-terminal-output-scheduler-throughput.bench.test.ts`, -`ORCA_TERMINAL_PERF_BENCH=1`): **background cadence = 1.9 MB/s — matching -prod's measured 2.0 MB/s agent-tui ceiling**; foreground = 27 MB/s (only -when arrivals re-poke 0ms drains; Chromium's ~4ms timer clamp makes the -sustained real-world HP ceiling ~8 MB/s). Classification: pty-connection's -`isLatencySensitiveForegroundOutput` routes sizable no-recent-input chunks -to the queue, so floods always ride the drip. - -Fix (committed 9e8bb2243): high-priority drains are now **parse-clocked** — -a pacer re-arms a 0ms drain when xterm's write callback confirms the batch -parsed — and carry 8 writes/tick (128KB ≈ 1.3ms parse). Isolation ceiling: -27 → **117.6 MB/s** (parse-limited). Background cadence deliberately -unchanged (protects the focused pane; hidden panes are term-speed-2's job). -`DRAIN_TIME_BUDGET_MS` still bounds tick work (cooperative-drain intent of -#7139 preserved; its budget-yield test still passes). 621 tests green. - -Open follow-ups from this attribution: (a) end-to-end dev verification (in -progress); (b) whether main's `background:true` delivery marking demotes -visible-pane floods to the background drip — check -`window.__terminalOutputSchedulerDebug` counters in a dev run; (c) ascii-log -gap (13 vs 83 MB/s headless) — likely per-chunk `beforeWrite` side-effect -scanning; profile after (a). - -### 2026-07-03 — THE WHALE: main's retained-tail redraw path is O(tail) per chunk - -Parse-clock fix didn't move end-to-end (agent-tui still 0.7 MB/s dev). Layered -probes (renderer scheduler counters → main whole-method timer → per-section -timers → targeted micro-benches) attributed it fully: - -- Renderer receives only ~350–770 KB/s — it is **starved**, not slow. -- `OrcaRuntime.onPtyData` consumes **~93% of main's event loop** during the - flood (~950 ms/s at ~450 chunks/s ≈ 2.1 ms/chunk). -- All wrapped sub-calls (OSC scanners, agent detect, watchers, headless - track, leaves loop, mobile touch) together: **~3.5%**. The remainder is the - pty-record tail block. -- Micro-bench (`appendNormalizedToTailBuffer` with a real agent-TUI frame - containing `ESC[10A ESC[0J`): **0.888 ms/chunk at a 2,000-line tail** — 32× - the plain-append path. Cause: `appendNormalizedToMultilineTailBuffer` - materializes ~2,001 row objects per chunk (orca-runtime.ts:22324) and - `finalizeRetainedTerminalRows` allocates them all again plus runs a - trailing-whitespace regex per row (:22458) — ~4k allocations + 2k regexes - per tiny chunk, twice the tail length in O(n) passes. Every Claude-Code - frame (cursor-up + erase-below) takes this path; plain logs don't — which - is exactly the measured agent-tui vs ascii asymmetry. - -Chain: TUI flood → O(tail) work per chunk in main → main event loop -saturates → daemon socket backpressures → renderer starved at ~0.4 MB/s → -deep queue → 134 ms DSR-under-load. - -Fix (in progress): run the existing algorithm on a lazy suffix window (the -cursor's maximum upward reach, computed from the chunk) with the untouched -prefix shared by reference; differential fuzz test proves output equality -against the original implementation. Worst case (pathological full-height -cursor-up) falls back to today's cost. - -### 2026-07-03 — windowed-tail fix: partial end-to-end win; next suspect queued - -Dev-build bench after the windowed redraw-tail fix (label dev-tailfix, same -protocol as dev-parseclock): agent-tui **0.7 → 1.0 MB/s (+43%)**, DSR-under- -load **p50 161 → 108 ms, p99 624 → 154 ms (4×)**. Real movement for the -first time, but the pipeline is still far from the renderer's 27–117 MB/s -capacity — another main-side consumer remains hot. - -Next cycle (exact recipe): re-apply the whole-method main probe -(`onPtyDataMs` sampler in `pty.ts` bindProviderListeners) on the fixed -build. If onPtyData still dominates, the remaining O(tail)/per-chunk -suspects in priority order: (1) `buildTerminalWaitText` ×2 per chunk (full -tail join, 0.116 ms/chunk in prod-node isolation — likely 2-4× that in -dev); (2) `normalizeTerminalChunk` (regex over every chunk, never measured); -(3) the per-leaf duplicate tail path when `tailStateMatches` fails. If -onPtyData no longer dominates, probe the main→renderer delivery batching -next. The probe/bench cycle is mechanical: relaunch dev -(`ELECTRON_ENABLE_LOGGING=1 pnpm dev`), `orca-dev terminal create --command -" --label X --size-mb 3 --dsr-timeout-ms 120000"`, grep the log. - -### 2026-07-03 — post-fix attribution: `blockedCheck` is the remaining whale - -Post-windowed-tail probe run (dev build, agent-tui): `onPtyData` still -~90% of main's event loop (~930 ms/s). Bucket split per second: -**blockedCheck ≈ 700–790 ms (~85%)**, waitText ≈ 70, append ≈ 25 (windowed -fix confirmed), normalize ≈ 7, preview ≈ 0. - -Mechanism (orca-runtime.ts:23128 `nextTailHasNewerBlockedReason` + its -callers): per chunk, TWO full wait texts are built (`buildTerminalWaitText` -joins the whole ≤256KB tail), then the check calls `.toLowerCase()` on both -(another ~512KB of string allocation per chunk) and runs multi-pattern -blocked/ready scans (`findTerminalWaitBlockedSignal`, -`findKnownReadyPromptIndex` — lastIndexOf/regex passes over the full text) -— all to timestamp `waitBlockedAt` for `terminal wait`. - -Fix design (next session): blocked/ready prompts are end-anchored — an -actionable prompt is at the END of output. (1) Run the check on a bounded -suffix of the wait text (last ~64 lines / 16KB) instead of the full tail; -(2) cheap pre-filter: skip entirely unless the appended chunk (plus a small -carry for split keywords) can contain a blocked keyword; (3) build the two -wait texts only when the check runs. Verification mirrors the windowed-tail -pattern: keep the full-text check as reference + differential fuzz over -randomized tails/prompts (split-across-chunks cases included — the -`appendCandidateSignal` ordering semantics at :23146 must be preserved), -plus the terminal-wait contract tests. Expected effect: removes ~85% of -remaining onPtyData cost; combined with the two landed fixes should -finally unlock the pipeline toward the renderer's measured 27–117 MB/s. - -### 2026-07-03 — pipeline unlocked: three stacked fixes, 16× throughput, 9× latency - -Dev-build bench with all three fixes (parse-clocked drains 9e8bb2243, -windowed tail 4e08a28cd, throttled blocked-check 66f20258e), label -dev-blockedfix, same protocol/config as prior dev rows: - -| metric | pre-fix dev | +tail fix | +blocked fix | -|---|---|---|---| -| agent-tui MB/s | 0.7 | 1.0 | **11.5** | -| DSR load p50/p99 (ms) | 161 / 624 | 108 / 154 | **18.8 / 24.9** | -| DSR idle p50/p99 (ms) | 0.95 / 21 | 1.09 / 18 | **0.52 / 8.6** | -| ascii-log MB/s | 6.4 | 4.7 | **9.6** | - -The agent-TUI-specific penalty is gone (agent-tui ≈ cjk ≈ ascii now). The -throttled blocked-check delivered the predicted ~85% cut. Dev mode carries -~2× overhead vs prod, so the prod build should land near ~10ms DSR-under- -load — from the 134ms baseline (~13×) — pending a packaged-build rerun. -Remaining floor is structural cadence (8ms daemon batch + 4ms HP drain -ticks + xterm 12ms slices), which flow control (#6) does not target; -re-evaluate the "within 10× of Terminal.app" goal line after a prod -measurement. Next: term-speed-2 revival (#4), then flow control (#6). - -### 2026-07-03 — term-speed-2 revival: merged, green, NOT yet mergeable (perf gate) - -`revive/term-speed-2` pushed (merge a5052c35f, tip 64b6f7abe): 144 files, -typecheck clean, ~2,776 targeted tests green, all three of our fixes -verified present, chain features present and kill-switched (subagent's -six review risks recorded in its report). Bench verdict on the revived -build (dev): DSR-load p50 ~19ms holds, but **throughput regressed ~35% -unconditionally** (agent-tui 11.5 → 7.2–7.4 MB/s; all-switches-OFF round -proved the kill switches are NOT the cost) and idle p50 doubled. - -Attribution so far: main exonerated (whole-method probe: onPtyData ~60ms/s -≈ 6%); renderer reconcile + HP-first selection O(1)-checked; **daemon -CONVICTED by unit bench — `Session` ingest 103 → 39.5/47.7 MB/s (2.2–2.6×) -on the revive branch** (`session-ingest-throughput.bench.test.ts`, -ORCA_TERMINAL_PERF_BENCH=1). Cause: the chain's headless-emulator -restructure (scanner classes / query-reply forwarding / view-attribute -responder) added per-byte cost to the daemon hot path. Chunks reaching -main are now ~5.8KB vs ~650B (daemon emits slower, batches bigger). - -NEXT (fast inner loop — pure unit bench, no app restarts): on -revive/term-speed-2, diff `headless-emulator.ts`/`session.ts` vs -7839fb9db, find the per-chunk scanner cost, restore our bounded-parser -fast paths (the daemon emulator must never pay per-byte JS scanning for -bytes that contain no ESC — same pre-filter pattern as the blocked-check -keyword bypass), verify with the ingest bench back at ~100 MB/s, then -full dev bench expecting blockedfix parity (~11.5 MB/s), THEN merge to -orca-performance. A residual renderer-side share is possible once the -daemon is fixed — re-attribute after. - -Merge gate: revive branch merges only at ≥ blockedfix numbers. - -**RETRACTION (2026-07-03, later):** the daemon conviction above was a -confounded measurement — the 39–48 MB/s ingest runs executed while a dev -app was still running. On a quiet machine the revive branch ingests at -**82–109 MB/s** (≈ pre-merge) and its HeadlessEmulator alone does 99.5 MB/s -vs raw xterm 77.7. The daemon is innocent. Consequently the end-to-end -revival delta (11.5 → 7.2/7.4 dev) is also UNTRUSTED — none of those runs -were load-controlled, and unit benches show up to 2.6× machine-load -variance. Scanner pre-filters landed anyway on revive (71c89da9b; -strictly positive, 641 daemon tests green). - -**New measurement protocol (mandatory from here):** quiet machine (no dev -apps or benches concurrent), paired A/B runs back-to-back alternating -branches, n≥2 per side, report spread not just p50. The merge-gate -comparison (blockedfix vs revive) must be redone under this protocol -before any verdict. Next: run the controlled A/B; if the delta -disappears, merge revive into orca-performance and proceed to flow -control (#6); if it persists, resume attribution renderer-side (probe -pty-connection dataCallback additions per chunk). - -### 2026-07-03 — A/B gate passed; term-speed-2 MERGED to orca-performance - -Load-controlled alternating A/B (fresh app per run, n=2/side, agent-tui + -DSR-load): perf 6.7/5.2 MB/s, dsr p50 19.9/21.3, p99 107.8/218.1; revive -6.1/3.6 MB/s, dsr p50 21.4/20.3, **p99 63.4/26.1**. Verdict: latency p50 -tied, p99 better on revive, throughput within overlapping noise (revive2's -3.6 followed two runtime-busy create failures). The earlier "35% -regression" is confirmed noise. Note: both branches ~5-7 MB/s today vs -11.5 yesterday — dev benches carry ~2x day-to-day machine variance; -absolute dev numbers are only comparable within one A/B session. - -Merged revive/term-speed-2 → orca-performance; typecheck clean, 288 -post-merge spot tests green. orca-performance now = main-ish base + #7153 -+ three perf fixes + full term-speed-2 chain (kill-switched, default ON) -+ scanner pre-filters. Extended user testing now covers everything. -Remaining from the revival agent's risk list: gate×drain e2e specs -(terminal-hidden-*, parked-memory, sleep-wake) still not run — queue them. -Next: producer flow control (#6) per design §5; prod packaged-build bench -for the real headline numbers. - -### 2026-07-03 — flow control merged; goal-state accounting - -Producer flow control merged to orca-performance (348aeb325): protocol -v19 `pausePty`/`resumePty`, 256KB/32KB watermarks on main's pendingData, -node-pty kernel backpressure, 5s daemon-side lost-resume failsafe + -main-side pause re-assert, resume on every teardown path, version-gated -(v≤18/SSH no-op), kill switch `PRODUCER_FLOW_CONTROL_ENABLED` -(ipc/pty.ts:143), 29 new tests. Typecheck + 292 post-merge spot tests -green. - -**Definition-of-done accounting:** -- 51× loss: ATTRIBUTED AND FIXED (three fixes; agent-tui 0.7→11.5 MB/s - and DSR-load p50 161→18.8 dev, results committed). -- term-speed-2: REVIVED AND MERGED (A/B gate passed). -- Flow control: IMPLEMENTED AND MERGED. -- "Within 10× of Terminal.app (4.5ms)": RE-SCOPED to pending a packaged - RC measurement. Evidence: dev = 18.8ms with ~2× dev overhead → prod - projection ~9-10ms ≈ 20× Terminal.app (vs 300× at baseline). The - remaining gap is structural cadence (daemon 8ms batch, renderer drain - ticks, xterm 12ms parse slices) — tunable follow-ups, distinct from the - waste class this initiative eliminated. Prod verification path: - electron-vite preview CANNOT host the bench (CLI-created panes are not - adopted by the preview window's renderer → no ACKs → pending-cap drop; - two attempts, documented) — measure on the next packaged RC cut from - orca-performance using the committed rig + protocol instead. - -**Deferred, ordered:** (1) sync orca-performance with main — conflicts -incl. stream-opcode collision (chain `Ack=12` vs main's #7205-era -`Metadata=12`; renumber chain side, audit mobile/web stream consumers); -(2) chain's e2e specs (hidden parking / parked memory / sleep-wake) — -gate×drain risk; (3) cadence tuning toward the 10× line; (4) rig -extensions + P90 telemetry (tasks #3/#8). - -### 2026-07-03 — PROD VERDICT: v1.4.121-rc.0 benchmarked (the headline numbers) - -Same rig, same protocol, same machine as the 1.4.91 baseline: - -| metric | 1.4.91 baseline | v1.4.121-rc.0 | change | -|---|---|---|---| -| DSR idle p50 | 0.69 ms | **0.44 ms** | = Terminal.app (0.45) | -| DSR under load p50 | 134 ms | **18.6 ms** | 7.2x | -| DSR under load p99 | 292 ms | **29.7 ms** | 9.8x | -| agent-tui | 2.0 MB/s | **11.2 MB/s** | 5.6x | -| styles-stress | 7.8 MB/s | **10.4 MB/s** | 1.3x | -| ascii-log | 13 MB/s | 11.0 MB/s | ~0.85x | -| cjk-emoji | 15 MB/s | 12.2 MB/s | ~0.81x | - -Reading: the anomalous TUI penalty is GONE — all four fixtures now sit at -a uniform ~11-12 MB/s, which is the scheduler pacing ceiling, not parse -CPU (prod ≈ dev for both latency and throughput; the pipeline is -cadence-bound, so faster prod code changes nothing). That uniform cap -also explains plain-text dipping slightly below baseline: ascii/cjk used -to run unpaced ahead of the old scheduler; now everything flows through -the same parse-clocked path. Goal line check: 18.6 ms = 41x Terminal.app -under load (goal was 10x = 4.5 ms) — NOT met; down from 300x. Idle IS at -parity. The remaining 4x is the named cadence stack (daemon 8 ms batch, -scheduler drain ticks + 8x16KB per-tick budget, xterm 12 ms slices) — -next lever, tunable, tracked as follow-up. p99 tail (the freeze class) -is 29.7 ms — users cannot perceive it. - -Caveat: measured on the user's live app (this session active in it); -idle p99 118 ms reflects that activity, not the terminal path. - -### 2026-07-03 — Same-engine reference: VS Code head-to-head (same machine, same rig) - -| metric | Orca v1.4.121-rc.0 | VS Code | verdict | -|---|---|---|---| -| DSR idle p50 | **0.44 ms** | 7.00 ms | Orca 16x faster | -| DSR load p50 | 18.6 ms | **7.18 ms** | VS Code 2.6x faster | -| DSR load p99 | **29.7 ms** | 43.4 ms | Orca 1.5x better tail | -| ascii-log | **11.0 MB/s** | 9.0 | Orca +22% | -| cjk-emoji | 12.2 | 11.3 | tie | -| agent-tui | 11.2 | 11.7 | tie | -| styles-stress | **10.4 MB/s** | 2.0 | Orca 5.2x | - -Orca now beats or ties the best-known xterm.js terminal on 5 of 6 -metrics — including 16x at idle (what users feel all day) and 5x on -SGR-heavy output — and holds a better p99 tail under load. Throughput -sits at the shared engine ceiling (~9-12 MB/s), confirming the class -limit. - -The one loss (load p50) has a clean mechanism: VS Code's producer flow -control caps unacked output at ~100KB, so its standing queue is -~100KB / 11.7 MB/s ≈ 8.5 ms — matching its 7.18. Our standing queue -(18.6 ms ≈ ~200KB at 11 MB/s) is set by the main→renderer ACK window -(512KB/pty high water) + drain re-arm cadence (Chromium clamps nested -setTimeout to ~4ms). Two levers, both cheap to test: (1) MessageChannel -drain scheduling (sub-ms re-arm; also raises the throughput ceiling); -(2) tighter effective in-flight window on the renderer delivery path. -Target: VS Code's ~7ms class or below without giving back throughput. - -### 2026-07-03 — Batch windows were the gap: dev DSR-load p50 19 -> 8.0ms - -Lever results (dev, 3MB protocol, same session): -- MessageChannel drains (2434dfaae): 19.01ms — NO change. Proved the - ~19ms was NOT queue depth: at 1MB/s vs ~11MB/s capacity (9% util) - there is no standing queue. Kept (correct, removes a real clamp). -- Batch windows 8->2ms on BOTH hops (e67a91d7a: daemon - STREAM_DATA_BATCH_INTERVAL_MS + main PTY_BATCH_INTERVAL_MS): - **p50 8.00 / p90 10.13 / p99 12.26ms** (from 19.01/22.7/28.1). - Throughput unchanged (agent-tui 9.8 vs 9.1, ambient noise). 239 - batcher+pty tests green after timing updates. - -Dev-mode 8.0ms already matches VS Code prod (7.18); prod build should -land BELOW VS Code. p99: ours 12.3 vs VS Code 43.4. The remaining -fixed-latency terms are renderer/xterm-internal (12ms parse slices). -Note: main's interactive bypass (input-gated) means real keystroke echo -skips batching entirely — the DSR metric understates real typing -responsiveness; VS Code measured on the same freight path, comparison -fair. - -Next: cut RC, confirm in prod, re-baseline vs Terminal.app (expect -~8-15x from 300x at baseline; goal line 10x = 4.5ms now plausibly in -reach). - -### 2026-07-03 — Chain e2e debt PAID: all 6 hidden-pane specs green - -terminal-hidden-view-parking (parks + restores rich TUI on reveal; bell/ -title side effects live while parked), terminal-sleep-wake-restore -(output restored + input accepted after wake), terminal-parked-memory -(renderer memory released on park; views retained when kill-switched -off): 6/6 passed, electron-headless, 1.1m. The gate x drain interplay — -the revival's top flagged risk — now has e2e coverage on the exact -branch the RC ships from. Remaining garble-hardening: differential -hide/reveal fuzz harness (next build), reveal-time seq diagnostics. - -## Success criteria (baseline-relative; finalize after task 1) - -- DSR-under-load p90 in Orca within striking distance of iTerm2 on the same - box; zero DSR timeouts (today's freeze class). -- Fenced agent-tui throughput ≥ VS Code on the same box. -- Idle RSS with 0–1 agents materially down (target set after the memory - harness lands; hidden-pane parking is the main lever). -- Zero >100 ms event-loop stalls in main/renderer during a 10 MB flood. -- Production P90 typing latency down and monitored continuously. diff --git a/notes/windows-perf-progress.md b/notes/windows-perf-progress.md deleted file mode 100644 index 51bee65e4f6..00000000000 --- a/notes/windows-perf-progress.md +++ /dev/null @@ -1,306 +0,0 @@ -# Windows Performance Investigation — Progress Log - -Goal: (1) significantly improve Windows startup time (~1 min cold start reported), -(2) fix OpenCode-driven UI freezes, (3) improve overall Windows performance. -All changes must be proven with before/after benchmark numbers. - -## Phase 2 (2026-07-02, branch Jinwoo-H/windows-performance-improvement) — terminal interaction latency - -Complaints: slow workspace switching, slow tab create/switch (terminal-related), occasional crashes. -Harness: `tools/benchmarks/terminal-perf-bench.mjs` (CDP-driven dev app, renderer-clock phase -timings; scenarios tab-create / tab-switch / workspace-switch; local git fixture). -Main-process spawn attribution: `ORCA_PTY_SPAWN_TIMING=1` → `[pty-spawn-timing]` lines -(pty.ts handler phases: preflight/auth/host_env/options/provider_spawn). - -Findings (baseline, this machine): -- Workspace switch: every hide disposed each pane's WebGL context; resume recreated it — - ~5ms macOS, 100-500ms/pane Windows ANGLE (the comment in terminal-visibility-resume.ts - admitted this). Premise (16-context budget) stale since #7064 raised budget to 128. -- Tab create: ~550ms steady state; main handler only ~115ms (host_env≈50ms, daemon - provider_spawn≈68ms). Remainder is renderer-side (xterm open + WebGL context for the new - pane + React mount). First-ever spawn paid +2.7s inside provider_spawn = daemon's first - ConPTY (native module + conpty.dll + OpenConsole + Defender), lazily on the user's first terminal. -- Tab switch: paint settle median 80-99ms; longtasks 64-151ms — every light tab resume runs - scheduleTerminalWebglAtlasRecovery: 3× (frame/120ms/500ms) global shared-atlas clear + - refresh of EVERY pane in EVERY manager. Parse-time recovery (pty-connection.ts - recoverWebglAtlasAfterParse / hiddenOutputNeedsAtlasRecoveryAfterParse) already covers - risky output including hidden. CAUTION: #7058 changed this area and was reverted (#7073) — - left as follow-up. -- LocalPtyProvider spawned without useConptyDll while the daemon path used it (legacy system - ConPTY corruption + perf differences on degraded-mode/fresh-local spawns). - -Fixes on this branch (PR #7080 merged in — WebGL release on dispose + stale pty:exit synthesis): -- A/D: WebGL context retention across hide/show and the suspended-pane atlas recovery scoping - were reverted/parked for more terminal lifecycle testing. Hidden workspaces return to the - previous dispose-on-hide behavior. -- B: useConptyDll for LocalPtyProvider spawns (local-pty-utils.ts) — parity with daemon. -- F: daemon boots a throwaway `cmd.exe /c exit` ConPTY (windows-conpty-warmup.ts) so the - first user terminal doesn't pay the ~2.7s first-ConPTY cost. - -Follow-ups (documented, not in this branch): -- Gate/scope the light-tab-switch atlas burst (see #7058/#7073 history first). Residual - tab-switch cost besides the burst: debounced ResizeObserver re-fit can reflow scrollback - when column count changed while hidden. -- Renderer-side tab-create cost (~400ms): mount chain runs new Terminal() + 5 eager addons - + synchronous attachWebgl (pane-lifecycle.ts:108) before the spawn IPC (deferred one rAF, - pty-connection.ts:5158). Candidate: defer WebGL attach for brand-new panes. -- Cold-restore respawn fan-out: reconnectPersistedTerminals does NOT spawn; the fan-out is - Terminal.tsx mounting a TerminalPane per restored tab at once — each fires connectPanePty - → rAF-deferred spawn IPC (pty-connection.ts:5158) with no concurrency cap. Cap belongs at - that renderer connect layer, not in reconnectPersistedTerminals. -- First terminal opened immediately after launch also waits on the one-time daemon-init - barrier (pty:spawn awaits getLocalPtyStartupPromise, ipc/pty.ts:2518; measured - preflight=0 in the bench because hydration had finished first, but an early Ctrl+T pays - it). In-daemon Windows shell resolution (pwsh -Version probe, PowerShell exe-chain - existsSync/statSync scan) is uncached per spawn; the conpty warm-up spawns cmd.exe so it - does not warm PowerShell resolution. Note the warm-up and an early first spawn serialize - on the daemon's single thread — the 1255ms post-fix first-spawn number is mostly queueing - behind the in-flight warm-up, not unwarmed cost. -- node-pty ≥1.2.0-beta defers conpty connect (spawn returns pid=0 fast) — would stop spawn - storms serializing the daemon loop. - -Pre-existing Windows-only test failures (also on main, CI is ubuntu-only): 5 attribution-shim -PATH assertions in src/main/ipc/pty.test.ts (path-separator artifacts). - -## Status - -- [x] Benchmark harness for startup time (`tools/benchmarks/startup-time-bench.mjs`) -- [x] Startup bottleneck FIXED + verified: **19.31s → 1.80s median** (fixture); - real-world profile was 62s of blocked main thread → now 0 icacls spawns steady-state -- [x] OpenCode freeze ROOT CAUSE found + fixed: MessagePart hook flood (see F5/D2). - Benchmark: 22.9 MB / 540 ms / 400 main-process fanouts per turn → 469 KB / 79 ms / 120 - (legacy vs throttled plugin behavior through the real hook HTTP pipeline) -- [x] General Windows sync-work audit (results below); audit item #2 (readHooksJson per - status IPC) investigated and found NOT hot — renderer barely calls those handlers. - Fixed pre-existing Windows-only test failures (hydrate-shell-path delimiter). -- [x] Windows ConPTY e2e perf validation (F7 below) - -## Key facts / environment - -- Branch: `Jinwoo-H/windows-launch-time` -- Electron app, entry: `src/main/index.ts` (~1557 lines) -- Existing startup diagnostics: `ORCA_STARTUP_DIAGNOSTICS=1` writes `[startup] ` lines to stderr - (`src/main/startup/startup-diagnostics.ts`) -- Prior art: PR #4618 "perf: speed up desktop startup", #5011 "stop main-thread PowerShell ACL storm - on env-store reads", #4526 "Avoid OpenCode config cleanup freezes on Windows", b240d5eee - "Measure startup hydration phases" - -## Follow-ups / known issues (out of scope for this branch) - -- Pre-existing Windows-only unit test failures: `daemon-pty-adapter.test.ts` (61) and - `history-manager.test.ts` (3, chmod-based fs-error simulation is a no-op on Windows). - Identical with/without this branch's changes. CI never sees them (ubuntu-only). -- Consider a Windows CI lane for the terminal-perf e2e suite (F6/F7) and these unit suites. -- Typing-latency load-sensitivity (F7): possible deeper work on daemon checkpoint - scheduling/priority if user reports persist after D3. -- Audit leftovers (F4): non-recursive `grantDirAcl` execFileSync on hook install - (installer-utils.ts:210) could be async; readHooksJson caching unnecessary (not hot). - -### D3 — Async checkpoint writes (implemented) - -`HistoryManager.checkpoint` (every ~5s per dirty session, Electron main process) switched -from writeFileSync+renameSync (~1MB snapshot JSON, inflated by Defender on Windows) to -fs.promises with the same tmp+rename atomicity; ordering preserved by the adapter's -checkpointInFlight guard. - -## Suspects (startup) - -1. **`grantDirAcl(userData, { recursive: true })`** — `src/main/index.ts:517-523`, win32 only, - runs **synchronously on the main process inside `openMainWindow()` before window creation**. - Spawns `icacls /grant:r :(OI)(CI)(F) /T /C` with a **60s timeout**. - The comment itself admits large userData dirs (tens of thousands of Chromium cache files) - can take >10s. This blocks first paint for the whole walk. Matches "1 minute launch" and - "Windows only". -2. Windows Defender real-time scan of exe/asar/native modules on cold start (environmental, - can't fix in code, but reducing file count / sync IO helps). -3. TBD: store sync load, daemon init, i18n init, sherpa-onnx native module load. - -### F3 — Baseline benchmark (2026-06-10) - -Harness: `node tools/benchmarks/startup-time-bench.mjs --label baseline --iterations 3 --files 28000` -(28k-file synthetic Chromium-cache-shaped userData fixture in %TEMP%, headless launch of -the electron-vite build with `ORCA_STARTUP_DIAGNOSTICS=1`, milestones parsed from stderr). - -| phase (median of 3) | baseline | -|---|---| -| spawnToAppReady | 857ms | -| appReadyToServices | 178ms | -| servicesToI18n | 2ms | -| i18nToOpenWindow | 7ms | -| **aclGrantMs** | **15.65s** | -| windowCreatedToLoaded | 1.06s | -| **totalToDidFinishLoad** | **19.31s** | - -ACL walk = 81% of total. (Fixture is kinder than the real profile: same file count but -freshly-written small files → real %APPDATA%\Orca measured 62s for the same command.) -JSON: tools/benchmarks/results/startup-baseline-2026-06-10T19-36-01-305Z.json - -### F4 — Sync main-thread audit (subagent, 2026-06-10) - -Ranked offenders beyond the ACL grant (#1): -2. `readHooksJson` + JSON.parse re-read per agent-status IPC call across ~10 hook services - (`src/main/*/hook-service.ts` via `agent-hooks/installer-utils.ts:50`) — 10-100ms per - status snapshot, all platforms. Remediation: in-memory cache. -3. `whoami.exe` SID resolution (win32-utils.ts:92) — already cached, OK. -4. macOS-only `defaults read` per browser probe — not Windows. -5. `installer-utils.ts:210` non-recursive grantDirAcl on hook install (execFileSync, - 500ms-2s) — infrequent write path, low priority. -6. `secure-file.ts` sync PowerShell on credential write path — by design (#5011), leave. - -## Suspects (OpenCode freeze) - -- User report: UI freezes ~5s after sending prompt; OpenCode session itself continues fine - (visible from external terminal). So the agent process is healthy — the freeze is in Orca's - main process or renderer. Spinner in left panel still animates (= renderer compositor alive? - or just that one timer). Need to find sync main-process work triggered by OpenCode activity. -- Prior fix #4526 "Avoid OpenCode config cleanup freezes on Windows" — re-check that path. - -### Research results (subagent, 2026-06-10) — ranked candidates - -1. **ConPTY output flood vs PTY batching/backpressure** (HIGH): Windows ConPTY re-renders - full TUI frames → 10-100x output volume vs macOS. Batching in `src/main/ipc/pty.ts` - (16KB chunks / 8ms flush, 512KB renderer in-flight window). If renderer xterm.write is - slow, ACKs stall → in-flight fills → main stalls. Tests: terminal-foreground-redraw-freeze, - artificial-opencode-terminal-load e2e. -2. **Sync `runtime.onPtyData` per data event before batching** (MED-HIGH): - `src/main/ipc/pty.ts:1376-1430` → `orca-runtime.ts:3256-3420`: normalizeTerminalChunk + - tail-buffer append + agent-status OSC parsing run synchronously per chunk on main. - Daemon PTY path. High event rate × per-event cost can saturate the main loop. -3. **`mirrorUserConfig` recursive fs work in `buildPtyEnv` on PTY spawn** (MED): - `src/main/opencode/hook-service.ts:359-524` + `pty/overlay-mirror.ts:63-110` — - readdir/safeRemoveTree/symlinks on main thread at spawn; #4526 fixed only clearPty side. - Timing mismatch with "5s after prompt" though. -4. Agent-status event fan-out per OSC title (LOW-MED). 5. Tail-buffer O(n²) (LOW). - -Gap in coverage: no test exercises rapid continuous ConPTY-scale data + sync onPtyData -accumulation on Windows. - -### F5 — ROOT CAUSE (2026-06-10): OpenCode MessagePart hook flood - -Eliminated candidates first: ran `terminal-foreground-redraw-freeze.spec.ts` on THIS Windows -machine (real ConPTY + daemon provider) — passes; renderer output scheduler protections hold. -The raw TUI-output-flood theory doesn't explain an OpenCode-specific permanent freeze. - -The actual mechanism (src/main/opencode/hook-service.ts plugin source): -- OpenCode publishes `message.part.updated` with the FULL accumulated text of the part on - every streamed append (architecture: parts are republished, not deltas). -- Orca's plugin POSTed that full text to the agent-hook server on EVERY event → - **O(n²) bytes per streaming turn**. A 120KB reply in 400 updates = ~23 MB through - loopback HTTP + main-process JSON.parse; real turns are worse (per-token updates). -- Main process spends its whole loop on HTTP receive + parse + normalize + fanout. UI symptom - matches the user report exactly: everything dead (window close needs main + renderer - round-trip), EXCEPT the sidebar agent indicator — which is the one thing fed by the very - agentStatus:set flood that's starving everything else. -- Why Windows-biased: same flood exists on macOS but combines on Windows with ConPTY - full-frame redraw volume and generally slower process IO; also Windows daemon-PTY path - adds main-process onPtyData work. -- Why "5 seconds after sending the prompt": that's when the accumulated text gets big. -- Why OpenCode keeps working: plugin POST failures are swallowed; the session is healthy. -- Downstream payloads were already bounded (prompt 200 chars, lastAssistantMessage 8000 - chars via agent-status-types normalization) — the renderer wasn't the bottleneck; the - main-process ingest was. - -### F7 — Windows ConPTY e2e perf validation (2026-06-10) - -Ran the terminal-perf budget specs on this Windows machine (real ConPTY + daemon PTY -provider — a path CI never exercises): -- `terminal-output-scheduler.spec.ts`: PASS (all tests) -- `terminal-foreground-redraw-freeze.spec.ts`: PASS -- `terminal-typing-latency.spec.ts`: PASSES in isolation, repeatedly — median 13.6-23.1ms, - worst 34-42ms (budgets: 250ms median / 1000ms worst). Two earlier runs that exceeded the - worst-key budget (1054.9ms, 2016.1ms outlier on a single key) occurred while other heavy - tooling (vitest/tsgo/builds) ran concurrently on the machine → load-sensitivity, not a - deterministic product defect. Note the product implication: under heavy host load - (exactly what coding agents generate), a keystroke can stall >1s on Windows. Plausible - contributors for follow-up: daemon checkpoint ticks (5s interval; snapshot serialize in - daemon + sync writeFileSync of checkpoint JSON on main — daemon-pty-adapter.ts:592, - history-manager.ts:109), Defender scanning fresh build artifacts. - -### F6 — Windows e2e perf coverage gap - -All terminal-perf e2e specs run on ubuntu-latest in CI. Verified they DO run on a Windows -dev machine (`npx playwright test ... --project electron-headless` works locally). Consider -a Windows CI lane for the terminal-perf suite. - -## OpenCode fix (D2) - -1. **Plugin throttle + cap (source fix)** — `src/main/opencode/hook-service.ts`: - assistant MessagePart posts are trailing-edge coalesced to ≥250ms apart and text is - capped at 4000 chars (leading edge posts immediately so previews stay snappy; pending - snapshot flushed before SessionIdle so the done-row preview is the final message; user - prompts bypass the throttle slot). Plugin file is rewritten on every Orca-launched - OpenCode spawn, so the fix deploys to new sessions immediately. -2. **Listener-side cap (stale-plugin defense)** — `src/shared/agent-hook-listener.ts`: - OpenCode MessagePart text capped at 8000 chars at ingest (OPENCODE_HOOK_TEXT_MAX_CHARS) - so pre-fix plugins in long-running OpenCode processes can't blow up state maps. -3. **Benchmark/regression test** — `src/main/agent-hooks/opencode-message-part-flood-bench.test.ts` - drives the real hook HTTP pipeline with both behaviors. Measured on this machine: - | metric/turn | legacy plugin | throttled plugin | - |---|---|---| - | posts | 400 | 120 | - | bytes through main | 22.9 MB | 469 KB (49x less) | - | wall time | 540 ms | 79 ms | - | listener fanouts | 400 | 120 | -4. Behavioral plugin tests — `src/main/opencode/hook-plugin-message-part-throttle.test.ts` - executes the generated plugin with fake timers + stubbed fetch. - -## Findings - -### F1 — Recursive icacls walk is the ~1 min startup (CONFIRMED, 2026-06-10) - -- This machine's real packaged-Orca userData: `%APPDATA%\Orca` = **28,650 files / 2.06 GB** - (mostly Chromium caches: Cache, Code Cache, GPUCache, blob_storage…). -- Measured the exact command Orca runs in `openMainWindow()` (src/main/index.ts:517-523): - - `icacls /grant:r :(OI)(CI)(F) /T /C` → **62.0 s** - - App runs it with `execFileSync` (main thread, BLOCKING, before BrowserWindow creation) - with a **60s timeout** → every cold launch freezes ~60s, then the grant *times out and - silently fails* (execFileSync throws, caught). Users pay the full minute and get nothing. - - Non-recursive root-only grant: **4.8 s** (NTFS propagates inheritable ACE internally). - - `icacls \* /grant:r …` (immediate children, 48 entries): **4.7 s**. -- Why it exists (PR #1152): Chromium's BrowserWindow ctor resets userData DACL with - Inherit-Only ACEs → EPERM on writes in existing subdirs (codex-runtime-home, agent-hooks…). - Explicit child ACEs survive propagation. Per-write EPERM retries exist as backstop in - `codex-accounts/fs-utils.ts` + `agent-hooks/installer-utils.ts`. -- Windows ACL inheritance recalculates from the immediate parent during propagation, so - explicit ACEs on userData + immediate children are sufficient; per-file ACEs on 28k - Chromium cache files are useless work. - -### F2 — Instrumentation prior art - -- `ORCA_STARTUP_DIAGNOSTICS=1` → `[startup] ` lines on stderr (startup-diagnostics.ts). - Only 2 events exist today (single-instance lock). Commit b240d5eee (branch - perf/startup-first-window, NOT merged here) has a full StartupPhaseTimer framework — - too large to cherry-pick; adding minimal milestone logs instead. -- Hermetic benchmark launch path: `ORCA_E2E_USER_DATA_DIR=` redirects userData - (works packaged + dev), `ORCA_E2E_HEADLESS=1` keeps window hidden. Dev/preview mode - skips single-instance lock → safe alongside installed Orca. - -## Decisions / fixes - -### D0 — RESULTS: ACL fix benchmark (2026-06-10) - -| phase (median) | baseline (3 it.) | after fix (4 it.) | steady state (3 it.) | -|---|---|---|---| -| aclGrantMs | **15.65s sync/blocking** | async (off critical path) | **0ms (marker hit)** | -| totalToWindowCreated | 18.25s | 930ms | 814ms | -| totalToDidFinishLoad | **19.31s** | **2.04s** | **1.80s** | - -- First launch after fix: total 2.06s while the background grant ran 6.81s concurrently. -- Marker verified written by real icacls run; subsequent launches log `acl-grant-done - mode=marker-hit` with zero spawns. -- JSON evidence: tools/benchmarks/results/startup-{baseline,acl-fix,acl-fix-steady}-*.json -- Files: src/main/startup/windows-user-data-acl.ts (+tests), src/main/index.ts (wire-up + - startup milestones), src/main/win32-utils.ts (export identity resolver), - tools/benchmarks/startup-time-bench.mjs (harness). - -### D1 — ACL grant fix (implemented as planned) - -Replace the synchronous recursive walk with: -1. A persisted marker (`windows-acl-grant.json` in userData, keyed on identity + scheme - version): when present → skip everything (steady-state launches: 0 icacls spawns, 0 ms). -2. When marker missing (first launch after install/profile import): grant root + - immediate children via **async spawn** (never blocks window creation); write marker - on success. Per-write EPERM retries remain the backstop during the async window — - that's exactly what they're for (#1152 comment says so). -3. Drop the /T full-tree walk entirely; it grants nothing the immediate-children - ACEs + inheritance propagation don't already cover. diff --git a/package.json b/package.json index e62fce386e9..22c7335d5c2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "orca", - "version": "1.4.139-rc.2", + "version": "1.4.139", "description": "Next-gen IDE for parallel agentic development", "homepage": "https://github.com/stablyai/orca", "author": "stablyai", diff --git a/src/main/browser/popup-origin-bar-window.test.ts b/src/main/browser/popup-origin-bar-window.test.ts index c0161dd94e8..3e816e74b07 100644 --- a/src/main/browser/popup-origin-bar-window.test.ts +++ b/src/main/browser/popup-origin-bar-window.test.ts @@ -53,6 +53,11 @@ const { fakeElectron } = vi.hoisted(() => { webContents: FakeWebContents setBounds = vi.fn() constructor(options: { webContents?: FakeWebContents; webPreferences?: unknown }) { + // Why: Electron rejects explicit undefined instead of treating it as + // omitted, which the popup fallback path depends on. + if (Object.hasOwn(options, 'webContents') && options.webContents === undefined) { + throw new TypeError('options.webContents must be a WebContents') + } this.options = options this.webContents = options.webContents ?? createFakeWebContents() FakeWebContentsView.instances.push(this) @@ -186,6 +191,7 @@ describe('openPopupWithOriginBar', () => { it('loads the target itself only when no pre-created contents were provided', () => { const popup = openPopupWithOriginBar({}, 'https://example.com/login') + expect(lastViews().content.options).not.toHaveProperty('webContents') expect(popup.contentWebContents.loadURL).toHaveBeenCalledWith('https://example.com/login') }) diff --git a/src/main/browser/popup-origin-bar-window.ts b/src/main/browser/popup-origin-bar-window.ts index 504c862a5b8..c9ac057c177 100644 --- a/src/main/browser/popup-origin-bar-window.ts +++ b/src/main/browser/popup-origin-bar-window.ts @@ -120,7 +120,9 @@ export function openPopupWithOriginBar( webPreferences: { contextIsolation: true, nodeIntegration: false, sandbox: true } }) const contentView = new WebContentsView({ - webContents: options.webContents, + // Why: Electron rejects an explicitly undefined webContents; omitting it + // lets WebContentsView create contents for Cmd/Ctrl-click popups. + ...(options.webContents === undefined ? {} : { webContents: options.webContents }), webPreferences: options.webPreferences }) window.contentView.addChildView(contentView) diff --git a/src/main/github/client-work-items.test.ts b/src/main/github/client-work-items.test.ts index 66e728908df..d0e93eb306c 100644 --- a/src/main/github/client-work-items.test.ts +++ b/src/main/github/client-work-items.test.ts @@ -189,7 +189,9 @@ describe('listWorkItems', () => { '--repo', 'acme/widgets', '--assignee', - '@me' + '@me', + '--search', + 'sort:updated-desc' ], { cwd: '/repo-root' } ) @@ -205,7 +207,9 @@ describe('listWorkItems', () => { '--repo', 'acme/widgets', '--assignee', - '@me' + '@me', + '--search', + 'sort:updated-desc' ], { cwd: '/repo-root' } ) @@ -406,7 +410,9 @@ describe('listWorkItems', () => { 'acme/widgets', '--state', 'open', - '--draft' + '--draft', + '--search', + 'sort:updated-desc' ], { cwd: '/repo-root' } ) @@ -464,7 +470,9 @@ describe('listWorkItems', () => { '--repo', 'acme/widgets', '--state', - 'merged' + 'merged', + '--search', + 'sort:updated-desc' ], { cwd: '/repo-root' } ) @@ -528,7 +536,7 @@ describe('listWorkItems', () => { const { items } = await listWorkItems('/repo-root', 10, 'is:pr is:closed') expect(ghExecFileAsyncMock).toHaveBeenCalledWith( - expect.arrayContaining(['--state', 'closed', '--search', '-is:merged']), + expect.arrayContaining(['--state', 'closed', '--search', '-is:merged sort:updated-desc']), { cwd: '/repo-root' } ) expect(items).toMatchObject([{ id: 'pr:9', type: 'pr', state: 'closed' }]) @@ -597,7 +605,7 @@ describe('listWorkItems', () => { expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) expect(ghExecFileAsyncMock).toHaveBeenCalledWith( - expect.arrayContaining(['--search', 'review-requested:@me']), + expect.arrayContaining(['--search', 'review-requested:@me sort:updated-desc']), { cwd: '/repo-root' } ) expect(ghExecFileAsyncMock).not.toHaveBeenCalledWith( @@ -606,6 +614,34 @@ describe('listWorkItems', () => { ) }) + it('pins list ordering to updated-desc so the updatedAt cursor pages consistently', async () => { + getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }) + + await listWorkItems('/repo-root', 10, 'is:issue is:open') + + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + expect.arrayContaining(['--search', 'sort:updated-desc']), + { cwd: '/repo-root' } + ) + }) + + it('combines the inclusive updatedAt cursor with updated-desc ordering on later pages', async () => { + getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }) + + await listWorkItems('/repo-root', 10, 'is:issue is:open', '2026-07-01T00:00:00Z') + + // Why: the bound is inclusive (`<=`) so boundary items sharing the cursor's + // exact updatedAt aren't skipped; the renderer dedupes the re-fetched rows. + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + expect.arrayContaining(['--search', 'updated:<=2026-07-01T00:00:00Z sort:updated-desc']), + { cwd: '/repo-root' } + ) + }) + it('returns open issues and PRs for the all-open preset query', async () => { getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) @@ -652,7 +688,9 @@ describe('listWorkItems', () => { '--repo', 'acme/widgets', '--state', - 'open' + 'open', + '--search', + 'sort:updated-desc' ], { cwd: '/repo-root' } ) @@ -667,7 +705,9 @@ describe('listWorkItems', () => { '--repo', 'acme/widgets', '--state', - 'open' + 'open', + '--search', + 'sort:updated-desc' ], { cwd: '/repo-root' } ) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 5848454e6dd..5330de5f701 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -455,6 +455,12 @@ type MainWorkItem = Omit const WORK_ITEM_ISSUE_LIST_JSON_FIELDS = 'number,title,state,url,labels,updatedAt,author,assignees' +// Why: the Tasks pager slices pages on updatedAt, so every list/search fetch +// must return rows newest-updated-first for the cursor to advance correctly. +// This is the search-qualifier spelling of the same contract the REST list +// paths express as `sort=updated&direction=desc`; keep the two in sync (#8649). +const WORK_ITEM_LIST_SORT_QUALIFIER = 'sort:updated-desc' + const WORK_ITEM_PR_LIST_JSON_FIELDS = 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests' @@ -996,11 +1002,13 @@ function buildWorkItemListArgs(args: { if (excludeMergedFromClosed) { searchParts.push('-is:merged') } - // Why: cursor-based pagination. GitHub search supports updated: 0) { - out.push('--search', searchParts.join(' ')) - } + // Why: pagination cursors slice on updatedAt, but `gh issue list` defaults + // to created-desc and `--search` defaults to best-match. `gh issue/pr list` + // has no --sort flag, so the only lever is the search query — which forces us + // to always emit --search. Without pinning the sort, recently-updated old + // items never appear on any page, so the pager advertises pages the fetch + // chain can never reach (#8649). + searchParts.push(WORK_ITEM_LIST_SORT_QUALIFIER) + out.push('--search', searchParts.join(' ')) return out } diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index 3b538d44335..99fc42d7111 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -205,6 +205,7 @@ import { type TaskPageRepoSourceState } from '@/components/task-page-cache-selectors' import { shouldHideTaskPageListChrome } from '@/components/task-page-list-chrome-visibility' +import { accumulateWorkItemPages } from '@/components/task-page-work-item-pagination' import LinearIssueAttributeFilterDropdowns from '@/components/linear-issue-attribute-filter-dropdowns' import { resolveLinearIssueAttributeFilterPrimaryTeam } from '@/components/linear-issue-attribute-filter-primary-team' import { @@ -6280,32 +6281,32 @@ export default function TaskPage(): React.JSX.Element { setPaginationLoading(true) setLoadingTargetPage(target) try { - let cursor = oldestItem.updatedAt - let loadedPages = pages.length - const newPages: GitHubWorkItem[][] = [] - - while (loadedPages <= target) { - const { items } = await fetchWorkItemsNextPage( - repoArgs, - PER_REPO_FETCH_LIMIT, - CROSS_REPO_DISPLAY_LIMIT, - q, - cursor - ) - if (paginationGenerationRef.current !== requestGeneration) { - return + const result = await accumulateWorkItemPages({ + existingPages: pages, + initialCursor: oldestItem.updatedAt, + targetPage: target, + // Why: uniform page size keeps deduped pages from shrinking below the + // size `totalPages` (count ÷ effectivePageSize) assumes — otherwise + // the per-page overlap loss would strand the tail items. + pageSize: effectivePageSize, + isCancelled: () => paginationGenerationRef.current !== requestGeneration, + fetchPage: async (cursor) => { + const { items } = await fetchWorkItemsNextPage( + repoArgs, + PER_REPO_FETCH_LIMIT, + CROSS_REPO_DISPLAY_LIMIT, + q, + cursor + ) + return { items } } - if (items.length === 0) { - break - } - newPages.push(items) - cursor = items.at(-1)!.updatedAt - loadedPages += 1 + }) + if (result.cancelled) { + return } - - if (newPages.length > 0) { - setPages((prev) => [...prev, ...newPages]) - setCurrentPage(target < loadedPages ? target : loadedPages - 1) + if (result.newPages.length > 0) { + setPages((prev) => [...prev, ...result.newPages]) + setCurrentPage(target < result.loadedPages ? target : result.loadedPages - 1) } } catch (err) { console.error('Failed to load next page:', err) @@ -6316,7 +6317,14 @@ export default function TaskPage(): React.JSX.Element { } } }, - [paginationLoading, selectedRepos, pages, appliedTaskSearch, fetchWorkItemsNextPage] + [ + paginationLoading, + selectedRepos, + pages, + appliedTaskSearch, + fetchWorkItemsNextPage, + effectivePageSize + ] ) useEffect(() => { diff --git a/src/renderer/src/components/native-chat/NativeChatApprovalCard.tsx b/src/renderer/src/components/native-chat/NativeChatApprovalCard.tsx index cbf041f62e3..9b394ada907 100644 --- a/src/renderer/src/components/native-chat/NativeChatApprovalCard.tsx +++ b/src/renderer/src/components/native-chat/NativeChatApprovalCard.tsx @@ -19,35 +19,37 @@ export function NativeChatApprovalCard({ onChoose }: NativeChatApprovalCardProps): React.JSX.Element { return ( -
-
-
- -
-

{approval.title}

- {approval.detail ? ( -

- {approval.detail} -

- ) : null} +
+
+
+
+ +
+

{approval.title}

+ {approval.detail ? ( +

+ {approval.detail} +

+ ) : null} +
+
+
+ {approval.options.map((opt, i) => ( + + ))}
-
-
- {approval.options.map((opt, i) => ( - - ))}
diff --git a/src/renderer/src/components/native-chat/NativeChatComposerField.tsx b/src/renderer/src/components/native-chat/NativeChatComposerField.tsx index d3149c8c4ca..5a7b2a90255 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposerField.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposerField.tsx @@ -85,8 +85,9 @@ export function NativeChatComposerField({ }: NativeChatComposerFieldProps): React.JSX.Element { return (
-
-
+ {/* Extra bottom padding keeps the input box off the window rim. */} +
+
{autocomplete.mode === 'slash' && autocomplete.suggestions.length > 0 ? ( {imageAttachments.length > 0 ? ( diff --git a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx index 39bbba9a75e..947d5f65c3e 100644 --- a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx +++ b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useState } from 'react' +import { useEffect, useMemo, useRef, useState } from 'react' import { useAppStore } from '../../store' import { parseInteractivePrompt } from './native-chat-interactive-prompt' import { nativeChatCardDismissKey } from './native-chat-dismiss-key' @@ -23,11 +23,19 @@ import type { NativeChatInteractiveSend } from './use-native-chat-interactive-se export function NativeChatInteractiveCard({ paneKey, send, - canSend + canSend, + onShowingQuestionChange, + answerInputRef }: { paneKey: string send: NativeChatInteractiveSend canSend: boolean + /** Reports whether a question card is on screen so the view can replace the + * composer with it (the card's free-text row is the answer input). */ + onShowingQuestionChange?: (showing: boolean) => void + /** Forwarded to the question card's free-text row so pane-level Paste keeps + * a target while the composer is unmounted. */ + answerInputRef?: React.RefObject }): React.JSX.Element | null { const interactivePrompt = useAppStore( (s) => s.agentStatusByPaneKey[paneKey]?.interactivePrompt ?? null @@ -43,15 +51,40 @@ export function NativeChatInteractiveCard({ ) const cardKey = useMemo(() => nativeChatCardDismissKey(card), [card]) const [dismissedKey, setDismissedKey] = useState(null) + // A question answer is a paced multi-step write (body→Enter per question); keep + // the card up until it settles instead of dismissing on the click, so it doesn't + // vanish mid-send. `submitting` also gates a second submit racing the first. + const dismissTimerRef = useRef | null>(null) + const submittingRef = useRef(false) + const clearDismissTimer = (): void => { + if (dismissTimerRef.current) { + clearTimeout(dismissTimerRef.current) + dismissTimerRef.current = null + } + submittingRef.current = false + } + // Keyed on cardKey (and unmount): a new prompt can replace the current one + // before the settle timer fires, and a stale `submitting` gate would swallow + // the new card's first answer. + useEffect(() => clearDismissTimer, [cardKey]) // Forget the dismissal once the prompt clears so a fresh prompt can show. const present = card != null useEffect(() => { if (!present) { setDismissedKey(null) + clearDismissTimer() } }, [present]) + // Tell the view when a question card is up so it can hide the composer (this + // card supplies its own input). Reset on unmount so the composer comes back. + const showingQuestion = card?.kind === 'question' && canSend && cardKey !== dismissedKey + useEffect(() => { + onShowingQuestionChange?.(showingQuestion) + return () => onShowingQuestionChange?.(false) + }, [showingQuestion, onShowingQuestionChange]) + if (!card || !canSend || cardKey === dismissedKey) { return null } @@ -60,11 +93,23 @@ export function NativeChatInteractiveCard({ { - setDismissedKey(cardKey) - sendAnswer(text) + if (submittingRef.current) { + return + } + submittingRef.current = true + const settleMs = sendAnswer(text) + // Hold the card until the paced write finishes, then mark it answered + // (which hides it and restores the composer). + dismissTimerRef.current = setTimeout(() => { + setDismissedKey(cardKey) + submittingRef.current = false + dismissTimerRef.current = null + }, settleMs) }} onCancel={() => { + clearDismissTimer() setDismissedKey(cardKey) cancel() }} diff --git a/src/renderer/src/components/native-chat/NativeChatMessageList.tsx b/src/renderer/src/components/native-chat/NativeChatMessageList.tsx index 313845c2030..71aee362083 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageList.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.tsx @@ -167,7 +167,9 @@ function MessageRow({ // transcript caught up.) return (
-
+ {/* User turns get a distinct muted fill (not the card/canvas color) so + the prompt reads apart from the assistant's body copy. */} +
{markdown ? ( <> @@ -213,7 +215,7 @@ function MessageRow({ ) : null} @@ -251,6 +253,7 @@ export function NativeChatMessageList({ failedDeliveryMessageIds?: ReadonlySet }): React.JSX.Element { const scrollRef = useRef(null) + const contentRef = useRef(null) const [stuckToBottom, setStuckToBottom] = useState(true) const [showJump, setShowJump] = useState(false) @@ -312,6 +315,11 @@ export function NativeChatMessageList({ if (!container) { return } + // Detach synchronously (not just via the pending onScroll) so an in-place + // streaming growth can't re-pin to the bottom mid-flight and fight this + // deliberate scroll. The ref is what the resize observer reads. + stuckToBottomRef.current = false + setStuckToBottom(false) const delta = el.getBoundingClientRect().top - container.getBoundingClientRect().top container.scrollTo({ top: container.scrollTop + delta, behavior: 'smooth' }) }, []) @@ -334,17 +342,31 @@ export function NativeChatMessageList({ } }, [messages.length, isWorking, showTypingIndicator, scrollToBottom]) - // Keep the affordances in sync if the container resizes (e.g. composer mounts, - // viewport reflow) without a scroll event. + // Content growing without a message-count change (a streaming assistant turn + // extends its own message in place) never re-fires the layout effect above. + // Observe the container so those in-place growths still re-pin: stay glued to + // the bottom while stuck, otherwise just refresh the jump affordance. This is + // what removes most "Jump to latest" clicks during a live response. useEffect(() => { const el = scrollRef.current if (!el || typeof ResizeObserver === 'undefined') { return } - const observer = new ResizeObserver(handleScroll) + const observer = new ResizeObserver(() => { + if (stuckToBottomRef.current) { + scrollToBottom() + } else { + handleScroll() + } + }) + // Observe the growing content, not just the fixed-height viewport, so an + // in-place streaming growth is seen; also watch the viewport for reflows. observer.observe(el) + if (contentRef.current) { + observer.observe(contentRef.current) + } return () => observer.disconnect() - }, [handleScroll]) + }, [handleScroll, scrollToBottom]) return (
@@ -354,7 +376,8 @@ export function NativeChatMessageList({ className="scrollbar-sleek h-full overflow-y-auto px-3 pt-10 pb-4 sm:px-4" >
void /** Dismiss the prompt (sends Escape to the agent). */ onCancel: () => void + /** Exposes the free-text row so pane-level Paste can target it while the + * card replaces the composer. */ + answerInputRef?: RefObject } -// Synthetic option value for the "Other…" free-text row, kept out of the -// answer text and replaced by the typed value when selected. -const OTHER = '__other__' - /** - * Native renderer for an agent's AskUserQuestion prompt as a wizard: one - * question per step with tabs across the top (tap to jump, a check once - * answered), single- or multi-select option rows, an "Other…" row that reveals a - * free-text input, and a Next button that advances and becomes "Send answer" on - * the last step. Matches the desktop chat's neutral shadcn styling. + * Native renderer for an agent's AskUserQuestion prompt: a numbered pick-list + * (mobile/Claude-Code parity) with a header + close, a hover-highlighted row per + * option, and an always-present free-text row for a custom answer. Single-select + * commits on click; multi-select toggles and confirms via the trailing action. + * Multi-question prompts step through tabs across the top. Neutral shadcn tokens. */ export function NativeChatQuestionCard({ prompt, onAnswer, - onCancel + onCancel, + answerInputRef }: NativeChatQuestionCardProps): React.JSX.Element { const [index, setIndex] = useState(0) const [selections, setSelections] = useState(() => prompt.questions.map(() => [])) const [otherText, setOtherText] = useState(() => prompt.questions.map(() => '')) - const toggle = (qi: number, label: string, multi: boolean): void => { - setSelections((prev) => { - const next = prev.map((s) => [...s]) - const cur = next[qi] ?? [] - if (multi) { - next[qi] = cur.includes(label) ? cur.filter((l) => l !== label) : [...cur, label] - } else { - next[qi] = cur.includes(label) ? [] : [label] - } - return next - }) - } + const total = prompt.questions.length + const isLast = index === total - 1 + const q = prompt.questions[index]! const setOther = (qi: number, value: string): void => { setOtherText((prev) => { @@ -53,28 +44,19 @@ export function NativeChatQuestionCard({ }) } - // The resolved answer for a question: picked labels plus the typed "Other" - // value (which replaces the synthetic OTHER marker). - const answerFor = (qi: number): string => { - const picked = (selections[qi] ?? []).filter((l) => l !== OTHER) - const other = (selections[qi] ?? []).includes(OTHER) ? (otherText[qi] ?? '').trim() : '' - return [...picked, other].filter((p) => p.length > 0).join(', ') + // The resolved answer for a question: picked labels plus any typed free-text. + const answerFor = (qi: number, sel = selections, oth = otherText): string => { + const picked = sel[qi] ?? [] + const other = (oth[qi] ?? '').trim() + return [...picked, ...(other ? [other] : [])].join(', ') } - const total = prompt.questions.length - const isLast = index === total - 1 - const currentAnswered = useMemo( - () => answerFor(index).length > 0, - // eslint-disable-next-line react-hooks/exhaustive-deps - [selections, otherText, index] - ) + const currentAnswered = answerFor(index).length > 0 - const submit = (): void => { - // Build per-question label lists, substituting the typed Other value, then - // format to one line per answered question. + const submitAll = (sel: string[][], oth: string[]): void => { const resolved = prompt.questions.map((_, i) => { - const picked = (selections[i] ?? []).filter((l) => l !== OTHER) - const other = (selections[i] ?? []).includes(OTHER) ? (otherText[i] ?? '').trim() : '' + const picked = sel[i] ?? [] + const other = (oth[i] ?? '').trim() return [...picked, ...(other ? [other] : [])] }) const text = formatAskAnswer(prompt, resolved) @@ -83,22 +65,60 @@ export function NativeChatQuestionCard({ } } - const advance = (): void => { + // Advance to the next question, or submit on the last one — always from an + // explicit snapshot so a just-committed single-select pick isn't lost to the + // async setState. + const advanceOrSubmit = (sel: string[][], oth: string[]): void => { if (isLast) { - submit() + submitAll(sel, oth) } else { setIndex((i) => Math.min(i + 1, total - 1)) } } - const q = prompt.questions[index]! - const otherSelected = (selections[index] ?? []).includes(OTHER) + // Selecting only highlights the row; submitting is an explicit step via the + // trailing Send/Next button. (Auto-submitting on the first click dismissed the + // card before the user saw any feedback, which read as "nothing happened".) + const pickOption = (label: string): void => { + setSelections((prev) => { + const next = prev.map((s) => [...s]) + const cur = next[index] ?? [] + if (q.multiSelect) { + next[index] = cur.includes(label) ? cur.filter((l) => l !== label) : [...cur, label] + } else { + next[index] = cur.includes(label) ? [] : [label] + } + return next + }) + } + + // Trailing action (also fired by Enter). On any non-final question this just + // advances — "Next" when answered, "Skip" when not — so skipping one question + // never discards answers already given on the others. Only the final question + // submits; an explicit Skip click there with nothing answered anywhere + // dismisses, but a reflexive Enter in the empty field is a no-op so it can't + // throw away the whole prompt. + const confirm = (fromKeyboard = false): void => { + if (!isLast) { + advanceOrSubmit(selections, otherText) + return + } + const anyAnswered = prompt.questions.some((_, i) => answerFor(i).length > 0) + if (anyAnswered) { + submitAll(selections, otherText) + } else if (!fromKeyboard) { + onCancel() + } + } return ( -
-
+ // Part of the composer: docked in the bottom input region, matching the + // composer's width and padding, rendered as the "ask" dialog card directly + // above the text input. Its free-text row is the answer input. +
+
{total > 1 ? ( -
+
{prompt.questions.map((qq, i) => ( +
+ + {/* Scroll only kicks in on long option lists; the sleek scrollbar rides + the card's right edge instead of crowding the choices. */} +
+ {q.options.map((opt, i) => ( toggle(index, opt.label, q.multiSelect)} + onSelect={() => pickOption(opt.label)} /> ))} - toggle(index, OTHER, q.multiSelect)} - /> - {otherSelected ? ( -