Keep slow daemon negotiation on v2 and expose legacy fallback

[coneilen]

Summary

Keep a slow Windows daemon hello on protocol v2 instead of silently downgrading after 1.5 seconds. Implement the repository-owner decision on #867: allow about 5 seconds, require an explicit legacy/refused reply for fallback, and expose the fallback reason and elapsed time.

Refs #867.

Changes

• Wait up to 5 seconds for v2 negotiation. A no-reply timeout closes that attempt, reports the timeout, logs its elapsed time, and retries v2 with the existing bounded reconnect backoff; it never selects v1.
• Classify the actual top-level JSON reply. Known v1 event shapes, an explicit v1 selection, and an unsupportedVersion refusal retain legacy compatibility. Invalid, unrelated, nested, or unsupported selections become visible protocol errors rather than a downgrade.
• Record each v2 -> v1 decision, its specific reason, and measured elapsed milliseconds in graphcode-windows.log through the existing diagnostics writer. Keep the same notice on the existing connection status surface after v1 connects; a later successful v2 hello clears it.
• Add seven inline contracts to the already registered DaemonClient.zig test root, including bounded real named-pipe slow-v2, legacy-command, no-reply, and log checks. Keep the subscription drain's original 1.5-second budget, framing limits, request correlation, and connection-generation behavior.
• Only graphcode-windows/src/DaemonClient.zig changes. No daemon changes, late-reply draining, v1 removal, shared schema changes, desktop tests, or parity-ledger promotion.

Test plan

The commands below ran from graphcode-windows with bootstrap's pinned Zig 0.15.2 and owned provider checkout. TEMP/TMP, support state, and caches were isolated to this lane. Raw logs remain preserved separately as red-initial.log, green-initial.log, green-final.log, and the regression/build logs in the session artifacts.

RED: & $env:GRAPHCODE_ZIG0152 test src\DaemonClient.zig -target x86_64-windows-msvc -lc -ladvapi32 "-I$env:GRAPHCODE_WINGHOSTTY_ROOT\include" --cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-local --global-cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-global --test-filter negotiation -> 7 executed: 1 passed, 6 behavioral failures in 14.800 s; slow v2 expected .negotiating, found .unavailable; no reply expected .v2, found .v1; fallback decision/reason diagnostics absent.
GREEN: & $env:GRAPHCODE_ZIG0152 test src\DaemonClient.zig -target x86_64-windows-msvc -lc -ladvapi32 "-I$env:GRAPHCODE_WINGHOSTTY_ROOT\include" --cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-local --global-cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-global --test-filter negotiation -> 8/8 passed, 0 skipped, 13.785 s; real 2500 ms v2 hello served a correlated request; genuine v1 command round trip and persistent reason/elapsed diagnostic/log passed; bounded no-reply remained v2.
REGRESSION: & $env:GRAPHCODE_ZIG0152 test src\DaemonClient.zig -target x86_64-windows-msvc -lc -ladvapi32 "-I$env:GRAPHCODE_WINGHOSTTY_ROOT\include" --cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-local --global-cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-global -> 215/215 passed, 0 skipped, 149.226 s, including connection generations, coalesced frames, request IDs/correlation, outbound accounting, subscription drain, and imported adjacent contracts.

Additional executed commands, using the same target/link/include/cache arguments as above:

Command	Actual result
zig test src\FrameBuffer.zig	169/169 passed, 0 skipped, 99.787 s (includes imported wire/model contracts).
zig test src\DaemonRoundTripTests.zig --test-filter "daemon round-trip support helper"	1/1 passed, 7.660 s; support-name helper only, not the production-daemon persistence phases.
zig build -Demit-win32-host=true in the owned pinned Winghostty checkout	Passed; produced the host library used by the shell build.
zig build -j1 -Doptimize=ReleaseSafe "-Dwinghostty-dir=$env:GRAPHCODE_WINGHOSTTY_ROOT" --cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles\.build\lane5\zig-build-local --global-cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles\.build\lane5\zig-build-global	Passed in 69.275 s; produced zig-out\bin\graphcode-windows.exe.
One fresh-eyes task / code-review, default model/reasoning/context settings	No high-confidence findings in the reviewed diff.
git diff --check	Passed.

