From b6b617f3cfe40c47b26818f3c2bc45f7e7cd52ef Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 23 Oct 2026 15:22:55 +0200 Subject: [PATCH 1/2] Fix Windows clippy failure in quarto-mcp-launcher non-UTF-8 path test (bd-j0pgiips) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit q2_path_injection_skips_non_utf8_exe_paths built its non-UTF-8 OsString via the unix-only OsStringExt::from_vec, so `cargo clippy -p quarto-mcp-launcher --all-targets -- -D warnings` (a cargo xtask verify step) failed on Windows with E0433/E0599, and the crate's whole test target was un-runnable there — masking 4 more Windows failures in cache_tests (filed as bd-5tdzaf3t). Per .claude/rules/cross-platform.md, keep the test portable: a cfg pair of non_utf8_os_string() helpers — from_vec(0xff, 0xfe) on unix, from_wide(lone surrogate) on Windows — so the same q2_path_for_child rejection path is exercised on all three platforms. Verified on Windows: clippy -D warnings green; 27/27 lib tests pass. --- crates/quarto-mcp-launcher/src/lib.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/crates/quarto-mcp-launcher/src/lib.rs b/crates/quarto-mcp-launcher/src/lib.rs index 1d2e59fff..8acdf956d 100644 --- a/crates/quarto-mcp-launcher/src/lib.rs +++ b/crates/quarto-mcp-launcher/src/lib.rs @@ -504,10 +504,26 @@ mod tests { ); } + /// Building a non-UTF-8 `OsString` is platform-specific, so the + /// fixture is a `cfg` pair and the test itself stays portable + /// (`.claude/rules/cross-platform.md`; bd-j0pgiips). + #[cfg(unix)] + fn non_utf8_os_string() -> std::ffi::OsString { + use std::os::unix::ffi::OsStringExt; + std::ffi::OsString::from_vec(vec![0xff, 0xfe]) + } + + /// A lone surrogate is ill-formed UTF-16, so `to_str()` returns + /// `None` for it just as it does for non-UTF-8 bytes on unix. + #[cfg(windows)] + fn non_utf8_os_string() -> std::ffi::OsString { + use std::os::windows::ffi::OsStringExt; + std::ffi::OsString::from_wide(&[0xD800]) + } + #[test] fn q2_path_injection_skips_non_utf8_exe_paths() { - use std::os::unix::ffi::OsStringExt; - let weird = std::ffi::OsString::from_vec(vec![0xff, 0xfe]); + let weird = non_utf8_os_string(); assert_eq!(q2_path_for_child(None, Some(PathBuf::from(weird))), None); } } From e0ed1c3555f06532bf82a13729ba4721478f93d2 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 23 Oct 2026 15:28:59 +0200 Subject: [PATCH 2/2] Docs: cross-check recipe for Windows compilation before landing (bd-tkubuse4) Windows is not in CI, and each feature PR kept breaking it because neither .claude/rules/cross-platform.md nor claude-notes/instructions/windows-dev.md told contributors HOW to verify a Windows build without a Windows machine. - cross-platform.md: new 'Verify before landing' section with the recipe: cargo clippy -p --all-targets --target x86_64-pc-windows-msvc -- -D warnings (check/clippy don't link, so no MSVC toolchain needed on unix), plus the C-build-script caveat and the compilation-only scope limit. - windows-dev.md: CI section now points at the same recipe as the pre-landing gate. - AGENTS.md: replaced the stale 'crates must be manually excluded from tests' testing bullet (resolved 2026-03-23) with the cross-check rule. Recipe validated on Windows: against the pre-fix bd-j0pgiips code it reproduces the exact E0433/E0599 clippy failure; against the fixed code it is clean. NOTE: validated natively on Windows, not cross from unix; the no-link claim is standard cargo behavior but the first unix run may surface per-crate build-script quirks. --- .claude/rules/cross-platform.md | 16 ++++++++++++++++ AGENTS.md | 2 +- claude-notes/instructions/windows-dev.md | 13 +++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/.claude/rules/cross-platform.md b/.claude/rules/cross-platform.md index f9f0c47ec..f316b158f 100644 --- a/.claude/rules/cross-platform.md +++ b/.claude/rules/cross-platform.md @@ -21,6 +21,22 @@ When the gated code uses no platform-specific API and only *running* it is platf When gated code changes, update the comment that explains why the gate is needed. A stale explanation can lead someone to widen or copy an unnecessary gate. +## Verify before landing + +Windows is not in CI, so the person (or agent) writing platform-specific code is the only line of defense. `cargo check` and `cargo clippy` do not link, so you can type-check the Windows build from any OS without an MSVC toolchain: + +```bash +rustup target add x86_64-pc-windows-msvc # once +cargo clippy -p --all-targets --target x86_64-pc-windows-msvc -- -D warnings +``` + +`--all-targets` matters: the breakage is usually in tests (bd-j0pgiips, bd-lhi82470). This catches `std::os::unix` misuse (E0433/E0599) and, under `-D warnings`, cfg-gated dead code. Run it for every crate you touched; run the same command without `--target` for the host platform. + +Caveats: + +- Crates whose build scripts compile C for the target (e.g. `ring`, `v8`) may not cross-check from unix. If a crate you touched can't cross-check, say so in the PR so a Windows user can run the native check: `cargo clippy -p --all-targets -- -D warnings`. +- Cross-checking proves compilation only. Runtime behavior (file locking, line endings, process signals) still needs a Windows run — flag tests that exercise those for manual verification. + ## File paths - Use `std::path::Path`/`PathBuf`, never hardcode `/` or `\` separators diff --git a/AGENTS.md b/AGENTS.md index 330303038..957a9161a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -422,7 +422,7 @@ The changelog is rendered in the About section of the hub-client UI. - **CRITICAL**: If you'll be writing tests, read the special instructions on file claude-notes/instructions/testing.md - **CRITICAL**: For hub-client changes, passing tests alone is NOT sufficient. You must also verify that `npm run build:all` (from `hub-client/`) succeeds before claiming work is done. The production build (`tsc -b && vite build`) is stricter than `tsc --noEmit` and `vitest` — it uses project references mode and catches errors the other tools miss. - **CRITICAL**: For any CLI- or user-visible feature, passing tests alone is NOT sufficient. See **End-to-end verification before declaring success** below. -- **Windows**: Some crates must be manually excluded from tests. See claude-notes/instructions/windows-dev.md for details. +- **Windows**: Windows is not in CI. When your change uses platform-specific APIs (`std::os::unix`/`std::os::windows`), adds `#[cfg(...)]` gates, or touches test helpers, cross-check before landing: `cargo clippy -p --all-targets --target x86_64-pc-windows-msvc -- -D warnings` (works from any OS; no linking). See `.claude/rules/cross-platform.md` and claude-notes/instructions/windows-dev.md. ## End-to-end verification before declaring success diff --git a/claude-notes/instructions/windows-dev.md b/claude-notes/instructions/windows-dev.md index a6c32fc7a..4e638945a 100644 --- a/claude-notes/instructions/windows-dev.md +++ b/claude-notes/instructions/windows-dev.md @@ -36,3 +36,16 @@ significantly. ## CI Windows is not currently in the CI matrix. All Windows testing is manual. + +Until a Windows leg exists, the pre-landing gate is a cross-check, which works +from any OS because `cargo check`/`cargo clippy` do not link: + +```bash +rustup target add x86_64-pc-windows-msvc # once +cargo clippy -p --all-targets --target x86_64-pc-windows-msvc -- -D warnings +``` + +Run it for every crate you touch when your change uses `std::os::unix` / +`std::os::windows`, adds `#[cfg(...)]` gates, or touches test helpers. The full +rules and patterns are in `.claude/rules/cross-platform.md`; failure history: +bd-j0pgiips, bd-lhi82470, bd-5tdzaf3t.