From 79e6c7bfbd04452e9d977f4d4203777fecf5f1b9 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Fri, 17 Jul 2026 01:22:00 -0400 Subject: [PATCH 1/6] chore: remove code unreachable on every supported platform The Security and code health job's advisory deadcode step reports 152 unreachable functions on main. Running the same check under GOOS=linux, windows, and darwin shows 13 of those are false positives whose callers live in windows build-tagged files, so only functions unreachable on all three platforms are touched. Functions with no references anywhere are deleted, including the internal/reasoning package, which nothing imports. Functions that only tests reference move into per-package export_test.go files so tests keep their seams while the production binary sheds the code. Test call sites of deleted convenience wrappers are ported to the live APIs (scoped tool constructors, tools.DeferredSource, FinishReason checks). Kept despite the report: cron.Store.Runs (only reader of runs.jsonl, which production writes), and agenteval.TraceEventKeys and MissingTraceEvents (used across packages by the trace contract test). --- internal/acp/agent.go | 3 - internal/acp/export_test.go | 11 + internal/acp/types.go | 8 - .../additional_permissions_validation_test.go | 8 +- internal/agent/command_prefix.go | 12 - internal/agent/compaction_preserve.go | 9 - internal/agent/compaction_test.go | 8 +- internal/agent/deferred_loop_test.go | 6 +- internal/agent/denial_test.go | 2 +- internal/agent/export_test.go | 41 + internal/agent/guardrails_test.go | 14 +- internal/agent/loop.go | 13 - internal/agent/loop_test.go | 75 +- internal/agent/partition_cache_stable_test.go | 2 +- internal/agent/request_permissions_test.go | 2 +- internal/agent/types.go | 7 - internal/agenteval/agent_command.go | 4 - internal/agenteval/export_test.go | 8 + internal/config/export_test.go | 72 ++ internal/config/types.go | 8 - internal/config/validate.go | 23 - internal/config/writer.go | 33 - internal/contextreport/contextreport_test.go | 4 +- internal/cron/export_test.go | 4 + internal/cron/schedule.go | 2 - internal/dictation/dictation.go | 10 - internal/lsp/export_test.go | 14 + internal/lsp/registry.go | 11 - internal/mcp/registry_test.go | 14 +- internal/modelregistry/export_test.go | 21 + internal/modelregistry/modelsdev.go | 8 - internal/modelregistry/modes.go | 10 - internal/notify/export_test.go | 7 + internal/notify/notify.go | 5 - internal/providercatalog/catalog.go | 37 - internal/providercatalog/export_test.go | 39 + internal/providers/openai/codex.go | 12 - internal/providers/openai/export_test.go | 18 + internal/providers/openai/provider_test.go | 4 +- internal/providers/providerio/export_test.go | 14 + internal/providers/providerio/providerio.go | 7 - internal/reasoning/capability.go | 147 --- internal/reasoning/catalog.go | 99 -- internal/reasoning/catalog_test.go | 249 ---- internal/reasoning/modelsdev_snapshot.json | 1082 ----------------- internal/reltime/reltime.go | 64 - internal/repomap/export_test.go | 12 + internal/repomap/repomap.go | 5 - internal/review/export_test.go | 11 + internal/review/review.go | 9 - internal/sandbox/export_test.go | 78 ++ internal/sandbox/grants.go | 4 - internal/sandbox/profile.go | 4 - internal/sandbox/runner.go | 52 - internal/sandbox/windows_setup.go | 11 - internal/sessions/checkpoint.go | 16 - internal/sessions/export_test.go | 20 + internal/sessions/replay.go | 4 - internal/skills/export_test.go | 66 + internal/skills/install.go | 17 - internal/skills/skills.go | 45 - internal/specialist/export_test.go | 55 + internal/specialist/manifest_test.go | 4 +- internal/specialist/output_tool.go | 4 - internal/specialist/stop_tool.go | 4 - internal/specialist/streamer.go | 37 - internal/streamjson/export_test.go | 10 + internal/streamjson/streamjson.go | 8 - internal/tools/apply_patch.go | 4 - internal/tools/argtolerance_test.go | 30 +- internal/tools/ask_user_test.go | 2 +- internal/tools/bash.go | 9 - internal/tools/bash_auto_allow_test.go | 8 +- internal/tools/bash_tool_test.go | 40 +- internal/tools/builtin_catalog.go | 6 +- internal/tools/deferred.go | 134 -- internal/tools/edit_file.go | 4 - internal/tools/edit_replacers_test.go | 2 +- internal/tools/export_test.go | 168 +++ internal/tools/file_safety_test.go | 2 +- internal/tools/file_tools_test.go | 48 +- internal/tools/format_on_write_test.go | 6 +- internal/tools/inline_diagnostics_test.go | 6 +- internal/tools/lsp_navigate.go | 5 - internal/tools/lsp_navigate_test.go | 8 +- internal/tools/read_exclusions_test.go | 8 +- internal/tools/read_minified_file_test.go | 6 +- internal/tools/registry.go | 6 - internal/tools/registry_test.go | 14 +- internal/tools/search_cancellation_test.go | 12 +- internal/tools/spill_hardening_test.go | 2 +- internal/tools/tool_io_bench_test.go | 4 +- internal/tools/web_fetch.go | 4 - internal/tools/write_file.go | 4 - internal/tools/write_tools_test.go | 56 +- internal/trace/emit.go | 83 -- internal/trace/export_test.go | 86 ++ internal/trace/trace.go | 1 + internal/tui/assistant_markdown.go | 9 - internal/tui/autocomplete.go | 12 - internal/tui/command_polish_test.go | 4 +- internal/tui/commands.go | 26 - internal/tui/export_test.go | 298 +++++ internal/tui/image_attach.go | 14 - internal/tui/model.go | 17 - internal/tui/model_test.go | 4 +- internal/tui/mouse.go | 4 - internal/tui/plan_panel.go | 14 - internal/tui/pr_status.go | 14 - internal/tui/render_cache.go | 18 - internal/tui/selectable_list.go | 59 - internal/tui/session_test.go | 12 +- internal/tui/spec_mode_test.go | 2 +- internal/tui/specialist_card.go | 21 - internal/tui/startup.go | 4 - internal/tui/streaming_decoder.go | 12 - internal/tui/streaming_fade.go | 11 - internal/tui/stt_model_picker.go | 7 - internal/tui/tool_render_registry.go | 4 - internal/tui/transcript_selection.go | 41 - internal/tui/transcript_viewport.go | 4 - internal/update/export_test.go | 32 + internal/update/update.go | 31 - internal/verify/export_test.go | 40 + internal/verify/verify.go | 31 - internal/workspaceindex/workspaceindex.go | 10 - internal/zerocommands/backend_snapshots.go | 24 - internal/zerocommands/contracts.go | 8 - internal/zerocommands/export_test.go | 31 + internal/zeroruntime/finish_reason_test.go | 6 - internal/zeroruntime/helpers.go | 7 - 131 files changed, 1365 insertions(+), 2961 deletions(-) create mode 100644 internal/acp/export_test.go create mode 100644 internal/agent/export_test.go create mode 100644 internal/agenteval/export_test.go create mode 100644 internal/config/export_test.go create mode 100644 internal/cron/export_test.go create mode 100644 internal/lsp/export_test.go create mode 100644 internal/modelregistry/export_test.go create mode 100644 internal/notify/export_test.go create mode 100644 internal/providercatalog/export_test.go create mode 100644 internal/providers/openai/export_test.go create mode 100644 internal/providers/providerio/export_test.go delete mode 100644 internal/reasoning/capability.go delete mode 100644 internal/reasoning/catalog.go delete mode 100644 internal/reasoning/catalog_test.go delete mode 100644 internal/reasoning/modelsdev_snapshot.json create mode 100644 internal/repomap/export_test.go create mode 100644 internal/review/export_test.go create mode 100644 internal/sandbox/export_test.go create mode 100644 internal/sessions/export_test.go create mode 100644 internal/skills/export_test.go create mode 100644 internal/specialist/export_test.go create mode 100644 internal/streamjson/export_test.go create mode 100644 internal/tools/export_test.go create mode 100644 internal/trace/export_test.go create mode 100644 internal/tui/export_test.go create mode 100644 internal/update/export_test.go create mode 100644 internal/verify/export_test.go create mode 100644 internal/zerocommands/export_test.go diff --git a/internal/acp/agent.go b/internal/acp/agent.go index 16c7110cd..b3fafb32a 100644 --- a/internal/acp/agent.go +++ b/internal/acp/agent.go @@ -90,9 +90,6 @@ func NewAgent(conn *Conn, deps Deps) *Agent { return a } -// Serve runs the connection read loop until the stream closes or ctx is done. -func (a *Agent) Serve(ctx context.Context) error { return a.conn.Serve(ctx) } - // ---- initialize ---- func (a *Agent) handleInitialize(_ context.Context, params json.RawMessage) (any, error) { diff --git a/internal/acp/export_test.go b/internal/acp/export_test.go new file mode 100644 index 000000000..8467e8941 --- /dev/null +++ b/internal/acp/export_test.go @@ -0,0 +1,11 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package acp + +import "context" + +// Serve runs the connection read loop until the stream closes or ctx is done. +func (a *Agent) Serve(ctx context.Context) error { return a.conn.Serve(ctx) } + +func ImageBlock(base64Data, mimeType string) ContentBlock { + return ContentBlock{Type: "image", Data: base64Data, MimeType: mimeType} +} diff --git a/internal/acp/types.go b/internal/acp/types.go index 340c9be88..b00bf672a 100644 --- a/internal/acp/types.go +++ b/internal/acp/types.go @@ -103,10 +103,6 @@ type ContentBlock struct { func TextBlock(text string) ContentBlock { return ContentBlock{Type: "text", Text: text} } -func ImageBlock(base64Data, mimeType string) ContentBlock { - return ContentBlock{Type: "image", Data: base64Data, MimeType: mimeType} -} - // ---- sessions ---- // McpServer mirrors the editor-provided MCP server entry. ZERO owns its own MCP @@ -233,10 +229,6 @@ func ToolContent(block ContentBlock) ToolCallContent { return ToolCallContent{Type: "content", Content: &block} } -func ToolDiff(path, oldText, newText string) ToolCallContent { - return ToolCallContent{Type: "diff", Path: path, OldText: oldText, NewText: newText} -} - type ToolCallLocation struct { Path string `json:"path"` Line *int `json:"line,omitempty"` diff --git a/internal/agent/additional_permissions_validation_test.go b/internal/agent/additional_permissions_validation_test.go index e6831cc67..f87d2dedb 100644 --- a/internal/agent/additional_permissions_validation_test.go +++ b/internal/agent/additional_permissions_validation_test.go @@ -16,7 +16,7 @@ import ( func TestExecuteToolCallRejectsMissingAdditionalPermissionsBeforePrompt(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) promptCalls := 0 result, err := executeToolCall( @@ -51,7 +51,7 @@ func TestExecuteToolCallRejectsMissingAdditionalPermissionsBeforePrompt(t *testi func TestExecuteToolCallRejectsAdditionalPermissionsWithoutFlagBeforePrompt(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) promptCalls := 0 result, err := executeToolCall( @@ -88,7 +88,7 @@ func TestExecuteToolCallRejectsAdditionalPermissionsWithoutFlagBeforePrompt(t *t func TestExecuteToolCallMissingAdditionalPermissionsErrorIsActionable(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) result, err := executeToolCall( context.Background(), @@ -117,7 +117,7 @@ func TestExecuteToolCallMissingAdditionalPermissionsErrorIsActionable(t *testing func TestExecuteToolCallStillPromptsForValidAdditionalPermissions(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) promptCalls := 0 _, err := executeToolCall( diff --git a/internal/agent/command_prefix.go b/internal/agent/command_prefix.go index 0e92cdd09..2d0f5a606 100644 --- a/internal/agent/command_prefix.go +++ b/internal/agent/command_prefix.go @@ -501,15 +501,3 @@ func hasStringPrefix(values []string, prefix []string) bool { } return true } - -func equalStringSlices(left []string, right []string) bool { - if len(left) != len(right) { - return false - } - for index := range left { - if left[index] != right[index] { - return false - } - } - return true -} diff --git a/internal/agent/compaction_preserve.go b/internal/agent/compaction_preserve.go index e6f8c0dc3..0e38ef5f5 100644 --- a/internal/agent/compaction_preserve.go +++ b/internal/agent/compaction_preserve.go @@ -610,15 +610,6 @@ func formatPreservedState(plan string, task *preservedTaskState, edits, tools, s return preservedStateLabel + "\n" + string(encoded) } -// parsePreservedState recovers the plan + skills from a prior summary's preserved -// block. JSON escaping makes this lossless even when a skill body contains -// markdown headings, code fences, or quotes. Returns ("", nil) when absent or -// malformed. -func parsePreservedState(summaryContent string) (string, []skillEntry) { - state := parsePreservedStateBlock(summaryContent) - return state.Plan, preservedSkillsToEntries(state.Skills) -} - func parsePreservedStateBlock(summaryContent string) preservedState { idx := strings.LastIndex(summaryContent, preservedStateLabel) if idx < 0 { diff --git a/internal/agent/compaction_test.go b/internal/agent/compaction_test.go index 1dbe2fe4c..292ba4f17 100644 --- a/internal/agent/compaction_test.go +++ b/internal/agent/compaction_test.go @@ -336,7 +336,7 @@ func TestRunProactiveCompactionTriggers(t *testing.T) { } registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(t.TempDir())) + registry.Register(tools.NewScopedReadFileTool(t.TempDir(), nil)) result, err := Run(context.Background(), strings.Repeat("y", 8000), provider, Options{ Registry: registry, @@ -364,7 +364,7 @@ func TestRunNoCompactionWhenContextWindowZero(t *testing.T) { }, } registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(t.TempDir())) + registry.Register(tools.NewScopedReadFileTool(t.TempDir(), nil)) _, err := Run(context.Background(), strings.Repeat("y", 8000), provider, Options{ Registry: registry, @@ -427,7 +427,7 @@ func TestRunReactiveCompactionRecovers(t *testing.T) { // the compaction closure. read_file also returns a sizeable result that // bloats the history. registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(t.TempDir())) + registry.Register(tools.NewScopedReadFileTool(t.TempDir(), nil)) // ContextWindow large enough that proactive compaction never triggers, so // only the reactive path can save the run. @@ -491,7 +491,7 @@ func TestRunReactiveRetryDoesNotDoubleEmitText(t *testing.T) { finalText: "recovered", } registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(t.TempDir())) + registry.Register(tools.NewScopedReadFileTool(t.TempDir(), nil)) var deltas []string result, err := Run(context.Background(), strings.Repeat("z", 6000), provider, Options{ diff --git a/internal/agent/deferred_loop_test.go b/internal/agent/deferred_loop_test.go index 1f84219fb..8be4001e1 100644 --- a/internal/agent/deferred_loop_test.go +++ b/internal/agent/deferred_loop_test.go @@ -150,7 +150,7 @@ func assertNoDeferredDiscoveryMessage(t *testing.T, request zeroruntime.Completi func TestPartitionToolsInactiveIsByteIdenticalAndDropsToolSearch(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) registry.Register(fakeDeferredTool{name: "mcp__srv__a", desc: "tool a"}) registry.Register(fakeToolSearchTool{}) @@ -305,8 +305,8 @@ func TestPartitionToolsActiveExcludesDisabledDeferredFromDiscoveryAndExposed(t * func TestPartitionToolsActiveHidesUnloadedExposesLoaded(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) // non-deferred builtin - registry.Register(fakeToolSearchTool{}) // non-deferred, must stay exposed + registry.Register(tools.NewScopedReadFileTool(root, nil)) // non-deferred builtin + registry.Register(fakeToolSearchTool{}) // non-deferred, must stay exposed registry.Register(fakeDeferredTool{name: "mcp__srv__alpha", desc: "alpha tool"}) registry.Register(fakeDeferredTool{name: "mcp__srv__beta", desc: "beta tool"}) diff --git a/internal/agent/denial_test.go b/internal/agent/denial_test.go index 0411e8ee8..ff6b624e0 100644 --- a/internal/agent/denial_test.go +++ b/internal/agent/denial_test.go @@ -11,7 +11,7 @@ import ( func TestExecuteToolCallCategorizesFilteredDenial(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) result, _ := executeToolCall(context.Background(), registry, ToolCall{ID: "c1", Name: "read_file", Arguments: `{"path":"x"}`}, PermissionModeAuto, Options{ DisabledTools: []string{"read_file"}, diff --git a/internal/agent/export_test.go b/internal/agent/export_test.go new file mode 100644 index 000000000..1a2a153f6 --- /dev/null +++ b/internal/agent/export_test.go @@ -0,0 +1,41 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package agent + +import ( + "github.com/Gitlawb/zero/internal/tools" + "github.com/Gitlawb/zero/internal/zeroruntime" +) + +func equalStringSlices(left []string, right []string) bool { + if len(left) != len(right) { + return false + } + for index := range left { + if left[index] != right[index] { + return false + } + } + return true +} + +// parsePreservedState recovers the plan + skills from a prior summary's preserved +// block. JSON escaping makes this lossless even when a skill body contains +// markdown headings, code fences, or quotes. Returns ("", nil) when absent or +// malformed. +func parsePreservedState(summaryContent string) (string, []skillEntry) { + state := parsePreservedStateBlock(summaryContent) + return state.Plan, preservedSkillsToEntries(state.Skills) +} + +// partitionTools builds the per-turn advertised tool list and optional +// tool_search discovery text. INACTIVE (DeferThreshold <= 0 or the eligible count is +// below it): every visible tool is exposed with its full schema EXCEPT tool_search +// (dropped so it is never advertised when it cannot help), and the discovery text is +// empty — byte-identical to the pre-deferral output. ACTIVE: a deferred-eligible +// tool is exposed only when loaded[name]; otherwise it is hidden and searchable +// through tool_search. Non-deferred tools (including tool_search) are always +// exposed. The exposed slice is alpha-sorted by name, matching the legacy order +// so the inactive path is stable. +func partitionTools(registry *tools.Registry, permissionMode PermissionMode, options Options, loaded map[string]bool) ([]zeroruntime.ToolDefinition, string) { + return partitionToolsCached(registry, permissionMode, options, loaded, nil) +} diff --git a/internal/agent/guardrails_test.go b/internal/agent/guardrails_test.go index fc8079f13..4534b2396 100644 --- a/internal/agent/guardrails_test.go +++ b/internal/agent/guardrails_test.go @@ -138,7 +138,7 @@ func TestRunResetsEmptyTurnCounterOnToolCall(t *testing.T) { root := t.TempDir() writeAgentTestFile(t, root+"/notes.txt", "alpha") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ @@ -260,7 +260,7 @@ func TestRunInjectsPlanNotCalledReminderForMultiStepTask(t *testing.T) { root := t.TempDir() writeAgentTestFile(t, root+"/notes.txt", "alpha") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ @@ -291,7 +291,7 @@ func TestRunDoesNotInjectPlanReminderForTrivialTask(t *testing.T) { root := t.TempDir() writeAgentTestFile(t, root+"/notes.txt", "alpha") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ @@ -319,7 +319,7 @@ func TestRunDoesNotInjectNotCalledReminderWhenPlanUsed(t *testing.T) { root := t.TempDir() writeAgentTestFile(t, root+"/notes.txt", "alpha") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) registry.Register(tools.NewUpdatePlanTool()) provider := &mockProvider{ @@ -349,7 +349,7 @@ func TestRunInjectsStalePlanReminderAfterManyToolCalls(t *testing.T) { root := t.TempDir() writeAgentTestFile(t, root+"/notes.txt", "alpha") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) registry.Register(tools.NewUpdatePlanTool()) // Turn 1 calls update_plan (so the not-called reminder never triggers), then @@ -383,7 +383,7 @@ func TestRunStalePlanReminderIsOneShotPerInterval(t *testing.T) { root := t.TempDir() writeAgentTestFile(t, root+"/notes.txt", "alpha") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) registry.Register(tools.NewUpdatePlanTool()) turns := [][]zeroruntime.StreamEvent{ @@ -418,7 +418,7 @@ func TestRunInjectsToolOnlyProgressReminder(t *testing.T) { root := t.TempDir() writeAgentTestFile(t, root+"/notes.txt", "alpha") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) turns := make([][]zeroruntime.StreamEvent, 0, toolOnlyProgressReminderAt+1) for i := 0; i < toolOnlyProgressReminderAt; i++ { diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 0b1f8dffb..8356cd316 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -2866,19 +2866,6 @@ func permissionActionFromSandbox(action sandbox.Action) PermissionAction { } } -// partitionTools builds the per-turn advertised tool list and optional -// tool_search discovery text. INACTIVE (DeferThreshold <= 0 or the eligible count is -// below it): every visible tool is exposed with its full schema EXCEPT tool_search -// (dropped so it is never advertised when it cannot help), and the discovery text is -// empty — byte-identical to the pre-deferral output. ACTIVE: a deferred-eligible -// tool is exposed only when loaded[name]; otherwise it is hidden and searchable -// through tool_search. Non-deferred tools (including tool_search) are always -// exposed. The exposed slice is alpha-sorted by name, matching the legacy order -// so the inactive path is stable. -func partitionTools(registry *tools.Registry, permissionMode PermissionMode, options Options, loaded map[string]bool) ([]zeroruntime.ToolDefinition, string) { - return partitionToolsCached(registry, permissionMode, options, loaded, nil) -} - // partitionToolsCached is partitionTools with an optional per-tool definition // cache. The partitioning itself (visibility, deferral, ordering) is recomputed // every call — it must be, because a tool's deferred state can flip mid-run (e.g. diff --git a/internal/agent/loop_test.go b/internal/agent/loop_test.go index 69913a420..86258f432 100644 --- a/internal/agent/loop_test.go +++ b/internal/agent/loop_test.go @@ -863,9 +863,6 @@ func TestRunReportsTruncationFinishReason(t *testing.T) { if result.FinishReason != zeroruntime.FinishReasonLength { t.Fatalf("FinishReason = %q, want %q", result.FinishReason, zeroruntime.FinishReasonLength) } - if !result.Truncated() { - t.Fatal("Truncated() = false, want true for a length-capped response") - } if result.TruncationNotice() == "" { t.Fatal("TruncationNotice() empty for a truncated response") } @@ -883,7 +880,7 @@ func TestRunNormalCompletionIsNotTruncated(t *testing.T) { if err != nil { t.Fatal(err) } - if result.Truncated() || result.TruncationNotice() != "" { + if result.TruncationNotice() != "" { t.Fatalf("normal completion reported as truncated: reason=%q", result.FinishReason) } } @@ -960,7 +957,7 @@ func TestRunEmitsUsageEvents(t *testing.T) { func TestRunAdvertisesRuntimeToolDefinitions(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{{ {Type: zeroruntime.StreamEventText, Content: "done"}, @@ -1167,9 +1164,9 @@ func TestRunRequestsPermissionBeforeWebSearchExecution(t *testing.T) { func TestRunFiltersAdvertisedTools(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) - registry.Register(tools.NewGrepTool(root)) - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) + registry.Register(tools.NewScopedGrepTool(root, nil)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{{ {Type: zeroruntime.StreamEventText, Content: "done"}, @@ -1197,7 +1194,7 @@ func TestRunFiltersAdvertisedTools(t *testing.T) { func TestRunRejectsFilteredToolCalls(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -1234,7 +1231,7 @@ func TestRunRejectsFilteredToolCalls(t *testing.T) { func TestRunRejectsToolCallsOutsideEnabledList(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -1272,7 +1269,7 @@ func TestRunExecutesToolCallThroughRegistry(t *testing.T) { root := t.TempDir() writeAgentTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta\n") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -1356,7 +1353,7 @@ func TestRunPreservesRequestPrefixAcrossTurns(t *testing.T) { func TestRunSanitizesMalformedToolCallArgumentsBeforeRetry(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -1424,7 +1421,7 @@ func TestRunRecoversFirstObjectFromConcatenatedToolArgs(t *testing.T) { t.Fatal(err) } registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -1471,8 +1468,8 @@ func TestRunDefersSelfCorrectFeedbackUntilAfterToolBatch(t *testing.T) { t.Fatal(err) } registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ @@ -1547,7 +1544,7 @@ func TestRunBatchesSelfCorrectOncePerTurn(t *testing.T) { // after a later call in the same turn supersedes it. root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ @@ -1604,7 +1601,7 @@ func TestRunBatchesSelfCorrectOncePerTurn(t *testing.T) { func TestRunDeniesPromptToolWithoutUnsafePermission(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write denied") var permissionEvents []PermissionEvent @@ -1647,7 +1644,7 @@ func TestRunDeniesPromptToolWithoutUnsafePermission(t *testing.T) { func TestRunRequestsPromptToolPermissionBeforeExecution(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write approved") var requests []PermissionRequest var permissionEvents []PermissionEvent @@ -1703,7 +1700,7 @@ func TestRunRequestsPromptToolPermissionBeforeExecution(t *testing.T) { func TestRunAllowsWorkspaceWriteWithoutPromptWhenSandboxPolicyPermits(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write done") var permissionEvents []PermissionEvent @@ -1752,7 +1749,7 @@ func TestRunAllowsWorkspaceWriteWithoutPromptWhenSandboxPolicyPermits(t *testing func TestRunDeniesPromptToolWhenPermissionRequestDenied(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write denied") var requests []PermissionRequest var permissionEvents []PermissionEvent @@ -1800,7 +1797,7 @@ func TestRunDeniesPromptToolWhenPermissionRequestDenied(t *testing.T) { func TestRunAbortsWhenPermissionRequestCanceled(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write should not continue") var permissionEvents []PermissionEvent @@ -1896,7 +1893,7 @@ func TestRunPersistsAlwaysAllowPermissionDecision(t *testing.T) { t.Fatal(err) } registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write approved") var permissionEvents []PermissionEvent policy := sandbox.DefaultPolicy() @@ -1968,7 +1965,7 @@ func TestRunSessionAllowSkipsMatchingPromptWithoutPersistentGrant(t *testing.T) t.Fatal(err) } registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -2049,7 +2046,7 @@ func TestRunSessionAllowSkipsMatchingPromptWithoutPersistentGrant(t *testing.T) func TestRunCommandPrefixApprovalSkipsLaterMatchingBashPrompt(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -2251,7 +2248,7 @@ func TestRunPersistentCommandPrefixApprovalSkipsFutureSessionPrompt(t *testing.T t.Fatal(err) } registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) policy := sandbox.DefaultPolicy() policy.Network = sandbox.NetworkAllow @@ -2358,7 +2355,7 @@ func TestRunPersistentCommandPrefixStillPromptsForNetwork(t *testing.T) { t.Fatalf("seed command prefix: %v", err) } registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -2419,7 +2416,7 @@ func TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant(t *testing.T) { } } registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -2486,7 +2483,7 @@ func TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant(t *testing.T) { func TestRunDoesNotOfferPrefixApprovalForUnsafeBashCommand(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -2529,7 +2526,7 @@ func TestRunDoesNotOfferPrefixApprovalForUnsafeBashCommand(t *testing.T) { func TestRunPromptsForDestructiveShellInsteadOfSandboxDeny(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -2598,7 +2595,7 @@ func TestRunAlwaysAllowWithoutSandboxStillAllowsCall(t *testing.T) { // prior code denied it because persistPermissionGrant errors when Sandbox==nil. root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write approved") var permissionEvents []PermissionEvent @@ -2662,7 +2659,7 @@ func TestRunCancellationPreservesContextCanceledIdentity(t *testing.T) { func TestRunGrantsPromptToolInUnsafeMode(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write done") var permissionEvents []PermissionEvent @@ -2714,7 +2711,7 @@ func TestRunEmitsPermissionEventForPersistentSandboxGrant(t *testing.T) { t.Fatal(err) } registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write done") var permissionEvents []PermissionEvent @@ -2893,7 +2890,7 @@ func TestRunAppliesSandboxEvenInUnsafeMode(t *testing.T) { root := t.TempDir() outside := filepath.Join(tempDirOutsideDefaultTemp(t), "escape.txt") registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWritePathThenAnswer(outside, "sandbox handled") var permissionEvents []PermissionEvent @@ -2942,7 +2939,7 @@ func TestRunStopsAfterMaxTurns(t *testing.T) { root := t.TempDir() writeAgentTestFile(t, filepath.Join(root, "notes.txt"), "alpha") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{{ {Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "read_file"}, @@ -2972,7 +2969,7 @@ func TestRunRequestsFinalAnswerAfterMaxTurns(t *testing.T) { root := t.TempDir() writeAgentTestFile(t, filepath.Join(root, "notes.txt"), "alpha") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -3231,7 +3228,7 @@ func TestBuildSystemPromptAllowsSpecModeOverride(t *testing.T) { func TestSpecDraftAdvertisesOnlySafeDraftTools(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - for _, tool := range tools.CoreTools(root) { + for _, tool := range tools.CoreToolsScoped(root, nil) { registry.Register(tool) } specmode.RegisterDraftTools(registry, root, nil) @@ -3268,7 +3265,7 @@ func TestSpecDraftAdvertisesOnlySafeDraftTools(t *testing.T) { func TestSpecDraftDeniesHiddenToolCalls(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("done") result, err := Run(context.Background(), "draft", provider, Options{ @@ -3301,7 +3298,7 @@ func TestSpecDraftDeniesHiddenToolCalls(t *testing.T) { func TestSpecDraftDeniesBashToolCalls(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewBashTool(root)) + registry.Register(tools.NewScopedBashTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -3442,7 +3439,7 @@ func TestRunSurfacesDroppedToolCallAlongsideValidCall(t *testing.T) { t.Fatal(err) } registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ diff --git a/internal/agent/partition_cache_stable_test.go b/internal/agent/partition_cache_stable_test.go index 9befbfae1..5d6692e0c 100644 --- a/internal/agent/partition_cache_stable_test.go +++ b/internal/agent/partition_cache_stable_test.go @@ -14,7 +14,7 @@ func TestPartitionToolsActiveAppendsLoadedToolAfterEagerBlock(t *testing.T) { // "read_file". root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) // non-deferred, "read_file" + registry.Register(tools.NewScopedReadFileTool(root, nil)) // non-deferred, "read_file" registry.Register(fakeToolSearchTool{}) registry.Register(fakeDeferredTool{name: "mcp__srv__alpha", desc: "alpha"}) registry.Register(fakeDeferredTool{name: "mcp__srv__beta", desc: "beta"}) diff --git a/internal/agent/request_permissions_test.go b/internal/agent/request_permissions_test.go index fb12d6e73..459da068a 100644 --- a/internal/agent/request_permissions_test.go +++ b/internal/agent/request_permissions_test.go @@ -180,7 +180,7 @@ func TestInlineAdditionalPermissionsRequiresPromptUnderAutoAllow(t *testing.T) { }, } decision := &sandbox.Decision{Action: sandbox.ActionAllow, AutoAllowed: true} - if !shouldRequestPermission(tools.NewBashTool(t.TempDir()), args, false, decision) { + if !shouldRequestPermission(tools.NewScopedBashTool(t.TempDir(), nil), args, false, decision) { t.Fatal("inline additional permissions must request approval even when a sandboxed shell would otherwise auto-allow") } } diff --git a/internal/agent/types.go b/internal/agent/types.go index 0e3391b9e..cfee5b20e 100644 --- a/internal/agent/types.go +++ b/internal/agent/types.go @@ -427,13 +427,6 @@ type Result struct { IncompleteReason string } -// Truncated reports whether the final response ended abnormally (cut off at the -// output token cap or withheld by a content filter) rather than completing -// naturally. Callers can use it to warn the user that FinalAnswer is incomplete. -func (result Result) Truncated() bool { - return result.FinishReason != "" -} - // TruncationNotice returns a user-facing warning when the final response was // truncated, or "" for a normal completion. Shared by the CLI and TUI so the // wording stays consistent. diff --git a/internal/agenteval/agent_command.go b/internal/agenteval/agent_command.go index 88f3f8757..a07d32ef2 100644 --- a/internal/agenteval/agent_command.go +++ b/internal/agenteval/agent_command.go @@ -31,10 +31,6 @@ type AgentRunner interface { type AgentRunnerFunc func(context.Context, AgentRunInput) AgentRunResult -func (fn AgentRunnerFunc) Run(ctx context.Context, input AgentRunInput) AgentRunResult { - return fn(ctx, input) -} - // defaultAgentOutputLimit caps captured stdout/stderr per stream so a chatty or // runaway agent cannot exhaust memory or bloat the benchmark report. const defaultAgentOutputLimit = 1 << 20 // 1 MiB per stream diff --git a/internal/agenteval/export_test.go b/internal/agenteval/export_test.go new file mode 100644 index 000000000..194859ff1 --- /dev/null +++ b/internal/agenteval/export_test.go @@ -0,0 +1,8 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package agenteval + +import "context" + +func (fn AgentRunnerFunc) Run(ctx context.Context, input AgentRunInput) AgentRunResult { + return fn(ctx, input) +} diff --git a/internal/config/export_test.go b/internal/config/export_test.go new file mode 100644 index 000000000..cb0544124 --- /dev/null +++ b/internal/config/export_test.go @@ -0,0 +1,72 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package config + +import ( + "encoding/json" + "fmt" + "os" + "strings" +) + +// ToolsOverride builds a ToolsConfig that explicitly overrides the deferred-tool +// threshold (including to 0, which disables deferral). Use this for programmatic +// Overrides — a bare ToolsConfig{DeferThreshold: 0} is indistinguishable from +// "unset" and will not override. +func ToolsOverride(deferThreshold int) ToolsConfig { + return ToolsConfig{DeferThreshold: deferThreshold, deferThresholdSet: true} +} + +// ValidateFile reads and parses path as a Zero FileConfig and runs the same +// semantic provider/model rules used during resolution. It returns the parsed +// config (zero value on parse failure) plus any structured issues. A parse +// failure yields a single issue whose Message wraps the underlying JSON error +// so callers can extract *json.SyntaxError / *json.UnmarshalTypeError offsets +// via errors.As. +func ValidateFile(path string) (FileConfig, []Issue) { + data, err := os.ReadFile(path) + if err != nil { + return FileConfig{}, []Issue{{Message: fmt.Sprintf("read config %s: %v", path, err)}} + } + + var cfg FileConfig + if err := json.Unmarshal(data, &cfg); err != nil { + return FileConfig{}, []Issue{{Message: fmt.Errorf("invalid config JSON %s: %w", path, err).Error()}} + } + + issues := validateSemantics(cfg) + issues = append(issues, unknownFieldIssues(data)...) + return cfg, issues +} + +// SetProviderDescription sets a provider's description VERBATIM — including to +// empty. The generic UpsertProvider merge treats empty fields as "leave +// unchanged", so clearing a description needs this dedicated setter. +func SetProviderDescription(path string, name string, description string) (FileConfig, error) { + path = strings.TrimSpace(path) + if path == "" { + return FileConfig{}, fmt.Errorf("config path is required") + } + name = strings.TrimSpace(name) + if name == "" { + return FileConfig{}, fmt.Errorf("provider name is required") + } + + data, err := os.ReadFile(path) + if err != nil { + return FileConfig{}, fmt.Errorf("read config %s: %w", path, err) + } + cfg := FileConfig{} + if err := json.Unmarshal(data, &cfg); err != nil { + return FileConfig{}, fmt.Errorf("invalid config JSON %s: %w", path, err) + } + for index := range cfg.Providers { + if strings.EqualFold(strings.TrimSpace(cfg.Providers[index].Name), name) { + cfg.Providers[index].Description = strings.TrimSpace(description) + if err := writeConfigFile(path, cfg); err != nil { + return FileConfig{}, err + } + return cfg, nil + } + } + return FileConfig{}, fmt.Errorf("provider %q not found", name) +} diff --git a/internal/config/types.go b/internal/config/types.go index 8e9cc5476..6c91fdaf3 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -322,14 +322,6 @@ type SwarmConfig struct { MaxTeamSize int `json:"maxTeamSize,omitempty"` } -// ToolsOverride builds a ToolsConfig that explicitly overrides the deferred-tool -// threshold (including to 0, which disables deferral). Use this for programmatic -// Overrides — a bare ToolsConfig{DeferThreshold: 0} is indistinguishable from -// "unset" and will not override. -func ToolsOverride(deferThreshold int) ToolsConfig { - return ToolsConfig{DeferThreshold: deferThreshold, deferThresholdSet: true} -} - func (cfg *ToolsConfig) UnmarshalJSON(data []byte) error { type rawTools struct { DeferThreshold *int `json:"deferThreshold"` diff --git a/internal/config/validate.go b/internal/config/validate.go index 847eb52fc..631c54164 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -3,7 +3,6 @@ package config import ( "encoding/json" "fmt" - "os" ) // Issue is a single structured problem found while validating a config file. @@ -13,28 +12,6 @@ type Issue struct { Message string `json:"message"` } -// ValidateFile reads and parses path as a Zero FileConfig and runs the same -// semantic provider/model rules used during resolution. It returns the parsed -// config (zero value on parse failure) plus any structured issues. A parse -// failure yields a single issue whose Message wraps the underlying JSON error -// so callers can extract *json.SyntaxError / *json.UnmarshalTypeError offsets -// via errors.As. -func ValidateFile(path string) (FileConfig, []Issue) { - data, err := os.ReadFile(path) - if err != nil { - return FileConfig{}, []Issue{{Message: fmt.Sprintf("read config %s: %v", path, err)}} - } - - var cfg FileConfig - if err := json.Unmarshal(data, &cfg); err != nil { - return FileConfig{}, []Issue{{Message: fmt.Errorf("invalid config JSON %s: %w", path, err).Error()}} - } - - issues := validateSemantics(cfg) - issues = append(issues, unknownFieldIssues(data)...) - return cfg, issues -} - // ValidateBytes parses data as a Zero FileConfig and runs the same semantic // provider/model rules as ValidateFile. It returns the parsed config (zero // value on parse failure) plus any structured issues. A parse failure yields a diff --git a/internal/config/writer.go b/internal/config/writer.go index 38f395d60..861eb3146 100644 --- a/internal/config/writer.go +++ b/internal/config/writer.go @@ -431,39 +431,6 @@ func EditProvider(path string, edit ProviderEdit) (FileConfig, error) { return cfg, nil } -// SetProviderDescription sets a provider's description VERBATIM — including to -// empty. The generic UpsertProvider merge treats empty fields as "leave -// unchanged", so clearing a description needs this dedicated setter. -func SetProviderDescription(path string, name string, description string) (FileConfig, error) { - path = strings.TrimSpace(path) - if path == "" { - return FileConfig{}, fmt.Errorf("config path is required") - } - name = strings.TrimSpace(name) - if name == "" { - return FileConfig{}, fmt.Errorf("provider name is required") - } - - data, err := os.ReadFile(path) - if err != nil { - return FileConfig{}, fmt.Errorf("read config %s: %w", path, err) - } - cfg := FileConfig{} - if err := json.Unmarshal(data, &cfg); err != nil { - return FileConfig{}, fmt.Errorf("invalid config JSON %s: %w", path, err) - } - for index := range cfg.Providers { - if strings.EqualFold(strings.TrimSpace(cfg.Providers[index].Name), name) { - cfg.Providers[index].Description = strings.TrimSpace(description) - if err := writeConfigFile(path, cfg); err != nil { - return FileConfig{}, err - } - return cfg, nil - } - } - return FileConfig{}, fmt.Errorf("provider %q not found", name) -} - // migrateStoredProviderKey moves a credential-store entry to a new provider // name: write-new-then-delete-old, so an interruption can leave a duplicate but // never a missing key. A missing source entry is a no-op (the marker may be diff --git a/internal/contextreport/contextreport_test.go b/internal/contextreport/contextreport_test.go index 3a2d07bdf..374a99600 100644 --- a/internal/contextreport/contextreport_test.go +++ b/internal/contextreport/contextreport_test.go @@ -16,7 +16,7 @@ func TestBuildCountsProjectGuidelinesAndFreeBudget(t *testing.T) { writeTestFile(t, root, "AGENTS.md", strings.Repeat("project rules\n", 200)) registry := tools.NewRegistry() - for _, tool := range tools.CoreTools(root) { + for _, tool := range tools.CoreToolsScoped(root, nil) { registry.Register(tool) } @@ -80,7 +80,7 @@ func TestBuildHasStableJSONContractAndCategoryMath(t *testing.T) { writeTestFile(t, root, "ZERO.md", strings.Repeat("zero rules\n", 16)) registry := tools.NewRegistry() - for _, tool := range tools.CoreTools(root) { + for _, tool := range tools.CoreToolsScoped(root, nil) { registry.Register(tool) } diff --git a/internal/cron/export_test.go b/internal/cron/export_test.go new file mode 100644 index 000000000..a9ff4c9b3 --- /dev/null +++ b/internal/cron/export_test.go @@ -0,0 +1,4 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package cron + +func (s Schedule) String() string { return s.expr } diff --git a/internal/cron/schedule.go b/internal/cron/schedule.go index e6074180d..0e4a87df3 100644 --- a/internal/cron/schedule.go +++ b/internal/cron/schedule.go @@ -23,8 +23,6 @@ type Schedule struct { expr string } -func (s Schedule) String() string { return s.expr } - // full returns a bitset with every value in [min,max] set. func full(min, max int) uint64 { var m uint64 diff --git a/internal/dictation/dictation.go b/internal/dictation/dictation.go index 2475093de..43d97d5bd 100644 --- a/internal/dictation/dictation.go +++ b/internal/dictation/dictation.go @@ -37,16 +37,6 @@ const ( StreamProviderOpenAI = "openai" ) -// BatchProviders lists valid stt.provider values, for config validation. -func BatchProviders() []string { - return []string{ProviderLocal, ProviderGroq, ProviderOpenAI} -} - -// StreamProviders lists valid stt.streamProvider values, for config validation. -func StreamProviders() []string { - return []string{StreamProviderLocal, StreamProviderDeepgram, StreamProviderOpenAI} -} - // AudioFormat identifies a recorded container format, sniffed from the bytes — // desktop recorders produce WAV, Termux's recorder produces M4A/AAC. type AudioFormat string diff --git a/internal/lsp/export_test.go b/internal/lsp/export_test.go new file mode 100644 index 000000000..edb66b578 --- /dev/null +++ b/internal/lsp/export_test.go @@ -0,0 +1,14 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package lsp + +import "os/exec" + +// Available reports whether a configured server for the path exists on PATH. +func Available(path string) bool { + cmd, ok := ServerFor(path) + if !ok { + return false + } + _, err := exec.LookPath(cmd[0]) + return err == nil +} diff --git a/internal/lsp/registry.go b/internal/lsp/registry.go index 0331db6fe..e9638c52b 100644 --- a/internal/lsp/registry.go +++ b/internal/lsp/registry.go @@ -1,7 +1,6 @@ package lsp import ( - "os/exec" "path/filepath" "sort" "strings" @@ -176,16 +175,6 @@ func LanguageID(path string) (string, bool) { return id, ok } -// Available reports whether a configured server for the path exists on PATH. -func Available(path string) bool { - cmd, ok := ServerFor(path) - if !ok { - return false - } - _, err := exec.LookPath(cmd[0]) - return err == nil -} - func extKey(path string) string { return strings.ToLower(filepath.Ext(path)) } diff --git a/internal/mcp/registry_test.go b/internal/mcp/registry_test.go index 597ac8aaa..d684010b7 100644 --- a/internal/mcp/registry_test.go +++ b/internal/mcp/registry_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "path/filepath" - "strings" "testing" "time" @@ -289,7 +288,7 @@ func TestRegistryToolIsDeferredEligible(t *testing.T) { // TestRegistryToolReportsMCPServerName verifies the registryTool reports its true // configured server name (not the sanitized tool-name token) so the deferred-tools -// reminder labels a multi-token server correctly via tools.DeferredLine. +// discovery label names a multi-token server correctly via tools.DeferredSource. func TestRegistryToolReportsMCPServerName(t *testing.T) { client := &fakeToolClient{} // A server name that sanitizes to a token containing an underscore ("git_hub"): @@ -305,14 +304,9 @@ func TestRegistryToolReportsMCPServerName(t *testing.T) { t.Fatalf("MCPServerName() = %q, want %q", tool.MCPServerName(), "git hub") } - // DeferredLine must prefer the reported server name over the name-derived token. - line := tools.DeferredLine(tool) - if !strings.Contains(line, "server: git hub") { - t.Fatalf("DeferredLine = %q, want it to label server as %q via MCPServerName()", line, "git hub") - } - // The truncated token-only label ("git") must NOT be the server segment. - if strings.Contains(line, "server: git |") { - t.Fatalf("DeferredLine = %q, mislabeled multi-token server with the truncated token", line) + // DeferredSource must prefer the reported server name over the name-derived token. + if source := tools.DeferredSource(tool); source != "git hub" { + t.Fatalf("DeferredSource = %q, want %q via MCPServerName()", source, "git hub") } } diff --git a/internal/modelregistry/export_test.go b/internal/modelregistry/export_test.go new file mode 100644 index 000000000..9c63b761c --- /dev/null +++ b/internal/modelregistry/export_test.go @@ -0,0 +1,21 @@ +package modelregistry + +import "sync" + +// resetModelsDevCacheForTest clears the process-level cache memoization and +// disables the overlay. +func resetModelsDevCacheForTest() { + modelsDevOnce = sync.Once{} + modelsDevCached = nil + modelsDevEnabled.Store(false) +} + +// Modes returns a copy of the preset catalog, preserving declaration order so +// list output and help text stay stable. +func Modes() []Mode { + modes := make([]Mode, len(defaultModes)) + for index, mode := range defaultModes { + modes[index] = cloneMode(mode) + } + return modes +} diff --git a/internal/modelregistry/modelsdev.go b/internal/modelregistry/modelsdev.go index 4a3b51acc..8e92b50d6 100644 --- a/internal/modelregistry/modelsdev.go +++ b/internal/modelregistry/modelsdev.go @@ -197,14 +197,6 @@ func cachedModelsDevProviders() map[string]map[string]modelsDevModel { return modelsDevCached } -// resetModelsDevCacheForTest clears the process-level cache memoization and -// disables the overlay. -func resetModelsDevCacheForTest() { - modelsDevOnce = sync.Once{} - modelsDevCached = nil - modelsDevEnabled.Store(false) -} - // RefreshModelsDevCache fetches models.dev/api.json into the on-disk cache // when the cache is missing or older than modelsDevRefreshAfter. It is safe to // call fire-and-forget from startup (use a goroutine); it never affects the diff --git a/internal/modelregistry/modes.go b/internal/modelregistry/modes.go index 67403c515..3136dbb89 100644 --- a/internal/modelregistry/modes.go +++ b/internal/modelregistry/modes.go @@ -62,16 +62,6 @@ var defaultModes = []Mode{ }, } -// Modes returns a copy of the preset catalog, preserving declaration order so -// list output and help text stay stable. -func Modes() []Mode { - modes := make([]Mode, len(defaultModes)) - for index, mode := range defaultModes { - modes[index] = cloneMode(mode) - } - return modes -} - // LookupMode returns the preset registered under name (case-insensitive, // whitespace-trimmed). The second result reports whether a preset matched. func LookupMode(name string) (Mode, bool) { diff --git a/internal/notify/export_test.go b/internal/notify/export_test.go new file mode 100644 index 000000000..cf9a928e4 --- /dev/null +++ b/internal/notify/export_test.go @@ -0,0 +1,7 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package notify + +// Enabled reports whether mode will ever emit a notification. +func Enabled(mode Mode) bool { + return mode != "" && mode != ModeOff +} diff --git a/internal/notify/notify.go b/internal/notify/notify.go index 93426c489..79906d5ac 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -167,11 +167,6 @@ func sequence(mode Mode, message string) string { } } -// Enabled reports whether mode will ever emit a notification. -func Enabled(mode Mode) bool { - return mode != "" && mode != ModeOff -} - // sanitizeMessage drops control bytes (so the message can't break the escape or // inject terminal control) and clamps to maxMessageLen runes. func sanitizeMessage(s string) string { diff --git a/internal/providercatalog/catalog.go b/internal/providercatalog/catalog.go index 9108b857d..0d6e6e726 100644 --- a/internal/providercatalog/catalog.go +++ b/internal/providercatalog/catalog.go @@ -187,14 +187,6 @@ func All() []Descriptor { return copied } -func IDs() []string { - ids := make([]string, 0, len(descriptors)) - for _, descriptor := range descriptors { - ids = append(ids, descriptor.ID) - } - return ids -} - func Get(id string) (Descriptor, bool) { normalized := NormalizeID(id) for _, descriptor := range descriptors { @@ -219,35 +211,6 @@ func Require(id string) (Descriptor, error) { return descriptor, nil } -func ListByTransport(transport Transport) []Descriptor { - normalized := Transport(NormalizeID(string(transport))) - items := make([]Descriptor, 0) - for _, descriptor := range descriptors { - if descriptor.Transport == normalized { - items = append(items, cloneDescriptor(descriptor)) - } - } - return items -} - -func ValidTransport(transport Transport) bool { - switch Transport(NormalizeID(string(transport))) { - case TransportOpenAI, TransportAnthropic, TransportGoogle, TransportBedrock, TransportVertex, TransportOpenAICompatible, TransportAnthropicCompatible: - return true - default: - return false - } -} - -func ValidAPIFormat(format APIFormat) bool { - switch format { - case APIFormatOpenAIResponses, APIFormatOpenAIChatCompletions, APIFormatAnthropicMessages, APIFormatGoogleGenerateContent, APIFormatBedrockConverse, APIFormatVertexGenerateContent: - return true - default: - return false - } -} - func NormalizeID(id string) string { var builder strings.Builder lastDash := false diff --git a/internal/providercatalog/export_test.go b/internal/providercatalog/export_test.go new file mode 100644 index 000000000..a10cfd154 --- /dev/null +++ b/internal/providercatalog/export_test.go @@ -0,0 +1,39 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package providercatalog + +func IDs() []string { + ids := make([]string, 0, len(descriptors)) + for _, descriptor := range descriptors { + ids = append(ids, descriptor.ID) + } + return ids +} + +func ListByTransport(transport Transport) []Descriptor { + normalized := Transport(NormalizeID(string(transport))) + items := make([]Descriptor, 0) + for _, descriptor := range descriptors { + if descriptor.Transport == normalized { + items = append(items, cloneDescriptor(descriptor)) + } + } + return items +} + +func ValidAPIFormat(format APIFormat) bool { + switch format { + case APIFormatOpenAIResponses, APIFormatOpenAIChatCompletions, APIFormatAnthropicMessages, APIFormatGoogleGenerateContent, APIFormatBedrockConverse, APIFormatVertexGenerateContent: + return true + default: + return false + } +} + +func ValidTransport(transport Transport) bool { + switch Transport(NormalizeID(string(transport))) { + case TransportOpenAI, TransportAnthropic, TransportGoogle, TransportBedrock, TransportVertex, TransportOpenAICompatible, TransportAnthropicCompatible: + return true + default: + return false + } +} diff --git a/internal/providers/openai/codex.go b/internal/providers/openai/codex.go index a9688f3f8..c9eba9c44 100644 --- a/internal/providers/openai/codex.go +++ b/internal/providers/openai/codex.go @@ -3,7 +3,6 @@ package openai import ( "context" "encoding/json" - "errors" "fmt" "net/http" "strings" @@ -207,14 +206,3 @@ func (p *CodexProvider) resolveAccount(ctx context.Context) (string, bool, error } return "", false, nil } - -// ValidateAccount is a convenience for tests/callers that want to confirm the -// account id is the right shape (non-empty, trimmed). It is a no-op helper -// rather than a constructor check so a Codex provider can be built before the -// first login completes. -func ValidateAccount(account string) error { - if strings.TrimSpace(account) == "" { - return errors.New("openai codex: account id is empty") - } - return nil -} diff --git a/internal/providers/openai/export_test.go b/internal/providers/openai/export_test.go new file mode 100644 index 000000000..e1a49a4d1 --- /dev/null +++ b/internal/providers/openai/export_test.go @@ -0,0 +1,18 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package openai + +import ( + "errors" + "strings" +) + +// ValidateAccount is a convenience for tests/callers that want to confirm the +// account id is the right shape (non-empty, trimmed). It is a no-op helper +// rather than a constructor check so a Codex provider can be built before the +// first login completes. +func ValidateAccount(account string) error { + if strings.TrimSpace(account) == "" { + return errors.New("openai codex: account id is empty") + } + return nil +} diff --git a/internal/providers/openai/provider_test.go b/internal/providers/openai/provider_test.go index 753df866c..b88f1714d 100644 --- a/internal/providers/openai/provider_test.go +++ b/internal/providers/openai/provider_test.go @@ -945,9 +945,9 @@ func TestStreamCompletionSurfacesLengthFinishReason(t *testing.T) { t.Fatalf("done FinishReason = %q, want %q", doneReason, zeroruntime.FinishReasonLength) } - // And it round-trips through the runtime collector as Truncated. + // And it round-trips through the runtime collector's FinishReason. collected := zeroruntime.CollectStream(context.Background(), replay(events)) - if !collected.Truncated() || collected.FinishReason != zeroruntime.FinishReasonLength { + if collected.FinishReason != zeroruntime.FinishReasonLength { t.Fatalf("collected = %+v, want truncated length", collected) } } diff --git a/internal/providers/providerio/export_test.go b/internal/providers/providerio/export_test.go new file mode 100644 index 000000000..22fda2b89 --- /dev/null +++ b/internal/providers/providerio/export_test.go @@ -0,0 +1,14 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package providerio + +import ( + "bufio" + "io" +) + +// ScanSSEData parses Server-Sent Event data fields from a streaming response. +func ScanSSEData(reader io.Reader, handle func(data string) bool) error { + scanner := bufio.NewScanner(reader) + scanner.Buffer(make([]byte, 0, 4096), maxSSELineBytes) + return scanSSEPayloads(scanner, handle, nil) +} diff --git a/internal/providers/providerio/providerio.go b/internal/providers/providerio/providerio.go index 374269f65..39f5f745b 100644 --- a/internal/providers/providerio/providerio.go +++ b/internal/providers/providerio/providerio.go @@ -195,13 +195,6 @@ func SendEvent(ctx context.Context, events chan<- zeroruntime.StreamEvent, event } } -// ScanSSEData parses Server-Sent Event data fields from a streaming response. -func ScanSSEData(reader io.Reader, handle func(data string) bool) error { - scanner := bufio.NewScanner(reader) - scanner.Buffer(make([]byte, 0, 4096), maxSSELineBytes) - return scanSSEPayloads(scanner, handle, nil) -} - // scanSSEPayloads accumulates SSE "data:" lines into payloads (joined across // continuation lines, flushed on a blank line or EOF) and forwards each to // handle. It is the shared core of ScanSSEData and the idle-aware variant. diff --git a/internal/reasoning/capability.go b/internal/reasoning/capability.go deleted file mode 100644 index bd681aef6..000000000 --- a/internal/reasoning/capability.go +++ /dev/null @@ -1,147 +0,0 @@ -// Package reasoning models how each provider's models expose reasoning-effort -// control. Providers disagree on the concept — OpenAI uses a discrete effort -// enum, Anthropic and Gemini 2.5 use a thinking-token budget, some models only -// toggle thinking on/off — so a single flat effort string cannot describe them. -// -// Capability is the typed, per-model description of that control, sourced from a -// community capability catalog (models.dev). It is the data the rest of Zero -// consults to decide which reasoning tiers a model actually supports, replacing -// model-name guessing. This package depends only on the standard library, so the -// model registry and provider adapters can import it without a cycle. -// -// Source of truth: this catalog is intended to become the authoritative reasoning -// -capability source. When it is wired into the live path, modelregistry's -// name-pattern inference (reasoningEffortsForModelName) is demoted to a fallback -// for models the catalog does not cover; the catalog wins where it has an entry. -// This commit is additive groundwork — nothing consumes it yet — so there is no -// behavioral divergence until that wiring lands. -package reasoning - -import "strings" - -// ControlKind enumerates how a model exposes reasoning control. The values -// mirror the models.dev reasoning_options[].type field. -type ControlKind string - -const ( - // ControlEffort is a discrete effort enum (OpenAI reasoning_effort, Gemini 3 - // thinkingLevel, newer Claude output_config.effort). - ControlEffort ControlKind = "effort" - // ControlBudget is a thinking-token budget (Gemini 2.5 thinkingBudget, legacy - // Claude thinking.budget_tokens). - ControlBudget ControlKind = "budget_tokens" - // ControlToggle is an on/off thinking switch with no levels. - ControlToggle ControlKind = "toggle" -) - -// Control is one reasoning control a model accepts. For an effort control, -// Values lists the accepted tiers ordered weakest to strongest. For a budget -// control, Min/Max bound the thinking-token budget; each is nil when the -// provider does not bound that side, kept distinct from an explicit 0 (Gemini -// uses min: 0 to mean "thinking can be disabled", which a plain int would lose). -type Control struct { - Kind ControlKind `json:"type"` - Values []string `json:"values,omitempty"` - Min *int `json:"min,omitempty"` - Max *int `json:"max,omitempty"` -} - -// clone returns a deep copy of the control so the embedded catalog cannot be -// mutated through a returned Control's slice or budget pointers. -func (ctrl Control) clone() Control { - out := Control{Kind: ctrl.Kind} - if ctrl.Values != nil { - out.Values = append([]string(nil), ctrl.Values...) - } - if ctrl.Min != nil { - min := *ctrl.Min - out.Min = &min - } - if ctrl.Max != nil { - max := *ctrl.Max - out.Max = &max - } - return out -} - -// Capability is the reasoning capability of a single model: whether it reasons -// at all, and through which controls. A model may carry more than one control -// (e.g. a newer Claude model exposes both an effort enum and a token budget); a -// reasoning model with no controls reasons but exposes no knob (always-on). -type Capability struct { - Reasoning bool `json:"reasoning"` - Controls []Control `json:"reasoning_options,omitempty"` -} - -// clone returns a deep copy of the capability, cloning each control, so a caller -// cannot mutate the shared catalog through the returned value's slices. -func (c Capability) clone() Capability { - out := Capability{Reasoning: c.Reasoning} - if c.Controls != nil { - out.Controls = make([]Control, len(c.Controls)) - for i, ctrl := range c.Controls { - out.Controls[i] = ctrl.clone() - } - } - return out -} - -// Supported reports whether the model performs any reasoning. -func (c Capability) Supported() bool { return c.Reasoning } - -// EffortControl returns the model's effort control and whether it has one. The -// returned Control is a deep copy, so a caller cannot mutate the shared catalog -// through its Values slice. -func (c Capability) EffortControl() (Control, bool) { - for _, ctrl := range c.Controls { - if ctrl.Kind == ControlEffort { - return ctrl.clone(), true - } - } - return Control{}, false -} - -// EffortValues returns the ordered effort tiers the model accepts, or nil when -// it has no effort control (a budget- or toggle-only model, or a non-reasoning -// model). -func (c Capability) EffortValues() []string { - if ctrl, ok := c.EffortControl(); ok { - return ctrl.Values - } - return nil -} - -// SupportsEffort reports whether tier is one of the model's accepted effort -// values (case-insensitive). -func (c Capability) SupportsEffort(tier string) bool { - tier = strings.ToLower(strings.TrimSpace(tier)) - for _, v := range c.EffortValues() { - if strings.ToLower(v) == tier { - return true - } - } - return false -} - -// BudgetControl returns the model's token-budget control and whether it has one. -// The returned Control is a deep copy: its Min/Max are independent pointers, so a -// caller cannot mutate the shared catalog through them. Min/Max are nil when that -// bound is unspecified (a real 0, e.g. Gemini's min: 0, is a non-nil pointer to 0). -func (c Capability) BudgetControl() (Control, bool) { - for _, ctrl := range c.Controls { - if ctrl.Kind == ControlBudget { - return ctrl.clone(), true - } - } - return Control{}, false -} - -// HasControl reports whether the model exposes a control of the given kind. -func (c Capability) HasControl(kind ControlKind) bool { - for _, ctrl := range c.Controls { - if ctrl.Kind == kind { - return true - } - } - return false -} diff --git a/internal/reasoning/catalog.go b/internal/reasoning/catalog.go deleted file mode 100644 index 763eebfd0..000000000 --- a/internal/reasoning/catalog.go +++ /dev/null @@ -1,99 +0,0 @@ -package reasoning - -import ( - _ "embed" - "encoding/json" - "fmt" - "strings" -) - -// modelsdev_snapshot.json is a trimmed snapshot of https://models.dev/api.json: -// for a set of first-party providers, each model's api id mapped to its -// reasoning capability (the `reasoning` flag and `reasoning_options`). Routers -// (OpenRouter/Azure/etc.) are intentionally excluded — they carry generic or -// stale option lists, so a lookup resolves to the first-party provider instead. -// -// Provenance is recorded in the snapshot's `_source`/`_fetched` header fields. -// To regenerate: fetch api.json, then for each provider in -// {anthropic, openai, google, xai, groq, deepseek} keep `{reasoning, -// reasoning_options}` per api id, and bump `_fetched`. Runtime auto-refresh is a -// separate change. -// -//go:embed modelsdev_snapshot.json -var snapshotBytes []byte - -// snapshotDoc is the on-disk shape of the embedded snapshot. -type snapshotDoc struct { - Providers map[string]map[string]Capability `json:"providers"` -} - -// Catalog is a reasoning-capability lookup keyed by provider slug and api model -// id. It is read-only after construction. -type Catalog struct { - byProvider map[string]map[string]Capability -} - -// ParseCatalog builds a Catalog from a models.dev-style snapshot document. -func ParseCatalog(data []byte) (Catalog, error) { - var doc snapshotDoc - if err := json.Unmarshal(data, &doc); err != nil { - return Catalog{}, fmt.Errorf("reasoning: parse catalog: %w", err) - } - if len(doc.Providers) == 0 { - return Catalog{}, fmt.Errorf("reasoning: catalog has no providers") - } - return Catalog{byProvider: doc.Providers}, nil -} - -// embedded is the catalog parsed from the bundled snapshot. A malformed embed is -// a build/release error, so it panics at init rather than failing silently. -var embedded = mustParseEmbedded() - -func mustParseEmbedded() Catalog { - c, err := ParseCatalog(snapshotBytes) - if err != nil { - panic("reasoning: invalid embedded snapshot: " + err.Error()) - } - return c -} - -// Embedded returns the catalog parsed from the bundled models.dev snapshot. -func Embedded() Catalog { return embedded } - -// Lookup returns the reasoning capability for a model identified by its Zero -// provider kind and the API model id — the provider's wire name (e.g. -// "claude-opus-4-1-20250805"), NOT the friendly registry id ("claude-opus-4.1"). -// The match is exact: the api model id is only trimmed, not case-folded, and no -// router prefixes ("openai/…") or suffixes (":cloud") are stripped. ok is false -// when the provider or model is not in the snapshot, in which case the caller -// falls back to its next capability source. -func (c Catalog) Lookup(provider, apiModel string) (Capability, bool) { - apiModel = strings.TrimSpace(apiModel) - if apiModel == "" { - return Capability{}, false - } - for _, slug := range providerSlugs(provider) { - models, ok := c.byProvider[slug] - if !ok { - continue - } - if entry, ok := models[apiModel]; ok { - // Deep copy so callers cannot mutate the shared catalog through the - // returned Controls / Values slices or budget pointers. - return entry.clone(), true - } - } - return Capability{}, false -} - -// providerSlugs maps a Zero provider kind to the models.dev provider slug to -// look up. Gemini and Google both resolve to "google" (AI Studio), the -// first-party source for Gemini models. -func providerSlugs(provider string) []string { - switch strings.ToLower(strings.TrimSpace(provider)) { - case "gemini": - return []string{"google"} - default: - return []string{strings.ToLower(strings.TrimSpace(provider))} - } -} diff --git a/internal/reasoning/catalog_test.go b/internal/reasoning/catalog_test.go deleted file mode 100644 index f1f3890cf..000000000 --- a/internal/reasoning/catalog_test.go +++ /dev/null @@ -1,249 +0,0 @@ -package reasoning - -import "testing" - -func TestEmbeddedCatalogLoads(t *testing.T) { - c := Embedded() - if len(c.byProvider) == 0 { - t.Fatal("embedded catalog is empty") - } - if _, ok := c.Lookup("openai", "gpt-5"); !ok { - t.Fatal("embedded catalog missing a well-known model (gpt-5)") - } -} - -func TestParseCatalogRejectsEmpty(t *testing.T) { - if _, err := ParseCatalog([]byte(`{}`)); err == nil { - t.Fatal("expected an error for a catalog with no providers") - } - if _, err := ParseCatalog([]byte(`not json`)); err == nil { - t.Fatal("expected an error for malformed json") - } -} - -func TestGroundTruthOpenAI(t *testing.T) { - c := Embedded() - cases := []struct { - api string - reason bool - kind ControlKind - values []string - }{ - {"gpt-5", true, ControlEffort, []string{"minimal", "low", "medium", "high"}}, - {"gpt-5-codex", true, ControlEffort, []string{"low", "medium", "high"}}, - {"gpt-5-pro", true, ControlEffort, []string{"high"}}, - {"gpt-5.1", true, ControlEffort, []string{"none", "low", "medium", "high"}}, - {"gpt-5.1-codex-max", true, ControlEffort, []string{"low", "medium", "high", "xhigh"}}, - {"o3", true, ControlEffort, []string{"low", "medium", "high"}}, - } - for _, tc := range cases { - entry, ok := c.Lookup("openai", tc.api) - if !ok { - t.Errorf("%s: not found", tc.api) - continue - } - if entry.Supported() != tc.reason { - t.Errorf("%s: Supported=%v want %v", tc.api, entry.Supported(), tc.reason) - } - if !equalStrings(entry.EffortValues(), tc.values) { - t.Errorf("%s: efforts=%v want %v", tc.api, entry.EffortValues(), tc.values) - } - } - - // Non-reasoning models report reasoning:false and expose no effort. - for _, api := range []string{"gpt-4o", "gpt-4o-mini", "gpt-4.1", "gpt-4-turbo"} { - entry, ok := c.Lookup("openai", api) - if !ok { - t.Errorf("%s: not found", api) - continue - } - if entry.Supported() { - t.Errorf("%s: Supported=true, want false (non-reasoning)", api) - } - if entry.EffortValues() != nil { - t.Errorf("%s: efforts=%v, want nil", api, entry.EffortValues()) - } - } - - // Version splits: minimal vs none are not interchangeable. - gpt5, _ := c.Lookup("openai", "gpt-5") - if !gpt5.SupportsEffort("minimal") || gpt5.SupportsEffort("none") { - t.Error("gpt-5 should support minimal, not none") - } - gpt51, _ := c.Lookup("openai", "gpt-5.1") - if gpt51.SupportsEffort("minimal") || !gpt51.SupportsEffort("none") { - t.Error("gpt-5.1 should support none, not minimal") - } -} - -func TestGroundTruthAnthropic(t *testing.T) { - c := Embedded() - - // Legacy / budget-only models: a thinking-token budget, no effort enum. The - // budget carries an explicit min (1024) and no max — Max stays nil. - for _, api := range []string{ - "claude-opus-4-1-20250805", "claude-sonnet-4-5-20250929", "claude-haiku-4-5-20251001", - } { - entry, ok := c.Lookup("anthropic", api) - if !ok { - t.Errorf("%s: not found", api) - continue - } - ctrl, ok := entry.BudgetControl() - if !ok { - t.Errorf("%s: expected a budget control", api) - continue - } - if ctrl.Min == nil || *ctrl.Min != 1024 { - t.Errorf("%s: budget Min=%v, want non-nil 1024", api, ctrl.Min) - } - if ctrl.Max != nil { - t.Errorf("%s: budget Max=%d, want nil (unbounded)", api, *ctrl.Max) - } - if _, ok := entry.EffortControl(); ok { - t.Errorf("%s: did not expect an effort control", api) - } - } - - // Newer models: native effort enum with xhigh and the budget control removed. - opus48, ok := c.Lookup("anthropic", "claude-opus-4-8") - if !ok { - t.Fatal("claude-opus-4-8 not found") - } - if !equalStrings(opus48.EffortValues(), []string{"low", "medium", "high", "xhigh", "max"}) { - t.Errorf("opus-4-8 efforts=%v", opus48.EffortValues()) - } - if _, ok := opus48.BudgetControl(); ok { - t.Error("opus-4-8 should not carry a budget control (removed)") - } -} - -func TestGroundTruthGemini(t *testing.T) { - c := Embedded() - - pro, ok := c.Lookup("gemini", "gemini-2.5-pro") // gemini kind -> google slug - if !ok { - t.Fatal("gemini-2.5-pro not found via the gemini provider kind") - } - ctrl, ok := pro.BudgetControl() - if !ok || ctrl.Min == nil || *ctrl.Min != 128 || ctrl.Max == nil || *ctrl.Max != 32768 { - t.Errorf("gemini-2.5-pro budget=%+v ok=%v, want min=128 max=32768", ctrl, ok) - } - if _, ok := pro.EffortControl(); ok { - t.Error("gemini-2.5-pro should be budget-only, no effort control") - } - - // gemini-2.5-flash can disable thinking: a toggle plus a budget whose explicit - // min: 0 must survive (a plain int + omitempty would drop it). - flash, _ := c.Lookup("google", "gemini-2.5-flash") - if !flash.HasControl(ControlToggle) { - t.Error("gemini-2.5-flash should expose a toggle (thinking can be disabled)") - } - if fctrl, ok := flash.BudgetControl(); !ok || fctrl.Min == nil || *fctrl.Min != 0 { - t.Errorf("gemini-2.5-flash budget Min=%v, want a non-nil pointer to 0", fctrl.Min) - } - - // Gemini 3.x switched to an effort enum. - if g3, ok := c.Lookup("google", "gemini-3-pro-preview"); ok { - if _, ok := g3.EffortControl(); !ok { - t.Error("gemini-3-pro-preview should expose an effort control") - } - } -} - -// TestCoversZeroShippedReasoningModels pins that every reasoning model Zero -// currently ships resolves in the catalog (by its api model id), so the -// models.dev fallback actually covers Zero's catalog rather than just well-known -// ids. The api ids mirror internal/modelregistry's curated entries. -func TestCoversZeroShippedReasoningModels(t *testing.T) { - c := Embedded() - shipped := []struct { - provider, api string - wantKind ControlKind - }{ - {"anthropic", "claude-opus-4-1-20250805", ControlBudget}, - {"anthropic", "claude-sonnet-4-5-20250929", ControlBudget}, - {"anthropic", "claude-haiku-4-5-20251001", ControlBudget}, - {"google", "gemini-2.5-pro", ControlBudget}, - {"google", "gemini-2.5-flash", ControlToggle}, - {"google", "gemini-2.5-flash-lite", ControlToggle}, - } - for _, m := range shipped { - entry, ok := c.Lookup(m.provider, m.api) - if !ok { - t.Errorf("%s/%s: not covered by the snapshot", m.provider, m.api) - continue - } - if !entry.Supported() { - t.Errorf("%s/%s: Supported=false, want a reasoning model", m.provider, m.api) - } - if !entry.HasControl(m.wantKind) { - t.Errorf("%s/%s: missing expected control %q (controls=%+v)", m.provider, m.api, m.wantKind, entry.Controls) - } - } -} - -// TestLookupReturnsDeepCopy pins that a caller cannot corrupt the shared -// embedded catalog by mutating a returned Capability's slices. -func TestLookupReturnsDeepCopy(t *testing.T) { - c := Embedded() - first, ok := c.Lookup("openai", "gpt-5") - if !ok || len(first.Controls) == 0 || len(first.Controls[0].Values) == 0 { - t.Fatal("setup: gpt-5 should have an effort control with values") - } - first.Controls[0].Values[0] = "MUTATED" - first.Controls[0].Kind = "MUTATED" - - second, _ := c.Lookup("openai", "gpt-5") - if second.Controls[0].Values[0] == "MUTATED" || second.Controls[0].Kind == "MUTATED" { - t.Error("Lookup leaked a shared reference; a caller's mutation reached the catalog") - } -} - -// TestAccessorsReturnDeepCopies pins that EffortControl/BudgetControl hand back -// independent copies, so a caller holding a Capability that shares state with the -// catalog cannot corrupt it by mutating an accessor result's slice or pointers. -func TestAccessorsReturnDeepCopies(t *testing.T) { - min := 100 - c := Capability{Reasoning: true, Controls: []Control{ - {Kind: ControlEffort, Values: []string{"low", "high"}}, - {Kind: ControlBudget, Min: &min}, - }} - - eff, _ := c.EffortControl() - eff.Values[0] = "MUTATED" - if c.Controls[0].Values[0] != "low" { - t.Error("EffortControl leaked the Values slice") - } - - bud, _ := c.BudgetControl() - *bud.Min = 999 - if *c.Controls[1].Min != 100 { - t.Error("BudgetControl leaked the Min pointer") - } -} - -func TestLookupMissFallsThrough(t *testing.T) { - c := Embedded() - if _, ok := c.Lookup("openai", "totally-made-up-model"); ok { - t.Error("unknown model should miss") - } - if _, ok := c.Lookup("no-such-provider", "gpt-5"); ok { - t.Error("unknown provider should miss") - } - if _, ok := c.Lookup("openai", " "); ok { - t.Error("blank api model should miss") - } -} - -func equalStrings(a, b []string) bool { - if len(a) != len(b) { - return false - } - for i := range a { - if a[i] != b[i] { - return false - } - } - return true -} diff --git a/internal/reasoning/modelsdev_snapshot.json b/internal/reasoning/modelsdev_snapshot.json deleted file mode 100644 index 9c7c705be..000000000 --- a/internal/reasoning/modelsdev_snapshot.json +++ /dev/null @@ -1,1082 +0,0 @@ -{ - "_source": "models.dev/api.json", - "_fetched": "2026-06-28", - "_note": "Trimmed snapshot: per-provider apiModel -> {reasoning, reasoning_options}. First-party providers only (anthropic, openai, google, xai, groq, deepseek); routers excluded. See internal/reasoning/catalog.go for the regeneration recipe.", - "providers": { - "anthropic": { - "claude-opus-4-5": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - }, - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-haiku-4-5-20251001": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-opus-4-0": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-3-opus-20240229": { - "reasoning": false - }, - "claude-opus-4-1-20250805": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-sonnet-4-5": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-opus-4-7": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high", - "xhigh", - "max" - ] - } - ] - }, - "claude-opus-4-5-20251101": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - }, - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-3-5-sonnet-20241022": { - "reasoning": false - }, - "claude-opus-4-8": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high", - "xhigh", - "max" - ] - } - ] - }, - "claude-opus-4-20250514": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-3-5-sonnet-20240620": { - "reasoning": false - }, - "claude-sonnet-4-20250514": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-opus-4-1": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-3-haiku-20240307": { - "reasoning": false - }, - "claude-fable-5": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high", - "xhigh", - "max" - ] - } - ] - }, - "claude-sonnet-4-0": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-3-7-sonnet-20250219": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-haiku-4-5": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-opus-4-6": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high", - "max" - ] - }, - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-sonnet-4-5-20250929": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 1024 - } - ] - }, - "claude-3-sonnet-20240229": { - "reasoning": false - }, - "claude-sonnet-4-6": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high", - "max" - ] - }, - { - "type": "budget_tokens", - "min": 1024 - } - ] - } - }, - "openai": { - "o3": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "text-embedding-3-large": { - "reasoning": false - }, - "gpt-5.2-pro": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "medium", - "high", - "xhigh" - ] - } - ] - }, - "gpt-5": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "minimal", - "low", - "medium", - "high" - ] - } - ] - }, - "gpt-3.5-turbo": { - "reasoning": false - }, - "gpt-5-pro": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "high" - ] - } - ] - }, - "gpt-4o": { - "reasoning": false - }, - "gpt-4": { - "reasoning": false - }, - "o4-mini": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "o3-pro": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "chatgpt-image-latest": { - "reasoning": false - }, - "gpt-4o-2024-05-13": { - "reasoning": false - }, - "gpt-5.4-nano": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "none", - "low", - "medium", - "high", - "xhigh" - ] - } - ] - }, - "gpt-5-chat-latest": { - "reasoning": true - }, - "gpt-5.1-codex": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "gpt-5.3-codex-spark": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "none", - "low", - "medium", - "high", - "xhigh" - ] - } - ] - }, - "gpt-5.1-codex-max": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high", - "xhigh" - ] - } - ] - }, - "gpt-5.3-chat-latest": { - "reasoning": false - }, - "gpt-4o-2024-08-06": { - "reasoning": false - }, - "text-embedding-ada-002": { - "reasoning": false - }, - "o3-mini": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "gpt-5.2": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "none", - "low", - "medium", - "high", - "xhigh" - ] - } - ] - }, - "gpt-5.3-codex": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "none", - "low", - "medium", - "high", - "xhigh" - ] - } - ] - }, - "text-embedding-3-small": { - "reasoning": false - }, - "gpt-5.1-codex-mini": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "gpt-5.1-chat-latest": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "medium" - ] - } - ] - }, - "gpt-5.2-chat-latest": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "medium" - ] - } - ] - }, - "o4-mini-deep-research": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "medium" - ] - } - ] - }, - "gpt-image-1.5": { - "reasoning": false - }, - "gpt-4.1-nano": { - "reasoning": false - }, - "gpt-4o-2024-11-20": { - "reasoning": false - }, - "o1": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "o1-pro": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "gpt-5.4": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "none", - "low", - "medium", - "high", - "xhigh" - ] - } - ] - }, - "gpt-5.4-mini": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "none", - "low", - "medium", - "high", - "xhigh" - ] - } - ] - }, - "gpt-4.1": { - "reasoning": false - }, - "o3-deep-research": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "medium" - ] - } - ] - }, - "gpt-5-mini": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "minimal", - "low", - "medium", - "high" - ] - } - ] - }, - "gpt-image-1": { - "reasoning": false - }, - "gpt-4.1-mini": { - "reasoning": false - }, - "gpt-4-turbo": { - "reasoning": false - }, - "gpt-image-1-mini": { - "reasoning": false - }, - "gpt-5-nano": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "minimal", - "low", - "medium", - "high" - ] - } - ] - }, - "gpt-5.4-pro": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "medium", - "high", - "xhigh" - ] - } - ] - }, - "gpt-5.5-pro": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "medium", - "high", - "xhigh" - ] - } - ] - }, - "gpt-4o-mini": { - "reasoning": false - }, - "gpt-5-codex": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "gpt-5.2-codex": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high", - "xhigh" - ] - } - ] - }, - "gpt-image-2": { - "reasoning": false - }, - "gpt-5.1": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "none", - "low", - "medium", - "high" - ] - } - ] - }, - "gpt-5.5": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "none", - "low", - "medium", - "high", - "xhigh" - ] - } - ] - } - }, - "google": { - "gemini-3.1-flash-lite": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "minimal", - "low", - "medium", - "high" - ] - } - ] - }, - "gemini-2.5-flash-preview-tts": { - "reasoning": false - }, - "gemini-2.5-pro": { - "reasoning": true, - "reasoning_options": [ - { - "type": "budget_tokens", - "min": 128, - "max": 32768 - } - ] - }, - "gemini-2.5-flash": { - "reasoning": true, - "reasoning_options": [ - { - "type": "toggle" - }, - { - "type": "budget_tokens", - "min": 0, - "max": 24576 - } - ] - }, - "gemini-3.5-flash": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "minimal", - "low", - "medium", - "high" - ] - } - ] - }, - "gemma-4-31b-it": { - "reasoning": true, - "reasoning_options": [ - { - "type": "toggle" - } - ] - }, - "gemini-2.0-flash": { - "reasoning": false - }, - "gemini-embedding-001": { - "reasoning": false - }, - "gemini-3.1-pro-preview-customtools": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "gemini-flash-lite-latest": { - "reasoning": true, - "reasoning_options": [ - { - "type": "toggle" - }, - { - "type": "budget_tokens", - "min": 512, - "max": 24576 - } - ] - }, - "gemini-3-pro-image-preview": { - "reasoning": true - }, - "gemini-2.5-flash-image": { - "reasoning": true - }, - "gemini-2.5-flash-lite": { - "reasoning": true, - "reasoning_options": [ - { - "type": "toggle" - }, - { - "type": "budget_tokens", - "min": 512, - "max": 24576 - } - ] - }, - "gemini-3.1-flash-image-preview": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "minimal", - "high" - ] - } - ] - }, - "gemini-3.1-pro-preview": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "gemma-4-26b-a4b-it": { - "reasoning": true, - "reasoning_options": [ - { - "type": "toggle" - } - ] - }, - "gemini-3-pro-preview": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "high" - ] - } - ] - }, - "gemini-3-flash-preview": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "minimal", - "low", - "medium", - "high" - ] - } - ] - }, - "gemini-2.5-pro-preview-tts": { - "reasoning": false - }, - "gemini-flash-latest": { - "reasoning": true, - "reasoning_options": [ - { - "type": "toggle" - }, - { - "type": "budget_tokens", - "min": 0, - "max": 24576 - } - ] - }, - "gemini-3.1-flash-lite-preview": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "minimal", - "low", - "medium", - "high" - ] - } - ] - }, - "gemini-2.0-flash-lite": { - "reasoning": false - } - }, - "xai": { - "grok-4.20-multi-agent-0309": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high", - "xhigh" - ] - } - ] - }, - "grok-4.20-0309-non-reasoning": { - "reasoning": false - }, - "grok-4.3": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "none", - "low", - "medium", - "high" - ] - } - ] - }, - "grok-imagine-image-quality": { - "reasoning": false - }, - "grok-imagine-video": { - "reasoning": false - }, - "grok-4.20-0309-reasoning": { - "reasoning": true - }, - "grok-imagine-image": { - "reasoning": false - }, - "grok-build-0.1": { - "reasoning": true - } - }, - "groq": { - "llama-3.3-70b-versatile": { - "reasoning": false - }, - "llama-3.1-8b-instant": { - "reasoning": false - }, - "whisper-large-v3-turbo": { - "reasoning": false - }, - "whisper-large-v3": { - "reasoning": false - }, - "meta-llama/llama-prompt-guard-2-86m": { - "reasoning": false - }, - "meta-llama/llama-prompt-guard-2-22m": { - "reasoning": false - }, - "meta-llama/llama-4-scout-17b-16e-instruct": { - "reasoning": false - }, - "openai/gpt-oss-safeguard-20b": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "openai/gpt-oss-120b": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "openai/gpt-oss-20b": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "low", - "medium", - "high" - ] - } - ] - }, - "canopylabs/orpheus-v1-english": { - "reasoning": false - }, - "canopylabs/orpheus-arabic-saudi": { - "reasoning": false - }, - "groq/compound": { - "reasoning": false - }, - "groq/compound-mini": { - "reasoning": false - }, - "qwen/qwen3-32b": { - "reasoning": true, - "reasoning_options": [ - { - "type": "effort", - "values": [ - "none", - "default" - ] - } - ] - } - }, - "deepseek": { - "deepseek-v4-flash": { - "reasoning": true, - "reasoning_options": [ - { - "type": "toggle" - }, - { - "type": "effort", - "values": [ - "high", - "max" - ] - } - ] - }, - "deepseek-v4-pro": { - "reasoning": true, - "reasoning_options": [ - { - "type": "toggle" - }, - { - "type": "effort", - "values": [ - "high", - "max" - ] - } - ] - }, - "deepseek-reasoner": { - "reasoning": true - }, - "deepseek-chat": { - "reasoning": false - } - } - } -} \ No newline at end of file diff --git a/internal/reltime/reltime.go b/internal/reltime/reltime.go index a04c566a4..3278c1f2b 100644 --- a/internal/reltime/reltime.go +++ b/internal/reltime/reltime.go @@ -1,65 +1 @@ package reltime - -import "fmt" - -// RelTime renders a duration in seconds as a human-readable relative-time -// string. This is the ORIGINAL implementation, kept verbatim so -// characterization tests can be pinned against it before refactoring. -func RelTime(d int) string { - if d < 0 { - d = -d - if d < 60 { - if d == 1 { - return "1 second ago" - } - return fmt.Sprintf("%d seconds ago", d) - } - if d < 3600 { - m := d / 60 - if m == 1 { - return "1 minute ago" - } - return fmt.Sprintf("%d minutes ago", m) - } - if d < 86400 { - h := d / 3600 - if h == 1 { - return "1 hour ago" - } - return fmt.Sprintf("%d hours ago", h) - } - x := d / 86400 - if x == 1 { - return "1 day ago" - } - return fmt.Sprintf("%d days ago", x) - } else if d == 0 { - return "just now" - } else { - if d < 60 { - if d == 1 { - return "in 1 second" - } - return fmt.Sprintf("in %d seconds", d) - } - if d < 3600 { - m := d / 60 - if m == 1 { - return "in 1 minute" - } - return fmt.Sprintf("in %d minutes", m) - } - if d < 86400 { - h := d / 3600 - if h == 1 { - return "in 1 hour" - } - return fmt.Sprintf("in %d hours", h) - } - x := d / 86400 - if x == 1 { - return "in 1 day" - } - return fmt.Sprintf("in %d days", x) - } -} diff --git a/internal/repomap/export_test.go b/internal/repomap/export_test.go new file mode 100644 index 000000000..432d1cef8 --- /dev/null +++ b/internal/repomap/export_test.go @@ -0,0 +1,12 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package repomap + +import ( + "io/fs" + + "github.com/Gitlawb/zero/internal/workspaceindex" +) + +func handleWalkError(cleanRoot string, current string, entry fs.DirEntry, walkErr error, truncated *bool) (bool, error) { + return workspaceindex.HandleWalkError(cleanRoot, current, entry, walkErr, truncated) +} diff --git a/internal/repomap/repomap.go b/internal/repomap/repomap.go index 74d6f864f..dd2b2e2b4 100644 --- a/internal/repomap/repomap.go +++ b/internal/repomap/repomap.go @@ -2,7 +2,6 @@ package repomap import ( - "io/fs" "sort" "strings" @@ -82,10 +81,6 @@ func Scan(root string, options Options) (Snapshot, error) { return snapshot, err } -func handleWalkError(cleanRoot string, current string, entry fs.DirEntry, walkErr error, truncated *bool) (bool, error) { - return workspaceindex.HandleWalkError(cleanRoot, current, entry, walkErr, truncated) -} - func fileDepth(rel string) int { return workspaceindex.FileDepth(rel) } diff --git a/internal/review/export_test.go b/internal/review/export_test.go new file mode 100644 index 000000000..78d973931 --- /dev/null +++ b/internal/review/export_test.go @@ -0,0 +1,11 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package review + +func HasBlockingChecks(checks []Check) bool { + for _, check := range checks { + if IsBlocking(check.Outcome) { + return true + } + } + return false +} diff --git a/internal/review/review.go b/internal/review/review.go index 15b882b2a..cf420c47d 100644 --- a/internal/review/review.go +++ b/internal/review/review.go @@ -76,15 +76,6 @@ func BuildChecksFromEnv(env map[string]string) []Check { return checks } -func HasBlockingChecks(checks []Check) bool { - for _, check := range checks { - if IsBlocking(check.Outcome) { - return true - } - } - return false -} - func IsBlocking(outcome Outcome) bool { switch outcome { case OutcomeFailure, OutcomeCancelled, OutcomeTimedOut, OutcomeActionRequired, OutcomeUnknown: diff --git a/internal/sandbox/export_test.go b/internal/sandbox/export_test.go new file mode 100644 index 000000000..cd0de0438 --- /dev/null +++ b/internal/sandbox/export_test.go @@ -0,0 +1,78 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package sandbox + +import ( + "path/filepath" + "strings" +) + +func FormatGrantList(grants []Grant) string { + return FormatGrantListWithCommandPrefixes(grants, nil) +} + +func DefaultPermissionProfile(workspaceRoot string) PermissionProfile { + return PermissionProfileFromPolicy(workspaceRoot, DefaultPolicy(), nil) +} + +// writeRoots returns the full ordered write-root list for command plans: +// the workspace root plus any granted extra roots. The single-root fallback +// only applies to engines built without a workspace root (NewEngine always +// builds a scope otherwise); it is kept as defense in depth. +func (engine *Engine) writeRoots(workspaceRoot string) []string { + var roots []string + if engine.scope != nil { + roots = engine.scope.Roots() + } else { + roots = []string{workspaceRoot} + } + // Reflect the policy's AllowWrite roots in the OS backend write binds so a + // sandboxed shell command may write where the policy grants writes. DenyWrite + // is enforced at the policy gate, and on sandbox-exec additionally as an + // explicit deny rule (see sandboxExecProfile). + policy := engine.effectivePolicy(engine.policy) + if extra := resolveWriteRootPaths(policy.AllowWrite); len(extra) > 0 { + roots = dedupeStrings(append(roots, extra...)) + } + return roots +} + +func sandboxEnvironment(policy Policy, backend BackendName, workspaceRoot string) []string { + return sandboxEnvironmentForCommand(nil, policy, backend, workspaceRoot) +} + +func sandboxExecProfile(writeRoots []string, policy Policy, denialTag string) string { + return seatbeltProfileFromPermissionProfile(seatbeltCompatibilityPermissionProfile(writeRoots, policy), policy, denialTag) +} + +func seatbeltCompatibilityPermissionProfile(writeRoots []string, policy Policy) PermissionProfile { + fs := FileSystemPolicy{ + Kind: FileSystemUnrestricted, + ReadRoots: []string{string(filepath.Separator)}, + IncludePlatformRoots: true, + AllowTemp: true, + } + if policy.EnforceWorkspace { + fs.Kind = FileSystemRestricted + fs.WriteRoots = make([]WritableRoot, 0, len(writeRoots)) + for _, root := range writeRoots { + fs.WriteRoots = append(fs.WriteRoots, WritableRoot{Root: root}) + } + } + fs.DenyRead = dedupeStrings(append(normalizeProfilePaths(policy.DenyRead), credentialDenyReadPaths(policy)...)) + fs.DenyWrite = normalizeProfilePaths(policy.DenyWrite) + return PermissionProfile{ + FileSystem: fs, + Network: NetworkPolicy{Mode: policy.Network}, + } +} + +// WindowsSandboxSetupPathForRunner derives the setup helper's path from a +// standalone command-runner path (the sibling .exe in the release layout). +// Retained for that layout; self-dispatch callers use +// ResolveWindowsSandboxSetupHelper instead. +func WindowsSandboxSetupPathForRunner(runnerPath string) string { + if strings.TrimSpace(runnerPath) == "" { + return "" + } + return filepath.Join(filepath.Dir(runnerPath), WindowsSandboxSetupName) +} diff --git a/internal/sandbox/grants.go b/internal/sandbox/grants.go index ebccc630a..46b4ca8fa 100644 --- a/internal/sandbox/grants.go +++ b/internal/sandbox/grants.go @@ -427,10 +427,6 @@ func (store *GrantStore) Clear() (int, error) { return count, nil } -func FormatGrantList(grants []Grant) string { - return FormatGrantListWithCommandPrefixes(grants, nil) -} - func FormatGrantListWithCommandPrefixes(grants []Grant, prefixes []CommandPrefixGrant) string { if len(grants) == 0 && len(prefixes) == 0 { return "No persistent sandbox grants." diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index 915bb92be..2bd186719 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -69,10 +69,6 @@ func gitMetadataWriteCarveouts(root string) []string { } } -func DefaultPermissionProfile(workspaceRoot string) PermissionProfile { - return PermissionProfileFromPolicy(workspaceRoot, DefaultPolicy(), nil) -} - func PermissionProfileFromPolicy(workspaceRoot string, policy Policy, scope *Scope) PermissionProfile { if policy.Mode == "" { policy = DefaultPolicy() diff --git a/internal/sandbox/runner.go b/internal/sandbox/runner.go index 248c0a528..5c5f5d833 100644 --- a/internal/sandbox/runner.go +++ b/internal/sandbox/runner.go @@ -143,28 +143,6 @@ func (engine *Engine) PrepareExecution(ctx context.Context, request execution.Re }, nil } -// writeRoots returns the full ordered write-root list for command plans: -// the workspace root plus any granted extra roots. The single-root fallback -// only applies to engines built without a workspace root (NewEngine always -// builds a scope otherwise); it is kept as defense in depth. -func (engine *Engine) writeRoots(workspaceRoot string) []string { - var roots []string - if engine.scope != nil { - roots = engine.scope.Roots() - } else { - roots = []string{workspaceRoot} - } - // Reflect the policy's AllowWrite roots in the OS backend write binds so a - // sandboxed shell command may write where the policy grants writes. DenyWrite - // is enforced at the policy gate, and on sandbox-exec additionally as an - // explicit deny rule (see sandboxExecProfile). - policy := engine.effectivePolicy(engine.policy) - if extra := resolveWriteRootPaths(policy.AllowWrite); len(extra) > 0 { - roots = dedupeStrings(append(roots, extra...)) - } - return roots -} - func (engine *Engine) BuildCommandPlan(spec CommandSpec) (CommandPlan, error) { if engine == nil { return directCommandPlan(spec, Backend{Name: BackendUnavailable, Message: "sandbox disabled"}, Policy{}, ""), nil @@ -453,32 +431,6 @@ func seatbeltCommandPlanWithProfile(spec CommandSpec, workspaceRoot string, prof return plan } -func seatbeltCompatibilityPermissionProfile(writeRoots []string, policy Policy) PermissionProfile { - fs := FileSystemPolicy{ - Kind: FileSystemUnrestricted, - ReadRoots: []string{string(filepath.Separator)}, - IncludePlatformRoots: true, - AllowTemp: true, - } - if policy.EnforceWorkspace { - fs.Kind = FileSystemRestricted - fs.WriteRoots = make([]WritableRoot, 0, len(writeRoots)) - for _, root := range writeRoots { - fs.WriteRoots = append(fs.WriteRoots, WritableRoot{Root: root}) - } - } - fs.DenyRead = dedupeStrings(append(normalizeProfilePaths(policy.DenyRead), credentialDenyReadPaths(policy)...)) - fs.DenyWrite = normalizeProfilePaths(policy.DenyWrite) - return PermissionProfile{ - FileSystem: fs, - Network: NetworkPolicy{Mode: policy.Network}, - } -} - -func sandboxEnvironment(policy Policy, backend BackendName, workspaceRoot string) []string { - return sandboxEnvironmentForCommand(nil, policy, backend, workspaceRoot) -} - func sandboxEnvironmentForCommand(specEnv []string, policy Policy, backend BackendName, workspaceRoot string) []string { return sandboxEnvironmentForCommandWithSensitiveEnv(specEnv, policy, backend, workspaceRoot, nil) } @@ -664,10 +616,6 @@ func sandboxMachLookupRule() string { return "(allow mach-lookup\n " + strings.Join(filters, "\n ") + ")" } -func sandboxExecProfile(writeRoots []string, policy Policy, denialTag string) string { - return seatbeltProfileFromPermissionProfile(seatbeltCompatibilityPermissionProfile(writeRoots, policy), policy, denialTag) -} - func seatbeltProfileFromPermissionProfile(profile PermissionProfile, policy Policy, denialTag string) string { networkRule := networkRuleForProfile(profile.Network) readRule := seatbeltReadRule(profile.FileSystem) diff --git a/internal/sandbox/windows_setup.go b/internal/sandbox/windows_setup.go index 0e9b93bdd..3fc9634e8 100644 --- a/internal/sandbox/windows_setup.go +++ b/internal/sandbox/windows_setup.go @@ -45,17 +45,6 @@ type WindowsSandboxSetupMarker struct { NetworkFilters int `json:"networkFilters"` } -// WindowsSandboxSetupPathForRunner derives the setup helper's path from a -// standalone command-runner path (the sibling .exe in the release layout). -// Retained for that layout; self-dispatch callers use -// ResolveWindowsSandboxSetupHelper instead. -func WindowsSandboxSetupPathForRunner(runnerPath string) string { - if strings.TrimSpace(runnerPath) == "" { - return "" - } - return filepath.Join(filepath.Dir(runnerPath), WindowsSandboxSetupName) -} - func WindowsSandboxSetupMarkerPath(sandboxHome string) string { return filepath.Join(sandboxHome, "windows-setup.json") } diff --git a/internal/sessions/checkpoint.go b/internal/sessions/checkpoint.go index 2fe0fb453..1b36b83a4 100644 --- a/internal/sessions/checkpoint.go +++ b/internal/sessions/checkpoint.go @@ -248,22 +248,6 @@ func (store *Store) readBlob(sessionID, hash string) ([]byte, error) { return content, nil } -// pruneOrphanBlobs removes blobs not referenced by any checkpoint event (e.g. after -// a rewind discards later checkpoints). Best-effort; returns count removed. It -// acquires the session lock so it cannot delete a blob that a concurrent -// CaptureToolCheckpoint has just written but not yet referenced by its event. -func (store *Store) pruneOrphanBlobs(sessionID string) (int, error) { - if !ValidSessionID(sessionID) { - return 0, fmt.Errorf("invalid zero session id %q", sessionID) - } - unlock, err := store.lockSession(sessionID) - if err != nil { - return 0, err - } - defer unlock() - return store.pruneOrphanBlobsLocked(sessionID) -} - // pruneOrphanBlobsLocked is the body of pruneOrphanBlobs WITHOUT acquiring the // session lock. The caller MUST already hold store.lockSession(sessionID). // Holding the lock around referencedBlobs + ReadDir + Remove is what closes the diff --git a/internal/sessions/export_test.go b/internal/sessions/export_test.go new file mode 100644 index 000000000..1aee19e6f --- /dev/null +++ b/internal/sessions/export_test.go @@ -0,0 +1,20 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package sessions + +import "fmt" + +// pruneOrphanBlobs removes blobs not referenced by any checkpoint event (e.g. after +// a rewind discards later checkpoints). Best-effort; returns count removed. It +// acquires the session lock so it cannot delete a blob that a concurrent +// CaptureToolCheckpoint has just written but not yet referenced by its event. +func (store *Store) pruneOrphanBlobs(sessionID string) (int, error) { + if !ValidSessionID(sessionID) { + return 0, fmt.Errorf("invalid zero session id %q", sessionID) + } + unlock, err := store.lockSession(sessionID) + if err != nil { + return 0, err + } + defer unlock() + return store.pruneOrphanBlobsLocked(sessionID) +} diff --git a/internal/sessions/replay.go b/internal/sessions/replay.go index 21e711818..915cc77f7 100644 --- a/internal/sessions/replay.go +++ b/internal/sessions/replay.go @@ -244,10 +244,6 @@ func RehydrateEvents(events []Event) ([]Event, error) { return cloneEvents(events), nil } -func ReplayEvents(events []Event) ([]Event, error) { - return RehydrateEvents(events) -} - func decodeCompactionPayload(event Event) (CompactionPayload, error) { var payload CompactionPayload if err := json.Unmarshal(event.Payload, &payload); err != nil { diff --git a/internal/skills/export_test.go b/internal/skills/export_test.go new file mode 100644 index 000000000..7847d90dd --- /dev/null +++ b/internal/skills/export_test.go @@ -0,0 +1,66 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package skills + +import "strings" + +// Info returns the named skill plus its recorded source and hash, or ok=false if +// it is not discoverable in dir. +func Info(dir string, name string) (SkillInfo, bool) { + skill, ok := Get(dir, name) + if !ok { + return SkillInfo{}, false + } + info := SkillInfo{Skill: skill} + if lock, err := ReadLock(dir); err == nil { + if entry, found := lock[skill.Name]; found { + info.Source = entry.Source + info.Hash = entry.Hash + } + } + return info, true +} + +// DiscoveryRoots returns ordered skill roots for runtime discovery: primary +// DefaultDir, optional AgentsDir when present, then pluginRoots. Empty strings +// are omitted. Earlier entries win on name clashes. +func DiscoveryRoots(env map[string]string, pluginRoots []string) []string { + return collectRoots(DefaultDir(env), AgentsDir(env), pluginRoots) +} + +// Duplicates returns the duplicate-name collisions Load resolved by the +// first-directory-wins rule, so a caller can warn the user that a shadowed skill +// was dropped. A missing directory yields no duplicates and no error. +func Duplicates(dir string) ([]DuplicateName, error) { + _, dups, err := load(dir) + return dups, err +} + +// Get loads the named skill from dir, returning false if it is not found. +func Get(dir string, name string) (Skill, bool) { + loaded, err := Load(dir) + if err != nil { + return Skill{}, false + } + target := strings.TrimSpace(name) + for _, skill := range loaded { + if skill.Name == target { + return skill, true + } + } + return Skill{}, false +} + +// List loads the skills directory and returns each skill without its (possibly +// large) Content body — handy for `zero skills` listings. +func List(dir string) ([]Skill, error) { + loaded, err := Load(dir) + if err != nil { + return nil, err + } + listed := make([]Skill, 0, len(loaded)) + for _, skill := range loaded { + skill.Content = "" + listed = append(listed, skill) + } + return listed, nil +} diff --git a/internal/skills/install.go b/internal/skills/install.go index 610fe8831..2d9f27f3c 100644 --- a/internal/skills/install.go +++ b/internal/skills/install.go @@ -232,23 +232,6 @@ func Remove(dir string, name string) error { return nil } -// Info returns the named skill plus its recorded source and hash, or ok=false if -// it is not discoverable in dir. -func Info(dir string, name string) (SkillInfo, bool) { - skill, ok := Get(dir, name) - if !ok { - return SkillInfo{}, false - } - info := SkillInfo{Skill: skill} - if lock, err := ReadLock(dir); err == nil { - if entry, found := lock[skill.Name]; found { - info.Source = entry.Source - info.Hash = entry.Hash - } - } - return info, true -} - // InfoFromRoots resolves the named skill across discovery roots (earlier roots // win). Lock source/hash are attached only when the winning skill lives under // primaryDir and that dir's lockfile has an entry — agents-only skills return diff --git a/internal/skills/skills.go b/internal/skills/skills.go index 6a7550566..11d8b0207 100644 --- a/internal/skills/skills.go +++ b/internal/skills/skills.go @@ -92,13 +92,6 @@ func AgentsDir(env map[string]string) string { return dir } -// DiscoveryRoots returns ordered skill roots for runtime discovery: primary -// DefaultDir, optional AgentsDir when present, then pluginRoots. Empty strings -// are omitted. Earlier entries win on name clashes. -func DiscoveryRoots(env map[string]string, pluginRoots []string) []string { - return collectRoots(DefaultDir(env), AgentsDir(env), pluginRoots) -} - // collectRoots assembles ordered non-empty skill roots. primary is typically // DefaultDir (or an injected test dir); agents is typically AgentsDir's result. func collectRoots(primary string, agents string, pluginRoots []string) []string { @@ -220,14 +213,6 @@ func ListFromRoots(dirs []string) ([]Skill, []DuplicateName, error) { return listed, dups, nil } -// Duplicates returns the duplicate-name collisions Load resolved by the -// first-directory-wins rule, so a caller can warn the user that a shadowed skill -// was dropped. A missing directory yields no duplicates and no error. -func Duplicates(dir string) ([]DuplicateName, error) { - _, dups, err := load(dir) - return dups, err -} - // confineSkillPath resolves manifestPath through symlinks and returns the real // path only if it stays within rootReal (the already-symlink-resolved skills // root). This stops a symlinked SKILL.md — or a symlinked skill directory — from @@ -343,36 +328,6 @@ func load(dir string) ([]Skill, []DuplicateName, error) { return skills, duplicates, nil } -// List loads the skills directory and returns each skill without its (possibly -// large) Content body — handy for `zero skills` listings. -func List(dir string) ([]Skill, error) { - loaded, err := Load(dir) - if err != nil { - return nil, err - } - listed := make([]Skill, 0, len(loaded)) - for _, skill := range loaded { - skill.Content = "" - listed = append(listed, skill) - } - return listed, nil -} - -// Get loads the named skill from dir, returning false if it is not found. -func Get(dir string, name string) (Skill, bool) { - loaded, err := Load(dir) - if err != nil { - return Skill{}, false - } - target := strings.TrimSpace(name) - for _, skill := range loaded { - if skill.Name == target { - return skill, true - } - } - return Skill{}, false -} - // parseSkill splits optional `---`-delimited frontmatter from the markdown body. // Frontmatter is a simple line parser for `name:`/`description:` keys (no YAML // dependency). Without frontmatter, Name defaults to the directory name and diff --git a/internal/specialist/export_test.go b/internal/specialist/export_test.go new file mode 100644 index 000000000..ec3441620 --- /dev/null +++ b/internal/specialist/export_test.go @@ -0,0 +1,55 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package specialist + +import ( + "bufio" + "encoding/json" + "fmt" + "io" + "strings" + + "github.com/Gitlawb/zero/internal/background" + "github.com/Gitlawb/zero/internal/streamjson" +) + +func NewOutputTool(manager *background.Manager) *OutputTool { + return &OutputTool{manager: manager} +} + +func NewStopTool(manager *background.Manager) *StopTool { + return &StopTool{manager: manager} +} + +func ParseStream(reader io.Reader) ([]streamjson.Event, error) { + // Read with a bufio.Reader rather than bufio.Scanner: a Scanner caps a single + // token at 64 KiB/1 MiB and returns bufio.ErrTooLong past it, which aborted the + // whole specialist run when a child emitted one large stream-json line (a big + // tool result or final answer). ReadString has no per-line limit; the child is + // our own trusted subprocess, so its line length is the legitimate bound. + buffered := bufio.NewReader(reader) + events := []streamjson.Event{} + lineNumber := 0 + for { + raw, readErr := buffered.ReadString('\n') + if len(raw) > 0 { + lineNumber++ + if line := strings.TrimSpace(raw); line != "" { + var event streamjson.Event + if err := json.Unmarshal([]byte(line), &event); err != nil { + return nil, fmt.Errorf("parse stream-json line %d: %w", lineNumber, err) + } + if event.Type == "" { + return nil, fmt.Errorf("parse stream-json line %d: type is required", lineNumber) + } + events = append(events, event) + } + } + if readErr != nil { + if readErr == io.EOF { + break + } + return nil, fmt.Errorf("read stream-json output: %w", readErr) + } + } + return events, nil +} diff --git a/internal/specialist/manifest_test.go b/internal/specialist/manifest_test.go index b3e90f7b9..8e2100f6d 100644 --- a/internal/specialist/manifest_test.go +++ b/internal/specialist/manifest_test.go @@ -379,9 +379,9 @@ Prompt.`) func TestKnownToolNamesMatchCoreRegistry(t *testing.T) { // web_search is only registered when a search backend is configured; set one so - // CoreTools() exposes the full set this list is meant to mirror. + // CoreToolsScoped(, nil) exposes the full set this list is meant to mirror. t.Setenv("ZERO_WEBSEARCH_BASE_URL", "https://search.example/api") - core := tools.CoreTools(t.TempDir()) + core := tools.CoreToolsScoped(t.TempDir(), nil) got := make([]string, 0, len(knownToolNames)) for name := range knownToolNames { got = append(got, name) diff --git a/internal/specialist/output_tool.go b/internal/specialist/output_tool.go index 64d6e8234..3c4dd4667 100644 --- a/internal/specialist/output_tool.go +++ b/internal/specialist/output_tool.go @@ -35,10 +35,6 @@ type outputParameters struct { TimeoutMS int } -func NewOutputTool(manager *background.Manager) *OutputTool { - return &OutputTool{manager: manager} -} - func newOutputToolWithManagerFunc(managerFunc BackgroundManagerFunc, sessionStore *sessions.Store) *OutputTool { return &OutputTool{managerFunc: managerFunc, SessionStore: sessionStore} } diff --git a/internal/specialist/stop_tool.go b/internal/specialist/stop_tool.go index 77bbcdea1..48dc4bc97 100644 --- a/internal/specialist/stop_tool.go +++ b/internal/specialist/stop_tool.go @@ -18,10 +18,6 @@ type stopParameters struct { TaskID string } -func NewStopTool(manager *background.Manager) *StopTool { - return &StopTool{manager: manager} -} - func newStopToolWithManagerFunc(managerFunc BackgroundManagerFunc) *StopTool { return &StopTool{managerFunc: managerFunc} } diff --git a/internal/specialist/streamer.go b/internal/specialist/streamer.go index ea681eb2d..f9a3a2337 100644 --- a/internal/specialist/streamer.go +++ b/internal/specialist/streamer.go @@ -1,10 +1,7 @@ package specialist import ( - "bufio" - "encoding/json" "fmt" - "io" "strings" "github.com/Gitlawb/zero/internal/streamjson" @@ -38,40 +35,6 @@ func (usage StreamUsage) EffectiveTotalTokens() int { return usage.TotalTokens } -func ParseStream(reader io.Reader) ([]streamjson.Event, error) { - // Read with a bufio.Reader rather than bufio.Scanner: a Scanner caps a single - // token at 64 KiB/1 MiB and returns bufio.ErrTooLong past it, which aborted the - // whole specialist run when a child emitted one large stream-json line (a big - // tool result or final answer). ReadString has no per-line limit; the child is - // our own trusted subprocess, so its line length is the legitimate bound. - buffered := bufio.NewReader(reader) - events := []streamjson.Event{} - lineNumber := 0 - for { - raw, readErr := buffered.ReadString('\n') - if len(raw) > 0 { - lineNumber++ - if line := strings.TrimSpace(raw); line != "" { - var event streamjson.Event - if err := json.Unmarshal([]byte(line), &event); err != nil { - return nil, fmt.Errorf("parse stream-json line %d: %w", lineNumber, err) - } - if event.Type == "" { - return nil, fmt.Errorf("parse stream-json line %d: type is required", lineNumber) - } - events = append(events, event) - } - } - if readErr != nil { - if readErr == io.EOF { - break - } - return nil, fmt.Errorf("read stream-json output: %w", readErr) - } - } - return events, nil -} - func SummarizeStream(events []streamjson.Event, processExitCode int) StreamResult { result := StreamResult{Events: append([]streamjson.Event(nil), events...), ExitCode: processExitCode} textParts := []string{} diff --git a/internal/streamjson/export_test.go b/internal/streamjson/export_test.go new file mode 100644 index 000000000..b33f839fd --- /dev/null +++ b/internal/streamjson/export_test.go @@ -0,0 +1,10 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package streamjson + +func ParsePrompt(input string) (string, error) { + events, err := ParseInput(input) + if err != nil { + return "", err + } + return ResolvePrompt(events) +} diff --git a/internal/streamjson/streamjson.go b/internal/streamjson/streamjson.go index fe5bae6ad..e9e66e218 100644 --- a/internal/streamjson/streamjson.go +++ b/internal/streamjson/streamjson.go @@ -238,14 +238,6 @@ func ResolveImages(events []InputEvent) ([]zeroruntime.ImageBlock, error) { return images, nil } -func ParsePrompt(input string) (string, error) { - events, err := ParseInput(input) - if err != nil { - return "", err - } - return ResolvePrompt(events) -} - func validateOutputEvent(event Event) error { if event.SchemaVersion != SchemaVersion { return fmt.Errorf("schemaVersion must be %d", SchemaVersion) diff --git a/internal/tools/apply_patch.go b/internal/tools/apply_patch.go index 7e84e81ba..ab6fab952 100644 --- a/internal/tools/apply_patch.go +++ b/internal/tools/apply_patch.go @@ -16,10 +16,6 @@ type applyPatchTool struct { scope PathScope } -func NewApplyPatchTool(workspaceRoot string) Tool { - return NewScopedApplyPatchTool(workspaceRoot, nil) -} - func NewScopedApplyPatchTool(workspaceRoot string, scope PathScope) Tool { return applyPatchTool{ baseTool: baseTool{ diff --git a/internal/tools/argtolerance_test.go b/internal/tools/argtolerance_test.go index ba815b996..47f54e337 100644 --- a/internal/tools/argtolerance_test.go +++ b/internal/tools/argtolerance_test.go @@ -154,7 +154,7 @@ func TestReadFileToolAcceptsPathAliases(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta") for _, key := range []string{"file", "file_path", "filepath", "filename"} { - res := NewReadFileTool(root).Run(context.Background(), map[string]any{key: "notes.txt"}) + res := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{key: "notes.txt"}) if res.Status != StatusOK { t.Fatalf("alias %q: expected ok, got %s: %s", key, res.Status, res.Output) } @@ -168,7 +168,7 @@ func TestListDirectoryToolAcceptsDirAliases(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "sub", "x.txt"), "data") for _, key := range []string{"directory", "dir"} { - res := NewListDirectoryTool(root).Run(context.Background(), map[string]any{key: "sub"}) + res := NewScopedListDirectoryTool(root, nil).Run(context.Background(), map[string]any{key: "sub"}) if res.Status != StatusOK { t.Fatalf("alias %q: expected ok, got %s: %s", key, res.Status, res.Output) } @@ -183,7 +183,7 @@ func TestGlobToolAcceptsPatternAndCwdAliases(t *testing.T) { writeTestFile(t, filepath.Join(root, "sub", "a.go"), "package sub") // pattern aliases for _, key := range []string{"glob", "match", "query", "expression"} { - res := NewGlobTool(root).Run(context.Background(), map[string]any{key: "**/*.go"}) + res := NewScopedGlobTool(root, nil).Run(context.Background(), map[string]any{key: "**/*.go"}) if res.Status != StatusOK { t.Fatalf("pattern alias %q: expected ok, got %s: %s", key, res.Status, res.Output) } @@ -193,7 +193,7 @@ func TestGlobToolAcceptsPatternAndCwdAliases(t *testing.T) { } // cwd aliases (scope the scan to sub/) for _, key := range []string{"dir", "directory", "path"} { - res := NewGlobTool(root).Run(context.Background(), map[string]any{"pattern": "*.go", key: "sub"}) + res := NewScopedGlobTool(root, nil).Run(context.Background(), map[string]any{"pattern": "*.go", key: "sub"}) if res.Status != StatusOK { t.Fatalf("cwd alias %q: expected ok, got %s: %s", key, res.Status, res.Output) } @@ -207,7 +207,7 @@ func TestGrepToolAcceptsPatternAndPathAliases(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "sub", "main.go"), "func main() {}\n") for _, key := range []string{"query", "regex", "search", "expression"} { - res := NewGrepTool(root).Run(context.Background(), map[string]any{key: "func main"}) + res := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{key: "func main"}) if res.Status != StatusOK { t.Fatalf("pattern alias %q: expected ok, got %s: %s", key, res.Status, res.Output) } @@ -216,7 +216,7 @@ func TestGrepToolAcceptsPatternAndPathAliases(t *testing.T) { } } for _, key := range []string{"dir", "directory"} { - res := NewGrepTool(root).Run(context.Background(), map[string]any{"pattern": "func main", key: "sub"}) + res := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{"pattern": "func main", key: "sub"}) if res.Status != StatusOK { t.Fatalf("path alias %q: expected ok, got %s: %s", key, res.Status, res.Output) } @@ -233,7 +233,7 @@ func TestOptionalPathArgsTreatEmptyAsDefault(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "main.go"), "func main() {}\n") - grepRes := NewGrepTool(root).Run(context.Background(), map[string]any{"pattern": "func main", "path": ""}) + grepRes := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{"pattern": "func main", "path": ""}) if grepRes.Status != StatusOK { t.Fatalf("grep path:\"\" expected ok, got %s: %s", grepRes.Status, grepRes.Output) } @@ -241,7 +241,7 @@ func TestOptionalPathArgsTreatEmptyAsDefault(t *testing.T) { t.Fatalf("grep path:\"\" expected hit, got %q", grepRes.Output) } - globRes := NewGlobTool(root).Run(context.Background(), map[string]any{"pattern": "**/*.go", "cwd": ""}) + globRes := NewScopedGlobTool(root, nil).Run(context.Background(), map[string]any{"pattern": "**/*.go", "cwd": ""}) if globRes.Status != StatusOK { t.Fatalf("glob cwd:\"\" expected ok, got %s: %s", globRes.Status, globRes.Output) } @@ -249,7 +249,7 @@ func TestOptionalPathArgsTreatEmptyAsDefault(t *testing.T) { t.Fatalf("glob cwd:\"\" expected match, got %q", globRes.Output) } - listRes := NewListDirectoryTool(root).Run(context.Background(), map[string]any{"path": ""}) + listRes := NewScopedListDirectoryTool(root, nil).Run(context.Background(), map[string]any{"path": ""}) if listRes.Status != StatusOK { t.Fatalf("list_directory path:\"\" expected ok, got %s: %s", listRes.Status, listRes.Output) } @@ -261,7 +261,7 @@ func TestOptionalPathArgsTreatEmptyAsDefault(t *testing.T) { func TestWriteFileToolAcceptsPathAliases(t *testing.T) { root := t.TempDir() for _, key := range []string{"file", "file_path", "filename"} { - res := NewWriteFileTool(root).Run(context.Background(), map[string]any{key: key + ".txt", "content": "x"}) + res := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{key: key + ".txt", "content": "x"}) if res.Status != StatusOK { t.Fatalf("path alias %q: expected ok, got %s: %s", key, res.Status, res.Output) } @@ -276,7 +276,7 @@ func TestEditFileToolAcceptsAliases(t *testing.T) { path := filepath.Join(root, "code.go") writeTestFile(t, path, "const a = 1\n") // path/old/new aliases all at once. - res := NewEditFileTool(root).Run(context.Background(), map[string]any{ + res := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{ "file": "code.go", "old": "const a = 1", "new": "const a = 2", @@ -291,7 +291,7 @@ func TestEditFileToolAcceptsAliases(t *testing.T) { // other alias spellings for old_string/new_string. writeTestFile(t, path, "const a = 1\n") - res = NewEditFileTool(root).Run(context.Background(), map[string]any{ + res = NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{ "file_path": "code.go", "search": "const a = 1", "replace": "const a = 3", @@ -318,7 +318,7 @@ func TestApplyPatchToolAcceptsDiffAlias(t *testing.T) { "+new", "", }, "\n") - res := NewApplyPatchTool(root).Run(context.Background(), map[string]any{"diff": patch}) + res := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"diff": patch}) if res.Status != StatusOK { if gitApplyUnavailable(res.Output) { t.Skipf("git binary unavailable: %s", res.Output) @@ -334,7 +334,7 @@ func TestApplyPatchToolAcceptsDiffAlias(t *testing.T) { func TestBashToolAcceptsCommandAliases(t *testing.T) { root := t.TempDir() for _, key := range []string{"cmd", "script", "shell"} { - res := NewBashTool(root).Run(context.Background(), map[string]any{key: "echo hi"}) + res := NewScopedBashTool(root, nil).Run(context.Background(), map[string]any{key: "echo hi"}) if res.Status != StatusOK { t.Fatalf("command alias %q: expected ok, got %s: %s", key, res.Status, res.Output) } @@ -403,7 +403,7 @@ func TestWriteFileAcceptsStringOverwrite(t *testing.T) { t.Fatal(err) } // minimax-style: overwrite as the string "true". - res := NewWriteFileTool(root).Run(context.Background(), map[string]any{ + res := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "shop.html", "content": "new", "overwrite": "true", }) if res.Status != StatusOK { diff --git a/internal/tools/ask_user_test.go b/internal/tools/ask_user_test.go index a85969074..06f26fc51 100644 --- a/internal/tools/ask_user_test.go +++ b/internal/tools/ask_user_test.go @@ -155,7 +155,7 @@ func TestAskUserToolRunRejectsMissingQuestions(t *testing.T) { func TestCoreReadOnlyToolsIncludeAskUser(t *testing.T) { found := false - for _, tool := range CoreReadOnlyTools(t.TempDir()) { + for _, tool := range CoreReadOnlyToolsScoped(t.TempDir(), nil) { if tool.Name() == "ask_user" { found = true break diff --git a/internal/tools/bash.go b/internal/tools/bash.go index a417fb8a7..2a40e2934 100644 --- a/internal/tools/bash.go +++ b/internal/tools/bash.go @@ -453,15 +453,6 @@ const bashOutputBudgetBytes = 32 * 1024 // the model's recovery path — cover 6× more of the output. const bashCaptureBudgetBytes = 96 * 1024 -// budgetBashOutput truncates stdout and stderr to bashOutputBudgetBytes each, -// keeping the head and tail of anything larger, and records raw/emitted byte -// counts plus a truncated flag in meta (mirroring outputBudgetMeta's shape for -// the read/search tools). Detection that needs the full output (sandbox-denial -// scanning) must run on the raw strings before this is applied. -func budgetBashOutput(stdout string, stderr string, meta map[string]string) (string, string, bool) { - return budgetBashCapture(stdout, len(stdout), stderr, len(stderr), meta) -} - // prepareBashOutput keeps direct callers on the established positional budget. // Registry calls retain the existing bounded capture but leave final semantic // reduction and spill creation to the post-redaction registry boundary. diff --git a/internal/tools/bash_auto_allow_test.go b/internal/tools/bash_auto_allow_test.go index ab8ceed1c..ef335229d 100644 --- a/internal/tools/bash_auto_allow_test.go +++ b/internal/tools/bash_auto_allow_test.go @@ -32,7 +32,7 @@ const permissionRequiredFragment = "Permission required for bash" func TestBashAutoAllowedWhenSandboxActive(t *testing.T) { root := t.TempDir() registry := NewRegistry() - registry.Register(NewBashTool(root)) + registry.Register(NewScopedBashTool(root, nil)) engine := sandbox.NewEngine(sandbox.EngineOptions{ WorkspaceRoot: root, Policy: sandboxedBashPolicy(), @@ -59,7 +59,7 @@ func TestBashAutoAllowedWhenSandboxActive(t *testing.T) { func TestBashRequireEscalatedPromptsWhenSandboxActive(t *testing.T) { root := t.TempDir() registry := NewRegistry() - registry.Register(NewBashTool(root)) + registry.Register(NewScopedBashTool(root, nil)) engine := sandbox.NewEngine(sandbox.EngineOptions{ WorkspaceRoot: root, Policy: sandboxedBashPolicy(), @@ -87,7 +87,7 @@ func TestBashRequireEscalatedPromptsWhenSandboxActive(t *testing.T) { func TestBashRequireEscalatedBypassesNativeSandboxAfterApproval(t *testing.T) { root := t.TempDir() registry := NewRegistry() - registry.Register(NewBashTool(root)) + registry.Register(NewScopedBashTool(root, nil)) engine := sandbox.NewEngine(sandbox.EngineOptions{ WorkspaceRoot: root, Policy: sandboxedBashPolicy(), @@ -133,7 +133,7 @@ func TestBashRequireEscalatedKeepsSandboxWhenDeniedReadsActive(t *testing.T) { func TestBashStillPromptsWithoutActiveSandbox(t *testing.T) { root := t.TempDir() registry := NewRegistry() - registry.Register(NewBashTool(root)) + registry.Register(NewScopedBashTool(root, nil)) engine := sandbox.NewEngine(sandbox.EngineOptions{ WorkspaceRoot: root, Policy: sandboxedBashPolicy(), diff --git a/internal/tools/bash_tool_test.go b/internal/tools/bash_tool_test.go index b13ac0031..204d9b194 100644 --- a/internal/tools/bash_tool_test.go +++ b/internal/tools/bash_tool_test.go @@ -84,7 +84,7 @@ func runBashToolHelper(command string) { } func TestCoreToolsExposeShellTools(t *testing.T) { - toolset := CoreTools(t.TempDir()) + toolset := CoreToolsScoped(t.TempDir(), nil) byName := make(map[string]Tool, len(toolset)) for _, tool := range toolset { byName[tool.Name()] = tool @@ -109,7 +109,7 @@ func TestCoreToolsExposeShellTools(t *testing.T) { } func TestBashToolDescribesHostShellSyntax(t *testing.T) { - tool := NewBashTool(t.TempDir()) + tool := NewScopedBashTool(t.TempDir(), nil) schema := tool.Parameters() descriptionParts := []string{tool.Description()} for _, property := range schema.Properties { @@ -316,7 +316,7 @@ func TestDetectShellOutputIssueAddsWindowsSyntaxHint(t *testing.T) { func TestRegistryBlocksBashWithoutGrant(t *testing.T) { registry := NewRegistry() - registry.Register(NewBashTool(t.TempDir())) + registry.Register(NewScopedBashTool(t.TempDir(), nil)) result := registry.Run(context.Background(), "bash", map[string]any{ "command": helperCommand("success"), @@ -333,7 +333,7 @@ func TestRegistryBlocksBashWithoutGrant(t *testing.T) { func TestBashToolRunsCommandInWorkspace(t *testing.T) { root := t.TempDir() - result := NewBashTool(root).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(root, nil).Run(context.Background(), map[string]any{ "command": helperCommand("success"), }) @@ -379,7 +379,7 @@ func TestBashToolBoundsRunawayOutputCapture(t *testing.T) { } const produced = 500000 // ~5× the 96 KiB budget - result := NewBashTool(t.TempDir()).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(t.TempDir(), nil).Run(context.Background(), map[string]any{ "command": fmt.Sprintf("yes ABCDEFGH | head -c %d", produced), }) @@ -414,7 +414,7 @@ func TestBashToolUsesRequestedCwd(t *testing.T) { t.Fatal(err) } - result := NewBashTool(root).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(root, nil).Run(context.Background(), map[string]any{ "command": helperCommand("pwd"), "cwd": "nested", }) @@ -434,7 +434,7 @@ func TestBashToolUsesRequestedCwd(t *testing.T) { func TestBashToolRejectsCwdOutsideWorkspace(t *testing.T) { outside := t.TempDir() - result := NewBashTool(t.TempDir()).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(t.TempDir(), nil).Run(context.Background(), map[string]any{ "command": helperCommand("success"), "cwd": outside, }) @@ -448,7 +448,7 @@ func TestBashToolRejectsCwdOutsideWorkspace(t *testing.T) { } func TestBashToolReturnsNonzeroExitAsError(t *testing.T) { - result := NewBashTool(t.TempDir()).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(t.TempDir(), nil).Run(context.Background(), map[string]any{ "command": helperCommand("fail"), }) @@ -469,7 +469,7 @@ func TestBashToolReturnsNonzeroExitAsError(t *testing.T) { } func TestBashToolTimesOut(t *testing.T) { - result := NewBashTool(t.TempDir()).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(t.TempDir(), nil).Run(context.Background(), map[string]any{ "command": helperCommand("sleep"), "timeout_ms": 20, }) @@ -508,7 +508,7 @@ func TestBashToolTimeoutKillsBackgroundChildren(t *testing.T) { command := fmt.Sprintf("(sleep 1; touch %s) & wait", shellQuote(sentinel)) start := time.Now() - result := NewBashTool(root).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(root, nil).Run(context.Background(), map[string]any{ "command": command, "timeout_ms": 300, }) @@ -533,7 +533,7 @@ func TestBashToolTimeoutKillsBackgroundChildren(t *testing.T) { func TestRegistryRunsWithDegradedUnavailableNativeSandbox(t *testing.T) { root := t.TempDir() registry := NewRegistry() - registry.Register(NewBashTool(root)) + registry.Register(NewScopedBashTool(root, nil)) engine := sandbox.NewEngine(sandbox.EngineOptions{ WorkspaceRoot: root, Policy: sandbox.DefaultPolicy(), @@ -577,7 +577,7 @@ func TestBashToolRequireEscalatedMsysGuard(t *testing.T) { }) } registry := NewRegistry() - registry.Register(NewBashTool(root)) + registry.Register(NewScopedBashTool(root, nil)) t.Run("default sandboxing still blocks an MSYS-prone command", func(t *testing.T) { result := registry.RunWithOptions(context.Background(), "bash", map[string]any{ @@ -644,7 +644,7 @@ func TestBashToolIgnoresMsysMarkersInCommandArgumentsAfterFailure(t *testing.T) // otherwise unused by the helper but still part of the command line text. command := helperCommand("fail") + ` "fatal error - CreateFileMapping S-1-5-21, Win32 error 5. Terminating. cygheap_user::init"` - result := NewBashTool(root).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(root, nil).Run(context.Background(), map[string]any{ "command": command, }) @@ -665,7 +665,7 @@ func TestBashToolRunsWithDegradedUnavailableNativeSandbox(t *testing.T) { Backend: sandbox.Backend{Name: sandbox.BackendUnavailable, Message: "native sandbox unavailable"}, }) - result := NewBashTool(root).(interface { + result := NewScopedBashTool(root, nil).(interface { RunWithSandbox(context.Context, map[string]any, *sandbox.Engine) Result }).RunWithSandbox(context.Background(), map[string]any{ "command": helperCommand("success"), @@ -725,7 +725,7 @@ func TestBashToolRunsWithHostSandboxBackendWhenAvailable(t *testing.T) { Backend: backend, }) - result := NewBashTool(root).(interface { + result := NewScopedBashTool(root, nil).(interface { RunWithSandbox(context.Context, map[string]any, *sandbox.Engine) Result }).RunWithSandbox(context.Background(), map[string]any{ "command": "printf sandbox-ok", @@ -745,7 +745,7 @@ func TestBashToolRunsWithHostSandboxBackendWhenAvailable(t *testing.T) { func TestBashToolBlocksInteractiveCommandBeforeExecution(t *testing.T) { root := t.TempDir() - result := NewBashTool(root).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(root, nil).Run(context.Background(), map[string]any{ "command": "vim main.go", }) @@ -778,7 +778,7 @@ func TestBashToolBlocksInteractiveCommandThroughSandbox(t *testing.T) { Backend: sandbox.Backend{Name: sandbox.BackendUnavailable, Message: "native sandbox unavailable"}, }) - result := NewBashTool(root).(interface { + result := NewScopedBashTool(root, nil).(interface { RunWithSandbox(context.Context, map[string]any, *sandbox.Engine) Result }).RunWithSandbox(context.Background(), map[string]any{ "command": "less /etc/hosts", @@ -802,7 +802,7 @@ func TestBashToolBlocksInteractiveCommandThroughSandbox(t *testing.T) { func TestBashToolAllowsNonInteractiveCommand(t *testing.T) { root := t.TempDir() - result := NewBashTool(root).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(root, nil).Run(context.Background(), map[string]any{ "command": helperCommand("success"), }) @@ -835,7 +835,7 @@ func TestBashToolPreservesEmbeddedQuotesOnWindows(t *testing.T) { // showed exactly where the corruption happened. commandText := executable + ` --zero-bash-helper echo-arg "hello / world"` - result := NewBashTool(root).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(root, nil).Run(context.Background(), map[string]any{ "command": commandText, }) @@ -862,7 +862,7 @@ func TestBashToolRunsCommandLineForLoopSyntax(t *testing.T) { } root := t.TempDir() - result := NewBashTool(root).Run(context.Background(), map[string]any{ + result := NewScopedBashTool(root, nil).Run(context.Background(), map[string]any{ "command": "for %i in (1 2 3) do echo %i", }) diff --git a/internal/tools/builtin_catalog.go b/internal/tools/builtin_catalog.go index ca23d1862..24c975581 100644 --- a/internal/tools/builtin_catalog.go +++ b/internal/tools/builtin_catalog.go @@ -19,9 +19,9 @@ func BuiltinCatalog(workspaceRoot string) []Tool { workspaceRoot = "." } var all []Tool - all = append(all, CoreReadOnlyTools(workspaceRoot)...) - all = append(all, CoreWriteTools(workspaceRoot)...) - all = append(all, CoreShellTools(workspaceRoot)...) + all = append(all, CoreReadOnlyToolsScoped(workspaceRoot, nil)...) + all = append(all, CoreWriteToolsScoped(workspaceRoot, nil)...) + all = append(all, CoreShellToolsScoped(workspaceRoot, nil)...) // Network tools: always include web_fetch; always include web_search for // classification even when no backend is configured at runtime (registration // remains conditional in CoreNetworkTools). diff --git a/internal/tools/deferred.go b/internal/tools/deferred.go index 8b8ffb735..de922a68f 100644 --- a/internal/tools/deferred.go +++ b/internal/tools/deferred.go @@ -1,7 +1,6 @@ package tools import ( - "fmt" "regexp" "sort" "strings" @@ -19,107 +18,6 @@ var ( collapseWhitespace = regexp.MustCompile(`\s+`) ) -// normalizeDescriptionLine trims a line and unwraps a leading markdown heading. -func normalizeDescriptionLine(line string) string { - trimmed := strings.TrimSpace(line) - if m := headingPrefix.FindStringSubmatch(trimmed); m != nil { - return strings.TrimSpace(m[1]) - } - return trimmed -} - -func isGenericDescriptionHeading(line string) bool { - return genericHeading.MatchString(line) -} - -// truncateDescription clips desc to at most max runes, preferring a word -// boundary and appending a single-rune ellipsis when it had to cut. -func truncateDescription(desc string, max int) string { - runes := []rune(desc) - if max <= 0 || len(runes) <= max { - return desc - } - cut := string(runes[:max-1]) - if idx := strings.LastIndexByte(cut, ' '); idx > 0 { - cut = cut[:idx] - } - return strings.TrimRight(cut, " ") + "…" -} - -// shortenDescription reduces desc to a single meaningful line, collapses -// whitespace, and truncates to at most max runes with an ellipsis. -func shortenDescription(desc string, max int) string { - if desc == "" { - return "" - } - if max <= 0 { - max = defaultShortenMax - } - var lines []string - for _, raw := range strings.Split(desc, "\n") { - if line := normalizeDescriptionLine(raw); line != "" { - lines = append(lines, collapseWhitespace.ReplaceAllString(line, " ")) - } - } - if len(lines) == 0 { - return "" - } - meaningful := lines[0] - if isGenericDescriptionHeading(meaningful) && len(lines) > 1 { - meaningful = meaningful + " — " + lines[1] - } - return truncateDescription(meaningful, max) -} - -// formatInputSchemaHint renders a one-line summary of a tool's input schema, -// e.g. "inputs (* required): a (string)*, b (number); +N more". Property names -// are sorted for deterministic output (Schema.Properties is a map). Returns -// "(none)" when the schema declares no properties. At most maxSchemaHintParams -// params are shown; the rest are summarized as "; +N more". -func formatInputSchemaHint(schema Schema) string { - if len(schema.Properties) == 0 { - return "(none)" - } - - names := make([]string, 0, len(schema.Properties)) - for name := range schema.Properties { - names = append(names, name) - } - sort.Strings(names) - - required := make(map[string]bool, len(schema.Required)) - for _, name := range schema.Required { - required[name] = true - } - - shown := names - if len(shown) > maxSchemaHintParams { - shown = shown[:maxSchemaHintParams] - } - - parts := make([]string, 0, len(shown)) - for _, name := range shown { - prop := schema.Properties[name] - marker := "" - if required[name] { - marker = "*" - } - typePart := "" - if t := strings.TrimSpace(prop.Type); t != "" { - typePart = " (" + t + ")" - } - parts = append(parts, name+typePart+marker) - } - - more := "" - if len(names) > maxSchemaHintParams { - more = fmt.Sprintf("; +%d more", len(names)-maxSchemaHintParams) - } - - hint := "inputs (* required): " + strings.Join(parts, ", ") + more - return shortenDescription(hint, maxSchemaHintLen) -} - // mcpToolNamePrefix mirrors the prefix used by mcp.registryToolName. const mcpToolNamePrefix = "mcp_" @@ -139,22 +37,6 @@ func mcpServerFromToolName(name string) string { return rest[:sep] } -// formatDeferredToolLine renders a single compact advertisement line for a -// deferred tool: "name: | server: | ". The -// "server: " segment is omitted when server is empty (non-MCP tools). -func formatDeferredToolLine(name, description, server string, schema Schema) string { - desc := shortenDescription(description, defaultShortenMax) - if desc == "" { - desc = "No description provided" - } - parts := []string{name + ": " + desc} - if server != "" { - parts = append(parts, "server: "+server) - } - parts = append(parts, formatInputSchemaHint(schema)) - return strings.Join(parts, " | ") -} - // mcpServerNamed is an optional interface a deferred MCP tool implements to // report its true (un-sanitized-token) server name for discovery labels. When // a tool provides it, DeferredLine prefers it over the name-derived token, which @@ -165,22 +47,6 @@ type mcpServerNamed interface { MCPServerName() string } -// DeferredLine renders the compact advertisement line for a single deferred -// tool, deriving the MCP server label from the tool's reported server name when -// available, falling back to the token parsed from the tool's name. It is the -// exported entry point the agent loop uses to build compact deferred metadata, -// so callers in other packages never touch the -// unexported formatters. -func DeferredLine(t Tool) string { - server := mcpServerFromToolName(t.Name()) - if named, ok := t.(mcpServerNamed); ok { - if reported := strings.TrimSpace(named.MCPServerName()); reported != "" { - server = reported - } - } - return formatDeferredToolLine(t.Name(), t.Description(), server, t.Parameters()) -} - // DeferredSource reports the compact source label used in tool_search's dynamic // description. MCP tools use their configured server name; other deferred tools // fall back to the first name segment so families such as swarm_* are grouped. diff --git a/internal/tools/edit_file.go b/internal/tools/edit_file.go index 612c131c6..8bd2394ea 100644 --- a/internal/tools/edit_file.go +++ b/internal/tools/edit_file.go @@ -14,10 +14,6 @@ type editFileTool struct { scope PathScope } -func NewEditFileTool(workspaceRoot string) Tool { - return NewScopedEditFileTool(workspaceRoot, nil) -} - func NewScopedEditFileTool(workspaceRoot string, scope PathScope) Tool { return editFileTool{ baseTool: baseTool{ diff --git a/internal/tools/edit_replacers_test.go b/internal/tools/edit_replacers_test.go index b82a34cf0..918e7e251 100644 --- a/internal/tools/edit_replacers_test.go +++ b/internal/tools/edit_replacers_test.go @@ -17,7 +17,7 @@ func runEdit(t *testing.T, dir, initial string, args map[string]any) (Result, st if args["path"] == nil { args["path"] = "target.txt" } - result := NewEditFileTool(dir).Run(context.Background(), args) + result := NewScopedEditFileTool(dir, nil).Run(context.Background(), args) after, err := os.ReadFile(path) if err != nil { t.Fatal(err) diff --git a/internal/tools/export_test.go b/internal/tools/export_test.go new file mode 100644 index 000000000..d5efe4897 --- /dev/null +++ b/internal/tools/export_test.go @@ -0,0 +1,168 @@ +package tools + +import ( + "fmt" + "net/http" + "sort" + "strings" + + zeroSandbox "github.com/Gitlawb/zero/internal/sandbox" +) + +// newWebFetchToolWithClient is a test seam: it builds the live web_fetch tool +// with an injected HTTP client so tests can stub transport behavior. +func newWebFetchToolWithClient(client *http.Client) Tool { + return newWebFetchToolWithClientAndResolver(client, nil) +} + +// budgetBashOutput truncates stdout and stderr to bashOutputBudgetBytes each, +// keeping the head and tail of anything larger, and records raw/emitted byte +// counts plus a truncated flag in meta (mirroring outputBudgetMeta's shape for +// the read/search tools). Detection that needs the full output (sandbox-denial +// scanning) must run on the raw strings before this is applied. +func budgetBashOutput(stdout string, stderr string, meta map[string]string) (string, string, bool) { + return budgetBashCapture(stdout, len(stdout), stderr, len(stderr), meta) +} + +// DeferredLine renders the compact advertisement line for a single deferred +// tool, deriving the MCP server label from the tool's reported server name when +// available, falling back to the token parsed from the tool's name. It is the +// exported entry point the agent loop uses to build compact deferred metadata, +// so callers in other packages never touch the +// unexported formatters. +func DeferredLine(t Tool) string { + server := mcpServerFromToolName(t.Name()) + if named, ok := t.(mcpServerNamed); ok { + if reported := strings.TrimSpace(named.MCPServerName()); reported != "" { + server = reported + } + } + return formatDeferredToolLine(t.Name(), t.Description(), server, t.Parameters()) +} + +// formatDeferredToolLine renders a single compact advertisement line for a +// deferred tool: "name: | server: | ". The +// "server: " segment is omitted when server is empty (non-MCP tools). +func formatDeferredToolLine(name, description, server string, schema Schema) string { + desc := shortenDescription(description, defaultShortenMax) + if desc == "" { + desc = "No description provided" + } + parts := []string{name + ": " + desc} + if server != "" { + parts = append(parts, "server: "+server) + } + parts = append(parts, formatInputSchemaHint(schema)) + return strings.Join(parts, " | ") +} + +// formatInputSchemaHint renders a one-line summary of a tool's input schema, +// e.g. "inputs (* required): a (string)*, b (number); +N more". Property names +// are sorted for deterministic output (Schema.Properties is a map). Returns +// "(none)" when the schema declares no properties. At most maxSchemaHintParams +// params are shown; the rest are summarized as "; +N more". +func formatInputSchemaHint(schema Schema) string { + if len(schema.Properties) == 0 { + return "(none)" + } + + names := make([]string, 0, len(schema.Properties)) + for name := range schema.Properties { + names = append(names, name) + } + sort.Strings(names) + + required := make(map[string]bool, len(schema.Required)) + for _, name := range schema.Required { + required[name] = true + } + + shown := names + if len(shown) > maxSchemaHintParams { + shown = shown[:maxSchemaHintParams] + } + + parts := make([]string, 0, len(shown)) + for _, name := range shown { + prop := schema.Properties[name] + marker := "" + if required[name] { + marker = "*" + } + typePart := "" + if t := strings.TrimSpace(prop.Type); t != "" { + typePart = " (" + t + ")" + } + parts = append(parts, name+typePart+marker) + } + + more := "" + if len(names) > maxSchemaHintParams { + more = fmt.Sprintf("; +%d more", len(names)-maxSchemaHintParams) + } + + hint := "inputs (* required): " + strings.Join(parts, ", ") + more + return shortenDescription(hint, maxSchemaHintLen) +} + +// shortenDescription reduces desc to a single meaningful line, collapses +// whitespace, and truncates to at most max runes with an ellipsis. +func shortenDescription(desc string, max int) string { + if desc == "" { + return "" + } + if max <= 0 { + max = defaultShortenMax + } + var lines []string + for _, raw := range strings.Split(desc, "\n") { + if line := normalizeDescriptionLine(raw); line != "" { + lines = append(lines, collapseWhitespace.ReplaceAllString(line, " ")) + } + } + if len(lines) == 0 { + return "" + } + meaningful := lines[0] + if isGenericDescriptionHeading(meaningful) && len(lines) > 1 { + meaningful = meaningful + " — " + lines[1] + } + return truncateDescription(meaningful, max) +} + +func (manager *execSessionManager) len() int { + manager.mu.Lock() + defer manager.mu.Unlock() + return len(manager.sessions) +} + +func likelySandboxDenied(plan zeroSandbox.CommandPlan, exitCode int, sections ...string) bool { + kind, _ := sandboxDenialKind(plan, exitCode, sections...) + return kind != "" +} +func isGenericDescriptionHeading(line string) bool { + return genericHeading.MatchString(line) +} + +// normalizeDescriptionLine trims a line and unwraps a leading markdown heading. +func normalizeDescriptionLine(line string) string { + trimmed := strings.TrimSpace(line) + if m := headingPrefix.FindStringSubmatch(trimmed); m != nil { + return strings.TrimSpace(m[1]) + } + return trimmed +} + +// truncateDescription clips desc to at most max runes, preferring a word +// boundary and appending a single-rune ellipsis when it had to cut. +func truncateDescription(desc string, max int) string { + runes := []rune(desc) + if max <= 0 || len(runes) <= max { + return desc + } + cut := string(runes[:max-1]) + if idx := strings.LastIndexByte(cut, ' '); idx > 0 { + cut = cut[:idx] + } + return strings.TrimRight(cut, " ") + "…" +} diff --git a/internal/tools/file_safety_test.go b/internal/tools/file_safety_test.go index 564332711..16b443787 100644 --- a/internal/tools/file_safety_test.go +++ b/internal/tools/file_safety_test.go @@ -14,7 +14,7 @@ import ( func safetyRegistry(t *testing.T, dir string) *Registry { t.Helper() registry := NewRegistry() - for _, tool := range []Tool{NewReadFileTool(dir), NewEditFileTool(dir), NewWriteFileTool(dir)} { + for _, tool := range []Tool{NewScopedReadFileTool(dir, nil), NewScopedEditFileTool(dir, nil), NewScopedWriteFileTool(dir, nil)} { registry.Register(tool) } return registry diff --git a/internal/tools/file_tools_test.go b/internal/tools/file_tools_test.go index 7209a3d7b..b16829c89 100644 --- a/internal/tools/file_tools_test.go +++ b/internal/tools/file_tools_test.go @@ -15,7 +15,7 @@ func TestReadFileToolReadsLineRanges(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta\ngamma\ndelta") - result := NewReadFileTool(root).Run(context.Background(), map[string]any{ + result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "notes.txt", "start_line": 2, "max_lines": 2, @@ -42,7 +42,7 @@ func TestReadFileToolMarksTruncation(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "notes.txt"), "a\nb\nc\nd\ne") - result := NewReadFileTool(root).Run(context.Background(), map[string]any{ + result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "notes.txt", "max_lines": 2, }) @@ -61,7 +61,7 @@ func TestReadFileToolAppliesByteBudget(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "large.txt"), strings.Repeat("0123456789abcdef\n", 9000)) - result := NewReadFileTool(root).Run(context.Background(), map[string]any{ + result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "large.txt", }) @@ -89,7 +89,7 @@ func TestReadFileToolTracksWholeFileHashWhenReadingSmallRange(t *testing.T) { writeTestFile(t, path, content) tracker := NewFileTracker() - tool := NewReadFileTool(root).(optionsAwareTool) + tool := NewScopedReadFileTool(root, nil).(optionsAwareTool) result := tool.RunWithOptions(context.Background(), map[string]any{ "path": "large.txt", "start_line": 1000, @@ -120,7 +120,7 @@ func TestReadFileToolRejectsOutsideWorkspace(t *testing.T) { outside := filepath.Join(t.TempDir(), "secret.txt") writeTestFile(t, outside, "secret") - result := NewReadFileTool(root).Run(context.Background(), map[string]any{ + result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{ "path": outside, }) @@ -138,7 +138,7 @@ func TestListDirectoryToolListsRecursivelyAndIgnoresJunk(t *testing.T) { writeTestFile(t, filepath.Join(root, "node_modules", "leftpad", "index.js"), "module.exports = 1") writeTestFile(t, filepath.Join(root, "README.md"), "# Zero") - result := NewListDirectoryTool(root).Run(context.Background(), map[string]any{ + result := NewScopedListDirectoryTool(root, nil).Run(context.Background(), map[string]any{ "path": ".", "recursive": true, "max_depth": 2, @@ -164,7 +164,7 @@ func TestGlobToolFindsMatchesWithLimit(t *testing.T) { writeTestFile(t, filepath.Join(root, "nested", "b.go"), "package nested") writeTestFile(t, filepath.Join(root, "nested", "c.txt"), "text") - result := NewGlobTool(root).Run(context.Background(), map[string]any{ + result := NewScopedGlobTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "**/*.go", "limit": 1, }) @@ -190,7 +190,7 @@ func TestGlobToolCanIncludeDirectoryMatches(t *testing.T) { t.Fatal(err) } - result := NewGlobTool(root).Run(context.Background(), map[string]any{ + result := NewScopedGlobTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "src", "include_dirs": true, }) @@ -208,7 +208,7 @@ func TestGrepToolSearchesContent(t *testing.T) { writeTestFile(t, filepath.Join(root, "cmd", "main.go"), "package main\nfunc main() {}\n") writeTestFile(t, filepath.Join(root, "README.md"), "main docs\n") - result := NewGrepTool(root).Run(context.Background(), map[string]any{ + result := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "func main", "path": ".", "glob": "**/*.go", @@ -230,7 +230,7 @@ func TestGrepToolMakesHeadLimitTruncationVisible(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "notes.txt"), "needle 1\nneedle 2\nneedle 3\n") - result := NewGrepTool(root).Run(context.Background(), map[string]any{ + result := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "needle", "path": ".", "head_limit": 2, @@ -260,7 +260,7 @@ func TestGrepToolStopsAfterHeadLimitInContentMode(t *testing.T) { } writeTestFile(t, filepath.Join(root, "notes.txt"), builder.String()) - result := NewGrepTool(root).Run(context.Background(), map[string]any{ + result := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "needle", "path": ".", "head_limit": 3, @@ -320,7 +320,7 @@ func TestGrepGlobMatchesRelativeToSearchDir(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "subdir", "a.go"), "package sub\nfunc target() {}\n") - result := NewGrepTool(root).Run(context.Background(), map[string]any{ + result := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "func target", "path": "subdir", "glob": "*.go", @@ -339,7 +339,7 @@ func TestGrepToolSupportsFilesAndCountModes(t *testing.T) { writeTestFile(t, filepath.Join(root, "a.txt"), "needle\nneedle\n") writeTestFile(t, filepath.Join(root, "b.txt"), "needle\n") - files := NewGrepTool(root).Run(context.Background(), map[string]any{ + files := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "needle", "output_mode": "files_with_matches", }) @@ -350,7 +350,7 @@ func TestGrepToolSupportsFilesAndCountModes(t *testing.T) { t.Fatalf("expected both files, got %q", files.Output) } - count := NewGrepTool(root).Run(context.Background(), map[string]any{ + count := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "needle", "output_mode": "count", }) @@ -366,7 +366,7 @@ func TestGrepCountModeCountsMultipleHitsPerLine(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "notes.txt"), "needle needle\nneedle\n") - count := NewGrepTool(root).Run(context.Background(), map[string]any{ + count := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "needle", "output_mode": "count", }) @@ -393,7 +393,7 @@ func TestGrepDoesNotFollowSymlinkOutsideWorkspace(t *testing.T) { t.Skipf("symlinks unsupported: %v", err) } - res := NewGrepTool(root).Run(context.Background(), map[string]any{ + res := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "needle", "output_mode": "content", }) @@ -421,7 +421,7 @@ func TestGrepReturnsCleanRelativePathsUnderSymlinkedRoot(t *testing.T) { t.Skipf("symlinks unsupported: %v", err) } - res := NewGrepTool(linkRoot).Run(context.Background(), map[string]any{ + res := NewScopedGrepTool(linkRoot, nil).Run(context.Background(), map[string]any{ "pattern": "func main", "output_mode": "content", }) @@ -436,7 +436,7 @@ func TestGrepReturnsCleanRelativePathsUnderSymlinkedRoot(t *testing.T) { } // files_with_matches mode must likewise be clean-relative. - res = NewGrepTool(linkRoot).Run(context.Background(), map[string]any{ + res = NewScopedGrepTool(linkRoot, nil).Run(context.Background(), map[string]any{ "pattern": "func main", "output_mode": "files_with_matches", }) @@ -618,7 +618,7 @@ func TestUnscopedWriteRefusesInRootSymlinkTraversal(t *testing.T) { if err := os.Symlink(filepath.Join(workspace, "subdir"), link); err != nil { t.Skipf("symlinks unavailable: %v", err) } - res := NewWriteFileTool(workspace).Run(context.Background(), map[string]any{ + res := NewScopedWriteFileTool(workspace, nil).Run(context.Background(), map[string]any{ "path": filepath.Join(link, "x.txt"), "content": "nope", }) @@ -667,7 +667,7 @@ func TestScopedWriteThroughSymlinkIntoGrantedRoot(t *testing.T) { func TestUnscopedToolsStillRejectOutsideWrites(t *testing.T) { workspace := t.TempDir() outside := filepath.Join(t.TempDir(), "escape.txt") - res := NewWriteFileTool(workspace).Run(context.Background(), map[string]any{ + res := NewScopedWriteFileTool(workspace, nil).Run(context.Background(), map[string]any{ "path": outside, "content": "nope", }) @@ -705,7 +705,7 @@ func TestGrepSkipsAlwaysExcludedDirectories(t *testing.T) { mustWrite("vendor/pkg/lib.go", "needle here") mustWrite(".worktrees/branch/main.go", "needle here") - res := NewGrepTool(root).Run(context.Background(), map[string]any{ + res := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "needle", "output_mode": "files_with_matches", }) @@ -779,7 +779,7 @@ func TestGrepSkipsBinaryLikeFiles(t *testing.T) { writeTestFile(t, filepath.Join(root, "image.png"), "needle hidden") writeTestFile(t, filepath.Join(root, "archive.zip"), "needle hidden") - res := NewGrepTool(root).Run(context.Background(), map[string]any{ + res := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "needle", "output_mode": "files_with_matches", }) @@ -793,7 +793,7 @@ func TestGrepSkipsBinaryLikeFiles(t *testing.T) { t.Fatalf("expected only keep.txt, got:\n%s", res.Output) } - direct := NewGrepTool(root).Run(context.Background(), map[string]any{ + direct := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "needle", "path": "image.png", }) @@ -813,7 +813,7 @@ func TestGlobSkipsWorkspaceExcludedDirectoriesAndBinaryFiles(t *testing.T) { writeTestFile(t, filepath.Join(root, ".worktrees", "branch", "main.go"), "package main") writeTestFile(t, filepath.Join(root, "image.png"), "binary") - res := NewGlobTool(root).Run(context.Background(), map[string]any{ + res := NewScopedGlobTool(root, nil).Run(context.Background(), map[string]any{ "pattern": "**/*", "limit": 20, }) diff --git a/internal/tools/format_on_write_test.go b/internal/tools/format_on_write_test.go index 76a0960e2..aa5e2bbdf 100644 --- a/internal/tools/format_on_write_test.go +++ b/internal/tools/format_on_write_test.go @@ -24,7 +24,7 @@ func TestFormatOnWriteDisabledByDefault(t *testing.T) { dir := t.TempDir() ugly := "package a\n\nfunc A( ) { }\n" - result := NewWriteFileTool(dir).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ + result := NewScopedWriteFileTool(dir, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ "path": "a.go", "content": ugly, }, RunOptions{}) @@ -46,7 +46,7 @@ func TestFormatOnWriteFormatsAndKeepsTrackerConsistent(t *testing.T) { dir := t.TempDir() tracker := NewFileTracker() - write := NewWriteFileTool(dir).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ + write := NewScopedWriteFileTool(dir, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ "path": "a.go", "content": "package a\n\nfunc A( ) { }\n", }, RunOptions{FileTracker: tracker}) @@ -63,7 +63,7 @@ func TestFormatOnWriteFormatsAndKeepsTrackerConsistent(t *testing.T) { // The tracker must have been re-baselined to the POST-format content: a // follow-up edit must not trip the external-modification conflict guard. - edit := NewEditFileTool(dir).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ + edit := NewScopedEditFileTool(dir, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ "path": "a.go", "old_string": "func A() {", "new_string": "func B() {", diff --git a/internal/tools/inline_diagnostics_test.go b/internal/tools/inline_diagnostics_test.go index caa73f8c3..062405bfa 100644 --- a/internal/tools/inline_diagnostics_test.go +++ b/internal/tools/inline_diagnostics_test.go @@ -20,7 +20,7 @@ func TestMutatingToolsAppendInlineDiagnostics(t *testing.T) { return absPath + ":1:1: error: undefined: x" } - editResult := NewEditFileTool(dir).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ + editResult := NewScopedEditFileTool(dir, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ "path": "a.go", "old_string": "package a", "new_string": "package b", @@ -32,7 +32,7 @@ func TestMutatingToolsAppendInlineDiagnostics(t *testing.T) { t.Fatalf("edit output missing diagnostics block: %q", editResult.Output) } - writeResult := NewWriteFileTool(dir).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ + writeResult := NewScopedWriteFileTool(dir, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ "path": "b.go", "content": "package b\n", }, RunOptions{Diagnostics: diagnostics}) @@ -43,7 +43,7 @@ func TestMutatingToolsAppendInlineDiagnostics(t *testing.T) { t.Fatalf("write output missing diagnostics block: %q", writeResult.Output) } - clean := NewWriteFileTool(dir).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ + clean := NewScopedWriteFileTool(dir, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{ "path": "c.go", "content": "package c\n", }, RunOptions{Diagnostics: func(context.Context, string) string { return "" }}) diff --git a/internal/tools/lsp_navigate.go b/internal/tools/lsp_navigate.go index 79c65c16d..d667a98b7 100644 --- a/internal/tools/lsp_navigate.go +++ b/internal/tools/lsp_navigate.go @@ -23,11 +23,6 @@ type lspNavigateTool struct { manager *lsp.Manager } -// NewLSPNavigateTool builds the tool with workspace-only path confinement. -func NewLSPNavigateTool(workspaceRoot string) Tool { - return NewScopedLSPNavigateTool(workspaceRoot, nil) -} - // NewScopedLSPNavigateTool builds the tool with its own lazily-started LSP // manager (servers spin up on first use and are reused across calls within a // session). The model-supplied path is resolved through the same scoped diff --git a/internal/tools/lsp_navigate_test.go b/internal/tools/lsp_navigate_test.go index 4b1ca312d..3a158e170 100644 --- a/internal/tools/lsp_navigate_test.go +++ b/internal/tools/lsp_navigate_test.go @@ -14,7 +14,7 @@ func TestLSPNavigateConfinesPathToWorkspace(t *testing.T) { // rejected before any read or LSP open, so it can't exfiltrate files off disk // (reachable via indirect prompt injection). root := t.TempDir() - tool := NewLSPNavigateTool(root) // workspace-only scope (nil) + tool := NewScopedLSPNavigateTool(root, nil) // workspace-only scope (nil) escapes := []string{ "/etc/passwd", "../../../../etc/passwd", @@ -41,7 +41,7 @@ func TestLSPNavigateConfinesPathToWorkspace(t *testing.T) { } func TestLSPNavigateRejectsBadArgs(t *testing.T) { - tool := NewLSPNavigateTool(t.TempDir()) + tool := NewScopedLSPNavigateTool(t.TempDir(), nil) cases := []map[string]any{ {}, // missing op + path {"op": "definition"}, // missing path @@ -64,7 +64,7 @@ func TestLSPNavigateUnsupportedFileDegrades(t *testing.T) { if err := os.WriteFile(path, []byte("hello"), 0o644); err != nil { t.Fatal(err) } - tool := NewLSPNavigateTool(root) + tool := NewScopedLSPNavigateTool(root, nil) got := tool.Run(context.Background(), map[string]any{ "op": "definition", "path": "notes.unknownext", "line": 1, "character": 1, }) @@ -77,7 +77,7 @@ func TestLSPNavigateUnsupportedFileDegrades(t *testing.T) { } func TestLSPNavigateIsReadOnly(t *testing.T) { - tool := NewLSPNavigateTool(t.TempDir()) + tool := NewScopedLSPNavigateTool(t.TempDir(), nil) if s := tool.Safety(); s.SideEffect != SideEffectRead || s.Permission != PermissionAllow { t.Fatalf("lsp_navigate should be read-only/allow, got %+v", s) } diff --git a/internal/tools/read_exclusions_test.go b/internal/tools/read_exclusions_test.go index 2393d14bd..1feb9880b 100644 --- a/internal/tools/read_exclusions_test.go +++ b/internal/tools/read_exclusions_test.go @@ -48,7 +48,7 @@ func denyReadFixture(t *testing.T) (string, *sandbox.Engine) { func TestGrepSkipsDenyReadSubtree(t *testing.T) { ws, engine := denyReadFixture(t) - tool, ok := NewGrepTool(ws).(sandboxAwareTool) + tool, ok := NewScopedGrepTool(ws, nil).(sandboxAwareTool) if !ok { t.Fatal("grep tool must be sandbox-aware") } @@ -67,7 +67,7 @@ func TestGrepSkipsDenyReadSubtree(t *testing.T) { } // Without a sandbox, the same search includes the secret file (default behavior). - plain := NewGrepTool(ws).Run(context.Background(), args) + plain := NewScopedGrepTool(ws, nil).Run(context.Background(), args) if !strings.Contains(plain.Output, "creds.go") { t.Fatalf("non-sandboxed grep should include the secret file, got:\n%s", plain.Output) } @@ -75,7 +75,7 @@ func TestGrepSkipsDenyReadSubtree(t *testing.T) { func TestGlobSkipsDenyReadSubtree(t *testing.T) { ws, engine := denyReadFixture(t) - tool, ok := NewGlobTool(ws).(sandboxAwareTool) + tool, ok := NewScopedGlobTool(ws, nil).(sandboxAwareTool) if !ok { t.Fatal("glob tool must be sandbox-aware") } @@ -92,7 +92,7 @@ func TestGlobSkipsDenyReadSubtree(t *testing.T) { t.Fatalf("glob must NOT surface a DenyRead file, got:\n%s", sandboxed.Output) } - plain := NewGlobTool(ws).Run(context.Background(), args) + plain := NewScopedGlobTool(ws, nil).Run(context.Background(), args) if !strings.Contains(plain.Output, "creds.go") { t.Fatalf("non-sandboxed glob should include the secret file, got:\n%s", plain.Output) } diff --git a/internal/tools/read_minified_file_test.go b/internal/tools/read_minified_file_test.go index 55bd72325..b2dd19f44 100644 --- a/internal/tools/read_minified_file_test.go +++ b/internal/tools/read_minified_file_test.go @@ -14,7 +14,7 @@ func TestReadMinifiedFileStripsCommentsAndLineNumbers(t *testing.T) { if err := os.WriteFile(filepath.Join(dir, "f.go"), []byte(src), 0o644); err != nil { t.Fatal(err) } - res := NewReadMinifiedFileTool(dir).Run(context.Background(), map[string]any{"path": "f.go"}) + res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{"path": "f.go"}) if res.Status != StatusOK { t.Fatalf("status %v: %s", res.Status, res.Output) } @@ -39,7 +39,7 @@ func TestReadMinifiedFileStripsCommentsAndLineNumbers(t *testing.T) { func TestReadMinifiedFileRejectsTraversal(t *testing.T) { dir := t.TempDir() - res := NewReadMinifiedFileTool(dir).Run(context.Background(), map[string]any{"path": "../escape.go"}) + res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{"path": "../escape.go"}) if res.Status == StatusOK { t.Fatalf("expected traversal rejection, got OK:\n%s", res.Output) } @@ -51,7 +51,7 @@ func TestReadMinifiedFileAppliesByteBudget(t *testing.T) { t.Fatal(err) } - res := NewReadMinifiedFileTool(dir).Run(context.Background(), map[string]any{"path": "large.txt"}) + res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{"path": "large.txt"}) if res.Status != StatusOK || !res.Truncated { t.Fatalf("expected ok+truncated, got status=%s truncated=%v", res.Status, res.Truncated) } diff --git a/internal/tools/registry.go b/internal/tools/registry.go index 0a1d2afe9..5d52667af 100644 --- a/internal/tools/registry.go +++ b/internal/tools/registry.go @@ -257,9 +257,6 @@ func scrubResultSecrets(res Result) Result { return res } -func CoreReadOnlyTools(workspaceRoot string) []Tool { - return CoreReadOnlyToolsScoped(workspaceRoot, nil) -} func CoreReadOnlyToolsScoped(workspaceRoot string, scope PathScope) []Tool { return []Tool{ NewScopedReadFileTool(workspaceRoot, scope), @@ -280,7 +277,6 @@ func CoreReadOnlyToolsScoped(workspaceRoot string, scope PathScope) []Tool { } } -func CoreWriteTools(workspaceRoot string) []Tool { return CoreWriteToolsScoped(workspaceRoot, nil) } func CoreWriteToolsScoped(workspaceRoot string, scope PathScope) []Tool { return []Tool{ NewScopedWriteFileTool(workspaceRoot, scope), @@ -290,7 +286,6 @@ func CoreWriteToolsScoped(workspaceRoot string, scope PathScope) []Tool { } } -func CoreShellTools(workspaceRoot string) []Tool { return CoreShellToolsScoped(workspaceRoot, nil) } func CoreShellToolsScoped(workspaceRoot string, scope PathScope) []Tool { execManager := newExecSessionManager() return []Tool{ @@ -312,7 +307,6 @@ func CoreNetworkTools() []Tool { return tools } -func CoreTools(workspaceRoot string) []Tool { return CoreToolsScoped(workspaceRoot, nil) } func CoreToolsScoped(workspaceRoot string, scope PathScope) []Tool { tools := append([]Tool{}, CoreReadOnlyToolsScoped(workspaceRoot, scope)...) tools = append(tools, CoreWriteToolsScoped(workspaceRoot, scope)...) diff --git a/internal/tools/registry_test.go b/internal/tools/registry_test.go index a2526c53c..0b7556a89 100644 --- a/internal/tools/registry_test.go +++ b/internal/tools/registry_test.go @@ -29,7 +29,7 @@ func tempDirOutsideDefaultTemp(t *testing.T) string { } func TestCoreReadOnlyToolsExposeSafeMetadata(t *testing.T) { - toolset := CoreReadOnlyTools(t.TempDir()) + toolset := CoreReadOnlyToolsScoped(t.TempDir(), nil) if len(toolset) != 9 { t.Fatalf("expected 9 core read-only tools, got %d", len(toolset)) } @@ -144,7 +144,7 @@ func TestCoreNetworkToolsOmitWebSearchWhenUnconfigured(t *testing.T) { } func TestCoreToolsIncludeWebFetchButReadOnlyToolsDoNot(t *testing.T) { - readOnly := CoreReadOnlyTools(t.TempDir()) + readOnly := CoreReadOnlyToolsScoped(t.TempDir(), nil) for _, tool := range readOnly { if tool.Name() == "web_fetch" { t.Fatal("web_fetch should not be exposed by read-only core tools") @@ -152,7 +152,7 @@ func TestCoreToolsIncludeWebFetchButReadOnlyToolsDoNot(t *testing.T) { } found := false - for _, tool := range CoreTools(t.TempDir()) { + for _, tool := range CoreToolsScoped(t.TempDir(), nil) { if tool.Name() == "web_fetch" { found = true break @@ -165,7 +165,7 @@ func TestCoreToolsIncludeWebFetchButReadOnlyToolsDoNot(t *testing.T) { func TestRegistryRunsToolsThroughSafePath(t *testing.T) { registry := NewRegistry() - registry.Register(NewReadFileTool(t.TempDir())) + registry.Register(NewScopedReadFileTool(t.TempDir(), nil)) result := registry.Run(context.Background(), "read_file", map[string]any{ "path": "missing.txt", @@ -229,7 +229,7 @@ func TestRegistryAppliesSandboxBeforeToolExecution(t *testing.T) { root := t.TempDir() outside := filepath.Join(tempDirOutsideDefaultTemp(t), "escape.txt") registry := NewRegistry() - registry.Register(NewWriteFileTool(root)) + registry.Register(NewScopedWriteFileTool(root, nil)) engine := sandbox.NewEngine(sandbox.EngineOptions{ WorkspaceRoot: root, Policy: sandbox.DefaultPolicy(), @@ -262,7 +262,7 @@ func TestRegistrySandboxGatesPathAliasKeys(t *testing.T) { root := t.TempDir() outside := filepath.Join(tempDirOutsideDefaultTemp(t), "escape.txt") registry := NewRegistry() - registry.Register(NewWriteFileTool(root)) + registry.Register(NewScopedWriteFileTool(root, nil)) engine := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: root, Policy: sandbox.DefaultPolicy()}) result := registry.RunWithOptions(context.Background(), "write_file", map[string]any{ @@ -302,7 +302,7 @@ func TestRegistryAllowsPromptToolWithPersistentSandboxGrant(t *testing.T) { } registry := NewRegistry() - registry.Register(NewWriteFileTool(root)) + registry.Register(NewScopedWriteFileTool(root, nil)) engine := sandbox.NewEngine(sandbox.EngineOptions{ WorkspaceRoot: root, Policy: sandbox.DefaultPolicy(), diff --git a/internal/tools/search_cancellation_test.go b/internal/tools/search_cancellation_test.go index 1be00c273..dcb6349fd 100644 --- a/internal/tools/search_cancellation_test.go +++ b/internal/tools/search_cancellation_test.go @@ -53,7 +53,7 @@ func buildLargeSearchTree(t *testing.T, n int) string { // application was stuck until the walk finished on its own. func TestGrepStopsWalkOnCancelledContext(t *testing.T) { root := buildLargeSearchTree(t, 500) - tool := NewGrepTool(root) + tool := NewScopedGrepTool(root, nil) ctx, cancel := context.WithCancel(context.Background()) cancel() // already cancelled before the walk starts @@ -69,7 +69,7 @@ func TestGrepStopsWalkOnCancelledContext(t *testing.T) { func TestGrepRunWithSandboxStopsWalkOnCancelledContext(t *testing.T) { root := buildLargeSearchTree(t, 500) - tool := NewGrepTool(root) + tool := NewScopedGrepTool(root, nil) ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -88,7 +88,7 @@ func TestGrepRunWithSandboxStopsWalkOnCancelledContext(t *testing.T) { // fix only short-circuits on cancellation, it does not change matching. func TestGrepStillMatchesWithLiveContext(t *testing.T) { root := buildLargeSearchTree(t, 3) - tool := NewGrepTool(root) + tool := NewScopedGrepTool(root, nil) result := tool.Run(context.Background(), map[string]any{"pattern": "needle"}) if result.Status != StatusOK { @@ -172,7 +172,7 @@ func TestScanGlobStopsMidWalkOnCancelledContext(t *testing.T) { func TestGlobStopsWalkOnCancelledContext(t *testing.T) { root := buildLargeSearchTree(t, 500) - tool := NewGlobTool(root) + tool := NewScopedGlobTool(root, nil) ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -188,7 +188,7 @@ func TestGlobStopsWalkOnCancelledContext(t *testing.T) { func TestGlobRunWithSandboxStopsWalkOnCancelledContext(t *testing.T) { root := buildLargeSearchTree(t, 500) - tool := NewGlobTool(root) + tool := NewScopedGlobTool(root, nil) ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -205,7 +205,7 @@ func TestGlobRunWithSandboxStopsWalkOnCancelledContext(t *testing.T) { func TestGlobStillMatchesWithLiveContext(t *testing.T) { root := buildLargeSearchTree(t, 3) - tool := NewGlobTool(root) + tool := NewScopedGlobTool(root, nil) result := tool.Run(context.Background(), map[string]any{"pattern": "**/*.txt"}) if result.Status != StatusOK { diff --git a/internal/tools/spill_hardening_test.go b/internal/tools/spill_hardening_test.go index 77042288d..4385e90b7 100644 --- a/internal/tools/spill_hardening_test.go +++ b/internal/tools/spill_hardening_test.go @@ -84,7 +84,7 @@ func TestReadFileCanReadSpillFile(t *testing.T) { t.Fatal("spill must succeed") } workspace := t.TempDir() - result := NewReadFileTool(workspace).Run(context.Background(), map[string]any{"path": spillPath}) + result := NewScopedReadFileTool(workspace, nil).Run(context.Background(), map[string]any{"path": spillPath}) if result.Status != StatusOK || !strings.Contains(result.Output, "line two") { t.Fatalf("read_file must be able to follow the truncation notice: %s %q", result.Status, result.Output) } diff --git a/internal/tools/tool_io_bench_test.go b/internal/tools/tool_io_bench_test.go index 6517458d8..1374069c5 100644 --- a/internal/tools/tool_io_bench_test.go +++ b/internal/tools/tool_io_bench_test.go @@ -16,7 +16,7 @@ func BenchmarkReadFileLargeRangeSmall(b *testing.B) { return fmt.Sprintf("line-%06d abcdefghijklmnopqrstuvwxyz 0123456789\n", i) }) - tool := NewReadFileTool(root) + tool := NewScopedReadFileTool(root, nil) args := map[string]any{ "path": "large.txt", "start_line": lines / 2, @@ -44,7 +44,7 @@ func BenchmarkGrepLargeTreeHeadLimit(b *testing.B) { }) } - tool := NewGrepTool(root) + tool := NewScopedGrepTool(root, nil) args := map[string]any{ "pattern": "needle", "path": ".", diff --git a/internal/tools/web_fetch.go b/internal/tools/web_fetch.go index be3fe90e9..02154f657 100644 --- a/internal/tools/web_fetch.go +++ b/internal/tools/web_fetch.go @@ -99,10 +99,6 @@ func NewWebFetchTool() Tool { return newWebFetchToolWithClientAndResolver(nil, defaultWebFetchResolver{}) } -func newWebFetchToolWithClient(client *http.Client) Tool { - return newWebFetchToolWithClientAndResolver(client, nil) -} - func newWebFetchToolWithClientAndResolver(client *http.Client, resolver webFetchResolver) Tool { if client == nil { client = &http.Client{Timeout: webFetchTimeout} diff --git a/internal/tools/write_file.go b/internal/tools/write_file.go index febd67fd8..8bbb59ca4 100644 --- a/internal/tools/write_file.go +++ b/internal/tools/write_file.go @@ -14,10 +14,6 @@ type writeFileTool struct { scope PathScope } -func NewWriteFileTool(workspaceRoot string) Tool { - return NewScopedWriteFileTool(workspaceRoot, nil) -} - func NewScopedWriteFileTool(workspaceRoot string, scope PathScope) Tool { return writeFileTool{ baseTool: baseTool{ diff --git a/internal/tools/write_tools_test.go b/internal/tools/write_tools_test.go index 027be8d11..980c9bf03 100644 --- a/internal/tools/write_tools_test.go +++ b/internal/tools/write_tools_test.go @@ -9,7 +9,7 @@ import ( ) func TestCoreToolsExposeWriteAndPlanTools(t *testing.T) { - toolset := CoreTools(t.TempDir()) + toolset := CoreToolsScoped(t.TempDir(), nil) byName := make(map[string]Tool, len(toolset)) for _, tool := range toolset { byName[tool.Name()] = tool @@ -41,7 +41,7 @@ func TestRegistryBlocksPromptToolsWithoutGrant(t *testing.T) { root := t.TempDir() target := filepath.Join(root, "blocked.txt") registry := NewRegistry() - registry.Register(NewWriteFileTool(root)) + registry.Register(NewScopedWriteFileTool(root, nil)) result := registry.Run(context.Background(), "write_file", map[string]any{ "path": "blocked.txt", @@ -62,7 +62,7 @@ func TestRegistryBlocksPromptToolsWithoutGrant(t *testing.T) { func TestRegistryRunsPromptToolsWithGrant(t *testing.T) { root := t.TempDir() registry := NewRegistry() - registry.Register(NewWriteFileTool(root)) + registry.Register(NewScopedWriteFileTool(root, nil)) result := registry.RunWithOptions(context.Background(), "write_file", map[string]any{ "path": "allowed.txt", @@ -83,7 +83,7 @@ func TestRegistryRunsPromptToolsWithGrant(t *testing.T) { func TestWriteFileToolCreatesAndProtectsExistingFiles(t *testing.T) { root := t.TempDir() - tool := NewWriteFileTool(root) + tool := NewScopedWriteFileTool(root, nil) created := tool.Run(context.Background(), map[string]any{ "path": "nested/file.txt", @@ -131,7 +131,7 @@ func TestWriteFileToolCreatesAndProtectsExistingFiles(t *testing.T) { func TestWriteFileToolRecordsCreatedFileButNotOverwrite(t *testing.T) { root := t.TempDir() registry := NewRegistry() - registry.Register(NewWriteFileTool(root)) + registry.Register(NewScopedWriteFileTool(root, nil)) tracker := NewFileTracker() created := registry.RunWithOptions(context.Background(), "write_file", map[string]any{ @@ -171,7 +171,7 @@ func TestApplyPatchRecordsCreatedFileButNotExistingEdits(t *testing.T) { t.Fatal(err) } registry := NewRegistry() - registry.Register(NewApplyPatchTool(root)) + registry.Register(NewScopedApplyPatchTool(root, nil)) tracker := NewFileTracker() patch := strings.Join([]string{ "diff --git a/scratch.txt b/scratch.txt", @@ -211,7 +211,7 @@ func TestApplyPatchRecordsCreatedFileButNotExistingEdits(t *testing.T) { func TestWriteFileSummaryReportsLineCount(t *testing.T) { root := t.TempDir() - tool := NewWriteFileTool(root) + tool := NewScopedWriteFileTool(root, nil) // Three lines, no trailing newline -> "3 lines" (not a byte count). result := tool.Run(context.Background(), map[string]any{ "path": "multi.txt", @@ -231,7 +231,7 @@ func TestWriteFileSummaryReportsLineCount(t *testing.T) { func TestWriteFileToolAllowsEmptyContent(t *testing.T) { root := t.TempDir() - result := NewWriteFileTool(root).Run(context.Background(), map[string]any{ + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "empty.txt", "content": "", }) @@ -249,7 +249,7 @@ func TestWriteFileToolAllowsEmptyContent(t *testing.T) { } func TestWriteFileToolReportsTypeErrorsForEmptyAllowedStrings(t *testing.T) { - result := NewWriteFileTool(t.TempDir()).Run(context.Background(), map[string]any{ + result := NewScopedWriteFileTool(t.TempDir(), nil).Run(context.Background(), map[string]any{ "path": "bad.txt", "content": 42, }) @@ -266,7 +266,7 @@ func TestWriteFileToolRejectsOutsideWorkspace(t *testing.T) { root := t.TempDir() outside := filepath.Join(t.TempDir(), "outside.txt") - result := NewWriteFileTool(root).Run(context.Background(), map[string]any{ + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ "path": outside, "content": "secret", }) @@ -292,7 +292,7 @@ func TestWriteFileToolRejectsSymlinkParent(t *testing.T) { t.Skipf("symlink unavailable: %v", err) } - result := NewWriteFileTool(root).Run(context.Background(), map[string]any{ + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "link/escape.txt", "content": "secret", }) @@ -313,7 +313,7 @@ func TestEditFileToolReplacesExactStrings(t *testing.T) { path := filepath.Join(root, "code.go") writeTestFile(t, path, "const a = 1\nconst b = 2\n") - result := NewEditFileTool(root).Run(context.Background(), map[string]any{ + result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "code.go", "old_string": "const a = 1", "new_string": "const a = 42", @@ -336,7 +336,7 @@ func TestEditFileToolReplacesCRLF(t *testing.T) { path := filepath.Join(root, "code.go") writeTestFile(t, path, "const a = 1\r\nconst b = 2\r\n") - result := NewEditFileTool(root).Run(context.Background(), map[string]any{ + result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "code.go", "old_string": "const a = 1\nconst b = 2", "new_string": "const a = 42\nconst b = 24", @@ -357,7 +357,7 @@ func TestEditFileToolReplacesCRLF(t *testing.T) { func TestEditFileToolEmitsUnifiedDiff(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "code.go"), "const a = 1\nconst b = 2\n") - res := NewEditFileTool(root).Run(context.Background(), map[string]any{ + res := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "code.go", "old_string": "const a = 1", "new_string": "const a = 42", }) if res.Status != StatusOK { @@ -380,7 +380,7 @@ func TestEditFileToolEmitsUnifiedDiff(t *testing.T) { func TestWriteFileToolEmitsAdditionsDiff(t *testing.T) { root := t.TempDir() - res := NewWriteFileTool(root).Run(context.Background(), map[string]any{ + res := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "new.txt", "content": "line one\nline two\n", }) if res.Status != StatusOK { @@ -402,7 +402,7 @@ func TestWriteFileToolEmitsAdditionsDiff(t *testing.T) { func TestWriteFileToolOverwriteEmitsRedGreenDiff(t *testing.T) { root := t.TempDir() writeTestFile(t, filepath.Join(root, "f.txt"), "old line\nkeep\n") - res := NewWriteFileTool(root).Run(context.Background(), map[string]any{ + res := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "f.txt", "content": "new line\nkeep\n", "overwrite": true, }) if res.Status != StatusOK { @@ -423,7 +423,7 @@ func TestEditFileToolAllowsDeletingRegions(t *testing.T) { path := filepath.Join(root, "notes.txt") writeTestFile(t, path, "keep\nremove\nkeep\n") - result := NewEditFileTool(root).Run(context.Background(), map[string]any{ + result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "notes.txt", "old_string": "remove\n", "new_string": "", @@ -445,7 +445,7 @@ func TestEditFileToolRejectsMissingAndAmbiguousMatches(t *testing.T) { root := t.TempDir() path := filepath.Join(root, "dup.txt") writeTestFile(t, path, "x\nx\n") - tool := NewEditFileTool(root) + tool := NewScopedEditFileTool(root, nil) missing := tool.Run(context.Background(), map[string]any{ "path": "dup.txt", @@ -497,7 +497,7 @@ func TestApplyPatchToolAppliesUnifiedDiff(t *testing.T) { "", }, "\n") - result := NewApplyPatchTool(root).Run(context.Background(), map[string]any{ + result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{ "patch": patch, }) @@ -529,7 +529,7 @@ func TestApplyPatchToolHandlesHunkBodyLookingLikeHeader(t *testing.T) { "", }, "\n") - result := NewApplyPatchTool(root).Run(context.Background(), map[string]any{"patch": patch}) + result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch}) if result.Status != StatusOK { t.Fatalf("expected patch ok (hunk body must not be parsed as a header), got %s: %s", result.Status, result.Output) @@ -565,7 +565,7 @@ func TestApplyPatchToolRejectsHunkCountInflationHidingEscapePath(t *testing.T) { "", }, "\n") - result := NewApplyPatchTool(root).Run(context.Background(), map[string]any{"patch": patch}) + result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch}) if result.Status != StatusError { t.Fatalf("crafted hunk header must not hide the out-of-workspace path, got %s: %s", result.Status, result.Output) @@ -595,7 +595,7 @@ func TestApplyPatchToolRejectsSymlinkPath(t *testing.T) { "", }, "\n") - result := NewApplyPatchTool(root).Run(context.Background(), map[string]any{ + result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{ "patch": patch, }) @@ -614,7 +614,7 @@ func TestApplyPatchToolRejectsOutsideWorkspace(t *testing.T) { root := t.TempDir() outside := t.TempDir() - result := NewApplyPatchTool(root).Run(context.Background(), map[string]any{ + result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{ "cwd": outside, "patch": strings.Join([]string{ "diff --git a/nope.txt b/nope.txt", @@ -648,7 +648,7 @@ func TestApplyPatchReportsWorkspaceRelativeChangedFilesUnderCwd(t *testing.T) { } patch := "--- a/a.txt\n+++ b/a.txt\n@@ -1 +1 @@\n-one\n+two\n" - res := NewApplyPatchTool(root).Run(context.Background(), map[string]any{"patch": patch, "cwd": "sub/dir"}) + res := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch, "cwd": "sub/dir"}) if res.Status != StatusOK { if gitApplyUnavailable(res.Output) { t.Skipf("git binary unavailable: %s", res.Output) @@ -662,7 +662,7 @@ func TestApplyPatchReportsWorkspaceRelativeChangedFilesUnderCwd(t *testing.T) { func TestWriteFileReportsChangedFileAndDisplay(t *testing.T) { root := t.TempDir() - res := NewWriteFileTool(root).Run(context.Background(), map[string]any{"path": "notes.txt", "content": "hello"}) + res := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{"path": "notes.txt", "content": "hello"}) if res.Status != StatusOK { t.Fatalf("status=%s output=%s", res.Status, res.Output) } @@ -682,7 +682,7 @@ func TestEditFileReportsChangedFileAndDisplay(t *testing.T) { if err := os.WriteFile(filepath.Join(root, "f.txt"), []byte("alpha beta"), 0o644); err != nil { t.Fatal(err) } - res := NewEditFileTool(root).Run(context.Background(), map[string]any{"path": "f.txt", "old_string": "alpha", "new_string": "gamma"}) + res := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{"path": "f.txt", "old_string": "alpha", "new_string": "gamma"}) if res.Status != StatusOK { t.Fatalf("status=%s output=%s", res.Status, res.Output) } @@ -700,7 +700,7 @@ func TestApplyPatchReportsChangedFiles(t *testing.T) { t.Fatal(err) } patch := "--- a/a.txt\n+++ b/a.txt\n@@ -1 +1 @@\n-one\n+two\n" - res := NewApplyPatchTool(root).Run(context.Background(), map[string]any{"patch": patch}) + res := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch}) if res.Status != StatusOK { if gitApplyUnavailable(res.Output) { t.Skipf("git binary unavailable: %s", res.Output) @@ -724,7 +724,7 @@ func TestApplyPatchReportsChangedFiles(t *testing.T) { func TestWriteFileAcceptsContentAlias(t *testing.T) { root := t.TempDir() // minimax-style: content under an alias key instead of "content". - res := NewWriteFileTool(root).Run(context.Background(), map[string]any{ + res := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ "path": "shop.html", "contents": "hi", }) diff --git a/internal/trace/emit.go b/internal/trace/emit.go index 8a63cb354..e6270126f 100644 --- a/internal/trace/emit.go +++ b/internal/trace/emit.go @@ -2,7 +2,6 @@ package trace import ( "encoding/json" - "fmt" "io" "sort" "time" @@ -161,94 +160,12 @@ func WriteNDJSON(w io.Writer, t *TurnTrace) error { return nil } -// WriteText emits a human-readable trace: a header, one line per span with its -// exclusive time and share of wall, a coverage line, then counters. It returns -// the first write error encountered so a failing sink (e.g. a full disk) is not -// silently swallowed. -func WriteText(w io.Writer, t *TurnTrace) error { - if w == nil || t == nil { - return nil - } - wall := t.WallDuration() - var firstErr error - write := func(format string, args ...any) { - if firstErr != nil { - return - } - if _, err := fmt.Fprintf(w, format, args...); err != nil { - firstErr = err - } - } - write("trace run=%s session=%s profile=%s\n", t.RunID, t.SessionID, t.Profile) - write(" started=%s completed=%s wall=%s\n", formatTime(t.StartedAt), formatTime(t.CompletedAt), wall) - write(" attributed=%s coverage=%.1f%%\n", t.AttributedDuration(), t.Coverage()*100) - if !t.FirstVisibleEventAt.IsZero() { - write(" first_visible_event=%s (+%s)\n", formatTime(t.FirstVisibleEventAt), t.FirstVisibleEventAt.Sub(t.StartedAt)) - } - if !t.FirstUsefulActionAt.IsZero() { - write(" first_useful_action=%s (+%s)\n", formatTime(t.FirstUsefulActionAt), t.FirstUsefulActionAt.Sub(t.StartedAt)) - } - if !t.FirstTokenAt.IsZero() { - write(" first_token=%s (+%s)\n", formatTime(t.FirstTokenAt), t.FirstTokenAt.Sub(t.StartedAt)) - } - - spans := append([]Span(nil), t.Spans...) - sort.SliceStable(spans, func(i, j int) bool { - if spans[i].Name != spans[j].Name { - return spans[i].Name < spans[j].Name - } - return spans[i].Start.Before(spans[j].Start) - }) - write("spans:\n") - for _, span := range spans { - share := 0.0 - if wall > 0 { - share = float64(span.Exclusive) / float64(wall) - } - parent := "" - if span.Parent != "" { - parent = " [" + span.Parent + "]" - } - write(" %-18s %10s excl=%-10s %5.1f%%%s\n", span.Name, span.Duration, span.Exclusive, share*100, parent) - } - - counters := append([]Counter(nil), t.Counters...) - sort.Slice(counters, func(i, j int) bool { return counters[i].Name < counters[j].Name }) - write("counters:\n") - for _, c := range counters { - write(" %-22s %d\n", c.Name, c.Value) - } - if len(t.OutputBudgets) > 0 { - write("output budgets:\n") - for _, event := range t.OutputBudgets { - write(" tool=%s category=%s bytes=%d/%d tokens=%d/%d truncated=%t reason=%s spill=%t\n", - event.Tool, event.Category, event.RetainedBytes, event.OriginalBytes, - event.EstimatedRetainedTokens, event.EstimatedOriginalTokens, - event.Truncated, event.Reason, event.SpillCreated) - } - } - if len(t.TaskStates) > 0 { - latest := t.TaskStates[len(t.TaskStates)-1] - write("task state: revision=%d status=%s plan=%d/%d/%d/%d tools=%d/%d verification=%d/%d(%s) files=%d plan_parity=%s completion=%s\n", - latest.Revision, latest.Status, latest.PlanPending, latest.PlanInProgress, - latest.PlanCompleted, latest.PlanFailed, latest.ToolsSucceeded, - latest.ToolsFailed, latest.VerificationPassed, latest.VerificationFailed, - latest.VerificationOutcome, latest.ChangedFileCount, latest.PlanParity, - latest.CompletionDecision) - } - return firstErr -} - // NDJSONSink adapts an io.Writer as a Sink emitting NDJSON. type NDJSONSink struct{ W io.Writer } -func (s NDJSONSink) Emit(t *TurnTrace) error { return WriteNDJSON(s.W, t) } - // TextSink adapts an io.Writer as a Sink emitting human-readable text. type TextSink struct{ W io.Writer } -func (s TextSink) Emit(t *TurnTrace) error { return WriteText(s.W, t) } - // opentelemetrySink is a placeholder documenting the future OpenTelemetry // export path. It is intentionally not implemented in the baseline: doing so // would pull in the OTLP exporter dependency. When added, satisfy Sink by diff --git a/internal/trace/export_test.go b/internal/trace/export_test.go new file mode 100644 index 000000000..9ad0bbd1c --- /dev/null +++ b/internal/trace/export_test.go @@ -0,0 +1,86 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package trace + +import ( + "fmt" + "io" + "sort" +) + +// WriteText emits a human-readable trace: a header, one line per span with its +// exclusive time and share of wall, a coverage line, then counters. It returns +// the first write error encountered so a failing sink (e.g. a full disk) is not +// silently swallowed. +func WriteText(w io.Writer, t *TurnTrace) error { + if w == nil || t == nil { + return nil + } + wall := t.WallDuration() + var firstErr error + write := func(format string, args ...any) { + if firstErr != nil { + return + } + if _, err := fmt.Fprintf(w, format, args...); err != nil { + firstErr = err + } + } + write("trace run=%s session=%s profile=%s\n", t.RunID, t.SessionID, t.Profile) + write(" started=%s completed=%s wall=%s\n", formatTime(t.StartedAt), formatTime(t.CompletedAt), wall) + write(" attributed=%s coverage=%.1f%%\n", t.AttributedDuration(), t.Coverage()*100) + if !t.FirstVisibleEventAt.IsZero() { + write(" first_visible_event=%s (+%s)\n", formatTime(t.FirstVisibleEventAt), t.FirstVisibleEventAt.Sub(t.StartedAt)) + } + if !t.FirstUsefulActionAt.IsZero() { + write(" first_useful_action=%s (+%s)\n", formatTime(t.FirstUsefulActionAt), t.FirstUsefulActionAt.Sub(t.StartedAt)) + } + if !t.FirstTokenAt.IsZero() { + write(" first_token=%s (+%s)\n", formatTime(t.FirstTokenAt), t.FirstTokenAt.Sub(t.StartedAt)) + } + + spans := append([]Span(nil), t.Spans...) + sort.SliceStable(spans, func(i, j int) bool { + if spans[i].Name != spans[j].Name { + return spans[i].Name < spans[j].Name + } + return spans[i].Start.Before(spans[j].Start) + }) + write("spans:\n") + for _, span := range spans { + share := 0.0 + if wall > 0 { + share = float64(span.Exclusive) / float64(wall) + } + parent := "" + if span.Parent != "" { + parent = " [" + span.Parent + "]" + } + write(" %-18s %10s excl=%-10s %5.1f%%%s\n", span.Name, span.Duration, span.Exclusive, share*100, parent) + } + + counters := append([]Counter(nil), t.Counters...) + sort.Slice(counters, func(i, j int) bool { return counters[i].Name < counters[j].Name }) + write("counters:\n") + for _, c := range counters { + write(" %-22s %d\n", c.Name, c.Value) + } + if len(t.OutputBudgets) > 0 { + write("output budgets:\n") + for _, event := range t.OutputBudgets { + write(" tool=%s category=%s bytes=%d/%d tokens=%d/%d truncated=%t reason=%s spill=%t\n", + event.Tool, event.Category, event.RetainedBytes, event.OriginalBytes, + event.EstimatedRetainedTokens, event.EstimatedOriginalTokens, + event.Truncated, event.Reason, event.SpillCreated) + } + } + if len(t.TaskStates) > 0 { + latest := t.TaskStates[len(t.TaskStates)-1] + write("task state: revision=%d status=%s plan=%d/%d/%d/%d tools=%d/%d verification=%d/%d(%s) files=%d plan_parity=%s completion=%s\n", + latest.Revision, latest.Status, latest.PlanPending, latest.PlanInProgress, + latest.PlanCompleted, latest.PlanFailed, latest.ToolsSucceeded, + latest.ToolsFailed, latest.VerificationPassed, latest.VerificationFailed, + latest.VerificationOutcome, latest.ChangedFileCount, latest.PlanParity, + latest.CompletionDecision) + } + return firstErr +} diff --git a/internal/trace/trace.go b/internal/trace/trace.go index 37b28927a..c48a43aad 100644 --- a/internal/trace/trace.go +++ b/internal/trace/trace.go @@ -305,3 +305,4 @@ func OptionalEventKeys() []string { "task_state", } } + diff --git a/internal/tui/assistant_markdown.go b/internal/tui/assistant_markdown.go index dffb262df..fa8ec7fd5 100644 --- a/internal/tui/assistant_markdown.go +++ b/internal/tui/assistant_markdown.go @@ -1074,15 +1074,6 @@ func joinsPreviousMarkdownWord(text string) bool { return text != "" && strings.Trim(text, ".,!?;:%)]}") == "" } -func renderMarkdownInline(text string) string { - segments := parseMarkdownInline(text) - var builder strings.Builder - for _, segment := range segments { - builder.WriteString(renderMarkdownInlineSegment(segment)) - } - return builder.String() -} - func renderMarkdownInlineSegment(segment markdownInlineSegment) string { if segment.text == "" { return "" diff --git a/internal/tui/autocomplete.go b/internal/tui/autocomplete.go index 56a443d9e..7920f456b 100644 --- a/internal/tui/autocomplete.go +++ b/internal/tui/autocomplete.go @@ -259,14 +259,6 @@ func (m model) matchUserCommandSuggestions(token string) []commandSuggestion { return out } -// matchCommandSuggestions returns commands whose canonical name or any alias has -// the typed prefix (case-insensitive), preserving commandDefinitions order and -// capped at maxCommandSuggestions. A command matched via an alias is still listed -// by its canonical name (completing always inserts the canonical form). -func matchCommandSuggestions(token string) []commandSuggestion { - return matchCommandSuggestionsWithFilter(token, func(commandDefinition) bool { return true }) -} - func matchCommandSuggestionsWithFilter(token string, include func(commandDefinition) bool) []commandSuggestion { prefix := strings.ToLower(strings.TrimSpace(token)) if prefix == "" { @@ -496,10 +488,6 @@ func expandSpecialistMention(prompt string, specialists []agent.SpecialistInfo) " specialist, then report its result back to me:\n\n" + task, true } -func completePathQuery(value string, cursorPos int, selectedPath string) (string, int) { - return completePathQueryWithTrailingSpace(value, cursorPos, selectedPath, true) -} - func completePathQueryWithTrailingSpace(value string, cursorPos int, selectedPath string, trailingSpace bool) (string, int) { query := extractPathQuery(value, cursorPos) suffix := "" diff --git a/internal/tui/command_polish_test.go b/internal/tui/command_polish_test.go index 2758171fe..39c35c2b0 100644 --- a/internal/tui/command_polish_test.go +++ b/internal/tui/command_polish_test.go @@ -135,7 +135,7 @@ func TestToolsCommandRendersCommandCard(t *testing.T) { assertNotContains(t, emptyText, "registered tools:") registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(".")) + registry.Register(tools.NewScopedReadFileTool(".", nil)) m = newModel(context.Background(), Options{ Registry: registry, }) @@ -244,7 +244,7 @@ func TestToolsCommandShowsFullSortedCatalog(t *testing.T) { func TestContextAndPermissionsCommandsRenderProductState(t *testing.T) { registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(".")) + registry.Register(tools.NewScopedReadFileTool(".", nil)) store, err := sandbox.NewGrantStore(sandbox.StoreOptions{FilePath: filepath.Join(t.TempDir(), "sandbox-grants.json")}) if err != nil { diff --git a/internal/tui/commands.go b/internal/tui/commands.go index 766e1b891..889e9a26a 100644 --- a/internal/tui/commands.go +++ b/internal/tui/commands.go @@ -434,32 +434,6 @@ func resolveCommand(name string) (commandDefinition, bool) { return commandDefinition{}, false } -func listCommandNames() []string { - names := make([]string, 0, len(commandDefinitions)) - for _, command := range commandDefinitions { - names = append(names, command.name) - names = append(names, command.aliases...) - } - return names -} - -func formatCommandHelpLines() []string { - return formatGroupedCommandHelpLines() -} - -func formatGroupedCommandHelpLines() []string { - lines := make([]string, 0, len(commandDefinitions)+len(commandGroupOrder())) - for _, group := range commandGroupOrder() { - groupLines := commandHelpLinesForGroup(group) - if len(groupLines) == 0 { - continue - } - lines = append(lines, string(group)+":") - lines = append(lines, groupLines...) - } - return lines -} - func formatGroupedCommandHelp() string { lines := []string{"Commands", "status: info"} for _, group := range commandGroupOrder() { diff --git a/internal/tui/export_test.go b/internal/tui/export_test.go new file mode 100644 index 000000000..f9af6f45a --- /dev/null +++ b/internal/tui/export_test.go @@ -0,0 +1,298 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package tui + +import ( + "context" + "fmt" + "os" + "strings" + "time" + + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + "github.com/Gitlawb/zero/internal/dictation" +) + +func renderMarkdownInline(text string) string { + segments := parseMarkdownInline(text) + var builder strings.Builder + for _, segment := range segments { + builder.WriteString(renderMarkdownInlineSegment(segment)) + } + return builder.String() +} + +func completePathQuery(value string, cursorPos int, selectedPath string) (string, int) { + return completePathQueryWithTrailingSpace(value, cursorPos, selectedPath, true) +} + +// matchCommandSuggestions returns commands whose canonical name or any alias has +// the typed prefix (case-insensitive), preserving commandDefinitions order and +// capped at maxCommandSuggestions. A command matched via an alias is still listed +// by its canonical name (completing always inserts the canonical form). +func matchCommandSuggestions(token string) []commandSuggestion { + return matchCommandSuggestionsWithFilter(token, func(commandDefinition) bool { return true }) +} + +func formatCommandHelpLines() []string { + return formatGroupedCommandHelpLines() +} + +func formatGroupedCommandHelpLines() []string { + lines := make([]string, 0, len(commandDefinitions)+len(commandGroupOrder())) + for _, group := range commandGroupOrder() { + groupLines := commandHelpLinesForGroup(group) + if len(groupLines) == 0 { + continue + } + lines = append(lines, string(group)+":") + lines = append(lines, groupLines...) + } + return lines +} + +func listCommandNames() []string { + names := make([]string, 0, len(commandDefinitions)) + for _, command := range commandDefinitions { + names = append(names, command.name) + names = append(names, command.aliases...) + } + return names +} + +// renderImageChips builds a compact "[Image #1] [Image #2]" row for the pending +// image attachments, or "" when there are none, so the long file name never +// clutters the input. Kept plain so the renderer can wrap/style it consistently. +func renderImageChips(labels []string) string { + if len(labels) == 0 { + return "" + } + chips := make([]string, 0, len(labels)) + for i := range labels { + chips = append(chips, fmt.Sprintf("[Image #%d]", i+1)) + } + return strings.Join(chips, " ") +} + +func (m model) chatMaxScrollOffset() int { + _, maxOffset := m.chatScrollMetrics() + return maxOffset +} + +func (m model) scrollableTranscriptLayoutView(header string, body transcriptBodyLayout, footer string, width int, overlay string) string { + frame := m.scrollableTranscriptFrame(header, footer) + window := transcriptViewportForLayout(body, frame, m.chatScrollOffset).window() + + bodyWindow := body.visibleLines(window) + return m.renderScrollableTranscriptWindow(frame, bodyWindow, window, width, overlay) +} + +func (m model) scrollableTranscriptView(header string, body string, footer string, width int, overlay string) string { + return m.scrollableTranscriptLayoutView(header, transcriptBodyLayout{lines: viewLines(body)}, footer, width, overlay) +} + +func (m model) overlayMouseTop(overlayHeight int, width int) int { + return m.overlayMouseRect(overlayHeight, width).y +} + +// height returns the number of terminal lines renderPlanPanel will occupy at +// the given width (0 when the panel is not visible). The step list is shown +// when the panel is expanded or still running; a collapsed, finished plan is +// just the header and progress bar. +func (s planPanelState) height(width int, now time.Time) int { + if !s.visible(now) { + return 0 + } + if s.expanded || !s.isComplete() { + return 2 + len(s.steps) + } + return 2 +} + +func GetLocalDiffStats(baseBranch string) (additions int, deletions int, err error) { + cwd, err := os.Getwd() + if err != nil { + return 0, 0, err + } + ctx, cancel := context.WithTimeout(context.Background(), prCommandTimeout) + defer cancel() + return getLocalDiffStats(ctx, cwd, baseBranch, defaultPRCommandRunner) +} + +func WatchPRState(service *PrService, onChange func(PrState)) func() { + return WatchPRStateContext(context.Background(), service, onChange) +} + +func (c *staticRenderCache) retainedCharacters() int { + if c == nil { + return 0 + } + c.mu.Lock() + defer c.mu.Unlock() + return c.retained +} + +func (c *staticRenderCache) stats() renderCacheStats { + if c == nil { + return renderCacheStats{} + } + c.mu.Lock() + defer c.mu.Unlock() + return c.statsData +} + +func renderSelectableList(options selectableListOptions) string { + if len(options.Items) == 0 { + return "" + } + width := options.Width + if width <= 0 { + width = 80 + } + maxVisible := options.MaxVisible + if maxVisible <= 0 || maxVisible > len(options.Items) { + maxVisible = len(options.Items) + } + selected := clampInt(options.Selected, 0, len(options.Items)-1) + start := selectableListStart(len(options.Items), maxVisible, selected) + visible := options.Items[start : start+maxVisible] + + labelWidth := 0 + for _, item := range visible { + if w := lipgloss.Width(item.Label); w > labelWidth { + labelWidth = w + } + } + + lines := make([]string, 0, maxVisible+1) + for index, item := range visible { + absoluteIndex := start + index + surface := zeroTheme.onPanel + marker := surface(zeroTheme.faintest).Render(" ") + if absoluteIndex == selected { + surface = zeroTheme.onSel + marker = surface(zeroTheme.accent).Render("❯ ") + } + + label := surface(zeroTheme.ink).Render(item.Label) + pad := surface(zeroTheme.ink).Render(strings.Repeat(" ", maxInt(0, labelWidth-lipgloss.Width(item.Label)))) + line := marker + label + pad + if strings.TrimSpace(item.Description) != "" { + descWidth := width - lipgloss.Width(marker) - labelWidth - 2 + desc := truncateRunes(item.Description, maxInt(0, descWidth)) + if desc != "" { + line += surface(zeroTheme.faint).Render(" " + desc) + } + } + lines = append(lines, fitStyledLine(line, width)) + } + + if hidden := len(options.Items) - len(visible); hidden > 0 { + lines = append(lines, fitStyledLine(zeroTheme.faint.Render(fmt.Sprintf(" %d more", hidden)), width)) + } + return strings.Join(lines, "\n") +} + +// addTokens adds tokens to the running total for the specialist with +// childSessionID. Unknown specialists are ignored. +func (t *specialistTracker) addTokens(childSessionID string, tokens int) { + for index := range t.specialists { + if t.specialists[index].childSessionID == childSessionID { + t.specialists[index].tokenCount += tokens + return + } + } +} + +// hasRunning reports whether any tracked specialist is still running. +func (t *specialistTracker) hasRunning() bool { + for index := range t.specialists { + if t.specialists[index].status == specialistRunning { + return true + } + } + return false +} + +func borderedBlock(width int, lines []string) string { + return styledBlock(width, lines, zeroTheme.line) +} + +// tailLines returns the last tailCap content lines, including the in-progress one. +func (d *streamingDecoder) tailLines() []string { + out := append([]string(nil), d.tail...) + if len(d.cur) > 0 { + out = append(out, string(d.cur)) + } + if len(out) > d.tailCap { + out = out[len(out)-d.tailCap:] + } + return out +} + +// ensureAgeTickReschedule is a small helper used after a fade-state change +// to start the tick if it's not already running. The age-tick case +// short-circuits when fadeActive is false, so calling this on a no-op +// transition (e.g. a 0-byte delta) is safe. +func (m model) ensureAgeTickReschedule() tea.Cmd { + if !m.fadeActive { + return nil + } + return streamingFadeTick() +} + +// newSTTDownloadPicker builds the model-download chooser, seeded with the +// curated shortlist. The full model list from the release is fetched +// asynchronously and merged in (see fetchSTTModelsCmd / handleSTTModelsFetched). +func (m model) newSTTDownloadPicker() *commandPicker { + return newSTTDownloadPickerFrom(dictation.ModelVariants(), true, m.dictation.downloadRoot, m.engineDownloaded(), m.dictation.cfg.LocalModelPath) +} + +func toolBodyRendererFor(name string) toolBodyRenderer { + return defaultToolBodyRegistry.rendererFor(name) +} + +// renderSelectableSpecialistRow renders a specialist card and marks every line +// as a clickable specialistCard selectable line carrying the childSessionID. +// A left-click or Enter on any card line drills into that specialist's subchat. +func (m model) renderSelectableSpecialistRow(rowIndex int, row transcriptRow, width int, rc rowContext, startBodyY int) (string, []transcriptSelectableLine) { + return m.renderSelectableSpecialistRowFn(rowIndex, row, width, rc, startBodyY, m.renderRow) +} + +func (m model) renderSelectableToolResultRow(rowIndex int, row transcriptRow, width int, rc rowContext, startBodyY int) (string, []transcriptSelectableLine) { + return m.renderSelectableToolResultRowFn(rowIndex, row, width, rc, startBodyY, m.renderRow) +} + +func (m model) transcriptBody(width int, emptyOverlay string) (string, []transcriptSelectableLine) { + layout := m.transcriptBodyLayout(width, emptyOverlay) + return layout.String(), layout.selectable +} + +func (m model) transcriptBodyLayout(width int, emptyOverlay string) transcriptBodyLayout { + return layoutTranscriptBodyItems(m.transcriptBodyItems(width, emptyOverlay, false)) +} + +func (m model) transcriptViewportStart(body string, width int) (int, int, int) { + frame := m.scrollableTranscriptFrame(m.pinnedTitleBar(width), m.footerView(width)) + return transcriptViewportStartForFrame(body, frame, m.chatScrollOffset) +} + +func (l transcriptBodyLayout) visibleLines(window transcriptViewportWindow) []string { + start := clampInt(window.start, 0, len(l.lines)) + end := clampInt(window.end, start, len(l.lines)) + return append([]string(nil), l.lines[start:end]...) +} + +func transcriptViewportStartForFrame(body string, frame transcriptFrameLayout, scrollOffset int) (int, int, int) { + window := transcriptViewportForBody(body, frame, scrollOffset).window() + return window.start, window.height, frame.bodyRect.y +} + +func transcriptViewportStartForLayout(layout transcriptBodyLayout, frame transcriptFrameLayout, scrollOffset int) (int, int, int) { + window := transcriptViewportForLayout(layout, frame, scrollOffset).window() + return window.start, window.height, frame.bodyRect.y +} + +func transcriptViewportForBody(body string, frame transcriptFrameLayout, offset int) transcriptViewport { + return newTranscriptViewport(len(viewLines(body)), frame.bodyRect.height, offset) +} diff --git a/internal/tui/image_attach.go b/internal/tui/image_attach.go index abd862e85..ed06a873d 100644 --- a/internal/tui/image_attach.go +++ b/internal/tui/image_attach.go @@ -274,20 +274,6 @@ func (m model) removeLastAttachment() (model, bool) { return m, false } -// renderImageChips builds a compact "[Image #1] [Image #2]" row for the pending -// image attachments, or "" when there are none, so the long file name never -// clutters the input. Kept plain so the renderer can wrap/style it consistently. -func renderImageChips(labels []string) string { - if len(labels) == 0 { - return "" - } - chips := make([]string, 0, len(labels)) - for i := range labels { - chips = append(chips, fmt.Sprintf("[Image #%d]", i+1)) - } - return strings.Join(chips, " ") -} - // renderAttachmentChips builds the pending-attachment row from both staged images // and staged documents, e.g. "[Image #1] [Image #2] [Doc #1]". Returns "" when // nothing is staged. Numbered (not named) so a long screenshot path never shows diff --git a/internal/tui/model.go b/internal/tui/model.go index 280c8ccfb..3c0a48c6c 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -3041,18 +3041,6 @@ func (f transcriptFrameLayout) footerLineRect(line int) tuiRect { } } -func (m model) scrollableTranscriptView(header string, body string, footer string, width int, overlay string) string { - return m.scrollableTranscriptLayoutView(header, transcriptBodyLayout{lines: viewLines(body)}, footer, width, overlay) -} - -func (m model) scrollableTranscriptLayoutView(header string, body transcriptBodyLayout, footer string, width int, overlay string) string { - frame := m.scrollableTranscriptFrame(header, footer) - window := transcriptViewportForLayout(body, frame, m.chatScrollOffset).window() - - bodyWindow := body.visibleLines(window) - return m.renderScrollableTranscriptWindow(frame, bodyWindow, window, width, overlay) -} - func (m model) scrollableTranscriptItemsView(header string, items []transcriptBodyItem, footer string, width int, overlay string) string { frame := m.scrollableTranscriptFrame(header, footer) metrics := measureTranscriptBodyItems(items, m.transcriptBodyHeights) @@ -3203,11 +3191,6 @@ func (m model) scrollChat(delta int) model { return m } -func (m model) chatMaxScrollOffset() int { - _, maxOffset := m.chatScrollMetrics() - return maxOffset -} - func (m model) chatScrollMetrics() (int, int) { viewport, ok := m.chatTranscriptViewport() if !ok { diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 2c4dbfb13..2aa0f65e9 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -352,7 +352,7 @@ func TestClearCommandResetsTranscript(t *testing.T) { func TestToolsCommandListsRegisteredTools(t *testing.T) { registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(".")) + registry.Register(tools.NewScopedReadFileTool(".", nil)) m := newModel(context.Background(), Options{Registry: registry}) m.input.SetValue("/tools") @@ -463,7 +463,7 @@ func TestPlanCommandHandlesMissingPlanTool(t *testing.T) { func TestContextCommandShowsSessionState(t *testing.T) { registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(".")) + registry.Register(tools.NewScopedReadFileTool(".", nil)) m := newModel(context.Background(), Options{ Cwd: `D:\codings\Opensource\Zero`, ProviderName: "openai", diff --git a/internal/tui/mouse.go b/internal/tui/mouse.go index 47e5b4451..de8c2df63 100644 --- a/internal/tui/mouse.go +++ b/internal/tui/mouse.go @@ -568,10 +568,6 @@ func (m model) overlayMouseHit(msg tea.MouseMsg, overlay string, width int) (mou return mouseOverlayHit{x: mouseX(msg) - left, y: mouseY(msg) - rect.y}, true } -func (m model) overlayMouseTop(overlayHeight int, width int) int { - return m.overlayMouseRect(overlayHeight, width).y -} - func (m model) overlayMouseRect(overlayHeight int, width int) tuiRect { if overlayHeight <= 0 { return tuiRect{} diff --git a/internal/tui/plan_panel.go b/internal/tui/plan_panel.go index 088a38a58..5d2c3437e 100644 --- a/internal/tui/plan_panel.go +++ b/internal/tui/plan_panel.go @@ -206,20 +206,6 @@ func (s planPanelState) visible(now time.Time) bool { return true } -// height returns the number of terminal lines renderPlanPanel will occupy at -// the given width (0 when the panel is not visible). The step list is shown -// when the panel is expanded or still running; a collapsed, finished plan is -// just the header and progress bar. -func (s planPanelState) height(width int, now time.Time) int { - if !s.visible(now) { - return 0 - } - if s.expanded || !s.isComplete() { - return 2 + len(s.steps) - } - return 2 -} - // planNow returns the clock used for live plan durations. While the agent is // idle (no run in flight, activeRunID == 0) it freezes at the moment the last // run ended, so an in_progress step left mid-plan when the agent yields (e.g. diff --git a/internal/tui/pr_status.go b/internal/tui/pr_status.go index ae35cccf1..6f661a7f8 100644 --- a/internal/tui/pr_status.go +++ b/internal/tui/pr_status.go @@ -149,10 +149,6 @@ func (s *PrService) detect(ctx context.Context) PrState { return PrState{Status: PrNotFound} } -func WatchPRState(service *PrService, onChange func(PrState)) func() { - return WatchPRStateContext(context.Background(), service, onChange) -} - func WatchPRStateContext(ctx context.Context, service *PrService, onChange func(PrState)) func() { if service == nil || onChange == nil { return func() {} @@ -237,16 +233,6 @@ func detectGitLabMR(ctx context.Context, cwd string, run prCommandRunner) (PrSta return state, true } -func GetLocalDiffStats(baseBranch string) (additions int, deletions int, err error) { - cwd, err := os.Getwd() - if err != nil { - return 0, 0, err - } - ctx, cancel := context.WithTimeout(context.Background(), prCommandTimeout) - defer cancel() - return getLocalDiffStats(ctx, cwd, baseBranch, defaultPRCommandRunner) -} - func getLocalDiffStats(ctx context.Context, cwd string, baseBranch string, run prCommandRunner) (int, int, error) { baseBranch = strings.TrimSpace(baseBranch) if baseBranch == "" { diff --git a/internal/tui/render_cache.go b/internal/tui/render_cache.go index 0c8937392..a5ba02059 100644 --- a/internal/tui/render_cache.go +++ b/internal/tui/render_cache.go @@ -105,24 +105,6 @@ func (c *staticRenderCache) evictOverflow() { } } -func (c *staticRenderCache) stats() renderCacheStats { - if c == nil { - return renderCacheStats{} - } - c.mu.Lock() - defer c.mu.Unlock() - return c.statsData -} - -func (c *staticRenderCache) retainedCharacters() int { - if c == nil { - return 0 - } - c.mu.Lock() - defer c.mu.Unlock() - return c.retained -} - func (m model) renderRowCacheKey(row transcriptRow, width int, rc rowContext, opts cardRenderOptions, flush bool) (string, bool) { stable := true switch row.kind { diff --git a/internal/tui/selectable_list.go b/internal/tui/selectable_list.go index 454630933..540f8e997 100644 --- a/internal/tui/selectable_list.go +++ b/internal/tui/selectable_list.go @@ -1,12 +1,5 @@ package tui -import ( - "fmt" - "strings" - - "charm.land/lipgloss/v2" -) - type selectableListItem struct { Label string Description string @@ -21,58 +14,6 @@ type selectableListOptions struct { const selectableListAnchorRow = 3 -func renderSelectableList(options selectableListOptions) string { - if len(options.Items) == 0 { - return "" - } - width := options.Width - if width <= 0 { - width = 80 - } - maxVisible := options.MaxVisible - if maxVisible <= 0 || maxVisible > len(options.Items) { - maxVisible = len(options.Items) - } - selected := clampInt(options.Selected, 0, len(options.Items)-1) - start := selectableListStart(len(options.Items), maxVisible, selected) - visible := options.Items[start : start+maxVisible] - - labelWidth := 0 - for _, item := range visible { - if w := lipgloss.Width(item.Label); w > labelWidth { - labelWidth = w - } - } - - lines := make([]string, 0, maxVisible+1) - for index, item := range visible { - absoluteIndex := start + index - surface := zeroTheme.onPanel - marker := surface(zeroTheme.faintest).Render(" ") - if absoluteIndex == selected { - surface = zeroTheme.onSel - marker = surface(zeroTheme.accent).Render("❯ ") - } - - label := surface(zeroTheme.ink).Render(item.Label) - pad := surface(zeroTheme.ink).Render(strings.Repeat(" ", maxInt(0, labelWidth-lipgloss.Width(item.Label)))) - line := marker + label + pad - if strings.TrimSpace(item.Description) != "" { - descWidth := width - lipgloss.Width(marker) - labelWidth - 2 - desc := truncateRunes(item.Description, maxInt(0, descWidth)) - if desc != "" { - line += surface(zeroTheme.faint).Render(" " + desc) - } - } - lines = append(lines, fitStyledLine(line, width)) - } - - if hidden := len(options.Items) - len(visible); hidden > 0 { - lines = append(lines, fitStyledLine(zeroTheme.faint.Render(fmt.Sprintf(" %d more", hidden)), width)) - } - return strings.Join(lines, "\n") -} - func selectableListStart(total, maxVisible, selected int) int { if total <= maxVisible { return 0 diff --git a/internal/tui/session_test.go b/internal/tui/session_test.go index 96821794f..4f3d449d1 100644 --- a/internal/tui/session_test.go +++ b/internal/tui/session_test.go @@ -188,7 +188,7 @@ func TestPromptSubmitPersistsToolSessionEvents(t *testing.T) { }, }} registry := tools.NewRegistry() - registry.Register(tools.NewReadFileTool(root)) + registry.Register(tools.NewScopedReadFileTool(root, nil)) m := newModel(context.Background(), Options{ Cwd: root, ProviderName: "openai", @@ -251,7 +251,7 @@ func TestPromptSubmitPersistsPermissionSessionEvents(t *testing.T) { }, }} registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) runtimeMessages := []tea.Msg{} runtimeMessageCh := make(chan tea.Msg, 8) m := newModel(context.Background(), Options{ @@ -366,7 +366,7 @@ func TestPermissionPromptAllowWritesFileAndRecordsDecision(t *testing.T) { textScript("write allowed"), }} registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) runtimeMessageCh := make(chan tea.Msg, 8) m := newPermissionTestModel(root, provider, registry, store, nil, runtimeMessageCh) @@ -414,7 +414,7 @@ func TestPermissionPromptAlwaysPersistsGrantAndSkipsLaterPrompt(t *testing.T) { textScript("second write"), }} registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) runtimeMessageCh := make(chan tea.Msg, 8) m := newPermissionTestModel(root, provider, registry, store, grantStore, runtimeMessageCh) @@ -852,7 +852,7 @@ func TestCancelledRunFlushesCheckpointSessionEvents(t *testing.T) { textScript("never reached"), }} registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) runtimeMessageCh := make(chan tea.Msg, 8) m := newPermissionTestModel(root, provider, registry, store, nil, runtimeMessageCh) m.input.SetValue("rewrite notes") @@ -929,7 +929,7 @@ func TestCtrlCCancelsAndFlushesCheckpointSessionEvents(t *testing.T) { textScript("never reached"), }} registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) runtimeMessageCh := make(chan tea.Msg, 8) m := newPermissionTestModel(root, provider, registry, store, nil, runtimeMessageCh) m.input.SetValue("rewrite notes") diff --git a/internal/tui/spec_mode_test.go b/internal/tui/spec_mode_test.go index 1675ca341..16dcae805 100644 --- a/internal/tui/spec_mode_test.go +++ b/internal/tui/spec_mode_test.go @@ -194,7 +194,7 @@ func TestSpecReviewRejectLaunchesQueuedPrompt(t *testing.T) { func newSpecModeTestModel(root string, provider zeroruntime.Provider, store *sessions.Store) model { registry := tools.NewRegistry() - for _, tool := range tools.CoreTools(root) { + for _, tool := range tools.CoreToolsScoped(root, nil) { registry.Register(tool) } return newModel(context.Background(), Options{ diff --git a/internal/tui/specialist_card.go b/internal/tui/specialist_card.go index cf4624fb5..0ba119ba3 100644 --- a/internal/tui/specialist_card.go +++ b/internal/tui/specialist_card.go @@ -99,17 +99,6 @@ func (t *specialistTracker) incrementToolCount(childSessionID string) { } } -// addTokens adds tokens to the running total for the specialist with -// childSessionID. Unknown specialists are ignored. -func (t *specialistTracker) addTokens(childSessionID string, tokens int) { - for index := range t.specialists { - if t.specialists[index].childSessionID == childSessionID { - t.specialists[index].tokenCount += tokens - return - } - } -} - // setCurrentTool updates the live tool-call progress for the specialist with // childSessionID. Used by specialistProgressMsg to show ↳ toolName detail. func (t *specialistTracker) setCurrentTool(childSessionID, toolName, detail string) { @@ -161,16 +150,6 @@ func (t *specialistTracker) all() []specialistInfo { return out } -// hasRunning reports whether any tracked specialist is still running. -func (t *specialistTracker) hasRunning() bool { - for index := range t.specialists { - if t.specialists[index].status == specialistRunning { - return true - } - } - return false -} - // specialistStatusString returns the lowercase human label for a status. func specialistStatusString(s specialistStatus) string { switch s { diff --git a/internal/tui/startup.go b/internal/tui/startup.go index 69c4c1566..c24dd0902 100644 --- a/internal/tui/startup.go +++ b/internal/tui/startup.go @@ -150,10 +150,6 @@ func Wordmark() string { return strings.Join(lines, "\n") } -func borderedBlock(width int, lines []string) string { - return styledBlock(width, lines, zeroTheme.line) -} - // styledBlock draws a rounded box around lines with the given border style, // padding every row to the full width. func styledBlock(width int, lines []string, borderStyle lipgloss.Style) string { diff --git a/internal/tui/streaming_decoder.go b/internal/tui/streaming_decoder.go index 6d624a0a3..0210a8172 100644 --- a/internal/tui/streaming_decoder.go +++ b/internal/tui/streaming_decoder.go @@ -172,18 +172,6 @@ func (d *streamingDecoder) lineTotal() int { return n } -// tailLines returns the last tailCap content lines, including the in-progress one. -func (d *streamingDecoder) tailLines() []string { - out := append([]string(nil), d.tail...) - if len(d.cur) > 0 { - out = append(out, string(d.cur)) - } - if len(out) > d.tailCap { - out = out[len(out)-d.tailCap:] - } - return out -} - // jsonStringValueStart finds `"key"` then, tolerating whitespace around the colon, // returns the index just past the opening quote of its string value, or -1. func jsonStringValueStart(s, key string) int { diff --git a/internal/tui/streaming_fade.go b/internal/tui/streaming_fade.go index 676ffb9ad..e8aa1ac73 100644 --- a/internal/tui/streaming_fade.go +++ b/internal/tui/streaming_fade.go @@ -276,14 +276,3 @@ func (m model) styleStreamingLine(line string, visualIndex, visualCount int) str bornAt := streamingLineBornAt(visualIndex, visualCount, m.lineAges, m.lastStreamActivity) return ageDimLine(line, bornAt, m.now(), zeroTheme.ink) } - -// ensureAgeTickReschedule is a small helper used after a fade-state change -// to start the tick if it's not already running. The age-tick case -// short-circuits when fadeActive is false, so calling this on a no-op -// transition (e.g. a 0-byte delta) is safe. -func (m model) ensureAgeTickReschedule() tea.Cmd { - if !m.fadeActive { - return nil - } - return streamingFadeTick() -} diff --git a/internal/tui/stt_model_picker.go b/internal/tui/stt_model_picker.go index 97868740d..51ce0fb42 100644 --- a/internal/tui/stt_model_picker.go +++ b/internal/tui/stt_model_picker.go @@ -250,13 +250,6 @@ func (m model) handleSTTModelSelection(value string) (model, string) { return m.openSTTKeyPrompt(provider, value, hasKey), "" } -// newSTTDownloadPicker builds the model-download chooser, seeded with the -// curated shortlist. The full model list from the release is fetched -// asynchronously and merged in (see fetchSTTModelsCmd / handleSTTModelsFetched). -func (m model) newSTTDownloadPicker() *commandPicker { - return newSTTDownloadPickerFrom(dictation.ModelVariants(), true, m.dictation.downloadRoot, m.engineDownloaded(), m.dictation.cfg.LocalModelPath) -} - // engineDownloaded reports whether the shared engine is already on disk. func (m model) engineDownloaded() bool { return dictation.EngineDownloaded(m.dictation.downloadRoot, m.dictation.cfg.EngineVersion) diff --git a/internal/tui/tool_render_registry.go b/internal/tui/tool_render_registry.go index cb0271642..a28506536 100644 --- a/internal/tui/tool_render_registry.go +++ b/internal/tui/tool_render_registry.go @@ -164,10 +164,6 @@ func (registry *toolBodyRegistry) rendererFor(name string) toolBodyRenderer { return unknownToolBodyRenderer{} } -func toolBodyRendererFor(name string) toolBodyRenderer { - return defaultToolBodyRegistry.rendererFor(name) -} - func normalizeToolCardDetail(detail string) string { detail = strings.TrimRight(strings.ReplaceAll(detail, "\r\n", "\n"), "\n") // Terminal tab stops are unknowable from here and break the width math diff --git a/internal/tui/transcript_selection.go b/internal/tui/transcript_selection.go index f81991545..a04fe1724 100644 --- a/internal/tui/transcript_selection.go +++ b/internal/tui/transcript_selection.go @@ -130,15 +130,6 @@ type transcriptBodyLayout struct { spans []transcriptBodyItemSpan } -func (m model) transcriptBodyLayout(width int, emptyOverlay string) transcriptBodyLayout { - return layoutTranscriptBodyItems(m.transcriptBodyItems(width, emptyOverlay, false)) -} - -func (m model) transcriptBody(width int, emptyOverlay string) (string, []transcriptSelectableLine) { - layout := m.transcriptBodyLayout(width, emptyOverlay) - return layout.String(), layout.selectable -} - func (l transcriptBodyLayout) String() string { return strings.Join(l.lines, "\n") } @@ -151,12 +142,6 @@ func (l transcriptBodyLayout) totalLines() int { return len(l.lines) } -func (l transcriptBodyLayout) visibleLines(window transcriptViewportWindow) []string { - start := clampInt(window.start, 0, len(l.lines)) - end := clampInt(window.end, start, len(l.lines)) - return append([]string(nil), l.lines[start:end]...) -} - // padTranscriptBodyLines left-indents transcript body rows when a non-zero // gutter is configured. It is horizontal only — it never changes the line count, // so the width-keyed height cache stays valid. Two-column mode right-pads the @@ -793,10 +778,6 @@ func (m model) renderSelectableToolResultRowFn(rowIndex int, row transcriptRow, return rendered, selectable } -func (m model) renderSelectableToolResultRow(rowIndex int, row transcriptRow, width int, rc rowContext, startBodyY int) (string, []transcriptSelectableLine) { - return m.renderSelectableToolResultRowFn(rowIndex, row, width, rc, startBodyY, m.renderRow) -} - func (m model) renderSelectableRenderedRowFn(rowIndex int, row transcriptRow, width int, rc rowContext, startBodyY int, renderFn rowRenderFn) (string, []transcriptSelectableLine) { rendered := renderFn(row, width, rc) if rendered == "" { @@ -955,13 +936,6 @@ func (m model) renderSelectableSpecialistRowFn(rowIndex int, row transcriptRow, return rendered, selectable } -// renderSelectableSpecialistRow renders a specialist card and marks every line -// as a clickable specialistCard selectable line carrying the childSessionID. -// A left-click or Enter on any card line drills into that specialist's subchat. -func (m model) renderSelectableSpecialistRow(rowIndex int, row transcriptRow, width int, rc rowContext, startBodyY int) (string, []transcriptSelectableLine) { - return m.renderSelectableSpecialistRowFn(rowIndex, row, width, rc, startBodyY, m.renderRow) -} - func (m model) renderSelectableUserRow(rowIndex int, row transcriptRow, width int, startBodyY int) (string, []transcriptSelectableLine) { contentWidth := userPromptContentWidth(width) wrapped := wrapPlainText(row.text, maxInt(1, contentWidth)) @@ -1332,21 +1306,6 @@ func (m model) stopEdgeScroll() model { return m } -func (m model) transcriptViewportStart(body string, width int) (int, int, int) { - frame := m.scrollableTranscriptFrame(m.pinnedTitleBar(width), m.footerView(width)) - return transcriptViewportStartForFrame(body, frame, m.chatScrollOffset) -} - -func transcriptViewportStartForLayout(layout transcriptBodyLayout, frame transcriptFrameLayout, scrollOffset int) (int, int, int) { - window := transcriptViewportForLayout(layout, frame, scrollOffset).window() - return window.start, window.height, frame.bodyRect.y -} - -func transcriptViewportStartForFrame(body string, frame transcriptFrameLayout, scrollOffset int) (int, int, int) { - window := transcriptViewportForBody(body, frame, scrollOffset).window() - return window.start, window.height, frame.bodyRect.y -} - func transcriptSelectionPointForMouse(line transcriptSelectableLine, x int) transcriptSelectionPoint { lineEnd := line.textStart + lipgloss.Width(line.text) return transcriptSelectionPoint{ diff --git a/internal/tui/transcript_viewport.go b/internal/tui/transcript_viewport.go index cc735a1dd..df466a048 100644 --- a/internal/tui/transcript_viewport.go +++ b/internal/tui/transcript_viewport.go @@ -25,10 +25,6 @@ func newTranscriptViewport(totalLines int, height int, offset int) transcriptVie } } -func transcriptViewportForBody(body string, frame transcriptFrameLayout, offset int) transcriptViewport { - return newTranscriptViewport(len(viewLines(body)), frame.bodyRect.height, offset) -} - func transcriptViewportForLayout(layout transcriptBodyLayout, frame transcriptFrameLayout, offset int) transcriptViewport { return newTranscriptViewport(layout.totalLines(), frame.bodyRect.height, offset) } diff --git a/internal/update/export_test.go b/internal/update/export_test.go new file mode 100644 index 000000000..2ae69a340 --- /dev/null +++ b/internal/update/export_test.go @@ -0,0 +1,32 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package update + +// CompareSemver compares two semver-ish release tags. +func CompareSemver(left string, right string) (int, error) { + leftParts, err := parseSemver(left) + if err != nil { + return 0, err + } + rightParts, err := parseSemver(right) + if err != nil { + return 0, err + } + return compareSemverParts(leftParts, rightParts), nil +} + +// NormalizeVersionTag returns a comparable x.y.z version from a release tag. +func NormalizeVersionTag(version string) (string, error) { + return normalizeVersionTag(version) +} + +// ResolveEndpoint resolves a URL or owner/repo slug into a release API endpoint. +func ResolveEndpoint(endpointOrRepository string, repository string) (string, error) { + return resolveEndpoint(endpointOrRepository, repository) +} +func parseSemver(version string) (semverParts, error) { + normalized, err := NormalizeVersionTag(version) + if err != nil { + return semverParts{}, err + } + return parseSemverNormalized(normalized) +} diff --git a/internal/update/update.go b/internal/update/update.go index cd83577fd..43e875d84 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -86,29 +86,6 @@ func Endpoint(repository string) string { return fmt.Sprintf("https://api.github.com/repos/%s/releases/latest", repository) } -// ResolveEndpoint resolves a URL or owner/repo slug into a release API endpoint. -func ResolveEndpoint(endpointOrRepository string, repository string) (string, error) { - return resolveEndpoint(endpointOrRepository, repository) -} - -// NormalizeVersionTag returns a comparable x.y.z version from a release tag. -func NormalizeVersionTag(version string) (string, error) { - return normalizeVersionTag(version) -} - -// CompareSemver compares two semver-ish release tags. -func CompareSemver(left string, right string) (int, error) { - leftParts, err := parseSemver(left) - if err != nil { - return 0, err - } - rightParts, err := parseSemver(right) - if err != nil { - return 0, err - } - return compareSemverParts(leftParts, rightParts), nil -} - // ResolveTarget maps a release target name like windows-x64 to Go build // coordinates and release asset naming fields. func ResolveTarget(target string) (Target, error) { @@ -402,14 +379,6 @@ func normalizeVersionTag(version string) (string, error) { return fmt.Sprintf("%d.%d.%d", major, minor, patch), nil } -func parseSemver(version string) (semverParts, error) { - normalized, err := NormalizeVersionTag(version) - if err != nil { - return semverParts{}, err - } - return parseSemverNormalized(normalized) -} - func parseSemverNormalized(version string) (semverParts, error) { parts := strings.Split(version, ".") if len(parts) != 3 { diff --git a/internal/verify/export_test.go b/internal/verify/export_test.go new file mode 100644 index 000000000..2559a60c5 --- /dev/null +++ b/internal/verify/export_test.go @@ -0,0 +1,40 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package verify + +import ( + "context" + "time" + + "github.com/Gitlawb/zero/internal/redaction" +) + +func RunLoop(ctx context.Context, plan Plan, options LoopOptions) LoopReport { + now := options.Now + if now == nil { + now = time.Now + } + start := now() + report := LoopReport{ + StartedAt: formatTime(start), + OK: false, + } + maxAttempts := firstPositive(options.MaxAttempts, 1) + for attemptNumber := 1; attemptNumber <= maxAttempts; attemptNumber++ { + attemptReport := Run(ctx, plan, options.RunOptions) + attempt := Attempt{Number: attemptNumber, Report: attemptReport} + report.Attempts = append(report.Attempts, attempt) + report.Summary = attemptReport.Summary + if attemptReport.OK { + report.OK = true + break + } + if attemptNumber < maxAttempts && options.OnFailure != nil { + if err := options.OnFailure(ctx, attempt); err != nil { + report.Error = redaction.RedactString(err.Error(), redaction.Options{}) + break + } + } + } + report.EndedAt = formatTime(now()) + return report +} diff --git a/internal/verify/verify.go b/internal/verify/verify.go index a1219e782..363cad6ba 100644 --- a/internal/verify/verify.go +++ b/internal/verify/verify.go @@ -212,37 +212,6 @@ func Run(ctx context.Context, plan Plan, options RunOptions) Report { return report } -func RunLoop(ctx context.Context, plan Plan, options LoopOptions) LoopReport { - now := options.Now - if now == nil { - now = time.Now - } - start := now() - report := LoopReport{ - StartedAt: formatTime(start), - OK: false, - } - maxAttempts := firstPositive(options.MaxAttempts, 1) - for attemptNumber := 1; attemptNumber <= maxAttempts; attemptNumber++ { - attemptReport := Run(ctx, plan, options.RunOptions) - attempt := Attempt{Number: attemptNumber, Report: attemptReport} - report.Attempts = append(report.Attempts, attempt) - report.Summary = attemptReport.Summary - if attemptReport.OK { - report.OK = true - break - } - if attemptNumber < maxAttempts && options.OnFailure != nil { - if err := options.OnFailure(ctx, attempt); err != nil { - report.Error = redaction.RedactString(err.Error(), redaction.Options{}) - break - } - } - } - report.EndedAt = formatTime(now()) - return report -} - func shouldParseTestSummary(check Check) bool { if check.Kind == testrunner.KindTest { return true diff --git a/internal/workspaceindex/workspaceindex.go b/internal/workspaceindex/workspaceindex.go index 2d9a505d2..badfe5ca1 100644 --- a/internal/workspaceindex/workspaceindex.go +++ b/internal/workspaceindex/workspaceindex.go @@ -316,16 +316,6 @@ func normalizeSeparators(rel string) string { return strings.ReplaceAll(filepath.ToSlash(rel), "\\", "/") } -func MaxFileDepth(files []File) int { - maxDepth := 0 - for _, file := range files { - if file.Depth > maxDepth { - maxDepth = file.Depth - } - } - return maxDepth -} - func SortFiles(files []File) { sort.Slice(files, func(i, j int) bool { return files[i].Path < files[j].Path diff --git a/internal/zerocommands/backend_snapshots.go b/internal/zerocommands/backend_snapshots.go index e0f1e8d4a..89a6ac197 100644 --- a/internal/zerocommands/backend_snapshots.go +++ b/internal/zerocommands/backend_snapshots.go @@ -124,22 +124,6 @@ func MCPServerSnapshots(servers []mcp.Server) []MCPServerSnapshot { return snapshots } -// MCPServerSnapshotWithCounts returns a snapshot that also records -// how many tools the server exposes and how many persistent -// approvals are currently recorded. A nil counts struct is treated -// as zero values so callers that do not have a live registry can -// still call this helper. -func MCPServerSnapshotWithCounts(server mcp.Server, counts *MCPServerCounts) MCPServerSnapshot { - snapshot := MCPServerSnapshotFromServer(server) - if counts == nil { - return snapshot - } - snapshot.ToolCount = counts.ToolCount - snapshot.AllowGranted = counts.AllowGranted - snapshot.DenyGranted = counts.DenyGranted - return snapshot -} - // MCPServerCounts is the optional runtime count bundle that the // snapshot can carry. The struct is split out so callers that // have a live tool registry and a live permission store can @@ -178,14 +162,6 @@ func HookSnapshots(definitions []hooks.Definition) []HookSnapshot { return hookSnapshotsWithSource(definitions, "") } -// HookSnapshotsWithSource converts a slice of hooks.Definition and -// tags every snapshot with the same source string. The helper -// exists so callers that have a single hooks.LoadResult can build -// the snapshot slice in one pass. -func HookSnapshotsWithSource(definitions []hooks.Definition, source hooks.ConfigSource) []HookSnapshot { - return hookSnapshotsWithSource(definitions, source) -} - func hookSnapshotsWithSource(definitions []hooks.Definition, source hooks.ConfigSource) []HookSnapshot { snapshots := make([]HookSnapshot, 0, len(definitions)) for _, def := range definitions { diff --git a/internal/zerocommands/contracts.go b/internal/zerocommands/contracts.go index 7da2baf81..29a3a0194 100644 --- a/internal/zerocommands/contracts.go +++ b/internal/zerocommands/contracts.go @@ -134,14 +134,6 @@ func UsageError(message string) CommandError { return CommandError{Kind: ErrorKindUsage, Message: strings.TrimSpace(message)} } -func RuntimeError(message string) CommandError { - return CommandError{Kind: ErrorKindRuntime, Message: strings.TrimSpace(message)} -} - -func ProviderError(message string) CommandError { - return CommandError{Kind: ErrorKindProvider, Message: strings.TrimSpace(message), Recoverable: true} -} - func ConfigSnapshotFromResolved(resolved config.ResolvedConfig) ConfigSnapshot { snapshot := ConfigSnapshot{ Runtime: RuntimeGo, diff --git a/internal/zerocommands/export_test.go b/internal/zerocommands/export_test.go new file mode 100644 index 000000000..4b36e3182 --- /dev/null +++ b/internal/zerocommands/export_test.go @@ -0,0 +1,31 @@ +// Test seams: helpers only test code uses, kept out of the production binary. +package zerocommands + +import ( + "github.com/Gitlawb/zero/internal/hooks" + "github.com/Gitlawb/zero/internal/mcp" +) + +// HookSnapshotsWithSource converts a slice of hooks.Definition and +// tags every snapshot with the same source string. The helper +// exists so callers that have a single hooks.LoadResult can build +// the snapshot slice in one pass. +func HookSnapshotsWithSource(definitions []hooks.Definition, source hooks.ConfigSource) []HookSnapshot { + return hookSnapshotsWithSource(definitions, source) +} + +// MCPServerSnapshotWithCounts returns a snapshot that also records +// how many tools the server exposes and how many persistent +// approvals are currently recorded. A nil counts struct is treated +// as zero values so callers that do not have a live registry can +// still call this helper. +func MCPServerSnapshotWithCounts(server mcp.Server, counts *MCPServerCounts) MCPServerSnapshot { + snapshot := MCPServerSnapshotFromServer(server) + if counts == nil { + return snapshot + } + snapshot.ToolCount = counts.ToolCount + snapshot.AllowGranted = counts.AllowGranted + snapshot.DenyGranted = counts.DenyGranted + return snapshot +} diff --git a/internal/zeroruntime/finish_reason_test.go b/internal/zeroruntime/finish_reason_test.go index 6210535c1..3cac7baa8 100644 --- a/internal/zeroruntime/finish_reason_test.go +++ b/internal/zeroruntime/finish_reason_test.go @@ -17,9 +17,6 @@ func TestCollectStreamRecordsFinishReason(t *testing.T) { if got.FinishReason != FinishReasonLength { t.Fatalf("FinishReason = %q, want %q", got.FinishReason, FinishReasonLength) } - if !got.Truncated() { - t.Fatal("Truncated() = false, want true for a length-capped response") - } } // A finish reason may also arrive on an earlier event (e.g. a usage event) and @@ -34,7 +31,4 @@ func TestCollectStreamFinishReasonEmptyOnNormalCompletion(t *testing.T) { if got.FinishReason != "" { t.Fatalf("FinishReason = %q, want empty for a normal completion", got.FinishReason) } - if got.Truncated() { - t.Fatal("Truncated() = true, want false for a normal completion") - } } diff --git a/internal/zeroruntime/helpers.go b/internal/zeroruntime/helpers.go index 0a64f8d7e..0d7a2ec51 100644 --- a/internal/zeroruntime/helpers.go +++ b/internal/zeroruntime/helpers.go @@ -26,13 +26,6 @@ type CollectedStream struct { HasReasoning bool } -// Truncated reports whether the response ended for a non-normal reason (the -// output was cut at the token cap or withheld by a content filter), so callers -// can warn instead of treating a clipped answer as complete. -func (collected CollectedStream) Truncated() bool { - return collected.FinishReason != "" -} - // CollectOptions provides callbacks for consumers that need live stream updates. type CollectOptions struct { OnText func(string) From ee3becbdc90a3befd8336fd0e4ca0b630f3d4e18 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sat, 18 Jul 2026 01:49:13 -0400 Subject: [PATCH 2/6] docs(config): correct ValidateFile's errors.As claim Message is a flattened string (.Error() already called on the parse error), not a wrapped error, so callers can't recover the concrete json error type via errors.As. Fix the comment to match. Co-Authored-By: Claude Sonnet 5 --- internal/config/export_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/config/export_test.go b/internal/config/export_test.go index cb0544124..ac72ce3b8 100644 --- a/internal/config/export_test.go +++ b/internal/config/export_test.go @@ -19,9 +19,9 @@ func ToolsOverride(deferThreshold int) ToolsConfig { // ValidateFile reads and parses path as a Zero FileConfig and runs the same // semantic provider/model rules used during resolution. It returns the parsed // config (zero value on parse failure) plus any structured issues. A parse -// failure yields a single issue whose Message wraps the underlying JSON error -// so callers can extract *json.SyntaxError / *json.UnmarshalTypeError offsets -// via errors.As. +// failure yields a single issue whose Message is the underlying JSON error's +// text (already flattened via Error(), not chained — callers cannot recover +// the original *json.SyntaxError / *json.UnmarshalTypeError via errors.As). func ValidateFile(path string) (FileConfig, []Issue) { data, err := os.ReadFile(path) if err != nil { From 9ee8c70b525caef63a39a8c60e0d859986d251f6 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sat, 18 Jul 2026 16:30:41 -0400 Subject: [PATCH 3/6] chore(trace,skills): drop obsolete Sink adapters and fix stale doc references The dead-code pass removed the Emit methods but left Sink, NDJSONSink, and TextSink exported with a contract nothing implements; remove the types with their contract. Also remove the stray blank line at EOF of trace.go flagged by gofmt, and update skills package docs and HOW_ZERO_WORKS.md to stop directing callers to DiscoveryRoots/Duplicates, which are now test seams. Co-Authored-By: Claude Fable 5 --- docs/HOW_ZERO_WORKS.md | 2 +- internal/skills/skills.go | 16 ++++++++-------- internal/trace/emit.go | 30 +++++------------------------- internal/trace/trace.go | 1 - 4 files changed, 14 insertions(+), 35 deletions(-) diff --git a/docs/HOW_ZERO_WORKS.md b/docs/HOW_ZERO_WORKS.md index b07e52af4..488fb6826 100644 --- a/docs/HOW_ZERO_WORKS.md +++ b/docs/HOW_ZERO_WORKS.md @@ -749,7 +749,7 @@ flowchart TD - **Plugins** can add tools, hooks, and skill roots. Bootstrap always registers a multi-root skill tool: primary Zero skills dir, optional `~/.agents/skills`, then plugin skill roots (earlier wins). `internal/skills` owns that merge via - `LoadFromRoots` / `DiscoveryRoots`; single-root `Load` remains for install/write. + `LoadFromRoots`; single-root `Load` remains for install/write. - **Hooks** can observe or block tool lifecycle events. ## End-to-End Data Flow diff --git a/internal/skills/skills.go b/internal/skills/skills.go index 11d8b0207..5227e91f0 100644 --- a/internal/skills/skills.go +++ b/internal/skills/skills.go @@ -39,7 +39,7 @@ var errNotDirectory = errors.New("not a directory") // NOT created — a missing directory simply yields no skills. // // DefaultDir is the primary write root for install/remove/lock. Runtime discovery -// also considers AgentsDir and plugin skill roots via DiscoveryRoots / LoadFromRoots. +// also considers AgentsDir and plugin skill roots via LoadFromRoots. func DefaultDir(env map[string]string) string { if override := strings.TrimSpace(envValue(env, "ZERO_SKILLS_DIR")); override != "" { return override @@ -135,13 +135,13 @@ type DuplicateName struct { // directory name wins (os.ReadDir returns entries sorted by filename, so the // first one encountered is kept and later same-name duplicates are dropped). // This guarantees Load/List/Get always resolve a duplicated name to the same -// winner regardless of sort stability. Use Duplicates to surface a warning about -// any such collisions. +// winner regardless of sort stability. LoadFromRoots reports the dropped +// collisions so callers can warn about any such shadowing. // -// NOTE: Load scans one root. Runtime discovery uses LoadFromRoots / -// DiscoveryRoots (primary DefaultDir, optional ~/.agents/skills, then plugin -// skill roots). Prefer those multi-root helpers for agent/CLI discovery; keep -// Load for single-dir install/write call sites. +// NOTE: Load scans one root. Runtime discovery uses LoadFromRoots (primary +// DefaultDir, optional ~/.agents/skills, then plugin skill roots). Prefer +// that multi-root helper for agent/CLI discovery; keep Load for single-dir +// install/write call sites. func Load(dir string) ([]Skill, error) { skills, _, err := load(dir) return skills, err @@ -240,7 +240,7 @@ func confineSkillPath(rootReal string, manifestPath string) (string, bool) { return real, true } -// load is the shared scanner behind Load and Duplicates: it parses every +// load is the shared scanner behind Load and LoadFromRoots: it parses every // SKILL.md, deduplicates by frontmatter name (first directory wins) and reports // the dropped collisions. func load(dir string) ([]Skill, []DuplicateName, error) { diff --git a/internal/trace/emit.go b/internal/trace/emit.go index e6270126f..d8f35cb4d 100644 --- a/internal/trace/emit.go +++ b/internal/trace/emit.go @@ -7,14 +7,6 @@ import ( "time" ) -// Sink is the abstraction a finished TurnTrace is written to. The NDJSON and -// text sinks are implemented here; an OpenTelemetry sink is a documented -// future addition (see opentelemetrySink below) and is intentionally not -// pulled in as a dependency. -type Sink interface { - Emit(*TurnTrace) error -} - // WriteNDJSON emits the trace as newline-delimited JSON compatible with the // internal/agenteval trace contract: one object per line carrying a "type" // and (for spans/counters) a "name" so ParseTraceEventKeys keys them. @@ -160,23 +152,11 @@ func WriteNDJSON(w io.Writer, t *TurnTrace) error { return nil } -// NDJSONSink adapts an io.Writer as a Sink emitting NDJSON. -type NDJSONSink struct{ W io.Writer } - -// TextSink adapts an io.Writer as a Sink emitting human-readable text. -type TextSink struct{ W io.Writer } - -// opentelemetrySink is a placeholder documenting the future OpenTelemetry -// export path. It is intentionally not implemented in the baseline: doing so -// would pull in the OTLP exporter dependency. When added, satisfy Sink by -// translating each Span into an OTLP span and each Counter into an attribute, -// parented under the run's trace: -// -// type opentelemetrySink struct{ exp someExporter } -// func (s opentelemetrySink) Emit(t *TurnTrace) error { ... } -// -// It is left as a comment to avoid an unused-type lint while signaling the -// intended extension seam to the next PR. +// An OpenTelemetry export path is a documented future addition. It is +// intentionally not implemented in the baseline: doing so would pull in the +// OTLP exporter dependency. When added, mirror WriteNDJSON by translating +// each Span into an OTLP span and each Counter into an attribute, parented +// under the run's trace. func ms(d time.Duration) float64 { return round3(float64(d.Microseconds()) / 1000) } diff --git a/internal/trace/trace.go b/internal/trace/trace.go index c48a43aad..37b28927a 100644 --- a/internal/trace/trace.go +++ b/internal/trace/trace.go @@ -305,4 +305,3 @@ func OptionalEventKeys() []string { "task_state", } } - From 0b58d52d6cd280755599a5e54d9c65486113d695 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sun, 19 Jul 2026 04:49:40 -0400 Subject: [PATCH 4/6] chore: clean up remaining review nits on dead code removal Replace a pointless fmt.Errorf(...%w...).Error() with fmt.Sprintf using %v, since the wrapped error was discarded immediately anyway. Remove a comment block that was duplicated verbatim. --- internal/agent/loop.go | 4 ---- internal/config/export_test.go | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 8356cd316..8e8fc6947 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -267,10 +267,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) }) } - // Build the per-turn tool list first so proactive compaction can include - // the tool-definition tokens (they ride on every request) in its estimate. - // partitionTools depends only on registry/permissions/options/loaded, not on - // the messages, so computing it before compaction is safe. // Build the per-turn tool list first so proactive compaction can include // the tool-definition tokens (they ride on every request) in its estimate. // partitionTools depends only on registry/permissions/options/loaded, not on diff --git a/internal/config/export_test.go b/internal/config/export_test.go index ac72ce3b8..93da08130 100644 --- a/internal/config/export_test.go +++ b/internal/config/export_test.go @@ -30,7 +30,7 @@ func ValidateFile(path string) (FileConfig, []Issue) { var cfg FileConfig if err := json.Unmarshal(data, &cfg); err != nil { - return FileConfig{}, []Issue{{Message: fmt.Errorf("invalid config JSON %s: %w", path, err).Error()}} + return FileConfig{}, []Issue{{Message: fmt.Sprintf("invalid config JSON %s: %v", path, err)}} } issues := validateSemantics(cfg) From 0e1764e0ce06bb0c17c23f4fa50ef268e3dc9c5b Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Mon, 20 Jul 2026 07:05:34 -0400 Subject: [PATCH 5/6] docs(skills): drop remaining references to test-only List/Get Load and MergedSkills comments still named List/Get as production entry points after they moved to export_test.go. Point callers at Load and ListFromRoots instead. --- internal/plugins/activate.go | 2 +- internal/skills/skills.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/plugins/activate.go b/internal/plugins/activate.go index a684bf1c5..40bf5416f 100644 --- a/internal/plugins/activate.go +++ b/internal/plugins/activate.go @@ -284,7 +284,7 @@ const skillFileName = "SKILL.md" // MergedSkills loads the default skills directory, the shared ~/.agents/skills // root when present, plus the supplied plugin skill roots and returns one -// merged, name-deduplicated list (Content stripped, like skills.List) alongside +// merged, name-deduplicated list (Content stripped, like skills.ListFromRoots) alongside // the duplicate-name collisions across all roots. Earlier roots win a name // clash, matching skills.Load's first-wins rule; the default dir is always // considered first so a user skill shadows agents and plugins. A bad root simply diff --git a/internal/skills/skills.go b/internal/skills/skills.go index 5227e91f0..f5095a458 100644 --- a/internal/skills/skills.go +++ b/internal/skills/skills.go @@ -134,9 +134,9 @@ type DuplicateName struct { // DETERMINISTIC by a documented rule: the skill in the lexicographically-first // directory name wins (os.ReadDir returns entries sorted by filename, so the // first one encountered is kept and later same-name duplicates are dropped). -// This guarantees Load/List/Get always resolve a duplicated name to the same -// winner regardless of sort stability. LoadFromRoots reports the dropped -// collisions so callers can warn about any such shadowing. +// This guarantees Load always resolves a duplicated name to the same winner +// regardless of sort stability. LoadFromRoots reports the dropped collisions +// so callers can warn about any such shadowing. // // NOTE: Load scans one root. Runtime discovery uses LoadFromRoots (primary // DefaultDir, optional ~/.agents/skills, then plugin skill roots). Prefer From 2c92a5549cdeb2878800c128e33c71e4f6f38522 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Tue, 21 Jul 2026 21:33:24 -0400 Subject: [PATCH 6/6] chore: remove now-unreferenced export_test.go seams execSessionManager.len() and likelySandboxDenied/sandboxDenialKind had no remaining callers after the sandbox unify-command-execution rebase; the production code they wrapped for tests no longer exists. --- internal/tools/export_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/internal/tools/export_test.go b/internal/tools/export_test.go index d5efe4897..608cbe56c 100644 --- a/internal/tools/export_test.go +++ b/internal/tools/export_test.go @@ -5,8 +5,6 @@ import ( "net/http" "sort" "strings" - - zeroSandbox "github.com/Gitlawb/zero/internal/sandbox" ) // newWebFetchToolWithClient is a test seam: it builds the live web_fetch tool @@ -130,16 +128,6 @@ func shortenDescription(desc string, max int) string { return truncateDescription(meaningful, max) } -func (manager *execSessionManager) len() int { - manager.mu.Lock() - defer manager.mu.Unlock() - return len(manager.sessions) -} - -func likelySandboxDenied(plan zeroSandbox.CommandPlan, exitCode int, sections ...string) bool { - kind, _ := sandboxDenialKind(plan, exitCode, sections...) - return kind != "" -} func isGenericDescriptionHeading(line string) bool { return genericHeading.MatchString(line) }