From e6d3905dc682bac0565a72e7ce4729b47df46d55 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 18 Sep 2026 20:44:45 +0900 Subject: [PATCH] fix(repository-api): preserve optional fields and schema combinations --- .../src/repository_openapi_typescript.rs | 126 ++++++++++++++---- .../src/lib/workspace/api/workspace-model.ts | 4 +- web/workspace/tests/workspace-model.test.ts | 34 +++++ 3 files changed, 136 insertions(+), 28 deletions(-) diff --git a/crates/server-api/src/repository_openapi_typescript.rs b/crates/server-api/src/repository_openapi_typescript.rs index df3441e3e..24556e344 100644 --- a/crates/server-api/src/repository_openapi_typescript.rs +++ b/crates/server-api/src/repository_openapi_typescript.rs @@ -308,6 +308,25 @@ fn ensure_schema_keywords( Ok(()) } +fn literal_matches_type(kind: &str, value: &Value) -> bool { + matches!( + (kind, value), + ("string", Value::String(_)) | ("boolean", Value::Bool(_)) | ("null", Value::Null) + ) +} + +fn literal_schema_matches_type(schema: &Map, kind: &str) -> bool { + if let Some(constant) = schema.get("const") { + return literal_matches_type(kind, constant); + } + schema + .get("enum") + .and_then(Value::as_array) + .is_some_and(|values| { + !values.is_empty() && values.iter().all(|value| literal_matches_type(kind, value)) + }) +} + fn render_schema(value: &Value, context: &str) -> Result { let schema = object(value, context)?; if schema.contains_key("$ref") { @@ -327,10 +346,7 @@ fn render_schema(value: &Value, context: &str) -> Result Result Result Some(kind.as_str()), + Some(_) => { + return Err(GenerationError::invalid(format!( + "{context} contains an unsupported oneOf type" + ))); + } + None => None, + }; let branches = branches.as_array().ok_or_else(|| { GenerationError::invalid(format!("{context} contains a non-array oneOf")) })?; @@ -402,6 +411,13 @@ fn render_schema(value: &Value, context: &str) -> Result Result MAX_SAFE_INTEGER) - || (format == Some("uint64") && minimum.is_none_or(|minimum| minimum < 0.0)) - { + let expected_minimum = if format == Some("uint64") { + 0.0 + } else { + MIN_SAFE_INTEGER + }; + if minimum != Some(expected_minimum) || maximum != Some(MAX_SAFE_INTEGER) { return Err(GenerationError::invalid(format!( - "{context} integer range exceeds its JavaScript safe-number mapping" + "{context} integer range does not match its JavaScript safe-number mapping" ))); } Ok("number".to_owned()) @@ -672,6 +699,12 @@ mod tests { let error = generate_repository_typescript(&document.to_string()).unwrap_err(); assert!(error.to_string().contains("safe-number")); + let mut document: Value = serde_json::from_str(OPENAPI).unwrap(); + document["components"]["schemas"]["RepositorySummary"]["properties"]["source_revision"]["maximum"] = + serde_json::json!(9_007_199_254_740_990_u64); + let error = generate_repository_typescript(&document.to_string()).unwrap_err(); + assert!(error.to_string().contains("safe-number")); + let mut document: Value = serde_json::from_str(OPENAPI).unwrap(); document["components"]["schemas"]["RepositorySummary"]["properties"]["source_revision"]["format"] = serde_json::json!("int64"); @@ -728,6 +761,47 @@ mod tests { ); } + #[test] + fn generator_rejects_discarded_combined_schema_constraints() { + let mut document: Value = serde_json::from_str(OPENAPI).unwrap(); + document["components"]["schemas"]["DiagnosticSeverity"]["const"] = + serde_json::json!("info"); + let error = generate_repository_typescript(&document.to_string()).unwrap_err(); + assert!( + error + .to_string() + .contains("unsupported schema keyword `enum`") + ); + + let mut document: Value = serde_json::from_str(OPENAPI).unwrap(); + let null_branch = + document["components"]["schemas"]["RepositorySummary"]["properties"]["git"]["anyOf"] + .as_array_mut() + .unwrap() + .iter_mut() + .find(|branch| branch.get("type").and_then(Value::as_str) == Some("null")) + .unwrap(); + null_branch["maxLength"] = serde_json::json!(1); + let error = generate_repository_typescript(&document.to_string()).unwrap_err(); + assert!( + error + .to_string() + .contains("unsupported schema keyword `maxLength`") + ); + + let mut document: Value = serde_json::from_str(OPENAPI).unwrap(); + document["components"]["schemas"]["RepositorySourceKind"]["type"] = + serde_json::json!("string"); + document["components"]["schemas"]["RepositorySourceKind"]["oneOf"][0] = + serde_json::json!({"const": true, "type": "boolean"}); + let error = generate_repository_typescript(&document.to_string()).unwrap_err(); + assert!( + error + .to_string() + .contains("oneOf branch does not match outer type") + ); + } + #[test] fn generator_is_bounded_to_the_repository_operation_contract() { let mut document: Value = serde_json::from_str(OPENAPI).unwrap(); diff --git a/web/workspace/src/lib/workspace/api/workspace-model.ts b/web/workspace/src/lib/workspace/api/workspace-model.ts index 3f6d51cdb..0665f423b 100644 --- a/web/workspace/src/lib/workspace/api/workspace-model.ts +++ b/web/workspace/src/lib/workspace/api/workspace-model.ts @@ -235,8 +235,8 @@ function gitSummary(value: unknown, path: string): GitRepositorySummary { exactKeys(git, ["status", "head", "branch", "dirty", "remotes"], path); return { status: repositoryString(git.status, `${path}.status`), - head: nullableRepositoryString(git.head, `${path}.head`), - branch: nullableRepositoryString(git.branch, `${path}.branch`), + head: optionalNullableRepositoryString(git.head, `${path}.head`), + branch: optionalNullableRepositoryString(git.branch, `${path}.branch`), dirty: boolean(git.dirty, `${path}.dirty`), remotes: repositoryArray(git.remotes, `${path}.remotes`).map(( item, diff --git a/web/workspace/tests/workspace-model.test.ts b/web/workspace/tests/workspace-model.test.ts index de7aa3e30..7ae598119 100644 --- a/web/workspace/tests/workspace-model.test.ts +++ b/web/workspace/tests/workspace-model.test.ts @@ -50,6 +50,40 @@ Deno.test("generated repository wrapper validates current Backend JSON", () => { } }); +Deno.test("repository git optional nullable fields accept omission and null", () => { + const omitted = structuredClone(repositoryList) as Record; + const omittedItems = omitted.items as Array>; + omittedItems[0].git = { + status: "available", + dirty: false, + remotes: [], + }; + const omittedParsed = parseRepositoryListResponse(omitted); + if ( + omittedParsed.items[0]?.git?.head !== undefined || + omittedParsed.items[0]?.git?.branch !== undefined + ) { + throw new Error("omitted git fields were not preserved"); + } + + const nullable = structuredClone(repositoryList) as Record; + const nullableItems = nullable.items as Array>; + nullableItems[0].git = { + status: "available", + head: null, + branch: null, + dirty: false, + remotes: [], + }; + const nullableParsed = parseRepositoryListResponse(nullable); + if ( + nullableParsed.items[0]?.git?.head !== null || + nullableParsed.items[0]?.git?.branch !== null + ) { + throw new Error("null git fields were not preserved"); + } +}); + Deno.test("plain HTTP repository source kind fails closed at the JSON boundary", () => { const stale = structuredClone(repositoryList) as Record; const items = stale.items as Array>;