[sonichi]

Fixes #6199 (points 1–3; point 4 planned on the issue)

What changed, and why

Follow-ups from the code-owner review of #6151.

1. The seam test at the former final stop asserted nothing. test_seam_a_successor_at_the_final_manager_stop_is_never_stopped patched stop_supervisor, which the undo no longer calls, so its successor list stayed empty. It is replaced by test_seam_a_successor_at_the_former_final_stop_is_left_and_reported: it injects another actor's start exactly where the final stop used to be (after the retired run exited, before the manager's last read), and asserts that no stop_supervisor for the local shape runs after the start, that no stop verb reaches the manager, that the successor is alive and still the manager's run, and (connect) that the report names its pid.
2. A retire that ends in a restart was reported as "was stopped". job_identity now also reads how the last run ended, from the manager itself: systemd ExecMainCode/ExecMainStatus, launchd last exit code / last terminating signal, checked against each shape's restart policy (RestartPreventExitStatus=75; launchd KeepAlive/SuccessfulExit). _retire_run reports pid N ended with an exit its manager restarts, not as a retirement, so the job starts again after its restart delay, and an unreadable ending is reported as such, never as stopped. The undo's "could not be stopped" line now gives both the stop command and <command> uninstall.
   While writing the fail-first test I found the issue's premise does not hold today: cm.retired() keeps only cm.EXIT_RETRY (76), but the claude launcher's retry code is 74, so a retire whose stop_session failed exited 75. The manager then never restarted it, and the tmux session was left running with no launcher supervising it. retired() now takes the caller's retry code, and the launcher passes its 74. That makes the outcome the issue describes (the manager restarts the job, and the new launcher adopts the session) and the report says so.
3. _wire_bound's root-replaced/problem branch stopped the bridge by name (through unwire_bridge). It now goes through _unwire_ours, the rule the pending undo already uses: a registered or running bridge is left and reported with its stop command (and uninstall). Only an unregistered unit that still holds this attempt's bytes is removed. unwire_bridge stays for uninstall, which is a deliberate stop.

Where to look first

• packages/ag2-sparrow/ag2_sparrow/connect_common.py: job_identity (the new restarts field), _retire_run, _undo_start, retired
• packages/ag2-sparrow/ag2_sparrow/connect_bridge.py: _wire_bound
• packages/ag2-sparrow/tests/connect_undo_seams.py: the two seam tests (mixed into both the claude-local and codex-local suites)
• Fixtures: the fake managers now show the last exit (systemd properties, launchd line). The stand-in launchers have a <acted>.stop-fails switch that ends a retire with the launcher's retry code.

Behavior this proves

• A failed connect/install whose retired run ends with a restartable exit is reported as restarting, with the stop and uninstall commands, on systemd and launchd, for connect and install, for both adapters. After the report, the fake manager does restart it (respawns == 1).
• The claude launcher returns 74 (retry) for a retire whose session stop failed, and 75 when the stop succeeded.
• A successor started at the former final-stop seam survives and is reported. Nothing names the job to its manager after the start.
• In the root-replaced branch, a running bridge is left, nothing sends stop/bootout to it, and the report gives its stop command. On launchd, an unregistered bridge plist from a refused bootstrap is removed.

Documentation

N/A: no user-facing feature changes. Only the wording of the error lines a failed connect/install prints changes, and no canonical doc quotes those lines.

Before / after evidence

Fail-first (new and changed tests run against origin/main sources, with only the three source files reverted), excerpted:

FAIL: test_a_retire_whose_own_stop_fails_is_reported_as_restarting (how='connect', manager='systemd')
AssertionError: 'ended with an exit its manager restarts, ...' not found in 'error: ... this attempt was undone:
  ag2-sparrow-claude-local.service, the job its install started, was stopped; ...'
FAIL: test_a_retire_whose_own_stop_fails_is_reported_as_restarting (how='connect', manager='launchd')
AssertionError: ... 'ai.ag2.sparrow.claude-local, the job its install started, was stopped' ...
FAIL: test_a_retire_whose_session_stop_fails_asks_to_be_retried_not_retired [uncertain: x]
AssertionError: 75 != 74 : a session left running keeps its launcher's restart
FAIL: test_a_root_replaced_while_the_bridge_is_wired_leaves_the_running_bridge_and_unwinds_the_rest
AssertionError: 'the bridge this attempt wired (ag2-sparrow-claude-local-bridge.service) is active and was left, ...'
  not found in '... the bridge this attempt wired (ag2-sparrow-claude-local-bridge.service) was removed; ...'
