Skip to content

docs(spec): name both zero-rows sentinels in the published explain payload contract - #14900

Merged
os-sam merged 2 commits into
mainfrom
claude/issue-13961-explain-zero-rows-sentinels
Sep 3, 2026
Merged

docs(spec): name both zero-rows sentinels in the published explain payload contract#14900
os-sam merged 2 commits into
mainfrom
claude/issue-13961-explain-zero-rows-sentinels

Conversation

@claude

@claude claude Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #13961

The published explain payload contract enumerated the zero-rows vocabulary as a closed two-item list — null = unrestricted, { id: '__deny_all__' } = zero rows — on both fields that carry a composed row predicate. That enumeration had grown incomplete: explain also publishes plugin-security's fail-closed RLS denial, unrewritten. This PR names both shapes at both sites, regenerates the reference page and the emitted JSON Schema, and pins the claim.

Option (a) of the card only. No vocabulary unification (option C on #13639 is the maintainer's), no producer change, no new exported constant, and readFilter / rowFilter stay z.unknown() — the accept set and the emitted payload are byte-unchanged.

The two zero-rows sentinels

Spelling Producer Meaning What the decision fields say
{ id: '__deny_all__' } plugin-sharing's buildReadFilter / buildWriteFilter (sharing-service.ts:419, :499; sharing-plugin.ts:1082), plus the explain engine's own computeRlsFilter catch fallback The composed deny-all sentinel — the member the enumeration always had. When it is composed in, it REPLACES the whole published readFilter (explain-engine.ts:1371) rls layer verdict: 'denies'; decision.allowed: false
{ id: '__rls_deny__:' + a UUID-shaped suffix } RLS_DENY_FILTER, frozen in plugin-security/src/rls-compiler.ts:89-92; reached by the fail-closed RLS path (applicable policies exist but none compiles against the context — e.g. no active organization), named at objects/default-permission-sets.ts:555 and :719 An id equality against a UUID-shaped string no record carries, so the SQL layer returns zero rows without raising. Published as composed — the payload collapse is deliberately keyed to __deny_all__ alone — so it can also ride inside an $and composite on readFilter rls layer verdict: 'denies'; decision.allowed: false (since PR #13960)

Both rows end in the same place, and that is what the new text says: a reader who consults verdict / decision.allowed — or, per record, outcome / matchesRecord — is never misled. The gap was only for a consumer pattern-matching the payload itself, which the contract now tells to match both shapes.

The concrete UUID suffix is deliberately NOT quoted in the contract text: the prefix plus the suffix's nature is the stable half, and pinning the literal value into the published spec would be a step toward the vocabulary decision this PR does not take.

What changed, per site

Both sites are in packages/spec/src/security/explain.zod.ts.

1. ExplainRecordAttributionSchema.rowFilter — JSDoc and .describe(). The describe is the published face; here it is in full:

The effective row predicate this layer contributed for the record set, published as composed (null = unrestricted). Two shapes mean zero rows — { id: "__deny_all__" } and the fail-closed RLS denial (__rls_deny__ plus a colon and a UUID-shaped suffix) — so a consumer pattern-matching this payload alone must match both; the decision itself is outcome/matchesRecord plus the layer verdict.

2. ExplainDecisionSchema.readFilter — JSDoc, plus a .describe() it did not have:

The composed row filter the caller would be served with — the machine artifact behind the prose (null = unrestricted). Two shapes mean zero rows — { id: "__deny_all__" } and the fail-closed RLS denial (__rls_deny__ plus a colon and a UUID-shaped suffix, which can also ride inside an $and composite) — so a consumer pattern-matching this payload alone must match both; the decision itself is allowed plus the rls layer verdict.

Each JSDoc block carries the same claim at more length, plus the detail that only __deny_all__ triggers the payload collapse.

Measured: a JSDoc-only field publishes NOTHING. readFilter carried its whole enumeration in a JSDoc block, and no generator reads property-level TSDoc. On main its Description cell in content/docs/references/security/explain.mdx:109 was empty and the emitted json-schema/security/ExplainDecision.json carried no description for readFilter at all — so the field a consumer most needs the vocabulary for published no vocabulary. The .describe() closes that, and it moves no surface baseline: api-surface/ records export names, and authorable-surface already listed security/ExplainDecision:readFilter. Both gates ran green (below).

One adjacent correction, named rather than slipped in. The readFilter JSDoc opened "For read:". The engine publishes the field for read and exportexplain-engine.ts:1414-1416, whose own [#3544] comment says why — so the JSDoc now reads "For read and export (#3544)". Source-only: the new .describe() deliberately makes no claim about which operations carry the field, so nothing new about operation scope is published. Boundary scan for that class: readFilter and rowFilter are the only two properties in this file whose prose enumerates payload values, and both are rewritten here.

Consumers that pattern-match the payload — the card's unmeasured question

Measured by grep across packages/**, non-test sources, for __deny_all__, __rls_deny__, readFilter and rowFilter:

  • No consumer reads ExplainDecision.readFilter or ExplainRecordAttribution.rowFilter at all. Every non-test occurrence of either field name under packages/** is inside plugin-security/src/explain-engine.ts — the producer — or inside packages/spec itself (the schema, the authorable-surface baselines, the liveness empty-state registry).
  • The remaining __deny_all__ sites are producers (plugin-sharing/src/sharing-service.ts, sharing-plugin.ts) or prose in doc comments (plugin-audit/src/comment-access-hooks.ts:145, service-storage/src/attachment-access-hooks.ts:478, service-analytics/src/read-scope-sql.ts:1009). The remaining __rls_deny__ sites are the producer, service-analytics's two bound-SQL-parameter suites, and lint/formula prose. None consumes the explain payload.
  • objectui is not attached to this session, so nothing here measures it; the dispatch carried the PM's reading for that repo.

The card's own reading therefore stands, and it is now recorded rather than assumed: the exposure was the prose, not a misled consumer.

Regenerated artifacts

Regenerated with the spec tooling, never by hand:

  • pnpm --filter @objectstack/spec gen:schema — rebuilds the gitignored packages/spec/json-schema/ tree the reference pages render from. authorable-surface.base.json unchanged; authorable-defaults/ verified against upstream, 1236 defaults unchanged.
  • pnpm --filter @objectstack/spec gen:docscontent/docs/references/security/explain.mdx (merge=os-regen), three rows: ExplainDecision.readFilter at :109 (empty cell to the new text) and the two ExplainRecordAttribution.rowFilter rows at :172 and :219.

The emitted JSON Schema was read back to confirm the text reached it: json-schema/security/ExplainDecision.json now carries a description at properties/readFilter (absent before) and at its nested rowFilter, and json-schema/security/ExplainRecordAttribution.json at :15.

The pin

packages/spec/src/security/explain-zero-rows-sentinels.pin.test.ts — eight cases over both fields, reading the description through the schema (SchemaName.shape.FIELD.description), never by grepping the source: the description object is what the generators copy, and a .describe() detached from its field surfaces here as undefined instead of passing on a source line that still exists. It pins the claim shape, not the wording — both sentinel names, the unrestricted pole, the decision fields, and the "must match both" statement; rephrasing is free, dropping a member is not. The RLS prefix is a literal rather than an import: packages/spec carries no runtime dependency (Prime Directive #2) and a cross-package read would be a declared test input. The producer-side half — that the constant really is spelled this way — is owned by plugin-security's suites.

Verification record

Every reading below is at the final commit of this branch, 031ae9c49 (worktree objectstack-issue-13961, cut from origin/main 2263ca4d6). Heavy runs went through scripts/pm/os-verify-lock.sh; the wall-clock figures are shared-box seconds, not idle-box seconds. Exit codes were captured before any pipe, and every verdict quoted is the gate's own printed verdict line.

Run Result
pnpm --filter @objectstack/spec exec vitest run src/security/explain-zero-rows-sentinels.pin.test.ts src/security/explain.test.ts 2 files, 37 passed
pnpm --filter @objectstack/spec typecheck exit 0 — tsc --noEmit, check:scripts-typecheck, and check:test-typecheck ("54 file(s) / 262 error(s) / 146 pinned signature(s) held", unchanged, so the new test adds no debt). tsc -p tsconfig.test.json --listFiles confirms the new pin file IS in the program, so that green is about it
pnpm --filter '@objectstack/spec...' build exit 0, 34/34 declaration files emitted (rebuilt after the last source edit, so check:api-surface reads a dist newer than src)
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack 72 commands derived from the 4 changed paths against merge base 2263ca4d6
The derived family, re-run whole at 031ae9c49 71 commands: 64 exit 0, 7 PREREQUISITE NOT MET, 0 findings

The 7 non-zero results are NOT MEASURED, not red, and each says so in its own output — all seven are the same prerequisite, an unbuilt workspace, which is CI's to satisfy: check-dev-prereqs ("The workspace is not built — 1 unmet precondition, not a list of problems"), check-test-completeness (exit 3, "this gate grades a saved turbo run test log, and no log was named" — its text names this exact situation and says to record it as NOT MEASURED), check:doc-formula-expressions and check:doc-security-posture (exit 3, need @objectstack/formula / @objectstack/lint built), check:skill-examples ("packages/client-react/dist holds no .d.ts declarations"), check:dual-build-cjs-loads (exit 3, 85 packages with no dist/), and check:type-check-debt (exit 3, "54 workspace dependenc(ies) … have no built type entry point on disk"). Only @objectstack/spec and its dependency closure were built here.

Two declared narrowings. (1) The 72nd derived command, node scripts/pm/check-half-states.mjs, was stopped by PID after 3m09s and is excluded from the 71: it is the LIVE board sweep from the scheduled half-state-patrol.yml, report-only by construction, and CI runs only its offline self-test pnpm check:pm-half-states — which is separately in the derived set and passed. Filed as a finding (link below). (2) Repo-wide pnpm lint was not run locally; that is CI's run.

One real red, found and fixed here. The first sweep reported check:empty-state UNCLASSIFIED at explain.zod.ts:142, resolved to outcome. Cause: that gate attributes a permissive-empty statement to the nearest property within 8 lines, and the longer JSDoc pushed the "null = unrestricted" clause 9 lines above rowFilter, so it resolved backwards onto the previous property — a red the registry could not answer, because the statement belongs to a property already registered. The clause now closes each block two lines above its declaration, on one line so the detector still sees it (commit 031ae9c49). Prose only: both .describe() strings, and therefore every generated artifact, are byte-unchanged by that second commit. check:empty-state now reports "all classified (2 closed, 2 open, 4 output, 9 scope)".

Reverse verification of the pin, with the fix committed first. Predicted direction: RED. The schema file was restored to its pre-fix text at 2263ca4d6, and the mutation was confirmed on disk before anything ran — __rls_deny__ occurrences 4 to 0, working-tree blob be1a08bbb different from the HEAD blob b827898ce. The pin then failed 7 of its 8 cases. The one survivor is rowFilter "still names the unrestricted pole", which the pre-fix text also satisfied; every case carrying the new claim went red, and all four readFilter cases went red on the missing .describe() — the JSDoc-publishes-nothing measurement, stated as an assertion. Restore leg: git checkout HEAD -- ... inside an EXIT/INT/TERM trap against an absolute path resolved from git rev-parse --show-toplevel, then proven byte-identical — working-tree hash b827898ce equal to the HEAD blob, git diff HEAD empty, git status --porcelain empty.

Out of scope, filed not fixed

🤖 Generated with Claude Code

https://claude.ai/code/session_017RbbUMnxkUnWhE4j94v8FE


Generated by Claude Code

…yload contract

`ExplainDecision.readFilter` and `ExplainRecordAttribution.rowFilter` published
the zero-rows vocabulary as a closed two-item enumeration (`null` =
unrestricted, `{ id: '__deny_all__' }` = zero rows). A fail-closed RLS denial
composes plugin-security's `RLS_DENY_FILTER` — `__rls_deny__` plus a colon and
a UUID-shaped suffix — and is published as composed, so the contract named one
of the two shapes a consumer can actually meet.

Both sites now name both shapes, state that the RLS denial is published as
composed (so it can ride inside an `$and` composite on `readFilter`), and say
which fields are the decision. `readFilter` carried its prose in a JSDoc block
only, which no generator reads: its docs cell and its JSON Schema `description`
were empty, so it gains a `.describe()`.

Text only: both fields stay `z.unknown()`, no accepted value or emitted payload
moves, and the two sentinels are NOT unified.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017RbbUMnxkUnWhE4j94v8FE
…ate's window

`check:empty-state` resolves a permissive-empty statement to the nearest
property declaration within 8 lines. The longer JSDoc blocks pushed the
"`null` = unrestricted" clause 9 lines above `rowFilter`, so the gate resolved
it backwards to `outcome` and reported it UNCLASSIFIED — a red the registry
could not answer, because the statement belongs to a property that is already
registered.

The clause now closes each block, two lines above its declaration, on one line
so the detector still sees it. Prose only: both `.describe()` strings, and
therefore every generated artifact, are byte-unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017RbbUMnxkUnWhE4j94v8FE
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 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
  • 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 — 128 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 2b9f5810b777ee9d0211d9d6273d854bd8e2d5bdpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 76be23c48c763ac403d3e4012cd4e0062df05d68 — the merge of head 031ae9c4971f6266c5c243c2d5a7b24f976b38e7 into base 2b9f5810b777ee9d0211d9d6273d854bd8e2d5bd, 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 76be23c48c763ac403d3e4012cd4e0062df05d68 && git checkout 76be23c48c763ac403d3e4012cd4e0062df05d68
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2b9f5810b777ee9d0211d9d6273d854bd8e2d5bd 031ae9c4971f6266c5c243c2d5a7b24f976b38e7 && git checkout -B drift-repro 2b9f5810b777ee9d0211d9d6273d854bd8e2d5bd && git merge --no-ff 031ae9c4971f6266c5c243c2d5a7b24f976b38e7

node scripts/docs-audit/affected-docs.mjs --json 2b9f5810b777ee9d0211d9d6273d854bd8e2d5bd

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

os-sam commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Landing provenance (seat, 09:08Z):

⇒ ready for review + auto-merge (squash). Queue policy: a kick on a signature that is not this diff's gets one re-queue with a receipt; a second kick ⇒ the card goes pm:blocked on the anchor with Unlock-action: re-check PR #14900.


Generated by Claude Code

@os-sam
os-sam marked this pull request as ready for review September 3, 2026 09:08
@os-sam
os-sam enabled auto-merge September 3, 2026 09:08
@os-sam
os-sam added this pull request to the merge queue Sep 3, 2026
Merged via the queue into main with commit 7317cf2 Sep 3, 2026
36 checks passed
@os-sam
os-sam deleted the claude/issue-13961-explain-zero-rows-sentinels branch September 3, 2026 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] the published explain schema names only one zero-rows payload sentinel, and explain can now publish the other

2 participants