diff --git a/packages/cloud-agents/src/server/router/__tests__/external-issue-context.test.ts b/packages/cloud-agents/src/server/router/__tests__/external-issue-context.test.ts index 4b4c0308..a4e12f43 100644 --- a/packages/cloud-agents/src/server/router/__tests__/external-issue-context.test.ts +++ b/packages/cloud-agents/src/server/router/__tests__/external-issue-context.test.ts @@ -99,4 +99,142 @@ describe('gatherExternalIssueContext', () => { expect(result).toEqual({ contextMessages: [], toolsUsed: [] }); }); + + it('shares one lookup deadline across fetch attempts instead of extending it', async () => { + vi.useFakeTimers(); + + try { + vi.mocked(callRouterMcpTool).mockReturnValue(new Promise(() => {})); + + const pending = gatherExternalIssueContext( + createContext('Investigate https://github.com/acme/web/issues/42'), + ); + + await vi.advanceTimersByTimeAsync(8_000); + + expect(await pending).toEqual({ contextMessages: [], toolsUsed: [] }); + expect(callRouterMcpTool).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + + it('resolves a bare Linear identifier from the precheck', async () => { + vi.mocked(callRouterMcpTool).mockResolvedValue({ + title: 'Fix OAuth callback', + }); + + const result = await gatherExternalIssueContext( + createContext('Check ENG-512 when you get a chance'), + 'ENG-512', + ); + + expect(callRouterMcpTool).toHaveBeenCalledWith( + expect.objectContaining({ + serverId: 'linear', + toolName: 'get_issue', + args: { id: 'ENG-512' }, + }), + ); + expect(result.contextMessages[0]?.content).toContain('[ENG-512]'); + }); + + it('fans a bare issue number out across the configured repositories', async () => { + vi.mocked(callRouterMcpTool).mockResolvedValue({ title: 'Some issue' }); + + const result = await gatherExternalIssueContext( + createContextWithRepos('Check issue #234', ['acme/web', 'acme/api']), + '#234', + ); + + expect(callRouterMcpTool).toHaveBeenCalledWith( + expect.objectContaining({ + serverId: 'github', + args: expect.objectContaining({ repo: 'web', issue_number: 234 }), + }), + ); + expect(callRouterMcpTool).toHaveBeenCalledWith( + expect.objectContaining({ + serverId: 'github', + args: expect.objectContaining({ repo: 'api', issue_number: 234 }), + }), + ); + expect(result.contextMessages[0]?.content).toContain('[acme/web#234]'); + expect(result.contextMessages[0]?.content).toContain('[acme/api#234]'); + }); + + it('only resolves a repo-qualified reference against configured repositories', async () => { + vi.mocked(callRouterMcpTool).mockResolvedValue({ title: 'Some issue' }); + + const configured = await gatherExternalIssueContext( + createContextWithRepos('Check acme/web#42', ['acme/web']), + 'acme/web#42', + ); + + expect(configured.toolsUsed).toEqual(['github.issue_read']); + + vi.clearAllMocks(); + vi.mocked(callRouterMcpTool).mockResolvedValue({ title: 'Some issue' }); + + const unconfigured = await gatherExternalIssueContext( + createContextWithRepos('Check evil/repo#1', ['acme/web']), + 'evil/repo#1', + ); + + expect(callRouterMcpTool).not.toHaveBeenCalled(); + expect(unconfigured).toEqual({ contextMessages: [], toolsUsed: [] }); + }); + + it('caps a bare-number fan-out at the first five configured repositories', async () => { + vi.mocked(callRouterMcpTool).mockResolvedValue({ title: 'Some issue' }); + + const result = await gatherExternalIssueContext( + createContextWithRepos('Check issue #234', [ + 'acme/web', + 'acme/api', + 'acme/mobile', + 'acme/infra', + 'acme/docs', + 'acme/tooling', + 'acme/design', + ]), + '#234', + ); + + const fetchedRepos = vi + .mocked(callRouterMcpTool) + .mock.calls.map(([options]) => (options.args as { repo: string }).repo); + + expect(new Set(fetchedRepos)).toEqual( + new Set(['web', 'api', 'mobile', 'infra', 'docs']), + ); + expect(result.contextMessages[0]?.content).toContain('[acme/docs#234]'); + expect(result.contextMessages[0]?.content).not.toContain('acme/tooling'); + }); + + it('prefers pasted issue URLs over the precheck reference', async () => { + vi.mocked(callRouterMcpTool).mockResolvedValue({ title: 'Some issue' }); + + await gatherExternalIssueContext( + createContext('Investigate https://github.com/acme/web/issues/42'), + 'ENG-512', + ); + + expect(callRouterMcpTool).not.toHaveBeenCalledWith( + expect.objectContaining({ serverId: 'linear' }), + ); + }); }); + +function createContextWithRepos( + taskDescription: string, + repositoryNames: string[], +): RoutingContext { + return { + taskDescription, + source: { type: 'slack' }, + availableEnvironments: [ + { id: 'env-1', name: 'Full Stack', repositoryNames }, + ], + }; +} diff --git a/packages/cloud-agents/src/server/router/__tests__/router-service.test.ts b/packages/cloud-agents/src/server/router/__tests__/router-service.test.ts index 449a4222..3046df99 100644 --- a/packages/cloud-agents/src/server/router/__tests__/router-service.test.ts +++ b/packages/cloud-agents/src/server/router/__tests__/router-service.test.ts @@ -202,6 +202,53 @@ describe('routeTask', () => { }); }); + it('resolves a bare issue reference from the precheck against configured repos', async () => { + mockCallRouterMcpTool.mockResolvedValue({ + title: 'Dashboard export button broken', + }); + mockGenerateTrackedNonTaskObject + .mockResolvedValueOnce({ + object: { + workspaceValue: 'Full Stack', + reasoning: 'The message references an issue by number only.', + confidence: 0.4, + needsExternalLookup: true, + externalReference: '#234', + }, + }) + .mockResolvedValueOnce({ + object: { + workspaceValue: 'Full Stack', + reasoning: 'The referenced issue describes dashboard work.', + confidence: 0.9, + needsExternalLookup: false, + externalReference: null, + }, + }); + + const result = await routeTask( + createContext({ taskDescription: 'Check issue #234' }), + ); + + expect(mockCallRouterMcpTool).toHaveBeenCalledWith( + expect.objectContaining({ + serverId: 'github', + toolName: 'issue_read', + args: expect.objectContaining({ issue_number: 234 }), + }), + ); + expect(mockGenerateTrackedNonTaskObject).toHaveBeenCalledTimes(2); + expect(result).toMatchObject({ + status: 'routed', + result: { + debug: { + phase: 'mcp', + needsExternalLookup: true, + }, + }, + }); + }); + it('keeps the precheck decision when the requested fetch returns nothing', async () => { mockCallRouterMcpTool.mockRejectedValue(new Error('Not connected')); mockGenerateTrackedNonTaskObject.mockResolvedValue({ diff --git a/packages/cloud-agents/src/server/router/external-issue-context.ts b/packages/cloud-agents/src/server/router/external-issue-context.ts index 40a01ac8..f442a4f9 100644 --- a/packages/cloud-agents/src/server/router/external-issue-context.ts +++ b/packages/cloud-agents/src/server/router/external-issue-context.ts @@ -10,6 +10,7 @@ import type { RoutingContext } from './types'; const MAX_EXTERNAL_ISSUES = 2; const MAX_EXTERNAL_CONTEXT_CHARS = 8_000; const EXTERNAL_LOOKUP_TIMEOUT_MS = 8_000; +const MAX_BARE_REFERENCE_REPOS = 5; interface ExternalIssueReference { url: string; @@ -51,6 +52,104 @@ function parseExternalIssueReferences(text: string): ExternalIssueReference[] { return references; } +function referenceFromCanonicalUrl( + label: string, + canonicalUrl: string, +): ExternalIssueReference | null { + try { + const match = matchExternalIssueUrl(new URL(canonicalUrl)); + + return match ? { url: label, fetchAttempts: match.fetchAttempts } : null; + } catch { + return null; + } +} + +function uniqueConfiguredRepositories(context: RoutingContext): string[] { + const repositories = new Set(); + + for (const environment of context.availableEnvironments) { + for (const name of environment.repositoryNames ?? []) { + repositories.add(name); + } + } + + return [...repositories]; +} + +/** + * Resolves a bare reference the routing precheck extracted (for example + * "#234", "acme/web#234", or "ENG-512") against the deployment's own + * configuration. The model only proposes the reference text; the fetch + * targets always come from this closed candidate set, so a steered or + * hallucinated reference can at most read issues the deployment already + * routes for. An ambiguous issue number fans out across the first + * MAX_BARE_REFERENCE_REPOS configured repositories (environment order) so the + * fetch stays bounded on deployments with many repositories. + */ +function resolveBareIssueReferences( + context: RoutingContext, + externalReference: string | null | undefined, +): ExternalIssueReference[] { + const raw = externalReference?.trim(); + + if (!raw) { + return []; + } + + if (/^[A-Za-z]+-\d+$/.test(raw)) { + const reference = referenceFromCanonicalUrl( + raw, + `https://linear.app/_/issue/${raw}`, + ); + + return reference ? [reference] : []; + } + + const configuredRepositories = uniqueConfiguredRepositories(context); + const githubReference = (fullRepository: string, issueNumber: string) => + referenceFromCanonicalUrl( + `${fullRepository}#${issueNumber}`, + `https://github.com/${fullRepository}/issues/${issueNumber}`, + ); + + const qualified = raw.match( + /^(?[^\s/#]+)\/(?[^\s/#]+)#(?\d+)$/, + ); + + if (qualified?.groups) { + const fullRepository = configuredRepositories.find( + (name) => + name.toLowerCase() === + `${qualified.groups!.owner}/${qualified.groups!.repository}`.toLowerCase(), + ); + const reference = + fullRepository && + githubReference(fullRepository, qualified.groups.issueNumber!); + + return reference ? [reference] : []; + } + + const bareNumber = raw.match(/^#?(?\d+)$/); + + if (bareNumber?.groups && configuredRepositories.length > 0) { + // Environment order decides which repositories make the cut; repos where + // the issue number does not exist drop out at fetch time anyway. + return configuredRepositories + .slice(0, MAX_BARE_REFERENCE_REPOS) + .flatMap((fullRepository) => { + const reference = githubReference( + fullRepository, + bareNumber.groups!.issueNumber!, + ); + + return reference ? [reference] : []; + }); + } + + return []; +} + function serializeExternalContext(value: unknown): string { if (typeof value === 'string') { return value; @@ -64,9 +163,11 @@ function serializeExternalContext(value: unknown): string { } async function withExternalLookupTimeout( - operation: Promise, + operation: () => Promise, deadline: number, ): Promise { + // Checked before invoking so an exhausted deadline never dispatches + // another MCP call whose result nothing will await. const remainingMs = deadline - Date.now(); if (remainingMs <= 0) { return null; @@ -76,7 +177,7 @@ async function withExternalLookupTimeout( try { return await Promise.race([ - operation, + operation(), new Promise((resolve) => { timeout = setTimeout(resolve, remainingMs, null); }), @@ -97,12 +198,13 @@ async function fetchExternalIssueContext( for (const attempt of reference.fetchAttempts) { const result = await withExternalLookupTimeout( - callRouterMcpTool({ - context, - serverId: attempt.serverId, - toolName: attempt.toolName, - args: attempt.args, - }), + () => + callRouterMcpTool({ + context, + serverId: attempt.serverId, + toolName: attempt.toolName, + args: attempt.args, + }), deadline, ); @@ -120,8 +222,13 @@ async function fetchExternalIssueContext( export async function gatherExternalIssueContext( context: RoutingContext, + externalReference?: string | null, ): Promise<{ contextMessages: ModelMessage[]; toolsUsed: string[] }> { - const references = parseExternalIssueReferences(context.taskDescription); + const urlReferences = parseExternalIssueReferences(context.taskDescription); + const references = + urlReferences.length > 0 + ? urlReferences + : resolveBareIssueReferences(context, externalReference); const results = await Promise.all( references.map(async (reference) => ({ reference, diff --git a/packages/cloud-agents/src/server/router/mcp-gather.ts b/packages/cloud-agents/src/server/router/mcp-gather.ts index 1241e049..a965d224 100644 --- a/packages/cloud-agents/src/server/router/mcp-gather.ts +++ b/packages/cloud-agents/src/server/router/mcp-gather.ts @@ -91,9 +91,13 @@ export async function gatherContextFromConfiguredMcps< } // The precheck asked for the linked issue, so the fetch deadline is only - // paid when it can change the decision. Fail-open: with nothing fetched, - // the precheck decision stands. - const externalIssueContext = await gatherExternalIssueContext(context); + // paid when it can change the decision. Bare references (no pasted URL) + // resolve against the deployment's configured repositories only. Fail-open: + // with nothing fetched, the precheck decision stands. + const externalIssueContext = await gatherExternalIssueContext( + context, + response.externalReference, + ); if (externalIssueContext.contextMessages.length === 0) { return { response, toolsUsed: [], phase: 'direct', needsExternalLookup };