coilyco-flight-deck/ward

Fork 0

fix(director): defer dispatch-time infra failures instead of parking them failed (backlog wedge) #524

New issue

Closed

opened 2026-07-02 08:46:15 +00:00 by coilysiren · 4 comments

coilysiren commented

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

Owner

Symptom

ward agent director wedged its managed backlog flow: the heartbeat kept choosing DISPATCH: none tick after tick, refusing to dispatch any of 29 queued issues, citing that the last four dispatched runs (#470, #481, #482, #441) "all failed identically at the forgejo: get issue step."

The forgejo issue-fetch path was only transiently unhealthy (it reads fine now, exit 0), yet the director never recovered on its own.

Root cause

A dispatch-time infrastructure failure is misclassified as a terminal per-issue failure, and that turns a transient forge blip into a permanent wedge.

directorDispatchDisposition (cmd/ward/agent_director.go) classifies every dispatch error except a reservation conflict as terminal failed:

if isReservationConflict(err) {
    return "queued", ...deferred..., true
}
return "failed", &backlogOutcome{Status: "dispatch-error", ...}, false

The forgejo: get issue … failure comes from the pre-flight issue fetch (resolveAgentWork -> fetchIssue -> getIssue), which runs before any container launches. It reflects the forge/auth/network, not the issue, and consumes no autonomous run. But it still parks the queued issue terminal failed.
A failed entry is never re-queued: applyRankedBacklogEntry re-queues only skipped/surfaced, and dropClosedBacklogEntries keeps failed. So each stranded issue is stuck permanently.
The director LLM reads the RECENT OUTCOMES failing-streak (directorOutcomeLines -> directorDecidePrompt) and reasonably decides to hold. Because failed is terminal and nothing re-tests the fetch path, the streak never clears - the whole backlog wedges even after the forge recovers.

Fix

Only a coded per-issue decline - a pre-flight NO-GO (dispatchNoGo), a wrong-repo bounce (dispatchWrongRepo), or an untrusted-owner refusal (dispatchUntrustedOwner) - is a real verdict on the issue and stays terminal failed. Everything else (a reservation conflict, or a launch-time infra/fetch/bring-up failure) defers: it stays queued and retries on a later tick. This generalizes the existing reservation-conflict defer and makes the flow self-heal the moment the fetch path is healthy again.

A validated patch (builds, ward exec test green, vet clean, no new lint) is attached in the first comment below. It touches:

cmd/ward/agent_director.go - new isDispatchDecline helper (via exitcode.From), rewritten directorDispatchDisposition, exitcode import.
cmd/ward/agent_director_test.go - TestDirectorDispatchDisposition covers decline-parks vs infra-defers vs conflict-defers.
cmd/ward/agent_director_heartbeat_test.go - TestRunDirectorLoopParksDecline + new TestRunDirectorLoopDefersInfraFailure.
docs/agent-director.md - the "Dispatch errors defer, only per-issue declines park" section.

Engineer note: the attached patch stamps the placeholder ward#521 in comments/docs. Replace it with this issue's actual number before committing.

Not in scope (follow-up)

The four already-stranded ledger entries (#470/#481/#482/#441) live in the host's ~/.ward/backlog/*.yaml, parked failed by the old classification. This fix is forward-looking - future infra blips defer instead of parking - but does not retroactively re-queue entries the old code already stranded. Recovering those is a host-side operation (re-queue the four, or add a one-time migration that re-queues failed entries whose LastOutcome.Status == "dispatch-error"). File a follow-up if the live backlog needs the retroactive un-wedge.

## Symptom `ward agent director` wedged its managed backlog flow: the heartbeat kept choosing **DISPATCH: none** tick after tick, refusing to dispatch any of 29 queued issues, citing that the last four dispatched runs (#470, #481, #482, #441) "all failed identically at the `forgejo: get issue` step." The forgejo issue-fetch path was only **transiently** unhealthy (it reads fine now, exit 0), yet the director never recovered on its own. ## Root cause A dispatch-time infrastructure failure is misclassified as a terminal per-issue failure, and that turns a transient forge blip into a permanent wedge. 1. `directorDispatchDisposition` (cmd/ward/agent_director.go) classifies **every** dispatch error except a reservation conflict as terminal `failed`: ```go if isReservationConflict(err) { return "queued", ...deferred..., true } return "failed", &backlogOutcome{Status: "dispatch-error", ...}, false ``` 2. The `forgejo: get issue …` failure comes from the **pre-flight issue fetch** (`resolveAgentWork` -> `fetchIssue` -> `getIssue`), which runs *before* any container launches. It reflects the forge/auth/network, not the issue, and consumes **no** autonomous run. But it still parks the queued issue terminal `failed`. 3. A `failed` entry is never re-queued: `applyRankedBacklogEntry` re-queues only `skipped`/`surfaced`, and `dropClosedBacklogEntries` keeps `failed`. So each stranded issue is stuck permanently. 4. The director LLM reads the RECENT OUTCOMES failing-streak (`directorOutcomeLines` -> `directorDecidePrompt`) and reasonably decides to **hold**. Because `failed` is terminal and nothing re-tests the fetch path, the streak never clears - the whole backlog wedges even after the forge recovers. ## Fix Only a **coded per-issue decline** - a pre-flight NO-GO (`dispatchNoGo`), a wrong-repo bounce (`dispatchWrongRepo`), or an untrusted-owner refusal (`dispatchUntrustedOwner`) - is a real verdict on the issue and stays terminal `failed`. Everything else (a reservation conflict, or a launch-time infra/fetch/bring-up failure) **defers**: it stays `queued` and retries on a later tick. This generalizes the existing reservation-conflict defer and makes the flow self-heal the moment the fetch path is healthy again. A validated patch (builds, `ward exec test` green, vet clean, no new lint) is attached in the first comment below. It touches: - `cmd/ward/agent_director.go` - new `isDispatchDecline` helper (via `exitcode.From`), rewritten `directorDispatchDisposition`, `exitcode` import. - `cmd/ward/agent_director_test.go` - `TestDirectorDispatchDisposition` covers decline-parks vs infra-defers vs conflict-defers. - `cmd/ward/agent_director_heartbeat_test.go` - `TestRunDirectorLoopParksDecline` + new `TestRunDirectorLoopDefersInfraFailure`. - `docs/agent-director.md` - the "Dispatch errors defer, only per-issue declines park" section. **Engineer note:** the attached patch stamps the placeholder `ward#521` in comments/docs. Replace it with **this issue's actual number** before committing. ## Not in scope (follow-up) The four already-stranded ledger entries (#470/#481/#482/#441) live in the host's `~/.ward/backlog/*.yaml`, parked `failed` by the old classification. This fix is forward-looking - future infra blips defer instead of parking - but does **not** retroactively re-queue entries the old code already stranded. Recovering those is a host-side operation (re-queue the four, or add a one-time migration that re-queues `failed` entries whose `LastOutcome.Status == "dispatch-error"`). File a follow-up if the live backlog needs the retroactive un-wedge.

coilysiren commented

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

Author

Owner

Validated patch

Apply from repo root with git apply. Builds clean, ward exec test green, ward exec vet clean, no new lint findings.

diff --git a/cmd/ward/agent_director.go b/cmd/ward/agent_director.go
index fe1f6ef..5998bba 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"
 )
@@ -929,13 +930,36 @@ 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). Only a coded per-issue decline (NO-GO / wrong-repo / untrusted-owner) is a
+// real verdict on the issue, so it parks terminal `failed`. Everything else - a
+// reservation conflict, or a launch-time infrastructure failure (issue fetch, network,
+// container bring-up) - never judged the issue and consumed no autonomous run, so it
+// stays `queued` and retries on a later tick. Parking such an error terminal stranded a
+// good issue and fed the director a false failing-streak that wedged the whole backlog
+// when a forge read path went transiently sour. Pure + testable.
 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: "dispatch-error", Text: backlogTruncate(err.Error(), 300)}, false
 	}
