fix(delivery): preserve assignee on failed result so retries stay routable

[bborbe]

Resolve the retry-vs-escalate contradiction in the result deliverer.

Bug: a failed result cleared assignee while the comment promised retry via trigger_count / max_triggers. Clearing the assignee made the task unroutable (the executor skips empty-assignee tasks), so the retry silently never fired — skipped_trigger_cap stayed 0 while 22 tasks sat at trigger_count 2–4 (the 2026-08-10 silent job-failure wedge in agent-task-executor).

Fix (matches spec 010/021/027):

• failed (infra/transient: crash, timeout, OOM) → preserve assignee, task stays routable, retry via trigger_count/max_triggers fires.
• At cap exhaustion → escalate: assignee cleared to "" (operator-inbox signal), previous_assignee recorded (spec 027), phase left as the resume cursor.
• needs_input unchanged (immediate escalation — task-wrong, not transient).
• Applies to both the Kafka and file result-deliverer paths (applyResultFrontmatter helper keeps DeliverResult under the funlen limit).

Tests updated/added: below-cap preserves assignee, at-cap escalates with previous_assignee, for both deliverers. make precommit green (0 lint issues, all packages pass).

[ben-s-pull-request-reviewer]

Now I have enough information. Let me compile the findings.

Key verification results:

1. Counterfeiter mock exists: mocks/delivery-content-generator.go exists with the correct directive at content-generator.go:14
2. Suite file exists: delivery/delivery_suite_test.go is present (so suite-test-file-required findings are false positives)
3. TriggerCount()/MaxTriggers() are real methods with proper handling of zero/unset values (defaults to 0 and 3 respectively) - the concern is addressed
4. glog usage is pre-existing in this project (not new), so exempt from slog-not-glog-in-new-projects
5. Counterfeiter *Returns() methods are genuinely void (no return) - exempt from no-bare-error-call
6. Both code paths (content-generator.go and result-deliverer.go) correctly implement triggerCountAtCap with the same logic

Traceability: Candidate rules from Step 4b-i glob: The changed files are all Go files in delivery/, which matches the project rules. The mechanical findings from the pre-computed funnel (66 findings) are the authoritative set.

Now producing the consolidated report:

---

PR Review: fix(delivery): preserve assignee on failed result so retries stay routable

Must Fix (Critical)
None in the new code. The architectural debt in result-deliverer.go (package-level function calls in business logic) exists in both the old and new code and is not introduced by this PR.

Should Fix (Important)
None.

Nice to Have (Optional)

• delivery/content-generator.go:128 — triggerCountAtCap loops over frontmatter map without a ctx.Done() check. This is a short loop (one iteration over fmMap), so the practical risk is minimal, but the pattern should be consistent. (rule: go-context/cancel-check-in-loop)

---

Selector mode traceability:

• Candidates: 66 findings across go-architecture-assistant, go-context-assistant, go-quality-assistant, go-test-quality-assistant
• Applicable: 2 rules triggered by the diff (go-context/cancel-check-in-loop via the short loop at line 128, go-cli/slog-not-glog-in-new-projects for glog import — though exempt)
• False positives addressed: counterfeiter directive present + mock exists (go-testing/counterfeiter-mocks-required), suite file exists (go-testing/suite-test-file-required), counterfeiter *Returns() methods are void (go-testing/no-bare-error-call)

precommit skipped (selector mode) — CI covers lint+test

Notes:

• The bug fix logic is correct: AgentStatusFailed now preserves assignee below the trigger cap and only clears it at cap exhaustion, with previous_assignee recorded. Both the content-generator.go (file path) and result-deliverer.go (Kafka path) implement the same contract via triggerCountAtCap.
• TriggerCount() returns 0 when absent; MaxTriggers() returns 3 when absent — the >= comparison handles the zero/unset edge case correctly.
• The content-generator.go path correctly does NOT call buildFailureSection for AgentStatusFailed — it delegates to the caller (the ReplaceOrAppendSection in Generate). The result-deliverer.go path correctly does NOT add a ## Failure section (the generator already included it in its output).
• The existing architectural debt in result-deliverer.go (23 package-level function calls) is pre-existing and not introduced by this PR.

{
  "verdict": "approve",
  "summary": "Bugfix correctly implements retry-vs-escalate contract: failed results preserve assignee below the trigger cap so retries stay routable, and only escalate (assignee cleared, previous_assignee recorded) at cap exhaustion. Both code paths (content-generator and Kafka deliverer) implement the same logic. The architectural debt (package-level function calls in result-deliverer.go) is pre-existing and not introduced by this PR. The one SHOULD- tier finding (missing ctx.Done check in short loop) is pre-existing in the changed code.",
  "comments": [
    {
      "file": "delivery/content-generator.go",
      "line": 128,
      "severity": "minor",
      "message": "Short for-loop in triggerCountAtCap does not check ctx.Done(). While the loop is brief (one map iteration), the go-context/cancellation-in-loops pattern should be applied consistently. *(rule: go-context/cancel-check-in-loop)*"
    }
  ],
  "concerns_addressed": [
    "correctness: triggerCountAtCap logic confirmed correct — TriggerCount() defaults to 0, MaxTriggers() defaults to 3, >= comparison handles zero/unset",
    "correctness: both code paths (content-generator.go and result-deliverer.go) implement the same triggerCountAtCap logic and stay in sync",
    "correctness: getFrontmatterField returns empty string on absent/non-string values — yaml edge cases handled",
    "correctness: previous_assignee only recorded inside triggerCountAtCap branch — confirmed as the only escalation trigger"
  ]
}