-
Notifications
You must be signed in to change notification settings - Fork 65
feat(ci): add comment on pr workflow #663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,6 +80,7 @@ jobs: | |
|
|
||
| outputs: | ||
| published: ${{ steps.check-changes.outputs.has_changes }} | ||
| package_versions: ${{ steps.increment.outputs.package_versions }} | ||
|
|
||
| steps: | ||
| - name: Checkout Project | ||
|
|
@@ -126,10 +127,15 @@ jobs: | |
| echo "npmAuthToken: ${{ env.token }}" >> ~/.yarnrc.yml | ||
|
|
||
| - name: Sync and Increment Package Versions | ||
| id: increment | ||
| if: steps.check-changes.outputs.has_changes == 'true' | ||
| run: | | ||
| yarn package-tools sync --tag ${GITHUB_REF##*/} | ||
| yarn package-tools increment --packages ${{ needs.analyze-changes.outputs.node-recursive }} --tag ${GITHUB_REF##*/} | ||
| OUTPUT=$(yarn package-tools increment --packages ${{ needs.analyze-changes.outputs.node-recursive }} --tag ${GITHUB_REF##*/}) | ||
| echo "$OUTPUT" | ||
| echo "package_versions<<EOF" >> $GITHUB_OUTPUT | ||
| echo "$OUTPUT" >> $GITHUB_OUTPUT | ||
| echo "EOF" >> $GITHUB_OUTPUT | ||
| - name: Build Changed Packages | ||
| if: steps.check-changes.outputs.has_changes == 'true' | ||
| run: yarn run build:prod | ||
|
|
@@ -348,3 +354,175 @@ jobs: | |
| echo "==========================================" | ||
| echo "Tagging complete!" | ||
| echo "==========================================" | ||
|
|
||
| comment-on-pr: | ||
| name: Comment on Released PRs | ||
| needs: [publish-npm, publish-tag] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @riteshfyi comment-on-pr job runs whenever publish-npm and publish-tag complete successfully, regardless of whether packages were actually published. have you handled the case if there is no package published in that situation why we need to post the comment? Scenario A: Developer merges a PR that only updates README.md (no package changes) if: needs.publish-npm.outputs.published == 'true' why don't we add this for that case
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it will tell the user that no pacakge was affected that's a good thing right?. |
||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Get PR Number | ||
| id: get-pr | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const deploySha = context.sha; | ||
| const owner = context.repo.owner; | ||
| const repo = context.repo.repo; | ||
|
|
||
| try { | ||
| const prs = await github.rest.repos.listPullRequestsAssociatedWithCommit({ | ||
| owner, repo, commit_sha: deploySha | ||
|
Comment on lines
+374
to
+375
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The PR lookup is keyed only to Useful? React with 👍 / 👎. |
||
| }); | ||
| const mergedPR = prs.data.find(pr => pr.merged_at); | ||
| if (mergedPR) { | ||
| core.setOutput('pr_number', String(mergedPR.number)); | ||
| return; | ||
| } | ||
| } catch (error) { | ||
| console.log(`Failed to discover PR from commit: ${error.message}`); | ||
| } | ||
| core.setOutput('pr_number', ''); | ||
|
|
||
| - name: Post Release Comment on PR | ||
| if: steps.get-pr.outputs.pr_number != '' | ||
| uses: actions/github-script@v7 | ||
| with: | ||
|
Comment on lines
+389
to
+390
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This Useful? React with 👍 / 👎. |
||
| script: | | ||
| const prNumber = parseInt('${{ steps.get-pr.outputs.pr_number }}'); | ||
|
|
||
| const raw = `${{ needs.publish-npm.outputs.package_versions }}`; | ||
| const packageVersions = {}; | ||
| for (const line of raw.split('\n')) { | ||
| const match = line.match(/^(.+?)\s+=>\s+(.+)$/); | ||
| if (match) packageVersions[match[1].trim()] = match[2].trim(); | ||
| } | ||
| const packageEntries = Object.entries(packageVersions); | ||
|
|
||
| const owner = context.repo.owner; | ||
| const repo = context.repo.repo; | ||
| const repoUrl = `https://github.com/${owner}/${repo}`; | ||
| const hasPackages = packageEntries.length > 0; | ||
|
|
||
| const taggablePackages = { | ||
| '@webex/widgets': 'webex-widgets', | ||
| '@webex/cc-widgets': 'webex-cc-widgets' | ||
| }; | ||
|
|
||
| const aggregators = ['@webex/cc-widgets', '@webex/widgets']; | ||
|
|
||
| let commentBody; | ||
|
|
||
| if (hasPackages) { | ||
| const primaryPackage = aggregators.find(p => packageVersions[p]) | ||
| || packageEntries[0][0]; | ||
| const primaryVersion = packageVersions[primaryPackage]; | ||
| const stableVersion = primaryVersion | ||
| .replace(/-next\..*/, '') | ||
| .replace(/-[a-z]*\..*/, ''); | ||
|
|
||
| let cname = 'widgets.webex.com'; | ||
| try { | ||
| const cnameFile = await github.rest.repos.getContent({ owner, repo, path: 'docs/CNAME' }); | ||
| const encoded = cnameFile.data.content; | ||
| if (typeof encoded === 'string') { | ||
| cname = Buffer.from(encoded, 'base64').toString().trim(); | ||
| } | ||
| } catch (e) { | ||
| console.log(`Could not read docs/CNAME, using default changelog host: ${e.message}`); | ||
| } | ||
|
|
||
| const changelogUrl = new URL(`https://${cname}/changelog/`); | ||
| if (stableVersion) { | ||
| changelogUrl.searchParams.set('stable_version', stableVersion); | ||
| } | ||
| changelogUrl.searchParams.set('package', primaryPackage); | ||
| changelogUrl.searchParams.set('version', primaryVersion); | ||
|
|
||
| const tagLinkParts = Object.entries(taggablePackages) | ||
| .filter(([pkg]) => packageVersions[pkg]) | ||
| .map(([pkg, prefix]) => { | ||
| const tag = `${prefix}-v${packageVersions[pkg]}`; | ||
| return `[\`${tag}\`](${repoUrl}/releases/tag/${tag})`; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The PR comment builds tag links as Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. have to decide on this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the tag link finally redirects to releases/tag/${tag} only |
||
| }); | ||
| const releaseLine = tagLinkParts.length | ||
| ? `| **Released in:** ${tagLinkParts.join(', ')} |` | ||
| : ''; | ||
|
|
||
| const rows = packageEntries | ||
| .sort(([a], [b]) => { | ||
| if (a === '@webex/cc-widgets') return -1; | ||
| if (b === '@webex/cc-widgets') return 1; | ||
| if (a === '@webex/widgets') return -1; | ||
| if (b === '@webex/widgets') return 1; | ||
| return a.localeCompare(b); | ||
| }) | ||
| .map(([pkg, ver]) => `| \`${pkg}\` | \`${ver}\` |`) | ||
| .join('\n'); | ||
|
|
||
| const packagesTable = [ | ||
| '', | ||
| '| Packages Updated | Version |', | ||
| '|---------|---------|', | ||
| rows, | ||
| '' | ||
| ].join('\n'); | ||
|
|
||
| commentBody = [ | ||
| '| :tada: Your changes are now available! |', | ||
| '|---|', | ||
| releaseLine, | ||
| `| :book: **[View full changelog →](${changelogUrl})** |`, | ||
| packagesTable, | ||
| 'Thank you for your contribution!', | ||
| '', | ||
| `_:robot: This is an automated message. For queries, please contact [support](https://developer.webex.com/support)._` | ||
| ].filter(Boolean).join('\n'); | ||
| } else { | ||
| commentBody = [ | ||
| ':white_check_mark: **Your changes have been merged!**', | ||
| '', | ||
| 'Thank you for your contribution!', | ||
| '', | ||
| `_:robot: This is an automated message. For queries, please contact [support](https://developer.webex.com/support)._` | ||
| ].join('\n'); | ||
| } | ||
|
|
||
| try { | ||
| const pr = await github.rest.pulls.get({ owner, repo, pull_number: prNumber }); | ||
| if (!pr.data.merged_at) return; | ||
|
|
||
| const comments = await github.paginate(github.rest.issues.listComments, { | ||
| owner, repo, | ||
| issue_number: prNumber, | ||
| per_page: 100 | ||
| }); | ||
|
|
||
| const detailedComment = comments.find(comment => | ||
| comment.user.type === 'Bot' && | ||
| comment.body.includes('Your changes are now available') | ||
| ); | ||
| const mergedComment = comments.find(comment => | ||
| comment.user.type === 'Bot' && | ||
| comment.body.includes('Your changes have been merged') | ||
| ); | ||
|
|
||
| if (detailedComment) return; | ||
| if (!hasPackages && mergedComment) return; | ||
|
|
||
| if (mergedComment && hasPackages) { | ||
| await github.rest.issues.updateComment({ | ||
| owner, repo, | ||
| comment_id: mergedComment.id, | ||
| body: commentBody | ||
| }); | ||
| } else { | ||
| await github.rest.issues.createComment({ | ||
| owner, repo, | ||
| issue_number: prNumber, | ||
| body: commentBody | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| core.warning(`Failed to comment on PR #${prNumber}: ${error.message}`); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 High — Unquoted expression injection in shell
The
${{ needs.analyze-changes.outputs.node-recursive }}expression is expanded directly into the shell command without quoting. This is also a potential injection risk. Pass it through a step-levelenv:variable instead:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not raw user input from a form.