Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 62 additions & 2 deletions packages/client/src/auth-get-session-envelope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,42 @@
// `data.token` is absent from the normalized body too, so a regression back
// to `data.data?.token` cannot pass by accident, and the "enveloping it
// would have fixed refreshToken" reading stays refuted in code.
// - `⑥ the cold start stays outside every clocked window` — pins WHERE the
// first scenario's one-time cost is paid (next section).
//
// ## [#20327] Why one throwaway scenario runs at MODULE SCOPE
//
// The first scenario in a worker pays a one-time cost no later one sees:
// better-auth's lazily imported module graph, sql.js's WASM compile, and the
// first-use costs of the sync and the sign-up. The phases are measured in
// `auth-login-register-envelope.test.ts`, which carries the same arrangement
// and the same fix. Here, idle on 4 vCPU at `c74de10a9`, that cost put the
// first case at 1035 ms against 110-330 ms for the six after it. On a loaded
// `Test Core` shard (PR #20325's run) the first case crossed vitest's 5000 ms
// `testTimeout` while every other case passed. Twenty-four CPU-bound busy
// loops on this box reproduce that exact signature on the first case.
//
// So the cost is now paid by a module-scope `await`, during COLLECTION, which
// no vitest clock covers: `@vitest/runner@4.1.11` wraps hooks and test bodies
// in `withTimeout(...)` and awaits the file import bare. This is the repo's
// convention: "clocked windows measure behaviour, never loading" (AGENTS.md,
// Build & Test; `check:test-source-alias`). The warm-up is the file's own
// `signedIn()`, the arrangement five of the seven cases run, so no list of
// loads can drift from what the cases really pay.
//
// ⛔ It shares nothing a case asserts on. Its engine and manager are its own
// and are closed before any case starts. Every case still builds a fresh
// engine, a fresh `AuthManager` and a fresh sign-up. What it leaves warm is
// process-level: the module registry, sql.js's compiled WASM and the JIT,
// which the first case used to leave to every later case.
//
// ⛔ Do not move it into a hook, and do not answer a recurrence by raising a
// timeout: vitest clocks a hook exactly as it clocks a test body, and a wider
// window only moves the cliff to a heavier shard.

import { describe, it, expect, afterEach } from 'vitest';
import { readFileSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { ObjectQL } from '@objectstack/objectql';
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
import { AuthManager } from '@objectstack/plugin-auth';
Expand Down Expand Up @@ -250,12 +284,21 @@ const principalFor = async (manager: AuthManager, token: string | undefined) =>
return session?.user?.id ?? null;
};

afterEach(async () => {
/** Close every engine a scenario opened: after each case, and after the warm-up. */
const closeEngines = async (): Promise<void> => {
while (engines.length) {
const engine = engines.pop();
await (engine as unknown as { close?: () => Promise<void> })?.close?.().catch(() => {});
}
});
};

// [#20327] The first scenario's one-time cost, paid during COLLECTION, which
// no vitest clock covers (header, last section). ⛔ It stays at module scope,
// and `⑥` below pins that.
await signedIn();
await closeEngines();

afterEach(closeEngines);

