Repository navigation
fix(plugin-auth): revalidate the memoized default organization id when a user is bound - #21905
Conversation
…nd recreate A user created after the default organization is deleted and recreated in the same process must bind to the new id. Red at this commit: the cached id is returned without checking that its row still exists. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
…every call TenancyService.defaultOrgId() returned its memoized id for the life of the process. A default organization deleted and recreated in that process left new users under the auto policy bound to the deleted id, and since membership is decided once at creation (ADR-0093 D7) nothing repaired them. The memo is now checked with one read of sys_organization by primary key. Only a definite absence drops it, and the id is then resolved again by the same rule that set it (slug default first, else the sole organization). An unanswered read keeps the memo rather than binding the user to nothing. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
…n two sign-ups Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
…ization id Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
…ootstrap The harness pins the single-org bootstrap off unless orgContext is set, so nothing recreated the deleted organization. Boot with orgContext and assert the bootstrap's organization and the admin's owner membership as premises. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
check:where-matcher found the new double reading a combinator key as a field name. It now throws on any key it does not implement. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9db48c6bb87813eb5135449cf0018b0b2feeaffd && git checkout 9db48c6bb87813eb5135449cf0018b0b2feeaffd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e6dc7a240617eaeef9a64e788bf6e5561c107f1b 37233dd57553f031d80801edba43913618edeed7 && git checkout -B drift-repro e6dc7a240617eaeef9a64e788bf6e5561c107f1b && git merge --no-ff 37233dd57553f031d80801edba43913618edeed7
node scripts/docs-audit/affected-docs.mjs --json e6dc7a240617eaeef9a64e788bf6e5561c107f1b
|
…fault-org-id-revalidated
…ut of this change The booted scenario deletes the default organization through the organization delete door, and that door fails before the bind it measures whenever a federated object is provisioned on the runner: the engine's cascade scan probes the federated object's injected organization_id and the remote table has no such column. The scenario moves to that defect's finding as its acceptance pin. The memo fix stays covered by the unit pin in tenancy-service.test.ts and its ablation. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
|
CI note from
Generated by Claude Code |
…fault-org-id-revalidated
…ctory (objectstack-ai#21919) Fixes objectstack-ai#21914 Clause-②: no ## What changes Every dogfood test file now runs in its own temporary working directory. The suite fails when any file leaves `.objectstack/data` in `packages/qa/dogfood`. - **`test/per-file-cwd.setup.ts`** (new) is a `setupFiles` entry, wired explicitly in BOTH projects of `vitest.config.ts`, because inline projects inherit nothing from the root block. `shared-showcase` keeps `isolate: false`; the module still runs once per file there. - At module top level, before the test file's own imports, it creates a directory under the run's temporary root and `chdir`s into it. - In `afterAll` it restores the previous cwd. That `afterAll` is also **the guard**: it THROWS when `packages/qa/dogfood/.objectstack/data` exists. The message names the directory, its entries and the remedy. It also says the named file may be a concurrent one on another worker rather than the writer, and whether the directory was already present when the file started. - **`test/per-file-cwd.global-setup.ts`** (new) is a root-level `globalSetup`. It runs once per run, covering both projects and each `OS_TEST_SHARD` slice (measured). - At the START it clears a stale `packages/qa/dogfood/.objectstack`, so a developer's earlier run never reds the suite. - It creates one temporary root for the run and hands it to the workers with `provide` / `inject`. - At the END it removes that root, which is **where the per-file directories are removed**. The removal is run-level, not per file, because the memoized `shared-showcase` boot keeps its SQLite handles open in the directory of the file that booted it. - The teardown judges nothing (see Evidence: a throwing teardown is a false green). - **`vitest.config.ts`** wires the two modules. A header section explains why there are two halves and why the guard is not in the teardown. - **`test/enterprise-organizations.ts`**: the module-level `probeOrganizations()` now passes this package's root as `hostRoot`, resolved from the module's location (`new URL('..', import.meta.url)`), not the cwd. This was measured to be needed; see Evidence. No per-file edits. The five files the card names, and the other 87 measured writers, are covered by the module with no change of their own. Test isolation only: `@objectstack/dogfood` is `private: true`, so no published package moves and there is no changeset (`skip-changeset`). ## The invariant for every dogfood author - **Each test file runs in its own temporary cwd.** Anything it writes relative to the cwd is its own, no other file sees it, and it is removed at the end of the run. A file needs no `mkdtemp` / `chdir` of its own. - **A package-relative read must resolve from the module's location** (`new URL('..', import.meta.url)`, `import.meta.dirname`), never from `process.cwd()`. The cwd is a temporary directory. - **A file that writes into `packages/qa/dogfood/.objectstack/data` fails the run.** That happens through an absolute path built from the package root, or through a `process.chdir()` back to the package directory before a boot. The fix is to write relative to the file's own cwd. - Files that already `chdir` into a temp dir of their own still work, because they restore to the per-file directory. Their own `chdir` is now redundant and harmless. ## Why (measured) A per-file probe over the whole suite measured 92 test files leaving `.objectstack/data/showcase_external.db` in the package directory, not the five the card names: - 7 leave the populated federated fixture (24576 B, 2 tables): the card's five, plus `showcase-demo-personas-loginable` and `showcase-demo-personas-membership`, which pass `onEnable` in the bundle. - 85 leave an empty SQLite file (4096 B, 0 tables). The showcase's declared external datasource has a cwd-relative filename, and its auto-connect creates the file on every showcase boot, `onEnable` or not. A later boot on the same runner found or missed the federated tables depending on which files ran before it, and that ordering is how PR objectstack-ai#21905 went red only on dogfood shard 3/3. The seat chose this route (one module) and this guard (comment `6004950414` on objectstack-ai#21914), on the dev's measurement (comment `6004909676`). ## Evidence All runs are at head `967ce88a`, under the shared verify lock, from a clean package directory. - **Whole suite**: `pnpm --filter @objectstack/dogfood test` gave `Test Files 205 passed | 1 skipped (206)` and `Tests 1591 passed | 9 skipped (1600)`. Afterwards `packages/qa/dogfood/.objectstack` does not exist, and no `os-dogfood-run-*` root is left in the temp dir. - **CI's three-shard split**: CI's dogfood leg exports `OS_TEST_SHARD=k/3` and `vitest.config.ts` turns it into vitest's `shard`. Here each shard ran as `OS_TEST_SHARD=k/3 pnpm --filter @objectstack/dogfood test`: the same vitest selection, without turbo, so no cached replay. Each exited 0 and left no `.objectstack`: | shard | Test Files | Tests | |---|---|---| | 1/3 | 69 passed (69) | 507 passed (507) | | 2/3 | 69 passed (69) | 461 passed, 1 skipped (462) | | 3/3 | 67 passed, 1 skipped (68) | 623 passed, 8 skipped (631) | The three add up to the whole run: 206 files, 1600 tests. - **Ablation (H4)** through `scripts/ablation-replace.mjs`, wrap mode. The central `process.chdir(...)` was replaced by the bare `mkdtempSync(...)`: anchor count 1 to 0, blob `0991eb9c` to `ee5a65be`. - With the chdir dropped, `showcase-external-autoconnect` and `showcase-search` ran: `Test Files 2 failed (2)`, `Tests 8 passed (8)`, exit 1. Each failed in the guard: `.../packages/qa/dogfood/.objectstack/data exists after this test file ran. Entries: showcase_external.db` (plus `-shm` / `-wal` for the shared-showcase file). - Restore was proven by the tool: blob after restore equals HEAD (`0991eb9c`), and `git diff HEAD` is empty. - The same two files then gave `2 passed`, exit 0, and left nothing. - No build step is involved: vitest loads the mutated module from source. - **Stale directory**: `.objectstack/data/x.db` was planted, then 9 files were run. Result: `Test Files 9 passed (9)`, exit 0, nothing left (the globalSetup cleared it). - **Census**: those 9 files are the 7 populated-fixture writers plus `showcase-search` and `showcase-permission-zoo`, both `shared-showcase` files. - **`hostRoot` line, measured both ways**, running `rls-multitenant`, `org-create-default-team` and `enterprise-organizations.test`: - Without the line (commit `4d07dc29`), the skip text read `not resolvable from /tmp/os-dogfood-run-.../file-...` and told the reader to declare the package in that temp directory's `package.json`. - With it (`967ce88a`), the text names `packages/qa/dogfood/`. - The verdict is the same both ways (skipped), because no framework package declares `@objectstack/organizations`. - **Guard placement**: a throwing `globalSetup` teardown was measured on vitest 4.1.11 to print `error during close` and still exit 0, a false green. So the guard is the per-file `afterAll`. (A teardown that sets `process.exitCode = 1` does exit 1, but the summary still reads all-passed.) - **Typecheck and lint**: `pnpm --filter @objectstack/dogfood typecheck` is green, and `tsc --listFiles` includes both new modules and `enterprise-organizations.ts`. `pnpm lint` exits 0. - **Gates**: 130 commands at `967ce88a`, the dispatch list plus `pnpm check:dispatcher-error-vocabulary` from `dispatch-gates --commands`. `dispatch-gates --ran`: `48 derived famil(ies) accounted for — 48 run, 0 NOT-MEASURED`. - `check:dual-build-cjs-loads` and `check:published-readme-exports` first exited 3 (dist prerequisite: 7 packages unbuilt). After building those 7, both exit 0. - The three PR-context scripts (`check-closing-target-claim`, `check-partof-closing-keyword`, `check-single-claim-paths`) are re-run with this PR's context; the results are in the report on the card. ## Open PRs that add dogfood files | PR | new file | boots the showcase | own `chdir` | under this PR | |---|---|---|---|---| | objectstack-ai#21864 | `showcase-public-form-withdrawal-layers.dogfood.test.ts` | yes | no | Covered with no author action. Without this PR it would leave `.objectstack/data` in the package directory. | | objectstack-ai#21917 | `organization-delete-federated-fixture.dogfood.test.ts` | yes, with `onEnable` | yes | Unaffected; its own `chdir` is redundant. | | objectstack-ai#21906 | `external-import-code-datasource-namespace.dogfood.test.ts` (also edits three `external-*` files) | yes, with `onEnable` | yes | Unaffected. None of its files is edited here. | | objectstack-ai#21877 | `datasource-contractless-credentials.dogfood.test.ts` | yes | yes | Unaffected. | | objectstack-ai#21897 | `flow-node-config-values-at-registration.dogfood.test.ts` | no (fixture stack) | no | Runs in its own temp cwd; it reads nothing relative to the cwd. | None of these files reads a package-relative path through `process.cwd()`. Only their own `prevCwd` captures do. ## Acceptance notes - **Observation, not filed.** The showcase's external datasource is declared read-only (`schemaMode: 'external'`, `allowWrites: false`). Its auto-connect CREATES a missing `.objectstack/data/showcase_external.db`, plus `-wal` / `-shm` (measured on 85 harness boots). - The declaration's own comment in `showcase-external.datasource.ts` says that if the fixture file cannot be opened, "the boot stops with that as the reason rather than serving a showcase whose federation pages are quietly dead". - It was measured only through the verify harness's `bootStack`, never at a public door (`os start` / `os dev`), so it stays here. - **Latent, unreachable today.** `bootStack(..., { multiTenant: true })` also defaults its `hostRoot` to the cwd: `rls-multitenant.dogfood.test.ts:79`, and `attachments-permission-matrix.dogfood.test.ts:766` through `bootFixture`. Both are gated on `organizationsAvailable`, which is false in this repository because no framework package may declare `@objectstack/organizations` (ADR-0132). A run that declares it in this package would need those boots to pass the package root too. Carrier: whoever declares it. - The own `chdir` in `external-validate-sees-runtime-save`, `external-import-destructive-remedy`, PR objectstack-ai#21906's file and PR objectstack-ai#21917's file is now redundant. It is left untouched and can be removed once objectstack-ai#21906 lands. Carrier: the `domain:cli` seat. - Attribution limit: under parallel workers, the guard can name a file that ran at the same time as the writer. The message says so, and says whether the directory was already present when the named file started. --- _Generated by [Claude Code](https://claude.ai/code/session_01RWZbGvPFcRKvUqASZtunCU)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #21868
Clause-②: no
What
TenancyService.defaultOrgId()(packages/plugins/plugin-auth/src/tenancy-service.ts) memoized the default organization id for the life of the process and returned it without checking. The single-org bootstrap recreates a missingslug='default'organization on the nextsys_userinsert, under a new id. That same sign-up's membership bind then read the memo and bound the user to the deleted id. Under theautopolicy membership is decided once, at creation (ADR-0093 D7, the ruling recorded on #21791 and landed by #21813), so nothing repairs that bind later.tenancy.defaultOrgId(): the creation bind and the first-session settle inAuthManager, the self-registration grant, the admin create-user bind, thekernel:readybackfill, the anonymous form doors in@objectstack/rest, the check on organization-scoped form writes in@objectstack/metadata-protocol, and the email-template bootstrap. The check lives in the accessor once, with no copy per caller.sys_organizationby primary key (where: { id },limit: 1). If the row exists, the memo is returned and nothing is re-resolved.resolveDefaultOrgIdruns again, the resolver that set it: theslug='default'organization first, else the only organization. The replacement is what a fresh boot would pick. If nothing exists yet, the answer isnulland the next call resolves again.sys_organizationhook, no time-based expiry, no change to the membership policy, no new export.Tests
Red first. The pin was committed before the fix (
69916d1e12). There,src/tenancy-service.test.tsfailed:usr_secondwas bound toorg_old, expectedorg_new(1 failed, 34 passed).Unit (
plugin-auth,src/tenancy-service.test.ts, 39 passed). Added:reconcileMembership; the user binds to the new id;findper call, by id, and is not re-resolved;null, then the replacement;The existing
memoizes a positive resolutiontest asserted that a memoized call issued no query at all, which is the defect. It now asserts exactly one read, by primary key.Door pin (new file,
packages/qa/dogfood/test/default-organization-recreated-binds-new-id.dogfood.test.ts, 1 passed). A real showcase boot withorgContext, the harness switch for the real single-org bootstrap. The admin signs up the first user, who is bound to the bootstrap's organization. The admin deletes it through better-auth'sPOST /auth/organization/delete(200). The next sign-up recreates it, and that user's membership and first session carry the recreated id.Ablation through
scripts/ablation-replace.mjs, from the committed fix at13d72b32f9. The plugin-auth source has not changed since. The memo was made unconditional again (markerABLATION_21868_UNCHECKED_MEMO). Mutation landed: anchor 1 to 0, blobf0012491afbfto5856726a7b5f.plugin-authwas rebuilt andablation-dist-preflightfound the marker in 2 built files.expected 'org_old' to be 'org_new'.expected 'org_muvizcge3a2f1lgy' to be 'org_muvizdq45anuk3zq'. The user was bound to the deleted organization, not the recreated one.f0012491afbf) andgit diff HEADis empty. After a rebuild,--absentreported the marker absent from all 14 built files and the tree clean. Unit 39 passed, dogfood 1 passed.Packages.
@objectstack/plugin-authfull suite at3e2e917f27: 123 files, 2575 passed, 10 skipped. Later commits changed only the new test double in plugin-auth, and that file was re-run (39 passed). Typecheck green: tsc, the examples, andcheck:test-typecheckwith debt held.@objectstack/dogfoodtypecheck green;--listFilesincludes the new test.Lint, narrowed.
eslint --no-inline-config --format jsonover the 3 touched.tsfiles: 3 files, 0 errors, 0 warnings. Type-aware linting is not enabled ineslint.config.mjs(noparserOptions.project), so this diff cannot move the verdict on any untouched file. The fullpnpm lintrun is left to CI.Gates, at
8fe4041e75.dispatch-gates --commandsderived 68 families. All 68 were run and every one exited 0. That includescheck:dual-build-cjs-loads, measured after building the 8 packages its prerequisite named.dispatch-gates --ranover the exit-coded record:68 derived, 68 run, 0 NOT-MEASURED, 0 UNRUN(a derived zero).check:where-matcherfirst found the new test double reading a combinator key as a field name. The double now refuses one (8fe4041e75), and the unit ablation was re-run on that head: 4 red, restored, blob equal to HEAD.Acceptance notes
defaultOrgId()call with a memo now costs one primary-key read ofsys_organization. That includes each anonymous form request, which reads it to pick the form's organization. When the memo is stale, the call also pays the existing resolution (one or two reads).content/docs/**became false.deployment/tenancy-modes.mdxdescribesdefaultOrgId()as the reconciler's target and says nothing about memoization.makeStore:,check:where-matcherreported the matcher as unjudged (could not lift: ReferenceError: orgs is not defined). Rewording the message cleared it. The lifter appears to pull in the enclosing factory when its name appears in that text. No carrier.origin/main. Main is 2 commits ahead (1e18a0735c): a spec test-title change and aplatform-objectsaction retirement. Neither touchesplugin-auth,dogfoodorverify. The merge ref is CI's.Seat's append: patch rounds 1 and 2 (written by
domain:servicesseat 1 from the dev's reports; the dev does not edit this body)8fe4041e75is root-caused, and it is not this change. The door pin'sPOST /api/v1/auth/organization/deleteanswered500on CI shard 3/3. The cause is a pre-existingobjectqldefect, filed as finding(objectql): deleting an organization answers 500 when a federated object is provisioned, because the cascade scan probes the remote table on the platform-injected organization_id #21910. The cascade relation scan probes a federated object's platform-injectedorganization_idagainst a remote table that has no such column, so every organization delete fails once a federated object is provisioned. Earlier dogfood files on the same runner had provisioned the showcase federated fixture. Locally that file was absent, so the pin was green.4303d35abb), as finding(objectql): deleting an organization answers 500 when a federated object is provisioned, because the cascade scan probes the remote table on the platform-injected organization_id #21910's acceptance pin.4303d35abb: 64 derived, 64 run, all exit 0.4303d35abbwas cut by the repo's runner starvation (the seat's note6003048037). It needs a fresh run.Generated by Claude Code