[john-the-dev]

The gateway orphan sweep currently posts locally scheduled task completions to /v1/results. A source: cron task has no broker lease or destination, so even “nothing to notify” becomes an undeliverable recovered reply on every scheduled run.

This change leaves cron completion files to their local consumers before any recovery, duplicate retirement, or age quarantine. It reads the structured header with the existing task-last parser, so a body line cannot forge the source. Gateway replies keep their existing guarded delivery, suppression, retry, and quarantine behavior. Unreadable task files wait for another sweep.

Review remote_gateway_bridge._reconcile_orphan_results first, then the LocalCron regression cases. These cover live and archived tasks, normal and expired result ages, ordinary/suppressed/redirected output, and a gateway task whose body contains source: cron. This does not change proactive notification routing; the separate destination-selection gap remains tracked in #3870.

Actual before/after evidence using the same regression test:

# Parent c93e2b2a3 plus new tests
$ python3 tests/gateway-orphan-result-reconciler.test.py -v
... '/v1/results',
... {'body': '(recovered result — original delivery was lost)\n'
...          'unchanged; nothing to notify',
...  'id': 'task-00000000000000000a'})]
Ran 32 tests in 0.224s
FAILED (failures=13)

# Patched
$ python3 tests/gateway-orphan-result-reconciler.test.py -v
Ran 32 tests in 0.076s
OK
$ python3 src/remote-gateway-bridge.test.py
PASS — all checks green
$ python3 tests/ag2-sparrow-drift.test.py
PASS — ag2-sparrow bundled utils in sync with src/
$ python3 skills/release/scripts/docs_audit.py
docs-audit: PASS (48 indexed docs; 51 Markdown files checked)
$ bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

Live verification completed on the affected host with the exact PR-head gateway module: fresh owner DM → ordinary broker result → visible reply, silent cron completion preserved through a positively witnessed recovery sweep, and a synthetic notification from the actual pending-questions writer delivered to the owner DM. Original module and healthy service restored afterward. See the detailed live verification comment below for UTC timestamps, module hashes, controlled restart, baseline differences, and restoration evidence. The separate legacy task-mid cron writer is outside the documented pre-body source contract.

Documentation: updated the canonical remote gateway protocol and catalog verification date. No migration or new configuration. Rollback is reverting this commit. Cron completion retention remains owned by existing local consumers; this change neither discards their files nor redirects their contents to a default room. No host-specific paths added.

[bassilkhilo-ag2]

APPROVE at 30155fa8. Independent review — cloned fresh, verified the claims rather than trusting the pasted output.

Before/after regression reproduces exactly. Dropped the PR's test file onto main (c93e2b2a, this PR's actual parent):

main + new test:   FAILED (failures=12)
PR head:           OK, Ran 31 tests in 0.053s

The specific failure on main is the defect itself, not a proxy for it: the reconciler POSTs {'id': 'task-00000000000000000a', 'body': '(recovered result — original delivery was lost)\nunchanged; nothing to notify'} to /v1/results for a source: cron task — exactly the undeliverable recovered-reply pattern described.

Read the actual source change, not just the test:

task = find_task_file(TASKS_DIR, tid) or _archived_task_file(tid)
if task is not None:
    headers = local_task_protocol.parse_task_headers(task.read_text(...)).headers
    if headers.get("source") == "cron":
        continue

Small, targeted, and it hoists the existing find_task_file(...) or _archived_task_file(...) lookup earlier rather than duplicating it — confirmed the later use of task (the no-task quarantine branch) is unaffected, just reads the same variable.

The "a body line cannot forge the source" claim, checked at the source it depends on, not taken on faith: local_task_protocol.parse_task_headers (line 277) is explicitly the task-last parser — "collect key: value lines strictly BEFORE the first task: line; everything from task: onward is untrusted body" (PR #982/#1035's delimiter rule). That's the right function for this — the lenient/shape-union parser used elsewhere for legacy desktop writers would NOT give this guarantee. The PR's own second test (test_body_cannot_opt_gateway_task_out_of_recovery, a task with real header source: ag2space and a body line reading source: cron) is exactly the adversarial case this depends on, and it asserts the opposite outcome (len(self.posted) == 1) — a real control, not a decorative one.

Coverage matches the claim: live+archived tasks, normal+expired ages, ordinary/suppressed/redirected bodies, and the forged-body case — all present in the diff's new LocalCron class. I didn't find a gap in the matrix.

Independently reran the other evidence scripts, not just the CI badges:

ag2-sparrow-drift.test.py     PASS
docs_audit.py                 PASS (48 indexed, 51 checked)
grep for /Users// or /home/<user>/   clean

CI at review time: nothing failing, several suites still pending (typecheck+node&shell, python standalone 1-3, PowerShell, shell-tests-serial-tail).

Scope matches the claim — one function (_reconcile_orphan_results), the existing header parser (no new dependency), its regression test. Gateway reply delivery/suppression/retry/quarantine paths are untouched (confirmed by reading the diff, not just trusting the body text). Correctly does not touch proactive-notification routing (#3870 stays separate).

One thing worth restating, not a blocker: the PR itself says live post-restart verification is still pending and explicitly does not claim unit tests as a substitute — agreed, that's the right line to hold before merge, distinct from code-review sign-off.

This is one of the two reviewers requested.

[github-actions]

@cla-assistant check

[bassilkhilo-ag2]

Re-APPROVE at d7a72c73 — confirmed the delta from 30155fa8b (my prior approval) is test-only: git diff --stat shows exactly 17 lines in tests/gateway-orphan-result-reconciler.test.py, nothing outside tests/. The new case patches Path.read_text to raise PermissionError on the cron task file mid-sweep and asserts nothing gets posted and the result file is left untouched — the except OSError: continue deferral path I already read in the production diff. Reran the full suite at this head myself (not just trusted the pasted count): 32/32 pass.

[liususan091219]

APPROVE at d7a72c732.

Drift note: @rui asked for 30155fa8b; the branch moved to d7a72c732 while I was measuring. The new commit is test-only (test(gateway): cover deferral while cron task cannot be read) and git diff 30155fa8b..d7a72c732 -- packages/ docs/ is empty, so the production change is byte-identical to the head named. I reviewed the head that exists.

Control. New suite against the unpatched bridge:

origin/main   rc=1   FAILED (failures=13)   test_cron_completion_is_left_for_its_local_consumer,
                     parameterised across archived True/False x age 3600/86401 x
                     {plain body, [no-send], [channel: !owner:…]}
d7a72c732     rc=0   Ran 32 tests, OK

The suite reproduces the unwanted recovered sends and stops on the fix, as described.

The forgery question, answered with the production parser rather than the test's claim. The gate keys on headers.get("source") == "cron", so it is only safe if body text cannot manufacture that header. local_task_protocol.parse_task_headers on three inputs:

source: cron before task:    -> source='cron'      skipped
source: cron AFTER task:     -> source='ag2space'  not skipped
source: cron in the body     -> source='ag2space'  not skipped

task: terminates the header block, so a body line cannot forge it and the first real source wins. The gate holds.

---

One finding, and it is the reason I spent the time: a real in-the-wild cron task on this host is NOT covered by the gate.

This morning's task-wake-briefing-20260922 on susans-MacBook-Pro carries source: cron — but after task:. Running the production parser over the archived file:

raw line order:  ['id', 'timestamp', 'task', 'source', 'channel_id', 'user_id']
parse_task_headers().headers  ->  {'id': 'task-wake-briefing-20260922', 'timestamp': '2026-09-22T11:01:07Z'}
headers.get('source')          ->  None      # NOT 'cron' — this task would still enter orphan recovery

Nothing here is wrong with this PR: the code and the docs both say pre-body source: cron, and that is exactly what they check. The gap is upstream — CLAUDE.md already names it ("task: must be the LAST header … so source, channel_id, access_tier and priority all read as absent … the writer is what must change") — and whatever emits task-wake-briefing-* puts task: first. So the PR's reach is narrower than "cron completions" reads, through no fault of its own.

I am approving rather than blocking because this is a pre-existing writer defect, the change is a strict improvement over leaving every cron completion exposed, and blocking it would keep the larger hole open. Worth one sentence in remote-gateway-protocol.md making the precondition load-bearing rather than incidental — something like "a cron task whose source: line falls below task: parses as headerless and is not covered" — so the next person reading the doc knows the gate depends on writer discipline. I can file the writer bug separately if nobody has.

On the OSError path, which I had written up as an observation before the push: moving the task lookup to the top of the loop means an unreadable task file now continues where it previously fell through to the age check. That is the right direction (without headers you cannot tell cron from gateway, and re-delivery is the defect), and d7a72c732 now pins it — test_unreadable_task_defers_without_consuming_result asserts nothing is posted and the result file survives. Good addition; it closes the one thing I had.

Something I checked and could not use, stated because the negative matters. This host has 18 stale files in results/, which looked like evidence that nothing retires cron completions. It is not: 17 are engine-patch-check-* / meeting-prep-*, which _valid_local_tid rejects, so they were never in the sweep's population and this PR changes nothing about them. The one genuine source: cron task I could trace — the wake-briefing above — had both its task and its result archived by the local consumers within the hour, which is positive evidence for the PR's premise that local scheduling owns retirement.

Gates, read immediately before submitting. license/cla is ABSENT, not green: the combined status returns total_count: 0 with an empty context list, and state: pending is merely what an empty set reports. That needs to clear before merge (SHA-bound; close+reopen is the retry that works when it does not return on its own). CI: 14 success, 1 skipped, 2 still in flight (version-drift, PowerShell + Windows runtime) — nothing red, but not finished either.

Limit of my instrument. Everything above is in-process on macOS against a scratch tree. It says nothing about a live broker round trip, which is the evidence @rui already flagged as pending — and for a delivery-loop change that is the evidence that should gate the merge, not this review.

— Sutando (susans-MacBook-Pro)

[liususan091219]

Correction to one line of my review above: I wrote that license/cla was ABSENT (total_count: 0). It was, when I drafted — and it returned success with 1 context in the seconds between that read and the submit, which my pre-submit re-check caught too late to edit into the body.

So: the CLA is green at d7a72c732 and needs no close+reopen. Nothing else in the review depends on it — the approval, the control run and the wake-briefing finding all stand. CI still had version-drift and PowerShell + Windows runtime in flight at submit time; read the final state rather than my sentence.

— Sutando (susans-MacBook-Pro)

[john-the-dev]

Thanks for independently reproducing the regression and checking the parser. The legacy wake-briefing task-mid writer is outside the explicit pre-body source: cron contract documented here; widening to a body scan would let body text suppress genuine gateway recovery. I will retain that boundary. The affected-host verifier has been asked to confirm the real pending-questions task's header order as well as the live round trip. The production code remains fixed at d7a72c7 while verification runs. The separate notification destination gap remains #3870.

[github-actions]

Coverage Gate

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

Diff Coverage

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

• packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py (100%)

Summary

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

[john-the-dev]

Live verification of d7a72c732 completed on the affected host (2026-09-22, UTC). This is additional evidence beyond the 32-test before/after regression.

What ran: the vendored remote_gateway_bridge.py from this exact PR head, SHA-256 33f27dfd1530c3ac3d9bb4530e261d9d3a63c8b2f24e4a31cad989a7c0b2ac1b, under the installed Python 3.12 runtime, existing loader, real workspace, and production broker. Runtime status reported the same module hash. This was a temporary module swap, not a rebuild of the entire app. The installed baseline also predates main's proactive room-gate additions; that difference was inspected and the tested module was the complete PR-head module, not a hand-edited approximation. Independent before/after tests isolate the cron change itself.

Post-restart input → broker result → visible DM:

14:26:13 original child stopped; supervisor retained
14:26:18 PR-head module acquired singleton lock; no second primary poller
14:28:11 second controlled restart, with recovery fixtures already present
14:29:19 queued task-cd420fc43201f4dc1a (fresh owner-DM input)
14:30:11 result task-cd420fc43201f4dc1a delivered via DeliveryCore
14:30:11 delivered result for task-cd420fc43201f4dc1a

Production broker independently logged that same result routed to the owner DM and delivered with an event ID. The in-app browser visibly showed PR4609-RESULT-ACK. An earlier probe replied via a direct tool; it was not counted as the ordinary result-path witness.

Silent cron recovery: a synthetic, one-hour-old completion with a pre-body source: cron task in tasks/archive remained byte-for-byte unchanged. The same sweep quarantined a no-task control at 14:28:42, positively proving the sweep executed. The cron fixture never appeared in production broker logs, and the bridge never recovered or retired it. The actual affected pending-questions task's pre-body header order was independently checked on the host. Test-only task/result/control files were moved out of the live workspace before restoration.

Actual pending-questions notification writer: invoked notify_discord_dm() with one synthetic question, using the existing delivery path. Its proactive claim was recovered across the controlled restart and logged delivered to the owner DM at 14:28:13; the browser visibly confirmed the notification. This proves the existing path still works; it does not claim to fix the separate destination-selection policy in #3870.

Restoration: original module restored at 14:30:48, byte-verified as SHA-256 4e5696d23e9254de993fcaf3767bf71cae94250bf13cc1307b1a155a7a5284f3. The original supervisor remained running throughout, the dev instance was untouched, and an independent rollback watchdog was disarmed after restoration.

At 14:31:23, fresh runtime status reported connected: true, error: null, and the original module hash. Supervisor PID 13987 remained alive; the rollback watchdog had exited. The test is complete and the installed service is back on its original build. The permanent change takes effect when the updated engine is deployed.