Skip to content

Commit ca2da34

Browse files
committed
fix(markdown): prevent DOM dependency crashes in parse worker
Resolve the named character decoder to its portable entry so Markdown workers can initialize without document in development and production. Disable failed prewarming workers while retaining main-thread parsing. Cover production worker output, GFM parsing, and failure fallback.
1 parent 7ac47e6 commit ca2da34

7 files changed

Lines changed: 172 additions & 5 deletions

File tree

‎pnpm-lock.yaml‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/web-ui/AGENTS.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@ establish visual fidelity):
141141
pnpm --dir src/web-ui run test:run src/infrastructure/update src/infrastructure/api/service-api/SystemAPI.test.ts src/shared/notification-system/components/NotificationContainer.test.tsx
142142
```
143143

144+
For Markdown parse prewarming or dependency resolution, run
145+
`pnpm --dir src/web-ui run test:run src/infrastructure/markdown/markdownParse.worker.test.ts src/infrastructure/markdown/markdownParseCache.test.ts`.
146+
This builds the real Worker entry with the product aliases, checks parsing without
147+
a DOM, and covers failure fallback. Build configuration changes also need `pnpm run build:web`.
148+
144149
For shared overlay ordering, dismissal, focus or presence changes, run the
145150
focused DOM and ownership contracts in addition to `check:web` (these do not
146151
establish visual or remote transport behavior):

‎src/web-ui/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
"@xterm/addon-webgl": "^0.20.0-beta.100",
5959
"@xterm/headless": "6.1.0-beta.141",
6060
"@xterm/xterm": "^6.1.0-beta.101",
61+
"decode-named-character-reference": "1.3.0",
6162
"diff": "^8.0.3",
6263
"highlight.js": "^11.11.1",
6364
"i18next": "^25.8.0",
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// @vitest-environment node
2+
import path from 'node:path';
3+
import { fileURLToPath } from 'node:url';
4+
import { runInNewContext } from 'node:vm';
5+
import { build, resolveConfig } from 'vite';
6+
import { expect, it } from 'vitest';
7+
import { unified } from 'unified';
8+
import remarkParse from 'remark-parse';
9+
import remarkGfm from 'remark-gfm';
10+
11+
const configFile = fileURLToPath(new URL('../../../vite.config.ts', import.meta.url));
12+
const root = path.dirname(configFile);
13+
const entry = fileURLToPath(new URL('./markdownParseCache.ts', import.meta.url));
14+
15+
it('builds a Markdown worker that initializes and parses GFM without a DOM', async () => {
16+
const config = await resolveConfig({ root, configFile, logLevel: 'silent' }, 'build');
17+
// Use the product resolver and real new Worker URL, without unrelated app/font builds.
18+
const result = await build({
19+
root,
20+
configFile: false,
21+
publicDir: false,
22+
logLevel: 'silent',
23+
resolve: { alias: config.resolve.alias },
24+
build: {
25+
write: false,
26+
minify: true,
27+
rollupOptions: { input: entry, preserveEntrySignatures: 'strict' },
28+
},
29+
});
30+
if ('on' in result) throw new Error('Unexpected watch build');
31+
const output = (Array.isArray(result) ? result : [result]).flatMap(bundle => bundle.output);
32+
const worker = output.find(asset => asset.type === 'asset' && /markdownParse\.worker-.*\.js$/.test(asset.fileName));
33+
if (!worker || worker.type !== 'asset') throw new Error('Markdown worker asset was not emitted');
34+
const replies: unknown[] = [];
35+
const self = {
36+
postMessage: (message: unknown) => replies.push(message),
37+
onmessage: undefined as undefined | ((event: { data: string }) => void),
38+
};
39+
runInNewContext(String(worker.source), { self }, { timeout: 2000 });
40+
expect(self.onmessage).toBeTypeOf('function');
41+
for (const source of [
42+
'# Entities © & ≂̸ 🚀 &unknown;\n\n[link][target]\n\n[target]: https://example.com',
43+
'| A | B |\n| - | - |\n| ~~x~~ | y |\n\n- [x] done\n\nNote[^1]\n\n[^1]: Footnote',
44+
]) {
45+
self.onmessage!({ data: source });
46+
expect(replies.pop()).toEqual({ tree: unified().use(remarkParse).use(remarkGfm).parse(source) });
47+
}
48+
}, 20_000);
49+
50+
it('resolves the DOM-free decoder in development too', async () => {
51+
const config = await resolveConfig({ root, configFile, logLevel: 'silent' }, 'serve');
52+
const resolved = await config.createResolver()('decode-named-character-reference', entry);
53+
expect(resolved).toMatch(/\/decode-named-character-reference\/index\.js$/);
54+
});

‎src/web-ui/src/infrastructure/markdown/markdownParseCache.test.ts‎

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, it } from 'vitest';
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
22
import { unified } from 'unified';
33
import remarkParse from 'remark-parse';
44
import remarkGfm from 'remark-gfm';
@@ -39,3 +39,89 @@ describe('immutable Markdown parse cache', () => {
3939
expect(JSON.stringify(first)).not.toContain('onError');
4040
});
4141
});
42+
43+
describe('Markdown worker prewarming', () => {
44+
const text = 'Paragraph with © and **emphasis**.\n\n'.repeat(80);
45+
let workers: TestWorker[];
46+
class TestWorker {
47+
onerror?: () => void;
48+
onmessage?: (event: { data: { tree?: unknown } }) => void;
49+
postMessage = vi.fn();
50+
terminate = vi.fn();
51+
constructor() { workers.push(this); }
52+
}
53+
54+
beforeEach(() => {
55+
vi.resetModules();
56+
vi.useFakeTimers();
57+
workers = [];
58+
vi.stubGlobal('Worker', TestWorker);
59+
});
60+
afterEach(() => {
61+
vi.clearAllTimers();
62+
vi.useRealTimers();
63+
vi.unstubAllGlobals();
64+
});
65+
66+
it('stops restarting a failed worker and preserves main-thread parsing', async () => {
67+
const { prewarmMarkdownParse, remarkCachedParse: cachedParse } = await import('./markdownParseCache');
68+
prewarmMarkdownParse(text);
69+
prewarmMarkdownParse(`${text} queued`);
70+
workers[0].onerror!();
71+
prewarmMarkdownParse(`${text} streamed update`);
72+
vi.advanceTimersByTime(60_000);
73+
prewarmMarkdownParse(`${text} later update`);
74+
expect(workers).toHaveLength(1);
75+
expect(workers[0].terminate).toHaveBeenCalledOnce();
76+
expect(workers[0].postMessage).toHaveBeenCalledTimes(1);
77+
const processor = unified().use(remarkParse).use(remarkGfm).use(cachedParse);
78+
expect(processor.parse(text)).toEqual(unified().use(remarkParse).use(remarkGfm).parse(text));
79+
});
80+
81+
it('does not retry a worker whose constructor throws', async () => {
82+
const constructor = vi.fn(function () { throw new Error('Worker unavailable'); });
83+
vi.stubGlobal('Worker', constructor);
84+
const { prewarmMarkdownParse } = await import('./markdownParseCache');
85+
expect(() => prewarmMarkdownParse(text)).not.toThrow();
86+
prewarmMarkdownParse(`${text} update`);
87+
expect(constructor).toHaveBeenCalledOnce();
88+
});
89+
90+
it('disables and terminates the worker if posting fails', async () => {
91+
const { prewarmMarkdownParse } = await import('./markdownParseCache');
92+
prewarmMarkdownParse(text);
93+
workers[0].onmessage!({ data: {} });
94+
workers[0].postMessage.mockImplementation(() => { throw new Error('Cannot post'); });
95+
expect(() => prewarmMarkdownParse(`${text} next`)).not.toThrow();
96+
prewarmMarkdownParse(`${text} update`);
97+
expect(workers).toHaveLength(1);
98+
expect(workers[0].terminate).toHaveBeenCalledOnce();
99+
expect(workers[0].postMessage).toHaveBeenCalledTimes(2);
100+
});
101+
102+
it('caches successful replies and can restart after normal idle termination', async () => {
103+
const { prewarmMarkdownParse, remarkCachedParse: cachedParse } = await import('./markdownParseCache');
104+
const tree = unified().use(remarkParse).use(remarkGfm).parse(text);
105+
prewarmMarkdownParse(text);
106+
workers[0].onmessage!({ data: { tree } });
107+
const processor = unified().use(remarkParse).use(remarkGfm).use(cachedParse);
108+
expect(processor.parse(text)).toEqual(tree);
109+
prewarmMarkdownParse(text);
110+
expect(workers[0].postMessage).toHaveBeenCalledOnce();
111+
vi.advanceTimersByTime(30_000);
112+
expect(workers[0].terminate).toHaveBeenCalledOnce();
113+
prewarmMarkdownParse(`${text} next`);
114+
expect(workers).toHaveLength(2);
115+
});
116+
117+
it('handles posting failures while advancing the queue after a reply', async () => {
118+
const { prewarmMarkdownParse } = await import('./markdownParseCache');
119+
prewarmMarkdownParse(text);
120+
prewarmMarkdownParse(`${text} queued`);
121+
workers[0].postMessage.mockImplementation(() => { throw new Error('Cannot post'); });
122+
expect(() => workers[0].onmessage!({ data: {} })).not.toThrow();
123+
prewarmMarkdownParse(`${text} update`);
124+
expect(workers).toHaveLength(1);
125+
expect(workers[0].terminate).toHaveBeenCalledOnce();
126+
});
127+
});

‎src/web-ui/src/infrastructure/markdown/markdownParseCache.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,18 @@ const jobs = new Map<string, number>();
77
let worker: Worker | undefined;
88
let current: string | undefined;
99
let idle: ReturnType<typeof setTimeout> | undefined;
10+
let workerFailed = false;
11+
12+
function disableWorker() {
13+
// Prewarming is optional; parsing on the main thread remains available.
14+
// A broken worker must not be recreated for every streamed content update.
15+
workerFailed = true;
16+
clearTimeout(idle);
17+
worker?.terminate();
18+
worker = undefined;
19+
current = undefined;
20+
jobs.clear();
21+
}
1022

1123
const cacheTree = (text: string, tree: Root) => cache.set(text, tree, text.length * 14 + 256);
1224

@@ -38,20 +50,20 @@ function pump() {
3850
if (event.data.tree && jobs.has(current)) cacheTree(current, event.data.tree);
3951
jobs.delete(current); current = undefined;
4052
}
41-
pump();
53+
try { pump(); } catch { disableWorker(); }
4254
};
43-
worker.onerror = () => { worker?.terminate(); worker = undefined; current = undefined; jobs.clear(); };
55+
worker.onerror = disableWorker;
4456
}
4557
current = next;
4658
worker.postMessage(next);
4759
}
4860

4961
/** Optional, cancellable warmup of immutable text. No view waits for this queue. */
5062
export function prewarmMarkdownParse(text: string): () => void {
51-
if (text.length < 2000 || text.length > 400_000 || cache.has(text) || typeof Worker === 'undefined') return () => {};
63+
if (workerFailed || text.length < 2000 || text.length > 400_000 || cache.has(text) || typeof Worker === 'undefined') return () => {};
5264
if (!jobs.has(text) && jobs.size >= 16) return () => {};
5365
jobs.set(text, (jobs.get(text) ?? 0) + 1);
54-
try { pump(); } catch { jobs.clear(); current = undefined; worker?.terminate(); worker = undefined; }
66+
try { pump(); } catch { disableWorker(); }
5567
return () => {
5668
const count = jobs.get(text) ?? 0;
5769
if (count <= 1) jobs.delete(text); else jobs.set(text, count - 1);

‎src/web-ui/vite.config.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,12 @@ export default defineConfig(({ mode, command }) => {
177177
resolve: {
178178
dedupe: ['react', 'react-dom', 'lucide-react'],
179179
alias: [
180+
// The browser export uses document at module load and crashes Markdown workers.
181+
// Use the package's portable decoder in both the page and its workers.
182+
{
183+
find: /^decode-named-character-reference$/,
184+
replacement: requireFromWebUi.resolve('decode-named-character-reference'),
185+
},
180186
// @xterm/headless 6.1.0-beta.141 advertises a missing lib/xterm.mjs.
181187
// Resolve its published ESM entry for both dev optimization and builds.
182188
{

0 commit comments

Comments
 (0)