[john-the-dev]

Closes #5102

What changed, and why

After a network outage plus Claude auth expiry, the owner got one alert and then nothing for ~23h. The card for a blocking requirement only ever produced one notifying message:

• src/hitl/manager.py:175-176: re-detecting the same auth state returns the existing record unchanged. Its revision does not move, so needs_projection (manager.py:254) is false and nothing is sent.
• src/hitl/manager.py:177-180: when the guard changes, the revision is bumped. But src/hitl/projector.py:89-90 sends every revision after the first as op: edit of the CREATE event, and an edit notifies nobody.
• Nothing in src/hitl/, src/core-input-watch.py or the bridge re-alerts based on elapsed time.

Fix (all within src/hitl/):

• New src/hitl/renotify.py is the only place this policy lives. It applies to auth and core-blocked only; decision cards are left alone. While such a requirement is unresolved:
  • it sends a reminder as a NEW op: message after 30m, then after 2h more, then every 6h after that;
  • the reminder says how long it has been blocked, plus "N tasks are waiting" taken from blocked_task_ids.
  • When the requirement resolves, one "✓ Reconnected — resuming N queued tasks." message goes out right after the existing status edit.
• projector.py:
  • project() / pending_ids() take an injectable clock (default time.time).
  • pending_ids() now also counts a due notice, so the bridge's _project_hitl short-circuit (packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:1265) does not skip it.
  • Notices are sent only after the card is current, so the edit always lands first.
  • Notices are plain text with no space.ag2.hitl field. A second card would never receive the status edits, which target the CREATE event only, so it would stay "pending" forever.
• manager.py:
  • record_projection now merges into the existing ledger instead of replacing it.
  • The new record_notice merges in notified_at / reminders / recovered.
  • This state is persisted in the existing per-requirement JSON, so a restart neither re-sends nor loses the schedule.
  • A record projected before this change has no notified_at, so it counts from created_at. A card that is already stale gets one reminder on the first pulse after upgrade.
• Dedupe keys are hitl:<id>:remind:<n> and hitl:<id>:recovered. A rejected send records nothing, and the retry reuses the same key.

Review first

src/hitl/renotify.py (the policy), then the _send_notices tail of src/hitl/projector.py.

Behavior this proves

An auth card first sent 23h ago with 2 queued tasks goes through 3 bridge pulses while still blocked, then a recovery pulse. The repro script builds the record with hitl.detector.auth_requirement and drives the real project() with a recording sender (body truncated to 75 chars):

At parent afb79381 (origin/main):

message | ⚠ Sutando needs your attention — Claude Code needs to sign in
edit | ⚠ Sutando needs your attention — Claude Code needs to sign in ag

At HEAD:

message | ⚠ Sutando needs your attention — Claude Code needs to sign in
message | ⚠ Still blocked after 23h 0m — Claude Code needs to sign in a
edit | ⚠ Sutando needs your attention — Claude Code needs to sign in ag
message | ✓ Reconnected — resuming 2 queued tasks.

Tests

The new tests/hitl-renotify.test.py uses a fake clock and a fake sender. It covers:

• 30m → one new op: message;
• a little later → nothing;
• the backoff schedule sampled on a 5-min pulse over 24h: [30, 150, 510, 870, 1230] minutes;
• inside the interval → nothing;
• resolve() → the edit plus one recovery message, and no reminder after it;
• the schedule survives a restart (new HitlStore on the same dir);
• a rejected reminder is retried with the same dedupe key and not counted;
• a choice card is never re-alerted;
• a card that is already terminal when first sent gets no recovery notice.

Before — new test file run against the parent sources (fix stashed; src/hitl/renotify.py absent):

TypeError: project() got an unexpected keyword argument 'clock'
...
Ran 9 tests in 0.018s
FAILED (errors=9)

The behavioral before/after is the repro above. On main project() has no time input at all, so no clock can make it re-alert.

After:

Ran 9 tests in 0.273s

OK

Full hitl suites and the bridge projection suite at HEAD:

tests/hitl-click-header.test.py: Ran 6 tests in 0.000s OK
tests/hitl-detector.test.py: Ran 10 tests in 0.009s OK
tests/hitl-events.test.py: Ran 10 tests in 0.020s OK
tests/hitl-hook-driver.test.py: Ran 16 tests in 17.180s OK
tests/hitl-manager-identity.test.py: Ran 11 tests in 0.028s OK
tests/hitl-policy.test.py: Ran 7 tests in 0.027s OK
tests/hitl-projector.test.py: Ran 11 tests in 0.170s OK
tests/hitl-renotify.test.py: Ran 9 tests in 0.229s OK
tests/hitl-replies.test.py: Ran 44 tests in 0.058s OK
tests/hitl-schema.test.py: Ran 29 tests in 0.098s OK
tests/hitl-supervisor.test.py: Ran 5 tests in 0.029s OK
tests/hitl-task-relay-click.test.py: Ran 10 tests in 0.018s OK
tests/hitl-tui-gate.test.py: Ran 23 tests in 0.004s OK
tests/sparrow-hitl-projection.test.py: Ran 8 tests in 0.024s OK

These other hitl consumers also pass (exit 0): core-input-watch-card-step, core-input-watch-chat-escalation, core-supervisor-relay, pool-wedge-cards, pool-input-watch-per-seat, pool-wedge-card-ack-and-batching, report-feedback-recovery, runtime-api-human-action, runtime-api-tasks-view, runtime-api-e2e, activity-bus, src-map.

tests/report-feedback.test.py fails 2 TestReportFeedbackCloudAuth tests on this host and also exits 1 at the parent commit. Those tests read the host's real cloud-auth file, which is unrelated to this change.

Existing assertions changed on purpose: 3 of them assumed the resolve edit was the last thing sent.

• hitl-supervisor test_recovery_pass_… now checks the edit and then the recovery message.
• hitl-supervisor test_flap_… now expects [card message, card edit, plain message].
• hitl-projector test_edit_target_survives_… now filters to card events.

git diff origin/main... | bash scripts/review-checks.sh → review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean). docs/src-map.md was regenerated with scripts/gen-src-map.py.

Feature/doc consistency

N/A: no canonical doc covers HITL card delivery (no docs/catalog.json entry). The new module is indexed in docs/src-map.md.

Live path

This changes what the bridge's _project_hitl pulse sends. No post-restart live round trip was captured. Getting one means restarting the owner's live gateway bridge on a branch, and waiting 30m in a real blocked state. Evidence is limited to the unit tests above and the bridge suite (sparrow-hitl-projection), which drives the real _project_hitl against a real store. Reviewers should treat the live round trip as outstanding.

Not covered

• Off-host liveness. If the whole machine or its network is down, nothing on the host can send anything. That needs an external dead-man's switch (AG2 Space presence / feat(dead-mans-switch): off-machine liveness alerting via external ping monitor #1932).
• The keychain OAuth item disappearing (why auth expired in the first place) is a separate defect.
• Reminders reach the owner only once the bridge can post again. During the outage itself they are refused and retried, same as the card.

Update at 8de6108 (2026-10-07)

• Merged origin/main (was 17 behind). The previous run's 5 red jobs at 33dea0d were runner abandoned (the tsc + tests (clean install) summary step reported node-and-oot-tests, shell-tail-tests and python-standalone-tests as FAILED (abandoned)), not test failures. All of them pass on the re-run.
• The re-run then showed one real gap: diff coverage >= 95% (python) at 94%. The missing lines were renotify.py 44, 48, 67-68 and manager.py 276. Added 4 tests for those branches: an unprojected revision waits for its edit, a cancelled card gets no notice, a block longer than a day reads in days, and record_notice on a missing record is a no-op. Locally, diff-cover over the hitl suites now reports manager.py, projector.py and renotify.py at 100%.
• The original before/after still holds at current main (30b0a49d). With the new test file copied onto main: Ran 9 tests ... FAILED (errors=9). At head: Ran 13 tests ... OK. All 14 hitl suites plus sparrow-hitl-projection pass under env -i HOME=<tmp> PATH=$PATH TMPDIR=<tmp>. gen-src-map.py --check is up to date, and review-checks: PASS.
• CI at 8de6108: 32 pass, 1 skipped (Windows). The live post-restart round trip is still outstanding, as stated above.

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 91%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/hitl/manager.py (100%)
• src/hitl/projector.py (100%)
• src/hitl/renotify.py (100%)

Summary

• Total: 90 lines
• Missing: 0 lines
• Coverage: 100%

[bassilkhilo-ag2]

Reviewed at head 8de6108.

Verdict: changes requested. One blocker, found by running the head against real data. Not ready to merge.

Blocker: upgrade sends a burst of stale "Reconnected" messages

renotify.due() returns RECOVERY for any auth / core-blocked record with status == resolved, a current projection, and no recovered key. Every record resolved before this change matches that, because recovered did not exist yet. The store keeps terminal records (nothing prunes state/hitl/requirements), so the first bridge pulse after the upgrade owes a recovery notice for every historical block. Each has its own dedupe key (hitl:<id>:recovered), so the bridge does not collapse them.

Repro: I copied a real host's state/hitl/requirements (103 records: 86 resolved core-blocked, 13 resolved auth, 4 resolved permission) to a scratch dir and ran this head's pending_ids() and project() on it with a recording sender:

records 103 pending 99
99
[('✓ Reconnected — Sutando has continued it', 99)]
second pulse 0

So a single pulse sends 99 owner DMs announcing recoveries from blocks that ended days or weeks ago. That is exactly the spam this PR exists to prevent, in the other direction.

Suggested fix, in renotify.py where the policy lives: only owe a recovery for a record whose ledger this code wrote. For example, require notified_at (written by on_create) to be present before returning RECOVERY, and treat a resolved legacy record as already recovered. Then add a test that seeds a pre-change ledger ({"revision": n, "event_id": "$x"}) on a resolved auth record and asserts that project() sends nothing. The PR body already handles the legacy pending case (count from created_at, one reminder). The legacy resolved case needs the same deliberate handling.

Verified

• Schedule math: on_create stamps notified_at=now, reminders=0, and interval() clamps to (30m, 2h, 6h). That gives reminders at 30, 150, 510, 870... minutes, which matches the test.
• Dedupe keys are stable across a restart. remind:<n> comes from the persisted reminders count, a refused send records nothing, and the retry reuses the key. record_projection now merges instead of replacing, so the edit path keeps notified_at / reminders.
• Flapping: a resolved card is terminal, and manager.create() only dedupes against active(). A re-block therefore mints a new id with a fresh ledger. Each flap costs one card plus one recovery line, with no reminder storm.
• Edit-before-notice ordering holds (revision < req.revision returns None).
• Tests at head: hitl-renotify, hitl-projector, hitl-supervisor and sparrow-hitl-projection all OK under a clean env -i. review-checks: PASS on the source diff. Comments are within 2 lines and carry no PR or people references. CI: all green, Windows skipped.

Nits (non-blocking)

• "Quiet when the owner is acting" covers only decided_by. An auth card the owner has clicked (in_progress, decided_by unset) can still get a reminder mid-login. Consider skipping REMINDER while the status is in_progress.
• The reminder says "N tasks are waiting" from blocked_task_ids as it was at creation. If tasks attach later, the count can be stale. That is fine if intended.

Lesson 15

This changes what the bridge's _project_hitl pulse sends to the owner, so it is a notification live path. Even once the blocker is fixed, approval should wait for a real post-restart round trip: a blocked state held past 30m, a reminder that arrives as a new notifying message, then the recovery line. The PR body correctly lists this as outstanding.

[github-actions]

@cla-assistant check

[john-the-dev]

@bassil Fixed at 7f33544, in response to your blocker on the recovery-notice burst after upgrade.

Fix: renotify.due() now owes a recovery notice only when the ledger carries notified_at. Only the renotify path writes that field, either at CREATE or after a reminder. A record resolved before the upgrade has the bare {revision, event_id} ledger, so it gets no "Reconnected" DM. A pre-upgrade card that is still blocked and gets re-alerted after the upgrade does still get its recovery notice.

Tests (tests/hitl-renotify.test.py, +4):

• an old resolved record with no ledger fields gets no recovery;
• an old record whose resolve edit is still pending gets the edit only;
• one pulse over a store of 100 pre-upgrade records (auth / core-blocked / choice) sends no recovery;
• a pre-upgrade card re-alerted after the upgrade still announces recovery.

Before, at 8de6108: FAILED (failures=3), covering the first 3. On the 100-record store, one pulse sent 67 "Reconnected" messages, one per resolved auth or core-blocked record.
After: Ran 17 tests ... OK. The same store gets 25 status edits and 0 "Reconnected" messages.

All 14 hitl suites and sparrow-hitl-projection pass. Local diff coverage over src/hitl is 100%. CI at 7f33544: 32 pass, 1 skipped (Windows). One job, tests/browser-persistent.test.py, failed once: a headless-browser launch exited 1. It doesn't touch hitl, it passed at 8de6108, and it was green on rerun.

[bassilkhilo-ag2]

Re-review at 7f33544. The stale "Reconnected" blocker from my 8de6108 review is fixed.

What changed: renotify.due() now owes a recovery only when the ledger has notified_at, which only the new code writes. Diff 8de6108..7f33544 is that one condition plus 4 tests.

Re-measured on a copy of this host's real HITL store (103 records: 86 core-blocked, 13 auth, 4 permission, all resolved with current projections). Real store untouched; sends stubbed and counted.

head	pulse 1	pulse 2	pulse +1 day
8de6108	99 Reconnected, 0 edits	0	0
7f33544	0 sends (0 Reconnected, 0 edits)	0	0

Edge case checked: I turned one real auth record back into a pre-upgrade still-blocked card (bare {revision, event_id} ledger). Pulse 1: one "Still blocked" message, notified_at written. Pulse 2: nothing. After resolve(): one card edit plus exactly one "Reconnected" message. Two later pulses (including +1 day): nothing.

On the 25 card edits in your synthetic run: those come from the test store deliberately leaving every 4th record's projection one revision behind. On this host's real store there are 0. Where they do occur, they are the existing status edit (op: edit on the card), which this PR does not add and which notifies nobody.

Non-blocking note: a pre-upgrade card still open but younger than 30 minutes at upgrade, which then resolves before its first reminder, gets the resolve edit but no recovery message. The owner only ever saw the original card, so that seems acceptable to me. Flagging it so it's a deliberate choice.

CI: 32 checks green, Windows skipped, CLA green.

Verdict: COMMENT, not approve. This is a notification path (REVIEW.md lesson 15), and the evidence so far is unit tests plus my store replay, not a post-restart round trip. I'll approve once there is a real post-restart round trip (a block, its reminder and its recovery message actually arriving in a room), or once Rui explicitly accepts the store replay as a substitute.

[john-the-dev]

Owner decision (Rui, 2026-10-07 ~17:05Z, owner DM): "Accept 5126."

Rui accepts Bassil's replay against a copy of his real HITL store (103 records) as the substitute for a post-restart round trip on this PR:

• Old head: sent 99 "Reconnected" DMs in pulse 1.
• This head (7f33544): sent 0, made 0 card edits, and sent 0 on later pulses.
• Pre-upgrade card: a card that was still blocked got one reminder and, on resolve, exactly one "Reconnected" DM.

The live check after ship is still owed. The gap Bassil flagged (a pre-upgrade card under 30m old that resolves before its first reminder gets the edit but no recovery DM) is accepted as known and doesn't block.

[bassilkhilo-ag2]

Approving at 7f33544, converting my earlier COMMENT.

The stale "Reconnected" blocker is fixed. I replayed this head against a copy of a real HITL store (103 records) and got 0 recovery DMs and 0 edits on pulse 1, against 99 DMs at 8de6108. Later pulses also sent 0. A pre-upgrade card that is still blocked gets one reminder and, on resolve, exactly one "Reconnected" DM.

Rui accepted that replay as the substitute for the post-restart round trip: #5126 (comment). It was posted by john-the-dev, Rui's agent account. The comment's "~17:05Z" decision time is later than its own 16:50Z post time, so the timestamp is probably a different zone; the decision itself is recorded.

Two things remain open: the live check after ship, and the known gap where a pre-upgrade card under 30m old that resolves before its first reminder gets no recovery DM. Both are accepted as non-blocking.

[sonichi]

Code-owner review (Sutando, Echo Act IV Pro). Read the full diff. Clean separation: the card projection stays one message edited in place (notifies nobody, as the comment says), while hitl/renotify.py owns a separate NEW-message notice policy (REMINDER on a 30m/2h/6h backoff, one RECOVERY on resolve) via pure, directly-testable functions (due/on_create/after/body/dedupe_key) — the projector just sends what renotify returns and records the ledger fields back. 20 real tests in hitl-renotify.test.py cover the backoff schedule end-to-end, restart-survival (ledger persists notified_at/reminders), a rejected send not being counted (retried, same dedupe_key), decision cards never re-alerted, and careful pre-upgrade-ledger compatibility (a bare {revision, event_id} record from before this change gets no spurious recovery/reminder). hitl-supervisor.test.py and hitl-projector.test.py updated consistently. Matches the stated replay evidence (99 stale DMs -> 0) structurally — the dedupe_key + notified_at/reminders bookkeeping is exactly what prevents a revision-driven resend from also re-triggering a reminder. CI green, mergeable. Approving.