WC-5233 use patch apis for wrangler preview secret commands - #15043
WC-5233 use patch apis for wrangler preview secret commands#15043podonnell-dev wants to merge 5 commits into
wrangler preview secret commands#15043Conversation
🦋 Changeset detectedLatest commit: 98ecb45 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
GregBrimble
left a comment
GregBrimble
left a comment
There was a problem hiding this comment.
Only other thing I'd consider testing is: no preview name provided and not in a git worktree.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| json: { | ||
| describe: "Return output as JSON", | ||
| type: "boolean", | ||
| default: false, | ||
| }, |
There was a problem hiding this comment.
🟡 Output format flag still leaks into the settings update command
The parent settings command's output-format flag is registered as a shared flag (json at packages/wrangler/src/preview/settings.ts:55-59) without opting out of propagation, so it still appears on, and is silently accepted by, the settings update subcommand.
Impact: Users see and can pass an output-format option on a command that ignores it, and the new help test that asserts this option is absent will fail.
Why parent options propagate to subcommands in the yargs tree
Commands are registered nested: wrangler preview settings is a command whose builder also registers wrangler preview settings update (packages/wrangler/src/index.ts:909-916, builder in packages/wrangler/src/core/register-yargs-command.ts:66-129). In yargs, options declared in a parent command's builder default to global: true and are therefore not reset when a subcommand runs, so they remain registered (and displayed in --help) for the subcommand. That is exactly the leak this PR fixes for wrangler preview by adding global: false to every arg in packages/wrangler/src/preview/preview.ts:19-63. The same treatment was not applied to previewSettingsCommand's json arg, yet the new test at packages/wrangler/src/__tests__/preview.test.ts:62-72 forbids --json in preview settings update --help.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Out of scope for current PR added wrangler preview secrets - this is existing behavior
1e113e9 to
7c3c05b
Compare
a3ddfa7 to
98ecb45
Compare
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
| if ( | ||
| args.skipConfirmation || | ||
| (await confirm( | ||
| `Are you sure you want to permanently delete the secret ${args.key} on the Preview "${previewName}" for the Worker ${workerName}${args.env ? ` (${args.env})` : ""}?` | ||
| )) | ||
| ) { |
There was a problem hiding this comment.
🟡 Declining the delete confirmation prints nothing
When the user answers "no" to the deletion prompt (confirm(...) at packages/wrangler/src/preview/secrets/delete.ts:62), the command finishes with no output at all, so it looks like the deletion may have succeeded.
Impact: Users who cancel a secret deletion get a silent, ambiguous result instead of a clear cancellation message.
Lost "Aborted." feedback when the confirmation is declined
The previous implementation (packages/wrangler/src/preview/secret.ts before removal) explicitly logged "Aborted." and returned when the user declined the confirmation prompt. The new handler wraps the whole body in if (args.skipConfirmation || await confirm(...)) { ... } with no else branch, so declining produces zero output while still exiting with a success status. Other Wrangler flows (e.g. previewDelete in packages/deploy-helpers/src/preview/preview.ts:800-806) still log "Aborted." in this situation, so this is also an inconsistency with the surrounding conventions.
Prompt for agents
In packages/wrangler/src/preview/secrets/delete.ts, the handler only performs work when the user confirms, but there is no branch for the declined case. The previous implementation logged "Aborted." and returned. Restore user feedback when the confirmation prompt is declined (e.g. early-return with a logged "Aborted." message instead of an if-block with no else), matching previewDelete in packages/deploy-helpers/src/preview/preview.ts.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Mirrors existing wrangler secret delete - ignoring
3 participants
Fixes WC-5233
Use PATCH APIs for wrangler
preview secretcommands. Removing the dependence onpreview_defaults- all preview secret commands now patch secrets for the latest preview deployment. Also adjusted file conventions to match #14448 more closelyReviewable, but ask Patrick before merging - we'll be trying to time this release for Tuesday's release
A picture of a cute animal (not mandatory, but encouraged)