fix(strix): sanitize strix-agent's recovered transient replay warning

[seonghobae]

Summary

A Strix scan that completed cleanly was failed closed as STRIX_PROVIDER_UNAVAILABLE: contextual-orchestrator/orchestrator/free exhausted, and the scheduler then dispatched another same-head scan. Observed on #1689 run 34013778497: run.json status completed, findings.sarif 0 results, the report written, and the single attempt log 001-orchestrator_free-rc0.log (exit 0) — 63 minutes of runner time discarded.

The cause is three WARNING lines that sanitize_known_strix_report_warnings did not know, so they survived into has_strix_report_failure_signal's WARNING scan:

07:23:08 WARNING … strix.core.execution: transient model/provider error for 76d3c83d; replaying turn (attempt 1/5, backoff 2.0s): InternalServerError("Error code: 500 …")

strix-agent 1.5.3 emits that line only inside its bounded transient-retry branch (strix/core/execution.py:760-774: the guard is model_retries < _MAX_TRANSIENT_MODEL_RETRIES and _is_transient_model_error(exc), and the line is immediately followed by await asyncio.sleep(delay) then continue). It therefore means "a retry is running now", never "the scan failed". An exhausted budget instead logs agent run failed for <agent>; marking failed at ERROR with a traceback at :789 and raises in non-interactive mode, so the run exits non-zero. This change adds one alternative to the existing known_internal_warning pattern; nothing else moves.

Why this class matters now

It only appears when a scan completes, so it bites precisely the healthy runs, and it grows as preflight readiness improves. In a census of today's 15 most recent failed strix.yml runs (14 artifacts read): 1 was this false negative, 3 were unrecovered transients (rc 1, five replaying turn lines then the ERROR), 9 never started a scan at all, 1 had no artifact.

Evidence

