Skip to content

fix(wrangler): replace existing resource bindings during creation#14440

Open
james-elicx wants to merge 1 commit into
mainfrom
codex/fix-kv-placeholder-update
Open

fix(wrangler): replace existing resource bindings during creation#14440
james-elicx wants to merge 1 commit into
mainfrom
codex/fix-kv-placeholder-update

Conversation

@james-elicx

@james-elicx james-elicx commented Jun 26, 2026

Copy link
Copy Markdown
Member

When Wrangler creates a resource and config updates have been authorized, replace an existing binding with the selected name instead of appending a duplicate entry.

Authorization follows the existing command behavior:

  • Interactive users answer yes to Would you like Wrangler to add it on your behalf?.
  • Non-interactive users provide --update-config or --binding.

No additional replacement flag or confirmation is required. This allows project templates containing placeholder resource IDs to work in both interactive and automated workflows. Unrelated fields on the existing binding are preserved while the newly created resource details and selected local/remote setting are applied.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: This corrects Wrangler's existing config-update behavior without changing the public CLI or configuration schema.

A picture of a cute animal (not mandatory, but encouraged)

@changeset-bot

changeset-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3dbdc24

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
wrangler Patch
@cloudflare/vite-plugin Patch
@cloudflare/vitest-pool-workers Patch

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ All changesets look good

@ask-bonk

ask-bonk Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Now let me consider one subtle bug carefully. When bindingName from the prompt could match an existing binding, but what about case where the snippet's binding field differs from bindingName? Let me check what binding.binding resolves to. binding = { ...snippet(bindingName), remote }. The snippet sets binding: name ?? "KV" so binding.binding === bindingName. The findIndex compares existingBinding.binding === binding.binding. Consistent.

Let me consider the real concern: experimental_patchConfig with isArrayInsertion=false and the array contains an empty/unchanged binding might leave keys that should be removed. When replacing, for non-matching bindings, the code returns the raw existingBinding. getJSONPath recurses field by field and re-modifies them to the same value (no-op). Fine.

One genuinely worth-noting concern: comments in the config near the modified bindings. With isArrayInsertion=false, jsonc-parser modifies individual fields, generally preserving comments. OK.

I've thoroughly analyzed the logic, the merge semantics, the remote: undefined handling, the insertion vs replacement paths, and the test coverage. The implementation is correct, well-targeted, and adequately tested. The remote: undefined deletion behavior is validated by the new test. No logic bugs, security issues, or backward-compat violations found.

LGTM

github run

@pkg-pr-new

pkg-pr-new Bot commented Jun 26, 2026

Copy link
Copy Markdown
@cloudflare/autoconfig

npm i https://pkg.pr.new/@cloudflare/autoconfig@14440

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@14440

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/@cloudflare/deploy-helpers@14440

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@14440

miniflare

npm i https://pkg.pr.new/miniflare@14440

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@14440

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@14440

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@14440

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@14440

@cloudflare/workers-auth

npm i https://pkg.pr.new/@cloudflare/workers-auth@14440

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@14440

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@14440

wrangler

npm i https://pkg.pr.new/wrangler@14440

commit: 3dbdc24

@james-elicx james-elicx force-pushed the codex/fix-kv-placeholder-update branch 4 times, most recently from 80312b5 to 179cdcc Compare June 26, 2026 14:53
Comment thread packages/wrangler/src/__tests__/update-config-file.test.ts
@james-elicx james-elicx changed the title [wrangler] Replace existing resource bindings during creation fix(wrangler): replace existing resource bindings during creation Jun 26, 2026
@james-elicx james-elicx force-pushed the codex/fix-kv-placeholder-update branch 6 times, most recently from 5e80607 to b1a5985 Compare June 26, 2026 15:16
@james-elicx james-elicx force-pushed the codex/fix-kv-placeholder-update branch from b1a5985 to 3dbdc24 Compare June 26, 2026 15:18
@james-elicx james-elicx marked this pull request as ready for review June 26, 2026 15:30
@workers-devprod workers-devprod requested review from a team and NuroDev and removed request for a team June 26, 2026 15:30
@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • .changeset/tidy-kv-placeholder.md: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/update-config-file.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/utils/add-created-resource-config.ts: [@cloudflare/wrangler]

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

2 participants