Skip to content

Conversation

@endolith
Copy link
Contributor

@endolith endolith commented Oct 26, 2025

Describe the changes you have made:

Fix infinite WebSocket loops in test_server causing CI timeouts

Replace 6 infinite while True loops with retry-limited helper function.

PROBLEM:

SOLUTION:

  • Add wait_for_websocket_complete() helper function with max 5 attempts
  • Replace all 6 infinite loops with calls to helper function
  • Fail fast with descriptive error messages instead of hanging

This prevents 6-hour CI timeouts and provides clear debugging information
when WebSocket communication fails.

Reference any relevant issues (e.g. "Fixes #000"):

Pre-Submission Checklist (optional but appreciated):

  • I have included relevant documentation updates (stored in /docs)
  • I have read docs/CONTRIBUTING.md
  • I have read docs/ROADMAP.md

OS Tests (optional but appreciated):

  • Tested on Windows
  • Tested on MacOS
  • Tested on Linux

(to make subsequent commits easier to read)
Replace 6 infinite while True loops with retry-limited helper function.

PROBLEM:
- GitHub Actions tests were timing out after 6 hours on PRs
- PRs don't have access to GitHub secrets(?), causing authentication failures
- test_server() contained 6 infinite while True loops which waited for 'complete' status messages that never arrived
- Server never sends 'complete' status on auth failure, causing indefinite hangs
- No retry limits or timeouts caused indefinite blocking

SOLUTION:
- Add wait_for_websocket_complete() helper function with max 5 attempts
- Replace all 6 infinite loops with calls to helper function
- Fail fast with descriptive error messages instead of hanging

This prevents 6-hour CI timeouts and provides clear debugging information
when WebSocket communication fails.
@endolith endolith changed the title Classic/infinite tests Fix infinite WebSocket loops in test_server causing 6 hour CI timeouts Oct 26, 2025
@endolith
Copy link
Contributor Author

(Ironically this PR triggers the same problem: https://github.com/openinterpreter/open-interpreter/actions/runs/18819992234 😅)

@Notnaton
Copy link
Collaborator

Should we nuke the tests?
It needs an API key for the tests anyways so nothing will run.

@endolith
Copy link
Contributor Author

endolith commented Nov 3, 2025

They are running once it's merged to main: https://github.com/openinterpreter/open-interpreter/actions/runs/19034913261

So Killian must have put API keys into GitHub secrets, but feature branches don't have access to the secrets for security reasons, if I understand correctly. (So people can't submit PRs that steal the secrets. AI told me it's possible to set it up for manual triggering, too, like you could press a button to test with secrets after confirming the PR code is safe to access them.)

I also had some changes that skip API tests for feature branches, but I'm not sure of best practice there. But with this PR those should fail quickly at least.

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