diff --git a/CHANGELOG.md b/CHANGELOG.md index 342836a72..337220261 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Audit operation labels**: added human-readable descriptions for the new `SKILL_*`, `MCP_*`, and `SECRET_*` operation types, and a `humanize()` fallback so any future operation type renders as "Something New" instead of "Performed unknown operation" ### Fixed +- **GitHub repo name conflict returns 409 instead of 500**: creating a project on GitHub with a repo name that already exists on the linked account previously surfaced as an opaque "Internal Server Error". The GitHub provider now detects the `422 name already exists` response and raises a typed `GitProviderError::RepositoryAlreadyExists { name }`, which the handler maps to **409 Conflict** with an actionable detail message. The frontend `extractProblemDetails` helper was also fixed to (1) use the correct RFC 7807 field name `type` (previously `type_url`), (2) unwrap `body` / `error` / `data` envelopes used by the hey-api openapi-ts client, and (3) gate detection on a proper type guard so unrelated thrown objects are no longer misidentified as ProblemDetails. Users now see a clear "Repository Already Exists" toast and can pick a different name without contacting support. - **`/projects/new` browser navigation**: the source picker (Import / Template / Git URL / Manual) now mirrors the chosen mode to a `?source=` query param, plus the chosen template (`&template=`) or validated public Git URL (`&repo=`). Browser back/forward, deep links, and tab restore all work — reloading on `?source=templates&template=nextjs-saas-starter` lands directly in the template configurator, and `?source=git-url&repo=https%3A%2F%2F...` re-validates the URL on mount. Sub-keys are cleared automatically when the source changes so we never end up with stale state like `?source=git-url&template=foo`. The inline mode (used inside the onboarding flow) continues to use plain React state since it doesn't own the URL. - **`app_url` generator inherits port from external_url when preview_domain is set**: a dev install with `preview_domain=*.localho.st` + `external_url=http://localhost:8080` previously produced `https://my-app.localho.st` (missing port, wrong scheme) because `preview_domain` won the priority and didn't carry transport details. Now when both settings are present, scheme and port are inherited from `external_url` while the host comes from `preview_domain` — yielding `http://my-app.localho.st:8080`. - **Per-provider icons in Git connection pickers**: GitHub, GitLab, and other connections now render the correct icon in both the `/projects/new` source picker and the Template configurator's Git Provider field, instead of always showing the GitHub mark. Resolved by joining `ConnectionResponse.provider_id` against the cached providers list. diff --git a/crates/temps-git/src/handlers/base.rs b/crates/temps-git/src/handlers/base.rs index d929d52d0..8d7bb11a6 100644 --- a/crates/temps-git/src/handlers/base.rs +++ b/crates/temps-git/src/handlers/base.rs @@ -52,9 +52,7 @@ impl From for Problem { .with_title("Database Error") .with_detail(e.to_string()) } - GitProviderManagerError::ProviderError(e) => problem_new(StatusCode::BAD_REQUEST) - .with_title("Provider Error") - .with_detail(e.to_string()), + GitProviderManagerError::ProviderError(e) => Problem::from(e), GitProviderManagerError::ProviderNotFound(msg) => problem_new(StatusCode::NOT_FOUND) .with_title("Provider Not Found") .with_detail(msg), @@ -130,6 +128,15 @@ impl From for Problem { .with_type("https://docs.temps.sh/errors/api_error") .with_title("API Error") .with_detail(msg), + GitProviderError::RepositoryAlreadyExists { ref name } => { + problem_new(StatusCode::CONFLICT) + .with_type("https://docs.temps.sh/errors/repository_already_exists") + .with_title("Repository Already Exists") + .with_detail(format!( + "A repository named '{}' already exists in the target account. Choose a different name.", + name + )) + } GitProviderError::NotImplemented => problem_new(StatusCode::NOT_IMPLEMENTED) .with_type("https://docs.temps.sh/errors/not_implemented") .with_title("Not Implemented") diff --git a/crates/temps-git/src/services/git_provider.rs b/crates/temps-git/src/services/git_provider.rs index 7cead2ddc..a3a7de435 100644 --- a/crates/temps-git/src/services/git_provider.rs +++ b/crates/temps-git/src/services/git_provider.rs @@ -23,6 +23,9 @@ pub enum GitProviderError { #[error("API error: {0}")] ApiError(String), + #[error("A repository named '{name}' already exists on this account")] + RepositoryAlreadyExists { name: String }, + #[error("Not implemented for this provider")] NotImplemented, diff --git a/crates/temps-git/src/services/github_provider.rs b/crates/temps-git/src/services/github_provider.rs index 79f3be3ad..bd9a5409c 100644 --- a/crates/temps-git/src/services/github_provider.rs +++ b/crates/temps-git/src/services/github_provider.rs @@ -1457,6 +1457,35 @@ impl GitProviderService for GitHubProvider { .await .unwrap_or_else(|_| "Unknown error".to_string()); error!("Failed to create repository: {} - {}", status, error_text); + + // Detect GitHub's "name already exists" 422 and surface as a typed + // variant so handlers can return 409 Conflict with a clean message. + if status.as_u16() == 422 { + #[derive(Deserialize)] + struct GhFieldError { + field: Option, + message: Option, + } + #[derive(Deserialize)] + struct GhErrorBody { + errors: Option>, + } + if let Ok(body) = serde_json::from_str::(&error_text) { + let name_taken = body.errors.iter().flatten().any(|e| { + e.field.as_deref() == Some("name") + && e.message + .as_deref() + .map(|m| m.contains("already exists")) + .unwrap_or(false) + }); + if name_taken { + return Err(GitProviderError::RepositoryAlreadyExists { + name: name.to_string(), + }); + } + } + } + return Err(GitProviderError::ApiError(format!( "Failed to create repository: {} - {}", status, error_text diff --git a/crates/temps-projects/src/handlers/handlers.rs b/crates/temps-projects/src/handlers/handlers.rs index 205949c9b..98995ae2f 100644 --- a/crates/temps-projects/src/handlers/handlers.rs +++ b/crates/temps-projects/src/handlers/handlers.rs @@ -1245,12 +1245,9 @@ pub async fn create_project_from_template( .await .map_err(|e| { error!("Failed to create repository from template: {:?}", e); - problemdetails::new(http::StatusCode::INTERNAL_SERVER_ERROR) - .with_title("Repository Creation Failed") - .with_detail(format!( - "Failed to create repository and push template code: {}", - e - )) + // Forward the typed Problem (e.g. 409 for "name already exists", + // 401 for auth failures) instead of flattening everything to 500. + Problem::from(e) })?; info!( diff --git a/web/src/utils/errorHandling.ts b/web/src/utils/errorHandling.ts index e462953c1..9049f863e 100644 --- a/web/src/utils/errorHandling.ts +++ b/web/src/utils/errorHandling.ts @@ -48,21 +48,38 @@ export function getExpiredTokenMessage( } /** - * Extract ProblemDetails from an error + * Extract ProblemDetails from an error. + * + * The hey-api openapi-ts client (with `throwOnError: true`) throws the parsed + * response body directly, so `error` is typically the `ProblemDetails` object + * itself. Older fetch-style errors may nest it under `body`. RFC 7807 uses + * `type` (not `type_url`), so we identify a ProblemDetails by the presence + * of a `title` string plus at least one of the canonical fields. */ export function extractProblemDetails(error: unknown): ProblemDetails | null { if (!error || typeof error !== 'object') return null - const err = error as any + const looksLikeProblem = (val: unknown): val is ProblemDetails => { + if (!val || typeof val !== 'object') return false + const v = val as Record + return ( + typeof v.title === 'string' && + ('detail' in v || 'type' in v || 'status' in v || 'extensions' in v) + ) + } + + const err = error as Record - // Check if error has a body property (from fetch API) - if (err.body && typeof err.body === 'object') { - return err.body as ProblemDetails + // Some clients nest the parsed body under `body`, `error`, or `data`. + for (const key of ['body', 'error', 'data'] as const) { + if (looksLikeProblem(err[key])) { + return err[key] as ProblemDetails + } } - // Check if error itself is ProblemDetails - if ('title' in err && 'type_url' in err) { - return err as ProblemDetails + // Default case: the thrown value IS the ProblemDetails. + if (looksLikeProblem(err)) { + return err } return null