Skip to content

Commit 77626bb

Browse files
authored
Sessions: fixes for detect PR (#4629)
* Sessions: fixes for detect PR * Test and comment * Fix
1 parent 24f1408 commit 77626bb

File tree

2 files changed

+175
-5
lines changed

2 files changed

+175
-5
lines changed

src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,30 +1546,63 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
15461546
}
15471547
}
15481548

1549+
private static readonly _PR_DETECTION_RETRY_COUNT = 3;
1550+
private static readonly _PR_DETECTION_INITIAL_DELAY_MS = 2_000;
1551+
15491552
private async handlePullRequestCreated(session: ICopilotCLISession): Promise<void> {
1553+
const sessionId = session.sessionId;
15501554
let prUrl = session.createdPullRequestUrl;
15511555

15521556
if (!prUrl) {
1553-
prUrl = await this.detectPullRequestForSession(session.sessionId);
1557+
// Only attempt retry detection if the session has v2 worktree properties
1558+
// with branch info — v1 worktrees can't store PR URLs, and sessions
1559+
// without worktree properties have nothing to look up.
1560+
const worktreeProperties = await this.copilotCLIWorktreeManagerService.getWorktreeProperties(sessionId);
1561+
if (worktreeProperties?.version === 2 && worktreeProperties.branchName && worktreeProperties.repositoryPath) {
1562+
prUrl = await this.detectPullRequestWithRetry(sessionId);
1563+
}
15541564
}
15551565

15561566
if (!prUrl) {
15571567
return;
15581568
}
15591569

15601570
try {
1561-
const worktreeProperties = await this.copilotCLIWorktreeManagerService.getWorktreeProperties(session.sessionId);
1571+
const worktreeProperties = await this.copilotCLIWorktreeManagerService.getWorktreeProperties(sessionId);
15621572
if (worktreeProperties && worktreeProperties.version === 2) {
1563-
await this.copilotCLIWorktreeManagerService.setWorktreeProperties(session.sessionId, {
1573+
await this.copilotCLIWorktreeManagerService.setWorktreeProperties(sessionId, {
15641574
...worktreeProperties,
15651575
pullRequestUrl: prUrl,
15661576
changes: undefined,
15671577
});
15681578
this.sessionItemProvider.notifySessionsChange();
15691579
}
15701580
} catch (error) {
1571-
this.logService.error(`Failed to persist pull request metadata: ${error instanceof Error ? error.message : String(error)}`);
1581+
this.logService.error(`Failed to persist pull request metadata for session ${sessionId}: ${error instanceof Error ? error.message : String(error)}`);
1582+
}
1583+
}
1584+
1585+
/**
1586+
* Attempts to detect a pull request for a freshly-completed session using
1587+
* exponential backoff. The GitHub API may not have indexed the PR immediately
1588+
* after `gh pr create` returns, so we retry with increasing delays:
1589+
* attempt 1: 2s, attempt 2: 4s, attempt 3: 8s.
1590+
*/
1591+
private async detectPullRequestWithRetry(sessionId: string): Promise<string | undefined> {
1592+
const maxRetries = CopilotCLIChatSessionParticipant._PR_DETECTION_RETRY_COUNT;
1593+
const initialDelay = CopilotCLIChatSessionParticipant._PR_DETECTION_INITIAL_DELAY_MS;
1594+
1595+
for (let attempt = 0; attempt < maxRetries; attempt++) {
1596+
const delay = initialDelay * Math.pow(2, attempt);
1597+
await new Promise<void>(resolve => setTimeout(resolve, delay));
1598+
1599+
const prUrl = await this.detectPullRequestForSession(sessionId);
1600+
if (prUrl) {
1601+
return prUrl;
1602+
}
15721603
}
1604+
1605+
return undefined;
15731606
}
15741607

15751608
/**

src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import { IAgentSessionsWorkspace } from '../../common/agentSessionsWorkspace';
4040
import { IChatCustomAgentsService } from '../../common/chatCustomAgentsService';
4141
import { IChatSessionWorkspaceFolderService } from '../../common/chatSessionWorkspaceFolderService';
4242
import { IChatSessionWorktreeCheckpointService } from '../../common/chatSessionWorktreeCheckpointService';
43-
import { IChatSessionWorktreeService, type ChatSessionWorktreeFile, type ChatSessionWorktreeProperties } from '../../common/chatSessionWorktreeService';
43+
import { IChatSessionWorktreeService, type ChatSessionWorktreeFile, type ChatSessionWorktreeProperties, type ChatSessionWorktreePropertiesV2 } from '../../common/chatSessionWorktreeService';
4444
import { MockChatSessionMetadataStore } from '../../common/test/mockChatSessionMetadataStore';
4545
import { getWorkingDirectory, IWorkspaceInfo } from '../../common/workspaceInfo';
4646
import { IChatDelegationSummaryService } from '../../copilotcli/common/delegationSummaryService';
@@ -1971,4 +1971,141 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => {
19711971
expect(agent?.tools).toBeNull();
19721972
});
19731973
});
1974+
1975+
describe('PR detection with retry', () => {
1976+
let octoKitService: IOctoKitService;
1977+
1978+
const v2WorktreeProperties: ChatSessionWorktreePropertiesV2 = {
1979+
version: 2,
1980+
baseCommit: 'abc123',
1981+
branchName: 'copilot/test-branch',
1982+
baseBranchName: 'main',
1983+
repositoryPath: `${sep}repo`,
1984+
worktreePath: `${sep}worktree`,
1985+
};
1986+
1987+
const repoContext: RepoContext = {
1988+
rootUri: Uri.file(`${sep}repo`),
1989+
kind: 'repository',
1990+
remotes: ['origin'],
1991+
remoteFetchUrls: ['https://github.com/testowner/testrepo.git'],
1992+
} as unknown as RepoContext;
1993+
1994+
beforeEach(() => {
1995+
vi.useFakeTimers();
1996+
octoKitService = {
1997+
findPullRequestByHeadBranch: vi.fn(async () => undefined),
1998+
} as unknown as IOctoKitService;
1999+
2000+
// Set up folder & git repo so session creation succeeds with worktree isolation
2001+
folderRepositoryManager.setUntitledSessionFolder('untitled:pr-test', Uri.file(`${sep}repo`));
2002+
git.setRepo(repoContext);
2003+
(worktree.createWorktree as unknown as ReturnType<typeof vi.fn>).mockResolvedValue(v2WorktreeProperties);
2004+
// After session creation, getWorktreeProperties returns v2 for any session
2005+
(worktree.getWorktreeProperties as unknown as ReturnType<typeof vi.fn>).mockResolvedValue(v2WorktreeProperties);
2006+
TestCopilotCLISession.statusOverride = vscode.ChatSessionStatus.Completed;
2007+
2008+
// Recreate participant with the controllable octoKitService
2009+
participant = new CopilotCLIChatSessionParticipant(
2010+
contentProvider,
2011+
promptResolver,
2012+
itemProvider,
2013+
cloudProvider,
2014+
repositoryTracker,
2015+
git,
2016+
models as unknown as ICopilotCLIModels,
2017+
new NullCopilotCLIAgents(),
2018+
sessionService,
2019+
worktree,
2020+
worktreeCheckpointService,
2021+
workspaceFolderService,
2022+
telemetry,
2023+
logService,
2024+
new PromptsServiceImpl(new NullWorkspaceService()),
2025+
new (mock<IChatDelegationSummaryService>())(),
2026+
folderRepositoryManager,
2027+
configurationService,
2028+
sdk,
2029+
new MockChatSessionMetadataStore(),
2030+
customSessionTitleService,
2031+
octoKitService,
2032+
);
2033+
});
2034+
2035+
afterEach(() => {
2036+
vi.useRealTimers();
2037+
});
2038+
2039+
it('retries PR detection with exponential backoff and succeeds on second attempt', async () => {
2040+
const findPr = octoKitService.findPullRequestByHeadBranch as ReturnType<typeof vi.fn>;
2041+
findPr
2042+
.mockResolvedValueOnce(undefined) // attempt 1: not found
2043+
.mockResolvedValueOnce({ url: 'https://github.com/testowner/testrepo/pull/42' }); // attempt 2: found
2044+
2045+
const request = new TestChatRequest('Create a PR');
2046+
const context = createChatContext('untitled:pr-test', true);
2047+
const stream = new MockChatResponseStream();
2048+
const token = disposables.add(new CancellationTokenSource()).token;
2049+
2050+
const handlerPromise = participant.createHandler()(request, context, stream, token);
2051+
await vi.runAllTimersAsync();
2052+
await handlerPromise;
2053+
2054+
// Should have been called twice (after 2s delay, then after 4s delay)
2055+
expect(findPr).toHaveBeenCalledTimes(2);
2056+
// Should have persisted the PR URL
2057+
expect(worktree.setWorktreeProperties).toHaveBeenCalledWith(
2058+
expect.any(String),
2059+
expect.objectContaining({ pullRequestUrl: 'https://github.com/testowner/testrepo/pull/42' })
2060+
);
2061+
});
2062+
2063+
it('stops retrying once all attempts are exhausted', async () => {
2064+
const findPr = octoKitService.findPullRequestByHeadBranch as ReturnType<typeof vi.fn>;
2065+
findPr.mockResolvedValue(undefined); // always returns not found
2066+
2067+
const request = new TestChatRequest('Create something');
2068+
const context = createChatContext('untitled:pr-test', true);
2069+
const stream = new MockChatResponseStream();
2070+
const token = disposables.add(new CancellationTokenSource()).token;
2071+
2072+
const handlerPromise = participant.createHandler()(request, context, stream, token);
2073+
await vi.runAllTimersAsync();
2074+
await handlerPromise;
2075+
2076+
// 3 attempts total (after 2s, 4s, and 8s delays)
2077+
expect(findPr).toHaveBeenCalledTimes(3);
2078+
// Should NOT have persisted any PR URL since all attempts failed
2079+
const setPropsCallsWithPrUrl = (worktree.setWorktreeProperties as ReturnType<typeof vi.fn>).mock.calls
2080+
.filter((args: unknown[]) => (args[1] as { pullRequestUrl?: string })?.pullRequestUrl !== undefined);
2081+
expect(setPropsCallsWithPrUrl).toHaveLength(0);
2082+
});
2083+
2084+
it('skips retry when session already has createdPullRequestUrl', async () => {
2085+
const findPr = octoKitService.findPullRequestByHeadBranch as ReturnType<typeof vi.fn>;
2086+
2087+
// Make the session report a PR URL directly
2088+
TestCopilotCLISession.handleRequestHook = vi.fn(async () => {
2089+
const session = cliSessions[cliSessions.length - 1];
2090+
(session as any)._createdPullRequestUrl = 'https://github.com/testowner/testrepo/pull/99';
2091+
});
2092+
2093+
const request = new TestChatRequest('Create a PR via MCP');
2094+
const context = createChatContext('untitled:pr-test', true);
2095+
const stream = new MockChatResponseStream();
2096+
const token = disposables.add(new CancellationTokenSource()).token;
2097+
2098+
const handlerPromise = participant.createHandler()(request, context, stream, token);
2099+
await vi.runAllTimersAsync();
2100+
await handlerPromise;
2101+
2102+
// Should NOT have called the GitHub API since session had the URL
2103+
expect(findPr).not.toHaveBeenCalled();
2104+
// Should have persisted the session's PR URL
2105+
expect(worktree.setWorktreeProperties).toHaveBeenCalledWith(
2106+
expect.any(String),
2107+
expect.objectContaining({ pullRequestUrl: 'https://github.com/testowner/testrepo/pull/99' })
2108+
);
2109+
});
2110+
});
19742111
});

0 commit comments

Comments
 (0)