diff --git a/internal/context/builder_test.go b/internal/context/builder_test.go index efe32e55..b17921e7 100644 --- a/internal/context/builder_test.go +++ b/internal/context/builder_test.go @@ -777,3 +777,45 @@ func TestProjectToolMessagesForModelKeepsBuilderProjectionBehavior(t *testing.T) t.Fatal("expected source messages to remain unchanged") } } + +func TestDefaultBuilderBuildProjectsMetadataOnlyToolResult(t *testing.T) { + t.Parallel() + + builder := NewBuilder() + result, err := builder.Build(stdcontext.Background(), BuildInput{ + Messages: []providertypes.Message{ + {Role: providertypes.RoleUser, Content: "inspect README"}, + { + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-1", Name: "filesystem_read_file", Arguments: `{"path":"README.md"}`}, + }, + }, + { + Role: providertypes.RoleTool, + ToolCallID: "call-1", + Content: " ", + ToolMetadata: map[string]string{"tool_name": "filesystem_read_file", "path": "README.md"}, + }, + }, + Metadata: testMetadata(t.TempDir()), + }) + if err != nil { + t.Fatalf("Build() error = %v", err) + } + if len(result.Messages) != 3 { + t.Fatalf("len(result.Messages) = %d, want 3", len(result.Messages)) + } + toolMessage := result.Messages[2] + if toolMessage.Role != providertypes.RoleTool { + t.Fatalf("expected tool message at index 2, got %+v", toolMessage) + } + if !strings.Contains(toolMessage.Content, "tool result") || + !strings.Contains(toolMessage.Content, "tool: filesystem_read_file") || + !strings.Contains(toolMessage.Content, "meta.path: README.md") { + t.Fatalf("expected metadata-only tool result to be projected, got %q", toolMessage.Content) + } + if toolMessage.ToolMetadata != nil { + t.Fatalf("expected projected tool metadata to be cleared, got %#v", toolMessage.ToolMetadata) + } +} diff --git a/internal/context/projection.go b/internal/context/projection.go index 86c5a6dd..85e7ecd0 100644 --- a/internal/context/projection.go +++ b/internal/context/projection.go @@ -145,7 +145,10 @@ func isInjectableToolMessage(message providertypes.Message) bool { return false } content := strings.TrimSpace(message.Content) - return content != "" && content != microCompactClearedMessage + if content == microCompactClearedMessage { + return false + } + return content != "" || len(message.ToolMetadata) > 0 } // recentWindowMessageBudget 计算 recent window 可保留的消息总数硬上限，避免窗口体积失控。 diff --git a/internal/context/projection_test.go b/internal/context/projection_test.go index 84116afa..5b8f428a 100644 --- a/internal/context/projection_test.go +++ b/internal/context/projection_test.go @@ -43,10 +43,10 @@ func TestProjectToolMessagesForModelSkipsMessagesThatCannotBeProjected(t *testin t.Fatalf("non-tool message should remain unchanged, got %+v", projected[0]) } if projected[1].Content != "tool output" || projected[1].ToolMetadata != nil { - t.Fatalf("tool without metadata-free projection should remain unchanged, got %+v", projected[1]) + t.Fatalf("tool without projection metadata should remain unchanged, got %+v", projected[1]) } - if projected[2].Content != " " || projected[2].ToolMetadata == nil { - t.Fatalf("empty tool content should not be projected, got %+v", projected[2]) + if !strings.Contains(projected[2].Content, "tool result") || projected[2].ToolMetadata != nil { + t.Fatalf("metadata-only tool message should be projected, got %+v", projected[2]) } if projected[3].Content != microCompactClearedMessage || projected[3].ToolMetadata == nil { t.Fatalf("cleared tool content should not be projected, got %+v", projected[3]) @@ -221,7 +221,7 @@ func TestMatchedToolCallSpanRejectsInvalidAssistantStates(t *testing.T) { } } -func TestMatchedToolCallSpanRequiresInjectableResponsesAndSkipsDuplicates(t *testing.T) { +func TestMatchedToolCallSpanRequiresProjectableResponsesAndSkipsDuplicates(t *testing.T) { t.Parallel() messages := []providertypes.Message{ @@ -244,11 +244,35 @@ func TestMatchedToolCallSpanRequiresInjectableResponsesAndSkipsDuplicates(t *tes if len(span) != 3 { t.Fatalf("len(span) = %d, want 3 (%+v)", len(span), span) } - if span[0] != 0 || span[1] != 2 || span[2] != 5 { + if span[0] != 0 || span[1] != 1 || span[2] != 5 { t.Fatalf("unexpected span indexes %+v", span) } } +func TestMatchedToolCallSpanAcceptsMetadataOnlyResponses(t *testing.T) { + t.Parallel() + + messages := []providertypes.Message{ + { + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-1", Name: "webfetch", Arguments: `{}`}, + }, + }, + { + Role: providertypes.RoleTool, + ToolCallID: "call-1", + Content: " ", + ToolMetadata: map[string]string{"tool_name": "webfetch", "status_code": "200"}, + }, + } + + span := matchedToolCallSpan(messages, 0) + if len(span) != 2 || span[0] != 0 || span[1] != 1 { + t.Fatalf("unexpected metadata-only span %+v", span) + } +} + func TestMatchedToolCallSpanRejectsResponsesWithoutProjectionMetadata(t *testing.T) { t.Parallel() @@ -285,6 +309,11 @@ func TestIsInjectableToolMessage(t *testing.T) { message: providertypes.Message{Role: providertypes.RoleTool, Content: " "}, want: false, }, + { + name: "metadata-only", + message: providertypes.Message{Role: providertypes.RoleTool, Content: " ", ToolMetadata: map[string]string{"tool_name": "bash"}}, + want: true, + }, { name: "cleared", message: providertypes.Message{Role: providertypes.RoleTool, Content: microCompactClearedMessage}, diff --git a/internal/memo/llm_extractor_test.go b/internal/memo/llm_extractor_test.go index 7d1befea..10bc7864 100644 --- a/internal/memo/llm_extractor_test.go +++ b/internal/memo/llm_extractor_test.go @@ -299,6 +299,48 @@ func TestLLMExtractorExtractKeepsProjectedToolCallSpan(t *testing.T) { } } +func TestLLMExtractorExtractKeepsMetadataOnlyToolCallSpan(t *testing.T) { + generator := &stubTextGenerator{response: `[]`} + extractor := NewLLMExtractor(generator) + + _, err := extractor.Extract(context.Background(), []providertypes.Message{ + {Role: providertypes.RoleUser, Content: "remember this"}, + { + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call_1", Name: "filesystem_read_file", Arguments: `{"path":"README.md"}`}, + }, + }, + { + Role: providertypes.RoleTool, + ToolCallID: "call_1", + Content: "", + ToolMetadata: map[string]string{"tool_name": "filesystem_read_file", "path": "README.md"}, + }, + }) + if err != nil { + t.Fatalf("Extract() error = %v", err) + } + if len(generator.messages) != 3 { + t.Fatalf("len(generator.messages) = %d, want 3", len(generator.messages)) + } + toolMessage := generator.messages[2] + if toolMessage.Role != providertypes.RoleTool { + t.Fatalf("expected projected tool message, got %#v", toolMessage) + } + if !strings.Contains(toolMessage.Content, "tool result") || + !strings.Contains(toolMessage.Content, "tool: filesystem_read_file") || + !strings.Contains(toolMessage.Content, "meta.path: README.md") { + t.Fatalf("expected metadata-only tool text, got %q", toolMessage.Content) + } + if strings.Contains(toolMessage.Content, "content:\n") { + t.Fatalf("expected metadata-only projection to omit content section, got %q", toolMessage.Content) + } + if toolMessage.ToolMetadata != nil { + t.Fatalf("expected projected tool metadata to be cleared, got %#v", toolMessage.ToolMetadata) + } +} + func TestLLMExtractorExtractSkipsOrphanAndClearedToolMessages(t *testing.T) { generator := &stubTextGenerator{response: `[]`} extractor := NewLLMExtractor(generator) diff --git a/internal/runtime/runtime_internal_helpers_test.go b/internal/runtime/runtime_internal_helpers_test.go index 91e86a91..46ac92f4 100644 --- a/internal/runtime/runtime_internal_helpers_test.go +++ b/internal/runtime/runtime_internal_helpers_test.go @@ -177,6 +177,120 @@ func TestAppendToolMessageAndSaveSanitizesMetadata(t *testing.T) { } } +func TestAppendToolMessageAndSavePreservesMetadataOnlySuccessResult(t *testing.T) { + t.Parallel() + + store := newMemoryStore() + session := newRuntimeSession("session-append-tool-metadata-only") + store.sessions[session.ID] = cloneSession(session) + + service := &Service{sessionStore: store} + state := newRunState("run-append-tool-metadata-only", session) + call := providertypes.ToolCall{ID: "call-1", Name: "filesystem_read_file"} + result := tools.ToolResult{ + Name: "filesystem_read_file", + Content: "", + Metadata: map[string]any{ + "path": "README.md", + }, + } + + if err := service.appendToolMessageAndSave(context.Background(), &state, call, result); err != nil { + t.Fatalf("appendToolMessageAndSave() error = %v", err) + } + + msg := state.session.Messages[0] + if msg.Content != "" { + t.Fatalf("expected metadata-only success result to keep empty content, got %q", msg.Content) + } + if msg.ToolMetadata["tool_name"] != "filesystem_read_file" || msg.ToolMetadata["path"] != "README.md" { + t.Fatalf("expected metadata-only success result to keep sanitized metadata, got %+v", msg.ToolMetadata) + } +} + +func TestAppendToolMessageAndSaveNormalizesSemanticallyEmptySuccessResult(t *testing.T) { + t.Parallel() + + store := newMemoryStore() + session := newRuntimeSession("session-append-tool-empty-success") + store.sessions[session.ID] = cloneSession(session) + + service := &Service{sessionStore: store} + state := newRunState("run-append-tool-empty-success", session) + call := providertypes.ToolCall{ID: "call-1", Name: "filesystem_read_file"} + result := tools.ToolResult{ + Name: "filesystem_read_file", + Content: " ", + } + + if err := service.appendToolMessageAndSave(context.Background(), &state, call, result); err != nil { + t.Fatalf("appendToolMessageAndSave() error = %v", err) + } + + msg := state.session.Messages[0] + if msg.Content != "ok" { + t.Fatalf("expected empty success result to be normalized to ok, got %q", msg.Content) + } + if msg.ToolMetadata["tool_name"] != "filesystem_read_file" { + t.Fatalf("expected tool_name metadata to be preserved after normalization, got %+v", msg.ToolMetadata) + } +} + +func TestAppendToolMessageAndSaveNormalizesToolNameOnlyMetadataSuccessResult(t *testing.T) { + t.Parallel() + + store := newMemoryStore() + session := newRuntimeSession("session-append-tool-name-only-metadata-success") + store.sessions[session.ID] = cloneSession(session) + + service := &Service{sessionStore: store} + state := newRunState("run-append-tool-name-only-metadata-success", session) + call := providertypes.ToolCall{ID: "call-1", Name: "filesystem_read_file"} + result := tools.ToolResult{ + Name: "filesystem_read_file", + Content: " ", + Metadata: map[string]any{ + "unsupported_key": "ignored", + }, + } + + if err := service.appendToolMessageAndSave(context.Background(), &state, call, result); err != nil { + t.Fatalf("appendToolMessageAndSave() error = %v", err) + } + + msg := state.session.Messages[0] + if msg.Content != "ok" { + t.Fatalf("expected tool_name-only metadata success to normalize content to ok, got %q", msg.Content) + } + if len(msg.ToolMetadata) != 1 || msg.ToolMetadata["tool_name"] != "filesystem_read_file" { + t.Fatalf("expected only tool_name metadata to remain, got %+v", msg.ToolMetadata) + } +} + +func TestAppendToolMessageAndSaveFallsBackToCallNameForToolMetadata(t *testing.T) { + t.Parallel() + + store := newMemoryStore() + session := newRuntimeSession("session-append-tool-name-fallback") + store.sessions[session.ID] = cloneSession(session) + + service := &Service{sessionStore: store} + state := newRunState("run-append-tool-name-fallback", session) + call := providertypes.ToolCall{ID: "call-1", Name: "filesystem_read_file"} + result := tools.ToolResult{ + Content: "ok", + } + + if err := service.appendToolMessageAndSave(context.Background(), &state, call, result); err != nil { + t.Fatalf("appendToolMessageAndSave() error = %v", err) + } + + msg := state.session.Messages[0] + if msg.ToolMetadata["tool_name"] != "filesystem_read_file" { + t.Fatalf("expected tool_name fallback from call name, got %+v", msg.ToolMetadata) + } +} + func TestAppendToolMessageAndSaveUnlocksStateBeforePersist(t *testing.T) { t.Parallel() diff --git a/internal/runtime/runtime_test.go b/internal/runtime/runtime_test.go index 7de08a4f..43c43114 100644 --- a/internal/runtime/runtime_test.go +++ b/internal/runtime/runtime_test.go @@ -545,6 +545,76 @@ func TestServiceRun(t *testing.T) { } }, }, + { + name: "metadata-only tool result is projected on follow-up provider round", + input: UserInput{RunID: "run-tool-metadata-only", Content: "inspect file"}, + providerStreams: [][]providertypes.StreamEvent{ + { + providertypes.NewToolCallStartStreamEvent(0, "call-1", "filesystem_read_file"), + providertypes.NewToolCallDeltaStreamEvent(0, "call-1", `{"path":"README.md"}`), + }, + { + providertypes.NewTextDeltaStreamEvent("done"), + }, + }, + registerTool: &stubTool{ + name: "filesystem_read_file", + executeFn: func(ctx context.Context, input tools.ToolCallInput) (tools.ToolResult, error) { + return tools.ToolResult{ + Name: "filesystem_read_file", + Content: "", + Metadata: map[string]any{ + "path": "README.md", + }, + }, nil + }, + }, + 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"}, + expectEventTypes: []EventType{EventUserMessage, EventToolStart, EventToolResult, EventAgentDone}, + assert: func(t *testing.T, store *memoryStore, scripted *scriptedProvider, tool *stubTool) { + t.Helper() + if len(scripted.requests) != 2 { + t.Fatalf("expected 2 provider requests, got %d", len(scripted.requests)) + } + second := scripted.requests[1] + foundToolResult := false + for _, message := range second.Messages { + if message.Role == providertypes.RoleTool && + message.ToolCallID == "call-1" && + strings.Contains(message.Content, "tool result") && + strings.Contains(message.Content, "tool: filesystem_read_file") && + strings.Contains(message.Content, "meta.path: README.md") { + foundToolResult = true + if strings.Contains(message.Content, "content:\n") { + t.Fatalf("expected metadata-only projection to omit content section, got %q", message.Content) + } + break + } + } + if !foundToolResult { + t.Fatalf("expected projected metadata-only tool result in second provider request: %+v", second.Messages) + } + + session := onlySession(t, store) + if session.Messages[2].Role != providertypes.RoleTool || session.Messages[2].Content != "" { + t.Fatalf("expected persisted tool message to keep empty raw content, got %+v", session.Messages[2]) + } + if session.Messages[2].ToolMetadata["tool_name"] != "filesystem_read_file" || + session.Messages[2].ToolMetadata["path"] != "README.md" { + t.Fatalf("expected persisted metadata-only tool message to keep sanitized metadata, got %+v", session.Messages[2].ToolMetadata) + } + }, + }, } for _, tt := range tests { @@ -3031,17 +3101,7 @@ func cloneBuildInput(input agentcontext.BuildInput) agentcontext.BuildInput { // 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 + return agentcontext.ProjectToolMessagesForModel(cloneMessages(messages)) } func containsError(err error, target string) bool { diff --git a/internal/runtime/session_mutation.go b/internal/runtime/session_mutation.go index bf914917..81c5844b 100644 --- a/internal/runtime/session_mutation.go +++ b/internal/runtime/session_mutation.go @@ -9,6 +9,8 @@ import ( "neo-code/internal/tools" ) +const toolNameMetadataKey = "tool_name" + // appendUserMessageAndSave 将用户消息追加到会话并立即持久化。 func (s *Service) appendUserMessageAndSave(ctx context.Context, state *runState, content string) error { message := providertypes.Message{ @@ -55,13 +57,7 @@ func (s *Service) appendToolMessageAndSave( result tools.ToolResult, ) error { state.mu.Lock() - toolMessage := providertypes.Message{ - Role: providertypes.RoleTool, - Content: result.Content, - ToolCallID: call.ID, - IsError: result.IsError, - ToolMetadata: tools.SanitizeToolMetadata(result.Name, result.Metadata), - } + toolMessage := normalizeToolMessageForPersistence(call, result) state.session.Messages = append(state.session.Messages, toolMessage) state.touchSession() sessionSnapshot := cloneSessionForPersistence(state.session) @@ -69,6 +65,38 @@ func (s *Service) appendToolMessageAndSave( return s.sessionStore.Save(ctx, &sessionSnapshot) } +// normalizeToolMessageForPersistence 负责在写入会话前收敛工具结果，避免成功结果落成完全空语义消息。 +func normalizeToolMessageForPersistence(call providertypes.ToolCall, result tools.ToolResult) providertypes.Message { + toolName := strings.TrimSpace(result.Name) + if toolName == "" { + toolName = strings.TrimSpace(call.Name) + } + + sanitizedMetadata := tools.SanitizeToolMetadata(toolName, result.Metadata) + content := result.Content + if !result.IsError && strings.TrimSpace(content) == "" && !hasNonToolNameToolMetadata(sanitizedMetadata) { + content = "ok" + } + + return providertypes.Message{ + Role: providertypes.RoleTool, + Content: content, + ToolCallID: call.ID, + IsError: result.IsError, + ToolMetadata: sanitizedMetadata, + } +} + +// hasNonToolNameToolMetadata 判断 metadata 中是否存在除 tool_name 外的语义字段。 +func hasNonToolNameToolMetadata(metadata map[string]string) bool { + for key := range metadata { + if key != toolNameMetadataKey { + return true + } + } + return false +} + // cloneSessionForPersistence 复制会话快照，避免持久化阶段与并发写入共享可变切片/映射。 func cloneSessionForPersistence(session agentsession.Session) agentsession.Session { cloned := session diff --git a/internal/session/store_test.go b/internal/session/store_test.go index e08710b1..ec1c9834 100644 --- a/internal/session/store_test.go +++ b/internal/session/store_test.go @@ -643,6 +643,56 @@ func TestJSONStoreSavePersistsProviderModelAndMessages(t *testing.T) { } } +func TestJSONStoreSaveRoundTripsMetadataOnlyToolMessage(t *testing.T) { + t.Parallel() + + baseDir := t.TempDir() + workspaceRoot := t.TempDir() + store := NewJSONStore(baseDir, workspaceRoot) + + session := &Session{ + SchemaVersion: CurrentSchemaVersion, + ID: "metadata-only-tool-message", + Title: "Metadata Only Tool Message", + CreatedAt: time.Now().Add(-time.Hour), + UpdatedAt: time.Now(), + Messages: []providertypes.Message{ + {Role: providertypes.RoleUser, Content: "inspect"}, + { + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-1", Name: "filesystem_read_file", Arguments: `{"path":"README.md"}`}, + }, + }, + { + Role: providertypes.RoleTool, + ToolCallID: "call-1", + Content: "", + ToolMetadata: map[string]string{ + "tool_name": "filesystem_read_file", + "path": "README.md", + }, + }, + }, + } + + if err := store.Save(context.Background(), session); err != nil { + t.Fatalf("save session: %v", err) + } + + loaded, err := store.Load(context.Background(), session.ID) + if err != nil { + t.Fatalf("load saved session: %v", err) + } + if loaded.Messages[2].Content != "" { + t.Fatalf("expected empty content to round-trip, got %q", loaded.Messages[2].Content) + } + if loaded.Messages[2].ToolMetadata["tool_name"] != "filesystem_read_file" || + loaded.Messages[2].ToolMetadata["path"] != "README.md" { + t.Fatalf("expected metadata-only tool message round-trip, got %+v", loaded.Messages[2].ToolMetadata) + } +} + func TestDecodeStoredSummaryUsesLightweightMetadataPath(t *testing.T) { t.Parallel()