Skip to content

Commit 0c8b281

Browse files
committed
fix(tui): free syntax styles after theme pins drop them
SyntaxStyle.destroy frees the native style and nothing finalizes it. Destroying on cache overwrite use-after-frees a style the rows or prompt still paint, so the previous style stays queued until they drop it.
1 parent ed57bc8 commit 0c8b281

8 files changed

Lines changed: 274 additions & 75 deletions

File tree

‎src/tui/runner/settings-theme.test.ts‎

Lines changed: 117 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@
1111
*/
1212

1313
import { afterEach, describe, expect, test } from "bun:test";
14-
import { RGBA, type CapturedSpan } from "@opentui/core";
14+
import { RGBA, SyntaxStyle, type CapturedSpan } from "@opentui/core";
1515

1616
import { corbitsDark, corbitsLight, setTheme, UI } from "../theme.js";
1717
import { transcriptSyntaxStyle } from "../stream.js";
1818
import { withTestRenderer, type Harness } from "../harness.js";
19+
import { settleSpans } from "../test-helpers.js";
1920
import { appendStreamRow } from "../shell/chrome.js";
2021
import { createAppShell } from "../shell/index.js";
22+
import { setPromptRecognitionSource } from "../shell/internals.js";
23+
import { syncPromptHighlights } from "../shell/prompt.js";
2124
import { applyThemePinLive, repaintShellForTheme } from "./settings.js";
2225

2326
afterEach(() => {
@@ -31,34 +34,20 @@ function registryHeadingFg(h: Harness): RGBA | undefined {
3134
.get("markup.heading.1")?.fg;
3235
}
3336

34-
function painted(spans: CapturedSpan[], text: string, fg: RGBA): boolean {
35-
return spans.some((s) => s.text.includes(text) && s.fg.equals(fg));
36-
}
37-
38-
/** 0.6.x leases a session buffer around each presented frame; a capture
39-
* landing inside that window throws OutputBusy. The poll retries on the
40-
* next round anyway, so a losing capture keeps polling instead of
41-
* throwing — the ready predicate, never the trace, decides the outcome. */
42-
function readSpans(h: Harness): CapturedSpan[] {
37+
/** `destroy()` frees the native style and there is no finalizer; a later
38+
* read throws instead of returning a palette. */
39+
function syntaxStyleAlive(style: SyntaxStyle): boolean {
4340
try {
44-
return h.captureSpans().lines.flatMap((line) => line.spans);
41+
style.getStyleCount();
42+
return true;
4543
} catch (err) {
46-
if (err instanceof Error && err.message.includes("OutputBusy")) return [];
44+
if (err instanceof Error && err.message.includes("destroyed")) return false;
4745
throw err;
4846
}
4947
}
5048

51-
async function settleSpans(
52-
h: Harness,
53-
ready: (spans: CapturedSpan[]) => boolean,
54-
): Promise<CapturedSpan[]> {
55-
let spans = readSpans(h);
56-
for (let i = 0; i < 400 && !ready(spans); i++) {
57-
await h.renderOnce();
58-
await new Promise((resolve) => setTimeout(resolve, 25));
59-
spans = readSpans(h);
60-
}
61-
return spans;
49+
function painted(spans: CapturedSpan[], text: string, fg: RGBA): boolean {
50+
return spans.some((s) => s.text.includes(text) && s.fg.equals(fg));
6251
}
6352

6453
describe("applyThemePinLive", () => {
@@ -153,4 +142,109 @@ describe("applyThemePinLive", () => {
153142
expect(registryHeadingFg(h)?.equals(RGBA.fromHex(UI.heading))).toBe(true);
154143
});
155144
});
145+
146+
test("a second pin destroys the transcript style the rows dropped", async () => {
147+
await withTestRenderer(
148+
async (h) => {
149+
setTheme("corbits-dark");
150+
const shell = createAppShell(h.renderer, {
151+
terminal: { columns: 80, rows: 40 },
152+
wireKeys: false,
153+
});
154+
try {
155+
appendStreamRow(shell, { role: "assistant", text: "# pin cycle" });
156+
const first = transcriptSyntaxStyle(h.renderer);
157+
applyThemePinLive("light", () => repaintShellForTheme(shell));
158+
const second = transcriptSyntaxStyle(h.renderer);
159+
expect(second).not.toBe(first);
160+
expect(syntaxStyleAlive(first)).toBe(false);
161+
expect(syntaxStyleAlive(second)).toBe(true);
162+
163+
applyThemePinLive("dark", () => repaintShellForTheme(shell));
164+
const third = transcriptSyntaxStyle(h.renderer);
165+
expect(third).not.toBe(second);
166+
expect(syntaxStyleAlive(second)).toBe(false);
167+
expect(syntaxStyleAlive(third)).toBe(true);
168+
} finally {
169+
shell.dispose();
170+
setTheme("corbits-dark");
171+
}
172+
},
173+
{ width: 80, height: 40 },
174+
);
175+
});
176+
177+
test("refreshing the transcript cache leaves a style rows still paint", async () => {
178+
await withTestRenderer(
179+
async (h) => {
180+
setTheme("corbits-dark");
181+
const shell = createAppShell(h.renderer, {
182+
terminal: { columns: 80, rows: 40 },
183+
wireKeys: false,
184+
});
185+
try {
186+
appendStreamRow(shell, { role: "assistant", text: "# pin cycle" });
187+
const painted = transcriptSyntaxStyle(h.renderer);
188+
setTheme("corbits-light");
189+
const rebuilt = transcriptSyntaxStyle(h.renderer);
190+
expect(rebuilt).not.toBe(painted);
191+
expect(syntaxStyleAlive(painted)).toBe(true);
192+
repaintShellForTheme(shell);
193+
expect(syntaxStyleAlive(painted)).toBe(false);
194+
expect(syntaxStyleAlive(transcriptSyntaxStyle(h.renderer))).toBe(
195+
true,
196+
);
197+
} finally {
198+
shell.dispose();
199+
setTheme("corbits-dark");
200+
}
201+
},
202+
{ width: 80, height: 40 },
203+
);
204+
});
205+
206+
test("a pin destroys the prompt style only after the prompt drops it", async () => {
207+
await withTestRenderer(
208+
async (h) => {
209+
setTheme("corbits-dark");
210+
const shell = createAppShell(h.renderer, {
211+
terminal: { columns: 80, rows: 40 },
212+
wireKeys: false,
213+
});
214+
setPromptRecognitionSource(shell, () => ({
215+
commandNames: ["implement"],
216+
}));
217+
const originalDestroy = SyntaxStyle.prototype.destroy;
218+
let destroyedWhileHeld = false;
219+
try {
220+
shell.prompt.value = "/implement";
221+
syncPromptHighlights(shell);
222+
const painted = shell.prompt.syntaxStyle;
223+
expect(painted).not.toBeNull();
224+
if (painted === null) return;
225+
SyntaxStyle.prototype.destroy = function destroy(this: SyntaxStyle) {
226+
if (this === painted && shell.prompt.syntaxStyle === painted) {
227+
destroyedWhileHeld = true;
228+
}
229+
return originalDestroy.call(this);
230+
};
231+
applyThemePinLive("light", () => repaintShellForTheme(shell));
232+
expect(shell.prompt.syntaxStyle).toBe(painted);
233+
expect(syntaxStyleAlive(painted)).toBe(true);
234+
syncPromptHighlights(shell);
235+
expect(destroyedWhileHeld).toBe(false);
236+
expect(shell.prompt.syntaxStyle).not.toBe(painted);
237+
expect(syntaxStyleAlive(painted)).toBe(false);
238+
const current = shell.prompt.syntaxStyle;
239+
expect(current).not.toBeNull();
240+
if (current !== null) expect(syntaxStyleAlive(current)).toBe(true);
241+
} finally {
242+
SyntaxStyle.prototype.destroy = originalDestroy;
243+
shell.dispose();
244+
setTheme("corbits-dark");
245+
}
246+
},
247+
{ width: 80, height: 40 },
248+
);
249+
});
156250
});

‎src/tui/shell/chrome.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ import {
143143
transcriptRowChildren,
144144
transcriptRowLayout,
145145
transcriptRowOffset,
146+
releaseRetiredTranscriptSyntaxStyles,
146147
} from "./transcript.js";
147148

148149
function syncPending(shell: AppShell): void {
@@ -1411,6 +1412,7 @@ export function repaintTranscriptWindow(shell: AppShell): void {
14111412
),
14121413
);
14131414
});
1415+
releaseRetiredTranscriptSyntaxStyles(shell);
14141416
}
14151417

