Skip to content

Commit 909a296

Browse files
committed
🤖 fix: incorporate linter changes and improvements to task service
- Add guardrail to prevent spawning tasks after agent_report - Improve trunk branch detection for worktree runtimes (use parent's current branch) - Add fallback report synthesis when agent_report is missing - Add proper failure handling with postFailureToParent - Add maybeResumeParentStream for auto-resuming parent after task completion - Add hasActiveDescendantTasks check before resuming - Add cleanupTaskSubtree for proper cleanup of task hierarchies - Add hasPendingTaskToolCalls guard in workspaceService - Add appendToHistoryAndEmit helper method - Various robustness improvements from linter suggestions Change-Id: I8a9c29a435b3bb96e0146628d55fab77fb3d3c91 Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent f5fd3d2 commit 909a296

File tree

6 files changed

+1406
-139
lines changed

6 files changed

+1406
-139
lines changed

src/browser/utils/messages/retryEligibility.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,17 @@ export function hasInterruptedStream(
103103
return false;
104104
}
105105

106+
// task is also a special case: an unfinished tool call represents an intentional
107+
// "waiting for subagent task completion" state, not a stream interruption.
108+
//
109+
// Treating it as interrupted causes RetryBarrier + auto-resume to fire on app
110+
// restart, which drops the unfinished tool call from the prompt and can lead
111+
// to duplicate task spawns. TaskService injects the tool output and auto-resumes
112+
// the parent once the subagent reports.
113+
if (lastMessage.type === "tool" && lastMessage.toolName === "task" && lastMessage.status === "executing") {
114+
return false;
115+
}
116+
106117
return (
107118
lastMessage.type === "stream-error" || // Stream errored out (show UI for ALL error types)
108119
lastMessage.type === "user" || // No response received yet (app restart during slow model)

src/node/services/aiService.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import type { TelemetryService } from "@/node/services/telemetryService";
4949
import { getRuntimeTypeForTelemetry, roundToBase2 } from "@/common/telemetry/utils";
5050
import type { MCPServerManager, MCPWorkspaceStats } from "@/node/services/mcpServerManager";
5151
import type { TaskService } from "@/node/services/taskService";
52+
import { getAgentPreset, getAgentToolPolicy } from "@/common/constants/agentPresets";
5253
import { buildProviderOptions } from "@/common/utils/ai/providerOptions";
5354
import type { ThinkingLevel } from "@/common/types/thinking";
5455
import type {
@@ -1064,6 +1065,8 @@ export class AIService extends EventEmitter {
10641065
// Construct plan mode instruction if in plan mode
10651066
// This is done backend-side because we have access to the plan file path
10661067
let effectiveAdditionalInstructions = additionalSystemInstructions;
1068+
let includeCustomInstructions = true;
1069+
let effectiveToolPolicy: ToolPolicy | undefined = toolPolicy;
10671070
const planFilePath = getPlanFilePath(metadata.name, metadata.projectName);
10681071

10691072
// Read plan file (handles legacy migration transparently)
@@ -1081,6 +1084,17 @@ export class AIService extends EventEmitter {
10811084
: planModeInstruction;
10821085
}
10831086

1087+
// Agent-task workspaces enforce preset tool policy + system prompt centrally.
1088+
// V1 uses "replace" semantics: ignore workspace/user custom instructions and use the preset prompt.
1089+
if (metadata.agentType) {
1090+
const preset = getAgentPreset(metadata.agentType);
1091+
effectiveToolPolicy = getAgentToolPolicy(metadata.agentType, toolPolicy);
1092+
includeCustomInstructions = false;
1093+
effectiveAdditionalInstructions = effectiveAdditionalInstructions
1094+
? `${preset.systemPrompt}\n\n${effectiveAdditionalInstructions}`
1095+
: preset.systemPrompt;
1096+
}
1097+
10841098
// Read plan content for mode transition (plan → exec)
10851099
// Only read if switching to exec mode and last assistant was in plan mode
10861100
let planContentForTransition: string | undefined;
@@ -1160,7 +1174,8 @@ export class AIService extends EventEmitter {
11601174
mode,
11611175
effectiveAdditionalInstructions,
11621176
modelString,
1163-
mcpServers
1177+
mcpServers,
1178+
{ includeCustomInstructions }
11641179
);
11651180

11661181
// Count system message tokens for cost tracking
@@ -1246,7 +1261,7 @@ export class AIService extends EventEmitter {
12461261
);
12471262

12481263
// Apply tool policy to filter tools (if policy provided)
1249-
const tools = applyToolPolicy(allTools, toolPolicy);
1264+
const tools = applyToolPolicy(allTools, effectiveToolPolicy);
12501265

12511266
const effectiveMcpStats: MCPWorkspaceStats =
12521267
mcpStats ??
@@ -1296,7 +1311,7 @@ export class AIService extends EventEmitter {
12961311
workspaceId,
12971312
model: modelString,
12981313
toolNames: Object.keys(tools),
1299-
hasToolPolicy: Boolean(toolPolicy),
1314+
hasToolPolicy: Boolean(effectiveToolPolicy && effectiveToolPolicy.length > 0),
13001315
});
13011316

13021317
// Create assistant message placeholder with historySequence from backend

src/node/services/systemMessage.ts

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -251,47 +251,54 @@ export async function buildSystemMessage(
251251
mode?: string,
252252
additionalSystemInstructions?: string,
253253
modelString?: string,
254-
mcpServers?: MCPServerMap
254+
mcpServers?: MCPServerMap,
255+
options?: { includeCustomInstructions?: boolean }
255256
): Promise<string> {
256257
if (!metadata) throw new Error("Invalid workspace metadata: metadata is required");
257258
if (!workspacePath) throw new Error("Invalid workspace path: workspacePath is required");
258259

259-
// Read instruction sets
260-
const [globalInstructions, contextInstructions] = await readInstructionSources(
261-
metadata,
262-
runtime,
263-
workspacePath
264-
);
265-
266-
// Combine: global + context (workspace takes precedence over project) after stripping scoped sections
267-
const sanitizeScopedInstructions = (input?: string | null): string | undefined => {
268-
if (!input) return undefined;
269-
const stripped = stripScopedInstructionSections(input);
270-
return stripped.trim().length > 0 ? stripped : undefined;
271-
};
260+
const includeCustomInstructions = options?.includeCustomInstructions ?? true;
272261

273-
const customInstructionSources = [
274-
sanitizeScopedInstructions(globalInstructions),
275-
sanitizeScopedInstructions(contextInstructions),
276-
].filter((value): value is string => Boolean(value));
277-
const customInstructions = customInstructionSources.join("\n\n");
278-
279-
// Extract mode-specific section (context first, then global fallback)
262+
let customInstructions = "";
280263
let modeContent: string | null = null;
281-
if (mode) {
282-
modeContent =
283-
(contextInstructions && extractModeSection(contextInstructions, mode)) ??
284-
(globalInstructions && extractModeSection(globalInstructions, mode)) ??
285-
null;
286-
}
287-
288-
// Extract model-specific section based on active model identifier (context first)
289264
let modelContent: string | null = null;
290-
if (modelString) {
291-
modelContent =
292-
(contextInstructions && extractModelSection(contextInstructions, modelString)) ??
293-
(globalInstructions && extractModelSection(globalInstructions, modelString)) ??
294-
null;
265+
266+
if (includeCustomInstructions) {
267+
// Read instruction sets
268+
const [globalInstructions, contextInstructions] = await readInstructionSources(
269+
metadata,
270+
runtime,
271+
workspacePath
272+
);
273+
274+
// Combine: global + context (workspace takes precedence over project) after stripping scoped sections
275+
const sanitizeScopedInstructions = (input?: string | null): string | undefined => {
276+
if (!input) return undefined;
277+
const stripped = stripScopedInstructionSections(input);
278+
return stripped.trim().length > 0 ? stripped : undefined;
279+
};
280+
281+
const customInstructionSources = [
282+
sanitizeScopedInstructions(globalInstructions),
283+
sanitizeScopedInstructions(contextInstructions),
284+
].filter((value): value is string => Boolean(value));
285+
customInstructions = customInstructionSources.join("\n\n");
286+
287+
// Extract mode-specific section (context first, then global fallback)
288+
if (mode) {
289+
modeContent =
290+
(contextInstructions && extractModeSection(contextInstructions, mode)) ??
291+
(globalInstructions && extractModeSection(globalInstructions, mode)) ??
292+
null;
293+
}
294+
295+
// Extract model-specific section based on active model identifier (context first)
296+
if (modelString) {
297+
modelContent =
298+
(contextInstructions && extractModelSection(contextInstructions, modelString)) ??
299+
(globalInstructions && extractModelSection(globalInstructions, modelString)) ??
300+
null;
301+
}
295302
}
296303

297304
// Get runtime type from metadata (defaults to "local" for legacy workspaces without runtimeConfig)

0 commit comments

Comments
 (0)