Skip to content

Commit db07ecc

Browse files
committed
Revert "refactor(backend): decouple tool execution from WebSocket layer"
This reverts commit c710a2d.
1 parent 5833d37 commit db07ecc

File tree

8 files changed

+73
-29
lines changed

8 files changed

+73
-29
lines changed

backend/src/__tests__/run-programmatic-step.test.ts

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@ import {
1515
clearAgentGeneratorCache,
1616
runProgrammaticStep,
1717
} from '../run-programmatic-step'
18-
import { mockFileContext } from './test-utils'
18+
import { mockFileContext, MockWebSocket } from './test-utils'
1919
import * as agentRun from '../agent-run'
2020
import * as toolExecutor from '../tools/tool-executor'
2121
import * as requestContext from '../websockets/request-context'
22+
import * as websocketAction from '../websockets/websocket-action'
2223

2324
import type { AgentTemplate, StepGenerator } from '../templates/types'
2425
import type { PublicAgentState } from '@codebuff/common/types/agent-template'
@@ -28,7 +29,7 @@ import type {
2829
} from '@codebuff/common/types/messages/content-part'
2930
import type { AgentState } from '@codebuff/common/types/session-state'
3031
import type { Logger } from '@codebuff/common/types/contracts/logger'
31-
import type { SendSubagentChunkFn } from '@codebuff/common/types/contracts/client'
32+
import type { WebSocket } from 'ws'
3233

3334
const logger: Logger = {
3435
debug: () => {},
@@ -44,8 +45,7 @@ describe('runProgrammaticStep', () => {
4445
let executeToolCallSpy: any
4546
let getRequestContextSpy: any
4647
let addAgentStepSpy: any
47-
let sendSubagentChunk: SendSubagentChunkFn
48-
let sentSubagentChunks: Parameters<SendSubagentChunkFn>[0][]
48+
let sendActionSpy: any
4949

5050
beforeEach(() => {
5151
// Mock analytics
@@ -72,10 +72,10 @@ describe('runProgrammaticStep', () => {
7272
async () => 'test-step-id',
7373
)
7474

75-
sentSubagentChunks = []
76-
sendSubagentChunk = (data) => {
77-
sentSubagentChunks.push(data)
78-
}
75+
// Mock sendAction
76+
sendActionSpy = spyOn(websocketAction, 'sendAction').mockImplementation(
77+
() => {},
78+
)
7979

8080
// Mock crypto.randomUUID
8181
spyOn(crypto, 'randomUUID').mockImplementation(
@@ -132,10 +132,10 @@ describe('runProgrammaticStep', () => {
132132
fileContext: mockFileContext,
133133
assistantMessage: undefined,
134134
assistantPrefix: undefined,
135+
ws: new MockWebSocket() as unknown as WebSocket,
135136
localAgentTemplates: {},
136137
stepsComplete: false,
137138
stepNumber: 1,
138-
sendSubagentChunk,
139139
logger,
140140
}
141141
})
@@ -226,6 +226,14 @@ describe('runProgrammaticStep', () => {
226226
mockTemplate.handleSteps = () => mockGenerator
227227
mockTemplate.toolNames = ['add_message', 'read_files', 'end_turn']
228228

229+
// Track chunks sent via sendSubagentChunk
230+
const sentChunks: string[] = []
231+
sendActionSpy.mockImplementation((ws: any, action: any) => {
232+
if (action.type === 'subagent-response-chunk') {
233+
sentChunks.push(action.chunk)
234+
}
235+
})
236+
229237
const result = await runProgrammaticStep(mockParams)
230238

231239
// Verify add_message tool was executed
@@ -245,16 +253,15 @@ describe('runProgrammaticStep', () => {
245253
)
246254

247255
// Check that no tool call chunk was sent for add_message
248-
const addMessageToolCallChunk = sentSubagentChunks.find(
249-
({ chunk }) =>
256+
const addMessageToolCallChunk = sentChunks.find(
257+
(chunk) =>
250258
chunk.includes('add_message') && chunk.includes('Hello world'),
251259
)
252260
expect(addMessageToolCallChunk).toBeUndefined()
253261

254262
// Check that tool call chunk WAS sent for read_files (normal behavior)
255-
const readFilesToolCallChunk = sentSubagentChunks.find(
256-
({ chunk }) =>
257-
chunk.includes('read_files') && chunk.includes('test.txt'),
263+
const readFilesToolCallChunk = sentChunks.find(
264+
(chunk) => chunk.includes('read_files') && chunk.includes('test.txt'),
258265
)
259266
expect(readFilesToolCallChunk).toBeDefined()
260267

backend/src/__tests__/sandbox-generator.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@ import {
88
clearAgentGeneratorCache,
99
runProgrammaticStep,
1010
} from '../run-programmatic-step'
11-
import { mockFileContext } from './test-utils'
11+
import { mockFileContext, MockWebSocket } from './test-utils'
1212
import * as agentRun from '../agent-run'
1313
import * as requestContext from '../websockets/request-context'
14+
import * as websocketAction from '../websockets/websocket-action'
1415

1516
import type { AgentTemplate } from '../templates/types'
1617
import type { Logger } from '@codebuff/common/types/contracts/logger'
17-
import type { SendSubagentChunkFn } from '@codebuff/common/types/contracts/client'
18+
import type { WebSocket } from 'ws'
1819

1920
const logger: Logger = {
2021
debug: () => {},
@@ -27,7 +28,6 @@ describe('QuickJS Sandbox Generator', () => {
2728
let mockAgentState: AgentState
2829
let mockParams: any
2930
let mockTemplate: AgentTemplate
30-
let sendSubagentChunk: SendSubagentChunkFn
3131

3232
beforeEach(() => {
3333
clearAgentGeneratorCache({ logger })
@@ -39,11 +39,11 @@ describe('QuickJS Sandbox Generator', () => {
3939
spyOn(requestContext, 'getRequestContext').mockImplementation(() => ({
4040
processedRepoId: 'test-repo-id',
4141
}))
42+
spyOn(websocketAction, 'sendAction').mockImplementation(() => {})
4243
spyOn(crypto, 'randomUUID').mockImplementation(
4344
() =>
4445
'mock-uuid-0000-0000-0000-000000000000' as `${string}-${string}-${string}-${string}-${string}`,
4546
)
46-
sendSubagentChunk = () => {}
4747

4848
// Reuse common test data structure
4949
mockAgentState = {
@@ -91,10 +91,10 @@ describe('QuickJS Sandbox Generator', () => {
9191
fileContext: mockFileContext,
9292
assistantMessage: undefined,
9393
assistantPrefix: undefined,
94+
ws: new MockWebSocket() as unknown as WebSocket,
9495
localAgentTemplates: {},
9596
stepsComplete: false,
9697
stepNumber: 1,
97-
sendSubagentChunk,
9898
logger,
9999
}
100100
})

backend/src/__tests__/spawn-agents-permissions.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -739,12 +739,12 @@ describe('Spawn Agents Permissions', () => {
739739
writeToClient: () => {},
740740
getLatestState: () => ({ messages: [] }),
741741
state: {
742-
// Missing required fields like fingerprintId, messages, etc.
742+
// Missing required fields like ws, fingerprintId, etc.
743743
agentTemplate: parentAgent,
744744
localAgentTemplates: {},
745745
},
746746
})
747-
}).toThrow('Missing fingerprintId in state')
747+
}).toThrow('Missing WebSocket in state')
748748
expect(mockLoopAgentSteps).not.toHaveBeenCalled()
749749
})
750750
})

backend/src/run-programmatic-step.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,15 @@ import { addAgentStep } from './agent-run'
66
import { executeToolCall } from './tools/tool-executor'
77
import { SandboxManager } from './util/quickjs-sandbox'
88
import { getRequestContext } from './websockets/request-context'
9+
import { sendAction } from './websockets/websocket-action'
910

1011
import type { CodebuffToolCall } from '@codebuff/common/tools/list'
1112
import type {
1213
AgentTemplate,
1314
StepGenerator,
1415
PublicAgentState,
1516
} from '@codebuff/common/types/agent-template'
16-
import type {
17-
HandleStepsLogChunkFn,
18-
SendSubagentChunkFn,
19-
} from '@codebuff/common/types/contracts/client'
17+
import type { HandleStepsLogChunkFn } from '@codebuff/common/types/contracts/client'
2018
import type { Logger } from '@codebuff/common/types/contracts/logger'
2119
import type {
2220
ParamsExcluding,
@@ -28,6 +26,7 @@ import type {
2826
} from '@codebuff/common/types/messages/content-part'
2927
import type { PrintModeEvent } from '@codebuff/common/types/print-mode'
3028
import type { AgentState } from '@codebuff/common/types/session-state'
29+
import type { WebSocket } from 'ws'
3130

3231
// Global sandbox manager for QuickJS contexts
3332
const sandboxManager = new SandboxManager()
@@ -60,10 +59,10 @@ export async function runProgrammaticStep(
6059
userInputId: string
6160
fingerprintId: string
6261
onResponseChunk: (chunk: string | PrintModeEvent) => void
62+
ws: WebSocket
6363
localAgentTemplates: Record<string, AgentTemplate>
6464
stepsComplete: boolean
6565
stepNumber: number
66-
sendSubagentChunk: SendSubagentChunkFn | undefined
6766
handleStepsLogChunk: HandleStepsLogChunkFn
6867
logger: Logger
6968
} & ParamsExcluding<
@@ -92,9 +91,9 @@ export async function runProgrammaticStep(
9291
userInputId,
9392
fingerprintId,
9493
onResponseChunk,
94+
ws,
9595
localAgentTemplates,
9696
stepsComplete,
97-
sendSubagentChunk,
9897
handleStepsLogChunk,
9998
logger,
10099
} = params
@@ -180,13 +179,25 @@ export async function runProgrammaticStep(
180179
const toolCalls: CodebuffToolCall[] = []
181180
const toolResults: ToolResultPart[] = []
182181
const state = {
182+
ws,
183183
fingerprintId,
184184
userId,
185185
repoId,
186186
agentTemplate: template,
187187
localAgentTemplates,
188188
system,
189-
sendSubagentChunk,
189+
sendSubagentChunk: (data: {
190+
userInputId: string
191+
agentId: string
192+
agentType: string
193+
chunk: string
194+
prompt?: string
195+
}) => {
196+
sendAction(ws, {
197+
type: 'subagent-response-chunk',
198+
...data,
199+
})
200+
},
190201
agentState: cloneDeep({
191202
...agentState,
192203
runId: agentState.runId!, // We've already verified runId exists above

backend/src/tools/handlers/tool/read-files.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import type {
99
import type { ParamsExcluding } from '@codebuff/common/types/function-params'
1010
import type { Message } from '@codebuff/common/types/messages/codebuff-message'
1111
import type { ProjectFileContext } from '@codebuff/common/util/file'
12+
import type { WebSocket } from 'ws'
13+
1214
type ToolName = 'read_files'
1315
export const handleReadFiles = ((
1416
params: {
@@ -19,6 +21,7 @@ export const handleReadFiles = ((
1921
fileContext: ProjectFileContext
2022

2123
state: {
24+
ws?: WebSocket
2225
userId?: string
2326
fingerprintId?: string
2427
repoId?: string
@@ -36,8 +39,11 @@ export const handleReadFiles = ((
3639
fileContext,
3740
state,
3841
} = params
39-
const { fingerprintId, userId, repoId, messages } = state
42+
const { ws, fingerprintId, userId, repoId, messages } = state
4043
const { paths } = toolCall.input
44+
if (!ws) {
45+
throw new Error('Internal error for read_files: Missing WebSocket in state')
46+
}
4147
if (!messages) {
4248
throw new Error('Internal error for read_files: Missing messages in state')
4349
}

backend/src/tools/handlers/tool/spawn-agent-inline.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type { AgentState } from '@codebuff/common/types/session-state'
1919
import type { ProjectFileContext } from '@codebuff/common/util/file'
2020
import type { ParamsExcluding } from '@codebuff/common/types/function-params'
2121
import type { Logger } from '@codebuff/common/types/contracts/logger'
22+
import type { WebSocket } from 'ws'
2223

2324
type ToolName = 'spawn_agent_inline'
2425
export const handleSpawnAgentInline = ((
@@ -32,6 +33,7 @@ export const handleSpawnAgentInline = ((
3233

3334
getLatestState: () => { messages: Message[] }
3435
state: {
36+
ws?: WebSocket
3537
fingerprintId?: string
3638
userId?: string
3739
agentTemplate?: AgentTemplate
@@ -54,6 +56,7 @@ export const handleSpawnAgentInline = ((
5456
| 'parentSystemPrompt'
5557
| 'onResponseChunk'
5658
| 'clearUserPromptMessagesAfterResponse'
59+
| 'ws'
5760
| 'fingerprintId'
5861
>,
5962
): { result: Promise<CodebuffToolOutput<ToolName>>; state: {} } => {
@@ -71,6 +74,7 @@ export const handleSpawnAgentInline = ((
7174
params: spawnParams,
7275
} = toolCall.input
7376
const {
77+
ws,
7478
fingerprintId,
7579
userId,
7680
agentTemplate: parentAgentTemplate,
@@ -111,6 +115,7 @@ export const handleSpawnAgentInline = ((
111115

112116
const result = await executeSubagent({
113117
...params,
118+
ws,
114119
userInputId: `${userInputId}-inline-${agentType}${childAgentState.agentId}`,
115120
prompt: prompt || '',
116121
spawnParams,

backend/src/tools/handlers/tool/spawn-agent-utils.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,16 @@ import type {
1919
Subgoal,
2020
} from '@codebuff/common/types/session-state'
2121
import type { ProjectFileContext } from '@codebuff/common/util/file'
22+
import type { WebSocket } from 'ws'
23+
2224
export interface SpawnAgentParams {
2325
agent_type: string
2426
prompt?: string
2527
params?: any
2628
}
2729

2830
export interface BaseSpawnState {
31+
ws?: WebSocket
2932
fingerprintId?: string
3033
userId?: string
3134
agentTemplate?: AgentTemplate
@@ -50,6 +53,7 @@ export function validateSpawnState(
5053
toolName: string,
5154
): Omit<Required<BaseSpawnState>, 'userId'> & { userId: string | undefined } {
5255
const {
56+
ws,
5357
fingerprintId,
5458
agentTemplate: parentAgentTemplate,
5559
localAgentTemplates,
@@ -59,6 +63,11 @@ export function validateSpawnState(
5963
system,
6064
} = state
6165

66+
if (!ws) {
67+
throw new Error(
68+
`Internal error for ${toolName}: Missing WebSocket in state`,
69+
)
70+
}
6271
if (!fingerprintId) {
6372
throw new Error(
6473
`Internal error for ${toolName}: Missing fingerprintId in state`,
@@ -87,6 +96,7 @@ export function validateSpawnState(
8796
}
8897

8998
return {
99+
ws,
90100
fingerprintId,
91101
userId,
92102
agentTemplate: parentAgentTemplate,

backend/src/tools/handlers/tool/spawn-agents.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type { ParamsExcluding } from '@codebuff/common/types/function-params'
1818
import type { Message } from '@codebuff/common/types/messages/codebuff-message'
1919
import type { PrintModeEvent } from '@codebuff/common/types/print-mode'
2020
import type { AgentState } from '@codebuff/common/types/session-state'
21+
import type { WebSocket } from 'ws'
2122

2223
export type SendSubagentChunk = (data: {
2324
userInputId: string
@@ -39,6 +40,7 @@ export const handleSpawnAgents = ((
3940

4041
getLatestState: () => { messages: Message[] }
4142
state: {
43+
ws?: WebSocket
4244
fingerprintId?: string
4345
userId?: string
4446
agentTemplate?: AgentTemplate
@@ -55,6 +57,7 @@ export const handleSpawnAgents = ((
5557
> &
5658
ParamsExcluding<
5759
typeof executeSubagent,
60+
| 'ws'
5861
| 'userInputId'
5962
| 'prompt'
6063
| 'spawnParams'
@@ -90,6 +93,7 @@ export const handleSpawnAgents = ((
9093
}
9194

9295
const {
96+
ws,
9397
fingerprintId,
9498
userId,
9599
agentTemplate: parentAgentTemplate,
@@ -131,6 +135,7 @@ export const handleSpawnAgents = ((
131135

132136
const result = await executeSubagent({
133137
...params,
138+
ws,
134139
userInputId: `${userInputId}-${agentType}${subAgentState.agentId}`,
135140
prompt: prompt || '',
136141
spawnParams,

0 commit comments

Comments
 (0)