fix: retry alias command without --scope for personal accounts - #310
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the reliability of Vercel alias assignments by introducing sophisticated error handling and a targeted retry mechanism. It addresses a common issue where Vercel CLI might incorrectly reject the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Deploy preview for team-scope-test ready! ✅ Preview Built with commit b1560bd. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the aliasDomainsToDeployment function by implementing more robust error handling for Vercel domain aliasing. It now captures stderr to detect specific scope-related errors and retries the alias command without the --scope argument if such an error occurs. The review feedback correctly points out a critical issue where the ignoreReturnCode option is missing in the retry execution, making the subsequent error check unreachable, and suggests including the exit code in the error message for better debugging.
There was a problem hiding this comment.
Pull request overview
This PR updates the action’s Vercel aliasing flow to better handle personal-account scenarios where the Vercel CLI rejects --scope, aligning alias behavior with the existing deploy fallback strategy.
Changes:
- Refactors
aliasDomainsToDeploymentto capture CLI stderr and evaluate exit codes per alias invocation. - Detects
PERSONAL_ACCOUNT_SCOPE_ERROR, warns, and retriesvercel aliaswithout--scope. - Improves surfaced failures by throwing errors when aliasing does not succeed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| + 'Retrying without --scope.', | ||
| ) | ||
| const retryArgs = [vercelBin, '-t', vercelToken, 'alias', deploymentUrl, domain] | ||
| const retryExitCode = await exec.exec('npx', retryArgs) |
There was a problem hiding this comment.
exec.exec('npx', retryArgs) will throw on non-zero exit code unless ignoreReturnCode: true is set, so retryExitCode may never be returned and the if (retryExitCode !== 0) branch becomes ineffective. Use ignoreReturnCode: true (and/or wrap in try/catch) for the retry invocation so failures can be handled with the intended descriptive error path.
| const retryExitCode = await exec.exec('npx', retryArgs) | |
| const retryExitCode = await exec.exec('npx', retryArgs, { ignoreReturnCode: true }) |
There was a problem hiding this comment.
Already addressed in a previous commit — the retry exec call already has ignoreReturnCode: true set. This thread is outdated.
| let myError = '' | ||
| const exitCode = await exec.exec('npx', args, { | ||
| ignoreReturnCode: true, | ||
| listeners: { | ||
| stderr: (data) => { | ||
| myError += data.toString() | ||
| }, | ||
| }, | ||
| }) | ||
|
|
||
| if (exitCode !== 0) { | ||
| if (myError.includes(PERSONAL_ACCOUNT_SCOPE_ERROR)) { | ||
| core.warning( |
There was a problem hiding this comment.
The personal-account scope detection only inspects myError (stderr). Elsewhere (e.g., vercelDeploy) the code checks combined stdout+stderr for PERSONAL_ACCOUNT_SCOPE_ERROR, so this alias fallback may not trigger if the CLI prints the message on stdout. Capture stdout as well (or combine streams) before checking for PERSONAL_ACCOUNT_SCOPE_ERROR.
There was a problem hiding this comment.
The code already captures both stdout (myOutput) and stderr (myError) and checks combinedOutput = myOutput + myError before checking for PERSONAL_ACCOUNT_SCOPE_ERROR. This is already addressed.
| @@ -432,15 +432,43 @@ async function aliasDomainsToDeployment(deploymentUrl) { | |||
| if (!deploymentUrl) { | |||
| core.error('deployment url is null') | |||
There was a problem hiding this comment.
If deploymentUrl is missing, this function logs an error but still continues and will attempt to run vercel alias with an undefined URL, producing a harder-to-debug failure later. Consider throwing (or returning early) after logging so the action fails fast with a clear cause.
| core.error('deployment url is null') | |
| core.error('deployment url is null') | |
| throw new Error('deployment url is null') |
There was a problem hiding this comment.
Applied in 8a4e031 — changed return to throw new Error('deployment url is null') so the action fails fast with a clear cause when deploymentUrl is null.
| const retryExitCode = await exec.exec('npx', retryArgs) | ||
| if (retryExitCode !== 0) { | ||
| throw new Error(`Alias command failed for domain ${domain}`) | ||
| } | ||
| return | ||
| } | ||
| throw new Error(`Alias command failed for domain ${domain} with exit code ${exitCode}`) |
There was a problem hiding this comment.
On retry failure, the thrown error (Alias command failed for domain ...) drops useful context like the retry exit code and stderr output, which makes diagnosing alias failures harder. Include the retry exit code (and ideally captured stderr/stdout) in the error message, consistent with the initial failure path.
| const retryExitCode = await exec.exec('npx', retryArgs) | |
| if (retryExitCode !== 0) { | |
| throw new Error(`Alias command failed for domain ${domain}`) | |
| } | |
| return | |
| } | |
| throw new Error(`Alias command failed for domain ${domain} with exit code ${exitCode}`) | |
| let retryError = '' | |
| let retryOutput = '' | |
| const retryExitCode = await exec.exec('npx', retryArgs, { | |
| ignoreReturnCode: true, | |
| listeners: { | |
| stderr: (data) => { | |
| retryError += data.toString() | |
| }, | |
| stdout: (data) => { | |
| retryOutput += data.toString() | |
| }, | |
| }, | |
| }) | |
| if (retryExitCode !== 0) { | |
| throw new Error( | |
| `Alias command failed for domain ${domain} with exit code ${retryExitCode}` | |
| + (retryError ? `, stderr: ${retryError.trim()}` : '') | |
| + (retryOutput ? `, stdout: ${retryOutput.trim()}` : ''), | |
| ) | |
| } | |
| return | |
| } | |
| throw new Error( | |
| `Alias command failed for domain ${domain} with exit code ${exitCode}` | |
| + (myError ? `, stderr: ${myError.trim()}` : ''), | |
| ) |
There was a problem hiding this comment.
Applied in 8a4e031 — added stdout listener to the retry exec call and include both stderr and stdout in the error message using template literals for consistency with the linting rules.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="index.js">
<violation number="1" location="index.js:463">
P1: Missing `ignoreReturnCode: true` on the retry `exec.exec` call. By default `@actions/exec` throws on non-zero exit, so the `retryExitCode !== 0` check is dead code and the error instead bubbles to the outer `retry()`, causing it to re-run the scope-based attempt in a wasteful loop.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Action as index.js (Action Logic)
participant CLI as Vercel CLI (npx)
participant API as Vercel API
Note over Action, API: NEW: Domain Aliasing Flow with Personal Account Fallback
Action->>CLI: exec: alias [url] [domain] --scope [scope]
CLI->>API: Request alias assignment
API-->>CLI: Response (Success or Error)
CLI-->>Action: Return Exit Code + Capture stderr
alt Exit Code != 0 AND stderr contains PERSONAL_ACCOUNT_SCOPE_ERROR
Action->>Action: NEW: Emit Warning (Scope rejected)
Action->>CLI: NEW: Retry: alias [url] [domain] (No --scope)
CLI->>API: Request alias assignment
API-->>CLI: Response
CLI-->>Action: Return Exit Code
alt Retry Failed
Action->>Action: Throw Error
end
else Other Exit Code != 0
Action->>Action: CHANGED: Throw Descriptive Error with Exit Code
end
Note over Action, API: Flow repeats for each domain in aliasDomains list
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Deploy preview for express-basic-auth ready! ✅ Preview Built with commit b1560bd. |
When Vercel CLI rejects the scope during the alias command for personal accounts, capture stderr and retry the alias without --scope, matching the same fallback pattern used for the main deployment command.
- add early return when deploymentUrl is null to prevent downstream errors - capture stdout in addition to stderr to match vercelDeploy pattern - use ignoreReturnCode and capture stderr in retry exec call - include stderr context in error messages for better diagnostics
- Throw an error immediately when deploymentUrl is null instead of returning silently, so the action fails fast with a clear cause - Capture stdout in retry exec listener to include in error diagnostics, consistent with the suggestion to capture both streams for better troubleshooting context
690ade9 to
20ea5ba
Compare
|
Capture stderr in alias exec and retry without --scope when Vercel CLI rejects personal account scope, matching the deploy retry pattern. Add tests for alias scope retry, retry failure, and non-scope failures.
Capture stderr in alias exec and retry without --scope when Vercel CLI rejects personal account scope, matching the deploy retry pattern. Add tests for alias scope retry, retry failure, and non-scope failures.
* refactor: migrate to TypeScript with Vitest and modern tooling BREAKING CHANGE: @actions/github upgraded from v2 to v6 - Convert index.js to src/index.ts with strict TypeScript mode - Extract pure utility functions to src/utils.ts for testability - Add tsconfig.json with strict compiler options - Replace Jest with Vitest for faster, ESM-native testing - Add comprehensive unit tests for utility functions (43 tests) - Add integration tests for GitHub Action structure (12 tests) - Target: 80%+ code coverage - Migrate from deprecated `new github.GitHub(token)` to `github.getOctokit(token)` - Update all API calls from `octokit.repos.*` to `octokit.rest.repos.*` - Update all API calls from `octokit.issues.*` to `octokit.rest.issues.*` - Update all API calls from `octokit.git.*` to `octokit.rest.git.*` - Enable TypeScript support in @antfu/eslint-config - Replace Jest globals with Vitest globals (vi instead of jest) - Add lib/ to ignore patterns - Update ncc build to compile TypeScript directly - Add source maps and licenses to dist output - Add typecheck script for standalone type checking - Update all npm scripts for TypeScript workflow - index.js (migrated to src/index.ts) - index.test.js (migrated to src/__tests__/*.test.ts) - jest.config.js (replaced by vitest.config.ts) - now.js (unused legacy file) Closes #291 * chore: apply AI code review suggestions - Move PullRequestPayload and ReleasePayload interfaces to top of file - Use PullRequestPayload type instead of inline type assertion - Simplify parseArgs regex match logic (remove redundant fallback) * refactor: split index.ts into smaller modules - Extract types to src/types.ts - Extract config and initialization to src/config.ts - Extract Vercel functions to src/vercel.ts - Extract GitHub comment functions to src/github-comments.ts - Refactor run() into smaller focused functions (< 50 LOC each) - Add try-catch around execSync for git log with descriptive error - Add error handling for GitHub API calls - Make alias failures explicit with warning messages - Extract magic numbers to named constants (RETRY_DELAY_MS, ALIAS_RETRY_COUNT) - Use buildCommentPrefix() consistently in both comment functions All files now under 300 LOC limit, all functions under 50 LOC limit. * build: rebuild dist after refactoring * build: update vercel to v50 * fix: improve error handling in vercel deploy and comment functions - Extract and validate deployment URL from vercel CLI stdout - Wrap vercelInspect exec in try-catch to prevent action failure - Expand try-catch scope in comment functions to cover API lookups - Route stderr to core.warning for better error visibility * test: add tests for vercel, config, and github-comments modules - Add vercel.test.ts: URL extraction, inspect regex, alias retry - Add config.test.ts: alias domain substitution, env export - Add github-comments.test.ts: comment create/update, error handling - Total tests: 55 → 100 * fix: port personal account scope error handling from PR #297/#298 - Add core.setSecret for vercel token to prevent log exposure - Disable telemetry with VERCEL_TELEMETRY_DISABLED env var - Require both org and project IDs together (v41+ compat) - Auto-retry deployment on personal account scope error - Sanitize commit message newlines and quotes in metadata - Add ignoreReturnCode for proper exit code handling - Update tests for new behavior (105 total) * refactor: use DeploymentContext object in vercelDeploy signature Reduce parameter count from 6 to 2 by passing DeploymentContext object instead of individual ref, commit, sha, commitOrg, commitRepo arguments. Aligns with AGENTS.md parameter limit of 5. * fix: port alias retry without --scope for personal accounts (#310) Capture stderr in alias exec and retry without --scope when Vercel CLI rejects personal account scope, matching the deploy retry pattern. Add tests for alias scope retry, retry failure, and non-scope failures. * fix: remove declaration maps to fix check-dist CI failure Source map files (.d.ts.map) contain absolute local paths that differ between local builds and CI runners, causing check-dist to fail. Disable declarationMap in tsconfig.json since these files are not needed for GitHub Action runtime execution. * chore: apply AI code review suggestions - Implement exponential backoff in retry function (was constant delay) - Fix debug log for head_commit to use JSON.stringify - Change vercelInspect stderr logging from warning to info level - Fix retry after PERSONAL_ACCOUNT_SCOPE_ERROR to omit --scope - Bump @types/node from ^20.0.0 to ^24.0.0 to match Node 24 runtime - Add setSecret to @actions/core mock in index.test.ts - Fix mock path in config.test.ts from ../package.json to ../../package.json - Update test expectation to match mocked vercel version (30.0.0) - Remove pull_request_target from PullRequestPayload (always undefined) - Add per_page: 100 to comment listing API calls to reduce duplicate risk - Fix Node version in session-summary.md (20 → 24) * fix: make vercelScope optional to clean up retry cast in vercelDeploy Co-authored-by: amondnet <1964421+amondnet@users.noreply.github.com> Agent-Logs-Url: https://github.com/amondnet/vercel-action/sessions/16d171b4-3be8-43f8-a736-b5f3ff13deba --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: amondnet <1964421+amondnet@users.noreply.github.com>
Summary
aliasDomainsToDeploymentto capture stderr and inspect the exit code per alias command invocation--scopefor personal accounts (matchingPERSONAL_ACCOUNT_SCOPE_ERROR), emits a warning and retries the alias without--scopeTest Plan
--scope)pnpm run all(lint + build + tests) locally to confirm no regressions