diff --git a/cmd/olliesrv/internal/agent/agent.go b/cmd/olliesrv/internal/agent/agent.go index 2a1ddfd..22dbd76 100644 --- a/cmd/olliesrv/internal/agent/agent.go +++ b/cmd/olliesrv/internal/agent/agent.go @@ -583,52 +583,44 @@ type BgProc struct { Status string // "running" or "exited (N)" } -// ListProcs returns a list of background processes owned by this agent. +// ListProcs returns a list of background processes from toolsrv. func (ag *Agent) ListProcs() []BgProc { - procs := ag.bgProcs.List() ts := ag.ToolServer() - out := make([]BgProc, len(procs)) - for i, p := range procs { - status := "running" - if ts != nil { - info := ts.ListDetachedRaw() - for _, raw := range info { - m, ok := raw.(map[string]any) - if !ok { - continue - } - if pid, ok := m["pid"].(int); ok && pid == p.PID { - if exited, ok := m["exited"].(bool); ok && exited { - code := 0 - if c, ok := m["exit_code"].(int); ok { - code = c - } - status = fmt.Sprintf("exited (%d)", code) - } - break - } - } + if ts == nil { + return nil + } + + var out []BgProc + for _, raw := range ts.ListDetachedRaw() { + m, ok := raw.(map[string]any) + if !ok { + continue } - out[i] = BgProc{PID: p.PID, Tool: p.Tool, Cmd: p.Cmd, Status: status} + pid, _ := m["pid"].(int) + cmd, _ := m["command"].(string) + + status := "running" + if exited, ok := m["exited"].(bool); ok && exited { + code := 0 + if c, ok := m["exit_code"].(int); ok { + code = c + } + status = fmt.Sprintf("exited (%d)", code) + } + + out = append(out, BgProc{ + PID: pid, + Tool: "shell", // toolsrv doesn't track tool name currently + Cmd: cmd, + Status: status, + }) } return out } -// SignalProc sends a signal to a background process owned by this agent. +// SignalProc sends a signal to a background process. // sig is "term" or "kill". func (ag *Agent) SignalProc(pid int, sig string) error { - // Verify this process belongs to this agent - procs := ag.bgProcs.List() - found := false - for _, p := range procs { - if p.PID == pid { - found = true - break - } - } - if !found { - return fmt.Errorf("process %d not found", pid) - } ts := ag.runtime.ToolServer if ts == nil { return fmt.Errorf("no tool server") @@ -645,20 +637,8 @@ func (ag *Agent) SignalProc(pid int, sig string) error { return ts.SignalDetached(pid, s) } -// ProcOutput returns the output of a background process owned by this agent. +// ProcOutput returns the output of a background process. func (ag *Agent) ProcOutput(pid int) (string, error) { - // Verify this process belongs to this agent - procs := ag.bgProcs.List() - found := false - for _, p := range procs { - if p.PID == pid { - found = true - break - } - } - if !found { - return "", fmt.Errorf("process %d not found", pid) - } ts := ag.runtime.ToolServer if ts == nil { return "", fmt.Errorf("no tool server") @@ -666,28 +646,16 @@ func (ag *Agent) ProcOutput(pid int) (string, error) { return ts.GetDetachedOutput(pid) } -// DismissProc dismisses a background process owned by this agent. +// DismissProc dismisses a background process. func (ag *Agent) DismissProc(pid int) error { - // Verify this process belongs to this agent - procs := ag.bgProcs.List() - found := false - for _, p := range procs { - if p.PID == pid { - found = true - break - } - } - if !found { - return fmt.Errorf("process %d not found", pid) - } ts := ag.runtime.ToolServer if ts == nil { return fmt.Errorf("no tool server") } if !ts.DismissDetached(pid) { - return fmt.Errorf("failed to dismiss process %d", pid) + return fmt.Errorf("process %d not found", pid) } - ag.bgProcs.Remove(pid) + ag.bgProcs.Remove(pid) // Also remove from local tracker if present return nil } diff --git a/cmd/olliesrv/internal/agent/background.go b/cmd/olliesrv/internal/agent/background.go index 8a32c54..5e4b437 100644 --- a/cmd/olliesrv/internal/agent/background.go +++ b/cmd/olliesrv/internal/agent/background.go @@ -56,6 +56,7 @@ func (bt *bgTracker) CollectInterrupts(conn *toolsrv.Conn) string { } var sb strings.Builder + var gone []int for i := range bt.procs { p := &bt.procs[i] @@ -63,7 +64,8 @@ func (bt *bgTracker) CollectInterrupts(conn *toolsrv.Conn) string { // Get current output output, err := conn.GetDetachedOutput(p.PID) if err != nil { - // Process may have been dismissed from toolsrv or doesn't exist - skip it + // Process no longer exists in toolsrv (GC'd or dismissed) - mark for removal + gone = append(gone, i) continue } @@ -107,6 +109,12 @@ func (bt *bgTracker) CollectInterrupts(conn *toolsrv.Conn) string { sb.WriteString("\n") } + // Remove procs that no longer exist in toolsrv (reverse order to preserve indices) + for i := len(gone) - 1; i >= 0; i-- { + idx := gone[i] + bt.procs = append(bt.procs[:idx], bt.procs[idx+1:]...) + } + return sb.String() } diff --git a/doc/TODO.md b/doc/TODO.md new file mode 100644 index 0000000..485ec8e --- /dev/null +++ b/doc/TODO.md @@ -0,0 +1,16 @@ + +## Background Process Tracking Cleanup + +The agent maintains a local `bgTracker` (`bgProcs`) that partially duplicates toolsrv's proc state. This creates sync issues: + +- `ListProcs()` now queries toolsrv directly (correct) +- `CollectInterrupts()` still uses local tracker for `LastOutput` diff detection +- `bgProcs.Add()` is still called when starting background procs +- `bgProcs.Remove()` is called on dismiss + +Consider: +1. Remove local tracker entirely, track only `LastOutput` map keyed by PID +2. Or add agent ID to toolsrv Proc struct for proper ownership filtering +3. Move `LastOutput` tracking to toolsrv (add a "read offset" or "last read hash") + +Current state works but is fragile - toolsrv is source of truth for proc list, local tracker is only for interrupt diff detection.