scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

refactor(server): collapse 409 conflict into standard ApiError

Browse source 
Replace hand-built 409 response that inlined {errors,pull_request} with
a regular ApiError::with_code(CONFLICT, ..., "pull_request_exists") and
drop the optional pull_request field that had been added to the
ErrorResponse OpenAPI schema solely to carry the existing record.

Clients receiving a 409 can GET /runs/{id}/pull_request to retrieve the
stored record when they need it — the detail string still includes the
existing html_url, which is the field most clients branch on.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-23 22:45:29 -04:00
parent 7d63e2c298
commit 0bf2dd30ef
No known key found for this signature in database
3 changed files with 13 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
docs/api-reference/fabro-api.yaml
View file

@ -949,9 +949,8 @@ paths:
$ref: "#/components/schemas/ErrorResponse"
"409":
description: >-
Pull request already exists for this run. The `pull_request` field
contains the existing record so clients can recover its URL and
number without an extra call.
Pull request already exists for this run. Clients can GET
/runs/{id}/pull_request to retrieve the stored record.
content:
application/json:
schema:
@ -3801,13 +3800,6 @@ components:
failure responses only.
items:
type: string
pull_request:
allOf:
- $ref: "#/components/schemas/PullRequestRecord"
description: >-
Optional existing pull request record. Populated by `POST
/runs/{id}/pull_request` 409 responses so clients can recover the
stored record without a follow-up request.
ActorKind:
description: High-level category of an event actor.

31
lib/crates/fabro-server/src/server.rs
View file

@ -5069,22 +5069,12 @@ fn github_pull_request_not_found_error(record: &PullRequestRecord) -> ApiError {
)
}
fn pull_request_already_exists_response(record: &PullRequestRecord) -> Response {
let status = StatusCode::CONFLICT;
let title = status.canonical_reason().unwrap_or("Conflict").to_string();
(
status,
Json(serde_json::json!({
"errors": [{
"status": status.as_u16().to_string(),
"title": title,
"detail": format!("Pull request already exists at {}", record.html_url),
"code": "pull_request_exists",
}],
"pull_request": record,
})),
fn pull_request_already_exists_error(record: &PullRequestRecord) -> ApiError {
ApiError::with_code(
StatusCode::CONFLICT,
format!("Pull request already exists at {}", record.html_url),
"pull_request_exists",
)
.into_response()
}
fn missing_repo_origin_error() -> ApiError {
@ -5196,7 +5186,7 @@ async fn create_run_pull_request(
};
if let Some(record) = run_state.pull_request.as_ref() {
return pull_request_already_exists_response(record);
return pull_request_already_exists_error(record).into_response();
}
let Some(run_spec) = run_state.spec.as_ref() else {
@ -9662,10 +9652,11 @@ slug = "fabro"
let body = response_json!(response, StatusCode::CONFLICT).await;
assert_eq!(body["errors"][0]["code"], "pull_request_exists");
assert_eq!(body["pull_request"]["number"], 42);
assert_eq!(
body["pull_request"]["html_url"],
"https://github.com/acme/widgets/pull/42"
assert!(
body["errors"][0]["detail"]
.as_str()
.unwrap()
.contains("https://github.com/acme/widgets/pull/42")
);
}

7
lib/packages/fabro-api-client/src/models/error-response.ts
View file

@ -16,9 +16,6 @@
// May contain unused imports in some cases
// @ts-ignore
import type { ErrorResponseEntry } from './error-response-entry';
// May contain unused imports in some cases
// @ts-ignore
import type { PullRequestRecord } from './pull-request-record';
/**
* Standard error response containing one or more error entries.
@ -36,9 +33,5 @@ export interface ErrorResponse {
* Optional list of runtime env keys that were actually removed before an install failure. Currently populated by `POST /install/finish` failure responses only.
*/
'removed_env_keys'?: Array<string>;
/**
* Optional existing pull request record. Populated by `POST /runs/{id}/pull_request` 409 responses so clients can recover the stored record without a follow-up request.
*/
'pull_request'?: PullRequestRecord;
}