fix(tui): probe loopback CDP discovery endpoints - #26865
Open
seilk wants to merge 1 commit into
Open
Conversation
Contributor
|
Thanks for the focused reproducer and regression test. The split-listener premise remains valid on current main: Problems
Suggested changes
Automated hermes-sweeper review. |
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
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.
What does this PR do?
Fixes #26864.
This fixes default-local TUI
/browser connectwhen the valid Chrome CDP endpoint is bound on IPv6 loopback ([::1]:9222) while IPv4 loopback (127.0.0.1:9222) has a listener that is not a usable CDP discovery endpoint.The change makes the TUI gateway validate default-local loopback candidates by probing Chrome's CDP discovery endpoints and persisting the first candidate that actually serves
/json/versionor/json.Related Issue
Fixes #26864
Related but non-duplicative upstream work checked before opening this PR:
localhostto127.0.0.1for IPv6-first localhost failures. This PR handles the inverse case./browser connectlocal CDP handling.Type of Change
Changes Made
tui_gateway/server.py::1as a default local CDP host.127.0.0.1,::1, andlocalhost, preserving the requested host first./json/versionor/jsonendpoint responds successfully.tests/test_tui_gateway_server.py127.0.0.1:9222is not CDP but[::1]:9222is the valid CDP endpoint.localhostconnects preserve the requested URL and launch-retry tests still exercise the intended retry path.How to Test
Reproduction observed before the fix:
9222on macOS.curl -i http://127.0.0.1:9222/json/version # HTTP/1.1 404 Not Found/browser connectthrough the TUI browser manage path.http://[::1]:9222when that is the working endpoint.Local validation run:
I also ran
graphify update .per local repo guidance, but graph visualization generation failed because the graph is too large for HTML output. It did not affect the code/test changes.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passNote: I ran the targeted TUI gateway suite (
178 passed), not the full repository test suite.Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
Observed before fix: