Files
kit/internal/core/bash_test.go
T
Ed Zynda e8e99b19a8 refactor: dedupe cross-package logic and remove dead code from audit (#58)
* Remove dead code: 5 unused symbols across internal packages

- internal/models: LoadModelSettingsFromConfig (zero refs)
- internal/prompts: PromptTemplate.ExpandWithArgs (zero refs)
- internal/app: NewMessageStore (tests migrated to NewMessageStoreWithMessages)
- internal/config: HasEnvVars (+ its test)
- internal/core: ContextWithSudoPassword (test migrated to context.WithValue)

* pkg/kit: use TreeManager alias in exported signatures

NewTreeManagerAdapter and InitTreeSession now spell their signatures with
the public kit.TreeManager alias instead of internal/session.TreeManager,
so go doc renders domain types rather than internal paths.

* Consolidate tool-kind classification into internal/extensions

coreToolKinds + toolKindFor were duplicated verbatim in
internal/extensions/wrapper.go and pkg/kit/events.go, risking silent
divergence between extension events and SDK events. Single source of
truth now lives in internal/extensions/toolkinds.go; pkg/kit re-exports
the constants.

* Consolidate Anthropic OAuth detection and usage-tracker refresh

The 'is the active Anthropic credential a stored OAuth token' check was
copy-pasted at 5 sites, all prefix-matching the magic string
'stored OAuth' produced in internal/auth. Now:

- internal/auth: new CredentialSourceOAuth constant + IsAnthropicOAuth()
- internal/ui: new UpdateUsageTrackerForModel(); CreateUsageTracker and
  SetupCLI share lookupTrackableModel (SetupCLI no longer re-inlines the
  tracker construction)
- cmd/root.go + cmd/extension_context.go: verbatim-duplicated tracker
  refresh blocks replaced with ui.UpdateUsageTrackerForModel
- pkg/kit isAnthropicOAuth delegates to auth.IsAnthropicOAuth
- internal/models compares source against the constant

* pkg/kit: consolidate model-path helpers and argument tokenizer

- ExtractModelFromPath mis-parsed model IDs containing '/' (e.g.
  'openrouter/meta/llama' -> 'meta'); it now delegates to
  RemoveProviderFromModel and is deprecated alongside
  ExtractProviderFromPath (-> GetCurrentProvider)
- parseFields delegated to prompts.ParseCommandArgs so extension argument
  parsing and builtin prompt-template parsing share one quote/escape
  grammar; ParseCommandArgs now also splits on tabs (superset of both
  previous tokenizers)

* Unify the two {{variable}} template engines

internal/skills and pkg/kit/template_bridge each had their own grammar:
skills rejected '{{ name }}' (whitespace) but allowed digit-first names;
the bridge was the opposite. A template behaved differently depending on
whether it was loaded as a skill prompt or via the extension API.

internal/skills is now the single engine using the superset grammar
(\{\{\s*(\w+)\s*\}\}); pkg/kit ParseTemplate/RenderTemplate are thin
adapters over it. Expand is now regex-based so whitespace placeholders
expand consistently; missing variables are still left as-is.

* internal/ui: extract switchModel helper for model-switch flow

The model-selector handler (ModelSelectedMsg) and /model slash command
duplicated the full switch sequence (thinking-level fallback, setModel,
display-state update, preference persistence, ModelChange emit) and had
already drifted in ordering. Both now call a single switchModel method.
Display state is still updated directly (no prog.Send from Update).

* extbridge: extract shared BaseContext for extension wiring

cmd/extension_context.go and internal/acpserver/session.go each built a
giant extensions.Context literal, duplicating ~15 delegation closures
(GetContextStats, GetMessages, AppendEntry, options, SetModel core,
Complete, SpawnSubagent, ...) that had to be kept in sync by hand. New
data-access fields had to be wired in both places or ACP-mode extensions
silently got nil function fields.

extbridge.BaseContext now provides the headless half; both call sites
overlay only their UI-specific closures. As a side effect ACP mode gains
previously-missing APIs (state, tree navigation, skills, template
parsing, model resolution) that were nil before. The interactive TUI
keeps its exact SetModel/ReloadExtensions ordering via overrides.

* internal/tools: extract withOAuthRetry and marshalToolResult helpers

ExecuteTool repeated the OAuth-error/re-auth/retry stanza verbatim twice
(sync and task-augmented paths) and the marshal-and-wrap stanza four
times. Both are now single helpers with identical error strings, so a
fix to OAuth retry or error categorization applies everywhere at once.

* internal/ui: extract buildShareFile with defer-based cleanup

handleShareCommand repeated the close/remove/print/return cleanup chain
four times across its temp-file write error paths. File assembly now
lives in buildShareFile with a single deferred cleanup on error.

* cmd: extract flag validation, preference restore, and provider-URL routing from runNormalMode

runNormalMode opened with ~150 lines of policy logic (flag-combination
validation, persisted model/thinking-level preference restoration, and
two subtle --provider-url model-rewrite rules). These are now standalone
functions (validateModeFlags, restorePersistedPreferences,
applyProviderURLRouting) so the routing policy is independently readable
and testable. Behaviour unchanged; ordering preserved.

* fix: address review findings on SDK godoc and nil guard

- pkg/kit: remove internal package paths from exported godoc on
  ParseTemplate and the ToolKind* constants (SDK doc surface must not
  reference internal packages)
- internal/tools: guard marshalToolResult against a nil CallToolResult
  (json.Marshal(nil) succeeds as 'null', then result.IsError panics if
  a client returns nil result with nil error)

Skipped the TreeNode Children deep-copy suggestion: the slice already
comes from TreeManager.GetChildren which returns a fresh copy per call
into a throwaway intermediate, so no internal state is exposed.
2026-06-11 16:13:18 +03:00

199 lines
5.2 KiB
Go

package core
import (
"context"
"encoding/json"
"testing"
"time"
"charm.land/fantasy"
)
// helper to create a bash tool call with the given command and optional timeout.
func bashCall(command string, timeout float64) fantasy.ToolCall {
args := map[string]any{"command": command}
if timeout > 0 {
args["timeout"] = timeout
}
input, _ := json.Marshal(args)
return fantasy.ToolCall{
ID: "test-call",
Name: "bash",
Input: string(input),
}
}
func TestBash_SimpleCommand(t *testing.T) {
resp, err := executeBash(context.Background(), bashCall("echo hello", 0), "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if resp.IsError {
t.Fatalf("expected success, got error: %s", resp.Content)
}
if resp.Content != "hello\n" {
t.Errorf("expected 'hello\\n', got %q", resp.Content)
}
}
func TestBash_TimeoutKillsProcess(t *testing.T) {
start := time.Now()
resp, err := executeBash(context.Background(), bashCall("sleep 60", 2), "")
elapsed := time.Since(start)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !resp.IsError {
t.Fatal("expected error response for timed-out command")
}
if elapsed > 10*time.Second {
t.Errorf("command took %v, expected ~2s timeout", elapsed)
}
}
func TestBash_BackgroundProcessDoesNotHang(t *testing.T) {
// This command spawns a background sleep that would hold pipes open
// forever if we didn't have process group killing + WaitDelay.
start := time.Now()
resp, err := executeBash(context.Background(), bashCall("echo done; sleep 3600 &", 5), "")
elapsed := time.Since(start)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// The foreground command (echo) should complete quickly
if elapsed > 5*time.Second {
t.Errorf("command took %v, should complete in <5s (background process should not block)", elapsed)
}
if resp.IsError {
t.Fatalf("expected success, got error: %s", resp.Content)
}
}
func TestBash_BackgroundProcessDoesNotHang_Streaming(t *testing.T) {
// Same test but in streaming mode (with output callback).
ctx := ContextWithToolOutputCallback(context.Background(), func(_, _, _ string, _ bool) {})
start := time.Now()
resp, err := executeBash(ctx, bashCall("echo streaming; sleep 3600 &", 5), "")
elapsed := time.Since(start)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if elapsed > 5*time.Second {
t.Errorf("streaming command took %v, should complete in <5s", elapsed)
}
if resp.IsError {
t.Fatalf("expected success, got error: %s", resp.Content)
}
}
func TestBash_ContextCancellation(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
done := make(chan struct{})
go func() {
defer close(done)
_, _ = executeBash(ctx, bashCall("sleep 60", 0), "")
}()
// Cancel after a short delay
time.Sleep(500 * time.Millisecond)
cancel()
// Should return promptly after cancellation
select {
case <-done:
// success
case <-time.After(5 * time.Second):
t.Fatal("executeBash did not return after context cancellation")
}
}
func TestBash_BannedCommand(t *testing.T) {
resp, err := executeBash(context.Background(), bashCall("alias foo=bar", 0), "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !resp.IsError {
t.Fatal("expected error for banned command")
}
}
func TestBash_EmptyCommand(t *testing.T) {
resp, err := executeBash(context.Background(), bashCall("", 0), "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !resp.IsError {
t.Fatal("expected error for empty command")
}
}
func TestRewriteSudoForStdin(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
name: "simple sudo",
input: "sudo apt update",
expected: "sudo -S -p '' apt update",
},
{
name: "sudo with env var",
input: "DEBIAN_FRONTEND=noninteractive sudo apt update",
expected: "DEBIAN_FRONTEND=noninteractive sudo -S -p '' apt update",
},
{
name: "sudo in pipeline",
input: "echo test | sudo tee /etc/test.conf",
expected: "echo test | sudo -S -p '' tee /etc/test.conf",
},
{
name: "sudo after &&",
input: "apt update && sudo apt upgrade",
expected: "apt update && sudo -S -p '' apt upgrade",
},
{
name: "already has -S flag",
input: "sudo -S apt update",
expected: "sudo -S apt update",
},
{
name: "no sudo",
input: "apt update && apt upgrade",
expected: "apt update && apt upgrade",
},
{
name: "sudo in string (should not match)",
input: "echo 'use sudo carefully'",
expected: "echo 'use sudo carefully'",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := rewriteSudoForStdin(tt.input)
if result != tt.expected {
t.Errorf("rewriteSudoForStdin(%q) = %q, want %q", tt.input, result, tt.expected)
}
})
}
}
func TestSudoPasswordFromContext(t *testing.T) {
// Test with password in context
ctx := context.WithValue(context.Background(), sudoPasswordKey, "secret123")
pw := sudoPasswordFromContext(ctx)
if pw != "secret123" {
t.Errorf("expected password 'secret123', got %q", pw)
}
// Test without password
ctx = context.Background()
pw = sudoPasswordFromContext(ctx)
if pw != "" {
t.Errorf("expected empty password, got %q", pw)
}
}