Skip to content

Conversation

@zemaj
Copy link

@zemaj zemaj commented Jan 13, 2026

Summary

  • /browser or /chrome not working #508: keep CDP browser sessions alive by treating chromiumoxide oneshot cancellations as non-fatal, and ignore proxy env vars during localhost CDP /json/version discovery.
  • /diff is having off by one space err #473: strip the context prefix space when parsing diffs so /diff context lines align correctly.
  • Add regression coverage for proxy env vs CDP discovery.

Tests

  • cargo test -p code-browser --manifest-path code-rs/Cargo.toml --lib
  • cargo test -p code-tui --manifest-path code-rs/Cargo.toml --lib history_cell::diff::tests::diff_record_strips_context_prefix_space
  • ./build-fast.sh

Fixes #508
Fixes #473

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 55adf2878b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 172 to 175
let client_start = tokio::time::Instant::now();
let client = Client::builder()
.no_proxy()
.timeout(Duration::from_secs(5)) // Allow Chrome time to bring up /json/version on fresh launch

Choose a reason for hiding this comment

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

P2 Badge Preserve proxies for non-local CDP discovery

Calling Client::builder().no_proxy() here disables all proxy use for discover_ws_via_host_port, not just localhost. If a user sets connect_host to a remote Chrome endpoint that is only reachable via HTTP(S)_PROXY (common in corporate networks), discovery will now bypass the proxy and fail to reach /json/version. The change is intended for localhost, but it regresses remote/proxied CDP setups by forcing direct connections.

Useful? React with 👍 / 👎.

@zemaj zemaj merged commit bfd1533 into main Jan 14, 2026
15 checks passed
@zemaj zemaj deleted the fix/issues-508-473 branch January 14, 2026 00:33
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.

/browser or /chrome not working /diff is having off by one space err

2 participants