Say precisely what GetFullPathNameW does, and why it stays

[MikeGrier]

Peels the GetFullPathNameW correction ("G") out of #56, and completes M2.6.

This description has been rewritten. The version it replaces asserted, in bold, that "touches no filesystem" is the claim that holds -- which this PR went on to measure false. Leaving that standing while the diff below disproves it would have been one more instance of the defect the PR is about, so the record is corrected here rather than only in the code.

The defect

full_path.rs said, in two consecutive sentences:

This call is lexical. It resolves relative components and ./.. against the process current directory[.]

Those disagree. A lexical canonicalizer is a pure function of its input; this reads mutable process state -- the current directory, and for a drive-relative path such as C:foo the per-drive current directory Windows keeps in the hidden =C: environment variables.

That was the entry point. The subject turned out not to be one wrong word.

What the call actually does

Measured by P/Invoke, not read off documentation:

• It does not verify what it produces. That is the documented guarantee, and it is narrower than it sounds. "Touches no filesystem" is false, not merely unproven.
• Two separable jobs. Collapsing ./.. is pure string work that reads no process state (C:\a\..\b is C:\b under any current directory). Only rooting reads process state.
• A drive-relative path touches the filesystem and writes the environment. Resolving X:foo for a non-current drive validates the =X: entry against both a shape and existence -- an existing file is rejected, as is a missing directory, C:/..., ...\., ...\..\... and \\?\... -- and an entry that fails is written back as the drive root, created when absent, so this happens on a pristine host on every call. An accepted entry is used verbatim, even onto another drive, and literally: C:\Windows\ yields C:\Windows\\foo.
• For the current drive, neither happens -- the entry makes no difference to the result and is not rewritten. (Whether it is read is deliberately not claimed: installing a value and watching the outcome cannot separate "not read" from "read and ignored".)
• A legacy device name short-circuits rooting entirely -- and NUL does so even from a fully qualified path (C:\NUL resolves to \\.\NUL, where every other device word roots normally once anything precedes it). CON, CON:, CON., con, CONIN$ and COM/LPT plus one digit -- including the superscripts U+00B9/B2/B3, which a hand-written denylist misses.
• Cost. The probe's build - clone gap (~165 ns) is an upper bound spanning one net allocation and a builder chain; the call alone measures ~110 ns.

The decision, recorded as D-18

A genuinely lexical canonicalizer exists -- PathCchCanonicalizeEx, or PathAllocCanonicalize.

It is the wrong call here, and the reason is the property rather than the price. Resolving against the current directory at submission is what this crate is buying. A lexical canonicalizer would leave a relative path relative, so its meaning would be settled on the worker at execution time, against a directory any thread may have changed in between -- reintroducing exactly the race preparation exists to close. No cost claim is made about the alternatives, because none was measured.

Whether it enters the kernel is still not established, and the earlier argument for that -- "both are ordinary process memory" -- has been withdrawn. The reasoning was sound and the conclusion was false: the drive-relative form queries the filesystem and writes the environment block.

The real subject: claims that outrun their evidence

The PR ran to fourteen review rounds, and nearly every one found the fix for the previous round committing a fresh instance of the same error. That history is now the worked example, recorded in DESIGN-RATIONALE.md: nine successive wrong drafts, each naming a mechanism the evidence did not reach.

The sharpest instance is not about GetFullPathNameW at all. A test helper folded "empty =X: value" into "absent entry", in a comment that called the equivalence measured. The measurement had never cleared the last error -- the only thing that distinguishes them -- so it could read nothing but what an earlier call left behind. Cleared and re-measured, they are distinct. The consequence was live: the guard that restores a borrowed entry on unwind deleted an inherited empty one, destroying the state it exists to preserve.

So: "measured" is itself a claim, and a procedure can be wrong in ways its result never reveals.

The sweep

M2.6 named one file. Sweeping found ten statements of the same fact in six files -- including one in a file an earlier commit had already edited, and one used as a live premise inside an open milestone in the root CHECKLIST. The reported site is a sample, not the population.

Deliberately not touched: the 2026-08-27 design session uses "lexical canonicalisation" generically and correctly, and a Tier 3 transcript is a raw record rather than a normative statement.

Scope

