fix(delivery): record previous_assignee on needs_input escalation too

[bborbe]

spec 027 mandates previous_assignee on every assignee-clear event. The retry-vs-escalate fix (v0.81.2, #46) stamps it on cap-exhaustion only; the needs_input path (task-wrong, permanent — e.g. "no date range", "zero trades") cleared assignee without the stamp, so the operator inbox couldn't show which agent parked the task.

Fix: both deliverer paths (Kafka + file) now record the pre-clear owner on needs_input before clearing assignee. needs_input stays an immediate escalation (agent already did the work, retry can't help); this only adds the query-ability stamp.

Tests: needs_input with an incoming owner records previous_assignee + clears assignee, for both deliverers. make precommit green (0 lint issues).

[ben-s-pull-request-reviewer]

I have all the information I need. Let me compile the consolidated report.

---

PR Review: fix(delivery): record previous_assignee on needs_input escalation too

Target: master ← fix/needs-input-prev-assignee (c547039)

Verdict

approve — the bug fix is correct, well-tested, and the new tests verify the exact contract change. All three concerns from the plan are resolved. The remaining findings are pre-existing violations unrelated to this PR, or mechanical-tool false positives.

---

Must Fix (Critical)

None — no new critical issues introduced by this PR.

---

Should Fix (Important)

None — no new important issues introduced by this PR.

---

Nice to Have (Optional)

None.

---

Pre-existing Issues (not introduced by this PR, but visible in the diff)

The mechanical funnel surfaced real violations that pre-exist this PR in result-deliverer.go. Flagging them for visibility, not as blocking this PR:

• delivery/result-deliverer.go:128 — for k, v := range fmMap loop has no ctx.Done() check. Cancellation mid-loop (e.g. during a slow iteration if the map grows) would not abort early. (rule: go-context/cancel-check-in-loop)
• delivery/result-deliverer.go:116 — DeliverResult function iterates the frontmatter map and calls applyResultFrontmatter without a ctx.Done() guard between iterations. (rule: go-functional-composition/list-checks-ctx-done)
• delivery/result-deliverer.go:18 — github.com/golang/glog import. This project is mid-migration; if glog is being phased out, the glog.V(2).Infof at line 165 and glog.Warningf at line 226 are candidates. (rule: go-cli/slog-not-glog-in-new-projects)

Mechanical Funnel False Positives (for traceability)

The following findings from the ast-grep run are incorrect — do not flag as issues:

• delivery/content-generator.go:17 — The //counterfeiter:generate directive is present on line 14 of the same file. The scanner reported line 17 (the interface body start) but the directive is 3 lines above it. No action needed.
• delivery/result-deliverer_test.go:77,113,115,132,152,177,203,… — All no-bare-error-call violations on *.Returns(...) mock calls. Counterfeiter-generated mock methods return only (...) — there is no error to check. These are all false positives from the errcheck rule.
• delivery/*_test.go suite-test-file-required findings — The companion delivery/delivery_suite_test.go exists and registers RunSpecs(t, "Delivery Suite"). Ginkgo discovers specs across all files in the package; no missing suite.

---

Plan Concern Resolution

Concern	Resolution
correctness — content-generator.go: order of previous_assignee before assignee clear	✅ Confirmed: lines 78–81 set previous_assignee before clearing assignee, matching the cap-exhaustion path at lines 96–99
correctness — result-deliverer.go: AgentStatusNeedsInput stamps previous_assignee before clearing	✅ Confirmed: lines 217–220 do previous_assignee before assignee = "", identical order to content-generator.go
correctness — result-deliverer.go: assignee/previous_assignee initialized from incoming frontmatter before stamp/clear	✅ Confirmed: frontmatter is built from fmMap (copied from generator output) at lines 128–131, so the stamp reads from the same source as the content-generator path

---

Notes

• The bug fix is minimal and targeted: needs_input now stamps previous_assignee before clearing assignee in both the content-generator and kafka-deliverer paths, matching the cap-exhaustion behavior from v0.81.2.
• New tests cover the case where an assignee is present (previous_assignee recorded) and when no assignee is present (no spurious empty stamp — both paths guard with if prev != "" / if prev != "").
• The AgentStatusNeedsInput behavior is now: status: in_progress, assignee: "", previous_assignee: <prior-owner>, phase preserved. This is consistent across file and Kafka deliverers.

---

{
  "verdict": "approve",
  "summary": "Bug fix is correct and well-tested. Both deliverer paths (content-generator and kafka-result-deliverer) now record previous_assignee before clearing assignee on needs_input, matching the cap-exhaustion behavior from v0.81.2. New tests verify both the with-assignee and without-assignee cases. The plan concerns are all resolved. Remaining findings are pre-existing violations or mechanical-tool false positives.",
  "comments": [
    {
      "file": "delivery/result-deliverer.go",
      "line": 128,
      "severity": "minor",
      "message": "for k, v := range fmMap loop has no ctx.Done() check — cancellation mid-iteration would not abort early. Pre-existing issue, not introduced by this PR. (rule: go-context/cancel-check-in-loop)"
    },
    {
      "file": "delivery/result-deliverer.go",
      "line": 116,
      "severity": "minor",
      "message": "DeliverResult function iterates frontmatter without ctx.Done() between iterations. Pre-existing issue. (rule: go-functional-composition/list-checks-ctx-done)"
    },
    {
      "file": "delivery/result-deliverer.go",
      "line": 18,
      "severity": "nit",
      "message": "Uses github.com/golang/glog instead of log/slog. Pre-existing. (rule: go-cli/slog-not-glog-in-new-projects)"
    }
  ],
  "concerns_addressed": [
    "correctness: content-generator.go order of previous_assignee before assignee clear — addressed: lines 78-81 set previous_assignee before clearing assignee",
    "correctness: result-deliverer.go needs_input stamps previous_assignee before clearing — addressed: lines 217-220 match content-generator.go order",
    "correctness: assignee and previous_assignee fields initialized from incoming frontmatter before stamp/clear — addressed: frontmatter built from fmMap at lines 128-131, stamp reads same source as content-generator path"
  ]
}