Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions internal/context/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package context

import (
"context"
"strings"

providertypes "neo-code/internal/provider/types"
"neo-code/internal/tools"
)

// DefaultBuilder preserves the current runtime context-building behavior.
Expand Down Expand Up @@ -64,8 +66,31 @@ func (b *DefaultBuilder) Build(ctx context.Context, input BuildInput) (BuildResu

// applyReadTimeContextProjection 负责在 provider 请求前按开关应用只读上下文投影,避免改写原始会话消息。
func applyReadTimeContextProjection(messages []providertypes.Message, options CompactOptions, policies MicroCompactPolicySource) []providertypes.Message {
var projected []providertypes.Message
if options.DisableMicroCompact {
return cloneContextMessages(messages)
projected = cloneContextMessages(messages)
} else {
projected = microCompactMessagesWithPolicies(messages, policies)
}
return microCompactMessagesWithPolicies(messages, policies)
return projectToolMessagesForModel(projected)
}

// projectToolMessagesForModel 仅在 provider 读取路径上格式化 tool 消息,避免污染持久化会话内容。
func projectToolMessagesForModel(messages []providertypes.Message) []providertypes.Message {
for i := range messages {
message := messages[i]
if message.Role != providertypes.RoleTool {
continue
}
if len(message.ToolMetadata) == 0 {
continue
}
content := strings.TrimSpace(message.Content)
if content == "" || content == microCompactClearedMessage {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a tool succeeds with empty Content but meaningful ToolMetadata, this branch skips projection entirely. The provider adapters do not serialize ToolMetadata, so the next model round receives an empty tool message and loses the result semantics completely. That is reachable for generic ToolManager/adapter implementations even if most built-in tools currently return "ok". Please either project metadata-only tool results as well, or normalize successful empty-content results before they are persisted.

continue
}
messages[i].Content = tools.FormatToolMessageForModel(message)
messages[i].ToolMetadata = nil
}
return messages
}
35 changes: 35 additions & 0 deletions internal/context/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -630,3 +630,38 @@ func TestBuildShouldAutoCompactAboveThreshold(t *testing.T) {
t.Fatalf("expected ShouldAutoCompact true when tokens above threshold")
}
}

func TestApplyReadTimeContextProjectionFormatsToolMessagesWithoutMutatingSessionState(t *testing.T) {
t.Parallel()

original := []providertypes.Message{
{Role: providertypes.RoleUser, Content: "edit this"},
{
Role: providertypes.RoleTool,
ToolCallID: "call-1",
Content: "ok",
ToolMetadata: map[string]string{
"tool_name": "filesystem_edit",
"path": "main.go",
"search_length": "12",
},
},
}

got := applyReadTimeContextProjection(original, CompactOptions{}, nil)
if len(got) != len(original) {
t.Fatalf("expected projected messages to keep length, got %d", len(got))
}
if !strings.Contains(got[1].Content, "tool result") || !strings.Contains(got[1].Content, "tool: filesystem_edit") {
t.Fatalf("expected tool message to be projected for model, got %q", got[1].Content)
}
if got[1].ToolMetadata != nil {
t.Fatalf("expected projected provider message to clear tool metadata, got %+v", got[1].ToolMetadata)
}
if original[1].Content != "ok" {
t.Fatalf("expected original session message to keep raw content, got %q", original[1].Content)
}
if original[1].ToolMetadata["path"] != "main.go" {
t.Fatalf("expected original tool metadata to remain intact, got %+v", original[1].ToolMetadata)
}
}
6 changes: 6 additions & 0 deletions internal/context/compact/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ func cloneMessages(messages []providertypes.Message) []providertypes.Message {
for _, message := range messages {
next := message
next.ToolCalls = append([]providertypes.ToolCall(nil), message.ToolCalls...)
if len(message.ToolMetadata) > 0 {
next.ToolMetadata = make(map[string]string, len(message.ToolMetadata))
for key, value := range message.ToolMetadata {
next.ToolMetadata[key] = value
}
}
out = append(out, next)
}
return out
Expand Down
6 changes: 6 additions & 0 deletions internal/context/microcompact.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ func cloneContextMessages(messages []providertypes.Message) []providertypes.Mess
for _, message := range messages {
next := message
next.ToolCalls = append([]providertypes.ToolCall(nil), message.ToolCalls...)
if len(message.ToolMetadata) > 0 {
next.ToolMetadata = make(map[string]string, len(message.ToolMetadata))
for key, value := range message.ToolMetadata {
next.ToolMetadata[key] = value
}
}
cloned = append(cloned, next)
}
return cloned
Expand Down
11 changes: 10 additions & 1 deletion internal/context/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,24 @@ var defaultPromptSections = []promptSection{
},
{
title: "Tool Usage",
content: "- Use tools when they reduce uncertainty or are required to complete the task safely.\n" +
content: "- Use the minimum set of tools needed to make progress or verify a result safely.\n" +
"- Only call tools that are actually exposed in the current tool schema. Do not invent tool names.\n" +
"- Prefer structured workspace tools over `bash` whenever possible: use `filesystem_read_file`, `filesystem_grep`, and `filesystem_glob` for reading/search, `filesystem_edit` for precise edits, and `filesystem_write_file` only for new files or full rewrites.\n" +
"- Do not use `bash` to edit files when the filesystem tools can make the change safely.\n" +
"- When using `bash`, avoid interactive or blocking commands and pass non-interactive flags when they are available.\n" +
"- For risky operations, call the relevant tool first and let the runtime permission layer decide ask/allow/deny.\n" +
"- Do not self-reject a user-requested operation before attempting the proper tool call and permission flow.\n" +
"- Read tool results carefully before acting. Treat `status`, `truncated`, `tool_call_id`, `meta.*`, and `content` as the authoritative outcome of that call.\n" +
"- Do not repeat the same tool call with identical arguments unless the workspace changed or the prior result was errored, truncated, or clearly incomplete.\n" +
"- After a successful write or edit, do at most one focused verification call; if that verifies the change, stop calling tools and respond.\n" +
"- If a successful tool result already answers the question or confirms completion, stop using tools and give the user the result.\n" +
"- Stay within the current workspace unless the user clearly asks for something else.\n" +
"- Do not claim work is done unless the needed files, commands, or verification actually succeeded.",
},
{
title: "Failure Recovery",
content: "- If blocked, identify the concrete blocker and try the next reasonable path before giving up.\n" +
"- When retrying, change something concrete: use different arguments, a different tool, or explain why further tool calls would not help.\n" +
"- Surface risky assumptions, partial progress, or missing verification instead of hiding them.\n" +
"- When constraints prevent completion, return the best safe result and explain what remains.",
},
Expand Down
37 changes: 35 additions & 2 deletions internal/context/prompt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,18 @@ func TestComposeSystemPromptSkipsEmptySections(t *testing.T) {
}
}

func TestDefaultToolUsagePromptEncouragesAskFlow(t *testing.T) {
func TestDefaultToolUsagePromptIncludesPermissionAndAntiLoopGuidance(t *testing.T) {
t.Parallel()

sections := defaultSystemPromptSections()
var toolUsage string
var failureRecovery string
for _, section := range sections {
if section.title == "Tool Usage" {
toolUsage = section.content
break
}
if section.title == "Failure Recovery" {
failureRecovery = section.content
}
}
if toolUsage == "" {
Expand All @@ -111,7 +114,37 @@ func TestDefaultToolUsagePromptEncouragesAskFlow(t *testing.T) {
if !strings.Contains(toolUsage, "permission layer") {
t.Fatalf("expected Tool Usage to mention permission layer, got %q", toolUsage)
}
if !strings.Contains(toolUsage, "Do not invent tool names") {
t.Fatalf("expected Tool Usage to forbid invented tool names, got %q", toolUsage)
}
if !strings.Contains(toolUsage, "`filesystem_read_file`, `filesystem_grep`, and `filesystem_glob`") {
t.Fatalf("expected Tool Usage to prefer structured read/search tools, got %q", toolUsage)
}
if !strings.Contains(toolUsage, "`filesystem_edit` for precise edits") {
t.Fatalf("expected Tool Usage to describe edit tool preference, got %q", toolUsage)
}
if !strings.Contains(toolUsage, "Do not use `bash` to edit files") {
t.Fatalf("expected Tool Usage to discourage bash file edits, got %q", toolUsage)
}
if !strings.Contains(toolUsage, "avoid interactive or blocking commands") {
t.Fatalf("expected Tool Usage to constrain bash usage, got %q", toolUsage)
}
if !strings.Contains(toolUsage, "Do not self-reject") {
t.Fatalf("expected Tool Usage to discourage self-reject, got %q", toolUsage)
}
if !strings.Contains(toolUsage, "Do not repeat the same tool call with identical arguments") {
t.Fatalf("expected Tool Usage to include anti-loop guidance, got %q", toolUsage)
}
if !strings.Contains(toolUsage, "focused verification call") {
t.Fatalf("expected Tool Usage to limit write verification retries, got %q", toolUsage)
}
if !strings.Contains(toolUsage, "stop using tools and give the user the result") {
t.Fatalf("expected Tool Usage to tell the agent when to stop, got %q", toolUsage)
}
if !strings.Contains(toolUsage, "`status`, `truncated`, `tool_call_id`, `meta.*`, and `content`") {
t.Fatalf("expected Tool Usage to explain structured tool results, got %q", toolUsage)
}
if !strings.Contains(failureRecovery, "change something concrete") {
t.Fatalf("expected Failure Recovery to discourage identical retries, got %q", failureRecovery)
}
}
11 changes: 6 additions & 5 deletions internal/provider/types/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ const RoleTool = "tool"

// Message 表示对话中的单条消息。
type Message struct {
Role string `json:"role"`
Content string `json:"content"`
ToolCalls []ToolCall `json:"tool_calls,omitempty"`
ToolCallID string `json:"tool_call_id,omitempty"`
IsError bool `json:"is_error,omitempty"`
Role string `json:"role"`
Content string `json:"content"`
ToolCalls []ToolCall `json:"tool_calls,omitempty"`
ToolCallID string `json:"tool_call_id,omitempty"`
IsError bool `json:"is_error,omitempty"`
ToolMetadata map[string]string `json:"tool_metadata,omitempty"`
}

// ToolCall 表示模型发起的工具调用请求。
Expand Down
13 changes: 7 additions & 6 deletions internal/provider/types/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,15 @@ func TestMessageStructFields(t *testing.T) {
t.Parallel()

msg := Message{
Role: RoleUser,
Content: "hello",
ToolCalls: []ToolCall{{ID: "t1"}},
ToolCallID: "tc_1",
IsError: true,
Role: RoleUser,
Content: "hello",
ToolCalls: []ToolCall{{ID: "t1"}},
ToolCallID: "tc_1",
IsError: true,
ToolMetadata: map[string]string{"path": "main.go"},
}
if msg.Role != RoleUser || msg.Content != "hello" || len(msg.ToolCalls) != 1 ||
msg.ToolCallID != "tc_1" || !msg.IsError {
msg.ToolCallID != "tc_1" || !msg.IsError || msg.ToolMetadata["path"] != "main.go" {
t.Fatalf("message fields not as expected: %+v", msg)
}
}
Expand Down
9 changes: 5 additions & 4 deletions internal/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,11 @@ func (s *Service) Run(ctx context.Context, input UserInput) error {
}

toolMessage := providertypes.Message{
Role: providertypes.RoleTool,
Content: result.Content,
ToolCallID: call.ID,
IsError: result.IsError,
Role: providertypes.RoleTool,
Content: result.Content,
ToolCallID: call.ID,
IsError: result.IsError,
ToolMetadata: tools.SanitizeToolMetadata(result.Name, result.Metadata),
}
session.Messages = append(session.Messages, toolMessage)
session.UpdatedAt = time.Now()
Expand Down
50 changes: 46 additions & 4 deletions internal/runtime/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,14 @@ func TestServiceRun(t *testing.T) {
name: "filesystem_edit",
content: "tool output",
},
contextBuilder: &stubContextBuilder{
buildFn: func(ctx context.Context, input agentcontext.BuildInput) (agentcontext.BuildResult, error) {
return agentcontext.BuildResult{
SystemPrompt: "stub system prompt",
Messages: projectToolMessagesForProviderTest(input.Messages),
}, nil
},
},
expectProviderCalls: 2,
expectToolCalls: 1,
expectMessageRoles: []string{"user", "assistant", "tool", "assistant"},
Expand All @@ -401,14 +409,27 @@ func TestServiceRun(t *testing.T) {
second := scripted.requests[1]
foundToolResult := false
for _, message := range second.Messages {
if message.Role == "tool" && message.ToolCallID == "call-1" && message.Content == "tool output" {
if message.Role == "tool" &&
message.ToolCallID == "call-1" &&
strings.Contains(message.Content, "tool result") &&
strings.Contains(message.Content, "tool: filesystem_edit") &&
strings.Contains(message.Content, "status: ok") &&
strings.Contains(message.Content, "content:\ntool output") {
foundToolResult = true
break
}
}
if !foundToolResult {
t.Fatalf("expected tool result message in second provider request: %+v", second.Messages)
}

session := onlySession(t, store)
if session.Messages[2].Role != providertypes.RoleTool || session.Messages[2].Content != "tool output" {
t.Fatalf("expected persisted tool message to keep raw content, got %+v", session.Messages[2])
}
if session.Messages[2].ToolMetadata["tool_name"] != "filesystem_edit" {
t.Fatalf("expected persisted tool metadata to keep tool name, got %+v", session.Messages[2].ToolMetadata)
}
},
},
}
Expand Down Expand Up @@ -901,7 +922,7 @@ func TestServiceRunDefaultBuilderUsesGenericToolManagerMicroCompactPolicies(t *t
}},
}

service := NewWithFactory(manager, toolManager, store, &scriptedProviderFactory{provider: scripted}, nil)
service := NewWithFactory(manager, toolManager, store, &scriptedProviderFactory{provider: scripted}, &stubContextBuilder{})

This comment was marked as outdated.

This comment was marked as outdated.

if err := service.Run(context.Background(), UserInput{
SessionID: session.ID,
RunID: "run-preserve-history-generic-manager",
Expand Down Expand Up @@ -986,6 +1007,9 @@ func TestServiceRunUsesToolManager(t *testing.T) {
result: tools.ToolResult{
Name: "filesystem_edit",
Content: "tool manager output",
Metadata: map[string]any{
"path": "main.go",
},
},
}

Expand All @@ -999,7 +1023,7 @@ func TestServiceRunUsesToolManager(t *testing.T) {
},
}

service := NewWithFactory(manager, toolManager, store, &scriptedProviderFactory{provider: scripted}, nil)
service := NewWithFactory(manager, toolManager, store, &scriptedProviderFactory{provider: scripted}, &stubContextBuilder{})
if err := service.Run(context.Background(), UserInput{RunID: "run-tool-manager", Content: "edit file"}); err != nil {
t.Fatalf("Run() error = %v", err)
}
Expand All @@ -1020,7 +1044,10 @@ func TestServiceRunUsesToolManager(t *testing.T) {
session := onlySession(t, store)
foundToolMessage := false
for _, message := range session.Messages {
if message.Role == providertypes.RoleTool && message.Content == "tool manager output" {
if message.Role == providertypes.RoleTool &&
message.Content == "tool manager output" &&
message.ToolMetadata["tool_name"] == "filesystem_edit" &&
message.ToolMetadata["path"] == "main.go" {
foundToolMessage = true
break
}
Expand Down Expand Up @@ -2695,6 +2722,21 @@ func cloneBuildInput(input agentcontext.BuildInput) agentcontext.BuildInput {
return cloned
}

// projectToolMessagesForProviderTest 模拟 context 层在 provider 请求前对 tool 消息做的只读投影。
func projectToolMessagesForProviderTest(messages []providertypes.Message) []providertypes.Message {
projected := append([]providertypes.Message(nil), messages...)
for i, message := range projected {
if message.Role != providertypes.RoleTool || len(message.ToolMetadata) == 0 {
continue
}
next := message
next.Content = tools.FormatToolMessageForModel(message)
next.ToolMetadata = nil
projected[i] = next
}
return projected
}

func containsError(err error, target string) bool {
return err != nil && strings.Contains(err.Error(), target)
}
Expand Down
18 changes: 17 additions & 1 deletion internal/session/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,15 @@ func TestJSONStoreSavePersistsProviderModelAndMessages(t *testing.T) {
{ID: "call-1", Name: "webfetch", Arguments: `{"url":"https://example.com"}`},
},
},
{Role: providertypes.RoleTool, ToolCallID: "call-1", Content: "ok"},
{
Role: providertypes.RoleTool,
ToolCallID: "call-1",
Content: "ok",
ToolMetadata: map[string]string{
"tool_name": "webfetch",
"http_status": "200",
},
},
},
}

Expand Down Expand Up @@ -566,6 +574,14 @@ func TestJSONStoreSavePersistsProviderModelAndMessages(t *testing.T) {
if decoded["workdir"] != session.Workdir {
t.Fatalf("expected workdir persisted as %q, got %+v", session.Workdir, decoded["workdir"])
}

loaded, err := store.Load(context.Background(), session.ID)
if err != nil {
t.Fatalf("load saved session: %v", err)
}
if loaded.Messages[2].ToolMetadata["tool_name"] != "webfetch" || loaded.Messages[2].ToolMetadata["http_status"] != "200" {
t.Fatalf("expected tool metadata round-trip, got %+v", loaded.Messages[2].ToolMetadata)
}
}

func mustWriteSessionFile(t *testing.T, path string, content string) {
Expand Down
Loading
Loading