From 68c30c500fd19d8a20a95fd36f8e511183796e94 Mon Sep 17 00:00:00 2001 From: "Sanket M." Date: Mon, 13 Jul 2026 18:38:02 +0530 Subject: [PATCH 1/3] fix(api-catalog): resolve discriminator.mapping refs to #/$defs pointers (DEVX-548) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API catalog left external file paths inside OpenAPI `discriminator.mapping` blocks (e.g. "./BasicChannel.yaml", "../payment-token/ApplePayPaymentToken.yaml"). Unlike `$ref` objects, mapping values are plain strings, so the dereferencer never rewrote them — and `api describe` surfaced broken, unresolvable schema references for polymorphic commerce types (Amount, Fee, FundingSource, Channel, PricingModel). The referenced subschemas are already lifted into `$defs` via the same key derivation, so rewrite each external mapping value to the matching `#/$defs/` pointer (only when the def exists, so a mapping is never pointed at a missing schema). Also extend the unresolved-ref guard to treat external mapping values as build-failing, same as an external `$ref`, so future drift is caught in CI. - generator: add rewrite_discriminator_mappings, run it after lifting in load_and_dereference; extend collect_unresolved_refs to scan mappings - regenerate-equivalent: rewrite 52 mappings across channels, payment-requests, payments, subscriptions, transactions catalogs - tests: mapping rewrite + drift-guard unit tests Note: two residual external references remain and are scoped as follow-ups: GraphQL `schemaRef` ("./graphql/schema.graphql") in catalog-products/taxes (tied to the GraphQL work in DEVX-546), and JSON-Schema `$id` URIs (valid identifiers, not broken refs). Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/schemas/api/channels.json | 12 +- rust/schemas/api/payment-requests.json | 16 +-- rust/schemas/api/payments.json | 28 ++-- rust/schemas/api/subscriptions.json | 4 +- rust/schemas/api/transactions.json | 44 +++---- rust/tools/generate-api-catalog/src/main.rs | 139 +++++++++++++++++++- 6 files changed, 190 insertions(+), 53 deletions(-) diff --git a/rust/schemas/api/channels.json b/rust/schemas/api/channels.json index 70aaf861..7dfebe81 100644 --- a/rust/schemas/api/channels.json +++ b/rust/schemas/api/channels.json @@ -58,12 +58,12 @@ "description": "Resource representing the Channel", "discriminator": { "mapping": { - "DEFAULT": "./BasicChannel.yaml", - "MARKETPLACE": "./MarketplaceChannel.yaml", - "MOBILE": "./MobileChannel.yaml", - "ONLINE": "./OnlineChannel.yaml", - "RETAIL": "./RetailChannel.yaml", - "SOCIAL": "./SocialChannel.yaml" + "DEFAULT": "#/$defs/BasicChannel", + "MARKETPLACE": "#/$defs/MarketplaceChannel", + "MOBILE": "#/$defs/MobileChannel", + "ONLINE": "#/$defs/OnlineChannel", + "RETAIL": "#/$defs/RetailChannel", + "SOCIAL": "#/$defs/SocialChannel" }, "propertyName": "type" }, diff --git a/rust/schemas/api/payment-requests.json b/rust/schemas/api/payment-requests.json index ce636897..30e2b97f 100644 --- a/rust/schemas/api/payment-requests.json +++ b/rust/schemas/api/payment-requests.json @@ -35,8 +35,8 @@ "description": "Transaction amount details", "discriminator": { "mapping": { - "DETAILED": "./DetailedAmount.yaml", - "SIMPLE": "./SimpleAmount.yaml" + "DETAILED": "#/$defs/DetailedAmount", + "SIMPLE": "#/$defs/SimpleAmount" }, "propertyName": "amountType" }, @@ -88,10 +88,10 @@ "description": "Authentication configuration for callback endpoints. Supports multiple authentication methods for secure callback delivery.\n\nRecommended method: HMAC for production environments as it provides cryptographic proof of authenticity and prevents tampering.\n", "discriminator": { "mapping": { - "apikey": "./APIKeyAuthentication.yaml", - "bearer": "./BearerTokenAuthentication.yaml", - "hmac": "./HMACAuthentication.yaml", - "none": "./NoAuthentication.yaml" + "apikey": "#/$defs/APIKeyAuthentication", + "bearer": "#/$defs/BearerTokenAuthentication", + "hmac": "#/$defs/HMACAuthentication", + "none": "#/$defs/NoAuthentication" }, "propertyName": "type" }, @@ -396,8 +396,8 @@ "description": "Fee associated with transaction", "discriminator": { "mapping": { - "ADDITIONAL": "./SimpleFee.yaml", - "GOVERNED": "./GovernedFee.yaml" + "ADDITIONAL": "#/$defs/SimpleFee", + "GOVERNED": "#/$defs/GovernedFee" }, "propertyName": "feeType" }, diff --git a/rust/schemas/api/payments.json b/rust/schemas/api/payments.json index 9f1b7025..08908730 100644 --- a/rust/schemas/api/payments.json +++ b/rust/schemas/api/payments.json @@ -116,8 +116,8 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "discriminator": { "mapping": { - "DETAILED": "./DetailedAmount.yaml", - "SIMPLE": "./SimpleAmount.yaml" + "DETAILED": "#/$defs/DetailedAmount", + "SIMPLE": "#/$defs/SimpleAmount" }, "propertyName": "amountType" }, @@ -610,8 +610,8 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "discriminator": { "mapping": { - "ADDITIONAL": "./SimpleFee.yaml", - "GOVERNED": "./GovernedFee.yaml" + "ADDITIONAL": "#/$defs/SimpleFee", + "GOVERNED": "#/$defs/GovernedFee" }, "propertyName": "feeType" }, @@ -640,12 +640,12 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "discriminator": { "mapping": { - "APPLE_PAY": "../payment-token/ApplePayPaymentToken.yaml", - "EBT_CARD": "./EBTCard.yaml", - "GODADDY": "../payment-token/GDPaymentTokenReference.yaml", - "GOOGLE_PAY": "../payment-token/GooglePayPaymentToken.yaml", - "PAYMENT_CARD": "./PaymentCard.yaml", - "PAYPAL": "../payment-token/PaypalPaymentToken.yaml" + "APPLE_PAY": "#/$defs/ApplePayPaymentToken", + "EBT_CARD": "#/$defs/EBTCard", + "GODADDY": "#/$defs/GDPaymentTokenReference", + "GOOGLE_PAY": "#/$defs/GooglePayPaymentToken", + "PAYMENT_CARD": "#/$defs/PaymentCard", + "PAYPAL": "#/$defs/PaypalPaymentToken" }, "propertyName": "sourceType" }, @@ -1056,8 +1056,8 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "discriminator": { "mapping": { - "CREDIT_CARD": "./CreditCardProcessorResponse.yaml", - "DEBIT_CARD": "./DebitCardProcessorResponse.yaml" + "CREDIT_CARD": "#/$defs/CreditCardProcessorResponse", + "DEBIT_CARD": "#/$defs/DebitCardProcessorResponse" }, "propertyName": "processorType" }, @@ -1290,8 +1290,8 @@ "description": "Verification data for funding source", "discriminator": { "mapping": { - "CARD": "../verification-data/CardVerificationData.yaml", - "TOKEN": "../verification-data/TokenVerificationData.yaml" + "CARD": "#/$defs/CardVerificationData", + "TOKEN": "#/$defs/TokenVerificationData" }, "propertyName": "type" }, diff --git a/rust/schemas/api/subscriptions.json b/rust/schemas/api/subscriptions.json index 805e8c11..02ffce87 100644 --- a/rust/schemas/api/subscriptions.json +++ b/rust/schemas/api/subscriptions.json @@ -177,8 +177,8 @@ "description": "Polymorphic pricing configuration for a subscription plan. The pricingModel field acts as the discriminator to determine which pricing schema applies.\n", "discriminator": { "mapping": { - "FIXED_RECURRING": "./FixedRecurringPricing.yaml", - "FREE": "./FreePricing.yaml" + "FIXED_RECURRING": "#/$defs/FixedRecurringPricing", + "FREE": "#/$defs/FreePricing" }, "propertyName": "pricingModel" }, diff --git a/rust/schemas/api/transactions.json b/rust/schemas/api/transactions.json index b00b7737..e1ad003c 100644 --- a/rust/schemas/api/transactions.json +++ b/rust/schemas/api/transactions.json @@ -68,8 +68,8 @@ "description": "Transaction amount details", "discriminator": { "mapping": { - "DETAILED": "./DetailedAmount.yaml", - "SIMPLE": "./SimpleAmount.yaml" + "DETAILED": "#/$defs/DetailedAmount", + "SIMPLE": "#/$defs/SimpleAmount" }, "propertyName": "amountType" }, @@ -667,8 +667,8 @@ "description": "Fee associated with transaction", "discriminator": { "mapping": { - "ADDITIONAL": "./SimpleFee.yaml", - "GOVERNED": "./GovernedFee.yaml" + "ADDITIONAL": "#/$defs/SimpleFee", + "GOVERNED": "#/$defs/GovernedFee" }, "propertyName": "feeType" }, @@ -698,15 +698,15 @@ "description": "Funding source of Transaction", "discriminator": { "mapping": { - "APPLE_PAY": "../payment-token/ApplePayPaymentToken.yaml", - "BANK_ACCOUNT": "./BankAccount.yaml", - "CASH": "./Cash.yaml", - "CUSTOM": "./CustomFundingSource.yaml", - "EBT_CARD": "./EBTCard.yaml", - "GODADDY": "../payment-token/GoDaddyPaymentToken.yaml", - "GOOGLE_PAY": "../payment-token/GooglePayPaymentToken.yaml", - "PAYMENT_CARD": "./PaymentCard.yaml", - "PAZE": "../payment-token/PazePaymentToken.yaml" + "APPLE_PAY": "#/$defs/ApplePayPaymentToken", + "BANK_ACCOUNT": "#/$defs/BankAccount", + "CASH": "#/$defs/Cash", + "CUSTOM": "#/$defs/CustomFundingSource", + "EBT_CARD": "#/$defs/EBTCard", + "GODADDY": "#/$defs/GoDaddyPaymentToken", + "GOOGLE_PAY": "#/$defs/GooglePayPaymentToken", + "PAYMENT_CARD": "#/$defs/PaymentCard", + "PAZE": "#/$defs/PazePaymentToken" }, "propertyName": "sourceType" }, @@ -2937,8 +2937,8 @@ "description": "Response from processor received as part of processing transaction", "discriminator": { "mapping": { - "CREDIT_CARD": "./CreditCardProcessorResponse.yaml", - "DEBIT_CARD": "./DebitCardProcessorResponse.yaml" + "CREDIT_CARD": "#/$defs/CreditCardProcessorResponse", + "DEBIT_CARD": "#/$defs/DebitCardProcessorResponse" }, "propertyName": "processorType" }, @@ -3116,11 +3116,11 @@ "description": "Transaction details", "discriminator": { "mapping": { - "ADJUSTMENT": "./Adjustment.yaml", - "AUTHORIZE": "./Authorization.yaml", - "CAPTURE": "./Capture.yaml", - "REFUND": "./Refund.yaml", - "SALE": "./Sale.yaml" + "ADJUSTMENT": "#/$defs/Adjustment", + "AUTHORIZE": "#/$defs/Authorization", + "CAPTURE": "#/$defs/Capture", + "REFUND": "#/$defs/Refund", + "SALE": "#/$defs/Sale" }, "propertyName": "type" }, @@ -3325,8 +3325,8 @@ "description": "Verification data for funding source", "discriminator": { "mapping": { - "CARD": "../verification-data/CardVerificationData.yaml", - "TOKEN": "../verification-data/TokenVerificationData.yaml" + "CARD": "#/$defs/CardVerificationData", + "TOKEN": "#/$defs/TokenVerificationData" }, "propertyName": "type" }, diff --git a/rust/tools/generate-api-catalog/src/main.rs b/rust/tools/generate-api-catalog/src/main.rs index 7e55a38a..19eba695 100644 --- a/rust/tools/generate-api-catalog/src/main.rs +++ b/rust/tools/generate-api-catalog/src/main.rs @@ -998,6 +998,20 @@ fn collect_unresolved_refs_inner(value: &Value, found: &mut Vec) { { found.push(r.clone()); } + // `discriminator.mapping` values are schema refs carried as plain strings. + // Any that still point outside this document (not `#/...`) are unresolved + // and must fail the build, same as an external `$ref`. + if let Some(Value::Object(disc)) = map.get("discriminator") + && let Some(Value::Object(mapping)) = disc.get("mapping") + { + for v in mapping.values() { + if let Value::String(s) = v + && !s.starts_with('#') + { + found.push(s.clone()); + } + } + } for v in map.values() { collect_unresolved_refs_inner(v, found); } @@ -1011,6 +1025,47 @@ fn collect_unresolved_refs_inner(value: &Value, found: &mut Vec) { } } +/// Rewrite `discriminator.mapping` values that still point at external files +/// (e.g. `./BasicChannel.yaml`, `../payment-token/ApplePayPaymentToken.yaml`) to +/// the local `#/$defs/` pointer produced by ref-lifting. +/// +/// The OpenAPI `discriminator.mapping` holds schema references as *plain strings*, +/// not `$ref` objects, so `dereference()` never touches them — leaving broken +/// external file paths in the generated catalog that surface through `api describe`. +/// The referenced subschemas are lifted into `$defs` via the same key derivation +/// (`derive_defs_key_for_path`), so we rewrite each mapping value to the matching +/// `#/$defs/` — but only when that def exists, so a mapping is never pointed +/// at a missing schema. +fn rewrite_discriminator_mappings(value: &mut Value, def_keys: &HashSet) { + match value { + Value::Object(map) => { + if let Some(Value::Object(disc)) = map.get_mut("discriminator") + && let Some(Value::Object(mapping)) = disc.get_mut("mapping") + { + for v in mapping.values_mut() { + if let Value::String(s) = v + && !s.starts_with('#') + { + let key = derive_defs_key_for_path(s); + if def_keys.contains(&key) { + *s = format!("#/$defs/{}", json_pointer_escape(&key)); + } + } + } + } + for v in map.values_mut() { + rewrite_discriminator_mappings(v, def_keys); + } + } + Value::Array(arr) => { + for v in arr { + rewrite_discriminator_mappings(v, def_keys); + } + } + _ => {} + } +} + fn load_and_dereference( spec_file: &Path, common_types_dir: Option<&Path>, @@ -1020,7 +1075,7 @@ fn load_and_dereference( let parsed = parse_yaml_or_json(&src, spec_file)?; let spec_dir = spec_file.parent().unwrap_or(spec_file); let mut defs = IndexMap::new(); - let dereffed = dereference( + let mut dereffed = dereference( &parsed.clone(), parsed, spec_dir, @@ -1028,6 +1083,14 @@ fn load_and_dereference( &mut defs, 0, ); + // Rewrite discriminator mappings (which carry schema refs as plain strings) to + // the lifted `#/$defs/` pointers. Run after all lifting so every def key + // is known; rewrite both the top-level spec and the lifted defs themselves. + let def_keys: HashSet = defs.keys().cloned().collect(); + rewrite_discriminator_mappings(&mut dereffed, &def_keys); + for v in defs.values_mut() { + rewrite_discriminator_mappings(v, &def_keys); + } Ok((dereffed, defs)) } @@ -1876,6 +1939,80 @@ mod tests { assert_eq!(unresolved, vec!["./models/Foo.yaml"]); } + #[test] + fn collect_unresolved_refs_finds_external_discriminator_mapping() { + // A discriminator.mapping value pointing outside the document is unresolved, + // even though it is a plain string rather than a `$ref` object. + let value = serde_json::json!({ + "$defs": { + "Channel": { + "discriminator": { + "propertyName": "kind", + "mapping": { + "DEFAULT": "./BasicChannel.yaml", + "LOCAL": "#/$defs/LocalChannel" + } + } + } + } + }); + let unresolved = collect_unresolved_refs(&value); + assert_eq!(unresolved, vec!["./BasicChannel.yaml"]); + } + + #[test] + fn discriminator_mapping_rewritten_to_defs_pointer() { + let dir = tempfile::tempdir().expect("tempdir"); + let schemas_dir = dir.path().join("schemas"); + fs::create_dir_all(&schemas_dir).expect("create schemas dir"); + + fs::write( + schemas_dir.join("BasicChannel.yaml"), + "type: object\nproperties:\n kind:\n type: string\n", + ) + .expect("write model"); + + // The oneOf `$ref` lifts BasicChannel into `$defs`; the discriminator.mapping + // carries the same target as a plain string that dereference() won't touch. + let spec_path = schemas_dir.join("openapi.yaml"); + fs::write( + &spec_path, + concat!( + "openapi: '3.0.0'\n", + "components:\n", + " schemas:\n", + " Channel:\n", + " oneOf:\n", + " - $ref: './BasicChannel.yaml'\n", + " discriminator:\n", + " propertyName: kind\n", + " mapping:\n", + " DEFAULT: './BasicChannel.yaml'\n", + ), + ) + .expect("write spec"); + + let (result, defs) = load_and_dereference(&spec_path, None).expect("dereference"); + + // No external refs remain — the mapping value now points inside the document. + let mut unresolved = collect_unresolved_refs(&result); + for def_val in defs.values() { + unresolved.extend(collect_unresolved_refs(def_val)); + } + assert!( + unresolved.is_empty(), + "expected zero unresolved refs after mapping rewrite, got: {unresolved:?}" + ); + + // The mapping value is rewritten to the lifted def pointer. + let mapping = + result["components"]["schemas"]["Channel"]["discriminator"]["mapping"]["DEFAULT"] + .as_str() + .expect("mapping DEFAULT string"); + assert_eq!(mapping, "#/$defs/BasicChannel"); + assert!(defs.contains_key("BasicChannel")); + } + #[test] fn dereference_lifts_external_ref_to_defs() { let dir = tempfile::tempdir().expect("tempdir"); From e5603a8e88c5800e4d966fa3a100c2a737b760a9 Mon Sep 17 00:00:00 2001 From: "Sanket M." Date: Mon, 13 Jul 2026 18:44:16 +0530 Subject: [PATCH 2/3] chore: gitignore legacy TS build artifacts node_modules/, dist/, .pnpm-store/, and .npmrc are artifacts of the legacy TS CLI and are not part of the Rust port. They surface as untracked noise when switching into the rust-port working tree; ignore them. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.gitignore b/.gitignore index 9a1c1d3e..932313e2 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,12 @@ rust/target/ __pycache__ +# Node/TS build artifacts (legacy TS CLI; not part of the Rust port) +node_modules/ +dist/ +.pnpm-store/ +.npmrc + # Environment files .env .env.dev From 10501cfb58d269acab684593f109abd626355c7f Mon Sep 17 00:00:00 2001 From: "Sanket M." Date: Tue, 14 Jul 2026 17:08:16 +0530 Subject: [PATCH 3/3] test(api-catalog): cover the un-liftable discriminator-mapping branch (DEVX-548) Adds the negative-path test for the safety guard: a discriminator.mapping value whose target is never lifted into $defs is left external (not pointed at a missing key), and collect_unresolved_refs then flags it so the build fails. Locks in the `def_keys.contains` guard behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- rust/tools/generate-api-catalog/src/main.rs | 49 +++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/rust/tools/generate-api-catalog/src/main.rs b/rust/tools/generate-api-catalog/src/main.rs index 19eba695..d64701bd 100644 --- a/rust/tools/generate-api-catalog/src/main.rs +++ b/rust/tools/generate-api-catalog/src/main.rs @@ -2013,6 +2013,55 @@ mod tests { assert!(defs.contains_key("BasicChannel")); } + #[test] + fn discriminator_mapping_target_not_lifted_stays_external_and_fails_guard() { + // Safety branch: a mapping value whose target is NOT otherwise referenced + // (no oneOf/$ref lifts it into $defs) must be left external — never pointed + // at a missing #/$defs key — so collect_unresolved_refs then flags it and + // the build fails, rather than silently producing a dangling pointer. + let dir = tempfile::tempdir().expect("tempdir"); + let schemas_dir = dir.path().join("schemas"); + fs::create_dir_all(&schemas_dir).expect("create schemas dir"); + + // Note: no BasicChannel.yaml on disk and no oneOf/$ref to it — nothing lifts it. + let spec_path = schemas_dir.join("openapi.yaml"); + fs::write( + &spec_path, + concat!( + "openapi: '3.0.0'\n", + "components:\n", + " schemas:\n", + " Channel:\n", + " type: object\n", + " discriminator:\n", + " propertyName: kind\n", + " mapping:\n", + " DEFAULT: './BasicChannel.yaml'\n", + ), + ) + .expect("write spec"); + + let (result, defs) = load_and_dereference(&spec_path, None).expect("dereference"); + + // The mapping value is left untouched (no matching def to point at)... + let mapping = + result["components"]["schemas"]["Channel"]["discriminator"]["mapping"]["DEFAULT"] + .as_str() + .expect("mapping DEFAULT string"); + assert_eq!(mapping, "./BasicChannel.yaml"); + assert!(!defs.contains_key("BasicChannel")); + + // ...and the unresolved-ref guard flags it, so the build would fail. + let mut unresolved = collect_unresolved_refs(&result); + for def_val in defs.values() { + unresolved.extend(collect_unresolved_refs(def_val)); + } + assert!( + unresolved.contains(&"./BasicChannel.yaml".to_owned()), + "expected the un-liftable mapping to be flagged as unresolved, got: {unresolved:?}" + ); + } + #[test] fn dereference_lifts_external_ref_to_defs() { let dir = tempfile::tempdir().expect("tempdir");