fix(cli): report incomplete stops and bound port inspection

[kovtcharov]

Port discovery and forceful termination are bounded, and partial stops report every killed, refused, and failed listener. Both CLI stop paths return failure when the stop fails; an already-free port remains a successful no-op.

Fixes #3565, fixes #3564, fixes #3570.

Test plan:

• Port helper and existing CLI refusal/exit-contract suites: 64 passed.
• Real owned Python listener stopped successfully; real empty-port behavior is exercised by the CLI contract suite.
• Platform-shaped process listings, timeouts, mixed outcomes, and both Lemonade fallback branches covered.
• Formatting/import checks, whitespace checks, and independent review passed.

[github-actions]

Request changes

This bounds the port-listing subprocesses at five seconds and makes a partial stop report every killed / refused / failed process instead of claiming success — both good fixes, and the new platform-shaped test fixtures are the strongest part of the PR. Two behaviour questions should be settled before merge.

gaia kill --port N now exits 1 when nothing is listening on that port. Running it to free a port before starting a service is the common case, and "the port was already free" is now indistinguishable from "the stop failed" — any gaia kill --port N && start-something or set -e script breaks the moment the port is idle. Decide whether an empty port counts as success, and document whichever you pick; the docs note added here covers partial stops but not this case.

gaia kill --lemonade still exits 0 when its stop fails. When the lemonade-server command is missing or fails, it falls back to the same port kill, prints ❌ on refusal or failure, and returns success — exactly the "gaia kill && next-step runs having killed nothing" problem this PR fixes for --port, left in place one branch over.

Worth noting too: the hang this closes is only in the listing half. The kill itself still runs without a timeout, so a wedged taskkill can still block the command indefinitely.

Real-world evidence

No evidence bundle was produced for this PR, and the description's test plan lists unit tests only — there's no captured gaia kill --port output. This change alters what a user sees at the terminal (new exit code, new combined message), so the three interesting cases are worth pasting in: a successful kill, a port with nothing on it, and a refused non-GAIA listener. My verdict here rests on static review and reading the new tests; I could not execute them (no pytest in the reviewer environment), so the assertions were checked by hand against the code rather than run.

🔍 Technical details

🟡 Important

Empty port now exits nonzero (src/gaia/cli.py:4551, surfaced by the new sys.exit(1) at src/gaia/cli.py:3597)

kill_process_by_port has always returned success: False for "No process is listening on port N", but until this PR that only affected the printed glyph. Now it terminates the CLI with status 1. gaia api stop already behaved this way, so consistency is a defensible answer — but it's a new, undocumented contract for gaia kill, and the failure mode (a script that stops working only when the port happens to be free) is the kind that shows up in CI rather than in review. Either treat "nothing to stop" as success, or add a sentence to the docs note stating that gaia kill --port exits 1 when nothing is listening.

A related edge falls out of the same success expression at src/gaia/cli.py:4582: if two processes hold the port (SO_REUSEPORT workers, or a supervisor and its child) and killing the first makes the second exit, kill -9 on the now-dead pid raises CalledProcessError, lands in failed, and the command reports failure and exits 1 even though the port is free.

--lemonade failure path doesn't propagate (src/gaia/cli.py:3576-3589)

Both fallback blocks print the failure and fall through to return:

                port_result = kill_process_by_port(13305)
                if port_result["success"]:
                    print(f"✅ {port_result['message']}")
                else:
                    print(f"❌ {port_result['message']}")
                    sys.exit(1)

The same two added lines are needed in the result.returncode != 0 block at 3576 (indented one level deeper). The comment at 3598 — "A refusal must not report success" — is the argument for it.

🟢 Minor

terminate_pid is still unbounded (src/gaia/ports.py:202-207). Issue #3565 is about the command hanging forever; listing is now capped but signalling isn't. src/gaia/api/app.py:238 already passes timeout=5 to its taskkill:

def terminate_pid(pid: int) -> None:
    """Terminate ``pid`` with the platform's forceful kill."""
    if sys.platform.startswith("win"):
        subprocess.run(
            ["taskkill", "/PID", str(pid), "/F"], shell=False, check=True, timeout=5
        )
    else:
        subprocess.run(["kill", "-9", str(pid)], shell=False, check=True, timeout=5)

If you take this, widen the handler at src/gaia/cli.py:4563 to except (subprocess.SubprocessError, OSError) — TimeoutExpired is not a CalledProcessError, so it would escape the loop uncaught and traceback out of gaia kill.

Second Kill section in the CLI reference wasn't updated (docs/reference/cli.mdx:3149-3152). cli.mdx documents gaia kill twice; the duplicate still says only "Provide feedback about success or failure". Per CLAUDE.md's rule that a functional change updates every doc describing it, that one needs the new exit-code behaviour too (or the duplicate section should go).

Singular wording on a multi-pid failure (src/gaia/cli.py:4579). The message now composes with the killed/refused lines, so "the process" reads wrong when several failed:

            f"Failed to kill process(es) on port {port} ({'; '.join(failed)})."

