ci: add windows-latest to the CI matrix - #48
Merged
Conversation
Rust clippy and tests ran only on ubuntu-latest, so Windows-specific defects were invisible to CI. This is how the item-locking bug in #35 (os error 33 on every mutating operation) shipped and stayed unnoticed for months — the release workflow builds a Windows binary but only smoke-tests `--version`. Rustfmt and the frontend lint/build stay on Linux only: both are platform-independent, and running them twice adds minutes for no signal. Clippy and tests run on both, since those are where platform differences actually surface. Bumps timeout 20 -> 30 minutes; Windows Rust builds are slower on a cold cache. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GR33TA8BuP9NAKh3PALrVc
ddkingsley
added a commit
to relsas/markplane
that referenced
this pull request
Sep 2, 2026
Follow-up to the sidecar-lock fix, addressing three gaps: 1. clippy::suspicious_open_options failed the build — OpenOptions with .create(true) and neither .truncate() nor .append(). Added .truncate(false), which is the correct intent for a lock file whose contents are never read or written. 2. Lock files were landing in users' repositories. `markplane init` alone produced six of them via seeded starter content, all staged by `git add -A`. Adding `.locks/` to GITIGNORE_TEMPLATE would only fix projects created after this change, since that file is written once at init. Writing a self-ignoring `.gitignore` containing `*` into `.locks/` when the directory is created fixes existing projects too, with no migration. 3. No test coverage. Added three: write_item under a held lock (the exact incompatible pair), update_status across a status sequence (the path users hit), and the lock directory being self-ignoring. Also expanded the doc comment to record why lock files are never reclaimed — deletion races with acquisition. The regression tests only fail on Windows; on Unix flock never blocked rename. They are meaningful because CI now covers windows-latest (zerowand01#48). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GR33TA8BuP9NAKh3PALrVc
ddkingsley
added a commit
that referenced
this pull request
Sep 2, 2026
`lock_item` took an fs2 byte-range lock on the markdown item file itself, which is incompatible with the atomic tempfile persist in `write_item`: on Windows the lock blocks MoveFileEx from replacing the target, failing with os error 33 (ERROR_LOCK_VIOLATION). Because the flaw was in `lock_item`, it broke every mutating operation on Windows — update_status, all four typed update_* methods, and link_items — not just status updates. Locks now live on sidecar files under .markplane/.locks/, preserving cross-process serialization while leaving the item file free to be replaced. The lock directory writes a self-ignoring .gitignore containing `*` when created, so lock files stay out of users' repositories. Adding .locks/ to GITIGNORE_TEMPLATE would only have helped projects created after this change, since that file is written once at init. Verified against the windows-latest CI matrix added in #48. Before: 27 of 75 CLI integration tests failed with os error 33, and the core and MCP suites never ran. After: 444 tests pass on both platforms, with three new regression tests covering write_item under a held lock, update_status across a status sequence, and the lock directory being self-ignoring. Diagnosis and the original fix by @relsas. Co-authored-by: DANIEL KINGSLEY <dan@zerowand.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GR33TA8BuP9NAKh3PALrVc
5 tasks
ddkingsley
added a commit
that referenced
this pull request
Sep 2, 2026
CI gained windows-latest in #48, but that was invisible in the docs: a contributor reading CONTRIBUTING.md had no way to know their PR runs on two platforms, or that Windows-only failures are possible. Adds a Continuous Integration section covering what runs where and the two failure modes single-machine testing misses — platform-specific behavior and stale lockfiles. Also aligns the documented check commands with what CI actually runs. CONTRIBUTING.md and the PR template both asked for bare `cargo test --workspace` / `cargo clippy --workspace`, but CI passes `--locked` and `--all-targets -- -D warnings`. A contributor could pass locally and fail CI on a stale Cargo.lock, or on a lint that only fires under --all-targets — which is exactly what happened on #35. Records the decision in TASK-sk3g3, alongside the original Linux-only rationale in TASK-yzftd whose "add platforms only if platform-specific failures emerge" clause this triggered. Claude-Session: https://claude.ai/code/session_01GR33TA8BuP9NAKh3PALrVc Co-authored-by: Claude Opus 5 (1M context) <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
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
CI ran Rust clippy and tests only on
ubuntu-latest. Windows-specific defects were structurally invisible.This is the direct cause of #35 sitting undetected:
lock_item()took anfs2byte-range lock on the item file itself, which on Windows blocks theNamedTempFile::persist()inwrite_item()from replacing it (os error 33,ERROR_LOCK_VIOLATION). Every mutating operation —update_status,update_task/epic/plan/note,link_items— failed on Windows.release.ymlbuilds a Windows binary but only smoke-tests--version, so it caught nothing either.Adding Windows here is a prerequisite for merging #35: without it, a regression test for that bug passes on Linux whether or not the bug exists, because
flocknever blocked renames.What runs where
fail-fast: falseso one platform failing still reports the other. Timeout 20 → 30 minutes for cold-cache Windows builds.Expected
First Windows run may surface pre-existing unrelated failures. That is the point of the change, and the reason it lands separately from the #35 fix rather than bundled into it.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GR33TA8BuP9NAKh3PALrVc