Skip to content

Commit 49185cd

Browse files
refactor(plugin-auth): move AuthManager's actor and organization guards into auth-manager-actor-guards.ts (#25224 cut 1) (#25273)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuACdPWFJTLDcTrByFCHLzqb)_ Part of #25224 Clause-②: no Metadata-protocol: none Ruling-ref: 6348891160 The basis for the `Clause-②: no` line: this is a move. No accept set, export, route, refusal or answer changes. The `.d.ts` bar is ruling `6348891160` item 1 (on #25222, covering this card): `@objectstack/plugin-auth`'s published `dist/index.d.ts` and `dist/index.d.mts` are raw byte-identical to base (0 diff lines, so the union-order normalisation has nothing to do), with the ruling's positive control, and its entry exports the same 155 names under ESM and CJS (measured below). Cut 1 of the `auth-manager.ts` split: **the actor and organization guards.** Eight `AuthManager` method bodies, 358 lines, move unedited (re-indented one level) into one new module, `packages/plugins/plugin-auth/src/auth-manager-actor-guards.ts`, together with the one module-level block only they read. #25224 stays open for the later cuts: `buildPluginList`, `createAuthInstance` and the other areas remain in `auth-manager.ts`, each a later PR on the merged file. This follows the card's claim `6347670835`, amended by `6349004758`. That claim put `buildPluginList` in cut 1; measured, that move needs the module-level helpers only it reads plus a shared module for two names it shares with the rest of the file, about 3,075 changed lines before any reader, over the card's 3,000-line human-merge line. It becomes a later cut, landed by hand. The shape copied is #23050's (landed note `6136102437`). Branch: `claude/project-thread-lg3wrj`, the harness-pinned branch for this session. It names no issue, so the card relation is this body's first line. `auth-manager.ts` goes from 10,349 to 10,004 lines. The cut is +598 / −412 over 7 files (1,010 changed lines, under the 3,000-line line). Not a governed surface (`check-governed-merges.mjs --branch HEAD`: 0 of the paths hit the register). ## What moved | method | base body | lines | members it reads | |---|---|---:|---| | `resolveActor` | `:8176`–`:8238` | 63 | none | | `isPlatformAdminUserId` | `:8281`–`:8286` | 6 | `getDataEngine` | | `assertActorNotHeld` | `:8307`–`:8317` | 11 | `computeAuthGate` | | `assertOrganizationDeletable` | `:8365`–`:8455` | 91 | `getDataEngine`, `resolveActor`, `orgRolesMap`, `organizationDeleteStandingJudge` | | `assertRemoveMemberPermitted` | `:8457`–`:8555` | 99 | `getDataEngine`, `resolveActor`, `orgRolesMap` | | `assertRevokeSessionIdentifiesRecord` | `:8572`–`:8599` | 28 | `resolveActor` | | `assertAdminRevokeUserSessionIdentifiesRecord` | `:8619`–`:8658` | 40 | none | | `resolveMembershipRole` | `:8689`–`:8708` | 20 | `getDataEngine` | Lines are base `0f1eeee`. The members are a walk of every `this.X` in each body with the TypeScript compiler API: no bare `this`, no `this[...]`, no `arguments`, no `super`, no assignment to a `this` member. Module level: base `:1464`–`:1490`, `INVITATION_ROLE_CAP_READ_FAULT_CODE` and `invitationRoleCapReadFaultApiError`, read only by the moved bodies, verbatim. The imports that only the moved bodies used left with them (`hasPlatformAdminStanding` with its comment, `judgeHeldSession`, the remove-member, revoke-session and admin-revoke-session guard imports, and four names of the organization-delete import); each is imported by the new module from the module `auth-manager.ts` imported it from. ## The shape - **The class keeps every method**, at its place, with its name, signature and docblock: the published `.d.ts` lists each one. Its body is now a host literal of exactly the members that body reads, typed as a `Pick` of `ActorGuardHost`, plus `.call`. `resolveActor` and `assertAdminRevokeUserSessionIdentifiesRecord` read no member and call their function directly. - **`ActorGuardHost`** (in the new module) names the five members the bodies read, each typed as the class member (`AuthManager['getDataEngine']`, …), so a signature change on the class is a change there too. The new module imports `AuthManager` as a type only, so there is no runtime cycle. - **Methods go as late-bound closures** (`(...a) => this.getDataEngine(...a)`), so a test double or spy set on the manager is still the one a body reaches. That includes `resolveActor`, which three guards ask through the manager. - **Two fields go as getters**, `orgRolesMap` and `organizationDeleteStandingJudge`: the class reassigns both after construction (the first on every plugin-list build, the second when `AuthPlugin` hands the last-administrator guard over), and the guards read them after an `await`. - **The eight methods are no longer `async`.** Each returns the moved async function's own promise, so a caller awaits the same promise the body settles. Building the host cannot throw, so no method throws synchronously where it used to reject. - **Nothing re-exports the new module.** `index.ts` does `export * from './auth-manager.js'`, so exporting a helper from `auth-manager.ts` would have widened the published surface; the one helper block the bodies need moved with them instead. ## Readers re-pointed in the same PR Each reads `auth-manager.ts` by path and its reading included a moved line. Each now reads both files, with its sum unchanged. | reader | what it read | base | head | |---|---|---|---| | `api-error-code.pin.test.ts` | the per-file floor of `new APIError(` sites in `auth-manager.ts` (≥ 24) | 27 | 18 + 9 = 27, floor kept at 24 over the two files | | `better-auth-schema-parity.test.ts` | the plugin factory imports `auth-manager.ts` can load | 16 names | the same 16 over both files (14 in `auth-manager.ts` alone: `hasPermission` and `defaultRoles` moved, and the `stale` half named them) | | `managed-extension-fields.test.ts` | the same scan, its own copy | 16 names | the same 16 | | `organization-delete-read-fault.test.ts` | "the first `AuthManager` method on the stack" picks which read to fail | frames named `AuthManager.` plus a method name | also a frame of a moved body, which runs under the method's own name (a helper of the module that is no class method does not count) | | `packages/runtime/src/dispatcher-error-vocabulary.ts` (**cross-lane, `domain:cli`**) | the `YOU_ARE_NOT_ALLOWED_TO_DELETE_THIS_MEMBER` row's `file:` (the stamp sits in `assertRemoveMemberPermitted`) | `auth-manager.ts` | `auth-manager-actor-guards.ts`; verdict and evidence unchanged | Each went red at head before its edit and green after (below). No `docs/qa/platform-checklist` anchor, no ADR anchor (`scripts/adr-anchors/…auth-manager.ts.json`) and no docs page names a moved symbol (measured by name across the tree). The other readers of the file by path were read and do not read a moved line: `check-auth-mount-ledger.mjs` (the base-path default), `tenant-audit-census.mjs` (the SCIM adapter row), `query-options-erasure-baseline.json` (its count for `auth-manager.ts` holds), `check-dispatcher-error-vocabulary.mjs`'s live member (`USER_ALREADY_EXISTS…`), `auth-manager-base-path.test.ts`, and the `deliverPhoneOtp` row. ## Proof Builds and suites ran through `os-verify-lock`, every exit code captured before any pipe. Base `0f1eeee`; head `26c8debee0` (the move `18633671`, the readers `e55cb6d9`, and a changeset added in `e315a803` and dropped in `26c8debe` once ruling `6348891160` item 4 was read, so the tree's delta is the 7 files above). **1. The moved text.** For each of the eight methods, the base class body and the new module's function body are the same TypeScript token sequence (scanner tokens, kind and text, trivia skipped), every line is the base line minus exactly two leading spaces, the parameters and return type are the same text, and the class method keeps its docblock and signature with `async` dropped: 8 of 8 pass, 358 body lines. Base `:1464`–`:1490` (27 lines) is verbatim in the module and absent from `auth-manager.ts`. An independent reading: `git diff --color-moved=plain --color-moved-ws=allow-indentation-change 0f1eeee 26c8deb` over the two files marks 398 of the 407 deleted lines and 435 of the 566 added lines as moved. The 9 other deleted lines are the 8 `private async …(` signature lines (their methods stay, without `async`) and the `hasPlatformAdminStanding,` import name; the 131 other added lines are the 8 class methods' new signature lines and host literals, `ActorGuardHost`, the module header, the per-function pointer docblocks and the imports. **2. `.d.ts` and exports (ruling `6348891160` items 1 and 2).** Built through `turbo run build` at base and at head: | file | raw base | raw head | raw diff lines | normalised base | normalised head | |---|---|---|---:|---|---| | `dist/index.d.ts` | `82729d0115e4a866ed30dde301da18f96bad321e9a78fcd12b2cdd77bca3801f` | `82729d0115e4a866ed30dde301da18f96bad321e9a78fcd12b2cdd77bca3801f` | 0 | `27d278f6825004c0b604eb60bbf9b3cd0fd8f66a89300bbe62bf7304238f6802` | `27d278f6825004c0b604eb60bbf9b3cd0fd8f66a89300bbe62bf7304238f6802` | | `dist/index.d.mts` | `9665953d56da469ac783292e8406d09544f8cfe219bac7ef892625fe9a73f96a` | `9665953d56da469ac783292e8406d09544f8cfe219bac7ef892625fe9a73f96a` | 0 | `16f1ed01f0391a8bb5d7de138cbd0f9a7915b83faa9675009cc2996d8cbdb841` | `16f1ed01f0391a8bb5d7de138cbd0f9a7915b83faa9675009cc2996d8cbdb841` | - The normaliser, verbatim (the ruling's one-liner): `node -e 'const s=require("fs").readFileSync(process.argv[1],"utf8");process.stdout.write(s.replace(/"[^"\n]*"(?:\s*\|\s*"[^"\n]*")+/g,r=>r.split(/\s*\|\s*/).sort().join(" | ")))' dist/index.d.ts | sha256sum`. - The raw files are already byte-identical, so no union-order swap occurs in this cut: items 1(b) and 1(d) hold with nothing to account for. - **Positive control:** the head `dist/index.d.ts` with one private member made public (`private resolveActor;` → `resolveActor;`, line 3662, a one-line change) hashes raw `35f7e05b837ddc0f4610783c831b6857e7c5ae7b3bafd23d726ad871255ea299` and normalised `284877cf4dad17a7fe392f3ba8600f57ed6e5321c33a5109bd96b98e1bb66a29`, both different from the readings above. - **Export lists:** ESM `dist/index.mjs` 155 names and CJS `dist/index.js` 155 names at base and at head, the same list (sha256 of the sorted names `ed3ac8b902031d30…` on all four). **3. Tests.** `pnpm exec vitest run --maxWorkers=3` in `packages/plugins/plugin-auth` at `e55cb6d9` (no source changed after it): `Test Files 185 passed (185)`, `Tests 3487 passed | 10 skipped (3497)`, VERDICT command-exit 0, the same counts as base. Each re-pointed reader, red before its edit and green after: - at `18633671` (the move alone), the full suite: `Test Files 3 failed | 182 passed (185)`, 7 tests: `organization-delete-read-fault.test.ts` 5 of 7 (the injected read never fired, so the delete went ahead), and both scans with `AUTH_MANAGER_PLUGINS names plugins auth-manager.ts no longer imports: defaultRoles, hasPermission`; - `api-error-code.pin.test.ts` at that head, run alone: `expected 18 to be greater than or equal to 24`, exit 1. (The full run at that head did not report this file red; not reproduced, and the re-pointed floor is green in both shapes.) - `pnpm check:dispatcher-error-vocabulary` at that head: exit 1, `[stale-row] … 'YOU_ARE_NOT_ALLOWED_TO_DELETE_THIS_MEMBER' at packages/plugins/plugin-auth/src/auth-manager.ts (objlitconst) but the scan no longer finds it` and the matching `[unclassified-site]` in `auth-manager-actor-guards.ts`. After the row's `file:` moved: exit 0, `56 unregistered code-stamping site(s) found; 56 classified`. `pnpm --filter @objectstack/plugin-auth typecheck` (both `tsc` projects and `check:test-typecheck`): exit 0. `pnpm --filter @objectstack/runtime typecheck`: exit 0, after the full build (before it, five `TS2307` in untouched test files naming unbuilt packages). `eslint --no-inline-config` (the root `lint` script's flags) over the 7 files: exit 0. **4. Gates.** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `26c8debee0`: 59 commands. All 59 ran after a full `turbo run build` and after deepening the clone; all 59 exit 0. `--ran`: `59 derived, 59 run, 0 NOT-MEASURED, 0 UNRUN`. `node scripts/pm/check-governed-merges.mjs --branch HEAD`: NOT governed, 1,010 changed lines. Census gates, base against head (base read in a scratch worktree of `0f1eeee` with no build): the same summary for `check-system-context-census`, `check-tenant-audit-census`, `check-registry-log-declared`, `check:durability-log-level`, `check:auth-mount-ledger` and `check:cross-package-test-inputs`. Only scanned counts move, by the new module (and, beyond it, the git-ignored `packages/spec/src/migrations/registry.ts` the head build generates): `check-undeclared-dep-imports` 3280 → 3282 files and 2813 → 2817 specifiers (the module's four `@objectstack/*` imports), `check:logger-receiver-detach` 3596 → 3598 files, `check:org-identifier` 3623 → 3624 files. `check:query-options-erasure` at head: 62 sites in 16 files, none new. ## Changeset None: `skip-changeset`, per ruling `6348891160` item 4. The published declarations and export lists are unchanged; the bundle's bytes move only because code moved inside it. `packages/runtime`'s edit is a row of a table nothing in that package's entry imports: its built `dist/` carries neither the table nor the code string (measured, 0 matches). --- _Generated by [Claude Code](https://claude.ai/code/session_01LSXBc2GCBwiAL7vV4kWkyr)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LSXBc2GCBwiAL7vV4kWkyr --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1f78b64 commit 49185cd

7 files changed

Lines changed: 598 additions & 412 deletions

‎packages/plugins/plugin-auth/src/api-error-code.pin.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,12 @@ describe('every new APIError( in plugin-auth/src carries an ADR-0112 code (#2424
173173
// 24, `objectql-adapter.ts` 3, `impersonation-bearer-rotation.ts` 2,
174174
// `admin-impersonate-endpoint.ts` 1). A floor, not an exact count, so a new
175175
// throw does not have to edit this line; only losing them does.
176+
// [#25224] `AuthManager`'s throws are counted over the files its methods'
177+
// bodies live in: `auth-manager.ts` and the bodies moved out of it whole.
178+
const authManagerFiles = ['auth-manager.ts', 'auth-manager-actor-guards.ts'];
176179
expect(sites.length).toBeGreaterThanOrEqual(30);
177180
expect(new Set(sites.map((s) => s.file)).size).toBeGreaterThanOrEqual(4);
178-
expect(perFile.find((f) => f.file === 'auth-manager.ts')?.sites.length ?? 0).toBeGreaterThanOrEqual(24);
181+
expect(perFile.filter((f) => authManagerFiles.includes(f.file)).reduce((n, f) => n + f.sites.length, 0)).toBeGreaterThanOrEqual(24);
179182
});
180183

181184
it('the textual control agrees with the walk in every file', () => {

‎packages/plugins/plugin-auth/src/auth-manager-actor-guards.ts‎

Lines changed: 504 additions & 0 deletions
Large diffs are not rendered by default.

‎packages/plugins/plugin-auth/src/auth-manager.ts‎

Lines changed: 62 additions & 407 deletions
Large diffs are not rendered by default.

‎packages/plugins/plugin-auth/src/better-auth-schema-parity.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,11 @@ function betterAuthPluginSet(): unknown[] {
319319
* they declare no schema.
320320
*/
321321
function authManagerPluginFactories(): string[] {
322-
const source = readFileSync(join(PKG, 'src', 'auth-manager.ts'), 'utf8');
322+
// [#25224] The auth manager's source: `auth-manager.ts` and the method
323+
// bodies moved out of it whole. A factory imported in either is one it loads.
324+
const source = ['auth-manager.ts', 'auth-manager-actor-guards.ts']
325+
.map((file) => readFileSync(join(PKG, 'src', file), 'utf8'))
326+
.join('\n');
323327
const found = new Set<string>();
324328
const pattern =
325329
/const\s*\{\s*([A-Za-z0-9_]+)\s*\}\s*=\s*await import\('((?:better-auth\/plugins|@better-auth\/)[^']*)'\)/g;

‎packages/plugins/plugin-auth/src/managed-extension-fields.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,11 @@ function findCollisions(
629629
* they declare no schema.
630630
*/
631631
function authManagerPluginFactories(): string[] {
632-
const source = readFileSync(join(HERE, 'auth-manager.ts'), 'utf8');
632+
// [#25224] The auth manager's source: `auth-manager.ts` and the method
633+
// bodies moved out of it whole. A factory imported in either is one it loads.
634+
const source = ['auth-manager.ts', 'auth-manager-actor-guards.ts']
635+
.map((file) => readFileSync(join(HERE, file), 'utf8'))
636+
.join('\n');
633637
const found = new Set<string>();
634638
const pattern =
635639
/const\s*\{\s*([A-Za-z0-9_]+)\s*\}\s*=\s*await import\('((?:better-auth\/plugins|@better-auth\/)[^']*)'\)/g;

‎packages/plugins/plugin-auth/src/organization-delete-read-fault.test.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,21 @@ function recordAttemptedWrites(engine: ObjectQL): string[] {
154154
return attempted;
155155
}
156156

157+
/**
158+
* The first `AuthManager` method on `stack`: an `AuthManager.<method>` frame,
159+
* or [#25224] a frame of a method's body moved whole into
160+
* `auth-manager-actor-guards.ts`, which runs under the method's own name (a
161+
* helper of that module that is no method of the class does not count).
162+
*/
163+
function firstAuthManagerMethod(stack: string): string | undefined {
164+
const methods = new Set(Object.getOwnPropertyNames(AuthManager.prototype));
165+
for (const match of stack.matchAll(/AuthManager\.(\w+)|at (?:async )?(?:Object\.)?(\w+) \([^)\n]*auth-manager-actor-guards\.ts:/g)) {
166+
if (match[1] !== undefined) return match[1];
167+
if (methods.has(match[2])) return match[2];
168+
}
169+
return undefined;
170+
}
171+
157172
/** The current stack, deep enough to reach the request's frames (V8 keeps ten by default). */
158173
function deepStack(): string {
159174
const limit = Error.stackTraceLimit;
@@ -191,7 +206,7 @@ function failAdmissionRead(engine: ObjectQL, read: AdmissionRead) {
191206
const real = target[method].bind(engine);
192207
return vi.spyOn(target, method).mockImplementation(async (name: string, ...rest: unknown[]) => {
193208
const stack = deepStack();
194-
const first = /AuthManager\.(\w+)/.exec(stack)?.[1];
209+
const first = firstAuthManagerMethod(stack);
195210
if (fired < times && name === object && stack.includes('assertOrganizationDeletable') && stack.includes(frame) && first === caller) {
196211
fired += 1;
197212
throw new Error(`${object} store unavailable (injected)`);

‎packages/runtime/src/dispatcher-error-vocabulary.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,8 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [
434434
},
435435
{
436436
code: 'YOU_ARE_NOT_ALLOWED_TO_DELETE_THIS_MEMBER',
437-
file: 'packages/plugins/plugin-auth/src/auth-manager.ts',
437+
// [#25224] The stamp moved with `assertRemoveMemberPermitted`'s body.
438+
file: 'packages/plugins/plugin-auth/src/auth-manager-actor-guards.ts',
438439
shape: 'objlitconst',
439440
door: 'none',
440441
verdict: 'foreign-vocabulary',

0 commit comments

Comments
 (0)