Stream large -z git output instead of buffering it against the cap

[Tyrie Vella (tyrielv)]

Summary

Follow-up to #2048. That PR bounded the previously-unbounded git stdout/stderr capture and, for the two commands whose full result is correctness-critical, added a large stdout cap plus fail-safe valves if the cap was ever exceeded. This PR removes that truncation exposure entirely by streaming those commands' output instead of buffering it.

• DiffCachedNameStatus (diff --cached --name-status -z) feeds every staged path into ModifiedPaths.
• StatusPorcelain (status --porcelain -z) drives the sparse dirty check.

Both use -z (NUL-delimited), so line-based streaming can't chunk them — git delivers the whole blob as a single "line."

Change

Add a NUL-delimited streaming mode to InvokeGitImpl: a new parseStdOutToken callback reads stdout synchronously and splits on \0, invoking the callback once per record as it arrives. stderr stays async (BeginErrorReadLine), so the synchronous stdout read is deadlock-safe ("one sync, one async"). Only one record is held in memory, so an arbitrarily large result streams with no buffering, truncation, or OOM. It keeps git's robust -z format (no path-unquoting) and is guarded to timeoutMs == -1 and mutual exclusion with parseStdOutLine.

DiffCachedNameStatus and StatusPorcelain now stream tokens; their callers drive small status/path state machines. This removes the interim fail-safe valves from #2048 (there's no longer a partial result to guard against) and the dead -z string parsers (GetPathsNotCoveredBySparseFolders, GetNextGitPath). Result.OutputTruncated/ErrorsTruncated remain for the still-buffered commands (stderr is still bounded), but the two converted commands can no longer trip OutputTruncated.

Tests

• ReadStdOutTokens: NUL splitting, empty input, embedded empty records, a trailing record without a NUL, and a record spanning the 8KB read buffer.
• DiffCachedNameStatus/StatusPorcelain stream records through the mock.
• Replaces the removed GetNextGitPath test; PathCoveredBySparseFolders tests unchanged.

Full unit suite: 889 passed, 0 failed (11 pre-existing native-hook skips).

[Nathan Baird (ShiningMassXAcc)]

The streaming rewrite and the deadlock reasoning look sound to me — one sync/one async read, stderr drained via BeginErrorReadLine, watchdog kill guarded under processLock/readCompleted. I have one robustness concern I'd like addressed before final approval.

After ReadStdOutTokens returns (stdout at EOF), the streaming branch calls the parameterless this.executingProcess.WaitForExit() (infinite). By that point readCompleted = true is already set in the finally and the watchdog is disposed — so this final wait has no safety net. On the timeout-kill path (killedByTimeout), you TryKillProcessTree(...) and then fall into this same unbounded WaitForExit(). If the tree-kill only partially succeeds (a surviving grandchild still holding the pipe, or a wedged git), the calling thread pins forever — which is exactly the maintenance/prefetch hang this PR family (cf. #2046) is trying to eliminate. Narrow window, but it reintroduces the failure mode.

Suggestions:

1. Bound the post-EOF wait — WaitForExit(grace) with a small finite grace; on expiry, TryKillProcessTree again and return a failure/timeout Result instead of trusting a partial exit code. Keeps "never hang indefinitely" true on both the stream and buffer paths.
2. Still call the parameterless WaitForExit() once on the success path (it's instant once the process is gone) so the async stderr readers are guaranteed flushed before you read Errors — don't lose that property by switching to the int overload alone.
3. Derive the grace from timeoutMs when one was set (reuse it, or a generous fixed ceiling of 30–60s when infinite) so callers that opted into a bound get a bound end-to-end.
4. Log a distinct event if the post-EOF wait expires (same spirit as Bound runtime credential fetch so prefetch can't hang on auth #2046's CredentialFetchTimedOut) so a field occurrence is measurable rather than an invisible hang.

Net: the logic is good; the only real gap is that the final wait is the one place with neither a timeout nor the watchdog covering it. A bounded wait + re-kill closes it.

[Tyrie Vella (tyrielv)]

Great catch — that final wait was the one spot with neither a timeout nor the watchdog covering it. Fixed in the latest push (10dfad58):

1. The post-EOF wait is now bounded. If git hasn't exited by the grace, it re-kills the process tree and returns a distinct, self-identifying failure Result rather than trusting a partial exit code or blocking — so "never hang indefinitely" holds on both the stream and buffer paths.
2. Once exit is confirmed it still calls the parameterless WaitForExit() so the async stderr readers are guaranteed flushed before Errors is read — kept that property as you noted.
3. The grace reuses the caller's timeout when one was set, otherwise a generous 60s ceiling, so an opted-in bound is honored end-to-end.
4. On expiry it returns a distinct error message. GitProcess has no tracer of its own, so rather than thread one through here I made the message greppable/measurable via the caller's existing error logging.

On the "reuse vs. reimplement" point: the bounded wait now goes through a shared WaitForExitWithCancellation helper (single WaitForExit(timeoutMs) when there's no token; a short poll loop when there is, so mount shutdown is observed promptly). It's written so the credential-bounding work in #2082 can share the same idiom instead of each path rolling its own. FYI this streaming path is also off by default (gvfs.stream-git-status-output), so it only runs once the rollout flag enables it.

[Nathan Baird (ShiningMassXAcc)]

Approving — both outstanding asks are addressed, and I verified them in the code at 10dfad58 rather than just on the replies.

My ask (bounded post-EOF wait) — closed. All four points landed:

1. The post-EOF wait is bounded: WaitForExitWithCancellation(postReadGraceMs, out _), and on expiry it re-kills the process tree and returns a failure Result instead of trusting a partial exit code. "Never hang indefinitely" now holds on the stream path as well as the buffer path.
2. The parameterless WaitForExit() is still called once exit is confirmed, so the async stderr readers are guaranteed flushed before Errors is read. That property is preserved.
3. postReadGraceMs = timeoutMs != Timeout.Infinite ? timeoutMs : DefaultPostReadGraceMs (60s) — an opted-in bound is honored end-to-end.
4. The distinct message is genuinely measurable: FileSystemCallbacks puts result.Errors into EventMetadata on the failure path, so "GitProcess streaming read: git did not exit within ...ms after stdout closed" reaches the tracer. Given GitProcess has no tracer of its own, threading one through just for this isn't worth it — agreed with the call.

The WaitForExitWithCancellation helper reads well as a shared idiom for the credential-bounding work to pick up.

Keith's ask (don't mutate ModifiedPaths before git diff succeeds) — closed. The streaming callback now only accumulates into pendingRecords; handleRecord runs over them solely under if (result.ExitCodeIsSuccess). A mid-stream failure can't leave a partial mutation behind, and the unpaired-trailing-status check is a nice addition — failing on a truncated-at-the-boundary stream rather than silently dropping the last entry is the right direction. handleRecord being shared by both the streaming and buffered paths means the two can't drift per record.

SparseVerb is fail-safe too: the streaming path mutates dirtyPathsNotInSparseSet as it goes, but a failure returns false and the caller reports statusResult.Errors and aborts, so the partial set is never acted on. The Clear() at the top of the delegate covers a ShowStatusWhileRunning retry.

Verified locally at 10dfad58 (build clean, 0 warnings): full unit suite 1010 tests, 1005 passed. Every test touching this change passes — the six ReadStdOutTokens_* cases, DiffCachedNameStatus_StreamsRecordsAsTokens, StatusPorcelain_StreamsRecordsAsTokens, both *_BufferedFallbackReturnsOutput fallbacks, and the PathCoveredBySparseFolders_* set. The 5 failures are PostIndexChangedHookTests (SkipsNotification_*), which fail identically on origin/vnext without this PR — pre-existing and unrelated.

Two non-blocking notes for whenever this next gets touched:

• The PR description says the -z string parsers (GetPathsNotCoveredBySparseFolders, GetNextGitPath) were removed and the GetNextGitPath test replaced. They're still present, correctly — the buffered fallback behind gvfs.stream-git-status-output needs them, and GetNextGitPathGetsPaths still passes. Just worth refreshing the description so it matches the flagged shape before merge.
• WaitForExitWithCancellation's cancellationToken / cancellationRequested are unexercised at every current call site. That's deliberate groundwork, so fine — but it's dead until something passes a token, and it'd be good for the credential work to actually consume it rather than leave it indefinitely unused.

Defaulting the flag to false and gating the runtime entry point is exactly the right rollout shape. Signing off.