Skip to content

fix(metadata): 声明了 capabilities.write 的 datasource loader 必须实现 delete(),注册期响亮拒绝 (#5276) - #5652

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-5276-loader-delete-contract
Aug 5, 2026
Merged

fix(metadata): 声明了 capabilities.write 的 datasource loader 必须实现 delete(),注册期响亮拒绝 (#5276)#5652
os-zhuang merged 2 commits into
mainfrom
claude/issue-5276-loader-delete-contract

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#5276

问题

MetadataLoader 只声明 save?,没有 delete,于是 capabilities.write 在条目生命周期的两端含义分叉:对 register() 意味着「写进来」,对 unregister() 什么也不保证。unregister() 在调用点鸭子类型地探 delete,探不到就一声不吭地跳过——随后照常删 registry、失效 listCache、广播 deleted 事件并 notifyWatchers。调用方(Studio/Setup、REST DELETE、CLI、package teardown)被告知删除成功,行还留在那个 loader 里,下一次 list()/get() 直接读回来,重启后依旧,而且没有任何东西会重试。标准的 declared ≠ enforced(Prime Directive #10)。

改动(按 PM 裁决,方向 1+2 合并)

  1. 契约声明能力MetadataLoader 增加 delete?(type: string, name: string): Promise< void >,与既有 save? 同风格、同位置。unregister() 先失效 listCache 再删 loader:删除落库前到达的并发 list() 会把「已删项」重新缓存满 30s,之后没有任何东西再失效它 #5259 为了给这个鸭子类型的洞起名而引入的临时形状 DeletableMetadataLoader 随之退休,两处 cast 一并删除。
  2. 注册期响亮拒绝registerLoader() 校验「protocol: 'datasource:'capabilities.write: true ⇒ 必须实现 delete()」,不满足即 throw;错误信息写明是哪个 loader、声明了什么、后果是什么(unregister() 会跳过它却照常宣告删除成功,行被下一次读原样读回)、以及两条修法(实现 delete(),或者如果 loader 本就只读就改成 capabilities.write: false)。registerLoader()this.loaders 的唯一写入口——构造函数的 config.loaders 也走它——所以这个组合根本进不了运行时。
  3. unregister() 的现有守卫保留:对已注册 loader 由注册期校验保证不可达,作为防御性代码留下,注释写明其不可达性的来源;delete? 在接口上仍是可选的,因为其它协议的 loader 本来就可以没有。

门禁范围刻意等于 unregister() 实际作用的那个组合:file: / memory: / http: / s3: 的 loader 运行时从不被 manager 写入(register() 同样只挑 datasource:),没有自己的删除要收回,因此不受管;只读的 datasource: loader 同样不受管。

存量影响

全仓唯一的 datasource: loader 是 DatabaseLoader,它一直实现着 delete(),不受影响(PR 内有一条专门的回归用例钉住这点)。消费半径逐个核过:MetadataLoaderContract 在 spec + metadata 之外没有实现方,packages/rest 里唯一一处外部 MetadataManager 构造用的是 memory: 协议的 MemoryLoaderNodeMetadataManager 注册的是 file: 协议的 FilesystemLoader——都不在门禁范围内。

唯一被新校验拒绝的仓内 stubpackages/metadata/src/metadata.test.ts 的「should persist to datasource: protocol loaders」。它按意图就是一个可写的 datasource loader(断言的正是 save 被调用),所以按「补声明」处置——补上 delete,而不是收窄它的 capabilities。同文件 unregister 三个 stub 上的 as any 因为接口现在真的声明了 delete 而不再需要,一并删除;这顺带也证明了接口改动确实落地(若留着 as any,接口有没有 delete 测试都照样绿)。

验证

pnpm --filter @objectstack/metadata test(合并 origin/main 并重建依赖后):

 Test Files 24 passed (24)
Tests 495 passed (495)

新测试文件 packages/metadata/src/metadata-manager-loader-delete-contract.test.ts(11 例):注册期拒绝的正反两例、两个入口(registerLoader() 与构造函数 config.loaders)、拒绝后不半途注册、错误信息可读(点名 loader + 两条修法)、有 delete 的正例照常删除、门禁范围(只读 datasource + 四种非 datasource 协议)、DatabaseLoader 回归。

反向验证(方向事先预测:红):把 registerLoader() 里的门禁调用摘掉后重跑新测试,4 条拒绝用例转红、7 条(范围/正例/回归)保持绿——正是预测的方向与范围;门禁恢复后 11/11 绿。

跨包消费方 pnpm --filter @objectstack/rest exec vitest run src/rest-endpoint-surfaces-served-only.test.ts src/served-endpoints.test.ts:22 passed。

typecheck:packages/metadata 没有 typecheck 脚本(在 #4311 的 DEBT 台账里),所以直接对包跑 tsc --noEmit -p packages/metadata/tsconfig.json 并前后对比——改动前 92、改动后 92,且 loader-interface.tsmetadata-manager.ts、新测试文件三者零错误(台账里记的 87 是 7 月 31 日 b07d829 的旧测量值,不是本分支的基线)。

check:durability-log-levelcheck:startup-registry-verdictcheck:type-check-coveragecheck-nul-bytes 均绿。

changeset:.changeset/metadata-loader-delete-contract.md(patch,写明对第三方 loader 的影响与两条修法)。


Generated by Claude Code

…()` (#5276)
`MetadataLoader` declared `save?` and no `delete`, so `capabilities.write` meant
two different things at the two ends of an item's life: "persist into me" to
`register()`, and nothing at all to `unregister()`, which duck-typed `delete` at
the call site and silently skipped a loader that had none — then dropped the
registry entry, invalidated the list cache and announced a `deleted` event
anyway. The caller was told the delete succeeded while the row stayed in the
loader and was read straight back out by the next `list()`/`get()`, across
restarts, with nothing to retry it. declared != enforced (Prime Directive #10).
- `MetadataLoader` now declares `delete?(type, name): Promise<void>`, next to
`save?`. The temporary `DeletableMetadataLoader` shape #5259 introduced to
name the duck-typed hole is retired with both of its casts.
- `registerLoader()` rejects, loudly, a loader declaring `protocol:
'datasource:'` with `capabilities.write: true` and no `delete()` method,
naming the loader, the consequence, and both repairs (implement it, or
declare `capabilities.write: false`). It is the sole writer of the loader
map — the constructor's `config.loaders` funnel through it — so the
combination cannot reach the runtime.
- `unregister()`'s `typeof ... === 'function'` guard stays as defensive code
whose unreachability is now guaranteed by the registration gate; the comment
says so.
Scope is exactly the combination `unregister()` acts on: `file:`/`memory:`/
`http:`/`s3:` loaders are never written to by the manager at runtime and are
not gated, and a read-only `datasource:` loader is untouched. `DatabaseLoader`,
the only `datasource:` loader in the repo, has always had `delete()` and is
unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V7WetGmnfoXNn8cLieKKmx
@vercel

vercelBot commented Aug 5, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 5, 2026 10:07pm

Request Review

@github-actions

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.

@os-zhuang
os-zhuang marked this pull request as ready for review August 5, 2026 22:18
@os-zhuang
os-zhuang added this pull request to the merge queueAug 5, 2026
Merged via the queue into main with commit b5459bcAug 5, 2026
24 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-5276-loader-delete-contract branch August 5, 2026 22:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-zhuang@claude