diff --git a/CHANGELOG.md b/CHANGELOG.md index 5402ad3..5e99a3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ All notable changes to GopherAgent are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/); versions follow [Semantic Versioning](https://semver.org/) — pre-1.0, breaking API changes only require a minor bump. +## [v0.44.0] — 2026-10-25 + +### Added + +- **`AgentLoop.DeferApprovals`, `AgentLoop.Resume`, and `AgentLoop.PendingApprovals` — human approvals now survive a restart.** `ConfirmHITL` blocks the run's goroutine until it answers, which is fine when the reviewer clicks a button seconds later and fails as soon as the answer can take longer than the process lives: a deploy or crash in between lost every open approval, and nothing could resume the turn once the answer arrived. With `DeferApprovals` set, a call that needs approval does not block. The loop emits `ActionRequiredEvent` with the call's ID, finishes the other calls in that wave, saves the turn with the gated call left without a result, and ends the run with `ErrApprovalPending`. Calls in later waves wait, because they may depend on the gated call through ``. Session history is the durable record, so every `SessionManager` backend supports it with no schema change. `Resume(ctx, sessionKey, decisions)` reads that turn back, from any process sharing the store: approved calls run, denied calls get the usual denial directive, calls without a decision stay pending (deferring again), and once every call has a result the run continues normally. The tool block is rebuilt in call order, so answers recorded before and after the suspension stay aligned with their calls. `PendingApprovals` lists what is still open, for asking again after a restart or for deciding whether an incoming reply belongs to `Resume` or `Run`. A decision for an ID that is not awaiting approval returns `ErrUnknownApproval` rather than being ignored, so passing `false` for the wrong ID can never let a call run that the caller believed was denied. `ConfirmFunc` is unchanged and blocking stays the default. `Resume` does not lock the session: route one session's answers through one goroutine, or an approved call can run twice. Sub-agent workers and the plan-mode gate still block. (`pkg/agent/resume.go`, `pkg/agent/hitl_gate.go`, `pkg/agent/loop_iteration.go`) +- **`ActionRequiredEvent.ID`** carries the tool call's ID, so hosts no longer mint their own correlation ID and smuggle it through `Args`. Additive on the wire (`id`, omitted when empty). (`pkg/agent/event_types.go`) + +### Fixed + +- **Identical gated tool calls in one wave are confirmed once and executed once.** A model that emitted the same `RequiresConfirmation` call twice in one reply got two separate prompts, so the user was asked the same question twice, and an approved side effect ran twice. Calls with the same name and canonical arguments are now gated and executed once, and each duplicate's tool call ID receives a copy of the result, which keeps the transcript paired. Ungated tools and calls whose arguments differ are unaffected. (`pkg/agent/loop_execute.go`) +- **Gemini: parallel calls of one tool no longer overwrite each other's results.** Most Gemini models return no call IDs, and the adapter used the tool name as the ID, so two calls of the same tool in one reply shared an ID. The loop keys results by ID, so the second result replaced the first and the model received one of them twice. The failure mode was worst for gated calls: a denial recorded for a duplicate replaced the approved result, and the model told the user an action had failed when it had run. The adapter now uses the API's ID when one is returned, keeps the bare name for a tool's first call (so stored sessions remain valid), and suffixes later ones (`name#2`). `FunctionResponse.Name` is resolved from the call that issued it. What goes over the wire is unchanged. (`pkg/llm/gemini/gemini.go`) + ## [v0.43.0] — 2026-08-17 ### Added @@ -664,6 +676,7 @@ Multi-user, long-running, audit-friendly chat surface — the foundation for sid - README section on the permission flow — documents `RequiresConfirmation` × `ConfirmHITL` × `Permissions` interaction. - Enum struct tag support in `tools.SchemaFor[T]()` — emit values into JSON-Schema's `enum` array so providers reject invalid values upstream. +[v0.44.0]: https://github.com/hung12ct/gopheragent/releases/tag/v0.44.0 [v0.43.0]: https://github.com/hung12ct/gopheragent/releases/tag/v0.43.0 [v0.42.0]: https://github.com/hung12ct/gopheragent/releases/tag/v0.42.0 [v0.41.0]: https://github.com/hung12ct/gopheragent/releases/tag/v0.41.0 diff --git a/README.md b/README.md index 4087df5..06d1bff 100644 --- a/README.md +++ b/README.md @@ -51,8 +51,9 @@ go get github.com/hung12ct/gopheragent load only when a skill is used. - **Deterministic ReAct loop** — dependency-aware parallel tool scheduling with `` refs, anti-loop detection, token-budget-aware pruning. -- **Streaming & HITL** — SSE streaming, human approvals, plan mode, and - self-critique that keeps the best-scoring pass instead of the last one. +- **Streaming & HITL** — SSE streaming, human approvals (durable across + restarts with `DeferApprovals` + `Resume`), plan mode, and self-critique + that keeps the best-scoring pass instead of the last one. - **Honest terminals** — a `context_trace` event says exactly which messages pruning rewrote and why; a `degraded` terminal reports "the artifact landed, the bookkeeping did not" instead of forcing a turn into success or failure. diff --git a/docs/permissions.md b/docs/permissions.md index 4f79f22..e2535a4 100644 --- a/docs/permissions.md +++ b/docs/permissions.md @@ -33,3 +33,37 @@ sqlTool := builtin.NewSQLAgentTool(db, "", sm, provider). See [`examples/hitl_server`](../examples/hitl_server) for approvals delivered over HTTP with an async bridge. + +## Durable approvals (answers that arrive hours later) + +`ConfirmHITL` blocks the run until it returns, so a restart loses every +open approval. Where users answer much later (chat apps, email), set +`DeferApprovals` instead: + +```go +loop.DeferApprovals = true + +for ev := range loop.RunText(ctx, sessionKey, text) { + if p, ok := ev.Payload.(agent.ActionRequiredEvent); ok { + askUser(sessionKey, p.ID, p.Tool, p.Args) // e.g. a chat message + } + // the run ends with errors.Is(err, agent.ErrApprovalPending) +} + +// Later, from any process that shares the SessionManager: +seq, err := loop.Resume(ctx, sessionKey, map[string]bool{approvalID: true}) +``` + +The gated call is saved in session history without a result, so every +`SessionManager` backend keeps it across restarts. Other calls in the same +wave still run; calls in later waves wait, since they may depend on it. +`Resume` runs approved calls, records denials, and continues the turn. Calls +left without a decision stay pending. `PendingApprovals(ctx, sessionKey)` +lists what is open, for asking again after a restart or for routing a reply +to `Resume` instead of `Run`. + +- A new `Run` on the session seals pending calls as cancelled, which is + how a user moving on reads to the model. +- `Resume` does not lock the session. Route each session's answers + through one goroutine, or an approved call can run twice. +- Sub-agent workers and the plan-mode gate still block on their callbacks. diff --git a/pkg/agent/errors.go b/pkg/agent/errors.go index 901c553..71c4a6e 100644 --- a/pkg/agent/errors.go +++ b/pkg/agent/errors.go @@ -135,6 +135,20 @@ var ( // the previous run ended naturally, and the caller should send a new // user message instead of trying to extend it. ErrNothingToContinue = errors.New("agent: session ended cleanly; nothing to continue") + + // ErrApprovalPending ends a run whose tool calls are waiting on a + // deferred approval (AgentLoop.DeferApprovals). It is not a failure: the + // turn is saved, and AgentLoop.Resume picks it up once answered. + ErrApprovalPending = errors.New("agent: tool calls awaiting approval") + + // ErrNothingToResume is returned by AgentLoop.Resume when the session's + // latest tool turn has no call left without a result, typically because + // a later Run already sealed it. + ErrNothingToResume = errors.New("agent: no pending tool calls to resume") + + // ErrUnknownApproval is returned by AgentLoop.Resume when a decision + // names a call ID that is not pending in the session. + ErrUnknownApproval = errors.New("agent: no pending tool call with this ID") ) // ToolNotFoundError carries the name of the missing tool. diff --git a/pkg/agent/event_types.go b/pkg/agent/event_types.go index 74fe73f..a0c6995 100644 --- a/pkg/agent/event_types.go +++ b/pkg/agent/event_types.go @@ -200,7 +200,11 @@ func (ToolProgressEvent) eventType() StreamEventType { return EventTypeToolProgr // Adopters typically gate this through an out-of-band channel (UI prompt, // Slack message, webhook) and resume the loop only after the operator // responds via the configured ConfirmHITL callback. +// +// ID is the tool call's ID. With AgentLoop.DeferApprovals it is the key to +// answer the approval with in AgentLoop.Resume. type ActionRequiredEvent struct { + ID string `json:"id,omitempty"` Tool string `json:"tool"` Args string `json:"args,omitempty"` } diff --git a/pkg/agent/hitl_gate.go b/pkg/agent/hitl_gate.go index 9cd7151..c58a052 100644 --- a/pkg/agent/hitl_gate.go +++ b/pkg/agent/hitl_gate.go @@ -18,8 +18,30 @@ const ( hitlDenied hitlTimedOut hitlUnconfigured + // hitlDeferred leaves the call pending for Resume; it gets no tool + // message this run. + hitlDeferred ) +// resolveApproval decides a gated call. A decision handed to Resume wins; +// otherwise DeferApprovals parks the call for a later Resume, and the +// default asks ConfirmHITL and blocks until it answers. +func (al *AgentLoop) resolveApproval(ctx context.Context, st *iterationState, ws *waveState, tc PendingToolCall) hitlOutcome { + if approved, ok := st.decisions[tc.ID]; ok { + if approved { + return hitlApproved + } + al.emit(ctx, st.sessionKey, st.streamChan, Event(HITLDeniedEvent{Tool: tc.Name, Args: tc.ArgsJSON})) + return hitlDenied + } + if al.DeferApprovals { + ws.markPending(tc) + al.emit(ctx, st.sessionKey, st.streamChan, Event(ActionRequiredEvent{ID: tc.ID, Tool: tc.Name, Args: tc.ArgsJSON})) + return hitlDeferred + } + return al.runHITLGate(ctx, st.sessionKey, st.streamChan, ws, tc) +} + // runHITLGate emits the human-confirmation prompt under hitlMu and returns // a hitlOutcome describing how the gate resolved. The caller selects the // tool-message directive from the outcome: @@ -44,7 +66,7 @@ func (al *AgentLoop) runHITLGate(ctx context.Context, sessionKey string, streamC if al.ConfirmHITL == nil { al.warnMissingConfirmHITLOnce() - al.emit(ctx, sessionKey, streamChan, Event(ActionRequiredEvent{Tool: tc.Name, Args: tc.ArgsJSON})) + al.emit(ctx, sessionKey, streamChan, Event(ActionRequiredEvent{ID: tc.ID, Tool: tc.Name, Args: tc.ArgsJSON})) return hitlUnconfigured } diff --git a/pkg/agent/loop_execute.go b/pkg/agent/loop_execute.go index 61f054f..746bbba 100644 --- a/pkg/agent/loop_execute.go +++ b/pkg/agent/loop_execute.go @@ -44,7 +44,12 @@ func (al *AgentLoop) toolCallSemaphore(waveSize int) chan struct{} { return make(chan struct{}, n) } -func (al *AgentLoop) executeToolWaves(ctx context.Context, st *iterationState, scheduled []PendingToolCall) *waveState { +// ws may arrive seeded with results from an earlier run (see Resume); calls +// that already have a result are skipped, and scheduling still sees the +// whole turn so references to those results keep their order. A wave that +// leaves any call awaiting a deferred approval ends execution: later waves +// may depend on it, and Resume runs them once the approval is answered. +func (al *AgentLoop) executeToolWaves(ctx context.Context, st *iterationState, ws *waveState, scheduled []PendingToolCall) { waves, schedErr := scheduleToolCalls(scheduled) if schedErr != nil { al.emit(ctx, st.sessionKey, st.streamChan, Event(ThoughtEvent{ @@ -53,17 +58,18 @@ func (al *AgentLoop) executeToolWaves(ctx context.Context, st *iterationState, s waves = [][]PendingToolCall{scheduled} } - ws := newWaveState(len(scheduled)) for _, wave := range waves { + wave = ws.unfinished(wave) substitutedWave := substituteWaveArgs(ws, wave, func(toolName string, subErr error) { al.emit(ctx, st.sessionKey, st.streamChan, Event(ThoughtEvent{ Message: fmt.Sprintf("Tool scheduler: substitution for %q failed: %v — skipping execution.", toolName, subErr), })) }) - sem := al.toolCallSemaphore(len(substitutedWave)) + dispatch, dups := al.splitGatedDuplicates(substitutedWave) + sem := al.toolCallSemaphore(len(dispatch)) var wg sync.WaitGroup - for _, tc := range substitutedWave { + for _, tc := range dispatch { wg.Add(1) go func(tCall PendingToolCall) { defer wg.Done() @@ -75,12 +81,79 @@ func (al *AgentLoop) executeToolWaves(ctx context.Context, st *iterationState, s }(tc) } wg.Wait() + ws.copyDuplicateResults(dups) - if ws.fatalErr != nil { + if ws.fatalErr != nil || len(ws.pending) > 0 { break } } - return ws +} + +// gatedDuplicate is a confirmation-gated call that repeats an earlier call +// in the same wave with identical arguments; it reuses the leader's result. +type gatedDuplicate struct { + call PendingToolCall + leaderID string +} + +// splitGatedDuplicates removes calls to RequiresConfirmation tools that +// repeat an earlier call in the wave with the same name and canonical args. +// Models occasionally emit the same gated call twice in one turn; gating +// each one asks the user the same question twice and, once approved, runs +// the side effect twice. Ungated tools are left alone so read-only fan-out +// keeps its existing behavior. +func (al *AgentLoop) splitGatedDuplicates(wave []PendingToolCall) ([]PendingToolCall, []gatedDuplicate) { + if len(wave) < 2 { + return wave, nil + } + var seen map[string]string + var dups []gatedDuplicate + var out []PendingToolCall + for i, tc := range wave { + tool, ok := al.Tools.Get(tc.Name) + if !ok || !tool.Descriptor().RequiresConfirmation { + if dups != nil { + out = append(out, tc) + } + continue + } + key := toolCacheKey(tc.Name, tc.ArgsJSON) + if leaderID, dup := seen[key]; dup { + if dups == nil { + out = append(make([]PendingToolCall, 0, len(wave)), wave[:i]...) + } + dups = append(dups, gatedDuplicate{call: tc, leaderID: leaderID}) + continue + } + if seen == nil { + seen = make(map[string]string, 2) + } + seen[key] = tc.ID + if dups != nil { + out = append(out, tc) + } + } + if dups == nil { + return wave, nil + } + return out, dups +} + +// copyDuplicateResults records each duplicate's reply as a copy of its +// leader's, so every ToolCallID still gets a matching tool message. A +// leader with no recorded result (wave aborted) is skipped and left to +// synthesizeMissingToolErrors. Runs after wg.Wait, so no goroutine writes +// concurrently. +func (ws *waveState) copyDuplicateResults(dups []gatedDuplicate) { + for _, d := range dups { + msg, ok := ws.toolMsgs[d.leaderID] + if !ok { + continue + } + msg.ToolCallID = d.call.ID + _, isResult := ws.resultsByID[d.leaderID] + ws.recordToolMsg(d.call.ID, msg, isResult) + } } // executeToolCall is the per-tool goroutine body. The caller spawns it @@ -144,7 +217,10 @@ func (al *AgentLoop) executeToolCall(ctx context.Context, st *iterationState, ws // RequiresConfirmation covers tools that always prompt. Allow bypasses // both. if (desc.RequiresConfirmation || permDecision == PermissionConfirm) && permDecision != PermissionAllow { - outcome := al.runHITLGate(ctx, st.sessionKey, st.streamChan, ws, tCall) + outcome := al.resolveApproval(ctx, st, ws, tCall) + if outcome == hitlDeferred { + return + } if outcome != hitlApproved { var msg string switch outcome { diff --git a/pkg/agent/loop_execute_dedupe_test.go b/pkg/agent/loop_execute_dedupe_test.go new file mode 100644 index 0000000..a47974d --- /dev/null +++ b/pkg/agent/loop_execute_dedupe_test.go @@ -0,0 +1,118 @@ +package agent + +import ( + "context" + "sync/atomic" + "testing" + + "github.com/hung12ct/gopheragent/pkg/history" + "github.com/hung12ct/gopheragent/pkg/tools" +) + +// sideEffectTool counts executions so tests can tell a reused result from a +// second run of the side effect. +type sideEffectTool struct { + name string + confirm bool + runs atomic.Int32 +} + +func (t *sideEffectTool) Descriptor() tools.ToolDescriptor { + return tools.ToolDescriptor{ + Name: t.name, + Description: "counts runs", + RequiresConfirmation: t.confirm, + Display: tools.DefaultDisplay(t.name, "counts runs"), + } +} + +func (t *sideEffectTool) Execute(_ context.Context, args string) (tools.Result, error) { + t.runs.Add(1) + return tools.Text("ran:" + args), nil +} + +func toolReplies(t *testing.T, sm *history.InMemSessionManager) []history.Message { + t.Helper() + msgs, err := sm.History(context.Background(), "s1") + if err != nil { + t.Fatalf("history: %v", err) + } + var out []history.Message + for _, m := range msgs { + if m.Role == "tool" { + out = append(out, m) + } + } + return out +} + +func TestToolWaves_GatedDuplicatesConfirmAndRunOnce(t *testing.T) { + cases := map[string][]PendingToolCall{ + "distinct IDs": { + {ID: "c1", Name: "wipe", ArgsJSON: `{"a":1,"b":2}`}, + {ID: "c2", Name: "wipe", ArgsJSON: `{"b":2, "a":1}`}, + }, + // Gemini reuses the tool name as the call ID. + "shared ID": { + {ID: "wipe", Name: "wipe", ArgsJSON: `{}`}, + {ID: "wipe", Name: "wipe", ArgsJSON: `{}`}, + }, + } + for name, calls := range cases { + t.Run(name, func(t *testing.T) { + tool := &sideEffectTool{name: "wipe", confirm: true} + provider := &scriptProvider{turns: []LLMResult{{ToolCalls: calls}, {Content: "done"}}} + loop, sm := setup(provider, tool) + var asks atomic.Int32 + loop.ConfirmHITL = func(context.Context, string, string) bool { + return asks.Add(1) == 1 // a second prompt would be denied + } + + if _, err := loop.RunIteration(context.Background(), "s1", "wipe it"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if n := asks.Load(); n != 1 { + t.Fatalf("ConfirmHITL called %d times, want 1", n) + } + if n := tool.runs.Load(); n != 1 { + t.Fatalf("tool ran %d times, want 1", n) + } + replies := toolReplies(t, sm) + if len(replies) != len(calls) { + t.Fatalf("got %d tool replies, want %d", len(replies), len(calls)) + } + for i, m := range replies { + if m.IsError || m.ToolCallID != calls[i].ID { + t.Fatalf("reply %d = %+v, want approved result for %q", i, m, calls[i].ID) + } + } + }) + } +} + +func TestToolWaves_DuplicatesNotMergedWhenUngatedOrArgsDiffer(t *testing.T) { + gated := &sideEffectTool{name: "wipe", confirm: true} + plain := &sideEffectTool{name: "read"} + provider := &scriptProvider{turns: []LLMResult{{ToolCalls: []PendingToolCall{ + {ID: "c1", Name: "wipe", ArgsJSON: `{"k":1}`}, + {ID: "c2", Name: "wipe", ArgsJSON: `{"k":2}`}, + {ID: "c3", Name: "read", ArgsJSON: `{}`}, + {ID: "c4", Name: "read", ArgsJSON: `{}`}, + }}, {Content: "done"}}} + loop, _ := setup(provider, gated, plain) + var asks atomic.Int32 + loop.ConfirmHITL = func(context.Context, string, string) bool { + asks.Add(1) + return true + } + + if _, err := loop.RunIteration(context.Background(), "s1", "go"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if asks.Load() != 2 || gated.runs.Load() != 2 { + t.Fatalf("gated: asks=%d runs=%d, want 2 and 2", asks.Load(), gated.runs.Load()) + } + if n := plain.runs.Load(); n != 2 { + t.Fatalf("ungated tool ran %d times, want 2", n) + } +} diff --git a/pkg/agent/loop_iteration.go b/pkg/agent/loop_iteration.go index 7d9b8e8..db359c0 100644 --- a/pkg/agent/loop_iteration.go +++ b/pkg/agent/loop_iteration.go @@ -95,25 +95,43 @@ func (al *AgentLoop) runIteration(ctx context.Context, sessionKey string, stream al.emit(ctx, sessionKey, streamChan, errEvent(fmt.Errorf("agent: save history: %w", err))) } - ws := al.executeToolWaves(ctx, st, scheduled) + ws := newWaveState(len(scheduled)) + al.executeToolWaves(ctx, st, ws, scheduled) + done, iterErr := al.recordToolTurn(ctx, st, msgs, result.ToolCalls, droppedCalls, ws) + return len(scheduled), done +} + +// recordToolTurn appends a finished tool wave's results to *msgs in call +// order, persists them, and reports whether the run must end. It ends on a +// fatal wave error (every unanswered call is sealed so the transcript stays +// valid) and on deferred approvals, whose calls are left without a result +// for Resume to answer. The returned error is for the iteration span; the +// terminal event has already been emitted. +func (al *AgentLoop) recordToolTurn(ctx context.Context, st *iterationState, msgs *[]history.Message, calls, dropped []PendingToolCall, ws *waveState) (bool, error) { if ws.fatalErr != nil { - iterErr = ws.fatalErr // The assistant message persisted above carries tool_calls, so the // transcript is only valid once every call has a tool reply. Keep the // results that completed before the abort (real work, and the looping // call's own result is what the detector tripped on) and account for // the rest, so the saved history needs no downstream repair. - synthesizeMissingToolErrors(ws.toolMsgs, result.ToolCalls, fatalAbortToolReason) - *msgs = appendToolResultsInOrder(*msgs, result.ToolCalls, ws.toolMsgs) - al.saveSession(ctx, sessionKey, *msgs) - al.emit(ctx, sessionKey, streamChan, errEvent(fmt.Errorf("%w: %w", ErrLoopDetected, ws.fatalErr))) - return len(scheduled), true + synthesizeMissingToolErrors(ws.toolMsgs, calls, fatalAbortToolReason) + *msgs = appendToolResultsInOrder(*msgs, calls, ws.toolMsgs) + al.saveSession(ctx, st.sessionKey, *msgs) + al.emit(ctx, st.sessionKey, st.streamChan, errEvent(fmt.Errorf("%w: %w", ErrLoopDetected, ws.fatalErr))) + return true, ws.fatalErr } - synthesizeDroppedToolErrors(ws.toolMsgs, droppedCalls, al.MaxToolCallsPerTurn) - *msgs = appendToolResultsInOrder(*msgs, result.ToolCalls, ws.toolMsgs) - if err := al.Sessions.SaveHistory(ctx, sessionKey, *msgs); err != nil { - al.emit(ctx, sessionKey, streamChan, errEvent(fmt.Errorf("agent: save history: %w", err))) + synthesizeDroppedToolErrors(ws.toolMsgs, dropped, al.MaxToolCallsPerTurn) + *msgs = appendToolResultsInOrder(*msgs, calls, ws.toolMsgs) + if len(ws.pending) > 0 { + // Persist even when ctx was cancelled: the saved turn is what Resume + // reads, possibly from another process. + al.saveSession(ctx, st.sessionKey, *msgs) + al.emit(ctx, st.sessionKey, st.streamChan, errEvent(fmt.Errorf("%w: %d call(s)", ErrApprovalPending, len(ws.pending)))) + return true, nil + } + if err := al.Sessions.SaveHistory(ctx, st.sessionKey, *msgs); err != nil { + al.emit(ctx, st.sessionKey, st.streamChan, errEvent(fmt.Errorf("agent: save history: %w", err))) } - return len(scheduled), false + return false, nil } diff --git a/pkg/agent/loop_state.go b/pkg/agent/loop_state.go index d53a66d..5ee5402 100644 --- a/pkg/agent/loop_state.go +++ b/pkg/agent/loop_state.go @@ -25,6 +25,10 @@ type iterationState struct { // against. Only populated when RequestInvariant is set. Written and // read on the loop goroutine, between which no tool goroutine runs. sentMessages []history.Message + + // decisions holds approvals answered through Resume, keyed by tool + // call ID. Read-only once the waves start; nil outside Resume. + decisions map[string]bool } // waveState carries the per-wave shared mutable state used by the @@ -37,6 +41,10 @@ type waveState struct { fatalErr error fatalMu sync.Mutex hitlMu sync.Mutex + + // pending holds calls whose approval was deferred (DeferApprovals); they + // get no tool message this run. Guarded by completedMu. + pending []PendingToolCall } // newWaveState returns an initialised waveState with maps sized for the @@ -62,6 +70,38 @@ func (ws *waveState) recordToolMsg(id string, msg history.Message, isResult bool ws.completedMu.Unlock() } +// markPending records a call whose approval was deferred to Resume. +func (ws *waveState) markPending(tc PendingToolCall) { + ws.completedMu.Lock() + ws.pending = append(ws.pending, tc) + ws.completedMu.Unlock() +} + +// unfinished drops calls that already have a result, which only happens +// when ws was seeded from history by Resume. Returns wave itself when +// nothing is dropped. Called between waves, when no goroutine writes ws. +func (ws *waveState) unfinished(wave []PendingToolCall) []PendingToolCall { + if len(ws.toolMsgs) == 0 { + return wave + } + var out []PendingToolCall + for i, tc := range wave { + if _, done := ws.toolMsgs[tc.ID]; !done { + if out != nil { + out = append(out, tc) + } + continue + } + if out == nil { + out = append(make([]PendingToolCall, 0, len(wave)), wave[:i]...) + } + } + if out == nil { + return wave + } + return out +} + // setFatal records the first fatal error seen by any wave goroutine. // First-writer-wins: subsequent calls are no-ops. func (ws *waveState) setFatal(err error) { diff --git a/pkg/agent/loop_stream.go b/pkg/agent/loop_stream.go index 4a6be71..82dba7b 100644 --- a/pkg/agent/loop_stream.go +++ b/pkg/agent/loop_stream.go @@ -265,6 +265,16 @@ type AgentLoop struct { // denial) so it can ask the user to retry rather than seek an alternative. ConfirmHITLTimeout time.Duration + // DeferApprovals makes the HITL gate durable. A call that needs approval + // no longer blocks on ConfirmHITL: the loop emits ActionRequiredEvent + // with the call's ID, finishes the other calls in that wave, saves the + // turn with the call left without a result, and ends the run with + // ErrApprovalPending. Answer later, from any process sharing the + // SessionManager, with Resume. Use it where the answer can take longer + // than a process lives (chat apps, email approvals). Sub-agent workers + // and the plan-mode gate still block. + DeferApprovals bool + // Cache provides optional tool result caching with trigram similarity matching. // If nil, caching is disabled. Set via cache.NewSearchCache(). Cache *cache.SearchCache diff --git a/pkg/agent/resume.go b/pkg/agent/resume.go new file mode 100644 index 0000000..67f51dd --- /dev/null +++ b/pkg/agent/resume.go @@ -0,0 +1,210 @@ +package agent + +import ( + "context" + "fmt" + "iter" + "sync" + + "github.com/hung12ct/gopheragent/pkg/history" +) + +// Resume answers approvals that a DeferApprovals run left pending and +// continues that run. decisions maps a pending call's ID (the +// ActionRequiredEvent.ID, or PendingToolCall.ID from PendingApprovals) to +// approve (true) or deny (false). +// +// The state lives in session history, so Resume works from any process that +// shares the SessionManager, including after a restart. It finishes the +// session's latest tool turn: approved calls execute, denied ones get the +// usual denial result, calls that never ran because a later wave waited on +// an approval execute now, and gated calls without a decision ask again +// (deferred again under DeferApprovals). Once every call has a result the +// loop continues as a normal run; EventTypeContinued is the first frame. +// +// Returns ErrNothingToResume when no call is pending, and ErrUnknownApproval +// when decisions names an ID that PendingApprovals would not list, so a +// decision can never be silently ignored. Resume does not serialize +// callers: two concurrent Resumes for one session would both run an +// approved call, so route a session's answers through one goroutine. +func (al *AgentLoop) Resume(ctx context.Context, sessionKey string, decisions map[string]bool) (iter.Seq[StreamEvent], error) { + existing, err := al.Sessions.History(ctx, sessionKey) + if err != nil { + return nil, fmt.Errorf("agent: resume: load history: %w", err) + } + idx, pending := pendingToolTurn(existing) + if len(pending) == 0 { + return nil, ErrNothingToResume + } + open := al.approvalsIn(WithSessionKey(ctx, sessionKey), pending) + for id := range decisions { + if !containsCallID(open, id) { + return nil, fmt.Errorf("%w: %q", ErrUnknownApproval, id) + } + } + + firstFrame := continuedEvent(idx) + emitThoughts := al.EmitThoughts + return func(yield func(StreamEvent) bool) { + runCtx, cancel := context.WithCancel(ctx) + defer cancel() + if !yield(firstFrame) { + return + } + internalChan := make(chan StreamEvent, runIterationStreamBuffer) + go al.resumeLogicLoop(runCtx, sessionKey, decisions, internalChan) + yieldEvents(internalChan, yield, emitThoughts, cancel) + }, nil +} + +// PendingApprovals lists the calls in sessionKey's latest tool turn that +// still need a human decision, one entry per distinct call (identical +// duplicates share the leader's decision). Use it after a restart to ask +// the user again, or to route an incoming reply to Resume instead of Run. +func (al *AgentLoop) PendingApprovals(ctx context.Context, sessionKey string) ([]PendingToolCall, error) { + existing, err := al.Sessions.History(ctx, sessionKey) + if err != nil { + return nil, fmt.Errorf("agent: pending approvals: load history: %w", err) + } + _, pending := pendingToolTurn(existing) + return al.approvalsIn(WithSessionKey(ctx, sessionKey), pending), nil +} + +// approvalsIn filters pending down to the calls that wait on a decision: +// duplicates folded into their leader, and calls the gate would not stop +// (ungated calls from a later wave) left out. +func (al *AgentLoop) approvalsIn(ctx context.Context, pending []PendingToolCall) []PendingToolCall { + leaders, _ := al.splitGatedDuplicates(pending) + var out []PendingToolCall + for _, tc := range leaders { + if al.needsApproval(ctx, tc) { + out = append(out, tc) + } + } + return out +} + +// resumeLogicLoop finishes the pending tool turn with decisions applied, +// then hands over to iterateMessages. Mirrors continueLogicLoop's setup. +func (al *AgentLoop) resumeLogicLoop(ctx context.Context, sessionKey string, decisions map[string]bool, streamChan chan<- StreamEvent) { + defer close(streamChan) + ctx = WithSessionKey(ctx, sessionKey) + + var emitCost func() + ctx, emitCost = al.installRunCostAccumulator(ctx, sessionKey, streamChan) + defer emitCost() + var sweepDegraded func() + ctx, sweepDegraded = al.installDegradationAccumulator(ctx, sessionKey, streamChan) + defer sweepDegraded() + + msgs, err := al.Sessions.History(ctx, sessionKey) + if err != nil { + al.emit(ctx, sessionKey, streamChan, errEvent(fmt.Errorf("agent: resume: load history: %w", err))) + return + } + // History may have moved between Resume and this goroutine. + idx, pending := pendingToolTurn(msgs) + if len(pending) == 0 { + al.emit(ctx, sessionKey, streamChan, errEvent(ErrNothingToResume)) + return + } + + st := &iterationState{ + sessionKey: sessionKey, + streamChan: streamChan, + specMap: newSpeculativeMap(), + specMu: &sync.Mutex{}, + tracker: loopDetectorFromHistory(msgs), + decisions: decisions, + } + calls := pendingCallsOf(msgs[idx]) + ws := seededWaveState(msgs[idx+1:], len(calls)) + al.executeToolWaves(ctx, st, ws, calls) + + // Rebuild the tool block in call order: answers recorded before the + // suspension and those produced now interleave by call position. + turn := append(make([]history.Message, 0, idx+1+len(calls)), msgs[:idx+1]...) + if done, _ := al.recordToolTurn(ctx, st, &turn, calls, nil, ws); done { + return + } + al.iterateMessages(ctx, sessionKey, streamChan, turn) + al.fireConsolidator(ctx, sessionKey) +} + +// pendingToolTurn locates the latest tool turn: the last message must be an +// assistant message with tool calls, or a tool result inside the block that +// follows one. It returns that assistant message's index and its calls that +// have no result yet; idx is -1 and pending nil when there is no such turn. +func pendingToolTurn(msgs []history.Message) (int, []PendingToolCall) { + i := len(msgs) - 1 + for i >= 0 && msgs[i].Role == "tool" { + i-- + } + if i < 0 || msgs[i].Role != "assistant" || len(msgs[i].ToolCalls) == 0 { + return -1, nil + } + answered := make(map[string]struct{}, len(msgs)-1-i) + for _, m := range msgs[i+1:] { + answered[m.ToolCallID] = struct{}{} + } + var pending []PendingToolCall + for _, tc := range pendingCallsOf(msgs[i]) { + if _, ok := answered[tc.ID]; !ok { + pending = append(pending, tc) + } + } + if len(pending) == 0 { + return -1, nil + } + return i, pending +} + +// pendingCallsOf converts an assistant message's tool calls back into the +// loop's dispatch shape. +func pendingCallsOf(m history.Message) []PendingToolCall { + calls := make([]PendingToolCall, len(m.ToolCalls)) + for i, tc := range m.ToolCalls { + calls[i] = PendingToolCall{ID: tc.ID, Name: tc.Name, ArgsJSON: tc.Arguments} + } + return calls +} + +// seededWaveState returns a waveState holding the tool results already in +// history, so executeToolWaves skips those calls and +// references to them still resolve. +func seededWaveState(results []history.Message, capacity int) *waveState { + ws := newWaveState(capacity) + for _, m := range results { + ws.toolMsgs[m.ToolCallID] = m + if !m.IsError { + ws.resultsByID[m.ToolCallID] = m.Content + } + } + return ws +} + +// needsApproval reports whether tc would stop at the HITL gate, applying +// the same permission rules as executeToolCall. +func (al *AgentLoop) needsApproval(ctx context.Context, tc PendingToolCall) bool { + tool, ok := al.Tools.Get(tc.Name) + if !ok { + return false + } + decision := PermissionPrompt + if al.Permissions != nil { + decision = al.Permissions.Check(ctx, tc.Name, tc.ArgsJSON) + } + if decision == PermissionAllow || decision == PermissionDeny { + return false + } + return tool.Descriptor().RequiresConfirmation || decision == PermissionConfirm +} + +func containsCallID(calls []PendingToolCall, id string) bool { + for _, tc := range calls { + if tc.ID == id { + return true + } + } + return false +} diff --git a/pkg/agent/resume_test.go b/pkg/agent/resume_test.go new file mode 100644 index 0000000..a89db01 --- /dev/null +++ b/pkg/agent/resume_test.go @@ -0,0 +1,196 @@ +package agent + +import ( + "context" + "errors" + "iter" + "strings" + "testing" + + "github.com/hung12ct/gopheragent/pkg/history" + "github.com/hung12ct/gopheragent/pkg/tools" +) + +// deferredLoop builds a DeferApprovals loop over sm whose ConfirmHITL fails +// the test: a deferred gate must never block on it. +func deferredLoop(t *testing.T, sm *history.InMemSessionManager, provider LLMProvider, toolList ...tools.Tool) *AgentLoop { + t.Helper() + reg := tools.NewRegistry() + for _, tl := range toolList { + reg.Register(tl) + } + loop := NewAgentLoop(sm, reg, provider) + loop.DeferApprovals = true + loop.ConfirmHITL = func(context.Context, string, string) bool { + t.Error("ConfirmHITL called under DeferApprovals") + return false + } + return loop +} + +// drain collects a run's ActionRequiredEvent IDs and its terminal error. +func drain(seq iter.Seq[StreamEvent]) (ids []string, lastErr error) { + for ev := range seq { + switch p := ev.Payload.(type) { + case ActionRequiredEvent: + ids = append(ids, p.ID) + case ErrorEvent: + lastErr = p.Err + } + } + return ids, lastErr +} + +func toolTail(t *testing.T, sm *history.InMemSessionManager) []history.Message { + t.Helper() + msgs, err := sm.History(context.Background(), "s1") + if err != nil { + t.Fatalf("history: %v", err) + } + return msgs +} + +func TestDeferApprovals_SuspendThenResumeAfterRestart(t *testing.T) { + sm := history.NewInMemSessionManager("sys") + wipe := &sideEffectTool{name: "wipe", confirm: true} + read := &sideEffectTool{name: "read"} + first := deferredLoop(t, sm, &scriptProvider{turns: []LLMResult{{ToolCalls: []PendingToolCall{ + {ID: "c1", Name: "wipe", ArgsJSON: `{}`}, + {ID: "c2", Name: "read", ArgsJSON: `{}`}, + }}}}, wipe, read) + + ids, err := drain(first.RunText(context.Background(), "s1", "wipe it")) + if !errors.Is(err, ErrApprovalPending) { + t.Fatalf("terminal error = %v, want ErrApprovalPending", err) + } + if len(ids) != 1 || ids[0] != "c1" { + t.Fatalf("ActionRequired IDs = %v, want [c1]", ids) + } + if wipe.runs.Load() != 0 || read.runs.Load() != 1 { + t.Fatalf("runs wipe=%d read=%d, want 0 and 1", wipe.runs.Load(), read.runs.Load()) + } + + // A fresh loop over the same store stands in for a restarted process. + second := deferredLoop(t, sm, &scriptProvider{turns: []LLMResult{{Content: "wiped"}}}, wipe, read) + pending, err := second.PendingApprovals(context.Background(), "s1") + if err != nil || len(pending) != 1 || pending[0].ID != "c1" { + t.Fatalf("PendingApprovals = %v, %v; want [c1]", pending, err) + } + seq, err := second.Resume(context.Background(), "s1", map[string]bool{"c1": true}) + if err != nil { + t.Fatalf("Resume: %v", err) + } + if _, err := drain(seq); err != nil { + t.Fatalf("resumed run error: %v", err) + } + if wipe.runs.Load() != 1 || read.runs.Load() != 1 { + t.Fatalf("runs wipe=%d read=%d, want 1 and 1", wipe.runs.Load(), read.runs.Load()) + } + + msgs := toolTail(t, sm) + n := len(msgs) + if msgs[n-3].ToolCallID != "c1" || msgs[n-2].ToolCallID != "c2" || msgs[n-1].Content != "wiped" { + t.Fatalf("tail = %+v, want c1 result, c2 result, final answer", msgs[n-3:]) + } + if _, err := second.Resume(context.Background(), "s1", nil); !errors.Is(err, ErrNothingToResume) { + t.Fatalf("second Resume error = %v, want ErrNothingToResume", err) + } +} + +func TestDeferApprovals_ResumeDenyAndUnknownID(t *testing.T) { + sm := history.NewInMemSessionManager("sys") + wipe := &sideEffectTool{name: "wipe", confirm: true} + loop := deferredLoop(t, sm, &scriptProvider{turns: []LLMResult{ + {ToolCalls: []PendingToolCall{{ID: "c1", Name: "wipe", ArgsJSON: `{}`}}}, + {Content: "ok, not wiping"}, + }}, wipe) + if _, err := drain(loop.RunText(context.Background(), "s1", "wipe it")); !errors.Is(err, ErrApprovalPending) { + t.Fatalf("terminal error = %v, want ErrApprovalPending", err) + } + + if _, err := loop.Resume(context.Background(), "s1", map[string]bool{"nope": true}); !errors.Is(err, ErrUnknownApproval) { + t.Fatalf("unknown ID error = %v, want ErrUnknownApproval", err) + } + seq, err := loop.Resume(context.Background(), "s1", map[string]bool{"c1": false}) + if err != nil { + t.Fatalf("Resume: %v", err) + } + if _, err := drain(seq); err != nil { + t.Fatalf("resumed run error: %v", err) + } + if wipe.runs.Load() != 0 { + t.Fatalf("denied tool ran %d times", wipe.runs.Load()) + } + msgs := toolTail(t, sm) + reply := msgs[len(msgs)-2] + if reply.ToolCallID != "c1" || !reply.IsError || !strings.Contains(reply.Content, "permission gate") { + t.Fatalf("denial reply = %+v", reply) + } +} + +func TestDeferApprovals_DependentWaveWaitsForResume(t *testing.T) { + sm := history.NewInMemSessionManager("sys") + wipe := &sideEffectTool{name: "wipe", confirm: true} + read := &sideEffectTool{name: "read"} + loop := deferredLoop(t, sm, &scriptProvider{turns: []LLMResult{ + {ToolCalls: []PendingToolCall{ + {ID: "c1", Name: "wipe", ArgsJSON: `{}`}, + {ID: "c2", Name: "read", ArgsJSON: `{"after":""}`}, + }}, + {Content: "done"}, + }}, wipe, read) + if _, err := drain(loop.RunText(context.Background(), "s1", "go")); !errors.Is(err, ErrApprovalPending) { + t.Fatalf("terminal error = %v, want ErrApprovalPending", err) + } + if read.runs.Load() != 0 { + t.Fatalf("dependent call ran before its input was approved") + } + // c2 is pending but ungated: a decision for it would be ignored, so it + // must be rejected rather than let the call run "denied". + if _, err := loop.Resume(context.Background(), "s1", map[string]bool{"c2": false}); !errors.Is(err, ErrUnknownApproval) { + t.Fatalf("decision for ungated call: err = %v, want ErrUnknownApproval", err) + } + + seq, err := loop.Resume(context.Background(), "s1", map[string]bool{"c1": true}) + if err != nil { + t.Fatalf("Resume: %v", err) + } + if _, err := drain(seq); err != nil { + t.Fatalf("resumed run error: %v", err) + } + msgs := toolTail(t, sm) + c2 := msgs[len(msgs)-2] + if c2.ToolCallID != "c2" || !strings.Contains(c2.Content, `ran:{}`) { + t.Fatalf("c2 reply = %+v, want args substituted from c1", c2) + } +} + +func TestDeferApprovals_PartialDecisionsSuspendAgain(t *testing.T) { + sm := history.NewInMemSessionManager("sys") + wipe := &sideEffectTool{name: "wipe", confirm: true} + loop := deferredLoop(t, sm, &scriptProvider{turns: []LLMResult{ + {ToolCalls: []PendingToolCall{ + {ID: "c1", Name: "wipe", ArgsJSON: `{"k":1}`}, + {ID: "c2", Name: "wipe", ArgsJSON: `{"k":2}`}, + }}, + }}, wipe) + if ids, err := drain(loop.RunText(context.Background(), "s1", "go")); !errors.Is(err, ErrApprovalPending) || len(ids) != 2 { + t.Fatalf("first run: ids=%v err=%v", ids, err) + } + + seq, err := loop.Resume(context.Background(), "s1", map[string]bool{"c1": true}) + if err != nil { + t.Fatalf("Resume: %v", err) + } + ids, err := drain(seq) + if !errors.Is(err, ErrApprovalPending) || len(ids) != 1 || ids[0] != "c2" { + t.Fatalf("partial resume: ids=%v err=%v, want [c2] and ErrApprovalPending", ids, err) + } + if wipe.runs.Load() != 1 { + t.Fatalf("wipe ran %d times, want 1", wipe.runs.Load()) + } + pending, _ := loop.PendingApprovals(context.Background(), "s1") + if len(pending) != 1 || pending[0].ID != "c2" { + t.Fatalf("PendingApprovals = %v, want [c2]", pending) + } +} diff --git a/pkg/llm/gemini/call_id_test.go b/pkg/llm/gemini/call_id_test.go new file mode 100644 index 0000000..ee8fc51 --- /dev/null +++ b/pkg/llm/gemini/call_id_test.go @@ -0,0 +1,98 @@ +package gemini + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/hung12ct/gopheragent/pkg/agent" + "github.com/hung12ct/gopheragent/pkg/history" + "google.golang.org/genai" +) + +func TestGenerateStream_SameToolCallsGetUniqueIDs(t *testing.T) { + p := streamProvider(t, + `{"candidates":[{"content":{"role":"model","parts":[`+ + `{"functionCall":{"name":"search","args":{"q":"a"}}},`+ + `{"functionCall":{"name":"search","args":{"q":"b"}}},`+ + `{"functionCall":{"name":"fetch","args":{}}},`+ + `{"functionCall":{"id":"srv-1","name":"search","args":{"q":"c"}}}]}}]}`, + `{"candidates":[{"finishReason":"STOP"}]}`, + ) + res, _, err := runStream(t, p) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := []string{"search", "search#2", "fetch", "srv-1"} + if len(res.ToolCalls) != len(want) { + t.Fatalf("got %d tool calls, want %d", len(res.ToolCalls), len(want)) + } + for i, tc := range res.ToolCalls { + if tc.ID != want[i] { + t.Fatalf("call %d ID = %q, want %q", i, tc.ID, want[i]) + } + } +} + +func TestGenerateStream_FunctionResponseNamedByCall(t *testing.T) { + var body []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ = io.ReadAll(r.Body) + w.Header().Set("Content-Type", "text/event-stream") + fmt.Fprint(w, "data: {\"candidates\":[{\"content\":{\"role\":\"model\",\"parts\":[{\"text\":\"ok\"}]},\"finishReason\":\"STOP\"}]}\r\n\r\n") + })) + t.Cleanup(srv.Close) + client, err := genai.NewClient(context.Background(), &genai.ClientConfig{ + APIKey: "test-key", + Backend: genai.BackendGeminiAPI, + HTTPOptions: genai.HTTPOptions{BaseURL: srv.URL}, + }) + if err != nil { + t.Fatalf("new client: %v", err) + } + p := &Provider{client: client, model: "gemini-test"} + + memory := []history.Message{ + {Role: "user", Content: "hi"}, + {Role: "assistant", ToolCalls: []history.ToolCall{ + {ID: "search", Name: "search", Arguments: `{"q":"a"}`}, + {ID: "search#2", Name: "search", Arguments: `{"q":"b"}`}, + }}, + {Role: "tool", ToolCallID: "search", Content: "A"}, + {Role: "tool", ToolCallID: "search#2", Content: "B"}, + {Role: "tool", ToolCallID: "legacy", Content: "C"}, // no recorded call + } + ch := make(chan agent.StreamEvent, 16) + if _, err := p.GenerateStream(context.Background(), memory, nil, ch); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var req struct { + Contents []struct { + Parts []struct { + FunctionResponse *struct { + Name string `json:"name"` + } `json:"functionResponse"` + } `json:"parts"` + } `json:"contents"` + } + if err := json.Unmarshal(body, &req); err != nil { + t.Fatalf("decode request: %v", err) + } + var names []string + for _, c := range req.Contents { + for _, part := range c.Parts { + if part.FunctionResponse != nil { + names = append(names, part.FunctionResponse.Name) + } + } + } + want := []string{"search", "search", "legacy"} + if fmt.Sprint(names) != fmt.Sprint(want) { + t.Fatalf("FunctionResponse names = %v, want %v", names, want) + } +} diff --git a/pkg/llm/gemini/gemini.go b/pkg/llm/gemini/gemini.go index 45335f8..0b24a08 100644 --- a/pkg/llm/gemini/gemini.go +++ b/pkg/llm/gemini/gemini.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "os" + "strconv" "github.com/hung12ct/gopheragent/pkg/agent" "github.com/hung12ct/gopheragent/pkg/history" @@ -88,6 +89,7 @@ func New(apiKey string, model string, opts ...Option) (*Provider, error) { func (p *Provider) GenerateStream(ctx context.Context, memory []history.Message, availableTools *tools.Registry, streamChan chan<- agent.StreamEvent) (agent.LLMResult, error) { var contents []*genai.Content var systemInstruction *genai.Content + var callNames map[string]string // call ID -> tool name, for FunctionResponse.Name for _, m := range memory { // Only user messages render media. Parts on any other role are @@ -126,6 +128,10 @@ func (p *Provider) GenerateStream(ctx context.Context, memory []history.Message, parts = append(parts, &genai.Part{Text: m.Content}) } for _, tc := range m.ToolCalls { + if callNames == nil { + callNames = make(map[string]string, len(m.ToolCalls)) + } + callNames[tc.ID] = tc.Name var argsMap map[string]any if tc.Arguments != "" { _ = json.Unmarshal([]byte(tc.Arguments), &argsMap) @@ -146,7 +152,7 @@ func (p *Provider) GenerateStream(ctx context.Context, memory []history.Message, } parts = append(parts, &genai.Part{ FunctionResponse: &genai.FunctionResponse{ - Name: m.ToolCallID, // For gemini we need to match the func name here. We rely on ToolCallID housing the function name in this simple mapping or use a default. + Name: functionResponseName(callNames, m.ToolCallID), Response: resultObj, }, }) @@ -230,7 +236,7 @@ func (p *Provider) GenerateStream(ctx context.Context, memory []history.Message, if part.FunctionCall != nil { argsBytes, _ := json.Marshal(part.FunctionCall.Args) pendingCalls = append(pendingCalls, agent.PendingToolCall{ - ID: part.FunctionCall.Name, // Gemini doesn't use explicit call IDs, using name as ID is a common fallback + ID: pendingCallID(part.FunctionCall, pendingCalls), Name: part.FunctionCall.Name, ArgsJSON: string(argsBytes), }) @@ -258,6 +264,39 @@ func (p *Provider) GenerateStream(ctx context.Context, memory []history.Message, } // applySampling stamps the configured temperature/top_p/seed onto config. +// pendingCallID returns a call ID unique within one response. Most Gemini +// models return no ID, and the loop keys tool results by ID, so parallel +// calls of one tool sharing the bare name would overwrite each other's +// results. The first call of a tool keeps the bare name, which is what +// sessions recorded before this scheme carry. +func pendingCallID(fc *genai.FunctionCall, prior []agent.PendingToolCall) string { + if fc.ID != "" { + return fc.ID + } + n := 0 + for _, c := range prior { + if c.Name == fc.Name { + n++ + } + } + if n == 0 { + return fc.Name + } + return fc.Name + "#" + strconv.Itoa(n+1) +} + +// functionResponseName returns the tool name a reply answers. Gemini pairs +// a FunctionResponse with its call by name, while tool messages carry only +// the call ID, so the name comes from the assistant turn that issued the +// call. An ID with no recorded call is returned as is: older sessions used +// the tool name as the ID. +func functionResponseName(callNames map[string]string, callID string) string { + if name, ok := callNames[callID]; ok { + return name + } + return callID +} + func (p *Provider) applySampling(config *genai.GenerateContentConfig) { if p.temperature != nil { config.Temperature = genai.Ptr(float32(*p.temperature))