Skip to content

fix(code init): delegate tool login to /connect and /login — stop writing tool auth stores - #91

Merged
bergetbjork merged 2 commits into
mainfrom
fix/code-init-delegate-tool-auth
Oct 22, 2026
Merged

bergetbjork merged 2 commits into
mainfrom
fix/code-init-delegate-tool-auth

Conversation

@bergetbjork

Copy link
Copy Markdown
Contributor

Problem

berget code init copied the CLI's own OAuth tokens (or a freshly created API key) into the tool's private auth file (~/.local/share/opencode/auth.json, Pi's auth.json) and finished with "You're all set!".

OpenCode V2 moved provider credentials to a SQLite credential store and — verified against @opencode/[email protected] — never runs its legacy auth.json import on fresh installs. Result (real customer report): a new user ran init, picked their seat, and OpenCode was not connected. With BERGET_API_KEY set, requests silently fell back to the organization's API pot while the seat sat at 0%. Only /connect (which writes via V2's own credential API) connected the seat.

Writing another app's private data store was always fragile; V2 proved it. Detecting the OpenCode version and writing the V2 SQLite schema instead would be the same mistake again (drizzle schema, migration journal, WAL locking against a running server, per-version drift).

Change — delegate authentication to the tool, for both tools consistently

init configures; the tool authenticates. Same model for OpenCode and Pi:

Path Before After
Subscription (seat) Copied CLI tokens into tool auth file Note: "Run opencode, type /connect → Login with Berget" (Pi: /login → Use a subscription → Berget AI). Writes nothing.
API key Created key + wrote it into tool auth file Creates key, displays it once with paste/BERGET_API_KEY instructions. Writes nothing.
Reconfigure Overwrote the entry Removes the stale berget entry so the tool falls back to its own login flow instead of retrying a dead credential
Keep existing auth Untouched Untouched (still short-circuits as authenticated)

Side benefits:

  • No more shared refresh tokens between CLI and tools — each tool gets its own Keycloak session, so the latent refresh-rotation conflict (CLI rotates → tool's copy dies with invalid_grant) disappears without any sync logic.
  • Version-immune: we never touch OpenCode's/Pi's storage again; the next storage migration can't break us.
  • Net deletion: syncOAuthToTool/syncApiKeyToTool removed (−237/+179 lines incl. tests).

The final next-steps panel already contained the correct /connect///login instructions for the not-authenticated case, so those paths now surface automatically.

Tests

  • Updated auth-sync.test.ts / init.test.ts: every path asserts nothing is written to the tool auth store; new removeToolAuth suite (removal, preservation of other providers, missing/malformed file).
  • npm run test:run — 271 passed (24 files)
  • npm run typecheck, npm run lint, npm run format — clean

Follow-ups (separate PRs)

  1. console: add the /connect step to the "Invite your team" text (stops the onboarding bleed for new seat users)
  2. README/docs alignment in opencode-berget-auth + pi-provider

Refs: customer report (Dan Bergh Johnsson), root-cause research in opencode-berget-auth PR #45 / src/v2/migrate.ts (broken fresh-db V1 import in @opencode/[email protected]).

…ing tool auth stores

The CLI used to copy its own OAuth tokens (or a created API key) into the
tool's private auth file (~/.local/share/opencode/auth.json, Pi auth.json)
and report "You're all set!". OpenCode V2 moved to a credential database
and silently never imports that file on fresh installs, so new users ended
up unconnected — falling back to BERGET_API_KEY and draining the org API
pot while their seat sat at 0%.

Authentication is now always performed by the tool itself, which is the
only version-proof contract and gives each tool its own Keycloak session
(no shared refresh tokens):

- Subscription path: instruct /connect (OpenCode) or /login (Pi), write
  nothing
- API key path: create the key, display it once with paste/env
  instructions, write nothing
- Reconfigure: remove the stale berget entry so the tool falls back to
  its own login flow instead of retrying a dead credential
- Existing working auth is untouched ('keep' still short-circuits)

Removes syncOAuthToTool/syncApiKeyToTool; adds removeToolAuth.
Comment thread src/commands/code/auth-sync.ts Outdated
// Fall through to reconfigure
// Reconfigure: drop the stale entry so the tool falls back cleanly to its
// own login flow instead of retrying a potentially dead credential.
await removeToolAuth(files, homeDir, tool);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 warning — Reconfigure deletes the existing credential before the replacement flow completes — any later failure/cancellation leaves the user with no working auth; delete after success (or not at all, since /connect overwrites the entry).

Comment thread src/commands/code/auth-sync.ts Outdated
* tool is the only version-proof contract, and gives each tool its own
* Keycloak session (no shared refresh tokens).
*/
const SUBSCRIPTION_LOGIN_INSTRUCTIONS = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit — These login instructions duplicate the same /connect, /login and BERGET_API_KEY guidance in init.ts buildNextSteps; both notes are shown in one init run, so they can drift — share one source.

Comment thread src/commands/code/auth-sync.ts Outdated
type: 'oauth',
},
};
if (!('berget' in existing)) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit — 'berget' in existing throws TypeError on valid non-object JSON (e.g. "null"); guard with typeof existing !== 'object' || existing === null to honor the 'does nothing' contract.

Comment thread src/commands/code/auth-sync.ts Outdated
* private auth store from the CLI. OpenCode V2 moved its credential store and
* silently ignores the V1 auth.json we used to write; delegating login to the
* tool is the only version-proof contract, and gives each tool its own
* Keycloak session (no shared refresh tokens).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ good — Dropping the CLI-authored refresh token from the tool's auth store is a real security improvement — each tool now holds its own Keycloak session.

