[wrangler] Add PR metadata to preview deployments - #14966
Conversation
|
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| export function getRepositoryUrl(): string | undefined { | ||
| const repositoryUrl = | ||
| process.env.CI_PROJECT_URL ?? | ||
| process.env.CI_REPOSITORY_URL ?? | ||
| process.env.CIRCLE_REPOSITORY_URL ?? | ||
| process.env.BUILDKITE_REPO ?? | ||
| process.env.BITBUCKET_GIT_HTTP_ORIGIN ?? | ||
| process.env.BITBUCKET_GIT_SSH_ORIGIN ?? | ||
| process.env.REPOSITORY_URL; | ||
| if (repositoryUrl) { | ||
| return normalizeRepositoryUrl(repositoryUrl); | ||
| } | ||
|
|
||
| if (process.env.GITHUB_REPOSITORY) { | ||
| const githubServerUrl = process.env.GITHUB_SERVER_URL ?? "https://github.com"; | ||
| return normalizeRepositoryUrl( | ||
| `${githubServerUrl.replace(/\/$/, "")}/${process.env.GITHUB_REPOSITORY}` | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| execSync(`git rev-parse --is-inside-work-tree`, { stdio: "ignore" }); | ||
| return normalizeRepositoryUrl( | ||
| execSync(`git config --get remote.origin.url`).toString() | ||
| ); | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Release notes are missing for a user-visible change to published packages
No changeset file is added for this change to the published wrangler/deploy-helpers packages (new code added at packages/deploy-helpers/src/preview/shared.ts:92-216), so the new behaviour will not be released or announced.
Impact: The feature won't trigger a package release and users won't see it in the changelog.
Repository rule requiring a changeset for every published-package change
AGENTS.md states "All changes to published packages require a changeset" and "Never commit without changesets for user-facing changes". The .changeset/ directory currently only contains README.md and config.json, i.e. this PR adds none, while it modifies packages/deploy-helpers and packages/wrangler. Per the semver guidance in REVIEW.md, adding new capability (PR metadata annotations + new output row) would be a minor bump for wrangler.
Prompt for agents
This PR modifies published packages (packages/deploy-helpers and packages/wrangler) but contains no changeset file under .changeset/. Repository rules (AGENTS.md, CONTRIBUTING.md, REVIEW.md) require a changeset for every user-facing change to published packages. Add a changeset for the `wrangler` package describing the user-facing behaviour (preview deployments now record pull request metadata and the preview output shows the pull request link), classified as minor per the semver guidance, with no conventional-commit prefix in the title.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
|
|
||
| if (process.env.GITHUB_REPOSITORY) { | ||
| const githubServerUrl = process.env.GITHUB_SERVER_URL ?? "https://github.com"; |
There was a problem hiding this comment.
🟡 Newly added code is not formatted with the repository formatter
Several newly added lines are not formatted with the repository's formatter (e.g. the over-long line at packages/deploy-helpers/src/preview/shared.ts:106), so the automated formatting check will reject the change.
Impact: The formatting gate fails, blocking the change from being merged until it is reformatted.
Unformatted locations introduced by the diff
AGENTS.md requires "Format with oxfmt - run pnpm prettify in the workspace root before committing" and running pnpm check before pushing. Affected new lines include:
packages/deploy-helpers/src/preview/shared.ts:106andpackages/deploy-helpers/src/preview/shared.ts:181(lines exceed the print width and would be wrapped)packages/deploy-helpers/src/preview/preview.ts:452-453packages/wrangler/src/__tests__/preview.test.ts:74— thedescribe("getBranchName", ...)line is indented with an extra tab relative to its blockpackages/wrangler/src/__tests__/preview.test.ts:2584
Prompt for agents
Run `pnpm prettify` (oxfmt) at the workspace root and commit the result. Newly added lines in packages/deploy-helpers/src/preview/shared.ts (notably the GITHUB_SERVER_URL default assignment and the GitLab normalizedProjectUrl assignment), packages/deploy-helpers/src/preview/preview.ts (the deploymentPullRequestNumber assignment), and packages/wrangler/src/__tests__/preview.test.ts (the mis-indented `describe("getBranchName")` line and the repository_url expectation line) are not oxfmt-formatted and will fail `pnpm check`.
Was this helpful? React with 👍 or 👎 to provide feedback.
@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: |
d43d09c to
2587805
Compare
| const repositoryUrl = getRepositoryUrl(); | ||
| const pullRequest = getPullRequestMetadata(); |
There was a problem hiding this comment.
🟡 Local previews now send repository details even outside CI, unlike other automatically detected metadata
The repository address is attached to every preview deployment (getRepositoryUrl() at packages/deploy-helpers/src/preview/preview.ts:766) even when not running in CI, because it falls back to reading the local git remote, unlike the commit-based metadata which is only used in CI.
Impact: Developers running previews from their own machine unexpectedly upload their repository address with each deployment.
Inconsistency with the existing CI-metadata gating
Commit tag/message fallbacks are guarded by shouldUseCIMetadataFallback() (packages/deploy-helpers/src/preview/preview.ts:760-765), which requires CI=1/CI=true. getRepositoryUrl() (packages/deploy-helpers/src/preview/shared.ts:92-120) first checks CI env vars, but when none are present it shells out to git config --get remote.origin.url and returns the normalized remote, so local runs always populate workers/repository_url. The PR description states the metadata is added "when it can be detected from CI environment variables", so the unconditional git-remote fallback goes beyond the stated behavior. Consider gating the git fallback (or the whole call) behind shouldUseCIMetadataFallback().
Was this helpful? React with 👍 or 👎 to provide feedback.
2587805 to
136b13c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| export function getRepositoryUrl(): string | undefined { | ||
| const repositoryUrl = | ||
| process.env.CI_PROJECT_URL ?? | ||
| process.env.CI_REPOSITORY_URL ?? | ||
| process.env.CIRCLE_REPOSITORY_URL ?? | ||
| process.env.BUILDKITE_REPO ?? | ||
| process.env.BITBUCKET_GIT_HTTP_ORIGIN ?? | ||
| process.env.BITBUCKET_GIT_SSH_ORIGIN ?? | ||
| process.env.REPOSITORY_URL; | ||
| if (repositoryUrl) { | ||
| return normalizeRepositoryUrl(repositoryUrl); | ||
| } | ||
|
|
||
| if (process.env.GITHUB_REPOSITORY) { | ||
| const githubServerUrl = process.env.GITHUB_SERVER_URL ?? "https://github.com"; | ||
| return normalizeRepositoryUrl( | ||
| `${githubServerUrl.replace(/\/$/, "")}/${process.env.GITHUB_REPOSITORY}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔴 Repository and pull request detection silently fails when a CI variable is set but empty
The GitHub server address falls back to the default only when the variable is missing, not when it is present but blank (?? at packages/deploy-helpers/src/preview/shared.ts:106), so a blank value produces an invalid address that is discarded and no repository link is attached.
Impact: In environments (and in the new tests, which blank these variables) where such variables are defined but empty, the repository and pull request links are silently dropped from preview deployments.
Nullish coalescing vs empty-string env vars
process.env.GITHUB_SERVER_URL ?? "https://github.com" yields "" when the variable is set to an empty string, so the built URL is "/owner/repo", new URL throws inside normalizeRepositoryUrl (packages/deploy-helpers/src/preview/shared.ts:71-89) and undefined is returned. The new test helper clearPreviewMetadataEnvs in packages/wrangler/src/__tests__/preview.test.ts:42-63 uses vi.stubEnv(name, ""), which assigns empty strings rather than deleting the variables, so getRepositoryUrl test expecting https://github.com/cloudflare/workers-sdk (packages/wrangler/src/__tests__/preview.test.ts:123-129) will not get that value. The same ??-over-empty-string problem affects the source chains at packages/deploy-helpers/src/preview/shared.ts:93-100, :164, :196-200 — an earlier variable set to "" short-circuits and masks later, correctly populated variables. The pre-existing getBranchName uses || for exactly this reason (packages/deploy-helpers/src/preview/shared.ts:17-26).
| export function getRepositoryUrl(): string | undefined { | |
| const repositoryUrl = | |
| process.env.CI_PROJECT_URL ?? | |
| process.env.CI_REPOSITORY_URL ?? | |
| process.env.CIRCLE_REPOSITORY_URL ?? | |
| process.env.BUILDKITE_REPO ?? | |
| process.env.BITBUCKET_GIT_HTTP_ORIGIN ?? | |
| process.env.BITBUCKET_GIT_SSH_ORIGIN ?? | |
| process.env.REPOSITORY_URL; | |
| if (repositoryUrl) { | |
| return normalizeRepositoryUrl(repositoryUrl); | |
| } | |
| if (process.env.GITHUB_REPOSITORY) { | |
| const githubServerUrl = process.env.GITHUB_SERVER_URL ?? "https://github.com"; | |
| return normalizeRepositoryUrl( | |
| `${githubServerUrl.replace(/\/$/, "")}/${process.env.GITHUB_REPOSITORY}` | |
| ); | |
| } | |
| export function getRepositoryUrl(): string | undefined { | |
| const repositoryUrl = | |
| process.env.CI_PROJECT_URL || | |
| process.env.CI_REPOSITORY_URL || | |
| process.env.CIRCLE_REPOSITORY_URL || | |
| process.env.BUILDKITE_REPO || | |
| process.env.BITBUCKET_GIT_HTTP_ORIGIN || | |
| process.env.BITBUCKET_GIT_SSH_ORIGIN || | |
| process.env.REPOSITORY_URL; | |
| if (repositoryUrl) { | |
| return normalizeRepositoryUrl(repositoryUrl); | |
| } | |
| if (process.env.GITHUB_REPOSITORY) { | |
| const githubServerUrl = | |
| process.env.GITHUB_SERVER_URL || "https://github.com"; | |
| return normalizeRepositoryUrl( | |
| `${githubServerUrl.replace(/\/$/, "")}/${process.env.GITHUB_REPOSITORY}` | |
| ); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
|
👏 yes! super important that we do this and get this right — so that when people run |
3 participants
Adds pull request metadata to preview deployment annotations when it can be detected from CI environment variables.\n\nDisplays the pull request URL in the normal wrangler preview output.\n\nThis is experimental and AI generated.