[realtonyyoung]

Closes #1019 — AI-2960

What & why

A_descendant_holding_the_pipe_open_is_bounded_and_still_reads_the_version deliberately leaves a sleeping descendant holding the stub CLI's stdout after the CLI exits. Once reparented it is nobody's child, so it outlived the test by about 18 s and was still running when the test host tore down on a loaded macOS runner, where the host intermittently exited 139. The stub now launches that descendant with a per-test marker in its command line and records its pid. The test reaps it in a finally: it captures the pid's identity, confirms the command line still carries the marker, re-checks the identity, and only then kills. ProcessTree gains a Kill(pid, identity) overload that signals only while the pid still carries the captured identity.

Where to look

Ownership is proven by the marker, not by timing, so a descendant that exited early, or a pid reused since, is never signalled. The 139 is rare and load-sensitive; this removes the identified lingering descendant but is not proven to be the only cause.

Verification

The version-probe tests pass locally on the branch merged with main, and no marked process remains afterwards. With the kill replaced by a log line, the reap path was reached and the marked descendant stayed alive, so the reap is what removes it.

🤖 Generated with Claude Code

[linear-code]

AI-2960

[qodo-code-review]

PR Summary by Qodo

Reap leaked probe descendants before test-host teardown

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Records the background sleep PID created by the pipe-blocking probe stub.
• Reaps the descendant to prevent leakage into macOS test-host teardown.
• Uses bounded, best-effort cleanup for missing or already-exited processes.

Diagram

graph TD
    T["Probe test"] --> S["Stub CLI"] --> D["Sleep descendant"]
    S --> P["PID file"] --> R["Reap helper"] --> K["Process kill"]
    K -.-> D

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Run cleanup in try/finally

• ➕ Guarantees cleanup when the probe or either assertion fails
• ➕ Preserves the PID-file strategy and test scenario
• ➖ Adds minor control-flow structure to the test

Recommendation: Keep the PID-file cleanup strategy because it directly identifies the deliberately orphaned process without changing production behavior. Invoke ReapDescendantAsync from a finally block, however, so assertion or probe failures cannot leave the same descendant running during teardown.

Files changed (1) +15 / -1

Bug fix (1) +15 / -1

DaemonRunnerVersionProbeTests.csTrack and reap the probe stub's background descendant +15/-1

Track and reap the probe stub's background descendant

• The POSIX stub now writes its background sleep process ID beside the executable. After validating bounded probing and version recovery, the test waits briefly for that PID file and best-effort kills the descendant so it cannot survive into test-host teardown.

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Failed tests leak the descendant ✓ Resolved

Description

ReapDescendantAsync is called only after the probe and both assertions rather than from a
finally block. If the probe or either assertion throws, control skips cleanup and the background
sleep 20 survives into host teardown.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[123]

+        await ReapDescendantAsync(cli);

Relevance

●●● Strong

Recent tests require finally-based cleanup to prevent child leaks when assertions fail.

PR-#965
PR-#929

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The generated shell script starts and records the background process, while the only cleanup call
appears after every operation that can throw. TempDir.Dispose only removes filesystem contents,
and other process-owning tests use finally to guarantee cleanup.

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[105-123]
test/Capacitor.Tests.Helpers/TempDir.cs[76-94]
test/Capacitor.Cli.Core.Tests.Unit/StreamingRunnerTests.cs[92-107]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The descendant cleanup runs only after the probe and assertions complete successfully, so any exception bypasses it and leaves the background process running.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[113-123]

## Recommended Fix
Wrap the probe and assertions in `try` and invoke `await ReapDescendantAsync(cli)` from `finally`, ensuring cleanup runs for successful, failed, and cancelled test execution.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Cleanup returns before process exit ✓ Resolved

Description

ReapDescendantAsync calls Process.Kill() but returns without waiting for the process to
terminate. Since sending the kill signal and observing exit are separate operations, a successful
test can finish while the descendant still holds the inherited pipe, preserving the teardown race.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[133]

+        try { System.Diagnostics.Process.GetProcessById(pid).Kill(); } catch { }

Relevance

●●● Strong

Process cleanup precedents require waiting or reaping after kill to guarantee termination.

PR-#43
PR-#540

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The helper ends immediately after Kill() with no exit wait or liveness check. Elsewhere, the
repository waits for exit after killing when cleanup must guarantee that a process has terminated,
and the process-tree implementation likewise separates kill requests from exit observation.

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[128-134]
src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[31-36]
test/Capacitor.Cli.Core.Tests.Unit/ProcessRunnerTests.cs[157-158]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The cleanup helper requests process termination but does not confirm it before returning, so the descendant can remain alive during test-host teardown.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[128-133]