-	return "failed", &backlogOutcome{Status: "dispatch-error", 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 - a
+// verdict on the issue itself (NO-GO, wrong-repo) or its owner (untrusted-owner) that
+// retrying cannot change. A reservation conflict is deliberately excluded: it is a
+// "someone else got there first", retryable once the holder finishes. Every other error
+// (uncoded launch/infra failure, generic launch failure) is likewise retryable.
+func isDispatchDecline(err error) bool {
+	c := exitcode.From(err)
+	if c == nil {
+		return false
+	}
+	switch c.Code() {
+	case dispatchNoGo, dispatchWrongRepo, dispatchUntrustedOwner:
+		return true
+	}
+	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..a9f63e1 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,14 @@ 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 confirms the other half of ward#352 + ward#524: a coded
+// per-issue decline (here a pre-flight NO-GO) parks the issue failed (then the lane drains
+// and surfaces). An uncoded infra failure no longer parks - it defers, covered separately.
+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 +468,29 @@ 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 launch-time infra
+// failure (the forgejo issue-fetch breakage that wedged the director) leaves the issue
+// queued/eligible for a later tick, never parked failed, never counted as dispatched.
+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..df19f53 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,17 @@ 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: a coded per-issue decline
+// (NO-GO / wrong-repo / untrusted-owner) parks `failed`; a reservation conflict OR a
+// launch-time infrastructure failure (issue fetch, network, container bring-up) stays
+// `queued` and retries on a later tick rather than stranding the issue and wedging the
+// backlog with a false failing-streak.
 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 +33,50 @@ 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 infrastructure failure (the forgejo issue-fetch breakage that wedged
+	// the director) must defer too - the issue was never judged and no run was spent.
+	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)
+		t.Errorf("decline outcome = %+v, want status=dispatch-error", 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)
+		}
 	}
 }
 
