Skip to content

fix(plugin-security): retire seven dead { records } find-result limbs, and stop the permission-set loader inventing an empty page - #15838

Queued
os-warren wants to merge 6 commits into
mainfrom
claude/issue-15598-plugin-security-dead-limbs
Queued

fix(plugin-security): retire seven dead { records } find-result limbs, and stop the permission-set loader inventing an empty page#15838
os-warren wants to merge 6 commits into
mainfrom
claude/issue-15598-plugin-security-dead-limbs

Conversation

@os-warren

@os-warren os-warren commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Fixes #15598

Seven { records } union-normalizer blocks in packages/plugins/plugin-security, on engine find() results. Six carried a dead envelope limb. The seventh is the opposite defect and is repaired in the opposite direction.

Census, re-derived — 7, matching the card

Re-measured on origin/main 5315098df rather than trusted from the card (whose count came from the #15094 census taken 2026-09-04 on a tree that has since moved). Method: grep -rn '\.records' packages/plugins/plugin-security/src --include=*.ts, tests excluded, then each hit resolved to a distinct enclosing normalizer block by hand — 9 grep lines, 6 files, 7 blocks (seed-name-lookup.ts carries two blocks, and two blocks each span two grep lines). Diff against the card's 7: none. After this change the same scan returns zero hits in this package's source.

The .records anchor is load-bearing rather than incidental: normalize-managed-by.ts:55 was if (Array.isArray(rows?.records)) return rows.records; — a guard-clause spelling that a ternary-anchored regex misses.

The #14843 standard: every block driven, not inferred

IDataEngine.find declares Promise of any[], and that is not proof — #13706 is this repo's own find() that resolves an envelope and never an array. So a real ObjectQL was booted over a real SqlDriver (better-sqlite3 :memory:) and each block driven through the shipped function that owns it, with a recorder that forwards find verbatim and captures what came back. Attribution is by the source line that issued the call, not by my labels:

# block drive path call site observed shape resolved
1 seed-name-lookup.ts readNamePage buildExistingByName (batched) seed-name-lookup.ts:266 bare Array, len 1
2 seed-name-lookup.ts perItemIndex same, batched read refused so the per-item path is taken seed-name-lookup.ts:320 bare Array, len 1
3 bootstrap-declared-permissions.ts defaultLookup upsertPackagePermissionSet with no oracle (the ADR-0086 P2 publish-materializer path) bootstrap-declared-permissions.ts:86 bare Array, len 0
4 auto-org-admin-grant.ts tryFind reconcileOrgAdminGrant auto-org-admin-grant.ts:151 bare Array, len 0
5 claim-seed-ownership.ts idsFrom claimSeedOwnership, bulk write refused for its per-row-hook budget (the only path that reaches it) claim-seed-ownership.ts:345 bare Array, len 1
6 normalize-managed-by.ts tryFind normalizeManagedByVocab normalize-managed-by.ts:53 bare Array, len 1
7 security-plugin.ts loader SecurityPlugin.start() against the real engine, then its own loader security-plugin.ts:1284 bare Array, len 1

Every seam answered [object Array], no own records key, elements all objects — on a populated page and an empty one alike. Block 7 was driven with the plugin's own middleware really registered on the engine, since a middleware is what could rewrite the result.

Where a seam sits behind a trigger, only the trigger is shaped — the batched read is refused, or the bulk write is refused. The read under test always reaches the real engine.

The six dead limbs

The envelope limb is removed and each block keeps its existing disposition for a non-array. Removing dead code must not quietly convert a seam that gaps into one that invents an empty answer, so seed-name-lookup (both) and bootstrap-declared-permissions still answer unknown/null, and auto-org-admin-grant / claim-seed-ownership / normalize-managed-by still answer [] exactly as before.

The seventh — repaired in the opposite direction

security-plugin.ts's sys_permission_set loader mapped three different facts onto one value: the read succeeded on an empty catalog; the read threw (catch { rows = [] }); the read resolved something unreadable (?? []). Only the first is an answer.

This is the enforcement plane, and its own neighbouring comment already names the failure — dropping a row here "revokes standing access with no signal at the moment of loss". The swallow produced that loss for a whole page at once.

The refusal is not this seam's invention; its consumer already declared the handling. PermissionEvaluator.resolvePermissionSets catches a throwing loader, keeps the request fail-closed, and names the failure in a warn — because "without the warn, a transient DB error makes custom permission sets silently vanish and the resulting 403s are undiagnosable" (#2565). The loader swallowing its own read failure made that warn unreachable: the diagnostic this repo had already built could never fire. The read fault now propagates, and an unreadable page refuses with PermissionSetReadUnansweredError (DATABASE_ERROR, ADR-0112's existing vocabulary — no packages/spec change). A page carrying a non-row refuses too, where the trailing filter used to drop it in silence.

