fix(tests): corregir fallos de tests pre-existentes en entornos Windows

[Basparin]

🔗 Linked Issue

Closes #103

🏷️ PR Type

• type:bug — Bug fix (non-breaking change that fixes an issue)

🙏 Credit

Retoma el trabajo de @atarico en #104 (cerrado por un tema de proceso, no técnico — el issue #103 no tenía status:approved al momento). El commit original c765bef se preserva con su autoría intacta (Author: atarico); un segundo commit mío (test(sdd): extend disablePluginInstall to TestInjectCopiesAllFiles tests) extiende el mismo patrón a dos tests hermanos que quedaron afuera. Comenté en #104 avisándole antes de abrir este PR.

📝 Summary

Al correr go test ./... en Windows existen tres categorías de fallos pre-existentes causados por incompatibilidades del entorno, no por errores de lógica del proyecto. Este PR cubre las tres:

1. CRLF en golden files — la conversión automática de Git en Windows insertaba CRLF en los archivos bajo testdata/golden/, haciendo que los tests TestGolden* fallaran porque el código Go escribe LF. Solución: .gitattributes con * text eol=lf.
2. Separadores de ruta hardcodeados — TestSkillPathForAgent comparaba con paths /home/test/... que en Windows resultan \home\test\.... Solución: filepath.FromSlash() en el valor esperado.
3. Tests OpenCode que requieren bun/npm — varios tests instalan el plugin de background-agents, que falla si el package manager no está. Solución: skipIfNoPkgManager() para skip cuando el entorno no tiene manager, y disablePluginInstall() para mockear la lookup cuando el test ya no depende del plugin real.

📂 Changes

File / Area	What Changed
.gitattributes	Agrega * text eol=lf para forzar LF en todos los sistemas
internal/components/golden_test.go	Helper para goldens en Windows
internal/components/sdd/inject_test.go	Agrega skipIfNoPkgManager() + disablePluginInstall() helpers y los aplica a todos los tests afectados (atarico cubrió 15+ tests; este PR extiende a los 2 TestInjectCopiesAllFiles* restantes)
internal/components/skills/inject_test.go	filepath.FromSlash() en TestSkillPathForAgent
testdata/golden/sdd-vscode-instructions.golden	Regenerado tras rebase sobre el main actual (el original de atarico era de hace 4 semanas, antes del rewrite de persona-neutral de Alan)

Total: 5 files, +218/-3 lines, 2 commits.

🧪 Test Plan

Unit Tests (Windows 11 Pro local)

go test ./internal/components/...

• TestSkillPathForAgent — PASS (antes FAIL por path separator)
• TestGoldenEngram_Antigravity — PASS (antes FAIL por CRLF)
• TestGoldenSkills_Claude / _OpenCode / _Windsurf / _Kiro — PASS
• TestInjectOpenCodeMultiMode + 8 tests hermanos — PASS (antes FAIL por bun/unique-names-generator)
• TestInjectCopiesAllFilesFromSkillDirectory — PASS (extensión de este PR)
• TestInjectCopiesAllFilesReportedInResult — PASS (extensión de este PR)
• TestInjectStrictTDDIsIdempotent y el resto del suite — PASS

Fuera de scope (pre-existing, no relacionado a este issue): TestInjectVSCodeWritesContext7ToMCPConfigFile en internal/components/mcp falla con Inject() first changed = false tanto en origin/main limpio como en esta rama. Es un bug distinto que amerita su propio issue; no lo toco acá para no ampliar scope.

E2E Tests (Docker required)

Not run

✅ Contributor Checklist

• PR is linked to an approved issue (fix(tests): fallos de tests pre-existentes en entornos Windows #103, status:approved)
• Unit tests pass locally for all affected tests
• My commits follow Conventional Commits format
• My commits do not include Co-Authored-By trailers
• Branch name matches required regex
• Co-author (@atarico) preserved on original commit via cherry-pick

⏳ Pending maintainer actions

• Apply type:bug label to this PR (contributor cannot self-apply; CI validation blocks merge until then)

[Alan-TheGentleman]

La idea puede servir, pero falta ordenar el proceso y revisar solapamiento.

Sumá el label type:*, refrescá CI contra main y compará explícitamente este cambio con #490 para que no revisemos dos fixes de Windows que pisan el mismo problema.

[Basparin]

@Alan-TheGentleman refreshed against current main + addressed the #490 comparison you asked for.

Refresh

Merge commit: 4af2c9c (clean, zero conflicts).

While running the suite against the merged tree I caught a new test introduced by main that the original PR pattern hadn't reached yet:

• TestInjectCopiesNestedSDDSkillReferences — was missing disablePluginInstall(t), so it tried to install unique-names-generator into a temp dir on Windows and failed the post-install check.

Extension applied in commit 5553249 — one-line addition, same pattern this PR already uses across its sibling tests.

Local validation

go test ./internal/components/sdd/   -run "TestInjectCopiesNestedSDDSkillReferences|TestInjectCopiesAllFiles"
go test ./internal/components/skills/ -run TestSkillPathForAgent
go test ./internal/components/        -run TestGoldenEngram_Antigravity

All 3 previously-failing cases now PASS. The remaining unit-test failure on CI is from internal/cli/run_integration_test.go::TestRunInstallEngramForPiAndOpenCodeProvisionsBothMCPTargets — that's a Pi-feature regression that landed on main (last two fix(pi): commits left main red — see runs 25641715595, 25641492770). Same upstream issue blocking #374 right now — not anything this PR introduced.

Boundary vs #490 (tonyblu331)

You asked explicitly for this, so here it is concrete:

Aspect	#490 (tonyblu331)	#372 (this)
Scope	internal/cli/ only	internal/components/ (sdd + skills + golden)
Files touched	7, all under internal/cli/	5, all under internal/components/ + .gitattributes
Failure category	CLI test env contamination — real HOME / USERPROFILE / APPDATA bleeding into tests, Windows cmd.exe for shell capture, PATH normalization in CLI guidance	CRLF in golden files (.gitattributes), path-separator hardcoded in SkillPathForAgent, opencode-plugin install requiring bun add unique-names-generator in test temp dirs
Helpers introduced	isolateAgentDiscoveryEnv, ensureOpenCodeSDDNodeStub (in internal/cli/)	skipIfNoPkgManager, disablePluginInstall (in internal/components/sdd/)
Overlap	Zero files	Zero files

Both PRs link Closes #103 because #103 is the umbrella "Windows test failures" issue. They are complementary slices, not duplicate fixes:

• test(cli): stabilize cross-platform env and path assertions #490 hardens the internal/cli/... test harness against the developer's real environment.
• fix(tests): corregir fallos de tests pre-existentes en entornos Windows #372 hardens internal/components/... against (a) text-encoding (CRLF) drift, (b) hardcoded forward slashes in skill paths, (c) hard dependency on bun/npm in plugin-install test paths.

I checked both diffs file-by-file before refreshing — there is no shared symbol, no shared helper, and no shared test name. The two PRs can land in either order without rebase conflicts; whichever lands first the other will merge cleanly.

If you'd prefer them landed in a specific order or want me to align helper naming conventions across the two, happy to coordinate with @tonyblu331.

Pending maintainer action

• Apply the type:bug label (the only blocker I can't unblock as an external contributor).

[Alan-TheGentleman]

@Basparin — friendly nudge: 10/12 checks green, only 2 failures left. Could you take another pass at the failing jobs and refresh against current main (we shipped v1.31.0 yesterday with 15+ PRs touching the install/upgrade/Windows paths — some test fixtures may need an update)? Once the last 2 are green I can review for merge. Thanks for sticking with it!

[Alan-TheGentleman]

@Basparin — heads-up: this PR's Unit Tests failure is TestRunInstallEngramForPiAndOpenCodeProvisionsBothMCPTargets. That test was rewritten in main by PR #660 (fix(agents/pi): use positional pnpm dlx argument, landed in v1.31.0) to expect the new pnpm dlx gentle-engram@X pi-engram init form instead of the old pnpm dlx --package ... form.

Your branch still expects the old form, so the CI run uses the pre-#660 assertion and fails.

Fix: rebase against current main — the conflict should resolve cleanly (your changes are in different files than what #660 touched, but the integration test now has the new expectation). After the rebase, this specific failure goes away.

Pseudocode:

git fetch upstream main
git rebase upstream/main
# resolve any conflicts (likely none for this PR's scope)
git push --force-with-lease

The other PR you have open (and #371 + #374 + #372) all show the same failure for the same reason — same rebase fixes all three. Thanks for sticking with these.