Skip to content

Commit

Permalink
do not block on getting the site version or is-cody-enabled during au…
Browse files Browse the repository at this point in the history
…th (sourcegraph#5823)

The site version and Cody API version is now exposed in the
`siteVersion` global observable, which returns the site version and Cody
API version for the currently authenticated endpoint (and caches the
value).

If the Sourcegraph instance does not have Cody enabled, this means that
the error message will come later, not at sign-in. With one box, the
lines are getting blurrier anyway, and the logic for determining if Cody
was enabled was complex and often treated network errors as "Cody is
disabled". It is not worth it to invest time in making this more robust
because few users will encounter this, and those who do will just get an
error that Cody is disabled when they actually try to perform any
operation.

This means that initial auth takes 1-3 less HTTP requests.

## Test plan

CI & e2e
  • Loading branch information
sqs authored Oct 5, 2024
1 parent dba304f commit 356ba1a
Show file tree
Hide file tree
Showing 26 changed files with 249 additions and 390 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ data class AuthenticatedAuthStatus(
val isFireworksTracingEnabled: Boolean? = null,
val hasVerifiedEmail: Boolean? = null,
val requiresVerifiedEmail: Boolean? = null,
val siteVersion: String,
val codyApiVersion: Long,
val configOverwrites: CodyLLMSiteConfiguration? = null,
val primaryEmail: String? = null,
val displayName: String? = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package com.sourcegraph.cody.agent.protocol_generated;
data class ServerInfo(
val name: String,
val authenticated: Boolean? = null,
val codyVersion: String? = null,
val authStatus: AuthStatus? = null,
)

1 change: 0 additions & 1 deletion agent/src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,6 @@ export class Agent extends MessageHandler implements ExtensionClient {
return {
name: 'cody-agent',
authenticated: authStatus?.authenticated ?? false,
codyVersion: authStatus?.authenticated ? authStatus.siteVersion : undefined,
authStatus,
}
} catch (error) {
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"@types/isomorphic-fetch": "^0.0.39",
"@types/lodash": "^4.14.195",
"@types/node-fetch": "^2.6.4",
"@types/semver": "^7.5.0",
"@types/semver": "^7.5.8",
"@types/vscode": "^1.80.0",
"type-fest": "^4.26.1"
}
Expand Down
4 changes: 0 additions & 4 deletions lib/shared/src/auth/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ export interface AuthenticatedAuthStatus {

hasVerifiedEmail?: boolean
requiresVerifiedEmail?: boolean
siteVersion: string
codyApiVersion: number
configOverwrites?: CodyLLMSiteConfiguration

primaryEmail?: string
Expand Down Expand Up @@ -58,8 +56,6 @@ export const AUTH_STATUS_FIXTURE_AUTHED: AuthenticatedAuthStatus = {
endpoint: 'https://example.com',
authenticated: true,
username: 'alice',
codyApiVersion: 1,
siteVersion: '9999',
pendingValidation: false,
}

Expand Down
38 changes: 22 additions & 16 deletions lib/shared/src/chat/chat.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import type { AuthenticatedAuthStatus } from '../auth/types'
import { authStatus } from '../auth/authStatus'
import { firstValueFrom } from '../misc/observable'
import type { Message } from '../sourcegraph-api'
import type { SourcegraphCompletionsClient } from '../sourcegraph-api/completions/client'
import type {
CompletionGeneratorValue,
CompletionParameters,
} from '../sourcegraph-api/completions/types'
import { currentSiteVersion } from '../sourcegraph-api/siteVersion'

type ChatParameters = Omit<CompletionParameters, 'messages'>

Expand All @@ -15,28 +17,31 @@ const DEFAULT_CHAT_COMPLETION_PARAMETERS: Omit<ChatParameters, 'maxTokensToSampl
}

export class ChatClient {
constructor(
private completions: SourcegraphCompletionsClient,
private getAuthStatus: () => Pick<
AuthenticatedAuthStatus,
'authenticated' | 'endpoint' | 'codyApiVersion' | 'isFireworksTracingEnabled'
>
) {}

public chat(
constructor(private completions: SourcegraphCompletionsClient) {}

public async chat(
messages: Message[],
params: Partial<ChatParameters> & Pick<ChatParameters, 'maxTokensToSample'>,
abortSignal?: AbortSignal
): AsyncGenerator<CompletionGeneratorValue> {
const authStatus = this.getAuthStatus()

): Promise<AsyncGenerator<CompletionGeneratorValue>> {
// Replace internal models used for wrapper models with the actual model ID.
params.model = params.model?.replace(
'sourcegraph::2023-06-01::deep-cody',
'anthropic::2023-06-01::claude-3.5-sonnet'
)

const useApiV1 = authStatus.codyApiVersion >= 1 && params.model?.includes('claude-3')
const [versions, authStatus_] = await Promise.all([
currentSiteVersion(),
await firstValueFrom(authStatus),
])
if (!versions) {
throw new Error('unable to determine Cody API version')
}
if (!authStatus_.authenticated) {
throw new Error('not authenticated')
}

const useApiV1 = versions.codyAPIVersion >= 1 && params.model?.includes('claude-3')
const isLastMessageFromHuman = messages.length > 0 && messages.at(-1)!.speaker === 'human'

const isFireworks = params?.model?.startsWith('fireworks/')
Expand All @@ -62,13 +67,14 @@ export class ChatClient {

// Enabled Fireworks tracing for Sourcegraph teammates.
// https://readme.fireworks.ai/docs/enabling-tracing

const customHeaders: Record<string, string> =
isFireworks && authStatus.isFireworksTracingEnabled ? { 'X-Fireworks-Genie': 'true' } : {}
isFireworks && authStatus_.isFireworksTracingEnabled ? { 'X-Fireworks-Genie': 'true' } : {}

return this.completions.stream(
completionParams,
{
apiVersion: useApiV1 ? authStatus.codyApiVersion : 0,
apiVersion: useApiV1 ? versions.codyAPIVersion : 0,
customHeaders,
},
abortSignal
Expand Down
7 changes: 2 additions & 5 deletions lib/shared/src/cody-ignore/context-filters-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('ContextFiltersProvider', () => {
provider = new ContextFiltersProvider()
vi.useFakeTimers()

vi.spyOn(graphqlClient, 'isCodyEnabled').mockResolvedValue({ enabled: true, version: '6.0.0' })
vi.spyOn(graphqlClient, 'getSiteVersion').mockResolvedValue('6.0.0')
})

afterEach(() => {
Expand Down Expand Up @@ -298,10 +298,7 @@ describe('ContextFiltersProvider', () => {
}

it('should handle the case when version is older than the supported version', async () => {
vi.spyOn(graphqlClient, 'isCodyEnabled').mockResolvedValue({
enabled: true,
version: '5.3.2',
})
vi.spyOn(graphqlClient, 'getSiteVersion').mockResolvedValue('5.3.2')
await initProviderWithContextFilters({
include: [{ repoNamePattern: '^github\\.com/sourcegraph/cody' }],
exclude: [{ repoNamePattern: '^github\\.com/sourcegraph/sourcegraph' }],
Expand Down
1 change: 1 addition & 0 deletions lib/shared/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,3 +379,4 @@ export {
cachedUserProductSubscription,
userProductSubscription,
} from './sourcegraph-api/userProductSubscription'
export { siteVersion, currentSiteVersion } from './sourcegraph-api/siteVersion'
64 changes: 15 additions & 49 deletions lib/shared/src/sourcegraph-api/graphql/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1049,10 +1049,23 @@ export class SourcegraphGraphQLAPIClient {
}> {
// CONTEXT FILTERS are only available on Sourcegraph 5.3.3 and later.
const minimumVersion = '5.3.3'
const { enabled, version } = await this.isCodyEnabled()
const version = await this.getSiteVersion()
if (isError(version)) {
logError(
'SourcegraphGraphQLAPIClient',
'contextFilters getSiteVersion failed',
version.message
)

// Exclude everything in case of an unexpected error.
return {
filters: EXCLUDE_EVERYTHING_CONTEXT_FILTERS,
transient: true,
}
}
const insiderBuild = version.length > 12 || version.includes('dev')
const isValidVersion = insiderBuild || semver.gte(version, minimumVersion)
if (!enabled || !isValidVersion) {
if (!isValidVersion) {
return {
filters: INCLUDE_EVERYTHING_CONTEXT_FILTERS,
transient: false,
Expand Down Expand Up @@ -1119,53 +1132,6 @@ export class SourcegraphGraphQLAPIClient {
return result
}

/**
* Checks if Cody is enabled on the current Sourcegraph instance.
* @returns
* enabled: Whether Cody is enabled.
* version: The Sourcegraph version.
*
* This method first checks the Sourcegraph version using `getSiteVersion()`.
* If the version is before 5.0.0, Cody is disabled.
* If the version is 5.0.0 or newer, it checks for the existence of the `isCodyEnabled` field using `getSiteHasIsCodyEnabledField()`.
* If the field exists, it calls `getSiteHasCodyEnabled()` to check its value.
* If the field does not exist, Cody is assumed to be enabled for versions between 5.0.0 - 5.1.0.
*/
public async isCodyEnabled(signal?: AbortSignal): Promise<{
enabled: boolean
version: string
}> {
// Check site version.
const siteVersion = await this.getSiteVersion(signal)
if (isError(siteVersion)) {
return { enabled: false, version: 'unknown' }
}
signal?.throwIfAborted()
const insiderBuild = siteVersion.length > 12 || siteVersion.includes('dev')
if (insiderBuild) {
return { enabled: true, version: siteVersion }
}
// NOTE: Cody does not work on version later than 5.0
const versionBeforeCody = semver.lt(siteVersion, '5.0.0')
if (versionBeforeCody) {
return { enabled: false, version: siteVersion }
}
// Beta version is betwewen 5.0.0 - 5.1.0 and does not have isCodyEnabled field
const betaVersion = semver.gte(siteVersion, '5.0.0') && semver.lt(siteVersion, '5.1.0')
const hasIsCodyEnabledField = await this.getSiteHasIsCodyEnabledField(signal)
signal?.throwIfAborted()
// The isCodyEnabled field does not exist before version 5.1.0
if (!betaVersion && !isError(hasIsCodyEnabledField) && hasIsCodyEnabledField) {
const siteHasCodyEnabled = await this.getSiteHasCodyEnabled(signal)
signal?.throwIfAborted()
return {
enabled: !isError(siteHasCodyEnabled) && siteHasCodyEnabled,
version: siteVersion,
}
}
return { enabled: insiderBuild || betaVersion, version: siteVersion }
}

/**
* recordTelemetryEvents uses the new Telemetry API to record events that
* gets exported: https://sourcegraph.com/docs/dev/background-information/telemetry
Expand Down
25 changes: 25 additions & 0 deletions lib/shared/src/sourcegraph-api/siteVersion.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { describe, expect, test } from 'vitest'
import { inferCodyApiVersion } from './siteVersion'

describe('inferCodyApiVersion', () => {
test('returns API version 0 for a legacy instance', () => {
expect(inferCodyApiVersion('5.2.0', false)).toBe(0)
})

test('returns API version 1 for older versions', () => {
expect(inferCodyApiVersion('5.4.0', false)).toBe(1)
expect(inferCodyApiVersion('5.5.0', false)).toBe(1)
expect(inferCodyApiVersion('5.6.0', false)).toBe(1)
expect(inferCodyApiVersion('5.7.0', false)).toBe(1)
})

test('returns API version 2 for newer versions', () => {
expect(inferCodyApiVersion('5.8.0', false)).toBe(2)
expect(inferCodyApiVersion('5.9.0', false)).toBe(2)
expect(inferCodyApiVersion('5.10.1', false)).toBe(2)
})

test('returns API version 2 for dotcom', () => {
expect(inferCodyApiVersion('1.2.3', true)).toBe(2)
})
})
127 changes: 127 additions & 0 deletions lib/shared/src/sourcegraph-api/siteVersion.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import { Observable, map } from 'observable-fns'
import semver from 'semver'
import { authStatus } from '../auth/authStatus'
import { logError } from '../logger'
import {
distinctUntilChanged,
pick,
promiseFactoryToObservable,
storeLastValue,
} from '../misc/observable'
import {
firstResultFromOperation,
pendingOperation,
switchMapReplayOperation,
} from '../misc/observableOperation'
import { isError } from '../utils'
import { isDotCom } from './environments'
import { graphqlClient } from './graphql'

export interface SiteAndCodyAPIVersions {
siteVersion: string
codyAPIVersion: CodyApiVersion
}

/**
* Observe the site version and Cody API version of the currently authenticated endpoint.
*/
export const siteVersion: Observable<SiteAndCodyAPIVersions | null | typeof pendingOperation> =
authStatus.pipe(
pick('authenticated', 'endpoint', 'pendingValidation'),
distinctUntilChanged(),
switchMapReplayOperation(
(
authStatus
): Observable<SiteAndCodyAPIVersions | Error | null | typeof pendingOperation> => {
if (authStatus.pendingValidation) {
return Observable.of(pendingOperation)
}

if (!authStatus.authenticated) {
return Observable.of(null)
}

return promiseFactoryToObservable(signal => graphqlClient.getSiteVersion(signal)).pipe(
map((siteVersion): SiteAndCodyAPIVersions | null | typeof pendingOperation => {
if (isError(siteVersion)) {
logError(
'siteVersion',
`Failed to get site version from ${authStatus.endpoint}: ${siteVersion}`
)
return null
}
return {
siteVersion,
codyAPIVersion: inferCodyApiVersion(siteVersion, isDotCom(authStatus)),
}
})
)
}
),
map(result => (isError(result) ? null : result)) // the operation catches its own errors, so errors will never get here
)

const siteVersionStorage = storeLastValue(siteVersion)

/**
* Get the current site version. If authentication is pending, it awaits successful authentication.
*/
export function currentSiteVersion(): Promise<SiteAndCodyAPIVersions | null> {
return firstResultFromOperation(siteVersionStorage.observable)
}

type CodyApiVersion = 0 | 1 | 2

/** @internal Exported for testing only. */
export function inferCodyApiVersion(version: string, isDotCom: boolean): CodyApiVersion {
const parsedVersion = semver.valid(version)
const isLocalBuild = parsedVersion === '0.0.0'

if (isDotCom || isLocalBuild) {
// The most recent version is api-version=2, which was merged on 2024-09-11
// https://github.com/sourcegraph/sourcegraph/pull/470
return 2
}

// On Cloud deployments from main, the version identifier will use a format
// like "2024-09-11_5.7-4992e874aee2", which does not parse as SemVer. We
// make a best effort go parse the date from the version identifier
// allowing us to selectively enable new API versions on instances like SG02
// (that deploy frequently) without crashing on other Cloud deployments that
// release less frequently.
const isCloudBuildFromMain = parsedVersion === null
if (isCloudBuildFromMain) {
const date = parseDateFromPreReleaseVersion(version)
if (date && date >= new Date('2024-09-11')) {
return 2
}
// It's safe to bump this up to api-version=2 after the 5.8 release
return 1
}

// 5.8.0+ is the first version to support api-version=2.
if (semver.gte(parsedVersion, '5.8.0')) {
return 2
}

// 5.4.0+ is the first version to support api-version=1.
if (semver.gte(parsedVersion, '5.4.0')) {
return 1
}

return 0 // zero refers to the legacy, unversioned, Cody API
}

// Pre-release versions have a format like this "2024-09-11_5.7-4992e874aee2".
// This function return undefined for stable Enterprise releases like "5.7.0".
function parseDateFromPreReleaseVersion(version: string): Date | undefined {
try {
const dateString = version.split('_').at(1)
if (!dateString) {
return undefined
}
return new Date(dateString)
} catch {
return undefined
}
}
Loading

0 comments on commit 356ba1a

Please sign in to comment.