From cc169a7ff9da7a8e58135ddeb96a78dade26211b Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 25 Aug 2026 07:13:37 +0000 Subject: [PATCH] docs(agentctl): correct comments that describe adapters and protocols that don't exist (#3012) Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 --- .../internal/agent/runtime/lifecycle/types.go | 2 +- .../agentctl/server/adapter/adapter.go | 43 ++++++++++--------- .../adapter/transport/acp/adapter_prompt.go | 1 - .../server/adapter/transport/shared/config.go | 4 +- .../server/adapter/transport/shared/debug.go | 12 ++---- .../internal/agentctl/server/api/server.go | 4 +- .../internal/agentctl/server/config/config.go | 3 +- .../agentctl/server/process/manager.go | 4 +- 8 files changed, 35 insertions(+), 38 deletions(-) diff --git a/apps/backend/internal/agent/runtime/lifecycle/types.go b/apps/backend/internal/agent/runtime/lifecycle/types.go index 8c29451601..6a7d45e115 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/types.go +++ b/apps/backend/internal/agent/runtime/lifecycle/types.go @@ -50,7 +50,7 @@ type AgentExecution struct { WorkspaceSourceRoots []string // Canonical durable source roots permitted by agentctl file operations ACPSessionID string // ACP session ID to resume, if available AgentCommand string // Command to start the agent subprocess - ContinueCommand string // Command for follow-up prompts (one-shot agents like Amp) + ContinueCommand string // Command for follow-up prompts (one-shot agents) AgentArgs []string // Structured argv for AgentCommand ContinueArgs []string // Structured argv for ContinueCommand RuntimeName agentruntime.Runtime // Name of the runtime used (e.g., "docker", "standalone") diff --git a/apps/backend/internal/agentctl/server/adapter/adapter.go b/apps/backend/internal/agentctl/server/adapter/adapter.go index cccd5be49f..d47d43fdb7 100644 --- a/apps/backend/internal/agentctl/server/adapter/adapter.go +++ b/apps/backend/internal/agentctl/server/adapter/adapter.go @@ -1,9 +1,9 @@ -// Package adapter provides protocol adapters for different agent communication protocols. -// This abstraction allows agentctl to work with agents using ACP, REST, MCP, or custom protocols. +// Package adapter provides protocol adapters for agent communication. +// Only ACP is supported; non-ACP variants were removed in the ACP-first migration. // // Architecture: -// - Transport layer (transport/*): Handles protocol-level communication (ACP, stream-json, codex, opencode) -// - Factory: Creates the appropriate transport adapter based on agent's protocol +// - Transport layer (transport/*): Handles protocol-level communication (ACP is the only transport) +// - Factory: Creates the ACP transport adapter // // Agent configuration (commands, discovery, models) is handled by the agent/registry package // using agents.json as the source of truth. This package only handles protocol communication. @@ -52,9 +52,10 @@ const ( ) // OneShotAdapter is an optional interface implemented by adapters that spawn -// a new process per prompt (e.g., Amp). When an adapter is one-shot, the -// process manager skips subprocess creation and the adapter manages its own -// subprocess lifecycle internally. +// a new process per prompt. When an adapter is one-shot, the process manager +// skips subprocess creation and the adapter manages its own subprocess +// lifecycle internally. No production adapter implements this today; Amp is +// now an ACP agent and goes through the standard subprocess path. type OneShotAdapter interface { IsOneShot() bool } @@ -152,7 +153,7 @@ type AgentInfo struct { } // AgentAdapter defines the interface for protocol adapters. -// Each adapter translates a specific protocol (ACP, REST, MCP, etc.) into the +// Each adapter translates ACP, the only supported protocol, into the // normalized AgentEvent format that agentctl exposes via its HTTP API. // // Lifecycle: @@ -166,15 +167,13 @@ type AgentInfo struct { // 8. Call Close() when done type AgentAdapter interface { // PrepareEnvironment performs protocol-specific setup before the agent process starts. - // For ACP, this is a no-op (MCP servers are passed through the protocol). - // For OpenCode, this returns environment variables for server authentication. + // The ACP adapter is a no-op (MCP servers are passed through the protocol). // Must be called before the agent subprocess is started. // Returns a map of environment variables to add to the subprocess environment. PrepareEnvironment() (map[string]string, error) // PrepareCommandArgs returns extra command-line arguments for the agent process. - // For Codex, this returns -c flags for MCP servers and sandbox config. - // For other protocols, this returns nil (no extra args needed). + // The ACP adapter returns nil (no extra args needed). // Must be called after PrepareEnvironment and before starting the subprocess. PrepareCommandArgs() []string @@ -183,8 +182,7 @@ type AgentAdapter interface { Connect(stdin io.Writer, stdout io.Reader) error // Initialize establishes the connection with the agent and exchanges capabilities. - // For subprocess-based agents (ACP), this sends the initialize request. - // For HTTP-based agents (REST), this might do a health check. + // Sends the ACP initialize request over the subprocess's stdin/stdout. Initialize(ctx context.Context) error // GetAgentInfo returns information about the connected agent. @@ -216,8 +214,8 @@ type AgentAdapter interface { GetSessionID() string // GetOperationID returns the current operation/turn ID. - // Returns empty string if no operation is in progress or not supported by the protocol. - // For Codex this is the turn ID, for ACP this may be empty. + // Returns empty string if no operation is in progress, or if the connected + // ACP agent does not report a turn ID. GetOperationID() string // SetPermissionHandler sets the handler for permission requests. @@ -227,10 +225,11 @@ type AgentAdapter interface { Close() error // RequiresProcessKill returns true if the adapter's subprocess needs to be - // explicitly killed during shutdown. Adapters that communicate via stdin/stdout - // (ACP, Codex, Claude Code) return false because closing stdin causes the - // subprocess to exit. HTTP-server-based adapters (OpenCode) return true because - // they don't exit on stdin close. + // explicitly killed during shutdown. Most ACP agents return false because + // closing stdin causes the subprocess to exit. Some agents (e.g. opencode + // acp) keep an internal HTTP server and MCP child processes alive after + // stdin closes, so their config sets this true to force an immediate + // process-group kill instead of waiting on a graceful stdin close. RequiresProcessKill() bool } @@ -276,7 +275,9 @@ type Config struct { // Used for display purposes. AgentName string - // For HTTP-based adapters (REST) + // BaseURL, AuthHeader, AuthValue, and Headers are unused by any adapter + // today (no HTTP-based transport exists). Config.ToSharedConfig still + // propagates them to shared.Config, but no transport reads them. BaseURL string // Base URL of the agent's HTTP API AuthHeader string // Optional auth header name AuthValue string // Optional auth header value diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go index 8b676f8b22..cd7be6c4cb 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go @@ -265,7 +265,6 @@ func (a *Adapter) sendPrompt( } // Emit complete event via the stream, including the StopReason from the agent. - // This normalizes ACP behavior to match other adapters (stream-json, amp, copilot, opencode). a.logger.Debug("emitting complete event after prompt", zap.String("session_id", sessionID), zap.String("stop_reason", stopReason)) diff --git a/apps/backend/internal/agentctl/server/adapter/transport/shared/config.go b/apps/backend/internal/agentctl/server/adapter/transport/shared/config.go index 9196d84d6e..47b4d98512 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/shared/config.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/shared/config.go @@ -38,7 +38,9 @@ type Config struct { // Used for display purposes. AgentName string - // For HTTP-based adapters (REST) + // BaseURL, AuthHeader, AuthValue, and Headers are unused by any adapter + // today (no HTTP-based transport exists). Populated by + // adapter.Config.ToSharedConfig but never read by a transport. BaseURL string // Base URL of the agent's HTTP API AuthHeader string // Optional auth header name AuthValue string // Optional auth header value diff --git a/apps/backend/internal/agentctl/server/adapter/transport/shared/debug.go b/apps/backend/internal/agentctl/server/adapter/transport/shared/debug.go index a8177b5e65..cfbef0842e 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/shared/debug.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/shared/debug.go @@ -18,15 +18,9 @@ var debugMode = os.Getenv(envDebugMessages) == "true" // debugMode is true. var acpLog = newACPLogManager(acpLogConfigFromEnv()) -// Protocol constants for debug file naming -const ( - ProtocolACP = "acp" - ProtocolStreamJSON = "streamjson" - ProtocolCodex = "codex" - ProtocolOpenCode = "opencode" - ProtocolAmp = "amp" - ProtocolCopilot = "copilot" -) +// ProtocolACP names the ACP transport in debug file naming (raw-acp-*.jsonl, +// normalized-acp-*.jsonl). It is the only supported protocol. +const ProtocolACP = "acp" // ACPDebugEnabled reports whether agent-message debug logging is on. func ACPDebugEnabled() bool { return debugMode } diff --git a/apps/backend/internal/agentctl/server/api/server.go b/apps/backend/internal/agentctl/server/api/server.go index 19aecb9391..cb9f5d685f 100644 --- a/apps/backend/internal/agentctl/server/api/server.go +++ b/apps/backend/internal/agentctl/server/api/server.go @@ -428,10 +428,10 @@ func (s *Server) handleStart(c *gin.Context) { type AgentConfigureRequest struct { Command string `json:"command"` AgentArgs optionalArgs `json:"agent_args"` - ContinueCommand string `json:"continue_command,omitempty"` // For one-shot agents (Amp): command for follow-up prompts + ContinueCommand string `json:"continue_command,omitempty"` // For one-shot agents: command for follow-up prompts ContinueArgs optionalArgs `json:"continue_args"` Env map[string]string `json:"env,omitempty"` - ApprovalPolicy string `json:"approval_policy,omitempty"` // For Codex: "untrusted", "on-failure", "on-request", "never" + ApprovalPolicy string `json:"approval_policy,omitempty"` // "untrusted", "on-failure", "on-request", or "never" } type optionalArgs struct { diff --git a/apps/backend/internal/agentctl/server/config/config.go b/apps/backend/internal/agentctl/server/config/config.go index 0bb96f502a..c3a7144cd4 100644 --- a/apps/backend/internal/agentctl/server/config/config.go +++ b/apps/backend/internal/agentctl/server/config/config.go @@ -223,7 +223,8 @@ type InstanceConfig struct { // ContinueCommand is the command template for follow-up prompts in one-shot agents. // When set, the adapter spawns a new process per prompt using this command for - // continuation (thread ID appended at runtime). Only used by Amp. + // continuation (thread ID appended at runtime). No production adapter is + // one-shot today; see OneShotAdapter in adapter/adapter.go. ContinueCommand string // ContinueArgs is the structured argv for ContinueCommand. diff --git a/apps/backend/internal/agentctl/server/process/manager.go b/apps/backend/internal/agentctl/server/process/manager.go index acf9bab8d2..eacc820119 100644 --- a/apps/backend/internal/agentctl/server/process/manager.go +++ b/apps/backend/internal/agentctl/server/process/manager.go @@ -1295,7 +1295,7 @@ func (m *Manager) buildAdapterConfig() error { } // Configure one-shot mode when a continue command is provided. - // One-shot adapters (e.g., Amp) spawn a new subprocess per prompt. + // One-shot adapters spawn a new subprocess per prompt. continueArgs := m.cfg.ContinueArgs if continueArgs == nil && m.cfg.ContinueCommand != "" { continueArgs = config.ParseCommand(m.cfg.ContinueCommand) @@ -1589,7 +1589,7 @@ func (m *Manager) Configure(command string, agentArgs []string, agentArgsPresent m.cfg.ApprovalPolicy = approvalPolicy } - // Store continue command for one-shot adapters (e.g., Amp) + // Store continue command for one-shot adapters if continueArgsPresent { m.cfg.ContinueCommand = continueCommand m.cfg.ContinueArgs = continueArgs