-
-
Notifications
You must be signed in to change notification settings - Fork 28
[#1918] Updated CI config to export and restore exported codebase only for artifact deployment. #1924
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
Conversation
WalkthroughAdds a reusable .env loading step to CircleCI and GitHub Actions workflows and wires it into multiple jobs. Introduces conditional gating so export/upload/download of built codebase artifacts run only when VORTEX_DEPLOY_TYPES includes "artifact". Updates actionlint config to ignore SC1090 in a shared workflow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Trigger
participant CI as CI Runner
participant Build as Build Job
participant Env as Env Loader Step
participant Exp as Export Step
participant Store as Artifact Store
participant Deploy as Deploy Job
participant DL as Download Step
Dev->>CI: Push / Workflow dispatch
CI->>Build: Start matrix job(s)
Build->>Env: Source .env, export to env
alt VORTEX_DEPLOY_TYPES includes "artifact"
Build->>Exp: Export built codebase
Exp->>Store: Upload artifact
else
Build--xExp: Skip export/upload
end
CI->>Deploy: Start deploy job(s)
Deploy->>Env: Source .env, export to env
alt VORTEX_DEPLOY_TYPES includes "artifact"
Deploy->>Store: Download artifact
Store-->>Deploy: Provide exported codebase
else
Deploy--xDL: Skip artifact download
end
note over Env,Deploy: .env variables propagated to subsequent steps
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
🪧 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 (
|
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.circleci/config.yml (1)
275-281: Gate condition can be simpler and clearerThe grep pipeline works, but a shell pattern check is simpler and avoids subshells.
- echo "${VORTEX_DEPLOY_TYPES:-}" | grep -vq "artifact" && exit 0 || true + [[ "${VORTEX_DEPLOY_TYPES:-}" == *artifact* ]] || exit 0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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 (2)
.circleci/config.yml(2 hunks).github/workflows/build-test-deploy.yml(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.4)
🔇 Additional comments (3)
.github/workflows/build-test-deploy.yml (3)
254-259: LGTM: artifact export gated correctlyRunning export only on matrix.instance == 0 and when VORTEX_DEPLOY_TYPES contains 'artifact' aligns with the objective and avoids redundant work.
368-375: LGTM: artifact upload gating is correctThe extra guard for deps/* branches plus the 'artifact' check keeps uploads scoped to intended runs.
418-422: LGTM: artifact download gated properlyDownloading the exported codebase only when VORTEX_DEPLOY_TYPES includes 'artifact' satisfies the conditional deploy objective.
| - name: Load environment variables from .env | ||
| run: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$GITHUB_ENV" | ||
|
|
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.
🛠️ Refactor suggestion
Repeat the safe .env loader here as well
Same concerns as in the build job: avoid restoring the snapshot after sourcing .env and avoid dumping the whole environment to GITHUB_ENV.
- - name: Load environment variables from .env
- run: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$GITHUB_ENV"
+ - name: Load environment variables from .env (safe)
+ run: |
+ while IFS='=' read -r key rest; do
+ [ -z "${key}" ] && continue
+ case "${key}" in \#* ) continue;; esac
+ key="${key%%[[:space:]]*}"
+ {
+ echo "${key}<<__ENV__"
+ printf '%s\n' "${rest}"
+ echo "__ENV__"
+ } >> "$GITHUB_ENV"
+ done < .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.
| - name: Load environment variables from .env | |
| run: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$GITHUB_ENV" | |
| - name: Load environment variables from .env (safe) | |
| run: | | |
| while IFS='=' read -r key rest; do | |
| [ -z "${key}" ] && continue | |
| case "${key}" in \#* ) continue;; esac | |
| key="${key%%[[:space:]]*}" | |
| { | |
| echo "${key}<<__ENV__" | |
| printf '%s\n' "${rest}" | |
| echo "__ENV__" | |
| } >> "$GITHUB_ENV" | |
| done < .env |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1924 +/- ##
========================================
Coverage 78.69% 78.69%
========================================
Files 85 85
Lines 5027 5027
Branches 35 35
========================================
Hits 3956 3956
Misses 1071 1071 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
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)
.circleci/config.yml (1)
301-305: Harden artifact gating to match tokens and avoid false positives.Current grep -vq "artifact" succeeds on any non-matching line (including empty input) and treats substrings. Use a token-aware check and a clear message. This keeps the step a no-op unless VORTEX_DEPLOY_TYPES contains the artifact token.
- echo "${VORTEX_DEPLOY_TYPES:-}" | grep -vq "artifact" && exit 0 || true + if ! printf '%s\n' "${VORTEX_DEPLOY_TYPES:-}" | grep -qE '(^|[ ,:])artifact([ ,:]|$)'; then + echo "Skipping export: VORTEX_DEPLOY_TYPES does not contain 'artifact'." + exit 0 + fi
♻️ Duplicate comments (5)
.circleci/config.yml (5)
218-221: Stop printing full environment; remove high‑risk secret leakage.Dumping the entire environment to logs can expose credentials/tokens. Remove this step.
- - run: - name: TEST VARS BEFORE - command: env|sort
222-225: Delete commented-out unsafe .env loader to avoid confusion.Good that the unsafe loader is no longer executed; let’s also remove the dead code for clarity.
-# - run: -# name: Load environment variables from .env -# command: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$BASH_ENV"
241-243: Stop printing full environment post-mutation.Same exposure risk as earlier; remove.
- - run: - name: TEST VARS AFTER - command: env|sort
245-249: Fix unsafe and ineffective .env loading; current approach reverts .env values and writes unquoted exports.Problems:
- Resourcing the pre-.env snapshot overrides values set by .env.
- Appending unquoted export lines can break on spaces/newlines.
- Only VORTEX_DEPLOY_TYPES is persisted; others are lost.
Replace with a safe exporter that:
- Reads only keys from .env, ignores blanks/comments.
- Properly escapes values.
- Optionally logs variable names (not values) when VORTEX_DEBUG=1.
- - run: - name: Load environment variables from .env - TEST2 - command: | - t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" - echo "export VORTEX_DEPLOY_TYPES=${VORTEX_DEPLOY_TYPES}" >> "$BASH_ENV" + - run: + name: Load environment variables from .env (safe) + command: | + [ -f .env ] || exit 0 + # Export only variables defined in .env; ignore comments/empty lines. + while IFS='=' read -r key rest; do + # Skip empty lines and comments. + [ -z "${key}" ] && continue + case "${key}" in \#* ) continue;; esac + # Trim trailing whitespace in key. + key="${key%%[[:space:]]*}" + # Write safely escaped export to $BASH_ENV for subsequent steps. + printf 'export %s=%q\n' "$key" "$rest" >> "$BASH_ENV" + done < .env + # Optional: list names only when debugging + [ "${VORTEX_DEBUG:-}" = "1" ] && awk -F= '!/^($|#)/{print "Loaded: "$1}' .env || true
251-253: Remove second full environment dump.Avoid logging all env vars again; it provides no additional signal and leaks secrets.
- - run: - name: TEST2 VARS AFTER - command: env|sort
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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 (1)
.circleci/config.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (1)
.circleci/config.yml (1)
419-423: Workspace persist runs unconditionally; verify downstream restore behavior is guarded.Persisting an empty /tmp/workspace/code is harmless, but ensure downstream usage (e.g., in deploy scripts) is conditioned on VORTEX_DEPLOY_TYPES containing artifact, per PR objective “export and restore only for artifact deployments.”
Would you like me to scan the repo to confirm deploy.sh only consumes ${VORTEX_DEPLOY_ARTIFACT_SRC} when artifact deployments are selected? I can run a script to check for gating and usage patterns.
4a96863 to
6686ff8
Compare
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.circleci/config.yml (1)
265-267: Do not source .env; parse only what you need to avoid code execution and env clobbering.The current loader executes ./.env and then restores the prior environment snapshot. This is brittle and can execute arbitrary shell code embedded in .env values. It also reverts overlapping variables to their pre-.env values, which can surprise maintainers.
Gate on "artifact" without executing .env by reading only VORTEX_DEPLOY_TYPES and respecting pre-set env. Also match tokens robustly (comma/space separated).
Apply:
- # Load environment variables from .env file, respecting existing values. - t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" - echo "${VORTEX_DEPLOY_TYPES:-}" | grep -vq "artifact" && exit 0 || true + # Determine deploy types without executing ./.env; respect existing env value. + VDT="${VORTEX_DEPLOY_TYPES:-$(awk -F= '/^[[:space:]]*VORTEX_DEPLOY_TYPES[[:space:]]*=/{sub(/^[^=]*=/,""); gsub(/^[[:space:]]+|[[:space:]]+$/,""); print; exit}' ./.env)}" + # Split on commas/spaces and require an exact token match for 'artifact'. + printf '%s\n' "${VDT}" | tr ', ' '\n' | grep -qx "artifact" || exit 0If you later need these variables across subsequent steps, prefer the previously suggested safe exporter that writes only sanitized export lines to $BASH_ENV instead of dot-sourcing .env.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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 (2)
.circleci/config.yml(1 hunks).env(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (0)
.env
Outdated
| # Deployment occurs when tests pass in the CI environment. | ||
| # @see https://www.vortextemplate.com/docs/workflows/deployment | ||
| VORTEX_DEPLOY_TYPES=artifact | ||
| VORTEX_DEPLOY_TYPES=lagoon |
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.
💡 Verification agent
🧩 Analysis chain
Changing default to 'lagoon' effectively disables artifact export/restore by default; verify CI/docs expectations.
With VORTEX_DEPLOY_TYPES=lagoon, the CircleCI export step (and corresponding GH workflows per PR summary) will skip artifact-only paths unless the variable contains "artifact". This matches the PR objective. Please confirm:
- No CI contexts/org-level envs still set VORTEX_DEPLOY_TYPES=artifact, which would unintentionally re-enable artifact paths.
- Docs/examples mention the accepted, comma-separated values (e.g., "artifact,lagoon") and the “contains artifact” semantics.
Optionally add a note in .env.local.example to guide users on composing multiple deploy types.
Run this script to find other uses and ensure consistent gating across the repo:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for VORTEX_DEPLOY_TYPES usage and gating patterns..."
rg -n -C2 --hidden --glob '!.git' '\bVORTEX_DEPLOY_TYPES\b|artifact' || true
echo
echo "Check if any CI files hardcode 'artifact' behavior outside the gate:"
rg -n -C2 --hidden --glob '!.git' -g '.circleci/**' -g '.github/workflows/**' '(export|upload|download).*artifact' || trueLength of output: 222930
Update documentation for VORTEX_DEPLOY_TYPES default and confirm CI gating
Confirmed:
- The CircleCI config (
.circleci/config.yml) usesto skip the export step when “artifact” isn’t present, and the GitHub Actions workflows inecho "${VORTEX_DEPLOY_TYPES:-}" | grep -vq "artifact" && exit 0 || true
build-test-deploy.ymlguard all artifact export/download steps withcontains(env.VORTEX_DEPLOY_TYPES, 'artifact'). - No other CI files hardcode an “artifact” deployment outside these gates—other
upload-artifactusages are for test and coverage artifacts, not code deployments .
Please also manually verify that no organization-level CircleCI contexts or GitHub environment settings still set VORTEX_DEPLOY_TYPES=artifact, which would unintentionally re-enable code–artifact deployment.
Required changes:
- In
docs/content/workflows/variables.mdx, update the default forVORTEX_DEPLOY_TYPESfromartifacttolagoon:### `VORTEX_DEPLOY_TYPES` - Default value: `artifact` + Default value: `lagoon`
- (If you maintain a
.env.local.example) Add a comment illustrating how to compose multiple values, e.g.:# Comma-separate multiple deploy types (e.g. "artifact,lagoon") VORTEX_DEPLOY_TYPES=lagoon
🤖 Prompt for AI Agents
In .env (line 214) and docs/content/workflows/variables.mdx (update default),
change the documented/default VORTEX_DEPLOY_TYPES value from "artifact" to
"lagoon" and add a comment/example showing how to compose multiple values;
specifically update docs/content/workflows/variables.mdx to state the new
default is "lagoon" and, if you maintain a .env.local.example, add a one-line
comment before the variable demonstrating comma-separated values (e.g.
"artifact,lagoon") and set VORTEX_DEPLOY_TYPES=lagoon; finally, manually verify
there are no organisation-level CircleCI contexts or GitHub environment settings
that still set VORTEX_DEPLOY_TYPES=artifact to avoid unintentionally re-enabling
artifact deployments.
a90d11b to
e09a0f7
Compare
70b6ef2 to
bf5745e
Compare
…y for artifact deployment.
bf5745e to
8bef1f7
Compare
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.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.github/workflows/build-test-deploy.yml (2)
202-204: Replace unsafe .env loader; it negates .env values and dumps the full environmentIssues with the current one-liner:
- Restores the pre-.env snapshot after sourcing, overriding .env values.
- Appends the entire environment to GITHUB_ENV, risking leakage/pollution and breaking on special chars.
- Fails if .env is missing.
Use a safe, minimal exporter that reads .env without sourcing and writes only those keys to GITHUB_ENV with multiline safety.
Apply:
- - name: Load environment variables from .env - run: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$GITHUB_ENV" + - name: Load environment variables from .env (safe) + run: | + if [ -f .env ]; then + while IFS='=' read -r key rest; do + [ -z "${key}" ] && continue + case "${key}" in \#* ) continue;; esac + key="${key%%[[:space:]]*}" + { + echo "${key}<<__ENV__" + printf '%s\n' "${rest}" + echo "__ENV__" + } >> "$GITHUB_ENV" + done < .env + fi
413-415: Repeat the safe .env loader here to avoid sourcing and environment dumpingSame issues as in the build job. Replace with the safe loader to avoid SC1090 and prevent accidental env pollution.
Apply:
- - name: Load environment variables from .env - run: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$GITHUB_ENV" + - name: Load environment variables from .env (safe) + run: | + if [ -f .env ]; then + while IFS='=' read -r key rest; do + [ -z "${key}" ] && continue + case "${key}" in \#* ) continue;; esac + key="${key%%[[:space:]]*}" + { + echo "${key}<<__ENV__" + printf '%s\n' "${rest}" + echo "__ENV__" + } >> "$GITHUB_ENV" + done < .env + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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 ignored due to path filters (9)
.vortex/installer/tests/Fixtures/install/_baseline/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/ciprovider_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_all_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_none_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_none_gha/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deps_updates_provider_ci_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_profile/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_absent/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/timezone_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (3)
.circleci/config.yml(7 hunks).github/workflows/build-test-deploy.yml(4 hunks).github/workflows/vortex-test-common.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-docs
🔇 Additional comments (5)
.github/workflows/build-test-deploy.yml (3)
254-255: Gating export by VORTEX_DEPLOY_TYPES='artifact' looks correctThe condition ensures export runs only for artifact deployments and only once per matrix batch. This aligns with #1918.
Double-check callers of this artifact in the deploy job to ensure non-artifact deploy paths don’t assume the exported code exists.
368-369: Upload gating is consistent and excludes deps/ branches*Condition matches the objective: upload happens only on instance 0, not on deps/* branches, and only when artifact deployments are requested.
418-419: Artifact deployment gating verified
Thedeploy-artifact.shinvocation inscripts/vortex/deploy.sh(around line 111) is correctly wrapped in anif [ -z "${VORTEX_DEPLOY_TYPES##*artifact*}" ]check, so allVORTEX_DEPLOY_ARTIFACT_*validations inscripts/vortex/deploy-artifact.shonly run whenartifactis included inVORTEX_DEPLOY_TYPES. No unconditional artifact requirement was found..circleci/config.yml (2)
139-140: Confirmed .env variables loaded by CircleCI anchor
The provided script output shows all expected VORTEX_* keys present in the job environment, verifying that the updated anchor correctly loads variables from .env. No further changes are needed at any call sites.• Validated anchor usages in .circleci/config.yml (lines 139–140, 221, 407–408, 434–435, 506–507)
• All dependent steps now receive .env-derived variables as intendedApproving code changes.
270-273: Export built codebase gated behind VORTEX_DEPLOY_TYPES='artifact'This early exit prevents unnecessary export on non-artifact runs and matches the PR objective.
You can drop the trailing
|| trueaftergrep -vqsinceexit 0short-circuits; the next commands won’t execute.- echo "${VORTEX_DEPLOY_TYPES:-}" | grep -vq "artifact" && exit 0 || true + echo "${VORTEX_DEPLOY_TYPES:-}" | grep -vq "artifact" && exit 0Likely an incorrect or invalid review comment.
| - &load_variables_from_dotenv | ||
| run: | ||
| name: Load environment variables from .env file | ||
| # Load variables from .env file, respecting existing values, and make them available for the next steps. | ||
| command: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && export -p >> "$BASH_ENV" | ||
|
|
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.
Fix unsafe/ineffective .env loader anchor; export only .env keys to $BASH_ENV
Current anchor:
- Re-sources the pre-.env snapshot, undoing .env values.
- Appends the entire exported environment to $BASH_ENV, risking pollution and quoting issues.
- Triggers SC1090 due to sourcing .env.
Use a safe parser that writes only .env variables to $BASH_ENV with proper quoting. This also removes the need to ignore SC1090 elsewhere.
Apply:
- &load_variables_from_dotenv
run:
- name: Load environment variables from .env file
- # Load variables from .env file, respecting existing values, and make them available for the next steps.
- command: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && export -p >> "$BASH_ENV"
+ name: Load environment variables from .env file (safe)
+ command: |
+ if [ -f .env ]; then
+ # Export only keys from .env; ignore comments/blank lines.
+ while IFS='=' read -r key rest; do
+ [ -z "${key}" ] && continue
+ case "${key}" in \#* ) continue;; esac
+ key="${key%%[[:space:]]*}"
+ # Safely quote for BASH_ENV
+ printf 'export %s=%q\n' "$key" "$rest" >> "$BASH_ENV"
+ done < .env
+ fi
+ # Optional: names-only debug
+ [ "${VORTEX_DEBUG:-}" = "1" ] && awk -F= '!/^($|#)/{print "Loaded: "$1}' .env || true📝 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.
| - &load_variables_from_dotenv | |
| run: | |
| name: Load environment variables from .env file | |
| # Load variables from .env file, respecting existing values, and make them available for the next steps. | |
| command: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && export -p >> "$BASH_ENV" | |
| - &load_variables_from_dotenv | |
| run: | |
| name: Load environment variables from .env file (safe) | |
| command: | | |
| if [ -f .env ]; then | |
| # Export only keys from .env; ignore comments/blank lines. | |
| while IFS='=' read -r key rest; do | |
| [ -z "${key}" ] && continue | |
| case "${key}" in \#* ) continue;; esac | |
| key="${key%%[[:space:]]*}" | |
| # Safely quote for BASH_ENV | |
| printf 'export %s=%q\n' "$key" "$rest" >> "$BASH_ENV" | |
| done < .env | |
| fi | |
| # Optional: names-only debug | |
| [ "${VORTEX_DEBUG:-}" = "1" ] && awk -F= '!/^($|#)/{print "Loaded: "$1}' .env || true |
🤖 Prompt for AI Agents
In .circleci/config.yml around lines 100 to 105, replace the unsafe anchor that
snapshots the environment, sources .env, then appends the entire exported
environment to $BASH_ENV; instead remove the tmp snapshot and the export -p
append and implement a safe .env writer: parse .env without sourcing (ignore
comments/blank lines), validate KEY=VALUE lines, shell-quote values correctly,
and append only those KEY="quoted value" export statements to $BASH_ENV so
existing environment is respected, no pre-resourcing is undone, and SC1090 is
avoided.
|
|
||
| - name: Check coding standards with actionlint | ||
| run: docker run --rm -v "${GITHUB_WORKSPACE:-.}":/app --workdir /app rhysd/actionlint:1.7.2 -ignore 'SC2002:' -ignore 'SC2155:' -ignore 'SC2015:' -ignore 'SC2046:' | ||
| run: docker run --rm -v "${GITHUB_WORKSPACE:-.}":/app --workdir /app rhysd/actionlint:1.7.2 -ignore 'SC2002:' -ignore 'SC2155:' -ignore 'SC2015:' -ignore 'SC2046:' -ignore 'SC1090:' |
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.
🧹 Nitpick (assertive)
Avoid blanket-ignoring SC1090; eliminate the root cause instead
SC1090 is triggered by sourcing .env in other workflows. Prefer switching those steps to a non-sourcing .env loader (parsing key=value and writing to GITHUB_ENV/BASH_ENV) which removes SC1090 entirely. Then drop this global ignore to keep signal from shellcheck high.
If you keep SC1090 ignored here, scope it with a regex to only the specific jobs/files that need it, rather than every workflow.
Once you adopt the safe loader in build-test-deploy.yml and CircleCI, you can safely apply:
- run: docker run --rm -v "${GITHUB_WORKSPACE:-.}":/app --workdir /app rhysd/actionlint:1.7.2 -ignore 'SC2002:' -ignore 'SC2155:' -ignore 'SC2015:' -ignore 'SC2046:' -ignore 'SC1090:'
+ run: docker run --rm -v "${GITHUB_WORKSPACE:-.}":/app --workdir /app rhysd/actionlint:1.7.2 -ignore 'SC2002:' -ignore 'SC2155:' -ignore 'SC2015:' -ignore 'SC2046:'📝 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.
| run: docker run --rm -v "${GITHUB_WORKSPACE:-.}":/app --workdir /app rhysd/actionlint:1.7.2 -ignore 'SC2002:' -ignore 'SC2155:' -ignore 'SC2015:' -ignore 'SC2046:' -ignore 'SC1090:' | |
| run: docker run --rm -v "${GITHUB_WORKSPACE:-.}":/app --workdir /app rhysd/actionlint:1.7.2 -ignore 'SC2002:' -ignore 'SC2155:' -ignore 'SC2015:' -ignore 'SC2046:' |
🤖 Prompt for AI Agents
In .github/workflows/vortex-test-common.yml around line 276 the actionlint
invocation is blanket-ignoring SC1090; remove SC1090 from the global -ignore
list and instead fix the root cause by changing any workflow steps that "source"
.env to a safe loader that parses key=value and writes to GITHUB_ENV or BASH_ENV
(update build-test-deploy.yml and CircleCI accordingly); if you cannot update
all callers immediately, narrow the ignore to a regex that targets only the
specific workflow files/jobs that legitimately need SC1090 until the safe loader
is adopted, then drop the ignore entirely.
Closes #1918
Also variables from
.envto be available in the build steps, respecting existing values.Summary by CodeRabbit