diff --git a/.claude/skills/hera/SKILL.md b/.claude/skills/hera/SKILL.md index f7e1a867..138fd7c7 100644 --- a/.claude/skills/hera/SKILL.md +++ b/.claude/skills/hera/SKILL.md @@ -71,9 +71,9 @@ they opt in. Once you're spawned/promoted per the bullets above, everything belo ## 3. The coordination tools All take `cwd`. `orchestrator` is optional with exactly one live binding and **required** with 2+. -Arg names below are exact — do not invent others. These nine cover bootstrap, messaging, and status; -the plan-DAG authoring/mutation tools live in the companion `hera-plan` skill (pointer at the end of -this section). +Arg names below are exact — do not invent others. These ten cover bootstrap, messaging, status, and +revive; the plan-DAG authoring/mutation tools live in the companion `hera-plan` skill (pointer at the +end of this section). ### Bootstrap / join @@ -148,6 +148,18 @@ this section). The gater treats a `failed` blocker as explicitly failed (no need to wait for session death). Coordinators/freelancers just update status. +- **`hera_revive(cwd, role_name, [orchestrator])`** — coordinator-only PULL-revive of one role you + coordinate. Reach for this when a role you spawned looks stuck — `hera_tree_updates`/`hera_status` + show no progress, especially after something like a session-supervisor restart (which SIGHUPs every + PTY it owns, leaving a worker dead or suspended). It inspects the role's live session and takes + exactly one action: a dead session is restarted in place; a live-but-genuinely-stuck session (idle, + NOT parked at a prompt) is kicked (stopped and resumed in place); anything else — busy, blocked on a + question, a live coordinator, or a kick already in flight — is left untouched and reported as such + (`skipped_busy` / `skipped_blocked_on_prompt` / `skipped_coordinator_live` / `skipped_restart_pending`). + This is **pull-only** — nothing calls it automatically, and it can never thrash a session that is + actually working or waiting on an answer, since it applies the identical idle+not-blocked gate the + TUI's own `Enter`-key revive uses. It targets a DIFFERENT role than your own (self-targeting errors). + - **`hera_tree_updates(cwd, [orchestrator], [since])`** — scan the caller's orchestrator **subtree** (nested sub-orchestrators included) for messages since a cursor. Returns **TLDR-only subject lines — no bodies** (capped at 200), plus a `next_cursor`. The cursor is stored **per-role** and auto-advances @@ -220,6 +232,9 @@ or using in-session sub-agents. doorbell line. - **Want whole-team state?** `hera_tree_updates(cwd=$PWD)`, then `hera_get_messages(ids=[…])` for the ones worth reading. +- **A role looks stuck (no progress, especially after a session-supervisor restart)?** Don't spawn a + duplicate worker on a hunch — try `hera_revive(cwd=$PWD, role_name=)` first. It's a safe, + idle+not-blocked-gated no-op if the role turns out to be fine, busy, or waiting on a question. - **How completion flows back:** a worker finishing sends a closing `hera_send(status="done", …)` — the synchronous status apply rolls its task to `in_review` + `ready_to_close`, visible in the rail. A worker that cannot complete sends `hera_send(status="failed", …)` — rolls to `in_review` WITHOUT diff --git a/README.md b/README.md index ac822cb2..3bf1ec06 100644 --- a/README.md +++ b/README.md @@ -598,6 +598,7 @@ If the recipient has a live agent session the daemon also writes a single notifi | `hera_inbox` | Fetch the caller role's unread messages (oldest first), cancel their pending pane deliveries, and mark them read. | | `hera_mark_read` | Mark a specific list of message IDs read and cancel their pending deliveries. | | `hera_status` | Set the caller role's status (`idle`/`working`/`blocked`/`done`/`failed`), mirrored to `task_meta`; `done` rolls the worker's task to in-review + `ready_to_close`; `failed` rolls to in-review without `ready_to_close`. Optional `handoff_note` (string) and `request_recycle` (bool) are accepted from **any hera-bound role kind** (coordinator, worker, or freelance): `handoff_note` is stamped to `task_meta` for the next recycle's seed prompt, `request_recycle=true` flags a pending [self-service recycle](#context-budget-stop-hook) that the daemon acts on once the session goes idle — for a coordinator this is driven by the `coord-hook` budget nudge, for a worker/freelance role only by a human-initiated rail `B` bounce. | +| `hera_revive` | Coordinator-only PULL-revive of one role the caller coordinates, by `role_name`. A dead session (no live process) restarts in place; a live-but-genuinely-stuck session (idle, not blocked on a prompt) is kicked (stopped and resumed in place) — the same safety gate the rail's `Enter`-key revive already enforces, so it can never thrash a session that's actually working or waiting on an answer. Anything else (busy, blocked on a question, a live coordinator, a kick already in flight) is left untouched and reported as such. Pull-only — nothing calls it automatically; a coordinator reaches for it when `hera_status`/`hera_tree_updates` show no progress. | | `hera_tree_updates` | Scan the caller's orchestrator subtree for messages since a per-role cursor; returns TLDR subject lines only and auto-advances the cursor. | | `hera_get_messages` | Fetch full message bodies by ID (after `hera_tree_updates`), scoped to the caller's orchestrator subtree. | | `hera_plan_node` | Author a single planned node under the caller's orchestrator (coordinator-only). Params: `name`, `kind` (`worker`\|`subcoord`, default `worker`), `prompt` (worker nodes) or `goal` (subcoord nodes — required; the goal handed to the spawned coordinator), optional `archetype` ([diligence profile](#diligence-profiles-model-tiering)) persisted on the node and copied onto the task it materializes. A `subcoord` node materializes as a distinct coordinator agent with its own task, worktree, and child orchestrator. | diff --git a/context/knowledge/gotchas/daemon-rpc.md b/context/knowledge/gotchas/daemon-rpc.md index d6ea02bb..9030978b 100644 --- a/context/knowledge/gotchas/daemon-rpc.md +++ b/context/knowledge/gotchas/daemon-rpc.md @@ -110,6 +110,7 @@ - **`agent.ReconcileStaleSessionsExcept(db, alive)` is the reusable primitive; `ReconcileStaleSessions` is now a thin `Except(db, nil)` wrapper.** It flips InProgress→InReview for every task EXCEPT those in `alive` and RETURNS the flipped IDs (the true orphans) so the caller signals exactly them. Nil `alive` ⇒ flips all (the in-process equivalent). `sendBounceSignals(db, ids)` is the extracted ARGUS_BOUNCED inner loop shared by `replayBounceSignals` (file-fed, OFF) and `reattachSupervised` (orphan-fed, ON). - **OFF mode is byte-identical to pre-P3 — proven by the unchanged `replayBounceSignals`/`writeLiveTasksFile` suite passing.** The flag gates the entire re-attach path via `d.supClient != nil`; no behavior change when the supervisor is disabled. - **A re-attached LIVE worker stranded in InReview is RESTORED to InProgress on reattach — `reattachSupervised` revives the live set, not just orphans (BUG-B).** `ReconcileStaleSessionsExcept` only handles the orphan direction (InProgress→InReview for tasks NOT alive); a live worker the supervisor confirms alive that is already parked in InReview (from a prior BUG-050 roll or an earlier reconcile) would otherwise stay mislabeled forever — across repeated bounces EVERY live worker drifts into InReview (0 in_progress on a busy daemon). So after the re-attach `Get` loop, `reattachSupervised` calls `db.ReviveHeraWorkerToInProgress(id)` for every task in `liveSet`. The same helper backs the TUI's in-place revive (`reviveHeraWorker` success branch → `App.reviveRestoreInProgress`, local `*db.DB` only; `--remote` defers to the local daemon's reattach). **`ReviveHeraWorkerToInProgress` is the exact inverse of `RollHeraWorkerToReview` and MUST refuse to un-roll a genuinely-finished worker** — it no-ops unless the task is worker-bound AND currently InReview AND NOT awaiting close-out, where "awaiting close-out" = `meta:hera.ready_to_close=true` (the done/clean-exit stamp) OR a terminal role-status (`done`/`failed`). That guard is what keeps #707 / BUG-050 intact: a done/failed worker with a still-idle-alive session stays InReview for coordinator close-out; only a non-terminal live worker flips back. DB status only, never touches the session, idempotent. +- **`hera_revive` (add-hera-revive) is a THIRD caller of `ReviveHeraWorkerToInProgress`, giving a coordinator (not just a human at the TUI) a way to PULL-revive a bound role.** The gating sequence — dead session (any role kind) restarts unconditionally; a LIVE coordinator is never auto-restarted; a live worker/freelance session only gets kicked when idle AND not blocked on a prompt AND no restart already pending — lives ONCE in `internal/hera.ReviveRole` (mirrors `RecycleCoord`'s architecture: pure function over narrow `ReviveStore`/`ReviveRunner` interfaces), wired daemon-side by `daemon.HeraReviveRunner`. **Deliberately NOT unified with the TUI's `Enter`-key revive** (`internal/tui/heraactions.go`'s `heraReattach`/`reviveHeraWorker`) — see `openspec/changes/archive/*-add-hera-revive/design.md` D3: the TUI's kick additionally resizes to the CURRENT PANE's dimensions (no such surface exists for a headless MCP caller, which instead preserves the session's existing PTY size) and is threaded through tview's `QueueUpdateDraw` model. Every individual check the TUI's inline version performs stays single-sourced regardless (`agent.BlockedOnPrompt`, `ReviveHeraWorkerToInProgress` itself, `SessionRunner.KickRerender`/`StartOrReattach`) — only the ~10-line ORDERING of those checks is expressed twice, a known and accepted residual overlap, not silent duplication. ## Session-supervisor (P4 — default ON + rollback) diff --git a/context/knowledge/index.md b/context/knowledge/index.md index 456c63cc..feb3dbea 100644 --- a/context/knowledge/index.md +++ b/context/knowledge/index.md @@ -4,7 +4,7 @@ Non-obvious invariants and gotchas, split by topic. Read the relevant file when | File | Topic | Bullets | | --- | --- | --- | -| [gotchas/daemon-rpc.md](gotchas/daemon-rpc.md) | Daemon lifecycle, RPC timeouts, reconciliation races, session resume, Claude /clear recapture, binary staleness (SHA-256 content hash, not mtime), self-update, launchd auto-start + PATH, stream Since offset, paste-boundary flush, *.test fork-bomb backstop, singleton flock, PR poller (eligibility, terminal-state skip, batched per-repo graphql w/ alias-safe ids + chunked keep-stale), evidence-based completion (ExitInfo.CleanExit predicate; reconcile→InReview never Complete), hera worker finish policy (BUG-050 RollHeraWorkerToReview), startup hera-binding reconciliation, session-supervisor P1–P4 (dark PTY-owner, daemon-as-client behind cfg.Supervisor.Enabled, re-attach on bounce, default ON + in-process rollback, #707 cache-vs-EOF relay race), callWithTimeout nil-rpc guard, TUI supervisor restart, go-install skew (doctor restart-vs-path-divergence, supervisor-checked-on-auto-start, ProtocolVersion 2→3 old-supervisor-unknown, double-confirm supervisor restart), revive-restores-in_progress (BUG-B ReviveHeraWorkerToInProgress, inverse of RollHeraWorkerToReview), host-suspend watchdog (ARGUS_HOST_SUSPENDED advisory note — wall-clock gap>3m between 30s ticks, unconditional not Hera-gated, sibling of sendBounceSignals, one-shot no-dedup baseline-before-loop, monotonic-strip required, advisory-only no state mutation), Claude Code's own background-session supervisor (orphaned-worker root cause: single-PID SIGTERM can never reach a session Claude Code itself detached to its per-user supervisor; `claude agents`/`claude stop` detection+fix SHIPPED via internal/claudeagents + Runner.Stop fire-and-forget reap, not a signal-scoping bug), doctor Stop-hook registration check (detect-missing-coord-hook: REGISTERED/NOT REGISTERED/UNKNOWN, advisory-only, never gates the binary-coherence exit code), resume-time session-ID recapture (agent.RefreshResumeSessionID mirrors the exit hook because hera workers idle/StreamLost never reach captureSessionIDPostExit; Claude-only; wired at reattachSupervised orphans + TUI startSession + REST resume/restart; idempotent, never blanks/fabricates), doctor diligence-profile-library check (add-doctor-profile-check: FOUND/NONE FOUND/UNKNOWN, library-existence-only not per-project binding, missing-dir vs unreadable-dir tri-state, advisory-only) | 108 | +| [gotchas/daemon-rpc.md](gotchas/daemon-rpc.md) | Daemon lifecycle, RPC timeouts, reconciliation races, session resume, Claude /clear recapture, binary staleness (SHA-256 content hash, not mtime), self-update, launchd auto-start + PATH, stream Since offset, paste-boundary flush, *.test fork-bomb backstop, singleton flock, PR poller (eligibility, terminal-state skip, batched per-repo graphql w/ alias-safe ids + chunked keep-stale), evidence-based completion (ExitInfo.CleanExit predicate; reconcile→InReview never Complete), hera worker finish policy (BUG-050 RollHeraWorkerToReview), startup hera-binding reconciliation, session-supervisor P1–P4 (dark PTY-owner, daemon-as-client behind cfg.Supervisor.Enabled, re-attach on bounce, default ON + in-process rollback, #707 cache-vs-EOF relay race), callWithTimeout nil-rpc guard, TUI supervisor restart, go-install skew (doctor restart-vs-path-divergence, supervisor-checked-on-auto-start, ProtocolVersion 2→3 old-supervisor-unknown, double-confirm supervisor restart), revive-restores-in_progress (BUG-B ReviveHeraWorkerToInProgress, inverse of RollHeraWorkerToReview), host-suspend watchdog (ARGUS_HOST_SUSPENDED advisory note — wall-clock gap>3m between 30s ticks, unconditional not Hera-gated, sibling of sendBounceSignals, one-shot no-dedup baseline-before-loop, monotonic-strip required, advisory-only no state mutation), Claude Code's own background-session supervisor (orphaned-worker root cause: single-PID SIGTERM can never reach a session Claude Code itself detached to its per-user supervisor; `claude agents`/`claude stop` detection+fix SHIPPED via internal/claudeagents + Runner.Stop fire-and-forget reap, not a signal-scoping bug), doctor Stop-hook registration check (detect-missing-coord-hook: REGISTERED/NOT REGISTERED/UNKNOWN, advisory-only, never gates the binary-coherence exit code), resume-time session-ID recapture (agent.RefreshResumeSessionID mirrors the exit hook because hera workers idle/StreamLost never reach captureSessionIDPostExit; Claude-only; wired at reattachSupervised orphans + TUI startSession + REST resume/restart; idempotent, never blanks/fabricates), doctor diligence-profile-library check (add-doctor-profile-check: FOUND/NONE FOUND/UNKNOWN, library-existence-only not per-project binding, missing-dir vs unreadable-dir tri-state, advisory-only), hera_revive coordinator PULL-revive (add-hera-revive: third ReviveHeraWorkerToInProgress caller, shared internal/hera.ReviveRole gate, deliberately not unified with the TUI's Enter-key revive) | 109 | | [gotchas/pty-terminal.md](gotchas/pty-terminal.md) | PTY sizing, x/vt emulator, ring buffer, replay cache, paint cache, lazyScreen, test concurrency, ESC-boundary alignment, live rebuild from log tail, monotonic firstByteOffset, scrollOffset clamp, waitLoop close-after-drain order, rerender gates (unchanged-cols, cache invalidation, blocked-on-prompt), OSC 0x9C-in-UTF8 strip filter, persistent preview emulator (PreviewVT reuse-via-RIS), plugin terminalpane cursor sync, alt-screen keyboard-scroll + scroll-mode-entry suppression (BUG-031, not just the wheel), scroll-past-window lazy extend (BUG-E), scroll replay authored-width emulate-clip (live-scroll corruption), dimension-change resize-in-place instead of lossy 8MB-tail rebuild (BUG-068, overlapping/garbled live-view corruption), ring-wrap exact-offset log catch-up instead of lossy 8MB-tail rebuild (BUG-073, BUG-068's ring-wrap sibling — reached by backgrounding a busy agent's pane, not resize), live incremental-feed atomic (raw,total) snapshot instead of two separate racy calls (BUG-075, TOCTOU race distinct from BUG-068/073/074 — reachable on an actively-streamed pane with no bind/resize at all, causes a duplicated recent phrase + a couple of dropped characters) | 65 | | [gotchas/ui-threading.md](gotchas/ui-threading.md) | tview thread safety, tick-goroutine rules, lazyScreen fill invariant, paste/input batching, tmux UX-tearing post-mortem (no Sync; 3 legit repair callsites), OnBranchChange log-only contract, EventFocus drift recovery, stderr/stdout-after-Init fd 2 guards, status-bar notice auto-expire (15s TTL, lazy revert via 1s tick, no Sync/timer), SetScreen swallows tcell Init() errors (no-ctty nil-tty EnableMouse panic, probeTerminal preflight guard), probeTerminal false-positive-on-every-real-terminal regression (tcell devTty.Close() nil-`f` → os.ErrInvalid, fix discards Close() err + probeTerminalDev pty-slave test seam) | 29 | | [gotchas/ci-gates.md](gotchas/ci-gates.md) | `make pre-pr` per-gate failure recipes (fmt-check, test-cover-gate floor, lint-pr new-from-rev, vuln stdlib continue-on-error, macOS PTY-exhaustion flake in internal/agent under full-suite -race) | 5 | diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index fb7e7a5b..620ac670 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -500,6 +500,24 @@ func (d *Daemon) heraSpawnWorker(in mcp.HeraSpawnInput) (*mcp.HeraSpawnResult, e return &mcp.HeraSpawnResult{Task: res.Task, Role: res.Role, Binding: res.Binding}, nil } +// heraReviveRole performs the PULL-revive gating+action (add-hera-revive) for +// the hera_revive MCP tool, injected via SetHeraReviver. The gating sequence +// lives in the shared hera.ReviveRole primitive (mirrors RecycleCoord's +// architecture); this method just supplies the real daemon.HeraReviveRunner +// adapter over d.db + d.runner. See design.md D3 for why the TUI's Enter-key +// revive (internal/tui/heraactions.go) is NOT routed through this same call — +// it keeps its own inline implementation, sharing every underlying primitive +// (agent.BlockedOnPrompt, db.ReviveHeraWorkerToInProgress, +// SessionRunner.KickRerender/StartOrReattach) but not the top-level orchestration. +func (d *Daemon) heraReviveRole(in mcp.HeraReviveInput) (string, error) { + rr := NewHeraReviveRunner(d.db, d.runner, d.cfgFn) + outcome, err := hera.ReviveRole(d.db, rr, in.TaskID, in.IsCoordinator) + if err != nil { + return "", err + } + return string(outcome), nil +} + // heraGaterMaterialize is the gater's Materializer adapter (add-hera-plan-substrate): // it binds + starts a pre-created planned role via the shared // agent.MaterializeHeraWorker primitive. The gater resolves project / base_branch / @@ -1081,6 +1099,7 @@ func (d *Daemon) Serve(sockPath string) error { // in place when the native service is wired, and is simply absent here. if cfg.Hera.Enabled { mcpSrv.SetHeraService(hera.New(d.db, d.notifier), d.db, d.heraSpawnWorker) + mcpSrv.SetHeraReviver(d.heraReviveRole) } mcpSrv.SetArtifactManager(d.db) mcpSrv.SetProfileResolver(d.db) diff --git a/internal/daemon/revive.go b/internal/daemon/revive.go new file mode 100644 index 00000000..12b59eeb --- /dev/null +++ b/internal/daemon/revive.go @@ -0,0 +1,113 @@ +package daemon + +import ( + "fmt" + + "github.com/drn/argus/internal/agent" + "github.com/drn/argus/internal/config" + "github.com/drn/argus/internal/db" + "github.com/drn/argus/internal/hera" + "github.com/drn/argus/internal/model" +) + +// HeraReviveRunner implements hera.ReviveRunner against the daemon's real +// session runner and DB (add-hera-revive). It is the production counterpart +// to internal/hera/revive_test.go's fakeReviveRunner, wired into the daemon's +// hera_revive MCP tool via Daemon.heraReviveRole. See +// openspec/changes/add-hera-revive/design.md D3 for why the TUI's Enter-key +// revive path (internal/tui/heraactions.go) keeps its own inline +// implementation rather than sharing this adapter: it additionally resizes to +// the current pane's dimensions (no such rendering surface exists here) and is +// threaded through tview's QueueUpdateDraw model. Every underlying check this +// adapter performs is nonetheless single-sourced with the TUI's version — +// agent.BlockedOnPrompt, db.ReviveHeraWorkerToInProgress, and +// agent.SessionRunner's KickRerender/StartOrReattach are the same functions +// either way. +type HeraReviveRunner struct { + database *db.DB + runner agent.SessionRunner + cfgFn func() config.Config +} + +// NewHeraReviveRunner builds the production hera.ReviveRunner. +func NewHeraReviveRunner(database *db.DB, runner agent.SessionRunner, cfgFn func() config.Config) *HeraReviveRunner { + return &HeraReviveRunner{database: database, runner: runner, cfgFn: cfgFn} +} + +var _ hera.ReviveRunner = (*HeraReviveRunner)(nil) + +// IsAlive reports whether taskID has a live session at all. +func (r *HeraReviveRunner) IsAlive(taskID string) bool { + sess := r.runner.Get(taskID) + return sess != nil && sess.Alive() +} + +// IsIdle reports whether taskID's live session is currently idle. +func (r *HeraReviveRunner) IsIdle(taskID string) bool { + sess := r.runner.Get(taskID) + return sess != nil && sess.IsIdle() +} + +// BlockedOnPrompt reports whether taskID's live session is idle AND parked at +// a user prompt (selection UI overlay or trailing question) — the signature a +// kick must never dismiss. Delegates to agent.BlockedOnPrompt, which reads the +// session's own in-process ring buffer; that read is correct here (unlike a +// TUI running in daemon-client mode) because the daemon process always owns +// the live ring regardless of whether any client has attached a stream. +func (r *HeraReviveRunner) BlockedOnPrompt(taskID string) bool { + sess := r.runner.Get(taskID) + if sess == nil || !sess.IsIdle() { + return false + } + return agent.BlockedOnPrompt(sess) +} + +// HasPendingRestart reports whether a kick/restart is already queued for +// taskID. +func (r *HeraReviveRunner) HasPendingRestart(taskID string) bool { + return r.runner.HasPendingRestart(taskID) +} + +// KickRerender stops and resumes taskID's live session in place, at its +// existing PTY dimensions — there is no rendering surface to fit here, unlike +// the TUI's Enter-key revive which also resizes to the current pane's width +// (doubling as its own BUG-074 size-drift fix). +func (r *HeraReviveRunner) KickRerender(taskID string) error { + task, err := r.database.Get(taskID) + if err != nil { + return fmt.Errorf("revive kick: load task %s: %w", taskID, err) + } + if task == nil { + return fmt.Errorf("revive kick: task %s not found", taskID) + } + sess := r.runner.Get(taskID) + if sess == nil { + return fmt.Errorf("revive kick: no live session for task %s", taskID) + } + cols, rows := sess.PTYSize() + return r.runner.KickRerender(task, r.cfgFn(), uint16(rows), uint16(cols)) //nolint:gosec // bounded by terminal cell count +} + +// RestartDead restarts a session with no live process, resuming via +// --session-id when the task carries one — mirrors handleRestartTask / +// handleResumeTask (internal/api/handlers.go), the same daemon-side +// dead-session restart the REST API already exposes. +func (r *HeraReviveRunner) RestartDead(taskID string) error { + task, err := r.database.Get(taskID) + if err != nil { + return fmt.Errorf("revive restart: load task %s: %w", taskID, err) + } + if task == nil { + return fmt.Errorf("revive restart: task %s not found", taskID) + } + cfg := r.cfgFn() + agent.RefreshResumeSessionID(r.database, task) + resume := task.SessionID != "" + sess, _, err := r.runner.StartOrReattach(task, cfg, 24, 80, resume) + if err != nil { + return fmt.Errorf("revive restart: start task %s: %w", taskID, err) + } + task.SetStatus(model.StatusInProgress) + task.AgentPID = sess.PID() + return r.database.Update(task) +} diff --git a/internal/daemon/revive_test.go b/internal/daemon/revive_test.go new file mode 100644 index 00000000..37fed597 --- /dev/null +++ b/internal/daemon/revive_test.go @@ -0,0 +1,211 @@ +package daemon + +import ( + "time" + + "testing" + + "github.com/drn/argus/internal/agent" + "github.com/drn/argus/internal/config" + "github.com/drn/argus/internal/db" + "github.com/drn/argus/internal/model" + "github.com/drn/argus/internal/testutil" +) + +// seedHeraReviveWorker creates a task + orchestrator + bound worker role for +// HeraReviveRunner tests, mirroring seedHeraRecycleCoordinator (recycle_test.go) +// against the same DB surface. +func seedHeraReviveWorker(t *testing.T, database *db.DB, worktree, mission string) (*model.Task, *db.HeraRole) { + t.Helper() + task := &model.Task{ + ID: "worker-task", + Name: "worker-task", + Status: model.StatusInProgress, + Project: "test-project", + Worktree: worktree, + Backend: "test", + } + testutil.NoError(t, database.Add(task)) + + orch, err := database.CreateHeraOrchestrator("myorch", "master") + testutil.NoError(t, err) + + role, _, err := database.CreateHeraRoleWithBinding(db.CreateHeraRoleInput{ + OrchestratorID: orch.ID, + Name: "worker-1", + Kind: db.HeraKindWorker, + ArgusProject: task.Project, + Prompt: mission, + }, task.ID, task.Worktree) + testutil.NoError(t, err) + + return task, role +} + +func TestHeraReviveRunner_IsAlive_NoSessionIsFalse(t *testing.T) { + database, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = database.Close() }) + + runner := agent.NewRunner(nil) + cfg := recycleTestConfig() + r := NewHeraReviveRunner(database, runner, func() config.Config { return cfg }) + + testutil.Equal(t, r.IsAlive("no-such-task"), false) +} + +func TestHeraReviveRunner_IsIdle_NoSessionIsFalse(t *testing.T) { + database, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = database.Close() }) + + runner := agent.NewRunner(nil) + cfg := recycleTestConfig() + r := NewHeraReviveRunner(database, runner, func() config.Config { return cfg }) + + testutil.Equal(t, r.IsIdle("no-such-task"), false) +} + +func TestHeraReviveRunner_BlockedOnPrompt_NoSessionIsFalse(t *testing.T) { + database, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = database.Close() }) + + runner := agent.NewRunner(nil) + cfg := recycleTestConfig() + r := NewHeraReviveRunner(database, runner, func() config.Config { return cfg }) + + testutil.Equal(t, r.BlockedOnPrompt("no-such-task"), false) +} + +func TestHeraReviveRunner_HasPendingRestart_NoneIsFalse(t *testing.T) { + database, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = database.Close() }) + + runner := agent.NewRunner(nil) + cfg := recycleTestConfig() + r := NewHeraReviveRunner(database, runner, func() config.Config { return cfg }) + + testutil.Equal(t, r.HasPendingRestart("no-such-task"), false) +} + +func TestHeraReviveRunner_KickRerender_UnknownTaskErrors(t *testing.T) { + database, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = database.Close() }) + + runner := agent.NewRunner(nil) + cfg := recycleTestConfig() + r := NewHeraReviveRunner(database, runner, func() config.Config { return cfg }) + + if err := r.KickRerender("no-such-task"); err == nil { + t.Fatal("expected an error for an unknown task") + } +} + +func TestHeraReviveRunner_KickRerender_NoLiveSessionErrors(t *testing.T) { + database, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = database.Close() }) + + task, _ := seedHeraReviveWorker(t, database, t.TempDir(), "mission") + + runner := agent.NewRunner(nil) // no session ever started + cfg := recycleTestConfig() + r := NewHeraReviveRunner(database, runner, func() config.Config { return cfg }) + + if err := r.KickRerender(task.ID); err == nil { + t.Fatal("expected an error for a task with no live session") + } +} + +// TestHeraReviveRunner_KickRerender_PreservesPTYSize_EndToEnd pins design.md +// D3's stated divergence from the TUI's Enter-key kick: a headless caller has +// no pane to fit, so KickRerender must preserve the session's EXISTING PTY +// size rather than resizing it. +func TestHeraReviveRunner_KickRerender_PreservesPTYSize_EndToEnd(t *testing.T) { + database, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = database.Close() }) + + task, _ := seedHeraReviveWorker(t, database, t.TempDir(), "mission") + + runner := agent.NewRunner(nil) + cfg := recycleTestConfig() + + sess1, err := runner.Start(task, cfg, 31, 97, false) + testutil.NoError(t, err) + t.Cleanup(runner.StopAll) + + r := NewHeraReviveRunner(database, runner, func() config.Config { return cfg }) + testutil.NoError(t, r.KickRerender(task.ID)) + + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if runner.HasPendingRestart(task.ID) { + time.Sleep(10 * time.Millisecond) + continue + } + newSess := runner.Get(task.ID) + if newSess != nil && newSess != sess1 && newSess.Alive() { + cols, rows := newSess.PTYSize() + testutil.Equal(t, cols, 97) + testutil.Equal(t, rows, 31) + return // success + } + time.Sleep(10 * time.Millisecond) + } + t.Fatal("timeout waiting for KickRerender to resurrect the session") +} + +func TestHeraReviveRunner_RestartDead_UnknownTaskErrors(t *testing.T) { + database, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = database.Close() }) + + runner := agent.NewRunner(nil) + cfg := recycleTestConfig() + r := NewHeraReviveRunner(database, runner, func() config.Config { return cfg }) + + if err := r.RestartDead("no-such-task"); err == nil { + t.Fatal("expected an error for an unknown task") + } +} + +// TestHeraReviveRunner_RestartDead_EndToEnd pins the dead-session revive path: +// a task with no live session is started fresh, and the task row is flipped +// to in_progress with the new PID recorded — mirroring handleRestartTask's +// REST behavior (internal/api/handlers.go), the daemon-side counterpart this +// adapter reuses in spirit. +func TestHeraReviveRunner_RestartDead_EndToEnd(t *testing.T) { + database, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = database.Close() }) + + task, _ := seedHeraReviveWorker(t, database, t.TempDir(), "mission") + task.Status = model.StatusInReview + testutil.NoError(t, database.Update(task)) + + runner := agent.NewRunner(nil) // no session ever started for this task + cfg := recycleTestConfig() + r := NewHeraReviveRunner(database, runner, func() config.Config { return cfg }) + + testutil.NoError(t, r.RestartDead(task.ID)) + t.Cleanup(runner.StopAll) + + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if sess := runner.Get(task.ID); sess != nil && sess.Alive() { + updated, err := database.Get(task.ID) + testutil.NoError(t, err) + testutil.Equal(t, updated.Status, model.StatusInProgress) + if updated.AgentPID == 0 { + t.Fatal("expected AgentPID to be recorded") + } + return // success + } + time.Sleep(10 * time.Millisecond) + } + t.Fatal("timeout waiting for RestartDead to start a fresh session") +} diff --git a/internal/hera/revive.go b/internal/hera/revive.go new file mode 100644 index 00000000..1adca3b3 --- /dev/null +++ b/internal/hera/revive.go @@ -0,0 +1,127 @@ +package hera + +import ( + "fmt" + + "github.com/drn/argus/internal/model" +) + +// ReviveOutcome enumerates what a hera_revive attempt actually did, per +// design.md (add-hera-revive). Every path is reported — there is no silent +// no-op. +type ReviveOutcome string + +const ( + // ReviveRestartedDead means the role's session had no live process; it was + // restarted in place (resuming via --session-id when the task carries one). + ReviveRestartedDead ReviveOutcome = "restarted_dead" + // ReviveKickedStuck means the role's session was alive, idle, and not + // blocked on a prompt (the "genuinely stuck" signature); it was kicked + // (stopped and resumed in place). + ReviveKickedStuck ReviveOutcome = "kicked_stuck" + // ReviveSkippedCoordinatorLive means the target is a live coordinator role + // — presumed operator-interactive, never auto-restarted. + ReviveSkippedCoordinatorLive ReviveOutcome = "skipped_coordinator_live" + // ReviveSkippedBusy means the role's session is alive and actively + // producing output — left untouched. + ReviveSkippedBusy ReviveOutcome = "skipped_busy" + // ReviveSkippedBlocked means the role's session is idle but parked at a + // user prompt (selection UI or trailing question) — left untouched so the + // pending question is never dismissed. + ReviveSkippedBlocked ReviveOutcome = "skipped_blocked_on_prompt" + // ReviveSkippedPending means a kick/restart is already queued for this + // task — left untouched to avoid a duplicate. + ReviveSkippedPending ReviveOutcome = "skipped_restart_pending" + // ReviveSkippedNoSessionID means the session is alive but the task has no + // session id to resume in place — left untouched. + ReviveSkippedNoSessionID ReviveOutcome = "skipped_no_session_id" +) + +// ReviveStore is the narrow DB surface ReviveRole needs. Satisfied by the +// real *db.DB. +type ReviveStore interface { + Get(taskID string) (*model.Task, error) + // ReviveHeraWorkerToInProgress is the single shared helper (see + // internal/db/hera.go) that restores a worker-bound task from in_review + // back to in_progress, unless it's awaiting coordinator close-out. + ReviveHeraWorkerToInProgress(taskID string) (bool, error) +} + +// ReviveRunner is the daemon-side seam for session liveness checks and the +// actual restart/kick mechanics — injected so ReviveRole's gating logic is +// testable without a real PTY. Mirrors RecycleRunner's shape (recycle.go). +type ReviveRunner interface { + // IsAlive reports whether taskID has a live session at all. + IsAlive(taskID string) bool + // IsIdle reports whether taskID's live session is idle (no recent + // output). Only meaningful when IsAlive is true. + IsIdle(taskID string) bool + // BlockedOnPrompt reports whether taskID's live session is idle AND + // parked at a user prompt. Only meaningful when IsIdle is true. + BlockedOnPrompt(taskID string) bool + // HasPendingRestart reports whether a kick/restart is already queued for + // taskID. + HasPendingRestart(taskID string) bool + // KickRerender stops and resumes taskID's live session in place. + KickRerender(taskID string) error + // RestartDead starts (or --session-id resumes) a session for a task with + // no live process. + RestartDead(taskID string) error +} + +// ReviveRole attempts a PULL-revive of the hera role bound to taskID, per +// design.md (add-hera-revive) D3. isCoordinator identifies the TARGET role's +// kind (not the caller's) — a live coordinator is presumed +// operator-interactive and is never auto-restarted, mirroring the TUI's +// Enter-key gate (internal/tui/heraactions.go's heraReattach/ +// reviveHeraWorker), whose individual checks (agent.BlockedOnPrompt, +// db.ReviveHeraWorkerToInProgress, SessionRunner.KickRerender/StartOrReattach) +// this function's ReviveRunner/ReviveStore implementations reuse rather than +// reimplement. +// +// A dead session (any role kind, including a coordinator) is always +// restarted. A live session is otherwise gated, in order: coordinator role → +// skip; no session id → skip; a kick already in flight → skip; busy → skip; +// blocked on a prompt → skip; otherwise → kick, then best-effort restore the +// task to in_progress (a restore failure does not fail the kick itself — +// mirrors reviveRestoreInProgress's soft-fail). +func ReviveRole(store ReviveStore, runner ReviveRunner, taskID string, isCoordinator bool) (ReviveOutcome, error) { + task, err := store.Get(taskID) + if err != nil { + return "", fmt.Errorf("revive: load task %s: %w", taskID, err) + } + if task == nil { + return "", fmt.Errorf("revive: task %s not found", taskID) + } + + if !runner.IsAlive(taskID) { + if err := runner.RestartDead(taskID); err != nil { + return "", fmt.Errorf("revive: restart dead session for task %s: %w", taskID, err) + } + return ReviveRestartedDead, nil + } + + if isCoordinator { + return ReviveSkippedCoordinatorLive, nil + } + if task.SessionID == "" { + return ReviveSkippedNoSessionID, nil + } + if runner.HasPendingRestart(taskID) { + return ReviveSkippedPending, nil + } + if !runner.IsIdle(taskID) { + return ReviveSkippedBusy, nil + } + if runner.BlockedOnPrompt(taskID) { + return ReviveSkippedBlocked, nil + } + + if err := runner.KickRerender(taskID); err != nil { + return "", fmt.Errorf("revive: kick stuck session for task %s: %w", taskID, err) + } + // Best-effort: the kick itself already succeeded; a worker left stranded + // in in_review (e.g. awaiting close-out) can still be closed manually. + _, _ = store.ReviveHeraWorkerToInProgress(taskID) + return ReviveKickedStuck, nil +} diff --git a/internal/hera/revive_test.go b/internal/hera/revive_test.go new file mode 100644 index 00000000..6b8e1b74 --- /dev/null +++ b/internal/hera/revive_test.go @@ -0,0 +1,222 @@ +package hera + +import ( + "errors" + "testing" + + "github.com/drn/argus/internal/db" + "github.com/drn/argus/internal/model" + "github.com/drn/argus/internal/testutil" +) + +// --- hera_revive (add-hera-revive) --- +// +// fakeReviveRunner records calls so tests can assert gating/ordering without a +// real PTY/session, mirroring fakeRecycleRunner's shape (recycle_test.go). +type fakeReviveRunner struct { + alive bool + idle bool + blocked bool + pending bool + restartErr error + kickErr error + restartCalled bool + kickCalled bool + restartTaskID string + kickTaskID string +} + +func (f *fakeReviveRunner) IsAlive(taskID string) bool { return f.alive } +func (f *fakeReviveRunner) IsIdle(taskID string) bool { return f.idle } +func (f *fakeReviveRunner) BlockedOnPrompt(taskID string) bool { return f.blocked } +func (f *fakeReviveRunner) HasPendingRestart(taskID string) bool { return f.pending } + +func (f *fakeReviveRunner) KickRerender(taskID string) error { + f.kickCalled = true + f.kickTaskID = taskID + return f.kickErr +} + +func (f *fakeReviveRunner) RestartDead(taskID string) error { + f.restartCalled = true + f.restartTaskID = taskID + return f.restartErr +} + +func TestReviveRole_TaskNotFound(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + _, err = ReviveRole(d, &fakeReviveRunner{}, "no-such-task", false) + if err == nil { + t.Fatal("expected an error for a missing task") + } +} + +func TestReviveRole_DeadSessionRestarted(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + task, _, _ := seedRecycleRole(t, d, db.HeraKindWorker, "orch", "worker-1", "/wt/w1", "argus/w1", "mission") + + runner := &fakeReviveRunner{alive: false} + outcome, err := ReviveRole(d, runner, task.ID, false) + testutil.NoError(t, err) + testutil.Equal(t, outcome, ReviveRestartedDead) + testutil.Equal(t, runner.restartCalled, true) + testutil.Equal(t, runner.restartTaskID, task.ID) + testutil.Equal(t, runner.kickCalled, false) +} + +func TestReviveRole_DeadSessionRestartFails(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + task, _, _ := seedRecycleRole(t, d, db.HeraKindWorker, "orch", "worker-1", "/wt/w1", "argus/w1", "mission") + + runner := &fakeReviveRunner{alive: false, restartErr: errors.New("boom")} + _, err = ReviveRole(d, runner, task.ID, false) + if err == nil { + t.Fatal("expected the restart failure to propagate") + } +} + +// deadCoordinator (any role kind) is restarted too — the coordinator-live +// gate below only applies to a LIVE session. +func TestReviveRole_DeadCoordinatorRestarted(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + task, _, _ := seedRecycleCoordinator(t, d, "orch", "/wt/coord", "argus/coord", "mission") + + runner := &fakeReviveRunner{alive: false} + outcome, err := ReviveRole(d, runner, task.ID, true) + testutil.NoError(t, err) + testutil.Equal(t, outcome, ReviveRestartedDead) + testutil.Equal(t, runner.restartCalled, true) +} + +func TestReviveRole_LiveCoordinatorNeverRevived(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + task, _, _ := seedRecycleCoordinator(t, d, "orch", "/wt/coord", "argus/coord", "mission") + testutil.NoError(t, d.Update(withSessionID(task, "sess-1"))) + + runner := &fakeReviveRunner{alive: true, idle: true} + outcome, err := ReviveRole(d, runner, task.ID, true) + testutil.NoError(t, err) + testutil.Equal(t, outcome, ReviveSkippedCoordinatorLive) + testutil.Equal(t, runner.kickCalled, false) + testutil.Equal(t, runner.restartCalled, false) +} + +func TestReviveRole_NoSessionID(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + task, _, _ := seedRecycleRole(t, d, db.HeraKindWorker, "orch", "worker-1", "/wt/w1", "argus/w1", "mission") + // task.SessionID is empty by default from seedRecycleRole. + + runner := &fakeReviveRunner{alive: true, idle: true} + outcome, err := ReviveRole(d, runner, task.ID, false) + testutil.NoError(t, err) + testutil.Equal(t, outcome, ReviveSkippedNoSessionID) + testutil.Equal(t, runner.kickCalled, false) +} + +func TestReviveRole_RestartPending(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + task, _, _ := seedRecycleRole(t, d, db.HeraKindWorker, "orch", "worker-1", "/wt/w1", "argus/w1", "mission") + testutil.NoError(t, d.Update(withSessionID(task, "sess-1"))) + + runner := &fakeReviveRunner{alive: true, idle: true, pending: true} + outcome, err := ReviveRole(d, runner, task.ID, false) + testutil.NoError(t, err) + testutil.Equal(t, outcome, ReviveSkippedPending) + testutil.Equal(t, runner.kickCalled, false) +} + +func TestReviveRole_BusySkipped(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + task, _, _ := seedRecycleRole(t, d, db.HeraKindWorker, "orch", "worker-1", "/wt/w1", "argus/w1", "mission") + testutil.NoError(t, d.Update(withSessionID(task, "sess-1"))) + + runner := &fakeReviveRunner{alive: true, idle: false} + outcome, err := ReviveRole(d, runner, task.ID, false) + testutil.NoError(t, err) + testutil.Equal(t, outcome, ReviveSkippedBusy) + testutil.Equal(t, runner.kickCalled, false) +} + +func TestReviveRole_BlockedOnPromptSkipped(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + task, _, _ := seedRecycleRole(t, d, db.HeraKindWorker, "orch", "worker-1", "/wt/w1", "argus/w1", "mission") + testutil.NoError(t, d.Update(withSessionID(task, "sess-1"))) + + runner := &fakeReviveRunner{alive: true, idle: true, blocked: true} + outcome, err := ReviveRole(d, runner, task.ID, false) + testutil.NoError(t, err) + testutil.Equal(t, outcome, ReviveSkippedBlocked) + testutil.Equal(t, runner.kickCalled, false) +} + +func TestReviveRole_KickedStuckRestoresInProgress(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + task, _, _ := seedRecycleRole(t, d, db.HeraKindWorker, "orch", "worker-1", "/wt/w1", "argus/w1", "mission") + task = withSessionID(task, "sess-1") + task.Status = model.StatusInReview + testutil.NoError(t, d.Update(task)) + + runner := &fakeReviveRunner{alive: true, idle: true, blocked: false} + outcome, err := ReviveRole(d, runner, task.ID, false) + testutil.NoError(t, err) + testutil.Equal(t, outcome, ReviveKickedStuck) + testutil.Equal(t, runner.kickCalled, true) + testutil.Equal(t, runner.kickTaskID, task.ID) + + got, err := d.Get(task.ID) + testutil.NoError(t, err) + testutil.Equal(t, got.Status, model.StatusInProgress) +} + +func TestReviveRole_KickFails(t *testing.T) { + d, err := db.OpenInMemory() + testutil.NoError(t, err) + t.Cleanup(func() { _ = d.Close() }) + + task, _, _ := seedRecycleRole(t, d, db.HeraKindWorker, "orch", "worker-1", "/wt/w1", "argus/w1", "mission") + testutil.NoError(t, d.Update(withSessionID(task, "sess-1"))) + + runner := &fakeReviveRunner{alive: true, idle: true, kickErr: errors.New("boom")} + _, err = ReviveRole(d, runner, task.ID, false) + if err == nil { + t.Fatal("expected the kick failure to propagate") + } +} + +// withSessionID returns task with SessionID set — a small helper since +// seedRecycleRole (recycle_test.go) does not set one, and the "alive" gate +// checks below need a non-empty SessionID to reach the idle/blocked checks. +func withSessionID(task *model.Task, sessionID string) *model.Task { + task.SessionID = sessionID + return task +} diff --git a/internal/mcp/hera.go b/internal/mcp/hera.go index b4d1bd2b..82cddecc 100644 --- a/internal/mcp/hera.go +++ b/internal/mcp/hera.go @@ -83,15 +83,17 @@ type HeraStore interface { RollHeraWorkerFailed(taskID string) (bool, error) } -// heraToolDefs contains the 16 hera_* tool schemas. The first 9 are ported +// heraToolDefs contains the 18 hera_* tool schemas. The first 9 are ported // verbatim from Hera's daemon.toolDefinitions() — same param names, // descriptions, and required lists as the external Hera daemon so agents have an // identical surface when running natively. hera_move (fix-hera-join-move-binding) // is native-only — the external daemon has no equivalent. The next 3 (hera_plan_node / // hera_block / hera_plan) are the native plan-DAG authoring tools // (add-hera-plan-substrate); they are coordinator-only like hera_spawn_worker. -// The last 3 (hera_plan_node_update / hera_unblock / hera_plan_node_cancel) are -// the plan-mutation verbs (make-hera-plan-living D5). +// The next 3 (hera_plan_node_update / hera_unblock / hera_plan_node_cancel) are +// the plan-mutation verbs (make-hera-plan-living D5). The last (hera_revive) is +// the coordinator-only PULL-revive tool (add-hera-revive); its gating logic +// lives in the shared internal/hera.ReviveRole primitive. var heraToolDefs = []Tool{ { Name: "hera_new_orchestrator", @@ -210,6 +212,19 @@ var heraToolDefs = []Tool{ "required": []string{"cwd", "status"}, }, }, + { + Name: "hera_revive", + Description: "PULL-revive a hera role this coordinator coordinates. If its session is dead (no live process) it is restarted in place (--session-id resume when the task has one). If it's alive but genuinely stuck (idle, NOT blocked on a user prompt) its session is stopped and resumed in place at its existing size. A live coordinator role, a busy (actively working) role, one parked at a question, or one with a restart already in flight is left untouched and reported as such — this can never thrash a session that is actually working or waiting on an answer. Coordinator-only. Use when hera_status/hera_tree_updates show no progress from a role — e.g. after a session-supervisor restart SIGHUPs its PTY. This is pull/on-demand: nothing calls this automatically.", + InputSchema: map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "cwd": map[string]interface{}{"type": "string", "description": "Caller's worktree path (use $PWD)"}, + "role_name": map[string]interface{}{"type": "string", "description": "Name of the role to revive, within the caller's orchestrator. Must not be the caller's own role."}, + "orchestrator": map[string]interface{}{"type": "string", "description": "(optional) Disambiguates when the calling task holds multiple live coordinator bindings"}, + }, + "required": []string{"cwd", "role_name"}, + }, + }, { Name: "hera_spawn_worker", Description: "Spawn a new born-bound worker task under this orchestrator. Caller must hold a live coordinator binding. Creates an argus task (worktree + session) and, transactionally, a worker role + binding pre-bound to it. An orientation prefix naming the coordinator + orchestrator is prepended to the prompt; the verbatim prompt is stored on the role. Defaults the project to the coordinator's own. Pass `model` to match the worker's model to its task complexity (e.g. a strong model for a hard refactor, a cheaper/faster one for mechanical work).", @@ -390,6 +405,17 @@ func (s *Server) SetHeraService(svc *hera.Service, store HeraStore, spawner Hera s.heraSpawn = spawner } +// SetHeraReviver wires the hera_revive MCP tool (add-hera-revive) to the +// daemon's shared PULL-revive primitive. A new, independent setter (rather +// than a fourth SetHeraService parameter) matching the Server's existing +// multi-setter pattern (SetClipboard, SetScheduleManager, ...). Must be +// called before ListenAndServe, like every other Set* method. reviver may be +// nil — hera_revive then returns a "revive not configured" error rather than +// panicking. +func (s *Server) SetHeraReviver(reviver HeraReviver) { + s.heraRevive = reviver +} + // heraEnabled returns true when the hera service is wired AND task management // is also enabled — caller resolution via cwd requires task management. func (s *Server) heraEnabled() bool { @@ -1480,6 +1506,102 @@ func (s *Server) toolHeraStatus(id interface{}, args json.RawMessage) *Response return toolResult(id, b.String()) } +// toolHeraRevive implements the hera_revive MCP tool (add-hera-revive): +// coordinator-only PULL-revive of one role the caller coordinates. All gating +// logic (dead vs. stuck vs. skip) lives in the shared internal/hera.ReviveRole +// primitive, invoked here via s.heraRevive (wired by the daemon). +func (s *Server) toolHeraRevive(id interface{}, args json.RawMessage) *Response { + if !s.heraEnabled() { + return toolError(id, "hera not configured") + } + if s.heraRevive == nil { + return toolError(id, "hera revive not configured (daemon did not wire a reviver)") + } + var p struct { + Cwd string `json:"cwd"` + RoleName string `json:"role_name"` + Orchestrator string `json:"orchestrator"` + } + json.Unmarshal(args, &p) //nolint:errcheck + + if p.Cwd == "" { + return toolError(id, "cwd is required") + } + roleName := strings.TrimSpace(p.RoleName) + if roleName == "" { + return toolError(id, "role_name is required") + } + + caller, err := s.resolveCallerRole(p.Cwd, p.Orchestrator) + if err != nil { + return toolError(id, err.Error()) + } + if caller.role.Kind != db.HeraKindCoordinator { + return toolError(id, fmt.Sprintf( + "caller role %q has kind %q; only coordinators may revive a role", + caller.role.Name, caller.role.Kind)) + } + + target, errResp := s.resolveOrchRole(id, caller.orch.ID, caller.orch.Name, roleName) + if errResp != nil { + return errResp + } + if target.ID == caller.role.ID { + return toolError(id, fmt.Sprintf( + "role %q is your own (live, calling) role; hera_revive targets a DIFFERENT role you coordinate", + target.Name)) + } + + binding, err := s.heraStore.HeraLiveBindingByRole(target.ID) + if errors.Is(err, db.ErrHeraNotFound) { + return toolError(id, fmt.Sprintf("role %q has no live binding (never spawned, or ended)", target.Name)) + } + if err != nil { + return toolError(id, fmt.Sprintf("resolve binding for role %q: %v", target.Name, err)) + } + + outcome, err := s.heraRevive(HeraReviveInput{ + TaskID: binding.ArgusTaskID, + IsCoordinator: target.Kind == db.HeraKindCoordinator, + }) + if err != nil { + return toolError(id, fmt.Sprintf("revive %q: %v", target.Name, err)) + } + + slog.Info("[hera] revive", "orch", caller.orch.Name, "role", target.Name, "task_id", binding.ArgusTaskID, "outcome", outcome) + var b strings.Builder + fmt.Fprintf(&b, "%s\n\n", heraReviveOutcomeMessage(outcome, target.Name)) + fmt.Fprintf(&b, "- **role**: %s\n", target.Name) + fmt.Fprintf(&b, "- **argus_task_id**: %s\n", binding.ArgusTaskID) + fmt.Fprintf(&b, "- **outcome**: %s\n", outcome) + return toolResult(id, b.String()) +} + +// heraReviveOutcomeMessage renders a hera.ReviveOutcome (passed as its +// underlying string via HeraReviver, so the mcp package's function signatures +// stay independent of internal/hera's type per the HeraSpawner precedent) +// into a human-readable summary line for the tool response. +func heraReviveOutcomeMessage(outcome, roleName string) string { + switch outcome { + case string(hera.ReviveRestartedDead): + return fmt.Sprintf("%s's session was dead — restarted it in place.", roleName) + case string(hera.ReviveKickedStuck): + return fmt.Sprintf("%s was alive but stuck (idle, not blocked) — kicked it to resume.", roleName) + case string(hera.ReviveSkippedCoordinatorLive): + return fmt.Sprintf("%s is a live coordinator — never auto-revived; navigate to it or message it directly if it needs attention.", roleName) + case string(hera.ReviveSkippedBusy): + return fmt.Sprintf("%s is alive and actively working — left untouched.", roleName) + case string(hera.ReviveSkippedBlocked): + return fmt.Sprintf("%s is idle but parked at a question — left untouched to avoid dismissing it.", roleName) + case string(hera.ReviveSkippedPending): + return fmt.Sprintf("%s already has a revive/restart in flight — no action taken.", roleName) + case string(hera.ReviveSkippedNoSessionID): + return fmt.Sprintf("%s's session is alive but has no session id to resume — left untouched.", roleName) + default: + return fmt.Sprintf("%s: outcome %q.", roleName, outcome) + } +} + func (s *Server) toolHeraSpawnWorker(id interface{}, args json.RawMessage) *Response { if !s.heraEnabled() { return toolError(id, "hera not configured") diff --git a/internal/mcp/hera_revive_test.go b/internal/mcp/hera_revive_test.go new file mode 100644 index 00000000..79c0d2d1 --- /dev/null +++ b/internal/mcp/hera_revive_test.go @@ -0,0 +1,180 @@ +package mcp + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/drn/argus/internal/db" + "github.com/drn/argus/internal/testutil" +) + +// --- hera_revive (add-hera-revive) --- +// +// fakeHeraReviver records the HeraReviveInput it was called with and returns +// a fixed outcome/error, so these tests exercise resolution + response +// rendering without a real runner/PTY (the gating logic itself is covered by +// internal/hera/revive_test.go and internal/daemon/revive_test.go). +type fakeHeraReviver struct { + called bool + calledWith HeraReviveInput + outcome string + err error +} + +func (f *fakeHeraReviver) reviver() HeraReviver { + return func(in HeraReviveInput) (string, error) { + f.called = true + f.calledWith = in + return f.outcome, f.err + } +} + +func TestHeraRevive_NonCoordinatorRejected(t *testing.T) { + s, d := testHeraServer(t) + s.SetHeraReviver((&fakeHeraReviver{}).reviver()) + orch, err := d.CreateHeraOrchestrator("O", "") + testutil.NoError(t, err) + + callerTask := addHeraTestTask(t, d, "/wt/worker-caller") + _, _, err = d.CreateHeraRoleWithBinding(db.CreateHeraRoleInput{ + OrchestratorID: orch.ID, Name: "w1", Kind: db.HeraKindWorker, ArgusProject: "test-project", + }, callerTask.ID, callerTask.Worktree) + testutil.NoError(t, err) + + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_revive", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"role_name":"whatever","orchestrator":"O" + }`, callerTask.Worktree)), + }) + testutil.NoError(t, respErr(resp)) + cr := callResult(t, resp) + if !cr.IsError { + t.Fatal("expected a coordinator-only rejection") + } + testutil.Contains(t, cr.Content[0].Text, "only coordinators") +} + +func TestHeraRevive_UnknownRoleRejected(t *testing.T) { + s, d := testHeraServer(t) + s.SetHeraReviver((&fakeHeraReviver{}).reviver()) + coordTask := seedCoordinator(t, s, d, "O", "/wt/coord") + + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_revive", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"role_name":"ghost","orchestrator":"O" + }`, coordTask.Worktree)), + }) + testutil.NoError(t, respErr(resp)) + cr := callResult(t, resp) + if !cr.IsError { + t.Fatal("expected an unknown-role rejection") + } + testutil.Contains(t, cr.Content[0].Text, "not found") +} + +func TestHeraRevive_OwnRoleRejected(t *testing.T) { + s, d := testHeraServer(t) + s.SetHeraReviver((&fakeHeraReviver{}).reviver()) + coordTask := seedCoordinator(t, s, d, "O", "/wt/coord") + + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_revive", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"role_name":"coord","orchestrator":"O" + }`, coordTask.Worktree)), + }) + testutil.NoError(t, respErr(resp)) + cr := callResult(t, resp) + if !cr.IsError { + t.Fatal("expected a self-target rejection") + } + testutil.Contains(t, cr.Content[0].Text, "own") +} + +func TestHeraRevive_NoLiveBindingRejected(t *testing.T) { + s, d := testHeraServer(t) + s.SetHeraReviver((&fakeHeraReviver{}).reviver()) + coordTask := seedCoordinator(t, s, d, "O", "/wt/coord") + + orch, err := d.HeraOrchestratorByName("O") + testutil.NoError(t, err) + _, err = d.CreateHeraPlannedRole(db.CreateHeraRoleInput{ + OrchestratorID: orch.ID, Name: "planned-1", Kind: db.HeraKindWorker, ArgusProject: "test-project", Prompt: "later", + }) + testutil.NoError(t, err) + + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_revive", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"role_name":"planned-1","orchestrator":"O" + }`, coordTask.Worktree)), + }) + testutil.NoError(t, respErr(resp)) + cr := callResult(t, resp) + if !cr.IsError { + t.Fatal("expected a no-live-binding rejection") + } + testutil.Contains(t, cr.Content[0].Text, "no live binding") +} + +func TestHeraRevive_Success(t *testing.T) { + s, d := testHeraServer(t) + coordTask := seedCoordinator(t, s, d, "O", "/wt/coord") + + orch, err := d.HeraOrchestratorByName("O") + testutil.NoError(t, err) + workerTask := addHeraTestTask(t, d, "/wt/worker-1") + _, _, err = d.CreateHeraRoleWithBinding(db.CreateHeraRoleInput{ + OrchestratorID: orch.ID, Name: "worker-1", Kind: db.HeraKindWorker, ArgusProject: "test-project", + }, workerTask.ID, workerTask.Worktree) + testutil.NoError(t, err) + + fr := &fakeHeraReviver{outcome: "kicked_stuck"} + s.SetHeraReviver(fr.reviver()) + + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_revive", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"role_name":"worker-1","orchestrator":"O" + }`, coordTask.Worktree)), + }) + testutil.NoError(t, respErr(resp)) + cr := callResult(t, resp) + if cr.IsError { + t.Fatalf("expected success, got error: %s", cr.Content[0].Text) + } + testutil.Equal(t, fr.called, true) + testutil.Equal(t, fr.calledWith.TaskID, workerTask.ID) + testutil.Equal(t, fr.calledWith.IsCoordinator, false) + testutil.Contains(t, cr.Content[0].Text, "kicked_stuck") +} + +func TestHeraRevive_NotConfiguredWhenReviverNil(t *testing.T) { + s, d := testHeraServer(t) + coordTask := seedCoordinator(t, s, d, "O", "/wt/coord") + + orch, err := d.HeraOrchestratorByName("O") + testutil.NoError(t, err) + workerTask := addHeraTestTask(t, d, "/wt/worker-1") + _, _, err = d.CreateHeraRoleWithBinding(db.CreateHeraRoleInput{ + OrchestratorID: orch.ID, Name: "worker-1", Kind: db.HeraKindWorker, ArgusProject: "test-project", + }, workerTask.ID, workerTask.Worktree) + testutil.NoError(t, err) + + // testHeraServer does NOT call SetHeraReviver — s.heraRevive stays nil. + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_revive", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"role_name":"worker-1","orchestrator":"O" + }`, coordTask.Worktree)), + }) + testutil.NoError(t, respErr(resp)) + cr := callResult(t, resp) + if !cr.IsError { + t.Fatal("expected a not-configured rejection") + } + testutil.Contains(t, cr.Content[0].Text, "not configured") +} diff --git a/internal/mcp/hera_test.go b/internal/mcp/hera_test.go index 6c5ca721..87498eeb 100644 --- a/internal/mcp/hera_test.go +++ b/internal/mcp/hera_test.go @@ -149,14 +149,15 @@ func TestToolsList_HeraOn(t *testing.T) { names[tool.Name] = true } - // All 17 hera tools must appear (9 ported + hera_move + hera_rebind + 3 - // plan-authoring + 3 plan-mutation). + // All 18 hera tools must appear (9 ported + hera_move + hera_rebind + 3 + // plan-authoring + 3 plan-mutation + hera_revive). for _, want := range []string{ "hera_new_orchestrator", "hera_join", "hera_move", "hera_rebind", "hera_send", "hera_inbox", "hera_mark_read", "hera_status", "hera_spawn_worker", "hera_tree_updates", "hera_get_messages", "hera_plan_node", "hera_block", "hera_plan", "hera_plan_node_update", "hera_unblock", "hera_plan_node_cancel", + "hera_revive", } { if !names[want] { t.Errorf("hera tool missing from tools/list: %s", want) diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 9216d82c..59bf0f56 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -83,6 +83,21 @@ type HeraSpawnResult struct { // failure (the AfterPersist LIFO-cleanup contract). type HeraSpawner func(in HeraSpawnInput) (*HeraSpawnResult, error) +// HeraReviveInput carries a resolved PULL-revive target (add-hera-revive) into +// the daemon's shared primitive. TaskID is the target role's live-bound argus +// task; IsCoordinator identifies the TARGET role's kind (not the caller's) — +// a live coordinator is never auto-restarted. +type HeraReviveInput struct { + TaskID string + IsCoordinator bool +} + +// HeraReviver invokes the daemon's shared PULL-revive primitive +// (internal/hera.ReviveRole) against the daemon's real runner+DB and returns +// the resulting hera.ReviveOutcome as a string. Injected via SetHeraReviver; +// nil when hera is disabled or the daemon did not wire it. +type HeraReviver func(in HeraReviveInput) (string, error) + // TaskCreator creates a task with worktree and starts an agent session. // Same call shape used by daemon.HeadlessCreateTask (the daemon wraps it to // avoid an import cycle on the mcp package). @@ -166,6 +181,7 @@ type Server struct { heraSvc *hera.Service // optional; set via SetHeraService heraStore HeraStore // optional; set via SetHeraService heraSpawn HeraSpawner // optional; set via SetHeraService (born-bound spawn) + heraRevive HeraReviver // optional; set via SetHeraReviver (PULL-revive, add-hera-revive) profileCfg ConfigStore // optional; set via SetProfileResolver createMu sync.Mutex creating int // number of in-flight task_create calls @@ -935,6 +951,8 @@ func (s *Server) handleToolsCall(req *Request) *Response { return s.toolHeraUnblock(req.ID, params.Arguments) case "hera_plan_node_cancel": return s.toolHeraPlanNodeCancel(req.ID, params.Arguments) + case "hera_revive": + return s.toolHeraRevive(req.ID, params.Arguments) case "profile_resolve": return s.toolProfileResolve(req.ID, params.Arguments) default: diff --git a/openspec/changes/archive/2026-07-31-add-hera-revive/design.md b/openspec/changes/archive/2026-07-31-add-hera-revive/design.md new file mode 100644 index 00000000..13b75f00 --- /dev/null +++ b/openspec/changes/archive/2026-07-31-add-hera-revive/design.md @@ -0,0 +1,83 @@ +## Context + +The TUI's native Hera view already has a manual revive path, fired by `Enter` on a rail row (`internal/tui/hera/page.go`'s `OnReattach`, wired to `App.heraReattach` in `internal/tui/heraactions.go`): + +- No live session (`sess == nil || !sess.Alive()`) → `a.startSession(t)` restarts the session, resuming via `--session-id` when the task carries one. Fires for any role kind, including a dead coordinator. +- Live session, role kind `coordinator` → navigate-only. A live coordinator is presumed operator-interactive (a human may be reading its pane right now); `Enter` never auto-restarts it. +- Live session, role kind `worker`/`freelance` → `a.reviveHeraWorker(task, sess)`: off the main goroutine, checks `sess.IsIdle()` and `sessionBlockedOnPrompt` (a SIGTSTP'd or otherwise stalled agent is idle and NOT parked at a prompt — that's the "genuinely stuck" signature); if both hold, and no kick is already in flight (`HasPendingRestart`), it calls `agent.SessionRunner.KickRerender` (stop + resume in place at the pane's current width) and, on success, restores the task from `in_review` back to `in_progress` via `db.ReviveHeraWorkerToInProgress` — already documented as "the SINGLE shared helper behind BOTH revive triggers" (the other being the daemon's supervisor-mode startup reattach). + +This is a keystroke a human drives from the TUI. A hera coordinator is itself just another agent session — it cannot press `Enter`. It has no way today to notice a bound role has gone quiet (dead or SIGTSTP'd after a session-supervisor restart) and do anything about it, short of spawning a wasteful duplicate worker. + +## Goals / Non-Goals + +**Goals:** + +- Give a coordinator an MCP tool that inspects ONE role it coordinates and, if dead or genuinely stuck, revives it — using the identical safety gate the TUI's `Enter` key already enforces, so it cannot thrash a session that is actually working or waiting on a user prompt. +- Report a no-op clearly (busy / blocked / live coordinator / already mid-restart) rather than silently doing nothing. +- Keep this strictly pull/on-demand: the daemon never fires this automatically on supervisor restart. A coordinator calls it when it notices no progress (`hera_status`, `hera_tree_updates`). + +**Non-Goals:** + +- No automatic revive-on-supervisor-restart. That would be a push model; this change is deliberately pull-only (proposal.md). +- No change to the TUI's `Enter`-key behavior or its threading model. +- No new "needs-input detection" heuristic — reuses `agent.BlockedOnPrompt`/`agent.DetectNeedsInput` exactly as they exist today. + +## Decisions + +### D1 — Tool shape: `hera_revive(cwd, role_name, [orchestrator])`, coordinator-only + +Mirrors `hera_block`'s addressing (`resolveOrchRole`: a role name resolved within the caller's own orchestrator) and `hera_spawn_worker`'s coordinator-only guard. A worker/freelance caller is rejected with the same wording style as `hera_spawn_worker`/the plan-authoring tools ("only coordinators may ..."). Targeting the caller's own (necessarily-live, since it's calling right now) role is rejected explicitly with a clearer message than letting it fall through to "skipped: live coordinator." + +Alternative considered: let any hera-bound role revive any other role process-wide (no orchestrator scoping). Rejected — every other hera mutation tool (`hera_block`, `hera_plan_node`, `hera_send`'s `to`) scopes role addressing to the caller's own orchestrator; there's no reason to widen the blast radius here. + +### D2 — Outcome is always reported, never silently a no-op + +`hera_revive` returns one of: `restarted_dead`, `kicked_stuck`, `skipped_coordinator_live`, `skipped_busy`, `skipped_blocked_on_prompt`, `skipped_restart_pending`, `skipped_no_session_id`. This directly satisfies "if the role is fine, the tool should say so rather than doing anything" (task brief) — a coordinator gets a legible signal either way instead of having to infer success from a follow-up `hera_status` poll. + +### D3 — Extract the decision primitive; do NOT extract the TUI's call site (the central question this change was asked to settle) + +**What gets extracted:** a new pure function, `internal/hera.ReviveRole(store, runner, taskID, isCoordinator) (ReviveOutcome, error)`, over two narrow interfaces (`ReviveStore`, `ReviveRunner`) — architecturally identical to the existing `hera.RecycleCoord`/`RecycleStore`/`RecycleRunner` (`internal/hera/recycle.go`), which is this codebase's own precedent for "one gating+action function, called from both a TUI action and a daemon-side trigger." `ReviveRole` encodes the exact same ordered gate the TUI applies (alive? → coordinator-live? → has a session id? → kick already in flight? → idle? → blocked on prompt?) and the same two actions (`RestartDead` / `KickRerender` + best-effort `ReviveHeraWorkerToInProgress` restore). It is unit-testable with fakes, no real PTY or SQLite required (`internal/hera/revive_test.go`, mirroring `recycle_test.go`'s shape). + +A new daemon-side adapter, `daemon.HeraReviveRunner` (mirrors `daemon.HeraRecycleRunner` in `internal/daemon/recycle.go`), implements `ReviveRunner` against the real `*db.DB` + `agent.SessionRunner`, and is the ONLY thing `hera_revive`'s MCP handler wires up. + +**What does NOT get touched:** `internal/tui/heraactions.go`'s `heraReattach`/`reviveHeraWorker`/`reviveRestoreInProgress`. They keep their existing inline implementation. Three concrete reasons, not just "avoid churn": + +1. **A genuine behavioral difference, not an accidental one.** The TUI's kick resizes to `a.computePTYSize()` — the CURRENT PANE's dimensions, so a revived session also re-flows to whatever size the pane is now (this doubles as the BUG-074 size-drift fix path). A headless MCP caller has no pane and no rendering surface to fit; the correct default there is to PRESERVE the session's existing PTY size (`sess.PTYSize()`), which is what `daemon.HeraReviveRunner.KickRerender` does. Forcing one shared call site would mean either the daemon path grows a fake "target size" concept it has no use for, or the TUI path silently loses its resize-on-revive side effect. +2. **Threading.** The TUI's version deliberately splits "compute idle/blocked off the main goroutine" from "act inside `QueueUpdateDraw`" (mirroring `maybeKickRerenderAtWidth`) — a pattern this codebase has hard-won, well-tested threading rules around (`gotchas/ui-threading.md`). The MCP handler runs on its own request goroutine with no such constraint. Collapsing the TUI's two-phase dispatch into one straight-line call (as `ReviveRole` is) is a reasonable simplification in isolation, but changing it is exactly the kind of "refactor beyond what the task requires" this repo's conventions call out to avoid, against a code path with a long specific bug history (BUG-032/033/034/035/060/061/063/065/067/072 in `gotchas/events.md`, all in this exact idle/needs-input detection neighborhood). +3. **Scope.** The task asked for a coordinator-facing pull-revive tool, not a TUI refactor. The TUI path is provided as background/context, not as something in scope to change. + +**Why this isn't "silent duplication" despite the TUI keeping its own inline branch:** every individual CHECK the TUI's inline code performs is already, and remains, single-sourced regardless of this decision — `agent.BlockedOnPrompt` (already documented as "correct server-side... unreliable in daemon-client mode," i.e., already written with exactly this daemon-vs-TUI split in mind), `db.ReviveHeraWorkerToInProgress` (already documented as "the SINGLE shared helper" — this change adds a THIRD caller, not a fork), and `agent.SessionRunner.KickRerender`/`StartOrReattach`/`agent.RefreshResumeSessionID` (the same runner methods either way). The only thing expressed twice is the ~10-line ORDERING of those checks — a small, mechanical sequence (not a fuzzy detection heuristic) that is now captured ONCE for the daemon path in a unit-tested function, with the TUI's copy flagged here as a known, intentional, low-risk residual overlap rather than something nobody decided about. + +Alternative considered and rejected: force TUI's `reviveHeraWorker` to call `hera.ReviveRole` too (via a TUI-side `ReviveRunner` adapter using `a.computePTYSize()`). Two adapters CAN satisfy one shared `ReviveRunner` interface with different `KickRerender` sizing, which resolves reason (1) above — but reason (2) (threading) and (3) (scope/regression risk against tested, bug-history-heavy code) still argue against it for this change. Flagged as a reasonable follow-up if the TUI path is ever revisited on its own terms, not attempted here. + +### D4 — `ReviveHeraWorkerToInProgress` failure after a successful kick is soft-fail, not propagated + +Matches `reviveRestoreInProgress`'s existing behavior exactly (log-and-continue in the TUI; here, simply not treated as a tool error) — the kick itself already succeeded, and a worker stranded in `in_review` can still be closed out manually. Surfacing `kicked_stuck` either way keeps the tool's contract simple (the KICK is the operation being reported on; the in_progress restore is a best-effort side effect of a successful kick, exactly as it already is on the TUI side). + +## Risks / Trade-offs + +- **[Risk]** The small ordering overlap flagged in D3 could still drift (e.g., a future fix to the TUI's gate that isn't mirrored here). → **Mitigation:** both call sites are documented (this file + a new gotcha bullet) as sharing every underlying primitive except the ordering; a reviewer touching one gets pointed at the other. Low severity: the ordering itself is mechanical (six straight-line boolean checks), not a heuristic prone to the kind of subtle drift this codebase's needs-input detection family has suffered from historically. +- **[Risk]** A coordinator could call `hera_revive` repeatedly in a tight loop against a role that's genuinely just slow. → **Mitigation:** none needed structurally — the idle+blocked gate already refuses to act on a busy or prompt-parked session every single call, so a tight loop just produces repeated `skipped_busy`/`skipped_blocked_on_prompt` no-ops, never a thrash. This mirrors the existing acceptance of "nothing stops calling `hera_status` in a loop either" reasoning from `add-worker-bounce`'s design. +- **[Risk]** Reviving a role whose binding is planned-but-not-yet-materialized (no live binding at all) needs a clear error, not a crash. → **Mitigation:** `HeraLiveBindingByRole` returning `ErrHeraNotFound` is translated to an explicit "no live binding (never spawned, or ended)" tool error, mirroring `resolveOrchRole`'s existing not-found handling. + +## Migration Plan + +None needed — additive, no schema/data migration, no backwards-compatibility concern. + +## Open Questions + +None — all decisions above are settled for this change. + +## Acceptance criteria + +- It should let a coordinator revive a dead (no live session) role of any kind, restarting it in place (resuming via `--session-id` when the task has one). +- It should let a coordinator revive a live-but-stuck (idle, not blocked on a prompt) worker or freelance role by kicking it in place. +- It should restore a kicked worker's task from `in_review` back to `in_progress` when the kick succeeds and the worker isn't awaiting close-out (reusing `ReviveHeraWorkerToInProgress`'s existing guard). +- It should leave a live coordinator role untouched and report `skipped_coordinator_live`, never auto-restarting it. +- It should leave a busy (non-idle) live role untouched and report `skipped_busy`. +- It should leave an idle-but-blocked-on-a-prompt live role untouched and report `skipped_blocked_on_prompt`, never dismissing the pending question. +- It should leave a role with a kick/restart already in flight untouched and report `skipped_restart_pending`. +- It should reject a non-coordinator caller. +- It should reject an unknown role name within the caller's orchestrator. +- It should reject a role name resolving to the caller's own (live, calling) role. +- It should reject a role with no live binding (planned-but-not-materialized, or ended). diff --git a/openspec/changes/archive/2026-07-31-add-hera-revive/proposal.md b/openspec/changes/archive/2026-07-31-add-hera-revive/proposal.md new file mode 100644 index 00000000..36cd440c --- /dev/null +++ b/openspec/changes/archive/2026-07-31-add-hera-revive/proposal.md @@ -0,0 +1,32 @@ +## Why + +When the session-supervisor restarts, it SIGHUPs every PTY it owns. A hera worker's session survives as a live-but-stuck (SIGTSTP'd) process, or dies outright — either way it stops making progress. Today the only way to notice and fix this is a human watching the TUI: pressing `Enter` on the role's rail row drives `reviveHeraWorker` (kick a stuck-but-alive session back to life) or `startSession` (restart a dead one). A hera coordinator — itself just another agent session, not a human at a keyboard — has no way to notice a bound role has gone quiet and do anything about it. It can only wait, or spawn a wasteful duplicate worker (a mistake already documented in `hera-dont-declare-worker-dead-without-live-confirmation` memory). + +This change gives a coordinator a new MCP tool, `hera_revive`, that inspects one role it coordinates and, if it's dead or genuinely stuck, revives it — reusing the exact same safety gate (idle, not blocked on a prompt) the TUI's `Enter` key already enforces, so it can never thrash a session that's actually working or waiting on an answer. If the role is fine, the tool says so and does nothing. This is deliberately pull/on-demand — a coordinator calls it when `hera_status`/`hera_tree_updates` shows no progress — not an automatic daemon-side trigger on supervisor restart. + +## What Changes + +- New MCP tool `hera_revive(cwd, role_name, [orchestrator])`, coordinator-only: resolves `role_name` within the caller's orchestrator, and, based on its live session state, either restarts a dead session in place, kicks a stuck-but-alive session in place, or reports that no action was needed (busy, blocked on a prompt, a live coordinator, or already mid-restart) — mirroring the TUI Enter-key path's outcomes exactly. +- New shared primitive `internal/hera.ReviveRole` (mirrors the existing `RecycleCoord` architecture: a pure decision function over narrow `ReviveStore`/`ReviveRunner` interfaces) encodes the gating sequence once, unit-tested without a real PTY/SQLite. +- New daemon-side adapter `daemon.HeraReviveRunner` (mirrors `daemon.HeraRecycleRunner`) wires the primitive to the real `*db.DB` + `agent.SessionRunner`, reusing already-shared building blocks: `agent.BlockedOnPrompt` (ring-based, daemon-correct), `db.ReviveHeraWorkerToInProgress` (the existing single shared in_review→in_progress restore helper), and `agent.SessionRunner.KickRerender` / `StartOrReattach` / `agent.RefreshResumeSessionID`. +- The TUI's existing `Enter`-key revive path (`internal/tui/heraactions.go`) is left untouched — see `design.md` for why this is a deliberate, documented choice rather than silent duplication. + +## Capabilities + +### New Capabilities + +(none — this adds one tool + one internal primitive to the existing `hera-coordination` capability) + +### Modified Capabilities + +- `hera-coordination`: registers the fifteenth native `hera_*` MCP tool (`hera_revive`); the "Worker revive restores in_progress" requirement gains a third caller of the shared `ReviveHeraWorkerToInProgress` helper. + +## Impact + +- `internal/hera/revive.go` (new), `internal/hera/revive_test.go` (new) +- `internal/daemon/revive.go` (new), `internal/daemon/revive_test.go` (new) +- `internal/mcp/server.go` (`HeraReviveInput`/`HeraReviver` types, `heraRevive` field) +- `internal/mcp/hera.go` (`hera_revive` tool schema, `toolHeraRevive` handler, `SetHeraReviver`, dispatch case) +- `internal/daemon/daemon.go` (`heraReviveRole` adapter method, `SetHeraReviver` wiring) +- `.claude/skills/hera/SKILL.md`, `README.md` (MCP tools table), `context/knowledge/gotchas/daemon-rpc.md`, `context/knowledge/index.md` +- No schema/data migration, no new dependencies, no REST/API surface change, no TUI behavior change. diff --git a/openspec/changes/archive/2026-07-31-add-hera-revive/specs/hera-coordination/spec.md b/openspec/changes/archive/2026-07-31-add-hera-revive/specs/hera-coordination/spec.md new file mode 100644 index 00000000..a86dc831 --- /dev/null +++ b/openspec/changes/archive/2026-07-31-add-hera-revive/specs/hera-coordination/spec.md @@ -0,0 +1,143 @@ +## ADDED Requirements + +### Requirement: hera_revive coordinator PULL-revive of a bound role + +The system SHALL provide a coordinator-only `hera_revive(cwd, role_name, [orchestrator])` MCP tool that inspects one role the caller coordinates and, based on its live session state, takes exactly one of the following actions, always reporting which one: + +- **No live session** (dead, of any role kind including a nested coordinator) → restart it in place, resuming via `--session-id` when the task carries one. Reports `restarted_dead`. +- **Live, role kind is `coordinator`** → left untouched (a live coordinator is presumed operator-interactive). Reports `skipped_coordinator_live`. +- **Live, non-coordinator, no session id to resume** → left untouched. Reports `skipped_no_session_id`. +- **Live, non-coordinator, a kick/restart is already in flight** → left untouched. Reports `skipped_restart_pending`. +- **Live, non-coordinator, not idle** (busy) → left untouched. Reports `skipped_busy`. +- **Live, non-coordinator, idle, but blocked on a user prompt** (selection UI or trailing question) → left untouched, so the pending question is never dismissed. Reports `skipped_blocked_on_prompt`. +- **Live, non-coordinator, idle, not blocked** (genuinely stuck — e.g. SIGTSTP'd by a session-supervisor restart) → kicked (stop + resume in place at its existing PTY size), and, on success, restored from `in_review` back to `in_progress` via the shared `ReviveHeraWorkerToInProgress` helper (best-effort; a restore failure does not fail the tool call). Reports `kicked_stuck`. + +This is the SAME safety gate the TUI's `Enter`-key revive path (`heraReattach`/`reviveHeraWorker`) already enforces, reusing the same underlying primitives (`agent.BlockedOnPrompt`, `db.ReviveHeraWorkerToInProgress`, `agent.SessionRunner.KickRerender`/`StartOrReattach`) via a new shared decision function, `internal/hera.ReviveRole` — see `openspec/changes/add-hera-revive/design.md` D3 for why the TUI's own call site is not itself refactored to share this function. + +The tool SHALL reject a non-coordinator caller, an unknown `role_name` within the caller's orchestrator, a `role_name` resolving to the caller's own (live, calling) role, and a role with no live binding (planned-but-not-materialized, or ended). + +This is strictly pull/on-demand: nothing in the daemon calls `hera_revive` automatically. A coordinator calls it when it notices no progress from a role (e.g. via `hera_status`/`hera_tree_updates`). + +Derived from: `internal/hera/revive.go` (`ReviveRole`), `internal/daemon/revive.go` (`HeraReviveRunner`), `internal/mcp/hera.go` (`toolHeraRevive`). + +#### Scenario: Dead role is restarted + +- **WHEN** a coordinator calls `hera_revive` on a role with no live session +- **THEN** the session restarts in place (resuming via `--session-id` when the task has one) and the tool reports `restarted_dead` + +#### Scenario: Stuck worker is kicked and restored to in_progress + +- **WHEN** a coordinator calls `hera_revive` on a live worker role that is idle and not blocked on a prompt +- **THEN** the session is kicked (stopped and resumed in place) and, if the worker was parked in in_review awaiting nothing, its task is restored to in_progress; the tool reports `kicked_stuck` + +#### Scenario: Live coordinator is never auto-revived + +- **WHEN** a coordinator calls `hera_revive` targeting a live nested coordinator role +- **THEN** nothing is restarted and the tool reports `skipped_coordinator_live` + +#### Scenario: Busy role is left alone + +- **WHEN** a coordinator calls `hera_revive` on a live, non-idle worker/freelance role +- **THEN** nothing is restarted and the tool reports `skipped_busy` + +#### Scenario: Role blocked on a prompt is left alone + +- **WHEN** a coordinator calls `hera_revive` on a live, idle worker/freelance role that is parked at a user prompt +- **THEN** nothing is restarted (the pending question is preserved) and the tool reports `skipped_blocked_on_prompt` + +#### Scenario: A kick already in flight is not duplicated + +- **WHEN** a coordinator calls `hera_revive` on a role that already has a pending kick/restart queued +- **THEN** no second kick is queued and the tool reports `skipped_restart_pending` + +#### Scenario: Non-coordinator caller is rejected + +- **WHEN** a worker or freelance role calls `hera_revive` +- **THEN** the tool errors that only coordinators may revive a role + +#### Scenario: Unknown role name is rejected + +- **WHEN** `role_name` does not resolve to any role in the caller's orchestrator +- **THEN** the tool errors that the role was not found + +#### Scenario: Targeting one's own role is rejected + +- **WHEN** `role_name` resolves to the calling coordinator's own role +- **THEN** the tool errors rather than silently reporting `skipped_coordinator_live` + +#### Scenario: A planned-but-unmaterialized or ended role is rejected + +- **WHEN** `role_name` resolves to a role with no live binding +- **THEN** the tool errors that the role has no live binding + +## MODIFIED Requirements + +### Requirement: Native hera_* MCP tool surface + +The system SHALL register eighteen native `hera_*` MCP tools with the same names, parameters, descriptions, and required lists as the external Hera daemon where they overlap: `hera_new_orchestrator`, `hera_join`, `hera_move`, `hera_rebind`, `hera_send`, `hera_inbox`, `hera_mark_read`, `hera_status`, `hera_spawn_worker`, `hera_tree_updates`, `hera_get_messages`, the three plan-authoring tools `hera_plan_node`, `hera_block`, and `hera_plan`, the three plan-mutation verbs `hera_plan_node_update`, `hera_unblock`, and `hera_plan_node_cancel`, and `hera_revive`. The plan-authoring tools SHALL be coordinator-only (a worker or freelance caller is rejected, mirroring `hera_spawn_worker`): `hera_plan_node` creates a planned node, `hera_block` adds a blocking edge (cycle-checked, single-orchestrator), and `hera_plan` submits a whole graph of nodes and edges in one call. `hera_revive` SHALL likewise be coordinator-only. The tools SHALL be available only when the hera service is wired AND task management is enabled (caller resolution via `cwd` requires task management). A dup-tool guard SHALL suppress any plugin tool scoped `hera` while native Hera is enabled, so in-tree and plugin tools never both appear. + +NOTE: this requirement's tool count/list previously read "fourteen" and omitted the three plan-mutation verbs (a pre-existing drift from when `make-hera-plan-living` added them) — corrected here to the accurate eighteen while adding `hera_revive`, since both changes touch this same sentence. + +#### Scenario: Tools require task management + +- **WHEN** task management is disabled +- **THEN** the `hera_*` tools report "hera not configured" rather than acting + +#### Scenario: Native and plugin hera tools are mutually exclusive + +- **WHEN** native Hera is enabled +- **THEN** any plugin tool scoped `hera` is suppressed so only the in-tree tools appear + +#### Scenario: Plan-authoring tools are coordinator-only + +- **WHEN** a worker or freelance role calls `hera_plan_node`, `hera_block`, or `hera_plan` +- **THEN** the tool errors that only coordinators may author the plan + +#### Scenario: hera_revive is coordinator-only + +- **WHEN** a worker or freelance role calls `hera_revive` +- **THEN** the tool errors that only coordinators may revive a role + +### Requirement: Worker revive restores in_progress + +The system SHALL restore a worker-bound task from in_review back to in_progress when its session is genuinely revived/resumed and working again, via the single shared helper `ReviveHeraWorkerToInProgress` — the precise inverse of `RollHeraWorkerToReview`. The restore is worker-kind only, no-ops unless the task is currently in_review (so it never clobbers a human-set complete/pending and never disturbs an already-in_progress task), touches the DB status only (never the session), and is idempotent. + +The restore SHALL NOT fire when the worker is awaiting coordinator close-out — that is, when its bound task carries `meta:hera.ready_to_close` (the BUG-050 done / clean-exit stamp) OR any of its live worker roles has a terminal role-status (`done` or `failed`). This guard preserves the PR #707 / BUG-050 invariant: a genuinely-finished worker stays in_review even when its idle session is still alive, because a worker never self-completes — the coordinator/human closes it out or decides on a failure. + +Three trigger sites share the helper so they cannot drift: + +- The daemon's supervisor-mode startup reattach (`reattachSupervised`) calls it for every task the supervisor confirms ALIVE, so a live worker stranded in in_review by a prior roll or reconcile is restored to in_progress on each bounce (the true orphans the supervisor does NOT report alive still flip the other way, to in_review). +- The TUI's live-session revive (`reviveHeraWorker`, the Enter-key in-place `KickRerender` resume) calls it on a successful kick (local store only; `--remote` mode defers to the live local daemon's own reattach restore). +- The `hera_revive` MCP tool's shared `hera.ReviveRole` primitive calls it on a successful kick, identically to the TUI's own call (add-hera-revive). + +Derived from: `internal/db/hera.go` (`ReviveHeraWorkerToInProgress`), `internal/daemon/bounce.go` (`reattachSupervised`), `internal/tui/app.go` (`reviveRestoreInProgress`) + `internal/tui/heraactions.go` (`reviveHeraWorker`), `internal/hera/revive.go` (`ReviveRole`). + +#### Scenario: Stranded live worker is restored on reattach + +- **WHEN** the supervisor reports a worker's session still alive across a daemon bounce and that worker's task is parked in in_review with no close-out marker +- **THEN** the task is restored to in_progress while true orphans still flip to in_review + +#### Scenario: Revived suspended worker returns to in_progress + +- **WHEN** a live-but-suspended worker in in_review is revived in place via KickRerender and the kick succeeds +- **THEN** its task is restored to in_progress + +#### Scenario: A done or clean-exited worker stays in_review + +- **WHEN** a worker carries meta:hera.ready_to_close (reported done or cleanly exited) and its idle session is still alive on revive/reattach +- **THEN** the restore no-ops and the task stays in_review for coordinator close-out + +#### Scenario: A failed worker stays in_review + +- **WHEN** a worker's role status is failed and its session is still alive on revive/reattach +- **THEN** the restore no-ops and the task stays in_review for coordinator attention + +#### Scenario: Non-worker and non-review tasks are untouched + +- **WHEN** the task is coordinator-bound, holds no live worker binding, or is not currently in_review (in_progress / complete / pending) +- **THEN** the restore no-ops and the status is left unchanged + +#### Scenario: hera_revive's kick restores in_progress identically to the TUI + +- **WHEN** a coordinator calls `hera_revive` on a stuck worker and the kick succeeds +- **THEN** the same `ReviveHeraWorkerToInProgress` guard applies (restored unless awaiting close-out), exactly as the TUI's Enter-key kick diff --git a/openspec/changes/archive/2026-07-31-add-hera-revive/tasks.md b/openspec/changes/archive/2026-07-31-add-hera-revive/tasks.md new file mode 100644 index 00000000..befbe4b3 --- /dev/null +++ b/openspec/changes/archive/2026-07-31-add-hera-revive/tasks.md @@ -0,0 +1,55 @@ +**Design doc:** `openspec/changes/add-hera-revive/design.md` + +## 1. Tests (write failing first) + +- [x] 1.1 `internal/hera/revive_test.go`: fake `ReviveStore`/`ReviveRunner`, cases for every `ReviveOutcome` (`restarted_dead`, `kicked_stuck` incl. the in_progress restore call, `skipped_coordinator_live`, `skipped_busy`, `skipped_blocked_on_prompt`, `skipped_restart_pending`, `skipped_no_session_id`), plus a task-not-found error case. +- [x] 1.2 `internal/daemon/revive_test.go`: `daemon.HeraReviveRunner` against a real `db.OpenInMemory()` + `agent.NewRunner(nil)` (per `context/knowledge/testing.md`), proving `KickRerender` preserves the live session's existing PTY size and `RestartDead` mirrors `handleRestartTask`'s resume-via-session-id behavior. +- [x] 1.3 `internal/mcp/hera_revive_test.go` (new, mirrors `hera_rebind_test.go`'s shape): coordinator-only rejection, unknown role name, own-role rejection, no-live-binding rejection, and a success case asserting the wired `HeraReviver` func is called with the resolved task id + role kind and the outcome renders in the tool response. +- [x] 1.4 Confirm every `it should X` acceptance criterion in `design.md` has a corresponding failing test before moving to implementation. + +## 2. `internal/hera.ReviveRole` (the shared primitive) + +**Depends on:** Stage 1 + +- [x] 2.1 `internal/hera/revive.go`: `ReviveOutcome` type + constants, `ReviveStore`/`ReviveRunner` interfaces, `ReviveRole(store, runner, taskID string, isCoordinator bool) (ReviveOutcome, error)` implementing the gate from design.md D3 (alive? → coordinator-live? → has session id? → restart pending? → idle? → blocked on prompt? → kick, best-effort restore-to-in_progress). +- [x] 2.2 Run `internal/hera` tests; confirm Stage 1.1 passes. + +## 3. `daemon.HeraReviveRunner` (the daemon-side adapter) + +**Depends on:** Stage 2 + +- [x] 3.1 `internal/daemon/revive.go`: `HeraReviveRunner` struct (`*db.DB` + `agent.SessionRunner` + `cfgFn`), `NewHeraReviveRunner`, and the six `hera.ReviveRunner` methods — `IsAlive`/`IsIdle` via `runner.Get(taskID)`, `BlockedOnPrompt` via `agent.BlockedOnPrompt` (idle-gated), `HasPendingRestart` passthrough, `KickRerender` at the session's current `PTYSize()`, `RestartDead` mirroring `internal/api/handlers.go`'s `handleRestartTask` (`agent.RefreshResumeSessionID` + `StartOrReattach` + status flip). +- [x] 3.2 Run `internal/daemon` tests; confirm Stage 1.2 passes. + +## 4. Wire the `hera_revive` MCP tool + +**Depends on:** Stage 3 + +- [x] 4.1 `internal/mcp/server.go`: `HeraReviveInput{TaskID string; IsCoordinator bool}` and `HeraReviver func(HeraReviveInput) (string, error)` types (mirrors `HeraSpawnInput`/`HeraSpawner`); `heraRevive HeraReviver` field on `Server`. +- [x] 4.2 `internal/mcp/hera.go`: `hera_revive` entry in `heraToolDefs` (`cwd`, `role_name` required; `orchestrator` optional); `SetHeraReviver(reviver HeraReviver)` setter (a new independent setter, not folded into `SetHeraService`, matching the existing multi-setter pattern); `toolHeraRevive` handler — coordinator-only guard, `resolveOrchRole` for `role_name`, reject targeting the caller's own role, `HeraLiveBindingByRole` → `ErrHeraNotFound` translated to a clear error, call `s.heraRevive`, render the outcome with a human-readable message per outcome. +- [x] 4.3 `internal/mcp/server.go`'s tool dispatch switch: `case "hera_revive": return s.toolHeraRevive(req.ID, params.Arguments)`. +- [x] 4.4 Run `internal/mcp` tests; confirm Stage 1.3 passes. + +## 5. Wire the daemon + +**Depends on:** Stage 4 + +- [x] 5.1 `internal/daemon/daemon.go`: `heraReviveRole(in mcp.HeraReviveInput) (string, error)` method constructing `NewHeraReviveRunner(d.db, d.runner, d.cfgFn)` and calling `hera.ReviveRole(d.db, rr, in.TaskID, in.IsCoordinator)`. +- [x] 5.2 Wire `mcpSrv.SetHeraReviver(d.heraReviveRole)` alongside the existing `SetHeraService` call, inside the same `cfg.Hera.Enabled` gate. +- [x] 5.3 Run `internal/daemon` tests. + +## 6. Docs + +**Depends on:** Stage 5 + +- [x] 6.1 `.claude/skills/hera/SKILL.md`: add `hera_revive` to the §3 coordination-tools list (bootstrap/messaging/status section) with its pull-only, idle+not-blocked-gated semantics and when to reach for it (a role looks stuck after a supervisor restart, or `hera_status`/`hera_tree_updates` show no progress); add a one-line decision-rule pointer in §4 if it fits naturally alongside the existing "got a doorbell?" style bullets. +- [x] 6.2 `README.md`: add a `hera_revive` row to the Hera MCP tools table (Reference appendix, § MCP Tools). +- [x] 6.3 Add a gotcha bullet to `context/knowledge/gotchas/daemon-rpc.md` (same coverage cell as the existing `ReviveHeraWorkerToInProgress`/BUG-B entry) noting `hera_revive` is the third caller of the shared restore helper, and that the TUI's Enter-key gate is intentionally NOT unified with it (see design.md D3) — every individual check stays single-sourced, only the ordering is expressed twice. +- [x] 6.4 Update `context/knowledge/index.md`'s coverage-bullet cell for `gotchas/daemon-rpc.md` to reflect the new bullet. + +## 7. Archive + +**Depends on:** Stage 6 + +- [x] 7.1 Run `make pre-pr`; fix any failures. +- [x] 7.2 `openspec archive add-hera-revive` (or the manual merge-and-move fallback): merge the `hera-coordination` delta spec into `openspec/specs/hera-coordination/spec.md`, move the change folder to `openspec/changes/archive/2026-07-30-add-hera-revive/`, commit on the same branch before merge. diff --git a/openspec/specs/hera-coordination/spec.md b/openspec/specs/hera-coordination/spec.md index e8254fc5..2ce928da 100644 --- a/openspec/specs/hera-coordination/spec.md +++ b/openspec/specs/hera-coordination/spec.md @@ -24,7 +24,9 @@ Derived from: `internal/db/schema.go:447` (live-role unique index), `internal/db ### Requirement: Native hera_* MCP tool surface -The system SHALL register fourteen native `hera_*` MCP tools with the same names, parameters, descriptions, and required lists as the external Hera daemon where they overlap: `hera_new_orchestrator`, `hera_join`, `hera_move`, `hera_rebind`, `hera_send`, `hera_inbox`, `hera_mark_read`, `hera_status`, `hera_spawn_worker`, `hera_tree_updates`, `hera_get_messages`, and the three plan-authoring tools `hera_plan_node`, `hera_block`, and `hera_plan`. The plan-authoring tools SHALL be coordinator-only (a worker or freelance caller is rejected, mirroring `hera_spawn_worker`): `hera_plan_node` creates a planned node, `hera_block` adds a blocking edge (cycle-checked, single-orchestrator), and `hera_plan` submits a whole graph of nodes and edges in one call. The tools SHALL be available only when the hera service is wired AND task management is enabled (caller resolution via `cwd` requires task management). A dup-tool guard SHALL suppress any plugin tool scoped `hera` while native Hera is enabled, so in-tree and plugin tools never both appear. +The system SHALL register eighteen native `hera_*` MCP tools with the same names, parameters, descriptions, and required lists as the external Hera daemon where they overlap: `hera_new_orchestrator`, `hera_join`, `hera_move`, `hera_rebind`, `hera_send`, `hera_inbox`, `hera_mark_read`, `hera_status`, `hera_spawn_worker`, `hera_tree_updates`, `hera_get_messages`, the three plan-authoring tools `hera_plan_node`, `hera_block`, and `hera_plan`, the three plan-mutation verbs `hera_plan_node_update`, `hera_unblock`, and `hera_plan_node_cancel`, and `hera_revive`. The plan-authoring tools SHALL be coordinator-only (a worker or freelance caller is rejected, mirroring `hera_spawn_worker`): `hera_plan_node` creates a planned node, `hera_block` adds a blocking edge (cycle-checked, single-orchestrator), and `hera_plan` submits a whole graph of nodes and edges in one call. `hera_revive` SHALL likewise be coordinator-only. The tools SHALL be available only when the hera service is wired AND task management is enabled (caller resolution via `cwd` requires task management). A dup-tool guard SHALL suppress any plugin tool scoped `hera` while native Hera is enabled, so in-tree and plugin tools never both appear. + +NOTE: this requirement's tool count/list previously read "fourteen" and omitted the three plan-mutation verbs (a pre-existing drift from when `make-hera-plan-living` added them) — corrected here to the accurate eighteen while adding `hera_revive`, since both changes touch this same sentence. #### Scenario: Tools require task management @@ -41,10 +43,10 @@ The system SHALL register fourteen native `hera_*` MCP tools with the same names - **WHEN** a worker or freelance role calls `hera_plan_node`, `hera_block`, or `hera_plan` - **THEN** the tool errors that only coordinators may author the plan -#### Scenario: Whole-graph submission in one call +#### Scenario: hera_revive is coordinator-only -- **WHEN** a coordinator calls `hera_plan` with a set of nodes and blocking edges -- **THEN** the planned nodes and their cycle-checked edges are created together +- **WHEN** a worker or freelance role calls `hera_revive` +- **THEN** the tool errors that only coordinators may revive a role ### Requirement: Caller role resolution from cwd with orchestrator disambiguation @@ -381,12 +383,13 @@ The system SHALL restore a worker-bound task from in_review back to in_progress The restore SHALL NOT fire when the worker is awaiting coordinator close-out — that is, when its bound task carries `meta:hera.ready_to_close` (the BUG-050 done / clean-exit stamp) OR any of its live worker roles has a terminal role-status (`done` or `failed`). This guard preserves the PR #707 / BUG-050 invariant: a genuinely-finished worker stays in_review even when its idle session is still alive, because a worker never self-completes — the coordinator/human closes it out or decides on a failure. -Two trigger sites share the helper so they cannot drift: +Three trigger sites share the helper so they cannot drift: - The daemon's supervisor-mode startup reattach (`reattachSupervised`) calls it for every task the supervisor confirms ALIVE, so a live worker stranded in in_review by a prior roll or reconcile is restored to in_progress on each bounce (the true orphans the supervisor does NOT report alive still flip the other way, to in_review). - The TUI's live-session revive (`reviveHeraWorker`, the Enter-key in-place `KickRerender` resume) calls it on a successful kick (local store only; `--remote` mode defers to the live local daemon's own reattach restore). +- The `hera_revive` MCP tool's shared `hera.ReviveRole` primitive calls it on a successful kick, identically to the TUI's own call (add-hera-revive). -Derived from: `internal/db/hera.go` (`ReviveHeraWorkerToInProgress`), `internal/daemon/bounce.go` (`reattachSupervised`), `internal/tui/app.go` (`reviveRestoreInProgress`) + `internal/tui/heraactions.go` (`reviveHeraWorker`). +Derived from: `internal/db/hera.go` (`ReviveHeraWorkerToInProgress`), `internal/daemon/bounce.go` (`reattachSupervised`), `internal/tui/app.go` (`reviveRestoreInProgress`) + `internal/tui/heraactions.go` (`reviveHeraWorker`), `internal/hera/revive.go` (`ReviveRole`). #### Scenario: Stranded live worker is restored on reattach @@ -413,6 +416,11 @@ Derived from: `internal/db/hera.go` (`ReviveHeraWorkerToInProgress`), `internal/ - **WHEN** the task is coordinator-bound, holds no live worker binding, or is not currently in_review (in_progress / complete / pending) - **THEN** the restore no-ops and the status is left unchanged +#### Scenario: hera_revive's kick restores in_progress identically to the TUI + +- **WHEN** a coordinator calls `hera_revive` on a stuck worker and the kick succeeds +- **THEN** the same `ReviveHeraWorkerToInProgress` guard applies (restored unless awaiting close-out), exactly as the TUI's Enter-key kick + ### Requirement: hera_move relocates the caller's binding to a different orchestrator The system SHALL, on `hera_move`, relocate the calling task's live hera binding to a different orchestrator: it SHALL resolve the caller's current live binding (via the same cwd→task→binding resolution used elsewhere, accepting an optional `from_orchestrator` to disambiguate when the task holds 2+ live bindings), then — transactionally — end that binding (`ended_at`/`end_reason: "moved"`) and create a new role+binding of kind `worker` or `freelance` under the target `orchestrator` (rejecting `coordinator`, mirroring `hera_join`). It SHALL reject the call, ending and creating nothing, when the calling task holds no live binding at all (directing the caller to `hera_join` or `hera_new_orchestrator` instead — there is nothing to move), when the resolved source orchestrator equals the target orchestrator (a no-op; directing the caller to `hera_join` without `role_name` to see its current binding), or when the resolved SOURCE binding's role is coordinator-kind (a coordinator's binding IS its orchestrator's coordination — ending it would orphan the whole subtree the coordinator was running, leaving a disconnected worker/freelance stub under the target with no structural link back; the rejection names the caller's role and orchestrator and directs the caller to ask a human to use the Hera TUI's `J` adopt/reparent key instead, since no agent-facing tool nests an existing coordinator + subtree under a new parent). The response SHALL report the source orchestrator and role name that were moved, plus the new binding id. Required args: `cwd`, `orchestrator`, `role_name`, `kind`. Optional args: `from_orchestrator`, `status`. @@ -482,3 +490,73 @@ Derived from: `internal/mcp/hera.go:901` (`toolHeraRebind`). - **WHEN** hera_rebind(cwd, orchestrator=X) is called and no live binding for X exists at the caller's worktree or task - **THEN** the tool errors directing the caller to hera_join, and creates no binding +### Requirement: hera_revive coordinator PULL-revive of a bound role + +The system SHALL provide a coordinator-only `hera_revive(cwd, role_name, [orchestrator])` MCP tool that inspects one role the caller coordinates and, based on its live session state, takes exactly one of the following actions, always reporting which one: + +- **No live session** (dead, of any role kind including a nested coordinator) → restart it in place, resuming via `--session-id` when the task carries one. Reports `restarted_dead`. +- **Live, role kind is `coordinator`** → left untouched (a live coordinator is presumed operator-interactive). Reports `skipped_coordinator_live`. +- **Live, non-coordinator, no session id to resume** → left untouched. Reports `skipped_no_session_id`. +- **Live, non-coordinator, a kick/restart is already in flight** → left untouched. Reports `skipped_restart_pending`. +- **Live, non-coordinator, not idle** (busy) → left untouched. Reports `skipped_busy`. +- **Live, non-coordinator, idle, but blocked on a user prompt** (selection UI or trailing question) → left untouched, so the pending question is never dismissed. Reports `skipped_blocked_on_prompt`. +- **Live, non-coordinator, idle, not blocked** (genuinely stuck — e.g. SIGTSTP'd by a session-supervisor restart) → kicked (stop + resume in place at its existing PTY size), and, on success, restored from `in_review` back to `in_progress` via the shared `ReviveHeraWorkerToInProgress` helper (best-effort; a restore failure does not fail the tool call). Reports `kicked_stuck`. + +This is the SAME safety gate the TUI's `Enter`-key revive path (`heraReattach`/`reviveHeraWorker`) already enforces, reusing the same underlying primitives (`agent.BlockedOnPrompt`, `db.ReviveHeraWorkerToInProgress`, `agent.SessionRunner.KickRerender`/`StartOrReattach`) via a new shared decision function, `internal/hera.ReviveRole` — see `openspec/changes/add-hera-revive/design.md` D3 for why the TUI's own call site is not itself refactored to share this function. + +The tool SHALL reject a non-coordinator caller, an unknown `role_name` within the caller's orchestrator, a `role_name` resolving to the caller's own (live, calling) role, and a role with no live binding (planned-but-not-materialized, or ended). + +This is strictly pull/on-demand: nothing in the daemon calls `hera_revive` automatically. A coordinator calls it when it notices no progress from a role (e.g. via `hera_status`/`hera_tree_updates`). + +Derived from: `internal/hera/revive.go` (`ReviveRole`), `internal/daemon/revive.go` (`HeraReviveRunner`), `internal/mcp/hera.go` (`toolHeraRevive`). + +#### Scenario: Dead role is restarted + +- **WHEN** a coordinator calls `hera_revive` on a role with no live session +- **THEN** the session restarts in place (resuming via `--session-id` when the task has one) and the tool reports `restarted_dead` + +#### Scenario: Stuck worker is kicked and restored to in_progress + +- **WHEN** a coordinator calls `hera_revive` on a live worker role that is idle and not blocked on a prompt +- **THEN** the session is kicked (stopped and resumed in place) and, if the worker was parked in in_review awaiting nothing, its task is restored to in_progress; the tool reports `kicked_stuck` + +#### Scenario: Live coordinator is never auto-revived + +- **WHEN** a coordinator calls `hera_revive` targeting a live nested coordinator role +- **THEN** nothing is restarted and the tool reports `skipped_coordinator_live` + +#### Scenario: Busy role is left alone + +- **WHEN** a coordinator calls `hera_revive` on a live, non-idle worker/freelance role +- **THEN** nothing is restarted and the tool reports `skipped_busy` + +#### Scenario: Role blocked on a prompt is left alone + +- **WHEN** a coordinator calls `hera_revive` on a live, idle worker/freelance role that is parked at a user prompt +- **THEN** nothing is restarted (the pending question is preserved) and the tool reports `skipped_blocked_on_prompt` + +#### Scenario: A kick already in flight is not duplicated + +- **WHEN** a coordinator calls `hera_revive` on a role that already has a pending kick/restart queued +- **THEN** no second kick is queued and the tool reports `skipped_restart_pending` + +#### Scenario: Non-coordinator caller is rejected + +- **WHEN** a worker or freelance role calls `hera_revive` +- **THEN** the tool errors that only coordinators may revive a role + +#### Scenario: Unknown role name is rejected + +- **WHEN** `role_name` does not resolve to any role in the caller's orchestrator +- **THEN** the tool errors that the role was not found + +#### Scenario: Targeting one's own role is rejected + +- **WHEN** `role_name` resolves to the calling coordinator's own role +- **THEN** the tool errors rather than silently reporting `skipped_coordinator_live` + +#### Scenario: A planned-but-unmaterialized or ended role is rejected + +- **WHEN** `role_name` resolves to a role with no live binding +- **THEN** the tool errors that the role has no live binding +