ci: add windows-latest to the test matrix#926
Merged
Conversation
Recent PRs hit Windows-specific bugs that Ubuntu CI couldn't catch — literal forward-slash separators in path comparisons (#911) and `partial.startsWith("/")` as an "is absolute path" heuristic (#922) both sailed through `ubuntu-latest` while shipping broken on the platform that most of the user base actually runs. Add `windows-latest` to the existing matrix so every PR exercises the Windows code paths (taskkill tree-kill, MS Store node-shim detection, path separator handling, etc.) before review. Job name now includes the OS so the two runs are distinguishable in branch protection. Also swap the hardcoded `/tmp/tau-dry.json` in the τ-bench dry-run for `${{ runner.temp }}` — RUNNER_TEMP is set on every GitHub Actions runner including Windows, where `/tmp` isn't a path.
This was referenced May 15, 2026
esengine
added a commit
that referenced
this pull request
May 15, 2026
…870) (#932) * feat(skills): auto-discover .agents/skills as a default root Closes #870. The configurable-paths system from #925 lets users point at any directory, but a user with existing skills already sitting in `~/.agents/skills` or `<project>/.agents/skills` still has to find that setting and configure it — which is exactly the friction the original issue asked us to avoid. Append `.agents/skills` to the default discovery list in `SkillStore.roots()` at both project and global scope, alongside the existing `.reasonix/skills` entries. Anything sitting in the skills.sh-style convention now just works on first launch. * test(settings): make skillPaths absolute-path assertion platform-agnostic The combined-POST test from #925 asserts `resolved: "/opt/skills"` for an input of `/opt/skills`, which only holds on POSIX. On Windows `/opt/skills` isn't an absolute path — `path.resolve` rebases it against the current drive and returns `F:\opt\skills` (or whatever `CWD`'s drive is). Same behavior as the production code, just the test forgot to mirror it. Wrap the expected `resolved` in `resolve(absolute)` so both the production resolver and the test compute the value through the same function. Linux output unchanged. (This would have been caught by the matrix CI from #926, but #925's PR build ran a few minutes before that landed.) --------- Co-authored-by: reasonix <reasonix@deepseek.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Recent PRs landed Windows-specific bugs that Ubuntu-only CI couldn't catch — literal forward-slash separators in path comparisons (#911) and
partial.startsWith("/")as the "is absolute path" heuristic (#922) both sailed through green onubuntu-latestwhile shipping broken on the platform the bulk of our users actually run.Add
windows-latestto the existing matrix so every PR exercises the Windows code paths (taskkill tree-kill, MS Store node-shim detection, path separator handling,CREATE_NO_WINDOW, etc.) before review. Job name now includes the OS so the two runs are distinguishable in branch protection.Side fix: the τ-bench dry-run hardcoded
/tmp/tau-dry.json, which isn't a path on Windows. Swap for${{ runner.temp }}— set on every GH Actions runner, POSIX and Windows alike.Test plan
windows-latestwill be mostly green.windows-latestin CI that didn't break locally, I'll triage in a follow-up commit on this branch: either fix the test (preferred) or markit.skipIf(process.platform === "win32")with a one-line reason. Goal is the matrix lands green; we'll inspect the first run's output to decide which path each failure takes.Why not also macOS
Most behaviors mirror Linux. The differences (BSD-style
ps/pkill,detachedprocess-group semantics) aren't currently exercised by any test that doesn't also run on Linux. Cheap to add later if a Mac-specific bug surfaces.