app-server-test-client websocket client and thread tools#11755
Conversation
maxj-oai
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4ace3d3de
ℹ️ 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".
| let url = Url::parse(listen).with_context(|| format!("invalid --listen URL `{listen}`"))?; | ||
| let port = url | ||
| .port_or_known_default() | ||
| .with_context(|| format!("unable to infer port from --listen URL `{listen}`"))?; |
There was a problem hiding this comment.
When serve --kill is used, this code extracts a port via Url::port_or_known_default() and kills all listeners on that port before the app-server validates --listen; for malformed/unsupported values like ws://127.0.0.1 or http://127.0.0.1:4222, this can terminate unrelated services (for example port 80/443 defaults) and then fail to start anyway. The app-server transport only accepts ws://IP:PORT (AppServerTransport::from_listen_url), so kill should run only after that stricter validation succeeds.
Useful? React with 👍 / 👎.
| fn connect(endpoint: &Endpoint, config_overrides: &[String]) -> Result<Self> { | ||
| match endpoint { | ||
| Endpoint::SpawnCodex(codex_bin) => Self::spawn_stdio(codex_bin, config_overrides), | ||
| Endpoint::ConnectWs(url) => Self::connect_websocket(url), | ||
| } |
There was a problem hiding this comment.
--config is used with websocket endpoint
In websocket mode, --config values are silently dropped because connect() forwards config_overrides only to stdio spawn and ignores them for Endpoint::ConnectWs; this makes CLI runs look configured while they are actually testing the remote server’s existing config, which can invalidate manual experiments. Since the CLI still advertises --config as a global forwarding option, this path should error or explicitly handle overrides instead of accepting a no-op.
Useful? React with 👍 / 👎.
2 participants
Uh oh!
There was an error while loading. Please reload this page.