From 21e462ed8920c7d6a01a72db6554a30b5157393d Mon Sep 17 00:00:00 2001 From: Aaron Newton Date: Sun, 2 Aug 2026 17:11:42 -0700 Subject: [PATCH] Auto-revive a dead or stuck hera_send recipient (add-hera-send-auto-revive) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A coordinator sending to an explicit, different recipient now revives it first via the existing hera_revive/ReviveRole primitive, reused verbatim. Soft-fail throughout: no live binding, a lookup error, a revive error, or the reviver not being wired all skip silently and never block the send. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- context/knowledge/gotchas/messaging.md | 1 + context/knowledge/index.md | 2 +- internal/mcp/hera.go | 28 +++ internal/mcp/hera_send_revive_test.go | 212 ++++++++++++++++++ .../.openspec.yaml | 2 + .../design.md | 38 ++++ .../proposal.md | 27 +++ .../specs/hera-messaging/spec.md | 44 ++++ .../tasks.md | 33 +++ openspec/specs/hera-messaging/spec.md | 46 +++- 10 files changed, 430 insertions(+), 3 deletions(-) create mode 100644 internal/mcp/hera_send_revive_test.go create mode 100644 openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/.openspec.yaml create mode 100644 openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/design.md create mode 100644 openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/proposal.md create mode 100644 openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/specs/hera-messaging/spec.md create mode 100644 openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/tasks.md diff --git a/context/knowledge/gotchas/messaging.md b/context/knowledge/gotchas/messaging.md index 866f3893..6f73e5d0 100644 --- a/context/knowledge/gotchas/messaging.md +++ b/context/knowledge/gotchas/messaging.md @@ -190,3 +190,4 @@ true)` and `db.Delete(id)`; entrypoints that go through `db.Update` done keeps role active + messageable" / "explicit archive removes role from recipient resolution"). Do NOT "fix" a recipient bounce by making archived roles messageable — done must not archive; explicit archive must. +- **`hera_send` auto-revives an explicit-`to` recipient before delivering (add-hera-send-auto-revive)** — a coordinator sending to a DIFFERENT, explicitly-named role (never the worker/freelance default-to-coordinator route, never a self-send) reuses `internal/hera.ReviveRole`/`s.heraRevive` VERBATIM, the exact same primitive `hera_revive` calls — no new gating logic. It is entirely soft-fail: no live binding (`db.ErrHeraNotFound`, the common planned/never-spawned/ended case), a lookup error, a revive error, or `s.heraRevive == nil` (reviver not wired) all skip silently or Warn-log, and the message send proceeds and succeeds/fails purely on its own merits either way. A successful attempt renders a `- **revive**: ` line via the shared `heraReviveOutcomeMessage` and logs the identical `slog.Info("[hera] revive", ...)` line `hera_revive` emits — an auto-triggered revive is indistinguishable in logs from a manual one. diff --git a/context/knowledge/index.md b/context/knowledge/index.md index 00ad5823..412b1f5a 100644 --- a/context/knowledge/index.md +++ b/context/knowledge/index.md @@ -17,7 +17,7 @@ Non-obvious invariants and gotchas, split by topic. Read the relevant file when | [gotchas/orchestration.md](gotchas/orchestration.md) | depends_on DAG / depswatcher / link-unlink-halt / plan_slug all RETIRED (Hera is the single model; base_branch kept; tasks start immediately), task_set_result opaque-to-daemon, ARGUS_TASK_ID env export, (name,project) idempotency, schema column ordering, hera schema/store M1 (FK-cascade, NULL partial-index, per-(task,orchestrator) multi-binding + ErrHeraAmbiguous, transactional role+binding), hera M4 (born-bound spawn + AfterPersist hook; auto-adopt removed; ReconcileBindings keyed on task-row; HeraCoordinatorOrientation headline = coord DISPATCHES not implements (actual work → hera_spawn_worker regardless of repo; self-invoking hera_new_orchestrator = relabel-and-implement-solo antipattern, not a sub-team); new orch only for multi-project/phase sub-team whose WORKERS do the work; hera_new_orchestrator CODE guard rejects a caller already coordinating a DIFFERENT orch, before orch-create, fail-open, MCP-only), hera M5 (subtree TLDR roll-up: SubtreeOrchIDs BFS + cycle guard, tree_read_cursors cascade, advisory per-role cursor), hera plan-DAG substrate (planned node = role w/ no binding, gate on role-status done, failed-blocker → HOLD + ping coordinator, in-tx DFS cycle check, check-in pulled via inbox), hera_join reject+redirect to new hera_move tool (fix-hera-join-move-binding; self-promotion remains the only 2+-binding path), coordinator context management (coord-hook global-settings requirement + ARGUS_TASK_ID-first self-gate, unconditional context_size stamp vs budget-gated block, presence-=-block no-cooldown nudge, hera_status handoff_note/request_recycle widened to ANY hera-bound role kind not just coordinator (add-worker-bounce), request_recycle as flag-only vs watcher-driven action, recycle_coord self-service idle-wait vs human-forced immediate (human-forced stays coordinator-only), RecycleWatcher.tickTask role-kind filter widened to worker/freelance (coordinator-kind still preferred among 2+ live bindings), SessionID-clear-before-Recycle ordering, best-effort stray-job cleanup, zero-follow-up-call seed prompt, B key bounces worker/freelance via self-service instruct-and-wait instead of no-op (add-worker-bounce), no-op only on an empty selection), coord-hook stamp gate ALSO widened coordinator-only → any hera-bound role for context_size specifically (add-worker-context-indicator, budget/nudge/recycle stays coordinator-only — a separate widening from add-worker-bounce's hera_status/RecycleWatcher/B-key changes above), RecycleRunner.Restart must resolve via the caller-supplied roleID not a re-derived taskID-keyed lookup (fix-recycle-restart-ambiguous-binding, dual-bound self-service recycle previously wedged forever on ErrHeraAmbiguous), hera plan-DAG hygiene (add-hera-plan-hygiene: planned-node parent-orchestrator liveness filter + cascade-cancel on archive/nuke is belt-and-braces not either-alone, materializeNode consecutive-failure escalation mirrors holdAndPing's ping-once dedup not EscalateParkedSelection's torn-read threshold, freelance-role flat-section staleness is data-hygiene not a display bug) | 78 | | [gotchas/dag-rendering.md](gotchas/dag-rendering.md) | dagview is now a LAYOUT LIBRARY (dagview.Compute longest-path stage placement) consumed by internal/tui/planview — standalone widget no longer mounted; PLAN-DAG widget (planview) replaced the retired orchestration-tree graph, heraTreeNodes/tree.go DELETED. Sugiyama-lite layer math, rune-vs-byte truncation, single-line edges, branch-change log-only. planview node State/colour from RoleView.TaskStatus/TaskResult ({"failed":true}→red ✕); planview surfaces OnEnter/OnDrillIn/OnDrillOut/OnBranchChange (read-only nav, no edit callbacks); branchShape folds cursor+fanned-group+orch-title | 18 | | [gotchas/hera-view.md](gotchas/hera-view.md) | Native Hera view (`internal/tui/hera`): 2nd tab display label "Projects" (internal names stay Hera); 2nd tab always native (cfg.Hera.Enabled gates only daemon MCP tools, not tick refresh); structural multi-binding fan-out; freelance section; ready_to_close from task_meta; goroutine-free debounced Refresher on UI thread; rail nav j/k/Up/Down + Space (tab nav 1/2/3 only); no Sync + full-rect coverage. Panes fed from in-process runner ring (poll, not SSE); coord-vs-agent session rule; multi-binding disambiguated by role.OrchID; PTY align via ForceResyncPTY + off-thread SyncPanes. Thin mutation layer (ops.go over M1; shared agent.SpawnHeraWorker); rail keyset via OnXxx callbacks; multi-binding isolation; s/S step hera ROLE status; modal.ConfirmModal + NewInputForm; remote=nil ⇒ inert. Details 2 modes (worker→terminal, coordinator→stacked roster-over-PLAN — same geometry as roster-over-tree); embedded PLAN-DAG graph via heraPlanNodesWithBridge(orch, bridgeIndex) — coordinators not plan nodes, Drillable needs WithBridge form; planview↔hera import one-way (hera imports planview); OnDrillIn page-owned (drillIntoChild→bridgeIndex→PushOrch), OnEnter App-owned; handleDetailsKey Esc-at-root escapes pane; node colour from TaskStatus/TaskResult. Ctrl+Z→fullscreen (closes the Claude-Code-own-supervisor detach footgun, worse than a mere SIGTSTP); Enter revives dead (startSession) or suspended worker (reviveHeraWorker→KickRerender, idle+not-blocked gated; live coordinator navigate-only); jumpToLeaf expands ancestor coordinators (EnsureAncestorsExpanded via canonicalParents + OrchIDsForTask) before SelectByTaskID so a folded coord doesn't swallow the join (BUG-007); J-detach (DetachCoordinator) = re-parent teardown without recreate, shared teardownParentLinks, idempotent, detach sentinel by pointer identity; nested sub-coord = headerless worker-bridge row → heraCoordReparentTarget qualifies worker w/ BridgeChildOrchID!=0 so J detach/re-parent reaches the child orch (plain worker never misclassified); needs-input rail gate is LIVENESS-based for ALL kinds (buildRoleView `taskInProgress || rv.Live`; live worker/coordinator/freelance surfaces (?) regardless of task status, incl. a worker in in_review per #707 — needsInputForHeraRail admits the heraManaged union (workers+coordinators), BUG-A supersedes the BUG-028 worker-only carve-out; BUG-023 now guarded by binding-liveness not task-status; flat task-list stays in_progress-gated); needs-input OUTRANKS ready_to_close in RoleStatusIcon (an actively-blocked worker is not ready to close, BUG-A); coordinator-less orch header surfaces rollup via OrchView.SubtreeNeedsInput in drawOrchRow else-if (BUG-028); content-aware spinner (RoleView.IsActive gated on !SessionIdle, fed from App content-idle set so a parked fullscreen agent stops animating, BUG-036); IsActive spinner OUTRANKS ready_to_close/failed/done in RoleStatusIcon (BUG-F, icon-precedence completion of BUG-C; resting case kept via IsActive's running/!idle gate); bulk cascade-nuke silent multi-second freeze (BUG-062, no data race — synchronous per-task session-stop RPC on the tview goroutine) fixed via backgrounded stop (heraGoSafe) + SyncPTYSize panic recovery mirroring Draw(); kanban_status (add-hera-kanban-status) independent 4th axis on top-level coordinators (active/backlog/blocked/done, default active, hera_orchestrators column no CHECK) grouping the rail's Active bucket with dividers, stepped by m/M (wraps, distinct from s/S role-status clamp); needs-input rollup EXCLUDES archived nodes (exclude-archived-from-needs-input-rollup) via a DEDICATED archive-aware orchSubtreeNeedsInput walk (not BridgeSubtree reuse — BridgeSubtree keeps archived rows for dimmed-in-place rendering), gating descent on the bridging role's Archived AND the worker-bridge target orch's own !c.Archived; archived role's own row still shows (?); rail Enter-reattach `live` check now tests Alive() not mere non-nil (BUG-064, shared HeraPage.sessionLive) — a cached-but-disconnected coordinator handle (BUG-013) used to make the first Enter skip reattach and only focus, requiring a second Enter routed through the pane's own InputHandler to actually restart; kanban groups auto-fold to the focused group (add-kanban-focus-fold): Active gains a uniform header/divider (headerless special-case retired), Rail.focusedKanban resolved BEFORE buildRows via focusGroupOf (chicken-and-egg), step() boundary-crossing expand/collapse via landOnGroupMember, SetModel/SelectByTaskID/EnsureAncestorsExpanded each independently re-focus, not persisted; ctrl+j switcher literal case (mirrors Ctrl+Z) + exported JumpToTask (jumpToLeaf now a thin wrapper); ctrl+k global palette Hera reach (two enumerated non-keymap literal rows — fullscreen/copy — + heraRailActionRegistry over existing OnXxx callbacks); rail partial-fold reveal (appendOrchWorkers/appendWorkerRow revealOnly mode + appendOrchRevealPath, extends the one true traversal rather than forking a parallel one, fold state never mutated); BUG-064 (nested-reveal-lost-on-reexpand variant): appendWorkerRow's bridged-child branch needed the same else-if child.SubtreeNeedsInput fallback appendPinnedRole already had, else re-expanding an outer coordinator while a nested sub-coordinator stayed collapsed silently dropped the nested needs-input leaf; ctrl+g jump-to-next-needs-input (Rail.NextNeedsInputTaskID scan-and-cycle over built row order + HeraPage.JumpToNextNeedsInput reusing JumpToTask verbatim; candidates require row.role!=nil so a top-level coordinator's own need — folded into the rrOrch header, unreachable via SelectByTaskID — is deliberately excluded, unlike a nested sub-coordinator's bridging row), worker/freelance context-pressure indicator (add-worker-context-indicator: always-reserved trailing 2-col slot, coordinator-excluded, local-mode-only ContextPercent, bare coordinator count), context-size undercounting root cause is transcript_path's documented async-write lag not sidechains (fix-context-stop-lag: no Stop-hook field carries usage data, last_assistant_message is plain text; bounded retry-and-take-max in readContextSizeReal is the fix, gated by an early exit — contextSizeReadPrevious seam compares one scan against the task's prior stamp, skipping the retry unless the scan is below-prior or there's no prior stamp yet, so the ~200ms budget isn't an unconditional per-turn tax across the whole coordinator/worker/freelance fleet; isSidechain skip kept as cheap defensive hardening, empirically a no-op under the current CLI); ctrl+g/ctrl+b excursion re-arm was count-based not identity-based (BUG-069, live dogfood repro) — a stale never-resolved needs-input role kept the count >=1 forever so a restore re-armed and froze on the very next rebuild regardless of novelty, silently discarding the operator's post-restore navigation; fixed via role-ID set-membership tracking (`Rail.armedNeedsInputIDs`/`Model.needsInputRoleIDs`/`hasNewNeedsInputID`) that continuously refreshes until a genuinely new distinct role id appears; separately confirmed (not fixed) a narrow pre-existing `currentRef()`/`restoreCursor` gap shared with BUG-002 for cursor-on-fold-row capture; the identity-tracking fix still froze a bogus snapshot on a Rail's first-ever `SetModel` call when stale needs-input predated a TUI launch/relaunch (BUG-070, discovered dogfooding BUG-069 — no rows yet for `currentRef()`, fold state still the previous session's persisted layout) — fixed via a `r.rows == nil` early-return guard (seed-only, no capture) on the literal first call; the partial-fold reveal was stateless across rebuilds, so a selected role revealed only via needs-input vanished (yanking cursor + panes) the instant its own flag cleared (BUG-071) — fixed via `Rail.applyStickyReveal` forcing `SubtreeNeedsInput` along the selected row's ancestor chain in the fresh model before `buildRows`, re-derived from the current cursor identity every rebuild so it releases the moment selection moves elsewhere; size-drift kill+resume kick extended to Hera panes (BUG-074, `heraKickRerender`/`maybeKickPaneRerender`) — a plain `ForceResyncPTY()` can't repair scrollback already committed at a different width, and Hera panes are MORE exposed than the main agent view since `bindPane` resizes on every single bind; evaluated from `Draw()` (fresh per-pane width via `coordKickedFor`/`agentKickedFor`), never from `bindPane` itself (whose tracked width can still be 0 for a pane not yet shown, e.g. the agent pane during details mode); BUG-076 (false "Session not running" after ordinary rail nav) — root cause was `handleSessionExitUI`'s post-kick auto-restart gate checking ONLY `a.mode==modeAgent`, never true on the Hera tab, so every BUG-074 size-drift kick fired from a Hera pane genuinely stopped the session and then always skipped the restart as "user navigated away"; fixed via `App.isViewingTaskSession`/`HeraPage.IsBoundToTask` recognizing the Hera-tab-with-bound-pane case too | 170 | -| [gotchas/messaging.md](gotchas/messaging.md) | task_messages caps (64 KiB / 500 unread / 50/min), self-send rejection, recipient existence check, reliable-notify delivery (single-writer, Ctrl+U pre-clear, CR not LF, 5-min deadline, ack-cancels), archive cleanup, task_ask polling, REST send; hera M2 (read_at NULL invariant, enqueue-time delivery stamp, hera: delivery-ID prefix, eager inbox cancel, doorbell trust boundary, worker-done-must-not-archive-role) | 33 | +| [gotchas/messaging.md](gotchas/messaging.md) | task_messages caps (64 KiB / 500 unread / 50/min), self-send rejection, recipient existence check, reliable-notify delivery (single-writer, Ctrl+U pre-clear, CR not LF, 5-min deadline, ack-cancels), archive cleanup, task_ask polling, REST send; hera M2 (read_at NULL invariant, enqueue-time delivery stamp, hera: delivery-ID prefix, eager inbox cancel, doorbell trust boundary, worker-done-must-not-archive-role, hera_send auto-revive-on-send reuses ReviveRole verbatim + soft-fail never blocks delivery) | 34 | | [gotchas/remote-tui.md](gotchas/remote-tui.md) | `--remote URL --token` mode: apiclient + apistore architecture, two compile-time assertions, four TUI sites that type-assert to *db.DB for local-only ops, raw endpoints for full model.Task round-trip, 30s config refresher, daemon-admin actions that don't apply remotely | 13 | | [gotchas/events.md](gotchas/events.md) | Events ring + SSE stream: emission-outside-mu invariant, subscribe-before-snapshot fencing, sink save/restore in tests, ring eviction shape, idle watcher unconditional-run, task.completed double-emit, session.needs_input daemon-authoritative idle-gated sticky watcher, never-idle-parked-prompt flagged via content-stability fingerprint (BUG-032, streaming false-positive guard), emulated-screen detection for cursor-addressed alt-screen prompts (BUG-033, ScreenRenderer reuse-via-RIS, raw fast-path + emulate-on-miss), needs-input sticky flag clears on input-delivered-or-archive not signal-decay (BUG-034, shared agent.NeedsInputClear + needsInputSince baseline; clear filter reads LastUserInput NOT LastInput — system reliable-notify delivery uses WriteInputSystem so it never clears a parked worker's autonomous (?), the BUG-034 regression fix), never-idle pass flags free-text endsInQuestion gated on the working-affordance ("esc to interrupt") being ABSENT — content-stability ALONE re-breaks BUG-032 (BUG-035 GAP A, agent.AwaitingInputFingerprint replaces SelectionPromptFingerprint); selection matches any numbered option + wording-tolerant chooser footer (BUG-035 GAP B), content-aware idle for fullscreen agents (agent.ContentIdle emulated-screen stability + working-affordance gate, parallel to Session.IsIdle NOT folded into it; idle-push once-on-transition via shouldFireIdlePush cycle gate; RoleView.SessionIdle suppresses the rail spinner, BUG-036), never-converging content fingerprint escalates via a bounded consecutive-tick counter rather than loosening the chrome allowlist (BUG-029, agent.ParkedSelectionSignal + agent.EscalateParkedSelection, NeedsInputEscalationTicks=8, separate path from ContentFingerprint itself), escalation counter's original all-or-nothing reset was fragile against an isolated single-tick detection miss (blinking cursor glyph, or a torn read racing the daemon's concurrent log-file writer) — a genuinely, continuously-parked hera worker could never reach the threshold, explaining a live "first sibling flags reliably, later siblings under the same coordinator never do" repro; fixed via a one-tick grace period (negative-sentinel encoding, `escalated` stays true through an already-past-threshold grace tick to avoid flicker) rather than loosening detection itself (BUG-060), a fixed-size tail window can be PERMANENTLY (not just occasionally) flooded by Claude's blinking-cursor redraw until real content falls out of reach — deterministic 100%-miss confirmed via live repro, not a torn read (BUG-061, agent.SubstantiveTail expand-on-degenerate-tail read + degenerateSuffixStart raw-byte periodicity trim, wired into both the TUI disk-log read and the push watcher's ring-buffer read; sticky carry-forward in both detectNeedsInputSticky and computeNeedsInput no longer re-requires a fresh tail match, agent.NeedsInputClear is the only clear path), no hera adopt/reconcile loop on the ring, a cleared flag can be PERMANENTLY re-stuck by a stale re-candidacy after a candidacy gap (BUG-063, NeedsInputClear baseline forgotten the instant a task drops out of `candidates` even for one tick; fixed via a `running`-scoped cleared-marker (`prevCleared`/`newCleared`) that survives the gap and suppresses a same-timestamp re-candidacy; accepted scope limit: can't distinguish stale content from a genuinely distinct second prompt at the same timestamp), a hera coordinator's relayed answer (WriteInputSystem) could never clear the flag through BUG-034's own user-input path even after the worker demonstrably resumed real work (BUG-065, NeedsInputClear gained a third `resumedOf` clear condition fed by agent.ResumeActivityTick — mirrors EscalateParkedSelection but tracks sustained "working"-affordance ticks with no grace period on a miss, since under-clearing is safe but a false clear is not), a role's SELF-REPORTED hera_status="blocked" is a wholly separate signal ORed into the same rail (?) glyph (RoleView.needsInputOwn) with no auto-clear of its own — set only by an explicit hera_status tool call or manual s/S, so a direct pane reply never cleared it (BUG-066, agent.ClearBlockedRoleStatus: direct-reply-after-blockedAt clears immediately with no threshold, OR the same BUG-065 resumed-activity signal for a coordinator-relayed answer; db.ListBlockedHeraRoleBindings/ClearBlockedRoleStatus read/write split; App.autoClearBlockedHeraRoles + Server.autoClearBlockedHeraRoles run as a separate small pass scoped to the usually-empty blocked set, not folded into computeNeedsInput/detectNeedsInputSticky), BUG-063's own accepted scope limit resurfaces (and is fixed) in a multi-question AskUserQuestion/brainstorm flow — a SEPARATE, pre-existing bug from BUG-066, not a #904 regression, confirmed via a cross-task shared-ScreenRenderer test that disproves contamination (BUG-067, NeedsInputClear gained a fingerprintOf param + ClearedMarker{At,FP,HasFP} replacing the plain timestamp marker so a stale-recandidacy suppression additionally requires matching CONTENT, not just timestamp — a distinct later prompt at the identical lastInputOf timestamp now re-arms instead of being silently swallowed), a worker that resolves its own block and settles into idle FASTER than the resumed-activity threshold had NO clear path at all — stuck until an incidental keystroke (BUG-072, NeedsInputClear gained a fourth `settledOf` clear condition fed by agent.SettleTick — re-runs the SAME idle-gated signal check that raises the flag as a negative/clearing signal, gated on genuine Session.IsIdle() so it can never conflate with BUG-061's flooding hazard, small NeedsInputSettleTicks=2 threshold since idle rules out flooding by construction) | 20 | | [gotchas/macos-app.md](gotchas/macos-app.md) | Native macOS app (`macos/` SwiftPM): `swift test` silently runs ZERO tests on CLT-only (exits 0, failures "pass") ⇒ suite is an executable target run via `make mac-test`; SwiftPM sandbox can't NEST in an argus agent sandbox ⇒ `--disable-sandbox` on all mac-* targets; swift-testing on a non-test target needs explicit -F/-rpath/-plugin-path probed via FileManager (manifest sandbox forbids subprocesses); `_Concurrency.Task` never bare `Task` (ArgusKit Task model shadows it); stream state machines' streamOpening() re-arms reconnect-on-failure (else retries die forever); subscribe-before-snapshot event fencing client-side (buffer stream → snapshot /api/tasks → drain; resync/unknown re-snapshot never crash); TerminalControllers cached per task ID, pruned only on snapshot disappearance; `bytes.lines` swallows empty lines ⇒ SSE parsed via ByteLineSplitter raw-byte iteration (else no event ever dispatches + spliced-SGR garbage); SwiftTerm TerminalView never takes first responder under SwiftUI hosting ⇒ FocusTakingTerminalView (else terminal is read-only); Ctrl+Z (0x1A) stripped from outbound terminal input ⇒ pure `ArgusKit.TerminalInput.sanitize` called at `TerminalCoordinator.send` (else Claude Code's background-session supervisor orphans the session; TUI parity, swallow-not-remap); `open Foo.app` drops env ⇒ ARGUS_MAC_* hooks need direct binary exec | 12 | diff --git a/internal/mcp/hera.go b/internal/mcp/hera.go index 82cddecc..5cba11f8 100644 --- a/internal/mcp/hera.go +++ b/internal/mcp/hera.go @@ -1271,6 +1271,31 @@ func (s *Server) toolHeraSend(id interface{}, args json.RawMessage) *Response { } } + // Auto-revive (add-hera-send-auto-revive): a coordinator sending to an + // explicitly named, different role gets the exact same PULL-revive + // hera_revive already provides — reused verbatim via s.heraRevive / + // internal/hera.ReviveRole, no new gating logic. Soft-fail throughout: a + // missing binding, a lookup error, a revive error, or heraRevive not + // being wired all skip silently (or Warn-log) and never block the send. + var reviveOutcome string + if caller.role.Kind == db.HeraKindCoordinator && p.To != "" && toRole.ID != caller.role.ID && s.heraRevive != nil { + binding, bindErr := heraLiveOrNil(s.heraStore.HeraLiveBindingByRole(toRole.ID)) + if bindErr != nil { + slog.Warn("[hera] send: auto-revive binding lookup failed", "to_role", toRole.Name, "err", bindErr) + } else if binding != nil { + outcome, reviveErr := s.heraRevive(HeraReviveInput{ + TaskID: binding.ArgusTaskID, + IsCoordinator: toRole.Kind == db.HeraKindCoordinator, + }) + if reviveErr != nil { + slog.Warn("[hera] send: auto-revive failed", "to_role", toRole.Name, "task_id", binding.ArgusTaskID, "err", reviveErr) + } else { + reviveOutcome = outcome + slog.Info("[hera] revive", "orch", caller.orch.Name, "role", toRole.Name, "task_id", binding.ArgusTaskID, "outcome", outcome) + } + } + } + msg, err := s.heraSvc.Send(caller.role.ID, toRole.ID, p.Body, p.Tldr, p.InReplyTo) if err != nil { switch { @@ -1300,6 +1325,9 @@ func (s *Server) toolHeraSend(id interface{}, args json.RawMessage) *Response { fmt.Fprintf(&b, "- **message_id**: %d\n", msg.ID) fmt.Fprintf(&b, "- **to**: %s\n", toRole.Name) fmt.Fprintf(&b, "- **delivery_mode**: %s\n", msg.DeliveryMode) + if reviveOutcome != "" { + fmt.Fprintf(&b, "- **revive**: %s\n", heraReviveOutcomeMessage(reviveOutcome, toRole.Name)) + } return toolResult(id, b.String()) } diff --git a/internal/mcp/hera_send_revive_test.go b/internal/mcp/hera_send_revive_test.go new file mode 100644 index 00000000..81255017 --- /dev/null +++ b/internal/mcp/hera_send_revive_test.go @@ -0,0 +1,212 @@ +package mcp + +import ( + "encoding/json" + "errors" + "fmt" + "strings" + "testing" + + "github.com/drn/argus/internal/db" + "github.com/drn/argus/internal/testutil" +) + +// --- hera_send auto-revive (add-hera-send-auto-revive) --- +// +// These tests exercise toolHeraSend's new auto-revive attempt using the same +// fakeHeraReviver harness hera_revive_test.go defines: they assert whether +// the reviver was called, with what args, and how the outcome (or its +// absence) is rendered in the hera_send response — never a real PTY/runner. + +func TestHeraSend_AutoRevive_DeadRecipientRestartedBeforeSend(t *testing.T) { + s, d := testHeraServer(t) + coordWt, workerWt := setupOrchWithWorker(t, s, d) + _ = workerWt + + fr := &fakeHeraReviver{outcome: "restarted_dead"} + s.SetHeraReviver(fr.reviver()) + + orch, err := d.HeraOrchestratorByName("test-orch") + testutil.NoError(t, err) + workerRole, err := d.HeraRoleByName(orch.ID, "w1") + testutil.NoError(t, err) + binding, err := d.HeraLiveBindingByRole(workerRole.ID) + testutil.NoError(t, err) + + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_send", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"to":"w1","body":"wake up","tldr":"wake" + }`, coordWt)), + }) + 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, binding.ArgusTaskID) + testutil.Equal(t, fr.calledWith.IsCoordinator, false) + testutil.Contains(t, cr.Content[0].Text, "- **revive**:") + testutil.Contains(t, cr.Content[0].Text, "restarted") + testutil.Contains(t, cr.Content[0].Text, "Message sent") +} + +func TestHeraSend_AutoRevive_SkipOutcomesStillDeliver(t *testing.T) { + for _, outcome := range []string{ + "skipped_busy", + "skipped_blocked_on_prompt", + "skipped_coordinator_live", + "kicked_stuck", + } { + t.Run(outcome, func(t *testing.T) { + s, d := testHeraServer(t) + coordWt, _ := setupOrchWithWorker(t, s, d) + + fr := &fakeHeraReviver{outcome: outcome} + s.SetHeraReviver(fr.reviver()) + + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_send", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"to":"w1","body":"hi","tldr":"hi" + }`, coordWt)), + }) + 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.Contains(t, cr.Content[0].Text, "- **revive**:") + testutil.Contains(t, cr.Content[0].Text, "Message sent") + testutil.Contains(t, cr.Content[0].Text, "**to**: w1") + }) + } +} + +func TestHeraSend_AutoRevive_NoLiveBindingSkipsSilentlyAndSendSucceeds(t *testing.T) { + s, d := testHeraServer(t) + 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) + + fr := &fakeHeraReviver{outcome: "restarted_dead"} + s.SetHeraReviver(fr.reviver()) + + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_send", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"to":"planned-1","body":"hi","tldr":"hi" + }`, coordTask.Worktree)), + }) + testutil.NoError(t, respErr(resp)) + cr := callResult(t, resp) + if cr.IsError { + t.Fatalf("expected success (message still stored/attempted), got error: %s", cr.Content[0].Text) + } + testutil.Equal(t, fr.called, false) + if strings.Contains(cr.Content[0].Text, "- **revive**:") { + t.Fatalf("expected no revive line when recipient has no live binding, got: %s", cr.Content[0].Text) + } + testutil.Contains(t, cr.Content[0].Text, "Message sent") +} + +func TestHeraSend_AutoRevive_ReviverNilDoesNotBlockSend(t *testing.T) { + s, d := testHeraServer(t) + coordWt, _ := setupOrchWithWorker(t, s, d) + + // testHeraServer does NOT call SetHeraReviver — s.heraRevive stays nil. + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_send", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"to":"w1","body":"hi","tldr":"hi" + }`, coordWt)), + }) + testutil.NoError(t, respErr(resp)) + cr := callResult(t, resp) + if cr.IsError { + t.Fatalf("expected success, got error: %s", cr.Content[0].Text) + } + if strings.Contains(cr.Content[0].Text, "- **revive**:") { + t.Fatalf("expected no revive line when no reviver is wired, got: %s", cr.Content[0].Text) + } + testutil.Contains(t, cr.Content[0].Text, "Message sent") +} + +func TestHeraSend_AutoRevive_ReviveErrorDoesNotBlockSend(t *testing.T) { + s, d := testHeraServer(t) + coordWt, _ := setupOrchWithWorker(t, s, d) + + fr := &fakeHeraReviver{err: errors.New("boom")} + s.SetHeraReviver(fr.reviver()) + + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_send", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"to":"w1","body":"hi","tldr":"hi" + }`, coordWt)), + }) + testutil.NoError(t, respErr(resp)) + cr := callResult(t, resp) + if cr.IsError { + t.Fatalf("expected success despite revive error, got error: %s", cr.Content[0].Text) + } + testutil.Equal(t, fr.called, true) + if strings.Contains(cr.Content[0].Text, "- **revive**:") { + t.Fatalf("expected no revive line when the revive call itself errors, got: %s", cr.Content[0].Text) + } + testutil.Contains(t, cr.Content[0].Text, "Message sent") +} + +func TestHeraSend_AutoRevive_WorkerDefaultRouteNeverTriggers(t *testing.T) { + s, d := testHeraServer(t) + _, workerWt := setupOrchWithWorker(t, s, d) + + fr := &fakeHeraReviver{outcome: "restarted_dead"} + s.SetHeraReviver(fr.reviver()) + + // Worker sends with no explicit "to" → defaults to the (live) coordinator. + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_send", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"body":"status","tldr":"status","status":"working" + }`, workerWt)), + }) + 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, false) + if strings.Contains(cr.Content[0].Text, "- **revive**:") { + t.Fatalf("expected no revive line on the default worker->coordinator route, got: %s", cr.Content[0].Text) + } +} + +func TestHeraSend_AutoRevive_SelfSendNeverTriggers(t *testing.T) { + s, d := testHeraServer(t) + coordWt, _ := setupOrchWithWorker(t, s, d) + + fr := &fakeHeraReviver{outcome: "restarted_dead"} + s.SetHeraReviver(fr.reviver()) + + resp := doRequest(t, s, "tools/call", ToolCallParams{ + Name: "hera_send", + Arguments: json.RawMessage(fmt.Sprintf(`{ + "cwd":%q,"to":"coord","body":"hi","tldr":"hi" + }`, coordWt)), + }) + testutil.NoError(t, respErr(resp)) + cr := callResult(t, resp) + if !cr.IsError { + t.Fatal("expected the existing self-send rejection") + } + testutil.Contains(t, cr.Content[0].Text, "cannot send a message to self") + testutil.Equal(t, fr.called, false) +} diff --git a/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/.openspec.yaml b/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/.openspec.yaml new file mode 100644 index 00000000..e08b5f89 --- /dev/null +++ b/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-03 diff --git a/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/design.md b/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/design.md new file mode 100644 index 00000000..cace8f7e --- /dev/null +++ b/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/design.md @@ -0,0 +1,38 @@ +## Context + +`hera_revive` (`internal/mcp/hera.go:toolHeraRevive`, add-hera-revive) already gives a coordinator a pull/on-demand way to inspect one bound role's live session state and, based on it, restart a dead session, kick a stuck one, or leave it alone — via the shared, PTY-free-testable `internal/hera.ReviveRole` decision function. `hera_send` (`toolHeraSend`) resolves an explicit `to` recipient and hands off to `s.heraSvc.Send` with no session-liveness awareness at all. The gap this closes is purely "call the existing primitive automatically at the right moment in `hera_send`" — no new gating logic, no new MCP tool, no change to `ReviveRole` itself. + +## Goals / Non-Goals + +**Goals:** + +- A coordinator's `hera_send` to an explicit, different recipient transparently revives that recipient first, using the exact same outcome set and gating order `hera_revive` already uses. +- The revive attempt can never block, delay meaningfully, or fail the message send. Every failure mode (no live binding, lookup error, revive error, reviver not wired) is a silent or log-only skip. +- The coordinator can tell, from the `hera_send` response alone, whether the recipient was dead/stuck/fine — without a second round-trip. + +**Non-Goals:** + +- No change to `internal/hera.ReviveRole`, its outcomes, or its gating order — this change is a new CALL SITE only. +- No auto-revive for the worker/freelance default-to-coordinator send path. A worker/freelance's default recipient is "the active coordinator" — a live coordinator is already `ReviveRole`'s own `skipped_coordinator_live` case, so attempting it would almost always no-op; more importantly, only a coordinator has the authority to revive a role it coordinates (`hera_revive` itself is coordinator-only), and a worker/freelance sender is not the coordinator of its own coordinator. +- No auto-revive when the recipient equals the caller's own role (self-send is already rejected by `heraSvc.Send` on other grounds; the revive attempt is skipped before that rejection is even reached). +- No behavior change to `hera_revive` itself, and no removal of the standalone tool — this is additive. + +## Decisions + +**D1 — Gate on caller kind + explicit `to` + non-self, mirroring `hera_revive`'s own authority check.** `hera_revive` rejects non-coordinator callers outright (`caller.role.Kind != db.HeraKindCoordinator`). Auto-revive-on-send reuses the same authority boundary rather than inventing a softer one: only `caller.role.Kind == db.HeraKindCoordinator` triggers an attempt, and only when the recipient was resolved via an explicit `to` (the coordinator explicitly named a role it coordinates) — never the worker/freelance default-route path, which resolves a coordinator, not a role the sender coordinates. `toRole.ID != caller.role.ID` guards the degenerate self-target case, matching `hera_revive`'s explicit own-role rejection (here expressed as a skip rather than an error, since `hera_send` self-sends are already invalid on other grounds and should fail with the existing self-send error, not a revive-related one). + +**D2 — Placement: after recipient resolution, before `s.heraSvc.Send`, never blocking the send.** The attempt sits strictly between resolving `toRole` and calling `Send`. Every exit from the attempt — no live binding, a lookup error, a revive error, `s.heraRevive == nil` — falls through to the unconditional `Send` call. This is a deliberate asymmetry from `hera_revive` (a standalone tool where a revive failure IS the whole point and must surface as an error): here, revive is a courtesy side-effect of send, and send's own success/failure semantics must stay exactly as they are today. + +**D3 — Reuse `heraReviveOutcomeMessage` and the same log line verbatim; no new outcome vocabulary.** The `hera_send` response's `- **revive**: ` line and the `slog.Info("[hera] revive", ...)` call use the identical rendering/logging helpers `toolHeraRevive` already has, so an auto-triggered revive is indistinguishable in logs and in outcome vocabulary from a manual one. This also means no new documentation of outcome semantics is needed beyond a cross-reference to the existing `hera_revive` requirement. + +**D4 — Error handling granularity: `ErrHeraNotFound` is the expected/common case (Info/Debug at most), any other lookup or revive error is a Warn.** A recipient with no live binding (planned node, never spawned, ended) is not a bug — it's the everyday case of messaging a role that hasn't materialized yet or has wound down. Logging it above Debug/Info would spam the daemon log on every ordinary send to such a role. A different lookup error, or a `heraRevive` call error, is unexpected and worth a Warn — mirroring how `toolHeraSend`'s own status-apply soft-fail (D1 in make-hera-plan-living) already logs a Warn on failure while proceeding. + +## Risks / Trade-offs + +- **[Risk] A coordinator sending many messages to the same already-fine recipient pays a `HeraLiveBindingByRole` lookup (and, when there IS a live binding, a full `ReviveRole` gate evaluation — `IsAlive`/`IsIdle`/`BlockedOnPrompt`/`HasPendingRestart`) on every single send.** → Mitigation: these are the same checks `hera_revive` already performs synchronously per call, and `hera_send` already does comparable per-call DB work (role resolution, rate-limit check, inbox-cap check). No new I/O class is introduced; this is not expected to be a meaningful cost at hera's message volumes. +- **[Risk] A coordinator that intentionally messages a role it does NOT want woken (e.g. deliberately leaving a paused role parked) now wakes it as a side effect of sending.** → Mitigation: this is the intended behavior per the mission — the whole point is removing the need for a separate `hera_revive` call before messaging a sleeping child. `ReviveRole`'s own gating already protects the cases that matter (a live coordinator, a busy role, a role blocked on a prompt are all left untouched) — the only case actually revived-by-side-effect is a role that was dead or genuinely stuck, which is exactly the case a coordinator sending it a message wants alive anyway. +- **[Risk] Silent skip on `ErrHeraNotFound` could mask a genuine naming/resolution bug (e.g. a role that SHOULD have a live binding but doesn't due to a bug elsewhere).** → Mitigation: this is unchanged risk surface — `hera_send` already tolerates sending to a role with no live binding today (the message is durably stored regardless; only best-effort doorbell delivery is affected), so auto-revive attempting and silently skipping adds no new failure mode beyond what already exists. + +## Open Questions + +None — the mission brief fixes the design; see the brief's explicit numbered "Behavior to add" list for the exact call sequence. diff --git a/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/proposal.md b/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/proposal.md new file mode 100644 index 00000000..ca790160 --- /dev/null +++ b/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/proposal.md @@ -0,0 +1,27 @@ +## Why + +A coordinator that notices a bound role has gone quiet must currently make two calls to reach it: `hera_revive` to wake the session, then `hera_send` to actually deliver the message. The two-step dance is easy to forget — a coordinator sends first, gets no response, and only later thinks to check whether the recipient's session is even alive. `hera_revive` already encodes the exact safety gate needed (dead → restart, stuck-but-idle → kick, busy/blocked/live-coordinator → leave alone) via the shared `internal/hera.ReviveRole` primitive. `hera_send` should just call it automatically so a coordinator never has to remember the separate step. + +## What Changes + +- `hera_send` (`internal/mcp/hera.go`, `toolHeraSend`), when the caller is a coordinator sending to an explicitly named `to` recipient (not the worker/freelance default-to-coordinator path, and not itself), now attempts a revive of that recipient BEFORE delivering the message — reusing `s.heraRevive`/`internal/hera.ReviveRole` verbatim, the exact same primitive `hera_revive` already calls. No new gating logic is introduced. +- The attempt is soft-fail and best-effort: a recipient with no live binding (a planned node, never spawned, or ended role), a lookup error, a revive error, or `heraRevive` not being wired (daemon didn't configure a reviver) all skip the auto-revive step silently (Info/Debug log at most) and the message send proceeds regardless. +- On a successful revive attempt, the `hera_send` tool response gains a `- **revive**: ` line (rendered via the existing `heraReviveOutcomeMessage`) alongside the existing `message_id`/`to`/`delivery_mode` lines, so the coordinator learns in one round-trip whether the recipient was dead, stuck, or already fine. The line is omitted entirely when no revive attempt was made. +- A `slog.Info("[hera] revive", ...)` line is emitted matching `toolHeraRevive`'s existing one, so an auto-triggered revive is indistinguishable in logs from a manual `hera_revive` call. + +## Capabilities + +### New Capabilities + +(none) + +### Modified Capabilities + +- `hera-messaging`: the "hera_send recipient resolution and defaults" requirement's sibling gains a new requirement, "hera_send auto-revives a dead or stuck recipient," describing the coordinator-only, explicit-`to`-only auto-revive attempt and its soft-fail semantics. + +## Impact + +- `internal/mcp/hera.go` (`toolHeraSend`: auto-revive attempt wired between recipient resolution and `s.heraSvc.Send`) +- `internal/mcp/hera_test.go` (new test cases) +- `context/knowledge/gotchas/messaging.md`, `context/knowledge/index.md` +- No schema/data migration, no new dependencies, no new MCP tool, no REST/API surface change, no TUI behavior change. Reuses `internal/hera.ReviveRole` and `heraReviveOutcomeMessage` exactly as `hera_revive` already does — no changes to `internal/hera/revive.go`. diff --git a/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/specs/hera-messaging/spec.md b/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/specs/hera-messaging/spec.md new file mode 100644 index 00000000..97dc09e4 --- /dev/null +++ b/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/specs/hera-messaging/spec.md @@ -0,0 +1,44 @@ +## ADDED Requirements + +### Requirement: hera_send auto-revives a dead or stuck recipient + +The system SHALL, on `hera_send`, attempt to revive the recipient's live session before delivering the message, using the exact same `internal/hera.ReviveRole` primitive and outcome set as `hera_revive` (see `openspec/specs/hera-coordination/spec.md`'s "hera_revive coordinator PULL-revive of a bound role" requirement) — no new gating logic is introduced. The attempt SHALL be made ONLY when all of: the caller's role kind is `coordinator`; the recipient was resolved via an explicit `to` argument (not the worker/freelance default-to-coordinator route); and the recipient's role id differs from the caller's own role id. + +The attempt SHALL be soft-fail: it SHALL NOT block, delay meaningfully, or fail the message send under any outcome, including a recipient with no live binding, a binding-lookup error other than not-found, a revive error, or `hera_revive`'s underlying reviver not being configured. A recipient with no live binding is an expected, common case (a planned node, a role never spawned, or an ended role) and SHALL be skipped with at most an Info/Debug log; any other lookup or revive error SHALL be skipped with a Warn log. + +On a successful revive attempt, the `hera_send` tool response SHALL include a `- **revive**: ` line rendered via the same outcome-to-message renderer `hera_revive` uses, in addition to the existing `message_id`/`to`/`delivery_mode` lines. The line SHALL be omitted when no revive attempt was made. A successful revive attempt SHALL be logged via the same `slog.Info("[hera] revive", ...)` line `hera_revive` emits, so an auto-triggered revive is indistinguishable in logs from a manual `hera_revive` call. + +#### Scenario: Dead recipient is restarted before the message is delivered + +- **WHEN** a coordinator calls `hera_send` with an explicit `to` naming a different role whose session has no live process +- **THEN** the recipient's session is restarted in place before the send, the tool response includes `- **revive**: ` describing the restart, and the message is still delivered + +#### Scenario: Busy, blocked, or live-coordinator recipient is left untouched and the send still succeeds + +- **WHEN** a coordinator calls `hera_send` with an explicit `to` naming a role that is alive and actively working, alive and blocked on a prompt, or itself a live coordinator +- **THEN** no restart or kick is attempted, the tool response reports the corresponding skip outcome, and the message is delivered exactly as it would be without the auto-revive attempt + +#### Scenario: Recipient with no live binding does not block the send + +- **WHEN** a coordinator calls `hera_send` with an explicit `to` naming a role that has never been spawned, is only a planned node, or has ended +- **THEN** the auto-revive attempt is skipped silently (no more than an Info/Debug log), no `- **revive**:` line appears in the response, and the message is still stored and delivery is still attempted exactly as `hera_send` already behaves today + +#### Scenario: A revive lookup or call error does not block the send + +- **WHEN** the recipient's live-binding lookup fails with an error other than not-found, or the revive call itself returns an error +- **THEN** a Warn is logged, no `- **revive**:` line appears in the response, and the message send proceeds and succeeds or fails purely on its own existing merits + +#### Scenario: Reviver not configured does not block the send + +- **WHEN** the daemon has not wired a `hera_revive` reviver (`s.heraRevive` is nil) +- **THEN** the auto-revive attempt is skipped entirely with no error, no `- **revive**:` line appears in the response, and `hera_send` behaves exactly as it did before this change + +#### Scenario: Worker/freelance default-route sends never trigger auto-revive + +- **WHEN** a worker or freelance sender calls `hera_send` with no explicit `to` (defaulting to the orchestrator's active coordinator) +- **THEN** no auto-revive attempt is made regardless of the coordinator's session state, and the send proceeds exactly as it did before this change + +#### Scenario: A coordinator's self-send never triggers auto-revive + +- **WHEN** a coordinator calls `hera_send` with an explicit `to` that resolves to its own calling role +- **THEN** no auto-revive attempt is made, and the call fails with the existing self-send validation error exactly as it did before this change diff --git a/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/tasks.md b/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/tasks.md new file mode 100644 index 00000000..42d1fb1a --- /dev/null +++ b/openspec/changes/archive/2026-08-03-add-hera-send-auto-revive/tasks.md @@ -0,0 +1,33 @@ +**Design doc:** `openspec/changes/add-hera-send-auto-revive/design.md` + +## 1. Tests (write failing first) + +- [x] 1.1 `internal/mcp/hera_test.go`: dead explicit-`to` recipient is restarted before send succeeds (assert `fakeHeraReviver` called with the recipient's task id + kind, and the response contains `- **revive**:` plus `restarted_dead`). +- [x] 1.2 Busy/blocked/live-coordinator recipient: revive attempt made (reviver called), send still succeeds, response reports the skip outcome. +- [x] 1.3 Recipient with no live binding (planned role, `db.ErrHeraNotFound` from `HeraLiveBindingByRole`): reviver NOT called, no `- **revive**:` line, send still succeeds. +- [x] 1.4 `s.heraRevive == nil` (reviver not wired): no error, no `- **revive**:` line, send still succeeds exactly as today. +- [x] 1.5 Worker/freelance default-route send (no explicit `to`): reviver NOT called even when the target coordinator has a live binding. +- [x] 1.6 Coordinator self-send (`to` resolves to caller's own role): reviver NOT called; existing self-send error path is unchanged. +- [x] 1.7 A revive call error (fakeHeraReviver returns an error): send still succeeds, no `- **revive**:` line. +- [x] 1.8 Confirm every scenario in `specs/hera-messaging/spec.md`'s new requirement has a corresponding failing test before moving to implementation. + +## 2. Implement auto-revive in `toolHeraSend` + +**Depends on:** Stage 1 + +- [x] 2.1 `internal/mcp/hera.go`, `toolHeraSend`: after recipient resolution, before `s.heraSvc.Send`, add the gated auto-revive attempt per design.md D1/D2 — `caller.role.Kind == db.HeraKindCoordinator && p.To != "" && toRole.ID != caller.role.ID`. +- [x] 2.2 Resolve `s.heraStore.HeraLiveBindingByRole(toRole.ID)`; fold `db.ErrHeraNotFound` into a silent/Info-level skip; any other error → Warn log and skip. +- [x] 2.3 When a binding is found and `s.heraRevive != nil`, call `s.heraRevive(HeraReviveInput{TaskID: binding.ArgusTaskID, IsCoordinator: toRole.Kind == db.HeraKindCoordinator})` exactly as `toolHeraRevive` does; on error, Warn log and proceed to send. +- [x] 2.4 On a successful revive attempt, capture the outcome string for the response and emit `slog.Info("[hera] revive", ...)` matching `toolHeraRevive`'s existing fields. +- [x] 2.5 In the response builder, append `- **revive**: ` immediately when a revive attempt succeeded; omit entirely otherwise. +- [x] 2.6 Run `go test ./internal/mcp/...`; confirm Stage 1 passes. + +## 3. Verify and land + +**Depends on:** Stage 2 + +- [x] 3.1 Run `make pre-pr`; fix any failures (build, vet, fmt-check, lint-pr, vuln, test-cover-gate). +- [x] 3.2 `context/knowledge/gotchas/messaging.md`: add a bullet noting hera_send's auto-revive-on-send reuses `hera.ReviveRole` verbatim with no new gating logic, and is soft-fail so a revive failure never blocks message delivery. +- [x] 3.3 `context/knowledge/index.md`: update the `gotchas/messaging.md` coverage-bullet cell to reflect the new bullet. +- [x] 3.4 `openspec archive add-hera-send-auto-revive` (or the manual merge-and-move fallback): merge the `hera-messaging` delta spec into `openspec/specs/hera-messaging/spec.md`, move the change folder to `openspec/changes/archive/-add-hera-send-auto-revive/`, commit on the same branch before merge. +- [x] 3.5 Open the PR via `mcp__argus__iris_gh_pr_create`, base `master`. diff --git a/openspec/specs/hera-messaging/spec.md b/openspec/specs/hera-messaging/spec.md index 9c4dd1aa..1766829b 100644 --- a/openspec/specs/hera-messaging/spec.md +++ b/openspec/specs/hera-messaging/spec.md @@ -5,9 +5,7 @@ Hera Messaging is the role-addressed message bus beneath the Hera View (comparison area 9: messaging / doorbell surfacing). It persists messages between hera roles durably in SQLite and delivers them to a recipient's live agent pane via the SAME reliable, idle-gated notifier the `task_messages` path uses — no second delivery engine is introduced. This mirrors the plugin's `hera-delivery-receipt` capability. This is a faithful capture of current native behavior. Each requirement cites `file:line`; native-vs-plugin differences carry a `NOTE:`. - ## Requirements - ### Requirement: Role-addressed durable message store with caps The system SHALL persist hera messages addressed role→role, carrying a body, a one-line `tldr` subject, an optional `in_reply_to`, send timestamp, delivery stamp, and read state. It SHALL enforce three caps: body ≤ 64 KiB, recipient unread ≤ 500, and sender rolling 60-second rate ≤ 50 sends/min. It SHALL reject a self-send, a missing or too-long tldr, and a recipient role that is missing or archived. Storage is always durable regardless of delivery outcome. @@ -108,3 +106,47 @@ Derived from: `internal/hera/service.go:62` (security NOTE in `Send` doc). - **WHEN** a hera doorbell is composed - **THEN** it may embed the sender role name and tldr, unlike the task_messages nudge line which must not + +### Requirement: hera_send auto-revives a dead or stuck recipient + +The system SHALL, on `hera_send`, attempt to revive the recipient's live session before delivering the message, using the exact same `internal/hera.ReviveRole` primitive and outcome set as `hera_revive` (see `openspec/specs/hera-coordination/spec.md`'s "hera_revive coordinator PULL-revive of a bound role" requirement) — no new gating logic is introduced. The attempt SHALL be made ONLY when all of: the caller's role kind is `coordinator`; the recipient was resolved via an explicit `to` argument (not the worker/freelance default-to-coordinator route); and the recipient's role id differs from the caller's own role id. + +The attempt SHALL be soft-fail: it SHALL NOT block, delay meaningfully, or fail the message send under any outcome, including a recipient with no live binding, a binding-lookup error other than not-found, a revive error, or `hera_revive`'s underlying reviver not being configured. A recipient with no live binding is an expected, common case (a planned node, a role never spawned, or an ended role) and SHALL be skipped with at most an Info/Debug log; any other lookup or revive error SHALL be skipped with a Warn log. + +On a successful revive attempt, the `hera_send` tool response SHALL include a `- **revive**: ` line rendered via the same outcome-to-message renderer `hera_revive` uses, in addition to the existing `message_id`/`to`/`delivery_mode` lines. The line SHALL be omitted when no revive attempt was made. A successful revive attempt SHALL be logged via the same `slog.Info("[hera] revive", ...)` line `hera_revive` emits, so an auto-triggered revive is indistinguishable in logs from a manual `hera_revive` call. + +#### Scenario: Dead recipient is restarted before the message is delivered + +- **WHEN** a coordinator calls `hera_send` with an explicit `to` naming a different role whose session has no live process +- **THEN** the recipient's session is restarted in place before the send, the tool response includes `- **revive**: ` describing the restart, and the message is still delivered + +#### Scenario: Busy, blocked, or live-coordinator recipient is left untouched and the send still succeeds + +- **WHEN** a coordinator calls `hera_send` with an explicit `to` naming a role that is alive and actively working, alive and blocked on a prompt, or itself a live coordinator +- **THEN** no restart or kick is attempted, the tool response reports the corresponding skip outcome, and the message is delivered exactly as it would be without the auto-revive attempt + +#### Scenario: Recipient with no live binding does not block the send + +- **WHEN** a coordinator calls `hera_send` with an explicit `to` naming a role that has never been spawned, is only a planned node, or has ended +- **THEN** the auto-revive attempt is skipped silently (no more than an Info/Debug log), no `- **revive**:` line appears in the response, and the message is still stored and delivery is still attempted exactly as `hera_send` already behaves today + +#### Scenario: A revive lookup or call error does not block the send + +- **WHEN** the recipient's live-binding lookup fails with an error other than not-found, or the revive call itself returns an error +- **THEN** a Warn is logged, no `- **revive**:` line appears in the response, and the message send proceeds and succeeds or fails purely on its own existing merits + +#### Scenario: Reviver not configured does not block the send + +- **WHEN** the daemon has not wired a `hera_revive` reviver (`s.heraRevive` is nil) +- **THEN** the auto-revive attempt is skipped entirely with no error, no `- **revive**:` line appears in the response, and `hera_send` behaves exactly as it did before this change + +#### Scenario: Worker/freelance default-route sends never trigger auto-revive + +- **WHEN** a worker or freelance sender calls `hera_send` with no explicit `to` (defaulting to the orchestrator's active coordinator) +- **THEN** no auto-revive attempt is made regardless of the coordinator's session state, and the send proceeds exactly as it did before this change + +#### Scenario: A coordinator's self-send never triggers auto-revive + +- **WHEN** a coordinator calls `hera_send` with an explicit `to` that resolves to its own calling role +- **THEN** no auto-revive attempt is made, and the call fails with the existing self-send validation error exactly as it did before this change +