Skip to content

fix: make the agent run on Windows (fcntl import, path guard, home fallback) - #73

Open
abahanad69 wants to merge 1 commit into
ianalloway:mainfrom
abahanad69:fix/windows-portability
Open

abahanad69 wants to merge 1 commit into
ianalloway:mainfrom
abahanad69:fix/windows-portability

Conversation

@abahanad69

Copy link
Copy Markdown

Fixes the three defects that made the package unusable on Windows, despite the
Operating System :: OS Independent classifier and the Windows instructions in the docs.

I hit these running the demo on Windows 11 / Python 3.11: solvent init reported
treasury DB: No module named 'fcntl', and the CLI tracebacked on import solvent.treasury.

1. Unix-only import at module scope

treasury.py did import fcntl at the top, so importing the module raised
ModuleNotFoundError: No module named 'fcntl' on Windows. That took out the CLI and
20 of the 48 test modules, which could not even be collected.

Added solvent/_filelock.py -- a small portable advisory lock (fcntl.flock on POSIX,
msvcrt.locking over a one-byte range on Windows with a poll loop, since bare LK_LOCK
gives up after ~10s and that is not flock semantics). Both lock sites now go through it.

2. Hardcoded / in the path-traversal guard

safe_report_path tested containment with:

if not str(candidate).startswith(str(base) + "/") and candidate != base:

Path.resolve() returns backslash-separated strings on Windows, so this never matched and
every 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:

[J1] Escrow Refunded: Returned $49.00 (report path ... escapes output directory ...)
Net profit $0.00 (0.0% margin)   Cash balance $100.00

Now Path.is_relative_to(base).

3. Path.home() ignores $HOME on Windows

The documented fallback is ~/.solvent, but Path.home() reads USERPROFILE on Windows, so
relocating the home directory by exporting HOME silently wrote to the real one.
HOME now wins (it is the documented contract) with USERPROFILE as the native fallback,
translating MSYS-style /c/Users/name values so Git-Bash cannot produce a bogus C:\c\Users\name.

Also

  • RateLimiter opened a SQLite handle in __init__ and had no way to release it, which keeps
    the database file locked on Windows for the process lifetime. Added close() and
    context-manager support, and closed the handles the persistence tests leaked.
  • A dangling sqlite3.connect(...) in test_metrics_block blocked tmpdir teardown on Windows.
  • Added tests/test_filelock.py: acquire/release by file object and by raw fd, non-blocking
    refusal, idempotent release, re-acquisition.

Verification

before after
test collection errors 20 0
result could not run 544 passed, 10 skipped, 0 failed

The 10 skips are optional extras (qrcode, fastapi, python-telegram-bot) that were not
installed. ruff check and ruff format --check are clean.

python run_demo.py --seed 100 now books revenue with no spurious refunds, declines the
below-floor job correctly, and writes its reports:

[J1] booked P&L: profit $48.45 | treasury cash balance: $148.45
[J2] booked P&L: profit $74.45 | treasury cash balance: $222.90
[J3] Declined: order $6 below minimum order size $15
[..] Net profit $345.80 (99.4% margin)   Cash balance $445.80 (seed was $100.00)

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant