Skip to content

fix(client): organizations.getActiveMember addresses the organisation the caller NAMES, not whichever one the session has active - #16761

Draft
claude[bot] wants to merge 5 commits into
mainfrom
claude/issue-16568-get-active-member-organization-id
Draft

fix(client): organizations.getActiveMember addresses the organisation the caller NAMES, not whichever one the session has active#16761
claude[bot] wants to merge 5 commits into
mainfrom
claude/issue-16568-get-active-member-organization-id

Conversation

@claude

@claude claude Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #16568
Clause-②: yes

The defect

organizations.getActiveMember(organizationId) built GET /organization/get-active-member?organizationId=…. better-auth 1.7.2's handler for that path (plugins/organization/routes/crud-members.mjs) reads session.session.activeOrganizationId and never looks at ctx.query, so the query string was dead on arrival: a client doing a permission check for organisation B while A was active was told about A, at 200, with no diagnostic. The SDK's own JSDoc promised "the calling user's membership row in the given organisation" — a declared capability the runtime did not deliver.

Zone 1's hard precondition, measured BEFORE any implementation

Triage recommended list-members but said in writing it had not verified the query shape. It was driven first: a real AuthManager (better-auth 1.7.2, organization plugin, teams enabled) over a real SqlDriver (better-sqlite3 :memory:), one user owning two organisations with A active, plus a second member seeded into B so the filter has something to exclude. Transcript, trimmed to the ids that matter:

CREATE-A                                    -> 200 id=aSkH…  (member row role=owner)
CREATE-B                                    -> 200 id=YerN…  (member row role=owner)
SET-ACTIVE A                                -> 200

R1  get-active-member?organizationId=A      -> 200 {organizationId:A, id:Gbr…, role:'owner', user:{…}}
R2  get-active-member?organizationId=B      -> 200 {organizationId:A, id:Gbr…, role:'owner', user:{…}}   # SAME ROW
R8  get-active-member  (no active org)      -> 400 NO_ACTIVE_ORGANIZATION                                # the card's control

R3  list-members?organizationId=B&filterField=userId&filterValue=SELF        -> 200 {members:[{organizationId:B,…}], total:1}
R4  list-members?organizationId=A&filterField=userId&filterValue=SELF        -> 200 {members:[{organizationId:A,…}], total:1}
R5  same as R3 plus &limit=1                                                 -> 200 {members:[{organizationId:B,…}], total:1}
R9  R3 again with NO active organisation                                     -> 200 {members:[{organizationId:B,…}], total:1}
R7  list-members?organizationId=FOREIGN&filterField=userId&filterValue=SELF  -> 403 YOU_ARE_NOT_A_MEMBER_OF_THIS_ORGANIZATION

RA  list-members?organizationId=B  (unfiltered, B now has 2 members)         -> 200 {members:[OTHER, SELF], total:2}
RB  list-members?organizationId=B&filterField=userId&filterValue=SELF&limit=1-> 200 {members:[SELF],  total:1}
RC  list-members?organizationId=B&filterField=userId&filterValue=OTHER       -> 200 {members:[OTHER], total:1}

ANON get-session                            -> 200 null
ANON list-members                           -> 401 UNAUTHORIZED
ANON get-active-member                      -> 401 UNAUTHORIZED

RA/RB/RC are the discriminating leg: with two rows in B, the self filter returns exactly one and the other-user filter returns the other, so filterField=userId really narrows rather than merely not breaking. R3/R4 are the addressing leg. The precondition holds, so option 2 was implemented; nothing was improvised and the decision inbox was not needed.

