Make Builds Environment-Aware #169
Conversation
• GitHub Actions: Added EXTENSION_ENV to manage builds for dev, staging, and production. • Manifests: Renamed dev manifest to “[Development] Mocksi Lite” and updated production manifest for branding and security. • Vite Config: Enhanced to pick the right manifest based on EXTENSION_ENV.
WalkthroughWalkthroughThe changes involve updates to the GitHub Actions workflow, JSON manifest files, and Vite configuration for the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Build System
Developer->>GitHub Actions: Push code to branch
GitHub Actions->>GitHub Actions: Set EXTENSION_ENV based on branch
GitHub Actions->>Build System: Start build process with EXTENSION_ENV
Build System->>Build System: Create build artifacts
Build System-->>GitHub Actions: Return build artifacts
Tip We have updated our review workflow to use the Anthropic's Claude family of models. Please share any feedback in the discussion post on our Discord. TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
Outside diff range, codebase verification and nitpick comments (1)
apps/mocksi-lite-next/manifest.json (1)
28-28: Inconsistent product name across environments.The product name change to "Mocksi Lite" is not consistently applied across all environments. The old name "Mocksi Lite: Next" still appears in
apps/mocksi-lite-next/manifest.staging.json. Please update the staging manifest to ensure consistency across all configurations and avoid potential confusion.
apps/mocksi-lite-next/manifest.staging.json: Update the name to "Mocksi Lite".Analysis chain
Simplified product name in
manifest.json.The change from "Mocksi Lite: Next" to "Mocksi Lite" simplifies the product's identity, potentially reflecting a branding decision. Ensure that this change is consistently reflected across all user-facing interfaces and marketing materials.
Run the following script to verify the consistency of the product name across user interfaces and marketing materials:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of the product name across user interfaces and marketing materials. # Test: Search for occurrences of the old and new product names. rg "Mocksi Lite: Next" || echo "Old name not found." rg "Mocksi Lite" || echo "New name not found."Length of output: 594
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- .github/workflows/ci.yml (1 hunks)
- apps/mocksi-lite-next/manifest.dev.json (1 hunks)
- apps/mocksi-lite-next/manifest.json (1 hunks)
- apps/mocksi-lite-next/manifest.staging.json (1 hunks)
- apps/mocksi-lite-next/vite.config.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- apps/mocksi-lite-next/manifest.dev.json
Additional context used
actionlint
.github/workflows/ci.yml
45-45: shellcheck reported issue in this script: SC2086:info:1:55: Double quote to prevent globbing and word splitting
(shellcheck)
46-46: unexpected key "45" for "workflow" section. expected one of "concurrency", "defaults", "env", "jobs", "name", "on", "permissions", "run-name"
(syntax-check)
Additional comments not posted (4)
apps/mocksi-lite-next/manifest.json (2)
18-18: Updatedextension_pagesto new domain.The change in the
extension_pagesfield to usehttps://app.mocksi.aifor both child and frame sources reflects a shift in the backend service or hosting environment. Ensure that this new domain is properly configured and secure.Run the following script to verify the availability and security settings of the new domain:
22-22: Updatedmatchesfor external connectivity.The update in the
matchesfield tohttps://app.mocksi.com/*potentially broadens the extension's functionality or alters its integration with other services. Verify that this new URL pattern does not introduce any security or operational issues.Run the following script to verify the impact of the new URL pattern on the extension's functionality:
apps/mocksi-lite-next/vite.config.ts (1)
7-7: Enhanced manifest selection based on environment.The introduction of
stagingManifestand the implementation of a switch statement to select the appropriate manifest based on theEXTENSION_ENVvariable are significant improvements. These changes ensure that the correct settings are applied for each environment, enhancing the application's configurability.Run the following script to verify that the correct manifest is assigned based on the environment:
Also applies to: 19-33
Verification successful
Manifest selection logic is correctly implemented.
The switch statement in
apps/mocksi-lite-next/vite.config.tscorrectly assigns the appropriate manifest based on theEXTENSION_ENVvariable. This ensures that the correct settings are applied for each environment, enhancing the application's configurability.
- For "staging",
activeManifestis a combination ofmanifestandstagingManifest.- For "production",
activeManifestis set tomanifest.- For other environments,
activeManifestcombinesmanifestwithdevManifest.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the correct manifest is assigned based on the environment. # Test: Check the assignment of manifests in different environments. echo "Testing in staging environment:" export EXTENSION_ENV=staging node -e "console.log(require('./vite.config.ts').default)" echo "Testing in production environment:" export EXTENSION_ENV=production node -e "console.log(require('./vite.config.ts').default)"Length of output: 546
Script:
#!/bin/bash # Search for the switch statement or conditional logic in vite.config.ts that selects the manifest based on EXTENSION_ENV. ast-grep --lang typescript --pattern $'switch ($_EXTENSION_ENV) { $$$ }'Length of output: 6955
apps/mocksi-lite-next/manifest.staging.json (1)
1-48: Review of the Entire Manifest FileThis new staging manifest file appears to be well-structured and aligns with the typical conventions for web extensions. Here are some specific observations and suggestions:
- Default Icon (Lines 3-5): The icon file
mocksi-icon.pngis specified. Ensure this file is included in the project assets.- Background Script (Lines 7-9): The service worker is correctly set to
src/pages/background/index.ts. Verify that this file exists and contains appropriate logic for a staging environment.- Content Scripts (Lines 11-16): Scripts are set to run on all URLs, which is typical for browser extensions but ensure this is intended for staging.
- Content Security Policy (Lines 17-19): The policy restricts sources as expected. However, ensure that the domains listed (
https://nest-auth-ts-merge.onrender.com) are correct for staging environments.- Externally Connectable URLs (Lines 21-23): Matches URLs for external connections. Confirm that this URL is correct and that the server configuration allows these connections.
- Icons (Lines 24-26): The larger icon
mocksi-logo.pngis specified. As with the default icon, ensure this asset is available.- Permissions (Lines 30-41): The permissions listed are extensive and include sensitive capabilities like
cookies,downloads, anddebugger. Review these permissions to ensure they are necessary for the staging environment.- Web Accessible Resources (Lines 42-47): Lists resources that are web accessible. Verify that all listed resources are intended to be accessible and are present in the project.
Overall, the manifest file sets up the extension for the staging environment with appropriate settings. It is crucial to ensure all resources and URLs are correctly configured and that the security settings are appropriate for the environment.
.github/workflows/ci.yml
Outdated
| # Set EXTENSION_ENV variable to the current branch name | ||
| - name: Set EXTENSION_ENV variable | ||
| run: echo "EXTENSION_ENV=${GITHUB_REF#refs/heads///-/}" >> $GITHUB_ENV | ||
| 45: | ||
| - name: Build artifacts | ||
| run: pnpm build | ||
| env: | ||
| EXTENSION_ENV: ${{ env.EXTENSION_ENV }} |
There was a problem hiding this comment.
Set EXTENSION_ENV based on branch name.
The addition of setting the EXTENSION_ENV variable based on the branch name enhances the build process's configurability. However, there are issues that need to be addressed:
- Shell Script Issue: Ensure variables are properly quoted to prevent globbing and word splitting.
- Syntax Error: The unexpected key "45" seems to be a typo or copy-paste error.
Apply these fixes to address the issues:
- echo "EXTENSION_ENV=${GITHUB_REF#refs/heads///-/}" >> $GITHUB_ENV
+ echo "EXTENSION_ENV=\"${GITHUB_REF#refs/heads///-/}\"" >> $GITHUB_ENV
- 45:
+ # Correct this line to remove the unexpected keyCommittable 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.
| # Set EXTENSION_ENV variable to the current branch name | |
| - name: Set EXTENSION_ENV variable | |
| run: echo "EXTENSION_ENV=${GITHUB_REF#refs/heads///-/}" >> $GITHUB_ENV | |
| 45: | |
| - name: Build artifacts | |
| run: pnpm build | |
| env: | |
| EXTENSION_ENV: ${{ env.EXTENSION_ENV }} | |
| # Set EXTENSION_ENV variable to the current branch name | |
| - name: Set EXTENSION_ENV variable | |
| run: echo "EXTENSION_ENV=\"${GITHUB_REF#refs/heads///-/}\"" >> $GITHUB_ENV | |
| # Correct this line to remove the unexpected key | |
| - name: Build artifacts | |
| run: pnpm build | |
| env: | |
| EXTENSION_ENV: ${{ env.EXTENSION_ENV }} |
Tools
actionlint
45-45: shellcheck reported issue in this script: SC2086:info:1:55: Double quote to prevent globbing and word splitting
(shellcheck)
46-46: unexpected key "45" for "workflow" section. expected one of "concurrency", "defaults", "env", "jobs", "name", "on", "permissions", "run-name"
(syntax-check)
.github/workflows/ci.yml
Outdated
| # Set EXTENSION_ENV variable to the current branch name | ||
| - name: Set EXTENSION_ENV variable | ||
| run: echo "EXTENSION_ENV=${GITHUB_REF#refs/heads///-/}" >> $GITHUB_ENV | ||
| 45: |
There was a problem hiding this comment.
@elg0nz take a look at this was this intended? I ran the build and zip action and those were the errors ☝🏻
There was a problem hiding this comment.
Nope, that was a typo :face-palm: correcting it now!
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/ci.yml (1 hunks)
Additional context used
actionlint
.github/workflows/ci.yml
45-45: shellcheck reported issue in this script: SC2086:info:1:73: Double quote to prevent globbing and word splitting
(shellcheck)
Additional comments not posted (1)
.github/workflows/ci.yml (1)
48-49: LGTM!Including the
EXTENSION_ENVvariable in the build step's environment context is necessary for the build process to utilize this variable.
.github/workflows/ci.yml
Outdated
| # Set EXTENSION_ENV variable to the current branch name | ||
| - name: Set EXTENSION_ENV variable | ||
| run: echo "EXTENSION_ENV=\"${GITHUB_REF##refs/heads/}\"" | sed 's/\//-/g' >> $GITHUB_ENV |
There was a problem hiding this comment.
Great enhancement to make the build process environment-aware!
Setting the EXTENSION_ENV variable based on the current branch name is a nice way to make the build process more configurable.
Fix the shellcheck issue.
actionlint has flagged a potential shellcheck issue in this step. To prevent globbing and word splitting, double quote the variable.
Apply this fix to address the shellcheck issue:
- run: echo "EXTENSION_ENV=\"${GITHUB_REF##refs/heads/}\"" | sed 's/\//-/g' >> $GITHUB_ENV
+ run: echo "EXTENSION_ENV=\"${GITHUB_REF##refs/heads/}\"" | sed 's/\//-/g' >> "$GITHUB_ENV"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.
| # Set EXTENSION_ENV variable to the current branch name | |
| - name: Set EXTENSION_ENV variable | |
| run: echo "EXTENSION_ENV=\"${GITHUB_REF##refs/heads/}\"" | sed 's/\//-/g' >> $GITHUB_ENV | |
| # Set EXTENSION_ENV variable to the current branch name | |
| - name: Set EXTENSION_ENV variable | |
| run: echo "EXTENSION_ENV=\"${GITHUB_REF##refs/heads/}\"" | sed 's/\//-/g' >> "$GITHUB_ENV" |
Tools
actionlint
45-45: shellcheck reported issue in this script: SC2086:info:1:73: Double quote to prevent globbing and word splitting
(shellcheck)
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
.github/workflows/ci.yml (1)
43-45: Great enhancement to make the build process environment-aware!Setting the
EXTENSION_ENVvariable based on the current branch name is a nice way to make the build process more configurable.Fix the shellcheck issue.
actionlint has flagged a potential shellcheck issue in this step:
45-45: shellcheck reported issue in this script: SC2086:info:1:69: Double quote to prevent globbing and word splitting
To prevent globbing and word splitting, double quote the variable.
Apply this fix to address the shellcheck issue:
- run: echo "EXTENSION_ENV=${GITHUB_REF##refs/heads/}" | sed 's/\//-/g' >> $GITHUB_ENV + run: echo "EXTENSION_ENV=${GITHUB_REF##refs/heads/}" | sed 's/\//-/g' >> "$GITHUB_ENV"Tools
actionlint
45-45: shellcheck reported issue in this script: SC2086:info:1:69: Double quote to prevent globbing and word splitting
(shellcheck)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/ci.yml (1 hunks)
Additional context used
actionlint
.github/workflows/ci.yml
45-45: shellcheck reported issue in this script: SC2086:info:1:69: Double quote to prevent globbing and word splitting
(shellcheck)
|
lgtm! |
• GitHub Actions: Added EXTENSION_ENV to manage builds for dev, staging, and production.
• Manifests: Renamed dev manifest to “[Development] Mocksi Lite” and updated production manifest for branding and security.
• Vite Config: Enhanced to pick the right manifest based on EXTENSION_ENV.
Summary by CodeRabbit
New Features
Improvements
Branding Changes