Skip to content

fix(strix): keep orchestrator model preflight timeout positive - #1600

Closed
seonghobae wants to merge 5 commits into
mainfrom
fix/strix-orchestrator-timeout-current-main
Closed

fix(strix): keep orchestrator model preflight timeout positive#1600
seonghobae wants to merge 5 commits into
mainfrom
fix/strix-orchestrator-timeout-current-main

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Current-main Ready successor to closed Draft #1597. The previous branch was concurrently reset/closed after the verified repair had already produced the corrected workflow blob, so this successor is rebuilt non-destructively from protected main@547fcc875d70a9489b30a6863692c977f1444fe2 plus exactly the two intended #1597 files.

Root cause: Strix 1.5.3 feeds LLM_TIMEOUT into both request settings and asyncio.wait_for; LLM_TIMEOUT=0 cancels model preflight immediately instead of disabling a timeout. The exact production repair changes only export LLM_TIMEOUT=0 to export LLM_TIMEOUT=300, matching the existing RED regression contract. A hosted one-shot run installed the repository hash-pinned test tooling and proved the focused regression GREEN (1 passed); its later GitHub-App workflow-permission error occurred only while trying to publish the workflow-file commit. The transformed workflow blob was nevertheless captured and is the exact blob carried here. The temporary source-fix workflow is absent from this tree.

No provider bypass, fallback-model policy, retry count, finding threshold, fail-closed classification, or process/total timeout is changed. Fresh exact-head review and hosted evidence on this successor are authoritative; predecessor PR evidence does not transfer.


Devin Review

seonghobae and others added 5 commits September 1, 2026 20:57
Preserve protected-main Noema fixture repairs while carrying only #1597's verified Strix LLM timeout correction and regression forward. The temporary one-shot workflow is absent from the resulting tree.
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 41 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: fbd7602c-ba37-4b95-b6a7-b8998860ead9

📥 Commits

Reviewing files that changed from the base of the PR and between 547fcc8 and aa8157d.

📒 Files selected for processing (2)
  • .github/workflows/strix.yml
  • tests/test_strix_llm_timeout_contract.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

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

@seonghobae seonghobae closed this Sep 1, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Devin Review

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Five-minute preflight rejects healthy models

When model preflight exceeds five minutes, LLM_TIMEOUT cancels it despite the unbounded inference contract. Slow healthy routes fail central Strix checks.

Prompt for agents
Restore the repository's unbounded Strix inference contract without reintroducing Strix 1.5.3's immediate cancellation when LLM_TIMEOUT is zero. The current dependency feeds zero into asyncio.wait_for, while a positive value imposes a forbidden fixed deadline on preflight and request settings. Patch or upgrade the pinned Strix integration so the disabled value maps to no asyncio deadline, then keep .github/workflows/strix.yml, scripts/ci/test_strix_quick_gate.sh, and the Python timeout contract test aligned. Verify both that zero/disabled no longer cancels immediately and that a model call lasting beyond five minutes is not cancelled.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

workflow,
)

assert configured_timeouts == ["300"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Regression contracts disagree

The new test requires LLM_TIMEOUT=300, while assert_strix_workflow_pr_trigger_hardened requires zero. The changed-path quality check runs both and fails.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae seonghobae reopened this Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by merged #1601, which landed the same verified two-file current-main Strix timeout repair on protected main (5f81d8e665b7d3f51f379a090e077486dbf548c5). Closing this duplicate to avoid redundant Actions fanout.

@seonghobae seonghobae closed this Sep 1, 2026
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.

1 participant