semaphore: fix status reporting for pipelines#471
Merged
Yashprime1 merged 1 commit intomasterfrom Sep 4, 2025
Merged
Conversation
WalkthroughIntroduces strict shell error handling (set -euo pipefail) in staging and production deploy scripts and consolidates build commands into a single multiline script with the same strict mode in the main Semaphore pipeline. No functional command changes; indentation and script block structure adjusted. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant Semaphore
participant Build as Build Job
participant DeployStg as Deploy (Staging)
participant DeployProd as Deploy (Production)
participant AWS as AWS (S3/CloudFront)
Dev->>Semaphore: Push / Trigger
Semaphore->>Build: Run build step (strict shell)
note right of Build: set -euo pipefail<br/>npm install → npm run build_sw
Build-->>Semaphore: Artifacts ready
alt Promote to Staging
Semaphore->>DeployStg: Deploy Assets (strict shell)
note right of DeployStg: set -euo pipefail
DeployStg->>AWS: S3 sync → CloudFront invalidate
AWS-->>DeployStg: OK/Errors
end
alt Promote to Production
Semaphore->>DeployProd: Deploy Assets (strict shell)
note right of DeployProd: set -euo pipefail
DeployProd->>AWS: S3 sync → CloudFront invalidate
AWS-->>DeployProd: OK/Errors
end
opt Error handling
note over Build,DeployProd: Any command error or unset var causes immediate exit
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) ✅ code/snyk check is complete. No issues have been found. (View Details) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.semaphore/staging-deploy.yml (1)
19-35: Harden script: remove secret echo, fix rm flags, and batch CloudFront invalidation.
- Remove
env | grep AWSto avoid credential leakage and pipefail-induced failures.- Replace
rm -rwithrm -f --on files.- Combine the five invalidations into one call to reduce API calls and speed up deploys.
- set -euo pipefail + set -euo pipefail mkdir utils aws s3 cp s3://cfstack-init-sources3bucket-b5bxfzywj4ae/utils/generate_assumed_role_creds.py utils/generate_assumed_role_creds.py eval $(python3 utils/generate_assumed_role_creds.py --role-arn ${PRODUCTION_ACCOUNT_IAM_ROLE_ARN} --session-name SemaphoreAgent) - env | grep AWS aws sts get-caller-identity aws s3 cp ./clevertap.min.js s3://static.wizrocket.com/staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/ --acl public-read aws s3 cp ./clevertap.js s3://static.wizrocket.com/staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/ --acl public-read aws s3 cp ./clevertap.js.map s3://static.wizrocket.com/staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/ --acl public-read aws s3 cp ./sw_webpush.min.js s3://static.wizrocket.com/staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/ --acl public-read aws s3 cp ./sw_webpush.js s3://static.wizrocket.com/staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/ --acl public-read - rm -r clevertap.min.js clevertap.js clevertap.js.map sw_webpush.min.js sw_webpush.js - aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/clevertap.js - aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/clevertap.js.map - aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/sw_webpush.min.js - aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/sw_webpush.js - aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/clevertap.min.js + rm -f -- clevertap.min.js clevertap.js clevertap.js.map sw_webpush.min.js sw_webpush.js + aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths \ + "/staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/clevertap.js" \ + "/staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/clevertap.js.map" \ + "/staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/sw_webpush.min.js" \ + "/staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/sw_webpush.js" \ + "/staging/${SEMAPHORE_GIT_WORKING_BRANCH}/js/clevertap.min.js"
🧹 Nitpick comments (2)
.semaphore/semaphore.yml (2)
17-17: Fix YAML lint: trailing space after block scalar.
Remove the trailing space after- |to satisfy yamllint.- - | + - |
17-20: Prefernpm ciin CI for reproducible installs.
If a lockfile exists,npm ciis faster and deterministic.- npm install + npm ci
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.semaphore/production-deploy.yml(1 hunks).semaphore/semaphore.yml(1 hunks).semaphore/staging-deploy.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.semaphore/semaphore.yml
[error] 17-17: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.semaphore/production-deploy.yml (1)
15-21: Verify env/ACL assumptions underset -u.
- Ensure
${PRODUCTION_ACCOUNT_IAM_ROLE_ARN}is always present;set -uwill hard-fail otherwise.- If the S3 bucket enforces Object Ownership (ACLs disabled),
--acl public-readwill fail; confirm bucket settings or drop the flag..semaphore/staging-deploy.yml (2)
19-21: Strict mode adoption LGTM.
This should improve failure visibility and status reporting.
22-29: Confirm required env presence underset -u.
${PRODUCTION_ACCOUNT_IAM_ROLE_ARN}and${SEMAPHORE_GIT_WORKING_BRANCH}must always be set to avoid hard failures..semaphore/semaphore.yml (2)
17-20: Good move to a single strict script block.
Unifying withset -euo pipefailis an improvement.
13-22: Double-check artifact push ordering.
You pushclevertap*.jsartifacts (Lines 14–16) before running any build step; verify these are prebuilt/checked-in. If not, reorder to build before pushing.
Comment on lines
+15
to
+23
| set -euo pipefail | ||
| mkdir utils | ||
| aws s3 cp s3://cfstack-init-sources3bucket-b5bxfzywj4ae/utils/generate_assumed_role_creds.py utils/generate_assumed_role_creds.py | ||
| eval $(python3 utils/generate_assumed_role_creds.py --role-arn ${PRODUCTION_ACCOUNT_IAM_ROLE_ARN} --session-name SemaphoreAgent) | ||
| env | grep AWS | ||
| aws sts get-caller-identity | ||
| - 'aws s3 cp clevertap.min.js s3://static.wizrocket.com/js/ --acl public-read' | ||
| - rm -r clevertap.min.js | ||
| - aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /js/clevertap.min.js | ||
| aws s3 cp clevertap.min.js s3://static.wizrocket.com/js/ --acl public-read | ||
| rm -r clevertap.min.js | ||
| aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /js/clevertap.min.js |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strict mode added: good, but current script will leak AWS secrets and may fail due to pipefail.
env | grep AWSwill print creds to logs and, withset -euo pipefail, can fail the job ifgrepreturns 1. Remove it.- Use
rm -finstead ofrm -rfor single files to avoid unnecessary failure under-e.
Apply:
- set -euo pipefail
+ set -euo pipefail
mkdir utils
aws s3 cp s3://cfstack-init-sources3bucket-b5bxfzywj4ae/utils/generate_assumed_role_creds.py utils/generate_assumed_role_creds.py
eval $(python3 utils/generate_assumed_role_creds.py --role-arn ${PRODUCTION_ACCOUNT_IAM_ROLE_ARN} --session-name SemaphoreAgent)
- env | grep AWS
aws sts get-caller-identity
aws s3 cp clevertap.min.js s3://static.wizrocket.com/js/ --acl public-read
- rm -r clevertap.min.js
+ rm -f -- clevertap.min.js
aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /js/clevertap.min.js📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| set -euo pipefail | |
| mkdir utils | |
| aws s3 cp s3://cfstack-init-sources3bucket-b5bxfzywj4ae/utils/generate_assumed_role_creds.py utils/generate_assumed_role_creds.py | |
| eval $(python3 utils/generate_assumed_role_creds.py --role-arn ${PRODUCTION_ACCOUNT_IAM_ROLE_ARN} --session-name SemaphoreAgent) | |
| env | grep AWS | |
| aws sts get-caller-identity | |
| - 'aws s3 cp clevertap.min.js s3://static.wizrocket.com/js/ --acl public-read' | |
| - rm -r clevertap.min.js | |
| - aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /js/clevertap.min.js | |
| aws s3 cp clevertap.min.js s3://static.wizrocket.com/js/ --acl public-read | |
| rm -r clevertap.min.js | |
| aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /js/clevertap.min.js | |
| set -euo pipefail | |
| mkdir utils | |
| aws s3 cp s3://cfstack-init-sources3bucket-b5bxfzywj4ae/utils/generate_assumed_role_creds.py utils/generate_assumed_role_creds.py | |
| eval $(python3 utils/generate_assumed_role_creds.py --role-arn ${PRODUCTION_ACCOUNT_IAM_ROLE_ARN} --session-name SemaphoreAgent) | |
| aws sts get-caller-identity | |
| aws s3 cp clevertap.min.js s3://static.wizrocket.com/js/ --acl public-read | |
| rm -f -- clevertap.min.js | |
| aws cloudfront create-invalidation --distribution-id E1OCAMMKX0F1A1 --paths /js/clevertap.min.js |
🤖 Prompt for AI Agents
In .semaphore/production-deploy.yml around lines 15 to 23, remove the line that
prints environment variables with "env | grep AWS" (it can leak credentials and
under set -euo pipefail a non-zero grep exit will fail the job) and replace the
"rm -r clevertap.min.js" with "rm -f clevertap.min.js" so removing the file
won't error under strict mode.
singhkunal2050
approved these changes
Sep 4, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SNE-50382
Summary by CodeRabbit