diff --git a/strix/interface/cli.py b/strix/interface/cli.py index a8992ce49..482e625a0 100644 --- a/strix/interface/cli.py +++ b/strix/interface/cli.py @@ -97,7 +97,9 @@ async def run_cli(args: Any) -> None: # noqa: PLR0915 "workspace_files": getattr(args, "workspace_files", None) or [], "scope_mode": getattr(args, "scope_mode", "auto"), "diff_base": getattr(args, "diff_base", None), - "resume_instruction": getattr(args, "user_explicit_instruction", None) or "", + "resume_instruction": getattr(args, "resume_instruction", None) + or getattr(args, "user_explicit_instruction", None) + or "", } report_state = ReportState(args.run_name) diff --git a/strix/interface/cli_args.py b/strix/interface/cli_args.py index c9b6951be..b21230deb 100644 --- a/strix/interface/cli_args.py +++ b/strix/interface/cli_args.py @@ -24,6 +24,15 @@ from strix.report.runs import list_run_summaries # Severities ``--fail-on`` accepts, most severe first. FAIL_ON_SEVERITIES = ("critical", "high", "medium", "low", "info") +# Injected into a resumed session that was widened past its recorded diff +# scope, so the diff-scope preamble replayed from the original conversation +# stops telling the agents to report findings only inside the old diff. +RESUME_SCOPE_WIDENED_INSTRUCTION = ( + "The diff scope of the original run has been lifted (--scope-mode full): " + "analyze the full repository. Findings are no longer limited to the " + "changes listed in the earlier scope instructions." +) + def get_version() -> str: try: @@ -381,6 +390,14 @@ Strix Cloud: "--resume picks up where the prior run left off, including the " "original target list." ) + if args.scope_mode == "diff" or args.diff_base: + # Re-resolving a diff mid-run would rebuild the scope the resumed + # state was recorded against; only widening to "full" is supported. + parser.error( + "Cannot combine --resume with --scope-mode diff or --diff-base. " + "Start a fresh run to change the diff scope; --scope-mode full " + "is honored and widens the resume to the full repository." + ) if not args.resume.strip(): # A bare --resume: main() opens the inline picker of prior runs # before anything launches; headless has nobody to pick, so it @@ -532,6 +549,32 @@ def load_resume_state(args: argparse.Namespace) -> None: persisted_scan_mode = state.get("scan_mode") if persisted_scan_mode and args.scan_mode == "deep": args.scan_mode = persisted_scan_mode + # Same contract as scan_mode above: the persisted scope describes the run + # being resumed and wins over the flags' defaults, so the record never + # writes a scope_mode next to a diff_scope it contradicts. + persisted_scope_mode = state.get("scope_mode") + if persisted_scope_mode and args.scope_mode == "auto": + args.scope_mode = persisted_scope_mode + if state.get("diff_base") and not args.diff_base: + args.diff_base = state.get("diff_base") + # What reaches the resumed session as a fresh message: the runner injects + # scan_config["resume_instruction"], because a replayed session would + # otherwise ignore any new instruction entirely. + args.resume_instruction = args.user_explicit_instruction or "" + if args.scope_mode == "full": + # An explicit --scope-mode full widens the resume to the full + # repository, as --help promises; the flag's "auto" default keeps the + # intent unambiguous. + widened = bool(isinstance(args.diff_scope, dict) and args.diff_scope.get("active")) + args.diff_scope = {"active": False, "mode": "full"} + if widened and not args.user_explicit_instruction: + # The instruction recorded for a diff-scoped run carries the + # preamble prepare_run() prepended to it; the widened resume + # records the clean user text instead, and injects a note into + # the replayed session so the old restriction is lifted for + # the agents too, not just in the published record. + args.instruction = state.get("user_instruction") or None + args.resume_instruction = RESUME_SCOPE_WIDENED_INSTRUCTION agents_path = runtime_state_dir(run_dir) / "agents.json" if not agents_path.exists(): raise ResumeError( diff --git a/strix/interface/tui/runtime.py b/strix/interface/tui/runtime.py index ae9832369..88ab81986 100644 --- a/strix/interface/tui/runtime.py +++ b/strix/interface/tui/runtime.py @@ -99,7 +99,9 @@ class GoTuiRuntime: "workspace_files": getattr(self.args, "workspace_files", None) or [], "scope_mode": self.args.scope_mode, "diff_base": self.args.diff_base, - "resume_instruction": self.args.user_explicit_instruction or "", + "resume_instruction": getattr(self.args, "resume_instruction", None) + or getattr(self.args, "user_explicit_instruction", None) + or "", "workspace_mount": getattr(self.args, "workspace_mount", None) or "", "workspace_subdir": getattr(self.args, "workspace_subdir", None) or "", } diff --git a/tests/test_resume_scope.py b/tests/test_resume_scope.py new file mode 100644 index 000000000..91321c4f9 --- /dev/null +++ b/tests/test_resume_scope.py @@ -0,0 +1,201 @@ +"""Tests for how --resume interacts with the persisted diff scope.""" + +from __future__ import annotations + +import importlib +import json +import sys +from types import SimpleNamespace +from typing import TYPE_CHECKING, Any + +import pytest + +from strix.interface.cli_args import RESUME_SCOPE_WIDENED_INSTRUCTION + + +if TYPE_CHECKING: + from pathlib import Path + + +cli_main: Any = importlib.import_module("strix.interface.main") + +PREAMBLE = "Scope your review to the following changes.\n\n" + + +def _stub_settings(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + cli_main, + "load_settings", + lambda: SimpleNamespace(runtime=SimpleNamespace(max_local_copy_mb=1024)), + ) + + +def _write_run_record(runs_dir: Path, run_name: str, record: dict[str, Any]) -> None: + """Write a resumable run: its record plus the agent snapshot resume needs.""" + run_dir = runs_dir / run_name + run_dir.mkdir(parents=True, exist_ok=True) + (run_dir / "run.json").write_text(json.dumps(record), encoding="utf-8") + state_dir = run_dir / ".state" + state_dir.mkdir(exist_ok=True) + (state_dir / "agents.json").write_text("{}", encoding="utf-8") + + +def _diff_scoped_record(**overrides: Any) -> dict[str, Any]: + record: dict[str, Any] = { + "run_name": "pentest_abcd", + "targets_info": [], + "local_sources": [], + "instruction": PREAMBLE + "find auth bugs", + "user_instruction": "find auth bugs", + "scan_mode": "deep", + "scope_mode": "diff", + "diff_base": "origin/main", + "diff_scope": {"active": True, "mode": "diff", "repos": []}, + } + record.update(overrides) + return record + + +def _resume(monkeypatch: pytest.MonkeyPatch, *extra: str) -> Any: + monkeypatch.setattr(sys, "argv", ["strix", "--resume", "pentest_abcd", *extra]) + return cli_main.parse_arguments() + + +def test_resume_restores_persisted_scope_over_flag_defaults( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A plain resume keeps the run's recorded scope_mode and diff_base.""" + work = tmp_path / "project" + work.mkdir() + monkeypatch.chdir(tmp_path) + _write_run_record( + tmp_path / "strix_runs", + "pentest_abcd", + _diff_scoped_record(workspace_mount=str(work)), + ) + + args = _resume(monkeypatch) + + assert args.scope_mode == "diff" + assert args.diff_base == "origin/main" + assert args.diff_scope == {"active": True, "mode": "diff", "repos": []} + assert args.instruction == PREAMBLE + "find auth bugs" + assert args.resume_instruction == "" + + +def test_resume_honors_scope_mode_full(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """An explicit --scope-mode full widens the resume past the recorded diff.""" + work = tmp_path / "project" + work.mkdir() + monkeypatch.chdir(tmp_path) + _write_run_record( + tmp_path / "strix_runs", + "pentest_abcd", + _diff_scoped_record(workspace_mount=str(work)), + ) + + args = _resume(monkeypatch, "--scope-mode", "full") + + assert args.scope_mode == "full" + assert args.diff_scope == {"active": False, "mode": "full"} + # The recorded instruction carries the diff-scope preamble; the widened + # resume keeps only the clean user text, and injects a note lifting the + # old restriction into the replayed session. + assert args.instruction == "find auth bugs" + assert args.resume_instruction == RESUME_SCOPE_WIDENED_INSTRUCTION + + +def test_resume_honors_scope_mode_full_with_new_instruction( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A new instruction given alongside --scope-mode full is kept verbatim.""" + work = tmp_path / "project" + work.mkdir() + monkeypatch.chdir(tmp_path) + _write_run_record( + tmp_path / "strix_runs", + "pentest_abcd", + _diff_scoped_record(workspace_mount=str(work)), + ) + + args = _resume(monkeypatch, "--scope-mode", "full", "--instruction", "check the API") + + assert args.scope_mode == "full" + assert args.diff_scope == {"active": False, "mode": "full"} + assert args.instruction == "check the API" + assert args.resume_instruction == "check the API" + + +def test_resume_honors_scope_mode_full_without_recorded_user_instruction( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A widened resume drops a preamble-only instruction and injects the note.""" + work = tmp_path / "project" + work.mkdir() + monkeypatch.chdir(tmp_path) + _write_run_record( + tmp_path / "strix_runs", + "pentest_abcd", + _diff_scoped_record(workspace_mount=str(work), instruction=PREAMBLE, user_instruction=None), + ) + + args = _resume(monkeypatch, "--scope-mode", "full") + + assert args.diff_scope == {"active": False, "mode": "full"} + assert args.instruction is None + assert args.resume_instruction == RESUME_SCOPE_WIDENED_INSTRUCTION + + +def test_resume_with_scope_mode_full_on_a_full_run_injects_nothing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Re-asserting full on a run that was already full is a plain no-op.""" + work = tmp_path / "project" + work.mkdir() + monkeypatch.chdir(tmp_path) + _write_run_record( + tmp_path / "strix_runs", + "pentest_abcd", + { + "run_name": "pentest_abcd", + "targets_info": [], + "local_sources": [], + "instruction": "find auth bugs", + "user_instruction": "find auth bugs", + "scan_mode": "deep", + "scope_mode": "full", + "diff_scope": {"active": False, "mode": "full"}, + "workspace_mount": str(work), + }, + ) + + args = _resume(monkeypatch, "--scope-mode", "full") + + assert args.diff_scope == {"active": False, "mode": "full"} + assert args.instruction == "find auth bugs" + assert args.resume_instruction == "" + + +@pytest.mark.parametrize("extra", [["--scope-mode", "diff"], ["--diff-base", "origin/main"]]) +def test_resume_rejects_diff_scope_flags( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + extra: list[str], +) -> None: + """Re-resolving a diff mid-run is refused instead of silently ignored.""" + work = tmp_path / "project" + work.mkdir() + monkeypatch.chdir(tmp_path) + _write_run_record( + tmp_path / "strix_runs", + "pentest_abcd", + _diff_scoped_record(workspace_mount=str(work)), + ) + + with pytest.raises(SystemExit): + _resume(monkeypatch, *extra) + + assert "Cannot combine --resume with --scope-mode diff or --diff-base" in ( + capsys.readouterr().err + )