Skip to content

FIX Prevent duplicate attacks when scenario resume history cannot be read - #2857

Merged
Roman Lutz (romanlutz) merged 2 commits into
microsoft:mainfrom
biefan:fix/scenario-resume-read-failures
Sep 26, 2026
Merged

Roman Lutz (romanlutz) merged 2 commits into
microsoft:mainfrom
biefan:fix/scenario-resume-read-failures

Conversation

@biefan

@biefan biefan (biefan) commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Description

A transient failure while reading completed attack results can make a resumed scenario execute already-completed objectives again. If the progress query keeps failing but the other memory operations succeed, the old implementation can still run all objectives and mark the scenario COMPLETED.

Concrete example: a scenario has two groups: A has a persisted successful result; B has not run yet. During resume, get_attack_results(scenario_result_id=...) raises a database error.

Condition Before After
Progress read fails once, then recovers The error is logged; A and B both execute Retry the progress read first; execute only B
Progress reads keep failing A and B can still execute and the run can report completion Exhaust the existing retry budget and report failure; start no attacks
10 atomic groups in one reconciliation Read the entire scenario history 10 times Read and index one shared snapshot

Root cause: _get_completed_objective_hashes_for_attack() caught all read exceptions and returned an empty set, making an unavailable history indistinguishable from a successfully read empty history. _get_remaining_atomic_attacks_async() also called it once per atomic group, repeatedly loading and scanning every result in the scenario.

Change: load all completed objective hashes once per reconciliation, indexed by (parent_collection, parent_eval_hash), before mutating any group's remaining seeds. A read exception now reaches the existing scenario retry/failure handling. Each retry reads a fresh snapshot. Technique identities remain separate, legacy rows without an eval hash still match by collection name, and ERROR results remain eligible for retry. No schema migration or public API change is required.

This reduces the reconciliation history reads from N to 1 for N atomic groups. It is a query-count improvement verified by the test below, not a claim about measured production latency.

Tests and Documentation

Three regressions failed on unmodified upstream and pass with the fix:

  1. One transient history-read failure must not re-execute A; after recovery only B runs and there is still exactly one stored A result.
  2. Persistent read failures must exhaust the retry budget, persist FAILED when status storage is available, leave the original seed groups intact, and invoke neither A nor B.
  3. Ten atomic groups reconcile from exactly one get_attack_results() call and correctly skip the five already-completed groups.

The tests use the real Scenario retry loop and isolated SQLite persistence, with faults injected specifically at the history-read boundary. Existing tests continue to cover partial-result retry, technique identity separation, and legacy attribution.

  • Scenario/retry/partial-result tests: 105 passed.
  • make unit-test with Python 3.11/default dependencies: 20,414 passed, 146 skipped, 1 failed. The sole failure is the existing test_get_seed_dataset_summaries_follows_a_trailing_blank_insensitive_collation in tests/unit/memory/memory_interface/test_interface_seed_prompts.py, also reproduced at unmodified upstream 73d90a7 with the same environment.
  • Applicable pre-commit hooks passed with all optional dependencies installed, including repository-wide ty check pyrit.
  • Also applied together with FIX Stop scenario workers when an atomic attack is cancelled #2851 and FIX Drain nested scoring and batch tasks on failure #2856 on upstream 73d90a7: 582 relevant combined regressions passed. The patches apply together without conflicts.

To reproduce the targeted checks:

uv run pytest tests/unit/scenario/core/test_scenario_retry.py -q -k 'resume_read_failure or resume_loads_one'

Updated the private resume reconciliation docstrings to describe snapshot loading and error propagation. No notebooks changed; no live target or production database was used.

@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Sep 26, 2026
Merged via the queue into microsoft:main with commit e84f18f Sep 26, 2026
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants