Skip to content

fix(metadata): 坏文件不再被当作 data: null 广播 —— handleFileEvent 改走 loadDiagnosed (#5228) - #5626

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-5228-filewatch-degraded-broadcast
Aug 5, 2026
Merged

fix(metadata): 坏文件不再被当作 data: null 广播 —— handleFileEvent 改走 loadDiagnosed (#5228)#5626
os-zhuang merged 3 commits into
mainfrom
claude/issue-5228-filewatch-degraded-broadcast

Conversation

@os-zhuang

@os-zhuang os-zhuang commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #5228

注:GitHub 的正文消毒器会把裸的单引号与尖括号在存储时转义,下面刻意避开了这两类字符,测试证据段落里的引号一律改用「」,箭头一律用 →,免得关键证据被存成实体后读不出来。

一、前提核验(先于实现)

issue 的前提在合并后的 origin/main(e0b2ea7,已含同日的 #5436)上 仍然成立,三处 file:line 证据:

  • packages/metadata/src/metadata-manager.ts:2196 —— load() 的全部实现就是 return (await this.loadDiagnosed(type, name, options)).data;
  • packages/metadata/src/metadata-manager.ts:2221-2233 —— loadDiagnosed() 的循环体整体包在 try/catch 里:loader 抛出的异常被 errors.push(...) 记账、logger.warn 一条,然后返回 { data: null, degraded: errors.length 非零, errors },不重抛
  • packages/metadata/src/loaders/filesystem-loader.ts:137-144 —— FilesystemLoader.load() 对坏 JSON 确实先 logger.errorthrow error

串起来即 issue 所述:坏文件的异常在 loadDiagnosed 一层就被吃掉,handleFileEventcatch 永不进入、return 永不执行、那句 logger.error(Failed to load changed file) 一次都没打印过,事件带着 data: null 广播出去。而 data: null 恰是「这份元数据合法地什么都没有」的线上形状 —— ADR-0110 D3 要分开的 miss 与 outage,在这个调用点被抹平成同一个形状。

反向验证也把它钉死了:把源码退回改动前的版本,本 PR 新增的三条用例立刻变红(见第四节)。

二、修法(PM 裁决的方向 A)

handleFileEvent 改用 loadDiagnosed(),按 degraded 分三路:

情况 判据 行为
outage degraded === true(有 loader 抛异常且无人答出) 走那条死了的 catch 本来要走的路:logger.error 带上 filePath / metadataType / name / loadDiagnosederrors[],不广播
干净 miss data: null 且未 degraded(文件已不在,或合法为空) 保持现状语义,照旧广播 data: null
deleted 根本不读 照旧广播,data 保持 undefined

data 的静态类型顺手从 any 收到 unknown,与 MetadataWatchEventSchema 里的 data: z.unknown().optional() 对齐。

三、失效动作与广播动作的先后(#5218 不回退)

PM 特别点名的风险:提前 return 不得把缓存失效一起跳过。现状代码里 invalidateForForeignWrite() 排在读之后、广播之前,所以「读完发现 degraded 就 return」的朴素改法会连失效一起吞掉。

因此本 PR 把 invalidateForForeignWrite(type, name) 上移到读之前:失效是无条件的,广播才是有条件的,读的结论再也决定不了缓存掉不掉。这一步是安全的 —— loadDiagnosed 只走 loader,既不读也不写 listCache / registry

这样两件事同时成立:

四、测试与反向验证

新增 packages/metadata/src/node-metadata-manager-degraded-file-event.test.ts(6 例),并订正 #5218 用例上那段已经过时的注释(它当时写的是「事件会带 data: null 广播出去,这是另一个单独的问题」)。

反向验证的方向是事先预判后跑的,而且跑了两个方向,因为两种错法的红集合是不相交的:

方向一 —— 源码退回改动前(预判:两条「不广播」用例 + 日志可达性用例变红,实测一致):

 FAIL  announces nothing, and drops the caches anyway
       → expected [ { type: 「added」, …(5) } ] to deeply equal []
 FAIL  finally reaches the logger.error that never printed once
       → expected [] to have a length of 1 but got +0
 FAIL  a degraded api event still invalidates the endpoint index
       → expected [ { type: 「added」, …(5) } ] to deeply equal []
 Tests  3 failed | 14 passed (17)

干净 miss / 可读文件 / deleted 三例在改动前后都是绿的 —— 它们本就不是判别项,是「没被顺手改坏」的保护栏,如实记在用例注释里。

方向二 —— 朴素改法(先读、degraded 就 return,失效留在广播前),预判:#5218 自己的回归用例会红,实测一致:

 FAIL  #5228 announces nothing, and drops the caches anyway
       → expected [ 「view」 ] to deeply equal []
 FAIL  #5228 a degraded api event still invalidates the endpoint index
       → expected undefined to match object { endpoint: { name: 「list_users」 } }
 FAIL  #5218 still invalidates when the changed file cannot be parsed
       → expected [ 「view」 ] to deeply equal []
 Tests  3 failed | 14 passed (17)

也就是说:只有「失效在前、广播有条件」的实现能让 17 条全绿。注意 #5218 那条用例对本次改动本身不具判别力(改动前后都绿),这一点已写进它的注释里,免得下一个读者误以为它在守 #5228

绿色结果:

pnpm --filter @objectstack/metadata test
  Test Files  23 passed (23)
       Tests  484 passed (484)

pnpm check:type-check-coverage 通过;packages/metadata 的 tsc 原始报错数在改动前后都是 92(即本次净增 0,新增测试文件自身 0 报错);check-nul-bytes 通过,并对四个改动文件做了超出门禁扫描面的自扫(含 DEL),无控制字节。

五、消费者核验

notifyWatchers 的仓内订阅者共 4 处,逐个核对「degraded 不广播」是否让它们的失效/重载逻辑失效:

订阅者 event.data degraded 不广播的影响
metadata-manager.ts:391(api → endpointMatcher.invalidate()) —— 由 invalidateListCache 这条缝覆盖,新增用例直接钉住
objectql/src/plugin.ts:724(object) 否(回头 get() 重读) —— 坏文件重读也是 null,原本就只打一条 debug 后什么都不做
plugin-email/src/email-plugin.ts:919(email_template) 是,event?.data ?? await get(...) —— null 是 nullish,原本就落到 get(),同样拿到 null 后 return
metadata/src/routes/hmr-routes.ts:98 → Studio SSE 否(只转发 type/name/path/timestamp) 有一处行为变化,见下

相关套件全绿:@objectstack/plugin-email 20 files / 297 tests;@objectstack/objectqlplugin.integration / plugin.step2 / metadata-facade / metadata-diagnostics 4 files / 61 tests。

需要维护者知情的那一处行为变化

hmr-routes 把事件当作纯粹的「有东西变了,去重读」信号转给浏览器,它不读 data;而浏览器侧的缓存不在 invalidateForForeignWrite 的覆盖范围内。所以:一个被改坏且一直没修好的文件,现在不再唤醒 Studio,左栏会继续显示上一次的好定义,直到下一个事件或手动刷新;改动前 Studio 会被唤醒、重读、看着这个视图消失。

我判断这不构成 needs_decision,理由三条,但把它显式摆出来供否决:

  1. 这不是正确性回退,是有界且自愈的新鲜度差异 —— 编辑器把文件存完就会再来一个事件;
  2. added + 坏文件这个绝大多数场景下,list() 前后本来就没变,唤醒本身零信息;
  3. 与此同时开发者第一次真正拿到了 logger.error(带 errors[]),被告知「文件读不出来」这件事本身,比看着视图无声消失更接近真相。

若维护者更看重那条唤醒信号,最小改法是 degraded 时仍广播、但带 data: undefined(即 applyRepoEvent 已经在用的那个既有形状,不是新造第三种),约五行;按 PM 裁决与 issue 方向 A,本 PR 没有自行这么做。

`NodeMetadataManager.handleFileEvent()` guarded its re-read with a
`try/catch` that was unreachable for loader read/parse failures: `load()`
is `(await loadDiagnosed(...)).data`, and `loadDiagnosed` (ADR-0110 D3)
absorbs a loader throw into `{ data: null, degraded: true }` instead of
rethrowing. `FilesystemLoader.load()` does throw on an unparseable file,
but the throw died one frame below the handler — so the `catch` never
ran, its `logger.error` never printed once, and the event went out
carrying `data: null`: the wire shape of "this metadata legitimately
holds nothing". The miss/outage distinction ADR-0110 D3 exists to keep
was erased at the one call site using the variant that discards it.

Read through `loadDiagnosed` and split on `degraded`. An outage takes
the road the dead `catch` meant to take (log `filePath`, type, name and
`errors[]`; announce nothing). A clean miss and every `deleted` event
keep their existing semantics.

The invalidation moves ABOVE the read, so the read's verdict can never
decide whether the caches drop — #5218's contract survives the early
return, and the `api` endpoint index still rebuilds through
`invalidateListCache`, its first seam (#5089), without the broadcast.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V7WetGmnfoXNn8cLieKKmx
@vercel

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 5, 2026 9:08pm

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata.

7 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata)
  • content/docs/kernel/cluster.mdx (via packages/metadata)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/metadata)
  • content/docs/plugins/packages.mdx (via @objectstack/metadata)
  • content/docs/protocol/kernel/metadata-service.mdx (via @objectstack/metadata)
  • content/docs/releases/v12.mdx (via @objectstack/metadata)
  • content/docs/releases/v9.mdx (via @objectstack/metadata)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Copy link
Copy Markdown
Contributor Author

CI 红说明(engine-core 车道 PM):ESLint job 唯一失败点是 check:engine-double-contract 的已知 base 签名(action-execution-calldata-not-found.test.ts lines 69/102,#5604 立案、cli 车道在修),job 内其余门(durability、published-files、release 系列等)全绿,与本 PR 的 4 文件 diff 无关。不追此签名;#5604 落地后同步 main 重跑。


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 5, 2026 21:24
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 5, 2026
Merged via the queue into main with commit 2b2175b Aug 5, 2026
24 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-5228-filewatch-degraded-broadcast branch August 5, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

handleFileEvent 的 catch 对 loader 读/解析失败不可达 —— 坏文件被当作 data: null 广播出去

2 participants