test(coding-agent): make the harness-state atomicity test pass on Windows - #1325
test(coding-agent): make the harness-state atomicity test pass on Windows#1325Hotragn wants to merge 1 commit into
Conversation
…dows
Two assertions in "atomically replaces harness state without leaving temporary
files" assume POSIX and fail on Windows:
- `statePath.split("/").at(-1)` splits a path that `saveHarnessState` built
with `path.join`, so on Windows the separator is `\` and the split returns
the whole path instead of the filename. Use `basename`.
- `chmodSync(path, 0o600)` then asserting `mode & 0o777 === 0o600` cannot hold
on Windows, where chmod only toggles the read-only bit and Node reports
0o666. Run the mode-preservation assertions on POSIX only; the file is still
written and re-saved on every platform.
Coverage on Linux and macOS is unchanged. CI runs ubuntu-only today, so this
does not fix a red build; it removes a false failure for anyone developing on
Windows and is a prerequisite for running the suite on a Windows matrix leg.
jonaowen
left a comment
There was a problem hiding this comment.
basename is the correct cross-platform filename projection. The mode-preservation assertion is specifically about POSIX permission bits, which Windows cannot represent through Node chmod; retaining both save operations on Windows while gating only that assertion preserves the platform-independent atomic replacement coverage.
|
Hi, thanks for taking the time to contribute to Prime Agent! Since open sourcing the project, we’ve received far more pull requests than we can responsibly review and validate. Prime Agent runs directly on users’ machines, so we need to be deliberate about which changes we accept and how they are reviewed. Rather than leave a large backlog that we cannot meaningfully work through, we’re closing the current PR queue and moving to a discussion-first contribution process. We have established new contribution guidelines to help us continue iterating on Prime Agent and better manage contributions from the community. Going forward, we won’t review unsolicited pull requests. Instead, please start with a GitHub Discussion. We’ll identify recurring bugs and feature requests, create Issues for work we want to pursue, and invite pull requests from maintainers or vouched contributors when implementation is ready. Please read the full process documented in our contribution guidelines. While we’re closing this backlog, we’re still reviewing it at a high level to identify recurring bugs, useful ideas, and important problems that we should address ourselves. Thanks again for the time you put into this! |
test/refinement.test.ts > atomically replaces harness state without leaving temporary filesfails on Windows for two POSIX assumptions. Neither is a product bug — the test is wrong, not the code.1. Path separator
saveHarnessStatebuildsstatePathwithpath.join, so on Windows it contains\. Splitting on/returns the whole path rather than the filename:basenameis separator-correct on every platform.2. File mode
Windows
chmodonly toggles the read-only bit; Node reports0o666(438), so this asserts something the platform cannot express:The behaviour under test —
saveHarnessStatereading the existing mode and preserving it across the atomic rename — is genuinely POSIX-only, so the mode assertions now run on POSIX only. The save and re-save still execute on every platform, so the atomicity half of the test is unchanged everywhere. Coverage on Linux and macOS is identical to before.Scope
This does not fix a red build: CI is ubuntu-only today, where the test already passed. It removes a false failure for anyone running the suite on Windows, and it is a prerequisite for the Windows matrix leg proposed in #781. There are ~20 open Windows issues, so contributors landing there currently have to distinguish this failure from their own.
Verified:
packages/coding-agenttest/refinement.test.ts59/59 on Windows (was 58/59 with this one failing), andnpm run checkclean.Note
Fix harness-state atomicity test to pass on Windows
Updates the atomicity test in refinement.test.ts to use
basename(statePath)instead of a Unix-style string split, and skipschmod/statpermission assertions on Windows since the platform does not support POSIX file modes.Macroscope summarized b74f8c4.