Skip to content

feat: add Flagship binding support#13139

Open
roerohan wants to merge 18 commits intocloudflare:mainfrom
roerohan:roerohan/flagship
Open

feat: add Flagship binding support#13139
roerohan wants to merge 18 commits intocloudflare:mainfrom
roerohan:roerohan/flagship

Conversation

@roerohan
Copy link
Copy Markdown
Member

@roerohan roerohan commented Mar 31, 2026

Summary

Add end-to-end support for the Flagship binding across @cloudflare/workers-utils, miniflare, and wrangler.

Corresponding workerd PR: cloudflare/workerd#6456

  • @cloudflare/workers-utils: Config schema, validation, default values, CfFlagship type, WorkerMetadataBinding/Binding union entries, and metadata binding mapping
  • miniflare: New remote-only plugin (plugins/flagship/) with zod schema, proxy bindings, and remote proxy client wiring
  • wrangler: Deployment bundle upload, dev wiring (miniflare options, convertConfigToBindings), CLI binding display, type generation (Flags type in both code paths), OAuth scopes (flagship:read, flagship:write), and config-diffs support

  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because: This PR adds the plumbing only — tests will be added in a follow-up once the workerd Flagship binding types land
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: Public docs will be published closer to GA

Open with Devin

Add the Flagship binding to the shared config and type system:
- Add flagship binding type to EnvironmentNonInheritable (environment.ts)
- Add default value flagship: [] (config.ts)
- Add friendly names, notInheritable() validation, and validateFlagshipBinding (validation.ts)
- Add CfFlagship interface and CfWorkerInit.bindings.flagship field (worker.ts)
- Add flagship to WorkerMetadataBinding and Binding union types (types.ts)
- Add flagship case in mapWorkerMetadataBindings (map-worker-metadata-bindings.ts)
Create a new miniflare plugin for Flagship feature flag bindings:
- New plugins/flagship/index.ts with zod schema, getBindings,
  getNodeBindings, and getServices (remote proxy only, no local simulation)
- Register FLAGSHIP_PLUGIN in the PLUGINS object and WorkerOptions type
…elines

Add Flagship binding support across the deployment and dev subsystems:
- Add to getBindings() and createWorkerUploadForm() (deployment-bundle)
- Wire through dev.ts, start-dev.ts, and miniflare/index.ts (dev wiring)
- Add both convert functions in startDevWorker/utils.ts
- Add CLI display in print-bindings.ts (always remote mode)
- Add to ValidKeys exclusion in add-created-resource-config.ts
- Add flagship: [] default in secret/index.ts draft worker
- Add flagship -> Flags type mapping in both type generation code paths
  so `wrangler types` emits FLAGS: Flags for configured flagship bindings
- Add flagship:read and flagship:write to DefaultScopes for wrangler login
@roerohan roerohan requested a review from a team as a code owner March 31, 2026 09:45
@roerohan roerohan requested a review from vicb March 31, 2026 09:45
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 31, 2026

🦋 Changeset detected

Latest commit: b47e65c

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

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

@workers-devprod
Copy link
Copy Markdown
Contributor

workers-devprod commented Mar 31, 2026

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • packages/miniflare/src/plugins/flagship/index.ts: [@cloudflare/wrangler]
  • packages/miniflare/src/plugins/index.ts: [@cloudflare/wrangler]
  • packages/workers-utils/src/config/config.ts: [@cloudflare/wrangler]
  • packages/workers-utils/src/config/environment.ts: [@cloudflare/wrangler]
  • packages/workers-utils/src/config/validation.ts: [@cloudflare/wrangler]
  • packages/workers-utils/src/map-worker-metadata-bindings.ts: [@cloudflare/wrangler]
  • packages/workers-utils/src/types.ts: [@cloudflare/wrangler]
  • packages/workers-utils/src/worker.ts: [@cloudflare/wrangler]
  • packages/workers-utils/tests/config/validation/normalize-and-validate-config.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/deploy/core.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/type-generation.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/user.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/tests/whoami.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/api/startDevWorker/utils.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/deploy/config-diffs.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/deployment-bundle/create-worker-upload-form.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/dev/miniflare/index.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/type-generation/index.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/user/user.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/utils/add-created-resource-config.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/utils/print-bindings.ts: [@cloudflare/wrangler]

devin-ai-integration[bot]

This comment was marked as resolved.

…vice entry

Match the pattern used by AI and VPC plugins — pass the potentially-undefined
remoteProxyConnectionString through to remoteProxyClientWorker() which handles
the undefined case gracefully, instead of short-circuiting with an empty return.
devin-ai-integration[bot]

This comment was marked as resolved.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 31, 2026

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

wrangler

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

commit: b47e65c

…nding

Warn users who define flagship as an unsafe binding that it is directly
supported by wrangler, matching the pattern of other safe binding types.
devin-ai-integration[bot]

This comment was marked as resolved.

roerohan and others added 4 commits March 31, 2026 15:34
- Rename 'fs' to 'flagshipBinding' in type-generation to avoid shadowing
- Fix formatting in worker.ts and type-generation/index.ts
Add flagship:read and flagship:write to inline snapshot expectations
in user.test.ts (6), deploy/core.test.ts (2), and whoami.test.ts (3).
@petebacondarwin petebacondarwin added the blocked Blocked on other work label Mar 31, 2026
@roerohan
Copy link
Copy Markdown
Member Author

@fixture/vitest-pool-workers:test:ci: ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯
@fixture/vitest-pool-workers:test:ci: 
@fixture/vitest-pool-workers:test:ci:  FAIL   rpc  test/unit.test.ts > Durable Object > increments count and allows direct/rpc access to instance/storage
@fixture/vitest-pool-workers:test:ci: Error: Test timed out in 5000ms.
@fixture/vitest-pool-workers:test:ci: If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
@fixture/vitest-pool-workers:test:ci:  ❯ test/unit.test.ts:72:2
@fixture/vitest-pool-workers:test:ci:      70|   });
@fixture/vitest-pool-workers:test:ci:      71|  });
@fixture/vitest-pool-workers:test:ci:      72|  it("increments count and allows direct/rpc access to instance/storage…
@fixture/vitest-pool-workers:test:ci:        |  ^
@fixture/vitest-pool-workers:test:ci:      73|   expect,
@fixture/vitest-pool-workers:test:ci:      74|  }) => {
@fixture/vitest-pool-workers:test:ci: 
@fixture/vitest-pool-workers:test:ci: ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯
@fixture/vitest-pool-workers:test:ci: 
@fixture/vitest-pool-workers:test:ci: 
@fixture/vitest-pool-workers:test:ci:  Test Files  1 failed | 49 passed (50)
@fixture/vitest-pool-workers:test:ci:       Tests  1 failed | 131 passed (132)
@fixture/vitest-pool-workers:test:ci:    Start at  12:20:26
@fixture/vitest-pool-workers:test:ci:    Duration  59.98s (transform 13.40s, setup 8.40s, import 21.69s, tests 24.37s, environment 9ms)

This is the current CI error, a simple re-run should fix it
CC @petebacondarwin

@petebacondarwin
Copy link
Copy Markdown
Contributor

But this is not a flake: https://github.com/cloudflare/workers-sdk/actions/runs/23796373130/job/69344578952?pr=13139#step:6:1650

roerohan and others added 4 commits April 1, 2026 11:23
Add flagship binding to bindingsConfigMock to satisfy the
EnvironmentNonInheritable type constraint.
Update 3 inline snapshots that output the full generated Env interface
to include the FLAGS: Flags binding from the flagship config mock.
@petebacondarwin petebacondarwin requested review from petebacondarwin and removed request for vicb April 2, 2026 09:14
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.

🚩 No tests included for flagship binding integration

Per the AGENTS.md and CONTRIBUTING.md guidelines, every non-trivial change should include tests. This PR adds a new binding type across multiple packages (miniflare plugin, config validation, type generation, deployment form, print bindings) but includes no test files. Existing bindings like Hello World have tests in packages/miniflare/test/plugins/. Config validation changes typically have tests in packages/workers-utils/src/config/__tests__/. The type generation changes have tests in packages/wrangler/src/__tests__/type-generation/. At minimum, the miniflare plugin, config validation, and type generation should have corresponding tests.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +788 to +796
flagship: Object.fromEntries(
flagshipBindings.map((binding) => [
binding.binding,
{
app_id: binding.app_id,
remoteProxyConnectionString,
},
])
),
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.

🚩 Flagship miniflare plugin doesn't emit warnOrError for always-remote usage like vpc_services does

In packages/wrangler/src/dev/miniflare/index.ts:788-796, the flagship binding is set up for miniflare with remoteProxyConnectionString always passed through. Compare with vpc_services at packages/wrangler/src/dev/miniflare/index.ts:854-863 which calls warnOrError("vpc_service", vpc.remote, "always-remote") to warn users when they try to use it without remote dev. The flagship plugin silently passes through, which means users running wrangler dev (local only, no --remote) will get a non-functional binding without a helpful warning. This isn't a correctness bug (the binding won't crash — the remote proxy client just won't connect), but it's a missing user experience improvement compared to other always-remote bindings.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

});
}

addBinding(flagshipBinding.binding, "Flags", "flagship", envName);
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.

🚩 Type generation uses 'Flags' as the generated TypeScript type

The type generation at packages/wrangler/src/type-generation/index.ts:1834 and packages/wrangler/src/type-generation/index.ts:2763 uses "Flags" as the TypeScript type name for flagship bindings. This will generate FLAGS: Flags in the user's worker-configuration.d.ts. This type needs to exist in @cloudflare/workers-types for the generated types to compile correctly. If Flags is not yet exported from workers-types, users would get a compile error after running wrangler types. This may already be handled in a separate workers-types PR, but is worth verifying.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

Also, from the point of view of working well with the ecosystem, I would say that Flags as a global ambient type is a bit to general. It could easily conflict with something in the future. I would push for something a bit more specific, e.g. CloudflareFlags or Flagship or something. cc @danlapid

Comment on lines +462 to +473
if (flagship.length > 0) {
output.push(
...flagship.map(({ binding, app_id }) => {
return {
name: binding,
type: getBindingTypeFriendlyName("flagship"),
value: app_id,
mode: getMode({ isSimulatedLocally: false }),
};
})
);
}
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.

🚩 Flagship binding always shown as remote in print-bindings, unlike vpc_services which checks remote flag

In packages/wrangler/src/utils/print-bindings.ts:462-473, the flagship binding unconditionally uses getMode({ isSimulatedLocally: false }), meaning it always displays as remote mode. By contrast, other always-remote bindings like vpc_services (print-bindings.ts:370-379) still check remote && !context.remoteBindingsDisabled before deciding the mode. This is technically correct since the CfFlagship interface (packages/workers-utils/src/worker.ts:256-259) doesn't have a remote field, so there's no flag to check. However, this means the binding will show as remote even when remoteBindingsDisabled is true in context (e.g. --local mode), which could be confusing in the output since other always-remote bindings respect that context flag.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +358 to +363
case "flagship": {
for (const { binding, ...x } of info) {
output[binding] = { type: "flagship", ...x };
}
break;
}
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.

🚩 No pages guard for flagship in convertConfigToBindings

In packages/wrangler/src/api/startDevWorker/utils.ts:358-363, the flagship case does NOT have a if (pages) { break; } guard, unlike some other bindings (e.g., unsafe_hello_world at line 349-351, pipelines at line 335-336). This means flagship bindings would be processed even in Pages projects. Other always-remote bindings like secrets_store_secrets (line 343) also lack the pages guard, so this appears intentional. If flagship should NOT be available in Pages, this would need to be added.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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

Labels

blocked Blocked on other work

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

3 participants