fix: emit enum literal for required enum-typed request-body fields#339
Open
jwulf wants to merge 1 commit into
Open
fix: emit enum literal for required enum-typed request-body fields#339jwulf wants to merge 1 commit into
jwulf wants to merge 1 commit into
Conversation
The planner discarded enum constraints on required request-body fields:
even though the extractor captured them, buildRequestBodyFromCanonical
fell through to the seedBinding fallback and emitted ${fieldVar}
placeholders the universal seed prologue rewrote to random short
strings. Servers reject those with HTTP 400 "Value <random> is not a
valid <Enum>." Same for arrays whose items are enums (e.g.
permissionTypes: array of PermissionTypeEnum): the synthesised element
was a literal "placeholder" string the server rejected.
Thread enum metadata through extractor + canonical-schema walker +
planner, and emit enum[0] as an inline literal for required scalar
enums and [itemEnum[0]] for arrays of scalar enums. Both oneOf and
non-oneOf code paths covered. effectiveEnum walks $ref + allOf
directly (without going through resolveSchema, which would strip the
allOf wrap without propagating enum).
Class-scoped L3 invariant: configs/camunda-oca/regression-invariants
guards that no feature or variant scenario seeds a ${...} placeholder
or "placeholder" array element for a required enum-typed field across
all bundled-spec operations - not just the createAuthorization case
that triggered the report.
Closes #338
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes planner request-body seeding for required enum-typed fields by preserving enum metadata through extraction/canonical schema walking and emitting inline enum literals (including array item enums) instead of ${...Var} / "placeholder" values that cause server-side 400s.
Changes:
- Extend request oneOf variant metadata and canonical schema nodes to carry enum and array-item-enum information.
- Update
buildRequestBodyFromCanonicalto inlineenum[0]for required scalar enums and[itemEnum[0]]for required arrays-of-enums. - Add an L3 regression invariant guarding against placeholder seeding for required enum-typed request-body fields.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| path-analyser/src/types.ts | Adds fieldEnums / fieldItemEnums to request oneOf variant metadata. |
| path-analyser/src/extractSchemas.ts | Captures effective enum constraints (via $ref/allOf walking) for oneOf variant fields and array items. |
| path-analyser/src/canonicalSchemas.ts | Threads leaf-level enum and array-item itemEnum into canonical request schema nodes. |
| path-analyser/src/index.ts | Emits enum literals for required enum fields/arrays in request-body templates instead of seeding placeholders. |
| configs/camunda-oca/regression-invariants.test.ts | Adds class-scoped invariant to prevent enum-typed request-body fields from being seeded with placeholders. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+386
to
+394
| /** | ||
| * Walk a (possibly $ref- or allOf-wrapped) schema and return its `enum` | ||
| * array if one is declared. Used by `findOneOfGroups` and `walkSchema` to | ||
| * surface enum constraints to the planner so it can emit a real enum | ||
| * literal instead of seeding a `${var}` placeholder (#338). | ||
| * | ||
| * Returns `undefined` when no enum is declared anywhere along the chain. | ||
| * The result is the raw `enum` array — callers pick the first value. | ||
| */ |
Comment on lines
+8929
to
+8990
| function resolveRef(ref: string): SchemaObject | undefined { | ||
| const name = ref.split('/').pop() ?? ''; | ||
| return spec.components?.schemas?.[name]; | ||
| } | ||
|
|
||
| function resolveSchemaObj(s: SchemaObject, depth = 0): SchemaObject { | ||
| if (depth > 20) return s; | ||
| if (s.$ref) { | ||
| const target = resolveRef(s.$ref); | ||
| if (target) return resolveSchemaObj(target, depth + 1); | ||
| } | ||
| return s; | ||
| } | ||
|
|
||
| /** Walk $ref + allOf chain to find an enum array. */ | ||
| function effectiveEnum(s: SchemaObject, depth = 0): unknown[] | undefined { | ||
| if (depth > 20) return undefined; | ||
| const r = resolveSchemaObj(s, depth); | ||
| if (Array.isArray(r.enum)) return r.enum; | ||
| if (Array.isArray(r.allOf)) { | ||
| for (const part of r.allOf) { | ||
| const e = effectiveEnum(part, depth + 1); | ||
| if (e) return e; | ||
| } | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Walk a request-body schema (root + any oneOf variants) and collect, | ||
| * for every top-level required field, the enum values declared on the | ||
| * field's scalar schema (`fieldEnums`) and on its items' scalar schema | ||
| * when the field is an array (`itemEnums`). | ||
| */ | ||
| function collectEnumFields(rootSchema: SchemaObject): { | ||
| fieldEnums: Map<string, unknown[]>; | ||
| itemEnums: Map<string, unknown[]>; | ||
| } { | ||
| const fieldEnums = new Map<string, unknown[]>(); | ||
| const itemEnums = new Map<string, unknown[]>(); | ||
| const branches: SchemaObject[] = []; | ||
| const root = resolveSchemaObj(rootSchema); | ||
| if (root.properties) branches.push(root); | ||
| for (const v of root.oneOf ?? []) branches.push(resolveSchemaObj(v)); | ||
| for (const branch of branches) { | ||
| const required = new Set(branch.required ?? []); | ||
| for (const [field, sub] of Object.entries(branch.properties ?? {})) { | ||
| if (!required.has(field)) continue; | ||
| const fieldEnum = effectiveEnum(sub); | ||
| if (fieldEnum && fieldEnum.length > 0 && !fieldEnums.has(field)) { | ||
| fieldEnums.set(field, fieldEnum); | ||
| } | ||
| const resolved = resolveSchemaObj(sub); | ||
| if (resolved.type === 'array' && resolved.items) { | ||
| const itemEnum = effectiveEnum(resolved.items); | ||
| if (itemEnum && itemEnum.length > 0 && !itemEnums.has(field)) { | ||
| itemEnums.set(field, itemEnum); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return { fieldEnums, itemEnums }; |
Comment on lines
+9017
to
+9068
| const offenders: { | ||
| file: string; | ||
| scenario: string; | ||
| operationId: string; | ||
| field: string; | ||
| value: string; | ||
| expectedEnum: unknown[]; | ||
| }[] = []; | ||
| const placeholderPattern = /^\$\{[^}]+\}$/; | ||
|
|
||
| for (const dir of [FEATURE_SCENARIOS_DIR, VARIANT_SCENARIOS_DIR]) { | ||
| if (!existsSync(dir)) continue; | ||
| for (const f of readdirSync(dir)) { | ||
| if (!f.endsWith('-scenarios.json')) continue; | ||
| // biome-ignore lint/plugin: runtime contract boundary for parsed JSON | ||
| const file = JSON.parse(readFileSync(join(dir, f), 'utf8')) as FeatureScenarioFile; | ||
| for (const scenario of file.scenarios ?? []) { | ||
| for (const step of scenario.requestPlan ?? []) { | ||
| const enums = enumFieldsByOp.get(step.operationId); | ||
| if (!enums) continue; | ||
| for (const [field, value] of Object.entries(step.bodyTemplate ?? {})) { | ||
| // Scalar enum field: a string that looks like ${var} is a placeholder seed. | ||
| const fieldEnum = enums.fieldEnums.get(field); | ||
| if (fieldEnum && typeof value === 'string' && placeholderPattern.test(value)) { | ||
| offenders.push({ | ||
| file: f, | ||
| scenario: scenario.id, | ||
| operationId: step.operationId, | ||
| field, | ||
| value, | ||
| expectedEnum: fieldEnum, | ||
| }); | ||
| continue; | ||
| } | ||
| // Array enum field: any element that's a ${var} placeholder OR | ||
| // the generic synthesised "placeholder" literal is wrong. | ||
| const itemEnum = enums.itemEnums.get(field); | ||
| if (itemEnum && Array.isArray(value)) { | ||
| for (const elem of value) { | ||
| const isPlaceholderVar = | ||
| typeof elem === 'string' && placeholderPattern.test(elem); | ||
| const isGenericPlaceholder = elem === 'placeholder'; | ||
| if (isPlaceholderVar || isGenericPlaceholder) { | ||
| offenders.push({ | ||
| file: f, | ||
| scenario: scenario.id, | ||
| operationId: step.operationId, | ||
| field, | ||
| value: JSON.stringify(elem), | ||
| expectedEnum: itemEnum, | ||
| }); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #338
Problem
The planner discarded enum constraints on required request-body fields. Even though the extractor captured them,
buildRequestBodyFromCanonicalfell through to theseedBindingfallback and emitted${fieldVar}placeholders that the universal seed prologue rewrote to random short strings. Servers rejected those with HTTP 400Value <random> is not a valid <Enum>. Arrays whose items are enums (e.g.permissionTypes: array of PermissionTypeEnum) had the same problem — the synthesised element was a literal"placeholder"string the server rejected.Concretely,
post /authorizationsfeature-1previously emitted:{ "ownerId": "${ownerIdVar}", "ownerType": "${ownerTypeVar}", "resourceId": "${resourceIdVar}", "resourceType": "${resourceTypeVar}", "permissionTypes": ["placeholder"] }and now emits:
{ "ownerId": "${ownerIdVar}", "ownerType": "USER", "resourceId": "${resourceIdVar}", "resourceType": "AUDIT_LOG", "permissionTypes": ["ACCESS"] }Change
Thread enum metadata through extractor + canonical-schema walker + planner, and emit
enum[0]as an inline literal for required scalar enums and[itemEnum[0]]for arrays of scalar enums. Both oneOf and non-oneOf code paths covered.effectiveEnumwalks$ref+allOfdirectly (without going throughresolveSchema, which strips theallOfwrap without propagatingenum). This is required because OpenAPI enums commonly live behindallOf: [{ $ref: '#/components/schemas/SomeEnum' }]wrappers (e.g.resourceType,permissionTypes.items).Red / green / class-scoped
bundled-spec invariants: enum-typed request-body field seeding (#338)toconfigs/camunda-oca/regression-invariants.test.ts. Verified it fails against pre-fix output (reportsownerType: "${ownerTypeVar}",permissionTypes: ["placeholder"]offenders acrosscreateAuthorization,updateAuthorization, and several other ops).${...}placeholders on those fields (scalar) or${...}/"placeholder"array elements (arrays). The same category of bug cannot recur in a sibling op without the invariant catching it.Validation
npm run lint— cleantsc --noEmittypechecks — cleannpm test— 640 passed, 4 skipped, 61 files