Port upstream 0.69.0: reap Antigravity CLI usage probe process tree

[Finesssee]

Summary

The Antigravity CLI usage fallback (agy --version, agy -p /usage) now runs each probe inside its own Windows Job Object with kill-on-close. tokio kill_on_drop only terminates the direct child, so MCP server descendants started by agy outlived the probe (after success, error, or the 90s timeout). Dropping the probe's job now terminates the whole probe process tree. Only processes in the probe's own job are affected; unrelated agy processes are never touched.

Upstream reference

• CodexBar 0.69.0 release item: Antigravity probe reaping (upstream PR Antigravity: reap MCP servers left behind by the usage probe steipete/CodexBar#4077).
• Local spec: port-audit/0.69.0.md, PR 10 (antigravity-cli-usage-reap).
• Windows adaptation: upstream reaps by process/cwd on macOS; here a Job Object is used (reusing managed_process.rs job creation).

Ported / Deferred

• Ported: job containment in cli_fallback.rs::run_cli_command via new managed_process::ProcessJob (thin wrapper over the existing create_managed_job / assign_process_to_job; the latter is no longer test-only).
• Leak confirmation: a unit test spawns a probe with a descendant, drops the child (kill_on_drop) and asserts the descendant is still alive, then drops the job and asserts it dies. A real agy was not run.
• Limitation: assignment to the job happens right after spawn (tokio cannot pass a job-list attribute), so a descendant spawned in that sub-millisecond window is not contained. ManagedProcess (PTY path) is unchanged and still joins its job atomically. If assignment fails the probe proceeds with the previous kill_on_drop behavior and a debug log.
• Deferred: none.

Validation

• cargo +1.98.0 fmt --all: clean
• cargo +1.98.0 clippy --workspace --all-targets -- -D warnings: pass
• cargo +1.98.0 test -p codexbar antigravity -- --test-threads=4: 113 passed, 0 failed (includes new dropping_the_probe_reaps_descendants_but_not_unrelated_processes, which also asserts an unrelated bystander process survives)
• cargo +1.98.0 test -p codexbar managed_process -- --test-threads=4: 10 passed, 0 failed

Affected areas

Rust backend, Antigravity provider, managed_process (shared helper, additive). No frontend, tray, settings, or float bar changes.

UI proof

Not applicable

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: be63fcc5-36ce-47eb-867c-973387e89c92

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Finesssee]

Thermo-nuclear review

Scope: rust/src/providers/antigravity/cli_fallback.rs, rust/src/managed_process.rs (head 25aab81).

Verdict: the design is right and small. It reuses the existing kill-on-close job code in managed_process.rs instead of adding a second Job Object implementation, the provider logic stays in providers/antigravity/, and there is no new dependency. There are no structural blockers. Four items were worth tightening, and I am fixing them in a follow-up commit.

Findings (being fixed)

1. Wrapper type plus cfg split inside a flow (simplification). ProbeContainment was a newtype whose only purpose was to carry #[expect(dead_code)] on a field held for its Drop. spawn_contained also had a #[cfg(windows)] / #[cfg(not(windows))] pair of let statements in the middle of the function. Replace both with a type ProbeJob = ProcessJob alias (() off Windows) and a single contain_child that has one cfg-gated definition per platform. spawn_contained is then a straight-line function returning Option<ProbeJob>, the expect(dead_code) is gone, and no cfg statement remains in the control flow.
2. Silent fallback hides an unclear invariant. When job creation or assignment failed, or the child had already exited, the probe ran uncontained and logged only at debug. That is the exact leak this PR exists to prevent, so it now logs at warn (message only, no secrets). Behavior is unchanged: the probe still runs, best effort.
3. Test leaks a process on failure. The bystander powershell.exe was killed only on the happy path, so any assertion failure left a 120 s sleeper behind. It is now wrapped in a small KillOnDrop guard. The wait_until(marker.exists()) step was redundant with the following pid-parse wait and is removed.
4. Test coverage is otherwise good. The test proves both halves of the claim: kill_on_drop alone does not reap the descendant, and dropping the job does, while an unrelated process survives.

Checked, no change needed

• File sizes: cli_fallback.rs is about 510 lines. managed_process.rs was already over 1000 lines before this PR (1131 lines) and gains 22, so this PR does not cross the threshold. The new ProcessJob type reuses create_managed_job and assign_process_to_job, both of which existed already. No decomposition is requested.
• Cross-provider leakage: none. The only shared-layer change is promoting the previously test-only assign helper and exposing a small public ProcessJob.
• Known limit: tokio cannot spawn suspended, so a descendant started in the window between spawn and AssignProcessToJobObject would escape the job. ProcessJob::contain documents this. It is acceptable for a probe whose descendants start well after launch.
• Job lifetime: _containment lives to the end of run_cli_command, so success, error and timeout all reap the tree.

Validation of the current tree

• cargo +1.98.0 fmt --all: clean
• cargo +1.98.0 clippy --workspace --all-targets -- -D warnings: clean
• cargo +1.98.0 test -p codexbar antigravity: 113 passed, 0 failed (includes dropping_the_probe_reaps_descendants_but_not_unrelated_processes)
• cargo +1.98.0 test -p codexbar managed_process: 10 passed, 0 failed

[Finesssee]

Thermo-nuclear review follow-up

Pushed cecf41e ("Address thermo review") to port/micro-0.69.0-antigravity-cli-usage-reap. All four findings from the review above are addressed in rust/src/providers/antigravity/cli_fallback.rs:

1. ProbeContainment newtype and the in-flow cfg split are replaced by a ProbeJob type alias and one contain_child per platform; spawn_contained is straight-line and the expect(dead_code) is gone.
2. A failed or skipped job containment now logs at warn instead of debug. Behavior is unchanged (best effort).
3. The test's bystander process is wrapped in a KillOnDrop guard, and the redundant marker wait is removed.
4. No change was needed to managed_process.rs.

Validation on cecf41e (pinned Rust 1.98.0):

• cargo fmt --all: clean
• cargo clippy --workspace --all-targets -- -D warnings: clean (both manifests)
• cargo test -p codexbar antigravity: 113 passed, 0 failed
• cargo test -p codexbar managed_process: 10 passed, 0 failed
• Frontend untouched, so no vitest run. UI-affecting: no.