From 11a23e676a12421ff32fa67cd2f39f45c3db20bf Mon Sep 17 00:00:00 2001 From: chenkun Date: Mon, 5 Oct 2026 04:02:07 +0800 Subject: [PATCH] fix: back the file watcher with an mtime poll net (pollIntervalMs, default 5s) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FSEvents can drop events under load — observed 2026-10-04 on macOS CI, where no event arrived within 60 s and the hot reload was missed until restart. The event watcher stays the fast path; an unref'd low-frequency poll (one stat per interval) compares existence+mtime against the last applied signature and schedules the debounced reload on divergence, so a dropped event degrades to a few seconds' delay. - New pollIntervalMs option (default 5000, 0 disables, gated by watch). - Signature taken BEFORE the read: a mid-read write then records the older signature, yielding one extra diffed reload instead of a miss. - The tick skips while a debounce is pending or a reload is in flight — re-arming would starve the 300 ms debounce (poll interval < debounce) and pin the event loop forever. - Poll path is covered by a test that stubs fs.watch via t.mock.module; the unit runner now passes --experimental-test-module-mocks (available since node 22.3, still gated in 24). Mocking node:fs must enumerate the used exports — spreading the whole namespace dies on the getter-only 'constants' export. - READMEs document the option; CHANGELOG [Unreleased] updated. --- CHANGELOG.md | 6 ++++ README.md | 6 +++- README.zh-CN.md | 5 +++- index.ts | 67 ++++++++++++++++++++++++++++++++++++++++++++ test/index.test.ts | 40 ++++++++++++++++++++++++++ test/options.test.ts | 12 ++++++++ test/run.mjs | 14 +++++++-- 7 files changed, 145 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6696e4d..551c146 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ fixes, minor for features. ### Fixed +- Hot reload could permanently miss an edit when the OS dropped the + file-watch event — observed with FSEvents on loaded macOS CI runners, + where no event arrived within 60 s. The event watcher is now backed by a + low-frequency mtime poll (default 5 s; new `pollIntervalMs` option), so a + dropped event degrades to a few seconds' delay instead of a missed reload + until restart. - Unit tests are now hermetic on machines that export ambient `XDG_*`/`OPENCODE_*` variables: the test entry point strips them, and the inline-content rescan test pins its scan input. diff --git a/README.md b/README.md index 33257e4..2798cf1 100644 --- a/README.md +++ b/README.md @@ -146,6 +146,7 @@ works on both V1 and V2 hosts: | `override` | `boolean` | `false` | Overwrite variables that already exist in the real environment. By default the real environment always wins. | | `required` | `string[]` | `[]` | Variables that must exist after loading. A warning is logged for each missing one. | | `watch` | `boolean` | `true` | Watch the secrets file and hot-reload `process.env` when it changes (see below). | +| `pollIntervalMs` | `number` | `5000` | Interval of the mtime poll backing the file watcher (only with `watch` on). OS watch events are the fast path; the poll heals dropped events — FSEvents can drop them under load, which would otherwise miss the reload entirely. One stat per interval; `0` disables the net (not recommended). | | `mcpReconnect` | `boolean \| "all" \| string[]` | `true` | After a hot reload, reconnect MCP servers so they pick up new values. `true` = only servers whose config references a changed variable (precise), `"all"` = every enabled server, `["name"]` = only those servers, `false` = never. | | `quiet` | `boolean` | `false` | Silence info/debug messages (warnings are always shown). | | `debug` | `boolean` | `false` | Also log the *names* of applied/skipped keys. Values are never logged. | @@ -153,7 +154,10 @@ works on both V1 and V2 hosts: ## Hot reload When `watch` is enabled (the default), editing `secrets.env` takes effect -within about a second — no `opencode service restart` needed: +within about a second — no `opencode service restart` needed. The watcher is +event-driven, backed by a low-frequency mtime poll (default 5 s): if the OS +drops a watch event — FSEvents can, under load — the change still lands +within one poll interval instead of being missed until the next restart: - **Added keys** are injected into the running service's `process.env`. - **Changed keys** are updated in place — but only keys the plugin itself diff --git a/README.zh-CN.md b/README.zh-CN.md index 95a8ca3..a41844d 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -141,6 +141,7 @@ EMPTY= | `override` | `boolean` | `false` | 覆盖真实环境中已存在的变量。默认真实环境优先。 | | `required` | `string[]` | `[]` | 加载后必须存在的变量名，每个缺失的变量都会记录一条警告。 | | `watch` | `boolean` | `true` | 监听密钥文件变化并热更新 `process.env`（见下文）。 | +| `pollIntervalMs` | `number` | `5000` | 文件监听器兜底的 mtime 轮询间隔（仅 `watch` 开启时生效）。操作系统事件是快速路径；轮询负责治愈丢失的事件——FSEvents 在高负载下会丢事件，否则这次热更新将被彻底错过，直到重启。每个间隔一次 stat；`0` 关闭兜底（不推荐）。 | | `mcpReconnect` | `boolean \| "all" \| string[]` | `true` | 热更新后重连 MCP 服务器使其拿到新值。`true` = 仅重连配置中引用了变化变量的服务器（精确按需），`"all"` = 所有启用的服务器，`["名字"]` = 仅指定服务器，`false` = 从不重连。 | | `quiet` | `boolean` | `false` | 静默 info/debug 日志（警告始终输出）。 | | `debug` | `boolean` | `false` | 额外记录注入/跳过的**键名**（值永不记录）。 | @@ -148,7 +149,9 @@ EMPTY= ## 热更新 `watch` 开启时（默认），编辑 `secrets.env` 后约 1 秒内生效，无需 -`opencode service restart`： +`opencode service restart`。监听器是事件驱动的，并配有低频 mtime 轮询 +兜底（默认 5 秒）：如果操作系统丢了监听事件（FSEvents 在高负载下会丢）， +改动仍会在一个轮询间隔内落地，而不是被错过直到下次重启： - **新增的 key** 注入运行中服务的 `process.env`； - **修改的 key** 原地更新 —— 但只更新插件自己注入的 key，来自真实 shell diff --git a/index.ts b/index.ts index 6741146..041c693 100644 --- a/index.ts +++ b/index.ts @@ -19,6 +19,8 @@ export interface NormalizedOptions { path?: string override: boolean watch: boolean + /** mtime poll safety net backing the event watcher (gated by `watch`); 0 disables. */ + pollIntervalMs: number /** true = servers referencing changed keys, "all" = every enabled server, string[] = only these, false = none. */ mcpReconnect: McpReconnectOption quiet: boolean @@ -37,6 +39,12 @@ export function normalizeOptions(raw: Record | undefined): Norm path: typeof options.path === "string" && options.path.trim().length > 0 ? options.path.trim() : undefined, override: options.override === true, watch: options.watch !== false, + pollIntervalMs: + typeof options.pollIntervalMs === "number" && + Number.isFinite(options.pollIntervalMs) && + options.pollIntervalMs >= 0 + ? Math.floor(options.pollIntervalMs) + : 5000, mcpReconnect: Array.isArray(options.mcpReconnect) ? options.mcpReconnect.filter((name): name is string => typeof name === "string" && name.length > 0) : options.mcpReconnect === "all" @@ -132,6 +140,9 @@ interface FileState { instances: Set watcher?: FSWatcher timer?: ReturnType + pollTimer?: ReturnType + /** File signature as of the last successful apply — the poll net's baseline. */ + applied?: { exists: boolean; mtimeMs: number } busy: boolean queued: boolean } @@ -151,6 +162,20 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)) } +/** + * What the poll net compares: existence + mtime. Callers take the signature + * BEFORE reading so a write landing mid-read records the older signature — + * the next poll tick then schedules one extra (harmless, diffed) reload + * instead of missing the update. + */ +function fileSignature(file: string): { exists: boolean; mtimeMs: number } { + try { + return { exists: true, mtimeMs: statSync(file).mtimeMs } + } catch { + return { exists: false, mtimeMs: 0 } + } +} + // --------------------------------------------------------------------------- // Loading & reporting // --------------------------------------------------------------------------- @@ -196,6 +221,7 @@ function checkRequired(instance: Instance): void { function loadOnce(instance: Instance, context: string): ReturnType { const { file, options, emit } = instance const state = stateFor(file) + const signature = fileSignature(file) const read = readSecrets(file) if (read.error) { @@ -205,6 +231,7 @@ function loadOnce(instance: Instance, context: string): ReturnType { + // A pending debounce or an in-flight reload already covers the latest + // file content — re-arming here would starve the debounce whenever the + // poll interval is shorter than the debounce (and keep the event loop + // alive forever). + if (state.timer !== undefined || state.busy) return + const signature = fileSignature(instance.file) + const applied = state.applied + if (applied === undefined) { + // Never applied successfully (e.g. a read error at startup): take the + // current signature as the baseline and retry once per observed + // signature, so an unreadable file warns at most once per change. + state.applied = signature + if (signature.exists) scheduleReload(state, instance.file) + return + } + if (applied.exists === signature.exists && applied.mtimeMs === signature.mtimeMs) return + scheduleReload(state, instance.file) + }, interval) + timer.unref() + state.pollTimer = timer +} + function ensureWatcher(instance: Instance): void { if (!instance.options.watch) return const state = stateFor(instance.file) @@ -408,6 +473,7 @@ function dropWatcherAndStore(file: string): void { const state = files.get(file) if (!state || state.instances.size > 0) return if (state.timer) clearTimeout(state.timer) + if (state.pollTimer) clearInterval(state.pollTimer) state.watcher?.close() const released = state.store.release() if (released.length > 0) { @@ -479,6 +545,7 @@ function activate(options: NormalizedOptions, directory: string, mcp?: McpLike): // Watch before the initial load so an edit landing between the two is not // missed (the debounced reload picks it up). ensureWatcher(instance) + ensurePollNet(instance) loadOnce(instance, "startup") const envRefTransform = registerEnvRefTransform(instance) diff --git a/test/index.test.ts b/test/index.test.ts index ab34843..debeaa9 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -88,6 +88,46 @@ test("server() hot-reloads edits and withdrawals when the file changes", async ( }) }) +test("server() poll net applies edits even when fs.watch drops events", async (t) => { + // FSEvents can drop events under load (observed on macOS CI, 2026-10-04 — + // no event within 60 s). With the event path stubbed out, only the mtime + // poll net can deliver this reload. + const realFs = await import("node:fs") + // Spreading all of realFs fails ("Cannot redefine property: constants" — + // node:fs has getter-only exports), so provide exactly the exports the + // plugin chain (index.ts / env.ts / rawconfig.ts / @opencode/plugin) + // imports, overriding only `watch`. `namedExports` is deprecated in node + // 24 in favor of `exports`, but the latter does not exist on the 22.18 CI + // floor — keep the compatible spelling. + const { appendFileSync, existsSync, mkdirSync, readFileSync, readdirSync, renameSync, rmSync, statSync } = realFs + t.mock.module("node:fs", { + cache: false, + namedExports: { + appendFileSync, + existsSync, + mkdirSync, + readFileSync, + readdirSync, + renameSync, + rmSync, + statSync, + watch: () => ({ close() {}, on() {} }), + }, + }) + const fresh = await import("../index.ts") + await withTempDir(async (dir) => { + const file = join(dir, "secrets.env") + writeFileSync(file, `${KEY}=one\n`, { mode: 0o600 }) + const hooks = await fresh.default.server({ directory: dir }, { path: file, quiet: true, pollIntervalMs: 150 }) + assert.equal(process.env[KEY], "one") + + writeFileSync(file, `${KEY}=two\n`, { mode: 0o600 }) + await waitFor(() => process.env[KEY] === "two") + + await hooks.dispose() + }) +}) + test("server() picks up a secrets file created after startup", async () => { await withTempDir(async (dir) => { const file = join(dir, "secrets.env") diff --git a/test/options.test.ts b/test/options.test.ts index 01d1bb9..688ad47 100644 --- a/test/options.test.ts +++ b/test/options.test.ts @@ -7,6 +7,7 @@ test("normalizeOptions applies defaults for missing input", () => { path: undefined, override: false, watch: true, + pollIntervalMs: 5000, mcpReconnect: true, quiet: false, debug: false, @@ -21,6 +22,7 @@ test("normalizeOptions passes through well-typed values", () => { path: "~/secrets/work.env", override: true, watch: false, + pollIntervalMs: 2500, mcpReconnect: "all", quiet: true, debug: true, @@ -30,6 +32,7 @@ test("normalizeOptions passes through well-typed values", () => { path: "~/secrets/work.env", override: true, watch: false, + pollIntervalMs: 2500, mcpReconnect: "all", quiet: true, debug: true, @@ -43,6 +46,7 @@ test("normalizeOptions falls back to defaults on wrongly typed values", () => { path: 42, override: "yes", watch: 0, + pollIntervalMs: "fast", quiet: 1, debug: "true", required: "A", @@ -50,9 +54,17 @@ test("normalizeOptions falls back to defaults on wrongly typed values", () => { assert.equal(options.path, undefined) assert.equal(options.override, false) assert.equal(options.watch, true) + assert.equal(options.pollIntervalMs, 5000) assert.equal(options.quiet, false) assert.equal(options.debug, false) assert.deepEqual(options.required, []) + // Negative / non-finite intervals also fall back to the default. + assert.equal(normalizeOptions({ pollIntervalMs: -1 }).pollIntervalMs, 5000) + assert.equal(normalizeOptions({ pollIntervalMs: Number.NaN }).pollIntervalMs, 5000) + // Zero is meaningful: it disables the poll net. + assert.equal(normalizeOptions({ pollIntervalMs: 0 }).pollIntervalMs, 0) + // Fractional intervals are floored. + assert.equal(normalizeOptions({ pollIntervalMs: 1500.9 }).pollIntervalMs, 1500) }) test("normalizeOptions treats an empty path as unset", () => { diff --git a/test/run.mjs b/test/run.mjs index 5dde9eb..8e10eff 100644 --- a/test/run.mjs +++ b/test/run.mjs @@ -32,8 +32,16 @@ const defaultFiles = [ "test/changelog.test.ts", ] const files = process.argv.slice(2) -const result = spawnSync(process.execPath, ["--test", ...(files.length > 0 ? files : defaultFiles)], { - stdio: "inherit", -}) +// --experimental-test-module-mocks: enables t.mock.module (node:test module +// mocking), used by the poll-net test in index.test.ts to stub fs.watch. +// Available since 22.3, still gated in 24 — so it is required on the whole +// supported range. Only affects the unit suite. +const result = spawnSync( + process.execPath, + ["--experimental-test-module-mocks", "--test", ...(files.length > 0 ? files : defaultFiles)], + { + stdio: "inherit", + }, +) if (result.error) throw result.error process.exit(result.status ?? 1)