fix: Identify broken network processes - #678
Conversation
There was a problem hiding this comment.
Pull request overview
Adds gateway health probing so network start can replace defunct managed-network processes.
Changes:
- Probes the gateway status endpoint with a timeout.
- Stops unresponsive launchers and cleans stale descriptors.
- Adds an integration test for defunct launcher recovery.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
crates/icp/src/network/config.rs |
Adds gateway responsiveness probing and errors. |
crates/icp-cli/src/commands/network/start.rs |
Restarts alive but unresponsive networks. |
crates/icp-cli/tests/network_tests.rs |
Tests defunct launcher replacement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
marc0olo
left a comment
marc0olo
left a comment
There was a problem hiding this comment.
LGTM — reviewed the logic (port-freeing via stop_launcher, descriptor-write-after-init so booting networks can't be misclassified) and the test. Solid, well-documented, and conventioned.
One small note, assuming it's a conscious decision: is_responsive() only maps connect/timeout errors to Ok(false), so any other probe error (e.g. a malformed HTTP response, or DNS failure for a non-localhost host) makes network start abort rather than fall through to recovery. That's clearly deliberate per the doc comment ("leaves the question unanswered"), just flagging it since the whole point of the feature is to recover from a wedged launcher. Happy either way.
Fixes #577.