fix(strix): keep model preflight timeout positive - #1601
Merged
Conversation
Supersede closed draft #1597 from current protected main. The Strix workflow blob on main is byte-identical to the pre-fix blob verified in #1597; the one-line LLM_TIMEOUT=300 repair and regression contract already passed the focused runner test before the workflow-permission-only push rejection. No provider bypass, fallback, severity, retry, or fail-closed policy change.
|
Warning Review limit reachedNext included review available in 40 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
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. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 Contract edits can skip Strix quality
The changed-path workflow omits this test from its paths filter. A later contract-only edit can bypass the dedicated Strix quality job.
Was this helpful? React with 👍 or 👎 to provide feedback.
| IS_PR_EVIDENCE_RUN: ${{ (github.event_name == 'pull_request_target' || github.event.client_payload.pr_number != '') && 'true' || 'false' }} | ||
| run: | | ||
| export LLM_TIMEOUT=0 | ||
| export LLM_TIMEOUT=300 |
Contributor
There was a problem hiding this comment.
🟡 Required Strix quality check always fails
When LLM_TIMEOUT is 300, the required shell contract still demands 0 and records a failure. Every workflow change therefore fails quality CI.
Prompt for agents
Update scripts/ci/test_strix_quick_gate.sh in assert_strix_workflow_pr_trigger_hardened so its LLM_TIMEOUT contract requires the new positive 300-second value instead of 0. Keep the other unlimited timeout assertions unchanged, and ensure the existing Strix changed-path quality workflow executes both the Python regression test and the shell contract consistently.
Was this helpful? React with 👍 or 👎 to provide feedback.
seonghobae
added a commit
that referenced
this pull request
Sep 1, 2026
Supersede the temporary 300-second model deadline from #1601 with a version-gated Strix 1.5.3 compatibility launcher. Keep LLM_TIMEOUT=0 for actual inference, normalize only the two inconsistent warm-up wait_for calls, and retain pinned-executable SHA-256 validation and fail-closed module/version checks. Recorded RED: 05d4521.
This was referenced Sep 1, 2026
seonghobae
pushed a commit
that referenced
this pull request
Sep 1, 2026
#1601 (already on main) fixed a real bug: Strix 1.5.3 passes LLM_TIMEOUT to asyncio.wait_for, so LLM_TIMEOUT=0 immediately cancels contextual-orchestrator model preflight rather than meaning "unbounded" as intended. It changed the workflow's own export to LLM_TIMEOUT=300 (a positive value the compat launcher then neutralizes into an unbounded deadline), and added a dedicated regression for that contract, but left this test's assertion checking for the literal old string. Updated it to assert the new value.
seonghobae
pushed a commit
that referenced
this pull request
Sep 2, 2026
… main exact-head-path-policy failed on this branch's own copy of scripts/ci/test_strix_quick_gate.sh for two unrelated stale assertions, neither touching this PR's actual Strix evidence-hardening diff: 1. The LLM_TIMEOUT assertion (line 302) still expected the round-6 'export LLM_TIMEOUT=300' value this PR itself introduced on 2026-09-01 to match #1601's contemporary state. Main later reverted strix.yml back to 'export LLM_TIMEOUT=0' via #1658 ("remove the 300s LLM_TIMEOUT cap") without ever having carried the 300 assertion on main's own copy of this file, so a same-line 3-way merge always kept this branch's now-stale text with no conflict to surface it. Restored the assertion to match main's (and strix.yml's) current, unchanged content. 2. The scheduler-heartbeat cron assertion (line 1562) still expected the pre-#1704 'cron: "*/30 * * * *"' quarter-hourly schedule. #1704 ("lengthen scan-pr-queue's own heartbeat, don't drop it") lengthened pr-review-merge-scheduler.yml's repository-local scan to hourly ('cron: "30 * * * *"') for the same Actions-capacity reason as #1630, and added/updated the matching pytest contract (tests/test_actions_queue_saturation_scheduler_cadence.py, tests/test_required_workflow_queue_contract.py) but missed this repo's separate, duplicate shell-harness assertion of the same contract. Confirmed this exact failure reproduces identically on fresh main (same stale assertion, same actual hourly cron) -- it predates and is unrelated to this PR's diff. Updated the assertion to match #1704's now-current cron and added the mirroring assert_file_not_contains for the retired quarter-hourly string, same pattern #1704 already established in its own pytest contract. Verified on the merged head (origin/main merged in via the preceding merge commit, mergeable_state was "behind" only, no conflicts): - bash scripts/ci/test_strix_quick_gate.sh (full harness): PASS, 0 failures (previously 2: the LLM_TIMEOUT and cron assertions above). - PYTHONPATH=. python3.12 -m coverage run -m pytest tests -q: 2644 passed, 1 skipped, 21 subtests. - coverage report --show-missing: 100% on scripts/ci. - interrogate: 100% (RESULT: PASSED, minimum: 100.0%, actual: 100.0%). - python -m compileall on the five exact-head-path-policy test files, bash -n scripts/ci/strix_quick_gate.sh, git diff --exit-code: all clean after this commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Successor scope
Supersedes closed Draft #1597 without reusing or force-updating its branch. Current protected base is
main@547fcc875d70a9489b30a6863692c977f1444fe2; exact head isc6aa8761bb2b1007e9c05ac51748fd7734735ff3. Effective diff is exactly.github/workflows/strix.ymlplustests/test_strix_llm_timeout_contract.py.Root cause
Strix 1.5.3 passes
LLM_TIMEOUTtoasyncio.wait_for. The central workflow exportedLLM_TIMEOUT=0, which immediately cancels preflight instead of disabling the timeout. This explained the emptyLLM CONNECTION FAILEDseen after contextual-orchestrator health/route/probe had already succeeded.TDD / execution evidence
The predecessor #1597 recorded a RED contract requiring the documented 300-second timeout. Its one-shot repair runner
33506853161, job99852828311, then changed onlyLLM_TIMEOUT=0toLLM_TIMEOUT=300;PYTHONPATH=. python3 -m pytest -q tests/test_strix_llm_timeout_contract.pycompleted1 passed. The subsequent push failed solely because the workflow's GitHub App token lacked permission to update.github/workflows/strix.yml; source/test execution was already GREEN.Before constructing this successor, the current protected-main Strix workflow blob (
672c9b796e707d2b8ec345c95e16f17e66461ddb) was compared with the predecessor pre-fix writer blob and is byte-identical. The successor tree is therefore current protected main plus only the already-verified one-line timeout repair and regression contract. No stale main bytes, temporary source-fix workflow, force push, provider bypass, fallback, severity, retry, or fail-closed policy weakening are included.Fresh exact-head checks/reviews remain authoritative for landing. Queue/review waits are non-blocking; do not weaken gates to make them pass.