[Detail Bug] CLI: Endpoint URL parse errors incorrectly blame environment variables even when configured via config file

[detail-app]

Detail Bug Report

https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/bugs/bug_3c71a34c-de43-41e3-822a-ad480c9193eb

Introduced in #103 by @infiniteregrets on Jan 28, 2026

Summary

• Context: When endpoints are set via the config file (s2 config set account_endpoint X), and endpoint parsing fails, the error message incorrectly states "Unable to load S2 endpoints from environment" and asks users to check environment variables they never set.
• Bug: When endpoints are set via the config file (s2 config set account_endpoint X), and endpoint parsing fails, the error message incorrectly states "Unable to load S2 endpoints from environment" and asks users to check environment variables they never set.
• Actual vs. expected: User sets endpoint via s2 config set account_endpoint "https//a.s2.dev" (missing colon). Running any command produces an error directing them to check S2_ACCOUNT_ENDPOINT environment variable — which they never set. Expected: Error should not specify "from environment" since endpoints can come from config file. Help text should mention both config file and environment variables.
• Impact: Misleading error messages send users debugging in the wrong direction, wasting time checking environment variables that aren't set.

Code with Bug

// cli/src/config.rs
match (&config.account_endpoint, &config.basin_endpoint) {
    (Some(account_endpoint_str), Some(basin_endpoint_str)) => {
        let account_endpoint = AccountEndpoint::new(account_endpoint_str)
            .map_err(|e| CliError::EndpointsFromEnv(e.to_string()))?; // <-- BUG 🔴 misattributes parse failures to env vars even when values came from config file
        ...
    }
}

Explanation

load_cli_config() merges config-file and environment sources into a single CliConfig via the config crate. By the time sdk_config() parses account_endpoint/basin_endpoint, the original source (file vs env) is no longer known. However, parse failures are always wrapped in CliError::EndpointsFromEnv, which hardcodes the error message and help text to “from environment” and “Are you overriding S2_ACCOUNT_ENDPOINT…”, producing a factually incorrect message when the invalid value came from the config file.

Codebase Inconsistency

The config loader explicitly supports both sources:

// cli/src/config.rs
pub fn load_cli_config() -> Result<CliConfig, CliConfigError> {
    if path.exists() {
        builder = builder.add_source(config::File::new(...));  // File source
    }
    builder = builder.add_source(config::Environment::with_prefix("S2"));  // Env source
    Ok(builder.build()?.try_deserialize::<CliConfig>()?)  // Merges into single struct
}

Yet the error variant used during endpoint parsing assumes env-only.

Recommended Fix

Replace CliError::EndpointsFromEnv with a source-agnostic error (e.g., EndpointsInvalid) and update the help text to instruct users to check both the config file path (via existing config_path()) and S2_ACCOUNT_ENDPOINT / S2_BASIN_ENDPOINT environment variables.

History

This bug was introduced in commit dc3bd4e.