From 7a93429a69f0cf6275bf3ed5f35568c3c14cba24 Mon Sep 17 00:00:00 2001 From: Hugo Bjork Date: Thu, 22 Oct 2026 10:58:41 +0200 Subject: [PATCH 1/2] fix(code init): delegate tool login to /connect and /login, stop writing tool auth stores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/commands/code/__tests__/auth-sync.test.ts | 223 ++++++++---------- src/commands/code/__tests__/init.test.ts | 23 +- src/commands/code/auth-sync.ts | 170 ++++++------- 3 files changed, 179 insertions(+), 237 deletions(-) diff --git a/src/commands/code/__tests__/auth-sync.test.ts b/src/commands/code/__tests__/auth-sync.test.ts index 9a27278..139750b 100644 --- a/src/commands/code/__tests__/auth-sync.test.ts +++ b/src/commands/code/__tests__/auth-sync.test.ts @@ -8,8 +8,7 @@ import { ensureCliAuth, isToolAuthenticated, readCliAuth, - syncApiKeyToTool, - syncOAuthToTool, + removeToolAuth, } from '../auth-sync.js'; import { FatalError } from '../errors.js'; import { FakeApiKeyService } from './fake-api-key-service.js'; @@ -152,125 +151,75 @@ describe('decodeJwtPayload', () => { }); }); -describe('syncOAuthToTool', () => { - it('writes oauth tokens to opencode auth file', async () => { +describe('removeToolAuth', () => { + it('removes the berget entry from the opencode auth file', async () => { const files = new FakeFileStore(); - const auth = fakeCliAuth(); - - await syncOAuthToTool(files, HOME, 'opencode', auth); - - const written = files.getWrittenFiles(); - const content = written.get(HOME + '/.local/share/opencode/auth.json')!; - const parsed = JSON.parse(content); - // The expires field should now use the JWT's exp claim (converted to milliseconds) - const jwtPayload = JSON.parse( - Buffer.from(auth.access_token.split('.')[1], 'base64url').toString(), + files.seed( + HOME + '/.local/share/opencode/auth.json', + JSON.stringify({ berget: { access: 'tok', type: 'oauth' } }), ); - expect(parsed.berget).toEqual({ - access: auth.access_token, - expires: (jwtPayload.exp as number) * 1000, - refresh: auth.refresh_token, - type: 'oauth', - }); - }); - - it('writes oauth tokens to pi auth file', async () => { - const files = new FakeFileStore(); - const auth = fakeCliAuth(); - await syncOAuthToTool(files, HOME, 'pi', auth); + await removeToolAuth(files, HOME, 'opencode'); const written = files.getWrittenFiles(); - const content = written.get(HOME + '/.pi/agent/auth.json')!; - const parsed = JSON.parse(content); - expect(parsed.berget.type).toBe('oauth'); + const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); + expect(parsed.berget).toBeUndefined(); }); - it('merges with existing providers', async () => { + it('preserves other providers when removing the berget entry', async () => { const files = new FakeFileStore(); files.seed( HOME + '/.local/share/opencode/auth.json', - JSON.stringify({ openai: { key: 'sk-openai', type: 'api' } }), + JSON.stringify({ + berget: { access: 'tok', type: 'oauth' }, + openai: { key: 'sk-openai', type: 'api' }, + }), ); - const auth = fakeCliAuth(); - await syncOAuthToTool(files, HOME, 'opencode', auth); + await removeToolAuth(files, HOME, 'opencode'); const written = files.getWrittenFiles(); const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); + expect(parsed.berget).toBeUndefined(); expect(parsed.openai).toEqual({ key: 'sk-openai', type: 'api' }); - expect(parsed.berget.type).toBe('oauth'); }); - it('sets 0o600 permissions on the auth file', async () => { - const files = new FakeFileStore(); - const auth = fakeCliAuth(); - - await syncOAuthToTool(files, HOME, 'opencode', auth); - - const chmodCalls = files.getChmodCalls(); - expect(chmodCalls).toHaveLength(1); - expect(chmodCalls[0]).toEqual({ - mode: 0o600, - path: HOME + '/.local/share/opencode/auth.json', - }); - }); -}); - -describe('syncApiKeyToTool', () => { - it('writes api key to opencode auth file with type "api"', async () => { + it('removes the berget entry from the pi auth file', async () => { const files = new FakeFileStore(); + files.seed(HOME + '/.pi/agent/auth.json', JSON.stringify({ berget: { type: 'oauth' } })); - await syncApiKeyToTool(files, HOME, 'opencode', 'sk_ber_test'); + await removeToolAuth(files, HOME, 'pi'); const written = files.getWrittenFiles(); - const content = written.get(HOME + '/.local/share/opencode/auth.json')!; - const parsed = JSON.parse(content); - expect(parsed.berget).toEqual({ - key: 'sk_ber_test', - type: 'api', - }); + const parsed = JSON.parse(written.get(HOME + '/.pi/agent/auth.json')!); + expect(parsed.berget).toBeUndefined(); }); - it('writes api key to pi auth file with type "api_key"', async () => { + it('does nothing when the auth file does not exist', async () => { const files = new FakeFileStore(); - await syncApiKeyToTool(files, HOME, 'pi', 'sk_ber_pi'); + await removeToolAuth(files, HOME, 'opencode'); - const written = files.getWrittenFiles(); - const content = written.get(HOME + '/.pi/agent/auth.json')!; - const parsed = JSON.parse(content); - expect(parsed.berget).toEqual({ - key: 'sk_ber_pi', - type: 'api_key', - }); + expect(files.getWrittenFiles().size).toBe(0); }); - it('merges with existing providers', async () => { + it('does nothing when the auth file has no berget entry', async () => { const files = new FakeFileStore(); - files.seed( - HOME + '/.local/share/opencode/auth.json', - JSON.stringify({ anthropic: { key: 'sk-ant', type: 'api' } }), - ); + const original = JSON.stringify({ openai: { key: 'sk-openai', type: 'api' } }); + files.seed(HOME + '/.local/share/opencode/auth.json', original); - await syncApiKeyToTool(files, HOME, 'opencode', 'sk_ber_test'); + await removeToolAuth(files, HOME, 'opencode'); - const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); - expect(parsed.anthropic).toEqual({ key: 'sk-ant', type: 'api' }); + expect(files.getWrittenFiles().get(HOME + '/.local/share/opencode/auth.json')).toBe(original); }); - it('sets 0o600 permissions on the auth file', async () => { + it('does nothing when the auth file is malformed', async () => { const files = new FakeFileStore(); + files.seed(HOME + '/.local/share/opencode/auth.json', 'not json'); - await syncApiKeyToTool(files, HOME, 'opencode', 'sk_ber_test'); + await removeToolAuth(files, HOME, 'opencode'); - const chmodCalls = files.getChmodCalls(); - expect(chmodCalls).toHaveLength(1); - expect(chmodCalls[0]).toEqual({ - mode: 0o600, - path: HOME + '/.local/share/opencode/auth.json', - }); + expect(files.getWrittenFiles().get(HOME + '/.local/share/opencode/auth.json')).toBe('not json'); }); }); @@ -302,7 +251,7 @@ describe('configureAuth', () => { expect((deps.prompter as FakePrompter).calls).toHaveLength(1); // Only the select prompt }); - it('Case A reconfigure: already authenticated — reconfigure with valid CLI token', async () => { + it('Case A reconfigure: already authenticated — reconfigure removes stale entry and delegates login to the tool', async () => { const files = new FakeFileStore(); files.seed( HOME + '/.local/share/opencode/auth.json', @@ -327,10 +276,14 @@ describe('configureAuth', () => { }), ); - expect(result.authenticated).toBe(true); + expect(result.authenticated).toBe(false); + // Stale berget entry removed so the tool falls back to its own login flow + const written = files.getWrittenFiles(); + const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); + expect(parsed.berget).toBeUndefined(); }); - it('Case B: login success + berget_code_seat → chooses subscription', async () => { + it('Case B: login success + berget_code_seat → chooses subscription → instructs /connect, writes nothing', async () => { const files = new FakeFileStore(); const jwt = makeJwt({ exp: 9_999_999_999_999, @@ -351,14 +304,35 @@ describe('configureAuth', () => { }); const result = await configureAuth(deps, 'opencode', cliAuth); - expect(result.authenticated).toBe(true); - const written = files.getWrittenFiles(); - expect(written.has(HOME + '/.local/share/opencode/auth.json')).toBe(true); - const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); - expect(parsed.berget.type).toBe('oauth'); + expect(result.authenticated).toBe(false); + // The CLI must never write the tool's private auth store + expect(files.getWrittenFiles().has(HOME + '/.local/share/opencode/auth.json')).toBe(false); + const notes = prompter.calls.filter((c) => c.method === 'note'); + const instruction = notes.find((n) => JSON.stringify(n.args).includes('/connect')); + expect(instruction).toBeDefined(); + expect(JSON.stringify(instruction!.args)).toContain('Login with Berget'); }); - it('Case B variant: login success + seat → chooses api_key', async () => { + it('Case B (pi): subscription → instructs /login, writes nothing', async () => { + const files = new FakeFileStore(); + const prompter = new FakePrompter([select('subscription')]); + + const deps = makeAuthDeps({ + files, + prompter, + seatStatusService: new FakeSeatStatusService({ seatId: 1, tier: 'berget_code' }), + }); + const result = await configureAuth(deps, 'pi', fakeCliAuth()); + + expect(result.authenticated).toBe(false); + expect(files.getWrittenFiles().has(HOME + '/.pi/agent/auth.json')).toBe(false); + const notes = prompter.calls.filter((c) => c.method === 'note'); + const instruction = notes.find((n) => JSON.stringify(n.args).includes('/login')); + expect(instruction).toBeDefined(); + expect(JSON.stringify(instruction!.args)).toContain('Use a subscription'); + }); + + it('Case B variant: login success + seat → chooses api_key → displays key, writes nothing', async () => { const files = new FakeFileStore(); const jwt = makeJwt({ exp: 9_999_999_999_999, @@ -379,25 +353,26 @@ describe('configureAuth', () => { }); const result = await configureAuth(deps, 'opencode', cliAuth); - expect(result.authenticated).toBe(true); - const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); - expect(parsed.berget.type).toBe('api'); - expect(parsed.berget.key).toBe('sk_ber_test'); + expect(result.authenticated).toBe(false); + expect(files.getWrittenFiles().has(HOME + '/.local/share/opencode/auth.json')).toBe(false); + const notes = prompter.calls.filter((c) => c.method === 'note'); + const keyNote = notes.find((n) => JSON.stringify(n.args).includes('sk_ber_test')); + expect(keyNote).toBeDefined(); + expect(JSON.stringify(keyNote!.args)).toContain('/connect'); }); - it('Case C: login success + no seat → creates api key', async () => { + it('Case C: login success + no seat → creates api key → displays key, writes nothing', async () => { const files = new FakeFileStore(); const prompter = new FakePrompter([confirm(true)]); const deps = makeAuthDeps({ files, prompter }); const result = await configureAuth(deps, 'opencode', fakeCliAuth()); - expect(result.authenticated).toBe(true); - const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); - expect(parsed.berget.type).toBe('api'); - expect(parsed.berget.key).toBe('sk_ber_test'); + expect(result.authenticated).toBe(false); + expect(files.getWrittenFiles().has(HOME + '/.local/share/opencode/auth.json')).toBe(false); + const notes = prompter.calls.filter((c) => c.method === 'note'); + const keyNote = notes.find((n) => JSON.stringify(n.args).includes('sk_ber_test')); + expect(keyNote).toBeDefined(); }); it('Case B variant failure: login success + seat → chooses api_key + creation fails', async () => { @@ -471,14 +446,15 @@ describe('configureAuth', () => { }); const result = await configureAuth(deps, 'opencode', fakeCliAuth()); - expect(result.authenticated).toBe(true); - // Seat path: OAuth synced, NOT the no-seat API-key prompt - const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); - expect(parsed.berget.type).toBe('oauth'); + expect(result.authenticated).toBe(false); + // Seat path: /connect instructions, NOT the no-seat API-key prompt + const notes = prompter.calls.filter((c) => c.method === 'note'); + const instruction = notes.find((n) => JSON.stringify(n.args).includes('/connect')); + expect(instruction).toBeDefined(); + expect(files.getWrittenFiles().has(HOME + '/.local/share/opencode/auth.json')).toBe(false); }); - it('seat status unverifiable (API down) → warns and syncs OAuth anyway', async () => { + it('seat status unverifiable (API down) → warns and instructs, writes nothing', async () => { const files = new FakeFileStore(); const prompter = new FakePrompter([select('browser')]); @@ -489,14 +465,11 @@ describe('configureAuth', () => { }); const result = await configureAuth(deps, 'opencode', fakeCliAuth()); - expect(result.authenticated).toBe(true); - // OAuth synced despite unverified status - const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); - expect(parsed.berget.type).toBe('oauth'); - // Warning note displayed + expect(result.authenticated).toBe(false); + expect(files.getWrittenFiles().has(HOME + '/.local/share/opencode/auth.json')).toBe(false); const notes = prompter.calls.filter((c) => c.method === 'note'); - expect(notes.length).toBeGreaterThan(0); + const warningNote = notes.find((n) => JSON.stringify(n.args).includes('Could not verify')); + expect(warningNote).toBeDefined(); }); it('Case E: login fails → returns false when cliAuth is null', async () => { @@ -523,12 +496,12 @@ describe('configureAuth', () => { const result = await configureAuth(deps, 'opencode', cliAuth); - expect(result.authenticated).toBe(true); + expect(result.authenticated).toBe(false); // No seat-selection prompt should have been shown const selectCalls = prompter.calls.filter((c) => c.method === 'select'); expect(selectCalls).toHaveLength(0); - // OAuth was synced to the tool - expect(files.getWrittenFiles().has(HOME + '/.local/share/opencode/auth.json')).toBe(true); + // Nothing written to the tool's auth store + expect(files.getWrittenFiles().has(HOME + '/.local/share/opencode/auth.json')).toBe(false); // Warning note displayed const notes = prompter.calls.filter((c) => c.method === 'note'); const warningNote = notes.find((n) => (n.args as any).title === 'Authentication'); @@ -547,7 +520,7 @@ describe('configureAuth', () => { expect(written.size).toBe(0); // No files should be written }); - it('preserves existing providers during sync', async () => { + it('never writes the tool auth store, regardless of path', async () => { const files = new FakeFileStore(); files.seed( HOME + '/.local/share/opencode/auth.json', @@ -572,10 +545,10 @@ describe('configureAuth', () => { }), ); - const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); - expect(parsed.openai).toEqual({ key: 'sk-openai', type: 'api' }); - expect(parsed.berget).toBeDefined(); + // Other providers' entries are left untouched and nothing is added + expect(files.getWrittenFiles().get(HOME + '/.local/share/opencode/auth.json')).toBe( + JSON.stringify({ openai: { key: 'sk-openai', type: 'api' } }), + ); }); }); diff --git a/src/commands/code/__tests__/init.test.ts b/src/commands/code/__tests__/init.test.ts index 502ea39..54919ea 100644 --- a/src/commands/code/__tests__/init.test.ts +++ b/src/commands/code/__tests__/init.test.ts @@ -559,7 +559,7 @@ describe('runInit', () => { expect(JSON.stringify(lastNote)).toContain('/login'); }); - it('creates api key for pi when no seat', async () => { + it('creates api key for pi when no seat and displays it without writing the tool auth store', async () => { const files = new FakeFileStore(); const deps = makeDeps({ @@ -577,12 +577,14 @@ describe('runInit', () => { await runInit(deps); const written = files.getWrittenFiles(); - expect(written.has('/home/user/.pi/agent/auth.json')).toBe(true); - const parsed = JSON.parse(written.get('/home/user/.pi/agent/auth.json')!); - expect(parsed.berget.type).toBe('api_key'); + expect(written.has('/home/user/.pi/agent/auth.json')).toBe(false); + const prompter = deps.prompter as FakePrompter; + const notes = prompter.calls.filter((c) => c.method === 'note'); + const keyNote = notes.find((n) => JSON.stringify(n.args).includes('sk_ber_test')); + expect(keyNote).toBeDefined(); }); - it('uses subscription when berget_code_seat present', async () => { + it('subscription path delegates login to the tool via /connect instructions', async () => { const files = new FakeFileStore(); const farFuture = Math.floor(Date.now() / 1000) + 3600 * 24 * 365; // 1 year from now in seconds files.seed( @@ -609,8 +611,11 @@ describe('runInit', () => { await runInit(deps); const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get('/home/user/.local/share/opencode/auth.json')!); - expect(parsed.berget.type).toBe('oauth'); + expect(written.has('/home/user/.local/share/opencode/auth.json')).toBe(false); + const prompter = deps.prompter as FakePrompter; + const notes = prompter.calls.filter((c) => c.method === 'note'); + const instruction = notes.find((n) => JSON.stringify(n.args).includes('/connect')); + expect(instruction).toBeDefined(); }); }); @@ -744,7 +749,7 @@ describe('runInit', () => { expect(JSON.stringify(lastNote)).not.toContain("You're all set"); }); - it('shows auth-only next steps when toolConfigured=false but auth=true (pi)', async () => { + it('shows install + auth instructions when toolConfigured=false and subscription delegates login (pi)', async () => { const files = new FakeFileStore(); const farFuture = Math.floor(Date.now() / 1000) + 3600 * 24 * 365; files.seed( @@ -776,7 +781,7 @@ describe('runInit', () => { const noteText = JSON.stringify(lastNote); expect(noteText).toContain('Install Pi:'); expect(noteText).toContain('Run: pi'); - expect(noteText).toContain('Select model: /model'); + expect(noteText).toContain('Authenticate with Berget AI'); }); it('shows full setup instructions when toolConfigured=false and auth=false (opencode)', async () => { diff --git a/src/commands/code/auth-sync.ts b/src/commands/code/auth-sync.ts index f72e8fa..3ea69b2 100644 --- a/src/commands/code/auth-sync.ts +++ b/src/commands/code/auth-sync.ts @@ -33,10 +33,29 @@ const TOOL_AUTH_PATHS = { pi: getPiAuthPath, } as const; -const TOOL_API_KEY_TYPES: Record<'opencode' | 'pi', string> = { - opencode: 'api', - pi: 'api_key', -}; +const TOOL_NAMES = { + opencode: 'OpenCode', + pi: 'Pi', +} as const; + +/** + * Tool-specific login instructions. Authentication is ALWAYS performed by the + * tool itself (OpenCode /connect, Pi /login) — never by writing the tool's + * 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). + */ +const SUBSCRIPTION_LOGIN_INSTRUCTIONS = { + opencode: 'Run `opencode`, type `/connect` and choose "Login with Berget".', + pi: 'Run `pi`, type `/login` and choose "Use a subscription" → "Berget AI".', +} as const; + +const API_KEY_LOGIN_INSTRUCTIONS = { + opencode: + 'Run `opencode`, type `/connect` and choose "Enter Berget API Key manually",\nor set the BERGET_API_KEY environment variable.', + pi: 'Run `pi`, type `/login` and paste the key,\nor set the BERGET_API_KEY environment variable.', +} as const; export async function configureAuth( deps: Pick, @@ -49,7 +68,7 @@ export async function configureAuth( if (alreadyAuth) { const choice = await prompter.select<'keep' | 'reconfigure'>({ - message: `Account is already connected to Berget AI (${tool === 'opencode' ? 'OpenCode' : 'Pi'}). How do you want to proceed?`, + message: `Account is already connected to Berget AI (${TOOL_NAMES[tool]}). How do you want to proceed?`, options: [ { label: 'Keep existing authentication', value: 'keep' }, { label: 'Reconfigure — choose a different method', value: 'reconfigure' }, @@ -59,7 +78,9 @@ export async function configureAuth( if (choice === 'keep') { return { authenticated: true }; } - // 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); } else if (cliAuth !== null) { prompter.note('Authentication required to use Berget AI.', 'Connect your account'); } @@ -71,7 +92,7 @@ export async function configureAuth( const jwtPayload = decodeJwtPayload(cliAuth.access_token); if (!jwtPayload) { - return handleUnverifiedAuth(prompter, files, homeDir, tool, cliAuth); + return handleUnverifiedAuth(prompter, tool); } // Resolve the seat from the API's canonical source (berget.seat via @@ -80,16 +101,15 @@ export async function configureAuth( const seatStatus = await deps.seatStatusService.fetchSeatStatus(cliAuth.access_token); if (seatStatus === null) { - // Status could not be verified (network/5xx) — proceed with the OAuth - // sync and a warning, same as the undecodable-JWT path. - return handleUnverifiedAuth(prompter, files, homeDir, tool, cliAuth); + // Status could not be verified (network/5xx) — instruct, don't guess. + return handleUnverifiedAuth(prompter, tool); } if (seatStatus.seatId != null || seatStatus.tier != null) { - return handleHasSeat(prompter, apiKeyService, files, homeDir, tool, cliAuth, seatStatus.tier); + return handleHasSeat(prompter, apiKeyService, tool, seatStatus.tier); } - return handleNoSeat(prompter, apiKeyService, files, homeDir, tool); + return handleNoSeat(prompter, apiKeyService, tool); } export async function ensureCliAuth( @@ -227,88 +247,47 @@ export async function readCliAuth(files: FileStore, homeDir: string): Promise { const authPath = TOOL_AUTH_PATHS[tool](homeDir); - let existing: Record = {}; const content = await files.readFile(authPath); - if (content) { - try { - existing = JSON.parse(content) as Record; - } catch { - existing = {}; - } - } - - const updated = { - ...existing, - berget: { - key: apiKey, - type: TOOL_API_KEY_TYPES[tool], - }, - }; + if (!content) return; - await files.writeFile(authPath, JSON.stringify(updated, null, 2) + '\n'); - await files.chmod(authPath, 0o600); -} - -export async function syncOAuthToTool( - files: FileStore, - homeDir: string, - tool: 'opencode' | 'pi', - cliAuth: CliAuth, -): Promise { - const authPath = TOOL_AUTH_PATHS[tool](homeDir); - let existing: Record = {}; - - const content = await files.readFile(authPath); - if (content) { - try { - existing = JSON.parse(content) as Record; - } catch { - existing = {}; - } + let existing: Record; + try { + existing = JSON.parse(content) as Record; + } catch { + return; } - // Use the JWT's actual expiry time for consistency - const jwtExpiresAt = extractJwtExpiresAt(cliAuth.access_token); - - const updated = { - ...existing, - berget: { - access: cliAuth.access_token, - expires: jwtExpiresAt, - refresh: cliAuth.refresh_token, - type: 'oauth', - }, - }; + if (!('berget' in existing)) return; - await files.writeFile(authPath, JSON.stringify(updated, null, 2) + '\n'); + const { berget: _removed, ...remaining } = existing; + await files.writeFile(authPath, JSON.stringify(remaining, null, 2) + '\n'); await files.chmod(authPath, 0o600); } -async function createAndSyncApiKey( +async function createApiKey( prompter: Prompter, apiKeyService: ApiKeyServicePort, - files: FileStore, - homeDir: string, tool: 'opencode' | 'pi', ): Promise { const s = prompter.spinner(); s.start('Creating API key...'); + let key: string; try { - const { key } = await apiKeyService.create({ + ({ key } = await apiKeyService.create({ description: 'Created by berget code init', - name: `${tool === 'opencode' ? 'OpenCode' : 'Pi'} (created by berget CLI)`, - }); - await syncApiKeyToTool(files, homeDir, tool, key); - s.stop('API key created and saved.'); - return { authenticated: true }; + name: `${TOOL_NAMES[tool]} (created by berget CLI)`, + })); } catch (error: any) { s.stop('API key creation failed.'); throw new FatalError( @@ -316,6 +295,13 @@ async function createAndSyncApiKey( 'Could not create API key. Please create one manually with `berget api-keys create`.', ); } + s.stop('API key created.'); + + prompter.note( + `${key}\n\nThis key is shown once. To use it:\n${API_KEY_LOGIN_INSTRUCTIONS[tool]}`, + 'Your API key', + ); + return { authenticated: false }; } const TIER_LABELS: Record = { @@ -328,10 +314,7 @@ const TIER_LABELS: Record = { async function handleHasSeat( prompter: Prompter, apiKeyService: ApiKeyServicePort, - files: FileStore, - homeDir: string, tool: 'opencode' | 'pi', - cliAuth: CliAuth, tier: null | string, ): Promise { const tierLabel = (tier && TIER_LABELS[tier]) || 'Berget'; @@ -344,26 +327,19 @@ async function handleHasSeat( }); if (method === 'subscription') { - const s = prompter.spinner(); - s.start('Authenticating with Berget AI via subscription...'); - try { - await syncOAuthToTool(files, homeDir, tool, cliAuth); - s.stop('Authenticated.'); - } catch (error) { - s.stop('Authentication failed.'); - throw error; - } - return { authenticated: true }; + prompter.note( + `Your ${tierLabel} seat is ready. To connect ${TOOL_NAMES[tool]}:\n${SUBSCRIPTION_LOGIN_INSTRUCTIONS[tool]}`, + 'One last step', + ); + return { authenticated: false }; } - return createAndSyncApiKey(prompter, apiKeyService, files, homeDir, tool); + return createApiKey(prompter, apiKeyService, tool); } async function handleNoSeat( prompter: Prompter, apiKeyService: ApiKeyServicePort, - files: FileStore, - homeDir: string, tool: 'opencode' | 'pi', ): Promise { const shouldCreate = await prompter.confirm({ @@ -372,7 +348,7 @@ async function handleNoSeat( }); if (shouldCreate) { - return createAndSyncApiKey(prompter, apiKeyService, files, homeDir, tool); + return createApiKey(prompter, apiKeyService, tool); } prompter.note( @@ -384,23 +360,11 @@ async function handleNoSeat( async function handleUnverifiedAuth( prompter: Prompter, - files: FileStore, - homeDir: string, tool: 'opencode' | 'pi', - cliAuth: CliAuth, ): Promise { - const s = prompter.spinner(); - s.start('Authenticating with Berget AI...'); - try { - await syncOAuthToTool(files, homeDir, tool, cliAuth); - s.stop('Authenticated.'); - } catch (error) { - s.stop('Authentication failed.'); - throw error; - } prompter.note( - 'Warning: Could not verify your subscription status.\nIf you do not have a subscription, the tool may show an authorization error.', + `Could not verify your subscription status.\nIf you have a Berget subscription:\n${SUBSCRIPTION_LOGIN_INSTRUCTIONS[tool]}\n\nIf you do not have a subscription, create an API key with \`berget api-keys create\`.`, 'Authentication', ); - return { authenticated: true }; + return { authenticated: false }; } From 7c2a2b9c3154c7263c2c7f8b991d6a15caecab85 Mon Sep 17 00:00:00 2001 From: Hugo Bjork Date: Thu, 22 Oct 2026 11:12:25 +0200 Subject: [PATCH 2/2] =?UTF-8?q?refactor:=20address=20AI=20review=20?= =?UTF-8?q?=E2=80=94=20never=20delete=20existing=20auth,=20share=20login?= =?UTF-8?q?=20instructions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- src/commands/code/__tests__/auth-sync.test.ts | 89 ++----------------- src/commands/code/auth-sync.ts | 59 +++++------- src/commands/code/init.ts | 11 ++- 3 files changed, 35 insertions(+), 124 deletions(-) diff --git a/src/commands/code/__tests__/auth-sync.test.ts b/src/commands/code/__tests__/auth-sync.test.ts index 139750b..2739498 100644 --- a/src/commands/code/__tests__/auth-sync.test.ts +++ b/src/commands/code/__tests__/auth-sync.test.ts @@ -8,7 +8,6 @@ import { ensureCliAuth, isToolAuthenticated, readCliAuth, - removeToolAuth, } from '../auth-sync.js'; import { FatalError } from '../errors.js'; import { FakeApiKeyService } from './fake-api-key-service.js'; @@ -151,78 +150,6 @@ describe('decodeJwtPayload', () => { }); }); -describe('removeToolAuth', () => { - it('removes the berget entry from the opencode auth file', async () => { - const files = new FakeFileStore(); - files.seed( - HOME + '/.local/share/opencode/auth.json', - JSON.stringify({ berget: { access: 'tok', type: 'oauth' } }), - ); - - await removeToolAuth(files, HOME, 'opencode'); - - const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); - expect(parsed.berget).toBeUndefined(); - }); - - it('preserves other providers when removing the berget entry', async () => { - const files = new FakeFileStore(); - files.seed( - HOME + '/.local/share/opencode/auth.json', - JSON.stringify({ - berget: { access: 'tok', type: 'oauth' }, - openai: { key: 'sk-openai', type: 'api' }, - }), - ); - - await removeToolAuth(files, HOME, 'opencode'); - - const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); - expect(parsed.berget).toBeUndefined(); - expect(parsed.openai).toEqual({ key: 'sk-openai', type: 'api' }); - }); - - it('removes the berget entry from the pi auth file', async () => { - const files = new FakeFileStore(); - files.seed(HOME + '/.pi/agent/auth.json', JSON.stringify({ berget: { type: 'oauth' } })); - - await removeToolAuth(files, HOME, 'pi'); - - const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get(HOME + '/.pi/agent/auth.json')!); - expect(parsed.berget).toBeUndefined(); - }); - - it('does nothing when the auth file does not exist', async () => { - const files = new FakeFileStore(); - - await removeToolAuth(files, HOME, 'opencode'); - - expect(files.getWrittenFiles().size).toBe(0); - }); - - it('does nothing when the auth file has no berget entry', async () => { - const files = new FakeFileStore(); - const original = JSON.stringify({ openai: { key: 'sk-openai', type: 'api' } }); - files.seed(HOME + '/.local/share/opencode/auth.json', original); - - await removeToolAuth(files, HOME, 'opencode'); - - expect(files.getWrittenFiles().get(HOME + '/.local/share/opencode/auth.json')).toBe(original); - }); - - it('does nothing when the auth file is malformed', async () => { - const files = new FakeFileStore(); - files.seed(HOME + '/.local/share/opencode/auth.json', 'not json'); - - await removeToolAuth(files, HOME, 'opencode'); - - expect(files.getWrittenFiles().get(HOME + '/.local/share/opencode/auth.json')).toBe('not json'); - }); -}); - describe('configureAuth', () => { const makeAuthDeps = (overrides: Partial = {}): AuthDeps => ({ @@ -251,12 +178,10 @@ describe('configureAuth', () => { expect((deps.prompter as FakePrompter).calls).toHaveLength(1); // Only the select prompt }); - it('Case A reconfigure: already authenticated — reconfigure removes stale entry and delegates login to the tool', async () => { + it('Case A reconfigure: already authenticated — reconfigure delegates login to the tool and leaves the old entry untouched', async () => { const files = new FakeFileStore(); - files.seed( - HOME + '/.local/share/opencode/auth.json', - JSON.stringify({ berget: { type: 'oauth' } }), - ); + const original = JSON.stringify({ berget: { type: 'oauth' } }); + files.seed(HOME + '/.local/share/opencode/auth.json', original); const prompter = new FakePrompter([select('reconfigure'), select('subscription')]); @@ -277,10 +202,10 @@ describe('configureAuth', () => { ); expect(result.authenticated).toBe(false); - // Stale berget entry removed so the tool falls back to its own login flow - const written = files.getWrittenFiles(); - const parsed = JSON.parse(written.get(HOME + '/.local/share/opencode/auth.json')!); - expect(parsed.berget).toBeUndefined(); + // The old entry is never deleted mid-flow: the tool's own login + // (/connect) overwrites it on success, and a cancelled/failed run must + // not leave the user with less working auth than before. + expect(files.getWrittenFiles().get(HOME + '/.local/share/opencode/auth.json')).toBe(original); }); it('Case B: login success + berget_code_seat → chooses subscription → instructs /connect, writes nothing', async () => { diff --git a/src/commands/code/auth-sync.ts b/src/commands/code/auth-sync.ts index 3ea69b2..b0d7edc 100644 --- a/src/commands/code/auth-sync.ts +++ b/src/commands/code/auth-sync.ts @@ -39,19 +39,20 @@ const TOOL_NAMES = { } as const; /** - * Tool-specific login instructions. Authentication is ALWAYS performed by the - * tool itself (OpenCode /connect, Pi /login) — never by writing the tool's - * 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). + * Tool-specific login instructions — the single source of truth, also used by + * init.ts#buildNextSteps. Authentication is ALWAYS performed by the tool + * itself (OpenCode /connect, Pi /login) — never by writing the tool's 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). */ -const SUBSCRIPTION_LOGIN_INSTRUCTIONS = { +export const SUBSCRIPTION_LOGIN_INSTRUCTIONS = { opencode: 'Run `opencode`, type `/connect` and choose "Login with Berget".', pi: 'Run `pi`, type `/login` and choose "Use a subscription" → "Berget AI".', } as const; -const API_KEY_LOGIN_INSTRUCTIONS = { +export const API_KEY_LOGIN_INSTRUCTIONS = { opencode: 'Run `opencode`, type `/connect` and choose "Enter Berget API Key manually",\nor set the BERGET_API_KEY environment variable.', pi: 'Run `pi`, type `/login` and paste the key,\nor set the BERGET_API_KEY environment variable.', @@ -78,9 +79,10 @@ export async function configureAuth( if (choice === 'keep') { return { authenticated: true }; } - // 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); + // Reconfigure: fall through to method selection. The old entry is left in + // place deliberately — the tool's own login (/connect, /login) overwrites + // it on success, and deleting it now would leave the user with LESS + // working auth if this run is cancelled or fails mid-flow. } else if (cliAuth !== null) { prompter.note('Authentication required to use Berget AI.', 'Connect your account'); } @@ -203,6 +205,13 @@ export async function ensureCliAuth( }; } +/** + * Detects an existing berget entry in the tool's V1 auth file. Deliberately + * file-based only: OpenCode V2 stores credentials in a SQLite database we + * must NOT read (same private-store mistake as writing it). Consequence: on + * V2, users who connected via /connect are not detected here and init simply + * shows the login instructions again — harmless, and version-proof. + */ export async function isToolAuthenticated( files: FileStore, homeDir: string, @@ -247,34 +256,6 @@ export async function readCliAuth(files: FileStore, homeDir: string): Promise { - const authPath = TOOL_AUTH_PATHS[tool](homeDir); - - const content = await files.readFile(authPath); - if (!content) return; - - let existing: Record; - try { - existing = JSON.parse(content) as Record; - } catch { - return; - } - - if (!('berget' in existing)) return; - - const { berget: _removed, ...remaining } = existing; - await files.writeFile(authPath, JSON.stringify(remaining, null, 2) + '\n'); - await files.chmod(authPath, 0o600); -} - async function createApiKey( prompter: Prompter, apiKeyService: ApiKeyServicePort, diff --git a/src/commands/code/init.ts b/src/commands/code/init.ts index 9dc6417..4529774 100644 --- a/src/commands/code/init.ts +++ b/src/commands/code/init.ts @@ -12,7 +12,12 @@ import { ClackPrompter } from './adapters/clack-prompter.js'; import { FsFileStore } from './adapters/fs-file-store.js'; import { createSeatStatusService } from './adapters/seat-status.js'; import { SpawnCommandRunner } from './adapters/spawn-command-runner.js'; -import { configureAuth, ensureCliAuth } from './auth-sync.js'; +import { + API_KEY_LOGIN_INSTRUCTIONS, + configureAuth, + ensureCliAuth, + SUBSCRIPTION_LOGIN_INSTRUCTIONS, +} from './auth-sync.js'; import { CancelledError, CommandFailedError, FatalError, PrerequisiteError } from './errors.js'; import { getOpencodeLabel, @@ -162,8 +167,8 @@ function buildNextSteps( if (toolConfigured && !authenticated) { 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` + : `Next steps:\n\n1. Restart Pi or run /reload\n2. ${SUBSCRIPTION_LOGIN_INSTRUCTIONS.pi}\n ${API_KEY_LOGIN_INSTRUCTIONS.pi}\n3. Select model: /model`; } if (!toolConfigured && authenticated) {