[qingyun-wu]

What changed, and why

ag2-sparrow writes each relay task to TASKS_DIR/<task-id>.txt and stops there. Nothing wakes the agent, so every outside agent writes its own folder-polling hook. This adds the wake step to Sparrow itself.

New optional setting REMOTE_TASK_ON_TASK (env, like the other REMOTE_TASK_* settings; the package has no CLI flags, so none is added):

• Run once per task file Sparrow newly publishes, after the atomic write, the media sidecar and the room sidecar have landed.
• shlex.split on the value, no shell; the absolute task-file path is appended as the last argument. The environment adds SPARROW_TASK_ID and SPARROW_TASK_FILE.
• Credentials removed: every REMOTE_*/AG2_* variable whose name contains TOKEN is dropped (REMOTE_TASK_TOKEN, AG2_REMOTE_TOKEN, REMOTE_TASK_TOKEN_FILE, REMOTE_MEDIA_HS_TOKEN), as is any variable whose value contains the relay secret, the raw onboarding string, or the HS media token.
• Non-blocking, fail-safe: Popen(start_new_session=True) with stdio on /dev/null; a daemon thread reaps it and logs one line on a non-zero exit. A start failure (missing binary, unparseable string) is logged once. Sparrow never waits for, kills or retries the command. The task file, the ack and the in-flight ledger are not affected.
• At most once per task id: the call sits at the end of _write_task, on the fresh-publish path only. Every idempotent early return (still queued / .assigned-* / .claimed-* / archived / delivered) returns before it, so a redelivery or a restart does not fire it again. Chosen trade-off, stated in the README: if Sparrow dies between publishing the file and spawning the command, that one wake is skipped (no state-dir marker, no replay); the file is still queued for the agent's next run.
• Unset (default): _fire_on_task returns at once. Behavior is unchanged.

No version bump: __version__ changes only in publish PRs (git log of ag2_sparrow/__init__.py: #2302, #3074).

Where to look first

1. packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py: _on_task_env, _fire_on_task, and the call at the end of _write_task.
2. packages/ag2-sparrow/tests/test_on_task_wake.py (wired into ci.yml next to test_write_task_atomic.py), plus tests/ag2-sparrow-on-task-wake.test.py, a runpy wrapper so the diff-coverage gate measures it. Same pattern as tests/ag2-sparrow-human-action.test.py.
3. packages/ag2-sparrow/README.md: new "Waking your agent" section.

User behavior this proves

An agent paired with Sparrow sets REMOTE_TASK_ON_TASK="my-agent run-task" and gets one my-agent run-task <task-file> per new task, with no folder-watching hook of its own.

Feature/documentation consistency

• packages/ag2-sparrow/README.md: "Waking your agent" section with an example.
• docs/remote-gateway-protocol.md (canonical for the remote gateway, per docs/catalog.json): a REMOTE_TASK_ON_TASK row in the config table. last_verified bumped to 2026-10-10.

Before / after evidence

New unit test at the parent commit 0f920a148 (origin/main), with the new test file copied in:

Traceback (most recent call last):
  File ".../packages/ag2-sparrow/tests/test_on_task_wake.py", line 191, in <module>
    test_fires_once_per_new_task_with_its_path()
  File ".../packages/ag2-sparrow/tests/test_on_task_wake.py", line 84, in test_fires_once_per_new_task_with_its_path
    assert len(calls) == 1, calls
AssertionError: []

At HEAD:

PASS test_fires_once_per_new_task_with_its_path
PASS test_unset_runs_nothing
PASS test_failing_or_slow_command (missing binary)
PASS test_failing_or_slow_command (nonzero exit)
PASS test_failing_or_slow_command (unparseable)
PASS test_failing_or_slow_command (slow)
PASS test_redelivery_and_restart_do_not_refire
PASS test_token_never_reaches_the_command
ALL PASS test_on_task_wake

End-to-end through main() against a local mock relay. The real python -m ag2_sparrow.remote_gateway_bridge long-polls a local HTTP server that serves one task on the first poll. Run 1 is a fresh start. Run 2 restarts Sparrow on the same dirs, and the relay redelivers the same task id. The command is a recorder script. Isolated env: HOME/dirs in a temp dir, SUTANDO_BRIDGE_LOCK=0, REMOTE_TASK_POLL_WAIT=1, 6 s per run.

Parent 0f920a148:

run 1: polls=7 acks=1 queued_lines=1 dedup_lines=0 task_files=['task-1786000000000.txt'] wake_calls_total=0
run 2: polls=7 acks=2 queued_lines=0 dedup_lines=0 task_files=['task-1786000000000.txt'] wake_calls_total=0

HEAD:

run 1: polls=7 acks=1 queued_lines=1 dedup_lines=0 task_files=['task-1786000000000.txt'] wake_calls_total=1
   call: {'argv': ['/var/folders/.../tasks/task-1786000000000.txt'], 'id': 'task-1786000000000', 'tok': []}
run 2: polls=7 acks=2 queued_lines=0 dedup_lines=0 task_files=['task-1786000000000.txt'] wake_calls_total=1

tok: [] lists every env var name containing TOKEN that the command saw.

Tests run

• Every packages/ag2-sparrow/tests/test_*.py and tools/test_no_drift.py: all pass.
• src/remote-gateway-bridge.test.py: the same 8 proactive-drain cases fail locally both at the parent and at HEAD (identical FAIL lines, diffed), so they come from this host's environment and are not caused by this change. CI is the reference.
• bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD): PASS (hardcoded-paths + root-artifacts + prose-cap clean).

Live path

This is the bridge's task-write path. The only live-path change is one call that returns at once when the setting is unset, which is the default and how every current deployment runs. Not done: a round trip against the real AG2 Space relay with the setting on. The end-to-end above uses a local mock relay. The live Sutando core was not restarted for this.

Stacked PR?

N/A.

Hardcoded-path check

No host paths were added. The test uses tempfile and sys.executable.

🤖 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

• packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py (85.7%): Missing lines 3315-3317
• packages/ag2-sparrow/tests/test_on_task_wake.py (97.0%): Missing lines 103-105,142

Summary

• Total: 156 lines
• Missing: 7 lines
• Coverage: 95%

packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py

Lines 3311-3321

  3311         _log(f"on-task command failed to start for {tid}: {type(exc).__name__}: {exc}")
  3312         return
  3313 
  3314     def _reap() -> None:
! 3315         code = proc.wait()
! 3316         if code != 0:
! 3317             _log(f"on-task command exited {code} for {tid}")
  3318 
  3319     threading.Thread(target=_reap, name=f"on-task-{tid}", daemon=True).start()
  3320 

---

packages/ag2-sparrow/tests/test_on_task_wake.py

Lines 99-109

   99         real = m.subprocess.Popen
  100 
  101 
  102         def spy(*a, **k):
! 103             if "SPARROW_TASK_ID" in (k.get("env") or {}):
! 104                 spawned.append(a)
! 105             return real(*a, **k)
  106 
  107         m.subprocess.Popen = spy  # type: ignore[assignment]
  108         try:
  109             tid, _ = m._write_task(_task())

---

Lines 138-146

  138                 assert len(files) == 1, f"{name}: {files}"
  139             if name == "nonzero exit":
  140                 deadline = time.time() + 10
  141                 while time.time() < deadline and not any("exited 3" in ln for ln in logs):
! 142                     time.sleep(0.05)
  143             if name != "slow":
  144                 assert any("on-task command" in ln for ln in logs), f"{name}: {logs}"
  145             print(f"PASS test_failing_or_slow_command ({name})")

---

[github-actions]

@cla-assistant check

[sonichi]

Reviewed at head e61ef7d6. Traced the claims directly against the diff rather than the body alone.

Fire-once-on-fresh-publish, confirmed against the real control flow, not just the prose. _write_task has three paths that return before _fire_on_task is ever reached: _task_pending(tid) (still queued/assigned/claimed), task_archived or _delivered_copy_exists(tid) (redelivery of already-handled work), and only the genuine fresh-publish path falls through to the write, _write_owner_activity, then _fire_on_task. So a redelivery or restart hitting either early return never re-fires — matches test_redelivery_and_restart_do_not_refire.

Credential stripping is belt-and-suspenders, not just name-matching. _on_task_env() drops REMOTE_*/AG2_* keys containing TOKEN, and separately drops any variable whose value contains one of the actual secret strings (TOKEN, _RAW, HS_MEDIA_TOKEN) — so a credential stored under a differently-named variable is still caught. test_token_never_reaches_the_command checks both the absent-key and absent-value-substring cases, and separately confirms an unrelated setting is not swept up, so this isn't over-redacting either.

Non-blocking and fail-safe is real, not asserted. Popen(..., start_new_session=True), stdio to /dev/null, a daemon reaper thread, broad exception catch around the spawn itself with one log line — never raises into the pull loop. test_failing_or_slow_command_never_blocks_or_loses_the_task backs this with a < 3s elapsed assertion across missing-binary, nonzero-exit, unparseable-command and slow-command cases, each also confirming the task file itself still lands durably regardless.

CODEOWNERS: this touches /packages/ag2-sparrow/ (owned solely by @qingyun-wu per the /packages/ CODEOWNERS line — not @sonichi) plus .github/, docs/, tests/ under the repo-wide//tests/ lines. Authored under the shared sonichi login, so my own review can't count as the gate-clearing approval here regardless — qingyun-wu's is the one this needs, and for the /packages/ path specifically, hers is the only valid code-owner approval, full stop.

The one real gap, and it matches this repo's own standard: the PR body is honest about it — "Not done: a round trip against the real AG2 Space relay... the live Sutando core was not restarted." This is the gateway bridge's task-write path, squarely a live delivery-loop change, and the mock end-to-end (real redelivery + restart against a local HTTP relay) is good but isn't the live witness this class of change needs per CONTRIBUTING.md and this session's own precedent on #5278/#5287/#5304 etc. Approve on that witness landing, same as the rest of today's live-path PRs.

Nothing else to push back on — the design (one optional setting, defaults to today's no-op behavior, argv via shlex with no shell) is minimal and well-scoped.

[sonichi]

Correction to my own comment above: on reflection (Chi asked directly whether a core restart is actually needed here, and that's the right question), demanding a live Sutando core restart is the wrong ask for this specific change.

Checked directly: nothing in this repo sets REMOTE_TASK_ON_TASK anywhere (grep -rln REMOTE_TASK_ON_TASK . outside this PR's own new files comes back empty; src/startup-runtime.sh exports REMOTE_TASK_TOKEN_FILE/REMOTE_TASK_TIER/REMOTE_MEDIA_MARKER but never ON_TASK). So Sutando's own live core runs with this setting unset today and would keep running with it unset after merge — _fire_on_task returns at the first line every single time, identical to before this PR. Restarting Sutando's core would exercise zero new code; it's not the right witness for a feature this repo's own process doesn't turn on.

The already-present end-to-end test is the right shape of evidence for what's actually new here: a real local HTTP server implementing the long-poll protocol, a real process restart against persisted dirs with real redelivery, and a real subprocess.Popen spawning a real recorder script — the only thing it substitutes is the production AG2 Space relay URL for a local mock of the same protocol. That's a materially different (and narrower) gap than "the live core wasn't restarted," and I shouldn't have echoed the PR body's own phrasing without checking whether it was the right test for this change.

Net: I'm dropping the "needs a core restart witness" ask. Nothing else in my review above changes.

[liususan091219]

Reviewed head e61ef7d6babca03382315d9b3f76244f2fca7567. I checked the code itself, not just the PR body. No change-request condition fires. The verdict is COMMENT only because the PR has no formal approval at this head yet (sonichi's review is COMMENTED and uses the shared login). As qingyun-wu is the sole CODEOWNER of /packages/, that approval has to come from a maintainer other than the author.

Claims checked against packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:

• No shell: _fire_on_task builds shlex.split(ON_TASK_CMD) + [str(path)] and calls subprocess.Popen(argv, ...). It never uses shell=True. Task content is never placed in argv. The command gets only the task-file path, plus SPARROW_TASK_ID / SPARROW_TASK_FILE in its environment.
• No argument injection: the path is TASKS_DIR / f"{tid}.txt". tid comes from _local_tid after _valid_tid ([A-Za-z0-9._-]{1,64}), and it always starts with task-. The leading part of the path is the operator's task dir, so a relay-supplied id cannot add a flag.
• Never blocks: Popen(start_new_session=True) with all three stdio streams on DEVNULL. A daemon thread waits on the child. Any exception at spawn time is logged once and returns.
• Credentials scrubbed: _on_task_env removes REMOTE_*/AG2_* names that contain TOKEN. It also removes any variable whose value contains TOKEN, _RAW or HS_MEDIA_TOKEN. TOKEN is a module global that is reassigned on rotation (line ~2278), so a rotated secret is still caught by value. Those three are the only relay secrets the module holds. The token read from the ag2space channel .env or the vault never enters os.environ.
• Fires once per task: the call is the last line of _write_task, after _publish_staged. The _task_pending path (queued/assigned/claimed) and the archived/delivered path both return before it. The task file is the dedupe state, so a restart or a redelivery does not fire again. The skipped-wake-on-crash trade-off is stated in the README.
• Vendoring rule: remote_gateway_bridge.py is package-canonical. src/remote-gateway-bridge.py is a 342-line shim that executes the package source, and sync_from_src.py deliberately leaves this module out of MAP. That means no private copy exists to drift. tools/sync_from_src.py --check reports "in sync with src/".

Tests run here: packages/ag2-sparrow/tests/test_on_task_wake.py ALL PASS, tests/ag2-sparrow-on-task-wake.test.py ALL PASS, test_write_task_atomic.py ALL PASS. The tests call the real _write_task and record the real child process. They do not grep the source.

Revert controls (each restored afterwards):

• Fire call removed: test_fires_once... fails with AssertionError: [].
• Value scrub removed: test_token_never_reaches_the_command fails.
• Reaper made synchronous: the slow-command case fails with "the pull loop waited 12.2s".
• Name scrub removed: still ALL PASS (nit 1).

CI: at 056c3b53 one check failed: diff coverage >= 95% (python), with remote_gateway_bridge.py at 33.3% and lines 3294–3319 uncovered. The package test ran outside the coverage job. 426e4633 added the runpy wrapper tests/ag2-sparrow-on-task-wake.test.py to fix it. At e61ef7d6, all 32 check runs succeed (2 skipped), and license/cla = success with 1 context. I found only one failure on this PR's commits, not two.

Named, not blocking: there is no live round trip against the real AG2 Space relay with the setting on. The body says so. The mock-relay end-to-end through main() is good evidence for the code path.

Nits (none blocking):

1. Nothing pins the name-based scrub on its own. In the test, every token variable also holds the secret value, so the value scrub covers them. An assert that REMOTE_TASK_TOKEN_FILE (a path, not a secret) is absent would pin it.
2. The README says "absolute path". It is absolute only if AGENT_CONNECT_TASK_DIR is. str(dest.resolve()) would make that true.
3. "Never sees the token" covers the environment only. The child runs as the same user and can read the token file. Worth one README clause so nobody reads this as a sandbox.
4. A command that never exits leaves one reaper thread per task for the life of the process.

— Sutando (susans-MacBook-Pro)

[sonichi]

Approving at e61ef7d6, on Chi's explicit word.

I recommend APPROVE #5317, authored by qingyun-wu. What it does (from the diff, not the title): adds an optional setting to the ag2-sparrow task bridge that runs a command once each time it writes a new task file, so an outside agent doesn't need its own folder-polling loop. Never blocks the main pull loop, runs without a shell, and strips every token-bearing env var by both name and actual secret value before the child sees it.

Files: packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py (the logic), two test files, README.md + docs/remote-gateway-protocol.md, and .github/workflows/ci.yml (wires the new test in).

Breaking change: none declared, none found — purely additive, off by default. Confirmed nothing in this repo currently sets the new variable, so merging changes zero behavior for any existing deployment, including this core's own.

Live test: no round trip against the real AG2 Space relay with the setting on — the PR names this gap itself. The existing mock end-to-end (real local HTTP relay, real process restart, real subprocess spawn) is strong substitute evidence, but neither review exercised the actual production relay. Not a blocker given the feature is opt-in and inert everywhere today.

Gate values at this head: CI 32/32 green including license/cla, no CHANGES_REQUESTED anywhere, two independent prior reviews (mine and Susan's) both clean. She's the sole CODEOWNERS owner of packages/ag2-sparrow/ and the PR's own author — the singleton-owner-blocked-on-own-PR case this repo's CODEOWNERS names, whose recovery is a bypass approval/merge by sonichi or qingyun-wu. This approval is that.