Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/ship/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions internal/herdr/doc.go
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions internal/herdr/port.go
Original file line number Diff line number Diff line change
@@ -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)
}
107 changes: 107 additions & 0 deletions internal/herdr/reporter.go
Original file line number Diff line number Diff line change
@@ -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()
}
97 changes: 97 additions & 0 deletions internal/herdr/reporter_test.go
Original file line number Diff line number Diff line change
@@ -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
}
6 changes: 6 additions & 0 deletions internal/run/orchestrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}

Expand Down Expand Up @@ -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,
Expand Down
99 changes: 99 additions & 0 deletions internal/run/orchestrator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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.
Expand Down
Loading
Loading