From 31dc979572ac15813bedf5695ba5e028242b1268 Mon Sep 17 00:00:00 2001 From: bao-byterover Date: Mon, 4 May 2026 10:05:48 +0700 Subject: [PATCH] feat: [ENG-2544] improve byterover provider auth-gate error message (#595) * feat: [ENG-2544] improve byterover provider auth-gate error message Replace the dead-end "Run /login or brv login to sign in" with a multi-line message that names every onboarding path: interactive shell (brv login), and headless / SSH / CI (signup URL, api-key URL, brv login --api-key). Extract the wording to a module-level constant in provider-handler.ts so the connect and setActive paths cannot drift. * fix: [ENG-2544] address PR review comments - Single space after colon in 'Interactive shell:' for consistency with the other bullet line. - Extract STUB_BYTEROVER_AUTH_ERROR to test/helpers/provider-fixtures.ts to remove duplication between connect.test.ts and switch.test.ts. - Reword fixture comment to clarify it is a representative stub, not a mirror of the daemon's full message. - Add missing 'brv login --api-key' assertion to the SET_ACTIVE test in connect.test.ts for parity with the sibling CONNECT test. --- .../infra/transport/handlers/provider-handler.ts | 15 +++++++++++++-- test/commands/providers/connect.test.ts | 16 +++++++++------- test/commands/providers/switch.test.ts | 11 ++++++----- test/helpers/provider-fixtures.ts | 12 ++++++++++++ .../transport/handlers/provider-handler.test.ts | 6 ++++-- 5 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 test/helpers/provider-fixtures.ts diff --git a/src/server/infra/transport/handlers/provider-handler.ts b/src/server/infra/transport/handlers/provider-handler.ts index 38834f1ef..f1a2a59e5 100644 --- a/src/server/infra/transport/handlers/provider-handler.ts +++ b/src/server/infra/transport/handlers/provider-handler.ts @@ -52,6 +52,17 @@ import { ProviderCallbackTimeoutError, } from '../../provider-oauth/index.js' +const BYTEROVER_AUTH_REQUIRED_MESSAGE = [ + 'ByteRover Provider requires a ByteRover account.', + '', + ' • Interactive shell: brv login', + ' • Headless / SSH / CI: create an account at https://app.byterover.dev,', + ' generate an API key at https://app.byterover.dev/settings/keys, then:', + ' brv login --api-key ', + '', + 'Once signed in, retry: brv providers connect byterover', +].join('\n') + async function defaultValidateOpenAICompatibleEndpoint(params: { apiKey: string baseUrl: string @@ -257,7 +268,7 @@ export class ProviderHandler { const {apiKey, baseUrl, providerId} = data if (providerId === 'byterover' && !this.isByteRoverAuthSatisfied()) { - return {error: 'ByteRover Provider requires authentication. Run /login or brv login to sign in', success: false} + return {error: BYTEROVER_AUTH_REQUIRED_MESSAGE, success: false} } // Verify openai-compatible endpoint is reachable before persisting anything — @@ -388,7 +399,7 @@ export class ProviderHandler { ProviderEvents.SET_ACTIVE, async (data) => { if (data.providerId === 'byterover' && !this.isByteRoverAuthSatisfied()) { - return {error: 'ByteRover Provider requires authentication. Run /login or brv login to sign in', success: false} + return {error: BYTEROVER_AUTH_REQUIRED_MESSAGE, success: false} } await this.providerConfigStore.setActiveProvider(data.providerId) diff --git a/test/commands/providers/connect.test.ts b/test/commands/providers/connect.test.ts index 98d714aff..3893d3cbf 100644 --- a/test/commands/providers/connect.test.ts +++ b/test/commands/providers/connect.test.ts @@ -6,6 +6,7 @@ import {expect} from 'chai' import sinon, {restore, stub} from 'sinon' import ProviderConnect from '../../../src/oclif/commands/providers/connect.js' +import {STUB_BYTEROVER_AUTH_ERROR} from '../../helpers/provider-fixtures.js' // ==================== TestableProviderConnectCommand ==================== @@ -361,14 +362,14 @@ describe('Provider Connect Command', () => { providers: [{id: 'byterover', isConnected: false, name: 'ByteRover', requiresApiKey: false}], }) requestStub.onSecondCall().resolves({ - error: 'ByteRover Provider requires authentication. Run /login or brv login to sign in', + error: STUB_BYTEROVER_AUTH_ERROR, success: false, }) await createCommand('byterover').run() - expect(loggedMessages.some((m) => m.includes('authentication'))).to.be.true - expect(loggedMessages.some((m) => m.includes('login'))).to.be.true + expect(loggedMessages.some((m) => m.includes('ByteRover account'))).to.be.true + expect(loggedMessages.some((m) => m.includes('brv login --api-key'))).to.be.true }) it('should show auth error when server resolves SET_ACTIVE with success:false', async () => { @@ -377,13 +378,14 @@ describe('Provider Connect Command', () => { providers: [{id: 'byterover', isConnected: true, name: 'ByteRover', requiresApiKey: false}], }) requestStub.onSecondCall().resolves({ - error: 'ByteRover Provider requires authentication. Run /login or brv login to sign in', + error: STUB_BYTEROVER_AUTH_ERROR, success: false, }) await createCommand('byterover').run() - expect(loggedMessages.some((m) => m.includes('authentication'))).to.be.true + expect(loggedMessages.some((m) => m.includes('ByteRover account'))).to.be.true + expect(loggedMessages.some((m) => m.includes('brv login --api-key'))).to.be.true }) it('should show fallback error when CONNECT resolves with success:false and no error message', async () => { @@ -434,7 +436,7 @@ describe('Provider Connect Command', () => { providers: [{id: 'byterover', isConnected: false, name: 'ByteRover', requiresApiKey: false}], }) requestStub.onSecondCall().resolves({ - error: 'ByteRover Provider requires authentication. Run /login or brv login to sign in', + error: STUB_BYTEROVER_AUTH_ERROR, success: false, }) @@ -442,7 +444,7 @@ describe('Provider Connect Command', () => { const json = parseJsonOutput() expect(json.success).to.be.false - expect(json.data).to.have.property('error') + expect(json.data.error).to.equal(STUB_BYTEROVER_AUTH_ERROR) }) }) diff --git a/test/commands/providers/switch.test.ts b/test/commands/providers/switch.test.ts index 601cf8ae9..907f75168 100644 --- a/test/commands/providers/switch.test.ts +++ b/test/commands/providers/switch.test.ts @@ -6,6 +6,7 @@ import {expect} from 'chai' import sinon, {restore, stub} from 'sinon' import ProviderSwitch from '../../../src/oclif/commands/providers/switch.js' +import {STUB_BYTEROVER_AUTH_ERROR} from '../../helpers/provider-fixtures.js' // ==================== TestableProviderSwitchCommand ==================== @@ -150,14 +151,14 @@ describe('Provider Switch Command', () => { providers: [{id: 'byterover', isConnected: true, name: 'ByteRover'}], }) requestStub.onSecondCall().resolves({ - error: 'ByteRover Provider requires authentication. Run /login or brv login to sign in', + error: STUB_BYTEROVER_AUTH_ERROR, success: false, }) await createCommand('byterover').run() - expect(loggedMessages.some((m) => m.includes('authentication'))).to.be.true - expect(loggedMessages.some((m) => m.includes('login'))).to.be.true + expect(loggedMessages.some((m) => m.includes('ByteRover account'))).to.be.true + expect(loggedMessages.some((m) => m.includes('brv login --api-key'))).to.be.true }) }) @@ -196,7 +197,7 @@ describe('Provider Switch Command', () => { providers: [{id: 'byterover', isConnected: true, name: 'ByteRover'}], }) requestStub.onSecondCall().resolves({ - error: 'ByteRover Provider requires authentication. Run /login or brv login to sign in', + error: STUB_BYTEROVER_AUTH_ERROR, success: false, }) @@ -204,7 +205,7 @@ describe('Provider Switch Command', () => { const json = parseJsonOutput() expect(json.success).to.be.false - expect(json.data).to.have.property('error') + expect(json.data.error).to.equal(STUB_BYTEROVER_AUTH_ERROR) }) }) diff --git a/test/helpers/provider-fixtures.ts b/test/helpers/provider-fixtures.ts new file mode 100644 index 000000000..54a9e6118 --- /dev/null +++ b/test/helpers/provider-fixtures.ts @@ -0,0 +1,12 @@ +/** + * Test fixture providing a representative ByteRover auth-gate error for + * mocking daemon responses in CLI command tests. Keyword anchors + * ('ByteRover account', 'brv login --api-key') are validated by the + * consuming tests, so the real `BYTEROVER_AUTH_REQUIRED_MESSAGE` in + * `provider-handler.ts` must always contain those substrings. + */ +export const STUB_BYTEROVER_AUTH_ERROR = [ + 'ByteRover Provider requires a ByteRover account.', + ' • Interactive shell: brv login', + ' • Headless: brv login --api-key ', +].join('\n') diff --git a/test/unit/infra/transport/handlers/provider-handler.test.ts b/test/unit/infra/transport/handlers/provider-handler.test.ts index 360b80196..cac62be51 100644 --- a/test/unit/infra/transport/handlers/provider-handler.test.ts +++ b/test/unit/infra/transport/handlers/provider-handler.test.ts @@ -1037,7 +1037,8 @@ describe('ProviderHandler', () => { const result = await handler!({providerId: 'byterover'}, 'client-1') expect(result.success).to.be.false - expect(result.error).to.include('authentication') + expect(result.error).to.include('ByteRover account') + expect(result.error).to.include('brv login --api-key') }) it('should succeed when connecting byterover with valid auth', async () => { @@ -1069,7 +1070,8 @@ describe('ProviderHandler', () => { const result = await handler!({providerId: 'byterover'}, 'client-1') expect(result.success).to.be.false - expect(result.error).to.include('authentication') + expect(result.error).to.include('ByteRover account') + expect(result.error).to.include('brv login --api-key') }) it('should succeed when setting byterover active with valid auth', async () => {