Strengths

• The new tests go after the right thing: test_process_image_parser_from_platform_output drives the identity gate with real tasklist/ps output shapes including the INFO: No tasks… case, and test_process_image_lookup_failure_is_never_killable pins the fail-closed property that keeps a destructive command safe when the lookup breaks. That's the gap The identity gate guarding the destructive port-kill is never tested against real tasklist/ps output #3570 named.
• test_cli_exit_status_matches_stop_outcome drives main() through sys.argv rather than calling the handler directly, so the exit code is tested on the path a user actually hits.
• Swapping ports.py to gaia.logger.get_logger brings the module in line with the rest of src/gaia/.

[kovtcharov]

Addressed the stop-path feedback. Empty ports remain successful no-ops, while refusals, partial stops, and termination timeouts exit nonzero; both Lemonade fallback paths propagate failure. The helper and existing CLI exit-contract suites pass together: 64 tests. A real owned-listener smoke test also confirmed successful termination. The CI-reported empty-port regression is corrected in e78a40f.

[github-actions]

Verdict: Approve with suggestion

The gaia kill hardening — timeouts, partial-kill reporting, idempotent empty-port, and exit-status propagation — is logically sound. The implementation is clean and the test suite covers all new code paths well.

🟡 CLI evidence missing from PR description. The changed surface is gaia kill --port and gaia kill --lemonade; per project rubric, a CLI change needs the real command and its output on the PR. If the description already shows this (I couldn't access it from the review environment), disregard.

Everything else checks out:

• success = bool(killed) and not refused and not failed handles every combination correctly — partial kills (some refused or failed) exit 1, full no-op (empty port) exits 0.
• subprocess.TimeoutExpired is a SubprocessError subclass, so it's caught by the existing except (OSError, subprocess.SubprocessError) handlers in both kill_process_by_port and process_image_name — no unhandled exception path.
• Widening terminate_pid's catch from CalledProcessError to SubprocessError correctly captures TimeoutExpired from the new 5-second kill timeout.
• Refused-process entries include the name (f"{pid} ({name or 'unknown process'})") — matches the test assertions.
• Docstring Raises block updated to include TimeoutExpired. ✓

🔍 Technical details

ports.py:153 — docstring still says subprocess.CalledProcessError: the listing tool failed outright as the second raise; TimeoutExpired was correctly added as a third entry in this diff — no action needed, just confirming it's there.

cli.py:4554-4555 — not listeners → success=True is the idempotent no-op path; the test test_no_listener_is_successful_noop covers it.

test_kill_process_by_port.py:248 — assert "Refusing to kill 202 (nginx)" in result["message"] will match because refused.append(f"{pid} ({name or 'unknown process'})") produces "202 (nginx)" — confirmed in cli.py:4562.

[kovtcharov]

Here is real gaia kill --port output from this branch on macOS for the three cases asked about: a free port exits 0, a GAIA-shaped (Python) listener is killed and exits 0, and a non-GAIA listener is refused and exits 1. Current main is merged in (43cba48); no code changed.

$ gaia kill --port 51980          # nothing listening
✅ No process is listening on port 51980
exit=0

$ gaia kill --port 51981          # python -m http.server started for this test
✅ Killed process(es) 96231 listening on port 51981.
exit=0

$ gaia kill --port 51982          # a perl socket listener started for this test
❌ Refusing to kill 96350 (perl) on port 51982: not a GAIA or Lemonade process. Stop it with its own tooling, or kill it by PID if that is really what you want.
exit=1

gaia kill --lemonade was not run: it would stop the Lemonade Server a benchmark on this machine is using. It needs a maintainer run on a box where stopping Lemonade is safe.

🔍 Technical details

• Run as PYTHONPATH=<checkout>/src python -m gaia.cli kill --port N so it used this branch's code; the log line points at cli.py:3795 in this checkout. The INFO log line before each result is omitted above.
• Ports were OS-assigned free ports, and lsof -nP -iTCP:<port> -sTCP:LISTEN confirmed each was empty or held only the test listener before the run. Afterwards lsof showed 51981 free and the http.server PID gone. The refused perl listener was stopped by PID afterwards.
• pytest tests/unit/cli/ -q: 265 passed, 4 failed. The 4 are test_gaia_binary_on_path[*], which fail identically on origin/main because the venv's bin/ isn't on PATH here.

[kovtcharov-amd]

Merged main in — 24 commits behind, no conflicts. The port-inspection and stop-reporting changes are unaffected.

🔍 Technical details

Merge commit only — no hand resolution. Net vs main: src/gaia/cli.py, src/gaia/ports.py, docs/reference/cli.mdx, tests/unit/cli/test_kill_process_by_port.py (+206/−28).

Tests on the merge result: tests/unit/cli/test_kill_process_by_port.py — 60 passed. The whole tests/unit/cli/ run has 10 failures, all in test_cli_smoke.py and all "console script not on PATH" — they reproduce identically on untouched main in this environment (no editable install of the extra entry points), so they are environmental, not from this branch.