[petebacondarwin]

What changes

Nested oneOf flags can currently assemble bodies that violate the selected branch. For example, priority-intelligence create/update accept --dataset-scope-type datasets without IDs, or --dataset-scope-type default with IDs.

Preserve nested union branches selected by disjoint required enum values, including schemas without an explicit discriminator, and validate the assembled wire body with cf's existing JSON Schema validator before sending it or showing a dry-run preview. Enforce branch-specific required fields, closed-object rules, and array maxItems. Find tagged unions through enclosing unions while preserving their alternatives, so a valid sibling branch at the same nested path is accepted.

Keep explicit --body input as an intentional escape hatch from these projected union checks. Document that branch-specific missing fields fail validation without interactive prompting, including on a TTY.

Regenerate the ten affected commands and add a patch changeset. Generic fixtures execute freshly generated create/update builders and handlers, covering optional omission, valid branches, missing/forbidden fields, the 50/51-item boundary, renamed flags, referenced schemas, explicit/implicit selectors, boolean selectors, negative branch detection, enclosing alternatives, and the raw-body bypass.

Validation

• Rebased onto main at 8e5f074d6 (includes Keep required HTTP header parameters out of generated JSON request bodies #460).
• pnpm generate — passed; no generated drift after the rebase.
• pnpm --filter cf test — 2,175 passed, 6 skipped; includes all 56 nested-union regression tests.
• pnpm check — generation, build, workspace type checking, lint, and formatting passed.
• Reviewed all ten generated command diffs: only union metadata and validation calls differ from main.

Addresses the cf branch-validation portion of cloudflare/forge#33. Related upstream metadata work: cloudflare/forge#47. This change projects required fields, closed-object rules, and maxItems; minItems and UUID-format enforcement are outside its scope.

[pkg-pr-new]

npm i https://pkg.pr.new/cf@336

commit: daf120d

[github-actions]

cf --help startup

	mean	stddev	min	max
PR	292.1 ms	4.6 ms	285.2 ms	299.4 ms
base	289.6 ms	4.0 ms	282.9 ms	296.0 ms

Δ (PR vs base): ↑ +2.4 ms (+0.8%)

Measured with hyperfine on ubuntu-latest (3 warmup runs, 20 timed runs). CI numbers carry ±a few ms of runner jitter; treat small deltas as noise.

[clanky-the-clanker]

This pull request's reviews are posted as pull request reviews: see the review of abfc83f6.

[ask-bonk]

🤖 I've done a thorough review of your PR.

Adds generated nested tagged-union validation for flagged request bodies.

1. Warning: --body bypasses the new validation in dry-run and request paths. Posted a suggestion on packages/cli/generator/plugin/emit/handler.ts:279.

github run

[NuroDev]

Seems to be some file conflicts that need fixing @petebacondarwin

[ask-bonk]

LGTM

github run

[petebacondarwin]

@NuroDev - ready for another review.

[ask-bonk]

🤖 I've done a thorough review of your PR.

Adds generated validation for nested tagged-union flag bodies.

1. Warning: Posted 1 inline suggestion.

github run

[dario-piotrowicz]

On a surface level it looks good to me, however I think that the bonk's visit() comment might be right

[ask-bonk]

No new concerns since the last review

github run

[clanky-the-clanker]

Superseded by the review of c6b339a6.

This review, of abfc83f6

💬 Approved with comments — One WARNING: bodyUnions() does not descend into untagged unions, so a tagged union nested inside one is turned into flags but its body is never validated. None of the nine regenerated commands is affected today. The other two notes are optional (one SUGGESTION, one INFO).

Findings: 1 warning · 1 info · 1 suggestion (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #336.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## WARNING (1)

### [Code Quality] `bodyUnions()` only descends through objects and intersections. A tagged union nested inside an untagged outer `oneOf`/`anyOf` is still turned into flags but never validated, so invalid branch bodies (for example `type: datasets` with no IDs) are sent or previewed unchecked. This confirms the open inline thread.
- **File:** packages/cli/generator/plugin/body-unions.ts:130

## INFO (1)

### [Documentation] Say in `docs/architecture.md` that fields required by a selected nested-union branch fail validation instead of being prompted for on a TTY.
- **File:** docs/architecture.md:648

## SUGGESTION (1)

### [Code Quality] No tests cover the negative cases of `taggedMembers`: overlapping enum values, a selector that is not required in every member, a non-object member, and the `members.length < 2` guard. Boolean selectors such as the one in `k2/streams` are not covered either. A small unit test on `bodyUnions()` would catch regressions in the detector.
- **File:** packages/cli/generator/plugin/body-unions.ts:13

---

📋 Code Quality: ⚠️ 1 warning · 1 suggestion

The visit() gap raised in the open inline thread is confirmed at this commit. Branch-detection tests are still missing.

Recommendation: 📝 Minor Comments

• [WARNING] bodyUnions() only descends through objects and intersections. A tagged union nested inside an untagged outer oneOf/anyOf is still turned into flags but never validated, so invalid branch bodies (for example type: datasets with no IDs) are sent or previewed unchecked. This confirms the open inline thread. (packages/cli/generator/plugin/body-unions.ts:130)
• [SUGGESTION] No tests cover the negative cases of taggedMembers: overlapping enum values, a selector that is not required in every member, a non-object member, and the members.length < 2 guard. Boolean selectors such as the one in k2/streams are not covered either. A small unit test on bodyUnions() would catch regressions in the detector. (packages/cli/generator/plugin/body-unions.ts:13)

View full review

1. Tagged unions inside an untagged union are not validated (packages/cli/generator/plugin/body-unions.ts:126-132)

The point raised in the inline thread, which @​dario-piotrowicz [approver] agreed with, holds at this commit:

• visit() only recurses into object and intersection types.
• When taggedMembers rejects a union, nothing descends into its members.
• Flag derivation still reaches those members: oneOfConstraints in generator/plugin/body.ts merges the properties of every variant of an untagged oneOf/anyOf, so their fields still become flags.

So a spec with an outer untagged union, one of whose branches holds scope: oneOf[{type: default}, {type: datasets, dataset_ids}], still builds the invalid bodies this pull request fixes, and they are neither rejected nor flagged in a dry run.

The nine regenerated commands don't hit this today. A future spec with this shape would fail silently.

Fix: recurse through union members in visit and add a fixture with the tagged union wrapped in an untagged oneOf/anyOf. Check that valid input is not rejected when sibling outer branches define the same nested path differently.

2. Branch-detection tests (packages/cli/generator/plugin/body-unions.ts:13-42)

There are still no negative fixtures for:

• overlapping enum values
• a selector that is not required in every member
• a member that is not an object
• the members.length < 2 guard

There is also no fixture for a boolean selector, which the regenerated k2/streams create and update commands rely on.

---

📝 Documentation: 💬 1 info

The architecture guide still suggests that every missing required body field is prompted for.

Recommendation: 💬 Optional Nits

• [INFO] Say in docs/architecture.md that fields required by a selected nested-union branch fail validation instead of being prompted for on a TTY. (docs/architecture.md:648)

View full review

docs/architecture.md says missing required body fields are prompted for on a TTY. A field that only the selected nested-union branch requires is now rejected by validation instead of prompted for. The guide could mention that exception.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: Low. A patch changeset is appropriate for this bug fix.

---

🔒 Security: ✅ LGTM

This change adds client-side validation and crosses no new trust boundary. The server is still the final check on request bodies.

---

📖 AGENTS.md: ✅ LGTM

The change follows repository conventions. Generic emission rules are in generator/plugin/, the runtime helper in src/lib/ is product-agnostic, generated output was regenerated rather than edited by hand, and a patch changeset is included.

---

⚡ Performance: ✅ LGTM

Validation runs once per request over small schema fragments.

---

_{Reviewed commit: abfc83f6}

[clanky-the-clanker]

Superseded by the review of daf120da.

This review, of c6b339a6

💬 Approved with comments — One WARNING: bodyUnions() does not descend into untagged unions, so a tagged union nested inside one is turned into flags but its body is never validated. None of the nine regenerated commands is affected today. The other two notes are optional (one SUGGESTION, one INFO).

Findings: 1 warning · 1 info · 1 suggestion

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #336.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## WARNING (1)

### [Code Quality] `bodyUnions()` only descends through objects and intersections. A tagged union nested inside an untagged outer `oneOf`/`anyOf` is still turned into flags but never validated, so invalid branch bodies (for example `type: datasets` with no IDs) are sent or previewed unchecked. This confirms the open inline thread.
- **File:** packages/cli/generator/plugin/body-unions.ts:130

## INFO (1)

### [Documentation] Say in `docs/architecture.md` that fields required by a selected nested-union branch fail validation instead of being prompted for on a TTY.
- **File:** docs/architecture.md:648

## SUGGESTION (1)

### [Code Quality] No tests cover the negative cases of `taggedMembers`: overlapping enum values, a selector that is not required in every member, a non-object member, and the `members.length < 2` guard. Boolean selectors such as the one in `k2/streams` are not covered either. A small unit test on `bodyUnions()` would catch regressions in the detector.
- **File:** packages/cli/generator/plugin/body-unions.ts:13

---

📋 Code Quality: ⚠️ 1 warning · 1 suggestion

The visit() gap raised in the open inline thread is confirmed at this commit. Branch-detection tests are still missing.

Recommendation: 📝 Minor Comments

• [WARNING] bodyUnions() only descends through objects and intersections. A tagged union nested inside an untagged outer oneOf/anyOf is still turned into flags but never validated, so invalid branch bodies (for example type: datasets with no IDs) are sent or previewed unchecked. This confirms the open inline thread. (packages/cli/generator/plugin/body-unions.ts:130)
• [SUGGESTION] No tests cover the negative cases of taggedMembers: overlapping enum values, a selector that is not required in every member, a non-object member, and the members.length < 2 guard. Boolean selectors such as the one in k2/streams are not covered either. A small unit test on bodyUnions() would catch regressions in the detector. (packages/cli/generator/plugin/body-unions.ts:13)

View full review

1. Tagged unions inside an untagged union are not validated (packages/cli/generator/plugin/body-unions.ts:126-132)

The point raised in the inline thread, which @​dario-piotrowicz [approver] agreed with, holds at this commit:

• visit() only recurses into object and intersection types.
• When taggedMembers rejects a union, nothing descends into its members.
• Flag derivation still reaches those members: oneOfConstraints in generator/plugin/body.ts merges the properties of every variant of an untagged oneOf/anyOf, so their fields still become flags.

So a spec with an outer untagged union, one of whose branches holds scope: oneOf[{type: default}, {type: datasets, dataset_ids}], still builds the invalid bodies this pull request fixes, and they are neither rejected nor flagged in a dry run.

The nine regenerated commands don't hit this today. A future spec with this shape would fail silently.

Fix: recurse through union members in visit and add a fixture with the tagged union wrapped in an untagged oneOf/anyOf. Check that valid input is not rejected when sibling outer branches define the same nested path differently.

2. Branch-detection tests (packages/cli/generator/plugin/body-unions.ts:13-42)

There are still no negative fixtures for:

• overlapping enum values
• a selector that is not required in every member
• a member that is not an object
• the members.length < 2 guard

There is also no fixture for a boolean selector, which the regenerated k2/streams create and update commands rely on.

---

📝 Documentation: 💬 1 info

The architecture guide still suggests that every missing required body field is prompted for.

Recommendation: 💬 Optional Nits

• [INFO] Say in docs/architecture.md that fields required by a selected nested-union branch fail validation instead of being prompted for on a TTY. (docs/architecture.md:648)

View full review

docs/architecture.md says missing required body fields are prompted for on a TTY. A field that only the selected nested-union branch requires is now rejected by validation instead of prompted for. The guide could mention that exception.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: Low. A patch changeset is appropriate for this bug fix.

---

🔒 Security: ✅ LGTM

This change adds client-side validation and crosses no new trust boundary. The server is still the final check on request bodies.

---

📖 AGENTS.md: ✅ LGTM

The change follows repository conventions. Generic emission rules are in generator/plugin/, the runtime helper in src/lib/ is product-agnostic, generated output was regenerated rather than edited by hand, and a patch changeset is included.

---

⚡ Performance: ✅ LGTM

Validation runs once per request over small schema fragments.

---

_{Reviewed commit: c6b339a6 — same changes as abfc83f6, so this is that commit's review (no new review was run; line numbers are as of abfc83f6)}

[ask-bonk]

No new concerns since the last review

github run

[petebacondarwin]

Rebased onto main at 8e5f074d6 and regenerated; no generated drift remains. Pushed the review follow-up in daf120da2.

• Fixed traversal through enclosing unions while preserving sibling alternatives at the same nested path.
• Added boolean-selector and negative-detection coverage, plus generated-handler regressions for the intentional --body bypass.
• Clarified that branch-specific missing fields fail validation without prompting, including on a TTY.

Validation: pnpm --filter cf test — 2,175 passed, 6 skipped (including all 56 nested-union tests); pnpm check passed. The PR description now reflects the current implementation and verification.

[ask-bonk]

LGTM

github run

[clanky-the-clanker]

✅ Approved — No open findings: the follow-up commit fixes the nested-union traversal gap, adds the missing branch-detection tests and updates the docs. The --body bypass is intentional, documented and tested.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🚀 Release: ✅ LGTM

Risk: Low. A patch changeset is appropriate for this bug fix.

---

🔒 Security: ✅ LGTM

This change adds client-side validation of request bodies built from CLI flags and crosses no new trust boundary. The server still validates every request body. Skipping the checks for raw --body input only skips the checks this pull request adds. The path walk in validateBodyUnions uses Object.hasOwn, so a crafted key cannot reach prototype properties.

---

📖 AGENTS.md: ✅ LGTM

The change follows repository conventions:

• The generic emission rules live in generator/plugin/.
• The runtime helper in src/lib/ does not depend on any specific product.
• Generated output was regenerated rather than edited by hand.
• A patch changeset is included.

---

📋 Code Quality: ✅ LGTM

• bodyUnions() now descends through union members. A tagged union found inside an enclosing union is validated against the projected enclosing alternatives as an anyOf, so a valid sibling branch is not rejected.
• The new tests cover overlapping enum values, an optional selector, a non-object member, the single-member guard and boolean selectors.
• The validator supports every keyword the projection emits: anyOf, allOf, enum, maxItems and additionalProperties.
• The --body bypass is intentional, documented and covered by generated-handler tests.

---

⚡ Performance: ✅ LGTM

Validation runs once per request over small schema fragments.

---

📝 Documentation: ✅ LGTM

docs/architecture.md now says that fields required only by the selected branch fail validation and are not prompted for. It also describes the --body escape hatch.

---

_{Reviewed commit: daf120da}

Resolved