Skip to content

fix(cloud-connection): LocalManifestSource.read() 说清它的 null 指的是两件事里的哪一件 (#5426) - #5439

Merged
baozhoutao merged 1 commit into
mainfrom
claude/issue-5426-manifest-read-cause
Aug 5, 2026
Merged

fix(cloud-connection): LocalManifestSource.read() 说清它的 null 指的是两件事里的哪一件 (#5426)#5439
baozhoutao merged 1 commit into
mainfrom
claude/issue-5426-manifest-read-cause

Conversation

@baozhoutao

Copy link
Copy Markdown
Contributor

Fixes #5426

缺陷

read() 用一个 null 同时回答两个不同的问题 ——「这个 manifest 从没装过」和「装过,但那个文件读不出来」。方法自己的注释(null when absent or unreadable)说明合并是有意的;后果不是。

两个 admin 端点(reseed / purge)都是先 has()read(),也就是说走到 read() 时「没装过」这一支已经被排除了,于是它们唯一能说的只有:

500 { code: 'MARKETPLACE_STORAGE_FAILED', message: 'Failed to read manifest cache.' }

一句只说明「我刚做的事失败了」的话 —— 而上一行 has() 刚说过文件在。真正能指向修复动作的那个对象(Unexpected end of JSON input / EACCES / EISDIR)在不带绑定的 catch 处已经被丢掉,服务端日志里也没有补一行。属 #5403 / #5412 / #5413 家族,是 #5413 修掉的 list() 的另一半。

方案(裁定的 A,与 list() 对齐)

read() 返回 InstalledManifestLookup:

磁盘上的状态 entry failure
该 manifest 没有文件 null
文件在、读不出/解析不了 null { file, cause }
文件解析成功 entry

failure 复用 list() 已有的 SkippedManifestEntry 形状(#5424 落地),cause 是抛出的原对象 —— 不包装、不字符串化。failure === undefinedentry === null 就是「没装过」,这正是原来那个 null 抹掉的事实。

破坏性变更:read() 不再返回 InstalledManifestEntry | null。旧行为等价于读 .entry,迁移说明写在 changeset 里。新导出类型 InstalledManifestLookup

三个调用方

  1. :586 ADR-0120 D5e posture gate —— 行为不变,读 .entry。损坏的条目仍然算作「没有既往 attestation」,一次性仪式重新发问而不是跳过(fail-safe)。合并这两个 null 在这个调用点是对的;区别在于它现在是在调用点明着做的决定,而不是 ledger 替所有人做的。
  2. :929 reseed / :1009 purge —— 响应 code 不变(同一个失败,只是终于有了解释,按 code 分支的客户端不受影响);message 附上要修复/删除的那个文件路径与 cause 原话(fix(cli): os doctor 指名道姓报告非法 OS_TENANCY_POSTURE 并非零退出 (#5382) #5390 体例,面向操作者的 admin API),并补一行 warn 服务端日志(功能性降级,按 AGENTS.md 分级)。两处共用一个 unreadableLedgerEntry() 帮助方法。

read() 仍然校验解析出来的值的 shape(那是另一个契约决定),list() 仍然在目录本身枚举不了时抛出 —— 两点都是刻意不动。

反向验证(方向先判后跑)

判定:把两个 handler 还原成改前的 'Failed to read manifest cache.'(保留新的 read()),预期 reseed/purge 的新用例转红,producer 侧与 D5e 用例保持绿。

实测 3 红 / 34 绿,与判定一致,但有一处需要如实修正:我预判「4 个 reseed/purge 用例转红」,实际是 3 个。第 4 个(a package that was never installed still gets 404)钉的是未改动的 404 边界 —— 它验证「两个 null 被区分开」没有让「没装过」错读成「损坏」,还原 handler 并不触及它,所以它按设计保持绿,不是回归钉。

× reseed: the 500 names the file and quotes the cause (#5426)
  AssertionError: expected 'Failed to read manifest cache.' to match /JSON/i
× purge:  the 500 names the file and quotes the cause (#5426)
× reseed: an unreadable file, not only an unparseable one, is explained
  AssertionError: expected 'Failed to read manifest cache.' to contain 'EISDIR'

D5e 那条 fail-safe 用例(ASKS AGAIN when the attestation record is corrupt)还原后不会转红,这一点在用例注释里写明了:改前 read() 对损坏文件本来就返回裸 null,gate 本来就会重新发问,行为等价正是本次接线的要求。它钉的是将来 —— 既然损坏现在可辨识了,「反正装过,当作已 attest」只差一行,而那会因为一个没人读得出的文件跳过一次性仪式(ADR-0120 S10/S14)。

Fixture 分诊

  • local-manifest-source.test.tsexpect(src.read('com.acme.bad')).toBeNull()整条替换的那一类:它钉的正是被删掉的那条支路,改后会因为「合并的 null 两边都是 null」而为错误的理由保持绿。容忍性(一个坏文件只赔上它自己)拆成独立断言保留,新增用例钉旧断言看不见的事实。
  • 其余 read() 调用点(posture-gate / heal 测试)属机械改写为 .entry
  • 消费半径已按规则遍历:全仓只有 packages/clidoctor.ts 另外引用 LocalManifestSource,消费的是 list(),与本次无涉(未动 packages/cli);cloudobjectos-runtime 只是原样 re-export,没有 read() 调用点。

验证

  • pnpm --filter '@objectstack/cloud-connection' test15 files / 109 tests passed
  • pnpm --filter '@objectstack/cloud-connection' build(含 DTS)→ success
  • tsc --noEmit:改动文件零错误;该包在 main 上本就有 13 条 tsc 报错(全在我未触碰的测试文件里,git stash 前后同为 13),另行记录。
  • node scripts/check-nul-bytes.mjs → OK,并对改动文件做了控制字符自查。

Changeset

@objectstack/cloud-connection minor,含迁移说明(const { entry, failure } = source.read(id))。


Generated by Claude Code

… two things its null meant (#5426)

`read()` answered `null` to two different questions at once — "this manifest was
never installed" and "it was installed, but its ledger file cannot be read" —
and its own comment (`null when absent or unreadable`) says the merge was
deliberate. The consequence was not: two admin endpoints call `has()` first, so
absence is already ruled out by the time they read, and both could only answer
`500 MARKETPLACE_STORAGE_FAILED / "Failed to read manifest cache."` — a sentence
whose only content is that the thing it just did failed, one line after `has()`
said the file is there. `Unexpected end of JSON input` / `EACCES` / `EISDIR` had
already been dropped in an un-bound `catch`, and nothing reached the log either.

Option A of the decision point, aligned with `list()` (#5413): `read()` returns
`{ entry, failure }`, `failure` being the same `SkippedManifestEntry` shape with
the thrown object carried unwrapped, and present ONLY when a file exists that
would not parse. `failure === undefined` with `entry === null` now means "not
installed" — the fact the merged null erased.

Wiring, per triage:
- the ADR-0120 D5e posture gate reads `.entry` and is behaviourally UNCHANGED:
  a corrupt entry still counts as "no attestation on record", so the one-time
  ceremony is asked again rather than skipped. Conflating the two nulls is the
  right call at that call site; it is now made there, in the open, instead of by
  the ledger for everyone.
- reseed and purge keep `code: 'MARKETPLACE_STORAGE_FAILED'` — the same failure,
  newly explained, so a client branching on the code is unaffected — and their
  message now names the ledger file and quotes the cause (#5390 house style),
  with a matching `warn` line on the server.

`read()` still does not validate the parsed value's shape; `list()` still throws
when the directory itself cannot be enumerated. Both deliberately unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016FNvXhtSdnEGEfLEsMmvxh
@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 11:51am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling 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/cloud-connection.

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

  • content/docs/protocol/kernel/metadata-service.mdx (via @objectstack/cloud-connection)
  • content/docs/releases/implementation-status.mdx (via @objectstack/cloud-connection)

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.

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.

LocalManifestSource.read() 把「条目不存在」和「条目损坏」都答成 null,两个 HTTP 调用方因此只能回一句无因由的 500

2 participants