test(agent-core-v2): make the test suite pass on Windows#2035
Conversation
🦋 Changeset detectedLatest commit: 1abb298 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a39bb8b47
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| import { ZipFile } from 'yazl'; | ||
|
|
||
| /** Zip every file under `sourceRoot` (recursively) and return the archive bytes. */ |
There was a problem hiding this comment.
This per-function JSDoc sits outside the top-of-file helper header. In packages/agent-core-v2, local convention keeps comments solely in the file header and forbids comments beside functions/statements, so this explanation should be folded into the opening block instead of attached to the function.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
|
Good catch — folded all helper documentation into the top-of-file comment blocks and dropped the inline comments across the new harness helpers and touched test files (4f21469). Also swept the rest of the PR for the same convention violation; nothing else remains. |
Hook test commands were executed through spawn(shell: true), which on Windows means cmd.exe. Two POSIX assumptions broke them there: - 'echo ... >&2; exit 2' style commands: cmd.exe has no ';' separator. - 'node -e "<script>"' with JSON.stringify quoting: cmd.exe toggles quoting at every '"' (it does not understand \"), shredding scripts containing double quotes into multiple arguments. Introduce test/harness/nodeScriptCommand: write the script to a temp .cjs file and invoke node on it — no shell quoting on either platform. Also switch a tool-result path assertion to pathe join, matching the implementation's forward-slash rendering.
The test stubs and in-memory fake fs keyed paths with POSIX literals and node:path join, while session services resolve with node:path (drive-letter absolute on Windows) and normalize with pathe. Unify the test file on pathe and normalize fake-fs lookup keys; assertions unchanged.
URL.pathname yields a leading-slash form (/D:/...) on Windows, so every stdio fake server spawned by the MCP tests failed to start. fileURLToPath is platform-aware and identical on POSIX.
…zip CLI The plugin tests invoked the external Info-ZIP binary, which does not exist on Windows. Add test/harness/zipArchive.ts (yazl, already a repo dependency) producing the same archive layout.
Session services resolve configured dirs with node:path (drive letters on Windows) while the local-config backend normalizes with pathe. Derive seed keys and expected values from the same POSIX literals via resolve/normalize helpers instead of hard-coded POSIX paths.
…link capability sessionLifecycle additional-dir expectations now derive from node:path resolve (identical values on POSIX). The two profile symlink tests skip via a runtime EPERM probe when the platform cannot create symlinks (unprivileged Windows), keeping full coverage elsewhere.
- pathe-align path expectations with implementation output (agentRoots, skillRoots, skillCatalog, glob, toolResultTruncation) - probe symlink capability and skip when unavailable (hostFsService, workspaceRegistry, sessionExport) via test/harness/symlinkSupport.ts - skip the POSIX mkdir mode-bit assertion on Windows (persist), matching an existing fileStorageService precedent - route the fullCompaction hook command through nodeScriptCommand so the hook script survives cmd.exe (fullCompaction)
Fold helper documentation into top-of-file comment blocks and drop inline comments in test files, per packages/agent-core-v2/AGENTS.md comment conventions (review feedback).
9548d6b to
1abb298
Compare
1 participant
Problem
The
agent-core-v2test suite currently fails 98 tests across 23 files on Windows (Node 24, Windows 11) while passing on POSIX. All failures are platform assumptions in test code — no product bugs were found. This PR makes the suite fully green on Windows without weakening a single assertion, and keeps every test passing unchanged on POSIX.This is also a first step toward re-enabling the
test-windowsCI job disabled in #1144 (for theagent-core-v2package; other packages may still have Windows failures to address separately).Root causes and fixes (one commit per family)
Hook test commands are not shell-portable — hook tests spawn commands via
spawn(shell: true), which is cmd.exe on Windows. POSIX constructs (echo ... >&2; exit 2) andnode -e "<script>"with JSON.stringify quoting both break: cmd.exe toggles quoting at every"(it does not understand\"), shredding scripts that contain double quotes. → Newtest/harness/nodeScriptCommand.tswrites the script to a temp.cjsfile and invokesnodeon it — no shell quoting on either platform.fsService fake-fs path mismatch — stubs keyed paths with POSIX literals /
node:pathjoin while session services resolve withnode:path(drive letters on Windows) and normalize withpathe. → Unify the test file onpatheand normalize fake-fs lookup keys.MCP stdio fake servers fail to start — fixture paths were built with
URL.pathname, which yields/D:/...on Windows. →fileURLToPath(platform-aware, identical on POSIX).Plugin tests shell out to the
zipCLI — Info-ZIP does not exist on Windows. → Newtest/harness/zipArchive.tsbuilds the same archive layout withyazl(already a dependency).Hard-coded POSIX path expectations (sessionLifecycle, workspaceCommand) — expected values like
/tmp/extraresolve toD:\tmp\extraon Windows. → Derive expectations withresolve/normalizefrom the same literals; byte-identical values on POSIX.Remaining files — pathe-align path expectations with implementation output (agentRoots, skillRoots, skillCatalog, glob, toolResultTruncation); probe symlink capability at runtime and skip when unavailable (hostFsService, profile/context, workspaceRegistry, sessionExport) via new
test/harness/symlinkSupport.ts— on Developer-Mode Windows and POSIX these tests still run in full; skip the POSIX mkdir mode-bit assertion on Windows (persist), matching an existingfileStorageServiceprecedent; route the fullCompaction hook command throughnodeScriptCommand.Scope
packages/agent-core-v2/test/, plus two new harness helpers. Zero changes undersrc/.Verification
pnpm -C packages/agent-core-v2 typecheckclean;pnpm vitest run→ 3864 passed | 14 skipped (capability probes) | 0 failed (previously 98 failed).fileURLToPath/resolveproduce identical values; capability probes only skip when the capability is genuinely absent). CI will confirm.Checklist
@moonshot-ai/agent-core-v2patch).