[dario-piotrowicz]

This PR addressed a but that surfaced in #447 (as mentioned in #447 (comment))

[pkg-pr-new]

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

commit: c30abe3

[github-actions]

cf --help startup

	mean	stddev	min	max
PR	258.0 ms	4.5 ms	250.2 ms	265.1 ms
base	258.7 ms	4.6 ms	252.5 ms	267.7 ms

Δ (PR vs base): — -0.7 ms (-0.3%)

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]

Superseded by the review of 5a6da3d6.

This review, of 3f8a6757

💬 Approved with comments — The generator fix and the regenerated output look correct and are covered by tests. The only note is an optional clarification in the architecture guide.

Findings: 1 info

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

---

📝 Documentation: 💬 1 info

Optional clarification to the required-field prompting docs.

Recommendation: 💬 Optional Nits

• [INFO] docs/architecture.md:648: the 'Required-field interactive prompting' section could say that a required field inside an optional request-body object is enforced only when that object is supplied. (docs/architecture.md:648)

View full review

docs/architecture.md:648 says a missing required body field is prompted for whenever stdin is a TTY. A required field inside an optional object is only enforced when that object is supplied. That was already true before this pull request, and this change makes the scoping more precise. A short note on this exception would make the guide match the generator.

---

✅ All other checks passed

🔒 Security: ✅ LGTM

This is build-time generator logic plus the regenerated argument validation. No security concerns.

---

📖 AGENTS.md: ✅ LGTM

The generator was changed, its output regenerated rather than hand-edited, and a changeset added. This follows the repository conventions.

---

📋 Code Quality: ✅ LGTM

optionalParentPath is passed through every place in body.ts that builds a BodyParamInfo. optionalParents in args.ts builds one group per optional object, and its required fields come only from that object's nearest-parent fields. The single-flag skip in emit/builder.ts drops only checks that could never fire, so the shorter generated files lose no validation. The changeset uses patch, which matches the repository convention for fixes.

---

⚡ Performance: ✅ LGTM

The generator does only a little extra linear work per operation at build time. The generated command files get smaller.

---

_{Reviewed commit: 3f8a6757}

[clanky-the-clanker]

Superseded by the review of c30abe3b.

This review, of 5a6da3d6

✅ Approved — The generator fix and the regenerated output look correct and are covered by tests. The earlier documentation note has been addressed, and nothing new was found.

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

---

✅ All checks passed

🔒 Security: ✅ LGTM

This is build-time generator logic plus the regenerated client-side argument validation. It adds no new inputs, secrets, network calls or injection surface. The argv.body === undefined guard only skips per-flag group checks for raw JSON input, which those checks never covered. The server still validates the payload.

---

📖 AGENTS.md: ✅ LGTM

The generator plugin was changed and its output regenerated rather than hand-edited. A patch changeset was added for this user-facing fix. Both follow the repository conventions.

---

📋 Code Quality: ✅ LGTM

optionalParentPath is passed to every place in body.ts that builds a BodyParamInfo, and args.ts copies it onto each Arg. Each optional object gets one group in optionalParents. A group's required fields are those whose nearest optional parent is that object.

The new single-flag skip in emit/builder.ts drops a group only when its one required flag is also its one set-flag. That check could never fire, so no validation is lost. The new argv.body === undefined guard means the group checks apply to flag input only, not to raw JSON passed with --body. Tests cover the new behaviour.

---

⚡ Performance: ✅ LGTM

The generator does only a little extra linear work per operation at build time. The regenerated command files get smaller.

---

📝 Documentation: ✅ LGTM

The "Required-field interactive prompting" section of docs/architecture.md now says that required fields inside an optional request-body object are enforced and prompted for only when a flag supplies that object. That resolves the earlier note.

---

_{Reviewed commit: 5a6da3d6}

[ask-bonk]

@dario-piotrowicz Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[clanky-the-clanker]

✅ Approved — The generator fix and its regenerated output look correct and have tests. The only change since the last review is the docs/architecture.md update, which describes the new behaviour accurately. No findings.

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

---

✅ All checks passed

🔒 Security: ✅ LGTM

This changes build-time generator logic and the client-side argument checks it generates. It adds no new inputs, secrets, network calls or injection surface.

---

📖 AGENTS.md: ✅ LGTM

The generator plugin was changed and its output regenerated rather than edited by hand. A patch changeset was added for this user-facing fix. Both follow the repository's conventions.

---

📋 Code Quality: ✅ LGTM

Every place in body.ts that builds a BodyParamInfo now passes optionalParentPath, and args.ts copies it onto each Arg. In emit/builder.ts, the checks for required fields inside an optional object now run only when a flag supplies that object, and they are skipped for raw --body JSON. The regenerated commands are consistent with the generator changes, and the new tests cover the behaviour.

---

⚡ Performance: ✅ LGTM

The generator does a little extra linear work per operation at build time, and the regenerated command files get smaller.

---

📝 Documentation: ✅ LGTM

The "Required-field interactive prompting" section of docs/architecture.md now says that required fields inside an optional request-body object are enforced and prompted for only when a flag supplies that object. This matches the generated behaviour.

---

_{Reviewed commit: c30abe3b}

[ask-bonk]

LGTM

github run

[ask-bonk]

argv.body === undefined treats an empty --body "" as a raw-body bypass, but normal generated handlers use if (argv.body) (for example, o11y/queries/create.ts:206). Consequently, --body "" --parameters-order-by-order asc skips this conditional requirement and then assembles/sends the incomplete flag body. Match the handler predicate so empty input continues through flag validation.

Suggested change

	lines.push(" if (argv.body === undefined && groupSet) {");
	lines.push(" if (!argv.body && groupSet) {");

[ask-bonk]

Review: 1 findings.

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

Corrects generated validation for nested optional request-body objects.

1. Warning: posted 1 inline suggestion.

github run