diff --git a/packages/server/src/server/agent/agent-prompt.test.ts b/packages/server/src/server/agent/agent-prompt.test.ts index 84632dda1..a88a8b9a3 100644 --- a/packages/server/src/server/agent/agent-prompt.test.ts +++ b/packages/server/src/server/agent/agent-prompt.test.ts @@ -11,6 +11,97 @@ import { } from "./agent-prompt.js"; import type { AgentManagerEvent, ManagedAgent } from "./agent-manager.js"; +interface FinishNotificationScenarioOptions { + childLastAssistantMessage?: string | null; +} + +interface FinishNotificationScenario { + startWatchingChild(): void; + finishChildAndReadParentPrompt(): Promise; +} + +function createFinishNotificationScenario( + options?: FinishNotificationScenarioOptions, +): FinishNotificationScenario { + let subscriber: ((event: AgentManagerEvent) => void) | null = null; + let resolveParentPrompt: ((prompt: string) => void) | null = null; + + const childAgent: ManagedAgent = Object.create(null); + Reflect.set(childAgent, "id", "child-agent"); + Reflect.set(childAgent, "lifecycle", "idle"); + Reflect.set(childAgent, "config", { title: "Child Agent" }); + + const callerAgent: ManagedAgent = Object.create(null); + Reflect.set(callerAgent, "id", "caller-agent"); + Reflect.set(callerAgent, "lifecycle", "idle"); + Reflect.set(callerAgent, "config", { title: "Caller Agent" }); + + const agentManager: AgentManager = Object.create(AgentManager.prototype); + Reflect.set(agentManager, "getAgent", (agentId: string) => { + if (agentId === "child-agent") { + return childAgent; + } + if (agentId === "caller-agent") { + return callerAgent; + } + return null; + }); + Reflect.set(agentManager, "subscribe", (callback: (event: AgentManagerEvent) => void) => { + subscriber = callback; + return () => { + subscriber = null; + }; + }); + Reflect.set(agentManager, "getLastAssistantMessage", async () => { + return options?.childLastAssistantMessage ?? null; + }); + Reflect.set(agentManager, "tryRunOutOfBand", () => false); + Reflect.set(agentManager, "hasInFlightRun", () => false); + Reflect.set(agentManager, "streamAgent", (_agentId: string, prompt: string) => { + resolveParentPrompt?.(prompt); + return (async function* noop() {})(); + }); + + const agentStorage: AgentStorage = Object.create(AgentStorage.prototype); + Reflect.set(agentStorage, "get", async (agentId: string) => { + if (agentId === "child-agent") { + return { title: "Child Agent" }; + } + return null; + }); + + return { + startWatchingChild() { + setupFinishNotification({ + agentManager, + agentStorage, + childAgentId: "child-agent", + callerAgentId: "caller-agent", + logger: createTestLogger(), + }); + }, + async finishChildAndReadParentPrompt() { + const parentPrompt = new Promise((resolve) => { + resolveParentPrompt = resolve; + }); + + childAgent.lifecycle = "running"; + subscriber?.({ + type: "agent_state", + agent: childAgent, + }); + + childAgent.lifecycle = "idle"; + subscriber?.({ + type: "agent_state", + agent: childAgent, + }); + + return parentPrompt; + }, + }; +} + test("isSystemInjectedEnvelope matches the envelope formatSystemNotificationPrompt produces", () => { expect(isSystemInjectedEnvelope(formatSystemNotificationPrompt("child finished"))).toBe(true); expect(isSystemInjectedEnvelope("hello world")).toBe(false); @@ -55,6 +146,21 @@ test("sendPromptToAgent forwards the client message id as run options", async () }); }); +test("finish notifications tell the parent the child's last assistant message", async () => { + const scenario = createFinishNotificationScenario({ + childLastAssistantMessage: "Implemented the cleanup and all checks pass.", + }); + + scenario.startWatchingChild(); + const parentPrompt = await scenario.finishChildAndReadParentPrompt(); + + expect(parentPrompt).toEqual( + formatSystemNotificationPrompt( + "Agent child-agent (Child Agent) finished.\n\n\nImplemented the cleanup and all checks pass.\n", + ), + ); +}); + it("does not notify archived callers", async () => { let subscriber: ((event: AgentManagerEvent) => void) | null = null; diff --git a/packages/server/src/server/agent/agent-prompt.ts b/packages/server/src/server/agent/agent-prompt.ts index 6331a9d7c..e7c7d9973 100644 --- a/packages/server/src/server/agent/agent-prompt.ts +++ b/packages/server/src/server/agent/agent-prompt.ts @@ -252,6 +252,22 @@ export interface SetupFinishNotificationParams { logger: Logger; } +interface FinishNotificationBodyInput { + childAgentId: string; + title: string; + reason: "finished" | "errored" | "needs permission"; + lastAssistantMessage: string | null; +} + +function formatFinishNotificationBody(params: FinishNotificationBodyInput): string { + const statusLine = `Agent ${params.childAgentId} (${params.title}) ${params.reason}.`; + const lastAssistantMessage = params.lastAssistantMessage?.trim(); + if (!lastAssistantMessage) { + return statusLine; + } + return `${statusLine}\n\n\n${lastAssistantMessage}\n`; +} + export function setupFinishNotification(params: SetupFinishNotificationParams): void { const { agentManager, agentStorage, childAgentId, callerAgentId, logger } = params; let hasSeenRunning = false; @@ -265,9 +281,20 @@ export function setupFinishNotification(params: SetupFinishNotificationParams): fired = true; unsubscribe?.(); + const callerRecord = await agentStorage.get(callerAgentId); + if (callerRecord?.archivedAt) { + return; + } + const record = await agentStorage.get(childAgentId); const title = record?.title ?? childAgentId; - const body = `Agent ${childAgentId} (${title}) ${reason}.`; + const lastAssistantMessage = await agentManager.getLastAssistantMessage(childAgentId); + const body = formatFinishNotificationBody({ + childAgentId, + title, + reason, + lastAssistantMessage, + }); await sendPromptToAgent({ agentManager, diff --git a/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts b/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts index 438743e59..b2da3360c 100644 --- a/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts @@ -1,70 +1,17 @@ -import { afterEach, describe, expect, test, vi } from "vitest"; +import { describe, expect, test, vi } from "vitest"; import type { SpawnedACPProcess, SessionStateResponse } from "./acp-agent.js"; -import { CursorACPAgentClient, parseCursorAgentModelsOutput } from "./cursor-acp-agent.js"; +import { CursorACPAgentClient } from "./cursor-acp-agent.js"; import { createTestLogger } from "../../../test-utils/test-logger.js"; -import * as spawnUtils from "../../../utils/spawn.js"; -afterEach(() => { - vi.restoreAllMocks(); -}); - -describe("parseCursorAgentModelsOutput", () => { - test("parses Cursor model list output and marks default model", () => { - expect( - parseCursorAgentModelsOutput(` -Available models - -auto - Auto -composer-2-fast - Composer 2 Fast (default) -gpt-5.5-low - GPT-5.5 1M Low (current) - -Tip: use --model (or /model in interactive mode) to switch. -`), - ).toEqual([ - { provider: "acp", id: "auto", label: "Auto", isDefault: false }, - { - provider: "acp", - id: "composer-2-fast", - label: "Composer 2 Fast", - isDefault: true, - }, - { - provider: "acp", - id: "gpt-5.5-low", - label: "GPT-5.5 1M Low", - isDefault: false, - }, - ]); - }); - - test("falls back to first model as default when Cursor marks none", () => { - expect( - parseCursorAgentModelsOutput(` -Available models -composer-2 - Composer 2 -gpt-5.5-low - GPT-5.5 1M Low -`), - ).toEqual([ - { provider: "acp", id: "composer-2", label: "Composer 2", isDefault: true }, - { provider: "acp", id: "gpt-5.5-low", label: "GPT-5.5 1M Low", isDefault: false }, - ]); - }); -}); - -describe("CursorACPAgentClient model fallback", () => { +describe("CursorACPAgentClient model discovery", () => { class TestCursorACPAgentClient extends CursorACPAgentClient { - constructor(options: { - command?: [string, ...string[]]; - env?: Record; - response: SessionStateResponse; - }) { + constructor(response: SessionStateResponse) { super({ logger: createTestLogger(), - command: options.command ?? ["cursor-agent", "acp"], - env: options.env, + command: ["cursor-agent", "acp"], }); - this.response = options.response; + this.response = response; } private readonly response: SessionStateResponse; @@ -82,112 +29,42 @@ describe("CursorACPAgentClient model fallback", () => { protected override async closeProbe(): Promise {} } - test("uses cursor-agent models when Cursor ACP reports zero models", async () => { - const execCommand = vi.spyOn(spawnUtils, "execCommand").mockResolvedValue({ - stdout: "Available models\ncomposer-2-fast - Composer 2 Fast (default)\n", - stderr: "", - }); + test("returns only ACP model ids because Cursor CLI ids cannot select ACP models", async () => { const client = new TestCursorACPAgentClient({ - response: { sessionId: "session-1", models: null, configOptions: [] }, - }); - - const models = await client.listModels({ cwd: "/tmp/cursor", force: false }); - - expect(models).toEqual([ - { - provider: "acp", - id: "composer-2-fast", - label: "Composer 2 Fast", - isDefault: true, + sessionId: "session-1", + models: { + currentModelId: "gpt-5.4[context=272k,reasoning=medium,fast=false]", + availableModels: [ + { + modelId: "gpt-5.4[context=272k,reasoning=medium,fast=false]", + name: "gpt-5.4", + description: null, + }, + ], }, - ]); - expect(execCommand).toHaveBeenCalledWith( - "cursor-agent", - ["models"], - expect.objectContaining({ timeout: expect.any(Number) }), - ); - }); - - test("uses cursor-agent models for absolute cursor-agent commands", async () => { - const execCommand = vi.spyOn(spawnUtils, "execCommand").mockResolvedValue({ - stdout: "Available models\ncomposer-2-fast - Composer 2 Fast (default)\n", - stderr: "", - }); - const client = new TestCursorACPAgentClient({ - command: ["/opt/cursor/bin/cursor-agent", "acp"], - response: { sessionId: "session-1", models: null, configOptions: [] }, + configOptions: [], }); await expect(client.listModels({ cwd: "/tmp/cursor", force: false })).resolves.toEqual([ { provider: "acp", - id: "composer-2-fast", - label: "Composer 2 Fast", - isDefault: true, - }, - ]); - expect(execCommand).toHaveBeenCalledWith( - "/opt/cursor/bin/cursor-agent", - ["models"], - expect.objectContaining({ timeout: expect.any(Number) }), - ); - }); - - test("passes Cursor provider env to cursor-agent models fallback", async () => { - const execCommand = vi.spyOn(spawnUtils, "execCommand").mockResolvedValue({ - stdout: "Available models\ncomposer-2-fast - Composer 2 Fast (default)\n", - stderr: "", - }); - const env = { CURSOR_AGENT_LOG: "debug" }; - const client = new TestCursorACPAgentClient({ - env, - response: { sessionId: "session-1", models: null, configOptions: [] }, - }); - - await client.listModels({ cwd: "/tmp/cursor", force: false }); - - expect(execCommand).toHaveBeenCalledWith( - "cursor-agent", - ["models"], - expect.objectContaining({ envOverlay: env }), - ); - }); - - test("does not run fallback when ACP returns models", async () => { - const execCommand = vi.spyOn(spawnUtils, "execCommand"); - const client = new TestCursorACPAgentClient({ - response: { - sessionId: "session-1", - models: { - currentModelId: "acp-model", - availableModels: [{ modelId: "acp-model", name: "ACP Model", description: null }], - }, - configOptions: [], - }, - }); - - await expect(client.listModels({ cwd: "/tmp/cursor", force: false })).resolves.toEqual([ - { - provider: "acp", - id: "acp-model", - label: "ACP Model", + id: "gpt-5.4[context=272k,reasoning=medium,fast=false]", + label: "gpt-5.4", description: undefined, isDefault: true, thinkingOptions: undefined, defaultThinkingOptionId: undefined, }, ]); - expect(execCommand).not.toHaveBeenCalled(); }); - test("does not run fallback when command is not cursor-agent", async () => { - const execCommand = vi.spyOn(spawnUtils, "execCommand"); + test("does not fall back to cursor-agent models when ACP reports zero models", async () => { const client = new TestCursorACPAgentClient({ - command: ["other-agent", "acp"], - response: { sessionId: "session-1", models: null, configOptions: [] }, + sessionId: "session-1", + models: null, + configOptions: [], }); await expect(client.listModels({ cwd: "/tmp/cursor", force: false })).resolves.toEqual([]); - expect(execCommand).not.toHaveBeenCalled(); }); }); diff --git a/packages/server/src/server/agent/providers/cursor-acp-agent.ts b/packages/server/src/server/agent/providers/cursor-acp-agent.ts index 47943a62d..b72ca5f5f 100644 --- a/packages/server/src/server/agent/providers/cursor-acp-agent.ts +++ b/packages/server/src/server/agent/providers/cursor-acp-agent.ts @@ -1,8 +1,5 @@ -import { basename } from "node:path"; import type { Logger } from "pino"; -import type { AgentModelDefinition, ListModelsOptions } from "../agent-sdk-types.js"; -import * as spawnUtils from "../../../utils/spawn.js"; import { GenericACPAgentClient } from "./generic-acp-agent.js"; interface CursorACPAgentClientOptions { @@ -14,14 +11,9 @@ interface CursorACPAgentClientOptions { providerParams?: unknown; } -const CURSOR_MODELS_TIMEOUT_MS = 10_000; const CURSOR_INITIAL_COMMANDS_WAIT_TIMEOUT_MS = 10_000; -const CURSOR_MODEL_MARKER_PATTERN = /\s+\((?:default|current)\)$/; export class CursorACPAgentClient extends GenericACPAgentClient { - private readonly cursorCommand: [string, ...string[]]; - private readonly env?: Record; - constructor(options: CursorACPAgentClientOptions) { super({ logger: options.logger, @@ -34,104 +26,5 @@ export class CursorACPAgentClient extends GenericACPAgentClient { waitForInitialCommands: true, initialCommandsWaitTimeoutMs: CURSOR_INITIAL_COMMANDS_WAIT_TIMEOUT_MS, }); - this.cursorCommand = options.command; - this.env = options.env; - } - - override async listModels(options: ListModelsOptions): Promise { - const acpModels = await super.listModels(options); - if (acpModels.length > 0) { - return acpModels; - } - - if (this.canUseCursorModelsFallback()) { - return this.listCursorModelsFallback(acpModels); - } - - return acpModels; - } - - private canUseCursorModelsFallback(): boolean { - return basename(this.cursorCommand[0]) === "cursor-agent"; - } - - private async listCursorModelsFallback( - acpModels: AgentModelDefinition[], - ): Promise { - try { - const { stdout } = await spawnUtils.execCommand(this.cursorCommand[0], ["models"], { - envOverlay: this.env, - timeout: CURSOR_MODELS_TIMEOUT_MS, - maxBuffer: 1024 * 1024, - }); - const fallbackModels = parseCursorAgentModelsOutput(stdout); - if (fallbackModels.length === 0) { - this.logger.warn( - { provider: "cursor" }, - "Cursor ACP model fallback returned no parseable models", - ); - return acpModels; - } - return fallbackModels; - } catch (error) { - this.logger.warn( - { err: error, provider: "cursor" }, - "Failed to list Cursor models via cursor-agent models fallback", - ); - return acpModels; - } } } - -interface ParsedCursorModel { - provider: "acp"; - id: string; - label: string; - marker: "default" | "current" | null; -} - -export function parseCursorAgentModelsOutput(output: string): AgentModelDefinition[] { - const parsed = output - .split(/\r?\n/) - .map((line) => line.trim()) - .filter((line) => line && line !== "Available models" && !line.startsWith("Tip:")) - .map((line) => { - const separatorIndex = line.indexOf(" - "); - if (separatorIndex <= 0) { - return null; - } - - const id = line.slice(0, separatorIndex).trim(); - const rawLabel = line.slice(separatorIndex + 3).trim(); - if (!id || !rawLabel) { - return null; - } - - let marker: "default" | "current" | null = null; - if (rawLabel.endsWith(" (default)")) { - marker = "default"; - } else if (rawLabel.endsWith(" (current)")) { - marker = "current"; - } - - return { - provider: "acp", - id, - label: rawLabel.replace(CURSOR_MODEL_MARKER_PATTERN, ""), - marker, - }; - }) - .filter((model): model is ParsedCursorModel => model !== null); - - const defaultModelId = - parsed.find((model) => model.marker === "default")?.id ?? - parsed.find((model) => model.marker === "current")?.id ?? - parsed[0]?.id; - - return parsed.map((model) => ({ - provider: model.provider, - id: model.id, - label: model.label, - isDefault: model.id === defaultModelId, - })); -}