diff --git a/cmd/ship/main.go b/cmd/ship/main.go index ca88622..67082ab 100644 --- a/cmd/ship/main.go +++ b/cmd/ship/main.go @@ -10,6 +10,7 @@ import ( "github.com/maxBRT/ship-cli/internal/agent" "github.com/maxBRT/ship-cli/internal/gitops" + "github.com/maxBRT/ship-cli/internal/herdr" "github.com/maxBRT/ship-cli/internal/observe" "github.com/maxBRT/ship-cli/internal/run" "github.com/maxBRT/ship-cli/internal/throbber" @@ -60,6 +61,7 @@ func Main(args []string, stdout, stderr io.Writer, dir string) int { Config: cfg, Throbber: throbber.Line{Out: stderr, Color: true}, Observer: observe.New(dir, stderr), + Herdr: herdr.Reporter{}, Stdout: stdout, } if err := orchestrator.Run(context.Background()); err != nil { diff --git a/internal/herdr/doc.go b/internal/herdr/doc.go new file mode 100644 index 0000000..aee348d --- /dev/null +++ b/internal/herdr/doc.go @@ -0,0 +1,7 @@ +// Package herdr reports Phase lifecycle state to Herdr (and compatible +// multiplexers) so Agents sidebars show Ship as working while a Phase runs. +// +// The Run orchestrator calls Port.Working at Phase start and Port.Idle at +// Phase end (success, abort, or timeout). Reporter is the production adapter: +// it no-ops unless HERDR_ENV / HERDR_SOCKET_PATH / HERDR_PANE_ID are set. +package herdr diff --git a/internal/herdr/port.go b/internal/herdr/port.go new file mode 100644 index 0000000..6716263 --- /dev/null +++ b/internal/herdr/port.go @@ -0,0 +1,15 @@ +package herdr + +import "context" + +// Port is the multiplexer agent-state surface the Run orchestrator calls +// around each Phase. Optional; nil means no reporting. +// +// Reports are best-effort: implementations must not fail the Phase. +type Port interface { + // Working announces that a Phase is in progress. Message may describe the + // Phase / Ticket for sidebar display. + Working(ctx context.Context, message string) + // Idle announces that the Phase has ended (success, abort, or timeout). + Idle(ctx context.Context) +} diff --git a/internal/herdr/reporter.go b/internal/herdr/reporter.go new file mode 100644 index 0000000..05bc8d3 --- /dev/null +++ b/internal/herdr/reporter.go @@ -0,0 +1,107 @@ +package herdr + +import ( + "context" + "os" + "os/exec" +) + +// Source and AgentID identify Ship's lifecycle reports to Herdr. +// AgentID is the Herdr --agent value for Ship-the-product, not the coding Agent port. +const ( + Source = "ship:run" + AgentID = "ship" +) + +// Exec runs the herdr CLI with the given args. Tests inject a fake; nil means +// the real herdr binary (or HERDR_BIN_PATH when set). +type Exec func(ctx context.Context, args ...string) error + +// Reporter is the production Port: it shells to `herdr pane report-agent` +// when running under Herdr, and is a no-op otherwise. +type Reporter struct { + // LookupEnv reads process environment. Nil means os.LookupEnv. + LookupEnv func(key string) (string, bool) + // Exec runs herdr. Nil means the real binary. + Exec Exec + // Bin overrides the herdr binary name/path. Empty uses HERDR_BIN_PATH or "herdr". + Bin string +} + +var _ Port = Reporter{} + +// Working reports state=working when under Herdr; otherwise it is a no-op. +func (r Reporter) Working(ctx context.Context, message string) { + r.report(ctx, "working", message) +} + +// Idle reports state=idle when under Herdr; otherwise it is a no-op. +func (r Reporter) Idle(ctx context.Context) { + r.report(ctx, "idle", "") +} + +func (r Reporter) report(ctx context.Context, state, message string) { + paneID, ok := r.paneID() + if !ok { + return + } + args := []string{ + "pane", "report-agent", paneID, + "--source", Source, + "--agent", AgentID, + "--state", state, + } + if message != "" { + args = append(args, "--message", message) + } + execFn := r.Exec + if execFn == nil { + execFn = r.defaultExec + } + _ = execFn(ctx, args...) // best-effort; missing herdr must not fail the Phase +} + +func (r Reporter) paneID() (string, bool) { + if !r.active() { + return "", false + } + lookup := r.lookup() + id, _ := lookup("HERDR_PANE_ID") + return id, true +} + +func (r Reporter) active() bool { + lookup := r.lookup() + if v, ok := lookup("HERDR_ENV"); !ok || v != "1" { + return false + } + if v, ok := lookup("HERDR_SOCKET_PATH"); !ok || v == "" { + return false + } + if v, ok := lookup("HERDR_PANE_ID"); !ok || v == "" { + return false + } + return true +} + +func (r Reporter) lookup() func(string) (string, bool) { + if r.LookupEnv != nil { + return r.LookupEnv + } + return os.LookupEnv +} + +func (r Reporter) bin() string { + if r.Bin != "" { + return r.Bin + } + if v, ok := r.lookup()("HERDR_BIN_PATH"); ok && v != "" { + return v + } + return "herdr" +} + +func (r Reporter) defaultExec(ctx context.Context, args ...string) error { + cmd := exec.CommandContext(ctx, r.bin(), args...) // #nosec G204 -- fixed herdr binary; args built by Ship + return cmd.Run() +} diff --git a/internal/herdr/reporter_test.go b/internal/herdr/reporter_test.go new file mode 100644 index 0000000..4b73e54 --- /dev/null +++ b/internal/herdr/reporter_test.go @@ -0,0 +1,97 @@ +package herdr_test + +import ( + "context" + "errors" + "testing" + + "github.com/maxBRT/ship-cli/internal/herdr" +) + +func TestReporter_envAbsent_doesNotReport(t *testing.T) { + // Outside Herdr, Ship must not invoke the multiplexer CLI — normal + // terminals and CI stay unchanged even if herdr is missing. + var calls int + r := herdr.Reporter{ + LookupEnv: func(string) (string, bool) { return "", false }, + Exec: func(context.Context, ...string) error { + calls++ + return nil + }, + } + r.Working(context.Background(), "Implement") + r.Idle(context.Background()) + if calls != 0 { + t.Errorf("herdr exec calls = %d, want 0 when HERDR_* env is absent", calls) + } +} + +func TestReporter_envPresent_reportsWorkingAndIdle(t *testing.T) { + // Worked example from the Herdr socket contract / CLI wrapper. + env := map[string]string{ + "HERDR_ENV": "1", + "HERDR_SOCKET_PATH": "/tmp/herdr.sock", + "HERDR_PANE_ID": "w1:p1", + } + var calls [][]string + r := herdr.Reporter{ + LookupEnv: func(k string) (string, bool) { + v, ok := env[k] + return v, ok + }, + Exec: func(_ context.Context, args ...string) error { + cp := make([]string, len(args)) + copy(cp, args) + calls = append(calls, cp) + return nil + }, + } + + r.Working(context.Background(), "Iteration 2 · Review · #50 Pi Agent adapter") + r.Idle(context.Background()) + + want := [][]string{ + {"pane", "report-agent", "w1:p1", "--source", "ship:run", "--agent", "ship", "--state", "working", "--message", "Iteration 2 · Review · #50 Pi Agent adapter"}, + {"pane", "report-agent", "w1:p1", "--source", "ship:run", "--agent", "ship", "--state", "idle"}, + } + if len(calls) != len(want) { + t.Fatalf("herdr exec calls = %d, want %d; got %v", len(calls), len(want), calls) + } + for i := range want { + if !equalStrings(calls[i], want[i]) { + t.Errorf("call %d = %v, want %v", i, calls[i], want[i]) + } + } +} + +func TestReporter_execError_doesNotPanic(t *testing.T) { + // Missing herdr binary or socket failure must not surface into the Run. + env := map[string]string{ + "HERDR_ENV": "1", + "HERDR_SOCKET_PATH": "/tmp/herdr.sock", + "HERDR_PANE_ID": "w1:p1", + } + r := herdr.Reporter{ + LookupEnv: func(k string) (string, bool) { + v, ok := env[k] + return v, ok + }, + Exec: func(context.Context, ...string) error { + return errors.New("herdr: executable file not found") + }, + } + r.Working(context.Background(), "Final") + r.Idle(context.Background()) +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/internal/run/orchestrator.go b/internal/run/orchestrator.go index f209e31..52558b9 100644 --- a/internal/run/orchestrator.go +++ b/internal/run/orchestrator.go @@ -7,6 +7,7 @@ import ( "github.com/maxBRT/ship-cli/internal/agent" "github.com/maxBRT/ship-cli/internal/gitops" + "github.com/maxBRT/ship-cli/internal/herdr" "github.com/maxBRT/ship-cli/internal/observe" "github.com/maxBRT/ship-cli/internal/prompt" "github.com/maxBRT/ship-cli/internal/throbber" @@ -28,6 +29,7 @@ type Orchestrator struct { Config Config Throbber throbber.Port // optional; nil means no wait UI Observer observe.Port // optional; nil means no Phase observability + Herdr herdr.Port // optional; nil means no multiplexer agent-state reports Stdout io.Writer } @@ -193,6 +195,10 @@ func (r Orchestrator) runPhase(ctx context.Context, status throbber.Status, prom if r.Observer != nil { events = r.Observer.BeginPhase(status.Phase) } + if r.Herdr != nil { + r.Herdr.Working(ctx, status.Label()) + defer r.Herdr.Idle(ctx) + } work := func(ctx context.Context) error { return r.Agent.RunPhase(ctx, agent.PhaseRequest{ Prompt: promptText, diff --git a/internal/run/orchestrator_test.go b/internal/run/orchestrator_test.go index 3c45efd..30f9a70 100644 --- a/internal/run/orchestrator_test.go +++ b/internal/run/orchestrator_test.go @@ -15,6 +15,7 @@ import ( "github.com/maxBRT/ship-cli/internal/agent" "github.com/maxBRT/ship-cli/internal/gitops" + "github.com/maxBRT/ship-cli/internal/herdr" "github.com/maxBRT/ship-cli/internal/observe" "github.com/maxBRT/ship-cli/internal/run" "github.com/maxBRT/ship-cli/internal/throbber" @@ -134,6 +135,87 @@ func TestRun_wrapsEachPhaseInThrobberDuring(t *testing.T) { } } +func TestRun_reportsWorkingThenIdleAroundEachPhase(t *testing.T) { + // Under a multiplexer Port, each Phase must show working for its full + // duration and return to idle when it ends — independent of screen cues. + dir := initTempRepo(t) + tickets := &fakeTickets{ready: []ticket.Ticket{{Number: 1, Title: "one"}}} + prs := &fakePRs{} + rep := &recordingHerdr{} + ag := &fakeAgent{handler: func(idx int, req agent.PhaseRequest) error { + if len(rep.states) == 0 || rep.states[len(rep.states)-1] != "working" { + t.Fatalf("phase %d: last report = %v, want working during Agent work", idx, rep.states) + } + if isFinalPrompt(req.Prompt) { + prs.openPR("ship/run") + return nil + } + if idx == 1 { + writeAndCommit(t, req.Workspace, "impl.txt", "work\n", "implement") + } + return nil + }} + + r := run.Orchestrator{ + Tickets: tickets, + Agent: ag, + PRs: prs, + Repo: gitops.Repo{Dir: dir}, + Config: run.Config{Branch: "ship/run", MaxIterations: 10}, + Herdr: rep, + Stdout: &strings.Builder{}, + } + if err := r.Run(context.Background()); err != nil { + t.Fatalf("Run: %v", err) + } + + want := []string{ + "working", "idle", // Implement + "working", "idle", // Review + "working", "idle", // Final + } + if !slices.Equal(rep.states, want) { + t.Errorf("herdr states = %v, want %v", rep.states, want) + } + if len(rep.messages) != 3 { + t.Fatalf("working messages = %d, want 3", len(rep.messages)) + } + for i, msg := range rep.messages { + if msg == "" { + t.Errorf("working message %d is empty", i) + } + } +} + +func TestRun_reportsIdleWhenPhaseFails(t *testing.T) { + // Abort / timeout paths must still release the multiplexer to idle so the + // Agents sidebar does not stick on working after Ship stops. + dir := initTempRepo(t) + tickets := &fakeTickets{ready: []ticket.Ticket{{Number: 7, Title: "seven"}}} + rep := &recordingHerdr{} + ag := &fakeAgent{handler: func(int, agent.PhaseRequest) error { + return errors.New("agent boom") + }} + + r := run.Orchestrator{ + Tickets: tickets, + Agent: ag, + Repo: gitops.Repo{Dir: dir}, + Config: run.Config{Branch: "ship/run", MaxIterations: 10}, + Herdr: rep, + Stdout: &strings.Builder{}, + } + err := r.Run(context.Background()) + if err == nil { + t.Fatal("Run: want Abort error when a Phase fails") + } + + want := []string{"working", "idle"} + if !slices.Equal(rep.states, want) { + t.Errorf("herdr states = %v, want %v (idle even on failure)", rep.states, want) + } +} + func TestRun_emptyQueueExitsWithoutBranchOrWork(t *testing.T) { dir := initTempRepo(t) tickets := &fakeTickets{} @@ -1221,6 +1303,23 @@ func (r *recordingThrobber) During(ctx context.Context, status throbber.Status, return work(ctx) } +// recordingHerdr records multiplexer agent-state reports around Phases. +type recordingHerdr struct { + states []string + messages []string +} + +var _ herdr.Port = (*recordingHerdr)(nil) + +func (r *recordingHerdr) Working(_ context.Context, message string) { + r.states = append(r.states, "working") + r.messages = append(r.messages, message) +} + +func (r *recordingHerdr) Idle(context.Context) { + r.states = append(r.states, "idle") +} + // fakeQueue is the injectable picker/queue port for Orchestrator tests. // Without confirmFn it confirms every candidate in order. With confirmFn it // returns that selection (drop / reorder / empty / cancel) instead. diff --git a/internal/throbber/line.go b/internal/throbber/line.go index 0278035..2999706 100644 --- a/internal/throbber/line.go +++ b/internal/throbber/line.go @@ -56,20 +56,11 @@ func (l Line) During(ctx context.Context, status Status, work func(context.Conte } func formatStatus(s Status, spin rune) string { - var parts []string - if s.Iteration > 0 { - parts = append(parts, fmt.Sprintf("Iteration %d", s.Iteration)) - } - if s.Phase != "" { - parts = append(parts, s.Phase) - } - if s.Ticket != "" { - parts = append(parts, s.Ticket) - } - if len(parts) == 0 { + label := s.Label() + if label == "" { return string(spin) } - return fmt.Sprintf("%c %s", spin, strings.Join(parts, " · ")) + return fmt.Sprintf("%c %s", spin, label) } func finishLine(status Status, color, ok bool, d time.Duration) string { diff --git a/internal/throbber/port.go b/internal/throbber/port.go index b4b57c2..d7781a1 100644 --- a/internal/throbber/port.go +++ b/internal/throbber/port.go @@ -1,6 +1,10 @@ package throbber -import "context" +import ( + "context" + "fmt" + "strings" +) // Status labels a Phase wait: Iteration, Phase, and Ticket when applicable. type Status struct { @@ -9,6 +13,22 @@ type Status struct { Ticket string // e.g. "#7 one"; empty omits Ticket (Final) } +// Label is the human status text without spinner chrome, e.g. +// "Iteration 2 · Review · #50 Pi Agent adapter". Empty fields are omitted. +func (s Status) Label() string { + var parts []string + if s.Iteration > 0 { + parts = append(parts, fmt.Sprintf("Iteration %d", s.Iteration)) + } + if s.Phase != "" { + parts = append(parts, s.Phase) + } + if s.Ticket != "" { + parts = append(parts, s.Ticket) + } + return strings.Join(parts, " · ") +} + // Port is the Phase-wait UI surface the Run orchestrator calls. type Port interface { // During runs work while showing wait UI for status. It returns work's diff --git a/internal/throbber/port_test.go b/internal/throbber/port_test.go index 6d279a1..e929eca 100644 --- a/internal/throbber/port_test.go +++ b/internal/throbber/port_test.go @@ -11,6 +11,21 @@ import ( "github.com/maxBRT/ship-cli/internal/throbber" ) +func TestStatus_Label_joinsPresentFields(t *testing.T) { + got := throbber.Status{Phase: "Review", Iteration: 2, Ticket: "#50 Pi Agent adapter"}.Label() + want := "Iteration 2 · Review · #50 Pi Agent adapter" + if got != want { + t.Errorf("Label() = %q, want %q", got, want) + } +} + +func TestStatus_Label_omitsEmptyFields(t *testing.T) { + got := throbber.Status{Phase: "Final"}.Label() + if got != "Final" { + t.Errorf("Label() = %q, want %q", got, "Final") + } +} + func TestSilent_During_runsWorkAndReturnsItsError(t *testing.T) { want := errors.New("phase failed") called := false