ci: yml for testlinks #69
Conversation
Reviewer's Guide by SourceryThis PR introduces a new GitHub Actions workflow to generate Deriv App IDs for preview deployments. It captures the Vercel preview URL, generates a Deriv App ID using the Sequence diagram for Deriv App ID generation workflowsequenceDiagram
participant GH as GitHub Actions
participant VA as Vercel Preview URL Action
participant DA as Deriv App ID Action
participant PR as Pull Request
Note over GH: Triggered by issue comment edit
GH->>VA: Request preview URL capture
VA-->>GH: Return Vercel preview URL
GH->>DA: Generate App ID
Note over DA: Uses DERIV_API_TOKEN
DA-->>GH: Return App ID and URLs
GH->>PR: Post sticky comment with App ID and URLs
Note over GH: Store URL in artifact
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 - here's some feedback:
Overall Comments:
- Consider using
GITHUB_TOKENinstead ofPERSONAL_ACCESS_TOKENwhere possible for better security practices, unless there's a specific need for extended permissions
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 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.
| - name: Capture Vercel preview URL | ||
| id: vercel_preview_url | ||
| uses: binary-com/vercel-preview-url-action@v1.0.5 | ||
| with: | ||
| GITHUB_TOKEN: ${{ secrets.PERSONAL_ACCESS_TOKEN }} | ||
| preview_url_regexp: \[Visit Preview\]\((.*?.sx)\) |
There was a problem hiding this comment.
suggestion (bug_risk): Add retry mechanism for Vercel preview URL capture
While the app ID generation has retries, the Vercel preview URL capture doesn't. Consider adding similar retry logic to handle temporary API issues.
| - name: Capture Vercel preview URL | |
| id: vercel_preview_url | |
| uses: binary-com/vercel-preview-url-action@v1.0.5 | |
| with: | |
| GITHUB_TOKEN: ${{ secrets.PERSONAL_ACCESS_TOKEN }} | |
| preview_url_regexp: \[Visit Preview\]\((.*?.sx)\) | |
| - name: Capture Vercel preview URL | |
| id: vercel_preview_url | |
| uses: binary-com/vercel-preview-url-action@v1.0.5 | |
| with: | |
| GITHUB_TOKEN: ${{ secrets.PERSONAL_ACCESS_TOKEN }} | |
| preview_url_regexp: \[Visit Preview\]\((.*?.sx)\) | |
| max_retries: 5 | |
| retry_wait_seconds: 30 |
There was a problem hiding this comment.
AI Code Review
🔴 BUGS & LOGICAL ISSUES:
-
Potential regex mismatch for “vercel_preview_url”
• Description: The capture regex (preview_url_regexp: [Visit Preview]((.?.sx))) may fail if the actual Vercel URL does not end with ".sx" (e.g., “.vercel.app”). This can cause the captured URL to be empty or invalid.
• Potential impacts: The downstream steps (generating the app ID, etc.) will either fail or produce incomplete outputs if no valid URL is extracted.
• Reproduction scenarios:
– A comment containing “Visit Preview” instead of “.sx” domain.
– Editing the comment again without changing to “.sx” effectively breaks the workflow.
• Fix: Update the regex or provide multiple patterns. Example:
with:
preview_url_regexp: '[Visit Preview]((.?(?:.sx|.vercel.app)))' -
No fallback handling if “vercel_preview_url” is empty
• Description: The next steps rely heavily on steps.vercel_preview_url.outputs.vercel_preview_url. Currently, if the regex fails to find a URL or returns empty, subsequent steps will produce invalid or empty strings.
• Potential impacts: The job can silently fail, generate an empty App ID or break the comment posting step.
• Reproduction scenarios:
– The comment is edited so that “[Visit Preview]” link is not present.
– The action fails to parse, and no fallback logic is executed.
• Fix: Add a check for an empty or missing “vercel_preview_url” output before proceeding, for example:- name: Validate captured URL
if: ${{ steps.vercel_preview_url.outputs.vercel_preview_url == '' }}
run: |
echo "No valid preview URL found. Exiting."
exit 1
- name: Validate captured URL
🟡 RELIABILITY CONCERNS:
-
Repeated triggering on multiple comment edits
• Edge cases identified: If a user repeatedly edits an issue comment, the workflow might run multiple times, continually creating (or attempting to create) new IDs.
• Potential failure scenarios:
– Spamming comments triggers rate limits for DERIV_API_TOKEN.
– Overwrites or multiple “sticky” comments on the same PR.
• Mitigation steps:
– Add a condition that checks whether an App ID was already created for this pull request to avoid re-generation or confirm an overwrite.
Example:- name: Check existing comment content run: | # pseudo-code to parse existing comment content for an existing App ID # skip generation if one is found echo "Implement check logic here if needed." -
Lack of error handling in deriv-app-id-action usage
• Potential failure scenarios: If deriv-com/deriv-app-id-action@v1 fails internally (e.g., token expired), the job proceeds without a useful error message or fallback.
• Mitigation:
– Add “continue-on-error: false” (default) and ensure the job fails visibly if the action errors.
– Use the action’s output to conditionally proceed, e.g.:- name: Ensure deriv-app-id-action succeeded if: steps.generate_app_id.outcome == 'failure' run: exit 1
💡 ROBUSTNESS IMPROVEMENTS:
-
Explicit error-handing for empty or invalid “app_id”
• Enhancement: Before posting or storing the generated URL, confirm that steps.generate_app_id.outputs.app_id is non-empty.
• Code example:- name: Validate app_id is non-empty
if: ${{ steps.generate_app_id.outputs.app_id == '' }}
run: |
echo "No valid app_id generated. Exiting."
exit 1
- name: Validate app_id is non-empty
-
Pinning action versions for security and predictability
• Enhancement: Instead of “actions/upload-artifact@master” use a stable version or SHA to avoid breaking changes from the master branch.
• Code example:- name: Upload artifact
uses: actions/upload-artifact@v3
with:
name: generated_url
path: ${{ github.workspace }}/url.txt
retention-days: 1
- name: Upload artifact
-
Permission minimization
• Improvement: Only request the minimal required permissions in “permissions:” block. For example, if “pull-requests: write” is not strictly needed (or if read is enough), reduce it.
• Code example:permissions:
contents: write
pull-requests: write
# Other permissions only if absolutely necessary -
Early exit if environment secrets are missing
• Enhancement: Check for required secrets (DERIV_API_TOKEN, DERIV_APP_ID, etc.) before running the steps.
• Code example:- name: Check required secrets
run: |
if [ -z "${{ secrets.DERIV_API_TOKEN }}" ] || [ -z "${{ secrets.DERIV_APP_ID }}" ]; then
echo "Required secrets for DERIV are missing. Exiting."
exit 1
fi
- name: Check required secrets
Overall, these changes improve logical correctness (by preventing empty or invalid data usage), enhance reliability (by handling edge cases more gracefully), and bolster robustness (through error handling, version pinning, and permission minimization).
Summary by Sourcery
CI: