Priority: P2 subprocess liveness and truthful installed-version observation. Reuse the existing process execution boundary; do not add another polling/child-owner framework.
Source at #707 3d955ff998eef120cd398f163c613a54e3383ccb
crates/tracedecay-cli/src/upgrade.rs::installed_binary_version advertises a five-second deadline so a wedged binary cannot hang upgrade. It pipes stdout, repeatedly calls try_wait, and does not read stdout until after the child exits successfully. It then calls unbounded read_to_string outside the deadline loop.
This creates distinct holes:
- A child that fills its stdout pipe can block before exit while the parent only waits for exit. The timeout eventually kills the probe, misclassifying a drainable response as a wedged executable.
- If the child exits but another process still holds its inherited stdout,
read_to_string waits for EOF outside the five-second deadline. A timeout around the earlier wait loop does not cover this read.
- The
try_wait error branch immediately returns None without an explicit settlement path. Rust Child does not automatically kill/wait on Drop. Timeout cleanup also discards kill/wait errors, so its evidence of process settlement is lost.
parse_version_output accepts the final word of any nonempty stdout as a version. It only tests ordinary product output and empty text; malformed diagnostic output is not rejected by a real version contract at this boundary.
brew_linked_binary separately uses an unbounded Command::output for a small discovery response. Keep native package-manager installation (which can legitimately take longer) distinct from short bounded metadata/version probes; do not impose this probe's deadline on the entire upgrade.
Executed mechanism check, not repository acceptance
On the review container's Linux host, an isolated /bin/sh parent printed tracedecay 1.2.3, left a 0.7-second background child inheriting stdout, and exited successfully. Waiting for the parent took 10.0 ms; the subsequent read-to-EOF took another 698.9 ms. This confirms that successful parent exit does not close an inherited pipe. It does not execute the repository Rust function, reproduce a live installed binary, or establish a native Windows/macOS result. No performance percentage is inferred from it.
Smallest durable change
Use the existing suitable bounded process/command owner for discovery and version probes. Drain bounded stdout concurrently with child progress under one absolute operation deadline; treat exit status, complete permitted output and settlement as separate facts. Retain child/process-group cleanup as appropriate through errors and cancellation, and surface cleanup failure without fabricating a validated installed version.
Avoid replacing the loop with bare Command::output or unbounded wait_with_output: neither alone supplies the required end-to-end byte/deadline contract. Avoid detaching a read thread whose pipe can outlive the caller. Probe failures may remain nonfatal where the existing upgrade contract allows Unknown, but preserve the diagnostic reason and do not turn arbitrary trailing words into version evidence. Reuse the applicable product/version parser rather than a second handwritten version grammar.
Acceptance
- A valid small response yields the installed version; malformed prefix/version, truncated UTF-8, excess bytes and unsuccessful exit do not.
- A child producing more than pipe capacity is drained or boundedly refused without a wait-before-read deadlock.
- A parent that exits while a controlled descendant holds stdout cannot leave the probe blocked beyond its original deadline. Confirm child/descendant settlement or explicit retained cleanup failure; the caller receiving None is not alone proof of retirement.
- Cancellation and injected wait/read errors preserve cleanup ownership. No unowned worker survives the probe.
- Real upgrade classification still distinguishes a no-op from a new/unknown installation and respects held daemon service state. Reuse one focused process-boundary fixture instead of duplicating the full upgrade suite for every failure phase.
Primary references: https://doc.rust-lang.org/std/process/struct.Child.html and https://doc.rust-lang.org/std/io/trait.Read.html#method.read_to_string.
Related #1060 removes package-manager metadata emulation; this probe remains necessary for delegated installation verification. No repository tests, native platform suites or installed upgrades were run. Keep #707 draft.
Priority: P2 subprocess liveness and truthful installed-version observation. Reuse the existing process execution boundary; do not add another polling/child-owner framework.
Source at #707
3d955ff998eef120cd398f163c613a54e3383ccbcrates/tracedecay-cli/src/upgrade.rs::installed_binary_versionadvertises a five-second deadline so a wedged binary cannot hang upgrade. It pipes stdout, repeatedly callstry_wait, and does not read stdout until after the child exits successfully. It then calls unboundedread_to_stringoutside the deadline loop.This creates distinct holes:
read_to_stringwaits for EOF outside the five-second deadline. A timeout around the earlier wait loop does not cover this read.try_waiterror branch immediately returns None without an explicit settlement path. Rust Child does not automatically kill/wait on Drop. Timeout cleanup also discards kill/wait errors, so its evidence of process settlement is lost.parse_version_outputaccepts the final word of any nonempty stdout as a version. It only tests ordinary product output and empty text; malformed diagnostic output is not rejected by a real version contract at this boundary.brew_linked_binaryseparately uses an unbounded Command::output for a small discovery response. Keep native package-manager installation (which can legitimately take longer) distinct from short bounded metadata/version probes; do not impose this probe's deadline on the entire upgrade.Executed mechanism check, not repository acceptance
On the review container's Linux host, an isolated
/bin/shparent printedtracedecay 1.2.3, left a 0.7-second background child inheriting stdout, and exited successfully. Waiting for the parent took 10.0 ms; the subsequent read-to-EOF took another 698.9 ms. This confirms that successful parent exit does not close an inherited pipe. It does not execute the repository Rust function, reproduce a live installed binary, or establish a native Windows/macOS result. No performance percentage is inferred from it.Smallest durable change
Use the existing suitable bounded process/command owner for discovery and version probes. Drain bounded stdout concurrently with child progress under one absolute operation deadline; treat exit status, complete permitted output and settlement as separate facts. Retain child/process-group cleanup as appropriate through errors and cancellation, and surface cleanup failure without fabricating a validated installed version.
Avoid replacing the loop with bare
Command::outputor unboundedwait_with_output: neither alone supplies the required end-to-end byte/deadline contract. Avoid detaching a read thread whose pipe can outlive the caller. Probe failures may remain nonfatal where the existing upgrade contract allows Unknown, but preserve the diagnostic reason and do not turn arbitrary trailing words into version evidence. Reuse the applicable product/version parser rather than a second handwritten version grammar.Acceptance
Primary references: https://doc.rust-lang.org/std/process/struct.Child.html and https://doc.rust-lang.org/std/io/trait.Read.html#method.read_to_string.
Related #1060 removes package-manager metadata emulation; this probe remains necessary for delegated installation verification. No repository tests, native platform suites or installed upgrades were run. Keep #707 draft.