From 06a996e96783c2032c4952eb842b221e44276ade Mon Sep 17 00:00:00 2001 From: Ollie Agent Date: Sat, 8 Aug 2026 15:10:01 +0200 Subject: [PATCH] all: replace toolsrv.Runner interface with *toolsrv.Conn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent always talks to a *Conn (never a *Server directly). Replace the Runner interface with the concrete type throughout: - Runtime.ToolServer is now *toolsrv.Conn - BuildRuntime takes *toolsrv.Conn - session.Config, SessionInfra, InfraConfig use *toolsrv.Conn - All inline type assertions in agent.go deleted — methods called directly on *Conn (SetEnv, SetCWD, Close, Detach, ListDetachedRaw, SignalDetached, GetDetachedOutput, DismissDetached, SetAllowTools, SetOnToolsChanged, Ping, ToolRegistryRevision) - Added Conn.ToolRegistryRevision() that returns 0 (remote connections don't support revision tracking; use push-based OnToolsChanged instead) - Deleted the Runner interface and its test - Renamed LoadToolOnRunner → LoadToolOnConn --- agent/agent.go | 108 ++++++++++++----------------------------- agent/build_runtime.go | 8 ++- agent/new.go | 2 +- agent/runtime.go | 2 +- fs/session_node.go | 2 +- session/persist.go | 4 +- session/session.go | 33 ++++++------- session/setup.go | 42 ++++++++-------- toolsrv/conn.go | 4 ++ toolsrv/tools.go | 8 --- toolsrv/tools_test.go | 53 -------------------- 11 files changed, 78 insertions(+), 188 deletions(-) delete mode 100644 toolsrv/tools_test.go diff --git a/agent/agent.go b/agent/agent.go index ad48885..40f705c 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -28,7 +28,7 @@ type Agent struct { systemPrompt string // system prompt for /agent reloads envBlock string // environment block for /agent reloads promptEnvExtra []string // PRIME_* vars for prompt resolution - newToolServer func() toolsrv.Runner + newToolServer func() *toolsrv.Conn newBackend func(string) (backend.Backend, error) currentAction atomic.Pointer[actionHandle] warnedContext bool @@ -239,7 +239,7 @@ func (ag *Agent) SetOutput(h EventHandler) { // SetToolServer updates the tool server connection and factory. // Used when resuming a paused session that was restored without infra. -func (ag *Agent) SetToolServer(newToolServer func() toolsrv.Runner, conn toolsrv.Runner) { +func (ag *Agent) SetToolServer(newToolServer func() *toolsrv.Conn, conn *toolsrv.Conn) { ag.newToolServer = newToolServer if ag.runtime != nil { ag.runtime.ToolServer = conn @@ -316,16 +316,12 @@ func (ag *Agent) SetSessionEnv(sessionID string) { if ag.runtime.ToolServer == nil { return } - if srv := ag.runtime.ToolServer; srv != nil { - if es, ok := srv.(interface{ SetEnv(string, string) }); ok { - es.SetEnv("OLLIE_SESSION_ID", sessionID) - if ag.id != "" { - es.SetEnv("OLLIE_UNAME", ag.id) - } - if u := os.Getenv("USER"); u != "" { - es.SetEnv("USER", u) - } - } + ag.runtime.ToolServer.SetEnv("OLLIE_SESSION_ID", sessionID) + if ag.id != "" { + ag.runtime.ToolServer.SetEnv("OLLIE_UNAME", ag.id) + } + if u := os.Getenv("USER"); u != "" { + ag.runtime.ToolServer.SetEnv("USER", u) } } @@ -334,11 +330,7 @@ func (ag *Agent) SetEnv(key, value string) { if ag.runtime.ToolServer == nil { return } - if srv := ag.runtime.ToolServer; srv != nil { - if es, ok := srv.(interface{ SetEnv(string, string) }); ok { - es.SetEnv(key, value) - } - } + ag.runtime.ToolServer.SetEnv(key, value) } // Close releases agent resources (dispatcher, execute server). @@ -346,11 +338,7 @@ func (ag *Agent) Close() { if ag.runtime.ToolServer == nil { return } - if srv := ag.runtime.ToolServer; srv != nil { - if c, ok := srv.(interface{ Close() }); ok { - c.Close() - } - } + ag.runtime.ToolServer.Close() } // SetCWD updates the agent's working directory, preamble references, and dispatcher. @@ -363,11 +351,7 @@ func (ag *Agent) SetCWD(dir string) { ag.runtime.Preamble = strings.ReplaceAll(ag.runtime.Preamble, oldCwd, dir) } if ag.runtime.ToolServer != nil { - if srv := ag.runtime.ToolServer; srv != nil { - if ws, ok := srv.(interface{ SetCWD(string) }); ok { - ws.SetCWD(dir) - } - } + ag.runtime.ToolServer.SetCWD(dir) } ag.notifyChange() } @@ -466,27 +450,18 @@ func (ag *Agent) wireToolsChanged() { if ag.runtime == nil || ag.runtime.ToolServer == nil { return } - type hasHook interface { - SetOnToolsChanged(func(string)) - } - if srv, ok := ag.runtime.ToolServer.(hasHook); ok { - srv.SetOnToolsChanged(ag.refreshToolListing) - } + ag.runtime.ToolServer.SetOnToolsChanged(ag.refreshToolListing) } // toolsNeedRefresh reports whether the tool registry has changed since the // last time Tools was populated. If the tool server doesn't support revision -// tracking (e.g. remote Conn), this always returns true (safe fallback). +// tracking (e.g. remote Conn returns 0), this always returns true (safe fallback). func (ag *Agent) toolsNeedRefresh() bool { - type revisionProvider interface { - ToolRegistryRevision() uint64 + if ag.runtime.ToolServer == nil { + return true } - srv, ok := ag.runtime.ToolServer.(revisionProvider) - if !ok { - return true // no revision support — always refetch - } - rev := srv.ToolRegistryRevision() - if rev != ag.runtime.toolRegistryRevision { + rev := ag.runtime.ToolServer.ToolRegistryRevision() + if rev == 0 || rev != ag.runtime.toolRegistryRevision { ag.runtime.toolRegistryRevision = rev return true } @@ -577,7 +552,7 @@ func (ag *Agent) ListModels() []string { // ToolServer returns the tool execution server, or nil if unavailable. // Exported for use by the 9P filesystem layer to sync tool registries. -func (ag *Agent) ToolServer() toolsrv.Runner { +func (ag *Agent) ToolServer() *toolsrv.Conn { return ag.runtime.ToolServer } @@ -600,12 +575,10 @@ func (ag *Agent) BroadcastChange() { // Detach detaches the current running process to background. func (ag *Agent) Detach() bool { - if srv := ag.runtime.ToolServer; srv != nil { - if d, ok := srv.(interface{ Detach() bool }); ok { - return d.Detach() - } + if ag.runtime.ToolServer == nil { + return false } - return false + return ag.runtime.ToolServer.Detach() } // DetachedInfo describes a detached process. @@ -619,16 +592,10 @@ type DetachedInfo struct { // ListDetached returns info about all detached processes. func (ag *Agent) ListDetached() []DetachedInfo { - srv := ag.runtime.ToolServer - if srv == nil { + if ag.runtime.ToolServer == nil { return nil } - type listDetacher interface{ ListDetachedRaw() []any } - ld, ok := srv.(listDetacher) - if !ok { - return nil - } - raw := ld.ListDetachedRaw() + raw := ag.runtime.ToolServer.ListDetachedRaw() out := make([]DetachedInfo, 0, len(raw)) for _, r := range raw { if m, ok := r.(map[string]any); ok { @@ -656,35 +623,24 @@ func (ag *Agent) ListDetached() []DetachedInfo { // SignalDetached sends a signal to a detached process. func (ag *Agent) SignalDetached(pid int, signal int) error { - if srv := ag.runtime.ToolServer; srv != nil { - type signaler interface { - SignalDetached(int, syscall.Signal) error - } - if sg, ok := srv.(signaler); ok { - return sg.SignalDetached(pid, syscall.Signal(signal)) - } + if ag.runtime.ToolServer == nil { + return fmt.Errorf("no execute server available") } - return fmt.Errorf("no execute server available") + return ag.runtime.ToolServer.SignalDetached(pid, syscall.Signal(signal)) } // GetDetachedOutput reads output from a detached process. func (ag *Agent) GetDetachedOutput(pid int) (string, error) { - if srv := ag.runtime.ToolServer; srv != nil { - type outputGetter interface{ GetDetachedOutput(int) (string, error) } - if og, ok := srv.(outputGetter); ok { - return og.GetDetachedOutput(pid) - } + if ag.runtime.ToolServer == nil { + return "", fmt.Errorf("no execute server available") } - return "", fmt.Errorf("no execute server available") + return ag.runtime.ToolServer.GetDetachedOutput(pid) } // DismissDetached removes a finished detached process. func (ag *Agent) DismissDetached(pid int) bool { - if srv := ag.runtime.ToolServer; srv != nil { - type dismisser interface{ DismissDetached(int) bool } - if d, ok := srv.(dismisser); ok { - return d.DismissDetached(pid) - } + if ag.runtime.ToolServer == nil { + return false } - return false + return ag.runtime.ToolServer.DismissDetached(pid) } diff --git a/agent/build_runtime.go b/agent/build_runtime.go index a8f5572..0dfc9d5 100644 --- a/agent/build_runtime.go +++ b/agent/build_runtime.go @@ -18,7 +18,7 @@ import ( // env provides additional environment variables injected into prompt resolution // subprocesses (e.g. OLLIE_SESSION_ID=xxx). // The caller is responsible for registering all servers on d before calling this. -func BuildRuntime(cfg *AgentConfig, srv toolsrv.Runner, cwd string, env []string, systemPrompt, envBlock string) *Runtime { +func BuildRuntime(cfg *AgentConfig, srv *toolsrv.Conn, cwd string, env []string, systemPrompt, envBlock string) *Runtime { var messages []string var allToolInfos []toolsrv.ToolInfo @@ -45,10 +45,8 @@ func BuildRuntime(cfg *AgentConfig, srv toolsrv.Runner, cwd string, env []string } genParams = cfg.GenParams() maxSteps = cfg.MaxSteps - if len(cfg.AllowTools) > 0 { - if rs, ok := srv.(interface{ SetAllowTools([]string) }); ok { - rs.SetAllowTools(cfg.AllowTools) - } + if len(cfg.AllowTools) > 0 && srv != nil { + srv.SetAllowTools(cfg.AllowTools) } } diff --git a/agent/new.go b/agent/new.go index 34e96cd..854f6c4 100644 --- a/agent/new.go +++ b/agent/new.go @@ -20,7 +20,7 @@ type AgentCfg struct { SystemPrompt string EnvBlock string PromptEnvExtra []string - NewToolServer func() toolsrv.Runner + NewToolServer func() *toolsrv.Conn NewBackend func(string) (backend.Backend, error) Output EventHandler Log *olog.Logger diff --git a/agent/runtime.go b/agent/runtime.go index 11d1dea..32937cf 100644 --- a/agent/runtime.go +++ b/agent/runtime.go @@ -11,7 +11,7 @@ import ( // agents replaces it atomically. type Runtime struct { Backend backend.Backend - ToolServer toolsrv.Runner // the execute server (tool runtime, env, cwd) + ToolServer *toolsrv.Conn // the tool server connection (tool runtime, env, cwd) Preamble string // compiled system prompt Tools []backend.Tool ToolMeta map[string]toolsrv.ToolInfo // keyed by tool name — source of truth for all per-tool metadata diff --git a/fs/session_node.go b/fs/session_node.go index 809cc4a..a9016ef 100644 --- a/fs/session_node.go +++ b/fs/session_node.go @@ -65,7 +65,7 @@ func (s *SessionNode) Ctx() context.Context { return s.Core.Ctx() } -func (s *SessionNode) ToolsConn() toolsrv.Runner { +func (s *SessionNode) ToolsConn() *toolsrv.Conn { return s.Core.ToolsConn() } diff --git a/session/persist.go b/session/persist.go index e5d44bb..be5599c 100644 --- a/session/persist.go +++ b/session/persist.go @@ -277,8 +277,8 @@ func restoreMultiAgentSession(ps *PersistedSession) (*RestoredSession, error) { env := []string{"OLLIE_SESSION_ID=" + ps.ID, "OLLIE_UNAME=" + pa.ID} env = append(env, promptEnv...) - var toolsConn toolsrv.Runner - var newToolServer func() toolsrv.Runner + var toolsConn *toolsrv.Conn + var newToolServer func() *toolsrv.Conn if infra != nil { toolsConn = infra.ToolsConn newToolServer = infra.NewToolServer diff --git a/session/session.go b/session/session.go index fdb3a40..f00c11a 100644 --- a/session/session.go +++ b/session/session.go @@ -31,7 +31,7 @@ type Config struct { CWD string History *agent.History Runtime *agent.Runtime - NewToolServer func() toolsrv.Runner + NewToolServer func() *toolsrv.Conn NewBackend func(string) (backend.Backend, error) Log *olog.Logger MaxSteps int @@ -72,7 +72,7 @@ type Session struct { // Tool server lifecycle proc *toolsrv.Process // toolsrv subprocess keeper *toolsrv.ProcessKeeper // resilient process manager - toolsConn toolsrv.Runner // multiplexed RPC connection + toolsConn *toolsrv.Conn // multiplexed RPC connection paused bool // true when paused // Tool restrictions @@ -426,14 +426,14 @@ func (s *Session) SetKeeper(keeper *toolsrv.ProcessKeeper) { } // ToolsConn returns the tool server connection. -func (s *Session) ToolsConn() toolsrv.Runner { +func (s *Session) ToolsConn() *toolsrv.Conn { s.mu.RLock() defer s.mu.RUnlock() return s.toolsConn } // SetToolsConn sets the tool server connection. -func (s *Session) SetToolsConn(conn toolsrv.Runner) { +func (s *Session) SetToolsConn(conn *toolsrv.Conn) { s.mu.Lock() defer s.mu.Unlock() s.toolsConn = conn @@ -463,10 +463,7 @@ func (s *Session) IsConnected() bool { if paused || conn == nil { return false } - if pinger, ok := conn.(interface{ Ping() error }); ok { - return pinger.Ping() == nil - } - return true + return conn.Ping() == nil } // Pause stops the tool server to save resources. @@ -490,9 +487,7 @@ func (s *Session) Pause() error { } // Close any existing connection. if s.toolsConn != nil { - if c, ok := s.toolsConn.(interface{ Close() }); ok { - c.Close() - } + s.toolsConn.Close() s.toolsConn = nil } s.paused = true @@ -591,24 +586,24 @@ func (s *Session) LoadTool(name string, ag *agent.Agent) error { if blocked { return fmt.Errorf("tool %q is disallowed for this session", name) } - var runner toolsrv.Runner + var conn *toolsrv.Conn if s.toolsConn != nil { - runner = s.toolsConn + conn = s.toolsConn } else if ag != nil { - runner = ag.ToolServer() + conn = ag.ToolServer() } - return LoadToolOnRunner(runner, name) + return LoadToolOnConn(conn, name) } -// LoadToolOnRunner loads a tool on the given runner. -func LoadToolOnRunner(runner toolsrv.Runner, name string) error { - if runner == nil { +// LoadToolOnConn loads a tool on the given connection. +func LoadToolOnConn(conn *toolsrv.Conn, name string) error { + if conn == nil { return nil } ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() args, _ := json.Marshal(map[string]string{"name": name}) - if _, err := runner.CallTool(ctx, "tool_load", json.RawMessage(args)); err != nil { + if _, err := conn.CallTool(ctx, "tool_load", json.RawMessage(args)); err != nil { return fmt.Errorf("remote load: %w", err) } return nil diff --git a/session/setup.go b/session/setup.go index 866ea42..308c306 100644 --- a/session/setup.go +++ b/session/setup.go @@ -15,8 +15,8 @@ import ( type SessionInfra struct { Proc *toolsrv.Process Keeper *toolsrv.ProcessKeeper - ToolsConn toolsrv.Runner - NewToolServer func() toolsrv.Runner + ToolsConn *toolsrv.Conn + NewToolServer func() *toolsrv.Conn PromptEnv []string } @@ -36,7 +36,7 @@ type ToolServerConfig struct { type InfraConfig struct { Proc *toolsrv.Process Keeper *toolsrv.ProcessKeeper - ToolsConn toolsrv.Runner + ToolsConn *toolsrv.Conn } // SetupToolServer spawns the tool server process and creates the infrastructure. @@ -47,9 +47,9 @@ func SetupToolServer(cfg ToolServerConfig) (*SessionInfra, error) { // Reuse existing infrastructure if provided. if cfg.ReuseFrom != nil { r := cfg.ReuseFrom - var newToolServer func() toolsrv.Runner + var newToolServer func() *toolsrv.Conn if r.Keeper != nil { - newToolServer = func() toolsrv.Runner { + newToolServer = func() *toolsrv.Conn { conn, err := r.Keeper.Dial() if err != nil { return nil @@ -57,13 +57,13 @@ func SetupToolServer(cfg ToolServerConfig) (*SessionInfra, error) { return conn } } else { - newToolServer = func() toolsrv.Runner { + newToolServer = func() *toolsrv.Conn { return r.ToolsConn } } - // Update CWD if the connection supports it. - if cwc, ok := r.ToolsConn.(interface{ SetCWD(string) }); ok { - cwc.SetCWD(cfg.CWD) + // Update CWD on the reused connection. + if r.ToolsConn != nil { + r.ToolsConn.SetCWD(cfg.CWD) } return &SessionInfra{ Proc: r.Proc, @@ -75,7 +75,7 @@ func SetupToolServer(cfg ToolServerConfig) (*SessionInfra, error) { } var proc *toolsrv.Process var keeper *toolsrv.ProcessKeeper - var newToolServer func() toolsrv.Runner + var newToolServer func() *toolsrv.Conn var promptEnv []string var err error @@ -93,7 +93,7 @@ func SetupToolServer(cfg ToolServerConfig) (*SessionInfra, error) { CWD: cfg.CWD, }) }) - newToolServer = func() toolsrv.Runner { + newToolServer = func() *toolsrv.Conn { conn, err := keeper.Dial() if err != nil { return nil @@ -117,7 +117,7 @@ func SetupToolServer(cfg ToolServerConfig) (*SessionInfra, error) { keeper = toolsrv.NewProcessKeeper(cfg.Ctx, proc, func(ctx context.Context) (*toolsrv.Process, error) { return toolsrv.Spawn(ctx, cfg.CWD, dialOpts...) }) - newToolServer = func() toolsrv.Runner { + newToolServer = func() *toolsrv.Conn { conn, err := keeper.Dial() if err != nil { return nil @@ -178,24 +178,22 @@ func BuildPromptLayers(cfg *agent.AgentConfig, cwd, sessID, uname string, prompt // LoadAutoLoadTools loads all tools from the agent config's autoLoad list. // It first sends the session ID and uname to the tool server so that // ollie-remote can attach its tool registry before processing tool_load RPCs. -func LoadAutoLoadTools(cfg *agent.AgentConfig, runner toolsrv.Runner, sessID, uname string, logError func(string, ...any)) { - if cfg == nil || runner == nil { +func LoadAutoLoadTools(cfg *agent.AgentConfig, conn *toolsrv.Conn, sessID, uname string, logError func(string, ...any)) { + if cfg == nil || conn == nil { return } // Send session ID to the tool server first — ollie-remote uses this // to attach its tool registry via the OnEnvSet callback. - if es, ok := runner.(interface{ SetEnv(string, string) }); ok { - if sessID != "" { - es.SetEnv("OLLIE_SESSION_ID", sessID) - } - if uname != "" { - es.SetEnv("OLLIE_UNAME", uname) - } + if sessID != "" { + conn.SetEnv("OLLIE_SESSION_ID", sessID) + } + if uname != "" { + conn.SetEnv("OLLIE_UNAME", uname) } for _, tl := range cfg.AutoLoad { - if err := LoadToolOnRunner(runner, tl); err != nil { + if err := LoadToolOnConn(conn, tl); err != nil { if logError != nil { logError("autoLoad tool %q: %v", tl, err) } diff --git a/toolsrv/conn.go b/toolsrv/conn.go index 969ac07..f1e0dc1 100644 --- a/toolsrv/conn.go +++ b/toolsrv/conn.go @@ -103,6 +103,10 @@ func (c *Conn) SetOnToolsChanged(fn func(string)) { c.pendingMu.Unlock() } +// ToolRegistryRevision returns 0 for remote connections (revision tracking +// is not supported over RPC; use SetOnToolsChanged for push-based refresh). +func (c *Conn) ToolRegistryRevision() uint64 { return 0 } + func (c *Conn) Detach() bool { resp, err := c.call("detach", nil) if err != nil { diff --git a/toolsrv/tools.go b/toolsrv/tools.go index 080d354..d25d656 100644 --- a/toolsrv/tools.go +++ b/toolsrv/tools.go @@ -3,7 +3,6 @@ package toolsrv import ( - "context" "encoding/json" ) @@ -29,10 +28,3 @@ type ToolInfo struct { // the soft step-budget guardrail. ResetsCounter bool } - -// Runner is the minimal interface satisfied by any tool server (local or remote). -// Consumers that need polymorphism over Server and RemoteServer use this. -type Runner interface { - ListTools() ([]ToolInfo, error) - CallTool(ctx context.Context, tool string, args json.RawMessage) (json.RawMessage, error) -} diff --git a/toolsrv/tools_test.go b/toolsrv/tools_test.go deleted file mode 100644 index 632cbc3..0000000 --- a/toolsrv/tools_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package toolsrv_test - -import ( - "context" - "encoding/json" - "testing" - - "ollie/toolsrv" -) - -// stubRunner is a minimal Runner used to verify the contract. -type stubRunner struct { - name string - tools []toolsrv.ToolInfo -} - -func (s *stubRunner) ListTools() ([]toolsrv.ToolInfo, error) { return s.tools, nil } -func (s *stubRunner) CallTool(_ context.Context, tool string, _ json.RawMessage) (json.RawMessage, error) { - return json.RawMessage(`{"tool":"` + tool + `"}`), nil -} - -func newStub(name string, toolNames ...string) *stubRunner { - var ti []toolsrv.ToolInfo - for _, n := range toolNames { - ti = append(ti, toolsrv.ToolInfo{Name: n, Description: n + " desc"}) - } - return &stubRunner{name: name, tools: ti} -} - -// checkRunnerContract verifies Runner invariants. -func checkRunnerContract(t *testing.T, r toolsrv.Runner) { - t.Helper() - tl, err := r.ListTools() - if err != nil { - t.Fatalf("ListTools: %v", err) - } - if tl == nil { - t.Fatal("ListTools returned nil") - } - for _, ti := range tl { - res, err := r.CallTool(context.Background(), ti.Name, json.RawMessage(`{}`)) - if err != nil { - t.Errorf("CallTool(%q): %v", ti.Name, err) - } - if len(res) == 0 { - t.Errorf("CallTool(%q) returned empty result", ti.Name) - } - } -} - -func TestStubRunnerContract(t *testing.T) { - checkRunnerContract(t, newStub("s", "a", "b")) -}