diff --git a/pyproject.toml b/pyproject.toml index da00ee259..410d7f7ae 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,6 +7,7 @@ asyncio_mode = "auto" # tests/conftest.py, so unknown-mark warnings still flag genuine typos outside # the taxonomy. See tests/_taxonomy.py and tests/README.md. markers = [ + "serial: live smoke tests mutate one externally launched application; use -n 0", "area_security: tests covering auth, owner-scope, SSRF, XSS, confinement, redaction", "area_routes: tests covering HTTP route / API behavior", "area_services: tests covering service-layer behavior (llm, cookbook, email, calendar, ...)", diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 000000000..f99573c1d --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,4 @@ +# The complete application environment plus local parallel test tooling. +-r requirements.txt +# psutil lets `-n auto` use physical cores instead of logical CPU threads. +pytest-xdist[psutil]>=3.8,<4 diff --git a/src/agent_loop.py b/src/agent_loop.py index 86c3418c2..091b33323 100644 --- a/src/agent_loop.py +++ b/src/agent_loop.py @@ -8647,6 +8647,11 @@ def _web_search_unavailable_for_turn( """ if "web" not in set(intent_domains or ()): return False + # Mentioning web only to forbid its use is not affirmative web demand. + # Keep the web tools disabled, but let the normal model-only path answer + # instead of claiming the request cannot proceed without web access. + if _explicitly_avoids_web_lookup(text): + return False # A turn is unavailable only when every public-web route is disabled. # Exact-URL turns intentionally expose web_fetch while keeping broad # web_search disabled; the former intersection check incorrectly diff --git a/tests/README.md b/tests/README.md index 62e1036cf..30d56a84c 100644 --- a/tests/README.md +++ b/tests/README.md @@ -15,13 +15,12 @@ reference; that file is the standard the refactor works toward. ## Running focused subsets (taxonomy markers) -The shared static-server fixture defaults to loopback port 7011 and refuses an -occupied port rather than reusing another checkout's server. For focused tests -that do not load that fixed browser URL, use `ODYSSEUS_TEST_STATIC_PORT=0` to -allocate an ephemeral port. This permits direct subprocess/isolation tests on a -host already serving the application without stopping or changing that service. -Browser tests that hard-code port 7011 still need that port in their own isolated -network namespace; do not run them against an unrelated live server. +The shared static-server fixture binds an ephemeral loopback port and publishes +`ODYSSEUS_TEST_STATIC_ORIGIN` to browser tests and their Node subprocesses. +`ODYSSEUS_TEST_STATIC_PORT` can pin a port for an external client; leave it unset +for parallel runs. An occupied explicit port is refused rather than reused. The +server handles each connection on its own thread, so a speculative browser +connection that never sends a request cannot stall the requests behind it. `tests/conftest.py` tags every test at collection time with two markers derived from its filename by `tests/_taxonomy.py`: an `area_*` marker (e.g. @@ -126,6 +125,43 @@ the run with a usage error rather than quietly testing a subset. If you change the shard count, change `DEFAULT_SHARD_COUNT` and the `ci.yml` matrix together; `tests/test_shards.py` fails when they drift apart. +## Local pytest workers + +Install the application environment and parallel test tooling with +`python -m pip install -r requirements-dev.txt`. Parallelism is opt-in; ordinary +pytest remains serial, and the four CI shards are unchanged. + +```bash +python -m pytest -q -n 4 -p no:cacheprovider --max-worker-restart=0 +python -m pytest -q -n 0 -p no:cacheprovider # full serial release oracle +``` + +See [the Wave 6 measurements](WAVE6_TEST_PERFORMANCE_REPORT.md) before choosing +a worker count. `-n auto` uses physical cores through xdist's psutil extra; it +still needs enough memory for each worker's collection and application imports. + +Before collection, `tests.helpers.worker_runtime` gives each process private +data, attachment, embedding-cache, browser-runtime, and temporary directories. +pytest's basetemp is the controller root's `pytest` directory, with xdist's +`popen-gw` beneath it, which keeps `tmp_path` Unix sockets inside the +107-byte path limit. An explicit `--basetemp` still wins. +An atomic random suffix separates concurrent invocations with the same worker +label; paths inside the root have stable names. Function fixtures still own +their databases and test-specific state. Normal teardown removes the root and +restores the environment. A hard-killed standalone process can leave its owned +root behind, but later runs allocate a fresh namespace and never adopt it. + +`APP_PORT` opts into tests against an externally launched smoke application. +Run that suite with `-n 0` so its accounts, endpoints, and application data have +one owner. Unset `APP_PORT` for ordinary unit/regression invocations. Selected +live smoke tests are rejected under xdist with a collection failure; `-m +"not serial"` may exclude them. The existing smoke skips when no application +is launched remain visible in full-suite counts. + +Measurements disable pytest's advisory cache to keep concurrent invocations +from sharing last-failed metadata. They also disable worker restart so crashes +remain immediately visible. No test retries or default worker count are added. + ## Order-sensitivity reporting (report-only) `tests/run_order_report.py` runs pytest with the collected test items shuffled diff --git a/tests/WAVE6_TEST_PERFORMANCE_REPORT.md b/tests/WAVE6_TEST_PERFORMANCE_REPORT.md new file mode 100644 index 000000000..aee27953b --- /dev/null +++ b/tests/WAVE6_TEST_PERFORMANCE_REPORT.md @@ -0,0 +1,215 @@ +# Wave 6 test performance and worker isolation + +Starting branch: `wave6/test-performance-foundation`. +Starting HEAD: `bfa5ebb379713790bd1b47c75237fb1fbe989375`. +Starting tree: `3aa232c1c7d479d7ed48e22aa59ddfe8f53058ad`. + +The interrupted working tree was inspected before edits. It contained changes +in `pyproject.toml`, `tests/conftest.py`, `tests/test_research_report_read.py`, +and `tests/test_stt_leak.py`, plus untracked `requirements-dev.txt`, +`tests/helpers/worker_runtime.py`, and `tests/test_worker_runtime.py`. +All seven files belonged to this lane. No working-tree state was discarded. + +The interrupted implementation established private filesystem defaults before +application imports, isolated saved research reports and STT temporary-file +observations, declared optional xdist tooling, and guarded externally launched +smoke tests. The temporary-directory context manager, restoration tests, +subprocess inheritance test, report fixture, and STT fixture were coherent. +The root integration and refusal diagnostics needed additional validation and +correction. None of the recovered files was rejected or replaced wholesale. + +The final root wiring preserves the existing generated configuration-reference +locations and does not add test fixture environment reads to the public +configuration inventory. The first serial profile exposed two stale-reference +failures caused by the interrupted wiring. Both are fixed within test code; +`website/configuration-reference.md` and its generator remain unchanged. + +The live-smoke guard now runs after marker/shard deselection, before xdist +publishes runnable items. It emits a normal collection failure. Raising a +worker UsageError after notification had produced an xdist internal error and +lost the useful refusal message; the diagnostic probe exposed this and the +final probe proves the intended failure is visible. No smoke request executes +in that refused invocation. + +## Resource ownership + +`tests/helpers/worker_runtime.py` owns an atomically created temporary root for +each pytest process. The worker label is diagnostic, not an allocation key. +The random suffix prevents two independent invocations of `gw0` from adopting +each other's files. Paths below that root are stable: `data`, `mail`, +`fastembed`, `runtime`, `tmp`, and the controller's `pytest` basetemp. This is +deterministic ownership, not a fixed reusable path. Ordinary serial runs use +the same architecture with label `main`; xdist knowledge stays in the helper +and root hooks. + +| Resource | Isolation boundary | +| --- | --- | +| DATABASE_URL and collection engine | Existing foundation forces `sqlite:///:memory:` before import in each process; inherited developer URLs are never opened. | +| File-backed SQLite | Existing fixtures own temporary engines/files, restore bindings without replacing ORM classes, dispose engines before deletion. | +| Postgres | Audited tests use mock dialects/DDL engines; this lane does not connect workers to a shared Postgres database. | +| Runtime data, jobs, publications, process/browser registries | Existing constants derive from private ODYSSEUS_DATA_DIR before collection; narrower fixtures still patch their owned roots. | +| Effects/provenance/evidence | In-memory structures are process-local; file-backed resource publications live under private data/process/browser roots or explicit fixture workspaces. | +| Attachments and embedding cache | Dedicated environment overrides point inside the owned root. | +| Search/cache/generated artifacts | Data-derived caches use the owned data root; explicit artifact fixtures use tmp_path. Installed source/browser assets are read together. | +| Temporary files and shell logs | TMPDIR/TMP/TEMP and cached tempfile.tempdir point into the owned root and are inherited by subprocesses. pytest's basetemp is the controller root's `pytest` directory, with distinct xdist `popen-gw` worker basetemps beneath it. | +| Browser profiles and Unix sockets | Private runtime/data roots; existing short AF_UNIX socket helper owns atomic /tmp directories rather than long pytest paths. | +| Static HTTP servers | Each process holds its kernel-assigned loopback socket; fixed external pins are rejected for parallel execution. Connections get their own threads, so a silent client cannot stall others. | +| Chroma refusal probes | Bound non-listening sockets reserve closed ports until teardown, replacing bind-and-release guesses. | +| Launcher refusal probe | An owned ephemeral listener exercises foreign-server refusal without a host-dependent derived-port skip. Port derivation retains its separate assertions. | +| Process tests | Sleeper fixture owns process sessions/groups, signals the group before reaping its leader, and propagates cleanup errors; it previously killed only the shell and swallowed errors. | +| Module globals/environment | Process-local under xdist; foundation import-state/database restoration guards and test monkeypatches remain intact. | +| Pytest advisory cache | Measurements disable cacheprovider. Built-in worker last-failed/node-ID writers are suppressed by pytest; concurrent controllers should not share advisory metadata. No suite test uses the cache fixture. | +| Live smoke application | Explicit APP_PORT opts into external application ownership; selected live smoke tests require -n 0. Existing no-application skips remain visible. | + +Normal cleanup restores the caller's environment/tempfile cache and removes +only the root owned by that context. Cleanup exceptions are not suppressed. +A hard-killed standalone process can leave its root behind; subsequent runs +allocate a different root, so abandoned state is not reused. This is isolation +against crash residue, not a claim that arbitrary SIGKILL removes every file +or process. No global sweeper, kill-by-name, PID-derived namespace, or guessed +per-worker port was added. + +The audit found actual filesystem/port/process fixture boundaries to repair, +not a need to serialize ordinary unit tests. Process-local globals alone are +not cross-worker shared state. A delegated broad audit was partial and offered +speculative concerns; those were checked locally rather than accepted as +proven defects. + +## Dependencies and release modes + +Foundation requirements declared pytest and pytest-asyncio, but not xdist. +The recovered development declaration is retained: `requirements-dev.txt` +includes `requirements.txt` and `pytest-xdist[psutil]>=3.8,<4`. Runtime dependency +requirements remain unchanged. Installed tooling: pytest 9.1.1, xdist 3.8.0, +psutil 7.2.2, Python 3.11. Physical cores: 8; logical CPUs: 16. The psutil extra +makes auto choose physical cores on this host. + +Parallelism stays opt-in. The full serial release oracle and four deterministic +CI shards remain unchanged. The two strict negative-web production xfails +remain xfails: memory-only turns are still incorrectly short-circuited with +"Web access is disabled." No production/runtime file, Wave 4 behavior, CI +workflow, or production contract was repaired in this lane. + +## Takeover after the interrupted session + +Implementation commits `a94fc54c` and `915e6ed1` were inherited at takeover +HEAD `915e6ed12571b0c9036a3546c334c3561443a310`, tree +`6a646603ace8e40e1497212c85ef47b57443f043`. The uncommitted README section and +this report were retained and completed. The inherited commits were audited +against their diffs and kept unchanged. + +Two full-suite `-n 2` runs on that HEAD each failed one test. Neither failure +was a product defect, and both are fixed in test infrastructure: + +1. `test_legacy_cleanup_against_a_private_real_tmux_server` failed + deterministically under any worker count with `File name too long`. This + lane had moved pytest's default basetemp beneath the private `TMPDIR`, + which gave + `/tmp/ody-main-XXXXXXXX/tmp/pytest-of-/pytest-0/popen-gw0//tmux.sock`, + 110 bytes against Linux's 107-byte AF_UNIX limit. The same path was 87 bytes + before this lane and 100 bytes in a serial run, which is why the serial + profile passed. The controller now sets basetemp to the private root's + `pytest` directory, and xdist hands each worker `popen-gw` beneath it. + That path is now 81 bytes regardless of the username length. An explicit + `--basetemp` is still honoured. `test_worker_runtime.py` binds an AF_UNIX + socket at the same path budget without needing tmux, and fails under + `-n 2` without the fix. +2. With the basetemp fix in place, the next `-n 2` run had a single different + failure. `test_reordering_two_conflicting_declarations_moves_the_digest` + ran 31.7 s instead of about 5.3 s, and node crashed with + `route.fetch: Request context disposed`. That message hides the real + failure, a 30 s `page.goto` stall: the capture's `finally` closed the + browser while a stylesheet `route.fetch` was still pending. The shared + static server was a single-threaded `socketserver.TCPServer`. Chromium + sometimes opens a speculative connection that never sends a request, and + every queued request then waited behind it. A server-instrumented capture + loop under full CPU saturation reproduced it: 4 of 12 captures had a + request-less connection holding the server for about 28.9 s, and those + captures failed. With a threaded server, 12 of 12 captures passed. The idle + connection still appeared in 5 of them but lived 1.0 to 1.5 s without + blocking anything. The fixture now uses `ThreadingTCPServer` with daemon + handler threads. `test_static_server_serves_this_worktree` holds a silent + connection open while it fetches; against the serial server, that request + times out after 5 s. This defect predates the lane and depends on load, + which parallel execution increases. Assertions, timeouts, and the capture + harness are unchanged. + +Both fixes keep the existing first-location lines that +`website/configuration-reference.md` records, and that page is unchanged. The +abandoned root of the first `-n 2` run, killed by its monitor, was +`/tmp/ody-main-x7hmcf4_` (261 MB, last written before takeover). No process +held it, and it was removed. No later run left a root behind. + +## Measurements + +Host: Python 3.11.15, pytest 9.1.1, pytest-xdist 3.8.0, psutil 7.2.2, 8 physical +cores, 16 logical CPUs, 29 GiB RAM. Every run collects the same full suite. The +command is `python -m pytest -q -p no:cacheprovider --durations=100 -rsx +--max-worker-restart=0 -o faulthandler_timeout=60 -n `. The serial +performance-lane profile predates the last option pair. The measurement wrapper +records wall time and descendant RSS. After each run it checks for surviving +descendants (pid plus create time), new TCP listeners, mutated files under +`data/` and the inherited data directory, and leftover `/tmp/ody-main-*` +runtime roots. + +| Run | Code | Workers | Wall s | Passed | Failed | Skipped | XFail | Subtests | Peak RSS | +| --- | --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | +| Isolation foundation oracle | `bfa5ebb3` | 0 | — | 12,556 | 0 | 59 | 2 | 6 | — | +| Lane serial profile | pre-`a94fc54c` wiring | 0 | 506.4 | 12,558 | 2* | 59 | 2 | 6 | — | +| full-n2 | `915e6ed1` | 2 | n/a† | — | 1 (`F` marker) | — | — | — | — | +| full-n2-measured | `915e6ed1` | 2 | 262.1 | 12,559 | 1 (tmux) | 59 | 2 | 6 | 5.5 GB | +| full-n2-r2 | + basetemp fix | 2 | 271.1 | 12,560 | 1 (CSS stall) | 59 | 2 | 6 | 6.3 GB | +| full-n2-r3 | final | 2 | 267.6 | 12,561 | 0 | 59 | 2 | 6 | 5.7 GB | +| full-n4-r1 | final | 4 | 155.7 | 12,561 | 0 | 59 | 2 | 6 | 9.1 GB | +| full-n4-r2 | final | 4 | 155.5 | 12,561 | 0 | 59 | 2 | 6 | 8.7 GB | +| serial-final | final | 0 | 496.3 | 12,561 | 0 | 59 | 2 | 6 | 3.9 GB | + +\* Stale configuration-reference failures from the interrupted wiring, fixed +before `a94fc54c`. † The monitor crashed on a hardened Chromium process +(`AccessDenied`) after pytest reached 100%, so no summary was retained. Its +single progress-line `F` matches the deterministic tmux failure. + +Every run with the final code had no worker crashes, no hangs, no surviving +test descendants, no new listeners, no persistent-state mutations, and no +remaining runtime roots. Passed counts increase only with the lane's added +tests. The skips are the same 59: 16 smoke tests without `APP_PORT` plus +opt-in live and platform gates. The two xfails are the known strict +negative-web production defects. + +Monitoring gaps: the wrapper could not read `/proc//environ` for two to +four short-lived `python` descendants per run (`AccessDenied`). That only limits +worker-ID observation. Leak detection uses process identity, not the +environment, and reported nothing. Hardened Chromium processes are tolerated +as unreadable instead of aborting the measurement. No unrelated process was +signalled. + +### Scaling and recommendation + +Compared with the 496.3 s final serial oracle, two workers are 1.85x faster +(267.6 s) and four workers are 3.19x faster (155.6 s mean). Going from two to +four workers is 1.72x faster. Each worker collects 12,622 items in about 11 s +(0.9 GB RSS) before running anything. The longest single test is the +computed-style capture at about 23 s. The auth-concurrency, rich-document +browser, media, and generated-reference groups keep their real work. + +Recommended local configuration: `-n 4`. Both full repeats were green and +within 0.3 s of each other, at about 2.2 GB RSS per worker. + +`-n auto` (8 workers here) was not run. Projected from the measured per-worker +peak, it needs about 18 GB, against about 21 GB available on this desktop host +while its browser and Chroma services run. Fixed collection cost and the 23 s +longest test bound the best case to roughly 85 to 100 s. The CSS stall above +also showed that browser timing is sensitive to load. That gain does not +justify the risk of swapping or OOM, or of timing distortion. Evaluate six +workers on a host with more headroom before considering `-n auto`. + +The full serial run remains the release oracle and the CI shards are +unchanged. Live smoke tests under `APP_PORT` are the only serial-only tests. +They share one external application, its accounts, and its endpoints, and +are refused under xdist. + +No new product defect was found. A follow-up for test tooling: when a +`route.fetch` handler is still pending, `tests/css_snapshot/capture.mjs` can +replace a navigation error with `Request context disposed`. Fixing that would +make future capture failures easier to diagnose; it is not needed for +correctness here. diff --git a/tests/cli/test_dev_cli_isolation.py b/tests/cli/test_dev_cli_isolation.py index 267d35994..be017ecf2 100644 --- a/tests/cli/test_dev_cli_isolation.py +++ b/tests/cli/test_dev_cli_isolation.py @@ -92,11 +92,11 @@ def test_a_chromadb_we_did_not_start_is_refused_not_adopted(cli, worktree, monke ports = cli.derive_ports(worktree) foreign = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - foreign.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) - try: - foreign.bind(("127.0.0.1", ports["chroma"])) - except OSError: - pytest.skip(f"derived chroma port {ports['chroma']} is unavailable on this host") + foreign.bind(("127.0.0.1", 0)) + ports["chroma"] = foreign.getsockname()[1] + # Port derivation is covered above. This refusal test owns a held socket + # rather than depending on a derived port being free on the host. + monkeypatch.setattr(cli, "derive_ports", lambda _root: ports) foreign.listen(1) try: with pytest.raises(SystemExit): diff --git a/tests/conftest.py b/tests/conftest.py index 0915d519c..d6d60233c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,15 +8,13 @@ import pytest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) -# Importing core.database below runs init_db() at import time, and its default -# (sqlite:///./data/app.db) can't be opened in a clean worktree because SQLite -# won't create the missing ./data parent dir - pytest then dies during -# collection, before any test module loads. Default to an in-memory DB for the -# test session so collection is deterministic and writes no repo-local -# artifacts. An explicit DATABASE_URL (a real test/CI database) is preserved. -# This only unblocks collection/import-time init; it does not provide a shared -# file-backed DB across processes - tests needing that must set DATABASE_URL. -os.environ.setdefault("DATABASE_URL", "sqlite:///:memory:") +# Isolate import-time database and filesystem defaults before collection. +# File-backed databases remain fixture-owned. Cleanup restores the caller. +# Keep the existing generated environment-reference locations stable. +_database_environment = pytest.MonkeyPatch() +_database_environment.setenv("DATABASE_URL", "sqlite:///:memory:") +from tests.helpers.worker_runtime import bootstrap_runtime, configure_runtime +_runtime_environment = bootstrap_runtime() # Pre-import real heavy modules BEFORE any test file's module-level stubs can # replace them with MagicMock. Some test files (e.g. test_llm_core_sanitize_*) @@ -101,6 +99,8 @@ def pytest_configure(config): unknown-mark warnings still surface genuine typos outside the taxonomy. This only registers marker names; it imports no production module. """ + config.add_cleanup(_database_environment.undo) + import pathlib from tests._taxonomy import discover_markers @@ -195,8 +195,8 @@ def _serve_test_static(): return "text/css" return super().guess_type(path) - class _Server(socketserver.TCPServer): - allow_reuse_address = True + class _Server(socketserver.ThreadingTCPServer): + allow_reuse_address = daemon_threads = True requested = int(os.environ.get("ODYSSEUS_TEST_STATIC_PORT") or 0) if not 0 <= requested <= 65535: @@ -214,6 +214,8 @@ def _serve_test_static(): previous_origin = os.environ.get("ODYSSEUS_TEST_STATIC_ORIGIN") os.environ["ODYSSEUS_TEST_STATIC_ORIGIN"] = origin + # One thread per connection: Chromium can hold a speculative connection + # open without a request, which stalled serial service for ~30s. thread = threading.Thread(target=server.serve_forever, daemon=True) thread.start() try: @@ -349,3 +351,37 @@ def _no_context_window_network_probe(request): def context_probe_ledger(_no_context_window_network_probe): """Metadata requests the context resolver attempted during this test.""" return _no_context_window_network_probe + + +# Before pytest's tmpdir plugin reads the basetemp this sets. +@pytest.hookimpl(specname="pytest_configure", tryfirst=True) +def pytest_configure_worker_runtime(config): + configure_runtime(config, _runtime_environment) + + +@pytest.hookimpl(specname="pytest_collection_modifyitems", tryfirst=True) +def pytest_collection_worker_runtime(items): + # Mark before pytest applies -m. The final guard also respects --shard. + for item in items: + if "smoke" in item.path.parts: + item.add_marker(pytest.mark.serial) + + +@pytest.hookimpl(tryfirst=True) +def pytest_collection_finish(session): + """Refuse shared live resources after marker and shard deselection.""" + config = session.config + parallel = bool(getattr(config.option, "numprocesses", None)) or hasattr(config, "workerinput") + if (os.environ.get("APP_PORT") and parallel + and any(item.get_closest_marker("serial") for item in session.items)): + message = ( + "live smoke tests share one external application, accounts, and endpoints; " + "run tests/smoke with -n 0" + ) + # A worker UsageError here races xdist's collection notification and + # can lose its message. Emit a normal collection failure before xdist + # sees any runnable items. Nothing may contact the external instance. + config.hook.pytest_collectreport(report=pytest.CollectReport( + nodeid="tests/smoke", outcome="failed", longrepr=message, result=[], + )) + session.items.clear() diff --git a/tests/helpers/database.py b/tests/helpers/database.py new file mode 100644 index 000000000..57c19dd69 --- /dev/null +++ b/tests/helpers/database.py @@ -0,0 +1,51 @@ +"""Disposable databases for tests that exercise the real ORM and session manager.""" + +from contextlib import contextmanager +from tempfile import TemporaryDirectory + +import pytest +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from sqlalchemy.pool import NullPool + + +@contextmanager +def disposable_database(tmp_path): + """Own the database file and engine; keep the canonical ORM classes intact.""" + import core.database as database + + with TemporaryDirectory(prefix="database-", dir=tmp_path) as directory: + engine = create_engine( + f"sqlite:///{directory}/test.db", + connect_args={"check_same_thread": False}, + poolclass=NullPool, + ) + try: + database.Base.metadata.create_all(engine) + yield sessionmaker(bind=engine, autoflush=False, autocommit=False) + finally: + engine.dispose() + + +@contextmanager +def isolated_session_database(tmp_path): + """Temporarily bind the real manager and database aliases without reloading. + + Reloading core.database changes its ORM classes while existing imports keep + the old classes and factories. Patch only resource bindings instead, and + undo them before disposing the owned engine and removing its files. + """ + import core.database as database + import core.session_manager as manager + import src.database as compatibility_database + + with disposable_database(tmp_path) as factory: + engine = factory.kw["bind"] + with pytest.MonkeyPatch.context() as patcher: + patcher.setenv("DATABASE_URL", str(engine.url)) + for module in (database, compatibility_database): + patcher.setattr(module, "DATABASE_URL", str(engine.url)) + patcher.setattr(module, "engine", engine) + patcher.setattr(module, "SessionLocal", factory) + patcher.setattr(manager, "SessionLocal", factory) + yield manager.SessionManager(), database diff --git a/tests/helpers/worker_runtime.py b/tests/helpers/worker_runtime.py new file mode 100644 index 000000000..44a76bc4c --- /dev/null +++ b/tests/helpers/worker_runtime.py @@ -0,0 +1,70 @@ +"""Private filesystem defaults established before application imports.""" + +import os +import tempfile +from contextlib import contextmanager +from pathlib import Path + +import pytest + + +def bootstrap_runtime(): + """Establish defaults before collection; APP_PORT opts into live smoke.""" + worker = os.environ.get("PYTEST_XDIST_WORKER") + if os.environ.get("APP_PORT") and not worker: + return None + runtime = isolated_runtime(worker or "main") + runtime.root = runtime.__enter__() + return runtime + + +def configure_runtime(config, runtime): + """Register ownership even when configuration or collection fails.""" + if runtime is not None: + config.add_cleanup(lambda: runtime.__exit__(None, None, None)) + # pytest's default /pytest-of-/pytest- beneath the + # private TMPDIR, plus xdist's popen-gw, overflows the 107-byte + # AF_UNIX limit for sockets in tmp_path. Workers inherit a basetemp + # under the controller's; an explicit --basetemp still wins. + if config.option.basetemp is None and not hasattr(config, "workerinput"): + config.option.basetemp = str(runtime.root / "pytest") + parallel = bool(getattr(config.option, "numprocesses", None)) or hasattr(config, "workerinput") + # This consumes the existing test option; its public read and documented + # source location remain in the static-server fixture. + port_variable = "ODYSSEUS_TEST_STATIC_PORT" + if parallel and int(os.environ.get(port_variable) or 0): + raise pytest.UsageError( + "parallel tests require an ephemeral static-server port; " + "unset ODYSSEUS_TEST_STATIC_PORT or use -n 0" + ) + + +@contextmanager +def isolated_runtime(worker="main"): + """Own mutable defaults for one pytest process, including subprocesses. + + The random suffix separates simultaneous runs, even with the same worker + name. Test-specific monkeypatches and function-scoped databases still own + their resources; this is a fallback namespace, not a shared DB fixture. + """ + with tempfile.TemporaryDirectory(prefix=f"ody-{worker}-") as directory: + root = Path(directory) + with pytest.MonkeyPatch.context() as patcher: + paths = { + "ODYSSEUS_DATA_DIR": root / "data", + "ODYSSEUS_MAIL_ATTACHMENTS_DIR": root / "mail", + "FASTEMBED_CACHE_PATH": root / "fastembed", + "XDG_RUNTIME_DIR": root / "runtime", + } + for name, path in paths.items(): + path.mkdir(mode=0o700) + patcher.setenv(name, str(path)) + # Browser resolution falls back to our XDG runtime directory. + patcher.delenv("AGENT_BROWSER_SOCKET_DIR", raising=False) + tmp = root / "tmp" + tmp.mkdir(mode=0o700) + for name in ("TMPDIR", "TMP", "TEMP"): + patcher.setenv(name, str(tmp)) + # tempfile may already have cached the caller's directory. + patcher.setattr(tempfile, "tempdir", str(tmp)) + yield root diff --git a/tests/test_checkin_digest_owner_scope.py b/tests/test_checkin_digest_owner_scope.py index a2e8ebb17..68f1575d4 100644 --- a/tests/test_checkin_digest_owner_scope.py +++ b/tests/test_checkin_digest_owner_scope.py @@ -5,23 +5,23 @@ check-in for one user pulled EVERY user's calendar events (summaries, locations) into their digest — a cross-tenant leak. Ownership lives on CalendarCal.owner; the query must join it, like routes/calendar_routes. """ -import tempfile import uuid +import sys from datetime import datetime import pytest -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker -from sqlalchemy.pool import NullPool +from tests.helpers.database import disposable_database -import core.database as cdb from core.database import CalendarEvent, CalendarCal from src.task_scheduler import _checkin_calendar_events -_TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False) -_ENGINE = create_engine(f"sqlite:///{_TMPDB.name}", connect_args={"check_same_thread": False}, poolclass=NullPool) -cdb.Base.metadata.create_all(_ENGINE) -_TS = sessionmaker(bind=_ENGINE, autoflush=False, autocommit=False) + +@pytest.fixture(autouse=True) +def _digest_database(tmp_path): + with disposable_database(tmp_path) as factory: + with pytest.MonkeyPatch.context() as patcher: + patcher.setattr(sys.modules[__name__], "_TS", factory, raising=False) + yield def _seed(): diff --git a/tests/test_chroma_client.py b/tests/test_chroma_client.py index 0a57fee2a..bb8e34aec 100644 --- a/tests/test_chroma_client.py +++ b/tests/test_chroma_client.py @@ -12,19 +12,17 @@ import pytest import src.chroma_client as cc -def _free_port() -> int: - """Bind to port 0, grab the assigned port, release it — nothing listens.""" - s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - s.bind(("127.0.0.1", 0)) - port = s.getsockname()[1] - s.close() - return port +@pytest.fixture +def closed_port(): + """Reserve a port without listening, so another worker cannot take it.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as reserved: + reserved.bind(("127.0.0.1", 0)) + yield reserved.getsockname()[1] -def test_port_open_false_for_closed_port_and_is_fast(): - port = _free_port() +def test_port_open_false_for_closed_port_and_is_fast(closed_port): t0 = time.monotonic() - assert cc._port_open("127.0.0.1", port, timeout=1.0) is False + assert cc._port_open("127.0.0.1", closed_port, timeout=1.0) is False # The whole point: we fail fast, nowhere near the 30-60s OS timeout. assert time.monotonic() - t0 < 5.0 @@ -40,11 +38,11 @@ def test_port_open_true_for_listening_socket(): srv.close() -def test_get_chroma_client_does_not_cache_when_unreachable(monkeypatch): +def test_get_chroma_client_does_not_cache_when_unreachable(monkeypatch, closed_port): pytest.importorskip("chromadb") cc.reset_client() monkeypatch.setenv("CHROMADB_HOST", "127.0.0.1") - monkeypatch.setenv("CHROMADB_PORT", str(_free_port())) + monkeypatch.setenv("CHROMADB_PORT", str(closed_port)) with pytest.raises(RuntimeError): cc.get_chroma_client() # A failed connection must leave the singleton unset so a later call diff --git a/tests/test_database_test_isolation.py b/tests/test_database_test_isolation.py new file mode 100644 index 000000000..f0e72e27a --- /dev/null +++ b/tests/test_database_test_isolation.py @@ -0,0 +1,135 @@ +"""Guard database ownership at the helper and actual pytest lifecycle seams.""" + +import os +from pathlib import Path +import subprocess +import sys +import textwrap + +import pytest + +from tests.helpers.database import isolated_session_database + + +@pytest.mark.parametrize("fail_inside", [False, True]) +def test_session_database_restores_bindings_and_removes_files(tmp_path, fail_inside): + import core.database as database + import core.session_manager as manager + import src.database as compatibility_database + from core.models import ChatMessage + + modules = (database, compatibility_database, manager) + names = ("DATABASE_URL", "engine", "SessionLocal", "Base", "Session", "ChatMessage") + before = [{name: getattr(module, name) for name in names if hasattr(module, name)} + for module in modules] + previous_url = os.environ.get("DATABASE_URL") + saved_manager_class = manager.SessionManager + saved_db_session = manager.DbSession + saved_db_message = manager.DbChatMessage + listener = database.set_sqlite_pragma + + class IntentionalFailure(Exception): + pass + + try: + with isolated_session_database(tmp_path) as (sm, db_module): + owned_path = Path(db_module.engine.url.database) + assert owned_path.is_file() + assert owned_path.is_relative_to(tmp_path) + assert db_module.Session is saved_db_session + assert db_module.ChatMessage is saved_db_message + assert sm.__class__ is saved_manager_class + assert compatibility_database.SessionLocal is manager.SessionLocal + sm.create_session(session_id="owned", name="t", endpoint_url="x", + model="m", rag=False, owner="alice") + sm.add_message("owned", ChatMessage("user", "keep")) + sm.add_message("owned", ChatMessage("user", "remove")) + assert sm.truncate_messages("owned", 1) + with db_module.SessionLocal() as db: + assert db.query(saved_db_message).filter_by(session_id="owned").count() == 1 + assert db.query(saved_db_session).filter_by(id="owned").one().message_count == 1 + if fail_inside: + raise IntentionalFailure + except IntentionalFailure: + assert fail_inside + + assert os.environ.get("DATABASE_URL") == previous_url + for module, bindings in zip(modules, before): + for name, value in bindings.items(): + assert getattr(module, name) is value + assert database.set_sqlite_pragma is listener + assert manager.DbSession is saved_db_session + assert manager.DbChatMessage is saved_db_message + assert manager.SessionManager is saved_manager_class + assert not owned_path.parent.exists() + + with isolated_session_database(tmp_path) as (sm, db_module): + assert db_module.engine.url.database != str(owned_path) + with pytest.raises(KeyError, match="Session owned not found"): + sm.get_session("owned") + with db_module.SessionLocal() as db: + assert db.query(saved_db_session).count() == 0 + + +@pytest.mark.parametrize("truncation_first", [True, False]) +def test_actual_tests_restore_process_state_and_ignore_inherited_database(tmp_path, truncation_first): + # An inherited developer URL must never be opened, even during collection. + inherited_db = tmp_path / "developer.db" + sentinel = b"a developer database must not be opened or initialized" + inherited_db.write_bytes(sentinel) + inherited_url = f"sqlite:///{inherited_db}" + truncation = "tests/test_truncate_message_count_regression.py" + owner = "tests/test_manage_tasks_owner_scope.py::test_edit_allowed_for_matching_owner" + manifest = [truncation, owner] if truncation_first else [owner, truncation] + script = textwrap.dedent(''' + import os + import sys + import pytest + + def snapshot(): + import core + import src + import core.database as db + import core.session_manager as sm + import src.database as compat + return ( + os.environ.get("DATABASE_URL"), + sys.modules["core.database"], core.database, + sys.modules["core.session_manager"], core.session_manager, + sys.modules["src.database"], src.database, + db.DATABASE_URL, db.engine, db.SessionLocal, db.Base, + db.Session, db.ChatMessage, db.ScheduledTask, db.set_sqlite_pragma, + compat.DATABASE_URL, compat.engine, compat.SessionLocal, + compat.Session, compat.ChatMessage, + sm.SessionLocal, sm.DbSession, sm.DbChatMessage, sm.SessionManager, + ) + + class StateGuard: + def pytest_sessionstart(self): + self.initial = snapshot() + assert self.initial[0] == "sqlite:///:memory:" + + def pytest_collection_finish(self): + assert snapshot() == self.initial, "collection changed database bindings" + + @pytest.hookimpl(hookwrapper=True, tryfirst=True) + def pytest_runtest_teardown(self): + yield + assert snapshot() == self.initial, "test leaked database or module state" + + inherited_url = os.environ["DATABASE_URL"] + result = pytest.main(["-q", "-p", "no:cacheprovider", *sys.argv[1:]], + plugins=[StateGuard()]) + assert os.environ["DATABASE_URL"] == inherited_url + raise SystemExit(result) + ''') + result = subprocess.run( + [sys.executable, "-c", script, *manifest], + cwd=Path(__file__).resolve().parents[1], + env={**os.environ, "DATABASE_URL": inherited_url}, + capture_output=True, text=True, timeout=60, + ) + assert result.returncode == 0, result.stdout + result.stderr + assert "3 passed" in result.stdout + assert inherited_db.read_bytes() == sentinel + assert sorted(path.name for path in tmp_path.iterdir()) == ["developer.db"] diff --git a/tests/test_document_session_owner_scope.py b/tests/test_document_session_owner_scope.py index f776d9822..372092306 100644 --- a/tests/test_document_session_owner_scope.py +++ b/tests/test_document_session_owner_scope.py @@ -5,36 +5,31 @@ document route tests. This keeps coverage on the real closures without spinning up middleware. """ -import tempfile import uuid +import sys from types import SimpleNamespace from unittest.mock import MagicMock import pytest from fastapi import HTTPException -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker -from sqlalchemy.pool import NullPool - +from tests.helpers.database import disposable_database from tests.helpers.import_state import clear_fake_database_modules clear_fake_database_modules() -import core.database as cdb import routes.document_routes as droutes from core.database import Document from core.database import Session as DbSession from routes.document_helpers import DocumentPatch from routes.document_helpers import _owner_session_filter -_TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False) -_ENGINE = create_engine( - f"sqlite:///{_TMPDB.name}", - connect_args={"check_same_thread": False}, - poolclass=NullPool, -) -cdb.Base.metadata.create_all(_ENGINE) -_TS = sessionmaker(bind=_ENGINE, autoflush=False, autocommit=False) + +@pytest.fixture(autouse=True) +def _document_database(tmp_path): + with disposable_database(tmp_path) as factory: + with pytest.MonkeyPatch.context() as patcher: + patcher.setattr(sys.modules[__name__], "_TS", factory, raising=False) + yield def _req(user="alice"): diff --git a/tests/test_gallery_owner_filter_single_user.py b/tests/test_gallery_owner_filter_single_user.py index 7032410c6..215bc5911 100644 --- a/tests/test_gallery_owner_filter_single_user.py +++ b/tests/test_gallery_owner_filter_single_user.py @@ -4,22 +4,22 @@ When AUTH_ENABLED=false, get_current_user returns None and gallery routes should stay all-visible. When AUTH_ENABLED=true and no current user resolves, the same None means an anonymous caller and gallery queries must fail closed. """ -import tempfile import uuid +import sys import pytest -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker -from sqlalchemy.pool import NullPool +from tests.helpers.database import disposable_database -import core.database as cdb from core.database import GalleryImage from routes.gallery_helpers import _owner_filter -_TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False) -_ENGINE = create_engine(f"sqlite:///{_TMPDB.name}", connect_args={"check_same_thread": False}, poolclass=NullPool) -cdb.Base.metadata.create_all(_ENGINE) -_TS = sessionmaker(bind=_ENGINE, autoflush=False, autocommit=False) + +@pytest.fixture(autouse=True) +def _gallery_database(tmp_path): + with disposable_database(tmp_path) as factory: + with pytest.MonkeyPatch.context() as patcher: + patcher.setattr(sys.modules[__name__], "_TS", factory, raising=False) + yield def _seed(*owners): diff --git a/tests/test_manage_tasks_owner_scope.py b/tests/test_manage_tasks_owner_scope.py index 14797a2f4..2ef5e7410 100644 --- a/tests/test_manage_tasks_owner_scope.py +++ b/tests/test_manage_tasks_owner_scope.py @@ -12,14 +12,12 @@ permissive than the reader. """ import json -import tempfile +import sys from datetime import datetime import pytest -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker -from sqlalchemy.pool import NullPool +from tests.helpers.database import disposable_database from tests.helpers.import_state import clear_fake_database_modules clear_fake_database_modules() @@ -28,17 +26,16 @@ import core.database as cdb from core.database import ScheduledTask from src.tools.system import do_manage_tasks -_TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False) -_ENGINE = create_engine( - f"sqlite:///{_TMPDB.name}", - connect_args={"check_same_thread": False}, - poolclass=NullPool, -) -cdb.Base.metadata.create_all(_ENGINE) -_TS = sessionmaker(bind=_ENGINE, autoflush=False, autocommit=False) -# do_manage_tasks does `from core.database import SessionLocal` at call time, -# so patching the module attribute is enough to point it at the temp DB. -cdb.SessionLocal = _TS + +@pytest.fixture(autouse=True) +def _task_database(tmp_path): + # do_manage_tasks imports SessionLocal at call time. Own this binding for + # just one test, including helpers that seed and inspect its rows. + with disposable_database(tmp_path) as factory: + with pytest.MonkeyPatch.context() as patcher: + patcher.setattr(sys.modules[__name__], "_TS", factory, raising=False) + patcher.setattr(cdb, "SessionLocal", factory) + yield def _seed(task_id, owner, *, name=None): diff --git a/tests/test_process_ownership.py b/tests/test_process_ownership.py index 571ff1247..8727cdfc3 100644 --- a/tests/test_process_ownership.py +++ b/tests/test_process_ownership.py @@ -15,6 +15,7 @@ because an absent mechanism read as a successful answer. """ import os +import signal import subprocess import pytest @@ -35,11 +36,11 @@ def sleeper(): yield _spawn for proc in procs: - try: - proc.kill() - proc.wait(timeout=5) - except Exception: - pass + # Each child owns a session/process group. Keep the leader unreaped + # until its group is signalled, so the group id cannot be recycled. + if proc.returncode is None: + os.killpg(proc.pid, signal.SIGKILL) + proc.wait(timeout=5) # ── Verdicts, against real processes ──────────────────────────────────────── diff --git a/tests/test_research_report_read.py b/tests/test_research_report_read.py index 5559ee558..d1b586fb1 100644 --- a/tests/test_research_report_read.py +++ b/tests/test_research_report_read.py @@ -14,21 +14,20 @@ These tests pin both halves: web_fetching the HTML report. """ import json -from pathlib import Path import pytest from src.tool_implementations import do_manage_research from src.agent_loop import TOOL_SECTIONS -_DATA_DIR = Path("data/deep_research") - @pytest.fixture -def saved_report(): - _DATA_DIR.mkdir(parents=True, exist_ok=True) +def saved_report(tmp_path, monkeypatch): + from src.tools import research + + monkeypatch.setattr(research, "DEEP_RESEARCH_DIR", str(tmp_path)) rid = "rp-testreport1363" - path = _DATA_DIR / f"{rid}.json" + path = tmp_path / f"{rid}.json" path.write_text(json.dumps({ "query": "trending blender video ideas", "result": "## Findings\nShort-form Geometry Nodes tutorials are trending.", diff --git a/tests/test_runtime_behavior_regressions.py b/tests/test_runtime_behavior_regressions.py index 1b4d28b2d..73f9852c3 100644 --- a/tests/test_runtime_behavior_regressions.py +++ b/tests/test_runtime_behavior_regressions.py @@ -29,7 +29,8 @@ def _collect(gen): def _delta_chunk(text): - payload = {"choices": [{"delta": {"content": text}}]} + # stream_llm_with_fallback exposes normalized SSE, not provider wire JSON. + payload = {"delta": text} return f"data: {json.dumps(payload)}\n\n" @@ -57,7 +58,7 @@ def _run_turn(monkeypatch, messages, **kwargs): yield "data: [DONE]\n\n" monkeypatch.setattr(al, "stream_llm_with_fallback", _fake_stream, raising=False) - _collect( + chunks = _collect( al.stream_agent_loop( "http://local.test/v1", "moonshotai/kimi-k3", @@ -68,7 +69,7 @@ def _run_turn(monkeypatch, messages, **kwargs): **kwargs, ) ) - return offered + return offered, chunks @@ -100,60 +101,64 @@ def _contract(offered=("ask_user", "update_plan", "manage_notes"), # failure this guards is a model that obeys the wording while the runtime # contradicted it by offering the tool anyway. # -# SCOPE, and it matters: these cover the inferred path, where the turn has no -# explicit web toggle and the runtime decides from intent. Measured on -# lab@c499c01b, detection there is partial: "don't search online" suppresses -# the intent and the web tools are withheld; "Do not search the web" and "No -# web search please" do not, and the tools are offered. -# -# When the user explicitly enables web for the turn, wording does not withhold -# anything: confirmed end to end against a local Qwen3.5-9B Q4_K_M, where all -# three phrasings were offered web_search, web_fetch and private_browser. That -# may well be correct, an explicit toggle beating an inferred negative, so it -# is recorded here rather than asserted either way. -# -# The two inferred-path cases that do not hold are xfail(strict=True): they -# document the target, run on every suite, and fail the moment the behaviour -# lands. Delete the marker then. +# These cover inferred intent with no explicit web toggle or supplied contract. +# On the frozen Wave 3 base all three negatives are recognized. Two are still +# misclassified as web-dependent turns and short-circuit to "web disabled", +# preventing the requested answer from memory. Only that precise failure is +# expected below; unrelated exceptions must fail normally. HELD = ["Answer from memory only, don't search online."] -NOT_HELD_YET = [ +BLOCKED_MEMORY_ONLY = [ "Summarise what you already know. Do not search the web.", "No web search please, just tell me what you know about Python decorators.", ] +class _MemoryOnlyTurnBlocked(AssertionError): + """The real loop blocked a memory-only answer as requiring web access.""" + + +def _assert_negative_web_turn(monkeypatch, phrasing): + offered, chunks = _run_turn(monkeypatch, [{"role": "user", "content": phrasing}]) + events = [json.loads(chunk[6:]) for chunk in chunks + if chunk.startswith("data: ") and chunk.strip() != "data: [DONE]"] + finals = [event for event in events if event.get("type") == "final_response"] + if not offered and finals == [{ + "type": "final_response", + "content": "Web access is disabled for this turn. Enable web search and resend the request.", + }]: + raise _MemoryOnlyTurnBlocked( + f"Memory-only turn was blocked before any model call: {phrasing!r}; " + f"actual response: {finals[0]['content']}" + ) + + assert len(offered) == 1, f"Expected one memory-only model call; events: {events!r}" + names = _schema_names(offered[0]) + assert "web_search" not in names, f"web_search offered despite: {phrasing!r}" + assert "web_fetch" not in names, f"web_fetch offered despite: {phrasing!r}" + assert any(event.get("delta") == "ok" for event in events), events + assert chunks[-1] == "data: [DONE]\n\n" + + @pytest.mark.parametrize("phrasing", HELD) def test_negative_web_wording_withholds_the_web_tools(monkeypatch, phrasing): - offered = _run_turn(monkeypatch, [{"role": "user", "content": phrasing}]) - - names = _schema_names(offered[0]) - assert "web_search" not in names, f"web_search offered despite: {phrasing!r}" - assert "web_fetch" not in names, f"web_fetch offered despite: {phrasing!r}" + _assert_negative_web_turn(monkeypatch, phrasing) -@pytest.mark.xfail( - strict=True, - reason="negative web wording is only partially detected on lab@c499c01b; " - "these phrasings still get the web tools offered", -) -@pytest.mark.parametrize("phrasing", NOT_HELD_YET) -def test_negative_web_wording_withholds_the_web_tools_unhandled(monkeypatch, phrasing): - offered = _run_turn(monkeypatch, [{"role": "user", "content": phrasing}]) - - names = _schema_names(offered[0]) - assert "web_search" not in names, f"web_search offered despite: {phrasing!r}" - assert "web_fetch" not in names, f"web_fetch offered despite: {phrasing!r}" +@pytest.mark.parametrize("phrasing", BLOCKED_MEMORY_ONLY) +def test_negative_web_wording_withholds_the_web_tools_memory_only(monkeypatch, phrasing): + _assert_negative_web_turn(monkeypatch, phrasing) def test_plain_web_request_still_offers_search(monkeypatch): """The guard above must not become a blanket removal of the web tools.""" - offered = _run_turn( + offered, chunks = _run_turn( monkeypatch, [{"role": "user", "content": "Search the web for the latest Python release."}], ) + assert len(offered) == 1, chunks assert "web_search" in _schema_names(offered[0]) diff --git a/tests/test_static_test_server.py b/tests/test_static_test_server.py index f55546e9b..7e5bfaad4 100644 --- a/tests/test_static_test_server.py +++ b/tests/test_static_test_server.py @@ -7,7 +7,9 @@ running its own suite took the whole session down with it. import os import re +import socket import urllib.request +from urllib.parse import urlsplit from pathlib import Path @@ -31,8 +33,12 @@ def test_static_origin_does_not_reuse_the_application_port() -> None: def test_static_server_serves_this_worktree() -> None: origin = os.environ["ODYSSEUS_TEST_STATIC_ORIGIN"] + address = urlsplit(origin) - with urllib.request.urlopen(f"{origin}/static/js/documentStats.js", timeout=5) as r: + # Chromium may open a speculative connection and never send a request; + # that must not stall the requests queued behind it. + with socket.create_connection((address.hostname, address.port), timeout=5), \ + urllib.request.urlopen(f"{origin}/static/js/documentStats.js", timeout=5) as r: assert r.status == 200 assert r.headers.get_content_type() == "application/javascript" diff --git a/tests/test_stt_leak.py b/tests/test_stt_leak.py index ff752badd..356339113 100644 --- a/tests/test_stt_leak.py +++ b/tests/test_stt_leak.py @@ -3,7 +3,8 @@ import tempfile from services.stt.stt_service import STTService -def test_stt_local_transcribe_leak_on_error(): +def test_stt_local_transcribe_leak_on_error(tmp_path, monkeypatch): + monkeypatch.setattr(tempfile, "tempdir", str(tmp_path)) service = STTService() class MockWhisper: diff --git a/tests/test_truncate_message_count_regression.py b/tests/test_truncate_message_count_regression.py index 6f3d4ba0f..a632f8693 100644 --- a/tests/test_truncate_message_count_regression.py +++ b/tests/test_truncate_message_count_regression.py @@ -9,30 +9,21 @@ inconsistent with the actual rows. get_session relies on message_count>0 to decide whether to lazily hydrate from the DB, so an inflated count is a latent correctness hazard. """ -import os -import tempfile +import pytest + +from tests.helpers.database import isolated_session_database -def _make_manager(): - db_fd, db_path = tempfile.mkstemp(suffix=".db") - os.close(db_fd) - os.environ["DATABASE_URL"] = f"sqlite:///{db_path}" - - # Import after DATABASE_URL is set so the engine binds to the temp DB. - import importlib - import core.database as database - importlib.reload(database) - database.Base.metadata.create_all(bind=database.engine) - - import core.session_manager as sm_mod - importlib.reload(sm_mod) - return sm_mod.SessionManager(), database, sm_mod +@pytest.fixture +def manager_database(tmp_path): + with isolated_session_database(tmp_path) as resources: + yield resources -def test_truncate_keep_count_exceeds_total_does_not_inflate_count(): +def test_truncate_keep_count_exceeds_total_does_not_inflate_count(manager_database): from core.models import ChatMessage - sm, database, sm_mod = _make_manager() + sm, database = manager_database sid = "short-session" sm.create_session(session_id=sid, name="t", endpoint_url="x", model="m", rag=False, owner="u") @@ -59,10 +50,10 @@ def test_truncate_keep_count_exceeds_total_does_not_inflate_count(): db.close() -def test_truncate_keeps_history_alias_for_context_messages(): +def test_truncate_keeps_history_alias_for_context_messages(manager_database): from core.models import ChatMessage - sm, database, sm_mod = _make_manager() + sm, database = manager_database sid = "alias-after-truncate" sm.create_session(session_id=sid, name="t", endpoint_url="x", model="m", rag=False, owner="u") diff --git a/tests/test_worker_runtime.py b/tests/test_worker_runtime.py new file mode 100644 index 000000000..a91157902 --- /dev/null +++ b/tests/test_worker_runtime.py @@ -0,0 +1,81 @@ +"""The default namespace must protect callers and simultaneous pytest runs.""" + +import os +import socket +import subprocess +import sys +import tempfile +from pathlib import Path + +import pytest + +from tests.helpers.worker_runtime import isolated_runtime + + +@pytest.mark.parametrize("fail", [False, True]) +def test_runtime_restores_environment_and_removes_files(monkeypatch, tmp_path, fail): + caller = tmp_path / "caller" + caller.mkdir() + sentinel = caller / "sentinel" + sentinel.write_text("keep") + for name in ("ODYSSEUS_DATA_DIR", "ODYSSEUS_MAIL_ATTACHMENTS_DIR", + "FASTEMBED_CACHE_PATH", "XDG_RUNTIME_DIR", "AGENT_BROWSER_SOCKET_DIR", + "TMPDIR", "TMP", "TEMP"): + monkeypatch.setenv(name, str(caller)) + before = dict(os.environ) + previous_tmp = tempfile.tempdir + root = None + try: + with isolated_runtime("gw0") as root: + assert root != caller + assert "AGENT_BROWSER_SOCKET_DIR" not in os.environ + for name in ("ODYSSEUS_DATA_DIR", "ODYSSEUS_MAIL_ATTACHMENTS_DIR", + "FASTEMBED_CACHE_PATH", "XDG_RUNTIME_DIR", "TMPDIR", "TMP", "TEMP"): + path = Path(os.environ[name]) + assert path.is_dir() and path.is_relative_to(root) + (path / "owned").write_text("test") + assert Path(tempfile.gettempdir()).is_relative_to(root) + if fail: + raise RuntimeError("test failure") + except RuntimeError: + assert fail + assert dict(os.environ) == before + assert tempfile.tempdir == previous_tmp + assert root is not None and not root.exists() + assert sentinel.read_text() == "keep" + assert list(caller.iterdir()) == [sentinel] + + +def test_same_worker_name_gets_distinct_namespaces(): + data_variable = "ODYSSEUS_DATA_DIR" + with isolated_runtime("gw0") as first: + (first / "data" / "state").write_text("first") + with isolated_runtime("gw0") as second: + assert first != second + assert not (second / "data" / "state").exists() + assert Path(os.environ[data_variable]) == second / "data" + assert Path(os.environ[data_variable]) == first / "data" + assert (first / "data" / "state").read_text() == "first" + assert not first.exists() and not second.exists() + + +def test_subprocess_inherits_private_temp_and_data_directories(): + with isolated_runtime("gw1") as root: + result = subprocess.run( + [sys.executable, "-c", "import os,tempfile; from pathlib import Path; " + "name = 'ODYSSEUS_DATA_DIR'; Path(os.environ[name], 'child').write_text('data'); " + "Path(tempfile.gettempdir(), 'child').write_text('temp')"], + capture_output=True, text=True, timeout=10, + ) + assert result.returncode == 0, result.stderr + assert (root / "data" / "child").read_text() == "data" + assert (root / "tmp" / "child").read_text() == "temp" + assert not root.exists() + + +@pytest.mark.skipif(not hasattr(socket, "AF_UNIX"), reason="requires AF_UNIX") +def test_tmp_path_under_private_runtime_fits_unix_socket(tmp_path): + # pytest truncates this name to 30 characters, as for the real-tmux + # witness. A nested pytest-of- basetemp made it 110 bytes under xdist. + with socket.socket(socket.AF_UNIX) as sock: + sock.bind(str(tmp_path / "tmux.sock")) diff --git a/website/configuration-reference.md b/website/configuration-reference.md index 40d6309d2..bbca97a61 100644 --- a/website/configuration-reference.md +++ b/website/configuration-reference.md @@ -72,8 +72,8 @@ The source tree reads **112** `ODYSSEUS_*` variables: 81 an operator may want to | Variable | Default | Read in | What it does | |---|---|---|---| | `ODYSSEUS_DISABLE_MCP` | `''` | `src/builtin_mcp.py:89` | Truthy disables MCP entirely, as an escape hatch for compatibility problems with a server. | -| `ODYSSEUS_MAX_VISUAL_EVIDENCE_FRAMES` | `'3'` | `src/agent_loop.py:15361` | How many video frames one tool result may contribute. Clamped to 1-8. | -| `ODYSSEUS_MAX_VISUAL_EVIDENCE_IMAGES` | `'1'` | `src/agent_loop.py:15329` | How many images one tool result may contribute to the model turn. Clamped to 1-8. | +| `ODYSSEUS_MAX_VISUAL_EVIDENCE_FRAMES` | `'3'` | `src/agent_loop.py:15366` | How many video frames one tool result may contribute. Clamped to 1-8. | +| `ODYSSEUS_MAX_VISUAL_EVIDENCE_IMAGES` | `'1'` | `src/agent_loop.py:15334` | How many images one tool result may contribute to the model turn. Clamped to 1-8. | | `ODYSSEUS_MCP_ALLOWED_COMMANDS` | `''` | `src/agent_tools/admin_tools.py:140` | Security-relevant. Comma-separated allowlist of MCP launcher basenames the agent may start. Empty by default, and the deny list still wins. | | `ODYSSEUS_PYTHON_TOOL_SITE_PACKAGES` | `''` | `src/agent_runtime/process_resources.py:59` (+2 more) | Security-relevant. Absolute package roots, separated by the platform path separator, exposed to the sandboxed Python tool. Empty exposes none. | | `ODYSSEUS_SCRIPT_HOST` | `'localhost'` | `src/builtin_actions.py:925` | Default host for the run-script action. `localhost`, `127.0.0.1`, `local` and empty run locally; any other value runs over SSH. |