diff --git a/src/api/client.rs b/src/api/client.rs index 59571bf8..aec203ec 100644 --- a/src/api/client.rs +++ b/src/api/client.rs @@ -8,7 +8,8 @@ use crate::analysis::static_analyzer::registry::all_framework_names; use crate::analysis::types::{FrameworkCategory, Language, RepoAnalysis}; use crate::api::types::{ AdvisorPoll, AdvisorQueryRequest, AdvisorQueryStarted, AdvisorQueryStatus, ApiErrorResponse, - CanonicalIRFacet, CanonicalIRPayload, CategoriesResponse, FrameworksResponse, HealthResponse, + CanonicalIRFacet, CanonicalIRPayload, CategoriesResponse, ConnectedReposErrorBody, + ConnectedRepository, FrameworksResponse, GetConnectedReposResponse, HealthResponse, LanguagesResponse, MatchFramework, MatchOptions, MatchProject, MatchRequest, MatchResponse, TelemetryRequest, }; @@ -177,6 +178,53 @@ impl ActualApiClient { }) } + /// List the repositories connected to `org_id` (`GET /v1/connected-repos`). + /// Lets a caller resolve a repository name to its `repo_unique_id`. The + /// bearer token's own organization must match `org_id`; the server hides + /// other organizations behind a 404 rather than revealing their existence. + pub async fn list_connected_repos( + &self, + org_id: &str, + ) -> Result, ActualError> { + let url = format!("{}/v1/connected-repos", self.base_url); + let response = self + .authed(self.client.get(&url).query(&[("org_id", org_id)])) + .send() + .await + .map_err(|e| ActualError::ApiError(e.to_string()))?; + let status = response.status(); + if status == reqwest::StatusCode::UNAUTHORIZED { + return Err(ActualError::NotLoggedIn); + } + if status == reqwest::StatusCode::FORBIDDEN { + return Err(Self::forbidden_org_error()); + } + if !status.is_success() { + return Err(Self::map_connected_repos_error(status, response).await); + } + response + .json::() + .await + .map(|body| body.repositories) + .map_err(|e| ActualError::ApiError(e.to_string())) + } + + /// Map a non-success connected-repos response to an error. This endpoint + /// returns a flat `{error, message}` body (unlike the advisor routes' nested + /// `{error:{code,message}}`), so surface its `message` directly, falling back + /// to the status line when the body is absent, empty, or unparseable. + async fn map_connected_repos_error( + status: reqwest::StatusCode, + response: reqwest::Response, + ) -> ActualError { + match response.json::().await { + Ok(body) if !body.message.is_empty() => { + ActualError::ApiError(format!("HTTP {status}: {}", body.message)) + } + _ => ActualError::ApiError(format!("HTTP {status}")), + } + } + async fn get(&self, path: &str) -> Result { let url = format!("{}{}", self.base_url, path); let response = self @@ -1838,4 +1886,213 @@ mod tests { ); mock.assert_async().await; } + + // --- Connected-repos client tests --- + + const CONNECTED_REPOS_BODY: &str = r#"{ + "repositories": [ + { + "repo_unique_id": "33333333-3333-3333-3333-333333333333", + "name": "actual-cli", + "external_owner": "actual-software", + "url": "https://github.com/actual-software/actual-cli" + }, + { + "repo_unique_id": "44444444-4444-4444-4444-444444444444", + "name": "web-app", + "external_owner": "actual-software", + "url": "https://github.com/actual-software/web-app" + } + ] + }"#; + + #[tokio::test] + async fn test_list_connected_repos_success_sends_bearer_and_org_id() { + let mut server = mockito::Server::new_async().await; + let org = "11111111-1111-1111-1111-111111111111"; + let mock = server + .mock("GET", "/v1/connected-repos") + .match_header("authorization", "Bearer tok-123") + .match_query(mockito::Matcher::UrlEncoded("org_id".into(), org.into())) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(CONNECTED_REPOS_BODY) + .create_async() + .await; + let client = ActualApiClient::new(&server.url()) + .unwrap() + .with_bearer("tok-123"); + let repos = client.list_connected_repos(org).await.unwrap(); + assert_eq!(repos.len(), 2); + assert_eq!(repos[0].name, "actual-cli"); + assert_eq!( + repos[0].repo_unique_id, + "33333333-3333-3333-3333-333333333333" + ); + assert_eq!(repos[0].external_owner, "actual-software"); + assert_eq!(repos[1].name, "web-app"); + mock.assert_async().await; + } + + #[tokio::test] + async fn test_list_connected_repos_empty_list() { + let mut server = mockito::Server::new_async().await; + let mock = server + .mock("GET", "/v1/connected-repos") + .match_query(mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"repositories":[]}"#) + .create_async() + .await; + let client = ActualApiClient::new(&server.url()) + .unwrap() + .with_bearer("t"); + let repos = client + .list_connected_repos("11111111-1111-1111-1111-111111111111") + .await + .unwrap(); + assert!(repos.is_empty()); + mock.assert_async().await; + } + + #[tokio::test] + async fn test_list_connected_repos_unauthorized_maps_to_not_logged_in() { + let mut server = mockito::Server::new_async().await; + let mock = server + .mock("GET", "/v1/connected-repos") + .match_query(mockito::Matcher::Any) + .with_status(401) + .with_body(r#"{"error":"unauthorized","message":"authentication required"}"#) + .create_async() + .await; + let client = ActualApiClient::new(&server.url()) + .unwrap() + .with_bearer("bad"); + let err = client.list_connected_repos("org").await.unwrap_err(); + assert!(matches!(err, ActualError::NotLoggedIn)); + mock.assert_async().await; + } + + #[tokio::test] + async fn test_list_connected_repos_forbidden_maps_to_org_mismatch() { + let mut server = mockito::Server::new_async().await; + let mock = server + .mock("GET", "/v1/connected-repos") + .match_query(mockito::Matcher::Any) + .with_status(403) + .with_body(r#"{"error":"forbidden","message":"cross-org"}"#) + .create_async() + .await; + let client = ActualApiClient::new(&server.url()) + .unwrap() + .with_bearer("t"); + let err = client.list_connected_repos("org").await.unwrap_err(); + assert!( + matches!(err, ActualError::OrgMismatch { .. }), + "expected OrgMismatch for 403, got {err:?}" + ); + mock.assert_async().await; + } + + #[tokio::test] + async fn test_list_connected_repos_not_found_surfaces_flat_message() { + let mut server = mockito::Server::new_async().await; + // The connected-repos endpoint returns a flat {error, message} body; the + // human-readable message is surfaced (map_connected_repos_error message arm). + let mock = server + .mock("GET", "/v1/connected-repos") + .match_query(mockito::Matcher::Any) + .with_status(404) + .with_header("content-type", "application/json") + .with_body( + r#"{"error":"not_found","message":"organization not found or not accessible"}"#, + ) + .create_async() + .await; + let client = ActualApiClient::new(&server.url()) + .unwrap() + .with_bearer("t"); + let err = client.list_connected_repos("org").await.unwrap_err(); + assert!( + matches!(err, ActualError::ApiError(ref m) if m.contains("404") && m.contains("organization not found")), + "got: {err:?}" + ); + mock.assert_async().await; + } + + #[tokio::test] + async fn test_list_connected_repos_empty_message_falls_back_to_status() { + let mut server = mockito::Server::new_async().await; + // A parseable body with a blank message → the guard is false, so the + // status-only fallback arm is taken. + let mock = server + .mock("GET", "/v1/connected-repos") + .match_query(mockito::Matcher::Any) + .with_status(400) + .with_header("content-type", "application/json") + .with_body(r#"{"error":"bad_request","message":""}"#) + .create_async() + .await; + let client = ActualApiClient::new(&server.url()) + .unwrap() + .with_bearer("t"); + let err = client.list_connected_repos("org").await.unwrap_err(); + assert!( + matches!(err, ActualError::ApiError(ref m) if m.contains("400")), + "got: {err:?}" + ); + mock.assert_async().await; + } + + #[tokio::test] + async fn test_list_connected_repos_unparseable_error_body_falls_back_to_status() { + let mut server = mockito::Server::new_async().await; + // A non-JSON error body → the parse fails and the status-only fallback + // arm is taken (map_connected_repos_error catch-all). + let mock = server + .mock("GET", "/v1/connected-repos") + .match_query(mockito::Matcher::Any) + .with_status(500) + .with_body("boom") + .create_async() + .await; + let client = ActualApiClient::new(&server.url()) + .unwrap() + .with_bearer("t"); + let err = client.list_connected_repos("org").await.unwrap_err(); + assert!( + matches!(err, ActualError::ApiError(ref m) if m.contains("500")), + "got: {err:?}" + ); + mock.assert_async().await; + } + + #[tokio::test] + async fn test_list_connected_repos_success_unparseable_body_is_api_error() { + let mut server = mockito::Server::new_async().await; + let mock = server + .mock("GET", "/v1/connected-repos") + .match_query(mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body("not valid json") + .create_async() + .await; + let client = ActualApiClient::new(&server.url()) + .unwrap() + .with_bearer("t"); + let err = client.list_connected_repos("org").await.unwrap_err(); + assert!(matches!(err, ActualError::ApiError(_)), "got: {err:?}"); + mock.assert_async().await; + } + + #[tokio::test] + async fn test_list_connected_repos_network_error() { + let client = ActualApiClient::new("http://127.0.0.1:1") + .unwrap() + .with_bearer("t"); + let err = client.list_connected_repos("org").await.unwrap_err(); + assert!(matches!(err, ActualError::ApiError(_))); + } } diff --git a/src/api/types.rs b/src/api/types.rs index 0cbd1b22..9d9f3e77 100644 --- a/src/api/types.rs +++ b/src/api/types.rs @@ -172,6 +172,39 @@ pub struct HealthResponse { pub version: String, } +// --- Connected-repos types (GET /v1/connected-repos) --- + +/// One repository connected to an organization. Fields are snake_case to match +/// the other CLI-facing (advisor) response bodies. Lets a client map a +/// repository `name` (or `external_owner`/`name`) to its `repo_unique_id`. +/// Extra server fields are ignored. +#[derive(Debug, Clone, PartialEq, Deserialize)] +pub struct ConnectedRepository { + pub repo_unique_id: String, + pub name: String, + pub external_owner: String, + #[serde(default)] + pub url: String, +} + +/// Response body for `GET /v1/connected-repos`. `repositories` defaults to empty +/// when the org has none connected. +#[derive(Debug, Clone, PartialEq, Deserialize)] +pub struct GetConnectedReposResponse { + #[serde(default)] + pub repositories: Vec, +} + +/// Flat error body returned by `GET /v1/connected-repos` (`{error, message}`), +/// distinct from the advisor routes' nested [`ApiErrorResponse`]. Only the +/// human-readable `message` is consumed (the sibling `error` code is ignored); +/// it defaults to empty so a partial or unexpected body still deserializes. +#[derive(Debug, Clone, PartialEq, Deserialize)] +pub struct ConnectedReposErrorBody { + #[serde(default)] + pub message: String, +} + // --- Telemetry types (POST /counter/record) --- #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -1068,4 +1101,68 @@ mod tests { assert_eq!(out.interpreter.related_adrs[0].url, None); assert_eq!(out.interpreter.related_adrs[1].url, None); } + + #[test] + fn test_connected_repos_response_deserializes_all_fields() { + let json = r#"{ + "repositories": [ + { + "repo_unique_id": "33333333-3333-3333-3333-333333333333", + "name": "actual-cli", + "external_owner": "actual-software", + "url": "https://github.com/actual-software/actual-cli" + } + ] + }"#; + let resp: GetConnectedReposResponse = serde_json::from_str(json).unwrap(); + assert_eq!(resp.repositories.len(), 1); + let repo = &resp.repositories[0]; + assert_eq!(repo.repo_unique_id, "33333333-3333-3333-3333-333333333333"); + assert_eq!(repo.name, "actual-cli"); + assert_eq!(repo.external_owner, "actual-software"); + assert_eq!(repo.url, "https://github.com/actual-software/actual-cli"); + } + + #[test] + fn test_connected_repository_missing_url_defaults_empty() { + // `url` is never read by name resolution, so a server body that omits + // (or renames) it must still deserialize rather than fail the whole + // response — the `#[serde(default)]` path. + let json = r#"{ + "repositories": [ + { + "repo_unique_id": "33333333-3333-3333-3333-333333333333", + "name": "actual-cli", + "external_owner": "actual-software" + } + ] + }"#; + let resp: GetConnectedReposResponse = serde_json::from_str(json).unwrap(); + assert_eq!(resp.repositories.len(), 1); + assert_eq!(resp.repositories[0].name, "actual-cli"); + assert_eq!(resp.repositories[0].url, ""); + } + + #[test] + fn test_connected_repos_response_missing_repositories_defaults_empty() { + // A body without `repositories` deserializes to an empty list (the + // `#[serde(default)]` path), not an error. + let resp: GetConnectedReposResponse = serde_json::from_str("{}").unwrap(); + assert!(resp.repositories.is_empty()); + } + + #[test] + fn test_connected_repos_error_body_reads_message() { + let body: ConnectedReposErrorBody = + serde_json::from_str(r#"{"error":"not_found","message":"organization not found"}"#) + .unwrap(); + assert_eq!(body.message, "organization not found"); + } + + #[test] + fn test_connected_repos_error_body_missing_message_defaults_empty() { + // Absent `message` (and an ignored, unknown `error` code) defaults to "". + let body: ConnectedReposErrorBody = serde_json::from_str("{}").unwrap(); + assert_eq!(body.message, ""); + } } diff --git a/src/cli/args.rs b/src/cli/args.rs index 78d3d34e..be2e2082 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -360,10 +360,13 @@ pub struct AdvisorArgs { #[arg(long, value_name = "ORG_ID")] pub org: Option, - /// Scope the query to a specific connected repository (UUID). When omitted, - /// the query runs at org level. Auto-detection from the git remote is not - /// available yet — it requires the connected-repos API. - #[arg(long, value_name = "REPO_ID")] + /// Scope the query to a specific connected repository, given either as its + /// name (e.g. `actual-cli`, or `owner/actual-cli` to disambiguate a name + /// shared across owners) or as its UUID. A name is resolved to a repo id via + /// the connected-repos API; an unrecognized name fails with the list of + /// repositories you can choose from. When omitted, the query runs at org + /// level. + #[arg(long, value_name = "REPO")] pub repo: Option, } diff --git a/src/cli/commands/advisor.rs b/src/cli/commands/advisor.rs index fd2cb7e5..a388d07c 100644 --- a/src/cli/commands/advisor.rs +++ b/src/cli/commands/advisor.rs @@ -9,8 +9,11 @@ use std::time::{Duration, Instant}; use chrono::{Duration as ChronoDuration, Utc}; +use uuid::Uuid; + use crate::api::types::{ AdvisorJobStatus, AdvisorOutput, AdvisorPoll, AdvisorQueryRequest, AdvisorSink, AdvisorSurface, + ConnectedRepository, }; use crate::api::{ActualApiClient, DEFAULT_API_URL}; use crate::auth::oauth; @@ -58,6 +61,82 @@ fn resolve_api_url(flag: Option<&str>) -> String { .unwrap_or_else(|| DEFAULT_API_URL.to_string()) } +/// Resolve a `--repo` value to a `repo_unique_id`. A value that parses as a UUID +/// is already an id and is returned as-is (no lookup, backward compatible); any +/// other value is treated as a repository name and resolved against the org's +/// connected repositories via `GET /v1/connected-repos`. +async fn resolve_repo( + value: &str, + client: &ActualApiClient, + org_id: &str, +) -> Result { + if Uuid::parse_str(value).is_ok() { + return Ok(value.to_string()); + } + let repos = client.list_connected_repos(org_id).await?; + resolve_repo_name(value, &repos) +} + +/// Match a repository name against the org's connected repos. A bare `name` +/// matches the `name` field; an `owner/name` form additionally constrains the +/// owner, disambiguating a name shared across owners. Zero matches — or a bare +/// name shared across owners — is a `RepoNotFound` error whose message lists the +/// choices. +fn resolve_repo_name(value: &str, repos: &[ConnectedRepository]) -> Result { + let matches: Vec<&ConnectedRepository> = match value.split_once('/') { + Some((owner, name)) => repos + .iter() + .filter(|r| { + r.external_owner.eq_ignore_ascii_case(owner) && r.name.eq_ignore_ascii_case(name) + }) + .collect(), + None => repos + .iter() + .filter(|r| r.name.eq_ignore_ascii_case(value)) + .collect(), + }; + match matches.as_slice() { + [single] => Ok(single.repo_unique_id.clone()), + [] => Err(ActualError::RepoNotFound(not_found_message(value, repos))), + multiple => Err(ActualError::RepoNotFound(ambiguous_message( + value, multiple, + ))), + } +} + +/// A connected repo rendered as `owner/name` for user-facing lists. +fn qualified_name(repo: &ConnectedRepository) -> String { + format!("{}/{}", repo.external_owner, repo.name) +} + +/// Build the "no match" error message, listing every connected repository (or +/// noting when the organization has none connected). +fn not_found_message(value: &str, repos: &[ConnectedRepository]) -> String { + if repos.is_empty() { + return format!( + "No repository named '{value}': this organization has no connected repositories." + ); + } + let mut msg = format!("No connected repository matches '{value}'. Connected repositories:"); + for repo in repos { + msg.push_str("\n • "); + msg.push_str(&qualified_name(repo)); + } + msg +} + +/// Build the "ambiguous bare name" error message, listing the owner-qualified +/// candidates so the caller can pick one. +fn ambiguous_message(value: &str, matches: &[&ConnectedRepository]) -> String { + let mut msg = + format!("'{value}' matches multiple connected repositories; qualify it as owner/name:"); + for repo in matches { + msg.push_str("\n • "); + msg.push_str(&qualified_name(repo)); + } + msg +} + fn enrich_org_mismatch( err: ActualError, session_org: &str, @@ -139,9 +218,21 @@ async fn run( let base_url = resolve_api_url(args.api_url.as_deref()); let client = ActualApiClient::new(&base_url)?.with_bearer(&creds.access_token); + // Resolve --repo to a repo id: a UUID is used as-is; any other value is a + // repository name resolved via the connected-repos API. A 403 during that + // lookup gets the same cross-org guidance as the query itself. + let repo_unique_id = match args.repo.as_deref() { + Some(value) => Some( + resolve_repo(value, &client, &org_id) + .await + .map_err(|e| enrich_org_mismatch(e, &session_org, &org_id, explicit_org))?, + ), + None => None, + }; + let request = AdvisorQueryRequest::new( org_id.clone(), - args.repo.clone(), + repo_unique_id, args.query.clone(), AdvisorSurface::cli(), AdvisorSink::None, @@ -904,4 +995,202 @@ mod tests { "remediation should not be in the message: {message}" ); } + + // --- --repo name resolution --- + + fn repo(owner: &str, name: &str, id: &str) -> ConnectedRepository { + ConnectedRepository { + repo_unique_id: id.to_string(), + name: name.to_string(), + external_owner: owner.to_string(), + url: format!("https://github.com/{owner}/{name}"), + } + } + + #[test] + fn test_qualified_name_is_owner_slash_name() { + assert_eq!( + qualified_name(&repo("actual-software", "actual-cli", "id")), + "actual-software/actual-cli" + ); + } + + #[test] + fn test_resolve_repo_name_by_bare_name() { + let repos = vec![ + repo("actual-software", "actual-cli", "id-cli"), + repo("actual-software", "web-app", "id-web"), + ]; + assert_eq!(resolve_repo_name("actual-cli", &repos).unwrap(), "id-cli"); + } + + #[test] + fn test_resolve_repo_name_by_owner_slash_name() { + // Two repos share the short name; the owner-qualified form disambiguates. + let repos = vec![ + repo("actual-software", "cli", "id-a"), + repo("other-org", "cli", "id-b"), + ]; + assert_eq!(resolve_repo_name("other-org/cli", &repos).unwrap(), "id-b"); + } + + #[test] + fn test_resolve_repo_name_is_case_insensitive() { + // GitHub owner/repo names are case-insensitive in practice; a + // differently-cased --repo value still resolves, in both the bare-name + // and owner-qualified forms. + let repos = vec![repo("actual-software", "actual-cli", "id-cli")]; + assert_eq!(resolve_repo_name("ACTUAL-CLI", &repos).unwrap(), "id-cli"); + assert_eq!( + resolve_repo_name("Actual-Software/Actual-CLI", &repos).unwrap(), + "id-cli" + ); + } + + #[test] + fn test_resolve_repo_name_not_found_lists_connected_repos() { + let repos = vec![ + repo("actual-software", "actual-cli", "id-cli"), + repo("actual-software", "web-app", "id-web"), + ]; + let err = resolve_repo_name("nope", &repos).unwrap_err(); + assert!(matches!(err, ActualError::RepoNotFound(_)), "got: {err:?}"); + // RepoNotFound Displays as its message ({0}); assert on that. + let msg = err.to_string(); + assert!(msg.contains("nope"), "got: {msg}"); + assert!(msg.contains("actual-software/actual-cli"), "got: {msg}"); + assert!(msg.contains("actual-software/web-app"), "got: {msg}"); + } + + #[test] + fn test_resolve_repo_name_ambiguous_bare_name() { + // A bare name shared across owners is ambiguous and asks for owner/name. + let repos = vec![ + repo("actual-software", "cli", "id-a"), + repo("other-org", "cli", "id-b"), + ]; + let err = resolve_repo_name("cli", &repos).unwrap_err(); + assert!(matches!(err, ActualError::RepoNotFound(_)), "got: {err:?}"); + let msg = err.to_string(); + assert!(msg.contains("multiple"), "got: {msg}"); + assert!(msg.contains("actual-software/cli"), "got: {msg}"); + assert!(msg.contains("other-org/cli"), "got: {msg}"); + } + + #[test] + fn test_not_found_message_when_org_has_no_connected_repos() { + let msg = not_found_message("anything", &[]); + assert!(msg.contains("anything"), "got: {msg}"); + assert!( + msg.contains("no connected repositories"), + "expected empty-org phrasing in: {msg}" + ); + } + + #[tokio::test] + async fn test_resolve_repo_uuid_passes_through_without_lookup() { + // A UUID short-circuits: the unreachable client is never called. + let client = ActualApiClient::new("http://127.0.0.1:1") + .unwrap() + .with_bearer("t"); + let id = "33333333-3333-3333-3333-333333333333"; + assert_eq!(resolve_repo(id, &client, "org").await.unwrap(), id); + } + + #[tokio::test] + async fn test_resolve_repo_by_name_calls_connected_repos_api() { + let mut server = mockito::Server::new_async().await; + let m = server + .mock("GET", "/v1/connected-repos") + .match_query(mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body( + r#"{"repositories":[{"repo_unique_id":"33333333-3333-3333-3333-333333333333","name":"actual-cli","external_owner":"actual-software","url":"https://github.com/actual-software/actual-cli"}]}"#, + ) + .create_async() + .await; + let client = ActualApiClient::new(&server.url()) + .unwrap() + .with_bearer("t"); + let id = resolve_repo("actual-cli", &client, "org").await.unwrap(); + assert_eq!(id, "33333333-3333-3333-3333-333333333333"); + m.assert_async().await; + } + + #[tokio::test] + async fn test_run_with_repo_name_resolves_and_scopes_request() { + let _lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); + let _g1 = EnvGuard::remove("ACTUAL_CONFIG"); + let tmp = tempdir().unwrap(); + let _g2 = EnvGuard::set("ACTUAL_CONFIG_DIR", tmp.path().to_str().unwrap()); + store::save(&test_creds()).unwrap(); + + let mut server = mockito::Server::new_async().await; + // Name → id lookup happens first. + let repos = server + .mock("GET", "/v1/connected-repos") + .match_query(mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body( + r#"{"repositories":[{"repo_unique_id":"33333333-3333-3333-3333-333333333333","name":"actual-cli","external_owner":"actual-software","url":"https://github.com/actual-software/actual-cli"}]}"#, + ) + .create_async() + .await; + // The advisor request then carries the resolved repo id. + let start = server + .mock("POST", "/v1/advisor/query") + .match_body(mockito::Matcher::PartialJsonString( + r#"{"repo_unique_id":"33333333-3333-3333-3333-333333333333"}"#.to_string(), + )) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(START_BODY) + .create_async() + .await; + let _poll = server + .mock("GET", POLL_PATH) + .with_body(succeeded_body("")) + .with_header("content-type", "application/json") + .create_async() + .await; + + let mut a = args(&server.url(), None); + a.repo = Some("actual-cli".to_string()); + run(&a, Duration::from_secs(60), Duration::ZERO) + .await + .unwrap(); + repos.assert_async().await; + start.assert_async().await; + } + + #[tokio::test] + async fn test_run_with_unknown_repo_name_errors_before_query() { + let _lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); + let _g1 = EnvGuard::remove("ACTUAL_CONFIG"); + let tmp = tempdir().unwrap(); + let _g2 = EnvGuard::set("ACTUAL_CONFIG_DIR", tmp.path().to_str().unwrap()); + store::save(&test_creds()).unwrap(); + + let mut server = mockito::Server::new_async().await; + let _repos = server + .mock("GET", "/v1/connected-repos") + .match_query(mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"repositories":[]}"#) + .create_async() + .await; + + let mut a = args(&server.url(), None); + a.repo = Some("ghost".to_string()); + let err = run(&a, Duration::from_secs(60), Duration::ZERO) + .await + .unwrap_err(); + assert!( + matches!(err, ActualError::RepoNotFound(ref m) if m.contains("ghost")), + "got: {err:?}" + ); + } } diff --git a/src/error.rs b/src/error.rs index 72e08827..07087ca4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -97,6 +97,14 @@ pub enum ActualError { /// rebuilds both with the concrete session and target orgs. #[error("{message}")] OrgMismatch { message: String, hint: String }, + + /// The `--repo ` argument named a repository that could not be + /// resolved to a connected repo — nothing matched, a short name was shared + /// across owners, or the organization has no connected repositories. The + /// message carries the full explanation, including the list of repositories + /// the caller can choose from; the fix rides on the `hint()` line. + #[error("{0}")] + RepoNotFound(String), } impl ActualError { @@ -114,7 +122,8 @@ impl ActualError { | Self::CodexCliModelRequiresApiKey { .. } | Self::NotLoggedIn | Self::NoRunnerAvailable { .. } - | Self::OrgMismatch { .. } => 2, + | Self::OrgMismatch { .. } + | Self::RepoNotFound(_) => 2, Self::CreditBalanceTooLow { .. } => 3, Self::ApiError(_) | Self::ApiResponseError { .. } | Self::ServiceUnavailable => 3, Self::IoError(_) => 5, @@ -168,6 +177,9 @@ impl ActualError { // target org), so it is carried on the variant rather than matched // to a static string like `NotLoggedIn`. Self::OrgMismatch { hint, .. } => Some(Cow::Owned(hint.clone())), + Self::RepoNotFound(_) => Some(Cow::Borrowed( + "Pass a connected repository name to --repo, or omit --repo to query the whole organization", + )), _ => None, } } @@ -822,4 +834,28 @@ mod tests { "expected remediation in hint: {hint}" ); } + + #[test] + fn test_repo_not_found_exit_code_display_and_hint() { + let err = ActualError::RepoNotFound( + "No connected repository matches 'foo'. Connected repositories:\n • acme/bar" + .to_string(), + ); + // A bad --repo value is a re-invoke-and-fix class failure, like NotLoggedIn. + assert_eq!(err.exit_code(), 2); + // The message passes through Display verbatim (it carries the repo list). + let msg = err.to_string(); + assert!(msg.contains("No connected repository"), "got: {msg}"); + assert!(msg.contains("acme/bar"), "expected repo list in: {msg}"); + // Remediation rides on the Fix/hint line, not Display. + let hint = err.hint().expect("expected a Fix hint for RepoNotFound"); + assert!( + hint.contains("--repo"), + "expected --repo guidance in hint: {hint}" + ); + assert!( + !msg.contains("Pass a connected"), + "remediation should not be in Display: {msg}" + ); + } }