From 55af920defaa3b5f845a91dab8c2b3d464e4801d Mon Sep 17 00:00:00 2001 From: Turtle Date: Wed, 22 Jul 2026 17:36:57 +0800 Subject: [PATCH] fix(vendor/include): keep config reloads resilient --- ...-20-config-hot-reload-resilience.i18n.yaml | 6 + ...2026-07-20-config-hot-reload-resilience.md | 38 ++++++ ...6-07-20-config-hot-reload-resilience.zh.md | 38 ++++++ ...26-07-20-dsh-cli-personal-config.i18n.yaml | 4 +- .../2026-07-20-dsh-cli-personal-config.md | 2 + .../2026-07-20-dsh-cli-personal-config.zh.md | 2 + .../2026-07-21-tui-reload-command.i18n.yaml | 4 +- .../feature/2026-07-21-tui-reload-command.md | 6 +- .../2026-07-21-tui-reload-command.zh.md | 6 +- .../ui/app-boot/tests/config-reload.spec.ts | 128 ++++++++++++++++++ vendor/README.md | 1 + vendor/include/src/index.ts | 63 +++++++-- 12 files changed, 274 insertions(+), 24 deletions(-) create mode 100644 .agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.i18n.yaml create mode 100644 .agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.md create mode 100644 .agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.zh.md create mode 100644 packages/ui/app-boot/tests/config-reload.spec.ts diff --git a/.agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.i18n.yaml new file mode 100644 index 0000000000..b16ef70d7c --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# 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-20-config-hot-reload-resilience.md: 1a8e29c603ede50b60199e9151fca58dadcc3d40 +2026-07-20-config-hot-reload-resilience.zh.md: 6c7a421bfa84504a36d5329e13a485bf72cc6b6c diff --git a/.agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.md b/.agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.md new file mode 100644 index 0000000000..1a8e29c603 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.md @@ -0,0 +1,38 @@ +# Agent Note: A config hot-reload must not kill or degrade a live app + +Status: implemented + +English | [中文](2026-07-20-config-hot-reload-resilience.zh.md) + +## Problem + +The demo apps mount `@cordisjs/plugin-hmr` as a leaf so a running agent picks up `cordis.yml` edits. One bad edit killed the process: `Include.refresh()` rethrew the YAML parse error, the HMR watcher awaits `refresh()` inside an async chokidar callback nobody catches, and the resulting unhandled rejection tripped `dsh-app-boot`'s fail-loud handler — `exit(1)` mid-session, losing the live TUI. Two adjacent defects made even *valid* reloads wrong: a file that parses to `undefined` (empty or mid-write truncated — editors and `sed -i` routinely produce these states) crashed the entry walk instead of reading as invalid, and a re-read never re-applied the include's `config.patches`, so any hot-reload of an overlay-based tree (Code Mode, personal overlays) silently reverted patched entries and removed inserted ones. + +## Decision + +Harden the vendored `@cordisjs/plugin-include` (logged as local modification 8 in [vendor/README.md](../../../../vendor/README.md)) rather than the callers: + +- `refresh()` awaits the whole read-and-update and catches failures, logs a warning, and keeps the last good entry tree. A hot-reload is advisory; the invariant is that no file state reachable by an editor may take the process down. +- `read()` rejects a non-array parse result with a `TypeError`, folding the `undefined`-parse case into the same "invalid file" signal, and commits `content`/`data` only after a successful parse — so reverting an edit to the exact last good content correctly reads as "unchanged". +- `refresh()` and the `internal/update` listener apply `this.applyPatches(...)` before `root.update()`, restoring parity with `[Service.init]`. `applyPatches` deep-copies the cached parse (`structuredClone`) instead of mutating it, so repeated application converges and removing a patch reverts to the file's own values. The listener uses the incoming config's `patches` and persists that config itself: it vetoes the fiber restart (children update in place), and `Fiber.update` only assigns `this.config` behind `next()`, so without the explicit assignment the next re-read would re-apply the old overlay. + +Boot-time behavior stays fail-loud and gets a sharper diagnostic: `[Service.init]` falls back to `initial` (or "config file not found") only on `ENOENT`; an existing-but-invalid file now fails with its real parse error instead of being mislabelled as absent or silently overwritten by `initial`. + +## Alternatives considered + +**Catch in the HMR watcher callback instead of `refresh()`.** Rejected: it would leave `refresh()` a trap for every other caller (the `internal/update` path shares the same tree-update logic), and it cannot fix the `undefined`-parse or patch-loss defects, which live inside the include. + +**Filter config-file rejections in `installFailLoud`.** Rejected: the fail-loud handler exists to make late load failures visible; teaching it to classify exceptions by origin would silently swallow genuine boot failures and leave the stale-`data` crash in place. + +**A PTY e2e proving the TUI survives a bad edit.** Rejected as the primary gate: the PTY smoke reads the repo's committed `cordis.yml`, so corrupting it in-place is not test-safe, and a temp copy cannot resolve the tree's bare package specifiers. The unit spec drives the exact `refresh()` entry point the watcher calls; the fix was additionally verified manually against the live TUI (bad YAML, empty file, restored file). + +## Consequences + +- A bad `cordis.yml` edit now logs `ignoring config reload at ` and the agent keeps running on the last good tree; the next valid edit applies normally. With no logger exporter mounted in the TUI demos the warning is currently invisible on screen — surfacing loader warnings in the TUI is deferred. +- Overlay trees survive base-file reloads with patches intact instead of silently reverting to the unpatched base. +- The vendored include diverges further from upstream; the divergence is logged in the vendor manifest and re-applies on the next sync. +- Known gap, out of scope here: the HMR watcher only handles chokidar `change` events, so editors that replace the file by rename (BSD `sed -i`, `git checkout`) do not trigger a config reload at all; and a reloaded app-entry config does not visibly restart the running TUI (pre-existing on the unmodified tree). + +## Testing + +`packages/ui/app-boot/tests/config-reload.spec.ts` boots real Loader trees against temp configs and pins: an invalid-YAML edit and an empty-file edit both resolve `refresh()` without rejection and keep the previous entry config; a subsequent valid edit applies; an overlay tree re-applies both entry patches and inserted entries on re-read; a hot-update of the include entry's own `patches` applies immediately, survives the next file re-read, and reverts cleanly when the patches are removed. The assertions fail on the unpatched vendored include. diff --git a/.agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.zh.md b/.agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.zh.md new file mode 100644 index 0000000000..6c7a421bfa --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-07-20-config-hot-reload-resilience.zh.md @@ -0,0 +1,38 @@ +# Agent Note: 配置热重载不得杀死或降级正在运行的应用 + +Status: implemented + +[English](2026-07-20-config-hot-reload-resilience.md) | 中文 + +## Problem + +各示例应用把 `@cordisjs/plugin-hmr` 挂载为叶子配置项，让运行中的 agent 能感知 `cordis.yml` 的编辑。一次错误的编辑就会杀死进程：`Include.refresh()` 把 YAML 解析错误原样抛出，HMR 的文件监听器在一个无人捕获的异步 chokidar 回调里 await `refresh()`，产生的未处理 rejection 触发 `dsh-app-boot` 的快速失败处理器——会话中途 `exit(1)`，正在运行的 TUI 就此丢失。另有两个相邻缺陷让*合法*的重载也出错：解析结果为 `undefined` 的文件（空文件或写入中途被截断的文件——编辑器和 `sed -i` 常态性地产生这类中间状态）会让配置项遍历直接崩溃，而不是被判定为无效文件；并且重新读取时从不重新应用 include 的 `config.patches`，因此对基于 overlay 的配置树（Code Mode、个人 overlay）做任何热重载，都会悄悄把打过补丁的配置项回退、并把插入的配置项移除。 + +## Decision + +加固 vendor 的 `@cordisjs/plugin-include`（在 [vendor/README.md](../../../../vendor/README.md) 中记录为本地修改第 8 条），而不是修改调用方： + +- `refresh()` await 整个「读取并更新」过程并捕获失败，记录一条警告，并保留上一份完好的配置树。热重载是尽力而为的；不变式是编辑器可能产生的任何文件状态都不得导致进程退出。 +- `read()` 对非数组的解析结果抛出 `TypeError`，把 `undefined` 解析结果并入同一个「无效文件」信号，并且只在解析成功后才提交 `content`/`data`——因此把编辑撤销回与上一份完好内容完全一致时，会正确地判定为「无变化」。 +- `refresh()` 与 `internal/update` 监听器在 `root.update()` 之前调用 `this.applyPatches(...)`，与 `[Service.init]` 保持一致。`applyPatches` 对缓存的解析结果做深拷贝（`structuredClone`）而不是就地修改，因此重复应用会收敛，移除补丁会回退到文件自身的值。监听器使用传入配置中的 `patches` 并自行持久化该配置：它否决 fiber 重启（子配置项就地更新），而 `Fiber.update` 只在 `next()` 之后才赋值 `this.config`，若不显式赋值，下一次重新读取会重新应用旧的 overlay。 + +启动期行为保持快速失败并获得更准确的诊断：`[Service.init]` 只在 `ENOENT` 时回退到 `initial`（或「config file not found」）；存在但无效的文件现在会以真实的解析错误失败，而不是被误标为文件缺失、或被 `initial` 静默覆盖。 + +## Alternatives considered + +**在 HMR 监听回调里捕获，而不是在 `refresh()` 里。** 否决：这会让 `refresh()` 继续成为其他所有调用方的陷阱（`internal/update` 路径共享同一套树更新逻辑），而且无法修复 `undefined` 解析结果与补丁丢失这两个位于 include 内部的缺陷。 + +**在 `installFailLoud` 里过滤配置文件相关的 rejection。** 否决：快速失败处理器的存在意义就是让延迟出现的加载失败可见；教它按来源给异常分类会悄悄吞掉真正的启动失败，并且原样保留陈旧 `data` 导致的崩溃。 + +**用 PTY e2e 证明 TUI 能在错误编辑后存活。** 否决其作为主要门禁：PTY 冒烟测试读取仓库中已提交的 `cordis.yml`，就地破坏它对测试不安全，而临时副本无法解析该配置树的裸包说明符。单元测试直接驱动监听器所调用的 `refresh()` 入口；此外还对运行中的 TUI 做了人工验证（错误 YAML、空文件、恢复文件）。 + +## Consequences + +- 现在错误的 `cordis.yml` 编辑会记录 `ignoring config reload at `，agent 继续运行在上一份完好的配置树上；下一次合法编辑正常生效。TUI 示例没有挂载任何日志导出器，这条警告目前不会显示在屏幕上——在 TUI 中呈现 loader 警告的工作暂缓。 +- overlay 配置树在基础文件重载后补丁保持完整，不再悄悄回退到未打补丁的基础配置。 +- vendor 的 include 与上游进一步分叉；该分叉已记录在 vendor 的 manifest 里，下次同步时重新应用。 +- 已知缺口，不在本次范围内：HMR 监听器只处理 chokidar 的 `change` 事件，因此通过重命名替换文件的编辑方式（BSD `sed -i`、`git checkout`）完全不会触发配置重载；应用配置项重载后也不会可见地重启运行中的 TUI（未修改的代码树上即已如此）。 + +## Testing + +`packages/ui/app-boot/tests/config-reload.spec.ts` 用真实 Loader 树加载临时配置并固定以下行为：无效 YAML 编辑和空文件编辑都让 `refresh()` 正常 resolve 而不产生 rejection，并保留之前的配置项配置；随后的合法编辑正常生效；overlay 配置树在重新读取时重新应用配置项补丁和插入的配置项；对 include 配置项自身 `patches` 的热更新立即生效、在下一次文件重读后依然保持、并在补丁移除后干净地回退。这些断言在未打补丁的 vendor include 上会失败。 diff --git a/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.i18n.yaml b/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.i18n.yaml index a8c9c28d58..7addc991d2 100644 --- a/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.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-20-dsh-cli-personal-config.md: e349374a6bc7fc0137bf14836469aef8bae8d49d -2026-07-20-dsh-cli-personal-config.zh.md: 88210dc386a245002de927950dab2852e40218ea +2026-07-20-dsh-cli-personal-config.md: 514bb5b12a3e04c7deaad1e8616472eed1c920e1 +2026-07-20-dsh-cli-personal-config.zh.md: 16fada82c59c8a356e6df112234e6b7565aae1bf diff --git a/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.md b/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.md index e349374a6b..514bb5b12a 100644 --- a/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.md +++ b/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.md @@ -22,6 +22,8 @@ Two coupled pieces, aligned with the `apps/` assembly tier proposed by the `dsh The PTY smoke's launcher isolates `$DSH_HOME` to a per-test directory, exactly as it already isolates `DSH_AGENTS_HOME`, so a developer's real personal overlay cannot leak into fixtures; only the dsh CLI reads personal config, so no other test launcher needed changes. +Hot-reload interplay: the include re-applies its `patches` on every config re-read (the [config hot-reload resilience Agent Note](../bug-fix/2026-07-20-config-hot-reload-resilience.md)), so a live `cordis.yml` edit keeps the personal overlay applied. + ## Alternatives considered **A standalone `bin/dsh` wrapper owning the `dsh` name.** Rejected after reading PR #443: that PR establishes `apps/cli` as the `dsh` CLI with subcommand dispatch (`web`, `-p`) and leaves the default slot unclaimed. Two competing `dsh` entrypoints would collide in `$PATH` and in product identity; claiming the default slot inside the same package shape confines the eventual merge conflict to the small dispatch chain. diff --git a/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.zh.md b/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.zh.md index 88210dc386..16fada82c5 100644 --- a/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.zh.md +++ b/.agents/notes/implemented/feature/2026-07-20-dsh-cli-personal-config.zh.md @@ -22,6 +22,8 @@ Status: implemented PTY 冒烟测试的启动器把 `$DSH_HOME` 隔离到每个测试自己的目录，与它已有的 `DSH_AGENTS_HOME` 隔离方式完全一致，开发者真实的个人 overlay 不可能泄漏进 fixture；只有 dsh CLI 读取个人配置，因此其他测试启动器无需改动。 +与热重载的交互：include 在每次配置重读时重新应用其 `patches`（见[配置热重载韧性 Agent Note](../bug-fix/2026-07-20-config-hot-reload-resilience.md)），因此运行中编辑 `cordis.yml` 后个人 overlay 仍保持生效。 + ## Alternatives considered **独立的 `bin/dsh` 包装脚本占有 `dsh` 这个名字。** 读过 PR #443 后否决：该 PR 把 `apps/cli` 确立为带子命令分发（`web`、`-p`）的 `dsh` CLI，并且默认位空缺。两个互相竞争的 `dsh` 入口会在 `$PATH` 和产品身份上冲突；在同一包形态内认领默认位，把最终的合并冲突限制在小小的分发链上。 diff --git a/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.i18n.yaml b/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.i18n.yaml index 321131ac96..f3fba8a006 100644 --- a/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.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-21-tui-reload-command.md: de9a5502214a610d88024730b1c0c1044a396c92 -2026-07-21-tui-reload-command.zh.md: 25d1d448459221698ca63377f8f18d05a0fa3d21 +2026-07-21-tui-reload-command.md: e5600f0ab5cd82dc556df76006fcf532d8c7d302 +2026-07-21-tui-reload-command.zh.md: 3798b0518df1c379cca808bd4af38490016567cb diff --git a/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.md b/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.md index de9a550221..e5600f0ab5 100644 --- a/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.md +++ b/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.md @@ -10,7 +10,7 @@ HMR's file watcher only reacts to in-place `change` events under its configured ## Decision -`dsh-tui` gains an **experimental, dev-only** `/reload` slash command: it walks `ctx.loader.entries()` and calls `refresh()` on every file-backed subtree (`Include`), i.e. the exact code path the HMR watcher's config-change branch drives, invoked manually and watcher-independent. Unchanged files are no-ops (content comparison in `Include.read`). +`dsh-tui` gains an **experimental, dev-only** `/reload` slash command: it walks `ctx.loader.entries()` and calls `refresh()` on every file-backed subtree (`Include`), i.e. the exact code path the HMR watcher's config-change branch drives, invoked manually and watcher-independent. Unchanged files are no-ops (content comparison in `Include.read`); invalid files warn and keep the running tree (the hot-reload-resilience contract); include `patches` — including the dsh CLI's personal overlay — re-apply on every re-read. The TUI reaches the Loader **structurally** (`ctx.loader` via a local type, not `inject`): tests and embedders run the TUI without a Loader, where `/reload` degrades to a warning notice instead of failing the mount. Module-source hot reload stays watcher-owned; `/reload` refreshes configs only. @@ -28,8 +28,8 @@ The TUI reaches the Loader **structurally** (`ctx.loader` via a local type, not - The command reports tree count and completion as transcript notices; per-file failures surface only in loader logs, which the TUI does not display — acceptable for a dev-only surface, noted in the completion message. - A re-entrancy guard serializes reloads: `/reload` while one is in flight is refused with a warning, keeping the loader's unmutexed tree-update pass single-writer; the guard releases on completion or failure. - `/reload` runs only while the agent is idle: a reload can dispose and re-mount entries, which under an active turn could tear tools or the adapter out from under in-flight calls. The check is advisory (a send can race in after it) but removes the common footgun. -- If any `refresh()` rejects, the command reports the failure instead of leaving an unhandled rejection. +- If `refresh()`'s never-reject contract ever changes, the command reports the failure instead of leaving an unhandled rejection. ## Testing -`packages/ui/tui/tests/tui.spec.ts` pins: `/reload` refreshes every file-backed subtree and skips plain entries (structural fake Loader), reports completion, refuses re-entry while a gated refresh is in flight and runs again after release, releases the guard on the failure arm, refuses a running agent and runs again at idle, reports a rejecting refresh, and degrades to a warning without a Loader — including mounted as a real plugin fiber, where a throwing service lookup would escape. Verified live in tmux against the real tree: a probe edit reloads successfully. +`packages/ui/tui/tests/tui.spec.ts` pins: `/reload` refreshes every file-backed subtree and skips plain entries (structural fake Loader), reports completion, refuses re-entry while a gated refresh is in flight and runs again after release, releases the guard on the failure arm, refuses a running agent and runs again at idle, reports a rejecting refresh, and degrades to a warning without a Loader — including mounted as a real plugin fiber, where a throwing service lookup would escape. Verified live in tmux against the real tree: probe edit → reload applies; invalid edit → reload keeps the running tree. diff --git a/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.zh.md b/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.zh.md index 25d1d44845..3798b0518d 100644 --- a/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.zh.md +++ b/.agents/notes/implemented/feature/2026-07-21-tui-reload-command.zh.md @@ -10,7 +10,7 @@ HMR 的文件监听器只对其配置根目录（示例中即配置叶子所在 ## Decision -`dsh-tui` 增加一个**实验性、仅供开发**的 `/reload` 斜杠命令：遍历 `ctx.loader.entries()`，对每个文件后端的子树（`Include`）调用 `refresh()`——即 HMR 监听器配置变更分支所走的同一条代码路径，改为手动触发、不依赖监听器。未变化的文件是无操作（`Include.read` 做内容比较）。 +`dsh-tui` 增加一个**实验性、仅供开发**的 `/reload` 斜杠命令：遍历 `ctx.loader.entries()`，对每个文件后端的子树（`Include`）调用 `refresh()`——即 HMR 监听器配置变更分支所走的同一条代码路径，改为手动触发、不依赖监听器。未变化的文件是无操作（`Include.read` 做内容比较）；无效文件记录警告并保留运行中的树（热重载韧性契约）；include 的 `patches`——包括 dsh CLI 的个人 overlay——在每次重读时重新应用。 TUI 以**结构方式**访问 Loader（通过局部类型访问 `ctx.loader`，而非 `inject`）：测试和嵌入方在没有 Loader 的情况下运行 TUI，此时 `/reload` 退化为一条警告通知而不是挂载失败。模块源码热重载仍由监听器负责；`/reload` 只刷新配置。 @@ -28,8 +28,8 @@ TUI 以**结构方式**访问 Loader（通过局部类型访问 `ctx.loader`， - 命令以 transcript 通知报告树数量与完成；单文件失败只出现在 loader 日志里，TUI 不显示——对仅供开发的表面可以接受，完成消息中已注明。 - 重入保护串行化重载：前一次进行中时 `/reload` 会被拒绝并提示警告，使 loader 无互斥的树更新过程保持单写者；保护在完成或失败时释放。 - `/reload` 只在 agent 空闲时运行：重载可能卸载并重新挂载配置项，在活跃轮次下这会把工具或适配器从进行中的调用脚下抽掉。检查是建议性的（检查后仍可能有 send 竞争进来），但消除了常见的坑。 -- 任一 `refresh()` 若 reject，命令会报告失败而不是留下未处理的 rejection。 +- 若 `refresh()` 的永不 reject 契约将来改变，命令会报告失败而不是留下未处理的 rejection。 ## Testing -`packages/ui/tui/tests/tui.spec.ts` 固定：`/reload` 刷新每个文件后端子树并跳过普通配置项（结构化的假 Loader）、报告完成、在门控的刷新进行中拒绝重入并在释放后可再次运行、失败分支同样释放保护、拒绝运行中的 agent 并在空闲后可再次运行、报告 reject 的 refresh、无 Loader 时退化为警告——包括作为真实插件 fiber 挂载的情形，在那里会抛出的服务查找会泄露出去。已在 tmux 中对真实配置树实机验证：探针编辑后 reload 成功生效。 +`packages/ui/tui/tests/tui.spec.ts` 固定：`/reload` 刷新每个文件后端子树并跳过普通配置项（结构化的假 Loader）、报告完成、在门控的刷新进行中拒绝重入并在释放后可再次运行、失败分支同样释放保护、拒绝运行中的 agent 并在空闲后可再次运行、报告 reject 的 refresh、无 Loader 时退化为警告——包括作为真实插件 fiber 挂载的情形，在那里会抛出的服务查找会泄露出去。已在 tmux 中对真实配置树实机验证：探针编辑 → reload 生效；无效编辑 → reload 保留运行中的树。 diff --git a/packages/ui/app-boot/tests/config-reload.spec.ts b/packages/ui/app-boot/tests/config-reload.spec.ts new file mode 100644 index 0000000000..05ebe80b58 --- /dev/null +++ b/packages/ui/app-boot/tests/config-reload.spec.ts @@ -0,0 +1,128 @@ +/** + * Config hot-reload resilience of the booted include tree. `dsh-app-boot` + * installs a fail-loud unhandled-rejection handler, so a `refresh()` that + * rethrows a config-file parse error would kill a live app on one bad + * `cordis.yml` edit (the HMR watcher awaits `refresh()` in an async event + * callback nobody else catches). These tests pin the vendored + * `@cordisjs/plugin-include` contract that boot relies on: an invalid file + * keeps the last good tree, and a valid re-read re-applies overlay patches + * exactly like the initial load. + */ + +import { mkdtempSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import type { Context } from 'cordis' +import type { Include } from '@cordisjs/plugin-include' +import { boot } from '../src/index.ts' + +const NAME = 'dsh-test-bin' + +const NOOP_PLUGIN = 'export const name = "noop"\nexport function apply() {}\n' + +interface TreeFixture { + ctx: Context + dir: string + include: Include +} + +async function bootTree(configBody: string): Promise { + const dir = mkdtempSync(join(tmpdir(), 'dsh-config-reload-')) + writeFileSync(join(dir, 'noop.mjs'), NOOP_PLUGIN) + writeFileSync(join(dir, 'cordis.yml'), configBody) + const ctx = await boot(NAME, join(dir, 'cordis.yml')) + const entry = [...ctx.loader.entries()].find(candidate => candidate.subtree !== undefined) + if (entry?.subtree === undefined) throw new Error('booted tree has no include entry') + return { ctx, dir, include: entry.subtree as Include } +} + +function entryConfig(ctx: Context, id: string): unknown { + return [...ctx.loader.entries()].find(entry => entry.options.id === id)?.options.config +} + +describe('include refresh with an invalid file', () => { + it('keeps the last good tree instead of throwing, then applies the next valid edit', async () => { + const { ctx, dir, include } = await bootTree('- id: noop\n name: ./noop.mjs\n config:\n value: 1\n') + try { + expect(entryConfig(ctx, 'noop')).toEqual({ value: 1 }) + + writeFileSync(join(dir, 'cordis.yml'), 'invalid: [unclosed\n') + await expect(include.refresh()).resolves.toBeUndefined() + expect(entryConfig(ctx, 'noop')).toEqual({ value: 1 }) + + // An empty file parses to `undefined` without a YAML error; it must be + // treated exactly like a parse failure, not crash the entry walk. + writeFileSync(join(dir, 'cordis.yml'), '') + await expect(include.refresh()).resolves.toBeUndefined() + expect(entryConfig(ctx, 'noop')).toEqual({ value: 1 }) + + writeFileSync(join(dir, 'cordis.yml'), '- id: noop\n name: ./noop.mjs\n config:\n value: 2\n') + await include.refresh() + await ctx.loader.await() + expect(entryConfig(ctx, 'noop')).toEqual({ value: 2 }) + } finally { + await ctx.fiber.dispose() + } + }) +}) + +describe('include refresh with overlay patches', () => { + it('re-applies entry patches and inserted entries on every re-read (parity with initial load)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'dsh-config-reload-overlay-')) + writeFileSync(join(dir, 'noop.mjs'), NOOP_PLUGIN) + writeFileSync(join(dir, 'base.yml'), '- id: noop\n name: ./noop.mjs\n config:\n value: base\n') + writeFileSync(join(dir, 'cordis.yml'), [ + '- id: base', + " name: 'cordis:include'", + ' config:', + ' path: ./base.yml', + ' patches:', + ' - id: noop', + ' name: ./noop.mjs', + ' config:', + ' value: patched', + ' - insert:', + ' - id: extra', + ' name: ./noop.mjs', + '', + ].join('\n')) + const ctx = await boot(NAME, join(dir, 'cordis.yml')) + try { + const entry = [...ctx.loader.entries()].find(candidate => candidate.options.id === 'base') + if (entry?.subtree === undefined) throw new Error('overlay tree has no base include entry') + const include = entry.subtree as Include + expect(entryConfig(ctx, 'noop')).toEqual({ value: 'patched' }) + expect(entryConfig(ctx, 'extra')).toBeUndefined() + expect([...ctx.loader.entries()].some(candidate => candidate.options.id === 'extra')).toBe(true) + + writeFileSync(join(dir, 'base.yml'), '- id: noop\n name: ./noop.mjs\n config:\n value: edited\n') + await include.refresh() + await ctx.loader.await() + expect(entryConfig(ctx, 'noop')).toEqual({ value: 'patched' }) + expect([...ctx.loader.entries()].some(candidate => candidate.options.id === 'extra')).toBe(true) + + // Hot-update of the include entry's own config (the `internal/update` + // path): the new patches must apply now AND stick for later re-reads — + // the listener vetoes the fiber restart, so it must persist the new + // config itself or the next refresh() re-applies the old overlay. + await entry.update({ config: { path: './base.yml', patches: [{ id: 'noop', name: './noop.mjs', config: { value: 'patched-v2' } }] } }) + await ctx.loader.await() + expect(entryConfig(ctx, 'noop')).toEqual({ value: 'patched-v2' }) + expect([...ctx.loader.entries()].some(candidate => candidate.options.id === 'extra')).toBe(false) + + writeFileSync(join(dir, 'base.yml'), '- id: noop\n name: ./noop.mjs\n config:\n value: edited-2\n') + await include.refresh() + await ctx.loader.await() + expect(entryConfig(ctx, 'noop')).toEqual({ value: 'patched-v2' }) + + // Removing every patch must revert to the file's own values: patching + // may not bake earlier patch results into the cached parse. + await entry.update({ config: { path: './base.yml', patches: [] } }) + await ctx.loader.await() + expect(entryConfig(ctx, 'noop')).toEqual({ value: 'edited-2' }) + } finally { + await ctx.fiber.dispose() + } + }) +}) diff --git a/vendor/README.md b/vendor/README.md index ae43760ceb..1f4d61e6b1 100644 --- a/vendor/README.md +++ b/vendor/README.md @@ -37,6 +37,7 @@ Keep this log exhaustive — every divergence from upstream must be listed. 5. **`schemastery/tsdown.config.ts` and `logger-console/tsdown.config.ts`**: ours, not upstream files — per-package build-shape overrides (dual ESM+CJS output; separate node/browser entries) for the repo-root tsdown build. They read the JS emitted under `lib/types` and then write the publish runtime entries under `lib/`. Like the regenerated tsconfigs, they are not part of the upstream sync surface. 6. **`cordis/src/fiber.ts` lifecycle hardening**: locally closes three reentrant disposal gaps. An effect's owner-list wrapper is registered before its setup body runs, so an unload begun from inside setup awaits setup and every collected cleanup; synchronous setup failure removes the wrapper and rolls back collected cleanup. Async cleanup stays owner-visible until quiescence, and Cordis's internal effect composition joins an already-running cleanup while repeated public disposer calls retain their upstream single-shot result. Effect creation is rejected while the owner is `UNLOADING` (while `PENDING` and `LOADING` remain legal), preventing cleanup-time registrations from escaping the unload snapshot. Child fibers register and receive their parent-owned disposer before `internal/plugin` publication, resolve dependency declarations added by that notification before activation, drain effects attached while pending, skip plugin execution when reentrant disposal invalidates the load epoch before its first checkpoint, and contain teardown-notification failures per observer so one callback cannot starve peers or interrupt ownership cleanup. 7. **`cordis/src/*.ts` JSDoc enrichment**: added `@param`/`@returns` tags and contract documentation (disposal semantics, waterfall veto, bail conditions, error cases) across the public plugin-author surface — `Context` (class, statics, and the `Context` interface properties incl. `root`), `EventsService`, `Fiber`, `RegistryService`, `ReflectService`, `Service`, `LoggerService` and their `declare module './context.ts'` overloads. Comment-only; no code changes. Motivation: the website API-reference generator renders these docs and hard-errors on undocumented members. Retire this entry when the enrichment is upstreamed to the fork. +8. **`include/src/index.ts` hot-reload hardening**: `refresh()` awaits the full read-and-update and catches failures (logging a warning and keeping the last good entry tree) instead of rethrowing — upstream's throw escaped `@cordisjs/plugin-hmr`'s async watcher callback as an unhandled rejection, so one bad `cordis.yml` edit killed a live app. `read()` rejects a non-array parse result (an empty or mid-write truncated file parses to `undefined`, which upstream later crashed on) and commits `content`/`data` only on success, so reverting an edit to the exact last good content reads as "unchanged". `refresh()` and the `internal/update` listener re-apply `config.patches` before `root.update()`, matching initial load; upstream applied patches only in `[Service.init]`, so any config hot-reload silently reverted overlay-patched entries and removed inserted ones. `applyPatches` deep-copies via `structuredClone` instead of mutating the cached parse (repeated application converges; removing a patch reverts), and the veto-style `internal/update` listener persists the incoming config itself (`Fiber.update` only assigns behind `next()`), so later re-reads use the new patches. `[Service.init]` falls back to `initial` only on `ENOENT`; an existing-but-invalid file fails loud with its real parse error instead of "config file not found" (or a silent overwrite). Covered by `packages/ui/app-boot/tests/config-reload.spec.ts`. ## Sync procedure diff --git a/vendor/include/src/index.ts b/vendor/include/src/index.ts index 2258d3af06..b1517d5458 100644 --- a/vendor/include/src/index.ts +++ b/vendor/include/src/index.ts @@ -77,7 +77,15 @@ export class Include extends EntryTree { ctx.on('internal/update', (config, _, next) => { if (config.path !== this.config.path) return next() - this.root.update(this.data!) + // Veto the fiber restart (children update in place), but persist the new + // config ourselves — `Fiber.update` only assigns `this.config` behind + // `next()`, and a stale `this.config.patches` would make the next + // `refresh()` re-apply the old overlay. + this.config = config + this.root.update(this.applyPatches(this.data!, config.patches)).catch((error) => { + this.ctx.logger.warn('config update at %C failed', this.filename) + this.ctx.logger.warn(error) + }) }) } @@ -93,22 +101,37 @@ export class Include extends EntryTree { private async read(forced = false) { const content = await readFile(this.filename, 'utf8') if (!forced && this.content === content) return false - this.content = content + let data: any if (this.type === 'application/yaml') { - this.data = yaml.load(this.content, { schema }) as any + data = yaml.load(content, { schema }) } else if (this.type === 'application/json') { - this.data = JSON.parse(this.content) as any + data = JSON.parse(content) } else { const module = await import(/* @vite-ignore */ this.filename) - this.data = module.default || module + data = module.default || module } + // An empty or truncated file (common mid-edit: editors and `sed -i` write + // through temp states) parses to `undefined`, not an error; reject every + // non-array shape here so callers see one "invalid file" signal. Content + // and data commit only on success, so an edit that is later reverted to + // the exact last good content correctly reads as "unchanged". + if (!Array.isArray(data)) { + throw new TypeError(`config file must be a top-level array of entries: ${this.filename}`) + } + this.content = content + this.data = data await this.checkAccess() return true } - private applyPatches(data: EntryOptions[]): EntryOptions[] { - const { patches } = this.config - if (!patches?.length) return data + private applyPatches(data: EntryOptions[], patches = this.config.patches): EntryOptions[] { + // Always detach from the cached parse: patching shared entry objects would + // bake earlier patch values into `this.data`, so repeated application + // (config hot-reloads) could never revert a removed or changed patch. The + // supported extensions guarantee JSON-safe plain data, so `structuredClone` + // cannot throw here. + if (!patches?.length) return [...data] + data = structuredClone(data) const entryMap = new Map() const buildMap = (entries: EntryOptions[]) => { @@ -174,7 +197,11 @@ export class Include extends EntryTree { async* [Service.init]() { try { await this.read() - } catch { + } catch (error) { + // Only a missing file falls back to `initial` (or the not-found error): + // an existing-but-invalid file must fail loud with its real parse error, + // never be mislabelled as absent or silently overwritten. + if ((error as NodeJS.ErrnoException | null)?.code !== 'ENOENT') throw error if (this.config.initial) { this.writeFile(this.config.initial as any) await this.read() @@ -184,18 +211,26 @@ export class Include extends EntryTree { } yield () => this.stop() - const data = this.applyPatches([...this.data!]) - await this.root.update(data) + await this.root.update(this.applyPatches(this.data!)) } stop() { this.root.stop() } - /** Re-read the file and refresh child entries when content changed. */ + /** + * Re-read the file and refresh child entries when content changed. An + * unreadable or unparsable file logs a warning and keeps the last good + * tree: a hot-reload of a live app must never take the process down. + */ async refresh() { - if (!await this.read()) return - this.root.update(this.data!) + try { + if (!await this.read()) return + await this.root.update(this.applyPatches(this.data!)) + } catch (error) { + this.ctx.logger.warn('config reload at %C failed; keeping the running tree', this.filename) + this.ctx.logger.warn(error) + } } private async _writeFile(config: EntryOptions[]) {