Skip to content

feat(tui): add dev server readiness tool#3376

Open
cyq1017 wants to merge 2 commits into
Hmbown:mainfrom
cyq1017:codex/codewhale-3360-dev-server-readiness
Open

feat(tui): add dev server readiness tool#3376
cyq1017 wants to merge 2 commits into
Hmbown:mainfrom
cyq1017:codex/codewhale-3360-dev-server-readiness

Conversation

@cyq1017

@cyq1017 cyq1017 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Addresses #3360.

Summary

  • add a wait_for_dev_server tool for loopback-only TCP/HTTP readiness checks
  • wire it into the TUI tool registry, catalog, approval risk, history, and tool card groups
  • reject non-loopback targets and URL credentials while canonicalizing localhost/loopback hosts

Verification

  • cargo test -p codewhale-tui --bin codewhale-tui --locked dev_server_readiness
  • cargo fmt -- --check
  • cargo check -p codewhale-tui --bin codewhale-tui --locked
  • git diff --check

Add a loopback-only wait_for_dev_server tool for local webapp automation. It polls TCP readiness and optional same-port HTTP healthchecks with bounded timeout output.

Verification:

- cargo test -p codewhale-tui --bin codewhale-tui --locked dev_server_readiness

- cargo test -p codewhale-tui --bin codewhale-tui --locked test_builder_with_web_tools_no_longer_includes_finance

- cargo test -p codewhale-tui --bin codewhale-tui --locked tool_names_route_to_families_by_verb

- cargo test -p codewhale-tui --bin codewhale-tui --locked risk_query_only_network_is_benign_but_fetch_is_destructive

- cargo test -p codewhale-tui --bin codewhale-tui --locked non_yolo_mode_retains_default_defer_policy

- cargo test -p codewhale-tui --bin codewhale-tui --locked agent_catalog_advertises_and_searches_core_action_tools

- cargo fmt -- --check

- git diff --check

@greptile-apps greptile-apps 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.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces a new local dev-server readiness tool (wait_for_dev_server) that polls a loopback TCP port and optionally an HTTP(S) health URL to determine if a local dev server is ready. The tool is integrated into the tool registry, approval rules, and UI widgets. The review feedback highlights a critical timeout starvation issue where individual HTTP and TCP poll attempts are capped at the short poll interval (default 250ms). This can cause slow-compiling dev servers to be repeatedly cancelled and retried, leading to false-positive failures. Increasing the individual attempt timeouts to 10 seconds for HTTP requests and 2 seconds for TCP connections is recommended.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +298 to +302
match run_until_deadline(
deadline,
request.poll_interval,
client.get(url.clone()).send(),
)

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.

high

Timeout Starvation / Retry Storm Bug

Using request.poll_interval (which defaults to 250ms) as the timeout for each HTTP request attempt is highly problematic.

Local dev servers (such as Vite, Next.js, or Webpack) are notorious for having a slow first request (often taking 1 to 5 seconds) because they lazily compile and bundle entrypoints on the first hit. If each poll attempt is strictly aborted after 250ms, the tool will continuously cancel the in-flight request and retry, never giving the dev server enough time to complete compilation and respond with a 200 OK. This results in persistent, false-positive readiness failures.

We should allow each HTTP request attempt to have a much larger timeout (e.g., 10 seconds), while still respecting the overall deadline via run_until_deadline's remaining.min(...) logic.

Suggested change
match run_until_deadline(
deadline,
request.poll_interval,
client.get(url.clone()).send(),
)
match run_until_deadline(
deadline,
Duration::from_secs(10),
client.get(url.clone()).send(),
)

Comment on lines +243 to +247
match run_until_deadline(
deadline,
request.poll_interval,
TcpStream::connect((request.host.as_str(), request.port)),
)

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.

medium

TCP Connection Timeout Starvation

Similarly to the HTTP request timeout, capping the TCP connection attempt to request.poll_interval (250ms) can be too restrictive under transient local network latency or firewall overhead.

Using a slightly larger, safer timeout (e.g., 2 seconds) for the TCP connection attempt ensures robustness while still respecting the overall deadline.

Suggested change
match run_until_deadline(
deadline,
request.poll_interval,
TcpStream::connect((request.host.as_str(), request.port)),
)
match run_until_deadline(
deadline,
Duration::from_secs(2),
TcpStream::connect((request.host.as_str(), request.port)),
)

@greptile-apps greptile-apps 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.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@cyq1017 cyq1017 marked this pull request as ready for review June 22, 2026 04:14
@cyq1017 cyq1017 requested a review from Hmbown as a code owner June 22, 2026 04:14

@greptile-apps greptile-apps 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.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@Hmbown

Hmbown commented Jun 22, 2026

Copy link
Copy Markdown
Owner

Thanks @cyq1017 — I carried this into the v0.8.64 integration branch with your commits preserved:

  • 9b8a7dfd4 feat(tui): add dev server readiness tool
  • d254d2a5e fix(tui): decouple readiness probe timeout

The only integration adjustment was resolving the history-rendering conflict against the current refactor: the readiness tool classification now lives in crates/tui/src/tui/history/tool_run.rs instead of the older inline history.rs block.

Verified on the integration branch with:

  • cargo test -p codewhale-tui --bin codewhale-tui --locked dev_server_readiness -- --nocapture
  • cargo test -p codewhale-tui --bin codewhale-tui --locked with_web_tools -- --nocapture
  • cargo test -p codewhale-tui --bin codewhale-tui --locked risk_query_only_network_is_benign_but_fetch_is_destructive -- --nocapture
  • cargo test -p codewhale-tui --bin codewhale-tui --locked detect_tool_runs_summarizes_safe_command_tools -- --nocapture
  • cargo test -p codewhale-tui --bin codewhale-tui --locked tool_names_route_to_families_by_verb -- --nocapture
  • RUSTFLAGS=-Dwarnings cargo check -p codewhale-tui --bin codewhale-tui --locked
  • cargo fmt --all -- --check
  • git diff --check
  • python3 scripts/check-coauthor-trailers.py --author-map .github/AUTHOR_MAP --range HEAD~2..HEAD --check-authors

I am keeping this draft PR open until the integration branch lands and #3360 can be closed against main.

@Hmbown

Hmbown commented Jun 22, 2026

Copy link
Copy Markdown
Owner

Thank you @cyq1017 — the dev-server readiness tool landed in the v0.8.64 release branch (crates/tui/src/tools/dev_server_readiness.rs, registered in the tool catalog). Tracked via #3360 (now closed as fixed). Great contribution!

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