Skip to content

Prevent concurrent wrangler dev instances from fighting over the skills-install prompt#14199

Open
dario-piotrowicz wants to merge 3 commits into
mainfrom
dario/fix-14116-kittiwatm-issue
Open

Prevent concurrent wrangler dev instances from fighting over the skills-install prompt#14199
dario-piotrowicz wants to merge 3 commits into
mainfrom
dario/fix-14116-kittiwatm-issue

Conversation

@dario-piotrowicz
Copy link
Copy Markdown
Member

Fixes the concurrency bug described in #14116 (comment)

When two wrangler dev processes start in parallel, both could reach the interactive "Install Cloudflare skills?" prompt simultaneously. The second process's TTY output would overwrite the first's interactive prompt, making it invisible and unresponsive to the user.

A collision-detection mechanism now prevents this using the metadata JSONC file itself as a lock. Before showing the prompt, each process atomically creates the metadata file with a pending marker containing its PID and waits a 500 ms grace period. If a second process starts during that window, it overwrites the marker with its own PID and skips. The first process detects the PID change after the grace period and also skips. This ensures the prompt is only shown when a single wrangler dev instance is running. Stale pending markers (older than 60 seconds, e.g. from a crashed process) are automatically cleaned up.


  • 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: self explanatory

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

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 5, 2026

🦋 Changeset detected

Latest commit: 818496c

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

This PR includes changesets to release 4 packages
Name Type
wrangler Patch
@cloudflare/vite-plugin Patch
@cloudflare/vitest-pool-workers Patch
@cloudflare/wrangler-bundler 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

github-actions Bot commented Jun 5, 2026

✅ All changesets look good

ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Jun 5, 2026

Review posted successfully to PR #14199 with a summary and two inline suggestion comments (fd leak fix + stale-marker liveness check), plus a ranked list of the three issues found.

github run

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jun 5, 2026

create-cloudflare

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

@cloudflare/deploy-helpers

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-auth

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

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

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

wrangler

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

@cloudflare/wrangler-bundler

npm i https://pkg.pr.new/@cloudflare/wrangler-bundler@14199

commit: 818496c

@dario-piotrowicz dario-piotrowicz marked this pull request as ready for review June 5, 2026 23:11
@workers-devprod workers-devprod requested review from a team and james-elicx and removed request for a team June 5, 2026 23:11
@workers-devprod
Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • .changeset/fix-concurrent-skills-prompt.md: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/agents-skills-install.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/agents-skills-install.ts: [@cloudflare/wrangler]

Copy link
Copy Markdown
Contributor

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

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 potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

Copy link
Copy Markdown
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

This feature is getting pretty complex! Is there some way we could simplify the whole approach? In my mind I feel like we could just say: the question is only ever shown once, whether or not it is answered. We can just write a "no" answer into the metadata file before we even ask the question; if the user decides to say yes, we overwrite it with a "yes". Then if a second Wrangler starts up while the first is showing the question, it will not ask the question since it thinks it has already been answered. This would also avoid the Ctrl-C problem too.

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.

3 participants