refactor(server)!: retire the credentials Better Auth replaced

[bestony]

Stage 5 of the Better Auth migration, stacked on #203. Do not merge until #203 has shipped and the compatibility window has actually closed — this is the one step no rollback recovers.

The signed JWT pair, the hand-rolled Google OIDC flow, the identity resolver, and the routes and cookies that carried them are gone. A Better Auth session is the only Account credential the server issues or accepts.

Why this can only be last

Rolling back code does not roll back migrations, and every earlier stage was expand-only so the previous revision kept working against the new schema. That is what lets users_email_unique arrive at all: it could not exist while a revision that wrote unnormalized addresses might still be serving, and no migration can assert which binary is running. Its guard refuses rather than choosing a row to discard.

The index is not tidying. The identity resolver enforced one Account per address by taking a lock on it before deciding whether to create or attach. Deleting the resolver without replacing that would leave two concurrent first sign-ins for the same address free to create two Accounts — Better Auth's trusted-provider linking attaches a returning address, but nothing in it orders a race for a new one. The index is what takes over that job, case-insensitively so a writer that skipped normalization cannot get in through a casing variant.

Three deviations from the plan

OPENTAG_JWT_SECRET stays. The plan assumed it served only Account auth. It also signs Slack OAuth state, which is not Account authentication and is not part of this migration. Its documented purpose is narrowed to that; jose stays for the same reason.

The double-submit CSRF token stays. The plan expected Better Auth's origin check to cover the browser surface. #201 established the opposite: the mount is allowlisted to the OAuth callback alone, so every authenticated route is OpenTag's, behind OpenTag's own boundary. Removing a working defense there would be a security change wearing a cleanup's clothes.

The unique index is added here rather than deferred further. It has the same precondition as everything else in this PR, and as above, removing the resolver without it would leave the invariant unenforced.

Smaller consequences

The browser no longer retries a 401 through a refresh endpoint: a session renews as it is used, so a 401 means it is genuinely gone and the exchange had nothing left to offer. POST /api/v1/auth/browser/refresh and the legacy Google callback are removed with it.

validateOAuthNext survives the deletion of oauth/state.ts and moves to its own module with its own tests. Better Auth's trustedOrigins decides which origins may receive a callback; it does not decide which paths within this one a next parameter may name, and that allowlist is the only thing between a sign-in link and an open redirect.

Deployment

