Repository navigation
feat(subagents): add capabilities and cancellation #382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import { | |
| captureAgentProviderResult, | ||
| isProgrammerDefect, | ||
| } from "./local-agent-errors.js"; | ||
| import { bindLocalAgentAbort, localAgentCancelledError } from "./local-agent-cancellation.js"; | ||
| import { terminateProcessTree } from "./process-platform.js"; | ||
| import { DEVSPACE_VERSION } from "./version.js"; | ||
| import { | ||
|
|
@@ -49,6 +50,7 @@ const ACP_COMMANDS: Record<AcpProvider, [string, ...string[]]> = { | |
| interface AcpConnectionLike { | ||
| agent: { | ||
| request(method: string, params?: unknown): Promise<unknown>; | ||
| notify(method: string, params?: unknown): Promise<void>; | ||
| }; | ||
| close(error?: unknown): void; | ||
| closed: Promise<void>; | ||
|
|
@@ -142,6 +144,9 @@ export class AcpRuntime implements LocalAgentRuntime { | |
| throw new TypeError(`${this.provider} ACP session ${sessionId} already has an active turn.`); | ||
| } | ||
| this.activeSessions.add(sessionId); | ||
| const removeAbort = bindLocalAgentAbort(input.signal, () => ( | ||
| this.connection.agent.notify("session/cancel", { sessionId }) | ||
| )); | ||
| const queue = this.queues.get(sessionId) ?? { values: [] }; | ||
| this.queues.set(sessionId, queue); | ||
| const promptId = this.provider === "grok" ? this.nextPromptId() : undefined; | ||
|
|
@@ -166,9 +171,16 @@ export class AcpRuntime implements LocalAgentRuntime { | |
| prompt: [{ type: "text", text: input.prompt }], | ||
| ...(promptId ? { _meta: { promptId, requestId: promptId } } : {}), | ||
| }); | ||
| const response = completion | ||
| ? await Promise.race([standardResponse, completion]) | ||
| : await standardResponse; | ||
| let response: unknown; | ||
| try { | ||
| response = completion | ||
| ? await Promise.race([standardResponse, completion]) | ||
| : await standardResponse; | ||
| } catch (cause) { | ||
| if (input.signal?.aborted) throw localAgentCancelledError(this.provider, "run", cause); | ||
| throw cause; | ||
| } | ||
| if (input.signal?.aborted) throw localAgentCancelledError(this.provider, "run"); | ||
|
Comment on lines
+175
to
+183
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a Cursor or Copilot provider does not answer Knowledge Base Used: Agent runtime and provider adapters ArtifactsMock ACP stop reproduction script
Stop output with a responsive provider
Stop output with a silent provider
|
||
| if (completion && isGrokPromptCompletion(response)) { | ||
| await yieldToAcpQueue(); | ||
| } else if (promptId) { | ||
|
|
@@ -193,6 +205,7 @@ export class AcpRuntime implements LocalAgentRuntime { | |
| items: updates, | ||
| }; | ||
| } finally { | ||
| removeAbort(); | ||
| if (promptId) this.grokCompletionRegistry?.remove(sessionId, promptId); | ||
| this.activeSessions.delete(sessionId); | ||
| } | ||
|
|
@@ -416,6 +429,13 @@ export class AcpLocalAgentDriver implements LocalAgentDriver { | |
| authority: "write_mode", | ||
| idleTimeoutMs: 5 * 60_000, | ||
| } as const; | ||
| readonly capabilities = { | ||
| sessions: { resume: true, close: true }, | ||
| turns: { interrupt: true }, | ||
| configuration: { modelOverride: true, effortOverride: true }, | ||
| permissions: { enforcement: "native" }, | ||
| mcp: { supported: true }, | ||
| } as const; | ||
| private commandResolved = false; | ||
| private resolvedCommand?: string; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import { AgentProviderCancelledError } from "./local-agent-errors.js"; | ||
| import type { LocalAgentDriverKind } from "./local-agent-provider.js"; | ||
|
|
||
| export function localAgentCancelledError( | ||
| provider: LocalAgentDriverKind, | ||
| operation: string, | ||
| cause?: unknown, | ||
| ): AgentProviderCancelledError { | ||
| return new AgentProviderCancelledError({ | ||
| code: "PROVIDER_CANCELLED", | ||
| provider, | ||
| operation, | ||
| retryable: false, | ||
| ...(cause === undefined ? {} : { cause }), | ||
| message: `${provider} agent turn was stopped.`, | ||
| }); | ||
| } | ||
|
|
||
| export function bindLocalAgentAbort( | ||
| signal: AbortSignal | undefined, | ||
| interrupt: () => void | Promise<void>, | ||
| ): () => void { | ||
| if (!signal) return () => undefined; | ||
| const onAbort = () => { void Promise.resolve(interrupt()).catch(() => undefined); }; | ||
| if (signal.aborted) onAbort(); | ||
| else signal.addEventListener("abort", onAbort, { once: true }); | ||
| return () => signal.removeEventListener("abort", onAbort); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,6 +49,7 @@ import type { | |
| AgentListError, | ||
| AgentLookupError, | ||
| AgentStartError, | ||
| AgentStopError, | ||
| AgentWaitError, | ||
| LocalAgentWaitResult, | ||
| RunOverrides, | ||
|
|
@@ -160,6 +161,14 @@ export class LocalAgentClient { | |
| return decodeRequestResult(result, "agent.wait", decodeAgentWaitResults); | ||
| } | ||
|
|
||
| async stopAgent( | ||
| agentId: string, | ||
| scope: LocalAgentWorkspaceScope, | ||
| ): Promise<BetterResult<LocalAgentRecord, AgentStopError | AgentDaemonError>> { | ||
| const result = await this.request("agent.stop", { id: agentId, scope }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A stop taking over 30 seconds exceeds this request’s default client deadline even though the daemon continues stopping the agent. The CLI reports Knowledge Base Used: Local agent daemon protocol Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! ArtifactsControlled agent-stop CLI and daemon repro script
Changed lines and clean tracked-file check
Agent stop response before the timeout threshold
Agent stop response after the timeout threshold
|
||
| return decodeRequestResult(result, "agent.stop", decodeAgentRecord); | ||
| } | ||
|
|
||
| async status(): Promise<BetterResult<LocalAgentDaemonStatus, AgentDaemonError>> { | ||
| const result = await this.requestExisting("daemon.status", {}); | ||
| return decodeRequestResult(result, "daemon.status", decodeDaemonStatus); | ||
|
|
@@ -695,6 +704,11 @@ function isRequestError( | |
| case "agent.get": | ||
| case "agent.wait": | ||
| return category === "target" || category === "scope" || category === "store"; | ||
| case "agent.stop": | ||
| return category === "target" | ||
| || category === "scope" | ||
| || category === "conflict" | ||
| || category === "store"; | ||
| case "agent.list": | ||
| return category === "scope" || category === "store"; | ||
| case "hello": | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import { | |
| AgentProviderUnavailableError, | ||
| captureAgentProviderResult, | ||
| } from "./local-agent-errors.js"; | ||
| import { bindLocalAgentAbort, localAgentCancelledError } from "./local-agent-cancellation.js"; | ||
| import { removeDevspaceNodeModulesBinFromPath } from "./local-agent-path.js"; | ||
| import { terminateProcessTree } from "./process-platform.js"; | ||
| import { DEVSPACE_VERSION } from "./version.js"; | ||
|
|
@@ -147,7 +148,14 @@ export class CodexAppServerRuntime implements LocalAgentRuntime { | |
| } | ||
|
|
||
| await callbacks?.onSessionId?.(threadId); | ||
| const completed = await this.rpc.runTurn(threadId, turnParams(input, threadId)); | ||
| let completed: CodexTurnResult; | ||
| try { | ||
| completed = await this.rpc.runTurn(threadId, turnParams(input, threadId), input.signal); | ||
| } catch (cause) { | ||
| if (input.signal?.aborted) throw localAgentCancelledError(this.provider, "run", cause); | ||
| throw cause; | ||
| } | ||
| if (input.signal?.aborted) throw localAgentCancelledError(this.provider, "run"); | ||
| const parsed = parseCompletedTurn(completed.event.params, completed.items); | ||
| if (parsed.failure) { | ||
| throw new AgentProviderExecutionError({ | ||
|
|
@@ -232,6 +240,13 @@ export class CodexLocalAgentDriver implements LocalAgentDriver { | |
| readonly provider = "codex" as const; | ||
| readonly providerInstanceId = "codex"; | ||
| readonly runtimePolicy = { scope: "instance", idleTimeoutMs: 5 * 60_000 } as const; | ||
| readonly capabilities = { | ||
| sessions: { resume: true, close: false }, | ||
| turns: { interrupt: true }, | ||
| configuration: { modelOverride: true, effortOverride: true }, | ||
| permissions: { enforcement: "native" }, | ||
| mcp: { supported: true }, | ||
| } as const; | ||
|
|
||
| private commandResolved = false; | ||
| private resolvedCommand?: ResolvedCodexCommand; | ||
|
|
@@ -354,7 +369,7 @@ class CodexAppServerRpc { | |
| this.write({ method, ...(params === undefined ? {} : { params }) }); | ||
| } | ||
|
|
||
| async runTurn(threadId: string, params: unknown): Promise<CodexTurnResult> { | ||
| async runTurn(threadId: string, params: unknown, signal?: AbortSignal): Promise<CodexTurnResult> { | ||
| if (this.fatalError) throw this.fatalError; | ||
| if (this.turns.has(threadId)) throw new Error(`Codex thread ${threadId} already has an active turn.`); | ||
| let resolveTurn!: (result: CodexTurnResult) => void; | ||
|
|
@@ -370,12 +385,20 @@ class CodexAppServerRpc { | |
| reject: rejectTurn, | ||
| }; | ||
| this.turns.set(threadId, turn); | ||
| const removeAbort = bindLocalAgentAbort(signal, async () => { | ||
| if (!turn.turnId) return; | ||
| await this.request("turn/interrupt", { threadId, turnId: turn.turnId }); | ||
| }); | ||
|
Comment on lines
+388
to
+391
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '365,425p' src/local-agent-codex.ts
sed -n '140,165p' src/local-agent-codex.tsRepository: Waishnav/devspace Length of output: 3544 🏁 Script executed: printf '%s\\n' '--- local-agent-codex symbols ---'
ast-grep outline src/local-agent-codex.ts
printf '%s\\n' '--- abort helper binding and implementation ---'
rg -n -C 5 'bindLocalAgentAbort|function request|private request|turn/interrupt|turn/completed|turn/failed|dispose|cleanup|kill\\(' src/local-agent-codex.ts src
printf '%s\\n' '--- local-agent-codex relevant RPC sections ---'
sed -n '1,90p' src/local-agent-codex.ts
sed -n '280,465p' src/local-agent-codex.ts
printf '%s\\n' '--- callers and cancellation persistence references ---'
rg -n -C 4 'localAgentCancelledError|PROVIDER_CANCELLED|cancelled|canceled|status.*stopped|stopped' src --glob '*.ts' --glob '*.tsx' | head -n 240Repository: Waishnav/devspace Length of output: 28895 🏁 Script executed: printf '%s\n' '--- cancellation helper ---'
cat -n src/local-agent-cancellation.ts
printf '%s\n' '--- Codex runtime lifecycle ---'
sed -n '80,240p' src/local-agent-codex.ts | cat -n
printf '%s\n' '--- manager stop and persistence symbols ---'
rg -n -C 6 'async stop|stop\(|PROVIDER_CANCELLED|status: "stopped"|status = "stopped"|set.*stopped|mark.*stopped|runTurn\(' src/local-agent-manager.ts src/local-agent-runtime.ts src/local-agent-store.ts
printf '%s\n' '--- Codex changed-path consumers ---'
rg -n -C 5 'class CodexAppServerRpc|runTurn\(|captureAgentProviderResult|localAgentCancelledError' src/local-agent-codex.tsRepository: Waishnav/devspace Length of output: 16994 🏁 Script executed: printf '%s\n' '--- manager stop and runTurn ---'
sed -n '250,430p' src/local-agent-manager.ts | cat -n
printf '%s\n' '--- manager run completion and stop helpers ---'
sed -n '430,620p' src/local-agent-manager.ts | cat -n
printf '%s\n' '--- store turn lifecycle ---'
sed -n '1,190p' src/local-agent-store.ts | cat -nRepository: Waishnav/devspace Length of output: 22950 🏁 Script executed: printf '%s\n' '--- runtime pool implementation ---'
rg -n -C 8 'class .*Pool|class LocalAgentRuntimePool|async run\(|releaseSession|runtime\.close|close\(\).*runtime|pool\.run|pool\.release' src --glob '*.ts'
printf '%s\n' '--- Codex interruption tests and protocol references ---'
rg -n -C 8 'turn/interrupt|interrupt.*reject|interrupt.*error|CodexAppServerRuntime|CodexAppServerRpc' src --glob '*codex*' --glob '*.test.ts' --glob '*.ts'Repository: Waishnav/devspace Length of output: 41858 🏁 Script executed: pool_file=$(rg -l 'class LocalAgentRuntimePool' src --glob '*.ts' | head -n 1)
printf 'pool_file=%s\n' "$pool_file"
if [ -n "$pool_file" ]; then
rg -n -C 12 'class LocalAgentRuntimePool|async run\(|finally|runtime\.close|close\(|activeRuns|evictIdle' "$pool_file"
fiRepository: Waishnav/devspace Length of output: 13579 Do not persist a stopped turn after The post- Keep the turn running until completion is observed, or terminate the Codex runtime before persisting it as stopped. Retain the interrupt failure for diagnostics. The rejection from the abort callback is swallowed, but that callback path does not itself create the stopped record. 🤖 Prompt for AI Agents |
||
| try { | ||
| const response = await this.request("turn/start", params); | ||
| turn.turnId = readString(asRecord(response)?.turn, "id"); | ||
| if (signal?.aborted && turn.turnId) { | ||
| await this.request("turn/interrupt", { threadId, turnId: turn.turnId }); | ||
| } | ||
| if (turn.completed) return { event: turn.completed, items: turn.items }; | ||
| return await completion; | ||
| } finally { | ||
| removeAbort(); | ||
| if (this.turns.get(threadId) === turn) this.turns.delete(threadId); | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '105,215p' src/local-agent-acp.tsRepository: Waishnav/devspace
Length of output: 4785
🏁 Script executed:
Repository: Waishnav/devspace
Length of output: 21543
🏁 Script executed:
Repository: Waishnav/devspace
Length of output: 13551
🏁 Script executed:
Repository: Waishnav/devspace
Length of output: 15644
🏁 Script executed:
Repository: Waishnav/devspace
Length of output: 32340
🏁 Script executed:
Repository: Waishnav/devspace
Length of output: 20237
🌐 Web query:
Agent Client Protocol session/cancel semantics session/prompt cancellation official documentation💡 Result:
🏁 Script executed:
Repository: Waishnav/devspace
Length of output: 12151
🏁 Script executed:
Repository: Waishnav/devspace
Length of output: 8065
🏁 Script executed:
Repository: Waishnav/devspace
Length of output: 21855
Check for cancellation before starting
session/prompt.When
stop()aborts whileopenSession()is pending,bindLocalAgentAbortsendssession/cancelafter session creation. The code then startssession/prompteven though the cancellation targeted no ongoing prompt. ACP cancellation does not apply to this later prompt. Sincestop()waits for the turn completion, a long-running prompt can keepstop()waiting and continue provider work.Suggested fix
🧰 Tools
🪛 ast-grep (0.45.3)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import type { ChildProcessWithoutNullStreams } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents