fix(plugin-audit): restore the #4630 unscoped multi-delete refusal on sys_comment through the wired engine

[os-warren]

Fixes#9798

sys_comment's #4630 guard declares that a multi-write carrying no id and no where is refused outright — "Refusing an unscoped multi-{update|delete} of comments — scope the write to the rows you mean" — rather than authorizing the whole table by resolving zero rows. That refusal never fired through the wired engine on either verb, by the #9719 mechanism: the per-row dispatch contract (#5038/#5574) binds input.id on every beforeUpdate/beforeDelete dispatch of a predicate write, so resolveTargetRows always took its by-id branch, and a zero-match predicate dispatched nothing at all.

This PR implements the DELETE half, inheriting #9719's ruling. The UPDATE half is split out as the decision card it was flagged to be — see below.

The change

One line of product behaviour, plus the test re-point it requires:

• packages/plugins/plugin-audit/src/comment-access-hooks.ts — the beforeDelete registration declares dispatchUnscopedMultiDelete: true, the engine mechanism landed by The #4757 unscoped multi-delete refusal on sys_attachment never fires through ObjectQL.delete — per-row hook dispatch bypasses it #9719 (PR fix(objectql): restore the #4757 unscoped multi-delete refusal on sys_attachment through the wired engine #9797, squash 1258dcaee), read on the merged ref. The engine now delivers the whole-operation context once, before any row is resolved, zero-match included, so the handler's own sys_comment has no record-level authorization: any org member reads and writes comments on records they cannot see #4630 branch is reachable with its declared envelope (RECORD_NOT_ACCESSIBLE, 403). The duck-typed CommentAccessEngine.registerHook options type is widened by the one optional key; the mechanism itself is not restated here (it is owned by HookEntry.dispatchUnscopedMultiDelete in @objectstack/objectql).
• Comments now say which dispatch can deliver the shape per verb, because the branch reads as verb-neutral and is not.

No sys_attachment-style engine change was needed: the mechanism is already on main.

Tests — the false green is gone, and the remaining gap is pinned, not hidden

The old block called the handlers directly with a whole-operation context the test file built itself — a shape the per-row dispatch never produces — so it was green on both verbs while the wired engine refused neither. It is replaced by wired-path pins driving a real ObjectQL + in-memory driver + the production installer, mirroring the attachment suite's shape after #9797. Every refusal asserts the rows survived.

• DELETE, restored (7 pins): unscoped { multi: true } refused even when the caller authored every matched row; explicit where: null likewise; the empty-table (zero-match) limb; positive control that an empty table is not refused per se (where: {} resolves); the per-row refusal stays a distinguishable message; scoped controls (by id, real where, match-all where: {}) all still resolve; system context still bypasses.
• UPDATE, measured (4 pins): the refusal cannot be restored the same way — dispatchUnscopedMultiDelete is beforeDelete-only and the engine refuses it elsewhere by design. Rather than drop the false-green block silently or assert a refusal that does not happen, the three limbs are pinned as measured, annotated to go RED when the decision lands.

Rejection assertions carry code and status per ADR-0112, plus the declared first sentence — the wording is contract-adjacent prose and is unchanged, verbatim.

Ablation

Removing the load-bearing line (dispatchUnscopedMultiDelete: true) turns exactly the three unscoped-delete pins RED with the predicted signature — the write silently succeeding, refusal absent:

× refuses `{ multi: true }` even when the caller AUTHORED every matched row → promise resolved "2" instead of rejecting
× refuses an explicitly null `where` the same way → promise resolved "1" instead of rejecting
× refuses on an EMPTY table → promise resolved "+0" instead of rejecting
Tests 3 failed | 35 passed (38)

The resolved values are the deleted-row counts: without the flag the table is actually wiped. All controls stayed green — the per-row gate, the scoped writes, the system bypass, the positive control, every UPDATE pin and every direct-call unit pin. Restored byte-identically (sha256sum -c: OK). The mutated subject is plugin-audit's own src, imported by the test through a relative specifier, and this package has no dist at all, so no built output could have shadowed it; the engine is the un-aliased import (KNOWN_UNALIASED_TEST_IMPORTS) and was built before any color was read.

The UPDATE half — #9974, deliberately not a rider

