fix(plugin-auth): refuse a self-registration grant on a malformed permission-set row, and remove the 14 dead find() envelope limbs behind it

[os-warren]

Fixes #15597

plugin-auth carried the repo's largest concentration of the #15094 census class: array-or-envelope union normalizers of the shape Array.isArray(x) ? x : x.records ?? [], whose envelope limb is dead code that reads as a contract. One of them was the opposite defect and is fixed in the opposite direction.

Review fix-up applied at 180187383 — three closes from the clause-② review, none of them a rework: the changeset now states the Case D refusal cost plainly; the ⭐ wrong-grant pin now resolves an organization so it drives the stamp that actually bit; and two prose counts are corrected (four guard-clause spellings, not five; 47 measured .find( sites, not an unmeasured "~140"). Details in the sections below.

The census is re-derived, and the card's number was stale

The card says 12 records-limb blocks plus a data-limb rowsOf. Measured on origin/main at claim time (c99449ab5) with a comment-masked scan over the package's 56 non-test source files, anchored on the limb rather than on Array.isArray, with a 5-line window so multi-line ternaries are seen:

14 blocks — 13 records-limb, 1 data-limb.

The delta is exactly one, and it reconciles: PR #15738 landed as aedbaeff1 hours before this round and added hasExistingUserFor, whose rows line is this shape. git merge-base --is-ancestor aedbaeff1 HEAD confirms it is in the base; the commit's own diff shows the added line. 12 + 1 = 13.

The repo-wide #15094 census is not undercounted, and nothing here suggests it is. That census is Array.isArray-anchored per its own Method section and both package counts re-derive exact (#15094 = 12+1 at ca46f8f12, before #15738 landed; #15598 = 7). A first pass at this scan found only 10 — that was an artefact of this round's own limb-anchored spelling, not a defect in the census: four of the fourteen are written as a guard clause (if (Array.isArray(raw)) return raw; const records = raw?.records; …) rather than a ternary. That spelling is the same defect and is included.

#	block	file	limb
B1	isBootstrapCreation	auth-manager.ts	records
B2	hasPendingInvitationFor	auth-manager.ts	records
B3	hasExistingUserFor (new in #15738)	auth-manager.ts	records
B4	findPermissionSetRows	auth-manager.ts	records
B5	settleSelfRegistrationGrant	auth-manager.ts	records
B6	tryFind	ensure-default-organization.ts	records
B7	findRows	tenancy-service.ts	records
B8	findRows	reconcile-membership.ts	records
B9	tryFind	backfill-account-issuer.ts	records
B10	the sys_member scan	member-role-canonical.ts	records
B11	asRows (guard clause)	dev-admin-seed-gate.ts	records
B12	asRows (guard clause)	walled-owner-verification-path.ts	records
B13	asRows (guard clause)	boot-sign-in-reachability.ts	records
B14	rowsOf (guard clause)	phone-sms-texts.ts	data

Re-running the same scan on this branch reports 0.

The engine was driven, not inferred

#14843's standard, and the clause the card says gets skipped: resolve the concrete engine each block reads and drive it once, because IDataEngine.find being declared to resolve to any[] is not proof — #13706 is this repo's counter-example.

All fourteen blocks resolve to one concrete engine: the ObjectQL instance the kernel registers as the objectql / data service. auth-plugin.ts obtains it with ctx.getService('objectql') and hands it to every standalone module; AuthManager reads this.config.dataEngine through withSystemReadContext, which forwards find without touching its result (it only merges a system context into the query).

Driven on a real ObjectQL over a real SqlDriver (better-sqlite3, :memory:), 23 reads — every block's exact object and query, populated and empty:

• every read answered a bare array; no records, no data, no bare-row lift
• every empty read answered [], never a nullish value, so the : [] tail was unreachable too

The static half agrees and is recorded for the reviewer: the only { records } producer anywhere in packages/** is metadata/src/loaders/database-loader.ts's queryHistory, a different port that no block here can reach. And no test fixture in the package ever fed an envelope to these ports either — the limbs were never exercised even in test.

The one place the limb was NOT impossible, and why removal is still right

ObjectQL.find returns hookContext.result on its hook path, and nothing re-checks it against the declared array. Driven: an afterFind hook assigning ctx.result = { records: [ … ] } really does make find() resolve to an envelope.

Nothing in this tree does that — the repo's only registered afterFind is plugin-audit's read recorder, which never touches ctx.result. And the limb did not repair that case, it masked it: a hook corrupting find() would be a contract violation, and the limb silently absorbed it at fourteen sites while this package's remaining find() call sites broke anyway (47 .find( sites in its non-test source in total, a measured count that already includes Array#find). Fourteen sites of false immunity is worse than one visible failure.

That seam is a real question and it is not this card's (this card forbids a spec change and a gate change), so it is filed separately as #15823 with the measurement attached. The review widened that finding with a second door on the same unguarded return — the middleware seam (executeWithMiddleware returning ctx.result, with security-plugin.ts:3143 assigning it, shape-preserving today) — and it is recorded on that card.

B5 is the opposite defect

settleSelfRegistrationGrant also carried #15092's DROP shape. Its candidate filter was:

const rows = (await this.findPermissionSetRows(staged.setName)).filter(
  (r) => r?.active !== false && typeof r?.id === 'string' && r.id,
);

active !== false is a genuine selection predicate — a deactivated set legitimately does not resolve — and it stays a filter. The id test is not a selection: it silently drops a malformed row, before the code decides which row to grant. Two ways that goes wrong, both silent, and both measured by ablation on this branch:

1. A false cause. With the malformed row as the only candidate, the family narrows to nothing and the operator is told Cause: no active sys_permission_set row named 'portal_user' resolves — while an active row named exactly that is present. That report is the only signal this path emits and nothing retries it, so a wrong cause is the whole failure.
2. A wrong grant. With the malformed row the organization-scoped one and a well-formed global row also carrying the name, dropping it let the organization_id == null arm match instead. The pin now resolves an organization (via getTenancy), which is the shape that actually bit, and the ablation output is verbatim: permission_set_id: "ps_global", organization_id: "org_1" — the global set granted and stamped with the organization, so the store ends up asserting that org_1 granted a set org_1 never declared. The rows.length === 1 arm has the same problem: it can fire on a family that was never singular, because the count is taken after the silent removal.

The fix refuses instead of resolving by accident: malformed candidates are detected, the grant is declined, and reportUngrantedSelfRegistrant names the malformed row. Same gap-not-a-silent-answer direction the rest of the method already takes. A well-formed family grants exactly as before.

The refusal has a cost, and the changeset states it rather than leaving it to release notes: a deployment whose sys_permission_set already holds an active, correctly-named row with a missing or blank id now gets a loud refusal where it previously got a grant — including Case D, a malformed global row sitting beside a well-formed org-scoped one, which used to be dropped harmlessly. That is deliberate (the old behaviour could not tell that family apart from the one where the drop granted the wrong set) and it is fully reversible with no code change: repair or delete the malformed row and the grant proceeds as before. Nothing is written while the refusal stands.

The #15587 pin

Untouched and green. B3's edit removes only the envelope limb from the rows line; the try/catch fall-through direction and its single log line are not changed. signup-existing-address-refusal.test.ts runs 8/8, case ⑦ included.

Tests — one pin per block, and every pin mutated

src/find-envelope-limb-removal.test.ts, 25 cases, all on a real ObjectQL + SqlDriver:

• 14 shape pins, one per block, each driving that block's exact read through that block's own call facade, populated and empty.
• 6 production-entry drives — probeHumanUsersPresence / probeSignInAccountsPresence, decideDevAdminSeedGate, loadPhoneSmsTemplateBody + seedPhoneSmsTemplates, resolveDefaultOrgId, backfillAccountIssuer + canonicalizeStoredMemberRoles, AuthManager.findPermissionSetRows.
• 4 behaviour pins for B5: well-formed still grants; a malformed sole row is refused and named; the ⭐ wrong-grant case (org-resolved, asserting by column that no row claims org_1 granted ps_global, not merely that the table is empty); and a deactivated row still reports the ordinary non-resolution, which is the boundary between the two filter clauses.

Mutation, both kinds:

• The 14 shape pins are mutated by an in-file control case that registers the afterFind hook above, drives all 14 reads under it, asserts each really returns an envelope, and asserts every pin's assertion rejects it. It uses the only mechanism in the tree that can actually produce the shape, so the pins are not tautologies.
• B5's fix is mutated by source ablation: the pre-fix filter restored, mutation proved on disk by marker counts and a changed git hash-object (30ee8736 → d30e2475), never by the editor's exit code; then restored with git checkout HEAD -- ABSOLUTE_PATH and proved by an empty git diff HEAD plus a hash back to the HEAD blob, under an EXIT INT TERM trap with absolute paths. Result: 2 of the 4 red, including the org-resolved ⭐ case, which reds with the stamped row quoted above. The other two stayed green by design: they are the no-regression and boundary guards, not discriminators. No rebuild was needed and none is claimed — the code under test is reached by a relative import, so vitest reads src, which the ablation itself proves by going red on a source-only change.

Gates

Family re-derived from the actual changed files with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no hand-built path list). Its first run refused politely: the tree was 8 commits behind and 7 gate scripts had moved. Merged origin/main and re-derived — 49 gates. All exit codes captured by redirect, never through a pipe.

All 49 green, plus scripts/check-adr-0087-registration.mjs green with its --self-test control green.

Two first returned exit 3 = PREREQUISITE NOT MET = not measured (check:dual-build-cjs-loads, check:published-readme-exports — both read built output, and 39 packages had no dist). Rather than report those as unmeasured, pnpm build was run to completion (72/72 tasks) and both re-run: both then genuinely exit 0. No gate is reported from a run whose exit was 3, 124 or 137.

On the fix-up commit 180187383: package suite 96 files / 2035 tests exit 0, typecheck exit 0 (its check:test-typecheck self-test control green), and the six gates that read the edited files re-run individually — check:changeset-gate-self-tests, check:nul-bytes, check:cross-package-test-inputs, check:pm-governed-prose, check:partof-closing-keyword, check:test-source-alias — all exit 0.

Scope

No packages/spec change. No gate change. plugin-security untouched. service-analytics and service-automation untouched. #15587's pin file unmodified. No exported type surface moved: the three narrowed local ports (BootProbeEngine, DevAdminSeedProbeEngine, PhoneSmsTemplateEngine) and the four private helpers appear 0 times in the built dist/index.d.ts, so no consumer can see them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y

[github-actions]

📓 Docs Drift Check

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

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

• content/docs/api/error-catalog.mdx (via sys_permission_set (literal, a string literal in findPermissionSetRows))
• content/docs/data-modeling/objects.mdx (via sys_user_permission_set (literal, a string literal in settleSelfRegistrationGrant))
• content/docs/deployment/environment-variables.mdx (via sys_permission_set (literal, a string literal in findPermissionSetRows), sys_user_permission_set (literal, a string literal in settleSelfRegistrationGrant))
• content/docs/deployment/self-hosting.mdx (via sys_invitation (literal, a string literal in hasPendingInvitationFor))
• content/docs/permissions/authorization.mdx (via sys_permission_set (literal, a string literal in findPermissionSetRows), sys_user_permission_set (literal, a string literal in settleSelfRegistrationGrant))
• content/docs/permissions/delegated-administration.mdx (via sys_permission_set (literal, a string literal in findPermissionSetRows), sys_user_permission_set (literal, a string literal in settleSelfRegistrationGrant))
• content/docs/permissions/permission-sets.mdx (via sys_permission_set (literal, a string literal in findPermissionSetRows), sys_user_permission_set (literal, a string literal in settleSelfRegistrationGrant))

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

• content/docs/releases/implementation-status.mdx (via sys_user_permission_set (literal, a string literal in settleSelfRegistrationGrant))
• content/docs/releases/v12.mdx (via sys_permission_set (literal, a string literal in findPermissionSetRows))
• content/docs/releases/v13.mdx (via sys_permission_set (literal, a string literal in findPermissionSetRows), sys_user_permission_set (literal, a string literal in settleSelfRegistrationGrant))
• content/docs/releases/v14.mdx (via sys_user_permission_set (literal, a string literal in settleSelfRegistrationGrant))
• content/docs/releases/v15.mdx (via sys_permission_set (literal, a string literal in findPermissionSetRows))
• content/docs/releases/v16.mdx (via sys_user_permission_set (literal, a string literal in settleSelfRegistrationGrant))
• content/docs/releases/v17.mdx (via tryFind (symbol, a top-level function), sys_invitation (literal, a string literal in hasPendingInvitationFor), sys_permission_set (literal, a string literal in findPermissionSetRows), sys_user_permission_set (literal, a string literal in settleSelfRegistrationGrant))

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

• 1 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 — 11 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 ec0a6e7bd90f11334967c85f37ee489a3808c0c8 → packageMentionDocs.

Which tree this was computed on

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

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

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

[os-warren]

Clause-② contract review — PR #15827 (card #15597) — verdict: PASS with it noted

Head reviewed: 37fa7a250 · merge-base d8d27768a · 12 files (+580/−77) · dedicated detached worktree, tree provably clean at the head after every mutation (git status --short empty, auth-manager.ts blob 30ee873… == HEAD).
Tier line: CONTRACT_REVIEW_TIER = 'claude-fable-5-1' (scripts/pm/dispatch-gates.mjs:9429) — override + self-report: the PM seat attests an explicit model: fable override on this Agent call; this reviewer self-reports identity claude-fable-5-1. get_session not used (in mode:subagent it measures the dispatching session — contract-review.md "降档保险丝"). This is not an "exact match" reading.
Independence pair: Implemented-by: claude/issue-15597-plugin-auth-dead-limbs (mode:subagent dev) · Reviewed-by: context-isolated review subagent of the domain:services PM seat, PM session 03324ae2-0f5b-5ad2-8a2e-cf4aaff5a909 (session_01XpTx2tbq3pZRYAdoGt6E6Y). Stated plainly: dev and reviewer are different subagent contexts under the same dispatching session; per contract-review.md the dev line is a branch (a subagent has no session of its own), so I have not treated this as the SELF-REVIEW case — the PM seat rules if it reads otherwise.

Already verified by the PM and not redone here: head match, merge-base, the three fences (plugin-security 0, packages/spec 0, signup-existing-address-refusal.test.ts 0 — I re-ran the fence grep once as a side effect: 0 files), ADR-0087 exit 0.

1 ⭐ The census methodology — tested, and the answer is better than the PR's framing

Two-anchor census (git ls-tree + git show at a ref, tests excluded; harness in my scratchpad, not committed):

ref · scope	limb-anchored single-line (Array.isArray(x?.records|data))	Array.isArray-anchored, ±4-line window, merged
d8d27768a · plugin-auth	10	14 — hit-for-hit with B1–B14
ca46f8f12 (the #15094 census commit) · plugin-auth	9	13
37fa7a250 (this head) · plugin-auth	0	0

• The dev's "a first pass found only 10" is real and reproducible. What the limb-anchored regex misses is the four helpers whose limb is spelled through a type cast — const records = (raw as { records?: unknown } | null | undefined)?.records; return Array.isArray(records) ? … : [] (boot-sign-in-reachability.ts:183, dev-admin-seed-gate.ts:98, walled-owner-verification-path.ts:242, and the data twin in phone-sms-texts.ts:158). The identifier adjacent to .records is a cast, not the tested variable, so \bID\??\.records cannot see it.
• The { records }-normalizer gate's population is app-showcase page modules, but all three instances found so far are outside it — #14460's stated evidence threshold is now met #15094's census is NOT undercounted on this evidence — and it is not ternary-anchored. Its Method section (comment 5547410463) defines detector C as "an Array.isArray(ID) test where the same ID is also read as .records or .data inside the same brace-balanced enclosing block", i.e. the corrected anchor. Ground truth at ca46f8f12: git merge-base --is-ancestor aedbaeff1 ca46f8f12 → no, fix(plugin-auth): refuse a sign-up for an address that already exists, instead of a 200 for a row never written #15738 had not landed, so plugin-auth held 13 blocks = 12 records + 1 data — exactly what the census reported (12 in Table 3, plus the phone-sms-texts.ts:157 rowsOf it filed separately as "a fourth same-signature instance"). It counted the three cast-spelled guard clauses.
• plugin-security: 7 { records } union-normalizer blocks on find() results — dead limbs, one of them also #15092's DROP shape (security-plugin.ts:1292) #15598's 7 is exact. The windowed anchor returns 25 hits in plugin-security; classified by hand, 18 are opCtx.data write-payload probes (false positives for this class) and 7 are real find()-result normalizers: auto-org-admin-grant.ts:152, bootstrap-declared-permissions.ts:94, claim-seed-ownership.ts:178, normalize-managed-by.ts:54 (a guard-clause spelling, inline rows?.records), security-plugin.ts:1292 (the DROP one), seed-name-lookup.ts:286 and :328. The plugin-security: 7 { records } union-normalizer blocks on find() results — dead limbs, one of them also #15092's DROP shape (security-plugin.ts:1292) #15598 worker should anchor on Array.isArray, as this PR did, not on the ternary — but the card's number needs no correction.
• Two prose errors in the PR body, table is right: "five of the fourteen use the guard-clause spelling" — the table marks four (B11–B14); the fifth thing a single-line ternary regex misses is B5's multi-line ternary (auth-manager.ts:4630–4634), which is not a guard clause. And "~140 other find() call sites in this package" — I count 47 .find( occurrences in non-test source, a share of them Array.prototype.find; the argument holds at any count > 14, the number is just wrong (it is repeated in [finding] ObjectQL.find returns hookContext.result unguarded, so an afterFind hook can make a find() declared to resolve to any[] resolve to an envelope instead — measured #15823).
• NOT MEASURED: the repo-wide 104 beyond these two packages (the census harness was never committed; my windowed anchor is an upper bound with heavy write-payload noise — 117 on packages/** — and cannot be compared to it).

2 ⭐ Pins-are-not-tautologies — the mechanism is real, by reading and by run

• Reading: triggerHooks (engine.ts:2828) calls entry.handler(context) on the shared hookContext; hookMatchesObject (:1622) treats an absent object as global, so the control's object-less afterFind fires on all 14 reads; find() then does maskSecretFields(object, hookContext.result) (tolerates a non-array: Array.isArray(rows) ? rows : [rows]), stripSearchCompanionFromRead(...), and return hookContext.result (:9360–9374) with no re-check. The control's per-block expect(Array.isArray(value)).toBe(false) guards that the hook took effect before the pin is tried; expectBareArray's first assertion is the limb's own Array.isArray test, so it throws on the envelope, the catch records nothing, and expect(survivors).toEqual([]) is a real assertion over a real list.
• Run (my worktree, real ObjectQL + SqlDriver, better-sqlite3 :memory:): find-envelope-limb-removal.test.ts 25/25, control included.

3 ⭐ #15823 — measurement confirmed; premise survives; one addendum

• ObjectQL.find returns hookContext.result unguarded on the hook path (engine.ts:9374); findOne has the identical seam (:9582–9593). Confirmed.
• Only registered afterFind in the tree, any spelling, any extension, non-test: plugin-audit/src/read-audit.ts:600. Its handler reads ctx.result (:565, :569) and never assigns it (grep for .result = in that file: none). Confirmed. Labels on [finding] ObjectQL.find returns hookContext.result unguarded, so an afterFind hook can make a find() declared to resolve to any[] resolve to an envelope instead — measured #15823: pm:queue only. Confirmed.
• Addendum for [finding] ObjectQL.find returns hookContext.result unguarded, so an afterFind hook can make a find() declared to resolve to any[] resolve to an envelope instead — measured #15823: there is a second door on the same unguarded return. executeWithMiddleware (engine.ts:3541–3576) also ends return ctx.result, and a middleware may assign ctx.result with or without calling next(). One find-path middleware does assign it today — plugin-security's field masker, security-plugin.ts:3143 — and it is shape-preserving (field-masker.ts:94 maps arrays with results.map(...)). So "the only mechanism in the tree" in the pin file's header is slightly overstated; both doors converge on one return, which strengthens fork option 1 (guard at find()'s return closes both).
• My read on the class: the premise survives. "Dead" here means (a) no producer of the shape exists in the tree, measured, and (b) the limb was never a repair for the one hypothetical producer — a reshaping hook would break every un-limbed find() caller in the same package anyway. The dev's "fourteen sites of false immunity is worse than one visible failure" is the right frame; the pin file now makes such a hook fail at the pin, with a readable message, rather than downstream.

4 B5 — the DROP shape reproduces verbatim; the ⭐ pin does not drive the mechanism the PR describes

Source ablation in my worktree (pre-fix filter restored by script; on disk 1 file changed, 2 insertions(+), 14 deletions(-), blob 30ee873… → 7e6c34b…; restored with git checkout HEAD --, diff 0 lines, blob == HEAD):

• 2 red / 2 green, exactly as claimed. Red: malformed sole row — Cause: no active sys_permission_set row named 'portal_user' resolves (the false cause, with an active row of that name present); wrong-grant case — granted anyway: [{… "permission_set_id":"ps_global","organization_id":null}]. Green: well-formed (grants ps_ok under both filters — a no-regression guard) and deactivated (the retained active !== false clause behaves identically under both filters — the boundary guard). That classification is honest: neither of the two can discriminate this fix by construction, and neither is presented as doing so.
• ⚠️ But read the red evidence again: organization_id: null. The pin file's settle() helper wires no getTenancy, so no organization is ever resolved in that case, and with no org the pre-fix code's organization_id == null arm is the method's documented fallback, not a bypass. I drove the org-resolved scenarios myself (scratch test, both trees, then deleted):

case (set portal_user)	org resolved	HEAD	pre-fix filter
A. well-formed org row + global	org_1	grants ps_org / org_1	grants ps_org / org_1
B. malformed org row + global	org_1	refuses, names the row	grants ps_global stamped organization_id: "org_1" ← the real wrong grant
C. well-formed org row + global	none	grants ps_global / null	grants ps_global / null
D. malformed global + well-formed org row	org_1	refuses	grants ps_org / org_1 (correct)

Case C shows the ⭐ pin's setup grants ps_global on both trees when the org row is well-formed — so that pin reds on "malformed → refuse" only; the org-arm bypass the PR narrates (case B) is real, is closed by this fix, and is not pinned. Fix is a 3-line follow-up: give the ⭐ case getTenancy: () => ({ defaultOrgId: async () => 'org_1' }) and assert the ablation evidence carries organization_id: "org_1".

• Case D is a behaviour cost the changeset states only generically ("a malformed row is no longer a selection at all — the grant is refused") and whose other sentence undersells: a malformed sibling now blocks a well-formed, correctly-scoped row the old filter granted, so "a well-formed family grants exactly as before" holds only for families with no malformed member. The direction is still gap/refusal, loud, names the row, and is reversible by fixing the row — right per the card — but that sentence should say so.

5 The one it did NOT change — verified by drive, correct call, one residual

selfRegistrationSetResolvable (:4372, .some(active && usable id)) driven on the same families: sole malformed → false (self-registration refused — fail-closed); deactivated → false; malformed org row + well-formed global → true (admits; B5 then refuses the grant with the named cause). No family makes it admit on a malformed row, so leaving it is right. Residual, not a defect: on the mixed family admission and grant now disagree (admitted, then ungranted with an error-level report) — that is the pre-existing "declared≠enforced" reporting path, and tightening admission is a contract change outside this card.

6 The engine claim — wiring confirmed, both halves measured

auth-plugin.ts:407 ctx.getService('data') → AuthManager.config.dataEngine; every standalone module gets ctx.getService('objectql') (:712, :1043, :1100, :1129, :1150, with 'data' as the fallback); objectql/src/plugin.ts:421/423 registers the same this.ql under both names; withSystemReadContext = withSystemContext forwards find: (m, q) => e.find(m, asSystem(q)) untouched. One concrete engine. SqlDriver.find → findRows → knex array; ObjectQL's non-hook path return opCtx.result as any[] (:9381). Empty reads: the 14 pins' empty half plus my own five query shapes ({limit:1}, {limit:50}, {where,limit:1}, {limit:100,offset:100}, {where,limit:5000}) all answered [], never nullish — the : [] tail was unreachable.

7 Honesty audit

• check:test-typecheck "first failed with 1 type error naming the pin file": consistent — root tsconfig.json:15 noUnusedLocals: true, and commit e9993ef72 removes an unused vi import from the pin file. Re-run here: pnpm --filter plugin-auth typecheck (tsc --noEmit + examples + check:test-typecheck --self-test + package run) exit 0.
• Gates: re-derived off the merge base on the head tree with --repo objectstack-ai/objectstack --json: 12 paths, 55 commands = 41 gates + 14 --self-test controls, with the script's own STALE-TREE warning (this head is 6 commits behind today's origin/main; expected for a head-pinned review). The dev's 49 is not reproduced and check:published-readme-exports is not in my derivation — most plausibly a derivation taken before the changeset file was committed; unexplained, recorded. Ran all 55 sequentially, exit codes by redirect: 53 of 55 exit 0; the two non-zero are both exit 3 = PREREQUISITE NOT MET on my 28-package build closure — check:dual-build-cjs-loads (40 packages with no dist/) and check:type-check-debt (--re-measure refuses because six workspace dependencies of the ledgered packages have no built .d.ts: driver-turso, runtime, service-automation, service-cluster, service-job, service-settings). Both are NOT MEASURED here, not findings; the dev built 72/72 and reports both 0, and CI's "Type Check · debt ledger" job is green on this head.
• Exit-3 semantics exist in both scripts the dev named (check-dual-build-cjs-loads.mjs:1126, check-published-readme-exports.mjs:346); 124 as the timeout wrapper's code is standard. NOT MEASURED here: check:dual-build-cjs-loads and check:type-check-debt (exit 3 above) and check:published-readme-exports (not in my derivation at all; dev reports 0).
• "No downstream typecheck sweep" justified: dist/index.d.ts (built on this tree) contains 0 of BootProbeEngine, DevAdminSeedProbeEngine, PhoneSmsTemplateEngine, asRows, rowsOf, tryFind, findRows — the three ports' modules are not exported from src/index.ts and the helpers are module-private — while the five exported functions whose modules changed are still present. And CI's four Type Check jobs are green on this head, which is the downstream sweep.
• Disclosure on my side: my dependency build was 27/27 turbo cache hits (content-hashed, so the dist matches this source), not a fresh compile.

8 #15587 case ⑦

signup-existing-address-refusal.test.ts 8/8 on my run, ⑦ included; B3's diff removes only the normalizer line, the try/catch fall-through and its single log line are untouched.

Contract judgment (contract-review.md ①②③)

① Derived judgments: 14 limb removals — right, engine driven (populated + empty) on the one concrete engine; three local ports narrowed to Promise<Array<Record<string, unknown>>> — right, unexported (d.ts 0); tryFind/findRows now return await ql.find(...) inside try — right, and a hypothetical nullish resolve fails closed in every caller (.some/.filter throw into the admission gate's or settle's catch); B5 refusal — right direction, broader than the two defects (case D) and the ⭐ pin drives the wrong variant (§4). ② Semver: patch — correct; no exported type moved, behaviour change is a bug-fix refusal on malformed data. ③ Boundary flags: #15823 filed out of scope with pm:queue only — right call, with the middleware-door addendum above; no open_questions raised by the dev.

Blocking bar, stated for the two B5 findings (an unpinned fix and an under-declared behaviour cost are the two shapes this lane holds PRs on, so the call is made explicitly rather than left inside "noted"):

• Unpinned org-arm variant — not blocking. The fix's mechanism is "any malformed candidate ⇒ refuse", keyed on malformed presence alone, not on org resolution; the existing ⭐ pin reds under ablation on exactly that mechanism, so a regression of the fix cannot pass the file as it stands. What is unpinned is the scenario the narrative describes, which the same mechanism covers (HEAD refuses case B, measured). That is a tests-accuracy defect, not a coverage hole over the fix. Smallest close: the 3-line getTenancy addition above — ideally before landing, so the evidence line and the PR text agree.
• Case D refusal cost — not blocking. The cost falls only on a family that already contains a malformed row — an operator error state, not a valid configuration — and replaces a pre-fix behaviour that on that same family was silently right (D) or silently wrong (B) depending on which row was malformed. Loud-always-refuse on malformed data, error-level, naming the row, reversible by fixing the row, is the card's stated direction (gap/refusal). It would block if the refusal reached a well-formed family, or if the changeset denied the rule; it does neither — its rule sentence is generic and its "exactly as before" sentence is what needs the one-line qualification.

Verdict: PASS with it noted. The three things the PM made the outcome turn on all hold — the census claim (and #15094/#15598 with it), the tautology guard, and B5's mutation. Noted, and owed as one small follow-up commit (they do not change the accept/reject judgment): (i) wire getTenancy into the ⭐ wrong-grant case so the org-arm bypass is what reds; (ii) one changeset sentence for case D; (iii) the "five"/"~140" prose. CI on the head: 33 check runs, all completed, none failed (Test Core 6/6 shards, Lint & Repo Gates, Type Check ×4, Build Core). Unexplained and recorded, not papered over by the tally: the dev's 49-gate count vs my 55-command derivation on the same head (§7).

---

Generated by Claude Code

---

Generated by Claude Code

[os-warren]

Fix-up for the clause-② review — three closes, no rework

New head: 180187383471faa7c19c7495f5374717c2c3f8a3 (180187383). PR left as a draft. Nothing re-measured that already held; no scope change; plugin-security, packages/spec, content/docs/releases/ and #15587's pin file all untouched.

1. The changeset now states the Case D refusal cost

Added as an explicit Upgrade note, so it reaches release notes as a behaviour change rather than as generic prose:

Upgrade note — one family now gets a refusal where it previously got a grant. If a deployment's sys_permission_set already contains a row that is active and carries the declared name but whose id is missing or blank, self-registration grants against that name now stop and report, including the case where the malformed row is one nobody was relying on: a malformed global row sitting alongside a well-formed organization-scoped row used to be dropped silently, letting the org row be granted, and is now refused. This is deliberate — the old behaviour could not tell that family apart from the one where the silent drop granted the wrong set — and it is fully reversible without a code change: repair or delete the malformed row and the grant proceeds exactly as before. The refusal is loud and names the row, so it is visible rather than something to discover later; nothing is written while it stands.

Which family, what they now get, and that it is reversible — all three stated.

2. The ⭐ pin resolves an organization, and reds on the stamp

settle() gained a getTenancy seam read exactly the way the method reads it (this.config.getTenancy?.() then defaultOrgId()), and the ⭐ case passes 'org_1'. The assertion is now by column, not by row count:

expect(rows.some((r) => r.permission_set_id === 'ps_global' && r.organization_id === 'org_1')).toBe(false)

Shown red under the same B5 ablation (pre-fix filter restored, nothing else changed):

HEAD blob:     30ee8736580debe45d2b3b6444c5922c1e73c939
pre-mutation:  30ee8736580debe45d2b3b6444c5922c1e73c939
mutation proof: injected=1 (want 1) · fix-marker-remaining=0 (want 0)
post-mutation: d30e2475dbe313ffc720a0f2b97b6a7b14d64e7a
MUTATED_RUN_EXIT=1
post-restore:  30ee8736580debe45d2b3b6444c5922c1e73c939
restore proof: git diff HEAD empty AND hash == HEAD blob

Tests 2 failed | 3 passed | 20 skipped, and the ⭐ failure now quotes the grant that actually bit:

AssertionError: granted anyway: [{"id":"ups_mto87kj1yr80h1gh", …,
  "user_id":"usr_new","permission_set_id":"ps_global","organization_id":"org_1"}]

organization_id: "org_1", not null — the org-stamped wrong grant is pinned directly now rather than leaning on its sibling. The other two B5 cases stay green under ablation by design (no-regression and boundary guards).

3. Two prose numbers corrected

was	now	how
"five guard-clause spellings"	four	the fifth match was member-role-canonical.ts's if (Array.isArray(raw)) return raw.join(',') — a string joiner, not an envelope limb. Re-derived at the base commit.
"~140 other find() call sites"	47 .find( sites in non-test source, a count that already includes Array#find	measured; the old figure was never measured.

Fixed in the changeset, the test-file header, and the PR body. One more in the same sentence while I was there: the changeset said "the four local engine ports" while naming three — corrected to three (WalledOwnerProbeEngine is an alias of BootProbeEngine, not a fourth port).

I also folded your census finding into the PR body: the repo-wide #15094 census is not undercounted — it is Array.isArray-anchored per its own Method section and both package counts re-derive exact — and my "only 10" was my own limb-anchored first pass. The body now says so, so the record does not leave a false doubt about the class's premise.

Verification on 180187383

Exit codes captured immediately after a single redirected command, never off a multi-command line, and never through a pipe.

run	exit
pnpm --filter @objectstack/plugin-auth exec vitest run src/find-envelope-limb-removal.test.ts	0 — Test Files 1 passed (1) / Tests 25 passed (25)
pnpm --filter @objectstack/plugin-auth test	0 — Test Files 96 passed (96) / Tests 2035 passed (2035)
pnpm --filter @objectstack/plugin-auth typecheck	0 — check:test-typecheck: OK, its --self-test control green
check:changeset-gate-self-tests · check:nul-bytes · check:cross-package-test-inputs · check:pm-governed-prose · check:partof-closing-keyword · check:test-source-alias	0 each

The dependency-closure build was re-run first (exit 0, VERDICT line read from the lock wrapper) because the worktree was recreated — an earlier run failed to resolve @objectstack/objectql and reported Tests no tests, which I read as NOT WIRED, not a pass, and did not report as one.

Noted, nothing done: the middleware seam (executeWithMiddleware returning ctx.result) added to #15823's record, and the shared-scratchpad filename hazard — everything I wrote this round is suffixed -15597.

---

Generated by Claude Code

[os-warren]

PM verification of the review fix-up — undrafted and armed

All three closes verified independently at head 180187383471faa7c19c7495f5374717c2c3f8a3 before undrafting. No third review round: the fix-up touches two files, neither of them production source — .changeset/plugin-auth-find-envelope-limbs.md and find-envelope-limb-removal.test.ts, +39/−6. auth-manager.ts is byte-identical to the reviewed revision, so the review's PASS on the behaviour change still stands on the code it was given.

Scope fences on the incremental diff 37fa7a250 → 180187383, all 0: ^packages/spec/, content/docs/releases, packages/plugins/plugin-security/, packages/services/service-analytics/, packages/services/service-automation/, scripts/.

node scripts/check-adr-0087-registration.mjs --base origin/main --head 180187383… → exit 0, ✓ this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).

The two corrected numbers were re-derived here, not taken on report. At the merge-base d8d27768a:

• "four" guard-clause spellings, not five. A git grep -A2 'if (Array\.isArray(' over the package's non-test source returns exactly four blocks whose next return is a records/data limb — boot-sign-in-reachability.ts:183, dev-admin-seed-gate.ts:98, phone-sms-texts.ts:158 (the data one), walled-owner-verification-path.ts:242. The fifth match the earlier count included, member-role-canonical.ts:125, is if (Array.isArray(raw)) return raw.join(',') falling through to typeof raw === 'string' ? raw : null — a string joiner on a scalar, not an envelope limb. Two further Array.isArray guards in the same scan (impersonation-bearer-rotation.ts:142, two-factor-rotated-token-echo.ts:84) fall through to joined ? [joined] : [] and are likewise not limbs. Four is correct.
• 47 .find( sites. git grep -o '\.find(' over the same file set returns 47, exact — with a deliberately-impossible control pattern in the same scan returning 0, so the 47 is not a silent no-match. The previous "~140" was unmeasured.

One observation, recorded rather than held. The ⭐ pin's substance is the getTenancy seam: without a resolved org the wrong-grant case cannot produce the stamped row at all, so that addition is what makes the case drive the shape that actually bit, and the ablation output (permission_set_id: "ps_global", organization_id: "org_1") is the proof. The new by-column expect(rows.some(…)).toBe(false) sitting below expect(rows).toEqual([]) is defensive only — the stricter assertion above it fires first under ablation and vitest never reaches it, and the quoted failure text comes from that line's own granted anyway: message. So the test comment's "Asserted by column below, not just by row count, so the stamp itself is pinned" attributes the pinning to the wrong line. Harmless — the stamp is pinned, by the getTenancy seam plus the existing assertion, and the by-column check becomes live if anyone ever relaxes the line above it — but the record should not leave a reader thinking that assertion is the discriminator. Not worth another push cycle on a verified PR; noted here so it is accurate.

The census question that opened this round is settled in the body and I agree with the resolution: #15094 is not undercounted. It is Array.isArray-anchored per its own Method section, both package counts re-derive exact, and the "only 10" was an artefact of this round's limb-anchored spelling.

Undrafted and auto-merge armed at 10:34Z.

---

Generated by Claude Code