From e2404dda4211b842af33ef0bbf3829650d492ffe Mon Sep 17 00:00:00 2001 From: KNambiarDJsc Date: Tue, 15 Sep 2026 22:42:09 +0530 Subject: [PATCH 1/2] fix: guard fcntl imports so the CLI doesn't crash on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit repo2rlenv is completely non-functional on Windows right now, in the current v0.9.0 PyPI release: `repo2rlenv --version` raises `ModuleNotFoundError: No module named 'fcntl'` before argument parsing even completes. Verified from a fresh `pip install repo2rlenv` on a clean venv, from a from-scratch git clone + uv sync, and from upstream/main directly — not specific to any one install path. Root cause: cli.py's dispatcher imports every subsystem's argparse registration unconditionally, before routing to any subcommand, so this runs on every invocation including --version and --help. Four files reachable from that chain do `import fcntl` at module level — fcntl is POSIX-only and has no stdlib equivalent on Windows at all (msvcrt.locking byte-ranges a file rather than advisory-locking a whole handle, so it isn't a drop-in swap): quality/loop/runner.py, tasksmith/runner.py, tasksmith/batch.py, pipelines/recipes/history/worker.py Each uses it identically, for the same thing: an advisory, mostly non-blocking flock() on a ".lock" file so two concurrent controllers/loops don't run against the same output directory. Guard both the import and the flock() call by `sys.platform`, preserving each site's exact existing behavior (return type, exception handling) on POSIX unchanged. Locking becomes a documented no-op on Windows rather than a crash — a real gap, but not a regression, since locking was never functional there before this either. Added tests/test_posix_only_imports.py: an AST-based static check (same shape as test_subprocess_encoding.py) that fails if any file under src/repo2rlenv imports fcntl/pwd/grp/termios/tty/pty/posix/ resource/crypt/nis/spwd unconditionally at module level. Verified it flags all 4 original offenders when the fix is reverted, and passes clean with it applied — CI runs Linux-only, where none of these modules are missing, so nothing else would have caught a regression here. Verified on Windows 11 / Python 3.12: `repo2rlenv --version` and `--help` both now exit 0 and build the full subcommand list. Verified on Linux (WSL Ubuntu, uv run --all-extras pytest -q — CI's exact command): 1866 passed, 0 failed, both before and after this patch — no regressions. Refs #128 --- .../pipelines/recipes/history/worker.py | 12 ++++- src/repo2rlenv/quality/loop/runner.py | 19 +++++-- src/repo2rlenv/tasksmith/batch.py | 11 +++- src/repo2rlenv/tasksmith/runner.py | 12 ++++- tests/test_posix_only_imports.py | 51 +++++++++++++++++++ 5 files changed, 94 insertions(+), 11 deletions(-) create mode 100644 tests/test_posix_only_imports.py diff --git a/src/repo2rlenv/pipelines/recipes/history/worker.py b/src/repo2rlenv/pipelines/recipes/history/worker.py index b2500143..1d494bdc 100644 --- a/src/repo2rlenv/pipelines/recipes/history/worker.py +++ b/src/repo2rlenv/pipelines/recipes/history/worker.py @@ -3,17 +3,24 @@ from __future__ import annotations import argparse -import fcntl import hashlib import io import json import os import shutil import subprocess +import sys import tarfile import uuid from pathlib import Path +# fcntl is POSIX-only and has no stdlib equivalent on Windows. Import it +# lazily so this module — imported eagerly by cli.py just to register its +# argparse subcommand — doesn't crash the whole CLI on Windows. The lock +# itself becomes a documented no-op there; see the call site below. +if sys.platform != "win32": + import fcntl + from repo2rlenv.execution.lifecycle import save_record from repo2rlenv.execution.python_repository import bootstrap_snapshot, test_image from repo2rlenv.pipelines.recipes.history.selection import check_entities, within @@ -131,7 +138,8 @@ def prepare(config: dict, destination: Path) -> dict: root = Path("/work/history") / hashlib.sha256(repo.url.encode()).hexdigest()[:16] root.parent.mkdir(parents=True, exist_ok=True) with root.with_suffix(".lock").open("a") as lock: - fcntl.flock(lock, fcntl.LOCK_EX) + if sys.platform != "win32": + fcntl.flock(lock, fcntl.LOCK_EX) if not root.exists(): temporary = root.with_name(root.name + "-" + uuid.uuid4().hex) try: diff --git a/src/repo2rlenv/quality/loop/runner.py b/src/repo2rlenv/quality/loop/runner.py index 9e8ca61b..6434c636 100644 --- a/src/repo2rlenv/quality/loop/runner.py +++ b/src/repo2rlenv/quality/loop/runner.py @@ -2,16 +2,24 @@ from __future__ import annotations -import fcntl import hashlib import json import re +import sys import tomllib from collections.abc import Callable from pathlib import Path from pydantic import ValidationError +# fcntl is POSIX-only and has no stdlib equivalent on Windows (msvcrt.locking +# byte-ranges a file rather than advisory-locking the whole handle). Import +# it lazily so this module — imported eagerly by cli.py just to register its +# argparse subcommand — doesn't crash the whole CLI on Windows. The lock +# itself becomes a documented no-op there; see the call site below. +if sys.platform != "win32": + import fcntl + from repo2rlenv.campaigns.budget import BudgetExceeded, BudgetLedger from repo2rlenv.campaigns.events import EventJournal, ProgressEvent from repo2rlenv.execution.lifecycle import save_record @@ -640,10 +648,11 @@ def run( raise ValueError("Quality output must be outside the input task") self.directory.mkdir(parents=True, exist_ok=True) with (self.directory / ".lock").open("a") as lock: - try: - fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) - except BlockingIOError as exc: - raise RuntimeError("Another controller owns this quality run") from exc + if sys.platform != "win32": + try: + fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + except BlockingIOError as exc: + raise RuntimeError("Another controller owns this quality run") from exc try: return self._run(task, baseline, oracle, rollout, probes, resume) finally: diff --git a/src/repo2rlenv/tasksmith/batch.py b/src/repo2rlenv/tasksmith/batch.py index 9d074dcc..0082c9b1 100644 --- a/src/repo2rlenv/tasksmith/batch.py +++ b/src/repo2rlenv/tasksmith/batch.py @@ -3,7 +3,6 @@ from __future__ import annotations import argparse -import fcntl import hashlib import json import os @@ -18,6 +17,13 @@ from decimal import Decimal from pathlib import Path, PurePosixPath +# fcntl is POSIX-only and has no stdlib equivalent on Windows. Import it +# lazily so this module — imported eagerly by cli.py just to register its +# argparse subcommand — doesn't crash the whole CLI on Windows. The lock +# itself becomes a documented no-op there; see the call site below. +if sys.platform != "win32": + import fcntl + from pydantic import Field, model_validator from repo2rlenv.campaigns.budget import BudgetLedger @@ -370,7 +376,8 @@ def run_batch( directory, campaign, wheel = directory.resolve(), campaign.resolve(), wheel.resolve() directory.mkdir(parents=True, exist_ok=True) with (directory / ".lock").open("a") as lock: - fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + if sys.platform != "win32": + fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) prior = [verified_result(path) for path in plan.prior_verified] if len({item["url"] for item in prior}) != len(prior): raise ValueError("Prior verified results contain duplicate PRs") diff --git a/src/repo2rlenv/tasksmith/runner.py b/src/repo2rlenv/tasksmith/runner.py index caa08060..720d4b87 100644 --- a/src/repo2rlenv/tasksmith/runner.py +++ b/src/repo2rlenv/tasksmith/runner.py @@ -3,14 +3,21 @@ from __future__ import annotations import asyncio -import fcntl import hashlib import json import shlex +import sys import time from pathlib import Path, PurePosixPath from typing import TypedDict +# fcntl is POSIX-only and has no stdlib equivalent on Windows. Import it +# lazily so this module — imported eagerly by cli.py just to register its +# argparse subcommand — doesn't crash the whole CLI on Windows. The lock +# itself becomes a documented no-op there; see the call site below. +if sys.platform != "win32": + import fcntl + from repo2rlenv.auth import resolve_llm_api_key from repo2rlenv.campaigns.budget import BudgetLedger from repo2rlenv.execution.artifacts import ( @@ -765,7 +772,8 @@ def run( raise ValueError("stop-after must be within the frozen panel size") self.directory.mkdir(parents=True, exist_ok=True) with (self.directory / ".lock").open("a") as lock: - fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + if sys.platform != "win32": + fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) return self._run(panel, limit, generation_run, reuse_evidence, source_records) def _run(self, panel, limit, generation_run, reuse_evidence=False, source_records=None): diff --git a/tests/test_posix_only_imports.py b/tests/test_posix_only_imports.py new file mode 100644 index 00000000..f547a74c --- /dev/null +++ b/tests/test_posix_only_imports.py @@ -0,0 +1,51 @@ +"""No POSIX-only stdlib module is imported unconditionally at module level. + +`fcntl` (and pwd/grp/termios/tty/pty/posix/resource/crypt/nis/spwd) don't +exist on Windows. An unconditional `import fcntl` at the top of a module +crashes with `ModuleNotFoundError` the moment anything imports that module — +even just to register an argparse subcommand, never to call anything +POSIX-specific. `cli.py`'s dispatcher eagerly imports every subsystem just to +build `--help`, so one such import anywhere in that chain took down +`repo2rlenv --version` entirely (#128). CI runs on Linux only, where these +modules exist, so the bug is invisible there — this static check keeps new +top-level imports explicit about the platforms they need. + +Guard the import instead: `if sys.platform != "win32": import fcntl`. +""" + +from __future__ import annotations + +import ast +from collections.abc import Iterator +from pathlib import Path + +import repo2rlenv + +SRC = Path(repo2rlenv.__file__).parent + +_POSIX_ONLY = frozenset( + {"fcntl", "pwd", "grp", "termios", "tty", "pty", "posix", "resource", "crypt", "nis", "spwd"} +) + + +def _unconditional_posix_only_imports() -> Iterator[str]: + for path in sorted(SRC.rglob("*.py")): + tree = ast.parse(path.read_text(encoding="utf-8")) + # Only module.body (top level): an import inside an `if` guard, e.g. + # `if sys.platform != "win32": import fcntl`, is intentionally exempt. + for node in tree.body: + if isinstance(node, ast.Import): + for alias in node.names: + if alias.name in _POSIX_ONLY: + where = f"{path.relative_to(SRC).as_posix()}:{node.lineno}" + yield f"{where} (import {alias.name})" + elif isinstance(node, ast.ImportFrom) and node.module in _POSIX_ONLY: + where = f"{path.relative_to(SRC).as_posix()}:{node.lineno}" + yield f"{where} (from {node.module} import ...)" + + +def test_no_unconditional_posix_only_imports(): + offenders = list(_unconditional_posix_only_imports()) + assert offenders == [], ( + f'guard with `if sys.platform != "win32":` before importing: {offenders}' + ) From 640fbcf0be675535efe794c8e8558529aa4d639f Mon Sep 17 00:00:00 2001 From: adithya-s-k Date: Tue, 15 Sep 2026 23:08:46 +0530 Subject: [PATCH 2/2] Preserve controller locks on Windows and gate wheel releases --- .github/workflows/ci.yml | 6 +- .github/workflows/release.yml | 8 +- .github/workflows/windows.yml | 44 +++++++ CONTRIBUTING.md | 1 + README.md | 4 + docs/quickstart.md | 6 + src/repo2rlenv/locking.py | 38 ++++++ .../pipelines/recipes/history/worker.py | 12 +- src/repo2rlenv/quality/loop/runner.py | 19 +-- src/repo2rlenv/tasksmith/batch.py | 11 +- src/repo2rlenv/tasksmith/runner.py | 12 +- tests/check_cli_install.py | 75 ++++++++++++ tests/test_locking.py | 113 ++++++++++++++++++ tests/test_posix_only_imports.py | 6 +- 14 files changed, 307 insertions(+), 48 deletions(-) create mode 100644 .github/workflows/windows.yml create mode 100644 src/repo2rlenv/locking.py create mode 100644 tests/check_cli_install.py create mode 100644 tests/test_locking.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d69a2459..00d5002f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,6 +24,10 @@ env: CI: "1" jobs: + windows: + name: Windows compatibility + uses: ./.github/workflows/windows.yml + lint: name: Lint (ruff) runs-on: ubuntu-latest @@ -160,7 +164,7 @@ jobs: build: name: Build sdist + wheel runs-on: ubuntu-latest - needs: [lint, test, owned-contracts, tasksmith-runtimes] + needs: [lint, test, owned-contracts, tasksmith-runtimes, windows] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1f908b17..04abeb65 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -31,6 +31,12 @@ concurrency: cancel-in-progress: false jobs: + windows: + name: Windows compatibility + uses: ./.github/workflows/windows.yml + with: + ref: ${{ github.event.release.tag_name || inputs.tag }} + # Sanity gate — run the full test suite on the release tag before we publish. test: name: Tests on release tag (py${{ matrix.python-version }}) @@ -75,7 +81,7 @@ jobs: build: name: Build sdist + wheel runs-on: ubuntu-latest - needs: [test, tasksmith-runtimes] + needs: [test, tasksmith-runtimes, windows] outputs: version: ${{ steps.read-version.outputs.version }} steps: diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml new file mode 100644 index 00000000..1917b482 --- /dev/null +++ b/.github/workflows/windows.yml @@ -0,0 +1,44 @@ +name: Windows compatibility + +on: + workflow_call: + inputs: + ref: + description: Commit or release tag to validate + required: false + type: string + +permissions: + contents: read + +jobs: + wheel: + name: Windows wheel (py${{ matrix.python-version }}) + runs-on: windows-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.12", "3.13", "3.14"] + env: + PYTHON_DOTENV_DISABLED: "1" + NO_COLOR: "1" + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + ref: ${{ inputs.ref }} + - uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 + with: + python-version: ${{ matrix.python-version }} + enable-cache: false + - run: uv build + - name: Install base wheel in isolation + run: | + uv venv "$env:RUNNER_TEMP/cli-check" + $wheel = (Get-ChildItem dist/*.whl).FullName + uv pip install --python "$env:RUNNER_TEMP/cli-check/Scripts/python.exe" $wheel + - name: Check console entrypoint, discovery and native task validation + run: '& "$env:RUNNER_TEMP/cli-check/Scripts/python.exe" tests/check_cli_install.py' + - name: Check real Windows process locks + run: | + uv pip install --python "$env:RUNNER_TEMP/cli-check/Scripts/python.exe" pytest + & "$env:RUNNER_TEMP/cli-check/Scripts/python.exe" -m pytest -q tests/test_locking.py tests/test_posix_only_imports.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 97531bd8..ab6e50be 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -75,6 +75,7 @@ Closes #N - **lint** — `uv run ruff check .` + `uv run ruff format --check .` - **test** — `uv run pytest -q` against Python 3.12, 3.13, 3.14 (matrix) +- **Windows** — base-only wheel CLI smoke checks and real process-lock contention on Python 3.12, 3.13, 3.14; the same gate runs before publication. Full controller portability remains separate work. - **build** — `uv build` produces sdist + wheel, smoke-installs the wheel, checks `repo2rlenv --version` A green CI is the floor for merge — green plus at least one approving review is the ceiling. diff --git a/README.md b/README.md index 9587487d..f1632704 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,10 @@ and repair emitted tasks; exporting a task alone does not establish its quality. Requires **Python 3.12+** and Git. This example generates PR-diff tasks without building a container: +Windows CI covers CLI startup, recipe discovery, native task emission and static +validation. Use Linux, macOS or WSL for Tasksmith, research-recipe generation and +the quality controller; their full native Windows execution is not yet supported. + ```bash pip install repo2rlenv diff --git a/docs/quickstart.md b/docs/quickstart.md index c2f39a38..6bcc50eb 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -21,6 +21,12 @@ hf auth login ## Install +Requires Python 3.12+. Windows CI checks the installed CLI, recipe discovery, +native task emission and static validation. Run Tasksmith, research-recipe +generation and the quality controller on Linux, macOS or WSL: those controllers +still depend on POSIX artifact permissions and process cleanup. Remote sandboxes +run Linux; choosing a cloud provider does not remove these host requirements. + ```bash pip install repo2rlenv # from PyPI # or: diff --git a/src/repo2rlenv/locking.py b/src/repo2rlenv/locking.py new file mode 100644 index 00000000..93b3356f --- /dev/null +++ b/src/repo2rlenv/locking.py @@ -0,0 +1,38 @@ +"""Process locks for controller receipts and shared worker checkouts.""" + +from __future__ import annotations + +import errno +import sys +import time +from typing import IO + + +def lock_file(handle: IO, *, blocking: bool = False) -> None: + """Exclusively lock an open file until it closes, without deleting its path. + + POSIX retains flock semantics, including interoperability with older + controllers. Windows locks byte zero; the region may extend beyond EOF, + so an empty lock file needs no write. All callers must keep the same path + and close their handle on success, failure or cancellation. + """ + if sys.platform != "win32": + import fcntl + + fcntl.flock(handle, fcntl.LOCK_EX | (0 if blocking else fcntl.LOCK_NB)) + return + + import msvcrt + + handle.seek(0) + while True: + try: + msvcrt.locking(handle.fileno(), msvcrt.LK_NBLCK, 1) + return + except OSError as error: + if error.errno not in {errno.EACCES, errno.EAGAIN, errno.EDEADLK}: + raise + if not blocking: + raise BlockingIOError(errno.EAGAIN, "Another process owns this lock") from error + # LK_LOCK gives up after ten retries; match flock's indefinite wait. + time.sleep(0.1) diff --git a/src/repo2rlenv/pipelines/recipes/history/worker.py b/src/repo2rlenv/pipelines/recipes/history/worker.py index 1d494bdc..a68eec24 100644 --- a/src/repo2rlenv/pipelines/recipes/history/worker.py +++ b/src/repo2rlenv/pipelines/recipes/history/worker.py @@ -9,20 +9,13 @@ import os import shutil import subprocess -import sys import tarfile import uuid from pathlib import Path -# fcntl is POSIX-only and has no stdlib equivalent on Windows. Import it -# lazily so this module — imported eagerly by cli.py just to register its -# argparse subcommand — doesn't crash the whole CLI on Windows. The lock -# itself becomes a documented no-op there; see the call site below. -if sys.platform != "win32": - import fcntl - from repo2rlenv.execution.lifecycle import save_record from repo2rlenv.execution.python_repository import bootstrap_snapshot, test_image +from repo2rlenv.locking import lock_file from repo2rlenv.pipelines.recipes.history.selection import check_entities, within from repo2rlenv.pipelines.recipes.history.test_suite import stage_tests from repo2rlenv.quality.python_evidence import test_excerpts @@ -138,8 +131,7 @@ def prepare(config: dict, destination: Path) -> dict: root = Path("/work/history") / hashlib.sha256(repo.url.encode()).hexdigest()[:16] root.parent.mkdir(parents=True, exist_ok=True) with root.with_suffix(".lock").open("a") as lock: - if sys.platform != "win32": - fcntl.flock(lock, fcntl.LOCK_EX) + lock_file(lock, blocking=True) if not root.exists(): temporary = root.with_name(root.name + "-" + uuid.uuid4().hex) try: diff --git a/src/repo2rlenv/quality/loop/runner.py b/src/repo2rlenv/quality/loop/runner.py index 6434c636..fa034849 100644 --- a/src/repo2rlenv/quality/loop/runner.py +++ b/src/repo2rlenv/quality/loop/runner.py @@ -5,24 +5,16 @@ import hashlib import json import re -import sys import tomllib from collections.abc import Callable from pathlib import Path from pydantic import ValidationError -# fcntl is POSIX-only and has no stdlib equivalent on Windows (msvcrt.locking -# byte-ranges a file rather than advisory-locking the whole handle). Import -# it lazily so this module — imported eagerly by cli.py just to register its -# argparse subcommand — doesn't crash the whole CLI on Windows. The lock -# itself becomes a documented no-op there; see the call site below. -if sys.platform != "win32": - import fcntl - from repo2rlenv.campaigns.budget import BudgetExceeded, BudgetLedger from repo2rlenv.campaigns.events import EventJournal, ProgressEvent from repo2rlenv.execution.lifecycle import save_record +from repo2rlenv.locking import lock_file from repo2rlenv.quality.loop.artifacts import ( EXPECTED_PASSES_CONTRACT, apply_repair, @@ -648,11 +640,10 @@ def run( raise ValueError("Quality output must be outside the input task") self.directory.mkdir(parents=True, exist_ok=True) with (self.directory / ".lock").open("a") as lock: - if sys.platform != "win32": - try: - fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) - except BlockingIOError as exc: - raise RuntimeError("Another controller owns this quality run") from exc + try: + lock_file(lock) + except BlockingIOError as exc: + raise RuntimeError("Another controller owns this quality run") from exc try: return self._run(task, baseline, oracle, rollout, probes, resume) finally: diff --git a/src/repo2rlenv/tasksmith/batch.py b/src/repo2rlenv/tasksmith/batch.py index 0082c9b1..68b9891d 100644 --- a/src/repo2rlenv/tasksmith/batch.py +++ b/src/repo2rlenv/tasksmith/batch.py @@ -17,18 +17,12 @@ from decimal import Decimal from pathlib import Path, PurePosixPath -# fcntl is POSIX-only and has no stdlib equivalent on Windows. Import it -# lazily so this module — imported eagerly by cli.py just to register its -# argparse subcommand — doesn't crash the whole CLI on Windows. The lock -# itself becomes a documented no-op there; see the call site below. -if sys.platform != "win32": - import fcntl - from pydantic import Field, model_validator from repo2rlenv.campaigns.budget import BudgetLedger from repo2rlenv.execution.artifacts import check_runtime_wheel from repo2rlenv.execution.lifecycle import save_record +from repo2rlenv.locking import lock_file from repo2rlenv.quality.loop.artifacts import digest from repo2rlenv.quality.loop.client import RunBudget from repo2rlenv.quality.loop.models import LoopResult, ProbeManifest @@ -376,8 +370,7 @@ def run_batch( directory, campaign, wheel = directory.resolve(), campaign.resolve(), wheel.resolve() directory.mkdir(parents=True, exist_ok=True) with (directory / ".lock").open("a") as lock: - if sys.platform != "win32": - fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + lock_file(lock) prior = [verified_result(path) for path in plan.prior_verified] if len({item["url"] for item in prior}) != len(prior): raise ValueError("Prior verified results contain duplicate PRs") diff --git a/src/repo2rlenv/tasksmith/runner.py b/src/repo2rlenv/tasksmith/runner.py index 720d4b87..7490b6e8 100644 --- a/src/repo2rlenv/tasksmith/runner.py +++ b/src/repo2rlenv/tasksmith/runner.py @@ -6,18 +6,10 @@ import hashlib import json import shlex -import sys import time from pathlib import Path, PurePosixPath from typing import TypedDict -# fcntl is POSIX-only and has no stdlib equivalent on Windows. Import it -# lazily so this module — imported eagerly by cli.py just to register its -# argparse subcommand — doesn't crash the whole CLI on Windows. The lock -# itself becomes a documented no-op there; see the call site below. -if sys.platform != "win32": - import fcntl - from repo2rlenv.auth import resolve_llm_api_key from repo2rlenv.campaigns.budget import BudgetLedger from repo2rlenv.execution.artifacts import ( @@ -34,6 +26,7 @@ save_record, stop_worker, ) +from repo2rlenv.locking import lock_file from repo2rlenv.quality.loop.artifacts import task_identity from repo2rlenv.quality.loop.client import RunBudget from repo2rlenv.quality.loop.models import ProbeManifest @@ -772,8 +765,7 @@ def run( raise ValueError("stop-after must be within the frozen panel size") self.directory.mkdir(parents=True, exist_ok=True) with (self.directory / ".lock").open("a") as lock: - if sys.platform != "win32": - fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + lock_file(lock) return self._run(panel, limit, generation_run, reuse_evidence, source_records) def _run(self, panel, limit, generation_run, reuse_evidence=False, source_records=None): diff --git a/tests/check_cli_install.py b/tests/check_cli_install.py new file mode 100644 index 00000000..ba8a5aeb --- /dev/null +++ b/tests/check_cli_install.py @@ -0,0 +1,75 @@ +"""Smoke-test an installed CLI from outside the checkout, without cloud calls.""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +import sysconfig +from importlib.metadata import version +from pathlib import Path +from tempfile import TemporaryDirectory + +from repo2rlenv.emitter.harbor import HarborTask, write_harbor_task +from repo2rlenv.ui import console + + +def check() -> dict: + executable = Path(sysconfig.get_path("scripts")) / ( + "repo2rlenv.exe" if sys.platform == "win32" else "repo2rlenv" + ) + with TemporaryDirectory(prefix="repo2rlenv CLI ") as directory: + root = Path(directory) + + def cli(*args: str, expected: int = 0) -> str: + result = subprocess.run( + [str(executable), *args], + cwd=root, + env={**os.environ, "PYTHON_DOTENV_DISABLED": "1", "PYTHONUTF8": "1"}, + capture_output=True, + encoding="utf-8", + timeout=60, + check=False, + ) + assert result.returncode == expected, result.stderr + result.stdout + return result.stdout + + assert version("repo2rlenv") in cli("--version") + assert "generate" in cli("--help") + for command in ( + "generate", + "validate", + "push", + "pull", + "tasksmith", + "quality", + "pipelines", + ): + assert "usage:" in cli(command, "--help") + listing = json.loads(cli("pipelines", "list", "--json")) + recipes = [item for item in listing["recipes"] if item["implemented"]] + for recipe in recipes: + detail = json.loads( + cli("pipelines", "describe", recipe["pipeline"], "--recipe", recipe["id"], "--json") + ) + assert detail["id"] == recipe["id"] + for command in ("tasksmith", "quality"): + error = json.loads(cli(command, "show", "missing.json", "--json", expected=2)) + assert set(error) == {"error", "message"} + task = HarborTask( + name="unicode-task", + org="smoke", + description="Handle café input", + instruction="# Task\n\nPreserve café and 日本語 in the output.\n", + oracle_diff="--- a/x.py\n+++ b/x.py\n@@ -1 +1 @@\n-1\n+2\n", + repo2env={"pipeline": "pr_diff", "pipeline_version": "0.1.0", "repo": "smoke/demo"}, + ) + output = write_harbor_task(task, root / "task output") + assert (output / "instruction.md").read_text(encoding="utf-8") == task.instruction + cli("validate", str(root / "task output"), "--deep") + return {"status": "passed", "version": version("repo2rlenv"), "recipes": len(recipes)} + + +if __name__ == "__main__": + console.json(check()) diff --git a/tests/test_locking.py b/tests/test_locking.py new file mode 100644 index 00000000..304720dc --- /dev/null +++ b/tests/test_locking.py @@ -0,0 +1,113 @@ +"""Exercise real OS locks between independent processes, including on Windows.""" + +from __future__ import annotations + +import subprocess +import sys +from concurrent.futures import ThreadPoolExecutor +from contextlib import contextmanager +from pathlib import Path + +import pytest + +from repo2rlenv.locking import lock_file + +_OWNER = """ +import sys +from repo2rlenv.locking import lock_file +with open(sys.argv[1], 'a') as handle: + lock_file(handle) + sys.stdout.write('locked\\n') + sys.stdout.flush() + sys.stdin.read() +""" + + +def _line(process: subprocess.Popen) -> str: + with ThreadPoolExecutor(max_workers=1) as reader: + ready = reader.submit(process.stdout.readline) + try: + return ready.result(timeout=15) + except BaseException: + process.kill() + raise + + +@contextmanager +def _owner(path: Path, code: str = _OWNER): + process = subprocess.Popen( + [sys.executable, "-c", code, str(path)], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + # A bounded handshake avoids a hanging test if the child cannot import. + assert _line(process) == "locked\n" + yield process + finally: + if process.poll() is None: + process.kill() + process.communicate(timeout=15) + + +def test_nonblocking_lock_and_release(tmp_path): + path = tmp_path / "controller.lock" + with _owner(path) as owner: + with path.open("a") as handle, pytest.raises(BlockingIOError): + lock_file(handle) + owner.communicate(timeout=15) + assert owner.returncode == 0 + with path.open("a") as handle: + lock_file(handle) + assert path.read_bytes() == b"" # An empty lock file needs no mutation. + + +def test_lock_released_after_process_death(tmp_path): + path = tmp_path / "controller.lock" + with _owner(path) as owner: + owner.kill() + owner.communicate(timeout=15) + with path.open("a") as handle: + lock_file(handle) + + +def test_blocking_lock_waits_for_owner(tmp_path): + path = tmp_path / "checkout.lock" + contender_code = _OWNER.replace("lock_file(handle)", "lock_file(handle, blocking=True)") + contender_code = contender_code.replace( + " lock_file(handle, blocking=True)", + " sys.stdout.write('attempting\\n')\n" + " sys.stdout.flush()\n" + " lock_file(handle, blocking=True)", + ) + contender_code = contender_code.replace(" sys.stdin.read()", "") + with _owner(path) as owner: + contender = subprocess.Popen( + [sys.executable, "-c", contender_code, str(path)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + assert _line(contender) == "attempting\n" + with pytest.raises(subprocess.TimeoutExpired): + contender.communicate(timeout=0.5) + owner.communicate(timeout=15) + stdout, stderr = contender.communicate(timeout=15) + assert contender.returncode == 0, stderr + assert stdout == "locked\n" + finally: + if contender.poll() is None: + contender.kill() + contender.communicate(timeout=15) + + +@pytest.mark.skipif(sys.platform == "win32", reason="Interoperability with earlier POSIX releases") +def test_interoperates_with_existing_flock(tmp_path): + path = tmp_path / "controller.lock" + code = _OWNER.replace("from repo2rlenv.locking import lock_file", "import fcntl") + code = code.replace("lock_file(handle)", "fcntl.flock(handle, fcntl.LOCK_EX)") + with _owner(path), path.open("a") as handle, pytest.raises(BlockingIOError): + lock_file(handle) diff --git a/tests/test_posix_only_imports.py b/tests/test_posix_only_imports.py index f547a74c..5d5d0814 100644 --- a/tests/test_posix_only_imports.py +++ b/tests/test_posix_only_imports.py @@ -6,9 +6,9 @@ even just to register an argparse subcommand, never to call anything POSIX-specific. `cli.py`'s dispatcher eagerly imports every subsystem just to build `--help`, so one such import anywhere in that chain took down -`repo2rlenv --version` entirely (#128). CI runs on Linux only, where these -modules exist, so the bug is invisible there — this static check keeps new -top-level imports explicit about the platforms they need. +`repo2rlenv --version` entirely (#128). This static check complements the +installed-wheel Windows CI checks and keeps new top-level imports explicit +about the platforms they need. Guard the import instead: `if sys.platform != "win32": import fcntl`. """