diff --git a/apps/web/src/components/cloud-agent-next/hooks/useSidebarSessions.test.ts b/apps/web/src/components/cloud-agent-next/hooks/useSidebarSessions.test.ts index 3bd2c2ebf0c..acde36ce187 100644 --- a/apps/web/src/components/cloud-agent-next/hooks/useSidebarSessions.test.ts +++ b/apps/web/src/components/cloud-agent-next/hooks/useSidebarSessions.test.ts @@ -1,4 +1,5 @@ import type { MessageDeliveryState, UserWebSessionEventData } from '@kilocode/cloud-agent-sdk'; +import { TRPCClientError } from '@trpc/client'; import React, { createElement, type ComponentProps } from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { DropdownMenuItem } from '@/components/ui/dropdown-menu'; @@ -22,6 +23,10 @@ import { groupSidebarSessions, groupSidebarSessionsByDate, mergeWorktreeChatSessions, + nextPrLinkVerification, + PR_LINK_ATTEMPT_HISTORY_LIMIT, + recordPrLinkVerificationAttempt, + shouldRetryPrLinkVerification, patchSidebarWorktreeSessionStatus, removeSidebarDbSession, sessionCacheKey, @@ -32,6 +37,112 @@ import { type SidebarWorktreeDetails, } from './useSidebarSessions'; +describe('shouldRetryPrLinkVerification', () => { + it.each([ + 'INTERNAL_SERVER_ERROR', + 'BAD_GATEWAY', + 'SERVICE_UNAVAILABLE', + 'GATEWAY_TIMEOUT', + 'TIMEOUT', + ])('retries %s only once', code => { + const error = new TRPCClientError('Temporary failure', { + result: { error: { code: -32603, message: 'Temporary failure', data: { code } } }, + }); + expect(shouldRetryPrLinkVerification(0, error)).toBe(true); + expect(shouldRetryPrLinkVerification(1, error)).toBe(false); + expect(shouldRetryPrLinkVerification(2, error)).toBe(false); + }); + + it.each([ + 'BAD_REQUEST', + 'TOO_MANY_REQUESTS', + 'UNAUTHORIZED', + 'FORBIDDEN', + 'NOT_FOUND', + 'PRECONDITION_FAILED', + ])('does not retry %s', code => { + const error = new TRPCClientError('Permanent failure', { + result: { error: { code: -32000, message: 'Permanent failure', data: { code } } }, + cause: new TypeError('Failed to fetch'), + }); + expect(shouldRetryPrLinkVerification(0, error)).toBe(false); + }); + + it('retries a tRPC network failure only once', () => { + const error = TRPCClientError.from(new TypeError('Failed to fetch')); + expect(shouldRetryPrLinkVerification(0, error)).toBe(true); + expect(shouldRetryPrLinkVerification(1, error)).toBe(false); + }); + + it('does not retry unknown or untyped failures', () => { + expect(shouldRetryPrLinkVerification(0, new Error('Unknown failure'))).toBe(false); + expect(shouldRetryPrLinkVerification(0, new TRPCClientError('Unknown failure'))).toBe(false); + expect(shouldRetryPrLinkVerification(0, { data: { code: 'TIMEOUT' } })).toBe(false); + }); +}); + +describe('recordPrLinkVerificationAttempt', () => { + it('evicts the oldest attempt at the fixed history limit', () => { + const attempted = new Map(); + for (let index = 0; index <= PR_LINK_ATTEMPT_HISTORY_LIMIT; index++) { + recordPrLinkVerificationAttempt(attempted, `ses_${index}`, 'link'); + } + expect(attempted.size).toBe(PR_LINK_ATTEMPT_HISTORY_LIMIT); + expect(attempted.has('ses_0')).toBe(false); + expect(attempted.get('ses_1')).toBe('link'); + expect(attempted.get(`ses_${PR_LINK_ATTEMPT_HISTORY_LIMIT}`)).toBe('link'); + }); + + it('retains attempts across candidate filter changes', () => { + const attempted = new Map(); + const first = { session_id: 'ses_first', prLinkVerificationKey: 'link-1' }; + const second = { session_id: 'ses_second', prLinkVerificationKey: 'link-2' }; + recordPrLinkVerificationAttempt(attempted, first.session_id, first.prLinkVerificationKey); + expect(nextPrLinkVerification([second], attempted)).toBe(second); + recordPrLinkVerificationAttempt(attempted, second.session_id, second.prLinkVerificationKey); + expect(nextPrLinkVerification([first], attempted)).toBeUndefined(); + }); + + it('updates changed evidence without growing history and retains it as the newest attempt', () => { + const attempted = new Map(); + for (let index = 0; index < PR_LINK_ATTEMPT_HISTORY_LIMIT; index++) { + recordPrLinkVerificationAttempt(attempted, `ses_${index}`, 'old-link'); + } + recordPrLinkVerificationAttempt(attempted, 'ses_0', 'new-link'); + recordPrLinkVerificationAttempt(attempted, 'ses_new', 'link'); + expect(attempted.size).toBe(PR_LINK_ATTEMPT_HISTORY_LIMIT); + expect(attempted.get('ses_0')).toBe('new-link'); + expect(attempted.has('ses_1')).toBe(false); + }); +}); + +describe('nextPrLinkVerification', () => { + it('verifies stored links even when there is no visible PR badge', () => { + const session = { session_id: 'ses_unverified', prLinkVerificationKey: 'link-1' }; + expect(nextPrLinkVerification([session], new Map())).toBe(session); + }); + + it('skips sessions without a verification candidate', () => { + expect(nextPrLinkVerification([{ session_id: 'ses_no_link' }], new Map())).toBeUndefined(); + }); + + it('attempts each session link once and proceeds to the next candidate', () => { + const first = { session_id: 'ses_first', prLinkVerificationKey: 'link-1' }; + const second = { session_id: 'ses_second', prLinkVerificationKey: 'link-1' }; + const attempted = new Map([[first.session_id, first.prLinkVerificationKey]]); + expect(nextPrLinkVerification([first, second], attempted)).toBe(second); + attempted.set(second.session_id, second.prLinkVerificationKey); + expect(nextPrLinkVerification([first, second], attempted)).toBeUndefined(); + }); + + it('verifies again when the stored link or head evidence changes', () => { + const session = { session_id: 'ses_changed', prLinkVerificationKey: 'new-evidence' }; + expect(nextPrLinkVerification([session], new Map([[session.session_id, 'old-evidence']]))).toBe( + session + ); + }); +}); + Object.assign(globalThis, { React }); let mockRenderOpenMenus = false; diff --git a/apps/web/src/components/cloud-agent-next/hooks/useSidebarSessions.ts b/apps/web/src/components/cloud-agent-next/hooks/useSidebarSessions.ts index 118968bdecd..3a2173e4325 100644 --- a/apps/web/src/components/cloud-agent-next/hooks/useSidebarSessions.ts +++ b/apps/web/src/components/cloud-agent-next/hooks/useSidebarSessions.ts @@ -7,7 +7,8 @@ import { useCallback, useEffect, useMemo, useRef } from 'react'; import { useAtomValue, useSetAtom } from 'jotai'; -import { useQueries, useQuery, useQueryClient } from '@tanstack/react-query'; +import { useMutation, useQueries, useQuery, useQueryClient } from '@tanstack/react-query'; +import { TRPCClientError } from '@trpc/client'; import { useTRPC } from '@/lib/trpc/utils'; import type { inferRouterOutputs } from '@trpc/server'; import type { RootRouter } from '@/routers/root-router'; @@ -657,6 +658,45 @@ type UseSidebarSessionsReturn = { renameSessionLocally: (sessionId: string, newTitle: string) => void; }; +export const PR_LINK_ATTEMPT_HISTORY_LIMIT = 1000; + +export function recordPrLinkVerificationAttempt( + attempted: Map, + sessionId: string, + verificationKey: string +) { + attempted.delete(sessionId); + attempted.set(sessionId, verificationKey); + if (attempted.size > PR_LINK_ATTEMPT_HISTORY_LIMIT) { + const oldestSessionId = attempted.keys().next().value; + if (oldestSessionId !== undefined) attempted.delete(oldestSessionId); + } +} + +export function shouldRetryPrLinkVerification(failureCount: number, error: unknown): boolean { + if (failureCount >= 1 || !(error instanceof TRPCClientError)) return false; + const code: unknown = error.data?.code; + return ( + code === 'INTERNAL_SERVER_ERROR' || + code === 'BAD_GATEWAY' || + code === 'SERVICE_UNAVAILABLE' || + code === 'GATEWAY_TIMEOUT' || + code === 'TIMEOUT' || + (code === undefined && error.cause instanceof TypeError) + ); +} + +export function nextPrLinkVerification( + sessions: readonly { session_id: string; prLinkVerificationKey?: string }[], + attempted: ReadonlyMap +) { + return sessions.find( + session => + session.prLinkVerificationKey !== undefined && + attempted.get(session.session_id) !== session.prLinkVerificationKey + ); +} + export function useSidebarSessions(options?: UseSidebarSessionsOptions): UseSidebarSessionsReturn { const { organizationId, searchQuery = '', createdOnPlatform, gitUrl } = options ?? {}; const trpc = useTRPC(); @@ -730,6 +770,27 @@ export function useSidebarSessions(options?: UseSidebarSessionsOptions): UseSide enabled: isSearchActive, }); + const attemptedPrLinks = useRef(new Map()); + const { mutate: verifyPrLink, isPending: isVerifyingPrLink } = useMutation({ + ...trpc.cliSessionsV2.refreshAssociatedPullRequest.mutationOptions(), + retry: shouldRetryPrLinkVerification, + onSuccess: result => { + if (result.associatedPr !== null) reconcileSidebarQueries(); + }, + }); + const verificationSessions = isSearchActive ? searchData?.results : listData?.cliSessions; + useEffect(() => { + if (isVerifyingPrLink || !verificationSessions) return; + const session = nextPrLinkVerification(verificationSessions, attemptedPrLinks.current); + if (!session?.prLinkVerificationKey) return; + recordPrLinkVerificationAttempt( + attemptedPrLinks.current, + session.session_id, + session.prLinkVerificationKey + ); + verifyPrLink({ sessionId: session.session_id }); + }, [verificationSessions, isVerifyingPrLink, verifyPrLink]); + // Track last processed data key to avoid unnecessary atom updates const lastDataKeyRef = useRef(null); diff --git a/apps/web/src/routers/cli-sessions-v2-router.test.ts b/apps/web/src/routers/cli-sessions-v2-router.test.ts index 9e48738bfbf..d7192a2c02f 100644 --- a/apps/web/src/routers/cli-sessions-v2-router.test.ts +++ b/apps/web/src/routers/cli-sessions-v2-router.test.ts @@ -17,7 +17,12 @@ import type { KiloSdkStoredMessage } from '@kilocode/session-ingest-contracts'; import * as githubAdapter from '@/lib/integrations/platforms/github/adapter'; import { TRPCClientError } from '@trpc/client'; import { TRPCError } from '@trpc/server'; -import { parseGitHubOwnerRepo, parseGitHubPrUrl } from '@/routers/cli-sessions-v2-router'; +import { + getPrLinkVerificationKey, + parseGitHubOwnerRepo, + parseGitHubPrUrl, + projectAssociatedPr, +} from '@/routers/cli-sessions-v2-router'; import type { fetchSessionMessagesPage as FetchSessionMessagesPageType } from '@/lib/session-ingest-client'; import { notifyCliSessionRenamed } from '@/lib/cloud-agent/session-events'; import { captureException } from '@sentry/nextjs'; @@ -134,6 +139,119 @@ let otherUser: User; let adminUser: User; let testOrganization: Organization; +describe('projectAssociatedPr', () => { + const row = { + session_id: 'ses_projection', + updated_at: '2026-01-01T00:00:00.000Z', + git_url: 'https://github.com/kilo/repo', + pr_url: null, + pr_number: null, + pr_state: null, + pr_title: null, + pr_head_sha: null, + pr_last_synced_at: null, + pr_review_decision: null, + review_decision_pending: null, + session_pr_platform: 'github', + session_pr_url: 'https://github.com/kilo/repo/pull/42', + session_pr_number: 42, + session_pr_head_ref: 'feature', + session_pr_head_sha: null, + session_pr_verified_at: '2026-01-01T00:00:00.000Z', + }; + + it('omits the optional verification key for an already verified link', () => { + const session = projectAssociatedPr(row); + expect(Object.keys(session).sort()).toEqual( + ['session_id', 'updated_at', 'git_url', 'associatedPr'].sort() + ); + expect(session).not.toHaveProperty('prLinkVerificationKey'); + expect(session.associatedPr).toMatchObject({ number: 42, platform: 'github' }); + }); + + it('includes the verification key for an unverified candidate', () => { + const session = projectAssociatedPr({ ...row, session_pr_verified_at: null }); + expect(session.prLinkVerificationKey).toMatch(/^[a-f0-9]{64}$/); + expect(session.associatedPr).toBeNull(); + }); + + it('omits the optional verification key when there is no stored link', () => { + const session = projectAssociatedPr({ + ...row, + session_pr_url: null, + session_pr_number: null, + session_pr_verified_at: null, + }); + expect(session).not.toHaveProperty('prLinkVerificationKey'); + expect(session.associatedPr).toBeNull(); + }); +}); + +describe('getPrLinkVerificationKey', () => { + const candidate = { + platform: 'github', + pr_url: 'https://github.com/kilo/repo/pull/77', + pr_number: 77, + pr_link_verified_at: null, + git_url: 'https://github.com/kilo/repo', + pr_head_ref: 'feature/list-x', + pr_head_sha: 'cafef00d', + }; + + it.each([ + 'https://github.com/Kilo/Repo.git', + 'git@github.com:Kilo/Repo.git', + 'ssh://git@github.com/Kilo/Repo.git', + ])('detects an unverified GitHub link matching repository %s', git_url => { + expect(getPrLinkVerificationKey({ ...candidate, git_url })).toEqual(expect.any(String)); + }); + + it('accepts a PR URL subpath without requiring head evidence', () => { + expect( + getPrLinkVerificationKey({ + ...candidate, + pr_url: `${candidate.pr_url}/files?diff=split#top`, + pr_head_ref: null, + pr_head_sha: null, + }) + ).toEqual(expect.any(String)); + }); + + it.each([ + { platform: null }, + { platform: 'gitlab' }, + { pr_link_verified_at: '2026-01-01 00:00:00+00' }, + { pr_url: null }, + { pr_url: 'not a URL' }, + { pr_url: 'https://gitlab.com/kilo/repo/pull/77' }, + { pr_url: 'https://github.com/kilo/other/pull/77' }, + { pr_url: 'https://github.com/kilo/repo/pull/78' }, + { pr_number: null }, + { pr_number: 0 }, + { pr_number: -1 }, + { pr_number: 77.5 }, + { pr_number: 2_147_483_648 }, + { git_url: null }, + { git_url: 'not a repository' }, + { git_url: 'https://github.com/kilo/repo/nested' }, + ])('rejects non-candidate evidence %j', overrides => { + expect(getPrLinkVerificationKey({ ...candidate, ...overrides })).toBeUndefined(); + }); + + it.each([ + { pr_url: `${candidate.pr_url}/files` }, + { pr_url: 'https://github.com/kilo/repo/pull/78', pr_number: 78 }, + { git_url: 'git@github.com:kilo/repo.git' }, + { pr_head_ref: 'feature/new-head' }, + { pr_head_sha: 'new-sha' }, + ])('changes the opaque key when stored link or head evidence changes: %j', overrides => { + const key = getPrLinkVerificationKey(candidate); + expect(key).toMatch(/^[a-f0-9]{64}$/); + expect(getPrLinkVerificationKey({ ...candidate, ...overrides })).not.toBe(key); + expect(getPrLinkVerificationKey({ ...candidate })).toBe(key); + }); +}); + describe('cli-sessions-v2-router', () => { beforeEach(() => { afterCallbacks.length = 0; @@ -2370,6 +2488,29 @@ describe('cli-sessions-v2-router', () => { // An unverified stored link must not render a badge or an Open-on-GitHub // link, even when the branch cache row exists. expect(unverified?.associatedPr).toBeNull(); + expect(unverified?.prLinkVerificationKey).toEqual(expect.any(String)); + expect(withPr?.prLinkVerificationKey).toBeUndefined(); + expect(withoutPr?.prLinkVerificationKey).toBeUndefined(); + expect(branchOnly?.prLinkVerificationKey).toBeUndefined(); + expect(unverified).not.toHaveProperty('session_pr_url'); + expect(unverified).not.toHaveProperty('session_pr_head_ref'); + expect(unverified).not.toHaveProperty('session_pr_head_sha'); + }); + + it('search and get emit the same candidate key as list without displaying an unverified PR', async () => { + const caller = await createCallerForUser(regularUser.id); + const list = await caller.cliSessionsV2.list({}); + const search = await caller.cliSessionsV2.search({ + search_string: 'session unverified link', + }); + const session = await caller.cliSessionsV2.get({ session_id: sessionUnverified }); + const listRow = list.cliSessions.find(row => row.session_id === sessionUnverified); + const searchRow = search.results.find(row => row.session_id === sessionUnverified); + + expect(listRow?.prLinkVerificationKey).toEqual(expect.any(String)); + expect(searchRow?.prLinkVerificationKey).toBe(listRow?.prLinkVerificationKey); + expect(session.prLinkVerificationKey).toBe(listRow?.prLinkVerificationKey); + expect(searchRow?.associatedPr).toBeNull(); }); it('list returns live cache fields when the stored link has a trailing subpath', async () => { diff --git a/apps/web/src/routers/cli-sessions-v2-router.ts b/apps/web/src/routers/cli-sessions-v2-router.ts index 0e069b37d2e..e68ed34a9d4 100644 --- a/apps/web/src/routers/cli-sessions-v2-router.ts +++ b/apps/web/src/routers/cli-sessions-v2-router.ts @@ -1,4 +1,5 @@ import 'server-only'; +import { createHash } from 'node:crypto'; import { baseProcedure, createTRPCRouter } from '@/lib/trpc/init'; import * as z from 'zod'; import { db } from '@/lib/drizzle'; @@ -251,6 +252,8 @@ type SessionPrRow = { session_pr_platform: string | null; session_pr_url: string | null; session_pr_number: number | null; + session_pr_head_ref: string | null; + session_pr_head_sha: string | null; /** * `cli_sessions_v2.pr_link_verified_at`. Non-null means the link passed * GitHub identity verification, so it may be shown at all (and the cache's @@ -459,6 +462,51 @@ export function formatAssociatedPr( return pendingPartialFromSession(session, opts?.partialReviewDecisionPending ?? true); } +export function getPrLinkVerificationKey( + session: Pick< + CliSessionV2, + | 'platform' + | 'pr_url' + | 'pr_number' + | 'pr_link_verified_at' + | 'git_url' + | 'pr_head_ref' + | 'pr_head_sha' + > +): string | undefined { + if ( + session.platform !== 'github' || + session.pr_link_verified_at !== null || + session.pr_url === null || + session.pr_number === null || + !Number.isInteger(session.pr_number) || + session.pr_number <= 0 || + session.pr_number > PG_MAX_INTEGER || + !pullRequestUrlMatchesRepo(session.pr_url, session.git_url) + ) { + return undefined; + } + const parsed = parseGitHubPrUrl(session.pr_url); + if ( + !parsed || + parsed.number !== session.pr_number || + !parseGitHubOwnerRepo(session.git_url ?? '') + ) { + return undefined; + } + return createHash('sha256') + .update( + JSON.stringify([ + session.pr_url, + session.git_url, + session.pr_number, + session.pr_head_ref, + session.pr_head_sha, + ]) + ) + .digest('hex'); +} + const createdOnPlatformField = z.string().min(1).max(100); /** @@ -501,6 +549,8 @@ const commonSessionFieldsWithPr = { session_pr_platform: cli_sessions_v2.platform, session_pr_url: cli_sessions_v2.pr_url, session_pr_number: cli_sessions_v2.pr_number, + session_pr_head_ref: cli_sessions_v2.pr_head_ref, + session_pr_head_sha: cli_sessions_v2.pr_head_sha, session_pr_verified_at: cli_sessions_v2.pr_link_verified_at, total_cost_microdollars: cli_sessions_v2.total_cost_microdollars, } as const; @@ -542,12 +592,13 @@ export const sessionPrJoinPredicate = and( * Strip the flat `pr_*` columns produced by `commonSessionFieldsWithPr` and * fold them into a single `associatedPr` field on each row. */ -function projectAssociatedPr< +export function projectAssociatedPr< T extends AssociatedPrRow & SessionPrRow & { updated_at: string | null; git_url: string | null }, >( row: T ): Omit & { associatedPr: z.infer | null; + prLinkVerificationKey?: string; } { const { pr_url, @@ -561,11 +612,23 @@ function projectAssociatedPr< session_pr_platform, session_pr_url, session_pr_number, + session_pr_head_ref, + session_pr_head_sha, session_pr_verified_at, ...rest } = row; + const prLinkVerificationKey = getPrLinkVerificationKey({ + platform: session_pr_platform, + pr_url: session_pr_url, + pr_number: session_pr_number, + pr_link_verified_at: session_pr_verified_at, + git_url: rest.git_url, + pr_head_ref: session_pr_head_ref, + pr_head_sha: session_pr_head_sha, + }); return { ...rest, + ...(prLinkVerificationKey === undefined ? {} : { prLinkVerificationKey }), associatedPr: formatAssociatedPr( { platform: session_pr_platform, @@ -1406,7 +1469,7 @@ export const cliSessionsV2Router = createTRPCRouter({ await ensureOrganizationAccess(ctx, session.organization_id); } - return session; + return { ...session, prLinkVerificationKey: getPrLinkVerificationKey(session) }; }), /** @@ -1617,6 +1680,7 @@ export const cliSessionsV2Router = createTRPCRouter({ // Associated GitHub pull request for this session's branch, if any. // Populated by the pull_request webhook handler or a manual refresh. associatedPr: associatedPrSchema.nullable(), + prLinkVerificationKey: z.string().optional(), }) ) .query(async ({ ctx, input }) => { @@ -1717,6 +1781,7 @@ export const cliSessionsV2Router = createTRPCRouter({ version: session.version, total_cost_microdollars: session.total_cost_microdollars, runtimeState, + prLinkVerificationKey: getPrLinkVerificationKey(session), associatedPr: formatAssociatedPr( { platform: session.platform,