Enforcement is unchanged for every result the shipped engine produces; an envelope or a non-row element now refuses (fail-closed) where the old code read through it. An unanswered read still grants nothing; what changes is that the loss is now sayable. Measured in both directions by the Clause-② review, driving resolvePermissionSets through this PR's own dbLoaderFor(ORG) at HEAD and at the merge-base blob on five engine conditions: a healthy page, a thrown read and undefined resolve identically before and after (the warn count on a thrown read going 0 to 1); an envelope and a page carrying a non-object element granted at base and refuse at HEAD. Both of those are fail-closed and both are unreachable on the measured engine — which is exactly what the eleven pins establish — so this is a declared narrowing, not a discovered move. The changeset carries this same sentence verbatim.

#2565 is the binding consumer contract for this seam, and it holds verbatim. The 2026-08-11 store-fault ruling (#7505) is precedent for the direction only — its own scope line is readRowById and its callers, so it is not the ruling of record here.

Pins, and the mutation that proves each one discriminates

Seven blocks, eleven pins in engine-find-bare-array.pin.test.ts, each driving its own seam against the real engine. Every pin was then mutated and confirmed red — nine legs, each proving the mutation reached disk by exact substring count before running, and each restoring via git checkout HEAD -- ABSOLUTE_PATH and proving the file byte-identical to its HEAD blob afterwards:

leg mutation pin that went red
M1 readNamePage reads rows.records only batched read is live (no degradation warn)
M2 perItemIndex reads rows.records only per-item path resolves present
M3 defaultLookup reads rows.records only unreadable: 0 / seeded: 1
M4 tryFind reads rows.records only grant verdict granted
M5 idsFrom reads rows.records only row re-owned through the id page
M6 tryFind reads rows.records only legacy managed_by rewritten
M7a loader reads rows.records loads the DB-authored set
M7b loader restored to Array.isArray(rows) ? rows : rows?.records ?? [] both refusal pins
M7c loader's try/catch { rows = [] } restored propagation pin and the #2565 warn pin

M7c is the direct evidence for the direction: reinstating the swallow makes the repo's own diagnostic unreachable again. It was independently reproduced by the review.

No mutation crosses a package boundary — every mutated file is plugin-security's own source, imported by the pins through a relative specifier, so no dist/ stands between the mutation and the subject and no rebuild leg is owed.

