Testing 1423 #1943
Testing 1423 #1943
Conversation
WalkthroughAdds branch- and event-based gating to CI: a CircleCI pre-deploy halt for PRs from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Circle as CircleCI
rect rgba(230,245,255,0.6)
note over GH: GitHub Actions workflow gating
Dev->>GH: Push / PR / Schedule
alt event == schedule
GH-->>GH: Skip Build & Deploy
GH->>GH: Run Database (conditional)
else event != schedule
alt head_ref startsWith "project/"
GH-->>GH: Skip Build & Deploy for PRs from project/*
else if ref_name or head_ref startsWith "deps/"
GH-->>GH: Skip Deploy (deps/*)
GH->>GH: Run allowed jobs (e.g., DB, Build as applicable)
else
GH->>GH: Run Database, Build, Deploy
end
end
end
rect rgba(235,255,235,0.6)
note over Circle: CircleCI pre-deploy guard
Dev->>Circle: Pipeline (PR or push)
alt PR and CIRCLE_BRANCH startsWith "project/"
Circle->>Circle: Print "Skipping deployment - PR from project/* branch"
Circle->>Circle: `circleci-agent step halt` -> Halt deploy step
Circle-->>Dev: Deploy skipped
else
Circle->>Circle: Proceed to Deploy
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-test-deploy.yml (1)
217-219: Quoting bug renders the “Show cache key” echo invalidSingle quotes are nested inside single quotes; the shell will treat the inner quotes as string terminators. Use double quotes outside.
- - name: Show cache key for database caching - run: echo 'v25.7.0-db11-${{ hashFiles('db_cache_branch') }}-${{ hashFiles('db_cache_fallback_yes') }}-${{ hashFiles('db_cache_timestamp') }}' + - name: Show cache key for database caching + run: echo "v25.7.0-db11-${{ hashFiles('db_cache_branch') }}-${{ hashFiles('db_cache_fallback_yes') }}-${{ hashFiles('db_cache_timestamp') }}"
📜 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 (3)
.circleci/config.yml(1 hunks).github/workflows/build-test-deploy.yml(3 hunks).vortex/CLAUDE.md(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-02T00:41:36.321Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#1697
File: .vortex/installer/tests/Fixtures/install/provision_database_lagoon/docs/onboarding.md:16-17
Timestamp: 2025-06-02T00:41:36.321Z
Learning: DO NOT EVER REVIEW FILES IN .vortex/installer/tests/Fixtures/ directory - these are test fixtures and should not be reviewed under any circumstances.
Applied to files:
.vortex/CLAUDE.md
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.
Applied to files:
.vortex/CLAUDE.md
📚 Learning: 2025-05-29T12:15:32.188Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#0
File: :0-0
Timestamp: 2025-05-29T12:15:32.188Z
Learning: Do not review files in `.vortex/installer/tests/Fixtures/install` directory as they are test fixtures.
Applied to files:
.vortex/CLAUDE.md
🪛 LanguageTool
.vortex/CLAUDE.md
[grammar] ~623-~623: Use correct spacing
Context: ...es ## Important AI Assistant Guidelines CRITICAL: NEVER directly modify files ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~625-~625: Use correct spacing
Context: ...ust be updated by the user MANUALLY. --- *This knowledge base should be updated wh...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
⏰ 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 (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
🔇 Additional comments (4)
.circleci/config.yml (1)
423-430: Clarification:circleci-agent step haltstops the entire job, not just the stepAccording to the CircleCI documentation, invoking
circleci-agent step halt“ends a job gracefully” and prevents any subsequent steps in that job from running (support.circleci.com). Community discussion further confirms that while commands remaining in the same run-step will still execute, the following steps in the job are skipped (discuss.circleci.com).
• Your existing guard in its own run-step (Lines 423–430) already halts the job and thus skips the “Deploy” step.
• There is no need to move the guard into the Deploy step to achieve the desired behavior.(Optional) For readability/style, you may still prefer the more idiomatic Bash checks and single-quoted regex as shown in the original diff. If you also want to skip direct pushes to
project/*branches (not just PRs), you can extend the condition to include a branch check on${CIRCLE_BRANCH}.Likely an incorrect or invalid review comment.
.github/workflows/build-test-deploy.yml (3)
57-57: DB job gating looks correct for PRs from project/ and for pushes/schedules*Condition runs DB on pushes and on PRs not from project/*; scheduled runs are allowed. This aligns with the stated policy.
Please confirm that DB should NOT run for PRs from project/* branches; current condition enforces that.
161-161: Build job gating correctly blocks schedules and PRs from project/The expression is sound: no schedules; allow pushes; block PRs where head_ref starts with project/. Good parity with CircleCI intent.
403-403: Deploy job gating is consistent and excludes deps/Blocks schedules, excludes deps/* (using head_ref || ref_name), and prevents deploys for PRs from project/*. Matches policy and complements CircleCI once the CircleCI guard is fixed.
| #;< !PROVISION_TYPE_PROFILE | ||
| database: | ||
| runs-on: ubuntu-latest | ||
| if: ${{ github.event_name == 'push' || !startsWith(github.head_ref, 'project/') }} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optional: coalesce head_ref to avoid null-string ambiguity
GitHub coerces empty to empty-string, but being explicit improves readability. Same applies to other startsWith checks.
- if: ${{ github.event_name == 'push' || !startsWith(github.head_ref, 'project/') }}
+ if: ${{ github.event_name == 'push' || !startsWith(github.head_ref || '', 'project/') }}Apply similarly where you rely on head_ref alone.
📝 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.
| if: ${{ github.event_name == 'push' || !startsWith(github.head_ref, 'project/') }} | |
| if: ${{ github.event_name == 'push' || !startsWith(github.head_ref || '', 'project/') }} |
🤖 Prompt for AI Agents
In .github/workflows/build-test-deploy.yml around line 57, the conditional uses
startsWith(github.head_ref, 'project/') which can be ambiguous when head_ref is
empty; update the expression to coalesce github.head_ref to an empty string
(e.g., startsWith(coalesce(github.head_ref, ''), 'project/')) and apply the same
coalesce pattern to any other places that rely on github.head_ref alone.
| ## Important AI Assistant Guidelines | ||
| **CRITICAL**: NEVER directly modify files under `.vortex/installer/tests/Fixtures/`. These are test fixtures that must be updated by the user **MANUALLY**. | ||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Tighten wording and fix spacing around the new guideline block
Minor polish: add blank lines for proper Markdown rendering and avoid ALL CAPS. Also cross-link the supported update mechanism for fixtures. I used the retrieved learnings you shared to ensure the guidance matches team policy.
-## Important AI Assistant Guidelines
-
-**CRITICAL**: NEVER directly modify files under `.vortex/installer/tests/Fixtures/`. These are test fixtures that must be updated by the user **MANUALLY**.
-
----
+## Important AI Assistant Guidelines
+
+**Critical:** Never directly modify files under `.vortex/installer/tests/Fixtures/`. These are test fixtures and must be updated manually using the process described in “Installer Fixture System” (see “Updating Fixtures”).
+
+---📝 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.
| ## Important AI Assistant Guidelines | |
| **CRITICAL**: NEVER directly modify files under `.vortex/installer/tests/Fixtures/`. These are test fixtures that must be updated by the user **MANUALLY**. | |
| ## Important AI Assistant Guidelines | |
| **Critical:** Never directly modify files under `.vortex/installer/tests/Fixtures/`. These are test fixtures and must be updated manually using the process described in “Installer Fixture System” (see “Updating Fixtures”). | |
| --- |
🧰 Tools
🪛 LanguageTool
[grammar] ~623-~623: Use correct spacing
Context: ...es ## Important AI Assistant Guidelines CRITICAL: NEVER directly modify files ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~625-~625: Use correct spacing
Context: ...ust be updated by the user MANUALLY. --- *This knowledge base should be updated wh...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
.vortex/CLAUDE.md lines 623-626: The guideline block needs wording tightened and
spacing fixed; replace the ALL CAPS "CRITICAL" phrasing with normal-case, add
blank lines before and after the block so Markdown renders correctly, rewrite
the sentence to be concise (e.g., "Important: Do not modify files under
`.vortex/installer/tests/Fixtures/`. These test fixtures must be updated
manually by the user.") and add a short parenthetical/link to the supported
fixture update mechanism (point to the repo's fixture update docs or script).
Ensure punctuation and spacing are consistent with surrounding paragraphs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1943 +/- ##
===========================================
- Coverage 76.26% 75.38% -0.88%
===========================================
Files 87 39 -48
Lines 5392 3185 -2207
Branches 35 0 -35
===========================================
- Hits 4112 2401 -1711
+ Misses 1280 784 -496 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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)
.env(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
.env (1)
scripts/vortex/task-custom-lagoon.sh (1)
lagoon(97-97)
⏰ 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 (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-deployment (1)
- 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)
| # 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 | ||
| #;> DEPLOYMENT | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Switching deploy target to 'lagoon' — verify consumers and required env are satisfied
Confirm the following before merging:
- Parsers of VORTEX_DEPLOY_TYPES accept the value "lagoon" (and whether multiple providers via comma-separated values are supported).
- CI/job gates added in this PR actually respect this value and won’t accidentally short-circuit deploys for project/* or schedule events.
- Lagoon-specific variables are present where needed (usually in CI): VORTEX_TASK_SSH_FILE, VORTEX_TASK_LAGOON_INSTANCE, VORTEX_TASK_LAGOON_PROJECT. The lagoon() wrapper in scripts/vortex/task-custom-lagoon.sh expects these. If they’re injected elsewhere, all good; otherwise consider documenting their source.
Optional: add a brief inline comment here to document accepted values and required vars for Lagoon, to save readers a grep.
Apply this small doc enhancement:
# Deployment occurs when tests pass in the CI environment.
# @see https://www.vortextemplate.com/docs/workflows/deployment
-VORTEX_DEPLOY_TYPES=lagoon
+# Deployment providers (comma-separated). Example: lagoon
+# For Lagoon deployments, ensure the following are provided (typically via CI secrets):
+# - VORTEX_TASK_SSH_FILE
+# - VORTEX_TASK_LAGOON_INSTANCE
+# - VORTEX_TASK_LAGOON_PROJECT
+VORTEX_DEPLOY_TYPES=lagoon📝 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.
| # 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 | |
| #;> DEPLOYMENT | |
| # Deployment occurs when tests pass in the CI environment. | |
| # @see https://www.vortextemplate.com/docs/workflows/deployment | |
| # Deployment providers (comma-separated). Example: lagoon | |
| # For Lagoon deployments, ensure the following are provided (typically via CI secrets): | |
| # - VORTEX_TASK_SSH_FILE | |
| # - VORTEX_TASK_LAGOON_INSTANCE | |
| # - VORTEX_TASK_LAGOON_PROJECT | |
| VORTEX_DEPLOY_TYPES=lagoon | |
| #;> DEPLOYMENT |
🤖 Prompt for AI Agents
In .env around lines 219-223, switching VORTEX_DEPLOY_TYPES to "lagoon" needs
verification and documentation: ensure any parser/consumer of
VORTEX_DEPLOY_TYPES accepts the literal "lagoon" (and decide/support
comma-separated multiple providers), update CI/job gate logic to read and
respect this value for project/* and schedule events, ensure lagoon-specific env
vars (VORTEX_TASK_SSH_FILE, VORTEX_TASK_LAGOON_INSTANCE,
VORTEX_TASK_LAGOON_PROJECT) are injected where CI expects them or document their
source, and add a concise inline comment here listing accepted deploy values and
the required Lagoon variables so future readers can grep and understand the
dependency.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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(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). (12)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-workflow (1)
🔇 Additional comments (2)
.circleci/config.yml (2)
423-430: Pre-deploy guard is correct and will halt the Deploy job early for PRs from project/Using circleci-agent step halt inside the guard ends the job successfully, preventing the subsequent Deploy step from running. This achieves the intended skip for PRs originating from project/* branches. (circleci.com, support.circleci.com)
423-430: Future-proof for GitHub App integration: CIRCLE_PULL_REQUEST may be absent; consider config-time gating with pipeline valuesOn GitHub App integrations, CIRCLE_PULL_REQUEST is often not set, which could let a PR deploy slip through this shell check. Prefer a config-time condition referencing pipeline values to prevent the Deploy step from being scheduled/executed at all when the event is a pull_request from project/*, making the guard resilient across integrations. (support.circleci.com, circleci.com)
Illustrative replacement for the Deploy step (YAML snippet; outside the shown range):
# Replace: # - run: # name: Deploy # command: | # VORTEX_DEPLOY_BRANCH="${CIRCLE_BRANCH}" \ # VORTEX_DEPLOY_PR="$(echo ${CIRCLE_PULL_REQUEST} | cut -d'/' -f 7)" \ # VORTEX_DEPLOY_PR_HEAD=${CIRCLE_SHA1} \ # ./scripts/vortex/deploy.sh # With config-time gating using `unless`: - unless: condition: and: - equal: [ pull_request, << pipeline.event.name >> ] - matches: pattern: "^project/.*$" value: << pipeline.event.github.pull_request.head.ref >> steps: - run: name: Deploy command: | VORTEX_DEPLOY_BRANCH="${CIRCLE_BRANCH}" \ VORTEX_DEPLOY_PR="$(echo ${CIRCLE_PULL_REQUEST} | cut -d'/' -f 7)" \ VORTEX_DEPLOY_PR_HEAD=${CIRCLE_SHA1} \ ./scripts/vortex/deploy.shNotes:
- Logic statements run at config compile-time, which is ideal for gating steps/jobs, and can reference pipeline.event.* values. (circleci.com, support.circleci.com)
- If you keep the shell guard, prefer
[ -n "${CIRCLE_PULL_REQUEST}" ]over!= ""for clarity. This is a nit; current form is valid.Would you confirm whether this project uses CircleCI’s GitHub App integration? If yes, I can submit a follow-up patch to convert this to config-time gating using pipeline values.
| - run: | ||
| name: Check if deployment should be skipped | ||
| command: | | ||
| if [ "${CIRCLE_PULL_REQUEST}" != "" ] && echo "${CIRCLE_BRANCH}" | grep -q "^project/"; then | ||
| echo "Skipping deployment - PR from project/* branch" | ||
| circleci-agent step halt | ||
| fi | ||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Move the guard before secrets and heavy setup to minimize exposure and save time
Today, the job adds SSH keys and loads .env before the guard runs. If the guard is going to halt, injects and processing are unnecessary and slightly expand the window where secrets are available. Recommend placing this guard as the first step after checkout (or even before add_ssh_keys) to fail-fast and reduce secret exposure in PR contexts.
Example reordering (YAML snippet; outside the shown range):
steps:
- attach_workspace:
at: /tmp/workspace
- checkout
- run:
name: Check if deployment should be skipped
command: |
if [ -n "${CIRCLE_PULL_REQUEST}" ] && echo "${CIRCLE_BRANCH}" | grep -q "^project/"; then
echo "Skipping deployment - PR from project/* branch"
circleci-agent step halt
fi
- add_ssh_keys:
fingerprints:
- *deploy_ssh_fingerprint
- *step_process_codebase_for_ci
- *load_variables_from_dotenv
# ... remaining steps🤖 Prompt for AI Agents
In .circleci/config.yml around lines 423 to 430, the "Check if deployment should
be skipped" guard runs after SSH key injection and dotenv loading which
unnecessarily exposes secrets and wastes time; move this run step to immediately
after checkout (or before add_ssh_keys and load_variables_from_dotenv) so the
guard executes first and halts the job early for PRs from project/* branches,
then proceed with add_ssh_keys, the heavy CI processing step, and dotenv loading
only if the guard passes.
related to #1423
Summary by CodeRabbit
Documentation
Chores