diff --git a/api/server/routes/__test-utils__/convos-route-mocks.js b/api/server/routes/__test-utils__/convos-route-mocks.js index bd86738d139..ce6a630d933 100644 --- a/api/server/routes/__test-utils__/convos-route-mocks.js +++ b/api/server/routes/__test-utils__/convos-route-mocks.js @@ -147,6 +147,11 @@ module.exports = { () => (_req, res) => res.status(200).json({ tasks: [] }), ), createBackgroundTaskPolicyMiddleware: jest.fn(() => (_req, _res, next) => next()), + createGitHubPullRequestSource: jest.fn(() => ({ find: jest.fn() })), + createPullRequestLookup: jest.fn(() => jest.fn()), + createConversationPullRequestHandler: jest.fn( + () => (_req, res) => res.status(200).json({ pullRequest: null }), + ), createBackgroundTaskCancelHandler: jest.fn( () => (_req, res) => res.status(200).json({ results: [] }), ), diff --git a/api/server/routes/__tests__/convos.spec.js b/api/server/routes/__tests__/convos.spec.js index 1cc49c7430f..0c448a67abc 100644 --- a/api/server/routes/__tests__/convos.spec.js +++ b/api/server/routes/__tests__/convos.spec.js @@ -237,6 +237,21 @@ describe('Convos Routes', () => { }); }); + describe('GET /:conversationId/pull-request', () => { + it('is registered and reaches the pull request handler', async () => { + const response = await request(app).get('/api/convos/ordinary/pull-request'); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ pullRequest: null }); + }); + + it('does not fall through to the conversation read', async () => { + await request(app).get('/api/convos/ordinary/pull-request'); + + expect(getConvo).not.toHaveBeenCalled(); + }); + }); + describe('POST /import', () => { const { importConversations } = require('~/server/utils/import'); diff --git a/api/server/routes/convos.js b/api/server/routes/convos.js index ebb02973b90..4c05abb57f6 100644 --- a/api/server/routes/convos.js +++ b/api/server/routes/convos.js @@ -19,6 +19,9 @@ const { createBackgroundTaskIndexHandler, createBackgroundTaskCancelHandler, createBackgroundTaskPolicyMiddleware, + createConversationPullRequestHandler, + createGitHubPullRequestSource, + createPullRequestLookup, backgroundTaskRegistry, createSubagentThreadViewHandler, createGeneratedTitleHandler, @@ -167,6 +170,12 @@ const backgroundTaskIndexHandler = createBackgroundTaskIndexHandler({ const backgroundTaskCancelHandler = createBackgroundTaskCancelHandler({ registry: backgroundTaskRegistry, }); +const conversationPullRequestHandler = createConversationPullRequestHandler({ + getConvoLaneGit: db.getConvoLaneGit, + getAppConfig, + lookup: createPullRequestLookup({ source: createGitHubPullRequestSource({ fetchFn: fetch }) }), + env: process.env, +}); router.use(requireJwtAuth); const isValidProjectFilter = (projectId) => @@ -239,6 +248,7 @@ router.post( subagentControlHandler, ); router.get('/:parentConversationId/subagents', parentSubagentIndexHandler); +router.get('/:conversationId/pull-request', conversationPullRequestHandler); router.get('/:conversationId/background-tasks', backgroundTaskPolicy, backgroundTaskIndexHandler); router.post( '/:conversationId/background-tasks/cancel', diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 77af238288e..cd1c63132a6 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -50,6 +50,7 @@ const { AGENT_EXPECTED_MCP_TOOLS_UNAVAILABLE, isFatalAgentInitializationError, codeExecutionAuthHeaders, + createLaneGitRecorder, createAttachedWorkspaceBashTool, createRepositoryInstructionSource, createRepositoryInstructionLoader, @@ -130,7 +131,13 @@ const { createOpenIDSessionTokenProvider } = require('~/server/services/OpenIDSe const { getMCPRequestContext } = require('~/server/services/MCPRequestContext'); const { recordUsage } = require('~/server/services/Threads'); const { loadTools } = require('~/app/clients/tools/util'); -const { findPluginAuthsByKeys, getRoleByName } = require('~/models'); +const { + findPluginAuthsByKeys, + getRoleByName, + setConvoLaneGit, + getConvoLaneContext, + reserveConvoLaneGitSeq, +} = require('~/models'); const { getFlowStateManager, getMCPServersRegistry } = require('~/config'); const { getLogStores } = require('~/cache'); @@ -2386,6 +2393,19 @@ async function loadToolsForExecution({ baseUrl: codeExecutionContext.baseUrl, workspaceId: codeExecutionContext.codeWorkspace.workspaceId, workspaceInstanceId: codeExecutionContext.codeWorkspace.workspaceInstanceId, + onLaneGit: await createLaneGitRecorder({ + enabled: req.config?.endpoints?.agents?.pullRequests?.enabled === true, + user: req.user.id, + conversationId: conversationId ?? runtimeRequestBody?.conversationId, + repo: codeExecutionContext.codeWorkspace.environment?.repo, + workspace: { + environmentId: codeExecutionContext.codeWorkspace.environmentId, + workspaceId: codeExecutionContext.codeWorkspace.workspaceId, + }, + getConvoLaneContext, + reserveConvoLaneGitSeq, + setConvoLaneGit, + }), linkedWorktrees: codeExecutionContext.codeWorkspace.linkedWorktrees, nativeSandbox: codeExecutionContext.codeWorkspace.nativeSandbox, environment: codeExecutionContext.codeWorkspace.environment, diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index df7cbed6be6..29d18e3b809 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -22,6 +22,11 @@ const mockGetCachedTools = jest.fn(); const mockSendEvent = jest.fn(); const mockEmitChunk = jest.fn(); const mockCreateAttachedWorkspaceBashTool = jest.fn(() => ({ name: AgentConstants.BASH_TOOL })); +const mockLaneGitRecorder = jest.fn(); +const mockCreateLaneGitRecorder = jest.fn(() => mockLaneGitRecorder); +const mockSetConvoLaneGit = jest.fn(); +const mockGetConvoLaneContext = jest.fn(); +const mockReserveConvoLaneGitSeq = jest.fn(); const attachedWorkspaceOperations = [ 'read_file', 'search_text', @@ -121,6 +126,7 @@ jest.mock('@librechat/api', () => ({ resolveCodeExecutionWorkspaceContext: (...args) => mockResolveCodeExecutionWorkspaceContext(...args), createAttachedWorkspaceBashTool: (...args) => mockCreateAttachedWorkspaceBashTool(...args), + createLaneGitRecorder: (...args) => mockCreateLaneGitRecorder(...args), })); const mockLoadToolsUtil = jest.fn(); @@ -171,6 +177,9 @@ const mockGetRoleByName = jest.fn(); jest.mock('~/models', () => ({ findPluginAuthsByKeys: jest.fn(), getRoleByName: (...args) => mockGetRoleByName(...args), + setConvoLaneGit: (...args) => mockSetConvoLaneGit(...args), + getConvoLaneContext: (...args) => mockGetConvoLaneContext(...args), + reserveConvoLaneGitSeq: (...args) => mockReserveConvoLaneGitSeq(...args), })); jest.mock('~/config', () => ({ getFlowStateManager: jest.fn(() => mockFlowManager), @@ -3221,6 +3230,7 @@ describe('ToolService - Action Capability Gating', () => { authHeaders: expect.any(Function), baseUrl: 'http://attached-code.test/v1', workspaceId: 'project-a', + onLaneGit: mockLaneGitRecorder, gitIdentity: { name: 'LibreChat Agent', email: 'agent@example.com' }, maxTimeoutMs: 65_000, defaultTimeoutMs: 60_000, @@ -3243,6 +3253,108 @@ describe('ToolService - Action Capability Gating', () => { expect(result.loadedTools).toContainEqual({ name: AgentConstants.BASH_TOOL }); }); + it('records the lane for the requesting user and the resolved conversation', async () => { + const capabilities = [ + AgentCapabilities.tools, + AgentCapabilities.execute_code, + AgentCapabilities.stateful_code_sessions, + ]; + const req = createMockReq(capabilities); + req.config.endpoints[EModelEndpoint.agents].pullRequests = { enabled: true }; + req.body = { + conversationId: 'body-convo', + codeWorkspaces: [{ environmentId: 'personal-machine', workspaceId: 'project-a' }], + }; + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + mockResolveCodeExecutionContext.mockReturnValueOnce({ + baseUrl: 'http://attached-code.test/v1', + codeSessionKey: 'execute_code:stateful:attached', + executionProfile: 'stateful', + statefulSessions: true, + environmentType: 'attached', + environmentId: 'personal-machine', + bridgeWorkerId: 'worker-abc', + }); + mockCreateLaneGitRecorder.mockClear(); + + await loadToolsForExecution({ + req, + res: {}, + agent: { + id: 'attached-agent', + tools: [Tools.execute_code], + stateful_code_sessions: true, + stateful_code_environment: 'agent-user', + }, + conversationId: 'resolved-convo', + toolNames: [AgentConstants.BASH_TOOL], + toolRegistry: new Map([[AgentConstants.BASH_TOOL, { name: AgentConstants.BASH_TOOL }]]), + actionsEnabled: false, + }); + + expect(mockCreateLaneGitRecorder).toHaveBeenCalledWith( + expect.objectContaining({ + enabled: true, + user: req.user.id, + conversationId: 'resolved-convo', + workspace: { environmentId: 'personal-machine', workspaceId: 'project-a' }, + getConvoLaneContext: expect.any(Function), + reserveConvoLaneGitSeq: expect.any(Function), + setConvoLaneGit: expect.any(Function), + }), + ); + const { setConvoLaneGit } = mockCreateLaneGitRecorder.mock.calls[0][0]; + const input = { user: 'u', conversationId: 'c', laneGit: { branch: 'main', head: null } }; + await setConvoLaneGit(input); + expect(mockSetConvoLaneGit).toHaveBeenCalledWith(input); + }); + + it.each([ + ['unset', undefined], + ['disabled', { enabled: false }], + ])('does not enable lane recording when pull requests are %s', async (_label, setting) => { + const capabilities = [ + AgentCapabilities.tools, + AgentCapabilities.execute_code, + AgentCapabilities.stateful_code_sessions, + ]; + const req = createMockReq(capabilities); + if (setting) req.config.endpoints[EModelEndpoint.agents].pullRequests = setting; + req.body = { + codeWorkspaces: [{ environmentId: 'personal-machine', workspaceId: 'project-a' }], + }; + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + mockResolveCodeExecutionContext.mockReturnValueOnce({ + baseUrl: 'http://attached-code.test/v1', + codeSessionKey: 'execute_code:stateful:attached', + executionProfile: 'stateful', + statefulSessions: true, + environmentType: 'attached', + environmentId: 'personal-machine', + bridgeWorkerId: 'worker-abc', + }); + mockCreateLaneGitRecorder.mockClear(); + + await loadToolsForExecution({ + req, + res: {}, + agent: { + id: 'attached-agent', + tools: [Tools.execute_code], + stateful_code_sessions: true, + stateful_code_environment: 'agent-user', + }, + conversationId: 'resolved-convo', + toolNames: [AgentConstants.BASH_TOOL], + toolRegistry: new Map([[AgentConstants.BASH_TOOL, { name: AgentConstants.BASH_TOOL }]]), + actionsEnabled: false, + }); + + expect(mockCreateLaneGitRecorder).toHaveBeenCalledWith( + expect.objectContaining({ enabled: false }), + ); + }); + it('passes negotiated lane and native sandbox capabilities to the attached bash tool', async () => { const capabilities = [ AgentCapabilities.tools, diff --git a/librechat.example.yaml b/librechat.example.yaml index a40971434ad..28ff6059494 100644 --- a/librechat.example.yaml +++ b/librechat.example.yaml @@ -922,6 +922,34 @@ endpoints: # completionReceiptBatching: true # # Allow owners to cooperatively cancel ordinary background tools, including BYOM Bash. # ordinaryToolCancellation: false + # # (optional) Show a conversation's pull request in the chat header. Off by default. It needs + # # a code worker built from LibreChat-AI/code-interpreter#311 or later that reports its + # # branch, with CODEAPI_BRIDGE_LANE_GIT=true set on the Code API; without both, every chat + # # simply shows no pull request. It also needs a GitHub token held on the server. Use a + # # fine-grained token limited to the repositories involved, with read-only "Pull requests", + # # "Checks" and "Contents" permissions. Without "Checks" every matched pull request fails to + # # load, and without "Contents" a chat whose branch has moved on since its last command does. + # pullRequests: + # enabled: false + # # Environment variable reference only, never the token itself. + # token: "${GITHUB_PULL_REQUEST_TOKEN}" + # # Repositories the token may be used for, as owner/name or owner/*. Required: a worker + # # reports its own repository, and only these are ever looked up with the server's token. + # allowedRepositories: ["LibreChat-AI/LibreChat"] + # # Seconds a looked-up pull request is reused before GitHub is asked again (5 to 3600). + # cacheTtlSeconds: 30 + # # Results cached per credential before the oldest is evicted (10 to 100000). Raise it when many + # # repositories and branches are active at once, or fresh results are evicted and GitHub is asked again. + # cacheMaxEntries: 500 + # # Credentials cached at once (1 to 10000), deployment-wide; the one idle longest is dropped + # # past it. When principals configure different values, the largest applies. + # cacheMaxCredentials: 256 + # # Bounds on one lookup. A repository with many check runs reads one page of 100 per request. + # requestTimeoutSeconds: 10 # longest single GitHub request (1 to 60) + # lookupTimeoutSeconds: 30 # longest the whole lookup may take (1 to 120) + # maxCheckRunPages: 10 # pages read before checks are reported as running (1 to 50) + # maxCandidatePullRequests: 10 # pull requests listed per state when matching a reused branch (1 to 100) + # maxHeadComparisons: 3 # candidates compared with the chat's last commit (0 to 20) # # "run_in_background" makes Code Interpreter tools eligible by default and enables per-tool MCP opt-in. # # "tool_intents" enables live model-written labels for native tools and opted-in MCP tools. # # (optional) Require user approval before matching tool calls. Disabled by default. diff --git a/packages/api/src/code/command.ts b/packages/api/src/code/command.ts index 88154a05d5e..593f96d2949 100644 --- a/packages/api/src/code/command.ts +++ b/packages/api/src/code/command.ts @@ -14,7 +14,11 @@ import type { import type { createBashProgrammaticToolCallingSchema } from '@librechat/agents'; import type { DynamicStructuredTool } from '@librechat/agents/langchain/tools'; import type { LCTool } from '@librechat/agents'; -import type { WorkspaceAdmissionOptions, WorkspaceExecuteCommandResult } from './workspace'; +import type { + WorkspaceAdmissionOptions, + WorkspaceExecuteCommandResult, + WorkspaceLaneGit, +} from './workspace'; import type { CodeExecutionContext } from '~/agents/execution'; import type { CodeBridgeFetch } from './bridge'; import { @@ -468,11 +472,15 @@ export function createAttachedWorkspaceBashTool({ minCommandAdmissionMs, linkedWorktrees = false, nativeSandbox = false, + onLaneGit, fetchImpl, }: { baseUrl: string; authHeaders: () => Promise> | Record; workspaceId: string; + /** Receives the lane's branch and head when a finished command reports them. It is not awaited + * and must not throw: a failure here never changes the command's result. */ + onLaneGit?: (laneGit: WorkspaceLaneGit) => void; workspaceInstanceId?: string; /** The worker runs each `.worktrees/` in its own lane; a matching `cwd` is routed there. */ linkedWorktrees?: boolean; @@ -609,6 +617,7 @@ export function createAttachedWorkspaceBashTool({ throw new Error('Attached workspace returned an unexpected command result.'); } logger.debug('[BYOMCommand] transport completed', trace); + if (result.laneGit != null) onLaneGit?.(result.laneGit); let content = formatCommandResult( result, selectedTimeoutMs, diff --git a/packages/api/src/code/index.ts b/packages/api/src/code/index.ts index 237e3a04561..b6adfe68253 100644 --- a/packages/api/src/code/index.ts +++ b/packages/api/src/code/index.ts @@ -5,6 +5,7 @@ export * from './bridge'; export * from './lifecycle'; export * from './workspace'; export * from './command'; +export * from './lane'; export * from './capabilities'; export * from './decision'; export * from './inheritance'; diff --git a/packages/api/src/code/lane.spec.ts b/packages/api/src/code/lane.spec.ts new file mode 100644 index 00000000000..922003e5731 --- /dev/null +++ b/packages/api/src/code/lane.spec.ts @@ -0,0 +1,607 @@ +import { logger } from '@librechat/data-schemas'; +import type { CodeBridgeFetch } from './bridge'; +import { createLaneGitRecorder as createRecorder } from './lane'; +import { createAttachedWorkspaceBashTool } from './command'; + +jest.mock('@librechat/data-schemas', () => ({ + ...jest.requireActual('@librechat/data-schemas'), + logger: { warn: jest.fn(), debug: jest.fn(), info: jest.fn(), error: jest.fn() }, +})); + +type RecorderOptions = Parameters[0]; + +/** Places the recorder once, as the tool does, then reports through it in call order. */ +const createLaneGitRecorder = (options: Omit) => { + const ready = createRecorder({ enabled: true, ...options }); + return (laneGit: Parameters>>[0]) => + ready.then((record) => (record ? record(laneGit) : false)); +}; + +const head = 'a'.repeat(40); +const laneGit = { branch: 'feat/pr-chip', head }; + +/** A reserver that hands out 1, 2, 3 ... like the database counter does. */ +const counter = () => { + let next = 0; + return jest.fn(async () => ++next); +}; + +describe('createLaneGitRecorder', () => { + const make = (overrides: Record = {}) => { + const setConvoLaneGit = jest.fn().mockResolvedValue(true); + const reserveConvoLaneGitSeq = counter(); + const record = createLaneGitRecorder({ + user: 'u1', + conversationId: 'c1', + reserveConvoLaneGitSeq, + setConvoLaneGit, + ...overrides, + }); + return { record, setConvoLaneGit, reserveConvoLaneGitSeq }; + }; + + it('writes the reported state with a reserved sequence number', async () => { + const { record, setConvoLaneGit, reserveConvoLaneGitSeq } = make(); + await expect(record?.(laneGit)).resolves.toBe(true); + expect(reserveConvoLaneGitSeq).toHaveBeenCalledWith('u1', 'c1'); + expect(setConvoLaneGit).toHaveBeenCalledWith({ + user: 'u1', + conversationId: 'c1', + laneGit, + seq: 1, + }); + }); + + it.each([ + ['a missing user', { user: undefined, conversationId: 'c1' }], + ['an empty user', { user: '', conversationId: 'c1' }], + ['a missing conversation', { user: 'u1', conversationId: undefined }], + ['an empty conversation', { user: 'u1', conversationId: '' }], + ])('does not record with %s', async (_label, ids) => { + const setConvoLaneGit = jest.fn(); + const record = await createRecorder({ + enabled: true, + ...ids, + reserveConvoLaneGitSeq: counter(), + setConvoLaneGit, + } as RecorderOptions); + expect(record).toBeUndefined(); + expect(setConvoLaneGit).not.toHaveBeenCalled(); + }); + + it('stores a plain owner/name repository with the lane', async () => { + const { record, setConvoLaneGit } = make({ repo: 'LibreChat-AI/LibreChat' }); + await record?.(laneGit); + expect(setConvoLaneGit).toHaveBeenCalledWith( + expect.objectContaining({ repo: 'LibreChat-AI/LibreChat' }), + ); + }); + + it.each(['../x', 'o/..', 'a b/c', 'owner', 'o/r/extra', 'x'.repeat(300)])( + 'drops the unsafe repository %s but still records the lane', + async (repo) => { + const { record, setConvoLaneGit } = make({ repo }); + await record?.(laneGit); + expect(setConvoLaneGit).toHaveBeenCalledTimes(1); + expect(setConvoLaneGit.mock.calls[0][0]).not.toHaveProperty('repo'); + }, + ); + + it('swallows a write failure and logs only safe metadata', async () => { + const setConvoLaneGit = jest.fn().mockRejectedValue(new Error('mongodb://user:secret@host')); + const { record } = make({ setConvoLaneGit }); + await expect(record?.(laneGit)).resolves.toBe(false); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(JSON.stringify((logger.warn as jest.Mock).mock.calls)).not.toContain('secret'); + }); + + it('swallows a reservation failure and writes nothing', async () => { + const reserveConvoLaneGitSeq = jest.fn().mockRejectedValue(new Error('mongodb://u:secret@h')); + const { record, setConvoLaneGit } = make({ reserveConvoLaneGitSeq }); + await expect(record?.(laneGit)).resolves.toBe(false); + expect(setConvoLaneGit).not.toHaveBeenCalled(); + expect(JSON.stringify((logger.warn as jest.Mock).mock.calls)).not.toContain('secret'); + }); + + it('writes nothing when the conversation is not saved yet', async () => { + const { record, setConvoLaneGit } = make({ + reserveConvoLaneGitSeq: jest.fn().mockResolvedValue(null), + }); + await expect(record?.(laneGit)).resolves.toBe(false); + expect(setConvoLaneGit).not.toHaveBeenCalled(); + }); +}); + +describe('attached bash tool lane reporting', () => { + const response = (extra: Record = {}) => + Response.json({ + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'project-a', + exitCode: 0, + stdout: 'ok', + stderr: '', + truncated: false, + timedOut: false, + ...extra, + }); + const build = (fetchImpl: CodeBridgeFetch, onLaneGit?: jest.Mock) => + createAttachedWorkspaceBashTool({ + baseUrl: 'https://code.example.com/v1', + authHeaders: () => ({}), + workspaceId: 'project-a', + onLaneGit, + fetchImpl, + }); + + it('hands the reported branch and head to the recorder', async () => { + const onLaneGit = jest.fn(); + await build( + jest.fn(async () => response({ laneGit })), + onLaneGit, + ).invoke({ command: 'git status' }); + expect(onLaneGit).toHaveBeenCalledWith(laneGit); + }); + + it('hands over a detached lane as nulls, not as unknown', async () => { + const onLaneGit = jest.fn(); + const detached = { branch: null, head: null }; + await build( + jest.fn(async () => response({ laneGit: detached })), + onLaneGit, + ).invoke({ command: 'git status' }); + expect(onLaneGit).toHaveBeenCalledWith(detached); + }); + + it('does not call the recorder when the worker sent no laneGit', async () => { + const onLaneGit = jest.fn(); + await build( + jest.fn(async () => response()), + onLaneGit, + ).invoke({ command: 'ls' }); + expect(onLaneGit).not.toHaveBeenCalled(); + }); + + it('still returns the command output when no recorder is configured', async () => { + const output = await build(jest.fn(async () => response({ laneGit }))).invoke({ + command: 'ls', + }); + expect(String(output)).toContain('ok'); + }); +}); + +describe('createLaneGitRecorder ordering', () => { + const flush = () => new Promise((resolve) => setImmediate(resolve)); + + it('takes sequence numbers in report order and writes each with its own', async () => { + const setConvoLaneGit = jest.fn().mockResolvedValue(true); + const record = createLaneGitRecorder({ + user: 'u1', + conversationId: 'order-1', + reserveConvoLaneGitSeq: counter(), + setConvoLaneGit, + }); + await Promise.all([record?.({ branch: 'a', head }), record?.({ branch: 'b', head })]); + const writes = setConvoLaneGit.mock.calls.map(([call]) => [call.laneGit.branch, call.seq]); + expect(writes.sort()).toEqual([ + ['a', 1], + ['b', 2], + ]); + }); + + it('gives a later report the higher number even when the first reservation is slow', async () => { + let issued = 0; + const releases: Array<() => void> = []; + /** The first reservation takes longer than any later one, as a slow replica round trip can. */ + const reserve = jest.fn( + () => + new Promise((resolve) => { + const slow = issued === 0; + issued += 1; + const number = issued; + if (slow) releases.push(() => resolve(number)); + else resolve(number); + }), + ); + const setConvoLaneGit = jest.fn().mockResolvedValue(true); + const record = createLaneGitRecorder({ + user: 'u1', + conversationId: 'order-slow', + reserveConvoLaneGitSeq: reserve, + setConvoLaneGit, + }); + const first = record?.({ branch: 'first', head }); + const second = record?.({ branch: 'second', head }); + await flush(); + expect(reserve).toHaveBeenCalledTimes(1); + releases[0](); + await Promise.all([first, second]); + const bySeq = setConvoLaneGit.mock.calls + .map(([call]) => [call.seq, call.laneGit.branch]) + .sort(); + expect(bySeq).toEqual([ + [1, 'first'], + [2, 'second'], + ]); + }); + + it('does not wait for one write to finish before reserving the next report', async () => { + const reserve = counter(); + const releases: Array<() => void> = []; + const setConvoLaneGit = jest.fn( + () => new Promise((resolve) => releases.push(() => resolve(true))), + ); + const record = createLaneGitRecorder({ + user: 'u1', + conversationId: 'order-2', + reserveConvoLaneGitSeq: reserve, + setConvoLaneGit, + }); + void record?.({ branch: 'a', head }); + void record?.({ branch: 'b', head }); + await flush(); + expect(reserve).toHaveBeenCalledTimes(2); + expect(setConvoLaneGit).toHaveBeenCalledTimes(2); + releases.forEach((release) => release()); + }); + + it('orders reservations across recorders built for the same conversation', async () => { + const reserve = counter(); + const setConvoLaneGit = jest.fn().mockResolvedValue(true); + const make = () => + createLaneGitRecorder({ + user: 'u1', + conversationId: 'order-3', + reserveConvoLaneGitSeq: reserve, + setConvoLaneGit, + }); + await Promise.all([make()?.({ branch: 'a', head }), make()?.({ branch: 'b', head })]); + const bySeq = setConvoLaneGit.mock.calls + .map(([call]) => [call.seq, call.laneGit.branch]) + .sort(); + expect(bySeq).toEqual([ + [1, 'a'], + [2, 'b'], + ]); + }); + + it('does not make one conversation or one user wait for another', async () => { + const releases: Array<() => void> = []; + const reserve = jest.fn( + () => new Promise((resolve) => releases.push(() => resolve(1))), + ); + const make = (user: string, conversationId: string) => + createLaneGitRecorder({ + user, + conversationId, + reserveConvoLaneGitSeq: reserve, + setConvoLaneGit: jest.fn().mockResolvedValue(true), + }); + void make('u1', 'order-4')?.({ branch: 'a', head }); + void make('u1', 'order-5')?.({ branch: 'b', head }); + void make('u2', 'order-4')?.({ branch: 'c', head }); + await flush(); + expect(reserve).toHaveBeenCalledTimes(3); + releases.forEach((release) => release()); + }); + + it('keeps reserving after one reservation fails', async () => { + const reserve = jest.fn().mockRejectedValueOnce(new Error('db down')).mockResolvedValue(7); + const setConvoLaneGit = jest.fn().mockResolvedValue(true); + const record = createLaneGitRecorder({ + user: 'u1', + conversationId: 'order-6', + reserveConvoLaneGitSeq: reserve, + setConvoLaneGit, + }); + await expect(record?.({ branch: 'a', head })).resolves.toBe(false); + await expect(record?.({ branch: 'b', head })).resolves.toBe(true); + expect(setConvoLaneGit.mock.calls[0][0]).toMatchObject({ seq: 7 }); + }); + + it('reports a write the database rejected as not applied, so a stale report is not remembered', async () => { + const setConvoLaneGit = jest.fn().mockResolvedValueOnce(false).mockResolvedValue(true); + const record = createLaneGitRecorder({ + user: 'u1', + conversationId: 'order-7', + reserveConvoLaneGitSeq: counter(), + setConvoLaneGit, + }); + await expect(record?.(laneGit)).resolves.toBe(false); + await expect(record?.(laneGit)).resolves.toBe(true); + expect(setConvoLaneGit).toHaveBeenCalledTimes(2); + }); + + it('does not skip a report that repeats the last one, since another writer may have changed the lane', async () => { + const reserve = counter(); + const setConvoLaneGit = jest.fn().mockResolvedValue(true); + const record = createLaneGitRecorder({ + user: 'u1', + conversationId: 'order-8', + reserveConvoLaneGitSeq: reserve, + setConvoLaneGit, + }); + await record?.(laneGit); + await record?.(laneGit); + expect(reserve).toHaveBeenCalledTimes(2); + expect(setConvoLaneGit).toHaveBeenCalledTimes(2); + }); + + it('lets a recorder restore its state after another recorder changed the same conversation', async () => { + const written: string[] = []; + const setConvoLaneGit = jest.fn( + async ({ laneGit: reported }: { laneGit: { branch: string | null } }) => { + written.push(String(reported.branch)); + return true; + }, + ); + const reserve = counter(); + const make = () => + createLaneGitRecorder({ + user: 'u1', + conversationId: 'order-shared', + reserveConvoLaneGitSeq: reserve, + setConvoLaneGit, + }); + const first = make(); + const second = make(); + await first?.({ branch: 'x', head }); + await second?.({ branch: 'y', head }); + await first?.({ branch: 'x', head }); + expect(written).toEqual(['x', 'y', 'x']); + }); + + it('writes again once the state changes, including back to an earlier one', async () => { + const setConvoLaneGit = jest.fn().mockResolvedValue(true); + const record = createLaneGitRecorder({ + user: 'u1', + conversationId: 'order-9', + reserveConvoLaneGitSeq: counter(), + setConvoLaneGit, + }); + await record?.({ branch: 'a', head }); + await record?.({ branch: 'b', head }); + await record?.({ branch: 'a', head }); + expect(setConvoLaneGit.mock.calls.map(([call]) => call.laneGit.branch)).toEqual([ + 'a', + 'b', + 'a', + ]); + }); + + it('orders reservations by the visible conversation two subagent threads share', async () => { + const releases: Array<() => void> = []; + const reserve = jest.fn( + () => new Promise((resolve) => releases.push(() => resolve(releases.length))), + ); + const getConvoLaneContext = jest + .fn() + .mockResolvedValue({ subagentThread: { rootConversationId: 'shared-root' } }); + const make = (conversationId: string) => + createLaneGitRecorder({ + user: 'u1', + conversationId, + workspace: { environmentId: 'code-mac', workspaceId: 'primary' }, + getConvoLaneContext, + reserveConvoLaneGitSeq: reserve, + setConvoLaneGit: jest.fn().mockResolvedValue(true), + }); + void make('child-a')?.({ branch: 'a', head }); + void make('child-b')?.({ branch: 'b', head }); + await flush(); + expect(reserve).toHaveBeenCalledTimes(1); + releases[0](); + await flush(); + expect(reserve).toHaveBeenCalledTimes(2); + releases[1](); + }); + + it('does not make threads of different visible conversations wait for each other', async () => { + const releases: Array<() => void> = []; + const reserve = jest.fn( + () => new Promise((resolve) => releases.push(() => resolve(1))), + ); + const getConvoLaneContext = jest.fn(async (_user: string, id: string) => ({ + subagentThread: { rootConversationId: id === 'child-a' ? 'root-a' : 'root-b' }, + })); + const make = (conversationId: string) => + createLaneGitRecorder({ + user: 'u1', + conversationId, + workspace: { environmentId: 'code-mac', workspaceId: 'primary' }, + getConvoLaneContext, + reserveConvoLaneGitSeq: reserve, + setConvoLaneGit: jest.fn().mockResolvedValue(true), + }); + void make('child-a')?.({ branch: 'a', head }); + void make('child-b')?.({ branch: 'b', head }); + await flush(); + expect(reserve).toHaveBeenCalledTimes(2); + releases.forEach((release) => release()); + }); +}); + +describe('createLaneGitRecorder target conversation', () => { + const workspace = { environmentId: 'code-mac', workspaceId: 'primary' }; + const build = (overrides: Record = {}) => { + const setConvoLaneGit = jest.fn().mockResolvedValue(true); + const reserveConvoLaneGitSeq = counter(); + const record = createLaneGitRecorder({ + user: 'u1', + conversationId: 'target-1', + workspace, + reserveConvoLaneGitSeq, + setConvoLaneGit, + ...overrides, + }); + return { record, setConvoLaneGit, reserveConvoLaneGitSeq }; + }; + const written = (setConvoLaneGit: jest.Mock) => setConvoLaneGit.mock.calls.map(([call]) => call); + + it('passes the workspace it ran in, so a stale writer can be fenced', async () => { + const { record, setConvoLaneGit } = build(); + await record(laneGit); + expect(written(setConvoLaneGit)[0]).toMatchObject({ + conversationId: 'target-1', + workspace: { ...workspace }, + }); + expect(written(setConvoLaneGit)[0].workspace.required).not.toBe(true); + }); + + it('records a subagent thread on the visible conversation, and requires its workspace to match', async () => { + const getConvoLaneContext = jest.fn().mockResolvedValue({ + subagentThread: { rootConversationId: 'visible-root', parentConversationId: 'mid' }, + codeAttachmentEpoch: 0, + }); + const { record, setConvoLaneGit, reserveConvoLaneGitSeq } = build({ + conversationId: 'child-thread', + getConvoLaneContext, + }); + await record(laneGit); + expect(getConvoLaneContext).toHaveBeenCalledWith('u1', 'child-thread'); + expect(reserveConvoLaneGitSeq).toHaveBeenCalledWith('u1', 'visible-root'); + expect(written(setConvoLaneGit)[0]).toMatchObject({ + conversationId: 'visible-root', + workspace: { ...workspace, required: true }, + }); + }); + + it('records an ordinary conversation on itself', async () => { + const getConvoLaneContext = jest.fn().mockResolvedValue({ codeAttachmentEpoch: 0 }); + const { record, setConvoLaneGit } = build({ conversationId: 'plain', getConvoLaneContext }); + await record(laneGit); + expect(written(setConvoLaneGit)[0].conversationId).toBe('plain'); + }); + + it('looks the conversation up once however many reports follow', async () => { + const getConvoLaneContext = jest.fn().mockResolvedValue({ + subagentThread: { rootConversationId: 'visible-root' }, + codeAttachmentEpoch: 0, + }); + const { record, setConvoLaneGit } = build({ conversationId: 'child-2', getConvoLaneContext }); + await record({ branch: 'a', head }); + await record({ branch: 'b', head }); + expect(getConvoLaneContext).toHaveBeenCalledTimes(1); + expect(written(setConvoLaneGit).map((call) => call.conversationId)).toEqual([ + 'visible-root', + 'visible-root', + ]); + }); + + it('records nothing when it cannot place the conversation, rather than guess', async () => { + const getConvoLaneContext = jest + .fn() + .mockRejectedValue(new Error('mongodb://user:secret@host')); + const setConvoLaneGit = jest.fn(); + const record = await createRecorder({ + enabled: true, + user: 'u1', + conversationId: 'child-3', + workspace, + getConvoLaneContext, + reserveConvoLaneGitSeq: counter(), + setConvoLaneGit, + }); + expect(record).toBeUndefined(); + expect(setConvoLaneGit).not.toHaveBeenCalled(); + expect(JSON.stringify((logger.warn as jest.Mock).mock.calls)).not.toContain('secret'); + }); + + it('does not write a subagent lane to the parent without a workspace to verify it against', async () => { + const getConvoLaneContext = jest.fn().mockResolvedValue({ + subagentThread: { rootConversationId: 'visible-root' }, + codeAttachmentEpoch: 0, + }); + const setConvoLaneGit = jest.fn(); + const record = await createRecorder({ + enabled: true, + user: 'u1', + conversationId: 'child-4', + getConvoLaneContext, + reserveConvoLaneGitSeq: counter(), + setConvoLaneGit, + }); + expect(record).toBeUndefined(); + expect(setConvoLaneGit).not.toHaveBeenCalled(); + }); + + it('places a conversation that is not saved yet on itself at epoch 0', async () => { + const getConvoLaneContext = jest.fn().mockResolvedValue(null); + const { record, setConvoLaneGit } = build({ getConvoLaneContext }); + await record(laneGit); + expect(written(setConvoLaneGit)[0]).toMatchObject({ + conversationId: 'target-1', + workspace: { ...workspace, epoch: 0 }, + }); + }); +}); + +describe('createLaneGitRecorder placement timing', () => { + const flush = () => new Promise((resolve) => setImmediate(resolve)); + const workspace = { environmentId: 'code-mac', workspaceId: 'primary' }; + + it('settles placement before any report, so a slow lookup cannot reorder reports', async () => { + let release: () => void = () => undefined; + const getConvoLaneContext = jest.fn( + () => + new Promise<{ codeAttachmentEpoch: number }>((resolve) => { + release = () => resolve({ codeAttachmentEpoch: 0 }); + }), + ); + const reserve = counter(); + const pending = createRecorder({ + enabled: true, + user: 'u1', + conversationId: 'slow-lookup', + workspace, + getConvoLaneContext, + reserveConvoLaneGitSeq: reserve, + setConvoLaneGit: jest.fn().mockResolvedValue(true), + }); + await flush(); + expect(reserve).not.toHaveBeenCalled(); + release(); + const record = await pending; + expect(record).toBeDefined(); + await record?.({ branch: 'a', head }); + expect(reserve).toHaveBeenCalledTimes(1); + }); + + it('carries the epoch read at creation into every write, even if the chat moves later', async () => { + const getConvoLaneContext = jest.fn().mockResolvedValue({ codeAttachmentEpoch: 3 }); + const setConvoLaneGit = jest.fn().mockResolvedValue(true); + const record = await createRecorder({ + enabled: true, + user: 'u1', + conversationId: 'epoch-1', + workspace, + getConvoLaneContext, + reserveConvoLaneGitSeq: counter(), + setConvoLaneGit, + }); + await record?.({ branch: 'a', head }); + await record?.({ branch: 'b', head }); + expect(setConvoLaneGit.mock.calls.map(([call]) => call.workspace.epoch)).toEqual([3, 3]); + expect(getConvoLaneContext).toHaveBeenCalledTimes(1); + }); +}); + +describe('createLaneGitRecorder when pull requests are off', () => { + it('reads and records nothing', async () => { + const getConvoLaneContext = jest.fn(); + const reserveConvoLaneGitSeq = counter(); + const setConvoLaneGit = jest.fn(); + const record = await createRecorder({ + enabled: false, + user: 'u1', + conversationId: 'off-1', + workspace: { environmentId: 'code-mac', workspaceId: 'primary' }, + getConvoLaneContext, + reserveConvoLaneGitSeq, + setConvoLaneGit, + }); + expect(record).toBeUndefined(); + expect(getConvoLaneContext).not.toHaveBeenCalled(); + expect(reserveConvoLaneGitSeq).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/api/src/code/lane.ts b/packages/api/src/code/lane.ts new file mode 100644 index 00000000000..61368ecb27b --- /dev/null +++ b/packages/api/src/code/lane.ts @@ -0,0 +1,168 @@ +import { logger } from '@librechat/data-schemas'; +import type { WorkspaceLaneGit } from './workspace'; +import { getSafeErrorMetadata } from '~/utils'; + +/** + * The workspace a command ran in. `required` demands the conversation have it recorded, and + * `epoch` is the attachment epoch read when the tool was created. + */ +export type LaneWorkspace = { + environmentId: string; + workspaceId: string; + required?: boolean; + epoch?: number; +}; + +export type LaneGitWriter = (input: { + user: string; + conversationId: string; + laneGit: WorkspaceLaneGit; + repo?: string; + /** Reserved when the command settled; the database ignores a report below the one it holds. */ + seq: number; + /** The database ignores the report once the conversation is no longer on this workspace. */ + workspace?: LaneWorkspace; +}) => Promise; + +/** Draws the next report sequence number from the database, so every replica shares one order. */ +export type LaneSeqReserver = (user: string, conversationId: string) => Promise; + +/** + * Reads what the recorder needs to place and fence its reports: the subagent route of the + * conversation and its workspace attachment epoch. Null when the conversation is not saved yet. + */ +export type LaneContextReader = ( + user: string, + conversationId: string, +) => Promise<{ + subagentThread?: { rootConversationId?: string | null } | null; + codeAttachmentEpoch?: number; +} | null>; + +const REPO_PATTERN = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/; +const MAX_REPO_LENGTH = 256; + +const isSafeRepo = (repo: string | undefined): repo is string => + repo != null && + repo.length <= MAX_REPO_LENGTH && + REPO_PATTERN.test(repo) && + repo.split('/').every((segment) => segment !== '.' && segment !== '..'); + +type Placement = { conversationId: string; required: boolean; epoch: number }; +type Reservation = Placement & { seq: number }; + +/** + * The tail of each visible conversation's pending sequence reservations in this process, keyed by + * the conversation the write lands on (the root of a subagent thread, not the thread). Two + * threads of one conversation therefore share a chain, and numbers are drawn one at a time in + * report order, so one process never takes a lower number for a later report. Writes are not + * queued: each carries its number and the database ignores a lower one, so a slow or reordered + * write cannot replace a newer state, here or on another replica. Entries leave the map as soon + * as their chain drains. + */ +const reservationTails = new Map>(); + +/** + * Builds the recorder for one tool, placing it once, here, so no report waits on a lookup that + * could let a later report take an earlier turn. The conversation's route and attachment epoch + * are read when the tool is created: a subagent thread is a hidden child of the conversation the + * user sees, its commands run on the parent's machine, so its lane belongs to the visible root + * and that write must match the root's recorded workspace; the epoch fences out a report that + * outlives a move of the workspace, even one that moved away and back. A conversation not saved + * yet is placed on itself at epoch 0. When the context cannot be read nothing is recorded: the + * branch is a header affordance, so a database outage never fails or delays the tool call. + * Resolves to undefined when there is nothing to record against. + */ +export async function createLaneGitRecorder({ + enabled, + user, + conversationId, + repo, + workspace, + getConvoLaneContext, + reserveConvoLaneGitSeq, + setConvoLaneGit, +}: { + /** Whether the deployment shows pull requests; when it does not, nothing is read or recorded. */ + enabled: boolean; + user: string | undefined; + conversationId: string | undefined; + repo?: string; + workspace?: { environmentId: string; workspaceId: string }; + getConvoLaneContext?: LaneContextReader; + reserveConvoLaneGitSeq: LaneSeqReserver; + setConvoLaneGit: LaneGitWriter; +}): Promise<((laneGit: WorkspaceLaneGit) => Promise) | undefined> { + if (!enabled || !user || !conversationId) return undefined; + const safeRepo = isSafeRepo(repo) ? repo : undefined; + + let placed: Placement; + try { + const context = getConvoLaneContext ? await getConvoLaneContext(user, conversationId) : null; + const root = context?.subagentThread?.rootConversationId; + placed = { + conversationId: root || conversationId, + required: Boolean(root), + epoch: context?.codeAttachmentEpoch ?? 0, + }; + } catch (error) { + logger.warn('[LaneGit] Failed to place a lane recorder', getSafeErrorMetadata(error)); + return undefined; + } + if (placed.required && workspace == null) return undefined; + const queueKey = `${user}\0${placed.conversationId}`; + + /** Never rejects, so a failed reservation cannot stall the ones queued behind it. */ + const reserve = async (): Promise => { + try { + const seq = await reserveConvoLaneGitSeq(user, placed.conversationId); + return seq == null ? null : { ...placed, seq }; + } catch (error) { + logger.warn('[LaneGit] Failed to reserve a lane report', getSafeErrorMetadata(error)); + return null; + } + }; + + const write = async (laneGit: WorkspaceLaneGit, reserved: Reservation): Promise => { + try { + return await setConvoLaneGit({ + user, + conversationId: reserved.conversationId, + laneGit, + ...(safeRepo ? { repo: safeRepo } : {}), + seq: reserved.seq, + ...(workspace + ? { + workspace: { + ...workspace, + ...(reserved.required ? { required: true } : {}), + epoch: reserved.epoch, + }, + } + : {}), + }); + } catch (error) { + logger.warn('[LaneGit] Failed to record lane state', getSafeErrorMetadata(error)); + return false; + } + }; + + /** + * Fire-and-forget for the command: it never throws and is never awaited. Each report takes its + * sequence number from the database in report order and is written with it, so the newest report + * wins whatever order the writes land in. Every report is written, repeats included, because + * another recorder (a sibling thread, another replica) may have changed the lane since this one + * last wrote, and the database already ignores a write that changes nothing the reader can see. + * Resolves to whether the write applied, which is false when a newer report is already stored + * and for a conversation not saved yet. A repo that is not a plain `owner/name` is dropped. + */ + return (laneGit) => { + const previous = reservationTails.get(queueKey) ?? Promise.resolve(); + const reservation = previous.then(() => reserve()); + reservationTails.set(queueKey, reservation); + void reservation.then(() => { + if (reservationTails.get(queueKey) === reservation) reservationTails.delete(queueKey); + }); + return reservation.then((reserved) => (reserved == null ? false : write(laneGit, reserved))); + }; +} diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index b5d05ebf3fa..54ba5cd1852 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -11,6 +11,7 @@ export * from './artifacts'; export * from './admin'; export * from './cdn'; export * from './code'; +export * from './pulls'; /* Auth */ export * from './auth'; /* API Keys */ diff --git a/packages/api/src/pulls/github.spec.ts b/packages/api/src/pulls/github.spec.ts new file mode 100644 index 00000000000..2351923b342 --- /dev/null +++ b/packages/api/src/pulls/github.spec.ts @@ -0,0 +1,679 @@ +import { createGitHubPullRequestSource, summarizeChecks } from './github'; +import { PullRequestSourceError } from './types'; + +const sha = 'a'.repeat(40); +const pull = (overrides: Record = {}) => ({ + number: 7, + title: 'Simplify single tool', + html_url: 'https://github.com/o/r/pull/7', + state: 'open', + merged: false, + draft: false, + additions: 12, + deletions: 3, + mergeable: true, + head: { sha }, + ...overrides, +}); +const listed = (state = 'open') => [{ number: 7, state, head: { sha } }]; +const json = (body: unknown, init?: ResponseInit) => Response.json(body, init); + +function sourceFor(routes: Record Response>) { + const fetchFn = jest.fn(async (input: string) => { + const url = String(input); + const key = Object.keys(routes).find((fragment) => url.includes(fragment)); + /** Unless a test says otherwise, no pull request carries the recorded commit. */ + if (key == null && /\/commits\/[a-f0-9]+\/pulls\?/.test(url)) return json([]); + if (key == null) throw new Error(`unexpected ${url}`); + return routes[key](); + }); + return { fetchFn, source: createGitHubPullRequestSource({ fetchFn }) }; +} + +const find = (source: ReturnType) => + source.find({ repo: 'o/r', branch: 'feat/x', token: 't' }); + +describe('summarizeChecks', () => { + it.each([ + ['none', []], + ['passing', [{ status: 'completed', conclusion: 'success' }]], + ['passing', [{ status: 'completed', conclusion: 'skipped' }]], + ['running', [{ status: 'in_progress', conclusion: null }]], + ['failing', [{ status: 'completed', conclusion: 'failure' }]], + [ + 'failing', + [ + { status: 'in_progress', conclusion: null }, + { status: 'completed', conclusion: 'timed_out' }, + ], + ], + ])('reports %s', (expected, runs) => { + expect(summarizeChecks(runs)).toBe(expected); + }); +}); + +describe('createGitHubPullRequestSource', () => { + const routes = (pullBody = pull(), checks: unknown = { check_runs: [] }) => ({ + '/pulls?state=open': () => json(listed()), + '/pulls/7': () => json(pullBody), + '/check-runs': () => json(checks), + }); + + it('maps an open, mergeable pull request with passing checks', async () => { + const { source } = sourceFor( + routes(pull(), { check_runs: [{ status: 'completed', conclusion: 'success' }] }), + ); + await expect(find(source)).resolves.toEqual({ + number: 7, + title: 'Simplify single tool', + url: 'https://github.com/o/r/pull/7', + additions: 12, + deletions: 3, + state: 'open', + isDraft: false, + mergeable: 'clean', + checks: 'passing', + }); + }); + + it('reports conflicts only while the pull request is open', async () => { + const open = sourceFor(routes(pull({ mergeable: false }))); + await expect(find(open.source)).resolves.toMatchObject({ mergeable: 'conflicting' }); + const merged = sourceFor(routes(pull({ state: 'closed', merged: true, mergeable: false }))); + await expect(find(merged.source)).resolves.toMatchObject({ + state: 'merged', + mergeable: 'unknown', + }); + }); + + it('keeps mergeable unknown while GitHub is still computing it', async () => { + const { source } = sourceFor(routes(pull({ mergeable: null }))); + await expect(find(source)).resolves.toMatchObject({ mergeable: 'unknown' }); + }); + + it('carries draft state', async () => { + const { source } = sourceFor(routes(pull({ draft: true }))); + await expect(find(source)).resolves.toMatchObject({ isDraft: true }); + }); + + it('asks for the open pull request first, so closed history cannot hide it', async () => { + const { source, fetchFn } = sourceFor({ + '/pulls?state=open': () => json(listed()), + '/pulls/7': () => json(pull()), + '/check-runs': () => json({ check_runs: [] }), + }); + await expect(find(source)).resolves.toMatchObject({ number: 7 }); + const urls = fetchFn.mock.calls.map(([url]) => String(url)); + expect(urls[0]).toContain('state=open'); + expect(urls.some((url) => url.includes('state=closed'))).toBe(false); + }); + + it('falls back to the most recently updated closed pull request when none is open', async () => { + const { source } = sourceFor({ + '/pulls?state=open': () => json([]), + '/pulls?state=closed': () => json([{ number: 9, state: 'closed', head: { sha } }]), + '/pulls/9': () => json(pull({ number: 9, state: 'closed', merged: true })), + '/check-runs': () => json({ check_runs: [] }), + }); + await expect(find(source)).resolves.toMatchObject({ number: 9, state: 'merged' }); + }); + + it('returns null when the branch has no pull request or the repository is not visible', async () => { + const none = sourceFor({ + '/pulls?state=open': () => json([]), + '/pulls?state=closed': () => json([]), + }); + await expect(find(none.source)).resolves.toBeNull(); + const hidden = sourceFor({ '/pulls?state=open': () => new Response('', { status: 404 }) }); + await expect(find(hidden.source)).resolves.toBeNull(); + expect(hidden.fetchFn).toHaveBeenCalledTimes(1); + }); + + it('queries only a plain owner/name and never contacts GitHub otherwise', async () => { + const { source, fetchFn } = sourceFor({}); + await expect(source.find({ repo: 'o/r', branch: '', token: 't' })).resolves.toBeNull(); + for (const repo of ['../x', 'o/..', './x', 'o/.', 'a b/c', 'o', 'o/r/extra']) { + await expect(source.find({ repo, branch: 'main', token: 't' })).resolves.toBeNull(); + } + expect(fetchFn).not.toHaveBeenCalled(); + }); + + it('encodes the branch in the head filter', async () => { + const { source, fetchFn } = sourceFor({ + '/pulls?state=open': () => json([]), + '/pulls?state=closed': () => json([]), + }); + await source.find({ repo: 'o/r', branch: 'feat/a&b=c', token: 't' }); + const url = String(fetchFn.mock.calls[0][0]); + expect(url).toContain('head=o%3Afeat%2Fa%26b%3Dc'); + }); + + it.each([ + ['a 429', new Response('', { status: 429 })], + [ + 'a 403 with no quota left', + new Response('', { status: 403, headers: { 'x-ratelimit-remaining': '0' } }), + ], + ])('maps %s to RATE_LIMITED', async (_label, response) => { + const { source } = sourceFor({ '/pulls?state=open': () => response }); + await expect(find(source)).rejects.toMatchObject({ code: 'RATE_LIMITED' }); + }); + + it.each([ + ['a 500', () => new Response('boom', { status: 500 })], + ['a plain 403', () => new Response('', { status: 403 })], + ['a malformed list', () => json({ not: 'a list' })], + ])('maps %s to UPSTREAM_ERROR', async (_label, respond) => { + const { source } = sourceFor({ '/pulls?state=open': respond }); + await expect(find(source)).rejects.toBeInstanceOf(PullRequestSourceError); + await expect(find(source)).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' }); + }); + + it('maps a network failure to UPSTREAM_ERROR without keeping its text', async () => { + const source = createGitHubPullRequestSource({ + fetchFn: jest.fn().mockRejectedValue(new Error('connect ECONNREFUSED token=secret')), + }); + const error = await find(source).catch((caught: Error) => caught); + expect(error).toMatchObject({ code: 'UPSTREAM_ERROR' }); + expect(String((error as Error).message)).not.toContain('secret'); + }); + + it.each([ + ['a non-github host', 'https://evil.example/o/r/pull/7'], + ['a javascript url', 'javascript:alert(1)'], + ['plain http', 'http://github.com/o/r/pull/7'], + ])('rejects %s as the pull request page', async (_label, html_url) => { + const { source } = sourceFor(routes(pull({ html_url }))); + await expect(find(source)).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' }); + }); + + it('bounds the title', async () => { + const { source } = sourceFor(routes(pull({ title: 'x'.repeat(1000) }))); + const result = await find(source); + expect(result?.title).toHaveLength(256); + }); +}); + +describe('check runs across pages', () => { + const run = (conclusion: string | null, status = 'completed') => ({ status, conclusion }); + const passing = (count: number) => Array.from({ length: count }, () => run('success')); + + function sourceWithPages(pages: unknown[][], totalCount: number) { + const fetchFn = jest.fn(async (input: string) => { + const url = String(input); + if (url.includes('/pulls?state=open')) return json(listed()); + if (url.includes('/pulls/7')) return json(pull()); + if (url.includes('/check-runs')) { + const page = Number(new URL(url).searchParams.get('page') ?? '1'); + return json({ total_count: totalCount, check_runs: pages[page - 1] ?? [] }); + } + throw new Error(`unexpected ${url}`); + }); + return { fetchFn, source: createGitHubPullRequestSource({ fetchFn }) }; + } + + it('reads every page, so a failure past the first hundred still turns the rollup red', async () => { + const { source } = sourceWithPages([passing(100), [run('failure')]], 101); + await expect(find(source)).resolves.toMatchObject({ checks: 'failing' }); + }); + + it('reads every page, so a check still running past the first hundred is not called passing', async () => { + const { source } = sourceWithPages([passing(100), [run(null, 'in_progress')]], 101); + await expect(find(source)).resolves.toMatchObject({ checks: 'running' }); + }); + + it('stops after a bounded number of pages and never calls an incomplete rollup passing', async () => { + const pages = Array.from({ length: 60 }, () => passing(100)); + const { source, fetchFn } = sourceWithPages(pages, 6000); + await expect(find(source)).resolves.toMatchObject({ checks: 'running' }); + const checkCalls = fetchFn.mock.calls.filter(([url]) => String(url).includes('/check-runs')); + expect(checkCalls.length).toBeLessThanOrEqual(10); + }); + + it('makes one request when every check run fits on the first page', async () => { + const { source, fetchFn } = sourceWithPages([passing(3)], 3); + await expect(find(source)).resolves.toMatchObject({ checks: 'passing' }); + const checkCalls = fetchFn.mock.calls.filter(([url]) => String(url).includes('/check-runs')); + expect(checkCalls).toHaveLength(1); + }); +}); + +describe('rate limit back-off hint', () => { + it('carries retry-after as milliseconds', async () => { + const { source } = sourceFor({ + '/pulls?state=open': () => + new Response('', { status: 429, headers: { 'retry-after': '30' } }), + }); + await expect(find(source)).rejects.toMatchObject({ + code: 'RATE_LIMITED', + retryAfterMs: 30_000, + }); + }); + + it('derives the wait from the quota reset when there is no retry-after', async () => { + const reset = Math.floor(Date.now() / 1000) + 120; + const { source } = sourceFor({ + '/pulls?state=open': () => + new Response('', { + status: 403, + headers: { 'x-ratelimit-remaining': '0', 'x-ratelimit-reset': String(reset) }, + }), + }); + const error = (await find(source).catch((caught: unknown) => caught)) as { + code: string; + retryAfterMs?: number; + }; + expect(error.code).toBe('RATE_LIMITED'); + expect(error.retryAfterMs).toBeGreaterThan(100_000); + expect(error.retryAfterMs).toBeLessThanOrEqual(120_000); + }); + + it('leaves the hint out when GitHub gives none', async () => { + const { source } = sourceFor({ '/pulls?state=open': () => new Response('', { status: 429 }) }); + const error = (await find(source).catch((caught: unknown) => caught)) as { + retryAfterMs?: number; + }; + expect(error.retryAfterMs).toBeUndefined(); + }); +}); + +describe('lookup bounds', () => { + const run = (conclusion: string | null, status = 'completed') => ({ status, conclusion }); + const everyRoute = (url: string, checks: () => Response) => { + if (url.includes('/pulls?state=open')) return json(listed()); + if (url.includes('/pulls/7')) return json(pull()); + if (url.includes('/check-runs')) return checks(); + throw new Error(`unexpected ${url}`); + }; + const aborts = (init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => reject(new Error('aborted'))); + }); + + it('stops reading check runs at the configured page limit and calls the rollup incomplete', async () => { + const fetchFn = jest.fn(async (url: string) => + everyRoute(url, () => + json({ total_count: 6000, check_runs: Array.from({ length: 100 }, () => run('success')) }), + ), + ); + const source = createGitHubPullRequestSource({ fetchFn }); + const result = await source.find({ + repo: 'o/r', + branch: 'feat/x', + token: 't', + limits: { maxCheckRunPages: 2 }, + }); + expect(result).toMatchObject({ checks: 'running' }); + expect(fetchFn.mock.calls.filter(([url]) => String(url).includes('/check-runs'))).toHaveLength( + 2, + ); + }); + + it('gives up on one request that outlives the configured request timeout', async () => { + const fetchFn = jest.fn((_url: string, init?: RequestInit) => aborts(init)); + const source = createGitHubPullRequestSource({ fetchFn }); + const started = Date.now(); + await expect( + source.find({ + repo: 'o/r', + branch: 'feat/x', + token: 't', + limits: { requestTimeoutMs: 20, lookupTimeoutMs: 5_000 }, + }), + ).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' }); + expect(Date.now() - started).toBeLessThan(2_000); + }); + + it('bounds the whole lookup, not only each request', async () => { + let calls = 0; + const fetchFn = jest.fn((url: string, init?: RequestInit) => { + calls += 1; + if (calls === 1 && url.includes('/pulls?state=open')) return Promise.resolve(json(listed())); + return aborts(init); + }); + const source = createGitHubPullRequestSource({ fetchFn }); + const started = Date.now(); + await expect( + source.find({ + repo: 'o/r', + branch: 'feat/x', + token: 't', + limits: { requestTimeoutMs: 10_000, lookupTimeoutMs: 30 }, + }), + ).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' }); + expect(Date.now() - started).toBeLessThan(2_000); + }); + + it("keeps today's defaults when no limits are given", async () => { + const seen: Array = []; + const fetchFn = jest.fn(async (url: string, init?: RequestInit) => { + seen.push(init?.signal); + return everyRoute(url, () => json({ total_count: 0, check_runs: [] })); + }); + const source = createGitHubPullRequestSource({ fetchFn }); + await expect(find(source)).resolves.toMatchObject({ number: 7 }); + expect(seen.every((signal) => signal != null && !signal.aborted)).toBe(true); + }); +}); + +describe('matching the recorded head', () => { + const recorded = 'c'.repeat(40); + const compare = (status: string) => () => json({ status }); + const findWith = ( + source: ReturnType, + head?: string | null, + ) => source.find({ repo: 'o/r', branch: 'feat/x', head, token: 't' }); + + const routes = (status: string) => ({ + '/pulls?state=open': () => json(listed()), + '/compare/': compare(status), + '/pulls/7': () => json(pull()), + '/check-runs': () => json({ total_count: 0, check_runs: [] }), + }); + + it('accepts the pull request whose head is the recorded commit, without a comparison', async () => { + const { source, fetchFn } = sourceFor({ + '/pulls?state=open': () => json([{ number: 7, state: 'open', head: { sha: recorded } }]), + '/pulls/7': () => json(pull({ head: { sha: recorded } })), + '/check-runs': () => json({ total_count: 0, check_runs: [] }), + }); + await expect(findWith(source, recorded)).resolves.toMatchObject({ number: 7 }); + expect(fetchFn.mock.calls.some(([url]) => String(url).includes('/compare/'))).toBe(false); + }); + + it.each(['ahead', 'identical'])( + 'accepts a pull request that is, or builds on, the recorded commit (%s)', + async (status) => { + const { source } = sourceFor(routes(status)); + await expect(findWith(source, recorded)).resolves.toMatchObject({ number: 7 }); + }, + ); + + it('rejects a pull request whose head is older than the recorded commit (behind)', async () => { + const { source } = sourceFor({ ...routes('behind'), '/pulls?state=closed': () => json([]) }); + await expect(findWith(source, recorded)).resolves.toBeNull(); + }); + + it('asks GitHub to compare the recorded commit to the candidate, in that order', async () => { + const { source, fetchFn } = sourceFor(routes('ahead')); + await findWith(source, recorded); + const url = String( + fetchFn.mock.calls.map(([u]) => String(u)).find((u) => u.includes('/compare/')), + ); + expect(url).toContain(`/compare/${recorded}...${sha}`); + }); + + it('rejects a pull request whose commits diverged from the recorded head', async () => { + const { source } = sourceFor({ ...routes('diverged'), '/pulls?state=closed': () => json([]) }); + await expect(findWith(source, recorded)).resolves.toBeNull(); + }); + + it('treats a recorded commit GitHub no longer knows as no match', async () => { + const { source } = sourceFor({ + ...routes('ahead'), + '/compare/': () => new Response('', { status: 404 }), + '/pulls?state=closed': () => json([]), + }); + await expect(findWith(source, recorded)).resolves.toBeNull(); + }); + + it('falls back to a closed pull request that matches when the open one does not', async () => { + const { source } = sourceFor({ + '/pulls?state=open': () => json([{ number: 9, state: 'open', head: { sha } }]), + '/pulls?state=closed': () => json([{ number: 7, state: 'closed', head: { sha: recorded } }]), + '/compare/': compare('diverged'), + '/pulls/7': () => json(pull({ state: 'closed', merged: true, head: { sha: recorded } })), + '/check-runs': () => json({ total_count: 0, check_runs: [] }), + }); + await expect(findWith(source, recorded)).resolves.toMatchObject({ + number: 7, + state: 'merged', + }); + }); + + it('compares as many candidates as configured, no more', async () => { + const many = Array.from({ length: 10 }, (_, index) => ({ + number: index + 1, + state: 'open', + head: { sha }, + })); + const { source, fetchFn } = sourceFor({ + '/pulls?state=open': () => json(many), + '/pulls?state=closed': () => json(many), + '/compare/': compare('diverged'), + }); + await expect( + source.find({ + repo: 'o/r', + branch: 'feat/x', + head: recorded, + token: 't', + limits: { maxHeadComparisons: 5 }, + }), + ).resolves.toBeNull(); + expect(fetchFn.mock.calls.filter(([u]) => String(u).includes('/compare/'))).toHaveLength(5); + }); + + it('lists as many candidate pull requests as configured', async () => { + const { source, fetchFn } = sourceFor({ + '/pulls?state=open': () => json([]), + '/pulls?state=closed': () => json([]), + }); + await source.find({ + repo: 'o/r', + branch: 'feat/x', + head: recorded, + token: 't', + limits: { maxCandidatePullRequests: 40 }, + }); + const urls = fetchFn.mock.calls.map(([u]) => String(u)); + expect(urls.every((u) => u.includes('per_page=40'))).toBe(true); + }); + + it('limits how many candidates it compares', async () => { + const many = Array.from({ length: 10 }, (_, index) => ({ + number: index + 1, + state: 'open', + head: { sha }, + })); + const { source, fetchFn } = sourceFor({ + '/pulls?state=open': () => json(many), + '/pulls?state=closed': () => json(many), + '/compare/': compare('diverged'), + }); + await expect(findWith(source, recorded)).resolves.toBeNull(); + const compares = fetchFn.mock.calls.filter(([url]) => String(url).includes('/compare/')); + expect(compares.length).toBeLessThanOrEqual(3); + }); + + it.each([null, undefined])('does not compare when no head was recorded (%s)', async (head) => { + const { source, fetchFn } = sourceFor(routes('diverged')); + await expect(findWith(source, head)).resolves.toMatchObject({ number: 7 }); + expect(fetchFn.mock.calls.some(([url]) => String(url).includes('/compare/'))).toBe(false); + }); + + it('ignores a recorded head that is not a commit id rather than putting it in a path', async () => { + const { source, fetchFn } = sourceFor(routes('diverged')); + await expect(findWith(source, '../../etc')).resolves.toMatchObject({ number: 7 }); + expect(fetchFn.mock.calls.some(([url]) => String(url).includes('/compare/'))).toBe(false); + }); +}); + +describe('check runs follow the current head of the pull request', () => { + const newer = 'd'.repeat(40); + + it('reads checks for the head the detail reports, not the one the list saw', async () => { + const { source, fetchFn } = sourceFor({ + '/pulls?state=open': () => json(listed()), + '/pulls/7': () => json(pull({ head: { sha: newer } })), + '/check-runs': () => json({ total_count: 0, check_runs: [] }), + }); + await expect(find(source)).resolves.toMatchObject({ number: 7 }); + const urls = fetchFn.mock.calls.map(([url]) => String(url)); + expect(urls.some((url) => url.includes(`/commits/${newer}/check-runs`))).toBe(true); + expect(urls.some((url) => url.includes(`/commits/${sha}/check-runs`))).toBe(false); + }); + + it('reads the detail before the checks, so they describe the same commit', async () => { + const { source, fetchFn } = sourceFor({ + '/pulls?state=open': () => json(listed()), + '/pulls/7': () => json(pull()), + '/check-runs': () => json({ total_count: 0, check_runs: [] }), + }); + await find(source); + const urls = fetchFn.mock.calls.map(([url]) => String(url)); + expect(urls.findIndex((url) => url.includes('/pulls/7'))).toBeLessThan( + urls.findIndex((url) => url.includes('/check-runs')), + ); + }); + + it.each([ + ['a missing head', { head: undefined }], + ['a head without a sha', { head: {} }], + ['a head that is not a commit id', { head: { sha: '../../x' } }], + ])('rejects a pull request detail with %s', async (_label, overrides) => { + const { source, fetchFn } = sourceFor({ + '/pulls?state=open': () => json(listed()), + '/pulls/7': () => json(pull(overrides)), + '/check-runs': () => json({ total_count: 0, check_runs: [] }), + }); + await expect(find(source)).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' }); + expect(fetchFn.mock.calls.some(([url]) => String(url).includes('/check-runs'))).toBe(false); + }); +}); + +describe('a head that moves after the candidate was listed', () => { + const recorded = 'c'.repeat(40); + const moved = 'd'.repeat(40); + const base = { + '/pulls?state=open': () => json([{ number: 7, state: 'open', head: { sha: recorded } }]), + '/check-runs': () => json({ total_count: 0, check_runs: [] }), + }; + const run = (source: ReturnType) => + source.find({ repo: 'o/r', branch: 'feat/x', head: recorded, token: 't' }); + + it('returns nothing when the detail head no longer contains the recorded commit', async () => { + const { source } = sourceFor({ + ...base, + '/compare/': () => json({ status: 'diverged' }), + '/pulls/7': () => json(pull({ head: { sha: moved } })), + }); + await expect(run(source)).resolves.toBeNull(); + }); + + it('keeps the pull request when the moved head still builds on the recorded commit', async () => { + const { source, fetchFn } = sourceFor({ + ...base, + '/compare/': () => json({ status: 'ahead' }), + '/pulls/7': () => json(pull({ head: { sha: moved } })), + }); + await expect(run(source)).resolves.toMatchObject({ number: 7 }); + expect( + fetchFn.mock.calls.some(([url]) => String(url).includes(`/commits/${moved}/check-runs`)), + ).toBe(true); + }); + + it('spends the comparison budget on the moved head too, and stops when it is spent', async () => { + const { source, fetchFn } = sourceFor({ + ...base, + '/compare/': () => json({ status: 'ahead' }), + '/pulls/7': () => json(pull({ head: { sha: moved } })), + }); + await expect( + source.find({ + repo: 'o/r', + branch: 'feat/x', + head: recorded, + token: 't', + limits: { maxHeadComparisons: 0 }, + }), + ).resolves.toBeNull(); + expect(fetchFn.mock.calls.some(([url]) => String(url).includes('/compare/'))).toBe(false); + }); + + it('does not compare again when the detail head is the one that matched', async () => { + const { source, fetchFn } = sourceFor({ + ...base, + '/pulls/7': () => json(pull({ head: { sha: recorded } })), + }); + await expect(run(source)).resolves.toMatchObject({ number: 7 }); + expect(fetchFn.mock.calls.some(([url]) => String(url).includes('/compare/'))).toBe(false); + }); +}); + +describe('a pull request whose head is in a fork', () => { + const recorded = 'c'.repeat(40); + const forked = (overrides: Record = {}) => ({ + number: 12, + state: 'open', + head: { sha: recorded, ref: 'feat/x' }, + ...overrides, + }); + const routes = (found: unknown[], status = 200) => ({ + '/pulls?state=open': () => json([]), + '/pulls?state=closed': () => json([]), + '/pulls?per_page': () => json(found, { status }), + '/pulls/12': () => json(pull({ number: 12, head: { sha: recorded } })), + }); + const run = (source: ReturnType) => + source.find({ repo: 'o/r', branch: 'feat/x', head: recorded, token: 't' }); + + it('finds it through the recorded commit when the owner-scoped lists are empty', async () => { + const { source, fetchFn } = sourceFor({ + ...routes([forked()]), + '/check-runs': () => json({ total_count: 0, check_runs: [] }), + }); + await expect(run(source)).resolves.toMatchObject({ number: 12 }); + expect( + fetchFn.mock.calls.some(([url]) => String(url).includes(`/commits/${recorded}/pulls`)), + ).toBe(true); + }); + + it('ignores a pull request for the commit that is on another branch', async () => { + const { source } = sourceFor(routes([forked({ head: { sha: recorded, ref: 'other' } })])); + await expect(run(source)).resolves.toBeNull(); + }); + + it('treats a commit GitHub does not know as no pull request', async () => { + const { source } = sourceFor(routes([], 422)); + await expect(run(source)).resolves.toBeNull(); + }); + + it('does not ask without a recorded commit', async () => { + const { source, fetchFn } = sourceFor(routes([forked()])); + await expect(source.find({ repo: 'o/r', branch: 'feat/x', token: 't' })).resolves.toBeNull(); + expect(fetchFn.mock.calls.some(([url]) => String(url).includes('/commits/'))).toBe(false); + }); +}); + +describe('a secondary rate limit without limiting headers', () => { + const secondary = () => + new Response( + JSON.stringify({ message: 'You have exceeded a secondary rate limit. Please wait.' }), + { status: 403 }, + ); + + it('is a rate limit that waits at least a minute', async () => { + const { source } = sourceFor({ '/pulls?state=open': secondary }); + const error = await find(source).catch((caught) => caught); + expect(error.code).toBe('RATE_LIMITED'); + expect(error.retryAfterMs).toBeGreaterThanOrEqual(60_000); + }); + + it('leaves an ordinary 403 as an upstream error', async () => { + const { source } = sourceFor({ + '/pulls?state=open': () => + new Response(JSON.stringify({ message: 'Resource not accessible by token' }), { + status: 403, + }), + }); + await expect(find(source)).rejects.toMatchObject({ code: 'UPSTREAM_ERROR' }); + }); + + it('does not put the response text in the error', async () => { + const { source } = sourceFor({ '/pulls?state=open': secondary }); + const error = await find(source).catch((caught) => caught); + expect(JSON.stringify(error)).not.toContain('secondary rate limit'); + expect(String(error.message)).not.toContain('secondary'); + }); +}); diff --git a/packages/api/src/pulls/github.ts b/packages/api/src/pulls/github.ts new file mode 100644 index 00000000000..2b273c0b9af --- /dev/null +++ b/packages/api/src/pulls/github.ts @@ -0,0 +1,410 @@ +import type { + PullRequestChecks, + PullRequestMergeable, + PullRequestState, + TConversationPullRequest, +} from 'librechat-data-provider'; +import type { PullRequestSource } from './types'; +import { PullRequestSourceError } from './types'; + +const GITHUB_API_BASE = 'https://api.github.com'; +/** Defaults for the operator bounds in `endpoints.agents.pullRequests`. */ +const DEFAULT_REQUEST_TIMEOUT_MS = 10_000; +const DEFAULT_LOOKUP_TIMEOUT_MS = 30_000; +const DEFAULT_MAX_CHECK_RUN_PAGES = 10; +const CHECK_RUN_PAGE_SIZE = 100; +const DEFAULT_MAX_CANDIDATE_PULL_REQUESTS = 10; +const DEFAULT_MAX_HEAD_COMPARISONS = 3; +/** + * Comparing `recorded...candidate`: `ahead` means the candidate builds on the recorded commit and + * `identical` that it is the recorded commit. `behind` means the candidate does not contain it + * (the recorded commit is newer), and `diverged` that the histories split, so neither matches. + */ +const CONTAINS_RECORDED_STATUSES = new Set(['ahead', 'identical']); +const COMMIT_ID = /^(?:[a-f0-9]{40}|[a-f0-9]{64})$/; +const MAX_TITLE_LENGTH = 256; +const REPO_PATTERN = /^([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+)$/; +const FAILING_CONCLUSIONS = new Set([ + 'failure', + 'timed_out', + 'cancelled', + 'action_required', + 'startup_failure', + 'stale', +]); + +/** The only part of `fetch` this module calls, so any compatible client can be injected. */ +export type PullRequestFetch = (input: string, init?: RequestInit) => Promise; + +type GitHubPull = { + number: number; + title: string; + url: string; + state: 'open' | 'closed'; + merged: boolean; + draft: boolean; + additions: number; + deletions: number; + mergeable: boolean | null; + /** The head this detail describes; the check runs are read for this same commit. */ + headSha: string; +}; + +type GitHubCheckRun = { status: string; conclusion: string | null }; + +const isRecord = (value: unknown): value is Record => + value != null && typeof value === 'object' && !Array.isArray(value); + +const isCount = (value: unknown): value is number => + typeof value === 'number' && Number.isSafeInteger(value) && value >= 0; + +/** The URL ends up in an href, so only a plain https github.com page is accepted. */ +function isGitHubPageUrl(value: unknown): value is string { + if (typeof value !== 'string') return false; + try { + const url = new URL(value); + return url.protocol === 'https:' && url.hostname === 'github.com'; + } catch { + return false; + } +} + +function parsePull(value: unknown): GitHubPull { + if ( + !isRecord(value) || + !isCount(value.number) || + typeof value.title !== 'string' || + !isGitHubPageUrl(value.html_url) || + (value.state !== 'open' && value.state !== 'closed') || + typeof value.merged !== 'boolean' || + typeof value.draft !== 'boolean' || + !isCount(value.additions) || + !isCount(value.deletions) || + (value.mergeable !== null && typeof value.mergeable !== 'boolean') || + !isRecord(value.head) || + typeof value.head.sha !== 'string' || + !COMMIT_ID.test(value.head.sha) + ) { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + return { + number: value.number, + title: value.title, + url: value.html_url, + state: value.state, + merged: value.merged, + draft: value.draft, + additions: value.additions, + deletions: value.deletions, + mergeable: value.mergeable, + headSha: value.head.sha, + }; +} + +type ListItem = { number: number; state: string; sha: string }; + +function parseListItem(value: unknown): ListItem { + if ( + !isRecord(value) || + !isCount(value.number) || + typeof value.state !== 'string' || + !isRecord(value.head) || + typeof value.head.sha !== 'string' || + !/^[a-f0-9]{40,64}$/.test(value.head.sha) + ) { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + return { number: value.number, state: value.state, sha: value.head.sha }; +} + +type CheckRunPage = { runs: GitHubCheckRun[]; total?: number }; + +function parseCheckRuns(value: unknown): CheckRunPage { + if (!isRecord(value) || !Array.isArray(value.check_runs)) { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + const runs = value.check_runs.map((run) => { + if (!isRecord(run) || typeof run.status !== 'string') { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + return { + status: run.status, + conclusion: typeof run.conclusion === 'string' ? run.conclusion : null, + }; + }); + return { runs, ...(isCount(value.total_count) ? { total: value.total_count } : {}) }; +} + +/** + * A failed check outranks one still running, so the header turns red as soon as it is known. + * An `incomplete` rollup (more runs than were read) is never reported as passing. + */ +export function summarizeChecks( + runs: readonly GitHubCheckRun[], + incomplete = false, +): PullRequestChecks { + if (runs.length === 0 && !incomplete) return 'none'; + if (runs.some((run) => run.conclusion != null && FAILING_CONCLUSIONS.has(run.conclusion))) { + return 'failing'; + } + if (incomplete || runs.some((run) => run.status !== 'completed')) return 'running'; + return 'passing'; +} + +function summarizeState(pull: GitHubPull): PullRequestState { + return pull.merged ? 'merged' : pull.state; +} + +/** Conflict state only means something while the pull request is open. */ +function summarizeMergeable(pull: GitHubPull): PullRequestMergeable { + if (summarizeState(pull) !== 'open' || pull.mergeable == null) return 'unknown'; + return pull.mergeable ? 'clean' : 'conflicting'; +} + +export function toConversationPullRequest( + pull: GitHubPull, + runs: readonly GitHubCheckRun[], + incomplete = false, +): TConversationPullRequest { + return { + number: pull.number, + title: pull.title.slice(0, MAX_TITLE_LENGTH), + url: pull.url, + additions: pull.additions, + deletions: pull.deletions, + state: summarizeState(pull), + isDraft: pull.draft, + mergeable: summarizeMergeable(pull), + checks: summarizeChecks(runs, incomplete), + }; +} + +/** GitHub asks for at least a minute when a secondary limit names no wait. */ +const SECONDARY_LIMIT_WAIT_MS = 60_000; +const SECONDARY_LIMIT_MESSAGE = /secondary rate limit|abuse detection/i; +const MAX_ERROR_BODY_CHARS = 2_000; + +function isRateLimited(response: Response): boolean { + return ( + response.status === 429 || + (response.status === 403 && + (response.headers.get('x-ratelimit-remaining') === '0' || + response.headers.get('retry-after') != null)) + ); +} + +/** + * A secondary limit can arrive as a 403 with neither limiting header, told apart only by its + * message. The body is read bounded and only matched, never stored or returned. + */ +async function isSecondaryLimit(response: Response): Promise { + if (response.status !== 403) return false; + try { + const text = (await response.text()).slice(0, MAX_ERROR_BODY_CHARS); + return SECONDARY_LIMIT_MESSAGE.test(text); + } catch { + return false; + } +} + +/** How long GitHub asked callers to wait: `retry-after` seconds, else the quota reset time. */ +function retryAfterMs(response: Response): number | undefined { + const retryAfter = Number(response.headers.get('retry-after')); + if (response.headers.get('retry-after') != null && Number.isFinite(retryAfter)) { + return Math.max(0, retryAfter * 1000); + } + const reset = Number(response.headers.get('x-ratelimit-reset')); + if (response.headers.get('x-ratelimit-reset') != null && Number.isFinite(reset)) { + return Math.max(0, reset * 1000 - Date.now()); + } + return undefined; +} + +/** + * Reads pull requests through GitHub's REST API with a token the caller supplies. Only the + * decision (absent, rate limited, failed) leaves this module; response bodies and error text + * never do. + */ +/** `.` and `..` match the character class but would rewrite the API path. */ +const isPathSegment = (value: string): boolean => value !== '.' && value !== '..'; + +export function createGitHubPullRequestSource({ + fetchFn, + apiBase = GITHUB_API_BASE, +}: { + /** The deployment's HTTP client, so proxying and instrumentation are decided by the caller. */ + fetchFn: PullRequestFetch; + apiBase?: string; +}): PullRequestSource { + /** One lookup's credential and deadlines, shared by every request it makes. */ + type Lookup = { + token: string; + requestTimeoutMs: number; + /** Aborts when the whole lookup runs out of time. */ + deadline: AbortSignal; + maxCheckRunPages: number; + maxCandidatePullRequests: number; + maxHeadComparisons: number; + }; + + async function getJson( + pathname: string, + lookup: Lookup, + /** Statuses that mean "nothing there" for this request, besides 404. */ + absent: readonly number[] = [], + ): Promise { + let response: Response; + try { + response = await fetchFn(`${apiBase}${pathname}`, { + headers: { + Accept: 'application/vnd.github+json', + Authorization: `Bearer ${lookup.token}`, + 'X-GitHub-Api-Version': '2022-11-28', + 'User-Agent': 'LibreChat-Pull-Requests', + }, + signal: AbortSignal.any([AbortSignal.timeout(lookup.requestTimeoutMs), lookup.deadline]), + }); + } catch { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + if (response.ok) { + try { + return await response.json(); + } catch { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + } + if (isRateLimited(response)) { + throw new PullRequestSourceError('RATE_LIMITED', retryAfterMs(response)); + } + if (await isSecondaryLimit(response)) { + throw new PullRequestSourceError('RATE_LIMITED', SECONDARY_LIMIT_WAIT_MS); + } + /** A repository the token cannot see is indistinguishable from one without pull requests. */ + if (response.status === 404 || absent.includes(response.status)) return null; + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + + /** Reads every page up to a bound, so a failure on a later page still shows. */ + async function readCheckRuns( + pathname: string, + lookup: Lookup, + ): Promise<{ runs: GitHubCheckRun[]; incomplete: boolean }> { + const runs: GitHubCheckRun[] = []; + for (let page = 1; page <= lookup.maxCheckRunPages; page++) { + const body = await getJson( + `${pathname}?per_page=${CHECK_RUN_PAGE_SIZE}&page=${page}`, + lookup, + ); + if (body == null) return { runs, incomplete: false }; + const parsed = parseCheckRuns(body); + runs.push(...parsed.runs); + const done = + parsed.total != null + ? runs.length >= parsed.total || parsed.runs.length === 0 + : parsed.runs.length < CHECK_RUN_PAGE_SIZE; + if (done) return { runs, incomplete: false }; + } + return { runs, incomplete: true }; + } + + return { + async find({ repo, branch, head: recordedHead, token, limits }) { + const lookup: Lookup = { + token, + requestTimeoutMs: limits?.requestTimeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS, + deadline: AbortSignal.timeout(limits?.lookupTimeoutMs ?? DEFAULT_LOOKUP_TIMEOUT_MS), + maxCheckRunPages: limits?.maxCheckRunPages ?? DEFAULT_MAX_CHECK_RUN_PAGES, + maxCandidatePullRequests: + limits?.maxCandidatePullRequests ?? DEFAULT_MAX_CANDIDATE_PULL_REQUESTS, + maxHeadComparisons: limits?.maxHeadComparisons ?? DEFAULT_MAX_HEAD_COMPARISONS, + }; + const match = REPO_PATTERN.exec(repo); + if (match == null || branch.length === 0) return null; + const [, owner, name] = match; + if (!isPathSegment(owner) || !isPathSegment(name)) return null; + const base = `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}`; + const headFilter = encodeURIComponent(`${owner}:${branch}`); + /** `undefined`: the repository is not visible. An empty list: it is, and nothing matched. */ + const listPulls = async (state: 'open' | 'closed', perPage: number) => { + const listed = await getJson( + `${base}/pulls?state=${state}&sort=updated&direction=desc&per_page=${perPage}&head=${headFilter}`, + lookup, + ); + if (listed == null) return undefined; + if (!Array.isArray(listed)) throw new PullRequestSourceError('UPSTREAM_ERROR'); + return listed.map(parseListItem); + }; + const recorded = + typeof recordedHead === 'string' && COMMIT_ID.test(recordedHead) ? recordedHead : undefined; + let comparisons = 0; + /** + * A branch name can be deleted and reused, so with a recorded commit a pull request counts + * only when its head is that commit or builds on it. A commit GitHub no longer knows is no + * match, and both the candidates listed and the comparisons made are bounded by config. + */ + /** Every comparison, the first match and the revalidation of a moved head alike, spends the + * same configured budget. */ + const contains = async (headSha: string): Promise => { + if (recorded == null || headSha === recorded) return true; + if (comparisons >= lookup.maxHeadComparisons) return false; + comparisons += 1; + const comparison = await getJson(`${base}/compare/${recorded}...${headSha}`, lookup); + if (comparison == null) return false; + if (!isRecord(comparison) || typeof comparison.status !== 'string') { + throw new PullRequestSourceError('UPSTREAM_ERROR'); + } + return CONTAINS_RECORDED_STATUSES.has(comparison.status); + }; + const matches = (candidate: ListItem): Promise => contains(candidate.sha); + const choose = async (candidates: ListItem[]): Promise => { + for (const candidate of candidates) { + if (await matches(candidate)) return candidate; + } + return null; + }; + /** Open first on its own, so closed history on a reused branch name cannot hide it. */ + const open = await listPulls('open', lookup.maxCandidatePullRequests); + if (open === undefined) return null; + /** + * The head filter names the repository's own owner, so a pull request from a fork is not in + * these lists. The recorded commit finds it instead: GitHub lists the pull requests that + * carry a commit whatever fork they come from, and the branch name narrows them. + */ + const forkCandidates = async (): Promise => { + if (recorded == null) return []; + const found = await getJson( + `${base}/commits/${recorded}/pulls?per_page=${lookup.maxCandidatePullRequests}`, + lookup, + [422], + ); + if (found == null) return []; + if (!Array.isArray(found)) throw new PullRequestSourceError('UPSTREAM_ERROR'); + return found + .filter((item) => isRecord(item) && isRecord(item.head) && item.head.ref === branch) + .map(parseListItem) + .sort((a, b) => Number(a.state !== 'open') - Number(b.state !== 'open')); + }; + const chosen = + (await choose(open)) ?? + (await choose( + (await listPulls('closed', recorded == null ? 1 : lookup.maxCandidatePullRequests)) ?? [], + )) ?? + (await choose(await forkCandidates())); + if (chosen == null) return null; + + /** + * The detail comes first and its head decides which commit's checks are read. Listing, + * detail and checks are separate requests, so a push between them could otherwise pair + * today's line counts and mergeability with yesterday's checks. + */ + const detail = await getJson(`${base}/pulls/${chosen.number}`, lookup); + if (detail == null) return null; + const pull = parsePull(detail); + /** A push between the list and the detail can move the head off the recorded commit. */ + if (pull.headSha !== chosen.sha && !(await contains(pull.headSha))) return null; + const checks = await readCheckRuns(`${base}/commits/${pull.headSha}/check-runs`, lookup); + return toConversationPullRequest(pull, checks.runs, checks.incomplete); + }, + }; +} diff --git a/packages/api/src/pulls/handler.spec.ts b/packages/api/src/pulls/handler.spec.ts new file mode 100644 index 00000000000..ec862de7b40 --- /dev/null +++ b/packages/api/src/pulls/handler.spec.ts @@ -0,0 +1,283 @@ +import type { TConversationPullRequest } from 'librechat-data-provider'; +import type { Response } from 'express'; +import type { ServerRequest } from '~/types'; +import { createConversationPullRequestHandler, resolveTokenReference } from './handler'; + +jest.mock('@librechat/data-schemas', () => ({ + ...jest.requireActual('@librechat/data-schemas'), + logger: { warn: jest.fn(), debug: jest.fn(), info: jest.fn(), error: jest.fn() }, +})); + +const pr: TConversationPullRequest = { + number: 7, + title: 't', + url: 'https://github.com/o/r/pull/7', + additions: 1, + deletions: 2, + state: 'open', + isDraft: false, + mergeable: 'clean', + checks: 'passing', +}; + +const enabled = { + enabled: true, + token: '${GH_TOKEN}', + cacheTtlSeconds: 30, + allowedRepositories: ['o/r'], +}; +const configWith = (pullRequests?: Record) => ({ + endpoints: { agents: { pullRequests } }, +}); + +type Lane = { branch: string | null; head: string | null; repo?: string } | null; + +function setup( + options: { + settings?: Record | null; + laneGit?: Lane; + lookup?: jest.Mock; + env?: Record; + } = {}, +) { + /** `null` means the block is absent; omitting the option means the enabled default. */ + const settings = options.settings === undefined ? enabled : (options.settings ?? undefined); + const laneGit: Lane = + options.laneGit === undefined ? { branch: 'feat/x', head: null, repo: 'o/r' } : options.laneGit; + const lookup = options.lookup ?? jest.fn().mockResolvedValue({ ok: true, value: pr }); + const env = options.env ?? { GH_TOKEN: 'ghp_secret' }; + const getConvoLaneGit = jest.fn().mockResolvedValue(laneGit); + const handler = createConversationPullRequestHandler({ + getConvoLaneGit, + getAppConfig: jest.fn().mockResolvedValue(configWith(settings)), + lookup, + env, + }); + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + const run = (params: { conversationId?: string } = { conversationId: 'c1' }, user = 'u1') => + handler({ user: { id: user }, params } as unknown as ServerRequest, res as unknown as Response); + return { run, res, getConvoLaneGit, lookup }; +} + +describe('resolveTokenReference', () => { + it.each([ + ['${GH_TOKEN}', { GH_TOKEN: ' abc ' }, 'abc'], + ['${GH_TOKEN}', {}, null], + ['${GH_TOKEN}', { GH_TOKEN: ' ' }, null], + ['literal-token', { 'literal-token': 'x' }, null], + [undefined, { GH_TOKEN: 'x' }, null], + ])('resolves %s', (reference, env, expected) => { + expect(resolveTokenReference(reference, env)).toBe(expected); + }); +}); + +describe('createConversationPullRequestHandler', () => { + it('returns the pull request for the owner-scoped branch', async () => { + const { run, res, getConvoLaneGit, lookup } = setup(); + await run(); + expect(getConvoLaneGit).toHaveBeenCalledWith('u1', 'c1'); + expect(lookup).toHaveBeenCalledWith({ + repo: 'o/r', + branch: 'feat/x', + token: 'ghp_secret', + head: null, + ttlMs: 30_000, + cacheMaxEntries: 500, + cacheMaxCredentials: 256, + limits: { + requestTimeoutMs: 10_000, + lookupTimeoutMs: 30_000, + maxCheckRunPages: 10, + maxCandidatePullRequests: 10, + maxHeadComparisons: 3, + }, + }); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ pullRequest: pr }); + }); + + it('passes the configured lookup bounds to GitHub', async () => { + const { run, lookup } = setup({ + settings: { + ...enabled, + requestTimeoutSeconds: 3, + lookupTimeoutSeconds: 8, + maxCheckRunPages: 4, + maxCandidatePullRequests: 25, + maxHeadComparisons: 6, + cacheMaxEntries: 77, + cacheMaxCredentials: 12, + }, + }); + await run(); + expect(lookup).toHaveBeenCalledWith( + expect.objectContaining({ + cacheMaxEntries: 77, + cacheMaxCredentials: 12, + limits: { + requestTimeoutMs: 3_000, + lookupTimeoutMs: 8_000, + maxCheckRunPages: 4, + maxCandidatePullRequests: 25, + maxHeadComparisons: 6, + }, + }), + ); + }); + + it('passes the recorded head so the pull request can be matched to what the chat ran', async () => { + const head = 'a'.repeat(40); + const { run, lookup } = setup({ laneGit: { branch: 'feat/x', head, repo: 'o/r' } }); + await run(); + expect(lookup).toHaveBeenCalledWith(expect.objectContaining({ head })); + }); + + describe('repository allowlist', () => { + const lane = (repo: string) => ({ branch: 'feat/x', head: null, repo }); + + it.each([ + ['an exact match', ['o/r'], 'o/r'], + ['a different case', ['O/R'], 'o/r'], + ['an owner wildcard', ['o/*'], 'o/r'], + ['one of several', ['x/y', 'o/r'], 'o/r'], + ])('looks up a repository allowed by %s', async (_label, allowedRepositories, repo) => { + const { run, lookup } = setup({ + settings: { ...enabled, allowedRepositories }, + laneGit: lane(repo), + }); + await run(); + expect(lookup).toHaveBeenCalledTimes(1); + }); + + it.each([ + ['another repository', ['o/r'], 'o/other'], + ['another owner', ['o/*'], 'x/r'], + ['a prefix of an allowed owner', ['org/*'], 'organization/r'], + ['an empty allowlist', [], 'o/r'], + ])('answers null without using the token for %s', async (_label, allowedRepositories, repo) => { + const { run, res, lookup } = setup({ + settings: { ...enabled, allowedRepositories }, + laneGit: lane(repo), + }); + await run(); + expect(lookup).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ pullRequest: null }); + }); + + it('does not reveal whether a disallowed repository exists', async () => { + const allowed = setup({ laneGit: lane('o/r') }); + const denied = setup({ laneGit: lane('secret/private') }); + await denied.run(); + await allowed.run(); + expect(denied.res.json).toHaveBeenCalledWith({ pullRequest: null }); + expect(JSON.stringify(denied.res.json.mock.calls)).not.toContain('secret'); + }); + }); + + it('answers null, and never touches GitHub, when the feature is off', async () => { + for (const settings of [null, { enabled: false }]) { + const { run, res, lookup } = setup({ settings }); + await run(); + expect(lookup).not.toHaveBeenCalled(); + expect(res.json).toHaveBeenCalledWith({ pullRequest: null }); + } + }); + + it('starts the config and the owner-scoped lane reads together, not one after the other', async () => { + const order: string[] = []; + let releaseConfig: (config: unknown) => void = () => undefined; + const getAppConfig = jest.fn( + () => + new Promise((resolve) => { + order.push('config:start'); + releaseConfig = resolve; + }), + ); + const getConvoLaneGit = jest.fn(async () => { + order.push('lane:start'); + return { branch: 'feat/x', head: null, repo: 'o/r' }; + }); + const handler = createConversationPullRequestHandler({ + getConvoLaneGit, + getAppConfig: getAppConfig as never, + lookup: jest.fn().mockResolvedValue({ ok: true, value: pr }), + env: { GH_TOKEN: 'ghp_secret' }, + }); + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + const done = handler( + { user: { id: 'u1' }, params: { conversationId: 'c1' } } as unknown as ServerRequest, + res as unknown as Response, + ); + await new Promise((resolve) => setImmediate(resolve)); + expect(order).toEqual(['config:start', 'lane:start']); + releaseConfig(configWith(enabled)); + await done; + expect(res.json).toHaveBeenCalledWith({ pullRequest: pr }); + }); + + it.each([ + ['no stored lane', null], + ['a detached lane', { branch: null, head: null, repo: 'o/r' }], + ['a lane with no repository', { branch: 'main', head: null }], + ])('answers null for %s without calling GitHub', async (_label, laneGit) => { + const { run, res, lookup } = setup({ laneGit }); + await run(); + expect(lookup).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ pullRequest: null }); + }); + + it('answers null when the branch has no pull request', async () => { + const { run, res } = setup({ lookup: jest.fn().mockResolvedValue({ ok: true, value: null }) }); + await run(); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ pullRequest: null }); + }); + + it('is a 404 for a missing or oversized conversation id', async () => { + for (const conversationId of [undefined, '', ' ', 'x'.repeat(257)]) { + const { run, res, getConvoLaneGit } = setup(); + await run({ conversationId }); + expect(res.status).toHaveBeenCalledWith(404); + expect(getConvoLaneGit).not.toHaveBeenCalled(); + } + }); + + it('reports a missing token as NOT_CONFIGURED without leaking config', async () => { + const { run, res, lookup } = setup({ env: {} }); + await run(); + expect(lookup).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(503); + expect(res.json).toHaveBeenCalledWith({ + error: 'Pull requests are not configured', + code: 'NOT_CONFIGURED', + }); + }); + + it.each(['RATE_LIMITED', 'UPSTREAM_ERROR'])( + 'maps a %s failure to a 503 with its code', + async (code) => { + const { run, res } = setup({ + lookup: jest.fn().mockResolvedValue({ ok: false, error: { code } }), + }); + await run(); + expect(res.status).toHaveBeenCalledWith(503); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ code })); + }, + ); + + it('answers 500 with fixed text when storage throws, never the exception', async () => { + const { run, res, getConvoLaneGit } = setup(); + getConvoLaneGit.mockRejectedValue(new Error('mongodb://user:secret@host')); + await run(); + expect(res.status).toHaveBeenCalledWith(500); + expect(JSON.stringify(res.json.mock.calls)).not.toContain('secret'); + }); + + it('never puts the token in a response', async () => { + const { run, res } = setup(); + await run(); + expect(JSON.stringify(res.json.mock.calls)).not.toContain('ghp_secret'); + }); +}); diff --git a/packages/api/src/pulls/handler.ts b/packages/api/src/pulls/handler.ts new file mode 100644 index 00000000000..42a72097836 --- /dev/null +++ b/packages/api/src/pulls/handler.ts @@ -0,0 +1,129 @@ +import { logger } from '@librechat/data-schemas'; +import { EModelEndpoint } from 'librechat-data-provider'; +import type { TAgentsEndpoint, TConversationPullRequestResponse } from 'librechat-data-provider'; +import type { AppConfig } from '@librechat/data-schemas'; +import type { Response } from 'express'; +import type { GetAppConfigOptions } from '~/app/service'; +import type { PullRequestLookup } from './types'; +import type { ServerRequest } from '~/types'; +import { getAppConfigOptionsFromUser } from '~/app/service'; +import { getSafeErrorMetadata } from '~/utils'; + +const MAX_CONVERSATION_ID_LENGTH = 256; +const TOKEN_REFERENCE = /^\$\{([A-Za-z_][A-Za-z0-9_]*)\}$/; + +export type ConversationLaneGit = { branch: string | null; head: string | null; repo?: string }; + +/** Resolves `${NAME}` against the environment; the config never holds the token itself. */ +export function resolveTokenReference( + reference: string | undefined, + env: Readonly>, +): string | null { + const name = reference == null ? undefined : TOKEN_REFERENCE.exec(reference)?.[1]; + if (name == null) return null; + return env[name]?.trim() || null; +} + +/** `owner/name` or `owner/*`, compared without case as GitHub does. Nothing else matches. */ +export function isAllowedRepository(repo: string, allowed: readonly string[] | undefined): boolean { + const [owner, name] = repo.toLowerCase().split('/'); + return (allowed ?? []).some((entry) => { + const [allowedOwner, allowedName] = entry.toLowerCase().split('/'); + return allowedOwner === owner && (allowedName === '*' || allowedName === name); + }); +} + +const validConversationId = (value: string | undefined): value is string => + value != null && value.trim() !== '' && value.length <= MAX_CONVERSATION_ID_LENGTH; + +const NONE: TConversationPullRequestResponse = { pullRequest: null }; + +/** + * Serves the pull request of the branch a conversation's code workspace last reported. The + * stored branch is read owner-scoped, so another user's conversation is indistinguishable from + * one without a pull request. Failures answer with a stable code and never with upstream text. + */ +export function createConversationPullRequestHandler(deps: { + getConvoLaneGit: (user: string, conversationId: string) => Promise; + getAppConfig: (options: GetAppConfigOptions) => Promise; + lookup: PullRequestLookup; + env: Readonly>; +}) { + return async (req: ServerRequest, res: Response): Promise => { + const userId = req.user?.id; + const { conversationId } = req.params as { conversationId?: string }; + if (!userId || !validConversationId(conversationId)) { + res.status(404).json({ error: 'Conversation not found' }); + return; + } + try { + /** + * Config and the owner-scoped lane are independent, so they load together. The lane is only + * used when the resolved config turns the feature on, which keeps a disabled deployment's + * answer the same as before. + */ + const [appConfig, laneGit] = await Promise.all([ + deps.getAppConfig({ + ...getAppConfigOptionsFromUser(req.user), + skipRuntimeAugmentation: true, + failClosed: true, + }), + deps.getConvoLaneGit(userId, conversationId), + ]); + const settings = (appConfig.endpoints?.[EModelEndpoint.agents] as TAgentsEndpoint | undefined) + ?.pullRequests; + if (settings?.enabled !== true) { + res.status(200).json(NONE); + return; + } + + if (laneGit?.branch == null || laneGit.repo == null) { + res.status(200).json(NONE); + return; + } + + /** The repository comes from the worker, so it is never used with the token unless the + * administrator named it. A repository that is not allowed looks like one without a pull + * request. */ + if (!isAllowedRepository(laneGit.repo, settings.allowedRepositories)) { + res.status(200).json(NONE); + return; + } + + const token = resolveTokenReference(settings.token, deps.env); + if (token == null) { + logger.warn('[PullRequests] Enabled without a usable token reference'); + res.status(503).json({ error: 'Pull requests are not configured', code: 'NOT_CONFIGURED' }); + return; + } + + const result = await deps.lookup({ + repo: laneGit.repo, + branch: laneGit.branch, + head: laneGit.head, + token, + ttlMs: (settings.cacheTtlSeconds ?? 30) * 1000, + cacheMaxEntries: settings.cacheMaxEntries ?? 500, + cacheMaxCredentials: settings.cacheMaxCredentials ?? 256, + limits: { + requestTimeoutMs: (settings.requestTimeoutSeconds ?? 10) * 1000, + lookupTimeoutMs: (settings.lookupTimeoutSeconds ?? 30) * 1000, + maxCheckRunPages: settings.maxCheckRunPages ?? 10, + maxCandidatePullRequests: settings.maxCandidatePullRequests ?? 10, + maxHeadComparisons: settings.maxHeadComparisons ?? 3, + }, + }); + if (!result.ok) { + res.status(503).json({ + error: 'Pull request lookup is unavailable', + code: result.error.code, + }); + return; + } + res.status(200).json({ pullRequest: result.value }); + } catch (error) { + logger.error('[PullRequests] Handler failed', getSafeErrorMetadata(error)); + res.status(500).json({ error: 'Failed to load the pull request' }); + } + }; +} diff --git a/packages/api/src/pulls/index.ts b/packages/api/src/pulls/index.ts new file mode 100644 index 00000000000..f6fca526708 --- /dev/null +++ b/packages/api/src/pulls/index.ts @@ -0,0 +1,4 @@ +export * from './types'; +export * from './github'; +export * from './lookup'; +export * from './handler'; diff --git a/packages/api/src/pulls/lookup.spec.ts b/packages/api/src/pulls/lookup.spec.ts new file mode 100644 index 00000000000..319d7c30ff9 --- /dev/null +++ b/packages/api/src/pulls/lookup.spec.ts @@ -0,0 +1,404 @@ +import type { TConversationPullRequest } from 'librechat-data-provider'; +import type { PullRequestSource } from './types'; +import { createPullRequestLookup } from './lookup'; +import { PullRequestSourceError } from './types'; + +const value: TConversationPullRequest = { + number: 1, + title: 't', + url: 'https://github.com/o/r/pull/1', + additions: 1, + deletions: 0, + state: 'open', + isDraft: false, + mergeable: 'clean', + checks: 'passing', +}; +const input = { repo: 'o/r', branch: 'main', token: 't', ttlMs: 30_000 }; + +describe('createPullRequestLookup', () => { + it('reuses a result within its lifetime and asks again after it', async () => { + let clock = 0; + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find }, now: () => clock }); + await expect(lookup(input)).resolves.toEqual({ ok: true, value }); + clock = 29_999; + await lookup(input); + expect(find).toHaveBeenCalledTimes(1); + clock = 30_001; + await lookup(input); + expect(find).toHaveBeenCalledTimes(2); + }); + + it('caches a documented absence', async () => { + const find = jest.fn().mockResolvedValue(null); + const lookup = createPullRequestLookup({ source: { find } }); + await expect(lookup(input)).resolves.toEqual({ ok: true, value: null }); + await lookup(input); + expect(find).toHaveBeenCalledTimes(1); + }); + + it('shares one request between concurrent callers', async () => { + let release: (pr: TConversationPullRequest) => void = () => undefined; + const find = jest.fn( + () => + new Promise((resolve) => { + release = resolve; + }), + ); + const lookup = createPullRequestLookup({ source: { find } }); + const first = lookup(input); + const second = lookup(input); + release(value); + await expect(Promise.all([first, second])).resolves.toEqual([ + { ok: true, value }, + { ok: true, value }, + ]); + expect(find).toHaveBeenCalledTimes(1); + }); + + it('keys by repository and branch', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + await lookup(input); + await lookup({ ...input, branch: 'other' }); + await lookup({ ...input, repo: 'o/other' }); + expect(find).toHaveBeenCalledTimes(3); + }); + + it('returns a failure as a coded result and retries it sooner than a success', async () => { + let clock = 0; + const find = jest + .fn() + .mockRejectedValueOnce(new PullRequestSourceError('RATE_LIMITED')) + .mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find }, now: () => clock }); + await expect(lookup(input)).resolves.toEqual({ + ok: false, + error: { code: 'RATE_LIMITED' }, + }); + clock = 5_000; + await expect(lookup(input)).resolves.toMatchObject({ ok: false }); + expect(find).toHaveBeenCalledTimes(1); + clock = 10_001; + await expect(lookup(input)).resolves.toEqual({ ok: true, value }); + }); + + it('turns an unexpected exception into UPSTREAM_ERROR without its text', async () => { + const source: PullRequestSource = { + find: jest.fn().mockRejectedValue(new Error('mongodb://user:secret@host')), + }; + const result = await createPullRequestLookup({ source })(input); + expect(result).toEqual({ ok: false, error: { code: 'UPSTREAM_ERROR' } }); + expect(JSON.stringify(result)).not.toContain('secret'); + }); + + it('evicts the oldest entry past its bound', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find }, maxEntries: 2 }); + await lookup({ ...input, branch: 'a' }); + await lookup({ ...input, branch: 'b' }); + await lookup({ ...input, branch: 'c' }); + await lookup({ ...input, branch: 'a' }); + expect(find).toHaveBeenCalledTimes(4); + }); +}); + +describe('credential scoping', () => { + it('never serves a result fetched with one credential to another', async () => { + const find = jest.fn(async ({ token }: { token: string }) => + token === 'tenant-a' ? value : null, + ); + const lookup = createPullRequestLookup({ source: { find } }); + await expect(lookup({ ...input, token: 'tenant-a' })).resolves.toEqual({ ok: true, value }); + await expect(lookup({ ...input, token: 'tenant-b' })).resolves.toEqual({ + ok: true, + value: null, + }); + expect(find).toHaveBeenCalledTimes(2); + }); + + it('still reuses a result for the same credential', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + await lookup({ ...input, token: 'tenant-a' }); + await lookup({ ...input, token: 'tenant-a' }); + expect(find).toHaveBeenCalledTimes(1); + }); + + it('does not share an in-flight request between credentials', async () => { + const releases: Array<() => void> = []; + const find = jest.fn( + ({ token }: { token: string }) => + new Promise((resolve) => { + releases.push(() => resolve(token === 'a' ? value : null)); + }), + ); + const lookup = createPullRequestLookup({ source: { find } }); + const a = lookup({ ...input, token: 'a' }); + const b = lookup({ ...input, token: 'b' }); + expect(find).toHaveBeenCalledTimes(2); + releases.forEach((release) => release()); + await expect(a).resolves.toEqual({ ok: true, value }); + await expect(b).resolves.toEqual({ ok: true, value: null }); + }); +}); + +describe('rate limit cooldown', () => { + const limited = (retryAfterMs?: number) => + new PullRequestSourceError('RATE_LIMITED', retryAfterMs); + const rateLimited = { ok: false, error: { code: 'RATE_LIMITED' } }; + + it('stops asking GitHub for every branch of the credential until the retry time', async () => { + let clock = 0; + const find = jest.fn().mockRejectedValueOnce(limited(60_000)).mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find }, now: () => clock }); + await expect(lookup(input)).resolves.toEqual(rateLimited); + clock = 30_000; + await expect(lookup({ ...input, branch: 'other' })).resolves.toEqual(rateLimited); + expect(find).toHaveBeenCalledTimes(1); + clock = 60_001; + await expect(lookup({ ...input, branch: 'other' })).resolves.toEqual({ ok: true, value }); + expect(find).toHaveBeenCalledTimes(2); + }); + + it('does not block a different credential', async () => { + const find = jest.fn().mockRejectedValueOnce(limited(60_000)).mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + await lookup({ ...input, token: 'a' }); + await expect(lookup({ ...input, token: 'b', branch: 'other' })).resolves.toEqual({ + ok: true, + value, + }); + }); + + it('waits at least a short floor when GitHub gives no retry time', async () => { + let clock = 0; + const find = jest.fn().mockRejectedValueOnce(limited()).mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find }, now: () => clock }); + await lookup(input); + clock = 9_999; + await expect(lookup({ ...input, branch: 'other' })).resolves.toEqual(rateLimited); + clock = 10_001; + await expect(lookup({ ...input, branch: 'other' })).resolves.toEqual({ ok: true, value }); + }); + + it('honors a reset GitHub reports well beyond ten minutes', async () => { + let clock = 0; + const find = jest + .fn() + .mockRejectedValueOnce(limited(40 * 60_000)) + .mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find }, now: () => clock }); + await lookup(input); + clock = 39 * 60_000; + await expect(lookup({ ...input, branch: 'other' })).resolves.toEqual(rateLimited); + expect(find).toHaveBeenCalledTimes(1); + clock = 40 * 60_000 + 1; + await expect(lookup({ ...input, branch: 'other' })).resolves.toEqual({ ok: true, value }); + }); + + it('caps a hint past the hour GitHub resets within, so one bad header cannot silence the feature for a day', async () => { + let clock = 0; + const find = jest + .fn() + .mockRejectedValueOnce(limited(24 * 60 * 60_000)) + .mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find }, now: () => clock }); + await lookup(input); + clock = 60 * 60_000 - 1; + await expect(lookup({ ...input, branch: 'other' })).resolves.toEqual(rateLimited); + clock = 60 * 60_000 + 1; + await expect(lookup({ ...input, branch: 'other' })).resolves.toEqual({ ok: true, value }); + }); + + it('keeps serving a cached result while the credential is cooling down', async () => { + let clock = 0; + const find = jest.fn().mockResolvedValueOnce(value).mockRejectedValueOnce(limited(60_000)); + const lookup = createPullRequestLookup({ source: { find }, now: () => clock }); + await lookup(input); + await lookup({ ...input, branch: 'other' }); + clock = 5_000; + await expect(lookup(input)).resolves.toEqual({ ok: true, value }); + expect(find).toHaveBeenCalledTimes(2); + }); +}); + +describe('lookup policy scoping', () => { + it('does not serve a result read under a tighter page limit to a caller allowing more', async () => { + const incomplete = { ...value, checks: 'running' as const }; + const find = jest.fn(async ({ limits }: { limits?: { maxCheckRunPages?: number } }) => + limits?.maxCheckRunPages === 1 ? incomplete : { ...value, checks: 'failing' as const }, + ); + const lookup = createPullRequestLookup({ source: { find } }); + await expect(lookup({ ...input, limits: { maxCheckRunPages: 1 } })).resolves.toEqual({ + ok: true, + value: incomplete, + }); + await expect(lookup({ ...input, limits: { maxCheckRunPages: 50 } })).resolves.toMatchObject({ + value: { checks: 'failing' }, + }); + expect(find).toHaveBeenCalledTimes(2); + }); + + it('honors the shorter freshness of a caller instead of the longer one another set', async () => { + let clock = 0; + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find }, now: () => clock }); + await lookup({ ...input, ttlMs: 3_600_000 }); + clock = 6_000; + await lookup({ ...input, ttlMs: 5_000 }); + expect(find).toHaveBeenCalledTimes(2); + }); + + it('does not share an in-flight request between different policies', async () => { + const find = jest.fn(() => new Promise(() => undefined)); + const lookup = createPullRequestLookup({ source: { find } }); + void lookup({ ...input, limits: { maxCheckRunPages: 1 } }); + void lookup({ ...input, limits: { maxCheckRunPages: 2 } }); + expect(find).toHaveBeenCalledTimes(2); + }); + + it('keys by the recorded head, so a reused branch name does not share a result', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + await lookup({ ...input, head: 'a'.repeat(40) }); + await lookup({ ...input, head: 'b'.repeat(40) }); + await lookup({ ...input, head: 'a'.repeat(40) }); + expect(find).toHaveBeenCalledTimes(2); + }); + + it('treats an unchanged policy as the same entry', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + await lookup({ ...input, limits: { maxCheckRunPages: 3, requestTimeoutMs: 5_000 } }); + await lookup({ ...input, limits: { requestTimeoutMs: 5_000, maxCheckRunPages: 3 } }); + expect(find).toHaveBeenCalledTimes(1); + }); +}); + +describe('lookup policy scoping of the history search', () => { + it.each([['maxCandidatePullRequests'], ['maxHeadComparisons']])( + 'does not share a result between callers with different %s', + async (field) => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + await lookup({ ...input, limits: { [field]: 3 } }); + await lookup({ ...input, limits: { [field]: 30 } }); + expect(find).toHaveBeenCalledTimes(2); + }, + ); +}); + +describe('cache capacity from the caller', () => { + const branches = ['a', 'b', 'c']; + const fill = async ( + lookup: ReturnType, + cacheMaxEntries?: number, + ) => { + for (const branch of branches) await lookup({ ...input, branch, cacheMaxEntries }); + await lookup({ ...input, branch: 'a', cacheMaxEntries }); + }; + + it('evicts fresh entries once the configured capacity is exceeded', async () => { + const find = jest.fn().mockResolvedValue(value); + await fill(createPullRequestLookup({ source: { find } }), 2); + expect(find).toHaveBeenCalledTimes(4); + }); + + it('keeps them while the configured capacity is large enough', async () => { + const find = jest.fn().mockResolvedValue(value); + await fill(createPullRequestLookup({ source: { find } }), 10); + expect(find).toHaveBeenCalledTimes(3); + }); + + it('falls back to its own default capacity when the caller gives none', async () => { + const find = jest.fn().mockResolvedValue(value); + await fill(createPullRequestLookup({ source: { find } })); + expect(find).toHaveBeenCalledTimes(3); + }); + + it('does not treat the capacity as part of the answer, so callers with different ones share an entry', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + await lookup({ ...input, cacheMaxEntries: 5 }); + await lookup({ ...input, cacheMaxEntries: 50 }); + expect(find).toHaveBeenCalledTimes(1); + }); +}); + +describe('cache capacity per credential', () => { + it("does not let one credential's small capacity evict another credential's entries", async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + await lookup({ ...input, token: 'big', branch: 'keep', cacheMaxEntries: 100 }); + for (const branch of ['a', 'b', 'c', 'd']) { + await lookup({ ...input, token: 'small', branch, cacheMaxEntries: 2 }); + } + const before = find.mock.calls.length; + await lookup({ ...input, token: 'big', branch: 'keep', cacheMaxEntries: 100 }); + expect(find).toHaveBeenCalledTimes(before); + }); + + it('bounds the credentials it keeps, dropping the one idle longest', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + await lookup({ ...input, token: 'first' }); + for (let index = 0; index < 256; index += 1) { + await lookup({ ...input, token: `other-${index}` }); + } + const before = find.mock.calls.length; + await lookup({ ...input, token: 'first' }); + expect(find).toHaveBeenCalledTimes(before + 1); + }); + + it('keeps more credentials when the caller configures a larger bound', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + const call = (token: string) => lookup({ ...input, token, cacheMaxCredentials: 300 }); + await call('first'); + for (let index = 0; index < 256; index += 1) await call(`other-${index}`); + const before = find.mock.calls.length; + await call('first'); + expect(find).toHaveBeenCalledTimes(before); + }); + + it('drops credentials past the configured bound', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + const call = (token: string) => lookup({ ...input, token, cacheMaxCredentials: 2 }); + await call('a'); + await call('b'); + await call('c'); + const before = find.mock.calls.length; + await call('a'); + expect(find).toHaveBeenCalledTimes(before + 1); + }); + + it('does not let a caller with a small bound evict the credentials of one with a large bound', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + for (const token of ['a', 'b', 'c']) { + await lookup({ ...input, token, cacheMaxCredentials: 100 }); + } + await lookup({ ...input, token: 'small', cacheMaxCredentials: 1 }); + const before = find.mock.calls.length; + for (const token of ['a', 'b', 'c']) { + await lookup({ ...input, token, cacheMaxCredentials: 100 }); + } + expect(find).toHaveBeenCalledTimes(before); + }); + + it('counts a cache hit as use, so an active credential is not the one dropped', async () => { + const find = jest.fn().mockResolvedValue(value); + const lookup = createPullRequestLookup({ source: { find } }); + const call = (token: string) => lookup({ ...input, token, cacheMaxCredentials: 2 }); + await call('active'); + await call('idle'); + await call('active'); + await call('newcomer'); + const before = find.mock.calls.length; + await call('active'); + expect(find).toHaveBeenCalledTimes(before); + }); +}); diff --git a/packages/api/src/pulls/lookup.ts b/packages/api/src/pulls/lookup.ts new file mode 100644 index 00000000000..c53fecd324a --- /dev/null +++ b/packages/api/src/pulls/lookup.ts @@ -0,0 +1,161 @@ +import { createHash } from 'node:crypto'; +import { logger } from '@librechat/data-schemas'; +import type { PullRequestLookup, PullRequestLookupResult, PullRequestSource } from './types'; +import { PullRequestSourceError } from './types'; +import { getSafeErrorMetadata } from '~/utils'; + +/** A failed lookup is remembered briefly so a failing upstream is not hammered by every poll. */ +const FAILURE_TTL_MS = 10_000; +/** Used when GitHub rate limits a credential without saying when to come back. */ +const MIN_COOLDOWN_MS = 10_000; +/** GitHub's primary limit resets within the hour; a larger hint is malformed and is capped. */ +const MAX_COOLDOWN_MS = 60 * 60_000; +const DEFAULT_MAX_ENTRIES = 500; +/** Credentials with a cache partition at once unless configured; the least recently used goes. */ +const DEFAULT_MAX_PARTITIONS = 256; + +type Entry = { result: PullRequestLookupResult; expiresAt: number }; + +/** Keys carry a digest, never the token, so no credential is held in a map key or a heap dump. */ +const scopeOf = (token: string): string => + createHash('sha256').update(token).digest('hex').slice(0, 32); + +/** + * Caches lookups per credential, repository, branch, recorded commit and lookup policy, and shares + * one in-flight request between concurrent callers with all of those in common. Each credential + * has its own bounded partition, sized by the capacity its callers ask for. What a token can see is part of the answer (a + * private repository is a pull request for one tenant and nothing for another), so the credential + * scopes both the cache and the in-flight map. A rate limit applies to the credential, not the + * branch, so it starts a cooldown for every branch under that credential. Callers must have + * authorized the conversation before asking. + */ +export function createPullRequestLookup({ + source, + now = Date.now, + maxEntries = DEFAULT_MAX_ENTRIES, +}: { + source: PullRequestSource; + now?: () => number; + maxEntries?: number; +}): PullRequestLookup { + /** One bounded partition per credential, so a capacity setting only ever evicts entries + * fetched with its own credential. */ + const partitions = new Map>(); + const inflight = new Map>(); + const cooldowns = new Map(); + const rateLimited: PullRequestLookupResult = { ok: false, error: { code: 'RATE_LIMITED' } }; + + /** + * The credential bound is one deployment-wide number. Callers may carry different configured + * values (per-principal config), and one asking for a small bound must not evict the credentials + * of another, so the largest value asked for so far applies. + */ + let credentialBound = 0; + + /** Recency of use orders the partitions, so the one idle longest is the one dropped. */ + function touch(scope: string): Map | undefined { + const partition = partitions.get(scope); + if (partition == null) return undefined; + partitions.delete(scope); + partitions.set(scope, partition); + return partition; + } + + function partitionOf(scope: string, credentials: number): Map { + credentialBound = Math.max(credentialBound, credentials); + const partition = touch(scope) ?? new Map(); + partitions.set(scope, partition); + while (partitions.size > credentialBound) { + const idle = partitions.keys().next(); + if (idle.done) break; + partitions.delete(idle.value); + } + return partition; + } + + function remember( + scope: string, + key: string, + result: PullRequestLookupResult, + ttlMs: number, + capacity: number, + credentials: number, + ): void { + const lifetime = result.ok ? ttlMs : Math.min(ttlMs, FAILURE_TTL_MS); + const entries = partitionOf(scope, credentials); + entries.delete(key); + entries.set(key, { result, expiresAt: now() + lifetime }); + while (entries.size > capacity) { + const oldest = entries.keys().next(); + if (oldest.done) break; + entries.delete(oldest.value); + } + } + + function startCooldown(scope: string, retryAfterMs?: number): void { + const wait = Math.min(MAX_COOLDOWN_MS, Math.max(MIN_COOLDOWN_MS, retryAfterMs ?? 0)); + cooldowns.set(scope, now() + wait); + for (const [other, until] of cooldowns) { + if (until <= now()) cooldowns.delete(other); + } + } + + return async ({ + repo, + branch, + head, + token, + ttlMs, + limits, + cacheMaxEntries, + cacheMaxCredentials, + }) => { + const scope = scopeOf(token); + /** Everything that changes the answer or how long it may be reused is part of the key. */ + const policy = [ + ttlMs, + limits?.requestTimeoutMs, + limits?.lookupTimeoutMs, + limits?.maxCheckRunPages, + limits?.maxCandidatePullRequests, + limits?.maxHeadComparisons, + ].join(','); + const key = `${scope}\0${repo}#${branch}\0${head ?? ''}\0${policy}`; + const cached = touch(scope)?.get(key); + if (cached != null && cached.expiresAt > now()) return cached.result; + const pending = inflight.get(key); + if (pending != null) return pending; + const until = cooldowns.get(scope); + if (until != null) { + if (until > now()) return rateLimited; + cooldowns.delete(scope); + } + + const run = (async (): Promise => { + let result: PullRequestLookupResult; + try { + result = { ok: true, value: await source.find({ repo, branch, head, token, limits }) }; + } catch (error) { + if (!(error instanceof PullRequestSourceError)) { + logger.warn('[PullRequests] Lookup failed', getSafeErrorMetadata(error)); + } + const code = error instanceof PullRequestSourceError ? error.code : 'UPSTREAM_ERROR'; + if (error instanceof PullRequestSourceError && error.code === 'RATE_LIMITED') { + startCooldown(scope, error.retryAfterMs); + } + result = { ok: false, error: { code } }; + } + remember( + scope, + key, + result, + ttlMs, + cacheMaxEntries ?? maxEntries, + cacheMaxCredentials ?? DEFAULT_MAX_PARTITIONS, + ); + return result; + })().finally(() => inflight.delete(key)); + inflight.set(key, run); + return run; + }; +} diff --git a/packages/api/src/pulls/types.ts b/packages/api/src/pulls/types.ts new file mode 100644 index 00000000000..e177e67bc6b --- /dev/null +++ b/packages/api/src/pulls/types.ts @@ -0,0 +1,64 @@ +import type { TConversationPullRequest } from 'librechat-data-provider'; + +/** Stable, user-safe reasons a lookup could not answer. None carries upstream text. */ +export const PULL_REQUEST_ERROR_CODES = [ + 'NOT_CONFIGURED', + 'RATE_LIMITED', + 'UPSTREAM_ERROR', +] as const; +export type PullRequestErrorCode = (typeof PULL_REQUEST_ERROR_CODES)[number]; + +/** Thrown by a source for an operational failure; the lookup translates it into a result. */ +export class PullRequestSourceError extends Error { + constructor( + public readonly code: PullRequestErrorCode, + /** For `RATE_LIMITED`: how long GitHub asked callers to wait, when it said. */ + public readonly retryAfterMs?: number, + ) { + super(`Pull request lookup failed: ${code}`); + this.name = 'PullRequestSourceError'; + } +} + +/** Operator bounds on one lookup; each defaults to the value the feature shipped with. */ +export type PullRequestLookupLimits = { + /** Longest one GitHub request may take. */ + requestTimeoutMs?: number; + /** Longest the whole lookup, every request together, may take. */ + lookupTimeoutMs?: number; + /** Most pages of check runs read before the rollup is reported as still running. */ + maxCheckRunPages?: number; + /** Most pull requests listed per state when searching a branch's history for the match. */ + maxCandidatePullRequests?: number; + /** Most candidates compared with the recorded commit before the search gives up. */ + maxHeadComparisons?: number; +}; + +export type PullRequestFindInput = { + /** `owner/name`. */ + repo: string; + branch: string; + /** The commit the conversation last ran at; a pull request must carry it to be the match. */ + head?: string | null; + token: string; + limits?: PullRequestLookupLimits; +}; + +/** Finds the pull request for a branch. Null is a documented absence, not a failure. */ +export type PullRequestSource = { + find(input: PullRequestFindInput): Promise; +}; + +export type PullRequestLookupResult = + | { ok: true; value: TConversationPullRequest | null } + | { ok: false; error: { code: PullRequestErrorCode } }; + +export type PullRequestLookupInput = PullRequestFindInput & { + ttlMs: number; + /** Entries the shared cache holds before it evicts the oldest; the lookup's own default if omitted. */ + cacheMaxEntries?: number; + /** Credentials cached at once; the lookup's own default if omitted. */ + cacheMaxCredentials?: number; +}; + +export type PullRequestLookup = (input: PullRequestLookupInput) => Promise; diff --git a/packages/data-provider/src/config.spec.ts b/packages/data-provider/src/config.spec.ts index 7123e83c4c4..99fb5de8b05 100644 --- a/packages/data-provider/src/config.spec.ts +++ b/packages/data-provider/src/config.spec.ts @@ -1125,25 +1125,93 @@ describe('agent pull request config', () => { version: '1.0', endpoints: { agents: { pullRequests: {} } }, }); - expect(empty.endpoints?.agents?.pullRequests).toEqual({ enabled: false, cacheTtlSeconds: 30 }); + expect(empty.endpoints?.agents?.pullRequests).toEqual({ + enabled: false, + cacheTtlSeconds: 30, + requestTimeoutSeconds: 10, + lookupTimeoutSeconds: 30, + maxCheckRunPages: 10, + maxCandidatePullRequests: 10, + maxHeadComparisons: 3, + cacheMaxEntries: 500, + cacheMaxCredentials: 256, + }); }); - it('accepts an enabled block with an environment variable reference', () => { - const result = parse({ enabled: true, token: '${GITHUB_PULL_REQUEST_TOKEN}' }); - expect(result.success).toBe(true); + const enabled = { + enabled: true, + token: '${GITHUB_PULL_REQUEST_TOKEN}', + allowedRepositories: ['LibreChat-AI/LibreChat'], + }; + + it('accepts an enabled block with a token reference and an allowed repository', () => { + expect(parse(enabled).success).toBe(true); }); it('requires a token reference when enabled', () => { - expect(parse({ enabled: true }).success).toBe(false); + expect(parse({ ...enabled, token: undefined }).success).toBe(false); + }); + + it('requires at least one allowed repository when enabled, so a worker cannot name its own', () => { + expect(parse({ ...enabled, allowedRepositories: undefined }).success).toBe(false); + expect(parse({ ...enabled, allowedRepositories: [] }).success).toBe(false); + }); + + it.each(['LibreChat-AI/*', 'o/r', 'My.Org/my_repo-2'])('accepts the repository %s', (repo) => { + expect(parse({ ...enabled, allowedRepositories: [repo] }).success).toBe(true); }); + it.each(['*/*', '*', 'owner', 'o/r/extra', '../x', 'o/..', 'a b/c', 'o/*x', ''])( + 'rejects %p as an allowed repository', + (repo) => { + expect(parse({ ...enabled, allowedRepositories: [repo] }).success).toBe(false); + }, + ); + it.each(['ghp_abcdef', '${bad name}', '$GITHUB_TOKEN', '${}'])( 'rejects %s as a token because only a reference is allowed', (token) => { - expect(parse({ enabled: true, token }).success).toBe(false); + expect(parse({ ...enabled, token }).success).toBe(false); }, ); + it.each([ + ['requestTimeoutSeconds', 0], + ['requestTimeoutSeconds', 61], + ['lookupTimeoutSeconds', 0], + ['lookupTimeoutSeconds', 121], + ['maxCheckRunPages', 0], + ['maxCheckRunPages', 51], + ['maxCheckRunPages', 1.5], + ['maxCandidatePullRequests', 0], + ['maxCandidatePullRequests', 101], + ['maxHeadComparisons', -1], + ['maxHeadComparisons', 21], + ['maxHeadComparisons', 1.5], + ['cacheMaxEntries', 9], + ['cacheMaxEntries', 100_001], + ['cacheMaxEntries', 1.5], + ['cacheMaxCredentials', 0], + ['cacheMaxCredentials', 10_001], + ['cacheMaxCredentials', 1.5], + ])('rejects %s of %s', (field, value) => { + expect(parse({ [field]: value }).success).toBe(false); + }); + + it('accepts the documented bounds', () => { + expect( + parse({ + requestTimeoutSeconds: 60, + lookupTimeoutSeconds: 120, + maxCheckRunPages: 50, + maxCandidatePullRequests: 100, + maxHeadComparisons: 20, + cacheMaxEntries: 100_000, + }).success, + ).toBe(true); + expect(parse({ maxHeadComparisons: 0 }).success).toBe(true); + }); + it.each([4, 3601, 1.5])('rejects a cache lifetime of %s seconds', (cacheTtlSeconds) => { expect(parse({ cacheTtlSeconds }).success).toBe(false); }); diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index 38e7fdfb279..05d522b8cd5 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -498,6 +498,17 @@ const skillSyncTenantIdSchema = z message: 'must not be the reserved system tenant id', }); +/** `owner/name`, or `owner/*` for every repository of one owner; never a bare `*`. */ +const pullRequestRepositorySchema = z + .string() + .trim() + .regex(/^[A-Za-z0-9_.-]+\/(?:[A-Za-z0-9_.-]+|\*)$/, { + message: 'must be owner/name or owner/*', + }) + .refine((value) => value.split('/').every((segment) => segment !== '.' && segment !== '..'), { + message: 'must not contain dot segments', + }); + const pullRequestTokenReferenceSchema = z .string() .trim() @@ -1851,8 +1862,31 @@ export const agentsEndpointSchema = baseEndpointSchema /** Environment variable reference holding a read-only GitHub token, e.g. * `${GITHUB_PULL_REQUEST_TOKEN}`. Never the token itself. */ token: pullRequestTokenReferenceSchema.optional(), + /** Repositories the token may be used for, as `owner/name` or `owner/*`. A worker reports + * its own repository, so without this list a user could point the server's token at any + * repository it can read. */ + allowedRepositories: z.array(pullRequestRepositorySchema).max(256).optional(), /** Seconds a looked-up pull request is reused before GitHub is asked again. */ cacheTtlSeconds: z.number().int().min(5).max(3600).optional().default(30), + /** Pull requests cached per credential before the oldest is evicted. Size it to the + * distinct repository and branch combinations seen within one cache lifetime, or fresh + * results are evicted and GitHub is asked again. */ + cacheMaxEntries: z.number().int().min(10).max(100_000).optional().default(500), + /** Credentials whose results are cached at once; past it the one idle longest is dropped. + * Deployment-wide: when principals configure different values, the largest applies. + * Raise it when many tenants each configure their own token. */ + cacheMaxCredentials: z.number().int().min(1).max(10_000).optional().default(256), + /** Longest one GitHub request may take. Raise it behind a slow proxy. */ + requestTimeoutSeconds: z.number().int().min(1).max(60).optional().default(10), + /** Longest a whole lookup, every request together, may hold the header request. */ + lookupTimeoutSeconds: z.number().int().min(1).max(120).optional().default(30), + /** Pages of 100 check runs read before the rollup is reported as still running. */ + maxCheckRunPages: z.number().int().min(1).max(50).optional().default(10), + /** Pull requests listed per state when matching a branch's history to the commit a chat + * last ran at. Raise it for branch names that are reused many times. */ + maxCandidatePullRequests: z.number().int().min(1).max(100).optional().default(10), + /** Candidates compared with that commit before the search gives up. Each is one request. */ + maxHeadComparisons: z.number().int().min(0).max(20).optional().default(3), }) .superRefine((value, ctx) => { if (value.enabled && !value.token) { @@ -1862,6 +1896,13 @@ export const agentsEndpointSchema = baseEndpointSchema message: 'A token reference is required when pull requests are enabled', }); } + if (value.enabled && (value.allowedRepositories?.length ?? 0) === 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['allowedRepositories'], + message: 'At least one allowed repository is required when pull requests are enabled', + }); + } }) .optional(), /** Conversational background-task delivery policy. Automatic completion wakeups are diff --git a/packages/data-schemas/src/methods/conversation.spec.ts b/packages/data-schemas/src/methods/conversation.spec.ts index 3498cf9e7fa..6215f558d72 100644 --- a/packages/data-schemas/src/methods/conversation.spec.ts +++ b/packages/data-schemas/src/methods/conversation.spec.ts @@ -1679,6 +1679,63 @@ describe('Conversation Operations', () => { }); }; + describe('stale lane state', () => { + const lane = { branch: 'feat/old', head: 'a'.repeat(40), repo: 'o/old' }; + const stored = (conversationId: string) => + methods.getConvoCodeEnvironmentDecision('user123', conversationId); + + it('is cleared when the chat moves to another workspace', async () => { + const conversationId = await seedDecision({ + codeEnvironmentMode: 'attached', + codeWorkspaces: [mac], + laneGit: lane, + }); + await expect(methods.getConvoLaneGit('user123', conversationId)).resolves.toEqual(lane); + await moveFromStored(conversationId, [team]); + await expect(methods.getConvoLaneGit('user123', conversationId)).resolves.toBeNull(); + }); + + it('is cleared when the chat leaves attached execution', async () => { + const conversationId = await seedDecision({ + codeEnvironmentMode: 'attached', + codeWorkspaces: [mac], + laneGit: lane, + }); + const decision = await stored(conversationId); + await methods.replaceConvoCodeEnvironmentDecision({ + user: 'user123', + conversationId, + expected: { + codeEnvironmentMode: decision?.codeEnvironmentMode, + codeWorkspaces: decision?.codeWorkspaces, + codeEnvironmentRevision: decision?.codeEnvironmentRevision, + }, + codeEnvironmentMode: 'without_attached', + }); + await expect(methods.getConvoLaneGit('user123', conversationId)).resolves.toBeNull(); + }); + + it('is kept when the move is refused', async () => { + const conversationId = await seedDecision({ + codeEnvironmentMode: 'attached', + codeWorkspaces: [mac], + laneGit: lane, + }); + await methods.replaceConvoCodeEnvironmentDecision({ + user: 'user123', + conversationId, + expected: { + codeEnvironmentMode: 'attached', + codeWorkspaces: [team], + codeEnvironmentRevision: 99, + }, + codeEnvironmentMode: 'attached', + codeWorkspaces: [vm], + }); + await expect(methods.getConvoLaneGit('user123', conversationId)).resolves.toEqual(lane); + }); + }); + it('replaces the decision it read without disturbing timestamps', async () => { const conversationId = await seedDecision({ codeEnvironmentMode: 'attached', @@ -9359,6 +9416,353 @@ describe('Conversation Operations', () => { }); }); +describe('laneGit', () => { + const seed = async (user = 'lane-user') => { + const conversationId = uuidv4(); + await Conversation.create({ conversationId, user, title: 'Lane test', endpoint: 'agents' }); + return conversationId; + }; + const head = 'a'.repeat(40); + /** A write with a sequence number drawn from the database, as the recorder draws one. */ + const record = async ( + input: Omit[0], 'seq'> & { seq?: number }, + ) => { + const seq = + input.seq ?? (await methods.reserveConvoLaneGitSeq(input.user, input.conversationId)); + if (seq == null) return false; + return methods.setConvoLaneGit({ ...input, seq }); + }; + + it('stores the reported branch and head for the owner and reads them back', async () => { + const conversationId = await seed(); + const laneGit = { branch: 'feat/pr-chip', head }; + await expect(record({ user: 'lane-user', conversationId, laneGit })).resolves.toBe(true); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toEqual(laneGit); + }); + + it('does not write for another owner or an unknown conversation', async () => { + const conversationId = await seed(); + const laneGit = { branch: 'main', head }; + await expect(record({ user: 'intruder', conversationId, laneGit })).resolves.toBe(false); + await expect(record({ user: 'lane-user', conversationId: uuidv4(), laneGit })).resolves.toBe( + false, + ); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toBeNull(); + await expect(methods.getConvoLaneGit('intruder', conversationId)).resolves.toBeNull(); + }); + + it('ignores a report with a lower sequence number, so a delayed write cannot win', async () => { + const conversationId = await seed(); + const user = 'lane-user'; + const first = await methods.reserveConvoLaneGitSeq(user, conversationId); + const second = await methods.reserveConvoLaneGitSeq(user, conversationId); + const newer = { branch: 'newer', head }; + const older = { branch: 'older', head: 'b'.repeat(40) }; + await expect( + methods.setConvoLaneGit({ user, conversationId, laneGit: newer, seq: second as number }), + ).resolves.toBe(true); + await expect( + methods.setConvoLaneGit({ user, conversationId, laneGit: older, seq: first as number }), + ).resolves.toBe(false); + await expect(methods.getConvoLaneGit(user, conversationId)).resolves.toEqual(newer); + }); + + it('issues increasing numbers per conversation and none for an unknown one', async () => { + const conversationId = await seed(); + const other = await seed(); + expect(await methods.reserveConvoLaneGitSeq('lane-user', conversationId)).toBe(1); + expect(await methods.reserveConvoLaneGitSeq('lane-user', conversationId)).toBe(2); + expect(await methods.reserveConvoLaneGitSeq('lane-user', other)).toBe(1); + expect(await methods.reserveConvoLaneGitSeq('intruder', conversationId)).toBeNull(); + expect(await methods.reserveConvoLaneGitSeq('lane-user', uuidv4())).toBeNull(); + }); + + it('issues distinct numbers to reservations made at the same time', async () => { + const conversationId = await seed(); + const numbers = await Promise.all( + Array.from({ length: 12 }, () => methods.reserveConvoLaneGitSeq('lane-user', conversationId)), + ); + expect(new Set(numbers).size).toBe(12); + expect(Math.min(...(numbers as number[]))).toBe(1); + expect(Math.max(...(numbers as number[]))).toBe(12); + }); + + it('is not skewed by the clocks of the processes that report', async () => { + const conversationId = await seed(); + const user = 'lane-user'; + const early = await methods.reserveConvoLaneGitSeq(user, conversationId); + const later = await methods.reserveConvoLaneGitSeq(user, conversationId); + /** An older report from a replica whose clock runs far ahead still carries the lower number. */ + await expect( + methods.setConvoLaneGit({ + user, + conversationId, + laneGit: { branch: 'later', head }, + seq: later as number, + }), + ).resolves.toBe(true); + await expect( + methods.setConvoLaneGit({ + user, + conversationId, + laneGit: { branch: 'early-but-future-clock', head }, + seq: early as number, + }), + ).resolves.toBe(false); + await expect(methods.getConvoLaneGit(user, conversationId)).resolves.toEqual({ + branch: 'later', + head, + }); + }); + + it('does not expose the sequence number or the counter through the read', async () => { + const conversationId = await seed(); + await record({ user: 'lane-user', conversationId, laneGit: { branch: 'main', head } }); + const stored = await methods.getConvoLaneGit('lane-user', conversationId); + expect(stored).not.toHaveProperty('seq'); + const read = await Conversation.findOne({ conversationId }).lean(); + expect(read).not.toHaveProperty('laneGitSeq'); + }); + + it('replaces the stored state when the branch or head changes', async () => { + const conversationId = await seed(); + const user = 'lane-user'; + await record({ user, conversationId, laneGit: { branch: 'main', head } }); + const next = { branch: 'feat/x', head: 'b'.repeat(40) }; + await expect(record({ user, conversationId, laneGit: next })).resolves.toBe(true); + await expect(methods.getConvoLaneGit(user, conversationId)).resolves.toEqual(next); + }); + + it('stores the repository with the lane and replaces it when the repository changes', async () => { + const conversationId = await seed(); + const user = 'lane-user'; + const laneGit = { branch: 'main', head }; + await expect(record({ user, conversationId, laneGit, repo: 'o/r' })).resolves.toBe(true); + await expect(methods.getConvoLaneGit(user, conversationId)).resolves.toEqual({ + ...laneGit, + repo: 'o/r', + }); + await expect(record({ user, conversationId, laneGit, repo: 'o/other' })).resolves.toBe(true); + await expect(methods.getConvoLaneGit(user, conversationId)).resolves.toEqual({ + ...laneGit, + repo: 'o/other', + }); + }); + + describe('workspace fence', () => { + const mac = { environmentId: 'code-mac', workspaceId: 'primary' }; + const team = { environmentId: 'code-team', workspaceId: 'shared' }; + const seedRaw = async (decision: Record) => { + const conversationId = uuidv4(); + await Conversation.collection.insertOne({ + conversationId, + user: 'lane-user', + title: 'Fence', + endpoint: 'agents', + messages: [], + ...decision, + }); + return conversationId; + }; + const write = (conversationId: string, workspace: Record) => + record({ + user: 'lane-user', + conversationId, + laneGit: { branch: 'feat/x', head }, + workspace: workspace as never, + }); + + it('records for the workspace the conversation is attached to', async () => { + const conversationId = await seedRaw({ + codeEnvironmentMode: 'attached', + codeWorkspaces: [mac], + }); + await expect(write(conversationId, mac)).resolves.toBe(true); + }); + + it('refuses a report from a workspace the conversation moved away from', async () => { + const conversationId = await seedRaw({ + codeEnvironmentMode: 'attached', + codeWorkspaces: [team], + }); + await expect(write(conversationId, mac)).resolves.toBe(false); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toBeNull(); + }); + + it('refuses a report once the conversation has left attached execution', async () => { + const conversationId = await seedRaw({ codeEnvironmentMode: 'without_attached' }); + await expect(write(conversationId, mac)).resolves.toBe(false); + }); + + it('does not let a write queued before a move repopulate the cleared lane', async () => { + const conversationId = await seedRaw({ + codeEnvironmentMode: 'attached', + codeWorkspaces: [mac], + laneGit: { branch: 'old', head, repo: 'o/old', seq: 1 }, + }); + const stored = await methods.getConvoCodeEnvironmentDecision('lane-user', conversationId); + await methods.replaceConvoCodeEnvironmentDecision({ + user: 'lane-user', + conversationId, + expected: { + codeEnvironmentMode: stored?.codeEnvironmentMode, + codeWorkspaces: stored?.codeWorkspaces, + codeEnvironmentRevision: stored?.codeEnvironmentRevision, + }, + codeEnvironmentMode: 'attached', + codeWorkspaces: [team], + }); + await expect(write(conversationId, mac)).resolves.toBe(false); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toBeNull(); + }); + + it('records when no workspace is stored yet, as for an agent default workspace', async () => { + const conversationId = await seedRaw({}); + await expect(write(conversationId, mac)).resolves.toBe(true); + }); + + it('refuses an unrecorded workspace when the caller requires a recorded one', async () => { + const conversationId = await seedRaw({}); + await expect(write(conversationId, { ...mac, required: true })).resolves.toBe(false); + }); + + it('accepts a required workspace that matches the recorded one', async () => { + const conversationId = await seedRaw({ + codeEnvironmentMode: 'attached', + codeWorkspaces: [mac, team], + }); + await expect(write(conversationId, { ...team, required: true })).resolves.toBe(true); + }); + }); + + describe('retention', () => { + const lane = { branch: 'feat/x', head, seq: 1 }; + const seedExpiry = async (expiredAt: Date | null) => { + const conversationId = uuidv4(); + await Conversation.collection.insertOne({ + conversationId, + user: 'lane-user', + title: 'Retention', + endpoint: 'agents', + messages: [], + laneGit: lane, + laneGitSeq: 1, + expiredAt, + }); + return conversationId; + }; + const past = () => new Date(Date.now() - 60_000); + const future = () => new Date(Date.now() + 3_600_000); + + it('does not serve the lane of an expired temporary chat', async () => { + const conversationId = await seedExpiry(past()); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toBeNull(); + }); + + it('does not reserve a number for, or write to, an expired temporary chat', async () => { + const conversationId = await seedExpiry(past()); + await expect(methods.reserveConvoLaneGitSeq('lane-user', conversationId)).resolves.toBeNull(); + await expect( + methods.setConvoLaneGit({ + user: 'lane-user', + conversationId, + laneGit: { branch: 'later', head }, + seq: 99, + }), + ).resolves.toBe(false); + }); + + it.each([ + ['no expiry', null], + ['an expiry in the future', future()], + ])('still serves, reserves for and writes to a chat with %s', async (_label, expiredAt) => { + const conversationId = await seedExpiry(expiredAt); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toMatchObject({ + branch: 'feat/x', + }); + const seq = await methods.reserveConvoLaneGitSeq('lane-user', conversationId); + expect(seq).toBe(2); + await expect( + methods.setConvoLaneGit({ + user: 'lane-user', + conversationId, + laneGit: { branch: 'later', head }, + seq: seq as number, + }), + ).resolves.toBe(true); + }); + }); + + it('keeps a detached or empty lane distinct from an unknown one', async () => { + const conversationId = await seed(); + const laneGit = { branch: null, head: null }; + await expect(record({ user: 'lane-user', conversationId, laneGit })).resolves.toBe(true); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toEqual(laneGit); + }); + + describe('server-written only', () => { + const kept = { branch: 'kept', head }; + const seedWithLane = async () => { + const conversationId = await seed(); + await record({ user: 'lane-user', conversationId, laneGit: kept }); + return conversationId; + }; + + it('is not overwritten by a generic save', async () => { + const conversationId = await seedWithLane(); + await saveConvo( + { userId: 'lane-user' }, + { conversationId, laneGit: { branch: 'forged', head: null } }, + ); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toEqual(kept); + }); + + it('is not cleared by a generic save', async () => { + const conversationId = await seedWithLane(); + await saveConvo({ userId: 'lane-user' }, { conversationId }, { unsetFields: { laneGit: 1 } }); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toEqual(kept); + }); + + it('is not accepted from a bulk import', async () => { + const imported = uuidv4(); + await methods.bulkSaveConvos([ + { + conversationId: imported, + user: 'lane-user', + title: 'Imported', + laneGit: { branch: 'x', head: null }, + }, + ]); + await expect(methods.getConvoLaneGit('lane-user', imported)).resolves.toBeNull(); + }); + + it('keeps the sequence counter out of generic saves, unset fields and imports', async () => { + const conversationId = await seedWithLane(); + const before = await methods.reserveConvoLaneGitSeq('lane-user', conversationId); + await saveConvo({ userId: 'lane-user' }, { conversationId, laneGitSeq: 0 }); + await saveConvo( + { userId: 'lane-user' }, + { conversationId }, + { unsetFields: { laneGitSeq: 1 } }, + ); + const imported = uuidv4(); + await methods.bulkSaveConvos([ + { conversationId: imported, user: 'lane-user', title: 'Imported', laneGitSeq: 99 }, + ]); + expect(await methods.reserveConvoLaneGitSeq('lane-user', conversationId)).toBe( + (before as number) + 1, + ); + expect(await methods.reserveConvoLaneGitSeq('lane-user', imported)).toBe(1); + }); + + it('is excluded from ordinary conversation reads', async () => { + const conversationId = await seedWithLane(); + const read = await Conversation.findOne({ conversationId }).lean(); + expect(read).not.toHaveProperty('laneGit'); + }); + }); +}); + describe('stampForcedRetention', () => { const ephemeral = { retentionMode: RetentionMode.EPHEMERAL, temporaryChatRetention: 1 }; let userId: string; @@ -9509,3 +9913,199 @@ describe('stampForcedRetention', () => { }, ); }); + +describe('laneGit review hardening', () => { + const head = 'a'.repeat(40); + const mac = { environmentId: 'code-mac', workspaceId: 'primary' }; + const team = { environmentId: 'code-team', workspaceId: 'shared' }; + const seedAttached = async ( + workspaces: Array<{ environmentId: string; workspaceId: string }>, + ) => { + const conversationId = uuidv4(); + await Conversation.collection.insertOne({ + conversationId, + user: 'lane-user', + title: 'Hardening', + endpoint: 'agents', + messages: [], + codeEnvironmentMode: 'attached', + codeWorkspaces: workspaces, + }); + return conversationId; + }; + const move = async ( + conversationId: string, + codeWorkspaces: Array<{ environmentId: string; workspaceId: string }>, + ) => { + const stored = await methods.getConvoCodeEnvironmentDecision('lane-user', conversationId); + return methods.replaceConvoCodeEnvironmentDecision({ + user: 'lane-user', + conversationId, + expected: { + codeEnvironmentMode: stored?.codeEnvironmentMode, + codeWorkspaces: stored?.codeWorkspaces, + codeEnvironmentRevision: stored?.codeEnvironmentRevision, + }, + codeEnvironmentMode: 'attached', + codeWorkspaces: codeWorkspaces as never, + }); + }; + + describe('dotted import and save keys', () => { + it('does not let an import set a lane through dotted keys', async () => { + const imported = uuidv4(); + await methods.bulkSaveConvos([ + { + conversationId: imported, + user: 'lane-user', + title: 'Imported', + 'laneGit.branch': 'forged', + 'laneGit.repo': 'o/forged', + 'laneGit.seq': 999, + 'laneGitSeq.x': 1, + 'codeAttachmentEpoch.x': 5, + }, + ]); + await expect(methods.getConvoLaneGit('lane-user', imported)).resolves.toBeNull(); + const raw = await Conversation.collection.findOne({ conversationId: imported }); + expect(raw).not.toHaveProperty('laneGit'); + expect(raw).not.toHaveProperty('codeAttachmentEpoch'); + expect(await methods.reserveConvoLaneGitSeq('lane-user', imported)).toBe(1); + }); + + it('does not let a save set a lane through dotted keys or unset keys', async () => { + const conversationId = uuidv4(); + await Conversation.create({ conversationId, user: 'lane-user', endpoint: 'agents' }); + const seq = (await methods.reserveConvoLaneGitSeq('lane-user', conversationId)) as number; + await methods.setConvoLaneGit({ + user: 'lane-user', + conversationId, + laneGit: { branch: 'kept', head }, + seq, + }); + await saveConvo( + { userId: 'lane-user' }, + { conversationId, 'laneGit.branch': 'forged', 'laneGit.seq': 999 }, + { unsetFields: { 'laneGit.repo': 1, 'laneGit.head': 1 } }, + ); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toEqual({ + branch: 'kept', + head, + }); + expect(await methods.reserveConvoLaneGitSeq('lane-user', conversationId)).toBe(seq + 1); + }); + }); + + describe('attachment epoch', () => { + const write = ( + conversationId: string, + epoch: number | undefined, + workspace: { environmentId: string; workspaceId: string }, + ) => + methods.reserveConvoLaneGitSeq('lane-user', conversationId).then((seq) => + methods.setConvoLaneGit({ + user: 'lane-user', + conversationId, + laneGit: { branch: 'feat/x', head }, + seq: seq as number, + workspace: { ...workspace, ...(epoch == null ? {} : { epoch }) }, + }), + ); + + it('reads as zero until the conversation is moved, then advances on every move', async () => { + const conversationId = await seedAttached([mac]); + expect( + (await methods.getConvoLaneContext('lane-user', conversationId))?.codeAttachmentEpoch ?? 0, + ).toBe(0); + await move(conversationId, [team]); + expect( + (await methods.getConvoLaneContext('lane-user', conversationId))?.codeAttachmentEpoch, + ).toBe(1); + await move(conversationId, [mac]); + expect( + (await methods.getConvoLaneContext('lane-user', conversationId))?.codeAttachmentEpoch, + ).toBe(2); + }); + + it('applies a report from the epoch it was captured in', async () => { + const conversationId = await seedAttached([mac]); + await expect(write(conversationId, 0, mac)).resolves.toBe(true); + }); + + it('refuses a report from before a move away and back to the same workspace', async () => { + const conversationId = await seedAttached([mac]); + await move(conversationId, [team]); + await move(conversationId, [mac]); + await expect(write(conversationId, 0, mac)).resolves.toBe(false); + await expect(methods.getConvoLaneGit('lane-user', conversationId)).resolves.toBeNull(); + await expect(write(conversationId, 2, mac)).resolves.toBe(true); + }); + + it('is not advanced by admitting a generation', async () => { + const conversationId = await seedAttached([mac]); + await methods.readAdmittedConvoCodeEnvironmentDecision('lane-user', conversationId); + await methods.readAdmittedConvoCodeEnvironmentDecision('lane-user', conversationId); + await expect(write(conversationId, 0, mac)).resolves.toBe(true); + }); + + it('keeps the epoch out of generic saves, unset fields and ordinary reads', async () => { + const conversationId = await seedAttached([mac]); + await move(conversationId, [team]); + await saveConvo( + { userId: 'lane-user' }, + { conversationId, codeAttachmentEpoch: 0 }, + { unsetFields: { codeAttachmentEpoch: 1 } }, + ); + expect( + (await methods.getConvoLaneContext('lane-user', conversationId))?.codeAttachmentEpoch, + ).toBe(1); + const read = await Conversation.findOne({ conversationId }).lean(); + expect(read).not.toHaveProperty('codeAttachmentEpoch'); + }); + }); + + describe('subagent thread epoch', () => { + const seedThread = async (rootId: string) => { + const conversationId = uuidv4(); + await Conversation.collection.insertOne({ + conversationId, + user: 'lane-user', + title: 'Thread', + endpoint: 'agents', + messages: [], + subagentThread: { rootConversationId: rootId, parentConversationId: rootId }, + }); + return conversationId; + }; + + it("reads the epoch of the visible root the report is written to, not the thread's own", async () => { + const rootId = await seedAttached([mac]); + await move(rootId, [team]); + const threadId = await seedThread(rootId); + await expect(methods.getConvoLaneContext('lane-user', threadId)).resolves.toMatchObject({ + codeAttachmentEpoch: 1, + subagentThread: { rootConversationId: rootId }, + }); + }); + + it('reads zero when the root is gone, so the write is refused rather than guessed', async () => { + const threadId = await seedThread(uuidv4()); + await expect(methods.getConvoLaneContext('lane-user', threadId)).resolves.toMatchObject({ + codeAttachmentEpoch: 0, + }); + }); + }); + + describe('lane context', () => { + it('returns the subagent route and the epoch in one read, owner-scoped', async () => { + const conversationId = await seedAttached([mac]); + await move(conversationId, [team]); + await expect(methods.getConvoLaneContext('lane-user', conversationId)).resolves.toMatchObject( + { + codeAttachmentEpoch: 1, + }, + ); + await expect(methods.getConvoLaneContext('intruder', conversationId)).resolves.toBeNull(); + }); + }); +}); diff --git a/packages/data-schemas/src/methods/conversation.ts b/packages/data-schemas/src/methods/conversation.ts index 6baa5c04427..1391d71b4e6 100644 --- a/packages/data-schemas/src/methods/conversation.ts +++ b/packages/data-schemas/src/methods/conversation.ts @@ -63,14 +63,30 @@ const ACTOR_CHECKPOINT_FIELDS = [ 'agentEventActorSuspension', ] as const; -function stripActorCheckpointFields(record: Record): void { +/** Removes each field and every dotted path beneath it, which Mongo reads as a write to the field. */ +function stripFields(record: Record, fields: readonly string[]): void { for (const key of Object.keys(record)) { - if (ACTOR_CHECKPOINT_FIELDS.some((field) => key === field || key.startsWith(`${field}.`))) { + if (fields.some((field) => key === field || key.startsWith(`${field}.`))) { delete record[key]; } } } +function stripActorCheckpointFields(record: Record): void { + stripFields(record, ACTOR_CHECKPOINT_FIELDS); +} + +/** Written only by the lane methods, so no save, unset or import may reach them. */ +const LANE_PRIVATE_FIELDS = ['laneGit', 'laneGitSeq', 'codeAttachmentEpoch'] as const; + +/** What a lane recorder needs to place and fence its writes, read once when its tool is created. */ +export type ConvoLaneContext = { + subagentThread?: IConversation['subagentThread'] | null; + /** Counts the owner's moves and detaches of the workspace the report is written to (the visible + * root for a subagent thread); 0 until the first. */ + codeAttachmentEpoch: number; +}; + const AGENT_EVENT_ACTOR_RECEIPT_RETENTION_MS = 90 * 24 * 60 * 60_000; const MAX_AGENT_EVENT_ACTOR_SUSPENSION_BYTES = 64 * 1_024; /** MeiliSearch's default `pagination.maxTotalHits` ceiling. */ @@ -329,6 +345,28 @@ export interface ConversationMethods { IConversation, 'conversationId' | 'codeEnvironmentMode' | 'codeWorkspaces' | 'codeEnvironmentRevision' > | null>; + reserveConvoLaneGitSeq(user: string, conversationId: string): Promise; + getConvoLaneContext(user: string, conversationId: string): Promise; + setConvoLaneGit(input: { + user: string; + conversationId: string; + laneGit: Pick, 'branch' | 'head'>; + repo?: string; + /** Reserved with `reserveConvoLaneGitSeq` when the command settled. */ + seq: number; + /** The workspace the command ran in; the write applies only while the chat is still on it. */ + workspace?: { + environmentId: string; + workspaceId: string; + required?: boolean; + /** The attachment epoch read when the tool was created; see `getConvoLaneContext`. */ + epoch?: number; + }; + }): Promise; + getConvoLaneGit( + user: string, + conversationId: string, + ): Promise, 'seq'> | null>; addConvoToolApprovalAllows(input: { user: string; conversationId: string; @@ -2452,6 +2490,7 @@ export function createConversationMethods( } /* Remembered tool approvals are granted only by a validated resume. */ delete update.toolApprovalAllows; + stripFields(update, LANE_PRIVATE_FIELDS); /** Ordinary saves may seed a decision, but only an explicit move may replace it. */ const decisionOnInsert = { ...(convo.codeEnvironmentMode != null && { @@ -2477,6 +2516,7 @@ export function createConversationMethods( delete unsetFields.titleSetByUser; delete unsetFields.titleRevision; delete unsetFields.toolApprovalAllows; + stripFields(unsetFields, LANE_PRIVATE_FIELDS); delete unsetFields.codeEnvironmentRevision; delete unsetFields.codeEnvironmentMode; delete unsetFields.codeWorkspaces; @@ -3003,6 +3043,177 @@ export function createConversationMethods( return result.matchedCount === 1; } + /** + * Reserve the next lane report sequence number for an owner's conversation. The counter lives in + * the database, so every replica draws from one order that no process clock can skew, and the + * number is taken when a command settles, before its write is attempted. Resolves to null when + * the conversation does not exist yet for this owner. + */ + async function reserveConvoLaneGitSeq( + user: string, + conversationId: string, + ): Promise { + const Conversation = mongoose.models.Conversation as Model; + const reserved = await withoutMeiliIndexing( + Conversation.findOneAndUpdate( + { user, conversationId, ...activeExpirationFilter() }, + { $inc: { laneGitSeq: 1 } }, + { new: true, timestamps: false }, + ), + ) + .select('laneGitSeq') + .lean | null>(); + return reserved?.laneGitSeq ?? null; + } + + /** + * The route and attachment epoch a lane recorder needs, in one owner-scoped read. A subagent + * thread's route names the visible conversation its lane belongs to. The epoch advances only + * when the owner moves or detaches the workspace (never when a generation is admitted), so a + * report captured before a move away and back to the same workspace still reads as stale. + * Resolves to null for a conversation that does not exist yet, or is not this owner's. + */ + async function getConvoLaneContext( + user: string, + conversationId: string, + ): Promise { + const Conversation = mongoose.models.Conversation as Model; + const stored = await Conversation.findOne( + { user, conversationId, ...activeExpirationFilter() }, + 'subagentThread +codeAttachmentEpoch', + ).lean>(); + if (stored == null) return null; + const rootId = stored.subagentThread?.rootConversationId; + /** A thread's report is written to its visible root, so it is fenced by the root's epoch. */ + const fenced = + rootId && rootId !== conversationId + ? await Conversation.findOne( + { user, conversationId: rootId, ...activeExpirationFilter() }, + '+codeAttachmentEpoch', + ).lean>() + : stored; + return { + subagentThread: stored.subagentThread ?? null, + codeAttachmentEpoch: fenced?.codeAttachmentEpoch ?? 0, + }; + } + + /** + * Record the branch and head a conversation's code lane last reported. Owner-scoped and fenced + * by the sequence number reserved when the command settled: the write matches only while the + * stored report carries a lower one, so a delayed older report, from this process or another + * replica, can never replace a newer state. When the caller names the workspace the command ran + * in, the write also matches only while the conversation is still attached to it, so a report + * queued before a move or detach cannot bring the old workspace's lane back. A conversation + * with no stored workspace (an agent default) accepts the report unless the caller requires a + * recorded one. Server-written only: generic saves and imports cannot set it. An expired + * temporary chat is treated as gone, here and in the reservation and the read, the way every + * other owner-scoped conversation access treats it. Resolves to whether the write applied; false + * means a newer report is already stored, the workspace no longer matches, or there is no such + * conversation for this owner. + */ + async function setConvoLaneGit({ + user, + conversationId, + laneGit, + repo, + seq, + workspace, + }: { + user: string; + conversationId: string; + laneGit: Pick, 'branch' | 'head'>; + repo?: string; + seq: number; + workspace?: { environmentId: string; workspaceId: string; required?: boolean; epoch?: number }; + }): Promise { + const Conversation = mongoose.models.Conversation as Model; + const next = { + branch: laneGit.branch, + head: laneGit.head, + ...(repo ? { repo } : {}), + seq, + }; + const epochFence = + workspace?.epoch == null + ? [] + : [ + { + codeAttachmentEpoch: workspace.epoch === 0 ? { $in: [null, 0] } : workspace.epoch, + }, + ]; + const workspaceFence = + workspace == null + ? [] + : [ + ...epochFence, + { + $or: [ + { + codeWorkspaces: { + $elemMatch: { + environmentId: workspace.environmentId, + workspaceId: workspace.workspaceId, + }, + }, + }, + ...(workspace.required === true + ? [] + : [ + { + codeEnvironmentMode: { $ne: 'without_attached' }, + $or: [ + { codeWorkspaces: { $exists: false } }, + { codeWorkspaces: null }, + { codeWorkspaces: { $size: 0 } }, + ], + }, + ]), + ], + }, + ]; + const result = await withoutMeiliIndexing( + Conversation.updateOne( + { + user, + conversationId, + ...activeExpirationFilter(), + $and: [ + { + $or: [ + { 'laneGit.seq': { $exists: false } }, + { 'laneGit.seq': null }, + { 'laneGit.seq': { $lt: seq } }, + ], + }, + ...workspaceFence, + ], + }, + { $set: { laneGit: next } }, + { timestamps: false }, + ), + ); + return result.modifiedCount === 1; + } + + /** The last reported lane state, or null when the conversation has none or is not the owner's. */ + async function getConvoLaneGit( + user: string, + conversationId: string, + ): Promise, 'seq'> | null> { + const Conversation = mongoose.models.Conversation as Model; + const stored = await Conversation.findOne({ + user, + conversationId, + ...activeExpirationFilter(), + }) + .select('laneGit') + .lean | null>(); + if (stored?.laneGit == null) return null; + const { branch, head, repo } = stored.laneGit; + return { branch, head, ...(repo ? { repo } : {}) }; + } + async function readAdmittedConvoCodeEnvironmentDecision(user: string, conversationId: string) { const Conversation = mongoose.models.Conversation as Model; return withoutMeiliIndexing( @@ -3054,12 +3265,17 @@ export function createConversationMethods( codeWorkspaces: expected.codeWorkspaces ?? { $in: [null] }, codeEnvironmentRevision: expected.codeEnvironmentRevision ?? { $in: [null] }, }, + /** The reported lane belongs to the workspace being replaced, so it goes with it. */ codeEnvironmentMode === 'attached' - ? { $set: { codeEnvironmentMode, codeWorkspaces }, $inc: { codeEnvironmentRevision: 1 } } + ? { + $set: { codeEnvironmentMode, codeWorkspaces }, + $unset: { laneGit: 1 }, + $inc: { codeEnvironmentRevision: 1, codeAttachmentEpoch: 1 }, + } : { $set: { codeEnvironmentMode }, - $unset: { codeWorkspaces: 1 }, - $inc: { codeEnvironmentRevision: 1 }, + $unset: { codeWorkspaces: 1, laneGit: 1 }, + $inc: { codeEnvironmentRevision: 1, codeAttachmentEpoch: 1 }, }, { new: true, timestamps: false }, ).lean(); @@ -3150,6 +3366,7 @@ export function createConversationMethods( delete sanitized.codeApprovalMode; delete sanitized.initial_agent_id; delete sanitized.toolApprovalAllows; + stripFields(sanitized, LANE_PRIVATE_FIELDS); delete sanitized.codeEnvironmentRevision; stripActorCheckpointFields(sanitized); if (typeof sanitized.user === 'string' && typeof sanitized.chatProjectId === 'string') { @@ -4232,6 +4449,10 @@ export function createConversationMethods( appendConvoMessageReference, getConvoCodeEnvironmentDecision, addConvoToolApprovalAllows, + reserveConvoLaneGitSeq, + setConvoLaneGit, + getConvoLaneGit, + getConvoLaneContext, readAdmittedConvoCodeEnvironmentDecision, replaceConvoCodeEnvironmentDecision, bulkSaveConvos, diff --git a/packages/data-schemas/src/schema/convo.ts b/packages/data-schemas/src/schema/convo.ts index c0d940bbcf4..8aa5f4d6036 100644 --- a/packages/data-schemas/src/schema/convo.ts +++ b/packages/data-schemas/src/schema/convo.ts @@ -47,6 +47,27 @@ const convoSchema: Schema = new Schema( /** Exact tool names the owner chose to auto-approve for this conversation. * Server-written only, through `addConvoToolApprovalAllows`. */ toolApprovalAllows: { type: [String], default: undefined }, + /** Branch and head of the code lane, as the worker last reported them. Server-written only, + * through `setConvoLaneGit`, and excluded from ordinary conversation reads. */ + laneGit: { + type: { + branch: { type: String, default: null }, + head: { type: String, default: null }, + repo: { type: String, default: undefined }, + /** Sequence number reserved when the command settled; fences out an older report. */ + seq: { type: Number, default: undefined }, + }, + _id: false, + default: undefined, + select: false, + }, + /** Counter that issues `laneGit.seq`, shared by every replica. Server-written only, through + * `reserveConvoLaneGitSeq`, and excluded from ordinary conversation reads. */ + laneGitSeq: { type: Number, default: undefined, select: false }, + /** Counts the owner's moves and detaches of the conversation's workspace. A lane report + * carries the value read when its tool was created and applies only while it still matches, + * so moving away and back cannot revive an older report. Server-written only. */ + codeAttachmentEpoch: { type: Number, default: undefined, select: false }, agent_id: { type: String, }, diff --git a/packages/data-schemas/src/types/convo.ts b/packages/data-schemas/src/types/convo.ts index 66b0b21dd78..fe0d4938c4d 100644 --- a/packages/data-schemas/src/types/convo.ts +++ b/packages/data-schemas/src/types/convo.ts @@ -21,6 +21,16 @@ export const MAX_AGENT_EVENT_ACTOR_ENCODING_LENGTH = 128; */ export const AGENT_EVENT_ACTOR_SUMMARY_VERSION = 1; +/** Branch and head commit the conversation's code lane last reported; null when it has none. */ +export interface IConversationLaneGit { + branch: string | null; + head: string | null; + /** `owner/name` of the lane's repository, as its worker registered it. */ + repo?: string; + /** Reserved sequence number; internal, never returned by `getConvoLaneGit`. */ + seq?: number; +} + export interface ISubagentThreadLease { token: string; taskId: string; @@ -290,6 +300,12 @@ export interface IConversation extends Document { codeWorkspaces?: CodeWorkspaceSelection[]; /** Exact tool names auto-approved for this conversation; server-written only. */ toolApprovalAllows?: string[]; + /** Last lane branch and head a command reported; server-written only and excluded from reads. */ + laneGit?: IConversationLaneGit; + /** Counter behind `laneGit.seq`; server-written only and excluded from reads. */ + laneGitSeq?: number; + /** Moves and detaches of the workspace; fences lane reports from an earlier attachment. */ + codeAttachmentEpoch?: number; /** Immutable primary persisted-agent attribution for Insights. */ initial_agent_id?: string | null; subagentThread?: TSubagentThreadLineage;