Skip to content

Commit 9fad07f

Browse files
baozhoutaoclaude
andauthored
fix(cli): os doctor 不再在 ledger 读不到时打出 ✓ Unique scope (#5412) (#5414)
`readInstalledPackageEntries()` 用一个不带绑定的 `catch` 罩住了两件性质完全 不同的事,并对两者都返回空数组: 1. `@objectstack/cloud-connection` 解析不到 —— 可选包没装。这一条静默是 对的,保持不变:`os doctor` 必须能在没有该包的 checkout 里跑完。 2. ledger 目录**存在**(`fs.existsSync` 已经通过)但读取抛错。 第 2 种被当成第 1 种,于是它以「没有已装包」的身份抵达 ADR-0120 D5e 的 unique-scope 建议,建议无话可说,报告打出 `✓ Unique scope`。一个装了包、 ledger 却读不到的 isolated 环境拿到的是一张干净体检单 —— false PASS 比漏报 更糟,它让操作者停止查看。 两者现已拆开:`import()` 失败保留自己的静默 catch;读取失败带着 cause 回到 调用方,成功行被扣住(它是对建议**两半**的断言,只有两半都跑过才能打), 改为经 `renderHealthCheckResult()` 输出一条 warning 档 `HealthCheckResult`, `--verbose` 展开随之免费获得。已跑过的那一半的 findings 仍照常上报。 不在本单范围:ledger 内**单个条目**损坏 —— `LocalManifestSource.list()` 在 自己的 per-file catch 里跳过它,调用成功且返回短列表,消费者无从分辨。 已另立 #5413,并由本 PR 的 SCOPE BOUNDARY 测试钉住。 Claude-Session: https://claude.ai/code/session_016FNvXhtSdnEGEfLEsMmvxh Co-authored-by: Claude <noreply@anthropic.com>
1 parent b4872a8 commit 9fad07f

3 files changed

Lines changed: 639 additions & 22 deletions

File tree

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): `os doctor` no longer prints `✓ Unique scope` when it could not read the installed-package ledger (#5412)
6+
7+
`readInstalledPackageEntries()` wrapped two unrelated facts in one un-bound
8+
`catch` and returned an empty entry list for both:
9+
10+
- **`@objectstack/cloud-connection` does not resolve** — the optional package is
11+
not installed. Silence is correct here and stays: `os doctor` must run to
12+
completion in a checkout that never had it.
13+
- **The ledger directory exists and could not be read**`fs.existsSync()` had
14+
already confirmed the directory is there, and producing its entry list threw
15+
(the path is occupied by a file, the filesystem refused, the read failed).
16+
17+
The second was handled as the first. It reached the ADR-0120 D5e unique-scope
18+
advisory as "no installed packages", the advisory found nothing to report, and
19+
the run printed:
20+
21+
```
22+
✓ Unique scope No unconfirmed installation-wide uniques for this 'isolated' environment
23+
```
24+
25+
So an environment **with** installed packages, whose ledger doctor could not
26+
read, got a clean bill of health for the one constraint the `isolated` posture
27+
makes dangerous. That is worse than a missing check: a false PASS is what stops
28+
an operator looking further.
29+
30+
The two are now separate. The `import()` failure keeps its own silent `catch`;
31+
a ledger read failure comes back as a cause the caller can report, and the
32+
success line is withheld — it is a claim about **both** halves of the advisory
33+
(this project's metadata, and the manifests of installed packages), so it may
34+
only be printed when both halves ran. In its place doctor prints an ordinary
35+
`HealthCheckResult` through the same renderer every other check uses:
36+
37+
```
38+
⚠ Unique scope Could not read the installed-package ledger (installed packages NOT
39+
checked for installation-wide uniques) — ENOTDIR: not a directory, …
40+
```
41+
42+
**Warning, not error**, and the exit code is unchanged: the environment still
43+
runs; what broke is doctor's ability to see part of it. The cause is quoted from
44+
the thrower rather than paraphrased, and `--verbose` expands the untruncated
45+
original plus which half of the check did not run — both free from reusing
46+
`renderHealthCheckResult()`.
47+
48+
Findings from the half that **did** run are still reported: a ledger failure
49+
does not swallow the uniques this project's own metadata declares.
50+
51+
**Not covered by this change**: a single *corrupt entry* inside an otherwise
52+
readable ledger. `LocalManifestSource.list()` skips unparseable files in its own
53+
per-file `catch`, so a truncated manifest is dropped inside the producer and the
54+
call succeeds with a short list that no consumer can distinguish from a complete
55+
one. Fixing that is a change to `@objectstack/cloud-connection`'s own contract
56+
and is tracked separately; the boundary is pinned by a test so it is not
57+
mistaken for covered.

0 commit comments

Comments
 (0)