lkn
/
ollie-core
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

agent: testability, dead code removal, coverage to 83% 

- Hook timeout configurable via hookTimeout var (unlocks <-timeout branch)
- newBackend injection into agentCore (unlocks /backend switch in tests)
- Remove dead code: WatchSignals, Prompt(), Goal(), isComplete/markComplete,
  contextStatsString, extractMCPText, DefaultAgentsDir, DefaultSessionsDir,
  Dispatcher() on AgentEnv
- Signal handling moved to process entry points; core has no OS signal knowledge
- Tests: extractMCPResult, mcpToolsToBackend, RestoreSession, SetEnv,
  BuildAgentEnv, firstSentence, hook timeout, backend injection

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Levi Neely 2026-04-21 12:28:29 +02:00
parent 248b45e688
commit c6f6e7feeb
10 changed files with 417 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
pkg/agent/core.go
View File

@ -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"`

405
pkg/agent/core_test.go
View File

@ -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)
}
}

7
pkg/agent/hooks.go
View File

@ -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{}
}
}

3
pkg/agent/loop.go
View File

@ -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
}

16
pkg/agent/session.go
View File

@ -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

41
pkg/agent/signals.go
View File

@ -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")
}
}
}
}()
}

10
pkg/agent/signals_unix.go
View File

@ -1,10 +0,0 @@
//go:build !windows
package agent
import "syscall"
const (
haveSIGTERM = true
sigTERM = syscall.SIGTERM
)

9
pkg/agent/signals_windows.go
View File

@ -1,9 +0,0 @@
//go:build windows
package agent
import "os"
const haveSIGTERM = false
var sigTERM = os.Signal(nil)

5
pkg/agent/state.go
View File

@ -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
}

3
pkg/agent/types.go
View File

@ -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