fix(agents): keep Windows checkouts and rendered docs platform-independent

[tiendungdev]

First tranche of the Windows CI work for #652. Scope is deliberately small and touches no runtime behaviour outside the rendered-document path: it removes the failures that block reading the Windows suite at all.

What is wrong

1. The repository has no .gitattributes.

Git for Windows installs with core.autocrlf=true — the GitHub windows-latest runner included — so a Windows checkout rewrites every text file to CRLF. Two things break:

• internal/agents/opencode/plugin/gortex.js and internal/agents/pi/extension/index.ts are go:embed'd and written verbatim into a user's project. A source build on Windows therefore ships CRLF assets. TestPluginFailsOpen already detects it — it splits pluginSource on "\n}\n", which stops matching.

• gofmt -l flags every Go file in the tree. Control on an untouched file:

  $ file -b internal/agents/aider/adapter.go
  Unicode text, UTF-8 text, with CRLF line terminators
  $ gofmt -l internal/agents/aider/adapter.go
  internal/agents/aider/adapter.go          # unformatted
  
  # same file, re-checked-out under the new .gitattributes
  $ file -b internal/agents/aider/adapter.go
  Unicode text, UTF-8 text
  $ gofmt -l internal/agents/aider/adapter.go
                                            # clean
  

  So any formatting or lint gate on the Windows leg would fail wholesale, and a Windows contributor cannot separate real findings from the noise.

The committed blobs are already LF. git add --renormalize . reports no content change — only the files this PR edits appear — so this pins the checkout and rewrites nothing.

*.ps1 keeps CRLF; *.png / *.gz are marked binary.

2. GlobalPointerBody builds the @-include with filepath.Join.

On Windows that writes

@C:\Users\me\.gortex\instructions\active.md

into ~/.claude/CLAUDE.md. That line is document content, not a filesystem call, and every other path in the file is /-spelled. shellSafeHookBinary already normalises unconditionally for exactly this reason, with the rationale written in its doc comment.

UpsertMarkedBlock keys idempotency on the start/end markers (instructions.go:203, :216), not on the path string, so an existing install has its block replaced in place on the next run — no duplicate, no orphan.

I checked for a consumer that parses the line back: the only production caller is claudecode/adapter.go:268, and it only writes.

3. normalizeRender scrubs only the native spelling.

gortexBinaryPaths() collects native paths (os.Executable, exec.LookPath), and home / root arrive native too — but a rendered manifest carries /-spelled paths by design. On Windows the ReplaceAll never matches, so TestAgentsRenderGolden leaks a machine-specific absolute into the comparison:

-@$HOME/.gortex/instructions/active.md
+@D:/tmp/gobuild/gortex-render-home-3991093423/.gortex/instructions/active.md

and it drifts for any developer who has gortex installed on PATH. This is the same class the file's own canonicalManifestKey already guards against.

Verification

windows/amd64, go1.26.6, -count=1, measured against origin/main 699f351e:

package	before	after
internal/agents	ok	ok
internal/agents/opencode	1	0
cmd/gortex	19	18
internal/agents/claudecode	2	2 — both pre-existing (TestResolveHookCommand, TestEmitPluginBundle_HookHandlerExecBit)

Newly-broken set is empty. gofmt and go vet clean on the touched files.

Each fix was sabotage-verified separately:

• revert (1) → TestPluginFailsOpen: callHook has no closing brace this test can find
• revert (2) → claude-code golden drifts to @$HOME\.gortex\instructions\active.md
• revert (3) → claude-code and hermes goldens drift to absolute paths

TestAgentsRenderGolden now passes on Windows with the full PATH — i.e. with gortex actually installed — which it did not before.

Two assertions changed, and why

TestGlobalPointerBody_ShapeAndSentinel and TestGlobalInstall_FatToSlimReplacement built their expected @-include with filepath.Join. That asserts the native mangling on Windows, so it passes there whether or not the renderer normalises. They now use path.Join / filepath.ToSlash. In install_diet_test.go both spellings are kept deliberately — the @-include assertion takes the slash form, the os.ReadFile next to it keeps the native one.

Limitation, stated up front

As in #646, none of this can fail on the linux/macos matrix: filepath.ToSlash is a no-op on POSIX, and the CRLF conversion never happens there. The windows-latest leg from #652 is the only place these bind. I have not added a selector step — after your note on #652 that narrow selectors can miss the regression they are meant to protect, that seems like the wrong direction.

On the rest of #652

I ran the full suite on origin/main 699f351e (windows/amd64, go1.26.6, no -race): 173 failing tests across 28 packages, of which 2 are my machine, not the repo — internal/analysis TestMapGitDiff* and internal/releases leave a .git/ai/working_logs/ directory behind that a local git fork creates, and os.RemoveAll then fails. I will post the full classification on #652 with the buckets and the three questions that need your call before I can fix them.

[zzet]

Request changes: two Windows-only regressions are not protected by hosted CI.

1. P2 — Exercise the LF checkout guarantee on Windows (.gitattributes:14)

   The Windows job runs go test ./internal/agents, which does not recurse into internal/agents/opencode; it therefore never runs TestPluginFailsOpen, the regression detector for CRLF-embedded gortex.js. Removing this attribute would leave hosted checks green while Windows source builds could ship CRLF assets again. Please add the OpenCode package test under a CRLF-prone Windows checkout, or an equivalent explicit attribute/byte assertion.

2. P2 — Run the render golden on Windows (internal/agents/render.go:357)

   This second replacement only changes behavior on Windows because filepath.ToSlash is a no-op in the Linux/macOS jobs. TestAgentsRenderGolden lives in cmd/gortex, while the Windows job tests only ./internal/agents, so reverting this line would leave hosted checks green. Please run go test -timeout=5m -count=1 -run "^TestAgentsRenderGolden$" ./cmd/gortex on windows-latest, or add a direct Windows-binding unit test.

The implementation otherwise looks correct; I found no security or dead-code defects.

[tiendungdev]

Both right, and both for the same reason: neither fix was reachable from hosted CI, so reverting either would have left every check green. Pushed fc56f87e, rebased onto 6347e7f2.

1 — the OpenCode package never ran

Confirmed: go list ./internal/agents returns one package, so the step never recursed into internal/agents/opencode and TestPluginFailsOpen never executed on Windows. The step now names it explicitly.

But naming it only helps if the runner actually converts line endings, and I would rather not have the guard depend on that. So I took the second option you offered as well — an explicit assertion — and split it in two, because the halves fail in different situations:

• byte half — reads the embedded assets and rejects a \r\n. Fires on any checkout that already converted.
• attribute half — asks git check-attr eol and requires lf. Fires on the removal itself, and is the half that still binds on a runner configured not to convert, where a byte assertion would pass whatever .gitattributes said.

It also covers internal/agents/pi/extension/index.ts, which is embedded the same way and had nothing watching it.

2 — the golden is in the wrong package to bind

Confirmed: TestAgentsRenderGolden is in cmd/gortex and the Windows job only builds that. Rather than add a step for a single test, I put the binding assertion next to the code, in internal/agents, which the job already runs. TestNormalizeRenderScrubsTheSlashSpelledPath hands the function a native home and root with a /-spelled body — the shape a renderer actually emits — and requires both to be scrubbed.

Happy to add the -run "^TestAgentsRenderGolden$" ./cmd/gortex step instead if you would rather have the end-to-end version on the runner; it is one line either way.

Sabotage, each independently — windows/amd64, go1.26.6

reverted	result
the ToSlash pass in normalizeRender	all three assertions fail: home survived, root survived, machine-specific absolute still present
eol=lf dropped, files left LF on disk	eol attribute is "unspecified", want "lf" — byte half stays silent, which is exactly the case it exists for
eol=lf dropped, staged, asset re-checked-out	CRLF line ending at byte 40, plus the attribute half

One of those was wrong the first time and it is worth recording: removing .gitattributes from the working tree alone proves nothing, because git falls back to the staged copy — the asset came back LF and the assertion passed. The attribute has to be removed from the index before the sabotage is real. I had used the working-tree-only version earlier in this PR and got a valid result then only because the file was still untracked at that point.

internal/agents           ok
internal/agents/opencode  ok

gofmt and go vet clean.

[tiendungdev]

build-linux-static is red on infrastructure, not on this diff. It needs a re-run, which I cannot trigger from a fork.

The failing step is Run it where the dynamic build could not — the one that pulls debian:11 and alpine:3:

docker: Error response from daemon: received unexpected HTTP status: 502 Bad Gateway
##[error]Process completed with exit code 125

A registry 502 on docker run. Everything before it in the same job passed, including the actual subject of that job:

success  Build with the release link flags
success  Verify the binary is self-contained
success  Test the static-only code paths
failure  Run it where the dynamic build could not

This PR touches .gitattributes, two files under internal/agents, and the build-windows block of ci.yml — nothing that reaches the static link flags or the docker step.

The other eleven checks are green, and build-windows ran the new step:

success  Test Windows agent integrations      # now ./internal/agents ./internal/agents/opencode
success  Test native-separator store path comparisons

[zzet]

Re-review at head fc56f87 still requires changes.

1. P2 — the new EOL test breaks nested or copied source trees

repoRoot assumes that git rev-parse --show-toplevel is the Gortex module root. This was reproduced by placing the exact-head source under an enclosing Git repository at /src: the focused test attempted to open /internal/agents/opencode/plugin/gortex.js instead of /src/internal/agents/opencode/plugin/gortex.js and failed. That makes valid copied, vendored, monorepo, and packaging layouts fail go test.

Please locate the module or source root independently. Use Git attributes only after verifying that the discovered Git top-level is the same checkout; otherwise continue the byte assertion from the module root and skip only the attribute half. Add a nested-parent-worktree regression case.

2. P2 — executable-path normalization is still not guarded on Windows

normalizeRender now applies slash-aware scrubbing to gortexBinaryPaths(), but TestNormalizeRenderScrubsTheSlashSpelledPath puts only home and root paths in its input. Reverting only the binary-path loop change leaves the hosted Windows package tests green. Linux and macOS cannot distinguish this branch because filepath.ToSlash is a no-op there, while Windows builds but does not run TestAgentsRenderGolden.

Please include a slash-spelled executable path in the direct Windows-run test and require it to normalize to gortex, or run the end-to-end render golden on windows-latest.

The previous review findings are otherwise fixed, all hosted checks are green, and no security or dead-code issue was found. These remaining test-suite regression and coverage gap block merge.

[tiendungdev]

Both reproduced before fixing. Pushed 91bef013.

1 — the EOL test broke nested trees

Reproduced exactly as you described. Source copied to <outer-git>/src, .git removed from the copy:

embedded_asset_eol_test.go:43: internal/agents/opencode/plugin/gortex.js:
  open D:\tmp\nested\outer\internal\agents\opencode\plugin\gortex.js:
  The system cannot find the path specified.

git rev-parse --show-toplevel answered for the enclosing work tree, and the test then addressed files that were not there. Vendored copies, monorepo subdirectories and packaging staging trees all have that shape, so I broke go test in every one of them.

The root now comes from the nearest go.mod above the test's working directory, and the two halves are separated as you suggested:

• the byte assertion always runs, from the module root
• the attribute assertion runs only once the discovered git work tree is confirmed to be that same directory, and otherwise stands down with a logged reason

The identity check is os.SameFile, not string equality — git answers with forward slashes on Windows while filepath produces backslashes, and either side can arrive through a symlink or a different case.

Same nested reproduction after the fix:

--- PASS: TestEmbeddedTextAssetsArePinnedToLF
    module root D:\tmp\nested\outer2\src is not its own git work tree (<nil>);
    asserted the embedded bytes only, skipped the attribute half

TestModuleRootFromIgnoresAnEnclosingCheckout pins it, with an enclosing work tree and an enclosing go.mod, so neither a git lookup nor a careless upward walk passes by accident.

2 — the binary-path scrub was unguarded

Right: the test fed only home and root, so the third scrub target was never exercised and reverting that loop alone kept Windows green.

The test now pins gortexBinaryPaths to a known path and requires the slash-spelled executable to normalize to bare gortex. Pinning keeps the assertion about the scrub rather than about whatever gortex happens to be installed on the machine running it; no test in this package calls t.Parallel, so the swap cannot race one.

Sabotage, reverting only that loop to the native strings.ReplaceAll:

render_test.go:290: the slash-spelled executable survived normalizeRender
render_test.go:296: machine-specific absolute "/tools/bin/gortex" is still in the manifest

internal/agents           ok
internal/agents/opencode  ok

gofmt clean.

Separately: build-linux-static was red on the previous head from a Docker Hub 502 in the debian:11 pull, not from this diff — details in the comment above, in case the re-run is still needed.