From 2096050824507db3775177f44e81a127d7731042 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 25 Jul 2026 01:22:46 +0800 Subject: [PATCH] fix(webserver): registry-owned stat poll replaces fs.watchFile baseline race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dev bundle watch missed rebuilds that landed while the registry was constructing: fs.watchFile captures its comparison baseline with an ASYNCHRONOUS first stat, so a write racing that window is absorbed into the baseline and never reported. Standalone repro missed 24/400 same-tick rewrites; the CI flake in web-plugins.spec.ts ('watch mode: a bundle content change re-hashes the row...') was exactly this — the spec writes immediately after createHostWebPluginRegistry returns. The watch now polls from one registry-owned setInterval against a stat baseline the scan itself captures synchronously, stat-before-read: a write landing between stat and read leaves the hash newer than the baseline (next tick re-hashes to the same rev, no spurious notify); a write landing after the read leaves the baseline older (next tick detects and notifies). No blind window. The poll iterates the live table, so rescans retarget the watch for free and dispose clears one timer. Stress: real-registry same-tick rewrite 0/600 missed (was 1/300); spec watch tests 0/50. New regression test pins the same-tick-as-construction write. Its rewrite deliberately differs in size from the seed: a same-millisecond same-size rewrite is invisible to any mtime+size poll (coarse fs timestamps) — a stat-polling limit, not this regression. Loading-model Agent Note updated in both languages (pair re-recorded). --- ...7-23-client-plugin-loading-model.i18n.yaml | 4 +- .../2026-07-23-client-plugin-loading-model.md | 2 +- ...26-07-23-client-plugin-loading-model.zh.md | 2 +- packages/host/webserver/src/web-plugins.ts | 115 +++++++++++------- .../host/webserver/tests/web-plugins.spec.ts | 21 ++++ 5 files changed, 93 insertions(+), 51 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.i18n.yaml b/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.i18n.yaml index 54df5f07ab..9e73a9a48e 100644 --- a/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write -2026-07-23-client-plugin-loading-model.md: 58651fd258a6b2929c58bb6f93b44adb6e8e1818 -2026-07-23-client-plugin-loading-model.zh.md: f60b06c7bfaa9c70170082ac4384ba2bd899676e +2026-07-23-client-plugin-loading-model.md: 5a26561be300eefc4bbbadae5d3cc26ba4068f47 +2026-07-23-client-plugin-loading-model.zh.md: baa524e33b45afd290f8be2c6ae57f10fa4968b7 diff --git a/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.md b/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.md index 58651fd258..5a26561be3 100644 --- a/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.md +++ b/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.md @@ -76,7 +76,7 @@ Why is the roster a hand-written list and not a scan? Because which plugins comp Whether hot reload is active is a composition decision: dev graphs include the `client-hmr` row (a normal plugin package) and turn on bundle watching; prod graphs do neither. -How does a rebuilt bundle become a reload signal? The webserver observes it itself — no builder tells it. The registry scan already holds every plugin's bundle path (`clientPath`), so in dev mode the registry stat-polls each scanned bundle file with `fs.watchFile`. Polling is by design: inotify does not fire on the weka network mount, the same reason the build-side watcher needs `--poll`. On a mtime/size change the registry re-hashes that row (`rebuilt(id)`); when the `rev` actually changed, it broadcasts a `rebuilt` frame on `GET /plugins/events` — a system SSE channel that sends the full graph on connect and `rebuilt` frames on change, presentation-only wire that never enters the session log. Watch set membership follows the table: rescans add watches for new rows and drop them for vanished ones, dispose drops all. The poll interval is a validated config field (default 500ms), not a constant. Rebuilding the bundles is any tsdown watch process's business — `scripts/dev-web.ts` remains as the watch-build entry point, its package list dshClient-discovered by scanning `packages/*/*/package.json` at startup — and builder and host share zero protocol. A torn read of a half-written bundle self-heals: the stats keep changing while the write completes, so the next poll tick re-hashes again and broadcasts the final rev. +How does a rebuilt bundle become a reload signal? The webserver observes it itself — no builder tells it. The registry scan already holds every plugin's bundle path (`clientPath`), so in dev mode one registry-owned interval stat-polls every scanned bundle file against the stat baseline its own scan captured (synchronously, immediately before hashing that content — not `fs.watchFile`, whose asynchronous first-stat baseline silently absorbs a write landing during registry construction). Polling is by design: inotify does not fire on the weka network mount, the same reason the build-side watcher needs `--poll`. On a mtime/size change the registry re-hashes that row (`rebuilt(id)`); when the `rev` actually changed, it broadcasts a `rebuilt` frame on `GET /plugins/events` — a system SSE channel that sends the full graph on connect and `rebuilt` frames on change, presentation-only wire that never enters the session log. The poll iterates the live table, so rescans retarget the watch for free (fresh rows carry fresh baselines) and dispose clears the one timer. The poll interval is a validated config field (default 500ms), not a constant. Rebuilding the bundles is any tsdown watch process's business — `scripts/dev-web.ts` remains as the watch-build entry point, its package list dshClient-discovered by scanning `packages/*/*/package.json` at startup — and builder and host share zero protocol. A torn read of a half-written bundle self-heals: the stats keep changing while the write completes, so the next poll tick re-hashes again and broadcasts the final rev. On the browser side, the driver reloads one plugin per frame, serialized: diff --git a/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.zh.md b/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.zh.md index f60b06c7bf..baa524e33b 100644 --- a/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.zh.md +++ b/.agents/notes/implemented/architecture/2026-07-23-client-plugin-loading-model.zh.md @@ -76,7 +76,7 @@ vendored Loader 经其 `internal` seam 消费模块系统——唯一调用点 热重载是否启用是一项组合决策：dev 图包含 `client-hmr` 行（一个常规的插件包）并开启 bundle 监视；prod 图两者皆无。 -重建好的 bundle 怎么变成重载信号？webserver 自己观察——没有构建器来通知它。注册表扫描本就握有每个插件的 bundle 路径（`clientPath`），因此 dev 模式下注册表用 `fs.watchFile` 对每个已扫描的 bundle 文件做 stat 轮询。轮询是刻意选择：inotify 在 weka 网络挂载上不触发，构建侧监视器需要 `--poll` 也是同一原因。mtime/size 一变，注册表就重哈希该行（`rebuilt(id)`）；当 `rev` 真的变了，才在 `GET /plugins/events` 上广播 `rebuilt` 帧——这是一条系统级 SSE（Server-Sent Events）通道，连接即发全量图，变更时发 `rebuilt` 帧，仅供呈现的 wire，永不进会话日志。监视集合的成员随表走：重扫为新行添加监视、为消失的行撤下监视，dispose（资源释放）撤掉全部。轮询间隔是一个经校验的配置字段（默认 500ms），不是常量。重建 bundle 则是任意一个 tsdown watch 进程的事——`scripts/dev-web.ts` 仍作为 watch 构建入口保留，其包清单在启动时扫描 `packages/*/*/package.json` 按 dshClient 发现——构建器与 host 共享零协议。写一半的 bundle 被撕裂读取会自愈：写入完成期间 stat 持续变化，下一个轮询节拍会再次重哈希并广播最终的 rev。 +重建好的 bundle 怎么变成重载信号？webserver 自己观察——没有构建器来通知它。注册表扫描本就握有每个插件的 bundle 路径（`clientPath`），因此 dev 模式下由注册表自持的单个定时器对每个已扫描的 bundle 文件做 stat 轮询，比对基线是扫描自己捕获的 stat（同步地、恰在哈希该内容之前采集——不用 `fs.watchFile`：它以异步首次 stat 建立基线，会把注册表构造期间落盘的写入静默吸收进基线）。轮询是刻意选择：inotify 在 weka 网络挂载上不触发，构建侧监视器需要 `--poll` 也是同一原因。mtime/size 一变，注册表就重哈希该行（`rebuilt(id)`）；当 `rev` 真的变了，才在 `GET /plugins/events` 上广播 `rebuilt` 帧——这是一条系统级 SSE（Server-Sent Events）通道，连接即发全量图，变更时发 `rebuilt` 帧，仅供呈现的 wire，永不进会话日志。轮询直接遍历活表，因此重扫天然重定向监视（新行自带新基线），dispose（资源释放）只需清掉那一个定时器。轮询间隔是一个经校验的配置字段（默认 500ms），不是常量。重建 bundle 则是任意一个 tsdown watch 进程的事——`scripts/dev-web.ts` 仍作为 watch 构建入口保留，其包清单在启动时扫描 `packages/*/*/package.json` 按 dshClient 发现——构建器与 host 共享零协议。写一半的 bundle 被撕裂读取会自愈：写入完成期间 stat 持续变化，下一个轮询节拍会再次重哈希并广播最终的 rev。 浏览器侧，驱动插件每帧重载一个插件，串行执行： diff --git a/packages/host/webserver/src/web-plugins.ts b/packages/host/webserver/src/web-plugins.ts index 9e32cfc012..4d03a15c4b 100644 --- a/packages/host/webserver/src/web-plugins.ts +++ b/packages/host/webserver/src/web-plugins.ts @@ -22,8 +22,7 @@ */ import { createHash } from 'node:crypto' -import { readFileSync, unwatchFile, watchFile } from 'node:fs' -import type { Stats } from 'node:fs' +import { readFileSync, statSync } from 'node:fs' import { dirname, join } from 'node:path' import type { Context } from 'cordis' @@ -106,10 +105,13 @@ export interface WebPluginRegistryDeps { /** Sink for rescan failures (the initial scan throws instead — misconfiguration fails loud at load). */ onError: (err: Error) => void /** - * Dev-mode bundle watching: stat-poll every scanned row's client bundle - * (fs.watchFile — polling by design: network mounts deliver no inotify - * events) and re-hash + notify onRebuilt subscribers on change. Absent = - * no watching (prod composition). + * Dev-mode bundle watching: one registry-owned interval stat-polls every + * scanned row's client bundle (polling by design: network mounts deliver no + * inotify events) and re-hashes + notifies onRebuilt subscribers on change. + * Each row's stat baseline is captured synchronously before its content is + * hashed, so a rebuild landing while the registry constructs is still + * detected on the first tick (fs.watchFile's asynchronous baseline lost + * that window). Absent = no watching (prod composition). */ watch?: { /** Stat-poll interval in milliseconds; default 500 (the build-side watcher's polling default). */ @@ -128,6 +130,15 @@ interface DshClientDeclaration { interface WebPluginRecord { entry: WebBootEntry clientPath: string + /** + * Bundle stat captured immediately BEFORE the content read that produced + * `entry.rev` — the watch baseline. The stat→read order makes a write + * racing the scan converge instead of being absorbed: landing between stat + * and read leaves the hash newer than the baseline (next tick re-hashes to + * the same rev, no spurious notify); landing after the read leaves the + * baseline older (next tick detects, re-hashes, notifies). + */ + stat: { mtimeMs: number; size: number } } /** Narrow an unknown parsed JSON value to the dshClient declaration, throwing on malformed fields. */ @@ -211,57 +222,62 @@ export function createHostWebPluginRegistry(deps: WebPluginRegistryDeps): HostWe const rebuilt = (id: string): string | undefined => { const record = table.get(id) if (record === undefined) return undefined + // stat BEFORE read, like scan(): a write racing this pair converges (see + // WebPluginRecord.stat) instead of desynchronizing baseline and rev. + const stat = statSync(record.clientPath) const rev = shortHash(readFileSync(record.clientPath)) + record.stat = { mtimeMs: stat.mtimeMs, size: stat.size } record.entry = graphRow(id, rev, record.entry.inject, record.entry.immediately === true) graph = composeGraph(table) return rev } - // Dev bundle watch: one fs.watchFile stat poll per table row. A torn read - // of a half-written bundle self-heals — the ongoing write keeps changing - // the stats, so the next poll tick re-hashes the completed file. - const watched = new Map void }>() - const syncWatches = (): void => { - if (watchInterval === undefined) return - for (const [id, watch] of watched) { - if (table.get(id)?.clientPath === watch.path) continue - unwatchFile(watch.path, watch.listener) - watched.delete(id) - } + // Dev bundle watch: one registry-owned setInterval stat-polls every table + // row against the record's own baseline. fs.watchFile is unusable here: it + // captures its comparison baseline with an ASYNCHRONOUS first stat, so a + // rebuild landing between scan()'s content read and that stat is absorbed + // into the baseline and never reported — and the missed window is exactly + // registry construction, when a dev build is most likely to be finishing. + // The record baseline has no such window: scan()/rebuilt() stat before they + // read, so any write the hash missed is newer than the baseline and lands + // on the next tick. A torn read of a half-written bundle self-heals the + // same way — the ongoing write keeps changing the stats. + const pollTick = (): void => { for (const [id, record] of table) { - if (watched.has(id)) continue - const listener = (curr: Stats, prev: Stats): void => { - // fs.watchFile fires on any stat delta (atime included); only content - // signals count. An all-zero curr means the file vanished mid-rebuild - // — the completing write fires the next tick, so skipping is safe. - if (curr.mtimeMs === prev.mtimeMs && curr.size === prev.size) return - if (curr.mtimeMs === 0) return - const before = table.get(id)?.entry.rev - let rev: string | undefined + let stat: { mtimeMs: number; size: number } + try { + stat = statSync(record.clientPath) + } catch (error) { + const code = (error as NodeJS.ErrnoException).code + if (code === 'ENOENT') continue // mid-rename window; the completed write lands on a later tick + deps.onError(error instanceof Error ? error : new Error(String(error))) + continue + } + if (stat.mtimeMs === record.stat.mtimeMs && stat.size === record.stat.size) continue + const before = record.entry.rev + let rev: string | undefined + try { + rev = rebuilt(id) + } catch (error) { + const code = (error as NodeJS.ErrnoException).code + if (code === 'ENOENT') continue // vanished between stat and read; same self-heal + deps.onError(error instanceof Error ? error : new Error(String(error))) + continue + } + if (rev === undefined || rev === before) continue + for (const notify of rebuildListeners) { + // A throwing subscriber must not skip later subscribers or escape + // into the timer callback (that would kill the process). try { - rev = rebuilt(id) + notify(id, rev) } catch (error) { - const code = (error as NodeJS.ErrnoException).code - if (code === 'ENOENT') return // mid-rename window; the completed write fires the next poll tick deps.onError(error instanceof Error ? error : new Error(String(error))) - return - } - if (rev === undefined || rev === before) return - for (const notify of rebuildListeners) { - // A throwing subscriber must not escape the fs.watchFile callback - // (that would skip later subscribers and can kill the process). - try { - notify(id, rev) - } catch (error) { - deps.onError(error instanceof Error ? error : new Error(String(error))) - } } } - watchFile(record.clientPath, { interval: watchInterval, persistent: false }, listener) - watched.set(id, { path: record.clientPath, listener }) } } - syncWatches() + const pollTimer = watchInterval === undefined ? undefined : setInterval(pollTick, watchInterval) + pollTimer?.unref() let pending = false const unsubscribe = deps.ctx.on('internal/plugin', () => { @@ -270,9 +286,10 @@ export function createHostWebPluginRegistry(deps: WebPluginRegistryDeps): HostWe queueMicrotask(() => { pending = false try { + // The poll iterates `table` directly, so the swap also retargets the + // watch: fresh records carry fresh stat baselines from scan(). table = scan(deps) graph = composeGraph(table) - syncWatches() } catch (error) { // Keep serving the previous graph: a mid-flight rescan failure must not // take down the boot manifest for plugins that were fine. @@ -291,8 +308,7 @@ export function createHostWebPluginRegistry(deps: WebPluginRegistryDeps): HostWe }, dispose: () => { unsubscribe() - for (const { path, listener } of watched.values()) unwatchFile(path, listener) - watched.clear() + if (pollTimer !== undefined) clearInterval(pollTimer) rebuildListeners.clear() }, } @@ -314,8 +330,13 @@ function scan(deps: WebPluginRegistryDeps): Map { throw new Error(`web-plugins: ${name} declares dshClient but exports no "./client" bundle`) } const clientPath = join(dirname(pkgPath), clientRel) + const stat = statSync(clientPath) const rev = shortHash(readFileSync(clientPath)) - table.set(name, { entry: graphRow(name, rev, decl.inject, decl.immediately === true), clientPath }) + table.set(name, { + entry: graphRow(name, rev, decl.inject, decl.immediately === true), + clientPath, + stat: { mtimeMs: stat.mtimeMs, size: stat.size }, + }) } return table } diff --git a/packages/host/webserver/tests/web-plugins.spec.ts b/packages/host/webserver/tests/web-plugins.spec.ts index b9efb1c5c9..ed530ab06f 100644 --- a/packages/host/webserver/tests/web-plugins.spec.ts +++ b/packages/host/webserver/tests/web-plugins.spec.ts @@ -147,6 +147,27 @@ describe('createHostWebPluginRegistry', () => { expect(rebuilds).toHaveLength(1) }) + it('watch mode: a write landing during registry construction is still detected (regression: fs.watchFile baseline absorption)', async () => { + // The old fs.watchFile watch captured its comparison baseline with an + // ASYNCHRONOUS first stat; a rewrite in the same tick as construction was + // absorbed into that baseline and never reported (the CI flake). The + // record-baseline poll stats synchronously before hashing, so this exact + // timing must now always notify. + const { deps, root } = makeDeps([{ name: 'watched', pkg: webDecl() }]) + deps.watch = { intervalMs: 20 } + const registry = createHostWebPluginRegistry(deps) + const rebuilds: string[] = [] + registry.onRebuilt(id => rebuilds.push(id)) + // Same tick as construction — inside the old watch's blind window. The + // rewrite deliberately differs in SIZE from the seed: a same-millisecond + // same-size rewrite is invisible to any mtime+size poll by construction + // (coarse filesystem timestamps), which is a stat-polling limit, not the + // regression under test. + writeFileSync(join(root, 'watched', 'lib', 'client.js'), '// same-tick rewritten contents') + await vi.waitFor(() => { expect(rebuilds).toEqual(['watched']) }, { timeout: 5000 }) + registry.dispose() + }) + it('rejects a non-positive or non-integer watch interval at build time', () => { for (const intervalMs of [0, -5, 1.5]) { const { deps } = makeDeps([{ name: 'p', pkg: webDecl() }])