fix(plugin-approvals): screen the team approver expansion to the request's organization

[os-warren]

Fixes#10230

team was the last approver expansion that resolved people without asking which
organization was asking. A request raised in org_a, routed to a team approver
whose sys_team.organization_id is org_b, placed that team's members into
pending_approvers — approval authority over a record they are not a tenant of.

Premise, reproduced before editing anything

One tree, one run, on dd41df30c, from this package's existing harness:

[PROBE W] org_a request, org_b team -> pending_approvers = ["u_team_b"]
[PROBE B] org_a request, only an org_b `cfo` holder -> pending_approvers = ["position:cfo"]
[PROBE B2] same-org `cfo` holder present -> pending_approvers = ["u_pos_a"]

Exactly the card's readings: the unscreened type routes the foreign people, the
screened sibling next to it in the same run does not.

Why "thread an organization" and not "bring it under bounded(...)" — measured, not inferred

The card named the bounded(...) asymmetry as the core clue: team is the only
graph-shaped type not wrapped in it. The asymmetry is real, but the obvious repair
it suggests is a no-op, and this is the measurement rather than the reading:

1. bounded(...) applies filterApproversWhoCanRead only when
   crossOrg === (directoryOrg !== organizationId).
2. directoryOrg comes from resolveApproverDirectoryOrg(...), which returns the
   request's own organization when no organization is declared, and throws
   (VALIDATION_FAILED) when one is declared on a type that is not org-scoped.
3. APPROVER_ORG_SCOPED.team is false. So for a static team approver
   crossOrg is structurally always false — the wrapper can never fire.

Measured on this tree by temporarily wrapping the team branch in bounded(...):

[PROBE W] org_a request, org_b team -> pending_approvers = ["u_team_b"] (unchanged)

An unchanged reading is the ambiguous result, so it carries a positive control:
forcing the same branch to yield nobody flipped the same probe to
["team:team_b"] and turned the test red, proving the edit reached the runtime.
Both temporary edits were reverted; the file's blob hash is byte-identical to its
committed state.

⇒ bounded(...) would have added a wrapper that never runs. The fix has to make
the expansion itself ask.

The screen: the team, not its members

// return types written as comments — GitHub's body sanitizer eats short angle-bracket fragmentsprivateasyncexpandTeamUsers(teamId: string,organizationId?: string |null)// -> string[]privateasyncteamIsProvablyOutsideOrg(teamId: string,organizationId?: string |null)// -> boolean

This is where the twin cards genuinely diverge, and #10334's shape is deliberately
not copied. sys_user carries no tenancy fact at all, so #10153's manager
screen can only place a person through his sys_member rows. sys_team carries
organization_id outright, so a team id transitively names exactly one
organization: one row answers the question, and a team that fails the screen
never fans out to its members at all.

Screening the members instead would be both a wider read and a different
assertion — it would rule on #7497 (does approver routing imply record read
visibility?), which this card does not.

Posture is the sibling's, fail-open on an absent tenancy fact and fail-closed
only on a present and negative one:

team row state	verdict
organization_id present, ≠ request org	screened out, loudly
organization_id present, = request org	routes, unchanged
organization_id null	routes, unchanged
no sys_team row at all	routes, unchanged
sys_team unreadable	routes, unchanged
request carries no organization	routes, unchanged, and performs no read

The null limb is #3807's ruling, not timidity: null on a platform object means
"owned by no organization" — what a seed writes, since a seed cannot know the id
the runtime mints at boot — and #3807 is the card where treating that as "not
mine" made every seeded department approver resolve to nobody. Measured here
too: this package's own team_ok expansion fixture is such a stack.

Both call sites, and why they pass different organizations

• static team branch → the request's ownorganizationId. team is not
  org-scoped, so a declaration on it is refused upstream and directoryOrg is
  provably equal here; naming the request org says what the screen asserts.