Filed as its own unassigned decision card with the measurement, per this issue's triage instruction and the unblock comment. Measured through the wire on this branch:

Case	unscoped multi: true update	Result
Caller authored every row	not refused	whole table rewritten
Empty table (zero match)	not refused	resolves; nothing ran
A row the caller may not touch is swept	refused	per-row message, not the unscoped one

So the update half is a partial, row-dependent guard rather than a total fail-open — narrower than the delete hole this PR closes, and still a hole, because the declared refusal is about the shape regardless of what the rows say. Wiring it means extending whole-operation dispatch to beforeUpdate's predicate path, which assertValidUnscopedMultiDeleteFlag's own comment names as a product-behaviour decision. #9974 is not addressed here.

Verification

Gates re-derived with node scripts/pm/dispatch-gates.mjs against the actual diff (it names more families than the dispatch did — the changeset family and five convention-triggered ones). All run green at 3a4b035c1:

check:cross-package-test-inputs · check:slot-lookup · check:test-source-alias · check:type-source-resolution · check:changeset-gate-self-tests · check:objectui-changeset · check:query-options-erasure · check:engine-double-contract · check:where-matcher · check:i18n · check:type-check-coverage · check:nul-bytes · check-adr-0087-registration · check-changeset-no-major · check-empty-changeset · docs-audit/check-affected-docs

Plus pnpm --filter @objectstack/plugin-audit typecheck (exit 0) and pnpm --filter @objectstack/plugin-audit test — 18 files, 295 tests passed.

Two gates refused to measure before they could run, and both were fixed rather than skipped: check:i18n reported "Nothing was checked" on an unbuilt CLI, and after turbo run build --filter=@objectstack/cli it reported OK (9 package(s) — all bundles in sync); check:type-check-debt --re-measure refused on an unbuilt closure entry (@objectstack/service-knowledge), and after building it reported OK — 33 ledger entr(ies) re-measured, 1925 raw tsc error(s) total, none above its recorded number.

One declared narrowing. That ratchet run measured the tree byte-identically equal to 3a4b035c1 — it was the step immediately before the commit that captured it — and the two attempts to re-run it at that sha were each cut by the shared verify lock (a sibling agent's gate run, then a full pnpm build), never by a failure. The re-run is also not load-bearing here: plugin-audit appears in neither the DEBT nor the TEST_DEBT ledger (it is COVERED by its own typecheck script, which is green above), and this diff touches only packages/plugins/plugin-audit/src/** plus a changeset, so no ledgered number is reachable from it. CI runs the family regardless.

Patch changeset included: the refusal is a user-visible behaviour change on a published package.

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-audit, touching 3 documentable anchor(s).

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/kernel/events.mdx(via registerHook (symbol))
• content/docs/plugins/development.mdx(via registerHook (symbol))
• content/docs/plugins/packages.mdx(via sys_comment (literal))
• content/docs/ui/setup-app.mdx(via sys_comment (literal))

⛔ 3 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v14.mdx(via sys_comment (literal))
• content/docs/releases/v16.mdx(via registerHook (symbol), sys_comment (literal))
• content/docs/releases/v17.mdx(via registerHook (symbol), sys_comment (literal))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 7 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 90f4d5dc32477aafef1b3d3bd64834e4b3ca6a81 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 85a17a49c36404638a9e60332f969b35805e6ec1 — the merge of head 3a4b035c1efb38d718be4418c841d1dfaa71585b into base 90f4d5dc32477aafef1b3d3bd64834e4b3ca6a81, 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 85a17a49c36404638a9e60332f969b35805e6ec1 && git checkout 85a17a49c36404638a9e60332f969b35805e6ec1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 90f4d5dc32477aafef1b3d3bd64834e4b3ca6a81 3a4b035c1efb38d718be4418c841d1dfaa71585b && git checkout -B drift-repro 90f4d5dc32477aafef1b3d3bd64834e4b3ca6a81 && git merge --no-ff 3a4b035c1efb38d718be4418c841d1dfaa71585b
node scripts/docs-audit/affected-docs.mjs --json 90f4d5dc32477aafef1b3d3bd64834e4b3ca6a81

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 90f4d5dc32477aafef1b3d3bd64834e4b3ca6a81 → pass the list as
args.docs, on the commit named under Which tree this was computed on.