14161418
/** Tear the landing down on the first transcript row. The prompt box jumps

‎src/tui/shell/prompt.ts‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ import {
3636
interrupt,
3737
} from "../delivery-queue.js";
3838
import { UI, onThemeChange } from "../theme.js";
39+
import {
40+
destroyUnpaintedSyntaxStyles,
41+
syntaxStyleForGeneration,
42+
} from "../syntax-style-cache.js";
3943

4044
import {
4145
type AppShell,
@@ -180,12 +184,15 @@ export function setPromptCostContext(
180184

181185
/** 0.6.x native styles must name their owning Context — the live renderer's
182186
* native scene — so the style is keyed per renderer instead of a module
183-
* global; `promptStyleGeneration` invalidates it like the old cache did. */
187+
* global; `promptStyleGeneration` invalidates it like the old cache did.
188+
* The previous style stays queued until the prompt assignment drops it. */
184189
const promptStylesByRenderer = new WeakMap<
185190
ShellRenderer,
186191
{ generation: number; style: SyntaxStyle }
187192
>();
188193

194+
const retiredPromptStyles = new WeakMap<ShellRenderer, SyntaxStyle[]>();
195+
189196
let promptStyleGeneration = 0;
190197

191198
const promptHighlightGenerations = new WeakMap<AppShell, number>();
@@ -199,19 +206,17 @@ onThemeChange(() => {
199206
* render lib.
200207
*/
201208
function promptStyleFor(renderer: ShellRenderer): SyntaxStyle {
202-
const cached = promptStylesByRenderer.get(renderer);
203-
if (cached !== undefined && cached.generation === promptStyleGeneration) {
204-
return cached.style;
205-
}
206-
const style = SyntaxStyle.fromStyles(
207-
{ recognized: { fg: UI.action } },
208-
renderer.nativeScene,
209+
return syntaxStyleForGeneration(
210+
promptStylesByRenderer,
211+
retiredPromptStyles,
212+
renderer,
213+
promptStyleGeneration,
214+
() =>
215+
SyntaxStyle.fromStyles(
216+
{ recognized: { fg: UI.action } },
217+
renderer.nativeScene,
218+
),
209219
);
210-
promptStylesByRenderer.set(renderer, {
211-
generation: promptStyleGeneration,
212-
style,
213-
});
214-
return style;
215220
}
216221

217222
const promptHighlightedValue = new WeakMap<AppShell, string>();
@@ -235,6 +240,11 @@ export function syncPromptHighlights(shell: AppShell): void {
235240
const style = promptStyleFor(shell.renderer);
236241
const styleId = style.resolveStyleId("recognized") ?? 0;
237242
shell.prompt.syntaxStyle = style;
243+
destroyUnpaintedSyntaxStyles(
244+
retiredPromptStyles,
245+
shell.renderer,
246+
(pending) => shell.prompt.syntaxStyle === pending,
247+
);
238248
shell.prompt.clearAllHighlights();
239249
const matcher = resolvePromptRecognitionMatcher(source);
240250
for (const span of resolvePromptHighlightSpans(value, matcher)) {

‎src/tui/shell/transcript.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
StyledText,
1010
type BaseRenderable,
1111
type CliRenderer,
12+
type SyntaxStyle,
1213
} from "@opentui/core";
1314
import { stringWidth } from "../view/height.js";
1415
import { viewToTableContent, type McpStructuredView } from "../mcp-view.js";
@@ -37,6 +38,7 @@ import {
3738
toolRowLines,
3839
toolSentenceLines,
3940
transcriptSyntaxStyle,
41+
dropRetiredTranscriptSyntaxStyles,
4042
type PaintedStreamLine,
4143
type RowLayout,
4244
type StreamRow,
@@ -464,6 +466,27 @@ function createMarkdownBody(
464466
return column;
465467
}
466468

469+
/** Markdown bodies still holding a syntax style. A theme pin must not free
470+
* a style that one of these nodes is still painting. */
471+
function paintedMarkdownSyntaxStyles(node: BaseRenderable): Set<SyntaxStyle> {
472+
const painted = new Set<SyntaxStyle>();
473+
const walk = (current: BaseRenderable): void => {
474+
if (current instanceof MarkdownRenderable) {
475+
painted.add(current.syntaxStyle);
476+
}
477+
for (const child of current.getChildren()) walk(child);
478+
};
479+
walk(node);
480+
return painted;
481+
}
482+
483+
/** After the transcript rows have been rebuilt, free styles they dropped. */
484+
export function releaseRetiredTranscriptSyntaxStyles(shell: AppShell): void {
485+
dropRetiredTranscriptSyntaxStyles(shell.renderer as CliRenderer, () =>
486+
paintedMarkdownSyntaxStyles(shell.renderer.root as BaseRenderable),
487+
);
488+
}
489+
467490
/** Gutter + one pre-coloured text line per body row (a diff, expanded tool
468491
* arguments); wraps land under the body. */
469492
function createStyledLinesRowRenderable(

‎src/tui/stream.ts‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ import {
1212
type Thought,
1313
} from "./thinking.js";
1414
import { UI, onThemeChange } from "./theme.js";
15+
import {
16+
syntaxStyleForGeneration,
17+
destroyUnpaintedSyntaxStyles,
18+
} from "./syntax-style-cache.js";
1519
import { pastTenseToolLabel } from "./tool-formatter.js";
1620

1721
/** One pre-coloured body line (an expanded call's structured args). */
@@ -776,30 +780,42 @@ function markdownStyles() {
776780
/** 0.6.x native styles must name the Context that owns them; the owner is the
777781
* live renderer, so the shared style is keyed per renderer instead of held in
778782
* a module global. A theme pin bumps the generation; the next reader rebuilds
779-
* from the new `UI` colors, matching the old invalidation. */
783+
* from the new `UI` colors, matching the old invalidation. The previous
784+
* style stays alive until rows drop it (`dropRetiredTranscriptSyntaxStyles`). */
780785
const transcriptStylesByRenderer = new WeakMap<
781786
CliRenderer,
782787
{ generation: number; style: SyntaxStyle }
783788
>();
784789

790+
const retiredTranscriptStyles = new WeakMap<CliRenderer, SyntaxStyle[]>();
791+
785792
let transcriptStyleGeneration = 0;
786793

787794
/** Shared transcript SyntaxStyle for `ctx`, built lazily — the native render
788795
* lib is unreachable until a renderer exists. */
789796
export function transcriptSyntaxStyle(ctx: CliRenderer): SyntaxStyle {
790-
const cached = transcriptStylesByRenderer.get(ctx);
791-
if (cached !== undefined && cached.generation === transcriptStyleGeneration) {
792-
return cached.style;
793-
}
794-
const style = SyntaxStyle.fromStyles(
795-
{ ...markdownStyles() },
796-
ctx.nativeScene,
797+
return syntaxStyleForGeneration(
798+
transcriptStylesByRenderer,
799+
retiredTranscriptStyles,
800+
ctx,
801+
transcriptStyleGeneration,
802+
() => SyntaxStyle.fromStyles({ ...markdownStyles() }, ctx.nativeScene),
803+
);
804+
}
805+
806+
/** Destroy transcript styles this renderer no longer paints. `painted` runs
807+
* only when a previous style is queued, and only after rows have been
808+
* rebuilt — a style the thunk still reports stays queued. */
809+
export function dropRetiredTranscriptSyntaxStyles(
810+
ctx: CliRenderer,
811+
painted: () => ReadonlySet<SyntaxStyle>,
812+
): void {
813+
const pending = retiredTranscriptStyles.get(ctx);
814+
if (pending === undefined || pending.length === 0) return;
815+
const held = painted();
816+
destroyUnpaintedSyntaxStyles(retiredTranscriptStyles, ctx, (style) =>
817+
held.has(style),
797818
);
798-
transcriptStylesByRenderer.set(ctx, {
799-
generation: transcriptStyleGeneration,
800-
style,
801-
});
802-
return style;
803819
}
804820

805821
onThemeChange(() => {

0 commit comments

Comments
 (0)