• expression / resolveAs: 'team' → directoryOrg. expressionis
  org-scoped, so a declaration there retargets a sibling organization
  legitimately, and the team must belong to the directory actually being
  consulted. filterApproversWhoCanRead then applies the D2 read screen on top,
  exactly as it already does for the other resolveAs kinds.

Failure shape: consistent with position, and that is a measurement

The card asked whether a different failure shape is warranted here. Measured in
one run on one tree (X1): both fall back to the dead type:value literal.

[PROBE X1] team = ["team:team_b"] · position = ["position:cfo"]

No reason to diverge: the literal fallback is this file's single answer to "the
graph resolved nobody", it keeps 15.x stored slots and substring fixtures working,
and #3807's warning makes it visible instead of silent.

Contract impact (Clause ②) — this narrows an externally observable accept set

Kept yes, and it reproduces on this branch rather than being inherited from the
sibling card. Under the non-default onEmptyApprovers: 'fail' policy a node whose
sole approver is a cross-org team used to open a request and now throws
NO_APPROVERS (C-a). Under the default admin_rescue the node still opens
(C-a2), which confines the flip to one non-default policy. That throw is
pre-existing code and a bare Error, not a minted ADR-0112 envelope, so there is
no code / status to assert on it — the same reading #10153's file recorded for
its own flip.

PR intentionally stays draft pending needs:contract-review. Not flipped
ready; no auto-merge.

What this implies for APPROVER_ORG_SCOPED — and why that table is untouched

⛔ No packages/spec file is modified by this PR. approval.zod.ts is read only.

The card is explicit that team: false / manager: false answers ADR-0105 D9
retargetability ("does an organization: declaration apply to this type"), not
"is the expansion screened to the request's organization". Both flags stay false
and both remain correct: neither type consults an org-scoped directory, so an
organization declaration on either would still have no effect and is still
rightly refused.

What has changed is that the pairing no longer implies anything shared. Before
#10153 the two false rows also happened to be the two unscreened expansions;
after this PR they are simply the two types you cannot retarget, and both are
screened. That removes the reading the card warned about — that moving one out
of the pair would be "splitting the class" — because there is no longer a
screening class hiding behind the retargetability table. The table's own docblock
does carry a now-stale justification (it explains team's false partly via
"sys_team_member carries no organization column and the engine never scoped it",
which is still true of sys_team_member but no longer describes the engine).
Correcting that comment is a packages/spec edit this lane does not own; filed
separately as #10548 rather than smuggled in.

Tests

New: packages/plugins/plugin-approvals/src/team-approver-org-screen.test.ts — 12
pins, two-directional throughout: T1 cross-org team screened out, T2
same-org team still resolves, T3/T4/T5 the three absent-tenancy-fact limbs
still resolve, T6 no-org request untouched and reads no sys_team, T7 the drop
is loud, E1/E2 both directions through the expression path, X1 the sibling
shape contrast, C-a/C-a2 the accept-to-reject flip and its confinement.

Inverted: manager-approver-org-screen.test.ts's PROBE W, which #10153 wrote
explicitly to hand this gap off to this card.

Ablation

Predicted signature written before mutating; mutation = deleting the screen call
from expandTeamUsers.

• Predicted red: T1, T7, E1, X1, C-a, C-a2, W (7).
• Predicted green: T2, T3, T4, T5, T6, E2 (6) — every one of these
  asserts routing is unchanged, which an unscreened build also satisfies. That is
  precisely why the one-directional half cannot be the only pinning.
• Observed: Tests 7 failed | 15 passed (22), the named seven, exactly.
• Restore: git checkout then git hash-object →
  b7a5aed819fd9f546b9a9838d467d7c7d6075fd9, identical to the committed blob
  (ablated blob was 1ac86f7fffc740aee46bece1cddd459fa42afd9c). Restore leg
  re-measured green: Tests 22 passed (22).

Resolution path: these tests import ./approval-service.jsrelatively, within
the same package, so vitest resolves to src/ and no dist/ is involved — and
that is established by the positive control above rather than assumed.

