Repository navigation
ci: dev alias follows PRs, and main ships to production on green - #10
Conversation
Looking at a PR in Trello meant deploying and re-aliasing by hand, because the Power-Up on the test board is registered against the dev alias and nothing else. The e2e job already builds the PR to test it, so it now points the alias at that same build once the suite passes. The cost is that the alias then stays on branch code, and the nightly run tests the dev alias — so a stale alias makes the nightly report on a branch instead of on main. reset-dev (workflow_dispatch) builds main and aliases it back; it shares the trello-test-board concurrency group so it cannot switch what dev serves while a suite is running. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Leaving the dev alias on branch code silently changes what the nightly run reports on, and remembering to click a button is exactly the kind of step that gets skipped. push to main is the workhorse: it catches every merge and runs the definition from main, so it fires even for branches created before this file existed - a pull_request trigger would not, since GitHub takes the workflow from the PR's own branch. pull_request/closed covers the other exit, a PR abandoned without merging, which no push to main would ever catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 21 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds deploy-on-green automation for Vercel production builds, updates the dev alias for pull-request previews, and restores the alias to ChangesDeployment automation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions as GitHub Actions
participant VercelCLI as Vercel CLI
participant StagedDeployment as Staged deployment
participant Production as Production
participant DevAlias as Dev alias
GitHubActions->>VercelCLI: Stage one Vercel production build
VercelCLI-->>GitHubActions: Return deployment URL
GitHubActions->>StagedDeployment: Run fixtures and e2e tests
GitHubActions->>VercelCLI: Promote tested deployment
VercelCLI->>Production: Assign tested deployment
GitHubActions->>VercelCLI: Set dev alias to deployment URL
VercelCLI->>DevAlias: Update trello-excel-preview-dev.vercel.app
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e.yml:
- Around line 112-117: Update the “Point dev alias at this build” step to map
steps.preview.outputs.url to the DEPLOYMENT_URL workflow environment variable,
then pass "$DEPLOYMENT_URL" to vercel alias set instead of interpolating the
output directly in the shell command.
In `@DEPLOY.md`:
- Around line 21-29: Update the earlier deployment-policy statement in DEPLOY.md
to acknowledge that reset-dev automatically deploys main and restores the dev
alias after pushes to main, while preserving the existing description of other
deployment behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: af8d5c66-441a-4d3a-9195-fb9a0d210a48
📒 Files selected for processing (3)
.github/workflows/e2e.yml.github/workflows/reset-dev.ymlDEPLOY.md
Both from the review on #10. GitHub substitutes ${{ }} into the run script before bash parses it, so a quote or newline in the value rewrites the command instead of being passed to it - and these commands carry VERCEL_TOKEN. The URLs come from our own vercel deploy output, so this is hardening rather than a live hole, but it costs one line per step. Applied to all three call sites, including the Playwright step the review did not flag. DEPLOY.md opened with "nothing deploys automatically on push or merge", which stopped being true the moment reset-dev started building main on merge. Now it says what still holds: nothing reaches the production domain by itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merging left production untouched until someone remembered to run `vercel deploy --prod` by hand, so main and what users have drifted apart - the rename fix sat merged for a while before it reached anyone. deploy.yml builds main once, runs the full e2e suite against that staged build, and only then promotes it. Building once is the point: the artifact users get is the artifact the tests ran against, not a rebuild of the same commit. A red suite promotes nothing, so production keeps serving what it served before. The same build goes on the dev alias, so the test board and production agree. That takes over the merge case from reset-dev, which now handles only what a push to main cannot see: a PR closed without merging. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oonufrienko
changed the title
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both branches appended to the top of the Added list, anchored on the same line, which git resolves as a conflict on whichever merges second. Same entry, just placed at the end of the list instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`pull_request: types: [closed]` fires on merge too, and deploy.yml already builds main and points the dev alias at that build on a merge. Running here as well built main a second time and raced deploy.yml for the alias, so the alias could end up on whichever build finished last rather than on the one that was tested and promoted. Merges belong to deploy.yml. This job keeps the exits deploy.yml never sees: a PR closed without merging, and the manual button. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ipped Two changes to the same problem: a red deploy run did not tell you whether the merge reached users. The health check now runs against the staged build before the promote, so a broken /api/health fails the run while production is still on the old build instead of after it has been replaced. The production check stays where it was — it confirms the domain actually moved. The promote step can still succeed while the dev alias or the production check fails, and that run is red with production already on the new build. A final step writes that to the run summary, with the rollback commands, so the rollback decision does not start with reading the log. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/deploy.yml (1)
128-132: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFixed
sleep 10before the production health check is fragile.If CDN propagation takes longer than 10 seconds,
curlfails even thoughpromotealready succeeded. That triggers the "Production shipped, post-promote steps failed" report for a build that is actually fine, sending whoever reads the run summary toward an unnecessary rollback decision. Replace the fixed sleep with a short retry loop.♻️ Proposed retry loop
- name: Verify production run: | - sleep 10 - curl -sf https://trello-excel-preview.vercel.app/api/health - echo + for i in $(seq 1 6); do + if curl -sf https://trello-excel-preview.vercel.app/api/health; then + echo + exit 0 + fi + sleep 5 + done + echo "Production health check failed after retries" >&2 + exit 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy.yml around lines 128 - 132, Replace the fixed sleep in the “Verify production” workflow step with a short retry loop around the health-check curl request. Retry for a bounded period to accommodate CDN propagation, preserve fail-fast behavior after retries are exhausted, and keep the successful response output and existing production health endpoint unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy.yml:
- Line 39: Disable credential persistence on the actions/checkout@v4 step in
.github/workflows/deploy.yml lines 39-39 by adding persist-credentials: false.
Apply the same setting to the actions/checkout@v4 step in
.github/workflows/reset-dev.yml lines 41-43, preserving its existing ref: main
configuration.
---
Nitpick comments:
In @.github/workflows/deploy.yml:
- Around line 128-132: Replace the fixed sleep in the “Verify production”
workflow step with a short retry loop around the health-check curl request.
Retry for a bounded period to accommodate CDN propagation, preserve fail-fast
behavior after retries are exhausted, and keep the successful response output
and existing production health endpoint unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5261a8bb-966c-43dc-b6a6-40d79d82cb3f
📒 Files selected for processing (5)
.github/workflows/deploy.yml.github/workflows/e2e.yml.github/workflows/reset-dev.ymlCHANGELOG.mdDEPLOY.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/e2e.yml
CodeRabbit on #10: actions/checkout leaves GITHUB_TOKEN in the local git config for the whole job, and every one of these jobs then runs third-party code over it — npm ci, npx vercel, playwright. None of them push back to the repo, so the token has nothing to do but sit there and be exfiltratable. persist-credentials: false on all three checkouts, plus contents: read at the workflow level, matching what prune-deployments.yml already does. The review named deploy.yml and reset-dev.yml; e2e.yml has the identical exposure over contributor code, so it gets the same treatment rather than waiting for the next review to say so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/deploy.yml (1)
112-114: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound health checks and poll production readiness.
curl -sfhas no request deadline. A stalled deployment can consume the workflow timeout. The fixedsleep 10can also fail a successful promotion when production propagation takes longer.Add connection and request timeouts to both checks. Replace the fixed delay with bounded retries.
Proposed fix
- name: Check the staged build's health endpoint run: | - curl -sf "$DEPLOYMENT_URL/api/health" + curl --fail --silent --show-error \ + --connect-timeout 5 --max-time 30 \ + "$DEPLOYMENT_URL/api/health" echo ... - name: Verify production run: | - sleep 10 - curl -sf https://trello-excel-preview.vercel.app/api/health - echo + for attempt in {1..12}; do + if curl --fail --silent --show-error \ + --connect-timeout 5 --max-time 15 \ + https://trello-excel-preview.vercel.app/api/health; then + echo + exit 0 + fi + [[ "$attempt" -lt 12 ]] && sleep 5 + done + echo "Production health check did not pass within the retry window" >&2 + exit 1Also applies to: 136-139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy.yml around lines 112 - 114, Update both health-check steps around “Check the staged build's health endpoint” and the corresponding production check to add curl connection and request timeouts. Replace the fixed sleep before the production health check with bounded polling retries that continue until the endpoint succeeds or the retry limit is reached, while preserving failure on exhausted retries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/deploy.yml:
- Around line 112-114: Update both health-check steps around “Check the staged
build's health endpoint” and the corresponding production check to add curl
connection and request timeouts. Replace the fixed sleep before the production
health check with bounded polling retries that continue until the endpoint
succeeds or the retry limit is reached, while preserving failure on exhausted
retries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7be9a88a-ab0e-4467-874e-621dfec03067
📒 Files selected for processing (3)
.github/workflows/deploy.yml.github/workflows/e2e.yml.github/workflows/reset-dev.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/e2e.yml
- .github/workflows/reset-dev.yml
…leeping CodeRabbit on #10, against the steps added in 433a0b6. Both curls ran without a deadline, so a hung endpoint would have sat there until the job's 25-minute timeout instead of failing its own step. And -s without --show-error left a red step with an empty log, which defeats the point of a check whose job is to explain why the run went red. The fixed `sleep 10` before the production check was the worse half. Promote propagation is not instant, and a guess that came up short would fail the step on a perfectly good deploy — which "Report that production already shipped" would then announce as shipped-but-broken, sending someone to weigh a rollback that nothing called for. It now polls for up to ~60s and stays quiet while doing so, because early misses are the expected case. Verified under `bash -e`, the same shell GitHub uses: the success path exits 0 on the first attempt against live production; the exhausted path prints the message and the real curl error, then exits 1; the staged check now exits 22 with "curl: (22) ... 404" where it used to print nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/deploy.yml (2)
124-129: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftHandle promotion timeouts before taking the workflow as ambiguous.
vercel promotecan exit after its CLI timeout while the Vercel promotion keeps running in the background. In that state, the workflow can skip the production summary and later steps remain blocked as failed, even though production may finish promoting. Set a longer--timeout, pollvercel promote status, or inspect the target deployment before aborting the build flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy.yml around lines 124 - 129, Update the “Promote the tested build to production” step around the promote command to handle CLI timeouts without immediately treating the workflow as failed: configure a longer timeout and/or add polling with `vercel promote status` or target-deployment inspection, then continue once promotion completes successfully. Preserve the production summary and subsequent workflow steps when promotion is still progressing in the background, while still failing for genuine promotion errors.
183-189: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict or sanitize the uploaded Playwright report.
This job runs with
TRELLO_API_KEY/TRELLO_USER_TOKENand restoresstorageState.json, while Playwright captures traces and videos on failure. Uploadingplaywright-report/withif: always()archives artifacts that can contain session/cookie data and sensitive request details. Store these artifacts in a trusted, access-controlled location or sanitize them with a credential-aware tool before upload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy.yml around lines 183 - 189, Update the “Upload Playwright report” step to prevent raw reports, traces, and videos from being uploaded with potentially sensitive session or request data; sanitize the artifacts using a credential-aware process or replace this public artifact upload with a trusted, access-controlled storage destination before invoking actions/upload-artifact@v4.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy.yml:
- Around line 141-159: Update the Verify production step to use a single
approximately 60-second absolute deadline rather than fixed retry counts and a
final request. In the polling loop, calculate remaining time, cap each curl and
sleep to that remaining duration, and stop when the deadline expires; capture
the last curl error/output during polling and print it after the deadline
without issuing another full-timeout curl.
---
Outside diff comments:
In @.github/workflows/deploy.yml:
- Around line 124-129: Update the “Promote the tested build to production” step
around the promote command to handle CLI timeouts without immediately treating
the workflow as failed: configure a longer timeout and/or add polling with
`vercel promote status` or target-deployment inspection, then continue once
promotion completes successfully. Preserve the production summary and subsequent
workflow steps when promotion is still progressing in the background, while
still failing for genuine promotion errors.
- Around line 183-189: Update the “Upload Playwright report” step to prevent raw
reports, traces, and videos from being uploaded with potentially sensitive
session or request data; sanitize the artifacts using a credential-aware process
or replace this public artifact upload with a trusted, access-controlled storage
destination before invoking actions/upload-artifact@v4.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d96ee183-e2df-4f05-85b5-0213f6c29ffa
📒 Files selected for processing (1)
.github/workflows/deploy.yml
| - name: Verify production | ||
| run: | | ||
| # Poll instead of sleeping a fixed 10s. Promote propagation is not | ||
| # instant, and a guess that comes up short would fail this step on a | ||
| # perfectly good deploy — which the step below would then announce as | ||
| # "shipped, post-promote steps failed" and send someone to consider a | ||
| # rollback for nothing. Quiet while polling: early misses are expected. | ||
| for attempt in $(seq 1 12); do | ||
| if curl --fail --silent --connect-timeout 5 --max-time 15 \ | ||
| https://trello-excel-preview.vercel.app/api/health; then | ||
| echo | ||
| exit 0 | ||
| fi | ||
| sleep 5 | ||
| done | ||
| echo "Production did not answer /api/health within ~60s. Last attempt:" >&2 | ||
| curl --fail --silent --show-error --connect-timeout 5 --max-time 15 \ | ||
| https://trello-excel-preview.vercel.app/api/health || true | ||
| exit 1 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Enforce the stated approximately 60-second overall deadline.
Twelve attempts at 15 seconds each, a 5-second sleep after every failure, and a final 15-second curl can take approximately 255 seconds, or 4 minutes 15 seconds. Even fast failures take at least 75 seconds because the loop sleeps for 60 seconds before the final request.
Use one absolute deadline. Limit each curl and sleep to the remaining time. Capture the last curl error and print it after the deadline instead of starting another full-timeout request.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/deploy.yml around lines 141 - 159, Update the Verify
production step to use a single approximately 60-second absolute deadline rather
than fixed retry counts and a final request. In the polling loop, calculate
remaining time, cap each curl and sleep to that remaining duration, and stop
when the deadline expires; capture the last curl error/output during polling and
print it after the deadline without issuing another full-timeout curl.
CodeRabbit on #10, against 82911ac. The comment promised "up to ~60s" and the code could take 4m15s: twelve 15-second request timeouts and twelve 5-second sleeps stack, and a final diagnostic curl re-requested what had already failed. A fast, unambiguous failure still burned 60 seconds of sleeps before saying so, because the loop counted attempts rather than watching the clock. An absolute deadline says what it means: production gets a minute, plus whichever request was already in flight. The last failing output is kept from the polling loop instead of fetching it again. Verified under `bash -e`: live production answers in 1s and exits 0; a fast 404 now exits 1 after 64s with "curl: (22) ... 404" carried over from the last poll, where the previous shape needed 75s and a second request to say the same thing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both branches appended to the top of the Added list, anchored on the same line, which git resolves as a conflict on whichever merges second. Same entry, just placed at the end of the list instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Why
Two gaps around what happens to a branch after CI touches it.
Looking at a PR meant deploying by hand. The Power-Up on the test board is
registered against
trello-excel-preview-dev.vercel.appand nothing else, so abuild with no alias is unreachable from Trello. The e2e job already builds the
PR in order to test it.
Merging shipped nothing. Production only moved when someone remembered
vercel deploy --prod, so main and what users have drifted apart — the renamefix from #8 sat merged for a while before it reached anyone.
What changed
e2e.yml— afterRun Playwright(so only on green), point the dev aliasat the staged build. PR events only.
deploy.yml(new) — on push to main: build once, run the full e2e suiteagainst that staged build, and promote it only if the suite is green. The
same build goes on the dev alias, so the test board and production agree.
Building once is the point: the artifact users get is the one the tests ran
against, not a rebuild of the same commit. A red suite promotes nothing.
reset-dev.yml(new) —workflow_dispatchpluspull_request: closed,for the case a push to main cannot see: a PR abandoned without merging, which
would otherwise leave dev on dead branch code forever.
DEPLOY.md— the opener claimed nothing deploys automatically. That is nolonger true, and the dev section described an alias that only ever moved by
hand.
Review fixes applied
env:rather than${{ }}interpolation — GitHub substitutes before bash parses, and these commands
carry
VERCEL_TOKEN. Applied to all call sites, including the Playwright stepthe review did not flag.
Trade-offs, stated plainly
A green-but-bad release now reaches users without a human in the loop. It fails
closed — a flaky or failing suite ships nothing — and instant rollback still
works, though #11 prunes deployments after 5 days, so older rollbacks go through
git.
With more than one PR open, dev shows whichever run finished last.
Rollback
CI-only; no product code. Close this PR, or after merge
git revert -m 1 <sha>.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Reliability