FAIL: test_a_run_that_ended_asking_to_be_restarted_is_never_reported_stopped (restarts=True)
AssertionError: '' != 'pid 76798 ended ...'
ERROR: test_how_the_last_run_ended_says_whether_its_manager_restarts_the_job ... KeyError: 'restarts'

The codex-local suite fails the same way (how='connect' on systemd and launchd, and the replaced-root bridge test: 3 failures).

Mutations (applied at this head, then reverted):

• Point 1: re-add a final stop by name in _retire_run (stop_supervisor(shape) just before the manager's last read). The new seam test fails in all 4 subcases (connect/install × systemd/launchd) on both adapters: AssertionError: Lists differ: ['claude-local'] != [], and likewise ['codex-local'] != [].
• Point 3: re-add unwire_bridge(cfg, command, bridge) in _wire_bound's problem branch. The replaced-root bridge test fails on both adapters: AssertionError: 'the bridge this attempt wired (...-bridge.service) is active and was left, ...' not found.

At HEAD, every one of these passes.

Tests run

Each run used env -i, nice -n 19, the shared test lock and load < 10, with no live service or workspace touched:

test_claude_code_connect   Ran 136 tests  OK
test_codex_local_connect   Ran 122 tests  OK
test_claude_code_install   Ran 84 tests   OK
test_claude_code_adapter   Ran 143 tests  OK
test_bridge_exclusive      Ran 22 tests   OK
test_adapter_boundaries    Ran 13 tests   OK
test_task_lifecycle        Ran 282 tests  OK
test_wheel_contents        rc=0
tests/ag2-sparrow-claude-local-bridge.test.py  22 OK
tests/gateway-credential-sources.test.py       179 OK
tests/sparrowd-agent-bridges.test.py           50 OK
tests/dotspace-literal-guard.test.py           7 OK
tools/test_no_drift.py     PASS
scripts/review-checks.sh   PASS (hardcoded-paths + root-artifacts + prose-cap clean)

Live path

N/A. This changes the error-path undo of the connect/install CLIs, not a running bridge, delivery loop or startup. Both managers are exercised through real child processes (the launchd branch with its shim) under fake manager front-ends.

Residuals

• The restart is read from the manager's record of the last exit. When that record cannot be read (systemd ExecMainCode=0, launchd (never exited)), the line says so and does not claim a stop.
• If the manager's restart delay has already passed when the undo reads the job, the new run's pid is reported (as before) as "runs again as pid N, which this operation did not start".

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

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

Diff Coverage

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

• packages/ag2-sparrow/ag2_sparrow/adapters/claude_code/launcher.py (100%)
• packages/ag2-sparrow/ag2_sparrow/connect_bridge.py (100%)
• packages/ag2-sparrow/ag2_sparrow/connect_common.py (100%)
• packages/ag2-sparrow/tests/claude_code_fixtures.py (100%)
• packages/ag2-sparrow/tests/connect_undo_seams.py (97.9%): Missing lines 252
• packages/ag2-sparrow/tests/test_claude_code_connect.py (100%)
• packages/ag2-sparrow/tests/test_codex_local_connect.py (100%)

Summary

• Total: 131 lines
• Missing: 1 line
• Coverage: 99%

packages/ag2-sparrow/tests/connect_undo_seams.py

Lines 248-256

  248                         return done
  249 
  250                     def stop_seen(shape, *a, **kw):
  251                         if shape.name == self.local.name and STARTS & {fake.runner.verb(c) for c in fake.runner.calls}:
! 252                             stops.append(shape.name)
  253                         return real_stop(shape, *a, **kw)
  254                     refused = []
  255                     with mock.patch.object(cm, "is_macos", return_value=mac), \
  256                             mock.patch.object(cm, "_await_exit", side_effect=exited_then_replaced), \

---

⚠️ Not measured

Changed, but absent from coverage.xml — outside [run] source in
.coveragerc, so diff-cover never examined these lines:

• packages/ag2-sparrow/tests/test_claude_code_adapter.py

[liususan091219]

Ready from my side; not approving yet because the approve bar isn't met: this PR has no approval from an ag2-sparrow expert. @qingyun-wu, who wrote the #6151 code-owner review these three points come from, is the natural one to give it.

Verified at 8db3232 (base 1f7f639 = current main), in throwaway worktrees:

• Head: test_claude_code_connect (136), test_codex_local_connect (122), test_claude_code_adapter (143) and test_bridge_exclusive all OK locally. CI is green, and the license/cla status is present and success.
• Fail-first: I ran the head tests against the base ag2_sparrow/ sources. The failures match the PR body: claude-local 6 failures and 9 errors (KeyError: 'restarts', the restart-report test on systemd and launchd, the replaced-root bridge test, the unregistered-plist test), codex-local 3 failures, and adapter 1 failure (75 != 74, the session-stop-failed case).
• Mutations: I re-added stop_supervisor(shape) before the last job_identity read in _retire_run. The new seam test then fails in all 4 subcases on both adapters (['claude-local'] != [], ['codex-local'] != []). I also re-added unwire_bridge(...) in _wire_bound's problem branch, and the replaced-root bridge test fails on both adapters. So point 1's test now really pins that nothing stops the job by name, which the old test did not.
• Point 2 logic: I checked it against what the managers do. systemd's unit has Restart=always and RestartPreventExitStatus=75, so only exit code 75 reads as not restarting. On launchd, the --launchd shim maps 75 to 0, and KeepAlive={SuccessfulExit: false} makes success == keep the right predicate. An unreadable ending is reported as unknown, never as stopped.
• The 74-vs-76 fix: retired() used to keep only cm.EXIT_RETRY (76) while the claude launcher's retry code is 74, so a retire whose session stop failed was held at 75 and left unsupervised. The fix is real. The codex connector calls retired() with the default, which is correct because it uses the shared retry code. cm.retired( has no other caller.
• Point 3: _wire_bound now goes through _unwire_ours, the same rule the pending undo uses. unwire_bridge stays only for uninstall.

The tests drive the real CLIs against fake manager front-ends and assert on behavior. None of them reads module source text or reimplements the unit under test. No hardcoded paths are added. A missing live round trip is not blocking here, since the change only touches the error-path undo.

— Sutando (susans-MacBook-Pro)

[qingyun-wu]

Reviewed at 8db3232. Approving.

Undo never stops a start it did not make. _undo_start -> _retire_run still signals only the exact run (pid + start time), and no step names the job to its manager. The replaced seam test (connect_undo_seams.py:233) injects another actor's start right after the retired run exits, and asserts no stop_supervisor for the local shape and no stop verb after the start. It passes on base too, because main already has the earlier fix, so it pins the behavior rather than failing first. Its proof is the mutation: re-adding stop_supervisor(shape) before the final read fails it on both adapters. test_seam_a_successor_after_the_signal_is_never_stopped still passes.

The restart report is truthful. connect_common.py:870-892: on systemd, ExecMainCode 1/2/3 maps to "restarts" except exit 75 (matches Restart=always + RestartPreventExitStatus=75 in render_service). On launchd, success == keep_alive_on_success matches KeepAlive {SuccessfulExit: false} together with launchd_exit mapping logical 75 to 0. A missing record (ExecMainCode=0, (never exited)) gives None, and _retire_run (:1441) then reports "cannot be read" instead of "stopped". I checked the last exit code = N / (never exited) line format against a real launchctl print on macOS, and the regex matches it.

Retry code. retired(..., retry=EXIT_RETRY) in launcher.py:623 keeps the claude launcher's 74. I confirmed that reverting it fails test_a_retire_whose_session_stop_fails_asks_to_be_retried_not_retired. The codex default of 76 is unchanged.

Bridge. _wire_bound (connect_bridge.py:385) now goes through _unwire_ours, so a registered or running bridge is left and reported with its stop and uninstall commands. Only an unregistered unit that still holds this attempt's bytes is unlinked. unwire_bridge remains only on the uninstall paths.

Tests (local, high load): test_bridge_exclusive and test_claude_code_adapter OK. In the connect suites, test_a_holder_that_takes_the_root_after_the_clean_check_keeps_the_seat fails identically on origin/main (a host Python cache file under the fake home), so it is environmental. test_a_launcher_holding_the_agent_lock_past_the_wait_is_left_and_reported failed once on its 2 s wait under load and passes in isolation. CI: 45 pass, 3 skipped, none failing.

Non-blocking: on systemd, _retire_run could compare after["since"] with ident["since"] before using ExecMainCode/ExecMainStatus. If a successor started and exited inside the window, the exit record is the successor's, and the line would attribute its ending to this operation's pid. The window is narrow and the restart claim still holds for the job, so this is wording only.