Skip to content

fix(cli): honor configured remote timeout - #516

Merged
peteski22 merged 3 commits into
mozilla-ai:mainfrom
ShiroKSH:fix/cli-http-timeout
Aug 3, 2026
Merged

fix(cli): honor configured remote timeout#516
peteski22 merged 3 commits into
mozilla-ai:mainfrom
ShiroKSH:fix/cli-http-timeout

Conversation

@ShiroKSH

@ShiroKSH ShiroKSH commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Pass CQ_TIMEOUT to the Go SDK when constructing CLI clients.
  • Cover a remote response that exceeds the SDK default but completes within the configured CLI timeout.

Checks

  • go test ./...
  • go vet ./...
  • make lint

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where the CLI’s configured timeout was not consistently applied to remote query requests.
    • Queries can now wait for slower remote responses within the configured timeout period, reducing premature request failures.
  • Tests

    • Added coverage to verify that delayed remote responses are handled successfully when a suitable timeout is configured.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b4bac08e-27c0-4fa9-aba0-b234e48d5892

📥 Commits

Reviewing files that changed from the base of the PR and between eb1d485 and f5b37ec.

📒 Files selected for processing (2)
  • cli/cmd/cli.go
  • cli/cmd/query_test.go

Walkthrough

The CLI now applies its configured timeout when it creates the SDK client. An integration test confirms that a delayed query response succeeds within the configured timeout.

Changes

CLI timeout configuration

Layer / File(s) Summary
Wire and validate CLI timeout
cli/cmd/cli.go, cli/cmd/query_test.go
newCLIClient passes cliTimeout() to the SDK client. An integration test verifies that a delayed remote response is returned within the configured timeout.

Suggested reviewers: peteski22

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the CLI fix to honour the configured remote timeout.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

ShiroKSH and others added 3 commits August 3, 2026 16:33
Link the configured CQ_TIMEOUT and the handler sleep to a single source
and assert the sleep stays below the configured timeout, so a future
change to the SDK default fails loudly here instead of flaking silently.

@peteski22 peteski22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🙏🏼

@peteski22
peteski22 merged commit a47f0af into mozilla-ai:main Aug 3, 2026
3 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