Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9894dd1de1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const matches = () => | ||
| Math.abs((amount(environment.dailyBudgetUsd) ?? NaN) - desired.dailyBudgetUsd) <= 0.00005 && | ||
| (!desired.defaultResourceLocation || text(environment.defaultResourceLocation) === desired.defaultResourceLocation) && | ||
| (!desired.placementMode || (text(environment.placementMode) || "off") === desired.placementMode); |
There was a problem hiding this comment.
Include placement mode in the deployment identity
When an existing Worker keeps the same Artifact and resources but changes placementMode, this code patches the environment and then calls ensureActiveDeployment; however, deploymentPrefix does not include placement mode, so currentMatchingDeployment can reuse the deployment created with the old setting. Since Smart Placement is applied as deployment metadata, toggling off/smart can report success without activating a new deployment; include placement mode in the deployment identity or otherwise force redeployment after this update.
Useful? React with 👍 / 👎.
| - Authentication precedence: `XAPI_KEY`, `XAPI_API_KEY`, then `~/.xapi/config.json`. Keys need `workers:read` and, for changes, `workers:write`, plus access to the target Worker. A scoped-out Worker can return 404. | ||
| - Production API host is `api.xapi.to`; testing uses `XAPI_API_HOST=api.test.xapi.to` (host only). Load secrets from the user's existing secure environment. Never print keys, include them in code/artifacts, or send the xAPI key to a public Worker URL or Cloudflare. Runtime application authentication is separate. | ||
| - Start with `workers get <worker-id>`, `workers capabilities`, and `workers resources list <worker-id> --env <environment>`. Read-only inspection needs no extra approval. Use existing user authorization for changes; don't expand cleanup from a test environment to production. | ||
| - Treat Worker execution and data placement separately. Worker code remains global. Use environment `defaultResourceLocation` only as the default for newly created D1/R2 resources and `placementMode: smart` only for Cloudflare Smart Placement. Never claim either setting migrates existing data. |
There was a problem hiding this comment.
Classify the bundled skill capability as feat(skill)
This changed line adds user-visible placement behavior and safety instructions to a bundled skill, but the commit is titled feat(workers): .... Reclassify the final commit/PR as feat(skill): ... or split the bundled-skill capability into a correctly scoped commit so Release Please produces the repository-required release-note classification.
AGENTS.md reference: AGENTS.md:L61-L62
Useful? React with 👍 / 👎.
|
Added compatibility for current Wrangler native bundles: validates and ignores diagnostic package_dependencies metadata, accepts only the declared static-assets binding, and continues to reject undeclared bindings or malformed metadata. Full CLI suite: 526 tests passed; typecheck passed. This was reproduced by packaging jarrodwatts/jev-trader with Wrangler 4.135.0. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6a3016695
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (existsSync(join(current, ".git"))) break; | ||
| const parent = dirname(current); | ||
| if (parent === current) break; | ||
| current = parent; |
There was a problem hiding this comment.
Stop at a package-local npm lockfile
When an existing app has its own package-lock.json but is nested inside a repository with a parent pnpm-lock.yaml, this upward search skips the local npm lock and selects pnpm. The initializer consequently rewrites xapi:build, the configured build command, and next steps to use pnpm even though the app is npm-managed; detect package-lock.json (and ideally the package's packageManager field) before ascending to an ancestor workspace.
Useful? React with 👍 / 👎.
|
按本轮 Workers 统一整理要求,由 Draft #38(feature/workers-refactor)接替,整合原生部署、Container、CLI 与 Skill。原改动和来源已保留并去重,placement deployment identity P1 继续在统一清单跟踪,不视为已解决。原分支不删除;没有 npm 发布或版本改动。 |
Problem
The CLI still sent complete Worker projects through the legacy JSON Artifact endpoint and could not manage Cloudflare-native environment placement. Users and agents therefore lacked a clear, single-project deployment flow for framework applications and APAC-oriented data resources.
Change
workers environmentfor budget, new-resource data location, and Smart PlacementValidation
bun install --frozen-lockfilebun run typecheckbun run buildbun run test— 522 tests passednpm pack --dry-run --ignore-scripts— 81 packaged filesgit diff --checkNo live Cloudflare or Dokploy changes were made.