Files
kandev/apps/backend/internal/agentctl/server/acp/client.go
T
João Salavisa 2e108b040f refactor: session resumption cleanup and AgentInstance → AgentExecution rename (#23)
* feat: session resumption with ACP session ID support

- Add useSessionResumption hook for automatic session recovery on page reload
- Fix ACPSessionID not being set on AgentInstance in Launch() and createInstance()
- Add task.session.status and task.session.resume WebSocket actions
- Fix shell terminal retry logic for resumed sessions
- Fix taskSessionState reading from Zustand store instead of local state
- Remove dead code: unused stream handling functions in lifecycle/manager.go
- Fix test mock interfaces (CleanupStaleInstanceByTaskID, NewManager signature)
- Add enhanced logging for session resumption debugging
- Simplify page-client.tsx state merging logic

* refactor: remove unused AgentController (dead code)

- Delete agent_controller.go - entirely unused duplicate of Manager methods
- Remove agentController field from Manager struct
- Remove AgentController initialization in NewManager

The AgentController duplicated StartAgentProcess, initializeACPSession, and
PromptAgent from Manager but was never called. All agent control operations
go through Manager's own methods.

* refactor: remove unused MemoryRepository (dead code)

- Delete memory.go - entire MemoryRepository implementation unused
  (replaced by SQLite repository)
- Delete memory_test.go - tests for removed MemoryRepository

Found via 'deadcode' tool analysis.

* refactor: consolidate agentctl packages into server/client structure

Reorganize the confusing dual agentctl packages:
- internal/agentctl/* → internal/agentctl/server/
- internal/agent/agentctl/* → internal/agentctl/client/

Structure is now:
  internal/agentctl/
  ├── client/      - HTTP client for talking to agentctl servers
  │   ├── client.go
  │   ├── standalone.go
  │   ├── acp.go
  │   └── launcher/
  ├── server/      - agentctl server implementation (runs in containers)
  │   ├── api/
  │   ├── adapter/
  │   ├── acp/
  │   ├── config/
  │   ├── instance/
  │   ├── process/
  │   └── shell/
  └── types/       - Shared types used by both client and server

Also consolidated shared types (GitStatusUpdate, FileInfo, etc.) from
process/types.go into the shared types/ package.

* refactor: rename AgentInstance to AgentExecution for clarity

Rename AgentInstance -> AgentExecution across the entire codebase to better
reflect what it represents: an agent actively working on a task, not just
a generic 'instance'.

Changes:
- AgentInstance struct -> AgentExecution
- InstanceStore -> ExecutionStore (new file: execution_store.go)
- RecoveredInstance -> RecoveredExecution
- All method names: GetInstance* -> GetExecution*, ListInstances -> ListExecutions, etc.
- All parameters: instanceID -> executionID
- All local variables: instance -> execution
- DTOs and JSON tags: agent_instance_id -> agent_execution_id
- Database columns: agent_instance_id -> agent_execution_id

Also consolidates duplicate prompt logic:
- Merge sendInitialPrompt and SendPrompt into single SendPrompt method
- Add validateStatus parameter to differentiate initial vs follow-up prompts

Also extracts ACP session management to session.go:
- Move initializeACPSession logic to SessionManager.InitializeAndPrompt
- Move PromptAgent logic to SessionManager.SendPrompt
- Manager now delegates to SessionManager for all prompt operations

Also removes duplicate event publishing:
- Remove 9 duplicate m.publish*() methods from manager.go
- Use existing EventPublisher methods via m.eventPublisher.Publish*()

Also extracts types to dedicated file:
- Move AgentExecution, LaunchRequest, PromptResult, etc. to types.go

Net reduction: ~600 lines removed from manager.go through deduplication
and better separation of concerns.

* fix(backend): Docker runtime now creates instance via ControlServer

The Docker runtime was incorrectly creating an agentctl.Client pointing
directly to port 9999 (the ControlServer port), then trying to call
instance-specific APIs like /api/v1/agent/configure which only exist
on per-instance servers.

Fixed by making Docker runtime follow the same flow as standalone mode:
- LaunchContainer now uses StandaloneCtl to create an instance via the
  control API, then creates Client pointing to the instance port
- RecoverInstances queries the ControlServer to get the instance port
- Added waitForHealth helper to wait for agentctl to be ready

Also improved HTTP error handling in agentctl client:
- All client methods now check HTTP status codes before JSON decoding
- Better error messages include status code and response body

* fix: restore session resumption imports and variables

* fix: add sessionId prop back to TaskLayout
2026-01-16 22:14:19 +00:00

344 lines
10 KiB
Go

// Package acp implements the ACP client interface for agentctl
package acp
import (
"context"
"fmt"
"os"
"path/filepath"
"strings"
"sync"
"github.com/coder/acp-go-sdk"
"github.com/kandev/kandev/internal/agentctl/types"
"go.uber.org/zap"
)
// UpdateHandler is called when session updates are received from the agent
type UpdateHandler func(notification acp.SessionNotification)
// PermissionRequestHandler is called when the agent requests permission
// Returns the selected option ID, or empty string with cancelled=true to cancel
type PermissionRequestHandler func(ctx context.Context, req *types.PermissionRequest) (*types.PermissionResponse, error)
// Client implements acp.Client interface and handles all agent requests
type Client struct {
logger *zap.Logger
workspaceRoot string
mu sync.RWMutex
updateHandler UpdateHandler
permissionHandler PermissionRequestHandler
}
// ClientOption configures a Client
type ClientOption func(*Client)
// WithLogger sets the logger
func WithLogger(l *zap.Logger) ClientOption {
return func(c *Client) {
c.logger = l
}
}
// WithWorkspaceRoot sets the workspace root for file operations
func WithWorkspaceRoot(root string) ClientOption {
return func(c *Client) {
c.workspaceRoot = root
}
}
// WithUpdateHandler sets the handler for session updates
func WithUpdateHandler(h UpdateHandler) ClientOption {
return func(c *Client) {
c.updateHandler = h
}
}
// WithPermissionHandler sets the handler for permission requests
func WithPermissionHandler(h PermissionRequestHandler) ClientOption {
return func(c *Client) {
c.permissionHandler = h
}
}
// NewClient creates a new ACP client implementation
func NewClient(opts ...ClientOption) *Client {
c := &Client{
logger: zap.NewNop(),
workspaceRoot: "/workspace",
}
for _, opt := range opts {
opt(c)
}
return c
}
// SetUpdateHandler sets the update handler (thread-safe)
func (c *Client) SetUpdateHandler(h UpdateHandler) {
c.mu.Lock()
defer c.mu.Unlock()
c.updateHandler = h
}
// SetPermissionHandler sets the permission request handler (thread-safe)
func (c *Client) SetPermissionHandler(h PermissionRequestHandler) {
c.mu.Lock()
defer c.mu.Unlock()
c.permissionHandler = h
}
// RequestPermission handles permission requests from the agent
// If a permission handler is set, it forwards the request to the handler.
// Otherwise, auto-approves by selecting the first "allow" option.
func (c *Client) RequestPermission(ctx context.Context, p acp.RequestPermissionRequest) (acp.RequestPermissionResponse, error) {
title := ""
if p.ToolCall.Title != nil {
title = *p.ToolCall.Title
}
c.logger.Info("received permission request",
zap.String("session_id", string(p.SessionId)),
zap.String("tool_call_id", string(p.ToolCall.ToolCallId)),
zap.String("title", title),
zap.Int("num_options", len(p.Options)))
// No options - cancel
if len(p.Options) == 0 {
c.logger.Warn("no options available, cancelling permission request")
return acp.RequestPermissionResponse{
Outcome: acp.RequestPermissionOutcome{
Cancelled: &acp.RequestPermissionOutcomeCancelled{},
},
}, nil
}
// Check if we have a permission handler
c.mu.RLock()
handler := c.permissionHandler
c.mu.RUnlock()
if handler != nil {
// Forward to external handler (e.g., backend/user)
return c.forwardPermissionRequest(ctx, handler, p)
}
// Fall back to auto-approve
return c.autoApprovePermission(p)
}
// forwardPermissionRequest forwards the permission request to an external handler
func (c *Client) forwardPermissionRequest(ctx context.Context, handler PermissionRequestHandler, p acp.RequestPermissionRequest) (acp.RequestPermissionResponse, error) {
// Convert ACP types to shared types
options := make([]types.PermissionOption, len(p.Options))
for i, opt := range p.Options {
options[i] = types.PermissionOption{
OptionID: string(opt.OptionId),
Name: opt.Name,
Kind: string(opt.Kind),
}
}
title := ""
if p.ToolCall.Title != nil {
title = *p.ToolCall.Title
}
req := &types.PermissionRequest{
SessionID: string(p.SessionId),
ToolCallID: string(p.ToolCall.ToolCallId),
Title: title,
Options: options,
}
c.logger.Info("forwarding permission request to handler",
zap.String("session_id", req.SessionID),
zap.String("tool_call_id", req.ToolCallID))
resp, err := handler(ctx, req)
if err != nil {
c.logger.Error("permission handler failed", zap.Error(err))
// On error, cancel the permission request
return acp.RequestPermissionResponse{
Outcome: acp.RequestPermissionOutcome{
Cancelled: &acp.RequestPermissionOutcomeCancelled{},
},
}, nil
}
if resp.Cancelled {
c.logger.Info("permission request cancelled by user")
return acp.RequestPermissionResponse{
Outcome: acp.RequestPermissionOutcome{
Cancelled: &acp.RequestPermissionOutcomeCancelled{},
},
}, nil
}
c.logger.Info("permission request approved by user",
zap.String("option_id", resp.OptionID))
return acp.RequestPermissionResponse{
Outcome: acp.RequestPermissionOutcome{
Selected: &acp.RequestPermissionOutcomeSelected{
OptionId: acp.PermissionOptionId(resp.OptionID),
},
},
}, nil
}
// autoApprovePermission auto-approves by selecting the first "allow" option
func (c *Client) autoApprovePermission(p acp.RequestPermissionRequest) (acp.RequestPermissionResponse, error) {
// Find the first "allow" option
var selectedOption *acp.PermissionOption
for i := range p.Options {
opt := &p.Options[i]
if opt.Kind == acp.PermissionOptionKindAllowOnce || opt.Kind == acp.PermissionOptionKindAllowAlways {
selectedOption = opt
break
}
}
// If no allow option, use the first option
if selectedOption == nil {
selectedOption = &p.Options[0]
}
c.logger.Info("auto-approving permission request",
zap.String("option_id", string(selectedOption.OptionId)),
zap.String("option_name", selectedOption.Name),
zap.String("kind", string(selectedOption.Kind)))
return acp.RequestPermissionResponse{
Outcome: acp.RequestPermissionOutcome{
Selected: &acp.RequestPermissionOutcomeSelected{
OptionId: selectedOption.OptionId,
},
},
}, nil
}
// SessionUpdate handles session update notifications from the agent
func (c *Client) SessionUpdate(ctx context.Context, n acp.SessionNotification) error {
c.mu.RLock()
handler := c.updateHandler
c.mu.RUnlock()
// Log the update type
u := n.Update
switch {
case u.AgentMessageChunk != nil:
if u.AgentMessageChunk.Content.Text != nil {
c.logger.Debug("agent message chunk",
zap.String("text", u.AgentMessageChunk.Content.Text.Text[:min(50, len(u.AgentMessageChunk.Content.Text.Text))]))
}
case u.ToolCall != nil:
c.logger.Info("tool call",
zap.String("tool_call_id", string(u.ToolCall.ToolCallId)),
zap.String("title", u.ToolCall.Title),
zap.String("status", string(u.ToolCall.Status)))
case u.ToolCallUpdate != nil:
c.logger.Debug("tool call update",
zap.String("tool_call_id", string(u.ToolCallUpdate.ToolCallId)))
case u.Plan != nil:
c.logger.Info("plan update", zap.Int("entries", len(u.Plan.Entries)))
}
// Forward to handler if set
if handler != nil {
handler(n)
}
return nil
}
// ReadTextFile reads a text file from the workspace
func (c *Client) ReadTextFile(ctx context.Context, p acp.ReadTextFileRequest) (acp.ReadTextFileResponse, error) {
c.logger.Debug("reading file", zap.String("path", p.Path))
// Validate path is absolute
if !filepath.IsAbs(p.Path) {
return acp.ReadTextFileResponse{}, fmt.Errorf("path must be absolute: %s", p.Path)
}
// Read the file
b, err := os.ReadFile(p.Path)
if err != nil {
return acp.ReadTextFileResponse{}, err
}
content := string(b)
// Handle line/limit parameters
if p.Line != nil || p.Limit != nil {
lines := strings.Split(content, "\n")
start := 0
if p.Line != nil && *p.Line > 0 {
start = *p.Line - 1
if start > len(lines) {
start = len(lines)
}
}
end := len(lines)
if p.Limit != nil && *p.Limit > 0 && start+*p.Limit < end {
end = start + *p.Limit
}
content = strings.Join(lines[start:end], "\n")
}
return acp.ReadTextFileResponse{Content: content}, nil
}
// WriteTextFile writes a text file to the workspace
func (c *Client) WriteTextFile(ctx context.Context, p acp.WriteTextFileRequest) (acp.WriteTextFileResponse, error) {
c.logger.Debug("writing file", zap.String("path", p.Path))
// Validate path is absolute
if !filepath.IsAbs(p.Path) {
return acp.WriteTextFileResponse{}, fmt.Errorf("path must be absolute: %s", p.Path)
}
// Create directory if needed
if dir := filepath.Dir(p.Path); dir != "" {
if err := os.MkdirAll(dir, 0o755); err != nil {
return acp.WriteTextFileResponse{}, err
}
}
return acp.WriteTextFileResponse{}, os.WriteFile(p.Path, []byte(p.Content), 0o644)
}
// CreateTerminal creates a new terminal
func (c *Client) CreateTerminal(ctx context.Context, p acp.CreateTerminalRequest) (acp.CreateTerminalResponse, error) {
c.logger.Debug("create terminal request", zap.String("command", p.Command))
// For now, return a dummy terminal ID - we can implement real terminal support later
return acp.CreateTerminalResponse{TerminalId: "t-1"}, nil
}
// KillTerminalCommand kills a terminal command
func (c *Client) KillTerminalCommand(ctx context.Context, p acp.KillTerminalCommandRequest) (acp.KillTerminalCommandResponse, error) {
c.logger.Debug("kill terminal request", zap.String("terminal_id", p.TerminalId))
return acp.KillTerminalCommandResponse{}, nil
}
// TerminalOutput gets terminal output
func (c *Client) TerminalOutput(ctx context.Context, p acp.TerminalOutputRequest) (acp.TerminalOutputResponse, error) {
c.logger.Debug("terminal output request", zap.String("terminal_id", p.TerminalId))
return acp.TerminalOutputResponse{Output: "ok", Truncated: false}, nil
}
// ReleaseTerminal releases a terminal
func (c *Client) ReleaseTerminal(ctx context.Context, p acp.ReleaseTerminalRequest) (acp.ReleaseTerminalResponse, error) {
c.logger.Debug("release terminal request", zap.String("terminal_id", p.TerminalId))
return acp.ReleaseTerminalResponse{}, nil
}
// WaitForTerminalExit waits for terminal to exit
func (c *Client) WaitForTerminalExit(ctx context.Context, p acp.WaitForTerminalExitRequest) (acp.WaitForTerminalExitResponse, error) {
c.logger.Debug("wait for terminal exit request", zap.String("terminal_id", p.TerminalId))
exitCode := 0
return acp.WaitForTerminalExitResponse{ExitCode: &exitCode}, nil
}
// Verify interface implementation
var _ acp.Client = (*Client)(nil)