From b338e12fbfd37c5db76b1b2543dbccfc7b2120d3 Mon Sep 17 00:00:00 2001 From: Snider Date: Tue, 14 Apr 2026 18:24:47 +0100 Subject: [PATCH] fix(agent): process action overrides survive ServiceStartup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit go-process's OnStartup re-registers process.start/run/kill with string-ID variants, clobbering the agent's custom handlers that return *process.Process. This broke pid/queue helpers and 7 tests that need the rich handle (TestPid_ProcessAlive_Good, TestQueue_CanDispatchAgent_Bad_AgentAtLimit, etc). Register a Startable override service that reapplies the agent handlers after every service finishes OnStartup โ€” since services run in registration order, "agentic.process-overrides" always runs last and wins. Co-Authored-By: Virgil --- pkg/agentic/process_register.go | 77 ++++++++++++++++++++++++++-- pkg/agentic/process_register_test.go | 37 +++++++++++++ 2 files changed, 109 insertions(+), 5 deletions(-) diff --git a/pkg/agentic/process_register.go b/pkg/agentic/process_register.go index a229c8c..d00f5d7 100644 --- a/pkg/agentic/process_register.go +++ b/pkg/agentic/process_register.go @@ -9,12 +9,27 @@ import ( "dappco.re/go/core/process" ) +// processActionHandlers owns the agent-side overrides for the +// `process.*` actions. Start/run/kill return rich values (the +// `*process.Process` handle) instead of the raw string IDs surfaced +// by go-process so dispatch code can reap, signal, and tree-kill +// managed children without another lookup. +// +// Usage: `handlers := &processActionHandlers{service: svc}` type processActionHandlers struct { service *process.Service } -// c := core.New(core.WithService(agentic.ProcessRegister)) -// processService := c.Service("process") +// ProcessRegister ensures a `*process.Service` is available under the +// "process" service name and installs the agent-specific action +// overrides. Registering as a Startable service means the agent +// handlers run AFTER go-process's own OnStartup (which installs the +// string-ID variants), so the dispatch-friendly overrides always win. +// +// Usage: +// +// c := core.New(core.WithService(agentic.ProcessRegister)) +// processService := c.Service("process") func ProcessRegister(c *core.Core) core.Result { if c == nil { return core.Result{Value: core.E("agentic.ProcessRegister", "core is required", nil), OK: false} @@ -44,13 +59,65 @@ func ProcessRegister(c *core.Core) core.Result { } handlers := &processActionHandlers{service: service} - c.Action("process.run", handlers.handleRun) - c.Action("process.start", handlers.handleStart) - c.Action("process.kill", handlers.handleKill) + // Install the overrides now โ€” good for callers who never run + // ServiceStartup (smaller test setups) and for the + // pre-registered-service path where go-process may already have + // started. + handlers.registerActions(c) + + // Also register as a Startable service so the overrides survive + // any subsequent `process` OnStartup that would otherwise + // clobber them. The override service runs last because it + // registers after `process`. + overrideName := "agentic.process-overrides" + if existing := c.Service(overrideName); !existing.OK { + if registerResult := c.RegisterService(overrideName, &processOverrideService{handlers: handlers, core: c}); !registerResult.OK { + return registerResult + } + } return core.Result{OK: true} } +// processOverrideService reinstalls the agent-side action overrides +// once Core finishes calling OnStartup on every registered service. +// go-process re-registers `process.start`/`process.kill`/`process.run` +// during its own OnStartup, so the override has to run after that to +// keep the dispatch-friendly contract. +// +// Usage: `c.RegisterService("agentic.process-overrides", &processOverrideService{handlers: h, core: c})` +type processOverrideService struct { + handlers *processActionHandlers + core *core.Core +} + +// OnStartup is called by Core after every underlying service has +// booted. The override is reapplied at the tail of the lifecycle so +// the agent-side handlers win. +// +// Usage: `_ = svc.OnStartup(ctx)` +func (s *processOverrideService) OnStartup(context.Context) core.Result { + if s == nil || s.handlers == nil { + return core.Result{OK: true} + } + s.handlers.registerActions(s.core) + return core.Result{OK: true} +} + +// registerActions wires the override handlers onto `c`. It is safe +// to call multiple times โ€” each call simply overwrites the same +// action names. +// +// Usage: `handlers.registerActions(c)` +func (h *processActionHandlers) registerActions(c *core.Core) { + if h == nil || c == nil { + return + } + c.Action("process.run", h.handleRun) + c.Action("process.start", h.handleStart) + c.Action("process.kill", h.handleKill) +} + func (h *processActionHandlers) handleRun(ctx context.Context, options core.Options) core.Result { output, err := h.service.RunWithOptions(ctx, process.RunOptions{ Command: options.String("command"), diff --git a/pkg/agentic/process_register_test.go b/pkg/agentic/process_register_test.go index 3c2d8fc..935d386 100644 --- a/pkg/agentic/process_register_test.go +++ b/pkg/agentic/process_register_test.go @@ -111,3 +111,40 @@ func TestProcessRegister_HandleStart_Ugly_StartAndKill(t *testing.T) { t.Fatal("process.kill did not stop the managed process") } } + +// ProcessOverrideService guards the RFC ยง7 dispatch contract: go-process's own +// OnStartup registers string-ID variants of `process.*` which would break the +// agent's pid/queue helpers. The override service reapplies agent handlers +// after ServiceStartup so the custom `*process.Process`-returning handlers win. + +func TestProcessRegister_OverrideService_Good_ServiceStartupPreservesAgentHandlers(t *testing.T) { + t.Setenv("CORE_WORKSPACE", t.TempDir()) + + c := core.New(core.WithService(ProcessRegister)) + require.True(t, c.ServiceStartup(context.Background(), nil).OK) + + r := c.Action("process.start").Run(context.Background(), core.NewOptions( + core.Option{Key: "command", Value: "sleep"}, + core.Option{Key: "args", Value: []string{"30"}}, + core.Option{Key: "detach", Value: true}, + )) + require.True(t, r.OK) + + proc, ok := r.Value.(*process.Process) + require.True(t, ok, "agent-side process.start must still return *process.Process after ServiceStartup") + require.NotEmpty(t, proc.ID) + + defer proc.Kill() +} + +func TestProcessRegister_OverrideService_Bad_NilHandlers(t *testing.T) { + svc := &processOverrideService{} + result := svc.OnStartup(context.Background()) + assert.True(t, result.OK, "OnStartup with nil handlers should succeed without panicking") +} + +func TestProcessRegister_OverrideService_Ugly_NilCore(t *testing.T) { + svc := &processOverrideService{handlers: &processActionHandlers{}, core: nil} + result := svc.OnStartup(context.Background()) + assert.True(t, result.OK, "OnStartup with nil core should no-op without panic") +}