feat(platform-probes): measure the long-path opt-in with a manifested pair by MikeGrier · Pull Request #80 · MikeGrier/windows-threadpool-sys

MikeGrier · 2026-09-07T23:31:18Z

Stage 2 of peeling windows-platform-probes off mikegrier/deferred-namespace-ops (#56), on top of the report sink #79 landed. It adds the long-path family: long_path (the measurement), long_path_report (the shared renderer), and the two binaries whose difference is the finding.

The question

Does the longPathAware opt-in lift MAX_PATH for a relative path?

It matters here because a library cannot add a manifest to someone else's executable, so whatever the un-opted-in case does is what a caller of this workspace's crates actually meets.

Neither half of that opt-in is a runtime switch -- it is a machine-wide registry value and a per-executable manifest -- so it cannot be a flag on one binary. Hence the pair: the same code, built twice, with build.rs embedding the manifest into probe-long-path-aware alone. rustc-link-arg-bin rather than rustc-link-arg-bins, because the latter would opt every probe in the crate into long paths and silently change what the others measure.

The aware binary reads a cargo::rustc-cfg the script emits from that same guarded block rather than asserting its own manifest, so the label and the linker cannot disagree. On a non-MSVC target the script emits neither, and a hardcoded claim would have had both halves measuring the un-opted-in case while one announced otherwise.

The result

Measured on the development host rather than described, and the two halves disagree, which is what proves the apparatus works at all:

shape	resolved	aware	unaware
plain	429	opened	REFUSED (error 3)
with `..`	434	opened	REFUSED (error 3)
forward slashes	429	opened	REFUSED (error 3)

The aware side also resolved .. and forward slashes past the ceiling exactly as below it, so on this host there is no evidence of a regularize-then-prefix implementation -- the sharp edge the report is written to catch.

What is worth a reviewer's attention

The two binaries were converted to the stage-1 sink, not carried over. On the originating branch they still read emit(&mut Stdout, &render(...)), which is the shape stage 1 found destroys a report when a renderer panics. render now composes the banner and header, then measures, then renders the body -- the ordering is load-bearing, because a panic in the measurement must still print what was already established.

The report never states more than the run established. That is the whole of verdict, and it is why its guards exist rather than being defensive padding:

a shape must have opened below the ceiling before "stopped resolving past it" can be said of it;
the reference shape must have been tried above it;
a long failure must be a length refusal rather than an access denial;
and both halves of the opt-in must be in effect before a refusal is read as contradicting the documentation.

Without that last one the un-opted-in binary declared Microsoft's documentation wrong on every correctly configured host -- from the half of the pair whose entire job is to be the baseline.

MAX_PATH_CONTENT is 259, not 260. A path of exactly MAX_PATH content units does not fit, because the terminator needs the last one. That is the convention windows-namespace-request-sys and windows-file-enumeration-sys already state and assert, and the ceiling column prints the number rather than naming it.

A temporary directory over 200 UTF-16 units is refused before anything else runs. In a longPathAware process an over-long %TMP%/%TEMP% makes both std::env::temp_dir() and the reg.exe spawn hang rather than fail -- the process spins on a core and prints nothing at all, since the report only reaches stdout when render returns. The limit is chosen rather than derived, and sits far enough short of where the hang starts that the exact boundary stops mattering. On that path registry_enabled is None, not false: the query was never issued, and a bool cannot say so.

Cargo.toml

wtf-string, for a UTF-16 code-unit count wherever a length is compared against a Windows limit. OsStr::len counts WTF-8 here, so the two disagree the moment a non-ASCII character appears in %TEMP% and put an attempt on the wrong side of the ceiling.
Win32_System_Environment, for Get/SetCurrentDirectoryW: the current directory is half of what a relative path resolves against, so the probe places it deliberately rather than inheriting whatever launched it.

Verified by running, not only by building

cargo check --all-targets on this package alone against main's tree, which establishes the subset is self-contained: the new code reaches only std, windows-sys, wtf-string, and stage 1's report.
Both binaries run to completion, exit 0, and disagree as tabled above.
The manifest reached exactly one binary, confirmed by searching the linked images for the manifest's own XML. Searching for the bare string longPathAware reports it in both, because the report's own label contains it -- the weaker test would have raised a false alarm about the central mechanism.
Workspace clippy --all-targets --all-features clean; fmt --check clean.
54 package tests pass. The renderer is split so body and verdict are pure functions of an Observation, which is what lets the report's own tests run without a filesystem: each is named for the wrong sentence it forbids, and asserts both that the right claim is made and that the wrong one is not.
check-workflow-refs, check-publishable, check-commit-scope and the encoding check all pass. Both probes are wired into the CI probe job, and deliberately both: either alone reports a number with no baseline to read it against.

Design record

Decisions are in the crate's DESIGN-NOTES.md -- the pair and the one-binary embed, the declined concurrency hardening for measure (which moves the process's current directory, and cannot be made safe by a unique root because the directory is per-process), and the finding that the ceiling applies to the path as written rather than after .. collapses.

That last one is easy to assume backwards, and the obvious experiment does not settle it: cmd.exe carries longPathAware in its own manifest on this host, so a long path that opens there says nothing about the ceiling. The probe rests on a binary this workspace builds and manifests itself.

Deliberately excluded

The four other modules lib.rs gains on the branch (doorbell_cost, queue_contention, request_cost, topology), the remaining 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 a couple of small but important correctness/maintainability issues in newly added long-path probe code and report wording that should be addressed before merging.

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

Pull request overview

Adds a new windows-platform-probes experiment to measure whether the Windows long-path opt-in (longPathAware + LongPathsEnabled) lifts MAX_PATH for relative paths, implemented as a manifested/unmanifested binary pair with a shared report renderer and supporting tests/CI wiring.

Changes:

Introduces long_path apparatus + long_path_report renderer (and unit tests) to measure/report relative-path behavior past MAX_PATH.
Adds two new probe binaries (probe-long-path-aware / probe-long-path-unaware) plus a build-script-embedded manifest for the aware half.
Updates CI and repo docs/checklists to include the new probe pair and keep prior-stage records accurate.

File summaries

File	Description
tools/check-encoding.ps1	Treats `.manifest` as text for encoding validation.
crates/windows-platform-probes/src/long_path/tests.rs	Adds unit tests ensuring `measure` cleans up process-wide state and temp artifacts.
crates/windows-platform-probes/src/long_path.rs	Adds the measurement apparatus and Win32 calls for the relative-path experiment.
crates/windows-platform-probes/src/long_path_report/tests.rs	Adds thorough tests for the verdict/body wording logic.
crates/windows-platform-probes/src/long_path_report.rs	Implements the shared renderer/verdict logic for both probe binaries.
crates/windows-platform-probes/src/lib.rs	Exposes the new `long_path` and `long_path_report` modules.
crates/windows-platform-probes/src/bin/long_path_unaware.rs	New probe binary for the unmanifested baseline run.
crates/windows-platform-probes/src/bin/long_path_aware.rs	New probe binary for the manifested run, keyed off a build-script cfg.
crates/windows-platform-probes/long-path-aware.manifest	Adds the `longPathAware` side-by-side manifest content.
crates/windows-platform-probes/DESIGN-NOTES.md	Records design decisions for the long-path pair and apparatus constraints.
crates/windows-platform-probes/CHECKLIST.md	Updates stage-2 follow-up checklist wording to avoid stale probe counts.
crates/windows-platform-probes/Cargo.toml	Registers the two new binaries and adds `wtf-string` + Windows env API feature.
crates/windows-platform-probes/build.rs	Embeds the manifest into only the aware binary and emits a matching cfg.
CHECKLIST-mutation-survivors.md	Updates a prior note to avoid a stale probe count reference.
Cargo.lock	Locks the new `wtf-string` dependency in the probe crate.
.github/workflows/ci.yml	Runs both halves of the long-path probe pair in CI.

Review details

Files reviewed: 15/16 changed files
Comments generated: 2
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 · 2026-09-07T23:40:02Z

Both correct and both fixed.

The table over-claimed on two of its three shapes. The annotation for a not-found past the ceiling said "the target provably exists, so this is the length refusal" for every shape. That is true for plain -- it is legal under \\?\ too, which is precisely why it is the control -- but for `..` and forward slashes it is the ambiguity this probe exists to resolve, not a fact about the run.

If the system prepends \\?\ past the ceiling, those two shapes stop being resolved, and the path as written then names something that was never created. The not-found is a genuine absence, and it is the sharp edge. Annotating it as the length refusal files the finding as its own control -- the report would have quietly ruled out the very thing it exists to detect.

The annotation is now keyed on survives_verbatim_parsing, which is the predicate that already distinguishes them:

plain                   429      yes  REFUSED    3 (not-found; this shape survives `\\?\` parsing and the target exists, so this is the length refusal)
with `..`               434      yes  REFUSED    3 (not-found; either the length refusal or this shape ceasing to resolve past the ceiling -- the verdict below decides which)
forward slashes         429      yes  REFUSED    3 (not-found; either the length refusal or this shape ceasing to resolve past the ceiling -- the verdict below decides which)

Pinned by a test, and sabotage-verified rather than assumed: forcing the shape check to true fails that test and no other.

deep_relative was dead. build_tree returns the relative path to the bottom, and only its side effect is wanted -- each attempt spells its own path, because the spelling is what is under test. The binding and the let _ = that discarded it are gone.

Re-verified: workspace clippy --all-targets --all-features and fmt --check clean, 55 tests pass, encoding check 612 files, and both probes still produce their coherent baseline/lifted pair.

Copilot

🟡 Changes recommended

build.rs manifest embedding is currently fragile (unquoted /MANIFESTINPUT: path) and the verbatim \\?\ path construction in the new probe is incorrect for UNC paths, risking false apparatus failures or incorrect measurement on real hosts.

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

Review details

Files reviewed: 15/16 changed files
Comments generated: 2
Review effort level: Lite

MikeGrier · 2026-09-08T00:09:00Z

Both findings from this round are addressed, but only one of them held.

UNC and ill-formed %TMP%/%TEMP% values -- accepted, refused. You are right on
both counts. The apparatus builds its verbatim paths by concatenation, so a UNC root
would need \\?\UNC\server\share rather than \\?\ glued onto \\server, and
Path::display substitutes U+FFFD for an unpaired surrogate, so the reported path
could name a different file than the one created. temp_dir_refusal now refuses both
alongside the length rule, with two tests (a_unc_temp_dir_is_refused_rather_than_spelled_wrong,
a_temp_dir_that_is_not_well_formed_utf16_is_refused), each sabotage-verified: disabling
either branch fails exactly its own test and nothing else. long_path::tests is 9 passing.

Supporting UNC properly is about three lines, but no machine this workspace is developed
or tested on can exercise it, and an untested path through the apparatus is worth less
than an honest refusal.

Unquoted /MANIFESTINPUT: -- refuted by experiment, no change made. I measured this
rather than reasoning about it, and the suggestion would have broken the build.

Built with the manifest under C:\lp spaces test\long-path-aware.manifest: the build
succeeds and the manifest embeds correctly. Cargo passes rustc-link-arg-bin through
as a single argv element, so the linker receives the whole path including the space --
there is no shell in the path to re-split it.
Then applied the suggested fix (literal " around the path) and the build fails:
LNK1181: cannot open input file 'spaces.obj'. The embedded quotes are what make the
linker split on the space.

build.rs is restored byte-identically (hash-checked) and left as it was.

Full gate green: cargo fmt clean, workspace clippy --all-targets --all-features clean,
cargo test -p windows-platform-probes 57 passed / 14 ignored, encoding check 612 files
clean, and both probes run to exit 0 with unchanged output. Force-pushed as 92a46f3.

Copilot

🟡 Changes recommended

There are a few correctness/robustness bugs in the new probe plumbing (linker arg quoting, registry parsing, and temp-dir path classification) that should be fixed before merging.

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

Review details

Suppressed comments (1)

crates/windows-platform-probes/build.rs:34

/MANIFESTINPUT: is emitted without quoting the manifest path. If the workspace path contains spaces, MSVC link.exe will split the argument and fail to find the manifest (or interpret the tail as a separate flag), breaking the “manifest-aware” build in common install locations like C:\Users\...\Source Repos\....

Quote the path in the linker argument so it remains a single token.

        println!("cargo::rustc-link-arg-bin=probe-long-path-aware=/MANIFEST:EMBED");
        println!(
            "cargo::rustc-link-arg-bin=probe-long-path-aware=/MANIFESTINPUT:{}",
            manifest.display()
        );

Files reviewed: 15/16 changed files
Comments generated: 2
Review effort level: Lite

… pair Stage 2 of peeling `windows-platform-probes` off `mikegrier/deferred-namespace-ops` (#56), on top of the report sink stage 1 landed. It adds the long-path family: `long_path` (the measurement), `long_path_report` (the shared renderer), and the two binaries whose *difference* is the finding. The question is whether the `longPathAware` opt-in lifts `MAX_PATH` for a **relative** path. It matters here because a library cannot add a manifest to someone else's executable, so whatever the un-opted-in case does is what a caller of this workspace's crates actually meets. Neither half of that opt-in is a runtime switch -- it is a machine-wide registry value and a per-executable manifest -- so it cannot be a flag on one binary. Hence the pair: the same code, built twice, with `build.rs` embedding the manifest into `probe-long-path-aware` alone. `rustc-link-arg-bin` rather than `rustc-link-arg-bins`, because the latter would opt every probe in the crate into long paths and silently change what the others measure. The aware binary reads a `cargo::rustc-cfg` the script emits from that same guarded block rather than asserting its own manifest, so the label and the linker cannot disagree -- on a non-MSVC target the script emits neither, and a hardcoded claim would have had both halves measuring the un-opted-in case while one announced otherwise. Measured on the development host rather than described, and the two halves do disagree, which is what proves the apparatus works at all: plain 429 chars aware: opened unaware: REFUSED (error 3) with `..` 434 chars aware: opened unaware: REFUSED (error 3) forward slashes 429 chars aware: opened unaware: REFUSED (error 3) The aware side also resolved `..` and forward slashes past the ceiling exactly as below it, so on this host there is no evidence of a regularize-then-prefix implementation -- the sharp edge the report is written to catch. **The two binaries were converted to the stage-1 sink, not carried over as they were.** On the branch they still read `emit(&mut Stdout, &render(...))`, which is the shape stage 1 found destroys a report when a renderer panics. So `long_path_report::render` composes the banner and header, then measures, then renders the body -- the ordering is load-bearing, because a panic in the measurement must still print what was already established. The invariant holds at 10 binaries and 10 `emit_report` call sites, with no direct printing anywhere in the crate. **The report never states more than the run established.** That is the whole of `verdict`, and it is why the guards exist rather than being defensive padding: a shape must have opened *below* the ceiling before "stopped resolving past it" can be said of it; the reference shape must have been tried *above* it; a long failure must be a length refusal rather than an access denial; and both halves of the opt-in must be in effect before a refusal is read as contradicting the documentation. Without that last one the un-opted-in binary declared Microsoft's documentation wrong on every correctly configured host, from the half of the pair whose entire job is to be the baseline. `MAX_PATH_CONTENT` is 259, not 260: a path of exactly `MAX_PATH` content units does not fit, because the terminator needs the last one. That is the convention `windows-namespace-request-sys` and `windows-file-enumeration-sys` already state and assert, and the ceiling column prints the number rather than naming it. **A temporary directory over 200 UTF-16 units is refused before anything else runs.** In a `longPathAware` process an over-long `%TMP%`/`%TEMP%` makes both `std::env::temp_dir()` and the `reg.exe` spawn hang rather than fail -- the process spins on a core and prints nothing at all, since the report only reaches stdout when `render` returns. The limit is chosen rather than derived, and sits far enough short of where the hang starts that the exact boundary stops mattering. On that path `registry_enabled` is `None`, not `false`: the query was never issued, and a `bool` cannot say so. Two Cargo.toml changes, both required by the above: - `wtf-string`, for a UTF-16 code-unit count wherever a length is compared against a Windows limit. `OsStr::len` counts WTF-8 here, so the two disagree the moment a non-ASCII character appears in `%TEMP%` and put an attempt on the wrong side of the ceiling. - `Win32_System_Environment`, for `Get`/`SetCurrentDirectoryW`: the current directory is half of what a relative path resolves against, so the probe places it deliberately rather than inheriting whatever launched it. Verified by running, not only by building: - `cargo check --all-targets` on this package alone against `main`'s tree, which establishes the subset is self-contained: the new code reaches only `std`, `windows-sys`, `wtf-string`, and stage 1's `report`. - Both binaries run to completion, exit 0, and disagree as tabled above. - **The manifest reached exactly one binary**, confirmed by searching the linked images for the manifest's own XML. Searching for the bare string `longPathAware` reports it in *both*, because the report's own label contains it -- the weaker test would have raised a false alarm about the central mechanism. - Workspace `clippy --all-targets --all-features` clean; `fmt --check` clean. - 62 package tests pass (61 unit, 1 integration; 14 are the ignored probe tier). The renderer is split so `body` and `verdict` are pure functions of an `Observation`, which is what lets the report's own tests run without a filesystem: each is named for the wrong sentence it forbids and asserts both that the right claim is made and that the wrong one is not. - `check-workflow-refs`, `check-publishable`, `check-commit-scope`, and the encoding check all pass. Both probes are wired into the CI probe job, and deliberately both: either alone reports a number with no baseline to read it against. Design decisions are recorded in the crate's DESIGN-NOTES.md -- the pair and the one-binary embed, the declined concurrency hardening for `measure` (which moves the process's current directory, and cannot be made safe by a unique root because the directory is per-process), and the finding that the ceiling applies to the path *as written* rather than after `..` collapses. That last one is easy to assume backwards and the obvious experiment does not settle it: `cmd.exe` carries `longPathAware` in its own manifest on this host, so a long path that opens there says nothing about the ceiling. Two things a review on the PR caught: - **The table called a not-found past the ceiling "the length refusal" for every shape**, asserting that "the target provably exists". True for plain, which is legal under `\\?\` too -- that is why it is the control. For `..` and forward slashes it is exactly the ambiguity this probe exists to resolve: if the system prepends `\\?\` past the ceiling, those shapes stop being resolved and the path as written names something that was never created, so the not-found is a genuine absence and *is* the sharp edge. Annotating it as the length refusal files the finding as its own control. The annotation is now keyed on `survives_verbatim_parsing`, and says plainly for the other two that the verdict below decides which. - `build_tree`'s returned relative path was bound and then discarded with `let _ = deep_relative;`. Only the side effect is wanted -- each attempt spells its own path, because the spelling is what is under test. A further review round raised two more; one held and one did not. - **A UNC or ill-formed `%TMP%`/`%TEMP%` is now refused.** The apparatus composes its `\\?\` paths by concatenation, and a UNC root needs `\\?\UNC\server\share` rather than the prefix glued onto `\\server`; separately, `Path::display` substitutes U+FFFD for an unpaired surrogate, which would name a different file than the one created. Both are refused up front for the same reason the length is. Supporting UNC properly is three lines, but no machine this workspace is developed or tested on could exercise it, and an untested path through the apparatus is worth less than an honest refusal. - **The unquoted `/MANIFESTINPUT:` path was reported as fragile against spaces. It is not, and the suggested fix breaks the build.** Measured rather than argued: pointing `build.rs` at a manifest under `C:\lp spaces test\` builds clean and embeds the manifest correctly, because cargo passes the link argument as one argv element and the linker receives it whole. Adding the suggested quotes fails with `LNK1181: cannot open input file 'spaces.obj'` -- the literal quotes make the linker split on the space it had been handling. Left as it is. A third round raised two more, both in the apparatus rather than in the measurement, and both fixed. - **The machine half of the opt-in is read as a number, not as a substring.** `reg query`'s output was tested with `.contains("0x1")`. The reported example -- `0x10` matching -- does not by itself change the answer, since a nonzero value is enabled on either reading; the failure that costs something is the mirror one. A machine set to `0x2` contains no `0x1` and read as *not* opted in, which inverts the conclusion the whole probe exists to reach. The value token is now parsed and compared numerically, and the reading is split from the `reg.exe` spawn so it is tested against captured output rather than against whatever the developing machine happens to be set to. - **`\\?\` and `\\.\` are no longer reported as UNC, and the encoding is classified before any prefix is read.** Both open with the two backslashes a UNC root does, so the UNC test claimed them and told someone with an ordinary local temporary directory that it was a server share. They are still refused, but for their own reason: `\\?\` turns off the path normalisation this probe exists to measure, so a run rooted there would measure the verbatim ceiling and label it the ordinary one. The well-formedness check now runs first, because the prefix tests read the value as text, and reading an ill-formed value as text is the exact substitution that refusal exists to prevent. The `/MANIFESTINPUT:` suggestion was raised again and is unchanged, for the reason measured above. Deliberately excluded, so this stays reviewable: the four other modules `lib.rs` gains on the branch, the remaining probe binaries, and the topology tests in `src/tests.rs`. Those are later stages and each depends on this one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

MikeGrier · 2026-09-08T00:24:07Z

Both new findings are real and both are fixed. The /MANIFESTINPUT: item is a re-raise
of the one measured and refuted last round; unchanged, detail below.

Registry value read as a substring -- accepted, with one correction to the example.
0x10 does not actually produce a false positive: it is nonzero, so both the substring
reading and a numeric one call the machine enabled. The failure that costs something is
the mirror case you also named -- the false negative. A machine set to 0x2 contains no
0x1, read as not opted in, and would have inverted the conclusion this probe exists
to reach, since every row would then be read against a baseline the report believed was
un-opted-in.

The value token is now parsed and compared numerically, and the reading is split out of
the reg.exe spawn (enabled_in) so it is tested against captured reg query output
rather than against whatever the developing machine is set to -- previously untestable.
Two tests cover it: 0x2/0x1/0x0, and the not-the-queried-value cases (absent, wrong
value name, REG_SZ, unparsable, truncated), all of which read as disabled. End-to-end
check: this host reads 0x1 and both probes still report 1.

\\?\ / \\.\ classified as UNC, and the ordering -- accepted, both. You are right
that the two-backslash test claims the device namespace, and the consequence is squarely
the defect this probe is written against: telling someone with an ordinary local temp
directory that it is a server share. They are still refused, but now for their own stated
reason -- \\?\ turns off the path normalisation the probe exists to measure, so a run
rooted there would measure the verbatim ceiling and label it the ordinary one.

The well-formedness check now runs first, for exactly the reason you give: the prefix
tests read the value as text, and reading an ill-formed value as text is the substitution
that refusal exists to prevent. Pinned by an_ill_formed_temp_dir_is_classified_before_its_prefix_is_read,
whose fixture is an ill-formed value beginning \\ -- under the old order it was refused
as UNC.

All five new guards sabotage-verified: disabling each fails exactly its own test and
nothing else, including reverting the ordering to its previous shape.

/MANIFESTINPUT: quoting -- still refuted, unchanged. Measured both ways last round:
building with the manifest under C:\lp spaces test\ succeeds and embeds correctly,
because cargo passes rustc-link-arg-bin as a single argv element and no shell re-splits
it. Applying the suggested quotes then fails with LNK1181: cannot open input file 'spaces.obj' -- the literal quotes are what cause the split.

Gate: fmt --check clean, workspace clippy --all-targets --all-features clean,
cargo test -p windows-platform-probes 62 passed / 14 ignored / 0 failed, encoding check
612 files clean, both probes exit 0. Force-pushed as 3138cc2.

Copilot

🟢 Approval recommended

The manifested-pair mechanism, report rendering/ordering, and the “no overclaim” guardrails are implemented consistently and are backed by targeted unit tests and CI wiring.

Review details

Files reviewed: 15/16 changed files
Comments generated: 0 new
Review effort level: Lite

Brings back stages 1 and 2 of peeling this branch apart: the probe output sink (#79) and the long-path probe pair (#80). Both were taken from this branch, reviewed, and corrected on the way through, so main holds the successor of every file they touch. Twenty-two conflicts, resolved on that basis: - The sixteen peeled sources -- `report.rs`, the long-path family, the eight converted probe binaries, and `build.rs` -- take main's version. Checked rather than assumed: the only content on this branch and not on main is what main replaced (the pre-sink `emit(&mut Stdout, &render())` shape, `registry_enabled: bool`, the `0x1` substring registry read, a bare `183` for ERROR_ALREADY_EXISTS, and the dead `let _ = deep_relative`). - `report.rs` drops this branch's `writeln_to`, which no probe called. Main keeps `emit` and `Stdout`, so the six probes still on this branch and not yet peeled continue to compile against it unchanged. - `lib.rs`, `Cargo.toml`, `ci.yml` and this crate's DESIGN-NOTES take the union: this branch's four probe modules and six binaries alongside the long-path pair, and both sides' design sections. - Both sides had `[[bin]]` entries for the long-path pair. Kept main's, which carries the comment explaining why they are two binaries rather than one with a flag, and dropped this branch's uncommented duplicate -- cargo rejects the manifest outright with both. - Root PLANS.md keeps this branch's newer `windows-ioring-sys` row (M1-M19, not M1-M7) and takes main's new `windows-platform-probes/CHECKLIST.md` row. Main's mutation-survivors row was already here verbatim, so it is not duplicated. One change beyond the resolution: the `GetSystemDirectoryW` comment in Cargo.toml sat above `Win32_System_Environment`, which is not the feature it documents. Moved beside `Win32_System_SystemInformation`, which is. Verified: `check --all-targets` clean with no warnings, `clippy --all-targets --all-features` clean, `fmt --check` clean, 73 package tests pass, encoding check 643 files clean, workflow references resolve, and all seventeen probe binaries build and run to exit 0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

MikeGrier · 2026-09-08T01:28:30Z

Processed. The one suppressed finding in this review was the /MANIFESTINPUT: quoting
suggestion, re-raised from review 5135879376 and refuted there by measurement: building with
the manifest under C:\lp spaces test\ succeeds and embeds correctly, because cargo passes
rustc-link-arg-bin as a single argv element and no shell re-splits it; applying the suggested
quotes then fails with LNK1181: cannot open input file 'spaces.obj'. build.rs is unchanged
and hash-verified. Its two inline findings were accepted and fixed in 3138cc2, and are resolved.

…mark the ones that are not PR #56 has accumulated 196 Copilot reviews and nothing said which had been dealt with. Judging by eye does not scale, and judging by "did a commit follow it" is guesswork. tools/scan-pr-reviews.ps1 reports the two signals that are real and adds the one GitHub does not have. Findings arrive in two shapes and only one of them has state: - Inline comments become review threads, which can be RESOLVED. That flag is durable, visible and queryable, so it is the tag -- nothing to invent. - Suppressed comments exist only as prose in the review body's <details> block. They create NO thread, so there is nothing to resolve, and a review is not a reactable object either: POST /pulls/{n}/reviews/{id}/reactions returns 404, while the same call against an inline comment succeeds (verified both ways). Nothing anywhere records that a suppressed finding was read, which is the gap this closes. For those, -MarkProcessed posts a marker comment on the pull request:  An HTML comment, so it does not render; on the pull request rather than in a file or a session, so it survives a new machine, contributor, or agent session; and read back by the script, so a handled review stops being reported. A -Summary is required alongside it, because a marker with no account of what was done is a claim with no evidence. The report separates unresolved threads into current and outdated. Outdated means the anchored line has since changed, which usually means the finding was fixed and the thread never resolved, so those are cheap to clear and are listed only under -IncludeOutdated. Exercised end to end on PR #80, which had four reviews all genuinely handled in this session: its six threads are now resolved, its one suppressed-only review (5136043258, the /MANIFESTINPUT: suggestion refuted by measurement) is marked, and a re-scan reads back clean at exit 0. Deliberately NOT done: back-filling PR #56's 131 suppressed-carrying reviews. A marker asserts the review was read. Almost all were addressed in the rounds that followed them, but "almost all" is not evidence, and marking them wholesale would convert an honest absence of information into a false record. Runs on both PowerShell hosts, through the shared Invoke-Native guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…st markers from write access Three defects from a review round on this branch. Two were the reviewer's; the third was found by running the tool this branch adds against its own pull request, which reported it. **scan-pr-reviews.ps1 exited 1 for both "there are findings" and "the tool broke".** Every error path was an unhandled `throw`, and an unhandled throw from a script also exits 1, so a caller gating on the code could not tell "2 unresolved threads" from "gh is not authenticated" or "no such pull request". That is the instrument-versus-finding distinction `run-numa-spikes` and `run-sabotage` already make by exit code. Error paths now leave through `Exit-Broken` with code 2; 1 stays the finding. A `ConvertFrom-Json` failure is covered too, which is the realistic shape of a proxy returning an HTML error page to a `gh` that still exited 0. Verified on both hosts: nonexistent PR -> 2, missing -Summary -> 2, PR with open threads -> 1, clean PR -> 0. **Processed markers were honoured from any comment author.** This repository is public, so anyone able to comment could post `` and permanently retire a review from every future scan. Because a suppressed-only review has no state anywhere else -- the whole reason the tool exists -- that would silently delete the only record that a finding was never read, and the summary would just report a smaller number. Only OWNER, MEMBER and COLLABORATOR are now honoured; CONTRIBUTOR means merely "has had a pull request merged" and is not enough. `author_association` arrives on every comment in the same request, so this costs no extra call. Markers from other authors are counted and reported rather than dropped in silence, because such a marker is either an honest mistake or an attempt to retire a finding and both are worth seeing. Verified on both hosts across all six association values, including a missing field, which fails closed; the real OWNER marker on PR #80 is still honoured, so the legitimate path is unaffected. **Restatement drift: two places still said the guard "restores" the preference.** The previous commit removed that restoration as dead code and corrected the implementation comment, but left the claim standing in `common.ps1`'s own summary paragraph and in DESIGN-NOTES. Both now state what the code does -- the flip is function-local rather than restored -- and why the property worth testing is that it does not escape. Swept `restor` across `tools/` and DESIGN-NOTES: 49 matches, 2 stale and corrected, the rest about file restoration in the sabotage harness and unrelated crate behaviour. Found by running `scan-pr-reviews.ps1 -Pr 81`, which is the tool this branch adds; it reported both stale sites as unresolved Copilot threads. The instrument catching a defect in its own change set is the outcome it was written for. Verified: `test-common.ps1` passes on both hosts, the sabotage suite passes on both, encoding 615 files clean, workflow references resolve. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…lds exiting as findings Two findings from a review round, both confirmed by measurement on both hosts. **Nullable API fields would have exited 1 -- the "findings" code.** Several property chains over GraphQL and REST data are nullable BY SCHEMA rather than by accident: `author` and `pullRequestReview` are null for a deleted account, REST `user` likewise, `submitted_at` is null on a PENDING review, and a thread's `comments.nodes` can be empty. Under `Set-StrictMode -Version Latest` every one of those is a TERMINATING error, and an unhandled throw from a script exits 1 -- which the exit contract added two commits ago defines as "there are outstanding findings". A contributor deleting their GitHub account would have silently converted this tool into a false positive. Measured on both hosts: `$null.login`, `@()[0]` and `[datetime]$null` all throw under StrictMode, and a JSON-sourced null behaves identically. Added `Get-Path`, which walks a property chain yielding `$null` instead of throwing, and routed every such access through it; a thread that cannot be attributed to a review is skipped rather than attributed to review 0, and a PENDING review renders as `pending` rather than being cast. **The marker-trust check was wider than it claimed.** It trusted `author_association` in OWNER/MEMBER/COLLABORATOR and described those as "the ones that imply write access". They are not: GitHub reports `COLLABORATOR` for anyone invited to collaborate, with no permission qualifier, so a collaborator with `read` or `triage` could post a marker and permanently retire a suppressed-only finding that has no other state anywhere -- without even being counted as ignored. Replaced with an actual permission lookup against the collaborators endpoint, accepting only `admin` or `write`. One call per DISTINCT marker author, cached, and markers are rare, so this is a call or two per scan. It **fails closed**: a 404 for a non-collaborator, a 403 because the account running the scan cannot query permissions, or any network failure leaves the marker unhonoured. That direction is deliberate -- over-reporting a handled finding is visible and recoverable, while wrongly honouring a marker silently deletes the only record that a finding was never read. The lookup deliberately does not go through `Invoke-GitHubJson`, because a non-zero exit there is the ordinary answer for a non-collaborator rather than a broken instrument. Verified against real accounts on both hosts: the owner may retire, `octocat` and `torvalds` may not, an empty login may not; and the existing OWNER marker on PR #80 is still honoured, so the legitimate path is unchanged. The claim is corrected in both places that stated it -- the script header and DESIGN-NOTES -- since the old wording described an enforcement the code did not perform. Swept for the stale phrasing; no occurrences remain. Verified: `test-common.ps1` passes on both hosts, the sabotage suite passes on both, exit contract intact (findings 1, clean 0, nonexistent PR 2), encoding 615 files clean, workflow references resolve. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

… its author A review round found that the marker-trust check collapsed two different outcomes into one message, and the message asserted the one the run had not established. That is the same defect this whole tool exists to prevent, so it is worth naming precisely. `GET /repos/{owner}/{repo}/collaborators/{user}/permission` requires the CALLER to have push access. An account without it gets a flat 403 for every login it asks about -- including the maintainer who wrote the markers. The old code mapped every non-zero exit to `$false` and printed: ignored markers: N (author lacks write access) In that case the author has `admin`. A reader following the message goes and checks the marker author's collaborator role, which is fine, rather than their own token, which is the actual cause. Measured against the live API, there are three outcomes and the old code had two. The third is not an error at all, which is what made the conflation easy: exit 0, admin|write -> allowed exit 0, read|none -> denied (a statement about the author) exit 1, HTTP 403 -> unverifiable (a statement about the caller) `octocat` on this repository returns exit 0 with `read`, and `github-actions[bot]` returns exit 0 with `none` -- both successful answers, not failures. Only the 403 is a failure, and only it is unverifiable. `Get-RetireAuthority` now returns those three, counted and reported separately. The unverifiable message says whose problem it is and that every marked review is being re-reported as a consequence. All three still fail closed -- only `allowed` honours a marker -- because over-reporting a handled finding is visible and recoverable, while wrongly honouring one silently deletes the only record that a finding was never read. The bot result is worth its own note, and is now in the script header and DESIGN-NOTES: a marker posted from a workflow using GITHUB_TOKEN is written by `github-actions[bot]`, whose permission reads `none`, so it can never be honoured. Mark from a real account. Measured rather than assumed. Verified on both hosts against real accounts -- owner allowed, non-collaborator denied, bot denied, a repository where this account lacks push unverifiable, and a comment with no author denied -- 5 of 5 correct. Exit contract intact (PR #80 0, PR #81 0, nonexistent PR 2), `test-common.ps1` and the sabotage suite pass on both hosts, encoding 615 files clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

MikeGrier · 2026-09-08T04:27:19Z

Re-marked in the sentinel-bracketed format.

The original marker on this pull request was a bare 
anywhere in the body, and a later review round pointed out that this cannot tell a comment
that IS a marker from one that merely MENTIONS one -- so quoting a marker in discussion, or
pasting the example out of the tool's own documentation, would silently retire a review.
Markers are now honoured only on a line of their own inside a begin/end sentinel block, and
the earlier bare marker correctly stopped counting. Its finding is unchanged and still
handled: the suppressed /MANIFESTINPUT: quoting suggestion was measured and refuted --
building with the manifest under a path containing spaces succeeds, and adding the suggested
quotes fails with LNK1181.

…r the text appears Copilot review 5137260817 on PR #81, whose two findings were both suppressed -- so they created no thread and, without this branch's own scanner, nothing would have surfaced them at all. Marker recognition matched `` ANYWHERE in a comment body. That cannot tell a comment that IS a marker from one that merely MENTIONS one, so a maintainer quoting a marker in discussion -- or pasting the example out of this script's own documentation -- would silently retire a review. Since a suppressed-only review has no other state anywhere, retiring it deletes the only record that a finding was never read. Not currently firing: the sole marker in the repository is the legitimate one on PR #80. But this branch publishes the literal marker text in its own PR body and DESIGN-NOTES, so the material for an accidental retirement is already written. Markers are now emitted bracketed:    and honoured only on a line of their own inside such a block. The sentinels are defined once so emitter and reader cannot drift, which is the failure this whole tool is about. Both documentation examples now spell the id as a placeholder rather than digits, so copying an example verbatim -- sentinels included -- matches nothing. Verified 9 discrimination cases on both hosts, 9 of 9: a real marker and a two-id block are honoured; a bare marker, one quoted inline in prose, one inside a fenced block, the placeholder example, a marker sharing a line with a sentinel, an unterminated block, and an empty body are all rejected. The existing marker on PR #80 was bare, so it correctly stopped counting -- the scan went back to exit 1 there, which is the fail-closed direction. Re-marked in the new format and confirmed exit 0 again; its finding is unchanged. Verified: exit contract intact on both hosts (PR #80 0, nonexistent PR 2), test-common.ps1 13 cases pass on both, the sabotage suite passes on both, encoding 615 files clean, workflow references resolve. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings September 7, 2026 23:31

Copilot started reviewing on behalf of MikeGrier September 7, 2026 23:31 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread crates/windows-platform-probes/src/long_path.rs Outdated

Comment thread crates/windows-platform-probes/src/long_path_report.rs Outdated

Copilot AI review requested due to automatic review settings September 7, 2026 23:39

MikeGrier force-pushed the mikegrier/probes-long-path branch from 439e01e to abb732b Compare September 7, 2026 23:39

Copilot started reviewing on behalf of MikeGrier September 7, 2026 23:40 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread crates/windows-platform-probes/build.rs

Comment thread crates/windows-platform-probes/src/long_path.rs

Copilot AI review requested due to automatic review settings September 8, 2026 00:08

MikeGrier force-pushed the mikegrier/probes-long-path branch from abb732b to 92a46f3 Compare September 8, 2026 00:08

Copilot started reviewing on behalf of MikeGrier September 8, 2026 00:09 View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

Comment thread crates/windows-platform-probes/src/long_path.rs

Comment thread crates/windows-platform-probes/src/long_path.rs Outdated

Copilot AI review requested due to automatic review settings September 8, 2026 00:23

MikeGrier force-pushed the mikegrier/probes-long-path branch from 92a46f3 to 3138cc2 Compare September 8, 2026 00:23

Copilot started reviewing on behalf of MikeGrier September 8, 2026 00:24 View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

MikeGrier merged commit d82048f into main Sep 8, 2026
29 checks passed

MikeGrier deleted the mikegrier/probes-long-path branch September 8, 2026 00:36

MikeGrier mentioned this pull request Sep 8, 2026

feat(tools): share the native-command guard, and report which Copilot reviews are outstanding #81

Merged

This was referenced Sep 9, 2026

Survey the machine topology and cross-check the shipping parse #82

Merged

Measure what a doorbell and a namespace request cost #83

Merged

Navigation Menu

Uh oh!

feat(platform-probes): measure the long-path opt-in with a manifested pair - #80

feat(platform-probes): measure the long-path opt-in with a manifested pair#80
MikeGrier merged 1 commit into
mainfrom
mikegrier/probes-long-path

MikeGrier commented Sep 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

MikeGrier commented Sep 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

MikeGrier commented Sep 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

MikeGrier commented Sep 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

MikeGrier commented Sep 8, 2026

Uh oh!

MikeGrier commented Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

MikeGrier commented Sep 7, 2026

The question

The result

What is worth a reviewer's attention

Cargo.toml

Verified by running, not only by building

Design record

Deliberately excluded

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

MikeGrier commented Sep 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

MikeGrier commented Sep 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

MikeGrier commented Sep 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

MikeGrier commented Sep 8, 2026

Uh oh!

MikeGrier commented Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants