From 15ded69e25800406c60148993a369aac9c783b4b Mon Sep 17 00:00:00 2001 From: vmelikyan Date: Sun, 19 Apr 2026 13:38:45 -0700 Subject: [PATCH] harden agent session startup and skills bootstrap --- .../__tests__/configSeeder.test.ts | 30 ++ .../__tests__/serviceAccountFactory.test.ts | 68 +++ src/server/lib/agentSession/configSeeder.ts | 61 ++- src/server/lib/agentSession/podFactory.ts | 8 +- .../lib/agentSession/serviceAccountFactory.ts | 21 +- .../__tests__/agentSandboxSession.test.ts | 96 ++++ .../services/__tests__/agentSession.test.ts | 25 + .../services/agent/CapabilityService.ts | 6 +- src/server/services/agent/RunExecutor.ts | 485 +++++++++++------- src/server/services/agent/RunService.ts | 51 +- .../agent/__tests__/CapabilityService.test.ts | 22 +- .../agent/__tests__/RunExecutor.test.ts | 157 +++++- .../agent/__tests__/ThreadService.test.ts | 98 ++++ src/server/services/agent/errors.ts | 49 ++ src/server/services/agentSandboxSession.ts | 90 ++-- src/server/services/agentSession.ts | 175 ++++--- sysops/workspace-gateway/skills-bootstrap.mjs | 91 ++-- sysops/workspace-gateway/skills-lib.mjs | 6 +- sysops/workspace-gateway/skills-lib.test.mjs | 22 + 19 files changed, 1178 insertions(+), 383 deletions(-) create mode 100644 src/server/lib/agentSession/__tests__/serviceAccountFactory.test.ts create mode 100644 src/server/services/agent/__tests__/ThreadService.test.ts create mode 100644 src/server/services/agent/errors.ts create mode 100644 sysops/workspace-gateway/skills-lib.test.mjs diff --git a/src/server/lib/agentSession/__tests__/configSeeder.test.ts b/src/server/lib/agentSession/__tests__/configSeeder.test.ts index f95f1a27..cd48d9da 100644 --- a/src/server/lib/agentSession/__tests__/configSeeder.test.ts +++ b/src/server/lib/agentSession/__tests__/configSeeder.test.ts @@ -163,6 +163,36 @@ describe('configSeeder', () => { expect(script).toContain('"/workspace/repos/org/api"'); }); + it('clones multiple repositories in parallel before continuing to installs', () => { + const script = generateInitScript({ + workspaceRepos: [ + { + repo: 'org/ui', + repoUrl: 'https://github.com/org/ui.git', + branch: 'feature/ui', + revision: null, + mountPath: '/workspace/repos/org/ui', + primary: true, + }, + { + repo: 'org/api', + repoUrl: 'https://github.com/org/api.git', + branch: 'feature/api', + revision: null, + mountPath: '/workspace/repos/org/api', + primary: false, + }, + ], + installCommand: 'pnpm install', + }); + + expect(script).toContain('clone_pids=""'); + expect(script).toContain('clone_pids="$clone_pids $!"'); + expect(script).toContain('for clone_pid in $clone_pids; do'); + expect(script).toContain(' wait "$clone_pid"'); + expect(script.indexOf('for clone_pid in $clone_pids; do')).toBeLessThan(script.indexOf('pnpm install')); + }); + it('starts with shebang', () => { const script = generateInitScript(baseOpts); expect(script.startsWith('#!/bin/sh')).toBe(true); diff --git a/src/server/lib/agentSession/__tests__/serviceAccountFactory.test.ts b/src/server/lib/agentSession/__tests__/serviceAccountFactory.test.ts new file mode 100644 index 00000000..2ecf2439 --- /dev/null +++ b/src/server/lib/agentSession/__tests__/serviceAccountFactory.test.ts @@ -0,0 +1,68 @@ +/** + * Copyright 2026 GoodRx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const mockSetupReadOnlyServiceAccountInNamespace = jest.fn(); + +jest.mock('server/lib/kubernetes/rbac', () => ({ + setupReadOnlyServiceAccountInNamespace: mockSetupReadOnlyServiceAccountInNamespace, +})); + +function loadModule() { + let loadedModule: typeof import('../serviceAccountFactory'); + jest.isolateModules(() => { + loadedModule = require('../serviceAccountFactory'); + }); + + return loadedModule!; +} + +describe('serviceAccountFactory', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('reuses the in-flight setup promise for the same namespace', async () => { + let resolveSetup!: () => void; + mockSetupReadOnlyServiceAccountInNamespace.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSetup = resolve; + }) + ); + + const { ensureAgentSessionServiceAccount } = loadModule(); + const firstCall = ensureAgentSessionServiceAccount('test-ns'); + const secondCall = ensureAgentSessionServiceAccount('test-ns'); + + expect(mockSetupReadOnlyServiceAccountInNamespace).toHaveBeenCalledTimes(1); + + resolveSetup(); + + await expect(firstCall).resolves.toBe('agent-sa'); + await expect(secondCall).resolves.toBe('agent-sa'); + }); + + it('clears the namespace cache after a failed setup', async () => { + const setupError = new Error('setup failed'); + mockSetupReadOnlyServiceAccountInNamespace.mockRejectedValueOnce(setupError).mockResolvedValueOnce(undefined); + + const { ensureAgentSessionServiceAccount } = loadModule(); + + await expect(ensureAgentSessionServiceAccount('test-ns')).rejects.toThrow('setup failed'); + await expect(ensureAgentSessionServiceAccount('test-ns')).resolves.toBe('agent-sa'); + expect(mockSetupReadOnlyServiceAccountInNamespace).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/server/lib/agentSession/configSeeder.ts b/src/server/lib/agentSession/configSeeder.ts index ccf1b909..573528c3 100644 --- a/src/server/lib/agentSession/configSeeder.ts +++ b/src/server/lib/agentSession/configSeeder.ts @@ -33,6 +33,33 @@ export interface InitScriptOpts { useGitHubToken?: boolean; } +function buildRepoCloneLines(repo: AgentSessionWorkspaceRepo): string[] { + const parentDir = pathPosix.dirname(repo.mountPath); + const cloneRoot = parentDir === '/' ? repo.mountPath : parentDir; + const lines = [ + `mkdir -p "${escapeDoubleQuotedShell(cloneRoot)}"`, + `git clone --progress --depth 50 --branch "${escapeDoubleQuotedShell( + repo.branch + )}" --single-branch "${escapeDoubleQuotedShell(repo.repoUrl)}" "${escapeDoubleQuotedShell(repo.mountPath)}"`, + `cd "${escapeDoubleQuotedShell(repo.mountPath)}"`, + ]; + + if (!repo.revision) { + return lines; + } + + lines.push( + `if ! git rev-parse --verify --quiet "${escapeDoubleQuotedShell(repo.revision)}^{commit}" >/dev/null; then`, + ` git fetch --unshallow origin "${escapeDoubleQuotedShell( + repo.branch + )}" || git fetch origin "${escapeDoubleQuotedShell(repo.branch)}"`, + 'fi', + `git checkout "${escapeDoubleQuotedShell(repo.revision)}"` + ); + + return lines; +} + function escapeDoubleQuotedShell(value: string): string { return value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\$/g, '\\$').replace(/`/g, '\\`'); } @@ -139,30 +166,20 @@ export function generateInitScript(opts: InitScriptOpts): string { appendGitIdentityAndAuthLines(lines, opts); - for (const repo of workspaceRepos) { - const parentDir = pathPosix.dirname(repo.mountPath); - const cloneRoot = parentDir === '/' ? repo.mountPath : parentDir; - lines.push( - '', - `mkdir -p "${escapeDoubleQuotedShell(cloneRoot)}"`, - `git clone --progress --depth 50 --branch "${escapeDoubleQuotedShell( - repo.branch - )}" --single-branch "${escapeDoubleQuotedShell(repo.repoUrl)}" "${escapeDoubleQuotedShell(repo.mountPath)}"`, - `cd "${escapeDoubleQuotedShell(repo.mountPath)}"` - ); - - if (!repo.revision) { - continue; + if (workspaceRepos.length === 1) { + lines.push('', ...buildRepoCloneLines(workspaceRepos[0])); + } else { + lines.push('', 'clone_pids=""'); + + for (const repo of workspaceRepos) { + lines.push('(', ' set -e'); + for (const repoLine of buildRepoCloneLines(repo)) { + lines.push(` ${repoLine}`); + } + lines.push(') &', 'clone_pids="$clone_pids $!"'); } - lines.push( - `if ! git rev-parse --verify --quiet "${escapeDoubleQuotedShell(repo.revision)}^{commit}" >/dev/null; then`, - ` git fetch --unshallow origin "${escapeDoubleQuotedShell( - repo.branch - )}" || git fetch origin "${escapeDoubleQuotedShell(repo.branch)}"`, - 'fi' - ); - lines.push(`git checkout "${escapeDoubleQuotedShell(repo.revision)}"`); + lines.push('for clone_pid in $clone_pids; do', ' wait "$clone_pid"', 'done'); } if (installCommand) { diff --git a/src/server/lib/agentSession/podFactory.ts b/src/server/lib/agentSession/podFactory.ts index edfdbf96..50cffb51 100644 --- a/src/server/lib/agentSession/podFactory.ts +++ b/src/server/lib/agentSession/podFactory.ts @@ -632,8 +632,8 @@ export function buildSessionWorkspacePodSpec(opts: SessionWorkspacePodOptions): path: '/healthz', port: SESSION_WORKSPACE_EDITOR_PORT, }, - initialDelaySeconds: 2, - periodSeconds: 5, + initialDelaySeconds: 1, + periodSeconds: 2, }, volumeMounts: [ workspaceVolumeMount, @@ -682,8 +682,8 @@ export function buildSessionWorkspacePodSpec(opts: SessionWorkspacePodOptions): path: '/health', port: SESSION_WORKSPACE_GATEWAY_PORT, }, - initialDelaySeconds: 2, - periodSeconds: 5, + initialDelaySeconds: 1, + periodSeconds: 2, }, volumeMounts: [ workspaceVolumeMount, diff --git a/src/server/lib/agentSession/serviceAccountFactory.ts b/src/server/lib/agentSession/serviceAccountFactory.ts index dd954bc2..ccc1840b 100644 --- a/src/server/lib/agentSession/serviceAccountFactory.ts +++ b/src/server/lib/agentSession/serviceAccountFactory.ts @@ -17,8 +17,25 @@ import { setupReadOnlyServiceAccountInNamespace } from 'server/lib/kubernetes/rbac'; export const AGENT_SESSION_SERVICE_ACCOUNT_NAME = 'agent-sa'; +const serviceAccountSetupByNamespace = new Map>(); export async function ensureAgentSessionServiceAccount(namespace: string): Promise { - await setupReadOnlyServiceAccountInNamespace(namespace, AGENT_SESSION_SERVICE_ACCOUNT_NAME); - return AGENT_SESSION_SERVICE_ACCOUNT_NAME; + let setupPromise = serviceAccountSetupByNamespace.get(namespace); + if (!setupPromise) { + setupPromise = (async () => { + await setupReadOnlyServiceAccountInNamespace(namespace, AGENT_SESSION_SERVICE_ACCOUNT_NAME); + return AGENT_SESSION_SERVICE_ACCOUNT_NAME; + })(); + serviceAccountSetupByNamespace.set(namespace, setupPromise); + } + + try { + return await setupPromise; + } catch (error) { + if (serviceAccountSetupByNamespace.get(namespace) === setupPromise) { + serviceAccountSetupByNamespace.delete(namespace); + } + + throw error; + } } diff --git a/src/server/services/__tests__/agentSandboxSession.test.ts b/src/server/services/__tests__/agentSandboxSession.test.ts index 9dd3f842..cd550ca3 100644 --- a/src/server/services/__tests__/agentSandboxSession.test.ts +++ b/src/server/services/__tests__/agentSandboxSession.test.ts @@ -59,6 +59,21 @@ import { BuildEnvironmentVariables } from 'server/lib/buildEnvVariables'; import { fetchLifecycleConfig, getDeployingServicesByName } from 'server/models/yaml'; import { BuildStatus, BuildKind } from 'shared/constants'; +function createDeferred() { + let resolve!: (value: T) => void; + let reject!: (error?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + + return { + promise, + resolve, + reject, + }; +} + describe('agentSandboxSession', () => { beforeEach(() => { jest.clearAllMocks(); @@ -216,6 +231,87 @@ describe('agentSandboxSession', () => { expect(selected.map((item: any) => item.name)).toEqual(['frontend', 'worker']); }); + it('resolves sandbox candidates in parallel', async () => { + const service = new AgentSandboxSessionService({} as any, {} as any, {} as any, {} as any); + const frontendSource = createDeferred(); + const workerSource = createDeferred(); + + jest + .spyOn(service as any, 'resolveServiceSource') + .mockImplementationOnce(() => frontendSource.promise) + .mockImplementationOnce(() => workerSource.promise); + + const resolvePromise = (service as any).resolveCandidateServices( + { + uuid: 'base-build', + deploys: [ + { + id: 1, + active: true, + deployable: { name: 'frontend' }, + repository: { fullName: 'example-org/frontend' }, + branchName: 'main', + }, + { + id: 2, + active: true, + deployable: { name: 'worker' }, + repository: { fullName: 'example-org/worker' }, + branchName: 'main', + }, + ], + } as any, + { + environment: { + defaultServices: [{ name: 'frontend' }, { name: 'worker' }], + optionalServices: [], + }, + } as any, + { + repo: 'example-org/environment', + branch: 'main', + } + ); + + await new Promise((resolve) => setImmediate(resolve)); + + expect((service as any).resolveServiceSource).toHaveBeenCalledTimes(2); + + frontendSource.resolve({ + repo: 'example-org/frontend', + branch: 'main', + yamlService: { + name: 'frontend', + dev: { image: 'node:20', command: 'pnpm dev' }, + github: { + docker: { + app: { + dockerfilePath: 'Dockerfile', + }, + }, + }, + }, + }); + workerSource.resolve({ + repo: 'example-org/worker', + branch: 'main', + yamlService: { + name: 'worker', + dev: { image: 'node:20', command: 'pnpm start' }, + github: { + docker: { + app: { + dockerfilePath: 'Dockerfile', + }, + }, + }, + }, + }); + + const candidates = await resolvePromise; + expect(candidates.map((candidate: any) => candidate.name)).toEqual(['frontend', 'worker']); + }); + it('maps selected services to cloned sandbox deploys by base deploy id', () => { const service = new AgentSandboxSessionService({} as any, {} as any, {} as any, {} as any); const selectedService = { diff --git a/src/server/services/__tests__/agentSession.test.ts b/src/server/services/__tests__/agentSession.test.ts index b74e64ee..22d557b1 100644 --- a/src/server/services/__tests__/agentSession.test.ts +++ b/src/server/services/__tests__/agentSession.test.ts @@ -22,6 +22,7 @@ const mockGetCompatibleReadyPrewarm = jest.fn(); const mockGetReadyPrewarmByPvc = jest.fn(); const mockExecInPod = jest.fn(); const mockResolveSessionPodServersForRepo = jest.fn().mockResolvedValue([]); +const mockGetDefaultThreadForSession = jest.fn().mockResolvedValue({ uuid: 'default-thread-1' }); jest.mock('server/models/AgentSession'); jest.mock('server/models/Build'); @@ -86,6 +87,12 @@ jest.mock('server/services/agentPrewarm', () => ({ getReadyPrewarmByPvc: mockGetReadyPrewarmByPvc, })), })); +jest.mock('server/services/agent/ThreadService', () => ({ + __esModule: true, + default: { + getDefaultThreadForSession: mockGetDefaultThreadForSession, + }, +})); jest.mock('server/lib/nativeHelm/helm', () => ({ deployHelm: jest.fn().mockResolvedValue(undefined), })); @@ -408,6 +415,7 @@ describe('AgentSessionService', () => { mockedBuildServiceModule.deleteBuild.mockResolvedValue(undefined); mockGetCompatibleReadyPrewarm.mockResolvedValue(null); mockGetReadyPrewarmByPvc.mockResolvedValue(null); + mockGetDefaultThreadForSession.mockResolvedValue({ uuid: 'default-thread-1' }); mockExecInPod.mockImplementation( async ( _namespace: string, @@ -549,6 +557,23 @@ describe('AgentSessionService', () => { expect(session.status).toBe('active'); }); + it('does not block session readiness on default thread creation', async () => { + const defaultThread = createDeferred<{ uuid: string }>(); + mockGetDefaultThreadForSession.mockImplementationOnce(() => defaultThread.promise); + + const sessionPromise = AgentSessionService.createSession(baseOpts); + const result = await Promise.race([ + sessionPromise.then(() => 'resolved'), + new Promise((resolve) => setTimeout(() => resolve('timeout'), 20)), + ]); + + expect(result).toBe('resolved'); + expect(mockGetDefaultThreadForSession).toHaveBeenCalledWith('aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', 'user-123'); + + defaultThread.resolve({ uuid: 'default-thread-1' }); + await sessionPromise; + }); + it('reuses a compatible ready prewarm PVC and skips workspace bootstrap', async () => { mockGetCompatibleReadyPrewarm.mockResolvedValue({ uuid: 'prewarm-1', diff --git a/src/server/services/agent/CapabilityService.ts b/src/server/services/agent/CapabilityService.ts index f407bfb4..11db8ff0 100644 --- a/src/server/services/agent/CapabilityService.ts +++ b/src/server/services/agent/CapabilityService.ts @@ -36,6 +36,7 @@ import { SESSION_WORKSPACE_READONLY_TOOL_NAME, } from './toolKeys'; import { getSessionWorkspaceCatalogEntriesForRuntimeTool } from './sandboxToolCatalog'; +import { SessionWorkspaceGatewayUnavailableError } from './errors'; type ToolExecutionHooks = { onToolStarted?: (audit: AgentToolAuditRecord) => Promise; @@ -108,7 +109,10 @@ async function resolveSessionWorkspaceGatewayServer( { error }, `AgentExec: workspace gateway unavailable sessionId=${session.uuid} namespace=${session.namespace} podName=${session.podName}` ); - return null; + throw new SessionWorkspaceGatewayUnavailableError({ + sessionId: session.uuid, + cause: error, + }); } finally { await client.close(); } diff --git a/src/server/services/agent/RunExecutor.ts b/src/server/services/agent/RunExecutor.ts index f5dd0b2d..72d25788 100644 --- a/src/server/services/agent/RunExecutor.ts +++ b/src/server/services/agent/RunExecutor.ts @@ -31,6 +31,7 @@ import AgentProviderRegistry from './ProviderRegistry'; import AgentRunService from './RunService'; import type { AgentFileChangeData, AgentUIMessage } from './types'; import { applyApprovalResponsesToFileChangeParts, buildResultFileChanges } from './fileChanges'; +import { AgentRunTerminalFailure, SessionWorkspaceGatewayUnavailableError } from './errors'; function buildSystemPrompt(parts: Array): string | undefined { const normalized = parts.map((part) => part?.trim()).filter(Boolean) as string[]; @@ -98,6 +99,61 @@ function calculateDurationMs(startedAt?: string | null, completedAt?: string | n return Math.max(0, completedAtMs - startedAtMs); } +function classifyTerminalRunFailure({ + finishReason, + maxIterations, +}: { + finishReason?: string; + maxIterations: number; +}): AgentRunTerminalFailure | null { + switch (finishReason) { + case undefined: + case 'stop': + return null; + case 'tool-calls': + return new AgentRunTerminalFailure({ + code: 'max_iterations_exceeded', + message: `Agent stopped after reaching the configured iteration limit of ${maxIterations}.`, + details: { + finishReason, + maxIterations, + }, + }); + case 'length': + return new AgentRunTerminalFailure({ + code: 'token_limit_reached', + message: 'Agent stopped before completing because the model hit its token limit.', + details: { + finishReason, + }, + }); + case 'content-filter': + return new AgentRunTerminalFailure({ + code: 'content_filtered', + message: 'Agent stopped before completing because the model response was blocked by content filtering.', + details: { + finishReason, + }, + }); + case 'error': + return new AgentRunTerminalFailure({ + code: 'stream_error', + message: 'Agent stream finished with error.', + details: { + finishReason, + }, + }); + default: + return new AgentRunTerminalFailure({ + code: 'run_incomplete', + message: 'Agent stopped before completing the response.', + details: { + finishReason, + }, + }); + } +} + export default class AgentRunExecutor { static async execute({ session, @@ -136,15 +192,6 @@ export default class AgentRunExecutor { requestApiKey, requestApiKeyProvider, }); - const run = await AgentRunService.createRun({ - thread, - session, - provider: selection.provider, - model: selection.modelId, - policy: approvalPolicy, - }); - const controller = new AbortController(); - AgentRunService.registerAbortController(run.uuid, controller); const observabilityTracker = new AgentRunObservabilityTracker(); const touchSessionActivity = async () => { try { @@ -156,208 +203,268 @@ export default class AgentRunExecutor { ); } }; - const effectiveSessionConfig = await AgentSessionConfigService.getInstance().getEffectiveConfig(repoFullName); const sessionPrompt = await AgentSessionService.getSessionAppendSystemPrompt( session.uuid, repoFullName, effectiveSessionConfig.appendSystemPrompt ); - const tools = await AgentCapabilityService.buildToolSet({ - session, - repoFullName, - userIdentity, - approvalPolicy, - workspaceToolDiscoveryTimeoutMs: effectiveSessionConfig.workspaceToolDiscoveryTimeoutMs, - workspaceToolExecutionTimeoutMs: effectiveSessionConfig.workspaceToolExecutionTimeoutMs, - toolRules: effectiveSessionConfig.toolRules, - hooks: { - onToolStarted: async (audit) => { - const pendingAction = audit.toolCallId - ? await AgentPendingAction.query() - .where({ threadId: thread.id, runId: run.id }) - .whereRaw(`payload->>'toolCallId' = ?`, [audit.toolCallId]) - .orderBy('createdAt', 'desc') - .first() - : null; - - await AgentToolExecution.query().insert({ - threadId: thread.id, - runId: run.id, - pendingActionId: pendingAction?.id || null, - source: audit.source, - serverSlug: audit.serverSlug || null, - toolName: audit.toolName, - toolCallId: audit.toolCallId || null, - args: audit.args, - status: 'running', - safetyLevel: audit.capabilityKey, - approved: pendingAction?.status === 'approved' ? true : pendingAction?.status === 'denied' ? false : null, - startedAt: new Date().toISOString(), - } as Partial); - }, - onToolFinished: async (audit) => { - const executionQuery = AgentToolExecution.query().where({ runId: run.id }); + let run: Awaited> | null = null; - if (audit.toolCallId) { - executionQuery.where({ toolCallId: audit.toolCallId }); - } else { - executionQuery.where({ toolName: audit.toolName }); - } + const requireRun = () => { + if (!run) { + throw new Error('Agent run has not been initialized.'); + } - const execution = await executionQuery.orderBy('createdAt', 'desc').first(); + return run; + }; - if (!execution) { - return; - } + try { + const tools = await AgentCapabilityService.buildToolSet({ + session, + repoFullName, + userIdentity, + approvalPolicy, + workspaceToolDiscoveryTimeoutMs: effectiveSessionConfig.workspaceToolDiscoveryTimeoutMs, + workspaceToolExecutionTimeoutMs: effectiveSessionConfig.workspaceToolExecutionTimeoutMs, + toolRules: effectiveSessionConfig.toolRules, + hooks: { + onToolStarted: async (audit) => { + const activeRun = requireRun(); + const pendingAction = audit.toolCallId + ? await AgentPendingAction.query() + .where({ threadId: thread.id, runId: activeRun.id }) + .whereRaw(`payload->>'toolCallId' = ?`, [audit.toolCallId]) + .orderBy('createdAt', 'desc') + .first() + : null; + + await AgentToolExecution.query().insert({ + threadId: thread.id, + runId: activeRun.id, + pendingActionId: pendingAction?.id || null, + source: audit.source, + serverSlug: audit.serverSlug || null, + toolName: audit.toolName, + toolCallId: audit.toolCallId || null, + args: audit.args, + status: 'running', + safetyLevel: audit.capabilityKey, + approved: pendingAction?.status === 'approved' ? true : pendingAction?.status === 'denied' ? false : null, + startedAt: new Date().toISOString(), + } as Partial); + }, + onToolFinished: async (audit) => { + const activeRun = requireRun(); + const executionQuery = AgentToolExecution.query().where({ runId: activeRun.id }); + + if (audit.toolCallId) { + executionQuery.where({ toolCallId: audit.toolCallId }); + } else { + executionQuery.where({ toolName: audit.toolName }); + } - const completedAt = new Date().toISOString(); - const fileChanges = audit.toolCallId - ? buildResultFileChanges({ - toolCallId: audit.toolCallId, - sourceTool: audit.toolName, - input: audit.args, - result: audit.result, - failed: audit.status === 'failed', - }) - : []; - await AgentToolExecution.query().patchAndFetchById(execution.id, { - status: audit.status, - result: { - value: audit.result, - ...(fileChanges.length > 0 ? { fileChanges } : {}), - }, - completedAt, - durationMs: calculateDurationMs(execution.startedAt, completedAt), - } as Partial); + const execution = await executionQuery.orderBy('createdAt', 'desc').first(); + + if (!execution) { + return; + } + + const completedAt = new Date().toISOString(); + const fileChanges = audit.toolCallId + ? buildResultFileChanges({ + toolCallId: audit.toolCallId, + sourceTool: audit.toolName, + input: audit.args, + result: audit.result, + failed: audit.status === 'failed', + }) + : []; + await AgentToolExecution.query().patchAndFetchById(execution.id, { + status: audit.status, + result: { + value: audit.result, + ...(fileChanges.length > 0 ? { fileChanges } : {}), + }, + completedAt, + durationMs: calculateDurationMs(execution.startedAt, completedAt), + } as Partial); + }, + onFileChange: async (change) => { + await onFileChange?.(change); + }, }, - onFileChange: async (change) => { - await onFileChange?.(change); + }); + + run = await AgentRunService.createRun({ + thread, + session, + provider: selection.provider, + model: selection.modelId, + policy: approvalPolicy, + }); + const controller = new AbortController(); + AgentRunService.registerAbortController(run.uuid, controller); + const agent = new ToolLoopAgent({ + model, + instructions: buildSystemPrompt([effectiveSessionConfig.systemPrompt, sessionPrompt]), + tools, + stopWhen: stepCountIs(effectiveSessionConfig.maxIterations), + onStepFinish: async (step) => { + try { + const usageSummary = observabilityTracker.updateFromStep({ + usage: (step as { usage?: unknown }).usage as + | Parameters[0]['usage'] + | undefined, + stepNumber: + typeof (step as { stepNumber?: unknown }).stepNumber === 'number' + ? (step as { stepNumber: number }).stepNumber + : undefined, + toolCalls: Array.isArray((step as { toolCalls?: unknown[] }).toolCalls) + ? (step as { toolCalls: unknown[] }).toolCalls + : undefined, + }); + + await AgentRunService.patchRun(run.uuid, { + usageSummary: usageSummary as Record, + }); + await touchSessionActivity(); + } catch (error) { + getLogger().warn( + { error, runId: run.uuid }, + `AgentExec: step observability patch failed runId=${run.uuid}` + ); + } }, - }, - }); - const agent = new ToolLoopAgent({ - model, - instructions: buildSystemPrompt([effectiveSessionConfig.systemPrompt, sessionPrompt]), - tools, - stopWhen: stepCountIs(effectiveSessionConfig.maxIterations), - onStepFinish: async (step) => { - try { - const usageSummary = observabilityTracker.updateFromStep({ - usage: (step as { usage?: unknown }).usage as - | Parameters[0]['usage'] + onFinish: (event) => { + observabilityTracker.finalize({ + usage: (event as { totalUsage?: unknown }).totalUsage as + | Parameters[0]['usage'] | undefined, - stepNumber: - typeof (step as { stepNumber?: unknown }).stepNumber === 'number' - ? (step as { stepNumber: number }).stepNumber - : undefined, - toolCalls: Array.isArray((step as { toolCalls?: unknown[] }).toolCalls) - ? (step as { toolCalls: unknown[] }).toolCalls + providerMetadata: (event as { providerMetadata?: unknown }).providerMetadata as + | Parameters[0]['providerMetadata'] + | undefined, + steps: Array.isArray((event as { steps?: unknown[] }).steps) + ? (event as { steps: Array<{ toolCalls?: unknown[] }> }).steps + : undefined, + finishReason: + typeof (event as { finishReason?: unknown }).finishReason === 'string' + ? (event as { finishReason: string }).finishReason + : null, + rawFinishReason: + typeof (event as { rawFinishReason?: unknown }).rawFinishReason === 'string' + ? (event as { rawFinishReason: string }).rawFinishReason + : null, + warnings: Array.isArray((event as { warnings?: unknown[] }).warnings) + ? (event as { warnings: unknown[] }).warnings : undefined, + response: (event as { response?: unknown }).response as + | Parameters[0]['response'] + | undefined, }); + }, + }); - await AgentRunService.patchRun(run.uuid, { - usageSummary: usageSummary as Record, - }); - await touchSessionActivity(); - } catch (error) { - getLogger().warn({ error, runId: run.uuid }, `AgentExec: step observability patch failed runId=${run.uuid}`); - } - }, - onFinish: (event) => { - observabilityTracker.finalize({ - usage: (event as { totalUsage?: unknown }).totalUsage as - | Parameters[0]['usage'] - | undefined, - providerMetadata: (event as { providerMetadata?: unknown }).providerMetadata as - | Parameters[0]['providerMetadata'] - | undefined, - steps: Array.isArray((event as { steps?: unknown[] }).steps) - ? (event as { steps: Array<{ toolCalls?: unknown[] }> }).steps - : undefined, - finishReason: - typeof (event as { finishReason?: unknown }).finishReason === 'string' - ? (event as { finishReason: string }).finishReason - : null, - rawFinishReason: - typeof (event as { rawFinishReason?: unknown }).rawFinishReason === 'string' - ? (event as { rawFinishReason: string }).rawFinishReason - : null, - warnings: Array.isArray((event as { warnings?: unknown[] }).warnings) - ? (event as { warnings: unknown[] }).warnings - : undefined, - response: (event as { response?: unknown }).response as - | Parameters[0]['response'] - | undefined, - }); - }, - }); + return { + run, + agent, + abortSignal: controller.signal, + selection, + onStreamFinish: async ({ + messages: updatedMessages, + finishReason, + isAborted: _isAborted, + }: { + messages: AgentUIMessage[]; + finishReason?: string; + isAborted: boolean; + }) => { + const observabilitySummary = observabilityTracker.getSummary(); + try { + const messagesWithApprovalStages = applyApprovalResponsesToFileChangeParts(updatedMessages); + const messagesWithObservability = applyFinalObservabilityToMessages( + messagesWithApprovalStages, + run.uuid, + buildMessageObservabilityMetadataPatch(observabilitySummary) + ); + const persistedMessages = await AgentMessageStore.syncMessages( + thread.uuid, + userIdentity.userId, + messagesWithObservability, + run.uuid + ); + const pendingApprovals = await ApprovalService.syncApprovalRequestsFromMessages({ + thread, + run, + messages: persistedMessages, + }); + await touchSessionActivity(); - return { - run, - agent, - abortSignal: controller.signal, - selection, - onStreamFinish: async ({ - messages: updatedMessages, - finishReason, - isAborted: _isAborted, - }: { - messages: AgentUIMessage[]; - finishReason?: string; - isAborted: boolean; - }) => { - const observabilitySummary = observabilityTracker.getSummary(); - const messagesWithApprovalStages = applyApprovalResponsesToFileChangeParts(updatedMessages); - const messagesWithObservability = applyFinalObservabilityToMessages( - messagesWithApprovalStages, - run.uuid, - buildMessageObservabilityMetadataPatch(observabilitySummary) - ); - const persistedMessages = await AgentMessageStore.syncMessages( - thread.uuid, - userIdentity.userId, - messagesWithObservability, - run.uuid - ); - const pendingApprovals = await ApprovalService.syncApprovalRequestsFromMessages({ - thread, - run, - messages: persistedMessages, - }); - await touchSessionActivity(); + const currentRun = await AgentRunService.getRunByUuid(run.uuid); + if (currentRun?.status === 'cancelled') { + return; + } - const currentRun = await AgentRunService.getRunByUuid(run.uuid); - if (currentRun?.status === 'cancelled') { - return; - } + if (pendingApprovals.length > 0) { + await AgentRunService.patchStatus(run.uuid, 'waiting_for_approval', { + usageSummary: observabilitySummary as Record, + streamState: { + finishReason: finishReason || null, + }, + }); + return; + } - if (pendingApprovals.length > 0) { - await AgentRunService.patchStatus(run.uuid, 'waiting_for_approval', { - usageSummary: observabilitySummary as Record, - streamState: { - finishReason: finishReason || null, - }, - }); - return; - } + const terminalFailure = classifyTerminalRunFailure({ + finishReason, + maxIterations: effectiveSessionConfig.maxIterations, + }); + if (terminalFailure) { + await AgentRunService.markFailed(run.uuid, terminalFailure, observabilitySummary, { + finishReason: finishReason || null, + }); + return; + } - if (finishReason === 'error') { - await AgentRunService.markFailed( - run.uuid, - new Error('Agent stream finished with error'), - observabilitySummary, - { + await AgentRunService.markCompleted(run.uuid, observabilitySummary, { finishReason: finishReason || null, - } - ); - return; - } + }); + } catch (error) { + await AgentRunService.markFailed(run.uuid, error, observabilitySummary, { + finishReason: finishReason || null, + }).catch((runFailureError) => { + getLogger().warn( + { error: runFailureError, runId: run.uuid }, + `AgentExec: stream finalization failure record failed runId=${run.uuid}` + ); + }); - await AgentRunService.markCompleted(run.uuid, observabilitySummary, { - finishReason: finishReason || null, + throw error; + } + }, + }; + } catch (error) { + if (error instanceof SessionWorkspaceGatewayUnavailableError) { + await AgentSessionService.markSessionRuntimeFailure(session.uuid, error).catch((runtimeFailureError) => { + getLogger().warn( + { error: runtimeFailureError, sessionId: session.uuid }, + `Session: runtime failure record failed sessionId=${session.uuid}` + ); }); - }, - }; + } + + if (run) { + await AgentRunService.markFailed(run.uuid, error, observabilityTracker.getSummary()).catch( + (runFailureError) => { + getLogger().warn( + { error: runFailureError, runId: run.uuid }, + `AgentExec: run failure record failed runId=${run.uuid}` + ); + } + ); + } + + throw error; + } } } diff --git a/src/server/services/agent/RunService.ts b/src/server/services/agent/RunService.ts index 50099607..96001dc0 100644 --- a/src/server/services/agent/RunService.ts +++ b/src/server/services/agent/RunService.ts @@ -32,6 +32,47 @@ function cloneChunk(chunk: T): T { return JSON.parse(JSON.stringify(chunk)) as T; } +function serializeRunError(error: unknown): Record { + if (error instanceof Error) { + const typedError = error as Error & { + code?: unknown; + details?: unknown; + }; + const serialized: Record = { + message: error.message, + stack: error.stack || null, + }; + + if (error.name) { + serialized.name = error.name; + } + + if (typedError.code !== undefined) { + serialized.code = typedError.code; + } + + if (typedError.details !== undefined) { + serialized.details = typedError.details; + } + + return serialized; + } + + if (error && typeof error === 'object') { + const record = { ...(error as Record) }; + const message = typeof record.message === 'string' ? record.message.trim() : ''; + + return { + ...record, + message: message || 'Agent run failed.', + }; + } + + return { + message: String(error), + }; +} + function isUuid(value: string): boolean { return UUID_PATTERN.test(value); } @@ -202,15 +243,7 @@ export default class AgentRunService { completedAt: new Date().toISOString(), usageSummary: (usageSummary || {}) as Record, streamState: streamState || {}, - error: - error instanceof Error - ? { - message: error.message, - stack: error.stack || null, - } - : { - message: String(error), - }, + error: serializeRunError(error), }); } diff --git a/src/server/services/agent/__tests__/CapabilityService.test.ts b/src/server/services/agent/__tests__/CapabilityService.test.ts index 37e1dab2..4f79e353 100644 --- a/src/server/services/agent/__tests__/CapabilityService.test.ts +++ b/src/server/services/agent/__tests__/CapabilityService.test.ts @@ -64,6 +64,7 @@ jest.mock('../PolicyService', () => ({ })); import AgentCapabilityService from '../CapabilityService'; +import { SessionWorkspaceGatewayUnavailableError } from '../errors'; describe('AgentCapabilityService.buildToolSet', () => { const session = { @@ -207,7 +208,7 @@ describe('AgentCapabilityService.buildToolSet', () => { expect(mockCallTool).toHaveBeenCalledWith('workspace.read_file', {}, 22000); }); - it('omits session-pod stdio connectors when the sandbox gateway is unavailable', async () => { + it('fails the session tool setup when the sandbox gateway is unavailable', async () => { mockConnect.mockImplementation(async (transport) => { currentTransport = transport as Record; if ( @@ -219,16 +220,17 @@ describe('AgentCapabilityService.buildToolSet', () => { } }); - const tools = await AgentCapabilityService.buildToolSet({ - session, - repoFullName: 'example-org/example-repo', - userIdentity, - approvalPolicy: {} as any, - workspaceToolDiscoveryTimeoutMs: 3000, - workspaceToolExecutionTimeoutMs: 15000, - }); + await expect( + AgentCapabilityService.buildToolSet({ + session, + repoFullName: 'example-org/example-repo', + userIdentity, + approvalPolicy: {} as any, + workspaceToolDiscoveryTimeoutMs: 3000, + workspaceToolExecutionTimeoutMs: 15000, + }) + ).rejects.toBeInstanceOf(SessionWorkspaceGatewayUnavailableError); - expect(tools.mcp__figma__get_design_context).toBeUndefined(); expect(mockLoggerWarn).toHaveBeenCalled(); }); diff --git a/src/server/services/agent/__tests__/RunExecutor.test.ts b/src/server/services/agent/__tests__/RunExecutor.test.ts index 75a1e551..7a4c9ce0 100644 --- a/src/server/services/agent/__tests__/RunExecutor.test.ts +++ b/src/server/services/agent/__tests__/RunExecutor.test.ts @@ -52,6 +52,10 @@ jest.mock('server/services/agent/CapabilityService', () => ({ const mockCreateRun = jest.fn().mockResolvedValue({ id: 11, uuid: 'run-1', status: 'running' }); const mockRegisterAbortController = jest.fn(); const mockPatchRun = jest.fn().mockResolvedValue(undefined); +const mockGetRunByUuid = jest.fn(); +const mockPatchStatus = jest.fn(); +const mockMarkFailed = jest.fn(); +const mockMarkCompleted = jest.fn(); jest.mock('server/services/agent/RunService', () => ({ __esModule: true, @@ -59,10 +63,10 @@ jest.mock('server/services/agent/RunService', () => ({ createRun: (...args: unknown[]) => mockCreateRun(...args), registerAbortController: (...args: unknown[]) => mockRegisterAbortController(...args), patchRun: (...args: unknown[]) => mockPatchRun(...args), - getRunByUuid: jest.fn(), - patchStatus: jest.fn(), - markFailed: jest.fn(), - markCompleted: jest.fn(), + getRunByUuid: (...args: unknown[]) => mockGetRunByUuid(...args), + patchStatus: (...args: unknown[]) => mockPatchStatus(...args), + markFailed: (...args: unknown[]) => mockMarkFailed(...args), + markCompleted: (...args: unknown[]) => mockMarkCompleted(...args), }, })); @@ -82,6 +86,7 @@ jest.mock('server/services/agentSession', () => ({ default: { getSessionAppendSystemPrompt: (...args: unknown[]) => mockGetSessionAppendSystemPrompt(...args), touchActivity: (...args: unknown[]) => mockTouchActivity(...args), + markSessionRuntimeFailure: jest.fn(), }, })); @@ -148,6 +153,14 @@ jest.mock('server/models/AgentPendingAction', () => ({ })); import AgentRunExecutor from 'server/services/agent/RunExecutor'; +import ApprovalService from 'server/services/agent/ApprovalService'; +import AgentMessageStore from 'server/services/agent/MessageStore'; +import AgentSessionService from 'server/services/agentSession'; +import { SessionWorkspaceGatewayUnavailableError } from 'server/services/agent/errors'; + +const mockSyncApprovalRequests = ApprovalService.syncApprovalRequestsFromMessages as jest.Mock; +const mockSyncMessages = AgentMessageStore.syncMessages as jest.Mock; +const mockMarkSessionRuntimeFailure = AgentSessionService.markSessionRuntimeFailure as jest.Mock; describe('AgentRunExecutor', () => { beforeEach(() => { @@ -162,8 +175,13 @@ describe('AgentRunExecutor', () => { mockBuildToolSet.mockResolvedValue({}); mockCreateRun.mockResolvedValue({ id: 11, uuid: 'run-1', status: 'running' }); mockPatchRun.mockResolvedValue(undefined); + mockGetRunByUuid.mockResolvedValue({ id: 11, uuid: 'run-1', status: 'running' }); + mockPatchStatus.mockResolvedValue(undefined); + mockMarkFailed.mockResolvedValue(undefined); + mockMarkCompleted.mockResolvedValue(undefined); mockGetSessionAppendSystemPrompt.mockResolvedValue('Append prompt'); mockTouchActivity.mockResolvedValue(undefined); + mockMarkSessionRuntimeFailure.mockResolvedValue(undefined); mockGetEffectiveSessionConfig.mockResolvedValue({ systemPrompt: 'DB prompt as stored', appendSystemPrompt: undefined, @@ -174,6 +192,8 @@ describe('AgentRunExecutor', () => { }); mockPendingActionFirst.mockResolvedValue(null); mockToolExecutionFirst.mockResolvedValue(undefined); + mockSyncMessages.mockImplementation(async (_threadId, _userId, messages) => messages); + mockSyncApprovalRequests.mockResolvedValue([]); }); it('builds agent instructions from the control-plane and session prompts', async () => { @@ -290,4 +310,133 @@ describe('AgentRunExecutor', () => { }) ); }); + + it('does not create a run when tool setup fails before execution starts', async () => { + mockBuildToolSet.mockRejectedValueOnce(new Error('tool setup failed')); + + await expect( + AgentRunExecutor.execute({ + session: { uuid: 'sess-1' } as any, + thread: { id: 7, uuid: 'thread-1' } as any, + userIdentity: { userId: 'sample-user' } as any, + messages: [], + }) + ).rejects.toThrow('tool setup failed'); + + expect(mockCreateRun).not.toHaveBeenCalled(); + expect(mockMarkFailed).not.toHaveBeenCalled(); + }); + + it('records a runtime session failure when the workspace gateway is unavailable', async () => { + mockBuildToolSet.mockRejectedValueOnce( + new SessionWorkspaceGatewayUnavailableError({ + sessionId: 'sess-1', + cause: new Error('sandbox unavailable'), + }) + ); + + await expect( + AgentRunExecutor.execute({ + session: { uuid: 'sess-1' } as any, + thread: { id: 7, uuid: 'thread-1' } as any, + userIdentity: { userId: 'sample-user' } as any, + messages: [], + }) + ).rejects.toThrow('Session workspace gateway unavailable: sandbox unavailable'); + + expect(mockMarkSessionRuntimeFailure).toHaveBeenCalledWith( + 'sess-1', + expect.any(SessionWorkspaceGatewayUnavailableError) + ); + expect(mockCreateRun).not.toHaveBeenCalled(); + }); + + it('marks the run failed if agent construction throws after the run is created', async () => { + mockToolLoopAgent.mockImplementationOnce(() => { + throw new Error('agent init failed'); + }); + + await expect( + AgentRunExecutor.execute({ + session: { uuid: 'sess-1' } as any, + thread: { id: 7, uuid: 'thread-1' } as any, + userIdentity: { userId: 'sample-user' } as any, + messages: [], + }) + ).rejects.toThrow('agent init failed'); + + expect(mockCreateRun).toHaveBeenCalled(); + expect(mockMarkFailed).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ message: 'agent init failed' }), + expect.any(Object) + ); + }); + + it('marks loop-cap terminal tool-calls as a failed run with structured details', async () => { + const execution = await AgentRunExecutor.execute({ + session: { uuid: 'sess-1', id: 17 } as any, + thread: { id: 7, uuid: 'thread-1' } as any, + userIdentity: { userId: 'sample-user' } as any, + messages: [], + }); + + await execution.onStreamFinish({ + messages: [ + { + id: 'assistant-1', + role: 'assistant', + parts: [{ type: 'text', text: 'Still working' }], + metadata: { runId: 'run-1' }, + } as any, + ], + finishReason: 'tool-calls', + isAborted: false, + }); + + expect(mockMarkFailed).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + code: 'max_iterations_exceeded', + details: expect.objectContaining({ + finishReason: 'tool-calls', + maxIterations: 8, + }), + }), + expect.any(Object), + { + finishReason: 'tool-calls', + } + ); + expect(mockMarkCompleted).not.toHaveBeenCalled(); + }); + + it('marks the run failed if stream finalization persistence throws', async () => { + mockSyncMessages.mockRejectedValueOnce(new Error('message sync failed')); + + const execution = await AgentRunExecutor.execute({ + session: { uuid: 'sess-1', id: 17 } as any, + thread: { id: 7, uuid: 'thread-1' } as any, + userIdentity: { userId: 'sample-user' } as any, + messages: [], + }); + + await expect( + execution.onStreamFinish({ + messages: [], + finishReason: 'stop', + isAborted: false, + }) + ).rejects.toThrow('message sync failed'); + + expect(mockMarkFailed).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ message: 'message sync failed' }), + expect.any(Object), + { + finishReason: 'stop', + } + ); + expect(mockMarkCompleted).not.toHaveBeenCalled(); + }); }); diff --git a/src/server/services/agent/__tests__/ThreadService.test.ts b/src/server/services/agent/__tests__/ThreadService.test.ts new file mode 100644 index 00000000..6e724849 --- /dev/null +++ b/src/server/services/agent/__tests__/ThreadService.test.ts @@ -0,0 +1,98 @@ +/** + * Copyright 2026 GoodRx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const mockAgentSessionQuery = jest.fn(); +const mockAgentThreadQuery = jest.fn(); + +jest.mock('server/models/AgentSession', () => ({ + __esModule: true, + default: { + query: (...args: unknown[]) => mockAgentSessionQuery(...args), + }, +})); + +jest.mock('server/models/AgentThread', () => ({ + __esModule: true, + default: { + query: (...args: unknown[]) => mockAgentThreadQuery(...args), + }, +})); + +import AgentThreadService from 'server/services/agent/ThreadService'; + +describe('AgentThreadService', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('retries a conflicting default-thread insert by returning the concurrent winner', async () => { + const session = { id: 17, uuid: 'session-1', userId: 'user-123' }; + const existingThread = { uuid: 'thread-1', sessionId: 17, isDefault: true }; + + mockAgentSessionQuery.mockReturnValueOnce({ + findOne: jest.fn().mockResolvedValue(session), + }); + mockAgentThreadQuery + .mockReturnValueOnce({ + findOne: jest.fn().mockResolvedValue(null), + }) + .mockReturnValueOnce({ + insertAndFetch: jest.fn().mockRejectedValue(new Error('duplicate key value violates unique constraint')), + }) + .mockReturnValueOnce({ + findOne: jest.fn().mockResolvedValue(existingThread), + }); + + await expect(AgentThreadService.getDefaultThreadForSession('session-1', 'user-123')).resolves.toBe(existingThread); + }); + + it('creates a default thread before listing threads for a session', async () => { + const session = { id: 17, uuid: 'session-1', userId: 'user-123' }; + const createdThread = { uuid: 'thread-1', sessionId: 17, isDefault: true, archivedAt: null }; + const listedThreads = [createdThread]; + + mockAgentSessionQuery + .mockReturnValueOnce({ + findOne: jest.fn().mockResolvedValue(session), + }) + .mockReturnValueOnce({ + findOne: jest.fn().mockResolvedValue(session), + }); + mockAgentThreadQuery + .mockReturnValueOnce({ + findOne: jest.fn().mockResolvedValue(null), + }) + .mockReturnValueOnce({ + insertAndFetch: jest.fn().mockResolvedValue(createdThread), + }) + .mockReturnValueOnce( + (() => { + const query = { + where: jest.fn(() => query), + whereNull: jest.fn(() => query), + orderBy: jest + .fn() + .mockImplementationOnce(() => query) + .mockImplementationOnce(() => Promise.resolve(listedThreads)), + }; + + return query; + })() + ); + + await expect(AgentThreadService.listThreadsForSession('session-1', 'user-123')).resolves.toEqual(listedThreads); + }); +}); diff --git a/src/server/services/agent/errors.ts b/src/server/services/agent/errors.ts new file mode 100644 index 00000000..1e63628d --- /dev/null +++ b/src/server/services/agent/errors.ts @@ -0,0 +1,49 @@ +/** + * Copyright 2026 GoodRx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +function normalizeErrorMessage(error: unknown, fallback: string): string { + if (error instanceof Error && error.message.trim()) { + return error.message.trim(); + } + + if (typeof error === 'string' && error.trim()) { + return error.trim(); + } + + return fallback; +} + +export class AgentRunTerminalFailure extends Error { + readonly code: string; + readonly details?: Record; + + constructor({ code, message, details }: { code: string; message: string; details?: Record }) { + super(message); + this.name = 'AgentRunTerminalFailure'; + this.code = code; + this.details = details; + } +} + +export class SessionWorkspaceGatewayUnavailableError extends Error { + readonly sessionId: string; + + constructor({ sessionId, cause }: { sessionId: string; cause: unknown }) { + super(`Session workspace gateway unavailable: ${normalizeErrorMessage(cause, 'Connection failed.')}`); + this.name = 'SessionWorkspaceGatewayUnavailableError'; + this.sessionId = sessionId; + } +} diff --git a/src/server/services/agentSandboxSession.ts b/src/server/services/agentSandboxSession.ts index d78beada..fefef87a 100644 --- a/src/server/services/agentSandboxSession.ts +++ b/src/server/services/agentSandboxSession.ts @@ -81,6 +81,10 @@ function normalizeRepoKey(value: string): string { return value.trim().toLowerCase(); } +function elapsedMs(startedAt: number): number { + return Math.max(Date.now() - startedAt, 0); +} + export function formatRequestedSandboxServiceLabel(service?: RequestedSandboxService | null): string { if (!service) { return 'unknown service'; @@ -163,7 +167,10 @@ export default class AgentSandboxSessionService extends BaseService { private readonly deployService = new DeployService(this.db, this.redis, this.redlock, this.queueManager); async launch(opts: LaunchSandboxSessionOptions): Promise { + const launchStartedAt = Date.now(); + const resolveCandidatesStartedAt = Date.now(); const { baseBuild, environmentSource, lifecycleConfig, candidates } = await this.loadBaseBuildAndCandidates(opts); + const candidateResolutionMs = elapsedMs(resolveCandidatesStartedAt); if (candidates.length === 0) { throw new Error( `No dev-mode sandboxable services were found in ${environmentSource.repo}:${environmentSource.branch}` @@ -201,21 +208,24 @@ export default class AgentSandboxSessionService extends BaseService { getLogger().info( `Sandbox: starting baseBuildUuid=${opts.baseBuildUuid} services=${selectedServices .map((service) => `${service.name}@${service.serviceRepo}:${service.serviceBranch}`) - .join(',')}` + .join(',')} candidateResolutionMs=${candidateResolutionMs}` ); await opts.onProgress?.('creating_sandbox_build', `Creating sandbox build for ${selectedServiceSummary}`); + const buildCloneStartedAt = Date.now(); const { build: sandboxBuild, sandboxDeploysByBaseDeployId } = await this.createSandboxBuild({ baseBuild, environmentSource, selectedServices, }); + const sandboxBuildMs = elapsedMs(buildCloneStartedAt); try { const runUUID = nanoid(); await sandboxBuild.$query().patch({ runUUID, status: BuildStatus.QUEUED }); await sandboxBuild.$fetchGraph('[environment, pullRequest.[repository], deploys.[deployable, repository]]'); + const deployStartedAt = Date.now(); await opts.onProgress?.('resolving_environment', `Resolving environment variables for ${selectedServiceSummary}`); await new BuildEnvironmentVariables(this.db).resolve(sandboxBuild); @@ -238,6 +248,7 @@ export default class AgentSandboxSessionService extends BaseService { if (!deployed) { throw new Error(`Sandbox deployment failed for ${sandboxBuild.uuid}`); } + const deployMs = elapsedMs(deployStartedAt); const selectedSandboxServices = this.resolveSelectedSandboxDeploys( selectedServices, @@ -245,6 +256,7 @@ export default class AgentSandboxSessionService extends BaseService { ); await opts.onProgress?.('opening_session', `Opening sandbox for ${selectedServiceSummary}`); + const openSessionStartedAt = Date.now(); const session = await AgentSessionService.createSession({ userId: opts.userId, buildUuid: sandboxBuild.uuid, @@ -277,11 +289,14 @@ export default class AgentSandboxSessionService extends BaseService { resources: mergeAgentSessionResources(opts.resources, lifecycleConfig.environment?.agentSession?.resources), userIdentity: opts.userIdentity, }); + const sessionOpenMs = elapsedMs(openSessionStartedAt); getLogger().info( `Sandbox: ready baseBuildUuid=${opts.baseBuildUuid} buildUuid=${sandboxBuild.uuid} sessionId=${ session.uuid - } services=${selectedServices.map((service) => service.name).join(',')}` + } services=${selectedServices.map((service) => service.name).join(',')} durationMs=${elapsedMs( + launchStartedAt + )} candidateResolutionMs=${candidateResolutionMs} sandboxBuildMs=${sandboxBuildMs} deployMs=${deployMs} sessionOpenMs=${sessionOpenMs}` ); return { @@ -368,47 +383,48 @@ export default class AgentSandboxSessionService extends BaseService { ): Promise { const configCache = new Map>(); const activeDeploys = this.getActiveDeploys(baseBuild); - const resolvedCandidates: ResolvedSandboxService[] = []; + const resolvedCandidates = await Promise.all( + this.getEnvironmentServiceReferences(lifecycleConfig).map(async (serviceRef) => { + const serviceName = serviceRef.name; + if (!serviceName) { + return null; + } - for (const serviceRef of this.getEnvironmentServiceReferences(lifecycleConfig)) { - const serviceName = serviceRef.name; - if (!serviceName) { - continue; - } + const baseDeploy = this.findActiveDeployForReference(activeDeploys, serviceRef); + if (!baseDeploy) { + return null; + } - const baseDeploy = this.findActiveDeployForReference(activeDeploys, serviceRef); - if (!baseDeploy) { - continue; - } + try { + const serviceSource = await this.resolveServiceSource({ + serviceRef, + baseDeploy, + fallbackSource: environmentSource, + configCache, + }); - try { - const serviceSource = await this.resolveServiceSource({ - serviceRef, - baseDeploy, - fallbackSource: environmentSource, - configCache, - }); + if (!serviceSource.yamlService?.dev || !hasLifecycleManagedDockerBuild(serviceSource.yamlService)) { + return null; + } - if (!serviceSource.yamlService?.dev || !hasLifecycleManagedDockerBuild(serviceSource.yamlService)) { - continue; + return { + name: serviceSource.yamlService.name, + devConfig: serviceSource.yamlService.dev, + baseDeploy, + serviceRepo: serviceSource.repo, + serviceBranch: serviceSource.branch, + yamlService: serviceSource.yamlService, + }; + } catch (error) { + getLogger({ buildUuid: baseBuild.uuid, serviceName, error }).warn( + `Sandbox: candidate skipped service=${serviceName} buildUuid=${baseBuild.uuid} reason=config_error` + ); + return null; } + }) + ); - resolvedCandidates.push({ - name: serviceSource.yamlService.name, - devConfig: serviceSource.yamlService.dev, - baseDeploy, - serviceRepo: serviceSource.repo, - serviceBranch: serviceSource.branch, - yamlService: serviceSource.yamlService, - }); - } catch (error) { - getLogger({ buildUuid: baseBuild.uuid, serviceName, error }).warn( - `Sandbox: candidate skipped service=${serviceName} buildUuid=${baseBuild.uuid} reason=config_error` - ); - } - } - - return resolvedCandidates; + return resolvedCandidates.filter((candidate): candidate is ResolvedSandboxService => Boolean(candidate)); } private resolveSelectedServices( diff --git a/src/server/services/agentSession.ts b/src/server/services/agentSession.ts index 725fdee6..d8c82586 100644 --- a/src/server/services/agentSession.ts +++ b/src/server/services/agentSession.ts @@ -103,6 +103,7 @@ const SESSION_REDIS_TTL = 7200; const ACTIVE_ENVIRONMENT_SESSION_UNIQUE_INDEX = 'agent_sessions_active_environment_build_unique'; const DEV_MODE_REDEPLOY_GRAPH = '[deployable.[repository], repository, service, build.[pullRequest.[repository]]]'; const SESSION_DEPLOY_GRAPH = '[deployable, repository, service]'; +const agentNetworkPolicySetupByNamespace = new Map>(); type AgentSessionSummaryRecordBase = AgentSession & { id: string; @@ -145,6 +146,38 @@ export class ActiveEnvironmentSessionError extends Error { } } +function elapsedMs(startedAt: number): number { + return Math.max(Date.now() - startedAt, 0); +} + +async function ensureAgentNetworkPolicy(namespace: string): Promise { + let setupPromise = agentNetworkPolicySetupByNamespace.get(namespace); + if (!setupPromise) { + setupPromise = (async () => { + const kc = new k8s.KubeConfig(); + kc.loadFromDefault(); + const netApi = kc.makeApiClient(k8s.NetworkingV1Api); + const policy = buildAgentNetworkPolicy(namespace); + await netApi.createNamespacedNetworkPolicy(namespace, policy).catch((err: any) => { + if (err?.statusCode !== 409) { + throw err; + } + }); + })(); + agentNetworkPolicySetupByNamespace.set(namespace, setupPromise); + } + + try { + await setupPromise; + } catch (error) { + if (agentNetworkPolicySetupByNamespace.get(namespace) === setupPromise) { + agentNetworkPolicySetupByNamespace.delete(namespace); + } + + throw error; + } +} + async function restoreDeploys(deploys: Deploy[]): Promise { if (deploys.length === 0) { return; @@ -791,6 +824,7 @@ export default class AgentSessionService { } static async createSession(opts: CreateSessionOptions) { + const sessionStartedAt = Date.now(); const sessionUuid = uuid(); const buildKind = opts.buildKind || BuildKind.ENVIRONMENT; const podName = buildAgentSessionPodName(sessionUuid, opts.buildUuid); @@ -823,64 +857,61 @@ export default class AgentSessionService { services: resolvedServices || [], }); const primaryWorkspaceRepo = workspaceRepos.find((repo) => repo.primary) || workspaceRepos[0]; + const providerUserIdentity = { + userId: opts.userId, + githubUsername: opts.userIdentity?.githubUsername || null, + }; + const preflightStartedAt = Date.now(); const selection = await AgentProviderRegistry.resolveSelection({ repoFullName: primaryWorkspaceRepo?.repo, requestedModelId, }); resolvedModelId = selection.modelId; - await AgentProviderRegistry.getRequiredStoredApiKey({ - provider: selection.provider, - userIdentity: { - userId: opts.userId, - githubUsername: opts.userIdentity?.githubUsername || null, - }, - requestApiKey: opts.requestApiKey, - requestApiKeyProvider: opts.requestApiKeyProvider, - }); - const providerApiKeys = await AgentProviderRegistry.resolveCredentialEnvMap({ - repoFullName: primaryWorkspaceRepo?.repo, - userIdentity: { - userId: opts.userId, - githubUsername: opts.userIdentity?.githubUsername || null, - }, - requestApiKey: opts.requestApiKey, - requestApiKeyProvider: opts.requestApiKeyProvider, - }); - const sessionPodMcpConfigJson = primaryWorkspaceRepo?.repo - ? serializeSessionWorkspaceGatewayServers( - await new McpConfigService().resolveSessionPodServersForRepo( + const resolvedServiceNames = (resolvedServices || []).map((service) => service.name); + const [, providerApiKeys, sessionPodServers, compatiblePrewarm, forwardedAgentEnv] = await Promise.all([ + AgentProviderRegistry.getRequiredStoredApiKey({ + provider: selection.provider, + userIdentity: providerUserIdentity, + requestApiKey: opts.requestApiKey, + requestApiKeyProvider: opts.requestApiKeyProvider, + }), + AgentProviderRegistry.resolveCredentialEnvMap({ + repoFullName: primaryWorkspaceRepo?.repo, + userIdentity: providerUserIdentity, + requestApiKey: opts.requestApiKey, + requestApiKeyProvider: opts.requestApiKeyProvider, + }), + primaryWorkspaceRepo?.repo + ? new McpConfigService().resolveSessionPodServersForRepo( primaryWorkspaceRepo.repo, undefined, opts.userIdentity || null ) - ) - : '[]'; - const resolvedServiceNames = (resolvedServices || []).map((service) => service.name); - const compatiblePrewarm = + : Promise.resolve([]), workspaceRepos.length === 1 - ? await resolveCompatiblePrewarm( + ? resolveCompatiblePrewarm( opts.buildUuid, resolvedServiceNames, primaryWorkspaceRepo?.revision || opts.revision ) - : null; + : Promise.resolve(null), + resolveForwardedAgentEnv(resolvedServices, opts.namespace, sessionUuid, opts.buildUuid), + ]); + const sessionPodMcpConfigJson = serializeSessionWorkspaceGatewayServers(sessionPodServers); const pvcName = compatiblePrewarm?.pvcName || `agent-pvc-${sessionUuid.slice(0, 8)}`; - const forwardedAgentEnv = await resolveForwardedAgentEnv( - resolvedServices, - opts.namespace, - sessionUuid, - opts.buildUuid - ); const forwardedPlainAgentEnv = Object.fromEntries( Object.entries(forwardedAgentEnv.env).filter( ([envKey]) => !forwardedAgentEnv.secretRefs.some((secretRef) => secretRef.envKey === envKey) ) ); + const preflightMs = elapsedMs(preflightStartedAt); logger().info( `Session: starting sessionId=${sessionUuid} buildKind=${buildKind} namespace=${opts.namespace} buildUuid=${ opts.buildUuid || 'none' - } services=${resolvedServiceNames.join(',') || 'none'} prewarm=${compatiblePrewarm ? 'reused' : 'new'}` + } services=${resolvedServiceNames.join(',') || 'none'} prewarm=${ + compatiblePrewarm ? 'reused' : 'new' + } preflightMs=${preflightMs}` ); try { @@ -906,7 +937,9 @@ export default class AgentSessionService { } as unknown as Partial); sessionPersisted = true; - const [, , agentServiceAccountName] = await Promise.all([ + const combinedInstallCommand = buildCombinedInstallCommand(resolvedServices); + const infraSetupStartedAt = Date.now(); + const [, , agentServiceAccountName, useGvisor] = await Promise.all([ compatiblePrewarm ? Promise.resolve(null) : createAgentPvc(opts.namespace, pvcName, '10Gi', opts.buildUuid), createAgentApiKeySecret( opts.namespace, @@ -920,12 +953,12 @@ export default class AgentSessionService { } ), ensureAgentSessionServiceAccount(opts.namespace), + isGvisorAvailable(), ]); - - const useGvisor = await isGvisorAvailable(); - const combinedInstallCommand = buildCombinedInstallCommand(resolvedServices); + const infraSetupMs = elapsedMs(infraSetupStartedAt); failureStage = 'connect_runtime'; + const podStartupStartedAt = Date.now(); const workspacePod = await createSessionWorkspacePod({ podName, namespace: opts.namespace, @@ -954,6 +987,7 @@ export default class AgentSessionService { serviceAccountName: agentServiceAccountName, resources: opts.resources, }); + const podStartupMs = elapsedMs(podStartupStartedAt); const agentNodeName = workspacePod.spec?.nodeName || null; if ((resolvedServices || []).length > 0 && keepAttachedServicesOnSessionNode && !agentNodeName) { @@ -985,34 +1019,34 @@ export default class AgentSessionService { } as unknown as Partial); } - for (const service of enabledServices) { - await Deploy.query().findById(service.deployId).patch({ - devMode: true, - devModeSessionId: session.id, - }); - persistedDevModeDeployIds.push(service.deployId); - } - - await createSessionWorkspaceService(opts.namespace, podName, opts.buildUuid); - - const kc = new k8s.KubeConfig(); - kc.loadFromDefault(); - const netApi = kc.makeApiClient(k8s.NetworkingV1Api); - const policy = buildAgentNetworkPolicy(opts.namespace); - await netApi.createNamespacedNetworkPolicy(opts.namespace, policy).catch((err: any) => { - if (err?.statusCode !== 409) throw err; - }); - await redis.setex( - `${SESSION_REDIS_PREFIX}${sessionUuid}`, - SESSION_REDIS_TTL, - JSON.stringify({ podName, namespace: opts.namespace, status: 'active' }) + await Promise.all( + enabledServices.map((service) => + Deploy.query().findById(service.deployId).patch({ + devMode: true, + devModeSessionId: session.id, + }) + ) ); + persistedDevModeDeployIds.push(...enabledServices.map((service) => service.deployId)); - await AgentSession.query() - .findById(session.id) - .patch({ - status: 'active', - } as unknown as Partial); + const finalizeStartedAt = Date.now(); + await Promise.all([ + createSessionWorkspaceService(opts.namespace, podName, opts.buildUuid), + ensureAgentNetworkPolicy(opts.namespace), + ]); + await Promise.all([ + redis.setex( + `${SESSION_REDIS_PREFIX}${sessionUuid}`, + SESSION_REDIS_TTL, + JSON.stringify({ podName, namespace: opts.namespace, status: 'active' }) + ), + AgentSession.query() + .findById(session.id) + .patch({ + status: 'active', + } as unknown as Partial), + ]); + const finalizeMs = elapsedMs(finalizeStartedAt); session = { ...session, @@ -1024,12 +1058,19 @@ export default class AgentSessionService { logger().info( `Session: ready sessionId=${sessionUuid} namespace=${opts.namespace} podName=${podName} services=${ resolvedServiceNames.join(',') || 'none' - } prewarm=${compatiblePrewarm ? 'reused' : 'new'}` + } prewarm=${compatiblePrewarm ? 'reused' : 'new'} durationMs=${elapsedMs( + sessionStartedAt + )} preflightMs=${preflightMs} infraMs=${infraSetupMs} podMs=${podStartupMs} finalizeMs=${finalizeMs}` ); - const AgentThreadService = (await import('server/services/agent/ThreadService')).default; const readySession = session; - await AgentThreadService.getDefaultThreadForSession(readySession.uuid, opts.userId).catch((error: unknown) => { + // Default-thread creation stays best-effort here so chat readiness does not + // depend on secondary DB work; ThreadService.listThreadsForSession() will + // create or retry the default thread on first access if this async warm-up fails. + void (async () => { + const AgentThreadService = (await import('server/services/agent/ThreadService')).default; + await AgentThreadService.getDefaultThreadForSession(readySession.uuid, opts.userId); + })().catch((error: unknown) => { logger().warn( { error, sessionId: readySession.uuid }, `Session: default thread creation skipped sessionId=${readySession.uuid}` diff --git a/sysops/workspace-gateway/skills-bootstrap.mjs b/sysops/workspace-gateway/skills-bootstrap.mjs index 4c946024..179f9158 100644 --- a/sysops/workspace-gateway/skills-bootstrap.mjs +++ b/sysops/workspace-gateway/skills-bootstrap.mjs @@ -52,7 +52,7 @@ async function runGit(args, cwd) { } async function ensureRepoAtBranch(repo) { - const repoKey = buildSkillSourceRepoKey(repo.repo); + const repoKey = buildSkillSourceRepoKey(repo.repo, repo.branch); const sourceRoot = resolve(SKILL_SOURCES_ROOT, repoKey); const gitDir = resolve(sourceRoot, '.git'); @@ -94,54 +94,73 @@ async function ensureRepoAtBranch(repo) { async function main() { const skillPlan = parseSkillPlanArg(); const repos = new Map(); - const skills = []; await mkdir(SKILLS_ROOT, { recursive: true }); for (const skill of skillPlan.skills) { const repoKey = `${skill.repo}::${skill.branch}`; - if (!repos.has(repoKey)) { - repos.set( + if (repos.has(repoKey)) { + continue; + } + + repos.set(repoKey, null); + } + + const preparedRepos = await Promise.all( + Array.from(repos.keys()).map(async (repoKey) => { + const [repo, branch] = repoKey.split('::'); + const skill = skillPlan.skills.find((entry) => entry.repo === repo && entry.branch === branch); + if (!skill) { + throw new Error(`Missing skill metadata for ${repoKey}`); + } + + return [ repoKey, await ensureRepoAtBranch({ repo: skill.repo, repoUrl: skill.repoUrl, branch: skill.branch, - }) - ); - } - } - - for (const skill of skillPlan.skills) { - const sourceRepo = repos.get(`${skill.repo}::${skill.branch}`); - if (!sourceRepo) { - throw new Error(`Skill repo was not prepared for ${skill.repo}@${skill.branch}`); - } - - const normalizedPath = normalizeRelativeSkillPath(skill.path); - const skillDir = resolve(sourceRepo.sourceRoot, normalizedPath); - if (!isWithinRoot(skillDir, sourceRepo.sourceRoot)) { - throw new Error(`Skill path must stay within source repo: ${skill.path}`); - } - - const skillFile = resolve(skillDir, 'SKILL.md'); - const skillFileStat = await stat(skillFile).catch(() => null); - if (!skillFileStat?.isFile()) { - throw new Error(`Missing SKILL.md for ${skill.repo}@${skill.branch}:${normalizedPath}`); - } - - const metadata = await readSkillMetadata(skillFile, normalizedPath.split('/').pop() || normalizedPath); + }), + ]; + }) + ); - skills.push({ - ...skill, - path: normalizedPath, - sourceRoot: toSessionRelativePath(sourceRepo.sourceRoot), - shortName: metadata.shortName, - title: metadata.title, - description: metadata.description, - }); + for (const [repoKey, preparedRepo] of preparedRepos) { + repos.set(repoKey, preparedRepo); } + const skills = await Promise.all( + skillPlan.skills.map(async (skill) => { + const sourceRepo = repos.get(`${skill.repo}::${skill.branch}`); + if (!sourceRepo) { + throw new Error(`Skill repo was not prepared for ${skill.repo}@${skill.branch}`); + } + + const normalizedPath = normalizeRelativeSkillPath(skill.path); + const skillDir = resolve(sourceRepo.sourceRoot, normalizedPath); + if (!isWithinRoot(skillDir, sourceRepo.sourceRoot)) { + throw new Error(`Skill path must stay within source repo: ${skill.path}`); + } + + const skillFile = resolve(skillDir, 'SKILL.md'); + const skillFileStat = await stat(skillFile).catch(() => null); + if (!skillFileStat?.isFile()) { + throw new Error(`Missing SKILL.md for ${skill.repo}@${skill.branch}:${normalizedPath}`); + } + + const metadata = await readSkillMetadata(skillFile, normalizedPath.split('/').pop() || normalizedPath); + + return { + ...skill, + path: normalizedPath, + sourceRoot: toSessionRelativePath(sourceRepo.sourceRoot), + shortName: metadata.shortName, + title: metadata.title, + description: metadata.description, + }; + }) + ); + await writeFile( SKILLS_INDEX_PATH, JSON.stringify( diff --git a/sysops/workspace-gateway/skills-lib.mjs b/sysops/workspace-gateway/skills-lib.mjs index ba472b27..2ce2d97b 100644 --- a/sysops/workspace-gateway/skills-lib.mjs +++ b/sysops/workspace-gateway/skills-lib.mjs @@ -23,8 +23,10 @@ export function normalizeRelativeSkillPath(inputPath) { .replace(/\/+$/, ''); } -export function buildSkillSourceRepoKey(repo) { - return repo.trim().replace(/[^a-zA-Z0-9._-]+/g, '__'); +export function buildSkillSourceRepoKey(repo, branch) { + const repoPart = repo.trim(); + const branchPart = typeof branch === 'string' && branch.trim() ? `branch-${branch.trim()}` : ''; + return [repoPart, branchPart].filter(Boolean).join('__').replace(/[^a-zA-Z0-9._-]+/g, '__'); } export function isWithinRoot(candidatePath, rootPath) { diff --git a/sysops/workspace-gateway/skills-lib.test.mjs b/sysops/workspace-gateway/skills-lib.test.mjs new file mode 100644 index 00000000..7e5430b9 --- /dev/null +++ b/sysops/workspace-gateway/skills-lib.test.mjs @@ -0,0 +1,22 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { buildSkillSourceRepoKey } from './skills-lib.mjs'; + +test('buildSkillSourceRepoKey separates same-repo different-branch checkouts', () => { + assert.notEqual( + buildSkillSourceRepoKey('example-org/example-repo', 'feature/one'), + buildSkillSourceRepoKey('example-org/example-repo', 'feature/two') + ); +}); + +test('buildSkillSourceRepoKey stays stable for the same repo and branch', () => { + assert.equal( + buildSkillSourceRepoKey('example-org/example-repo', 'feature/one'), + buildSkillSourceRepoKey('example-org/example-repo', 'feature/one') + ); +}); + +test('buildSkillSourceRepoKey keeps repo-only callers working', () => { + assert.equal(buildSkillSourceRepoKey('example-org/example-repo'), 'example-org__example-repo'); +});