mirror of
https://github.com/usestrix/strix.git
synced 2026-10-17 04:34:03 +00:00
fix(cli): honor --scope-mode full on --resume and keep scope records consistent
--resume silently ignored --scope-mode and --diff-base: prepare_run() returned early before resolve_diff_scope_context() saw the flags, and load_resume_state() restored the persisted diff_scope verbatim, while scan_config still recorded the flag's scope_mode next to the old diff_scope. - reject --scope-mode diff and --diff-base with --resume: re-resolving a diff mid-run would rebuild the scope the resumed state was recorded against; only widening to full is supported - honor an explicit --scope-mode full by widening the resume past the recorded diff, rebuilding the instruction from the clean user text so the stale diff-scope preamble does not survive - restore the persisted scope_mode/diff_base when the flags are at their defaults, mirroring the scan_mode restore, so run.json/coverage.json cannot record a scope_mode next to a diff_scope it contradicts Fixes #1508
This commit is contained in:
parent
62b496430d
commit
88b19267c2
2 changed files with 188 additions and 0 deletions
|
|
@ -381,6 +381,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 +540,22 @@ 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")
|
||||
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. The persisted instruction carries the diff-scope
|
||||
# preamble, so a widened resume rebuilds it from the clean user text.
|
||||
args.diff_scope = {"active": False, "mode": "full"}
|
||||
if not args.user_explicit_instruction and args.user_instruction:
|
||||
args.instruction = args.user_instruction
|
||||
agents_path = runtime_state_dir(run_dir) / "agents.json"
|
||||
if not agents_path.exists():
|
||||
raise ResumeError(
|
||||
|
|
|
|||
164
tests/test_resume_scope.py
Normal file
164
tests/test_resume_scope.py
Normal file
|
|
@ -0,0 +1,164 @@
|
|||
"""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
|
||||
|
||||
|
||||
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"
|
||||
|
||||
|
||||
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.
|
||||
assert args.instruction == "find auth bugs"
|
||||
|
||||
|
||||
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"
|
||||
|
||||
|
||||
def test_resume_honors_scope_mode_full_without_recorded_user_instruction(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Widening still takes effect when the run predates user_instruction."""
|
||||
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 == PREAMBLE
|
||||
|
||||
|
||||
@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
|
||||
)
|
||||
Loading…
Add table
Reference in a new issue