Skip to content

Commit 710def7

Browse files
bugerclaude
andcommitted
fix: parallel sandbox execution and improve MCP/workflow debugging
- Fix sandbox parallel execution errors by passing `log` through scope instead of declaring in code header (prevents "already declared" errors) - Only inject `log` when injectLog=true, allowing callers to define their own - Fix wrapFunction default to handle partial options correctly - Add detailed logging for MCP SSE communication and workflow tool execution - Make worktree IDs deterministic for reuse across multiple calls - Reuse existing project paths in workspace manager to prevent duplicates - Update @probelabs/probe to v0.6.0-rc210 - Add enableTasks option propagation to ProbeAgent Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent fdbe52f commit 710def7

File tree

8 files changed

+106
-25
lines changed

8 files changed

+106
-25
lines changed

package-lock.json

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@
9797
"@octokit/auth-app": "^8.1.0",
9898
"@octokit/core": "^7.0.3",
9999
"@octokit/rest": "^22.0.0",
100-
"@probelabs/probe": "^0.6.0-rc207",
100+
"@probelabs/probe": "^0.6.0-rc210",
101101
"@types/commander": "^2.12.0",
102102
"@types/uuid": "^10.0.0",
103103
"ajv": "^8.17.1",

src/ai-review-service.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,8 @@ export interface AIReviewConfig {
181181
mcpServers?: Record<string, import('./types/config').McpServerConfig>;
182182
// Enable delegate tool for task distribution to subagents
183183
enableDelegate?: boolean;
184+
// Enable task management for tracking multi-goal requests
185+
enableTasks?: boolean;
184186
// ProbeAgent persona/prompt family (e.g., 'engineer', 'code-review', 'architect')
185187
promptType?: string;
186188
// System prompt to prepend (baseline/preamble). Replaces legacy customPrompt
@@ -1816,6 +1818,11 @@ ${'='.repeat(60)}
18161818
(options as any).enableDelegate = this.config.enableDelegate;
18171819
}
18181820

1821+
// Enable task management if configured
1822+
if (this.config.enableTasks !== undefined) {
1823+
(options as any).enableTasks = this.config.enableTasks;
1824+
}
1825+
18191826
// Pass retry configuration to ProbeAgent
18201827
if (this.config.retry) {
18211828
(options as any).retry = this.config.retry;

src/providers/mcp-custom-sse-server.ts

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -489,12 +489,24 @@ export class CustomToolsSSEServer implements CustomMCPServer {
489489
private sendSSE(connection: SSEConnection, event: string, data: unknown): void {
490490
try {
491491
const dataStr = typeof data === 'string' ? data : JSON.stringify(data);
492+
493+
// Log SSE message being sent (important for debugging MCP communication)
494+
const preview =
495+
dataStr.length > 300
496+
? `${dataStr.substring(0, 150)}...${dataStr.substring(dataStr.length - 150)}`
497+
: dataStr;
498+
logger.debug(
499+
`[CustomToolsSSEServer:${this.sessionId}] Sending SSE event='${event}' size=${dataStr.length} preview=${preview}`
500+
);
501+
492502
connection.response.write(`event: ${event}\n`);
493503
connection.response.write(`data: ${dataStr}\n\n`);
504+
505+
logger.debug(
506+
`[CustomToolsSSEServer:${this.sessionId}] SSE message sent successfully, event='${event}'`
507+
);
494508
} catch (error) {
495-
if (this.debug) {
496-
logger.error(`[CustomToolsSSEServer:${this.sessionId}] Error sending SSE: ${error}`);
497-
}
509+
logger.error(`[CustomToolsSSEServer:${this.sessionId}] Error sending SSE: ${error}`);
498510
}
499511
}
500512

@@ -518,11 +530,20 @@ export class CustomToolsSSEServer implements CustomMCPServer {
518530
// Handle tools/call request
519531
if (message.method === 'tools/call') {
520532
const request = message as MCPToolCallRequest;
533+
const argsPreview = JSON.stringify(request.params.arguments).substring(0, 200);
534+
logger.info(
535+
`[CustomToolsSSEServer:${this.sessionId}] Received tools/call for '${request.params.name}' id=${request.id} args=${argsPreview}`
536+
);
537+
521538
const response = await this.handleToolCall(
522539
request.id,
523540
request.params.name,
524541
request.params.arguments
525542
);
543+
544+
logger.info(
545+
`[CustomToolsSSEServer:${this.sessionId}] Sending response for '${request.params.name}' id=${request.id} hasError=${!!response.error}`
546+
);
526547
this.sendSSE(connection, 'message', response);
527548
return;
528549
}
@@ -635,13 +656,19 @@ export class CustomToolsSSEServer implements CustomMCPServer {
635656
// Format result as MCP response
636657
const resultText = typeof result === 'string' ? result : JSON.stringify(result, null, 2);
637658

638-
if (this.debug) {
639-
logger.debug(
640-
`[CustomToolsSSEServer:${this.sessionId}] Tool execution completed: ${toolName}`
641-
);
642-
}
659+
// Always log tool completion with result details (important for debugging MCP issues)
660+
const resultPreview =
661+
resultText.length > 500
662+
? `${resultText.substring(0, 250)}...TRUNCATED(${resultText.length} chars)...${resultText.substring(resultText.length - 250)}`
663+
: resultText;
664+
logger.info(
665+
`[CustomToolsSSEServer:${this.sessionId}] Tool ${toolName} completed. Result size: ${resultText.length} chars`
666+
);
667+
logger.debug(
668+
`[CustomToolsSSEServer:${this.sessionId}] Tool ${toolName} result preview: ${resultPreview}`
669+
);
643670

644-
return {
671+
const response: MCPToolCallResponse = {
645672
jsonrpc: '2.0',
646673
id,
647674
result: {
@@ -653,6 +680,12 @@ export class CustomToolsSSEServer implements CustomMCPServer {
653680
],
654681
},
655682
};
683+
684+
logger.debug(
685+
`[CustomToolsSSEServer:${this.sessionId}] Returning MCP response for ${toolName}, id=${id}, content_length=${resultText.length}`
686+
);
687+
688+
return response;
656689
} catch (error) {
657690
const errorMsg = error instanceof Error ? error.message : 'Unknown error';
658691
logger.error(

src/providers/workflow-tool-executor.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,16 +226,27 @@ export async function executeWorkflowAsTool(
226226
// Return the output from the workflow execution
227227
// The workflow output is the most useful part for AI tools
228228
const output = (result as any).output;
229+
230+
// Log what we're returning (important for debugging MCP tool results)
231+
const outputKeys = output ? Object.keys(output) : [];
232+
const outputPreview = output ? JSON.stringify(output).substring(0, 500) : 'undefined';
233+
logger.info(
234+
`[WorkflowToolExecutor] Workflow '${workflowId}' completed. Output keys: [${outputKeys.join(', ')}]`
235+
);
236+
logger.debug(`[WorkflowToolExecutor] Workflow '${workflowId}' output preview: ${outputPreview}`);
237+
229238
if (output !== undefined) {
230239
return output;
231240
}
232241

233242
// Fall back to content if no structured output
234243
if ((result as any).content) {
244+
logger.debug(`[WorkflowToolExecutor] Using content fallback for '${workflowId}'`);
235245
return (result as any).content;
236246
}
237247

238248
// Fall back to the full result
249+
logger.debug(`[WorkflowToolExecutor] Using full result fallback for '${workflowId}'`);
239250
return result;
240251
}
241252

src/utils/sandbox.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ export function validateJsSyntax(code: string): JsSyntaxValidationResult {
4646
: `return (() => {\n${trimmed}\n})();\n`
4747
: `return (\n${trimmed}\n);\n`;
4848

49-
// Add log injection header (same as compileAndRun)
50-
const header = `const __lp = "[syntax-check]"; const log = (...a) => { try { console.log(__lp, ...a); } catch {} };\n`;
49+
// For syntax validation, we just need to ensure 'log' is defined so code using it parses correctly
50+
// No need for unique IDs since validation creates a fresh sandbox and only compiles (no execution)
51+
const header = `var log = function() {};\n`;
5152
const fullCode = `${header}${body}`;
5253

5354
try {
@@ -238,10 +239,21 @@ export function compileAndRun<T = unknown>(
238239
.replace(/[`$\\]/g, '') // strip backticks, dollar (template) and backslashes
239240
.replace(/\$\{/g, '') // remove template openings if present
240241
.slice(0, 64);
241-
// Build a safe header without string concatenation inside user code
242-
const header = inject
243-
? `const __lp = ${JSON.stringify(safePrefix)}; const log = (...a) => { try { console.log(__lp, ...a); } catch {} };\n`
244-
: '';
242+
// Inject log function through scope to avoid "already declared" errors
243+
// when multiple sandbox executions run in parallel with shared global state.
244+
// Only add log when injectLog is true - when false, user code may declare its own log.
245+
const scopeWithLog = inject
246+
? {
247+
...scope,
248+
log: (...args: unknown[]) => {
249+
try {
250+
console.log(safePrefix, ...args);
251+
} catch {}
252+
},
253+
}
254+
: scope;
255+
// No header needed - log is passed through scope
256+
const header = '';
245257
// When wrapping, execute user code inside an IIFE and return its value.
246258
// This reliably captures the value of the last expression or any explicit
247259
// return statements inside the script, without requiring the caller to
@@ -256,7 +268,9 @@ export function compileAndRun<T = unknown>(
256268
// (e.g., `(() => { ... })()` or `(function(){ ... })()`), return its value
257269
// directly to avoid swallowing the result by nesting it inside another block.
258270
const looksLikeIife = /\)\s*\(\s*\)\s*;?$/.test(src.trim());
259-
const body = opts.wrapFunction
271+
// Default wrapFunction to true if not explicitly set to false
272+
const shouldWrap = opts.wrapFunction !== false;
273+
const body = shouldWrap
260274
? looksLikeBlock
261275
? looksLikeIife
262276
? `return (\n${src}\n);\n`
@@ -274,7 +288,7 @@ export function compileAndRun<T = unknown>(
274288

275289
let out: any;
276290
try {
277-
out = exec(scope);
291+
out = exec(scopeWithLog);
278292
} catch (e) {
279293
const msg = e instanceof Error ? e.message : String(e);
280294
throw new Error(`sandbox_execution_error: ${msg}`);

src/utils/workspace-manager.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ export class WorkspaceManager {
213213

214214
/**
215215
* Add a project to the workspace (creates symlink to worktree)
216+
* If the same repository + worktreePath combination already exists, returns the existing path.
216217
*/
217218
async addProject(
218219
repository: string,
@@ -223,10 +224,22 @@ export class WorkspaceManager {
223224
throw new Error('Workspace not initialized. Call initialize() first.');
224225
}
225226

227+
// Check if this exact project (same repo + worktree path) is already added
228+
// This prevents duplicate checkouts when tyk-code-talk is called multiple times
229+
for (const [existingName, existingProject] of this.projects.entries()) {
230+
if (
231+
existingProject.repository === repository &&
232+
existingProject.worktreePath === worktreePath
233+
) {
234+
logger.debug(`Reusing existing project: ${existingName} (${repository})`);
235+
return existingProject.path;
236+
}
237+
}
238+
226239
// Extract project name and sanitize to prevent path traversal
227240
let projectName = sanitizePathComponent(description || this.extractRepoName(repository));
228241

229-
// Handle duplicate names
242+
// Handle duplicate names (only if not reusing existing)
230243
projectName = this.getUniqueName(projectName);
231244
this.usedNames.add(projectName);
232245

src/utils/worktree-manager.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,17 @@ export class WorktreeManager {
100100
}
101101

102102
/**
103-
* Generate a unique worktree ID
103+
* Generate a deterministic worktree ID based on repository and ref.
104+
* This allows worktrees to be reused when the same repo+ref is requested again.
104105
*/
105106
private generateWorktreeId(repository: string, ref: string): string {
106107
const sanitizedRepo = repository.replace(/[^a-zA-Z0-9-]/g, '-');
107108
const sanitizedRef = ref.replace(/[^a-zA-Z0-9-]/g, '-');
109+
// Use deterministic hash (no Date.now()) so same repo+ref produces same ID
110+
// This enables worktree reuse across multiple calls
108111
const hash = crypto
109112
.createHash('md5')
110-
.update(`${repository}:${ref}:${Date.now()}`)
113+
.update(`${repository}:${ref}`)
111114
.digest('hex')
112115
.substring(0, 8);
113116
return `${sanitizedRepo}-${sanitizedRef}-${hash}`;

0 commit comments

Comments
 (0)