fix(cli): respect SURFPOOL_STUDIO_HOST environment variable for server binding

[Amit5601]

fixes #588
This resolves network binding restrictions when running the Studio backend within containerized environments (like Docker) or isolated local network architectures where an explicit host override is required.

@MicaiahReid @lgalabru Re-opening this cleanly.

[greptile-apps]

Greptile Summary

This PR makes the Studio HTTP server respect the SURFPOOL_STUDIO_HOST environment variable for its bind address, fixing a usability gap in Docker and other isolated-network environments where the default network_binding value is not reachable.

• The env-var resolution logic handles IPv4, bracketed and bare IPv6, and host-with-port forms, then falls back to the original network_binding when the variable is unset.
• Two robustness gaps exist: the hardcoded "18488" fallback for default_port is unreachable (a split on a non-empty string always produces Some), and the count == 1 arm passes through values like "myhost:" verbatim, causing a silent bind failure with no reference to the env var.

Confidence Score: 4/5

The core change is narrow and the common Docker use-case (plain IPv4 host override) works correctly; the edge-case issues both result in an explicit bind error rather than silent misbehavior, and the default (no env var) path is completely unaffected.

The address-resolution logic is mostly correct for the intended scenarios. The unreachable fallback and the port-less single-colon passthrough are real but non-critical gaps — both cause a startup error rather than wrong runtime behavior.

crates/cli/src/http/mod.rs — specifically the default_port extraction and the count == 1 branch of the env-var logic.

Important Files Changed

Filename	Overview
crates/cli/src/http/mod.rs	Adds SURFPOOL_STUDIO_HOST env-var resolution before binding the Actix HTTP server; IPv6/IPv4 detection logic is mostly sound but the default-port fallback is unreachable and the single-colon passthrough can silently produce a port-less address.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["start_studio_and_scenario_server(network_binding)"] --> B["Extract default_port\nfrom network_binding"]
    B --> C{"SURFPOOL_STUDIO_HOST\nenv var set?"}
    C -- "No (VarError)" --> D["Use network_binding\nas bind address"]
    C -- "Yes → host" --> E{"host contains ']'?\n(bracketed IPv6)"}
    E -- "Yes" --> F{"rfind(':') > rfind(']')?\n(port already after bracket)"}
    F -- "Yes" --> G["Use host as-is"]
    F -- "No" --> H["Append default_port:\nhost:default_port"]
    E -- "No" --> I{"Count ':' in host"}
    I -- "0" --> J["host:default_port"]
    I -- "1" --> K["Use host as-is\n⚠ no port validation"]
    I -- "2+" --> L["[host]:default_port\n(bare IPv6 wrapping)"]
    G --> M["final_bind_addr"]
    H --> M
    J --> M
    K --> M
    L --> M
    D --> M
    M --> N[".bind(final_bind_addr)"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["start_studio_and_scenario_server(network_binding)"] --> B["Extract default_port\nfrom network_binding"]
    B --> C{"SURFPOOL_STUDIO_HOST\nenv var set?"}
    C -- "No (VarError)" --> D["Use network_binding\nas bind address"]
    C -- "Yes → host" --> E{"host contains ']'?\n(bracketed IPv6)"}
    E -- "Yes" --> F{"rfind(':') > rfind(']')?\n(port already after bracket)"}
    F -- "Yes" --> G["Use host as-is"]
    F -- "No" --> H["Append default_port:\nhost:default_port"]
    E -- "No" --> I{"Count ':' in host"}
    I -- "0" --> J["host:default_port"]
    I -- "1" --> K["Use host as-is\n⚠ no port validation"]
    I -- "2+" --> L["[host]:default_port\n(bare IPv6 wrapping)"]
    G --> M["final_bind_addr"]
    H --> M
    J --> M
    K --> M
    L --> M
    D --> M
    M --> N[".bind(final_bind_addr)"]

Loading

_{Reviews (1): Last reviewed commit: "fix(cli): respect SURFPOOL_STUDIO_HOST e..." | Re-trigger Greptile}

[greptile-apps]

No log when env var overrides the bind address

When SURFPOOL_STUDIO_HOST is set, the resolved bind address is silently substituted for network_binding with no log output. In Docker/containerised environments — exactly the use-case this PR targets — a missing or mis-typed env var will leave operators with no indication of which address the server actually tried to bind on. Adding an info!("Binding studio server to {} (from SURFPOOL_STUDIO_HOST)", final_bind_addr) line before the HttpServer::new(...) call would make this diagnosable without a debugger.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

unwrap_or("18488") is unreachable — split on a non-empty string always yields at least one element

str::split on a non-empty network_binding string always returns at least one element, so .last() always returns Some(_), meaning .unwrap_or("18488") can never activate. If network_binding carries no colon (e.g. a bare hostname), default_port becomes the whole network_binding string and the formatted address will be malformed. Using rsplit_once makes the intent explicit and provides a real fallback.

Suggested change

	let default_port = network_binding.split(':').last().unwrap_or("18488");
	let default_port = network_binding
	.rsplit_once(':')
	.map(|(_, port)| port)
	.unwrap_or("18488");

[greptile-apps]

Verbatim passthrough for the count == 1 case doesn't guard against a missing port

When SURFPOOL_STUDIO_HOST contains exactly one colon (e.g. "myhost:" or ":") the value is returned as-is. actix-web will then fail at bind() with a generic "invalid address" error that gives no hint the env var is to blame. Verifying that the part after the colon is non-empty would surface the mis-configuration before the bind attempt.

Suggested change

	match host.matches(':').count() {
	0 => format!("{}:{}", host, default_port),
	1 => host,
	_ => format!("[{}]:{}", host, default_port),
	}
	match host.matches(':').count() {
	0 => format!("{}:{}", host, default_port),
	1 => {
	if host.rsplit_once(':').map_or(false, |(_, p)| !p.is_empty()) {
	host
	} else {
	format!("{}:{}", host.trim_end_matches(':'), default_port)
	}
	}
	_ => format!("[{}]:{}", host, default_port),
	}