fix: make the agent run on Windows (fcntl import, path guard, home fallback) - #73
Open
abahanad69 wants to merge 1 commit into
Open
abahanad69 wants to merge 1 commit into
abahanad69 wants to merge 1 commit into
Conversation
Three portability defects made the package unusable on Windows despite the "OS Independent" classifier and the Windows instructions in the docs. 1. `treasury.py` imported `fcntl` at module scope. `fcntl` is Unix-only, so importing `solvent.treasury` raised ModuleNotFoundError and took the CLI plus 20 of the 48 test modules with it. Added `solvent/_filelock.py`, a small portable advisory lock (fcntl.flock on POSIX, msvcrt.locking with a poll loop on Windows), and routed both lock sites through it. 2. `security.safe_report_path` tested containment with `str(candidate).startswith(str(base) + "/")`. The hardcoded POSIX separator never matches the backslash-separated strings `Path.resolve()` returns on Windows, so every legitimate report path was rejected as a traversal attempt. The escrow guard then refunded every accepted job and the agent could never book a profit. Use `Path.is_relative_to`. 3. `paths.base_dir()` resolved the fallback home via `Path.home()`, which ignores `$HOME` on Windows. Honour `HOME` (the documented contract) with `USERPROFILE` as the native fallback, translating MSYS-style `/c/Users/x` values so Git-Bash cannot produce a bogus `C:\c\Users\x`. Also: - `RateLimiter` held an open SQLite handle for its whole lifetime with no way to release it, which keeps the database file locked on Windows. Added `close()` and context-manager support, and closed the handles the persistence tests leaked. - Closed a dangling connection in test_metrics_block that blocked tmpdir teardown on Windows. - Added tests/test_filelock.py covering acquire/release, non-blocking refusal, idempotent release and re-acquisition. Verified: 544 passed, 10 skipped, 0 failed (the skips are optional extras that are not installed). `ruff check` and `ruff format --check` clean. `python run_demo.py --seed 100` books revenue with no spurious refunds and writes its reports.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the three defects that made the package unusable on Windows, despite the
Operating System :: OS Independentclassifier and the Windows instructions in the docs.I hit these running the demo on Windows 11 / Python 3.11:
solvent initreportedtreasury DB: No module named 'fcntl', and the CLI tracebacked onimport solvent.treasury.1. Unix-only import at module scope
treasury.pydidimport fcntlat the top, so importing the module raisedModuleNotFoundError: No module named 'fcntl'on Windows. That took out the CLI and20 of the 48 test modules, which could not even be collected.
Added
solvent/_filelock.py-- a small portable advisory lock (fcntl.flockon POSIX,msvcrt.lockingover a one-byte range on Windows with a poll loop, since bareLK_LOCKgives up after ~10s and that is not
flocksemantics). Both lock sites now go through it.2. Hardcoded
/in the path-traversal guardsafe_report_pathtested containment with:Path.resolve()returns backslash-separated strings on Windows, so this never matched andevery legitimate report path was rejected as a traversal attempt. The escrow guard then
refunded every accepted job, so the agent could not book a profit at all:
Now
Path.is_relative_to(base).3.
Path.home()ignores$HOMEon WindowsThe documented fallback is
~/.solvent, butPath.home()readsUSERPROFILEon Windows, sorelocating the home directory by exporting
HOMEsilently wrote to the real one.HOMEnow wins (it is the documented contract) withUSERPROFILEas the native fallback,translating MSYS-style
/c/Users/namevalues so Git-Bash cannot produce a bogusC:\c\Users\name.Also
RateLimiteropened a SQLite handle in__init__and had no way to release it, which keepsthe database file locked on Windows for the process lifetime. Added
close()andcontext-manager support, and closed the handles the persistence tests leaked.
sqlite3.connect(...)intest_metrics_blockblocked tmpdir teardown on Windows.tests/test_filelock.py: acquire/release by file object and by raw fd, non-blockingrefusal, idempotent release, re-acquisition.
Verification
The 10 skips are optional extras (
qrcode,fastapi,python-telegram-bot) that were notinstalled.
ruff checkandruff format --checkare clean.python run_demo.py --seed 100now books revenue with no spurious refunds, declines thebelow-floor job correctly, and writes its reports: