From 70132921dd8df0fcaff743a17754839b696e2e55 Mon Sep 17 00:00:00 2001 From: EVMCOD <214792911+EVMCOD@users.noreply.github.com> Date: Thu, 29 Oct 2026 16:42:38 +0100 Subject: [PATCH 1/5] Windows base (2/2): windows-latest compile and tests job (not required) New workflow windows.yml: cargo check --locked --all-targets of the Tauri wrapper for Windows plus url-policy tests, and the pytest suite on Windows minus the files in tests/windows_skips.py (each with a reason, applied on win32 only). No signing, release, artifact or secret; SHA-pinned actions; stacked on PR 8. --- .github/workflows/windows.yml | 110 +++++++++++++++++++++++++++++++++ CHANGELOG.md | 1 + docs/PORTABILITY.md | 8 +++ tests/conftest.py | 14 +++++ tests/test_windows_workflow.py | 103 ++++++++++++++++++++++++++++++ tests/windows_skips.py | 19 ++++++ 6 files changed, 255 insertions(+) create mode 100644 .github/workflows/windows.yml create mode 100644 tests/test_windows_workflow.py create mode 100644 tests/windows_skips.py diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml new file mode 100644 index 0000000..d4c557c --- /dev/null +++ b/.github/workflows/windows.yml @@ -0,0 +1,110 @@ +name: Windows (compile and tests) + +# Windows is NOT a supported platform yet (docs/PORTABILITY.md). This workflow +# only answers two questions on every change that could affect it, so a Windows +# regression is seen before anyone ships Windows: +# 1. does the Tauri wrapper still COMPILE for Windows (cargo check), and +# 2. does the Windows-safe part of the Python suite still pass on Windows. +# It builds no installer, signs nothing, publishes nothing, uploads no artifact +# and uses no secret. It is not a required check (branch protection does not +# list it), so a red run here does not block a merge; look at it when it is red. +# Runs only on GitHub-hosted runners (this repo is public); never a +# self-hosted one (tests/test_workflows_guard.py). +on: + workflow_dispatch: + push: + branches: [main] + paths: &paths + - "ops_dashboard/**" + - "tests/**" + - "desktop/**" + - "requirements*.txt" + - "pyproject.toml" + - ".github/workflows/windows.yml" + pull_request: + paths: *paths + +permissions: + contents: read + +concurrency: + group: windows-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + windows-python: + # The Python suite on Windows. Tests that need something Windows does not + # have (bubblewrap/seatbelt, bash scripts, AF_UNIX agent sockets, POSIX + # permission bits) are skipped with an explicit reason: see + # tests/windows_skips.py, which tests/conftest.py applies on win32 only. + runs-on: windows-latest + timeout-minutes: 45 + defaults: + run: + shell: bash + env: + # The sidecar will be started with UTF-8 mode on Windows (plan item 8); + # the CI matches that. Encoding problems of the default cp1252 are not + # what this job looks for. + PYTHONUTF8: "1" + steps: + - name: Keep line endings as they are in the repository + run: git config --global core.autocrlf false + + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 + with: + python-version: "3.14" + cache: pip + cache-dependency-path: requirements*.txt + + - name: Install dev dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements-dev.txt + + - name: Test (pytest, Windows-safe subset) + # Not run here: the live license worker, the two headless-Chrome JS + # tests and the *_ui_browser tests (they need Chrome/Playwright and + # run in ci.yml), and no coverage floor (ci.yml owns it). + run: > + pytest -m "not live" -p no:cacheprovider -q -ra + --ignore=tests/test_license_worker.py + --ignore=tests/test_browser_visibility_js.py + --ignore=tests/test_desktop_nav_js.py + --ignore-glob="tests/test_*ui_browser.py" + + windows-rust: + # Compile-only: cargo check of the Tauri wrapper for the Windows target + # (this is where a missing #[cfg(windows)] branch shows up) and the tests of + # the pure-Rust URL policy. The wrapper's own tests are not run: they would + # link the WebView2 loader, and nothing here launches the app. + runs-on: windows-latest + timeout-minutes: 40 + defaults: + run: + shell: bash + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: dtolnay/rust-toolchain@b725eda525362e7cf34af9a7243cac1e3f01a7ec # stable + with: + toolchain: stable + + - uses: Swatinem/rust-cache@d1527c8f67ef9af357290475d4af4b0fab8f12e0 # v2.9.3 + with: + workspaces: desktop/src-tauri + + - name: Placeholder sidecar (tauri-build checks externalBin exists) + run: | + mkdir -p desktop/src-tauri/binaries + triple="$(rustc -vV | sed -n 's/host: //p' | tr -d '\r')" + : > "desktop/src-tauri/binaries/conductloop-server-${triple}.exe" + + - name: cargo check (locked) + working-directory: desktop/src-tauri + run: cargo check --locked --all-targets + + - name: cargo test (url-policy) + run: cargo test --manifest-path desktop/url-policy/Cargo.toml diff --git a/CHANGELOG.md b/CHANGELOG.md index 93ea02a..35bb3b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ User-facing changes to ConductLoop (the desktop app and its local server), newes ## Unreleased +- CI only: a "Windows (compile and tests)" workflow checks on every relevant pull request that the desktop wrapper still compiles for Windows and that the Windows-safe part of the Python tests passes there. It builds no installer, signs nothing, publishes nothing and is not a required check. Windows is still not supported. - **Windows groundwork: four fixes that cannot be tested on a real Windows machine yet.** The check that the desktop app is still running no longer ends the app on Windows (it used a signal that Windows treats as "terminate"); the local server refuses to start when its port is already taken, instead of sharing it with another program; the app token and server nonce are generated on Windows as well; and setting private file permissions is a documented no-op there instead of an error. macOS and Linux behave exactly as before. Windows itself is still not supported: there is no installer and no secure key storage yet. - Linux: a CI workflow, "Linux bundles", builds the AppImage and the .deb on every pull request that touches the desktop app and keeps them for 3 days as downloadable artifacts. It builds on `ubuntu-22.04` instead of `ubuntu-latest` so the AppImage works with an older glibc; it is the only job pinned to a runner image. CI only: the app does not change. - **Linux: a script to build an AppImage and a .deb.** `scripts/build-linux.sh` freezes the server with PyInstaller and runs `tauri build --bundles appimage,deb`; the Linux settings live in `desktop/src-tauri/tauri.linux.conf.json`, so the macOS build does not change. No signing and no updates on Linux (the app does not look for updates at start-up there). Experimental: the packages have not been installed or run on a Linux desktop yet. diff --git a/docs/PORTABILITY.md b/docs/PORTABILITY.md index eefda2e..c5b2f6e 100644 --- a/docs/PORTABILITY.md +++ b/docs/PORTABILITY.md @@ -773,6 +773,14 @@ byte-for-byte (see "Invariantes de macOS"). "Tests" are what that PR must add. 9. **`windows-latest` compile-only job** (Rust check and the Windows-safe Python subset, which also runs the real Windows tests of PR 8). *Tests:* the job itself; `test_workflows_guard.py`. + **Done in PR 9** (`.github/workflows/windows.yml`, stacked on PR 8): two + `windows-latest` jobs. `windows-rust` runs `cargo check --locked + --all-targets` of `desktop/src-tauri` (with a placeholder sidecar) and the + `desktop/url-policy` tests. `windows-python` runs the pytest suite + (`-m "not live"`, with `PYTHONUTF8=1`) minus the files listed with a reason in + `tests/windows_skips.py` (sandbox, bash scripts, AF_UNIX bridge), applied by + `tests/conftest.py` on `win32` only. It builds, signs and publishes nothing, + uses no secret and is not a required check. 10. **Windows Credential Manager backend** (with the blob-size handling). *Tests:* selector and size-limit tests with a fake backend. 11. **Windows packaging (NSIS), Help menu entry and release-manifest design:** a diff --git a/tests/conftest.py b/tests/conftest.py index 3213705..1380d04 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -54,3 +54,17 @@ def _vault_open(): secret_vault.unlock(TEST_PASSPHRASE) yield secret_vault.lock() + + +def pytest_collection_modifyitems(config, items): + """On Windows, skip the files of tests/windows_skips.py, with their reason.""" + import sys + + if sys.platform != "win32": + return + from windows_skips import SKIPPED_ON_WINDOWS + + for item in items: + reason = SKIPPED_ON_WINDOWS.get(Path(str(item.fspath)).name) + if reason: + item.add_marker(pytest.mark.skip(reason=f"not on Windows: {reason}")) diff --git a/tests/test_windows_workflow.py b/tests/test_windows_workflow.py new file mode 100644 index 0000000..4bbbd41 --- /dev/null +++ b/tests/test_windows_workflow.py @@ -0,0 +1,103 @@ +"""Guards on the Windows CI job (.github/workflows/windows.yml). + +It is a compile-and-test job and must stay one: no signing, no release, no +artifact, no secret, not a required check. And the list of tests skipped on +Windows must stay honest: every entry exists and says why. +""" +from __future__ import annotations + +import re +import sys +from pathlib import Path +from types import SimpleNamespace + +import yaml + +ROOT = Path(__file__).resolve().parent.parent +TESTS = ROOT / "tests" +WORKFLOW = ROOT / ".github" / "workflows" / "windows.yml" +TEXT = WORKFLOW.read_text(encoding="utf-8") +DOC = yaml.safe_load(TEXT) +sys.path.insert(0, str(TESTS)) + +from windows_skips import SKIPPED_ON_WINDOWS # noqa: E402 + + +def _steps(): + for name, job in DOC["jobs"].items(): + for step in job["steps"]: + yield name, step + + +def test_every_job_runs_on_windows_latest_with_a_timeout(): + assert set(DOC["jobs"]) == {"windows-python", "windows-rust"} + for name, job in DOC["jobs"].items(): + assert job["runs-on"] == "windows-latest", name + assert job["timeout-minutes"] <= 60, name + + +def test_it_cannot_publish_sign_or_read_secrets(): + code = "\n".join(re.sub(r"(^|\s)#.*$", "", line) for line in TEXT.splitlines()) + for word in ("secrets.", "upload-artifact", "gh release", "tauri build", "tauri-action", + "signtool", "codesign", "notariz", "APPLE_", "TAURI_SIGNING", "GITHUB_TOKEN", + "release-desktop", "git push", "vars."): + assert word not in code, f"{word!r} does not belong in a compile-and-test job" + assert DOC["permissions"] == {"contents": "read"} + for _, step in _steps(): + assert "env" not in step or not any("secret" in str(v).lower() for v in step["env"].values()) + + +def test_it_is_a_check_only_job_for_rust(): + runs = [s.get("run", "") for n, s in _steps() if n == "windows-rust"] + joined = "\n".join(runs) + assert "cargo check --locked --all-targets" in joined + assert "cargo build" not in joined and "tauri" not in joined.replace("src-tauri", "").replace("tauri-build", "") + + +def test_the_python_job_runs_pytest_without_the_live_worker(): + runs = "\n".join(s.get("run", "") for n, s in _steps() if n == "windows-python") + assert 'pytest -m "not live"' in runs + assert "--ignore=tests/test_license_worker.py" in runs + + +def test_it_is_not_a_required_check(): + """Required checks live in branch protection, not in the repo; the names a + job reports must not collide with the two that are required there.""" + for job in DOC["jobs"].values(): + assert job.get("name", "") not in {"test", "gitleaks"} + assert set(DOC["jobs"]).isdisjoint({"test", "gitleaks"}) + + +def test_every_skipped_file_exists_and_has_a_reason(): + assert SKIPPED_ON_WINDOWS + for name, reason in SKIPPED_ON_WINDOWS.items(): + assert (TESTS / name).is_file(), f"{name} is listed but does not exist" + assert len(reason.split()) >= 4, f"{name}: say why it cannot run on Windows" + assert not any(n.startswith("test_windows") for n in SKIPPED_ON_WINDOWS), \ + "the Windows tests themselves must run on Windows" + + +def _items(*names): + return [SimpleNamespace(fspath=str(TESTS / n), marks=[], add_marker=lambda m, _n=n: marks.setdefault(_n, []).append(m)) + for n in names] + + +marks: dict[str, list] = {} + + +def test_the_conftest_hook_skips_only_on_windows(monkeypatch): + import conftest + victim = next(iter(SKIPPED_ON_WINDOWS)) + items = _items(victim, "test_auth.py") + + marks.clear() + monkeypatch.setattr(sys, "platform", "linux") + conftest.pytest_collection_modifyitems(None, items) + assert marks == {}, "nothing may be skipped on macOS/Linux" + + monkeypatch.setattr(sys, "platform", "win32") + conftest.pytest_collection_modifyitems(None, items) + assert list(marks) == [victim] + mark = marks[victim][0] + assert mark.name == "skip" and SKIPPED_ON_WINDOWS[victim] in mark.kwargs["reason"] + diff --git a/tests/windows_skips.py b/tests/windows_skips.py new file mode 100644 index 0000000..71d229c --- /dev/null +++ b/tests/windows_skips.py @@ -0,0 +1,19 @@ +"""Test files that are skipped on Windows, each with the reason. + +Applied by tests/conftest.py on ``sys.platform == "win32"`` only (the +``windows-latest`` job of .github/workflows/windows.yml); nothing changes on +macOS and Linux. A file belongs here only when what it tests does not exist on +Windows, never to hide a failure: a real Windows bug is fixed, or skipped with +the reason, in the same change. tests/test_windows_workflow.py checks that every +file listed exists and every reason is written down. +""" +from __future__ import annotations + +SKIPPED_ON_WINDOWS: dict[str, str] = { + "test_sandbox_sockets.py": "tests the Linux bubblewrap socket scan (AF_UNIX sockets, bwrap); no OS sandbox on Windows", + "test_sessions_code_tools.py": "runs commands inside bubblewrap/seatbelt; code sessions are disabled on Windows", + "test_sessions_worktrees.py": "drives git worktrees through POSIX shell scripts; code sessions are disabled on Windows", + "test_verify_script.py": "runs scripts/verify.sh (bash) with POSIX tools", + "test_linux_packaging.py": "checks scripts/build-linux.sh and Linux packaging files (bash)", + "test_browser_agent.py": "uses AF_UNIX sockets for the agent bridge; Windows has no bridge (docs/PORTABILITY.md 3.3)", +} From 51c354263d8137f99efdb41e401c252de129bef8 Mon Sep 17 00:00:00 2001 From: EVMCOD <214792911+EVMCOD@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:13:04 +0100 Subject: [PATCH 2/5] Windows CI: fix the evals temp-dir cleanup, skip Windows-impossible tests with reasons First windows-python run: 1676 passed, 108 failed. 40 were the evals harness failing to delete a temp dir holding an open sqlite file (ignore_cleanup_errors). One test helper compared a backslash path. The rest (code sessions, macOS keychain backend, POSIX permission bits, POSIX paths, vault/skills tools not yet investigated) are skipped on win32 only, each with a reason. --- docs/PORTABILITY.md | 6 ++++++ ops_dashboard/evals/record.py | 2 +- ops_dashboard/evals/runner.py | 5 ++++- tests/conftest.py | 4 +++- tests/test_skill_proposals.py | 2 +- tests/test_windows_workflow.py | 16 +++++++++++++--- tests/windows_skips.py | 25 +++++++++++++++++++++++++ 7 files changed, 53 insertions(+), 7 deletions(-) diff --git a/docs/PORTABILITY.md b/docs/PORTABILITY.md index c5b2f6e..55aba44 100644 --- a/docs/PORTABILITY.md +++ b/docs/PORTABILITY.md @@ -781,6 +781,12 @@ byte-for-byte (see "Invariantes de macOS"). "Tests" are what that PR must add. `tests/windows_skips.py` (sandbox, bash scripts, AF_UNIX bridge), applied by `tests/conftest.py` on `win32` only. It builds, signs and publishes nothing, uses no secret and is not a required check. + The first run found 108 Windows failures (1676 tests passed, including the + real-Windows tests of PR 8): the evals harness leaked an open sqlite file into + a temp dir it then could not delete (fixed: `ignore_cleanup_errors`), and the + rest is skipped with a written reason in `tests/windows_skips.py`; the entries + marked "not investigated" (vault index/notes tools, skills compat, one HTTP + test) are the to-do list for Windows support. 10. **Windows Credential Manager backend** (with the blob-size handling). *Tests:* selector and size-limit tests with a fake backend. 11. **Windows packaging (NSIS), Help menu entry and release-manifest design:** a diff --git a/ops_dashboard/evals/record.py b/ops_dashboard/evals/record.py index 892030f..b95e777 100644 --- a/ops_dashboard/evals/record.py +++ b/ops_dashboard/evals/record.py @@ -132,7 +132,7 @@ def record_case(suite: _runner.Suite, case: _runner.Case, *, provider: str, mode if suite.target != "brain.ask": raise RecordRefused("no_cassette_target", f"suite {suite.name} does not use cassettes") rec = RecordingProvider(provider_builder(provider, model, key_name)) - with tempfile.TemporaryDirectory(prefix="cl-eval-rec-") as td: + with tempfile.TemporaryDirectory(prefix="cl-eval-rec-", ignore_cleanup_errors=True) as td: with _runner.case_env(suite, case, Path(td)): from .. import brain orig = brain.resolve_provider diff --git a/ops_dashboard/evals/runner.py b/ops_dashboard/evals/runner.py index fdfedb0..f2b681b 100644 --- a/ops_dashboard/evals/runner.py +++ b/ops_dashboard/evals/runner.py @@ -363,7 +363,10 @@ def run_case(suite: Suite, case: Case, *, guard: Any = None, error: str | None = None results: list[dict] = [] before = len(guard.attempts) if guard is not None else 0 - with tempfile.TemporaryDirectory(prefix="cl-eval-") as td: + # ignore_cleanup_errors: the case's sqlite file may still be open (a connection + # not yet garbage-collected); Windows refuses to delete it, and a throwaway + # directory that cannot be removed must not fail the case. + with tempfile.TemporaryDirectory(prefix="cl-eval-", ignore_cleanup_errors=True) as td: provider: Any = None try: with case_env(suite, case, Path(td)) as patches: diff --git a/tests/conftest.py b/tests/conftest.py index 1380d04..d6ec815 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -65,6 +65,8 @@ def pytest_collection_modifyitems(config, items): from windows_skips import SKIPPED_ON_WINDOWS for item in items: - reason = SKIPPED_ON_WINDOWS.get(Path(str(item.fspath)).name) + name = Path(str(item.fspath)).name + test = getattr(item, "originalname", None) or item.name + reason = SKIPPED_ON_WINDOWS.get(name) or SKIPPED_ON_WINDOWS.get(f"{name}::{test}") if reason: item.add_marker(pytest.mark.skip(reason=f"not on Windows: {reason}")) diff --git a/tests/test_skill_proposals.py b/tests/test_skill_proposals.py index 2da707f..aa4a878 100644 --- a/tests/test_skill_proposals.py +++ b/tests/test_skill_proposals.py @@ -108,7 +108,7 @@ def resolve(name=None): def _files_under(root: Path) -> list[str]: - return sorted(str(p.relative_to(root)) for p in root.rglob("*") if p.is_file()) + return sorted(p.relative_to(root).as_posix() for p in root.rglob("*") if p.is_file()) # ---------- migration ---------- diff --git a/tests/test_windows_workflow.py b/tests/test_windows_workflow.py index 4bbbd41..d57f478 100644 --- a/tests/test_windows_workflow.py +++ b/tests/test_windows_workflow.py @@ -71,14 +71,18 @@ def test_it_is_not_a_required_check(): def test_every_skipped_file_exists_and_has_a_reason(): assert SKIPPED_ON_WINDOWS for name, reason in SKIPPED_ON_WINDOWS.items(): - assert (TESTS / name).is_file(), f"{name} is listed but does not exist" + file, _, test = name.partition("::") + assert (TESTS / file).is_file(), f"{name} is listed but does not exist" + if test: + assert re.search(rf"^\s*def {re.escape(test)}\(", (TESTS / file).read_text(encoding="utf-8"), re.M), \ + f"{name}: no such test" assert len(reason.split()) >= 4, f"{name}: say why it cannot run on Windows" assert not any(n.startswith("test_windows") for n in SKIPPED_ON_WINDOWS), \ "the Windows tests themselves must run on Windows" def _items(*names): - return [SimpleNamespace(fspath=str(TESTS / n), marks=[], add_marker=lambda m, _n=n: marks.setdefault(_n, []).append(m)) + return [SimpleNamespace(fspath=str(TESTS / n), name="test_x", marks=[], add_marker=lambda m, _n=n: marks.setdefault(_n, []).append(m)) for n in names] @@ -89,6 +93,12 @@ def test_the_conftest_hook_skips_only_on_windows(monkeypatch): import conftest victim = next(iter(SKIPPED_ON_WINDOWS)) items = _items(victim, "test_auth.py") + node = next(n for n in SKIPPED_ON_WINDOWS if "::" in n) + nfile, _, ntest = node.partition("::") + items.append(SimpleNamespace(fspath=str(TESTS / nfile), name=ntest, marks=[], + add_marker=lambda m: marks.setdefault(node, []).append(m))) + items.append(SimpleNamespace(fspath=str(TESTS / nfile), name="test_some_other_test", marks=[], + add_marker=lambda m: marks.setdefault("other", []).append(m))) marks.clear() monkeypatch.setattr(sys, "platform", "linux") @@ -97,7 +107,7 @@ def test_the_conftest_hook_skips_only_on_windows(monkeypatch): monkeypatch.setattr(sys, "platform", "win32") conftest.pytest_collection_modifyitems(None, items) - assert list(marks) == [victim] + assert sorted(marks) == sorted([victim, node]) mark = marks[victim][0] assert mark.name == "skip" and SKIPPED_ON_WINDOWS[victim] in mark.kwargs["reason"] diff --git a/tests/windows_skips.py b/tests/windows_skips.py index 71d229c..2f415d1 100644 --- a/tests/windows_skips.py +++ b/tests/windows_skips.py @@ -6,6 +6,11 @@ Windows, never to hide a failure: a real Windows bug is fixed, or skipped with the reason, in the same change. tests/test_windows_workflow.py checks that every file listed exists and every reason is written down. + +A key is either a file name or ``file.py::test_name`` for a single test. The +reasons marked "not investigated" are real Windows failures seen in the first +run of the job that are not small enough to fix here: they are the to-do list +for making Windows work, not a statement that the feature is fine. """ from __future__ import annotations @@ -16,4 +21,24 @@ "test_verify_script.py": "runs scripts/verify.sh (bash) with POSIX tools", "test_linux_packaging.py": "checks scripts/build-linux.sh and Linux packaging files (bash)", "test_browser_agent.py": "uses AF_UNIX sockets for the agent bridge; Windows has no bridge (docs/PORTABILITY.md 3.3)", + # Code sessions are switched off on Windows (platform_caps): the API refuses. + "test_sessions_api.py": "code sessions are turned off on Windows (platform_unsupported), so the API refuses to start them", + "test_sessions_supervisor.py": "needs code sessions and git worktrees, which are turned off on Windows", + # The macOS keychain backend (the `security` tool); Windows gets its own in PR 10 of the plan. + "test_secrets_store_keychain.py": "tests the macOS `security` keychain backend; the Windows Credential Manager backend does not exist yet", + # Real failures, not investigated yet (see the module docstring). + "test_vault_index_drift.py": "not investigated: the vault index tools fail on Windows paths", + "test_vault_conventions.py": "not investigated: the vault index preview fails on Windows paths", + "test_notes_structure.py": "not investigated: the notes structure tools fail on Windows paths", + "test_skills_compat.py": "not investigated: skill directory resolution and symlinks differ on Windows", + # Single tests. + "test_byok_keys.py::test_config_written_owner_only": "asserts POSIX mode 0600; Windows has no such permission bits (docs/PORTABILITY.md 4.3)", + "test_secrets_store.py::test_file_is_owner_only": "asserts POSIX mode 0600; Windows has no such permission bits (docs/PORTABILITY.md 4.3)", + "test_learning_feedback.py::test_export_is_local_anonymised_and_private": "asserts POSIX mode 0600; Windows has no such permission bits (docs/PORTABILITY.md 4.3)", + "test_token_kit.py::test_config_validation": "asserts POSIX mode 0600; Windows has no such permission bits (docs/PORTABILITY.md 4.3)", + "test_linux_bundles_workflow.py::test_the_script_it_calls_exists_and_is_executable": "asserts the POSIX executable bit of a shell script", + "test_release_check.py::test_not_a_git_repo_fails": "the test's cleanup cannot delete read-only .git objects on Windows", + "test_media.py::test_assemble_refuses_files_outside_the_data_directory": "uses a POSIX /tmp path", + "test_lessons.py::test_scope_validation": "uses a POSIX absolute path (repo:/...), which is not absolute on Windows", + "test_integration_e2e.py::test_analytics_and_setup_over_http": "not investigated: times out on Windows (possibly localhost resolving to ::1 first, docs/PORTABILITY.md 4.2)", } From c5fb0849531cb8636026cfd37f05b007afeca8e6 Mon Sep 17 00:00:00 2001 From: EVMCOD <214792911+EVMCOD@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:33:29 +0100 Subject: [PATCH 3/5] Evals checks: compare document paths with forward slashes (Windows indexes backslashes) Last failure of the first Windows run: the RAG eval cases check a posix path suffix against citations whose path has backslashes on Windows. Linux/macOS unchanged. --- ops_dashboard/evals/checks.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ops_dashboard/evals/checks.py b/ops_dashboard/evals/checks.py index ae9594a..c74d761 100644 --- a/ops_dashboard/evals/checks.py +++ b/ops_dashboard/evals/checks.py @@ -82,6 +82,11 @@ class CheckSpecError(ValueError): _CITE_RE = re.compile(r"\[(\d{1,3})\]") +def _posix(path: Any) -> str: + """A document path with forward slashes: Windows indexes ``a\\b.md``, the cases say ``a/b.md``.""" + return str(path).replace("\\", "/") + + def _req(spec: dict, key: str) -> Any: if key not in spec: raise CheckSpecError(f"check {spec.get('type')!r} needs {key!r}") @@ -136,7 +141,7 @@ def _check(o: Outcome, spec: dict) -> CheckResult: # noqa: C901 - flat dispatch + (f"; invalid {invalid}" if invalid else "")) if t == "cites": path = str(_req(spec, "path")) - nums = [c["n"] for c in o.citations if str(c.get("path", "")).endswith(path)] + nums = [c["n"] for c in o.citations if _posix(c.get("path", "")).endswith(path)] answer_refs = {int(x) for x in _CITE_RE.findall(o.answer)} ok = any(n in answer_refs for n in nums) return CheckResult(t, ok, f"{path} is citation {nums or 'none'}; answer cites {sorted(answer_refs)}") @@ -144,7 +149,7 @@ def _check(o: Outcome, spec: dict) -> CheckResult: # noqa: C901 - flat dispatch path = str(_req(spec, "path")) top = spec.get("within_top") pool = o.citations[: int(top)] if top else o.citations - ok = any(str(c.get("path", "")).endswith(path) for c in pool) + ok = any(_posix(c.get("path", "")).endswith(path) for c in pool) return CheckResult(t, ok, f"{path} {'in' if ok else 'not in'} " f"{'top ' + str(top) if top else 'citations'} ({len(o.citations)} total)") if t == "citations_min": @@ -172,7 +177,7 @@ def _check(o: Outcome, spec: dict) -> CheckResult: # noqa: C901 - flat dispatch path = str(_req(spec, "path")) top = spec.get("within_top") pool = o.results[: int(top)] if top else o.results - ok = any(str(r.get("path", "")).endswith(path) for r in pool) + ok = any(_posix(r.get("path", "")).endswith(path) for r in pool) return CheckResult(t, ok, f"{path} {'in' if ok else 'not in'} {len(o.results)} result(s)") if t == "result_title": title = str(_req(spec, "text")) From 0aaff85babef78c2bc00a542b1079e5545ee54b8 Mon Sep 17 00:00:00 2001 From: EVMCOD <214792911+EVMCOD@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:51:41 +0100 Subject: [PATCH 4/5] Windows tests: mark POSIX-permission tests systematically and guard against new ones - posix_perms marker (skipped on win32 by conftest, with the reason) for tests that are about permission bits; tests/posix_perms.py helpers (no-ops on Windows) for tests where the bits are one assertion among others, so the rest still runs on Windows. - tests/test_posix_perms_guard.py fails if a test reads st_mode/S_IMODE/S_I*/ os.access(R/W/X_OK) without the marker or a helper. - Per-test entries for permission tests removed from windows_skips.py. macOS/Linux assert exactly what they did. --- docs/PORTABILITY.md | 6 + pytest.ini | 3 +- tests/conftest.py | 9 +- tests/posix_perms.py | 52 ++++++++ tests/test_byok_keys.py | 1 + tests/test_learning_feedback.py | 4 +- tests/test_linux_bundles_workflow.py | 2 + tests/test_linux_packaging.py | 1 + tests/test_posix_perms_guard.py | 138 +++++++++++++++++++++ tests/test_secrets_store.py | 1 + tests/test_secrets_store_secret_service.py | 5 +- tests/test_token_kit.py | 4 +- tests/test_verify_script.py | 1 + tests/test_windows_portability.py | 3 +- tests/test_windows_workflow.py | 12 +- tests/windows_skips.py | 8 +- 16 files changed, 232 insertions(+), 18 deletions(-) create mode 100644 tests/posix_perms.py create mode 100644 tests/test_posix_perms_guard.py diff --git a/docs/PORTABILITY.md b/docs/PORTABILITY.md index e208214..6ac9bbe 100644 --- a/docs/PORTABILITY.md +++ b/docs/PORTABILITY.md @@ -874,6 +874,12 @@ byte-for-byte (see "Invariantes de macOS"). "Tests" are what that PR must add. rest is skipped with a written reason in `tests/windows_skips.py`; the entries marked "not investigated" (vault index/notes tools, skills compat, one HTTP test) are the to-do list for Windows support. + Tests about POSIX permission bits (`st_mode`, `S_IMODE`, the executable bit, + `os.access`) are not listed one by one: they carry `@pytest.mark.posix_perms` + (skipped on `win32` by `tests/conftest.py`) or assert through + `tests/posix_perms.py`, whose helpers do nothing on Windows and leave the rest + of the test running. `tests/test_posix_perms_guard.py` fails when a test reads + permission bits without one of the two. 10. **Windows Credential Manager backend** (with the blob-size handling). *Tests:* selector and size-limit tests with a fake backend. 11. **Windows packaging (NSIS), Help menu entry and release-manifest design:** a diff --git a/pytest.ini b/pytest.ini index 24f6a4c..715dfde 100644 --- a/pytest.ini +++ b/pytest.ini @@ -12,4 +12,5 @@ python_functions = test_* addopts = -v --tb=short --strict-markers -m "not live" markers = live: hits the live Cloudflare Worker (requires network) - slow: tests that take >5 seconds \ No newline at end of file + slow: tests that take >5 seconds + posix_perms: asserts POSIX permission bits (st_mode, chmod, executable bit); skipped on Windows (tests/posix_perms.py) \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index d6ec815..92a185e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -57,11 +57,16 @@ def _vault_open(): def pytest_collection_modifyitems(config, items): - """On Windows, skip the files of tests/windows_skips.py, with their reason.""" + """On Windows, skip what cannot exist there, each with its reason. + + * the files and tests of tests/windows_skips.py; + * every test marked ``posix_perms`` (asserts POSIX permission bits). + """ import sys if sys.platform != "win32": return + from posix_perms import REASON as PERMS_REASON from windows_skips import SKIPPED_ON_WINDOWS for item in items: @@ -70,3 +75,5 @@ def pytest_collection_modifyitems(config, items): reason = SKIPPED_ON_WINDOWS.get(name) or SKIPPED_ON_WINDOWS.get(f"{name}::{test}") if reason: item.add_marker(pytest.mark.skip(reason=f"not on Windows: {reason}")) + elif item.get_closest_marker("posix_perms"): + item.add_marker(pytest.mark.skip(reason=f"not on Windows: {PERMS_REASON}")) diff --git a/tests/posix_perms.py b/tests/posix_perms.py new file mode 100644 index 0000000..f8895d8 --- /dev/null +++ b/tests/posix_perms.py @@ -0,0 +1,52 @@ +"""Assertions about POSIX permission bits, which Windows does not have. + +``chmod(0o600)`` only toggles the read-only attribute on Windows and ``st_mode`` +reports 0o666 for ordinary files, so a test that asserts "owner only" or "is +executable" cannot pass there, and it is not a bug (docs/PORTABILITY.md 4.3, +``ops_dashboard/fileperms.py``). Two ways to deal with such a test, and the only +two the guard in tests/test_windows_workflow.py accepts: + +* the whole test is about the bits: mark it ``@pytest.mark.posix_perms`` (skipped + on Windows by tests/conftest.py, with the reason below); +* the bits are one assertion among others: make that assertion with one of the + helpers here, which do nothing on Windows, and keep the rest of the test + running there. + +macOS and Linux assert exactly what they asserted before. +""" +from __future__ import annotations + +import os +import stat +import sys +from pathlib import Path + +REASON = ("POSIX permission bits (st_mode, chmod, the executable bit) do not exist on Windows " + "(docs/PORTABILITY.md 4.3)") + + +def on_windows() -> bool: + return sys.platform == "win32" + + +def assert_mode(path: str | os.PathLike[str], expected: int) -> None: + """``S_IMODE(stat(path)) == expected``, except on Windows.""" + if on_windows(): + return + actual = stat.S_IMODE(Path(path).stat().st_mode) + assert actual == expected, f"{path}: mode {oct(actual)}, expected {oct(expected)}" + + +def assert_no_group_other_access(path: str | os.PathLike[str]) -> None: + """No permission bit for group or others, except on Windows.""" + if on_windows(): + return + actual = stat.S_IMODE(Path(path).stat().st_mode) + assert actual & 0o077 == 0, f"{path}: mode {oct(actual)} gives access to group/others" + + +def assert_executable(path: str | os.PathLike[str]) -> None: + """The file has an execute bit, except on Windows.""" + if on_windows(): + return + assert Path(path).stat().st_mode & 0o111, f"{path} is not executable" diff --git a/tests/test_byok_keys.py b/tests/test_byok_keys.py index 6ea8139..177853e 100644 --- a/tests/test_byok_keys.py +++ b/tests/test_byok_keys.py @@ -127,6 +127,7 @@ def test_empty_key_clears_both(monkeypatch): assert _cfg() == {} +@pytest.mark.posix_perms def test_config_written_owner_only(monkeypatch): FakeKeychain(enabled=False).install(monkeypatch) brain.set_provider_key("openai", api_key=FAKE) diff --git a/tests/test_learning_feedback.py b/tests/test_learning_feedback.py index 2941df0..4a1cfb0 100644 --- a/tests/test_learning_feedback.py +++ b/tests/test_learning_feedback.py @@ -10,7 +10,6 @@ import os import socket import sqlite3 -import stat import threading import time import urllib.error @@ -18,6 +17,7 @@ from http.server import ThreadingHTTPServer import pytest +from posix_perms import assert_mode from ops_dashboard import migrations @@ -230,7 +230,7 @@ def test_export_is_local_anonymised_and_private(tmpdb): comment="comentario") path = learning.export_local() assert path.parent == learning.store.export_dir() - assert stat.S_IMODE(os.stat(path).st_mode) == 0o600 + assert_mode(path, 0o600) text = path.read_text() assert "respuesta secreta" not in text and "comentario" not in text data = json.loads(text) diff --git a/tests/test_linux_bundles_workflow.py b/tests/test_linux_bundles_workflow.py index 04783d1..615dff8 100644 --- a/tests/test_linux_bundles_workflow.py +++ b/tests/test_linux_bundles_workflow.py @@ -10,6 +10,7 @@ import re from pathlib import Path +import pytest import yaml ROOT = Path(__file__).resolve().parent.parent @@ -59,6 +60,7 @@ def test_it_publishes_nothing_and_takes_no_secret(): assert DOC["permissions"] == {"contents": "read"} +@pytest.mark.posix_perms def test_the_script_it_calls_exists_and_is_executable(): script = ROOT / "scripts" / "build-linux.sh" assert script.is_file() and script.stat().st_mode & 0o111 diff --git a/tests/test_linux_packaging.py b/tests/test_linux_packaging.py index 4988d75..3ae4f1e 100644 --- a/tests/test_linux_packaging.py +++ b/tests/test_linux_packaging.py @@ -105,6 +105,7 @@ def test_the_macos_release_script_is_not_wired_to_the_linux_one(): assert "build-linux" not in (ROOT / "scripts" / "release-desktop.sh").read_text() +@pytest.mark.posix_perms def test_the_script_is_executable_and_valid_bash(): assert os.access(SCRIPT, os.X_OK) subprocess.run(["bash", "-n", str(SCRIPT)], check=True) diff --git a/tests/test_posix_perms_guard.py b/tests/test_posix_perms_guard.py new file mode 100644 index 0000000..83c5d1c --- /dev/null +++ b/tests/test_posix_perms_guard.py @@ -0,0 +1,138 @@ +"""A test that looks at POSIX permission bits must say so (so Windows can skip it). + +Windows has no such bits: ``chmod(0o600)`` is a no-op there and ``st_mode`` reads +0o666. A test that asserts them fails on ``windows-latest`` for a reason that is +not a bug. Rather than discovering each one in a red run, this test reads every +test file and fails when it finds one that inspects permission bits (``st_mode``, +``S_IMODE``/``S_I*`` constants, ``os.access`` with an R/W/X flag) outside the +accepted ways (see tests/posix_perms.py): + +* the test function carries ``@pytest.mark.posix_perms`` (or the module has + ``pytestmark = pytest.mark.posix_perms``); +* the check is done through the helpers of tests/posix_perms.py; +* the whole file, or that test, is in tests/windows_skips.py with a reason. + +``chmod`` used to *set up* a fixture (a fake executable) is not an inspection and +is not flagged. +""" +from __future__ import annotations + +import ast +import re +import sys +from pathlib import Path + +TESTS = Path(__file__).resolve().parent +sys.path.insert(0, str(TESTS)) + +from windows_skips import SKIPPED_ON_WINDOWS # noqa: E402 + +# The helper module and this guard talk about the bits without asserting them. +EXEMPT_FILES = {"posix_perms.py", "test_posix_perms_guard.py"} +ACCESS_FLAGS = {"R_OK", "W_OK", "X_OK"} +STAT_CONSTANT = re.compile(r"^S_(IMODE|IFMT|ISUID|ISGID|ISVTX|I[RWX](USR|GRP|OTH|EXEC)|IRWX[UGO]|IREAD|IWRITE)$") + + +def _names(node: ast.AST): + if isinstance(node, ast.Attribute): + yield node.attr + elif isinstance(node, ast.Name): + yield node.id + + +def _found(root: ast.AST): + """(line, what) for each permission-bit inspection under ``root``.""" + for n in ast.walk(root): + for name in _names(n): + if name == "st_mode" or STAT_CONSTANT.match(name): + yield n.lineno, name + if isinstance(n, ast.Call): + f = n.func + if getattr(f, "attr", getattr(f, "id", None)) == "access" and any( + flag in ACCESS_FLAGS for a in n.args for flag in _names(a)): + yield n.lineno, "os.access(..., R_OK/W_OK/X_OK)" + + +def inspections(tree: ast.Module): + """Yield (owning test function or None, line, what). Methods of a class own themselves.""" + funcs = (ast.FunctionDef, ast.AsyncFunctionDef) + for node in tree.body: + if isinstance(node, funcs): + owned = [(node, node)] + elif isinstance(node, ast.ClassDef): + owned = [(m, m) if isinstance(m, funcs) else (m, None) for m in node.body] + else: + owned = [(node, None)] + for root, owner in owned: + for line, what in _found(root): + yield owner, line, what + + +def _marked(fn) -> bool: + return fn is not None and any("posix_perms" in ast.unparse(d) for d in fn.decorator_list) + + +def _module_marked(tree: ast.Module) -> bool: + return any(isinstance(n, ast.Assign) and any(isinstance(t, ast.Name) and t.id == "pytestmark" for t in n.targets) + and "posix_perms" in ast.unparse(n.value) for n in tree.body) + + +def problems(path: Path, source: str | None = None) -> list[str]: + name = path.name + if name in EXEMPT_FILES or name in SKIPPED_ON_WINDOWS: + return [] + tree = ast.parse(source if source is not None else path.read_text(encoding="utf-8")) + if _module_marked(tree): + return [] + out = [] + for fn, line, what in inspections(tree): + if _marked(fn) or (fn is not None and f"{name}::{fn.name}" in SKIPPED_ON_WINDOWS): + continue + where = f"{fn.name}()" if fn is not None else "module level" + out.append(f"{name}:{line}: {what} in {where}") + return sorted(set(out)) + + +def test_no_test_inspects_permission_bits_without_saying_so(): + files = sorted(TESTS.glob("test_*.py")) + assert files + found = [p for f in files for p in problems(f)] + assert not found, ( + "These tests look at POSIX permission bits, which Windows does not have. Mark the test " + "@pytest.mark.posix_perms, or assert through tests/posix_perms.py (assert_mode, " + "assert_no_group_other_access, assert_executable):\n " + "\n ".join(found)) + + +# ── the detector itself: it must not pass on nothing ───────────────────────── + +BAD = ''' +import os, stat +def test_a(tmp_path): + assert stat.S_IMODE(tmp_path.stat().st_mode) == 0o700 +''' + + +def test_the_detector_flags_each_way_of_reading_the_bits(): + for snippet, what in [ + ("def test_x(p):\n assert p.stat().st_mode & 0o111\n", "st_mode"), + ("import stat\ndef test_x(p):\n assert stat.S_IMODE(1)\n", "S_IMODE"), + ("from stat import S_IXUSR\ndef test_x(p):\n assert p & S_IXUSR\n", "S_IXUSR"), + ("import os\ndef test_x(p):\n assert os.access(p, os.X_OK)\n", "os.access"), + ("import os\nclass TestX:\n def test_x(self, p):\n assert os.stat(p).st_mode\n", "st_mode"), + ("import os\nassert os.stat('.').st_mode\n", "st_mode"), + ]: + got = problems(Path("test_fake.py"), snippet) + assert got and what in got[0], (snippet, got) + + +def test_the_accepted_ways_are_accepted(): + marked = BAD.replace("def test_a", "@pytest.mark.posix_perms\ndef test_a") + assert problems(Path("test_fake.py"), marked) == [] + assert problems(Path("test_fake.py"), "pytestmark = pytest.mark.posix_perms\n" + BAD) == [] + helper = "from posix_perms import assert_mode\ndef test_a(p):\n assert_mode(p, 0o600)\n" + assert problems(Path("test_fake.py"), helper) == [] + setup_only = "def test_a(tmp_path):\n f = tmp_path / 'x'\n f.write_text('')\n f.chmod(0o755)\n" + assert problems(Path("test_fake.py"), setup_only) == [] + assert problems(Path("test_fake.py"), BAD) # unmarked: flagged + # A comment or a string that merely mentions st_mode is not an inspection. + assert problems(Path("test_fake.py"), 'def test_a():\n # st_mode\n assert "st_mode"\n') == [] diff --git a/tests/test_secrets_store.py b/tests/test_secrets_store.py index 413e496..b4d6c87 100644 --- a/tests/test_secrets_store.py +++ b/tests/test_secrets_store.py @@ -34,6 +34,7 @@ def factory(): assert calls["n"] == 1 # factory ran once, then read from store +@pytest.mark.posix_perms def test_file_is_owner_only(store, tmp_path): store.set_secret("sekret", "value") f = tmp_path / ".sekret.secret" diff --git a/tests/test_secrets_store_secret_service.py b/tests/test_secrets_store_secret_service.py index 73552d2..13acc8f 100644 --- a/tests/test_secrets_store_secret_service.py +++ b/tests/test_secrets_store_secret_service.py @@ -11,13 +11,13 @@ from __future__ import annotations import logging -import stat import sys import threading import types from pathlib import Path import pytest +from posix_perms import assert_mode SECRET = "sk-test-VALUE-0123456789" @@ -181,7 +181,8 @@ def test_set_secret_falls_back_to_a_0600_file_and_warns_without_the_value(ss, mo s.set_secret("fernet-key", SECRET) s.set_secret("fernet-key", SECRET + "2") f = tmp_path / "files" / ".fernet-key.secret" - assert f.read_text() == SECRET + "2" and stat.S_IMODE(f.stat().st_mode) == 0o600 + assert f.read_text() == SECRET + "2" + assert_mode(f, 0o600) assert s.get_secret("fernet-key") == SECRET + "2" msgs = [r.getMessage() for r in caplog.records] assert any("0600 file" in m and "fernet-key" in m for m in msgs) diff --git a/tests/test_token_kit.py b/tests/test_token_kit.py index 5169ca3..3951925 100644 --- a/tests/test_token_kit.py +++ b/tests/test_token_kit.py @@ -14,6 +14,7 @@ from pathlib import Path import pytest +from posix_perms import assert_mode from token_fake import FAKE_TOKEN, Recorded, fake_recorded, make_fake, token_env pytest.importorskip("cryptography") @@ -172,7 +173,8 @@ def test_config_validation(env): base.save_config("fakenotes", {"folders": "../etc"}) assert e.value.code == "bad_config" state = base._state_path("fakenotes") - assert oct(state.stat().st_mode & 0o777) == "0o600" and FAKE_TOKEN not in state.read_text() + assert_mode(state, 0o600) + assert FAKE_TOKEN not in state.read_text() def test_none_mode_and_disconnect(env): diff --git a/tests/test_verify_script.py b/tests/test_verify_script.py index 2980096..de26538 100644 --- a/tests/test_verify_script.py +++ b/tests/test_verify_script.py @@ -145,6 +145,7 @@ def test_mypy_ratchet_list_is_shared_and_valid(): assert not any(e.startswith("#") for e in entries), "no comments in the list: $(cat ...) feeds it to mypy" +@pytest.mark.posix_perms def test_verify_script_is_executable_shell_that_parses(): assert VERIFY.stat().st_mode & 0o111, "scripts/verify.sh must be executable" for script in (VERIFY, ROOT / "scripts" / "install-hooks.sh", ROOT / ".githooks" / "pre-push"): diff --git a/tests/test_windows_portability.py b/tests/test_windows_portability.py index b543aaa..24d832a 100644 --- a/tests/test_windows_portability.py +++ b/tests/test_windows_portability.py @@ -20,7 +20,6 @@ ROOT = Path(__file__).resolve().parent.parent windows_only = pytest.mark.skipif(sys.platform != "win32", reason="needs a real Windows") -posix_only = pytest.mark.skipif(sys.platform == "win32", reason="POSIX permission bits do not exist on Windows") # ── parent_watch.alive: never os.kill on Windows ───────────────────────────── @@ -188,7 +187,7 @@ def test_elsewhere_the_bind_is_exactly_what_it_was(monkeypatch, platform): # ── owner-only permissions ─────────────────────────────────────────────────── -@posix_only +@pytest.mark.posix_perms def test_files_and_dirs_are_restricted_on_posix(tmp_path): f = tmp_path / "secret" f.write_text("x") diff --git a/tests/test_windows_workflow.py b/tests/test_windows_workflow.py index d57f478..013b963 100644 --- a/tests/test_windows_workflow.py +++ b/tests/test_windows_workflow.py @@ -82,7 +82,8 @@ def test_every_skipped_file_exists_and_has_a_reason(): def _items(*names): - return [SimpleNamespace(fspath=str(TESTS / n), name="test_x", marks=[], add_marker=lambda m, _n=n: marks.setdefault(_n, []).append(m)) + return [SimpleNamespace(fspath=str(TESTS / n), name="test_x", marks=[], get_closest_marker=lambda _n: None, + add_marker=lambda m, _n=n: marks.setdefault(_n, []).append(m)) for n in names] @@ -95,10 +96,14 @@ def test_the_conftest_hook_skips_only_on_windows(monkeypatch): items = _items(victim, "test_auth.py") node = next(n for n in SKIPPED_ON_WINDOWS if "::" in n) nfile, _, ntest = node.partition("::") - items.append(SimpleNamespace(fspath=str(TESTS / nfile), name=ntest, marks=[], + items.append(SimpleNamespace(fspath=str(TESTS / nfile), name=ntest, marks=[], get_closest_marker=lambda _n: None, add_marker=lambda m: marks.setdefault(node, []).append(m))) items.append(SimpleNamespace(fspath=str(TESTS / nfile), name="test_some_other_test", marks=[], + get_closest_marker=lambda _n: None, add_marker=lambda m: marks.setdefault("other", []).append(m))) + items.append(SimpleNamespace(fspath=str(TESTS / "test_auth.py"), name="test_about_modes", marks=[], + get_closest_marker=lambda n: object() if n == "posix_perms" else None, + add_marker=lambda m: marks.setdefault("perms", []).append(m))) marks.clear() monkeypatch.setattr(sys, "platform", "linux") @@ -107,7 +112,8 @@ def test_the_conftest_hook_skips_only_on_windows(monkeypatch): monkeypatch.setattr(sys, "platform", "win32") conftest.pytest_collection_modifyitems(None, items) - assert sorted(marks) == sorted([victim, node]) + assert sorted(marks) == sorted([victim, node, "perms"]) + assert marks["perms"][0].name == "skip" and "permission bits" in marks["perms"][0].kwargs["reason"] mark = marks[victim][0] assert mark.name == "skip" and SKIPPED_ON_WINDOWS[victim] in mark.kwargs["reason"] diff --git a/tests/windows_skips.py b/tests/windows_skips.py index 2f415d1..6b20abd 100644 --- a/tests/windows_skips.py +++ b/tests/windows_skips.py @@ -31,12 +31,8 @@ "test_vault_conventions.py": "not investigated: the vault index preview fails on Windows paths", "test_notes_structure.py": "not investigated: the notes structure tools fail on Windows paths", "test_skills_compat.py": "not investigated: skill directory resolution and symlinks differ on Windows", - # Single tests. - "test_byok_keys.py::test_config_written_owner_only": "asserts POSIX mode 0600; Windows has no such permission bits (docs/PORTABILITY.md 4.3)", - "test_secrets_store.py::test_file_is_owner_only": "asserts POSIX mode 0600; Windows has no such permission bits (docs/PORTABILITY.md 4.3)", - "test_learning_feedback.py::test_export_is_local_anonymised_and_private": "asserts POSIX mode 0600; Windows has no such permission bits (docs/PORTABILITY.md 4.3)", - "test_token_kit.py::test_config_validation": "asserts POSIX mode 0600; Windows has no such permission bits (docs/PORTABILITY.md 4.3)", - "test_linux_bundles_workflow.py::test_the_script_it_calls_exists_and_is_executable": "asserts the POSIX executable bit of a shell script", + # Single tests. (Tests about POSIX permission bits are not listed here: they carry the + # `posix_perms` marker or use tests/posix_perms.py, which the guard in test_windows_workflow.py enforces.) "test_release_check.py::test_not_a_git_repo_fails": "the test's cleanup cannot delete read-only .git objects on Windows", "test_media.py::test_assemble_refuses_files_outside_the_data_directory": "uses a POSIX /tmp path", "test_lessons.py::test_scope_validation": "uses a POSIX absolute path (repo:/...), which is not absolute on Windows", From fa0ae3adefa9553d6699643412f9ae1aa19ef2a7 Mon Sep 17 00:00:00 2001 From: EVMCOD <214792911+EVMCOD@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:20:40 +0100 Subject: [PATCH 5/5] posix_perms helpers: decide Windows once at import, not from a monkeypatched sys.platform windows-latest run 40926549010 (windows-python, job 123633486845): 1 failed, 1707 passed. test_secrets_store_secret_service patches sys.platform to 'linux' to exercise the Linux code path, so the helper no longer saw Windows and asserted 0o600 on a real Windows file (mode 0o666). The helpers now follow the real OS; a test pins that. --- tests/posix_perms.py | 7 ++++++- tests/test_posix_perms_guard.py | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/posix_perms.py b/tests/posix_perms.py index f8895d8..707fb0d 100644 --- a/tests/posix_perms.py +++ b/tests/posix_perms.py @@ -25,8 +25,13 @@ "(docs/PORTABILITY.md 4.3)") +# Decided once at import: some tests monkeypatch sys.platform to exercise another OS's +# code path, but the file system underneath is still the real one. +_WINDOWS = sys.platform == "win32" + + def on_windows() -> bool: - return sys.platform == "win32" + return _WINDOWS def assert_mode(path: str | os.PathLike[str], expected: int) -> None: diff --git a/tests/test_posix_perms_guard.py b/tests/test_posix_perms_guard.py index 83c5d1c..3f7aa51 100644 --- a/tests/test_posix_perms_guard.py +++ b/tests/test_posix_perms_guard.py @@ -22,6 +22,8 @@ import sys from pathlib import Path +import pytest + TESTS = Path(__file__).resolve().parent sys.path.insert(0, str(TESTS)) @@ -136,3 +138,21 @@ def test_the_accepted_ways_are_accepted(): assert problems(Path("test_fake.py"), BAD) # unmarked: flagged # A comment or a string that merely mentions st_mode is not an inspection. assert problems(Path("test_fake.py"), 'def test_a():\n # st_mode\n assert "st_mode"\n') == [] + + +def test_the_helpers_follow_the_real_os_not_a_monkeypatched_sys_platform(tmp_path, monkeypatch): + """windows-latest run 40926549010: a test patched sys.platform to "linux" and the helper + then asserted 0o600 on a real Windows file. The OS is decided once, at import.""" + import sys + + import posix_perms + + f = tmp_path / "f" + f.write_text("x") + f.chmod(0o644) + monkeypatch.setattr(sys, "platform", "win32" if sys.platform != "win32" else "linux") + if posix_perms.on_windows(): + posix_perms.assert_mode(f, 0o600) # no-op on the real Windows + else: + with pytest.raises(AssertionError): + posix_perms.assert_mode(f, 0o600)