Package suite: Tests 519 passed (519) (baseline before this change: 507; +12 new).
typecheck: clean.

Gates

Union derived by node scripts/pm/dispatch-gates.mjs (no hand-written path list),
re-derived after the final commit — which is how check:cross-package-test-inputs
entered the set, matched via the scripts/** ledger path that only the second
commit created. All run on a clean worktree at 20f511ece, exit codes captured
before any pipe:

check:changeset-gate-self-tests · check:cross-package-test-inputs ·
check:objectui-changeset · check:slot-lookup · check:test-source-alias ·
check:type-source-resolution · check-adr-0087-registration ·
check-changeset-no-major · check-empty-changeset · check-affected-docs ·
check:engine-double-contract · check:where-matcher ·
check:query-options-erasure · check:i18n · check:nul-bytes ·
check:type-check-coverage · check:type-check-debt --re-measure — 17/17 exit 0.

Verdict lines the ratchets printed for themselves:

slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new … baseline key set verified against dd41df3: no files added.
where-matcher: 0 silently-wrong and 0 unjudged matcher(s) … none new.
query-options-erasure: test surface 240 site(s) in 47 file(s) — at the ceiling … no files added.
check-engine-double-contract: OK — 344 pinned, 133 in the DEBT ledger, 2 exempt.
check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured, 1924 raw tsc error(s) total, none above its recorded number.

check:engine-double-contract retained the new file's delete/update doubles as
unrecorded coverage and prescribed its own repair; regenerated with --write —
2 rows added, 0 lost, no DEBT entry raised. @objectstack/plugin-auth's
TEST_DEBT is untouched at 109 (its file is unmodified in this branch, and the
--re-measure above re-ran it).

⚠️Lint & Repo Gates may still go red on check-query-options-erasure-ratchet
with packages/spec/src/migrations/registry.ts:0:0 — Parsing error: Maximum call stack size exceeded. That is #10449 (the gate calls ESLint in-process and bypasses
--stack-size=4000), red on main's population and unrelated to this diff.

Release condition, not an observation

The screen bites only where teams carry an organization stamp. A deployment
whose sys_team rows are seeded with organization_id: null keeps today's
cross-org routing for those teams, by the deliberate #3807 fail-open above. That is
the accepted residual, and it is a condition on the fix's security value rather
than a footnote: for this to actually close the boundary in a group-posture
deployment, teams must be created through better-auth's create-team (which stamps
organizationId) or otherwise backfilled. Teams created by seed and never
backfilled remain routable across the boundary.

Out of scope, filed as #10547: sys_team_member rows carry no tenancy fact of
their own, so a team that passes this screen still routes to every listed member
even if that member holds no membership in the organization.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

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

24 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 359f5956d7910aed7ae9f8fccc9fbb988b3e4882.

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

What this run could not see

• the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 5 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 359f5956d7910aed7ae9f8fccc9fbb988b3e4882 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 832ee1ee7606de5ccb7a9c3c0afcfcd9e3ae9624 — the merge of head 20f511ece0b50f358e9a2bf21c858f307caf9f42 into base 359f5956d7910aed7ae9f8fccc9fbb988b3e4882, 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 832ee1ee7606de5ccb7a9c3c0afcfcd9e3ae9624 && git checkout 832ee1ee7606de5ccb7a9c3c0afcfcd9e3ae9624
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 359f5956d7910aed7ae9f8fccc9fbb988b3e4882 20f511ece0b50f358e9a2bf21c858f307caf9f42 && git checkout -B drift-repro 359f5956d7910aed7ae9f8fccc9fbb988b3e4882 && git merge --no-ff 20f511ece0b50f358e9a2bf21c858f307caf9f42
node scripts/docs-audit/affected-docs.mjs --json 359f5956d7910aed7ae9f8fccc9fbb988b3e4882

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