fix(pierre): recover failed highlight worker pools

[Pearcekieser]

Problem / Motivation

A Pierre highlight worker failure currently latches disableWorkerPool for the rest of the browser tab. This moves later Shiki work onto the renderer main thread and still does not reliably recover already-mounted imperative Pierre renderers.

Workers that remain alive but stop responding also have no request timeout, so their surfaces can wait forever.

Why it matters

Code-heavy sessions can become unresponsive after one worker failure. The fallback intended to prevent blank code blocks can instead move expensive highlighting onto the UI thread for every later code and diff surface until reload.

Mounted surfaces, collapsed file rows, unsaved editor drafts, editor diagnostics, and file-header interactions also need to remain intact while recovery is in progress.

What changed (motivation → approach → change)

Replaced the one-way worker-health latch with an application-owned, generation-based lifecycle:

• render complete app-owned plain text while the pool is starting, recovering, cooling down, or unavailable
• never pass disableWorkerPool as recovery
• monitor worker requests by request ID and recycle a pool when a request remains unsettled for 30 seconds
• treat error, messageerror, and synchronous postMessage failure as pool failures
• leave request-local protocol errors under Pierre's existing handling rather than recycling a healthy pool
• publish plain text before terminating the old manager, then terminate every worker and pending request
• publish a replacement generation only after initialization succeeds
• ignore events, timers, and initialization completion from retired generations
• retry after 250 ms and 1 second, then enter a 30-second cooldown
• retain the failure budget until a replacement remains healthy for 60 seconds
• make one half-open attempt after cooldown, then remain in app-owned plain text until reload if it also fails
• warn once per unstable recovery episode instead of once per cooldown forever
• preserve complete code, patch, and both file-pair sides during recovery
• respect collapsed/header-disabled options and preserve header actions plus filename click selectors
• preserve in-memory editor drafts, stale-handle safety, pending reveals, and diagnostics across remounts
• remove the obsolete workerHealth.ts latch

During recovery the editor swaps to an app-owned plain textarea rather than invoking Pierre without workers. This keeps typing and save available without moving Shiki onto the renderer thread; the latest draft remains visible and remounts into Pierre when a replacement pool is ready.

Relationship to Pierre work

This remains a focused standalone worker-lifecycle fix:

• perf(chat): reachable archived history and stable phone scrolling #7916 is merged into the base. This rebase preserves its warm-swap height stabilization, bounded archived-file fallback, and worker-free plain file-pairs while layering recoverable highlighted generations on top.
• perf(chat): bound Pierre file-pair rendering #8328 is merged into the base. This rebase preserves its 400-line/128-KiB file-pair budget, oversized complete-content fallback, and live-editor degradation while keeping worker recovery for in-budget highlighted surfaces.
• feat: render PR changes as one Pierre CodeView and unify the chrome #4895 is a conflicting draft that redesigns PR changes around a CodeView surface.
• fix(diffs): render a headerless diff fence as a diff, not plain text #4758 is a draft focused on headerless diff-fence parsing.

Keeping worker recovery separate gives it an independent rollback boundary and avoids coupling a correctness/resilience fix to broader rendering redesigns.

Tests

Added or expanded deterministic coverage for:

• initial readiness and whole-manager/all-worker termination
• worker error, messageerror, synchronous posting failure, and hung-request recovery
• stale events from retired generations
• request-local protocol errors not recycling the pool
• 250 ms / 1 s retry progression, 30-second cooldown, and 60-second stability reset
• warning suppression through retries/cooldown
• complete code, patch, and two-sided file-pair fallbacks
• collapsed rows, disabled headers, header actions, and filename click selectors
• unsaved editor drafts surviving editable plain fallback and generation remount
• typing and save remaining available during startup, recovery, cooldown, and unavailable states without main-thread highlighting
• authoritative external file.contents updates superseding retained drafts
• hidden WarmSwap staging excluded from transcript search occurrence counts
• cold-start and replacement-generation diagnostics
• continued shiki-wasm use and absence of disableWorkerPool

Validation performed:

• full frontend suite: 1,804 test files and 28,465 tests passed before the final focused amendments
• post-rebase affected convergence: 169 tests passed, including the previously failing Markdown search integration
• final worker/editor/lifecycle convergence: 61 tests passed
• workerPoolLifecycle.ts focused coverage: 85.39% statements and 84.48% branches (above the 80% per-file floor)
• TypeScript, full ESLint, i18n, phantom classes, docs, vendor, scrub, CloudFormation, jscpd, production build, and bundle-size gates passed
• backend scoped gate matched clean main with zero branch-only failures (74 branch failures; clean main had the same 74 plus one)
• Electron gate matched clean main exactly after dependency setup: 1,561 passed, 0 failed, 26 downstream cancellations from the pre-existing gateway-stop pending-promise case
• full frontend scoped rerun hit the 20-minute local ceiling without a failure marker; it was not restarted after timeout because the full suite had already passed and all final edits are covered by focused convergence
• local GPT review found one blocking draft-loss path; fixed and exact-SHA verified
• local Opus review findings on cold-start diagnostics and warning churn were fixed; current-head Opus's editor-availability advisory was addressed with an app-owned plain textarea

Manual verification

No live gateway or runtime configuration was modified. Worker failures, stale events, hangs, retry/cooldown boundaries, mounted fallback, file-header interactions, draft retention, and editor remounting were exercised with deterministic worker and manager doubles.

Screenshots / video

Why no screenshot: This changes worker failure/recovery mechanics and reuses the existing plain-text fallback styling; it introduces no intentional layout, component, theme, or copy change.

Related Issues

Fixes #8268

Pattern harvest

Rule candidate: review-prompt
Pattern: A failed worker must degrade to app-owned output and replace its complete lifecycle owner; it must not recover by moving unbounded work onto the UI thread.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (N/A — no public contract or configuration changed)
• No secrets, credentials, or internal references in the diff

[github-actions]

UX Review (Fable 5, fork) — 🟡 CONCERNS

UX-level review of 5e7baf28d21085534dc01d7253606fcbf6d2318a via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: CONCERNS

Recovery keeps content readable, but the new fallback states shipped unseen, degrade silently, and the terminal state gives users no way to know a reload restores highlighting.

Watch

• After the half-open attempt fails (phase: 'unavailable'), every code, diff, and editor surface stays plain text until reload with only a console.warn — a user watching highlighting vanish mid-session gets no in-UI cue that reload fixes it. Low frequency × session-long persistence. Fix: one muted, catalog-keyed inline hint on affected surfaces ("Syntax highlighting paused — reload to restore").
• The editor swap (poolState.phase === 'ready' ? <PierreShell>… : <textarea>) drops focus both ways: a worker failure mid-typing unmounts the focused Pierre editor and keystrokes land nowhere until the user re-clicks; the remount repeats it. Fix: transfer focus (and caret where possible) into the textarea on mount and back on recovery.
• filePairFallbackText stacks both full bodies under bare ---/+++ markers, while the sibling PlainFilePairFallback renders labeled per-side sections with a "simplified view" explainer — two plain representations of the same job; the recovery one reads like one corrupted file. Fix: per-side labels or the same explainer.

Evidence gaps

• Editor plain-textarea fallback (starting/recovering/cooldown/unavailable) — no screenshot; the PR's no-visual-delta claim doesn't hold for this new surface.
• FilePairPlainFallback recovery state, expanded and collapsed: true header-only — no screenshot.
• Patch fallback with the new header-actions row (fallbackHeader) — no screenshot.
• Highlighted → plain → highlighted round-trip (blocks and editor) — a recording would show geometry stability and focus behavior; none is committed or linked.

