Repository navigation
fix: include force flags in generated command metadata - #497
Conversation
commit: |
|
LGTM! |
![clanky-the-clanker[bot]](https://avatars.githubusercontent.com/in/5306670?s=60&v=4){kind=small}
There was a problem hiding this comment.
Superseded by the review of
b12903ec.
This review, of 2b19fdcb
✅ Approved — The confirmation rule is now shared correctly across runtime emission, telemetry and metadata, with regression tests; no issues found.
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. The patch changeset is appropriate for this fix.
🔒 Security: ✅ LGTM
No security issues found.
📖 AGENTS.md: ✅ LGTM
No issues found.
📋 Code Quality: ✅ LGTM
No issues found.
⚡ Performance: ✅ LGTM
These are build-time generator changes plus static metadata, so nothing here affects performance.
📝 Documentation: ✅ LGTM
No issues found.
Reviewed commit: 2b19fdcb
|
| mean | stddev | min | max | |
|---|---|---|---|---|
| PR | 230.4 ms | 3.2 ms | 224.9 ms | 236.0 ms |
| base | 234.0 ms | 7.1 ms | 226.1 ms | 256.9 ms |
Δ (PR vs base): ↓ -3.5 ms (-1.5%)
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.
There was a problem hiding this comment.
✅ Approved — No issues found. The test-guard update keeps duplicate and API-field collision detection, and the shared confirmation rule is applied the same way to runtime, telemetry and metadata.
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 fits this fix.
🔒 Security: ✅ LGTM
No security issues found. Destructive commands still accept only --force / -f to skip confirmation.
📖 AGENTS.md: ✅ LGTM
Removing the resolved gap from packages/cli/AGENTS.md is accurate.
📋 Code Quality: ✅ LGTM
requiresConfirmation is now the single rule used by the builder, telemetry aliases and metadata. This removes the old disagreement over an empty requireConfirmation annotation. The CF_EMITTED update in the flag-name test still catches duplicate --force options and API fields named force or f.
⚡ Performance: ✅ LGTM
These are build-time generator changes plus static metadata, so nothing here affects performance.
📝 Documentation: ✅ LGTM
No issues found.
Reviewed commit: b12903ec
|
LGTM |
Generated destructive commands accept
--force/-f, but theircommands.jsonmetadata omits the option, socf completecannot suggest the confirmation bypass.Share the confirmation rule across runtime emission, telemetry aliases, and metadata, then regenerate the 450 affected command entries. Add regression coverage for DELETE operations, confirmation-annotated POST/PUT/PATCH operations, ordinary commands, and empty annotations. Update the flag ownership guard to recognize cf's emitted
--forceoption while retaining API-field collision and duplicate detection. Include a patch changeset and remove the resolved gap from the CLI instructions.Addresses the generator issue identified in #477 (comment).
Validation:
pnpm --filter cf test: full CLI suite passed (2,353 tests passed, six skipped), including the generator, completion, and flag ownership tests.pnpm check: passed.accounts delete --fsuggests--force;accounts list --fdoes not.