mirror of
https://github.com/docker/docker-agent.git
synced 2026-11-04 10:02:15 +00:00
fix(acp): report consistent tool-call lifecycles
Signed-off-by: David Gageot <david@gageot.net> Assisted-By: docker-agent
This commit is contained in:
12 files changed
+647
-113
No files matched your search
@@ -166,6 +166,18 @@ These are path checks, not an atomic sandbox around client I/O. The ACP client m
|
||||
|
||||
Configured `post_edit` commands run **locally**, in the session toolset's working directory, only after a successful client write. They receive the checked target path in `${file}` and match patterns against that target, not a symlink alias. Hooks require the client and agent to share a coherent on-disk filesystem; editor-buffer-only writes are not mirrored to local disk. A hook failure reports that the write succeeded but the hook failed, without retrying or rolling back the write.
|
||||
|
||||
## Tool-Call Lifecycle
|
||||
|
||||
ACP reports a tool awaiting permission as `pending`. Actual execution is reported as `in_progress`, followed by `completed` or `failed`. Permission requests and subsequent updates refer to the same opaque ACP tool-call ID; permission approval is not inferred from display status.
|
||||
|
||||
Rejected, policy-denied, or unavailable tools can produce a terminal result without executing. These become failed tool items instead of aborting the conversation or inventing a running phase. Tool kinds use one display classifier across permission and execution updates: operation names take precedence over generic hints, and a destructive hint alone does not mean deletion.
|
||||
|
||||
ACP assigns fresh IDs rather than exposing runtime IDs directly. Correlation distinguishes agent names and supports sequential runtime-ID reuse, including across turns. The runtime does not yet provide enough identity to disambiguate simultaneous identical IDs from two sub-sessions of the same agent.
|
||||
|
||||
On interruption, ACP drains runtime events before finishing the prompt. Known results retain their actual status and output; visible calls with no result receive a best-effort failed update explaining that side effects may have occurred. Cancellation still determines the prompt's `cancelled` stop reason. Notification failure cannot undo tool execution, and the transport does not guarantee a hard timeout for blocked writes. Terminal write failures are not retried because delivery may have been partial.
|
||||
|
||||
Partial tool arguments and incremental tool output are not streamed by this adapter. Completed results, including eligible file diffs, remain available; this avoids presenting unapproved partial input or treating independently transformed output chunks as whole-output redaction.
|
||||
|
||||
## Tool Locations and File Diffs
|
||||
|
||||
Tool-start and permission updates report absolute file locations. Relative paths are resolved lexically against the session's known workspace, with home-directory expansion; this display metadata does not read files or grant access. URI-shaped values and relative paths without an absolute workspace are omitted.
|
||||
|
||||
@@ -271,10 +271,16 @@ func deferredStreamDrain(p *cop.Pass, d *ast.DeferStmt, source runStreamSource)
|
||||
case *ast.ExprStmt: // e.g. cancel() before draining
|
||||
continue
|
||||
case *ast.RangeStmt:
|
||||
if matches(p, stmt.X) && len(stmt.Body.List) == 0 {
|
||||
return kind
|
||||
if !matches(p, stmt.X) {
|
||||
return streamDrainNone
|
||||
}
|
||||
return streamDrainNone
|
||||
for _, bodyStmt := range stmt.Body.List {
|
||||
// Calls can retain final results without leaving the drain loop.
|
||||
if _, ok := bodyStmt.(*ast.ExprStmt); !ok {
|
||||
return streamDrainNone
|
||||
}
|
||||
}
|
||||
return kind
|
||||
default:
|
||||
return streamDrainNone
|
||||
}
|
||||
|
||||
@@ -34,6 +34,8 @@ func TestDrainRunStreamBeforeRelease(t *testing.T) {
|
||||
{name: "conditional defer", body: `events := r.RunStream(); if stop { defer func(){ for range events {} }() }; for range events { return }`, want: 1},
|
||||
{name: "unrelated channel defer", body: `events := r.RunStream(); defer func(){ for range other {} }(); for range events { return }`, want: 1},
|
||||
{name: "deferred drain can return early", body: `events := r.RunStream(); defer func(){ for range events { return } }(); for range events { return }`, want: 1},
|
||||
{name: "deferred drain can break", body: `events := r.RunStream(); defer func(){ for range events { cancel(); break } }(); for range events { return }`, want: 1},
|
||||
{name: "deferred drain can nil channel", body: `events := r.RunStream(); defer func(){ events = nil; for range events { cancel() } }(); for range events { return }`, want: 1},
|
||||
{name: "closure return does not drain owner", body: `events := r.RunStream(); _ = func(){ for range events {} }; for range events { return }`, want: 1},
|
||||
{name: "shadowed stream", body: `events := r.RunStream(); for range events { events := other; for range events {}; return }`, want: 1},
|
||||
{name: "select cancellation", body: `events := r.RunStream(); for events != nil { select { case _, ok := <-events: if !ok { events = nil; continue }; if stop { return }; case <-other: return } }`, want: 1},
|
||||
@@ -51,6 +53,8 @@ func TestDrainRunStreamBeforeRelease(t *testing.T) {
|
||||
{name: "nested loop break", body: `events := r.RunStream(); for range events { for { break } }`},
|
||||
{name: "nested function return", body: `events := r.RunStream(); for range events { func(){ return }() }`},
|
||||
{name: "deferred drain", body: `events := r.RunStream(); defer func(){ cancel(); for range events {} }(); for range events { return }`},
|
||||
{name: "deferred drain retains results", body: `events := r.RunStream(); defer func(){ cancel(); for event := range events { retain(event) } }(); for range events { return }`},
|
||||
{name: "deferred snapshot retains results", body: `events := r.RunStream(); defer func(ch <-chan Event){ cancel(); for event := range ch { retain(event) } }(events); for range events { return }`},
|
||||
{name: "deferred snapshot", body: `events := r.RunStream(); defer func(ch <-chan Event){ cancel(); for range ch {} }(events); for range events { return }`},
|
||||
{name: "select until closed", body: `events := r.RunStream(); for events != nil { select { case _, ok := <-events: if !ok { events = nil; continue } } }`},
|
||||
{name: "return only after closed", body: `events := r.RunStream(); for { _, ok := <-events; if !ok { return } }`},
|
||||
@@ -84,6 +88,7 @@ func runDrainCop(t *testing.T, filename, body string) []cop.Offense {
|
||||
func (otherRuntime) RunStream() <-chan int { return nil }
|
||||
func cancel() {}
|
||||
func consume(<-chan Event) {}
|
||||
func retain(Event) {}
|
||||
func run(r Runtime, other <-chan Event, stop bool) { ` + body + ` }
|
||||
`
|
||||
fset := token.NewFileSet()
|
||||
|
||||
+29
-21
@@ -814,7 +814,7 @@ func (a *Agent) sendUpdate(ctx context.Context, sessionID string, update acp.Ses
|
||||
}
|
||||
|
||||
// runAgent runs a single agent loop and streams updates to the ACP client.
|
||||
func (a *Agent) runAgent(ctx context.Context, acpSess *Session) (acp.StopReason, error) {
|
||||
func (a *Agent) runAgent(ctx context.Context, acpSess *Session) (stopReason acp.StopReason, retErr error) {
|
||||
slog.DebugContext(ctx, "Running agent turn", "session_id", acpSess.id)
|
||||
|
||||
ctx = withSessionID(ctx, acpSess.id)
|
||||
@@ -825,17 +825,25 @@ func (a *Agent) runAgent(ctx context.Context, acpSess *Session) (acp.StopReason,
|
||||
|
||||
runCtx, cancel := context.WithCancel(ctx)
|
||||
eventsChan := acpSess.rt.RunStream(runCtx, acpSess.sess)
|
||||
// Cancel on handler errors too, before waiting for runtime teardown.
|
||||
var toolCalls toolCallTracker
|
||||
// Cancel before draining; final tool updates must precede the prompt response.
|
||||
defer func() {
|
||||
cancel()
|
||||
for range eventsChan {
|
||||
for event := range eventsChan {
|
||||
toolCalls.retainResult(event)
|
||||
}
|
||||
cleanupCtx, stopCleanup := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Second)
|
||||
defer stopCleanup()
|
||||
updateErr := toolCalls.interrupt(cleanupCtx, a, acpSess)
|
||||
if retErr == nil && ctx.Err() == nil {
|
||||
retErr = updateErr
|
||||
}
|
||||
}()
|
||||
toolCalls := map[string]struct{}{}
|
||||
outcome := promptOutcome{sessionID: acpSess.sess.ID}
|
||||
|
||||
for event := range eventsChan {
|
||||
if ctx.Err() != nil {
|
||||
toolCalls.retainResult(event)
|
||||
return "", ctx.Err()
|
||||
}
|
||||
|
||||
@@ -852,25 +860,23 @@ func (a *Agent) runAgent(ctx context.Context, acpSess *Session) (acp.StopReason,
|
||||
}
|
||||
|
||||
case *runtime.ToolCallConfirmationEvent:
|
||||
if err := a.handleToolCallConfirmation(ctx, acpSess, e); err != nil {
|
||||
state, err := toolCalls.report(ctx, a, acpSess, e.AgentName, e.ToolCall, e.ToolDefinition, acp.ToolCallStatusPending)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
rejected, err := a.handleToolCallConfirmation(ctx, acpSess, e, state.id)
|
||||
state.rejected = rejected
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
case *runtime.ToolCallEvent:
|
||||
toolCalls[e.ToolCall.ID] = struct{}{}
|
||||
workingDir, _ := acpSess.workspaceSnapshot()
|
||||
if err := a.sendUpdate(ctx, acpSess.id, buildToolCallStart(e.ToolCall, e.ToolDefinition, workingDir)); err != nil {
|
||||
if _, err := toolCalls.report(ctx, a, acpSess, e.AgentName, e.ToolCall, e.ToolDefinition, acp.ToolCallStatusInProgress); err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
case *runtime.ToolCallResponseEvent:
|
||||
_, ok := toolCalls[e.ToolCallID]
|
||||
if !ok {
|
||||
return "", fmt.Errorf("missing tool call arguments for tool call ID %s", e.ToolCallID)
|
||||
}
|
||||
delete(toolCalls, e.ToolCallID)
|
||||
|
||||
if err := a.sendUpdate(ctx, acpSess.id, buildToolCallComplete(e)); err != nil {
|
||||
if err := toolCalls.complete(ctx, a, acpSess, e); err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
@@ -934,9 +940,10 @@ func (a *Agent) runAgent(ctx context.Context, acpSess *Session) (acp.StopReason,
|
||||
}
|
||||
|
||||
// handleToolCallConfirmation handles tool call permission requests.
|
||||
func (a *Agent) handleToolCallConfirmation(ctx context.Context, acpSess *Session, e *runtime.ToolCallConfirmationEvent) error {
|
||||
func (a *Agent) handleToolCallConfirmation(ctx context.Context, acpSess *Session, e *runtime.ToolCallConfirmationEvent, id acp.ToolCallId) (bool, error) {
|
||||
workingDir, _ := acpSess.workspaceSnapshot()
|
||||
toolCallUpdate := buildToolCallUpdate(e.ToolCall, e.ToolDefinition, acp.ToolCallStatusPending, workingDir)
|
||||
toolCallUpdate.ToolCallId = id
|
||||
|
||||
permResp, err := a.conn.RequestPermission(ctx, acp.RequestPermissionRequest{
|
||||
SessionId: acp.SessionId(acpSess.id),
|
||||
@@ -960,16 +967,16 @@ func (a *Agent) handleToolCallConfirmation(ctx context.Context, acpSess *Session
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
return err
|
||||
return false, err
|
||||
}
|
||||
|
||||
if permResp.Outcome.Cancelled != nil {
|
||||
acpSess.rt.Resume(ctx, runtime.ResumeRequest{Type: runtime.ResumeTypeReject})
|
||||
return nil
|
||||
return true, nil
|
||||
}
|
||||
|
||||
if permResp.Outcome.Selected == nil {
|
||||
return errors.New("unexpected permission outcome")
|
||||
return false, errors.New("unexpected permission outcome")
|
||||
}
|
||||
|
||||
switch string(permResp.Outcome.Selected.OptionId) {
|
||||
@@ -979,11 +986,12 @@ func (a *Agent) handleToolCallConfirmation(ctx context.Context, acpSess *Session
|
||||
acpSess.rt.Resume(ctx, runtime.ResumeApproveTool(e.ToolCall.Function.Name))
|
||||
case "reject":
|
||||
acpSess.rt.Resume(ctx, runtime.ResumeRequest{Type: runtime.ResumeTypeReject})
|
||||
return true, nil
|
||||
default:
|
||||
return fmt.Errorf("unexpected permission option: %s", permResp.Outcome.Selected.OptionId)
|
||||
return false, fmt.Errorf("unexpected permission option: %s", permResp.Outcome.Selected.OptionId)
|
||||
}
|
||||
|
||||
return nil
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// handleMaxIterationsReached handles max iterations events.
|
||||
|
||||
@@ -180,7 +180,8 @@ func TestPromptDeclinesUnsupportedElicitation(t *testing.T) {
|
||||
assert.Equal(t, "Done", s.sess.GetLastAssistantMessageContent())
|
||||
reqs := peer.recordedRequests()
|
||||
require.Len(t, reqs, (turn+1)*2, "only permission and max-iteration requests should reach the client")
|
||||
assert.Equal(t, acpsdk.ToolCallId("call-1"), reqs[turn*2].ToolCall.ToolCallId)
|
||||
assert.NotEmpty(t, reqs[turn*2].ToolCall.ToolCallId)
|
||||
assert.Equal(t, "ask_user", *reqs[turn*2].ToolCall.Title)
|
||||
assert.Equal(t, acpsdk.ToolCallId("max_iterations"), reqs[turn*2+1].ToolCall.ToolCallId)
|
||||
}
|
||||
assert.Equal(t, 2, loads)
|
||||
|
||||
@@ -300,7 +300,7 @@ func TestRuntimeEditSnapshotSurvivesOutputTransform(t *testing.T) {
|
||||
Params acpsdk.SessionNotification `json:"params"`
|
||||
}
|
||||
require.NoError(t, json.Unmarshal([]byte(line), &message))
|
||||
if update := message.Params.Update.ToolCallUpdate; update != nil && update.ToolCallId == "edit" {
|
||||
if update := message.Params.Update.ToolCallUpdate; update != nil && update.Status != nil && *update.Status == acpsdk.ToolCallStatusCompleted {
|
||||
require.Len(t, update.Content, 2)
|
||||
diff = update.Content[1].Diff
|
||||
assert.Equal(t, "transformed response", update.Content[0].Content.Content.Text.Text)
|
||||
|
||||
@@ -29,7 +29,7 @@ func (r *drainingPromptRuntime) RunStream(ctx context.Context, _ *session.Sessio
|
||||
defer close(events)
|
||||
defer close(r.stopped)
|
||||
if r.fail {
|
||||
events <- &runtime.ToolCallResponseEvent{ToolCallID: "missing-start"}
|
||||
events <- &runtime.ToolCallResponseEvent{ToolCallID: ""}
|
||||
}
|
||||
<-ctx.Done()
|
||||
events <- runtime.Warning("teardown started", "root")
|
||||
@@ -67,7 +67,7 @@ func TestPrompt_DrainsBeforeReleasingTurn(t *testing.T) {
|
||||
synctest.Wait()
|
||||
result := <-done
|
||||
if fail {
|
||||
require.ErrorContains(t, result.err, "missing tool call arguments")
|
||||
require.ErrorContains(t, result.err, "tool call ID is required")
|
||||
} else {
|
||||
require.NoError(t, result.err)
|
||||
assert.Equal(t, acpsdk.StopReasonCancelled, result.response.StopReason)
|
||||
|
||||
+84
-63
@@ -15,6 +15,7 @@ import (
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"testing/synctest"
|
||||
"time"
|
||||
|
||||
acpsdk "github.com/coder/acp-go-sdk"
|
||||
@@ -806,16 +807,16 @@ func TestRunAgent_ToolCallLifecycle(t *testing.T) {
|
||||
|
||||
start := updates[1].ToolCall
|
||||
require.NotNil(t, start)
|
||||
assert.Equal(t, acpsdk.ToolCallId("call-1"), start.ToolCallId)
|
||||
assert.NotEmpty(t, start.ToolCallId)
|
||||
assert.Equal(t, "Run Shell", start.Title)
|
||||
assert.Equal(t, acpsdk.ToolKindExecute, start.Kind)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusPending, start.Status)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusInProgress, start.Status)
|
||||
assert.Equal(t, map[string]any{"command": "ls", "path": "."}, start.RawInput)
|
||||
assert.Equal(t, []acpsdk.ToolCallLocation{{Path: wd}}, start.Locations)
|
||||
|
||||
completed := updates[2].ToolCallUpdate
|
||||
require.NotNil(t, completed)
|
||||
assert.Equal(t, acpsdk.ToolCallId("call-1"), completed.ToolCallId)
|
||||
assert.Equal(t, start.ToolCallId, completed.ToolCallId)
|
||||
require.NotNil(t, completed.Status)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusCompleted, *completed.Status)
|
||||
require.Len(t, completed.Content, 1)
|
||||
@@ -827,7 +828,7 @@ func TestRunAgent_ToolCallLifecycle(t *testing.T) {
|
||||
require.NotNil(t, updates[3].ToolCall)
|
||||
failed := updates[4].ToolCallUpdate
|
||||
require.NotNil(t, failed)
|
||||
assert.Equal(t, acpsdk.ToolCallId("call-2"), failed.ToolCallId)
|
||||
assert.Equal(t, updates[3].ToolCall.ToolCallId, failed.ToolCallId)
|
||||
require.NotNil(t, failed.Status)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusFailed, *failed.Status)
|
||||
|
||||
@@ -850,22 +851,29 @@ func TestRunAgent_ToolCallLifecycle(t *testing.T) {
|
||||
assert.Empty(t, rt.resumeRequests())
|
||||
}
|
||||
|
||||
func TestRunAgent_ToolCallResponseWithoutStartFails(t *testing.T) {
|
||||
func TestRunAgent_ToolCallResponseWithoutStartCreatesTerminalCall(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
rt := &fakeRuntime{events: []runtime.Event{
|
||||
runtime.ToolCallResponse("orphan", tools.Tool{Name: "shell"}, tools.ResultSuccess("ok"), "ok", "root"),
|
||||
runtime.AgentChoice("root", testSessionID, "never emitted"),
|
||||
}}
|
||||
f := newRunAgentFixture(t, rt, &captureWriter{})
|
||||
|
||||
err := f.runAgent(t.Context(), f.sess)
|
||||
require.EqualError(t, err, "missing tool call arguments for tool call ID orphan")
|
||||
|
||||
// The failure must stop the loop before later events are mapped.
|
||||
updates := f.sessionUpdates(t)
|
||||
require.Len(t, updates, 1)
|
||||
requireAvailableCommands(t, updates[0])
|
||||
for _, failed := range []bool{false, true} {
|
||||
result := tools.ResultSuccess("result")
|
||||
status := acpsdk.ToolCallStatusCompleted
|
||||
if failed {
|
||||
result = tools.ResultError("rejected")
|
||||
status = acpsdk.ToolCallStatusFailed
|
||||
}
|
||||
rt := &fakeRuntime{events: []runtime.Event{
|
||||
runtime.ToolCallResponse("orphan", tools.Tool{Name: "shell"}, result, result.Output, "root"),
|
||||
runtime.AgentChoice("root", testSessionID, "continued"),
|
||||
}}
|
||||
f := newRunAgentFixture(t, rt, &captureWriter{})
|
||||
require.NoError(t, f.runAgent(t.Context(), f.sess))
|
||||
updates := f.sessionUpdates(t)
|
||||
require.Len(t, updates, 3)
|
||||
require.NotNil(t, updates[1].ToolCall)
|
||||
assert.Equal(t, status, updates[1].ToolCall.Status)
|
||||
assert.Equal(t, "shell", updates[1].ToolCall.Title)
|
||||
assert.Nil(t, updates[1].ToolCall.RawInput)
|
||||
assert.Equal(t, "continued", agentMessageText(t, updates[2]))
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunAgent_ToolCallConfirmationRequestFields(t *testing.T) {
|
||||
@@ -890,7 +898,7 @@ func TestRunAgent_ToolCallConfirmationRequestFields(t *testing.T) {
|
||||
require.Len(t, reqs, 1)
|
||||
req := reqs[0]
|
||||
assert.Equal(t, acpsdk.SessionId(testSessionID), req.SessionId)
|
||||
assert.Equal(t, acpsdk.ToolCallId("confirm-1"), req.ToolCall.ToolCallId)
|
||||
assert.NotEmpty(t, req.ToolCall.ToolCallId)
|
||||
require.NotNil(t, req.ToolCall.Title)
|
||||
assert.Equal(t, "Run Shell", *req.ToolCall.Title)
|
||||
require.NotNil(t, req.ToolCall.Kind)
|
||||
@@ -909,9 +917,14 @@ func TestRunAgent_ToolCallConfirmationRequestFields(t *testing.T) {
|
||||
// The turn keeps streaming after the permission round trip and the
|
||||
// interleaved request does not disturb the captured notifications.
|
||||
updates := f.sessionUpdates(t)
|
||||
require.Len(t, updates, 2)
|
||||
require.Len(t, updates, 4)
|
||||
requireAvailableCommands(t, updates[0])
|
||||
assert.Equal(t, "approved", agentMessageText(t, updates[1]))
|
||||
require.NotNil(t, updates[1].ToolCall)
|
||||
assert.Equal(t, req.ToolCall.ToolCallId, updates[1].ToolCall.ToolCallId)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusPending, updates[1].ToolCall.Status)
|
||||
assert.Equal(t, "approved", agentMessageText(t, updates[2]))
|
||||
assert.Equal(t, req.ToolCall.ToolCallId, updates[3].ToolCallUpdate.ToolCallId)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusFailed, *updates[3].ToolCallUpdate.Status)
|
||||
}
|
||||
|
||||
func TestRunAgent_ToolCallConfirmationOutcomes(t *testing.T) {
|
||||
@@ -1001,48 +1014,52 @@ func (p *approvalTestProvider) CreateChatCompletionStream(context.Context, []cha
|
||||
func TestRunAgent_AlwaysAllowKeepsOtherToolsGated(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var executed []string
|
||||
handler := func(_ context.Context, call tools.ToolCall, _ tools.Runtime) (*tools.ToolCallResult, error) {
|
||||
executed = append(executed, call.Function.Name)
|
||||
return tools.ResultSuccess("ok"), nil
|
||||
}
|
||||
prov := &approvalTestProvider{
|
||||
mockProvider: mockProvider{id: modelsdev.NewID("test", "approval")},
|
||||
toolNames: []string{"allowed_tool", "allowed_tool", "other_tool"},
|
||||
}
|
||||
root := agentpkg.New("root", "You are a test agent", agentpkg.WithModel(prov), agentpkg.WithTools(
|
||||
tools.Tool{Name: "allowed_tool", Parameters: map[string]any{"type": "object"}, Handler: handler},
|
||||
tools.Tool{Name: "other_tool", Parameters: map[string]any{"type": "object"}, Handler: handler},
|
||||
))
|
||||
rt, err := runtime.New(t.Context(), team.New(team.WithAgents(root)), runtime.WithSessionCompaction(false))
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { require.NoError(t, rt.Close()) })
|
||||
|
||||
f := newRunAgentFixtureWithPermissions(t, &fakeRuntime{}, &captureWriter{}, func(req acpsdk.RequestPermissionRequest) any {
|
||||
if req.ToolCall.ToolCallId == "call-1" {
|
||||
return permissionSelected("allow-always")
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
var executed []string
|
||||
handler := func(_ context.Context, call tools.ToolCall, _ tools.Runtime) (*tools.ToolCallResult, error) {
|
||||
executed = append(executed, call.Function.Name)
|
||||
return tools.ResultSuccess("ok"), nil
|
||||
}
|
||||
return permissionSelected("allow")
|
||||
prov := &approvalTestProvider{
|
||||
mockProvider: mockProvider{id: modelsdev.NewID("test", "approval")},
|
||||
toolNames: []string{"allowed_tool", "allowed_tool", "other_tool"},
|
||||
}
|
||||
root := agentpkg.New("root", "You are a test agent", agentpkg.WithModel(prov), agentpkg.WithTools(
|
||||
tools.Tool{Name: "allowed_tool", Parameters: map[string]any{"type": "object"}, Handler: handler},
|
||||
tools.Tool{Name: "other_tool", Parameters: map[string]any{"type": "object"}, Handler: handler},
|
||||
))
|
||||
rt, err := runtime.New(t.Context(), team.New(team.WithAgents(root)), runtime.WithSessionCompaction(false))
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { require.NoError(t, rt.Close()) })
|
||||
|
||||
f := newRunAgentFixtureWithPermissions(t, &fakeRuntime{}, &captureWriter{}, func(req acpsdk.RequestPermissionRequest) any {
|
||||
synctest.Wait()
|
||||
if req.ToolCall.Title != nil && *req.ToolCall.Title == "allowed_tool" {
|
||||
return permissionSelected("allow-always")
|
||||
}
|
||||
return permissionSelected("allow")
|
||||
})
|
||||
f.sess.rt = rt
|
||||
f.sess.sess = session.New(
|
||||
session.WithUserMessage("Run the tools"),
|
||||
session.WithSafetyPolicy(session.SafetyPolicyStrict),
|
||||
)
|
||||
|
||||
ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
|
||||
defer cancel()
|
||||
require.NoError(t, f.runAgent(ctx, f.sess))
|
||||
|
||||
assert.Equal(t, []string{"allowed_tool", "allowed_tool", "other_tool"}, executed)
|
||||
reqs := f.peer.recordedRequests()
|
||||
require.Len(t, reqs, 2)
|
||||
assert.Equal(t, "allowed_tool", *reqs[0].ToolCall.Title)
|
||||
assert.Equal(t, "other_tool", *reqs[1].ToolCall.Title)
|
||||
assert.NotEqual(t, reqs[0].ToolCall.ToolCallId, reqs[1].ToolCall.ToolCallId)
|
||||
assert.Equal(t, session.SafetyPolicyStrict, f.sess.sess.GetSafetyPolicy())
|
||||
perms := f.sess.sess.ClonePermissions()
|
||||
require.NotNil(t, perms)
|
||||
assert.Equal(t, []string{"allowed_tool"}, perms.Allow)
|
||||
})
|
||||
f.sess.rt = rt
|
||||
f.sess.sess = session.New(
|
||||
session.WithUserMessage("Run the tools"),
|
||||
session.WithSafetyPolicy(session.SafetyPolicyStrict),
|
||||
)
|
||||
|
||||
ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
|
||||
defer cancel()
|
||||
require.NoError(t, f.runAgent(ctx, f.sess))
|
||||
|
||||
assert.Equal(t, []string{"allowed_tool", "allowed_tool", "other_tool"}, executed)
|
||||
reqs := f.peer.recordedRequests()
|
||||
require.Len(t, reqs, 2)
|
||||
assert.Equal(t, acpsdk.ToolCallId("call-1"), reqs[0].ToolCall.ToolCallId)
|
||||
assert.Equal(t, acpsdk.ToolCallId("call-3"), reqs[1].ToolCall.ToolCallId)
|
||||
assert.Equal(t, session.SafetyPolicyStrict, f.sess.sess.GetSafetyPolicy())
|
||||
perms := f.sess.sess.ClonePermissions()
|
||||
require.NotNil(t, perms)
|
||||
assert.Equal(t, []string{"allowed_tool"}, perms.Allow)
|
||||
}
|
||||
|
||||
func TestRunAgent_ToolCallConfirmationBadOutcomeFailsRun(t *testing.T) {
|
||||
@@ -1089,8 +1106,12 @@ func TestRunAgent_ToolCallConfirmationBadOutcomeFailsRun(t *testing.T) {
|
||||
assert.Empty(t, rt.resumeRequests())
|
||||
// The failure must stop the loop before later events are mapped.
|
||||
updates := f.sessionUpdates(t)
|
||||
require.Len(t, updates, 1)
|
||||
require.Len(t, updates, 3)
|
||||
requireAvailableCommands(t, updates[0])
|
||||
require.NotNil(t, updates[1].ToolCall)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusPending, updates[1].ToolCall.Status)
|
||||
require.NotNil(t, updates[2].ToolCallUpdate)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusFailed, *updates[2].ToolCallUpdate.Status)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
package acp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"time"
|
||||
|
||||
"github.com/coder/acp-go-sdk"
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/docker/docker-agent/pkg/runtime"
|
||||
"github.com/docker/docker-agent/pkg/tools"
|
||||
)
|
||||
|
||||
type toolCallKey struct {
|
||||
agent string
|
||||
id string
|
||||
}
|
||||
|
||||
type toolCallState struct {
|
||||
id acp.ToolCallId
|
||||
rejected bool
|
||||
result *runtime.ToolCallResponseEvent
|
||||
}
|
||||
|
||||
type toolCallTracker struct {
|
||||
active map[toolCallKey]*toolCallState
|
||||
}
|
||||
|
||||
func (t *toolCallTracker) report(ctx context.Context, a *Agent, s *Session, agentName string, call tools.ToolCall, definition tools.Tool, status acp.ToolCallStatus) (*toolCallState, error) {
|
||||
if call.ID == "" {
|
||||
return nil, errors.New("tool call ID is required")
|
||||
}
|
||||
key := toolCallKey{agent: agentName, id: call.ID}
|
||||
state, exists := t.active[key]
|
||||
if exists && state.rejected && status == acp.ToolCallStatusInProgress {
|
||||
// Out-of-band rejections can synthesize a start without executing anything.
|
||||
return state, nil
|
||||
}
|
||||
if !exists {
|
||||
state = &toolCallState{id: acp.ToolCallId(uuid.NewString())}
|
||||
if t.active == nil {
|
||||
t.active = make(map[toolCallKey]*toolCallState)
|
||||
}
|
||||
}
|
||||
call.ID = string(state.id)
|
||||
workingDir, _ := s.workspaceSnapshot()
|
||||
var update acp.SessionUpdate
|
||||
if exists {
|
||||
fields := buildToolCallUpdate(call, definition, status, workingDir)
|
||||
update = acp.UpdateToolCall(state.id,
|
||||
acp.WithUpdateTitle(*fields.Title), acp.WithUpdateKind(*fields.Kind),
|
||||
acp.WithUpdateStatus(status), acp.WithUpdateRawInput(fields.RawInput), acp.WithUpdateLocations(fields.Locations),
|
||||
)
|
||||
} else {
|
||||
update = buildToolCallStart(call, definition, workingDir)
|
||||
update.ToolCall.Status = status
|
||||
}
|
||||
if err := a.sendUpdate(ctx, s.id, update); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
t.active[key] = state
|
||||
return state, nil
|
||||
}
|
||||
|
||||
func (t *toolCallTracker) complete(ctx context.Context, a *Agent, s *Session, event *runtime.ToolCallResponseEvent) error {
|
||||
if event.ToolCallID == "" {
|
||||
return errors.New("tool call ID is required")
|
||||
}
|
||||
key := toolCallKey{agent: event.AgentName, id: event.ToolCallID}
|
||||
state, exists := t.active[key]
|
||||
if !exists {
|
||||
state = &toolCallState{id: acp.ToolCallId(uuid.NewString())}
|
||||
}
|
||||
mappedEvent := *event
|
||||
mappedEvent.ToolCallID = string(state.id)
|
||||
update := buildToolCallComplete(&mappedEvent)
|
||||
if !exists {
|
||||
fields := update.ToolCallUpdate
|
||||
title := event.ToolDefinition.Annotations.Title
|
||||
if title == "" {
|
||||
title = event.ToolDefinition.Name
|
||||
}
|
||||
if title == "" {
|
||||
title = "Tool call"
|
||||
}
|
||||
update = acp.StartToolCall(state.id, title,
|
||||
acp.WithStartKind(determineToolKind(event.ToolDefinition.Name, event.ToolDefinition)),
|
||||
acp.WithStartStatus(*fields.Status), acp.WithStartContent(fields.Content), acp.WithStartRawOutput(fields.RawOutput),
|
||||
)
|
||||
}
|
||||
// Never retry a terminal write: a writer error can mean partial delivery.
|
||||
delete(t.active, key)
|
||||
sendCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Second)
|
||||
defer cancel()
|
||||
return a.sendUpdate(sendCtx, s.id, update)
|
||||
}
|
||||
|
||||
func (t *toolCallTracker) retainResult(event runtime.Event) {
|
||||
if response, ok := event.(*runtime.ToolCallResponseEvent); ok {
|
||||
if state := t.active[toolCallKey{agent: response.AgentName, id: response.ToolCallID}]; state != nil {
|
||||
state.result = response
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (t *toolCallTracker) interrupt(ctx context.Context, a *Agent, s *Session) error {
|
||||
for key, state := range t.active {
|
||||
if state.result != nil {
|
||||
if err := t.complete(ctx, a, s, state.result); err != nil {
|
||||
return err
|
||||
}
|
||||
continue
|
||||
}
|
||||
update := acp.UpdateToolCall(state.id,
|
||||
acp.WithUpdateStatus(acp.ToolCallStatusFailed),
|
||||
acp.WithUpdateContent([]acp.ToolCallContent{acp.ToolContent(acp.TextBlock("Tool call interrupted before a result was available; side effects may have occurred."))}),
|
||||
)
|
||||
if err := a.sendUpdate(ctx, s.id, update); err != nil {
|
||||
return err
|
||||
}
|
||||
delete(t.active, key)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,358 @@
|
||||
package acp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"testing/synctest"
|
||||
"time"
|
||||
|
||||
acpsdk "github.com/coder/acp-go-sdk"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/docker/docker-agent/pkg/agent"
|
||||
"github.com/docker/docker-agent/pkg/modelsdev"
|
||||
"github.com/docker/docker-agent/pkg/runtime"
|
||||
"github.com/docker/docker-agent/pkg/session"
|
||||
"github.com/docker/docker-agent/pkg/team"
|
||||
"github.com/docker/docker-agent/pkg/tools"
|
||||
)
|
||||
|
||||
func toolUpdates(updates []acpsdk.SessionUpdate) []acpsdk.SessionUpdate {
|
||||
var result []acpsdk.SessionUpdate
|
||||
for _, update := range updates {
|
||||
if update.ToolCall != nil || update.ToolCallUpdate != nil {
|
||||
result = append(result, update)
|
||||
}
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
func TestToolLifecyclePendingRunningCompletion(t *testing.T) {
|
||||
t.Parallel()
|
||||
wd := t.TempDir()
|
||||
call := tools.ToolCall{ID: "runtime-id", Function: tools.FunctionCall{Name: "edit_file", Arguments: `{"file":"before.txt"}`}}
|
||||
changed := call
|
||||
changed.Function.Arguments = `{"file":"after.txt"}`
|
||||
tool := tools.Tool{Name: "edit_file", Annotations: tools.ToolAnnotations{Title: "Edit", DestructiveHint: new(true)}}
|
||||
rt := &fakeRuntime{events: []runtime.Event{
|
||||
runtime.ToolCallConfirmation(call, tool, "root", nil),
|
||||
runtime.ToolCall(changed, tool, "root"),
|
||||
runtime.ToolCallResponse(call.ID, tool, tools.ResultSuccess("done"), "done", "root"),
|
||||
}}
|
||||
f := newRunAgentFixtureWithPermissions(t, rt, &captureWriter{}, func(acpsdk.RequestPermissionRequest) any { return permissionSelected("allow") })
|
||||
f.sess.workingDir = wd
|
||||
require.NoError(t, f.runAgent(t.Context(), f.sess))
|
||||
updates := toolUpdates(f.sessionUpdates(t))
|
||||
require.Len(t, updates, 3)
|
||||
pending, running, finished := updates[0].ToolCall, updates[1].ToolCallUpdate, updates[2].ToolCallUpdate
|
||||
require.NotNil(t, pending)
|
||||
require.NotNil(t, running)
|
||||
require.NotNil(t, finished)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusPending, pending.Status)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusInProgress, *running.Status)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusCompleted, *finished.Status)
|
||||
assert.Equal(t, pending.ToolCallId, running.ToolCallId)
|
||||
assert.Equal(t, pending.ToolCallId, finished.ToolCallId)
|
||||
req := f.peer.recordedRequests()[0]
|
||||
assert.Equal(t, pending.ToolCallId, req.ToolCall.ToolCallId)
|
||||
assert.Equal(t, acpsdk.ToolKindEdit, pending.Kind)
|
||||
assert.Equal(t, pending.Kind, *req.ToolCall.Kind)
|
||||
assert.Equal(t, pending.Kind, *running.Kind)
|
||||
assert.Equal(t, map[string]any{"file": "before.txt"}, pending.RawInput)
|
||||
assert.Equal(t, pending.RawInput, req.ToolCall.RawInput)
|
||||
assert.Equal(t, map[string]any{"file": "after.txt"}, running.RawInput)
|
||||
assert.Equal(t, filepath.Join(wd, "after.txt"), running.Locations[0].Path)
|
||||
}
|
||||
|
||||
func TestToolLifecycleSuppressesRejectedSyntheticStart(t *testing.T) {
|
||||
t.Parallel()
|
||||
for _, outcome := range []string{"reject", "cancelled"} {
|
||||
t.Run(outcome, func(t *testing.T) {
|
||||
call := tools.ToolCall{ID: "nested", Function: tools.FunctionCall{Name: "shell", Arguments: `{"cmd":"never execute"}`}}
|
||||
tool := tools.Tool{Name: "shell"}
|
||||
rt := &fakeRuntime{events: []runtime.Event{
|
||||
runtime.ToolCallConfirmation(call, tool, "root", nil),
|
||||
runtime.ToolCall(call, tool, "root"),
|
||||
runtime.ToolCallResponse(call.ID, tool, tools.ResultError("rejected"), "rejected", "root"),
|
||||
}}
|
||||
f := newRunAgentFixtureWithPermissions(t, rt, &captureWriter{}, func(acpsdk.RequestPermissionRequest) any {
|
||||
if outcome == "cancelled" {
|
||||
return map[string]any{"outcome": map[string]any{"outcome": "cancelled"}}
|
||||
}
|
||||
return permissionSelected("reject")
|
||||
})
|
||||
require.NoError(t, f.runAgent(t.Context(), f.sess))
|
||||
updates := toolUpdates(f.sessionUpdates(t))
|
||||
require.Len(t, updates, 2)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusPending, updates[0].ToolCall.Status)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusFailed, *updates[1].ToolCallUpdate.Status)
|
||||
assert.Equal(t, updates[0].ToolCall.ToolCallId, updates[1].ToolCallUpdate.ToolCallId)
|
||||
assert.Equal(t, []runtime.ResumeRequest{{Type: runtime.ResumeTypeReject}}, rt.resumeRequests())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolLifecycleScopesAndReusesRuntimeIDs(t *testing.T) {
|
||||
t.Parallel()
|
||||
call := tools.ToolCall{ID: "same", Function: tools.FunctionCall{Name: "shell", Arguments: "{}"}}
|
||||
tool := tools.Tool{Name: "shell"}
|
||||
rt := &fakeRuntime{events: []runtime.Event{
|
||||
runtime.ToolCall(call, tool, "root"), runtime.ToolCall(call, tool, "worker"),
|
||||
runtime.ToolCallResponse("same", tool, tools.ResultSuccess("worker"), "worker", "worker"),
|
||||
runtime.ToolCallResponse("same", tool, tools.ResultSuccess("root"), "root", "root"),
|
||||
runtime.ToolCall(call, tool, "root"), runtime.ToolCallResponse("same", tool, tools.ResultSuccess("reused"), "reused", "root"),
|
||||
}}
|
||||
f := newRunAgentFixture(t, rt, &captureWriter{})
|
||||
require.NoError(t, f.runAgent(t.Context(), f.sess))
|
||||
updates := toolUpdates(f.sessionUpdates(t))
|
||||
require.Len(t, updates, 6)
|
||||
root, worker, reused := updates[0].ToolCall.ToolCallId, updates[1].ToolCall.ToolCallId, updates[4].ToolCall.ToolCallId
|
||||
assert.NotEqual(t, root, worker)
|
||||
assert.NotEqual(t, root, reused)
|
||||
assert.NotEqual(t, worker, reused)
|
||||
assert.Equal(t, worker, updates[2].ToolCallUpdate.ToolCallId)
|
||||
assert.Equal(t, root, updates[3].ToolCallUpdate.ToolCallId)
|
||||
assert.Equal(t, reused, updates[5].ToolCallUpdate.ToolCallId)
|
||||
require.NoError(t, f.runAgent(t.Context(), f.sess))
|
||||
all := toolUpdates(f.sessionUpdates(t))
|
||||
assert.NotEqual(t, root, all[6].ToolCall.ToolCallId)
|
||||
}
|
||||
|
||||
func TestToolLifecycleRuntimeRejectAndPolicyDeny(t *testing.T) {
|
||||
t.Parallel()
|
||||
for _, mode := range []string{"allow", "reject", "policy deny", "unavailable"} {
|
||||
t.Run(mode, func(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
var executed bool
|
||||
tool := tools.Tool{Name: "change", Parameters: map[string]any{"type": "object"}, Handler: func(context.Context, tools.ToolCall, tools.Runtime) (*tools.ToolCallResult, error) {
|
||||
executed = true
|
||||
return tools.ResultSuccess("changed"), nil
|
||||
}}
|
||||
prov := &elicitationTestProvider{approvalTestProvider: approvalTestProvider{mockProvider: mockProvider{id: modelsdev.NewID("test", "lifecycle")}, toolNames: []string{"change"}}}
|
||||
root := agent.New("root", "test", agent.WithModel(prov))
|
||||
if mode != "unavailable" {
|
||||
agent.WithTools(tool)(root)
|
||||
}
|
||||
rt, err := runtime.New(t.Context(), team.New(team.WithAgents(root)), runtime.WithSessionCompaction(false))
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { require.NoError(t, rt.Close()) })
|
||||
f := newRunAgentFixtureWithPermissions(t, &fakeRuntime{}, &captureWriter{}, func(acpsdk.RequestPermissionRequest) any { synctest.Wait(); return permissionSelected(mode) })
|
||||
f.sess.rt = rt
|
||||
f.sess.sess.SetSafetyPolicy(session.SafetyPolicyStrict)
|
||||
if mode == "policy deny" {
|
||||
f.sess.sess.Permissions = &session.PermissionsConfig{Deny: []string{"change"}}
|
||||
}
|
||||
f.agent.sessions[testSessionID] = f.sess
|
||||
response, err := f.agent.Prompt(t.Context(), promptRequest("do work"))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, acpsdk.StopReasonEndTurn, response.StopReason)
|
||||
assert.Equal(t, "Done", f.sess.sess.GetLastAssistantMessageContent())
|
||||
assert.Equal(t, mode == "allow", executed)
|
||||
updates := toolUpdates(f.sessionUpdates(t))
|
||||
switch mode {
|
||||
case "allow":
|
||||
require.Len(t, updates, 3)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusInProgress, *updates[1].ToolCallUpdate.Status)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusCompleted, *updates[2].ToolCallUpdate.Status)
|
||||
case "reject":
|
||||
require.Len(t, updates, 2)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusFailed, *updates[1].ToolCallUpdate.Status)
|
||||
default:
|
||||
require.Len(t, updates, 1)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusFailed, updates[0].ToolCall.Status)
|
||||
assert.Nil(t, updates[0].ToolCall.RawInput)
|
||||
}
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
type lateToolResultRuntime struct {
|
||||
fakeRuntime
|
||||
|
||||
started chan struct{}
|
||||
resultReady chan struct{}
|
||||
resultSent chan struct{}
|
||||
finish chan struct{}
|
||||
}
|
||||
|
||||
func (r *lateToolResultRuntime) RunStream(ctx context.Context, _ *session.Session) <-chan runtime.Event {
|
||||
events := make(chan runtime.Event)
|
||||
go func() {
|
||||
defer close(events)
|
||||
call := tools.ToolCall{ID: "call", Function: tools.FunctionCall{Name: "shell", Arguments: "{}"}}
|
||||
events <- runtime.ToolCall(call, tools.Tool{Name: "shell"}, "root")
|
||||
close(r.started)
|
||||
<-r.resultReady
|
||||
events <- runtime.ToolCallResponse("call", tools.Tool{Name: "shell"}, tools.ResultSuccess("actual result"), "actual result", "root")
|
||||
close(r.resultSent)
|
||||
<-r.finish
|
||||
}()
|
||||
return events
|
||||
}
|
||||
|
||||
func TestCanceledPromptPreservesLateToolResultAfterSlowDrain(t *testing.T) {
|
||||
t.Parallel()
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
rt := &lateToolResultRuntime{started: make(chan struct{}), resultReady: make(chan struct{}), resultSent: make(chan struct{}), finish: make(chan struct{})}
|
||||
a, _, peer := newPromptTestAgent(t, rt)
|
||||
done := promptAsync(a, t.Context(), promptRequest("work"))
|
||||
<-rt.started
|
||||
synctest.Wait()
|
||||
require.NoError(t, a.Cancel(t.Context(), acpsdk.CancelNotification{SessionId: testSessionID}))
|
||||
close(rt.resultReady)
|
||||
<-rt.resultSent
|
||||
// Runtime teardown may outlive the final-notification attempt budget.
|
||||
time.Sleep(3 * time.Second) //nolint:forbidigo // fake time advances a deliberately slow teardown
|
||||
close(rt.finish)
|
||||
result := <-done
|
||||
require.NoError(t, result.err)
|
||||
assert.Equal(t, acpsdk.StopReasonCancelled, result.response.StopReason)
|
||||
out := peer.out.(*captureWriter)
|
||||
assert.Contains(t, strings.Join(out.lines(), "\n"), `"status":"completed"`)
|
||||
assert.Contains(t, strings.Join(out.lines(), "\n"), "actual result")
|
||||
assert.NotContains(t, strings.Join(out.lines(), "\n"), "interrupted")
|
||||
})
|
||||
}
|
||||
|
||||
func TestToolCompletionSurvivesCancellationBeforeSend(t *testing.T) {
|
||||
t.Parallel()
|
||||
f := newRunAgentFixture(t, &fakeRuntime{}, &captureWriter{})
|
||||
var tracker toolCallTracker
|
||||
call := tools.ToolCall{ID: "call", Function: tools.FunctionCall{Name: "shell", Arguments: "{}"}}
|
||||
_, err := tracker.report(t.Context(), f.agent, f.sess, "root", call, tools.Tool{Name: "shell"}, acpsdk.ToolCallStatusInProgress)
|
||||
require.NoError(t, err)
|
||||
ctx, cancel := context.WithCancel(t.Context())
|
||||
cancel()
|
||||
err = tracker.complete(ctx, f.agent, f.sess, runtime.ToolCallResponse("call", tools.Tool{Name: "shell"}, tools.ResultSuccess("actual result"), "actual result", "root").(*runtime.ToolCallResponseEvent))
|
||||
require.NoError(t, err)
|
||||
updates := toolUpdates(f.sessionUpdates(t))
|
||||
require.Len(t, updates, 2)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusCompleted, *updates[1].ToolCallUpdate.Status)
|
||||
assert.Empty(t, tracker.active)
|
||||
}
|
||||
|
||||
func TestToolCompletionWriteFailureIsNotRetried(t *testing.T) {
|
||||
t.Parallel()
|
||||
out := &captureWriter{failOn: func(n int) error {
|
||||
if n == 2 {
|
||||
return io.ErrClosedPipe
|
||||
}
|
||||
return nil
|
||||
}}
|
||||
f := newRunAgentFixture(t, &fakeRuntime{}, out)
|
||||
var tracker toolCallTracker
|
||||
call := tools.ToolCall{ID: "call", Function: tools.FunctionCall{Name: "shell", Arguments: "{}"}}
|
||||
_, err := tracker.report(t.Context(), f.agent, f.sess, "root", call, tools.Tool{Name: "shell"}, acpsdk.ToolCallStatusInProgress)
|
||||
require.NoError(t, err)
|
||||
err = tracker.complete(t.Context(), f.agent, f.sess, runtime.ToolCallResponse("call", tools.Tool{Name: "shell"}, tools.ResultSuccess("done"), "done", "root").(*runtime.ToolCallResponseEvent))
|
||||
require.Error(t, err)
|
||||
require.NoError(t, tracker.interrupt(t.Context(), f.agent, f.sess))
|
||||
assert.Equal(t, 2, out.writes)
|
||||
}
|
||||
|
||||
type interruptedToolRuntime struct {
|
||||
fakeRuntime
|
||||
|
||||
started chan struct{}
|
||||
release chan struct{}
|
||||
fail bool
|
||||
}
|
||||
|
||||
func (r *interruptedToolRuntime) RunStream(ctx context.Context, _ *session.Session) <-chan runtime.Event {
|
||||
events := make(chan runtime.Event)
|
||||
go func() {
|
||||
defer close(events)
|
||||
call := tools.ToolCall{ID: "call", Function: tools.FunctionCall{Name: "shell", Arguments: "{}"}}
|
||||
events <- runtime.ToolCall(call, tools.Tool{Name: "shell"}, "root")
|
||||
close(r.started)
|
||||
if r.fail {
|
||||
events <- &runtime.ToolCallResponseEvent{}
|
||||
}
|
||||
<-ctx.Done()
|
||||
<-r.release
|
||||
}()
|
||||
return events
|
||||
}
|
||||
|
||||
func TestInterruptedToolWithoutResultClosesAfterDrain(t *testing.T) {
|
||||
t.Parallel()
|
||||
for _, failure := range []bool{false, true} {
|
||||
name := "cancellation"
|
||||
if failure {
|
||||
name = "handler error"
|
||||
}
|
||||
t.Run(name, func(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
rt := &interruptedToolRuntime{started: make(chan struct{}), release: make(chan struct{}), fail: failure}
|
||||
a, _, peer := newPromptTestAgent(t, rt)
|
||||
done := promptAsync(a, t.Context(), promptRequest("work"))
|
||||
<-rt.started
|
||||
synctest.Wait()
|
||||
if !failure {
|
||||
require.NoError(t, a.Cancel(t.Context(), acpsdk.CancelNotification{SessionId: testSessionID}))
|
||||
}
|
||||
synctest.Wait()
|
||||
select {
|
||||
case <-done:
|
||||
t.Fatal("prompt returned before runtime teardown")
|
||||
default:
|
||||
}
|
||||
out := peer.out.(*captureWriter)
|
||||
assert.NotContains(t, strings.Join(out.lines(), "\n"), `"status":"failed"`)
|
||||
close(rt.release)
|
||||
result := <-done
|
||||
if failure {
|
||||
require.EqualError(t, result.err, "tool call ID is required")
|
||||
} else {
|
||||
require.NoError(t, result.err)
|
||||
assert.Equal(t, acpsdk.StopReasonCancelled, result.response.StopReason)
|
||||
}
|
||||
f := &runAgentFixture{out: out}
|
||||
updates := toolUpdates(f.sessionUpdates(t))
|
||||
require.Len(t, updates, 2)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusInProgress, updates[0].ToolCall.Status)
|
||||
assert.Equal(t, updates[0].ToolCall.ToolCallId, updates[1].ToolCallUpdate.ToolCallId)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusFailed, *updates[1].ToolCallUpdate.Status)
|
||||
assert.Contains(t, updates[1].ToolCallUpdate.Content[0].Content.Content.Text.Text, "side effects may have occurred")
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolLifecycleDoesNotExposePartialArgumentsOrOutput(t *testing.T) {
|
||||
t.Parallel()
|
||||
call := tools.ToolCall{ID: "call", Function: tools.FunctionCall{Name: "shell", Arguments: `{"secret":"partial`}}
|
||||
rt := &fakeRuntime{events: []runtime.Event{
|
||||
runtime.PartialToolCall(call, tools.Tool{Name: "shell"}, "root"),
|
||||
runtime.ToolCallOutput(call.ID, tools.Tool{Name: "shell"}, "unbuffered-secret", "root"),
|
||||
}}
|
||||
f := newRunAgentFixture(t, rt, &captureWriter{})
|
||||
require.NoError(t, f.runAgent(t.Context(), f.sess))
|
||||
assert.Empty(t, toolUpdates(f.sessionUpdates(t)))
|
||||
assert.NotContains(t, strings.Join(f.out.lines(), "\n"), "secret")
|
||||
}
|
||||
|
||||
func TestToolLifecycleRepeatedStartUpdatesExistingCall(t *testing.T) {
|
||||
t.Parallel()
|
||||
call := tools.ToolCall{ID: "call", Function: tools.FunctionCall{Name: "shell", Arguments: "{}"}}
|
||||
tool := tools.Tool{Name: "shell"}
|
||||
start := runtime.ToolCall(call, tool, "root")
|
||||
response := runtime.ToolCallResponse("call", tool, tools.ResultSuccess("private result"), "transformed result", "root")
|
||||
rt := &fakeRuntime{events: []runtime.Event{start, start, response}}
|
||||
f := newRunAgentFixture(t, rt, &captureWriter{})
|
||||
require.NoError(t, f.runAgent(t.Context(), f.sess))
|
||||
updates := toolUpdates(f.sessionUpdates(t))
|
||||
require.Len(t, updates, 3)
|
||||
assert.Equal(t, updates[0].ToolCall.ToolCallId, updates[1].ToolCallUpdate.ToolCallId)
|
||||
assert.Equal(t, updates[0].ToolCall.ToolCallId, updates[2].ToolCallUpdate.ToolCallId)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusInProgress, *updates[1].ToolCallUpdate.Status)
|
||||
assert.Equal(t, map[string]any{"content": "transformed result"}, updates[2].ToolCallUpdate.RawOutput)
|
||||
assert.NotContains(t, strings.Join(f.out.lines(), "\n"), "private result")
|
||||
assert.Equal(t, "call", start.(*runtime.ToolCallEvent).ToolCall.ID)
|
||||
assert.Equal(t, "call", response.(*runtime.ToolCallResponseEvent).ToolCallID)
|
||||
}
|
||||
+9
-16
@@ -29,12 +29,13 @@ func buildToolCallStart(toolCall tools.ToolCall, tool tools.Tool, workingDir str
|
||||
|
||||
opts := []acp.ToolCallStartOpt{
|
||||
acp.WithStartKind(kind),
|
||||
acp.WithStartStatus(acp.ToolCallStatusPending),
|
||||
acp.WithStartStatus(acp.ToolCallStatusInProgress),
|
||||
acp.WithStartRawInput(args),
|
||||
}
|
||||
|
||||
if len(locations) > 0 {
|
||||
opts = append(opts, acp.WithStartLocations(locations))
|
||||
// WithStartLocations also injects rawInput.path, changing non-path arguments.
|
||||
opts = append(opts, func(call *acp.SessionUpdateToolCall) { call.Locations = locations })
|
||||
}
|
||||
|
||||
return acp.StartToolCall(
|
||||
@@ -68,13 +69,9 @@ func buildToolCallComplete(event *runtime.ToolCallResponseEvent) acp.SessionUpda
|
||||
|
||||
// buildToolCallUpdate creates a tool call update for permission requests.
|
||||
func buildToolCallUpdate(toolCall tools.ToolCall, tool tools.Tool, status acp.ToolCallStatus, workingDir string) acp.ToolCallUpdate {
|
||||
kind := acp.ToolKindExecute
|
||||
kind := determineToolKind(toolCall.Function.Name, tool)
|
||||
title := cmp.Or(tool.Annotations.Title, toolCall.Function.Name)
|
||||
|
||||
if tool.Annotations.ReadOnlyHint {
|
||||
kind = acp.ToolKindRead
|
||||
}
|
||||
|
||||
args := parseToolCallArguments(toolCall.Function.Arguments)
|
||||
return acp.ToolCallUpdate{
|
||||
ToolCallId: acp.ToolCallId(toolCall.ID),
|
||||
@@ -88,13 +85,6 @@ func buildToolCallUpdate(toolCall tools.ToolCall, tool tools.Tool, status acp.To
|
||||
|
||||
// determineToolKind maps tool names and annotations to ACP tool kinds.
|
||||
func determineToolKind(toolName string, tool tools.Tool) acp.ToolKind {
|
||||
if tool.Annotations.ReadOnlyHint {
|
||||
return acp.ToolKindRead
|
||||
}
|
||||
if tool.Annotations.DestructiveHint != nil && *tool.Annotations.DestructiveHint {
|
||||
return acp.ToolKindDelete
|
||||
}
|
||||
|
||||
switch {
|
||||
case strings.HasPrefix(toolName, "read_"),
|
||||
strings.HasPrefix(toolName, "get_"),
|
||||
@@ -110,8 +100,7 @@ func determineToolKind(toolName string, tool tools.Tool) acp.ToolKind {
|
||||
return acp.ToolKindEdit
|
||||
|
||||
case strings.HasPrefix(toolName, "delete_"),
|
||||
strings.HasPrefix(toolName, "remove_"),
|
||||
strings.HasPrefix(toolName, "stop_"):
|
||||
strings.HasPrefix(toolName, "remove_"):
|
||||
return acp.ToolKindDelete
|
||||
|
||||
case strings.HasPrefix(toolName, "search_"),
|
||||
@@ -126,6 +115,7 @@ func determineToolKind(toolName string, tool tools.Tool) acp.ToolKind {
|
||||
return acp.ToolKindFetch
|
||||
|
||||
case toolName == "shell",
|
||||
strings.HasPrefix(toolName, "stop_"),
|
||||
strings.HasPrefix(toolName, "run_"),
|
||||
strings.HasPrefix(toolName, "exec_"):
|
||||
return acp.ToolKindExecute
|
||||
@@ -135,6 +125,9 @@ func determineToolKind(toolName string, tool tools.Tool) acp.ToolKind {
|
||||
return acp.ToolKindSwitchMode
|
||||
|
||||
default:
|
||||
if tool.Annotations.ReadOnlyHint {
|
||||
return acp.ToolKindRead
|
||||
}
|
||||
return acp.ToolKindOther
|
||||
}
|
||||
}
|
||||
|
||||
@@ -22,9 +22,9 @@ func TestDetermineToolKind(t *testing.T) {
|
||||
annotations tools.ToolAnnotations
|
||||
want acpsdk.ToolKind
|
||||
}{
|
||||
{name: "read only hint wins", toolName: "delete_everything", annotations: tools.ToolAnnotations{ReadOnlyHint: true}, want: acpsdk.ToolKindRead},
|
||||
{name: "read only hint wins over destructive", toolName: "shell", annotations: tools.ToolAnnotations{ReadOnlyHint: true, DestructiveHint: new(true)}, want: acpsdk.ToolKindRead},
|
||||
{name: "destructive hint", toolName: "shell", annotations: tools.ToolAnnotations{DestructiveHint: new(true)}, want: acpsdk.ToolKindDelete},
|
||||
{name: "specific deletion wins over read hint", toolName: "delete_everything", annotations: tools.ToolAnnotations{ReadOnlyHint: true}, want: acpsdk.ToolKindDelete},
|
||||
{name: "shell remains execute despite hints", toolName: "shell", annotations: tools.ToolAnnotations{ReadOnlyHint: true, DestructiveHint: new(true)}, want: acpsdk.ToolKindExecute},
|
||||
{name: "destructive shell is not deletion", toolName: "shell", annotations: tools.ToolAnnotations{DestructiveHint: new(true)}, want: acpsdk.ToolKindExecute},
|
||||
{name: "destructive hint false falls through", toolName: "shell", annotations: tools.ToolAnnotations{DestructiveHint: new(false)}, want: acpsdk.ToolKindExecute},
|
||||
{name: "read_ prefix", toolName: "read_file", want: acpsdk.ToolKindRead},
|
||||
{name: "get_ prefix", toolName: "get_issue", want: acpsdk.ToolKindRead},
|
||||
@@ -37,7 +37,7 @@ func TestDetermineToolKind(t *testing.T) {
|
||||
{name: "add_ prefix", toolName: "add_comment", want: acpsdk.ToolKindEdit},
|
||||
{name: "delete_ prefix", toolName: "delete_file", want: acpsdk.ToolKindDelete},
|
||||
{name: "remove_ prefix", toolName: "remove_directory", want: acpsdk.ToolKindDelete},
|
||||
{name: "stop_ prefix", toolName: "stop_container", want: acpsdk.ToolKindDelete},
|
||||
{name: "stop_ prefix", toolName: "stop_container", want: acpsdk.ToolKindExecute},
|
||||
{name: "search_ prefix", toolName: "search_files_content", want: acpsdk.ToolKindSearch},
|
||||
{name: "find_ prefix", toolName: "find_symbol", want: acpsdk.ToolKindSearch},
|
||||
{name: "think", toolName: "think", want: acpsdk.ToolKindThink},
|
||||
@@ -49,6 +49,11 @@ func TestDetermineToolKind(t *testing.T) {
|
||||
{name: "transfer_task", toolName: "transfer_task", want: acpsdk.ToolKindSwitchMode},
|
||||
{name: "handoff", toolName: "handoff", want: acpsdk.ToolKindSwitchMode},
|
||||
{name: "unknown", toolName: "banana", want: acpsdk.ToolKindOther},
|
||||
{name: "read-only search", toolName: "search_files_content", annotations: tools.ToolAnnotations{ReadOnlyHint: true}, want: acpsdk.ToolKindSearch},
|
||||
{name: "destructive edit", toolName: "edit_file", annotations: tools.ToolAnnotations{DestructiveHint: new(true)}, want: acpsdk.ToolKindEdit},
|
||||
{name: "destructive write", toolName: "write_file", annotations: tools.ToolAnnotations{DestructiveHint: new(true)}, want: acpsdk.ToolKindEdit},
|
||||
{name: "unknown read-only", toolName: "banana", annotations: tools.ToolAnnotations{ReadOnlyHint: true}, want: acpsdk.ToolKindRead},
|
||||
{name: "unknown destructive", toolName: "banana", annotations: tools.ToolAnnotations{DestructiveHint: new(true)}, want: acpsdk.ToolKindOther},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
@@ -264,7 +269,7 @@ func TestBuildToolCallStart(t *testing.T) {
|
||||
assert.Equal(t, acpsdk.ToolCallId("call-1"), tc.ToolCallId)
|
||||
assert.Equal(t, "Read File", tc.Title)
|
||||
assert.Equal(t, acpsdk.ToolKindRead, tc.Kind)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusPending, tc.Status)
|
||||
assert.Equal(t, acpsdk.ToolCallStatusInProgress, tc.Status)
|
||||
assert.Equal(t, map[string]any{"path": "a.txt"}, tc.RawInput)
|
||||
assert.Equal(t, []acpsdk.ToolCallLocation{{Path: filepath.Join(wd, "a.txt")}}, tc.Locations)
|
||||
})
|
||||
@@ -308,7 +313,7 @@ func TestBuildToolCallUpdate(t *testing.T) {
|
||||
readOnly := tools.Tool{Name: "shell", Annotations: tools.ToolAnnotations{ReadOnlyHint: true}}
|
||||
update = buildToolCallUpdate(toolCall, readOnly, acpsdk.ToolCallStatusCompleted, "")
|
||||
require.NotNil(t, update.Kind)
|
||||
assert.Equal(t, acpsdk.ToolKindRead, *update.Kind)
|
||||
assert.Equal(t, acpsdk.ToolKindExecute, *update.Kind)
|
||||
}
|
||||
|
||||
func TestIsTodoTool(t *testing.T) {
|
||||
|
||||
Reference in new issue
Block a user