From 941c960650de67ecc7844fe447f1e874874981e4 Mon Sep 17 00:00:00 2001 From: Ahmed Allam Date: Wed, 2 Sep 2026 12:05:33 +0000 Subject: [PATCH] fix(ci): make the pre-commit mypy hook and the test suite pass on a fresh checkout --- .pre-commit-config.yaml | 29 ++++++------ pyproject.toml | 5 +- strix/config/codex.py | 2 +- strix/core/runner.py | 1 + strix/interface/cloud/__init__.py | 2 +- strix/report/state.py | 2 +- tests/conftest.py | 27 +++++++++++ tests/test_cloud_cli.py | 73 ++++++++++++++++-------------- tests/test_cloud_cli_runtime.py | 55 +++++++++++----------- tests/test_cloud_idempotency.py | 6 ++- tests/test_cloud_payment_proxy.py | 10 ++-- tests/test_cloud_session.py | 5 +- tests/test_docker_client_delete.py | 18 +++++--- tests/test_optional_deps.py | 3 +- tests/test_runner_mcp.py | 4 +- tests/test_sarif_stride.py | 3 +- tests/test_session_fd.py | 45 ++++++++++++++---- 17 files changed, 185 insertions(+), 105 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7ceb3cc4..1de107b3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,3 +1,6 @@ +# Built viewer bundles are generated output, not hand-edited source. +exclude: ^strix/interface/viewer/static/assets/ + repos: # Ruff for fast linting and formatting - repo: https://github.com/astral-sh/ruff-pre-commit @@ -9,21 +12,18 @@ repos: - id: ruff-format name: ruff-format - # MyPy for static type checking - - repo: https://github.com/pre-commit/mirrors-mypy - rev: v1.17.1 + # MyPy for static type checking. Runs the project's own mypy from the uv + # environment (`make dev-install`) so it sees the same dependencies and + # stubs as `make check-all`. + - repo: local hooks: - id: mypy - additional_dependencies: [ - types-requests, - types-python-dateutil, - pydantic, - fastapi, - pytest, - hatchling, - "openai-agents[litellm]>=0.19.0,<0.20", - ] - args: [--install-types, --non-interactive] + name: mypy + entry: uv run mypy + language: system + types_or: [python, pyi] + files: ^(strix|tests)/ + require_serial: true # Built-in hooks for basic file checks - repo: https://github.com/pre-commit/pre-commit-hooks @@ -62,5 +62,6 @@ ci: autoupdate_branch: "" autoupdate_commit_msg: "[pre-commit.ci] pre-commit autoupdate" autoupdate_schedule: weekly - skip: [] + # pre-commit.ci cannot run `language: system` hooks; mypy runs via `make check-all`. + skip: [mypy] submodules: false diff --git a/pyproject.toml b/pyproject.toml index 7ef8848b..8382f986 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -250,6 +250,7 @@ ignore = [ # Stdlib HTTP handler overrides (do_GET/do_POST) and lazy imports that avoid a # circular dependency with strix.telemetry / strix.interface.viewer.report_pdf. "strix/interface/viewer/server.py" = ["N802", "PLC0415"] +"strix/interface/cloud/payment_proxy.py" = ["N802"] # Lazy telemetry import to avoid importing PostHog before the viewer starts. "strix/interface/viewer/cli.py" = ["PLC0415"] # Lazy imports inside functions to avoid circular dependency with @@ -413,6 +414,8 @@ known_third_party = ["pydantic", "litellm"] # ============================================================================ [tool.bandit] -exclude_dirs = ["docs", "build", "dist"] +# Tests are covered by ruff's flake8-bandit rules (see per-file-ignores above), +# which is where fixture tokens and loopback URL opens are already waived. +exclude_dirs = ["docs", "build", "dist", "tests"] skips = ["B101", "B601", "B404", "B603", "B607"] # Skip assert, shell injection, subprocess import and partial path checks severity = "medium" diff --git a/strix/config/codex.py b/strix/config/codex.py index cf34f003..9f81ff6d 100644 --- a/strix/config/codex.py +++ b/strix/config/codex.py @@ -183,7 +183,7 @@ def build_authorize_url(challenge: str, state: str) -> str: "code_challenge": challenge, "code_challenge_method": "S256", "state": state, - "id_token_add_organizations": "true", + "id_token_add_organizations": "true", # nosec B105 - boolean flag, not a secret "codex_cli_simplified_flow": "true", "originator": ORIGINATOR, } diff --git a/strix/core/runner.py b/strix/core/runner.py index 576715d7..71c08742 100644 --- a/strix/core/runner.py +++ b/strix/core/runner.py @@ -428,6 +428,7 @@ async def run_strix_scan( } for summary in mcp_registry.summaries() ] + # Feed a non-secret connection roster (name / provider / # tool_count / dead) to two consumers: once now (all # currently healthy) and again whenever a connection later diff --git a/strix/interface/cloud/__init__.py b/strix/interface/cloud/__init__.py index 37250e9e..a906b439 100644 --- a/strix/interface/cloud/__init__.py +++ b/strix/interface/cloud/__init__.py @@ -14,7 +14,7 @@ import sys from rich.console import Console from rich.markup import escape -import strix.interface.cloud.http as http # noqa: PLR0402 +from strix.interface.cloud import http from strix.interface.cloud.render import json_mode from strix.interface.cloud.runner import resolve, run from strix.interface.cloud.session import run_session diff --git a/strix/report/state.py b/strix/report/state.py index 3c5465f2..6a082371 100644 --- a/strix/report/state.py +++ b/strix/report/state.py @@ -167,7 +167,7 @@ def get_global_report_state() -> Optional["ReportState"]: return _global_report_state -def set_global_report_state(report_state: "ReportState") -> None: +def set_global_report_state(report_state: Optional["ReportState"]) -> None: global _global_report_state # noqa: PLW0603 _global_report_state = report_state # New run: drop any streamed-cost entries a prior run left unconsumed. diff --git a/tests/conftest.py b/tests/conftest.py index a840a37b..946ab206 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -24,3 +24,30 @@ def _isolate_mcp_config( monkeypatch.setenv("STRIX_MCP_CONFIG", str(missing)) monkeypatch.delenv("STRIX_MCP_ONLY", raising=False) monkeypatch.delenv("STRIX_MCP_EXCLUDE", raising=False) + + +@pytest.fixture(autouse=True) +def _plain_terminal(monkeypatch: pytest.MonkeyPatch) -> None: + """Make Rich output identical on every developer's machine. + + Many CLI tests force ``isatty()`` to ``True`` to exercise the human-readable + code path and then assert on the plain text. Rich picks its color system + from ``TERM``, ``COLORTERM``, and ``FORCE_COLOR``, so on a real terminal + those assertions would meet ANSI escape codes instead of the words they + look for. A dumb terminal renders the same text without any styling. + """ + monkeypatch.setenv("TERM", "dumb") + for name in ("COLORTERM", "FORCE_COLOR", "NO_COLOR", "TTY_COMPATIBLE"): + monkeypatch.delenv(name, raising=False) + + +@pytest.fixture(autouse=True) +def _isolate_wallet_config(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep a developer's real mppx wallet out of the top-up tests. + + ``strix cloud billing topup`` chooses the Stripe Link flow or the + preconfigured mppx wallet from these variables, so leaving them set would + silently switch which branch a test runs. + """ + for name in ("MPPX_ACCOUNT", "MPPX_STRIPE_SECRET_KEY", "MPPX_STRIPE_PAYMENT_METHOD"): + monkeypatch.delenv(name, raising=False) diff --git a/tests/test_cloud_cli.py b/tests/test_cloud_cli.py index f2c23435..363f4564 100644 --- a/tests/test_cloud_cli.py +++ b/tests/test_cloud_cli.py @@ -6,6 +6,7 @@ import io import json import shutil import subprocess +import sys import urllib.request import webbrowser from pathlib import Path @@ -16,7 +17,7 @@ import requests from rich.console import Console from strix.interface import cloud, platform_cli -from strix.interface.cloud import billing, http, payment_proxy, render, runner, workspaces +from strix.interface.cloud import http, render, runner, workspaces from strix.interface.cloud.spec import GROUP_HELP, SPEC @@ -147,7 +148,7 @@ def test_read_groups_have_safe_defaults(group: str, verb: str) -> None: resolved = runner.resolve(group, []) assert resolved is not None command, remaining = resolved - assert command is runner.SPEC[group][verb] + assert command is SPEC[group][verb] assert remaining == [] @@ -561,7 +562,7 @@ def test_stored_token_is_never_sent_to_a_different_platform_origin( lambda: {"api_token": "stored-secret", "app_url": "https://app.strix.ai"}, ) monkeypatch.setattr( - http.requests, + requests, "request", lambda *_args, **_kwargs: pytest.fail("a mismatched origin must not receive the token"), ) @@ -576,7 +577,7 @@ def test_stored_token_requires_an_issuer_binding(monkeypatch: pytest.MonkeyPatch monkeypatch.setattr(http, "_app_url_override", "https://app.strix.ai") monkeypatch.setattr(http, "read_record", lambda: {"api_token": "legacy-secret"}) monkeypatch.setattr( - http.requests, + requests, "request", lambda *_args, **_kwargs: pytest.fail("an unbound token must not be sent"), ) @@ -602,7 +603,7 @@ def test_stored_token_is_sent_only_to_its_bound_platform( seen.update(url=url, headers=kwargs["headers"]) return FakeResponse(payload={"balance": 1}) - monkeypatch.setattr(http.requests, "request", request) + monkeypatch.setattr(requests, "request", request) response = http.request("GET", "/billing/credits") assert response.status_code == 200 @@ -626,7 +627,7 @@ def test_explicit_token_can_target_an_explicit_platform( seen.update(url=url, headers=kwargs["headers"]) return FakeResponse(payload={"balance": 1}) - monkeypatch.setattr(http.requests, "request", request) + monkeypatch.setattr(requests, "request", request) override_value = "explicit-preview-" + str(1) response = http.request("GET", "/billing/credits", token=override_value) @@ -706,9 +707,9 @@ def test_topup_noninteractive_requires_explicit_payment_approval( monkeypatch.setattr( http, "request", lambda *_a, **_k: FakeResponse(status_code=402, payload=challenge) ) - monkeypatch.setattr(runner.sys.stdin, "isatty", lambda: False) + monkeypatch.setattr(sys.stdin, "isatty", lambda: False) monkeypatch.setattr( - billing.subprocess, + subprocess, "run", lambda *_a, **_k: pytest.fail("wallet must not run without --yes"), ) @@ -732,10 +733,10 @@ def test_topup_machine_output_never_prompts_even_with_terminal_stdin( monkeypatch.setattr( http, "request", lambda *_a, **_k: FakeResponse(status_code=402, payload=challenge) ) - monkeypatch.setattr(runner.sys.stdin, "isatty", lambda: True) - monkeypatch.setattr(runner.sys.stdout, "isatty", lambda: stdout_tty) + monkeypatch.setattr(sys.stdin, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: stdout_tty) monkeypatch.setattr( - runner.Console, + Console, "input", lambda *_a, **_k: pytest.fail("machine-readable top-up must not prompt"), ) @@ -839,7 +840,7 @@ def test_topup_keeps_token_out_of_wallet_process_and_forwards_payment( upstream.update(method=method, url=url, **kwargs) return FakeResponse(payload=receipt, content=json.dumps(receipt).encode()) - monkeypatch.setattr(payment_proxy.requests, "request", fake_upstream_request) + monkeypatch.setattr(requests, "request", fake_upstream_request) def fake_run(command: list[str], **kwargs: Any) -> Any: commands.append(command) @@ -905,6 +906,7 @@ def test_topup_wallet_failure_is_one_redacted_json_object( http, "request", lambda *_a, **_k: FakeResponse(status_code=402, payload=challenge) ) monkeypatch.setattr(http, "api_token", lambda *_a, **_k: "tok") + monkeypatch.setenv("MPPX_ACCOUNT", "agent") monkeypatch.setattr(shutil, "which", lambda _name: "/usr/bin/npx") monkeypatch.setattr( subprocess, @@ -946,6 +948,7 @@ def test_topup_wallet_interruption_reports_unknown_payment_outcome( ), ) monkeypatch.setattr(http, "api_token", lambda *_a, **_k: "tok") + monkeypatch.setenv("MPPX_ACCOUNT", "agent") monkeypatch.setattr(shutil, "which", lambda _name: "/usr/bin/npx") monkeypatch.setattr( subprocess, "run", lambda *_a, **_k: (_ for _ in ()).throw(KeyboardInterrupt) @@ -971,6 +974,7 @@ def test_topup_non_json_wallet_success_requires_balance_verification( ), ) monkeypatch.setattr(http, "api_token", lambda *_a, **_k: "tok") + monkeypatch.setenv("MPPX_ACCOUNT", "agent") monkeypatch.setattr(shutil, "which", lambda _name: "/usr/bin/npx") monkeypatch.setattr( subprocess, @@ -997,6 +1001,7 @@ def test_topup_rejects_parseable_wallet_error_as_a_success( ), ) monkeypatch.setattr(http, "api_token", lambda *_a, **_k: "tok") + monkeypatch.setenv("MPPX_ACCOUNT", "agent") monkeypatch.setattr(shutil, "which", lambda _name: "/usr/bin/npx") monkeypatch.setattr( subprocess, @@ -1036,6 +1041,7 @@ def test_topup_does_not_trust_an_unobserved_wallet_receipt( ), ) monkeypatch.setattr(http, "api_token", lambda *_a, **_k: "tok") + monkeypatch.setenv("MPPX_ACCOUNT", "agent") monkeypatch.setattr(shutil, "which", lambda _name: "/usr/bin/npx") monkeypatch.setattr( subprocess, @@ -1056,7 +1062,7 @@ def test_topup_does_not_trust_an_unobserved_wallet_receipt( def test_topup_human_mode_requires_a_bridge_confirmed_receipt( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -1066,9 +1072,10 @@ def test_topup_human_mode_requires_a_bridge_confirmed_receipt( ), ) monkeypatch.setattr(http, "api_token", lambda *_a, **_k: "tok") + monkeypatch.setenv("MPPX_ACCOUNT", "agent") monkeypatch.setattr(shutil, "which", lambda _name: "/usr/bin/npx") monkeypatch.setattr( - payment_proxy.requests, + requests, "request", lambda *_a, **_k: FakeResponse(status_code=200, content=b"not a receipt"), ) @@ -1597,7 +1604,7 @@ def test_handoff_links_reject_non_http_schemes( monkeypatch: pytest.MonkeyPatch, capsys: Any, ) -> None: - monkeypatch.setattr(runner.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -1882,7 +1889,7 @@ def test_workspace_use_preserves_definitive_conflict( def test_group_help_lists_all_verbs_instead_of_default_verb_help( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) assert cloud.run_cloud(["workspaces", "-h"]) == 0 output = capsys.readouterr().out assert "workspaces verbs" in output @@ -1906,7 +1913,7 @@ def test_workspace_alias_routes_to_workspaces(monkeypatch: pytest.MonkeyPatch) - def test_workspace_human_list_is_numbered_and_hides_ids( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -1954,7 +1961,7 @@ def test_integrations_human_list_exposes_installation_id_and_json_stays_full( ], "bitbucket_oauth_enabled": True, } - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr(http, "request", lambda *_a, **_k: FakeResponse(payload=payload)) assert cloud.run_cloud(["integrations", "list"]) == 0 @@ -1972,7 +1979,7 @@ def test_integrations_human_list_exposes_installation_id_and_json_stays_full( def test_pr_review_human_list_prioritizes_actionable_fields( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -2047,7 +2054,7 @@ def test_pr_review_human_list_shows_pull_request_state( capsys: Any, pr_state: str, ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -2110,7 +2117,7 @@ def test_scan_human_list_identifies_internal_and_uploaded_targets( record: dict[str, Any], expected_targets: tuple[str, ...], ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -2274,7 +2281,7 @@ def test_human_lists_prioritize_actionable_fields( visible: tuple[str, ...], hidden: tuple[str, ...], ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr(http, "request", lambda *_a, **_k: FakeResponse(payload=payload)) assert cloud.run_cloud(command) == 0 @@ -2288,7 +2295,7 @@ def test_human_lists_prioritize_actionable_fields( def test_token_human_list_shows_lifecycle_status( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -2645,7 +2652,7 @@ def test_nonstandard_human_list_envelopes_are_actionable( visible: tuple[str, ...], hidden: tuple[str, ...], ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr(http, "request", lambda *_a, **_k: FakeResponse(payload=payload)) assert cloud.run_cloud(command) == 0 @@ -2695,7 +2702,7 @@ def test_chat_credentials_human_view_separates_attached_and_available_sources( } ], } - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr(http, "request", lambda *_a, **_k: FakeResponse(payload=payload)) command = ["chat", "credentials", "chat-id", "--scan-ids", "source-scan-id"] @@ -2921,7 +2928,7 @@ def test_named_human_list_views_match_api_fields( visible: tuple[str, ...], hidden: tuple[str, ...], ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr(http, "request", lambda *_a, **_k: FakeResponse(payload=payload)) assert cloud.run_cloud(command) == 0 @@ -2935,7 +2942,7 @@ def test_named_human_list_views_match_api_fields( def test_supply_chain_org_summary_human_view_shows_totals_and_repository_risk( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -3096,7 +3103,7 @@ def test_wrapped_detail_human_views_are_unwrapped_and_actionable( visible: tuple[str, ...], hidden: tuple[str, ...], ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr(http, "request", lambda *_a, **_k: FakeResponse(payload=payload)) assert cloud.run_cloud(command) == 0 @@ -3110,7 +3117,7 @@ def test_wrapped_detail_human_views_are_unwrapped_and_actionable( def test_trace_human_view_summarizes_events_and_preserves_selector( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) seen_query: dict[str, Any] = {} def fake_trace_request(_method: str, _path: str, **kwargs: Any) -> FakeResponse: @@ -3206,7 +3213,7 @@ def test_trace_human_view_summarizes_events_and_preserves_selector( def test_paginated_human_list_shows_total_and_continuation_command( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -3308,7 +3315,7 @@ def test_page_pagination_explains_an_out_of_range_page() -> None: def test_human_detail_preserves_long_prose_beyond_table_cell_limit( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) description = ( " ".join(["authorization context"] * 12) + " final-description-marker\nsecond-line-marker" ) @@ -3386,7 +3393,7 @@ def test_large_vulnerability_detail_prioritizes_evidence_and_remediation() -> No def test_test_user_human_view_joins_latest_verification( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -3483,7 +3490,7 @@ def test_wide_knowledge_table_keeps_title_readable_with_long_identifiers() -> No def test_human_get_prioritizes_details_and_hides_internal_identity_fields( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", diff --git a/tests/test_cloud_cli_runtime.py b/tests/test_cloud_cli_runtime.py index 2d6ccc93..4e2a5e5c 100644 --- a/tests/test_cloud_cli_runtime.py +++ b/tests/test_cloud_cli_runtime.py @@ -6,13 +6,16 @@ import argparse import io import json import sys +import time from typing import TYPE_CHECKING, Any import pytest +import requests from rich.console import Console from strix.interface import cloud, platform_cli from strix.interface.cloud import http, render, runner, source_scan +from strix.interface.cloud.source_upload import prepare_source from strix.interface.main import main as interface_main @@ -169,7 +172,7 @@ def test_connector_enrollment_command_is_complete_multiline_and_terminal_safe( " -e LABEL=before\x1b]52;c;copied\x07after \\\n" " ghcr.io/usestrix/connector:latest" ) - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -293,8 +296,8 @@ def test_source_prompt_shows_paths_and_literal_confirmation( return "n" monkeypatch.setattr(console, "input", answer) - monkeypatch.setattr(source_scan.sys.stdin, "isatty", lambda: True) - monkeypatch.setattr(source_scan.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdin, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) args = argparse.Namespace( source=str(tmp_path), dry_run=False, @@ -324,7 +327,7 @@ def test_source_prompt_interruption_removes_temporary_archive( (tmp_path / "app.py").write_text("print('ok')\n", encoding="utf-8") console = Console(file=io.StringIO(), width=100) archive_paths: list[Path] = [] - original_prepare = source_scan.prepare_source + original_prepare = prepare_source def capture_bundle(*args: Any, **kwargs: Any) -> Any: bundle = original_prepare(*args, **kwargs) @@ -336,8 +339,8 @@ def test_source_prompt_interruption_removes_temporary_archive( monkeypatch.setattr(source_scan, "prepare_source", capture_bundle) monkeypatch.setattr(console, "input", interrupt) - monkeypatch.setattr(source_scan.sys.stdin, "isatty", lambda: True) - monkeypatch.setattr(source_scan.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdin, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) args = argparse.Namespace( source=str(tmp_path), dry_run=False, @@ -394,7 +397,7 @@ def test_device_login_rejects_non_http_verification_url( monkeypatch: pytest.MonkeyPatch, ) -> None: monkeypatch.setattr( - platform_cli.requests, + requests, "post", lambda *_a, **_k: FakeResponse( { @@ -427,7 +430,7 @@ def test_boolean_query_values_are_lowercase_for_url_search_params( seen["params"] = kwargs.get("params") return FakeResponse({"items": []}) - monkeypatch.setattr(http.requests, "request", fake_request) + monkeypatch.setattr(requests, "request", fake_request) http.request("GET", "/test", query={"enabled": True, "disabled": False}) assert seen["params"] == {"enabled": "true", "disabled": "false"} @@ -481,7 +484,7 @@ def test_binary_response_refuses_to_write_to_a_terminal( monkeypatch: pytest.MonkeyPatch, capsys: Any, ) -> None: - monkeypatch.setattr(runner.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", @@ -513,7 +516,7 @@ def test_binary_response_can_be_intentionally_redirected( return None redirected = RedirectedStdout() - monkeypatch.setattr(runner.sys, "stdout", redirected) + monkeypatch.setattr(sys, "stdout", redirected) monkeypatch.setattr( http, "request", @@ -534,7 +537,7 @@ def test_redirected_binary_errors_never_append_diagnostics_to_stdout( def iter_content(self, *, chunk_size: int) -> Any: assert chunk_size == 1024 * 1024 yield b"%PDF-partial" - raise http.requests.ConnectionError("connection lost") + raise requests.ConnectionError("connection lost") response = ( FakeResponse({"detail": "report rejected"}, status_code=500) @@ -645,7 +648,7 @@ def test_binary_download_streams_and_preserves_existing_file_on_failure( def iter_content(self, *, chunk_size: int) -> Any: assert chunk_size == 1024 * 1024 yield b"partial" - raise http.requests.ConnectionError("connection lost") + raise requests.ConnectionError("connection lost") def close(self) -> None: self.closed = True @@ -770,7 +773,7 @@ def test_session_help_is_specific_and_human_whoami_shows_scopes( "scopes": ["scans:read", "organizations:read"], } ) - monkeypatch.setattr(platform_cli.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) assert cloud.run_cloud(["whoami", "--help"]) == 0 who_help = capsys.readouterr().out assert "strix cloud whoami" in who_help @@ -784,7 +787,7 @@ def test_non_tty_whoami_and_logout_emit_json( ) -> None: monkeypatch.delenv("STRIX_API_TOKEN", raising=False) monkeypatch.setattr(platform_cli, "AUTH_PATH", tmp_path / "auth.json") - monkeypatch.setattr(platform_cli.sys.stdout, "isatty", lambda: False) + monkeypatch.setattr(sys.stdout, "isatty", lambda: False) platform_cli.save_record( { "api_token": "secret", @@ -799,7 +802,7 @@ def test_non_tty_whoami_and_logout_emit_json( assert json.loads(capsys.readouterr().out)["email"] == "agent@example.test" monkeypatch.setattr( - platform_cli.requests, + requests, "delete", lambda *_args, **_kwargs: type("Response", (), {"status_code": 200})(), ) @@ -831,7 +834,7 @@ def test_scope_picker_labels_match_the_server_presets() -> None: def test_noninteractive_login_never_prompts_for_workspace( monkeypatch: pytest.MonkeyPatch, ) -> None: - monkeypatch.setattr(platform_cli.sys.stdin, "isatty", lambda: False) + monkeypatch.setattr(sys.stdin, "isatty", lambda: False) console = Console(file=io.StringIO()) console.input = lambda *_args, **_kwargs: pytest.fail("must not prompt") # type: ignore[method-assign] @@ -896,10 +899,10 @@ def test_device_flow_slow_down_never_exceeds_the_poll_interval_cap( sleeps.append(seconds) now += seconds - monkeypatch.setattr(platform_cli.requests, "post", post) + monkeypatch.setattr(requests, "post", post) monkeypatch.setattr(platform_cli, "_app_url", lambda: "https://example.test") - monkeypatch.setattr(platform_cli.time, "monotonic", monotonic) - monkeypatch.setattr(platform_cli.time, "sleep", sleep) + monkeypatch.setattr(time, "monotonic", monotonic) + monkeypatch.setattr(time, "sleep", sleep) with pytest.raises(platform_cli.PlatformAuthError, match="expired"): platform_cli._run_device_flow( @@ -933,9 +936,9 @@ def test_device_flow_accepts_external_authkit_url_and_binds_token_origin( ), ] ) - monkeypatch.setattr(platform_cli.requests, "post", lambda *_a, **_k: next(responses)) + monkeypatch.setattr(requests, "post", lambda *_a, **_k: next(responses)) monkeypatch.setattr(platform_cli, "_app_url", lambda: "https://preview.strix.ai") - monkeypatch.setattr(platform_cli.time, "sleep", lambda _seconds: None) + monkeypatch.setattr(time, "sleep", lambda _seconds: None) record = platform_cli._run_device_flow( Console(file=io.StringIO()), @@ -984,7 +987,7 @@ def test_root_help_accepts_json_before_help_and_leaf_help_stays_specific( def test_non_tty_dispatcher_always_emits_structured_json( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: False) + monkeypatch.setattr(sys.stdout, "isatty", lambda: False) assert cloud.run_cloud([]) == 0 assert json.loads(capsys.readouterr().out)["command"] == "strix cloud" @@ -1020,7 +1023,7 @@ def test_source_upload_rejects_untrusted_destinations_before_reading_file( source.write_bytes(b"approved source") monkeypatch.setattr(http, "_app_url_override", "https://app.strix.ai") monkeypatch.setattr( - http.requests, + requests, "put", lambda *_args, **_kwargs: pytest.fail("an untrusted URL must not receive source bytes"), ) @@ -1059,7 +1062,7 @@ def test_source_upload_allows_only_managed_or_same_origin_storage( return response monkeypatch.setattr(http, "_app_url_override", app_url) - monkeypatch.setattr(http.requests, "put", put) + monkeypatch.setattr(requests, "put", put) http.upload_file(signed_url, "upload-token", source) assert request_options["allow_redirects"] is False @@ -1080,7 +1083,7 @@ def test_source_upload_refuses_redirects_without_following_them( return response monkeypatch.setattr(http, "_app_url_override", "https://app.strix.ai") - monkeypatch.setattr(http.requests, "put", put) + monkeypatch.setattr(requests, "put", put) with pytest.raises(http.CloudError, match="unexpected redirect"): http.upload_file( @@ -1095,7 +1098,7 @@ def test_source_upload_refuses_redirects_without_following_them( def test_one_time_api_token_has_save_now_warning( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: - monkeypatch.setattr(render.sys.stdout, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) monkeypatch.setattr( http, "request", diff --git a/tests/test_cloud_idempotency.py b/tests/test_cloud_idempotency.py index 80246912..0da8dbb9 100644 --- a/tests/test_cloud_idempotency.py +++ b/tests/test_cloud_idempotency.py @@ -3,9 +3,11 @@ from __future__ import annotations import json +import time from typing import Any import pytest +import requests from strix.interface import cloud from strix.interface.cloud import http, runner @@ -30,7 +32,7 @@ class FakeResponse: @pytest.fixture(autouse=True) def _token(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("STRIX_API_TOKEN", "idempotency-test-token") - monkeypatch.setattr(runner.time, "sleep", lambda _seconds: None) + monkeypatch.setattr(time, "sleep", lambda _seconds: None) def test_scan_start_generates_and_sends_one_stable_key( @@ -197,7 +199,7 @@ def test_http_client_places_key_in_the_header(monkeypatch: pytest.MonkeyPatch) - seen.update(kwargs) return FakeResponse({"ok": True}) - monkeypatch.setattr(http.requests, "request", request) + monkeypatch.setattr(requests, "request", request) http.request("POST", "/scans", body={}, idempotency_key="header-key") assert seen["headers"]["Idempotency-Key"] == "header-key" assert seen["headers"]["Authorization"] == "Bearer idempotency-test-token" diff --git a/tests/test_cloud_payment_proxy.py b/tests/test_cloud_payment_proxy.py index e1a0ea04..b940e680 100644 --- a/tests/test_cloud_payment_proxy.py +++ b/tests/test_cloud_payment_proxy.py @@ -7,6 +7,7 @@ import urllib.request from typing import TYPE_CHECKING, Any import pytest +import requests from strix.interface.cloud import payment_proxy @@ -39,7 +40,8 @@ def _post(url: str, body: bytes, headers: dict[str, str] | None = None) -> bytes method="POST", ) with urllib.request.urlopen(request, timeout=2) as response: # noqa: S310 - return response.read() + body_bytes: bytes = response.read() + return body_bytes def test_bridge_bounds_decompressed_upstream_response(monkeypatch: pytest.MonkeyPatch) -> None: @@ -50,7 +52,7 @@ def test_bridge_bounds_decompressed_upstream_response(monkeypatch: pytest.Monkey return response monkeypatch.setattr(payment_proxy, "_MAX_UPSTREAM_RESPONSE_BYTES", 4) - monkeypatch.setattr(payment_proxy.requests, "request", fake_request) + monkeypatch.setattr(requests, "request", fake_request) with payment_proxy.wallet_payment_bridge( upstream_url="https://app.example.test/api/v1/billing/topup", @@ -80,7 +82,7 @@ def test_bridge_forwards_only_the_approved_request_and_protected_headers( captured.append(kwargs) return _StreamingResponse([b'{"ok":true}']) - monkeypatch.setattr(payment_proxy.requests, "request", fake_request) + monkeypatch.setattr(requests, "request", fake_request) with payment_proxy.wallet_payment_bridge( upstream_url="https://app.example.test/api/v1/billing/topup", api_token="strix-secret", # noqa: S106 @@ -124,7 +126,7 @@ def test_bridge_limits_valid_wallet_attempts(monkeypatch: pytest.MonkeyPatch) -> calls += 1 return _StreamingResponse([b"{}"]) - monkeypatch.setattr(payment_proxy.requests, "request", fake_request) + monkeypatch.setattr(requests, "request", fake_request) with payment_proxy.wallet_payment_bridge( upstream_url="https://app.example.test/api/v1/billing/topup", api_token="strix-secret", # noqa: S106 diff --git a/tests/test_cloud_session.py b/tests/test_cloud_session.py index e9ccc862..70349c1a 100644 --- a/tests/test_cloud_session.py +++ b/tests/test_cloud_session.py @@ -6,6 +6,7 @@ import json from typing import TYPE_CHECKING, Any import pytest +import requests from rich.console import Console from strix.interface import cloud, platform_cli, platform_identity @@ -55,7 +56,7 @@ def test_http_workspace_pin_is_captured_once( sent.append(dict(kwargs["headers"])) return Response({}) - monkeypatch.setattr(http.requests, "request", fake_request) + monkeypatch.setattr(requests, "request", fake_request) http.configure() platform_cli.save_record( { @@ -116,7 +117,7 @@ def test_logout_keeps_local_token_when_remote_outcome_is_not_definitive( } ) monkeypatch.setattr( - platform_cli.requests, + requests, "delete", lambda *_args, **_kwargs: Response({"detail": "unavailable"}, 503), ) diff --git a/tests/test_docker_client_delete.py b/tests/test_docker_client_delete.py index 1080cd05..ef93723c 100644 --- a/tests/test_docker_client_delete.py +++ b/tests/test_docker_client_delete.py @@ -12,6 +12,7 @@ would let it escape and surface a traceback on every teardown. from __future__ import annotations from types import SimpleNamespace +from typing import TYPE_CHECKING, cast from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -22,6 +23,10 @@ from requests.exceptions import ConnectionError as RequestsConnectionError from strix.runtime.docker_client import StrixDockerSandboxClient +if TYPE_CHECKING: + from agents.sandbox.session.sandbox_session import SandboxSession + + def _client_with_kill_error(exc: Exception) -> StrixDockerSandboxClient: """A StrixDockerSandboxClient whose containers.get(...).kill() raises ``exc``.""" client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient) @@ -31,9 +36,10 @@ def _client_with_kill_error(exc: Exception) -> StrixDockerSandboxClient: return client -def _session() -> object: +def _session(container_id: str | None = "abc123") -> SandboxSession: # delete() reads session._inner.state.container_id - return SimpleNamespace(_inner=SimpleNamespace(state=SimpleNamespace(container_id="abc123"))) + fake = SimpleNamespace(_inner=SimpleNamespace(state=SimpleNamespace(container_id=container_id))) + return cast("SandboxSession", fake) @pytest.mark.parametrize( @@ -45,7 +51,7 @@ def _session() -> object: ], ) @pytest.mark.asyncio -async def test_delete_swallows_best_effort_kill_errors(exc): +async def test_delete_swallows_best_effort_kill_errors(exc: Exception) -> None: """A torn-down socket (ConnectionError) or a gone/unhappy container (NotFound/APIError) during the kill must not propagate; delete() still delegates to the SDK's delete().""" @@ -62,7 +68,7 @@ async def test_delete_swallows_best_effort_kill_errors(exc): @pytest.mark.asyncio -async def test_delete_does_not_swallow_unrelated_errors(): +async def test_delete_does_not_swallow_unrelated_errors() -> None: """A programming error (e.g. ValueError) is not part of best-effort kill and must still propagate.""" client = _client_with_kill_error(ValueError("boom")) @@ -71,11 +77,11 @@ async def test_delete_does_not_swallow_unrelated_errors(): @pytest.mark.asyncio -async def test_delete_noop_without_container_id(): +async def test_delete_noop_without_container_id() -> None: """No container_id -> no kill attempt, just delegate.""" client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient) client.docker_client = MagicMock() - session = SimpleNamespace(_inner=SimpleNamespace(state=SimpleNamespace(container_id=None))) + session = _session(container_id=None) with patch.object( DockerSandboxClient, "delete", new=AsyncMock(return_value=session) diff --git a/tests/test_optional_deps.py b/tests/test_optional_deps.py index 43911dca..7acf18f3 100644 --- a/tests/test_optional_deps.py +++ b/tests/test_optional_deps.py @@ -11,7 +11,8 @@ PYPROJECT = Path(__file__).resolve().parent.parent / "pyproject.toml" def _optional_dependencies() -> dict[str, list[str]]: data = tomllib.loads(PYPROJECT.read_text(encoding="utf-8")) - return data["project"]["optional-dependencies"] + extras: dict[str, list[str]] = data["project"]["optional-dependencies"] + return extras def test_vertex_extra_pins_google_auth() -> None: diff --git a/tests/test_runner_mcp.py b/tests/test_runner_mcp.py index 814da0f7..3b5a79e9 100644 --- a/tests/test_runner_mcp.py +++ b/tests/test_runner_mcp.py @@ -185,6 +185,4 @@ async def test_roster_is_persisted_even_without_a_status_sink( ) assert persisted, "roster must persist even when no status sink is attached" - assert persisted[-1] == [ - {"name": "local_fs", "provider": None, "tool_count": 3, "dead": False} - ] + assert persisted[-1] == [{"name": "local_fs", "provider": None, "tool_count": 3, "dead": False}] diff --git a/tests/test_sarif_stride.py b/tests/test_sarif_stride.py index 70658bb0..c27f6749 100644 --- a/tests/test_sarif_stride.py +++ b/tests/test_sarif_stride.py @@ -34,7 +34,8 @@ def _finding(**overrides: Any) -> dict[str, Any]: def _rule_tags(doc: dict[str, Any]) -> list[str]: - return doc["runs"][0]["tool"]["driver"]["rules"][0]["properties"]["tags"] + tags: list[str] = doc["runs"][0]["tool"]["driver"]["rules"][0]["properties"]["tags"] + return tags def test_stride_tags_on_rule_for_known_cwe() -> None: diff --git a/tests/test_session_fd.py b/tests/test_session_fd.py index 976758be..7adb8144 100644 --- a/tests/test_session_fd.py +++ b/tests/test_session_fd.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import os from pathlib import Path from typing import Any, cast @@ -9,13 +10,40 @@ import pytest from strix.core.sessions import open_agent_session -def _count_open_fds() -> int | None: +def _fd_dir() -> Path | None: for path in (Path("/proc/self/fd"), Path("/dev/fd")): if path.is_dir(): - return len(list(path.iterdir())) + return path return None +def _count_open_fds() -> int | None: + fd_dir = _fd_dir() + return None if fd_dir is None else len(list(fd_dir.iterdir())) + + +def _count_open_fds_to(files: list[Path]) -> int | None: + """Count the descriptors this process holds on exactly ``files``. + + Matching on inode rather than on the process-wide total keeps the check + immune to sockets and pipes that unrelated background threads open while + the test runs. + """ + fd_dir = _fd_dir() + if fd_dir is None: + return None + wanted = {(stat.st_dev, stat.st_ino) for stat in (path.stat() for path in files)} + held = 0 + for entry in fd_dir.iterdir(): + try: + stat = os.fstat(int(entry.name)) + except (OSError, ValueError): + continue + if (stat.st_dev, stat.st_ino) in wanted: + held += 1 + return held + + @pytest.mark.asyncio async def test_sessions_hold_no_descriptors_while_parked(tmp_path: Path) -> None: """Descriptor use must track live operations, not the number of sessions. @@ -25,21 +53,20 @@ async def test_sessions_hold_no_descriptors_while_parked(tmp_path: Path) -> None scan, and fan-out multiplies those handles until the process runs out of file descriptors (#1018). A session that is not mid-operation should hold none. """ - baseline = _count_open_fds() - if baseline is None: + if _fd_dir() is None: pytest.skip("no /proc/self/fd or /dev/fd on this platform") - sessions = [open_agent_session(f"a{i}", tmp_path / f"s{i}.db") for i in range(60)] + db_paths = [tmp_path / f"s{i}.db" for i in range(60)] + sessions = [open_agent_session(f"a{i}", path) for i, path in enumerate(db_paths)] try: for _ in range(4): await asyncio.gather( *(s.add_items([{"role": "user", "content": "x"}]) for s in sessions) ) await asyncio.gather(*(s.get_items() for s in sessions)) - parked = _count_open_fds() - assert parked is not None - # 60 parked sessions, yet descriptors are back at the baseline. - assert parked - baseline <= 5, f"parked fds grew by {parked - baseline}" + parked = _count_open_fds_to(db_paths) + # 60 parked sessions, yet none of them holds its database open. + assert parked == 0, f"parked sessions hold {parked} database descriptors" finally: for s in sessions: s.close()