diff --git a/internal/sessions/exec_prompt_tool_call_identity_test.go b/internal/sessions/exec_prompt_tool_call_identity_test.go new file mode 100644 index 000000000..ad55e34b2 --- /dev/null +++ b/internal/sessions/exec_prompt_tool_call_identity_test.go @@ -0,0 +1,463 @@ +package sessions + +import ( + "fmt" + "strings" + "testing" +) + +// A CALL'S PAYLOAD MUST NOT RIDE INTO A LATER PROMPT ANY MORE THAN A RESULT'S. +// +// toolResultOutcome drops result bodies, and calls were left verbatim, so an +// interrupted write_file put its content into the next turn's prompt while the +// matching result body did not. Same secret, one door left open. The identity +// still has to survive, since a resumed turn knowing WHICH file was written is +// the whole point of admitting calls at all. +// +// Pinned as whole lines, compared as fields, for the same reason the result test +// is: a substring search for one fixture secret passes when a truncated or +// reworded copy leaks, and the renderer walks a map so field order varies. +func TestResumePromptCarriesToolCallIdentityWithoutPayload(t *testing.T) { + const secret = "AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG" + const token = "Bearer sk-live-4eC39HqLyjWDarjtT1zdp7dc" + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "rotate the deploy key"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c1", "name": "write_file", "arguments": `{"path":"deploy/prod.env","content":"` + secret + `"}`})}, + {Sequence: 3, Type: EventToolResult, Payload: toolContextPayload(t, map[string]any{"name": "write_file", "status": "ok", "output": "written"})}, + {Sequence: 4, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c2", "name": "edit_file", "arguments": `{"path":"deploy/prod.env","old_string":"` + secret + `","new_string":"rotated"}`})}, + {Sequence: 5, Type: EventToolResult, Payload: toolContextPayload(t, map[string]any{"name": "edit_file", "status": "ok"})}, + {Sequence: 6, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c3", "name": "exec_command", "arguments": `{"cmd":"curl -H '` + token + `' https://api.example/rotate","workdir":"deploy"}`})}, + {Sequence: 7, Type: EventToolResult, Payload: toolContextPayload(t, map[string]any{"name": "exec_command", "status": "ok"})}, + {Sequence: 8, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c4", "name": "apply_patch", "arguments": `{"patch":"*** Begin Patch\n*** Update File: deploy/prod.env\n-` + secret + `\n+rotated\n*** End Patch"}`})}, + {Sequence: 9, Type: EventToolResult, Payload: toolContextPayload(t, map[string]any{"name": "apply_patch", "status": "ok"})}, + {Sequence: 10, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c5", "name": "read_file", "arguments": `{"path":"deploy/prod.env","offset":1,"limit":40}`})}, + {Sequence: 11, Type: EventToolResult, Payload: toolContextPayload(t, map[string]any{"name": "read_file", "status": "ok", "output": secret})}, + {Sequence: 12, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error: upstream timeout"})}, + } + out := resumePrompt(t, events) + + // The payloads: none of them, in any form. + for _, leaked := range []string{secret, secret[:16], token, token[:14], "rotated", "Begin Patch", "curl -H"} { + if strings.Contains(out, leaked) { + t.Errorf("tool call payload %q reached the resume prompt:\n%s", leaked, out) + } + } + + // The identities: every one of them, from the call side. + want := map[int]struct { + kind string + fields []string + }{ + 2: {"tool_call", []string{"c1", "write_file", `{"path":"deploy/prod.env"}`}}, + 4: {"tool_call", []string{"c2", "edit_file", `{"path":"deploy/prod.env"}`}}, + 6: {"tool_call", []string{"c3", "exec_command", `{"workdir":"deploy"}`}}, + 8: {"tool_call", []string{"c4", "apply_patch"}}, + 10: {"tool_call", []string{"c5", "read_file"}}, + } + seen := map[int]bool{} + for _, line := range strings.Split(out, "\n") { + if !strings.HasPrefix(line, "- #") || !strings.Contains(line, "tool_call:") { + continue + } + var sequence int + var kind string + rest, _ := strings.CutPrefix(line, "- #") + if _, err := fmt.Sscanf(rest, "%d %s", &sequence, &kind); err != nil { + t.Fatalf("unparsable context line %q: %v", line, err) + } + expected, wanted := want[sequence] + if !wanted { + t.Errorf("unexpected tool_call line %q", line) + continue + } + seen[sequence] = true + _, payload, _ := strings.Cut(rest, ": ") + fields := strings.Fields(payload) + for _, field := range expected.fields { + if !containsField(fields, field) { + t.Errorf("line #%d lost identity field %q: %q", sequence, field, line) + } + } + } + for sequence := range want { + if !seen[sequence] { + t.Errorf("tool_call #%d did not reach the resume prompt at all, so the interrupted work is invisible again", sequence) + } + } + // read_file keeps its window too, or a resumed turn re-reads a part it had. + if !strings.Contains(out, `"offset":1`) || !strings.Contains(out, `"limit":40`) { + t.Errorf("read_file lost its offset/limit window:\n%s", out) + } +} + +func containsField(fields []string, want string) bool { + for _, field := range fields { + if field == want { + return true + } + } + // The reduced arguments object may carry more than one identity key, in map + // order, so a single-key expectation is also satisfied by an object that + // contains that key. + if strings.HasPrefix(want, "{") { + key := strings.TrimSuffix(strings.TrimPrefix(want, "{"), "}") + for _, field := range fields { + if strings.HasPrefix(field, "{") && strings.Contains(field, key) { + return true + } + } + } + return false +} + +// ARGUMENTS THAT CANNOT BE READ ARE NOT PASSED THROUGH AS TEXT. +// +// An allow-list only protects what it can see. Arguments that do not decode as +// an object are dropped rather than rendered, since text this file could not +// inspect is text it cannot vouch for. +func TestResumePromptDropsUnreadableToolCallArguments(t *testing.T) { + const secret = "sk-live-unparseable-9f8e7d6c" + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "go"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c1", "name": "bash", "arguments": `not json at all ` + secret})}, + {Sequence: 3, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error"})}, + } + out := resumePrompt(t, events) + if strings.Contains(out, secret) { + t.Errorf("unparseable arguments were rendered as text:\n%s", out) + } + if !strings.Contains(out, "bash") { + t.Errorf("the call's identity was lost along with its unreadable arguments:\n%s", out) + } +} + +// AND A TOOL THIS FILE HAS NEVER HEARD OF STILL KEEPS ITS PATH. +// +// The allow-list is by key, not by tool name, so an MCP tool or a future core +// tool whose argument is a path or a url carries it into the resume prompt +// without being enumerated here, while any body field it has is dropped. +func TestResumePromptKeepsIdentityForUnknownTools(t *testing.T) { + const body = "large opaque payload that must not be replayed" + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "go"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c1", "name": "mcp_uploader", "arguments": `{"url":"https://files.example/x","blob":"` + body + `"}`})}, + {Sequence: 3, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error"})}, + } + out := resumePrompt(t, events) + if strings.Contains(out, body) { + t.Errorf("an unlisted body field was replayed:\n%s", out) + } + if !strings.Contains(out, "https://files.example/x") { + t.Errorf("an unlisted tool lost its url identity:\n%s", out) + } +} + +// AND AN ALLOW-LISTED KEY IS NOT A SAFE VALUE. +// +// web_fetch accepts a credential in the query string and redacts the URL it +// reports back, so an interrupted fetch had its token dropped from the result +// and kept verbatim in the call. This projection is what admits the call into a +// later turn's prompt, so the token rode into the next resume or fork. The +// identity the resumed turn actually needs is the host and path, and those +// survive. +func TestResumePromptRedactsCredentialsInsideRetainedValues(t *testing.T) { + const token = "secret-value-9f8e7d6c5b4a" + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "check the feed"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c1", "name": "web_fetch", "arguments": `{"url":"https://api.example/data?access_token=` + token + `&page=2"}`})}, + {Sequence: 3, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c2", "name": "web_fetch", "arguments": `{"url":"https://reader:hunter2@api.example/feed"}`})}, + {Sequence: 4, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error: upstream timeout"})}, + } + out := resumePrompt(t, events) + + for _, leaked := range []string{token, token[:12], "hunter2"} { + if strings.Contains(out, leaked) { + t.Errorf("credential %q reached the resume prompt:\n%s", leaked, out) + } + } + // The non-secret identity is the whole reason calls are admitted at all. + for _, kept := range []string{"api.example", "/data", "page=2", "/feed"} { + if !strings.Contains(out, kept) { + t.Errorf("URL identity %q was lost to the scrub:\n%s", kept, out) + } + } +} + +// The scrub must not eat ordinary identities. A path, a glob and a query are +// what the resumed turn navigates by, and none of them is a credential. +func TestResumePromptKeepsOrdinaryIdentitiesThroughTheScrub(t *testing.T) { + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "go"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c1", "name": "read_file", "arguments": `{"path":"internal/sessions/exec_session.go","offset":1,"limit":40}`})}, + {Sequence: 3, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c2", "name": "grep", "arguments": `{"pattern":"func toolCallIdentity","glob":"src/**/*.tsx"}`})}, + {Sequence: 4, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error"})}, + } + out := resumePrompt(t, events) + for _, kept := range []string{"internal/sessions/exec_session.go", "func toolCallIdentity", "src/**/*.tsx"} { + if !strings.Contains(out, kept) { + t.Errorf("identity %q was lost to the scrub:\n%s", kept, out) + } + } + // Numbers are not strings and are nothing to scrub; they must survive whole. + if !strings.Contains(out, "40") { + t.Errorf("a numeric read window did not survive the scrub:\n%s", out) + } +} + +// AND A FIELD THIS FILE HAS NEVER HEARD OF DOES NOT RIDE ALONG. +// +// toolResultOutcome builds its output from the fields it keeps, so an unknown +// one cannot reach a prompt through it. The call side edited arguments in place +// and returned every sibling key, which is the opposite contract: a producer +// recording one more top-level field, or the same payload with no arguments at +// all, put that field straight into the next turn. The projection names what +// survives, so both halves now drop what they do not name. +func TestResumePromptProjectsTopLevelToolCallFields(t *testing.T) { + const body = "opaque top-level payload that must not be replayed" + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "go"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{ + "id": "c1", "name": "write_file", "arguments": `{"path":"deploy/prod.env"}`, "rawInput": body, + })}, + // The same shape with no arguments at all, which used to return the whole + // payload untouched. + {Sequence: 3, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{ + "id": "c2", "name": "read_file", "stashedContent": body, + })}, + {Sequence: 4, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error"})}, + } + out := resumePrompt(t, events) + + for _, leaked := range []string{body, "rawInput", "stashedContent"} { + if strings.Contains(out, leaked) { + t.Errorf("an unnamed top-level field %q reached the resume prompt:\n%s", leaked, out) + } + } + // The identity itself still survives, from both calls. + for _, kept := range []string{"c1", "write_file", "deploy/prod.env", "c2", "read_file"} { + if !strings.Contains(out, kept) { + t.Errorf("call identity %q was lost to the projection:\n%s", kept, out) + } + } +} + +// AND A PERMITTED KEY IS NOT PERMISSION FOR WHATEVER HANGS UNDER IT. +// +// The projection scrubbed strings and passed everything else through, which +// reads as "a number is nothing to scrub" and is true of a number and not of an +// object. MCP schemas allow object and array properties, so a whole payload can +// arrive under a permitted key. Redacting a serialized object would not help: +// ordinary private text has no credential shape to match. +func TestResumePromptDropsNestedArgumentContainers(t *testing.T) { + const body = "private body text with no credential shape at all" + const token = "secret-value-9f8e7d6c5b4a" + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "go"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{ + "id": "c1", "name": "mcp_search", + "arguments": `{"query":{"api_key":"` + token + `","content":"` + body + `"}}`, + })}, + {Sequence: 3, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{ + "id": "c2", "name": "web_fetch", + "arguments": `{"url":["https://api.example/data?access_token=` + token + `"]}`, + })}, + {Sequence: 4, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error"})}, + } + out := resumePrompt(t, events) + + for _, leaked := range []string{body, body[:20], token, token[:12], "api_key"} { + if strings.Contains(out, leaked) { + t.Errorf("nested argument content %q reached the resume prompt:\n%s", leaked, out) + } + } + // The call still says which tool ran, so a resumed turn can ask again. + for _, kept := range []string{"c1", "mcp_search", "c2", "web_fetch"} { + if !strings.Contains(out, kept) { + t.Errorf("call identity %q was lost along with the container:\n%s", kept, out) + } + } +} + +// THE SEARCH ALIASES ARE TOOL-SPECIFIC, AND ONE OF THEM IS A TRAP. +// +// grep accepts `search` for its pattern and edit_file accepts the same word for +// the text being replaced. Admitting it globally for the sake of the first would +// replay file contents through the second. +func TestResumePromptKeepsGrepSearchAndDropsEditSearch(t *testing.T) { + const oldText = "OLD FILE CONTENTS THAT MUST NOT BE REPLAYED" + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "go"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{ + "id": "c1", "name": "grep", "arguments": `{"search":"func toolCallIdentity","path":"internal/sessions"}`, + })}, + {Sequence: 3, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{ + "id": "c2", "name": "edit_file", "arguments": `{"path":"a.go","search":"` + oldText + `","replace":"new"}`, + })}, + {Sequence: 4, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error"})}, + } + out := resumePrompt(t, events) + + if !strings.Contains(out, "func toolCallIdentity") { + t.Errorf("grep lost the pattern it was searching for:\n%s", out) + } + if strings.Contains(out, oldText) || strings.Contains(out, oldText[:20]) { + t.Errorf("edit_file replayed the text it was replacing:\n%s", out) + } + // Both calls keep the path, which is identity for either tool. + for _, kept := range []string{"internal/sessions", "a.go"} { + if !strings.Contains(out, kept) { + t.Errorf("path identity %q was lost:\n%s", kept, out) + } + } +} + +// AND THE SUPPORTED SEARCH AND WINDOW FORMS SURVIVE, not only the canonical +// ones. A resumed turn that knows the path but not which slice was asked for +// has half the identity and will read again. +func TestResumePromptKeepsSupportedAliasesAndWindows(t *testing.T) { + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "go"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{ + "id": "c1", "name": "glob", "arguments": `{"match":"src/**/*.go"}`, + })}, + {Sequence: 3, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{ + "id": "c2", "name": "read_file", "arguments": `{"path":"large.go","byte_offset":4096,"byte_limit":512}`, + })}, + {Sequence: 4, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{ + "id": "c3", "name": "read_file", "arguments": `{"path":"big.go","start_line":40,"end_line":80,"max_lines":41}`, + })}, + {Sequence: 5, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error"})}, + } + out := resumePrompt(t, events) + + // KEY AND VALUE TOGETHER, NOT THE NUMBER ALONE. Searching for "40" finds + // it inside "4096", so the start_line assertion passed whether or not + // start_line survived at all, which is the opposite of what it claims. + for _, kept := range []string{ + "src/**/*.go", + `"byte_offset":4096`, `"byte_limit":512`, + `"start_line":40`, `"end_line":80`, `"max_lines":41`, + } { + if !strings.Contains(out, kept) { + t.Errorf("supported identity or window %s was lost:\n%s", kept, out) + } + } +} + +// THE SHARED TABLE MUST NEVER NAME A MUTATING TOOL'S BODY ARGUMENT. +// +// This is the rule the grep/edit_file collision taught, kept as a test so the +// next alias added for a search tool cannot quietly open the same door. The +// aliases below are the ones the mutating tools accept for the content they +// write; they belong in the per-tool table or nowhere. +func TestIdentityKeysNeverNameAMutatingToolBodyArgument(t *testing.T) { + bodyAliases := map[string][]string{ + "edit_file": {"old_string", "old", "search", "find", "old_str", "new_string", "new", "replace", "replacement", "new_str"}, + "write_file": {"content", "contents", "text", "body", "data", "file_content"}, + "apply_patch": {"patch"}, + } + for tool, aliases := range bodyAliases { + for _, alias := range aliases { + if toolCallIdentityKeys[alias] { + t.Errorf("%s accepts %q for content it writes, and the shared identity table admits it: a resumed prompt would replay the body", tool, alias) + } + } + } + // The per-tool table is where an ambiguous name belongs, and grep's use of + // `search` is the case that proves the mechanism is wired. + if !toolCallIdentityKeysByTool["grep"]["search"] { + t.Error("grep lost its search alias, so an interrupted grep resumes without the pattern") + } +} + +// THROUGH THE STORE, NOT JUST THE RENDERER. +// +// Every test above hands FormatExecPrompt an events slice it built in memory, +// so none of them shows the projection still applies to events that were +// written, reloaded and prepared by the real resume and fork paths. That is +// where it has to hold: the leak this guards against is a later turn reading a +// recorded call, and the recording is what makes it later. +// +// Includes a call the tool would have REJECTED. The agent records OnToolCall +// before execution, so a read_file whose path is an object is persisted even +// though the tool refuses it, and a projection that only considered +// well-formed calls would carry it through. +func TestPersistedToolCallsAreProjectedOnResumeAndFork(t *testing.T) { + const body = "private body text with no credential shape at all" + const token = "secret-value-9f8e7d6c5b4a" + const oldText = "OLD FILE CONTENTS THAT MUST NOT BE REPLAYED" + + store := NewStore(StoreOptions{RootDir: t.TempDir()}) + session, err := store.Create(CreateInput{SessionID: "projection_persisted"}) + if err != nil { + t.Fatal(err) + } + appends := []AppendEventInput{ + {Type: EventMessage, Payload: map[string]any{"role": "user", "content": "go"}}, + {Type: EventToolCall, Payload: map[string]any{ + "id": "c1", "name": "mcp_search", + "arguments": `{"query":{"api_key":"` + token + `","content":"` + body + `"}}`, + }}, + {Type: EventToolCall, Payload: map[string]any{ + "id": "c2", "name": "edit_file", + "arguments": `{"path":"a.go","search":"` + oldText + `","replace":"new"}`, + }}, + // Rejected by read_file, recorded anyway. + {Type: EventToolCall, Payload: map[string]any{ + "id": "c3", "name": "read_file", + "arguments": `{"path":{"nested":"` + body + `"},"start_line":40}`, + }}, + {Type: EventToolCall, Payload: map[string]any{ + "id": "c4", "name": "grep", + "arguments": `{"search":"func toolCallIdentity","path":"internal/sessions"}`, + }}, + {Type: EventError, Payload: map[string]any{"message": "provider error: upstream timeout"}}, + } + if _, err := store.AppendEvents(session.SessionID, appends); err != nil { + t.Fatal(err) + } + + for _, mode := range []struct { + name string + options PrepareExecOptions + }{ + {"resume", PrepareExecOptions{Store: store, Resume: session.SessionID}}, + {"fork", PrepareExecOptions{Store: store, Fork: session.SessionID, SessionID: "projection_persisted_fork"}}, + } { + t.Run(mode.name, func(t *testing.T) { + prepared, err := PrepareExec(mode.options) + if err != nil { + t.Fatalf("PrepareExec: %v", err) + } + prompt := FormatExecPrompt("continue", prepared) + + for _, leaked := range []string{body, body[:20], token, token[:12], oldText, oldText[:20], "api_key"} { + if strings.Contains(prompt, leaked) { + t.Errorf("%q survived persistence into the %s prompt:\n%s", leaked, mode.name, prompt) + } + } + // The identities that make the tail worth carrying at all. + for _, kept := range []string{"mcp_search", "edit_file", "read_file", "grep", "func toolCallIdentity", "internal/sessions", "a.go"} { + if !strings.Contains(prompt, kept) { + t.Errorf("identity %q was lost from the %s prompt:\n%s", kept, mode.name, prompt) + } + } + }) + } + + // AND THE STORE STILL HOLDS THE ORIGINALS. The projection is a view for the + // prompt, not an edit to the record: /rewind and an audit read the events. + events, err := store.ReadEvents(session.SessionID) + if err != nil { + t.Fatal(err) + } + stored := "" + for _, event := range events { + stored += string(event.Payload) + } + for _, want := range []string{body, token, oldText} { + if !strings.Contains(stored, want) { + t.Errorf("the projection modified the persisted events: %q is no longer in the store", want) + } + } +} diff --git a/internal/sessions/exec_prompt_tool_context_test.go b/internal/sessions/exec_prompt_tool_context_test.go new file mode 100644 index 000000000..ad9e5a401 --- /dev/null +++ b/internal/sessions/exec_prompt_tool_context_test.go @@ -0,0 +1,227 @@ +package sessions + +import ( + "encoding/json" + "fmt" + "slices" + "strings" + "testing" +) + +func toolContextPayload(t *testing.T, value map[string]any) json.RawMessage { + t.Helper() + raw, err := json.Marshal(value) + if err != nil { + t.Fatal(err) + } + return json.RawMessage(raw) +} + +// interruptedTurnEvents is a turn that read two files and then died on a +// provider error, which is the shape #913 reports: no assistant answer was ever +// produced, so nothing in the conversation describes the work. +func interruptedTurnEvents(t *testing.T) []Event { + t.Helper() + return []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "add retries to the http client"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c1", "name": "read_file", "arguments": `{"path":"internal/http/client.go"}`})}, + {Sequence: 3, Type: EventToolResult, Payload: toolContextPayload(t, map[string]any{"name": "read_file", "content": "package http"})}, + {Sequence: 4, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c2", "name": "read_file", "arguments": `{"path":"internal/http/retry.go"}`})}, + {Sequence: 5, Type: EventToolResult, Payload: toolContextPayload(t, map[string]any{"name": "read_file", "content": "var defaultBackoff = 250ms"})}, + {Sequence: 6, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error: upstream timeout"})}, + } +} + +func resumePrompt(t *testing.T, events []Event) string { + t.Helper() + return FormatExecPrompt("continue", PreparedExec{ + Mode: ModeResume, + Session: Metadata{SessionID: "s1"}, + ContextEvents: events, + }) +} + +// AN INTERRUPTED TURN HAS TO SAY WHAT IT DID. +// +// A turn that ends normally describes its own work in the assistant's answer, +// so the next turn inherits a prose record. A turn killed by a provider error +// produces no answer, and the tool events that were the only record of the work +// were filtered out of the resume context. The next turn was told a request had +// been made and an error had happened, and nothing else, so it re-read from +// scratch (#913). +func TestResumePromptCarriesInterruptedToolWork(t *testing.T) { + out := resumePrompt(t, interruptedTurnEvents(t)) + + for _, want := range []string{"internal/http/client.go", "internal/http/retry.go"} { + if !strings.Contains(out, want) { + t.Errorf("the resume prompt does not name %s, so the next turn cannot know it was already read:\n%s", want, out) + } + } + // The conversation spine is still there. + for _, want := range []string{"add retries to the http client", "upstream timeout"} { + if !strings.Contains(out, want) { + t.Errorf("the resume prompt lost conversation context %q:\n%s", want, out) + } + } +} + +// AND THE ORDER STAYS THE ORDER IT HAPPENED IN. +// +// The sequence number is rendered into each line, so a list that jumps +// backwards reads as a corrupted history rather than a merge artifact. +func TestResumePromptKeepsEventsInSequenceOrder(t *testing.T) { + out := resumePrompt(t, interruptedTurnEvents(t)) + + last := -1 + for _, line := range strings.Split(out, "\n") { + if !strings.HasPrefix(line, "- #") { + continue + } + var seq int + if _, err := fmt.Sscanf(line, "- #%d", &seq); err != nil { + t.Fatalf("unparsable context line %q", line) + } + if seq <= last { + t.Fatalf("context is out of order at %q (previous #%d):\n%s", line, last, out) + } + last = seq + } + if last < 0 { + t.Fatal("SETUP INVALID: the prompt rendered no context lines") + } +} + +// TOOL EVENTS MUST NOT EVICT CONVERSATION. +// +// This is what the original filter was added for (#460): tool events vastly +// outnumber messages, so admitting them into the same trailing budget would let +// one tool-heavy turn push every earlier message out of the context. A separate +// allowance is what keeps both properties at once. +func TestToolEventsNeverEvictConversation(t *testing.T) { + var events []Event + seq := 0 + next := func() int { seq++; return seq } + // An early message that must survive, then a flood of tool work. + events = append(events, Event{Sequence: next(), Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "EARLIEST-REQUEST-MARKER"})}) + for i := 0; i < 500; i++ { + arguments := fmt.Sprintf(`{"path":"noise/%03d.go"}`, i) + events = append(events, Event{Sequence: next(), Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"name": "read_file", "arguments": arguments})}) + events = append(events, Event{Sequence: next(), Type: EventToolResult, Payload: toolContextPayload(t, map[string]any{"name": "read_file", "content": "noise"})}) + } + events = append(events, Event{Sequence: next(), Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "LATEST-REQUEST-MARKER"})}) + + out := resumePrompt(t, events) + for _, want := range []string{"EARLIEST-REQUEST-MARKER", "LATEST-REQUEST-MARKER"} { + if !strings.Contains(out, want) { + t.Errorf("a tool-heavy turn evicted conversation event %q, which is the regression the filter exists to prevent", want) + } + } + + selected := promptContextEvents(events) + tools := 0 + for _, event := range selected { + if event.Type == EventToolCall || event.Type == EventToolResult { + tools++ + } + } + if tools == 0 { + t.Error("no tool work survived at all, so an interrupted tool-heavy turn still says nothing about what it did") + } + if len(selected) > 80 { + t.Errorf("selected %d events, over the 80 the prompt budget allows", len(selected)) + } +} + +// A session with no tool work renders exactly as it did before. +func TestResumePromptUnchangedWithoutToolEvents(t *testing.T) { + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "hello"})}, + {Sequence: 2, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "assistant", "content": "hi"})}, + } + out := resumePrompt(t, events) + if strings.Count(out, "- #") != 2 { + t.Fatalf("a tool-free session rendered %d context lines, want 2:\n%s", strings.Count(out, "- #"), out) + } +} + +// TOOL OUTPUT MUST NOT RIDE ALONG INTO A LATER PROMPT. +// +// The tail is there to say what the interrupted turn did, and the call already +// says that. Carrying the result body would put up to 500 bytes of raw tool +// output into a prompt on a later turn, and nothing redacts on the way in. +func TestResumePromptCarriesToolOutcomeWithoutOutput(t *testing.T) { + secret := "AKIAIOSFODNN7EXAMPLE-and-more-file-contents" + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: toolContextPayload(t, map[string]any{"role": "user", "content": "look at the config"})}, + {Sequence: 2, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c1", "name": "read_file", "arguments": `{"path":"deploy/prod.env"}`})}, + {Sequence: 3, Type: EventToolResult, Payload: toolContextPayload(t, map[string]any{"name": "read_file", "status": "ok", "output": secret})}, + {Sequence: 4, Type: EventToolCall, Payload: toolContextPayload(t, map[string]any{"id": "c2", "name": "read_file", "arguments": `{"path":"deploy/missing.env"}`})}, + {Sequence: 5, Type: EventToolResult, Payload: toolContextPayload(t, map[string]any{"name": "read_file", "status": "error", "output": "no such file"})}, + {Sequence: 6, Type: EventError, Payload: toolContextPayload(t, map[string]any{"message": "provider error: upstream timeout"})}, + } + out := resumePrompt(t, events) + + // THE TOOL LINES ARE PINNED WHOLE, NOT SEARCHED. + // + // Two reasons the obvious assertions are too weak to say what the trim + // promises. Searching the prompt for the fixture's full secret only rejects + // that exact string, so a change that carried a truncated or partly + // redacted prefix of the output would pass it. And asking whether "ok" and + // "error" each appear somewhere among the result lines passes just as well + // when the two statuses are swapped, or both attached to the wrong call. + // + // Pinning the whole of every tool line covers both: nothing but the tool + // name and the outcome survives a result, each outcome sits on its own + // line, and the line order pairs each one with the call above it. + // + // Compared as a set of fields per line rather than as a string, because the + // renderer walks the payload map and Go randomizes that order, so the same + // events render their fields in a different order on every run. + want := []struct { + sequence int + kind string + fields []string + }{ + {2, "tool_call", []string{"c1", "read_file", `{"path":"deploy/prod.env"}`}}, + {3, "tool_result", []string{"read_file", "ok"}}, + {4, "tool_call", []string{"c2", "read_file", `{"path":"deploy/missing.env"}`}}, + {5, "tool_result", []string{"read_file", "error"}}, + } + var toolLines []string + for _, line := range strings.Split(out, "\n") { + if strings.HasPrefix(line, "- #") && (strings.Contains(line, "tool_call:") || strings.Contains(line, "tool_result:")) { + toolLines = append(toolLines, line) + } + } + if len(toolLines) != len(want) { + t.Fatalf("the resume prompt carries %d tool lines, want %d:\n%s", len(toolLines), len(want), out) + } + for index, line := range toolLines { + var sequence int + var kind string + rest, found := strings.CutPrefix(line, "- #") + if !found { + t.Fatalf("unparsable context line %q", line) + } + if _, err := fmt.Sscanf(rest, "%d %s", &sequence, &kind); err != nil { + t.Fatalf("unparsable context line %q: %v", line, err) + } + kind = strings.TrimSuffix(kind, ":") + _, payload, _ := strings.Cut(rest, ": ") + fields := strings.Fields(payload) + slices.Sort(fields) + expected := slices.Clone(want[index].fields) + slices.Sort(expected) + if sequence != want[index].sequence || kind != want[index].kind || !slices.Equal(fields, expected) { + t.Errorf("tool line %d is not what the trim promises.\ngot: #%d %s %v\nwant: #%d %s %v\nfull prompt:\n%s", + index, sequence, kind, fields, want[index].sequence, want[index].kind, expected, out) + } + } + // Named on its own so a leak reports as a leak rather than as the + // formatting drift the comparison above would also catch. A PREFIX, because + // the danger is output reaching a later prompt at all, not this exact + // fixture string reaching it. + if strings.Contains(out, secret[:16]) { + t.Errorf("tool output reached the resume prompt:\n%s", out) + } +} diff --git a/internal/sessions/exec_session.go b/internal/sessions/exec_session.go index b86b364b5..774f9a296 100644 --- a/internal/sessions/exec_session.go +++ b/internal/sessions/exec_session.go @@ -4,8 +4,11 @@ import ( "encoding/json" "fmt" "log" + "sort" "strings" "unicode/utf8" + + "github.com/Gitlawb/zero/internal/redaction" ) type ExecMode string @@ -188,23 +191,307 @@ func FormatExecPrompt(prompt string, prepared PreparedExec) string { }, "\n") } +// promptContextEvents chooses what a resumed turn is told about the session so +// far. +// +// A RESUMED TURN NEEDS TO KNOW WHAT THE PREVIOUS ONE DID, NOT ONLY WHAT IT SAID. +// +// Conversation events alone were selected here, so a turn that read six files +// and then died before answering left this behind: +// +// - #1 message: add retries to the http client +// - #6 error: provider error: upstream timeout +// +// Nothing names the files, so the next turn re-reads them from scratch (#913). +// On a turn that ENDS NORMALLY the assistant's answer describes the work, which +// is why this was survivable; an interrupted turn produces no such answer, and +// the record of the work goes with it. +// +// ONLY THE UNSUMMARIZED TAIL, NOT EVERY TOOL EVENT. Filtering tool events out +// was deliberate (#460) and is right for work the assistant has already +// described: forty read_file results add length and no information next to the +// answer that explains them. It is wrong only for work with no answer after it, +// which is precisely what an interrupted turn leaves behind. So the events after +// the last assistant message come along, and everything the assistant already +// spoke for stays filtered. +// +// The tail also gets its own allowance rather than sharing the conversation +// budget, so a tool-heavy interrupted turn can never push an earlier message out +// of the context. That is the property #460 added this filter for and it still +// holds. func promptContextEvents(events []Event) []Event { const maxPromptContextEvents = 80 + // Enough to describe an interrupted turn's work, small enough that the + // conversation still dominates the prompt. + const maxPromptContextTailEvents = 24 - filtered := make([]Event, 0, len(events)) - for _, event := range events { + lastSpoken := -1 + for index, event := range events { + if event.Type == EventMessage && payloadRole(event.Payload) == "assistant" { + lastSpoken = index + } + } + + conversation := make([]Event, 0, len(events)) + tail := make([]Event, 0, maxPromptContextTailEvents) + for index, event := range events { switch event.Type { case EventMessage, EventCompaction, EventSessionFork, EventSessionChild, EventSpecialistStart, EventSpecialistStop, EventError: - filtered = append(filtered, event) + conversation = append(conversation, event) + case EventToolCall: + if index > lastSpoken { + tail = append(tail, toolCallIdentity(event)) + } + case EventToolResult: + if index > lastSpoken { + tail = append(tail, toolResultOutcome(event)) + } } } - if len(filtered) == 0 { - filtered = append(filtered, events...) + if len(conversation) == 0 && len(tail) == 0 { + // An unrecognized event stream still says more than nothing. + conversation = append(conversation, events...) + } + if len(conversation) > maxPromptContextEvents { + conversation = conversation[len(conversation)-maxPromptContextEvents:] + } + // Tail slots exist only when the conversation uses fewer than the 80-event + // cap. A session whose conversation already fills it carries no interrupted + // tool work, which is what every session did before tool events were admitted + // at all, and what the #460 cap is there to hold. Reserving a minimum tail by + // trimming conversation further is a product decision, not this change. + tailBudget := min(maxPromptContextTailEvents, maxPromptContextEvents-len(conversation)) + if tailBudget < 0 { + tailBudget = 0 + } + if len(tail) > tailBudget { + tail = tail[len(tail)-tailBudget:] + } + if len(tail) == 0 { + return conversation + } + // Merged back into execution order: the sequence numbers are rendered, so a + // list that jumps backwards would read as a corrupted history. + merged := make([]Event, 0, len(conversation)+len(tail)) + merged = append(merged, conversation...) + merged = append(merged, tail...) + sort.SliceStable(merged, func(i, j int) bool { return merged[i].Sequence < merged[j].Sequence }) + return merged +} + +// toolCallIdentityKeys are the argument fields that say WHAT a call was about +// without carrying what it was about to write, run, or search for. +// +// Path, directory, URL, name and pattern fields identify the work: the file that +// was read, the tree that was listed, the expression that was searched. Body +// fields carry payload: write_file's content, edit_file's old and new strings, +// apply_patch's hunks, a shell command with whatever credential was on its +// line. The result side already drops payload through toolResultOutcome, and +// admitting calls into resume context without the same care put an interrupted +// write_file's content into the next turn's prompt while its result body did +// not. Same fact, one door left open. +// +// AN ALLOW-LIST, NOT A DENY-LIST, so an argument this file has never heard of is +// dropped rather than replayed. A new tool with a new body field is then a +// missing path in a resume prompt, which is visible, instead of a new leak, which +// is not. The keys cover every alias the tools accept for the identity fields. +var toolCallIdentityKeys = map[string]bool{ + // files and directories + "path": true, "file": true, "file_path": true, "filepath": true, "filename": true, + "dir": true, "directory": true, "cwd": true, "workdir": true, + // what a search was for; these are what the interrupted turn was looking at + "pattern": true, "glob": true, "query": true, "regex": true, "expression": true, + "match": true, + // fetches and named resources + "url": true, "name": true, + // read windows, so a resumed turn knows which part it already had. Every + // form read_file accepts, because a resumed turn that knows the path but not + // which slice was asked for has half the identity and will read again. + "offset": true, "limit": true, + "start_line": true, "end_line": true, "max_lines": true, + "byte_offset": true, "byte_limit": true, +} + +// toolCallIdentityKeysByTool holds the argument names whose meaning depends on +// which tool was called, so they cannot live in the table above. +// +// grep accepts `search` for its pattern, and edit_file accepts the same word +// for the text being replaced. Admitting it globally for the sake of the first +// would replay file contents through the second, which is the whole thing this +// projection exists to stop. The rule this encodes: an argument name that any +// MUTATING tool accepts for body content is scoped to the tools that mean +// something else by it, never added to the shared table. +var toolCallIdentityKeysByTool = map[string]map[string]bool{ + "grep": {"search": true}, +} + +// retainedIdentityValue decides whether a value under a permitted key may be +// kept, and what it looks like if so. +// +// AN ALLOW-LISTED KEY IS NOT A SAFE VALUE. A url is a valid place for a +// credential to appear: web_fetch accepts a query token and redacts the URL it +// reports back, so an interrupted fetch of +// https://api.example/data?access_token=... had its token dropped from the +// result and kept verbatim in the call. This projection is what admits call +// arguments into a later turn's prompt, so the same scrub belongs here or the +// credential is replayed on resume. Host and path survive it, which is the +// identity the resumed turn needs. +// +// AND A KEY IS NOT PERMISSION FOR WHATEVER HANGS UNDER IT. Scrubbing strings +// and passing everything else through read as "numbers are nothing to scrub", +// which is true of a number and not of an object: MCP schemas allow object and +// array properties, so {"query":{"api_key":"...","content":"private body"}} +// arrives under a permitted key and carries a whole payload with it. Redacting +// a serialized object would not help either, since ordinary private text has no +// credential shape to match. +// +// So the retained shapes are named: a string, scrubbed, and the scalars a read +// window is made of. A container is dropped. The call keeps its name and id, so +// a resumed turn still knows which tool ran and can ask again; what it does not +// get is arbitrary nested content it was never meant to see. +func retainedIdentityValue(value any) (any, bool) { + switch typed := value.(type) { + case string: + return redaction.RedactString(typed, redaction.Options{}), true + case float64, int, int64, bool, json.Number: + return typed, true + default: + // Objects, arrays, null, and anything a future decoder invents. + return nil, false + } +} + +// toolCallIdentityKeyAllowed reports whether key is an identity field for the +// tool that was called. +func toolCallIdentityKeyAllowed(toolName, key string) bool { + lowered := strings.ToLower(key) + if toolCallIdentityKeys[lowered] { + return true } - if len(filtered) > maxPromptContextEvents { - filtered = filtered[len(filtered)-maxPromptContextEvents:] + return toolCallIdentityKeysByTool[strings.ToLower(strings.TrimSpace(toolName))][lowered] +} + +// toolCallIdentityFields are the top-level payload fields a projected call +// keeps: which call it was, and which tool. Everything else on the payload is +// dropped, including a field added by a future producer that this file has +// never seen. +var toolCallIdentityFields = []string{"id", "name"} + +// toolCallIdentity keeps a tool call's identity and drops its payload, the +// symmetric half of toolResultOutcome. +// +// The arguments travel as a JSON string inside the payload. They are decoded, +// reduced to the identity keys, and re-encoded; anything that does not decode as +// an object is removed outright rather than passed through as text, since text +// that could not be read is text that cannot be checked. +// +// REBUILT, NOT PATCHED, WHICH IS WHAT MAKES IT THE SYMMETRIC HALF. +// toolResultOutcome constructs its output from the two fields it keeps, so a +// field it has never heard of cannot reach a prompt through it. Editing +// arguments in place and returning the rest of the payload is the opposite +// contract: it drops what it recognises as a body and forwards every sibling +// key, so a producer recording one more top-level field would put that field +// into the next turn. The projection names what survives. +func toolCallIdentity(event Event) Event { + var decoded map[string]json.RawMessage + if err := json.Unmarshal(event.Payload, &decoded); err != nil { + event.Payload = json.RawMessage(`{}`) + return event + } + projected := map[string]json.RawMessage{} + for _, field := range toolCallIdentityFields { + if value, present := decoded[field]; present { + projected[field] = value + } + } + // The tool decides what some argument names mean, so the projection has to + // know which tool this was before it can say which keys are identity. + toolName := "" + if raw, present := decoded["name"]; present { + _ = json.Unmarshal(raw, &toolName) + } + kept := map[string]any{} + if raw, present := decoded["arguments"]; present { + var argumentsText string + if err := json.Unmarshal(raw, &argumentsText); err == nil { + var arguments map[string]any + if err := json.Unmarshal([]byte(argumentsText), &arguments); err == nil { + for key, value := range arguments { + if !toolCallIdentityKeyAllowed(toolName, key) { + continue + } + if retained, ok := retainedIdentityValue(value); ok { + kept[key] = retained + } + } + } + } + } + if len(kept) > 0 { + if reduced, err := json.Marshal(kept); err == nil { + if quoted, err := json.Marshal(string(reduced)); err == nil { + projected["arguments"] = quoted + } + } + } + rebuilt, err := json.Marshal(projected) + if err != nil { + event.Payload = json.RawMessage(`{}`) + return event + } + event.Payload = json.RawMessage(rebuilt) + return event +} + +// toolResultOutcome strips a tool result down to WHICH tool ran and HOW IT +// ENDED, dropping the output body. +// +// The tail exists so a resumed turn knows what the interrupted one did, and the +// CALL already carries that: the tool name and its arguments, which is the path +// for a read. The result adds only a 500-byte prefix of the output, which is +// worth little beside the call and is the one part of an event that can carry +// file contents. Nothing redacts on the way into a prompt, so keeping it would +// re-emit raw tool output into a later turn on the strength of a truncation +// limit alone. +// +// The status stays, because dropping it would be worse than dropping the whole +// result: a bare call reads as work that succeeded, so a failed read would come +// back as a file the next turn believes it already has. +func toolResultOutcome(event Event) Event { + var decoded struct { + Name string `json:"name"` + Status string `json:"status"` + } + if err := json.Unmarshal(event.Payload, &decoded); err != nil { + // Undecodable: drop the payload rather than pass an unknown shape through. + event.Payload = json.RawMessage(`{}`) + return event + } + trimmed, err := json.Marshal(map[string]string{ + "name": decoded.Name, + "status": decoded.Status, + }) + if err != nil { + event.Payload = json.RawMessage(`{}`) + return event + } + event.Payload = json.RawMessage(trimmed) + return event +} + +// payloadRole reads the "role" field of a message payload, returning "" when the +// payload is not a decodable object or carries no role. +func payloadRole(payload json.RawMessage) string { + if len(payload) == 0 { + return "" + } + var decoded struct { + Role string `json:"role"` + } + if err := json.Unmarshal(payload, &decoded); err != nil { + return "" } - return filtered + return strings.TrimSpace(decoded.Role) } func forkTitle(title string) string { diff --git a/internal/sessions/store_test.go b/internal/sessions/store_test.go index 685cb52dc..ee83f57e6 100644 --- a/internal/sessions/store_test.go +++ b/internal/sessions/store_test.go @@ -610,8 +610,60 @@ func TestFormatExecPromptKeepsConversationMessagesWhenNoisyEventsFollow(t *testi t.Fatalf("expected prompt to contain %q, got %q", want, prompt) } } + // The tool results here follow the last assistant answer, so they are work + // nothing has spoken for yet. They are carried on purpose: that is the whole + // of what an interrupted turn leaves behind (#913). The property this test + // was written for (#460) is that CONVERSATION survives a noisy turn, and the + // assertions above still hold it. + // The tool results here follow the last assistant answer, so they are work + // nothing has spoken for yet, and an interrupted turn leaves exactly this + // behind (#913). Their IDENTITY is carried; their OUTPUT is not, because the + // call beside them already names what was touched and the body is the part + // that can carry file contents into a later prompt. + if !strings.Contains(prompt, "tool_result: read_file") { + t.Fatalf("expected unanswered tool work to be named, got %q", prompt) + } if strings.Contains(prompt, "noisy tool result") { - t.Fatalf("expected prompt to omit noisy non-conversation events, got %q", prompt) + t.Fatalf("tool output body reached the prompt, got %q", prompt) + } + if count := strings.Count(prompt, "tool_result:"); count > 24 { + t.Fatalf("carried %d tool events, over the tail allowance of 24: %q", count, prompt) + } +} + +// AND WORK THE ASSISTANT ALREADY SPOKE FOR STAYS FILTERED. +// +// The counterpart to the case above, and what keeps that one from being +// satisfied by carrying every tool event ever recorded. Once an answer describes +// the work, repeating the raw results adds length and no information. +func TestFormatExecPromptOmitsToolWorkAnAnswerAlreadyCovers(t *testing.T) { + events := []Event{ + {Sequence: 1, Type: EventMessage, Payload: json.RawMessage(`{"role":"user","content":"first user request"}`)}, + } + for sequence := 2; sequence <= 20; sequence++ { + events = append(events, Event{Sequence: sequence, Type: EventToolResult, Payload: json.RawMessage(`{"name":"read_file","output":"already summarized tool result"}`)}) + } + events = append(events, + Event{Sequence: 21, Type: EventMessage, Payload: json.RawMessage(`{"role":"assistant","content":"I read the files and here is the summary"}`)}, + Event{Sequence: 22, Type: EventMessage, Payload: json.RawMessage(`{"role":"user","content":"latest user request"}`)}, + ) + + prompt := FormatExecPrompt("continue", PreparedExec{ + Mode: ModeResume, + Session: Metadata{SessionID: "session-already-answered"}, + ContextEvents: events, + }) + + if strings.Contains(prompt, "already summarized tool result") { + t.Fatalf("tool work the assistant already described was repeated verbatim, got %q", prompt) + } + if strings.Contains(prompt, "tool_result:") { + t.Fatalf("an answered turn still listed its tool events, got %q", prompt) + } + for _, want := range []string{"first user request", "here is the summary", "latest user request"} { + if !strings.Contains(prompt, want) { + t.Fatalf("expected prompt to contain %q, got %q", want, prompt) + } } }