Skip to content

Correct the reuse-safety claim on the filter-subtree mark, and pin the invariant it rests on - #9073

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-8836-provenance-invariant-pin
Aug 16, 2026
Merged

Correct the reuse-safety claim on the filter-subtree mark, and pin the invariant it rests on#9073
os-zhuang merged 1 commit into
mainfrom
claude/issue-8836-provenance-invariant-pin

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8836

Both halves of the graded scope, together, as the triage required: the docblock correction and the executable pin. Comments and tests only — no mechanism change.

Premise re-verified on origin/main @ 65589d6b7

The false sentence was live, unchanged, in markFilterSubtreeProvenance's docblock:

Marking is therefore idempotent, and safe on filter objects that are reused across requests (view metadata, cached scopes): the classification of one subtree does not change between requests.

getReadFilter is at packages/spec/src/contracts/security-service.ts:215 as the card says. grep finds the sentence in exactly one file in the whole repo, so no generated page carries it — confirmed below.

1. The docblock correction

Lifted from #8794's survey comment ("The docblock's unstated precondition, written down"). The claim holds only where provenance is intrinsic to the subtree, and the two halves fail in opposite directions:

  • a 'policy' scope is intrinsically policy, so a stale mark is still correct next request and can only withhold — the direction the rule was written for;
  • a caller's where is contextual, so a stale 'author' mark discloses, and because first-mark-wins is absolute the corrective 'policy' stamp a later boundary would apply is a silent no-op.

The invariant is now written at the call site, and named as what it is — a property of the caller population that this module cannot enforce, because "the request" is not a concept it can see:

no filter object that can be vouched 'author' may outlive the request that vouched it.

getReadFilter gets the second half of the correction, since it is the published API that hands hosts the excluded shape: its documented use puts a platform-authored predicate into the options.where slot the merge boundaries vouch as the caller's. It now says request-scoped, never memoise the result, with both consequences spelled out.

2. The executable pin — and the fork clause, measured

The triage's premise-first clause said to stop if the pin turned out to be unwritable without a mechanism change. It is writable: four tests in filter-subtree-provenance.test.ts (15 tests before, 19 after), using nothing but the module's existing exports.

The invariant sentence is the describe name verbatim, so the next surveyor greps straight to it. What the block pins is the cost of breaking the invariant, not a should-be:

  1. the violation — a filter vouched in request 1, reused in request 2 where a sibling middleware composed the AST first so this request's boundary vouches nothing, still resolves 'author' for its { $field } comparand;
  2. the control — the identical shape built fresh per request, same position, same middleware, resolves null. Only the object's lifetime differs;
  3. the repair is unavailable — the mirror of the existing "first mark wins" test: a stale 'author' mark resists the 'policy' correction, and defineProperty throws;
  4. the asymmetry — same reuse, same unvouched root: stale 'policy' withholds, stale 'author' discloses.

Why (1) is not a tautology: (1) and (2) bracket the mechanism from both sides. A resolver that always disclosed would red (2); one that always withheld would red (1). The pair discriminates on object identity across requests, which is exactly the invariant's content.

No mechanism change — asserted, not claimed

First-mark-wins, the non-writable symbol and positional resolution are byte-identical. Independent confirmation that the accept face did not move:

  • check:api-surface — "public API surface + factory signatures unchanged"
  • check:authorable-surface — 1594 schemas regenerated, working tree still clean
  • check:generated — all 13 generated artifacts up to date; check:docs — 228 generated files in sync

So the PM's assumption that these docblocks do not feed generated reference pages is verified, not assumed: nothing regenerated, and nothing needed committing.

Reverse verification

Directions predicted before running, both observed as predicted. Fix committed first; the module was restored with git checkout and git status confirmed byte-identical afterwards.

Ablation A — delete the first-mark-wins guard, make the mark writable. Predicted: red on the new no-op test and on the pre-existing "first mark wins" test, since they are the two halves of one rule. Observed: Tests 2 failed | 17 passed (19) — exactly those two.

Ablation B — make the resolver require the ROOT itself to carry a mark (the "obvious fix" for the stale-mark disclosure, which is precisely the mechanism change #8794's ruling routes to a spec-seat ruling first). Predicted: red on pin tests 1 and 4, plus the pre-existing tests whose roots are unmarked $and shapes; the control stays green because it already expects null. Observed: Tests 5 failed | 14 passed (19) — pin tests 1 and 4, plus "a node inside the author arm…", "…arms that AGREE…", "terminates on cyclic input…". Control green, as predicted.

Verification — gate union at final head fb61435be

Gates re-derived from the actual changed paths with node scripts/pm/dispatch-gates.mjs; it returned the same nine path-derived families plus the five test-file convention families the dispatch named — no additions.

  • pnpm --filter @objectstack/spec test406 files, 10757 passed
  • pnpm --filter @objectstack/spec typecheck — clean (incl. check:test-typecheck: 55 files / 263 errors held, shrink-only)
  • pnpm exec vitest run src/data/filter-subtree-provenance.test.ts19 passed (19)
  • check:nul-bytes OK (5959 files) · check:changeset-gate-self-tests OK · check:cross-package-test-inputs OK (12 packages) · check:doc-formula-expressions OK · check:merge-driver OK · check:spec-parsed-alias OK · check:type-source-resolution OK · check-adr-0087-registration OK · check-dev-prereqs OK
  • convention families: check:query-options-erasure OK (test surface at ceiling, no files added) · check:type-check-coverage OK · check:type-check-debt OK (33 entries re-measured, none above its number) · check:engine-double-contract OK · check:where-matcher OK (244 matchers)

check:doc-formula-expressions and check-dev-prereqs were red on the first pass purely because a fresh worktree has no dist/; both green after turbo run build. The full workspace closure was built before the ratchet families ran.

Changeset: skip-changeset

Comments and tests only; nothing user-visible is released. This follows the measured convention for docblock-only source PRs in this repo — #8979 and #8944 both edited packages/*/src comments with zero changesets under the skip-changeset label, and the four most recent test(...)-only PRs did the same. The label is applied on this PR.


Generated by Claude Code

…, and pin the invariant it rests on
The mark's docblock called itself "safe on filter objects that are reused
across requests (view metadata, cached scopes)". The #8794 survey measured
that claim: it holds only where provenance is INTRINSIC to the subtree. A
'policy' scope is intrinsically policy, so a stale mark is still correct and
can only withhold. A caller's `where` is contextual — a stale 'author' mark
discloses under a root the next request never vouched, and first-mark-wins
makes the corrective 'policy' stamp a silent no-op.
Corrects both docblocks (the mark's, and `getReadFilter`'s — the public
method whose documented use puts a platform predicate in the vouched slot),
and adds four pin tests naming the invariant verbatim: no filter object that
can be vouched 'author' may outlive the request that vouched it.
No mechanism change: first-mark-wins, the non-writable symbol and positional
resolution are untouched, and the accepted metadata set is unmoved.
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec.

106 hand-written doc(s) reference the affected code — list omitted above 15 rows. Re-derive: node scripts/docs-audit/affected-docs.mjs --json origin/main.

7 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

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.

@os-zhuangos-zhuang added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 16, 2026 — with Claude
@os-zhuang
os-zhuang marked this pull request as ready for review August 16, 2026 08:55
@os-zhuang
os-zhuang added this pull request to the merge queueAug 16, 2026
Merged via the queue into main with commit 1850ebbAug 16, 2026
30 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8836-provenance-invariant-pin branch August 16, 2026 09:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

protocol:datasize/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-zhuang@claude