diff --git a/CHANGELOG.md b/CHANGELOG.md index cfa4d336..83eff6a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ The format is based on Keep a Changelog and the project follows Semantic Version ### Fixed +- **에이전트 턴의 마지막 호출이 캐시 계보를 벗어나던 문제 (#921)** — 루프가 반복을 소진한 뒤 답변을 받아내는 finalization 호출은 도구 목록을 통째로 뺀 채(`Tools: nil`) 나갔다. 프로바이더는 `tools` → `system` → `messages` 순으로 하나의 prefix 캐시 키를 만들기 때문에, 도구 없는 요청은 바로 앞 루프 반복들이 방금 쓴 prefix를 **읽을 수 없고**, 자기 캐시 write를 1.25배 요금으로 치르면서도 그 엔트리를 뒤이어 읽어줄 호출이 없었다. 이제 도구 목록을 유지하고 `tool_choice: none`으로 억제해 같은 계보에 머문다. 부수적으로 `ToolChoiceNone()`이 처음으로 실제 전송된다 — 모든 프로바이더가 `tool_choice`를 `len(tools) > 0`일 때만 실으므로 지금까지는 조용히 버려지고 있었다. 프로바이더가 `none`을 무시하고 도구 호출을 돌려주면 예전 방식(도구 제거)으로 한 번 재시도해, 텍스트 답변 보장은 그대로 유지한다. + - **온보딩 마법사 저장이 커스텀 tier를 지우던 문제 (#931)** — `buildConfigPayload`는 `llm_providers`를 디스크 기존값과 병합하면서 `llm_tiers`는 `heavy`/`standard`/`light`만으로 새로 만들어 통째로 반환했다. alias-keyed PATCH가 보낸 값으로 on-disk 집합을 대체하므로, `vision` 같은 커스텀 tier를 쓰는 사용자가 마법사에서 모델 하나만 바꿔 저장해도 그 tier가 설정에서 사라졌다. Config 페이지가 `llm_tiers` 편집을 마법사 딥링크로 넘기면서 이 경로가 일반 사용자에게 열렸다. 이제 tier도 provider와 동일하게 기존값 위에 덮어쓴다. - **Inspect 뷰가 저장되지 않은 값을 보여주던 문제 (#931)** — Quick Start 밖에서는 Save/Discard 버튼이 숨는데 `getDisplayValue`는 view mode와 무관하게 dirty 값을 우선해서, "Read-only inspection" 배너 아래에 커밋되지 않은 편집이 서버 상태처럼 보였다. 사용자가 반영됐다고 믿고 재시작하면 편집은 사라진다. 이제 Inspect는 서버가 실제로 로드한 값만 보여준다. - **DESIGN.md가 스스로 모순되던 문제 (#931)** — Quick Start 13개 게이트가 "stay interactive"라고 적힌 줄과, 같은 필드가 read-only가 된다고 적힌 줄이 공존했다. `embodiment_providers_json`의 실제 동작, 자격증명 7개 중 콘솔 입력란이 남은 2개와 YAML 전용이 된 5개(토큰 rotation에 호스트 파일 접근이 필요해진다는 결과 포함), 그리고 편집기 삭제 후 참조가 끊긴 `configStructured.ts` 드래프트 빌더 목록을 명시했다. diff --git a/internal/agent/loop.go b/internal/agent/loop.go index bef99fdb..18f5c07b 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -428,9 +428,35 @@ func (l *Loop) Run(ctx context.Context, initial []llm.ChatMessage, opts RunOptio finalResp, finalErr := l.client.Chat(ctx, messages, llm.ChatOptions{ OnDelta: opts.OnDelta, OnReasoningDelta: opts.OnReasoningDelta, - Tools: nil, - ToolChoice: llm.ToolChoiceNone(), + // Keep the tool list and suppress it with tool_choice=none rather than + // dropping it. Providers render tools ahead of messages into one + // prefix-matched cache key, so a tools-absent request lands in a + // different cache lineage than the loop iterations it follows: it + // cannot read the prefix they just wrote, and pays its own write + // premium for an entry those iterations can never read back. + // + // This is also the first time ToolChoiceNone reaches the wire. Every + // provider emits tool_choice only inside `if len(tools) > 0`, so with + // Tools=nil the "none" was silently dropped and suppression relied + // entirely on there being no tools to call. + Tools: llmTools, + ToolChoice: llm.ToolChoiceNone(), }) + // Dropping the tools is what used to make a text answer structural rather + // than a request the provider might ignore. Keep that guarantee as a + // fallback: if the provider honored neither tool_choice=none nor the + // implicit "answer now", retry the way this call used to be made. Costs an + // extra round trip only when a provider misbehaves, and without it that + // case degrades to the max-iterations error instead of an answer. + if finalErr == nil && len(finalResp.Message.ToolCalls) > 0 && strings.TrimSpace(finalResp.Message.Content) == "" { + l.emit(ctx, Event{Type: EventBeforeLLM, Iteration: finalIter, MessageCount: len(messages)}) + finalResp, finalErr = l.client.Chat(ctx, messages, llm.ChatOptions{ + OnDelta: opts.OnDelta, + OnReasoningDelta: opts.OnReasoningDelta, + Tools: nil, + ToolChoice: llm.ToolChoiceNone(), + }) + } if finalErr == nil { afterLLMEvent := Event{Type: EventAfterLLM, Iteration: finalIter, MessageCount: len(messages), SessionID: strings.TrimSpace(finalResp.SessionID)} if opts.AfterLLM != nil { diff --git a/internal/agent/loop_test.go b/internal/agent/loop_test.go index 5badad1d..18a4e173 100644 --- a/internal/agent/loop_test.go +++ b/internal/agent/loop_test.go @@ -816,14 +816,67 @@ func TestLoop_Run_FinalizesWithoutToolsWhenMaxIterationsReached(t *testing.T) { if len(client.seenToolCounts) != 3 { t.Fatalf("expected 3 llm calls, got %d", len(client.seenToolCounts)) } - if client.seenToolCounts[2] != 0 { - t.Fatalf("expected finalization call without tools, got %d", client.seenToolCounts[2]) + // The finalization call keeps the tool list so it stays in the same cache + // lineage as the loop iterations; suppression is tool_choice=none. What + // this test guards is the outcome — finalization yields text — not the + // mechanism. See the ignores-none fallback test below for the case where a + // provider does not honor the suppression. + if client.seenToolCounts[2] == 0 { + t.Fatalf("finalization should keep the tools it was called with, got %d", client.seenToolCounts[2]) } if client.seenToolChoice[2] != "none" { t.Fatalf("expected finalization tool_choice=none, got %q", client.seenToolChoice[2]) } } +// A provider that ignores tool_choice=none would hand back another tool call +// with no text, which the loop cannot use — before this fallback existed that +// turn degraded into the max-iterations error instead of an answer. +func TestLoop_Run_FinalizationRetriesWithoutToolsWhenProviderIgnoresNone(t *testing.T) { + reg := tool.NewRegistry() + reg.Register(tool.NewSessionStatusTool(func(_ context.Context) (tool.SessionStatus, error) { + return tool.SessionStatus{SessionID: "sess"}, nil + })) + + client := &scriptedLLMClient{ + responses: []llm.ChatResponse{ + {Message: llm.ChatMessage{Role: "assistant", ToolCalls: []llm.ToolCall{ + {ID: "call_1", Name: "session_status", Arguments: `{}`}, + }}}, + // Finalization attempt: provider ignores none and calls a tool. + {Message: llm.ChatMessage{Role: "assistant", ToolCalls: []llm.ToolCall{ + {ID: "call_2", Name: "session_status", Arguments: `{}`}, + }}}, + // Retry with the tools removed: now it answers. + {Message: llm.ChatMessage{Role: "assistant", Content: "fallback answer"}}, + }, + } + + loop := NewLoop(client, reg) + resp, err := loop.Run(context.Background(), []llm.ChatMessage{ + {Role: "user", Content: "go"}, + }, RunOptions{ + MaxIterations: 1, + Tools: reg.Schemas(), + ToolChoice: llm.ToolChoiceAuto(), + }) + if err != nil { + t.Fatalf("fallback should still produce an answer, got %v", err) + } + if resp.Message.Content != "fallback answer" { + t.Fatalf("unexpected final content: %q", resp.Message.Content) + } + if len(client.seenToolCounts) != 3 { + t.Fatalf("expected loop + finalization + retry = 3 calls, got %v", client.seenToolCounts) + } + if client.seenToolCounts[1] == 0 { + t.Fatalf("first finalization attempt should carry tools, got %v", client.seenToolCounts) + } + if client.seenToolCounts[2] != 0 { + t.Fatalf("retry must drop the tools to make a text answer structural, got %v", client.seenToolCounts) + } +} + type testRecordingEmitter struct { events []testEmittedLine } @@ -1103,3 +1156,61 @@ func TestLoop_Run_HonorsCallerResumeSessionID(t *testing.T) { t.Fatalf("expected iter 1 to receive caller resume id 'carried', got %v", client.seenResumeIDs) } } + +// The turn's final call used to drop the tool list. Providers render tools +// ahead of messages into one prefix-matched cache key, so a tools-absent +// request lands in a different cache lineage than the tool-bearing loop +// iterations it follows — it cannot read what they just wrote, and its own +// cache write is paid at a premium for an entry the next iteration cannot use. +// +// Keeping the tools and suppressing them with tool_choice=none puts the call +// back in the turn's lineage. It also makes ToolChoiceNone actually reach the +// wire: every provider emits tool_choice only when tools are present, so with +// Tools=nil the "none" was silently dropped. +func TestLoop_Run_FinalCallKeepsToolsAndSuppressesWithToolChoiceNone(t *testing.T) { + reg := tool.NewRegistry() + reg.Register(tool.NewSessionStatusTool(func(_ context.Context) (tool.SessionStatus, error) { + return tool.SessionStatus{SessionID: "sess"}, nil + })) + + client := &scriptedLLMClient{ + responses: []llm.ChatResponse{ + { + Message: llm.ChatMessage{ + Role: "assistant", + ToolCalls: []llm.ToolCall{ + {ID: "call-1", Name: "session_status", Arguments: "{}"}, + }, + }, + StopReason: "tool_use", + }, + {Message: llm.ChatMessage{Role: "assistant", Content: "done"}}, + }, + } + + loop := NewLoop(client, reg) + if _, err := loop.Run(context.Background(), []llm.ChatMessage{ + {Role: "user", Content: "status please"}, + }, RunOptions{ + Tools: reg.Schemas(), + ToolChoice: llm.ToolChoiceAuto(), + MaxIterations: 1, + }); err != nil { + t.Fatalf("run: %v", err) + } + + if len(client.seenToolCounts) < 2 { + t.Fatalf("expected the loop to reach its final call, got %v", client.seenToolCounts) + } + last := len(client.seenToolCounts) - 1 + if client.seenToolCounts[last] == 0 { + t.Fatalf("final call must still carry the tool list so it shares the turn's cache prefix, got %v", client.seenToolCounts) + } + if client.seenToolCounts[last] != client.seenToolCounts[0] { + t.Fatalf("final call tool count %d should match the loop's %d — a different tool set is a different cache prefix", + client.seenToolCounts[last], client.seenToolCounts[0]) + } + if got := client.seenToolChoice[last]; got != "none" { + t.Fatalf("final call must suppress tool use with tool_choice=none, got %q", got) + } +}