27 commits. Not documentation-only, and the earlier claim that it was is corrected here: the branch adds executable tests and a [dev-dependencies] windows-sys entry (Win32_Foundation, Win32_System_Environment) needed because std::env cannot address a key containing =. Commit types are scoped accordingly -- docs, test, and two fix commits in windows-namespace-request-sys plus one in the probes crate. The library's public API and behaviour are unchanged; every code change is in tests, comments, or the probe's report text.

234 lib tests, 37 doctests, 20 + 11 acceptance tests, and 158 probe tests pass; clippy --all-targets, rustfmt, rustdoc and the encoding gate over 629 files are clean.

[Copilot]

🟢 Approval recommended

The changes are documentation/design-note corrections with a consistent workspace-wide sweep and no remaining inconsistencies found in the updated hunks.

Pull request overview

This PR corrects and precisely documents GetFullPathNameW semantics (not “lexical”, and “touches no filesystem” vs. “reads mutable process state”), and records the decision to keep it because submission-time resolution is the property the crate needs.

Changes:

• Updates windows-namespace-request-sys docs (and its design notes) to state exactly what GetFullPathNameW does, why it remains the right call, and names the genuinely-lexical alternatives.
• Sweeps and corrects restatements across the workspace (design notes, checklists, probe docs, tests/examples) to remove the incorrect “lexical” claim.
• Marks the windows-platform-probes checklist item M2.6 complete and links readers to the owning-crate decision (D-18) instead of re-deriving assumptions in the probe.

File summaries

File	Description
DESIGN-NOTES.md	Updates the root design note to remove the incorrect “lexical” rationale and point to windows-namespace-request-sys decision D-18.
crates/windows-platform-probes/src/request_cost.rs	Adds a pointer to the owning crate’s D-18 decision to avoid probe-side re-derivation.
crates/windows-platform-probes/src/bin/device_map.rs	Rewords output text to remove the incorrect “lexical resolution” phrasing.
crates/windows-platform-probes/CHECKLIST.md	Marks M2.6 as completed.
crates/windows-namespace-request-sys/tests/acceptance/operations.rs	Updates an acceptance comment to remove “lexical” wording.
crates/windows-namespace-request-sys/src/path.rs	Corrects the crate docs to state “touches no filesystem” and “not lexical”, including the process-state inputs.
crates/windows-namespace-request-sys/src/full_path/tests.rs	Updates test docs/comments to remove the “lexical” claim and keep the “touches no filesystem” property.
crates/windows-namespace-request-sys/src/full_path.rs	Expands and clarifies the module docs: what it solves, why not the lexical alternatives, and what is/ isn’t established about kernel transitions.
crates/windows-namespace-request-sys/DESIGN-NOTES.md	Adds D-18 capturing the correction, the “keep GetFullPathNameW” decision, and the named alternative(s).
CHECKLIST.md	Updates M20.1 premise to remove “lexical” and retain the true invariant (“never expands a drive letter”).
CHECKLIST-thread-ambient.md	Updates the audited call table and M26.5 record to remove “lexical” and preserve the correct properties.

Review details

• Files reviewed: 11/11 changed files
• Comments generated: 0
• Review effort level: Lite

---

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

[Copilot]

🟡 Changes recommended

Several statements exceed the documented Win32 contract, make unmeasured cost claims, and leave completed checklist records unarchived.

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

Review details

Suppressed comments (4)

crates/windows-namespace-request-sys/src/full_path/tests.rs:50

• This test only establishes that the resulting path need not exist; it cannot establish that the API never accesses the filesystem. Phrase the comment in terms of the behavior under test.

    // The call touches no filesystem. A consumer wanting a verified path wants
    // an open plus GetFinalPathNameByHandleW.

crates/windows-namespace-request-sys/src/full_path.rs:147

• The example should not claim that no filesystem access occurs when the documented contract only guarantees no validity/existence check.

/// // `.` and `..` are resolved without touching the filesystem.

crates/windows-namespace-request-sys/src/full_path.rs:164

• Resolving a nonexistent path proves that existence is not checked, but not that the function performs no filesystem access. Use the actual documented guarantee here.

/// // A path to nothing resolves perfectly happily, because the call
/// // touches no filesystem. A consumer wanting a verified path wants an open plus
/// // GetFinalPathNameByHandleW instead.

