From 9dc9db20956c1252b12c2ccf01fa57e83987872e Mon Sep 17 00:00:00 2001 From: Aaron Newton Date: Thu, 30 Jul 2026 00:54:34 -0700 Subject: [PATCH] Fix hera plan-DAG hygiene: orphaned planned nodes + silent materialize retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug A: heragater polled planned nodes forever after their orchestrator was archived/nuked (ListHeraPlannedNodes never checked the parent). Fixed with a belt-and-braces pair: ArchiveHeraOrchestrator/NukeHeraOrchestrator now cascade-cancel still-planned child roles, and ListHeraPlannedNodes also joins hera_orchestrators to exclude nodes under an ended orchestrator regardless of cause — retroactive with no data migration. Bug B: materializeNode retried a permanently-broken planned node (e.g. blank argus_project) in total silence forever. Fixed with a bounded consecutive-failure counter that escalates to the coordinator once, mirroring holdAndPing's ping-once dedup shape. Never auto-cancels or reconfigures the node — advisory only. Bug C (freelance roles stuck in the rail's flat Freelance section) is confirmed data-hygiene, not a display defect — no code change; see design.md. OpenSpec change fix-hera-plan-dag-hygiene archived in this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- context/knowledge/gotchas/orchestration.md | 8 ++ context/knowledge/index.md | 2 +- internal/db/hera.go | 50 +++++++- internal/db/hera_plan.go | 9 ++ internal/db/hera_plan_test.go | 55 +++++++++ internal/db/hera_test.go | 63 ++++++++++ internal/heragater/heragater.go | 110 +++++++++++++++++- internal/heragater/heragater_test.go | 108 +++++++++++++++++ .../.openspec.yaml | 2 + .../design.md | 78 +++++++++++++ .../proposal.md | 29 +++++ .../specs/task-orchestration/spec.md | 54 +++++++++ .../tasks.md | 35 ++++++ openspec/specs/task-orchestration/spec.md | 53 +++++++++ 14 files changed, 645 insertions(+), 11 deletions(-) create mode 100644 openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/.openspec.yaml create mode 100644 openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/design.md create mode 100644 openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/proposal.md create mode 100644 openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/specs/task-orchestration/spec.md create mode 100644 openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/tasks.md diff --git a/context/knowledge/gotchas/orchestration.md b/context/knowledge/gotchas/orchestration.md index d791e660..efa4de68 100644 --- a/context/knowledge/gotchas/orchestration.md +++ b/context/knowledge/gotchas/orchestration.md @@ -104,6 +104,14 @@ Hera orchestration + task-creation invariants that caused bugs when violated. - **The seed prompt requires zero follow-up tool calls from the recycled session by construction, not by convention.** `BuildRecycleSeedPrompt` concatenates the role's stored mission prompt + a plan-DAG section (sibling role names/statuses) + the `handoff_note` meta value (if any) into one static string assembled server-side, handed to the new session as its literal opening prompt (`task.Prompt`). Only the role lookup is fatal; a missing plan-DAG or absent `handoff_note` degrades to an empty section rather than blocking the recycle. - **The `B` rail key now bounces a worker/freelance selection instead of no-op'ing (add-worker-bounce); only an empty selection stays a true no-op.** `page.go`'s `case 'B'` switches on `sel.IsCoordinator()` (unchanged: confirm modal → `RecycleCoord(..., RecycleHumanForced)` directly, restarting mid-turn without waiting — the intended behavior for a session the self-service path can't reach because it's wedged) vs `sel.IsWorkerOrFreelance()` (new: confirm modal → `heraDoBounceWorker` → `sess.WriteInputSystem` sends a plain instruction asking the role to call `hera_status(handoff_note=..., request_recycle=true)` ITSELF — no direct kill/restart, no new `RecycleTrigger`; the now-widened self-service pipeline above completes the recycle once the role goes idle and makes that call). Nothing polls for a response and nothing times out if the role never calls it (D6, explicit scope decision) — the human can just press `B` again. +## Hera plan-DAG hygiene (add-hera-plan-hygiene) + +Three independently-verified data-hygiene bugs, found live in `~/.argus/data.sql` — deliberately NOT unified under one root cause; two got code fixes, one did not. + +- **A planned node outliving its archived/nuked orchestrator needs BOTH a write-time cascade AND a read-time filter — neither alone is sufficient.** `ArchiveHeraOrchestrator`/`NukeHeraOrchestrator` (`internal/db/hera.go`) cascade-cancel (`cancelled_at`) their still-planned (never-materialized) worker-kind child roles via `cancelStillPlannedChildRoles`, so a FUTURE archive/nuke never orphans a pollable dead node. But that alone does not repair rows that predate the fix (a real live bug: roles 343/358 retried `heragater` materialization every ~60s tick for over a month behind orchestrators archived+nuked a month earlier, spamming `hold: no coordinator to ping: `). `ListHeraPlannedNodes` (`internal/db/hera_plan.go`) therefore ALSO joins `hera_orchestrators` and requires `archived_at IS NULL AND nuked_at IS NULL` on the parent, independent of the node's own `cancelled_at` — this is what makes the fix retroactive with zero data migration. Both exist for the same reason the nuked-orchestrator check is duplicated in `internal/tui/hera/model.go`'s `BuildModel` even though the DB list query already excludes nuked rows: list queries are this codebase's belt-and-braces layer, not just the write path. +- **`heragater.materializeNode`'s "stays planned, retry next tick" failure path had no escalation — a permanently-broken node (e.g. blank `argus_project`) retried in total silence forever.** Fixed by a bounded consecutive-failure counter (`Watcher.materializeFailures`/`escalatedMaterializeFailures`, `materializeFailureEscalationTicks=5`) that sends ONE coordinator notice on crossing the threshold, mirroring `holdAndPing`'s ping-once-per-condition dedup shape (NOT `agent.EscalateParkedSelection`'s 8-tick threshold — that guards isolated torn reads on a sub-second polling loop, a noise source that doesn't exist for a synchronous, deterministic materialize call). Escalation is advisory ONLY: it never auto-cancels or reconfigures the node — a node with zero remaining blockers is presumptively genuine pending work, and only a human can say what's actually wrong with it. Both maps are swept every `Tick()` for any node id no longer in the planned set (`sweepMaterializeFailures`, mirroring `rearmHeldPings`'s cleanup of `heldPings`) — an unbounded per-node-forever map would itself be exactly the class of hygiene defect this fix exists to close. +- **A `kind=freelance` role stuck in the Hera rail's flat top-level Freelance section with an intact role→orchestrator→binding→task chain is a data-hygiene gap, not a display bug.** `internal/tui/hera/model.go`'s rule (`role.Kind == HeraKindFreelance && role.ArchivedAt == nil && o.ArchivedAt == nil` → flat section) is intentional and working as designed; a `kind=worker` role nests under its orchestrator unconditionally regardless of archived state, but a freelance role escapes the flat section only by being archived. Two live rows (roles 813/814) simply were never archived once their work finished (tasks already `in_review`/`complete`) — the identical "task finished, binding still open" shape is the NORMAL, common case for ~150 other historical `kind=worker` roles that render fine. No `Clean`/`Prune`/`Sweep`-named function touching hera data exists anywhere in this codebase; whatever produced this pair's un-archived state was very likely a manual/ad hoc action outside argus. Resolution is a one-time data cleanup (`ArchiveHeraRole` + `EndHeraBinding` on the two rows), NOT a code change — auto-archiving a freelance role on binding-end would be a genuine, separately-scoped behavior change with its own design surface (when exactly does "binding ended" count?), deliberately not smuggled into a three-bug hygiene fix. + ## Hera subtree TLDR roll-up (M5) - **The subtree (TLDR roll-up) and the orchestration tree share the SAME nesting graph now.** `SubtreeOrchIDs` walks the orchestrator *nesting* graph (multi-binding bridges) for message roll-up; `heraTreeNodes` (the Details-pane graph) walks the SAME nesting in-memory for rendering. Both derive from role bindings — there is no longer a separate `depends_on` dependency graph to conflate them with (it was retired). A message roll-up is still a distinct concern from the rendered tree, but they no longer use orthogonal data sources. diff --git a/context/knowledge/index.md b/context/knowledge/index.md index a7dac478..7ebe13cd 100644 --- a/context/knowledge/index.md +++ b/context/knowledge/index.md @@ -14,7 +14,7 @@ Non-obvious invariants and gotchas, split by topic. Read the relevant file when | [gotchas/tasklist-ui.md](gotchas/tasklist-ui.md) | Task list cursor, modals, focus guards, filter, archive, pinned, spinner, row rendering, form done-flag reset, needs-input icon, agent attention bar, sticky needs-input across idle gate, ❯+NBSP prompt anchor, decoration-line skip, PR indicator cell, filter-picker narrow-terminal width clamp (shared taskswitcher/sessionpicker/fuzzylinkpicker), shared TaskStatusIcon classifier (task list + switcher), new-task optional name field (ntFieldName Tab-only, EnteredName sanitize+user-chosen signal, onDone 2nd param=name, heraSpawnName override) | 56 | | [gotchas/web-remote.md](gotchas/web-remote.md) | SPA + REST API + service worker, Tailscale-only binding, EventSource auth, xterm.js, HTML escaping, virtual keys, detail/files views, prompt modal, input history, link picker, new-task defaults, file uploads, offline view, Web Push, per-device tokens, settings endpoints, share target + iOS Shortcut, test harness, idle status, daemon-side status flip on exit, static-output replay, mobile compose + key bar, skill autocomplete, task-list search, diff wrap + stacked panels, full-history scrollback, global hotkeys, session artifacts (manifest-scoped serving, X-Frame-Options, blob+sandbox isolation), PR badge from cached pr_state, compose split text→CR send, artifact Open overlay vs paneled, Hera tab (read-only /api/hera roster), Settings System panel (sysmetrics cached snapshot, visible-only poller, *_avail "—" fallback, collector started in ListenAndServe), list-view bottom padding to clear iOS bottom bar, Ctrl+Z swallowed at xterm key layer (session-orphan prevention) | 193 | | [gotchas/misc.md](gotchas/misc.md) | DB patterns, Go idioms, Codex, Pi/ollama prelaunch, MCP, PRs, file explorer, quick-add, scheduled tasks, links, task auto-naming, settings two-pane UI, plugin-scoped token tagging, task_meta sidecar, plugin settings registry, Claude session discovery (folder encoding, JSONL parse), config.toml override layer (precedence, partial-map zeroing, mtime live-reload), hera plugin contract (POST/DELETE /notify), model selection (--model injection, codex-resume re-append), per-schedule model override (TaskCreator taskModel → HeadlessInput.Model), wall-clock duration clamp, link PR indicator (IsPR single-source + prLinkGutter), openspec archive workflow (rc0-on-abort, authoring-order fold, orphaned-superseded-requirement audit, authoring-drift fixes, doubled-date-prefix on pre-dated change folders), diligence profiles (resolution fail-open, in-repo precedence, daemon-side resolution vs sandbox EPERM, [panel] deferred seam, 3 schema columns, embedded seed install via InstallDefaults, archetype/rigor JSON casing fix + native-dispatch convention pointer), opencode backend (capture-style/no --session-id, --session resume, root-commit-keyed capture filtered by directory, SQLite-first+JSON-fallback fail-open, MCP type:"remote", ctrl+r switcher excludes it), builtin skills/routing spawn-time injection (unconditional --add-dir + --append-system-prompt-file, self-gating content, isTestBinary() short-circuit, ensureBuiltinRoutingFn test seam, manual install-claude-skills.sh mechanism retired, skills embed-drift vs .claude/skills unfixed, generic directory-tree iteration with no per-name whitelist, hera-review/hera-review-test-adversary parity shipped + hera-spawn-review/resolve-archetype-model parity shipped once profile_resolve/internal-review infra landed), landing risk of stacking work on a not-yet-merged integration branch (hera-nav-palette took 3 attempts across #865/#882/#886 — worktree/branch cleanup before merge stranded stacked branches twice despite the code itself being correct), `argus doctor` diligence-profile-library check cross-ref (add-doctor-profile-check: see gotchas/daemon-rpc.md) | 194 | -| [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) | 75 | +| [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) | 169 | | [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 | diff --git a/internal/db/hera.go b/internal/db/hera.go index 4f6be411..2fad633b 100644 --- a/internal/db/hera.go +++ b/internal/db/hera.go @@ -360,9 +360,21 @@ func (d *DB) ListHeraOrchestrators(includeArchived bool) ([]*HeraOrchestrator, e // ArchiveHeraOrchestrator stamps archived_at (current time) and CLEARS // pinned_at — pin and archive are mutually exclusive. Idempotent: re-archiving // preserves the original archived_at. Returns ErrHeraNotFound if no row matches. +// +// Cascade-cancels the orchestrator's still-planned (never-materialized) child +// roles (add-hera-plan-hygiene Bug A) — a planned node whose parent just ended +// would otherwise retry materialization forever with no coordinator to ping. +// The cascade is best-effort: a failure is logged, never returned, so archive's +// existing error contract (nil / ErrHeraNotFound) is unchanged. func (d *DB) ArchiveHeraOrchestrator(id int64) error { - return d.heraSetFlag(`UPDATE hera_orchestrators SET archived_at=?, pinned_at=NULL WHERE id=? AND archived_at IS NULL`, - heraOrchExistsProbe, id, formatTime(time.Now())) + if err := d.heraSetFlag(`UPDATE hera_orchestrators SET archived_at=?, pinned_at=NULL WHERE id=? AND archived_at IS NULL`, + heraOrchExistsProbe, id, formatTime(time.Now())); err != nil { + return err + } + if err := d.cancelStillPlannedChildRoles(id); err != nil { + slog.Warn("ArchiveHeraOrchestrator: cascade-cancel planned children failed", "orchestrator_id", id, "err", err) + } + return nil } // UnarchiveHeraOrchestrator clears archived_at. Idempotent. Returns @@ -395,11 +407,41 @@ func (d *DB) UnpinHeraOrchestrator(id int64) error { // rail-feeding lists (ListHeraOrchestrators) but its row is retained and still // returned by id (HeraOrchestrator) for DB-only recovery. NEVER a hard delete. // Returns ErrHeraNotFound if no row matches id. +// +// Cascade-cancels still-planned child roles exactly like ArchiveHeraOrchestrator +// (add-hera-plan-hygiene Bug A) — same best-effort, log-only-on-failure contract. func (d *DB) NukeHeraOrchestrator(id int64) error { now := formatTime(time.Now()) - return d.heraSetFlag( + if err := d.heraSetFlag( `UPDATE hera_orchestrators SET nuked_at=COALESCE(nuked_at, ?), archived_at=COALESCE(archived_at, ?), pinned_at=NULL WHERE id=?`, - heraOrchExistsProbe, id, now, now) + heraOrchExistsProbe, id, now, now); err != nil { + return err + } + if err := d.cancelStillPlannedChildRoles(id); err != nil { + slog.Warn("NukeHeraOrchestrator: cascade-cancel planned children failed", "orchestrator_id", id, "err", err) + } + return nil +} + +// cancelStillPlannedChildRoles stamps cancelled_at on every still-planned +// (never-materialized) worker-kind child role of an orchestrator — mirrors +// ListHeraPlannedNodes's own definition of a planned node exactly (kind=worker, +// not archived, not cancelled, no binding ever) so the cascade only ever +// touches rows that query would otherwise keep surfacing forever. Idempotent +// (COALESCE) and scoped to one orchestrator; a materialized (bound) child is +// never touched. +func (d *DB) cancelStillPlannedChildRoles(orchID int64) error { + d.mu.Lock() + defer d.mu.Unlock() + _, err := d.conn.Exec( + `UPDATE hera_roles SET cancelled_at=COALESCE(cancelled_at, ?) + WHERE orchestrator_id=? AND kind=? AND archived_at IS NULL AND cancelled_at IS NULL + AND NOT EXISTS (SELECT 1 FROM hera_bindings b WHERE b.role_id = hera_roles.id)`, + formatTime(time.Now()), orchID, string(HeraKindWorker)) + if err != nil { + return fmt.Errorf("cancel still-planned child roles: %w", err) + } + return nil } // SetHeraOrchestratorKanbanStatus sets the orchestrator's independent kanban diff --git a/internal/db/hera_plan.go b/internal/db/hera_plan.go index a96c3127..cb1c56f1 100644 --- a/internal/db/hera_plan.go +++ b/internal/db/hera_plan.go @@ -356,6 +356,13 @@ type HeraPlannedNode struct { // materialized (it has a binding row, live or ended) is NOT a planned node, even // if its binding has since ended; the gater never re-materializes. Archived // roles are excluded. +// +// The parent orchestrator's own archived_at/nuked_at is ALSO checked (JOIN, +// not just the node's own columns) — a defensive filter independent of +// ArchiveHeraOrchestrator/NukeHeraOrchestrator's cascade-cancel of still-planned +// children. The cascade prevents the state going forward; this filter tolerates +// it regardless of cause, including rows that predate the cascade existing at +// all, with no data migration required (add-hera-plan-hygiene Bug A). func (d *DB) ListHeraPlannedNodes() ([]*HeraRole, error) { d.mu.Lock() defer d.mu.Unlock() @@ -363,7 +370,9 @@ func (d *DB) ListHeraPlannedNodes() ([]*HeraRole, error) { `SELECT r.id, r.orchestrator_id, r.name, r.kind, r.argus_project, r.prompt, r.created_at, r.archived_at, r.pinned_at, r.nuked_at, r.node_kind, r.cancelled_at, r.archetype FROM hera_roles r + JOIN hera_orchestrators o ON o.id = r.orchestrator_id WHERE r.kind=? AND r.archived_at IS NULL AND r.cancelled_at IS NULL + AND o.archived_at IS NULL AND o.nuked_at IS NULL AND NOT EXISTS (SELECT 1 FROM hera_bindings b WHERE b.role_id = r.id) ORDER BY r.id ASC`, string(HeraKindWorker)) diff --git a/internal/db/hera_plan_test.go b/internal/db/hera_plan_test.go index f7841a24..b24db779 100644 --- a/internal/db/hera_plan_test.go +++ b/internal/db/hera_plan_test.go @@ -109,6 +109,61 @@ func TestListHeraPlannedNodes_StaysPlannedAfterBindingEnds(t *testing.T) { _ = role } +func TestListHeraPlannedNodes_ExcludesArchivedOrNukedOrchestrator(t *testing.T) { + // add-hera-plan-hygiene Bug A: a planned node whose PARENT orchestrator has + // ended must not keep surfacing forever, even if the node's own + // cancelled_at was never stamped (the defensive read-path filter must work + // independent of ArchiveHeraOrchestrator/NukeHeraOrchestrator's cascade — + // this is what makes the fix retroactive for rows that predate it). + t.Run("archived orchestrator", func(t *testing.T) { + d := testDB(t) + orch := planTestOrch(t, d, "archived-orch") + node := plannedRole(t, d, orch, "node") + + // Stamp archived_at directly, bypassing ArchiveHeraOrchestrator's own + // cascade, to prove the list query itself excludes the node. + _, err := d.conn.Exec(`UPDATE hera_orchestrators SET archived_at=? WHERE id=?`, "2026-06-20T00:00:00Z", orch) + testutil.NoError(t, err) + + got, err := d.ListHeraPlannedNodes() + testutil.NoError(t, err) + for _, r := range got { + if r.ID == node.ID { + t.Fatalf("planned node %d under an archived orchestrator must not be listed", node.ID) + } + } + }) + + t.Run("nuked orchestrator", func(t *testing.T) { + d := testDB(t) + orch := planTestOrch(t, d, "nuked-orch") + node := plannedRole(t, d, orch, "node") + + _, err := d.conn.Exec(`UPDATE hera_orchestrators SET nuked_at=?, archived_at=? WHERE id=?`, + "2026-06-20T00:00:00Z", "2026-06-20T00:00:00Z", orch) + testutil.NoError(t, err) + + got, err := d.ListHeraPlannedNodes() + testutil.NoError(t, err) + for _, r := range got { + if r.ID == node.ID { + t.Fatalf("planned node %d under a nuked orchestrator must not be listed", node.ID) + } + } + }) + + t.Run("active orchestrator's planned node is unaffected", func(t *testing.T) { + d := testDB(t) + orch := planTestOrch(t, d, "active-orch") + node := plannedRole(t, d, orch, "node") + + got, err := d.ListHeraPlannedNodes() + testutil.NoError(t, err) + testutil.Equal(t, len(got), 1) + testutil.Equal(t, got[0].ID, node.ID) + }) +} + func TestPlannedNode_ShortIDIsStableAcrossPlanEdits(t *testing.T) { // The planner-assigned short-id-prefixed name is a durable handle: it is // persisted verbatim and never recomputed by a plan operation (adding or diff --git a/internal/db/hera_test.go b/internal/db/hera_test.go index 022989d4..675fb8a1 100644 --- a/internal/db/hera_test.go +++ b/internal/db/hera_test.go @@ -1063,6 +1063,69 @@ func TestNukeHeraOrchestrator(t *testing.T) { }) } +// TestArchiveNukeOrchestrator_CascadeCancelPlannedChildren pins the +// add-hera-plan-hygiene Bug A cascade: archiving or nuking an orchestrator +// stamps cancelled_at on its still-planned (never-materialized) worker-kind +// children, so they stop being retried by the gater forever, while an +// already-materialized (bound) child is left untouched. +func TestArchiveNukeOrchestrator_CascadeCancelPlannedChildren(t *testing.T) { + t.Run("archive cancels a still-planned child", func(t *testing.T) { + d := heraTestDB(t) + o := mkOrch(t, d, "alpha") + planned := mkRole(t, d, o.ID, "planned", HeraKindWorker) + + testutil.NoError(t, d.ArchiveHeraOrchestrator(o.ID)) + + got, err := d.HeraRole(planned.ID) + testutil.NoError(t, err) + if got.CancelledAt == nil { + t.Fatal("expected the still-planned child role to be cancelled") + } + }) + + t.Run("nuke cancels a still-planned child", func(t *testing.T) { + d := heraTestDB(t) + o := mkOrch(t, d, "beta") + planned := mkRole(t, d, o.ID, "planned", HeraKindWorker) + + testutil.NoError(t, d.NukeHeraOrchestrator(o.ID)) + + got, err := d.HeraRole(planned.ID) + testutil.NoError(t, err) + if got.CancelledAt == nil { + t.Fatal("expected the still-planned child role to be cancelled") + } + }) + + t.Run("a materialized (bound) child is left untouched", func(t *testing.T) { + d := heraTestDB(t) + o := mkOrch(t, d, "gamma") + bound, _, err := d.CreateHeraRoleWithBinding(CreateHeraRoleInput{ + OrchestratorID: o.ID, Name: "bound", Kind: HeraKindWorker, ArgusProject: "proj", + }, "task-1", "/wt/bound") + testutil.NoError(t, err) + + testutil.NoError(t, d.ArchiveHeraOrchestrator(o.ID)) + + got, err := d.HeraRole(bound.ID) + testutil.NoError(t, err) + testutil.Nil(t, got.CancelledAt) + }) + + t.Run("an already-cancelled or already-archived child is left as-is", func(t *testing.T) { + d := heraTestDB(t) + o := mkOrch(t, d, "delta") + archived := mkRole(t, d, o.ID, "archived", HeraKindWorker) + testutil.NoError(t, d.ArchiveHeraRole(archived.ID)) + + testutil.NoError(t, d.ArchiveHeraOrchestrator(o.ID)) + + got, err := d.HeraRole(archived.ID) + testutil.NoError(t, err) + testutil.Nil(t, got.CancelledAt) // archived, not cancelled — cascade skips it + }) +} + // TestHeraOrchestratorKanbanStatus pins the add-hera-kanban-status axis: default // value, round-tripping through Set + both read paths, independence from // pin/archive, and the missing-row error. diff --git a/internal/heragater/heragater.go b/internal/heragater/heragater.go index 3c6bbda2..2bb0397f 100644 --- a/internal/heragater/heragater.go +++ b/internal/heragater/heragater.go @@ -41,6 +41,18 @@ import ( // Tests override via SetInterval. const defaultInterval = time.Minute +// materializeFailureEscalationTicks is the number of CONSECUTIVE materialize +// failures a planned node tolerates before the gater stops retrying in total +// silence and pings the coordinator once (add-hera-plan-hygiene Bug B). +// Deliberately smaller than agent.EscalateParkedSelection's +// NeedsInputEscalationTicks (8, see context/knowledge/gotchas/events.md +// BUG-029/BUG-060): that counter guards against isolated torn reads on a +// fast, sub-second polling loop, a noise source that does not exist here — a +// materialize failure is a fully synchronous, deterministic result (config +// resolves or it doesn't), so the threshold only needs to rule out "the very +// next tick will succeed," not survive flaky reads. +const materializeFailureEscalationTicks = 5 + // Materializer binds + starts a pre-created planned role. agent.MaterializeHeraWorker // satisfies this via the daemon adapter; tests inject a fake. Returns the live // task or an error (a materialize failure HOLDS the node — it stays planned and @@ -71,17 +83,27 @@ type Watcher struct { // heldPings dedups failure-hold pings per (blockedRole, blockerRole) so a // holding node doesn't spam the coordinator every tick. heldPings map[[2]int64]bool + + // materializeFailures counts CONSECUTIVE materialize failures per node id + // (add-hera-plan-hygiene Bug B); escalatedMaterializeFailures dedups the + // one-shot escalation notice per node id the same way heldPings dedups + // hold notices. Both are swept in sweepMaterializeFailures once a node + // leaves the planned set. + materializeFailures map[int64]int + escalatedMaterializeFailures map[int64]bool } // New builds a Watcher. It does not tick until Start is called. func New(database *db.DB, materialize Materializer, ping CoordinatorPinger) *Watcher { return &Watcher{ - db: database, - materialize: materialize, - ping: ping, - interval: defaultInterval, - stopCh: make(chan struct{}), - heldPings: map[[2]int64]bool{}, + db: database, + materialize: materialize, + ping: ping, + interval: defaultInterval, + stopCh: make(chan struct{}), + heldPings: map[[2]int64]bool{}, + materializeFailures: map[int64]int{}, + escalatedMaterializeFailures: map[int64]bool{}, } } @@ -190,6 +212,23 @@ func (w *Watcher) Tick() { } } w.rearmHeldPings(plannedByID) + w.sweepMaterializeFailures(plannedByID) +} + +// sweepMaterializeFailures discards any per-node failure/escalation bookkeeping +// for a node that is no longer in the planned set — materialized, cancelled, or +// removed (add-hera-plan-hygiene Bug B). Mirrors rearmHeldPings's cleanup of +// heldPings: without this, both maps would grow one stale entry per node ever +// planned, for the lifetime of the daemon process. +func (w *Watcher) sweepMaterializeFailures(plannedByID map[int64]*db.HeraRole) { + w.mu.Lock() + defer w.mu.Unlock() + for id := range w.materializeFailures { + if _, ok := plannedByID[id]; !ok { + delete(w.materializeFailures, id) + delete(w.escalatedMaterializeFailures, id) + } + } } // rearmHeldPings sweeps the held-ping dedup (D4) so a held node is re-reported @@ -468,8 +507,10 @@ func (w *Watcher) materializeNode(node *db.HeraRole) { if err := w.materialize(node, taskPrompt, project, branch, "", ""); err != nil { w.logf("[heragater] materialize %d (%s) FAILED (stays planned, retry next tick): %v", node.ID, node.Name, err) + w.recordMaterializeFailure(node, err) return } + w.clearMaterializeFailures(node.ID) w.logf("[heragater] materialized node %d (%s) in orch %q (base_branch=%q)", node.ID, node.Name, orch.Name, branch) if len(blockerIDs) > 1 && winningBlockerID != 0 { w.pingFanIn(node, blockerIDs, winningBlockerID, branch) @@ -503,8 +544,10 @@ func (w *Watcher) materializeSubCoord(node *db.HeraRole, parentOrchName, coordNa if err := seam(node, taskPrompt, project, branch, "", ""); err != nil { w.logf("[heragater] materialize SUBCOORD %d (%s) FAILED (stays planned, retry next tick): %v", node.ID, node.Name, err) + w.recordMaterializeFailure(node, err) return } + w.clearMaterializeFailures(node.ID) w.logf("[heragater] materialized SUBCOORD node %d (%s) in orch %q (child_orch=%s, base_branch=%q)", node.ID, node.Name, parentOrchName, node.Name, branch) if cb := w.materializeCallback(); cb != nil { cb(node) @@ -687,6 +730,61 @@ func (w *Watcher) pingFanIn(node *db.HeraRole, blockerIDs []int64, winningBlocke node.ID, node.Name, branch, winnerName, len(siblingNames)) } +// recordMaterializeFailure increments a planned node's consecutive-failure +// counter and, once it crosses materializeFailureEscalationTicks, pings the +// coordinator EXACTLY ONCE (add-hera-plan-hygiene Bug B) — a node that can +// never materialize (e.g. a blank argus_project) must not retry in total +// silence forever. Mirrors holdAndPing's ping-once-per-condition dedup, keyed +// on node id alone (this escalation is per-node, not per-blocker). Never +// cancels or reconfigures the node — escalation is advisory only. +func (w *Watcher) recordMaterializeFailure(node *db.HeraRole, cause error) { + w.mu.Lock() + if w.materializeFailures == nil { + w.materializeFailures = map[int64]int{} + } + w.materializeFailures[node.ID]++ + count := w.materializeFailures[node.ID] + alreadyEscalated := w.escalatedMaterializeFailures[node.ID] + w.mu.Unlock() + + if count < materializeFailureEscalationTicks || alreadyEscalated { + return + } + coords, err := w.db.ListHeraRolesByKind(node.OrchestratorID, db.HeraKindCoordinator) + if err != nil || len(coords) == 0 { + w.logf("[heragater] escalate %d: no coordinator to ping: %v", node.ID, err) + return + } + body := fmt.Sprintf( + "Planned node %s has FAILED to materialize %d consecutive ticks and will keep retrying silently forever unless you intervene. Last error: %v", + node.Name, count, cause) + tldr := fmt.Sprintf("stuck: %s failed to materialize %d times", node.Name, count) + if w.ping != nil { + if pErr := w.ping(node.ID, coords[0].ID, body, tldr); pErr != nil { + w.logf("[heragater] escalate %d: notify coordinator failed (will retry once threshold is crossed again next tick): %v", node.ID, pErr) + return + } + } + w.mu.Lock() + if w.escalatedMaterializeFailures == nil { + w.escalatedMaterializeFailures = map[int64]bool{} + } + w.escalatedMaterializeFailures[node.ID] = true + w.mu.Unlock() + w.logf("[heragater] escalated node %d (%s): %d consecutive materialize failures; pinged coordinator", node.ID, node.Name, count) +} + +// clearMaterializeFailures resets a node's failure counter and escalation flag +// once materialization succeeds. A node that fails a few times and later +// succeeds (e.g. because a config gap was fixed concurrently) must not carry a +// stale escalated-flag forward if it is ever re-planned. +func (w *Watcher) clearMaterializeFailures(nodeID int64) { + w.mu.Lock() + delete(w.materializeFailures, nodeID) + delete(w.escalatedMaterializeFailures, nodeID) + w.mu.Unlock() +} + // roleName resolves a role id to its display name, degrading to a placeholder // on lookup failure rather than propagating an error into a notice string. func (w *Watcher) roleName(roleID int64) string { diff --git a/internal/heragater/heragater_test.go b/internal/heragater/heragater_test.go index 051cc0fe..3fb93ad9 100644 --- a/internal/heragater/heragater_test.go +++ b/internal/heragater/heragater_test.go @@ -434,6 +434,114 @@ func TestGater_MaterializeFailureLeavesNodePlanned(t *testing.T) { testutil.Equal(t, planned[0].ID, node.ID) } +// TestGater_MaterializeFailureBelowThresholdNoEscalation pins add-hera-plan- +// hygiene Bug B: a node that has failed fewer than +// materializeFailureEscalationTicks consecutive times retries in silence, with +// no coordinator notice yet. +func TestGater_MaterializeFailureBelowThresholdNoEscalation(t *testing.T) { + f := newGaterFixture(t) + orch := f.seedCoord(t, "orch") + f.planned(t, orch, "1a") + f.matFail = true + + for i := 0; i < materializeFailureEscalationTicks-1; i++ { + f.w.Tick() + } + + testutil.Equal(t, f.pingCount(), 0) +} + +// TestGater_MaterializeFailureEscalatesAtThreshold pins the crossing behavior: +// on the Nth consecutive failure the coordinator gets exactly one notice +// naming the node, instead of retrying in total silence forever. +func TestGater_MaterializeFailureEscalatesAtThreshold(t *testing.T) { + f := newGaterFixture(t) + orch := f.seedCoord(t, "orch") + node := f.planned(t, orch, "2a-team") + f.matFail = true + + for i := 0; i < materializeFailureEscalationTicks; i++ { + f.w.Tick() + } + + testutil.Equal(t, f.pingCount(), 1) + last := f.lastPing() + testutil.Equal(t, last.from, node.ID) + testutil.Equal(t, strings.Contains(last.tldr, "stuck"), true) + testutil.Equal(t, strings.Contains(last.body, node.Name), true) + // Never auto-cancelled or reconfigured — escalation is advisory only. + got, err := f.d.HeraRole(node.ID) + testutil.NoError(t, err) + testutil.Nil(t, got.CancelledAt) +} + +// TestGater_MaterializeFailureEscalationDoesNotRepeat pins the one-shot +// contract: once escalated, continued failure does not re-ping every tick. +func TestGater_MaterializeFailureEscalationDoesNotRepeat(t *testing.T) { + f := newGaterFixture(t) + orch := f.seedCoord(t, "orch") + f.planned(t, orch, "2a") + f.matFail = true + + for i := 0; i < materializeFailureEscalationTicks+3; i++ { + f.w.Tick() + } + + testutil.Equal(t, f.pingCount(), 1) +} + +// TestGater_MaterializeFailureSuccessClearsCounter pins that a node which +// later succeeds has its failure/escalation bookkeeping cleared, so a +// re-planned node under the same id (a re-plan is a fresh row in practice, but +// the counter must not leak stale state regardless) never inherits it. +func TestGater_MaterializeFailureSuccessClearsCounter(t *testing.T) { + f := newGaterFixture(t) + orch := f.seedCoord(t, "orch") + node := f.planned(t, orch, "1a") + f.matFail = true + + for i := 0; i < materializeFailureEscalationTicks-1; i++ { + f.w.Tick() + } + testutil.Equal(t, f.pingCount(), 0) // not yet escalated + + f.matFail = false + f.w.Tick() // succeeds this time — materializes + + testutil.Equal(t, len(f.materialized()), 1) + f.w.mu.Lock() + _, stillCounted := f.w.materializeFailures[node.ID] + _, stillEscalated := f.w.escalatedMaterializeFailures[node.ID] + f.w.mu.Unlock() + testutil.Equal(t, stillCounted, false) + testutil.Equal(t, stillEscalated, false) +} + +// TestGater_MaterializeFailureSweptOnNodeRemoval pins sweepMaterializeFailures: +// once a node leaves the planned set for any reason other than success (here, +// cancellation), its failure/escalation bookkeeping is discarded rather than +// growing the map forever. +func TestGater_MaterializeFailureSweptOnNodeRemoval(t *testing.T) { + f := newGaterFixture(t) + orch := f.seedCoord(t, "orch") + node := f.planned(t, orch, "1a") + f.matFail = true + + f.w.Tick() // one recorded failure, well under threshold + f.w.mu.Lock() + _, counted := f.w.materializeFailures[node.ID] + f.w.mu.Unlock() + testutil.Equal(t, counted, true) + + testutil.NoError(t, f.d.CancelHeraPlannedNode(node.ID)) + f.w.Tick() // node no longer planned; sweep runs + + f.w.mu.Lock() + _, stillCounted := f.w.materializeFailures[node.ID] + f.w.mu.Unlock() + testutil.Equal(t, stillCounted, false) +} + // TestGater_HoldNoCoordinatorNoPanic covers the holdAndPing path when no // coordinator exists to ping (logged, no panic, no ping recorded). func TestGater_HoldNoCoordinatorNoPanic(t *testing.T) { diff --git a/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/.openspec.yaml b/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/.openspec.yaml new file mode 100644 index 00000000..ab396754 --- /dev/null +++ b/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-30 diff --git a/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/design.md b/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/design.md new file mode 100644 index 00000000..f43d1fdf --- /dev/null +++ b/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/design.md @@ -0,0 +1,78 @@ +## Context + +Three hera plan-DAG / rail data-hygiene bugs were independently verified against the live `~/.argus/data.sql`. They are unrelated defects with different root causes — this design deliberately does NOT try to unify them under one fix. + +- **Bug A** (`ListHeraPlannedNodes`, `internal/db/hera_plan.go:359-371`): the query filters only the node's own `archived_at`/`cancelled_at`, never the parent orchestrator's `archived_at`/`nuked_at`. Roles 343 ("4a-flex", orch 47 "plan-view-dogfood") and 358 ("3a-integrate", orch 49 "dag-states-test") have retried every ~60s heragater tick for over a month behind orchestrators archived+nuked on 2026-06-20. +- **Bug B** (`materializeNode`, `internal/heragater/heragater.go:439-480`): role 184 ("2a-team", live orchestrator 8 "sherlock-mvp", zero remaining blockers) has a blank `argus_project`. `agent.CreateAndStart` (`internal/agent/create.go:119`) errors `project %q not found in config`, and the gater retries forever with no escalation. Unlike Bug A, this is genuine pending work — the fix must not guess a project or cancel it. +- **Bug C** (`internal/tui/hera/model.go:1043`): freelance roles 813/814 render in the flat top-level Freelance section (which a `kind=freelance` role only escapes by being archived, unlike `kind=worker` which always nests). Their data chain — role, orchestrator, binding, task — is fully intact; they were simply never archived once their work finished (task 1782110807411961000 in_review with a merged PR, task 1781194310470051000 complete). ~150 other historical roles show the identical "task finished, binding still open" shape and render correctly because they're `kind=worker`. + +## Goals / Non-Goals + +**Goals:** + +- Bug A: a planned node under an archived/nuked orchestrator must stop being polled/retried, both prospectively (new archives/nukes) and for rows that already exist (the defensive list-query filter covers both without a data migration). +- Bug B: a node that can never materialize must not retry in total silence forever — it should escalate to the coordinator after a bounded number of consecutive failures, with zero guessing at the actual fix. +- Bug C: understand and document why this shape occurred, and clean up the two known-broken rows, without changing rendering logic that is behaving as designed. +- Retroactively repair the specific broken rows already in the live DB (Part 2), through the daemon's own `*db.DB`, not hand-edited SQL. + +**Non-Goals:** + +- Guessing or auto-assigning `argus_project` for role 184, or auto-cancelling it. It has zero remaining blockers and is genuine pending work; only a human can say what it should target. +- Auto-archiving freelance roles when their binding ends. This would be a real behavior change (see Decision: Bug C below) and is out of scope for a hygiene fix targeting three specific verified bugs. +- Any change to `hera_bindings` uniqueness semantics, the plan-DAG's cycle detection, or fan-in branch resolution — all unrelated to these three bugs. + +## Decisions + +### Bug A: defensive filter in the read path, not just a write-time cascade + +Two independent fixes are needed, not one: + +1. `ArchiveHeraOrchestrator`/`NukeHeraOrchestrator` cascade-cancel their still-planned (never-materialized) child roles at the moment of archive/nuke. This prevents the bug from recurring for any FUTURE archive/nuke. +2. `ListHeraPlannedNodes` additionally joins `hera_orchestrators` and requires `archived_at IS NULL AND nuked_at IS NULL` on the parent. + +(1) alone does not fix rows 343/358, which predate the fix — a one-time data migration would be needed to make (1) retroactive. (2) alone works for existing AND future rows without any migration, but leaves a defensive gap if a future code path other than Archive/Nuke ever flips those columns without going through the cascade. Both together mean the invariant holds from either direction: the write path prevents the state from occurring, and the read path tolerates it if it ever does anyway (matching this codebase's existing "list queries are the belt-and-braces layer" pattern already used for nuked orchestrators — see `internal/tui/hera/model.go`'s defense-in-depth comment on `o.NukedAt`). + +Alternative considered: only fix the read-path filter and skip the cascade-cancel. Rejected — `ListHeraPlannedNodes` would then silently and permanently exclude cancellable dead nodes from ever appearing anywhere (including future plan-view tooling that might want to show "N nodes cancelled because their orchestrator ended"), leaving them un-cancelled forever in a state that looks alive everywhere except the one query that matters today. Stamping `cancelled_at` is the more honest representation and costs one extra `UPDATE`. + +### Bug B: bounded consecutive-failure escalation, mirroring `agent.EscalateParkedSelection` + +`heragater.Watcher` already has a proven shape for "this transient-looking failure has actually been going on for a suspiciously long time, tell someone" — `holdAndPing`'s per-(node,blocker) dedup map, and (in a different package) `agent.EscalateParkedSelection`'s bounded consecutive-tick counter (`NeedsInputEscalationTicks=8`, see `context/knowledge/gotchas/events.md` BUG-029/BUG-060). This change adds the same shape for materialize failures: an in-memory `map[int64]int` counting consecutive failures per node id, and a `map[int64]bool` recording that the one-time escalation ping has already fired for that node (so it does not spam every tick after crossing the threshold — matching `holdAndPing`'s ping-once contract). Both maps are swept each tick for any node id no longer in the planned set (materialized, cancelled, or removed), mirroring `rearmHeldPings`'s cleanup of `heldPings` — an unbounded-forever-growing map would be exactly the kind of hygiene defect this change exists to fix. + +Threshold: 5 consecutive ticks (`materializeFailureEscalationTicks`), i.e. ~5 minutes at the gater's 1-minute tick interval. This is deliberately smaller than `NeedsInputEscalationTicks` (8) — that counter guards against isolated torn reads/blink-off frames on a fast, sub-second polling loop, a noise source that does not apply here. A materialize failure is a fully synchronous, deterministic result (config resolves or it doesn't) with no torn-read risk, so the threshold only needs to rule out "the coordinator is mid-recovering something and the very next tick will succeed" — 5 minutes is generous for that and still surfaces a permanently-broken node well before "over a month," the actual duration Bug A's rows sat silent. + +On success, both maps are cleared for that node id (a node that fails a few times then succeeds — e.g. because a project config gap was fixed concurrently — should not carry a stale escalated-flag forward if it is later re-planned). + +Alternative considered: auto-cancel the node after N failures, treating repeated failure as equivalent to "this was a mistake." Rejected per the explicit brief: node 184 has zero remaining blockers and is verified pending work; auto-cancelling any node exceeding the threshold could silently discard real DAG state that a human just hasn't gotten to yet. Escalate-and-wait is the correct action; auto-remediation is not. + +Alternative considered: make the escalation notice re-fire periodically (like a nagging reminder) rather than once. Rejected for consistency with the existing `holdAndPing`/`pingFanIn` one-shot-or-dedup conventions already established in this file — a coordinator that ignores the first escalation notice pressing it again every tick adds noise, not information; the node stays visible in the plan-DAG view regardless. + +### Bug C: no code change — this is a data-hygiene finding, not a display defect + +The flat-Freelance-section rule (`role.Kind == HeraKindFreelance && role.ArchivedAt == nil && o.ArchivedAt == nil`) is intentional, documented behavior (`context/knowledge/gotchas/hera-view.md`: "Freelance section"). The bug is that nobody archived roles 813/814 once their work finished — an operational gap, not a logic error. No `Clean`/`Prune`/`Sweep`-named function touching hera data exists anywhere in this codebase; whatever produced this specific pair's un-archived state was very likely a manual/ad hoc action outside argus, not an argus feature bug. + +Considered and rejected: auto-archiving a freelance role once its binding ends (mirroring how a worker role's nesting is unconditional regardless of archived state, an auto-archive would make freelance behave the same way from the rail's perspective). Rejected here because it is a genuine behavior change with its own design surface (when exactly does "binding ended" count — immediately, after a grace period, only on terminal task status?) that the three-bug brief explicitly scoped OUT: Bug C's fix, per the task brief, is Part 2 (data cleanup) only. If this shape recurs, it is a separate, affirmatively-scoped follow-up change, not smuggled into this one. + +## Risks / Trade-offs + +- [Bug A's JOIN adds a small query-plan cost to `ListHeraPlannedNodes`, called every gater tick] → negligible: `hera_orchestrators` is tiny (one row per orchestrator ever created) and the query is already `O(planned nodes)`; adding an indexed-PK join does not change its asymptotic cost. +- [Bug B's escalation maps live only in the in-process `Watcher` struct — a daemon restart forgets any in-flight failure streak] → acceptable: the streak is a "how long has this been broken lately" heuristic, not durable state; a restart simply restarts the count, and a permanently-broken node re-crosses the threshold within another 5 ticks. +- [Cascade-cancel could theoretically race a concurrent `hera_plan_node` create on the same orchestrator between the archive UPDATE and the cascade UPDATE] → both statements scope by `orchestrator_id` and the cascade query re-checks `archived_at IS NULL AND cancelled_at IS NULL AND NOT EXISTS (binding)` at execution time; a create that lands in that narrow window would either land before the cascade (and get cancelled, correct) or after (and become an orphaned planned node under an archived orchestrator — but Bug A's own read-path filter in `ListHeraPlannedNodes` already covers that case defensively, so the DAG-hygiene invariant holds either way). + +## Migration Plan + +**Code (this PR):** no schema change. `cancelled_at`, `archived_at`, `nuked_at` are pre-existing nullable columns (see `context/knowledge/gotchas/orchestration.md` "Hera schema / store (M1)" and "Hera living plan-DAG"). + +**Data (Part 2, run once against the live `~/.argus/data.sql`, through the daemon's own `*db.DB`, not raw `sqlite3 UPDATE`):** + +1. Fresh backup: `cp ~/.argus/data.sql ~/.argus/data.sql.bak-` immediately before the mutation (a pre-investigation backup already exists at `.bak-20260729-213549`, but a fresh one is taken right before the actual write). +2. `CancelHeraPlannedNode(343)`, `CancelHeraPlannedNode(358)` — the exact pre-existing store method Bug A's own fix relies on; this migration is effectively "run the same cascade the code fix would have run, had it existed a month ago." +3. `ArchiveHeraRole(813)`, `ArchiveHeraRole(814)` — pre-existing store method. +4. `EndHeraBinding(803, endReason)`, `EndHeraBinding(804, endReason)` (or the equivalent store call — see `internal/db/hera.go` for the exact signature) with an end reason describing "task finished, binding never closed." +5. Node 184 is explicitly excluded — no mutation touches it. +6. Verify via `sqlite3 -readonly ~/.argus/data.sql`: 343/358 no longer satisfy `ListHeraPlannedNodes`'s WHERE clause; 813/814 have non-null `archived_at`; bindings 803/804 have non-null `ended_at`; role 184 is unchanged. + +**Rollback:** restore from the fresh pre-mutation backup; all four mutations are simple column stamps with no cascading side effects (cancel/archive do not touch bindings; ending a binding does not touch the role), so no compensating script is needed beyond a straight file restore. + +## Open Questions + +- What `argus_project` should role 184 ("2a-team" under "sherlock-mvp") target? Left to Aaron — reported back via `hera_send` as an open question, not resolved in this change. diff --git a/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/proposal.md b/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/proposal.md new file mode 100644 index 00000000..1a0d297a --- /dev/null +++ b/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/proposal.md @@ -0,0 +1,29 @@ +## Why + +Three independently-verified hera plan-DAG / rail data-hygiene bugs were found live in `~/.argus/data.sql`, each a different failure mode with a different root cause: + +- **Bug A**: `heragater` polls planned nodes forever even after their orchestrator has been archived or nuked. `ListHeraPlannedNodes` filters only on the node's own `archived_at`/`cancelled_at`, never the parent orchestrator's. Two live rows (roles 343 and 358) have retried materialization every ~60s tick for over a month behind orchestrators archived+nuked on 2026-06-20, spamming daemon.log with `hold: no coordinator to ping: `. +- **Bug B**: a planned node (role 184, "2a-team" under live orchestrator "sherlock-mvp") has a blank `argus_project`, so `agent.CreateAndStart` fails materialization every tick, forever, with zero operator visibility — it just retries silently. This node has no remaining blockers and is genuine pending work, not dead data, so it must not be auto-cancelled or guessed at. +- **Bug C**: two freelance roles (813, 814) render as orphaned in the Hera rail's flat top-level Freelance section even though their role→orchestrator→binding→task chain is fully intact — they were simply never archived once their work finished (a `kind=freelance` role only nests inside its orchestrator when both it and the orchestrator are archived-or-not in sync; `kind=worker` roles always nest regardless). This is confirmed data-hygiene, not a display defect — the identical "task complete, binding still open" shape is the NORMAL, common case for ~150 other historical worker roles that render fine because they nest unconditionally. + +This change fixes the two genuine code defects (A and B) and separately cleans up the specific broken rows already sitting in the live DB (A's two orphaned planned nodes, C's two un-archived freelance roles + their stale-open bindings). Node 184 (Bug B) is left untouched — nobody knows what project it should target, so it is flagged back to the human rather than resolved here. + +## What Changes + +- `ListHeraPlannedNodes` (`internal/db/hera_plan.go`) additionally excludes any planned node whose parent orchestrator has `archived_at` or `nuked_at` set — a defensive filter that also retroactively silences existing broken rows without needing a data migration to take effect. +- `ArchiveHeraOrchestrator` and `NukeHeraOrchestrator` (`internal/db/hera.go`) cascade-cancel (`cancelled_at`) their still-planned (never-materialized) child roles, so a future archive/nuke does not orphan pollable dead nodes in the first place. +- `heragater.materializeNode` (`internal/heragater/heragater.go`) tracks consecutive materialization failures per planned node and, after a bounded threshold, sends a one-time escalation notice to the coordinator instead of retrying silently forever — mirroring the existing `agent.EscalateParkedSelection` consecutive-tick-escalation shape used elsewhere in Hera. It never auto-cancels or guesses a fix for the failing node. +- One-shot data cleanup (not shipped as application code) against the live `~/.argus/data.sql`, run through the daemon's own `*db.DB` connection: cancel planned nodes 343 and 358 (Bug A), archive freelance roles 813 and 814 and close their live bindings 803/804 with an explicit end reason (Bug C). Node 184 (Bug B) is explicitly excluded from this cleanup. + +## Capabilities + +### Modified Capabilities + +- `task-orchestration`: the planned-node listing requirement gains a parent-orchestrator liveness condition (Bug A), and the gater materialization requirement gains a bounded-escalation-on-repeated-failure behavior (Bug B). + +## Impact + +- `internal/db/hera_plan.go`, `internal/db/hera.go`, `internal/heragater/heragater.go`, plus new/updated tests in `internal/db/hera_plan_test.go`, `internal/db/hera_test.go`, `internal/heragater/heragater_test.go`. +- `context/knowledge/gotchas/orchestration.md` gains a new invariant entry. +- No schema change — `cancelled_at`, `archived_at`, `nuked_at` are pre-existing nullable columns. +- Bug C (freelance-role display) requires no code change — see design.md for why. diff --git a/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/specs/task-orchestration/spec.md b/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/specs/task-orchestration/spec.md new file mode 100644 index 00000000..7bf92327 --- /dev/null +++ b/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/specs/task-orchestration/spec.md @@ -0,0 +1,54 @@ +## ADDED Requirements + +### Requirement: A planned node is excluded once its parent orchestrator archives or is nuked + +The system SHALL treat a planned node as no longer eligible for gating or materialization once its parent orchestrator has been archived or nuked, even though the node's own `archived_at`/`cancelled_at` are unaffected by an orchestrator-level action. Archiving or nuking an orchestrator SHALL cascade-cancel (stamp `cancelled_at`) every still-planned (never-materialized, not already archived or cancelled) worker-kind child role belonging to it, at the moment of the archive/nuke. Independently of that cascade, the planned-node listing query SHALL also exclude any node whose parent orchestrator has `archived_at` or `nuked_at` set, regardless of the node's own `cancelled_at` — so a node that predates this fix (its orchestrator ended before the cascade existed) is excluded without requiring any data migration. + +#### Scenario: Archiving an orchestrator cancels its still-planned children + +- **WHEN** a coordinator's orchestrator with a never-materialized planned child node is archived +- **THEN** the child node's `cancelled_at` is stamped + +#### Scenario: Nuking an orchestrator cancels its still-planned children + +- **WHEN** an orchestrator with a never-materialized planned child node is nuked +- **THEN** the child node's `cancelled_at` is stamped + +#### Scenario: A materialized (already-bound) child is not cancelled + +- **WHEN** an orchestrator is archived or nuked +- **THEN** any child role that already holds a binding (materialized) is left untouched — cascade-cancel only reaches never-bound planned nodes + +#### Scenario: A planned node under an archived orchestrator is excluded from the gate even without cascade-cancel having run + +- **WHEN** a planned node's parent orchestrator has `archived_at` or `nuked_at` set, regardless of whether the node itself carries `cancelled_at` +- **THEN** the node does not appear in the set of nodes the gater evaluates for materialization + +### Requirement: Materialization failures escalate after repeated retries + +The system SHALL track, per planned node, the number of CONSECUTIVE materialization failures since the node last succeeded or was last evaluated as a fresh planned node. When that count reaches a bounded threshold, the system SHALL send a ONE-TIME escalation notice to the coordinator naming the node and the last error, instead of continuing to retry in total silence. The system SHALL NOT automatically cancel, reconfigure, or guess a fix for a node that has crossed the threshold — escalation is advisory only, and the node SHALL remain planned and continue to be retried on the normal tick schedule. A node that later succeeds, or that is no longer a planned node (materialized, cancelled, or removed), SHALL have its failure count and escalation state cleared. + +#### Scenario: A node under the threshold retries silently + +- **WHEN** a planned node's materialization fails fewer than the escalation threshold's consecutive times +- **THEN** no notice is sent and the node remains planned for the next tick + +#### Scenario: Crossing the threshold sends a one-time notice + +- **WHEN** a planned node's materialization fails the escalation threshold's consecutive times +- **THEN** the coordinator receives exactly one escalation notice naming the node and the last error + +#### Scenario: The notice does not repeat every subsequent tick + +- **WHEN** a node continues failing after already crossing the escalation threshold +- **THEN** no further escalation notice is sent for that node + +#### Scenario: A later success clears the failure count + +- **WHEN** a previously-failing planned node materializes successfully +- **THEN** its consecutive-failure count and escalation state are cleared + +#### Scenario: Escalation never auto-cancels or reconfigures the node + +- **WHEN** a planned node crosses the escalation threshold +- **THEN** the node's `argus_project`, prompt, and `cancelled_at` are left unchanged — only a notice is sent diff --git a/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/tasks.md b/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/tasks.md new file mode 100644 index 00000000..ac947c9b --- /dev/null +++ b/openspec/changes/archive/2026-07-30-fix-hera-plan-dag-hygiene/tasks.md @@ -0,0 +1,35 @@ +## 1. Bug A — planned node outlives its archived/nuked orchestrator + +- [x] 1.1 `internal/db/hera_plan.go` `ListHeraPlannedNodes`: join `hera_orchestrators` and require `archived_at IS NULL AND nuked_at IS NULL` on the parent, in addition to the existing node-level filters. +- [x] 1.2 `internal/db/hera.go`: add an unexported `cancelStillPlannedChildRoles(orchID int64) error` helper mirroring `ListHeraPlannedNodes`'s planned-node definition (kind=worker, not archived, not cancelled, no binding ever), scoped to one orchestrator. +- [x] 1.3 Wire `cancelStillPlannedChildRoles` into `ArchiveHeraOrchestrator` and `NukeHeraOrchestrator`, logging (not propagating) a cascade failure so the primary archive/nuke call keeps its existing error contract. +- [x] 1.4 `internal/db/hera_plan_test.go`: add a test that a planned node under an archived orchestrator, and one under a nuked orchestrator, are excluded from `ListHeraPlannedNodes` — including the case where the node itself has no `cancelled_at` (proving the defensive read-path filter works independent of the cascade). +- [x] 1.5 `internal/db/hera_test.go`: extend the existing `TestArchiveHeraOrchestrator`/`TestNukeHeraOrchestrator` coverage (or add sibling tests) proving a still-planned child role gets `cancelled_at` stamped, while an already-materialized (bound) child role is left untouched. + +## 2. Bug B — materializeNode escalates instead of retrying silently forever + +- [x] 2.1 `internal/heragater/heragater.go`: add `materializeFailures map[int64]int` and `escalatedMaterializeFailures map[int64]bool` to `Watcher`, plus a `materializeFailureEscalationTicks` constant. +- [x] 2.2 Add `recordMaterializeFailure`/`clearMaterializeFailures` helpers; wire them into `materializeNode`'s failure/success paths. +- [x] 2.3 On crossing the threshold, send a one-shot coordinator notice (same `CoordinatorPinger` seam as `holdAndPing`) naming the node and the last error; never auto-cancel or reconfigure the node. +- [x] 2.4 Sweep both maps each `Tick()` for node ids no longer in the planned set (mirrors `rearmHeldPings`'s cleanup of `heldPings`). +- [x] 2.5 `internal/heragater/heragater_test.go`: add tests for under-threshold silence, one-time notice on crossing the threshold, no repeat notice on continued failure, count/escalation-state clearing on a later success, and sweep-on-node-removal. + +## 3. Documentation + +- [x] 3.1 Add a new entry to `context/knowledge/gotchas/orchestration.md` documenting: the parent-orchestrator liveness filter + cascade-cancel invariant (Bug A), the materialize-failure escalation counter/threshold (Bug B), and a short note that Bug C was data-hygiene only (no code change) with a pointer to why. +- [x] 3.2 Update `context/knowledge/index.md`'s orchestration.md row bullet count if it changes materially. + +## 4. Verification + +- [x] 4.1 `make pre-pr`: build/vet/fmt-check/lint-pr clean; vuln gate shows only stdlib-only advisory findings (CI runs `continue-on-error`, matching `context/knowledge/gotchas/ci-gates.md`); test-cover-gate's two `internal/agent` profile-env test failures are the documented pre-existing hera-worker-sandbox `ARGUS_TASK_ID`/`ARGUS_ARCHETYPE`/`ARGUS_MODEL` env-contamination artifact (unrelated package to this diff; confirmed via `go run ./scripts/coverfilter` against the run's own `coverage.out`: 88.7% filtered, above the 88% floor). +- [x] 4.2 Archive this change (`openspec archive fix-hera-plan-dag-hygiene`) in the same PR before merge. + +## 5. Part 2 — retroactive data cleanup (not application code; run once against the live daemon DB) + +- [ ] 5.1 Fresh backup: `cp ~/.argus/data.sql ~/.argus/data.sql.bak-` immediately before the mutation. +- [ ] 5.2 Through the daemon's own `*db.DB` connection (a small one-off Go program or an admin code path — not raw `sqlite3 UPDATE`): `CancelHeraPlannedNode(343)`, `CancelHeraPlannedNode(358)`. +- [ ] 5.3 `ArchiveHeraRole(813)`, `ArchiveHeraRole(814)`. +- [ ] 5.4 `EndHeraBinding(803, )`, `EndHeraBinding(804, )` with an end reason describing "task finished, binding never closed." +- [ ] 5.5 Leave role 184 completely untouched. +- [ ] 5.6 Verify via `sqlite3 -readonly ~/.argus/data.sql`: 343/358 no longer satisfy the planned-node query, 813/814 have non-null `archived_at`, bindings 803/804 have non-null `ended_at`, role 184 is unchanged. +- [ ] 5.7 Report role 184 back to Aaron as an open question (what project should it target) — do not resolve it. diff --git a/openspec/specs/task-orchestration/spec.md b/openspec/specs/task-orchestration/spec.md index 77336fc2..5b798f68 100644 --- a/openspec/specs/task-orchestration/spec.md +++ b/openspec/specs/task-orchestration/spec.md @@ -305,3 +305,56 @@ When all blockers of a `subcoord` planned node have reached role-status `done`, - **WHEN** a `subcoord` node materializes - **THEN** the resulting sub-coordinator has its own distinct agent and does not share the parent coordinator's task +### Requirement: A planned node is excluded once its parent orchestrator archives or is nuked + +The system SHALL treat a planned node as no longer eligible for gating or materialization once its parent orchestrator has been archived or nuked, even though the node's own `archived_at`/`cancelled_at` are unaffected by an orchestrator-level action. Archiving or nuking an orchestrator SHALL cascade-cancel (stamp `cancelled_at`) every still-planned (never-materialized, not already archived or cancelled) worker-kind child role belonging to it, at the moment of the archive/nuke. Independently of that cascade, the planned-node listing query SHALL also exclude any node whose parent orchestrator has `archived_at` or `nuked_at` set, regardless of the node's own `cancelled_at` — so a node that predates this fix (its orchestrator ended before the cascade existed) is excluded without requiring any data migration. + +#### Scenario: Archiving an orchestrator cancels its still-planned children + +- **WHEN** a coordinator's orchestrator with a never-materialized planned child node is archived +- **THEN** the child node's `cancelled_at` is stamped + +#### Scenario: Nuking an orchestrator cancels its still-planned children + +- **WHEN** an orchestrator with a never-materialized planned child node is nuked +- **THEN** the child node's `cancelled_at` is stamped + +#### Scenario: A materialized (already-bound) child is not cancelled + +- **WHEN** an orchestrator is archived or nuked +- **THEN** any child role that already holds a binding (materialized) is left untouched — cascade-cancel only reaches never-bound planned nodes + +#### Scenario: A planned node under an archived orchestrator is excluded from the gate even without cascade-cancel having run + +- **WHEN** a planned node's parent orchestrator has `archived_at` or `nuked_at` set, regardless of whether the node itself carries `cancelled_at` +- **THEN** the node does not appear in the set of nodes the gater evaluates for materialization + +### Requirement: Materialization failures escalate after repeated retries + +The system SHALL track, per planned node, the number of CONSECUTIVE materialization failures since the node last succeeded or was last evaluated as a fresh planned node. When that count reaches a bounded threshold, the system SHALL send a ONE-TIME escalation notice to the coordinator naming the node and the last error, instead of continuing to retry in total silence. The system SHALL NOT automatically cancel, reconfigure, or guess a fix for a node that has crossed the threshold — escalation is advisory only, and the node SHALL remain planned and continue to be retried on the normal tick schedule. A node that later succeeds, or that is no longer a planned node (materialized, cancelled, or removed), SHALL have its failure count and escalation state cleared. + +#### Scenario: A node under the threshold retries silently + +- **WHEN** a planned node's materialization fails fewer than the escalation threshold's consecutive times +- **THEN** no notice is sent and the node remains planned for the next tick + +#### Scenario: Crossing the threshold sends a one-time notice + +- **WHEN** a planned node's materialization fails the escalation threshold's consecutive times +- **THEN** the coordinator receives exactly one escalation notice naming the node and the last error + +#### Scenario: The notice does not repeat every subsequent tick + +- **WHEN** a node continues failing after already crossing the escalation threshold +- **THEN** no further escalation notice is sent for that node + +#### Scenario: A later success clears the failure count + +- **WHEN** a previously-failing planned node materializes successfully +- **THEN** its consecutive-failure count and escalation state are cleared + +#### Scenario: Escalation never auto-cancels or reconfigures the node + +- **WHEN** a planned node crosses the escalation threshold +- **THEN** the node's `argus_project`, prompt, and `cancelled_at` are left unchanged — only a notice is sent +