test(desktop): make the two Linux-only tests run on Windows - #177
Merged
Conversation
Both were POSIX-only by accident rather than design, and both were found by the Windows lane's first run. update-relaunch.test.ts rooted its fixture at the bare literal /home/u/.hermes/hermes-agent. resolveUnpackedRelease normalizes its execPath with path.resolve, which on Windows prepends the current drive, so a driveless root could never equal the resolved exec path. ROOT is now path.resolve of the same literal — a no-op on POSIX, and it makes both sides agree on Windows. windows-hermes-path.test.ts hard-coded /venv/lib/python3.12/site-packages in its POSIX case, while the Windows case three tests above already built its expectation with path.join. The implementation picks the layout from isWindows but still joins with the host separator, so the literal never matched on Windows. Now built with path.join, matching the sibling. Verified by simulating Windows path semantics, since neither a Windows host nor vitest is available here — the test files use extensionless imports, which vite resolves and bare Node does not. Faithful replicas of both implementations were run with the path flavour injected and path.resolve given the runner's real cwd, so the drive-letter prepend is reproduced rather than assumed. The simulation reproduced the CI failures before fixing them: under win32 the old code failed exactly the two reported assertions plus the second assertion inside the first test, which vitest never reached because it stops at the first failure in a test. After the change, 14 passed and 0 failed across both posix and win32. The neighbouring negative assertions — AppImage mount, system install, dev electron, empty path — were included and hold under both flavours, so resolving ROOT does not weaken them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 11, 2026
Held back from #177 deliberately — over-including files caused both earlier rounds on this lane, so the fix landed first and the widening comes now that both files are cross-platform on main. Four files to six. The check I owe, since these are whole files (16 and 22 tests) rather than just the two assertions repaired in #177: in the first full-suite Windows run, job 93652696640, the only failures from these two files were update-relaunch.test.ts:54 and windows-hermes-path.test.ts:177. Every other test in them was observed passing on a Windows runner. The ssh-connection failures from that run are in a file that stays excluded. One gap observation could not cover: vitest stops at the first failing assertion in a test, so the second assertion of the test at :53 was never reached on Windows. The win32 simulation covers it explicitly and it passes after the repair — 14 passed, 0 failed across posix and win32. Selection re-verified: six of the project's 80 test files, with before-pack, stage-native-deps and ssh-connection still excluded. Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8 Co-authored-by: Claude <noreply@anthropic.com>
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 11, 2026
The last open item from the Windows-lane work. Seven files now. Inspected before adding rather than adding and hoping: over-including unverified files caused rounds #174 and #175, and since #176 the lane is a required check, so being wrong now blocks everyone's PRs rather than just mine. The sweep covered Windows-hostile syscalls, separator-dependent assertions and shell assumptions. It found a third instance of the defect class behind #177: `assert.match(a, /\/[0-9a-f]{16}\.sock$/)` against controlSocketPath, which builds with path.join — a leading-slash regex can only match on POSIX. Now asserted on path.basename(a), which is separator-independent and is what the sun_path limit is actually about. Checked under both path flavours. Everything else in the file is safe for stated reasons rather than by assumption: the /var/folders/ check is a negative assertion; ControlPath passes through baseSshOptions verbatim, which does no path joining; the `cd '/home/me/project'` assertion is a remote shell command and remotes are always POSIX; and the path.join calls create directories inside the two tests that already carry win32 guards. Also corrects the workflow header, which #179 had made false — it still said ssh-connection was POSIX-only and that making it cross-platform was tracked as follow-up, which is precisely what #179 did. Remaining exposure is unknown-unknowns across ~40 tests that cannot be executed from this sandbox. If the lane goes red on this, the fix is to drop ssh-connection back out of the list — one word, and the test repairs stand on their own. Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8 Co-authored-by: Claude <noreply@anthropic.com>
11 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 11, 2026
…it (#183) The same defect turned up three times across #177 and #180: a hard-coded absolute POSIX path meeting a path the code builds with path.join. On Windows path.join emits backslashes and path.resolve prepends a drive, so the two sides can never be equal and the assertion is dead on that platform — silently, because the file had only ever run on Linux. Two were found by the Windows lane going red; the third by reading a file before adding it. One at a time, after the fact, on a required check. The rule does NOT ban leading-slash literals. A blanket ban is unusable: the seven lane files hold 71 absolute POSIX literals, and all but the three above are inputs — a fake execPath, a ControlPath handed to ssh, a path the stub filesystem is asked about. Coping with those is the job of the code under test. The defect is where a literal meets a computed path, so the rule keys on that meeting: path.join onto a POSIX root (fix: wrap in path.resolve, a no-op on POSIX), a comparison against a POSIX literal, and a regex that opens with a path separator. "Opens with" rather than "contains" keeps the shebang check and the remote-shell-command assertion — both legitimately POSIX, both in lane files today — quiet. Scoped to the suites in test:desktop:win-install, and that list is parsed from package.json rather than restated in the config. A second copy fails silently: a suite added to the lane but missing from the config gets no cross-platform linting at exactly the moment it starts needing it. Verified by observation, not assertion. Run over the pre-fix files (1770376^, 50e2b2b^) the rule reports exactly the three known defects and nothing else, and none of them on the current files. The self-maintenance claim was proved the same way: a probe file with a known defect drew no report until its name alone was added to the lane script — no config edit — after which it did. Two false positives are kept and annotated rather than engineered away. windows-hermes-path compares against /root/venv/Scripts/python.exe twice, and both are safe because every path helper that resolver touches is injected by makeDeps and joins with '/'. The disables carry that reason, which is the rule earning its keep — "why is this POSIX literal safe on Windows?" is what the next person adding a suite needs answered. The rule's own tests caught two bugs in it first: expect(x).toBe(v) puts the expected value at argument 0, not 1, so expect-style assertions were never checked; and the lane parser consumed the token after every flag, so a boolean -u swallowed the suite name behind it. It now consumes values only for known value-taking flags, erring towards a glob that matches nothing rather than silently dropping a suite. Separately, the first disable comments did not work at all — eslint-disable-next-line followed by more comment lines applies to the next comment. Found by running lint, not by reading it. lint now covers eslint-rules/ and scripts/*.test.mjs, because three lane suites live in scripts/ and were outside `eslint src/ electron/`. The rest of scripts/ stays unlinted; it carries 10 pre-existing errors that are not this change. eslint becomes a declared root devDependency — it was only ever an auto-installed peer, and the rule's tests import RuleTester from it. The lockfile delta is mechanical peer-marker removal with no version movement. check:lint 0 errors (89 warnings, unchanged), test:desktop:platforms 976 passed, lane script 117 passed. Claude-Session: https://claude.ai/code/session_012689txgT12g2hjRczcUZi8 Co-authored-by: Claude <noreply@anthropic.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
2 participants
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.
Clears the last actionable follow-up from the Windows lane work. Both tests were POSIX-only by accident rather than design, and the Windows job found both on its first run.
The two fixes
update-relaunch.test.tsrooted its fixture at the bare literal/home/u/.hermes/hermes-agent.resolveUnpackedReleasenormalizes its execPath withpath.resolve, which on Windows prepends the current drive — so a driveless root can never equal the resolved exec path:ROOTis nowpath.resolve(...)of the same literal: a no-op on POSIX, and both sides agree on Windows.windows-hermes-path.test.tshard-coded/venv/lib/python3.12/site-packagesin its POSIX case — while the Windows case three tests above already builds its expectation withpath.join. The implementation picks the layout fromisWindowsbut still joins with the host separator, so the literal never matched on Windows. Now built withpath.join, matching its sibling.Verification: simulated Windows semantics
Neither a Windows host nor vitest is available in my sandbox — the test files use extensionless imports, which vite resolves and bare Node doesn't. So rather than assert a Linux pass and call it done (which would prove nothing about a Windows-only bug), I built faithful replicas of both implementations with the path flavour injected, giving
path.resolvethe runner's real cwd so the drive-letter prepend is reproduced rather than assumed.Run under both posix and win32 semantics, before and after:
The simulation independently reproduces the CI failures. Note it shows three where CI reported two: "unpacked itself" is the second assertion inside the first test, which vitest never reached because it stops at the first failure in a test. That's consistent, not a discrepancy.
The neighbouring negative assertions — AppImage mount, system install, dev electron, empty path — were included in the simulation and hold under both flavours, so resolving
ROOTdoesn't weaken them.Not included
These two files are still not in the Windows lane's
test:desktop:win-installlist. I'd rather land the fix and let a later change add them, than widen the lane in the same PR that claims to fix them — the last two rounds on that lane were both caused by me over-including files. Adding them is a one-line change once this is onmainand they've been seen green there.Follow-ups still open
electron-buildercan't be imported under vitest on Windows (likely upstream).Generated by Claude Code