diff --git a/docs/agent-director.md b/docs/agent-director.md
index 19a4331..265c8ab 100644
--- a/docs/agent-director.md
+++ b/docs/agent-director.md
@@ -66,10 +66,23 @@ 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)
-
-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.
+## Dispatch errors defer, only per-issue declines park (ward#352, ward#524)
+
+Dispatch classifies a launch error by whether it judged the **issue**. Only a coded
+per-issue decline - a pre-flight **NO-GO**, a **wrong-repo** bounce, or an
+**untrusted-owner** refusal - parks the issue `failed`: that verdict will not change on a
+retry. Everything else **defers** - left eligible, 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.
+
+This matters because a systemic infra failure (a sour forge read path) would otherwise park
+every dispatched issue `failed` one by one, and 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, since nothing re-tested the recovered path. Deferring instead makes the
+flow self-heal: the moment the fetch path is healthy again, the next tick dispatches.
 
 ## See also

## Validated patch Apply from repo root with `git apply`. Builds clean, `ward exec test` green, `ward exec vet` clean, no new lint findings. ```diff diff --git a/cmd/ward/agent_director.go b/cmd/ward/agent_director.go index fe1f6ef..5998bba 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" ) @@ -929,13 +930,36 @@ 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). Only a coded per-issue decline (NO-GO / wrong-repo / untrusted-owner) is a +// real verdict on the issue, so it parks terminal `failed`. Everything else - a +// reservation conflict, or a launch-time infrastructure failure (issue fetch, network, +// container bring-up) - never judged the issue and consumed no autonomous run, so it +// stays `queued` and retries on a later tick. Parking such an error terminal stranded a +// good issue and fed the director a false failing-streak that wedged the whole backlog +// when a forge read path went transiently sour. Pure + testable. 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: "dispatch-error", Text: backlogTruncate(err.Error(), 300)}, false } - return "failed", &backlogOutcome{Status: "dispatch-error", 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 - a +// verdict on the issue itself (NO-GO, wrong-repo) or its owner (untrusted-owner) that +// retrying cannot change. A reservation conflict is deliberately excluded: it is a +// "someone else got there first", retryable once the holder finishes. Every other error +// (uncoded launch/infra failure, generic launch failure) is likewise retryable. +func isDispatchDecline(err error) bool { + c := exitcode.From(err) + if c == nil { + return false + } + switch c.Code() { + case dispatchNoGo, dispatchWrongRepo, dispatchUntrustedOwner: + return true + } + 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..a9f63e1 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,14 @@ 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 confirms the other half of ward#352 + ward#524: a coded +// per-issue decline (here a pre-flight NO-GO) parks the issue failed (then the lane drains +// and surfaces). An uncoded infra failure no longer parks - it defers, covered separately. +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 +468,29 @@ 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 launch-time infra +// failure (the forgejo issue-fetch breakage that wedged the director) leaves the issue +// queued/eligible for a later tick, never parked failed, never counted as dispatched. +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..df19f53 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,17 @@ 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: a coded per-issue decline +// (NO-GO / wrong-repo / untrusted-owner) parks `failed`; a reservation conflict OR a +// launch-time infrastructure failure (issue fetch, network, container bring-up) stays +// `queued` and retries on a later tick rather than stranding the issue and wedging the +// backlog with a false failing-streak. 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 +33,50 @@ 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 infrastructure failure (the forgejo issue-fetch breakage that wedged + // the director) must defer too - the issue was never judged and no run was spent. + 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) + t.Errorf("decline outcome = %+v, want status=dispatch-error", 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) + } } } diff --git a/docs/agent-director.md b/docs/agent-director.md index 19a4331..265c8ab 100644 --- a/docs/agent-director.md +++ b/docs/agent-director.md @@ -66,10 +66,23 @@ 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) - -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. +## Dispatch errors defer, only per-issue declines park (ward#352, ward#524) + +Dispatch classifies a launch error by whether it judged the **issue**. Only a coded +per-issue decline - a pre-flight **NO-GO**, a **wrong-repo** bounce, or an +**untrusted-owner** refusal - parks the issue `failed`: that verdict will not change on a +retry. Everything else **defers** - left eligible, 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. + +This matters because a systemic infra failure (a sour forge read path) would otherwise park +every dispatched issue `failed` one by one, and 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, since nothing re-tested the recovered path. Deferring instead makes the +flow self-heal: the moment the fetch path is healthy again, the next tick dispatches. ## See also ```

coilyco-ops commented

2026-07-02 08:47:08 +00:00

Member

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

coilysiren referenced this issue from a commit

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

fix(director): defer dispatch-time infra failures instead of parking them failed

coilysiren closed this issue

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

coilysiren commented

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

Author

Owner

WARD-OUTCOME: done - infra/fetch dispatch failures now defer (stay queued) instead of parking failed; only coded per-issue declines stay terminal. Landed on main as 434714a.

Honestly this one went smoothly on the substance and fought me entirely on the packaging. The attached patch was already correct and already stamped ward#524 (the "replace ward#521 placeholder" note was stale), so the core change - flip the default to defer and gate failed behind a coded isDispatchDecline via exitcode.From - dropped in cleanly and all three unit tests plus the two loop tests went green first try. The inversion reads well: the old code was "conflict defers, else fail", the new code is "decline fails, else defer", which is a nicer shape.

What actually ate the time was the doc caps. docs/agent-director.md was pinned at 3999/4000 chars and 78/80 lines - deliberately maxed - so my new "Dispatch defers, only per-issue declines park" section blew straight through both the char cap and the 90-char/2-line code-comment discipline. The verbose comments in the "validated" patch would have failed the code-comments hook too. I ended up condensing the code comments hard and doing a round of lossless trimming across several existing sections to reclaim ~250 chars of budget. No behavior touched, but it's the kind of thing that turns a 5-minute change into a 30-minute one.

Confidence is high: pure, well-covered logic, and exitcode.From already recovers the reservation-conflict Coded error so the classification is consistent across all the dispatch exit codes. One rough edge worth a follow-up, already flagged in the issue's own not-in-scope: the four already-stranded ledger entries (#470/#481/#482/#441) are still parked failed in the host's ~/.ward/backlog/*.yaml - this fix is forward-looking and won't retroactively un-wedge them. A one-time host-side re-queue (or a small migration for failed entries whose LastOutcome.Status == "dispatch-error") is the clean way to recover those; worth its own issue if the live backlog still needs it.

WARD-OUTCOME: done - infra/fetch dispatch failures now defer (stay queued) instead of parking failed; only coded per-issue declines stay terminal. Landed on main as 434714a. Honestly this one went smoothly on the substance and fought me entirely on the packaging. The attached patch was already correct and already stamped ward#524 (the "replace ward#521 placeholder" note was stale), so the core change - flip the default to defer and gate `failed` behind a coded `isDispatchDecline` via `exitcode.From` - dropped in cleanly and all three unit tests plus the two loop tests went green first try. The inversion reads well: the old code was "conflict defers, else fail", the new code is "decline fails, else defer", which is a nicer shape. What actually ate the time was the doc caps. `docs/agent-director.md` was pinned at 3999/4000 chars and 78/80 lines - deliberately maxed - so my new "Dispatch defers, only per-issue declines park" section blew straight through both the char cap and the 90-char/2-line code-comment discipline. The verbose comments in the "validated" patch would have failed the `code-comments` hook too. I ended up condensing the code comments hard and doing a round of lossless trimming across several existing sections to reclaim ~250 chars of budget. No behavior touched, but it's the kind of thing that turns a 5-minute change into a 30-minute one. Confidence is high: pure, well-covered logic, and `exitcode.From` already recovers the reservation-conflict Coded error so the classification is consistent across all the dispatch exit codes. One rough edge worth a follow-up, already flagged in the issue's own not-in-scope: the four already-stranded ledger entries (#470/#481/#482/#441) are still parked `failed` in the host's `~/.ward/backlog/*.yaml` - this fix is forward-looking and won't retroactively un-wedge them. A one-time host-side re-queue (or a small migration for `failed` entries whose `LastOutcome.Status == "dispatch-error"`) is the clean way to recover those; worth its own issue if the live backlog still needs it.

coilysiren commented

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

Author

Owner

The follow-up ward#527 carries a gate-clean patch that is the complete end-state (this ward#524 fix + the recovery). If ward#527 lands first, this is subsumed; if this lands first, ward#527 reconciles onto it. Either way the target is the end-state in ward#527 issuecomment-20569.

Heads-up: the patch above trips the catalog pre-commit caps (code-comments: 2-line contiguous blocks, 90-char comment lines; documentation-layout: 80-line/4000-char doc cap). Keep code comments short and move the durable rationale into a docs file - `docs/agent-director.md` is already at the char cap, so add a small pointer section there and put the detail in a new `docs/agent-director-dispatch.md`. The follow-up ward#527 carries a gate-clean patch that is the **complete end-state** (this ward#524 fix + the recovery). If ward#527 lands first, this is subsumed; if this lands first, ward#527 reconciles onto it. Either way the target is the end-state in ward#527 issuecomment-20569.