-
Notifications
You must be signed in to change notification settings - Fork 105
feat: add ACPConversation #189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: cj/exp/acp-a
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ type StatusResponse struct { | |
| Body struct { | ||
| Status AgentStatus `json:"status" doc:"Current agent status. 'running' means that the agent is processing a message, 'stable' means that the agent is idle and waiting for input."` | ||
| AgentType mf.AgentType `json:"agent_type" doc:"Type of the agent being used by the server."` | ||
| Backend string `json:"backend" doc:"Backend transport being used ('acp' or 'pty')."` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason for picking |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ import ( | |
| mf "github.com/coder/agentapi/lib/msgfmt" | ||
| st "github.com/coder/agentapi/lib/screentracker" | ||
| "github.com/coder/agentapi/lib/termexec" | ||
| "github.com/coder/agentapi/x/acpio" | ||
| "github.com/coder/quartz" | ||
| "github.com/danielgtaylor/huma/v2" | ||
| "github.com/danielgtaylor/huma/v2/adapters/humachi" | ||
|
|
@@ -42,12 +43,13 @@ type Server struct { | |
| mu sync.RWMutex | ||
| logger *slog.Logger | ||
| conversation st.Conversation | ||
| agentio *termexec.Process | ||
| agentio st.AgentIO | ||
| agentType mf.AgentType | ||
| emitter *EventEmitter | ||
| chatBasePath string | ||
| tempDir string | ||
| clock quartz.Clock | ||
| transport string | ||
| } | ||
|
|
||
| func (s *Server) NormalizeSchema(schema any) any { | ||
|
|
@@ -98,7 +100,8 @@ const snapshotInterval = 25 * time.Millisecond | |
|
|
||
| type ServerConfig struct { | ||
| AgentType mf.AgentType | ||
| Process *termexec.Process | ||
| AgentIO st.AgentIO | ||
| Transport string | ||
| Port int | ||
| ChatBasePath string | ||
| AllowedHosts []string | ||
|
|
@@ -252,18 +255,34 @@ func NewServer(ctx context.Context, config ServerConfig) (*Server, error) { | |
| initialPrompt = FormatMessage(config.AgentType, config.InitialPrompt) | ||
| } | ||
|
|
||
| conversation := st.NewPTY(ctx, st.PTYConversationConfig{ | ||
| AgentType: config.AgentType, | ||
| AgentIO: config.Process, | ||
| Clock: config.Clock, | ||
| SnapshotInterval: snapshotInterval, | ||
| ScreenStabilityLength: 2 * time.Second, | ||
| FormatMessage: formatMessage, | ||
| ReadyForInitialPrompt: isAgentReadyForInitialPrompt, | ||
| FormatToolCall: formatToolCall, | ||
| InitialPrompt: initialPrompt, | ||
| Logger: logger, | ||
| }, emitter) | ||
| // Create appropriate conversation based on transport type | ||
| var conversation st.Conversation | ||
| if config.Transport == "acp" { | ||
| // For ACP, cast AgentIO to *acpio.ACPAgentIO | ||
| acpIO, ok := config.AgentIO.(*acpio.ACPAgentIO) | ||
| if !ok { | ||
| return nil, fmt.Errorf("ACP transport requires ACPAgentIO") | ||
| } | ||
| conversation = acpio.NewACPConversation(ctx, acpIO, logger, initialPrompt, emitter, config.Clock) | ||
| } else { | ||
| // Default to PTY transport | ||
|
Comment on lines
+258
to
+268
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. non-blocking: thoughts on if these comments add any value?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They probably don't and there was most likely no thought behind them. |
||
| proc, ok := config.AgentIO.(*termexec.Process) | ||
| if !ok && config.AgentIO != nil { | ||
| return nil, fmt.Errorf("PTY transport requires termexec.Process") | ||
| } | ||
| conversation = st.NewPTY(ctx, st.PTYConversationConfig{ | ||
| AgentType: config.AgentType, | ||
| AgentIO: proc, | ||
| Clock: config.Clock, | ||
| SnapshotInterval: snapshotInterval, | ||
| ScreenStabilityLength: 2 * time.Second, | ||
| FormatMessage: formatMessage, | ||
| ReadyForInitialPrompt: isAgentReadyForInitialPrompt, | ||
| FormatToolCall: formatToolCall, | ||
| InitialPrompt: initialPrompt, | ||
| Logger: logger, | ||
| }, emitter) | ||
| } | ||
|
|
||
| // Create temporary directory for uploads | ||
| tempDir, err := os.MkdirTemp("", "agentapi-uploads-") | ||
|
|
@@ -278,24 +297,25 @@ func NewServer(ctx context.Context, config ServerConfig) (*Server, error) { | |
| port: config.Port, | ||
| conversation: conversation, | ||
| logger: logger, | ||
| agentio: config.Process, | ||
| agentio: config.AgentIO, | ||
| agentType: config.AgentType, | ||
| emitter: emitter, | ||
| chatBasePath: strings.TrimSuffix(config.ChatBasePath, "/"), | ||
| tempDir: tempDir, | ||
| clock: config.Clock, | ||
| transport: config.Transport, | ||
| } | ||
|
|
||
| // Register API routes | ||
| s.registerRoutes() | ||
|
|
||
| // Start the conversation polling loop if we have a process. | ||
| // Process is nil only when --print-openapi is used (no agent runs). | ||
| // The process is already running at this point - termexec.StartProcess() | ||
| // blocks until the PTY is created and the process is active. Agent | ||
| // readiness (waiting for the prompt) is handled asynchronously inside | ||
| // conversation.Start() via ReadyForInitialPrompt. | ||
| if config.Process != nil { | ||
| // Start the conversation polling loop if we have an agent IO. | ||
| // AgentIO is nil only when --print-openapi is used (no agent runs). | ||
| // For PTY transport, the process is already running at this point - | ||
| // termexec.StartProcess() blocks until the PTY is created and the process | ||
| // is active. Agent readiness (waiting for the prompt) is handled | ||
| // asynchronously inside conversation.Start() via ReadyForInitialPrompt. | ||
| if config.AgentIO != nil { | ||
| s.conversation.Start(ctx) | ||
| } | ||
|
|
||
|
|
@@ -417,6 +437,7 @@ func (s *Server) getStatus(ctx context.Context, input *struct{}) (*StatusRespons | |
| resp := &StatusResponse{} | ||
| resp.Body.Status = agentStatus | ||
| resp.Body.AgentType = s.agentType | ||
| resp.Body.Backend = s.transport | ||
|
|
||
| return resp, nil | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of this. I'd expect
checkACPModeto return a bool, and the caller to add this "attach is not supported in ACP mode" context.