Vercel testlink workflow setup #70
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Reviewer's Guide by SourceryThis pull request sets up a workflow to generate a Deriv App ID and URL for Vercel deployments. The workflow is triggered by issue comments and push events. It uses the Sequence diagram for Vercel testlink workflowsequenceDiagram
participant GH as GitHub
participant WF as Workflow
participant VA as Vercel Action
participant DA as Deriv App ID Action
participant PC as PR Comment Action
GH->>WF: Trigger on issue comment/push
WF->>VA: Capture preview URL
VA-->>WF: Return preview URL
WF->>DA: Generate App ID
DA-->>WF: Return App ID
WF->>PC: Create sticky comment
PC-->>WF: Comment posted
WF->>GH: Store URL as artifact
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
AI Code Review
🔴 BUGS & LOGICAL ISSUES:
-
Missing Pull Request/Issue Context
• Issue Description: In the “Comment on pull request with App ID” step, the YAML references “${{github.event.issue.number}}” and “${{ steps.generate_app_id.outputs.pr_url }}”. However, the workflow triggers on both “push” and “issue_comment” events, which may not always provide an issue or PR context.
• Potential Impacts: If the event is a regular push (not linked to a PR) or a comment on an issue that is not a pull request, then “number: ${{github.event.issue.number}}” and “pr_url” outputs could be undefined. This could cause the action to fail or post to a non-existent issue number.
• Reproduction Scenarios:
– Pushing to a branch without an open PR.
– Commenting on an issue that is not a PR.
• Fix Implementation (Conditionally limit the step to PR context):- name: Comment on pull request with App ID and URLs
id: sticky_comment_on_pr
if: ${{ github.event_name == 'pull_request' && steps.generate_app_id.outputs.should_post_comment == 'true' }}
uses: marocchino/sticky-pull-request-comment@331f8f5
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
header: deriv-app-id-action
number: ${{ github.event.pull_request.number }}
message: |
...
- name: Comment on pull request with App ID and URLs
-
Possible Empty Vercel Preview URL
• Issue Description: The “Capture Vercel preview URL” step uses a regular expression to parse a custom link from an issue comment or commit. If the target link is not matched or the comment text changes, “vercel_preview_url” could be empty. Subsequent steps then pass this empty URL onward.
• Potential Impacts: An empty URL can lead to invalid “App ID + Server” links in the generated comment or artifact, creating broken links.
• Reproduction Scenario: The comment text does not match “Visit Preview”, or the user posts an incompatible link format.
• Fix Implementation (Validate output and skip/fail gracefully):- name: Validate Preview URL
if: steps.vercel_preview_url.outputs.vercel_preview_url == ''
run: |
echo "No valid Vercel Preview URL found."
exit 1
or skip the generation step if no URL
- name: Generate Deriv App ID
if: steps.vercel_preview_url.outputs.vercel_preview_url != ''
...
- name: Validate Preview URL
🟡 RELIABILITY CONCERNS:
-
No Fallback or Retry if Vercel URL Extraction Fails
• Edge Cases Identified: The current YAML does not provide a fallback if the extraction fails (e.g., the comment text is malformed).
• Potential Failure Scenarios: Inconsistent labeling of preview URLs or small typos in the comment text would cause the entire workflow to produce empty outputs.
• Mitigation Steps (Retry or Provide Default):
– Implement a more robust pattern or multiple fallback regex patterns.
– Skip dependent steps if the URL is not found.Example (Skip if no URL):
- name: Generate Deriv App ID for deployment Preview URL
if: steps.vercel_preview_url.outputs.vercel_preview_url != ''
uses: deriv-com/deriv-app-id-action@v1
...
- name: Generate Deriv App ID for deployment Preview URL
-
Token/Secrets Handling
• Potential Failures: If secrets (DERIV_API_TOKEN, DERIV_APP_ID) are invalid or not provided, the action fails silently.
• Mitigation: Add explicit error checks in the deriv-com/deriv-app-id-action step or capture failures to surface them in logs.
💡 ROBUSTNESS IMPROVEMENTS:
-
Enhanced Error Handling Around “generate_app_id”
• Improvement: Check for valid API responses and log clarity on failures, e.g., respond with a user-friendly message if the Deriv API token is invalid.
• Code Example:- name: Generate Deriv App ID for deployment Preview URL
id: generate_app_id
uses: deriv-com/deriv-app-id-action@v1
with:
DERIV_API_TOKEN: ${{ secrets.DERIV_API_TOKEN }}
DERIV_APP_ID: ${{ secrets.DERIV_APP_ID }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
max_retries: 5
vercel_preview_url: ${{ steps.vercel_preview_url.outputs.vercel_preview_url }}
continue-on-error: false # Fail fast if token is invalid
- name: Generate Deriv App ID for deployment Preview URL
-
Input Validation on Deriv App ID
• Rationale: Ensure the returned app ID is a valid numeric/string before passing it on to the comment.
• Code Example:- name: Validate Generated App ID
run: |
if [[ !$APP_ID =~ ^[0-9]+$ ]]; then
echo "Invalid App ID: $APP_ID"
exit 1
fi
- name: Validate Generated App ID
-
State Management for Multi-PR Concurrency
• Recommendation: Use “concurrency: group” for the workflow if multiple PRs may trigger it simultaneously, preventing race conditions in comment handling.
• Code Example:concurrency:
group: deriv-app-id-${{ github.ref }}
cancel-in-progress: false
This ensures each PR’s workflow runs in isolation and does not accidentally overwrite comments from another in-progress run.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @aum-deriv - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| jobs: | ||
| generate_app_id: | ||
| timeout-minutes: 5 |
There was a problem hiding this comment.
suggestion (performance): Consider increasing the timeout duration to accommodate potential API delays and retries
With 5 retries configured for the app ID generation, 5 minutes might be insufficient if there are network delays or service issues. Consider increasing to 10-15 minutes to prevent timeout failures.
| timeout-minutes: 5 | |
| timeout-minutes: 15 |
| permissions: | ||
| actions: write | ||
| checks: write | ||
| contents: write | ||
| deployments: write | ||
| pull-requests: write | ||
| statuses: write |
There was a problem hiding this comment.
🚨 suggestion (security): Consider reducing the workflow permissions to only those actually required
The workflow appears to only need 'pull-requests' and possibly 'contents' permissions. Following the principle of least privilege, consider removing unused permissions to reduce security risk.
| permissions: | |
| actions: write | |
| checks: write | |
| contents: write | |
| deployments: write | |
| pull-requests: write | |
| statuses: write | |
| permissions: | |
| contents: write | |
| pull-requests: write |
Summary by Sourcery
CI: