diff --git a/src/common/orpc/schemas/agentDefinition.ts b/src/common/orpc/schemas/agentDefinition.ts index 06f1b3a81a..7c0037a42a 100644 --- a/src/common/orpc/schemas/agentDefinition.ts +++ b/src/common/orpc/schemas/agentDefinition.ts @@ -114,5 +114,14 @@ export const AgentDefinitionPackageSchema = z scope: AgentDefinitionScopeSchema, frontmatter: AgentDefinitionFrontmatterSchema, body: z.string(), + /** + * Exact source identity of the winning candidate: "built-in" for embedded + * definitions, otherwise the discovery root the file was read from (per-plugin + * agents dirs are unique per plugin). Scope alone is not a provenance + * identifier — project files and project plugins both report "project" — so + * strict explicit-agent sends pin this to detect a different definition + * taking over the same id between launch validation and streaming. + */ + source: z.string().optional(), }) .strict(); diff --git a/src/common/orpc/schemas/errors.ts b/src/common/orpc/schemas/errors.ts index 65ae587b48..602a941f20 100644 --- a/src/common/orpc/schemas/errors.ts +++ b/src/common/orpc/schemas/errors.ts @@ -43,6 +43,7 @@ export const StreamErrorTypeSchema = z.enum([ "stream_truncated", // Provider stream closed before its terminal finish event "max_output_tokens", // Provider truncated the response at max_tokens (finishReason: "length") "model_refusal", // Provider declined to answer (refusal/content-filter); retrying the same request will refuse again + "agent_resolution", // Strict explicit-agent contract failure (agent missing/hidden/disabled/provenance changed); deterministic, retrying reproduces it "unknown", // Catch-all ]); diff --git a/src/common/orpc/schemas/stream.ts b/src/common/orpc/schemas/stream.ts index d1c6cbcd59..cee51a5812 100644 --- a/src/common/orpc/schemas/stream.ts +++ b/src/common/orpc/schemas/stream.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { AgentIdSchema } from "./agentDefinition"; +import { AgentDefinitionScopeSchema, AgentIdSchema } from "./agentDefinition"; import { OpenAIReasoningModeSchema, ThinkingLevelSchema } from "../../types/thinking"; import { AgentModeSchema } from "../../types/mode"; import { ChatUsageDisplaySchema } from "./chatStats"; @@ -800,6 +800,49 @@ export const SendMessageOptionsSchema = z.object({ * iterating on agent files - a broken agent in the worktree won't affect message sending. */ disableWorkspaceAgents: z.boolean().optional(), + /** + * When truthy, a top-level send whose agentId cannot be resolved (or is hidden or + * disabled) at stream time fails loudly instead of silently falling back to exec. + * Workspace-turn launches with explicit agent overrides set this: pre-dispatch + * validation races init hooks and user edits, so stream-time resolution — which + * runs after initialization completes — is the last sound gate against running + * a different agent than the caller asked for. The object form additionally pins + * the validated definition's provenance: if the id resolves from a different + * scope than launch validation saw (e.g. a validated project shadow vanished and + * a global/built-in definition with the same id took over), the send fails + * instead of running a different prompt/tool policy. A single field (rather than + * a sibling flag) so every option-preservation path copies it verbatim. + */ + strictAgentResolution: z + .union([ + z.boolean(), + z.object({ + expectedScope: AgentDefinitionScopeSchema, + /** + * Exact source identity from AgentDefinitionPackage.source ("built-in" or the + * discovery root). Scope alone collapses distinct candidates (project files + * and project plugins both report "project"), so this pins the definition + * itself when known. + */ + expectedSource: z.string().optional(), + /** + * Provenance of the full resolved base chain (leaf first), pinned because + * stream-time inheritance resolution reloads every base independently — a + * vanished base shadow must not silently swap a different definition into + * the chain's prompt/tool policy. + */ + expectedChain: z + .array( + z.object({ + id: AgentIdSchema, + scope: AgentDefinitionScopeSchema, + source: z.string().optional(), + }) + ) + .optional(), + }), + ]) + .optional(), /** * Desktop/app-only capability: expose set_goal so an agent can create a * continuation-backed goal for its current parent workspace. Headless callers diff --git a/src/common/types/message.ts b/src/common/types/message.ts index 6afd7d70c1..191a977d4d 100644 --- a/src/common/types/message.ts +++ b/src/common/types/message.ts @@ -58,6 +58,7 @@ type PreservedSendOptions = Pick< | "providerOptions" | "experiments" | "disableWorkspaceAgents" + | "strictAgentResolution" | "allowAgentSetGoal" | "skipAiSettingsPersistence" >; @@ -74,6 +75,10 @@ export function pickPreservedSendOptions(options: SendMessageOptions): Preserved providerOptions: options.providerOptions, experiments: options.experiments, disableWorkspaceAgents: options.disableWorkspaceAgents, + // Delegated turns with explicit agent overrides must stay loud across the + // compaction replay too — dropping this would let the follow-up silently + // fall back to exec if the agent vanished in the meantime. + strictAgentResolution: options.strictAgentResolution, allowAgentSetGoal: options.allowAgentSetGoal, skipAiSettingsPersistence: options.skipAiSettingsPersistence, }; @@ -91,6 +96,7 @@ export type StartupRetrySendOptions = Pick< | "providerOptions" | "experiments" | "disableWorkspaceAgents" + | "strictAgentResolution" | "allowAgentSetGoal" > & { /** Correlation for a delegated workspace turn that must survive restart recovery. */ @@ -124,6 +130,8 @@ export function pickStartupRetrySendOptions( providerOptions: options.providerOptions, experiments: options.experiments, disableWorkspaceAgents: options.disableWorkspaceAgents, + // Keep explicit-agent turns loud across restart recovery (see pickPreservedSendOptions). + strictAgentResolution: options.strictAgentResolution, allowAgentSetGoal: options.allowAgentSetGoal, ...(workspaceTurnMuxMetadata != null ? { muxMetadata: workspaceTurnMuxMetadata } : {}), ...(agentInitiated === true ? { agentInitiated: true } : {}), diff --git a/src/common/utils/messages/retryEligibility.test.ts b/src/common/utils/messages/retryEligibility.test.ts index b53ce0e03e..cf692065da 100644 --- a/src/common/utils/messages/retryEligibility.test.ts +++ b/src/common/utils/messages/retryEligibility.test.ts @@ -477,11 +477,16 @@ describe("isProviderConfigFixableError", () => { "aborted", "runtime_not_ready", "model_not_found", + "agent_resolution", ]) { it(`does not flag ${type} as config-fixable`, () => { expect(isProviderConfigFixableError(type)).toBe(false); }); } + + it("flags agent_resolution as non-retryable (deterministic strict contract failure)", () => { + expect(isNonRetryableStreamError({ type: "agent_resolution" })).toBe(true); + }); }); describe("isNonRetryableSendError", () => { diff --git a/src/common/utils/messages/retryEligibility.ts b/src/common/utils/messages/retryEligibility.ts index 5753f78eda..f6726f2d7f 100644 --- a/src/common/utils/messages/retryEligibility.ts +++ b/src/common/utils/messages/retryEligibility.ts @@ -53,6 +53,7 @@ const NON_RETRYABLE_STREAM_ERRORS = [ "aborted", // User cancelled - should not auto-retry "runtime_not_ready", // Container/runtime unavailable - permanent failure "model_refusal", // Provider declined to answer - retrying the same request will refuse again + "agent_resolution", // Strict explicit-agent contract failure - deterministic, retrying reproduces it ] as const satisfies readonly StreamErrorType[]; const NON_RETRYABLE_STREAM_ERROR_SET = new Set(NON_RETRYABLE_STREAM_ERRORS); diff --git a/src/common/utils/tools/toolDefinitions.test.ts b/src/common/utils/tools/toolDefinitions.test.ts index 20c5b439e7..517afea936 100644 --- a/src/common/utils/tools/toolDefinitions.test.ts +++ b/src/common/utils/tools/toolDefinitions.test.ts @@ -94,6 +94,34 @@ describe("TOOL_DEFINITIONS", () => { } }); + it("accepts workspace task args with an agent id", () => { + const parsed = TaskToolArgsSchema.safeParse({ + kind: "workspace", + agentId: "plan", + prompt: "Plan a small change", + title: "Plan dogfood", + }); + + expect(parsed.success).toBe(true); + if (parsed.success) { + expect(parsed.data.agentId).toBe("plan"); + } + }); + + it("still rejects subagent_type for workspace tasks", () => { + const parsed = TaskToolArgsSchema.safeParse({ + kind: "workspace", + subagent_type: "plan", + prompt: "Plan a small change", + title: "Plan dogfood", + }); + + expect(parsed.success).toBe(false); + if (!parsed.success) { + expect(parsed.error.issues[0]?.path).toEqual(["subagent_type"]); + } + }); + it("rejects workspace task fanout until workspace handles support it", () => { expect( TaskToolArgsSchema.safeParse({ diff --git a/src/common/utils/tools/toolDefinitions.ts b/src/common/utils/tools/toolDefinitions.ts index d60f0e4541..984c8c8c6c 100644 --- a/src/common/utils/tools/toolDefinitions.ts +++ b/src/common/utils/tools/toolDefinitions.ts @@ -321,6 +321,7 @@ export function buildTaskToolDescription(runtimeMode: RuntimeMode | undefined): "\n\nIMPORTANT: Whether a sub-agent can see uncommitted changes depends on the runtime. " + `${getTaskRuntimeVisibilityGuidance(runtimeMode)} ` + "\n\nProvide agentId (preferred) or subagent_type, prompt, title, run_in_background, and optional n. For sub-agents, use title as a short, friendly reusable role name (for example, Reviewer or Simplicity Auditor), not a task summary. For kind=workspace, use a normal work-specific chat title. " + + 'For kind=workspace, agentId optionally selects the agent mode for the launched turn (for example "plan"); it defaults to exec, and internal agents are not eligible. ' + "Use n only when you want several agents to try the same prompt independently. Omit it for a single task, and prefer non-interfering sub-agents for grouped runs (for example read-only agents like explore). " + `\n\nA terminal report makes the child inactive but leaves its workspace persistent. Keep each parent's direct standalone bench small and role-based: aim for at most ${SUBAGENT_REUSABLE_BENCH_TARGET} and keep it below ${SUBAGENT_REUSABLE_BENCH_EXCLUSIVE_LIMIT}; deliberate grouped n runs are temporary exceptions. Before spawning standalone work, prefer reawakening a known inactive child when its context or expertise fits, and retitle it if its reusable responsibility changes. At the target, add a role only for a genuinely distinct responsibility and prune an inactive overlapping or least-useful role before reaching the limit. Reawakening preserves the child's checkout, so for repository-dependent work, reuse it only when that snapshot is appropriate or instruct the child to verify and synchronize before acting; otherwise spawn a new child. Stop active work with task_stop; use irreversible task_remove for consumed grouped candidates, bench consolidation, explicit user requests, or clearly obsolete context—not routine end-of-turn cleanup. ` + "\n\nWhen the user explicitly asks for best-of-n work, the parent should begin with light preliminary analysis to extract shared context, constraints, or evaluation criteria that would otherwise be duplicated across children. " + @@ -379,11 +380,13 @@ function refineTaskToolAgentArgs( const hasSubagentType = typeof args.subagent_type === "string" && args.subagent_type.length > 0; if (kind === "workspace") { - if (hasAgentId || hasSubagentType) { + // Workspace tasks accept agentId (agent mode for the launched turn, e.g. "plan") but keep + // rejecting the deprecated sub-agent alias subagent_type. + if (hasSubagentType) { ctx.addIssue({ code: z.ZodIssueCode.custom, - message: "Workspace tasks do not accept agentId or subagent_type", - path: ["agentId"], + message: "Workspace tasks do not accept subagent_type", + path: ["subagent_type"], }); } if (args.n != null) { diff --git a/src/node/services/agentDefinitions/agentDefinitionsService.ts b/src/node/services/agentDefinitions/agentDefinitionsService.ts index 80254fd435..039c4064ed 100644 --- a/src/node/services/agentDefinitions/agentDefinitionsService.ts +++ b/src/node/services/agentDefinitions/agentDefinitionsService.ts @@ -659,6 +659,9 @@ export async function readAgentDefinition( scope: candidate.scope, frontmatter: parsed.frontmatter, body: parsed.body, + // Exact provenance for strict-send pinning; per-plugin candidate roots are + // unique per plugin, so root identity distinguishes file vs plugin sources. + source: candidate.root, }; const validated = AgentDefinitionPackageSchema.safeParse(pkg); @@ -677,7 +680,7 @@ export async function readAgentDefinition( if (!skipScopes.has("built-in")) { const builtIn = getBuiltInAgentDefinitions().find((pkg) => pkg.id === agentId); if (builtIn) { - const validated = AgentDefinitionPackageSchema.safeParse(builtIn); + const validated = AgentDefinitionPackageSchema.safeParse({ ...builtIn, source: "built-in" }); if (!validated.success) { throw new Error( `Invalid built-in agent definition '${agentId}': ${validated.error.message}` diff --git a/src/node/services/agentDefinitions/resolveAgentInheritanceChain.ts b/src/node/services/agentDefinitions/resolveAgentInheritanceChain.ts index c9f40c8549..d361ececdc 100644 --- a/src/node/services/agentDefinitions/resolveAgentInheritanceChain.ts +++ b/src/node/services/agentDefinitions/resolveAgentInheritanceChain.ts @@ -18,6 +18,9 @@ export interface AgentForInheritance { uiColor?: string; /** Per-hop (unmerged) frontmatter `ai` defaults for AI-settings resolution. */ ai?: AgentDefinitionPackage["frontmatter"]["ai"]; + /** Provenance of the hop's winning definition (strict-send chain pinning). */ + scope: AgentDefinitionPackage["scope"]; + source?: string; } interface ResolveAgentInheritanceChainOptions { @@ -71,6 +74,8 @@ export async function resolveAgentInheritanceChain( tools: currentDefinition.frontmatter.tools, uiColor: currentDefinition.frontmatter.ui?.color, ai: currentDefinition.frontmatter.ai, + scope: currentDefinition.scope, + ...(currentDefinition.source != null ? { source: currentDefinition.source } : {}), }); const baseId = currentDefinition.frontmatter.base; diff --git a/src/node/services/agentDefinitions/resolveNodeAgentAiSettings.ts b/src/node/services/agentDefinitions/resolveNodeAgentAiSettings.ts index 41ec987d8c..37dd826670 100644 --- a/src/node/services/agentDefinitions/resolveNodeAgentAiSettings.ts +++ b/src/node/services/agentDefinitions/resolveNodeAgentAiSettings.ts @@ -32,6 +32,8 @@ export interface NodeAgentDefinitionContext { runtime: Runtime; workspacePath: string; workspaceId: string; + /** agent-plugins experiment: also resolve definitions contributed by Agent Plugins. */ + includeAgentPlugins?: boolean; } export interface ResolveNodeAgentAiSettingsParams { @@ -134,7 +136,8 @@ async function loadDefinitionLayers( const agentDefinition = await readAgentDefinition( context.runtime, context.workspacePath, - agentId + agentId, + { includeAgentPlugins: context.includeAgentPlugins } ); const chain = await resolveAgentInheritanceChain({ runtime: context.runtime, @@ -142,6 +145,7 @@ async function loadDefinitionLayers( agentId: agentDefinition.id, agentDefinition, workspaceId: context.workspaceId, + includeAgentPlugins: context.includeAgentPlugins, }); return collectDefinitionLayers(agentId, chain); diff --git a/src/node/services/agentResolution.test.ts b/src/node/services/agentResolution.test.ts index 0df455159a..d64af1e3a5 100644 --- a/src/node/services/agentResolution.test.ts +++ b/src/node/services/agentResolution.test.ts @@ -472,6 +472,321 @@ describe("resolveAgentForStream agent identity", () => { }); }); +describe("resolveAgentForStream strict resolution", () => { + function createTopLevelMetadata(projectPath: string): WorkspaceMetadata { + return { + id: PARENT_WORKSPACE_ID, + name: PARENT_WORKSPACE_ID, + projectName: path.basename(projectPath), + projectPath, + runtimeConfig: DEFAULT_RUNTIME_CONFIG, + }; + } + + async function resolveTopLevel(params: { + projectPath: string; + agentId: string; + strictAgentResolution: + | boolean + | { + expectedScope: "project" | "global" | "built-in"; + expectedSource?: string; + expectedChain?: Array<{ + id: string; + scope: "project" | "global" | "built-in"; + source?: string; + }>; + }; + agentAiDefaults?: ProjectsConfig["agentAiDefaults"]; + onError?: (event: { errorType?: string }) => void; + }) { + const cfg: ProjectsConfig = { + projects: new Map([ + [ + params.projectPath, + { + trusted: true, + workspaces: [ + { id: PARENT_WORKSPACE_ID, name: PARENT_WORKSPACE_ID, path: params.projectPath }, + ], + }, + ], + ]), + ...(params.agentAiDefaults ? { agentAiDefaults: params.agentAiDefaults } : {}), + }; + return await resolveAgentForStream({ + workspaceId: PARENT_WORKSPACE_ID, + metadata: createTopLevelMetadata(params.projectPath), + runtime: new LocalRuntime(params.projectPath), + workspacePath: params.projectPath, + requestedAgentId: params.agentId, + disableWorkspaceAgents: false, + strictAgentResolution: params.strictAgentResolution, + callerToolPolicy: undefined, + cfg, + emitError: (event) => params.onError?.(event), + isAdvisorExperimentEnabled: false, + }); + } + + test("unresolvable explicit agent fails loudly instead of falling back to exec", async () => { + using tempDir = new DisposableTempDir("agent-resolution-strict-unknown"); + const projectPath = path.join(tempDir.path, "project"); + await fs.mkdir(projectPath, { recursive: true }); + + // Default behavior: silent exec fallback keeps ordinary sends usable. + const lenient = await resolveTopLevel({ + projectPath, + agentId: "doesnotexist", + strictAgentResolution: false, + }); + expect(lenient.success).toBe(true); + if (lenient.success) expect(lenient.data.effectiveAgentId).toBe("exec"); + + // Strict mode (explicit workspace-turn overrides): running a different agent + // than requested must fail the stream, not silently swap in exec — and the + // failure is deterministic, so it must be classified non-retryable + // (agent_resolution) instead of the retryable catch-all. + const emittedErrorTypes: Array = []; + const strict = await resolveTopLevel({ + projectPath, + agentId: "doesnotexist", + strictAgentResolution: true, + onError: (event) => emittedErrorTypes.push(event.errorType), + }); + expect(strict.success).toBe(false); + if (!strict.success && strict.error.type === "unknown") { + expect(strict.error.raw).toContain("could not be resolved"); + } else { + expect(strict.success === false && strict.error.type).toBe("unknown"); + } + expect(emittedErrorTypes).toEqual(["agent_resolution"]); + }); + + test("hidden explicit agent fails loudly in strict mode for top-level workspaces", async () => { + using tempDir = new DisposableTempDir("agent-resolution-strict-hidden"); + const projectPath = path.join(tempDir.path, "project"); + const agentsDir = path.join(projectPath, ".mux", "agents"); + await fs.mkdir(agentsDir, { recursive: true }); + // A definition hidden between launch-time validation and streaming (init hook or + // concurrent edit): strict sends must uphold the workspace-task contract that + // internal agents are ineligible instead of running the hidden policy. + await fs.writeFile( + path.join(agentsDir, "custom.md"), + ["---", "name: Custom", "base: exec", "ui:", " hidden: true", "---", "Hidden agent."].join( + "\n" + ) + ); + + const strict = await resolveTopLevel({ + projectPath, + agentId: "custom", + strictAgentResolution: true, + }); + expect(strict.success).toBe(false); + if (!strict.success && strict.error.type === "unknown") { + expect(strict.error.raw).toContain("not selectable"); + } else { + expect(strict.success === false && strict.error.type).toBe("unknown"); + } + + // Lenient top-level sends keep today's behavior (no visibility gate). + const lenient = await resolveTopLevel({ + projectPath, + agentId: "custom", + strictAgentResolution: false, + }); + expect(lenient.success).toBe(true); + if (lenient.success) expect(lenient.data.effectiveAgentId).toBe("custom"); + }); + + test("hidden exec shadow fails loudly in strict mode instead of running the shadow", async () => { + using tempDir = new DisposableTempDir("agent-resolution-strict-exec-shadow"); + const projectPath = path.join(tempDir.path, "project"); + const agentsDir = path.join(projectPath, ".mux", "agents"); + await fs.mkdir(agentsDir, { recursive: true }); + // Discovery prefers a project exec shadow over the built-in; strict sends must gate + // exec like any other id instead of skipping the eligibility block for it. + await fs.writeFile( + path.join(agentsDir, "exec.md"), + ["---", "name: Exec", "ui:", " hidden: true", "---", "Hidden exec shadow."].join("\n") + ); + + const strict = await resolveTopLevel({ + projectPath, + agentId: "exec", + strictAgentResolution: true, + }); + expect(strict.success).toBe(false); + if (!strict.success && strict.error.type === "unknown") { + expect(strict.error.raw).toContain("not selectable"); + } else { + expect(strict.success === false && strict.error.type).toBe("unknown"); + } + }); + + test("strict mode rejects a definition resolving from a different scope than validated", async () => { + using tempDir = new DisposableTempDir("agent-resolution-strict-provenance"); + const projectPath = path.join(tempDir.path, "project"); + await fs.mkdir(projectPath, { recursive: true }); + + // Launch validation saw a project shadow that has since vanished (init hook or + // concurrent edit): the same id now resolves from the built-in scope. The strict + // provenance pin must fail the send instead of running the different definition. + const strict = await resolveTopLevel({ + projectPath, + agentId: "plan", + strictAgentResolution: { expectedScope: "project" }, + }); + expect(strict.success).toBe(false); + if (!strict.success && strict.error.type === "unknown") { + expect(strict.error.raw).toContain("different definition"); + } else { + expect(strict.success === false && strict.error.type).toBe("unknown"); + } + + // Scope alone is not a provenance identifier (project files and project plugins + // both report "project"): a same-scope pin with a different exact source must + // also fail. + const sameScopeDifferentSource = await resolveTopLevel({ + projectPath, + agentId: "plan", + strictAgentResolution: { expectedScope: "built-in", expectedSource: ".xum/agents" }, + }); + expect(sameScopeDifferentSource.success).toBe(false); + if (!sameScopeDifferentSource.success && sameScopeDifferentSource.error.type === "unknown") { + expect(sameScopeDifferentSource.error.raw).toContain("different definition"); + } else { + expect( + sameScopeDifferentSource.success === false && sameScopeDifferentSource.error.type + ).toBe("unknown"); + } + + // A matching scope + source streams normally. + const matching = await resolveTopLevel({ + projectPath, + agentId: "plan", + strictAgentResolution: { expectedScope: "built-in", expectedSource: "built-in" }, + }); + expect(matching.success).toBe(true); + if (matching.success) expect(matching.data.effectiveAgentId).toBe("plan"); + }); + + test("strict mode rejects a base chain resolving differently than validated", async () => { + using tempDir = new DisposableTempDir("agent-resolution-strict-chain"); + const projectPath = path.join(tempDir.path, "project"); + const agentsDir = path.join(projectPath, ".mux", "agents"); + await fs.mkdir(agentsDir, { recursive: true }); + await fs.writeFile( + path.join(agentsDir, "custom.md"), + ["---", "name: Custom", "base: plan", "---", "Custom agent."].join("\n") + ); + + // Launch validation saw the custom agent inheriting a project plan shadow that has + // since been removed: the leaf provenance is unchanged, but the built-in plan now + // takes over the base hop — the strict chain pin must fail the send. + const staleChain = await resolveTopLevel({ + projectPath, + agentId: "custom", + strictAgentResolution: { + expectedScope: "project", + expectedChain: [ + { id: "custom", scope: "project" }, + { id: "plan", scope: "project" }, + ], + }, + }); + expect(staleChain.success).toBe(false); + if (!staleChain.success && staleChain.error.type === "unknown") { + expect(staleChain.error.raw).toContain("base chain"); + } else { + expect(staleChain.success === false && staleChain.error.type).toBe("unknown"); + } + + // The chain as it actually resolves streams normally. + const matching = await resolveTopLevel({ + projectPath, + agentId: "custom", + strictAgentResolution: { + expectedScope: "project", + expectedChain: [ + { id: "custom", scope: "project" }, + { id: "plan", scope: "built-in", source: "built-in" }, + ], + }, + }); + expect(matching.success).toBe(true); + if (matching.success) expect(matching.data.effectiveAgentId).toBe("custom"); + }); + + test("strict mode fails closed when eligibility resolution throws", async () => { + using tempDir = new DisposableTempDir("agent-resolution-strict-broken-base"); + const projectPath = path.join(tempDir.path, "project"); + const agentsDir = path.join(projectPath, ".mux", "agents"); + await fs.mkdir(agentsDir, { recursive: true }); + // A base pointing at a missing definition makes frontmatter resolution throw; + // strict sends must not stream a partially resolved prompt/tool policy. + await fs.writeFile( + path.join(agentsDir, "custom.md"), + ["---", "name: Custom", "base: missing-base", "---", "Broken chain."].join("\n") + ); + + const strict = await resolveTopLevel({ + projectPath, + agentId: "custom", + strictAgentResolution: true, + }); + expect(strict.success).toBe(false); + if (!strict.success && strict.error.type === "unknown") { + expect(strict.error.raw).toContain("could not be"); + } else { + expect(strict.success === false && strict.error.type).toBe("unknown"); + } + + // Lenient sends keep the best-effort behavior. + const lenient = await resolveTopLevel({ + projectPath, + agentId: "custom", + strictAgentResolution: false, + }); + expect(lenient.success).toBe(true); + }); + + test("disabled explicit agent fails loudly in strict mode for top-level workspaces", async () => { + using tempDir = new DisposableTempDir("agent-resolution-strict-disabled"); + const projectPath = path.join(tempDir.path, "project"); + const agentsDir = path.join(projectPath, ".mux", "agents"); + await fs.mkdir(agentsDir, { recursive: true }); + await fs.writeFile( + path.join(agentsDir, "custom.md"), + ["---", "name: Custom", "base: exec", "---", "Custom agent."].join("\n") + ); + const agentAiDefaults = { custom: { enabled: false } }; + + const lenient = await resolveTopLevel({ + projectPath, + agentId: "custom", + strictAgentResolution: false, + agentAiDefaults, + }); + expect(lenient.success).toBe(true); + if (lenient.success) expect(lenient.data.effectiveAgentId).toBe("exec"); + + const strict = await resolveTopLevel({ + projectPath, + agentId: "custom", + strictAgentResolution: true, + agentAiDefaults, + }); + expect(strict.success).toBe(false); + if (!strict.success && strict.error.type === "unknown") { + expect(strict.error.raw).toContain("disabled"); + } else { + expect(strict.success === false && strict.error.type).toBe("unknown"); + } + }); +}); + describe("resolveAgentForStream advisor defaults", () => { test("enables advisor by default for Exec and Plan sub-agents when the experiment is enabled", async () => { const [execPolicy, planPolicy] = await Promise.all([ diff --git a/src/node/services/agentResolution.ts b/src/node/services/agentResolution.ts index 4f1b47ea9e..69ef9729d7 100644 --- a/src/node/services/agentResolution.ts +++ b/src/node/services/agentResolution.ts @@ -14,6 +14,7 @@ import { resolveAdvisorEnabledForAgent } from "@/common/constants/advisor"; import { AgentIdSchema } from "@/common/orpc/schemas"; import type { SendMessageError } from "@/common/types/errors"; +import type { SendMessageOptions } from "@/common/orpc/types"; import type { Result } from "@/common/types/result"; import { Err, Ok } from "@/common/types/result"; import type { ErrorEvent } from "@/common/types/stream"; @@ -32,6 +33,7 @@ import { resolveAgentFrontmatter, } from "@/node/services/agentDefinitions/agentDefinitionsService"; import { isAgentEffectivelyDisabled } from "@/node/services/agentDefinitions/agentEnablement"; +import { resolveAgentVisibility } from "@/node/services/agentDefinitions/agentVisibility"; import { resolveAgentInheritanceChain } from "@/node/services/agentDefinitions/resolveAgentInheritanceChain"; import { resolveToolPolicyForAgent } from "@/node/services/agentDefinitions/resolveToolPolicy"; import { log } from "./log"; @@ -49,6 +51,17 @@ export interface ResolveAgentOptions { requestedAgentId: string | undefined; /** When true, skip workspace-specific agents (for "unbricking" broken agent files). */ disableWorkspaceAgents: boolean; + /** + * When truthy, a top-level requested agent that cannot be resolved (or is + * hidden or disabled) fails the stream loudly instead of silently falling back + * to exec. Set by workspace-turn launches with explicit agent overrides: their + * pre-dispatch validation races init hooks and user edits, so this post-init + * resolution is the last sound gate against running a different agent than + * the caller asked for. The object form additionally pins the validated + * definition's provenance (scope) — see SendMessageOptionsSchema. Sub-agent + * workspaces already fail loudly. + */ + strictAgentResolution?: SendMessageOptions["strictAgentResolution"]; /** Caller-supplied tool policy (applied AFTER agent policy for further restriction). */ callerToolPolicy: ToolPolicy | undefined; /** Loaded config from Config.loadConfigOrDefault(). */ @@ -186,6 +199,7 @@ export async function resolveAgentForStream( workspacePath, requestedAgentId: rawAgentId, disableWorkspaceAgents, + strictAgentResolution, callerToolPolicy, cfg, emitError, @@ -221,6 +235,12 @@ export async function resolveAgentForStream( let agentDiscoveryPath = agentDiscoveryCandidates[0]?.workspacePath ?? workspacePath; const isSubagentWorkspace = Boolean(metadata.parentWorkspaceId); + // Strict explicit-agent gating applies only to top-level sends: sub-agent workspaces + // already fail loudly and legitimately run hidden agents (explore, compact). + const strictTopLevel = + strictAgentResolution != null && strictAgentResolution !== false && !isSubagentWorkspace; + const strictExpectedScope = + typeof strictAgentResolution === "object" ? strictAgentResolution.expectedScope : undefined; // --- Load agent definition (with fallback to exec) --- let agentDefinition: Awaited> | undefined; @@ -265,6 +285,17 @@ export async function resolveAgentForStream( } if (agentDefinition == null) { + if (strictTopLevel) { + const errorMessage = `Agent '${requestedAgentId}' could not be resolved in this workspace; refusing to fall back to exec for an explicit agent request.`; + emitError( + createErrorEvent(workspaceId, { + messageId: createAssistantMessageId(), + error: errorMessage, + errorType: "agent_resolution", + }) + ); + return Err({ type: "unknown", raw: errorMessage }); + } workspaceLog.warn("Failed to load agent definition; falling back", { requestedAgentIds, agentDiscoveryPaths: agentDiscoveryCandidates.map((candidate) => candidate.workspacePath), @@ -275,6 +306,31 @@ export async function resolveAgentForStream( }); } + // Strict provenance pin: launch validation approved a specific definition, not just + // an id. If that definition vanished (e.g. an init hook or edit deleted a validated + // project shadow) and a different candidate now resolves the same id — a lower + // scope, or a same-scope sibling like a project plugin taking over for a removed + // project file — the turn would run a different prompt/tool policy with AI settings + // derived from the validated one. Scope alone is not a provenance identifier, so + // the exact source (discovery root / "built-in") is compared when pinned. + if (strictTopLevel && strictExpectedScope != null) { + const expectedSource = + typeof strictAgentResolution === "object" ? strictAgentResolution.expectedSource : undefined; + const scopeMismatch = agentDefinition.scope !== strictExpectedScope; + const sourceMismatch = expectedSource != null && agentDefinition.source !== expectedSource; + if (scopeMismatch || sourceMismatch) { + const errorMessage = `Agent '${requestedAgentId}' now resolves from a different definition than launch validation saw (expected ${strictExpectedScope}${expectedSource != null ? ` @ ${expectedSource}` : ""}, found ${agentDefinition.scope}${agentDefinition.source != null ? ` @ ${agentDefinition.source}` : ""}); refusing to stream an explicit agent request.`; + emitError( + createErrorEvent(workspaceId, { + messageId: createAssistantMessageId(), + error: errorMessage, + errorType: "agent_resolution", + }) + ); + return Err({ type: "unknown", raw: errorMessage }); + } + } + // Keep agent ID aligned with the actual definition used (may fall back to exec). effectiveAgentId = agentDefinition.id; @@ -282,7 +338,9 @@ export async function resolveAgentForStream( // Disabled agents should never run as sub-agents, even if a task workspace already exists // on disk (e.g., config changed since creation). // For top-level workspaces, fall back to exec to keep the workspace usable. - if (agentDefinition.id !== "exec") { + // Strict sends also verify exec itself: discovery can select a project/global exec + // shadow, and a hidden shadow must hit the selectability gate below like any other id. + if (agentDefinition.id !== "exec" || strictTopLevel) { try { const resolvedFrontmatter = await resolveAgentFrontmatter( agentDiscoveryRuntime, @@ -294,6 +352,23 @@ export async function resolveAgentForStream( } ); + // Strict explicit-agent sends must also reject definitions that are no longer + // selectable: the workspace-task contract excludes internal (ui.hidden) agents, + // and an init hook or concurrent edit could hide the definition between + // launch-time validation and this stream. Sub-agent workspaces legitimately run + // hidden agents (explore, compact), so only strict top-level sends are gated. + if (strictTopLevel && !resolveAgentVisibility(resolvedFrontmatter.ui).selectable) { + const errorMessage = `Agent '${agentDefinition.id}' is not selectable for explicit agent requests.`; + emitError( + createErrorEvent(workspaceId, { + messageId: createAssistantMessageId(), + error: errorMessage, + errorType: "agent_resolution", + }) + ); + return Err({ type: "unknown", raw: errorMessage }); + } + const effectivelyDisabled = isAgentEffectivelyDisabled({ cfg, agentId: agentDefinition.id, @@ -303,13 +378,13 @@ export async function resolveAgentForStream( if (effectivelyDisabled) { const errorMessage = `Agent '${agentDefinition.id}' is disabled.`; - if (isSubagentWorkspace) { + if (isSubagentWorkspace || strictAgentResolution) { const errorMessageId = createAssistantMessageId(); emitError( createErrorEvent(workspaceId, { messageId: errorMessageId, error: errorMessage, - errorType: "unknown", + errorType: strictTopLevel ? "agent_resolution" : "unknown", }) ); return Err({ type: "unknown", raw: errorMessage }); @@ -328,6 +403,20 @@ export async function resolveAgentForStream( effectiveAgentId = agentDefinition.id; } } catch (error: unknown) { + // Strict sends fail closed when eligibility cannot be verified: a hook or edit + // that breaks the definition (e.g. a base pointing at a missing definition) after + // launch validation would otherwise stream a partially resolved prompt/tool policy. + if (strictTopLevel) { + const errorMessage = `Agent '${agentDefinition.id}' eligibility could not be verified (${getErrorMessage(error)}); refusing to stream an explicit agent request.`; + emitError( + createErrorEvent(workspaceId, { + messageId: createAssistantMessageId(), + error: errorMessage, + errorType: "agent_resolution", + }) + ); + return Err({ type: "unknown", raw: errorMessage }); + } // Best-effort only — do not fail a stream due to disablement resolution. workspaceLog.debug("Failed to resolve agent enablement; continuing", { agentId: agentDefinition.id, @@ -346,6 +435,45 @@ export async function resolveAgentForStream( includeAgentPlugins, }); + // Strict chain pin: inheritance resolution reloads every base independently, so a + // vanished base shadow could otherwise silently swap a different definition into + // the chain's prompt/tool policy even though the validated leaf is unchanged. + const strictExpectedChain = + typeof strictAgentResolution === "object" ? strictAgentResolution.expectedChain : undefined; + if (strictTopLevel && strictExpectedChain != null) { + const chainMatches = + agentsForInheritance.length === strictExpectedChain.length && + strictExpectedChain.every((expected, index) => { + const actual = agentsForInheritance[index]; + return ( + actual != null && + actual.id === expected.id && + actual.scope === expected.scope && + (expected.source == null || actual.source === expected.source) + ); + }); + if (!chainMatches) { + const describeChain = ( + entries: ReadonlyArray<{ id: string; scope: string; source?: string }> + ) => + entries + .map( + (entry) => + `${entry.id}(${entry.scope}${entry.source != null ? ` @ ${entry.source}` : ""})` + ) + .join(" -> "); + const errorMessage = `Agent '${requestedAgentId}' base chain now resolves differently than launch validation saw (expected ${describeChain(strictExpectedChain)}, found ${describeChain(agentsForInheritance)}); refusing to stream an explicit agent request.`; + emitError( + createErrorEvent(workspaceId, { + messageId: createAssistantMessageId(), + error: errorMessage, + errorType: "agent_resolution", + }) + ); + return Err({ type: "unknown", raw: errorMessage }); + } + } + const agentIsPlanLike = isPlanLikeInResolvedChain(agentsForInheritance); const effectiveMode = agentDefinition.id === "compact" ? "compact" : agentIsPlanLike ? "plan" : "exec"; diff --git a/src/node/services/agentSession.autoCompaction.test.ts b/src/node/services/agentSession.autoCompaction.test.ts index 53ceb0d016..b5a72e859e 100644 --- a/src/node/services/agentSession.autoCompaction.test.ts +++ b/src/node/services/agentSession.autoCompaction.test.ts @@ -600,6 +600,49 @@ describe("AgentSession on-send auto-compaction snapshot deferral", () => { session.dispose(); }); + test("clears strictAgentResolution on the internal compact request", async () => { + const workspaceId = "ws-auto-compaction-clears-strict"; + + const streamMessage = mock((_request: unknown) => Promise.resolve(Ok(undefined))); + const { session } = await createSessionHarness({ + workspaceId, + streamMessage: streamMessage as unknown as AIService["streamMessage"], + }); + + // A strict explicit-agent workspace turn hitting auto-compaction: the internal + // request intentionally runs the hidden compact agent, so the strict gate must not + // apply to it (it would reject compact as not selectable and break compaction). + const baseOptions: SendMessageOptions = { + model: "anthropic:claude-sonnet-4-6", + agentId: "plan", + strictAgentResolution: true, + }; + const followUpContent: CompactionFollowUpRequest = { + text: "Continue", + model: baseOptions.model, + agentId: "plan", + }; + + const internals = session as unknown as { + buildAutoCompactionRequest: (params: { + followUpContent: CompactionFollowUpRequest; + baseOptions: SendMessageOptions; + reason: "on-send" | "mid-stream"; + }) => { sendOptions: SendMessageOptions }; + }; + + const compactionRequest = internals.buildAutoCompactionRequest({ + followUpContent, + baseOptions, + reason: "mid-stream", + }); + + expect(compactionRequest.sendOptions.agentId).toBe("compact"); + expect(compactionRequest.sendOptions.strictAgentResolution).toBeUndefined(); + + session.dispose(); + }); + test("compaction model explicit override takes priority over baseOptions.model", async () => { const workspaceId = "ws-auto-compaction-explicit-model-overrides-base-model"; diff --git a/src/node/services/agentSession.continueMessageAgentId.test.ts b/src/node/services/agentSession.continueMessageAgentId.test.ts index e672a7d61e..e136091419 100644 --- a/src/node/services/agentSession.continueMessageAgentId.test.ts +++ b/src/node/services/agentSession.continueMessageAgentId.test.ts @@ -231,6 +231,29 @@ describe("AgentSession continue-message agentId fallback", () => { expect(internals.lastAutoRetryResumeRequest?.agentInitiated).toBe(true); }); + test("dispatchPendingFollowUp forwards strictAgentResolution to the resumed turn", async () => { + let dispatchedOptions: SendOptions | undefined; + const { internals } = await createSession([ + compactionSummaryMessage("summary-strict", { + text: "continue delegated work", + model: "openai:gpt-4o", + agentId: "plan", + strictAgentResolution: true, + }), + ]); + internals.sendMessage = mock((_message: string, options?: SendOptions) => { + dispatchedOptions = options; + return Promise.resolve({ success: true as const }); + }); + + await internals.dispatchPendingFollowUp(); + + // The requested agent may have been removed/hidden/disabled while compaction ran; + // the resumed turn must stay loud instead of silently falling back to exec. + expect(dispatchedOptions?.agentId).toBe("plan"); + expect(dispatchedOptions?.strictAgentResolution).toBe(true); + }); + test("dispatchPendingFollowUp skips idle-only follow-ups when queued user input exists", async () => { const { session, historyService, internals } = await createSession([ compactionSummaryMessage("summary-idle-only", idleFollowUp()), diff --git a/src/node/services/agentSession.ts b/src/node/services/agentSession.ts index f536d32a14..956cbc4b06 100644 --- a/src/node/services/agentSession.ts +++ b/src/node/services/agentSession.ts @@ -37,7 +37,7 @@ import { type GoalSyntheticMessageKind, } from "@/constants/goals"; import type { SendMessageError } from "@/common/types/errors"; -import { AgentIdSchema, SkillNameSchema } from "@/common/orpc/schemas"; +import { AgentIdSchema, SendMessageOptionsSchema, SkillNameSchema } from "@/common/orpc/schemas"; import { normalizeAgentId, resolvePersistedAgentIdCandidates } from "@/common/utils/agentIds"; import { buildStreamErrorEventData, @@ -1900,6 +1900,24 @@ export class AgentSession { if (typeof persistedDisableWorkspaceAgents === "boolean") { retryRequest.disableWorkspaceAgents = persistedDisableWorkspaceAgents; } + // Explicit-agent delegated turns must stay loud across restart recovery: without + // this, a replay after the agent was removed/disabled would silently run exec. + // History stores retrySendOptions as an untyped blob, so the persisted value is + // re-validated against the canonical schema first — a malformed provenance pin + // must be discarded (lenient replay) rather than failing every recovered attempt + // with a false mismatch. A valid pin is copied verbatim (keeps expectedScope/ + // expectedSource). + const persistedStrictAgentResolution = + SendMessageOptionsSchema.shape.strictAgentResolution.safeParse( + persistedRetrySendOptions?.strictAgentResolution + ); + if ( + persistedStrictAgentResolution.success && + persistedStrictAgentResolution.data != null && + persistedStrictAgentResolution.data !== false + ) { + retryRequest.strictAgentResolution = persistedStrictAgentResolution.data; + } if (persistedRetrySendOptions?.agentInitiated === true) { retryRequest.agentInitiated = true; @@ -4280,6 +4298,10 @@ export class AgentSession { const sendOptions: SendMessageOptions = { ...params.baseOptions, agentId: "compact", + // This internal request intentionally runs the hidden compact agent, so the + // caller's strict explicit-agent gate must not apply to it. The post-compaction + // follow-up re-arms strictness via pickPreservedSendOptions. + strictAgentResolution: undefined, skipAiSettingsPersistence: true, model: resolved.selected.model, // Effective (clamped) thinking: this internal request skips persistence, @@ -4715,6 +4737,7 @@ export class AgentSession { workspaceGoalService: this.workspaceGoalService, experiments: options?.experiments, disableWorkspaceAgents: options?.disableWorkspaceAgents, + strictAgentResolution: options?.strictAgentResolution, hasQueuedMessages: this.hasQueuedMessages.bind(this), openaiTruncationModeOverride, // Mid-turn thinking overrides clamp against the same floor as the @@ -6704,6 +6727,9 @@ export class AgentSession { experiments: followUp.experiments, allowAgentSetGoal: followUp.allowAgentSetGoal, disableWorkspaceAgents: followUp.disableWorkspaceAgents, + // Explicit-agent turns stay loud on the resumed turn too: the requested agent + // may have been removed/hidden/disabled while compaction ran. + strictAgentResolution: followUp.strictAgentResolution, skipAiSettingsPersistence: followUp.skipAiSettingsPersistence, }; diff --git a/src/node/services/aiService.test.ts b/src/node/services/aiService.test.ts index 4a9806bc6e..2991b7820a 100644 --- a/src/node/services/aiService.test.ts +++ b/src/node/services/aiService.test.ts @@ -268,7 +268,7 @@ function resolvedAgentResultFor( agentDiscoveryRuntime: new LocalRuntime(metadata.projectPath), agentDiscoveryPath: metadata.projectPath, isSubagentWorkspace: false, - agentInheritanceChain: [{ id: "exec", tools: { add: [".*"] } }], + agentInheritanceChain: [{ id: "exec", scope: "built-in", tools: { add: [".*"] } }], agentIsPlanLike: false, effectiveMode: "exec", taskSettings: DEFAULT_TASK_SETTINGS, diff --git a/src/node/services/aiService.ts b/src/node/services/aiService.ts index 5a1e543d85..444212a103 100644 --- a/src/node/services/aiService.ts +++ b/src/node/services/aiService.ts @@ -293,6 +293,8 @@ export interface StreamMessageOptions { /** Internal-only flag for Copilot billing attribution; never sourced from IPC schemas. */ agentInitiated?: boolean; agentId?: string; + /** See SendMessageOptionsSchema.strictAgentResolution: explicit-agent sends fail loudly instead of falling back to exec. */ + strictAgentResolution?: SendMessageOptions["strictAgentResolution"]; /** ACP prompt correlation id used to match stream events to a specific request. */ acpPromptId?: string; /** Tool names that should be delegated back to ACP clients for this request. */ @@ -1409,6 +1411,7 @@ export class AIService extends EventEmitter { muxProviderOptions, agentInitiated, agentId, + strictAgentResolution, acpPromptId, delegatedToolNames, recordFileState, @@ -1952,6 +1955,7 @@ export class AIService extends EventEmitter { runtime, workspacePath, requestedAgentId: agentId, + strictAgentResolution, disableWorkspaceAgents: disableWorkspaceAgents ?? false, callerToolPolicy: toolPolicy, cfg, diff --git a/src/node/services/taskService.test.ts b/src/node/services/taskService.test.ts index c5b33115f7..665e1dd80c 100644 --- a/src/node/services/taskService.test.ts +++ b/src/node/services/taskService.test.ts @@ -97,6 +97,25 @@ function initGitRepo(projectPath: string): void { execSync('git commit -m "init"', { cwd: projectPath, stdio: "ignore" }); } +/** + * Git-prove the owner workspace's checked-out branch: owner-side agent prechecks and + * unreachable-target vouching only apply when the effective base branch is verified + * against the branch actually checked out in the owner. + */ +function checkoutOwnerBranch(projectPath: string, branch: string): void { + initGitRepo(projectPath); + execSync(`git checkout -b ${branch}`, { cwd: projectPath, stdio: "ignore" }); +} + +/** + * Commit pending agent-definition files: owner-side vouching additionally requires the + * agent-definition paths to be clean (uncommitted changes diverge from the committed + * base a new checkout is created from). + */ +function commitOwnerAgentFiles(projectPath: string): void { + execSync("git add -A && git commit -q -m agents", { cwd: projectPath, stdio: "ignore" }); +} + async function collectFullHistory(service: HistoryService, workspaceId: string) { const messages: MuxMessage[] = []; const result = await service.iterateFullHistory(workspaceId, "forward", (chunk) => { @@ -279,6 +298,7 @@ async function saveLocalParentWorkspace( agentAiDefaults?: AgentAiDefaults; subagentAiDefaults?: Record; parentAiSettings?: { model: string; thinkingLevel: ThinkingLevel }; + workspaceName?: string; } ): Promise<{ parentId: string; projectPath: string }> { const projectPath = await createTestProject(rootDir, "repo", { initGit: false }); @@ -290,7 +310,7 @@ async function saveLocalParentWorkspace( { path: projectPath, id: parentId, - name: "parent", + name: options?.workspaceName ?? "parent", createdAt: new Date().toISOString(), runtimeConfig: { type: "local" }, aiSettings: options?.parentAiSettings ?? { @@ -967,6 +987,882 @@ describe("TaskService", () => { }); }); + test("createWorkspaceTurn launches a new workspace with an explicit agent id", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: createWorkspaceTurnMetadata(projectPath) })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "plan", + prompt: "Plan a small change", + title: "Plan dogfood", + workspace: { mode: "new" }, + }); + + expect(result.success).toBe(true); + expect(sendMessage).toHaveBeenCalledTimes(1); + const sendMessageCall = sendMessage.mock.calls[0] as unknown[]; + expect(sendMessageCall[0]).toBe("childworkspace"); + // Explicit overrides also arm stream-time strict resolution, pinning the validated + // definition's provenance (scope + exact source): pre-dispatch validation races + // init hooks/user edits, so the stream must fail loudly instead of silently + // swapping in exec (or running a different definition for the same id) post-init. + expect(sendMessageCall[2]).toMatchObject({ + agentId: "plan", + strictAgentResolution: { + expectedScope: "built-in", + expectedSource: "built-in", + // The full base chain is pinned too: stream-time inheritance resolution + // reloads every base independently. + expectedChain: [{ id: "plan", scope: "built-in", source: "built-in" }], + }, + }); + }); + + test("createWorkspaceTurn keeps prechecks advisory when the owner has uncommitted agent changes", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + checkoutOwnerBranch(projectPath, "parent"); + // A GITIGNORED hidden shadow of the built-in plan exists only in the owner's + // working tree (plain `git status` would not even list it): the new checkout is + // created from committed branch state and validly resolves the built-in, so the + // owner-side miss must stay advisory (branch equality is not checkout equality). + await fsPromises.writeFile(path.join(projectPath, ".gitignore"), ".mux/agents/\n"); + commitOwnerAgentFiles(projectPath); + const agentsDir = path.join(projectPath, ".mux", "agents"); + await fsPromises.mkdir(agentsDir, { recursive: true }); + await fsPromises.writeFile( + path.join(agentsDir, "plan.md"), + ["---", "name: Plan", "base: plan", "ui:", " hidden: true", "---", "Shadow."].join("\n") + ); + + const cleanCheckout = path.join(rootDir, "clean-target-checkout"); + await fsPromises.mkdir(cleanCheckout, { recursive: true }); + const targetMetadata: WorkspaceMetadata & { namedWorkspacePath: string } = { + ...createWorkspaceTurnMetadata(projectPath), + runtimeConfig: { type: "worktree", srcBaseDir: path.join(rootDir, "wt") }, + namedWorkspacePath: cleanCheckout, + }; + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: targetMetadata })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "plan", + prompt: "Plan from the committed base", + title: "Dirty owner shadow", + workspace: { mode: "new" }, + }); + + expect(result.success).toBe(true); + expect(sendMessage).toHaveBeenCalledTimes(1); + const sendMessageCall = sendMessage.mock.calls[0] as unknown[]; + expect(sendMessageCall[2]).toMatchObject({ agentId: "plan" }); + }); + + test("createWorkspaceTurn owner-side misses are always advisory (target decides)", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + checkoutOwnerBranch(projectPath, "parent"); + + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: createWorkspaceTurnMetadata(projectPath) })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + // No owner-side equivalence proof is sound (fetched origin commits, existing + // branchName targets, submodules, init hooks): the created checkout is the only + // authoritative source, so a miss defers instead of rejecting pre-create. + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "doesnotexist", + prompt: "Should defer to the target", + title: "Advisory miss", + workspace: { mode: "new" }, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("unknown agentId"); + expect(result.error).toContain("no turn was dispatched"); + } + expect(createWorkspace).toHaveBeenCalledTimes(1); + expect(sendMessage).not.toHaveBeenCalled(); + }); + + test("createWorkspaceTurn rejects bad agent ids without ever dispatching a turn", async () => { + const config = await createTestConfig(rootDir); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + checkoutOwnerBranch(projectPath, "parent"); + + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: createWorkspaceTurnMetadata(projectPath) })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const attempt = (agentId: string) => + taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId, + prompt: "Should not run", + title: "Bad agent", + workspace: { mode: "new" }, + }); + + // Syntactically invalid ids are checkout-independent and fail before any + // workspace exists. + const invalidSyntax = await attempt("Not A Valid Id!"); + expect(invalidSyntax.success).toBe(false); + if (!invalidSyntax.success) expect(invalidSyntax.error).toContain("invalid agentId"); + expect(createWorkspace).not.toHaveBeenCalled(); + + // Definition-dependent verdicts are decided by the created target checkout + // (owner-side prechecks are advisory): unknown and internal (ui.hidden) ids + // fail there, with the workspace retained as owned evidence and no dispatch. + const unknown = await attempt("doesnotexist"); + expect(unknown.success).toBe(false); + if (!unknown.success) { + expect(unknown.error).toContain("unknown agentId"); + expect(unknown.error).toContain("no turn was dispatched"); + } + + const internal = await attempt("compact"); + expect(internal.success).toBe(false); + if (!internal.success) { + expect(internal.error).toContain("not selectable"); + expect(internal.error).toContain("no turn was dispatched"); + } + + expect(sendMessage).not.toHaveBeenCalled(); + }); + + test("createWorkspaceTurn rejects disabled agents without dispatching a turn", async () => { + const config = await createTestConfig(rootDir); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir, { + agentAiDefaults: { custom: { enabled: false } }, + }); + checkoutOwnerBranch(projectPath, "parent"); + await writeCustomAgentDefinition(projectPath); + commitOwnerAgentFiles(projectPath); + + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: createWorkspaceTurnMetadata(projectPath) })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "custom", + prompt: "Should not run", + title: "Disabled agent", + workspace: { mode: "new" }, + }); + + // Enablement is decided at the created target checkout (owner prechecks are + // advisory); the disabled verdict settles post-create with no dispatch. + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("disabled"); + expect(result.error).toContain("no turn was dispatched"); + } + expect(sendMessage).not.toHaveBeenCalled(); + }); + + test("createWorkspaceTurn does not dispatch when the agent is unavailable in the created workspace", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + // Project-local agent exists in the OWNER's checkout, but the created workspace's checkout + // diverges (no agent definition there) — post-create re-validation must fail instead of + // silently streaming exec. Worktree runtime: the only local runtime whose created + // workspaces get a checkout separate from the project root. + await writeCustomAgentDefinition(projectPath); + const divergedCheckout = path.join(rootDir, "diverged-checkout"); + await fsPromises.mkdir(divergedCheckout, { recursive: true }); + + const divergedMetadata: WorkspaceMetadata & { namedWorkspacePath: string } = { + ...createWorkspaceTurnMetadata(projectPath), + runtimeConfig: { type: "worktree", srcBaseDir: path.join(rootDir, "wt") }, + namedWorkspacePath: divergedCheckout, + }; + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: divergedMetadata })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "custom", + prompt: "Should not dispatch", + title: "Diverged agent", + workspace: { mode: "new" }, + // Background launch: on this synchronous failure the policy must NOT be persisted — + // settleWorkspaceTurn derives the terminal wake from the persisted record, which + // would duplicate the Err returned directly to the caller. + attentionPolicy: "notify_on_terminal", + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("no turn was dispatched"); + } + expect(createWorkspace).toHaveBeenCalledTimes(1); + expect(sendMessage).not.toHaveBeenCalled(); + + // The failure settles through the handle machinery, so the created workspace stays + // owner-owned: a mode="existing" retry must pass the ownership check (not invalid_scope). + const turns = await ( + taskService as unknown as { + taskHandleStore: { + listAllWorkspaceTurns: () => Promise>; + }; + } + ).taskHandleStore.listAllWorkspaceTurns(); + expect(turns).toHaveLength(1); + expect(turns[0]).toMatchObject({ status: "error", createdWorkspace: true }); + expect(turns[0]?.attentionPolicy).toBeUndefined(); + + const retry = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + prompt: "Retry without the diverged agent", + title: "Diverged agent retry", + workspace: { mode: "existing", workspaceId: "childworkspace" }, + }); + expect(retry.success).toBe(true); + expect(sendMessage).toHaveBeenCalledTimes(1); + }); + + test("createWorkspaceTurn respects a project shadow of a built-in id at the target", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + checkoutOwnerBranch(projectPath, "parent"); + // Shadow the built-in plan agent with a hidden project-local override: target-side + // eligibility must consult the shadow, not just the embedded definition. + const agentsDir = path.join(projectPath, ".mux", "agents"); + await fsPromises.mkdir(agentsDir, { recursive: true }); + await fsPromises.writeFile( + path.join(agentsDir, "plan.md"), + [ + "---", + "name: Plan", + "description: Hidden shadowed plan", + "base: plan", + "ui:", + " hidden: true", + "---", + "", + "Shadow body.", + "", + ].join("\n"), + "utf-8" + ); + commitOwnerAgentFiles(projectPath); + + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: createWorkspaceTurnMetadata(projectPath) })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "plan", + prompt: "Should not run", + title: "Shadowed plan", + workspace: { mode: "new" }, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("not selectable"); + expect(result.error).toContain("no turn was dispatched"); + } + expect(sendMessage).not.toHaveBeenCalled(); + }); + + test("createWorkspaceTurn divergent trunkBranch defers validation to the target checkout", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + // Agent exists ONLY on the target branch checkout, not in the owner's checkout: an + // owner-side miss must not fail-fast when a different base branch was requested. + const targetBranchCheckout = path.join(rootDir, "target-branch-checkout"); + const targetAgentsDir = path.join(targetBranchCheckout, ".mux", "agents"); + await fsPromises.mkdir(targetAgentsDir, { recursive: true }); + await fsPromises.writeFile( + path.join(targetAgentsDir, "custom.md"), + [ + "---", + "name: Custom", + "description: Target-branch-only agent", + "base: exec", + "---", + "", + "Body.", + "", + ].join("\n"), + "utf-8" + ); + + const targetMetadata: WorkspaceMetadata & { namedWorkspacePath: string } = { + ...createWorkspaceTurnMetadata(projectPath), + runtimeConfig: { type: "worktree", srcBaseDir: path.join(rootDir, "wt") }, + namedWorkspacePath: targetBranchCheckout, + }; + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: targetMetadata })) + ); + const sendMessage = mock( + (..._args: unknown[]): Promise> => Promise.resolve(Ok(undefined)) + ); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "custom", + prompt: "Run the target-branch agent", + title: "Target branch agent", + workspace: { mode: "new", trunkBranch: "feature-branch" }, + }); + + expect(result.success).toBe(true); + expect(sendMessage).toHaveBeenCalledTimes(1); + expect(sendMessage.mock.calls[0]?.[2]).toMatchObject({ agentId: "custom" }); + }); + + test("createWorkspaceTurn divergent trunkBranch fails closed for unreachable targets", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + + // A different base branch can shadow ANY id (even built-ins), so an unreachable + // target created from it cannot be verified at all. + const unreachableMetadata: WorkspaceMetadata & { namedWorkspacePath: string } = { + ...createWorkspaceTurnMetadata(projectPath), + runtimeConfig: { type: "worktree", srcBaseDir: path.join(rootDir, "wt") }, + namedWorkspacePath: path.join(rootDir, "not-provisioned-branch"), + }; + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: unreachableMetadata })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "plan", + prompt: "Should not dispatch", + title: "Divergent unreachable", + workspace: { mode: "new", trunkBranch: "feature-branch" }, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("not reachable"); + expect(result.error).toContain("no turn was dispatched"); + } + expect(sendMessage).not.toHaveBeenCalled(); + }); + + test("createWorkspaceTurn unreachable created checkout: built-ins launch, custom agents fail with a reachability error", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + checkoutOwnerBranch(projectPath, "parent"); + await writeCustomAgentDefinition(projectPath); + commitOwnerAgentFiles(projectPath); + // Deferred-provisioning runtimes return from create before the checkout is reachable. + const unreachableCheckout = path.join(rootDir, "not-provisioned-yet"); + + const deferredMetadata: WorkspaceMetadata & { namedWorkspacePath: string } = { + ...createWorkspaceTurnMetadata(projectPath), + runtimeConfig: { type: "worktree", srcBaseDir: path.join(rootDir, "wt") }, + namedWorkspacePath: unreachableCheckout, + }; + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: deferredMetadata })) + ); + const sendMessage = mock( + (..._args: unknown[]): Promise> => Promise.resolve(Ok(undefined)) + ); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + // Custom agents cannot be verified in an unreachable checkout; dispatching anyway + // would risk a silent exec fallback at stream time, so the launch must fail loudly. + const custom = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "custom", + prompt: "Launch despite pending provisioning", + title: "Deferred runtime", + workspace: { mode: "new" }, + }); + expect(custom.success).toBe(false); + if (!custom.success) { + expect(custom.error).toContain("not reachable"); + expect(custom.error).toContain("no turn was dispatched"); + } + expect(sendMessage).not.toHaveBeenCalled(); + + // Built-in agents are embedded in every checkout, so the launch is provably safe. + const builtIn = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "plan", + prompt: "Plan despite pending provisioning", + title: "Deferred runtime plan", + workspace: { mode: "new" }, + }); + expect(builtIn.success).toBe(true); + expect(sendMessage).toHaveBeenCalledTimes(1); + const sendMessageCall = sendMessage.mock.calls[0]; + expect(sendMessageCall?.[2]).toMatchObject({ agentId: "plan" }); + }); + + test("createWorkspaceTurn treats sanitized branch-name collisions as unproven bases", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + // Owner is checked out on feature/foo, whose workspace name sanitizes to feature-foo. + // A request for the DISTINCT branch feature-foo collides with that name, so the owner + // must not vouch for the unreachable target: even a built-in id fails closed (the + // colliding branch could shadow it). + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir, { + workspaceName: "feature-foo", + }); + checkoutOwnerBranch(projectPath, "feature/foo"); + + const unreachableMetadata: WorkspaceMetadata & { namedWorkspacePath: string } = { + ...createWorkspaceTurnMetadata(projectPath), + runtimeConfig: { type: "worktree", srcBaseDir: path.join(rootDir, "wt") }, + namedWorkspacePath: path.join(rootDir, "not-provisioned-collision"), + }; + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: unreachableMetadata })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "plan", + prompt: "Should not dispatch", + title: "Colliding branch", + workspace: { mode: "new", trunkBranch: "feature-foo" }, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("not reachable"); + expect(result.error).toContain("no turn was dispatched"); + } + expect(sendMessage).not.toHaveBeenCalled(); + + // The owner's real branch, by contrast, is a proven base: the same launch succeeds. + const sameBranch = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "plan", + prompt: "Plan from the owner's own branch", + title: "Same branch", + workspace: { mode: "new", trunkBranch: "feature/foo" }, + }); + expect(sameBranch.success).toBe(true); + expect(sendMessage).toHaveBeenCalledTimes(1); + }); + + test("createWorkspaceTurn defers owner-side misses to the target when the base is unproven", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + // Owner is on feature/foo but its workspace name is feature-foo: with trunkBranch + // omitted, the child is created from the DISTINCT feature-foo branch, which may carry + // agents absent from the owner's branch. An owner-side miss must not fail-fast here — + // the created target checkout is authoritative. + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir, { + workspaceName: "feature-foo", + }); + checkoutOwnerBranch(projectPath, "feature/foo"); + + const targetOnlyCheckout = path.join(rootDir, "target-only-agent"); + const targetAgentsDir = path.join(targetOnlyCheckout, ".mux", "agents"); + await fsPromises.mkdir(targetAgentsDir, { recursive: true }); + await fsPromises.writeFile( + path.join(targetAgentsDir, "custom.md"), + ["---", "name: Custom", "base: exec", "---", "Target-only agent."].join("\n") + ); + + const targetMetadata: WorkspaceMetadata & { namedWorkspacePath: string } = { + ...createWorkspaceTurnMetadata(projectPath), + runtimeConfig: { type: "worktree", srcBaseDir: path.join(rootDir, "wt") }, + namedWorkspacePath: targetOnlyCheckout, + }; + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: targetMetadata })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "custom", + prompt: "Run the target-only agent", + title: "Target-only agent", + workspace: { mode: "new" }, + }); + + expect(result.success).toBe(true); + expect(sendMessage).toHaveBeenCalledTimes(1); + const sendMessageCall = sendMessage.mock.calls[0] as unknown[]; + expect(sendMessageCall[2]).toMatchObject({ agentId: "custom" }); + }); + + test("createWorkspaceTurn unreachable cross-host target fails closed even for built-ins", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + checkoutOwnerBranch(projectPath, "parent"); + + // The created target lives on a remote host (per-workspace containers, Coder-style + // per-workspace hosts). The owner's global agent roots say nothing about that host — + // even a built-in could be shadowed by a target-host global definition — so + // owner-side resolution must not vouch while the checkout is unreachable. + const remoteMetadata: WorkspaceMetadata & { namedWorkspacePath: string } = { + ...createWorkspaceTurnMetadata(projectPath), + runtimeConfig: { type: "docker", image: "node:20" }, + namedWorkspacePath: "/workspace/repo", + }; + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: remoteMetadata })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "plan", + prompt: "Should not dispatch", + title: "Cross-host unreachable", + workspace: { mode: "new" }, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("different host"); + expect(result.error).toContain("no turn was dispatched"); + } + expect(sendMessage).not.toHaveBeenCalled(); + }); + + test("createWorkspaceTurn does not verify agents while the created workspace is still initializing", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "turnhandle"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + await writeCustomAgentDefinition(projectPath); + // Reachable checkout whose init hook is still running: the hook may still be + // installing/rewriting agent definitions, so a strict-validation miss is not a + // trustworthy "unknown agentId" verdict — the launch must fail with a transient + // error instead of a definitive one (and never dispatch an unverified id). + const initializingCheckout = path.join(rootDir, "initializing-checkout"); + await fsPromises.mkdir(initializingCheckout, { recursive: true }); + + const initializingMetadata: WorkspaceMetadata & { namedWorkspacePath: string } = { + ...createWorkspaceTurnMetadata(projectPath), + runtimeConfig: { type: "worktree", srcBaseDir: path.join(rootDir, "wt") }, + namedWorkspacePath: initializingCheckout, + }; + const createWorkspace = mock( + (): Promise> => + Promise.resolve(Ok({ metadata: initializingMetadata })) + ); + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const initStateManager = { + startInit: mock(() => undefined), + enterHookPhase: mock(() => undefined), + appendOutput: mock(() => undefined), + endInit: mock(() => Promise.resolve()), + getInitState: mock((workspaceId: string) => + workspaceId === "childworkspace" ? { status: "running" } : undefined + ), + readInitStatus: mock(() => Promise.resolve(null)), + } as unknown as InitStateManager; + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + initStateManager, + }); + + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "custom", + prompt: "Should not dispatch", + title: "Initializing target", + workspace: { mode: "new" }, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("still initializing"); + expect(result.error).toContain("no turn was dispatched"); + } + expect(sendMessage).not.toHaveBeenCalled(); + }); + + test("createWorkspaceTurn unreachable existing target: explicit overrides fail closed, default identity works", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, [ + "childworkspace", + "firstturn", + "planhandle", + "planturn", + "customhandle", + ]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + await writeCustomAgentDefinition(projectPath); + // Simulates a stopped-container/deferred target: entry exists, checkout unreachable. + const unreachableCheckout = path.join(rootDir, "stopped-target"); + + const createWorkspace = mock(async (): Promise> => { + await config.editConfig((cfg) => { + const project = cfg.projects.get(projectPath); + assert(project, "test project must exist"); + project.workspaces.push({ + path: unreachableCheckout, + id: "childworkspace", + name: "workspace-turn", + title: "Workspace turn", + createdAt: "2026-06-19T00:00:00.000Z", + runtimeConfig: { type: "worktree", srcBaseDir: path.join(rootDir, "wt") }, + }); + return cfg; + }); + return Ok({ + metadata: { + ...createWorkspaceTurnMetadata(projectPath), + runtimeConfig: { type: "worktree", srcBaseDir: path.join(rootDir, "wt") }, + }, + }); + }); + const sendMessage = mock(async (...args: unknown[]): Promise> => { + const internal = args[3] as { onAccepted?: () => Promise | void } | undefined; + await internal?.onAccepted?.(); + return Ok(undefined); + }); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const first = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + prompt: "Initial turn", + title: "Workspace turn", + workspace: { mode: "new" }, + }); + expect(first.success).toBe(true); + expect(sendMessage).toHaveBeenCalledTimes(1); + + // Existing targets have unknown checkout provenance (any branch, uncommitted shadows), + // so ALL explicit overrides fail closed while the checkout is unreachable — even + // built-ins, whose id could be shadowed by a project definition on the target. + for (const agentId of ["plan", "custom"]) { + const overridden = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId, + prompt: `${agentId} follow-up`, + title: `${agentId} follow-up`, + workspace: { mode: "existing", workspaceId: "childworkspace" }, + }); + expect(overridden.success).toBe(false); + if (!overridden.success) { + expect(overridden.error).toContain("not reachable"); + expect(overridden.error).not.toContain("unknown agentId"); + } + } + expect(sendMessage).toHaveBeenCalledTimes(1); + + // Omitting agentId keeps working: the default identity needs no verification. + const withoutOverride = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + prompt: "Default follow-up", + title: "Default follow-up", + workspace: { mode: "existing", workspaceId: "childworkspace" }, + }); + expect(withoutOverride.success).toBe(true); + expect(sendMessage).toHaveBeenCalledTimes(2); + }); + + test("createWorkspaceTurn rejects explicit agentId for descendant agent workspace targets", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["overridehandle", "overrideturn"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + const childWorkspaceId = "reported-child-override"; + await config.editConfig((cfg) => { + const project = cfg.projects.get(projectPath); + assert(project, "test project must exist"); + project.workspaces.push({ + path: path.join(projectPath, "reported-child"), + id: childWorkspaceId, + name: "agent_explore_reported_child", + createdAt: "2026-06-19T00:00:00.000Z", + parentWorkspaceId: parentId, + agentType: "explore", + taskStatus: "reported", + reportedAt: "2026-06-19T00:00:00.000Z", + runtimeConfig: { type: "local" }, + }); + return cfg; + }); + + const sendMessage = mock((): Promise> => Promise.resolve(Ok(undefined))); + const { workspaceService } = createWorkspaceServiceMocks({ sendMessage }); + const { taskService } = createTaskServiceHarness(config, { workspaceService }); + + // Persistent children are pinned to their persisted identity at stream time + // (resolveAgentForStream ignores per-send agentId when parentWorkspaceId is set), + // so an override must be rejected instead of silently running the old agent. + const result = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "plan", + prompt: "Re-plan the follow-up", + title: "Override turn", + allowAgentWorkspace: true, + workspace: { mode: "existing", workspaceId: childWorkspaceId }, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("descendant agent workspaces"); + } + expect(sendMessage).not.toHaveBeenCalled(); + const childEntry = findWorkspaceInConfig(config, childWorkspaceId); + expect(childEntry?.agentType).toBe("explore"); + expect(childEntry?.agentId).toBeUndefined(); + }); + + test("createWorkspaceTurn existing-target agent override dispatches without persisting AI settings", async () => { + const config = await createTestConfig(rootDir); + stubStableIds(config, ["childworkspace", "firstturn", "followuphandle", "followupturn"]); + const { parentId, projectPath } = await saveLocalParentWorkspace(config, rootDir); + + const createWorkspace = mock(async (): Promise> => { + await config.editConfig((cfg) => { + const project = cfg.projects.get(projectPath); + assert(project, "test project must exist"); + project.workspaces.push({ + // Project-dir local workspaces execute in the project root itself. + path: projectPath, + id: "childworkspace", + name: "workspace-turn", + title: "Workspace turn", + createdAt: "2026-06-19T00:00:00.000Z", + runtimeConfig: { type: "local" }, + }); + return cfg; + }); + return Ok({ metadata: createWorkspaceTurnMetadata(projectPath) }); + }); + const sendMessage = mock(async (...args: unknown[]): Promise> => { + const internal = args[3] as { onAccepted?: () => Promise | void } | undefined; + await internal?.onAccepted?.(); + return Ok(undefined); + }); + const workspaceMocks = createWorkspaceServiceMocks({ create: createWorkspace, sendMessage }); + const { taskService } = createTaskServiceHarness(config, { + workspaceService: workspaceMocks.workspaceService, + }); + + const first = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + prompt: "Initial turn", + title: "Workspace turn", + workspace: { mode: "new" }, + }); + expect(first.success).toBe(true); + + const followUp = await taskService.createWorkspaceTurn({ + ownerWorkspaceId: parentId, + agentId: "plan", + prompt: "Per-turn plan follow-up", + title: "Override follow-up", + workspace: { mode: "existing", workspaceId: "childworkspace" }, + }); + expect(followUp.success).toBe(true); + + expect(sendMessage).toHaveBeenCalledTimes(2); + const followUpCall = sendMessage.mock.calls[1]; + expect(followUpCall[0]).toBe("childworkspace"); + // Override reaches the stream (normal workspaces honor the per-send agentId) but must + // not overwrite the target's saved agent/settings. + expect(followUpCall[2]).toMatchObject({ agentId: "plan", skipAiSettingsPersistence: true }); + // The default path keeps persisting (first send carries no override). + expect(sendMessage.mock.calls[0]?.[2]).not.toMatchObject({ skipAiSettingsPersistence: true }); + }); + test("createWorkspaceTurn inherits pro mode from the parent's active non-exec agent", async () => { const config = await createTestConfig(rootDir); stubStableIds(config, ["childworkspace", "turnhandle"]); diff --git a/src/node/services/taskService.ts b/src/node/services/taskService.ts index 2e462aef2f..2b7a5e3d13 100644 --- a/src/node/services/taskService.ts +++ b/src/node/services/taskService.ts @@ -46,6 +46,7 @@ import { } from "@/node/services/agentDefinitions/agentDefinitionsService"; import { resolveAgentInheritanceChain } from "@/node/services/agentDefinitions/resolveAgentInheritanceChain"; import { isAgentEffectivelyDisabled } from "@/node/services/agentDefinitions/agentEnablement"; +import { resolveAgentVisibility } from "@/node/services/agentDefinitions/agentVisibility"; import { orchestrateFork } from "@/node/services/utils/forkOrchestrator"; import { createRuntimeContextForWorkspace, @@ -57,9 +58,13 @@ import type { InitLogger, Runtime } from "@/node/runtime/Runtime"; import { readPlanFile } from "@/node/utils/runtime/helpers"; import { coerceNonEmptyString, + tryReadGitBranchMatchesOrigin, + tryReadGitCurrentBranch, tryReadGitHeadCommitSha, + tryReadGitPathsClean, findWorkspaceEntry, } from "@/node/services/taskUtils"; +import { listProjectMetadataRelativePaths } from "@/common/compat/legacyMux"; import { validateWorkspaceName } from "@/common/utils/validation/workspaceValidation"; import { getTaskGroupCount } from "@/common/utils/tools/taskGroups"; import { stripTrailingSlashes } from "@/node/utils/pathUtils"; @@ -81,10 +86,15 @@ import { defaultModel, normalizeSelectedModel } from "@/common/utils/ai/models"; import { EXPERIMENT_IDS } from "@/common/constants/experiments"; import { SCRATCH_PROJECT_CONFIG_KEY, SCRATCH_PROJECT_NAME } from "@/common/constants/scratch"; import { DEFAULT_RUNTIME_CONFIG } from "@/common/constants/workspace"; -import { runtimeModeSupportsSharedTaskWorkspace, type RuntimeConfig } from "@/common/types/runtime"; +import { + RUNTIME_MODE, + runtimeModeSupportsSharedTaskWorkspace, + type RuntimeConfig, +} from "@/common/types/runtime"; import type { ProjectRef, WorkspaceMetadata } from "@/common/types/workspace"; import { getRuntimeType } from "@/node/runtime/initHook"; import { AgentIdSchema } from "@/common/orpc/schemas"; +import type { AgentDefinitionScope } from "@/common/types/agentDefinition"; import { normalizeAgentId, resolvePersistedAgentId, @@ -517,10 +527,72 @@ function isAgentRunnableAsChild( type WorkspaceTurnQueueDispatchMode = "tool-end" | "turn-end"; +/** + * Project-relative paths that contribute agent definitions to discovery + * (project agent roots, project plugin containers). Owner-side vouching for a + * freshly created checkout requires these to be clean in the owner: the child is + * created from committed branch state, so uncommitted changes here make + * owner-side agent resolution diverge from what the target will see. + */ +const AGENT_DEFINITION_PROJECT_PATHSPECS: readonly string[] = [ + ...listProjectMetadataRelativePaths("agents"), + ...listProjectMetadataRelativePaths("plugins"), + ".agents/plugins", +]; + +/** Provenance of one hop of a validated agent's base chain (strict-send pinning). */ +interface WorkspaceTurnAgentChainEntry { + id: string; + scope: AgentDefinitionScope; + source?: string; +} + +/** Agent-discovery context for a workspace involved in a workspace turn. */ +interface WorkspaceTurnAgentContext { + runtime: Runtime; + workspacePath: string; + includeAgentPlugins: boolean; + /** Source config, kept so owner/target contexts can be compared for host identity. */ + runtimeConfig: RuntimeConfig; +} + +/** + * Whether agent discovery for both runtime configs reads the same host filesystem, + * i.e. the owner's global/plugin agent roots are literally the target's roots. + * Local-family runtimes (local/worktree) share the local machine. Plain SSH shares + * the remote home only for an identical host/port with no Coder indirection — + * CoderSSHRuntime.finalizeConfig derives a distinct per-workspace host, so a Coder + * owner's ~/.xum/agents says nothing about the child's. Docker/devcontainer get + * per-workspace containers and never share. + */ +function runtimeConfigsShareAgentHost(a: RuntimeConfig, b: RuntimeConfig): boolean { + const isLocalFamily = (rc: RuntimeConfig): boolean => + rc.type === RUNTIME_MODE.LOCAL || rc.type === RUNTIME_MODE.WORKTREE; + if (isLocalFamily(a) && isLocalFamily(b)) { + return true; + } + if (a.type === RUNTIME_MODE.SSH && b.type === RUNTIME_MODE.SSH) { + return ( + a.coder == null && b.coder == null && a.host === b.host && (a.port ?? 22) === (b.port ?? 22) + ); + } + return false; +} + export interface WorkspaceTurnCreateArgs { ownerWorkspaceId: string; prompt: string; title: string; + /** + * Agent mode for the launched turn (e.g. "plan"). Defaults to exec for new + * workspaces and to the resumed identity for existing descendant agent + * workspaces. For a new workspace the requested agent becomes its default; + * on an existing normal workspace it is a per-turn override dispatched with + * AI-settings persistence disabled, so the target's saved agent/settings are + * untouched. Rejected for descendant agent workspaces, whose persisted + * identity always wins at stream time. + */ + agentId?: string; modelString?: string; thinkingLevel?: ParsedThinkingInput; parentRuntimeAiSettings?: { modelString?: string; thinkingLevel?: ThinkingLevel }; @@ -3604,6 +3676,241 @@ export class TaskService { this.scheduleMaybeStartQueuedTasks(); } + /** + * Agent-discovery context for a workspace involved in a workspace turn. Uses + * createRuntimeContextForWorkspace — the same helper the stream uses in + * aiService — so validation resolves agents from the exact discovery path that + * will stream (Docker container-side paths, subproject directories included). + */ + private buildWorkspaceTurnAgentContext(params: { + runtimeConfig: RuntimeConfig; + projectPath: string; + workspaceName: string; + persistedWorkspacePath?: string; + subProjectPath?: string; + }): WorkspaceTurnAgentContext { + const context = createRuntimeContextForWorkspace({ + runtimeConfig: params.runtimeConfig, + projectPath: params.projectPath, + name: params.workspaceName, + namedWorkspacePath: coerceNonEmptyString(params.persistedWorkspacePath), + subProjectPath: coerceNonEmptyString(params.subProjectPath), + }); + return { + ...context, + includeAgentPlugins: this.workspaceService.isExperimentEnabled(EXPERIMENT_IDS.AGENT_PLUGINS), + runtimeConfig: params.runtimeConfig, + }; + } + + /** + * Fail-fast eligibility check for an explicit workspace-turn agentId. + * resolveAgentForStream silently falls back to exec for top-level workspaces, + * which would hide a caller's mistake — so unknown, internal (ui.hidden), and + * disabled agents are rejected here before any turn is dispatched. Mirrors + * the UI agent picker rule set (including Agent Plugins roots when that + * experiment is enabled), so custom user-visible agents pass without a + * hardcoded allowlist. + */ + private async validateWorkspaceTurnAgentId(params: { + cfg: ReturnType; + agentId: string; + /** Workspace the validation is for (log correlation in chain resolution). */ + workspaceId: string; + /** Discovery context of the workspace whose turn will run (or the owner pre-create). */ + runtime: Runtime; + workspacePath: string; + includeAgentPlugins: boolean; + }): Promise< + Result< + { + scope: AgentDefinitionScope; + source?: string; + chain: WorkspaceTurnAgentChainEntry[]; + }, + string + > + > { + assert(params.agentId.length > 0, "validateWorkspaceTurnAgentId: agentId must be non-empty"); + const parsedAgentId = AgentIdSchema.safeParse(params.agentId); + if (!parsedAgentId.success) { + return Err(`Task.createWorkspaceTurn: invalid agentId (${params.agentId})`); + } + // The winning definition's provenance (scope + exact source) AND the resolved + // base chain's provenance are captured so the dispatched turn can pin them at + // stream time (strictAgentResolution): a validated definition or base shadow + // vanishing must not let a different candidate with the same id run under a + // strict send. + let scope: AgentDefinitionScope; + let source: string | undefined; + let chain: WorkspaceTurnAgentChainEntry[]; + try { + const definition = await readAgentDefinition( + params.runtime, + params.workspacePath, + parsedAgentId.data, + { includeAgentPlugins: params.includeAgentPlugins } + ); + scope = definition.scope; + source = definition.source; + const resolvedChain = await resolveAgentInheritanceChain({ + runtime: params.runtime, + workspacePath: params.workspacePath, + agentId: parsedAgentId.data, + agentDefinition: definition, + workspaceId: params.workspaceId, + includeAgentPlugins: params.includeAgentPlugins, + }); + chain = resolvedChain.map((entry) => ({ + id: entry.id, + scope: entry.scope, + ...(entry.source != null ? { source: entry.source } : {}), + })); + } catch { + return Err(`Task.createWorkspaceTurn: unknown agentId (${params.agentId})`); + } + let frontmatter: Awaited>; + try { + frontmatter = await resolveAgentFrontmatter( + params.runtime, + params.workspacePath, + params.agentId, + { includeAgentPlugins: params.includeAgentPlugins } + ); + } catch { + return Err(`Task.createWorkspaceTurn: unknown agentId (${params.agentId})`); + } + if (!resolveAgentVisibility(frontmatter.ui).selectable) { + return Err( + `Task.createWorkspaceTurn: agentId is not selectable for workspace turns (${params.agentId})` + ); + } + if ( + isAgentEffectivelyDisabled({ + cfg: params.cfg, + agentId: params.agentId, + resolvedFrontmatter: frontmatter, + }) + ) { + return Err(`Task.createWorkspaceTurn: agentId is disabled (${params.agentId})`); + } + return Ok({ scope, ...(source != null ? { source } : {}), chain }); + } + + /** + * Validate an explicit agentId against the TARGET workspace's checkout, tolerating + * targets that are not reachable yet (deferred provisioning, stopped containers) + * without permitting a silent exec fallback later: + * - reachable checkout: strict validation against the target. If that fails while + * the target's init hook is still running, the verdict is not trustworthy (the + * hook may still be installing/rewriting agent definitions), so the launch fails + * with an explicitly transient error instead of a definitive "unknown agentId"; + * - unreachable checkout: resolve the definition via the OWNER's context, but only + * when the owner and target run agent discovery on the same host filesystem + * (runtimeConfigsShareAgentHost) — cross-host (Coder per-workspace hosts, per- + * workspace containers), the owner's global roots say nothing about the target's, + * and even a built-in could be shadowed by a target-host global definition, so + * everything fails closed. Same-host, project shadows and global roots are + * visible through the owner: a checkout-dependent (project-scope) winner still + * fails with a reachability error — it cannot be verified in the target and + * dispatching anyway would let resolveAgentForStream silently fall back to exec + * (wrong prompt/tool policy) — while host-side definitions (built-in, global, + * plugin) are validated against the owner context and the launch proceeds. + * Waiting for provisioning/init here is not an option: createWorkspaceTurn holds + * the service-wide task mutex for its whole body. Failures in these windows settle + * as retryable errors (mode="existing" once the target is ready) rather than + * dispatching an unverified id. + */ + private async validateWorkspaceTurnAgentIdForTarget(params: { + cfg: ReturnType; + agentId: string; + /** Target workspace id (log correlation in chain resolution). */ + workspaceId: string; + target: WorkspaceTurnAgentContext; + owner: WorkspaceTurnAgentContext; + /** Whether the target workspace's init hook is still running (see doc above). */ + targetInitPending: boolean; + /** + * Whether the owner's checkout is a sound predictor of the target's agent + * definitions. Only true for workspaces this call just created from the branch + * verified (via git) to be checked out in the owner right now. False for existing + * targets (their checkout has unknown provenance — any branch, uncommitted shadows) + * and for new workspaces whose base branch differs from or cannot be proven equal + * to the owner's. When false, unreachable targets fail closed instead of trusting + * owner-side resolution. + */ + ownerResolutionPredictsTarget: boolean; + }): Promise< + Result< + { + validatedContext: WorkspaceTurnAgentContext; + scope: AgentDefinitionScope; + source?: string; + chain: WorkspaceTurnAgentChainEntry[]; + }, + string + > + > { + const reachable = await runtimePathExists(params.target.runtime, params.target.workspacePath); + if (reachable) { + const validation = await this.validateWorkspaceTurnAgentId({ + cfg: params.cfg, + agentId: params.agentId, + workspaceId: params.workspaceId, + ...params.target, + }); + if (validation.success) { + return Ok({ validatedContext: params.target, ...validation.data }); + } + if (params.targetInitPending) { + return Err( + `Task.createWorkspaceTurn: the target workspace is still initializing, so agentId (${params.agentId}) could not be verified yet (${validation.error})` + ); + } + return validation; + } + if (!params.ownerResolutionPredictsTarget) { + return Err( + `Task.createWorkspaceTurn: target checkout is not reachable (provisioning, stopped runtime, or unknown checkout state), so agentId (${params.agentId}) cannot be verified there` + ); + } + if (!runtimeConfigsShareAgentHost(params.owner.runtimeConfig, params.target.runtimeConfig)) { + return Err( + `Task.createWorkspaceTurn: target checkout is not reachable and runs on a different host than the owner, so agentId (${params.agentId}) cannot be verified there` + ); + } + const parsedAgentId = AgentIdSchema.safeParse(params.agentId); + if (!parsedAgentId.success) { + return Err(`Task.createWorkspaceTurn: invalid agentId (${params.agentId})`); + } + let resolvedScope: AgentDefinitionScope; + try { + const definition = await readAgentDefinition( + params.owner.runtime, + params.owner.workspacePath, + parsedAgentId.data, + { includeAgentPlugins: params.owner.includeAgentPlugins } + ); + resolvedScope = definition.scope; + } catch { + return Err(`Task.createWorkspaceTurn: unknown agentId (${params.agentId})`); + } + if (resolvedScope === "project") { + return Err( + `Task.createWorkspaceTurn: target checkout is not reachable yet (provisioning or stopped runtime), so project-local agentId (${params.agentId}) cannot be verified there` + ); + } + const validation = await this.validateWorkspaceTurnAgentId({ + cfg: params.cfg, + agentId: params.agentId, + workspaceId: params.workspaceId, + ...params.owner, + }); + return validation.success + ? Ok({ validatedContext: params.owner, ...validation.data }) + : validation; + } + async createWorkspaceTurn( args: WorkspaceTurnCreateArgs ): Promise> { @@ -3624,6 +3931,17 @@ export class TaskService { if (queueDispatchMode !== "tool-end" && queueDispatchMode !== "turn-end") { return Err("Task.createWorkspaceTurn: unsupported queueDispatchMode"); } + // Explicit agent override: validate syntax up front; eligibility (existence, + // selectability, enablement) is checked below against the workspace whose turn will run. + let requestedAgentId: string | undefined; + const rawAgentId = coerceNonEmptyString(args.agentId); + if (rawAgentId != null) { + const parsedAgentId = AgentIdSchema.safeParse(normalizeAgentId(rawAgentId, "")); + if (!parsedAgentId.success) { + return Err(`Task.createWorkspaceTurn: invalid agentId (${rawAgentId})`); + } + requestedAgentId = parsedAgentId.data; + } await using _lock = await this.mutex.acquire(); @@ -3670,8 +3988,25 @@ export class TaskService { const turnId = this.config.generateStableId(); const createdAt = getIsoNow(); // New workspace turns use exec. Follow-ups in a persistent agent-task workspace preserve that - // child's original agent identity and task-level model settings. + // child's original agent identity and task-level model settings. An explicit args.agentId + // (validated above/below) overrides either default for this turn only. let workspaceTurnAgentId = "exec"; + // Definition context of the checkout the explicit agent was validated against, so + // agent-authored frontmatter `ai` defaults participate in AI-settings resolution + // (mirrors the sub-agent creation path). Left unset for default exec/resume turns. + let agentDefinitionContext: NodeAgentDefinitionContext | undefined; + // Provenance (scope + exact source + base chain) of the definition that + // authorized the dispatch; pinned at stream time via strictAgentResolution so a + // vanished definition or base shadow cannot silently hand any hop to a + // different candidate. + let validatedAgentProvenance: + | { scope: AgentDefinitionScope; source?: string; chain: WorkspaceTurnAgentChainEntry[] } + | undefined; + // Post-create target validation failure. Deferred (not returned immediately) so the + // workspace-turn record is still written first: the record marks the created workspace + // as owner-owned and retryable via mode="existing", and the failure settles through the + // normal handle machinery instead of stranding an unowned workspace. + let agentValidationError: string | undefined; let targetWorkspaceId: string; let targetAiSettings: ResolvedWorkspaceAiSettings | undefined; let targetTaskModelString: string | undefined; @@ -3720,6 +4055,64 @@ export class TaskService { targetTaskThinkingLevel = targetEntry.workspace.taskThinkingLevel; targetTaskExperiments = targetEntry.workspace.taskExperiments; } + if (requestedAgentId != null) { + // Persistent agent-task children are pinned to their persisted identity: + // resolveAgentForStream ignores the per-send agentId whenever metadata has + // parentWorkspaceId, so an override could never actually run — reject instead of + // resolving settings for an agent that will not stream. + if (targetIsAgentWorkspace) { + return Err( + `Task.createWorkspaceTurn: explicit agentId is not supported for descendant agent workspaces (${targetWorkspaceId} keeps its persisted agent identity)` + ); + } + // Per-turn override for a normal owner-created workspace: wins for this turn but + // never mutates the target workspace's saved agent/settings (the dispatch below + // skips AI-settings persistence). Validate against the TARGET workspace's checkout + // (project-local agent definitions can diverge across branches). + const ownerContext = this.buildWorkspaceTurnAgentContext({ + runtimeConfig: parentMeta.runtimeConfig, + projectPath: parentMeta.projectPath, + workspaceName: parentMeta.name, + persistedWorkspacePath: parentEntry?.workspace.path, + subProjectPath: parentMeta.subProjectPath, + }); + const targetContext = + targetEntry != null + ? this.buildWorkspaceTurnAgentContext({ + runtimeConfig: targetEntry.workspace.runtimeConfig ?? parentMeta.runtimeConfig, + projectPath: targetEntry.projectPath, + // Entries created by workspaceService.create always carry a name; the fallback + // only satisfies the optional persisted-config type. + workspaceName: coerceNonEmptyString(targetEntry.workspace.name) ?? parentMeta.name, + persistedWorkspacePath: targetEntry.workspace.path, + subProjectPath: targetEntry.workspace.subProjectPath, + }) + : ownerContext; + const validation = await this.validateWorkspaceTurnAgentIdForTarget({ + cfg, + agentId: requestedAgentId, + workspaceId: existingWorkspaceId, + target: targetContext, + owner: ownerContext, + targetInitPending: + this.initStateManager.getInitState(existingWorkspaceId)?.status === "running", + // Existing targets have unknown checkout provenance (any branch, uncommitted + // shadows), so an unreachable checkout fails closed rather than trusting the + // owner's resolution. Omitting agentId still works (default identity). + ownerResolutionPredictsTarget: false, + }); + if (!validation.success) return Err(validation.error); + workspaceTurnAgentId = requestedAgentId; + validatedAgentProvenance = { + scope: validation.data.scope, + ...(validation.data.source != null ? { source: validation.data.source } : {}), + chain: validation.data.chain, + }; + agentDefinitionContext = { + ...validation.data.validatedContext, + workspaceId: targetWorkspaceId, + }; + } // Follow-up sends continue the target workspace's own last-used settings // (persisted on every send, or manually changed by the user in that // workspace) instead of re-inheriting the owner's live settings on each @@ -3738,6 +4131,96 @@ export class TaskService { if (!slot.success) return Err(slot.error); } } else { + let ownerContext: WorkspaceTurnAgentContext | undefined; + // Owner-side vouching for the created checkout's agent definitions requires proof + // that the target's base IS the branch actually checked out in the owner. The + // workspace name cannot prove it: branch→name sanitization is not injective + // (feature/foo and feature-foo both map to feature-foo) in BOTH directions — a + // request naming the owner's workspace name may be a distinct colliding branch, and + // the omitted-arg default (parentMeta.name, passed as trunkBranch to create below) + // may itself differ from a slash-branch owner's real branch. A different/unproven + // base means agents may exist only on the target branch (owner-side misses must not + // fail-fast) and the target branch may shadow ANY id (unreachable targets fail + // closed in validateWorkspaceTurnAgentIdForTarget). + const requestedTrunkBranch = coerceNonEmptyString(args.workspace?.trunkBranch); + let ownerVouchesForTargetBase = false; + if (requestedAgentId != null) { + ownerContext = this.buildWorkspaceTurnAgentContext({ + runtimeConfig: parentMeta.runtimeConfig, + projectPath: parentMeta.projectPath, + workspaceName: parentMeta.name, + persistedWorkspacePath: parentEntry?.workspace.path, + subProjectPath: parentMeta.subProjectPath, + }); + const effectiveTrunkBranch = requestedTrunkBranch ?? parentMeta.name; + const ownerBranch = await tryReadGitCurrentBranch( + ownerContext.runtime, + ownerContext.workspacePath + ); + const ownerBranchMatchesTargetBase = + ownerBranch != null && ownerBranch === effectiveTrunkBranch; + // Branch equality is not checkout equality: the child is created from COMMITTED + // branch state, so uncommitted agent-definition changes in the owner (a shadow + // added or removed) make owner-side resolution diverge from what the target will + // actually see. Vouching therefore also requires the agent-definition paths to + // be clean; unknown cleanliness (no git output) fails the vouch. + const ownerAgentDirsClean = ownerBranchMatchesTargetBase + ? await tryReadGitPathsClean( + ownerContext.runtime, + ownerContext.workspacePath, + AGENT_DEFINITION_PROJECT_PATHSPECS + ) + : undefined; + // Nor is the owner's HEAD necessarily the target's base COMMIT: worktree + // creation may branch from origin/ when the local branch can + // fast-forward, so a stale (or diverged) owner cannot vouch for definitions + // added or removed in the newer remote commit. + const ownerCommitMatchesOrigin = + ownerBranchMatchesTargetBase && ownerAgentDirsClean === true + ? await tryReadGitBranchMatchesOrigin( + ownerContext.runtime, + ownerContext.workspacePath, + effectiveTrunkBranch + ) + : undefined; + // An explicit branchName can attach the worktree to an EXISTING branch of that + // name (WorktreeManager detects and reuses it), making the trunk comparison + // above meaningless for the actual base — never vouch in that case. + const requestedBranchName = coerceNonEmptyString(args.workspace?.branchName); + ownerVouchesForTargetBase = + requestedBranchName == null && + ownerBranchMatchesTargetBase && + ownerAgentDirsClean === true && + ownerCommitMatchesOrigin === true; + // Pre-create stage: catch obviously bad ids (unknown/hidden/disabled) against the + // OWNER's checkout before creating any workspace. Fatal only when the owner's + // checked-out branch provably IS the target's base (an omitted trunkBranch still + // resolves to parentMeta.name, which may be a DIFFERENT branch than a slash-branch + // owner's — that distinct branch could carry target-only agents); otherwise the + // miss is advisory and the target checkout is authoritative post-create. + const validation = await this.validateWorkspaceTurnAgentId({ + cfg, + agentId: requestedAgentId, + workspaceId: ownerWorkspaceId, + ...ownerContext, + }); + if (!validation.success) { + // Owner-side misses are ALWAYS advisory: the created checkout is the only + // authoritative source of the target's agent definitions. No owner-side + // equivalence proof is sound here — worktree creation may fetch a newer + // origin commit, attach to an existing branchName, initialize submodules the + // owner never materialized, or run a committed init hook that installs the + // requested agent — so a pre-create rejection could deny a launch the real + // target would accept. Post-create validation (and, for anything it cannot + // see, the stream-time strict provenance pin) fails loudly instead. + log.debug( + "Task.createWorkspaceTurn: owner-side agent validation failed; deferring to the target checkout", + { agentId: requestedAgentId, error: validation.error } + ); + } else { + agentDefinitionContext = { ...ownerContext, workspaceId: ownerWorkspaceId }; + } + } const slot = await ensureParallelSlot(); if (!slot.success) return Err(slot.error); const tags = { @@ -3760,6 +4243,56 @@ export class TaskService { } targetWorkspaceId = createResult.data.metadata.id; createdWorkspace = true; + if (requestedAgentId != null && ownerContext != null) { + // Post-create stage: re-validate against the TARGET checkout — project-local agent + // definitions can diverge across branches/worktrees, so owner-path resolution is not + // an invariant. On failure, do NOT return before the workspace-turn record exists: + // the record marks the created workspace as owner-owned, so a mode="existing" retry + // (once the checkout is ready or with a valid agent) passes the ownership check + // instead of hitting invalid_scope. The failure settles through the normal handle + // machinery below. + const createdMeta = createResult.data.metadata; + const targetContext = this.buildWorkspaceTurnAgentContext({ + runtimeConfig: createdMeta.runtimeConfig, + projectPath: createdMeta.projectPath, + workspaceName: createdMeta.name, + persistedWorkspacePath: createdMeta.namedWorkspacePath, + subProjectPath: createdMeta.subProjectPath, + }); + const validation = await this.validateWorkspaceTurnAgentIdForTarget({ + cfg, + agentId: requestedAgentId, + workspaceId: targetWorkspaceId, + target: targetContext, + owner: ownerContext, + // create() starts runBackgroundInit asynchronously; a reachable checkout whose + // init hook is still running may not have its final agent definitions yet. + targetInitPending: + this.initStateManager.getInitState(targetWorkspaceId)?.status === "running", + ownerResolutionPredictsTarget: ownerVouchesForTargetBase, + }); + if (!validation.success) { + // Disposable workspaces are removed by the settlement's disposable cleanup. + // That cleanup is best-effort (failures are logged and swallowed), so the + // wording must not assert completed removal; if cleanup fails the workspace + // stays owner-owned and a mode="existing" retry still passes ownership. + agentValidationError = + args.workspace?.disposable === true + ? `${validation.error} — no turn was dispatched; automatic cleanup of the disposable workspace (${targetWorkspaceId}) was scheduled (if cleanup fails, it remains owned by this caller and retryable via workspace.mode="existing")` + : `${validation.error} — no turn was dispatched; the created workspace (${targetWorkspaceId}) is owned by this caller and can be retried via workspace.mode="existing" once ready`; + } else { + validatedAgentProvenance = { + scope: validation.data.scope, + ...(validation.data.source != null ? { source: validation.data.source } : {}), + chain: validation.data.chain, + }; + agentDefinitionContext = { + ...validation.data.validatedContext, + workspaceId: targetWorkspaceId, + }; + } + } + workspaceTurnAgentId = requestedAgentId ?? workspaceTurnAgentId; } // Unified per-field precedence (see resolveAgentAiSettings): explicit @@ -3799,6 +4332,16 @@ export class TaskService { } : undefined, fallbacks: this.buildParentAiSettingsFallbacks(parentMeta, workspaceTurnAgentId), + // Explicit agent overrides resolve the agent's own frontmatter `ai` defaults from the + // checkout they were validated against (mirrors resolveTaskAISettings' definitionContext). + // Known tradeoff: these launch AI defaults are a snapshot — an init hook that later + // rewrites the agent's `ai` frontmatter does not retroactively change the model/thinking + // already selected here (waiting for init is not an option under the service-wide mutex). + // This is bounded to convenience defaults: callers wanting determinism pass explicit + // model/thinking, the send path re-clamps thinking and re-gates reasoning per model at + // request time, and the authoritative prompt/tool policy is always resolved at stream + // time (after init) with strictAgentResolution guarding agent identity. + ...(agentDefinitionContext != null ? { definitionContext: agentDefinitionContext } : {}), }); // Selected (not effective) values: sendMessage persists what it // receives, and the send path re-clamps thinking and re-gates reasoning @@ -3828,7 +4371,13 @@ export class TaskService { prompt, modelString: model, ...(thinkingLevel != null ? { thinkingLevel } : {}), - ...(args.attentionPolicy != null ? { attentionPolicy: args.attentionPolicy } : {}), + // Synchronous validation failure below returns the error directly to the caller, so + // never persist notify_on_terminal for it: settleWorkspaceTurn derives the terminal + // wake from the PERSISTED record's attentionPolicy, which would enqueue a duplicate + // notification on top of the synchronous Err. + ...(args.attentionPolicy != null && agentValidationError == null + ? { attentionPolicy: args.attentionPolicy } + : {}), }; await this.taskHandleStore.upsertWorkspaceTurn(record); if (targetIsAgentWorkspace) { @@ -3841,6 +4390,26 @@ export class TaskService { }); } + if (agentValidationError != null) { + // Deferred post-create validation failure: the record above keeps the created + // workspace owner-owned (retryable via mode="existing"); settle the handle as a + // normal error instead of dispatching the turn. The record was persisted without + // attentionPolicy (see above), so settlement cannot enqueue a terminal wake on + // top of the synchronous Err returned below. + const next: WorkspaceTurnTaskHandleRecord = { + ...record, + status: "error", + updatedAt: getIsoNow(), + error: agentValidationError, + }; + await this.settleWorkspaceTurn({ + record, + next, + waiterSettlement: { status: "error", error: new Error(agentValidationError) }, + }); + return Err(agentValidationError); + } + const markWorkspaceTurnAccepted = async () => { await this.workspaceTurnSettlementLocks.withLock(handleId, async () => { const current = await this.taskHandleStore.getWorkspaceTurn(ownerWorkspaceId, handleId); @@ -3890,6 +4459,31 @@ export class TaskService { muxMetadata: this.buildWorkspaceTurnMuxMetadata(record), experiments: args.experiments ?? targetTaskExperiments, ...(mode === "existing" ? { queueDispatchMode } : {}), + // A per-turn agent override on an existing workspace must not overwrite the target's + // saved agent/settings (maybePersistAISettingsFromOptions persists them on every + // ordinary send). New workspaces still persist: the requested agent IS their default. + ...(mode === "existing" && requestedAgentId != null + ? { skipAiSettingsPersistence: true } + : {}), + // Explicit overrides were validated pre-dispatch, but that validation races init + // hooks and later edits; stream-time resolution runs after initialization and must + // fail loudly rather than silently swap in exec — and, when the validated scope is + // known, must not run a different-provenance definition for the same id (see + // strictAgentResolution docs). + ...(requestedAgentId != null + ? { + strictAgentResolution: + validatedAgentProvenance != null + ? { + expectedScope: validatedAgentProvenance.scope, + ...(validatedAgentProvenance.source != null + ? { expectedSource: validatedAgentProvenance.source } + : {}), + expectedChain: validatedAgentProvenance.chain, + } + : true, + } + : {}), }, { startStreamInBackground: true, @@ -4239,6 +4833,9 @@ export class TaskService { runtime, workspacePath: parentWorkspacePath, workspaceId: parentWorkspaceId, + includeAgentPlugins: this.workspaceService.isExperimentEnabled( + EXPERIMENT_IDS.AGENT_PLUGINS + ), }, })); } catch (error) { diff --git a/src/node/services/taskUtils.test.ts b/src/node/services/taskUtils.test.ts new file mode 100644 index 0000000000..d0d455c52f --- /dev/null +++ b/src/node/services/taskUtils.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, test } from "bun:test"; +import { execSync } from "node:child_process"; +import * as fsPromises from "node:fs/promises"; +import * as path from "node:path"; + +import { LocalRuntime } from "@/node/runtime/LocalRuntime"; +import { DisposableTempDir } from "@/node/services/tempDir"; +import { tryReadGitBranchMatchesOrigin } from "./taskUtils"; + +function initGitRepo(repoPath: string): void { + execSync("git init -b main", { cwd: repoPath, stdio: "ignore" }); + execSync('git config user.email "test@example.com"', { cwd: repoPath, stdio: "ignore" }); + execSync('git config user.name "test"', { cwd: repoPath, stdio: "ignore" }); + execSync("git config commit.gpgsign false", { cwd: repoPath, stdio: "ignore" }); + execSync("git commit -q --allow-empty -m init", { cwd: repoPath, stdio: "ignore" }); +} + +describe("tryReadGitBranchMatchesOrigin", () => { + test("shell-quotes repo-controlled branch names (no command injection)", async () => { + using tempDir = new DisposableTempDir("taskutils-branch-injection"); + const repoPath = path.join(tempDir.path, "repo"); + await fsPromises.mkdir(repoPath, { recursive: true }); + initGitRepo(repoPath); + + // A git-valid branch name crafted to break out of naive single-quoting: without + // shell quoting this would execute `touch injected-marker` in the owner runtime. + const maliciousBranch = "safe';touch injected-marker;#"; + const result = await tryReadGitBranchMatchesOrigin( + new LocalRuntime(repoPath), + repoPath, + maliciousBranch + ); + + // No origin ref exists for the branch, so the helper reports "local is the only + // candidate base" — and the injected command must never have run. + expect(result).toBe(true); + try { + await fsPromises.access(path.join(repoPath, "injected-marker")); + expect.unreachable("injected command created a marker file"); + } catch (error) { + expect(error).toBeDefined(); + } + }); + + test("distinguishes matching, diverged, and missing origin refs", async () => { + using tempDir = new DisposableTempDir("taskutils-branch-origin"); + const repoPath = path.join(tempDir.path, "repo"); + const bareRemote = path.join(tempDir.path, "origin.git"); + await fsPromises.mkdir(repoPath, { recursive: true }); + initGitRepo(repoPath); + execSync(`git init --bare -q '${bareRemote}'`, { cwd: tempDir.path, stdio: "ignore" }); + execSync(`git remote add origin '${bareRemote}'`, { cwd: repoPath, stdio: "ignore" }); + execSync("git push -q origin main", { cwd: repoPath, stdio: "ignore" }); + + const runtime = new LocalRuntime(repoPath); + expect(await tryReadGitBranchMatchesOrigin(runtime, repoPath, "main")).toBe(true); + + execSync("git commit -q --allow-empty -m ahead", { cwd: repoPath, stdio: "ignore" }); + expect(await tryReadGitBranchMatchesOrigin(runtime, repoPath, "main")).toBe(false); + + expect(await tryReadGitBranchMatchesOrigin(runtime, repoPath, "no-such-branch")).toBe(true); + }); +}); diff --git a/src/node/services/taskUtils.ts b/src/node/services/taskUtils.ts index 76b7339abb..e6d53f9ed9 100644 --- a/src/node/services/taskUtils.ts +++ b/src/node/services/taskUtils.ts @@ -6,6 +6,7 @@ import assert from "node:assert/strict"; import type { Config, Workspace as WorkspaceConfigEntry } from "@/node/config"; import type { Runtime } from "@/node/runtime/Runtime"; import { execBuffered } from "@/node/utils/runtime/helpers"; +import { shellQuote } from "@/common/utils/shell"; import { resolveModelFallbackChain } from "@/common/utils/ai/modelFallbacks"; export function coerceNonEmptyString(value: unknown): string | undefined { @@ -36,6 +37,110 @@ export async function tryReadGitHeadCommitSha( } } +/** + * Branch currently checked out in the workspace, or undefined when it cannot be + * determined (no git, detached HEAD, unreachable runtime). Callers use this as + * proof of a checkout's base branch, so "unknown" must stay distinguishable + * from any real branch name. + */ +export async function tryReadGitCurrentBranch( + runtime: Runtime, + workspacePath: string +): Promise { + assert(workspacePath.length > 0, "tryReadGitCurrentBranch: workspacePath must be non-empty"); + + try { + const result = await execBuffered(runtime, "git rev-parse --abbrev-ref HEAD", { + cwd: workspacePath, + timeout: 10, + }); + if (result.exitCode !== 0) { + return undefined; + } + + const branch = result.stdout.trim(); + // Detached HEAD reports the literal string "HEAD" — not a branch identity. + if (branch.length === 0 || branch === "HEAD") { + return undefined; + } + return branch; + } catch { + return undefined; + } +} + +/** + * True when the checkout's HEAD commit matches its origin ref for the given + * branch (or no origin ref exists, making local HEAD the only base candidate); + * false when they differ; undefined when this cannot be determined. Worktree + * creation may branch from origin/ when the local branch can + * fast-forward, so callers must not treat a stale local checkout as the + * authoritative base commit. + */ +export async function tryReadGitBranchMatchesOrigin( + runtime: Runtime, + workspacePath: string, + branch: string +): Promise { + assert( + workspacePath.length > 0, + "tryReadGitBranchMatchesOrigin: workspacePath must be non-empty" + ); + assert(branch.length > 0, "tryReadGitBranchMatchesOrigin: branch must be non-empty"); + + const headSha = await tryReadGitHeadCommitSha(runtime, workspacePath); + if (headSha == null) { + return undefined; + } + try { + // SECURITY: branch names are repo-controlled input (a git-valid branch can contain + // quotes); the full revision argument must be shell-quoted before interpolation. + const revision = shellQuote(`origin/${branch}^{commit}`); + const result = await execBuffered(runtime, `git rev-parse --verify --quiet ${revision}`, { + cwd: workspacePath, + timeout: 10, + }); + if (result.exitCode !== 0) { + // No origin ref for this branch: the local commit is the only candidate base. + return true; + } + return result.stdout.trim() === headSha; + } catch { + return undefined; + } +} + +/** + * True when the checkout has no uncommitted changes (including untracked AND + * gitignored files — an ignored local file still shadows committed state for + * discovery-style readers) under the given pathspecs; undefined when this cannot + * be determined (no git, unreachable runtime). Callers use this as proof that the + * committed base equals the working tree for those paths, so "unknown" must stay + * distinguishable from "clean". + */ +export async function tryReadGitPathsClean( + runtime: Runtime, + workspacePath: string, + pathspecs: readonly string[] +): Promise { + assert(workspacePath.length > 0, "tryReadGitPathsClean: workspacePath must be non-empty"); + assert(pathspecs.length > 0, "tryReadGitPathsClean: pathspecs must be non-empty"); + + try { + const quoted = pathspecs.map((pathspec) => shellQuote(pathspec)).join(" "); + const result = await execBuffered(runtime, `git status --porcelain --ignored -- ${quoted}`, { + cwd: workspacePath, + timeout: 10, + }); + if (result.exitCode !== 0) { + return undefined; + } + return result.stdout.trim().length === 0; + } catch { + return undefined; + } +} + /** * Resolve the effective refusal-fallback chain for a workspace's turn. * Task children can opt out via taskOnRefusal: "fail" (e.g. workflow verifier diff --git a/src/node/services/tools/task.test.ts b/src/node/services/tools/task.test.ts index 677b7b9269..f32ead99e1 100644 --- a/src/node/services/tools/task.test.ts +++ b/src/node/services/tools/task.test.ts @@ -191,6 +191,50 @@ describe("task tool", () => { }); }); + it("forwards agentId to createWorkspaceTurn for workspace kind", async () => { + using tempDir = new TestTempDir("test-task-tool-workspace-turn-agent-id"); + const baseConfig = createTestToolConfig(tempDir.path, { workspaceId: "parent-workspace" }); + + const createWorkspaceTurn = mock(() => + Ok({ + taskId: "wst_child-turn", + kind: "workspace_turn" as const, + status: "running" as const, + workspaceId: "child-workspace", + }) + ); + const taskService = { createWorkspaceTurn } as unknown as TaskService; + const tool = createTaskTool({ ...baseConfig, taskService }); + + const result: unknown = await Promise.resolve( + tool.execute!( + { + kind: "workspace", + agentId: "plan", + prompt: "plan a small change", + title: "Plan dogfood", + run_in_background: true, + }, + mockToolCallOptions + ) + ); + + expect(createWorkspaceTurn).toHaveBeenCalledTimes(1); + const createWorkspaceTurnCall = createWorkspaceTurn.mock.calls[0] as unknown[]; + expect(createWorkspaceTurnCall[0]).toMatchObject({ + ownerWorkspaceId: "parent-workspace", + agentId: "plan", + prompt: "plan a small change", + workspace: { mode: "new" }, + }); + expect(result).toMatchObject({ + status: "running", + taskId: "wst_child-turn", + workspaceId: "child-workspace", + handleKind: "workspace_turn", + }); + }); + it("forwards workspace turn queue dispatch mode", async () => { using tempDir = new TestTempDir("test-task-tool-workspace-turn-queue-mode"); const baseConfig = createTestToolConfig(tempDir.path, { workspaceId: "parent-workspace" }); diff --git a/src/node/services/tools/task.ts b/src/node/services/tools/task.ts index b72023ecc0..7219b8fc0b 100644 --- a/src/node/services/tools/task.ts +++ b/src/node/services/tools/task.ts @@ -421,6 +421,8 @@ export const createTaskTool: ToolFactory = (config: ToolConfiguration) => { ownerWorkspaceId: workspaceId, prompt, title, + // Agent mode for the launched turn (e.g. "plan"); createWorkspaceTurn defaults to exec. + ...(agentId != null ? { agentId } : {}), experiments: config.experiments, ...(aiOverrides.modelString != null ? { modelString: aiOverrides.modelString } : {}), ...(aiOverrides.thinkingLevel != null diff --git a/src/node/services/workspaceService.test.ts b/src/node/services/workspaceService.test.ts index daa1e1d2ca..50ab597238 100644 --- a/src/node/services/workspaceService.test.ts +++ b/src/node/services/workspaceService.test.ts @@ -49,7 +49,7 @@ import type { TerminalService } from "@/node/services/terminalService"; import type { DesktopSessionManager } from "@/node/services/desktop/DesktopSessionManager"; import type { WorktreeArchiveSnapshot } from "@/common/schemas/project"; import type { BashToolResult } from "@/common/types/tools"; -import type { WorkspaceChatMessage } from "@/common/orpc/types"; +import type { SendMessageOptions, WorkspaceChatMessage } from "@/common/orpc/types"; import { createMuxMessage } from "@/common/types/message"; import { buildStagedAttachmentNotice } from "@/browser/features/ChatInput/stagedAttachments"; import { @@ -14575,6 +14575,237 @@ describe("WorkspaceService.getGoalContinuationRuntimeState", () => { } }); + // -------------------------------------------------------------------------- + // getDelegatedTurnContinuationSendOptions — bash-monitor wake continuations + // -------------------------------------------------------------------------- + + describe("delegated-turn continuation send options", () => { + async function makeServiceWithHistory(): Promise<{ + service: WorkspaceService; + historyService: HistoryService; + }> { + const mockAIService = { + isStreaming: mock(() => false), + on: mock(() => undefined), + off: mock(() => undefined), + } as unknown as AIService; + const mockInitStateManager: Partial = { + on: mock(() => undefined as unknown as InitStateManager), + getInitState: mock(() => undefined), + }; + const mockConfig: Partial = { + srcDir: "/tmp/test", + getAllWorkspaceMetadata: mock(() => Promise.resolve([])), + getSessionDir: mock(() => "/tmp/test/sessions"), + generateStableId: mock(() => "test-id"), + }; + const { historyService } = await createTestHistoryService(); + const service = new WorkspaceService( + mockConfig as Config, + historyService, + mockAIService, + mockInitStateManager as InitStateManager, + {} as ExtensionMetadataService, + {} as BackgroundProcessManager + ); + return { service, historyService }; + } + + interface DelegatedContinuationInternals { + getDelegatedTurnContinuationSendOptions: ( + workspaceId: string + ) => Promise; + } + + const delegatedTurnCorrelation = { + type: "workspace-turn-task" as const, + taskHandleId: "wst_handle", + ownerWorkspaceId: "owner-ws", + turnId: "turn-1", + }; + + const delegatedTurnMessage = (id: string) => + createMuxMessage(id, "user", "Delegated prompt", { + timestamp: Date.now(), + muxMetadata: delegatedTurnCorrelation, + retrySendOptions: { + model: "anthropic:claude-opus-4-6", + agentId: "plan", + strictAgentResolution: true, + agentInitiated: true, + }, + }); + + /** Correlated assistant response; "tool-calls" is the queue-dispatch cut that leaves the turn open. */ + const delegatedAssistantMessage = (id: string, finishReason: "tool-calls" | "stop") => + createMuxMessage(id, "assistant", "Working…", { + timestamp: Date.now(), + partial: false, + finishReason, + muxMetadata: delegatedTurnCorrelation, + }); + + test("continues a still-open delegated turn under its own per-turn options", async () => { + const workspaceId = "ws-delegated-continuation"; + const { service, historyService } = await makeServiceWithHistory(); + await historyService.appendToHistory(workspaceId, delegatedTurnMessage("delegated-1")); + await historyService.appendToHistory( + workspaceId, + delegatedAssistantMessage("assistant-cut", "tool-calls") + ); + // A previous wake continuation must not hide the delegated turn's options. + await historyService.appendToHistory( + workspaceId, + createMuxMessage("wake-1", "user", "Monitor matched", { + timestamp: Date.now(), + muxMetadata: { type: "bash-monitor-wake" as const, records: [] }, + }) + ); + + const internals = service as unknown as DelegatedContinuationInternals; + const options = await internals.getDelegatedTurnContinuationSendOptions(workspaceId); + + expect(options).not.toBeNull(); + // Per-turn overrides (agent, strictness) continue the turn; they never become + // workspace defaults, and internal-only fields are not forwarded. + expect(options).toMatchObject({ + model: "anthropic:claude-opus-4-6", + agentId: "plan", + strictAgentResolution: true, + skipAiSettingsPersistence: true, + }); + expect(options && "agentInitiated" in options).toBe(false); + expect(options?.muxMetadata).toBeUndefined(); + }); + + test("recovers options from a wake row after on-send compaction hid the delegated row", async () => { + const workspaceId = "ws-delegated-post-compaction"; + const { service, historyService } = await makeServiceWithHistory(); + // On-send compaction consumed a wake continuation: the original delegated row is + // behind the boundary; the compaction summary proves the turn is still open and + // the follow-up wake-typed row is the remaining carrier of the turn's options. + await historyService.appendToHistory( + workspaceId, + createMuxMessage("summary-1", "assistant", "Summary", { + timestamp: Date.now(), + muxMetadata: { + type: "compaction-summary" as const, + pendingFollowUp: { + text: "Continue", + model: "anthropic:claude-opus-4-6", + agentId: "plan", + workspaceTurnMetadata: delegatedTurnCorrelation, + }, + }, + }) + ); + await historyService.appendToHistory( + workspaceId, + createMuxMessage("wake-followup", "user", "Monitor matched", { + timestamp: Date.now(), + muxMetadata: { type: "bash-monitor-wake" as const, records: [] }, + retrySendOptions: { + model: "anthropic:claude-opus-4-6", + agentId: "plan", + strictAgentResolution: { expectedScope: "built-in" }, + }, + }) + ); + + const internals = service as unknown as DelegatedContinuationInternals; + const options = await internals.getDelegatedTurnContinuationSendOptions(workspaceId); + expect(options).toMatchObject({ + model: "anthropic:claude-opus-4-6", + agentId: "plan", + strictAgentResolution: { expectedScope: "built-in" }, + skipAiSettingsPersistence: true, + }); + }); + + test("sanitizes persisted options through the canonical whitelist", async () => { + const workspaceId = "ws-delegated-sanitized"; + const { service, historyService } = await makeServiceWithHistory(); + const tamperedRetrySendOptions: Record = { + model: "anthropic:claude-opus-4-6", + agentId: "plan", + editMessageId: "innocent-message", + muxMetadata: { type: "workspace-turn-task" }, + }; + const malformedRetrySendOptions: Record = { agentId: "plan" }; // model missing + await historyService.appendToHistory( + workspaceId, + createMuxMessage("delegated-tampered", "user", "Delegated prompt", { + timestamp: Date.now(), + muxMetadata: delegatedTurnCorrelation, + // History is untrusted at rest: injected fields outside the whitelist + // (editMessageId would flip the send into the edit/truncation flow) must + // never reach the internal continuation send. + retrySendOptions: tamperedRetrySendOptions as never, + }) + ); + await historyService.appendToHistory( + workspaceId, + delegatedAssistantMessage("assistant-cut-3", "tool-calls") + ); + + const internals = service as unknown as DelegatedContinuationInternals; + const options = await internals.getDelegatedTurnContinuationSendOptions(workspaceId); + expect(options).toMatchObject({ agentId: "plan", skipAiSettingsPersistence: true }); + expect(options && "editMessageId" in options && options.editMessageId).toBeFalsy(); + expect(options?.muxMetadata).toBeUndefined(); + + // A row whose options fail schema validation entirely yields nothing. + const malformedWorkspaceId = "ws-delegated-malformed"; + await historyService.appendToHistory( + malformedWorkspaceId, + createMuxMessage("delegated-malformed", "user", "Delegated prompt", { + timestamp: Date.now(), + muxMetadata: delegatedTurnCorrelation, + retrySendOptions: malformedRetrySendOptions as never, + }) + ); + await historyService.appendToHistory( + malformedWorkspaceId, + delegatedAssistantMessage("assistant-cut-4", "tool-calls") + ); + expect( + await internals.getDelegatedTurnContinuationSendOptions(malformedWorkspaceId) + ).toBeNull(); + }); + + test("yields nothing after a terminal assistant response closed the delegated turn", async () => { + const workspaceId = "ws-delegated-closed"; + const { service, historyService } = await makeServiceWithHistory(); + await historyService.appendToHistory(workspaceId, delegatedTurnMessage("delegated-2")); + // finishReason "stop" ends the delegated turn: a later monitor match is a NEW + // synthetic turn and must resolve from persisted defaults, not stale overrides. + await historyService.appendToHistory( + workspaceId, + delegatedAssistantMessage("assistant-final", "stop") + ); + + const internals = service as unknown as DelegatedContinuationInternals; + expect(await internals.getDelegatedTurnContinuationSendOptions(workspaceId)).toBeNull(); + }); + + test("yields nothing once another user send follows the delegated prompt", async () => { + const workspaceId = "ws-delegated-superseded"; + const { service, historyService } = await makeServiceWithHistory(); + await historyService.appendToHistory(workspaceId, delegatedTurnMessage("delegated-3")); + await historyService.appendToHistory( + workspaceId, + delegatedAssistantMessage("assistant-cut-2", "tool-calls") + ); + await historyService.appendToHistory( + workspaceId, + createMuxMessage("manual-1", "user", "Manual user message", { timestamp: Date.now() }) + ); + + const internals = service as unknown as DelegatedContinuationInternals; + expect(await internals.getDelegatedTurnContinuationSendOptions(workspaceId)).toBeNull(); + }); + }); + // -------------------------------------------------------------------------- // getGoalContinuationKickoffSendOptions — model-resolution cascade // -------------------------------------------------------------------------- diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index c796bf0b7b..9511c7be72 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -31,6 +31,7 @@ import { AgentSession, clearProviderConfigFixableAbandonMarkers, CONTEXT_MUTATION_SEND_BLOCKED_MESSAGE, + inheritOpenWorkspaceTurnMetadata, type StreamErrorRecoveryOutcome, } from "@/node/services/agentSession"; import type { HistoryService } from "@/node/services/historyService"; @@ -257,6 +258,7 @@ import type { WorkspaceGoalDefaultsOverrideSchema, WorkspaceHeartbeatSettingsSchema, } from "@/common/orpc/schemas"; +import { SendMessageOptionsSchema } from "@/common/orpc/schemas"; import type { ArchiveLossyUntrackedFilesConfirmation, ArchivePreflightResult, @@ -1756,6 +1758,29 @@ function extractUserPromptText(message: MuxMessage): string { return stripStagedAttachmentNotice(partsText).trim(); } +/** + * Canonical whitelist for options replayed from a persisted delegated-turn row + * (see getDelegatedTurnContinuationSendOptions). History metadata stores + * retrySendOptions as an untyped blob, so a malformed or tampered row must be + * rejected (parse failure) or stripped to exactly these fields — never spread + * verbatim into an internal send where extras like editMessageId would trigger + * the edit/truncation flow. + */ +const DELEGATED_TURN_CONTINUATION_OPTIONS_SCHEMA = SendMessageOptionsSchema.pick({ + model: true, + agentId: true, + thinkingLevel: true, + reasoningMode: true, + toolPolicy: true, + additionalSystemInstructions: true, + maxOutputTokens: true, + providerOptions: true, + experiments: true, + disableWorkspaceAgents: true, + strictAgentResolution: true, + allowAgentSetGoal: true, +}); + // eslint-disable-next-line @typescript-eslint/no-unsafe-declaration-merging export class WorkspaceService extends EventEmitter { private readonly sessions = new Map(); @@ -2459,7 +2484,11 @@ export class WorkspaceService extends EventEmitter { return; } - const sendOptions = await this.getWorkflowContinuationSendOptions(ownerWorkspaceId); + // An in-flight delegated workspace turn continues under its own send options + // (per-turn agent/model overrides are not in the workspace's persisted defaults). + const sendOptions = + (await this.getDelegatedTurnContinuationSendOptions(ownerWorkspaceId)) ?? + (await this.getWorkflowContinuationSendOptions(ownerWorkspaceId)); if (sendOptions == null) { log.debug("Bash monitor wake has no send options; leaving pending", { ownerWorkspaceId }); return; @@ -12173,6 +12202,80 @@ export class WorkspaceService extends EventEmitter { return this.getGoalContinuationKickoffSendOptions(workspaceId); } + /** + * Send options for continuing a STILL-OPEN delegated workspace turn (bash-monitor + * wakes cut turns at tool boundaries). The delegated prompt's persisted + * retrySendOptions carry the turn's own settings — including per-turn overrides + * (agentId, model, strictAgentResolution) that are deliberately NOT in the + * workspace's persisted defaults when the launch used skipAiSettingsPersistence — + * so resolving from workspace defaults would continue the turn under the wrong + * agent. Openness is decided by the same rule as workspace-turn correlation + * (inheritOpenWorkspaceTurnMetadata): only a correlated assistant cut with + * finishReason "tool-calls" leaves the turn open. Once a terminal assistant + * response closed the turn (or any other user send took over the conversation), + * a late monitor match is a NEW synthetic turn and resolves from the target's + * persisted defaults instead of resurrecting stale per-turn overrides. + * + * Carrier rows for the open turn's options, newest first: the correlated + * workspace-turn user row itself, and this mechanism's own wake continuations + * (their sends were dispatched with the delegated options and re-stamped them) — + * after an on-send compaction consumed a wake, the follow-up wake-typed row is + * the only carrier left inside the boundary while the summary still proves the + * turn is open. Persisted options are rebuilt through a canonical schema + * whitelist (history is untrusted at rest; a tampered row must not inject fields + * like editMessageId into an internal send). Continuations never persist these + * options as workspace defaults. + */ + private async getDelegatedTurnContinuationSendOptions( + workspaceId: string + ): Promise { + // Tests construct WorkspaceService with partial HistoryService mocks (same + // defensive pattern as the iterateFullHistory caller above). + if (typeof this.historyService.getHistoryFromLatestBoundary !== "function") { + return null; + } + const history = await this.historyService.getHistoryFromLatestBoundary(workspaceId); + if (!history.success) { + return null; + } + const openTurn = inheritOpenWorkspaceTurnMetadata(history.data); + if (openTurn == null) { + return null; + } + for (let i = history.data.length - 1; i >= 0; i--) { + const message = history.data[i]; + if (message.role !== "user") { + continue; + } + const muxMetadata = message.metadata?.muxMetadata; + const isOpenTurnRow = + muxMetadata?.type === "workspace-turn-task" && + muxMetadata.taskHandleId === openTurn.taskHandleId && + muxMetadata.turnId === openTurn.turnId; + const isWakeContinuationRow = muxMetadata?.type === "bash-monitor-wake"; + if (!isOpenTurnRow && !isWakeContinuationRow) { + continue; + } + const parsed = DELEGATED_TURN_CONTINUATION_OPTIONS_SCHEMA.safeParse( + message.metadata?.retrySendOptions + ); + if (parsed.success) { + return { + ...parsed.data, + // Per-turn continuation settings must not become workspace defaults. + skipAiSettingsPersistence: true, + }; + } + if (isOpenTurnRow) { + // The anchor row itself has no usable options; nothing older can be more + // authoritative for this turn. + return null; + } + // A wake row without valid options: keep walking toward the anchor row. + } + return null; + } + /** * Defensive providers-config read: tests construct WorkspaceService with * partial AIService mocks, so a missing method degrades to null instead of