Skip to content

fix(auth): teamId on organization/add-member has no active-team fallback - #10715

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-10532-add-member-teamid-no-fallback
Aug 21, 2026
Merged

fix(auth): teamId on organization/add-member has no active-team fallback#10715
os-warren merged 1 commit into
mainfrom
claude/issue-10532-add-member-teamid-no-fallback

Conversation

@os-warren

@os-warrenos-warren commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Fixes#10532

What was wrong

Two source comments asserted that better-auth's organization/add-member
defaults bothorganizationId and teamId to the caller's active
org/team. Only the organization half is true.

Vendor re-measurement (done in this worktree, not taken from the card)

Installed version confirmed better-auth 1.7.1 — resolved from
packages/plugins/plugin-auth, realpath
node_modules/.pnpm/better-auth@1.7.1_…/node_modules/better-auth, whose
package.json reports 1.7.1. File:
dist/plugins/organization/routes/crud-members.mjs, inside addMember's
handler (:41, :43):

constorgId=ctx.body.organizationId||session?.session.activeOrganizationId;if(!orgId)throwAPIError.from("BAD_REQUEST",ORGANIZATION_ERROR_CODES.NO_ACTIVE_ORGANIZATION);constteamId="teamId"inctx.body ? ctx.body.teamId : void0;

The finding is a zero-hit judgement, so it is reported with the
neighbouring positive control that proves the search works — same grep,
same file:

termhits
activeOrganizationId (control — known present)8
activeTeamId (the claim)0

An omitted teamId therefore stays undefined, and every if (teamId) branch
downstream (team lookup, TEAM_NOT_FOUND, the per-team limit) is skipped. The
card's reading reproduced exactly.

Scope correction — the claim lives at TWO sites, not three

The card and the dispatch both name three sites. Measured here, the false
sentence exists at two:

  1. packages/platform-objects/src/identity/sys-member.object.ts — the origin
    (the add_member action).
  2. packages/plugins/plugin-auth/src/organization-add-member.ts — the module
    header, which cited the origin as the authority for forwarding headers.

packages/plugins/plugin-auth/src/auth-plugin.ts does cite the same action
metadata above the mount, but it cites it for a different and correct fact —
that the sys_member action has always targeted this URL, which is what
justifies restoring the route. It carries no claim about defaults, so there is
nothing false there to correct and it is left untouched. Verified by an
exhaustive scan rather than by reading the one comment: repo-wide greps for
active org/team, default(s) to the caller and active[Tt]eam return only
the two sites above (plus sys-team.object.ts, which asserts the
organization half only — that half is true — and one plugin-auth test
comment likewise quoting only the org half).

The corrected sentence

Both sites now carry the same statement:

organizationId defaults to the caller's active organization when omitted;
teamId has no such fallback — omit it and the member simply joins no team.

which is also the wording the #10050 docs already publish in
content/docs/permissions/authentication.mdx.

Nothing was misled at runtime

The add_member action's params list is userId, role, organizationId
no teamId (checked in the file, not assumed from the card). The toolbar
therefore never sent one and the claim was never exercised. This is a wrong
citation
, not a defect that reached a deployment. teamId forwarding itself
stays correct: pass it and it works. No runtime behaviour changes in this PR.

The pin

packages/plugins/plugin-auth/src/organization-add-member-team-fallback.test.ts
reads the fact out of the installed vendor artifact — it resolves
better-auth with createRequire seeded from this package, reads
crud-members.mjs, slices the top-level addMember declaration, and extracts
the orgId / teamId binding initialisers as vendor bytes. It asserts
nothing about our own comments or our own constants: an assertion whose two
sides both derive from this repo would prove only that we wrote what we wrote.

Every negative leg is paired with a positive control running the same
instrument over a neighbouring known-present fact:

  • one matcher, SESSION_ACTIVE_FALLBACK, must match the orgId binding
    before its non-match on the teamId binding is read as evidence;
  • activeOrganizationId must occur in the module before the zero count for
    active[Tt]eam* counts.

A vendor upgrade that moves or renames the declaration throws with a message
naming the file and what to re-decide, rather than silently matching nothing
and passing.