@berget-ai

berget-ai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Review

Summary

This PR reverses the berget code init auth strategy: the CLI no longer writes OAuth tokens or API keys into the tool's private auth store (opencode/pi auth.json) and instead prints instructions delegating login to the tool (/connect, /login); the only remaining store mutation is removeToolAuth, used to delete the legacy berget entry when the user explicitly reconfigures.

Risk

MEDIUM — the direction is sound and the security posture improves (no more shared refresh tokens), but a mid-flow failure on the reconfigure path now leaves the user with less working auth than before, and the detect/keep/cleanup logic still targets the very auth.json that the PR's own comment says OpenCode V2 no longer consults.

Issues

  • warning src/commands/code/auth-sync.ts:83 — "Reconfigure" deletes the existing credential before the replacement is established: if anything later fails or is cancelled (seat API down, createApiKey → FatalError, cliAuth === null after failed login, Ctrl+C at the method prompt), the user ends up worse off than before the run. Since the tool only reads this file when launched later, and a successful /connect login overwrites the berget entry anyway, the early delete serves no purpose for the success path — remove only on success (or not at all).
  • warning src/commands/code/auth-sync.ts:67 — per the PR's own comment (lines 44–45), OpenCode V2 "moved its credential store and silently ignores the V1 auth.json", yet isToolAuthenticated detects authentication only via a berget key in that exact V1 file; V2 users who logged in via /connect will never be detected, so berget code init re-runs the full connect flow every time and "Keep existing authentication" is dead code for the version this PR targets.
  • nit src/commands/code/auth-sync.ts:49 — SUBSCRIPTION_LOGIN_INSTRUCTIONS/API_KEY_LOGIN_INSTRUCTIONS re-encode the same /connect / /login / BERGET_API_KEY guidance that already exists verbatim in init.ts#buildNextSteps, and both notes are shown in the same init run, so the two sources of truth can drift (e.g. one mentioning the env var, the other not).
  • nit src/commands/code/auth-sync.ts:271 — 'berget' in existing throws an uncaught TypeError when the file contains valid non-object JSON (null, a number, a string); while unreachable via configureAuth (gated by isToolAuthenticated), the exported helper's contract ("does nothing" — tests cover 'not json' and missing files) breaks for that input. Fix: if (typeof existing !== 'object' || existing === null || !('berget' in existing)) return;
  • good src/commands/code/auth-sync.ts:47 — no longer copying the CLI's refresh token into the tool's auth store removes a shared long-lived credential from less-protected files; each tool gets its own Keycloak session.

Suggestions

  • src/commands/code/auth-sync.ts — move the removeToolAuth call after the new method succeeds (or drop it; /connect//login overwrites the berget entry), so a failed reconfigure never destroys working auth.
  • src/commands/code/auth-sync.ts — export the login-instruction constants (or a small nextStepsFor(tool) helper) and reuse them in init.ts#buildNextSteps so the CLI emits one canonical set of instructions.

Architecture

This inverts the trust boundary between CLI and tool: the tool's auth store becomes tool-owned (CLI only deletes legacy entries), making auth version-proof per the TOOL_NAMES/instruction constants; the authenticated result now means "no further CLI action needed" and init.ts's pre-existing buildNextSteps branches pick up the delegation messaging unchanged.

Code quality: 8/10 — clean, well-commented state-machine with dead paths removed and comprehensive test updates, held back by duplicated instruction strings and one over-narrow defensive guard.

Inline findings


Berget AI (berget/zai-org/GLM-5.3-Flash) | PR #91

… instructions

- Drop removeToolAuth: deleting the old berget entry before the
  replacement was established could leave a cancelled/failed reconfigure
  with LESS working auth. The tool's own login overwrites the entry on
  success, so the delete served no purpose on the success path either.
- Export SUBSCRIPTION/API_KEY_LOGIN_INSTRUCTIONS and reuse them in
  init.ts#buildNextSteps — one canonical source for /connect //login
  guidance, no drift between the inline notes and the final panel.
- Document that isToolAuthenticated is deliberately V1-file-only: reading
  OpenCode V2's SQLite credential store would repeat the private-store
  mistake this PR removes.

@berget-ai berget-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline findings


Berget AI (berget/zai-org/GLM-5.3-Flash) | PR #91

Comment thread src/commands/code/init.ts
return tool === 'opencode'
? 'Next steps:\n\n1. Run: opencode\n2. Type: /connect\n3. Choose your auth method:\n • "Login with Berget" — Berget Code plan\n • "Enter Berget API Key manually"\n • (or set BERGET_API_KEY env var)\n4. Select model: /models'
: 'Next steps:\n\n1. Restart Pi or run /reload\n2. Type: /login\n3. Choose your auth method:\n • "Use a subscription" → Berget AI\n • (or set BERGET_API_KEY env var)\n4. Select model: /model';
? `Next steps:\n\n1. ${SUBSCRIPTION_LOGIN_INSTRUCTIONS.opencode}\n ${API_KEY_LOGIN_INSTRUCTIONS.opencode}\n2. Select model: /models`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit — API_KEY_LOGIN_INSTRUCTIONS.* contains an embedded \n, so its second line ('or set the BERGET_API_KEY environment variable.') renders flush-left instead of indented in the next-steps note (same for pi at line 171).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant