diff --git a/.gitattributes b/.gitattributes index 60c3b54..27016d2 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,6 +1,6 @@ # Check every text file out with LF on all platforms. Generated artifacts embed the bytes of # descriptors, profiles, and docs verbatim, and the byte-exact tests compare against LF fixtures, so -# a CRLF working tree on Windows silently changes program output. +# line-ending conversion would silently change program output. * text=auto eol=lf # Golden fixtures are byte-exact — never EOL-normalize them. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 191bf69..7e7f4e4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,13 +16,7 @@ env: jobs: test: name: Test suite - strategy: - # Windows is the platform this matrix exists to cover, so a Linux failure - # must not cancel it — that is exactly when its result is worth having. - fail-fast: false - matrix: - os: [ubuntu-latest, windows-latest] - runs-on: ${{ matrix.os }} + runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - name: Install Rust toolchain @@ -31,33 +25,14 @@ jobs: components: rustfmt, clippy - name: Cache cargo build uses: Swatinem/rust-cache@v2 - - name: Install jq - # Git for Windows supplies sh, xargs, tr, and wc, but not jq, and the - # judge-recipe tests execute the shipped pipeline text rather than a - # stand-in for it. - if: runner.os == 'Windows' - run: choco install jq --yes --no-progress - - name: Permit symlink creation - # Windows creates symlinks only under Developer Mode or elevation, and - # the core::fs round-trips need one. Asking for it explicitly beats - # depending on how the runner's token happens to be built. - if: runner.os == 'Windows' - run: > - reg add "HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock" - /t REG_DWORD /f /v AllowDevelopmentWithoutDevLicense /d 1 - name: Format check run: cargo fmt --all -- --check - name: Clippy run: cargo clippy --all-targets --all-features -- -D warnings - name: Test - # Capability-gated tests (the POSIX recipe pipelines, symlink - # round-trips, long-path staging) skip with a printed reason on a host - # that lacks the capability. This turns every such skip into a failure, - # so neither runner can quietly stop covering them. Ubuntu ships the - # recipe tools; the steps above provide them on Windows. Long paths need - # no provisioning — the runner passes core.longpaths to git itself. - # EVAL_MAGIC_SH stays unset deliberately: discovering the shell from the - # Git install root is what a Windows user hits, so CI should run it too. + # Capability-gated tests skip with a printed reason on a host that lacks + # the capability. CI turns every such skip into a failure so coverage + # cannot shrink silently. env: EVAL_MAGIC_REQUIRE_POSIX_TOOLS: 1 run: cargo test --all-targets diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6ea2445..1f4be2b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -113,9 +113,6 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} BUILD_MANIFEST_NAME: target/distrib/${{ join(matrix.targets, '-') }}-dist-manifest.json steps: - - name: enable windows longpaths - run: | - git config --global core.longpaths true - uses: actions/checkout@v6 with: persist-credentials: false @@ -146,9 +143,7 @@ jobs: echo "dist ran successfully" - id: cargo-dist name: Post-build - # We force bash here just because github makes it really hard to get values up - # to "real" actions without writing to env-vars, and writing to env-vars has - # inconsistent syntax between shell and powershell. + # Force bash so every release runner writes GitHub outputs with one syntax. shell: bash run: | # Parse out what we just built and upload it to scratch storage diff --git a/AGENTS.md b/AGENTS.md index c92f4d2..5ecdd4d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,40 +54,35 @@ Extraction is a size decision, not a style preference; don't split a small inlin **Spawning a child process from a test.** Use the hidden `__fixture` subcommand, never `sh`, `true`, `printf`, or a `#!/bin/sh` stub. It exits with a chosen code, emits chosen bytes, writes a chosen file, or checks a file or variable — see `FixtureArgs` in `src/cli/args.rs`. One invocation parses -the same under `sh -c` and `cmd /C`, which is what keeps `command_check` tests off per-OS command -strings. Build the command with the `fixture` helper (`tests/run/helpers.rs` for integration tests, -the one in `src/pipeline/grade/command_check/tests.rs` for unit tests). Because the fixture is the -binary, `cargo test --lib` alone does not build it — run `cargo test`, or `cargo build` first. - -**Tests are gated on capabilities, not on the OS.** `#[cfg(unix)]` on a test hides it from -compilation and clippy on the other host and hides the coverage gap. Instead, probe for what the -test actually needs and call `report_skip` (`src/core/runtime.rs`), which prints the reason and -returns `true`. Setting `EVAL_MAGIC_REQUIRE_POSIX_TOOLS=1` turns every skip into a failure; CI sets -it on both runners, so neither can quietly stop covering something. Two capabilities are gated -today: symlink creation, which Windows allows only under Developer Mode, and creating a path past -Windows' 259-character limit (`deep_task_root`, `src/cli/run/orchestrate/git.rs`). The Windows -runner is provisioned for both rather than exempted from them, so a skip there is a red build. The -shell is not one of them; it is a hard requirement, per the section below. Where a genuine per-OS -difference is the behavior under test — signals, path separators — branch on `cfg!(windows)` at -runtime so both arms still compile everywhere. +predictably under `sh -c`, which keeps `command_check` tests focused on runner behavior instead of +the host's utility implementations. Build the command with the `fixture` helper +(`tests/run/helpers.rs` for integration tests, the one in +`src/pipeline/grade/command_check/tests.rs` for unit tests). Because the fixture is the binary, +`cargo test --lib` alone does not build it — run `cargo test`, or `cargo build` first. + +**Tests are gated on capabilities, not on broad platform labels.** Probe for what the test actually +needs and call `report_skip` (`src/core/runtime.rs`), which prints the reason and returns `true`. +Setting `EVAL_MAGIC_REQUIRE_POSIX_TOOLS=1` turns every skip into a failure; CI sets it so coverage +cannot quietly shrink. Symlink creation is capability-gated because the backing filesystem may +forbid it. The shell is not capability-gated; it is a hard requirement, per the section below. **A POSIX shell is required, for use and for development.** Harness `exec_template`s are POSIX command lines, so the dispatch and probe paths spawn `sh` through `run_in_posix_shell` / `posix_shell()` (`src/core/runtime.rs`) rather than a hardcoded `/bin/sh`: it searches `PATH`, then -a Git for Windows install. Set `EVAL_MAGIC_SH` to override it. `cargo test` inherits the -requirement — the dispatch tests spawn a `#!/bin/sh` harness stub through the resolved shell and do -not skip — so a host without `sh` fails the suite instead of quietly covering less. The shell is -the whole requirement: `jq` was needed only while operators pasted the generated dispatch and judge -recipes, and `eval-magic dispatch` drives both itself. +checks `/bin/sh`. Set `EVAL_MAGIC_SH` to override it. `cargo test` inherits the requirement — the +dispatch tests spawn a `#!/bin/sh` harness stub through the resolved shell and do not skip — so a +host without `sh` fails the suite instead of quietly covering less. The shell is the whole +requirement: `jq` was needed only while operators pasted the generated dispatch and judge recipes, +and `eval-magic dispatch` drives both itself. `POSIX_TOOLING_REQUIREMENT` (`src/core/runtime.rs`) is the one wording the Markdown-carrying surfaces reuse: the shell-discovery errors, the `run` preflight warnings, `RUNBOOK.md`, and `dispatch-manifest.md`. State the requirement from there rather than rephrasing it. `--help` is the one deliberate restatement (`AFTER_HELP` in `src/cli/help.rs`), hard-wrapped and backtick-free because clap renders into a terminal; keep the two in step by hand. -Which platforms that requirement is honored on — and why preparing on Windows but dispatching from -WSL is a correctness boundary rather than a preference — is stated once under "Platform support" in -`docs/developer_overview.md`. +eval-magic supports Linux and macOS. On Windows, use and develop eval-magic entirely inside WSL; +native Windows is unsupported. The complete boundary and the portable-data exception are stated +under "Platform support" in `docs/developer_overview.md`. **Where user-facing warnings come from.** Library modules (`pipeline`, `workspace`, `sandbox`, `adapters`) never print. They return warning strings on their result struct — `#[serde(skip)]` when diff --git a/README.md b/README.md index eef601b..e2ee34a 100644 --- a/README.md +++ b/README.md @@ -37,31 +37,22 @@ The installed CLI is the primary manual. Start with `eval-magic --help`, and use ## Install -Git is required at runtime, plus a POSIX shell: harness dispatch commands are POSIX command lines, -and `eval-magic dispatch` runs them itself, so the host it runs on needs a shell that resolves the -workspace's own paths. On Windows that is Git Bash (Git for Windows). WSL resolves a different -filesystem namespace, so run eval-magic inside WSL rather than dispatching into it. -Set `EVAL_MAGIC_SH` to select a specific `sh`. +eval-magic supports Linux and macOS. On Windows, install and run eval-magic inside Windows +Subsystem for Linux (WSL); native Windows is unsupported. Keep the repository, workspace, and +harness commands inside the same WSL environment. -Windows support runs through Git Bash and is deprecated: a future release will require WSL. +Git and a POSIX shell are required. Set `EVAL_MAGIC_SH` to select a specific `sh`. -Prebuilt binaries for macOS, Linux, and Windows are attached to each +Prebuilt binaries for macOS and Linux are attached to each [GitHub release](https://github.com/slowdini/eval-magic/releases). -macOS or Linux: +Install on macOS, Linux, or inside WSL: ```bash curl --proto '=https' --tlsv1.2 -LsSf \ https://github.com/slowdini/eval-magic/releases/latest/download/eval-magic-installer.sh | sh ``` -Windows PowerShell: - -```powershell -powershell -ExecutionPolicy Bypass -c \ - "irm https://github.com/slowdini/eval-magic/releases/latest/download/eval-magic-installer.ps1 | iex" -``` - Or build and install from crates.io: ```bash @@ -151,9 +142,11 @@ Issues and planned work are tracked in the ## Development -Development carries the same host requirement as use: a POSIX shell. The dispatch tests spawn -`#!/bin/sh` harness stubs through the resolved shell and do not skip, so the suite cannot pass -without one. Tests that need symlink creation report a skip instead. +Development carries the same host requirement as use: Linux or macOS with a POSIX shell. On +Windows, clone the repository and run the complete toolchain inside WSL; native Windows development +is unsupported. The dispatch tests spawn `#!/bin/sh` harness stubs through the resolved shell and +do not skip, so the suite cannot pass without one. Tests that need symlink creation report a skip +instead. ```bash cargo fmt --check diff --git a/dist-workspace.toml b/dist-workspace.toml index 9d1acb2..c87d3ea 100644 --- a/dist-workspace.toml +++ b/dist-workspace.toml @@ -7,10 +7,13 @@ members = ["cargo:."] cargo-dist-version = "0.32.0" # CI backends to support ci = "github" +# cargo-dist adds native Windows setup even when no Windows targets remain. +# Keep the Linux/macOS-only workflow as an intentional local customization. +allow-dirty = ["ci"] # The installers to generate for each app -installers = ["shell", "powershell"] +installers = ["shell"] # Target platforms to build apps for (Rust target-triple syntax) -targets = ["aarch64-apple-darwin", "aarch64-unknown-linux-gnu", "x86_64-apple-darwin", "x86_64-unknown-linux-gnu", "x86_64-pc-windows-msvc"] +targets = ["aarch64-apple-darwin", "aarch64-unknown-linux-gnu", "x86_64-apple-darwin", "x86_64-unknown-linux-gnu"] # Path that installers should place binaries in install-path = "CARGO_HOME" # Where to host releases diff --git a/docs/developer_overview.md b/docs/developer_overview.md index 93e5f0a..c502020 100644 --- a/docs/developer_overview.md +++ b/docs/developer_overview.md @@ -75,23 +75,19 @@ following authorities: | Tier | Platform | Verified by | | --- | --- | --- | -| Supported | Linux, macOS | the `ubuntu-latest` CI job | -| Deprecated | Windows, through Git Bash (Git for Windows) | the `windows-latest` CI job | -| Unsupported | preparing a workspace on Windows and dispatching it from WSL | — | - -Windows support is deprecated in favor of WSL. #256 has landed, so the recipe surface that carried -the largest Windows accommodation is gone; the remaining removal — the `cfg(windows)` sites, the CI -leg, and the msvc target — is #275. Until that lands, the Windows runner stays green and -Windows-native behavior is held to the same bar as any other platform: a Windows failure is a real -failure, not an accepted gap. Do not add new Windows-native accommodation in the meantime. - -The unsupported row is a correctness boundary rather than a preference. `dispatch` spawns each -harness command line with the workspace's own absolute paths, so the shell it resolves has to -resolve those. Git Bash shares the Windows filesystem, so those paths resolve; WSL resolves its own -namespace, where a `C:\…` path names nothing. Nothing in the tree translates between the two, so -the split fails quietly instead of loudly. `POSIX_TOOLING_REQUIREMENT` (`src/core/runtime.rs`) is -the single wording every user-facing surface reuses to state this; `src/cli/help.rs` restates it -for clap by hand. +| Supported | Linux, macOS, Linux inside WSL | the `ubuntu-latest` CI job | +| Unsupported | native Windows | — | + +Windows users run the Linux build inside Windows Subsystem for Linux (WSL). Keep the binary, +repository, eval workspaces, and harness processes inside the same WSL environment. `dispatch` +passes workspace-owned absolute paths to harness command lines, so crossing from a native Windows +process into WSL would change the filesystem namespace and invalidate those paths. + +Do not add native Windows accommodations or release targets. Preserve support for Windows-shaped +paths only where they are data read from artifacts or transcripts; those portable-data contracts +do not imply native Windows runtime support. `POSIX_TOOLING_REQUIREMENT` (`src/core/runtime.rs`) is +the single wording every user-facing Markdown surface reuses. `src/cli/help.rs` restates it for +clap by hand. ## Make and verify a change @@ -100,11 +96,11 @@ editing. Add a focused failing test at the narrowest useful boundary, implement run the focused test again. Cross-harness changes belong at shared descriptor, runner, or adapter boundaries unless the evidence requires a named harness capability. -Development carries the host requirement the tool itself declares: a POSIX shell. The dispatch +Development requires Linux or macOS with a POSIX shell. Windows contributors clone the repository +and run the complete toolchain inside WSL; native Windows development is unsupported. The dispatch tests spawn `#!/bin/sh` harness stubs through the resolved shell and do not skip, so the suite -cannot pass without one. Tests needing symlink creation or a path past Windows' 259-character limit -report a skip instead; `EVAL_MAGIC_REQUIRE_POSIX_TOOLS=1` turns those skips into failures, as CI -sets it to do on both its Ubuntu and its Windows runner. +cannot pass without one. Tests needing symlink creation report a skip instead; +`EVAL_MAGIC_REQUIRE_POSIX_TOOLS=1` turns those skips into failures in CI. Before handing work off, run: diff --git a/schema/evals.schema.json b/schema/evals.schema.json index c0b5d17..877aee0 100644 --- a/schema/evals.schema.json +++ b/schema/evals.schema.json @@ -220,7 +220,7 @@ "command": { "type": "string", "minLength": 1, - "description": "Trusted eval-author command executed by the runner in the task environment after agent dispatch." + "description": "Trusted eval-author POSIX shell command executed by the runner with `sh -c` in the task environment after agent dispatch." }, "env": { "type": "object", diff --git a/src/adapters/skill_shadow.rs b/src/adapters/skill_shadow.rs index 7da6374..d0da73a 100644 --- a/src/adapters/skill_shadow.rs +++ b/src/adapters/skill_shadow.rs @@ -288,9 +288,8 @@ impl ShadowSource { } } -/// The resolved real path, rendered as wire format. `canonicalize` returns a -/// verbatim (`\\?\`) path on Windows, which `artifact_path` strips — an OS -/// escape hatch has no business in a report an agent and a reviewer both read. +/// The resolved real path, rendered in the artifact wire format shared by +/// agents and reviewers. fn canonical_path(path: &Path) -> Option { path.canonicalize().ok().map(|path| artifact_path(&path)) } diff --git a/src/cli/args.rs b/src/cli/args.rs index 4683dfd..9d5ce0a 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -832,8 +832,8 @@ pub(crate) enum Commands { }, /// Internal test fixture. A predictable child process for the suite to /// spawn — one that exits with a chosen code, emits chosen bytes, or writes - /// a chosen file — so tests never reach for `sh`, `true`, or `printf`, none - /// of which exist under `cmd.exe`. Not for users; hidden from help. + /// a chosen file — so tests do not depend on the output conventions of + /// utilities such as `true` or `printf`. Not for users; hidden from help. #[command(hide = true, name = "__fixture")] Fixture(FixtureArgs), /// Internal generic PreToolUse hook entry point. Invoked by the installed @@ -882,8 +882,7 @@ pub struct FixtureArgs { #[arg(long)] pub pad: Option, /// Sleep this many milliseconds before doing anything else, so a caller can - /// overrun a deadline. The delay lives here rather than in a `sleep` call - /// because Windows has no such binary. + /// overrun a deadline without depending on an external `sleep` binary. #[arg(long = "sleep-ms")] pub sleep_ms: Option, /// Joins the fragments. Empty by default. diff --git a/src/cli/commands/fixture.rs b/src/cli/commands/fixture.rs index e600090..7f53bb8 100644 --- a/src/cli/commands/fixture.rs +++ b/src/cli/commands/fixture.rs @@ -3,10 +3,9 @@ //! //! Tests that exercise `command_check` grading need a program that exits with a //! chosen status, emits chosen bytes, or writes a chosen file. Reaching for -//! `sh`, `true`, or `printf` ties those tests to POSIX, and the `cmd.exe` -//! equivalents are not equivalent — `echo x>>f` appends CRLF, and -//! `echo|set /p=` cannot round-trip a value. One fixture invoked the same way -//! under both shells removes the dialect problem entirely. +//! external utilities such as `true`, `printf`, or `sleep` would make their +//! platform-specific output and availability part of the test. The fixture +//! keeps those effects predictable. use std::fs::{self, OpenOptions}; use std::io::{self, Write}; @@ -160,9 +159,8 @@ mod tests { } /// `--sleep-ms` delays the fixture before it does anything else, which is - /// what lets a dispatch-timeout test overrun a deadline on any host. `sleep` - /// is a POSIX binary Windows lacks, so the delay has to live in the fixture - /// itself. + /// what lets a dispatch-timeout test overrun a deadline without depending + /// on an external `sleep` binary. #[test] fn sleep_ms_delays_the_fixture_before_it_emits() { let started = std::time::Instant::now(); diff --git a/src/cli/help.rs b/src/cli/help.rs index 7c14544..2bed3ac 100644 --- a/src/cli/help.rs +++ b/src/cli/help.rs @@ -8,12 +8,10 @@ /// Worked examples shown at the end of `eval-magic --help`. pub(super) const AFTER_HELP: &str = "\ REQUIREMENTS: - Git, plus a POSIX shell. Harness dispatch commands are POSIX command - lines, and eval-magic dispatch runs them itself, so the host it runs on - needs a shell that resolves the workspace's own paths. On Windows that is - Git Bash (Git for Windows). WSL resolves a different filesystem namespace, - so run eval-magic inside WSL rather than dispatching into it. Set - EVAL_MAGIC_SH to select a specific sh. + eval-magic supports Linux and macOS. On Windows, install and run eval-magic + inside WSL; native Windows is unsupported. Keep the repository, workspace, + and harness commands inside the same WSL environment. Git and a POSIX shell + are required. Set EVAL_MAGIC_SH to select a specific sh. EXAMPLES: # Scaffold a first eval and prepare its isolated comparison environments diff --git a/src/cli/run/orchestrate/git.rs b/src/cli/run/orchestrate/git.rs index a3d98b3..b46a106 100644 --- a/src/cli/run/orchestrate/git.rs +++ b/src/cli/run/orchestrate/git.rs @@ -18,15 +18,6 @@ const BASELINE_NAME: &str = "eval-magic"; const BASELINE_EMAIL: &str = "eval-magic@localhost"; const BASELINE_DATE: &str = "2000-01-01T00:00:00Z"; -/// Windows' `MAX_PATH` (260) counts the terminating NUL, so 259 characters are -/// what a tool that is not long-path aware can actually use. -const WINDOWS_USABLE_PATH: usize = 259; - -/// Length of what a run writes below a task root before its deepest file, -/// `\.claude\skills\\SKILL.md`: 68 characters for a short slug, 85 -/// for a long skill and condition pair, rounded up. -const STAGED_SUFFIX_BUDGET: usize = 96; - pub(super) fn preflight_git(ctx: &RunContext) -> Result<(), RunError> { let output = run_git(&["--version"], &ctx.skill_subdir); if output.status == Some(0) { @@ -64,11 +55,8 @@ pub(super) fn initialize_task_repositories( forced_paths: runner_placed_paths(ctx, resolved, &target)?, }; initialize_task_repository(&plan).map_err(|error| { - let hint = path_budget_hint(&target.root, cfg!(windows)) - .map(|hint| format!("\n{hint}")) - .unwrap_or_default(); RunError::msg(format!( - "could not initialize task Git repository at {}: {error}{hint}", + "could not initialize task Git repository at {}: {error}", target.root.display() )) })?; @@ -121,24 +109,6 @@ struct TaskRepository { forced_paths: Vec, } -/// A sentence naming the Windows path budget, for a task root too deep to hold -/// what a run stages below it. -/// -/// Measures the root rather than matching git's `Filename too long`, which is a -/// localizable `strerror` mapping. -fn path_budget_hint(root: &Path, windows: bool) -> Option { - let length = root.as_os_str().to_string_lossy().chars().count(); - if !windows || length + STAGED_SUFFIX_BUDGET <= WINDOWS_USABLE_PATH { - return None; - } - Some(format!( - "This task root is {length} characters and a run stages roughly \ - {STAGED_SUFFIX_BUDGET} more below it, past the {WINDOWS_USABLE_PATH} Windows \ - allows a tool that is not long-path aware. If the failure above names a path \ - or filename length, re-run from a shorter workspace root." - )) -} - fn initialize_task_repository(plan: &TaskRepository) -> Result<(), String> { let root = plan.root.as_path(); let git = IsolatedGit::new()?; @@ -181,12 +151,6 @@ fn initialize_task_repository(plan: &TaskRepository) -> Result<(), String> { ("commit.gpgSign", "false"), ("tag.gpgSign", "false"), ("core.hooksPath", hooks_path.as_str()), - // Lifts Windows' `MAX_PATH`, which a staged skill under a deep workspace - // crosses. Task repositories run under isolated Git configuration, so an - // operator's own setting never reaches one. Written to the repository, - // not per invocation, so the agent under test and the pipeline inherit - // it; git ignores the key off Windows. - ("core.longpaths", "true"), ] { run_checked(&git, root, &["config", "--local", name, value], &[])?; } @@ -345,180 +309,3 @@ fn git_diagnostic(status: Option, stderr: &[u8]) -> String { (None, true) => "could not start git".to_string(), } } - -#[cfg(test)] -mod tests { - use super::*; - - use crate::core::runtime::report_skip; - - /// A repository with no codebase behind it — the shape these path-budget - /// tests exercise, and what a fixture-only run has always produced. - fn fixture_only(root: &Path) -> TaskRepository { - TaskRepository { - root: root.to_path_buf(), - sourced: false, - branch: INITIALIZED_BRANCH.to_string(), - forced_paths: Vec::new(), - } - } - - /// A staged skill's path relative to its task root: 68 characters, the - /// shortest realistic shape of `.claude/skills//SKILL.md`. - const STAGED_SKILL: &str = - ".claude/skills/slow-powers-eval-1-with_skill__widget-skill/SKILL.md"; - - /// `base` extended with padding components until it is `target` characters - /// long (or left as it is, when it is already longer). - fn padded_to(base: &Path, target: usize) -> PathBuf { - const PAD: &str = "eval-magic-path-budget-padding"; - let mut root = base.to_path_buf(); - while root.as_os_str().len() + 1 + PAD.len() <= target { - root = root.join(PAD); - } - let remaining = target.saturating_sub(root.as_os_str().len() + 1); - if remaining > 0 { - root = root.join(&PAD[..remaining]); - } - root - } - - /// `base` spelled the way git will report it. - /// - /// `std::env::temp_dir()` can hand back an 8.3 short name — `RUNNER~1` for - /// `runneradmin` on a GitHub runner — which git expands before it measures. - /// Those three characters are invisible to a length computed from the short - /// spelling, and three is enough to push `.git/config` past the limit on a - /// host where the same target fits locally. Measure what git measures. - fn long_form(base: &Path) -> PathBuf { - let Ok(canonical) = base.canonicalize() else { - return base.to_path_buf(); - }; - let text = canonical.to_string_lossy().into_owned(); - // Canonicalising on Windows yields a `\\?\` verbatim path; git reports - // the plain spelling, so drop the prefix to keep the two comparable. - PathBuf::from(text.strip_prefix(r"\\?\").unwrap_or(&text)) - } - - /// A `target`-character task root holding a staged `SKILL.md`, or `None` - /// when this host cannot write that deep. The probe is the same `std::fs` - /// write staging performs, so the gate is the capability, not the OS. - fn deep_task_root(base: &Path, target: usize, test: &str) -> Option { - let root = padded_to(&long_form(base), target); - let staged = root.join(STAGED_SKILL); - let written = fs::create_dir_all(staged.parent().expect("the staged path has a parent")) - .and_then(|()| fs::write(&staged, "---\nname: widget-skill\n---\n\nbody\n")); - if let Err(error) = written { - report_skip( - test, - &format!( - "this host cannot create a {}-character path ({error})", - staged.as_os_str().len() - ), - ); - return None; - } - Some(root) - } - - /// Rust's filesystem calls pass verbatim paths, so a deep workspace stages - /// its skill fine and only git meets Windows' `MAX_PATH` — the baseline - /// `git add` aborts with `Filename too long`. - #[test] - fn task_repository_initializes_when_the_staged_skill_exceeds_the_windows_path_limit() { - let test = - "task_repository_initializes_when_the_staged_skill_exceeds_the_windows_path_limit"; - let tmp = tempfile::TempDir::new().unwrap(); - // 195 characters puts the staged path past the budget while the - // repository's own `.git` bookkeeping stays under it. - let Some(root) = deep_task_root(tmp.path(), 195, test) else { - return; - }; - assert!( - root.join(STAGED_SKILL).as_os_str().len() > WINDOWS_USABLE_PATH, - "the fixture must exceed the Windows path budget to exercise anything" - ); - initialize_task_repository(&fixture_only(&root)) - .expect("a task root with a deep staged skill initializes"); - } - - /// A failure under a deep root has to name the path budget: git reports - /// `Filename too long` about one file, which says nothing about the - /// workspace root being the thing to shorten. - #[test] - fn path_budget_hint_names_the_budget_for_a_deep_windows_root() { - let root = padded_to(Path::new("C:/w"), 210); - let hint = path_budget_hint(&root, true).expect("a deep Windows root gets a hint"); - assert!(hint.contains("210"), "{hint}"); - assert!(hint.contains(&WINDOWS_USABLE_PATH.to_string()), "{hint}"); - assert!(hint.contains("shorter workspace root"), "{hint}"); - } - - /// The hint is a Windows path-budget explanation, so it stays out of the way - /// of every failure it cannot explain. - #[test] - fn path_budget_hint_stays_silent_off_windows_and_for_short_roots() { - let deep = padded_to(Path::new("C:/w"), 210); - assert_eq!(path_budget_hint(&deep, false), None); - assert_eq!(path_budget_hint(Path::new("C:/w/iteration-1"), true), None); - } - - /// Past a certain depth the failure goes quiet: git cannot open the staged - /// directory to enumerate it, so `git add` warns, exits zero, and leaves the - /// skill under test out of the baseline that later diffs are measured - /// against. Nothing downstream can flag a file git could not read. - #[test] - fn task_repository_baseline_tracks_a_staged_skill_past_the_windows_path_limit() { - let test = "task_repository_baseline_tracks_a_staged_skill_past_the_windows_path_limit"; - let tmp = tempfile::TempDir::new().unwrap(); - // 202 characters isolates the quiet mode: enumerating the staged - // directory needs 261, past the budget, while the repository's own loose - // objects still fit at 256. - let Some(root) = deep_task_root(tmp.path(), 202, test) else { - return; - }; - initialize_task_repository(&fixture_only(&root)) - .expect("a task root in the quiet band initializes"); - let tracked = run_git(&["ls-files"], &root); - assert!( - String::from_utf8_lossy(&tracked.stdout).contains("SKILL.md"), - "the baseline commit must track the staged skill, not skip it" - ); - } - - /// A root deep enough that `.git/objects/pack` crosses the budget: `git - /// init` creates it before any repository-local configuration exists, so the - /// lift has to reach that invocation too. - /// - /// 244 is not arbitrary and not the maximum. Git's long-path awareness is - /// per-operation: creating `.git/objects/pack` survives well past the - /// budget, `git init` writing `.git/config` stops at exactly - /// `WINDOWS_USABLE_PATH`, and the `git config --local` that follows gives up - /// two characters earlier still. 244 puts `pack` at 262 — past the budget, - /// which is the point — while leaving `.git/config` at 256, a deliberate - /// three inside the tightest of those ceilings. The assertions below pin the - /// window so a future edit cannot silently slide the root out of it. - #[test] - fn task_repository_initializes_when_its_git_directory_exceeds_the_windows_path_limit() { - const GIT_CONFIG: &str = ".git/config"; - const GIT_PACK: &str = ".git/objects/pack"; - let test = - "task_repository_initializes_when_its_git_directory_exceeds_the_windows_path_limit"; - let tmp = tempfile::TempDir::new().unwrap(); - let Some(root) = deep_task_root(tmp.path(), 244, test) else { - return; - }; - - let length = root.as_os_str().len(); - assert!( - length + 1 + GIT_PACK.len() > WINDOWS_USABLE_PATH, - "{length}-character root leaves `.git/objects/pack` inside the budget, testing nothing" - ); - assert!( - length + 1 + GIT_CONFIG.len() + 3 <= WINDOWS_USABLE_PATH, - "{length}-character root leaves `.git/config` no margin below the budget" - ); - initialize_task_repository(&fixture_only(&root)) - .expect("a task root deeper than `.git` needs initializes"); - } -} diff --git a/src/cli/run/orchestrate/shell.rs b/src/cli/run/orchestrate/shell.rs index 45598ab..685f61d 100644 --- a/src/cli/run/orchestrate/shell.rs +++ b/src/cli/run/orchestrate/shell.rs @@ -8,8 +8,9 @@ //! //! The gap is host-local: `dispatch` spawns each harness command line with the //! workspace's own absolute paths, so the shell it resolves has to resolve -//! those. Git Bash shares the Windows filesystem; WSL resolves its own. See -//! [`POSIX_TOOLING_REQUIREMENT`] for the declared rule the warning defers to. +//! those. Windows users keep preparation and dispatch inside the same WSL +//! environment. See [`POSIX_TOOLING_REQUIREMENT`] for the declared rule the +//! warning defers to. use std::path::Path; @@ -51,18 +52,24 @@ mod tests { /// workspace is still correct. #[test] fn a_missing_shell_warns_with_the_declared_requirement() { - let warning = tooling_warning(Err("no POSIX shell found. Use Git Bash or WSL.")) + let warning = tooling_warning(Err( + "no POSIX shell found. On Windows, run eval-magic inside WSL; native Windows is unsupported.", + )) .expect("a host with no POSIX shell must be told"); assert!(warning.contains("no POSIX shell found"), "{warning}"); - assert!(warning.contains("Git Bash"), "{warning}"); + assert!(warning.contains("WSL"), "{warning}"); + assert!( + warning.contains("native Windows is unsupported"), + "{warning}" + ); } - /// An unqualified "dispatch it from a POSIX shell" reads as an invitation to - /// prepare here and dispatch from WSL — the one split - /// [`POSIX_TOOLING_REQUIREMENT`] rules out, and the one that fails quietly. + /// An unqualified "dispatch it from a POSIX shell" could invite a caller to + /// cross filesystem namespaces. Confining it to the preparing host keeps the + /// generated absolute paths valid. #[test] fn a_missing_shell_confines_dispatch_to_the_host_that_prepared_the_workspace() { - let warning = tooling_warning(Err("no POSIX shell found. Use Git Bash.")) + let warning = tooling_warning(Err("no POSIX shell found.")) .expect("a host with no POSIX shell must be told"); assert!( warning.contains("this host"), diff --git a/src/core/context/tests.rs b/src/core/context/tests.rs index 02f3222..eb0b029 100644 --- a/src/core/context/tests.rs +++ b/src/core/context/tests.rs @@ -35,6 +35,16 @@ fn input_from(cwd: &Path) -> DetectInput { } } +fn create_symlink_or_skip(target: &Path, link: &Path, test: &str) -> bool { + match crate::core::fs::create_symlink(target, link) { + Ok(()) => false, + Err(error) => crate::core::runtime::report_skip( + test, + &format!("this filesystem does not permit symlink creation: {error}"), + ), + } +} + #[test] fn cwd_skill_dir_is_the_default_single_skill() { let tmp = TempDir::new().unwrap(); @@ -399,20 +409,25 @@ fn stage_root_default() { /// against paths the agent's own tools report — so an alias of the cwd has to /// collapse here, once, or the two sides disagree forever after. /// -/// Windows spells one directory several ways (8.3 short names, junctions, -/// `subst` drives, redirected profiles); each is one `canonicalize` apart -/// from the real path, so exercising one exercises the mechanism. +/// A symlink gives one directory two spellings; canonicalizing the run roots +/// once keeps every later comparison on the resolved spelling. #[test] fn a_cwd_alias_collapses_so_every_derived_root_shares_one_spelling() { let tmp = TempDir::new().unwrap(); let real = tmp.path().join("real-workspace"); fs::create_dir_all(&real).unwrap(); let alias = tmp.path().join("alias-workspace"); - crate::core::fs::create_directory_alias(&real, &alias).unwrap(); + if create_symlink_or_skip( + &real, + &alias, + "a_cwd_alias_collapses_so_every_derived_root_shares_one_spelling", + ) { + return; + } make_skill_dir(&real, &["foo"]); - // Enter through the alias, exactly as a user whose workspace sits under a - // junction or a redirected profile directory does. + // Enter through the alias, exactly as a user whose workspace sits below a + // symlinked directory does. let ctx = detect_run_context(DetectInput { skill: Some("foo".to_string()), ..input_from(&alias.join("skill-dir")) @@ -443,7 +458,13 @@ fn an_aliased_workspace_dir_flag_resolves_to_the_same_spelling() { let real = tmp.path().join("real-workspace"); fs::create_dir_all(&real).unwrap(); let alias = tmp.path().join("alias-workspace"); - crate::core::fs::create_directory_alias(&real, &alias).unwrap(); + if create_symlink_or_skip( + &real, + &alias, + "an_aliased_workspace_dir_flag_resolves_to_the_same_spelling", + ) { + return; + } let skill_dir = make_skill_dir(tmp.path(), &["foo"]); let ctx = detect_run_context(DetectInput { diff --git a/src/core/fs.rs b/src/core/fs.rs index 71fa8d2..213cf40 100644 --- a/src/core/fs.rs +++ b/src/core/fs.rs @@ -2,9 +2,9 @@ //! artifact path rendering, and tree copying, used by `pipeline`, `workspace`, //! `cli::run`, `adapters`, and `sandbox`. //! -//! [`artifact_path`] renders a path into the forward-slash wire format every -//! generated artifact carries; [`normalize_separators`] is its comparison-side -//! counterpart, for matching a path spelled by a different host. +//! [`artifact_path`] renders a supported-host path into the forward-slash wire +//! format every generated artifact carries; [`normalize_separators`] is its +//! comparison-side counterpart, for matching foreign path spellings in data. //! //! [`copy_entry_materialized`] is the one way to copy here, and it resolves //! symlinks into their target's content rather than mirroring them. Every @@ -27,73 +27,32 @@ use serde::Serialize; /// carries. /// /// Artifact path fields are a wire format: agents read them, downstream tools -/// join them, and the golden fixtures compare them byte for byte. `Path::join` -/// plus `Display` emits the *host's* separator, so on Windows a POSIX-rooted -/// base yields `/work/cond\run.json` — malformed for every reader. Forward -/// slashes are accepted by the Windows file APIs, so the result stays openable -/// by the stages that read these fields back. -/// -/// The rewrite is Windows-only: a POSIX filename may legally contain a literal -/// backslash, and rewriting it there would name a different file. A verbatim -/// (`\\?\`) prefix — what `Path::canonicalize` returns on Windows — is stripped -/// first, since it is an OS escape hatch rather than a path to hand an agent. +/// join them, and the golden fixtures compare them byte for byte. Supported +/// hosts use forward slashes natively. A POSIX filename may legally contain a +/// literal backslash, so rewriting one would name a different file. /// /// Not for paths handed to a process: a spawned command's argv and the guard /// hook command line must keep the host's own spelling. pub fn artifact_path(path: &Path) -> String { - let rendered = path.to_string_lossy(); - if !cfg!(windows) { - return rendered.into_owned(); - } - normalize_separators(&strip_verbatim_prefix(&rendered)) -} - -/// Drop Windows' verbatim (`\\?\`) prefix, keeping the host's own separators. -fn strip_verbatim_prefix(rendered: &str) -> String { - match rendered.strip_prefix(r"\\?\UNC\") { - // Verbatim UNC collapses back to the `\\server\share` form; dropping - // the whole prefix would leave a bare `UNC\` component. - Some(rest) => format!(r"\\{rest}"), - None => rendered - .strip_prefix(r"\\?\") - .unwrap_or(rendered) - .to_string(), - } + path.to_string_lossy().into_owned() } /// The one spelling of `path` that every participant in a run agrees on. /// -/// POSIX hands this out for free: `getcwd` resolves symlinks, so a Unix process -/// and everything it spawns already share one spelling of the working directory. -/// Windows makes no such promise — it hands back whatever spelling the cwd was -/// set with — and one directory there has several valid names: an 8.3 short -/// name (`RUNNER~1`), a junction, a `subst` drive, a redirected profile -/// directory. Tools then disagree about which to report: `git` prints the -/// resolved name, while node's `process.cwd()` and `cmd`'s `cd` echo the alias -/// back. Anything comparing those strings — the write guard's allowed roots -/// against the paths an agent's own tools hand it — silently stops matching. -/// -/// Resolving once, at the point a run's roots are derived, gives Windows the -/// guarantee POSIX already provides. The verbatim (`\\?\`) prefix comes off -/// because a spawned child reports the plain form, so plain is the spelling the -/// comparisons actually see. +/// `getcwd` and `canonicalize` resolve symlink aliases, so resolving once at the +/// point a run's roots are derived gives the write guard, Git, and spawned tools +/// one spelling to compare. /// /// A run names directories before it creates them, so resolution walks up to the -/// deepest ancestor that exists and re-attaches the rest: the alias always lives -/// in an ancestor — a temp dir, a junction, an 8.3 profile name — never in the -/// leaf about to be created. With no ancestor on disk at all, the lexical form -/// is all there is. +/// deepest ancestor that exists and re-attaches the rest. With no ancestor on +/// disk at all, the lexical form is all there is. pub fn real_path(path: &Path) -> io::Result { let absolute = std::path::absolute(path)?; let mut unresolved = Vec::new(); let mut anchor = absolute.as_path(); loop { if let Ok(canonical) = fs::canonicalize(anchor) { - let mut resolved = if cfg!(windows) { - PathBuf::from(strip_verbatim_prefix(&canonical.to_string_lossy())) - } else { - canonical - }; + let mut resolved = canonical; resolved.extend(unresolved.iter().rev()); return Ok(resolved); } @@ -180,25 +139,10 @@ pub fn hardlinks_available(from: &Path, to: &Path) -> bool { /// /// Test support. Copying here resolves links into content rather than /// recreating them, so the only callers left are fixtures that need a link to -/// exist and the probe that asks whether this host permits one. -/// -/// `to_directory` is consulted only on Windows, which has separate file and -/// directory link kinds; POSIX has one. Creating a symlink there also needs -/// either Developer Mode or elevation, so this can fail for reasons that have -/// nothing to do with the paths involved. +/// exist and the probe that asks whether this filesystem permits one. #[cfg(test)] -pub(crate) fn create_symlink(target: &Path, link: &Path, to_directory: bool) -> io::Result<()> { - #[cfg(unix)] - { - let _ = to_directory; - std::os::unix::fs::symlink(target, link) - } - #[cfg(windows)] - if to_directory { - std::os::windows::fs::symlink_dir(target, link) - } else { - std::os::windows::fs::symlink_file(target, link) - } +pub(crate) fn create_symlink(target: &Path, link: &Path) -> io::Result<()> { + std::os::unix::fs::symlink(target, link) } /// Create `path`'s parent directory chain, when it has one. @@ -209,37 +153,6 @@ fn create_parent(path: &Path) -> io::Result<()> { } } -/// Make `link` a second name for the directory `target`, for tests that need one -/// directory reachable by two spellings. -/// -/// Deliberately *not* gated on the symlink capability. The paths that resolve -/// aliases differently are a Windows problem, so a fixture that skips on a -/// stock Windows box would leave that platform uncovered exactly where it -/// matters. A junction is the Windows alias that needs no Developer Mode and no -/// elevation, and `canonicalize` collapses it the same way it collapses a -/// symlink, an 8.3 short name, or a `subst` drive. -#[cfg(test)] -pub(crate) fn create_directory_alias(target: &Path, link: &Path) -> io::Result<()> { - if !cfg!(windows) { - return create_symlink(target, link, true); - } - let status = std::process::Command::new("cmd") - .args(["/C", "mklink", "/J"]) - .arg(link) - .arg(target) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status()?; - if status.success() { - return Ok(()); - } - Err(io::Error::other(format!( - "mklink /J could not alias {} to {}", - link.display(), - target.display() - ))) -} - #[cfg(test)] mod tests { use super::*; @@ -248,17 +161,15 @@ mod tests { /// Whether this host lets the test process create a symlink at all. /// - /// A capability, not a platform: Windows can create symlinks, but only under - /// Developer Mode or elevation. Probing beats gating on the OS — the tests - /// then run wherever the capability exists instead of wherever the OS name - /// matches. + /// A capability, not a platform label: tests exercise links wherever the + /// backing filesystem permits them. fn symlinks_available(scratch: &Path) -> bool { let target = scratch.join("probe-target.txt"); let link = scratch.join("probe-link.txt"); if fs::write(&target, "probe").is_err() { return false; } - create_symlink(&target, &link, false).is_ok() + create_symlink(&target, &link).is_ok() } /// Report a skipped symlink test, deferring to the shared skip policy so the @@ -267,7 +178,7 @@ mod tests { !symlinks_available(scratch) && crate::core::runtime::report_skip( test, - "this host does not permit symlink creation (Windows needs Developer Mode)", + "this filesystem does not permit symlink creation", ) } @@ -276,11 +187,17 @@ mod tests { #[test] fn real_path_collapses_an_alias_onto_the_resolved_spelling() { let tmp = TempDir::new().unwrap(); + if skip_without_symlinks( + tmp.path(), + "real_path_collapses_an_alias_onto_the_resolved_spelling", + ) { + return; + } let real = tmp.path().join("real-dir"); fs::create_dir_all(&real).unwrap(); fs::create_dir_all(real.join("nested")).unwrap(); let alias = tmp.path().join("alias-dir"); - create_directory_alias(&real, &alias).unwrap(); + create_symlink(&real, &alias).unwrap(); assert_eq!( real_path(&alias.join("nested")).unwrap(), @@ -288,31 +205,24 @@ mod tests { ); } - /// The verbatim prefix is an OS escape hatch: a spawned child reports the - /// plain form, so a root carrying `\\?\` would fail to match every path the - /// comparisons actually see. - #[test] - fn real_path_never_returns_a_verbatim_prefix() { - let tmp = TempDir::new().unwrap(); - let resolved = real_path(tmp.path()).unwrap(); - assert!( - !resolved.to_string_lossy().starts_with(r"\\?\"), - "{resolved:?} still carries the verbatim prefix" - ); - } - /// A run names directories before it creates them — a workspace root, an /// iteration dir. Resolving only whole existing paths would leave exactly - /// those unresolved, and the alias lives in the *ancestor* anyway (a temp - /// dir, a junction, an 8.3 profile name), never in the leaf about to be - /// created. So resolve as far down as the disk goes and re-attach the rest. + /// those unresolved, and the alias lives in the *ancestor* anyway, never in + /// the leaf about to be created. Resolve as far down as the disk goes and + /// re-attach the rest. #[test] fn real_path_resolves_the_existing_ancestor_of_a_path_not_yet_created() { let tmp = TempDir::new().unwrap(); + if skip_without_symlinks( + tmp.path(), + "real_path_resolves_the_existing_ancestor_of_a_path_not_yet_created", + ) { + return; + } let real = tmp.path().join("real-dir"); fs::create_dir_all(&real).unwrap(); let alias = tmp.path().join("alias-dir"); - create_directory_alias(&real, &alias).unwrap(); + create_symlink(&real, &alias).unwrap(); let unborn = alias.join("workspace").join("iteration-1"); assert_eq!( @@ -328,11 +238,7 @@ mod tests { /// no worse than the spelling the caller passed in. #[test] fn real_path_falls_back_to_the_lexical_form_when_no_ancestor_exists() { - let absent = Path::new(if cfg!(windows) { - r"C:\no-such-root-here\child" - } else { - "/no-such-root-here/child" - }); + let absent = Path::new("/no-such-root-here/child"); assert_eq!( real_path(absent).unwrap(), std::path::absolute(absent).unwrap() @@ -349,39 +255,14 @@ mod tests { ); } - /// A backslash means different things per host, so `artifact_path` does too, - /// and both halves belong in one place. - /// - /// On Windows it is a separator: `Path::join` on a POSIX-rooted base emits - /// one, so a manifest entry would otherwise read `/work/cond\run.json`. A - /// verbatim `\\?\` prefix is stripped as well — an OS-level escape hatch, not - /// something an agent should ever be handed. On POSIX a backslash is a legal - /// filename character, so rewriting it would name a different file. + /// A backslash is a legal POSIX filename character, so artifact rendering + /// preserves it rather than naming a different file. #[test] - fn artifact_path_applies_host_separator_rules() { - if cfg!(windows) { - assert_eq!( - artifact_path(Path::new(r"/work/cond\run.json")), - "/work/cond/run.json" - ); - assert_eq!( - artifact_path(Path::new(r"C:\work\cond\run.json")), - "C:/work/cond/run.json" - ); - assert_eq!( - artifact_path(Path::new(r"\\?\C:\work\run.json")), - "C:/work/run.json" - ); - assert_eq!( - artifact_path(Path::new(r"\\?\UNC\host\share\run.json")), - "//host/share/run.json" - ); - } else { - assert_eq!( - artifact_path(Path::new(r"/work/od\dity.json")), - r"/work/od\dity.json" - ); - } + fn artifact_path_preserves_literal_backslashes() { + assert_eq!( + artifact_path(Path::new(r"/work/od\dity.json")), + r"/work/od\dity.json" + ); } /// Comparison normalization is unconditional, unlike [`artifact_path`]: its @@ -445,7 +326,7 @@ mod tests { let source = tmp.path().join("tree"); fs::create_dir_all(&source).unwrap(); fs::write(source.join("real.txt"), "frozen").unwrap(); - create_symlink(Path::new("real.txt"), &source.join("alias.txt"), false).unwrap(); + create_symlink(Path::new("real.txt"), &source.join("alias.txt")).unwrap(); let destination = tmp.path().join("copied"); copy_entry_materialized(&source, &destination).unwrap(); diff --git a/src/core/git.rs b/src/core/git.rs index 014ba01..a223c7c 100644 --- a/src/core/git.rs +++ b/src/core/git.rs @@ -70,10 +70,6 @@ impl IsolatedGit { pub(crate) fn run(&self, cwd: &Path, args: &[&str], env: &[(&str, &str)]) -> GitOutput { let mut command = Command::new("git"); command - // `git clone` and `git init` create paths inside `.git` before any - // repository-local configuration exists, so the Windows long-path - // lift has to ride on the invocation itself. - .args(["-c", "core.longpaths=true"]) .args(args) .current_dir(cwd) .env("GIT_CONFIG_NOSYSTEM", "1") diff --git a/src/core/runtime.rs b/src/core/runtime.rs index 8e266e8..5fe85b8 100644 --- a/src/core/runtime.rs +++ b/src/core/runtime.rs @@ -102,75 +102,24 @@ pub fn run_git(args: &[&str], cwd: &Path) -> GitOutput { /// `--help` restates it in `cli::help::AFTER_HELP` instead, hard-wrapped and /// without backticks, because clap renders into a terminal rather than Markdown. /// -/// A POSIX shell is the whole requirement. Harness `exec_template`s ship as -/// POSIX command lines (`/mingw64/libexec/git-core` — hence three levels up). -/// Always spelled `sh.exe`: this layout only exists on Windows, and pinning the -/// name keeps the function testable on every host. -fn git_shell_candidates(exec_path: &Path) -> Vec { - let Some(root) = exec_path.ancestors().nth(3) else { - return Vec::new(); - }; - if root.as_os_str().is_empty() { - return Vec::new(); - } - vec![ - root.join("bin").join("sh.exe"), - root.join("usr").join("bin").join("sh.exe"), - ] -} +/// A POSIX shell is the whole tooling requirement. Harness `exec_template`s ship +/// as POSIX command lines (` Option { - let file_name = if cfg!(windows) { - format!("{name}.exe") - } else { - name.to_string() - }; std::env::split_paths(&std::env::var_os("PATH")?) - .map(|directory| directory.join(&file_name)) + .map(|directory| directory.join(name)) .find(|candidate| candidate.is_file()) } -/// Where to look for `sh` on Windows once `PATH` has come up empty. Git for -/// Windows bundles one, but its default installer only puts `Git\cmd` on -/// `PATH`, so the shell has to be located through the install root instead. -fn windows_shell_candidates() -> Vec { - let mut candidates = Vec::new(); - let git = run_git(&["--exec-path"], Path::new(".")); - if git.status == Some(0) { - let exec_path = String::from_utf8_lossy(&git.stdout).trim().to_string(); - if !exec_path.is_empty() { - candidates.extend(git_shell_candidates(Path::new(&exec_path))); - } - } - candidates.push(PathBuf::from(r"C:\Program Files\Git\bin\sh.exe")); - candidates.push(PathBuf::from(r"C:\Program Files\Git\usr\bin\sh.exe")); - candidates -} - /// Locate a POSIX shell. `override_path` carries the operator's `EVAL_MAGIC_SH` /// value and is passed in rather than read here so tests can exercise it /// without mutating process environment. /// -/// Only ever searches for `sh`. On Windows `C:\Windows\System32\bash.exe` is -/// the WSL launcher, which resolves a different filesystem namespace — every -/// Windows path handed to it would name the wrong file. +/// Only ever searches for `sh`: first on `PATH`, then at `/bin/sh`. fn discover_posix_shell(override_path: Option<&OsStr>) -> Result { if let Some(value) = override_path { let path = PathBuf::from(value); @@ -187,13 +136,6 @@ fn discover_posix_shell(override_path: Option<&OsStr>) -> Result) -> Result Result<&'static Path, &'static str> { static SHELL: OnceLock> = OnceLock::new(); match SHELL.get_or_init(|| discover_posix_shell(std::env::var_os("EVAL_MAGIC_SH").as_deref())) { @@ -342,10 +284,8 @@ mod tests { ); assert_eq!(res.status, None); assert!(res.stdout.is_empty()); - // Deliberately not matched against an errno spelling: the OS wording - // differs per platform ("No such file or directory" vs "The system - // cannot find the path specified"), and the contract is a readable - // reason, not any particular one. + // Deliberately not matched against an errno spelling: the contract is a + // readable reason, not any particular libc wording. let reason = String::from_utf8_lossy(&res.stderr); assert!( !reason.trim().is_empty(), @@ -353,27 +293,6 @@ mod tests { ); } - /// The Git for Windows layout: `git --exec-path` points at - /// `/mingw64/libexec/git-core`, so the shell sits three levels up. - #[test] - fn git_shell_candidates_walk_up_from_the_git_core_exec_path() { - let root = Path::new("/opt/Git"); - assert_eq!( - git_shell_candidates(&root.join("mingw64").join("libexec").join("git-core")), - vec![ - root.join("bin").join("sh.exe"), - root.join("usr").join("bin").join("sh.exe"), - ] - ); - } - - /// An exec path too shallow to contain a Git root yields no candidates - /// rather than walking off the top into `/`. - #[test] - fn git_shell_candidates_are_empty_for_a_rootless_exec_path() { - assert!(git_shell_candidates(Path::new("git-core")).is_empty()); - } - #[test] fn discover_posix_shell_accepts_an_explicit_override() { let existing = std::env::current_exe().unwrap(); @@ -392,8 +311,8 @@ mod tests { .expect_err("a missing override should not fall through to discovery"); assert!(error.contains("EVAL_MAGIC_SH"), "{error}"); assert!(error.contains("/nonexistent-shell-for-tests"), "{error}"); - assert!(error.contains("Git Bash"), "{error}"); assert!(error.contains("WSL"), "{error}"); + assert!(error.contains("native Windows is unsupported"), "{error}"); // A POSIX shell is the whole requirement now. `jq` was only ever needed // by the generated recipes an operator pasted, and the runner dispatches // directly instead. @@ -413,22 +332,17 @@ mod tests { assert!(shell.is_file(), "{} is not a file", shell.display()); } - /// The declared requirement has to separate the two Windows options rather - /// than list them as equivalent. Git Bash shares the Windows filesystem, so a - /// workspace prepared by a native run dispatches from it correctly. WSL - /// resolves a different namespace, where the `C:\…` paths a native run wrote - /// name nothing — so WSL is only correct when eval-magic itself runs inside - /// it. Listing the two side by side invites a split that silently cannot work. + /// The declared requirement names the complete Windows support boundary: + /// eval-magic itself runs inside WSL, using the supported Linux build. #[test] - fn the_declared_requirement_places_wsl_around_eval_magic_not_downstream_of_it() { + fn the_declared_requirement_directs_windows_users_to_wsl() { assert!( - POSIX_TOOLING_REQUIREMENT.contains("Git Bash"), + POSIX_TOOLING_REQUIREMENT.contains("inside WSL"), "{POSIX_TOOLING_REQUIREMENT}" ); assert!( - POSIX_TOOLING_REQUIREMENT.contains("inside WSL"), - "WSL must be named as where eval-magic runs, not somewhere to dispatch \ - into: {POSIX_TOOLING_REQUIREMENT}" + POSIX_TOOLING_REQUIREMENT.contains("native Windows is unsupported"), + "{POSIX_TOOLING_REQUIREMENT}" ); } @@ -442,10 +356,8 @@ mod tests { ); } - /// Build a `__fixture` command line. The string is handed to a shell, so it - /// has to parse identically under `sh -c` and `cmd /C`: a double-quoted - /// program path followed by double-quoted arguments does, because both - /// shells strip the quotes and hand the tokens over unchanged. + /// Build a `__fixture` command line for `sh -c`. Quoting every argument keeps + /// paths with spaces and literal fixture values intact. fn fixture(args: &[&str]) -> String { let exe = assert_cmd::cargo::cargo_bin("eval-magic"); assert!( diff --git a/src/pipeline/grade/command_check.rs b/src/pipeline/grade/command_check.rs index c78b80d..07ff85b 100644 --- a/src/pipeline/grade/command_check.rs +++ b/src/pipeline/grade/command_check.rs @@ -345,27 +345,8 @@ fn execute_command_check_cell( eval_root: &Path, env: BTreeMap, ) -> Result { - #[cfg(unix)] - let mut command = { - let mut command = Command::new("sh"); - command.arg("-c").arg(&assertion.command); - command - }; - #[cfg(windows)] - let mut command = { - use std::os::windows::process::CommandExt; - let mut command = Command::new("cmd"); - // `raw_arg` plus `/S` and one wrapping pair of quotes is the only - // spelling that hands `cmd` the command verbatim. `arg` would escape the - // command's own quotes as `\"`, which `cmd` does not understand — a - // quoted argument arrives split at its spaces — and `/S` makes `cmd` - // strip exactly the wrapping pair rather than guessing. - command - .arg("/S") - .arg("/C") - .raw_arg(format!("\"{}\"", assertion.command)); - command - }; + let mut command = Command::new("sh"); + command.arg("-c").arg(&assertion.command); command.current_dir(eval_root); // The task root defines repository discovery for runner-owned checks. @@ -377,7 +358,7 @@ fn execute_command_check_cell( let output = output.map_err(|error| { PipelineError::Message(format!( - "could not launch the platform shell for command_check '{}': {error}", + "could not launch the POSIX shell for command_check '{}': {error}", assertion.id )) })?; @@ -433,8 +414,8 @@ fn execute_command_check_cell( } /// The evidence line for a child that ended without an exit code. Split from -/// [`termination_evidence`] so the wording is pinned on every platform, leaving -/// the per-OS arms below with nothing to do but read the signal. +/// [`termination_evidence`] so tests can pin the wording independently of +/// signal extraction. fn termination_message(signal: Option) -> String { match signal { Some(signal) => format!("command terminated by signal {signal}"), @@ -442,19 +423,11 @@ fn termination_message(signal: Option) -> String { } } -#[cfg(unix)] fn termination_evidence(status: &ExitStatus) -> String { use std::os::unix::process::ExitStatusExt; termination_message(status.signal()) } -/// Windows has no signals — `ExitStatus::code()` is always `Some`, so this arm -/// exists only to keep the caller platform-agnostic. -#[cfg(windows)] -fn termination_evidence(_status: &ExitStatus) -> String { - termination_message(None) -} - fn truncate_diagnostic(value: &str) -> String { if value.len() <= DIAGNOSTIC_LIMIT { return value.to_string(); diff --git a/src/pipeline/grade/command_check/tests.rs b/src/pipeline/grade/command_check/tests.rs index 978adcb..6328174 100644 --- a/src/pipeline/grade/command_check/tests.rs +++ b/src/pipeline/grade/command_check/tests.rs @@ -17,12 +17,8 @@ fn check(command: &str) -> AssertionCommandCheck { /// A `__fixture` invocation as a shell command line. /// -/// `execute_command_check` hands the string to the platform shell, so it has to -/// parse identically under `sh -c` and `cmd /C`. A double-quoted program path -/// followed by double-quoted arguments does: both shells strip the quotes and -/// hand the tokens to the program unchanged. Writing one command per shell -/// dialect instead invites silent divergence — `printf x` and `echo x` do not -/// agree on the trailing newline. +/// `execute_command_check` hands the string to `sh -c`. Double-quoting the +/// program path and arguments preserves spaces and literal fixture values. fn fixture(args: &[&str]) -> String { let exe = assert_cmd::cargo::cargo_bin("eval-magic"); assert!( @@ -134,12 +130,9 @@ fn expected_and_unexpected_exit_codes_are_assertion_results() { assert!(failed.evidence.contains("got 3")); } -/// An eval author's `command_check` reaches the shell with its own quoting -/// intact. Windows makes this easy to get wrong: Rust escapes a command's -/// embedded quotes as `\"`, which `cmd.exe` does not understand, so a quoted -/// argument silently arrives split at the space. +/// An eval author's `command_check` reaches `sh -c` with its own quoting intact. #[test] -fn command_reaches_the_platform_shell_with_its_quoting_intact() { +fn command_reaches_the_posix_shell_with_its_quoting_intact() { let root = tempfile::TempDir::new().unwrap(); let result = execute_command_check(&check(&fixture(&["--text", "spaced value"])), root.path()).unwrap(); @@ -147,6 +140,19 @@ fn command_reaches_the_platform_shell_with_its_quoting_intact() { assert_eq!(result.stdout, "spaced value"); } +#[test] +fn command_accepts_posix_environment_assignment_syntax() { + let root = tempfile::TempDir::new().unwrap(); + let command = format!( + "EVAL_MAGIC_TEST_VALUE=posix {}", + fixture(&["--require-env", "EVAL_MAGIC_TEST_VALUE=posix"]) + ); + + let result = execute_command_check(&check(&command), root.path()).unwrap(); + + assert!(result.passed, "{}", result.evidence); +} + #[test] fn stdout_regex_must_match_complete_lossy_stdout() { let root = tempfile::TempDir::new().unwrap(); @@ -410,13 +416,6 @@ fn termination_message_names_the_signal_when_there_is_one() { #[test] fn signal_termination_is_an_ordinary_failed_assertion() { - // Windows has no signals: a child always reports an exit code, so there is - // no way to reach the no-code path from the outside. The wording it would - // produce is pinned by `termination_message_names_the_signal_when_there_is_one` - // instead, which runs everywhere. - if cfg!(windows) { - return; - } let root = tempfile::TempDir::new().unwrap(); let result = execute_command_check(&check("kill -TERM $$"), root.path()).unwrap(); assert!(!result.passed); diff --git a/src/sandbox/install.rs b/src/sandbox/install.rs index 66f282a..cb93a20 100644 --- a/src/sandbox/install.rs +++ b/src/sandbox/install.rs @@ -204,11 +204,9 @@ mod tests { stage_root: PathBuf, } - /// The marker path as it appears *inside* a JSON string value: the hook - /// command embeds it, so every Windows separator is escaped to `\\`. - /// Interpolating `display()` raw builds an expectation that is not even - /// valid JSON, and the byte pin then fails on a difference the file does - /// not have. + /// The marker path as it appears *inside* a JSON string value. Serializing + /// it keeps the expectation valid JSON even when the path contains bytes + /// that need escaping. fn json_string_body(path: &Path) -> String { let quoted = serde_json::to_string(&path.to_string_lossy()).unwrap(); quoted[1..quoted.len() - 1].to_string() diff --git a/src/sandbox/shell_targets.rs b/src/sandbox/shell_targets.rs index 389cdcb..4ccad64 100644 --- a/src/sandbox/shell_targets.rs +++ b/src/sandbox/shell_targets.rs @@ -310,9 +310,8 @@ fn fd_duplication_end(chars: &[char], at: usize) -> Option { /// Applied to the *resolved* path, so `/dev/../etc/passwd` cannot launder an /// out-of-bounds target through the `/dev` prefix. /// -/// Matched by path component rather than by string: a resolved path renders -/// with the host's separator, so `/dev/fd/1` reads back as `fd\1` on Windows -/// and a `"fd/"` string prefix would miss it. +/// Matched by path component rather than by string so path rendering details do +/// not affect the result. fn is_non_file_device(resolved: &Path) -> bool { let Ok(rest) = resolved.strip_prefix("/dev") else { return false; diff --git a/tests/cli/docs.rs b/tests/cli/docs.rs index 6cc7b23..099a3e5 100644 --- a/tests/cli/docs.rs +++ b/tests/cli/docs.rs @@ -309,13 +309,25 @@ fn repository_documentation_map_names_each_surface() { // A POSIX shell is a development requirement, not a probed capability: the // dispatch tests spawn a `#!/bin/sh` stub through it and cannot skip. Both - // contributor-facing docs have to say so, or the next contributor on - // Windows rediscovers it as a test failure (issue #248). + // contributor-facing docs have to say so, including where Windows + // contributors run the toolchain. for (name, text) in [("AGENTS.md", &agents), ("developer overview", &overview)] { assert!( text.contains("POSIX shell"), "{name} should record the POSIX shell development requirement" ); + assert!( + text.contains("WSL"), + "{name} should direct Windows work to WSL" + ); + assert!( + text.contains("native Windows"), + "{name} should state the unsupported native-Windows boundary" + ); + assert!( + !text.contains("Git Bash"), + "{name} should not retain a Git Bash fallback" + ); } } @@ -329,8 +341,10 @@ fn help_states_the_posix_tooling_requirement() { .success() .stdout(contains("REQUIREMENTS:")) .stdout(contains("POSIX shell")) - .stdout(contains("Git Bash")) .stdout(contains("WSL")) + .stdout(contains("native Windows")) + .stdout(contains("Git Bash").not()) + .stdout(contains("PowerShell").not()) // `jq` was a requirement only while operators pasted the generated // recipes; the runner dispatches directly and needs no such toolchain. .stdout(contains("jq").not()); @@ -353,16 +367,19 @@ fn readme_is_a_concise_first_run_path() { "eval-magic docs isolation", "docs/developer_overview.md", // The declared host requirement, stated for both audiences the README - // serves: installing the tool, and developing it (issue #248). `jq` is - // deliberately absent — it was a requirement only while operators - // pasted the generated recipes. + // serves: installing the tool and developing it. `jq` is deliberately + // absent because the runner dispatches directly. "POSIX shell", - "Git Bash", "WSL", + "native Windows", ] { assert!(readme.contains(expected), "README is missing {expected}"); } + for retired in ["Git Bash", "Windows PowerShell", "eval-magic-installer.ps1"] { + assert!(!readme.contains(retired), "README still contains {retired}"); + } + assert!( readme.lines().count() <= 175, "README should hand detail to shipped docs instead of duplicating it" diff --git a/tests/cli/guard.rs b/tests/cli/guard.rs index 9cb03c9..a09f0a8 100644 --- a/tests/cli/guard.rs +++ b/tests/cli/guard.rs @@ -26,10 +26,9 @@ fn guard_subcommand_is_hidden_but_callable() { /// Write an armed guard marker scoping writes to `` under /// `//skills`, and return its path. /// -/// Serialized rather than string-interpolated: a Windows path embeds `\U`, -/// `\A`, `\T` — none of them valid JSON escapes — so a `format!`-built marker -/// is malformed, the guard reads it as absent, and every assertion below -/// silently passes through the fail-open path instead of testing anything. +/// Serialized rather than string-interpolated so path bytes that require JSON +/// escaping cannot make the marker malformed and send the assertions through +/// the fail-open path. fn write_marker_in( root: &std::path::Path, namespace: &str, diff --git a/tests/cli/helpers.rs b/tests/cli/helpers.rs index 444f9ac..2188d74 100644 --- a/tests/cli/helpers.rs +++ b/tests/cli/helpers.rs @@ -19,19 +19,12 @@ pub fn skill_eval() -> Command { cmd } -/// `fs::canonicalize` with Windows' verbatim (`\\?\`) prefix removed — the -/// spelling the CLI itself resolves paths to, and the one a child process -/// reports as its cwd. Fixtures built on any other spelling of the same -/// directory will not match the paths the CLI emits. +/// The canonical spelling the CLI resolves paths to. /// -/// Both halves matter, and each is a different host's problem: the resolution -/// covers macOS (/var → /private/var), the stripping covers Windows. +/// Fixtures built on an alias of the same directory will not match paths the +/// CLI emits. This matters on macOS, where `/var` resolves to `/private/var`. pub fn resolved(path: &Path) -> PathBuf { - let canonical = fs::canonicalize(path).unwrap(); - match canonical.to_string_lossy().strip_prefix(r"\\?\") { - Some(plain) => PathBuf::from(plain), - None => canonical, - } + fs::canonicalize(path).unwrap() } /// A temp root already in the spelling [`resolved`] describes. diff --git a/tests/cli/package.rs b/tests/cli/package.rs index b5149e7..9e9305c 100644 --- a/tests/cli/package.rs +++ b/tests/cli/package.rs @@ -13,6 +13,21 @@ fn read_repo_file(path: &str) -> String { }) } +fn rust_sources_under(path: &Path) -> Vec { + let mut sources = Vec::new(); + for entry in std::fs::read_dir(path).unwrap_or_else(|err| { + panic!("expected to read {}: {err}", path.display()); + }) { + let path = entry.expect("repository entry should be readable").path(); + if path.is_dir() { + sources.extend(rust_sources_under(&path)); + } else if path.extension().is_some_and(|extension| extension == "rs") { + sources.push(path); + } + } + sources +} + #[test] fn source_files_advertise_crates_io_publish_channel() { let manifest = read_repo_file("Cargo.toml"); @@ -70,26 +85,58 @@ fn ci_publishes_default_branch_coverage_for_readme_badge() { } } -/// Releases attach a Windows binary, so the suite has to run on Windows — but -/// the matrix entry alone proves nothing. Six of those tests are gated on -/// capabilities the runner has to be handed: `jq` for the judge recipes, and -/// symlink creation for the `core::fs` round-trips. Without the enforcement -/// variable they skip in silence, and the job reports green while covering -/// strictly less than it looks like it is. Every string below is load-bearing, -/// which is why they are pinned together rather than one standing for the rest. #[test] -fn ci_runs_the_suite_on_windows_with_capability_skips_enforced() { - let workflow = read_repo_file(".github/workflows/ci.yml"); +fn native_windows_runtime_and_release_surfaces_are_absent() { + let platform = "windows"; + let native_markers = [ + format!("cfg!({platform})"), + format!("#[cfg({platform})]"), + format!("cfg_attr({platform}"), + format!("target_os = \"{platform}\""), + format!("std::os::{platform}"), + format!("Command::new(\"{}\")", "cmd"), + format!("core.{}", "longpaths"), + ]; + let mut rust_sources = rust_sources_under(&repo_root().join("src")); + rust_sources.extend(rust_sources_under(&repo_root().join("tests"))); + for path in rust_sources { + let source = std::fs::read_to_string(&path) + .unwrap_or_else(|err| panic!("expected to read {}: {err}", path.display())); + for marker in &native_markers { + assert!( + !source.contains(marker), + "{} still contains native-Windows marker {marker}", + path.display() + ); + } + } - for expected in [ - "os: [ubuntu-latest, windows-latest]", - "fail-fast: false", - "EVAL_MAGIC_REQUIRE_POSIX_TOOLS: 1", - "choco install jq", - "AllowDevelopmentWithoutDevLicense", + let ci = read_repo_file(".github/workflows/ci.yml"); + assert!(ci.contains("runs-on: ubuntu-latest")); + for marker in [ + format!("{platform}-latest"), + "choco install jq".to_string(), + "AllowDevelopmentWithoutDevLicense".to_string(), ] { - assert!(workflow.contains(expected), "CI is missing {expected}"); + assert!(!ci.contains(&marker), "CI still contains {marker}"); } + + let dist = read_repo_file("dist-workspace.toml"); + assert!(dist.contains(r#"installers = ["shell"]"#)); + assert!( + dist.contains(r#"allow-dirty = ["ci"]"#), + "cargo-dist must allow the release workflow to omit its unconditional Windows setup" + ); + assert!(!dist.contains(&format!("{platform}-msvc"))); + assert!(!dist.contains(&format!("{}shell", "power"))); + + let release = read_repo_file(".github/workflows/release.yml"); + assert!(!release.contains(&format!("core.{}", "longpaths"))); + assert!(!release.contains(&format!("{}shell", "power"))); + + let evals_schema = read_repo_file("schema/evals.schema.json"); + assert!(evals_schema.contains("POSIX shell command")); + assert!(evals_schema.contains("sh -c")); } #[test] diff --git a/tests/golden/claude-code/manifest.golden.md b/tests/golden/claude-code/manifest.golden.md index fca698f..51b1937 100644 --- a/tests/golden/claude-code/manifest.golden.md +++ b/tests/golden/claude-code/manifest.golden.md @@ -8,7 +8,7 @@ Total dispatches: 2 In an agent session, read `dispatch.json` (sibling of this file) instead of this manifest. Each task has a `dispatch_prompt_path` field pointing at the file that holds the full prompt — dispatch the task with a short "read this file and follow it" instruction rather than inlining the prompt — plus exact paths for `run.json` and `timing.json`. -**Requires:** harness dispatch commands are POSIX command lines, and `eval-magic dispatch` runs them itself, so the host it runs on needs a POSIX shell — on Windows, Git Bash (Git for Windows). WSL resolves a different filesystem namespace, so run eval-magic inside WSL rather than dispatching into it. Set EVAL_MAGIC_SH to select a specific `sh`. +**Requires:** `eval-magic` supports Linux and macOS. On Windows, run `eval-magic` inside WSL; native Windows is unsupported. Git and a POSIX shell are required. Set EVAL_MAGIC_SH to select a specific `sh`. ## Dispatch diff --git a/tests/golden/claude-code/runbook.golden.md b/tests/golden/claude-code/runbook.golden.md index 43350cb..68dd4f0 100644 --- a/tests/golden/claude-code/runbook.golden.md +++ b/tests/golden/claude-code/runbook.golden.md @@ -4,7 +4,7 @@ This runbook is for a human driving the run from a terminal. Work from this iter and copy-paste each step. The workspace is self-contained — you should not need the surrounding repo. -> **Requires:** harness dispatch commands are POSIX command lines, and `eval-magic dispatch` runs them itself, so the host it runs on needs a POSIX shell — on Windows, Git Bash (Git for Windows). WSL resolves a different filesystem namespace, so run eval-magic inside WSL rather than dispatching into it. Set EVAL_MAGIC_SH to select a specific `sh`. +> **Requires:** `eval-magic` supports Linux and macOS. On Windows, run `eval-magic` inside WSL; native Windows is unsupported. Git and a POSIX shell are required. Set EVAL_MAGIC_SH to select a specific `sh`. - **Skill under test:** widget-skill - **Mode:** revision — comparing `old_skill` vs `new_skill` diff --git a/tests/golden/cline/manifest.golden.md b/tests/golden/cline/manifest.golden.md index 44605c1..528b643 100644 --- a/tests/golden/cline/manifest.golden.md +++ b/tests/golden/cline/manifest.golden.md @@ -8,7 +8,7 @@ Total dispatches: 2 In an agent session, read `dispatch.json` (sibling of this file) instead of this manifest. Each task has a `dispatch_prompt_path` field pointing at the file that holds the full prompt — dispatch the task with a short "read this file and follow it" instruction rather than inlining the prompt — plus exact paths for `run.json` and `timing.json`. -**Requires:** harness dispatch commands are POSIX command lines, and `eval-magic dispatch` runs them itself, so the host it runs on needs a POSIX shell — on Windows, Git Bash (Git for Windows). WSL resolves a different filesystem namespace, so run eval-magic inside WSL rather than dispatching into it. Set EVAL_MAGIC_SH to select a specific `sh`. +**Requires:** `eval-magic` supports Linux and macOS. On Windows, run `eval-magic` inside WSL; native Windows is unsupported. Git and a POSIX shell are required. Set EVAL_MAGIC_SH to select a specific `sh`. ## Dispatch diff --git a/tests/golden/cline/runbook.golden.md b/tests/golden/cline/runbook.golden.md index 8998d00..36364a8 100644 --- a/tests/golden/cline/runbook.golden.md +++ b/tests/golden/cline/runbook.golden.md @@ -4,7 +4,7 @@ This runbook is for a human driving the run from a terminal. Work from this iter and copy-paste each step. The workspace is self-contained — you should not need the surrounding repo. -> **Requires:** harness dispatch commands are POSIX command lines, and `eval-magic dispatch` runs them itself, so the host it runs on needs a POSIX shell — on Windows, Git Bash (Git for Windows). WSL resolves a different filesystem namespace, so run eval-magic inside WSL rather than dispatching into it. Set EVAL_MAGIC_SH to select a specific `sh`. +> **Requires:** `eval-magic` supports Linux and macOS. On Windows, run `eval-magic` inside WSL; native Windows is unsupported. Git and a POSIX shell are required. Set EVAL_MAGIC_SH to select a specific `sh`. - **Skill under test:** widget-skill - **Mode:** revision — comparing `old_skill` vs `new_skill` diff --git a/tests/golden/codex/manifest.golden.md b/tests/golden/codex/manifest.golden.md index 202a2a2..e262baf 100644 --- a/tests/golden/codex/manifest.golden.md +++ b/tests/golden/codex/manifest.golden.md @@ -8,7 +8,7 @@ Total dispatches: 2 In an agent session, read `dispatch.json` (sibling of this file) instead of this manifest. Each task has a `dispatch_prompt_path` field pointing at the file that holds the full prompt — dispatch the task with a short "read this file and follow it" instruction rather than inlining the prompt — plus exact paths for `run.json` and `timing.json`. -**Requires:** harness dispatch commands are POSIX command lines, and `eval-magic dispatch` runs them itself, so the host it runs on needs a POSIX shell — on Windows, Git Bash (Git for Windows). WSL resolves a different filesystem namespace, so run eval-magic inside WSL rather than dispatching into it. Set EVAL_MAGIC_SH to select a specific `sh`. +**Requires:** `eval-magic` supports Linux and macOS. On Windows, run `eval-magic` inside WSL; native Windows is unsupported. Git and a POSIX shell are required. Set EVAL_MAGIC_SH to select a specific `sh`. ## Dispatch diff --git a/tests/golden/codex/runbook.golden.md b/tests/golden/codex/runbook.golden.md index c1ec038..6515b77 100644 --- a/tests/golden/codex/runbook.golden.md +++ b/tests/golden/codex/runbook.golden.md @@ -4,7 +4,7 @@ This runbook is for a human driving the run from a terminal. Work from this iter and copy-paste each step. The workspace is self-contained — you should not need the surrounding repo. -> **Requires:** harness dispatch commands are POSIX command lines, and `eval-magic dispatch` runs them itself, so the host it runs on needs a POSIX shell — on Windows, Git Bash (Git for Windows). WSL resolves a different filesystem namespace, so run eval-magic inside WSL rather than dispatching into it. Set EVAL_MAGIC_SH to select a specific `sh`. +> **Requires:** `eval-magic` supports Linux and macOS. On Windows, run `eval-magic` inside WSL; native Windows is unsupported. Git and a POSIX shell are required. Set EVAL_MAGIC_SH to select a specific `sh`. - **Skill under test:** widget-skill - **Mode:** revision — comparing `old_skill` vs `new_skill` diff --git a/tests/golden/opencode/manifest.golden.md b/tests/golden/opencode/manifest.golden.md index 66110d4..a6d67c1 100644 --- a/tests/golden/opencode/manifest.golden.md +++ b/tests/golden/opencode/manifest.golden.md @@ -8,7 +8,7 @@ Total dispatches: 2 In an agent session, read `dispatch.json` (sibling of this file) instead of this manifest. Each task has a `dispatch_prompt_path` field pointing at the file that holds the full prompt — dispatch the task with a short "read this file and follow it" instruction rather than inlining the prompt — plus exact paths for `run.json` and `timing.json`. -**Requires:** harness dispatch commands are POSIX command lines, and `eval-magic dispatch` runs them itself, so the host it runs on needs a POSIX shell — on Windows, Git Bash (Git for Windows). WSL resolves a different filesystem namespace, so run eval-magic inside WSL rather than dispatching into it. Set EVAL_MAGIC_SH to select a specific `sh`. +**Requires:** `eval-magic` supports Linux and macOS. On Windows, run `eval-magic` inside WSL; native Windows is unsupported. Git and a POSIX shell are required. Set EVAL_MAGIC_SH to select a specific `sh`. ## Dispatch diff --git a/tests/golden/opencode/runbook.golden.md b/tests/golden/opencode/runbook.golden.md index b86f7eb..0e40956 100644 --- a/tests/golden/opencode/runbook.golden.md +++ b/tests/golden/opencode/runbook.golden.md @@ -4,7 +4,7 @@ This runbook is for a human driving the run from a terminal. Work from this iter and copy-paste each step. The workspace is self-contained — you should not need the surrounding repo. -> **Requires:** harness dispatch commands are POSIX command lines, and `eval-magic dispatch` runs them itself, so the host it runs on needs a POSIX shell — on Windows, Git Bash (Git for Windows). WSL resolves a different filesystem namespace, so run eval-magic inside WSL rather than dispatching into it. Set EVAL_MAGIC_SH to select a specific `sh`. +> **Requires:** `eval-magic` supports Linux and macOS. On Windows, run `eval-magic` inside WSL; native Windows is unsupported. Git and a POSIX shell are required. Set EVAL_MAGIC_SH to select a specific `sh`. - **Skill under test:** widget-skill - **Mode:** revision — comparing `old_skill` vs `new_skill` diff --git a/tests/run/codebase.rs b/tests/run/codebase.rs index 15a54b8..90ed618 100644 --- a/tests/run/codebase.rs +++ b/tests/run/codebase.rs @@ -295,9 +295,7 @@ fn a_path_codebase_is_recorded_as_host_local_with_its_origin_for_citation() { let upstream = codebase_repo(tmp.path(), "upstream", "main"); let local = codebase_repo(tmp.path(), "local", "main"); // Git stores a remote URL byte-for-byte, and eval-magic cites it unchanged - // rather than rewriting what a user configured. Registering it in the host's - // own spelling is what pins that: on Windows the separators are backslashes, - // so any normalization on the way to the artifact shows up here. + // rather than rewriting what a user configured. let origin_url = upstream.to_string_lossy().to_string(); git(&local, &["remote", "add", "origin", &origin_url]); let revision = git(&local, &["rev-parse", "HEAD"]); @@ -350,70 +348,12 @@ fn a_fixture_only_eval_still_gets_the_repository_it_always_had() { /// The number of hard links to `file` — the mechanism `git clone --local` uses /// to share the cache's object store with an environment instead of copying -/// it. Straight from stat metadata on Unix. -#[cfg(unix)] +/// it. Straight from filesystem metadata. fn link_count(file: &Path) -> u32 { use std::os::unix::fs::MetadataExt; fs::metadata(file).unwrap().nlink() as u32 } -/// The number of hard links to `file`, read from fsutil because Windows has no -/// stable std route to it: `number_of_links` rides the unstable -/// `windows_by_handle` trait. fsutil prints one path per hard link, sometimes -/// behind a `Hardlink list on ...` header — the header is the only printed -/// line that is not a path. -#[cfg(windows)] -fn link_count(file: &Path) -> u32 { - let output = Command::new("fsutil") - .args(["hardlink", "list"]) - .arg(file) - .output() - .expect("fsutil hardlink list must run"); - assert!( - output.status.success(), - "fsutil hardlink list failed for {}: {}", - file.display(), - String::from_utf8_lossy(&output.stderr) - ); - hardlink_list_count(&String::from_utf8_lossy(&output.stdout)) -} - -/// Count the hard links in `fsutil hardlink list` output: one path per line, -/// sometimes behind a `Hardlink list on ...` header — the header is the only -/// printed line that is not a path. -fn hardlink_list_count(output: &str) -> u32 { - output - .lines() - .map(str::trim) - .filter(|line| line.contains('\\') && !line.starts_with("Hardlink")) - .count() as u32 -} - -/// Both layouts `fsutil hardlink list` prints. Pinned here because the -/// Windows arm of `link_count` runs only on Windows, while the counting is -/// plain string logic every runner can execute. -#[test] -fn fsutil_link_list_output_is_counted_in_both_of_its_formats() { - // Modern Windows: one \?\-prefixed path per hard link, no header. - let modern = r"\\?\C:\cache\.git\objects\ab\cdef -\\?\C:\env\.git\objects\ab\cdef -"; - assert_eq!(hardlink_list_count(modern), 2); - // Older Windows: the same paths behind a `Hardlink list on ...` header, - // CRLF-terminated. - let older_lf = r"Hardlink list on C:\cache\.git\objects\ab\cdef -C:\cache\.git\objects\ab\cdef -C:\env\.git\objects\ab\cdef -"; - let older = older_lf.replace('\n', "\r\n"); - assert_eq!(hardlink_list_count(&older), 2); - // A file no other path shares lists exactly once — the count that fails - // the hard-link assertions when an environment was copied, not cloned. - let lone = r"\\?\C:\env\.git\objects\ab\cdef -"; - assert_eq!(hardlink_list_count(lone), 1); -} - /// A file from `repo`'s object store — a loose object or a pack — that a local /// clone shares with its source by hard link. `objects/info` is skipped: it /// holds per-repository metadata (an exclude file), not objects, and is never diff --git a/tests/run/git_isolation.rs b/tests/run/git_isolation.rs index 299f943..8d9b486 100644 --- a/tests/run/git_isolation.rs +++ b/tests/run/git_isolation.rs @@ -77,13 +77,6 @@ fn every_task_is_a_clean_local_git_repo_inside_a_dirty_ignored_parent_repo() { assert_eq!(git(eval_root, &["symbolic-ref", "--short", "HEAD"]), "work"); assert_eq!(git(eval_root, &["status", "--porcelain"]), ""); assert_eq!(git(eval_root, &["remote"]), ""); - // Without this, a staged skill under a deep workspace hits Windows' - // MAX_PATH. Asserted on every host, since a Linux runner cannot prove it - // with a deep path but can still catch the setting going missing. - assert_eq!( - git(eval_root, &["config", "--local", "--get", "core.longpaths"]), - "true" - ); assert_eq!( git( eval_root, diff --git a/tests/run/helpers.rs b/tests/run/helpers.rs index 801fd27..14f2bbf 100644 --- a/tests/run/helpers.rs +++ b/tests/run/helpers.rs @@ -71,10 +71,8 @@ pub fn wire_path(path: &Path) -> String { /// A `__fixture` invocation as a `command_check` command line. /// -/// The grader hands the string to the platform shell, so it has to parse the -/// same under `sh -c` and `cmd /C`: a double-quoted program path followed by -/// double-quoted arguments does. One such command covers what `test`, `true`, -/// and `fc` would each have to spell differently per shell. +/// The grader hands the string to `sh -c`. Double-quoting the program path and +/// arguments preserves spaces and literal fixture values. pub fn fixture(args: &[&str]) -> String { let mut command = format!("\"{}\" __fixture", env!("CARGO_BIN_EXE_eval-magic")); for arg in args { @@ -83,23 +81,12 @@ pub fn fixture(args: &[&str]) -> String { command } -/// `fs::canonicalize` with Windows' verbatim (`\\?\`) prefix removed. -/// /// Mirrors `eval_magic::core::fs::real_path`, which the CLI applies to its own /// roots. A test that compares a path the CLI emitted against one it built from -/// `TempDir` has to resolve its side the same way, because a temp dir reaches -/// the test under an alias on both CI hosts: macOS puts it under a symlinked -/// `/var`, so the CLI's paths resolve to `/private/var/...`, and Windows hands -/// out the 8.3 short name, so `C:\Users\RUNNER~1\...` resolves to -/// `C:\Users\runneradmin\...`. The verbatim prefix is the one part that does -/// *not* survive: a child process reports the plain form as its cwd, so plain is -/// the spelling every path the CLI emits actually carries. +/// `TempDir` has to resolve its side the same way. On macOS, for example, temp +/// directories under `/var` resolve to `/private/var/...`. pub fn resolved(path: &Path) -> PathBuf { - let canonical = fs::canonicalize(path).unwrap(); - match canonical.to_string_lossy().strip_prefix(r"\\?\") { - Some(plain) => PathBuf::from(plain), - None => canonical, - } + fs::canonicalize(path).unwrap() } /// The ref a task environment carries at the state the agent started from. diff --git a/tests/run/runbook.rs b/tests/run/runbook.rs index 3395334..a7e2dc0 100644 --- a/tests/run/runbook.rs +++ b/tests/run/runbook.rs @@ -119,13 +119,12 @@ fn run_writes_headless_runbook_for_claude() { ); assert!(!book.contains("{{"), "no unsubstituted tokens: {book}"); - // Issue #248: the runbook is the manual for a campaign, so it names the - // shell it expects — and names it *above* the first command a reader would - // paste, which is the whole point of stating the requirement at all. + // The runbook is the manual for a campaign, so it names the shell it + // expects above the first command a reader would paste. let requirement = book - .find("Git Bash") - .expect("the runbook states the POSIX shell requirement"); - assert!(book.contains("WSL"), "{book}"); + .find("WSL") + .expect("the runbook states the Windows-through-WSL requirement"); + assert!(!book.contains("Git Bash"), "{book}"); // Anchored at a line start: the requirement prose names the command too, // and what this pins is the order of the *pasteable* line against it. assert!( @@ -134,10 +133,8 @@ fn run_writes_headless_runbook_for_claude() { ); } -/// Issue #248: `run` used to succeed on a host with no POSIX shell and print -/// recipes only a POSIX shell can execute, with nothing to say a different shell -/// was expected. The prepared workspace is still correct, so this warns rather -/// than failing — but it must warn, and it must name the way out. +/// A prepared workspace remains correct when the host has no POSIX shell, so +/// `run` warns and names the required environment rather than failing. /// /// `EVAL_MAGIC_SH` pointing at nothing reproduces the shell-less host on every /// platform, so the test does not depend on what the developer has installed. @@ -159,7 +156,8 @@ fn run_warns_when_the_host_has_no_posix_shell() { ]) .assert() .success() - .stderr(contains("⚠").and(contains("Git Bash")).and(contains("WSL"))); + .stderr(contains("⚠").and(contains("WSL"))) + .stderr(contains("Git Bash").not()); } #[test] @@ -196,9 +194,9 @@ fn run_writes_headless_runbook_for_opencode() { "no unsubstituted tokens: {manifest}" ); // Dispatch shells out to POSIX command lines, so the manifest states the - // same requirement the runbook does (issue #248 names both artifacts). + // same requirement as the runbook. assert!( - manifest.contains("Git Bash"), + manifest.contains("WSL") && !manifest.contains("Git Bash"), "the manifest states the POSIX shell requirement: {manifest}" ); }