[UX-REVIEWED] 5e7baf2

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 5e7baf28d21085534dc01d7253606fcbf6d2318a via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound generation-owned lifecycle, but it couples to Pierre's private worker wire protocol and lets one slow request spend the tab-wide failure budget.

Watch

• createMonitoredWorker intercepts worker.postMessage and keys watchdogs on request.id/response.id — a private @pierre/diffs wire-protocol contract the library doesn't guarantee. All tests mock the worker module, so a dependency upgrade that changes the message shape fails either open (hang detection silently gone) or closed (every request "times out" at 30s → recycle → cooldown → 'unavailable', tab-wide plain text until reload). Partial<WorkerResponse> casts give type cover only if the published types move with the runtime.
  Clears when: a test exercises the watchdog against the real @pierre/diffs worker protocol (or the manager’s actual postMessage traffic), or an unrecognized message shape disarms the watchdog instead of recycling.
• The timeout path recycles the whole manager, then remounted surfaces re-request the same content: one legitimately slow highlight (whole-file PierreCodeImpl surfaces have no render budget, unlike file pairs) can burn both retries, the cooldown, and the half-open attempt on itself, ending in permanent 'unavailable' for every other surface. The failure budget is global but the offender is per-request.
  Clears when: timeout-classified failures don’t count toward the terminal latch (or the offending request/content is excluded from re-request), or evidence that in-scope content cannot approach PIERRE_WORKER_REQUEST_TIMEOUT_MS.

Suggestions

• The old workerHealth.ts "one-way on purpose" rationale (no readable per-worker readiness) is correctly answered by whole-manager replacement plus init-gated ready — worth one sentence in workerPoolLifecycle.ts's header so the next reviewer doesn't reopen it.