The vendor premise was re-confirmed on the same drive: the installed version is exactly better-auth 1.7.2 (pinned by PR #16634), and its getActiveMember handler still reads session state only. The card's premise stands.

What changed

packages/client/src/index.ts, organizations.getActiveMember — the signature and the declared return type are byte-identical; only the addressing moved:

  1. GET /get-session for the caller's own user id (bare { user, session } for a signed-in caller, the literal null for an anonymous one — measured);
  2. GET /organization/list-members?organizationId=…&filterField=userId&filterValue=SELF_USER_ID&limit=1, unwrapping the one-entry page.

list-members rows carry the identical shape — {id, organizationId, userId, role, createdAt, user:{id,name,email,image}} — which is why OrganizationMemberWithUserWire does not move.

The JSDoc is corrected in the same stroke, as triage required. #14314's PR had changed it to say the argument is ignored; that sentence is now false, so it is replaced by what the method does, plus every behaviour an existing caller can observe change.

Does the request-byte change constitute a published behaviour change? Yes — declared, not argued away

Triage asked for this in writing, so here it is, item by item. The request bytes change, and so does the answer:

  • naming a non-active organisation now answers that organisation's row instead of the active one's. This is the defect, and enforcing a declaration the SDK has always made;
  • a non-member of the named organisation is refused 403 YOU_ARE_NOT_A_MEMBER_OF_THIS_ORGANIZATION where the old shape produced 400 MEMBER_NOT_FOUND — and about a different organisation at that, since the old shape never asked about the named one. Two published error codes, and the input class that reaches each of them is re-chosen;
  • a caller with no active organisation now gets their row instead of 400 NO_ACTIVE_ORGANIZATION. setActive has stopped being a precondition;
  • an anonymous caller still gets 401 UNAUTHORIZED, thrown by the same session middleware that guarded the old route. Nothing client-side is substituted for the server's refusal;
  • one HTTP request became two.

Clause-②: yes — re-declared from the delivered diff

The dispatch carried a no as triage's reading, marked explicitly as not measured. Re-declared here, and it flips. The machine-read declaration is the standalone line at the top of this body, in the fixed spelling — this heading and the paragraphs under it are the argument, not the declaration.

The mechanical floor is clean: no new exported symbol, no new key on a published payload, no signature change, no type change (check:exported-any-returns is untouched, check:dts-closure and check:type-source-resolution both green). But the floor is not the whole test, and the contract-review rule names this exact case as one that needs judgement rather than a mechanism: "在两个已发布码之间重选输入类". That is precisely what the second bullet above is — the input class that produces each of two published ADR-0112 codes is re-chosen — and the answer to which row an existing caller receives changes with it. Under "claim 拿不准 ⇒ 按 yes" that is a yes twice over.

needs:contract-review is hung on this PR at creation, and on the card, as the double carrier requires.

Reverse verification

The fix was committed first, then the pre-fix packages/client/src/index.ts was restored for one run.

  • on-disk proof, both directions: the anchor `organization/list-members` counted 2 before and 1 after (the surviving one is a pre-existing JSDoc occurrence at line 1358 — the printed "expect 0" label in the ablation script was wrong about that constant, the observation was not), and the blob hash moved ef5fa760… to 7fa9e129…;
  • result: 5 of 7 cases red — ① wrong organisation (expected 'org_alpha' to be 'org_bravo'), ② the request bytes, ④ no-active-organisation, ⑤ the 403 envelope (expected undefined to be 'YOU_ARE_NOT_A_MEMBER…'), ⑥ the anonymous 401 request count. ③ (naming the active organisation) and ⑦ (the guard-the-guard leg that drives the dead route directly) stay green, as predicted — ③ is the one case the old shape got right by coincidence;
  • restore proven, not assumed: git checkout HEAD -- …, then git diff HEAD empty and the on-disk blob hash back to ef5fa760…, byte for byte. The script carried a trap … EXIT INT TERM with absolute paths throughout.

No dist is in the resolution path here: the suite imports ./index relatively, i.e. the source in this checkout, so the ablation could not have been read against a stale build.

Tests

New: packages/client/src/organization-get-active-member-addressing.test.ts, 7 cases. Its fixture is not an approximation — every status, code and row shape in it is a transcript line from the drive above, and it keeps the defect alive on get-active-member (that arm still answers the active organisation whatever the query names), so a regression to the old route fails on the row value rather than on a URL string.

run result
pnpm --filter @objectstack/client test 36 files / 461 tests passed
pnpm --filter @objectstack/client typecheck pass — tsc --noEmit + check:test-typecheck (0 files / 0 errors in the debt ledger)
pnpm --filter @objectstack/plugin-auth test 104 files / 2191 tests passed
pnpm --filter @objectstack/plugin-auth typecheck pass — debt ledger unchanged at 10 files / 94 errors / 23 pinned
pnpm --filter '@objectstack/client-react...' build pass (the dependency closure; also the prerequisite two gates below needed)

Gates

Derived from the delivered diff with node scripts/pm/dispatch-gates.mjs --commands, from a tree actually at origin/main (no STALE TREE banner — origin/main had moved twice during the round and was merged in first), and reconciled:

Run reconciliation — 59 derived, 59 run, 0 NOT-MEASURED, 0 UNRUN.
✓ dispatch-gates --ran: 59 derived famil(ies) accounted for — 59 run, 0 NOT-MEASURED.

All 59 exit 0, each captured before any pipe. Three needed a second lap and none of the three is a NOT MEASURED in the final record:

  • pnpm check:doc-authoring was genuinely red on this diff: the new ledger note carried #16568 in a runtime string, against the maintainer's ruling 「处理 issue 时犯的错应该总结成经验,保留 issue id没有意义」. The id is stripped; git history keeps the anchor. Green.
  • pnpm --filter @objectstack/spec run check:skill-examples and pnpm check:dual-build-cjs-loads both refused for want of built output (the second by its own exit 3 PREREQUISITE NOT MET). Both green after the client-react closure build — 258 prose examples type-check across 3 surfaces.
  • pnpm check:type-check-debt OOM-ed at --max-old-space-size=4096 and answered exit 3, its own PREREQUISITE-NOT-MET code. Re-run at 8192 (the gate itself runs tsc under a CI-shaped 6144 ceiling, so 4096 could never have held the wrapper): green, 5 ledger entries re-measured, none above its recorded number.

Lint is the full repo-wide union, not a narrowing: eslint . --no-inline-config --format json at 012d430b6347 files, 0 errors, 0 warnings, exit 0.

Declared scope extension: one ledger row outside the dispatched file surface

The dispatch named packages/client/src/index.ts plus a test under packages/client/. This PR also edits one row of packages/plugins/plugin-auth/src/auth-route-ledger.ts, and that is deliberate rather than drift: disposition: 'sdk' means "expressed by the SDK", and after this change no SDK method builds that URL, so leaving the row would ship a false statement in a truth ledger created by this diff. It is rebooked server-only with the rationale the hygiene test demands, client dropped, modelled on the neighbouring organization/add-member row which carries exactly this shape.

The bounded in-place exemption's four conditions, each checked rather than asserted: (i) same defect class as the card — a declared capability the runtime does not deliver; (ii) mechanical, with the target shape already pinned by AuthRouteDisposition and the hygiene case that demands a note on every non-sdk row; (iii) zero holders — scanned per-ref against each open PR's own merge-base, positive control fired; (iv) same gate family, no new validation surface (auth-route-ledger.conformance.test.ts, auth-route-ledger-coverage.test.ts and pnpm check:auth-mount-ledger already read this file, and all three are green).

Nothing published moves with it: the module has zero runtime importers in non-test source, and tsup builds only src/index.ts and src/rate-limit-storage.ts, so it cannot reach dist. Hence one changeset, for @objectstack/client alone.

Serial

packages/client/src/index.ts is the #12104 family's hard-serial hot file. Re-measured at claim time rather than inherited: zero holders across 11 of 11 open PRs, per-ref against each PR's own merge-base, with two positive controls firing (packages/cli/src/commands/validate.ts in #16727, packages/client/package.json in #15334). The same scan found zero holders on auth-route-ledger.ts.

验收备注


Generated by Claude Code

`organizations.getActiveMember(organizationId)` built
`GET /organization/get-active-member?organizationId=...`, and better-auth
1.7.2's handler for that path reads `session.session.activeOrganizationId`
and never looks at `ctx.query`. The query string was dead on arrival: a
permission check for organisation B while A was active answered A's row,
with a 200 and no diagnostic.

The method now asks the question honestly, in two requests: `GET
/get-session` for the caller's own user id, then `GET
/organization/list-members?organizationId=...&filterField=userId&filterValue=<self>&limit=1`,
unwrapping the one-entry page. `list-members` reads `ctx.query.organizationId`
and its rows carry the identical shape, so the signature and the declared
return type are unchanged.

The `get-active-member` ledger row is rebooked `server-only`: no SDK method
builds that URL any more, and `sdk` means "expressed by the SDK".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
check:doc-authoring — a runtime string reaches authors and generated
surfaces, none of whom can resolve `#NNNN`; git history keeps the anchor.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 2 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 23 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 73053ed27b8c1f63a5734f14b56d093cbe843a47packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 73053ed27b8c1f63a5734f14b56d093cbe843a47

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Check Changeset: a PR declaring clause-② yes may not grade a package it
grew `patch`. The maintainer's ruling of 2026-09-04 (decision batch #35)
holds that a change to a published package's public surface takes at
least `minor`; a commit type may raise a bump, never lower it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8

Copy link
Copy Markdown
Contributor

Contract review (CONTRACT_REVIEW_TIER, isolated seat) — PR #16761 @ eb75819

Verdict: PASS WITH FINDINGS

Ruling implemented: n/a — no ## Ruling recorded exists on card #16568 or on this PR. The directive the PR implements is the triage seat's recommendation (os-zhuang, comment 5576219184, "分诊席" — a seat, not a maintainer ruling), and the PR implements it exactly: option 2 (list-members + filterField=userId self-filter, signature held), the hard precondition measured before implementation, JSDoc corrected in the same stroke, vendor version re-confirmed. The only maintainer ruling cited anywhere (2026-09-04, batch #35 "WHICH LEVEL") governs the changeset level, not this card.

Everything below was verified independently from refs/review/16761 against origin/main (47f751d5d) and the vendor source at the pinned version; nothing was taken from the PR body.

Verification

  1. Card and thread. client SDK organizations.getActiveMember(organizationId) sends an organizationId the server ignores — it answers the session's ACTIVE organization, whatever id the caller names #16568 (6 comments): triage → claim (edited Clause-②: no → yes in place, with a stated reason) → delivery acceptance → two os-dev-report blocks → CI-green note. The claim comment and the PR body now agree on Clause-②: yes.
  2. Diff vs merge-base 7c12e475e — 4 files, +362/−11: .changeset/client-get-active-member-names-the-organisation.md (A), packages/client/src/index.ts (M, +56/−10), packages/client/src/organization-get-active-member-addressing.test.ts (A, 275), packages/plugins/plugin-auth/src/auth-route-ledger.ts (M, 1 row). Governed paths: no — none of docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md, content/docs/releases/** is touched.
  3. The contract. Signature is byte-identical before/after: getActiveMember: async (organizationId: string): Promise<OrganizationMemberWithUserWire>. Wire binding moves from GET /organization/get-active-member?organizationId=… (ledger row now server-only) to GET /get-session + GET /organization/list-members?organizationId=…&filterField=userId&filterValue=<self>&limit=1 (both already ledgered sdk). Server answer when the named org ≠ active org, read from better-auth@1.7.2 plugins/organization/routes/crud-members.mjs (pin confirmed in plugin-auth/package.json and the lockfile): listMembers runs orgSessionMiddleware, resolves organizationId = ctx.query.organizationId || session.activeOrganizationId, then findMemberByOrgId({ userId: session.user.id, organizationId })403 YOU_ARE_NOT_A_MEMBER_OF_THIS_ORGANIZATION before any filter is applied. The old getActiveMember handler reads session.session.activeOrganizationId only and never ctx.query — the card's premise stands at this version.
    Security: no authorization widening. The named-org read is authorised by the caller's own membership in the named organisation, not by session state; a non-member cannot read any row. The SDK fixes filterValue to the caller's own id, and even an arbitrary filter would expose only what organizations.listMembers (same route, already sdk) exposes today. Anonymous callers are refused 401 by the same middleware; the SDK substitutes nothing client-side.
  4. Clause-②. PR body line 2 carries the literal Clause-②: yes; the card's governing claim now matches. The yes is correct on its own ground (input classes re-chosen between two published ADR-0112 codes; the row an existing caller receives changes). Ledger conformance: client-url-conformance.test.ts drives every method with a recording fetch, so getActiveMember is pinned to build only ledgered URLs (both hit sdk rows); auth-route-ledger-coverage.test.ts resolves client: names — and no row names organizations.getActiveMember any more, so the method is pinned by URL but no longer by name (see F1). return-type-precision.test.ts:950 pins the return type unchanged.
  5. Changeset. @objectstack/client: minor — correct level per batch [WIP] Add query enhancements and advanced validation features #35 (fix( that moves published behaviour cannot be patch on a clause-② yes; check-changeset-no-major level-axis green). No **BREAKING** banner and no ADR-0087 marker, so the gate is silent; whether one is owed is F3.
  6. Tests. 7 cases, no .skip/.only/.todo. Case ① is the revert-reddening pin (named org_bravo while org_alpha active → asserts organizationId === 'org_bravo', id === 'mem_b_self'); the double keeps the defect alive on get-active-member and ⑦ proves it can serve the wrong row, so ① fails on the value under a revert, not on a URL string. Case ⑤ is the negative control (non-member → 403 YOU_ARE_NOT_A_MEMBER… in the code/httpStatus envelope); ⑥ pins anonymous 401 with two requests on the wire. tsconfig.test.json includes src/**/*, so the new file is under check:test-typecheck (debt ledger 0/0). Note the negative control is against the fixture's model of the vendor gate, not the vendor; the server-side authorisation is pinned here by source reading (item 3), not by an in-repo integration test — acceptable, since packages/client has no plugin-auth edge.
  7. CI on eb75819: 39 check runs — 36 success, 3 skipped, 0 failure, 0 in progress. mergeable_state: clean. Head is 21 commits behind origin/main (5 ahead); none of the 21 touch packages/client/src/index.ts, the ledger, or the new test, and a dry merge-tree reports 0 conflicts.

Findings

F1 — ledger rows left incomplete by the PR's own standard (low, same file already in the diff). The PR rebooks the get-active-member row because "a truth ledger must not ship a false statement". By the same standard two rows are now incomplete: GET /api/v1/auth/get-session carries note: 'auth.me and auth.refreshToken both target it' while organizations.getActiveMember now targets it too, and GET /api/v1/auth/organization/list-members names only organizations.listMembers while getActiveMember now builds it (the invite-member row is the precedent for exactly this, with a note). No gate pins it (hence CI green), which is why it is a finding rather than a red. Expectation: extend both notes so the ledger lists every SDK method that builds each URL; that also restores a by-name anchor for getActiveMember, which item 4 shows is otherwise pinned by URL only.

F2 — empty organizationId silently answers the ACTIVE organisation (low). listMembers resolves ctx.query.organizationId || session.activeOrganizationId, so getActiveMember('') returns the active org's row at 200 — the card's "wrong-but-plausible, silently" class, surviving on one input while the JSDoc now says "the GIVEN organisation". Same behaviour as before the PR, so not a regression. Expectation: refuse a falsy id client-side with a loud error (the method already throws loudly for the empty-page case), or document the fallback in the JSDoc; one pinned case either way.

F3 — breaking-ness carrier: the changeset prescribes a migration for an existing caller class but declares no **BREAKING** and no ADR-0087 disposition (medium; maintainer's call). The changeset says, in its own words, "Callers that relied on passing an arbitrary id to read the ACTIVE organisation's row should pass the active organisation's id" — a FROM → TO prescription for callers who followed the JSDoc as it stands on main today (#14314's "the argument is ignored"). Under the launch-window convention the level cannot carry breaking-ness; the banner + ADR-0087 disposition are the only carriers, and check-adr-0087-registration is by design silent unless the author declares. Against that, AGENTS.md rule 3 defines a breaking changeset as one that "removes or renames anything an author can write", and this removes nothing: signature, type and export are unchanged, and the change restores the contract the method was published with. Both readings are defensible; this seat does not manufacture a ruling. Expectation: the maintainer decides. If breaking: add **BREAKING** with explicit FROM → TO lines (three inputs move: non-active org → that org's row; non-member → 403 YOU_ARE_NOT_A_MEMBER… where it was 400 MEMBER_NOT_FOUND; no active org → success where it was 400 NO_ACTIVE_ORGANIZATION) plus one ADR-0087 marker (the gate prints the category set). If not: no edit, and the ruling on the card closes the question for the next PR of this shape.

F4 — informational. The get-session step types its body inline as { user?: { id?: string } } | null rather than reusing auth.me, which declares the wrong SessionResponse envelope (#16760, filed by this PR). Correct choice given #16760 is open; when #16760 lands, this call should collapse onto auth.me.

Landing note

Draft, needs:contract-review on both carriers, Clause-②: yes, and F3 is a declaration decision the maintainer owns — this is a maintainer-only merge. Nothing here is a defect in the code: the addressing is correct, the authorisation is by membership, and the tests would redden on a revert.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants