Skip to content

fix(cli): pass dev auth profiles through child environments - #453

Open
petebacondarwin wants to merge 1 commit into
mainfrom
codex/dev-auth-profile-env
Open

petebacondarwin wants to merge 1 commit into
mainfrom
codex/dev-auth-profile-env

Conversation

@petebacondarwin

Copy link
Copy Markdown
Collaborator

cf dev --profile work selects a profile in cf, but the development server runs in a child process and loses that explicit selection. Pass it as CLOUDFLARE_AUTH_PROFILE to both Cloudflare delegates and detected framework dev commands. An explicit flag overrides the inherited value, and framework-specific environment overrides are preserved.

Companion to cloudflare/workers-sdk#16599, which adds shared profile resolution in @cloudflare/workers-auth and initializes cf authentication in cf-wrangler. The selected development server needs a release containing that support. This provides the environment-based approach discussed for #308 without adding a --profile argument to the delegate contract.

Validation:

  • Full CLI test suite: 2,082 passed, 7 skipped, including real child-process coverage for cf-vite, cf-wrangler, and a framework dev command.
  • Root pnpm check: passed (types, build, lint, and formatting).

@ask-bonk

ask-bonk Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

@pkg-pr-new

pkg-pr-new Bot commented Oct 30, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/cf@453

commit: e287d92

@clanky-the-clanker clanky-the-clanker Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💬 Approved with comments — The change is small and correct: an explicit --profile is passed as CLOUDFLARE_AUTH_PROFILE to both the delegate and framework dev-command paths, and tests cover both. The only finding is an INFO about the README, which does not describe the new behaviour.

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

The README does not describe how cf dev --profile passes the profile to child processes.

Recommendation: 💬 Optional Nits

  • [INFO] Consider documenting in the README's authentication section that cf dev --profile <name> passes the selection to development servers through CLOUDFLARE_AUTH_PROFILE, and that this needs a compatible @cloudflare/workers-auth version in the development server. (packages/cli/README.md:34)
View full review

The changeset and packages/cli/AGENTS.md describe the new behaviour accurately. The user-facing README section on profile selection (packages/cli/README.md:34-38) does not yet mention that cf dev --profile sets CLOUDFLARE_AUTH_PROFILE for development servers, or that a development server only uses the selected credentials if it runs a compatible @cloudflare/workers-auth.


✅ All other checks passed

📋 Code Quality: ✅ LGTM

profileEnv is set only when --profile is given explicitly. On the framework path it is merged after details.env, and on the delegate path it goes through implEnvironment(), so the flag takes precedence over an inherited CLOUDFLARE_AUTH_PROFILE and the rest of the environment is still inherited. Tests cover both spawn paths.


Reviewed commit: e287d924

@github-actions

Copy link
Copy Markdown
Contributor

cf --help startup

mean stddev min max
PR 230.9 ms 4.0 ms 222.1 ms 238.9 ms
base 233.1 ms 6.3 ms 226.5 ms 255.1 ms

Δ (PR vs base): ↓ -2.2 ms (-1.0%)

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.

@MattieTK MattieTK left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Digging into our conversation earlier and this solution, Claude is suggesting a version range update which I think makes sense to avoid a delegation to a wrangler account that isn't explicitly part of the profile. See below:

The environment variable approach looks right to me. One gap I noticed is version mismatch with older delegates.

cf dev checks delegate versions against the versionConstraint floors in known-impls.ts. These are currently wrangler >=4.136.0 and @cloudflare/vite-plugin >=2.0.0-0 <3.0.0-0. Both ranges allow releases from before cloudflare/workers-sdk#16599, and those releases ignore CLOUDFLARE_AUTH_PROFILE:

  • An older cf-vite falls back to the directory binding or the default profile.
  • An older cf-wrangler reads Wrangler's credential store rather than cf's.

Either way, cf dev --profile work runs against a different account and no error appears.

Could we raise both floors to the first releases that include cloudflare/workers-sdk#16599? The existing check in resolveProjectImpl would then fail early with the install hint, rather than starting the server with other credentials. This would also block projects on older delegates that don't use profiles. Since an older cf-wrangler doesn't read cf's credentials anyway, I think that's acceptable.

The versions aren't known until cloudflare/workers-sdk#16599 is released, so this would mean landing this PR after that release. The fixture versions in the new auth profile tests (4.136.0 and 2.0.0-beta.sha-805ec1ff3) would also need updating to pass the new floors.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants