fix(doctor): --fix stops paying for the audit it cannot act on (2026.8.10.2) - #523
Open
Sunrisepeak wants to merge 1 commit into
Open
fix(doctor): --fix stops paying for the audit it cannot act on (2026.8.10.2)#523Sunrisepeak wants to merge 1 commit into
Sunrisepeak wants to merge 1 commit into
Conversation
…8.10.2)
`--deep` was one bool over two unrelated costs, and `--fix` implied it.
Measured on a real 71 GB home: `--fix` ran the payload audit SEVEN times -- one
detection plus six `refresh()` calls at ~196s each -- and no repair consumes
what that audit produces. LoaderLibcSplit and NssResolution appear in
detection, `count_` and `render_`, and nowhere else; every repair selects on
other kinds. About 23 minutes, entirely unused.
Two costs, so two fields:
remedies a local catalog resolve per broken coordinate. This is what turns
a BrokenPayload finding into an install command and tells the
ladder the package is reinstallable. `--fix` needs it. Cheap.
payloads walking every payload for ELF loader/libc pairing and the NSS
probe. Diagnostic only. Expensive.
self doctor remedies=0 payloads=0
self doctor --deep remedies=1 payloads=1
self doctor --fix remedies=1 payloads=0 <- the change
self doctor --fix --deep remedies=1 payloads=1
And when `--fix --deep` IS asked for, the audit is carried across refreshes
rather than recomputed. Only `repair_payloads_` can change what an audit would
see -- it reinstalls through subprocesses; the metadata repairs write the
workspace, manifests and shims, none of which live under the payload store the
audit walks. Seven audits become two. `refresh()` takes the claim as an
explicit parameter with no default, so a new call site cannot pick the wrong
side quietly, and the carried findings are passed WHOLE rather than merged --
a spliced Scan is a second answerer.
`--fix` without `--deep` now says what it did not look at:
▸ • not audited payload/runtime audit did not run — add `--deep` to include it
Without that line a clean `--fix` and a clean `--fix --deep` print the same
thing, and "did not check" and "checked, nothing wrong" become one output.
Quick `doctor` always had the same boundary and now discloses it too.
tests/e2e/self_doctor_fix_audit_cost_test.sh asserts the COUNT of audits, not a
duration -- timings vary by machine, the number of ELF walks does not. It also
carries the counter-assertion, because "walks no payloads" is satisfiable by
doing nothing: with and without `--deep`, everything `--fix` reports about
broken payloads and remedies must be identical, and the two outputs must
otherwise differ or that comparison is vacuous.
self_doctor_depth_test.sh asserted the old contract and now asserts the new one
plus the two checks that keep the flip honest: `--fix --deep` must still walk
payloads on the same home, and `--fix` must disclose the skip.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
一句话
self doctor --fix在真实 71 GB home 上要跑 7 次 payload 审计(约 196s 一次),而没有任何 repair 消费它们的结果 —— 约 23 分钟的纯浪费。本 PR 把一个 bool 拆成两个语义。
根因(读源码,不是推测)
--fix路径里detect_被调 7 次(1 次初始 + 6 次refresh())。audit.deep只 gate 两处:--fix需要吗doctor.cppm:1201remedy(catalog 解析)doctor.cppm:1475「不需要」是查出来的:
LoaderLibcSplit/NssResolution只出现在枚举、detection、count_、render_四处;逐个确认过每个 repair 的 FindingKind 选择集,没有一个选它们。变更
--fix --deep时审计跨 refresh 携带而不是重算 —— 只有repair_payloads_能改变payload(它通过子进程重装),其余 repair 写的是 workspace / manifest / shim,都不在
审计 walk 的 payload store 下。7 次降到 2 次。
refresh()把这个判断作为没有默认值的显式参数,新增调用点无法安静地选错边;携带的 findings 整份带过、不拼接(拼接就是第二个回答者)。
跳过必须说出来
没有这行,干净的
--fix和干净的--fix --deep输出一样,「没检查」和「检查过没问题」就变成同一个输出。quick
doctor一直有同样的边界,现在也会说了。测试
新增
self_doctor_fix_audit_cost_test.sh,断言的是次数不是耗时(耗时因机器而异,ELF walk 的次数不会)。用 PATH 上的 patchelf recorder 计数:quick 0 /
--deep1 /--fix0 /--fix --deep≤2。带反向断言:「不走 payload」可以靠「什么都不做」达成,所以还要求
带与不带
--deep时,--fix关于 broken payload / remedy / prune 的输出逐字一致,并且两次输出整体必须不同(否则差分是拿一个东西和自己比)。
self_doctor_depth_test.sh原本断言的是旧契约(--fix蕴含 deep),已翻转,并补两条让翻转保持诚实:同一个 home 上
--fix --deep必须仍然走 payload;--fix必须自曝跳过。实现过程中我自己犯的两个错,留档在设计文档 §F
~/.xlings/bin/xlings(实为 2026.8.9.2,PR fix: recover CLI stability and runtime safety #519 之前)当2026.8.10.1 的对照,一度误判自己引入了回归。最后用 E2E 同口径比较已发布的 2026.8.10.1
与本次构建,输出逐字一致才确认。
|| true吞掉:给 fixture 写的执行器结尾|| true,用它写的「索引不可用就 fail」守卫退出码恒为 0,从来没守过任何东西 —— 正是本仓库反复出现的形态,
这次出现在我自己的测试脚手架里。
验证
run_all.sh82/82(含新增 E2E-79)2026.8.10.2未做
真实 home 上
--fix的端到端计时。--fix会对切片发起真实网络重装,上一轮已因此中止并跑
verify-untouched确认真实 store 未损。审计次数下降是结构证据,计时留待有隔离环境时补。