refactor(platform-probes): route every probe's output through one sink

[MikeGrier]

Stage 1 of peeling windows-platform-probes off mikegrier/deferred-namespace-ops (#56). It comes first because everything else in that crate's remaining work is built on it: the new probes all report through report, and lib.rs on main has no such module.

What this does

The repository's architectural pre-step says an output abstraction is introduced at the first occurrence, and these eight probes had grown a println! per finding. Each now composes its whole report as text and hands it to emit, so the real stream is named in report and nowhere else -- a probe's main is one line that chooses no stream at all. Retargeting a probe -- to a file, a buffer, a test -- stops being a rewrite.

Every probe's report now opens with windows-placement-probe's banner naming the machine and whether the measurement is tainted. That is what makes a captured report safe to paste somewhere: without it a finding can be compared against a machine it does not describe. Rendering it inside the report rather than printing it separately is the point -- it travels with the text.

What the mutation run found

A run over report.rs found the first five unit tests insufficient, and the gap was the one that mattered: <impl Report for Stdout>::line replaced with () survived the whole suite. Captured is what every in-process test uses, so nothing exercised the implementation the probes actually run with -- a no-op there means every probe runs, exits zero, and prints nothing, which is the failure mode that looks most like success.

Covering it needs a real child process, since stable Rust cannot redirect this process's own stdout -- which is what makes tests/a_probe_writes_its_report_to_stdout.rs an integration test by the repository's criterion rather than by preference. It runs probe-error-mode (no privileges, no particular hardware) and asserts the report exists, opens with the host banner, and carries more than that banner. Confirmed load-bearing: 5 caught / 1 missed becomes 7 caught / 0 missed.

Three review rounds, and what they changed

Each round's findings were verified by measurement before being accepted, and fixed here rather than deferred.

Round 1 -- three defects in the first draft:

• Buffering the report meant a panic destroyed it. Each render composed into a local String and returned it, so nothing reached stdout unless it returned normally -- while worker_context composes several completed findings and then calls an observation documented to panic three ways, and cancel_io asserts after its first case. Printing line-by-line had made partial output automatic; the refactor silently gave that up, which for an instrument whose purpose is that a failure be diagnosable throws away exactly the information worth keeping. Fixed with emit_report, verified by injecting a panic mid-render: the already-measured lines print and the process still exits 101.
• writeln_to had no callers -- a pub item on a library target is exempt from dead_code, so nothing flagged it. Removed.
• The module doc claimed "All fourteen" probes route through the sink. There are eight; the count came from the originating branch, which is precisely the error a stage-1 peel invites.

Round 2 -- the more useful finding was that the test for round 1's fix did not test it:

• The panic test never called the function it was named for. It re-implemented catch-emit-resume against a Captured, so deleting the resume_unwind would have left it green -- and a probe that swallows its panic prints a partial report and exits 0, the failure that looks most like success. The logic now lives in emit_report_to, taking the sink as an argument. Verified by sabotage in both directions: deleting the resume_unwind fails one assertion, deleting the emit fails the other, and each leaves the other passing.

  Worth recording: a mutation run had reported 6/6 caught over the old test. Whole-function-body mutants never generate that single-branch deletion, so a green mutants run is not evidence that every branch is bound.

• Buffering still loses everything when termination does not unwind -- Ctrl-C, or an abort during unwinding. Not fixed here; the honest fix is renderers writing into a Report as they go, which changes every renderer and is a different design. Stated on emit_report and queued as milestone M1 of a new crates/windows-platform-probes/CHECKLIST.md, rather than left as prose in a design note.

Round 3 -- three stale restatements, two made stale by this commit:

• CHECKLIST-mutation-survivors.md MS-2.1 was invalidated -- it said twelve probes still print directly and the sink's checklist "is not in this repository yet", so whoever picked it up would think the prerequisite was blocked on a branch item that had just landed. Corrected, but not to the reviewer's reading: the 2026-09-02 sweep measured a fourteen-probe tree, and six of those exist only on the originating branch, so "zero probes now print directly" is true of this repository and not of what the sweep scored.
• report.rs's one-stream rationale asserted something the crate contradicts -- it justified having no problem method with "none of them is a diagnostic", while Impersonation::drop does eprintln! a RevertToSelf warning on exactly the panic path emit_report_to serves. Restated so the argument survives the counter-example instead of denying it.
• A count went stale the moment this commit added a line -- DESIGN-NOTES said "seven completed findings"; seven was right on main and the banner made it eight. Three further copies were wrong the same way. All now unnumbered, which is the only form that does not rot.

Cargo.toml

• windows-placement-probe added as a path dependency, for the banner.
• The version = "0.1.3" pin dropped from windows-threadpool-sys. A version beside a path is consulted only when packaging, but cargo still requires the path crate's version to satisfy it on every build -- so a pin left behind by a release breaks workspace resolution rather than just this crate's. This crate is publish = false (verified, not assumed), so the pin bought nothing.

Verified by running, not only by building

• cargo check --all-targets on this package alone against main's tree, which is what establishes the subset is self-contained rather than merely part of a working branch.
• Workspace clippy --all-targets --all-features clean; fmt --check clean.
• 25 unit + 1 integration tests pass; the report tests confirmed to execute by name, because a tests.rs that is never declared is a silent no-op.
• All eight probes run end to end with their banners; a refactor of code whose entire output is a report is not verified by compiling it.
• check-publishable, check-workflow-refs, check-commit-scope, and the encoding check all pass.

Deliberately excluded

The six other modules lib.rs gains on the branch, the new probe binaries, and the topology tests in src/tests.rs. Those are later stages and each depends on this one.

[Copilot]

🟡 Changes recommended

There are small but concrete fixups needed in the newly added report test module (unused import) and new-file header consistency.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Refactors windows-platform-probes so every probe renders its report as text and routes output through a single report sink, enabling capture/assertion in tests while keeping production output centralized. It also standardizes reports to begin with the host/taint banner from windows-placement-probe, and records the known buffering tradeoff with follow-up work queued.

Changes:

• Introduce report sink (Stdout/Captured) with emit_report / emit_report_to, and update probe binaries to render into a String and emit through the sink.
• Add unit + integration coverage to ensure the real Stdout sink produces output, and that panic paths still emit already-rendered lines while resuming unwind.
• Update windows-platform-probes planning/design docs and dependencies (add windows-placement-probe, drop version pin on the path dep).

File summaries

File	Description
PLANS.md	Adds windows-platform-probes checklist row to workspace plans tracker.
CHECKLIST-mutation-survivors.md	Updates mutation-sweep notes to reflect the sink landing and probe count differences vs originating branch.
Cargo.lock	Adds windows-placement-probe as a dependency of windows-platform-probes.
crates/windows-platform-probes/Cargo.toml	Adds windows-placement-probe path dep; drops version pin on windows-threadpool-sys path dep; documents rationale.
crates/windows-platform-probes/CHECKLIST.md	New crate-local checklist queuing follow-up work to restore true streaming output (Ctrl-C/abort cases).
crates/windows-platform-probes/DESIGN-NOTES.md	Updates crate intro and records the “buffered report” decision/tradeoff with an anchored note.
crates/windows-platform-probes/PLANS.md	Registers the new checklist in the crate-local plans table.
crates/windows-platform-probes/src/lib.rs	Exposes new report module.
crates/windows-platform-probes/src/report.rs	New sink abstraction (Report, Stdout, Captured) and emit_report helpers with unwind handling.
crates/windows-platform-probes/src/report/tests.rs	New unit tests for sink behavior, block splitting, and unwind emission/resume behavior.
crates/windows-platform-probes/tests/a_probe_writes_its_report_to_stdout.rs	New integration test ensuring a real probe run as a child process produces a non-empty stdout report starting with the host banner.
crates/windows-platform-probes/src/bin/worker_context.rs	Converts probe output to buffered render + emit_report, prepending banner line.
crates/windows-platform-probes/src/bin/pool_growth.rs	Converts probe output to buffered render + emit_report, prepending banner line.
crates/windows-platform-probes/src/bin/ioring.rs	Converts probe output to buffered render + emit_report, prepending banner line.
crates/windows-platform-probes/src/bin/handle_state.rs	Converts probe output to buffered render + emit_report, prepending banner line.
crates/windows-platform-probes/src/bin/error_mode.rs	Converts probe output to buffered render + emit_report, prepending banner line; keeps measurement/render coupling where needed.
crates/windows-platform-probes/src/bin/device_map.rs	Converts probe output to buffered render + emit_report, prepending banner line; refactors helper to take &mut String.
crates/windows-platform-probes/src/bin/completion_port.rs	Converts probe output to buffered render + emit_report, prepending banner line; refactors helpers to write into &mut String.
crates/windows-platform-probes/src/bin/cancel_io.rs	Converts probe output to buffered render + emit_report, prepending banner line.

Review details

• Files reviewed: 18/19 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[MikeGrier]

Two of the three are fixed; the third is a false positive, and I settled it by execution rather than by reading.

Copyright headers -- fixed. Correct, and worse than reported: src/report.rs and src/report/tests.rs carried // Copyright (c) 2026 Mike Grier taken verbatim from the originating branch, while the other nineteen files in this crate use // Copyright (c) Mike Grier. -- and the third file this PR adds, tests/a_probe_writes_its_report_to_stdout.rs, already used the crate's form, so the two new files were inconsistent even within their own change. Both aligned; the crate is now a single form across all 21 files.

Worth recording that this was a real choice rather than an obvious fix: repository-wide the year form is actually dominant (290 files against 114), so aligning to the crate deliberately moves these two files away from the repository majority. I took local consistency on the grounds that a reader scans a crate, not the whole tree -- happy to go the other way if you would rather the repository converge on the year form.

Report unused in the test module -- not correct, no change made. Captured::line is a trait method, so Report has to be in scope at the call sites (report/tests.rs:20-22). Removing it from the import list does not warn, it fails to compile:

error[E0599]: no method named `line` found for struct `report::Captured` in the current scope
  --> crates\windows-platform-probes\src\report\tests.rs:20
  --> crates\windows-platform-probes\src\report\tests.rs:21
  --> crates\windows-platform-probes\src\report\tests.rs:22

I removed the import and ran cargo check --all-targets to produce that, rather than reasoning about it. The prediction that this "can fail clippy if warnings are denied" is also checkable and false: workspace clippy --all-targets --all-features was clean before this review and is clean now, which it could not have been had the import been dead.

All 27 CI checks passed on the previous head; the force-push carries only the two header lines plus the commit-message record of this round.

[Copilot]

🟢 Approval recommended

The changes are coherent, tested (including an integration test for Stdout emission), and the remaining feedback is limited to minor comment wording corrections.

Review details

• Files reviewed: 18/19 changed files
• Comments generated: 8
• Review effort level: Lite

[MikeGrier]

Correct in all eight, fixed in all eight -- and the finding is sharper than it looks, because the comment was invalidated by this PR''s own earlier fix.

It was true when written. The first draft had fn main() { emit(&mut Stdout, &render()); }, which really did name the stream. Round 1 of review found that buffering meant a panic destroyed the whole report, and the fix replaced that line with emit_report(render). From that moment main named no stream at all -- while the comment sitting directly above it still claimed to be the one place that did. Nothing detects a comment invalidated by a change in a different file.

Where the stream is actually named, measured rather than asserted:

• report.rs:65 -- println!("{text}") in Stdout::line
• report.rs:145 -- emit_report_to(&mut Stdout, render) in emit_report
• Stdout appears zero times in any of the eight bins (the single match under src/bin/ is the word "stdout" in a pool_growth doc comment).

All eight now read:

fn main() {
    // The probe''s whole output policy, and it is one line: hand the renderer to
    // the sink. Nothing here or below names a stream -- that is chosen once, in
    // `report`, so retargeting a probe is not a rewrite.
    emit_report(render);
}

Swept rather than spot-fixed. The same sentence had propagated into two places your comments did not cover: the d-buffered-report decision in crates/windows-platform-probes/DESIGN-NOTES.md, and this PR''s own description. Both corrected, along with the commit message.

Worth stating plainly: this is the third consecutive round where the finding was a correction that failed to propagate rather than an original defect, and this one was self-inflicted -- a fix in one file silently invalidating a comment in eight others. Thanks for catching it in all eight rather than one; a single-site report would have left seven behind.

Re-verified after the change: workspace clippy --all-targets --all-features clean, fmt --check clean, 25 unit + 1 integration tests pass, and probe-error-mode still emits its full 18-line report with the host banner and exit 0.

[Copilot]

🟢 Approval recommended

The core refactor is coherent and backed by unit + integration coverage; remaining feedback is limited to minor documentation/link clarity.

Review details

• Files reviewed: 18/19 changed files
• Comments generated: 3
• Review effort level: Lite

[MikeGrier]

All three correct, all three fixed.

The before/after claim did not survive the change. The module doc said each conversion was checked by "capturing the probe''s output before and after and requiring the two to match" -- while the same conversion prepends a host banner, so the two cannot match. Restated: every pre-existing line must match in the same order, with the banner as the one deliberate difference and the only one permitted. That is what was actually done, so this is the doc catching up to the method rather than the method changing.

Worth naming, because it is now a pattern rather than an incident: this is the third consecutive round in which the finding was a sentence that was true when written and falsified by a later part of the same change --

round	the claim	what falsified it
3	"seven completed findings"	the banner added an eighth line
5	"the only place that names the real stream"	round 1 replaced emit(&mut Stdout, ...) with emit_report(render)
6	"requiring the two to match"	the banner again

Two of the three trace to the same banner. Nothing in the toolchain detects any of them -- clippy, 25 tests, and 27 CI checks were green through all three.

Both file references made into links. Correct per the repository''s rule that a reference the reader is meant to open should be a relative markdown link, source files included. Both were mine, added in rounds three and four. Targets verified to resolve from their respective directories.

I deliberately left the bare filenames elsewhere in CHECKLIST-mutation-survivors.md (fs.rs, watcher.rs, pattern.rs, ...) as they are: those are identifiers inside a survivor listing rather than navigation targets, they predate this commit, and converting them would put unrelated churn in a peel that exists to stay reviewable. Happy to do it as a follow-up if you would rather they were links.

Re-verified: workspace clippy --all-targets --all-features clean, fmt --check clean, 25 unit + 1 integration pass, encoding check 604 files clean, both edited markdown files 0 non-ASCII bytes.

[Copilot]

🟢 Approval recommended

The sink abstraction and probe refactor are coherent and exercised by both unit tests and a process-level stdout integration test, with only minor nits remaining.

Review details

Suppressed comments (1)

crates/windows-platform-probes/src/report/tests.rs:12

• Report is imported here but never used, which will trigger an unused_imports warning and adds noise to future edits. Drop it from the import list.

use super::{Captured, Report, emit, emit_report_to};

• Files reviewed: 18/19 changed files
• Comments generated: 0 new
• Review effort level: Lite

[MikeGrier]

Approval noted, and no change made -- the one suppressed comment repeats the Report finding from an earlier round, which is a false positive. I re-tested it against the current head (88a75bc, working tree clean, so the result applies to exactly what you reviewed) rather than just pointing back at the previous reply.

The comment makes two checkable claims. Both are false.

"Never used." Report is a trait, and Captured::line is its method, so the trait must be in scope at the three call sites:

captured.line("header");
captured.line("row one");
captured.line("row two");

Removing it from the import list does not produce a warning, it produces three hard errors:

error[E0599]: no method named `line` found for struct `report::Captured` in the current scope
  --> src\report\tests.rs:20, :21, :22
  = help: items from traits can only be used if the trait is in scope
help: trait `Report` which provides `line` is implemented but not in scope

"Can fail clippy if warnings are denied." Tested directly rather than reasoned about -- clippy --all-targets --all-features for this package with RUSTFLAGS="-D warnings":

{"reason":"build-finished","success":true}

Clean. If the import were dead, unused_imports would fire and -D warnings would turn it into a build failure, so this run is a decisive test of the claim, not a weaker proxy for it.

Since the import is load-bearing, dropping it would break the build rather than reduce noise, so I have left it exactly as is.