CHECKLIST-thread-ambient.md:423

• This completed record now embeds the same unsupported zero-filesystem-access claim. The documented behavior is that the result is not checked for validity or existence.

- [x] **M26.5** -- The `GetFullPathNameW` entry. Touches no filesystem: it resolves relative components
  and `.`/`..` against the process current directory, and never expands a drive letter, so it does
  **not** close the session-relative hazard from M20.1, and its documentation must say which problem it
  solves and which it leaves standing.

• Files reviewed: 11/11 changed files
• Comments generated: 11
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Unsupported cost claims and repository documentation/checklist conventions remain unresolved.

Review details

Suppressed comments (5)

crates/windows-namespace-request-sys/src/full_path.rs:35

• The claim that either alternative is cheaper is not established here: the cited probe measures GetFullPathNameW only, Microsoft documents no comparative cost, and PathAllocCanonicalize performs its own allocation. Since the decision depends only on avoiding process-state reads, remove the performance claim unless a direct measurement is added.

//! One exists: `PathCchCanonicalizeEx`, or `PathAllocCanonicalize`. Either is
//! cheaper and reads no process state at all.
//!
//! **They are the wrong call here, and the reason is the property above rather

crates/windows-namespace-request-sys/DESIGN-NOTES.md:558

• This decision repeats an unverified performance conclusion. The available probe does not benchmark either canonicalizer, and the platform documentation specifies behavior rather than relative cost. State the relevant property—no process-state lookup—without calling the alternatives cheaper.

**The decision: keep `GetFullPathNameW`.** A genuinely lexical canonicalizer
exists -- `PathCchCanonicalizeEx`, or `PathAllocCanonicalize` -- and is cheaper,
reading no process state at all. It is the wrong call here, and for the property
rather than the price: resolving against the current directory *at submission* is
what this crate is buying. A lexical canonicalizer would leave a relative path

crates/windows-platform-probes/src/request_cost.rs:83

• Calling the alternative “cheaper” reintroduces an unsupported mechanism/cost claim into the probe documentation. Neither alternative is measured by this probe, so describe it only as lexical or process-state-independent.

//! `D-18`, which states what the call actually does, records keeping it over
//! the cheaper lexical alternative, and says plainly that whether it enters the
//! kernel is not established.

crates/windows-platform-probes/CHECKLIST.md:91

• Checking this multi-line item leaves a large completed item in the active checklist. The repository’s completed-item rule requires moving an item over 100 characters (or with sub-items) immediately to COMPLETED-CHECKLIST.md and replacing it here with the one-line anchored completion stub.

- [x] **M2.6** -- Say precisely what `GetFullPathNameW` does, in the crate that owns it, and decide

crates/windows-namespace-request-sys/DESIGN-NOTES.md:537

• D-18 adds a new decision but keeps its correction history, sweep narrative, and rejected first-draft reasoning entirely in Tier 1. The repository’s design-document rules require new decisions in a component with substantial design history to record rationale in DESIGN-RATIONALE.md in the same commit; keep the current decision and constraint here, and move/link the historical material to Tier 2.

## <a id="d-18"></a>D-18: `GetFullPathNameW` is not lexical, and the genuinely lexical alternative is the wrong call

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The documentation asserts an unmeasured performance ordering and has unresolved design-record formatting and rationale issues.

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

Review details

Suppressed comments (3)

crates/windows-namespace-request-sys/src/full_path.rs:42

• Calling both alternatives “cheaper” is an unsupported performance claim: the only measurement documented below is the current construct-and-drop path, and PathAllocCanonicalize performs its own allocation. The semantic argument does not need this claim; describe only the observable contract unless an isolated comparison is added.

//! One exists: `PathCchCanonicalizeEx`, or `PathAllocCanonicalize`. Either is
//! cheaper and reads no process state at all.

crates/windows-namespace-request-sys/DESIGN-NOTES.md:566

• This repeats the unmeasured “cheaper” comparison as part of the canonical decision. The probe explicitly does not isolate this call, much less compare it with either PathCch API, so retain the relative-path semantics that decide the choice and remove the cost assertion.

**The decision: keep `GetFullPathNameW`.** A genuinely lexical canonicalizer
exists -- `PathCchCanonicalizeEx`, or `PathAllocCanonicalize` -- and is cheaper,
reading no process state at all. It is the wrong call here, and for the property

