Approval-notify: notification_state='failed' is terminal — never re-arms after config is fixed, even as new messages pile up on the held chat

[olegbrok]

Found 2026-08-03 while configuring owner-notification destinations on a fleet host that had none.

The defect

_approval_request_needs_notification (broker.py) hard-stops on notification_state == 'failed':

• the durable retry loop only picks up state='retrying' (list_due_approval_notifications), and
• a NEW held message on the same chat cannot re-arm it either — the failed check returns False before the new-holds logic is ever reached.

So one exhausted notification cycle permanently silences the alarm for that chat. If the failure cause was environmental (the exact observed case: owner_notification_destinations unset / tokens unbound), fixing the config does NOT resurrect the requests that already failed — they stay dark forever while held_count keeps growing.

Observed impact

On the affected host, 3 requests were stuck in failed with held messages — one group chat with 15 messages held ~16 days. The approval gate held them correctly; the owner was simply never told, and never will be without manual intervention.

Suggested fix

Two independent guards (either alone closes the observed case, both together are cheap):

1. New holds re-arm a failed request: move the failed short-circuit below the new_holds computation, so new_holds > 0 flips it back to retrying (attempts reset).
2. Config-change re-arm: when owner_notification_destinations or a token binding is written, flip failed → retrying (next_retry_at = now) for pending requests — the environment the failure was recorded under no longer exists.

Also worth an admin endpoint (POST /agents/{name}/approvals/{id}/renotify) so an operator can re-arm without DB surgery. Related family: #865 (fail-loud notifications), the upgrade-orphaned-group-chat class.

---

🤖 Filed by Barsik

[olegbrok]

Design constraint from fleet-side triage of the live held requests (seconded by two agents operating the affected host): for held INBOUND owner/instruction content, "release" must be a read-only triage surface, not resurrection-into-session. Replaying a 16-day-old directive into a live session produces stale action — same hazard class as replaying stale scheduler wakes (#966 family). The re-arm fixes proposed above apply to the notification; the held messages themselves should surface for human triage, not auto-deliver.

Triage of the observed 5 stuck requests, for scope calibration: only ONE was genuinely-undelivered substantive owner content (15 messages, ~16 days); two were delivered=1 with only the notification failed; two were low-content contact handshakes from unapproved senders (correctly gated, just un-notified). So the blast radius here was one conversation — but nothing about the mechanism bounds it to that.