diff --git a/pkg/agent/core.go b/pkg/agent/core.go index addeba4..e6505f4 100644 --- a/pkg/agent/core.go +++ b/pkg/agent/core.go @@ -137,14 +137,6 @@ func BuildAgentEnv(cfg *config.Config, d tools.Dispatcher, cwd string) AgentEnv } } -// Dispatcher returns the underlying tool dispatcher. -func (e *AgentEnv) Dispatcher() tools.Dispatcher { return e.dispatcher } - - -// DefaultAgentsDir returns the default directory for agent config files. -func DefaultAgentsDir() string { - return paths.CfgDir() + "/agents" -} // DefaultPromptsDir returns the default directory for prompt templates. func DefaultPromptsDir() string { @@ -181,10 +173,6 @@ func loadSystemPrompt(cwd string) string { return os.Expand(string(data), mapper) } -// DefaultSessionsDir returns the default directory for saved sessions. -func DefaultSessionsDir() string { - return paths.CfgDir() + "/sessions" -} // AgentConfigPath resolves the config file path for a named agent. func AgentConfigPath(agentsDir, name string) string { @@ -230,6 +218,7 @@ type AgentCoreConfig struct { Session *Session Env AgentEnv NewDispatcher func() tools.Dispatcher + NewBackend func() (backend.Backend, error) // if nil, defaults to backend.New } // agentCore is the Core implementation. It owns all agent and session state @@ -244,6 +233,7 @@ type agentCore struct { sessionID string dispatcher tools.Dispatcher newDispatcher func() tools.Dispatcher + newBackend func() (backend.Backend, error) cwd string agentPrompt string // agent-specific prompt suffix; kept for system prompt rebuilds startupMessages []string @@ -310,6 +300,9 @@ func NewAgentCore(cfg AgentCoreConfig) Core { if cfg.ModelName != "" { cfg.Backend.SetModel(cfg.ModelName) } + if cfg.NewBackend == nil { + cfg.NewBackend = backend.New + } loopcfg := loopConfig{ Backend: cfg.Backend, systemPrompt: cfg.Env.systemPrompt, @@ -337,6 +330,7 @@ func NewAgentCore(cfg AgentCoreConfig) Core { startupMessages: cfg.Env.Messages, dispatcher: cfg.Env.dispatcher, newDispatcher: cfg.NewDispatcher, + newBackend: cfg.NewBackend, state: "idle", } a.changeCond = sync.NewCond(&a.changeMu) @@ -352,13 +346,6 @@ func (s *agentCore) Close() { } } -func (s *agentCore) prompt() string { - return fmt.Sprintf("[%s :: %s] ", s.loopcfg.Backend.Name(), s.agentName) -} - -// Prompt returns the display prompt string for the current session state. -func (s *agentCore) Prompt() string { return s.prompt() } - func (s *agentCore) AgentName() string { v := s.agentName clog.Debug("AgentName() = %q", v) @@ -967,7 +954,7 @@ func (s *agentCore) handleCommand(ctx context.Context, input string, handler Eve return } os.Setenv("OLLIE_BACKEND", args[0]) //nolint:errcheck - be, err := backend.New() + be, err := s.newBackend() if err != nil { handler(infoEvent(fmt.Sprintf("error: failed to switch backend: %v", err))) return @@ -1301,11 +1288,6 @@ func mcpToolsToBackend(mcpTools []tools.ToolInfo) []backend.Tool { return out } -func extractMCPText(raw json.RawMessage) string { - text, _ := extractMCPResult(raw) - return text -} - func extractMCPResult(raw json.RawMessage) (text string, isError bool) { var result struct { IsError bool `json:"isError"` diff --git a/pkg/agent/core_test.go b/pkg/agent/core_test.go index b8c40bd..42a700a 100644 --- a/pkg/agent/core_test.go +++ b/pkg/agent/core_test.go @@ -12,6 +12,7 @@ import ( "time" "ollie/pkg/backend" + "ollie/pkg/config" "ollie/pkg/tools" ) @@ -1875,3 +1876,407 @@ func TestManualCompact_WithToolMessages(t *testing.T) { t.Errorf("/compact with tool messages: no 'compacted' event; got: %v", byRole(evs, "info")) } } + +// --- hook timeout --- + +func TestHookTimeout_Branch(t *testing.T) { + old := hookTimeout + hookTimeout = 0 // 0s timeout fires immediately + t.Cleanup(func() { hookTimeout = old }) + + // A hook that sleeps longer than the timeout. The timeout branch kills the + // process and returns HookResult{} (Ran=false), so the turn is NOT blocked + // and the backend runs normally. + hooks := Hooks{HookPreTurn: []string{"sleep 10"}} + c := newCore(t, nil, hooks) + evs := collectEvents(context.Background(), c, "hello") + + // Turn must have run: an assistant event proves the hook didn't block it. + if got := byRole(evs, "assistant"); len(got) == 0 { + t.Errorf("expected assistant event after hook timeout; hook must not have blocked the turn") + } + if c.State() != "idle" { + t.Errorf("State() = %q after timeout hook; want idle", c.State()) + } +} + +// --- /backend with injected newBackend --- + +func TestCommand_Backend_Switch(t *testing.T) { + c := newCore(t, nil, nil) + newBE := &mockBackend{name: "injected", model: "new-model"} + c.newBackend = func() (backend.Backend, error) { return newBE, nil } + + evs := collectEvents(context.Background(), c, "/backend other") + infos := byRole(evs, "info") + found := false + for _, s := range infos { + if strings.Contains(s, "injected") { + found = true + } + } + if !found { + t.Errorf("/backend switch: expected 'injected' in info events; got %v", infos) + } + if c.loopcfg.Backend != newBE { + t.Errorf("/backend switch: backend not updated") + } +} + +func TestCommand_Backend_Error(t *testing.T) { + c := newCore(t, nil, nil) + c.newBackend = func() (backend.Backend, error) { return nil, fmt.Errorf("no such backend") } + + evs := collectEvents(context.Background(), c, "/backend bad") + infos := byRole(evs, "info") + found := false + for _, s := range infos { + if strings.Contains(s, "no such backend") { + found = true + } + } + if !found { + t.Errorf("/backend error: expected error message; got %v", infos) + } +} + +// --- extractMCPResult --- + +func TestExtractMCPResult_Success(t *testing.T) { + raw := json.RawMessage(`{"isError":false,"content":[{"type":"text","text":"hello"}]}`) + text, isErr := extractMCPResult(raw) + if text != "hello" { + t.Errorf("text = %q; want %q", text, "hello") + } + if isErr { + t.Error("isError should be false") + } +} + +func TestExtractMCPResult_IsError(t *testing.T) { + raw := json.RawMessage(`{"isError":true,"content":[{"type":"text","text":"something failed"}]}`) + text, isErr := extractMCPResult(raw) + if text != "something failed" { + t.Errorf("text = %q; want %q", text, "something failed") + } + if !isErr { + t.Error("isError should be true") + } +} + +func TestExtractMCPResult_MultipleContentItems(t *testing.T) { + raw := json.RawMessage(`{"isError":false,"content":[{"type":"text","text":"a"},{"type":"text","text":"b"}]}`) + text, _ := extractMCPResult(raw) + if text != "a\nb" { + t.Errorf("text = %q; want %q", text, "a\nb") + } +} + +func TestExtractMCPResult_NonTextItemsSkipped(t *testing.T) { + raw := json.RawMessage(`{"isError":false,"content":[{"type":"image","text":"ignored"},{"type":"text","text":"kept"}]}`) + text, _ := extractMCPResult(raw) + if text != "kept" { + t.Errorf("text = %q; want %q", text, "kept") + } +} + +func TestExtractMCPResult_InvalidJSON(t *testing.T) { + raw := json.RawMessage(`not json`) + text, isErr := extractMCPResult(raw) + if text != "not json" { + t.Errorf("text = %q; want raw input on parse failure", text) + } + if isErr { + t.Error("isError should be false on parse failure") + } +} + +// --- mcpToolsToBackend --- + +func TestMcpToolsToBackend(t *testing.T) { + schema := json.RawMessage(`{"type":"object"}`) + infos := []tools.ToolInfo{ + {Name: "tool_a", Description: "does A.", InputSchema: schema}, + {Name: "tool_b", Description: "does B.", InputSchema: schema}, + } + got := mcpToolsToBackend(infos) + if len(got) != 2 { + t.Fatalf("len = %d; want 2", len(got)) + } + if got[0].Name != "tool_a" || got[0].Description != "does A." { + t.Errorf("got[0] = %+v", got[0]) + } + if string(got[1].Parameters) != string(schema) { + t.Errorf("got[1].Parameters = %s; want %s", got[1].Parameters, schema) + } +} + +func TestMcpToolsToBackend_Empty(t *testing.T) { + got := mcpToolsToBackend(nil) + if len(got) != 0 { + t.Errorf("expected empty slice, got %v", got) + } +} + +// --- RestoreSession round-trip --- + +func TestRestoreSession_RoundTrip(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "sess.json") + + s := newSession("first user message") + s.appendUserMessage("second message") + s.messages = append(s.messages, backend.Message{Role: "assistant", Content: "reply"}) + + if err := s.saveTo(path, "test-id", "test-agent"); err != nil { + t.Fatalf("saveTo: %v", err) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + var ps PersistedSession + if err := json.Unmarshal(data, &ps); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if ps.ID != "test-id" || ps.Agent != "test-agent" { + t.Errorf("ps.ID=%q ps.Agent=%q", ps.ID, ps.Agent) + } + + restored := RestoreSession(ps.Messages) + if restored.goal != "first user message" { + t.Errorf("goal = %q; want %q", restored.goal, "first user message") + } + if len(restored.messages) != len(s.messages) { + t.Errorf("messages len = %d; want %d", len(restored.messages), len(s.messages)) + } +} + +func TestRestoreSession_GoalFromFirstUserMessage(t *testing.T) { + msgs := []backend.Message{ + {Role: "assistant", Content: "preamble"}, + {Role: "user", Content: "the real goal"}, + {Role: "user", Content: "second user msg"}, + } + s := RestoreSession(msgs) + if s.goal != "the real goal" { + t.Errorf("goal = %q; want %q", s.goal, "the real goal") + } +} + +// --- SetEnv propagation --- + +// mockEnvServer is a tools.Server that also implements tools.EnvSetter. +type mockEnvServer struct { + mu sync.Mutex + env map[string]string +} + +func (m *mockEnvServer) ListTools() ([]tools.ToolInfo, error) { return nil, nil } +func (m *mockEnvServer) CallTool(_ context.Context, _ string, _ json.RawMessage) (json.RawMessage, error) { + return nil, fmt.Errorf("not implemented") +} +func (m *mockEnvServer) Close() {} +func (m *mockEnvServer) SetEnv(k, v string) { + m.mu.Lock() + defer m.mu.Unlock() + if m.env == nil { + m.env = make(map[string]string) + } + m.env[k] = v +} +func (m *mockEnvServer) get(k string) string { + m.mu.Lock() + defer m.mu.Unlock() + return m.env[k] +} + +func newCoreWithExecServer(t *testing.T, srv *mockEnvServer) *agentCore { + t.Helper() + d := tools.NewDispatcher() + d.AddServer("execute", srv) + env := AgentEnv{ + Hooks: Hooks{}, + systemPrompt: "test system prompt", + dispatcher: d, + } + c := NewAgentCore(AgentCoreConfig{ + Backend: defaultBE(), + AgentName: "test", + AgentsDir: t.TempDir(), + SessionsDir: t.TempDir(), + SessionID: NewSessionID(), + CWD: t.TempDir(), + Env: env, + NewDispatcher: tools.NewDispatcher, + }) + t.Cleanup(c.Close) + return c.(*agentCore) +} + +func TestSetEnv_PropagatestoExecuteServer(t *testing.T) { + srv := &mockEnvServer{} + c := newCoreWithExecServer(t, srv) + + c.SetEnv("MY_KEY", "my_value") + + if got := srv.get("MY_KEY"); got != "my_value" { + t.Errorf("execute server env MY_KEY = %q; want %q", got, "my_value") + } +} + +func TestSetEnv_StoredInCore(t *testing.T) { + srv := &mockEnvServer{} + c := newCoreWithExecServer(t, srv) + + c.SetEnv("FOO", "bar") + c.SetEnv("BAZ", "qux") + + c.envMu.RLock() + defer c.envMu.RUnlock() + if c.env["FOO"] != "bar" { + t.Errorf("env[FOO] = %q; want bar", c.env["FOO"]) + } + if c.env["BAZ"] != "qux" { + t.Errorf("env[BAZ] = %q; want qux", c.env["BAZ"]) + } +} + +func TestSetEnv_ShellCommandSeesEnv(t *testing.T) { + srv := &mockEnvServer{} + c := newCoreWithExecServer(t, srv) + + c.SetEnv("OLLIE_TEST_VAR", "sentinel_value") + + evs := collectEvents(context.Background(), c, "!echo $OLLIE_TEST_VAR") + infos := byRole(evs, "info") + found := false + for _, s := range infos { + if strings.Contains(s, "sentinel_value") { + found = true + } + } + if !found { + t.Errorf("shell command did not see OLLIE_TEST_VAR; info events: %v", infos) + } +} + +func TestSetEnv_NilDispatcher_NoPanic(t *testing.T) { + c := newCore(t, nil, nil) + c.dispatcher = nil + // Must not panic. + c.SetEnv("K", "V") + if c.env["K"] != "V" { + t.Errorf("env[K] = %q; want V", c.env["K"]) + } +} + +// --- firstSentence --- + +func TestFirstSentence_Period(t *testing.T) { + if got := firstSentence("Does a thing. More detail."); got != "Does a thing." { + t.Errorf("got %q", got) + } +} + +func TestFirstSentence_Newline(t *testing.T) { + if got := firstSentence("Does a thing\nMore detail"); got != "Does a thing" { + t.Errorf("got %q", got) + } +} + +func TestFirstSentence_TruncatesLong(t *testing.T) { + long := strings.Repeat("x", 100) + got := firstSentence(long) + if len(got) != 80 || !strings.HasSuffix(got, "...") { + t.Errorf("got %q (len %d)", got, len(got)) + } +} + +func TestFirstSentence_ShortNoSentenceEnd(t *testing.T) { + if got := firstSentence("short"); got != "short" { + t.Errorf("got %q", got) + } +} + +// --- BuildAgentEnv (nil config path) --- + +func setupCfgDir(t *testing.T, systemPrompt string) string { + t.Helper() + dir := t.TempDir() + if err := os.MkdirAll(dir+"/prompts", 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(dir+"/prompts/SYSTEM_PROMPT.md", []byte(systemPrompt), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("OLLIE_CFG_PATH", dir) + return dir +} + +func TestBuildAgentEnv_NilConfig(t *testing.T) { + setupCfgDir(t, "base prompt") + d := tools.NewDispatcher() + env := BuildAgentEnv(nil, d, t.TempDir()) + + if env.systemPrompt != "base prompt" { + t.Errorf("systemPrompt = %q; want %q", env.systemPrompt, "base prompt") + } + if len(env.Hooks) != 0 { + t.Errorf("expected no hooks; got %v", env.Hooks) + } + if len(env.tools) != 0 { + t.Errorf("expected no tools; got %v", env.tools) + } + if len(env.Messages) != 0 { + t.Errorf("expected no startup messages; got %v", env.Messages) + } +} + +func TestBuildAgentEnv_AgentPromptAppended(t *testing.T) { + setupCfgDir(t, "base prompt") + d := tools.NewDispatcher() + cfg := &config.Config{Prompt: "agent suffix"} + env := BuildAgentEnv(cfg, d, t.TempDir()) + + if !strings.Contains(env.systemPrompt, "base prompt") { + t.Errorf("systemPrompt missing base: %q", env.systemPrompt) + } + if !strings.Contains(env.systemPrompt, "agent suffix") { + t.Errorf("systemPrompt missing agent suffix: %q", env.systemPrompt) + } +} + +func TestBuildAgentEnv_HooksAndParams(t *testing.T) { + setupCfgDir(t, "base") + d := tools.NewDispatcher() + temp := 0.7 + cfg := &config.Config{ + Hooks: map[string]config.HookCmds{"preTurn": {"echo hi"}}, + MaxTokens: 512, + Temperature: &temp, + } + env := BuildAgentEnv(cfg, d, t.TempDir()) + + if cmds := env.Hooks[HookPreTurn]; len(cmds) != 1 || cmds[0] != "echo hi" { + t.Errorf("Hooks[preTurn] = %v", cmds) + } + if env.genParams.MaxTokens != 512 { + t.Errorf("MaxTokens = %d; want 512", env.genParams.MaxTokens) + } + if env.genParams.Temperature == nil || *env.genParams.Temperature != 0.7 { + t.Errorf("Temperature = %v; want 0.7", env.genParams.Temperature) + } +} + +func TestBuildAgentEnv_ExecUnknownTool(t *testing.T) { + setupCfgDir(t, "base") + d := tools.NewDispatcher() + env := BuildAgentEnv(nil, d, t.TempDir()) + + _, err := env.exec(context.Background(), "no_such_tool", json.RawMessage(`{}`)) + if err == nil || !strings.Contains(err.Error(), "unknown tool") { + t.Errorf("expected unknown tool error; got %v", err) + } +} diff --git a/pkg/agent/hooks.go b/pkg/agent/hooks.go index 137c9da..954fd81 100644 --- a/pkg/agent/hooks.go +++ b/pkg/agent/hooks.go @@ -22,6 +22,9 @@ const ( const defaultHookTimeout = 60 +// hookTimeout is the hook execution timeout in seconds. Overridable in tests. +var hookTimeout = defaultHookTimeout + // Hooks maps hook names to one or more shell commands. type Hooks map[string][]string @@ -90,7 +93,7 @@ func runHookCmd(ctx context.Context, name, cmdStr string, payloadJSON []byte, cw } go func() { done <- cmd.Wait() }() - timeout := time.After(time.Duration(defaultHookTimeout) * time.Second) + timeout := time.After(time.Duration(hookTimeout) * time.Second) select { case err := <-done: exitCode := 0 @@ -124,7 +127,7 @@ func runHookCmd(ctx context.Context, name, cmdStr string, payloadJSON []byte, cw case <-timeout: cmd.Process.Kill() //nolint:errcheck <-done - clog.Debug("hook %s: timed out after %ds", name, defaultHookTimeout) + clog.Debug("hook %s: timed out after %ds", name, hookTimeout) return HookResult{} } } diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 7b5389a..d77be23 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -218,9 +218,6 @@ func run(ctx context.Context, cfg loopConfig, state state) error { } if len(toolCalls) == 0 { - if err := state.markComplete(); err != nil { - return fmt.Errorf("mark complete: %w", err) - } break } diff --git a/pkg/agent/session.go b/pkg/agent/session.go index 6a64566..21f87fe 100644 --- a/pkg/agent/session.go +++ b/pkg/agent/session.go @@ -68,7 +68,6 @@ func RestoreSession(messages []backend.Message) *Session { type Session struct { goal string messages []backend.Message - complete bool // Cumulative usage tracking. TotalInputTokens int TotalOutputTokens int @@ -83,14 +82,10 @@ func newSession(goal string) *Session { return s } -func (s *Session) Goal() string { return s.goal } - func (s *Session) history() []backend.Message { return s.messages } -func (s *Session) isComplete() bool { return s.complete } - func (s *Session) addUsage(u backend.Usage, estimated bool) { s.TotalInputTokens += u.InputTokens s.TotalOutputTokens += u.OutputTokens @@ -112,11 +107,6 @@ func (s *Session) update(assistant backend.Message, results []toolResult) error return nil } -func (s *Session) markComplete() error { - s.complete = true - return nil -} - // PreCompactionSnapshot returns a copy of the current messages for persistence // before compaction. Call this before compact(). func (s *Session) PreCompactionSnapshot() []backend.Message { @@ -240,15 +230,9 @@ func flattenToolMessages(messages []backend.Message) []backend.Message { } func (s *Session) appendUserMessage(content string) { - s.complete = false s.messages = append(s.messages, backend.Message{Role: "user", Content: content}) } -// contextStatsString returns a one-line human-readable context summary. -func (s *Session) contextStatsString() string { - return fmt.Sprintf("context: ~%d tokens (%d msgs)", s.estimateTokens(), len(s.messages)) -} - // estimateTokens returns a rough token count (~4 chars per token). func (s *Session) estimateTokens() int { chars := 0 diff --git a/pkg/agent/signals.go b/pkg/agent/signals.go deleted file mode 100644 index d80a7be..0000000 --- a/pkg/agent/signals.go +++ /dev/null @@ -1,41 +0,0 @@ -package agent - -import ( - "context" - "fmt" - "io" - "os" - "os/signal" - "time" -) - -const CtrlCExitWindow = 750 * time.Millisecond - -// WatchSignals installs OS signal handlers for the lifetime of the program. -// SIGTERM cancels the app context; SIGINT interrupts the current agent turn. -// It is safe to call from any frontend (TUI, HTTP, CLI one-shot, etc.). -func WatchSignals(appCancel context.CancelCauseFunc, c Core, errStream io.Writer) { - ch := make(chan os.Signal, 16) - signals := []os.Signal{os.Interrupt} - if haveSIGTERM { - signals = append(signals, sigTERM) - } - signal.Notify(ch, signals...) - - go func() { - for sig := range ch { - switch sig { - case sigTERM: - signal.Stop(ch) - close(ch) - fmt.Fprint(errStream, "\n(terminated)\n") - appCancel(context.Canceled) - continue - case os.Interrupt: - if c.Interrupt(ErrInterrupted) { - fmt.Fprint(errStream, "\n^C\n") - } - } - } - }() -} diff --git a/pkg/agent/signals_unix.go b/pkg/agent/signals_unix.go deleted file mode 100644 index 80c5397..0000000 --- a/pkg/agent/signals_unix.go +++ /dev/null @@ -1,10 +0,0 @@ -//go:build !windows - -package agent - -import "syscall" - -const ( - haveSIGTERM = true - sigTERM = syscall.SIGTERM -) diff --git a/pkg/agent/signals_windows.go b/pkg/agent/signals_windows.go deleted file mode 100644 index 6586b8e..0000000 --- a/pkg/agent/signals_windows.go +++ /dev/null @@ -1,9 +0,0 @@ -//go:build windows - -package agent - -import "os" - -const haveSIGTERM = false - -var sigTERM = os.Signal(nil) diff --git a/pkg/agent/state.go b/pkg/agent/state.go index 85f2601..3a7d8ea 100644 --- a/pkg/agent/state.go +++ b/pkg/agent/state.go @@ -21,9 +21,4 @@ type state interface { // Called once per loop iteration after Act completes. update(assistant backend.Message, results []toolResult) error - // MarkComplete records that the goal has been achieved. - markComplete() error - - // IsComplete returns true once MarkComplete has been called. - isComplete() bool } diff --git a/pkg/agent/types.go b/pkg/agent/types.go index 209c942..b21c66f 100644 --- a/pkg/agent/types.go +++ b/pkg/agent/types.go @@ -39,9 +39,6 @@ type Core interface { // After the turn, any queued prompts are drained sequentially. Submit(ctx context.Context, input string, handler EventHandler) - // Prompt returns the display prompt string for the current session state. - Prompt() string - // Interrupt cancels the current in-progress agent turn. // Returns true if an action was running and was cancelled. Interrupt(cause error) bool