fix: harden and de-slop the merged sidebar/chat/settings batch

Post-merge follow-ups for #2740 #2735 #2734 #2690 #2676 #2738 #2684 #2689 #2733 #2739 #2462 #2687 #2736 #2618 #2697, plus three regressions found while reviewing them: - ctrl/cmd+digit while typing no longer switches session tabs (#2503 was still open in practice: the guard only covered the mod+alt surface binding) - Shiki template-call sanitizer now covers every bundled grammar, including the js/ts aliases and embedding grammars; timed-out highlight requests are memoized and no longer cancel unrelated in-flight requests - settings flush on suspend uses keepalive and also fires on Capacitor appStateChange; keeps the selected model persisted across mode switches - remote-only branches fetch before checkout; range helpers fail clearly - git status invalidation now fires for runtime adapters too - settings number inputs and select triggers size in ch so they scale with the interface font - recent-activity timestamps tick from one list-level ticker - Markdown preview find goes through the shared find_in_file keybind with containment, no longer counts its own bar, and debounces observer runs - #2676 reverted; #2524 fixed by fading the sticky header's own background instead of overlaying the content below it - sticky group headers in the model picker and sidebar render again (oc-sticky-fade-scroller class restored after 9b9d7069c) - project switcher names are left-aligned again (wrapper lost in 26dbc2f30) - tool card quick-open icon is always visible and opens the same line as the expanded card's button - tautological tests replaced or removed; new oxlint findings fixed
2026-08-29 01:06:43 +03:00
parent a182f4f4ff
commit 48bcac1758
53 changed files with 1125 additions and 375 deletions
@@ -9,6 +9,20 @@ interface TurnItemProps {
    renderMessage: (message: ChatMessageEntry) => React.ReactNode;
 }

