Skip to content

test(objectql): prove purge/transfer/restore never reach the security AST path, and pin it (#7809) - #8106

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-7809-ast-path-operation-normalisation
Aug 12, 2026
Merged

test(objectql): prove purge/transfer/restore never reach the security AST path, and pin it (#7809)#8106
os-zhuang merged 4 commits into
mainfrom
claude/issue-7809-ast-path-operation-normalisation

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Fixes#7809

Delivers the second of the two outcomes triage declared acceptable: the destructive verbs never take the AST path, so this proves it and pins it, rather than normalising code nothing reaches.

⚠️ Surface: this PR adds a file to packages/objectql, outside the declared surface

The dispatched surface was packages/plugins/plugin-security/src/**. The pin is in packages/objectql/src/ instead, deliberately, and that is a scope expansion the reviewer should see rather than discover.

The reason is that a pin in plugin-security could not fail. The invariant is a property of the engine's dispatch vocabulary; plugin-security's own suite drives a hand-built fake engine, so an assertion there would be a claim about the fixture's construction, not about what the engine dispatches — and binding against the real engine would need a new cross-package dev-dependency pointing the wrong way. So the pin lives where the change that would break it gets made.

plugin-security still carries the comment half: 23 lines at the two raw sites, zero code lines.

The reachability answer

purge / transfer / restore cannot reach the security middleware — not on the bulk AST path, and not on any other path.

  1. OperationContext['operation'] is a seven-member union — find, findOne, insert, update, delete, count, aggregate. No destructive lifecycle verb is a member.
  2. Middleware is reachable only through the privateexecuteWithMiddleware(ctx: OperationContext, …). Exactly seven call sites, no public seam that hands a middleware a context the engine did not build.
  3. So every dispatch must construct an OperationContext, and its operation is a member of that union by construction.

Corroborating, not load-bearing: repo-wide, operation: 'purge' and operation: 'restore' appear only in test files; transfer has no runtime dispatch at all; and API_METHOD_DERIVATION gives restore/purge a permanent flag: () = false since enable.trash was retired (#2377 / ADR-0049), with the recycle bin (#3146) parked.

Measured, not read. A real engine driven through every public data method dispatches:

operationcarries an ast
find / findOne / count / aggregateyes
insertno
update (by-id)no
update (bulk)yes
delete (by-id)no
delete (bulk)yes

That is the finding's own mechanism confirmed from the other side: the AST path is real and is reached by bulk update/delete — and by nothing destructive.

What this changes

Nothing at runtime. The pin has two halves because each covers the other's hole:

  • The weld reads the union out of engine.ts at runtime. It covers the case the probe structurally cannot — a new engine method added tomorrow, which still has to build an OperationContext to reach the middleware, so the union still has to widen.
  • The behavioural probe drives a real engine and records what middleware actually receives. It covers a dispatch site that lies about the union.

Ablation

Predictions were written down before the first run. Two of the three falsified something, which is the only reason this section is worth reading.

mutationpredictedobserved
widen the union onlytypecheck red, vitest greenboth green — the weld was dead; see below
a dispatch site emits purgeprobe redall 5 red, incl. the AST-path case
widen the pin's own array onlyweld redweld red; verb check correctly green

The first ablation killed the original design. The weld started as type-level assignability consts, and widening the union passed both runners, so it was rewritten to hold at runtime and re-ablated — red, as it should have been.

The stated cause was then itself wrong, and the correction is the more useful finding. "Enforced by nothing" was measured against pnpm typecheck alone, which does exclude this package's test layer. But check:type-check-coverage compiles it and ratchets the raw count through TEST_DEBT — verified from both sides on this PR: three type errors in the new file drove @objectstack/objectql to 358 against a recorded 355 and went red; fixing them returned it to exactly 355. So a type weld would have been caught, as "+1". The runtime weld is kept for diagnosis — it names what broke and fails in pnpm test — not because types are unenforced here. The file's comment now says this, because the earlier version would have taught the next reader something false.

The third row is the anti-tautology check: widening the pin's own array cannot make the destructive-verb assertion falsely pass, because that assertion reads the engine source rather than the array.

A real bug the gate feedback surfaced

Fixing check:query-options-erasure by typing the four read-options bags (rather than baselining) immediately exposed aggregations: [{ func: 'count', … }] in the new probe — copied from a sibling test that spells it behind an as any. The contract key is function. That is #4674's exact shape: an unknown key is silently dropped, never rejected, so the erasure was the only reason the wrong spelling compiled. Fixed here; the sibling's occurrence is untouched and reported as an out-of-scope finding.

Verification

  • @objectstack/objectql — 191 files / 3381 tests pass; tsc --noEmit clean; TEST_DEBT back at its recorded 355
  • @objectstack/plugin-security — 50 files / 1006 tests pass; tsc --noEmit clean
  • check:query-options-erasure — test surface back to 242, at the ceiling, baseline untouched
  • Gates re-derived for the final surface: check:cross-package-test-inputs, check:docs-audit-scope, check:test-source-alias, check:durability-log-level, check:engine-double-contract, check-engine-split-ratio, check:nul-bytes — all green

Worth noting for whoever tunes the dispatch tooling: neither gate that actually went red on this PR is path-derivable — dispatch-gates.mjs puts both in its "repo-wide / undetermined" bucket, so no per-card gate list would have named them.

No changeset, deliberately

The diff is one new test file plus 23 lines of comment — zero code lines changed in any shipped path. It releases nothing, so it carries skip-changeset rather than a changeset that would put a release-notes line against a behaviour change that does not exist.


Generated by Claude Code

…ctive verbs (#7809)
#7809 found that plugin-security's bulk AST path passes `opCtx.operation` raw
while the 2.7 by-id gate normalises purge->delete and transfer/restore->update.
Reachability was the finding's own first question; measured answer: those verbs
never reach a middleware at all. `OperationContext['operation']` is a 7-member
union with no destructive lifecycle verb, and middleware is reachable only via
the private `executeWithMiddleware(ctx: OperationContext, …)`.
Pins the invariant where it lives, in two halves that cover each other's hole:
a compile-time weld (red when the union widens — the case a behavioural probe
cannot see, since a new method does not exist to be driven) and a real-engine
probe (red when a dispatch site emits a verb outside it).
Records the invariant at the two raw sites in security-plugin.ts so the next
reader sees why raw is safe there, and what must normalise if it stops being.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
…t typecheck
The weld was first written as type-level assignability consts. The mandatory
ablation (widen `OperationContext['operation']` by one member, predict red)
found it DEAD: this package's tsconfig excludes every `.test.ts`, so
`tsc --noEmit` never saw those consts and the widened union passed both vitest
and typecheck. A type assertion in a test file here is enforced by nothing.
Replaced with a runtime weld that reads the union out of `engine.ts` and throws
rather than returning a partial list on any parse failure — a regex that
silently matched nothing would restore the same can't-fail property. The
destructive-verb assertion now reads the SOURCE list, so widening this file's
own array cannot fake it green (confirmed: that case leaves it correctly red on
the weld and green on the verb check).
Re-ablated after the rewrite: widened union -> red; a dispatch site emitting
`purge` -> all five red, including the AST-path case this finding is about.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 3:58pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security.

12 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx(via @objectstack/plugin-security)
  • content/docs/kernel/runtime-services/sharing-service.mdx(via @objectstack/plugin-security)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/access-recipes.mdx(via packages/plugins/plugin-security)
  • content/docs/permissions/authorization.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/explain.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/permissions-matrix.mdx(via packages/plugins/plugin-security)
  • content/docs/permissions/sharing-rules.mdx(via @objectstack/plugin-security)
  • content/docs/plugins/index.mdx(via @objectstack/plugin-security)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-security)
  • content/docs/ui/audience-based-interfaces.mdx(via packages/plugins/plugin-security)
  • content/docs/ui/dashboards.mdx(via @objectstack/plugin-security)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/plugin-security)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

…a pin here
Two red gates on the first head, both in the new pin file.
check:query-options-erasure (test surface 242 -> 246): the four READ calls
carried `as any`. Nothing here is deliberately off-contract, so the remedy is
the typed form, not `as unknown as EngineQueryOptions`. Back to 242, at the
ceiling, baseline untouched.
Typing them immediately earned its keep: it surfaced `aggregations: [{ func:
'count' }]`, copied from a sibling test that spells it behind an `as any`. The
contract key is `function`. That is #4674's exact shape — an unknown key is
silently dropped, never rejected — so the erasure was the only reason the wrong
spelling compiled.
check:type-check-coverage TEST_DEBT (@objectstack/objectql 355 -> 357): three
type errors in the new file. All three fixed, ledger untouched at 355, verified
against the gate's own synthesized project from both sides.
That measurement also corrects this file's own comment. It claimed a type pin in
an objectql test file is "enforced by nothing"; that was measured against
`pnpm typecheck` alone, which does exclude the test layer. TEST_DEBT compiles it
and ratchets it, so a type weld WOULD have been caught, as "+1". The runtime
weld stays for diagnosis, not because types are unenforced — and the comment now
says so.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
@os-zhuang
os-zhuang marked this pull request as ready for review August 12, 2026 16:25
@os-zhuang
os-zhuang added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit 28df4a6Aug 12, 2026
39 of 41 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7809-ast-path-operation-normalisation branch August 12, 2026 16:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-zhuang@claude