[wrangler] Let CLOUDFLARE_ACCOUNT_ID override the cached account id in pages project commands - #14983
[wrangler] Let CLOUDFLARE_ACCOUNT_ID override the cached account id in pages project commands#14983kdelay wants to merge 2 commits into
Conversation
|
Codeowners approval required for this PR:
Show detailed file reviewers |
@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: |
42d31a2 to
1ac190e
Compare
🦋 Changeset detectedLatest commit: 0f5edfe 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 |
![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.
workers-devprod
left a comment
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
|
Thanks for the review and for merging The remaining red checks on
One thing worth flagging: because turbo stops the run when |
|
Follow-up on my previous note, with one data point that confirms it.
I have merged the latest One thing I got wrong and should flag: because the push comes from a fork, the new head's workflows are sitting at "waiting for approval" rather than running, so the merge did not actually retrigger CI on its own. Happy to leave the branch alone from here if you would rather trigger the runs yourself. |
…n pages project commands
pages project list, create and delete passed the internal Pages cache
(pages.json) straight to requireAuth. Account selection treats
config.account_id as user-authored configuration and ranks it above
CLOUDFLARE_ACCOUNT_ID, so a stale cached account silently won and the
command targeted the wrong account.
Overlay the environment account id on top of the cache before resolving
auth, matching what pages deploy, pages deployment list, pages
deployment delete, pages download config and pages secret already do.
The three existing "should override cached accountId" tests used
vi.mock("getConfigCache", ...), which mocks a module specifier that does
not exist, so the cache was empty and the assertions passed against the
bug. They now seed the real cache with saveToConfigCache and fail
without the fix.
a47129f to
0f5edfe
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers |
|
Thanks for rebasing this. Quick triage on the two reds, since neither of them reaches code this PR touches.
The job installs pnpm 11.5.1 into
Both check names are red on #15130 too, there failing at The jobs that do cover this change are green on |
|
Triage update on the one remaining red check, correcting what I wrote in my previous comment: Attempt 2 (2026-08-11 20:17Z) failed with a single The 8 failing suites all come from three fixtures that omit Measured locally against this branch's lockfile, dispatching a trivial Worker per date:
Control: #15143 is unrelated to this change, pins the same So what clears it here is a branch update: One note that may matter beyond this PR: My diff is unchanged ( |
4 participants
Fixes #14970.
pages project list,pages project createandpages project deletepass the internal Pages cache (pages.json) straight torequireAuth(). Account selection treatsconfig.account_idas user-authored configuration and ranks it aboveCLOUDFLARE_ACCOUNT_ID(getActiveAccountIdinpackages/workers-auth/src/core/factory.ts), so a stale cached account silently wins. In a multi-account setup the command then targets the wrong account and fails withAuthentication error [code: 10000].This overlays the environment account id on top of the cache before resolving auth, so an explicit
CLOUDFLARE_ACCOUNT_IDtakes precedence and the cache stays a fallback. That is what the rest of the Pages commands already do:pages deploy,pages deployment list,pages deployment delete,pages download configandpages secretall build the same{ ...configCache, ...(envAccountId ? { account_id: envAccountId } : {}) }object. Onlyprojects.tswas missing it.On the tests
Each of the three commands already had a test named "should override cached accountId with CLOUDFLARE_ACCOUNT_ID environmental variable if provided", and all three passed against the bug. They seeded the cache with
"getConfigCache"is not a module specifier, so nothing was mocked and the cache was empty — which meantrequireAuthfell through to the env var and the assertion held for the wrong reason. The three tests now seed the real cache withsaveToConfigCache, matching howdeployment-list.test.tstests the same behaviour.Verified by reverting
projects.tstomainand re-running: all three fail, each requesting/accounts/original-account-id/.... With the fix they pass, andpnpm -w test:ci -F wrangler -- src/__tests__/pagesis green (20 files, 283 tests).pnpm checkpasses.CLOUDFLARE_ACCOUNT_IDfor three commands that were inconsistent with the rest ofwrangler pages; no user-facing interface changes.Note
This is a contribution from an AI agent: Claude Code, claude-opus-5.