fix(engine): aggregate boot-path advisory hits into one summary line, and count them by row - #15306
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
… write Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…visory-boot-path-aggregation
…hor census lines Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…visory-boot-path-aggregation
📓 Docs Drift CheckThis PR changes 3 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 37 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 673a194f90e15771b7ae1f3b5e071bf069388731 && git checkout 673a194f90e15771b7ae1f3b5e071bf069388731
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3a4373f4c3ff0b4d2e04a029f632392a0dc762b1 834991e11648c9f8c044b7a44fdde559ae68751e && git checkout -B drift-repro 3a4373f4c3ff0b4d2e04a029f632392a0dc762b1 && git merge --no-ff 834991e11648c9f8c044b7a44fdde559ae68751e
node scripts/docs-audit/affected-docs.mjs --json 3a4373f4c3ff0b4d2e04a029f632392a0dc762b1
|
Clause-② re-derived on the ACTUAL diff: yes.
|
| symbol | kind |
|---|---|
ADVISORY_SAMPLE_ROWS |
const |
AdvisoryHit |
interface |
AdvisoryGroup |
interface |
recordAdvisoryHit |
function |
isAggregatingAdvisories |
function |
runWithAdvisoryAggregation |
function |
@objectstack/core's exports map declares exactly two entry points — . and ./logger — so an addition to src/index.ts is the published surface. And core carries no @internal convention (the tag is used in packages/client, packages/metadata-protocol and plugin-auth, but git grep -c '@internal' -- packages/core/src is empty), so there is no existing seam that would have kept these out of the public API.
⇒ Widening the public surface is the second limb of clause ②, independent of accept/reject behaviour. It fires here.
2. ⚠️ My claim-time judgment was falsified, and the way it failed is reusable
My claim comment (#13889, 5537948162) recorded Clause-②: no, reasoning from the ruling's own words: 「⛔ 不改规则语义,只改日志形状」 and 「不触 spec 声明面」.
Both of those statements are true of this diff — rule semantics do not move, and packages/spec is untouched. The error is in what I concluded from the second one:
⭐ "Does not touch
packages/spec" is not the same as "does not widen the public surface."@objectstack/coreis a published package too. I checked the accept/reject limb properly and then let "spec is untouched" stand in for the public-surface limb, which it does not cover.
This is the failure mode the tier tooling is explicitly written against — scripts/pm/dispatch-gates.mjs says a path derivation cannot answer clause ② and that 「The enforcement lives one step later, in the PM skill's enqueue gate over the PR's ACTUAL diff」. That later gate is what caught it. It working as designed does not make the earlier miss costless: had this PR been a smaller delivery I might have enqueued it.
Standing check for this seat from here on: when judging clause ②, enumerate every published package the surface touches and diff its entry point — never infer the public-surface limb from the absence of packages/spec.
3. ⛔ This is not a mark against the delivery
The new export is disclosed in the PR body by the dev itself — 「adds a public export」 — which is precisely what made it findable. The seam's placement is argued rather than assumed (objectql → metadata-protocol runs one way, so neither end can import the other; AsyncLocalStorage over a module flag because a per-org seed replay runs on a live server), and the engine.ts reservation for #14666 was honoured with a stated reason for not threading a parameter from there. ⛔ No rework is being asked for on this ground.
4. Consequences — what happens and what does not
needs:contract-reviewis now on both carriers ([Decision]severity: 'warning'的校验规则没有受众概念 —— UI 级劝导在 seed/bootstrap 等机器写入路径上照样求值并打爆启动日志,且按写入次数而非按行计数 #13889 and this PR), hung together once a reviewable diff existed — ⛔ not pre-hung (maintainer 2026-08-28: 「⛔ 不预挂:可复审增量存在前永不挂标」). Read back on both.- ⛔ This PR stays draft. ⛔ Not flipped ready, ⛔ not enqueued, ⛔ no auto-merge.
- The gate is unmet by the absence of a review, not by a failed one — those are different states and the label cannot distinguish them, so it is stated here.
CONTRACT_REVIEW_TIER=claude-fable-5-1(dispatch-gates.mjs:8831, re-read onorigin/mainthis fire) is quota-exhausted in this session, measured three times today, most recently at 09:20Z with a minimal probe (req_011Cei9p9biyCTdZsq7tFbvj).⚠️ That is a fact about this session, not about the tier — the director seat ran an at-tier review at 09:05Z. ⛔ 免复核不放行. - CI on
28ea75b4ais still running; nothing here pre-judges it.
5. One item for the card record, so it is not lost when #13889 closes
The ruling parks option A behind a tripwire, verbatim: 「若出现「某 warning 规则就该在机器路径响」的实例 ⇒ 受众键成真需求,census 随卡回决策箱」. Fixes #13889 is correct — the ruling's acceptance anchor (one row rings once on a clean first boot; one summary line) is what this PR delivers, and the ruled scope is complete. But the tripwire then survives only in the ruling comment on a closed card, where no queue walk will surface it. Naming it here so the reopen path is on the record: a future instance of a warning rule that genuinely should fire on a machine path reopens #13889 rather than starting cold.
Generated by Claude Code
…visory-boot-path-aggregation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
Docs-accuracy round — three sentences this change falsified, repairedSupersedes the "47 gate families … final commit 28ea75b" line in the body: the two Verified on the tree actually edited, not on the tree the drift bot computed (
Net Two sentences checked and deliberately NOT edited, because they survive: ⛔ Generated by Claude Code |
Landing — contract review PASS at
|
Fixes #13889
Implements maintainer ruling B (comment 5494461940, 2026-09-01, verbatim 「同意」): aggregate advisory hits on the seed/boot load path into one summary line, and in the same stroke fix the by-write counting defect. Option A (an audience key on
ValidationRuleSchema) stays parked —packages/specis not touched. Option C is not taken.The two landing sites had to be located by behaviour, not by the ruling's names
The ruling names
demo_bootstrapand a claim-scanowner_idwrite-back; neither resolves in this repo (0 hits, with a firing control). Both platform halves were therefore located by symbol first:packages/objectql/src/validation/rule-validator.ts— thelogger.warninevaluateValidationRules, one line per row per violated rule. Not in the seed loader at all.demo_bootstrapclaim scanpackages/plugins/plugin-security/src/claim-seed-ownership.ts:342—ql.update(name, { owner_id: adminUserId }, { where: predicate, multi: true, context: { isSystem: true } })packages/objectql/src/engine.tsis not touched: it is the reserved serial resource for #14666. EveryevaluateValidationRulescall site lives in it, which is why neither half was implemented by threading a parameter from there.What changed
1. Aggregated reporting (
@objectstack/core, newrunWithAdvisoryAggregation/recordAdvisoryHit).SeedLoaderService.load()runs inside an advisory aggregation scope and reports one summary line per rule — rule, object, row count, the rule's own message, example rows — instead of one line per row. The scope lives incorebecause it is the layer both ends already depend on, and the dependency only runs one way (objectql depends on metadata-protocol, never the reverse). It usesAsyncLocalStoragerather than a module flag on purpose: a per-org seed replay runs on a live server, so a global would capture concurrent interactive writes' advisories into the replay's summary. Hits are folded into their(object, rule)group on arrival, so 100k rows tripping one rule hold one group, not 100k records.2. Advisory rules are counted by row, not by write (
rule-validator.ts). Anupdatewhose payload touches only platform-injected system columns changes no business field, so it no longer re-evaluates the object's advisory rules. The rule is dropped from the run, not merely silenced — the ruling asks for no re-evaluation, and suppressing output would leave the wasted predicate work in place. Membership is resolved per object by the declared authorityresolveInjectedSystemColumns, never a literal list, soownership: 'org'(noowner_id) andsystemFields: falseare judged on their own columns.error-severity rules are untouched by both halves. Rule semantics do not move: off the machine path the identical per-write line is emitted, byte for byte.Verification
Behavioural pins, driven through a real
ObjectQLengine, a realSeedLoaderService, and the real claim-scan write shape —packages/objectql/src/validation/advisory-boot-path.test.ts:errorrule still rejects both a system-column-only write and an ordinary one.Both pins were proved to red on the pre-change path (each ablation committed first, mutation confirmed on disk by anchored counts, rebuilt, marker confirmed in
distbyablation-dist-preflight, then restored and the marker re-proved absent):@objectstack/corerebuilt, marker live indist/index.js+dist/index.cjs): PIN 1 and PIN 2 red.expected [ …(2) ] to have a length of 1 but got 2, the row ringing twice; PIN 3b red too.Suites, all green:
@objectstack/core48 files / 1159 tests ·@objectstack/metadata-protocol161 files / 2370 tests ·@objectstack/objectql270 files / 4631 tests. Typecheck green for all three (theircheck:test-typecheckcovers the new test file, so it is genuinely type-checked).Two gates went red and were repaired, both real:
check:objectql-double-limit— the new test'sfinddouble ignored the caller's bound. Fixed by applying the bound after the filter, by presence; this matters becauseclaimSeedOwnership's paged fallback reads with alimit.check:system-context-census— pure line rot: the 61-line insertion inseed-loader.tsmoved three anchors cited bycontent/docs/permissions/system-context.mdx. Repaired with the gate's own--fix; only line numbers moved. That page's claim thatisSystemdoes not skip validation rules remains true — the new gate is keyed on the payload's columns, not on the context.All 47 gate families derived by
node scripts/pm/dispatch-gates.mjs(no path arguments) are green; the ratchet families and the pins were re-run on the final commit 28ea75b, after the last merge oforigin/main.A changeset is included and deliberately not
skip-changeset: this changes runtime behaviour on a shipped boot path and adds a public export.Generated by Claude Code