Preserved intermediate failures: the first GREEN attempt ran 8 cases, with 7 passing and an owned test-fixture TestPipeAcceptTimedOut on reconnect (16.667 s). Recreating the owned pipe server before redial corrected the fixture's listen ordering; no assertions were removed or weakened. The initial concurrent ReleaseSafe build failed with LLVM out-of-memory after 350.272 s; the serial -j1 retry above passed. The first validation prerequisite check reported the pinned tool environment absent; repository bootstrap then installed the exact pins into this worktree.

The tests establish actual named-pipe/component negotiation, legacy command framing, timeout behavior, and the diagnostics file/status text. They do not establish native GUI presentation, desktop/keyboard/UIA behavior, production-daemon persistence, or macOS parity. make test and make check are macOS/Xcode commands and were not run on this Windows host; the full Windows shell/live gate was not run locally. Hosted PR checks are reported separately against the published head, not substituted with a manually dispatched run.

Checklist

• I have read the Contributing Guidelines
• I have signed off my commits (git commit -s) per the DCO
• Tests pass locally (make test)
• Code follows the existing style (make check)
• I added the test/contract before the implementation and observed the intended RED failure

CI follow-up: separate pre-existing completed-read cancellation defect

Refs #867. This follow-up is on operator-rebased parent 3b4effb62e700cf7978740532ef09372b2e21596, not the original local head. The original implementation evidence and original fresh-eyes review above are historical evidence for aae472e3148fde73a8c3732dc1a5dfd43ab9927b; neither that review nor its 21-success/4-skipped hosted check snapshot is claimed for this repair or a later head.

The slow-v2 test failed on the actual rebased PR head in run 39170170880, Windows shell shard 1: 15/215 DaemonClient.test.negotiation slow v2 hello after 2500 ms stays v2 and serves a correlated request...FAIL (TestNegotiationStateTimedOut). The full raw failing job is retained as ci-testfix-red-shard1.log.

This exposed a separate pre-existing production transport bug, not simply an insufficient test budget. The pending-read timeout path called CancelIoEx, joined GetOverlappedResult, then unconditionally discarded any bytes. A read that completed just before cancellation could therefore lose its frame header. A budget-only attempt passed 8/8 focused cases (13.943 s), but failed the full root at 214/215 (73.551 s). A subsequent classified failure showed a reply at 2505 ms inside a 10000 ms test budget, followed by PayloadTooLarge and a reconnect. Those failures and temporary diagnostic runs are preserved separately, not replaced by the passing logs.

The operator explicitly approved the transport scope extension within DaemonClient.zig. The correction cancels and joins non-signaled pending reads before releasing their event, OVERLAPPED, or destination buffer. Successful bytes and aborted completions with bytes reach the existing append path exactly once. Only an aborted completion with zero bytes is no data; terminal read failures and wait failures remain visible. A successful zero-byte read still follows the existing EOF path. No framing limit, fallback policy, correlation check, daemon code, or production timeout changes.

The new deterministic real-pipe regression starts a header-only writer only after the client's read is pending. Its injected wait waits for the actual read-completion event, then reports a timeout once, forcing the exact completion-before-cancel boundary through the production read routine. Before the fix, the retained-length assertion failed (expected 97, found null). After the fix, the header remains pending until its JSON body arrives; the exact 97-byte response is delivered once and completes exactly one correlated request. Separate controls cover aborted zero-byte reads followed by a valid hello, terminal read errors, and cancellation/join before surfacing a wait failure.

The original real 2500 ms slow-v2 worker round trip, request-ID matching, final v2/generation assertions, genuine-v1 round trip, and fallback diagnostics assertions remain. Only the slow test receives a 10000 ms deadline and 5000 ms completion/I/O margins; it first asserts that production assigned exactly 5000 ms. That test-only margin is not the root-cause fix and is not a claim of production timeout equivalence. The existing no-reply test still exercises the real default five-second budget.

Commands below use the same pinned target/link/include/cache arguments as the original test commands above, in this owned worktree:

RED: & $env:GRAPHCODE_ZIG0152 test src\DaemonClient.zig -target x86_64-windows-msvc -lc -ladvapi32 "-I$env:GRAPHCODE_WINGHOSTTY_ROOT\include" --cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-local --global-cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-global --test-filter "negotiation cancel boundary" -> 3 executed before the transport correction: 1 passed / 2 behavioral failures, 3.378 s; completed header expected retained length 97, found null; terminal read error silently treated as no data; aborted-zero control passed.
GREEN: & $env:GRAPHCODE_ZIG0152 test src\DaemonClient.zig -target x86_64-windows-msvc -lc -ladvapi32 "-I$env:GRAPHCODE_WINGHOSTTY_ROOT\include" --cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-local --global-cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-global --test-filter negotiation -> final source 12/12 passed, zero skipped, 11.502 s, including four cancellation-boundary cases, slow v2 with correlated request, genuine v1 fallback, diagnostic logging, and bounded no reply.
REGRESSION: & $env:GRAPHCODE_ZIG0152 test src\DaemonClient.zig -target x86_64-windows-msvc -lc -ladvapi32 "-I$env:GRAPHCODE_WINGHOSTTY_ROOT\include" --cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-local --global-cache-dir D:\depot\copilot-worktrees\GraphCode\coneilen-congenial-goggles.build\lane5\zig-global -> final FULL registered root 219/219 passed, zero skipped, 73.440 s.

Final raw evidence: ci-transport-red.log, ci-transport-final-green.log, and ci-transport-final-regression.log. No original assertions or test cases were removed. Production five-second negotiation behavior and explicit-only fallback remain unchanged. This is pipe/component evidence, not native GUI/desktop, production-daemon persistence, or macOS parity. The operator owns readiness, new-head hosted CI, and merge; this repair does not claim any new hosted check result.

Repair commit: c15118c2669c0396f82b0627012d7878b039389a, parent 3b4effb62e700cf7978740532ef09372b2e21596. Only graphcode-windows/src/DaemonClient.zig changed in the repair; normal push only, with DCO sign-off and the required App coauthor/session trailers. The earlier local reviewed head is preserved at refs/lane5-preserved/original-reviewed-head.

One fresh-eyes focused task / code-review used default model/reasoning/context settings and covered only this transport correction, seam, regressions, and test timing delta against 3b4effb62e700cf7978740532ef09372b2e21596, not the entire original feature. Reviewer agent: bcfcbbf6-569f-4201-9c3e-b59b0cceb168. Final reviewed source blob: 08f279bf5ecbec4e82c253c46ae6638d5de44362; no source changes occurred after review launch, and the committed file has that exact blob. Thus the reviewed repair range is 3b4effb62e700cf7978740532ef09372b2e21596..c15118c2669c0396f82b0627012d7878b039389a.

Full unmodified focused-review verdict:

No significant issues found in the reviewed transport repair delta, wait-injection seam, or regression/test timing changes.

Integration follow-up: failure-only multiproject classification evidence

Refs #867. Instrumentation commit e5a0bc0dd47e6db4396e8ab010218e074917a20b is a normal signed commit above the exact operator-held parent e157b4c48230522a4e6994e431d5a580706dda93. Only Tools/windows/uia-live-gate.ps1 changes: the receipt reader's observational clocks/counters, a failure-only JSON writer, and the multiproject phase's failure/finally recording. Geometry/Open consumers, outgoing-hover literals, workflow upload rules, typed receipt validation, and the client are untouched. The original review/runtime evidence above remains bound to its named original heads; it is not new-head evidence.

CI RED: run 39177391071, original integration job 117822919322 on exact e157b4c48230522a4e6994e431d5a580706dda93 failed at Read-MultiProjectPeer: MULTIPROJECT_PEER: no readable owner/receipt snapshot. No JSON read-retry was logged. Both owned child jobs then exited, became empty, and drained their redirected streams.

Read-only classification: Stub-Daemon.ps1 is a v2 peer that immediately sends a flushed selectedVersion:2 hello, not a v1-only fixture. The five-second client limit is a maximum, not a mandatory latency. The peer file is written only after the first correlated request response and initial owner publications, not at process launch/hello. The downloaded actual uia-live-gate-diagnostics artifact contained four JSON files, but no multiproject peer file or child stream logs. That artifact absence does not prove the file was never written or written late. The original stalled stage and original CI cause remain unproven. The operator's successful rerun is a signal, not causal clearance.

On a failure only, after the existing owned teardown and joined stream readers, the gate now writes logs/multiproject-failure.json, which the existing logs/*.json upload already includes. It records the original exception type/message/ID; actual phase/read UTC and elapsed times; calls, attempts, missing-file and parse-failure counts; only actually observed peer counters (otherwise null); actual file existence/length/last-write time; held child identity and pre-teardown exit/status observations; the existing post-teardown proof; and at most 8192 characters of each existing captured stream. Exit codes are null when no exit was observed before teardown, not invented successes. Diagnostic-write failures add exception Data and a loud warning without replacing the original error. No new global process census or desktop probe is added.

The reader still performs exactly 40 attempts with 50 ms sleeps; no timeout was widened. Success results and all malformed/missing/foreign-owner/typed receipt rejection rules are unchanged. Production remains v2-first with the original five-second hello budget and the previously reviewed completed-read cancellation repair.

Actual local sequence, with failures preserved separately:

• The first private extraction attempt failed on an empty automatic PSScriptRoot before launching any child (110 ms). Its failure JSON has zero reader calls and no children; this is harness evidence, not a native integration result.
• The first actual native attempt ran from 2026-10-19T01:42:25.4821840Z to 01:42:27.7293050Z. The real peer receipt succeeded in 7 attempts / 403 ms under the unchanged two-second reader, with 2 requests, 2 responses, and 2 publications. The phase then failed because the private loader had omitted the original nested ConvertTo-SketchCanonicalJson dependency. Both owned jobs exited, became empty, and drained their streams (53 ms and 36 ms). The actual new failure diagnostic is retained at D:\g5-870-eaa6520b\logs\multiproject-failure.json. This is not a full integration pass or proof of the old CI cause.
• The operator explicitly allowed one corrected-harness completion attempt. The loader then bound the original source's complete 81-function dependency closure and original script-root path; no UI/control/receipt mocks or success defaults were substituted.
• That corrected actual native phase passed from 2026-10-19T01:44:18.5513069Z to 01:44:44.4802139Z (25.927 s): 12 requests / 12 responses, 2 owner graphs, 7 publications, 8 navigation observations, and 2 no-mutation cases. Exactly one complete actual typed receipt was emitted (14380 UTF-8 bytes). Both owned jobs again exited, became empty, and drained their streams, with no teardown failures (69 ms and 12 ms). The outer runner exited 0 with zero stderr bytes. No further local attempts were made.

RED: gh api repos/scgopi/GraphCode/actions/jobs/117822919322/logs --allow-escape-sequences -> exact e157 original integration failure: MULTIPROJECT_PEER: no readable owner/receipt snapshot; original failing log retained, causal stage unproven.
GREEN: pwsh -NoProfile -File Test-PeerFailureDiagnostics.ps1 -> 6/6 source-extracted private contracts passed: unchanged absent-file 40x50 ms rejection, successful snapshot return, missing-peer rejection, malformed JSON bounded retry, bounded streams/null unobserved fields, and loud diagnostic-write failure preserving the original exception; initial private warning-capture mistake was corrected in the harness only and retained in its separate failing log.
REGRESSION: pwsh -NoProfile -File Invoke-TargetedMultiProject.ps1 -> operator-approved corrected actual source-extracted multiproject native phase passed in 25.927 s, with exactly one typed receipt, 12/12 correlated requests/responses, all existing native assertions, and both owned jobs empty/exited/streams joined.

The private runner names above refer to retained session-artifact scripts, not newly committed test entry points. Existing source functions/assertions were executed, not reimplemented. Supporting records are peer-diagnostics-contracts-final.log, peer-targeted-dependency-audit.log, peer-targeted-local-native.log, peer-targeted-local-corrected-summary.json, and peer-targeted-corrected.stdout.log. The exact local source blob was 0749ca7d9a31cbea833afd50fb5bb0ee56dd4117, identical to the committed gate file. git diff --check and PowerShell parsing passed. Owned pinned provider and exact-head shell builds passed after the documented dependency fetch restored a missing zmx package; the initial missing-package build failure is retained separately.

This commit adds observability, not a claimed root-cause fix or a new timing guarantee. The 403 ms observation is one local receipt observation, not a global performance claim. Native/component success does not establish production-daemon persistence or macOS parity. No general review restart was performed for this diagnostics-only change; the earlier original-feature and transport reviews remain historical, separately scoped evidence. New-head hosted CI, readiness, merge, and any follow-up tracking issue are operator-owned; no new-head hosted success is claimed here.