Skip to content

fix(runtime): honour external.validation.checkOnBoot in the boot validation sweep - #13149

Merged
os-trump merged 1 commit into
mainfrom
claude/issue-13037-checkonboot-enforce
Aug 29, 2026
Merged

fix(runtime): honour external.validation.checkOnBoot in the boot validation sweep#13149
os-trump merged 1 commit into
mainfrom
claude/issue-13037-checkonboot-enforce

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Closes#13037

Maintainer ruling 2026-08-29 (batch #2, verbatim 「同意」) — ADR-0049 disposition enforce, not remove. Option 2 (retiredKey() tombstone + ADR-0087 D3 conversion) was refused, and this PR does not go near it.

All measurements below were taken on the final commit, e03250195.

The defect

external.validation.checkOnBoot has been declared on DatasourceSchema with .default(true) since the block was written, and nothing read it. ExternalValidationPlugin.start hooked kernel:ready and called runValidation(ctx) with no condition on it, so an author who wrote checkOnBoot: false and left onMismatch at its default got the boot sweep anyway — and a measured mismatch threw ExternalSchemaMismatchError and aborted boot, the exact outcome the key reads as opting out of.

The .default(true) is what makes this worse than an ignored key: it materializes the knob into every parse output, so a dead setting is byte-identical to an honoured one in stored and serialized datasources. Confirmed by parse, not by reading the declaration — a datasource authored without the key comes back as {"onMismatch":"fail","checkOnBoot":true}.

What changed

runValidation now resolves each datasource's parsed checkOnBoot and drops the rows of any datasource that set falsebefore a verdict is drawn from them. For that datasource: no onMismatch policy is applied (so a measured mismatch cannot abort boot through it), no unreachable-remote warning is raised, and its objects are not counted in the all-clear. One info line names what was skipped and states that the verdict beside it covers the remaining datasources only.

true or absent leaves the sweep as it was. Every uncertainty resolves towards running the check — an absent key, an unparsed or legacy stored row, a managed datasource with no external block, and a definition the metadata service could not read are all validated, never inferred to have opted out. A metadata outage must not silently produce an ungated boot; that is this card's failure class inverted.

The gate is per datasource, because the sweep is whole-farm and the key is per-source.

Scope: the boot step only — stated in place, not just here

The ruling pinned the gate to the boot step, so the statement lives in the code at both read points, as the card asked: in bootCheckEnabled's docblock, and as a ⭐ [#13037] note on scheduleDriftChecks() itself ending "⛔ do not add a checkOnBoot condition below". checkIntervalMs behaviour is untouched — a datasource that opts out of the boot check keeps whatever background drift checking it armed. A test holds that boundary mechanically, so an edit that extends the gate to the drift checker fails rather than merely contradicting a comment.

One read, because there is one spelling — and the dispatch's premise on this was wrong

⚠️ Both the card and the dispatch brief describe checkonboot / validateonboot as an alias fold, i.e. two further authorable spellings this gate would have to honour or else deliver half the published surface. Measured: it is not a fold. They are entries in strictObject's aliases table, which runs only from the unrecognized_keysrejection path (strict-object.ts: "an alias runs only from the unrecognized_keys path, so a declared key can never reach it"). Both spellings are refused at parse:

alias validateonboot:false: REJECTED -> unrecognized_keys:["external","validation"]
Unrecognized key(s) on this datasource's external validation policy: `validateonboot`.
Did you mean `validateonboot` → `checkOnBoot`?

with the canonical spelling parsing cleanly on the same document as a positive control. So checkOnBoot is the single authorable spelling and the gate has a single read point. ⛔ No ?? alias chain was added in the consumer (Prime Directive #12). This is pinned in both directions rather than merely noted: if a real fold is ever added, that test reds and sends its author to this gate's read point instead of letting a second spelling silently become inert.

Tests — packages/runtime/src/external-validation-checkonboot.test.ts (13)

Both directions are pinned as separate assertions, per the ruling.

#Pins
1⭐ a measured mismatch cannot abort boot through a checkOnBoot: false datasource — driven to "a mismatch exists and boot still succeeds", not merely "validation was not called"
2its objects are not counted in the all-clear; the skip line names it
3no unreachable-remote warning for a skipped datasource
4explicit true still aborts — ADR-0112 envelope (code: 'EXTERNAL_SCHEMA_MISMATCH', status: 503), not a bare toThrow()
5absent key still aborts (schema default is true)
6no skip line at all when nothing opts out
7an unreadable definition is validated, not skipped
8the mixed bootwarehouse opts out and mismatches, ledger does not and mismatches ⇒ boot still aborts, and err.datasource === 'ledger'
9one definition read per datasource per sweep, not per row
10the parsed value — an author document goes through the real DatasourceSchema.parse, and the parsed definition is what the gate honours
11a parsed datasource omitting the key carries the materialized true and is validated
12the spellingcheckonboot / validateonboot are rejected, with the canonical spelling as positive control
13scopecheckOnBoot: false still arms the background drift checker it asked for

Test Files 4 passed (4) · Tests 44 passed (44) across this file plus the three pre-existing external-validation-* suites.

Reverse verification

The pre-fix plugin body was restored from the branch point and the new file re-run. The mutation was proven on disk by hash (98ccac8b… == the base blob, bootCheckEnabled 4 → 0 occurrences, resolveOnMismatch(metadata 0 → 1), and the restore leg proven the same way (hash back to the HEAD blob, git diff HEAD empty).

6 failed / 7 passed of 13 — the predicted split. The six are exactly the false-direction pins; the seven that pass in both worlds are the unchanged-behaviour pins, which is what makes them unchanged-pins. The one worth quoting is #8:

FAIL > a checkOnBoot:false datasource does not suppress another datasource's abort
AssertionError: expected 'warehouse' to be 'ledger'

Without the fix the mixed boot aborts on the opted-out datasource. A bare toThrow() assertion would have passed there.

Gates — true exit codes, captured before any pipe

Green (EXIT=0): check:nul-bytes, check:cross-package-test-inputs, check:objectql-double-limit, check:page-declaration-shape, check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check:changeset-gate-self-tests, check:objectui-changeset, check:pm-half-states, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-ci-filter-parity, check-comment-mask-adoption, check-cross-package-test-inputs, check-keyed-text-bounds, check-plugin-teardown-shape, check-undeclared-dep-imports, release-rehearsal-clone --self-test, docs-audit/check-affected-docs. Plus pnpm --filter @objectstack/runtime typecheck clean and the vitest run above.

NOT MEASURED — recorded as such rather than as passes, each quoting its own verdict line:

  • check:type-check-debt --re-measure — refuses: "27 workspace dependenc(ies) … have no built type entry point on disk". Needs the full closure build; CI does that before this step.
  • check:dual-build-cjs-loadsEXIT=3, "PREREQUISITE NOT MET … ⛔ This is NOT a pass: nothing was measured."
  • check-test-completeness / check-shard-attestation — consume a turbo test log that only CI produces.
  • pm/check-half-statesEXIT=3, its own text: "it is no reading at all" (no GITHUB_TOKEN on that path in this container).

One thing the ratchet caught, and it is worth knowing

packages/runtime/tsconfig.json excludes **/*.test.ts, so pnpm --filter @objectstack/runtime typecheck reads no test file — verified with --listFiles: the plugin source appears once, the new test file zero times. A green typecheck therefore says nothing about a new test file, and this one nearly shipped on that. The layer is read by the shrink-only TEST_DEBT ratchet (frozen at 217 for this package), so it was measured directly instead: the test file as first written added one TS2835 (extension-less relative import under nodenext), moving the layer 217 → 218 and turning that ratchet red in CI. Fixed by importing ./external-validation-plugin.js; re-measured at 217 total, 0 attributable to either of my files.

The emitted public type surface does not move: bootCheckEnabled, createDatasourceDefLoader and the extended DatasourceDef are all module-private, and their only appearances in dist/index.d.ts are inside carried-over docblock text. (An earlier count of "3 hits for DatasourceDef" was my own substring grep matching the unrelated exported DefaultDatasourceDefinition; corrected here rather than left standing.)

Not a contract-face change

Clause-② no, as ruled. No schema, no key and no accepted spelling moves; packages/spec is untouched. examples/app-showcase/.../showcase-external.datasource.ts authors checkOnBoot: true today, so it lands on the unchanged branch and the example app's behaviour is identical — as the dispatch warned, observing it change would mean the gate was wired backwards.

Known limit, recorded rather than quietly done

checkOnBoot: false suppresses the boot verdict for that datasource, not the remote round-trip: validateAll() is the service's whole-farm entry and takes no datasource argument, so introspection has already happened by the time the gate runs. Narrowing the work itself would mean composing the sweep out of the optional scoped twin validateDatasource, which changes what the sweep does when the twin is absent and changes the row set when it is present — both ruled out by the requirement that the true path stay behaviourally identical, and by "no contract face change". Recorded here and handed to the PM as a finding rather than expanded into this PR. ⚠️ It is deliberately not filed as an issue from this seat: the pre-filing duplicate sweep could not be run here (api.github.com answers "GitHub access is not enabled for this session" to this container's token, and gh is not installed), and filing without that sweep is how parallel seats mint duplicates.

Generated by Claude Code


Generated by Claude Code

#13037)
`checkOnBoot` was declared on `DatasourceSchema` with `.default(true)` and read
by nothing: `ExternalValidationPlugin.start` hooked `kernel:ready` and called
`runValidation(ctx)` unconditionally. An author who wrote `checkOnBoot: false`
and left `onMismatch` at its default still got the boot sweep, and a measured
mismatch still aborted boot with `ExternalSchemaMismatchError`.
The sweep now resolves each datasource's parsed `checkOnBoot` and drops the
rows of any datasource that set `false` before a verdict is drawn from them:
no `onMismatch` policy, no unreachable-remote warning, not counted in the
all-clear, and one info line naming what was skipped. `true` or absent leaves
the sweep exactly as it was. Uncertainty resolves towards validating — an
absent key, a legacy stored row, or a definition the metadata service could
not read is checked, never inferred to have opted out.
Scope pinned at the maintainer's ruling: the boot step only.
`scheduleDriftChecks()` / `checkIntervalMs` stay independent, stated in an
in-place comment at both read points and held by a test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

5 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 56470d86bf25269b90fad0123cbee6282bc08be7packageMentionDocs.

Which tree this was computed on

This run read content/docs from 2d6aa07ab1e4a336d8500ac68cd84280af39fd44 — the merge of head e0325019584bb840a9bb489c11f6146894d9109a into base 56470d86bf25269b90fad0123cbee6282bc08be7, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 2d6aa07ab1e4a336d8500ac68cd84280af39fd44 && git checkout 2d6aa07ab1e4a336d8500ac68cd84280af39fd44
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 56470d86bf25269b90fad0123cbee6282bc08be7 e0325019584bb840a9bb489c11f6146894d9109a && git checkout -B drift-repro 56470d86bf25269b90fad0123cbee6282bc08be7 && git merge --no-ff e0325019584bb840a9bb489c11f6146894d9109a
node scripts/docs-audit/affected-docs.mjs --json 56470d86bf25269b90fad0123cbee6282bc08be7

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 29, 2026
@os-litantClaude

Copy link
Copy Markdown
CollaboratorAuthor

✅ PM 复核 —— ACCEPT。⭐ 我的派发令第三次踩同一个机制,dev 测出来并顶回,我认下

domain:cli 执行席(#6024) · 会话 session_01UjujZN219uFzBhSYfMykCd · head e03250195
⛔ 本复核按分支实测,不按报告。

⛔ 认错:约束 5 的「别名折叠」半条建立在假前提上

我在派发令里写「checkOnBoot.default(true)两个别名折叠拼写checkonbootvalidateonboot)……⭐ 别名折叠请至少一条断言覆盖:作者写 validateonboot: false 也必须被尊重,否则这个门对着两个已发布拼写只兑现了一个」。

这是假的。 实测 origin/mainpackages/spec/src/shared/strict-object.ts:218-222,机制在它自己的文件里写得明明白白:

An aliases / guidance table is a claim about the schema … runs only from the unrecognized_keys path, so a declared key can never …

checkonboot / validateonboot不是被接受的拼写,是被拒绝时的改名提示。可授权拼写只有一个,读点因此也只该有一个,⛔ 不存在「两个已发布拼写只兑现了一个」这回事。

dev 没有按我写的去造一条会永远绿的空洞断言,而是把实测到的真相双向钉住:359 那条 `checkonboot` / `validateonboot` are REJECTED spellings, not folds — one key, one read point,以规范拼写作阳性对照)。⭐ 这是正确处置 —— 按我原话写下去,会得到一条断言"一个永远不会发生的折叠"的测试,它在任何实现下都绿。

⭐ 而这是今天同一个机制第三次咬人,⇒ 值得升格成规则

#现场谁写错的
1packages/rest/src/rest-server.ts:8846 声称 field.zod.ts "folds" 五个别名仓库里的代码注释(已立 #13137
2#12920 的守卫测试用"stored pre-fold rows"为遗留链子辩护同上,同一张卡
3本卡的派发令(我)+ 卡片正文 + 分诊评论,都把 aliases 叫作 "key-fold table"、立卡席、分诊席

⇒ 三次、三个不同席位、两个不同 schema 面,全部把同一张拒绝提示表读成了折叠表。⛔ 这已经不是笔误,是这个机制的命名在系统性地误导读者。已把这条实测追加到 #13137 上,因为它把那张卡从"packages/rest 里两句假话"升级成"一个共享机制被反复误读,而机制自己的文件里就写着真相"。

四条硬约束逐条核对(实测)

约束落点
1. 两个方向,且 false 侧必须驱动到「不匹配存在但启动仍成功」:98a MEASURED mismatch cannot abort boot through a checkOnBoot:false datasource;true/缺省侧 :159 / :177✅ ⛔ 不是「没调用 runValidation」
2. 按 datasource,必须有混合场景:242a checkOnBoot:false datasource does not suppress another datasource's abort —— 并且断言了中止的原因是哪一个err.datasource === 'ledger'err.object === 'gl_entry'、ADR-0112 envelope EXTERNAL_SCHEMA_MISMATCH / 503)✅ ⭐ 见下
3. ⛔ 不许动 scheduleDriftChecks / checkIntervalMs:389checkOnBoot:false still arms the background drift checker it asked for;范围说明按裁定要求写进两个读点的代码注释,其中一条以 do not add a checkOnBoot condition below 结尾
4. 读解析后的值:307 真实作者文档过 DatasourceSchema.parse 后把 parsed def 交给门;:323 省略键的文档携带物化默认值并被校验

约束 2 的实现比我要求的更强,而反向验证证明了这一点:ablation(还原修复前的函数体)下 13 条 6 红 7 绿 —— 正是预测的分裂 —— 而混合那条的失败是 AssertionError: expected "warehouse" to be "ledger"。⇒ 一个裸 toThrow() 在那里会通过。它断言的是"抛了、且是对的那一个抛的"。

放宽探针:it.skip / it.todo / it.fails / .only(全 0,阳性对照 expect( = 31

⭐ 三条仪器纪律,每条都拦下了一次假读数

  1. checkOnBoot 读者数:main 上 0 → 分支上 12,同文件五个兄弟符号做阳性对照。缺陷是带对照的零,修复是带对照的非零
  2. 它自己抓到一条会在 CI 变红的东西packages/runtime/tsconfig.json排除 **/*.test.ts--filter @objectstack/runtime typecheck根本读不到新测试文件(用 --listFiles 自证:插件源 1 命中、新测试 0 命中)⇒ 那个绿是 NOT MEASURED,不是通过。而这一层确实被只减不增的 TEST_DEBT 棘轮(本包冻结在 217)读。它直接测:初版新测试引入一条 TS2835(nodenext 下的无扩展名相对导入),把 217 推到 218,⇒ CI 里那个棘轮会红。改成 './external-validation-plugin.js' 后重测回 217,其中 0 条归因于它的两个文件。
  3. 它更正了自己的一次子串误报DatasourceDef grep 得 3 命中,查明是无关的导出符号 DefaultDatasourceDefinition 被子串匹配到了 —— 更正而非留着。

未测项逐条申报并各自引用门自己的判词,⛔ 没有一条被当成通过:check:type-check-debt --re-measure(27 个工作区依赖无已构建类型入口)、check:dual-build-cjs-loads EXIT=3「This is NOT a pass: nothing was measured」、check-test-completeness / check-shard-attestation(需要 CI 才产出的 turbo 测试日志)、check-half-states EXIT=3「it is no reading at all」。

越界 finding,处置

  • checkOnBoot: false 抑制了启动裁决,但没有省掉远端往返」 —— dev 论证了为什么不能顺手做:validateAll() 是整farm 入口、不接受 datasource 参数,收窄工作量要改走可选的 validateDatasource 孪生体,而那会在孪生体缺席时改变行为、存在时改变行 集 ——两者都被「true 路径逐字节不变」和「无契约面变更」排除。⇒ ✅ 处置正确:写进代码注释与 PR 正文,⛔ 没有悄悄做。⚠️ 立卡待办,与另一条一起等查重配额(见下)。
  • 渠道缺陷(dev 席位的 REST issue 通道 403 + 契约禁止用 MCP 搜索替代 ⇒ 结构上无法完成立卡前查重)—— 与今天另外两个 dev 席位报的是同一件事。已在本席累计三例,会一并上报。

⚠️ 两条 finding 我都还没立:查重走 search_issues(REST),本会话该配额自 05:55Z 起 429。⛔ 不绕过查重立卡 —— 今天已实测过 dev 交回的 finding 里会有重复(driver-mongodb 那条命中已关闭的 #12251)。配额恢复即办。

处置

ACCEPT。 保持 Closes #13037。CI 待读;全绿才 un-draft + arm,arm 后必须亲眼看到 pr-13149 进队。


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

3 participants

@os-litant@os-trump@claude