Conversation
📝 WalkthroughWalkthroughAdds manual Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GH_Actions as GitHub Actions
participant GH_CLI as gh
participant Acquia
participant Runner
User->>GH_Actions: workflow_dispatch(deploy_target?, override_db?)
GH_Actions->>GH_Actions: resolve deploy target (if provided)
alt deploy_target matches PR-*
GH_Actions->>GH_CLI: fetch PR number, head SHA/branch
GH_CLI-->>GH_Actions: PR metadata
end
GH_Actions->>Runner: start jobs (lint/database/build) if deploy_target empty
alt database job runs
Runner->>Acquia: attempt DB download using VORTEX_ACQUIA_KEY/SECRET (optional)
Acquia-->>Runner: DB artifact
end
Runner->>Runner: build provisioning (may copy .data/${VORTEX_DOWNLOAD_DB2_FILE:-db2.sql})
alt deploy job
GH_Actions->>Runner: set VORTEX_DEPLOY_ACTION (deploy_override_db if override_db)
Runner->>Runner: checkout DEPLOY_BRANCH, run deploy script
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
This comment has been minimized.
This comment has been minimized.
![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)
565-577:⚠️ Potential issue | 🟠 MajorPin PR deploys to the resolved head SHA.
Resolve deploy targetrecordsDEPLOY_PR_HEAD_SHA, but checkout uses the mutable branch name. A push between those steps can deploy a different revision than the workflow metadata reports.🐛 Suggested fix
- ref: ${{ env.DEPLOY_BRANCH || github.head_ref || github.ref_name }} + ref: ${{ env.DEPLOY_PR_HEAD_SHA || env.DEPLOY_BRANCH || github.head_ref || github.ref_name }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-test-deploy.yml around lines 565 - 577, The workflow records DEPLOY_PR_HEAD_SHA but still checks out by branch name, so a concurrent push can change the branch between resolution and checkout; update the Checkout step (actions/checkout usage) to prefer the pinned commit by using env DEPLOY_PR_HEAD_SHA as the ref when present (falling back to DEPLOY_BRANCH || github.head_ref || github.ref_name) so the job always checks out the exact resolved head SHA.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-test-deploy.yml:
- Around line 588-595: The workflow allows a manual deploy (inputs.deploy_target
set) while VORTEX_DEPLOY_TYPES contains "artifact", which leads to
VORTEX_DEPLOY_ARTIFACT_SRC pointing to a non-existent /tmp/workspace/code; add
an explicit guard that rejects this unsupported configuration by adding a
workflow step before the Deploy step (or adding an if on the Deploy step) that
checks both inputs.deploy_target and whether env.VORTEX_DEPLOY_TYPES contains
"artifact" and fails early with a clear error (exit non-zero) if both are true;
reference the variables VORTEX_DEPLOY_TYPES, inputs.deploy_target and
VORTEX_DEPLOY_ARTIFACT_SRC and ensure the message explains that manual deploys
with artifact type are unsupported.
- Line 78: The deploy job is being skipped when lint/build are skipped because
its needs: [build, lint] creates an implicit success() gate; update the deploy
job (deploy) to use always() in its if expression so it will run even when
dependencies were skipped, and then combine always() with the existing condition
that checks inputs.deploy_target and the push/branch logic (i.e. change the
deploy job's if to start with always() && (your current condition OR
inputs.deploy_target) ), leaving needs: [build, lint] in place so
artifacts/outputs remain available when present but not blocking manual deploys.
---
Outside diff comments:
In @.github/workflows/build-test-deploy.yml:
- Around line 565-577: The workflow records DEPLOY_PR_HEAD_SHA but still checks
out by branch name, so a concurrent push can change the branch between
resolution and checkout; update the Checkout step (actions/checkout usage) to
prefer the pinned commit by using env DEPLOY_PR_HEAD_SHA as the ref when present
(falling back to DEPLOY_BRANCH || github.head_ref || github.ref_name) so the job
always checks out the exact resolved head SHA.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 416e6b8c-3872-4d62-9a6f-4516f4b4676a
📒 Files selected for processing (1)
.github/workflows/build-test-deploy.yml
| runs-on: ubuntu-latest | ||
| if: ${{ github.event_name != 'schedule' && (github.event_name == 'push' || !startsWith(github.head_ref, 'project/')) }} | ||
| #;< !PROVISION_TYPE_PROFILE | ||
| if: ${{ !inputs.deploy_target && github.event_name != 'schedule' && (github.event_name == 'push' || !startsWith(github.head_ref, 'project/')) }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/build-test-deploy.yml | sed -n '70,90p'Repository: drevops/website
Length of output: 1020
🏁 Script executed:
cat -n .github/workflows/build-test-deploy.yml | sed -n '300,315p'Repository: drevops/website
Length of output: 879
🏁 Script executed:
cat -n .github/workflows/build-test-deploy.yml | sed -n '530,550p'Repository: drevops/website
Length of output: 1088
🌐 Web query:
GitHub Actions job needs dependency skipped if condition semantics
💡 Result:
In GitHub Actions, a job that declares needs: is implicitly gated by if: success(). So if any job in its dependency chain is skipped (or fails/cancels), downstream jobs are skipped by default (“a failure or skip applies to all jobs in the dependency chain…”). [1]
To override that default and decide yourself, put always() in the dependent job’s if: (this removes the implicit success() gate), then check dependency results via needs.<job_id>.result (values: success, failure, cancelled, skipped). [1][2]
Example: run deploy even if build was skipped, but not if it failed/cancelled:
jobs:
build:
if: ${{ github.ref == 'refs/heads/main' }} # may be skipped
deploy:
needs: build
if: ${{ always() && (needs.build.result == 'success' || needs.build.result == 'skipped') }}
runs-on: ubuntu-latest
steps:
- run: echo "deploying..."If you only write if: needs.build.result == 'skipped' without always(), deploy will still be skipped, because the implicit success() gate prevents the job from running/evaluating as you expect when a dependency is skipped. [1][3]
Sources: GitHub Docs on dependency-chain skip behavior and using always() [1], and on needs.<job_id>.result values [2], plus GitHub Community discussion of success() being false when dependencies are skipped [3].
[1] (docs.github.com)
[2] (docs.github.com)
[3] (github.com)
Citations:
- 1: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions?text=For+more+information%2C+see+%22GitHub%27s+products.%22&utm_source=openai
- 2: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
- 3: https://github.com/orgs/community/discussions/45058?utm_source=openai
🏁 Script executed:
cat -n .github/workflows/build-test-deploy.yml | sed -n '546,565p'Repository: drevops/website
Length of output: 1081
Manual deploy_target runs are blocked by the implicit success gate on job dependencies.
When inputs.deploy_target is set, the lint and build jobs are skipped by their if conditions (lines 78 and 313). However, the deploy job declares needs: [build, lint] without always(), which adds an implicit if: success() gate to the entire dependency chain. GitHub Actions will skip the deploy job when its dependencies are skipped, regardless of the explicit if condition on line 551 evaluating to true.
Use always() to remove the implicit success gate and explicitly check dependency results only for the normal (non-manual) deployment path:
Suggested fix
deploy:
runs-on: ubuntu-latest
needs: [build, lint]
- if: ${{ !cancelled() && (inputs.deploy_target || (github.event_name != 'schedule' && !startsWith(github.head_ref || github.ref_name, 'deps/') && (github.event_name == 'push' || !startsWith(github.head_ref, 'project/')))) }}
+ if: ${{ always() && (inputs.deploy_target || (needs.build.result == 'success' && needs.lint.result == 'success' && github.event_name != 'schedule' && !startsWith(github.head_ref || github.ref_name, 'deps/') && (github.event_name == 'push' || !startsWith(github.head_ref, 'project/')))) }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-test-deploy.yml at line 78, The deploy job is being
skipped when lint/build are skipped because its needs: [build, lint] creates an
implicit success() gate; update the deploy job (deploy) to use always() in its
if expression so it will run even when dependencies were skipped, and then
combine always() with the existing condition that checks inputs.deploy_target
and the push/branch logic (i.e. change the deploy job's if to start with
always() && (your current condition OR inputs.deploy_target) ), leaving needs:
[build, lint] in place so artifacts/outputs remain available when present but
not blocking manual deploys.
| if: ${{ !inputs.deploy_target && contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }} | ||
| with: | ||
| name: code-artifact | ||
| path: "/tmp/artifacts" | ||
|
|
||
| - name: Unpack downloaded exported codebase | ||
| if: ${{ contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }} | ||
| if: ${{ !inputs.deploy_target && contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }} | ||
| run: | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's get the size and read the workflow file
wc -l .github/workflows/build-test-deploy.ymlRepository: drevops/website
Length of output: 102
🏁 Script executed:
# Read the relevant sections around lines 588-595 and 615
sed -n '580,630p' .github/workflows/build-test-deploy.ymlRepository: drevops/website
Length of output: 2690
🏁 Script executed:
# Search for VORTEX_DEPLOY_ARTIFACT_SRC and VORTEX_DEPLOY_TYPES to understand the flow
rg "VORTEX_DEPLOY_ARTIFACT_SRC|VORTEX_DEPLOY_TYPES" .github/workflows/build-test-deploy.yml -nRepository: drevops/website
Length of output: 614
🏁 Script executed:
# Also search for where deploy_target is used and how it affects artifact handling
rg "inputs.deploy_target" .github/workflows/build-test-deploy.yml -B 2 -A 2 -nRepository: drevops/website
Length of output: 1845
🏁 Script executed:
sed -n '610,620p' .github/workflows/build-test-deploy.ymlRepository: drevops/website
Length of output: 524
🏁 Script executed:
# Let's also check if there's any other artifact download/unpack step or similar condition
rg "Download|Unpack|artifact" .github/workflows/build-test-deploy.yml -nRepository: drevops/website
Length of output: 1594
🏁 Script executed:
sed -n '615,615p' .github/workflows/build-test-deploy.ymlRepository: drevops/website
Length of output: 93
🏁 Script executed:
# Also check the context around it
sed -n '620,635p' .github/workflows/build-test-deploy.ymlRepository: drevops/website
Length of output: 1246
Reject manual artifact deployments to prevent failures.
The download and unpack steps skip when inputs.deploy_target is set, but the Deploy step always sets VORTEX_DEPLOY_ARTIFACT_SRC to /tmp/workspace/code. When artifact is in VORTEX_DEPLOY_TYPES, this causes the deploy to fail because the artifact directory doesn't exist. Since the build job is skipped for manual deploys, the artifact is never created.
Add a guard to explicitly reject this unsupported configuration:
Suggested guard
- name: Load environment variables from .env
run: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$GITHUB_ENV"
+
+ - name: Reject manual artifact deploys until artifact preparation exists
+ if: ${{ inputs.deploy_target && contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }}
+ run: |
+ echo "Manual deploys are not supported for artifact deployments because the build job is skipped."
+ exit 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-test-deploy.yml around lines 588 - 595, The workflow
allows a manual deploy (inputs.deploy_target set) while VORTEX_DEPLOY_TYPES
contains "artifact", which leads to VORTEX_DEPLOY_ARTIFACT_SRC pointing to a
non-existent /tmp/workspace/code; add an explicit guard that rejects this
unsupported configuration by adding a workflow step before the Deploy step (or
adding an if on the Deploy step) that checks both inputs.deploy_target and
whether env.VORTEX_DEPLOY_TYPES contains "artifact" and fails early with a clear
error (exit non-zero) if both are true; reference the variables
VORTEX_DEPLOY_TYPES, inputs.deploy_target and VORTEX_DEPLOY_ARTIFACT_SRC and
ensure the message explains that manual deploys with artifact type are
unsupported.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/build-test-deploy.yml (2)
593-605:⚠️ Potential issue | 🟠 MajorAdd guard to reject manual artifact deployments.
The comment on line 596 acknowledges artifact deployments don't work for manual deploys, but there's no enforcement. If
VORTEX_DEPLOY_TYPEScontainsartifact, the deploy script will attempt to use/tmp/workspace/codewhich won't exist, causing a cryptic failure.Add an explicit guard to fail early with a clear message:
Suggested fix
- name: Load environment variables from .env run: t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && set +a && . "${t}" && env >> "$GITHUB_ENV" + - name: Reject manual artifact deploys + if: ${{ inputs.deploy_target && contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }} + run: | + echo "::error::Manual deploys are not supported when VORTEX_DEPLOY_TYPES contains 'artifact' (build job is skipped, no artifact available)." + exit 1 + # Artifact deployments do not work for manual deploys as the build job is skipped. - name: Download exported codebase as an artifact,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-test-deploy.yml around lines 593 - 605, Add an explicit failing step that triggers when a manual deploy is requested but VORTEX_DEPLOY_TYPES contains "artifact": create a job step (e.g., name "Reject manual artifact deployments") with an if: condition like ${{ inputs.deploy_target && contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }} and have it print a clear message and exit non‑zero (e.g., echo "Manual deploys cannot use artifact deploy type; set VORTEX_DEPLOY_TYPES appropriately or remove inputs.deploy_target" && exit 1). Place this before the "Download exported codebase as an artifact" / "Unpack downloaded exported codebase" steps so the workflow fails fast when inputs.deploy_target and env.VORTEX_DEPLOY_TYPES include "artifact".
549-552:⚠️ Potential issue | 🔴 CriticalManual
deploy_targetruns will still be blocked by implicit success gate.The condition uses
!cancelled()but this does not override the implicitsuccess()gate on job dependencies. Wheninputs.deploy_targetis set,lintandbuildare skipped (lines 78, 313), and GitHub Actions will skipdeploybecause itsneeds: [build, lint]dependencies were skipped—regardless of theifcondition evaluating to true.Replace
!cancelled()withalways()and explicitly check dependency results for the non-manual path:Suggested fix
deploy: runs-on: ubuntu-latest needs: [build, lint] #;< !PROVISION_TYPE_PROFILE - if: ${{ !cancelled() && (inputs.deploy_target || (github.event_name != 'schedule' && !startsWith(github.head_ref || github.ref_name, 'deps/') && (github.event_name == 'push' || !startsWith(github.head_ref, 'project/')))) }} + if: ${{ always() && (inputs.deploy_target || (needs.build.result == 'success' && needs.lint.result == 'success' && github.event_name != 'schedule' && !startsWith(github.head_ref || github.ref_name, 'deps/') && (github.event_name == 'push' || !startsWith(github.head_ref, 'project/')))) }} #;> !PROVISION_TYPE_PROFILE,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-test-deploy.yml around lines 549 - 552, The deploy job's if condition needs to allow manual runs to bypass skipped dependencies: replace the leading !cancelled() with always() and restructure the boolean so manual runs short-circuit while the normal path explicitly checks dependency results; e.g. change the condition on the deploy job (the block containing needs: [build, lint] and the long if expression) to use always() && (inputs.deploy_target || (needs.build.result == 'success' && needs.lint.result == 'success' && github.event_name != 'schedule' && !startsWith(github.head_ref || github.ref_name, 'deps/') && (github.event_name == 'push' || !startsWith(github.head_ref, 'project/')))).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-test-deploy.yml:
- Around line 567-578: Validate DEPLOY_TARGET when it matches '^pr-' by
extracting the suffix and ensuring it's a numeric PR ID before calling gh; for
DEPLOY_PR_NUMBER extraction (DEPLOY_TARGET#*), check the regex and if not
numeric emit a clear error and exit non‑zero. For each gh pr view invocation
that sets DEPLOY_PR_HEAD_SHA and DEPLOY_BRANCH, capture the command exit status
and on failure log a descriptive message including DEPLOY_PR_NUMBER and fail the
step (or set safe defaults) instead of allowing silent failure; ensure all uses
of gh pr view are guarded so env vars are only written when the gh command
succeeds.
---
Duplicate comments:
In @.github/workflows/build-test-deploy.yml:
- Around line 593-605: Add an explicit failing step that triggers when a manual
deploy is requested but VORTEX_DEPLOY_TYPES contains "artifact": create a job
step (e.g., name "Reject manual artifact deployments") with an if: condition
like ${{ inputs.deploy_target && contains(env.VORTEX_DEPLOY_TYPES, 'artifact')
}} and have it print a clear message and exit non‑zero (e.g., echo "Manual
deploys cannot use artifact deploy type; set VORTEX_DEPLOY_TYPES appropriately
or remove inputs.deploy_target" && exit 1). Place this before the "Download
exported codebase as an artifact" / "Unpack downloaded exported codebase" steps
so the workflow fails fast when inputs.deploy_target and env.VORTEX_DEPLOY_TYPES
include "artifact".
- Around line 549-552: The deploy job's if condition needs to allow manual runs
to bypass skipped dependencies: replace the leading !cancelled() with always()
and restructure the boolean so manual runs short-circuit while the normal path
explicitly checks dependency results; e.g. change the condition on the deploy
job (the block containing needs: [build, lint] and the long if expression) to
use always() && (inputs.deploy_target || (needs.build.result == 'success' &&
needs.lint.result == 'success' && github.event_name != 'schedule' &&
!startsWith(github.head_ref || github.ref_name, 'deps/') && (github.event_name
== 'push' || !startsWith(github.head_ref, 'project/')))).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e4524884-d301-4690-b0d2-68ef60357b17
📒 Files selected for processing (1)
.github/workflows/build-test-deploy.yml
| - name: Resolve deploy target | ||
| if: ${{ inputs.deploy_target }} | ||
| env: | ||
| DEPLOY_TARGET: ${{ inputs.deploy_target }} | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| echo "DEPLOY_BRANCH=${DEPLOY_TARGET}" >> "$GITHUB_ENV" | ||
| if echo "${DEPLOY_TARGET}" | grep -iq '^pr-'; then | ||
| echo "DEPLOY_PR_NUMBER=${DEPLOY_TARGET#*-}" >> "$GITHUB_ENV" | ||
| echo "DEPLOY_PR_HEAD_SHA=$(gh pr view "${DEPLOY_TARGET#*-}" --repo "${{ github.repository }}" --json headRefOid --jq '.headRefOid')" >> "$GITHUB_ENV" | ||
| echo "DEPLOY_BRANCH=$(gh pr view "${DEPLOY_TARGET#*-}" --repo "${{ github.repository }}" --json headRefName --jq '.headRefName')" >> "$GITHUB_ENV" | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add error handling for PR resolution.
If the PR number is invalid or the PR doesn't exist, gh pr view will fail silently (exit code captured but env vars won't be set correctly). Additionally, if someone passes pr-abc instead of pr-123, the behavior is undefined.
Consider adding validation and error handling:
Suggested improvement
- name: Resolve deploy target
if: ${{ inputs.deploy_target }}
env:
DEPLOY_TARGET: ${{ inputs.deploy_target }}
GH_TOKEN: ${{ github.token }}
run: |
echo "DEPLOY_BRANCH=${DEPLOY_TARGET}" >> "$GITHUB_ENV"
if echo "${DEPLOY_TARGET}" | grep -iq '^pr-'; then
- echo "DEPLOY_PR_NUMBER=${DEPLOY_TARGET#*-}" >> "$GITHUB_ENV"
- echo "DEPLOY_PR_HEAD_SHA=$(gh pr view "${DEPLOY_TARGET#*-}" --repo "${{ github.repository }}" --json headRefOid --jq '.headRefOid')" >> "$GITHUB_ENV"
- echo "DEPLOY_BRANCH=$(gh pr view "${DEPLOY_TARGET#*-}" --repo "${{ github.repository }}" --json headRefName --jq '.headRefName')" >> "$GITHUB_ENV"
+ PR_NUM="${DEPLOY_TARGET#*-}"
+ if ! [[ "${PR_NUM}" =~ ^[0-9]+$ ]]; then
+ echo "::error::Invalid PR number format: ${PR_NUM}. Expected 'pr-<number>'."
+ exit 1
+ fi
+ if ! gh pr view "${PR_NUM}" --repo "${{ github.repository }}" --json state --jq '.state' > /dev/null 2>&1; then
+ echo "::error::PR #${PR_NUM} not found or inaccessible."
+ exit 1
+ fi
+ echo "DEPLOY_PR_NUMBER=${PR_NUM}" >> "$GITHUB_ENV"
+ echo "DEPLOY_PR_HEAD_SHA=$(gh pr view "${PR_NUM}" --repo "${{ github.repository }}" --json headRefOid --jq '.headRefOid')" >> "$GITHUB_ENV"
+ echo "DEPLOY_BRANCH=$(gh pr view "${PR_NUM}" --repo "${{ github.repository }}" --json headRefName --jq '.headRefName')" >> "$GITHUB_ENV"
fi📝 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: Resolve deploy target | |
| if: ${{ inputs.deploy_target }} | |
| env: | |
| DEPLOY_TARGET: ${{ inputs.deploy_target }} | |
| GH_TOKEN: ${{ github.token }} | |
| run: | | |
| echo "DEPLOY_BRANCH=${DEPLOY_TARGET}" >> "$GITHUB_ENV" | |
| if echo "${DEPLOY_TARGET}" | grep -iq '^pr-'; then | |
| echo "DEPLOY_PR_NUMBER=${DEPLOY_TARGET#*-}" >> "$GITHUB_ENV" | |
| echo "DEPLOY_PR_HEAD_SHA=$(gh pr view "${DEPLOY_TARGET#*-}" --repo "${{ github.repository }}" --json headRefOid --jq '.headRefOid')" >> "$GITHUB_ENV" | |
| echo "DEPLOY_BRANCH=$(gh pr view "${DEPLOY_TARGET#*-}" --repo "${{ github.repository }}" --json headRefName --jq '.headRefName')" >> "$GITHUB_ENV" | |
| fi | |
| - name: Resolve deploy target | |
| if: ${{ inputs.deploy_target }} | |
| env: | |
| DEPLOY_TARGET: ${{ inputs.deploy_target }} | |
| GH_TOKEN: ${{ github.token }} | |
| run: | | |
| echo "DEPLOY_BRANCH=${DEPLOY_TARGET}" >> "$GITHUB_ENV" | |
| if echo "${DEPLOY_TARGET}" | grep -iq '^pr-'; then | |
| PR_NUM="${DEPLOY_TARGET#*-}" | |
| if ! [[ "${PR_NUM}" =~ ^[0-9]+$ ]]; then | |
| echo "::error::Invalid PR number format: ${PR_NUM}. Expected 'pr-<number>'." | |
| exit 1 | |
| fi | |
| if ! gh pr view "${PR_NUM}" --repo "${{ github.repository }}" --json state --jq '.state' > /dev/null 2>&1; then | |
| echo "::error::PR #${PR_NUM} not found or inaccessible." | |
| exit 1 | |
| fi | |
| echo "DEPLOY_PR_NUMBER=${PR_NUM}" >> "$GITHUB_ENV" | |
| echo "DEPLOY_PR_HEAD_SHA=$(gh pr view "${PR_NUM}" --repo "${{ github.repository }}" --json headRefOid --jq '.headRefOid')" >> "$GITHUB_ENV" | |
| echo "DEPLOY_BRANCH=$(gh pr view "${PR_NUM}" --repo "${{ github.repository }}" --json headRefName --jq '.headRefName')" >> "$GITHUB_ENV" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-test-deploy.yml around lines 567 - 578, Validate
DEPLOY_TARGET when it matches '^pr-' by extracting the suffix and ensuring it's
a numeric PR ID before calling gh; for DEPLOY_PR_NUMBER extraction
(DEPLOY_TARGET#*), check the regex and if not numeric emit a clear error and
exit non‑zero. For each gh pr view invocation that sets DEPLOY_PR_HEAD_SHA and
DEPLOY_BRANCH, capture the command exit status and on failure log a descriptive
message including DEPLOY_PR_NUMBER and fail the step (or set safe defaults)
instead of allowing silent failure; ensure all uses of gh pr view are guarded so
env vars are only written when the gh command succeeds.
This comment has been minimized.
This comment has been minimized.
4 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Code coverage (threshold: 80%) Per-class coverage |
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)
.github/workflows/build-test-deploy.yml (1)
575-588:⚠️ Potential issue | 🟠 MajorCheck out the resolved PR SHA, not the branch reference.
You resolve
DEPLOY_PR_HEAD_SHAat line 576, butactions/checkoutusesDEPLOY_BRANCHat line 588. If the PR branch receives a new push between these steps, checkout will pull a different commit than the one you resolved, causing the deployed code to diverge fromVORTEX_DEPLOY_PR_HEADthat's passed todeploy.sh.Suggested fix
- name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 with: ref: ${{ env.DEPLOY_PR_HEAD_SHA || env.DEPLOY_BRANCH || github.head_ref || github.ref_name }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-test-deploy.yml around lines 575 - 588, The checkout step currently uses DEPLOY_BRANCH which can drift; change it to explicitly checkout the resolved PR commit by using DEPLOY_PR_HEAD_SHA (fallback to github.head_ref or github.ref_name) as the ref for actions/checkout so the workspace matches the exact commit stored in DEPLOY_PR_HEAD_SHA; update the checkout `ref` to prefer env.DEPLOY_PR_HEAD_SHA (or equivalent variable) instead of DEPLOY_BRANCH to guarantee the deployed code matches VORTEX_DEPLOY_PR_HEAD passed to deploy.sh.
♻️ Duplicate comments (1)
.github/workflows/build-test-deploy.yml (1)
549-551:⚠️ Potential issue | 🔴 CriticalManual
deploy_targetruns are still blocked byneeds.This is the same blocker noted earlier: once
inputs.deploy_targetskipsbuildandlint, the implicitsuccess()gate fromneeds: [build, lint]keepsdeployskipped too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-test-deploy.yml around lines 549 - 551, The deploy job is still blocked by needs: [build, lint] even for manual runs via inputs.deploy_target; split into two jobs: keep the existing deploy job (retain needs: [build, lint]) but change its if to require inputs.deploy_target to be false (e.g. if: ${{ !inputs.deploy_target && <existing-expression> }}), and add a new deploy_manual (or deploy_dispatch) job with no needs and if: ${{ inputs.deploy_target }} that performs the same deployment steps for manual runs; reference inputs.deploy_target, the original deploy job name, and needs: [build, lint] when implementing this split so manual triggers bypass the needs gate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-test-deploy.yml:
- Around line 283-285: The migration DB download step that runs
"VORTEX_DB_INDEX=2 ./scripts/vortex/download-db.sh" should mirror the primary
download step by adding the same step-scoped Acquia credential environment
variables and a 30-minute timeout; update the "Download migration DB" step to
include the identical env entries used in the primary "Download DB" step (the
Acquia credentials) and add timeout: 30m so the invocation of
./scripts/vortex/download-db.sh with VORTEX_DB_INDEX=2 has the same credentials
and timeout behavior.
---
Outside diff comments:
In @.github/workflows/build-test-deploy.yml:
- Around line 575-588: The checkout step currently uses DEPLOY_BRANCH which can
drift; change it to explicitly checkout the resolved PR commit by using
DEPLOY_PR_HEAD_SHA (fallback to github.head_ref or github.ref_name) as the ref
for actions/checkout so the workspace matches the exact commit stored in
DEPLOY_PR_HEAD_SHA; update the checkout `ref` to prefer env.DEPLOY_PR_HEAD_SHA
(or equivalent variable) instead of DEPLOY_BRANCH to guarantee the deployed code
matches VORTEX_DEPLOY_PR_HEAD passed to deploy.sh.
---
Duplicate comments:
In @.github/workflows/build-test-deploy.yml:
- Around line 549-551: The deploy job is still blocked by needs: [build, lint]
even for manual runs via inputs.deploy_target; split into two jobs: keep the
existing deploy job (retain needs: [build, lint]) but change its if to require
inputs.deploy_target to be false (e.g. if: ${{ !inputs.deploy_target &&
<existing-expression> }}), and add a new deploy_manual (or deploy_dispatch) job
with no needs and if: ${{ inputs.deploy_target }} that performs the same
deployment steps for manual runs; reference inputs.deploy_target, the original
deploy job name, and needs: [build, lint] when implementing this split so manual
triggers bypass the needs gate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5067211d-c443-4b57-9cda-0ac83b113511
📒 Files selected for processing (1)
.github/workflows/build-test-deploy.yml
| - name: Download migration DB | ||
| run: VORTEX_DB_INDEX=2 ./scripts/vortex/download-db.sh | ||
| #;> MIGRATION |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && git ls-files .github/workflows/build-test-deploy.ymlRepository: drevops/website
Length of output: 129
🏁 Script executed:
sed -n '270,290p' .github/workflows/build-test-deploy.yml | cat -nRepository: drevops/website
Length of output: 1042
🏁 Script executed:
find . -type f -name "download-db.sh" | head -5Repository: drevops/website
Length of output: 90
🏁 Script executed:
cat ./scripts/vortex/download-db.sh | head -100Repository: drevops/website
Length of output: 3897
🏁 Script executed:
grep -r "VORTEX_DOWNLOAD_DB.*_SOURCE" .github/workflows/ .env* 2>/dev/null | head -20Repository: drevops/website
Length of output: 96
🏁 Script executed:
grep -r "VORTEX_DOWNLOAD_DB2_SOURCE\|VORTEX_DOWNLOAD_DB_SOURCE" . --include="*.md" --include="*.env*" --include="*.yml" 2>/dev/null | head -20Repository: drevops/website
Length of output: 98
🏁 Script executed:
cat ./scripts/vortex/download-db-acquia.sh | head -80Repository: drevops/website
Length of output: 3312
🏁 Script executed:
grep -r "VORTEX_DOWNLOAD_DB2_SOURCE\|VORTEX_DB2_" . --include="*.env*" --include="*.yml" 2>/dev/nullRepository: drevops/website
Length of output: 41
🏁 Script executed:
grep -r "VORTEX_DB_INDEX" .github/workflows/ 2>/dev/nullRepository: drevops/website
Length of output: 161
Mirror the primary DB download step's env and timeout.
This second download-db.sh invocation lacks the step-scoped Acquia credentials and the 30-minute timeout added above. Although the default source is lagoon, the migration step should maintain consistency with the primary download step and provide access to the same credentials in case the source is changed. Without the timeout, a hung download can consume the entire job timeout instead of failing fast.
Suggested fix
- name: Download migration DB
run: VORTEX_DB_INDEX=2 ./scripts/vortex/download-db.sh
+ timeout-minutes: 30
+ #;< DB_DOWNLOAD_SOURCE_ACQUIA
+ env:
+ VORTEX_ACQUIA_KEY: ${{ secrets.VORTEX_ACQUIA_KEY }}
+ VORTEX_ACQUIA_SECRET: ${{ secrets.VORTEX_ACQUIA_SECRET }}
+ #;> DB_DOWNLOAD_SOURCE_ACQUIA🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-test-deploy.yml around lines 283 - 285, The
migration DB download step that runs "VORTEX_DB_INDEX=2
./scripts/vortex/download-db.sh" should mirror the primary download step by
adding the same step-scoped Acquia credential environment variables and a
30-minute timeout; update the "Download migration DB" step to include the
identical env entries used in the primary "Download DB" step (the Acquia
credentials) and add timeout: 30m so the invocation of
./scripts/vortex/download-db.sh with VORTEX_DB_INDEX=2 has the same credentials
and timeout behavior.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #156 +/- ##
========================================
Coverage 87.73% 87.73%
========================================
Files 11 11
Lines 163 163
========================================
Hits 143 143
Misses 20 20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Checklist before requesting a review
[#123] Verb in past tense.#123added to descriptionChangedsectionChanged
Screenshots
Summary by CodeRabbit