+/**
+ * The sticky user header paints the chat background so assistant content scrolling
+ * underneath disappears behind it. The soft edge lives in the header's own background
+ * instead of an overlay below it: the bottom 0.75rem of the header box fades the
+ * background out, and that strip sits over the empty space the user bubble already
+ * reserves below itself. At rest the strip reveals the identical page background
+ * (`--background` is generated from the same `surface.background` token), so it is
+ * invisible and can never wash over the assistant content that follows.
+ */
+const STICKY_HEADER_BACKGROUND: React.CSSProperties = {
+    backgroundImage:
+        'linear-gradient(to bottom, var(--surface-background) calc(100% - 0.75rem), transparent)',
+};
+
 const TurnItem: React.FC<TurnItemProps> = ({ turn, stickyUserHeader = true, renderMessage }) => {
    return (
        <section
@@ -18,14 +32,13 @@ const TurnItem: React.FC<TurnItemProps> = ({ turn, stickyUserHeader = true, rend
            data-scroll-spy-id={turn.turnId}
        >
            {stickyUserHeader ? (
-                <div className="sticky top-0 z-20 relative bg-[var(--surface-background)] pb-4 sm:pb-8 [overflow-anchor:none]">
+                <div
+                    className="sticky top-0 z-20 [overflow-anchor:none]"
+                    style={STICKY_HEADER_BACKGROUND}
+                >
                    <div className="relative z-10">
                        {renderMessage(turn.userMessage)}
                    </div>
-                    <div
-                        aria-hidden="true"
-                        className="pointer-events-none absolute inset-x-0 bottom-0 z-0 h-4 bg-gradient-to-b from-[var(--surface-background)] to-transparent sm:h-8"
-                    />
                </div>
            ) : (
                renderMessage(turn.userMessage)
@@ -1,10 +1,7 @@
 /// <reference lib="webworker" />

 import { bundledLanguages, createHighlighter, type BundledLanguage, type LanguageRegistration, type ThemedToken } from 'shiki';
-import {
-  isTemplateCallLanguageId,
-  sanitizeTemplateCallGrammar,
-} from '../../../lib/shiki/sanitizeTemplateCallGrammar';
+import { sanitizeTemplateCallGrammar } from '../../../lib/shiki/sanitizeTemplateCallGrammar';
 import { MARKDOWN_SHIKI_THEME, MARKDOWN_SHIKI_THEME_DEFINITION } from './markdownShikiThemeDefinition';
 import type { MarkdownWorkerRequest, MarkdownWorkerResponse } from './markdown-worker-protocol';

@@ -69,15 +66,17 @@ type BundledLanguageModule = { default: LanguageRegistration[] };
 /**
 * Load a language, neutralizing the catastrophic JS/TS `template-call` rule
 * before it reaches the Oniguruma scanner (see sanitizeTemplateCallGrammar).
+ *
+ * Every bundled language is resolved and sanitized rather than a fixed id
+ * list: Shiki keys the JS/TS grammars under aliases too (`js`, `ts`, `cjs`,
+ * `mjs`, `mts`, `cts`), and embedding grammars (`vue`, `svelte`, `mdx`,
+ * `astro`, `html`) ship them as extra entries in their own module. Sanitizing
+ * every entry is free for the rest — `hasCatastrophicTemplateCall` returns the
+ * grammar untouched when the rule is absent.
 */
 const loadLanguageSafe = async (instance: Instance, lang: BundledLanguage): Promise<void> => {
-  if (!isTemplateCallLanguageId(lang)) {
-    await instance.loadLanguage(bundledLanguages[lang]);
-    return;
-  }
-
  // SAFETY: every Shiki bundled-language module default-exports its grammar
-  // array; `lang` is narrowed to a bundled id above.
+  // array; `lang` is narrowed to a bundled id by the caller.
  const mod = (await bundledLanguages[lang]()) as BundledLanguageModule;
  const grammars = mod.default.map((grammar) => sanitizeTemplateCallGrammar(grammar));
  await instance.loadLanguage(...grammars);
@@ -1,12 +1,137 @@
-import { describe, expect, test } from 'bun:test';
+import { describe, expect, mock, test } from 'bun:test';

-import { HIGHLIGHT_REQUEST_TIMEOUT_MS } from './markdown-worker-timeout';
+import type { MarkdownWorkerRequest } from './markdown-worker-protocol';
+
+/**
+ * Hang safety for the markdown Shiki worker client
+ * (openchamber/openchamber#2587, follow-up on #2618).
+ *
+ * Catastrophic Oniguruma backtracking is synchronous inside the worker, so the
+ * only recovery is terminating it from this thread. Two properties matter and
+ * neither is observable from the timeout constant alone: the hung request must
+ * resolve `null` after the worker is terminated, and the block that caused it
+ * must not be retried — a retry respawns a worker (Shiki + Oniguruma init) and
+ * burns another full budget of a core on every render and every scroll past it.
+ */
+
+const TEST_TIMEOUT_MS = 50;
+
+mock.module('./markdown-shiki.worker.ts?worker&url', () => ({ default: 'blob:test-shiki-worker' }));
+mock.module('./markdown-worker-timeout', () => ({ HIGHLIGHT_REQUEST_TIMEOUT_MS: TEST_TIMEOUT_MS }));
+
+/** A worker that accepts everything and answers nothing. */
+class SilentWorker {
+  static created = 0;
+  static terminated = 0;
+  static messages: MarkdownWorkerRequest[] = [];
+
+  onmessage: ((event: MessageEvent) => void) | null = null;
+  onerror: (() => void) | null = null;
+  onmessageerror: (() => void) | null = null;
+
+  constructor() {
+    SilentWorker.created += 1;
+  }
+
+  postMessage(message: MarkdownWorkerRequest): void {
+    SilentWorker.messages.push(message);
+  }
+
+  terminate(): void {
+    SilentWorker.terminated += 1;
+  }
+}
+
+/**
+ * bun test has no `window` or `Worker`; defining the properties directly
+ * installs the stubs without asserting they are the platform globals.
+ * `SilentWorker` implements exactly the members `markdown-worker` uses:
+ * postMessage, terminate, and the three handler slots.
+ */
+const installWorkerStub = (): void => {
+  Object.defineProperty(globalThis, 'window', { value: {}, configurable: true, writable: true });
+  Object.defineProperty(globalThis, 'Worker', { value: SilentWorker, configurable: true, writable: true });
+};
+
+/**
+ * Silent on the first instance, answering on every later one — so a request
+ * that was only queued behind the hung one can be observed being replayed
+ * against the replacement worker.
+ */
+class ReplayWorker {
+  static created = 0;
+  static terminated = 0;
+
+  onmessage: ((event: MessageEvent) => void) | null = null;
+  onerror: (() => void) | null = null;
+  onmessageerror: (() => void) | null = null;
+
+  private readonly answers: boolean;
+
+  constructor() {
+    ReplayWorker.created += 1;
+    this.answers = ReplayWorker.created > 1;
+  }
+
+  postMessage(message: MarkdownWorkerRequest): void {
+    if (!this.answers || message.type !== 'highlight') return;
+    setTimeout(() => {
+      this.onmessage?.(new MessageEvent('message', {
+        data: { type: 'highlight', id: message.id, html: '<pre>ok</pre>' },
+      }));
+    }, 0);
+  }
+
+  terminate(): void {
+    ReplayWorker.terminated += 1;
+  }
+}
+
+/** Same reasoning as installWorkerStub. */
+const installReplayWorkerStub = (): void => {
+  Object.defineProperty(globalThis, 'window', { value: {}, configurable: true, writable: true });
+  Object.defineProperty(globalThis, 'Worker', { value: ReplayWorker, configurable: true, writable: true });
+};

 describe('markdown-worker hang safety', () => {
-  test('exposes a finite highlight timeout budget', () => {
-    // Catastrophic Oniguruma backtracking must not run unbounded; the main
-    // thread terminates the worker after this budget (openchamber/openchamber#2587).
-    expect(HIGHLIGHT_REQUEST_TIMEOUT_MS).toBeGreaterThan(0);
-    expect(HIGHLIGHT_REQUEST_TIMEOUT_MS).toBeLessThan(15_001);
+  test('a hung block resolves null, terminates the worker, and is not retried', async () => {
+    installWorkerStub();
+    const { highlightCodeInWorker, resetMarkdownWorkerClientCacheForTests } = await import('./markdown-worker');
+    resetMarkdownWorkerClientCacheForTests();
+
+    const code = 'const label = `Account ${index + 1}`;';
+
+    const first = await highlightCodeInWorker(code, 'javascript');
+    expect(first).toBeNull();
+    expect(SilentWorker.terminated).toBe(1);
+
+    const createdAfterFirst = SilentWorker.created;
+    const messagesAfterFirst = SilentWorker.messages.length;
+
+    // Same content again: the timed-out key is memoized as failed, so nothing
+    // reaches a worker and none is spawned.
+    const second = await highlightCodeInWorker(code, 'javascript');
+    expect(second).toBeNull();
+    expect(SilentWorker.created).toBe(createdAfterFirst);
+    expect(SilentWorker.messages.length).toBe(messagesAfterFirst);
+  });
+
+  test('a timeout fails only the offending request and replays the queued one', async () => {
+    installReplayWorkerStub();
+    const { highlightCodeInWorker, resetMarkdownWorkerClientCacheForTests } = await import('./markdown-worker');
+    resetMarkdownWorkerClientCacheForTests();
+
+    const results = await Promise.all([
+      highlightCodeInWorker('const a = `one`;', 'javascript'),
+      highlightCodeInWorker('const b = `two`;', 'javascript'),
+    ]);
+
+    // Whichever request owns the first timer is the offender; the other was
+    // merely queued behind it and must survive on the replacement worker
+    // rather than being cancelled with it.
+    expect(results.filter((value) => value === null)).toHaveLength(1);
+    expect(results.filter((value) => value === '<pre>ok</pre>')).toHaveLength(1);
+    expect(ReplayWorker.terminated).toBe(1);
+    expect(ReplayWorker.created).toBe(2);
  });
 });
@@ -19,8 +19,14 @@ import { HIGHLIGHT_REQUEST_TIMEOUT_MS } from './markdown-worker-timeout';
 // The per-request timeout exists because TextMate grammars can enter catastrophic
 // backtracking on the Oniguruma WASM engine (openchamber/openchamber#2587).
 // Matching is synchronous inside the worker, so the only way to reclaim its heap
-// is to terminate it from this thread once a request exceeds the budget. A timed
-// out request resolves `null` like any other failure, so nothing is memoized.
+// is to terminate it from this thread once a request exceeds the budget.
+//
+// A timeout is scoped to the block that caused it: only that request resolves
+// `null`, and the requests that were merely queued behind it are re-dispatched
+// against the fresh worker. The timed-out key is memoized as failed, because a
+// block that hangs the grammar hangs it every time — without that, every
+// re-render and every scroll past the block would pay another worker spawn
+// (Shiki + Oniguruma init) plus the full timeout budget of a core.
 //
 // Results are memoized by content fingerprint (+ lang / theme). Unchanged
 // content must not re-enter the worker — that was the sustained ~40 msg/s
@@ -37,17 +43,30 @@ import { HIGHLIGHT_REQUEST_TIMEOUT_MS } from './markdown-worker-timeout';
 // repaints via CSS and must not invalidate these entries. Only
 // `highlightTokens` resolves concrete colors, so only its key carries a theme.

-type PendingResolver = (response: MarkdownWorkerResponse | null) => void;
+/**
+ * Why a request stopped, kept distinct so a hang can be memoized while a
+ * transient "no worker yet" failure is retried on the next render.
+ */
+type RequestOutcome =
+  | { status: 'ok'; response: MarkdownWorkerResponse }
+  | { status: 'failed' }
+  | { status: 'timeout' };
+
+type PendingResolver = (outcome: RequestOutcome) => void;

 type PendingEntry = {
  resolve: PendingResolver;
  timer: ReturnType<typeof setTimeout>;
+  payload: MarkdownWorkerRequest;
 };

 type CachedHighlight =
  | { type: 'highlight'; html: string }
  | { type: 'highlightLines'; lines: string[] }
-  | { type: 'highlightTokens'; lines: MarkdownTokenRun[][] };
+  | { type: 'highlightTokens'; lines: MarkdownTokenRun[][] }
+  // A block that timed out the worker. Memoized so it is attempted once per
+  // session instead of respawning a worker on every render.
+  | { type: 'failed' };

 const CLIENT_CACHE_MAX_ENTRIES = 2000;
 const CLIENT_CACHE_MAX_BYTES = 24 * 1024 * 1024;
@@ -74,6 +93,7 @@ const clearPendingTimers = (): void => {

 const entryBytes = (key: string, value: CachedHighlight): number => {
  const keyBytes = utf16Bytes(key);
+  if (value.type === 'failed') return keyBytes;
  if (value.type === 'highlight') return keyBytes + utf16Bytes(value.html);
  if (value.type === 'highlightLines') {
    let total = keyBytes;
@@ -83,13 +103,8 @@ const entryBytes = (key: string, value: CachedHighlight): number => {
  return keyBytes + estimateTokenRunsBytes(value.lines);
 };

-const failAll = (): void => {
-  clearPendingTimers();
-  pending.forEach((entry) => entry.resolve(null));
-  pending.clear();
+const disposeWorker = (): void => {
  sentThemes.clear();
-  // Drop in-flight waiters; cached results remain valid (pure fn of inputs).
-  inflight.clear();
  worker?.terminate();
  worker = undefined;
  workerCreation = undefined;
@@ -99,6 +114,16 @@ const failAll = (): void => {
  }
 };

+/** Worker crash / message error: nothing in flight can still be answered. */
+const failAll = (): void => {
+  clearPendingTimers();
+  pending.forEach((entry) => entry.resolve({ status: 'failed' }));
+  pending.clear();
+  // Drop in-flight waiters; cached results remain valid (pure fn of inputs).
+  inflight.clear();
+  disposeWorker();
+};
+
 const createWorker = async (): Promise<Worker | undefined> => {
  if (typeof window === 'undefined' || typeof Worker === 'undefined') return undefined;
  try {
@@ -117,7 +142,7 @@ const createWorker = async (): Promise<Worker | undefined> => {
      if (!entry) return;
      clearTimeout(entry.timer);
      pending.delete(event.data.id);
-      entry.resolve(event.data);
+      entry.resolve({ status: 'ok', response: event.data });
    };
    instance.onerror = failAll;
    instance.onmessageerror = failAll;
@@ -141,20 +166,56 @@ const getWorker = async (): Promise<Worker | undefined> => {
  return workerCreation;
 };

-const request = async (payload: (id: number) => MarkdownWorkerRequest): Promise<MarkdownWorkerResponse | null> => {
+/**
+ * Post one request and arm its timeout. The timer starts here, not when the
+ * caller enqueued, so a request re-dispatched after someone else's hang gets a
+ * whole budget on the fresh worker rather than an already-spent one.
+ */
+const dispatch = async (id: number, resolve: PendingResolver, payload: MarkdownWorkerRequest): Promise<void> => {
  const instance = await getWorker();
-  if (!instance) return Promise.resolve(null);
+  if (!instance) {
+    resolve({ status: 'failed' });
+    return;
+  }
+  const timer = setTimeout(() => handleTimeout(id), HIGHLIGHT_REQUEST_TIMEOUT_MS);
+  pending.set(id, { resolve, timer, payload });
+  instance.postMessage(payload);
+};
+
+/**
+ * One request exceeded the budget. Kill the worker so the WASM heap is freed
+ * instead of growing until the renderer OOMs, fail only the offending request,
+ * and replay the requests that were only waiting behind it.
+ */
+function handleTimeout(id: number): void {
+  const offender = pending.get(id);
+  if (!offender) return;
+  console.warn(`Shiki worker highlight timed out after ${HIGHLIGHT_REQUEST_TIMEOUT_MS}ms; terminating worker`);
+
+  const survivors = Array.from(pending.entries()).filter(([pendingId]) => pendingId !== id);
+  clearPendingTimers();
+  pending.clear();
+  disposeWorker();
+
+  offender.resolve({ status: 'timeout' });
+
+  for (const [survivorId, entry] of survivors) {
+    // A `highlightTokens` payload whose theme was already shipped to the dead
+    // worker cannot be replayed — the definition went with it, and the fresh
+    // worker would reject the bare theme name.
+    if (entry.payload.type === 'highlightTokens' && entry.payload.theme === undefined) {
+      entry.resolve({ status: 'failed' });
+      continue;
+    }
+    void dispatch(survivorId, entry.resolve, entry.payload);
+  }
+}
+
+const request = async (payload: (id: number) => MarkdownWorkerRequest): Promise<RequestOutcome> => {
  const id = ++nextId;
-  return new Promise<MarkdownWorkerResponse | null>((resolve) => {
-    const timer = setTimeout(() => {
-      if (!pending.has(id)) return;
-      // Hung tokenize (e.g. catastrophic backtracking): kill the worker so the
-      // WASM heap is freed instead of growing until the renderer OOMs.
-      console.warn(`Shiki worker highlight timed out after ${HIGHLIGHT_REQUEST_TIMEOUT_MS}ms; terminating worker`);
-      failAll();
-    }, HIGHLIGHT_REQUEST_TIMEOUT_MS);
-    pending.set(id, { resolve, timer });
-    instance.postMessage(payload(id));
+  const message = payload(id);
+  return new Promise<RequestOutcome>((resolve) => {
+    void dispatch(id, resolve, message);
  });
 };

@@ -171,6 +232,12 @@ const coalesce = (
  return pendingRequest;
 };

+const memoizeFailure = (key: string): CachedHighlight => {
+  const entry: CachedHighlight = { type: 'failed' };
+  resultCache.set(key, entry, entryBytes(key, entry));
+  return entry;
+};
+
 const cacheKeyFor = (kind: string, lang: string, code: string, themeName?: string): string => {
  const fp = contentFingerprint(code);
  return themeName === undefined ? `${kind}:${lang}:${fp}` : `${kind}:${themeName}:${lang}:${fp}`;
@@ -190,11 +257,13 @@ export const highlightCodeInWorker = async (code: string, lang: string): Promise
  const key = cacheKeyFor('highlight', lang, code);
  const cached = resultCache.get(key);
  if (cached?.type === 'highlight') return cached.html;
+  if (cached?.type === 'failed') return null;

  const result = await coalesce(key, async () => {
-    const response = await request((id) => ({ type: 'highlight', id, code, lang }));
-    if (response?.type !== 'highlight') return null;
-    const entry: CachedHighlight = { type: 'highlight', html: response.html };
+    const outcome = await request((id) => ({ type: 'highlight', id, code, lang }));
+    if (outcome.status === 'timeout') return memoizeFailure(key);
+    if (outcome.status !== 'ok' || outcome.response.type !== 'highlight') return null;
+    const entry: CachedHighlight = { type: 'highlight', html: outcome.response.html };
    resultCache.set(key, entry, entryBytes(key, entry));
    return entry;
  });
@@ -210,11 +279,13 @@ export const highlightLinesInWorker = async (code: string, lang: string): Promis
  const key = cacheKeyFor('highlightLines', lang, code);
  const cached = resultCache.get(key);
  if (cached?.type === 'highlightLines') return cached.lines;
+  if (cached?.type === 'failed') return null;

  const result = await coalesce(key, async () => {
-    const response = await request((id) => ({ type: 'highlightLines', id, code, lang }));
-    if (response?.type !== 'highlightLines') return null;
-    const entry: CachedHighlight = { type: 'highlightLines', lines: response.lines };
+    const outcome = await request((id) => ({ type: 'highlightLines', id, code, lang }));
+    if (outcome.status === 'timeout') return memoizeFailure(key);
+    if (outcome.status !== 'ok' || outcome.response.type !== 'highlightLines') return null;
+    const entry: CachedHighlight = { type: 'highlightLines', lines: outcome.response.lines };
    resultCache.set(key, entry, entryBytes(key, entry));
    return entry;
  });
@@ -242,10 +313,11 @@ export const highlightTokensInWorker = async (
  const key = cacheKeyFor('highlightTokens', lang, code, themeName);
  const cached = resultCache.get(key);
  if (cached?.type === 'highlightTokens') return cached.lines;
+  if (cached?.type === 'failed') return null;

  const result = await coalesce(key, async () => {
    const needsTheme = !sentThemes.has(themeName);
-    const response = await request((id) => ({
+    const outcome = await request((id) => ({
      type: 'highlightTokens',
      id,
      code,
@@ -253,9 +325,10 @@ export const highlightTokensInWorker = async (
      themeName,
      ...(needsTheme ? { theme } : {}),
    }));
-    if (response?.type !== 'highlightTokens') return null;
+    if (outcome.status === 'timeout') return memoizeFailure(key);
+    if (outcome.status !== 'ok' || outcome.response.type !== 'highlightTokens') return null;
    sentThemes.add(themeName);
-    const entry: CachedHighlight = { type: 'highlightTokens', lines: response.lines };
+    const entry: CachedHighlight = { type: 'highlightTokens', lines: outcome.response.lines };
    resultCache.set(key, entry, entryBytes(key, entry));
    return entry;
  });
@@ -202,11 +202,15 @@ export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerR
  }, []);

  const getClampedX = React.useCallback((anchorX: number) => (
-    getDesktopClampedX(anchorX, window.innerWidth, menuWidthRef.current)
+    typeof window === 'undefined'
+      ? anchorX
+      : getDesktopClampedX(anchorX, window.innerWidth, menuWidthRef.current)
  ), []);

  const getClampedY = React.useCallback((anchorY: number) => (
-    getDesktopClampedY(anchorY, window.innerHeight, menuHeightRef.current)
+    typeof window === 'undefined'
+      ? anchorY
+      : getDesktopClampedY(anchorY, window.innerHeight, menuHeightRef.current)
  ), []);

  const addMarkdownToChat = React.useCallback((markdownText: string) => {
@@ -287,7 +291,10 @@ export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerR
      x: getClampedX(prev.x),
      y: getClampedY(prev.y),
    }));
-  }, [getClampedX, getClampedY, isMobile, position.show]);
+    // Entering comment mode and typing into the comment box both grow the
+    // popup, so remeasuring on those keeps the cached height (and the Y clamp
+    // built from it) honest.
+  }, [commentMode, commentText, getClampedX, getClampedY, isMobile, position.show]);

  // The desktop popup hangs above its anchor, so a tall comment box near the
  // top of the chat can climb over the app header. On the desktop shell the
@@ -87,6 +87,7 @@ Use this doc when you ask an agent to change tool/header/description behavior.
 - The managed `openchamber` plugin tool uses the expandable path and hides its broad protocol input. The plugin supplies the selected action's human description as the native tool title; the UI renders that metadata without owning an action map. The full versioned result envelope renders through the same neutral JSON summary/tree/raw views as other tools, without a tool-specific output card.
 - `ToolPart` defers expanded content after a user toggle, preventing large tool input/output payloads from mounting during the initial chat render.
 - The rich tool diff preview lives in `ToolPartDiffPreview.tsx` and is lazy-loaded from `ToolPart`. It is the only tool-card piece that imports the `@pierre/diffs` + Shiki rendering stack, keeping that stack out of the eager chat startup graph. While its chunk loads (first rendered diff only) the plain-text patch from `PlainDiffFallback.tsx` renders as the Suspense fallback, mirroring the preview's error fallback. `ToolPart` itself must not statically import `@pierre/diffs` runtime modules or `@/lib/shiki/appThemeRegistry`.
+- The `@pierre/diffs` stack is knowingly unprotected against the JS/TS `template-call` backtracking that OOM'd the renderer in openchamber/openchamber#2587. Our own markdown Shiki worker sanitizes every grammar it loads (`@/lib/shiki/sanitizeTemplateCallGrammar`), but the diff worker pool runs `preferredHighlighter: 'shiki-wasm'` (`DiffWorkerProvider.tsx`) and resolves its languages by id through `@pierre/diffs`' own registry — `langs` accepts `SupportedLanguages` strings only, so there is no seam to hand it a pre-sanitized `LanguageRegistration`. A pathological template literal inside a rendered diff can therefore still hang that pool's Oniguruma engine. The available levers are upstream (a `langs` overload accepting grammar objects) or switching that pool to the JS regex engine; neither is done.
 - Running bash output falls back to `state.metadata.output` until canonical `state.output` arrives. Its output viewport grows with the content up to `46vh`, then scrolls and follows new output until the user scrolls up; following resumes when the user returns to the bottom. Live output appends or replaces rewritten snapshots as plain text without worker highlighting; finalized output normalizes ANSI terminal controls with a bounded synthetic-cell budget, bypasses the throttle, and receives the normal one-time highlighted rendering.
 - Thinking/Justification duration is hidden in `sorted` mode (handled in `ReasoningPart.tsx` + `JustificationBlock.tsx`).
 - Reasoning streaming presentation derives from the live stream phase (`streaming`/`cooldown`), never from missing persisted timing: a cached part without `time.end` is not live, and a part whose `time.end` is set never streams (issue #2020).
@@ -1,12 +1,67 @@
-import React from 'react';
+import React, { act } from 'react';
 import { describe, expect, test } from 'bun:test';
 import { renderToStaticMarkup } from 'react-dom/server';
+import { createRoot } from 'react-dom/client';
+import { Window } from 'happy-dom';
 import type { Part } from '@opencode-ai/sdk/v2';

 import { I18nProvider } from '@/lib/i18n';
 import ReasoningPart, { ReasoningTimelineBlock } from './ReasoningPart';
 import type { StreamPhase } from '../types';

+type ReasoningPartFixture = Extract<Part, { type: 'reasoning' }>;
+
+/**
+ * Mounts a real client root against a happy-dom document so mount/unmount
+ * lifecycle is observable. bun test shares globalThis across a file, so the
+ * globals React DOM reads are defined here and restored afterwards; defining
+ * them directly avoids asserting that happy-dom's objects are the platform
+ * `Window`/`Document`.
+ */
+const DOM_GLOBAL_NAMES = [
+  'window',
+  'document',
+  'navigator',
+  'Node',
+  'Element',
+  'HTMLElement',
+  'IS_REACT_ACT_ENVIRONMENT',
+] as const;
+
+const installDomStub = () => {
+  const happyWindow = new Window({ url: 'http://localhost' });
+  const previous = DOM_GLOBAL_NAMES.map(
+    (name) => [name, Object.getOwnPropertyDescriptor(globalThis, name)] as const,
+  );
+  const values = {
+    window: happyWindow,
+    document: happyWindow.document,
+    navigator: happyWindow.navigator,
+    Node: happyWindow.Node,
+    Element: happyWindow.Element,
+    HTMLElement: happyWindow.HTMLElement,
+    IS_REACT_ACT_ENVIRONMENT: true,
+  };
+  for (const name of DOM_GLOBAL_NAMES) {
+    Object.defineProperty(globalThis, name, { value: values[name], configurable: true, writable: true });
+  }
+
+  // Read back through the global bindings just installed, so the container is
+  // typed as the DOM element React expects rather than happy-dom's own class.
+  const container = document.createElement('div');
+  document.body.appendChild(container);
+
+  return {
+    container,
+    restore: () => {
+      for (const [name, descriptor] of previous) {
+        if (descriptor) Object.defineProperty(globalThis, name, descriptor);
+        else Reflect.deleteProperty(globalThis, name);
+      }
+    },
+  };
+};
+
 // A reasoning text whose summary (first 120 chars) fits in the header but
 // whose expanded body content should only appear when the disclosure is open.
 const LONG_REASONING =
@@ -126,20 +181,22 @@ describe('ReasoningPart streaming gating (issue #2020)', () => {

  const BUSY_INDICATOR = 'animate-busy-pulse';

-  const makeReasoningPart = (time?: { start?: number; end?: number }): Part =>
-    ({
-      id: 'prt_reasoning_2020',
-      sessionID: 'ses_2020',
-      messageID: 'msg_2020',
-      type: 'reasoning',
-      text: SHORT_REASONING,
-      time,
-    }) as unknown as Part;
+  const makeReasoningPart = (
+    time: ReasoningPartFixture['time'],
+    text: string = SHORT_REASONING,
+  ): ReasoningPartFixture => ({
+    id: 'prt_reasoning_2020',
+    sessionID: 'ses_2020',
+    messageID: 'msg_2020',
+    type: 'reasoning',
+    text,
+    time,
+  });

  // Server rendering reads the UI store's initial state, which is
  // chatRenderMode 'live' — the mode in which the streaming presentation is
  // reachable and the issue reproduces.
-  const renderPart = (part: Part, streamPhase?: StreamPhase): string =>
+  const renderPart = (part: ReasoningPartFixture, streamPhase?: StreamPhase): string =>
    renderToStaticMarkup(
      <I18nProvider>
        <ReasoningPart part={part} messageId="msg_2020" streamPhase={streamPhase} />
@@ -182,13 +239,60 @@ describe('ReasoningPart streaming gating (issue #2020)', () => {
    expect(markup).toContain('aria-expanded="true"');
  });

-  test('remounting a completed reasoning part does not re-trigger the streaming presentation', () => {
-    const part = makeReasoningPart({ start: 1_000 });
-    const first = renderPart(part, undefined);
-    const second = renderPart(part, undefined);
+  test('a live part with no committed text yet shows the busy header and no empty summary', () => {
+    // The streaming early-return keeps the block mounted before the block-level
+    // reveal commits a first line. The header must read as busy and must not
+    // paint an empty summary row.
+    const markup = renderPart(makeReasoningPart({ start: 1_000 }, ''), 'streaming');
+    const withText = renderPart(makeReasoningPart({ start: 1_000 }), undefined);

-    expect(second).toBe(first);
-    expect(second).not.toContain(BUSY_INDICATOR);
-    expect(second).toContain(SHORT_REASONING);
+    expect(markup).toContain(BUSY_INDICATOR);
+    expect(markup).toContain('role="button"');
+    // The summary span carries `title="<summary>"`; with no text there must be
+    // no summary span at all rather than an empty one.
+    expect(withText).toContain('title="');
+    expect(markup).not.toContain('title="');
+  });
+
+  test('remounting a completed reasoning part does not re-trigger the streaming presentation', async () => {
+    // renderToStaticMarkup cannot observe this: it has no mount lifecycle, so
+    // comparing two server renders is true by construction. Mount, unmount and
+    // remount a real client root instead, watching the busy indicator across
+    // every commit.
+    const dom = installDomStub();
+    const part = makeReasoningPart({ start: 1_000 });
+    const busySeen: boolean[] = [];
+    const root = createRoot(dom.container);
+
+    const renderTree = () =>
+      React.createElement(
+        I18nProvider,
+        null,
+        React.createElement(ReasoningPart, { part, messageId: 'msg_2020', streamPhase: undefined }),
+      );
+
+    try {
+      await act(async () => {
+        root.render(renderTree());
+      });
+      busySeen.push(dom.container.innerHTML.includes(BUSY_INDICATOR));
+      expect(dom.container.textContent).toContain(SHORT_REASONING);
+
+      await act(async () => {
+        root.render(null);
+      });
+      await act(async () => {
+        root.render(renderTree());
+      });
+      busySeen.push(dom.container.innerHTML.includes(BUSY_INDICATOR));
+
+      expect(busySeen).toEqual([false, false]);
+      expect(dom.container.textContent).toContain(SHORT_REASONING);
+    } finally {
+      await act(async () => {
+        root.unmount();
+      });
+      dom.restore();
+    }
  });
 });
@@ -61,6 +61,8 @@ import {
    getPatchText,
    getPrimaryDiffFromMetadata,
    getPrimaryToolPath,
+    getToolFallbackDiff,
+    resolveToolQuickOpenTarget,
    type DiffPatchEntry,
 } from './toolDiffUtils';
 import { isEmbeddedSessionChat } from '@/components/layout/contextPanelEmbeddedChat';
@@ -1248,12 +1250,7 @@ const ToolExpandedContent: React.FC<ToolExpandedContentProps> = React.memo(({
    });
    const outputString = isStreamingBash ? throttledOutputString : rawOutputString;
    const attachments = stateWithData.attachments;
-    const fileDiff = isRecord(metadata?.filediff) ? metadata.filediff : undefined;
-    const diffContent = getPatchText((metadata as { patch?: unknown } | undefined)?.patch)
-        ?? getPatchText(metadata?.diff)
-        ?? getPatchText(fileDiff?.patch)
-        ?? getPatchText(fileDiff?.diff)
-        ?? null;
+    const diffContent = getToolFallbackDiff(metadata) ?? null;
    const diffEntries = React.useMemo(
        () => getDiffPatchEntries(metadata, diffContent ?? undefined, (path) => getRelativePath(path, currentDirectory)),
        [currentDirectory, diffContent, metadata]
@@ -2055,16 +2052,14 @@ const ToolPartContent: React.FC<ToolPartProps> = ({
    const quickOpenTarget = React.useMemo<{ absolutePath: string; line?: number; toolDiff?: string; toolName: string } | null>(() => {
        if (isTaskTool) return null;
        const toolName = normalizedPartTool || part.tool;
-        const filePath = getPrimaryToolPath(toolName, input, metadata);
-        if (typeof filePath !== 'string') return null;
-        const absolutePath = toAbsoluteFilePath(currentDirectory, filePath);
-        let line: number | undefined;
-        let toolDiff: string | undefined;
-        if (toolName === 'edit' || toolName === 'multiedit' || toolName === 'apply_patch') {
-            line = getFirstChangedLineFromMetadata(toolName, metadata, filePath);
-            toolDiff = getPrimaryDiffFromMetadata(toolName, metadata, filePath);
-        }
-        return { absolutePath, line, toolDiff, toolName };
+        const target = resolveToolQuickOpenTarget(toolName, input, metadata);
+        if (!target) return null;
+        return {
+            absolutePath: toAbsoluteFilePath(currentDirectory, target.filePath),
+            line: target.line,
+            toolDiff: target.patch,
+            toolName,
+        };
    }, [isTaskTool, normalizedPartTool, part.tool, input, metadata, currentDirectory]);

    const openQuickTarget = () => {
@@ -2202,9 +2197,7 @@ const ToolPartContent: React.FC<ToolPartProps> = ({
                                        onClick={handleQuickOpen}
                                        className={cn(
                                            'flex-shrink-0 inline-flex h-4 w-4 items-center justify-center rounded transition-opacity hover:bg-[var(--surface-hover)]',
-                                            // Coarse pointers never hover, so the icon has to rest visible
-                                            // there or it stays invisible while remaining tappable.
-                                            'opacity-0 group-hover/tool:opacity-60 hover:opacity-100 focus-visible:opacity-100 pointer-coarse:opacity-60',
+                                            'opacity-60 hover:opacity-100 focus-visible:opacity-100',
                                        )}
                                        style={{ color: 'var(--tools-icon)' }}
                                        title={t('chat.toolPart.openFile')}
@@ -1,6 +1,7 @@
 import { describe, expect, test } from 'bun:test';

 import {
+    extractFirstChangedLineFromDiff,
    getApplyPatchFilePath,
    getDiffPatchEntries,
    getFirstChangedLineFromMetadata,
@@ -8,6 +9,7 @@ import {
    getPrimaryDiffFromMetadata,
    getPrimaryToolPath,
    getRenderablePatchInfo,
+    resolveToolQuickOpenTarget,
 } from './toolDiffUtils';

 const identity = (path: string) => path;
@@ -203,4 +205,52 @@ describe('toolDiffUtils', () => {
        expect(entries[0]?.renderMode).toBe('text');
        expect(entries[0]?.patch).toContain('@@');
    });
+    test('resolves the quick-open target from the same entry the expanded card renders', () => {
+        const patch = [
+            '--- a/src/file.ts',
+            '+++ b/src/file.ts',
+            '@@ -10,3 +12,4 @@',
+            ' context',
+            '+added',
+        ].join('\n');
+        const metadata = {
+            files: [{
+                filePath: '/workspace/project/src/file.ts',
+                relativePath: 'src/file.ts',
+                patch,
+                type: 'update',
+            }],
+        };
+        const entries = getDiffPatchEntries(metadata, undefined, identity);
+
+        expect(resolveToolQuickOpenTarget('apply_patch', undefined, metadata)).toEqual({
+            filePath: '/workspace/project/src/file.ts',
+            line: extractFirstChangedLineFromDiff(entries[0]?.patch ?? ''),
+            patch: entries[0]?.patch,
+        });
+    });
+
+    test('picks the entry matching the primary path in a multi-file apply_patch', () => {
+        const firstPatch = ['--- a/src/a.ts', '+++ b/src/a.ts', '@@ -1,2 +1,3 @@', ' a', '+first'].join('\n');
+        const secondPatch = ['--- a/src/b.ts', '+++ b/src/b.ts', '@@ -30,2 +40,3 @@', ' b', '+second'].join('\n');
+        const metadata = {
+            files: [
+                { filePath: '/workspace/project/src/a.ts', relativePath: 'src/a.ts', patch: firstPatch, type: 'delete' },
+                { filePath: '/workspace/project/src/b.ts', relativePath: 'src/b.ts', patch: secondPatch, type: 'update' },
+            ],
+        };
+        const target = resolveToolQuickOpenTarget('apply_patch', undefined, metadata);
+
+        expect(target?.filePath).toBe('/workspace/project/src/b.ts');
+        expect(target?.line).toBe(41);
+    });
+
+    test('reports no line when the tool has no diff entry', () => {
+        expect(resolveToolQuickOpenTarget('write', { filePath: '/workspace/project/src/new.ts' }, undefined))
+            .toEqual({ filePath: '/workspace/project/src/new.ts', line: undefined, patch: undefined });
+    });
+
+    test('returns no quick-open target without a primary path', () => {
+        expect(resolveToolQuickOpenTarget('bash', { command: 'ls' }, undefined)).toBeNull();
+    });
 });
@@ -261,6 +261,15 @@ export const getPrimaryDiffFromMetadata = (
    return getPatchText(metadata.patch) ?? getPatchText(metadata.diff);
 };

+/** Top-level patch a tool card falls back to when metadata carries no per-file entries. */
+export const getToolFallbackDiff = (metadata: Record<string, unknown> | undefined): string | undefined => {
+    const fileDiff = isRecord(metadata?.filediff) ? metadata.filediff : undefined;
+    return getPatchText(metadata?.patch)
+        ?? getPatchText(metadata?.diff)
+        ?? getPatchText(fileDiff?.patch)
+        ?? getPatchText(fileDiff?.diff);
+};
+
 export const extractFirstChangedLineFromDiff = (diffText: string): number | undefined => {
    if (!diffText) {
        return undefined;
@@ -330,6 +339,33 @@ export const getFirstChangedLineFromMetadata = (
    return firstPatch ? extractFirstChangedLineFromDiff(firstPatch) : undefined;
 };

+/**
+ * Quick-open target for a tool card: the primary mutated file plus the diff
+ * entry the expanded card renders for it. Both the collapsed header icon and
+ * the expanded "open file" button resolve their line from the same entry
+ * patch, so they always land on the same line.
+ */
+export const resolveToolQuickOpenTarget = (
+    toolName: string,
+    input: Record<string, unknown> | undefined,
+    metadata: Record<string, unknown> | undefined,
+): { filePath: string; line?: number; patch?: string } | null => {
+    const filePath = getPrimaryToolPath(toolName, input, metadata);
+    if (!filePath) {
+        return null;
+    }
+
+    const entries = getDiffPatchEntries(metadata, getToolFallbackDiff(metadata), (path) => path);
+    const matchedEntry = entries.find((entry) => entry.filePath === filePath)
+        ?? (entries.length === 1 ? entries[0] : undefined);
+    const patch = matchedEntry?.patch;
+    return {
+        filePath,
+        line: patch ? extractFirstChangedLineFromDiff(patch) : undefined,
+        patch,
+    };
+};
+
 const normalizeParsedPath = (path: string | undefined): string => {
    const trimmed = (path ?? '').trim().replace(/\t.*$/, '');
    if (!trimmed || trimmed === '/dev/null') {