Wait for the detached child to exit before reading its stdin payload

[alexeyzimarev]

Closes #1108 — AI-3122

What & why

The_payload_reaches_the_childs_stdin polls until the child's output file is non-empty, then reads it with File.ReadAllText. more writes the payload before cmd closes its > redirect, so on a slow runner the read lands while the file is still open for write, and Windows' mandatory sharing refuses it. The test now waits for the child to exit before reading, so the redirect handle is closed by construction; it also rules out reading a partial write.

Where to look

The pid lookup is safe here only because DetachedChild holds the process handle, which keeps the pid from being reused; a lookup that throws means the child is already gone, which is the state being waited for.

Verification

• Failed twice in a row on windows-latest for PR Keep local terminals responsive when the cloud terminal mirror stalls #1096 (runs 35700576106 and 35719445826), which does not touch this project, and once alone was the leg's only failure out of 15,064 tests.
• Compiles on macOS (dotnet build test/Capacitor.Cli.Tests.Unit, 0 warnings); the test itself is Windows-only, so this PR's own Windows leg is the run that exercises it.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-22T15:14:57.925628Z	e7562fa	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[qodo-code-review]

PR Summary by Qodo

Wait for detached child exit before reading stdin payload

🐞 Bug fix 🧪 Tests 🕐 Less than 10 minutes

AI Description

• Waits for detached Windows child termination before reading its redirected stdin payload.
• Prevents sharing violations and partial reads on slow Windows runners.

Diagram

sequenceDiagram
    participant T as Unit Test
    participant D as DetachedChild
    participant C as cmd Child
    participant P as Process API
    participant F as Payload File
    T->>D: Write stdin payload
    D->>C: Forward stdin
    C->>F: Redirect payload
    T->>P: Lookup child PID
    T->>P: Wait for exit
    C-->>P: Exit
    P-->>T: Exit confirmed
    T->>F: Read full payload

Loading

High-Level Assessment

Waiting for process termination is the strongest synchronization boundary because it guarantees cmd has closed the redirect and completed its write. Continuing to poll file length or retry file access would remain vulnerable to partial content and timing-dependent sharing violations.

Files changed (1) +10 / -3

Bug fix (1) +10 / -3

ProcessHelpersDetachedStdinTests.csWait for detached child termination before validating payload +10/-3

Wait for detached child termination before validating payload

• Replaces file-existence and length polling with a bounded asynchronous wait for the detached child to exit. Treats a failed PID lookup as an already-exited process, then reads the payload only after the redirect handle is closed.

test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs

[qodo-code-review]

Code Review by Qodo

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

1. Child termination bypasses identity helper 📘 Rule violation ☼ Reliability

Description

The detached-child test captures child.Pid, reconstructs a Process with
Process.GetProcessById(pid), waits on it, and later calls Kill(pid) instead of using
PidIdentity.Capture and the retained DetachedChild identity-aware termination helpers. After the
child exits, Windows may reuse the PID before the lookup, causing the test to wait on an unrelated
process or time out before reading the payload, while raw-PID cleanup also lacks ownership-safe
checks.

Code

test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs[R45-48]

+            try {
+                using var process = Process.GetProcessById(pid);
+                await process.WaitForExitAsync(exitBound.Token);
+            } catch (ArgumentException) {

Relevance

●●● Strong

Recent subprocess-test history accepts ownership-safe cleanup and identity-sensitive process
handling.

PR-#978
PR-#540

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The changed test directly obtains a Process from the integer pid and calls WaitForExitAsync,
then uses raw PID cleanup, which is the termination path disallowed by the checklist requiring
PidIdentity.Capture followed by IsGone or WaitUntilGoneAsync. DetachedChild documents that a
PID becomes reusable as soon as the child exits while its retained process handle is the
non-reusable identity; because the test does not use that handle and instead performs a fresh PID
lookup, it can target an unrelated process despite retaining the detached-child handle.

Rule 2807091: Tests must use PidIdentity helpers instead of directly asserting on raw PIDs
test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs[39-50]
test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs[39-49]
src/Capacitor.Cli/DetachedChild.cs[5-10]
src/Capacitor.Cli/ProcessHelpers.WindowsSpawn.cs[150-165]

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 detached-child test waits for termination by looking up and waiting on a raw PID after closing stdin, and performs raw PID cleanup instead of using the shared process-identity abstraction or retained process handle. Since Windows can reuse the PID after the child exits, `Process.GetProcessById(pid)` can target an unrelated process; preserve the child's identity for liveness, waiting, and termination while retaining the existing timeout cancellation and cleanup behavior.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs[39-53]
- test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs[45-50]
- src/Capacitor.Cli/DetachedChild.cs[15-49]
- src/Capacitor.Cli/ProcessHelpers.WindowsSpawn.cs[150-165]

## Recommended Fix
Capture the detached child with `PidIdentity.Capture` while it is alive and wait with `WaitUntilGoneAsync`, `IsGone`, or an equivalent identity-aware operation. Alternatively, expose an internal wait operation or process-backed wait handle on `DetachedChild` that uses its retained Windows process handle, and use that identity-preserving wait instead of `Process.GetProcessById(pid)`; update cleanup to use the approved process-tree or identity abstraction rather than raw PID operations while retaining timeout cancellation and cleanup behavior.

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

---

2. The pull request title lacks its issue reference 📘 Rule violation ⚙ Maintainability

Description

The pull request title is Wait for the detached child to exit before reading its stdin payload and
does not end with a GitHub issue reference in parentheses. The title is therefore missing the
required (#<digits>) suffix even though the supplied commit subject includes (#1108).

Code

test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs[39]

+            // Read only once the child has exited: until cmd closes its redirect the file is open

Relevance

●●● Strong

Repository history accepts enforcement of issue-reference and title-format rules.

PR-#659

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The provided pull request title is imperative but has no trailing (#<digits>) reference, while the
checklist requires exactly one GitHub issue reference at the end of the subject.

Rule 2897961: Enforce single-clause imperative commit subject with GitHub issue reference and 80-char limit

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 pull request title does not include the required GitHub issue reference suffix.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs[39-39]

## Recommended Fix
Update the pull request title to a single imperative clause ending with the explicit GitHub issue reference, for example `Wait for the detached child to exit before reading its stdin payload (#1108)`, and keep the total subject at or below 80 characters.

ⓘ 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: 🚀 Fast: This is a small, localized Windows-only test synchronization fix with one edit site and contained behavioral impact.

Tip of the day

💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-code-review]

1. Child termination bypasses identity helper 📘 Rule violation ☼ Reliability

The detached-child test captures child.Pid, reconstructs a Process with
Process.GetProcessById(pid), waits on it, and later calls Kill(pid) instead of using
PidIdentity.Capture and the retained DetachedChild identity-aware termination helpers. After the
child exits, Windows may reuse the PID before the lookup, causing the test to wait on an unrelated
process or time out before reading the payload, while raw-PID cleanup also lacks ownership-safe
checks.

Agent Prompt

## Issue description
The detached-child test waits for termination by looking up and waiting on a raw PID after closing stdin, and performs raw PID cleanup instead of using the shared process-identity abstraction or retained process handle. Since Windows can reuse the PID after the child exits, `Process.GetProcessById(pid)` can target an unrelated process; preserve the child's identity for liveness, waiting, and termination while retaining the existing timeout cancellation and cleanup behavior.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs[39-53]
- test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs[45-50]
- src/Capacitor.Cli/DetachedChild.cs[15-49]
- src/Capacitor.Cli/ProcessHelpers.WindowsSpawn.cs[150-165]

## Recommended Fix
Capture the detached child with `PidIdentity.Capture` while it is alive and wait with `WaitUntilGoneAsync`, `IsGone`, or an equivalent identity-aware operation. Alternatively, expose an internal wait operation or process-backed wait handle on `DetachedChild` that uses its retained Windows process handle, and use that identity-preserving wait instead of `Process.GetProcessById(pid)`; update cleanup to use the approved process-tree or identity abstraction rather than raw PID operations while retaining timeout cancellation and cleanup behavior.

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

[qodo-code-review]

2. The pull request title lacks its issue reference 📘 Rule violation ⚙ Maintainability

The pull request title is Wait for the detached child to exit before reading its stdin payload and
does not end with a GitHub issue reference in parentheses. The title is therefore missing the
required (#<digits>) suffix even though the supplied commit subject includes (#1108).

Agent Prompt

## Issue description
The pull request title does not include the required GitHub issue reference suffix.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/ProcessHelpersDetachedStdinTests.cs[39-39]

## Recommended Fix
Update the pull request title to a single imperative clause ending with the explicit GitHub issue reference, for example `Wait for the detached child to exit before reading its stdin payload (#1108)`, and keep the total subject at or below 80 characters.

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