## Recommended Fix
Keep the retrieved `Process` in a disposed local, call `Kill()`, and then await `WaitForExitAsync()` with a short bounded timeout. Preserve best-effort handling for an already-exited or missing process.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Replacement's children can be killed ✓ Resolved

Description

On Unix, Kill(pid, identity) checks the root identity before calling KillSubtree, but that
traversal enumerates children without checking the root identity again. If the root exits and its
PID is reused between those operations, the traversal captures and kills children of the replacement
process before its final root check detects the mismatch.

Code

src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[75]

+        if (ProcessIdentity.Matches(pid, identity)) KillSubtree(pid, identity);

Relevance

●●● Strong

Recent ProcessTree precedent accepts identity-safe PID reuse fixes, including validating targets
before signaling.

PR-#841

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The added dispatch checks identity only before entering KillSubtree. That method lists children by
the current parent PID and kills them before its final check of the root's identity; capturing each
child's own identity does not establish that it belonged to the originally verified root.

src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[73-92]
src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[94-96]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new identity-aware overload can enter descendant traversal after the verified PID has been recycled, allowing it to kill the replacement process's children.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[75-92]
## Recommended Fix
Ensure the root still has the supplied identity when discovering its children, and do not act on discovered children if that identity changes during traversal. Account for the remaining check-to-enumeration race when defining the overload's PID-reuse guarantee.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Quoted temp paths leave sleep running ✓ Resolved

Description

The stub interpolates pidFile inside a single-quoted shell string without escaping apostrophes. If
the system temporary directory contains an apostrophe, the stub cannot write the sidecar and
ReapDescendantAsync returns without killing the background sleep.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[107]

+            "faketool", $"#!/bin/sh\necho 'faketool 9.9.9'\nsleep 20 &\necho $! > '{pidFile}'\nexit 0\n");

Relevance

●●● Strong

This is a deterministic shell quoting bug that can leak a subprocess; repository precedents accept
subprocess cleanup hardening.

PR-#841
PR-#1104

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

TempDir creates its path under the system temporary directory, while the script puts that path
directly between POSIX single quotes. Cleanup explicitly returns if the resulting sidecar is absent.

test/Capacitor.Tests.Helpers/TempDir.cs[15-26]
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[105-107]
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[130-133]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The generated shell script breaks when the system temp path contains an apostrophe, leaving its descendant unreaped.
## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[105-107]
## Recommended Fix
Encode the sidecar path as a valid POSIX shell argument before embedding it in the script, or pass it to the script as an argument instead of interpolating it into shell source.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. A test comment misstates parentage ✓ Resolved

Description

The new comment says the reparented descendant is no longer anyone's child, but reparenting gives
it a new parent; it is no longer the test's child. When a maintainer uses this explanation to reason
about cleanup, it obscures which process can wait for the descendant.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[R102-104]

+        // The stub records the descendant's pid so the test can reap it: once reparented it is no
+        // longer anyone's child, and left alone it outlives the test host into teardown. The sidecar
+        // path comes from the fixture so the shell writer and the reaper cannot drift apart.

Relevance

●●● Strong

Recent test-comment precedents accept rewriting inaccurate or history-heavy comments to state
durable behavior precisely.

PR-#977
PR-#1031

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2762993 requires new comments to remain accurate without external context. The added comment
describes the descendant as reparented while also saying it is no longer anyone's child; reparenting
changes its parent rather than removing one.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[102-104]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The cleanup comment incorrectly says the reparented process is no longer anyone's child.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[102-104]

## Recommended Fix
Say that the descendant is no longer the test process's child, so the test cannot wait for it as its own child.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View medium (3)

6. Cleanup can kill a recycled process ✓ Resolved

Description

ReapDescendantAsync reads a bare integer from the sidecar and passes it to
Process.GetProcessById(pid).Kill() without capturing the process incarnation through
PidIdentity. If the sleeping descendant exits and the operating system reuses its PID before
cleanup runs, the call targets the replacement process rather than proving it still refers to the
stub descendant.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[133]

+        try { System.Diagnostics.Process.GetProcessById(pid).Kill(); } catch { }

Relevance

●●● Strong

Recent ProcessTree tests explicitly reject raw PID cleanup because PID reuse can target unrelated
processes.

PR-#841

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Compliance rule 2807091 requires tests to capture process incarnation identity and avoid termination
or waits based only on an integer PID. The changed cleanup kills the process selected solely by the
sidecar PID, while the helper documentation explicitly explains that a reparented process PID may be
reassigned at any moment.

Rule 2807091: Tests must use PidIdentity helpers instead of directly asserting on raw PIDs
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[132-133]
test/Capacitor.Tests.Helpers/PidIdentity.cs[5-16]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Descendant cleanup acts on a raw PID, which may identify an unrelated process after PID reuse.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[128-133]

## Recommended Fix
Capture the descendant with `PidIdentity.Capture(pid)` while it is alive, terminate it through `ProcessTree.Kill(pid)`, and use `PidIdentity.WaitUntilGoneAsync` with a bounded timeout to verify that the captured incarnation is gone. Preserve best-effort handling for a descendant that exits before identity capture.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. Sidecar path skips the temp helper ✓ Resolved

Description

ReapDescendantAsync constructs the sidecar path with cli + ".pid" even though cli is an entry
beneath a TempDir. The shell filename and cleanup suffix must now stay synchronized manually, so a
later rename or sidecar-layout change can make cleanup inspect a different path.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[129]

+        var pidFile = cli + ".pid";

Relevance

●●● Strong

Recent test precedents accept TempDir helper paths instead of manual path construction.

PR-#935
PR-#803

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Compliance rule 2767472 requires paths beneath a TempDir to be constructed through its members.
The changed test creates faketool through tmp.CreateFile but derives its adjacent PID file
through string concatenation instead of tmp.PathTo.

Rule 2767472: Use TempDir helper for all temporary filesystem state in tests
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[108-108]
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[129-132]
test/Capacitor.Tests.Helpers/TempDir.cs[25-27]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new PID sidecar path is constructed by concatenating a suffix onto a path owned by `TempDir`, bypassing the fixture's path helper.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[108-108]
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[123-132]

## Recommended Fix
Create the sidecar path with `tmp.PathTo("faketool.pid")` and pass that path into `ReapDescendantAsync`. Keep the shell output location and cleanup reader tied to this single fixture-derived path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. Exiting parents leave children running ✓ Resolved

Description

KillSubtree returns on a root identity mismatch before processing the children it already
enumerated. If the original parent exits between enumeration and that check, its listed descendants
are left running rather than killed.

Code

src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[R85-87]

+            // Act on the listed children only if the root is still the same incarnation after listing
+            // them: a pid reused in between would otherwise hand the replacement's children to this kill.
+            if (!ProcessIdentity.Matches(pid, identity)) return;

Relevance

●● Moderate

Identity-safety precedent supports the guard, but no close rejection precedent resolves the
descendant-cleanup tradeoff.

PR-#841

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new check returns before the child loop. On Linux, enumeration reads each process's parent PID,
so a child can be included while its parent is alive; the subsequent identity check can fail if that
parent exits. Direct Unix cleanup callers use this tree walk.

src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[82-94]
src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[126-148]
src/Capacitor.Cli.Daemon/Acp/AcpChildProcess.cs[166-181]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`KillSubtree` discards already-listed descendants if the original parent exits just before the new identity check. Preserve cleanup of children proven to belong to that parent without acting on children of a recycled PID.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[82-89]
## Recommended Fix
Capture sufficient parent/child identity evidence while enumerating, then distinguish an exited original parent from a recycled parent before deciding whether to process the listed children. Add a test in which the parent exits after its child is listed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 64 rules

✅ Cross-repo context — repo relationships

Review mode: ⚖️ Balanced: This localized change modifies process-tree termination and PID-reuse race handling, making it behaviorally and operationally high-risk despite the small diff.

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 6db281f

Results up to commit 0b0f11b ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Cleanup returns before process exit ✓ Resolved

Description

ReapDescendantAsync calls Process.Kill() but returns without waiting for the process to
terminate. Since sending the kill signal and observing exit are separate operations, a successful
test can finish while the descendant still holds the inherited pipe, preserving the teardown race.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[133]

+        try { System.Diagnostics.Process.GetProcessById(pid).Kill(); } catch { }

Relevance

●●● Strong

Process cleanup precedents require waiting or reaping after kill to guarantee termination.

PR-#43
PR-#540

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The helper ends immediately after Kill() with no exit wait or liveness check. Elsewhere, the
repository waits for exit after killing when cleanup must guarantee that a process has terminated,
and the process-tree implementation likewise separates kill requests from exit observation.

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[128-134]
src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[31-36]
test/Capacitor.Cli.Core.Tests.Unit/ProcessRunnerTests.cs[157-158]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The cleanup helper requests process termination but does not confirm it before returning, so the descendant can remain alive during test-host teardown.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[128-133]