Ablation — predicted, then observed

Prediction written down before running (mutate the vendor fact into the
has-fallback shape):

const teamId = "teamId" in ctx.body ? ctx.body.teamId : void 0;
-> const teamId = "teamId" in ctx.body ? ctx.body.teamId : session?.session.activeTeamId;
predicted: Test Files 1 failed (1) · Tests 2 failed | 2 passed (4)
1 CONTROL orgId falls back .................. PASS (untouched)
2 teamId binding has NO fallback ............ FAIL at not.toMatch(SESSION_ACTIVE_FALLBACK)
3 no active-team read anywhere .............. FAIL, expected 0 received 1
4 if (teamId) branches stay skipped ......... PASS (no `teamId ||` / `??` introduced)

Observed, exactly:

 Test Files 1 failed (1)
Tests 2 failed | 2 passed (4)
AssertionError: expected '"teamId" in ctx.body ? ctx.body.teamI…' not to match /session[\s\S]*\bactive[A-Z]\w*/
AssertionError: expected 1 to be +0 // Object.is equality

This also answers "where does the pin read from". Nothing in this repo was
touched and nothing was rebuilt for the ablation — the only edit was to the file
under node_modules, and the suite went red. That is a positive control for
node_modules-at-runtime, not a cached or derived copy. (The usual
rebuild-each-leg rule does not apply here precisely because the mutated artifact
is the vendor's, not our dist/.) Green again on the unmutated vendor:
Test Files 1 passed (1) · Tests 4 passed (4).

Hardlink safety. The vendor file had 6 hardlinks into the shared pnpm
store, so an in-place edit would have corrupted every sibling agent's
node_modules and the store itself. The link was broken for this worktree only
(rm the directory entry, then write a fresh inode), leaving the store intact,
and restored by copy from a pre-mutation backup:

hardlinks before ......... 6
hardlinks during ......... 1 (store inode untouched)
ORIG_SHA = 909fbfec01a59a0e36a12cb7ecc5ee7c8e9e5940
MUTATED_SHA = 657e8c3b5864bfe1ea9fe7fe8e393f405d92098d
RESTORED_SHA = 909fbfec01a59a0e36a12cb7ecc5ee7c8e9e5940 ← byte-identical, `cmp` also clean

Verification — all at 7064ec8ba, clean tree

Gate union derived with node scripts/pm/dispatch-gates.mjs (no path
arguments) after the final commit on a clean tree; exit codes captured
before any pipe. Every line below is the gate's own verdict.

gateexitverdict line
pnpm --filter @objectstack/plugin-auth typecheck0(tsc silent)
pnpm --filter @objectstack/platform-objects typecheck0(tsc silent)
pnpm --filter @objectstack/platform-objects test0Test Files 25 passed (25) · Tests 419 passed (419)
pnpm --filter @objectstack/plugin-auth test0Test Files 62 passed (62) · Tests 1363 passed (1363)
turbo run build --filter=./packages/* --filter=./packages/*/*0Tasks: 70 successful, 70 total
check:changeset-gate-self-tests0
check:objectui-changeset0
check:slot-lookup0
check:test-source-alias0
check:type-source-resolution0
check-adr-0087-registration.mjs0
check-changeset-no-major.mjs0
check-empty-changeset.mjs0
docs-audit/check-affected-docs.mjs0
check:query-options-erasure0query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new, and every file measured parsed.
check:engine-double-contract0
check:where-matcher0
check:i18n0check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys).
check:type-check-coverage0check-type-check-coverage: OK — 64/77 workspace packages type-checked (plus the root), 13 in the DEBT ledger …
check:type-check-debt --re-measure0check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 391.3s, 1913 raw tsc error(s) total, none above its recorded number.
check:nul-bytes0check-nul-bytes: OK (scanned 6214 text file(s) … no raw ASCII control bytes).

#10449 is fixed — asserted, not merely absent

