refactor: query toolsrv directly for proc list
- ListProcs() now queries toolsrv directly instead of local tracker - Remove ownership verification from SignalProc/ProcOutput/DismissProc - Clean up stale procs from local tracker when they disappear from toolsrv - Document local tracker quirk in TODO.md for future cleanup
This commit is contained in:
parent
1d5e9c4f8d
commit
6eb3bf960b
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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</system-proc-interrupt>")
|
||||
}
|
||||
|
||||
// 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()
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Reference in New Issue