feat(deploy): provider contract + wrangler reference provider

[runleveldev]

What & why

Addresses the control-plane integration seam from #5. Establishes a provider-agnostic deploy contract so @mieweb/cli can drive any deploy backend, with Cloudflare/wrangler as the reference implementation and opensource-server (os.mieweb.org) as a future provider — without either backend leaking into the shared interface.

This mirrors the data-plane design already in the repo: just as @mieweb/cloud defines Cloudflare-shaped binding contracts that @mieweb/cloud-adapters implement, this adds a control-plane contract that deploy providers implement.

@mieweb/deploy-contract   (this repo — the interface, zero deps)
        ▲                    ▲                         ▲
 implements              consumes                  implements
        │                    │                         │
@mieweb/deploy-wrangler   @mieweb/cli          opensource-server's provider
(reference)                                     (Manager API, details private)

Packages

@mieweb/deploy-contract (new — types + tiny runtime, zero deps)

• DeployProvider interface. deploy is the only required verb; dev/tail/destroy/login/logout/whoami are optional (CLI degrades with a clear "unsupported by provider" message).
• Neutral DeployContext (a projection of the CLI's config incl. the resolved manifestPath), DeployResult, and ResourceHandle whose id is opaque. No wrangler.jsonc field names or backend shapes leak in.
• Auth surface: AuthError (the control-plane analogue of UnsupportedBindingError) + AuthStatus. Credentials never travel through the contract in a serializable form — providers read them from the environment via createProvider(env); targetConfig is documented as non-secret.
• RESOURCE_KINDS is the single source of truth; the ResourceKind type is derived from it so the type and the runtime allowlist can't drift.
• Loads under bare node and strict node16/nodenext (runtime values ship as ESM, declarations as .d.mts). Ships a shared string-aware ./jsonc parser and a ./testkit conformance suite (runProviderConformance — the control-plane analogue of @mieweb/test-app).

@mieweb/deploy-wrangler (new — the Cloudflare reference provider)

• Wraps the project's wrangler binary; deploy/dev/tail + login/logout/whoami map to their wrangler equivalents. Forwards passthrough argv and a resolved --config (respects a user-supplied one, no duplicate).
• Reads resource handles from the manifest, reloaded from disk after deploy so wrangler's written-back ids surface (vectorize via index_name, queues via queues.producers; the AI object binding is intentionally omitted — no provisioned identity).
• TTY-safe auth classification: deploy/dev/tail preserve wrangler's interactive stdio. The failed verb's own stderr is the authority when captured (non-interactive/CI) — catching a 401/403 for a valid-but-forbidden token — with a strict, marker-only wrangler whoami probe as the interactive-TTY fallback. Ambiguous/network/signal failures are reported generically, never mislabeled as auth. Captured stderr is bounded (64 KiB) so long-running tail can't leak memory.
• Signal-killed child = failure (not success); dev exposes a closed promise so a crashed dev surfaces instead of hanging; Ctrl-C of interactive tail/dev is a clean stop.
• Runner resolved from the active package manager (npm/pnpm/yarn/bun, npx fallback); missing wrangler raises an actionable prerequisite error. wrangler is an optional peer dependency (with the MIEWEB_REAL_WRANGLER escape hatch).
• // @ts-check + @type {DeployProvider} make tsc --noEmit fail on contract drift despite the repo's global checkJs: false.

@mieweb/cli (wired to the contract)

• deploy/dev/tail/login/logout/whoami/destroy route through a resolved DeployProvider. Cloudflare resolves to the wrangler reference provider; other targets can name a provider package in mieweb.jsonc (targets[t].provider). Targets without a provider fall through to existing behavior unchanged.
• Provider resolution uses Node's ESM resolver rooted at the project (so an app-installed provider is found), with a CJS fallback; relative/absolute (incl. Windows) paths resolve against the project root.
• Secret boundary: targetConfig and the mieweb view are recursively redacted before reaching a provider — the data-plane bindings bag and any secret-bearing key at any depth are dropped. Deploy credentials come only from the environment.

On self-hosted / multi-instance backends (from the #5 discussion)

No contract change is needed to point at a self-hosted opensource-server instance. Instance URL is location, not identity — a provider sources it from targetConfig / env / an interactive login, and caches it machine-local (mirroring wrangler's OAuth cache). login is deliberately unspecified about what it acquires, which is exactly what lets a multi-instance backend slot in without the single-control-plane assumption touching the interface.

Testing

• tsc --noEmit clean across the workspace (contract .ts + @ts-check-ed provider/CLI .mjs; verified against a strict nodenext consumer).
• @mieweb/deploy-wrangler unit + hermetic fake-wrangler tests (10) cover the subprocess paths: argv/--config forwarding, resource extraction + manifest reload, auth vs generic vs valid-token-403 classification, whoami reporting, and dev termination.
• End-to-end verified via a fake wrangler across every verb and both degradation paths.

Follow-ups (not in this PR)

• CLI write-back of provider-returned ResourceHandle.ids into wrangler.jsonc (provider already surfaces them; CLI reports them today).
• The opensource-server provider (login/whoami/deploy against the Manager API) as a separate package, measured against the shipped test-kit.
• The mieweb/local host-harness dev path can migrate behind a provider later; left as-is to keep this focused on the contract seam.

Closes part of #5 (control-plane contract).

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings affect runtime compatibility, installation, resource reporting, authentication, and process termination.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Introduces a provider-agnostic deployment contract, Wrangler reference provider, and CLI provider routing.

Changes:

• Adds deploy/auth contracts and conformance utilities.
• Implements Wrangler-backed deploy, development, tail, and authentication commands.
• Routes configured provider operations through the CLI.

File summaries

File	Review summary
pnpm-lock.yaml	Registers workspace packages; no findings.
packages/deploy-wrangler/src/index.test.mjs	Adds provider and conformance tests; no findings.
packages/deploy-wrangler/src/index.mjs	Critical: reload the manifest after deployment and propagate child termination. Moderate: map deploy auth failures to AuthError, read vectorize.index_name, and treat signal exits as failures.
packages/deploy-wrangler/package.json	Critical: add the pinned Wrangler dependency and lockfile entry, or document it as an external prerequisite.
packages/deploy-contract/src/testkit.ts	Adds provider conformance utilities; no findings.
packages/deploy-contract/src/index.ts	Moderate: avoid exposing NodeJS.ProcessEnv without declaring Node typings.
packages/deploy-contract/package.json	Critical: publish executable JavaScript and declarations, or document compatible runtime loader requirements.
packages/cli/src/provider.mjs	Moderate: resolve relative providers against config.root and handle already-aborted signals.
packages/cli/src/index.mjs	Integrates provider routing; no findings.
packages/cli/package.json	Adds provider dependencies; no findings.
.changeset/deploy-provider-contract.md	Documents package changes; no findings.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (2)

packages/deploy-contract/src/index.ts:298

• This public type exposes NodeJS.ProcessEnv, but the package declares zero runtime/type dependencies and does not declare @types/node. A TypeScript consumer that does not otherwise install Node typings cannot resolve the NodeJS namespace, defeating the provider-agnostic contract. Use a dependency-free environment record type (or make Node typings an explicit package dependency).

export type DeployProviderFactory = (env: NodeJS.ProcessEnv) => DeployProvider;

packages/deploy-wrangler/src/index.mjs:98

• The test-app and Wrangler manifests use vectorize entries with index_name, not id (for example packages/test-app/wrangler.jsonc:25). Reading entry.id therefore returns an empty handle, so the CLI omits this resource and cannot persist/reuse its identity. Read the manifest's actual identity field here.

  collect('vectorize', 'vector', 'id');

• Files reviewed: 10/11 changed files
• Comments generated: 8
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain, including manifest-path handling and JSONC corruption.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (5)

packages/cli/src/provider.mjs:85

• A configured provider package is imported using the resolver for packages/cli/src/provider.mjs, not the project's config.root. Therefore a provider installed in the consuming project's node_modules and named in targets.<target>.provider cannot be found unless it happens to be hoisted into the CLI's dependency tree. Resolve package specifiers with a project-root search path before importing.

  const mod = await import(importable);

packages/deploy-contract/src/index.ts:196

• The TypeScript entry re-exports AuthError from ./runtime.mjs, but the companion declaration is named runtime.d.ts. TypeScript's ESM extension substitution for an .mjs import looks for runtime.mts/runtime.d.mts, not runtime.d.ts, so consumers resolving the package's types condition cannot use the declared AuthError shape reliably. Rename the declaration to runtime.d.mts (or otherwise make the declaration path match the .mjs import).

export { AuthError } from './runtime.mjs';

packages/deploy-contract/src/testkit.mjs:176

• The conformance kit is described as provider-agnostic, but its live idempotency check hard-codes Wrangler arrays and ID fields when merging the first result back into the manifest. A provider with a different manifest shape cannot receive the opaque IDs on the second run, producing a false failure (or bypassing the required check). Make the round-trip update provider-supplied or remove this Wrangler-specific simulation from the generic kit.

function mergeIdsIntoManifest(manifest, resources) {
  /** @type {Record<string, unknown>} */
  const clone = structuredClone(manifest);
  /** @param {string} binding */
  const idFor = (binding) => resources.find((r) => r.binding === binding)?.id;
  /** @param {string} key @param {string} idField */
  const patchArray = (key, idField) => {
    const arr = clone[key];
    if (!Array.isArray(arr)) return;
    for (const entry of arr) {
      if (entry && typeof entry === 'object' && typeof entry.binding === 'string') {
        const id = idFor(entry.binding);
        if (id !== undefined) entry[idField] = id;

packages/deploy-wrangler/src/index.mjs:54

• When MIEWEB_REAL_WRANGLER is unset, the provider hardcodes pnpm exec wrangler. The generated project instructions explicitly allow npm install, and an npm consumer can have the declared optional wrangler peer installed while not having pnpm; such deployments fail before the provider can invoke wrangler. Resolve the local executable/package-manager runner independently of the workspace's pnpm command.

  const useRunner = !real;
  const cmd = useRunner ? 'pnpm' : real;
  const finalArgs = useRunner ? ['exec', 'wrangler', ...args] : args;

packages/deploy-wrangler/src/index.mjs:296

• A wrangler process killed by a signal returns { code: null, signal: ... }, but this method discards signal and reports it as a normal unauthenticated status. mieweb whoami can therefore print 'Not authenticated' and exit 1 for interruption or process failure instead of surfacing the failure. Treat a signal result as an error before mapping the exit code to authenticated.

    const { code } = await runWrangler(['whoami'], { cwd: context.root, signal: context.signal });
    // Env-token auth vs. the OAuth cache both satisfy wrangler; report the
    // dominant method for diagnostics without asserting which one wrangler used.
    const method = process.env.CLOUDFLARE_API_TOKEN || process.env.CLOUDFLARE_API_KEY ? 'env' : 'oauth';
    return { authenticated: code === 0, method: code === 0 ? method : undefined };

• Files reviewed: 13/16 changed files
• Comments generated: 4
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Wrangler tail cancellation, authentication translation, and logger-routing issues remain unresolved.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

packages/deploy-wrangler/src/index.mjs:350

• wrangler tail can fail with the same 401/403 authentication errors as deploy, but this path always calls assertOk and therefore bypasses the provider's AuthError handling. The CLI will show a generic exit-code failure instead of the actionable login hint promised by the contract; capture stderr and translate explicit auth markers before preserving other failures.

This issue also appears on line 364 of the same file.

packages/deploy-wrangler/src/index.mjs:350

• When the CLI receives Ctrl-C, context.signal aborts runWrangler; Node reports that cancellation as an AbortError, which rejects this method and is then converted by runProviderVerb into exit code 1. tail is an interactive long-running command, so normal user cancellation should return successfully (as dev already does) rather than print an error/failure; swallow the abort only when context.signal.aborted, while preserving genuine child failures.

    const result = await runWrangler(['tail', ...configFlag(context), ...context.argv], {
      cwd: context.root,
      signal: context.signal,
    });
    assertOk(result, 'tail');

packages/deploy-wrangler/src/index.mjs:368

• This maps every non-zero wrangler whoami exit to authenticated: false. Network/DNS failures and invalid configuration also commonly produce non-zero exits, so mieweb whoami will falsely report that the user is logged out instead of surfacing that status could not be determined. Capture/classify the result like deploy: return false only for explicit auth failures and preserve other errors.

    const result = await runWrangler(['whoami'], { cwd: context.root, signal: context.signal });
    // A signal kill (code === null) is a failure to *determine* status, not a
    // "not authenticated" answer — surface it rather than reporting unauthed.
    if (result.code === null) {
      throw new Error(`wrangler whoami was terminated by signal ${result.signal}`);

packages/deploy-wrangler/src/index.mjs:64

• The contract requires providers to route human-facing output through DeployContext.logger (packages/deploy-contract/src/index.ts:41-43), but this subprocess uses inherited stdio and the capture path later writes directly to process.stderr. That bypasses the CLI's logging/formatting and makes provider output impossible for callers and tests to capture through the contract; either route the child streams through the logger or make subprocess I/O an explicit exception in the contract.

      stdio: capture ? ['inherit', 'inherit', 'pipe'] : 'inherit',
      signal: opts.signal,

• Files reviewed: 15/17 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved moderate issues affect provider resolution, authentication handling, and conformance validation.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (5)

packages/cli/src/provider.mjs:85

• On Windows, an absolute configured provider path such as C:\\work\\provider.mjs does not start with . or /, so it is incorrectly treated as a package specifier and the import fails. Use path.isAbsolute(specifier) for this branch (and import it) alongside the relative-path check.

  if (specifier.startsWith('.') || specifier.startsWith('/')) {

packages/deploy-contract/src/index.ts:184

• This documentation says credentials never travel through the contract and must be read from the provider environment, but DeployContext.targetConfig is passed to providers verbatim and its own contract docs explicitly allow deploy-time credentials there. That contradiction leaves provider authors without a reliable secret-handling boundary; either remove credentials from the context or document and constrain their handling in targetConfig.

 * Credentials themselves NEVER travel through the contract in a form that could
 * be logged or serialized — a provider reads them from the environment at
 * construction time (see {@link DeployProviderFactory}). This type only reports
 * *whether* the provider is authenticated and, optionally, a non-secret label

packages/deploy-contract/src/index.ts:24

• This contract documentation promises that the CLI persists opaque resource IDs back into config, but runProviderVerb currently only prints them (packages/cli/src/provider.mjs:276-284); the PR description also lists write-back as a follow-up. Please describe these handles as reported-only (or implement the write-back before publishing this contract), and update the corresponding ResourceHandle/DeployResult wording as well so provider authors are not relying on behavior that does not exist.

 * resource identity carried as an **opaque** {@link ResourceHandle.id} that the
 * CLI persists back into the config verbatim (short-circuiting the next
 * provision) without ever interpreting it.

packages/deploy-contract/src/testkit.mjs:75

• The conformance kit claims to validate ResourceKind, but this branch only checks that kind is any string. A provider returning kind: 'bogus' therefore passes the result-shape check even though it violates the contract's closed ResourceKind union. Validate against all declared resource kinds here.

    if (typeof res?.kind !== 'string') {
      return `resources[${i}].kind must be a ResourceKind string`;
    }

packages/deploy-wrangler/src/index.mjs:350

• tail sends all non-zero exits through assertOk, so an unauthenticated/forbidden tail is reported as a generic exit-code error and never becomes AuthError; the CLI therefore cannot show the provider's login hint. Capture and classify tail's stderr with the same auth mapping used by deploy before calling assertOk.

    const result = await runWrangler(['tail', ...configFlag(context), ...context.argv], {
      cwd: context.root,
      signal: context.signal,
    });
    assertOk(result, 'tail');

• Files reviewed: 15/17 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Unresolved moderate issues affect provider routing, validation, argument forwarding, and error handling.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (10)

Previously missed (2) — in code that hasn't changed since the last review.

packages/deploy-contract/src/testkit.mjs:73

• This conformance check accepts any string as kind, so a provider returning an invalid value such as "unknown" passes despite ResourceKind being a closed union. Validate against the allowed resource kinds here; otherwise the advertised provider conformance suite does not catch malformed DeployResults.
  packages/deploy-wrangler/src/jsonc.mjs:18
• This is a second, verbatim implementation of the JSONC parser already maintained in packages/cli/src/jsonc.mjs. Keeping the same parser in two packages means fixes for comments, escapes, or trailing commas can diverge and produce different config behavior; move the implementation to a shared runtime utility instead of duplicating it.

packages/cli/src/index.mjs:105

• The new comment documents only deploy/dev/tail, but PROVIDER_VERBS immediately below also routes login, logout, and whoami through the provider layer. Please keep this comment aligned with the actual dispatch set so future changes do not overlook the authentication verbs.

  // Deploy-contract verbs (`deploy`/`dev`/`tail`) route through a DeployProvider
  // when one resolves for the active target. Cloudflare resolves to the wrangler
  // reference provider; other targets can name a provider package in
  // mieweb.jsonc (`targets[t].provider`). Everything else (d1 migrations, etc.)
  // and any target without a provider falls through to the legacy paths below.

packages/cli/src/index.mjs:29

• DeployProvider explicitly exposes an optional destroy verb, but this routing set omits it (and runProviderVerb has no destroy branch). As a result, mieweb destroy can never reach a configured provider's implementation and instead falls through to legacy handling. Add destroy to the provider verb path and handle its optional/unsupported case there.

const PROVIDER_VERBS = new Set(['deploy', 'dev', 'tail', 'login', 'logout', 'whoami']);

packages/deploy-contract/src/index.ts:136

• This documentation promises that the CLI persists returned resource IDs, but the current CLI only prints them in reportDeploy; the PR description also lists write-back as a follow-up. That mismatch gives provider authors a false contract and makes the next-deploy short-circuit claim incorrect. Describe this as reporting/future persistence, or implement the write-back before documenting it as guaranteed.

 * A resource the provider ensured exists during a deploy. The CLI reports these
 * to the user and persists {@link id} back into the committed config so the
 * next deploy short-circuits provisioning for that binding.

packages/deploy-contract/src/testkit.mjs:133

• The idempotency branch never validates the second deployment result. For example, if the first result has no resources and the second call returns {}, again.resources ?? [] is also empty and the idempotency check passes even though the provider violated the DeployResult contract. Validate again with validateResult before comparing resource ids.

        const again = await provider.deploy(second);
        const stable =
          JSON.stringify(idset(first.resources ?? [])) === JSON.stringify(idset(again.resources ?? []));

packages/deploy-wrangler/src/index.mjs:90

• The promise resolves on the child's exit event even though the captured stderr stream may still have buffered data; Node emits close only after stdio has closed. An auth message can therefore arrive after this resolves, causing isAuthFailure to miss it and report a generic exit-code error instead of AuthError. Resolve on close so the captured stderr is complete.

    child.on('exit', (code, signal) => resolvePromise({ code, signal, stderr }));

packages/deploy-wrangler/src/index.mjs:420

• When the CLI receives SIGINT/SIGTERM during login, the context signal makes spawn reject with AbortError before runWrangler returns a { code } result, so this method skips its AuthError conversion and the CLI reports a generic abort error. Handle an aborted context here consistently with the documented cancelled-login path.

    const { code } = await runWrangler(['login', ...context.argv], {
      cwd: context.root,
      signal: context.signal,
    });
    if (code !== 0) {
      throw new AuthError('wrangler', context.target, 'wrangler login was cancelled or failed');

packages/deploy-wrangler/src/index.mjs:383

• DeployContext.argv is defined as the arguments following the verb, and the other provider verbs forward it, but whoami silently drops it here. Flags such as Wrangler's output/account options cannot be used through mieweb whoami; append ...context.argv to preserve the provider contract.

    const result = await runWrangler(['whoami'], {

packages/deploy-wrangler/src/index.mjs:437

• This invocation also drops all arguments supplied after mieweb logout, despite DeployContext.argv being the verb's passthrough argument list. Forward ...context.argv so the wrapper does not silently change the underlying Wrangler command.

    const result = await runWrangler(['logout'], { cwd: context.root, signal: context.signal });

• Files reviewed: 15/17 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Resolve the lockfile inconsistency and the Wrangler tail/login handling issues; consolidate the duplicated parser when practical.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (3)

packages/deploy-wrangler/src/index.mjs:357

• tail is intentionally long-running, but capture: true makes runWrangler append every stderr chunk to one unbounded stderr string for the entire session while also teeing it to the terminal. A busy tail can therefore grow the CLI's memory usage without limit even though the buffer is only needed to classify an eventual auth failure. Keep only a bounded diagnostic suffix (or otherwise stop accumulating after enough auth evidence) for this path.

        capture: true,

packages/deploy-wrangler/src/index.mjs:420

• When the CLI's context.signal is aborted (for example, Ctrl-C during the browser flow), spawn rejects runWrangler with AbortError before it returns a non-zero code, so this branch is never reached. The CLI then reports a generic abort instead of the documented cancelled-login AuthError; handle an aborted runWrangler here and translate it to the same AuthError as other login failures.

    const { code } = await runWrangler(['login', ...context.argv], {
      cwd: context.root,
      signal: context.signal,
    });
    if (code !== 0) {
      throw new AuthError('wrangler', context.target, 'wrangler login was cancelled or failed');

packages/deploy-wrangler/src/jsonc.mjs:18

• This is a second full copy of the CLI's JSONC parser (packages/cli/src/jsonc.mjs). The same trailing-comma bug already had to be fixed in both copies; keeping the implementations duplicated means future parser fixes can silently diverge between config loading and post-deploy manifest reload. Extract the parser into one shared runtime utility (or expose one package's utility) and have both callers use it.

export function parseJsonc(text) {

• Files reviewed: 15/17 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved manifest handling, secret projection, schema, parser, declaration, and coverage issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

packages/cli/src/provider.mjs:134

• The contract documents targetConfig as non-secret, but this projection forwards the entire config.targetConfig object to every provider. Existing target configs can contain credentials such as authToken, accessKeyId, and secretAccessKey (see packages/cli/mieweb-config.schema.json:85-111 and packages/test-app/mieweb.jsonc:35-44), so a control-plane provider can receive and accidentally log/serialize data the contract promises never travels through it. Pass a sanitized provider-specific projection or move data-plane secrets out of the context before invoking providers.

packages/cli/src/index.mjs:106

• This routing changes the documented Cloudflare behavior: deploy, dev, and tail no longer use the legacy verbatim delegateToWrangler path; the provider adds its own logging, config handling, stderr capture, and resource reporting, and adds new auth verbs. packages/cli/README.md:3-6 and README.md:49-56 still promise that every Cloudflare command is forwarded verbatim with zero overhead, so update the user-facing documentation in this PR.

  // Deploy-contract verbs route through a DeployProvider when one resolves for
  // the active target: deploy, dev, tail, login, logout, whoami, destroy (see
  // PROVIDER_VERBS). Cloudflare resolves to the wrangler reference provider;
  // other targets can name a provider package in mieweb.jsonc
  // (`targets[t].provider`). Everything else (d1 migrations, etc.) and any
  // target without a provider falls through to the legacy paths below.

packages/deploy-contract/src/index.ts:40

• DeployTarget is intentionally open-ended here, but the shipped packages/cli/mieweb-config.schema.json still restricts the top-level target property to the five built-in enum values. A project using a new provider target in mieweb.jsonc will therefore be rejected by the schema even though this contract and the provider resolver advertise arbitrary target names; update the schema/type boundary together so custom providers are usable through normal config validation.

/**
 * Which platform a deploy targets. Open-ended on purpose: the contract does not
 * enumerate a closed set, so new providers can introduce their own target names
 * and advertise them via {@link DeployProvider.supports}. The well-known values
 * mirror `CloudTarget` in `@mieweb/cloud`.
 */
export type DeployTarget = 'cloudflare' | 'local' | 'mieweb' | 'aws' | 'gcp' | (string & {});

packages/deploy-contract/src/jsonc.mjs:60

• This parser silently changes malformed JSONC: removing a block comment without inserting whitespace turns token-separated text such as 1/*comment*/2 into 12, and an unterminated /* after valid JSON is accepted because the scan reaches EOF and still calls JSON.parse. Treat block comments as whitespace and throw when no closing */ is found; this shared parser now feeds both CLI configuration and manifest reloads.

    if (ch === '/' && next === '*') {
      // Block comment: skip to closing */.
      i += 2;
      while (i < n && !(text[i] === '*' && text[i + 1] === '/')) i += 1;
      i += 2;
      continue;

packages/deploy-contract/src/runtime.d.mts:1

• This published declaration imports a .ts extension. Consumers that do not enable allowImportingTsExtensions can fail with TS5097 while resolving the package, even though the workspace's own tsconfig.json enables that option. Use a publishable declaration specifier such as ./index.js (TypeScript will apply extension substitution) instead.

import type { DeployTarget } from './index.ts';

packages/deploy-contract/src/testkit.mjs:145

• Because applyIds is optional, a live conformance run can report success after checking only the first deploy's result shape; the central idempotency obligation is silently skipped when the hook is absent. Require the hook for live conformance or record an explicit failed/skipped check, so a passing report cannot be mistaken for full provider conformance.

      if (typeof opts.applyIds === 'function') {
        const merged = opts.applyIds(opts.manifest, first.resources ?? []);

packages/deploy-wrangler/src/index.mjs:288

• The provider tests only exercise exports, supports(), optional-method presence, and the conformance kit's structural checks; they never execute this subprocess path. That leaves the reference implementation's argument passthrough, manifest reload, auth classification, signal handling, and login/logout/whoami behavior untested, despite these branches being the new CLI integration seam. Add hermetic tests using a fake Wrangler executable for the success and failure cases described by the PR.

   * ensures resources exist (native provisioning), and pushes the worker; we
   * then reload the (possibly written-back) manifest and surface its resources
   * as neutral handles. Idempotent because wrangler's own deploy is.
   *
   * A genuine auth failure (explicit 401/403 / "not authenticated" in wrangler's
   * output) is mapped to {@link AuthError} so the CLI can offer the login hint.

• Files reviewed: 16/19 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Critical credential exposure and manifest-path handling issues, plus missing Wrangler subprocess coverage, remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (3)

packages/cli/src/provider.mjs:131

• loadConfig always assigns wranglerPath to the default absolute path even when that file does not exist, while buildContext forwards it unconditionally. configFlag() then forces every provider invocation to use --config <nonexistent path> instead of preserving the documented undefined path for synthesized/discovered manifests, which can break projects that rely on Wrangler's normal discovery or an alternate manifest. Only set manifestPath after confirming the resolved file exists (or carry an explicit discovered path).

    manifestPath: config.wranglerPath,

packages/deploy-contract/src/testkit.mjs:61

• This duplicates the complete ResourceKind union from src/index.ts as a second runtime allowlist. The comment's “kept in sync” requirement is a future failure point: adding a valid kind to the contract will make every provider conformance run reject it until this unrelated list is also edited. Define the runtime kind list once and derive/reuse the type and test validation from that source.

const RESOURCE_KINDS = new Set([

packages/deploy-wrangler/src/index.test.mjs:41

• This is only a structural conformance run (live is omitted), so it never invokes the reference provider's deploy or exercises argument passthrough, manifest reload/resource extraction, or auth-failure mapping. The only live case uses a stub provider, meaning these core behaviors can regress while this package's tests remain green; add hermetic fake-wrangler coverage for the reference provider's subprocess paths.

test('passes the contract conformance test-kit (structural)', async () => {
  const report = await runProviderConformance(wranglerProvider, {
    target: 'cloudflare',
    manifest: {
      name: 'demo',
      d1_databases: [{ binding: 'DB', database_id: 'abc-123' }],
      r2_buckets: [{ binding: 'RECORDINGS', bucket_name: 'demo-recordings' }],
      kv_namespaces: [{ binding: 'SESSIONS', id: 'kv-1' }],
    },
  });

• Files reviewed: 16/19 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved critical findings affect credential safety and conformance execution, with additional auth handling and coverage gaps.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (2)

packages/deploy-wrangler/src/index.mjs:345

• The dev path never captures or classifies stderr, so a remote/credentialed wrangler dev that exits on a 401/403 rejects closed with a generic exit error instead of the contract-required AuthError and login hint. Preserve the live stderr tee while applying the same explicit auth-marker mapping used by deploy/tail before the generic failure path.

    const done = runWrangler(['dev', ...configFlag(context), ...context.argv], {
      cwd: context.root,
      signal: controller.signal,
    }).then(
      (result) => {
        // A clean exit (code 0) or a stop()-triggered abort is fine; anything
        // else is a dev failure the CLI should learn about via `closed`.
        if (result.code === 0 || controller.signal.aborted) return;
        throw new Error(
          result.signal
            ? `wrangler dev was terminated by signal ${result.signal}`
            : `wrangler dev exited with code ${result.code}`,
        );
      },
      (err) => {
        if (err?.name === 'AbortError') return; // expected on stop()
        throw err;

packages/deploy-wrangler/src/index.test.mjs:33

• This is the only conformance run for the reference provider and it leaves live false, so it never executes the provider's subprocess, manifest reload, auth classification, or resource-handle code; the bogus-resource test exercises only the kit's validator. Add fake-wrangler coverage for those paths so the high-risk delegation behavior is actually tested.

test('passes the contract conformance test-kit (structural)', async () => {
  const report = await runProviderConformance(wranglerProvider, {

• Files reviewed: 19/22 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Two moderate findings and documentation nits remain unresolved.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (4)

README.md:58

• This paragraph is now too broad: a custom target can set targets[t].provider, in which case provider verbs such as dev no longer run the Node host harness. Qualify the host-harness description to apply only to the built-in local/mieweb targets when no deploy provider is configured, otherwise the README contradicts the routing added here.

**On the other targets it is not a wrapper** — there is no `wrangler`
underneath; the CLI runs your unchanged worker on a Node host harness backed by
the adapters, reusing your `wrangler.jsonc` purely as configuration. The active

packages/cli/README.md:10

• This still states that every local/mieweb invocation uses the host harness, but a configured targets[t].provider now routes provider verbs away from it. Qualify this description for the no-provider case so users of the new custom-provider mechanism are not given the wrong execution model.

On `local`/`mieweb` the CLI runs your unchanged worker on the Node host harness
backed by the matching adapters. See the [root README](../../README.md) for the

packages/cli/src/provider.mjs:165

• This passes the entire parsed mieweb.jsonc into every provider. Because config.raw contains all targets, it exposes data-plane bindings and their authToken/accessKeyId/secretAccessKey values despite sanitizing targetConfig on the next line, defeating the contract's credential boundary. Project mieweb to a non-secret provider view (or omit it) before constructing the context.

    mieweb: /** @type {Record<string, unknown>} */ (config.raw),

packages/deploy-contract/src/testkit.mjs:179

• The idempotency assertion compares only binding and id, so a provider can return the same opaque id with a different kind on the second deploy and still pass conformance. Include kind in the comparison key; it is part of the ResourceHandle contract and a kind change is not an idempotent result.

  return resources
    .map((r) => /** @type {[string, string]} */ ([r.binding, r.id]))
    .sort((a, b) => a[0].localeCompare(b[0]));

• Files reviewed: 19/22 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Address the secret-redaction gap, TTY authentication handling, and missing type-check coverage.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (2)

packages/deploy-wrangler/src/index.mjs:91

• runWrangler disables stderr capture whenever the parent has a TTY, but whoami (and the auth mapping for deploy/tail) relies exclusively on the captured stderr markers. In an interactive shell an unauthenticated wrangler whoami therefore returns the generic “could not determine auth status” error instead of { authenticated: false }, and deploy auth failures cannot become AuthError; preserve TTY output while teeing/capturing stderr for these auth-sensitive calls, or use a reliable exit-code classification.
  packages/deploy-wrangler/src/index.mjs:24
• The repository's root tsconfig has checkJs: false, so the JSDoc annotations in this .mjs provider are not type-checked by tsc --noEmit despite this documentation saying they keep the shapes checked. A provider can therefore drift from DeployProvider without CI detecting it; enable checking for these provider files or add a separate type-check fixture/assertion.

 * Authored as plain ESM with JSDoc types (no build step) to match the rest of
 * the mieweb CLI tooling; the `@mieweb/deploy-contract` types are referenced
 * via `import('...')` JSDoc so `tsc --noEmit` still checks the shapes.

• Files reviewed: 19/22 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Wrangler auth classification relies on a secondary whoami probe and can misclassify operation failures.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (1)

packages/deploy-wrangler/src/index.mjs:282

• failedDueToAuth still infers the failed deploy/dev/tail operation from a second whoami call. This misses a real 401/403 when the token is valid but lacks the operation's permission (whoami exits 0), and the empty-stderr fallback also turns an otherwise ambiguous/network failure into an AuthError. Preserve and classify the original verb's explicit auth response (while teeing interactive stderr), rather than using this probe as the authority.

    if (who.code === 0) return false; // authenticated → not an auth failure
    // Non-zero whoami: treat as auth only on an explicit marker (or an empty
    // stderr, which wrangler emits for a plain "not logged in").
    return isAuthFailure(who.stderr) || who.stderr.trim() === '';

• Files reviewed: 19/22 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Correct whoami failure classification so non-authentication failures are not hidden as AuthError.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (1)

packages/deploy-wrangler/src/index.mjs:285

• An empty whoami stderr is not positive evidence of being logged out: a signal-terminated probe or a transient/network failure can also produce no stderr. This causes deploy/dev/tail failures to be mislabeled as AuthError and hides the original failure; only classify an explicit auth marker (and reject/ignore signal-terminated probes).

    if (who.code === 0) return false; // authenticated → not an auth failure
    // Non-zero whoami: treat as auth only on an explicit marker (or an empty
    // stderr, which wrangler emits for a plain "not logged in").
    return isAuthFailure(who.stderr) || who.stderr.trim() === '';

• Files reviewed: 19/22 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Unresolved moderate issues remain in provider input validation, target lookup, TTY authentication handling, and resource extraction.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (4)

packages/cli/src/provider.mjs:208

• The allowlist alone does not make these values safe: loadConfig does not validate mieweb.jsonc against the schema, so a malformed file such as {"wrangler":{"apiToken":"..."}} is copied verbatim into DeployContext.mieweb and reaches a provider despite the non-secret-context contract. Only copy target/wrangler after validating their expected string shape (or redact their values recursively) before exposing them.

    if (k !== 'targets') {
      out[k] = v;
      continue;

packages/cli/src/provider.mjs:75

• target is intentionally an open-ended string, but this plain object lookup also returns inherited properties. A valid custom target named toString, constructor, or __proto__ makes specifier a function/object, and specifier.startsWith(...) then throws instead of falling through to the no-provider path. Restrict the lookup to own properties.

  const specifier = configured ?? BUILTIN_PROVIDERS[config.target];

packages/deploy-wrangler/src/index.mjs:542

• On an interactive terminal, runWrangler forces capture to false whenever process.stderr.isTTY is true (line 94), so this result.stderr is always empty. Consequently mieweb whoami reports an undetermined error instead of authenticated: false, and the TTY fallback in failedDueToAuth can never recognize an unauthenticated deploy. Allow the status/probe invocation to capture its output (or otherwise classify the TTY result) without changing the interactive stdio behavior of the main command.

    // Non-zero: only an explicit auth signal means "logged out".
    if (isAuthFailure(result.stderr)) {
      return { authenticated: false };

packages/deploy-wrangler/src/index.mjs:337

• This extraction omits durable_objects.bindings and container-backed bindings entirely, even though the contract defines stateful and container resource kinds and the manifest can contain those provisioned bindings. Deploy results are therefore incomplete, so the CLI cannot report (or later persist) handles for those resources; map the relevant manifest entries or explicitly narrow the provider contract and tests to the supported kinds.

  collect('d1_databases', 'database', 'database_id');
  collect('r2_buckets', 'bucket', 'bucket_name');
  collect('kv_namespaces', 'kv', 'id');
  collect('vectorize', 'vector', 'index_name');

• Files reviewed: 19/22 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain in provider resolution, redaction, and Wrangler auth handling.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (5)

packages/cli/src/provider.mjs:107

• This fallback is not limited to built-in providers as the comment claims. It also runs when a configured provider cannot be resolved from the project, and import(specifier) can then resolve a same-named dependency from the CLI package (for example @mieweb/deploy-wrangler) instead of the provider the project declared. Only use the CLI-relative fallback when no provider was configured and the specifier is the built-in mapping; otherwise surface the project-resolution error.

        importable = specifier; // last resort: CLI-relative (built-ins)

packages/cli/src/provider.mjs:75

• The schema now accepts arbitrary target strings, but this plain object lookup also exposes inherited properties. A target named toString or constructor makes specifier a function rather than a provider package name, so the next specifier.startsWith(...) throws before resolution. Use an own-property lookup (or a null-prototype map) for the built-in mapping.

  const specifier = configured ?? BUILTIN_PROVIDERS[config.target];

packages/deploy-wrangler/src/index.mjs:295

• On a TTY, runWrangler disables capture even when capture: true, so this whoami probe receives stderr === '' and isAuthFailure(who.stderr) can never see the explicit unauthenticated marker. An interactive unauthenticated deploy/dev/tail is therefore reported as a generic exit failure instead of AuthError, contrary to the documented TTY fallback; use a deliberately captured non-interactive probe while leaving the original verb's stdio inherited, or another marker-preserving mechanism.

    const who = await runWrangler(['whoami'], {
      cwd: context.root,
      signal: context.signal,
      capture: true,
    });

packages/deploy-wrangler/src/index.mjs:291

• The empty-stderr branch invokes whoami without knowing whether the failed verb's stderr was actually captured. In CI/non-TTY mode capture: true is active, so a failed deploy/dev/tail that exits non-zero with no stderr can be reclassified as AuthError solely because the follow-up whoami says unauthenticated; conversely, on a TTY the probe cannot capture its marker (see the capture guard above). Pass capture/TTY state explicitly and only use this probe when the original verb was not captured.

  // 1. Prefer the verb's own output when we have it (captures 401/403 including
  //    valid-token-but-forbidden, which a whoami probe cannot see).
  if (result.stderr && result.stderr.trim() !== '') {
    return isAuthFailure(result.stderr);
  }
  // 2. Interactive TTY: the verb's stderr wasn't captured. Probe whoami, but
  //    only trust an *explicit* not-authenticated marker.
  try {
    const who = await runWrangler(['whoami'], {

packages/deploy-wrangler/src/index.mjs:546

• The auth marker is checked before result.signal, so a wrangler whoami process that is terminated after emitting an auth-looking message can return authenticated: false instead of reporting the signal failure. Signal termination should take precedence over marker classification, consistent with the provider's stated failure semantics.

    if (isAuthFailure(result.stderr)) {
      return { authenticated: false };
    }
    // Anything else (signal kill, network, config) is an *undetermined* status.
    if (result.signal) {
      throw new Error(`wrangler whoami was terminated by signal ${result.signal}`);

• Files reviewed: 19/22 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Copilot review overview

🔵 Needs a closer look

Unresolved moderate findings affect secret redaction, authentication handling, type/runtime consistency, and retry-safe publishing.

Review effort: Lite
Findings: 3

Open (3)

• The mieweb view is documented as recursively redacted, but allowlisted non-targets values are…
• The redaction regex misses common hyphenated secret keys such as api-key, x-api-key, and…
• The live phase runs even when the structural checks already show that the provider does not support…

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Previously missed (1)

In code that hasn't changed since last review

Classify only explicit auth failures as AuthError

packages/​deploy-wrangler/​src/​index.mjs:576

This maps every non-zero wrangler login result to AuthError, including network/config failures and signal termination. That violates the provider's strict auth classification and makes the CLI tell users to re-authenticate when the OAuth flow merely failed; only an explicit authentication/authorization failure should use AuthError, while cancellation, signals, and other exits should remain generic failures.

[Copilot]

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Review effort: Lite
Findings: 4

Open (4)

• Untrusted PR job has package write permissions · New
• The mieweb view is documented as recursively redacted, but allowlisted non-targets values are…
• The redaction regex misses common hyphenated secret keys such as api-key, x-api-key, and…
• The live phase runs even when the structural checks already show that the provider does not support…

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Previously missed (1)

In code that hasn't changed since last review

Inline version stamping prevents local reproduction

.github/​workflows/​pr-preview.yml:65

The version-stamping implementation is embedded as inline Node code in the workflow, so the preview process cannot be reproduced locally and diverges from the repository's established thin-wrapper CI/release workflows (.github/workflows/ci.yml:1-3, release.yml:1-8). Move this orchestration into a checked-in script and invoke that script from Actions.