The dispatch carried a warning that check-query-options-erasure-ratchet might
be red on packages/spec/src/migrations/registry.ts:0:0 — Parsing error: Maximum call stack size exceeded, and to attribute that to #10449 rather than
investigate. The PM withdrew the warning mid-task (#10449 closed 2026-08-20 by
PR #10464). Independently confirmed here, and more strongly than "it did not
reproduce": pnpm check:query-options-erasure is the exact command CI's Engine
query-options erasure ratchet
step runs (.github/workflows/lint.yml:109), it
exits 0 on this branch, and its own self-test line names the withdrawn
failure directly —

…and packages/spec/src/migrations/registry.ts parses at --stack-size=4000 through this gate's own channel.

The new test file also consumed no test-surface headroom
(test surface: 240 site(s) in 47 file(s) — at the ceiling).

class #10309 — the derivation was short, as warned. It did not name
check:route-envelope or check:dispatcher-error-vocabulary even though this
PR touches plugin-auth and platform-objects. Both were run explicitly,
including --self-test:

gateexit
check:route-envelope0
check:route-envelope --self-test0
check:dispatcher-error-vocabulary0
check:dispatcher-error-vocabulary --self-test0

Observation for #10615, deliberately NOT acted on here. The ratchet's
informational line reads: "@objectstack/plugin-auth: TEST_DEBT records 109, tsc
now reports 98 (-11) — the entry can be lowered."
The gate is green (a count
may sit below its ceiling); the dispatch fenced this entry off, so it was not
edited and --lower was never run. Flagging the −11 gap because it is a real
change to what #10615 records. The new test file contributes 0 errors — the
count moved down, not up.

Single-claim paths

scripts/check-single-claim-paths.mjs cannot run here — it needs PR_NUMBER +
a GITHUB_TOKEN, and with only the former it fails GitHub API 401, which is a
wiring failure and not a verdict. CI runs it. Checked directly instead: of the
16 other open PRs, the two that could plausibly overlap were inspected file by
file — #10660 (plugin-auth) touches auth-route-ledger.ts,
auth-route-ledger.conformance.test.ts and docs; #10633 (platform-objects)
touches sys-session.object.ts and two new tests. Neither touches any of this
PR's four files.
No conflicting claim.

Not in scope

No packages/spec file is touched. content/docs/releases/** is untouched —
the release-notes input is the changeset,
.changeset/add-member-team-id-no-active-team-fallback.md.


Generated by Claude Code

…ack (#10532)
Two source comments asserted that better-auth's `organization/add-member`
defaults BOTH `organizationId` and `teamId` to the caller's active org/team.
Measured on the installed better-auth 1.7.1
(`dist/plugins/organization/routes/crud-members.mjs`, inside `addMember`), only
the organization half is true:
const orgId = ctx.body.organizationId || session?.session.activeOrganizationId;
const teamId = "teamId" in ctx.body ? ctx.body.teamId : void 0;
`activeOrganizationId` is read 8 times in that module; `activeTeamId`, 0 times
(same grep, so the zero is a measurement and not a broken search).
Corrected at the origin (`sys_member`'s `add_member` action metadata) and at the
citation that named it as authority (`organization-add-member.ts`, which cited
it as the justification for forwarding request headers — forwarding buys the
organization default only).
No runtime behaviour changes and nothing was ever misled at runtime: the
`add_member` action's `params` list carries no `teamId`, so the claim was never
exercised. It was a wrong citation, not a live defect.
The asymmetry that #10050's docs now publish is held by a new pin,
`organization-add-member-team-fallback.test.ts`, which reads the fact out of the
INSTALLED vendor artifact rather than out of our own comments, with a positive
control on `activeOrganizationId` for every zero-hit leg.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/permissions/authentication.mdx(via teamId (literal))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 12 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 53a48c93f4e4016eb1d23a8780c6207f279088dapackageMentionDocs.

Which tree this was computed on

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

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 21, 2026
@os-warren
os-warren marked this pull request as ready for review August 21, 2026 09:31
@os-warren
os-warren enabled auto-merge August 21, 2026 09:31
@os-warren
os-warren added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 8f04d9aAug 21, 2026
38 checks passed
@os-warren
os-warren deleted the claude/issue-10532-add-member-teamid-no-fallback branch August 21, 2026 09:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: three source comments claim teamId on organization/add-member defaults to the caller's active team — better-auth 1.7.1 has no such fallback

1 participant

@os-warren