## Recommended Fix
Keep the retrieved `Process` in a disposed local, call `Kill()`, and then await `WaitForExitAsync()` with a short bounded timeout. Preserve best-effort handling for an already-exited or missing process.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Failed tests leak the descendant ✓ Resolved

Description

ReapDescendantAsync is called only after the probe and both assertions rather than from a
finally block. If the probe or either assertion throws, control skips cleanup and the background
sleep 20 survives into host teardown.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[123]

+        await ReapDescendantAsync(cli);

Relevance

●●● Strong

Recent tests require finally-based cleanup to prevent child leaks when assertions fail.

PR-#965
PR-#929

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The generated shell script starts and records the background process, while the only cleanup call
appears after every operation that can throw. TempDir.Dispose only removes filesystem contents,
and other process-owning tests use finally to guarantee cleanup.

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[105-123]
test/Capacitor.Tests.Helpers/TempDir.cs[76-94]
test/Capacitor.Cli.Core.Tests.Unit/StreamingRunnerTests.cs[92-107]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The descendant cleanup runs only after the probe and assertions complete successfully, so any exception bypasses it and leaves the background process running.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[113-123]

## Recommended Fix
Wrap the probe and assertions in `try` and invoke `await ReapDescendantAsync(cli)` from `finally`, ensuring cleanup runs for successful, failed, and cancelled test execution.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Sidecar path skips the temp helper ✓ Resolved

Description

ReapDescendantAsync constructs the sidecar path with cli + ".pid" even though cli is an entry
beneath a TempDir. The shell filename and cleanup suffix must now stay synchronized manually, so a
later rename or sidecar-layout change can make cleanup inspect a different path.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[129]

+        var pidFile = cli + ".pid";

Relevance

●●● Strong

Recent test precedents accept TempDir helper paths instead of manual path construction.

PR-#935
PR-#803

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Compliance rule 2767472 requires paths beneath a TempDir to be constructed through its members.
The changed test creates faketool through tmp.CreateFile but derives its adjacent PID file
through string concatenation instead of tmp.PathTo.

Rule 2767472: Use TempDir helper for all temporary filesystem state in tests
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[108-108]
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[129-132]
test/Capacitor.Tests.Helpers/TempDir.cs[25-27]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new PID sidecar path is constructed by concatenating a suffix onto a path owned by `TempDir`, bypassing the fixture's path helper.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[108-108]
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[123-132]

## Recommended Fix
Create the sidecar path with `tmp.PathTo("faketool.pid")` and pass that path into `ReapDescendantAsync`. Keep the shell output location and cleanup reader tied to this single fixture-derived path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Cleanup can kill a recycled process ✓ Resolved

Description

ReapDescendantAsync reads a bare integer from the sidecar and passes it to
Process.GetProcessById(pid).Kill() without capturing the process incarnation through
PidIdentity. If the sleeping descendant exits and the operating system reuses its PID before
cleanup runs, the call targets the replacement process rather than proving it still refers to the
stub descendant.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[133]

+        try { System.Diagnostics.Process.GetProcessById(pid).Kill(); } catch { }

Relevance

●●● Strong

Recent ProcessTree tests explicitly reject raw PID cleanup because PID reuse can target unrelated
processes.

PR-#841

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Compliance rule 2807091 requires tests to capture process incarnation identity and avoid termination
or waits based only on an integer PID. The changed cleanup kills the process selected solely by the
sidecar PID, while the helper documentation explicitly explains that a reparented process PID may be
reassigned at any moment.

Rule 2807091: Tests must use PidIdentity helpers instead of directly asserting on raw PIDs
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[132-133]
test/Capacitor.Tests.Helpers/PidIdentity.cs[5-16]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Descendant cleanup acts on a raw PID, which may identify an unrelated process after PID reuse.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[128-133]

## Recommended Fix
Capture the descendant with `PidIdentity.Capture(pid)` while it is alive, terminate it through `ProcessTree.Kill(pid)`, and use `PidIdentity.WaitUntilGoneAsync` with a bounded timeout to verify that the captured incarnation is gone. Preserve best-effort handling for a descendant that exits before identity capture.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit 23d41f5 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Replacement's children can be killed ✓ Resolved

Description

On Unix, Kill(pid, identity) checks the root identity before calling KillSubtree, but that
traversal enumerates children without checking the root identity again. If the root exits and its
PID is reused between those operations, the traversal captures and kills children of the replacement
process before its final root check detects the mismatch.

Code

src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[75]

+        if (ProcessIdentity.Matches(pid, identity)) KillSubtree(pid, identity);

Relevance

●●● Strong

Recent ProcessTree precedent accepts identity-safe PID reuse fixes, including validating targets
before signaling.

PR-#841

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The added dispatch checks identity only before entering KillSubtree. That method lists children by
the current parent PID and kills them before its final check of the root's identity; capturing each
child's own identity does not establish that it belonged to the originally verified root.

src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[73-92]
src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[94-96]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new identity-aware overload can enter descendant traversal after the verified PID has been recycled, allowing it to kill the replacement process's children.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[75-92]
## Recommended Fix
Ensure the root still has the supplied identity when discovering its children, and do not act on discovered children if that identity changes during traversal. Account for the remaining check-to-enumeration race when defining the overload's PID-reuse guarantee.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Quoted temp paths leave sleep running ✓ Resolved

Description

The stub interpolates pidFile inside a single-quoted shell string without escaping apostrophes. If
the system temporary directory contains an apostrophe, the stub cannot write the sidecar and
ReapDescendantAsync returns without killing the background sleep.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[107]

+            "faketool", $"#!/bin/sh\necho 'faketool 9.9.9'\nsleep 20 &\necho $! > '{pidFile}'\nexit 0\n");

Relevance

●●● Strong

This is a deterministic shell quoting bug that can leak a subprocess; repository precedents accept
subprocess cleanup hardening.

PR-#841
PR-#1104

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

TempDir creates its path under the system temporary directory, while the script puts that path
directly between POSIX single quotes. Cleanup explicitly returns if the resulting sidecar is absent.

test/Capacitor.Tests.Helpers/TempDir.cs[15-26]
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[105-107]
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[130-133]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The generated shell script breaks when the system temp path contains an apostrophe, leaving its descendant unreaped.
## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[105-107]
## Recommended Fix
Encode the sidecar path as a valid POSIX shell argument before embedding it in the script, or pass it to the script as an argument instead of interpolating it into shell source.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. A test comment misstates parentage ✓ Resolved

Description

The new comment says the reparented descendant is no longer anyone's child, but reparenting gives
it a new parent; it is no longer the test's child. When a maintainer uses this explanation to reason
about cleanup, it obscures which process can wait for the descendant.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[R102-104]

+        // The stub records the descendant's pid so the test can reap it: once reparented it is no
+        // longer anyone's child, and left alone it outlives the test host into teardown. The sidecar
+        // path comes from the fixture so the shell writer and the reaper cannot drift apart.

Relevance

●●● Strong

Recent test-comment precedents accept rewriting inaccurate or history-heavy comments to state
durable behavior precisely.

PR-#977
PR-#1031

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2762993 requires new comments to remain accurate without external context. The added comment
describes the descendant as reparented while also saying it is no longer anyone's child; reparenting
changes its parent rather than removing one.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[102-104]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The cleanup comment incorrectly says the reparented process is no longer anyone's child.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/DaemonRunnerVersionProbeTests.cs[102-104]

## Recommended Fix
Say that the descendant is no longer the test process's child, so the test cannot wait for it as its own child.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit b25e1ea ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Exiting parents leave children running ✓ Resolved

Description

KillSubtree returns on a root identity mismatch before processing the children it already
enumerated. If the original parent exits between enumeration and that check, its listed descendants
are left running rather than killed.

Code

src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[R85-87]

+            // Act on the listed children only if the root is still the same incarnation after listing
+            // them: a pid reused in between would otherwise hand the replacement's children to this kill.
+            if (!ProcessIdentity.Matches(pid, identity)) return;

Relevance

●● Moderate

Identity-safety precedent supports the guard, but no close rejection precedent resolves the
descendant-cleanup tradeoff.

PR-#841

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new check returns before the child loop. On Linux, enumeration reads each process's parent PID,
so a child can be included while its parent is alive; the subsequent identity check can fail if that
parent exits. Direct Unix cleanup callers use this tree walk.

src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[82-94]
src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[126-148]
src/Capacitor.Cli.Daemon/Acp/AcpChildProcess.cs[166-181]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`KillSubtree` discards already-listed descendants if the original parent exits just before the new identity check. Preserve cleanup of children proven to belong to that parent without acting on children of a recycled PID.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/ProcessTree.cs[82-89]
## Recommended Fix
Capture sufficient parent/child identity evidence while enumerating, then distinguish an exited original parent from a recycled parent before deciding whether to process the listed children. Add a test in which the parent exits after its child is listed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[realtonyyoung]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 23d41f5

[realtonyyoung]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit a5da81d

[realtonyyoung]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit b25e1ea

[realtonyyoung]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 6db281f