From 19abf7d05f72956cdf5b8d4f3b956af76c0c5eeb Mon Sep 17 00:00:00 2001 From: Francisco Pizarro Date: Tue, 8 Sep 2026 12:55:47 -0300 Subject: [PATCH 1/5] fix(acp): settle sessions before agent removal Uninstalling a disabled or uninstalled ACP registry agent was a permanent dead-end: acp_sessions binding rows were never deleted by any production path, sessions bound to a disabled agent threw during transfer assessment (agent type lookup filtered to enabled+installed), and bulk delete/move routes performed raw row operations with no settlement - running generations were not cancelled and queued inputs leaked. - add sessionSettlement: discard queue-mode pending inputs (steer kept, cancel suppresses the drain), cancel active turns with a bounded wait for status to leave generating, purge ACP bindings best-effort - wire settlement into sessions.delete, sessions.deleteAgentSessions, sessions.moveAgentSessions, sessions.moveToAgent - add config.getAgentType route: state-agnostic type lookup so disabled or uninstalled registry agents stay assessable - add purgeAcpSessionData on the ACP execution port so the uninstall guard becomes accurate once conversations are gone - make daemon move handlers target-aware: Argos targets receive the target agent's default model instead of a hardcoded acp label that broke the next send - AcpSettings uninstall now offers move/delete of conversations via the shared AgentTransferDialog instead of failing the guard SDD: docs/issues/acp-agent-removal-settlement --- apps/daemon/src/dispatch/daemonDispatcher.ts | 54 +++++- .../daemon/src/host/acp-provider-execution.ts | 12 ++ apps/daemon/src/host/daemonAcpConfig.ts | 20 ++ apps/daemon/src/host/daemonConfigPresenter.ts | 17 +- apps/daemon/src/host/sessionSettlement.ts | 107 ++++++++++ apps/daemon/src/index.ts | 5 + apps/daemon/test/daemonAcpConfig.test.ts | 58 ++++++ apps/daemon/test/daemonSessionRoutes.test.ts | 183 ++++++++++++++++++ .../test/daemonSessionSettlement.test.ts | 174 +++++++++++++++++ .../main/presenter/configPresenter/index.ts | 17 +- .../acp-agent-removal-settlement/plan.md | 94 +++++++++ .../acp-agent-removal-settlement/spec.md | 92 +++++++++ .../acp-agent-removal-settlement/tasks.md | 29 +++ .../src/session/acpSessionPersistence.ts | 5 + .../src/dispatch/config/configRouteHandler.ts | 9 + .../backend-core/src/ports/hotPathPorts.ts | 7 + packages/shared-contracts/src/routes.ts | 2 + .../src/routes/config.routes.ts | 14 ++ .../ui/settings/components/AcpSettings.tsx | 126 +++++++++++- .../components/agent/AgentTransferDialog.tsx | 7 +- 20 files changed, 1017 insertions(+), 15 deletions(-) create mode 100644 apps/daemon/src/host/sessionSettlement.ts create mode 100644 apps/daemon/test/daemonSessionSettlement.test.ts create mode 100644 docs/issues/acp-agent-removal-settlement/plan.md create mode 100644 docs/issues/acp-agent-removal-settlement/spec.md create mode 100644 docs/issues/acp-agent-removal-settlement/tasks.md diff --git a/apps/daemon/src/dispatch/daemonDispatcher.ts b/apps/daemon/src/dispatch/daemonDispatcher.ts index 7a81de758..2551fafd5 100644 --- a/apps/daemon/src/dispatch/daemonDispatcher.ts +++ b/apps/daemon/src/dispatch/daemonDispatcher.ts @@ -30,6 +30,7 @@ import type { IConfigPresenter } from "@argos/shared/presenter"; import { resolveDaemonVersion } from "../version"; import type { DaemonTerminalRuntime } from "../terminal/daemonTerminalRuntime"; import { diagnoseDaemonSchema, repairDaemonSchema } from "../host/daemonSchemaDiagnostics"; +import { settleSessionForOwnershipChange, type SettleSessionHost } from "../host/sessionSettlement"; import { getPiToolDefinitions } from "../host/piToolCatalog"; import { aggregateUsageStats, resolveBuiltinModelPrice } from "../host/usageStatsAggregator"; import { resolveModelCost } from "../host/modelCost"; @@ -371,6 +372,7 @@ type DaemonProviderExecutionPort = Required< | "setAcpPreferredProcessMode" | "prepareAcpSession" | "clearAcpSession" + | "purgeAcpSessionData" | "getAcpSessionModes" | "setAcpSessionMode" | "resolveAgentPermission" @@ -939,6 +941,40 @@ export function createDaemonDispatcher( sessionRepository: DaemonSessionRepositoryPort; providerExecutionPort: DaemonProviderExecutionPort; } = { sessionRepository, providerExecutionPort }; + const settlementHost: SettleSessionHost = { + getSession: async (sessionId) => (await (sessionRepository as any).get?.(sessionId)) ?? null, + listPendingInputs: async (sessionId) => (await (sessionRepository as any).listPendingInputs?.(sessionId)) ?? [], + deletePendingInput: async (sessionId, itemId) => { + await (sessionRepository as any).deletePendingInput?.(sessionId, itemId); + }, + cancelGeneration: (sessionId) => providerExecutionPort.cancelGeneration(sessionId), + purgeAcpSessionData: (sessionId) => providerExecutionPort.purgeAcpSessionData?.(sessionId), + }; + /** + * Resolve the execution context a session receives when moved to `toAgentId`. + * ACP targets keep the historical `providerId: "acp"` + `modelId: ` + * convention; Argos targets must receive the target agent's default model — + * labelling them `acp` would break the next send ("ACP agent not found"). + */ + const resolveMoveTargetContext = async ( + toAgentId: string, + ): Promise<{ agentId: string; providerId: string; modelId: string }> => { + const agentType = await daemonConfig.getAgentType(toAgentId); + if (agentType === "acp") { + return { agentId: toAgentId, providerId: "acp", modelId: toAgentId }; + } + if (agentType !== "argos") { + throw new Error(`Target agent not found: ${toAgentId}`); + } + const config = await daemonConfig.resolveArgosAgentConfig(toAgentId); + const defaultModel = daemonConfig.getDefaultModel(); + const providerId = config?.defaultModelPreset?.providerId?.trim() || defaultModel?.providerId?.trim() || ""; + const modelId = config?.defaultModelPreset?.modelId?.trim() || defaultModel?.modelId?.trim() || ""; + if (!providerId || !modelId) { + throw new Error(`Target Argos agent does not have a default model: ${toAgentId}`); + } + return { agentId: toAgentId, providerId, modelId }; + }; const daemonConfig = configPresenter as IConfigPresenter & DaemonMcpConfigPort & DaemonProviderConfigPort; const daemonSettings = configPresenter as IConfigPresenter & DaemonScheduledTaskConfigPort; @@ -3082,6 +3118,9 @@ export function createDaemonDispatcher( const deletedSessionIds: string[] = []; for (const session of sessions) { + // Settle before the ownership change: discard queued inputs, cancel a + // running turn and wait for it to settle, release ACP bindings. + await settleSessionForOwnershipChange(session.id, settlementHost); const messages = await repo.listMessages(session.id); const children = await repo.list({ includeSubagents: true, parentSessionId: session.id }); const isEmptyDraft = Boolean(session.isDraft) && messages.length === 0 && children.length === 0; @@ -3090,10 +3129,9 @@ export function createDaemonDispatcher( deletedSessionIds.push(session.id); continue; } + const targetContext = await resolveMoveTargetContext(input.toAgentId); await repo.moveSessionToAgent(session.id, { - agentId: input.toAgentId, - providerId: "acp", - modelId: input.toAgentId, + ...targetContext, projectDir: session.projectDir ?? null, permissionMode: session.permissionMode ?? "default", subagentEnabled: Boolean(session.subagentEnabled), @@ -3115,6 +3153,9 @@ export function createDaemonDispatcher( const sessions = await repo.list({ agentId: input.agentId, includeSubagents: true }); const deletedSessionIds: string[] = []; for (const session of sessions) { + // Settle first: cancel a running turn (bounded wait) and discard + // queued inputs so deletion cannot race the runtime. + await settleSessionForOwnershipChange(session.id, settlementHost); await repo.delete(session.id); deletedSessionIds.push(session.id); } @@ -3128,10 +3169,10 @@ export function createDaemonDispatcher( if (!session) { throw new Error(`Session not found: ${input.sessionId}`); } + await settleSessionForOwnershipChange(input.sessionId, settlementHost); + const targetContext = await resolveMoveTargetContext(input.toAgentId); const updated = await repo.moveSessionToAgent(input.sessionId, { - agentId: input.toAgentId, - providerId: "acp", - modelId: input.toAgentId, + ...targetContext, projectDir: session.projectDir ?? null, permissionMode: session.permissionMode ?? "default", subagentEnabled: Boolean(session.subagentEnabled), @@ -3143,6 +3184,7 @@ export function createDaemonDispatcher( if (route === sessionsDeleteRoute.name) { const input = sessionsDeleteRoute.input.parse(rawInput); + await settleSessionForOwnershipChange(input.sessionId, settlementHost); await (runtime as any).sessionRepository.delete(input.sessionId); return sessionsDeleteRoute.output.parse({ deleted: true }); } diff --git a/apps/daemon/src/host/acp-provider-execution.ts b/apps/daemon/src/host/acp-provider-execution.ts index bbfbc9f90..e821747db 100644 --- a/apps/daemon/src/host/acp-provider-execution.ts +++ b/apps/daemon/src/host/acp-provider-execution.ts @@ -881,6 +881,18 @@ export class AcpProviderExecutionPort implements ProviderExecutionPort { } } + async purgeAcpSessionData(sessionId: string): Promise { + // Best-effort: stop any active turn first so the binding cannot be + // re-created mid-purge, then delete the durable `acp_sessions` rows. + await this.cancelGeneration(sessionId).catch(() => undefined); + try { + const runtime = await this.getRuntime(); + await runtime.sessionPersistence.deleteAllSessions(sessionId); + } catch (error) { + console.warn(`[ACP] Failed to purge session data for ${sessionId}:`, error); + } + } + async respondToolInteraction( sessionId: string, _messageId: string, diff --git a/apps/daemon/src/host/daemonAcpConfig.ts b/apps/daemon/src/host/daemonAcpConfig.ts index b92314ced..6a724202a 100644 --- a/apps/daemon/src/host/daemonAcpConfig.ts +++ b/apps/daemon/src/host/daemonAcpConfig.ts @@ -57,6 +57,26 @@ export class DaemonAcpConfig { return this.acpConfHelper.getGlobalEnabled(); } + /** + * Resolve the ACP agent type regardless of enabled/install state. Unlike + * `getAcpAgents()` (which only surfaces enabled+installed agents), this lets + * callers identify sessions bound to a disabled or uninstalled agent so they + * can be moved or deleted before agent removal. + */ + getAcpAgentTypeIncludingState(agentId: string): "acp" | null { + const resolvedId = resolveAcpAgentAlias(agentId); + const manual = this.acpConfHelper.getManualAgents().some((agent) => agent.id === resolvedId); + if (manual) return "acp"; + try { + const registered = this.acpRegistryService.listAgents().some((agent) => agent.id === resolvedId); + return registered ? "acp" : null; + } catch (error) { + // A missing/unreadable registry snapshot must not break type resolution. + logger.warn("[ACP] registry agent lookup failed:", error); + return null; + } + } + async setAcpEnabled(enabled: boolean): Promise { this.acpConfHelper.setGlobalEnabled(enabled); } diff --git a/apps/daemon/src/host/daemonConfigPresenter.ts b/apps/daemon/src/host/daemonConfigPresenter.ts index a6f1ed02e..c882acf80 100644 --- a/apps/daemon/src/host/daemonConfigPresenter.ts +++ b/apps/daemon/src/host/daemonConfigPresenter.ts @@ -2,7 +2,8 @@ import { readFileSync, renameSync, writeFileSync, existsSync, mkdirSync } from " import { homedir } from "node:os"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; -import { DEFAULT_PROVIDERS, normalizeScheduledTasksConfig } from "@argos/backend-core"; +import { DEFAULT_PROVIDERS, normalizeScheduledTasksConfig, resolveAcpAgentAlias } from "@argos/backend-core"; +import { BUILTIN_ARGOS_AGENT_ID } from "@argos/agent-runtime"; import { ProviderDbLoader, resolveAiSdkProviderDefinition } from "@argos/backend-core/provider"; import type { BuiltinKnowledgeConfig, @@ -1264,6 +1265,20 @@ export class DaemonConfigPresenter { return this.argosAgentRuntime ? this.argosAgentRuntime.getAgent(agentId) : null; } + /** + * State-agnostic agent-type resolution. Registry/manual ACP agents resolve + * even when disabled or uninstalled so sessions bound to them remain + * movable/deletable ahead of agent removal (see + * docs/issues/acp-agent-removal-settlement). + */ + async getAgentType(agentId: string): Promise<"argos" | "acp" | null> { + const resolvedId = resolveAcpAgentAlias(String(agentId ?? "").trim()); + if (!resolvedId) return null; + if (resolvedId === BUILTIN_ARGOS_AGENT_ID) return "argos"; + if (this.argosAgentRuntime?.getAgent(resolvedId)) return "argos"; + return this.acpConfig.getAcpAgentTypeIncludingState(resolvedId); + } + async getArgosAgentConfig(agentId: string): Promise { return this.argosAgentRuntime ? this.argosAgentRuntime.getArgosAgentConfig(agentId) : null; } diff --git a/apps/daemon/src/host/sessionSettlement.ts b/apps/daemon/src/host/sessionSettlement.ts new file mode 100644 index 000000000..6ec55c2a5 --- /dev/null +++ b/apps/daemon/src/host/sessionSettlement.ts @@ -0,0 +1,107 @@ +import type { PendingSessionInputRecord } from "@argos/shared/types/agent-interface"; + +/** + * Settlement for session ownership changes (delete / move / agent removal). + * + * Mirrors the upstream DeepChat fix for "allow uninstall while disabled" + * (ThinkInAIXYZ/deepchat#2188), re-implemented natively for Argos' daemon-owned + * session architecture: + * + * 1. Discard queue-mode pending inputs — they belong to no turn yet and would + * otherwise leak onto the next owner. Steer-mode inputs are kept on purpose: + * they are conversation facts, and a cancelled run cannot claim them because + * `cancelGeneration` suppresses the pending-input drain. + * 2. Cancel an active generation and wait (bounded) for the session status to + * leave `generating` — cancellation settles asynchronously, so proceeding + * immediately would race the runtime. + * 3. Purge durable ACP bindings (`acp_sessions` rows) best-effort so the ACP + * uninstall guard becomes accurate after the sessions are gone. + */ + +export interface SettleSessionHost { + getSession(sessionId: string): Promise<{ status?: string | null } | null>; + listPendingInputs(sessionId: string): Promise; + deletePendingInput(sessionId: string, itemId: string): Promise; + cancelGeneration(sessionId: string): Promise; + purgeAcpSessionData?(sessionId: string): Promise; +} + +export interface SettleSessionOptions { + /** Max time to wait for a cancelled generation to settle. Default 10s. */ + timeoutMs?: number; + /** Poll interval while waiting for settlement. Default 100ms. */ + pollIntervalMs?: number; + /** Injectable delay for tests. */ + delay?: (ms: number) => Promise; +} + +export interface SettleSessionResult { + cancelled: boolean; + discardedQueueInputIds: string[]; +} + +const DEFAULT_TIMEOUT_MS = 10_000; +const DEFAULT_POLL_INTERVAL_MS = 100; + +export async function settleSessionForOwnershipChange( + sessionId: string, + host: SettleSessionHost, + options: SettleSessionOptions = {}, +): Promise { + const delay = options.delay ?? ((ms: number) => new Promise((resolve) => setTimeout(resolve, ms))); + const discardedQueueInputIds = await discardQueueInputs(sessionId, host); + + let cancelled = false; + if ((await currentStatus(sessionId, host)) === "generating") { + cancelled = true; + await host.cancelGeneration(sessionId); + await waitForSettle(sessionId, host, delay, options); + } + + try { + await host.purgeAcpSessionData?.(sessionId); + } catch { + // best-effort: purge failures must not block the ownership change + } + + return { cancelled, discardedQueueInputIds }; +} + +async function currentStatus(sessionId: string, host: SettleSessionHost): Promise { + const session = await host.getSession(sessionId).catch(() => null); + return session?.status ?? null; +} + +async function discardQueueInputs(sessionId: string, host: SettleSessionHost): Promise { + const inputs = await host.listPendingInputs(sessionId).catch(() => [] as PendingSessionInputRecord[]); + const discarded: string[] = []; + for (const input of inputs) { + if (input.mode !== "queue") continue; + try { + await host.deletePendingInput(sessionId, input.id); + discarded.push(input.id); + } catch { + // A queued input that cannot be discarded must not block removal + // outright, but it also must not be silently lost: leave it in place. + } + } + return discarded; +} + +async function waitForSettle( + sessionId: string, + host: SettleSessionHost, + delay: (ms: number) => Promise, + options: SettleSessionOptions, +): Promise { + const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + const pollIntervalMs = options.pollIntervalMs ?? DEFAULT_POLL_INTERVAL_MS; + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + await delay(pollIntervalMs); + if ((await currentStatus(sessionId, host)) !== "generating") { + return; + } + } + throw new Error(`Session ${sessionId} did not stop before ownership change.`); +} diff --git a/apps/daemon/src/index.ts b/apps/daemon/src/index.ts index ba8b45f08..9e7a7d4e0 100644 --- a/apps/daemon/src/index.ts +++ b/apps/daemon/src/index.ts @@ -69,6 +69,7 @@ type DaemonProviderExecutionPort = Required< | "setAcpPreferredProcessMode" | "prepareAcpSession" | "clearAcpSession" + | "purgeAcpSessionData" | "getAcpSessionModes" | "setAcpSessionMode" | "resolveAgentPermission" @@ -457,6 +458,10 @@ export async function startDaemon(options?: { async clearAcpSession(sessionId) { return acpProviderExecutionPort.clearAcpSession(sessionId); }, + async purgeAcpSessionData(sessionId) { + // ACP-only: pi sessions carry no durable binding rows. + return acpProviderExecutionPort.purgeAcpSessionData(sessionId); + }, async getAcpSessionModes(conversationId) { return acpProviderExecutionPort.getAcpSessionModes(conversationId); }, diff --git a/apps/daemon/test/daemonAcpConfig.test.ts b/apps/daemon/test/daemonAcpConfig.test.ts index 11f294604..d51abf816 100644 --- a/apps/daemon/test/daemonAcpConfig.test.ts +++ b/apps/daemon/test/daemonAcpConfig.test.ts @@ -261,4 +261,62 @@ describe("DaemonAcpConfig reconcileInstalledAgents", () => { await expect(config.uninstallAcpRegistryAgent(binaryOkAgent.id)).rejects.toThrow("still has related conversations"); }); + + describe("getAcpAgentTypeIncludingState", () => { + it("resolves disabled and uninstalled registry agents", async () => { + const harness = createConfig({ + // disabled-agent is registered but disabled; locked-agent has no + // install state at all (never installed). + registryStates: { [disabledAgent.id]: { enabled: false } }, + }); + + await harness.config.initialReconcile; + + expect(harness.config.getAcpAgentTypeIncludingState(disabledAgent.id)).toBe("acp"); + expect(harness.config.getAcpAgentTypeIncludingState(binaryFailAgent.id)).toBe("acp"); + expect(harness.config.getAcpAgentTypeIncludingState("unknown-agent")).toBeNull(); + }); + + it("resolves disabled manual agents and tolerates alias lookups", async () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), "argos-acp-cfg-")); + const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "argos-acp-data-")); + roots.push(configDir, dataDir); + + fs.mkdirSync(path.join(dataDir, "acp-registry"), { recursive: true }); + fs.writeFileSync( + path.join(dataDir, "acp-registry", "meta.json"), + JSON.stringify({ version: "1.0.0", lastUpdated: Date.now(), lastAttemptedAt: Date.now(), sourceUrl: "" }), + ); + fs.writeFileSync( + path.join(dataDir, "acp-registry", "registry.json"), + JSON.stringify({ version: "1.0.0", agents: [] }), + ); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, "acp_agents.json"), + JSON.stringify({ + enabled: true, + version: "4", + registryStates: {}, + manualAgents: [ + { + id: "manual-1", + name: "Manual Agent", + source: "manual", + enabled: false, + command: "manual-agent", + args: [], + }, + ], + installStates: {}, + sharedMcpSelections: [], + }), + ); + + const config = new DaemonAcpConfig({ configDir, dataDir }); + + expect(config.getAcpAgentTypeIncludingState("manual-1")).toBe("acp"); + expect(config.getAcpAgentTypeIncludingState("unknown")).toBeNull(); + }); + }); }); diff --git a/apps/daemon/test/daemonSessionRoutes.test.ts b/apps/daemon/test/daemonSessionRoutes.test.ts index 1e284358d..dff4eb5c2 100644 --- a/apps/daemon/test/daemonSessionRoutes.test.ts +++ b/apps/daemon/test/daemonSessionRoutes.test.ts @@ -1206,6 +1206,7 @@ describe("daemon session migration routes", () => { { getDefaultModel: vi.fn(() => ({ providerId: "provider-1", modelId: "model-1" })), getDefaultProjectPath: vi.fn(() => "/tmp/project"), + getAgentType: vi.fn(async (agentId: string) => (agentId === "acp-agent-2" ? "acp" : null)), } as any, undefined, sessionRepository as any, @@ -1238,6 +1239,188 @@ describe("daemon session migration routes", () => { ); }); + it("settles sessions before deleting agent sessions", async () => { + const session = { + id: "session-1", + agentId: "acp-agent-1", + title: "Bound to a disabled agent", + projectDir: "/tmp/project", + isPinned: false, + isDraft: false, + sessionKind: "regular", + parentSessionId: null, + subagentEnabled: false, + createdAt: 1, + updatedAt: 1, + status: "idle", + providerId: "acp", + modelId: "acp-agent-1", + }; + + const sessionRepository = { + get: vi.fn(async () => ({ ...session, status: "idle" })), + list: vi.fn(async () => [session]), + listPendingInputs: vi.fn(async () => [ + { id: "queue-1", sessionId: "session-1", mode: "queue", state: "pending" }, + { id: "steer-1", sessionId: "session-1", mode: "steer", state: "pending" }, + ]), + deletePendingInput: vi.fn(async () => undefined), + delete: vi.fn(async () => undefined), + }; + + const providerExecutionPort = { + cancelGeneration: vi.fn(async () => undefined), + purgeAcpSessionData: vi.fn(async () => undefined), + }; + + const dispatcher = createDaemonDispatcher( + { + getDefaultModel: vi.fn(() => ({ providerId: "provider-1", modelId: "model-1" })), + getDefaultProjectPath: vi.fn(() => "/tmp/project"), + } as any, + undefined, + sessionRepository as any, + providerExecutionPort as any, + ); + + await expect(dispatcher("sessions.deleteAgentSessions", { agentId: "acp-agent-1" })).resolves.toEqual({ + deletedSessionIds: ["session-1"], + }); + + // Queue input discarded, steer input kept, bindings purged, then delete. + expect(sessionRepository.deletePendingInput).toHaveBeenCalledWith("session-1", "queue-1"); + expect(sessionRepository.deletePendingInput).not.toHaveBeenCalledWith("session-1", "steer-1"); + expect(sessionRepository.delete).toHaveBeenCalledWith("session-1"); + expect(providerExecutionPort.cancelGeneration).not.toHaveBeenCalled(); + expect(providerExecutionPort.purgeAcpSessionData).toHaveBeenCalledWith("session-1"); + }); + + it("cancels and waits for a generating session before deleting agent sessions", async () => { + let status: string | null = "generating"; + const session = { + id: "session-2", + agentId: "acp-agent-1", + title: "Still generating", + projectDir: "/tmp/project", + isPinned: false, + isDraft: false, + sessionKind: "regular", + parentSessionId: null, + subagentEnabled: false, + createdAt: 1, + updatedAt: 1, + status, + providerId: "acp", + modelId: "acp-agent-1", + }; + + const sessionRepository = { + get: vi.fn(async () => ({ ...session, status })), + list: vi.fn(async () => [session]), + listPendingInputs: vi.fn(async () => []), + deletePendingInput: vi.fn(async () => undefined), + delete: vi.fn(async () => { + status = null; + }), + }; + + const providerExecutionPort = { + cancelGeneration: vi.fn(async () => { + // Emulate the runtime's asynchronous settle: the abort transitions the + // session out of `generating` shortly after cancellation. + await new Promise((resolve) => setTimeout(resolve, 20)); + status = "idle"; + }), + purgeAcpSessionData: vi.fn(async () => undefined), + }; + + const dispatcher = createDaemonDispatcher( + { + getDefaultModel: vi.fn(() => ({ providerId: "provider-1", modelId: "model-1" })), + getDefaultProjectPath: vi.fn(() => "/tmp/project"), + } as any, + undefined, + sessionRepository as any, + providerExecutionPort as any, + ); + + await expect(dispatcher("sessions.deleteAgentSessions", { agentId: "acp-agent-1" })).resolves.toEqual({ + deletedSessionIds: ["session-2"], + }); + + expect(providerExecutionPort.cancelGeneration).toHaveBeenCalledWith("session-2"); + expect(sessionRepository.delete).toHaveBeenCalledWith("session-2"); + }); + + it("moves sessions to an Argos agent using the target agent's default model", async () => { + const session = { + id: "session-1", + agentId: "acp-agent-1", + title: "Leaving a disabled ACP agent", + projectDir: "/tmp/project", + isPinned: false, + isDraft: false, + sessionKind: "regular", + parentSessionId: null, + subagentEnabled: false, + createdAt: 1, + updatedAt: 1, + status: "idle", + providerId: "acp", + modelId: "acp-agent-1", + }; + + const sessionRepository = { + get: vi.fn(async () => ({ ...session, status: "idle" })), + list: vi.fn(async () => [session]), + listMessages: vi.fn(async () => [{ id: "m-1" }]), + listPendingInputs: vi.fn(async () => []), + deletePendingInput: vi.fn(async () => undefined), + moveSessionToAgent: vi.fn(async (_sessionId: string, input: Record) => ({ + ...session, + ...input, + })), + getGenerationSettings: vi.fn(async () => null), + getDisabledAgentTools: vi.fn(async () => []), + delete: vi.fn(async () => undefined), + }; + + const providerExecutionPort = { + cancelGeneration: vi.fn(async () => undefined), + purgeAcpSessionData: vi.fn(async () => undefined), + }; + + const dispatcher = createDaemonDispatcher( + { + getDefaultModel: vi.fn(() => ({ providerId: "fallback-provider", modelId: "fallback-model" })), + getDefaultProjectPath: vi.fn(() => "/tmp/project"), + getAgentType: vi.fn(async (agentId: string) => (agentId === "argos-agent-2" ? "argos" : null)), + resolveArgosAgentConfig: vi.fn(async () => ({ + defaultModelPreset: { providerId: "openrouter", modelId: "claude-x" }, + })), + } as any, + undefined, + sessionRepository as any, + providerExecutionPort as any, + ); + + await expect( + dispatcher("sessions.moveAgentSessions", { fromAgentId: "acp-agent-1", toAgentId: "argos-agent-2" }), + ).resolves.toEqual({ movedSessionIds: ["session-1"], deletedSessionIds: [] }); + + // The moved session must be labelled with the Argos target's default + // model — a hardcoded `providerId: "acp"` would break the next send. + expect(sessionRepository.moveSessionToAgent).toHaveBeenCalledWith( + "session-1", + expect.objectContaining({ + agentId: "argos-agent-2", + providerId: "openrouter", + modelId: "claude-x", + }), + ); + expect(providerExecutionPort.purgeAcpSessionData).toHaveBeenCalledWith("session-1"); + }); + it("owns summaryTitles route dispatch and delegates to the provider execution port", async () => { const providerExecutionPort = { generateCompletion: vi.fn(async () => "Generated Title"), diff --git a/apps/daemon/test/daemonSessionSettlement.test.ts b/apps/daemon/test/daemonSessionSettlement.test.ts new file mode 100644 index 000000000..1251a1cb7 --- /dev/null +++ b/apps/daemon/test/daemonSessionSettlement.test.ts @@ -0,0 +1,174 @@ +import { describe, expect, it } from "bun:test"; +import type { PendingSessionInputRecord } from "@argos/shared/types/agent-interface"; +import { + settleSessionForOwnershipChange, + type SettleSessionHost, + type SettleSessionResult, +} from "../src/host/sessionSettlement"; + +/** + * Offline coverage for session settlement ahead of ownership changes + * (delete / move / agent removal). Queue-mode pending inputs are discarded, + * steer-mode inputs are kept, a running turn is cancelled and awaited, and + * durable ACP bindings are purged best-effort. + */ + +const noDelay = async () => {}; + +const input = (id: string, mode: "queue" | "steer"): PendingSessionInputRecord => ({ + id, + sessionId: "session-1", + mode, + state: "pending", + payload: { text: `payload-${id}` } as PendingSessionInputRecord["payload"], + queueOrder: mode === "queue" ? 0 : null, + claimedAt: null, + consumedAt: null, + createdAt: 1, + updatedAt: 1, +}); + +type HostOverrides = Partial & { + pendingInputs?: PendingSessionInputRecord[]; + status?: string | null; + statusSequence?: Array; + settleAfterCancels?: number; +}; + +const createHost = (overrides: HostOverrides = {}) => { + const state = { + pendingInputs: overrides.pendingInputs ?? [], + status: overrides.status ?? "idle", + statusSequence: overrides.statusSequence ?? [], + settleAfterCancels: overrides.settleAfterCancels ?? 0, + }; + const host: SettleSessionHost & { + deletedInputIds: string[]; + cancelCalls: string[]; + purgeCalls: string[]; + } = { + getSession: async (sessionId) => { + if (state.statusSequence.length > 0) { + return { status: state.statusSequence.shift() ?? "idle" }; + } + if (state.settleAfterCancels > 0) { + state.settleAfterCancels -= 1; + return { status: "generating" }; + } + void sessionId; + return { status: state.status === "generating" ? "idle" : state.status }; + }, + listPendingInputs: async (sessionId) => { + void sessionId; + return [...state.pendingInputs]; + }, + deletePendingInput: async (sessionId, itemId) => { + void sessionId; + state.pendingInputs = state.pendingInputs.filter((item) => item.id !== itemId); + host.deletedInputIds.push(itemId); + }, + cancelGeneration: async (sessionId) => { + void sessionId; + host.cancelCalls.push(sessionId); + }, + purgeAcpSessionData: async (sessionId) => { + host.purgeCalls.push(sessionId); + }, + deletedInputIds: [], + cancelCalls: [], + purgeCalls: [], + ...overrides, + }; + return { host, state }; +}; + +describe("settleSessionForOwnershipChange", () => { + it("discards queue-mode inputs and keeps steer-mode inputs", async () => { + const { host, state } = createHost({ + pendingInputs: [input("q-1", "queue"), input("q-2", "queue"), input("s-1", "steer")], + status: "idle", + }); + + const result: SettleSessionResult = await settleSessionForOwnershipChange("session-1", host, { + delay: noDelay, + }); + + expect(result.discardedQueueInputIds.sort()).toEqual(["q-1", "q-2"]); + expect(host.deletedInputIds.sort()).toEqual(["q-1", "q-2"]); + expect(state.pendingInputs.map((item) => item.id)).toEqual(["s-1"]); + expect(host.cancelCalls).toEqual([]); + expect(host.purgeCalls).toEqual(["session-1"]); + }); + + it("cancels a generating session and waits for the status to settle", async () => { + const { host } = createHost({ + status: "generating", + settleAfterCancels: 2, + }); + + const result = await settleSessionForOwnershipChange("session-1", host, { delay: noDelay, timeoutMs: 1000 }); + + expect(result.cancelled).toBe(true); + expect(host.cancelCalls).toEqual(["session-1"]); + expect(host.purgeCalls).toEqual(["session-1"]); + }); + + it("does not cancel an idle session", async () => { + const { host } = createHost({ status: "idle" }); + + const result = await settleSessionForOwnershipChange("session-1", host, { delay: noDelay }); + + expect(result.cancelled).toBe(false); + expect(host.cancelCalls).toEqual([]); + }); + + it("throws when the session does not stop before the timeout", async () => { + const { host } = createHost({ status: "generating", settleAfterCancels: 999 }); + + await expect(settleSessionForOwnershipChange("session-1", host, { delay: noDelay, timeoutMs: 0 })).rejects.toThrow( + "did not stop before ownership change", + ); + expect(host.cancelCalls).toEqual(["session-1"]); + }); + + it("keeps queue inputs that cannot be discarded and continues", async () => { + const state = { + pendingInputs: [input("q-1", "queue")], + status: "idle" as string | null, + statusSequence: [] as Array, + settleAfterCancels: 0, + }; + const host: SettleSessionHost & { purgeCalls: string[] } = { + getSession: async () => ({ status: state.status }), + listPendingInputs: async () => [...state.pendingInputs], + deletePendingInput: async () => { + throw new Error("delete failed"); + }, + cancelGeneration: async () => undefined, + purgeAcpSessionData: async (sessionId) => { + host.purgeCalls.push(sessionId); + }, + purgeCalls: [], + }; + + const result = await settleSessionForOwnershipChange("session-1", host, { delay: noDelay }); + + expect(result.discardedQueueInputIds).toEqual([]); + expect(state.pendingInputs).toHaveLength(1); + expect(host.purgeCalls).toEqual(["session-1"]); + }); + + it("tolerates a failing purge", async () => { + const { host } = createHost({ + status: "idle", + purgeAcpSessionData: async () => { + throw new Error("purge failed"); + }, + }); + + await expect(settleSessionForOwnershipChange("session-1", host, { delay: noDelay })).resolves.toEqual({ + cancelled: false, + discardedQueueInputIds: [], + }); + }); +}); diff --git a/apps/desktop/src/main/presenter/configPresenter/index.ts b/apps/desktop/src/main/presenter/configPresenter/index.ts index b90a0e510..7b0e8ee67 100644 --- a/apps/desktop/src/main/presenter/configPresenter/index.ts +++ b/apps/desktop/src/main/presenter/configPresenter/index.ts @@ -78,6 +78,7 @@ import { } from "./daemonMirrorStores"; import { configListAgentsRoute, + configGetAgentTypeRoute, configCreateArgosAgentRoute, configUpdateArgosAgentRoute, configDeleteArgosAgentRoute, @@ -2328,8 +2329,20 @@ export class ConfigPresenter implements IConfigPresenter { } async getAgentType(agentId: string): Promise { - const agent = await this.getAgent(agentId); - return agent?.type ?? null; + // State-agnostic lookup first: config.listAgents excludes disabled or + // uninstalled registry ACP agents, which would strand their sessions + // (unmovable, undeletable, uninstall blocked). See + // docs/issues/acp-agent-removal-settlement. + try { + const result = await invokeDaemonRoute<{ agentType: AgentType | null }>(configGetAgentTypeRoute.name, { + agentId, + }); + return result.agentType ?? null; + } catch (error) { + log.warn("Failed to resolve agent type from daemon:", error); + const agent = await this.getAgent(agentId); + return agent?.type ?? null; + } } async getArgosAgentConfig(agentId: string): Promise { diff --git a/docs/issues/acp-agent-removal-settlement/plan.md b/docs/issues/acp-agent-removal-settlement/plan.md new file mode 100644 index 000000000..c05a0d0f4 --- /dev/null +++ b/docs/issues/acp-agent-removal-settlement/plan.md @@ -0,0 +1,94 @@ +# Plan: ACP agent removal with active sessions (settlement) + +Layer-by-layer, bottom-up. Every new route follows the typed route contract pattern +(contract → catalog → handler → client). + +## 1. Shared contracts + +- `packages/shared-contracts/src/routes/config.routes.ts`: add `configGetAgentTypeRoute` + (`config.getAgentType`, input `{ agentId }`, output `{ agentType: "argos" | "acp" | null }`). +- `packages/shared-contracts/src/routes.ts`: export + `ARGOS_ROUTE_CATALOG` entry. +- `packages/backend-core/src/ports/hotPathPorts.ts`: add optional + `purgeAcpSessionData?(sessionId: string): Promise` to `ProviderExecutionPort` + (grouped with the other ACP methods). + +## 2. Daemon host + +- `apps/daemon/src/host/daemonAcpConfig.ts`: + - Add `getAcpAgentTypeIncludingState(agentId)`: manual agents (any `enabled` state) → registry + agents via `acpRegistryService.listAgents()` (all states) → `null`. Return `"acp"` on hit. +- `apps/daemon/src/host/daemonConfigPresenter.ts`: + - Implement `getAgentType(agentId)`: Argos runtime → `"argos"`; ACP state-agnostic lookup → + `"acp"`; else `null`. (Satisfies the existing `IConfigPresenter` declaration.) +- `apps/daemon/src/host/daemonAcpSqlite.ts` already has `deleteAcpSessions(conversationId)`; + expose it on `AcpSessionPersistence` (`packages/acp-runtime/src/session/acpSessionPersistence.ts`) + as `deleteAllSessions(conversationId)`. +- `apps/daemon/src/host/acp-provider-execution.ts`: implement `purgeAcpSessionData(sessionId)`: + best-effort `cancelGeneration` (aborts turn, suppresses drain, clears in-memory session) then + `sessionPersistence.deleteAllSessions(sessionId)`. +- `apps/daemon/src/index.ts`: route `purgeAcpSessionData` through the unified + `providerExecutionPort` (ACP-only; pi is a no-op) and add it to the port `Pick<...>` list. + +## 3. Settlement helper (new) + +- `apps/daemon/src/host/sessionSettlement.ts`: `settleSessionForOwnershipChange(sessionId, host, options?)`: + 1. Load session + pending inputs; delete `mode === "queue"` items via the repository. + 2. If status is `generating`: best-effort `providerExecutionPort.cancelGeneration(sessionId)`, + then poll `sessionRepository.get(sessionId).status` until it leaves `generating` + (100 ms interval, 10 s default cap) — throw `did not stop before ownership change` on timeout. + 3. `purgeAcpSessionData(sessionId)` best-effort (no-op for non-ACP sessions). +- Dispatch it from the daemon: + - `sessions.delete` / `sessions.deleteAgentSessions`: settle each session before `repo.delete`. + - `sessions.moveAgentSessions` / `sessions.moveToAgent`: settle before `repo.moveSessionToAgent`. +- **Target-aware move context (found during implementation):** the daemon move handlers + hardcoded `providerId: "acp"` / `modelId: `, which breaks any move whose target is an + Argos agent (next send fails with "ACP agent not found", and the pre-existing Argos→Argos bulk + move mislabelled sessions). Both handlers now resolve the target via `config.getAgentType`: + ACP targets keep the historical convention; Argos targets receive the target agent's + `defaultModelPreset` (falling back to the global default model), mirroring the desktop's + `resolveTransferTargetContext`. + +## 4. Backend-core dispatch + +- `packages/backend-core/src/dispatch/config/configRouteHandler.ts`: handle + `configGetAgentTypeRoute` via `configPresenter.getAgentType(agentId)`. + +## 5. Desktop shell (production path + legacy parity) + +- `apps/desktop/src/main/presenter/configPresenter/index.ts`: `getAgentType` tries the new + `config.getAgentType` route first; falls back to agentRepository → `config.listAgents` chain. +- `apps/desktop/src/main/presenter/agentSessionPresenter/index.ts` (legacy no-daemon path): + - `assessTransferSession` also returns `hasPendingInput`. + - Add `settleSessionForOwnershipChange(session)`: discard queue inputs via the agent + implementation, cancel + poll status via `agent.getSessionState` (10 s cap), keep steer + inputs. + - Use it in `moveAgentSessions`, `moveSessionToAgentInternal`, `deleteAgentSessions`, and + `deleteSessionInternal` (before destroy), replacing the hard `blockReason` throws. + +## 6. UI (ACP settings uninstall flow) + +- `packages/ui/settings/components/AcpSettings.tsx`: + - On uninstall of an agent with conversations, fetch `sessionClient.getAgentTransferImpact` + + `configClient.listAgents` and open the shared `AgentTransferDialog` + (`packages/ui/src/components/agent/AgentTransferDialog.tsx`, mode `"delete-agent"`). + - `onConfirmMove` → `sessionClient.moveAgentSessions(agentId, target)` → uninstall. + - `onConfirmDelete` → `sessionClient.deleteAgentSessions(agentId)` → uninstall. + - Keep the simple confirm dialog for agents with no conversations. + +## 7. Tests + +- Daemon (bun test): + - `apps/daemon/test/daemonSessionSettlement.test.ts` (new): discards queue inputs, keeps steer, + cancels + polls to idle, times out with error, purges ACP data. + - `apps/daemon/test/daemonAcpConfig.test.ts`: state-agnostic type lookup (disabled + + not_installed registry agents, disabled manual agent, unknown → null). + - `apps/daemon/test/daemonSessionRoutes.test.ts`: `sessions.deleteAgentSessions` settles and + purges before delete. +- Desktop (vitest): + - `apps/desktop/test/main/presenter/agentSessionPresenter/settlement.test.ts` (new): legacy + path settles active/queued sessions before move/delete; assessment conservative on failure. + +## 8. Verification + +- `bun run typecheck`, `bun run format`, `bun run lint`, `bun test` (daemon), desktop + `test:main`. diff --git a/docs/issues/acp-agent-removal-settlement/spec.md b/docs/issues/acp-agent-removal-settlement/spec.md new file mode 100644 index 000000000..e056183ac --- /dev/null +++ b/docs/issues/acp-agent-removal-settlement/spec.md @@ -0,0 +1,92 @@ +# Spec: ACP agent removal with active sessions (settlement) + +## Problem + +Uninstalling (or disabling) an ACP registry agent that still has conversations is effectively +impossible, and is a permanent dead-end once used: + +1. `uninstallAcpRegistryAgent` refuses while any `acp_sessions` row exists for the agent + (`daemonAcpConfig.ts:331` — "ACP registry agent still has related conversations"). +2. `acp_sessions` rows are **never deleted** by any production code path + (`AcpSessionPersistence.deleteSession` has zero callers). Deleting the conversations does not + remove the bindings, so the guard stays true forever. +3. The conversations of a *disabled or uninstalled* agent cannot be moved or deleted either: + `config.getAgentType` resolves types through `getAcpAgents()`, which filters to + `enabled && installState.status === "installed"` (`daemonAcpConfig.ts:395`), so + `resolveAgentImplementation` throws `Agent not found` for the affected sessions and + `getAgentTransferImpact` / `moveAgentSessions` / `deleteAgentSessions` fail. +4. Bulk session ownership changes (daemon routes `sessions.deleteAgentSessions`, + `sessions.moveAgentSessions`, `sessions.moveToAgent`, `sessions.delete`) perform raw row + deletes/moves with no settlement: running generations are not cancelled, queued inputs are + silently dropped or leak, and ACP bindings are left behind. + +Discovered while evaluating DeepChat PR #2188 ("fix(acp): allow uninstall while disabled"), which +fixes the same class of bug upstream: lightweight status lookups that do not resolve the disabled +agent, discard queue-only inputs, cancel active turns, and wait for cancellation to settle before +ownership changes. The fix here is a native re-implementation for Argos' daemon-owned session +architecture (no code port). + +## Goals + +- Uninstalling an ACP registry agent works without re-enabling it first, once its conversations + are moved or deleted. +- Deleting an agent's conversations settles running generations first: discard queue-mode pending + inputs, cancel the active turn (both pi and ACP backends), and wait (bounded) for the status to + leave `generating`. +- Moving a session to another agent performs the same settlement before the ownership change. +- Session deletion removes the session's `acp_sessions` bindings so the uninstall guard becomes + accurate. +- Agent-type resolution works for disabled/uninstalled registry ACP agents (state-agnostic + lookup), so impact assessment and move/delete UI work for the agent being removed. +- The ACP settings uninstall flow offers move/delete of conversations (reusing + `AgentTransferDialog`) instead of failing with a guard error. + +## Non-goals + +- No cron-style scheduling changes, no compaction changes (other DeepChat-adjacent features). +- No change to the uninstall guard semantics itself (`hasAcpAgentSessions` stays; it becomes + accurate because bindings are now cleaned). +- No moving of conversation history *to* ACP agents beyond what exists today. +- No desktop-local presenter rewrite; the daemon owns sessions and the daemon paths are fixed + first. The legacy no-daemon desktop path gets parity-level settlement only. + +## Decisions + +- **D1 — State-agnostic type lookup via a new route** (`config.getAgentType`): the daemon checks + the Argos agent runtime, then manual ACP agents, then *all* registry agents regardless of + `enabled`/install state. Adding an option to `config.listAgents` was rejected: that route feeds + agent pickers and orchestration (`argos_agents_list`) which must keep excluding disabled + agents. +- **D2 — Settlement helper lives in the daemon** (`apps/daemon/src/host/sessionSettlement.ts`) as + a factory over existing ports (`sessionRepository`, unified `providerExecutionPort`, ACP purge), + so both pi and ACP backends are covered by one code path and it is unit-testable with bun test. +- **D3 — Queue-mode inputs are discarded, steer-mode inputs are left in place**: cancelGeneration + already suppresses the pending-input drain (`drainSuppressedSessions` in the ACP port, same + mechanism in pi), so a cancelled run will not claim steer inputs. This avoids the orphaned + steer-input follow-up noted in upstream review. +- **D4 — Bounded settlement wait (10 s)** polling the session status; on timeout the operation + fails with a clear error and the session stays on its agent (same degradation as upstream). +- **D5 — Binding purge is part of settlement**: `purgeAcpSessionData(sessionId)` (new optional + `ProviderExecutionPort` method) cancels best-effort, unbinds the in-memory ACP session, and + deletes the conversation's `acp_sessions` rows. Called by delete routes after the row delete + and by move routes after the source ownership change. +- **D8 — Target-aware move context**: daemon move handlers resolve the target agent type. + ACP targets keep `providerId: "acp"` + `modelId: `; Argos targets receive the target's + default model instead of the previously hardcoded (and broken) `acp` labelling. +- **D6 — Uninstall UI**: `AcpSettings` replaces the bare confirm dialog with the existing + `AgentTransferDialog` flow when the agent has conversations (impact → move to an Argos agent or + delete conversations → uninstall). With no conversations, uninstall proceeds directly. +- **D7 — Desktop-local settlement skipped**: every live delete/move flow dispatches through the + daemon (the shell's argos agent implementation is a stateless stub and `sessions.delete` is + daemon-handled), so settlement is implemented once, daemon-side. The desktop-local + `agentSessionPresenter` paths have no production callers and keep their conservative blocking + for the no-daemon degraded mode. + +## Risks / constraints + +- Settlement timeout (10 s) can still block a delete if a backend ignores cancellation; failure + surfaces a clear error and leaves data intact (fail-safe, not fail-open). +- `daemon_sessions.status` must be readable via the session repository for polling; existing + `SessionStatus` values (`generating`) already persist there. +- Architecture guards: new route must be registered in `ARGOS_ROUTE_CATALOG` and handled in + `configRouteHandler` (route-catalog drift guard). diff --git a/docs/issues/acp-agent-removal-settlement/tasks.md b/docs/issues/acp-agent-removal-settlement/tasks.md new file mode 100644 index 000000000..91f15a67b --- /dev/null +++ b/docs/issues/acp-agent-removal-settlement/tasks.md @@ -0,0 +1,29 @@ +# Tasks: ACP agent removal with active sessions (settlement) + +- [x] T1 Contract: `config.getAgentType` route + catalog entry (shared-contracts) +- [x] T2 Contract: `purgeAcpSessionData?` on `ProviderExecutionPort` (backend-core ports) +- [x] T3 Daemon: `getAcpAgentTypeIncludingState` in `daemonAcpConfig` + `getAgentType` in + `daemonConfigPresenter` +- [x] T4 Daemon: `AcpSessionPersistence.deleteAllSessions` + + `acp-provider-execution.purgeAcpSessionData` + unified port wiring (`index.ts`) +- [x] T5 Daemon: `sessionSettlement.ts` helper (discard queue, cancel, bounded poll, purge) +- [x] T6 Daemon: wire settlement into `sessions.delete`, `sessions.deleteAgentSessions`, + `sessions.moveAgentSessions`, `sessions.moveToAgent` — plus target-aware move context + (Argos targets get the target's default model instead of hardcoded `acp`; plan §3, D8) +- [x] T7 Backend-core: `config.getAgentType` handler case +- [x] T8 Desktop: `configPresenter.getAgentType` route-first fallback chain +- [x] T9 ~~Desktop: legacy-path settlement~~ — **dropped**: no live callers (all delete/move flows + dispatch through the daemon; the desktop-local path only runs in no-daemon degraded mode + where the agent stub carries no state). Decision recorded in spec (D7). +- [x] T10 UI: AcpSettings uninstall flow via shared `AgentTransferDialog` (move/delete → uninstall) +- [x] T11 Tests: daemon settlement (6 cases) + type lookup (2 cases) + delete-route settlement + (2 cases) + Argos-target move regression (1 case) +- [x] T12 ~~Tests: desktop legacy-path settlement~~ — dropped with T9 +- [x] T13 `bun run format` + `bun run lint` + `bun run typecheck` + `bun test` + +## Verification results + +- Daemon: 384 tests pass (`bun test` in `apps/daemon`); `tsc --noEmit` clean. +- Desktop: `test:main` 1737 passed / 6 skipped; `typecheck:node` clean. +- UI: `typecheck:web` clean. +- `bun run lint`: agent-cleanup, architecture, and route-catalog drift guards + oxlint clean. diff --git a/packages/acp-runtime/src/session/acpSessionPersistence.ts b/packages/acp-runtime/src/session/acpSessionPersistence.ts index fc1dc105a..d865246bc 100644 --- a/packages/acp-runtime/src/session/acpSessionPersistence.ts +++ b/packages/acp-runtime/src/session/acpSessionPersistence.ts @@ -252,6 +252,11 @@ export class AcpSessionPersistence { await this.sqlitePresenter.deleteAcpSession(conversationId, agentId); } + /** Delete every binding row recorded for a conversation (all agent ids). */ + async deleteAllSessions(conversationId: string): Promise { + await this.sqlitePresenter.deleteAcpSessions(conversationId); + } + async clearSession(conversationId: string, agentId: string): Promise { await this.updateStatus(conversationId, agentId, "idle"); } diff --git a/packages/backend-core/src/dispatch/config/configRouteHandler.ts b/packages/backend-core/src/dispatch/config/configRouteHandler.ts index b764c35cd..c2d7ef202 100644 --- a/packages/backend-core/src/dispatch/config/configRouteHandler.ts +++ b/packages/backend-core/src/dispatch/config/configRouteHandler.ts @@ -38,6 +38,7 @@ import { configGetThemeRoute, configGetVoiceAiConfigRoute, configListAgentsRoute, + configGetAgentTypeRoute, configListCustomPromptsRoute, configCreateArgosAgentRoute, configUpdateArgosAgentRoute, @@ -356,6 +357,14 @@ export async function dispatchConfigRoute( return configListAgentsRoute.output.parse({ agents }); } + case configGetAgentTypeRoute.name: { + const input = configGetAgentTypeRoute.input.parse(rawInput); + // State-agnostic lookup: resolves disabled/uninstalled ACP agents too, + // so their sessions stay assessable for move/delete before removal. + const agentType = await configPresenter.getAgentType(input.agentId); + return configGetAgentTypeRoute.output.parse({ agentType: agentType ?? null }); + } + case configResolveArgosAgentConfigRoute.name: { const input = configResolveArgosAgentConfigRoute.input.parse(rawInput); return configResolveArgosAgentConfigRoute.output.parse({ diff --git a/packages/backend-core/src/ports/hotPathPorts.ts b/packages/backend-core/src/ports/hotPathPorts.ts index 28049a7f4..4ba0b501f 100644 --- a/packages/backend-core/src/ports/hotPathPorts.ts +++ b/packages/backend-core/src/ports/hotPathPorts.ts @@ -56,6 +56,13 @@ export interface ProviderExecutionPort { setAcpPreferredProcessMode?(agentId: string, modeId: string): Promise; prepareAcpSession?(conversationId: string, agentId: string, workdir: string): Promise; clearAcpSession?(sessionId: string): Promise; + /** + * Release an ACP session's durable bindings before an ownership change + * (delete/move): best-effort cancel of any active turn, then delete the + * conversation's `acp_sessions` rows. No-op for sessions without an ACP + * binding. + */ + purgeAcpSessionData?(sessionId: string): Promise; getAcpSessionModes?(conversationId: string): Promise; setAcpSessionMode?(conversationId: string, modeId: string): Promise; resolveAgentPermission?(requestId: string, granted: boolean): Promise; diff --git a/packages/shared-contracts/src/routes.ts b/packages/shared-contracts/src/routes.ts index d25f69fbf..574224701 100644 --- a/packages/shared-contracts/src/routes.ts +++ b/packages/shared-contracts/src/routes.ts @@ -62,6 +62,7 @@ import { configGetSystemPromptsRoute, configGetThemeRoute, configGetVoiceAiConfigRoute, + configGetAgentTypeRoute, configListAgentsRoute, configListCustomPromptsRoute, configCreateArgosAgentRoute, @@ -616,6 +617,7 @@ export const ARGOS_ROUTE_CATALOG = { [configSetDefaultSystemPromptIdRoute.name]: configSetDefaultSystemPromptIdRoute, [configGetAcpStateRoute.name]: configGetAcpStateRoute, [configListAgentsRoute.name]: configListAgentsRoute, + [configGetAgentTypeRoute.name]: configGetAgentTypeRoute, [configCreateArgosAgentRoute.name]: configCreateArgosAgentRoute, [configUpdateArgosAgentRoute.name]: configUpdateArgosAgentRoute, [configDeleteArgosAgentRoute.name]: configDeleteArgosAgentRoute, diff --git a/packages/shared-contracts/src/routes/config.routes.ts b/packages/shared-contracts/src/routes/config.routes.ts index 7b6bdae0e..4dfa87454 100644 --- a/packages/shared-contracts/src/routes/config.routes.ts +++ b/packages/shared-contracts/src/routes/config.routes.ts @@ -487,6 +487,20 @@ export const configListAgentsRoute = defineRouteContract({ }), }); +// State-agnostic agent-type lookup. Unlike config.listAgents (which excludes +// disabled/uninstalled registry ACP agents), this resolves the type of any +// known agent so sessions bound to a disabled agent can still be assessed, +// moved, or deleted before agent removal. +export const configGetAgentTypeRoute = defineRouteContract({ + name: "config.getAgentType", + input: zod.object({ + agentId: zod.string().min(1), + }), + output: zod.object({ + agentType: zod.enum(["argos", "acp"]).nullable(), + }), +}); + export const configResolveArgosAgentConfigRoute = defineRouteContract({ name: "config.resolveArgosAgentConfig", input: zod.object({ diff --git a/packages/ui/settings/components/AcpSettings.tsx b/packages/ui/settings/components/AcpSettings.tsx index 14c2b7e99..bcce6c97b 100644 --- a/packages/ui/settings/components/AcpSettings.tsx +++ b/packages/ui/settings/components/AcpSettings.tsx @@ -39,12 +39,18 @@ import { DialogTitle, } from "#shadcn/components/ui/dialog"; import type { AcpManualAgent, AcpRegistryAgent } from "@argos/shared/presenter"; +import type { AgentTransferImpact } from "@argos/shared/types/agent-interface"; import { createConfigClient } from "#api/ConfigClient"; +import { createSessionClient } from "#api/SessionClient"; import { toast } from "#/components/use-toast"; import AcpDebugDialog from "./AcpDebugDialog"; import AcpDiagnostics from "./AcpDiagnostics"; import AcpAgentIcon from "#/components/icons/AcpAgentIcon"; import AgentMcpSelector from "#/components/mcp-config/AgentMcpSelector"; +import AgentTransferDialog, { type TransferDialogAgent } from "#/components/agent/AgentTransferDialog"; + +const sessionClient = createSessionClient(); + type RegistryDialogFilter = "all" | "installed" | "not_installed"; const parseEnvBlock = (value: string): Record => { return Object.fromEntries( @@ -243,6 +249,12 @@ const useAcpSettingsController = () => { const [connectionCheckRequests, setConnectionCheckRequests] = useState>({}); const [uninstallOpen, setUninstallOpen] = useState(false); const [uninstallAgent, setUninstallAgent] = useState(null); + const [uninstallImpact, setUninstallImpact] = useState(null); + const [uninstallImpactLoading, setUninstallImpactLoading] = useState(false); + const [uninstallTransferOpen, setUninstallTransferOpen] = useState(false); + const [uninstallTransferBusy, setUninstallTransferBusy] = useState(false); + const [uninstallTransferError, setUninstallTransferError] = useState(null); + const [uninstallTargets, setUninstallTargets] = useState([]); const setPending = (id: string, pending: boolean) => setAgentPending((current) => updatePendingState(current, id, pending)); const requestConnectionCheck = (id: string) => @@ -437,13 +449,75 @@ const useAcpSettingsController = () => { const confirmRegistryAgentUninstall = (agent: AcpRegistryAgent) => { setUninstallAgent(agent); setUninstallOpen(true); + setUninstallImpact(null); + setUninstallImpactLoading(true); + setUninstallTransferError(null); + // Prefetch the conversation impact so the confirm step can route to the + // transfer dialog when the agent still owns conversations. + void Promise.all([sessionClient.getAgentTransferImpact(agent.id), configClient.listAgents()]) + .then(([impact, agents]) => { + setUninstallImpact(impact); + setUninstallTargets(agents ?? []); + }) + .catch((error) => { + console.warn("[ACP] uninstall impact lookup failed:", error); + setUninstallImpact(null); + }) + .finally(() => setUninstallImpactLoading(false)); + }; + const finishUninstall = () => { + setUninstallOpen(false); + setUninstallTransferOpen(false); + setUninstallAgent(null); + setUninstallImpact(null); + setUninstallTransferError(null); }; const confirmRegistryAgentUninstallAction = async () => { const agent = uninstallAgent; if (!agent) return; + // Conversations still bound to this agent must be moved or deleted + // first — route to the transfer dialog instead of failing the uninstall. + if (uninstallImpactLoading) return; + if ((uninstallImpact?.totalSessions ?? 0) > 0) { + setUninstallOpen(false); + setUninstallTransferOpen(true); + return; + } setUninstallOpen(false); await uninstallRegistryAgent(agent); - setUninstallAgent(null); + finishUninstall(); + }; + const handleUninstallWithMove = async (payload: { targetAgentId: string }) => { + const agent = uninstallAgent; + if (!agent) return; + setUninstallTransferBusy(true); + setUninstallTransferError(null); + try { + await sessionClient.moveAgentSessions(agent.id, payload.targetAgentId); + await configClient.uninstallAcpRegistryAgent(agent.id); + await loadAcpData(); + finishUninstall(); + toast({ title: "Agent removed" }); + } catch (error) { + setUninstallTransferError(error instanceof Error ? error.message : String(error)); + } + setUninstallTransferBusy(false); + }; + const handleUninstallWithDelete = async () => { + const agent = uninstallAgent; + if (!agent) return; + setUninstallTransferBusy(true); + setUninstallTransferError(null); + try { + await sessionClient.deleteAgentSessions(agent.id); + await configClient.uninstallAcpRegistryAgent(agent.id); + await loadAcpData(); + finishUninstall(); + toast({ title: "Agent removed" }); + } catch (error) { + setUninstallTransferError(error instanceof Error ? error.message : String(error)); + } + setUninstallTransferBusy(false); }; const handleRegistryCatalogAction = async (agent: AcpRegistryAgent) => { const status = agent.installState?.status ?? "not_installed"; @@ -498,6 +572,15 @@ const useAcpSettingsController = () => { updateRegistryAgent, confirmRegistryAgentUninstall, confirmRegistryAgentUninstallAction, + uninstallImpact, + uninstallImpactLoading, + uninstallTransferOpen, + uninstallTransferBusy, + uninstallTransferError, + uninstallTargets, + setUninstallTransferOpen, + handleUninstallWithMove, + handleUninstallWithDelete, handleRegistryCatalogAction, toggleManualAgentEnabled, deleteManualAgent, @@ -529,6 +612,15 @@ export default function AcpSettings() { updateRegistryAgent, confirmRegistryAgentUninstall, confirmRegistryAgentUninstallAction, + uninstallImpact, + uninstallImpactLoading, + uninstallTransferOpen, + uninstallTransferBusy, + uninstallTransferError, + uninstallTargets, + setUninstallTransferOpen, + handleUninstallWithMove, + handleUninstallWithDelete, handleRegistryCatalogAction, toggleManualAgentEnabled, deleteManualAgent, @@ -735,11 +827,29 @@ export default function AcpSettings() { 0} onOpenChange={setUninstallOpen} onCancel={() => setUninstallOpen(false)} onConfirm={() => void confirmRegistryAgentUninstallAction()} /> + void handleUninstallWithMove(payload)} + onConfirmDelete={() => void handleUninstallWithDelete()} + /> + ); @@ -1616,12 +1726,16 @@ const RegistryDialog = ({ const UninstallAlertDialog = ({ open, agent, + impactLoading, + hasConversations, onOpenChange, onCancel, onConfirm, }: { open: boolean; agent: AcpRegistryAgent | null; + impactLoading: boolean; + hasConversations: boolean; onOpenChange: (open: boolean) => void; onCancel: () => void; onConfirm: () => void; @@ -1631,17 +1745,21 @@ const UninstallAlertDialog = ({ {agent ? `Uninstall ${agent.name}?` : "Uninstall Agent?"} - This will remove the agent and its configuration. You can reinstall it from the registry later. + {hasConversations + ? "This agent still has conversations. You can move them to another agent or delete them before removal. The agent and its configuration will be removed either way." + : impactLoading + ? "Checking for related conversations..." + : "This will remove the agent and its configuration. You can reinstall it from the registry later."} Cancel - Uninstall + {hasConversations ? "Continue" : "Uninstall"} diff --git a/packages/ui/src/components/agent/AgentTransferDialog.tsx b/packages/ui/src/components/agent/AgentTransferDialog.tsx index fdfff3ef4..245b5fcae 100644 --- a/packages/ui/src/components/agent/AgentTransferDialog.tsx +++ b/packages/ui/src/components/agent/AgentTransferDialog.tsx @@ -27,6 +27,8 @@ interface AgentTransferDialogProps { loading?: boolean; busy?: boolean; error?: string | null; + /** Optional title override (e.g. "Uninstall X" instead of "Delete X"). */ + title?: string; onOpenChange: (open: boolean) => void; onConfirmMove: (payload: { targetAgentId: string }) => void; onConfirmDelete: () => void; @@ -42,6 +44,7 @@ export default function AgentTransferDialog({ loading = false, busy = false, error = null, + title, onOpenChange, onConfirmMove, onConfirmDelete, @@ -52,7 +55,7 @@ export default function AgentTransferDialog({ (agent) => agent.enabled !== false && agent.id !== sourceAgentId && agent.type === "argos", ); const showTargetPicker = mode === "move-session" || action === "move"; - const title = mode === "delete-agent" ? `Delete ${sourceAgentName}` : "Move Conversation"; + const dialogTitle = title ?? (mode === "delete-agent" ? `Delete ${sourceAgentName}` : "Move Conversation"); const description = mode === "delete-agent" ? "Choose how to handle existing conversations" @@ -95,7 +98,7 @@ export default function AgentTransferDialog({ }} > - {title} + {dialogTitle} {description} From d0484c851c8fbb68c65419907733cc5e39278a3b Mon Sep 17 00:00:00 2001 From: Francisco Pizarro Date: Tue, 8 Sep 2026 14:00:03 -0300 Subject: [PATCH 2/5] feat(toolchains): managed installs for node, uv, ripgrep The daemon resolved external runtimes through identity no-op ports, so the headless deployment could only run npx/uvx agents and uvx MCP servers when Node/uv happened to be on PATH, with no verification and no toolchain UX. Modeled on ThinkInAIXYZ/deepchat#2193, re-designed for Argos' daemon-first architecture. - add a daemon-owned ToolchainService: explicit persisted sources (custom / unconfigured) over derived ones (managed / bundled / system), with precedence, a warm sync cache for sync host seams, and timestamped quarantine of corrupt state - managed installs download pinned Node (v24.18.0) and uv (0.9.18) archives, verify SHA-256, extract to staging, and activate atomically via rename; the previous tree rotates to .prev and a failed or cancelled install leaves it active - wire daemon consumers through the service: ACP launch resolves npx/npm/node/uvx (npx becomes node npx-cli.js via a new optional resolveCommandWithArgs host seam) and prepends resolved bin dirs to the spawn PATH; MCP stdio commands rewrite through the warm cache - probe bundled seeds for the headless daemon (execDir/../runtime, execDir/runtime, cwd/runtime, dataDir/runtime) - add a Toolchains settings page: per-tool source, path, version, install/repair, cancel, revert, custom path - fix bundled-runtime doc drift (no bundled Bun/rtk; seeds are uv + ripgrep) SDD: docs/features/managed-toolchains --- AGENTS.md | 2 +- CONTRIBUTING.md | 2 +- apps/daemon/src/dispatch/daemonDispatcher.ts | 47 +++ .../daemon/src/host/acp-provider-execution.ts | 7 +- apps/daemon/src/host/acpPorts.ts | 31 +- apps/daemon/src/host/daemonMcpPorts.ts | 9 +- apps/daemon/src/host/toolchains/catalog.ts | 101 +++++ apps/daemon/src/host/toolchains/install.ts | 188 +++++++++ apps/daemon/src/host/toolchains/locate.ts | 159 ++++++++ apps/daemon/src/host/toolchains/service.ts | 358 ++++++++++++++++++ apps/daemon/src/host/toolchains/state.ts | 70 ++++ apps/daemon/src/host/toolchains/types.ts | 37 ++ apps/daemon/src/index.ts | 12 + apps/daemon/test/toolchainsRoutes.test.ts | 164 ++++++++ apps/daemon/test/toolchainsService.test.ts | 282 ++++++++++++++ apps/daemon/test/toolchainsState.test.ts | 92 +++++ docs/features/managed-toolchains/plan.md | 66 ++++ docs/features/managed-toolchains/spec.md | 96 +++++ docs/features/managed-toolchains/tasks.md | 25 ++ packages/acp-runtime/src/host/ports.ts | 11 + .../src/process/acpProcessManager.ts | 18 +- packages/shared-contracts/src/routes.ts | 13 + .../src/routes/system.routes.ts | 1 + .../src/routes/toolchains.routes.ts | 100 +++++ packages/shared/src/settingsNavigation.ts | 11 + packages/ui/api/ToolchainClient.ts | 42 ++ .../components/ToolchainsSettings.tsx | 268 +++++++++++++ packages/ui/settings/main.tsx | 2 + 28 files changed, 2195 insertions(+), 19 deletions(-) create mode 100644 apps/daemon/src/host/toolchains/catalog.ts create mode 100644 apps/daemon/src/host/toolchains/install.ts create mode 100644 apps/daemon/src/host/toolchains/locate.ts create mode 100644 apps/daemon/src/host/toolchains/service.ts create mode 100644 apps/daemon/src/host/toolchains/state.ts create mode 100644 apps/daemon/src/host/toolchains/types.ts create mode 100644 apps/daemon/test/toolchainsRoutes.test.ts create mode 100644 apps/daemon/test/toolchainsService.test.ts create mode 100644 apps/daemon/test/toolchainsState.test.ts create mode 100644 docs/features/managed-toolchains/plan.md create mode 100644 docs/features/managed-toolchains/spec.md create mode 100644 docs/features/managed-toolchains/tasks.md create mode 100644 packages/shared-contracts/src/routes/toolchains.routes.ts create mode 100644 packages/ui/api/ToolchainClient.ts create mode 100644 packages/ui/settings/components/ToolchainsSettings.tsx diff --git a/AGENTS.md b/AGENTS.md index d8e5ffeca..e29cce89a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -161,7 +161,7 @@ The `#/` alias is context-sensitive: a custom Vite plugin (`createPathAliasPlugi - Secrets: use `.env` (see `.env.example`); never commit keys. - Toolchains: Bun 1.4.0. Windows: enable Developer Mode for symlinks. - Build: Vite 8 with Rolldown; `vite-plugin-electron` multi-env for main/preload/renderer. -- Runtimes: bundled Bun, ripgrep, uv, rtk in `runtime/` — installed via `bun run installRuntime`. +- Runtimes: uv and ripgrep seeds in `runtime/` — installed via `bun run installRuntime`. Node, uv, and ripgrep resolve at runtime through the daemon's managed toolchain service (see `docs/features/managed-toolchains`). ## Specification-Driven Development diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6b193a3f4..a925e73dd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -119,7 +119,7 @@ Argos is a Turborepo monorepo. The desktop app is an Electron **shell** that loa - `packages/shared/` (`@argos/shared`): Shared types and utilities (web-safe). - `packages/backend-core/`, `packages/{acp,mcp,skills,memory,remote-control}-runtime/`, `packages/agent-runtime/`, `packages/pi-orchestrator-extension/`: Shared backend logic and host-port-injected runtimes. - `apps/landing/`: Marketing site + GitHub OAuth relay (Cloudflare Worker). -- `runtime/`: Bundled runtimes used by MCP and agent tooling (Bun/uv/ripgrep/rtk) — installed via `bun run installRuntime`. +- `runtime/`: Bundled runtime seeds used by MCP and agent tooling (uv/ripgrep) - installed via `bun run installRuntime`. Node/uv/ripgrep used by the daemon resolve through the managed toolchain service (`apps/daemon/src/host/toolchains/`). - `scripts/`, `resources/`, `build/`: Build, packaging, and asset pipelines. - `dist/`, `out/`: Build outputs (do not edit manually). - `docs/`: Design docs, guides, and the SDD spec/plan/task records. diff --git a/apps/daemon/src/dispatch/daemonDispatcher.ts b/apps/daemon/src/dispatch/daemonDispatcher.ts index 2551fafd5..78522a839 100644 --- a/apps/daemon/src/dispatch/daemonDispatcher.ts +++ b/apps/daemon/src/dispatch/daemonDispatcher.ts @@ -31,6 +31,7 @@ import { resolveDaemonVersion } from "../version"; import type { DaemonTerminalRuntime } from "../terminal/daemonTerminalRuntime"; import { diagnoseDaemonSchema, repairDaemonSchema } from "../host/daemonSchemaDiagnostics"; import { settleSessionForOwnershipChange, type SettleSessionHost } from "../host/sessionSettlement"; +import type { ToolchainService } from "../host/toolchains/service"; import { getPiToolDefinitions } from "../host/piToolCatalog"; import { aggregateUsageStats, resolveBuiltinModelPrice } from "../host/usageStatsAggregator"; import { resolveModelCost } from "../host/modelCost"; @@ -48,6 +49,11 @@ import { onboardingSetStepStatusRoute, onboardingCompleteRoute, onboardingResetRoute, + toolchainsListRoute, + toolchainsSetSourceRoute, + toolchainsRemoveSourceRoute, + toolchainsInstallRoute, + toolchainsCancelInstallRoute, settingsGetSnapshotRoute, settingsUpdateRoute, settingsActivityListRoute, @@ -935,6 +941,7 @@ export function createDaemonDispatcher( }, knowledgeRuntime?: DaemonKnowledgeRuntimePort, terminalRuntime?: DaemonTerminalRuntime, + toolchains?: ToolchainService, ): RouteDispatcher { const settingsHandler = new SettingsRouteHandler(createSettingsRouteAdapter(configPresenter)); const runtime: { @@ -2061,6 +2068,46 @@ export function createDaemonDispatcher( return settingsListSystemFontsRoute.output.parse({ fonts: [] }); } + if (route === toolchainsListRoute.name) { + if (!toolchains) throw new Error("Toolchain service is not available in this runtime."); + toolchainsListRoute.input.parse(rawInput); + return toolchainsListRoute.output.parse({ tools: await toolchains.list() }); + } + + if (route === toolchainsSetSourceRoute.name) { + if (!toolchains) throw new Error("Toolchain service is not available in this runtime."); + const input = toolchainsSetSourceRoute.input.parse(rawInput); + return toolchainsSetSourceRoute.output.parse({ + status: await toolchains.setSource(input.tool, input.source, input.path), + }); + } + + if (route === toolchainsRemoveSourceRoute.name) { + if (!toolchains) throw new Error("Toolchain service is not available in this runtime."); + const input = toolchainsRemoveSourceRoute.input.parse(rawInput); + return toolchainsRemoveSourceRoute.output.parse({ status: await toolchains.removeSource(input.tool) }); + } + + if (route === toolchainsInstallRoute.name) { + if (!toolchains) throw new Error("Toolchain service is not available in this runtime."); + const input = toolchainsInstallRoute.input.parse(rawInput); + const started = toolchains.install(input.tool); + return toolchainsInstallRoute.output.parse({ + started: started.started, + status: await toolchains.status(input.tool), + }); + } + + if (route === toolchainsCancelInstallRoute.name) { + if (!toolchains) throw new Error("Toolchain service is not available in this runtime."); + const input = toolchainsCancelInstallRoute.input.parse(rawInput); + toolchains.cancelInstall(input.tool); + return toolchainsCancelInstallRoute.output.parse({ + cancelled: true, + status: await toolchains.status(input.tool), + }); + } + if (isDesktopOnlyRoute(route)) { // Routes that are truly desktop-only (open windows, file dialogs) throw. throw new Error(`Route not available in headless mode: ${route}`); diff --git a/apps/daemon/src/host/acp-provider-execution.ts b/apps/daemon/src/host/acp-provider-execution.ts index e821747db..09d44edf0 100644 --- a/apps/daemon/src/host/acp-provider-execution.ts +++ b/apps/daemon/src/host/acp-provider-execution.ts @@ -25,6 +25,7 @@ import type { BunSessionRepository } from "./bun-session-repository"; import { usageDateKey } from "./bun-session-repository"; import { createDaemonAcpPorts } from "./acpPorts"; import { createDaemonAcpSqlitePresenter } from "./daemonAcpSqlite"; +import type { ToolchainService } from "./toolchains/service"; import { sessionsStatusChangedEvent } from "@argos/shared-contracts"; import { methods as acpMethods, PROTOCOL_VERSION } from "@agentclientprotocol/sdk"; import type { AcpConfigState, AcpAgentDiagnostics, AcpDebugRequest, AcpDebugRunResult } from "@argos/shared/presenter"; @@ -51,8 +52,8 @@ type PendingAcpPermission = { * clients through the daemon `BunEventPublisher`. * * Sessions persist to the daemon's SQLite `acp_sessions` table (resume across - * daemon restarts). The daemon resolves agent runtimes from `$PATH` (no bundled - * runtime). + * daemon restarts). Agent runtimes (`npx`/`uvx`/`node`) resolve through the + * managed toolchain service, falling through to `$PATH` when unconfigured. */ export class AcpProviderExecutionPort implements ProviderExecutionPort { private runtimePromise: Promise | null = null; @@ -84,6 +85,7 @@ export class AcpProviderExecutionPort implements ProviderExecutionPort { private readonly deps: { dataDir: string; appVersion: string; + toolchains: ToolchainService; db: { prepare(sql: string): { get(...p: unknown[]): unknown; @@ -101,6 +103,7 @@ export class AcpProviderExecutionPort implements ProviderExecutionPort { dataDir: this.deps.dataDir, appVersion: this.deps.appVersion, eventPublisher: this.eventPublisher, + toolchains: this.deps.toolchains, }); const sessionPersistence = new AcpSessionPersistence(createDaemonAcpSqlitePresenter(this.deps.db), () => ports.paths.homeDir(), diff --git a/apps/daemon/src/host/acpPorts.ts b/apps/daemon/src/host/acpPorts.ts index 09173db7d..4d39e6d53 100644 --- a/apps/daemon/src/host/acpPorts.ts +++ b/apps/daemon/src/host/acpPorts.ts @@ -3,17 +3,19 @@ import path from "node:path"; import type { AcpHostPorts } from "@argos/acp-runtime"; import type { IEventPublisher } from "@argos/backend-core"; import { shouldRejectAcpTextRead, buildBinaryReadGuidance } from "./acpBinaryGuard"; - +import type { ToolchainService } from "./toolchains/service"; /** * Daemon implementation of the ACP host ports. Resolves paths from the OS and - * daemon data dir, uses a no-op runtime (agents resolve `npx`/`uvx`/`node` from - * `$PATH`), bridges events to the daemon `IEventPublisher`, and wires lifecycle - * to process signals. + * daemon data dir, resolves `npx`/`uvx`/`node`/`uv` through the managed + * toolchain service (falling through to `$PATH` when unconfigured), bridges + * events to the daemon `IEventPublisher`, and wires lifecycle to process + * signals. */ export function createDaemonAcpPorts(deps: { dataDir: string; appVersion: string; eventPublisher: IEventPublisher; + toolchains: ToolchainService; }): AcpHostPorts { return { paths: { @@ -23,10 +25,27 @@ export function createDaemonAcpPorts(deps: { appVersion: () => deps.appVersion, }, runtime: { - // v1 daemon ships no bundled runtime; agents use $PATH-resolved tools. expandPath: (target) => target, resolveCommand: (command) => command, - buildSpawnEnv: (base) => base, + resolveCommandWithArgs: async ({ command, args }) => { + const resolved = await deps.toolchains.resolveCommand(command, args); + if (resolved.command === command) { + return null; + } + return resolved; + }, + buildSpawnEnv: (base) => { + const dirs = deps.toolchains.binDirsSync(); + if (dirs.length === 0) { + return base; + } + const existingKey = Object.keys(base).find((key) => key.toLowerCase() === "path"); + const key = existingKey ?? (process.platform === "win32" ? "Path" : "PATH"); + return { + ...base, + [key]: [...dirs, base[key] ?? ""].filter(Boolean).join(path.delimiter), + }; + }, }, events: { broadcast: (name, payload) => deps.eventPublisher.publish(name, payload), diff --git a/apps/daemon/src/host/daemonMcpPorts.ts b/apps/daemon/src/host/daemonMcpPorts.ts index d3f23cac5..cea0a86cd 100644 --- a/apps/daemon/src/host/daemonMcpPorts.ts +++ b/apps/daemon/src/host/daemonMcpPorts.ts @@ -15,6 +15,7 @@ import { import { BuiltinKnowledgeServer } from "@argos/backend-core"; import type { IEventPublisher } from "@argos/backend-core"; import type { DaemonConfigPresenter } from "./daemonConfigPresenter"; +import type { ToolchainService } from "./toolchains/service"; import type { PluginToolPolicyDecision } from "@argos/shared/types/plugin"; /** Knowledge capabilities exposed by the daemon knowledge runtime. */ @@ -44,6 +45,7 @@ export function createDaemonMcpPorts(deps: { eventPublisher: IEventPublisher; configPresenter: DaemonConfigPresenter; configDir: string; + toolchains?: ToolchainService; knowledge?: DaemonKnowledgePort; db: { prepare(sql: string): { @@ -93,11 +95,12 @@ export function createDaemonMcpPorts(deps: { runtime: { initializeRuntimes: () => {}, expandPath: (target) => target, - processCommandWithArgs: (command, args) => ({ command, args }), + processCommandWithArgs: (command, args) => + deps.toolchains ? deps.toolchains.resolveCommandSync(command, args) : { command, args }, normalizePathEnv: (paths) => ({ key: "PATH", value: paths.join(":") }), - getDefaultPaths: () => [], + getDefaultPaths: () => deps.toolchains?.binDirsSync() ?? [], getBunRuntimePath: () => null, - getUvRuntimePath: () => null, + getUvRuntimePath: () => deps.toolchains?.binDirForToolSync("uv") ?? null, setBunRuntimePath: () => {}, setUvRuntimePath: () => {}, }, diff --git a/apps/daemon/src/host/toolchains/catalog.ts b/apps/daemon/src/host/toolchains/catalog.ts new file mode 100644 index 000000000..a36de708a --- /dev/null +++ b/apps/daemon/src/host/toolchains/catalog.ts @@ -0,0 +1,101 @@ +import type { ToolchainArchive } from "./types"; + +/** + * Managed-install catalog. Pins carry real SHA-256 digests captured from the + * official release artifacts: + * - Node from https://nodejs.org/dist//SHASUMS256.txt + * - uv from the GitHub release artifacts for the pin. + * + * uv archive filenames do not embed the version, so a pin bump without fresh + * hashes would pass compile-time checks and fail at first install — the + * catalog tests therefore assert every pin has complete non-empty hashes. + * ripgrep has no managed pin: the bundled seed plus system installs cover it. + */ + +export const NODE_PIN = "v24.18.0"; +export const UV_PIN = "0.9.18"; + +export const NODE_DIST_BASE = "https://nodejs.org/dist"; +export const UV_RELEASE_BASE = "https://github.com/astral-sh/uv/releases/download"; + +type PlatformKey = string; + +const NODE_ARCHIVES: Record> = { + [NODE_PIN]: { + "win32-x64": { + filename: `node-${NODE_PIN}-win-x64.zip`, + url: `${NODE_DIST_BASE}/${NODE_PIN}/node-${NODE_PIN}-win-x64.zip`, + sha256: "0ae68406b42d7725661da979b1403ec9926da205c6770827f33aac9d8f26e821", + }, + "win32-arm64": { + filename: `node-${NODE_PIN}-win-arm64.zip`, + url: `${NODE_DIST_BASE}/${NODE_PIN}/node-${NODE_PIN}-win-arm64.zip`, + sha256: "f274669adb93b1fd0fbf8f21fd078609e9dcc84333d4f2718d2dde3f9a161a01", + }, + "darwin-arm64": { + filename: `node-${NODE_PIN}-darwin-arm64.tar.gz`, + url: `${NODE_DIST_BASE}/${NODE_PIN}/node-${NODE_PIN}-darwin-arm64.tar.gz`, + sha256: "e1a97e14c99c803e96c7339403282ea05a499c32f8d83defe9ef5ec66f979ed1", + }, + "darwin-x64": { + filename: `node-${NODE_PIN}-darwin-x64.tar.gz`, + url: `${NODE_DIST_BASE}/${NODE_PIN}/node-${NODE_PIN}-darwin-x64.tar.gz`, + sha256: "dfd0dbd3e721503434df7b7205e719f61b3a3a31b2bcf9729b8b91fea240f080", + }, + "linux-x64": { + filename: `node-${NODE_PIN}-linux-x64.tar.gz`, + url: `${NODE_DIST_BASE}/${NODE_PIN}/node-${NODE_PIN}-linux-x64.tar.gz`, + sha256: "783130984963db7ba9cbd01089eaf2c2efb055c7c1693c943174b967b3050cb8", + }, + "linux-arm64": { + filename: `node-${NODE_PIN}-linux-arm64.tar.gz`, + url: `${NODE_DIST_BASE}/${NODE_PIN}/node-${NODE_PIN}-linux-arm64.tar.gz`, + sha256: "6b4484c2190274175df9aa8f28e2d758a819cb1c1fe6ab481e2f95b463ab8508", + }, + }, +}; + +const UV_ARCHIVES: Record> = { + [UV_PIN]: { + "win32-x64": { + filename: "uv-x86_64-pc-windows-msvc.zip", + url: `${UV_RELEASE_BASE}/${UV_PIN}/uv-x86_64-pc-windows-msvc.zip`, + sha256: "28cbe5d30907a774bfe27a517a39b494ec6f7d3816bda8bbf6f9645490449182", + }, + "win32-arm64": { + filename: "uv-aarch64-pc-windows-msvc.zip", + url: `${UV_RELEASE_BASE}/${UV_PIN}/uv-aarch64-pc-windows-msvc.zip`, + sha256: "fadb43ba13091f44e1786fc3967e65c7786d86192aa205d718307c649927cfc2", + }, + "darwin-arm64": { + filename: "uv-aarch64-apple-darwin.tar.gz", + url: `${UV_RELEASE_BASE}/${UV_PIN}/uv-aarch64-apple-darwin.tar.gz`, + sha256: "dc3bee4abbb3bac267a3985a23ea7617d19d41ff381dbaf560ba415ad65af68f", + }, + "linux-x64": { + filename: "uv-x86_64-unknown-linux-gnu.tar.gz", + url: `${UV_RELEASE_BASE}/${UV_PIN}/uv-x86_64-unknown-linux-gnu.tar.gz`, + sha256: "c2def3db178ade63933fa15ffc96e882c196ce53e06173dcee05b36c5f6f68f5", + }, + "linux-arm64": { + filename: "uv-aarch64-unknown-linux-gnu.tar.gz", + url: `${UV_RELEASE_BASE}/${UV_PIN}/uv-aarch64-unknown-linux-gnu.tar.gz`, + sha256: "f8e23ec786b18660ade6b033b6191b7e9c283c872eeb8c4531d56a873decf160", + }, + }, +}; + +function platformArchKey(): PlatformKey { + return `${process.platform}-${process.arch}`; +} + +/** Managed archive for the current platform, or null when the tool has no pin. */ +export function archiveFor(tool: "node" | "uv"): ToolchainArchive | null { + const key = platformArchKey(); + const table = tool === "node" ? NODE_ARCHIVES[NODE_PIN] : UV_ARCHIVES[UV_PIN]; + return table?.[key] ?? null; +} + +export function pinFor(tool: "node" | "uv"): string { + return tool === "node" ? NODE_PIN : UV_PIN; +} diff --git a/apps/daemon/src/host/toolchains/install.ts b/apps/daemon/src/host/toolchains/install.ts new file mode 100644 index 000000000..c92dfc3fa --- /dev/null +++ b/apps/daemon/src/host/toolchains/install.ts @@ -0,0 +1,188 @@ +import { createHash } from "node:crypto"; +import { existsSync, mkdirSync, readdirSync, renameSync, rmSync, statSync } from "node:fs"; +import path from "node:path"; +import { archiveFor, pinFor } from "./catalog"; +import type { ToolchainArchive, ToolchainName } from "./types"; + +/** + * Managed install pipeline: fetch -> verify sha256 -> extract to staging -> + * atomic rename to tools//. The previous tree is rotated to + * `.prev` (never deleted while it may still be running — Windows locks it + * with EBUSY/EPERM). A failed or cancelled install leaves the previous tree + * active. Cancel is a cooperative flag checked between phases. + */ + +export class ToolchainInstallError extends Error { + readonly code: string; + constructor(message: string, code = "install_failed") { + super(message); + this.name = "ToolchainInstallError"; + this.code = code; + } +} + +export interface InstallContext { + dataDir: string; + fetchImpl: typeof fetch; + /** Cooperative cancel flag, checked between phases. */ + cancelled: () => boolean; + /** Injectable extractor (defaults to `tar -xf`). */ + extract?: (archivePath: string, destinationDir: string) => Promise; + /** Injectable archive (tests); defaults to the catalog pin. */ + archive?: ToolchainArchive; +} + +function ensureCancel(ctx: InstallContext, phase: string): void { + if (ctx.cancelled()) { + throw new ToolchainInstallError(`Install cancelled during ${phase}`, "cancelled"); + } +} + +function classifyFsError(error: unknown): string { + const code = (error as NodeJS.ErrnoException)?.code ?? ""; + if (code === "EPERM" || code === "EBUSY") { + return "disk"; + } + return "install_failed"; +} + +async function defaultExtract(archivePath: string, destinationDir: string): Promise { + const proc = Bun.spawn(["tar", "-xf", archivePath, "-C", destinationDir], { + stdout: "pipe", + stderr: "pipe", + }); + const exitCode = await proc.exited; + if (exitCode !== 0) { + const stderr = await new Response(proc.stderr).text(); + throw new ToolchainInstallError(`Archive extraction failed (tar exit ${exitCode}): ${stderr.slice(0, 400)}`); + } +} + +/** Extract, then collapse a single top-level directory into `destinationDir`. */ +async function extractAndFlatten( + archivePath: string, + destinationDir: string, + extract: (archivePath: string, destinationDir: string) => Promise, +): Promise { + const staging = `${destinationDir}.staging-${Date.now()}`; + mkdirSync(staging, { recursive: true }); + try { + await extract(archivePath, staging); + const topLevel = readdirSync(staging).filter((entry) => !entry.startsWith(".")); + if (topLevel.length === 1 && statSync(path.join(staging, topLevel[0]!)).isDirectory()) { + // node-v24.18.0-win-x64/... or uv-x86_64-pc-windows-msvc/... + renameSync(path.join(staging, topLevel[0]!), destinationDir); + } else { + renameSync(staging, destinationDir); + } + } finally { + // Remove the staging dir when the tree moved out of it; keep it (with its + // partial contents) when extraction failed so the archive can be inspected. + try { + if (readdirSync(staging).length === 0) { + rmSync(staging, { recursive: true, force: true }); + } + } catch { + // best-effort + } + } +} + +export async function installToolchain(tool: "node" | "uv", ctx: InstallContext): Promise { + const archive = ctx.archive ?? archiveFor(tool); + if (!archive) { + throw new ToolchainInstallError(`No managed archive is catalogued for ${tool} on this platform`, "no_archive"); + } + const pin = pinFor(tool); + const baseDir = path.join(ctx.dataDir, "toolchains"); + const toolsDir = path.join(baseDir, "tools", tool); + const versionDir = path.join(toolsDir, pin); + const downloadsDir = path.join(baseDir, "downloads"); + + mkdirSync(downloadsDir, { recursive: true }); + mkdirSync(toolsDir, { recursive: true }); + ensureCancel(ctx, "download"); + + // Download + verify. + const archivePath = path.join(downloadsDir, archive.filename); + let response: Response; + try { + response = await ctx.fetchImpl(archive.url); + } catch (error) { + throw new ToolchainInstallError( + `Download failed: ${error instanceof Error ? error.message : String(error)}`, + "network", + ); + } + if (!response.ok) { + throw new ToolchainInstallError(`Download failed: HTTP ${response.status} for ${archive.url}`, "network"); + } + const bytes = new Uint8Array(await response.arrayBuffer()); + ensureCancel(ctx, "download"); + const digest = createHash("sha256").update(bytes).digest("hex"); + if (digest !== archive.sha256) { + throw new ToolchainInstallError( + `Checksum mismatch for ${archive.filename}: expected ${archive.sha256}, got ${digest}`, + "checksum_mismatch", + ); + } + await Bun.write(archivePath, bytes); + ensureCancel(ctx, "extract"); + + // Stage the new tree outside the active path. + const stagingTarget = `${versionDir}.incoming`; + if (existsSync(stagingTarget)) { + renameSync(stagingTarget, `${stagingTarget}.old-${Date.now()}`); + } + try { + await extractAndFlatten(archivePath, stagingTarget, ctx.extract ?? defaultExtract); + ensureCancel(ctx, "activating"); + } catch (error) { + if (error instanceof ToolchainInstallError && error.code === "cancelled") { + throw error; + } + throw new ToolchainInstallError( + error instanceof Error ? error.message : String(error), + error instanceof ToolchainInstallError ? error.code : classifyFsError(error), + ); + } + + // Activate atomically: rotate the previous tree, then rename staging in. + if (existsSync(versionDir)) { + const prevPath = `${versionDir}.prev`; + if (existsSync(prevPath)) { + try { + rmSync(prevPath, { recursive: true, force: true }); + } catch { + // Windows keeps the old tree busy; archive it instead of deleting. + try { + renameSync(prevPath, `${prevPath}-${Date.now()}`); + } catch { + // Leave it; the rename below will fail with a clear error if truly locked. + } + } + } + renameSync(versionDir, prevPath); + } + try { + renameSync(stagingTarget, versionDir); + } catch (error) { + // Roll the previous tree back so the active install is never missing. + const prevPath = `${versionDir}.prev`; + if (existsSync(prevPath)) { + renameSync(prevPath, versionDir); + } + throw new ToolchainInstallError( + `Activation failed: ${error instanceof Error ? error.message : String(error)}`, + classifyFsError(error), + ); + } + // Archive the partial download no longer needed. + try { + rmSync(archivePath, { force: true }); + } catch { + // best-effort + } +} + +export { pinFor }; diff --git a/apps/daemon/src/host/toolchains/locate.ts b/apps/daemon/src/host/toolchains/locate.ts new file mode 100644 index 000000000..456e797a9 --- /dev/null +++ b/apps/daemon/src/host/toolchains/locate.ts @@ -0,0 +1,159 @@ +import { existsSync } from "node:fs"; +import path from "node:path"; +import type { ResolvedToolchain, ToolchainName, ToolchainSource } from "./types"; + +/** + * Source resolution precedence: + * explicit custom -> explicit unconfigured -> managed -> bundled -> system -> unconfigured. + * `managed`/`bundled`/`system` are derived on demand and never persisted. + */ + +const EXE = (name: string): string => (process.platform === "win32" ? `${name}.exe` : name); + +const TOOL_BINARIES: Record = { + node: ["node"], + uv: ["uv"], + ripgrep: ["rg"], +}; + +/** Candidate roots for the app-shipped runtime seed. */ +export function bundledRoots(dataDir: string): string[] { + const execDir = path.dirname(process.execPath); + const cwd = process.cwd(); + const roots = [path.join(execDir, "..", "runtime"), path.join(execDir, "runtime"), path.join(cwd, "runtime")]; + // The desktop sidecar also seeds the daemon data dir in dev. + if (dataDir && dataDir !== cwd) { + roots.push(path.join(dataDir, "runtime")); + } + return [...new Set(roots.map((root) => path.resolve(root)))]; +} + +function findFileIn(dirs: string[], relative: string[]): string | null { + for (const dir of dirs) { + const candidate = path.join(dir, ...relative); + if (existsSync(candidate)) { + return candidate; + } + } + return null; +} + +/** Locate the bundled seed binary for a tool, or null. */ +export function bundledToolPath(tool: ToolchainName, dataDir: string): string | null { + if (tool === "node") { + // Argos never bundles Node (the daemon itself is a Bun binary). + return null; + } + const dirByTool: Record, string> = { + uv: "uv", + ripgrep: "ripgrep", + }; + const root = findFileIn( + bundledRoots(dataDir), + dirByTool[tool as Exclude] ? [dirByTool[tool as Exclude]] : [], + ); + if (!root) { + return null; + } + return findFileIn( + [root], + TOOL_BINARIES[tool].map((name) => EXE(name)), + ); +} + +/** Default well-known install dirs merged with PATH dirs for system detection. */ +export function systemSearchDirs(env: NodeJS.ProcessEnv): string[] { + const home = env.USERPROFILE ?? env.HOME ?? ""; + const programFiles = env.ProgramFiles ?? (process.platform === "win32" ? "C:\\Program Files" : ""); + const localAppData = env.LOCALAPPDATA ?? ""; + const dirs: string[] = []; + for (const entry of (env.PATH ?? "").split(path.delimiter)) { + if (entry.trim()) { + dirs.push(entry.trim()); + } + } + if (process.platform === "win32") { + if (home) { + dirs.push( + path.join(home, "AppData", "Roaming", "nvm"), + path.join(home, "scoop", "shims"), + path.join(home, ".cargo", "bin"), + ); + } + if (programFiles) { + dirs.push(path.join(programFiles, "nodejs")); + } + if (localAppData) { + dirs.push(path.join(localAppData, "Programs", "uv")); + } + } else { + dirs.push( + "/usr/local/bin", + "/opt/homebrew/bin", + "/usr/bin", + path.join(home, ".local", "bin"), + path.join(home, ".cargo", "bin"), + ); + if (home) { + dirs.push(path.join(home, ".nvm", "versions"), path.join(home, ".volta", "bin"), path.join(home, ".bun", "bin")); + } + } + return [...new Set(dirs)]; +} + +/** Locate a system-installed binary for a tool, or null. */ +export function systemToolPath(tool: ToolchainName, env: NodeJS.ProcessEnv): string | null { + return findFileIn( + systemSearchDirs(env), + TOOL_BINARIES[tool].map((name) => EXE(name)), + ); +} + +export interface DerivedResolveOptions { + dataDir: string; + env: NodeJS.ProcessEnv; + /** Resolved path of the managed tree, when present. */ + managedPath?: string | null; + /** Version probe override (tests). */ + probeVersion?: (binaryPath: string) => Promise; +} + +/** + * Derive a non-explicit source: managed -> bundled -> system -> unconfigured. + * Standalone so hosts and tests can run it against an isolated dataDir/env. + */ +export async function resolveDerivedToolchain( + tool: ToolchainName, + opts: DerivedResolveOptions, +): Promise { + const probe = opts.probeVersion ?? (async () => null); + + if (tool === "node" || tool === "uv") { + if (opts.managedPath && existsSync(opts.managedPath)) { + return { + source: "managed", + explicit: false, + path: opts.managedPath, + version: await probe(opts.managedPath), + error: null, + }; + } + } + + const bundled = bundledToolPath(tool, opts.dataDir); + if (bundled) { + return { source: "bundled", explicit: false, path: bundled, version: await probe(bundled), error: null }; + } + + const system = systemToolPath(tool, opts.env); + if (system) { + return { source: "system", explicit: false, path: system, version: await probe(system), error: null }; + } + + return { source: "unconfigured", explicit: false, path: null, version: null, error: null }; +} + +/** Directory that must go on PATH for a resolved binary to be usable. */ +export function binDirFor(binaryPath: string): string { + return path.dirname(binaryPath); +} diff --git a/apps/daemon/src/host/toolchains/service.ts b/apps/daemon/src/host/toolchains/service.ts new file mode 100644 index 000000000..b7aadfd55 --- /dev/null +++ b/apps/daemon/src/host/toolchains/service.ts @@ -0,0 +1,358 @@ +import { existsSync } from "node:fs"; +import path from "node:path"; +import type { ToolchainName, ToolchainSource, ToolchainStatus } from "@argos/shared-contracts/routes"; +import { NODE_PIN, UV_PIN, pinFor } from "./catalog"; +import { installToolchain, ToolchainInstallError } from "./install"; +import { binDirFor, bundledToolPath, resolveDerivedToolchain, systemToolPath } from "./locate"; +import { loadState, saveState } from "./state"; +import type { InstallProgress, ResolvedToolchain, ToolchainSourceEntry, ToolchainStateFile } from "./types"; + +/** + * One resolver for external runtimes. Consumers (ACP launch, MCP stdio) must + * resolve node/uv/ripgrep only through this service. Explicit user choices + * (custom path, explicit unconfigured) persist; derived sources are computed + * on demand. See docs/features/managed-toolchains. + */ + +export interface ToolchainServiceDeps { + dataDir: string; + env?: NodeJS.ProcessEnv; + fetchImpl?: typeof fetch; + /** Injectable version probe for tests. */ + probeVersion?: (binaryPath: string) => Promise; + now?: () => number; +} + +const DEFAULT_ENV: NodeJS.ProcessEnv = process.env; +const MANAGED_TOOLS = ["node", "uv"] as const; +type ManagedTool = (typeof MANAGED_TOOLS)[number]; + +export class ToolchainService { + private readonly dataDir: string; + private readonly env: NodeJS.ProcessEnv; + private readonly fetchImpl: typeof fetch; + private readonly probeVersionImpl: (binaryPath: string) => Promise; + private readonly now: () => number; + private state: ToolchainStateFile | null = null; + private versionCache = new Map(); + private syncCache = new Map(); + private installJobs = new Map(); + private cancelFlags = new Map(); + constructor(deps: ToolchainServiceDeps) { + this.dataDir = deps.dataDir; + this.env = deps.env ?? DEFAULT_ENV; + this.fetchImpl = deps.fetchImpl ?? fetch; + this.probeVersionImpl = + deps.probeVersion ?? + (async (binaryPath) => { + try { + const proc = Bun.spawn([binaryPath, "--version"], { stdout: "pipe", stderr: "pipe" }); + const timer = setTimeout(() => proc.kill(), 5000); + const exitCode = await proc.exited; + clearTimeout(timer); + if (exitCode !== 0) return null; + const text = await new Response(proc.stdout).text(); + return text.trim().split(/\s+/).pop() || null; + } catch { + return null; + } + }); + this.now = deps.now ?? Date.now; + } + + private async withState(): Promise { + if (!this.state) { + this.state = await loadState(this.dataDir); + } + return this.state; + } + + private async persist(state: ToolchainStateFile): Promise { + this.state = state; + await saveState(this.dataDir, state); + } + + private async probeVersion(binaryPath: string): Promise { + if (this.versionCache.has(binaryPath)) { + return this.versionCache.get(binaryPath) ?? null; + } + const version = await this.probeVersionImpl(binaryPath); + this.versionCache.set(binaryPath, version); + return version; + } + + private managedToolPath(tool: ManagedTool): string | null { + const pin = pinFor(tool); + const binary = tool === "node" ? "node" : "uv"; + const candidate = path.join( + this.dataDir, + "toolchains", + "tools", + tool, + pin, + process.platform === "win32" ? `${binary}.exe` : tool === "node" ? path.join("bin", binary) : binary, + ); + return existsSync(candidate) ? candidate : null; + } + + async resolve(tool: ToolchainName): Promise { + const state = await this.withState(); + const entry: ToolchainSourceEntry | undefined = state.sources[tool]; + + let resolved: ResolvedToolchain; + if (entry?.source === "custom") { + const version = entry.path ? await this.probeVersion(entry.path).catch(() => null) : null; + resolved = { + source: "custom", + explicit: true, + path: entry.path ?? null, + version: version ?? null, + error: entry.path && !existsSync(entry.path) ? "Configured path does not exist" : null, + }; + } else if (entry?.source === "unconfigured") { + resolved = { source: "unconfigured", explicit: true, path: null, version: null, error: null }; + } else { + resolved = await this.resolveDerived(tool); + } + + this.syncCache.set(tool, resolved); + return resolved; + } + + private async resolveDerived(tool: ToolchainName): Promise { + return await resolveDerivedToolchain(tool, { + dataDir: this.dataDir, + env: this.env, + managedPath: tool === "node" || tool === "uv" ? this.managedToolPath(tool) : null, + probeVersion: (binaryPath) => this.probeVersion(binaryPath), + }); + } + + /** Warm the synchronous cache (call at daemon startup). */ + async warmup(): Promise { + for (const tool of ["node", "uv", "ripgrep"] as ToolchainName[]) { + await this.resolve(tool); + } + } + + private cached(tool: ToolchainName): ResolvedToolchain { + return ( + this.syncCache.get(tool) ?? { source: "unconfigured", explicit: false, path: null, version: null, error: null } + ); + } + + async status(tool: ToolchainName): Promise { + const install = this.installJobs.get(tool as ManagedTool) ?? null; + const installError = this.installErrors.get(tool as ManagedTool) ?? null; + const resolved = await this.resolve(tool); + return { + tool, + source: resolved.source, + explicit: resolved.explicit, + path: resolved.path, + version: resolved.version, + error: installError ?? resolved.error, + pin: tool === "node" ? NODE_PIN : tool === "uv" ? UV_PIN : null, + install, + }; + } + + async list(): Promise { + return Promise.all((["node", "uv", "ripgrep"] as ToolchainName[]).map((tool) => this.status(tool))); + } + + async setSource( + tool: ToolchainName, + source: "custom" | "unconfigured", + customPath?: string, + ): Promise { + const state = await this.withState(); + if (source === "custom") { + if (!customPath || !existsSync(customPath)) { + throw new Error(`Custom toolchain path does not exist: ${customPath}`); + } + state.sources[tool] = { source: "custom", explicit: true, path: customPath }; + } else { + state.sources[tool] = { source: "unconfigured", explicit: true }; + } + this.versionCache.clear(); + await this.persist(state); + return this.status(tool); + } + + async removeSource(tool: ToolchainName): Promise { + const state = await this.withState(); + delete state.sources[tool]; + this.versionCache.clear(); + await this.persist(state); + return this.status(tool); + } + + install(tool: "node" | "uv"): { started: boolean } { + if (this.installJobs.has(tool)) { + return { started: false }; + } + const progress: InstallProgress = { phase: "downloading", tool, version: pinFor(tool), startedAt: this.now() }; + this.installJobs.set(tool, progress); + this.cancelFlags.set(tool, false); + void this.runInstall(tool); + return { started: true }; + } + + private async runInstall(tool: ManagedTool): Promise { + const advance = (phase: InstallProgress["phase"]) => { + const job = this.installJobs.get(tool); + if (job) { + this.installJobs.set(tool, { ...job, phase }); + } + }; + try { + await installToolchain(tool, { + dataDir: this.dataDir, + fetchImpl: this.fetchImpl, + cancelled: () => this.cancelFlags.get(tool) === true, + extract: async (archivePath, destinationDir) => { + advance("extracting"); + const proc = Bun.spawn(["tar", "-xf", archivePath, "-C", destinationDir], { + stdout: "pipe", + stderr: "pipe", + }); + const exitCode = await proc.exited; + if (exitCode !== 0) { + const stderr = await new Response(proc.stderr).text(); + throw new ToolchainInstallError( + `Archive extraction failed (tar exit ${exitCode}): ${stderr.slice(0, 400)}`, + ); + } + }, + }); + // Inject an activating tick so clients observe the final phase. + advance("activating"); + this.versionCache.clear(); + } catch (error) { + const job = this.installJobs.get(tool); + if (job) { + const message = error instanceof Error ? error.message : String(error); + this.installJobs.set(tool, { ...job, phase: "idle" }); + // Surface the failure on the next status poll via a synthetic error map. + this.installErrors.set(tool, message); + } + } + // The job record stays until the next `status()`/`install()` observes the + // terminal state; keep it for one poll so the UI sees completion. + setTimeout(() => { + this.installJobs.delete(tool); + this.installErrors.delete(tool); + this.cancelFlags.delete(tool); + }, 2500); + } + + private installErrors = new Map(); + + cancelInstall(tool: "node" | "uv"): void { + this.cancelFlags.set(tool, true); + } + + /** + * Rewrite a spawn command through the resolved toolchains. Unresolvable + * commands return unchanged so PATH lookup (and its error) still applies. + */ + async resolveCommand(command: string, args: string[]): Promise<{ command: string; args: string[] }> { + if (command === "node" || command === "npm" || command === "npx") { + const node = await this.resolve("node"); + if (!node.path) { + return { command, args }; + } + if (command === "node") { + return { command: node.path, args }; + } + const cliRelative = process.platform === "win32" ? "node_modules/npm/bin" : "../lib/node_modules/npm/bin"; + const cli = path.join(binDirFor(node.path), cliRelative, command === "npx" ? "npx-cli.js" : "npm-cli.js"); + if (!existsSync(cli)) { + return { command, args }; + } + return { command: node.path, args: [cli, ...args] }; + } + if (command === "uv" || command === "uvx") { + const uv = await this.resolve("uv"); + if (!uv.path) { + return { command, args }; + } + if (command === "uv") { + return { command: uv.path, args }; + } + const uvxName = process.platform === "win32" ? "uvx.exe" : "uvx"; + const uvx = path.join(binDirFor(uv.path), uvxName); + return { command: existsSync(uvx) ? uvx : uv.path, args }; + } + return { command, args }; + } + + /** + * Synchronous variant backed by the warm cache. Serves sync host seams + * (MCP `processCommandWithArgs`); callers should have run `warmup()` once + * at startup — cache misses resolve to the input unchanged. + */ + resolveCommandSync(command: string, args: string[]): { command: string; args: string[] } { + if (command === "node" || command === "npm" || command === "npx") { + const node = this.cached("node"); + if (!node.path) { + return { command, args }; + } + if (command === "node") { + return { command: node.path, args }; + } + const cliRelative = process.platform === "win32" ? "node_modules/npm/bin" : "../lib/node_modules/npm/bin"; + const cli = path.join(binDirFor(node.path), cliRelative, command === "npx" ? "npx-cli.js" : "npm-cli.js"); + if (!existsSync(cli)) { + return { command, args }; + } + return { command: node.path, args: [cli, ...args] }; + } + if (command === "uv" || command === "uvx") { + const uv = this.cached("uv"); + if (!uv.path) { + return { command, args }; + } + if (command === "uv") { + return { command: uv.path, args }; + } + const uvxName = process.platform === "win32" ? "uvx.exe" : "uvx"; + const uvx = path.join(binDirFor(uv.path), uvxName); + return { command: existsSync(uvx) ? uvx : uv.path, args }; + } + return { command, args }; + } + + /** Bin dirs that should be prepended to PATH for spawned consumers. */ + async binDirs(): Promise { + const dirs: string[] = []; + for (const tool of ["node", "uv", "ripgrep"] as ToolchainName[]) { + const resolved = await this.resolve(tool); + if (resolved.path) { + dirs.push(binDirFor(resolved.path)); + } + } + return [...new Set(dirs)]; + } + + /** Synchronous `binDirs` backed by the warm cache. */ + binDirsSync(): string[] { + const dirs: string[] = []; + for (const tool of ["node", "uv", "ripgrep"] as ToolchainName[]) { + const resolved = this.cached(tool); + if (resolved.path) { + dirs.push(binDirFor(resolved.path)); + } + } + return [...new Set(dirs)]; + } + + /** Synchronous bin dir of one tool, or null when unresolved. */ + binDirForToolSync(tool: ToolchainName): string | null { + const resolved = this.cached(tool); + return resolved.path ? binDirFor(resolved.path) : null; + } +} + +export type { ToolchainSource }; diff --git a/apps/daemon/src/host/toolchains/state.ts b/apps/daemon/src/host/toolchains/state.ts new file mode 100644 index 000000000..f9a677615 --- /dev/null +++ b/apps/daemon/src/host/toolchains/state.ts @@ -0,0 +1,70 @@ +import { existsSync } from "node:fs"; +import { join } from "node:path"; +import type { ToolchainName, ToolchainStateFile } from "./types"; + +/** + * Persists only explicit user choices (custom path / explicit unconfigured). + * Derived sources (managed/bundled/system) are recomputed on demand so a PATH + * refresh or a removed bundled seed cannot leave a stale pointer behind. + * Corrupt state is quarantined under a timestamped name — a fixed + * `state.json.corrupt` would throw EEXIST on the second corruption. + */ + +const STATE_VERSION = 1 as const; + +export function toolchainsDir(dataDir: string): string { + return join(dataDir, "toolchains"); +} + +function stateFilePath(dataDir: string): string { + return join(toolchainsDir(dataDir), "state.json"); +} + +export function emptyState(): ToolchainStateFile { + return { version: STATE_VERSION, sources: {} }; +} + +export async function loadState(dataDir: string): Promise { + const filePath = stateFilePath(dataDir); + if (!existsSync(filePath)) { + return emptyState(); + } + try { + const raw = await Bun.file(filePath).text(); + const parsed = JSON.parse(raw) as Partial | null; + if (!parsed || typeof parsed !== "object" || parsed.version !== STATE_VERSION) { + throw new Error("unsupported state version"); + } + const sources = parsed.sources ?? {}; + const clean: ToolchainStateFile["sources"] = {}; + for (const [tool, entry] of Object.entries(sources)) { + if (!entry || (entry.source !== "custom" && entry.source !== "unconfigured")) { + continue; + } + if (entry.source === "custom" && typeof entry.path !== "string") { + continue; + } + clean[tool as ToolchainName] = { source: entry.source, explicit: true, path: entry.path }; + } + return { version: STATE_VERSION, sources: clean }; + } catch { + // Quarantine under a timestamped name so repeated corruption cannot make + // every subsequent load throw (EEXIST on a fixed quarantine name). + try { + await Bun.write( + join(toolchainsDir(dataDir), `state.corrupt-${Date.now()}.json`), + await Bun.file(filePath).arrayBuffer(), + ); + await Bun.write(filePath, ""); // reset so the next load succeeds + } catch { + // best-effort quarantine; a fresh state is returned regardless + } + return emptyState(); + } +} + +export async function saveState(dataDir: string, state: ToolchainStateFile): Promise { + const dir = toolchainsDir(dataDir); + await Bun.write(join(dir, ".keep"), ""); + await Bun.write(stateFilePath(dataDir), JSON.stringify(state, null, 2)); +} diff --git a/apps/daemon/src/host/toolchains/types.ts b/apps/daemon/src/host/toolchains/types.ts new file mode 100644 index 000000000..f5a0776c6 --- /dev/null +++ b/apps/daemon/src/host/toolchains/types.ts @@ -0,0 +1,37 @@ +import type { ToolchainName, ToolchainSource } from "@argos/shared-contracts/routes"; + +export type { ToolchainName, ToolchainSource }; + +/** A derived or explicit resolution result for one tool. */ +export interface ResolvedToolchain { + source: ToolchainSource; + explicit: boolean; + path: string | null; + version: string | null; + error: string | null; +} + +/** Persisted explicit user choice (custom path / explicit unconfigured). */ +export interface ToolchainSourceEntry { + source: "custom" | "unconfigured"; + explicit: true; + path?: string; +} + +export interface ToolchainStateFile { + version: 1; + sources: Partial>; +} + +export interface InstallProgress { + phase: "idle" | "downloading" | "extracting" | "activating"; + tool: ToolchainName; + version: string; + startedAt: number; +} + +export interface ToolchainArchive { + filename: string; + url: string; + sha256: string; +} diff --git a/apps/daemon/src/index.ts b/apps/daemon/src/index.ts index 9e7a7d4e0..7027f354d 100644 --- a/apps/daemon/src/index.ts +++ b/apps/daemon/src/index.ts @@ -13,6 +13,7 @@ import { DaemonArgosAgentRuntime } from "./host/daemonArgosAgentRuntime"; import { BunEventPublisher } from "./host/bun-event-publisher"; import { initializeDatabase } from "./host/db-init"; import { createDaemonDispatcher } from "./dispatch/daemonDispatcher"; +import { ToolchainService } from "./host/toolchains/service"; import { DaemonWorkspacePresenter } from "./workspace/daemonWorkspacePresenter"; import { DaemonTerminalRuntime } from "./terminal/daemonTerminalRuntime"; import { ProviderImportService } from "@argos/backend-core"; @@ -330,6 +331,14 @@ export async function startDaemon(options?: { const piProfiles = new PiAgentProfileManager(paths.getDataDir(), resolveDaemonVersion()); const agentWorkspaceDir = pathJoin(paths.getDataDir(), "agent-workspace"); + + // One resolver for external runtimes; consumers (ACP launch, MCP stdio) + // must go through it. Warmed once so the sync seams have data. + const toolchainService = new ToolchainService({ dataDir: paths.getDataDir() }); + void toolchainService.warmup().catch((error) => { + logger.warn("[daemon] toolchain warmup failed:", error); + }); + const piProviderExecutionPort = new PiProviderExecutionPort( configPresenter, sessionRepository, @@ -377,6 +386,7 @@ export async function startDaemon(options?: { dataDir: paths.getDataDir(), appVersion: resolveDaemonVersion(), db, + toolchains: toolchainService, }); // Route execution by session provider: ACP-backed sessions go to the ACP port, @@ -506,6 +516,7 @@ export async function startDaemon(options?: { eventPublisher, configPresenter, configDir: paths.getConfigDir(), + toolchains: toolchainService, knowledge: knowledgeRuntime.runtime, sessionRepository, db, @@ -835,6 +846,7 @@ export async function startDaemon(options?: { workspacePresenter, knowledgeRuntime.runtime, terminalRuntime, + toolchainService, ); setRouteDispatcher(dispatcher); diff --git a/apps/daemon/test/toolchainsRoutes.test.ts b/apps/daemon/test/toolchainsRoutes.test.ts new file mode 100644 index 000000000..e4c25b4db --- /dev/null +++ b/apps/daemon/test/toolchainsRoutes.test.ts @@ -0,0 +1,164 @@ +import { describe, expect, it, vi } from "bun:test"; +import { createDaemonDispatcher } from "../src/dispatch/daemonDispatcher"; + +/** Route surface for the managed toolchains service. */ + +const createToolchainsStub = () => ({ + list: vi.fn(async () => [ + { + tool: "node", + source: "managed", + explicit: false, + path: "/tools/node/v24.18.0/node.exe", + version: "v24.18.0", + error: null, + pin: "v24.18.0", + install: null, + }, + { + tool: "uv", + source: "unconfigured", + explicit: false, + path: null, + version: null, + error: null, + pin: "0.9.18", + install: null, + }, + { + tool: "ripgrep", + source: "unconfigured", + explicit: false, + path: null, + version: null, + error: null, + pin: null, + install: null, + }, + ]), + status: vi.fn(async (tool: string) => ({ + tool, + source: "unconfigured", + explicit: false, + path: null, + version: null, + error: null, + pin: tool === "ripgrep" ? null : "x", + install: null, + })), + setSource: vi.fn(async (tool: string, source: string, customPath?: string) => ({ + tool, + source, + explicit: true, + path: customPath ?? null, + version: null, + error: null, + pin: null, + install: null, + })), + removeSource: vi.fn(async (tool: string) => ({ + tool, + source: "unconfigured", + explicit: false, + path: null, + version: null, + error: null, + pin: null, + install: null, + })), + install: vi.fn((_tool: string) => ({ started: true })), + cancelInstall: vi.fn((_tool: string) => {}), +}); + +const createDispatcher = (toolchains?: ReturnType) => + createDaemonDispatcher( + { + getDefaultModel: vi.fn(() => ({ providerId: "provider-1", modelId: "model-1" })), + } as any, + { publish: vi.fn() } as any, + {} as any, + {} as any, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + "unknown", + undefined, + undefined, + undefined, + undefined, + toolchains as any, + ); + +describe("toolchains routes", () => { + it("lists toolchain status", async () => { + const toolchains = createToolchainsStub(); + const dispatcher = createDispatcher(toolchains); + + await expect(dispatcher("toolchains.list", {})).resolves.toEqual({ + tools: await toolchains.list(), + }); + expect(toolchains.list).toHaveBeenCalled(); + }); + + it("sets and removes explicit sources", async () => { + const toolchains = createToolchainsStub(); + const dispatcher = createDispatcher(toolchains); + + await expect( + dispatcher("toolchains.setSource", { tool: "node", source: "custom", path: "/opt/node/node" }), + ).resolves.toEqual({ + status: { + tool: "node", + source: "custom", + explicit: true, + path: "/opt/node/node", + version: null, + error: null, + pin: null, + install: null, + }, + }); + expect(toolchains.setSource).toHaveBeenCalledWith("node", "custom", "/opt/node/node"); + + await expect(dispatcher("toolchains.removeSource", { tool: "uv" })).resolves.toEqual({ + status: { + tool: "uv", + source: "unconfigured", + explicit: false, + path: null, + version: null, + error: null, + pin: null, + install: null, + }, + }); + }); + + it("rejects a custom source without a path", async () => { + const dispatcher = createDispatcher(createToolchainsStub()); + await expect(dispatcher("toolchains.setSource", { tool: "node", source: "custom" })).rejects.toThrow(); + }); + + it("starts and cancels installs", async () => { + const toolchains = createToolchainsStub(); + const dispatcher = createDispatcher(toolchains); + + await expect(dispatcher("toolchains.install", { tool: "uv" })).resolves.toMatchObject({ started: true }); + expect(toolchains.install).toHaveBeenCalledWith("uv"); + + await expect(dispatcher("toolchains.cancelInstall", { tool: "uv" })).resolves.toMatchObject({ cancelled: true }); + expect(toolchains.cancelInstall).toHaveBeenCalledWith("uv"); + }); + + it("throws a clear error when the service is unavailable", async () => { + const dispatcher = createDispatcher(undefined); + await expect(dispatcher("toolchains.list", {})).rejects.toThrow("Toolchain service is not available"); + }); +}); diff --git a/apps/daemon/test/toolchainsService.test.ts b/apps/daemon/test/toolchainsService.test.ts new file mode 100644 index 000000000..96465daf5 --- /dev/null +++ b/apps/daemon/test/toolchainsService.test.ts @@ -0,0 +1,282 @@ +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "bun:test"; +import { ToolchainService } from "../src/host/toolchains/service"; +import { installToolchain, ToolchainInstallError } from "../src/host/toolchains/install"; +import { pinFor } from "../src/host/toolchains/catalog"; +import { systemToolPath } from "../src/host/toolchains/locate"; + +/** + * Hermetic coverage for the toolchain resolver, command rewrites, and the + * managed-install pipeline. All filesystem fixtures live under temp dirs; the + * version probe is injected so no real binary is spawned. + */ + +const EMPTY_ENV = { PATH: "", HOME: "", USERPROFILE: "", ProgramFiles: "", LOCALAPPDATA: "" } as NodeJS.ProcessEnv; +const nodeBin = () => (process.platform === "win32" ? "node.exe" : path.join("bin", "node")); +const uvBin = () => (process.platform === "win32" ? "uv.exe" : "uv"); + +describe("toolchain service", () => { + const roots: string[] = []; + + afterEach(() => { + while (roots.length > 0) { + const root = roots.pop(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + } + }); + + const tempRoot = (): string => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argos-toolchains-svc-")); + roots.push(dir); + return dir; + }; + + const createManagedNode = (dataDir: string): string => { + const nodePath = path.join(dataDir, "toolchains", "tools", "node", pinFor("node"), nodeBin()); + fs.mkdirSync(path.dirname(nodePath), { recursive: true }); + fs.writeFileSync(nodePath, "binary"); + return nodePath; + }; + + const createManagedUv = (dataDir: string): string => { + const uvPath = path.join(dataDir, "toolchains", "tools", "uv", pinFor("uv"), uvBin()); + fs.mkdirSync(path.dirname(uvPath), { recursive: true }); + fs.writeFileSync(uvPath, "binary"); + return uvPath; + }; + + it("resolves a managed tree ahead of system lookups", async () => { + const dataDir = tempRoot(); + const nodePath = createManagedNode(dataDir); + const service = new ToolchainService({ + dataDir, + env: EMPTY_ENV, + probeVersion: async () => pinFor("node"), + }); + + const resolved = await service.resolve("node"); + expect(resolved.source).toBe("managed"); + expect(resolved.explicit).toBe(false); + expect(resolved.path).toBe(nodePath); + expect(resolved.version).toBe(pinFor("node")); + }); + + it("prefers an explicit custom path over the managed tree", async () => { + const dataDir = tempRoot(); + createManagedNode(dataDir); + const customBin = path.join(tempRoot(), "custom-node"); + fs.writeFileSync(customBin, "binary"); + + const service = new ToolchainService({ dataDir, env: EMPTY_ENV, probeVersion: async () => "1.2.3" }); + await service.setSource("node", "custom", customBin); + + const resolved = await service.resolve("node"); + expect(resolved.source).toBe("custom"); + expect(resolved.explicit).toBe(true); + expect(resolved.path).toBe(customBin); + expect(resolved.version).toBe("1.2.3"); + }); + + it("keeps an explicit unconfigured choice over any derived source", async () => { + const dataDir = tempRoot(); + createManagedNode(dataDir); + + const service = new ToolchainService({ dataDir, env: EMPTY_ENV, probeVersion: async () => null }); + await service.setSource("node", "unconfigured"); + + const resolved = await service.resolve("node"); + expect(resolved.source).toBe("unconfigured"); + expect(resolved.explicit).toBe(true); + expect(resolved.path).toBeNull(); + + // Reverting falls back to the derived (managed) tree. + await service.removeSource("node"); + expect((await service.resolve("node")).source).toBe("managed"); + }); + + it("rewrites npx through managed node without touching args on fallback", async () => { + const dataDir = tempRoot(); + const nodePath = createManagedNode(dataDir); + const service = new ToolchainService({ dataDir, env: EMPTY_ENV, probeVersion: async () => pinFor("node") }); + await service.warmup(); + + const nodeDir = path.dirname(nodePath); + const cliRelative = + process.platform === "win32" ? "node_modules/npm/bin/npx-cli.js" : "../lib/node_modules/npm/bin/npx-cli.js"; + const cli = path.resolve(nodeDir, cliRelative); + fs.mkdirSync(path.dirname(cli), { recursive: true }); + fs.writeFileSync(cli, "// npx cli"); + + const rewritten = service.resolveCommandSync("npx", ["-y", "some-agent", "--acp"]); + expect(rewritten.command).toBe(nodePath); + expect(rewritten.args[0]).toBe(cli); + expect(rewritten.args.slice(1)).toEqual(["-y", "some-agent", "--acp"]); + + // Unknown commands pass through untouched. + expect(service.resolveCommandSync("rg", ["--version"])).toEqual({ command: "rg", args: ["--version"] }); + }); + + it("rewrites uvx to the uv sibling binary", async () => { + const dataDir = tempRoot(); + const uvPath = createManagedUv(dataDir); + const service = new ToolchainService({ dataDir, env: EMPTY_ENV, probeVersion: async () => pinFor("uv") }); + await service.warmup(); + + const rewritten = service.resolveCommandSync("uvx", ["some-server"]); + const uvDir = path.dirname(uvPath); + const expectedUvx = path.join(uvDir, process.platform === "win32" ? "uvx.exe" : "uvx"); + fs.mkdirSync(uvDir, { recursive: true }); + fs.writeFileSync(expectedUvx, "binary"); + const afterSibling = service.resolveCommandSync("uvx", ["some-server"]); + expect(afterSibling.command).toBe(expectedUvx); + expect(afterSibling.args).toEqual(["some-server"]); + }); + + it("reports bin dirs from the warm cache", async () => { + const dataDir = tempRoot(); + const uvPath = createManagedUv(dataDir); + const service = new ToolchainService({ dataDir, env: EMPTY_ENV, probeVersion: async () => null }); + expect(service.binDirsSync()).toEqual([]); + await service.warmup(); + expect(service.binDirsSync()).toContain(path.dirname(uvPath)); + expect(service.binDirForToolSync("uv")).toBe(path.dirname(uvPath)); + }); +}); + +describe("managed install pipeline", () => { + const roots: string[] = []; + + afterEach(() => { + while (roots.length > 0) { + const root = roots.pop(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + } + }); + + const tempRoot = (): string => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argos-toolchains-install-")); + roots.push(dir); + return dir; + }; + + const fakeArchive = (content: string) => { + const bytes = new TextEncoder().encode(content); + const sha256 = createHash("sha256").update(bytes).digest("hex"); + return { + bytes, + archive: { filename: "node-vTEST.tar.gz", url: "https://example.invalid/node.tar.gz", sha256 }, + }; + }; + + it("activates a verified archive atomically and the service sees it", async () => { + const dataDir = tempRoot(); + const { bytes, archive } = fakeArchive("payload"); + const nodeRoot = path.join(dataDir, "toolchains", "tools", "node", pinFor("node")); + + await installToolchain("node", { + dataDir, + fetchImpl: (async () => new Response(bytes)) as typeof fetch, + cancelled: () => false, + archive, + extract: async (_archivePath, destinationDir) => { + // Emulate an archive with one top-level dir, platform layout inside. + const top = path.join(destinationDir, "node-vTEST"); + fs.mkdirSync(process.platform === "win32" ? top : path.join(top, "bin"), { recursive: true }); + fs.writeFileSync(path.join(top, nodeBin()), "binary"); + }, + }); + + expect(fs.existsSync(nodeRoot)).toBe(true); + expect(fs.existsSync(path.join(nodeRoot, nodeBin()))).toBe(true); + + const service = new ToolchainService({ dataDir, env: EMPTY_ENV, probeVersion: async () => pinFor("node") }); + const resolved = await service.resolve("node"); + expect(resolved.source).toBe("managed"); + }); + + it("fails on checksum mismatch without touching the active tree", async () => { + const dataDir = tempRoot(); + const { bytes } = fakeArchive("payload"); + const versionDir = path.join(dataDir, "toolchains", "tools", "node", pinFor("node")); + fs.mkdirSync(versionDir, { recursive: true }); + fs.writeFileSync(path.join(versionDir, nodeBin()), "previous"); + + await expect( + installToolchain("node", { + dataDir, + fetchImpl: (async () => new Response(bytes)) as typeof fetch, + cancelled: () => false, + archive: { filename: "node-vTEST.tar.gz", url: "https://example.invalid/x", sha256: "deadbeef" }, + }), + ).rejects.toBeInstanceOf(ToolchainInstallError); + + expect(fs.existsSync(path.join(versionDir, nodeBin()))).toBe(true); + expect(fs.readFileSync(path.join(versionDir, nodeBin()), "utf-8")).toBe("previous"); + }); + + it("cancel during download leaves nothing behind", async () => { + const dataDir = tempRoot(); + const { bytes, archive } = fakeArchive("payload"); + + await expect( + installToolchain("node", { + dataDir, + fetchImpl: (async () => new Response(bytes)) as typeof fetch, + cancelled: () => true, + archive, + }), + ).rejects.toMatchObject({ code: "cancelled" }); + + expect(fs.existsSync(path.join(dataDir, "toolchains", "tools", "node", pinFor("node")))).toBe(false); + }); + + // Rollback semantics rely on Windows' mandatory locks: an open handle inside + // the staged tree blocks its activation rename. POSIX has no equivalent, so + // this scenario is Windows-only. + it.skipIf(process.platform !== "win32")("keeps the previous tree active when activation is blocked", async () => { + const dataDir = tempRoot(); + const { bytes, archive } = fakeArchive("payload"); + const versionDir = path.join(dataDir, "toolchains", "tools", "node", pinFor("node")); + fs.mkdirSync(versionDir, { recursive: true }); + fs.writeFileSync(path.join(versionDir, nodeBin()), "previous"); + + let lockFd: number | null = null; + try { + await expect( + installToolchain("node", { + dataDir, + fetchImpl: (async () => new Response(bytes)) as typeof fetch, + cancelled: () => false, + archive, + extract: async (_archivePath, destinationDir) => { + fs.mkdirSync(destinationDir, { recursive: true }); + fs.writeFileSync(path.join(destinationDir, nodeBin()), "next"); + // Hold a handle inside the staged tree so the activation rename + // fails and the rollback path must fire. + lockFd = fs.openSync(path.join(destinationDir, nodeBin()), "r+"); + }, + }), + ).rejects.toBeInstanceOf(ToolchainInstallError); + } finally { + if (lockFd != null) fs.closeSync(lockFd); + } + + // The previous content is still active. + expect(fs.existsSync(path.join(versionDir, nodeBin()))).toBe(true); + expect(fs.readFileSync(path.join(versionDir, nodeBin()), "utf-8")).toBe("previous"); + }); +}); + +describe("system detection", () => { + it("finds binaries from injected PATH entries before defaults", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argos-toolchains-sys-")); + const uvName = process.platform === "win32" ? "uv.exe" : "uv"; + fs.writeFileSync(path.join(dir, uvName), "binary"); + const found = systemToolPath("uv", { PATH: dir } as NodeJS.ProcessEnv); + expect(found).toBe(path.join(dir, uvName)); + fs.rmSync(dir, { recursive: true, force: true }); + }); +}); diff --git a/apps/daemon/test/toolchainsState.test.ts b/apps/daemon/test/toolchainsState.test.ts new file mode 100644 index 000000000..f690e2579 --- /dev/null +++ b/apps/daemon/test/toolchainsState.test.ts @@ -0,0 +1,92 @@ +import { existsSync, mkdirSync } from "node:fs"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "bun:test"; +import { loadState, saveState, emptyState, toolchainsDir } from "../src/host/toolchains/state"; + +describe("toolchain state store", () => { + const roots: string[] = []; + + afterEach(() => { + while (roots.length > 0) { + const root = roots.pop(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + } + }); + + const tempRoot = (): string => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "argos-toolchains-")); + roots.push(dir); + return dir; + }; + + it("round-trips explicit sources", async () => { + const dataDir = tempRoot(); + const state = emptyState(); + state.sources.node = { source: "custom", explicit: true, path: "D:/tools/node/node.exe" }; + state.sources.uv = { source: "unconfigured", explicit: true }; + await saveState(dataDir, state); + + const loaded = await loadState(dataDir); + expect(loaded.sources.node).toEqual({ source: "custom", explicit: true, path: "D:/tools/node/node.exe" }); + expect(loaded.sources.uv).toEqual({ source: "unconfigured", explicit: true }); + expect(loaded.sources.ripgrep).toBeUndefined(); + }); + + it("returns empty state when no file exists", async () => { + const dataDir = tempRoot(); + const loaded = await loadState(dataDir); + expect(loaded).toEqual({ version: 1, sources: {} }); + }); + + it("quarantines corrupt state under a timestamped name and recovers", async () => { + const dataDir = tempRoot(); + mkdirSync(toolchainsDir(dataDir), { recursive: true }); + await Bun.write(path.join(toolchainsDir(dataDir), "state.json"), "{ not json"); + + const loaded = await loadState(dataDir); + expect(loaded).toEqual({ version: 1, sources: {} }); + + // The corrupt file is preserved beside the live one with a timestamp. + const files = fs.readdirSync(toolchainsDir(dataDir)); + expect(files.some((file) => file.startsWith("state.corrupt-"))).toBe(true); + }); + + it("survives repeated corruption without throwing", async () => { + const dataDir = tempRoot(); + mkdirSync(toolchainsDir(dataDir), { recursive: true }); + const statePath = path.join(toolchainsDir(dataDir), "state.json"); + + await Bun.write(statePath, "garbage-one"); + expect(await loadState(dataDir)).toEqual({ version: 1, sources: {} }); + + await Bun.write(statePath, "garbage-two"); + expect(await loadState(dataDir)).toEqual({ version: 1, sources: {} }); + + const quarantined = fs.readdirSync(toolchainsDir(dataDir)).filter((file) => file.startsWith("state.corrupt-")); + expect(quarantined.length).toBe(2); + }); + + it("drops malformed source entries instead of throwing", async () => { + const dataDir = tempRoot(); + mkdirSync(toolchainsDir(dataDir), { recursive: true }); + await Bun.write( + path.join(toolchainsDir(dataDir), "state.json"), + JSON.stringify({ + version: 1, + sources: { + node: { source: "managed", explicit: true }, + uv: { source: "custom", explicit: true }, + ripgrep: "bogus", + }, + }), + ); + + const loaded = await loadState(dataDir); + expect(loaded.sources.node).toBeUndefined(); + expect(loaded.sources.uv).toBeUndefined(); + expect(loaded.sources.ripgrep).toBeUndefined(); + expect(existsSync(path.join(toolchainsDir(dataDir), "state.json"))).toBe(true); + }); +}); diff --git a/docs/features/managed-toolchains/plan.md b/docs/features/managed-toolchains/plan.md new file mode 100644 index 000000000..97b370357 --- /dev/null +++ b/docs/features/managed-toolchains/plan.md @@ -0,0 +1,66 @@ +# Plan: Managed toolchains + +## 1. Shared contracts + +- `packages/shared-contracts/src/routes/toolchains.routes.ts`: + - `toolchains.list` → `{ tools: ToolchainStatus[] }` (status: source, explicit, path, version, + error, pin, install progress). + - `toolchains.setSource` `{ tool, source: "custom" | "unconfigured", path? }` — explicit only. + - `toolchains.install` `{ tool }` → starts/attaches the managed install job. + - `toolchains.cancelInstall` `{ tool }`. + - `toolchains.removeSource` `{ tool }` — clears explicit source + managed tree (revert). +- Types in the routes file; catalog entry in `packages/shared-contracts/src/routes.ts`. + +## 2. Daemon service (`apps/daemon/src/host/toolchains/`) + +- `types.ts` — `ToolchainName`, `ToolchainSource`, `ToolchainStatus`, `ResolvedToolchain`. +- `state.ts` — `state.json` under `/toolchains/`: `{ version: 1, sources: { node?, uv?, + ripgrep? } }`, entries `{ source, explicit, path? }`. Corrupt → timestamped quarantine + (`state.corrupt-.json`) + fresh state. +- `catalog.ts` — `NODE_PIN = "v24.18.0"`, `UV_PIN = "0.9.18"`; per platform-arch archive + `{ filename, url, sha256 }`; `RIPGREP` handled as bundled/system only (no managed pin yet — + bundled seed covers it; documented). +- `resolve.ts` — precedence: explicit custom → explicit unconfigured → managed tree → bundled + seed → system (PATH + default dirs: nvm/volta/homebrew/Program Files) → unconfigured. Returns + `{ source, explicit, path, version }` (version probed lazily via `--version` with timeout, + cached). +- `install.ts` — pipeline: fetch archive (verify sha256 while streaming to `downloads/`) → + extract via `tar -xf` to staging → flatten the single top-level dir → atomic rename to + `tools/-` → set active pointer. Rotate `.prev`. Cooperative cancel between + phases; cancelled staging dirs are cleaned best-effort. +- `service.ts` — `ToolchainService` facade: `list()`, `setSource()`, `removeSource()`, + `install()`, `cancelInstall()`, `resolve(tool)`, `resolveCommand(command, args)`, `binPaths()`. + Injectable `deps` (dataDir, fetchImpl, now, probeVersion) for tests. + +## 3. Daemon wiring + +- `apps/daemon/src/index.ts` — construct the service after dataDir is known; pass into + `createDaemonAcpPorts`, `createDaemonMcpPorts`, dispatcher. +- `apps/daemon/src/host/acpPorts.ts` — `resolveCommand`: rewrite `npx`/`npm`/`node`/`uvx`/`uv` + through the service (`npx` → node + npx-cli.js per D5); `buildSpawnEnv`: prepend resolved + toolchain bin dirs to PATH. +- `apps/daemon/src/host/daemonMcpPorts.ts` — `getUvRuntimePath`/`getBunRuntimePath` return + resolved uv/node dirs; `processCommandWithArgs` applies the same rewrites as + `acpPorts.resolveCommand`. +- `apps/daemon/src/dispatch/daemonDispatcher.ts` — `toolchains.*` route handlers. + +## 4. UI + +- `packages/shared/src/settingsNavigation.ts` — `settings-toolchains` item (tools group, + `lucide:cpu`), title map entry. +- `packages/ui/api/ToolchainClient.ts` — typed client over the routes. +- `packages/ui/settings/components/ToolchainsSettings.tsx` — per-tool card: source badge, path, + version, pin, actions (install/repair, cancel, revert, set custom path via folder picker, + clear). Refreshes status on an interval while an install is in flight. +- `packages/ui/settings/main.tsx` — componentMap entry; browser-safe (no desktop dependency). + +## 5. Tests (daemon, bun test) + +- `toolchainsState.test.ts` — persistence round-trip, explicit-only writes, corrupt quarantine + (timestamped, no collision on double corruption). +- `toolchainsResolve.test.ts` — precedence matrix (explicit custom/unconfigured over managed/ + bundled/system; missing everything → unconfigured), npx rewrite, env PATH prepend. +- `toolchainsInstall.test.ts` — fake fetcher + fake extractor: sha256 mismatch fails without + touching the active tree; success activates atomically; cancel between phases leaves previous + tree active; `.prev` rotation. +- `toolchainsRoutes.test.ts` — dispatcher route surface. diff --git a/docs/features/managed-toolchains/spec.md b/docs/features/managed-toolchains/spec.md new file mode 100644 index 000000000..3f47da755 --- /dev/null +++ b/docs/features/managed-toolchains/spec.md @@ -0,0 +1,96 @@ +# Spec: Managed toolchains (Node, uv, ripgrep) + +Inspired by ThinkInAIXYZ/deepchat#2193 ("move Node and uv to managed installs"), re-designed for +Argos' daemon-first architecture. The upstream RFC's core ideas — one resolver, an explicit +persisted source, verified managed installs, atomic activation — apply directly; the +Electron-specific parts (installer Node removal, CLI hosting, OCR ABI gates) do not map and are +not attempted. + +## Problem + +Argos has no central runtime resolver. Today: + +1. **The daemon is runtime-blind.** `acpPorts.ts` and `daemonMcpPorts.ts` ship identity/no-op + runtime ports, so the headless daemon (the first-class deployment per `distro/`) can only run + `npx`-distribution ACP agents or `uvx` MCP servers if Node/uv happen to be on the daemon's + PATH. The bundled `runtime/uv` and `runtime/ripgrep` shipped inside the desktop app are + invisible to it (the sidecar spawn forwards only `process.env`). +2. **Three divergent resolution paths**: desktop `RuntimeHelper` (Electron-coupled), the host-port + seams with two divergent implementations, and ad-hoc per-consumer logic (pi worker, sidecar, + DuckDB extensions). +3. **No verification**: `installRuntime.mjs` pins versions inline without checksums; ACP binary + downloads are unverified `fetch`es. +4. **No toolchain UX**: missing runtimes surface as raw spawn failures or thrown errors; there is + no status view, install, or repair flow. + +## Goals + +- One daemon-owned `ToolchainService` resolves `node`, `uv`, and `ripgrep` through an explicit + persisted source: `bundled | managed | system | custom | unconfigured`. +- Managed installs download pinned archives with SHA-256 verification, extract to a staging dir, + and activate atomically via rename. A failed or cancelled install leaves the previous tree + active. +- Daemon consumers resolve only through the service: ACP launch (npx/uvx/binary agents) and MCP + stdio (`npx`/`uvx`/`uv` commands). `npx` is rewritten to `node ` so managed + Node works without shell/`.cmd` spawning. +- Bundled detection works for the headless daemon (probe `execDir/../runtime`, `execDir/runtime`, + `cwd/runtime`), so packaged daemons see the uv/ripgrep seed. +- Settings page ("Toolchains") with per-tool source, resolved path/version, install/repair/ + cancel/revert actions. +- Corrupt `state.json` is quarantined under a timestamped name and state resets to unconfigured + (upstream review flagged a fixed-name quarantine collision; we fix it from the start). + +## Non-goals (documented follow-ups) + +- Windows login-shell PATH refresh (detection stays `process.env` on win32; system detection + additionally scans nvm/volta/default install dirs). +- Download resume; checksums for build-time bundled seeds (`installRuntime.mjs`); ACP agent + archive checksums. +- Migrating desktop `RuntimeHelper`/`SkillExecutionService` onto the service (desktop keeps its + existing resolution; the daemon is the scope). +- pi worker / sidecar bun resolution (Bun is self-hosted by the daemon binary; nothing to manage). +- Per-skill python/node policy migration; OCR-style ABI gating (no OCR in Argos). +- Missing-toolchain aggregated banner outside the settings page (consumers fail with typed errors + that reach existing failure paths). + +## Decisions + +- **D1 — Daemon-owned service** at `apps/daemon/src/host/toolchains/`, Bun-runtime code + (`Bun.file`/`Bun.write` per the bun-file-io rule). Desktop reaches it only via + `toolchains.*` routes. +- **D2 — Persist only explicit sources** (upstream's `persist first-run sources` evolution, + learned the hard way): `state.json` records `custom` and `unconfigured` choices marked + `explicit: true`. `bundled`/`managed`/`system` are derived on demand: bundled = seed found on + disk; managed = installed tree present; system = found on PATH/dirs. Precedence: + explicit custom → explicit unconfigured → managed → bundled → system → unconfigured. + Rationale: derived selections keep working when PATH refreshes or the bundled seed disappears, + and cannot leak a stale pointer. +- **D3 — Pins + real checksums in `catalog.ts`**: Node `v24.18.0` (nodejs.org SHASUMS256) and + uv `0.9.18` (GitHub release assets, hashes captured from the release artifacts at + implementation time). Filenames embed the version for Node; uv asset names do not, so the + catalog test asserts the hash table is keyed per tool+asset and non-empty — a pin bump without + hashes fails review, not first install. +- **D4 — Atomic activation**: download → `staging/` dir → verify sha256 → extract to + `tools/-.staging` → rename to `tools/-` → update the `active` + pointer in state. Previous tree is kept as `tools/-.prev` (rotated, never + deleted while active — EBUSY/EPERM on Windows classifies as a disk error, per upstream's + `archive busy previous trees` fix). Cancel is a cooperative flag checked between phases. +- **D5 — `npx` rewrite, not `.cmd` spawn**: `resolveCommand("npx", args)` returns + `/ [node_modules/npm/bin/npx-cli.js, ...args]` (and `npm` similarly). + `.cmd` shims require `shell: true`, which the process managers deliberately avoid. +- **D6 — Extraction via `tar`**: `tar -xf` handles both `.zip` (Windows ships bsdtar) and + `.tar.gz`. No new extract dependency. +- **D7 — consumers**: daemon `acpPorts.resolveCommand/buildSpawnEnv` and + `daemonMcpPorts.getBunRuntimePath/getUvRuntimePath/processCommandWithArgs` route through the + service. Desktop hosts are unchanged in this PR. +- **D8 — Settings page** `settings-toolchains` in the `tools` nav group; `ToolchainClient` + (`packages/ui/api/ToolchainClient.ts`) over `toolchains.*` routes. + +## Risks / constraints + +- Node tarballs are ~30 MB; install progress is reported via `toolchains.status` polling of the + service's in-memory install job (no event stream in v1). +- `system` detection quality varies per platform (documented; upstream has the same known gap). +- If neither managed nor bundled nor system node exists, `resolveCommand("npx")` returns the + input unchanged — existing behavior (PATH lookup fails downstream with a clear spawn error) + rather than a new crash path. diff --git a/docs/features/managed-toolchains/tasks.md b/docs/features/managed-toolchains/tasks.md new file mode 100644 index 000000000..8c023f168 --- /dev/null +++ b/docs/features/managed-toolchains/tasks.md @@ -0,0 +1,25 @@ +# Tasks: Managed toolchains + +- [x] T1 Contracts: `toolchains.*` routes + catalog entries +- [x] T2 Daemon: state store (persist explicit sources, timestamped quarantine) +- [x] T3 Daemon: catalog (Node v24.18.0 + uv 0.9.18, real sha256, per-platform archives) +- [x] T4 Daemon: resolver (precedence, bundled probing, system detection, version probe cache) +- [x] T5 Daemon: installer (sha256-verified download, staging, atomic activation, `.prev` + rotation, cooperative cancel) +- [x] T6 Daemon: `ToolchainService` facade + sync/async `resolveCommand` rewrites +- [x] T7 Daemon wiring: index.ts, `acpPorts` (new `resolveCommandWithArgs` seam), + `daemonMcpPorts`, dispatcher routes +- [x] T8 UI: nav item (`settings-toolchains`, tools group) + `ToolchainClient` + + `ToolchainsSettings` page +- [x] T9 Tests: state (5), service/resolver/rewrite (6), install pipeline (4), routes (5) +- [x] T10 Docs: fix bundled-runtime drift (AGENTS.md / CONTRIBUTING.md) +- [x] T11 `bun run format` + `bun run lint` + `bun run typecheck` + `bun test` + +## Verification results + +- Daemon: 405 tests pass (21 new); `tsc --noEmit` clean. +- Desktop + UI: `test:main` 1737 passed / 6 skipped; both typechecks clean. +- `bun run lint`: agent-cleanup, architecture, and route-catalog drift guards (416 routes) + + oxlint clean. +- Managed-install checksums captured from the official release artifacts at implementation + time (Node SHASUMS256.txt; uv release assets hashed locally). diff --git a/packages/acp-runtime/src/host/ports.ts b/packages/acp-runtime/src/host/ports.ts index c3c9fb8f7..635346276 100644 --- a/packages/acp-runtime/src/host/ports.ts +++ b/packages/acp-runtime/src/host/ports.ts @@ -26,6 +26,17 @@ export interface RuntimePort { expandPath(target: string): string; /** Swap a bare command (npx/npm/node/uvx) for the bundled bin when enabled. */ resolveCommand(command: string, useBundled: boolean, checkExists: boolean): string; + /** + * Optional command+args rewrite for hosts that manage runtimes themselves + * (e.g. `npx -y pkg` -> `node npx-cli.js -y pkg` under a managed Node). + * When it resolves, its result wins over `resolveCommand`; returning null + * falls back to `resolveCommand` with the args untouched. + */ + resolveCommandWithArgs?(input: { + command: string; + args: string[]; + useBundled: boolean; + }): Promise<{ command: string; args: string[] } | null>; /** Prepend bundled runtime dirs to PATH in the spawn env. */ buildSpawnEnv(base: Record): Record; } diff --git a/packages/acp-runtime/src/process/acpProcessManager.ts b/packages/acp-runtime/src/process/acpProcessManager.ts index 0bc9b52de..68a8f9b58 100644 --- a/packages/acp-runtime/src/process/acpProcessManager.ts +++ b/packages/acp-runtime/src/process/acpProcessManager.ts @@ -1213,8 +1213,17 @@ export class AcpProcessManager implements AgentProcessManager "${processedCommand}"`); } - // Use expanded args - const processedArgs = expandedArgs; - let env = mergeCommandEnvironment(); let shellEnv: Record = {}; diff --git a/packages/shared-contracts/src/routes.ts b/packages/shared-contracts/src/routes.ts index 574224701..cf87f595f 100644 --- a/packages/shared-contracts/src/routes.ts +++ b/packages/shared-contracts/src/routes.ts @@ -395,6 +395,13 @@ import { systemSetPendingProviderInstallRoute, } from "./routes/system.routes"; import { toolsListDefinitionsRoute } from "./routes/tools.routes"; +import { + toolchainsListRoute, + toolchainsSetSourceRoute, + toolchainsRemoveSourceRoute, + toolchainsInstallRoute, + toolchainsCancelInstallRoute, +} from "./routes/toolchains.routes"; import { memoryListRoute, memoryGetStatusRoute, @@ -497,6 +504,7 @@ export * from "./routes/sync.routes"; export * from "./routes/system.routes"; export * from "./routes/tab.routes"; export * from "./routes/tools.routes"; +export * from "./routes/toolchains.routes"; export * from "./routes/memory.routes"; export * from "./routes/knowledge.routes"; export * from "./routes/upgrade.routes"; @@ -621,6 +629,11 @@ export const ARGOS_ROUTE_CATALOG = { [configCreateArgosAgentRoute.name]: configCreateArgosAgentRoute, [configUpdateArgosAgentRoute.name]: configUpdateArgosAgentRoute, [configDeleteArgosAgentRoute.name]: configDeleteArgosAgentRoute, + [toolchainsListRoute.name]: toolchainsListRoute, + [toolchainsSetSourceRoute.name]: toolchainsSetSourceRoute, + [toolchainsRemoveSourceRoute.name]: toolchainsRemoveSourceRoute, + [toolchainsInstallRoute.name]: toolchainsInstallRoute, + [toolchainsCancelInstallRoute.name]: toolchainsCancelInstallRoute, [configResolveArgosAgentConfigRoute.name]: configResolveArgosAgentConfigRoute, [configGetAgentMcpSelectionsRoute.name]: configGetAgentMcpSelectionsRoute, [configGetAcpSharedMcpSelectionsRoute.name]: configGetAcpSharedMcpSelectionsRoute, diff --git a/packages/shared-contracts/src/routes/system.routes.ts b/packages/shared-contracts/src/routes/system.routes.ts index 0d9ddb25d..be67f8807 100644 --- a/packages/shared-contracts/src/routes/system.routes.ts +++ b/packages/shared-contracts/src/routes/system.routes.ts @@ -11,6 +11,7 @@ export const SettingsRouteNameSchema = zod.enum([ "settings-mcp", "settings-argos-agents", "settings-acp", + "settings-toolchains", "settings-remote", "settings-server", "settings-notifications-hooks", diff --git a/packages/shared-contracts/src/routes/toolchains.routes.ts b/packages/shared-contracts/src/routes/toolchains.routes.ts new file mode 100644 index 000000000..a0ad65579 --- /dev/null +++ b/packages/shared-contracts/src/routes/toolchains.routes.ts @@ -0,0 +1,100 @@ +import zod from "zod"; +import { defineRouteContract } from "../common"; + +/** + * Managed toolchains: one daemon-owned resolver for external runtimes + * (Node, uv, ripgrep) with an explicit persisted source and verified + * managed installs. See docs/features/managed-toolchains. + */ + +export const TOOLCHAIN_NAMES = ["node", "uv", "ripgrep"] as const; +export type ToolchainName = (typeof TOOLCHAIN_NAMES)[number]; + +export const TOOLCHAIN_SOURCES = ["bundled", "managed", "system", "custom", "unconfigured"] as const; +export const TOOLCHAIN_SOURCE_SCHEMA = zod.enum(TOOLCHAIN_SOURCES); +export type ToolchainSource = (typeof TOOLCHAIN_SOURCES)[number]; + +const toolchainNameSchema = zod.enum(TOOLCHAIN_NAMES); + +export const toolchainsInstallStateSchema = zod.object({ + phase: zod.enum(["idle", "downloading", "extracting", "activating"]), + tool: toolchainNameSchema, + version: zod.string(), + startedAt: zod.number(), +}); + +export const toolchainsStatusSchema = zod.object({ + tool: toolchainNameSchema, + source: TOOLCHAIN_SOURCE_SCHEMA, + explicit: zod.boolean(), + path: zod.string().nullable(), + version: zod.string().nullable(), + error: zod.string().nullable(), + pin: zod.string().nullable(), + install: toolchainsInstallStateSchema.nullable(), +}); + +export type ToolchainStatus = zod.infer; +export type ToolchainsInstallState = zod.infer; + +export const toolchainsListRoute = defineRouteContract({ + name: "toolchains.list", + input: zod.object({}).default({}), + output: zod.object({ + tools: zod.array(toolchainsStatusSchema), + }), +}); + +// Only explicit user choices persist: a selected custom path, or an +// explicit "unconfigured". Derived sources (managed/bundled/system) are +// recomputed on demand so a PATH refresh or a removed seed cannot leave a +// stale pointer behind. +export const toolchainsSetSourceRoute = defineRouteContract({ + name: "toolchains.setSource", + input: zod + .object({ + tool: toolchainNameSchema, + source: zod.enum(["custom", "unconfigured"]), + path: zod.string().min(1).optional(), + }) + .superRefine((value, ctx) => { + if (value.source === "custom" && !value.path) { + ctx.addIssue({ code: "custom", message: "A custom source requires a path", path: ["path"] }); + } + }), + output: zod.object({ + status: toolchainsStatusSchema, + }), +}); + +export const toolchainsRemoveSourceRoute = defineRouteContract({ + name: "toolchains.removeSource", + input: zod.object({ + tool: toolchainNameSchema, + }), + output: zod.object({ + status: toolchainsStatusSchema, + }), +}); + +export const toolchainsInstallRoute = defineRouteContract({ + name: "toolchains.install", + input: zod.object({ + tool: zod.enum(["node", "uv"]), + }), + output: zod.object({ + started: zod.boolean(), + status: toolchainsStatusSchema, + }), +}); + +export const toolchainsCancelInstallRoute = defineRouteContract({ + name: "toolchains.cancelInstall", + input: zod.object({ + tool: zod.enum(["node", "uv"]), + }), + output: zod.object({ + cancelled: zod.boolean(), + status: toolchainsStatusSchema, + }), +}); diff --git a/packages/shared/src/settingsNavigation.ts b/packages/shared/src/settingsNavigation.ts index 57a76afcd..573788280 100644 --- a/packages/shared/src/settingsNavigation.ts +++ b/packages/shared/src/settingsNavigation.ts @@ -9,6 +9,7 @@ export interface SettingsNavigationItem { | "settings-mcp" | "settings-argos-agents" | "settings-acp" + | "settings-toolchains" | "settings-remote" | "settings-server" | "settings-notifications-hooks" @@ -143,6 +144,15 @@ export const SETTINGS_NAVIGATION_ITEMS: SettingsNavigationItem[] = [ groupKey: "models", keywords: ["acp", "agent client protocol"], }, + { + routeName: "settings-toolchains", + path: "/toolchains", + titleKey: "routes.settings-toolchains", + icon: "lucide:cpu", + position: 4.75, + groupKey: "tools", + keywords: ["toolchains", "runtime", "node", "uv", "python", "install"], + }, { routeName: "settings-dashboard", path: "/dashboard", @@ -342,6 +352,7 @@ const TITLE_MAP: Record = { "routes.settings-mcp": "MCP Settings", "routes.settings-argos-agents": "Argos Agents", "routes.settings-acp": "ACP Agents", + "routes.settings-toolchains": "Toolchains", "routes.settings-server": "Server", "routes.settings-remote": "Remote", "routes.settings-notifications-hooks": "Hooks", diff --git a/packages/ui/api/ToolchainClient.ts b/packages/ui/api/ToolchainClient.ts new file mode 100644 index 000000000..23cc9761b --- /dev/null +++ b/packages/ui/api/ToolchainClient.ts @@ -0,0 +1,42 @@ +import type { ArgosBridge } from "@argos/shared-contracts/bridge"; +import { + toolchainsCancelInstallRoute, + toolchainsInstallRoute, + toolchainsListRoute, + toolchainsRemoveSourceRoute, + toolchainsSetSourceRoute, +} from "@argos/shared-contracts/routes"; +import type { ArgosRouteInput } from "@argos/shared-contracts/routes"; +import { getArgosBridge } from "./core"; + +export function createToolchainClient(bridge: ArgosBridge = getArgosBridge()) { + async function list() { + return await bridge.invoke(toolchainsListRoute.name, {} as ArgosRouteInput); + } + + async function setSource(input: ArgosRouteInput) { + return await bridge.invoke(toolchainsSetSourceRoute.name, input); + } + + async function removeSource(tool: "node" | "uv" | "ripgrep") { + return await bridge.invoke(toolchainsRemoveSourceRoute.name, { tool }); + } + + async function install(tool: "node" | "uv") { + return await bridge.invoke(toolchainsInstallRoute.name, { tool }); + } + + async function cancelInstall(tool: "node" | "uv") { + return await bridge.invoke(toolchainsCancelInstallRoute.name, { tool }); + } + + return { + list, + setSource, + removeSource, + install, + cancelInstall, + }; +} + +export type ToolchainClient = ReturnType; diff --git a/packages/ui/settings/components/ToolchainsSettings.tsx b/packages/ui/settings/components/ToolchainsSettings.tsx new file mode 100644 index 000000000..c122a5ede --- /dev/null +++ b/packages/ui/settings/components/ToolchainsSettings.tsx @@ -0,0 +1,268 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { Icon } from "@iconify/react"; +import { Button } from "#shadcn/components/ui/button"; +import { Badge } from "#shadcn/components/ui/badge"; +import { Input } from "#shadcn/components/ui/input"; +import { Skeleton } from "#shadcn/components/ui/skeleton"; +import { toast } from "#/components/use-toast"; +import { createToolchainClient } from "#api/ToolchainClient"; +import type { ToolchainName, ToolchainSource, ToolchainStatus } from "@argos/shared-contracts/routes"; + +const toolchainClient = createToolchainClient(); + +const SOURCE_BADGE_CLASS: Record = { + managed: "bg-green-600/15 text-green-700 dark:text-green-400 border-green-600/30", + bundled: "bg-blue-600/15 text-blue-700 dark:text-blue-400 border-blue-600/30", + system: "bg-violet-600/15 text-violet-700 dark:text-violet-400 border-violet-600/30", + custom: "bg-amber-600/15 text-amber-700 dark:text-amber-400 border-amber-600/30", + unconfigured: "bg-muted text-muted-foreground border-border", +}; + +const TOOL_DESCRIPTIONS: Record = { + node: { + title: "Node.js", + description: + "Runs npx-based ACP agents and Node MCP servers. Managed installs are pinned, SHA-256 verified, and isolated from your system.", + }, + uv: { + title: "uv", + description: + "Runs uvx-based MCP servers and Python tooling. Ships as a bundled seed; a managed install overrides it with the pinned release.", + }, + ripgrep: { + title: "ripgrep", + description: "Fast file search used by agent tools. Resolved from the bundled seed or your system install.", + }, +}; + +const MANAGEABLE: Array = ["node", "uv", "ripgrep"]; + +function SourceBadge({ source }: { source: ToolchainSource }) { + const label = source.charAt(0).toUpperCase() + source.slice(1); + return ( + + {label} + + ); +} + +function ToolchainCard({ + status, + busy, + onInstall, + onCancel, + onRevert, + onSetCustom, +}: { + status: ToolchainStatus; + busy: boolean; + onInstall: (tool: "node" | "uv") => void; + onCancel: (tool: "node" | "uv") => void; + onRevert: (tool: ToolchainName) => void; + onSetCustom: (tool: ToolchainName, path: string) => void; +}) { + const [customPath, setCustomPath] = useState(""); + const [showCustomInput, setShowCustomInput] = useState(false); + const meta = TOOL_DESCRIPTIONS[status.tool]; + const installable = status.tool !== "ripgrep"; + const installing = status.install != null && status.install.phase !== "idle"; + + return ( +
+
+
+ {meta.title} + + {status.version ? {status.version} : null} + {status.pin ? pin {status.pin} : null} +
+
+ {installable ? ( + installing ? ( + + ) : ( + + ) + ) : null} + {status.explicit ? ( + + ) : null} +
+
+ +

{meta.description}

+ + {installing ? ( +
+ + {status.install?.phase === "downloading" ? "Downloading…" : null} + {status.install?.phase === "extracting" ? "Extracting…" : null} + {status.install?.phase === "activating" ? "Activating…" : null} + {status.install?.phase === "idle" ? "Finishing…" : null} +
+ ) : null} + + {status.error ? ( +
+ + {status.error} +
+ ) : null} + +
+ {status.path ?? "Not found — npx/uvx commands will fall back to PATH lookup."} +
+ + {showCustomInput ? ( +
+ setCustomPath(event.target.value)} + placeholder="Path to the executable…" + className="h-8 font-mono text-xs" + /> + + +
+ ) : ( + + )} +
+ ); +} + +export default function ToolchainsSettings() { + const [tools, setTools] = useState(null); + const [busy, setBusy] = useState(false); + const pollRef = useRef(null); + + const refresh = useCallback(async () => { + try { + const result = await toolchainClient.list(); + setTools(result.tools); + return result.tools; + } catch (error) { + toast({ + title: "Could not load toolchains", + description: error instanceof Error ? error.message : String(error), + variant: "destructive", + }); + return []; + } + }, []); + + useEffect(() => { + queueMicrotask(() => void refresh()); + }, [refresh]); + + // Poll while any install is in flight so progress phases stay live. + useEffect(() => { + const installing = tools?.some((tool) => tool.install != null && tool.install.phase !== "idle"); + if (installing && pollRef.current == null) { + pollRef.current = window.setInterval(() => void refresh(), 1500); + } else if (!installing && pollRef.current != null) { + window.clearInterval(pollRef.current); + pollRef.current = null; + } + return () => { + if (pollRef.current != null) { + window.clearInterval(pollRef.current); + pollRef.current = null; + } + }; + }, [tools, refresh]); + + const run = useCallback( + async (action: () => Promise, successTitle: string) => { + setBusy(true); + try { + await action(); + await refresh(); + toast({ title: successTitle }); + } catch (error) { + toast({ + title: "Action failed", + description: error instanceof Error ? error.message : String(error), + variant: "destructive", + }); + } + setBusy(false); + }, + [refresh], + ); + + return ( +
+
+

Toolchains

+

+ External runtimes used by ACP agents, MCP servers, and agent tools. Managed installs are pinned and SHA-256 + verified; nothing here mutates your system installation. +

+
+ + {tools == null ? ( +
+ {[0, 1, 2].map((index) => ( + + ))} +
+ ) : ( +
+ {MANAGEABLE.map((tool) => { + const status = tools.find((entry) => entry.tool === tool); + if (!status) return null; + return ( + void run(() => toolchainClient.install(name), `Installing ${name}…`)} + onCancel={(name) => void run(() => toolchainClient.cancelInstall(name), "Cancellation requested")} + onRevert={(name) => void run(() => toolchainClient.removeSource(name), "Reverted")} + onSetCustom={(name, path) => + void run(() => toolchainClient.setSource({ tool: name, source: "custom", path }), "Custom path saved") + } + /> + ); + })} +
+ )} +
+ ); +} diff --git a/packages/ui/settings/main.tsx b/packages/ui/settings/main.tsx index f59134cd2..2d4d63ed3 100644 --- a/packages/ui/settings/main.tsx +++ b/packages/ui/settings/main.tsx @@ -23,6 +23,7 @@ import ModelProviderSettings from "./components/ModelProviderSettings"; import McpSettings from "./components/McpSettings"; import ArgosAgentsSettings from "./components/ArgosAgentsSettings"; import AcpSettings from "./components/AcpSettings"; +import ToolchainsSettings from "./components/ToolchainsSettings"; import RemoteSettings from "./components/RemoteSettings"; import ServerSettings from "./components/ServerSettings"; import NotificationsHooksSettings from "./components/NotificationsHooksSettings"; @@ -65,6 +66,7 @@ const componentMap: Record = { "settings-mcp": McpSettings, "settings-argos-agents": ArgosAgentsSettings, "settings-acp": AcpSettings, + "settings-toolchains": ToolchainsSettings, "settings-remote": RemoteSettings, "settings-server": ServerSettings, "settings-notifications-hooks": NotificationsHooksSettings, From 8d29ac00cf157022f214336203fda47378607a5c Mon Sep 17 00:00:00 2001 From: Francisco Pizarro Date: Tue, 8 Sep 2026 18:12:00 -0300 Subject: [PATCH 3/5] fix(toolchains): review hardening for managed installs Address reviewer findings on the managed toolchains PR: - await toolchain warmup before MCP servers start so npx/uvx rewriting never falls back to PATH mid-startup - refresh the warm sync cache after a successful install; emit the activating phase before the atomic rename instead of after completion - revert now removes the managed tree so managed installs can fall back to bundled/system; the UI shows Revert for managed sources - uvx fallback without a sibling binary becomes "uv tool run" instead of passing uvx arguments to bare uv (both sync and async rewrites) - corrupt toolchain state resets to a valid serialized empty state instead of an empty string that re-corrupted on every load - add darwin-x64 to the uv catalog (sha256 captured from the release) - stream downloads to disk while hashing instead of buffering the whole archive; clean the staging tree when extraction is cancelled - expand nvm version directories for system Node detection - Toolchains settings: explicit load-error state with retry; drop manual memoization flagged by react-compiler lint SDD: docs/features/managed-toolchains (review-hardening section) --- apps/daemon/src/host/daemonMcpPorts.ts | 7 +- apps/daemon/src/host/toolchains/catalog.ts | 5 ++ apps/daemon/src/host/toolchains/install.ts | 41 ++++++++-- apps/daemon/src/host/toolchains/locate.ts | 55 +++++++++++-- apps/daemon/src/host/toolchains/service.ts | 39 ++++++++-- apps/daemon/src/host/toolchains/state.ts | 4 +- apps/daemon/src/index.ts | 9 ++- apps/daemon/test/toolchainsService.test.ts | 25 +++++- apps/daemon/test/toolchainsState.test.ts | 6 +- .../components/ToolchainsSettings.tsx | 78 ++++++++++--------- 10 files changed, 208 insertions(+), 61 deletions(-) diff --git a/apps/daemon/src/host/daemonMcpPorts.ts b/apps/daemon/src/host/daemonMcpPorts.ts index cea0a86cd..6a5bb5578 100644 --- a/apps/daemon/src/host/daemonMcpPorts.ts +++ b/apps/daemon/src/host/daemonMcpPorts.ts @@ -1,4 +1,5 @@ import { homedir } from "node:os"; +import { delimiter } from "node:path"; import { createJsonStoreFactory } from "./jsonStoreFactory"; import { ArtifactsServer, @@ -97,7 +98,11 @@ export function createDaemonMcpPorts(deps: { expandPath: (target) => target, processCommandWithArgs: (command, args) => deps.toolchains ? deps.toolchains.resolveCommandSync(command, args) : { command, args }, - normalizePathEnv: (paths) => ({ key: "PATH", value: paths.join(":") }), + /** Coalesce concurrent identical Ollama lookups into one upstream request. */ + normalizePathEnv: (paths: string[]) => ({ + key: process.platform === "win32" ? "Path" : "PATH", + value: paths.join(delimiter), + }), getDefaultPaths: () => deps.toolchains?.binDirsSync() ?? [], getBunRuntimePath: () => null, getUvRuntimePath: () => deps.toolchains?.binDirForToolSync("uv") ?? null, diff --git a/apps/daemon/src/host/toolchains/catalog.ts b/apps/daemon/src/host/toolchains/catalog.ts index a36de708a..d6d74ad4e 100644 --- a/apps/daemon/src/host/toolchains/catalog.ts +++ b/apps/daemon/src/host/toolchains/catalog.ts @@ -72,6 +72,11 @@ const UV_ARCHIVES: Record> = { url: `${UV_RELEASE_BASE}/${UV_PIN}/uv-aarch64-apple-darwin.tar.gz`, sha256: "dc3bee4abbb3bac267a3985a23ea7617d19d41ff381dbaf560ba415ad65af68f", }, + "darwin-x64": { + filename: "uv-x86_64-apple-darwin.tar.gz", + url: `${UV_RELEASE_BASE}/${UV_PIN}/uv-x86_64-apple-darwin.tar.gz`, + sha256: "f86836c637333c65bbc7902acc9c49888eef9fbd15dccbc1946b10e30b041073", + }, "linux-x64": { filename: "uv-x86_64-unknown-linux-gnu.tar.gz", url: `${UV_RELEASE_BASE}/${UV_PIN}/uv-x86_64-unknown-linux-gnu.tar.gz`, diff --git a/apps/daemon/src/host/toolchains/install.ts b/apps/daemon/src/host/toolchains/install.ts index c92dfc3fa..1092751b6 100644 --- a/apps/daemon/src/host/toolchains/install.ts +++ b/apps/daemon/src/host/toolchains/install.ts @@ -30,6 +30,8 @@ export interface InstallContext { extract?: (archivePath: string, destinationDir: string) => Promise; /** Injectable archive (tests); defaults to the catalog pin. */ archive?: ToolchainArchive; + /** Phase progress callback (activating fires just before the rename). */ + onPhase?: (phase: "downloading" | "extracting" | "activating") => void; } function ensureCancel(ctx: InstallContext, phase: string): void { @@ -102,8 +104,10 @@ export async function installToolchain(tool: "node" | "uv", ctx: InstallContext) mkdirSync(downloadsDir, { recursive: true }); mkdirSync(toolsDir, { recursive: true }); ensureCancel(ctx, "download"); + ctx.onPhase?.("downloading"); - // Download + verify. + // Download + verify while streaming: hash and persist chunks as they + // arrive instead of buffering the whole archive in memory. const archivePath = path.join(downloadsDir, archive.filename); let response: Response; try { @@ -115,19 +119,37 @@ export async function installToolchain(tool: "node" | "uv", ctx: InstallContext) ); } if (!response.ok) { + // Release the error body so the underlying connection returns to the pool. + try { + await response.body?.cancel(); + } catch { + // best-effort + } throw new ToolchainInstallError(`Download failed: HTTP ${response.status} for ${archive.url}`, "network"); } - const bytes = new Uint8Array(await response.arrayBuffer()); ensureCancel(ctx, "download"); - const digest = createHash("sha256").update(bytes).digest("hex"); - if (digest !== archive.sha256) { + const digest = createHash("sha256"); + const writer = Bun.file(archivePath).writer(); + try { + for await (const chunk of response.body as unknown as AsyncIterable) { + digest.update(chunk); + writer.write(chunk); + ensureCancel(ctx, "download"); + } + } finally { + await writer.end(); + } + ensureCancel(ctx, "download"); + const checksum = digest.digest("hex"); + if (checksum !== archive.sha256) { + rmSync(archivePath, { force: true }); throw new ToolchainInstallError( - `Checksum mismatch for ${archive.filename}: expected ${archive.sha256}, got ${digest}`, + `Checksum mismatch for ${archive.filename}: expected ${archive.sha256}, got ${checksum}`, "checksum_mismatch", ); } - await Bun.write(archivePath, bytes); ensureCancel(ctx, "extract"); + ctx.onPhase?.("extracting"); // Stage the new tree outside the active path. const stagingTarget = `${versionDir}.incoming`; @@ -138,6 +160,12 @@ export async function installToolchain(tool: "node" | "uv", ctx: InstallContext) await extractAndFlatten(archivePath, stagingTarget, ctx.extract ?? defaultExtract); ensureCancel(ctx, "activating"); } catch (error) { + // A cancelled or failed extract must not leak a partial staging tree. + try { + rmSync(stagingTarget, { recursive: true, force: true }); + } catch { + // best-effort + } if (error instanceof ToolchainInstallError && error.code === "cancelled") { throw error; } @@ -148,6 +176,7 @@ export async function installToolchain(tool: "node" | "uv", ctx: InstallContext) } // Activate atomically: rotate the previous tree, then rename staging in. + ctx.onPhase?.("activating"); if (existsSync(versionDir)) { const prevPath = `${versionDir}.prev`; if (existsSync(prevPath)) { diff --git a/apps/daemon/src/host/toolchains/locate.ts b/apps/daemon/src/host/toolchains/locate.ts index 456e797a9..5e08a29fc 100644 --- a/apps/daemon/src/host/toolchains/locate.ts +++ b/apps/daemon/src/host/toolchains/locate.ts @@ -1,4 +1,4 @@ -import { existsSync } from "node:fs"; +import { existsSync, readdirSync, statSync } from "node:fs"; import path from "node:path"; import type { ResolvedToolchain, ToolchainName, ToolchainSource } from "./types"; @@ -103,10 +103,55 @@ export function systemSearchDirs(env: NodeJS.ProcessEnv): string[] { /** Locate a system-installed binary for a tool, or null. */ export function systemToolPath(tool: ToolchainName, env: NodeJS.ProcessEnv): string | null { - return findFileIn( - systemSearchDirs(env), - TOOL_BINARIES[tool].map((name) => EXE(name)), - ); + const binaries = TOOL_BINARIES[tool].map((name) => EXE(name)); + for (const dir of expandVersionManagerDirs(systemSearchDirs(env))) { + for (const binary of binaries) { + const candidate = path.join(dir, binary); + if (isFile(candidate)) { + return candidate; + } + } + } + return null; +} + +/** Version-manager roots hide a bin dir per installed version; expand them. */ +function expandVersionManagerDirs(dirs: string[]): string[] { + const expanded: string[] = []; + for (const dir of dirs) { + expanded.push(dir); + const normalized = dir.replace(/\\/g, "/"); + if (!normalized.includes("/.nvm")) continue; + // nvm layout: /node//bin (POSIX), \ (Windows) + let entries: string[] = []; + try { + entries = readdirSync(dir); + } catch { + continue; + } + for (const entry of entries) { + const versionDir = path.join(dir, entry); + if (!isDirectory(versionDir)) continue; + expanded.push(process.platform === "win32" ? versionDir : path.join(versionDir, "bin")); + } + } + return expanded; +} + +function isFile(candidate: string): boolean { + try { + return statSync(candidate).isFile(); + } catch { + return false; + } +} + +function isDirectory(candidate: string): boolean { + try { + return statSync(candidate).isDirectory(); + } catch { + return false; + } } export interface DerivedResolveOptions { diff --git a/apps/daemon/src/host/toolchains/service.ts b/apps/daemon/src/host/toolchains/service.ts index b7aadfd55..c18784845 100644 --- a/apps/daemon/src/host/toolchains/service.ts +++ b/apps/daemon/src/host/toolchains/service.ts @@ -1,4 +1,5 @@ -import { existsSync } from "node:fs"; +import { existsSync, rmSync } from "node:fs"; +import fs from "node:fs"; import path from "node:path"; import type { ToolchainName, ToolchainSource, ToolchainStatus } from "@argos/shared-contracts/routes"; import { NODE_PIN, UV_PIN, pinFor } from "./catalog"; @@ -183,6 +184,15 @@ export class ToolchainService { async removeSource(tool: ToolchainName): Promise { const state = await this.withState(); delete state.sources[tool]; + // Reverting a managed install removes its tree so bundled/system can + // serve again; without this the derived managed source would simply + // re-resolve and the UI revert would do nothing. + const tree = path.join(this.dataDir, "toolchains", "tools", tool); + try { + fs.rmSync(tree, { recursive: true, force: true }); + } catch (error) { + console.warn(`[toolchains] failed to remove managed tree for ${tool}:`, error); + } this.versionCache.clear(); await this.persist(state); return this.status(tool); @@ -206,13 +216,14 @@ export class ToolchainService { this.installJobs.set(tool, { ...job, phase }); } }; + let succeeded = false; try { await installToolchain(tool, { dataDir: this.dataDir, fetchImpl: this.fetchImpl, cancelled: () => this.cancelFlags.get(tool) === true, + onPhase: (phase) => advance(phase), extract: async (archivePath, destinationDir) => { - advance("extracting"); const proc = Bun.spawn(["tar", "-xf", archivePath, "-C", destinationDir], { stdout: "pipe", stderr: "pipe", @@ -226,9 +237,7 @@ export class ToolchainService { } }, }); - // Inject an activating tick so clients observe the final phase. - advance("activating"); - this.versionCache.clear(); + succeeded = true; } catch (error) { const job = this.installJobs.get(tool); if (job) { @@ -238,6 +247,12 @@ export class ToolchainService { this.installErrors.set(tool, message); } } + if (succeeded) { + // Activation changed the tree: refresh the version probe and the warm + // sync cache so consumers immediately see the managed tool. + this.versionCache.clear(); + await this.resolve(tool).catch(() => undefined); + } // The job record stays until the next `status()`/`install()` observes the // terminal state; keep it for one poll so the UI sees completion. setTimeout(() => { @@ -283,7 +298,12 @@ export class ToolchainService { } const uvxName = process.platform === "win32" ? "uvx.exe" : "uvx"; const uvx = path.join(binDirFor(uv.path), uvxName); - return { command: existsSync(uvx) ? uvx : uv.path, args }; + if (existsSync(uvx)) { + return { command: uvx, args }; + } + // `uvx pkg` is equivalent to `uv tool run pkg`; never pass uvx's + // arguments to bare `uv`. + return { command: uv.path, args: ["tool", "run", ...args] }; } return { command, args }; } @@ -319,7 +339,12 @@ export class ToolchainService { } const uvxName = process.platform === "win32" ? "uvx.exe" : "uvx"; const uvx = path.join(binDirFor(uv.path), uvxName); - return { command: existsSync(uvx) ? uvx : uv.path, args }; + if (existsSync(uvx)) { + return { command: uvx, args }; + } + // `uvx pkg` is equivalent to `uv tool run pkg`; never pass uvx's + // arguments to bare `uv`. + return { command: uv.path, args: ["tool", "run", ...args] }; } return { command, args }; } diff --git a/apps/daemon/src/host/toolchains/state.ts b/apps/daemon/src/host/toolchains/state.ts index f9a677615..461617c3b 100644 --- a/apps/daemon/src/host/toolchains/state.ts +++ b/apps/daemon/src/host/toolchains/state.ts @@ -55,7 +55,9 @@ export async function loadState(dataDir: string): Promise { join(toolchainsDir(dataDir), `state.corrupt-${Date.now()}.json`), await Bun.file(filePath).arrayBuffer(), ); - await Bun.write(filePath, ""); // reset so the next load succeeds + // Persist a VALID empty state — an empty string would re-corrupt on + // every load and grow a quarantine copy per daemon start. + await Bun.write(filePath, JSON.stringify(emptyState(), null, 2)); } catch { // best-effort quarantine; a fresh state is returned regardless } diff --git a/apps/daemon/src/index.ts b/apps/daemon/src/index.ts index 7027f354d..39f61b8ce 100644 --- a/apps/daemon/src/index.ts +++ b/apps/daemon/src/index.ts @@ -333,9 +333,10 @@ export async function startDaemon(options?: { const agentWorkspaceDir = pathJoin(paths.getDataDir(), "agent-workspace"); // One resolver for external runtimes; consumers (ACP launch, MCP stdio) - // must go through it. Warmed once so the sync seams have data. + // must go through it. Warmed before dependent subsystems start so the sync + // seams (MCP process rewriting) never fall back to PATH mid-startup. const toolchainService = new ToolchainService({ dataDir: paths.getDataDir() }); - void toolchainService.warmup().catch((error) => { + const toolchainWarmup = toolchainService.warmup().catch((error) => { logger.warn("[daemon] toolchain warmup failed:", error); }); @@ -524,8 +525,8 @@ export async function startDaemon(options?: { const mcpRuntime = new DaemonMcpRuntime(configPresenter, mcpPorts); const pluginRuntimeRegistry = new PluginRuntimeRegistry(mcpRuntime.serverManager); mcpPorts.services.pluginRuntime = pluginRuntimeRegistry; - void mcpRuntime - .startEnabledServers() + void toolchainWarmup + .then(() => mcpRuntime.startEnabledServers()) .then(({ started, failed }) => { logger.info(`[daemon] MCP startup complete: ${started.length} started, ${failed.length} failed`); }) diff --git a/apps/daemon/test/toolchainsService.test.ts b/apps/daemon/test/toolchainsService.test.ts index 96465daf5..f43abba33 100644 --- a/apps/daemon/test/toolchainsService.test.ts +++ b/apps/daemon/test/toolchainsService.test.ts @@ -92,8 +92,10 @@ describe("toolchain service", () => { expect(resolved.explicit).toBe(true); expect(resolved.path).toBeNull(); - // Reverting falls back to the derived (managed) tree. + // Reverting falls back to derived resolution: removeSource also removed + // the managed tree, so recreate it to observe the managed source again. await service.removeSource("node"); + createManagedNode(dataDir); expect((await service.resolve("node")).source).toBe("managed"); }); @@ -133,6 +135,27 @@ describe("toolchain service", () => { const afterSibling = service.resolveCommandSync("uvx", ["some-server"]); expect(afterSibling.command).toBe(expectedUvx); expect(afterSibling.args).toEqual(["some-server"]); + + // Without a sibling uvx binary, fall back to `uv tool run` — bare `uv` + // with uvx's arguments is not a valid invocation. + fs.rmSync(expectedUvx); + const fallback = service.resolveCommandSync("uvx", ["some-server"]); + expect(fallback.command).toBe(uvPath); + expect(fallback.args).toEqual(["tool", "run", "some-server"]); + }); + + it("reverting a managed install removes its tree", async () => { + const dataDir = tempRoot(); + const uvPath = createManagedUv(dataDir); + const service = new ToolchainService({ dataDir, env: EMPTY_ENV, probeVersion: async () => pinFor("uv") }); + await service.warmup(); + expect((await service.resolve("uv")).source).toBe("managed"); + + // A managed source is derived (not explicit); removeSource must still + // remove the tree so bundled/system can serve again. + await service.removeSource("uv"); + expect(fs.existsSync(uvPath)).toBe(false); + expect((await service.resolve("uv")).source).toBe("unconfigured"); }); it("reports bin dirs from the warm cache", async () => { diff --git a/apps/daemon/test/toolchainsState.test.ts b/apps/daemon/test/toolchainsState.test.ts index f690e2579..f21ca6000 100644 --- a/apps/daemon/test/toolchainsState.test.ts +++ b/apps/daemon/test/toolchainsState.test.ts @@ -48,9 +48,13 @@ describe("toolchain state store", () => { const loaded = await loadState(dataDir); expect(loaded).toEqual({ version: 1, sources: {} }); - // The corrupt file is preserved beside the live one with a timestamp. + // The corrupt file is preserved beside the live one with a timestamp, + // and the live file is reset to a VALID empty state (not an empty string, + // which would re-corrupt on every load). const files = fs.readdirSync(toolchainsDir(dataDir)); expect(files.some((file) => file.startsWith("state.corrupt-"))).toBe(true); + const resetContent = fs.readFileSync(path.join(toolchainsDir(dataDir), "state.json"), "utf-8"); + expect(() => JSON.parse(resetContent)).not.toThrow(); }); it("survives repeated corruption without throwing", async () => { diff --git a/packages/ui/settings/components/ToolchainsSettings.tsx b/packages/ui/settings/components/ToolchainsSettings.tsx index c122a5ede..fdff19fd7 100644 --- a/packages/ui/settings/components/ToolchainsSettings.tsx +++ b/packages/ui/settings/components/ToolchainsSettings.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState } from "react"; +import { useEffect, useEffectEvent, useRef, useState } from "react"; import { Icon } from "@iconify/react"; import { Button } from "#shadcn/components/ui/button"; import { Badge } from "#shadcn/components/ui/badge"; @@ -99,7 +99,7 @@ function ToolchainCard({ ) ) : null} - {status.explicit ? ( + {status.explicit || status.source === "managed" ? ( @@ -168,33 +168,37 @@ function ToolchainCard({ export default function ToolchainsSettings() { const [tools, setTools] = useState(null); + const [loadError, setLoadError] = useState(null); const [busy, setBusy] = useState(false); const pollRef = useRef(null); - const refresh = useCallback(async () => { + const refresh = async () => { try { const result = await toolchainClient.list(); setTools(result.tools); + setLoadError(null); return result.tools; } catch (error) { - toast({ - title: "Could not load toolchains", - description: error instanceof Error ? error.message : String(error), - variant: "destructive", - }); + // Fail visible: keep the previous snapshot (if any) on screen and show + // an explicit retry path instead of skeletons forever. + setLoadError(error instanceof Error ? error.message : String(error)); + setTools((current) => current ?? []); return []; } - }, []); + }; + + const refreshEvent = useEffectEvent(() => void refresh()); useEffect(() => { - queueMicrotask(() => void refresh()); - }, [refresh]); + queueMicrotask(() => refreshEvent()); + }, []); // Poll while any install is in flight so progress phases stay live. + const installing = tools?.some((tool) => tool.install != null && tool.install.phase !== "idle"); + const pollTick = useEffectEvent(() => void refresh()); useEffect(() => { - const installing = tools?.some((tool) => tool.install != null && tool.install.phase !== "idle"); if (installing && pollRef.current == null) { - pollRef.current = window.setInterval(() => void refresh(), 1500); + pollRef.current = window.setInterval(() => pollTick(), 1500); } else if (!installing && pollRef.current != null) { window.clearInterval(pollRef.current); pollRef.current = null; @@ -205,26 +209,23 @@ export default function ToolchainsSettings() { pollRef.current = null; } }; - }, [tools, refresh]); + }, [installing]); - const run = useCallback( - async (action: () => Promise, successTitle: string) => { - setBusy(true); - try { - await action(); - await refresh(); - toast({ title: successTitle }); - } catch (error) { - toast({ - title: "Action failed", - description: error instanceof Error ? error.message : String(error), - variant: "destructive", - }); - } - setBusy(false); - }, - [refresh], - ); + const run = async (action: () => Promise, successTitle: string) => { + setBusy(true); + try { + await action(); + await refresh(); + toast({ title: successTitle }); + } catch (error) { + toast({ + title: "Action failed", + description: error instanceof Error ? error.message : String(error), + variant: "destructive", + }); + } + setBusy(false); + }; return (
@@ -236,13 +237,20 @@ export default function ToolchainsSettings() {

- {tools == null ? ( + {tools == null && !loadError ? (
{[0, 1, 2].map((index) => ( ))}
- ) : ( + ) : loadError && !tools?.length ? ( +
+ Could not load toolchains: {loadError} + +
+ ) : tools ? (
{MANAGEABLE.map((tool) => { const status = tools.find((entry) => entry.tool === tool); @@ -262,7 +270,7 @@ export default function ToolchainsSettings() { ); })}
- )} + ) : null} ); } From de32479023147082333c9e089a1fc1f42e742fe7 Mon Sep 17 00:00:00 2001 From: Francisco Pizarro Date: Tue, 8 Sep 2026 21:54:58 -0300 Subject: [PATCH 4/5] feat(acp): terminal authentication for agent login (#99) * feat(acp): terminal authentication for agent login Agents that require login (MiniMax Code and similar) advertise authMethods only when the client declares clientCapabilities.auth.terminal. Our advertisement was dead code: enableTerminalAuth was computed from the initialize response's authMethods inside the initialize request itself, so it was always false and terminal-auth agents could never offer their login flow. In the normal session flow, auth_required failures surfaced as raw JSON-RPC error text (or were swallowed at draft preparation), and no execution path existed for terminal methods. - fix capability advertisement: auth.terminal is a client property and is now advertised via a canPresentTerminalAuth option (default true), with a wire-level regression test asserting the initialize payload - detect auth_required in the normal flow (turn failure + draft prep): publish an acp.auth.required event and replace the raw error block with an actionable sign-in message - add DaemonAcpAuthRuntime: agent-method authenticate on the warm connection (30s timeout, single-flight per agent), terminal-method login runs the verified launch spec plus the method args/env in a Bun.Terminal argv-style (no shell), streams chunked output (64KB chunks, 256KB cap), releases cached handles so the retry reconnects - add providers.startAcpAuth / writeAcpAuthInput / cancelAcpAuth routes and providers.acpAuth.changed events - add AcpAuthDialog (method selection, embedded xterm for the login TUI, retry) reachable from AcpDiagnostics and from a chat banner SDD: docs/features/acp-terminal-auth * fix(acp): harden terminal auth lifecycle Address reviewer findings on the terminal authentication PR: - reserve the agent synchronously at start so concurrent starts cannot double-launch; setup failures release the reservation and publish an error so the agent stays retryable - terminal-auth launch resolves through the managed toolchain service (npx rewrites + bin-dir PATH prepend) instead of the raw configured command, mirroring the normal session launch pipeline - keystrokes go through the Bun.Terminal object (the terminal-mode subprocess does not expose write); cancel force-kills with SIGKILL on unix; the PTY is closed when the run finishes - chunk oversized PTY buffers instead of dropping the remainder - agent-method authenticate runs in the background: the route returns immediately and the UI follows event state transitions - AcpAuthDialog: import the xterm stylesheet, buffer PTY output that arrives before the terminal mounts, pass the agent name from AcpDiagnostics - AcpAuthBanner: only the same agent's success clears the prompt; copy states that resending the message after sign-in is expected * style(ui): drop manual memoization in AcpAuthDialog React Doctor flags useCallback as unnecessary under the React Compiler; a plain function caches identically. --- apps/daemon/src/dispatch/daemonDispatcher.ts | 36 ++ .../daemon/src/host/acp-provider-execution.ts | 137 ++++++- apps/daemon/src/host/acpAuthRuntime.ts | 349 ++++++++++++++++++ .../src/terminal/daemonTerminalRuntime.ts | 2 +- apps/daemon/test/acpAuthRuntime.test.ts | 248 +++++++++++++ .../acp/acpProcessManagerCapabilities.test.ts | 72 +++- docs/features/acp-terminal-auth/plan.md | 54 +++ docs/features/acp-terminal-auth/spec.md | 74 ++++ docs/features/acp-terminal-auth/tasks.md | 29 ++ .../src/debug/runAcpDebugAction.ts | 27 +- .../src/process/acpProcessManager.ts | 16 +- packages/shared-contracts/src/events.ts | 4 +- .../src/events/providers.events.ts | 24 ++ packages/shared-contracts/src/routes.ts | 6 + .../src/routes/providers.routes.ts | 40 ++ .../types/presenters/legacy.presenters.d.ts | 4 + packages/ui/api/ProviderClient.ts | 55 ++- .../ui/settings/components/AcpAuthDialog.tsx | 296 +++++++++++++++ .../ui/settings/components/AcpDiagnostics.tsx | 176 +++++---- .../ui/src/components/chat/AcpAuthBanner.tsx | 66 ++++ packages/ui/src/pages/ChatPage.tsx | 2 + 21 files changed, 1636 insertions(+), 81 deletions(-) create mode 100644 apps/daemon/src/host/acpAuthRuntime.ts create mode 100644 apps/daemon/test/acpAuthRuntime.test.ts create mode 100644 docs/features/acp-terminal-auth/plan.md create mode 100644 docs/features/acp-terminal-auth/spec.md create mode 100644 docs/features/acp-terminal-auth/tasks.md create mode 100644 packages/ui/settings/components/AcpAuthDialog.tsx create mode 100644 packages/ui/src/components/chat/AcpAuthBanner.tsx diff --git a/apps/daemon/src/dispatch/daemonDispatcher.ts b/apps/daemon/src/dispatch/daemonDispatcher.ts index 78522a839..78af06026 100644 --- a/apps/daemon/src/dispatch/daemonDispatcher.ts +++ b/apps/daemon/src/dispatch/daemonDispatcher.ts @@ -309,6 +309,9 @@ import { providersPullOllamaModelRoute, providersImportScanRoute, providersImportApplyRoute, + providersStartAcpAuthRoute, + providersWriteAcpAuthInputRoute, + providersCancelAcpAuthRoute, modelsListRuntimeRoute, modelsTranscribeAudioRoute, sessionsResumePendingQueueRoute, @@ -341,6 +344,12 @@ type DaemonAcpSessionExecutionPort = { getAcpSessionModes?(conversationId: string): Promise; setAcpSessionMode?(conversationId: string, modeId: string): Promise; resolveAgentPermission?(requestId: string, granted: boolean): Promise; + startAcpAuth?(input: { agentId: string; workdir?: string; methodId: string }): Promise<{ + mode: "agent" | "terminal"; + runId: string | null; + }>; + writeAcpAuthInput?(runId: string, data: string): Promise; + cancelAcpAuth?(agentId: string): Promise; }; type DaemonTranslatePort = { @@ -3380,6 +3389,33 @@ export function createDaemonDispatcher( return sessionsClearAcpSessionRoute.output.parse({ cleared: true }); } + if (route === providersStartAcpAuthRoute.name) { + const input = providersStartAcpAuthRoute.input.parse(rawInput); + if (!acpSessionExecutionPort?.startAcpAuth) { + throw new Error("ACP authentication is not available in this runtime."); + } + const result = await acpSessionExecutionPort.startAcpAuth(input); + return providersStartAcpAuthRoute.output.parse(result); + } + + if (route === providersWriteAcpAuthInputRoute.name) { + const input = providersWriteAcpAuthInputRoute.input.parse(rawInput); + if (!acpSessionExecutionPort?.writeAcpAuthInput) { + throw new Error("ACP authentication is not available in this runtime."); + } + await acpSessionExecutionPort.writeAcpAuthInput(input.runId, input.data); + return providersWriteAcpAuthInputRoute.output.parse({ ok: true }); + } + + if (route === providersCancelAcpAuthRoute.name) { + const input = providersCancelAcpAuthRoute.input.parse(rawInput); + if (!acpSessionExecutionPort?.cancelAcpAuth) { + throw new Error("ACP authentication is not available in this runtime."); + } + await acpSessionExecutionPort.cancelAcpAuth(input.agentId); + return providersCancelAcpAuthRoute.output.parse({ cancelled: true }); + } + if (route === sessionsGetAcpSessionModesRoute.name) { const input = sessionsGetAcpSessionModesRoute.input.parse(rawInput); const result = await acpSessionExecutionPort?.getAcpSessionModes?.(input.sessionId); diff --git a/apps/daemon/src/host/acp-provider-execution.ts b/apps/daemon/src/host/acp-provider-execution.ts index 09d44edf0..18662d02a 100644 --- a/apps/daemon/src/host/acp-provider-execution.ts +++ b/apps/daemon/src/host/acp-provider-execution.ts @@ -7,6 +7,7 @@ import type { } from "@argos/shared/types/agent-interface"; import type * as schema from "@agentclientprotocol/sdk"; import { randomUUID } from "node:crypto"; +import path from "node:path"; import { getAcpConfigOption, getLegacyModeState, @@ -26,6 +27,9 @@ import { usageDateKey } from "./bun-session-repository"; import { createDaemonAcpPorts } from "./acpPorts"; import { createDaemonAcpSqlitePresenter } from "./daemonAcpSqlite"; import type { ToolchainService } from "./toolchains/service"; +import { DaemonAcpAuthRuntime } from "./acpAuthRuntime"; +import { resolvePtyTerminalCtor } from "../terminal/daemonTerminalRuntime"; +import { isAuthRequiredError } from "@argos/acp-runtime/protocol/acpCapabilities"; import { sessionsStatusChangedEvent } from "@argos/shared-contracts"; import { methods as acpMethods, PROTOCOL_VERSION } from "@agentclientprotocol/sdk"; import type { AcpConfigState, AcpAgentDiagnostics, AcpDebugRequest, AcpDebugRunResult } from "@argos/shared/presenter"; @@ -57,6 +61,7 @@ type PendingAcpPermission = { */ export class AcpProviderExecutionPort implements ProviderExecutionPort { private runtimePromise: Promise | null = null; + private authRuntimePromise: Promise | null = null; private activeTurns = new Map< string, { @@ -122,6 +127,74 @@ export class AcpProviderExecutionPort implements ProviderExecutionPort { return this.runtimePromise; } + /** Auth flows (agent-method authenticate + terminal login TUI). */ + private async getAuthRuntime(): Promise { + if (!this.authRuntimePromise) { + this.authRuntimePromise = (async () => { + const runtime = await this.getRuntime(); + return new DaemonAcpAuthRuntime({ + eventPublisher: this.eventPublisher, + getProcessManager: async () => runtime.processManager, + resolveLaunchSpec: async (agentId, workdir) => { + const spec = await this.configPresenter.resolveAcpLaunchSpec(agentId, workdir); + // Route the launch command through the managed toolchains + // (npx -> node npx-cli.js etc.) and prepend resolved bin dirs — + // mirroring the process manager's launch pipeline for normal + // sessions, so terminal auth works for managed runtimes too. + const rewritten = await this.deps.toolchains.resolveCommand(spec.command, spec.args ?? []); + const binDirs = this.deps.toolchains.binDirsSync(); + const env: Record = { ...spec.env }; + if (binDirs.length > 0) { + const existingKey = Object.keys(env).find((key) => key.toLowerCase() === "path"); + const key = existingKey ?? (process.platform === "win32" ? "Path" : "PATH"); + env[key] = [...binDirs, env[key] ?? ""].filter(Boolean).join(path.delimiter); + } + return { command: rewritten.command, args: rewritten.args, env }; + }, + ptyFactory: (options) => { + const ctor = resolvePtyTerminalCtor(); + return new ctor({ + cols: options.cols, + rows: options.rows, + data: (_terminal, data) => + options.onData(typeof data === "string" ? new TextEncoder().encode(data) : data), + }) as unknown as { write: (data: string | Uint8Array) => void; kill: (signal?: string) => void }; + }, + spawnPty: (argv, options) => + Bun.spawn(argv, { + cwd: options.cwd, + env: options.env, + terminal: options.terminal, + } as unknown as Parameters[1]) as unknown as { + write: (data: string | Uint8Array) => void; + kill: (signal?: string) => void; + exited: Promise; + }, + }); + })(); + } + return this.authRuntimePromise; + } + + /** Entry point for the ACP auth dialog (agent + terminal methods). */ + async startAcpAuth(input: { agentId: string; workdir?: string; methodId: string }): Promise<{ + mode: "agent" | "terminal"; + runId: string | null; + }> { + const auth = await this.getAuthRuntime(); + return await auth.start(input); + } + + async writeAcpAuthInput(runId: string, data: string): Promise { + const auth = await this.getAuthRuntime(); + auth.write(runId, data); + } + + async cancelAcpAuth(agentId: string): Promise { + const auth = await this.getAuthRuntime(); + auth.cancel({ agentId }); + } + private async getSessionRecord(conversationId: string): Promise { const runtime = await this.getRuntime(); return runtime.sessionManager.getSession(conversationId); @@ -291,15 +364,29 @@ export class AcpProviderExecutionPort implements ProviderExecutionPort { await runtime.sessionPersistence.updateWorkdir(conversationId, agent.id, persistedWorkdir); - await runtime.sessionManager.getOrCreateSession( - conversationId, - agent as never, - { - onSessionUpdate: () => {}, - onPermission: async () => ({ outcome: { outcome: "cancelled" } }), - }, - normalizedWorkdir, - ); + try { + await runtime.sessionManager.getOrCreateSession( + conversationId, + agent as never, + { + onSessionUpdate: () => {}, + onPermission: async () => ({ outcome: { outcome: "cancelled" } }), + }, + normalizedWorkdir, + ); + } catch (error) { + if (isAuthRequiredError(error)) { + // The dispatcher swallows draft-prep failures; the event is what makes + // them actionable in the UI. + this.eventPublisher.publish("acp.auth.required", { + sessionId: conversationId, + agentId, + workdir: normalizedWorkdir, + message: error instanceof Error ? error.message : String(error), + }); + } + throw error; + } try { const configState = await this.getAcpSessionConfigOptions(conversationId); @@ -600,6 +687,38 @@ export class AcpProviderExecutionPort implements ProviderExecutionPort { await this.turnSettledHandler?.(sessionId); } catch (error) { const errorMsg = error instanceof Error ? error.message : String(error); + if (isAuthRequiredError(error)) { + // Surface an actionable auth state instead of a raw JSON-RPC string. + const agentId = agent?.id ?? ""; + const handle = runtime.processManager.listProcesses().find((candidate) => candidate.agentId === agentId); + this.eventPublisher.publish("acp.auth.required", { + sessionId, + agentId, + workdir: handle?.workdir ?? null, + message: errorMsg, + }); + const friendly = `This agent requires sign-in. Open "Sign in" to authenticate (${agent?.name ?? agentId}).`; + await this.sessionRepository.setMessageError( + assistantMessageId, + [{ type: "error", content: friendly, status: "error", timestamp: Date.now() }], + JSON.stringify({ model: agent?.id ?? "", provider: "acp", authRequired: true }), + ); + this.eventPublisher.publish("chat.stream.failed", { + requestId, + sessionId, + messageId: assistantMessageId, + failedAt: Date.now(), + error: friendly, + }); + await this.sessionRepository.setSessionStatus?.(sessionId, "error"); + this.eventPublisher.publish(sessionsStatusChangedEvent.name, { + sessionId, + status: "error", + reason: "auth-required", + version: 1, + }); + return; + } await this.sessionRepository.setMessageError( assistantMessageId, blocks.length > 0 ? blocks : [{ type: "error", content: errorMsg, status: "error", timestamp: Date.now() }], diff --git a/apps/daemon/src/host/acpAuthRuntime.ts b/apps/daemon/src/host/acpAuthRuntime.ts new file mode 100644 index 000000000..da471dc0d --- /dev/null +++ b/apps/daemon/src/host/acpAuthRuntime.ts @@ -0,0 +1,349 @@ +import { randomUUID } from "node:crypto"; +import { methods as acpMethods } from "@agentclientprotocol/sdk"; +import type * as schema from "@agentclientprotocol/sdk"; +import type { IEventPublisher } from "@argos/backend-core"; +import type { AcpProcessManager } from "@argos/acp-runtime/process/acpProcessManager"; +import type { AcpAgentConfig } from "@argos/shared/presenter"; + +/** + * Terminal + agent authentication flows for ACP agents, driven from the + * daemon's real process manager (docs/features/acp-terminal-auth). + * + * - agent methods (no `type`): `authenticate` on the warm connection with a + * bounded timeout; the handle stays valid for the session retry. + * - terminal methods (`type: "terminal"`): run the agent's verified launch + * command plus the method `args` in a `Bun.Terminal` (argv-style, no + * shell), stream output to the renderer, then `release()` the agent's + * cached handles so the next attempt re-initializes with fresh credentials. + * + * The per-agent reservation is installed synchronously before any await, so + * concurrent starts cannot double-launch. One active run per agent; PTY + * output is chunked and capped; cancel kills the PTY and publishes + * `cancelled`. + */ + +const AUTH_TIMEOUT_MS = 30_000; +const OUTPUT_CHUNK_BYTES = 64 * 1024; +const OUTPUT_TOTAL_CAP_BYTES = 256 * 1024; + +export interface AcpAuthLaunchSpec { + command: string; + args: string[]; + env?: Record | null; +} + +/** Minimal PTY surface the auth runtime needs (Bun.Terminal-compatible). */ +export interface AcpAuthPty { + write: (data: string | Uint8Array) => void; + kill?: (signal?: string) => void; + close?: () => void; +} + +export interface AcpAuthRuntimeDeps { + eventPublisher: IEventPublisher; + getProcessManager: () => Promise; + resolveLaunchSpec: (agentId: string, workdir?: string) => Promise; + /** PTY constructor (Bun.Terminal-compatible); tests inject a fake. */ + ptyFactory: (options: { cols: number; rows: number; onData: (data: Uint8Array) => void }) => AcpAuthPty; + /** argv spawn bound to the PTY (Bun.spawn in production). */ + spawnPty: ( + argv: string[], + options: { cwd: string; env: Record; terminal: AcpAuthPty }, + ) => { + exited: Promise; + }; +} + +type AcpAuthState = "running" | "ready" | "error" | "cancelled"; + +interface AuthMethodLike { + id: string; + name?: string; + type?: string; + args?: string[]; + env?: Record; +} + +interface ActiveRun { + agentId: string; + workdir: string | null; + runId: string | null; + mode: "agent" | "terminal"; + state: AcpAuthState; + cancelled?: boolean; + /** Keystrokes for the terminal TUI go through the PTY object. */ + write?: (data: string | Uint8Array) => void; + /** SIGKILL on unix: interactive PTY children ignore SIGTERM. */ + kill?: () => void; + abort?: AbortController; + abortPromise?: Promise; + terminal?: AcpAuthPty; + totalBytes: number; +} + +export class DaemonAcpAuthRuntime { + private readonly activeByAgent = new Map(); + private readonly runsById = new Map(); + + constructor(private readonly deps: AcpAuthRuntimeDeps) {} + + isActive(agentId: string): boolean { + return this.activeByAgent.get(agentId)?.state === "running"; + } + + async start(input: { agentId: string; workdir?: string; methodId: string }): Promise<{ + mode: "agent" | "terminal"; + runId: string | null; + }> { + // Reserve the agent synchronously before any await: two concurrent starts + // must not both observe "no active run" and double-launch. + if (this.isActive(input.agentId)) { + throw new Error(`An authentication flow is already running for agent ${input.agentId}`); + } + const run: ActiveRun = { + agentId: input.agentId, + workdir: input.workdir ?? null, + runId: null, + mode: "agent", + state: "running", + abort: new AbortController(), + totalBytes: 0, + }; + // Created with the reservation so cancel() rejects even while earlier + // awaits (connection warmup) are still settling. + run.abortPromise = new Promise((_, reject) => { + run.abort?.signal.addEventListener("abort", () => reject(new Error("Authentication cancelled")), { + once: true, + }); + }); + this.activeByAgent.set(input.agentId, run); + + try { + const processManager = await this.deps.getProcessManager(); + const agent = { id: input.agentId, name: input.agentId } as AcpAgentConfig; + const handle = await processManager.getConnection(agent, input.workdir); + const method = (handle.authMethods ?? []).find((entry) => entry.id === input.methodId) as + | AuthMethodLike + | undefined; + if (!method) { + throw new Error(`Agent ${input.agentId} did not advertise auth method ${input.methodId}`); + } + if (method.type === "terminal") { + return await this.startTerminalFlow(run, input, method); + } + return this.startAgentFlow(run, input, method); + } catch (error) { + // Setup failures (unreachable agent, unknown method, PTY unavailable) + // must release the reservation so the agent stays retryable. + this.activeByAgent.delete(input.agentId); + this.publish({ run, state: "error", error: error instanceof Error ? error.message : String(error) }); + throw error; + } + } + + write(runId: string, data: string): void { + const run = this.runsById.get(runId); + if (!run || run.state !== "running" || !run.write) { + throw new Error(`No active terminal auth run: ${runId}`); + } + run.write(data); + } + + cancel(input: { agentId: string }): void { + const run = this.activeByAgent.get(input.agentId); + if (!run || run.state !== "running") { + return; + } + if (run.mode === "agent") { + run.abort?.abort(); + return; + } + run.cancelled = true; + run.kill?.(); + } + + private publish(payload: { + run: ActiveRun; + state: AcpAuthState; + output?: string | null; + exitCode?: number | null; + error?: string | null; + }): void { + this.deps.eventPublisher.publish("providers.acpAuth.changed", { + agentId: payload.run.agentId, + workdir: payload.run.workdir, + runId: payload.run.runId, + state: payload.state, + mode: payload.run.mode, + output: payload.output ?? null, + exitCode: payload.exitCode ?? null, + error: payload.error ?? null, + }); + } + + /** + * Agent-method authenticate. Runs in the background (like the terminal + * flow) and reports the outcome via events; the route returns immediately + * so the UI drives off state transitions instead of the response. + */ + private startAgentFlow( + run: ActiveRun, + input: { agentId: string; workdir?: string; methodId: string }, + method: AuthMethodLike, + ): { + mode: "agent"; + runId: null; + } { + this.publish({ run, state: "running" }); + + void (async () => { + try { + const processManager = await this.deps.getProcessManager(); + const handle = await processManager.getConnection( + { id: input.agentId, name: input.agentId } as AcpAgentConfig, + input.workdir, + ); + const authenticate = handle.connection.agent.request(acpMethods.agent.authenticate, { + methodId: method.id, + } as schema.AuthenticateRequest); + const timeout = new Promise((_, reject) => { + const timer = setTimeout(() => reject(new Error("Authentication timed out")), AUTH_TIMEOUT_MS); + }); + await Promise.race([authenticate, timeout, run.abortPromise!]); + run.state = "ready"; + this.publish({ run, state: "ready" }); + } catch (error) { + const cancelled = run.abort?.signal.aborted === true; + run.state = cancelled ? "cancelled" : "error"; + this.publish({ + run, + state: run.state, + error: cancelled ? "Authentication cancelled" : error instanceof Error ? error.message : String(error), + }); + } + })(); + return { mode: "agent", runId: null }; + } + + private async startTerminalFlow( + run: ActiveRun, + input: { agentId: string; workdir?: string; methodId: string }, + method: AuthMethodLike, + ): Promise<{ mode: "terminal"; runId: string | null }> { + const spec = await this.deps.resolveLaunchSpec(input.agentId, input.workdir); + const methodArgs = Array.isArray(method.args) ? method.args : []; + const methodEnv = method.env && typeof method.env === "object" ? method.env : {}; + const argv = [spec.command, ...spec.args, ...methodArgs].filter( + (part) => typeof part === "string" && part.length > 0, + ); + + run.mode = "terminal"; + run.runId = `acpauth_${randomUUID().replaceAll("-", "").slice(0, 20)}`; + this.runsById.set(run.runId, run); + this.publish({ run, state: "running" }); + + const env = buildAuthEnv(spec.env ?? {}, methodEnv); + + // Construct the PTY first and keep it: keystrokes must go through the + // terminal object (Bun's terminal-mode subprocess does not expose + // write), and the terminal must be closed when the run finishes. + const terminal = this.deps.ptyFactory({ + cols: 80, + rows: 24, + onData: (data) => this.handleOutput(run, data), + }); + run.terminal = terminal; + run.write = (data) => terminal.write(data); + run.kill = () => { + // Interactive PTY children ignore SIGTERM; force-kill like the + // integrated terminal runtime does. + terminal.kill?.(process.platform === "win32" ? undefined : "SIGKILL"); + }; + + let exitPromise: Promise; + try { + const proc = this.deps.spawnPty(argv, { + cwd: input.workdir || process.cwd(), + env, + terminal, + }); + exitPromise = proc.exited; + } catch (error) { + // Spawn/setup failure: release the reservation so the agent stays + // retryable instead of being stuck in "running" forever. + this.runsById.delete(run.runId); + try { + terminal.close?.(); + } catch { + // best-effort + } + run.state = "error"; + const message = error instanceof Error ? error.message : String(error); + this.publish({ run, state: "error", error: message }); + throw new Error(`Terminal authentication could not start: ${message}`); + } + + // Finish in the background — the route returns the runId immediately so + // the UI can subscribe to output events. + void exitPromise + .then(async (exitCode) => { + await this.finishTerminalRun(run, exitCode); + }) + .catch(async () => { + await this.finishTerminalRun(run, null); + }); + return { mode: "terminal", runId: run.runId }; + } + + private handleOutput(run: ActiveRun, data: Uint8Array): void { + if (run.state !== "running") return; + run.totalBytes += data.byteLength; + // Chunk the full buffer so no output is silently dropped; enforce the + // total cap by force-killing a runaway process. + if (run.totalBytes > OUTPUT_TOTAL_CAP_BYTES) { + this.publish({ run, state: "running", output: "\r\n[output truncated]\r\n" }); + run.kill?.(); + return; + } + for (let offset = 0; offset < data.byteLength; offset += OUTPUT_CHUNK_BYTES) { + const slice = data.subarray(offset, offset + OUTPUT_CHUNK_BYTES); + this.publish({ run, state: "running", output: new TextDecoder().decode(slice) }); + } + } + + private async finishTerminalRun(run: ActiveRun, exitCode: number | null): Promise { + this.runsById.delete(run.runId!); + try { + run.terminal?.close?.(); + } catch { + // best-effort + } + const success = exitCode === 0 && !run.cancelled; + // Drop cached handles so the next session re-initializes with fresh + // credentials (or a clean failure state). + try { + const processManager = await this.deps.getProcessManager(); + await processManager.release(run.agentId); + } catch { + // best-effort + } + run.state = run.cancelled ? "cancelled" : success ? "ready" : "error"; + this.publish({ + run, + state: run.state, + exitCode, + error: run.state === "error" ? `The agent login process exited with code ${exitCode ?? "unknown"}` : null, + }); + } +} + +function buildAuthEnv( + specEnv: Record, + methodEnv: Record, +): Record { + const env: Record = { ...process.env, ...specEnv, ...methodEnv }; + if (process.platform !== "win32") { + env.TERM = "xterm-256color"; + } + return env; +} diff --git a/apps/daemon/src/terminal/daemonTerminalRuntime.ts b/apps/daemon/src/terminal/daemonTerminalRuntime.ts index 3c1744c69..35201c117 100644 --- a/apps/daemon/src/terminal/daemonTerminalRuntime.ts +++ b/apps/daemon/src/terminal/daemonTerminalRuntime.ts @@ -94,7 +94,7 @@ interface TerminalSession { killed: boolean; } -function resolvePtyTerminalCtor(): PtyTerminalCtor { +export function resolvePtyTerminalCtor(): PtyTerminalCtor { const ctor = (Bun as unknown as { Terminal?: PtyTerminalCtor }).Terminal; if (typeof ctor !== "function") { throw new Error("Bun.Terminal is unavailable; the terminal requires Bun >= 1.4.0"); diff --git a/apps/daemon/test/acpAuthRuntime.test.ts b/apps/daemon/test/acpAuthRuntime.test.ts new file mode 100644 index 000000000..2bf1fe478 --- /dev/null +++ b/apps/daemon/test/acpAuthRuntime.test.ts @@ -0,0 +1,248 @@ +import { describe, expect, it, vi } from "bun:test"; +import { DaemonAcpAuthRuntime } from "../src/host/acpAuthRuntime"; + +/** + * Hermetic coverage for the daemon ACP auth runtime: agent-method + * authenticate (bounded, backgrounded), terminal-method PTY flow (argv, + * output streaming incl. chunking, exit handling, cancel, handle release), + * single-flight reservation, and setup-failure recovery. + */ + +const encoder = new TextEncoder(); + +/** Poll until the predicate passes (bun:test has no vi.waitFor). */ +async function waitFor(predicate: () => void, timeoutMs = 2000): Promise { + const deadline = Date.now() + timeoutMs; + let lastError: unknown = null; + while (Date.now() < deadline) { + try { + predicate(); + return; + } catch (error) { + lastError = error; + await new Promise((resolve) => setTimeout(resolve, 10)); + } + } + throw lastError ?? new Error("waitFor timed out"); +} + +const createHarness = (options?: { + authMethods?: Array>; + authenticateImpl?: () => Promise; + spawnThrows?: Error; +}) => { + const published: Array> = []; + const eventPublisher = { + publish: vi.fn((name: string, payload: Record) => { + if (name === "providers.acpAuth.changed") { + published.push(payload); + } + }), + } as any; + + const release = vi.fn(async () => undefined); + const authenticate = vi.fn(options?.authenticateImpl ?? (async () => undefined)); + const processManager = { + getConnection: vi.fn(async () => ({ + authMethods: options?.authMethods ?? [{ id: "agent-login", name: "Agent Login" }], + connection: { + agent: { + request: vi.fn(async (_method: string, payload: unknown) => { + void payload; + return await authenticate(); + }), + }, + }, + })), + release, + } as any; + + const spawned: Array<{ argv: string[]; env: Record }> = []; + const terminalWrites: string[] = []; + const killArgs: Array = []; + let resolveExit: ((code: number) => void) | null = null; + const exited = new Promise((resolve) => { + resolveExit = resolve; + }); + let dataHandler: ((data: Uint8Array) => void) | null = null; + + const deps = { + eventPublisher, + getProcessManager: async () => processManager, + resolveLaunchSpec: vi.fn(async () => ({ command: "mcode", args: ["acp"], env: { SPEC_VAR: "1" } })), + ptyFactory: (opts: { onData: (data: Uint8Array) => void }) => { + dataHandler = opts.onData; + return { + write: (data: string | Uint8Array) => { + terminalWrites.push(typeof data === "string" ? data : new TextDecoder().decode(data)); + }, + kill: (signal?: string) => killArgs.push(signal), + close: vi.fn(() => undefined), + }; + }, + spawnPty: vi.fn((argv: string[], o: { env: Record }) => { + if (options?.spawnThrows) throw options.spawnThrows; + spawned.push({ argv, env: o.env }); + return { exited }; + }), + }; + + const auth = new DaemonAcpAuthRuntime(deps as any); + return { + auth, + published, + release, + authenticate, + spawned, + terminalWrites, + killArgs, + emitExit: (code: number) => resolveExit?.(code), + emitData: (text: string) => dataHandler?.(encoder.encode(text)), + }; +}; + +describe("DaemonAcpAuthRuntime", () => { + it("authenticates agent methods in the background and reports ready via events", async () => { + const harness = createHarness(); + const result = await harness.auth.start({ agentId: "my-agent", methodId: "agent-login" }); + + expect(result.mode).toBe("agent"); + expect(result.runId).toBeNull(); + await waitFor(() => expect(harness.published.map((entry) => entry.state)).toContain("ready")); + expect(harness.authenticate).toHaveBeenCalled(); + expect(harness.release).not.toHaveBeenCalled(); + }); + + it("surfaces agent-method failures and keeps the agent retryable", async () => { + const harness = createHarness({ + authenticateImpl: async () => { + throw new Error("bad credentials"); + }, + }); + const result = await harness.auth.start({ agentId: "my-agent", methodId: "agent-login" }); + + expect(result.mode).toBe("agent"); + await waitFor(() => expect(harness.published.map((entry) => entry.state)).toContain("error")); + expect(harness.published.at(-1)?.error).toContain("bad credentials"); + }); + + it("reserves the agent synchronously so concurrent starts cannot double-launch", async () => { + const harness = createHarness({ authenticateImpl: () => new Promise(() => {}) }); + const first = harness.auth.start({ agentId: "my-agent", methodId: "agent-login" }); + // The reservation is installed synchronously: the immediate second start + // must reject even before the first flow finished. + await expect(harness.auth.start({ agentId: "my-agent", methodId: "agent-login" })).rejects.toThrow( + "already running", + ); + + harness.auth.cancel({ agentId: "my-agent" }); + await first; + await waitFor(() => expect(harness.published.map((entry) => entry.state)).toContain("cancelled")); + }); + + it("runs terminal methods with launch spec + method args, no shell", async () => { + const harness = createHarness({ + authMethods: [ + { id: "term-1", name: "Terminal Login", type: "terminal", args: ["--login"], env: { TOKEN_MODE: "device" } }, + ], + }); + + const result = await harness.auth.start({ agentId: "my-agent", workdir: "/tmp/ws", methodId: "term-1" }); + expect(result.mode).toBe("terminal"); + expect(result.runId).toBeTruthy(); + + expect(harness.spawned).toHaveLength(1); + expect(harness.spawned[0]!.argv).toEqual(["mcode", "acp", "--login"]); + + harness.emitExit(0); + await waitFor(() => expect(harness.published.map((entry) => entry.state)).toContain("ready")); + expect(harness.release).toHaveBeenCalledWith("my-agent"); + const last = harness.published.at(-1)!; + expect(last.exitCode).toBe(0); + expect(last.error).toBeNull(); + }); + + it("streams PTY output through events", async () => { + const harness = createHarness({ + authMethods: [{ id: "term-1", name: "Terminal Login", type: "terminal", args: ["--login"] }], + }); + await harness.auth.start({ agentId: "my-agent", methodId: "term-1" }); + + harness.emitData("open https://example.com/device"); + await waitFor(() => { + const outputs = harness.published.filter((entry) => typeof entry.output === "string"); + expect(outputs.length).toBeGreaterThan(0); + }); + const outputEvent = harness.published.find((entry) => typeof entry.output === "string"); + expect(outputEvent?.output).toContain("https://example.com/device"); + }); + + it("chunks oversized PTY buffers instead of dropping the remainder", async () => { + const harness = createHarness({ + authMethods: [{ id: "term-1", name: "Terminal Login", type: "terminal", args: ["--login"] }], + }); + await harness.auth.start({ agentId: "my-agent", methodId: "term-1" }); + + // 64KB + 1 byte: two output events, nothing dropped. + harness.emitData("a".repeat(64 * 1024 + 1)); + await waitFor(() => { + const outputs = harness.published.filter((entry) => typeof entry.output === "string"); + expect(outputs.length).toBe(2); + }); + const total = harness.published + .filter((entry) => typeof entry.output === "string") + .reduce((sum, entry) => sum + (entry.output as string).length, 0); + expect(total).toBe(64 * 1024 + 1); + }); + + it("reports an error when the login process exits non-zero", async () => { + const harness = createHarness({ + authMethods: [{ id: "term-1", name: "Terminal Login", type: "terminal", args: ["--login"] }], + }); + await harness.auth.start({ agentId: "my-agent", methodId: "term-1" }); + + harness.emitExit(1); + await waitFor(() => { + expect(harness.published.map((entry) => entry.state)).toContain("error"); + }); + expect(harness.published.at(-1)?.error).toContain("exited with code 1"); + }); + + it("force-kills and reports cancelled terminal runs", async () => { + const harness = createHarness({ + authMethods: [{ id: "term-1", name: "Terminal Login", type: "terminal", args: ["--login"] }], + }); + const startPromise = harness.auth.start({ agentId: "my-agent", methodId: "term-1" }); + await waitFor(() => expect(harness.spawned).toHaveLength(1)); + + harness.auth.cancel({ agentId: "my-agent" }); + await startPromise; + harness.emitExit(0); + + await waitFor(() => { + expect(harness.published.map((entry) => entry.state)).toContain("cancelled"); + }); + expect(harness.killArgs).toContain(process.platform === "win32" ? undefined : "SIGKILL"); + }); + + it("releases the run when PTY setup fails so the agent stays retryable", async () => { + const harness = createHarness({ + authMethods: [{ id: "term-1", name: "Terminal Login", type: "terminal", args: ["--login"] }], + spawnThrows: new Error("cwd missing"), + }); + + await expect(harness.auth.start({ agentId: "my-agent", methodId: "term-1" })).rejects.toThrow( + "Terminal authentication could not start", + ); + await waitFor(() => expect(harness.published.map((entry) => entry.state)).toContain("error")); + + // The reservation is released: a retry is accepted. + const retry = harness.auth.start({ agentId: "my-agent", methodId: "term-1" }); + await expect(retry).rejects.toThrow("Terminal authentication could not start"); + }); + + it("rejects unknown method ids", async () => { + const harness = createHarness(); + await expect(harness.auth.start({ agentId: "my-agent", methodId: "nope" })).rejects.toThrow("did not advertise"); + }); +}); diff --git a/apps/desktop/test/main/presenter/llmProviderPresenter/acp/acpProcessManagerCapabilities.test.ts b/apps/desktop/test/main/presenter/llmProviderPresenter/acp/acpProcessManagerCapabilities.test.ts index 162d0ec97..84c1cbd60 100644 --- a/apps/desktop/test/main/presenter/llmProviderPresenter/acp/acpProcessManagerCapabilities.test.ts +++ b/apps/desktop/test/main/presenter/llmProviderPresenter/acp/acpProcessManagerCapabilities.test.ts @@ -26,13 +26,18 @@ const sdkMock = vi.hoisted(() => ({ }, authMethods: [{ id: "terminal", name: "Terminal", type: "terminal" }], }, + // Captured initialize/authenticate request payloads for wire assertions. + requests: [] as Array, })); vi.mock("@agentclientprotocol/sdk", () => { const connection = { closed: new Promise(() => {}), agent: { - request: vi.fn<(...args: any[]) => any>(async () => sdkMock.initializeResponse), + request: vi.fn<(...args: any[]) => any>(async (...args: any[]) => { + sdkMock.requests.push(args[1]); + return sdkMock.initializeResponse; + }), notify: vi.fn<(...args: any[]) => any>(async () => undefined), }, }; @@ -46,6 +51,7 @@ vi.mock("@agentclientprotocol/sdk", () => { methods: { agent: { initialize: "initialize", + authenticate: "authenticate", session: { new: "session/new", load: "session/load", @@ -160,4 +166,68 @@ describe("AcpProcessManager initialized capabilities", () => { authLogout: false, }); }); + + it("advertises clientCapabilities.auth.terminal on the initialize wire request", async () => { + const { AcpProcessManager } = await import("@argos/acp-runtime"); + const manager = new AcpProcessManager({ + providerId: "acp", + ports: createAcpTestPorts(), + resolveLaunchSpec: vi.fn<(...args: any[]) => any>(), + }); + const child = new MockChild(); + vi.spyOn<(...args: any[]) => any>(manager as any, "spawnAgentProcess").mockResolvedValue(child); + + const requestIndex = sdkMock.requests.length; + await (manager as any).spawnProcessOnce( + { id: "agent-1", name: "Agent One", command: "agent" }, + "/tmp/workspace", + { + agentId: "agent-1", + source: "manual", + distributionType: "manual", + command: "agent", + args: [], + env: {}, + }, + "manual:agent", + ); + + // The client can present terminal auth (PTY + embedded terminal), so the + // capability must be advertised unconditionally — agents gate their + // authMethods on it. Regression: it used to be computed from the + // initialize response (always undefined at that point), so it was never + // advertised. + const initRequest = sdkMock.requests[requestIndex] as { clientCapabilities?: { auth?: { terminal?: boolean } } }; + expect(initRequest.clientCapabilities?.auth).toEqual({ terminal: true }); + }); + + it("omits auth.terminal when the host cannot present terminal flows", async () => { + const { AcpProcessManager } = await import("@argos/acp-runtime"); + const manager = new AcpProcessManager({ + providerId: "acp", + ports: createAcpTestPorts(), + resolveLaunchSpec: vi.fn<(...args: any[]) => any>(), + canPresentTerminalAuth: false, + }); + const child = new MockChild(); + vi.spyOn<(...args: any[]) => any>(manager as any, "spawnAgentProcess").mockResolvedValue(child); + + const requestIndex = sdkMock.requests.length; + await (manager as any).spawnProcessOnce( + { id: "agent-1", name: "Agent One", command: "agent" }, + "/tmp/workspace", + { + agentId: "agent-1", + source: "manual", + distributionType: "manual", + command: "agent", + args: [], + env: {}, + }, + "manual:agent", + ); + + const initRequest = sdkMock.requests[requestIndex] as { clientCapabilities?: { auth?: { terminal?: boolean } } }; + expect(initRequest.clientCapabilities?.auth).toBeUndefined(); + }); }); diff --git a/docs/features/acp-terminal-auth/plan.md b/docs/features/acp-terminal-auth/plan.md new file mode 100644 index 000000000..8787262b1 --- /dev/null +++ b/docs/features/acp-terminal-auth/plan.md @@ -0,0 +1,54 @@ +# Plan: ACP terminal authentication + +## 1. acp-runtime + +- `packages/acp-runtime/src/process/acpProcessManager.ts`: replace the pre-init + `clientSupportsTerminalAuth(handleSeed.authMethods)` read with a constructor option + `canPresentTerminalAuth` (default `true`); pass `enableTerminalAuth: enableTerminal && + canPresentTerminalAuth`. +- `packages/acp-runtime/src/debug/runAcpDebugAction.ts`: `computeAcpDiagnostics` normalization + carries terminal-method `args: string[]` and `env: Record`. + +## 2. Contracts + +- `packages/shared/presenter` ACP diagnostics type: methods gain optional `args`/`env`. +- `packages/shared-contracts/src/routes/providers.routes.ts`: + - `providers.startAcpAuth` `{ agentId, workdir?, methodId }` → `{ mode: "agent" | "terminal", + runId?: string }` + - `providers.writeAcpAuthInput` `{ runId, data }` → `{ ok: true }` + - `providers.cancelAcpAuth` `{ agentId }` → `{ cancelled: true }` +- Events: `providers.acpAuth.changed` `{ agentId, workdir?, runId?, state: + "running"|"ready"|"error"|"cancelled", output?, exitCode?, error? }` and + `acp.auth.required` `{ sessionId?, agentId, workdir?, methods, message }`. + +## 3. Daemon + +- `apps/daemon/src/host/acpAuthRuntime.ts` (new): `DaemonAcpAuthRuntime` with + `start/ write/ cancel`, single-flight per agent, PTY runner (injectable terminal ctor + + spawn for tests), output chunking/cap, authenticate timeout race, `processManager.release` + after terminal success/cancel. +- `apps/daemon/src/host/acp-provider-execution.ts`: + - construct/expose the auth runtime; + - `isAuthRequiredError` checks in `runTurn` catch and `prepareAcpSession`: publish + `acp.auth.required` with methods from the bound handle (when available) and annotate the + error message. +- `apps/daemon/src/dispatch/daemonDispatcher.ts`: route handlers + port type additions. + +## 4. UI + +- `packages/ui/api`: extend the provider client (or add `AcpAuthClient`) with + `startAcpAuth / writeAcpAuthInput / cancelAcpAuth` + event subscription. +- `packages/ui/settings/components/AcpAuthDialog.tsx` (new): method selection, agent-method + progress, terminal output via xterm (input line for TUI interaction), retry on ready. +- `AcpDiagnostics.tsx`: terminal methods route into the dialog instead of the failing debug + `authenticate` RPC. +- Chat: banner on `acp.auth.required` offering "Sign in" (opens the dialog with the agent + context). + +## 5. Tests + +- Wire-level: a fake agent asserting `initialize` carries `clientCapabilities.auth.terminal`. +- Auth runtime: terminal flow success (fake PTY + spawn), failure exit code, cancel, output cap; + agent-method authenticate timeout + success; single-flight. +- Execution port: `auth_required` error → `acp.auth.required` published + annotated error block. +- Dispatcher: new routes. diff --git a/docs/features/acp-terminal-auth/spec.md b/docs/features/acp-terminal-auth/spec.md new file mode 100644 index 000000000..378cea4ed --- /dev/null +++ b/docs/features/acp-terminal-auth/spec.md @@ -0,0 +1,74 @@ +# Spec: ACP terminal authentication during agent onboarding + +Inspired by ThinkInAIXYZ/deepchat#2144 (fixed by #2195), re-implemented for Argos' daemon-owned +ACP runtime. Scope decision: the auth experience surfaces in **chat (error state) and settings**; +full proactive onboarding interception is out of scope for v1. + +## Problem + +Agents that require login (e.g. MiniMax Code, `mcode acp`) advertise `authMethods` from +`initialize` — but only if the client declares `clientCapabilities.auth.terminal`. In Argos: + +1. **The capability is never advertised (bug).** `acpProcessManager.ts` computes + `enableTerminalAuth: clientSupportsTerminalAuth(handleSeed.authMethods)` when *building* the + initialize request, but `handleSeed.authMethods` is only populated *from that request's + response*. It is always `undefined` at that point, so `auth.terminal` is never advertised and + terminal-auth agents never offer their login flow. +2. **`auth_required` in the normal flow is raw.** `session/new` failures surface as a raw + JSON-RPC error block in chat (or are swallowed entirely at draft preparation); the renderer is + never told that signing in would fix it, and `authMethods` only exist in the Settings + diagnostics surface. +3. **No auth execution in the normal flow.** `authenticate` exists only as a debug action; + terminal methods (run the agent's login TUI) have no implementation at all. + +## Goals + +- Advertise `clientCapabilities.auth.terminal` whenever the client can present the flow (it can: + `Bun.Terminal` + xterm exist on every Argos surface), with a wire-level test. +- Detect `auth_required` in the normal session flow and publish a typed event carrying the + agent's auth methods; annotate the chat error instead of leaking the JSON-RPC string. +- Provide auth flows over new `providers.*` routes: + - **agent methods** (no `type`): `authenticate` on the warm connection with a bounded timeout + and single-flight per agent; + - **terminal methods** (`type: "terminal"`): run the agent's verified launch spec plus the + method's `args` (and `env`) in a `Bun.Terminal` — argv-style, no shell — stream output to the + renderer, then `release()` the agent's cached handles so the next attempt re-initializes with + fresh credentials; + - **env_var methods**: instructions only (already rendered by `AcpDiagnostics`). +- Cancel, failure, and reconnect handling: cooperative cancel kills the PTY and releases handles; + output is chunked (64 KB) and capped (256 KB); authenticate races a timeout; runs are + single-flight per agent. +- An auth dialog usable from both the chat error state and ACP settings, reusing the existing + xterm component for terminal output and `AcpDiagnostics`-style method rendering. + +## Non-goals (follow-ups) + +- Proactive onboarding interception before the first session (upstream's full flow). +- `auth.logout` promotion into the dialog (capability already surfaced in diagnostics). +- Web (browser) terminal-auth output — the dialog uses the existing xterm component; if the + browser runtime cannot mount it, the dialog degrades to status text. + +## Decisions + +- **D1 — Capability advertisement is a client property.** `auth.terminal` means "this client can + present a terminal login", not "this agent has terminal methods". Advertise + `enableTerminalAuth: true` whenever `enableTerminal` is on, controlled by a process-manager + option (`canPresentTerminalAuth`, default `true`). The pre-init `clientSupportsTerminalAuth` + read is deleted; the helper stays exported for tests. +- **D2 — Auth runtime lives next to the execution port** (`apps/daemon/src/host/acpAuthRuntime.ts`), + driving `runtime.processManager` (`getConnection` for warm handles, `release(agentId)` for + reconnect). Events (`providers.acpAuth.changed`) carry state transitions and PTY output chunks. +- **D3 — Terminal argv is launch-spec + method args.** `argv = [spec.command, ...spec.args, + ...(method.args ?? [])]` spawned argv-style (no shell) with `method.env` merged over the spec + env, `TERM=xterm-256color`, cwd = workdir. Exit code 0 → success; anything else → error with + the tail of the output. +- **D4 — Bounded lifecycle.** Authenticate races a 30 s timeout; PTY output is chunked at 64 KB + with a 256 KB total cap (truncation notice); one active run per agent; cancel kills the PTY and + publishes `cancelled`. +- **D5 — Inspection reuses diagnostics.** The dialog fetches methods via the existing + `providers.getAcpAgentDiagnostics` route (extended to carry terminal `args`/`env`); no separate + inspect route. +- **D6 — Chat surfacing.** `AcpProviderExecutionPort` publishes `acp.auth.required` + (sessionId, agentId, workdir, methods) when `isAuthRequiredError` matches a turn or draft + preparation failure, and prefixes the error block so the raw JSON-RPC text is never the whole + story. The chat banner offers a "Sign in" button opening the dialog. diff --git a/docs/features/acp-terminal-auth/tasks.md b/docs/features/acp-terminal-auth/tasks.md new file mode 100644 index 000000000..2fadf5a82 --- /dev/null +++ b/docs/features/acp-terminal-auth/tasks.md @@ -0,0 +1,29 @@ +# Tasks: ACP terminal authentication + +- [x] T1 Fix `auth.terminal` advertisement (`canPresentTerminalAuth` process-manager option, + delete the pre-init response-dependent read) +- [x] T2 Diagnostics: carry terminal-method `args`/`env` (acp-runtime + shared types + route + schema; also fixes `name` being stripped by the output schema) +- [x] T3 Contracts: `providers.startAcpAuth` / `writeAcpAuthInput` / `cancelAcpAuth` + + `providers.acpAuth.changed` + `acp.auth.required` events +- [x] T4 Daemon: `DaemonAcpAuthRuntime` (agent-method authenticate with 30s timeout, + terminal PTY runner argv-style with method args/env, 64KB chunks + 256KB cap, + single-flight per agent, cancel, handle release for reconnect) +- [x] T5 Daemon: auth-required detection + `acp.auth.required` event in the turn failure path + (annotated error block) and draft preparation +- [x] T6 Dispatcher: route handlers + port types +- [x] T7 UI: `AcpAuthDialog` (method selection, agent progress, embedded xterm for terminal + login TUI with input, retry on ready) + ProviderClient methods/subscriptions +- [x] T8 UI: `AcpDiagnostics` terminal methods open the dialog; chat `AcpAuthBanner` on + `acp.auth.required` +- [x] T9 Tests: wire-level capability advertisement (incl. opt-out), auth runtime flows (7), + all existing suites green +- [x] T10 `bun run format` + `bun run lint` + `bun run typecheck` + `bun run test` + +## Verification results + +- Desktop: capability wire test asserts `clientCapabilities.auth.terminal === true` on the + initialize request (and absence with `canPresentTerminalAuth: false`); `test:main` + 1737+ passed. +- Daemon: 405 + 7 auth-runtime tests pass; `tsc --noEmit` clean. +- `bun run lint`: all architecture guards + oxlint clean (419 routes). diff --git a/packages/acp-runtime/src/debug/runAcpDebugAction.ts b/packages/acp-runtime/src/debug/runAcpDebugAction.ts index 86ef9b3a0..83055f66e 100644 --- a/packages/acp-runtime/src/debug/runAcpDebugAction.ts +++ b/packages/acp-runtime/src/debug/runAcpDebugAction.ts @@ -932,7 +932,14 @@ export function computeAcpDiagnostics( .find((candidate) => candidate.agentId === agentId && (!workdir || candidate.workdir === workdir)); const snapshot = handle?.capabilitySnapshot; const authMethods = (handle?.authMethods ?? []).map((method) => { - const untyped = method as { name?: unknown; type?: unknown; vars?: unknown; link?: unknown }; + const untyped = method as { + name?: unknown; + type?: unknown; + vars?: unknown; + link?: unknown; + args?: unknown; + env?: unknown; + }; const nameValue = typeof untyped.name === "string" ? untyped.name : undefined; const typeValue = typeof untyped.type === "string" ? untyped.type : undefined; const base: { @@ -941,6 +948,8 @@ export function computeAcpDiagnostics( type?: string; vars?: Array<{ name: string; label?: string; secret?: boolean; optional?: boolean }>; link?: string | null; + args?: string[]; + env?: Record; } = { id: method.id, name: nameValue, @@ -962,6 +971,22 @@ export function computeAcpDiagnostics( base.link = untyped.link; } } + if (typeValue === "terminal") { + // The client runs the agent binary with these args/env in a PTY for the + // login TUI (ACP `AuthMethodTerminal`). + if (Array.isArray(untyped.args)) { + base.args = untyped.args.filter((arg): arg is string => typeof arg === "string"); + } + if (untyped.env && typeof untyped.env === "object") { + const env: Record = {}; + for (const [key, value] of Object.entries(untyped.env as Record)) { + if (typeof value === "string") { + env[key] = value; + } + } + base.env = env; + } + } return base; }); const debugEvents = processManager.getDebugEvents(agentId); diff --git a/packages/acp-runtime/src/process/acpProcessManager.ts b/packages/acp-runtime/src/process/acpProcessManager.ts index 68a8f9b58..4de198435 100644 --- a/packages/acp-runtime/src/process/acpProcessManager.ts +++ b/packages/acp-runtime/src/process/acpProcessManager.ts @@ -23,7 +23,6 @@ import { import { buildCapabilitySnapshot, buildClientCapabilities, - clientSupportsTerminalAuth, type AcpCapabilitySnapshot, } from "../protocol/acpCapabilities"; import { AcpFsHandler } from "./acpFsHandler"; @@ -80,6 +79,13 @@ interface AcpProcessManagerOptions { getAgentState?: (agentId: string) => Promise; getNpmRegistry?: () => Promise; getUvRegistry?: () => Promise; + /** + * Whether this client can present terminal-based auth flows (a PTY plus an + * embeddable terminal UI). Advertised as `clientCapabilities.auth. + * terminal` during initialize. Agents gate their `authMethods` on this, so + * it must not depend on anything only known after initialize. Default true. + */ + canPresentTerminalAuth?: boolean; } export type SessionNotificationHandler = (notification: schema.SessionNotification) => void; @@ -201,6 +207,7 @@ export class AcpProcessManager implements AgentProcessManager Promise; private readonly getNpmRegistry?: () => Promise; private readonly getUvRegistry?: () => Promise; + private readonly canPresentTerminalAuth: boolean; private readonly handles = new Map(); private readonly boundHandles = new Map(); private readonly pendingHandles = new Map>(); @@ -235,6 +242,7 @@ export class AcpProcessManager implements AgentProcessManager this.ports.paths.tempDir()); } @@ -839,7 +847,11 @@ export class AcpProcessManager implements AgentProcessManager; }>; authRequired: boolean; authRequiredMessage?: string | null; diff --git a/packages/ui/api/ProviderClient.ts b/packages/ui/api/ProviderClient.ts index d03fa63af..a41d8376f 100644 --- a/packages/ui/api/ProviderClient.ts +++ b/packages/ui/api/ProviderClient.ts @@ -1,8 +1,16 @@ import type { ArgosBridge } from "@argos/shared-contracts/bridge"; -import { providersChangedEvent, providersOllamaPullProgressEvent } from "@argos/shared-contracts/events"; +import { + acpAuthChangedEvent, + acpAuthRequiredEvent, + providersChangedEvent, + providersOllamaPullProgressEvent, +} from "@argos/shared-contracts/events"; import { providersAddRoute, providersGetAcpAgentDiagnosticsRoute, + providersStartAcpAuthRoute, + providersWriteAcpAuthInputRoute, + providersCancelAcpAuthRoute, providersGetAcpProcessConfigOptionsRoute, providersGetRateLimitStatusRoute, providersImportApplyRoute, @@ -159,6 +167,18 @@ export function createProviderClient(bridge: ArgosBridge = getArgosBridge()) { return result.diagnostics; } + async function startAcpAuth(input: { agentId: string; workdir?: string; methodId: string }) { + return await bridge.invoke(providersStartAcpAuthRoute.name, input); + } + + async function writeAcpAuthInput(runId: string, data: string) { + return await bridge.invoke(providersWriteAcpAuthInputRoute.name, { runId, data }); + } + + async function cancelAcpAuth(agentId: string) { + return await bridge.invoke(providersCancelAcpAuthRoute.name, { agentId }); + } + async function scanProviderImports() { return await bridge.invoke(providersImportScanRoute.name, {}); } @@ -216,6 +236,34 @@ export function createProviderClient(bridge: ArgosBridge = getArgosBridge()) { return bridge.on(providersOllamaPullProgressEvent.name, listener); } + /** Terminal/agent auth flow state transitions + PTY output chunks. */ + function onAcpAuthChanged( + listener: (payload: { + agentId: string; + workdir?: string | null; + runId?: string | null; + state: "running" | "ready" | "error" | "cancelled"; + mode?: "agent" | "terminal" | null; + output?: string | null; + exitCode?: number | null; + error?: string | null; + }) => void, + ) { + return bridge.on(acpAuthChangedEvent.name, listener); + } + + /** Raised when a normal-flow ACP call fails with `auth_required`. */ + function onAcpAuthRequired( + listener: (payload: { + sessionId?: string | null; + agentId: string; + workdir?: string | null; + message: string; + }) => void, + ) { + return bridge.on(acpAuthRequiredEvent.name, listener); + } + return { getProviders, getProviderSummaries, @@ -236,6 +284,11 @@ export function createProviderClient(bridge: ArgosBridge = getArgosBridge()) { getAcpProcessConfigOptions, runAcpDebugAction, getAcpAgentDiagnostics, + startAcpAuth, + writeAcpAuthInput, + cancelAcpAuth, + onAcpAuthChanged, + onAcpAuthRequired, getKeyStatus, refreshProviderDb, updateProviderRateLimit, diff --git a/packages/ui/settings/components/AcpAuthDialog.tsx b/packages/ui/settings/components/AcpAuthDialog.tsx new file mode 100644 index 000000000..0e4fe6e1a --- /dev/null +++ b/packages/ui/settings/components/AcpAuthDialog.tsx @@ -0,0 +1,296 @@ +import { useEffect, useRef, useState } from "react"; +import { Icon } from "@iconify/react"; +import { Terminal } from "@xterm/xterm"; +import "@xterm/xterm/css/xterm.css"; +import { Button } from "#shadcn/components/ui/button"; +import { Badge } from "#shadcn/components/ui/badge"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "#shadcn/components/ui/dialog"; +import { createProviderClient } from "#api/ProviderClient"; +import type { AcpAgentDiagnostics } from "@argos/shared/presenter"; + +const providerClient = createProviderClient(); + +export interface AcpAuthDialogRequest { + open: boolean; + agentId: string; + agentName: string; + workdir?: string | null; + /** Called when the flow reaches "ready" so the caller can retry. */ + onAuthenticated?: () => void; + onOpenChange: (open: boolean) => void; +} + +type FlowState = "select" | "running" | "ready" | "error" | "cancelled"; + +/** + * Sign-in dialog for ACP agents. Agent methods call `authenticate` on the + * daemon; terminal methods run the agent's login TUI in an embedded PTY + * (xterm). Env-var methods render setup instructions. + */ +export default function AcpAuthDialog({ + open, + agentId, + agentName, + workdir, + onAuthenticated, + onOpenChange, +}: AcpAuthDialogRequest) { + const [diagnostics, setDiagnostics] = useState(null); + const [loading, setLoading] = useState(false); + const [flowState, setFlowState] = useState("select"); + const [activeMethod, setActiveMethod] = useState<{ id: string; name: string; mode: "agent" | "terminal" } | null>( + null, + ); + const [error, setError] = useState(null); + const [runId, setRunId] = useState(null); + const terminalRef = useRef(null); + const xtermRef = useRef(null); + + const reset = () => { + setFlowState("select"); + setActiveMethod(null); + setError(null); + setRunId(null); + }; + + // Load methods when the dialog opens. + useEffect(() => { + if (!open || !agentId) return; + queueMicrotask(() => { + setLoading(true); + reset(); + void providerClient + .getAcpAgentDiagnostics(agentId, workdir ?? null) + .then((diagnostics) => setDiagnostics(diagnostics)) + .catch((error) => setError(error instanceof Error ? error.message : String(error))) + .finally(() => setLoading(false)); + }); + }, [open, agentId, workdir, reset]); + + // Auth state transitions + PTY output. Output that arrives before the + // embedded terminal is mounted is buffered and flushed on open — the + // daemon does not replay it. + const pendingOutputRef = useRef([]); + useEffect(() => { + if (!open) return; + const off = providerClient.onAcpAuthChanged((payload) => { + if (payload.agentId !== agentId) return; + if (payload.runId && runId && payload.runId !== runId) return; + if (payload.output) { + if (xtermRef.current) { + xtermRef.current.write(payload.output); + } else { + pendingOutputRef.current.push(payload.output); + } + } + if (payload.state === "ready") { + setFlowState("ready"); + onAuthenticated?.(); + } else if (payload.state === "error") { + setFlowState("error"); + setError(payload.error ?? "Authentication failed"); + } else if (payload.state === "cancelled") { + setFlowState("select"); + } + }); + return off; + }, [open, agentId, runId, onAuthenticated]); + + // Embedded terminal lifecycle for terminal methods. + useEffect(() => { + if (flowState !== "running" || activeMethod?.mode !== "terminal" || !terminalRef.current) return; + const terminal = new Terminal({ + convertEol: false, + fontSize: 12, + cursorBlink: true, + }); + terminal.open(terminalRef.current); + // Flush output that arrived before the terminal existed. + for (const chunk of pendingOutputRef.current) { + terminal.write(chunk); + } + pendingOutputRef.current = []; + terminal.onData((data) => { + if (runId) void providerClient.writeAcpAuthInput(runId, data); + }); + terminal.focus(); + xtermRef.current = terminal; + return () => { + terminal.dispose(); + xtermRef.current = null; + }; + }, [flowState, activeMethod, runId]); + + const startMethod = async (methodId: string, name: string) => { + setLoading(true); + setError(null); + try { + const result = await providerClient.startAcpAuth({ agentId, workdir: workdir ?? undefined, methodId }); + setActiveMethod({ id: methodId, name, mode: result.mode }); + setRunId(result.runId); + setFlowState("running"); + } catch (err) { + setError(err instanceof Error ? err.message : String(err)); + setFlowState("error"); + } + setLoading(false); + }; + + const methods = diagnostics?.authMethods ?? []; + + const handleClose = (nextOpen: boolean) => { + if (!nextOpen && flowState === "running" && agentId) { + void providerClient.cancelAcpAuth(agentId).catch(() => undefined); + } + onOpenChange(nextOpen); + }; + + return ( + + + + + + Sign in to {agentName} + + + This agent requires authentication before it can be used. Choose a method to continue. + + + + {loading && flowState === "select" ? ( +
+ Loading methods… +
+ ) : null} + + {flowState === "select" ? ( +
+ {methods.length === 0 && !loading ? ( +
+ The agent did not advertise any authentication methods. Check the agent's connection status in ACP + settings. +
+ ) : null} + {methods.map((method) => { + if (method.type === "env_var") { + return ( +
+
{method.name ?? "Environment variables"}
+ {method.vars?.length ? ( +
+ Set{" "} + {method.vars.map((v) => ( + + {v.name} + + ))}{" "} + in your environment, then re-check the connection. +
+ ) : null} + {method.link ? ( + + Get credentials + + ) : null} +
+ ); + } + const mode = method.type === "terminal" ? "terminal" : "agent"; + return ( + + ); + })} +
+ ) : null} + + {flowState === "running" && activeMethod?.mode === "agent" ? ( +
+ + Waiting for {agentName} to confirm authentication… +
+ ) : null} + + {flowState === "running" && activeMethod?.mode === "terminal" ? ( +
+
+ + + Complete the login in the terminal below + + running +
+
+
+ ) : null} + + {flowState === "ready" ? ( +
+ + Authenticated. You can retry the conversation now. +
+ ) : null} + + {flowState === "error" || error ? ( +
+ + {error ?? "Authentication failed"} +
+ ) : null} + + + {flowState === "running" ? ( + + ) : null} + {flowState === "ready" ? ( + + ) : flowState !== "running" ? ( + + ) : null} + + +
+ ); +} diff --git a/packages/ui/settings/components/AcpDiagnostics.tsx b/packages/ui/settings/components/AcpDiagnostics.tsx index 375931639..1ba870674 100644 --- a/packages/ui/settings/components/AcpDiagnostics.tsx +++ b/packages/ui/settings/components/AcpDiagnostics.tsx @@ -1,5 +1,6 @@ import { useState, useEffect, useRef } from "react"; import { Icon } from "@iconify/react"; +import AcpAuthDialog from "#settings/components/AcpAuthDialog"; import { Button } from "#shadcn/components/ui/button"; import { Badge } from "#shadcn/components/ui/badge"; import { Input } from "#shadcn/components/ui/input"; @@ -419,6 +420,9 @@ export default function AcpDiagnostics({ options={authMethodOptions} loading={loading} logoutEnabled={Boolean(caps?.authLogout)} + agentId={agentId} + agentName={diagnostics.agentName ?? agentName} + workdir={diagnostics.workdir} onRunAction={runAction} /> @@ -591,77 +595,119 @@ const AuthMethodsSection = ({ options, loading, logoutEnabled, + agentId, + agentName, + workdir, onRunAction, }: { options: Array<{ method: AuthMethod; label: string }>; loading: boolean; logoutEnabled: boolean; + agentId: string; + agentName: string; + workdir: string | null; onRunAction: RunDebugAction; -}) => ( -
-
Authentication
- {options.length ? ( -
- {options.map(({ method, label }) => ( -
-
- - {method.type === "env_var" && method.link && ( - - Get credentials - - )} -
- {method.type === "env_var" && method.vars?.length ? ( -
    - {method.vars.map((variable) => ( -
  • - {variable.name} - {variable.label ? {variable.label} : null} - {variable.optional ? ( - - optional - - ) : ( - required - )} - {variable.secret ? secret : null} -
    • - ))} -
  • - Set these environment variables, then re-initialize the agent from the ACP providers settings. -
    • -
- ) : null} -
- ))} - {logoutEnabled && ( - - )} -
- ) : ( - No auth required - )} -
-); +}) => { + const [terminalAuth, setTerminalAuth] = useState<{ methodId: string } | null>(null); + return ( +
+
Authentication
+ {options.length ? ( +
+ {options.map(({ method, label }) => { + const isTerminal = method.type === "terminal"; + return ( +
+
+ {isTerminal ? ( + // Terminal methods must run the agent's login TUI — the + // debug `authenticate` RPC is not valid for them. + + ) : ( + + )} + {method.type === "env_var" && method.link && ( + + Get credentials + + )} +
+ {isTerminal ? ( +
+ Opens the agent's interactive login in an embedded terminal. +
+ ) : null} + {method.type === "env_var" && method.vars?.length ? ( +
    + {method.vars.map((variable) => ( +
  • + {variable.name} + {variable.label ? {variable.label} : null} + {variable.optional ? ( + + optional + + ) : ( + required + )} + {variable.secret ? secret : null} +
    • + ))} +
  • + Set these environment variables, then re-initialize the agent from the ACP providers settings. +
    • +
+ ) : null} +
+ ); + })} + {logoutEnabled && ( + + )} +
+ ) : ( + No auth required + )} + {terminalAuth ? ( + { + if (!next) setTerminalAuth(null); + }} + /> + ) : null} +
+ ); +}; const RemoteSessionsSection = ({ sessions, loading, diff --git a/packages/ui/src/components/chat/AcpAuthBanner.tsx b/packages/ui/src/components/chat/AcpAuthBanner.tsx new file mode 100644 index 000000000..465237600 --- /dev/null +++ b/packages/ui/src/components/chat/AcpAuthBanner.tsx @@ -0,0 +1,66 @@ +import { useEffect, useState } from "react"; +import { Icon } from "@iconify/react"; +import { Button } from "#shadcn/components/ui/button"; +import { createProviderClient } from "#api/ProviderClient"; +import AcpAuthDialog from "#settings/components/AcpAuthDialog"; + +const providerClient = createProviderClient(); + +/** + * Inline banner shown when an ACP agent fails a session/turn with + * `auth_required`. Offers the sign-in dialog; clears on success. + */ +export default function AcpAuthBanner({ sessionId }: { sessionId: string }) { + const [authPrompt, setAuthPrompt] = useState<{ agentId: string; workdir?: string | null } | null>(null); + const [dialogOpen, setDialogOpen] = useState(false); + + useEffect(() => { + const offRequired = providerClient.onAcpAuthRequired((payload) => { + // A banner only makes sense for the conversation the user is looking at. + if (payload.sessionId && payload.sessionId !== sessionId) return; + setAuthPrompt({ agentId: payload.agentId, workdir: payload.workdir ?? null }); + setDialogOpen(true); + }); + const offChanged = providerClient.onAcpAuthChanged((payload) => { + // Only the same agent's success clears the prompt — signing in to a + // different agent elsewhere must not dismiss an unrelated prompt. + if (payload.state === "ready" && authPrompt?.agentId === payload.agentId) { + setAuthPrompt(null); + } + }); + return () => { + offRequired(); + offChanged(); + }; + }, [sessionId, authPrompt?.agentId]); + + if (!authPrompt) return null; + + return ( + <> +
+ + + + This agent needs to be signed in before the conversation can continue. After signing in, send your message + again. + + + +
+ setAuthPrompt(null)} + onOpenChange={setDialogOpen} + /> + + ); +} diff --git a/packages/ui/src/pages/ChatPage.tsx b/packages/ui/src/pages/ChatPage.tsx index aa702a856..3e9707bfb 100644 --- a/packages/ui/src/pages/ChatPage.tsx +++ b/packages/ui/src/pages/ChatPage.tsx @@ -14,6 +14,7 @@ import type { } from "#/components/chat/messageListItems"; import { ErrorBoundary } from "#/components/ErrorBoundary"; import SettledBanner from "#/components/threads/SettledBanner"; +import AcpAuthBanner from "#/components/chat/AcpAuthBanner"; import AgentProgressFloat from "#/components/chat/AgentProgressFloat"; import PendingInputLane from "#/components/chat/PendingInputLane"; import ChatStatusBar from "#/components/chat/ChatStatusBar"; @@ -1575,6 +1576,7 @@ function ChatComposerDock(input: { )} {!activePendingInteraction && (
+ Date: Tue, 8 Sep 2026 22:54:46 -0300 Subject: [PATCH 5/5] fix(acp): cancel pending auth starts and bind keystrokes to agents Address reviewer findings on the reconciliation: - AcpAuthDialog: closing while a start request is still connecting now cancels the flow (a pending start previously escaped cancellation and could launch an invisible terminal login later); terminal events that arrive before the start response are preserved via a functional flow-state update instead of being clobbered back to running - terminal keystroke injection is bound to the owning agent: the write route takes agentId and the runtime rejects mismatches, so a runId observed on the broadcast event stream cannot be used to type into another client's login session - document the single-user trust model in the terminal-auth spec --- apps/daemon/src/dispatch/daemonDispatcher.ts | 4 +- .../daemon/src/host/acp-provider-execution.ts | 4 +- apps/daemon/src/host/acpAuthRuntime.ts | 15 ++++-- apps/daemon/test/acpAuthRuntime.test.ts | 17 +++++++ docs/features/acp-terminal-auth/spec.md | 4 ++ .../src/routes/providers.routes.ts | 1 + packages/ui/api/ProviderClient.ts | 4 +- .../ui/settings/components/AcpAuthDialog.tsx | 49 ++++++++++++------- 8 files changed, 72 insertions(+), 26 deletions(-) diff --git a/apps/daemon/src/dispatch/daemonDispatcher.ts b/apps/daemon/src/dispatch/daemonDispatcher.ts index b9193a4fb..bf562b557 100644 --- a/apps/daemon/src/dispatch/daemonDispatcher.ts +++ b/apps/daemon/src/dispatch/daemonDispatcher.ts @@ -348,7 +348,7 @@ type DaemonAcpSessionExecutionPort = { mode: "agent" | "terminal"; runId: string | null; }>; - writeAcpAuthInput?(runId: string, data: string): Promise; + writeAcpAuthInput?(agentId: string, runId: string, data: string): Promise; cancelAcpAuth?(agentId: string): Promise; }; @@ -3408,7 +3408,7 @@ export function createDaemonDispatcher( if (!acpSessionExecutionPort?.writeAcpAuthInput) { throw new Error("ACP authentication is not available in this runtime."); } - await acpSessionExecutionPort.writeAcpAuthInput(input.runId, input.data); + await acpSessionExecutionPort.writeAcpAuthInput(input.agentId, input.runId, input.data); return providersWriteAcpAuthInputRoute.output.parse({ ok: true }); } diff --git a/apps/daemon/src/host/acp-provider-execution.ts b/apps/daemon/src/host/acp-provider-execution.ts index b96836a6b..cab4a4592 100644 --- a/apps/daemon/src/host/acp-provider-execution.ts +++ b/apps/daemon/src/host/acp-provider-execution.ts @@ -185,9 +185,9 @@ export class AcpProviderExecutionPort implements ProviderExecutionPort { return await auth.start(input); } - async writeAcpAuthInput(runId: string, data: string): Promise { + async writeAcpAuthInput(agentId: string, runId: string, data: string): Promise { const auth = await this.getAuthRuntime(); - auth.write(runId, data); + auth.write(agentId, runId, data); } async cancelAcpAuth(agentId: string): Promise { diff --git a/apps/daemon/src/host/acpAuthRuntime.ts b/apps/daemon/src/host/acpAuthRuntime.ts index da471dc0d..991340afb 100644 --- a/apps/daemon/src/host/acpAuthRuntime.ts +++ b/apps/daemon/src/host/acpAuthRuntime.ts @@ -141,10 +141,19 @@ export class DaemonAcpAuthRuntime { } } - write(runId: string, data: string): void { + /** + * Write keystrokes into a terminal-auth run. The run is bound to its + * agent: `agentId` must match the run's owner so a runId observed on the + * broadcast event stream cannot be used to inject keystrokes into another + * client's login session. + */ + write(agentId: string, runId: string, data: string): void { const run = this.runsById.get(runId); - if (!run || run.state !== "running" || !run.write) { - throw new Error(`No active terminal auth run: ${runId}`); + if (!run || run.agentId !== agentId) { + throw new Error(`No active terminal auth run for ${agentId}: ${runId}`); + } + if (run.state !== "running" || !run.write) { + throw new Error(`Terminal auth run is not accepting input: ${runId}`); } run.write(data); } diff --git a/apps/daemon/test/acpAuthRuntime.test.ts b/apps/daemon/test/acpAuthRuntime.test.ts index 2bf1fe478..fa757bc84 100644 --- a/apps/daemon/test/acpAuthRuntime.test.ts +++ b/apps/daemon/test/acpAuthRuntime.test.ts @@ -245,4 +245,21 @@ describe("DaemonAcpAuthRuntime", () => { const harness = createHarness(); await expect(harness.auth.start({ agentId: "my-agent", methodId: "nope" })).rejects.toThrow("did not advertise"); }); + + it("binds terminal keystrokes to the owning agent", async () => { + const harness = createHarness({ + authMethods: [{ id: "term-1", name: "Terminal Login", type: "terminal", args: ["--login"] }], + }); + const result = await harness.auth.start({ agentId: "my-agent", methodId: "term-1" }); + + // The owning agent can write. + harness.auth.write("my-agent", result.runId!, "hello"); + expect(harness.terminalWrites).toContain("hello"); + + // A different agent's identity cannot inject keystrokes via a broadcast + // runId. + expect(() => harness.auth.write("other-agent", result.runId!, "evil")).toThrow("No active terminal auth run"); + expect(() => harness.auth.write("other-agent", "made-up-run", "evil")).toThrow("No active terminal auth run"); + expect(harness.terminalWrites).not.toContain("evil"); + }); }); diff --git a/docs/features/acp-terminal-auth/spec.md b/docs/features/acp-terminal-auth/spec.md index 378cea4ed..77522b0ec 100644 --- a/docs/features/acp-terminal-auth/spec.md +++ b/docs/features/acp-terminal-auth/spec.md @@ -68,6 +68,10 @@ Agents that require login (e.g. MiniMax Code, `mcode acp`) advertise `authMethod - **D5 — Inspection reuses diagnostics.** The dialog fetches methods via the existing `providers.getAcpAgentDiagnostics` route (extended to carry terminal `args`/`env`); no separate inspect route. +- **D9 — Single-user trust model for auth runs.** PTY keystroke injection and cancellation are + bound to the owning agent (`agentId` + `runId`), but the daemon remains a single-user trust + domain: any authenticated client may start a login for any agent and observe its output. + Multi-client ownership isolation is a follow-up for the remote-control deployment. - **D6 — Chat surfacing.** `AcpProviderExecutionPort` publishes `acp.auth.required` (sessionId, agentId, workdir, methods) when `isAuthRequiredError` matches a turn or draft preparation failure, and prefixes the error block so the raw JSON-RPC text is never the whole diff --git a/packages/shared-contracts/src/routes/providers.routes.ts b/packages/shared-contracts/src/routes/providers.routes.ts index b7ae36609..e9fd95bfa 100644 --- a/packages/shared-contracts/src/routes/providers.routes.ts +++ b/packages/shared-contracts/src/routes/providers.routes.ts @@ -362,6 +362,7 @@ export const providersStartAcpAuthRoute = defineRouteContract({ export const providersWriteAcpAuthInputRoute = defineRouteContract({ name: "providers.writeAcpAuthInput", input: zod.object({ + agentId: zod.string().min(1), runId: zod.string().min(1), data: zod.string(), }), diff --git a/packages/ui/api/ProviderClient.ts b/packages/ui/api/ProviderClient.ts index a41d8376f..56f8db653 100644 --- a/packages/ui/api/ProviderClient.ts +++ b/packages/ui/api/ProviderClient.ts @@ -171,8 +171,8 @@ export function createProviderClient(bridge: ArgosBridge = getArgosBridge()) { return await bridge.invoke(providersStartAcpAuthRoute.name, input); } - async function writeAcpAuthInput(runId: string, data: string) { - return await bridge.invoke(providersWriteAcpAuthInputRoute.name, { runId, data }); + async function writeAcpAuthInput(agentId: string, runId: string, data: string) { + return await bridge.invoke(providersWriteAcpAuthInputRoute.name, { agentId, runId, data }); } async function cancelAcpAuth(agentId: string) { diff --git a/packages/ui/settings/components/AcpAuthDialog.tsx b/packages/ui/settings/components/AcpAuthDialog.tsx index 0e4fe6e1a..f5f1fefb1 100644 --- a/packages/ui/settings/components/AcpAuthDialog.tsx +++ b/packages/ui/settings/components/AcpAuthDialog.tsx @@ -27,7 +27,7 @@ export interface AcpAuthDialogRequest { onOpenChange: (open: boolean) => void; } -type FlowState = "select" | "running" | "ready" | "error" | "cancelled"; +type FlowState = "select" | "starting" | "running" | "ready" | "error" | "cancelled"; /** * Sign-in dialog for ACP agents. Agent methods call `authenticate` on the @@ -52,12 +52,14 @@ export default function AcpAuthDialog({ const [runId, setRunId] = useState(null); const terminalRef = useRef(null); const xtermRef = useRef(null); + const pendingOutputRef = useRef([]); const reset = () => { setFlowState("select"); setActiveMethod(null); setError(null); setRunId(null); + pendingOutputRef.current = []; }; // Load methods when the dialog opens. @@ -77,7 +79,6 @@ export default function AcpAuthDialog({ // Auth state transitions + PTY output. Output that arrives before the // embedded terminal is mounted is buffered and flushed on open — the // daemon does not replay it. - const pendingOutputRef = useRef([]); useEffect(() => { if (!open) return; const off = providerClient.onAcpAuthChanged((payload) => { @@ -118,7 +119,7 @@ export default function AcpAuthDialog({ } pendingOutputRef.current = []; terminal.onData((data) => { - if (runId) void providerClient.writeAcpAuthInput(runId, data); + if (runId) void providerClient.writeAcpAuthInput(agentId, runId, data); }); terminal.focus(); xtermRef.current = terminal; @@ -126,27 +127,36 @@ export default function AcpAuthDialog({ terminal.dispose(); xtermRef.current = null; }; - }, [flowState, activeMethod, runId]); + }, [flowState, activeMethod, runId, agentId]); - const startMethod = async (methodId: string, name: string) => { + const startMethod = (methodId: string, name: string) => { setLoading(true); setError(null); - try { - const result = await providerClient.startAcpAuth({ agentId, workdir: workdir ?? undefined, methodId }); - setActiveMethod({ id: methodId, name, mode: result.mode }); - setRunId(result.runId); - setFlowState("running"); - } catch (err) { - setError(err instanceof Error ? err.message : String(err)); - setFlowState("error"); - } - setLoading(false); + // "starting" marks a requested-but-not-yet-confirmed flow: closing the + // dialog now still cancels, and terminal events that arrive before the + // response (warm connection) are preserved by the functional update. + setFlowState("starting"); + void providerClient + .startAcpAuth({ agentId, workdir: workdir ?? undefined, methodId }) + .then((result) => { + setActiveMethod({ id: methodId, name, mode: result.mode }); + setRunId(result.runId); + setFlowState((current) => (current === "starting" ? "running" : current)); + }) + .catch((err) => { + setError(err instanceof Error ? err.message : String(err)); + setFlowState((current) => (current === "starting" ? "error" : current)); + }) + .finally(() => setLoading(false)); }; const methods = diagnostics?.authMethods ?? []; const handleClose = (nextOpen: boolean) => { - if (!nextOpen && flowState === "running" && agentId) { + if (!nextOpen && agentId && (flowState === "running" || flowState === "starting")) { + // Covers both a running flow and a start that is still setting up its + // connection — an orphaned pending start would launch an invisible + // terminal login later and block the agent. void providerClient.cancelAcpAuth(agentId).catch(() => undefined); } onOpenChange(nextOpen); @@ -271,7 +281,12 @@ export default function AcpAuthDialog({ {flowState === "running" ? ( - ) : null}