fix(director): self-heal legacy dispatch-error strandings + give declines a distinct status (ward#524 follow-up) #527

New issue

Closed

opened 2026-07-02 08:57:34 +00:00 by coilysiren · 3 comments

coilysiren commented

2026-07-02 08:57:34 +00:00

Owner

Why

ward#524 stops the director from parking a dispatch-time infra failure as a terminal failed - going forward those defer and retry, so a transient forge blip self-heals. But ward#524 is forward-looking only: it does not un-strand the issues the old classifier already parked. The four runs that wedged the live backlog (#470, #481, #482, #441) are still sitting failed with a dispatch-error outcome in the host's ~/.ward/backlog/*.yaml, and nothing re-queues them - so the managed backlog stays wedged until they are recovered.

What

A one-shot, self-limiting self-heal migration plus a small hardening of ward#524 so the migration can target legacy strandings precisely:

Distinct decline status (hardening of ward#524). A coded per-issue decline now parks with outcome status declined instead of dispatch-error. This makes dispatch-error a status that only the pre-ward#524 classifier ever produced - so it uniquely identifies a launch/infra stranding.
Re-queue on refresh. applyRankedBacklogEntry gains a case: a headless-lane entry that is failed with the legacy dispatch-error status (isStrandedDispatchError) is re-queued and its outcome cleared. It runs on the normal refresh path, so the moment the director next ticks with this code, the four stranded issues return to queued and dispatch (the forge read path is healthy again - verified). It is self-limiting: post-ward#524 nothing produces failed+dispatch-error, and a genuine declined entry is never matched.

The actual un-wedge happens on the host when the director next runs with this code - the ledger is host state, so this repo change is the vehicle, the next host tick is the effect.

Relationship to ward#524 (read before implementing)

This issue and ward#524 both touch directorDispatchDisposition and the director docs, and were dispatched close together. The attached patch is the complete end-state over origin/main - it contains the ward#524 classification fix and this recovery, all gate-clean (ward exec build/test/vet green, pre-commit run green, no new lint). So:

If ward#524 has already landed on main: integrate main, then reconcile directorDispatchDisposition and docs/agent-director*.md to the end-state in the patch (the net delta is the declined status + isStrandedDispatchError + the migration case + the TestRefreshBacklogLedger cases + the doc's ward#525 section).
If it has not: this patch lands both coherently.

Either way the target is one coherent end-state, so the race cannot leave main half-converged.

Notes for the engineer

Docs: ward#524's prose moved into docs/agent-director-dispatch.md (the main docs/agent-director.md is at the 4000-char cap; a pointer section links out). The new doc is added to CONTRIBUTOR_DOCS in scripts/lint_issue_refs.py so its terse ward#N refs pass the issue-refs hook.
Keep contiguous code-comment blocks to 2 lines and comment lines under 90 chars (the code-comments hook is strict); durable rationale lives in the doc.
Replace any ward#525 placeholder in comments/docs with this issue's actual number before committing.

Validated patch in the first comment.

## Why ward#524 stops the director from parking a dispatch-time infra failure as a terminal `failed` - going forward those defer and retry, so a transient forge blip self-heals. But ward#524 is **forward-looking only**: it does not un-strand the issues the *old* classifier already parked. The four runs that wedged the live backlog (#470, #481, #482, #441) are still sitting `failed` with a `dispatch-error` outcome in the host's `~/.ward/backlog/*.yaml`, and nothing re-queues them - so the managed backlog stays wedged until they are recovered. ## What A one-shot, self-limiting **self-heal migration** plus a small hardening of ward#524 so the migration can target legacy strandings precisely: 1. **Distinct decline status (hardening of ward#524).** A coded per-issue decline now parks with outcome status `declined` instead of `dispatch-error`. This makes `dispatch-error` a status that *only* the pre-ward#524 classifier ever produced - so it uniquely identifies a launch/infra stranding. 2. **Re-queue on refresh.** `applyRankedBacklogEntry` gains a case: a headless-lane entry that is `failed` with the legacy `dispatch-error` status (`isStrandedDispatchError`) is re-queued and its outcome cleared. It runs on the normal refresh path, so the moment the director next ticks with this code, the four stranded issues return to `queued` and dispatch (the forge read path is healthy again - verified). It is self-limiting: post-ward#524 nothing produces `failed`+`dispatch-error`, and a genuine `declined` entry is never matched. The actual un-wedge happens on the host when the director next runs with this code - the ledger is host state, so this repo change is the vehicle, the next host tick is the effect. ## Relationship to ward#524 (read before implementing) This issue and ward#524 both touch `directorDispatchDisposition` and the director docs, and were dispatched close together. **The attached patch is the complete end-state over `origin/main`** - it contains the ward#524 classification fix *and* this recovery, all gate-clean (`ward exec build`/`test`/`vet` green, `pre-commit run` green, no new lint). So: - If ward#524 has already landed on `main`: integrate `main`, then reconcile `directorDispatchDisposition` and `docs/agent-director*.md` to the end-state in the patch (the net delta is the `declined` status + `isStrandedDispatchError` + the migration case + the `TestRefreshBacklogLedger` cases + the doc's ward#525 section). - If it has not: this patch lands both coherently. Either way the target is one coherent end-state, so the race cannot leave `main` half-converged. ## Notes for the engineer - Docs: ward#524's prose moved into `docs/agent-director-dispatch.md` (the main `docs/agent-director.md` is at the 4000-char cap; a pointer section links out). The new doc is added to `CONTRIBUTOR_DOCS` in `scripts/lint_issue_refs.py` so its terse `ward#N` refs pass the issue-refs hook. - Keep contiguous code-comment blocks to 2 lines and comment lines under 90 chars (the code-comments hook is strict); durable rationale lives in the doc. - Replace any `ward#525` placeholder in comments/docs with this issue's actual number before committing. Validated patch in the first comment.

coilysiren commented

2026-07-02 08:58:23 +00:00

Author

Owner

Validated patch (complete end-state over `origin/main`)

Contains the ward#524 classification fix and this recovery. Gate-clean: ward exec build/test/vet green, pre-commit run green, no new lint. Apply from repo root with git apply; if ward#524 already landed, integrate main and reconcile to this end-state.

diff --git a/cmd/ward/agent_director.go b/cmd/ward/agent_director.go
index fe1f6ef..1261fe6 100644
--- a/cmd/ward/agent_director.go
+++ b/cmd/ward/agent_director.go
@@ -16,6 +16,7 @@ import (
 
 	"forgejo.coilysiren.me/coilyco-flight-deck/cli-guard/cli/verb"
 	"forgejo.coilysiren.me/coilyco-flight-deck/cli-guard/pkg/config"
+	"forgejo.coilysiren.me/coilyco-flight-deck/cli-guard/pkg/exitcode"
 	"github.com/urfave/cli/v3"
 	"gopkg.in/yaml.v3"
 )
@@ -623,6 +624,12 @@ func backlogNewEntryState(lane string) string {
 	}
 }
 
+// isStrandedDispatchError reports whether e is a pre-ward#524 launch/infra stranding:
+// parked `failed` with the "dispatch-error" status the old classifier stamped (ward#527).
+func isStrandedDispatchError(e *backlogEntry) bool {
+	return e != nil && e.State == "failed" && e.LastOutcome != nil && e.LastOutcome.Status == "dispatch-error"
+}
+
 // applyRankedBacklogEntry upserts one ranked issue into the ledger, seeding a new
 // entry's state by lane and re-queuing one a re-triage promoted into headless.
 func applyRankedBacklogEntry(led *backlogLedger, rk rankedBacklogIssue) {
@@ -635,6 +642,10 @@ func applyRankedBacklogEntry(led *backlogLedger, rk rankedBacklogIssue) {
 		// A re-triage promoted this into headless from a non-in-flight holding
 		// state: re-queue it rather than strand it.
 		entry.State = "queued"
+	case rk.Lane == "headless" && isStrandedDispatchError(entry):
+		// ward#527 self-heal: re-queue a legacy dispatch-error stranding.
+		entry.State = "queued"
+		entry.LastOutcome = nil
 	}
 	entry.Num = rk.Num
 	entry.Title = rk.Title
@@ -929,13 +940,27 @@ func (r *Runner) backlogDispatchOne(ctx context.Context, label string, dispatch
 	return nil
 }
 
-// directorDispatchDisposition classifies a dispatch error for the ledger (ward#352): a
-// conflict defers (stays queued/eligible), any other error parks failed. Pure + testable.
+// directorDispatchDisposition classifies a dispatch error for the ledger (ward#352,
+// ward#524, ward#527). See docs/agent-director-dispatch.md.
 func directorDispatchDisposition(err error) (state string, outcome *backlogOutcome, deferred bool) {
-	if isReservationConflict(err) {
-		return "queued", &backlogOutcome{Status: "deferred", Text: backlogTruncate(err.Error(), 300)}, true
+	if isDispatchDecline(err) {
+		return "failed", &backlogOutcome{Status: "declined", Text: backlogTruncate(err.Error(), 300)}, false
+	}
+	return "queued", &backlogOutcome{Status: "deferred", Text: backlogTruncate(err.Error(), 300)}, true
+}
+
+// isDispatchDecline reports whether err is a coded per-issue pre-flight decline
+// (NO-GO / wrong-repo / untrusted-owner). See docs/agent-director-dispatch.md.
+func isDispatchDecline(err error) bool {
+	c := exitcode.From(err)
+	if c == nil {
+		return false
+	}
+	switch c.Code() {
+	case dispatchNoGo, dispatchWrongRepo, dispatchUntrustedOwner:
+		return true
 	}
-	return "failed", &backlogOutcome{Status: "dispatch-error", Text: backlogTruncate(err.Error(), 300)}, false
+	return false
 }
 
 // backlogDispatch launches one issue's headless run in-process via the engineer command
diff --git a/cmd/ward/agent_director_heartbeat_test.go b/cmd/ward/agent_director_heartbeat_test.go
index 7de5504..10fd2cb 100644
--- a/cmd/ward/agent_director_heartbeat_test.go
+++ b/cmd/ward/agent_director_heartbeat_test.go
@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"errors"
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -451,13 +452,13 @@ func TestRunDirectorLoopDefersReservationConflict(t *testing.T) {
 	}
 }
 
-// TestRunDirectorLoopParksRealFailure confirms the other half of ward#352: a real launch
-// failure still parks the issue failed (then the lane drains and surfaces).
-func TestRunDirectorLoopParksRealFailure(t *testing.T) {
-	issue := &backlogEntry{Num: 5, Title: "real failure", Tier: "P0", Lane: "headless", State: "queued"}
+// TestRunDirectorLoopParksDecline covers ward#524: a coded NO-GO decline parks the issue
+// failed, then the lane drains and surfaces. See docs/agent-director-dispatch.md.
+func TestRunDirectorLoopParksDecline(t *testing.T) {
+	issue := &backlogEntry{Num: 5, Title: "infeasible issue", Tier: "P0", Lane: "headless", State: "queued"}
 	d := &dispoDirector{
 		fakeDirector: &fakeDirector{list: []*backlogEntry{issue}},
-		errs:         map[int]error{5: errors.New("image pull failed")},
+		errs:         map[int]error{5: dispatchDeclineErr(dispatchNoGo, "preflight_no_go", "issue a/b#5 is infeasible")},
 	}
 	d.surfaceFn = func() (bool, error) { return false, nil } // non-interactive: drain exits cleanly
 	cfg := backlogConfig{maxParallel: 2, pollInterval: time.Millisecond, maxCycles: 5}
@@ -466,7 +467,28 @@ func TestRunDirectorLoopParksRealFailure(t *testing.T) {
 		t.Fatalf("loop returned error: %v", err)
 	}
 	if issue.State != "failed" {
-		t.Errorf("a genuine launch failure must park failed, got %q", issue.State)
+		t.Errorf("a coded per-issue decline must park failed, got %q", issue.State)
+	}
+}
+
+// TestRunDirectorLoopDefersInfraFailure covers ward#524: an uncoded infra failure leaves
+// the issue queued, never parked, never dispatched. See docs/agent-director-dispatch.md.
+func TestRunDirectorLoopDefersInfraFailure(t *testing.T) {
+	issue := &backlogEntry{Num: 5, Title: "fetch blipped", Tier: "P0", Lane: "headless", State: "queued"}
+	d := &dispoDirector{
+		fakeDirector: &fakeDirector{list: []*backlogEntry{issue}},
+		errs:         map[int]error{5: fmt.Errorf("forgejo: get issue a/b#5: %w", errors.New("502 bad gateway"))},
+	}
+	cfg := backlogConfig{maxParallel: 2, pollInterval: time.Millisecond, maxCycles: 2}
+
+	if err := runDirectorLoop(context.Background(), cfg, d); err != nil {
+		t.Fatalf("loop returned error: %v", err)
+	}
+	if issue.State != "queued" {
+		t.Errorf("an infra failure must stay queued for retry, got %q", issue.State)
+	}
+	if d.dispatched != nil {
+		t.Errorf("a deferred dispatch did not launch, nothing should count as dispatched, got %v", d.dispatched)
 	}
 }
 
diff --git a/cmd/ward/agent_director_test.go b/cmd/ward/agent_director_test.go
index 967c12f..08ade0d 100644
--- a/cmd/ward/agent_director_test.go
+++ b/cmd/ward/agent_director_test.go
@@ -2,6 +2,7 @@ package main
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -9,12 +10,14 @@ import (
 	"testing"
 	"time"
 
+	"forgejo.coilysiren.me/coilyco-flight-deck/cli-guard/pkg/exitcode"
 	"github.com/urfave/cli/v3"
 )
 
-// TestDirectorDispatchDisposition covers ward#352: a reservation conflict defers (stays
-// `queued`, flagged "deferred"); any other dispatch error parks `failed`.
+// TestDirectorDispatchDisposition covers ward#352 + ward#524: only a coded per-issue
+// decline parks `failed`; everything else defers. See docs/agent-director-dispatch.md.
 func TestDirectorDispatchDisposition(t *testing.T) {
+	// A reservation conflict defers (retryable once the holder finishes).
 	conflict := newReservationConflict("issue a/b#5 is already reserved remotely")
 	state, outcome, deferred := directorDispatchDisposition(conflict)
 	if !deferred {
@@ -27,15 +30,49 @@ func TestDirectorDispatchDisposition(t *testing.T) {
 		t.Errorf("deferred outcome = %+v, want status=deferred", outcome)
 	}
 
-	state, outcome, deferred = directorDispatchDisposition(errors.New("image pull failed"))
+	// A launch-time infra failure (the forgejo issue-fetch breakage) must defer too.
+	fetchErr := fmt.Errorf("forgejo: get issue a/b#5: %w", errors.New("502 bad gateway"))
+	state, outcome, deferred = directorDispatchDisposition(fetchErr)
+	if !deferred {
+		t.Error("a launch-time infra failure must defer and retry, not park failed")
+	}
+	if state != "queued" {
+		t.Errorf("infra-failure state = %q, want queued (retried on a later tick)", state)
+	}
+	if outcome == nil || outcome.Status != "deferred" {
+		t.Errorf("infra-failure outcome = %+v, want status=deferred", outcome)
+	}
+
+	// An uncoded generic launch failure defers on the same reasoning.
+	state, _, deferred = directorDispatchDisposition(errors.New("image pull failed"))
+	if !deferred || state != "queued" {
+		t.Errorf("generic launch failure: state=%q deferred=%v, want queued+deferred", state, deferred)
+	}
+
+	// A coded per-issue decline is a real verdict on the issue: park it terminal.
+	noGo := dispatchDeclineErr(dispatchNoGo, "preflight_no_go", "issue a/b#5 is infeasible")
+	state, outcome, deferred = directorDispatchDisposition(noGo)
 	if deferred {
-		t.Error("a genuine launch failure must not defer")
+		t.Error("a coded NO-GO decline is a per-issue verdict, it must not defer")
 	}
 	if state != "failed" {
-		t.Errorf("launch-failure state = %q, want failed", state)
+		t.Errorf("decline state = %q, want failed", state)
 	}
-	if outcome == nil || outcome.Status != "dispatch-error" {
-		t.Errorf("failure outcome = %+v, want status=dispatch-error", outcome)
+	if outcome == nil || outcome.Status != "declined" {
+		t.Errorf("decline outcome = %+v, want status=declined", outcome)
+	}
+
+	// Wrong-repo and untrusted-owner are likewise terminal per-issue/owner verdicts.
+	for _, tc := range []struct {
+		name string
+		err  error
+	}{
+		{"wrong-repo", dispatchDeclineErr(dispatchWrongRepo, "preflight_wrong_repo_routed", "routed to c/d")},
+		{"untrusted-owner", exitcode.New(dispatchUntrustedOwner, "untrusted_owner", errors.New("owner not trusted"), "")},
+	} {
+		if _, _, def := directorDispatchDisposition(tc.err); def {
+			t.Errorf("%s decline must park terminal, not defer", tc.name)
+		}
 	}
 }
 
@@ -404,12 +441,18 @@ func TestRefreshBacklogLedger(t *testing.T) {
 		"7": {Num: 7, Lane: "interactive", State: "surfaced"},
 		// a done issue that has since closed (absent from the live set) -> dropped
 		"9": {Num: 9, Lane: "headless", State: "done"},
+		// ward#527: a pre-#524 dispatch-error stranding -> re-queued, outcome cleared
+		"13": {Num: 13, Lane: "headless", State: "failed", LastOutcome: &backlogOutcome{Status: "dispatch-error", Text: "forgejo: get issue a/b#13: 502"}},
+		// a genuine per-issue decline (declined) must NOT be re-queued
+		"14": {Num: 14, Lane: "headless", State: "failed", LastOutcome: &backlogOutcome{Status: "declined", Text: "infeasible"}},
 	}}
 	ranked := rankBacklogIssues([]backlogIssue{
 		{Number: 5, Title: "five", Labels: []string{"P0", "headless"}},
 		{Number: 7, Title: "seven", Labels: []string{"P1", "headless"}}, // promoted to headless
 		{Number: 11, Title: "eleven", Labels: []string{"P2", "interactive"}},
 		{Number: 12, Title: "twelve", Labels: nil}, // untriaged
+		{Number: 13, Title: "thirteen", Labels: []string{"P0", "headless"}},
+		{Number: 14, Title: "fourteen", Labels: []string{"P0", "headless"}},
 	})
 	refreshBacklogLedger(led, ranked)
 
@@ -428,6 +471,12 @@ func TestRefreshBacklogLedger(t *testing.T) {
 	if e := led.Issues["12"]; e == nil || e.State != "skipped" {
 		t.Errorf("#12 new untriaged should be skipped, got %+v", e)
 	}
+	if e := led.Issues["13"]; e == nil || e.State != "queued" || e.LastOutcome != nil {
+		t.Errorf("#13 stranded dispatch-error should be re-queued with a cleared outcome, got %+v", e)
+	}
+	if e := led.Issues["14"]; e == nil || e.State != "failed" {
+		t.Errorf("#14 genuine decline must stay failed, got %+v", e)
+	}
 }
 
 func TestParseBacklogOutcome(t *testing.T) {
diff --git a/docs/agent-director-dispatch.md b/docs/agent-director-dispatch.md
new file mode 100644
index 0000000..bfb4b19
--- /dev/null
+++ b/docs/agent-director-dispatch.md
@@ -0,0 +1,37 @@
+# Director dispatch-error disposition
+
+How `ward agent director` classifies a **dispatch error** - the error the engineer command
+returns before a container detaches - decides whether the issue is retried or parked. See
+[agent-director.md](agent-director.md) for the heartbeat this feeds.
+
+## The rule (ward#352, ward#524)
+
+`directorDispatchDisposition` splits errors by whether they judged the **issue itself**:
+
+- **Coded per-issue decline** - a pre-flight **NO-GO**, a **wrong-repo** bounce, or an
+  **untrusted-owner** refusal (a `dispatchDeclineErr` / `exitcode.Coded` value). This is a
+  real verdict that a retry cannot change, so the issue parks terminal `failed` with outcome
+  status `declined`.
+- **Everything else defers** - left `queued`, retried on a later tick:
+  - a **reservation conflict** (another run holds the 2h-TTL reservation), retryable once the
+    holder finishes (`--force` reclaims a stale/foreign hold);
+  - a **launch-time infrastructure failure** - the pre-flight issue fetch, a network blip, a
+    container bring-up error. These never judged the issue and consumed no autonomous run.
+
+### Why launch/infra errors must not park
+
+The pre-flight issue fetch (`resolveAgentWork` -> `fetchIssue`) runs on the host before any
+container starts. A systemic failure there (a sour forge read path) would otherwise park
+every dispatched issue `failed` one by one. That terminal-`failed` streak is exactly the
+signal the director LLM reads to "hold this tick", so a transient outage wedged the whole
+backlog permanently - nothing re-tested the recovered path. Deferring self-heals: the moment
+the fetch path is healthy again, the next tick dispatches.
+
+## The self-heal migration (ward#527)
+
+The ward#524 rule is forward-looking - it does not un-strand issues the **old** classifier
+already parked. On refresh, `applyRankedBacklogEntry` re-queues any headless-lane entry that
+is `failed` with the legacy `dispatch-error` status (`isStrandedDispatchError`), clearing its
+outcome. Because ward#524 gives genuine declines the distinct `declined` status, a
+`dispatch-error` entry can only be a pre-ward#524 launch/infra stranding, so this recovers
+exactly those without ever re-queuing a real per-issue decline.
diff --git a/docs/agent-director.md b/docs/agent-director.md
index 19a4331..6ef28cb 100644
--- a/docs/agent-director.md
+++ b/docs/agent-director.md
@@ -66,10 +66,10 @@ falls back to the cwd origin, flags override. Host-owned, no default.
   reaches each engineer, the full set the surface. `--branch`/`--no-preflight`,
   `--watch`/`--detach` absent.
 
-## Reservation conflicts defer (ward#352)
+## Dispatch-error disposition (ward#352/#524/#527)
 
-A dispatch onto a held reservation (another run, 2h TTL) **defers** - left eligible, retried
-later; only a real launch error parks `failed`. `--force` reclaims a stale/foreign hold.
+Only a coded per-issue decline parks `failed`; a conflict or launch/infra failure defers
+and retries: [agent-director-dispatch.md](agent-director-dispatch.md).
 
 ## See also
 
diff --git a/scripts/lint_issue_refs.py b/scripts/lint_issue_refs.py
index 1131355..e93f929 100755
--- a/scripts/lint_issue_refs.py
+++ b/scripts/lint_issue_refs.py
@@ -83,6 +83,7 @@ CONTRIBUTOR_DOCS = {
     "docs/agent-advisor.md",
     "docs/agent-attribution.md",
     "docs/agent-director.md",
+    "docs/agent-director-dispatch.md",
     "docs/agent-engineer.md",
     "docs/agent-flags.md",
     "docs/agent-host-net.md",

## Validated patch (complete end-state over `origin/main`) Contains the ward#524 classification fix **and** this recovery. Gate-clean: `ward exec build`/`test`/`vet` green, `pre-commit run` green, no new lint. Apply from repo root with `git apply`; if ward#524 already landed, integrate `main` and reconcile to this end-state. ```diff diff --git a/cmd/ward/agent_director.go b/cmd/ward/agent_director.go index fe1f6ef..1261fe6 100644 --- a/cmd/ward/agent_director.go +++ b/cmd/ward/agent_director.go @@ -16,6 +16,7 @@ import ( "forgejo.coilysiren.me/coilyco-flight-deck/cli-guard/cli/verb" "forgejo.coilysiren.me/coilyco-flight-deck/cli-guard/pkg/config" + "forgejo.coilysiren.me/coilyco-flight-deck/cli-guard/pkg/exitcode" "github.com/urfave/cli/v3" "gopkg.in/yaml.v3" ) @@ -623,6 +624,12 @@ func backlogNewEntryState(lane string) string { } } +// isStrandedDispatchError reports whether e is a pre-ward#524 launch/infra stranding: +// parked `failed` with the "dispatch-error" status the old classifier stamped (ward#527). +func isStrandedDispatchError(e *backlogEntry) bool { + return e != nil && e.State == "failed" && e.LastOutcome != nil && e.LastOutcome.Status == "dispatch-error" +} + // applyRankedBacklogEntry upserts one ranked issue into the ledger, seeding a new // entry's state by lane and re-queuing one a re-triage promoted into headless. func applyRankedBacklogEntry(led *backlogLedger, rk rankedBacklogIssue) { @@ -635,6 +642,10 @@ func applyRankedBacklogEntry(led *backlogLedger, rk rankedBacklogIssue) { // A re-triage promoted this into headless from a non-in-flight holding // state: re-queue it rather than strand it. entry.State = "queued" + case rk.Lane == "headless" && isStrandedDispatchError(entry): + // ward#527 self-heal: re-queue a legacy dispatch-error stranding. + entry.State = "queued" + entry.LastOutcome = nil } entry.Num = rk.Num entry.Title = rk.Title @@ -929,13 +940,27 @@ func (r *Runner) backlogDispatchOne(ctx context.Context, label string, dispatch return nil } -// directorDispatchDisposition classifies a dispatch error for the ledger (ward#352): a -// conflict defers (stays queued/eligible), any other error parks failed. Pure + testable. +// directorDispatchDisposition classifies a dispatch error for the ledger (ward#352, +// ward#524, ward#527). See docs/agent-director-dispatch.md. func directorDispatchDisposition(err error) (state string, outcome *backlogOutcome, deferred bool) { - if isReservationConflict(err) { - return "queued", &backlogOutcome{Status: "deferred", Text: backlogTruncate(err.Error(), 300)}, true + if isDispatchDecline(err) { + return "failed", &backlogOutcome{Status: "declined", Text: backlogTruncate(err.Error(), 300)}, false + } + return "queued", &backlogOutcome{Status: "deferred", Text: backlogTruncate(err.Error(), 300)}, true +} + +// isDispatchDecline reports whether err is a coded per-issue pre-flight decline +// (NO-GO / wrong-repo / untrusted-owner). See docs/agent-director-dispatch.md. +func isDispatchDecline(err error) bool { + c := exitcode.From(err) + if c == nil { + return false + } + switch c.Code() { + case dispatchNoGo, dispatchWrongRepo, dispatchUntrustedOwner: + return true } - return "failed", &backlogOutcome{Status: "dispatch-error", Text: backlogTruncate(err.Error(), 300)}, false + return false } // backlogDispatch launches one issue's headless run in-process via the engineer command diff --git a/cmd/ward/agent_director_heartbeat_test.go b/cmd/ward/agent_director_heartbeat_test.go index 7de5504..10fd2cb 100644 --- a/cmd/ward/agent_director_heartbeat_test.go +++ b/cmd/ward/agent_director_heartbeat_test.go @@ -3,6 +3,7 @@ package main import ( "context" "errors" + "fmt" "reflect" "strings" "testing" @@ -451,13 +452,13 @@ func TestRunDirectorLoopDefersReservationConflict(t *testing.T) { } } -// TestRunDirectorLoopParksRealFailure confirms the other half of ward#352: a real launch -// failure still parks the issue failed (then the lane drains and surfaces). -func TestRunDirectorLoopParksRealFailure(t *testing.T) { - issue := &backlogEntry{Num: 5, Title: "real failure", Tier: "P0", Lane: "headless", State: "queued"} +// TestRunDirectorLoopParksDecline covers ward#524: a coded NO-GO decline parks the issue +// failed, then the lane drains and surfaces. See docs/agent-director-dispatch.md. +func TestRunDirectorLoopParksDecline(t *testing.T) { + issue := &backlogEntry{Num: 5, Title: "infeasible issue", Tier: "P0", Lane: "headless", State: "queued"} d := &dispoDirector{ fakeDirector: &fakeDirector{list: []*backlogEntry{issue}}, - errs: map[int]error{5: errors.New("image pull failed")}, + errs: map[int]error{5: dispatchDeclineErr(dispatchNoGo, "preflight_no_go", "issue a/b#5 is infeasible")}, } d.surfaceFn = func() (bool, error) { return false, nil } // non-interactive: drain exits cleanly cfg := backlogConfig{maxParallel: 2, pollInterval: time.Millisecond, maxCycles: 5} @@ -466,7 +467,28 @@ func TestRunDirectorLoopParksRealFailure(t *testing.T) { t.Fatalf("loop returned error: %v", err) } if issue.State != "failed" { - t.Errorf("a genuine launch failure must park failed, got %q", issue.State) + t.Errorf("a coded per-issue decline must park failed, got %q", issue.State) + } +} + +// TestRunDirectorLoopDefersInfraFailure covers ward#524: an uncoded infra failure leaves +// the issue queued, never parked, never dispatched. See docs/agent-director-dispatch.md. +func TestRunDirectorLoopDefersInfraFailure(t *testing.T) { + issue := &backlogEntry{Num: 5, Title: "fetch blipped", Tier: "P0", Lane: "headless", State: "queued"} + d := &dispoDirector{ + fakeDirector: &fakeDirector{list: []*backlogEntry{issue}}, + errs: map[int]error{5: fmt.Errorf("forgejo: get issue a/b#5: %w", errors.New("502 bad gateway"))}, + } + cfg := backlogConfig{maxParallel: 2, pollInterval: time.Millisecond, maxCycles: 2} + + if err := runDirectorLoop(context.Background(), cfg, d); err != nil { + t.Fatalf("loop returned error: %v", err) + } + if issue.State != "queued" { + t.Errorf("an infra failure must stay queued for retry, got %q", issue.State) + } + if d.dispatched != nil { + t.Errorf("a deferred dispatch did not launch, nothing should count as dispatched, got %v", d.dispatched) } } diff --git a/cmd/ward/agent_director_test.go b/cmd/ward/agent_director_test.go index 967c12f..08ade0d 100644 --- a/cmd/ward/agent_director_test.go +++ b/cmd/ward/agent_director_test.go @@ -2,6 +2,7 @@ package main import ( "errors" + "fmt" "os" "path/filepath" "reflect" @@ -9,12 +10,14 @@ import ( "testing" "time" + "forgejo.coilysiren.me/coilyco-flight-deck/cli-guard/pkg/exitcode" "github.com/urfave/cli/v3" ) -// TestDirectorDispatchDisposition covers ward#352: a reservation conflict defers (stays -// `queued`, flagged "deferred"); any other dispatch error parks `failed`. +// TestDirectorDispatchDisposition covers ward#352 + ward#524: only a coded per-issue +// decline parks `failed`; everything else defers. See docs/agent-director-dispatch.md. func TestDirectorDispatchDisposition(t *testing.T) { + // A reservation conflict defers (retryable once the holder finishes). conflict := newReservationConflict("issue a/b#5 is already reserved remotely") state, outcome, deferred := directorDispatchDisposition(conflict) if !deferred { @@ -27,15 +30,49 @@ func TestDirectorDispatchDisposition(t *testing.T) { t.Errorf("deferred outcome = %+v, want status=deferred", outcome) } - state, outcome, deferred = directorDispatchDisposition(errors.New("image pull failed")) + // A launch-time infra failure (the forgejo issue-fetch breakage) must defer too. + fetchErr := fmt.Errorf("forgejo: get issue a/b#5: %w", errors.New("502 bad gateway")) + state, outcome, deferred = directorDispatchDisposition(fetchErr) + if !deferred { + t.Error("a launch-time infra failure must defer and retry, not park failed") + } + if state != "queued" { + t.Errorf("infra-failure state = %q, want queued (retried on a later tick)", state) + } + if outcome == nil || outcome.Status != "deferred" { + t.Errorf("infra-failure outcome = %+v, want status=deferred", outcome) + } + + // An uncoded generic launch failure defers on the same reasoning. + state, _, deferred = directorDispatchDisposition(errors.New("image pull failed")) + if !deferred || state != "queued" { + t.Errorf("generic launch failure: state=%q deferred=%v, want queued+deferred", state, deferred) + } + + // A coded per-issue decline is a real verdict on the issue: park it terminal. + noGo := dispatchDeclineErr(dispatchNoGo, "preflight_no_go", "issue a/b#5 is infeasible") + state, outcome, deferred = directorDispatchDisposition(noGo) if deferred { - t.Error("a genuine launch failure must not defer") + t.Error("a coded NO-GO decline is a per-issue verdict, it must not defer") } if state != "failed" { - t.Errorf("launch-failure state = %q, want failed", state) + t.Errorf("decline state = %q, want failed", state) } - if outcome == nil || outcome.Status != "dispatch-error" { - t.Errorf("failure outcome = %+v, want status=dispatch-error", outcome) + if outcome == nil || outcome.Status != "declined" { + t.Errorf("decline outcome = %+v, want status=declined", outcome) + } + + // Wrong-repo and untrusted-owner are likewise terminal per-issue/owner verdicts. + for _, tc := range []struct { + name string + err error + }{ + {"wrong-repo", dispatchDeclineErr(dispatchWrongRepo, "preflight_wrong_repo_routed", "routed to c/d")}, + {"untrusted-owner", exitcode.New(dispatchUntrustedOwner, "untrusted_owner", errors.New("owner not trusted"), "")}, + } { + if _, _, def := directorDispatchDisposition(tc.err); def { + t.Errorf("%s decline must park terminal, not defer", tc.name) + } } } @@ -404,12 +441,18 @@ func TestRefreshBacklogLedger(t *testing.T) { "7": {Num: 7, Lane: "interactive", State: "surfaced"}, // a done issue that has since closed (absent from the live set) -> dropped "9": {Num: 9, Lane: "headless", State: "done"}, + // ward#527: a pre-#524 dispatch-error stranding -> re-queued, outcome cleared + "13": {Num: 13, Lane: "headless", State: "failed", LastOutcome: &backlogOutcome{Status: "dispatch-error", Text: "forgejo: get issue a/b#13: 502"}}, + // a genuine per-issue decline (declined) must NOT be re-queued + "14": {Num: 14, Lane: "headless", State: "failed", LastOutcome: &backlogOutcome{Status: "declined", Text: "infeasible"}}, }} ranked := rankBacklogIssues([]backlogIssue{ {Number: 5, Title: "five", Labels: []string{"P0", "headless"}}, {Number: 7, Title: "seven", Labels: []string{"P1", "headless"}}, // promoted to headless {Number: 11, Title: "eleven", Labels: []string{"P2", "interactive"}}, {Number: 12, Title: "twelve", Labels: nil}, // untriaged + {Number: 13, Title: "thirteen", Labels: []string{"P0", "headless"}}, + {Number: 14, Title: "fourteen", Labels: []string{"P0", "headless"}}, }) refreshBacklogLedger(led, ranked) @@ -428,6 +471,12 @@ func TestRefreshBacklogLedger(t *testing.T) { if e := led.Issues["12"]; e == nil || e.State != "skipped" { t.Errorf("#12 new untriaged should be skipped, got %+v", e) } + if e := led.Issues["13"]; e == nil || e.State != "queued" || e.LastOutcome != nil { + t.Errorf("#13 stranded dispatch-error should be re-queued with a cleared outcome, got %+v", e) + } + if e := led.Issues["14"]; e == nil || e.State != "failed" { + t.Errorf("#14 genuine decline must stay failed, got %+v", e) + } } func TestParseBacklogOutcome(t *testing.T) { diff --git a/docs/agent-director-dispatch.md b/docs/agent-director-dispatch.md new file mode 100644 index 0000000..bfb4b19 --- /dev/null +++ b/docs/agent-director-dispatch.md @@ -0,0 +1,37 @@ +# Director dispatch-error disposition + +How `ward agent director` classifies a **dispatch error** - the error the engineer command +returns before a container detaches - decides whether the issue is retried or parked. See +[agent-director.md](agent-director.md) for the heartbeat this feeds. + +## The rule (ward#352, ward#524) + +`directorDispatchDisposition` splits errors by whether they judged the **issue itself**: + +- **Coded per-issue decline** - a pre-flight **NO-GO**, a **wrong-repo** bounce, or an + **untrusted-owner** refusal (a `dispatchDeclineErr` / `exitcode.Coded` value). This is a + real verdict that a retry cannot change, so the issue parks terminal `failed` with outcome + status `declined`. +- **Everything else defers** - left `queued`, retried on a later tick: + - a **reservation conflict** (another run holds the 2h-TTL reservation), retryable once the + holder finishes (`--force` reclaims a stale/foreign hold); + - a **launch-time infrastructure failure** - the pre-flight issue fetch, a network blip, a + container bring-up error. These never judged the issue and consumed no autonomous run. + +### Why launch/infra errors must not park + +The pre-flight issue fetch (`resolveAgentWork` -> `fetchIssue`) runs on the host before any +container starts. A systemic failure there (a sour forge read path) would otherwise park +every dispatched issue `failed` one by one. That terminal-`failed` streak is exactly the +signal the director LLM reads to "hold this tick", so a transient outage wedged the whole +backlog permanently - nothing re-tested the recovered path. Deferring self-heals: the moment +the fetch path is healthy again, the next tick dispatches. + +## The self-heal migration (ward#527) + +The ward#524 rule is forward-looking - it does not un-strand issues the **old** classifier +already parked. On refresh, `applyRankedBacklogEntry` re-queues any headless-lane entry that +is `failed` with the legacy `dispatch-error` status (`isStrandedDispatchError`), clearing its +outcome. Because ward#524 gives genuine declines the distinct `declined` status, a +`dispatch-error` entry can only be a pre-ward#524 launch/infra stranding, so this recovers +exactly those without ever re-queuing a real per-issue decline. diff --git a/docs/agent-director.md b/docs/agent-director.md index 19a4331..6ef28cb 100644 --- a/docs/agent-director.md +++ b/docs/agent-director.md @@ -66,10 +66,10 @@ falls back to the cwd origin, flags override. Host-owned, no default. reaches each engineer, the full set the surface. `--branch`/`--no-preflight`, `--watch`/`--detach` absent. -## Reservation conflicts defer (ward#352) +## Dispatch-error disposition (ward#352/#524/#527) -A dispatch onto a held reservation (another run, 2h TTL) **defers** - left eligible, retried -later; only a real launch error parks `failed`. `--force` reclaims a stale/foreign hold. +Only a coded per-issue decline parks `failed`; a conflict or launch/infra failure defers +and retries: [agent-director-dispatch.md](agent-director-dispatch.md). ## See also diff --git a/scripts/lint_issue_refs.py b/scripts/lint_issue_refs.py index 1131355..e93f929 100755 --- a/scripts/lint_issue_refs.py +++ b/scripts/lint_issue_refs.py @@ -83,6 +83,7 @@ CONTRIBUTOR_DOCS = { "docs/agent-advisor.md", "docs/agent-attribution.md", "docs/agent-director.md", + "docs/agent-director-dispatch.md", "docs/agent-engineer.md", "docs/agent-flags.md", "docs/agent-host-net.md", ```

coilyco-ops commented

2026-07-02 08:58:46 +00:00

Member

🔒 Reserved by ward agent --driver claude — container engineer-claude-ward-527 on host kais-macbook-pro-2.local is carrying this issue (reserved 2026-07-02T08:58:43Z). Concurrent ward agent runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); --force overrides.

— Claude (she/her), via ward agent

🔒 Reserved by `ward agent --driver claude` — container `engineer-claude-ward-527` on host `kais-macbook-pro-2.local` is carrying this issue (reserved 2026-07-02T08:58:43Z). Concurrent `ward agent` runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); `--force` overrides.  — Claude (she/her), via `ward agent`

coilyco-ops added the

consult

labels

2026-07-02 09:02:27 +00:00

coilysiren referenced this issue from a commit

2026-07-02 09:03:27 +00:00

fix(director): self-heal legacy dispatch-error strandings + give declines a distinct status

coilysiren closed this issue

2026-07-02 09:03:27 +00:00

coilysiren commented

2026-07-02 09:03:54 +00:00

Author

Owner

WARD-OUTCOME: done - declined status split out from dispatch-error, plus a self-limiting refresh-path migration that re-queues the four legacy strandings on the director's next tick.

This one went smoothly, mostly because ward#524 had already landed on main, so the reconcile was clean: the real net delta was isStrandedDispatchError + the migration case, flipping the decline outcome from dispatch-error to declined, and moving the ward#524 prose out into docs/agent-director-dispatch.md behind a pointer. What fought back a little was the 4000-char cap on docs/agent-director.md - it sat at 3995 before the edit, so the pointer-section swap had to be a net shrink to stay legal (it dropped to 3771). Nothing surprising in the logic; the two-axis test in TestRefreshBacklogLedger (#13 re-queues, #14 stays failed) makes the self-limiting property concrete.

Confidence is high: build/test/vet green, full pre-commit run --all-files green, the migration is provably scoped to the pre-ward#524 failed+dispatch-error combination which nothing new produces. The one thing this repo change cannot do is the actual un-wedge - that happens on the host when the director next ticks with this code against ~/.ward/backlog/*.yaml. Worth a follow-up glance at the live backlog after the next director run to confirm #470/#481/#482/#441 actually returned to queued.

WARD-OUTCOME: done - `declined` status split out from `dispatch-error`, plus a self-limiting refresh-path migration that re-queues the four legacy strandings on the director's next tick. This one went smoothly, mostly because ward#524 had already landed on `main`, so the reconcile was clean: the real net delta was `isStrandedDispatchError` + the migration case, flipping the decline outcome from `dispatch-error` to `declined`, and moving the ward#524 prose out into `docs/agent-director-dispatch.md` behind a pointer. What fought back a little was the 4000-char cap on `docs/agent-director.md` - it sat at 3995 before the edit, so the pointer-section swap had to be a net shrink to stay legal (it dropped to 3771). Nothing surprising in the logic; the two-axis test in `TestRefreshBacklogLedger` (#13 re-queues, #14 stays failed) makes the self-limiting property concrete. Confidence is high: build/test/vet green, full `pre-commit run --all-files` green, the migration is provably scoped to the pre-ward#524 `failed`+`dispatch-error` combination which nothing new produces. The one thing this repo change *cannot* do is the actual un-wedge - that happens on the host when the director next ticks with this code against `~/.ward/backlog/*.yaml`. Worth a follow-up glance at the live backlog after the next director run to confirm #470/#481/#482/#441 actually returned to `queued`.