Addresses the unit-windows CI failures from PR #459. Most are test-infra
fixes; two are real Windows portability gaps in production code.

Test infra:
- testutil_test.go: new setHome(t, dir) helper that sets HOME plus, on
  Windows, USERPROFILE/HOMEDRIVE/HOMEPATH so os.UserHomeDir resolves to
  the test tempdir instead of C:\\Users\\runneradmin. gitT() mirrors the
  same env scrubbing for git subprocesses on Windows. Replaced 77
  occurrences of t.Setenv("HOME", ...) across main/config/github/review_
  file/plans/share/daemon/auth tests.
- daemon_test.go, auth_test.go: skip Unix file-mode bit assertions on
  Windows where the perm() always reads as 0666/0777.
- daemon_e2e_test.go: append .exe to the built crit binary name on
  Windows so exec can find it on PATH.

Production:
- comment_cli.go (addCommentToCritJSONScoped, addFileCommentToCritJSONScoped):
  normalize cleaned filepath via filepath.ToSlash before storing in the
  review JSON. The review file is a cross-platform artefact (synced via
  crit-web, GitHub, share) so paths must be POSIX-style on every host.
- session.go (NewSessionFromFiles): same normalization for the relPath
  used as the file key.
- git.go (WalkFiles): same normalization — output flows into the picker
  UI and ignore-pattern matching, both of which assume forward slashes.
- atomic_write.go + atomic_{unix,windows}.go: extract the rename step
  into a renameAtomic helper. Unix keeps os.Rename; Windows wraps it in
  a brief retry loop on ERROR_ACCESS_DENIED / ERROR_SHARING_VIOLATION /
  ERROR_LOCK_VIOLATION (10 attempts, exponential backoff capped at
  50ms). Concurrent writers race against short-lived readers there, and
  os.Rename otherwise fails when a reader holds the destination handle
  without FILE_SHARE_DELETE.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Addresses the 10 remaining unit-windows failures from the previous CI run.

A. Concurrent save read-side sharing violation
   - atomic_unix.go: add readFileShared (= os.ReadFile on POSIX).
   - atomic_windows.go: add readFileShared with the same retry recipe
     as renameAtomic (10 attempts, 1→50ms backoff, retries on
     ERROR_SHARING_VIOLATION/ERROR_LOCK_VIOLATION). Inverse of the
     writer-side gap fixed last round.
   - review_file.go (loadCritJSON, findReviewFileByCommentID,
     findReviewFileByBranch), daemon.go (readSessionFile, orphan scan,
     listSessionsForCWD, listSessionsForRepoRoot), watch.go, share.go,
     github.go, main.go, session_write.go: route review-file and
     session-file reads through readFileShared.
   - concurrent_save_test.go: same in the worker loop so the test itself
     stops racing.

B. Forward-slash incomplete
   - config.go (matchPattern): filepath.ToSlash the path before pattern
     matching so "generated/" matches "generated\\types.go" on Windows.
   - comment_cli.go (addCommentToCritJSONScoped,
     addFileCommentToCritJSONScoped): also reject POSIX-absolute paths
     (strings.HasPrefix "/") in addition to filepath.IsAbs, since
     filepath.IsAbs("/foo") is false on Windows — security-relevant.

C. TestDaemonLifecycle: drop DETACHED_PROCESS
   - daemon_windows.go: keep only CREATE_NEW_PROCESS_GROUP. Combining
     DETACHED_PROCESS with handle inheritance (ExtraFiles for the
     readiness pipe) is fragile; the child often fails to inherit FD 3,
     breaking the port handshake. CREATE_NEW_PROCESS_GROUP alone is
     enough Ctrl+C isolation.

D. Embedded-content CRLF mismatch
   - .gitattributes: force LF on every checkout including Windows so
     embed.FS bytes match the freshly-installed file. Fixes the three
     TestCheckInstalledIntegrations / TestDetectInstalledIntegrations
     failures.

E. listSessionsForRepoRoot subdirectory matching
   - daemon.go (listSessionsForRepoRoot, listSessionsForCWD): normalize
     stored CWD and probe path with filepath.ToSlash before comparison.
     The "/" separator literal in the prefix was breaking matches on
     Windows even when both paths referred to the same directory.

F. Cross-drive filepath.Rel garbage
   - session.go (NewSessionFromFiles): if filepath.Rel produces a
     ../../../ traversal (cross-volume / short-name boundary on
     Windows), fall back to the absolute path so downstream consumers
     get a stable string instead of useless traversal.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

1. comment_cli.go absolute-path validator
   filepath.Clean("/etc/passwd") on Windows returns "\\etc\\passwd",
   defeating the previous strings.HasPrefix(cleaned, "/") check. Extract
   the validation into isAbsoluteOrTraversal() and run it on the RAW
   input — checking both POSIX ("/foo") and Windows ("\\foo") prefixes
   plus filepath.IsAbs — before any Clean. Applied at all three sites
   (addCommentToCritJSONScoped, addFileCommentToCritJSONScoped, bulk
   entry handler). Bulk handler also gains the cross-platform ToSlash
   normalization the other two already had.

2. daemon_e2e_test.go (TestDaemonLifecycle)
   The test built a custom cmd.Env that filtered HOME but did not
   filter/set USERPROFILE. The spawned daemon therefore inherited the
   GH Actions runner's real USERPROFILE and wrote its session file into
   C:\\Users\\runneradmin instead of the test's tempdir. Mirror the
   testutil_test.go gitT() pattern: drop USERPROFILE/HOMEDRIVE/HOMEPATH
   from the inherited env on Windows and set USERPROFILE to homeDir
   alongside HOME.

3. TestCreateSession_FilesMode_LoadsShareFromReviewPath skipped on Windows
   GH Actions exposes the test temp dir under an 8.3 short name
   (RUNNER~1) while git rev-parse returns the long form. filepath.Rel
   produces a useless ../../../... traversal, breaking shareScope
   matching. Real Windows users work in long-form paths so this only
   surfaces on the runner. Skip with a TODO referencing the path-
   normalization gap; revisit once resolveGitContext and
   expandAndDedupPaths share a single path-canonicalization step.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cmd.Wait hangs after proc.Kill on Windows for the detached daemon child
spawned via ExtraFiles. Skip the E2E with a TODO; runtime daemon code is
still covered by daemon_test.go unit tests on Windows.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Adds TestClientExitsOnFinish in client_finish_test.go. Models the agent-
integration contract that every supported AI tool depends on: agent runs
crit, the process blocks until Finish, then exits with code 0 and the
review summary on stdout (the JSON body of /api/review-cycle).

Workflow:
  1. Build the binary (handles .exe on Windows).
  2. Init a temp git repo with a tracked file change so git mode has
     something to review.
  3. Spawn `crit --no-open --port 0` with HOME (and USERPROFILE on
     Windows) pinned to a temp dir so the daemon writes its session file
     under the test sandbox.
  4. Poll the session file for the assigned port; poll /api/session for
     readiness.
  5. POST /api/finish on a 250ms tick until the client exits. Re-firing
     handles the race where the client hasn't yet subscribed inside
     handleReviewCycle (the SSE finish event is only delivered to
     subscribers present at fire time).
  6. Bound the wait at 15s with a goroutine + select; a hang fails fast
     instead of hitting the 10-minute test timeout.
  7. Assert exit code 0 and that stdout parses as the review summary
     {status:"finished", review_file, approved:true}.

Runs on darwin/linux/windows. Never invokes proc.Kill on the client and
never depends on `crit stop` semantics, so it's immune to the Windows
detached-child kill issue that forces TestDaemonLifecycle to skip.

Verified locally on darwin: passes in ~1.4s. Cross-compiled clean for
windows/amd64.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

ExtraFiles is documented as unsupported on Windows (os/exec/exec.go:231),
so the FD 3 readiness pipe was silently dropped: the parent set
cmd.ExtraFiles=[writeEnd] and _CRIT_READY_FD=3, but the child's inherited
handle never materialized. os.NewFile(3, "ready-pipe") returned a stale
handle, the port write went nowhere, and the parent timed out without a
session file ever being written. This is what was producing the
TestClientExitsOnFinish failure on windows-latest.

Switch to stdout-based readiness signaling instead. cmd.Stdout = writeEnd
works on every supported OS because stdin/stdout/stderr inheritance is
guaranteed. After the child writes its port, openReadyPipe repoints
os.Stdout at os.Stderr (the daemon log file) so subsequent stray writes
don't race the handshake or hold the parent's read-end open.

Also:
- platform_windows.go: replace literal 259 with a named stillActive
  constant (golang.org/x/sys/windows doesn't export STILL_ACTIVE).
- test.yml: kill leftover crit.exe processes after the unit-windows job
  so actions/setup-go's post-cache step doesn't fail packing the cache
  tar with a locked binary.

Five Windows-only test failures introduced by the v4 folder-format review
storage migration on main. All were test-side portability bugs, not
production logic bugs:

1. TestReviewPathsFor — hardcoded forward-slash expectations. Rebuild
   expected paths via filepath.Join so the assertion matches whichever
   separator the platform's filepath.Join produces.

2. TestFindReviewFileByCommentID_FolderForm,
   TestFindReviewFileByBranch_FolderForm/_OrphanFolderSkipped/
   _MigrationFallback/_MalformedJSONSkipped,
   TestFindReviewFileByCommentID_AmbiguousAcrossFolderAndFlat,
   TestWalkReviewIdentities_SkipsOrphanFolderSilently — used bare
   t.Setenv("HOME", ...) to redirect reviewsDir(). On Windows
   os.UserHomeDir reads USERPROFILE, not HOME, so the override silently
   no-op'd and the scan walked the real ~/.crit/reviews/. Cross-run
   pollution there caused the ambiguous-match failures. Switch to the
   existing setHome(t, dir) helper, which sets HOME, USERPROFILE, and
   the HOMEDRIVE/HOMEPATH fallback together.

3. TestCarryForwardAllComments_NoDuplicateOnDisk and
   TestCarryForwardComments_NoDuplicateOnDisk — passed ReviewFilePath as
   a flat <dir>/review.json path, which the v4 layout treats as the
   identity folder and resolves to <dir>/review.json/review.json.
   atomicWriteFile then tried MkdirAll on a path component that was a
   regular file. On POSIX the test passed by accident: the write failed
   silently, leaving the pre-seeded flat file (1 comment) intact, and
   the disk-state assertion happened to match. On Windows the same
   structural mismatch surfaces as ERROR_PATH_NOT_FOUND. Move the
   fixtures to the canonical v4 layout: identity = <dir>/.crit folder,
   review.json inside.

No production code changed.

… subprocesses

Two distinct test-infra bugs the windows-latest e2e job exposed:

1. setup-fixtures.sh / -range-mode.sh / -singlefile.sh used the raw
   `mktemp -d` output. On Git Bash that returns an MSYS-style path
   (e.g. /tmp/tmp.XXX) which gets written to the per-port state file and
   then read back by Node and passed to Go subprocesses — both interpret
   the path on the wrong drive root. The filemode/multifile/nogit fixtures
   already wrap mktemp in `realpath` for exactly this reason; do the same
   for the remaining three. Also realpath FAKE_HOME so USERPROFILE is a
   native Windows path (D:\...) — Go's filepath.Join on Windows does not
   round-trip MSYS paths, so the fixture's ~/.crit.config.json (which sets
   agent_cmd) was never loaded by the daemon.

2. cli-comment.spec.ts only set HOME on the spawned `crit comment`
   subprocess. On Windows os.UserHomeDir() reads USERPROFILE, so the
   subprocess looked at the runner's real home and missed the fixture's
   daemon registry. Pass USERPROFILE alongside HOME.

The finish handler returned `review_file: <path>` to clients (CLI, e2e
tests) and expected the file to be readable at that path immediately.
Two issues let the response race ahead of the on-disk write, surfacing
on Windows e2e as ENOENT when Node opened the path:

1. handleFinish called the bare WriteFiles() which does not coordinate
   with the 200ms debounce timer armed by earlier comment writes. Two
   concurrent atomicWriteFile calls on the same target in the same
   directory could fail (sharing-violation retry exhaustion under
   Windows AV / temp-file thrash) — and even when both succeeded, the
   handler did not check for write errors.
2. atomicWriteFile errors were logged to stderr and swallowed. The
   handler still returned 200 with a path that had no file behind it.

Fix:
- Add Session.SyncWriteFiles: takes writeMu, stops the pending debounce
  timer, bumps writeGen so a timer that already passed Stop() but has
  not yet acquired writeMu observes the gen change and bails. Returns
  the underlying write error.
- Refactor WriteFiles into writeFilesErr (returns error). WriteFiles
  remains a void wrapper for the timer callback and best-effort
  flushes (focus change, shutdown) which preserve the prior
  log-and-continue semantics.
- handleFinish uses SyncWriteFiles and surfaces failures as 500. The
  e2e suite now sees a real error rather than a 200 + dangling path.

This is structurally a Linux race too — POSIX rename atomicity covers
the file-presence invariant (so Linux readers always see one of the
two writes), but the unchecked write error and racing snapshots are
both latent regardless of platform. Windows just lost the lottery
more often because of the wider sharing-violation retry window.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

`realpath "$(mktemp -d)"` on Git Bash returns POSIX form (`/tmp/tmp.XXX`)
even with coreutils. The previous fix assumed it converted to native
Windows form. Two failure modes that re-surfaced on the windows-latest
e2e job:

1. FAKE_HOME exported as POSIX path → Go's `os.UserHomeDir()` reads it
   from USERPROFILE, then `filepath.Join` on Windows builds
   `\tmp\tmp.XXX\.crit\reviews\<key>\review.json` — a drive-less path
   that resolves against the calling process's current drive. The
   daemon's working drive and the Node test reader's drive can differ,
   so the test sees ENOENT for a file the daemon wrote on a different
   drive. Symptom: ~25 failures reading `finishData.review_file`.

2. fixtureDir as POSIX path passed as Node spawn cwd → child_process
   rejects it before launching cmd.exe. Symptom: cli-comment.spec.ts
   surfaces this as `spawnSync C:\Windows\system32\cmd.exe ENOENT`.

Add `e2e_native_path` / `e2e_native_tempdir` helpers in lib.sh that use
`cygpath -m` on Git Bash to emit drive-letter-prefixed paths
(`C:/Users/...`) that Go's filepath.Join and Node's spawn cwd both
understand. Replace every `realpath \"\$(mktemp -d)\"` in setup-fixtures*
with `e2e_native_tempdir`. Also rewrite the existing portability
comments to capture the actual realpath/cygpath distinction.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>