Skip to content
90 changes: 84 additions & 6 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,12 @@ jobs:
needs: [publish-npm, publish-tag]
runs-on: ubuntu-latest

outputs:
pr_number: ${{ steps.get-pr.outputs.pr_number }}
primary_package: ${{ steps.post-comment.outputs.primary_package }}
primary_version: ${{ steps.post-comment.outputs.primary_version }}
changelog_url: ${{ steps.post-comment.outputs.changelog_url }}
Comment on lines +365 to +367
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Decouple release metadata outputs from PR discovery

These job outputs are sourced only from steps.post-comment, but that step is skipped when PR lookup fails (get-pr catches errors and sets an empty PR number). In that case npm publish can still succeed, yet primary_package/primary_version/changelog_url stay empty and downstream notification loses release details. Compute metadata independently of PR lookup or provide a fallback source.

Useful? React with 👍 / 👎.


steps:
- name: Get PR Number
id: get-pr
Expand All @@ -385,6 +391,7 @@ jobs:
core.setOutput('pr_number', '');

- name: Post Release Comment on PR
id: post-comment
if: steps.get-pr.outputs.pr_number != ''
Comment on lines +394 to 395
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Compute release metadata even when PR lookup fails

The post-comment step is gated on steps.get-pr.outputs.pr_number != '', but this same step is the only place that sets primary_package, primary_version, and changelog_url outputs. If Get PR Number hits a transient GitHub API error (it catches and sets an empty PR number), this step is skipped, those outputs remain empty, and notify-webex-space exits without sending any deployment details even when npm publish succeeded. Release metadata generation should not depend on PR discovery.

Useful? React with 👍 / 👎.

uses: actions/github-script@v7
with:
Expand Down Expand Up @@ -412,11 +419,14 @@ jobs:
const aggregators = ['@webex/cc-widgets', '@webex/widgets'];

let commentBody;
let primaryPackage = '';
let primaryVersion = '';
let changelogUrl = '';

if (hasPackages) {
const primaryPackage = aggregators.find(p => packageVersions[p])
primaryPackage = aggregators.find(p => packageVersions[p])
|| packageEntries[0][0];
const primaryVersion = packageVersions[primaryPackage];
primaryVersion = packageVersions[primaryPackage];
const stableVersion = primaryVersion
.replace(/-next\..*/, '')
.replace(/-[a-z]*\..*/, '');
Expand All @@ -432,12 +442,13 @@ jobs:
console.log(`Could not read docs/CNAME, using default changelog host: ${e.message}`);
}

const changelogUrl = new URL(`https://${cname}/changelog/`);
const changelogUrlObj = new URL(`https://${cname}/changelog/`);
if (stableVersion) {
changelogUrl.searchParams.set('stable_version', stableVersion);
changelogUrlObj.searchParams.set('stable_version', stableVersion);
}
changelogUrl.searchParams.set('package', primaryPackage);
changelogUrl.searchParams.set('version', primaryVersion);
changelogUrlObj.searchParams.set('package', primaryPackage);
changelogUrlObj.searchParams.set('version', primaryVersion);
changelogUrl = changelogUrlObj.toString();

const tagLinkParts = Object.entries(taggablePackages)
.filter(([pkg]) => packageVersions[pkg])
Expand Down Expand Up @@ -526,3 +537,70 @@ jobs:
} catch (error) {
core.warning(`Failed to comment on PR #${prNumber}: ${error.message}`);
}

// Set outputs for downstream jobs
core.setOutput('primary_package', primaryPackage);
core.setOutput('primary_version', primaryVersion);
core.setOutput('changelog_url', changelogUrl);
Comment on lines +541 to +544
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Set release outputs before early returns

This step now feeds downstream notification data via core.setOutput(...), but those calls are placed after multiple early return paths in the same script (for example when a detailed bot comment already exists). In reruns of a successful release, the script can exit before setting outputs, so notify-webex-space receives empty primary_version/changelog_url values and sends an incomplete PR-only message despite a real publish. Ensure outputs are set before any early return (or replace returns with guarded branches) so downstream jobs always receive release metadata.

Useful? React with 👍 / 👎.


notify-webex-space:
name: Send Webex Space Notification
needs: [publish-tag, publish-npm, comment-on-pr]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate Webex notifications on workflow re-runs

There is no idempotency key; re-running a successful Deploy CD can post the same Webex message again. If that is undesirable, document it as accepted behavior or add a guard (for example only when github.run_attempt == 1, or a stronger “already notified this version” signal if you add state).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akulakum I have addressed this one

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include docs publish job in notification dependencies

The notification job does not depend on publish-documentation, so it can run (and post release/changelog links) while docs publication is still running or has failed. This creates a false completion signal where teams see a deployment notification before the documentation/changelog side of the deploy pipeline has actually succeeded.

Useful? React with 👍 / 👎.

runs-on: ubuntu-latest
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider least-privilege permissions for this job

If this job only posts to Webex via curl (no extra GitHub writes), you can usually tighten permissions at the job level so future edits cannot accidentally expand token scope beyond what notification needs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The job currently only posts to Webex via curl and doesn't need write permissions.

if: always() && github.run_attempt == 1
Comment on lines +548 to +550
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wait for docs publish before sending Webex notification

notify-webex-space currently depends only on publish-tag, publish-npm, and comment-on-pr, so it can send a “deployment complete” message even when publish-documentation is still running or has failed. In that case the notification may point to a changelog/docs state that is not yet published, which is a false-positive release signal for downstream teams. Add publish-documentation to this job’s needs (and gate on successful completion) so Webex messages are emitted only after the full deploy pipeline finishes.

Useful? React with 👍 / 👎.


steps:
- name: Post Webex Space Message
env:
WEBEX_BOT_TOKEN: ${{ secrets.WEBEX_BOT_TOKEN }}
WEBEX_ROOM_ID: ${{ secrets.WEBEX_ROOM_ID }}
COMMIT_MESSAGE: ${{ github.event.head_commit.message }}
run: |
PACKAGE="${{ needs.comment-on-pr.outputs.primary_package }}"
VERSION="${{ needs.comment-on-pr.outputs.primary_version }}"
PR_NUMBER="${{ needs.comment-on-pr.outputs.pr_number }}"
CHANGELOG_URL="${{ needs.comment-on-pr.outputs.changelog_url }}"

if [ -n "${PR_NUMBER}" ]; then
PR_LINK="https://github.com/${{ github.repository }}/pull/${PR_NUMBER}"
else
PR_LINK=""
fi

PR_TITLE=$(echo "${COMMIT_MESSAGE}" | head -n 1)

# Build message parts as an array
MESSAGE_PARTS=()

if [ -n "${VERSION}" ] && [ -n "${PACKAGE}" ]; then
MESSAGE_PARTS+=("**Version:** ${PACKAGE}@${VERSION}")
fi

if [ -n "${PR_LINK}" ]; then
MESSAGE_PARTS+=("**PR:** [${PR_TITLE}](${PR_LINK})")
fi

if [ -n "${CHANGELOG_URL}" ]; then
MESSAGE_PARTS+=("**Changelog:** ${CHANGELOG_URL}")
fi

if [ ${#MESSAGE_PARTS[@]} -eq 0 ]; then
echo "No version or PR info available. Skipping notification."
exit 0
fi

# Join parts with double newlines and add extra newline for markdown spacing
MESSAGE=$(printf '%s\n\n' "${MESSAGE_PARTS[@]}" | sed '$ s/\n\n$//')

echo "Sending message to Webex Space..."
echo "Message content:"
echo "${MESSAGE}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Message content / logging

Echoing the full markdown can be useful while iterating, but confirm this matches your logging policy. The “PR title” is derived from the first line of head_commit.message, which can include ticket IDs or wording you may not want duplicated into a space; if needed, trim/sanitize or use PR metadata via API instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message content logging is intentional for debugging and visibility.


BODY=$(jq -n --arg room "${WEBEX_ROOM_ID}" --arg md "${MESSAGE}" '{roomId: $room, markdown: $md}')
curl -sSf \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add timeouts to curl

Without --connect-timeout / --max-time, a hung TLS connection can consume the runner until the job timeout. Recommend a bounded request (for example --max-time 30).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The curl request doesn't have explicit timeout, but GitHub Actions jobs have built-in timeouts

-H "Authorization: Bearer ${WEBEX_BOT_TOKEN}" \
-H "Content-Type: application/json" \
-d "${BODY}" \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Runner egress to Webex

This step requires outbound HTTPS to webexapis.com. In locked-down environments (proxies, egress allowlists), failures can look like mysterious CI flakes—worth a short ops note in workflow comments or internal runbooks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub runners have outbound HTTPS access to webexapis.com configured.

https://webexapis.com/v1/messages > /dev/null
Comment on lines +600 to +604
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard Webex post when bot secrets are unset

The workflow calls curl -sSf against Webex without first validating WEBEX_BOT_TOKEN and WEBEX_ROOM_ID. If either secret is missing/empty (for example in a newly configured repo, fork, or after secret rotation), Webex returns an HTTP error and -f makes this step exit non-zero, causing the deploy workflow to fail after publish/tag work already completed. Add an explicit secret presence check and skip notification when they are absent to keep notification config issues from breaking deployments.

Useful? React with 👍 / 👎.

Comment on lines +600 to +604
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Skip Webex API call when notification secrets are missing

This step calls curl -sSf unconditionally, so if WEBEX_BOT_TOKEN or WEBEX_ROOM_ID is unset (for example after secret rotation or in a newly configured environment), Webex returns an HTTP error and -f makes the job fail. Because this runs after publish/tag jobs, a notification configuration issue can incorrectly mark an otherwise successful deployment workflow as failed.

Useful? React with 👍 / 👎.


echo "Message sent successfully!"
Loading