[DESIGN-REVIEWED] 5e7baf2

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 5e7baf28d21085534dc01d7253606fcbf6d2318a via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence is gathered: the fix has a linked issue (#8268), the deleted "one-way on purpose" pin's stated premise (no readiness signal to read from a live pool) is answered by the new design's initialize()-gated fresh manager, the plain-mode no-worker saving stays pinned, onVisible and the aria-hidden staging already exist in base, and I counted consumers for the overlapping fallback components and the new lifecycle surface. Final review:

First-Principles-Verdict: CONCERNS

FilePairPlainFallback is a second spelling of PlainFilePairFallback, in the same file, with a mirror-image name; and the lifecycle ships zero-consumer test seams.

Not justified as shipped

• Item 6 — duplicate of website/src/pierre/PlainCodeFallback.tsx:56: base PlainFilePairFallback already renders a file pair plainly honoring renderHeaderPrefix/renderHeaderFilenameSuffix/renderHeaderMetadata, collapsed, and disableFileHeader (grep PlainFilePairFallback|FilePairPlainFallback: 1 real consumer, index.tsx:278). Two components now carry the same header/collapse semantics and will diverge; the geometry difference belongs inside the existing one.
• Item 10 — zero consumers: WorkerPoolLifecycle.stop() and the schedule/cancel options are called only by the new tests (grep .stop() under website/src: no Pierre production caller); the sibling test file proves vi.useFakeTimers suffices.
• Item 8 — rides along: real pre-existing harm (base WarmSwap stages under aria-hidden, index.tsx:184), mechanism-level, but declared only in the Tests list.

What this change ships

Inventory (10 items) — 7 justified

Intent: a FIX — one worker failure must stop permanently moving highlighting onto the UI thread and instead recover (#8268).

1. Worker failure now recovers: plain text, then re-highlighted surfaces on a replacement pool — justified
2. A request unanswered for 30 s recycles the whole pool — justified
3. Surfaces show plain text during cold-start pool initialization too — justified
4. Editor becomes an editable plain textarea during recovery, keeping the unsaved draft — justified
5. 250 ms/1 s retries, 30 s cooldown, one half-open attempt, 60 s stability reset, warn-once-per-episode — justified
6. File-pair fallbacks gain both sides plus header/collapse handling via new FilePairPlainFallback — duplicate of website/src/pierre/PlainCodeFallback.tsx:56
7. Patch fallback shows header actions for real diffs; hidden staged impl withholds them until visible — justified
8. Transcript search skips aria-hidden/inert/hidden subtrees — rides along (undeclared outside the Tests list)
9. Diagnostics reapply after a generation remount — justified
10. stop() plus schedule/cancel injection on the lifecycle — zero consumers (tests only)

The deleted workerHealth.ts "one-way on purpose" pin is answered, not recast: its premise (no readiness signal to reuse a pool) is removed by gating a fresh manager on initialize().

Watch

• The 30 s watchdog rests on an unverified vendored-tool premise: that WorkerPoolManager posts every request through the patched instance postMessage with a string id. If wrong, the timeout silently never arms (degrades to base behavior, not an outage). Clears when: confirmed against @pierre/diffs' worker manager source or an integration test driving the real manager.

Subtractions

• Delete WorkerPoolLifecycle.stop() and the schedule/cancel options (0 production consumers); the sibling test already drives the same timers with vi.useFakeTimers.
• Fold the recovery fallback into PlainFilePairFallback (a geometry-matching variant) instead of shipping the near-namesake FilePairPlainFallback; delete filePairFallbackText with it.

[FIRST-PRINCIPLES-REVIEWED] 5e7baf2

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 5e7baf28d21085534dc01d7253606fcbf6d2318a via the fork AI-review pipeline; updated in place on each push.

Review details

The single candidate rests on the behavior of @pierre/diffs' worker protocol — whether Pierre ever posts an id-tagged message that is never answered with a same-id reply. That library is not vendored or installed in this repo (no node_modules/@pierre/diffs, no local type source), so I cannot open the code that would establish input (a). The in-repo evidence points the other way: the wrapper's own contract comment states "Pierre assigns at most one active request to a worker," the id-keyed timer is disarmed by any same-id response, and the 30s recycle is a deliberately-chosen constant (PIERRE_WORKER_REQUEST_TIMEOUT_MS) whose documented purpose is exactly to catch a hung request. Establishing the defect would require assuming external code I did not open — the candidate's own confidence line concedes this — so it drops under Step 1. No other hunk (draft-preservation, marker reapplication across generations, plain/recovery fallbacks, the acceptNode filter) yields a grounded defect at the 80+ bar.

No findings.

[OPUS-REVIEWED] 5e7baf2

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 5e7baf28d21085534dc01d7253606fcbf6d2318a via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 5e7baf2

[Pearcekieser]

• fixed span=edfaa83ae6de — PlainCodeFallback now renders a file-pair header only when disableFileHeader === false, so omitted options inherit Pierre's shared header-disabled default. Commit fd2e610ff227a16a43bf6760c29bb7b2bafd9240 adds a recovery-state regression covering omitted options.

options?.disableFileHeader treats omission as header-enabled, so lazy DiffPanel surfaces briefly show a header despite the default disabling it.

[Pearcekieser]

• fixed span=9d107e017ad9 — the editor recovery fallback now owns an h-full overflow-auto container, so long drafts remain vertically reachable while workers recover. Commit 5db6879fc25199e89e55ffde6fc4854c417784ef adds a recovery-state scroller regression.

Long files are vertically clipped during recovery because PlainCodeFallback sits inside the editor's overflow-hidden wrapper without a vertical scroller.

[Pearcekieser]

• fixed span=e807fad8ae39 — startup, recovery, cooldown, and unavailable states now render an app-owned plain textarea instead of a read-only <pre>. Typing and save remain available without moving Shiki onto the renderer thread, and edits remount into Pierre when a ready generation returns. Commit cc6881d0cdda56b9e0cf8dc6f8a3ff5437efbeed adds an exact recovery-edit/remount regression.

The editor surface renders read-only PlainCodeFallback whenever the pool is not ready, preventing typing and save on cold start and after retries exhaust.

[Pearcekieser]

• fixed span=870a47a9b540 — the controlled recovery textarea now stores its draft in React state, updates that state for both Pierre and textarea changes, and reconciles real file.contents prop changes as authoritative. Commit 976df96f3421863f7c0b45df54342b5200d0018e strengthens the regression with consecutive edits from a ref-backed caller and verifies the final draft remounts into Pierre.

Recovery textarea discards edits from ref-backed callers because its controlled value is backed only by refs that do not trigger a render.

[bolichen97]

Rebased onto main 6fbb06bc by a maintainer as part of the 2026-09-08 open-PR audit. New head: 90d6ad34 (was 97477117).

Clean rebase, no conflicts, no content changes: the diff against main is still 12 files, +1112/-378, identical to the pre-rebase diff.

Gates run locally in an isolated worktree:

• npx tsc --noEmit -p website/tsconfig.json - pass
• npx vitest run on the four touched test files (PierreEditorImpl, PierreImpl.workerPool, domHighlight, pierre.workerHealth) - 85 tests, all pass

Please sanity-check the rebased branch. Note that a maintainer push makes the maintainer the last pusher, so under this repo's last-push rule a second approver is needed. Reply here if anything looks wrong.

[bolichen97]

Requesting changes at 5e7baf2 — this is shipped as a "focused standalone worker-lifecycle fix" under a <!-- no-visual-delta --> waiver, but it changes rendering on every code surface and carries an unrelated behavior change.

1. Rendering changes on all surfaces. PierreImpl.tsx (PierreCodeImpl / PatchImpl / FilePairImpl) now render PlainCodeFallback until phase === 'ready', so every code/patch/file-pair surface has a new cold-start plain-text phase until 4 workers + WASM initialize, where main mounted Pierre immediately. PierreEditorImpl.tsx ~L266-275 turns the editor into a bare <textarea> (no gutter, line numbers, diff, markers) during starting/recovering/cooldown/unavailable. PlainCodeFallback.tsx ~L27 adds a new 36 px data-diffs-header row to fallbacks, and PierrePatch now renders header metadata inside the fallback while suppressing it on the impl until implVisible. Those are rendered deltas; the waiver is inaccurate. Per the repo rule this is a UX change and needs a maintainer decision, not a bug-fix approval.
2. Smuggled behavior change. website/src/utils/domHighlight.ts ~L20-27 makes transcript search skip every [aria-hidden="true"], [inert], [hidden] subtree. That changes search occurrence counts app-wide, is absent from "What changed", and surfaces only under "Tests" as the fix for the previously failing Markdown search integration. Split it out or disclose and justify it.
3. WarmSwap regression vs the body's claim. The in-impl fallback's non-empty <pre> satisfies scrollHeight > WARM_PAINT_MIN_PX, so painted flips true, warmSwapHeights records the FALLBACK height, and the later fallback→Pierre swap happens outside the height freeze — the opposite of "preserves #7916's warm-swap height stabilization".
4. Uncontrolled editor callers regress render inputs. setDraftContents on every keystroke feeds <File edit> a new file / cacheKey per keystroke when the parent does not echo contents (EditableCodeBlock.tsx ~L78-79 keeps only valueRef); Pierre's useFileInstance then calls instance.render({file}) with newContent=true inside an active edit session. Main kept file stable here; the tests only exercise mocked surfaces.

Smaller: publish-before-terminate ordering relies on React's sync-lane microtask beating queueMicrotask(() => handle?.terminate()) (workerPoolLifecycle.ts ~L116); FilePairPlainFallback duplicates the existing PlainFilePairFallback and drops its i18n label/split handling; pierre.workerHealth.test.tsx keeps its name after workerHealth.ts is deleted; in any Worker-less environment every surface is now permanently plain text where main used Pierre on the main thread.