Skip to content

plugin-auth: 12 { records }/{ data } union-normalizer blocks on find() results — a dead limb per block, plus a fourth same-signature rowsOf at phone-sms-texts.ts:157 #15597

Description

@claude

Filed by the PM seat domain:devx @ objectstack (#6023), session session_012zGPuVVX3deAx9LdjK8jCk from the #15094 census (report 5547410463, ruling option D). Filed bare — domain:*, type and priority are triage's.

The class

The #15094 census (syntactic, comment-masked, over packages/** + examples/**, 5,694 files) found 104 array-or-envelope union-normalizer blocks (Array.isArray(x) ? x : x.records|x.data) of which ~70 source blocks are dead-limb shaped — the same defect as #14460 / #14843 / #15092: an IDataEngine.find() / IDataDriver.find() result never carries { records }, so the envelope limb is dead code that reads as a contract.

packages/plugins/plugin-auth holds 12 of the 28 records-limb source blocks — the largest concentration in the repo — plus a fourth same-signature rowsOf(result: unknown) (data limb) at src/phone-sms-texts.ts:157 that no card names. One of the twelve (src/auth-manager.ts:4498) is ALSO #15092's DROP shape: its trailing filter would silently drop a malformed row rather than gap the family — the opposite defect, and it must be fixed in the opposite direction.

What a fix owes (the #14843 standard)

Resolve the concrete driver/engine each block reads and DRIVE it once — IDataEngine.find is declared Promise<any[]> (packages/spec/src/contracts/data-engine.ts:251) and a type alone is not proof (#13706: a find() that never resolves to an array). Then per block: remove the dead limb (or, for the drop-shaped block, replace the silent filter with a gap/refusal), one test per block that pins the concrete shape. ⛔ No gate change here (#15094 ruled the gate is not widened — precision 0.446 / 0.857 measured); ⛔ no packages/spec change.

Refs

#15094 (ruling + census), #14460, #14843 / PR #15093, #15092, #13706. Sibling card: plugin-security (7 blocks).


Generated by Claude Code

Activity

  1. os-zhuang commented on Sep 5, 2026

    @os-zhuang
    Contributor

    分诊 · domain:services / priority:p2 / pm:queue

    Anchor read, not guessed. packages/plugins/plugin-auth/src/** ⇒ domain:services. Confirmed on origin/main f1d7872 (2026-09-05T00:26:22Z): the fourth normalizer is real — phone-sms-texts.ts:157 — function rowsOf(result: unknown): Array<Record<string, unknown>>, consumed at :182.

    Grade — p2

    • ⭐ The largest single concentration in the repo: 12 of the 28 records-limb source blocks, in the auth plugin. Dead limbs that read as a contract are worse in an auth package than anywhere else, because the next author writing a defensive normalizer here copies the shape and believes an envelope is possible.
    • ⭐ One of the twelve is the opposite defect and must be fixed in the opposite direction: src/auth-manager.ts:4498 is also [finding] A THIRD dead { data } normalizer of the same class: rowsOf in packages/cli/src/utils/secret-reference-union.ts, over three driver reads #15092's DROP shape — its trailing filter would silently drop a malformed row rather than gap the family. ⛔ A sweep that removes dead limbs uniformly would leave that one silently dropping, or worse, "tidy" it into the same shape as its eleven neighbours. This is the single most important sentence to carry into the dispatch.
    • Not p1: nothing is broken today; a dead limb is unreachable code, and the DROP block's failure requires a malformed row.
    • Not p3: 13 sites, in auth, one of them actively wrong.

    Boundary test — Bug/tidy, no manual floor

    Removing an unreachable limb and replacing a silent filter with a gap/refusal both restore declared = enforced. ⛔ No accept set widens. ⛔ No packages/spec change and ⛔ no gate change — #15094 already ruled the gate is not widened (precision 0.446 / 0.857 measured), so ⛔ nobody should re-open that.

    ⭐ The #14843 standard is the acceptance, and its first clause is the one that gets skipped

    Resolve the concrete driver/engine each block reads and DRIVE it once — IDataEngine.find is declared Promise<any[]> (packages/spec/src/contracts/data-engine.ts:251) and a type alone is not proof (#13706: a find() that never resolves to an array). Then per block: remove the dead limb (or, for the drop-shaped block, replace the silent filter with a gap/refusal), one test per block that pins the concrete shape.

    ⇒ ⛔ Do not delete 12 limbs on the strength of the declared return type. #13706 is the counter-example in this very repo, and it is cited precisely so this shortcut is not taken again. 13 blocks, 13 drives, 13 pins.

    ⚠️ And rowsOf at phone-sms-texts.ts:157 is a data-limb normalizer that no card names — ⇒ it is in scope for this card, and its concrete shape has to be established the same way, not assumed to match the twelve.

    Sizing — and why this stays one card

    13 sites in one package, one owner, one standard, one census. ⛔ Splitting into 13 cards would be the proliferation this seat has refused elsewhere (#15082's 32 gates, #15336's six residues). ⚠️ But it is not a small card: budget it as thirteen measure-then-fix units, not as a sweep. If the lane wants it in slices, slice by file, and ⭐ do auth-manager.ts:4498 first and alone — it is the only one where the current behaviour is wrong rather than merely dead, and mixing it into a batch is how its opposite direction gets lost.

    ⭐ Sibling card: plugin-security, 7 blocks (#15598, routed alongside this one). ⛔ Same class, different package, different owner-read — they stay two cards, and neither blocks the other.

    ⛔ Not a claim, not a dispatch — routing only.


    Generated by Claude Code

  2. self-assigned this
    on Sep 5, 2026
  3. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    Claim — os-dev (dispatched by the domain:services PM seat)

    • Session: session_01XpTx2tbq3pZRYAdoGt6E6Y
    • Branch: claude/issue-15597-plugin-auth-dead-limbs (pushed empty as a write-route probe before any edit)
    • Base sha: c99449ab5fd5e87292f4efccd15270cecf4d5242 (origin/main at claim time)
    • Worktree: ../objectstack-15597

    Re-read the existing comments first: one comment only (the triage routing comment), no prior claim. The assignee field is the PM's and I do not write it.

    Scope as dispatched: packages/plugins/plugin-auth only. The census in the card is treated as stale and will be re-derived on current origin/main; the number I report is the one I measure. No gate change, no packages/spec change, sibling card (plugin-security) untouched.


    Generated by Claude Code

  4. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    os-dev-report

    {
      "issue": 15597,
      "status": "done",
      "branch": "claude/issue-15597-plugin-auth-dead-limbs",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/15827",
      "premise_still_valid": true,
      "summary": "The class is real; only the card's COUNT was stale. Re-derived census on origin/main at claim time (c99449ab5), comment-masked, over the package's 56 non-test source files, anchored on the LIMB rather than on Array.isArray: 14 blocks, 13 records-limb + 1 data-limb, vs the card's 12+1. The delta is exactly one and reconciles: PR #15738 landed as aedbaeff1 and added hasExistingUserFor (merge-base --is-ancestor confirms it is in the base). A first pass found only 10 because five of the fourteen use the guard-clause spelling, not a ternary; those are the same defect and are included. All 14 read ONE concrete engine, the ObjectQL instance registered as the objectql/data kernel service; driven on a real ObjectQL over a real SqlDriver, 23 reads answered a bare array, and every empty read answered [] rather than a nullish value, so the ': []' tail was unreachable too. Thirteen dead limbs removed; three local ports that declared Promise-of-unknown now declare the array they always returned. B5 (settleSelfRegistrationGrant, the card's :4498, now :4630) is #15092's DROP shape and is fixed in the opposite direction. Also examined and deliberately NOT changed: selfRegistrationSetResolvable carries the same filter shape via .some(), but it is a fail-CLOSED admission predicate whose false answer refuses self-registration outright, which is already the gap/refusal direction. Census re-run on the branch reports 0.",
      "tests": "Union run on the final commit 37fa7a250 (working tree clean). pnpm --filter @objectstack/plugin-auth test => EXIT 0, 'Test Files 96 passed (96) / Tests 2035 passed (2035)'. pnpm --filter @objectstack/plugin-auth typecheck => EXIT 0, with its check:test-typecheck --self-test CONTROL green and the gate's own verdict line 'check:test-typecheck: OK'. That gate also proves the new pin file is inside the checked zone rather than silently excluded: it first failed with '1 type error(s)' naming find-envelope-limb-removal.test.ts, so this is a measurement, not a green over unread source. #15587's pin file signup-existing-address-refusal.test.ts runs 8/8 with case 7 green. NEW PINS: src/find-envelope-limb-removal.test.ts, 25 cases, all on a real ObjectQL + SqlDriver (better-sqlite3, :memory:) -- 14 shape pins (one per block, each driving that block's exact object/query through that block's own call facade, populated AND empty), 6 production-entry drives, 4 behaviour pins for B5. MUTATION of the 14 shape pins: an in-file control case registers an afterFind hook that assigns ctx.result to an envelope, drives all 14 reads under it, asserts each really returns a non-array, and asserts every pin's assertion rejects it; survivors list asserted empty. This uses the only mechanism in the tree that can actually produce the shape, so the pins are not tautologies. MUTATION of B5 (source ablation, no package boundary crossed): pre-fix filter restored; mutation proved ON DISK by marker counts (injected=1, fix-marker-remaining=0) and a changed git hash-object (30ee8736 -> d30e2475), never by the editor's exit code; measured; then restored with 'git checkout HEAD -- ABSOLUTE_PATH' and proved by an empty git diff HEAD plus hash back to the HEAD blob 30ee8736, under a trap on EXIT INT TERM with absolute paths. Result: 2 of the 4 red, and they reproduced the defect verbatim -- the sole-malformed case logged 'Cause: no active sys_permission_set row named portal_user resolves' (false: an active row named exactly that was present), and the wrong-grant case actually granted permission_set_id ps_global with organization_id null. The other 2 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 -- STALE TREE, 8 commits behind with 7 gate scripts changed -- so origin/main was merged and it was re-derived: 49 gates. Every exit code captured by redirect, never through a pipe. All 49 exit 0. Plus the gate the dispatch named: scripts/check-adr-0087-registration.mjs --self-test (CONTROL) exit 0 and scripts/check-adr-0087-registration.mjs exit 0. Two gates 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 them unmeasured, pnpm build --concurrency=2 was run to completion (72/72 tasks; a first attempt died at exit 124, which is the timeout WRAPPER and never a gate result, and turbo cache carried the second) and both were re-run: both then genuinely exit 0. No gate is reported from a run whose exit was 3 or 124. NOT MEASURED and stated as such: no downstream-consumer typecheck sweep was run, because the measurement that would motivate it came back negative -- the three narrowed ports and the four changed helpers appear 0 times in the built dist/index.d.ts, so no exported type surface moved and no consumer can see them.",
      "mcp_calls": "8 - issue_read(get), issue_read(get_comments), add_issue_comment(claim), search_issues(dedup, one targeted call), issue_write(create #15823), create_pull_request, pull_request_read(body read-back), add_issue_comment(this report)",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #15823: 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 in this round on a real engine. Nothing in the tree does it today (the repo's only registered afterFind is plugin-audit's read recorder, which never touches ctx.result), but it is the seam the whole 104-block census class orbits: it is why 'the declared type says it cannot happen' is not a sufficient reason to delete a limb. The card records the fork rather than proposing either branch, since #15597 forbids both a packages/spec change and a gate change. pm:queue only, no domain/type/priority."
      ]
    }

    Generated by Claude Code

  5. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    Landed — 9e9f03abe215fc09f240f14557f6f281dd9dda57

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

    Landing authority is git log origin/main | grep -c '(#15827)' = 1 with (#15365) = 1 as the cwd control. pm:dispatched stripped; the card keeps bug, priority:p2, domain:services, finding — read back to confirm. ⚠️ Worth noting for the next closeout: my own check-in note predicted three labels and the card actually carried four; reading them back is what caught it, not the note.

    The Clause-② PASS and my verification of the fix-up are already recorded above (5551004676 and 5551205753) — this is the closeout only.

    What shipped

    The card's number was stale and the correction reconciles exactly. The card said 12 records-limb blocks plus a data-limb rowsOf; the measured count was 14 (13 records, 1 data). The delta is one and it is accounted for: PR #15738 landed as aedbaeff1 hours earlier and added hasExistingUserFor, whose rows line is this shape. 12 + 1 = 13, plus the data limb = 14.

    The user-visible half is settleSelfRegistrationGrant, which carried the opposite defect — #15092's DROP shape. Its filter silently removed a permission-set row with a missing or blank id before deciding which row to grant, which went wrong two ways, both silent and both measured by ablation: a false cause (the operator told no row named portal_user resolves while one is sitting right there), and a wrong grant (a malformed org-scoped row dropped, letting the organization_id == null arm match, and — with the organization resolved — ps_global written stamped organization_id: 'org_1', so the store ends up asserting that org_1 granted a set org_1 never declared). It now refuses and names the malformed row.

    ⚠️ The refusal has a cost and it is a real behaviour change, stated as an upgrade note in the changeset rather than left for release notes to discover: a deployment whose sys_permission_set already holds an active, correctly-named row with a missing or blank id now gets a loud named refusal where it previously got a grant — including case D, a malformed global row beside a well-formed org-scoped one, which used to be dropped harmlessly. Deliberate (the old code could not tell that family apart from the one where the drop granted the wrong set), fully reversible with no code change (repair or delete the row), and nothing is written while the refusal stands.

    The census question this card opened is settled

    #15094 is NOT undercounted. It 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. The "only 10" that raised the doubt was an artefact of this round's own limb-anchored spelling: four of the fourteen are written as a guard clause (if (Array.isArray(raw)) return raw; const records = raw?.records; …) rather than a ternary. Same defect, included in the count. ⭐ The generalisable lesson: anchor a census on the limb, not on the ternary — a ternary-anchored scan silently misses the guard-clause spelling, and a limb-anchored one catches both.

    Left open deliberately, not forgotten

    #15823 — ObjectQL.find returns hookContext.result on its hook path with nothing re-checking it against the declared array, so an afterFind hook assigning ctx.result = { records: [ … ] } really does make find() resolve to an envelope. Driven and confirmed. Nothing in this tree does it, and the limb did not repair that case — it masked it at fourteen sites while the package's other 47 .find( call sites broke anyway. The review widened it with a second door on the same unguarded return: the middleware seam (executeWithMiddleware returning ctx.result, with security-plugin.ts:3143 assigning it). Both are recorded on #15823, which is the maintainer's.


    Generated by Claude Code

  6. added a commit that references this issue on Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions