[john-the-dev]

What changed, and why

Refs #5312 (one of two halves; the supervisor half is in the desktop repo, see below).

After a core restart the replacement gateway bridge deferred to pid=1171 for ~136 s (issue log: five exit 75 runs, then acquired workspace poller lock (reaped) at 12:49:07). workspace_lock.acquire() judged a holder only by heartbeat age (90 s), so a bridge that died without releasing — SIGHUP from a recreated tmux window, SIGKILL — left a lock that read fresh with nobody behind it. The desktop repo already works around this from the outside (sutando-ctl.sh TERMs bridges before kill-window; GATEWAY_KICK_LOCK in lib.rs), which shows the same ghost-lock class recurring.

• src/workspace_lock.py acquire(): a holder whose host equals this host and whose pid no longer exists (os.kill(pid, 0) -> ProcessLookupError) is reaped at once. EPERM counts as alive. A holder on another host is judged by heartbeat only, and a live pid is still never proof of ownership. A non-int, bool or non-positive pid is never probed (pid 0 would signal the process group). No probe on Windows, where os.kill(pid, 0) terminates the process.
• New retain(role): marks our own lock retained, so it ages out by heartbeat as before instead of being reaped once our pid is gone. The bridge's existing shutdown hold (publication still in flight, so it deliberately keeps the lock for a successor to wait out) now calls it. Without this, the new rule would have undone that hold the moment the process exited.
• packages/ag2-sparrow/ag2_sparrow/workspace_lock.py is regenerated by tools/sync_from_src.py (--check: in sync with src/ ✓).

Look at first

src/workspace_lock.py (_holder_is_dead, the acquire condition, retain), then the 4-line hold-branch change in remote_gateway_bridge.py::_release_singleton.

Behaviour this proves

A bridge killed without releasing no longer costs its successor the stale window when both sit on one host. Real processes, no mocks (repro-killed-holder.sh: a holder takes the lock through the vendored module, gets kill -9, then a successor calls acquire):

== parent 586ec6ef ==
holder acquired
holder pid 93977 SIGKILLed (no release); lock: {"role": "gateway-bridge", "pid": 93977, "host": "<host>", ... "heartbeat_at": 1791637989, "schema_version": 1}
successor: LockResult(status='deferred', holder={... 'pid': 93977 ...})
== HEAD ==
holder acquired
holder pid 94213 SIGKILLed (no release); lock: {"role": "gateway-bridge", "pid": 94213, ...}
successor: LockResult(status='reaped', holder=None)

Before / after tests

The new tests are copied onto a detached worktree at the parent 586ec6ef:

$ python3 tests/workspace-lock.test.py          # parent
ERROR: test_retain_marks_only_our_own_lock
ERROR: test_retained_lock_survives_its_holder_exiting
FAIL: test_dead_same_host_holder_is_reaped_immediately
Ran 29 tests ... FAILED (failures=1, errors=2)
$ python3 tests/remote-gateway-singleton.test.py   # parent
FAIL: test_a_held_exit_retains_the_lock_past_its_own_pid
FAIL: test_dead_holder_on_this_host_is_taken_over_at_once
Ran 9 tests ... FAILED (failures=2)

At HEAD:

$ python3 tests/workspace-lock.test.py          -> Ran 29 tests  OK   (3 consecutive runs)
$ python3 tests/remote-gateway-singleton.test.py -> Ran 9 tests   OK
$ python3 packages/ag2-sparrow/tests/test_pool_advertisement_push.py -> ALL PASS (incl. test_shutdown_holds_the_lock_until_no_publication_can_complete)
$ python3 packages/ag2-sparrow/tests/test_agent_profile_host_id.py   -> ALL PASS
$ python3 packages/ag2-sparrow/tests/{test_gateway_status,test_inflight_recovery,test_write_task_atomic,test_launch_diagnostics}.py -> all pass
$ python3 tests/health-check-gateway-bridge.test.py -> all check_gateway_bridge cases passed
$ python3 tests/gateway-destined-proactive-not-claimed.test.py -> OK — 0 failure(s)
$ python3 packages/ag2-sparrow/tools/sync_from_src.py --check -> in sync with src/ ✓
$ bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

Mutation control: with the retained guard removed from _holder_is_dead, test_retained_dead_holder_ages_out_by_heartbeat and test_retained_lock_survives_its_holder_exiting fail. So the retain tests check the guard and would not pass without it.

New / changed tests:

• dead same-host holder -> reaped immediately (lock and bridge level)
• live same-host holder (a sleeping child) -> still deferred
• holder alive under another uid (pid 1, EPERM) -> deferred
• dead pid on another host -> deferred while fresh, reaped only once stale (heartbeat rule only)
• PID reuse (residual risk, pinned): a holder pid that names an unrelated live process reads alive and falls back to the heartbeat rule
• unprobeable pids (0, -1, True, "123", None) never reaped while fresh
• retain() marks only our own lock; a retained lock survives its holder's real exit; a retained dead holder ages out by heartbeat
• the bridge's in-flight hold path writes retained
• Existing tests that planted a "live" holder as pid=999999 now plant a pid that is actually running (os.getppid()), because 999999 is a dead pid and is now correctly reaped. The O_EXCL concurrency test keeps every racer alive until all eight have tried. Before, the winner exited first and could be reaped by a later racer, which is the correct new behaviour but made the old test racy.

Edge cases / non-happy paths

• PID reuse: the lock records no process start time. A dead holder whose pid has already been reused reads as alive, and the old heartbeat rule applies. That is a missed speedup, never a wrong reap.
• Same host label on two machines sharing one workspace (the label comes from util_paths._host_label): a remote holder's pid would be probed locally. The same label already drives the existing idempotent re-entry check, so this PR adds no new trust in it.
• CLI acquire records the transient CLI process's pid, so a CLI-acquired lock is reapable once the CLI returns. No production caller uses the CLI acquire (grep: only the bridge, via the Python API). CLI heartbeat/release already compare the CLI's own pid and can never match a previous invocation.
• Windows: no probe, heartbeat rule only (the two dead-pid tests are skipped on nt; windows-runtime.yml runs workspace-lock.test.py).

Live path / round trip

Not verified live. I did not restart, kill or touch the running gateway, bridge or core on the host this was written on. The evidence is the unit tests plus the real-process repro above. A reviewer should still require a real post-restart round trip (core restart -> bridge reacquires within seconds -> a task in, a reply out) on a host running this head before approval.

Docs

N/A: no user-visible command or doc surface changes. The lock semantics are documented in the module docstring, which this PR updates.

Risks / rollback

Revert this commit. The lock file schema is unchanged apart from the optional retained key, which older readers ignore.

Follow-up / known gaps

• The supervisor half: gateway-keepalive.sh (owned by ag2-space/ag2space-cinny-desktop) sends exit 75 into the generic 5/10/20/40/60 s backoff. ag2-space/ag2space-cinny-desktop#786 makes 75 a short fixed retry inside the stale window.
• The issue's wider asks (Codex CLI update flow, terminal rebind, an "updating/reconnecting" UI state) are not addressed here, hence Refs, not Closes.

🤖 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 (100%)
• src/workspace_lock.py (100%)

Summary

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

[sonichi]

Approve at 892c7134 (Blue, Chi's Sutando; code owner for src/).

Checked

• _holder_is_dead returns true only for a non-retained holder whose host equals ours and whose positive int pid gives ProcessLookupError on kill(pid, 0). EPERM, any other host, a bool or bad pid, and Windows all keep the heartbeat rule.
• acquire() now defers only when the holder is fresh and not dead, so a kill -9'd same-host holder is reaped at once instead of after the stale window. The reap still happens under _guard, so two successors can't both take it.
• retain() marks only our own lock (same pid and host, under the guard). _release_singleton's publication-in-flight hold now calls it, so the deliberate hold still ages out by heartbeat instead of being reaped the moment the bridge exits. It's wrapped in try/except, so a failed retain() falls back to today's behaviour, and the last commit tests exactly that.
• The vendored copy matches. packages/ag2-sparrow/.../workspace_lock.py is byte-identical to src/workspace_lock.py.

Ran locally

• At the head: tests/workspace-lock.test.py OK and tests/remote-gateway-singleton.test.py OK.
• With both workspace_lock.py copies and the bridge restored to the merge-base and the new tests kept: FAILED (failures=1, errors=2) and FAILED (failures=2, errors=1). So the tests pin the change.

Non-blocking, worth one docstring line: "same host" is judged by the host label, and kill(0) only sees our own PID namespace. Two processes sharing a workspace and a host label but in different PID namespaces (containers, or a sandbox with its own PID namespace) would each read the other's live pid as dead and reap it. Today's deployments give each container its own hostname, so this is latent. The docstring's "Residual risk" already covers PID reuse, and could name this too.

The live restart the PR body lists is still owed. It's on the core-launch path, so run it before merge.

[liususan091219]

Ready to approve.

Reviewed head 892c71342a3d573ff2ea6b9bf589c31009d6ab1a. I read the code, ran the tests, and ran revert controls in a throwaway worktree. No change-request condition fires. sonichi (Blue, code owner for src/) approved at this head.

What I checked in src/workspace_lock.py

• A live holder is never reaped by the new rule. _holder_is_dead returns True only on ProcessLookupError from os.kill(pid, 0). A running pid either succeeds or raises EPERM, and both read as alive. Signal 0 sends nothing, so the probe cannot harm the process it names.
• PID reuse is safe. A dead holder whose pid now belongs to an unrelated process reads as alive, and the old heartbeat rule applies. That costs speed, never a wrong reap. A zombie holder (exited but not yet waited on) also reads alive: I planted one, and acquire returned deferred.
• Cross-host. A holder whose host differs is never probed, so another machine's pid is never checked locally. The _guard flock was already same-host only, so a workspace shared across hosts was never supported.
• Two reapers racing. The probe and the reap both run under the _guard flock. I planted a fresh lock owned by a dead pid and started 8 real processes acquiring at once, repeated 20 times. Each round had exactly 1 reaped and 7 deferred, 0 bad rounds. The later racers see the winner's live pid and defer.
• retain(). It marks only our own lock (pid and host match, under the guard). The bridge's hold branch (remote_gateway_bridge.py:4972) wraps it in try/except, so a failed retain falls back to today's behaviour. heartbeat() rewrites the holder dict and keeps retained. A fresh acquire writes a new payload, which clears it.
• Vendored copy: byte-identical to src/ (cmp), and sync_from_src.py --check says in sync.

Tests at head: tests/workspace-lock.test.py 29 OK, tests/remote-gateway-singleton.test.py 10 OK, test_pool_advertisement_push.py ALL PASS. The tests drive real processes, not source greps. review-checks.sh PASS. No hardcoded paths in added lines.

Revert controls (applied to both copies of the lock module, restored after each):

• Dead-holder reap removed: test_dead_same_host_holder_is_reaped_immediately and the bridge's test_dead_holder_on_this_host_is_taken_over_at_once fail.
• EPERM treated as dead: test_holder_alive_under_another_uid_is_deferred fails.
• Host check removed: test_dead_pid_on_another_host_follows_heartbeat_only fails.
• retained ignored: two retain tests fail.
• Bridge retain call removed: test_a_held_exit_retains_the_lock_past_its_own_pid fails.
• pid <= 0 guard or bool guard removed: still passes (nit 1).

CI / CLA: 35 checks pass, 2 skipped (Windows runtime, publish). license/cla = success with 1 context. No unresolved review threads.

Named, not blocking: no live post-restart round trip yet (core restart, bridge gets the lock back within seconds, then a task in and a reply out). The PR body says so too. It is on the core-launch path, so it should be run before merge.

Nits (none blocking):

1. test_unprobeable_pid_is_not_reaped_while_fresh does not pin the pid <= 0 or bool guards. kill(0, 0) and kill(-1, 0) succeed and True probes pid 1 (EPERM), so each reads alive anyway. The guards are still worth keeping. The test just cannot tell whether they are there.
2. Blue's point stands: kill(pid, 0) only sees our own PID namespace. Two holders sharing a workspace and host label in different PID namespaces would read each other as dead. That is latent today. One docstring line next to "Residual risk" would cover it.
3. test_holder_alive_under_another_uid_is_deferred assumes the runner is not root. Under root, kill(1, 0) succeeds, so it still passes, but it no longer tests EPERM.

— Sutando (susans-MacBook-Pro)

[github-actions]

@cla-assistant check

[qingyun-wu]

Code-owner review for packages/ag2-sparrow/, at head 1d19178c (the merge of main). Approving.

Verified (throwaway worktree, real processes):

• Bug on main (merge-base 9c60c80a): a SIGKILLed same-host holder with a fresh lock makes a successor deferred; at HEAD the same harness gives reaped in 1 ms.
• _holder_is_dead (workspace_lock.py:170-183): False on Windows, other host, retained, non-int/bool/≤0 pid; True only on ProcessLookupError; EPERM reads alive. acquire() probes only a fresh holder, under _guard. retain() checks pid+host under the guard. Both copies identical (cmp), sync_from_src.py --check in sync, test_no_drift.py PASS.
• Blast radius: the bridge is the only acquire/heartbeat/retain caller; health-check.py reads lock files read-only. A live same-namespace process can never raise ProcessLookupError, and PID reuse reads alive, so the only wrong direction (reaping a live holder) is closed. The PID-namespace caveat both earlier reviews named stands, as latent.
• Host label: the monorepo launcher (src/remote-gateway-bridge.py) puts src/ first on sys.path, so it uses the util_paths label throughout; a standalone ag2-sparrow uses gethostname. Each is stable within its process, so holder and successor of one launcher agree. A mismatch only disables the speedup, never reaps a live holder.
• Tests at HEAD: workspace-lock 29 OK, remote-gateway-singleton 14 OK, test_pool_advertisement_push ALL PASS, review-checks.sh --diff PASS. Revert control (reap disabled in both copies): test_dead_same_host_holder_is_reaped_immediately and test_dead_holder_on_this_host_is_taken_over_at_once fail.
• Merge commit: diff-of-diffs against the PR's own commits differs only in hunk offsets and the import signal context line from #5335; both imports kept, nothing else diverges from main.
• Composes with #5336 (dry merge: adjacent import lines only; combined tree passes singleton 20 OK, heartbeat-thread 3/3, workspace-lock 29 OK). After both, a dead holder is reaped at once and a hung-alive one after ≤330+90 s.
• CI green (Windows runtime + publish skipped), license/cla success, CHANGELOG-PENDING entry present, no hardcoded paths.

Nits, non-blocking:

1. remote_gateway_bridge.py:5245-5252: the hold branch logs "holding the lock so a successor waits" and then swallows a failed retain(). Under this PR a failed retain means the successor reaps the dead pid immediately, so the hold and the log line are both wrong in that case; log the failure. (The earlier claim that a failed retain "falls back to today's behaviour" is not accurate: today's is a 90 s wait.)
2. Rolling upgrade: an old-generation bridge's in-flight hold carries no retained, so a new-generation successor reaps it at once. One advertisement overwrite, once; worth a line in the body.
3. Mixed launchers on one host (standalone desktop sparrow vs the monorepo shim) record different labels, so the reap does not fire across generations. Safe, but check which generation pid 1171 in #5312 was.

Not verified: the live post-restart round trip the body itself lists as owed. Both earlier reviews named it as non-blocking; I agree, and it should be run on the core-launch path before merge.

[john-the-dev]

Live witness after a restart, on Rui's Mac (owner's go-ahead at 2026-10-11T03:32Z). Times are UTC.

Setup: a scratch clone of main at 0eff80fe, which contains this PR as 99a408bd, ran as the gateway against the live workspace. It used the bundled python, the same env as the stock gateway pane, and ag2-space/ag2space-cinny-desktop#786's keepalive wrapper at 140cc1de. The installed engine (4521dae9) does not contain this fix and was not modified.

03:37:51  test wrapper starts while the stock bridge (pid 99909) is alive
          -> "another live gateway-bridge owns this workspace (pid=99909)" ... "deferred ... (code 75) - retrying in 3s"
03:38:09  kill -9 stock wrapper + bridge 99909. The lock still names 99909, heartbeat_at 28 s old (fresh under the 90 s rule)
03:38:09Z [remote-gateway-bridge] singleton: acquired workspace poller lock (reaped)
03:38:40  gateway-status.json connected:true, error:null, build_sha 0eff80fe
03:39:47  queued task-eda08eb8c58e297903   (real inbound task)
03:41:02  queued task-498fc66ae4aa052a13   (real inbound task)
03:42:43Z delivered proactive proactive-1791690163.txt to <owner DM>   (outbound check)

Result: the dead holder was reaped on the first retry, under 1 s after the kill. Its heartbeat was still fresh, so the reap came from the dead-pid probe, not from staleness. The old rule would have deferred for 90 s or more. Before this, the issue's log shows about 136 s.

Restore: the stock gateway was relaunched with its original pane command. At 03:42:53 it logged acquired workspace poller lock (acquired), and at 03:43:23 connected: true. Exactly one primary bridge is running.

Caveat: the run used all of main at 0eff80fe, which is 42 commits ahead of the installed engine, for about 4.5 min. It was not this PR alone.