Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe pull request enhances the Vercel deployment workflow by capturing deployment output and adding automated smoke-checks to verify both the deployment URL and a fixed production URL return valid responses with expected content. Vercel configuration is updated to disable direct Git deployments on the master branch, enforcing use of the custom workflow instead. Tests are added to validate the new workflow behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/vercel-web-deploy.yml (1)
74-79: Add an explicit network timeout in smoke checks.Line 74 currently relies on default fetch behavior. Adding a per-request timeout makes CI failures faster and more deterministic during external network stalls.
Suggested resilience tweak
- const response = await fetch(url, { redirect: 'follow' }); + const response = await fetch(url, { + redirect: 'follow', + signal: AbortSignal.timeout(15_000), + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/vercel-web-deploy.yml around lines 74 - 79, The smoke check uses fetch(url) without a timeout; add an AbortController-based per-request timeout (e.g., 10s) and pass its signal into fetch so stalled network calls get aborted; ensure you clear the timeout on success, and when catching an abort/timeout convert it into a clear error (using label and url) before throwing so CI fails fast and deterministically in the smoke check code that references response, fetch, html, label, and url.scripts/tests/vercel-web-deploy-workflow.test.mjs (1)
45-47: Tighten workflow contract assertions to prevent false positives.Line 47 currently matches any
BlockDatatoken. Consider asserting the exact deploy-output wiring and exact HTML marker to make this contract harder to bypass accidentally.Proposed test assertion hardening
assert.match(workflow, /vercel@latest deploy --prebuilt --prod/, 'workflow must deploy the prebuilt output'); assert.match(workflow, /id:\s*deploy/, 'workflow must capture the production deploy step output'); + assert.match( + workflow, + /DEPLOYMENT_URL:\s*\$\{\{\s*steps\.deploy\.outputs\.deployment_url\s*\}\}/, + 'workflow must wire deploy output into smoke-check env', + ); assert.match(workflow, /PRODUCTION_URL:\s*https:\/\/blockdata\.run/, 'workflow must smoke-check the official production domain'); - assert.match(workflow, /BlockData/, 'workflow smoke verification must assert the production shell marker'); + assert.match( + workflow, + /<title>BlockData<\/title>/, + 'workflow smoke verification must assert the production HTML title marker', + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/tests/vercel-web-deploy-workflow.test.mjs` around lines 45 - 47, The current loose assertion /BlockData/ can match many unintended places; tighten the workflow assertions by making the deploy id and production output wiring explicit and matching the exact HTML marker: in the test where you assert against the workflow string (the three assert.match lines operating on the workflow variable), change the deploy assertion to ensure the step id is exactly "deploy" (e.g., match /id:\s*deploy\b/), assert the production output wiring by matching the exact PRODUCTION_URL assignment or output key (e.g., match /PRODUCTION_URL:\s*https:\/\/blockdata\.run\b/ or include the outputs key name used in the workflow), and replace the loose /BlockData/ with an exact HTML marker match such as /<div[^>]*>\s*BlockData\s*<\/div>/ so the test only passes when the precise deploy output and page marker are present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/vercel-web-deploy.yml:
- Around line 74-79: The smoke check uses fetch(url) without a timeout; add an
AbortController-based per-request timeout (e.g., 10s) and pass its signal into
fetch so stalled network calls get aborted; ensure you clear the timeout on
success, and when catching an abort/timeout convert it into a clear error (using
label and url) before throwing so CI fails fast and deterministically in the
smoke check code that references response, fetch, html, label, and url.
In `@scripts/tests/vercel-web-deploy-workflow.test.mjs`:
- Around line 45-47: The current loose assertion /BlockData/ can match many
unintended places; tighten the workflow assertions by making the deploy id and
production output wiring explicit and matching the exact HTML marker: in the
test where you assert against the workflow string (the three assert.match lines
operating on the workflow variable), change the deploy assertion to ensure the
step id is exactly "deploy" (e.g., match /id:\s*deploy\b/), assert the
production output wiring by matching the exact PRODUCTION_URL assignment or
output key (e.g., match /PRODUCTION_URL:\s*https:\/\/blockdata\.run\b/ or
include the outputs key name used in the workflow), and replace the loose
/BlockData/ with an exact HTML marker match such as
/<div[^>]*>\s*BlockData\s*<\/div>/ so the test only passes when the precise
deploy output and page marker are present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 195da3a4-a1f6-45bf-896c-da46209ac404
📒 Files selected for processing (3)
.github/workflows/vercel-web-deploy.ymlscripts/tests/vercel-web-deploy-workflow.test.mjsweb/vercel.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
ode --test scripts/tests/vercel-web-deploy-workflow.test.mjs
Why this matters
This forces production to flow through the repo-owned GitHub Actions path instead of letting Vercel auto-build master directly.
Summary by CodeRabbit