From 8a23b71d0b3544a126e8c8a107b3d3342d92f500 Mon Sep 17 00:00:00 2001 From: Sam Bretz Date: Wed, 16 Sep 2026 09:46:25 -0700 Subject: [PATCH 1/3] feat: cut the dashboard to Chat, Checkpoint, Changes, Tests and a Decision Log The detail area had eight tabs, and on envctl's own runs most of them did not help follow or steer one. Services duplicated the preview URL already in the run summary. Graph repeated the stage strip for linear workflows. History listed revision and checkpoint IDs, which say what exists but not what happened or why. And the problems that actually stop a run sat on Readiness, two keypresses from where you land. The tabs are now Chat, Checkpoint, Changes, Tests and Decision Log. Nothing that can block a run was dropped with them: Chat now leads with the readiness problems Plan must resolve, any recovery in progress, and any service that is down in the VM, one line each so a small terminal keeps room for the stage itself. Healthy services, which were most of the Services tab, are left out on purpose. A stage's dependencies, the useful part of Graph, are on its Checkpoint. The Decision Log is derived from state the run already keeps, with no new persisted fields. Each entry has a time, stage, actor and a one-line reason: Plan's discovered requirements, supervisor accepts and rejections with the correction, steering with where it went, approvals, rewinds, and past needs-attention causes from the captain's log when a tracker is configured. A supervisor rejection is recorded on the attempt as a failure prefixed "supervisor correction:", so it is read back and attributed to the supervisor rather than reported as an ordinary failure. A rewind is reported as what it reran rather than the stage that was picked: that choice is not stored on the revision, and changing the objective reruns every stage whatever was picked, so naming it would sometimes be false. The composer now says it steers and that this changes the work, so an ask mode can sit beside it without the two being confused. Asking the supervisor a question and getting an answer is not part of this change; it needs a new agent invocation path and is tracked separately. Refs #24 Co-Authored-By: Claude Opus 5 (1M context) --- docs/src/content/docs/cli-reference.mdx | 2 +- docs/src/content/docs/first-workflow.mdx | 6 +- internal/tui/decisions.go | 222 +++++++++++++++++++++++ internal/tui/model.go | 88 ++++----- internal/tui/model_test.go | 2 +- internal/tui/panels_test.go | 171 +++++++++++++++++ internal/tui/review.go | 29 --- internal/tui/services.go | 32 ---- internal/tui/services_test.go | 26 --- 9 files changed, 436 insertions(+), 142 deletions(-) create mode 100644 internal/tui/decisions.go create mode 100644 internal/tui/panels_test.go delete mode 100644 internal/tui/services.go delete mode 100644 internal/tui/services_test.go diff --git a/docs/src/content/docs/cli-reference.mdx b/docs/src/content/docs/cli-reference.mdx index a640109..6c0f3c9 100644 --- a/docs/src/content/docs/cli-reference.mdx +++ b/docs/src/content/docs/cli-reference.mdx @@ -55,7 +55,7 @@ envctl run message --node code --to worker --text "keep the public API `run show` prints each running attempt's phase (`worker`, `stack`, `checks`, `supervisor`), its current check, and its latest agent activity: messages, tool and command invocations, and check output. `--json` includes the same `progress` object on each attempt, with at most 20 lines of 300 bytes each. Activity comes from the harness's own event stream, redacted by the guest journal and again for configured credentials. It is display state only and never checkpoint evidence. Activity-only updates are written at most every few seconds; phase changes and deliveries are written at once. -`run message` (and `i` in Bubble Tea) records a message against the current revision. `--node` limits it to one stage; without it, the message applies to every stage. `run show` and the Bubble Tea Conversation panel show where each message went: +`run message` (and `i` in Bubble Tea) records a message against the current revision. `--node` limits it to one stage; without it, the message applies to every stage. `run show` and the Bubble Tea Chat panel show where each message went: - **included when … started**: the message existed when the agent's prompt was frozen. - **delivered live … (resume N)**: the message reached a running agent. diff --git a/docs/src/content/docs/first-workflow.mdx b/docs/src/content/docs/first-workflow.mdx index 3923e6a..c3962ec 100644 --- a/docs/src/content/docs/first-workflow.mdx +++ b/docs/src/content/docs/first-workflow.mdx @@ -111,8 +111,8 @@ The dashboard shows the stage strip, your in-flight runs, and a focused panel: | --- | --- | | `↑` `↓` | Move between runs | | `←` `→` | Move between stages | -| `tab` | Cycle panels: Conversation, Checkpoint, Changes, Tests, Services, Readiness, Graph, History | -| `i` | Write to the agent; `s` switches between worker and supervisor | +| `tab` | Cycle panels: Chat, Checkpoint, Changes, Tests, Decision Log | +| `i` | Steer the agent (this changes the work); `s` switches between worker and supervisor | | `d` | Diff the stage; `b` sets a comparison base, `B` clears it | | `o` | Open the selected artifact in your browser; `,` and `.` select | | `[` `]` | Browse revision history | @@ -122,6 +122,8 @@ The dashboard shows the stage strip, your in-flight runs, and a focused panel: | `T` | Pick a color theme, with live preview | | `q` | Detach; execution continues | +**Chat** leads with anything blocking the run — readiness problems Plan must resolve, recovery in progress, or a service down in the VM — then the stage's attempts, summaries and steering messages. **Checkpoint** shows the accepted result and which stages it runs after. **Decision Log** reads like a record of the run: when each thing was decided, by whom (worker, supervisor, you, or the coordinator), and why — Plan's findings, supervisor accepts and rejections with their corrections, your steering and approvals, and rewinds with what they reran and whether the objective or configuration changed. + Closing the dashboard never stops the run. For scripts, `envctl run show --json` has the same information. Prefer a browser? `envctl web` serves the same dashboard on this machine and opens it: diff --git a/internal/tui/decisions.go b/internal/tui/decisions.go new file mode 100644 index 0000000..6551020 --- /dev/null +++ b/internal/tui/decisions.go @@ -0,0 +1,222 @@ +package tui + +import ( + "fmt" + "slices" + "strings" + "time" + + "github.com/sam-bretz/envctl/internal/workflow" +) + +// decision is one line of the Decision Log: who decided what, and why. +type decision struct { + At time.Time + Stage string + Actor string // worker, supervisor, you, or coordinator + Reason string +} + +// correctionPrefix is how the backend records a supervisor rejection on the +// attempt it failed. The log reads it back so a rejection is attributed to +// the supervisor with its correction, not reported as an ordinary failure. +const correctionPrefix = "supervisor correction: " + +// decisions derives the Decision Log from state the run already keeps. It +// replaces History, which listed revision and checkpoint IDs: those say what +// exists, not what happened or why. +func decisions(r *workflow.Run) []decision { + var out []decision + for i := range r.Revisions { + rev := &r.Revisions[i] + plan := rev.Config.Workflow.PlanID() + if rev.Parent == "" { + out = append(out, decision{At: rev.CreatedAt, Actor: "coordinator", Reason: "run started: " + firstLine(rev.Objective)}) + } else if parent := r.Revision(rev.Parent); parent != nil { + out = append(out, decision{At: rev.CreatedAt, Stage: rewoundTo(rev), Actor: "you", Reason: rewindReason(parent, rev)}) + } + for _, req := range rev.DiscoveredRequirements { + out = append(out, decision{ + At: planTime(rev, plan), Stage: plan, Actor: "worker", + Reason: fmt.Sprintf("found that %s needs %s: %s", strings.Join(req.Nodes, ", "), req.Capability, firstLine(req.Reason)), + }) + } + for _, a := range rev.Attempts { + switch { + case a.State == "failed" && strings.HasPrefix(a.Error, correctionPrefix): + out = append(out, decision{At: a.UpdatedAt, Stage: a.Node, Actor: "supervisor", + Reason: fmt.Sprintf("rejected attempt %d: %s", a.Number, firstLine(strings.TrimPrefix(a.Error, correctionPrefix)))}) + case a.State == "failed": + out = append(out, decision{At: a.UpdatedAt, Stage: a.Node, Actor: "coordinator", + Reason: fmt.Sprintf("attempt %d failed: %s", a.Number, firstLine(a.Error))}) + case a.Result != nil && a.Result.Review.Accepted: + out = append(out, decision{At: a.UpdatedAt, Stage: a.Node, Actor: "supervisor", + Reason: fmt.Sprintf("accepted attempt %d: %s", a.Number, firstLine(a.Result.Review.Summary))}) + } + if a.Approval != nil { + out = append(out, decision{At: a.Approval.At, Stage: a.Node, Actor: approver(a.Approval.Actor), + Reason: fmt.Sprintf("approved attempt %d", a.Number)}) + } + } + for _, msg := range rev.Messages { + stage := msg.Node + if stage == "" { + stage = "every stage" + } + out = append(out, decision{At: msg.CreatedAt, Stage: stage, Actor: "you", + Reason: fmt.Sprintf("told the %s: %s [%s]", msg.Recipient, firstLine(msg.Body), rev.MessageStatus(msg))}) + } + } + // The captain's log already records past needs-attention causes with their + // time, which a revision's current Recovery cannot: it only holds the + // latest one. It exists only when a tracker is configured. + for _, entry := range r.TrackerLog { + if entry.Kind == workflow.TrackerKindNeedsAttention { + out = append(out, decision{At: entry.Occurred, Stage: entry.Node, Actor: "coordinator", Reason: "needed attention: " + firstLine(entry.Detail)}) + } + } + slices.SortStableFunc(out, func(a, b decision) int { return a.At.Compare(b.At) }) + // What needs attention now is current, not historical, so it goes last + // rather than at whatever time a retry happens to be scheduled for. + if cur := r.Current(); cur != nil && cur.Recovery != nil { + out = append(out, decision{Actor: "coordinator", Reason: fmt.Sprintf("needs attention now (%s): %s", cur.Recovery.Phase, firstLine(cur.Recovery.Detail))}) + } + return out +} + +// rewoundTo is the first stage the new revision has to run again: the one it +// was rewound to. +func rewoundTo(rev *workflow.Revision) string { + order, _ := rev.Config.Workflow.Order() + for _, id := range order { + if _, kept := rev.Checkpoints[id]; !kept { + return id + } + } + return "" +} + +// rewindReason says what a rewind reran. The stage the person picked is not +// stored on the revision, and changing the objective reruns every stage +// regardless of it, so the first stage that had to run again is the truthful +// and the more useful thing to report. +func rewindReason(parent, rev *workflow.Revision) string { + reason := "rewound" + if stage := rewoundTo(rev); stage != "" { + reason += ", rerunning from " + stage + } + var changed []string + if parent.Objective != rev.Objective { + changed = append(changed, "the objective") + } + if workflow.Digest(parent.Config) != workflow.Digest(rev.Config) { + changed = append(changed, "the configuration") + } + if len(changed) > 0 { + reason += ", changing " + strings.Join(changed, " and ") + } + return reason +} + +// planTime places Plan's findings when Plan was accepted, the moment they +// became decisions rather than work in progress. +func planTime(rev *workflow.Revision, plan string) time.Time { + if cp, ok := rev.Checkpoints[plan]; ok { + return cp.CreatedAt + } + return rev.CreatedAt +} + +func approver(actor string) string { + if actor == "" || actor == "local" { + return "you" + } + return actor +} + +func firstLine(s string) string { + line, _, _ := strings.Cut(strings.TrimSpace(s), "\n") + if r := []rune(line); len(r) > 120 { + line = string(r[:120]) + "…" + } + return line +} + +// decisionLog renders the log for the dashboard panel. +func (m Model) decisionLog() string { + r := m.current() + if r == nil { + return "No run selected." + } + entries := decisions(r) + if len(entries) == 0 { + return "Nothing has been decided yet." + } + lines := make([]string, 0, len(entries)) + for _, d := range entries { + when := "now " + if !d.At.IsZero() { + when = d.At.Local().Format("15:04") + } + stage := d.Stage + if stage == "" { + stage = "run" + } + lines = append(lines, fmt.Sprintf("%s %-16s %-11s %s", when, stage, d.Actor, d.Reason)) + } + return strings.Join(lines, "\n") +} + +// blocking lists what is stopping a run right now: the readiness problems +// Plan must resolve and any recovery in progress, for the revision and for the +// selected stage's branch runtime. These used to live on the Readiness tab, +// where they sat behind two keypresses on the one screen that matters when a +// run is stuck, so Chat now leads with them. It is empty when nothing blocks. +func blocking(rev *workflow.Revision, node string) []string { + var out []string + // One line each: on a small terminal a full list pushed the stage's own + // content off the screen, which is worse than a truncated alert. + if problems := rev.ReadinessProblems(time.Now(), rev.Requirements()); len(problems) > 0 { + out = append(out, fmt.Sprintf("⚠ Plan must resolve %s: %s", plural(len(problems), "readiness problem"), strings.Join(problems, "; "))) + } + if rev.Recovery != nil { + out = append(out, recoveryLine("⚠ Needs attention", rev.Recovery)) + } + if child := rev.ChildRuntimes[node]; child != nil && child.Recovery != nil { + out = append(out, recoveryLine("⚠ "+node+"'s branch runtime needs attention", child.Recovery)) + } + // The Services tab listed every service. Healthy ones are noise while + // following a run, but one that is down is often why a stage failed, so + // only those are kept. + var down []string + for _, svc := range rev.Runtime.Services { + if !healthy(svc.State) { + down = append(down, fmt.Sprintf("%s is %s", svc.Name, svc.State)) + } + } + if len(down) > 0 { + out = append(out, "⚠ Services not healthy in the VM: "+strings.Join(down, "; ")) + } + return out +} + +// healthy reads a Compose service state such as "running/healthy", +// "running", "running/unhealthy" or "exited". +func healthy(state string) bool { + return strings.HasPrefix(state, "running") && !strings.Contains(state, "unhealthy") && !strings.Contains(state, "starting") +} + +func recoveryLine(title string, r *workflow.Recovery) string { + line := fmt.Sprintf("%s (%s): %s", title, r.Phase, firstLine(r.Detail)) + if !r.RetryAt.IsZero() { + line += " · retrying after " + r.RetryAt.Local().Format("15:04:05") + } + return line +} + +func plural(n int, word string) string { + if n == 1 { + return "1 " + word + } + return fmt.Sprintf("%d %ss", n, word) +} diff --git a/internal/tui/model.go b/internal/tui/model.go index ddeb98a..5cdcb91 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -106,7 +106,12 @@ type openedMsg struct { err error } -var panels = []string{"Conversation", "Checkpoint", "Changes", "Tests", "Services", "Readiness", "Graph", "History"} +// panels are the detail views. Services, Readiness and Graph were cut because +// they did not help follow or steer a run: the preview URL is already in the +// run summary, the readiness problems and recovery that can block a run now +// lead the Chat panel whenever they exist, and a stage's dependencies are +// shown on its Checkpoint. History became the Decision Log. +var panels = []string{"Chat", "Checkpoint", "Changes", "Tests", "Decision Log"} func New(api API, root string) Model { m := Model{API: api, Root: root, Width: 100, Height: 30, Recipient: "supervisor"} @@ -802,16 +807,19 @@ func (m Model) footer(width int, compact bool) []string { if m.Mode == "new-ref" { label = "issue link (optional, Enter to skip)" } + if m.Mode == "chat" { + label = "steer " + m.Recipient + " · changes the work" + } if compact || width < 24 { lines = append(lines, ansi.Truncate(t.bold().Render(label+" ")+input, width, "")) } else { lines = append(lines, t.box(label, []string{input}, width)...) } } else { - prompt := t.dim().Render("To ") + t.fg(t.accent()).Render(m.Recipient) + - t.dim().Render(" · i chat · n new · r rewind · a approve · c close · o artifact · d diff · p/P plugins · x cancel") + prompt := t.dim().Render("Steer ") + t.fg(t.accent()).Render(m.Recipient) + + t.dim().Render(" · i steer · n new · r rewind · a approve · c close · o artifact · d diff · p/P plugins · x cancel") if compact { - prompt = t.dim().Render("i chat · c close · o artifact") + prompt = t.dim().Render("i steer · c close · o artifact") } lines = append(lines, ansi.Truncate(prompt, width, "")) } @@ -899,9 +907,22 @@ func (m Model) details() string { rev := m.viewRevision() node := m.nodeID() switch panels[m.Panel] { - case "Conversation": + case "Chat": agents := rev.Config.NodeAgents(node) - lines := []string{fmt.Sprintf("Worker model: %s · Supervisor model: %s", formatModelTUI(agents.Worker.Model), formatModelTUI(agents.Supervisor.Model))} + lines := blocking(rev, node) + // On an empty stage, what to do next matters more than which model is + // configured, and a small terminal only has room for a few lines. + if len(rev.Attempts) == 0 && len(rev.Messages) == 0 { + lines = append(lines, "No stage messages yet. Press i to steer the "+m.Recipient+".") + } + lines = append(lines, fmt.Sprintf("Worker model: %s · Supervisor model: %s", formatModelTUI(agents.Worker.Model), formatModelTUI(agents.Supervisor.Model))) + if len(rev.Config.Plugins) > 0 { + var attached []string + for _, p := range rev.Config.Plugins { + attached = append(attached, p.ID+"@"+p.Version) + } + lines = append(lines, "Plugins: "+strings.Join(attached, ", ")+" (p add or replace, P remove; reopens Plan)") + } for _, a := range rev.Attempts { if a.Node == node { lines = append(lines, fmt.Sprintf("Worker attempt %d: %s", a.Number, a.State)) @@ -926,61 +947,26 @@ func (m Model) details() string { } for _, msg := range rev.Messages { if msg.Node == "" || msg.Node == node { - lines = append(lines, "To "+msg.Recipient+": "+msg.Body+"\n ["+rev.MessageStatus(msg)+"]") + lines = append(lines, "Steer "+msg.Recipient+": "+msg.Body+"\n ["+rev.MessageStatus(msg)+"]") } } - if len(lines) == 1 { - return lines[0] + "\n\nNo stage messages yet. Press i to address the " + m.Recipient + "." - } return strings.Join(lines, "\n\n") - case "Readiness": - if child := rev.ChildRuntimes[node]; child != nil { - return "Branch runtime for " + node + ":\n" + pretty(child) + "\n\nParent Plan requirements: " + strings.Join(rev.Requirements(), ", ") - } - problems := rev.ReadinessProblems(time.Now(), rev.Requirements()) - var attachments []string - for _, p := range rev.Config.Plugins { - attachments = append(attachments, fmt.Sprintf("%s@%s %s\nCapabilities: %s", p.ID, p.Version, p.Digest, strings.Join(p.Provides, ", "))) - } - pluginText := "\n\nInvocation plugins (p add/replace, P remove; reopens Plan):\n" + strings.Join(attachments, "\n") - if len(rev.DiscoveredRequirements) > 0 { - pluginText += "\n\nDiscovered during Plan:\n" - for _, requirement := range rev.DiscoveredRequirements { - pluginText += fmt.Sprintf("%s -> %s: %s\n", requirement.Capability, strings.Join(requirement.Nodes, ", "), requirement.Reason) - } - } - if rev.Recovery != nil { - problems = append(problems, fmt.Sprintf("%s: %s\nEvidence: %s\nRetry after: %s", rev.Recovery.Phase, rev.Recovery.Detail, rev.Recovery.EvidenceDigest, rev.Recovery.RetryAt.Format(time.RFC3339))) - } - if len(problems) == 0 { - return "All declared capabilities have current readiness evidence." + pluginText - } - return "Plan must resolve these before downstream execution:\n\n" + strings.Join(problems, "\n") + pluginText - case "Services": - if child := rev.ChildRuntimes[node]; child != nil { - return "Branch runtime for " + node + ":\n" + runtimeSummary(child.Runtime) - } - return runtimeSummary(rev.Runtime) - case "Graph": - order, _ := rev.Config.Workflow.Order() - lines := []string{} - for _, id := range order { - n := rev.Config.Workflow.Nodes[id] - lines = append(lines, fmt.Sprintf("%s [%s] <- %s", id, status(rev, id), strings.Join(n.Needs, ", "))) - } - return strings.Join(lines, "\n") case "Checkpoint": + needs := "" + if deps := rev.Config.Workflow.Nodes[node].Needs; len(deps) > 0 { + needs = "Runs after: " + strings.Join(deps, ", ") + "\n\n" + } if cp, ok := rev.Checkpoints[node]; ok { - return checkpointSummary(cp, m.ArtifactIndex) + return needs + checkpointSummary(cp, m.ArtifactIndex) } for _, a := range rev.Attempts { if a.Node == node && a.State == "awaiting-approval" { - return "Awaiting approval of this exact result (a to approve):\n" + pretty(a.Result) + return needs + "Awaiting approval of this exact result (a to approve):\n" + pretty(a.Result) } } - return "No accepted checkpoint for " + node + "." - case "History": - return m.history() + return needs + "No accepted checkpoint for " + node + "." + case "Decision Log": + return m.decisionLog() case "Changes": base := "revision source pins" if m.CompareRun == r.ID && m.CompareFrom != "" { diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index fd550cc..c189865 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -86,7 +86,7 @@ func TestDashboardResizeAndReadiness(t *testing.T) { } m.Width = 120 m.Height = 40 - m.Panel = 5 + m = panel(t, m, "Chat") if !strings.Contains(m.View().Content, "harness.worker: not probed") { t.Fatal("missing readiness not visible") } diff --git a/internal/tui/panels_test.go b/internal/tui/panels_test.go new file mode 100644 index 0000000..79c3f95 --- /dev/null +++ b/internal/tui/panels_test.go @@ -0,0 +1,171 @@ +package tui + +import ( + "slices" + "strings" + "testing" + "time" + + "github.com/sam-bretz/envctl/internal/workflow" +) + +// panel selects a detail view by name, so a test does not break when the +// tabs are reordered. +func panel(t *testing.T, m Model, name string) Model { + t.Helper() + i := slices.Index(panels, name) + if i < 0 { + t.Fatalf("no %q panel in %v", name, panels) + } + m.Panel = i + return m +} + +func TestTheTabsAreCutDownToWhatHelpsReviewTheWork(t *testing.T) { + want := []string{"Chat", "Checkpoint", "Changes", "Tests", "Decision Log"} + if !slices.Equal(panels, want) { + t.Fatalf("tabs: %v, want %v", panels, want) + } +} + +func TestRemovingTheReadinessTabDidNotHideWhatBlocksARun(t *testing.T) { + // This was on Readiness, two keypresses from where you land. A run that + // cannot proceed has to say so on the first screen. + m := panel(t, modelFixture(t), "Chat") + m.Width, m.Height = 120, 40 + if !strings.Contains(m.View().Content, "harness.worker: not probed") { + t.Fatalf("a readiness problem is not visible on Chat:\n%s", m.View().Content) + } + rev := m.Runs[0].Current() + rev.Recovery = &workflow.Recovery{Phase: "publication", Detail: "GitHub rejected the push"} + if !strings.Contains(m.details(), "GitHub rejected the push") { + t.Fatalf("recovery is not visible on Chat:\n%s", m.details()) + } +} + +func TestOnlyUnhealthyServicesSurfaceNowTheServicesTabIsGone(t *testing.T) { + m := panel(t, modelFixture(t), "Chat") + m.Width, m.Height = 120, 40 + rev := m.Runs[0].Current() + rev.Runtime = workflow.RuntimeState{ID: "vm", State: "running", Ready: true, PreviewURL: "http://127.0.0.1:41234/", Services: []workflow.Service{ + {Name: "web", State: "running/healthy"}, {Name: "db", State: "exited"}, + }} + details := m.details() + if !strings.Contains(details, "db is exited") { + t.Fatalf("a down service is hidden:\n%s", details) + } + if strings.Contains(details, "web is") { + t.Fatalf("a healthy service is listed as a problem:\n%s", details) + } + // The preview URL was the Services tab's other job; the header keeps it. + if !strings.Contains(m.View().Content, "Preview: http://127.0.0.1:41234/") { + t.Fatal("the preview URL is no longer shown anywhere") + } +} + +func TestACheckpointSaysWhichStagesItRunsAfter(t *testing.T) { + // Graph was removed; a stage's dependencies are the part worth keeping. + m := panel(t, modelFixture(t), "Checkpoint") + m.Node = slices.Index(orderOf(m), "plan") + if !strings.Contains(m.details(), "Runs after: task") { + t.Fatalf("dependencies not shown:\n%s", m.details()) + } +} + +func TestTheComposerSaysItSteersTheWork(t *testing.T) { + // A later "ask" mode must not be mistaken for this one. + m := modelFixture(t) + m.Width, m.Height = 120, 40 + if !strings.Contains(m.View().Content, "Steer ") { + t.Fatalf("the idle prompt does not say it steers:\n%s", m.View().Content) + } + m, _ = key(m, "i") + if !strings.Contains(m.View().Content, "changes the work") { + t.Fatalf("the composer does not say it changes the work:\n%s", m.View().Content) + } +} + +func orderOf(m Model) []string { + order, _ := m.Runs[0].Current().Config.Workflow.Order() + return order +} + +func TestTheDecisionLogExplainsWhatHappenedInOrder(t *testing.T) { + m := modelFixture(t) + r := &m.Runs[0] + rev := r.Current() + base := rev.CreatedAt + at := func(min int) time.Time { return base.Add(time.Duration(min) * time.Minute) } + rev.DiscoveredRequirements = []workflow.Requirement{{Capability: "runtime.compose", Nodes: []string{"qa"}, Reason: "QA needs the app running"}} + rev.Attempts = []workflow.Attempt{ + {ID: "a1", Node: "code", Number: 1, State: "failed", Error: correctionPrefix + "tests do not cover the error path\nmore detail", UpdatedAt: at(3)}, + {ID: "a2", Node: "code", Number: 2, State: "checkpointed", Result: &workflow.Result{Review: workflow.Review{Accepted: true, Summary: "covers every path now"}}, UpdatedAt: at(5)}, + {ID: "a3", Node: "approved-change", Number: 1, State: "checkpointed", Approval: &workflow.Approval{Actor: "local", At: at(7)}, UpdatedAt: at(6)}, + {ID: "a4", Node: "qa", Number: 1, State: "failed", Error: "guest job timed out", UpdatedAt: at(4)}, + } + rev.Messages = []workflow.Message{{ID: "m1", Node: "code", Recipient: "worker", Body: "handle the empty case", CreatedAt: at(2)}} + + log := decisions(r) + reasons := make([]string, len(log)) + for i, d := range log { + reasons[i] = d.Actor + ": " + d.Reason + } + joined := strings.Join(reasons, "\n") + + for _, want := range []string{ + "coordinator: run started", + "worker: found that qa needs runtime.compose: QA needs the app running", + "you: told the worker: handle the empty case", + // A rejection is the supervisor's, with its correction, not a failure. + "supervisor: rejected attempt 1: tests do not cover the error path", + "coordinator: attempt 1 failed: guest job timed out", + "supervisor: accepted attempt 2: covers every path now", + "you: approved attempt 1", + } { + if !strings.Contains(joined, want) { + t.Fatalf("missing %q in:\n%s", want, joined) + } + } + if strings.Contains(joined, "more detail") { + t.Fatalf("an entry is not kept to one line:\n%s", joined) + } + for i := 1; i < len(log); i++ { + if log[i].At.Before(log[i-1].At) { + t.Fatalf("out of order at %d:\n%s", i, joined) + } + } +} + +func TestADecisionLogSaysWhatARewindRerunAndWhatChanged(t *testing.T) { + log := func(r *workflow.Run) string { + joined := "" + for _, d := range decisions(r) { + joined += d.Actor + ": " + d.Reason + "\n" + } + return joined + } + + // Keeping the objective keeps task's checkpoint, so a rewind to plan + // reruns from plan. + m := modelFixture(t) + r := &m.Runs[0] + r.Current().Checkpoints["task"] = workflow.Checkpoint{ID: "cp_task", Node: "task", Result: workflow.Result{Summary: "task"}} + if _, err := r.Rewind("plan", "", nil, time.Now()); err != nil { + t.Fatal(err) + } + if got := log(r); !strings.Contains(got, "you: rewound, rerunning from plan\n") { + t.Fatalf("rewind not explained:\n%s", got) + } + + // Changing the objective reruns every stage whatever was picked, so the + // log must not claim it reran from plan. + m = modelFixture(t) + r = &m.Runs[0] + r.Current().Checkpoints["task"] = workflow.Checkpoint{ID: "cp_task", Node: "task", Result: workflow.Result{Summary: "task"}} + if _, err := r.Rewind("plan", "a sharper objective", nil, time.Now()); err != nil { + t.Fatal(err) + } + if got := log(r); !strings.Contains(got, "you: rewound, rerunning from task, changing the objective") { + t.Fatalf("objective change not explained:\n%s", got) + } +} diff --git a/internal/tui/review.go b/internal/tui/review.go index 3a979aa..598f3f0 100644 --- a/internal/tui/review.go +++ b/internal/tui/review.go @@ -169,35 +169,6 @@ func checkpointSummary(cp workflow.Checkpoint, selected int) string { } return strings.Join(lines, "\n") } -func (m Model) history() string { - r := m.current() - if r == nil { - return "No run selected." - } - lines := []string{"[ previous revision · ] next/current revision", "Viewing history leaves execution running.", ""} - for _, rev := range r.Revisions { - marker := " " - if rev.ID == m.viewRevision().ID { - marker = ">" - } - label := rev.State - if rev.ID == r.CurrentRevision { - label += " · current" - } - lines = append(lines, fmt.Sprintf("%s %s · %s\n %s\n Parent: %s · restored checkpoint: %s", marker, rev.ID, label, rev.Objective, rev.Parent, rev.FromCheckpoint)) - order, _ := rev.Config.Workflow.Order() - for _, node := range order { - if cp, ok := rev.Checkpoints[node]; ok { - kind := "accepted" - if cp.HistoricalOnly { - kind = "historical only" - } - lines = append(lines, fmt.Sprintf(" %s: %s · %s", node, cp.ID, kind)) - } - } - } - return strings.Join(lines, "\n") -} func artifactPreview(raw []byte) string { var value any if json.Unmarshal(raw, &value) == nil { diff --git a/internal/tui/services.go b/internal/tui/services.go deleted file mode 100644 index 839d0bd..0000000 --- a/internal/tui/services.go +++ /dev/null @@ -1,32 +0,0 @@ -package tui - -import ( - "fmt" - "strings" - - "github.com/sam-bretz/envctl/internal/workflow" -) - -// runtimeSummary shows the preview (host loopback) apart from service -// endpoints, which are guest-loopback addresses reachable only inside the VM. -func runtimeSummary(rt workflow.RuntimeState) string { - ready := "not ready" - if rt.Ready { - ready = "ready" - } - lines := []string{fmt.Sprintf("Runtime %s · %s · %s", rt.ID, rt.State, ready)} - if rt.PreviewURL != "" { - lines = append(lines, "Preview (this machine): "+rt.PreviewURL) - } - if len(rt.Services) > 0 { - lines = append(lines, "", "Services (endpoints are inside the VM):") - for _, s := range rt.Services { - line := fmt.Sprintf(" %-16s %-18s", s.Name, s.State) - if s.URL != "" { - line += " " + s.URL - } - lines = append(lines, strings.TrimRight(line, " ")) - } - } - return strings.Join(lines, "\n") + "\n\n" + pretty(rt) -} diff --git a/internal/tui/services_test.go b/internal/tui/services_test.go deleted file mode 100644 index c7fb258..0000000 --- a/internal/tui/services_test.go +++ /dev/null @@ -1,26 +0,0 @@ -package tui - -import ( - "strings" - "testing" - - "github.com/sam-bretz/envctl/internal/workflow" -) - -func TestServicesPanelSeparatesHostPreviewFromGuestEndpoints(t *testing.T) { - m := modelFixture(t) - m.Width, m.Height = 120, 40 - rev := m.Runs[0].Current() - rev.Runtime = workflow.RuntimeState{ID: "envctl-rev-a", State: "running", Ready: true, PreviewURL: "http://127.0.0.1:41234/", Services: []workflow.Service{{Name: "web", State: "running/healthy", URL: "http://127.0.0.1:18089"}, {Name: "db", State: "running"}}} - m.Panel = 4 - view := m.View().Content - for _, want := range []string{"Preview: http://127.0.0.1:41234/", "Preview (this machine): http://127.0.0.1:41234/", "Services (endpoints are inside the VM):", "http://127.0.0.1:18089"} { - if !strings.Contains(view, want) { - t.Fatalf("services panel missing %q:\n%s", want, view) - } - } - rev.Runtime.PreviewURL = "" - if strings.Contains(m.View().Content, "Preview (this machine)") { - t.Fatal("unreachable preview still shown") - } -} From baa2d823c8e520fa253f7394178a243638c6a13c Mon Sep 17 00:00:00 2001 From: Sam Bretz Date: Wed, 16 Sep 2026 10:22:54 -0700 Subject: [PATCH 2/3] feat: ask a stage's supervisor questions and finish the dashboard rework Chat could send instructions but never get an answer. "Is there a PR open for this?" sat queued for the next attempt and was never answered, because a message steers the work and there was no way to ask about it. A question is now its own thing, recorded on the revision beside messages rather than inside any attempt, so answering one cannot change an attempt's result, review, digest or approval. The coordinator answers it with a separate, read-only invocation of the stage's supervisor, on the stage's supervisor model: Claude gets only Read, Glob and Grep, Codex runs in its read-only sandbox, and neither can edit or run commands. A running worker keeps going, since the answer is a different job. With Claude the answer branches the supervisor's own session with --fork-session, so it knows what it reviewed without appending the question to the transcript it resumes when steered. Codex cannot branch a session and continuing one would pollute it, so it refuses to fork and starts fresh from the stage's summaries, review, checks, pull requests and steering; follow-ups see earlier answers either way. The job ID is derived from the question, like a readiness probe's, so a restart finds the job it started instead of asking twice, and an answer is recorded once. A run at its token ceiling refuses questions, answers count toward that ceiling attributed to the stage, and a question whose VM has been released fails saying to rewind. `envctl run ask` waits for and prints the answer, `run show` lists questions, and MCP can ask too. The rest of the issue: Checkpoint is gone as a tab, so the tabs are Chat, Changes, Tests and Decision Log. Chat is now one conversation in the order it happened, with the stage's artifacts for , . and o, and approvals with who and when. It also reads checkpoints a rewind carried over without their attempts, which would otherwise show nothing. What blocks a run moved to the run summary, visible from every tab, with service states shown when a preview is configured or one is down. The Decision Log now covers questions, Plan's scope, retries and stall nudges, token-ceiling stops, and what publishing did. Those coordinator decisions had no durable record: Recovery keeps only the latest cause and is cleared when a runtime is released, and the captain's log exists only with a tracker. Revisions now keep notes, appended inside the same mutation that makes each decision so each is written once, with stall nudges carried up from the backend, which alone knows when it sent one. Refs #24 Co-Authored-By: Claude Opus 5 (1M context) --- cmd/envctl/ask.go | 130 +++++++++++++++ cmd/envctl/ask_test.go | 114 +++++++++++++ cmd/envctl/workflow.go | 16 +- docs/public/agent-guide.md | 2 +- docs/src/content/docs/cli-reference.mdx | 9 ++ docs/src/content/docs/first-workflow.mdx | 23 ++- internal/agent/claude.go | 14 +- internal/agent/claude_test.go | 61 +++++++ internal/agent/codex.go | 21 ++- internal/agent/contracts.go | 18 +++ internal/daemon/api.go | 3 + internal/daemon/api_test.go | 26 +++ internal/engine/attempt.go | 45 ++++++ internal/engine/children.go | 1 + internal/engine/engine.go | 10 ++ internal/engine/notes_test.go | 79 +++++++++ internal/engine/questions.go | 134 ++++++++++++++++ internal/engine/questions_test.go | 187 ++++++++++++++++++++++ internal/localexec/attempt.go | 10 +- internal/localexec/questions.go | 179 +++++++++++++++++++++ internal/localexec/questions_test.go | 111 +++++++++++++ internal/localexec/stall.go | 23 ++- internal/mcpserver/server.go | 6 +- internal/tui/agents_test.go | 2 +- internal/tui/chat.go | 195 +++++++++++++++++++++++ internal/tui/chat_test.go | 125 +++++++++++++++ internal/tui/decisions.go | 85 +++++++--- internal/tui/model.go | 99 ++++-------- internal/tui/panels_test.go | 94 ++++++++--- internal/tui/progress_test.go | 4 +- internal/tui/review.go | 58 ------- internal/tui/review_test.go | 2 +- internal/workflow/notes.go | 35 ++++ internal/workflow/questions.go | 89 +++++++++++ internal/workflow/questions_test.go | 97 +++++++++++ internal/workflow/state.go | 43 ++--- internal/workflow/usage.go | 5 + 37 files changed, 1942 insertions(+), 213 deletions(-) create mode 100644 cmd/envctl/ask.go create mode 100644 cmd/envctl/ask_test.go create mode 100644 internal/engine/notes_test.go create mode 100644 internal/engine/questions.go create mode 100644 internal/engine/questions_test.go create mode 100644 internal/localexec/questions.go create mode 100644 internal/localexec/questions_test.go create mode 100644 internal/tui/chat.go create mode 100644 internal/tui/chat_test.go create mode 100644 internal/workflow/notes.go create mode 100644 internal/workflow/questions.go create mode 100644 internal/workflow/questions_test.go diff --git a/cmd/envctl/ask.go b/cmd/envctl/ask.go new file mode 100644 index 0000000..3b07385 --- /dev/null +++ b/cmd/envctl/ask.go @@ -0,0 +1,130 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/sam-bretz/envctl/internal/daemon" + "github.com/sam-bretz/envctl/internal/workflow" +) + +// answerWait covers the coordinator starting the answer and the answer's own +// time limit, with room for a slow start. +const answerWait = 12 * time.Minute + +// runGetter is the part of the daemon client awaitAnswer needs, so a test can +// supply the run's progress without a coordinator. +type runGetter interface { + Get(ctx context.Context, id string) (*workflow.Run, error) +} + +// awaitAnswer prints the answer to the question just asked. Without --wait it +// reports that the question was recorded and returns. +func awaitAnswer(cmd *cobra.Command, g *globals, client runGetter, asked *workflow.Run, req daemon.ActionRequest, wait bool, timeout time.Duration) error { + id := askedQuestion(asked, req) + if id == "" { + return errors.New("the question was accepted but is not on the run; run envctl run show to find it") + } + if !wait { + return printQuestion(cmd, g, asked.Current().Question(id)) + } + ctx, cancel := context.WithTimeout(cmd.Context(), timeout) + defer cancel() + tick := time.NewTicker(questionPoll) + defer tick.Stop() + for { + run, err := client.Get(ctx, asked.ID) + if err != nil { + return err + } + if q := findQuestion(run, id); q != nil && q.Finished() { + return printQuestion(cmd, g, q) + } + select { + case <-ctx.Done(): + return fmt.Errorf("no answer within %s; the question stays open, and envctl run show will show the answer when it arrives", timeout) + case <-tick.C: + } + } +} + +// questionPoll is how often awaitAnswer checks for an answer. +var questionPoll = 2 * time.Second + +// askedQuestion finds the question this request just added: the latest one on +// the stage with that text, which is the one the action appended. +func askedQuestion(run *workflow.Run, req daemon.ActionRequest) string { + rev := run.Revision(req.Revision) + if rev == nil { + rev = run.Current() + } + text := strings.TrimSpace(req.Message) + for i := len(rev.Questions) - 1; i >= 0; i-- { + if q := rev.Questions[i]; q.Node == req.Node && q.Text == text { + return q.ID + } + } + return "" +} + +func findQuestion(run *workflow.Run, id string) *workflow.Question { + for i := range run.Revisions { + if q := run.Revisions[i].Question(id); q != nil { + return q + } + } + return nil +} + +func printQuestion(cmd *cobra.Command, g *globals, q *workflow.Question) error { + if q == nil { + return errors.New("question not found") + } + if g.jsonOut { + return json.NewEncoder(cmd.OutOrStdout()).Encode(q) + } + out := cmd.OutOrStdout() + switch q.State { + case workflow.QuestionAnswered: + _, err := fmt.Fprintf(out, "%s's supervisor:\n%s\n", q.Node, q.Answer) + return err + case workflow.QuestionFailed: + return fmt.Errorf("the %s supervisor could not answer: %s", q.Node, q.Detail) + } + _, err := fmt.Fprintf(out, "Asked %s's supervisor (%s). envctl run show will show the answer.\n", q.Node, q.ID) + return err +} + +// printQuestions lists questions asked on a revision and their answers, which +// is where `envctl run ask --wait=false` says the answer will appear. +func printQuestions(out io.Writer, rev *workflow.Revision) { + if len(rev.Questions) == 0 { + return + } + fmt.Fprintln(out, " questions:") + for _, q := range rev.Questions { + fmt.Fprintf(out, " %s asked %s's supervisor: %s\n", askerName(q.Asker), q.Node, q.Text) + switch q.State { + case workflow.QuestionAnswered: + fmt.Fprintf(out, " answer: %s\n", strings.ReplaceAll(q.Answer, "\n", "\n ")) + case workflow.QuestionFailed: + fmt.Fprintf(out, " not answered: %s\n", q.Detail) + default: + fmt.Fprintf(out, " waiting for an answer (%s)\n", q.State) + } + } +} + +func askerName(asker string) string { + if asker == "" || asker == "local" { + return "you" + } + return asker +} diff --git a/cmd/envctl/ask_test.go b/cmd/envctl/ask_test.go new file mode 100644 index 0000000..add152d --- /dev/null +++ b/cmd/envctl/ask_test.go @@ -0,0 +1,114 @@ +package main + +import ( + "bytes" + "context" + "strings" + "testing" + "time" + + "github.com/spf13/cobra" + + "github.com/sam-bretz/envctl/internal/daemon" + "github.com/sam-bretz/envctl/internal/workflow" +) + +// progress hands back a run whose question moves through the given states, +// one per poll. +type progress struct { + run *workflow.Run + states []workflow.Question + polls int +} + +func (p *progress) Get(context.Context, string) (*workflow.Run, error) { + i := min(p.polls, len(p.states)-1) + p.polls++ + next := *workflow.Clone(p.run) + next.Current().Questions = []workflow.Question{p.states[i]} + return &next, nil +} + +func askedRun(t *testing.T) (*workflow.Run, daemon.ActionRequest) { + t.Helper() + c, err := workflow.Parse([]byte("version: 2\nproject: demo\nrepositories: [{id: app, url: /source}]\nworkflow: {template: feature}\n")) + if err != nil { + t.Fatal(err) + } + r, err := workflow.NewRun("demo", "ship it", "dev", c, time.Now()) + if err != nil { + t.Fatal(err) + } + r.Current().Questions = []workflow.Question{{ID: "question_1", Node: "code", Text: "is there a PR?", State: workflow.QuestionPending}} + return r, daemon.ActionRequest{Revision: r.CurrentRevision, Node: "code", Message: " is there a PR? "} +} + +func runAsk(t *testing.T, client runGetter, r *workflow.Run, req daemon.ActionRequest, wait bool, timeout time.Duration) (string, error) { + t.Helper() + old := questionPoll + questionPoll = time.Millisecond + t.Cleanup(func() { questionPoll = old }) + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + var out bytes.Buffer + cmd.SetOut(&out) + err := awaitAnswer(cmd, &globals{}, client, r, req, wait, timeout) + return out.String(), err +} + +func TestRunAskWaitsForTheAnswerAndPrintsIt(t *testing.T) { + r, req := askedRun(t) + p := &progress{run: r, states: []workflow.Question{ + {ID: "question_1", Node: "code", State: workflow.QuestionPending}, + {ID: "question_1", Node: "code", State: workflow.QuestionAnswering}, + {ID: "question_1", Node: "code", State: workflow.QuestionAnswered, Answer: "No PR yet; approved-change opens it."}, + }} + out, err := runAsk(t, p, r, req, true, time.Second) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "No PR yet; approved-change opens it.") { + t.Fatalf("answer not printed:\n%s", out) + } +} + +func TestRunAskReportsAnUnanswerableQuestionAsAnError(t *testing.T) { + r, req := askedRun(t) + p := &progress{run: r, states: []workflow.Question{{ID: "question_1", Node: "code", State: workflow.QuestionFailed, Detail: "the stage's VM is no longer running"}}} + if _, err := runAsk(t, p, r, req, true, time.Second); err == nil || !strings.Contains(err.Error(), "VM is no longer running") { + t.Fatalf("a failed answer was not reported: %v", err) + } +} + +func TestRunAskGivesUpWaitingWithoutLosingTheQuestion(t *testing.T) { + r, req := askedRun(t) + p := &progress{run: r, states: []workflow.Question{{ID: "question_1", Node: "code", State: workflow.QuestionAnswering}}} + _, err := runAsk(t, p, r, req, true, 20*time.Millisecond) + if err == nil || !strings.Contains(err.Error(), "stays open") { + t.Fatalf("a timeout did not say the question is still open: %v", err) + } +} + +func TestRunAskWithoutWaitingSaysWhereTheAnswerWillBe(t *testing.T) { + r, req := askedRun(t) + out, err := runAsk(t, &progress{run: r}, r, req, false, time.Second) + if err != nil || !strings.Contains(out, "envctl run show") { + t.Fatalf("no-wait did not point to where the answer appears: %v\n%s", err, out) + } +} + +func TestRunShowListsQuestionsAndTheirAnswers(t *testing.T) { + r, _ := askedRun(t) + r.Current().Questions = []workflow.Question{ + {Node: "code", Asker: "sam", Text: "is there a PR?", State: workflow.QuestionAnswered, Answer: "not yet"}, + {Node: "qa", Text: "why?", State: workflow.QuestionFailed, Detail: "VM released"}, + {Node: "plan", Text: "scope?", State: workflow.QuestionAnswering}, + } + var out bytes.Buffer + printQuestions(&out, r.Current()) + for _, want := range []string{"sam asked code's supervisor: is there a PR?", "answer: not yet", "you asked qa's supervisor: why?", "not answered: VM released", "waiting for an answer (answering)"} { + if !strings.Contains(out.String(), want) { + t.Fatalf("missing %q:\n%s", want, out.String()) + } + } +} diff --git a/cmd/envctl/workflow.go b/cmd/envctl/workflow.go index 8913a9c..038c3dc 100644 --- a/cmd/envctl/workflow.go +++ b/cmd/envctl/workflow.go @@ -299,7 +299,7 @@ func runCmd(g *globals) *cobra.Command { return printRun(cmd, g, run) }}) } - for _, kind := range []string{"message", "rewind", "cancel", "close", "approve", "priority"} { + for _, kind := range []string{"message", "ask", "rewind", "cancel", "close", "approve", "priority"} { c.AddCommand(runActionCmd(g, kind)) } plugins := &cobra.Command{Use: "plugin", Short: "Change invocation plugins and reopen Plan in a new revision"} @@ -384,6 +384,7 @@ func runActionCmd(g *globals, action string) *cobra.Command { req := daemon.ActionRequest{Action: action} var pluginFile string var configFile string + wait, waitFor := true, answerWait c := &cobra.Command{Use: action + " ", Short: action + " a workflow", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { client, err := connect(cmd.Context(), g) if err != nil { @@ -438,6 +439,9 @@ func runActionCmd(g *globals, action string) *cobra.Command { if err != nil { return err } + if action == "ask" { + return awaitAnswer(cmd, g, client, result, req, wait, waitFor) + } return printRun(cmd, g, result) }} c.Flags().StringVar(&req.OperationID, "operation-id", "", "idempotency key") @@ -455,6 +459,15 @@ func runActionCmd(g *globals, action string) *cobra.Command { _ = c.MarkFlagRequired("text") c.Flags().StringVar(&req.Recipient, "to", "supervisor", "supervisor or worker") c.Flags().StringVar(&req.Node, "node", "", "stage to address") + case "ask": + c.Short = "ask a stage's supervisor a question, without changing the work" + c.Flags().StringVar(&req.Node, "node", "", "stage whose supervisor answers") + _ = c.MarkFlagRequired("node") + c.Flags().StringVar(&req.Message, "text", "", "the question") + _ = c.MarkFlagRequired("text") + c.Flags().StringVar(&req.Actor, "actor", os.Getenv("USER"), "who is asking") + c.Flags().BoolVar(&wait, "wait", true, "wait for the answer and print it") + c.Flags().DurationVar(&waitFor, "timeout", answerWait, "how long to wait for an answer") case "rewind": c.Flags().StringVar(&req.Node, "to", "", "stage to revisit") _ = c.MarkFlagRequired("to") @@ -488,6 +501,7 @@ func printRun(cmd *cobra.Command, g *globals, r *workflow.Run) error { } fmt.Fprintf(out, " VM: %s\n", vm) printAttention(out, r) + printQuestions(out, rev) printRuntime(out, "", rev.Runtime) for node, child := range rev.ChildRuntimes { if child != nil && (child.Runtime.PreviewURL != "" || len(child.Runtime.Services) > 0) { diff --git a/docs/public/agent-guide.md b/docs/public/agent-guide.md index 7cb89cf..ebad42a 100644 --- a/docs/public/agent-guide.md +++ b/docs/public/agent-guide.md @@ -18,7 +18,7 @@ Plan must verify downstream requirements before dependent execution. Attach a lo In Bubble Tea, `[` / `]` browse revisions, `,` / `.` select artifacts, `o` opens them, and `p` attaches a plugin reference file. `T` opens a theme picker; `envctl theme list` and `envctl theme set ` do the same from a shell, saving to `~/.config/envctl/config.yaml`. Browsing and detaching do not cancel execution. Historical views are read-only; rewind creates a new revision. -Running attempts expose bounded, redacted `progress` (phase, current check, recent agent activity) in `envctl run show ` (`--json` for the full object), the Bubble Tea Conversation panel, and MCP run reads. Steer with `envctl run message --node --to worker|supervisor --text ...` or `i` in Bubble Tea. A message reaches a running agent by interrupting its guest job and resuming the same harness session with the message; each message's status (included at start, delivered live, pending, or queued for the next attempt) is shown next to it. Live delivery waits until the harness reports its session, is limited to 8 resumes per role per attempt, and does not apply once an attempt's review has finished. Supervisors see every message for their stage and should reject work that ignores worker steering. Each run shows its token usage in `run show` and the dashboard and stops its agents at `limits.run_tokens` (default 20M counted tokens; cache reads count at `limits.cache_read_weight`, default 10%) or `limits.run_cost_usd`; a run marked needs-attention for its token ceiling needs the user to raise the limit through `run rewind --config` or cancel it. Workers write each output document to a Markdown file in their attempt's `outputs/` directory, and the coordinator reads it from there. +Running attempts expose bounded, redacted `progress` (phase, current check, recent agent activity) in `envctl run show ` (`--json` for the full object), the Bubble Tea Chat panel, and MCP run reads. Steer with `envctl run message --node --to worker|supervisor --text ...` or `i` in Bubble Tea. To find something out without changing the work, ask instead: `envctl run ask --node --text ...`, `?` in Bubble Tea, or MCP `envctl_action` action `ask`. The stage's supervisor answers through a separate read-only invocation that cannot edit files or run commands, so an attempt's result, review, digest and approval are unchanged and a running worker is not interrupted; the answer appears under the revision's `questions`. Asking needs the stage to have started and the run's VM to still be running, counts toward the token ceiling, and is refused once the run is at that ceiling. A message reaches a running agent by interrupting its guest job and resuming the same harness session with the message; each message's status (included at start, delivered live, pending, or queued for the next attempt) is shown next to it. Live delivery waits until the harness reports its session, is limited to 8 resumes per role per attempt, and does not apply once an attempt's review has finished. Supervisors see every message for their stage and should reject work that ignores worker steering. Each run shows its token usage in `run show` and the dashboard and stops its agents at `limits.run_tokens` (default 20M counted tokens; cache reads count at `limits.cache_read_weight`, default 10%) or `limits.run_cost_usd`; a run marked needs-attention for its token ceiling needs the user to raise the limit through `run rewind --config` or cancel it. Workers write each output document to a Markdown file in their attempt's `outputs/` directory, and the coordinator reads it from there. An agent silent for its stall window (`limits.stall_seconds`, default 600, overridable per node) is nudged once to report status; a still-silent agent, or a third stall, fails the attempt with evidence and the retry budget applies. A running command extends the window once. If a legitimate step, such as a long test suite, runs quietly for longer than twice the window, raise `stall_seconds` for that node rather than treating the failure as flaky. diff --git a/docs/src/content/docs/cli-reference.mdx b/docs/src/content/docs/cli-reference.mdx index 6c0f3c9..6522b75 100644 --- a/docs/src/content/docs/cli-reference.mdx +++ b/docs/src/content/docs/cli-reference.mdx @@ -66,6 +66,15 @@ Neither harness accepts input mid-run, so live delivery interrupts and resumes. Worker-directed messages reach the running worker. The supervisor's prompt includes every message for its stage, with an instruction to reject work that ignores steering addressed to the worker. Messages that arrive while the supervisor is reviewing interrupt and resume the supervisor the same way. Each role allows 8 live resumes per attempt. After that, progress reports the limit, and further messages reach the supervisor review or the next attempt. Messages that arrive after an attempt's review has finished apply to that stage's next attempt, or to a rewind. + +### Asking a stage's supervisor + +```bash +envctl run ask --node --text "" # waits and prints the answer +envctl run ask --node --text "" --wait=false +``` + +`run ask` puts a question to the stage's supervisor and, by default, waits up to 12 minutes (`--timeout`) for the answer. Unlike `run message`, it never steers: the answer comes from a separate read-only invocation that can read the stage's worktree but cannot edit it or run commands, so the attempt's result, review, digest and approval are unchanged, and a running worker is not interrupted. Claude answers from a branch of the supervisor's session (`--fork-session`), leaving the original transcript as it was; Codex starts a fresh read-only session from the stage's facts. A question needs the stage to have started and the run's VM to still be running, and a run at its token ceiling refuses it; answers count toward that ceiling. `run show` lists questions and answers, and MCP clients ask with `envctl_action` action `ask` and read answers from `envctl_run`. ## Pull requests ```bash diff --git a/docs/src/content/docs/first-workflow.mdx b/docs/src/content/docs/first-workflow.mdx index c3962ec..b869965 100644 --- a/docs/src/content/docs/first-workflow.mdx +++ b/docs/src/content/docs/first-workflow.mdx @@ -111,8 +111,9 @@ The dashboard shows the stage strip, your in-flight runs, and a focused panel: | --- | --- | | `↑` `↓` | Move between runs | | `←` `→` | Move between stages | -| `tab` | Cycle panels: Chat, Checkpoint, Changes, Tests, Decision Log | -| `i` | Steer the agent (this changes the work); `s` switches between worker and supervisor | +| `tab` | Cycle panels: Chat, Changes, Tests, Decision Log | +| `i` | Steer the agent, which changes the work; `s` switches between worker and supervisor | +| `?` | Ask the stage's supervisor a question, which does not; `tab` in the composer switches between the two | | `d` | Diff the stage; `b` sets a comparison base, `B` clears it | | `o` | Open the selected artifact in your browser; `,` and `.` select | | `[` `]` | Browse revision history | @@ -122,7 +123,9 @@ The dashboard shows the stage strip, your in-flight runs, and a focused panel: | `T` | Pick a color theme, with live preview | | `q` | Detach; execution continues | -**Chat** leads with anything blocking the run — readiness problems Plan must resolve, recovery in progress, or a service down in the VM — then the stage's attempts, summaries and steering messages. **Checkpoint** shows the accepted result and which stages it runs after. **Decision Log** reads like a record of the run: when each thing was decided, by whom (worker, supervisor, you, or the coordinator), and why — Plan's findings, supervisor accepts and rejections with their corrections, your steering and approvals, and rewinds with what they reran and whether the objective or configuration changed. +Anything blocking the run sits in the run summary above the tabs, visible from every tab: readiness problems Plan must resolve, recovery in progress, and service states when a preview is configured or a service is down. + +**Chat** is the selected stage's conversation in the order it happened: attempts starting, the worker's summaries, the supervisor's accepts and rejections, your steering, your questions and the supervisor's answers, and approvals with who approved and when. It also lists the stage's artifacts for `,` `.` and `o`. **Decision Log** reads like a record of the whole run: when each thing was decided, by whom (worker, supervisor, you, or the coordinator), and why. That includes Plan's scope and findings, supervisor accepts and rejections with their corrections, steering and questions and where they went, approvals, rewinds with what they reran, retries and stall nudges, stops at the token ceiling, and what publishing did, including the pull request or why it failed. Closing the dashboard never stops the run. For scripts, `envctl run show --json` has the same information. @@ -163,6 +166,20 @@ envctl run message --node code --to worker --text "Use the existing CSV wr This reaches the running agent within seconds. envctl interrupts its turn and resumes the same session with your message, keeping the work it has already done. The supervisor also sees your messages and should reject work that ignores them. Each message shows its status: delivered live, or queued for the next attempt. +### Ask instead of steering + +A message changes the work. To find something out without changing it, ask the stage's supervisor: + +```bash +envctl run ask --node code --text "Is there a PR open for this yet?" +``` + +In the dashboard, press `?`. The answer comes back in the stage's Chat, and `envctl run ask` waits for it and prints it (`--wait=false` to return at once; `envctl run show` lists questions and answers). + +An answer never touches the work. It comes from a separate, read-only invocation of the stage's supervisor, using the stage's supervisor model: it can read the worktree but cannot edit files or run commands, and a running worker keeps going undisturbed. With Claude the answer branches the supervisor's own session, so it knows what it reviewed without adding the question to the transcript it resumes later; Codex, which cannot branch a session, starts fresh from the stage's summaries, review, checks and pull requests. Follow-up questions see earlier answers. + +You can ask while a stage is running, while it waits for approval, and after it is accepted, for as long as the run keeps its VM. After `envctl run close`, or on a superseded revision, a question fails and says to rewind. Answers count toward the run's token ceiling, and a run already at its ceiling refuses new questions. + ## 6. Review a checkpoint Every stage ends in an immutable checkpoint: the agent's output, the source commits, check results, and an independent supervisor review. diff --git a/internal/agent/claude.go b/internal/agent/claude.go index a766dac..97d03b8 100644 --- a/internal/agent/claude.go +++ b/internal/agent/claude.go @@ -105,6 +105,15 @@ chmod 755 "$destination" "$destination/claude" `, "envctl-claude-install", ClaudeVersion, ClaudeArmSHA, ClaudeAMD64SHA, claudeDownload}}) } +// claudeTools drops every tool that can change files or run commands from a +// read-only invocation. Bash goes too: it can write as easily as it reads. +func claudeTools(i Invocation) string { + if i.ReadOnly { + return "Read,Glob,Grep" + } + return "Bash,Read,Edit,Write,Glob,Grep" +} + func claudeHome(role string) string { return "/work/envctl/harness-homes/claude-" + role } func (c Claude) Request(i Invocation, credential Credential) (guestjob.Request, error) { @@ -112,12 +121,15 @@ func (c Claude) Request(i Invocation, credential Credential) (guestjob.Request, return guestjob.Request{}, err } dir := invocationDir(i.ID) - args := []string{"python3", "-c", claudeProcess, dir + "/schema.json", dir + "/result.json", ClaudeBinary, "--print", "--verbose", "--output-format", "stream-json", "--safe-mode", "--setting-sources", "", "--strict-mcp-config", "--mcp-config", `{"mcpServers":{}}`, "--dangerously-skip-permissions", "--tools", "Bash,Read,Edit,Write,Glob,Grep"} + args := []string{"python3", "-c", claudeProcess, dir + "/schema.json", dir + "/result.json", ClaudeBinary, "--print", "--verbose", "--output-format", "stream-json", "--safe-mode", "--setting-sources", "", "--strict-mcp-config", "--mcp-config", `{"mcpServers":{}}`, "--dangerously-skip-permissions", "--tools", claudeTools(i)} if i.Model != "" { args = append(args, "--model", i.Model) } if i.Session != "" { args = append(args, "--resume", i.Session) + if i.Fork { + args = append(args, "--fork-session") + } } env := map[string]string{"CLAUDE_CONFIG_DIR": claudeHome(i.Role), "DISABLE_AUTOUPDATER": "1", "PYTHONDONTWRITEBYTECODE": "1"} if credential.APIKey != "" { diff --git a/internal/agent/claude_test.go b/internal/agent/claude_test.go index 45397ea..633c466 100644 --- a/internal/agent/claude_test.go +++ b/internal/agent/claude_test.go @@ -85,3 +85,64 @@ func TestInvocationEnvironmentIsScopedToEnvctlValues(t *testing.T) { } } } + +func TestAQuestionCannotChangeTheWorkItAsksAbout(t *testing.T) { + c := Claude{} + base := Invocation{ID: "question_1", Role: "supervisor", Directory: "/work/envctl/repos/app", Prompt: "is there a PR?", Schema: AnswerSchema(), TimeoutSeconds: 60} + + readOnly := base + readOnly.ReadOnly = true + req, err := c.Request(readOnly, Credential{APIKey: "k"}) + if err != nil { + t.Fatal(err) + } + tools := argAfter(req.Args, "--tools") + for _, writer := range []string{"Bash", "Edit", "Write"} { + if strings.Contains(tools, writer) { + t.Fatalf("a read-only invocation can still use %s: %q", writer, tools) + } + } + + // Forking leaves the supervisor's own transcript untouched; it is the one + // the supervisor resumes when steered. + forked := readOnly + forked.Session, forked.Fork = "01a08f59-7073-7a83-b959-867ca896ce48", true + req, err = c.Request(forked, Credential{APIKey: "k"}) + if err != nil { + t.Fatal(err) + } + if argAfter(req.Args, "--resume") != forked.Session || !slices.Contains(req.Args, "--fork-session") { + t.Fatalf("session not forked: %v", req.Args) + } + + // An ordinary worker keeps its tools and continues its own session. + req, _ = c.Request(base, Credential{APIKey: "k"}) + if !strings.Contains(argAfter(req.Args, "--tools"), "Edit") || slices.Contains(req.Args, "--fork-session") { + t.Fatalf("a normal invocation changed: %v", req.Args) + } +} + +func TestCodexRefusesToForkRatherThanAppendToASession(t *testing.T) { + i := Invocation{ID: "question_1", Role: "supervisor", Directory: "/work/envctl/repos/app", Prompt: "p", Schema: AnswerSchema(), TimeoutSeconds: 60, + Session: "01a08f59-7073-7a83-b959-867ca896ce48", Fork: true} + if _, err := (Codex{}).Request(i, Credential{APIKey: "k"}); err == nil { + t.Fatal("codex continued a session it cannot fork") + } + i.Session, i.Fork, i.ReadOnly = "", false, true + req, err := (Codex{}).Request(i, Credential{APIKey: "k"}) + if err != nil { + t.Fatal(err) + } + if slices.Contains(req.Args, "--dangerously-bypass-approvals-and-sandbox") || argAfter(req.Args, "--sandbox") != "read-only" { + t.Fatalf("a read-only codex invocation is not sandboxed: %v", req.Args) + } +} + +func argAfter(args []string, flag string) string { + for i := 0; i+1 < len(args); i++ { + if args[i] == flag { + return args[i+1] + } + } + return "" +} diff --git a/internal/agent/codex.go b/internal/agent/codex.go index 98211fd..73880d5 100644 --- a/internal/agent/codex.go +++ b/internal/agent/codex.go @@ -109,6 +109,14 @@ type Invocation struct { Model string Session string TimeoutSeconds int + // ReadOnly restricts the agent to reading: it can inspect the worktree + // but not edit it or run commands. Answering a question uses it so the + // answer cannot change the work being asked about. + ReadOnly bool + // Fork resumes Session into a new session instead of continuing it, so + // the original transcript, which the supervisor itself resumes when + // steered, is left exactly as it was. Only Claude can fork. + Fork bool // Env carries coordinator-derived ENVCTL_* values, such as guest service // endpoints. It cannot override harness homes or credentials. Env map[string]string `json:",omitempty"` @@ -210,12 +218,23 @@ func (c Codex) Request(i Invocation, credential Credential) (guestjob.Request, e if err := validateInvocation(i); err != nil { return guestjob.Request{}, err } + // Codex has no way to branch a session, and continuing one would append + // to the transcript the agent resumes later. Refuse rather than pollute it. + if i.Fork { + return guestjob.Request{}, errors.New("codex cannot fork a session; start a fresh read-only invocation instead") + } dir := invocationDir(i.ID) args := []string{CodexBinary, "exec"} if i.Session != "" { args = append(args, "resume") } - args = append(args, "--json", "--ignore-user-config", "--ignore-rules", "--skip-git-repo-check", "--dangerously-bypass-approvals-and-sandbox", "--output-schema", dir+"/schema.json", "--output-last-message", dir+"/result.json") + sandbox := []string{"--dangerously-bypass-approvals-and-sandbox"} + if i.ReadOnly { + sandbox = []string{"--sandbox", "read-only"} + } + args = append(args, "--json", "--ignore-user-config", "--ignore-rules", "--skip-git-repo-check") + args = append(args, sandbox...) + args = append(args, "--output-schema", dir+"/schema.json", "--output-last-message", dir+"/result.json") if i.Model != "" { args = append(args, "--model", i.Model) } diff --git a/internal/agent/contracts.go b/internal/agent/contracts.go index cfe9172..b5a61c4 100644 --- a/internal/agent/contracts.go +++ b/internal/agent/contracts.go @@ -113,6 +113,24 @@ func ParseAssessment(raw []byte) (Assessment, error) { return result, err } +// AnswerSchema is the contract for answering a person's question. +func AnswerSchema() map[string]any { + return map[string]any{"type": "object", "additionalProperties": false, "required": []string{"answer"}, "properties": map[string]any{ + "answer": map[string]any{"type": "string", "minLength": 1}, + }} +} + +// ParseAnswer reads an answer to a question. +func ParseAnswer(raw []byte) (string, error) { + var out struct { + Answer string `json:"answer"` + } + if err := decodeContract(raw, AnswerSchema(), &out); err != nil { + return "", err + } + return strings.TrimSpace(out.Answer), nil +} + // StrictSchema returns schema with every object property required, for // harnesses whose structured output mode requires that. Existing required // entries keep their order, so an already-strict schema is unchanged. diff --git a/internal/daemon/api.go b/internal/daemon/api.go index ce8807a..7fa135c 100644 --- a/internal/daemon/api.go +++ b/internal/daemon/api.go @@ -183,6 +183,9 @@ func (s *Server) action(w http.ResponseWriter, r *http.Request) { switch req.Action { case "message": return run.Message(req.Revision, req.Node, req.Recipient, req.Message, now) + case "ask": + _, err := run.Ask(req.Revision, req.Node, req.Message, req.Actor, now) + return err case "priority": run.Priority = req.Priority return nil diff --git a/internal/daemon/api_test.go b/internal/daemon/api_test.go index de3738d..2f24117 100644 --- a/internal/daemon/api_test.go +++ b/internal/daemon/api_test.go @@ -204,3 +204,29 @@ func TestServeSingleOwnerAndRestart(t *testing.T) { t.Fatal("coordinator failed to stop") } } + +func TestAskingRecordsAQuestionWithoutTouchingTheWork(t *testing.T) { + c, store := testAPI(t) + r := create(t, c) + updated, err := store.Mutate(context.Background(), r.ID, r.Version, "start", "fixture.attempt", nil, func(run *workflow.Run) error { + run.Current().Attempts = []workflow.Attempt{{ID: "attempt_1", Node: "code", Number: 1, State: "running"}} + return nil + }) + if err != nil { + t.Fatal(err) + } + asked, err := c.Action(context.Background(), r.ID, ActionRequest{OperationID: "ask-1", ExpectedVersion: updated.Version, Revision: updated.CurrentRevision, Action: "ask", Node: "code", Message: "is there a PR open for this?", Actor: "sam"}) + if err != nil { + t.Fatal(err) + } + qs := asked.Current().Questions + if len(qs) != 1 || qs[0].Attempt != "attempt_1" || qs[0].Asker != "sam" || qs[0].State != workflow.QuestionPending { + t.Fatalf("question not recorded: %+v", qs) + } + if asked.Current().Attempts[0].State != "running" || len(asked.Current().Messages) != 0 { + t.Fatal("asking steered or changed the running attempt") + } + if _, err = c.Action(context.Background(), r.ID, ActionRequest{OperationID: "ask-2", ExpectedVersion: asked.Version, Revision: asked.CurrentRevision, Action: "ask", Node: "qa", Message: "why?"}); err == nil { + t.Fatal("accepted a question for a stage that has not started") + } +} diff --git a/internal/engine/attempt.go b/internal/engine/attempt.go index ff87f16..2d0ccde 100644 --- a/internal/engine/attempt.go +++ b/internal/engine/attempt.go @@ -4,6 +4,8 @@ import ( "context" "errors" "slices" + "sort" + "strings" "time" "github.com/sam-bretz/envctl/internal/workflow" @@ -131,6 +133,21 @@ func (e *Engine) reconcileAttempt(ctx context.Context, run *workflow.Run, rev *w }) return true, err } + if fresh := newNotes(rev, observation.Notes); len(fresh) > 0 { + _, err = e.update(ctx, id, revision, "revision.notes", func(_ *workflow.Run, v *workflow.Revision) error { + added := false + for _, n := range fresh { + added = v.AddNote(n) || added + } + if !added { + return workflow.ErrConflict + } + return nil + }) + if err != nil && !errors.Is(err, workflow.ErrConflict) { + return true, err + } + } if observation.Session != "" && a.Session != observation.Session { _, err = e.update(ctx, id, revision, "attempt.session", func(_ *workflow.Run, v *workflow.Revision) error { current := v.Attempt(a.ID) @@ -201,6 +218,7 @@ func (e *Engine) reconcileAttempt(ctx context.Context, run *workflow.Run, rev *w // logged as its own entry rather than buried in the stage's. if len(prs) > 0 { r.AppendTrackerLog(v.Config.Tracker, workflow.TrackerKindPublished, v.ID, current.Node, current.ID, "", e.now()) + v.AddNote(workflow.Note{At: e.now(), Kind: workflow.NotePublished, Node: current.Node, Detail: prLinks(prs)}) } return nil }) @@ -226,3 +244,30 @@ func (e *Engine) reconcileAttempt(ctx context.Context, run *workflow.Run, rev *w } return false, nil } + +// prLinks lists pull requests in a stable order, one repository each. +func prLinks(prs map[string]string) string { + keys := make([]string, 0, len(prs)) + for k := range prs { + keys = append(keys, k) + } + sort.Strings(keys) + links := make([]string, 0, len(keys)) + for _, k := range keys { + links = append(links, k+": "+prs[k]) + } + return strings.Join(links, ", ") +} + +// newNotes filters reported notes down to those the revision has not kept, +// so a note reported on every poll causes one write. +func newNotes(rev *workflow.Revision, reported []workflow.Note) []workflow.Note { + var fresh []workflow.Note + for _, n := range reported { + probe := workflow.Revision{Notes: rev.Notes} + if probe.AddNote(n) { + fresh = append(fresh, n) + } + } + return fresh +} diff --git a/internal/engine/children.go b/internal/engine/children.go index 899c2ea..f3a426b 100644 --- a/internal/engine/children.go +++ b/internal/engine/children.go @@ -56,6 +56,7 @@ func (e *Engine) recoverAssignment(ctx context.Context, a Assignment, phase stri child.Recovery = &workflow.Recovery{Phase: phase, Detail: cause.Error(), EvidenceDigest: artifact.Digest, Failures: failures, RetryAt: e.now().Add(backoff(failures))} if phase == "publication" && failures == 1 { run.AppendTrackerLog(v.Config.Tracker, workflow.TrackerKindNeedsAttention, v.ID, "", "", cause.Error(), e.now()) + v.AddNote(workflow.Note{At: e.now(), Kind: workflow.NotePublishFailed, Node: a.Child, Detail: cause.Error()}) } return nil }) diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 1e82912..e4bd41a 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -32,6 +32,10 @@ type Observation struct { Result *workflow.Result Detail string // already redacted by the backend Session string // explicit harness session identity for durable continuation + // Notes are decisions the backend made that the run should keep, such as + // nudging a stalled agent. They are reported cumulatively; the engine + // records each once. + Notes []workflow.Note // Progress and Delivered are display state for running attempts. Delivered // lists user messages already submitted to an agent invocation. Progress *workflow.Progress @@ -146,6 +150,9 @@ func (e *Engine) Tick(ctx context.Context) error { e.launch(ctx, run.ID+"/"+rev.ID+"/child/"+node, func() error { return e.ReconcileChild(ctx, run.ID, rev.ID, node) }) } e.schedulePreviews(ctx, run, rev) + // Before the completed-revision skip below: a finished stage can + // still be asked about while its VM is kept. + e.scheduleQuestions(ctx, run, rev) if (rev.State == "completed" && run.ClosedAt.IsZero()) || rev.State == "needs-attention" { continue } @@ -400,6 +407,7 @@ func (e *Engine) Reconcile(ctx context.Context, id, revision string) error { return workflow.ErrConflict } v.State = "needs-attention" + v.AddNote(workflow.Note{At: e.now(), Kind: workflow.NoteAttemptBudget, Node: exhausted, Detail: reason}) run.AppendTrackerLog(v.Config.Tracker, workflow.TrackerKindNeedsAttention, v.ID, "", "", reason, e.now()) return nil }) @@ -416,6 +424,7 @@ func (e *Engine) Reconcile(ctx context.Context, id, revision string) error { return workflow.ErrConflict } v.State = "needs-attention" + v.AddNote(workflow.Note{At: e.now(), Kind: workflow.NoteTokenCeiling, Detail: budgetReason}) run.AppendTrackerLog(v.Config.Tracker, workflow.TrackerKindNeedsAttention, v.ID, "", "", reason, e.now()) return nil }) @@ -577,6 +586,7 @@ func (e *Engine) recover(ctx context.Context, id, revision, phase string, cause // person" moments the captain's log is for. if phase == "publication" && failures == 1 { run.AppendTrackerLog(v.Config.Tracker, workflow.TrackerKindNeedsAttention, v.ID, "", "", cause.Error(), e.now()) + v.AddNote(workflow.Note{At: e.now(), Kind: workflow.NotePublishFailed, Detail: cause.Error()}) } return nil }) diff --git a/internal/engine/notes_test.go b/internal/engine/notes_test.go new file mode 100644 index 0000000..3c7f4ff --- /dev/null +++ b/internal/engine/notes_test.go @@ -0,0 +1,79 @@ +package engine + +import ( + "strings" + "testing" + + "github.com/sam-bretz/envctl/internal/workflow" +) + +func notesOfKind(v *workflow.Revision, kind string) []workflow.Note { + var out []workflow.Note + for _, n := range v.Notes { + if n.Kind == kind { + out = append(out, n) + } + } + return out +} + +func TestPublishingRecordsThePullRequestItOpened(t *testing.T) { + h := setup(t) + h.until(waitingForApproval) + r := h.run() + v := r.Current() + a := v.Attempts[len(v.Attempts)-1] + h.mutate(func(r *workflow.Run) error { return r.Approve(h.rev, a.ID, "developer", a.Result.WorkDigest(), h.now) }) + h.until(func(v *workflow.Revision) bool { return v.State == "completed" }) + for i := 0; i < 3; i++ { + h.step() + } + notes := notesOfKind(h.run().Current(), workflow.NotePublished) + if len(notes) != 1 || !strings.Contains(notes[0].Detail, "https://example.test/pr/1") { + t.Fatalf("publication not recorded exactly once with its URL: %+v", notes) + } +} + +func TestStoppingAtTheTokenCeilingIsRecordedOnce(t *testing.T) { + h := setup(t) + h.until(func(v *workflow.Revision) bool { return len(v.Checkpoints) >= 1 }) + h.mutate(func(r *workflow.Run) error { + r.Current().Config.Limits.RunTokens = 1 + r.Current().Attempts[0].Usage = &workflow.Usage{Input: 1000} + return nil + }) + h.until(func(v *workflow.Revision) bool { return v.State == "needs-attention" }) + // The engine reaches this decision on every tick while the run is stuck. + for i := 0; i < 5; i++ { + h.step() + } + notes := notesOfKind(h.run().Current(), workflow.NoteTokenCeiling) + if len(notes) != 1 || !strings.Contains(notes[0].Detail, "token ceiling") { + t.Fatalf("ceiling stop not recorded exactly once: %+v", notes) + } +} + +func TestANudgeReportedOnEveryPollIsRecordedOnce(t *testing.T) { + h := setup(t) + h.backend.startRunning = true + h.until(func(v *workflow.Revision) bool { + for _, a := range v.Attempts { + if a.State == "running" { + return true + } + } + return false + }) + v := h.run().Current() + running := v.Attempts[len(v.Attempts)-1] + nudge := workflow.Note{At: h.now, Kind: workflow.NoteStallNudge, Node: running.Node, Detail: "nudged the worker after a silence (nudge 1 of 2)"} + obs := h.backend.observations[running.ID] + obs.Notes = []workflow.Note{nudge} + h.backend.observations[running.ID] = obs + for i := 0; i < 5; i++ { + h.step() + } + if got := notesOfKind(h.run().Current(), workflow.NoteStallNudge); len(got) != 1 { + t.Fatalf("a nudge reported on every poll was recorded %d times: %+v", len(got), got) + } +} diff --git a/internal/engine/questions.go b/internal/engine/questions.go new file mode 100644 index 0000000..13382e5 --- /dev/null +++ b/internal/engine/questions.go @@ -0,0 +1,134 @@ +package engine + +import ( + "context" + "errors" + + "github.com/sam-bretz/envctl/internal/workflow" +) + +// QuestionBackend answers a person's question about a stage. It is optional, +// like PreviewBackend: a backend without it reports questions as unanswerable +// rather than blocking the run. +type QuestionBackend interface { + // Answer is idempotent. The first call starts a read-only invocation for + // the question; later calls report its progress, then its answer. + Answer(context.Context, Assignment, workflow.Question) (QuestionObservation, error) +} + +// QuestionObservation is what the backend knows about one answer. +type QuestionObservation struct { + State string // running, answered or failed + Answer string + Detail string + Usage *workflow.Usage +} + +// scheduleQuestions answers open questions on every revision that could still +// have a supervisor to ask, including completed runs kept for inspection: +// being able to ask about finished work is the point. +func (e *Engine) scheduleQuestions(ctx context.Context, run workflow.Run, rev workflow.Revision) { + for _, q := range rev.Questions { + if q.Finished() { + continue + } + id := q.ID + e.launch(ctx, run.ID+"/"+rev.ID+"/question/"+id, func() error { return e.reconcileQuestion(ctx, run.ID, rev.ID, id) }) + } +} + +func (e *Engine) reconcileQuestion(ctx context.Context, runID, revision, id string) error { + run, err := e.Store.Get(ctx, runID) + if err != nil { + return err + } + rev := run.Revision(revision) + if rev == nil { + return nil + } + q := rev.Question(id) + if q == nil || q.Finished() { + return nil + } + fail := func(detail string) error { + return e.finishQuestion(ctx, runID, revision, id, QuestionOutcome{State: workflow.QuestionFailed, Detail: detail}) + } + + backend, ok := e.Backend.(QuestionBackend) + if !ok { + return fail("this coordinator cannot answer questions") + } + attempt := rev.Attempt(q.Attempt) + if attempt == nil { + return fail("the attempt this question was about no longer exists") + } + a := assign(run, rev, attempt) + // The answer reads the stage's worktree inside its VM. Once a run is + // closed, cancelled or superseded that VM is gone, and a rewind is the + // way back to a supervisor that can look. + if !a.Revision.Runtime.Ready { + return fail("the stage's VM is no longer running, so its supervisor cannot look at the work; rewind to reopen it") + } + // A question that has not started yet is checked against the ceiling + // again: other work may have used the budget since it was asked. + if q.State == workflow.QuestionPending { + if exceeded, reason := run.Budget(); exceeded { + return fail("the run reached its ceiling (" + reason + ") before this could be answered") + } + } + obs, err := backend.Answer(ctx, a, *q) + if err != nil { + return err + } + switch obs.State { + case workflow.QuestionAnswered: + return e.finishQuestion(ctx, runID, revision, id, QuestionOutcome{State: workflow.QuestionAnswered, Answer: obs.Answer, Usage: obs.Usage}) + case workflow.QuestionFailed: + return e.finishQuestion(ctx, runID, revision, id, QuestionOutcome{State: workflow.QuestionFailed, Detail: obs.Detail, Usage: obs.Usage}) + } + if q.State == workflow.QuestionAnswering { + return nil + } + _, err = e.update(ctx, runID, revision, "question.answering", func(_ *workflow.Run, v *workflow.Revision) error { + current := v.Question(id) + if current == nil || current.State != workflow.QuestionPending { + return workflow.ErrConflict + } + current.State = workflow.QuestionAnswering + return nil + }) + return ignoreConflict(err) +} + +// QuestionOutcome is a question's final state. +type QuestionOutcome struct { + State string + Answer string + Detail string + Usage *workflow.Usage +} + +// finishQuestion records an answer exactly once. A question already finished +// is left alone, so a reconcile racing a restart cannot record it twice. +func (e *Engine) finishQuestion(ctx context.Context, runID, revision, id string, out QuestionOutcome) error { + _, err := e.update(ctx, runID, revision, "question."+out.State, func(_ *workflow.Run, v *workflow.Revision) error { + current := v.Question(id) + if current == nil || current.Finished() { + return workflow.ErrConflict + } + current.State, current.Answer, current.Detail, current.Usage = out.State, out.Answer, out.Detail, out.Usage + current.AnsweredAt = e.now() + return nil + }) + return ignoreConflict(err) +} + +// ignoreConflict treats a lost race as success. A question's transitions are +// guarded by its current state, so a conflict means another reconcile has +// already made the change. +func ignoreConflict(err error) error { + if errors.Is(err, workflow.ErrConflict) { + return nil + } + return err +} diff --git a/internal/engine/questions_test.go b/internal/engine/questions_test.go new file mode 100644 index 0000000..a5e40b5 --- /dev/null +++ b/internal/engine/questions_test.go @@ -0,0 +1,187 @@ +package engine + +import ( + "context" + "testing" + + "github.com/sam-bretz/envctl/internal/workflow" +) + +// answeringBackend is the fixture backend plus the ability to answer. It +// answers on the second call, so a test sees "answering" before "answered". +type answeringBackend struct { + *fixtureBackend + calls map[string]int + fail bool +} + +func (b *answeringBackend) Answer(_ context.Context, a Assignment, q workflow.Question) (QuestionObservation, error) { + b.calls[q.ID]++ + if b.fail { + return QuestionObservation{State: workflow.QuestionFailed, Detail: "harness exited"}, nil + } + if b.calls[q.ID] < 2 { + return QuestionObservation{State: "running"}, nil + } + return QuestionObservation{State: workflow.QuestionAnswered, Answer: "PR #7 is open for " + a.Attempt.Node, Usage: &workflow.Usage{Input: 30, Output: 5}}, nil +} + +func answering(h *harness) *answeringBackend { + b := &answeringBackend{fixtureBackend: h.backend, calls: map[string]int{}} + h.engine.Backend = b + return b +} + +func (h *harness) ask(node, text string) string { + h.t.Helper() + var id string + h.mutate(func(r *workflow.Run) error { + var err error + id, err = r.Ask(h.rev, node, text, "you", h.now) + return err + }) + return id +} + +func (h *harness) reconcileQuestion(id string) { + h.t.Helper() + if err := h.engine.reconcileQuestion(context.Background(), h.id, h.rev, id); err != nil { + h.t.Fatal(err) + } +} + +func waitingForApproval(v *workflow.Revision) bool { + for _, a := range v.Attempts { + if a.State == "awaiting-approval" { + return true + } + } + return false +} + +func TestAQuestionIsAnsweredWhetherTheStageIsRunningWaitingOrAccepted(t *testing.T) { + h := setup(t) + h.until(waitingForApproval) + v := h.run().Current() + waiting := "" + for _, a := range v.Attempts { + if a.State == "awaiting-approval" { + waiting = a.Node + } + } + + // Running: a job that stays running. + running := setup(t) + running.backend.startRunning = true + running.until(func(v *workflow.Revision) bool { + for _, a := range v.Attempts { + if a.State == "running" { + return true + } + } + return false + }) + runningNode := "" + for _, a := range running.run().Current().Attempts { + if a.State == "running" { + runningNode = a.Node + } + } + + for _, c := range []struct { + name string + h *harness + node string + }{ + {"running", running, runningNode}, + {"waiting for approval", h, waiting}, + {"accepted", h, "task"}, + } { + t.Run(c.name, func(t *testing.T) { + answering(c.h) + before := *workflow.Clone(c.h.run().Current()) + id := c.h.ask(c.node, "is there a PR open for this?") + + c.h.reconcileQuestion(id) + if q := c.h.run().Current().Question(id); q.State != workflow.QuestionAnswering { + t.Fatalf("not marked answering: %+v", q) + } + c.h.reconcileQuestion(id) + q := c.h.run().Current().Question(id) + if q.State != workflow.QuestionAnswered || q.Answer == "" || q.Usage == nil || q.AnsweredAt.IsZero() { + t.Fatalf("not answered: %+v", q) + } + + // The work is exactly as it was: same attempts, states, results + // and approvals. + after := c.h.run().Current() + if workflow.Digest(before.Attempts) != workflow.Digest(after.Attempts) || workflow.Digest(before.Checkpoints) != workflow.Digest(after.Checkpoints) { + t.Fatal("answering a question changed the work") + } + }) + } +} + +func TestAnAnswerIsRecordedOnceAndNeverAskedForAgain(t *testing.T) { + h := setup(t) + h.until(waitingForApproval) + b := answering(h) + id := h.ask("task", "why?") + h.reconcileQuestion(id) + h.reconcileQuestion(id) + calls := b.calls[id] + h.reconcileQuestion(id) + h.reconcileQuestion(id) + if b.calls[id] != calls { + t.Fatalf("a finished question went back to the harness: %d calls, then %d", calls, b.calls[id]) + } +} + +func TestAQuestionFailsClearlyWhenItCannotBeAnswered(t *testing.T) { + t.Run("the harness failed", func(t *testing.T) { + h := setup(t) + h.until(waitingForApproval) + answering(h).fail = true + id := h.ask("task", "why?") + h.reconcileQuestion(id) + if q := h.run().Current().Question(id); q.State != workflow.QuestionFailed || q.Detail == "" { + t.Fatalf("a failed answer was not reported: %+v", q) + } + }) + t.Run("the VM was released", func(t *testing.T) { + h := setup(t) + h.until(waitingForApproval) + b := answering(h) + id := h.ask("task", "why?") + h.mutate(func(r *workflow.Run) error { r.Current().Runtime.Ready = false; return nil }) + h.reconcileQuestion(id) + q := h.run().Current().Question(id) + if q.State != workflow.QuestionFailed || b.calls[id] != 0 { + t.Fatalf("answered without a VM to read the work from: %+v, %d calls", q, b.calls[id]) + } + }) + t.Run("the backend cannot answer", func(t *testing.T) { + h := setup(t) + h.until(waitingForApproval) + id := h.ask("task", "why?") + h.reconcileQuestion(id) + if q := h.run().Current().Question(id); q.State != workflow.QuestionFailed { + t.Fatalf("a backend without answers left the question open: %+v", q) + } + }) + t.Run("the ceiling was reached before it started", func(t *testing.T) { + h := setup(t) + h.until(waitingForApproval) + b := answering(h) + id := h.ask("task", "why?") + h.mutate(func(r *workflow.Run) error { + r.Current().Config.Limits.RunTokens = 1 + r.Current().Attempts[0].Usage = &workflow.Usage{Input: 1000} + return nil + }) + h.reconcileQuestion(id) + if q := h.run().Current().Question(id); q.State != workflow.QuestionFailed || b.calls[id] != 0 { + t.Fatalf("started an answer over the ceiling: %+v", q) + } + }) +} diff --git a/internal/localexec/attempt.go b/internal/localexec/attempt.go index f285ee1..5fea5f1 100644 --- a/internal/localexec/attempt.go +++ b/internal/localexec/attempt.go @@ -446,7 +446,7 @@ func (b *Backend) failed(a engine.Assignment, r *attemptRecord, detail string) ( if err := b.save(a, r); err != nil { return engine.Observation{}, err } - return engine.Observation{State: "failed", Detail: detail, Session: r.Session, Delivered: r.delivered(), Usage: b.usage(a, r)}, nil + return engine.Observation{State: "failed", Detail: detail, Session: r.Session, Notes: r.notes(a), Delivered: r.delivered(), Usage: b.usage(a, r)}, nil } func (b *Backend) workerFailed(ctx context.Context, a engine.Assignment, r *attemptRecord, detail string) (engine.Observation, error) { @@ -524,7 +524,7 @@ func (b *Backend) Poll(ctx context.Context, a engine.Assignment) (engine.Observa return engine.Observation{}, err } running := func() (engine.Observation, error) { - return engine.Observation{State: "running", Session: r.Session, Progress: b.progress(a, r), Delivered: r.delivered(), Usage: b.usage(a, r)}, nil + return engine.Observation{State: "running", Session: r.Session, Notes: r.notes(a), Progress: b.progress(a, r), Delivered: r.delivered(), Usage: b.usage(a, r)}, nil } // agentJob reconciles one role's active generation: it finishes stopping a // superseded generation, starts a missing job, records delivery once the @@ -822,11 +822,11 @@ func (b *Backend) Poll(ctx context.Context, a engine.Assignment) (engine.Observa if err = b.save(a, r); err != nil { return engine.Observation{}, err } - return engine.Observation{State: "completed", Result: &r.Result, Session: r.Session, Delivered: r.delivered(), Usage: b.usage(a, r)}, nil + return engine.Observation{State: "completed", Result: &r.Result, Session: r.Session, Notes: r.notes(a), Delivered: r.delivered(), Usage: b.usage(a, r)}, nil case "completed": - return engine.Observation{State: "completed", Result: &r.Result, Session: r.Session, Delivered: r.delivered(), Usage: b.usage(a, r)}, nil + return engine.Observation{State: "completed", Result: &r.Result, Session: r.Session, Notes: r.notes(a), Delivered: r.delivered(), Usage: b.usage(a, r)}, nil case "failed": - return engine.Observation{State: "failed", Detail: r.Detail, Session: r.Session, Delivered: r.delivered(), Usage: b.usage(a, r)}, nil + return engine.Observation{State: "failed", Detail: r.Detail, Session: r.Session, Notes: r.notes(a), Delivered: r.delivered(), Usage: b.usage(a, r)}, nil default: return engine.Observation{}, errors.New("unknown durable attempt phase") } diff --git a/internal/localexec/questions.go b/internal/localexec/questions.go new file mode 100644 index 0000000..616a277 --- /dev/null +++ b/internal/localexec/questions.go @@ -0,0 +1,179 @@ +package localexec + +import ( + "context" + "fmt" + "sort" + "strings" + + "github.com/sam-bretz/envctl/internal/agent" + "github.com/sam-bretz/envctl/internal/engine" + "github.com/sam-bretz/envctl/internal/workflow" +) + +// The engine finds this through a type assertion, so a signature drift would +// quietly turn every question into "cannot answer" instead of failing to +// build. +var _ engine.QuestionBackend = (*Backend)(nil) + +// answerTimeoutSeconds bounds one answer. A question is a read, not a stage, +// so it gets far less time than an attempt. +const answerTimeoutSeconds = 600 + +// Answer answers a question about a stage with a separate, read-only +// invocation of that stage's supervisor. It never touches the attempt's own +// jobs or record: the worker keeps running and the review stays as it was. +// +// It follows the shape of a readiness probe: the job ID is derived from the +// question, so a restart finds the job it already started instead of asking +// twice. +func (b *Backend) Answer(ctx context.Context, a engine.Assignment, q workflow.Question) (engine.QuestionObservation, error) { + h := a.Revision.Config.Agents.Supervisor + harness, err := agent.SelectVersion(h.Kind, h.Version, b.guest(a)) + if err != nil { + return unanswerable("the supervisor's harness is unavailable"), nil + } + id := q.ID + "_answer" + status, err := b.guest(a).Poll(ctx, id, 0) + if err != nil { + return engine.QuestionObservation{}, err + } + switch { + case status.State == "missing": + return b.startAnswer(ctx, a, q, id, h, harness) + case status.State == "pending": + _, err = b.guest(a).Reconcile(ctx, id) + return engine.QuestionObservation{State: "running"}, err + case pending(status.State): + return engine.QuestionObservation{State: "running"}, nil + } + usage := harness.Usage(status.Output) + if status.State != "completed" { + return engine.QuestionObservation{State: workflow.QuestionFailed, Detail: "answering ended in " + status.State, Usage: &usage}, nil + } + raw, err := harness.Result(ctx, id) + if err != nil { + return engine.QuestionObservation{State: workflow.QuestionFailed, Detail: "the supervisor's answer could not be read", Usage: &usage}, nil + } + answer, err := agent.ParseAnswer(clean(a, raw)) + if err != nil { + return engine.QuestionObservation{State: workflow.QuestionFailed, Detail: "the supervisor did not return an answer: " + err.Error(), Usage: &usage}, nil + } + return engine.QuestionObservation{State: workflow.QuestionAnswered, Answer: answer, Usage: &usage}, nil +} + +func (b *Backend) startAnswer(ctx context.Context, a engine.Assignment, q workflow.Question, id string, h workflow.Harness, harness agent.Harness) (engine.QuestionObservation, error) { + record, err := b.load(a) + if err != nil || record.Worker.Directory == "" { + return unanswerable("the stage's worktree is no longer available, so its supervisor cannot look at the work"), nil + } + credential, err := agent.ResolveCredential(a.Revision.Config.Dir, h.Kind, h.Credential) + if err != nil { + return unanswerable("the supervisor's harness credential is unavailable"), nil + } + if _, err = harness.Start(ctx, answerInvocation(a, record, q, id, h), credential); err != nil { + return engine.QuestionObservation{}, err + } + return engine.QuestionObservation{State: "running"}, nil +} + +// answerInvocation is the read-only invocation that answers a question. +func answerInvocation(a engine.Assignment, record *attemptRecord, q workflow.Question, id string, h workflow.Harness) agent.Invocation { + i := agent.Invocation{ + ID: id, Role: "supervisor", Directory: record.Worker.Directory, + Prompt: questionPrompt(a, record, q), Schema: agent.AnswerSchema(), + // The stage's own supervisor model, so the answer comes from the + // same kind of reviewer that judged the work. + Model: a.Revision.Config.NodeAgents(q.Node).Supervisor.Model, + TimeoutSeconds: answerTimeoutSeconds, ReadOnly: true, + } + // Answering from the supervisor's own session grounds the answer in what + // it actually reviewed. Only Claude can branch that session; continuing it + // would append the question to the transcript the supervisor resumes when + // steered, so Codex starts fresh from the facts in the prompt instead. + if h.Kind == "claude" && record.SupervisorSession != "" { + i.Session, i.Fork = record.SupervisorSession, true + } + return i +} + +func unanswerable(detail string) engine.QuestionObservation { + return engine.QuestionObservation{State: workflow.QuestionFailed, Detail: detail} +} + +// questionPrompt gives the supervisor what it needs to answer from facts +// rather than memory: the stage, the work and its review, the checks bound to +// commits, any pull requests, the steering it received, and the earlier +// questions on this stage so a follow-up makes sense. +func questionPrompt(a engine.Assignment, record *attemptRecord, q workflow.Question) string { + var p strings.Builder + node := a.Revision.Config.Workflow.Nodes[q.Node] + fmt.Fprintf(&p, "A person is asking about the %s stage (%s) of an envctl run.\n\nRun objective:\n%s\n\n", q.Node, node.Kind, a.Revision.Objective) + fmt.Fprintf(&p, "Attempt %d is %s.\n", a.Attempt.Number, a.Attempt.State) + result := record.Result + if a.Attempt.Result != nil { + result = *a.Attempt.Result + } + if result.Summary != "" { + fmt.Fprintf(&p, "\nWorker's summary:\n%s\n", result.Summary) + } + if result.Review.Summary != "" { + fmt.Fprintf(&p, "\nSupervisor's review:\n%s\n", result.Review.Summary) + } + if len(result.Checks) > 0 { + p.WriteString("\nChecks:\n") + for _, c := range result.Checks { + outcome := "passed" + if !c.Passed { + outcome = fmt.Sprintf("failed (exit %d)", c.ExitCode) + } + fmt.Fprintf(&p, "- %s %s\n", c.Name, outcome) + } + } + if prs := publishedPRs(a, result); len(prs) > 0 { + p.WriteString("\nPull requests opened for this run:\n") + for _, line := range prs { + p.WriteString("- " + line + "\n") + } + } else { + p.WriteString("\nNo pull request has been opened for this run yet.\n") + } + var steering []string + for _, m := range a.Revision.Messages { + if m.Node == "" || m.Node == q.Node { + steering = append(steering, "to the "+m.Recipient+": "+m.Body) + } + } + if len(steering) > 0 { + p.WriteString("\nSteering this stage received:\n- " + strings.Join(steering, "\n- ") + "\n") + } + for _, earlier := range a.Revision.Questions { + if earlier.Node == q.Node && earlier.ID != q.ID && earlier.State == workflow.QuestionAnswered { + fmt.Fprintf(&p, "\nEarlier question: %s\nYour answer: %s\n", earlier.Text, earlier.Answer) + } + } + fmt.Fprintf(&p, "\nQuestion:\n%s\n\n", q.Text) + p.WriteString("Answer the question directly and concisely. You may read the worktree to check your answer, but you cannot change anything: this is a question, not a request to redo or alter the work. If the answer is not knowable from the work and the facts above, say so plainly instead of guessing. Return only the answer.") + return p.String() +} + +// publishedPRs lists pull requests from any accepted checkpoint of the run's +// current revision, since a question on an early stage may still be about the +// change the run opened later. +func publishedPRs(a engine.Assignment, result workflow.Result) []string { + found := map[string]string{} + for key, url := range result.PRs { + found[key] = url + } + for _, cp := range a.Revision.Checkpoints { + for key, url := range cp.Result.PRs { + found[key] = url + } + } + lines := make([]string, 0, len(found)) + for key, url := range found { + lines = append(lines, key+": "+url) + } + sort.Strings(lines) + return lines +} diff --git a/internal/localexec/questions_test.go b/internal/localexec/questions_test.go new file mode 100644 index 0000000..8b6d1ff --- /dev/null +++ b/internal/localexec/questions_test.go @@ -0,0 +1,111 @@ +package localexec + +import ( + "strings" + "testing" + "time" + + "github.com/sam-bretz/envctl/internal/agent" + "github.com/sam-bretz/envctl/internal/engine" + "github.com/sam-bretz/envctl/internal/workflow" +) + +func questionAssignment(t *testing.T) (engine.Assignment, *attemptRecord, workflow.Question) { + t.Helper() + c, err := workflow.Parse([]byte("version: 2\nproject: demo\nrepositories: [{id: app, url: /source}]\nworkflow: {template: feature, nodes: {code: {agents: {supervisor: {model: claude-opus-5}}}}}\n")) + if err != nil { + t.Fatal(err) + } + rev := workflow.Revision{ID: "rev_1", Objective: "Add CSV export", Config: c, + Checkpoints: map[string]workflow.Checkpoint{}, + Messages: []workflow.Message{{Node: "code", Recipient: "worker", Body: "handle empty files"}}, + } + attempt := workflow.Attempt{ID: "attempt_1", Node: "code", Number: 2, State: "awaiting-approval", Result: &workflow.Result{ + Summary: "Wrote the exporter", Review: workflow.Review{Summary: "Covers the empty-file case"}, + Checks: []workflow.CheckResult{{Name: "unit", Passed: true}, {Name: "lint", Passed: false, ExitCode: 1}}, + }} + q := workflow.Question{ID: "question_1", Node: "code", Attempt: "attempt_1", Text: "is there a PR open for this?"} + record := &attemptRecord{Worker: agent.Invocation{Directory: "/work/envctl/repos/app"}, SupervisorSession: "01a08f59-7073-7a83-b959-867ca896ce48"} + return engine.Assignment{Revision: rev, Attempt: attempt}, record, q +} + +func TestAnAnswerComesFromTheStagesOwnSupervisorAndCannotWrite(t *testing.T) { + a, record, q := questionAssignment(t) + claude := workflow.Harness{Kind: "claude"} + i := answerInvocation(a, record, q, "question_1_answer", claude) + if !i.ReadOnly || i.Role != "supervisor" || i.Directory != "/work/envctl/repos/app" { + t.Fatalf("not a read-only supervisor invocation in the worktree: %+v", i) + } + if i.Model != "claude-opus-5" { + t.Fatalf("answered by %q, not the stage's supervisor model", i.Model) + } + // Claude branches the supervisor's session, so the answer is grounded in + // what it reviewed without touching the transcript it resumes later. + if i.Session != record.SupervisorSession || !i.Fork { + t.Fatalf("claude did not fork the supervisor's session: %+v", i) + } + // Codex cannot branch a session, so it must start fresh rather than + // append to one. + codex := answerInvocation(a, record, q, "question_1_answer", workflow.Harness{Kind: "codex"}) + if codex.Session != "" || codex.Fork { + t.Fatalf("codex would continue the supervisor's session: %+v", codex) + } + // With no supervisor session yet, as while the worker is still running, + // the answer starts fresh from the facts. + record.SupervisorSession = "" + if fresh := answerInvocation(a, record, q, "question_1_answer", claude); fresh.Session != "" || fresh.Fork { + t.Fatalf("resumed a session that does not exist: %+v", fresh) + } +} + +func TestTheAnswerIsGroundedInTheWorkNotTheModelsMemory(t *testing.T) { + a, record, q := questionAssignment(t) + prompt := questionPrompt(a, record, q) + for _, want := range []string{ + "Add CSV export", "Attempt 2 is awaiting-approval", + "Wrote the exporter", "Covers the empty-file case", + "unit passed", "lint failed (exit 1)", + "to the worker: handle empty files", + // The example the issue is built around: this must be answerable. + "No pull request has been opened for this run yet.", + "is there a PR open for this?", + "cannot change anything", + "say so plainly instead of guessing", + } { + if !strings.Contains(prompt, want) { + t.Fatalf("prompt is missing %q:\n%s", want, prompt) + } + } + + // Once a pull request exists, the prompt names it instead. + a.Revision.Checkpoints["approved-change"] = workflow.Checkpoint{Result: workflow.Result{PRs: map[string]string{"app": "https://github.com/o/r/pull/7"}}} + prompt = questionPrompt(a, record, q) + if !strings.Contains(prompt, "app: https://github.com/o/r/pull/7") || strings.Contains(prompt, "No pull request") { + t.Fatalf("the published pull request is not in the prompt:\n%s", prompt) + } + + // A follow-up sees what was already answered, which is what makes this a + // conversation rather than a series of unrelated questions. + a.Revision.Questions = []workflow.Question{{ID: "question_0", Node: "code", State: workflow.QuestionAnswered, Text: "which parser?", Answer: "encoding/csv"}, q} + if prompt = questionPrompt(a, record, q); !strings.Contains(prompt, "Earlier question: which parser?\nYour answer: encoding/csv") { + t.Fatalf("earlier answers are not carried into a follow-up:\n%s", prompt) + } +} + +func TestEachStallNudgeIsReportedWithWhenItWasSent(t *testing.T) { + a, _, _ := questionAssignment(t) + first, second := time.Date(2026, 9, 16, 10, 0, 0, 0, time.UTC), time.Date(2026, 9, 16, 10, 12, 0, 0, time.UTC) + r := &attemptRecord{Stall: map[string]*stallState{ + "worker": {Nudges: []int{1, 2}, NudgedAt: []time.Time{first, second}}, + }} + notes := r.notes(a) + if len(notes) != 2 || !notes[0].At.Equal(first) || !notes[1].At.Equal(second) { + t.Fatalf("nudges not reported with their times: %+v", notes) + } + if notes[1].Kind != workflow.NoteStallNudge || notes[1].Node != "code" || !strings.Contains(notes[1].Detail, "worker") || !strings.Contains(notes[1].Detail, "nudge 2 of") { + t.Fatalf("nudge note does not say which role or which nudge: %+v", notes[1]) + } + if (&attemptRecord{}).notes(a) != nil { + t.Fatal("an attempt that was never nudged reported notes") + } +} diff --git a/internal/localexec/stall.go b/internal/localexec/stall.go index 7447d05..9d655d7 100644 --- a/internal/localexec/stall.go +++ b/internal/localexec/stall.go @@ -10,6 +10,7 @@ import ( "github.com/sam-bretz/envctl/internal/agent" "github.com/sam-bretz/envctl/internal/engine" "github.com/sam-bretz/envctl/internal/guestjob" + "github.com/sam-bretz/envctl/internal/workflow" ) // MaxStallNudges bounds coordinator nudges per agent role in one attempt. A @@ -24,7 +25,10 @@ type stallState struct { Since time.Time `json:"since"` // its last output, or when it was first observed Extended bool `json:"extended,omitempty"` // window extended once for a command in flight Nudges []int `json:"nudges,omitempty"` // live generations started by stall nudges - Failing string `json:"failing,omitempty"` // evidence; the stalled job is being stopped + // NudgedAt is when each nudge was sent, parallel to Nudges, so the + // Decision Log can say when the coordinator stepped in. + NudgedAt []time.Time `json:"nudged_at,omitempty"` + Failing string `json:"failing,omitempty"` // evidence; the stalled job is being stopped } func (b *Backend) now() time.Time { @@ -104,6 +108,7 @@ func (b *Backend) watchStall(a engine.Assignment, r *attemptRecord, role, logs s s.Failing = fmt.Sprintf("%s; live resume limit (%d) reached", evidence, MaxLiveSteering) default: s.Nudges = append(s.Nudges, r.advance(a, role, nudgePrompt(role, idle))) + s.NudgedAt = append(s.NudgedAt, now) return true, b.save(a, r) } s.Failing = string(clean(a, []byte(s.Failing))) @@ -180,3 +185,19 @@ func short(d time.Duration) string { } return s } + +// notes reports the stall nudges this attempt has sent, for the run to keep. +func (r *attemptRecord) notes(a engine.Assignment) []workflow.Note { + var out []workflow.Note + for _, role := range []string{"worker", "supervisor"} { + s := r.Stall[role] + if s == nil { + continue + } + for i, at := range s.NudgedAt { + out = append(out, workflow.Note{At: at, Kind: workflow.NoteStallNudge, Node: a.Attempt.Node, + Detail: fmt.Sprintf("nudged the %s after a silence (nudge %d of %d)", role, i+1, MaxStallNudges)}) + } + } + return out +} diff --git a/internal/mcpserver/server.go b/internal/mcpserver/server.go index a1ec511..5d461cf 100644 --- a/internal/mcpserver/server.go +++ b/internal/mcpserver/server.go @@ -65,7 +65,7 @@ type actionInput struct { OperationID string `json:"operation_id" jsonschema:"Stable caller-generated ID; reuse with identical arguments on reconnect"` ExpectedVersion int64 `json:"expected_version" jsonschema:"Observed run version; stale mutations are rejected"` Revision string `json:"revision" jsonschema:"Observed current revision; never silently updated by the server"` - Action string `json:"action" jsonschema:"One of message, rewind, cancel, priority, plugin-attach, plugin-remove"` + Action string `json:"action" jsonschema:"One of message, ask, rewind, cancel, priority, plugin-attach, plugin-remove. ask puts message to node's supervisor as a question; it never changes the work, and the answer appears on the run's current revision under questions"` Node string `json:"node,omitempty"` Task string `json:"task,omitempty"` Recipient string `json:"recipient,omitempty"` @@ -170,11 +170,11 @@ func New(api API, options Options) *mcp.Server { return nil, map[string]any{"run": r}, err }) } - mcp.AddTool(server, tool("envctl_action", "Send a message, rewind, cancel, change priority, or amend invocation plugins. Requires exact observed version/revision and a stable operation_id. Rewind/plugin changes create revisions. Human approval and publication are not agent tools.", false), func(ctx context.Context, _ *mcp.CallToolRequest, in actionInput) (*mcp.CallToolResult, any, error) { + mcp.AddTool(server, tool("envctl_action", "Send a message, ask a stage's supervisor a question, rewind, cancel, change priority, or amend invocation plugins. Requires exact observed version/revision and a stable operation_id. Rewind/plugin changes create revisions. Human approval and publication are not agent tools.", false), func(ctx context.Context, _ *mcp.CallToolRequest, in actionInput) (*mcp.CallToolResult, any, error) { if err := scope(in.Run); err != nil { return nil, nil, err } - if !slices.Contains([]string{"message", "rewind", "cancel", "priority", "plugin-attach", "plugin-remove"}, in.Action) { + if !slices.Contains([]string{"message", "ask", "rewind", "cancel", "priority", "plugin-attach", "plugin-remove"}, in.Action) { return nil, nil, errors.New("unsupported agent action; human approval and checkpoint publication are separate operations") } if in.OperationID == "" || in.ExpectedVersion < 1 || in.Revision == "" { diff --git a/internal/tui/agents_test.go b/internal/tui/agents_test.go index eb19168..8bf7cc5 100644 --- a/internal/tui/agents_test.go +++ b/internal/tui/agents_test.go @@ -44,7 +44,7 @@ func TestConversationModelLineCoexistsWithAttemptDetail(t *testing.T) { rev.Config.Workflow.Nodes[node] = nodeCfg rev.Attempts = []workflow.Attempt{{ID: "attempt_one", Node: node, Number: 1, State: "running"}} view := m.View().Content - if !strings.Contains(view, "Worker model: fixture-worker-model") || !strings.Contains(view, "Worker attempt 1: running") { + if !strings.Contains(view, "Worker model: fixture-worker-model") || !strings.Contains(view, "attempt 1 started") { t.Fatalf("conversation lost either the model line or the attempt detail:\n%s", view) } } diff --git a/internal/tui/chat.go b/internal/tui/chat.go new file mode 100644 index 0000000..fafd9c0 --- /dev/null +++ b/internal/tui/chat.go @@ -0,0 +1,195 @@ +package tui + +import ( + "fmt" + "slices" + "strings" + "time" + + "github.com/sam-bretz/envctl/internal/workflow" +) + +// chatEntry is one line of a stage's conversation. +type chatEntry struct { + At time.Time + // Who said it: you, worker, supervisor or coordinator. To is set for + // what you sent, and names who it went to. + Who, To string + // Kind is steer, ask, answer, thinking, summary, review, approval, + // progress or status. + Kind string + Text string + Status string +} + +// chatThread is a stage's conversation in the order it happened. The panel +// used to print attempts in one block and messages in another, which read as +// two logs rather than a conversation; progress summaries are now entries in +// the same thread as what you said. +func chatThread(rev *workflow.Revision, node string) []chatEntry { + var out []chatEntry + for _, a := range rev.Attempts { + if a.Node != node { + continue + } + out = append(out, chatEntry{At: a.StartedAt, Who: "coordinator", Kind: "status", Text: fmt.Sprintf("attempt %d started", a.Number)}) + if a.Result != nil && a.Result.Summary != "" { + out = append(out, chatEntry{At: a.UpdatedAt, Who: "worker", Kind: "summary", Text: a.Result.Summary}) + } + switch { + case a.State == "failed" && strings.HasPrefix(a.Error, correctionPrefix): + out = append(out, chatEntry{At: a.UpdatedAt, Who: "supervisor", Kind: "review", Text: "rejected: " + strings.TrimPrefix(a.Error, correctionPrefix)}) + case a.State == "failed": + out = append(out, chatEntry{At: a.UpdatedAt, Who: "coordinator", Kind: "status", Text: fmt.Sprintf("attempt %d failed: %s", a.Number, a.Error)}) + case a.Result != nil && a.Result.Review.Accepted: + out = append(out, chatEntry{At: a.UpdatedAt, Who: "supervisor", Kind: "review", Text: "accepted: " + a.Result.Review.Summary}) + } + if a.State == "awaiting-approval" { + out = append(out, chatEntry{At: a.UpdatedAt, Who: "coordinator", Kind: "status", Text: "waiting for your approval · a to approve"}) + } + if a.Approval != nil { + out = append(out, chatEntry{At: a.Approval.At, Who: approver(a.Approval.Actor), Kind: "approval", + Text: fmt.Sprintf("approved attempt %d at %s", a.Number, a.Approval.At.Local().Format("2006-01-02 15:04"))}) + } + if p := a.Progress; a.State == "running" && p != nil { + // "live:" says this is happening now, and keeps a phase named + // "worker" from reading as "worker: worker". + text := "live: " + p.Phase + if p.Generation > 0 { + text += fmt.Sprintf(" (resume %d)", p.Generation) + } + if p.Detail != "" { + text += " — " + p.Detail + } + if len(p.Activity) > 0 { + text += "\n" + strings.Join(p.Activity, "\n") + } + out = append(out, chatEntry{At: p.UpdatedAt, Who: "worker", Kind: "progress", Text: text}) + } + } + // A rewind carries earlier stages' checkpoints into the new revision + // without the attempts that produced them. Without this, an inherited + // stage's accepted result would show nothing at all in Chat. + if cp, ok := rev.Checkpoints[node]; ok && rev.Attempt(cp.Attempt) == nil { + out = append(out, chatEntry{At: cp.CreatedAt, Who: "coordinator", Kind: "status", Text: "result carried over from an earlier revision"}) + if cp.Result.Summary != "" { + out = append(out, chatEntry{At: cp.CreatedAt, Who: "worker", Kind: "summary", Text: cp.Result.Summary}) + } + if cp.Result.Review.Summary != "" { + out = append(out, chatEntry{At: cp.CreatedAt, Who: "supervisor", Kind: "review", Text: "accepted: " + cp.Result.Review.Summary}) + } + if cp.Approval != nil { + out = append(out, chatEntry{At: cp.Approval.At, Who: approver(cp.Approval.Actor), Kind: "approval", + Text: "approved at " + cp.Approval.At.Local().Format("2006-01-02 15:04")}) + } + } + for _, msg := range rev.Messages { + if msg.Node == "" || msg.Node == node { + out = append(out, chatEntry{At: msg.CreatedAt, Who: "you", To: msg.Recipient, Kind: "steer", Text: msg.Body, Status: rev.MessageStatus(msg)}) + } + } + for _, q := range rev.Questions { + if q.Node != node { + continue + } + out = append(out, chatEntry{At: q.CreatedAt, Who: "you", To: "supervisor", Kind: "ask", Text: q.Text}) + switch q.State { + case workflow.QuestionAnswered: + out = append(out, chatEntry{At: q.AnsweredAt, Who: "supervisor", Kind: "answer", Text: q.Answer}) + case workflow.QuestionFailed: + out = append(out, chatEntry{At: q.AnsweredAt, Who: "supervisor", Kind: "answer", Text: "could not answer: " + q.Detail}) + default: + // Placed a moment after the question so it always sorts beneath it. + out = append(out, chatEntry{At: q.CreatedAt.Add(time.Nanosecond), Who: "supervisor", Kind: "thinking"}) + } + } + slices.SortStableFunc(out, func(a, b chatEntry) int { return a.At.Compare(b.At) }) + return out +} + +var thinkingFrames = []string{"thinking", "thinking.", "thinking..", "thinking..."} + +// chat renders the Chat panel for the selected stage. +func (m Model) chat() string { + rev := m.viewRevision() + node := m.nodeID() + agents := rev.Config.NodeAgents(node) + head := []string{fmt.Sprintf("Worker model: %s · Supervisor model: %s", formatModelTUI(agents.Worker.Model), formatModelTUI(agents.Supervisor.Model))} + // A stage's dependencies are the part of the old Graph tab worth keeping. + if deps := rev.Config.Workflow.Nodes[node].Needs; len(deps) > 0 { + head = append(head, "Runs after: "+strings.Join(deps, ", ")) + } + if len(rev.Config.Plugins) > 0 { + var attached []string + for _, p := range rev.Config.Plugins { + attached = append(attached, p.ID+"@"+p.Version) + } + head = append(head, "Plugins: "+strings.Join(attached, ", ")+" (p add or replace, P remove; reopens Plan)") + } + + thread := chatThread(rev, node) + if len(thread) == 0 { + // On an empty stage what to do next matters more than which model is + // configured, and a small terminal only has room for a few lines. + return strings.Join(append([]string{"No stage messages yet. i to steer the " + m.Recipient + ", ? to ask the supervisor."}, head...), "\n") + } + lines := append(head, "") + for _, e := range thread { + lines = append(lines, m.chatLine(e)) + } + if artifacts := m.chatArtifacts(rev, node); artifacts != "" { + lines = append(lines, "", artifacts) + } + return strings.Join(lines, "\n") +} + +func (m Model) chatLine(e chatEntry) string { + when := " " + if !e.At.IsZero() { + when = e.At.Local().Format("15:04") + } + who := e.Who + if e.To != "" { + // Saying which kind a message was keeps an ask from being mistaken + // for an instruction that changed the work. + who += " → " + e.To + " (" + e.Kind + ")" + } + text := e.Text + if e.Kind == "thinking" { + text = thinkingFrames[m.Frame%len(thinkingFrames)] + } + if e.Status != "" { + text += " [" + e.Status + "]" + } + indent := strings.Repeat(" ", len(when)+2) + return when + " " + who + ": " + strings.ReplaceAll(strings.TrimRight(text, "\n"), "\n", "\n"+indent) +} + +// chatArtifacts lists what the selected stage produced, so , . and o keep +// working now that the Checkpoint tab is gone. +func (m Model) chatArtifacts(rev *workflow.Revision, node string) string { + artifacts := []workflow.Artifact{} + title := "Artifacts (,/. select · o open):" + if cp, ok := rev.Checkpoints[node]; ok { + artifacts = cp.Result.Artifacts + } else { + for _, a := range rev.Attempts { + if a.Node == node && a.State == "awaiting-approval" && a.Result != nil { + artifacts = a.Result.Artifacts + title = "Artifacts awaiting approval:" + } + } + } + if len(artifacts) == 0 { + return "" + } + lines := []string{title} + for i, a := range artifacts { + marker := " " + if i == m.ArtifactIndex { + marker = ">" + } + lines = append(lines, fmt.Sprintf("%s %s · %s · %d bytes", marker, a.Name, a.MediaType, a.Size)) + } + return strings.Join(lines, "\n") +} diff --git a/internal/tui/chat_test.go b/internal/tui/chat_test.go new file mode 100644 index 0000000..ab8d2d7 --- /dev/null +++ b/internal/tui/chat_test.go @@ -0,0 +1,125 @@ +package tui + +import ( + "strings" + "testing" + "time" + + "github.com/sam-bretz/envctl/internal/workflow" +) + +func TestChatReadsAsOneConversationInTheOrderItHappened(t *testing.T) { + m := panel(t, modelFixture(t), "Chat") + m.Width, m.Height = 160, 60 + rev := m.Runs[0].Current() + node := m.nodeID() + base := time.Date(2026, 9, 16, 9, 0, 0, 0, time.Local) + at := func(min int) time.Time { return base.Add(time.Duration(min) * time.Minute) } + rev.Attempts = []workflow.Attempt{{ID: "attempt_1", Node: node, Number: 1, State: "awaiting-approval", StartedAt: at(0), UpdatedAt: at(3), + Result: &workflow.Result{Summary: "Wrote the exporter", Review: workflow.Review{Accepted: true, Summary: "covers every path"}}}} + rev.Messages = []workflow.Message{{ID: "m1", Node: node, Recipient: "worker", Body: "handle empty files", CreatedAt: at(1)}} + rev.Questions = []workflow.Question{ + {ID: "q1", Node: node, Text: "is there a PR open for this?", State: workflow.QuestionAnswered, Answer: "Not yet; approved-change opens it.", CreatedAt: at(4), AnsweredAt: at(5)}, + {ID: "q2", Node: node, Text: "which parser?", State: workflow.QuestionAnswering, CreatedAt: at(6)}, + {ID: "q3", Node: node, Text: "why?", State: workflow.QuestionFailed, Detail: "the stage's VM is no longer running", CreatedAt: at(7), AnsweredAt: at(7)}, + } + + chat := m.details() + order := []string{ + "coordinator: attempt 1 started", + "you → worker (steer): handle empty files", + "worker: Wrote the exporter", + "supervisor: accepted: covers every path", + "waiting for your approval", + "you → supervisor (ask): is there a PR open for this?", + "supervisor: Not yet; approved-change opens it.", + "you → supervisor (ask): which parser?", + "supervisor: thinking", + "you → supervisor (ask): why?", + "supervisor: could not answer: the stage's VM is no longer running", + } + last := -1 + for _, want := range order { + i := strings.Index(chat, want) + if i < 0 { + t.Fatalf("missing %q:\n%s", want, chat) + } + if i < last { + t.Fatalf("%q is out of order:\n%s", want, chat) + } + last = i + } +} + +func TestAnApprovalShowsWhoApprovedAndWhenWithTheStage(t *testing.T) { + m := panel(t, modelFixture(t), "Chat") + rev := m.Runs[0].Current() + node := m.nodeID() + approved := time.Date(2026, 9, 16, 14, 30, 0, 0, time.Local) + rev.Attempts = []workflow.Attempt{{ID: "attempt_1", Node: node, Number: 1, State: "checkpointed", + Approval: &workflow.Approval{Actor: "sam", At: approved}}} + if chat := m.details(); !strings.Contains(chat, "sam: approved attempt 1 at 2026-09-16 14:30") { + t.Fatalf("approval actor and time not shown with the stage:\n%s", chat) + } +} + +func TestAStageCarriedOverByARewindStillShowsItsResult(t *testing.T) { + // A rewind copies earlier checkpoints into the new revision without the + // attempts that produced them, so the thread must read the checkpoint too. + m := panel(t, modelFixture(t), "Chat") + rev := m.Runs[0].Current() + node := m.nodeID() + rev.Attempts = nil + rev.Checkpoints[node] = workflow.Checkpoint{ID: "cp_old", Node: node, Attempt: "attempt_in_an_older_revision", + Result: workflow.Result{Summary: "the original design", Review: workflow.Review{Summary: "sound"}, + Artifacts: []workflow.Artifact{{Name: "design", MediaType: "text/markdown", Size: 42}}}} + chat := m.details() + for _, want := range []string{"carried over from an earlier revision", "worker: the original design", "supervisor: accepted: sound"} { + if !strings.Contains(chat, want) { + t.Fatalf("an inherited result is invisible (missing %q):\n%s", want, chat) + } + } + // The artifact list replaces the Checkpoint tab, so , . and o still have + // something to select. + if !strings.Contains(chat, "> design · text/markdown · 42 bytes") { + t.Fatalf("the stage's artifacts are not listed in Chat:\n%s", chat) + } +} + +func TestQuestionMarkAsksTheSupervisorWithoutSteering(t *testing.T) { + m := modelFixture(t) + m.Width, m.Height = 120, 40 + m, _ = key(m, "?") + if m.Mode != "ask" || !strings.Contains(m.View().Content, "does not change the work") { + t.Fatalf("? did not open the ask composer: mode %q", m.Mode) + } + for _, r := range "is there a PR?" { + m, _ = key(m, string(r)) + } + m, cmd := key(m, "enter") + if cmd == nil { + t.Fatal("asking sent nothing") + } + cmd() + req := m.API.(*fakeAPI).request + if req.Action != "ask" || req.Node != m.nodeID() || req.Message != "is there a PR?" || req.Recipient != "" { + t.Fatalf("not sent as a question to this stage's supervisor: %+v", req) + } +} + +func TestTabSwitchesAMessageBetweenSteeringAndAsking(t *testing.T) { + m := modelFixture(t) + m.Width, m.Height = 120, 40 + m, _ = key(m, "i") + if m.Mode != "chat" || !strings.Contains(m.View().Content, "changes the work") { + t.Fatalf("i did not open the steer composer: mode %q", m.Mode) + } + m, _ = key(m, "tab") + if m.Mode != "ask" { + t.Fatalf("tab did not switch to asking: %q", m.Mode) + } + m, _ = key(m, "tab") + if m.Mode != "chat" { + t.Fatalf("tab did not switch back to steering: %q", m.Mode) + } +} diff --git a/internal/tui/decisions.go b/internal/tui/decisions.go index 6551020..9346b13 100644 --- a/internal/tui/decisions.go +++ b/internal/tui/decisions.go @@ -35,6 +35,10 @@ func decisions(r *workflow.Run) []decision { } else if parent := r.Revision(rev.Parent); parent != nil { out = append(out, decision{At: rev.CreatedAt, Stage: rewoundTo(rev), Actor: "you", Reason: rewindReason(parent, rev)}) } + // Plan's accepted result is where the scope of the work was decided. + if cp, ok := rev.Checkpoints[plan]; ok && cp.Result.Summary != "" && rev.Attempt(cp.Attempt) != nil { + out = append(out, decision{At: cp.CreatedAt, Stage: plan, Actor: "worker", Reason: "scoped the work: " + firstLine(cp.Result.Summary)}) + } for _, req := range rev.DiscoveredRequirements { out = append(out, decision{ At: planTime(rev, plan), Stage: plan, Actor: "worker", @@ -66,13 +70,23 @@ func decisions(r *workflow.Run) []decision { out = append(out, decision{At: msg.CreatedAt, Stage: stage, Actor: "you", Reason: fmt.Sprintf("told the %s: %s [%s]", msg.Recipient, firstLine(msg.Body), rev.MessageStatus(msg))}) } - } - // The captain's log already records past needs-attention causes with their - // time, which a revision's current Recovery cannot: it only holds the - // latest one. It exists only when a tracker is configured. - for _, entry := range r.TrackerLog { - if entry.Kind == workflow.TrackerKindNeedsAttention { - out = append(out, decision{At: entry.Occurred, Stage: entry.Node, Actor: "coordinator", Reason: "needed attention: " + firstLine(entry.Detail)}) + for _, q := range rev.Questions { + outcome := "waiting for an answer" + switch q.State { + case workflow.QuestionAnswered: + outcome = "answered: " + firstLine(q.Answer) + case workflow.QuestionFailed: + outcome = "not answered: " + firstLine(q.Detail) + } + out = append(out, decision{At: q.CreatedAt, Stage: q.Node, Actor: approver(q.Asker), + Reason: fmt.Sprintf("asked the supervisor: %s [%s]", firstLine(q.Text), outcome)}) + } + // Notes are the decisions no other state keeps: ceiling stops, stall + // nudges and what publishing did. They replace reading needs-attention + // causes back from the captain's log, which exists only when a tracker + // is configured. + for _, n := range rev.Notes { + out = append(out, decision{At: n.At, Stage: n.Node, Actor: "coordinator", Reason: noteReason(n)}) } } slices.SortStableFunc(out, func(a, b decision) int { return a.At.Compare(b.At) }) @@ -84,6 +98,22 @@ func decisions(r *workflow.Run) []decision { return out } +func noteReason(n workflow.Note) string { + switch n.Kind { + case workflow.NoteTokenCeiling: + return "stopped at the token ceiling: " + firstLine(n.Detail) + case workflow.NoteAttemptBudget: + return "stopped retrying: " + firstLine(n.Detail) + case workflow.NoteStallNudge: + return firstLine(n.Detail) + case workflow.NotePublished: + return "opened the pull request: " + firstLine(n.Detail) + case workflow.NotePublishFailed: + return "could not publish: " + firstLine(n.Detail) + } + return firstLine(n.Detail) +} + // rewoundTo is the first stage the new revision has to run again: the one it // was rewound to. func rewoundTo(rev *workflow.Revision) string { @@ -167,37 +197,40 @@ func (m Model) decisionLog() string { return strings.Join(lines, "\n") } -// blocking lists what is stopping a run right now: the readiness problems -// Plan must resolve and any recovery in progress, for the revision and for the -// selected stage's branch runtime. These used to live on the Readiness tab, -// where they sat behind two keypresses on the one screen that matters when a -// run is stuck, so Chat now leads with them. It is empty when nothing blocks. -func blocking(rev *workflow.Revision, node string) []string { +// headerBlockers lists what is stopping a run right now that the run summary +// does not already show: the readiness problems Plan must resolve, and +// recovery on the selected stage's branch runtime. Revision recovery already +// has its own summary line. One line each, so a small terminal keeps room for +// the stage. +func headerBlockers(rev *workflow.Revision, node string) []string { var out []string - // One line each: on a small terminal a full list pushed the stage's own - // content off the screen, which is worse than a truncated alert. if problems := rev.ReadinessProblems(time.Now(), rev.Requirements()); len(problems) > 0 { out = append(out, fmt.Sprintf("⚠ Plan must resolve %s: %s", plural(len(problems), "readiness problem"), strings.Join(problems, "; "))) } - if rev.Recovery != nil { - out = append(out, recoveryLine("⚠ Needs attention", rev.Recovery)) - } if child := rev.ChildRuntimes[node]; child != nil && child.Recovery != nil { out = append(out, recoveryLine("⚠ "+node+"'s branch runtime needs attention", child.Recovery)) } - // The Services tab listed every service. Healthy ones are noise while - // following a run, but one that is down is often why a stage failed, so - // only those are kept. - var down []string + return out +} + +// serviceLine shows service states when there is a reason to: a preview is +// configured, so the services are what you came to look at, or one is down, +// which is often why a stage failed. Otherwise healthy services are noise. +func serviceLine(rev *workflow.Revision) string { + var all, down []string for _, svc := range rev.Runtime.Services { + all = append(all, svc.Name+" "+svc.State) if !healthy(svc.State) { - down = append(down, fmt.Sprintf("%s is %s", svc.Name, svc.State)) + down = append(down, svc.Name+" "+svc.State) } } - if len(down) > 0 { - out = append(out, "⚠ Services not healthy in the VM: "+strings.Join(down, "; ")) + switch { + case len(down) > 0: + return "⚠ Services not healthy: " + strings.Join(down, " · ") + case rev.Config.Preview != nil && len(all) > 0: + return "Services: " + strings.Join(all, " · ") } - return out + return "" } // healthy reads a Compose service state such as "running/healthy", diff --git a/internal/tui/model.go b/internal/tui/model.go index 5cdcb91..bd9e6d9 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -106,12 +106,12 @@ type openedMsg struct { err error } -// panels are the detail views. Services, Readiness and Graph were cut because -// they did not help follow or steer a run: the preview URL is already in the -// run summary, the readiness problems and recovery that can block a run now -// lead the Chat panel whenever they exist, and a stage's dependencies are -// shown on its Checkpoint. History became the Decision Log. -var panels = []string{"Chat", "Checkpoint", "Changes", "Tests", "Decision Log"} +// panels are the detail views. Services, Readiness, Graph and Checkpoint were +// cut because they did not help follow or steer a run. What was worth keeping +// moved: blocking readiness problems, recovery and service states to the run +// summary; a stage's dependencies, artifacts and approval into Chat. History +// became the Decision Log. +var panels = []string{"Chat", "Changes", "Tests", "Decision Log"} func New(api API, root string) Model { m := Model{API: api, Root: root, Width: 100, Height: 30, Recipient: "supervisor"} @@ -323,6 +323,14 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.Mode = "" m.Input = "" case "tab", "shift+tab": + // Flip a message between steering and asking without retyping + // it: the two are one keystroke apart but do different things. + switch m.Mode { + case "chat": + m.Mode = "ask" + case "ask": + m.Mode = "chat" + } if m.Mode == "new" && len(m.Workflows) > 1 { step := 1 if key == "shift+tab" { @@ -351,6 +359,9 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if mode == "chat" { return m, m.act(daemon.ActionRequest{Action: "message", Node: m.nodeID(), Recipient: m.Recipient, Message: body}) } + if mode == "ask" { + return m, m.act(daemon.ActionRequest{Action: "ask", Node: m.nodeID(), Message: body, Actor: "local"}) + } if mode == "rewind" { return m, m.act(daemon.ActionRequest{Action: "rewind", Node: m.nodeID(), Task: body}) } @@ -460,6 +471,8 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.Offset = max(0, m.Offset-max(1, m.Height/3)) case "i": m.beginInput("chat") + case "?": + m.beginInput("ask") case "n": m.beginInput("new") m.loadWorkflows() @@ -774,10 +787,17 @@ func (m Model) header(width int) []chrome { text := t.fg(t.warn()).Render(clean("Recovery (" + rev.Recovery.Phase + "): " + rev.Recovery.Detail)) rows = append(rows, chrome{text: ansi.Truncate(text, width, "…")}) } + // What blocks a run belongs where you look first, not on a tab. + for _, line := range headerBlockers(rev, m.nodeID()) { + rows = append(rows, chrome{text: ansi.Truncate(t.fg(t.warn()).Render(clean(line)), width, "…")}) + } if rev.Runtime.PreviewURL != "" { text := t.fg(t.success()).Render("Preview: ") + clean(rev.Runtime.PreviewURL) rows = append(rows, chrome{text: ansi.Truncate(text, width, "…"), optional: true}) } + if line := serviceLine(rev); line != "" { + rows = append(rows, chrome{text: ansi.Truncate(t.dim().Render(clean(line)), width, "…"), optional: true}) + } if pending, failed := r.TrackerLogCounts(); pending+failed > 0 { style := t.dim() if failed > 0 { @@ -808,7 +828,10 @@ func (m Model) footer(width int, compact bool) []string { label = "issue link (optional, Enter to skip)" } if m.Mode == "chat" { - label = "steer " + m.Recipient + " · changes the work" + label = "steer " + m.Recipient + " · changes the work · tab to ask instead" + } + if m.Mode == "ask" { + label = "ask " + m.nodeID() + "'s supervisor · does not change the work · tab to steer instead" } if compact || width < 24 { lines = append(lines, ansi.Truncate(t.bold().Render(label+" ")+input, width, "")) @@ -817,9 +840,9 @@ func (m Model) footer(width int, compact bool) []string { } } else { prompt := t.dim().Render("Steer ") + t.fg(t.accent()).Render(m.Recipient) + - t.dim().Render(" · i steer · n new · r rewind · a approve · c close · o artifact · d diff · p/P plugins · x cancel") + t.dim().Render(" · i steer · ? ask · n new · r rewind · a approve · c close · o artifact · d diff · p/P plugins · x cancel") if compact { - prompt = t.dim().Render("i steer · c close · o artifact") + prompt = t.dim().Render("i steer · ? ask · o artifact") } lines = append(lines, ansi.Truncate(prompt, width, "")) } @@ -908,63 +931,7 @@ func (m Model) details() string { node := m.nodeID() switch panels[m.Panel] { case "Chat": - agents := rev.Config.NodeAgents(node) - lines := blocking(rev, node) - // On an empty stage, what to do next matters more than which model is - // configured, and a small terminal only has room for a few lines. - if len(rev.Attempts) == 0 && len(rev.Messages) == 0 { - lines = append(lines, "No stage messages yet. Press i to steer the "+m.Recipient+".") - } - lines = append(lines, fmt.Sprintf("Worker model: %s · Supervisor model: %s", formatModelTUI(agents.Worker.Model), formatModelTUI(agents.Supervisor.Model))) - if len(rev.Config.Plugins) > 0 { - var attached []string - for _, p := range rev.Config.Plugins { - attached = append(attached, p.ID+"@"+p.Version) - } - lines = append(lines, "Plugins: "+strings.Join(attached, ", ")+" (p add or replace, P remove; reopens Plan)") - } - for _, a := range rev.Attempts { - if a.Node == node { - lines = append(lines, fmt.Sprintf("Worker attempt %d: %s", a.Number, a.State)) - if a.Error != "" { - lines = append(lines, a.Error) - } - if a.Result != nil { - lines = append(lines, a.Result.Summary, "Supervisor: "+a.Result.Review.Summary) - } - if p := a.Progress; a.State == "running" && p != nil { - live := "Live: " + p.Phase - if p.Generation > 0 { - live += fmt.Sprintf(" (resume %d)", p.Generation) - } - if p.Detail != "" { - live += " — " + p.Detail - } - live += " · updated " + p.UpdatedAt.Local().Format("15:04:05") - lines = append(lines, live+"\n "+strings.Join(p.Activity, "\n ")) - } - } - } - for _, msg := range rev.Messages { - if msg.Node == "" || msg.Node == node { - lines = append(lines, "Steer "+msg.Recipient+": "+msg.Body+"\n ["+rev.MessageStatus(msg)+"]") - } - } - return strings.Join(lines, "\n\n") - case "Checkpoint": - needs := "" - if deps := rev.Config.Workflow.Nodes[node].Needs; len(deps) > 0 { - needs = "Runs after: " + strings.Join(deps, ", ") + "\n\n" - } - if cp, ok := rev.Checkpoints[node]; ok { - return needs + checkpointSummary(cp, m.ArtifactIndex) - } - for _, a := range rev.Attempts { - if a.Node == node && a.State == "awaiting-approval" { - return needs + "Awaiting approval of this exact result (a to approve):\n" + pretty(a.Result) - } - } - return needs + "No accepted checkpoint for " + node + "." + return m.chat() case "Decision Log": return m.decisionLog() case "Changes": diff --git a/internal/tui/panels_test.go b/internal/tui/panels_test.go index 79c3f95..baa2196 100644 --- a/internal/tui/panels_test.go +++ b/internal/tui/panels_test.go @@ -22,50 +22,57 @@ func panel(t *testing.T, m Model, name string) Model { } func TestTheTabsAreCutDownToWhatHelpsReviewTheWork(t *testing.T) { - want := []string{"Chat", "Checkpoint", "Changes", "Tests", "Decision Log"} + want := []string{"Chat", "Changes", "Tests", "Decision Log"} if !slices.Equal(panels, want) { t.Fatalf("tabs: %v, want %v", panels, want) } } -func TestRemovingTheReadinessTabDidNotHideWhatBlocksARun(t *testing.T) { - // This was on Readiness, two keypresses from where you land. A run that - // cannot proceed has to say so on the first screen. - m := panel(t, modelFixture(t), "Chat") - m.Width, m.Height = 120, 40 - if !strings.Contains(m.View().Content, "harness.worker: not probed") { - t.Fatalf("a readiness problem is not visible on Chat:\n%s", m.View().Content) - } - rev := m.Runs[0].Current() - rev.Recovery = &workflow.Recovery{Phase: "publication", Detail: "GitHub rejected the push"} - if !strings.Contains(m.details(), "GitHub rejected the push") { - t.Fatalf("recovery is not visible on Chat:\n%s", m.details()) +func TestWhatBlocksARunIsInTheRunSummaryWhateverTabIsOpen(t *testing.T) { + // Readiness and recovery used to be on a tab of their own, two keypresses + // from where you land. They belong in the summary, visible from any tab. + for _, name := range panels { + m := panel(t, modelFixture(t), name) + m.Width, m.Height = 120, 40 + rev := m.Runs[0].Current() + rev.Recovery = &workflow.Recovery{Phase: "publication", Detail: "GitHub rejected the push"} + view := m.View().Content + if !strings.Contains(view, "harness.worker: not probed") || !strings.Contains(view, "GitHub rejected the push") { + t.Fatalf("a blocker is hidden on the %s tab:\n%s", name, view) + } } } -func TestOnlyUnhealthyServicesSurfaceNowTheServicesTabIsGone(t *testing.T) { - m := panel(t, modelFixture(t), "Chat") +func TestServiceStatesShowWhenThereIsAReasonToLook(t *testing.T) { + m := modelFixture(t) m.Width, m.Height = 120, 40 rev := m.Runs[0].Current() rev.Runtime = workflow.RuntimeState{ID: "vm", State: "running", Ready: true, PreviewURL: "http://127.0.0.1:41234/", Services: []workflow.Service{ {Name: "web", State: "running/healthy"}, {Name: "db", State: "exited"}, }} - details := m.details() - if !strings.Contains(details, "db is exited") { - t.Fatalf("a down service is hidden:\n%s", details) + // A service that is down is shown, because it is often why a stage failed. + view := m.View().Content + if !strings.Contains(view, "db exited") { + t.Fatalf("a down service is hidden:\n%s", view) } - if strings.Contains(details, "web is") { - t.Fatalf("a healthy service is listed as a problem:\n%s", details) - } - // The preview URL was the Services tab's other job; the header keeps it. - if !strings.Contains(m.View().Content, "Preview: http://127.0.0.1:41234/") { + if !strings.Contains(view, "Preview: http://127.0.0.1:41234/") { t.Fatal("the preview URL is no longer shown anywhere") } + // All healthy and no preview configured: nothing worth a line. + rev.Runtime.Services[1].State = "running/healthy" + if view = m.View().Content; strings.Contains(view, "web running/healthy") { + t.Fatalf("healthy services shown with no preview to look at:\n%s", view) + } + // With a preview configured, the services are what you came to look at. + rev.Config.Preview = &workflow.Preview{Service: "web", Port: 8080} + if view = m.View().Content; !strings.Contains(view, "web running/healthy") { + t.Fatalf("services hidden although a preview is configured:\n%s", view) + } } -func TestACheckpointSaysWhichStagesItRunsAfter(t *testing.T) { +func TestChatSaysWhichStagesTheSelectedStageRunsAfter(t *testing.T) { // Graph was removed; a stage's dependencies are the part worth keeping. - m := panel(t, modelFixture(t), "Checkpoint") + m := panel(t, modelFixture(t), "Chat") m.Node = slices.Index(orderOf(m), "plan") if !strings.Contains(m.details(), "Runs after: task") { t.Fatalf("dependencies not shown:\n%s", m.details()) @@ -169,3 +176,40 @@ func TestADecisionLogSaysWhatARewindRerunAndWhatChanged(t *testing.T) { t.Fatalf("objective change not explained:\n%s", got) } } + +func TestTheDecisionLogIncludesQuestionsAndTheCoordinatorsStops(t *testing.T) { + m := modelFixture(t) + r := &m.Runs[0] + rev := r.Current() + base := rev.CreatedAt + at := func(min int) time.Time { return base.Add(time.Duration(min) * time.Minute) } + rev.Questions = []workflow.Question{ + {ID: "q1", Node: "code", Asker: "local", Text: "is there a PR open?", State: workflow.QuestionAnswered, Answer: "not yet", CreatedAt: at(1)}, + {ID: "q2", Node: "code", Text: "why?", State: workflow.QuestionFailed, Detail: "VM released", CreatedAt: at(2)}, + } + rev.Notes = []workflow.Note{ + {At: at(3), Kind: workflow.NoteStallNudge, Node: "code", Detail: "nudged the worker after a silence (nudge 1 of 2)"}, + {At: at(4), Kind: workflow.NoteAttemptBudget, Node: "qa", Detail: "stage qa exhausted its configured 3 attempts"}, + {At: at(5), Kind: workflow.NoteTokenCeiling, Detail: "run counted 20.0M of its 20.0M token ceiling"}, + {At: at(6), Kind: workflow.NotePublishFailed, Detail: "base branch main moved"}, + {At: at(7), Kind: workflow.NotePublished, Node: "approved-change", Detail: "app: https://github.com/o/r/pull/7"}, + } + joined := "" + for _, d := range decisions(r) { + joined += d.Stage + " | " + d.Actor + ": " + d.Reason + "\n" + } + for _, want := range []string{ + "code | you: asked the supervisor: is there a PR open? [answered: not yet]", + // A question with no recorded asker, as from MCP, is still yours. + "code | you: asked the supervisor: why? [not answered: VM released]", + "code | coordinator: nudged the worker after a silence (nudge 1 of 2)", + "qa | coordinator: stopped retrying: stage qa exhausted its configured 3 attempts", + "| coordinator: stopped at the token ceiling: run counted 20.0M of its 20.0M token ceiling", + "| coordinator: could not publish: base branch main moved", + "approved-change | coordinator: opened the pull request: app: https://github.com/o/r/pull/7", + } { + if !strings.Contains(joined, want) { + t.Fatalf("missing %q in:\n%s", want, joined) + } + } +} diff --git a/internal/tui/progress_test.go b/internal/tui/progress_test.go index 770e65c..1f103a0 100644 --- a/internal/tui/progress_test.go +++ b/internal/tui/progress_test.go @@ -23,7 +23,7 @@ func TestConversationShowsLiveProgressAndSteeringStatus(t *testing.T) { {ID: "msg_waiting", Node: node, Recipient: "worker", Body: "add a changelog entry"}, } view := m.View().Content - for _, want := range []string{"Live: worker (resume 1)", "ran (exit 0): pytest", "included when " + node + " worker attempt 1 started", "delivered live to " + node + " worker attempt 1 (resume 1)", "pending live delivery to " + node + " attempt 1"} { + for _, want := range []string{"live: worker (resume 1)", "ran (exit 0): pytest", "included when " + node + " worker attempt 1 started", "delivered live to " + node + " worker attempt 1 (resume 1)", "pending live delivery to " + node + " attempt 1"} { if !strings.Contains(view, want) { t.Fatalf("conversation lacks %q:\n%s", want, view) } @@ -36,7 +36,7 @@ func TestConversationShowsLiveProgressAndSteeringStatus(t *testing.T) { t.Fatal("stall watch not shown in the conversation:\n" + view) } rev.Attempts[0].State = "failed" - if view = m.View().Content; strings.Contains(view, "Live: worker") || !strings.Contains(view, "queued for the next "+node+" attempt") { + if view = m.View().Content; strings.Contains(view, "live: worker") || !strings.Contains(view, "queued for the next "+node+" attempt") { t.Fatal("finished attempt still shows live progress or lost the waiting message:\n" + view) } } diff --git a/internal/tui/review.go b/internal/tui/review.go index 598f3f0..c75eee0 100644 --- a/internal/tui/review.go +++ b/internal/tui/review.go @@ -3,8 +3,6 @@ package tui import ( "encoding/json" "fmt" - "sort" - "strings" "github.com/sam-bretz/envctl/internal/workflow" ) @@ -113,62 +111,6 @@ func (m Model) selectedCheckpoint() (workflow.Checkpoint, bool) { } return workflow.Checkpoint{}, false } -func checkpointSummary(cp workflow.Checkpoint, selected int) string { - lines := []string{cp.Node + " · " + cp.ID, cp.Result.Summary, "", "Supervisor: " + cp.Result.Review.Summary} - if cp.HistoricalOnly { - lines = append(lines, "Historical only: this output cannot satisfy a new workflow dependency.") - } - if cp.Approval != nil { - lines = append(lines, "Approved by "+cp.Approval.Actor+" at "+cp.Approval.At.Format("2006-01-02 15:04:05 MST"), "Approved work: "+cp.Approval.ResultDigest) - } - for _, requirement := range cp.Result.Requirements { - lines = append(lines, fmt.Sprintf("Requirement %s -> %s: %s", requirement.Capability, strings.Join(requirement.Nodes, ", "), requirement.Reason)) - } - lines = append(lines, "", "Artifacts (,/. select; o open):") - for i, a := range cp.Result.Artifacts { - marker := " " - if i == selected { - marker = ">" - } - lines = append(lines, fmt.Sprintf("%s %s · %s · %d bytes", marker, a.Name, a.MediaType, a.Size)) - } - lines = append(lines, "", "Repository commits:") - keys := make([]string, 0, len(cp.Result.Commits)) - for k := range cp.Result.Commits { - keys = append(keys, k) - } - sort.Strings(keys) - for _, k := range keys { - lines = append(lines, k+": "+cp.Result.Commits[k]) - parents := make([]string, 0, len(cp.Result.MergeParents[k])) - for parent := range cp.Result.MergeParents[k] { - parents = append(parents, parent) - } - sort.Strings(parents) - for _, parent := range parents { - lines = append(lines, " Merge input "+parent+": "+cp.Result.MergeParents[k][parent]) - } - if objects, ok := cp.Result.SourceObjects[k]; ok { - lines = append(lines, fmt.Sprintf(" Submodule/LFS objects: %s (%d bytes)", objects.Digest, objects.Size)) - } - if link := cp.Result.PRs[k]; link != "" { - lines = append(lines, " PR: "+link) - } - } - if len(cp.Result.Datasets) > 0 { - lines = append(lines, "", "Datasets: "+cp.Result.DatasetDigest) - keys = nil - for k := range cp.Result.Datasets { - keys = append(keys, k) - } - sort.Strings(keys) - for _, k := range keys { - d := cp.Result.Datasets[k] - lines = append(lines, fmt.Sprintf("%s · %s · %s\n Snapshot: %s (%d bytes)\n Verification: %s", k, d.Adapter, d.ToolVersion, d.Source.Digest, d.Source.Size, d.Evidence.Digest)) - } - } - return strings.Join(lines, "\n") -} func artifactPreview(raw []byte) string { var value any if json.Unmarshal(raw, &value) == nil { diff --git a/internal/tui/review_test.go b/internal/tui/review_test.go index 2c1d3ce..13af683 100644 --- a/internal/tui/review_test.go +++ b/internal/tui/review_test.go @@ -17,7 +17,7 @@ func TestRevisionReviewIsIndependentAndReadOnly(t *testing.T) { t.Fatal(err) } r.Current().Checkpoints["task"] = workflow.Checkpoint{ID: "cp_new", Node: "task", Result: workflow.Result{Summary: "revised task"}} - m.Panel = 1 + m = panel(t, m, "Chat") m, cmd := key(m, "[") if cmd != nil || m.viewRevision().ID != old || !strings.Contains(m.details(), "original task") { t.Fatal("history did not select the old checkpoint") diff --git a/internal/workflow/notes.go b/internal/workflow/notes.go new file mode 100644 index 0000000..590b56a --- /dev/null +++ b/internal/workflow/notes.go @@ -0,0 +1,35 @@ +package workflow + +import "time" + +// Note kinds. +const ( + NoteTokenCeiling = "token-ceiling" + NoteAttemptBudget = "attempt-budget" + NoteStallNudge = "stall-nudge" + NotePublished = "published" + NotePublishFailed = "publish-failed" +) + +// Note records a coordinator decision that no other run state keeps. Recovery +// holds only the latest cause and is cleared when a runtime is released, so a +// run that stopped at its token ceiling, nudged a stalled agent or failed to +// publish would otherwise lose that history. The Decision Log reads these. +type Note struct { + At time.Time `json:"at"` + Kind string `json:"kind"` + Node string `json:"node,omitempty"` + Detail string `json:"detail"` +} + +// AddNote appends a note unless an identical one is already recorded, so a +// decision reported on every poll is written once. +func (r *Revision) AddNote(note Note) bool { + for _, n := range r.Notes { + if n.Kind == note.Kind && n.Node == note.Node && n.Detail == note.Detail && n.At.Equal(note.At) { + return false + } + } + r.Notes = append(r.Notes, note) + return true +} diff --git a/internal/workflow/questions.go b/internal/workflow/questions.go new file mode 100644 index 0000000..7a40e67 --- /dev/null +++ b/internal/workflow/questions.go @@ -0,0 +1,89 @@ +package workflow + +import ( + "errors" + "fmt" + "strings" + "time" +) + +// Question states. A question is pending until the coordinator starts +// answering it, and ends answered or failed. +const ( + QuestionPending = "pending" + QuestionAnswering = "answering" + QuestionAnswered = "answered" + QuestionFailed = "failed" +) + +// MaxQuestionLength bounds a question, which is placed into an agent prompt. +const MaxQuestionLength = 4000 + +// Question is something a person asked a stage's supervisor. It is answered +// by a separate, read-only harness invocation, so asking never interrupts or +// alters the work it asks about. +type Question struct { + ID string `json:"id"` + // Node and Attempt are the stage and the attempt whose supervisor + // answers: the latest attempt of that stage when the question was asked. + Node string `json:"node"` + Attempt string `json:"attempt"` + Text string `json:"text"` + Asker string `json:"asker,omitempty"` + CreatedAt time.Time `json:"created_at"` + State string `json:"state"` + Answer string `json:"answer,omitempty"` + Detail string `json:"detail,omitempty"` + AnsweredAt time.Time `json:"answered_at,omitzero"` + // Usage is what answering cost. It counts toward the run's token ceiling + // and is attributed to the stage through Node. + Usage *Usage `json:"usage,omitempty"` +} + +// Finished reports whether a question has reached a final state. +func (q Question) Finished() bool { return q.State == QuestionAnswered || q.State == QuestionFailed } + +// Ask records a question for a stage's supervisor on the current revision. +func (r *Run) Ask(revision, node, text, asker string, now time.Time) (string, error) { + if revision != r.CurrentRevision { + return "", errors.New("questions go to the current revision; an older one no longer has a running supervisor") + } + rev := r.Current() + if _, ok := rev.Config.Workflow.Nodes[node]; !ok { + return "", fmt.Errorf("workflow has no stage %q", node) + } + text = strings.TrimSpace(text) + if text == "" { + return "", errors.New("ask a question") + } + if len(text) > MaxQuestionLength { + return "", fmt.Errorf("a question is at most %d characters", MaxQuestionLength) + } + // Answering spends tokens, so a run already at its ceiling cannot ask: + // the answer would go over a limit the person set. + if exceeded, reason := r.Budget(); exceeded { + return "", fmt.Errorf("the run has reached its ceiling (%s), and answering would spend more", reason) + } + attempt := "" + for _, a := range rev.Attempts { + if a.Node == node { + attempt = a.ID + } + } + if attempt == "" { + return "", fmt.Errorf("%s has not started, so there is nothing to ask its supervisor about yet", node) + } + id := ID("question") + rev.Questions = append(rev.Questions, Question{ID: id, Node: node, Attempt: attempt, Text: text, Asker: asker, CreatedAt: now, State: QuestionPending}) + return id, nil +} + +// Question returns a question by ID. +func (r *Revision) Question(id string) *Question { + for i := range r.Questions { + if r.Questions[i].ID == id { + return &r.Questions[i] + } + } + return nil +} diff --git a/internal/workflow/questions_test.go b/internal/workflow/questions_test.go new file mode 100644 index 0000000..01dc39d --- /dev/null +++ b/internal/workflow/questions_test.go @@ -0,0 +1,97 @@ +package workflow + +import ( + "strings" + "testing" + "time" +) + +func questionRun(t *testing.T) *Run { + t.Helper() + c, err := Parse([]byte("version: 2\nproject: demo\nrepositories: [{id: app, url: /source}]\nworkflow: {template: feature}\n")) + if err != nil { + t.Fatal(err) + } + r, err := NewRun("demo", "ship it", "dev", c, time.Now()) + if err != nil { + t.Fatal(err) + } + return r +} + +func TestAQuestionGoesToTheLatestAttemptOfItsStage(t *testing.T) { + r := questionRun(t) + rev := r.Current() + rev.Attempts = []Attempt{{ID: "a1", Node: "code", Number: 1, State: "failed"}, {ID: "a2", Node: "code", Number: 2, State: "running"}} + id, err := r.Ask(r.CurrentRevision, "code", " is there a PR open for this? ", "sam", time.Now()) + if err != nil { + t.Fatal(err) + } + q := rev.Question(id) + if q == nil || q.Attempt != "a2" || q.State != QuestionPending || q.Text != "is there a PR open for this?" { + t.Fatalf("question not recorded against the latest attempt: %+v", q) + } +} + +func TestAQuestionIsRefusedWhenItCannotBeAnswered(t *testing.T) { + r := questionRun(t) + r.Current().Attempts = []Attempt{{ID: "a1", Node: "code", Number: 1, State: "running"}} + cases := map[string]func() error{ + "an unknown stage": func() error { _, err := r.Ask(r.CurrentRevision, "nope", "why?", "", time.Now()); return err }, + "a stage not yet begun": func() error { _, err := r.Ask(r.CurrentRevision, "qa", "why?", "", time.Now()); return err }, + "an empty question": func() error { _, err := r.Ask(r.CurrentRevision, "code", " ", "", time.Now()); return err }, + "an old revision": func() error { _, err := r.Ask("rev_old", "code", "why?", "", time.Now()); return err }, + "an oversized question": func() error { + _, err := r.Ask(r.CurrentRevision, "code", strings.Repeat("x", MaxQuestionLength+1), "", time.Now()) + return err + }, + } + for name, ask := range cases { + if err := ask(); err == nil { + t.Fatalf("accepted %s", name) + } + } + if len(r.Current().Questions) != 0 { + t.Fatalf("a refused question was recorded: %+v", r.Current().Questions) + } +} + +func TestAQuestionIsRefusedOnceTheRunIsAtItsTokenCeiling(t *testing.T) { + r := questionRun(t) + rev := r.Current() + rev.Config.Limits.RunTokens = 1000 + rev.Attempts = []Attempt{{ID: "a1", Node: "code", Number: 1, State: "running", Usage: &Usage{Input: 5000}}} + _, err := r.Ask(r.CurrentRevision, "code", "why?", "", time.Now()) + if err == nil || !strings.Contains(err.Error(), "ceiling") { + t.Fatalf("a question was accepted at the token ceiling: %v", err) + } +} + +func TestAnsweringAQuestionCountsTowardTheRunsTokens(t *testing.T) { + r := questionRun(t) + rev := r.Current() + rev.Attempts = []Attempt{{ID: "a1", Node: "code", Number: 1, State: "running", Usage: &Usage{Input: 100}}} + before := r.Usage().Tokens() + rev.Questions = []Question{{ID: "q1", Node: "code", Attempt: "a1", State: QuestionAnswered, Usage: &Usage{Input: 40, Output: 10}}} + if got := r.Usage().Tokens() - before; got != 50 { + t.Fatalf("an answer added %d tokens to the run, want 50", got) + } +} + +func TestQuestionsLeaveTheWorkAndItsApprovalUnchanged(t *testing.T) { + // Questions live beside the attempt, never inside its result, so the work + // digest an approval binds to cannot move when one is asked or answered. + r := questionRun(t) + rev := r.Current() + result := &Result{Summary: "built it", Commits: map[string]string{"app": strings.Repeat("a", 40)}} + rev.Attempts = []Attempt{{ID: "a1", Node: "code", Number: 1, State: "awaiting-approval", Result: result}} + bound := result.WorkDigest() + if _, err := r.Ask(r.CurrentRevision, "code", "why this design?", "", time.Now()); err != nil { + t.Fatal(err) + } + rev.Questions[0].State, rev.Questions[0].Answer = QuestionAnswered, "because" + a := rev.Attempt("a1") + if a.State != "awaiting-approval" || a.Result.WorkDigest() != bound { + t.Fatalf("a question changed the attempt: state %s", a.State) + } +} diff --git a/internal/workflow/state.go b/internal/workflow/state.go index f871fdf..6dd22c1 100644 --- a/internal/workflow/state.go +++ b/internal/workflow/state.go @@ -116,24 +116,31 @@ func (r *Run) TrackerLogCounts() (pending, failed int) { } type Revision struct { - ID string `json:"id"` - Parent string `json:"parent,omitempty"` - FromCheckpoint string `json:"from_checkpoint,omitempty"` - Objective string `json:"objective"` - Config Config `json:"config"` - State string `json:"state"` - Runtime RuntimeState `json:"runtime"` - ChildRuntimes map[string]*ChildRuntime `json:"child_runtimes,omitempty"` - Readiness []Probe `json:"readiness"` - Attempts []Attempt `json:"attempts"` - Checkpoints map[string]Checkpoint `json:"checkpoints"` - Messages []Message `json:"messages"` - CreatedAt time.Time `json:"created_at"` - Recovery *Recovery `json:"recovery,omitempty"` - SourcePins map[string]string `json:"source_pins,omitempty"` - ReadinessCheckedAt time.Time `json:"readiness_checked_at,omitempty"` - DrainNodes []string `json:"drain_nodes,omitempty"` - DiscoveredRequirements []Requirement `json:"discovered_requirements,omitempty"` + ID string `json:"id"` + Parent string `json:"parent,omitempty"` + FromCheckpoint string `json:"from_checkpoint,omitempty"` + Objective string `json:"objective"` + Config Config `json:"config"` + State string `json:"state"` + Runtime RuntimeState `json:"runtime"` + ChildRuntimes map[string]*ChildRuntime `json:"child_runtimes,omitempty"` + Readiness []Probe `json:"readiness"` + Attempts []Attempt `json:"attempts"` + Checkpoints map[string]Checkpoint `json:"checkpoints"` + Messages []Message `json:"messages"` + // Questions are asked of a stage's supervisor. Unlike Messages they never + // steer the work: answering one cannot change an attempt's result, + // review, digest or approval. + Questions []Question `json:"questions,omitempty"` + // Notes are coordinator decisions no other field keeps: ceiling stops, + // stall nudges and publication results. + Notes []Note `json:"notes,omitempty"` + CreatedAt time.Time `json:"created_at"` + Recovery *Recovery `json:"recovery,omitempty"` + SourcePins map[string]string `json:"source_pins,omitempty"` + ReadinessCheckedAt time.Time `json:"readiness_checked_at,omitempty"` + DrainNodes []string `json:"drain_nodes,omitempty"` + DiscoveredRequirements []Requirement `json:"discovered_requirements,omitempty"` // ProbeUsage is cumulative usage of real harness readiness probes, by role. ProbeUsage map[string]Usage `json:"probe_usage,omitempty"` } diff --git a/internal/workflow/usage.go b/internal/workflow/usage.go index 2edfa5f..9e31ee5 100644 --- a/internal/workflow/usage.go +++ b/internal/workflow/usage.go @@ -76,6 +76,11 @@ func (r *Run) Usage() Usage { for _, u := range v.ProbeUsage { total = total.Add(u) } + for _, q := range v.Questions { + if q.Usage != nil { + total = total.Add(*q.Usage) + } + } for _, a := range v.Attempts { switch { case a.Usage != nil: From 0d5a89109bc3548a43ac553732595573382dbae2 Mon Sep 17 00:00:00 2001 From: Sam Bretz Date: Wed, 16 Sep 2026 10:36:57 -0700 Subject: [PATCH 3/3] fix: open Changes by name when pressing d Removing the Checkpoint tab shifted every tab after it, and the diff key still set a fixed panel index, so pressing d to see a change opened Tests instead. Nothing asserted where d lands, so it passed the suite. The tab is now looked up by name. Refs #24 Co-Authored-By: Claude Opus 5 (1M context) --- internal/tui/model.go | 4 +++- internal/tui/panels_test.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/internal/tui/model.go b/internal/tui/model.go index bd9e6d9..a16d7fc 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -579,7 +579,9 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.Notice = "Comparison base: revision source pins" case "d": if _, ok := m.selectedCheckpoint(); ok { - m.Panel = 2 + // By name: a fixed index silently pointed at Tests once the + // Checkpoint tab was removed and the tabs shifted. + m.Panel = slices.Index(panels, "Changes") m.clearArtifact() m.DiffRequest = m.diffKey() api, id, request := m.API, m.current().ID, m.DiffRequest diff --git a/internal/tui/panels_test.go b/internal/tui/panels_test.go index baa2196..86d90cc 100644 --- a/internal/tui/panels_test.go +++ b/internal/tui/panels_test.go @@ -213,3 +213,13 @@ func TestTheDecisionLogIncludesQuestionsAndTheCoordinatorsStops(t *testing.T) { } } } + +func TestTheDiffKeyOpensChangesWhereverTheTabsAre(t *testing.T) { + m := modelFixture(t) + node := m.nodeID() + m.Runs[0].Current().Checkpoints[node] = workflow.Checkpoint{ID: "cp", Node: node} + m, _ = key(m, "d") + if got := panels[m.Panel]; got != "Changes" { + t.Fatalf("d opened %s, not Changes", got) + } +}