[Tyrie Vella (tyrielv)]

Problem and Context

AGENTS.md mentions the functional-test dev runner, but several details can mislead agents. The documented positional command fails because PowerShell binds the test filter to -Arch. The payload project only copies existing outputs, so unchanged instructions can cause tests to run stale binaries.

The guidance also overstates isolation between concurrent test processes. These gaps can lead agents to request a full GVFS installation, trust stale results, or run conflicting fixtures.

Changes

• Explain that GVFS.Payload assembles existing outputs and does not build its source projects.
• Publish the changed GVFS project before assembling the example payload.
• Use named -Configuration and -Arch arguments in functional-test examples.
• Explain how dev mode resolves gvfs.exe and GVFS.Service.exe from local build output.
• Clarify that dev mode requires no GVFS installation.
• Warn that some fixtures still share machine paths and drive mappings.
• Tell agents to publish each changed payload project to prevent stale test binaries.
• Add guidance that prevents Microsoft-internal references in this public repository.

[Nathan Baird (ShiningMassXAcc)]

Reviewed against origin/master @ f4728ba1. I verified the claims in this diff against the source rather than reading for plausibility, since AGENTS.md is auto-loaded by coding agents and a wrong-but-plausible line here misdirects every future session.

First, confirming the premise of this PR is correct: GVFS.Payload.csproj really does have no ProjectReference items. The only match in the file sits inside an <ItemDefinitionGroup> (lines 7-10) that sets metadata defaults for items that are never added — a genuinely easy misread, and the text this PR removes ("cascades to its dependencies … via ProjectReference") was flatly wrong. Build.bat:172-186 corroborates, publishing the four managed projects as separate invocations precisely because nothing cascades.

Given that, one gap I'd like to raise — it's the same failure mode this PR is fixing, one step further down.

The Path B recipe can still assemble an incomplete payload, silently

layout.bat copies from four managed publish directories:

layout.bat:86-89
xcopy /Y /S %BUILD_OUT%\GVFS\%MANAGED_OUT_FRAGMENT%\* %OUTPUT%
xcopy /Y /S %BUILD_OUT%\GVFS.Hooks\%MANAGED_OUT_FRAGMENT%\* %OUTPUT%
xcopy /Y /S %BUILD_OUT%\GVFS.Mount\%MANAGED_OUT_FRAGMENT%\* %OUTPUT%
xcopy /Y /S %BUILD_OUT%\GVFS.Service\%MANAGED_OUT_FRAGMENT%\* %OUTPUT%

The example publishes GVFS.FunctionalTests, GVFS, then GVFS.Payload. The new callout frames GVFS.Mount / GVFS.Hooks / GVFS.Service as needed only "when you change those projects" — which holds if a full Build.bat already ran, but the prerequisite blockquote just above explicitly offers an alternative that doesn't satisfy it:

build the native projects via VS MSBuild first

On that path the documented recipe assembles a payload missing GVFS.Mount.exe, GVFS.Hooks.exe and GVFS.Service.exe.

The part that makes this worth fixing rather than just noting is that nothing reports the failure — at three independent levels:

1. xcopy against a missing source directory doesn't fail. I confirmed this directly:

xcopy /Y /S <missing>\* <dest>\   ->  "File not found - *"
                                      "0 File(s) copied"
                                      LASTEXITCODE=0

2. layout.bat has no error handling on the copy block at all. There is no IF ERRORLEVEL check and no || GOTO ERROR after lines 86-93 — in fact the :ERROR label at lines 123-125 is unreachable: there is no GOTO ERROR anywhere in the file. Control flows straight from the xcopy block through cleanup to GOTO EOF at line 110.

3. The last statements executed before that are DEL /Q ... 2>nul (lines 103-109), which return 0 regardless.

So the <Exec> in CreatePayload sees a clean success and the build is green.

The script's own guard doesn't catch it either — RunFunctionalTests-Dev.ps1:72-74 only tests for GVFS.exe. So the run proceeds and dies later inside the test process, at GVFSServiceProcess.cs:279:

File.Exists(Properties.Settings.Default.PathToGVFSService).ShouldBeTrue("Failed to locate GVFS.Service.exe");

That's a long way from the cause, and "Failed to locate GVFS.Service.exe" reads like a missing installation — which is exactly the wrong conclusion to hand an agent, and the one this PR is otherwise working to prevent.

Suggestion: either list all four managed projects in the Path B block, or change the callout from "when you change those projects" to something like "at least once in a new enlistment, and again whenever you change them" — and extend the prerequisite blockquote so it notes the managed payload projects must also have been published once, i.e. the VS-MSBuild-native-only option isn't sufficient for Path B.

Everything else I checked in this diff holds up — the -Configuration/-Arch binding warning is correct (I reproduced the ValidateSet failure on both PowerShell 5.1 and 7.6.6), the dev-mode resolution description is accurate including "no installation required", and the revised concurrency warning is better supported than the claim it replaces. Happy to write those up separately if useful. I have two smaller notes as well — one on RunFunctionalTests-Dev.ps1's own .EXAMPLE block, one on the new hygiene section — which I'll add if you'd like them here rather than in a follow-up.

[Tyrie Vella (tyrielv)]

Thanks for the catch — I updated the guidance to make the Path B prerequisite explicit: the native C++ projects and all managed payload projects must already be built/published before the functional test inner loop, otherwise the payload can be incomplete and silently stale. I also clarified that GVFS.Payload.csproj does not populate output via ProjectReference, so the doc now calls out the publish step directly.

[Nathan Baird (ShiningMassXAcc)]

Verified fb3ea437 against origin/master. This fully addresses the gap I raised, and it took all three parts of the suggestion rather than just the quickest one:

• The Path B example now publishes all four managed payload projects (GVFS, GVFS.Hooks, GVFS.Mount, GVFS.Service).
• The callout changed from "when you change those projects" to "at least once in a new enlistment, and publish a project again when you change it" — which is the part that actually protects a fresh enlistment.
• The prerequisite blockquote now covers managed projects too, and the VS-MSBuild-only alternative explicitly says to publish them as well. That was the path that silently produced a broken payload before.

Checks I ran against fb3ea437:

• The documented set exactly matches what the payload consumes. layout.bat:86-89 copies from GVFS, GVFS.Hooks, GVFS.Mount, GVFS.Service — the same four now listed, no more and no fewer.
• All six documented project paths resolve on the branch (GVFS.FunctionalTests, GVFS, GVFS.Hooks, GVFS.Mount, GVFS.Service, GVFS.Payload), so the recipe is runnable as written.
• All six PowerShell blocks in the file parse clean via [Parser]::ParseInput, including the backtick line-continuations in the expanded example.
• The #native-c-projects-need-msbuild-not-dotnet-build anchor still resolves after the blockquote rewrite — the target heading is unchanged.
• Still documentation-only: one file, +51/-14.

Combined with what I confirmed on the first pass — the GVFS.Payload / ProjectReference correction, the -Configuration/-Arch binding warning (reproduced on PowerShell 5.1 and 7.6.6), the dev-mode resolution description, and the better-supported concurrency warning — this is a clear improvement to a file that agents load automatically, where a wrong line is expensive.

Approving.

One optional follow-up, non-blocking and outside this file's scope: scripts/RunFunctionalTests-Dev.ps1:31 still advertises the positional form in its .EXAMPLE block:

.\RunFunctionalTests-Dev.ps1 Debug --test=GVFS.FunctionalTests.Tests.EnlistmentPerFixture.WorktreeTests

So Get-Help on the script now contradicts AGENTS.md. Worth a one-line fix whenever that script is next touched, so the two agree.