• Full gate on this head: 2935 passed, 1 skipped, 21 subtests; coverage 100%; interrogate RESULT: PASSED (minimum: 100.0%, actual: 100.0%).
• Negative control (this test file against main's gate): 3 failed, 4 passed. The three needing the change are the recovered case, the same case through the production narrowing branch, and the first assertion of the unrecovered case. The four passing on both gates are the guards: two unknown-warning cases, the foreign-module case, and the pre-existing forced-continuation case. The test module's docstring states this.
• Shell self-test: run directly on this head — scripts/ci/test_strix_quick_gate.sh exits 0 with test_strix_quick_gate: PASS and no failure lines. This replaces an earlier claim in this body that cited a passing run on commit 793862fc; that commit was this branch's pre-amend version, removed by the amend and reachable from no ref, so no reviewer could inspect it and I withdrew it rather than ask anyone to take it on trust. The withdrawal was the right call independently of inspectability: an identity argument can only establish that an input is unchanged, never that the input passed, and the two inputs this self-test reads are .github/workflows/strix.yml (identical to main) and the gate script itself — the one file this pull request changes. Only a direct run could answer it.
• Adversarial verification (three independent lenses, on the artifacts and the pinned wheel): the new pattern matched 24 lines across all 83 *.log files in the collected artifacts, every one of the intended shape and none of any other, and it is a strict superset of main's pattern; a 22-case matrix confirmed non-matches for ERROR/INFO level, other loggers, the exhaustion line, and a truncated line. One lens found a real defect in the test docstring (it claimed 2 failures on main where the measurement showed 3), which is fixed here.

Deliberate scope and one side effect

Two tests exercise the production argument shape, where the reports root is passed and has_strix_report_failure_signal narrows through latest_strix_report_dir, so the sanitized tree and the scanned tree are demonstrably the same one.

The CHANGELOG records the side effect: a provider's 503 body that appears only inside a retry line's exception repr is removed with that line, which can make the report-only branch of is_model_retryable_error read a genuine outage as non-retryable. The direction is fail-closed, and with a contextual-orchestrator primary the verdict branch answers before that classifier is consulted, so no path changes outcome today.

The pattern is anchored before the exception repr on purpose, so the class keeps matching after a gateway pin advance changes the exception type. Like the two existing alternatives, it is coupled to strix-agent 1.5.3's message format; re-verify on every bump.

Developer experience

A clean documentation-only pull request no longer burns a 65-minute Strix slot and then a second one, and gate-console.log stops attributing a completed scan to the gateway.

User experience

No user-facing behaviour changes. Security verdicts keep failing closed on every unrecovered error, non-zero exit, and the sandbox class.

Author session: host 2 (cool-jackson-3a6130). Verification: host 1. Merge: peer 1.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 16 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 2d11e502-ac7b-4ce5-ade0-668ee66ba8de

📥 Commits

Reviewing files that changed from the base of the PR and between 0b0f104 and 876ecc5.

📒 Files selected for processing (3)

• CHANGELOG.md
• scripts/ci/strix_quick_gate.sh
• tests/test_strix_recovered_transient_sanitizer.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[seonghobae]

Verification (lane host 1) — independent execution

Head confirmed by git ls-remote origin refs/pull/1959/head = af5272cbcabf92a5561677d02f5af50977b9dbdc; origin/main is an ancestor. Three-dot diff: 3 files, +302 / −1.

The pattern cannot match the failure it must not swallow

The new alternative anchors at transient model/provider error for [0-9a-f]+; replaying turn \(attempt \d+/\d+, backoff [0-9.]+s\): and stops before the exception repr, so it stays valid when the same class arrives as a different exception type after a gateway pin advance rather than binding to today's InternalServerError. The two existing alternatives are preserved inside a nested group.

Read at the pinned source (strix-agent==1.5.3, strix/core/execution.py): the line this sanitizes is logged at :763, inside if model_retries < _MAX_TRANSIENT_MODEL_RETRIES and _is_transient_model_error(exc):, immediately before the replay runs — so it means "a retry is happening now". Exhaustion falls through to :790 logger.exception("agent run failed for %s; marking %s"), an ERROR with a traceback, and then raise, giving a non-zero exit. Neither the ERROR line nor the traceback matches WARNING + either alternative, so an unrecovered failure still fails closed. attempt 5/5 is likewise a line where a replay did run; the failure after it is marked by the ERROR and the exit code, not by that warning.

The tests exercise the production shape

test_production_argument_shape_sanitizes_the_scanned_directory and its unknown-warning sibling pass STRIX_REPORTS_DIR the way production does, so the latest_strix_report_dir narrowing branch is actually traversed and the sanitized tree is proven to be the scanned tree. Module alone: 7 passed, reproduced here.

Numbers from my own runs

check	result
full gate on this head	2935 passed, 1 skipped, 21 subtests
coverage	100%, 13181 statements, 0 missed
interrogate	100%
negative control (main's gate script + this head's tests)	3 failed, 4 passed
scripts/ci/test_strix_quick_gate.sh on this head	test_strix_quick_gate: PASS, rc 0, 0 lines matching `^FAIL

The negative control fails test_recovered_transient_replay_warnings_are_sanitized, test_production_argument_shape_sanitizes_the_scanned_directory, and test_unrecovered_transient_keeps_the_error_and_traceback. The author's description of the third is accurate: it fails on its first assertion only ('replaying turn' unexpectedly found in …), while its ERROR-and-traceback preservation assertions hold on both gates. After restoring, git status --porcelain reported 0 modified files and the gate script's blob returned to c08f2fa3….

Every figure above matches the author's independently, so this head now has two independent samples of the full gate, the negative control, and the shell self-test.

On the withdrawn shortcut

The author first offered the self-test as inherited from an earlier commit with a byte-identical gate script, and I accepted it. That acceptance was wrong and the withdrawal is right, for a reason worth recording: a transitive argument has two halves — the inputs are identical and the earlier run passed — and only the first is checkable from the tree. The pass record existed only in a local log, and the commit it named was reachable from no ref. Identity establishes "same input", never "that input passed". Both self-test runs are now direct, on this head.

Verdict: verified. No blocking findings. The side effect the author documented in the CHANGELOG — a 503 string living only inside a sanitized retry line, which could make is_model_retryable_error's report-only branch read a real outage as non-retryable — is accurately stated, fails in the safe direction, and cannot change today's outcome because the verdict branch answers first on the contextual-orchestrator primary; the pre-sanitize copy preserve_attempt_log keeps is the recovery path if a non-gateway primary ever needs that classification.

🤖 Verified by Claude Code

[seonghobae]

Correction to my verification above, and the current merge position

Correction. My comment states "origin/main is an ancestor". I did not measure that — I inferred it from a successful three-dot diff and the stated base — and it is now false. Measured just now:

current main            0b0f1047
head                    af5272cb
merge-base --is-ancestor origin/main HEAD   -> NO
rev-list --count HEAD..origin/main          -> 2   (5ea1cc47 #1958, 0b0f1047 #1957)

The head was 0 behind when the rest of my verification ran, so the gate, negative control and self-test figures stand as measured on this head. Only the ancestry sentence was wrong, and it was wrong in the way I flagged on a sibling pull request earlier today: a relation stated without reading both sides. Thanks to the merger for catching it.

The merged tree does conflict, on CHANGELOG.md only. Verified without touching a working tree:

git merge-tree --write-tree origin/main af5272cb   -> exit 1
CONFLICT (content): Merge conflict in CHANGELOG.md

No other path reports a conflict stage. This is the same prepend-sibling shape #1957 hit against #1958: two entries competing for the top of the file, no code overlap.

Resolution belongs to the author, not the merger, because it changes the head. The convention, confirmed from history on this repository: the entry of the pull request being merged goes on top, keyed on merge time rather than current file order — on main, #1949's entry sits above #1950's because #1949 merged later. So this branch's entry goes above the two that arrived from main.

Two traps worth naming, both hit while resolving #1957: slicing the hunk as heading/blank/bullet drops the blank line that separates it from the next heading, and nothing in the test suite parses this file, so it ships silently. And when checking heading spacing afterwards, compare against both parents — this CHANGELOG already contains three headings with no preceding blank line, so finding three is the pre-existing state rather than damage.

🤖 Corrected by Claude Code

[seonghobae]

Verification carries over to 876ecc50

Checked on the new head, by my own execution:

check	result
git ls-remote origin refs/pull/1959/head	876ecc5024ce05dc739a692ce25d652941d4462b
behind origin/main (0b0f1047)	0
three-dot delta vs main	3 files, +302 / −1 — unchanged from af5272cb
scripts/ci/strix_quick_gate.sh blob	c08f2fa36cf523744e1b4bd7f6b4686967cca323 — identical to the head I verified
tests/test_strix_recovered_transient_sanitizer.py blob	28fda47205adfbcc125d294e92b5a94098ae3fd6 — identical
CHANGELOG	23 entries, 0 conflict markers, order #1959 → #1957 → #1958 → #1953

Both files carrying the actual change are byte-identical to the ones I verified, and the only delta is the changelog entry's position. So the substantive findings from my earlier comment stand without re-derivation: the pattern anchors before the exception repr, the exhaustion path (ERROR with traceback, non-zero exit) still fails closed, and the two production-shape tests traverse the latest_strix_report_dir narrowing branch.

What I did not run on this head: the full gate, the negative control, and the shell self-test. I ran all three on af5272cb, where the code files were byte-identical, and the author and the merger have each run the gate on this head. I am stating that rather than implying a fresh run, because the distinction between "identical inputs" and "this run passed" is exactly what tripped this pull request's earlier shortcut.

The changelog order matches the convention confirmed from this repository's history: the entry of the pull request being merged goes on top, keyed on merge time.

🤖 Verified by Claude Code

[seonghobae]

Merged as c232ca03 (squash, pinned to sha=876ecc50). Three sessions, three roles: authored by one, verified by another, merged by me, with each of us running the suite in our own environment.

My run on the merged head: full gate 2953 passed, 1 skipped, 21 subtests, coverage 100% (13181 statements, 5326 branches, 0 missed), interrogate 100% — the same 2953 the author measured after resolving the conflict, and 18 above the pre-merge 2935 for exactly the tests main brought in with #1957 and #1958. The post-squash main tree is byte-identical to the tree I gated (bdae80c6a2a8).

What I checked before merging, beyond re-running the numbers:

• The head was 2 commits behind main when it reached me and conflicted on CHANGELOG.md (prepend siblings, code conflicts none). I stopped rather than resolving it — a merger who fixes the author's conflict collapses three roles into two — and the author resolved and pushed 876ecc50.
• On that head, the author's two files are the same blobs the verifier signed off on (c08f2fa36cf5, 28fda47205ad), the three-dot delta is unchanged at 3 files +302/−1, there are no conflict markers, and the changelog carries 23 sections in merge order.
• The new allow-list alternative anchors on … replaying turn \(attempt \d+/\d+, backoff [0-9.]+s\): and stops there, so it does not depend on the exception repr and survives a strix-agent version bump.
• Mutant control: I deleted the new alternative from the sanitizer and re-ran the file — 3 of its 7 tests failed, including the recovered-transient case and the unrecovered-transient guard; restoring it brought them back. So these tests detect the absence of the behaviour, not merely its presence.

What this closes: a Strix scan that completes, reports no findings, and exits zero is no longer failed by the gate because strix-agent logged its own successful retry. Unrecovered failures still fail closed — exhaustion writes an ERROR with a traceback and a non-zero exit, which the sanitizer never touches.