fix(cli): reject a malformed DORA_COORDINATOR_ADDR/PORT in dora up

[phil-opp]

Summary

dora up is the only place in the CLI that parses the coordinator address and port env vars by hand:

let addr = std::env::var("DORA_COORDINATOR_ADDR")
    .ok().and_then(|s| s.parse().ok()).unwrap_or(LOCALHOST);
let port = std::env::var("DORA_COORDINATOR_PORT")
    .ok().and_then(|s| s.parse().ok()).unwrap_or(DORA_COORDINATOR_PORT_WS_DEFAULT);

The .and_then(|s| s.parse().ok()) swallows a parse error, so a malformed value silently falls back to the default endpoint:

• DORA_COORDINATOR_PORT=70000 (out of u16 range) → binds the default port
• DORA_COORDINATOR_PORT=6O12 (letter O typo) → binds the default port
• DORA_COORDINATOR_ADDR=<not-an-ip> → binds loopback

Every other subcommand routes the same env vars through clap's typed value_parser (see common.rs, daemon.rs, build/mod.rs, coordinator.rs), which hard-errors on a bad value. So the next dora start / dora list / dora stop in the same workflow rejects the value up just ignored — or connects to the default port and reports the coordinator as "not running". The two halves of a single workflow disagree about where the coordinator lives, with no diagnostic.

Fix

Parse both vars strictly via two small helpers that surface a contextual error (invalid DORA_COORDINATOR_ADDR …``), falling back to the default only when the var is unset — matching clap's behavior in the sibling subcommands. The helpers take an Option<String> (rather than reading the env internally) so the parse logic is unit-testable without mutating process env.

Note: a var set to an empty string now hard-errors, which is intentional — clap's env= in the other subcommands rejects an empty value too, so this keeps up consistent with them (treating empty as "unset" would reintroduce the very divergence this fixes).

Validation

• Added unit tests for the absent (→ default), valid, and malformed (→ error naming the offending variable, both out-of-range and non-numeric) cases.
• cargo fmt -p dora-cli -- --check, cargo clippy -p dora-cli -- -D warnings (the CI gate command), and cargo test -p dora-cli coordinator_env_tests pass locally on rustc 1.97.1. Only up.rs is touched.

---

⚠️ This is a machine-generated pull request, authored by Claude (Claude Code). It has been verified locally as described above but should receive human review before merge.

🤖 Generated with Claude Code

---

Generated by Claude Code

[trunk-io]

Merging to main in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[phil-opp]

Automated review by Claude (fully automated; no human in the loop)

No issues found. The change correctly replaces the error-swallowing .and_then(|s| s.parse().ok()).unwrap_or(default) with strict helpers that only fall back to the default when the variable is unset and otherwise surface a contextual hard error, matching the clap-parsed behavior of the other subcommands. This closes the real split-brain case where dora up silently binds the default endpoint while a later dora start/list rejects the same env value. The unit tests call the extracted helpers directly and cover the absent, valid, out-of-u16-range, and non-numeric cases, so they exercise the actual code path rather than a mock. Behavior does become stricter (a previously-ignored malformed value now errors), but that is the intended fix and is consistent with the rest of the CLI.

---

Generated by Claude Code

[phil-opp]

Automated review by Claude (fully automated; no human in the loop)

This change looks to have been overtaken by #3100 ("fix(cli): reject a malformed coordinator address/port in dora up"), which merged to main after this PR was opened and implements the same fix. main's binaries/cli/src/command/up.rs now has a generic parse_coordinator_env(var_name, raw: Option<&str>, default) helper (plus coordinator_env_value) and its own coordinator_env_tests module in the same spot this PR touches. That collision is why the PR is currently in a conflicting / needs-rebase state — the - side of this diff (the old .and_then(|s| s.parse().ok()).unwrap_or(default)) no longer exists on main.

The semantics of the two approaches match: unset → default, malformed → hard error naming the offending variable, empty string → error (consistent with clap's env= on the sibling subcommands). The parsing in this diff is itself correct (empty, IPv6, out-of-u16, and non-numeric all handled), and the tests exercise the real helpers — so no issues with the code itself.

Given #3100 already landed the same behavior, this PR looks redundant. I'd suggest closing it, unless the two-typed-helper form here is preferred over the merged generic helper — in which case it needs a rebase that reconciles the duplicate coordinator_env_tests module and the now-removed original code.

---

Generated by Claude Code