crates/windows-platform-probes/src/request_cost.rs:83

• This cross-reference turns the unmeasured “cheaper” comparison into probe documentation. D-18’s decisive fact is that the alternative leaves relative input relative; refer to that contract rather than asserting a performance ordering the probe never measured.

//! `D-18`, which states what the call actually does, records keeping it over
//! the cheaper lexical alternative, and says plainly that whether it enters the
//! kernel is not established.

• Files reviewed: 12/12 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The documentation makes unsupported performance attributions and does not follow the repository’s design and checklist archival conventions.

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

Review details

Suppressed comments (7)

crates/windows-namespace-request-sys/src/full_path.rs:48

• The “cheaper” comparison is not established: the probe never invokes either PathCch alternative, and the API documentation makes no relative-performance guarantee. Since this change is specifically avoiding mechanism claims beyond the evidence, please remove the cost claim (or add an actual benchmark) and keep the decision based on the required rooting behavior.

//! One exists: `PathCchCanonicalizeEx`, or `PathAllocCanonicalize`. Either is
//! cheaper and reads no process state at all.

crates/windows-namespace-request-sys/DESIGN-NOTES.md:566

• The new decision records “cheaper” as a fact, but no benchmark in this repository calls either PathCch alternative, and their API contracts provide no comparative performance guarantee. Remove the unsupported cost conclusion or benchmark the alternatives; the current-directory-rooting requirement is sufficient to justify the decision.

**The decision: keep `GetFullPathNameW`.** A genuinely lexical canonicalizer
exists -- `PathCchCanonicalizeEx`, or `PathAllocCanonicalize` -- and is cheaper,
reading no process state at all. It is the wrong call here, and for the property

crates/windows-platform-probes/src/request_cost.rs:83

• This repeats the unmeasured assertion that the lexical alternative is cheaper. The probe does not call PathCchCanonicalizeEx or PathAllocCanonicalize; it only compares building a request with cloning an existing path. Describe the alternative by its established behavior, not by an unsupported relative cost.

//! The owning crate now settles both halves rather than leaving them to be
//! re-derived from a probe: see `windows-namespace-request-sys`'
//! [DESIGN-NOTES.md](../../windows-namespace-request-sys/DESIGN-NOTES.md) ->
//! `D-18`, which states what the call actually does, records keeping it over
//! the cheaper lexical alternative, and says plainly that whether it enters the
//! kernel is not established.

crates/windows-platform-probes/COMPLETED-CHECKLIST.md:104

• The resolution summary turns the unmeasured “cheaper” premise into a completed finding. No probe here invokes PathCchCanonicalizeEx, so the archive should record only the established semantic difference unless a comparative benchmark is added.

`GetFullPathNameW` collapses `.`/`..` lexically but roots a path that is not fully qualified against
process state, so it is not a lexical call as a whole; `PathCchCanonicalizeEx` is genuinely lexical,
cheaper, and the wrong call, because rooting at submission is the property being bought. Whether it
enters the kernel is recorded as NOT established. The item's body below is the request as it was

crates/windows-platform-probes/COMPLETED-CHECKLIST.md:97

• This archive entry does not follow the repository’s completed-item format (.github/copilot-instructions.md:1398-1412): the date group must be a ## heading, followed by a separate ### anchored item heading that repeats the stub summary and carries the inline completion timestamp. Keeping that shape makes the stub target and archive index consistent with the other completed items.

## <a id="m26"></a>Moved 2026-09-09 22:54:01 -04:00 -- M2.6: what `GetFullPathNameW` does, and whether it stays

crates/windows-namespace-request-sys/DESIGN-NOTES.md:537

• This new D-18 section mixes the current decision with draft history and alternative-analysis prose, but the repository’s design-document convention requires new decisions to be written to both Tier 1 DESIGN-NOTES.md and Tier 2 DESIGN-RATIONALE.md in the same commit (.github/copilot-instructions.md:1495-1512). Please keep a compact canonical decision here and add the historical reasoning to a cross-referenced Tier 2 file.

## <a id="d-18"></a>D-18: `GetFullPathNameW` is not lexical, and the genuinely lexical alternative is the wrong call

crates/windows-namespace-request-sys/DESIGN-NOTES.md:560

• Repository documentation requires file references to be clickable links. path.rs is the only explicit file name in this inventory left as inline code; link it like the adjacent full_path.rs and tests.rs references.

with two more in `path.rs`, two in [tests.rs](src/full_path/tests.rs), one in an

• Files reviewed: 12/12 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The documentation includes version-dependent device-name behavior and unsupported performance claims, alongside required design/checklist formatting issues.

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

Review details

Suppressed comments (8)

crates/windows-namespace-request-sys/src/full_path.rs:56

• Neither the Windows contract nor any repository benchmark establishes that these alternatives are cheaper; reading less process state does not by itself prove lower runtime cost, and PathAllocCanonicalize allocates its result. Remove the performance claim and keep the property-based rationale.

//! One exists: `PathCchCanonicalizeEx`, or `PathAllocCanonicalize`. Either is
//! cheaper and reads no process state at all.

crates/windows-namespace-request-sys/DESIGN-NOTES.md:566

• The decision repeats an unverified performance claim: no direct comparison in this repository establishes that either lexical API is cheaper, and PathAllocCanonicalize also performs allocation. The keep/remove decision already rests on semantics, so state only that the alternatives omit current-directory rooting.

**The decision: keep `GetFullPathNameW`.** A genuinely lexical canonicalizer
exists -- `PathCchCanonicalizeEx`, or `PathAllocCanonicalize` -- and is cheaper,
reading no process state at all. It is the wrong call here, and for the property

crates/windows-platform-probes/src/request_cost.rs:83

• This propagates the unsupported claim that the lexical alternative is cheaper. The probe does not compare these APIs, and reduced process-state access is not a performance measurement; describe it simply as the lexical alternative.

//! [DESIGN-NOTES.md](../../windows-namespace-request-sys/DESIGN-NOTES.md) ->
//! `D-18`, which states what the call actually does, records keeping it over
//! the cheaper lexical alternative, and says plainly that whether it enters the
//! kernel is not established.

crates/windows-platform-probes/COMPLETED-CHECKLIST.md:103

• The resolved summary presents PathCchCanonicalizeEx as cheaper even though no comparison in this PR or the probe establishes that. Remove the cost claim here as well so this restatement matches the property-based decision.

`GetFullPathNameW` collapses `.`/`..` lexically but roots a path that is not fully qualified against
process state, so it is not a lexical call as a whole; `PathCchCanonicalizeEx` is genuinely lexical,
cheaper, and the wrong call, because rooting at submission is the property being bought. Whether it

crates/windows-namespace-request-sys/DESIGN-NOTES.md:537

• This adds a new canonical decision but no Tier 2 rationale. The repository design-document rule requires new decisions to be recorded in both DESIGN-NOTES.md and DESIGN-RATIONALE.md in the same commit; this component has substantial design history, so add the rationale file/section and cross-link D-18.

## <a id="d-18"></a>D-18: `GetFullPathNameW` is not lexical, and the genuinely lexical alternative is the wrong call

crates/windows-namespace-request-sys/DESIGN-NOTES.md:560

• path.rs is a repository file reference but is left as inline code. The markdown cross-reference rule requires references readers should open to be clickable relative links.

with two more in `path.rs`, two in [tests.rs](src/full_path/tests.rs), one in an

crates/windows-platform-probes/COMPLETED-CHECKLIST.md:97

• This archive entry does not follow the required completed-item structure: the date group must be an unanchored ## Moved ... heading, followed by a ### item heading carrying the m26 anchor, the same summary as the stub, and the inline completion timestamp.

## <a id="m26"></a>Moved 2026-09-09 22:54:01 -04:00 -- M2.6: what `GetFullPathNameW` does, and whether it stays

crates/windows-platform-probes/CHECKLIST.md:92

• The completed-item rule requires the replacement stub to occupy one physical line. Keeping it on two lines defeats the compact active-checklist format used by the repository's existing completed stubs.

- [x] **M2.6** -- Say precisely what `GetFullPathNameW` does, in the crate that owns it, and decide
  whether it is still the call `prepare` wants. -> [completed 2026-09-09](COMPLETED-CHECKLIST.md#m26)

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The new contract documentation has an internal sequencing contradiction and lacks required rationale separation and coverage for its platform-sensitive claims.

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

Review details

Suppressed comments (1)

crates/windows-namespace-request-sys/DESIGN-NOTES.md:537

• This new decision is recorded only in Tier 1 and embeds substantial draft history and alternatives. The repository convention requires new decisions to be written to both DESIGN-NOTES.md and DESIGN-RATIONALE.md; move the historical material (especially the draft chronology) to a new Tier 2 entry and keep D-18 focused on the current decision and constraints. See .github/copilot-instructions.md:1493-1512.

## <a id="d-18"></a>D-18: `GetFullPathNameW` is not lexical, and the genuinely lexical alternative is the wrong call

• Files reviewed: 12/12 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Several summaries still omit the current-drive exception, and the PR description overstates whether the entry is consulted.

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

Review details

Suppressed comments (5)

DESIGN-NOTES.md:1467

• This summary omits D-18’s current-drive exception: X:foo uses the process current directory when X is the current drive; only another drive uses its recorded =X: entry. As written, this restates the contract too broadly.

  but roots most paths that are not fully qualified against process state -- the
  current directory, or for a drive-relative path the entry recorded for that
  drive, which moves independently of it -- and that rooting is the property

CHECKLIST-thread-ambient.md:423

• The completed item still states the drive-relative rule without its current-drive arm. The new tests and D-18 show that the current drive uses the process directory and ignores the recorded entry; only another drive uses =X:.

- [x] **M26.5** -- The `GetFullPathNameW` entry. Does not verify its result: it collapses `.`/`..`
  lexically and roots most paths that are not fully qualified against process state -- the current
  directory, or for a drive-relative path the entry recorded for that drive -- and never expands a
  drive letter, so it

crates/windows-platform-probes/src/request_cost.rs:71

• This decomposition again makes every drive-relative path sound as though it reads the per-drive entry. D-18 and the added test establish a separate current-drive arm that uses the process current directory instead, so this probe documentation should preserve that distinction.

//! That call reads **process state** when it has to root a path -- the current
//! directory, or for a drive-relative path the entry recorded for that drive in
//! the `=C:` environment variables. **Neither sample here is rooted**: both are

crates/windows-namespace-request-sys/DESIGN-RATIONALE.md:110

• This historical summary says the final enumeration has three forms, but its drive-relative form still omits the measured current-drive branch. On the current drive the process directory wins; the recorded =X: entry is used only for another drive.

- **The rooting forms.** Stated first as one case (the current directory), then
  two, and finally three: a relative path takes the current directory, a
  root-relative path takes only that directory's *root* (which is
  `\\server\share\` under a UNC current directory, so "current drive" was
  wrong), and a drive-relative path takes the entry recorded for that drive
  from `=C:`.

crates/windows-namespace-request-sys/src/full_path/tests.rs:552

• full_path.rs explicitly documents .\CON as rooting normally, but this exact-output test excludes it and the sibling negative test proves only that it does not start with \\.\. Returning .\CON unchanged would therefore pass both tests. Add an exact assertion for the collapsed-and-rooted result.

    // `.\CON` is excluded deliberately -- the `.` component collapses, so its
    // expected form is the bare name, which the loop below would have to
    // special-case. It is covered by the device-negative test instead.

• Files reviewed: 22/22 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

New filesystem-sensitive unit tests bypass the suite’s handle-allocation lock, creating races with raw-handle tests.

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

Review details

Suppressed comments (2)

crates/windows-namespace-request-sys/src/full_path/tests.rs:632

• probe_directory creates/removes directories, and its callers also run exists/is_file and filesystem-querying drive-relative resolutions, but none takes the test suite's handle-allocation read lock. Concurrent raw-handle tests hold the write side specifically to prevent stale handle values from being reused (src/handle/tests.rs:25-31,263-274), so these helpers can invalidate that isolation. Make ProbeDir retain a read guard through its Drop, or have every caller hold one for the complete probe lifetime.

fn probe_directory(tag: &str) -> ProbeDir {
    // The full shape an accepted `=X:` entry must have, not just its first

crates/windows-namespace-request-sys/src/full_path/tests.rs:378

• This other-drive resolution is now documented and tested as consulting the filesystem, but the test does not participate in handle_allocation(). It can therefore allocate a transient handle while a write-locked raw-handle test assumes no reuse is possible (src/handle/tests.rs:25-31,263-274). Acquire the read guard for the duration of this test.

fn a_drive_relative_path_is_rooted_at_that_drive_and_not_the_process_directory() {
    // The third rooting form. A drive-relative path is rooted at *that drive's*

• Files reviewed: 22/22 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The UNC assertion is case-sensitive for a case-insensitive path, and one newly documented device-path boundary remains untested.

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

Review details

• Files reviewed: 22/22 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Some documentation still overstates what observation proves, one documented NUL boundary lacks coverage, and the enlarged test module needs splitting.

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

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

crates/windows-namespace-request-sys/DESIGN-RATIONALE.md:37

• Changing the current directory and observing the same result proves output invariance, not that the implementation “reads no process state at all.” Keep this rationale at the observable boundary, consistently with D-18’s treatment of the current-drive entry.
  crates/windows-namespace-request-sys/src/full_path.rs:39
• The examples here establish that the output is invariant under the current directory; they do not establish the internal claim that the call reads no process state. That repeats the mechanism-overreach D-18 warns about. State the observable lexical semantics instead unless there is implementation-level evidence for the read claim.

• Files reviewed: 22/22 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The expanded full-path test module needs splitting at its clear drive-entry fracture point under the repository’s large-module rule.

Review details

Suppressed comments (1)

crates/windows-namespace-request-sys/src/full_path/tests.rs:707

• This addition grows src/full_path/tests.rs to 60.5 KiB and leaves two clear responsibilities in one large test module: device/rooting behavior and the process-environment/drive-entry fixtures below. The repository's source-module-size rule says large test modules should split readily at such fracture points; please extract the drive-entry tests and helpers into a focused child module (for example, src/full_path/tests/drive_entries.rs) before this file reaches the 64 KiB XL threshold.

/// A directory that exists, is in canonical `X:\...` form, and is neither a
/// drive root nor the process current directory.
///

• Files reviewed: 22/22 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The enlarged test module needs splitting, and two checklist archive-format violations remain.

Review details

Suppressed comments (3)

crates/windows-platform-probes/COMPLETED-CHECKLIST.md:97

• This moved-group heading uses a timestamp, while archive groups use a date-only heading and reserve the precise timestamp for the anchored item heading below. Keeping the date-group format consistent prevents one completed item from introducing a separate grouping key.

## Moved 2026-09-09 22:54:01 -04:00 -- M2.6: what `GetFullPathNameW` does, and whether it stays

crates/windows-namespace-request-sys/src/full_path/tests.rs:908

• This material addition leaves tests.rs at 63,791 bytes, just below the 64 KiB threshold, and the file now has clear test-specific fracture points (rooting/device behavior versus the =X: fixture and drive-entry cases). Split those responsibilities into child test modules now; keeping the whole 1,436-line test module together makes the next small change cross the XL boundary and makes these independent fixtures harder to maintain.

mod probe_drives {

CHECKLIST-thread-ambient.md:424

• This edited, checked item is still a multi-line historical body in the active checklist. Archive the completed M26.5 text and replace it here with the required one-line stub and stable item anchor, so the active checklist remains an action queue rather than retaining completed detail.

- [x] **M26.5** -- The `GetFullPathNameW` entry. Does not verify its result: it collapses `.`/`..`
  lexically and roots most paths that are not fully qualified against process state -- the current
  directory, or for a drive-relative path naming another drive the entry recorded for that drive,
  while on the current drive that entry makes no difference -- and never expands a
  drive letter, so it

• Files reviewed: 22/22 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

A device-mapping assertion remains too weak, and several documentation references or contract restatements are stale.

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

Review details

Suppressed comments (1)

crates/windows-namespace-request-sys/COMPLETED-CHECKLIST.md:44

• The candidate table and its enforcing test were extracted to src/full_path/tests/drive_entry.rs; this link now opens the facade rather than the inventory described here.

  live in one table in [tests.rs](src/full_path/tests.rs) with a test enforcing

• Files reviewed: 23/23 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The documentation, tests, planning records, and probe output are consistent, and the affected CI checks pass.

Review details

• Files reviewed: 23/23 changed files
• Comments generated: 0 new
• Review effort level: Balanced