describe('[#16760] /get-session is lifted into the SessionResponse envelope it declares', () => {
describe('① me() delivers the envelope it declares', () => {
Expand Down Expand Up @@ -427,4 +470,21 @@ describe('[#16760] /get-session is lifted into the SessionResponse envelope it d
expect(typeof res.data.session?.token).toBe('string');
});
});

describe('⑥ the cold start stays outside every clocked window', () => {
it('pays the first scenario at module scope, and no hook carries it', () => {
// [#20327] Read off this file's own text, so "do not move it into a
// hook" is an assertion rather than a sentence nobody reads.
const code = readFileSync(fileURLToPath(import.meta.url), 'utf8');

// Exactly one warm-up call, and it opens its own line at column 0. So it
// sits in no function body, which is what "paid during collection"
// reduces to. Comment lines start with `//` and cannot match.
expect(code.match(/^await signedIn\(\);$/gm) ?? []).toHaveLength(1);

// ⛔ No `before*` hook may come back to carry it: vitest clocks a hook
// with `hookTimeout` exactly as it clocks a test body with `testTimeout`.
expect(code).not.toMatch(/^\s*before(All|Each)\s*\(/m);
});
});
});
110 changes: 97 additions & 13 deletions packages/client/src/auth-rotated-session-token.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,51 @@
// that header in the shared `fetch` wrapper instead of on the rotating routes
// would rewrite the stored credential on an ordinary write — and every
// assertion in ① and ② would stay green.
// - `④ the cold start stays outside every clocked window` — pins WHERE the
// first scenario's one-time cost is paid (next section).
//
// ## [#20327] Why one throwaway scenario runs at MODULE SCOPE
//
// The first scenario in a worker pays a one-time cost no later one sees:
// better-auth's lazily imported module graph, sql.js's WASM compile, and the
// first-use costs of the sync and the sign-up. The phases are measured in
// `auth-login-register-envelope.test.ts`, which carries the same arrangement
// and the same fix. Every case here used to carry an explicit `60_000`
// timeout, which WIDENED the first case's window around that cost instead of
// moving the cost out of it: every budget a cost is moved into can be
// exhausted by a heavier shard (`check:test-source-alias`, clocked-window
// rule).
//
// So the cost is now paid by a module-scope `await`, during COLLECTION, which
// no vitest clock covers: `@vitest/runner@4.1.11` wraps hooks and test bodies
// in `withTimeout(...)` and awaits the file import bare. This is the repo's
// convention: "clocked windows measure behaviour, never loading" (AGENTS.md,
// Build & Test). The warm-up is the file's own `signedIn()`, the arrangement
// every case runs, so no list of loads can drift from what the cases pay. With
// the cost moved out, the `60_000`s are gone and every case runs on the
// default `testTimeout`.
//
// ⚠ What a window holds now is behaviour, and ① holds the most: the card's
// whole probe, a sign-up plus five more calls that each check a password or a
// TOTP code. It takes about 700 ms on 4 vCPU with the box otherwise quiet, and
// a warm-up that ran the WHOLE probe left it no faster than this one does, so
// none of that is loading. Under sixteen CPU-bound busy loops it took 4.1-4.2
// s. Under twenty-four, the load that times out every one of these suites'
// cold first cases, it reached the 5000 ms budget on its own work. If CI ever
// reds it, the lever is that case's own work, ⛔ not a timeout.
//
// ⛔ It shares nothing a case asserts on. Its engine and manager are its own
// and are destroyed before any case starts. Every case still builds a fresh
// engine, a fresh `AuthManager` and a fresh sign-up. What it leaves warm is
// process-level: the module registry, sql.js's compiled WASM and the JIT,
// which the first case used to leave to every later case.
//
// ⛔ Do not move it into a hook, and do not answer a recurrence by raising a
// timeout: vitest clocks a hook exactly as it clocks a test body.

import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { readFileSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { createHmac } from 'node:crypto';
import { ObjectQL } from '@objectstack/objectql';
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
Expand Down Expand Up @@ -261,12 +304,8 @@ const signedCredentialFor = async (
return String(signed);
};

beforeEach(() => {
vi.spyOn(console, 'warn').mockImplementation(() => {});
vi.spyOn(console, 'error').mockImplementation(() => {});
});
afterEach(async () => {
vi.restoreAllMocks();
/** Destroy every engine a scenario opened: after each case, and after the warm-up. */
const closeEngines = async (): Promise<void> => {
while (engines.length) {
const e = engines.pop();
try {
Expand All @@ -275,6 +314,21 @@ afterEach(async () => {
/* noop */
}
}
};

// [#20327] The first scenario's one-time cost, paid during COLLECTION, which
// no vitest clock covers (header, last section). ⛔ It stays at module scope,
// and `④` below pins that.
await signedIn();
await closeEngines();

beforeEach(() => {
vi.spyOn(console, 'warn').mockImplementation(() => {});
vi.spyOn(console, 'error').mockImplementation(() => {});
});
afterEach(async () => {
vi.restoreAllMocks();
await closeEngines();
});

// ───────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -335,7 +389,7 @@ describe("#16534 ① the card's own probe, with the manual re-set deleted", () =
// status code: after the whole sequence the client is still holding a
// credential that resolves to the same principal.
expect(await principalForStoredToken(manager, client)).toBe(userId);
}, 60_000);
});
});

// ───────────────────────────────────────────────────────────────────────────
Expand All @@ -358,7 +412,7 @@ describe('#16534 ② one assertion per rotating route — all three', () => {
expect(await principalForStoredToken(manager, client)).toBe(userId);
// …and the one it replaced is genuinely gone.
expect(await principalFor(manager, before)).toBeNull();
}, 60_000);
});

it('changePassword WITHOUT revokeOtherSessions rotates nothing and stores nothing', async () => {
// The other half of the same route: `token` is `null` there, and a client
Expand All @@ -374,7 +428,7 @@ describe('#16534 ② one assertion per rotating route — all three', () => {
expect(result.token).toBeNull();
expect(storedToken(client)).toBe(before);
expect(await principalForStoredToken(manager, client)).not.toBeNull();
}, 60_000);
});

it('twoFactor.verifyTotp on the enrolment lane — the body echoes the LIVE token', async () => {
const { engine, manager, client, email } = await signedIn();
Expand All @@ -390,7 +444,7 @@ describe('#16534 ② one assertion per rotating route — all three', () => {
// The row behind the replaced value was deleted, so asserting only "the
// stored token changed" would not have been enough.
expect(await principalFor(manager, before)).toBeNull();
}, 60_000);
});

it('twoFactor.disable — the credential arrives ONLY in the `set-auth-token` header', async () => {
// The route triage singled out: it answers `{ status: true }`, so an
Expand All @@ -410,7 +464,7 @@ describe('#16534 ② one assertion per rotating route — all three', () => {
expect(storedToken(client)).not.toBe(before);
expect(await principalForStoredToken(manager, client)).toBe(userId);
expect(await principalFor(manager, before)).toBeNull();
}, 60_000);
});
});

// ───────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -439,7 +493,7 @@ describe('#16534 ③ the negative control — a NON-rotating route changes nothi
// Still the same live session at the end of it — the invariant is "did not
// move", not "was emptied".
expect(await principalForStoredToken(manager, client)).not.toBeNull();
}, 60_000);
});

it("verifyBackupCode's already-logged-in lane leaves the stored credential byte-identical", async () => {
// `/two-factor/verify-backup-code` shares `AuthTwoFactorVerificationResult`
Expand Down Expand Up @@ -472,5 +526,35 @@ describe('#16534 ③ the negative control — a NON-rotating route changes nothi
expect(result.token).not.toBe(signed);
expect(storedToken(bearerClient), 'verifyBackupCode moved the stored credential').toBe(signed);
expect(await principalForStoredToken(manager, bearerClient)).toBe(userId);
}, 60_000);
});
});

// ───────────────────────────────────────────────────────────────────────────
describe('#16534 ④ the cold start stays outside every clocked window', () => {
it('pays the first scenario at module scope, no hook carries it, and no case widens its clock', () => {
// [#20327] Read off this file's own text, so "do not move it into a
// hook" is an assertion rather than a sentence nobody reads.
const code = readFileSync(fileURLToPath(import.meta.url), 'utf8');

// Exactly one warm-up call, and it opens its own line at column 0. So it
// sits in no function body, which is what "paid during collection"
// reduces to. Comment lines start with `//` and cannot match.
expect(code.match(/^await signedIn\(\);$/gm) ?? []).toHaveLength(1);

// ⛔ No `before*` hook may come back to carry it: vitest clocks a hook
// with `hookTimeout` exactly as it clocks a test body with `testTimeout`.
// This file's one hook is the console-spy `beforeEach`. Each hook is read
// up to the next column-0 `});`, so a hook indented inside a `describe`
// reads on to that block's close and cannot hide a scenario from this.
const hooks = [...code.matchAll(/^\s*before(?:All|Each)\s*\(([\s\S]*?)^\}\);$/gm)].map(
(m) => m[1],
);
expect(hooks).toHaveLength(1);
for (const body of hooks) expect(body).not.toMatch(/\b(signedIn|arrange)\(/);

// ⛔ And no case widens its own clock again. Every case here used to end
// `}, 60_000);`, which kept the cold start inside a wider window instead
// of moving it out.
expect(code).not.toMatch(/^\s*\},\s*[\d_]+\s*\);$/m);
});
});
Loading
Loading