Verification

  • pnpm --filter @objectstack/plugin-security test97 files / 1827 tests pass (exit 0)
  • pnpm --filter @objectstack/plugin-security typecheck — exit 0, and check:test-typecheck reports 0 files / 0 errors, so the new pins really are type-checked
  • Gate family re-derived from the actual changed files with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack — re-derived a second time after the diff grew, which raised it from 58 to 93 families. 93/93 exit 0, exit codes captured by redirect, never through a pipe.
  • Two gates went red first and were repaired, which is the control that their zeros are not blind: check:engine-double-contract (the pin's forwarding recorder is an engine double — its update/findOne/delete now open with the producer's own predicates, and the ledger gains two rows: the ratchet grows, it is not weakened) and check:system-context-census (pure line rot — this change adds a net +66 lines to security-plugin.ts, 77 added / 11 deleted, and every rotted anchor was off by exactly 66; repaired with the gate's own --fix, line numbers only, no prose).
  • check:dual-build-cjs-loads, check:i18n and check:type-check-debt first answered exit 3 = PREREQUISITE NOT MET = NOT MEASURED; the full build closure was then built (turbo run build, 71/71 successful) and all three re-run exit 0.
  • Seven families are NOT MEASURED and cannot be: their argv comes from the workflow ($RUNNER_TEMP, matrix-shard expansions) — three shard-attestation invocations, two test-completeness invocations, cross-package-test-inputs' CI invocation, and half-state-patrol's. CI measures them.

⛔ No gate was widened — #15094 ruled against it (precision 0.446 / 0.857 measured). ⛔ No packages/spec change. ⛔ Nothing outside plugin-security except the two ledger/anchor files the gates themselves rewrote.

Found, not fixed

Three of the six dead-limb blocks (auto-org-admin-grant.ts tryFind, claim-seed-ownership.ts idsFrom, normalize-managed-by.ts tryFind) still answer [] for a non-array, and their catch arms turn a failed read into [] too — a smaller instance of the same invention the seventh block was repaired for. Left alone deliberately: each is a behaviour change with its own blast radius, and this card's face is the dead limbs plus the one site it names. Filed as #15840 rather than ridden in here; the review has since measured that one of the three persists a withdrawal rather than gapping passively, and the PM is regrading that card accordingly.


Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y

os-warren and others added 3 commits September 5, 2026 09:08
…bs, and stop the permission-set loader inventing an empty page

Six of the seven `Array.isArray(x) ? x : x.records` blocks in this plugin carried
an unreachable envelope limb over an engine `find()` result. The limb is removed
on a MEASUREMENT rather than on the declared type: `IDataEngine.find` says
`Promise<any[]>`, but a declared type is not proof here — this repo also carries
a `find()` that resolves a `QueryResult` envelope and never an array. A real
`ObjectQL` over a real `SqlDriver` was booted and each seam driven through the
shipped function that owns it; every one answered a bare array with no own
`records` key, on a populated page and an empty one alike.

Each of the six keeps its existing disposition for a non-array. Removing a dead
limb must not quietly convert a seam that gaps into one that invents an empty.

The seventh block is the opposite defect and is repaired in the opposite
direction. `SecurityPlugin`'s `sys_permission_set` loader swallowed a thrown read
into `[]` and mapped an unreadable result to `[]` too, so three distinct facts
left by one door. This is the enforcement plane: "no permission sets" silently
withdraws grants that exist while every request still looks normal, and the
swallow made `PermissionEvaluator.resolvePermissionSets`' own "db lookup failed"
warn unreachable — the diagnostic this repo had already built for exactly this
loss. The read fault now propagates and an unreadable page is refused with
`DATABASE_ERROR`; a page carrying a non-row refuses too, where the trailing
filter used to drop it in silence. Enforcement is unchanged in both directions —
an unanswered read still grants nothing — but it is now sayable.

Seven pins, one per block, each driving its own seam against the real engine.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…er predicates, and re-anchor the rotted system-context rows

Two gate repairs on the #15598 pins, neither a behaviour change.

`check:engine-double-contract`: the pin file's `observed()` recorder forwards
every verb to a real `ObjectQL`, but a seam that merely forwards is exactly the
shape that reads as "not a double" and then admits a call the real engine would
refuse. Its `update`/`findOne`/`delete` now open with the producer's own
predicates (`assertEngineUpdateDispatch` / `assertEngineFindOnePredicate` /
`assertEngineDeleteDispatch` from `@objectstack/metadata-core`), and the ledger
learns about the newly pinned double — the ratchet grows, it is not weakened.

`check:system-context-census`: line rot, not a finding. The loader repair added
a net 66 lines to `security-plugin.ts` (77 added / 11 deleted), and every rotted
anchor the census reported was off by exactly 66. Repaired with the gate's own
`--fix`; only line numbers in `content/docs/permissions/system-context.mdx`
change, no prose.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
@github-actions github-actions Bot added the size/l label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/api/error-catalog.mdx (via DATABASE_ERROR (literal, a string literal in PermissionSetReadUnansweredError), sys_permission_set (literal, a string literal in start))
  • content/docs/data-modeling/drivers.mdx (via DATABASE_ERROR (literal, a string literal in PermissionSetReadUnansweredError))
  • content/docs/deployment/environment-variables.mdx (via sys_permission_set (literal, a string literal in start))
  • content/docs/permissions/authorization.mdx (via sys_permission_set (literal, a string literal in start))
  • content/docs/permissions/delegated-administration.mdx (via sys_permission_set (literal, a string literal in start))
  • content/docs/permissions/field-level-security.mdx (via statusCode (symbol, a field of class PermissionSetReadUnansweredError))
  • content/docs/permissions/permission-sets.mdx (via sys_permission_set (literal, a string literal in start))
  • content/docs/permissions/sharing-rules.mdx (via /sharing/rules/:idOrName/evaluate (route, bridged from symbol statusCode — its route source's handler names it))

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

  • content/docs/releases/v12.mdx (via sys_permission_set (literal, a string literal in start))
  • content/docs/releases/v13.mdx (via sys_permission_set (literal, a string literal in start))
  • content/docs/releases/v15.mdx (via sys_permission_set (literal, a string literal in start))
  • content/docs/releases/v17.mdx (via statusCode (symbol, a field of class PermissionSetReadUnansweredError), tryFind (symbol, a top-level function), sys_permission_set (literal, a string literal in start))

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
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 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 — 14 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 cee3961759160ed72aacb407f15288cbc018d2ebpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 93dc262c158434fab60e692acefde48c615beccb — the merge of head 8101c7dcb99faeeb70060f4159653e5906ac3702 into base cee3961759160ed72aacb407f15288cbc018d2eb, 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 93dc262c158434fab60e692acefde48c615beccb && git checkout 93dc262c158434fab60e692acefde48c615beccb
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cee3961759160ed72aacb407f15288cbc018d2eb 8101c7dcb99faeeb70060f4159653e5906ac3702 && git checkout -B drift-repro cee3961759160ed72aacb407f15288cbc018d2eb && git merge --no-ff 8101c7dcb99faeeb70060f4159653e5906ac3702

node scripts/docs-audit/affected-docs.mjs --json cee3961759160ed72aacb407f15288cbc018d2eb

⚠️ 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 cee3961759160ed72aacb407f15288cbc018d2eb → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

Clause-② contract review — PR #15838 (card #15598) — verdict: PASS

Tier: CONTRACT_REVIEW_TIER = claude-fable-5-1 (scripts/pm/dispatch-gates.mjs:8659). Evidence: the PM's attestation that this Agent call carried an explicit model: fable override, plus my own system-prompt identity (claude-fable-5-1) — override + self-report. Not read from get_session.

Subject: head 31f059524, merge-base d4f9b2a9d, detached worktree /home/user/objectstack-review-15838, 11 files +670/−35. Fences (plugin-auth, service-automation, service-analytics, packages/spec, content/docs/releases, content/docs/references) and ADR-0087 were verified by the PM before dispatch and are not redone here. origin/main moved during the review (#15826, #15827); the merge-base is fixed, and every number below was re-measured after the 11:30Z resume on a --force-rebuilt closure (turbo 18/18 rebuilt, 0 cached, 5m53s, concurrency 1, every dist mtime 13:12–13:15Z). The pre-kill runs (10:31–10:44Z, cache-restored closure) agree to the digit and are cited only as a cross-check.

1 · The refusal — enforcement measured in both directions (weighted first)

Method: a scratch vitest probe (unique name, deleted afterwards) boots the real SecurityPlugin over a real ObjectQL + SqlDriver (:memory:), seeds five sys_permission_set rows — active org-scoped, inactive, org-less residue, other-org, NULL-active — and drives PermissionEvaluator.resolvePermissionSets through the plugin's own dbLoaderFor(ORG) under five engine conditions. Run once at HEAD, once with security-plugin.ts checked out at the merge-base blob (5aef5433), file restored byte-identical to HEAD (44f433c7) afterwards.

engine condition resolved @ base resolved @ HEAD #2565 warn base → HEAD
A · healthy page ps_active, ps_nullactive, ps_residue identical 0 → 0
B · read throws [] [] 0 → 1
C · envelope { records } ps_active, ps_nullactive, ps_residue [] 0 → 1
D · page with a non-object element ps_active, ps_nullactive, ps_residue [] 0 → 1
E · undefined [] [] 0 → 1
  • On every result the shipped engine produces (A, B, E) enforcement is byte-identical — residue arm, ADR-0049 inactive drop, NULL-active and the tenant wall included — and the throw never escapes the request path: all three dbLoaderForContext call sites (security-plugin.ts:4794, :4814, :4959) feed resolvePermissionSets, whose catch is the loader's only consumer. No silent 403 becomes a hard failure; the 403 stays and gains the warn. Direction 1 (a grant that was issued is still issued) and direction 2 (a denial is still a denial) both hold on the reachable inputs.
  • C and D are where the blanket sentence "unchanged in both directions" over-claims. At base the envelope limb and the trailing r?.name === name filter read through those inputs and granted; at HEAD the page refuses, and on a mixed page the valid rows are withheld with the junk one. Three facts keep this non-blocking: it is fail-closed; both inputs are unreachable on the measured engine (which is precisely what the eleven pins establish); and both refusals are declared in the PR body and the changeset ("refuses an unreadable result", "a page carrying a non-row refuses too"). Declared narrowing, not a discovered move. ⇒ Ask before undraft (PM's call): sharpen the changeset line to "unchanged for every result the shipped engine produces; an envelope or a non-row element now refuses (fail-closed) where the old code read through it" — the changeset feeds release notes and the current wording is circular under the new classification.

2 · The two authorities

3 · Census — 7, zero delta, right anchor

git grep -n '\.records' over packages/plugins/plugin-security/src/*.ts minus tests, at d4f9b2a9d and at the dev's 5315098df: 9 lines / 6 files (the report says 8 — claim-seed-ownership.ts:180-181 and seed-name-lookup.ts:288-289 are two lines of one block each), resolving to 7 blocks; delta vs the card: 0. At HEAD: 0 source hits (the 2 remaining are the pin's own prose). The anchor matters: normalize-managed-by.ts:55 was if (Array.isArray(rows?.records)) return rows.records; — the guard-clause spelling a ternary-anchored regex misses; .records catches it. A wider -w records sweep at HEAD finds prose only.

4 · Driven, and the attribution

observed() forwards find verbatim, so seen is what ObjectQL produced, not a fixture. The seven attributed lines at 5315098df are each a ql.find( call site, checked one by one (seed-name-lookup.ts:266 / :320, bootstrap-declared-permissions.ts:86, auto-org-admin-grant.ts:151, claim-seed-ownership.ts:345, normalize-managed-by.ts:53, security-plugin.ts:1284). Behind-a-trigger seams spot-checked against source: block 2 refuses only the $in batched read → readNamePage answers unreadablebuildExistingByName degrades to perItemIndex (seed-name-lookup.ts:455-463), whose read reaches the engine; present discriminates against unknown. Block 5 refuses only the multi + where write with ERR_BULK_PER_ROW_HOOK_LIMIT, the sole route to idsFrom (claim-seed-ownership.ts:243-249); the read reaches the engine and the re-owned row is the discriminator. Block 3 passes no existingByName, the only path to defaultLookup (:226-228). Pin file: 11/11.

5 · The two red-first gates — repaired, not loosened

  • (a) check:engine-double-contract. Base ledger restored into the worktree → exit 1 naming exactly RETAINED [findOne] and RETAINED [update] for the pin file, nothing else; HEAD → exit 0 (self-test 0), 785 pinned / 133 debt / 3 exempt. engine-double-contract.baseline.json (the shrink-only DEBT ledger) is untouched; pinned.json (COVERAGE, grow-only) gained two rows, none removed, no count decreased. The ratchet grew. Incidental control: my probe file, whose verbs do not open with the predicates, reddened the gate with three PINNED rows while the PR's recorder did not; the gate result reported here was taken with the probe removed.
    Non-blocking: the recorder's delete: (o, id, opt) is the driver arity — the engine is delete(object, options) (engine.ts:12941, auto-org-admin-grant 的所有 revoke 都是静默空操作 —— ql.delete 调用签名与引擎不符(降级后仍是 tenant admin) #4640's exact confusion) — so the delete slice vetoed it out of scan scope rather than judging it. No pin exercises it, and it is stricter than the engine, not looser (an engine-shaped delete(o, { where, context }) through it would land as assertEngineDeleteDispatch(undefined) → reject). Fix to (o, opt) when the file is next touched.
  • (b) check-system-context-census. Base .mdx restored → exit 1, 16 problems; HEAD → exit 0 (self-test 0), 107 read sites, 141 anchors resolve. numstat for security-plugin.ts is 77/11 = +66; the .mdx diff with digits stripped is 7 rows, each exactly once as - and once as + — line numbers only, no prose.

6 · The disclosed defective proof — real mechanism

Reproduced on GNU grep 3.11: a two-line HEAD anchor passed as one -F argument against the M7c-mutated text → grep -F -c reports 1 (the second line 'sys_permission_set', survives the wrap, and -F splits its pattern at newlines into independent patterns), while python str.count reports 0. A tripped guard is a pre-run abort, so nothing was read from that leg; the substring-count proof is what I used for §2 as well. Trap worth naming for the rest of this lane.

7 · #15840's restraint

Right to leave; one reason under-stated and one grade under-called. None of the three has a consumer contract of the #2565 kind, so nothing settles the card — but two declare a disposition: normalize-managed-by.ts ("Best-effort and non-fatal, like the sibling boot reconcilers") and auto-org-admin-grant.ts's catch ("debug rather than warn — but it is no longer nothing (#4640)"). Those settle non-fatal, not report vs. silent. For the PM's grading of #15840: reconcileOrgAdminGrant's sys_member read (tryFind, :602) answering [] on a fault makes shouldGrant = false and enters the revoke branch (:713 ff.), which deletes a standing org-admin grant — a persisting withdrawal on a write path, healed only by the next successful reconcile. That is the seventh block's failure mode with a longer half-life; it belongs above "finding".

8 · Honesty audit

  • Families. My own dispatch-gates.mjs derivation on the 11 changed paths reconciles to 93 runnable + 7 workflow-only (the tool's own "⛔ NOT RUNNABLE LOCALLY"): 3× check-shard-attestation (dogfood matrix, dogfood-verify, test matrix), 2× check-test-completeness, check-cross-package-test-inputs, check-half-states — argv genuinely from $RUNNER_TEMP / ${{ matrix.shard }} / $PROVENANCE. True, not convenient. The report's prose says "two shard-attestation invocations"; it is three; the total of 7 is right.
  • CI on head 31f0595: 34 check runs, all success/skipped — Test Core 6/6, Dogfood 3/3, Dogfood Verify CLI, Type Check · debt ledger, Lint & Repo Gates — so the seven workflow-only families and the exit-3 trio (dual-build-cjs-loads, i18n, type-check-debt) are measured there. I did not build the 71-package closure: those three are NOT MEASURED by me locally.
  • exit 124: internally consistent (85 + 8 = 93) and explicitly excluded by the dev as a wrapper timeout, not a gate result; not independently verifiable — NOT MEASURED, taken as reported.
  • Run by me post-resume (exit codes by redirect, never through a pipe): pin file 11/11; M7c 2 red / 9 green; probe 5/5 × 2; check:engine-double-contract 0 (+ self-test, probe removed); check:system-context-census 0 (+ self-test); check:test-source-alias 0; check:agent-test-spelling 0; @objectstack/spec check:strictness-ledger 0; @objectstack/plugin-security test 97 files / 1827 tests; typecheck 0 including check:test-typecheck 0 files / 0 errors.

Verdict — PASS

Enforcement is unchanged on every input the shipped engine produces and moves only fail-closed on two inputs it cannot produce, both declared; both cited authorities exist and say what is claimed (one with a scope caveat); both red-first gates were repaired by growing the ledger and re-anchoring numbers, not by loosening anything. Requested before undraft, PM's call and not blocking the direction: sharpen the changeset sentence per §1. Non-blocking notes: recorder delete arity (§5a); "8 grep lines" → 9 (§3); "two shard-attestation" → three (§8); #7505 scope wording (§2b); #15840 grade (§7). ⛔ Nothing pushed, undrafted or merged; worktree left clean at 31f059524.


Generated by Claude Code — reviewer for the domain:services PM seat (PM session 03324ae2-0f5b-5ad2-8a2e-cf4aaff5a909)


Generated by Claude Code

os-warren and others added 2 commits September 5, 2026 13:28
…eachable input, fail-closed on the two that are not

Clause-② review measured `PermissionEvaluator.resolvePermissionSets` through
this PR's own `dbLoaderFor(ORG)` over a real ObjectQL/SqlDriver, at HEAD and at
the merge-base blob, on five engine conditions. A healthy page, a thrown read
and `undefined` resolve identically before and after — every result the shipped
engine actually produces. An envelope and a page carrying a non-object element
do NOT: they granted at base and refuse at HEAD.

Both are fail-closed and both are unreachable on the measured engine, which is
what the eleven pins establish, so this is a declared narrowing rather than a
discovered move — but "unchanged in both directions" is an unqualified claim
about behaviour and it sits in the changeset, which feeds release notes. The
reviewer's wording replaces it verbatim.

⛔ No source change: the refusal itself was reviewed and passed exactly as it
stands. Changeset and PR body carry identical wording.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…ugin-security-dead-limbs

# Conflicts:
#	content/docs/permissions/system-context.mdx
@os-warren
os-warren added this pull request to the merge queue Sep 5, 2026
Any commits made after this event will not be merged.

Copy link
Copy Markdown
Collaborator Author

Conflict resolved and re-armed — with the one blob that does not match, verified rather than waived

This PR was not queued, it was conflicted: GitHub reported mergeable_state: dirty while git merge-tree --write-tree origin/main <head> exited 0 with no conflict markers. The cause is that content/docs/permissions/system-context.mdx carries merge=os-regen in .gitattributes, and GitHub's server-side merge does not run local merge drivers — exactly the corollary #15885 exists to state. ⚠️ Recording it here because a local probe reading clean is the most misleading possible signal: it says "nothing to do" on a PR that will never merge. The other three PRs this seat had armed were probed with the same tool and returned 0 os-regen notes against this PR's 1, so the probe fires — it just does not answer GitHub's question.

Head is now 8101c7dcb99faeeb70060f4159653e5906ac3702, mergeable_state: clean, read from GitHub's PR object rather than from merge-tree. Auto-merge re-armed.

⚠️ One blob does NOT match the reviewed head, and that is correct

My standing check before re-arming is that packages/plugins/plugin-security/ is byte-identical to the reviewed head 31f059524. Seven of eight files match; security-plugin.ts does not (44f433c709a084ae26d2d588). That is unavoidable in a merge and is not a change to this PR: f9a3c3216 — PR #15878, card #15813, "the Layer 0 wall records its verdict on the operation" — is the only non-merge commit touching that path in 31f059524..HEAD, which I verified directly.

The author proposed a measure for "my contribution is unchanged" and I re-derived it independently rather than accepting it: hash the content lines of the delta, dropping the ---/+++ headers so @@ offsets cannot move it.

git diff origin/main HEAD   -- …/security-plugin.ts | grep -E '^[+-]' | grep -vE '^(\+\+\+|---)' | git hash-object --stdin
git diff d4f9b2a9d 31f059524 -- …/security-plugin.ts | grep -E '^[+-]' | grep -vE '^(\+\+\+|---)' | git hash-object --stdin
  both → 258a6439350d8e5420bebf14a4db56770fa65f92

⭐ With an unrelated file (errors.ts) run through the same measure as a control: 9a1097fee235…, different — so the match is a measurement, not a hash that collapses everything to one value. ⇒ The review's PASS stands on the authored change; what moved under it came from main.

The merge was NOT pure line rot, and the author flagged that rather than smoothing it

git merge origin/main conflicted (exit 1) and the driver reported the file mixed — generated anchors plus hand-written prose — so it text-merged instead of deferring. Two decisions in there deserve to be on the record:

  • The conflicted hunk was prose-identical on both sides, differing only in anchor numbers, and that was asserted mechanically (both sides normalised on .ts:N and compared) before a side was picked. ⛔ Only that hunk was resolved — never --ours across the file, which would have silently dropped main's cleanly-merged rows.
  • --fix then refused, and refusing was right: "the page anchors 9 distinct line(s) into this file, but the tree holds 8 anchorable line(s). The POPULATION changed, this is not a shift." Main's f9a3c3216 had added row 29, citing the same middleware short-circuit site as row 1; the naive resolution left two numbers for one site. ⇒ Pointing row 29 at row 1's site is a structural decision made by hand, after which --fix rewrote 10 anchors and row 29 landed on :1686 alongside row 1 — which is itself the confirmation that they are one site. ⛔ No number was hand-written.

⭐ A gate that refuses because the population changed, rather than shifting numbers to make itself green, is the reason this did not become a silent anchor rot. Worth remembering the next time one of these refuses.

Verification on the new head

Built before measuring, so no red is attributable to an unbuilt tree — and unlike the previous round, zero gates answered 3. Gate family re-derived with no stale-tree warning: 94 families, 94/94 exit 0. check-adr-0087-registration exit 0 with --self-test exit 0 as its control. check-regen-pending.mjs exit 0. ⚠️ The first pnpm gen:system-context-census answered exit 3 = PREREQUISITE NOT MET on a fresh worktree with no node_modules, was not read as a pass, and was re-run after pnpm install.

Two staleness notes, deliberately not fixed

  • The Verification section above still reads "93/93" and "58 to 93 families" — true of head 31f059524 when written, now 94/94. Left as-is rather than spending another cycle on a PR that is verified and about to land; corrected here.
  • The first commit's message still carries the pre-review "unchanged in both directions" phrasing. That matters only on a squash, and this repo does squash from concatenated commit messages (A claim corrected during review still ships in history: the squash message is built from commit messages, which no review round reads #15913) — but the landed message will read as claim-then-correction, the changeset that feeds release notes is right, and rewriting history across a merge commit risks more than the imperfection is worth.

Generated by Claude Code

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/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-security: 7 { records } union-normalizer blocks on find() results — dead limbs, one of them also #15092's DROP shape (security-plugin.ts:1292)

2 participants