From 1ec0c585dbe78b281bc6380e49adddc90ffdf581 Mon Sep 17 00:00:00 2001 From: PyVDisk Team Date: Wed, 30 Sep 2026 01:45:24 +0800 Subject: [PATCH 1/5] ci: cover Windows with the zero-dependency job and an errors-only lint job (#7) PR #5 and the compat layer were validated on a real Windows machine exactly once, by hand. Every platform primitive the package touches lives in pyvdisk/compat.py, and the emulation tests force those paths on Linux -- but only a real Windows runner exercises msvcrt.locking, the directory-fsync no-op and drive-letter handling against the actual OS. GitHub-hosted windows-latest runners are free for public repos, so: - no-dependencies gains an os matrix (ubuntu-latest, windows-latest): the zero-dependency promise, the full stdlib-only suite and a quickstart smoke now hold on both platforms; - Lint gains errors-windows: pylint -E resolves imports against the running interpreter, so an unguarded `import fcntl` -- the exact bug that made PR #5 necessary -- is an import-error on Windows, not a runtime surprise. The hnswlib tests job stays Linux-only: the accelerator has no Windows wheels and building it on the runner adds flake for no extra coverage of compat. --- .github/workflows/ci.yml | 17 ++++++++++++++--- .github/workflows/pylint.yml | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b52969e..a6f5eb0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,9 +29,18 @@ jobs: no-dependencies: # The zero-dependency claim is a promise to users, so it gets its own job: install - # the package alone and prove the core still imports and passes. - runs-on: ubuntu-latest - timeout-minutes: 10 + # the package alone and prove the core still imports and passes. The matrix is + # where Windows gets covered (#7): every platform primitive the package touches + # lives in pyvdisk/compat.py, and the emulation tests force those paths on Linux, + # but only a real Windows runner exercises msvcrt.locking, the directory-fsync + # no-op and drive-letter paths against the actual OS. PR #5 proved the port + # works; this job keeps proving it on every push. + runs-on: ${{ matrix.os }} + timeout-minutes: 15 + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 @@ -47,3 +56,5 @@ jobs: python -c "import pyvdisk; print('pyvdisk', pyvdisk.__version__, 'imported with the standard library only')" - name: Tests (accelerator-dependent cases skip) run: python -m pytest -q --no-header -p no:cacheprovider + - name: Quickstart smoke (stdlib only, end to end) + run: python examples/quickstart.py diff --git a/.github/workflows/pylint.yml b/.github/workflows/pylint.yml index 157709b..5461c8c 100644 --- a/.github/workflows/pylint.yml +++ b/.github/workflows/pylint.yml @@ -26,3 +26,24 @@ jobs: # declared 3.9 floor still parses is proven by the CI test matrix, not here. - name: Lint (errors only) run: python -m pylint -E --score=n pyvdisk tests + + errors-windows: + # Same errors-only gate on Windows (#7). pylint -E resolves imports against + # the running interpreter, so an unguarded `import fcntl` -- the exact bug + # that made PR #5 necessary -- becomes an import-error here instead of a + # runtime surprise on a user's machine. The msvcrt half of compat.py is + # likewise only *real* on Windows, so attribute checks see it bound. + runs-on: windows-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Install + run: | + python -m pip install --upgrade pip + python -m pip install -e .[dev] + python -m pip install "pylint==4.0.9" + - name: Lint (errors only) + run: python -m pylint -E --score=n pyvdisk tests From b4cf01641a8c7dd5a096a629867e032501c0de13 Mon Sep 17 00:00:00 2001 From: PyVDisk Team Date: Wed, 30 Sep 2026 01:55:47 +0800 Subject: [PATCH 2/5] fix(compat): one positional-IO lock per descriptor, not one per process (#8) PR #5's disk.py locked emulated positional IO per fd; the compat layer collapsed that into a single module-level RLock, which on Windows serialises block IO across *every* open DataDisk -- disk A's writes queue disk B's. The docstring's 'not a global lock' claim did not survive the refactor it was written for. Restore the per-descriptor design from the PR: _PIO_LOCKS maps fd -> RLock, created on demand behind a tiny guard. Entries are not pruned -- a reused fd number shares its predecessor's lock, which can only add contention, never incorrectness, and pruning would need a close hook this layer does not own. Two tests pin the behaviour: same fd reuses one lock and different fds never share one, and IO on descriptor b completes while descriptor a's lock is held -- the exact scenario the global lock froze. --- pyvdisk/compat.py | 30 +++++++++++++++++++++++++----- tests/test_compat.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/pyvdisk/compat.py b/pyvdisk/compat.py index a096313..874b98a 100644 --- a/pyvdisk/compat.py +++ b/pyvdisk/compat.py @@ -16,8 +16,12 @@ Why a lock on the emulated positional IO: ``os.pread``/``os.pwrite`` are atomic, ``lseek`` + ``read`` is not. Two threads sharing one handle could interleave and read the wrong block, so the emulation serialises through a -module-level lock. That costs throughput on Windows only, and only while a -block IO is in flight; it is not a global lock over the whole disk. +**per-descriptor** lock: one handle queues behind itself, different handles +-- and therefore different disks -- never wait on each other. Stale entries +after a descriptor is closed are harmless: an fd number reused by a new file +at worst shares a lock with its predecessor and picks up spurious +contention, never incorrectness. That costs throughput on Windows only, and +only while a block IO on the *same* handle is in flight. The Windows byte-range lock covers ``[0, LOCK_BYTES)`` -- the same "whole file" scope ``flock`` gives us, because every handle in this package locks from @@ -62,7 +66,23 @@ #: ``pread``/``pwrite``. _HAS_POSITIONAL = hasattr(os, "pread") and hasattr(os, "pwrite") -_POSITIONAL_LOCK = threading.RLock() +#: One lock per descriptor, created on demand (issue #8: the original +#: module-level lock serialised every open disk behind one RLock). Entries +#: are intentionally not pruned: an fd number reused after close shares its +#: predecessor's lock, which can only add contention, never break correctness, +#: and pruning would need a close hook this layer does not own. +_PIO_LOCKS: dict = {} +_PIO_GUARD = threading.Lock() + + +def _pio_lock(fd: int) -> threading.RLock: + """The emulation lock for this descriptor -- same fd, same lock, always.""" + with _PIO_GUARD: + lock = _PIO_LOCKS.get(fd) + if lock is None: + lock = threading.RLock() + _PIO_LOCKS[fd] = lock + return lock #: Errnos that mean "the lock is held by someone else" (or "retry the syscall"): #: ``flock(LOCK_NB)`` raises EAGAIN/EWOULDBLOCK (and EACCES on some systems), @@ -135,7 +155,7 @@ def pread(fd: int, length: int, offset: int) -> bytes: """ if _HAS_POSITIONAL and implementation() == "posix": return os.pread(fd, length, offset) - with _POSITIONAL_LOCK: + with _pio_lock(fd): os.lseek(fd, offset, os.SEEK_SET) return os.read(fd, length) @@ -147,7 +167,7 @@ def pwrite(fd: int, data: bytes, offset: int) -> int: """ if _HAS_POSITIONAL and implementation() == "posix": return os.pwrite(fd, data, offset) - with _POSITIONAL_LOCK: + with _pio_lock(fd): os.lseek(fd, offset, os.SEEK_SET) return os.write(fd, data) diff --git a/tests/test_compat.py b/tests/test_compat.py index b20484b..62b298d 100644 --- a/tests/test_compat.py +++ b/tests/test_compat.py @@ -19,6 +19,7 @@ import os import re import sys +import threading import time from pathlib import Path @@ -232,6 +233,48 @@ def test_emulated_positional_io_matches_the_native_one(tmp_path, windows_emulati os.close(fd) +def test_positional_io_lock_is_per_descriptor(windows_emulation): + """Same fd shares one lock; different fds get different locks (#8). + + The pre-#8 module-level lock returned the same RLock for every handle, + which serialised separate disks against each other on Windows. The + assertion only needs descriptors, not files -- a pipe pair provides them. + """ + r, w = os.pipe() + try: + assert compat._pio_lock(w) is compat._pio_lock(w), "same fd, same lock" + assert compat._pio_lock(w) is not compat._pio_lock(r), "different fds must not share a lock" + finally: + os.close(r) + os.close(w) + + +def test_pio_on_one_descriptor_does_not_block_another(tmp_path, windows_emulation): + """While handle A is mid-pwrite, handle B must still make progress (#8). + + Regression for the module-level lock: under it, the worker thread would + wait for A's lock and never finish. This holds a lock on one descriptor + and proves IO on another descriptor completes regardless. The descriptors + must be seekable files: the emulated pwrite is lseek + write, and pipes + would fail with ESPIPE regardless of locking. + """ + a = os.open(str(tmp_path / "a.bin"), os.O_CREAT | os.O_RDWR) + b = os.open(str(tmp_path / "b.bin"), os.O_CREAT | os.O_RDWR) + done = [] + worker = threading.Thread( + target=lambda: (compat.pwrite(b, b"ok", 0), done.append(True)) + ) + with compat._pio_lock(a): # simulate disk A with a block IO in flight + worker.start() + worker.join(timeout=5) + try: + assert done == [1], "IO on descriptor b must not wait for descriptor a's lock" + assert compat.pread(b, 2, 0) == b"ok" + finally: + os.close(a) + os.close(b) + + def test_emulated_locking_contends_and_times_out(tmp_path, windows_emulation): first, second = _lock_file(tmp_path), _lock_file(tmp_path) try: From c49be71adc9d32ebdeafb9d710bcfc0039baaccb Mon Sep 17 00:00:00 2001 From: PyVDisk Team Date: Wed, 30 Sep 2026 01:55:47 +0800 Subject: [PATCH 3/5] fix(sandbox): refuse fs verbs the door does not carry, in words (#9) _SandboxFS wrapped move/copy/link/symlink/readlink/truncate/chmod/chown/ utime, but the capability-scoped view underneath implements none of them: the wrappers could only ever die with a bare AttributeError. The VScript stdlib advertises the full disk's fs API, so an agent script that reached for one of these verbs got an error that read like a sandbox bug, giving the model nothing to correct with. Drop the dead wrappers and make __getattr__ refuse anything the scoped view does not implement with the carried-verb list instead -- which also keeps future scoped verbs from leaking into the sandbox door unreviewed. Path-less diagnostics (df, fsck, statfs) still pass straight through, and stat() now forwards follow=, which stdlib fs_chmod/fs_chown rely on. The new test drives the refusal through run_script for symlink/chmod/ truncate and asserts the message names the verbs that do exist. --- pyvdisk/sandbox.py | 65 ++++++++++++++++++++----------------- tests/test_agent_sandbox.py | 18 ++++++++++ 2 files changed, 53 insertions(+), 30 deletions(-) diff --git a/pyvdisk/sandbox.py b/pyvdisk/sandbox.py index c2ace50..ac17eac 100644 --- a/pyvdisk/sandbox.py +++ b/pyvdisk/sandbox.py @@ -77,15 +77,40 @@ class _SandboxFS: rules, and ``fs.listdir(sandbox, "/")`` would hand the model the very paths ``list_files`` refuses to show it. Paths are translated here and the calls are forwarded, so the capability-scoped namespace underneath still applies. + + The door carries exactly the verbs defined below (plus the path-less + diagnostics the scoped view itself provides, such as ``df`` and ``fsck``). + The VScript stdlib advertises the full disk's fs API -- ``fs.symlink``, + ``fs.chmod`` and friends -- so an agent script that reaches for one of the + verbs the sandbox does not carry must be told so *in words*, not with a + bare AttributeError from the missing scoped method (#9). """ + #: The verbs this door actually carries, for the refusal message. + VERBS = ( + "append_file", "df", "du", "exists", "fsck", "isdir", "isfile", + "listdir", "listdir_with_stat", "mkdir", "makedirs", "open", + "read_file", "remove", "rename", "rmtree", "stat", "statfs", + "walk", "write_file", + ) + def __init__(self, target: Any, translate: Any): self._target = target self._translate = translate def __getattr__(self, name: str): - """Everything without a path (``df``, ``fsck``, ...) is passed straight on.""" - return getattr(object.__getattribute__(self, "_target"), name) + """Path-less diagnostics (``df``, ``fsck``, ``statfs``) pass straight on. + + Anything else the scoped view does not implement is refused with the + carried-verb list, so a script (and the model reading the result) + learns what the sandbox *does* offer instead of guessing. + """ + try: + return getattr(object.__getattribute__(self, "_target"), name) + except AttributeError: + raise SandboxError( + f"fs.{name} 不在沙箱内可用；可用动词: {', '.join(self.VERBS)}" + ) from None @staticmethod def _keep(top: str, name: str) -> bool: @@ -134,8 +159,9 @@ def isdir(self, path: str) -> bool: target = self._translate(path) return False if _hidden(target) else self._target.isdir(target) - def stat(self, path: str) -> Any: - return self._forward("stat", path) + def stat(self, path: str, *args: Any, **kwargs: Any) -> Any: + # follow= passes through: stdlib fs_chmod/fs_chown stat with follow. + return self._forward("stat", path, *args, **kwargs) def read_file(self, path: str) -> bytes: return self._forward("read_file", path) @@ -161,17 +187,11 @@ def rmtree(self, path: str, *args: Any, **kwargs: Any) -> Any: def rename(self, source: str, destination: str, *args: Any, **kwargs: Any) -> Any: return self._target.rename(self._translate(source), self._translate(destination), *args, **kwargs) - def move(self, source: str, destination: str, *args: Any, **kwargs: Any) -> Any: - return self._target.move(self._translate(source), self._translate(destination), *args, **kwargs) - - def copy(self, source: str, destination: str, *args: Any, **kwargs: Any) -> Any: - return self._target.copy(self._translate(source), self._translate(destination), *args, **kwargs) - - def link(self, source: str, destination: str, *args: Any, **kwargs: Any) -> Any: - return self._target.link(self._translate(source), self._translate(destination), *args, **kwargs) - - def symlink(self, source: str, destination: str, *args: Any, **kwargs: Any) -> Any: - return self._target.symlink(self._translate(source), self._translate(destination), *args, **kwargs) + # move/copy/link/symlink/readlink/truncate/chmod/chown/utime are deliberately + # NOT carried: the scoped view implements none of them, and the Python tool + # door does not offer them either. A script that calls one now gets the + # refusal from __getattr__ above, with the carried-verb list, instead of a + # bare AttributeError that reads like a bug in the sandbox (#9). def open(self, path: str, *args: Any, **kwargs: Any) -> Any: return self._forward("open", path, *args, **kwargs) @@ -179,21 +199,6 @@ def open(self, path: str, *args: Any, **kwargs: Any) -> Any: def du(self, path: str = "/") -> Any: return self._forward("du", path) - def readlink(self, path: str) -> Any: - return self._forward("readlink", path) - - def truncate(self, path: str, *args: Any, **kwargs: Any) -> Any: - return self._forward("truncate", path, *args, **kwargs) - - def chmod(self, path: str, *args: Any, **kwargs: Any) -> Any: - return self._forward("chmod", path, *args, **kwargs) - - def chown(self, path: str, *args: Any, **kwargs: Any) -> Any: - return self._forward("chown", path, *args, **kwargs) - - def utime(self, path: str, *args: Any, **kwargs: Any) -> Any: - return self._forward("utime", path, *args, **kwargs) - class SandboxError(Exception): """The sandbox refused an operation before it reached the disk.""" diff --git a/tests/test_agent_sandbox.py b/tests/test_agent_sandbox.py index dd957cb..4ceba7b 100644 --- a/tests/test_agent_sandbox.py +++ b/tests/test_agent_sandbox.py @@ -196,6 +196,24 @@ def test_scripts_cannot_touch_container_bookkeeping(box): assert not result.ok and "bookkeeping" in result.content, source +def test_fs_verbs_outside_the_sandbox_door_refuse_with_guidance(box): + """The stdlib advertises the full disk's fs API; the sandbox carries a subset. + + A script reaching for a verb the door does not carry (fs.symlink, fs.chmod, + ...) used to die with a bare AttributeError from the missing scoped method. + Now it is refused in words, and the words list what IS available (#9). + """ + # symlink refuses immediately; chmod/truncate first stat/read the target, + # so each source creates its file before reaching the refusal. + for source in ('import std.fs as fs; fs.symlink(sandbox, "/a", "/b");', + 'import std.fs as fs; fs.write(sandbox, "/a.txt", "x"); fs.chmod(sandbox, "/a.txt", 420);', + 'import std.fs as fs; fs.write(sandbox, "/b.txt", "x"); fs.truncate(sandbox, "/b.txt", 0);'): + result = box.call("run_script", {"source": source}) + assert not result.ok, source + assert "不在沙箱内可用" in result.content, source + assert "write_file" in result.content and "read_file" in result.content, source + + def test_the_script_and_python_doors_share_one_workspace(box): """A file written by a script is an ordinary file for every other verb.""" box.dispatch("run_script", {"source": 'import std.fs as fs; fs.write(sandbox, "/shared.txt", "from script");'}) From ab308ca80ef926d94acffe421fbf338835f19d2d Mon Sep 17 00:00:00 2001 From: PyVDisk Team Date: Wed, 30 Sep 2026 01:59:22 +0800 Subject: [PATCH 4/5] fix(lint): scope no-member disables to the platform-guarded os members The new errors-windows job earned its keep on the first push: on Windows, pylint's os brain has no pread/pwrite/getuid/getgid to infer from, so the guarded POSIX branches in compat (and the uid/gid assertions) read as E1101. Disable no-member on exactly those four lines, each with a comment naming the guard -- a bare module-level disable would also hide real no-member bugs on os elsewhere. --- pyvdisk/compat.py | 6 ++++-- tests/test_compat.py | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pyvdisk/compat.py b/pyvdisk/compat.py index 874b98a..06829e9 100644 --- a/pyvdisk/compat.py +++ b/pyvdisk/compat.py @@ -154,7 +154,9 @@ def pread(fd: int, length: int, offset: int) -> bytes: Returns fewer bytes only at end of file, like ``os.pread``. """ if _HAS_POSITIONAL and implementation() == "posix": - return os.pread(fd, length, offset) + # The guard above is invisible to pylint on Windows, where os has no + # pread/pwrite to infer from -- hence the scoped disable. + return os.pread(fd, length, offset) # pylint: disable=no-member with _pio_lock(fd): os.lseek(fd, offset, os.SEEK_SET) return os.read(fd, length) @@ -166,7 +168,7 @@ def pwrite(fd: int, data: bytes, offset: int) -> int: Returns the number of bytes written, like ``os.pwrite``. """ if _HAS_POSITIONAL and implementation() == "posix": - return os.pwrite(fd, data, offset) + return os.pwrite(fd, data, offset) # pylint: disable=no-member with _pio_lock(fd): os.lseek(fd, offset, os.SEEK_SET) return os.write(fd, data) diff --git a/tests/test_compat.py b/tests/test_compat.py index 62b298d..91c6057 100644 --- a/tests/test_compat.py +++ b/tests/test_compat.py @@ -99,8 +99,9 @@ def test_uid_and_gid_are_integers(): assert isinstance(compat.uid(), int) assert isinstance(compat.gid(), int) if os.name == "posix": - assert compat.uid() == os.getuid() - assert compat.gid() == os.getgid() + # pylint on Windows cannot see this guard -- os has no getuid there. + assert compat.uid() == os.getuid() # pylint: disable=no-member + assert compat.gid() == os.getgid() # pylint: disable=no-member else: assert (compat.uid(), compat.gid()) == (0, 0) From 403101a584565c910893a58ab969e21c66f9fc0c Mon Sep 17 00:00:00 2001 From: PyVDisk Team Date: Wed, 30 Sep 2026 02:02:49 +0800 Subject: [PATCH 5/5] fix(test): the host-roots sandbox test interpolates the path as_posix() Found by the new no-dependencies (windows-latest) job, first real-Windows run of the sandbox suite: str(tmp_path) carries backslashes, and the VScript lexer correctly rejects '\U' inside a plain string -- so the test failed with a LexError before it ever reached the disk. Same rule PR #5 taught the host tests; the policy layer takes the mixed separators fine. On Linux as_posix() is a no-op, which is why CI never saw it. Windows suite now stands at 445 passed / 6 skipped, one failure gone. --- tests/test_agent_sandbox.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_agent_sandbox.py b/tests/test_agent_sandbox.py index 4ceba7b..48a9392 100644 --- a/tests/test_agent_sandbox.py +++ b/tests/test_agent_sandbox.py @@ -235,8 +235,11 @@ def test_host_roots_are_opt_in(tmp_path): allowed.mkdir() (allowed / "input.txt").write_text("from the host", encoding="utf-8") with AgentSandbox.create(str(tmp_path / "host.vdisk"), host_read_roots=[str(allowed)]) as box: + # as_posix(): backslashes cannot appear in a plain VScript string + # (the lexer refuses \U etc.), the same rule PR #5 taught the host + # tests; the policy layer accepts the mixed separators on Windows. result = box.call("run_script", { - "source": f'import std.host as host; print(host.read("{allowed}/input.txt"));', + "source": f'import std.host as host; print(host.read("{allowed.as_posix()}/input.txt"));', }) assert result.ok and "from the host" in result.content