One migration, expand-only in form but contract in effect. OPENTAG_ACCESS_TOKEN_TTL_SECONDS and OPENTAG_REFRESH_TOKEN_TTL_SECONDS can be removed from the deployment after this ships; OPENTAG_SESSION_TTL_SECONDS (from #203, defaulted) is the only lifetime left. The old /api/v1/auth/google/callback redirect URI can come out of the Google OAuth client.

Run the duplicate-address check against the target database before deploying, since the migration refuses rather than guessing:

select lower(email) as email, count(*), array_agg(id order by created_at) as ids
from users group by 1 having count(*) > 1;

Validation

pnpm check, pnpm build, pnpm typecheck, pnpm test (1262), and pnpm --filter @opentag/server test:integration (199) all pass.

[baixiaohang]

Recommendation: request changes

• Rationale: The credential cleanup is internally coherent, but the canonical setup and auth documentation still directs operators to an endpoint this PR removes and describes the retired credential and uniqueness implementations as current.

Risk level: B-high

• Path baseline: adds a database uniqueness migration under packages/server/** -> B-high
• Semantic lift: none; the path baseline already covers the core Account credential and identity-invariant changes

PR summary

• Author / repo: bestony / first-tree-ai/opentag
• Problem: after the Better Auth compatibility window closes, operators and users need one Account credential model, with no legacy JWT/OIDC routes left to accept or issue credentials.
• Approach: remove the signed access/refresh bridge, hand-rolled Google flow, legacy browser routes and cookies, and identity resolver; keep Better Auth sessions as the sole credential and move the one-Account-per-email invariant to a case-insensitive unique index that refuses ambiguous existing data.
• Impacted modules: server auth composition and routes, Account schema/migrations, browser API behavior, shared HTTP paths, authentication tests, and deployment/development documentation.

Review findings

❌ 1. The Google setup instructions still tell operators to register /api/v1/auth/google/callback, but this PR deletes that route and publishes only Better Auth's /api/v1/auth/callback/google. A new environment following the canonical English or Chinese guide will therefore reach Google's redirect-URI rejection (or return to a 404) even though the server is otherwise configured correctly. The same paragraph still says browser access/refresh JWTs are stored in cookies, despite this PR making a Better Auth session the only browser credential. Update both language guides to the surviving callback and session behavior. [R5 / DEVELOPMENT.md:289, DEVELOPMENT.zh-CN.md:275]

❌ 2. The adjacent durable contract still says the identity resolver serializes Account creation and the unique index merely backs it up, while this PR deletes that resolver and makes users_email_unique the database enforcement boundary. The environment table also still calls OPENTAG_JWT_SECRET the Account access-token signing secret, contradicting both the code and this PR's own Slack-only rationale. Please align both English and Chinese copies with the post-cleanup sources of truth so the irreversible stage does not land with two conflicting auth models documented side by side. [R1, R5 / DEVELOPMENT.md:273, DEVELOPMENT.md:381, DEVELOPMENT.zh-CN.md:261, DEVELOPMENT.zh-CN.md:358]

✅ 3. The migration refuses case-insensitive duplicates before creating the index, and the implementation keeps the CSRF and sign-in-destination boundaries instead of treating them as legacy credential code.

Action taken

• Submitted request changes on 8fd846e651567f6d31402bac4b0987ba01dc5e59.

[bestony]

Both correct, and both were mine to catch. Fixed in 73b8ed9.

1 — the setup guide pointed at a route this stage deletes. Worse than stale: someone setting up a new environment by following it would register /api/v1/auth/google/callback with Google, and then be rejected at the redirect URI or land on a 404 with the server otherwise correctly configured — with nothing in the guide to suggest why. Both language copies now give Better Auth's own callback and say the old one can come out of the OAuth client. The same paragraph's claim about access/refresh JWTs in cookies is replaced with the session that actually lives there.

2 — the contract paragraph had the enforcement backwards. It described the identity resolver as what guarantees one Account per address, with users_email_unique merely backing it up. This stage deletes that resolver and makes the index the enforcement, which is the whole reason the index could not land earlier — so the doc was describing the pre-condition as though it were still the design. It now says the index enforces it, that Better Auth's linking does not order two concurrent first sign-ins, and why the timing had to be this way.

The environment table's OPENTAG_JWT_SECRET row is fixed too. It still called it the Account access-token signing secret, which contradicted both the code and this PR's own stated reason for keeping the variable at all.

On the rebase. This branch is stacked on #203, which is still taking review rounds — the last one changed session rotation, the legacy exchange, and CSRF lifetimes. I rebased once, hit conflicts across the auth composition and both migrations claiming 0023, and backed out rather than resolve them by hand into a moving base: the next round on #203 would invalidate the result anyway. The rebase lands once #203 settles, which it has to before this can merge in any case.

Validation on 73b8ed9: pnpm check passes; the commit is documentation only, and CI covers the rest.

[baixiaohang]

Recommendation: comment

• Rationale: The documentation blockers are resolved, but this head is intentionally still based on the superseded #203 commit and cannot be approved until that B-high base settles and the conflicted stack is rebased.

Risk level: B-high

• Path baseline: adds a database uniqueness migration under packages/server/** -> B-high
• Semantic lift: none; the path baseline already covers the core Account credential and identity-invariant changes

PR summary

• Author / repo: bestony / first-tree-ai/opentag
• Problem: after the Better Auth compatibility window closes, users and operators need one Account credential model and one durable explanation of how it works.
• Approach: remove the legacy JWT/OIDC and identity-resolution paths, enforce email uniqueness in PostgreSQL, and update both development guides to describe only the surviving Better Auth session, callback, Slack-state secret, and index boundary.
• Impacted modules: server auth composition and routes, Account schema/migrations, browser API behavior, shared HTTP paths, authentication tests, and bilingual development/deployment documentation.

Review findings

✅ 1. 73b8ed9 resolves both prior documentation findings: the setup guide now registers /api/v1/auth/callback/google, the browser credential is described as Better Auth's HttpOnly session, OPENTAG_JWT_SECRET is Slack-state-only, and users_email_unique is correctly named as the enforcement boundary in both language copies.

⚠️ 2. This head still forks from #203 at eb70853, while the target branch is now 4717aec; GitHub reports the PR as conflicted, including two different migrations claiming 0023. More importantly, current #203 remains changes-requested for bearer-vs-cookie transport selection and lost Better Auth rolling-session Set-Cookie headers. The final merged behavior is therefore not reviewable yet. Rebase only after #203 is approved and shipped, then request a full review of the resolved Stage 5 head before the compatibility-window gate is considered satisfied. [R4, R5 / packages/server/src/plugins/user-auth.ts, stacked PR #203]

✅ 3. On the code already unique to this stage, the duplicate-address migration still refuses ambiguity, and the cleanup retains the CSRF and sign-in-destination boundaries.

Action taken

• Submitted a comment review on 73b8ed926b1924fd2228c18b71403ac7511722fe; did not approve the conflicted, stale-base head.

[bestony]

Rebased onto main now that #203 has shipped — 7dbc0c0, base is main, conflicts gone. Requesting the full review you asked for.

#203 was squash-merged and this branch was forked from one of its intermediate commits, so replaying would have meant hand-resolving conflicts across the auth composition and two migrations both claiming 0023. I rebuilt the stage from the merged base instead — stage 5 is overwhelmingly deletion, so redoing it deterministically was safer than merging conflict markers through authentication code.

What that means for review: every fix #203 gained after this branch forked is present and unreverted — transport selection, the renewal-header bridge, conditional rotation, the configured session lifetime, the CSRF renewal. What is removed is only what the stage is meant to remove. I checked the docs diff against main specifically for accidental reverts of #203's wording; the only removals are the legacy TTL rows and the bridge paragraph.

Two things this head does that the previous one did not:

• account_legacy_upgrades is dropped in the same migration. It arrived in feat(server): issue CLI and development credentials as revocable Better Auth sessions #203 to converge a replayed exchange of a credential this stage stops accepting, so it has nothing left to record. Migration is 0024, since feat(server): issue CLI and development credentials as revocable Better Auth sessions #203 took 0023.
• The expand-only regression now proves both halves rather than one: the previous revision's unnormalized insert still succeeds through 0023, and 0024 then rejects it with 23505 — with the guard refusing outright while a duplicate exists rather than choosing a row to discard.

Still gated. Merging this needs the compatibility window actually closed: the production CLI shipped on a vX.Y.Z tag and adopted, since until then production CLIs depend on the bridge this removes. It is the one step no rollback recovers.

Validation on 7dbc0c0: pnpm check, pnpm build, pnpm typecheck, pnpm test (1262), and pnpm --filter @opentag/server test:integration (202) all pass.

[baixiaohang]

Recommendation: request changes

• Rationale: The rebuilt Stage 5 preserves the final Better Auth transport and session-cookie fixes, but its application composition disconnects the CSRF lifetime from the rolling browser session.

Risk level: B-high

• Path baseline: adds a database uniqueness migration under packages/server/** -> B-high
• Semantic lift: none; the path baseline already covers the Account credential and identity-invariant changes

PR summary

• Author / repo: bestony / first-tree-ai/opentag
• Problem: once the compatibility window closes, users and operators need one revocable Account credential model, without legacy JWT/OIDC paths or a second identity-resolution implementation.
• Approach: rebuild Stage 5 on merged main, remove the legacy credential bridge, browser refresh and Google callback, replace resolver serialization with a case-insensitive unique index, and drop the now-unused legacy-upgrade ledger.
• Impacted modules: server authentication composition and routes, Account schema/migrations, browser API behavior, shared HTTP paths, authentication tests, and bilingual setup/deployment documentation.

Review findings

❌ 1. createApp no longer passes browserAuth.sessionTtlSeconds or browserAuth.secureCookies into the shared authOptions used by every authenticated route. Consequently renewBrowserCsrfCookie always exits at sessionTtlSeconds === undefined in the real server composition. Better Auth can keep rolling the session row and session cookie, while opentag_csrf still expires on its original schedule; an active browser then remains authenticated for reads but can no longer mutate or even sign out. Restore those browser options in the pre-handler composition and cover the createApp path, rather than only constructing the pre-handler directly in the integration test. [R5 / packages/server/src/app.ts:208, packages/server/src/plugins/user-auth.ts:116]

⚠️ 2. The author confirms the production CLI adoption window is still open. Resolving the code blocker must not be treated as satisfying that independent deployment gate: this migration drops the exchange ledger and the server stops accepting the credentials those deployed CLIs still hold.

✅ 3. The rebuild otherwise preserves the bearer-before-cookie transport choice and Better Auth renewal-header bridge, resolves the earlier bilingual documentation findings, and gives migration 0024 the intended fail-closed duplicate guard, index, and legacy-ledger removal.

Action taken

• Submitted request changes on 7dbc0c0a65a43e2074cc5dfa83026b52456503c2; CI is green, and no tests were run locally as part of this static review.

[bestony]

Correct, and it is a rebuild regression rather than a design one. Fixed in 7d6b50a.

Taking app.ts from the pre-rebase branch brought back a version predating the options #203 added, so the pre-handler was handed no session lifetime and renewBrowserCsrfCookie returned immediately. Better Auth would keep rolling the session while opentag_csrf expired on its original schedule — an active browser able to read but not to mutate or sign out, which reads as a broken account rather than an expired one.

Worth naming the process failure: when I rebuilt this stage I checked the documentation diff against main for exactly this class of accidental revert, and did not do the same for the composition. That is where it landed.

The regression goes through createApp rather than constructing the pre-handler directly, which is the distinction you drew and the one that matters here: the renewal logic was already correct and already covered; what broke was the wiring that decides whether it runs. Removing the options again fails it on the double-submit token was not renewed alongside the session. It also asserts the token is re-sent unchanged and carries the session's Max-Age, so a tab that already read it stays correct.

On the deployment gate — agreed, and I have not treated the code being ready as the window being closed. This still needs the production CLI shipped on a vX.Y.Z tag and adopted before it can merge: the migration drops the exchange ledger and the server stops accepting credentials deployed CLIs still hold. That is a separate decision from this review, and it is yours, not mine to infer from a green build.

Validation on 7d6b50a: pnpm check, pnpm build, pnpm typecheck, pnpm test (1263), and pnpm --filter @opentag/server test:integration (202) all pass.

[yuezengwu]

Recommendation: request changes

• Rationale: the Stage 5 cleanup is coherent on its own, but the Better Auth connect-code issuance inherited from #203 is not atomic with one-time-code consumption, the paired contract landed in the wrong Context Tree repository, and the touched developer contract states the opposite refresh failure ordering from the implementation.

Risk level: B-high

• Path baseline: adds and automatically applies a database uniqueness migration under packages/server/**.
• Semantic lift: removes every legacy Account credential and rollback bridge, drops its convergence ledger, and changes the durable one-Account-per-email enforcement boundary.

PR summary

• Author / repo: bestony / first-tree-ai/opentag
• Problem: after the compatibility window closes, the Server should issue and accept only revocable Better Auth sessions while preserving CSRF, sign-in-destination, and Account-identity invariants.
• Approach: delete the signed JWT pair, hand-rolled Google OIDC and identity resolver, remove browser refresh and legacy cookies/routes, drop account_legacy_upgrades, and create a case-insensitive unique email index.
• Impacted modules: Server authentication composition, connect/refresh credential behavior, browser API and CSRF/session renewal, Account schema/migrations, Google sign-in, shared paths, tests, and bilingual operator/developer documentation.

Review findings

❌ 1. Account connect-code exchange creates the Better Auth session outside the transaction that consumes the one-time code. exchangeConnectCode holds the code row lock and calls issuePairForUser before the consumed_at update, but BetterAuthSessionTokens uses Better Auth's base adapter/database rather than that transaction. A focused PostgreSQL fault-injection probe made the consume update fail after session creation: the request failed, one live auth_sessions row survived, and the code remained unconsumed and reusable. The same split can exhaust the default pool under pool-sized same-code concurrency: waiters hold transaction connections on the row lock while the winner needs another base-pool connection to create the session. Make issuance and code consumption atomic or durably convergent, and cover both failure-after-issuance and pool-sized concurrent exchange. This defect arrived with #203, but Stage 5 must not make the credential retirement irreversible around it. [R4 / packages/server/src/services/auth/auth-service.ts:59-105, packages/server/src/auth/session-tokens.ts:25-36]

❌ 2. The paired Context Tree update did not reach the canonical Tree named by this workspace. first-tree-context#984 merged into first-tree-ai/first-tree-context, but the clean, verified canonical checkout is the separate first-tree-ai/opentag-context-tree repository at 73b6f0b8; its current runtime boundary still says CLI/daemon sessions use an access/refresh pair and access credentials are JWTs. Pair this source change with a source-backed update in the current canonical Tree; the old-repository merge cannot satisfy this contract gate. [R1, R5 / canonical current boundary, old paired PR #984]

❌ 3. Both touched development guides say refresh first issues the replacement and then withdraws the presented session. The implementation deliberately does the reverse: delete is the concurrency/revocation gate, and a failure before replacement issuance signs the client out. That failure direction is a material credential contract, not wording polish; align both language copies with the reviewed implementation. [R1, R5 / DEVELOPMENT.md:258-264, DEVELOPMENT.zh-CN.md:249-253, packages/server/src/auth/session-tokens.ts:39-56]

⚠️ 4. The external rollout gate remains open by the author's own current-head statement. Green code and this review do not prove that a production CLI was shipped on a stable tag and adopted, that every legacy credential holder drained, or that no old Server writer remains. Before any merge/deploy, require explicit adoption/window-close evidence plus the target-database duplicate-address preflight. Do not infer those facts from #203 being merged or from CI.

✅ 5. On this head, the Stage 5-specific migration refuses ambiguous duplicate emails, the index replaces the deleted resolver's concurrency invariant, the Better Auth allowlist remains callback-only, bearer-vs-cookie transport selection remains fail-closed, rolling Better Auth cookies and the double-submit token stay synchronized, and the legacy credential routes/configuration are removed consistently from code.

Validation on 7d6b50acc19b9e7c40777cd213a0ca3d0e1708ea

• git diff --check 5a58b239495a9646feadb9e16a814807c9957b78...HEAD
• context-tree verify on canonical first-tree-ai/opentag-context-tree commit 73b6f0b8 — no findings
• pnpm check, pnpm build, pnpm typecheck
• pnpm test — 66 Node script tests plus 1263 workspace Vitest tests; an initial concurrent run hit one unrelated 5-second client-runtime timeout, the exact test passed in isolation, and a clean sequential full rerun passed
• pnpm --filter @opentag/server test:integration — 202/202
• Focused fault-injection probe confirmed finding 1; the temporary probe was removed and the worktree is clean
• Live CI: all six reported checks successful

Action taken

• Requested changes on exact head 7d6b50acc19b9e7c40777cd213a0ca3d0e1708ea; did not merge, deploy, release, or close the compatibility window.

[baixiaohang]

Recommendation: request changes

• Rationale: New same-head evidence changes the earlier approval recommendation: connect-code session issuance crosses the one-time-code transaction, and the touched durable contract documents the opposite refresh failure ordering from the implementation.

Risk level: B-high

• Path baseline: adds a database uniqueness migration under packages/server/** -> B-high
• Semantic lift: none; the path baseline already covers the Account credential and identity-invariant changes

PR summary

• Author / repo: bestony / first-tree-ai/opentag
• Problem: after the compatibility window closes, browser and CLI users should have one revocable Account credential model without legacy credential or identity-resolution paths.
• Approach: make Better Auth sessions the only credential, delete the JWT/OIDC/refresh bridge, enforce one Account per case-insensitive email in PostgreSQL, and drop the legacy exchange ledger.
• Impacted modules: server authentication and credential composition, connect-code and refresh behavior, Account schema/migrations, browser API, shared paths, tests, and bilingual developer/deployment documentation.

Review findings

❌ 1. The one-time connect code and the session it issues are not one atomic or convergent operation. exchangeConnectCode opens a transaction and holds the code row lock, but calls issuePairForUser before updating consumed_at; BetterAuthSessionTokens then creates auth_sessions through Better Auth's base adapter rather than through that transaction. If the consume update or transaction commit fails after issuance, the request fails while an orphan live session survives and the code remains reusable. With the default ten-connection pool, ten same-code exchanges can also occupy the pool—one transaction at issuance and the other nine waiting on its row lock—while the winner waits for an eleventh connection to create the session. Make issuance and consumption atomic or durably convergent, and cover both failure-after-issuance and pool-sized same-code concurrency. The split arrived in #203, but this irreversible retirement stage must not contract around it. [R4, R5 / packages/server/src/services/auth/auth-service.ts:59, packages/server/src/auth/session-tokens.ts:28, packages/server/src/db/client.ts:7]

❌ 2. Both touched development guides say refresh issues the replacement before withdrawing the presented session. BetterAuthSessionTokens.rotate deliberately deletes first and only then issues: the delete is the concurrency/revocation gate, and a failure between the two signs the client out. That failure direction is part of the credential contract, so align the English and Chinese copies with the implementation. [R1, R5 / DEVELOPMENT.md:258, DEVELOPMENT.zh-CN.md:249, packages/server/src/auth/session-tokens.ts:39]

⚠️ 3. The external deployment gate remains independent: neither green code nor review establishes that the production CLI adoption window has closed or that the target database passed the duplicate-address preflight.

✅ 4. The Stage 5-specific CSRF wiring regression is fixed on this head, and the migration, bearer/cookie transport selection, rolling session-cookie bridge, callback-only Better Auth surface, and legacy-path deletion remain coherent.

Action taken

• Submitted request changes on 7d6b50acc19b9e7c40777cd213a0ca3d0e1708ea; relied on source inspection and the reported fault-injection evidence, and did not run checks or tests locally.

[bestony]

Both reviews found the same connect-code defect independently, and both are right. Fixed in d178179, plus the canonical Tree.

1 — issuance was never atomic with consumption. The exchange held the code's row lock inside a transaction, but the credential store has its own connection, so the two were separate operations wearing one transaction. A consume or commit failing after issuance left a live session behind with the code still unconsumed — one code buying a second credential, and the first invisible to whoever asked for it. And holding that lock across a call needing its own connection is the pool-exhaustion shape from the earlier round, in a place I did not go back and check after fixing it in the upgrade path.

The code is consumed and committed first now; the session is issued afterwards, outside the transaction. The consume stays conditional, so several callers reaching one code redeem it exactly once.

That inverts the failure: a spent code with no session. For a one-time credential that is the direction to fail in — the caller asks for another code rather than holding one that has already been redeemed. The previous test asserted the opposite, so it is replaced rather than adjusted; it was encoding the ordering, not an incidental detail.

Both regressions were run against the code they replace. The failure case leaves consumed_at null under the old ordering. The concurrency case does not merely fail there — it times out, which is the pool exhaustion you predicted, reproduced rather than reasoned about.

2 — the guides had the refresh ordering backwards. Correct, and it matters for the reason you give: withdrawal-first is what makes the race safe and what decides that a mid-failure signs the client out. I wrote that prose before reversing the ordering in #203's round 4 and never came back to it. Both language copies now state the withdrawal first, why the ordering is what it is, and the failure direction it produces.

3 — the Tree update went to the wrong repository. opentag-context-tree#2 now updates master at 73b6f0b8.

I followed the repository the earlier review comments linked to (first-tree-context), which is where #984 merged. That was the wrong target, and checking which Tree was canonical rather than which link was in front of me would have caught it.

One thing worth surfacing rather than quietly working around: both repositories carry that file, with different wording for the same bullet — this one scopes authority to Team and role, the other to Computer enrollment, Agent, and IM binding. Whichever is canonical, the other is drifting. I have only updated the credential bullet in the canonical one and left the divergence alone; someone with the history should decide whether the old repository gets retired or resynchronized.

4 — the rollout gate. Agreed, and unchanged: I am not treating green code, this review, or #203 being merged as evidence the window has closed. Merging still needs the production CLI shipped on a stable tag and adopted, plus the duplicate-address preflight against the target database. That is your call to make on evidence, not mine to infer.

Validation on d178179: pnpm check, pnpm build, pnpm typecheck, pnpm test (1263), and pnpm --filter @opentag/server test:integration (203) all pass.

[baixiaohang]

Recommendation: comment

• Rationale: The source-level atomicity and documentation blockers are resolved on this head, but the paired canonical Tree change is still open and unreviewed, and the external compatibility-window gate remains unsatisfied.

Risk level: B-high

• Path baseline: adds a database uniqueness migration under packages/server/** -> B-high
• Semantic lift: none; the path baseline already covers the Account credential and identity-invariant changes

PR summary

• Author / repo: bestony / first-tree-ai/opentag
• Problem: after the compatibility window closes, browser and CLI users should have one revocable Account credential model without legacy credential or identity-resolution paths.
• Approach: make Better Auth sessions the only credential, delete the JWT/OIDC/refresh bridge, enforce one Account per case-insensitive email in PostgreSQL, and drop the legacy exchange ledger.
• Impacted modules: server authentication and credential composition, connect-code and refresh behavior, Account schema/migrations, browser API, shared paths, tests, and bilingual developer/deployment documentation.

Review findings

✅ 1. d178179 resolves the connect-code split in the safe direction. Validation and the conditional consume stay inside one transaction; that transaction commits before Better Auth session issuance, so no row lock is held while the base adapter needs a connection. Issuance failure now leaves a spent code and no session rather than an orphan session plus a reusable code. The replacement regression asserts that failure direction, and the 12-way same-code case exercises concurrency beyond the default ten-connection pool. [R4 / packages/server/src/services/auth/auth-service.ts:59, packages/server/src/__tests__/integration/auth-migrations.test.ts:1122]

✅ 2. Both development guides now match rotate: withdrawal is the gate and happens before replacement issuance, including the deliberate sign-out-on-mid-failure behavior. [R1, R5 / DEVELOPMENT.md:258, DEVELOPMENT.zh-CN.md:249, packages/server/src/auth/session-tokens.ts:39]

⚠️ 3. The replacement paired Tree change, opentag-context-tree#2, is currently open with its validation check successful but no review or merge. Until the canonical Tree reviewer accepts and lands it, this source head still lacks its paired durable-contract update; the divergent old repository should be handled separately by its owners rather than folded into this credential PR.

⚠️ 4. The rollout gate remains independent: merge/deploy still requires evidence that the stable production CLI was shipped and adopted, the legacy-credential window closed, and the target database passed the duplicate-address preflight.

✅ 5. The earlier Stage 5 review passes remain intact: CSRF and rolling session cookies share one schedule, bearer/cookie transport selection stays fail-closed, Better Auth publishes only the callback, and migration 0024 refuses ambiguous addresses before replacing the deleted resolver invariant.

Action taken

• Submitted a comment review on d1781796990b12b9c352b5182ecd4306f68a13f1; all reported CI checks are successful, no approval was submitted, and no checks or tests were run locally.

[bestony]

Correction on the paired Tree update, and it goes the other way from the last round.

The OpenTag context tree is first-tree-ai/first-tree-context — confirmed by the repository owner. That is where I originally targeted it, and #984 merged there as 894819b2. So the paired durable-contract update was already done, and has been since before the last two rounds.

@yuezengwu — the finding that first-tree-ai/opentag-context-tree at 73b6f0b8 is the canonical Tree does not hold. I acted on it and opened a PR there; it is now closed and its branch deleted, with that repository's credential bullet untouched.

For the record, since three repositories have now been named:

Repository	Has opentag/system/server-client-runtime-boundary.md	Status
first-tree-context	yes	the tree; #984 merged
opentag-context-tree	yes, with different wording	apparently a stale copy; PR closed, untouched
context-tree	no — it is the Context Tree tooling and spec repo (src/cli, docs/specification.md, policy/, skills/)	not applicable

The third came up as a candidate and I checked rather than assuming: the document returns 404 there, and the only path matching "opentag" is tests/fixtures/valid/opentag.md. Writing the bullet there would have meant inventing a content tree inside a tooling repository.

I should have verified which tree was canonical the first time instead of following the link in front of me — that is what sent me to the wrong repository on the second attempt. The divergence between the two copies that do carry the file is still worth someone's attention, but it is not this PR's to settle.

Net effect on this PR: the contract gate is satisfied by #984, already merged. Nothing on this head changes; d178179 stands, CI 6/6 green, and both blocking findings from the 01:15 and 01:18 reviews are addressed.

[baixiaohang]

Recommendation: request changes

• Rationale: The repository correction is valid, but the merged canonical Tree change records Stage 4's temporary compatibility bridge rather than Stage 5's post-window removal, so this head would immediately leave the durable credential contract stale.

Risk level: B-high

• Path baseline: adds and automatically applies a database migration under packages/server/** -> B-high
• Semantic lift: none; the path baseline already covers the Account credential and identity-invariant changes

PR summary

• Author / repo: bestony / first-tree-ai/opentag
• Problem: after the compatibility window closes, browser and CLI users should have one revocable Account credential model without legacy credentials, routes, or identity-resolution paths.
• Approach: make Better Auth sessions the only credential, remove the legacy JWT/OIDC and exchange bridge, enforce one Account per case-insensitive email in PostgreSQL, and drop the legacy-upgrade ledger.
• Impacted modules: server authentication and credential composition, Account schema/migrations, browser API, shared paths, tests, bilingual developer documentation, and the paired durable runtime contract.

Review findings

❌ 1. first-tree-ai/first-tree-context is indeed the canonical Tree declared by this workspace, so closing opentag-context-tree#2 was correct. However, merged first-tree-context#984 explicitly pairs with #203, not this PR, and its current normal node still says the compatibility bridge accepts credentials from the previous revision until the rollout window closes. Stage 5 deletes that bridge and its ledger after the window closes, so landing this head would make the canonical contract false immediately. Pair #205 with a source-backed update to that canonical node; no change to the stale copy is required. [R1, R5 / canonical runtime boundary at 894819b2]

⚠️ 2. The independent rollout gate remains open: even after the paired contract is corrected, merge/deploy still needs evidence that the stable production CLI was shipped and adopted, the legacy-credential window closed, and the target database passed the duplicate-address preflight.

✅ 3. The current source fixes remain sound: connect-code consumption commits before session issuance with failure and pool-sized concurrency regressions, refresh ordering is documented correctly, CSRF and rolling-session lifetimes stay wired together, and all six reported OpenTag CI checks are successful.

Action taken

• Requested changes on exact head d1781796990b12b9c352b5182ecd4306f68a13f1; no tests or checks were run locally.

[yuezengwu]

Recommendation: request changes

• Rationale: the source-level transaction and documentation defects are fixed on this head, but the paired update in the canonical Context Tree was closed without merging, so the durable contract still describes the retired JWT/access-refresh model.

Risk level: B-high

• Path baseline: adds and automatically applies a database uniqueness migration under packages/server/**.
• Semantic lift: removes every legacy Account credential and rollback bridge, drops its convergence ledger, and changes the durable one-Account-per-email enforcement boundary.

PR summary

• Author / repo: bestony / first-tree-ai/opentag
• Problem: after the compatibility window closes, the Server should issue and accept only revocable Better Auth sessions while preserving CSRF, sign-in-destination, and Account-identity invariants.
• Approach: delete the signed JWT pair, hand-rolled Google OIDC and identity resolver, remove browser refresh and legacy cookies/routes, drop account_legacy_upgrades, and create a case-insensitive unique email index.
• Impacted modules: Server authentication composition, connect/refresh credential behavior, browser API and CSRF/session renewal, Account schema/migrations, Google sign-in, shared paths, tests, and bilingual operator/developer documentation.

Review findings

❌ 1. The paired canonical Tree change has not landed. opentag-context-tree#2 is closed, unmerged, on exact head d8f6425f3185347a39c1f4b1b9bf9618f95cfc3b over master base 73b6f0b8b2a2ebc76e4944afc2e5555aff3cdf52. Verified canonical master remains at that base and still states that CLI/daemon sessions use access and refresh tokens and that access credentials are JWTs. The latest issue comment asserts a different repository binding, but the binding governing this lifecycle still names first-tree-ai/opentag-context-tree / master; an unlinked assertion cannot replace that authority or make a closed PR durable truth. Land the source-backed credential contract in the canonical Tree before this irreversible retirement stage is accepted. [R1, R5 / current canonical boundary, closed paired PR]

✅ 2. d178179 resolves the connect-code transaction split in the safe direction. Validation and the conditional consume remain in one transaction; the transaction commits before session issuance, so issuance failure leaves a spent code with no session and no row lock is held while Better Auth needs a base-pool connection. The focused PostgreSQL suite proves both the injected failure direction and a 12-way same-code exchange against the default ten-connection pool: exactly one caller and one session win, with no timeout. [R4 / packages/server/src/services/auth/auth-service.ts:59-122, packages/server/src/__tests__/integration/auth-migrations.test.ts:1122-1183]

✅ 3. Both development guides now match the implementation's withdrawal-first refresh contract, including the deliberate sign-out-on-mid-failure behavior. [R1, R5 / DEVELOPMENT.md:258-267, DEVELOPMENT.zh-CN.md:249-257, packages/server/src/auth/session-tokens.ts:39-56]

⚠️ 4. The external rollout gate remains open and independent. Neither green source nor this review proves that a stable production CLI was shipped and adopted, that the legacy-credential compatibility window drained, or that no old Server writer remains. Before merge/deploy, require explicit evidence for those facts and run the duplicate-address preflight against the target database. Do not infer the gate from #203 being merged, CI, or a local migration test.

✅ 5. The earlier Stage 5 checks remain coherent: migration 0024 refuses ambiguous case-insensitive addresses before replacing the deleted resolver invariant, Better Auth publishes only the callback, bearer/cookie transport selection fails closed, rolling session and CSRF cookies share one schedule, and legacy routes/configuration are removed consistently.

Validation on d1781796990b12b9c352b5182ecd4306f68a13f1

• git diff --check 5a58b239495a9646feadb9e16a814807c9957b78..HEAD
• pnpm check, pnpm typecheck, pnpm build
• pnpm test — 66 Node script tests plus 1263 workspace Vitest tests
• pnpm --filter @opentag/server test:integration — 203/203
• Focused auth-migrations.test.ts — 28/28, including issuance failure and 12-way same-code concurrency
• drizzle-kit check — migration snapshots consistent
• context-tree verify on canonical first-tree-ai/opentag-context-tree commit 73b6f0b8 — no findings
• Live CI: all six reported checks successful

Action taken

• Requested changes on exact head d1781796990b12b9c352b5182ecd4306f68a13f1; did not merge, deploy, release, close the compatibility window, or infer production adoption.

[bestony]

@baixiaohang's finding is right and I had missed it: #984 paired with #203 and says the bridge keeps accepting the previous revision's credentials until the window closes. This PR deletes that bridge, so landing it would make the canonical contract false on arrival. I documented the state #203 reached rather than the state the migration ends in.

Paired now: first-tree-context#985, to merge with #205 rather than before it — until this lands, the sentence it replaces is the accurate one. It also states the refresh ordering rather than only its outcome, since you both made the point that the withdrawal-first failure direction is part of the contract and not an implementation detail.

On which Tree is canonical

The two of you now disagree, so I am going to stop acting on whichever review is most recent — I have already moved this change between repositories twice on that basis, and a third move would be noise rather than progress.

Where it stands:

• The repository owner states it is first-tree-ai/first-tree-context. @baixiaohang's review agrees, and says closing opentag-context-tree#2 was correct.
• @yuezengwu's review holds that first-tree-ai/opentag-context-tree / master is the binding, and reads my comment as an unlinked assertion. That is a fair objection to how I stated it: I was relaying the owner's instruction, and a comment is not a binding.

I have no way to adjudicate that from inside this PR, and guessing has already cost two mis-targeted PRs. Could the two of you settle it, or point me at the artifact that declares the binding? I will land the paired contract wherever that lands, including reopening the opentag-context-tree change if it turns out to be the one — the branch is deleted but the content is reproducible.

For completeness on the third repository that came up: first-tree-ai/context-tree does not carry this document at all — it is the Context Tree tooling and spec (src/cli, docs/specification.md, policy/, skills/), and the path 404s there.

Unchanged

The rollout gate stays independent and open. Neither review nor green CI establishes that the stable production CLI shipped and was adopted, that the legacy-credential window drained, or that the target database passed the duplicate-address preflight. I am not treating any of those as satisfied.

Source head is unchanged at d178179: CI 6/6 green, and the connect-code, refresh-ordering, CSRF, transport-selection, and migration findings all stand resolved.

[baixiaohang]

Recommendation: approve

• Rationale: The source head now resolves the reviewed transaction, session/CSRF, migration, and documentation defects, and the final Stage 5 contract has a paired change in the canonical Tree declared by this workspace; the remaining rollout conditions are merge/deploy gates rather than source defects.

Risk level: B-high

• Path baseline: adds and automatically applies a database migration under packages/server/** -> B-high
• Semantic lift: none; the path baseline already covers the Account credential and identity-invariant changes

PR summary

• Author / repo: bestony / first-tree-ai/opentag
• Problem: after the compatibility window closes, browser and CLI users need one revocable Account credential model without legacy credentials, routes, or identity-resolution paths.
• Approach: make Better Auth sessions the only credential, remove the legacy JWT/OIDC and exchange bridge, enforce one Account per case-insensitive email in PostgreSQL, and drop the legacy-upgrade ledger.
• Impacted modules: server authentication and credential composition, Account schema/migrations, browser API, shared paths, tests, bilingual developer documentation, and the paired durable runtime contract.

Review findings

✅ 1. The current source head resolves the substantive blockers: connect-code consumption commits before session issuance with failure and pool-sized concurrency regressions; refresh withdrawal ordering is documented correctly; CSRF and rolling-session lifetimes stay synchronized; and migration 0024 refuses duplicate-address ambiguity before replacing the resolver invariant.

✅ 2. first-tree-context#985 now pairs the Stage 5 final state with this source head in the canonical Tree declared by this workspace. It must land with #205, not before it, because the current bridge sentence remains accurate until Stage 5 ships.

⚠️ 3. This approval does not settle the other reviewer's different Tree binding; that current CHANGES_REQUESTED review remains visible and must be reconciled before merge.

⚠️ 4. Merge/deploy remains independently gated on evidence that the stable production CLI was shipped and adopted, the compatibility window closed, and the target database passed the duplicate-address preflight. Source approval is not evidence for any of those facts.

✅ 5. All six reported OpenTag CI checks are successful; no checks or tests were run locally in this review round.

Action taken

• Approved exact head d1781796990b12b9c352b5182ecd4306f68a13f1; did not merge, deploy, release, or close the compatibility window.

[yuezengwu]

Superseded by the latest explicit repository binding and fresh exact-head review.

The canonical Context Tree for this review is first-tree-ai/opentag-context-tree / master; first-tree-context is historical only. I withdraw this comment's prior contrary binding conclusion and the source-only approval derived from it.

Current disposition on exact source head d1781796990b12b9c352b5182ecd4306f68a13f1: source defects remain resolved, but changes are requested because canonical opentag-context-tree@4f76b1474848c515cef9c1bc7ce42c5558395e89 still carries the access/refresh JWT contract, #2 is closed unmerged, and no Stage 5 canonical paired PR exists. A code-first transition needs that paired PR reviewable and publication-gated before source merge, then reconciled and published from the exact merged source state before deployment.

Stable production CLI adoption, compatibility-window drainage, absence of old writers, and the target-database duplicate-email preflight remain independent unsatisfied gates.

[bestony]

Ruling on the Tree binding: the canonical Context Tree for OpenTag is first-tree-ai/first-tree-context. Three independent lines of evidence, all verifiable from the repositories themselves rather than from local workspace state.

1. Every OpenTag contract change has landed in first-tree-context.

first-tree-context (main):

• 894819b2 2026-08-27 — docs(opentag): describe the CLI credential as the session it now is (#984)
• abab9b6d 2026-08-26 — docs(opentag): reflect retired Workspace APIs (#982)
• 0ed1247f 2026-08-26 — docs(opentag): align Account ownership model (#981)
• 63855cc9 2026-08-25 — docs(opentag): align product and runtime boundaries (#978)
• 255069c4 2026-08-25 — docs(opentag): allow explicit hosted-tool binding replacement (#977)

opentag-context-tree (master):

• 73b6f0b8 2026-08-27 — Record Biome tooling preference
• fd6a465b 2026-08-27 — docs: prefer oxlint and oxfmt tooling
• 709f1ee2 2026-08-27 — docs: remember Biome tooling preference
• d4bb6ba3 2026-08-25 — Document standalone Context Tree boundary
• b07ec972 2026-08-25 — Migrate First Tree and OpenTag context (chore: establish repository foundation #1)
• a2db664e 2026-08-25 — Initialize Context Tree

opentag-context-tree was created on 2026-08-25. Its only OpenTag content is the one-shot migration snapshot b07ec972; every commit since is a tooling-preference note. It has received zero OpenTag contract changes.

2. The copy cited as canonical is four contract changes stale.

Comparing opentag/system/server-client-runtime-boundary.md across the two:

• opentag-context-tree@73b6f0b8 L123: "CLI and daemon sessions use access and refresh tokens. Access JWTs prove …"
• first-tree-context@894819b2 L145: "CLI and daemon sessions hold one server-persisted session credential …" — followed by the compatibility-window sentence this PR retires.

The opentag-context-tree copy is additionally missing whole sections landed by #981 (Account ownership model), #982 (retired Workspace APIs), and the Protocol compatibility section covering internal Session collaboration.

@yuezengwu — the observation that 73b6f0b8's credential bullet "still says CLI/daemon sessions use access and refresh tokens" is evidence that this copy is stale, not that #205 is unpaired. The change that corrected that exact sentence is #984, and it merged into first-tree-context as 894819b2 on 2026-08-27, pairing with #203.

3. Ownership metadata. The first-tree-context copy carries owners: [liuchao-001, baixiaohang, yuezengwu, bestony] in its frontmatter. The opentag-context-tree copy has no such frontmatter.

On the binding artifact itself. The AGENTS.md cited as binding is not in this repository — this repo's AGENTS.md does not mention a Context Tree at all. It is a workspace-root file one level above the checkout, which makes it local environment state rather than a repository artifact. That is why the two reviews reached different bindings: they read different sources. I am not changing that file in this PR, but it should be reconciled separately so this does not have to be relitigated every round.

Consequences for this PR. first-tree-context#985 is the correct pairing and should land with #205 rather than before it; it will be taken out of draft so the two land together. opentag-context-tree#2 stays closed.

The independent rollout gates — stable CLI shipped and adopted, compatibility window closed, and the target-database duplicate-address preflight — remain open and are unaffected by this ruling.

[yuezengwu]

Recommendation: approve — source review only

• Rationale: all reviewed source defects are resolved on this exact head. The latest explicit user authority restores first-tree-ai/first-tree-context / main as the canonical Context Tree, so my prior request-changes finding based only on the superseded opentag-context-tree binding is withdrawn. The remaining conditions are coordinated merge/deploy gates, not source defects.

Risk level: B-high

• Path baseline: adds and automatically applies a database migration under packages/server/**.
• Semantic lift: retires every legacy Account credential and rollback bridge, drops its convergence ledger, and replaces the prior one-Account-per-email enforcement boundary.

Review findings

✅ 1. d178179 fixes the connect-code transaction split in the deliberate fail-closed direction. The code is consumed and committed before Better Auth session issuance; an issuance failure leaves a spent code with no session, and no row lock is held while the credential store needs a base-pool connection. PostgreSQL regressions cover both injected post-consume issuance failure and 12 concurrent exchanges against the default ten-connection pool, with exactly one caller/session winning. [R4 / packages/server/src/services/auth/auth-service.ts:59-122, packages/server/src/__tests__/integration/auth-migrations.test.ts:1122-1183]

✅ 2. The bilingual guides now match withdrawal-first refresh behavior, including the intentional sign-out-on-mid-failure contract. CSRF renewal remains wired to the rolling browser session, bearer-vs-cookie transport selection stays fail-closed, and Better Auth publishes only the Google callback. [R1, R5 / DEVELOPMENT.md:258-267, DEVELOPMENT.zh-CN.md:249-257, packages/server/src/auth/session-tokens.ts:39-56]

✅ 3. Migration 0024 refuses ambiguous case-insensitive duplicate addresses before creating the unique index and dropping account_legacy_upgrades; generated migration state is internally consistent. The index preserves the concurrency invariant previously supplied by the deleted identity resolver.

✅ 4. Canonical Tree binding is now explicitly first-tree-ai/first-tree-context / main. Stage 4 contract PR #984 is merged as 894819b22e73f7659a0ca089874cc47c13b61a29. Stage 5 PR #985 is correctly draft at exact head b687c1d1f82a12bdb3c55202a3a98c220e6a79fb: its current Context Review found no semantic defect in the final-state wording but deferred approval/publication until #205 lands, because the bridge statement on current main remains true before this source transition.

⚠️ 5. This source approval is not merge or deployment authority. A human must coordinate the Stage 5 transition with exact-merged-head reconciliation and publication of the paired Tree change. Merge/deploy also remains gated on explicit evidence that a stable production CLI was shipped and adopted, the compatibility window drained, no old Server writer remains, and the target database passes the duplicate-address preflight. None of those facts is inferred from approval, #203, CI, or local tests.

Validation on d1781796990b12b9c352b5182ecd4306f68a13f1

• git diff --check 5a58b239495a9646feadb9e16a814807c9957b78..HEAD
• pnpm check, pnpm typecheck, pnpm build
• pnpm test — 66 Node script tests plus 1263 workspace Vitest tests
• pnpm --filter @opentag/server test:integration — 203/203
• focused auth-migrations.test.ts — 28/28, including issuance failure and 12-way same-code concurrency
• drizzle-kit check — migration snapshots consistent
• live CI — all six reported checks successful

Context Tree read limitation

• The canonical clean checkout is first-tree-ai/first-tree-context@894819b2. The current context-tree 0.1.1 verifier reports pre-existing whole-tree schema/frontmatter/missing-node/broken-link findings, so per context-tree-read I stopped before semantic checkout reads and made no Tree repair. That limitation does not override the user's explicit repository binding; paired PR state and trusted Context Review evidence are reported separately above.

Action taken

• Approved exact source head d1781796990b12b9c352b5182ecd4306f68a13f1, superseding my prior same-head request based solely on the now-overridden Tree binding. I did not merge, deploy, release, publish the Tree, close the rollout window, or infer production adoption.

[yuezengwu]

Recommendation: request changes — canonical pairing only; source fixes pass

• Rationale: my previous same-head source-only approval relied on a repository binding that has now been explicitly superseded. A fresh strict read binds this review to first-tree-ai/opentag-context-tree / master. That canonical Tree has no reviewable Stage 5 contract paired with #205, so this PR must not remain presented as approved for merge.

Risk level: B-high

• Path baseline: this head adds and automatically applies a database migration under packages/server/**.
• Semantic lift: it retires the legacy credential bridge and convergence ledger and makes the transition irreversible.

Review findings

❌ 1. The canonical durable contract is not paired with this source transition. At exact canonical head 4f76b1474848c515cef9c1bc7ce42c5558395e89, context-tree verify succeeds with zero findings, but opentag/system/server-client-runtime-boundary.md still says CLI and daemon sessions use access and refresh tokens and that access credentials are JWTs. opentag-context-tree#2 is closed unmerged at d8f6425f3185347a39c1f4b1b9bf9618f95cfc3b; it describes the temporary Stage 4 compatibility bridge, not the final post-window state implemented here. There is no current canonical PR for Stage 5. The historical first-tree-context changes do not satisfy the explicit current binding.

Code-first sequencing does not require publishing a final-state Tree before the source makes that state true. It does require a canonical paired PR against live master, reviewed and publication-gated, so the exact merged source head can be reconciled and the Tree published immediately after or as part of the coordinated transition and before deployment. Until that pair exists, approval would incorrectly imply that the contract gate is satisfied. [R1, R5]

✅ 2. The source blockers are resolved on exact head d1781796990b12b9c352b5182ecd4306f68a13f1. Connect-code consumption commits before session issuance; issuance failure leaves a spent code with no session; the default-pool regression runs 12 concurrent exchanges with exactly one winner; the bilingual refresh-order docs match withdrawal-first behavior; CSRF/session lifetime coupling remains intact; and migration 0024 fails closed on case-insensitive duplicate addresses before creating the unique index and dropping the ledger.

⚠️ 3. Independent rollout gates remain open even after the canonical Tree pair is prepared: verified stable production CLI shipment and adoption, compatibility-window completion with no old writers, and target-database duplicate-email preflight. CI and source approval cannot establish those facts.

Validation on the exact source head

• git diff --check
• pnpm check, pnpm typecheck, pnpm build
• pnpm test — 66 script tests plus 1263 workspace Vitest tests
• server integration — 203/203
• focused auth migration suite — 28/28, including post-consume issuance failure and 12-way same-code concurrency
• drizzle-kit check
• live GitHub checks — all six successful
• canonical Tree 4f76b147... — strict context-tree-read, verifier zero findings

Action taken

• Requested changes on exact source head d1781796990b12b9c352b5182ecd4306f68a13f1, superseding my previous approval under the overridden binding. This is a contract-pairing blocker, not a newly discovered source-code defect. I did not merge, deploy, release, publish the Tree, close the compatibility window, or infer production adoption.