Updated Vortex to 1.38.0.#169
Conversation
….stage_file_proxy.php'.
….trusted_hosts.php'.
…gs.container.php'.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPlatform/tooling pins and PHP requirement raised to 8.4, new reusable PHPUnit/Behat test traits and base classes added, many vortex scripts gain task timing and notification-branch gating, CI/workflows and action pins updated, and assorted settings/docs adjusted. ChangesPHP 8.4 & Lagoon 26.4.0 Upgrade with Test Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #169 +/- ##
========================================
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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
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)
510-533:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCheckout the resolved PR SHA instead of the mutable branch ref.
For
PR-*manual deploys, this step capturesDEPLOY_PR_HEAD_SHA, but Line 533 still checks outDEPLOY_BRANCH. A new push between these two steps can make the workspace differ from the SHA sent viaVORTEX_DEPLOY_PR_HEAD, so the job may deploy a different revision than the one it 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 current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-test-deploy.yml around lines 510 - 533, The checkout step currently uses DEPLOY_BRANCH which can be mutated after resolving DEPLOY_PR_HEAD_SHA; update the actions/checkout ref logic so that when DEPLOY_PR_HEAD_SHA is set it checks out that immutable SHA instead of DEPLOY_BRANCH — i.e., change the ref input for the checkout action (the actions/checkout@... step) to prefer env.DEPLOY_PR_HEAD_SHA (falling back to DEPLOY_BRANCH or github.head_ref/github.ref_name) so the workspace matches the resolved PR head SHA used for deployment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/vortex/doctor.sh`:
- Around line 69-71: The doctor script was altered to add custom functions
note(), task(), and pass() directly into a Vortex-managed script; remove these
definitions from the Vortex-managed file and instead place them into a separate
custom script in the repo's custom-scripts area, keeping the exact function
names (note, task, pass) and behavior, then wire that custom script by sourcing
it from the invocation path (so the Vortex-managed script stays unmodified but
still gets the functions when run).
In `@scripts/vortex/download-db-ftp.sh`:
- Around line 55-56: The one-line task() and pass() functions use chained
commands (&&, ||) and command substitution, so expand them into multi-line
functions: in task() call date once to set _TASK_START, then use an if ... then
... fi to test TERM and tput and choose the printf form; in pass() compute
duration using builtin arithmetic ($(( ... ))) after calling date, unset
_TASK_START in its own statement, and replace all chained &&/|| with explicit
if/then/else blocks; update the functions named task and pass accordingly and
keep each external command on its own simple command line (no chaining, piping,
or inline command substitution).
In `@scripts/vortex/download-db-s3.sh`:
- Around line 57-58: The one-line implementations of task() and pass() use
chaining (&&, ||) and command substitution which violates the Bash rule;
refactor both functions into multi-line bodies that use exactly one simple
command per tool call: assign _TASK_START with a plain assignment, check TERM
and run tput as a single command inside an if block, compute duration using a
separate arithmetic assignment, and call printf once per branch without
chaining; update task() to perform the color-capable check and print the blue
"[TASK]" line in an if/else, and update pass() to compute _d in a separate
statement, unset _TASK_START separately, then print the green "[ OK ]" line with
a single printf call per branch, referencing the existing function names task
and pass.
In `@scripts/vortex/download-db.sh`:
- Around line 49-51: The changes modify managed Vortex helper functions (note(),
task(), pass()) inside scripts/vortex/download-db.sh; instead of editing these
files, revert these edits and move any project-specific wrapper or overrides
into scripts/custom/, for example create a script in scripts/custom/ that
defines your project-specific note/task/pass behavior or exports wrapper
functions and then source or invoke that custom script from higher-level
entrypoint logic; ensure scripts/vortex/download-db.sh remains unchanged and
that any callers load scripts/custom/ when project-specific behavior is
required.
In `@scripts/vortex/export-db-image.sh`:
- Around line 33-35: The custom logging functions note(), task(), and pass()
were added inside the Vortex-managed script and should be moved to a custom
script to avoid being overwritten; extract these functions from
scripts/vortex/export-db-image.sh and place them into a new or existing file
under scripts/custom/ (e.g., scripts/custom/logging.sh) and source that file
where needed, then remove the custom definitions from export-db-image.sh so the
Vortex file stays unmodified; ensure the calling code still sources or
references the new file so note(), task(), and pass() remain available at
runtime.
- Around line 34-35: The one-line task() and pass() helpers use chained &&/||
operators; rewrite both as multi-line functions that avoid any chaining by using
explicit if/then/fi blocks and separate statements: in task(), set _TASK_START
with date, then use an if to test TERM and tput and call printf in each branch;
in pass(), compute _d in a separate conditional when _TASK_START is set, unset
_TASK_START in its own statement, then use an if to test TERM/tput and call
printf in each branch to emit the colored or plain "[ OK ]" message; keep the
function names task and pass and the _TASK_START/_d variables unchanged so the
rest of the script continues to work.
In `@scripts/vortex/notify-email.sh`:
- Around line 68-73: The branch-allowlist check in notify-email.sh relies on
VORTEX_NOTIFY_BRANCH but that file can't be changed; ensure the caller
(notify.sh) guarantees VORTEX_NOTIFY_BRANCH is always set/exported (e.g., set a
default like empty string or explicitly export it) before invoking
notify-email.sh, or implement the branch-filter wrapper in scripts/custom/
(e.g., scripts/custom/notify-email.sh) that performs the allowlist check instead
of editing scripts/vortex/; in either fix, make the check robust to an unset
variable and avoid piped commands (use a single simple command such as grep -F
with the constructed string or a shell case) when testing
VORTEX_NOTIFY_EMAIL_BRANCHES against VORTEX_NOTIFY_BRANCH.
In `@scripts/vortex/notify-github.sh`:
- Around line 64-67: The branch filter is comparing the wrong variable: replace
usages of VORTEX_NOTIFY_BRANCH with the intended VORTEX_NOTIFY_GITHUB_BRANCH in
the branch-check logic (specifically the grep check that builds
",${VORTEX_NOTIFY_GITHUB_BRANCHES}," | grep -qF ",${VORTEX_NOTIFY_BRANCH},") so
it checks against ",${VORTEX_NOTIFY_GITHUB_BRANCH},"; also ensure variables are
quoted/expanded safely (e.g., "${VORTEX_NOTIFY_GITHUB_BRANCH}") to avoid
failures under set -u.
In `@scripts/vortex/notify-newrelic.sh`:
- Around line 95-99: The branch filtering block dereferences
VORTEX_NOTIFY_BRANCH without guarding, which fails under set -u if that variable
is unset; update the conditional to use a safe expansion (e.g.
${VORTEX_NOTIFY_BRANCH:-}) or add a prior check for the variable (e.g. test if
VORTEX_NOTIFY_BRANCH is set/empty) before using it so the echo/grep line uses a
non-empty default and does not trigger an unbound-variable error; target the
conditional that references VORTEX_NOTIFY_NEWRELIC_BRANCHES and
VORTEX_NOTIFY_BRANCH and ensure VORTEX_NOTIFY_BRANCH is handled safely.
In `@scripts/vortex/notify-webhook.sh`:
- Around line 67-72: Replace the echo|grep inclusion test with a pure-shell
pattern match and guard against unset variables: use the parameter-expansion
defaults ${VORTEX_NOTIFY_WEBHOOK_BRANCHES-} and ${VORTEX_NOTIFY_BRANCH-} and a
case statement like case ",${VORTEX_NOTIFY_WEBHOOK_BRANCHES-}," in
*,",${VORTEX_NOTIFY_BRANCH-},") ... ;; esac to decide whether to skip; this
removes the pipe and prevents failures under set -u while keeping the same
membership semantics referenced in the existing block.
In `@scripts/vortex/provision-sanitize-db.sh`:
- Around line 32-33: The one-line helper functions task() and pass() use command
substitution and chaining operators which violate the shell-tool rule and the
scripts/vortex location rule; fix by moving any custom helpers out of
scripts/vortex/ into scripts/custom/ or rewrite task() and pass() as
multi-statement, single-command shells (no $(...), no &&/||/; no pipes or
heredocs): compute timestamps using a separate simple command, store results in
variables, and call tput/printf each as their own single simple command in the
function body (refer to the task and pass function names to locate and update
them).
In `@scripts/vortex/provision.sh`:
- Around line 89-90: Split the one-line task() and pass() into multiple simple
statements and remove command chaining and command substitution by using the
shell's SECONDS variable and explicit if blocks: in task() set
_TASK_START=$SECONDS as a plain assignment, call tput in its own if block to
test colors, then call printf as a single command to emit the colored or plain
"[TASK]" message; in pass() compute duration with arithmetic like _d="
($(($SECONDS - _TASK_START))s)" only if _TASK_START is set, unset _TASK_START on
its own line, call tput in an if block to detect colors, and finally call printf
once to emit the colored or plain "[ OK ]" message — update the existing task()
and pass() functions accordingly to reference _TASK_START, SECONDS, tput, and
printf without any &&, ||, |, ; or $(...) usage.
In `@scripts/vortex/task-custom-lagoon.sh`:
- Around line 57-59: Move the file from scripts/vortex/task-custom-lagoon.sh to
scripts/custom/task-custom-lagoon.sh and refactor the note, task, and pass
functions to remove command substitution and &&/|| chaining: replace inline
chained checks like `[ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 &&
printf ... || printf ...` with explicit if/then/else blocks that run tput once
(e.g., if tput colors >/dev/null 2>&1; then printf colored output; else printf
plain output; fi), and avoid inline command substitution/arithmetic in pass by
assigning the start and end times to variables in separate statements (e.g., run
date once to set _TASK_START, later run date again into another var, compute
duration with $((end - _TASK_START))) and unset _TASK_START in its own
statement; use the existing function names (note, task, pass) to locate and
update the implementations.
In `@scripts/vortex/upload-db-s3.sh`:
- Around line 48-49: The one-line implementations of task() and pass() use
command substitution and &&/|| chains which violate the repo shell rules;
rewrite both as multi-line shell functions that set TASK_START using the SECONDS
special variable in task(), and in pass() compute duration with arithmetic
(duration=$((SECONDS - TASK_START))) and unset TASK_START, and replace chained
color checks with explicit if tests calling tput (e.g., if tput colors
>/dev/null 2>&1; then printf with ANSI color escapes; else printf without
colors; fi) so each external tool call is a single simple command and there are
no $(...), &&, ||, or chained statements in task() or pass().
In `@web/modules/custom/do_base/tests/src/Kernel/ExampleTest.php`:
- Around line 25-29: The testAdd method contains an unused optional parameter
$expectExceptionMessage and an unused branch that expects exceptions; remove the
dead code by changing the testAdd signature to only accept the numeric inputs
and expected result (remove string|null $expectExceptionMessage), delete the if
($expectExceptionMessage) { ... } block (the expectException and
expectExceptionMessage calls), and update any data providers or calls
referencing testAdd to stop providing that extra parameter; apply the same
cleanup to the other kernel test methods showing the same pattern (the other
test methods around the same tests that include an unused
$expectExceptionMessage parameter).
In `@web/modules/custom/do_base/tests/src/Traits/ReflectionTrait.php`:
- Around line 78-83: getProtectedValue currently passes the ReflectionClass
($class) into ReflectionProperty::getValue, causing a TypeError; update
getProtectedValue to call $property->setAccessible(true) and then call
$property->getValue($property->isStatic() ? null : $object) so getValue receives
null for static properties or the original $object instance for instance
properties; keep the function name getProtectedValue and the
ReflectionClass/ReflectionProperty usage but change the getValue argument as
described.
In `@web/modules/custom/do_base/tests/src/Unit/ExampleTest.php`:
- Around line 25-29: The test methods (e.g., testAdd) include an unused optional
parameter $expectExceptionMessage and a dead branch that calls
expectException/expectExceptionMessage; remove the $expectExceptionMessage
parameter from the method signature and delete the conditional block that sets
exception expectations, then update any corresponding data providers to stop
supplying that value (or adjust datasets accordingly); do the same cleanup for
the other test methods with the same pattern (the ones referenced at the other
ranges) so exception-focused behavior is implemented only in dedicated tests.
In `@web/sites/default/includes/modules/settings.stage_file_proxy.php`:
- Line 15: The replacement currently injects raw env credentials into
$stage_file_proxy_origin which can break the URL if $stage_file_proxy_user or
$stage_file_proxy_pass contain reserved characters; instead URL-encode the
username and password (e.g., via rawurlencode) before building the credential
string and then insert that encoded credential string into
$stage_file_proxy_origin (reference the variables $stage_file_proxy_origin,
$stage_file_proxy_user, $stage_file_proxy_pass to locate the change).
---
Outside diff comments:
In @.github/workflows/build-test-deploy.yml:
- Around line 510-533: The checkout step currently uses DEPLOY_BRANCH which can
be mutated after resolving DEPLOY_PR_HEAD_SHA; update the actions/checkout ref
logic so that when DEPLOY_PR_HEAD_SHA is set it checks out that immutable SHA
instead of DEPLOY_BRANCH — i.e., change the ref input for the checkout action
(the actions/checkout@... step) to prefer env.DEPLOY_PR_HEAD_SHA (falling back
to DEPLOY_BRANCH or github.head_ref/github.ref_name) so the workspace matches
the resolved PR head SHA used for deployment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 83e7c229-5c9f-4aa3-8875-a2e558c2440a
⛔ Files ignored due to path filters (2)
composer.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (82)
.ahoy.yml.docker/clamav.dockerfile.docker/cli.dockerfile.docker/database.dockerfile.docker/nginx-drupal.dockerfile.docker/php.dockerfile.docker/solr.dockerfile.dockerignore.eslintrc.json.github/workflows/assign-author.yml.github/workflows/build-test-deploy.yml.github/workflows/draft-release-notes.yml.github/workflows/update-dependencies.yml.lagoon.ymlAGENTS.mdREADME.mdbehat.ymlcomposer.jsonphpcs.xmlphpstan.neonphpunit.xmlrector.phprenovate.jsonscripts/vortex/deploy-artifact.shscripts/vortex/deploy-container-registry.shscripts/vortex/deploy-lagoon.shscripts/vortex/deploy-webhook.shscripts/vortex/deploy.shscripts/vortex/doctor.shscripts/vortex/download-db-acquia.shscripts/vortex/download-db-container-registry.shscripts/vortex/download-db-ftp.shscripts/vortex/download-db-lagoon.shscripts/vortex/download-db-s3.shscripts/vortex/download-db-url.shscripts/vortex/download-db.shscripts/vortex/export-db-file.shscripts/vortex/export-db-image.shscripts/vortex/export-db.shscripts/vortex/info.shscripts/vortex/login-container-registry.shscripts/vortex/login.shscripts/vortex/logout.shscripts/vortex/notify-email.shscripts/vortex/notify-github.shscripts/vortex/notify-jira.shscripts/vortex/notify-newrelic.shscripts/vortex/notify-slack.shscripts/vortex/notify-webhook.shscripts/vortex/notify.shscripts/vortex/provision-sanitize-db.shscripts/vortex/provision.shscripts/vortex/reset.shscripts/vortex/setup-ssh.shscripts/vortex/task-copy-db-acquia.shscripts/vortex/task-copy-files-acquia.shscripts/vortex/task-custom-lagoon.shscripts/vortex/task-purge-cache-acquia.shscripts/vortex/update-vortex.shscripts/vortex/upload-db-s3.shtests/behat/features/homepage.featuretests/behat/features/login.featuretests/phpunit/Drupal/EnvironmentSettingsTest.phptests/phpunit/Drupal/SettingsTestCase.phpweb/modules/custom/do_base/tests/src/Functional/DoBaseFunctionalTestBase.phpweb/modules/custom/do_base/tests/src/Functional/ExampleTest.phpweb/modules/custom/do_base/tests/src/FunctionalJavascript/DoBaseFunctionalJavascriptTestBase.phpweb/modules/custom/do_base/tests/src/FunctionalJavascript/ExampleTest.phpweb/modules/custom/do_base/tests/src/Kernel/DoBaseKernelTestBase.phpweb/modules/custom/do_base/tests/src/Kernel/ExampleTest.phpweb/modules/custom/do_base/tests/src/Traits/ArrayTrait.phpweb/modules/custom/do_base/tests/src/Traits/AssertTrait.phpweb/modules/custom/do_base/tests/src/Traits/BrowserHtmlDebugTrait.phpweb/modules/custom/do_base/tests/src/Traits/MockTrait.phpweb/modules/custom/do_base/tests/src/Traits/ReflectionTrait.phpweb/modules/custom/do_base/tests/src/Unit/DoBaseUnitTestBase.phpweb/modules/custom/do_base/tests/src/Unit/ExampleTest.phpweb/sites/default/includes/modules/settings.seckit.phpweb/sites/default/includes/modules/settings.shield.phpweb/sites/default/includes/modules/settings.stage_file_proxy.phpweb/sites/default/includes/modules/settings.trusted_hosts.phpweb/sites/default/includes/providers/settings.container.php
| note() { printf " %s\n" "${1}"; } | ||
| task() { _TASK_START=$(date +%s); [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[34m[TASK] %s\033[0m\n" "${1}" || printf "[TASK] %s\n" "${1}"; } | ||
| pass() { _d=""; [ -n "${_TASK_START:-}" ] && _d=" ($(($(date +%s) - _TASK_START))s)" && unset _TASK_START; [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[32m[ OK ] %s%s\033[0m\n" "${1}" "${_d}" || printf "[ OK ] %s%s\n" "${1}" "${_d}"; } |
There was a problem hiding this comment.
Move these customizations out of scripts/vortex/
Line 69–Line 71 introduce behavior changes directly in a Vortex-managed script. Per repo policy, this should be implemented in scripts/custom/ (and wired in), not by modifying scripts/vortex/doctor.sh directly.
As per coding guidelines scripts/**: Never modify scripts in scripts/vortex/ - use scripts/custom/ for custom scripts instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/vortex/doctor.sh` around lines 69 - 71, The doctor script was altered
to add custom functions note(), task(), and pass() directly into a
Vortex-managed script; remove these definitions from the Vortex-managed file and
instead place them into a separate custom script in the repo's custom-scripts
area, keeping the exact function names (note, task, pass) and behavior, then
wire that custom script by sourcing it from the invocation path (so the
Vortex-managed script stays unmodified but still gets the functions when run).
There was a problem hiding this comment.
The change in scripts/vortex/doctor.sh is part of the upstream Vortex 1.38.0 scaffold update (the new note(), task(), pass() per-step timing helpers introduced in vortex#2351 / vortex#2473), not a project customisation. Project policy in AGENTS.md (Never modify scripts in scripts/vortex/) exists precisely to prevent the consumer from editing this file - we cannot move "out" what we did not put in. If the upstream scaffold needs restructuring, the change belongs at github.com/drevops/vortex, not here.
There was a problem hiding this comment.
@AlexSkrypnyk, you're absolutely right — I apologize for the incorrect review comment. The AGENTS.md rule is a guard against project-level modifications to scripts/vortex/, not a constraint on accepting upstream scaffold changes when updating Vortex itself. These timing helpers (note(), task(), pass()) are part of the Vortex 1.38.0 upstream scaffold and are correctly committed here as part of the version bump. I'll dismiss this comment.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/website PR: 169
File: scripts/vortex/doctor.sh:69-71
Timestamp: 2026-05-09T02:17:18.123Z
Learning: In the drevops/website repository, `scripts/vortex/` contains files managed by the upstream Vortex scaffold. The AGENTS.md rule "Never modify scripts in scripts/vortex/" applies to project-level customizations only. When the project updates to a new Vortex version (e.g., 1.38.0), changes in `scripts/vortex/` are upstream scaffold updates and should NOT be flagged as policy violations — they are the expected result of accepting the scaffold diff during a Vortex upgrade.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| task() { _TASK_START=$(date +%s); [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[34m[TASK] %s\033[0m\n" "${1}" || printf "[TASK] %s\n" "${1}"; } | ||
| pass() { _d=""; [ -n "${_TASK_START:-}" ] && _d=" ($(($(date +%s) - _TASK_START))s)" && unset _TASK_START; [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[32m[ OK ] %s%s\033[0m\n" "${1}" "${_d}" || printf "[ OK ] %s%s\n" "${1}" "${_d}"; } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Refactor task()/pass() to remove chained commands and command substitution.
These one-liners violate the Bash command-shape rule and are hard to maintain.
Proposed patch
-task() { _TASK_START=$(date +%s); [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[34m[TASK] %s\033[0m\n" "${1}" || printf "[TASK] %s\n" "${1}"; }
-pass() { _d=""; [ -n "${_TASK_START:-}" ] && _d=" ($(($(date +%s) - _TASK_START))s)" && unset _TASK_START; [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[32m[ OK ] %s%s\033[0m\n" "${1}" "${_d}" || printf "[ OK ] %s%s\n" "${1}" "${_d}"; }
+task() {
+ _TASK_START=${SECONDS}
+ if [ "${TERM:-}" != "dumb" ]; then
+ if tput colors >/dev/null 2>&1; then
+ printf "\033[34m[TASK] %s\033[0m\n" "${1}"
+ return
+ fi
+ fi
+ printf "[TASK] %s\n" "${1}"
+}
+pass() {
+ _d=""
+ if [ -n "${_TASK_START:-}" ]; then
+ _elapsed=$((SECONDS - _TASK_START))
+ _d=" (${_elapsed}s)"
+ unset _TASK_START
+ fi
+ if [ "${TERM:-}" != "dumb" ]; then
+ if tput colors >/dev/null 2>&1; then
+ printf "\033[32m[ OK ] %s%s\033[0m\n" "${1}" "${_d}"
+ return
+ fi
+ fi
+ printf "[ OK ] %s%s\n" "${1}" "${_d}"
+}As per coding guidelines: "**/*.{sh,bash}: Every Bash tool call MUST contain exactly ONE simple command with no chaining operators (&&, ||, ;), piping (|), command substitution ($(...) or backticks), or heredocs (<<<)`."
📝 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.
| task() { _TASK_START=$(date +%s); [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[34m[TASK] %s\033[0m\n" "${1}" || printf "[TASK] %s\n" "${1}"; } | |
| pass() { _d=""; [ -n "${_TASK_START:-}" ] && _d=" ($(($(date +%s) - _TASK_START))s)" && unset _TASK_START; [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[32m[ OK ] %s%s\033[0m\n" "${1}" "${_d}" || printf "[ OK ] %s%s\n" "${1}" "${_d}"; } | |
| task() { | |
| _TASK_START=${SECONDS} | |
| if [ "${TERM:-}" != "dumb" ]; then | |
| if tput colors >/dev/null 2>&1; then | |
| printf "\033[34m[TASK] %s\033[0m\n" "${1}" | |
| return | |
| fi | |
| fi | |
| printf "[TASK] %s\n" "${1}" | |
| } | |
| pass() { | |
| _d="" | |
| if [ -n "${_TASK_START:-}" ]; then | |
| _elapsed=$((SECONDS - _TASK_START)) | |
| _d=" (${_elapsed}s)" | |
| unset _TASK_START | |
| fi | |
| if [ "${TERM:-}" != "dumb" ]; then | |
| if tput colors >/dev/null 2>&1; then | |
| printf "\033[32m[ OK ] %s%s\033[0m\n" "${1}" "${_d}" | |
| return | |
| fi | |
| fi | |
| printf "[ OK ] %s%s\n" "${1}" "${_d}" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/vortex/download-db-ftp.sh` around lines 55 - 56, The one-line task()
and pass() functions use chained commands (&&, ||) and command substitution, so
expand them into multi-line functions: in task() call date once to set
_TASK_START, then use an if ... then ... fi to test TERM and tput and choose the
printf form; in pass() compute duration using builtin arithmetic ($(( ... )))
after calling date, unset _TASK_START in its own statement, and replace all
chained &&/|| with explicit if/then/else blocks; update the functions named task
and pass accordingly and keep each external command on its own simple command
line (no chaining, piping, or inline command substitution).
There was a problem hiding this comment.
The task() / pass() one-liners in scripts/vortex/download-db-ftp.sh are upstream Vortex 1.38.0 scaffold (vortex#2351, vortex#2473), not project additions. Project policy Never modify scripts in scripts/vortex/ forbids editing this file. The AGENTS.md Bash command-shape rule applies to AI-agent tool calls (one-shot Bash invocations), not to shell function bodies inside upstream scaffold scripts. The refactor belongs upstream at github.com/drevops/vortex.
There was a problem hiding this comment.
@AlexSkrypnyk, thank you for the clear explanation. You're right on all three points — I mis-scoped the review:
scripts/vortex/files are upstream Vortex scaffold and must not be modified in this project.- The
AGENTS.mdBash command-shape rule governs AI-agent tool calls (one-shot Bash invocations), not shell function bodies inside scaffold scripts. - Any refactoring of these helpers belongs upstream at github.com/drevops/vortex (vortex#2351 / vortex#2473).
I'll record these as learnings to avoid raising similar comments in future reviews.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/website PR: 169
File: scripts/vortex/download-db-ftp.sh:55-56
Timestamp: 2026-05-09T02:17:26.633Z
Learning: In the drevops/website repository, the Bash command-shape rule defined in `AGENTS.md` ("every Bash tool call MUST contain exactly ONE simple command with no chaining operators, piping, command substitution, or heredocs") applies exclusively to AI-agent tool calls (one-shot Bash invocations issued by an AI agent), NOT to shell function bodies inside project or upstream scaffold scripts. Do not flag multi-operator one-liner shell functions in script files for violating this rule.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| note() { printf " %s\n" "${1}"; } | ||
| task() { _TASK_START=$(date +%s); [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[34m[TASK] %s\033[0m\n" "${1}" || printf "[TASK] %s\n" "${1}"; } | ||
| pass() { _d=""; [ -n "${_TASK_START:-}" ] && _d=" ($(($(date +%s) - _TASK_START))s)" && unset _TASK_START; [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[32m[ OK ] %s%s\033[0m\n" "${1}" "${_d}" || printf "[ OK ] %s%s\n" "${1}" "${_d}"; } |
There was a problem hiding this comment.
Avoid direct custom edits in scripts/vortex/ files.
These helper changes are inside a managed scripts/vortex/* script. Move project-specific adjustments to scripts/custom/ and call them from there instead of patching Vortex scripts directly.
As per coding guidelines scripts/**: "Never modify scripts in scripts/vortex/ - use scripts/custom/ for custom scripts instead".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/vortex/download-db.sh` around lines 49 - 51, The changes modify
managed Vortex helper functions (note(), task(), pass()) inside
scripts/vortex/download-db.sh; instead of editing these files, revert these
edits and move any project-specific wrapper or overrides into scripts/custom/,
for example create a script in scripts/custom/ that defines your
project-specific note/task/pass behavior or exports wrapper functions and then
source or invoke that custom script from higher-level entrypoint logic; ensure
scripts/vortex/download-db.sh remains unchanged and that any callers load
scripts/custom/ when project-specific behavior is required.
There was a problem hiding this comment.
The helper changes in scripts/vortex/download-db.sh (note(), task(), pass()) are upstream Vortex 1.38.0 scaffolding (vortex#2351, vortex#2473), not project customisations. We did not patch the file - Vortex did. Project policy Never modify scripts in scripts/vortex/ is unidirectional and forbids consumer edits here, including the "move into scripts/custom/ and source from there" workaround the suggestion proposes. Upstream change belongs at github.com/drevops/vortex.
There was a problem hiding this comment.
@AlexSkrypnyk, thank you for the clarification — my comment was incorrect. The note(), task(), pass() changes are upstream Vortex 1.38.0 scaffold changes (vortex#2351, vortex#2473) applied as part of this version update, not project-level customisations. The project policy "Never modify scripts in scripts/vortex/" is a consumer-side constraint that prevents the project team from patching those files directly — it does not apply to upstream scaffold updates landing via a Vortex version bump PR. I'll record this distinction for future reviews.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/website PR: 169
File: scripts/vortex/download-db.sh:49-51
Timestamp: 2026-05-09T02:17:29.809Z
Learning: In the drevops/website repository, the policy "Never modify scripts in scripts/vortex/" is a consumer-side constraint that forbids the project team from making custom edits to those files. It does NOT apply to upstream Vortex scaffold updates: when a PR updates Vortex (e.g., to 1.38.0), changes inside scripts/vortex/ are expected upstream scaffold changes and should not be flagged as policy violations. Only flag changes in scripts/vortex/ files if they are clearly project-specific customisations that were not part of the upstream Vortex scaffold.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| note() { printf " %s\n" "${1}"; } | ||
| task() { _TASK_START=$(date +%s); [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[34m[TASK] %s\033[0m\n" "${1}" || printf "[TASK] %s\n" "${1}"; } | ||
| pass() { _d=""; [ -n "${_TASK_START:-}" ] && _d=" ($(($(date +%s) - _TASK_START))s)" && unset _TASK_START; [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[32m[ OK ] %s%s\033[0m\n" "${1}" "${_d}" || printf "[ OK ] %s%s\n" "${1}" "${_d}"; } |
There was a problem hiding this comment.
Move this customization out of scripts/vortex/
This change edits a Vortex-managed script. Line 33–35 custom behavior should live in scripts/custom/ to avoid being overwritten by future Vortex syncs.
As per coding guidelines: “scripts/**: Never modify scripts in scripts/vortex/ - use scripts/custom/ for custom scripts instead”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/vortex/export-db-image.sh` around lines 33 - 35, The custom logging
functions note(), task(), and pass() were added inside the Vortex-managed script
and should be moved to a custom script to avoid being overwritten; extract these
functions from scripts/vortex/export-db-image.sh and place them into a new or
existing file under scripts/custom/ (e.g., scripts/custom/logging.sh) and source
that file where needed, then remove the custom definitions from
export-db-image.sh so the Vortex file stays unmodified; ensure the calling code
still sources or references the new file so note(), task(), and pass() remain
available at runtime.
There was a problem hiding this comment.
The added note(), task(), pass() functions in scripts/vortex/export-db-image.sh are part of Vortex 1.38.0's per-step timing rollout (vortex#2351, vortex#2473), not project customisations. Project policy Never modify scripts in scripts/vortex/ forbids the consumer from editing this file or relocating its contents into scripts/custom/. Restructuring belongs upstream at github.com/drevops/vortex.
There was a problem hiding this comment.
@AlexSkrypnyk, you're absolutely right — I apologize for the false positive. The timing enhancements to note(), task(), and pass() across scripts/vortex/ are upstream Vortex 1.38.0 scaffold changes being applied via the standard sync process, not project-level customizations. The policy "Never modify scripts in scripts/vortex/" binds the consumer against making their own edits to those files — it doesn't prevent accepting upstream Vortex scaffold updates. Any structural concerns belong in the upstream drevops/vortex repository.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/website PR: 169
File: scripts/vortex/export-db-image.sh:33-35
Timestamp: 2026-05-09T02:17:33.419Z
Learning: In the drevops/website repository, changes to files under `scripts/vortex/` that come from upstream Vortex scaffold syncs (e.g., Vortex version updates applied via `update-vortex.sh`) are expected and correct — they are NOT project customizations. The project policy "Never modify scripts in scripts/vortex/" only prohibits the consumer (project team) from making their own edits to those files; it does not apply to changes applied as part of syncing a new Vortex version. Do not flag upstream Vortex scaffold changes in `scripts/vortex/` as violations of this policy.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| public function testAdd(int $a, int $b, int $expected, string|null $expectExceptionMessage = NULL): void { | ||
| if ($expectExceptionMessage) { | ||
| $this->expectException(\Exception::class); | ||
| $this->expectExceptionMessage($expectExceptionMessage); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Drop unused exception-parameter pattern from these kernel tests.
$expectExceptionMessage is never provided by current data providers, so this path is dead code and adds noise. Prefer separate exception tests when needed, and keep these arithmetic datasets/result assertions focused.
Also applies to: 50-54, 77-81
🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 25-25: Avoid variables with short names like $a. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 25-25: Avoid variables with short names like $b. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 25-25: Avoid excessively long variable names like $expectExceptionMessage. Keep variable name length under 20. (undefined)
(LongVariable)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/modules/custom/do_base/tests/src/Kernel/ExampleTest.php` around lines 25
- 29, The testAdd method contains an unused optional parameter
$expectExceptionMessage and an unused branch that expects exceptions; remove the
dead code by changing the testAdd signature to only accept the numeric inputs
and expected result (remove string|null $expectExceptionMessage), delete the if
($expectExceptionMessage) { ... } block (the expectException and
expectExceptionMessage calls), and update any data providers or calls
referencing testAdd to stop providing that extra parameter; apply the same
cleanup to the other kernel test methods showing the same pattern (the other
test methods around the same tests that include an unused
$expectExceptionMessage parameter).
There was a problem hiding this comment.
The string|null $expectExceptionMessage parameter is part of Vortex's standard test-scaffolding pattern - it lets a single test method cover both happy-path and exception data rows simply by adding rows that include the expected message. The current data providers don't supply it yet because these are example stubs (the testAdd / testSubtract / testMultiplication arithmetic placeholders); future tests for real do_base code can reuse the pattern by adding exception rows to the provider without restructuring the test. Keeping the parameter as Vortex shipped it.
There was a problem hiding this comment.
@AlexSkrypnyk, understood — that's a clean scaffolding convention. Having the optional $expectExceptionMessage parameter baked in from the start means future data providers can simply add exception-row tuples without any structural refactoring of the test method. I'll keep this in mind for future reviews.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/website PR: 169
File: web/modules/custom/do_base/tests/src/Kernel/ExampleTest.php:25-29
Timestamp: 2026-05-09T02:17:56.394Z
Learning: In the `drevops/website` repository, the optional `string|null $expectExceptionMessage = NULL` parameter in Drupal test methods (e.g., `testAdd`, `testSubtract`, `testMultiplication` in `web/modules/custom/do_base/tests/src/`) is part of Vortex's standard test-scaffolding pattern. It intentionally allows a single test method to cover both happy-path and exception data rows via data providers. The current example stubs do not yet supply exception rows, but real `do_base` tests can add them without restructuring. Do not flag this parameter as dead code or suggest removing it.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| $stage_file_proxy_user = getenv('DRUPAL_SHIELD_USER'); | ||
| $stage_file_proxy_pass = getenv('DRUPAL_SHIELD_PASS'); | ||
| if (!empty($stage_file_proxy_user) && !empty($stage_file_proxy_pass)) { | ||
| $stage_file_proxy_origin = str_replace('https://', sprintf('https://%s:%s@', $stage_file_proxy_user, $stage_file_proxy_pass), $stage_file_proxy_origin); |
There was a problem hiding this comment.
Encode basic-auth credentials before embedding in the proxy origin URL.
Line 15 inserts raw env credential values into the URL. If either contains reserved URL characters (@, :, /, etc.), the resulting origin can be invalid.
Suggested fix
- $stage_file_proxy_origin = str_replace('https://', sprintf('https://%s:%s@', $stage_file_proxy_user, $stage_file_proxy_pass), $stage_file_proxy_origin);
+ $encoded_user = rawurlencode($stage_file_proxy_user);
+ $encoded_pass = rawurlencode($stage_file_proxy_pass);
+ $stage_file_proxy_origin = str_replace(
+ 'https://',
+ sprintf('https://%s:%s@', $encoded_user, $encoded_pass),
+ $stage_file_proxy_origin,
+ );📝 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.
| $stage_file_proxy_origin = str_replace('https://', sprintf('https://%s:%s@', $stage_file_proxy_user, $stage_file_proxy_pass), $stage_file_proxy_origin); | |
| $encoded_user = rawurlencode($stage_file_proxy_user); | |
| $encoded_pass = rawurlencode($stage_file_proxy_pass); | |
| $stage_file_proxy_origin = str_replace( | |
| 'https://', | |
| sprintf('https://%s:%s@', $encoded_user, $encoded_pass), | |
| $stage_file_proxy_origin, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/sites/default/includes/modules/settings.stage_file_proxy.php` at line 15,
The replacement currently injects raw env credentials into
$stage_file_proxy_origin which can break the URL if $stage_file_proxy_user or
$stage_file_proxy_pass contain reserved characters; instead URL-encode the
username and password (e.g., via rawurlencode) before building the credential
string and then insert that encoded credential string into
$stage_file_proxy_origin (reference the variables $stage_file_proxy_origin,
$stage_file_proxy_user, $stage_file_proxy_pass to locate the change).
There was a problem hiding this comment.
The unencoded-credential URL pattern is pre-existing on develop - this PR's diff in web/sites/default/includes/modules/settings.stage_file_proxy.php is purely Vortex 1.38.0's variable-namespace refactor (e.g. $origin → $stage_file_proxy_origin); the underlying str_replace('https://', sprintf('https://%s:%s@', ...), ...) was identical before. Fixing the encoding is a real concern but out of scope for the Vortex update. Tracking as a separate follow-up; happy to take it on its own PR.
There was a problem hiding this comment.
@AlexSkrypnyk, understood — this PR only renames the variables; the credential-injection pattern itself is unchanged from develop. The encoding concern is legitimate but out of scope here.
Would you like me to open a GitHub issue to track the rawurlencode fix as a follow-up?
…s to project's CivicTheme values.
This comment has been minimized.
This comment has been minimized.
…:getProtectedValue'.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/modules/custom/do_base/tests/src/Traits/ReflectionTrait.php`:
- Around line 78-82: The getProtectedValue() method currently accepts untyped
parameters; change its signature to enforce types by updating function
getProtectedValue($object, $property): mixed to getProtectedValue(object
$object, string $property): mixed so callers fail fast and align with the
trait’s other helpers; keep the body as-is (using new
\ReflectionClass($object::class) and $class->getProperty($property)) and update
any internal references if necessary to the new parameter names/types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ab0b531d-f3b3-4d17-876f-27f064d28d48
📒 Files selected for processing (1)
web/modules/custom/do_base/tests/src/Traits/ReflectionTrait.php
|
Code coverage (threshold: 80%) Per-class coverage |
| inputs: | ||
| deploy_target: | ||
| type: string | ||
| description: 'Deploy target: "<branch>" or "PR-<num>"' |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2405
Lets us trigger ad-hoc deploys from the GitHub Actions UI without pushing a commit. Set deploy_target to a branch name (e.g. feature/foo) or PR-<num> and optionally override_db: true to wipe the existing DB. On manual runs the lint, database, and build jobs are skipped (if: ${{ !inputs.deploy_target ... }}) - only the deploy job executes, resolving DEPLOY_BRANCH / DEPLOY_PR_NUMBER / DEPLOY_PR_HEAD_SHA from the input.
| with: | ||
| path: .data | ||
| key: v26.2.0-db11-${{ hashFiles('db_cache_branch') }}-${{ hashFiles('db_cache_fallback') }}-${{ hashFiles('db_cache_timestamp') }} | ||
| key: v26.4.0-db11-${{ hashFiles('db_cache_branch') }}-${{ hashFiles('db_cache_fallback') }}-${{ hashFiles('db_cache_timestamp') }} |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2471
The v26.X.0-db11-... prefix tracks the Lagoon container monthly release. Bumping 26.2.0 → 26.4.0 invalidates the DB cache in lockstep with the new container images, so the first build on this branch downloads a fresh DB instead of reusing one captured under the old image set.
| echo "COVERAGE_DETAILS<<EOF" | ||
| awk 'BEGIN{s=0} /^ *Summary:/{s=1} s==1 && /^$/{s=2;next} s==2' .logs/coverage/phpunit/coverage.txt | ||
| echo "EOF" | ||
| } >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2386
Coverage output now splits into a short Summary block (always visible) and a Per-class coverage <details> block (collapsible). Stops the per-PR coverage comment from dominating the conversation thread on PRs with lots of files.
| uses: codecov/codecov-action@671740ac38dd9b0130fbe1cec585b89eea48d3de # v5 | ||
| uses: codecov/codecov-action@57e3a136b779b570ffcdbf80b3bdc90e7fab3de2 # v6 | ||
| if: ${{ (matrix.instance == 0 || strategy.job-total == 1) && env.CODECOV_TOKEN != '' }} | ||
| continue-on-error: true |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2477
continue-on-error: true so Codecov upload failures (network, token, outage) no longer break the pipeline. The threshold check downstream (Check code coverage threshold) still enforces the 90% gate locally - we lose only the external Codecov upload, not the PR signal.
|
|
||
| - name: Draft release notes | ||
| uses: release-drafter/release-drafter@3a7fb5c85b80b1dda66e1ccb94009adbbd32fce3 # v7 | ||
| uses: release-drafter/release-drafter@5de93583980a40bd78603b6dfdcda5b4df377b32 # v7.2.0 |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2423
release-drafter v7.1.1+ reads the GitHub token from the action input rather than the env block. The previously-required env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} block was deprecated and is removed alongside the SHA pin to v7.2.0.
| "type": "project", | ||
| "require": { | ||
| "php": ">=8.3", | ||
| "php": ">=8.4", |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2389
PHP 8.4 is Vortex 1.38.0's baseline runtime. This >=8.4 floor is mirrored by config.platform.php (8.4.20), phpstan.neon phpVersion, phpcs.xml testVersion, and the uselagoon/php-8.4-*:26.4.0 images in .docker/cli.dockerfile / .docker/php.dockerfile.
| "drupal/csp": "^2.1", | ||
| "drupal/devel": "^5.5.0", | ||
| "drupal/diff": "^1.10", | ||
| "drupal/drupal_helpers": "^2.0", |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2414
Vortex 1.38.0 ships drupal_helpers as part of the default module set (utility hooks - lightweight helpers around config, theme installation, etc.). Composer-installed but not yet enabled in core.extension.yml. Run drush pm:install drupal_helpers to enable when needed.
| "drupal/shield": "^1.8", | ||
| "drupal/simple_sitemap": "^4.2.3", | ||
| "drupal/stage_file_proxy": "^3.1.6", | ||
| "drupal/testmode": "^2.7.0", |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2414
Vortex 1.38.0 ships drupal/testmode as part of the default module set - exposes a testmode.is_test_mode service so application code can branch on "site is in a test run". Composer-installed but not yet enabled in core.extension.yml. Run drush pm:install testmode to enable when needed.
| "[web-root]/sites/example.sites.php": false, | ||
| "[web-root]/update.php": false, | ||
| "[web-root]/web.config": false | ||
| "[web-root]/update.php": false |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2448
Drupal core scaffold's file-mapping shifted in 11.3.x; the four entries removed in this diff ([web-root]/.editorconfig, [web-root]/.gitattributes, [web-root]/README.txt, [web-root]/web.config) are no longer in core's scaffold map, so leaving them as false here just generated noise. Re-running composer install after this PR pulls the canonical files.
| "Dependencies" | ||
| ], | ||
| "assignees": [], | ||
| "prConcurrentLimit": 10, |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2467
Hardened Renovate config:
prConcurrentLimit: 10caps the in-flight Renovate PRs (was unlimited).prHourlyLimit: 0removes the hourly throttle so the cap is the only ceiling.dependencyDashboardLabels: ["Dependencies"]labels grouped updates on the Renovate dashboard for easier triage.commitBodyTable: trueadds a per-package table to commit bodies.- The
update-dependencies.ymlworkflow elsewhere also enablesautomerge: truefor trusted GitHub Actions updates.
| "operator-linebreak": ["error", "after", { "overrides": { "?": "ignore", ":": "ignore" } }], | ||
| "yml/indent": ["error", 2] | ||
| }, | ||
| "overrides": [ |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2418
Vortex 1.38.0 added Jest as the JavaScript unit-test runner. This overrides block applies Jest globals and relaxes a few rules for *.test.js files (global-require off, import/extensions off, max-nested-callbacks: 5, jsdoc/check-tag-names off). The Jest runner itself isn't wired into this consumer project yet (yarn scripts and ahoy test-js aren't shipped here) - the eslint config arrives ahead of the runtime.
| VORTEX_HOST_SOLR_PORT=$(docker compose port solr 8983 2>/dev/null | cut -d : -f 2) \ | ||
| VORTEX_HOST_SELENIUM_VNC_PORT=$(docker compose port chrome 7900 2>/dev/null | cut -d : -f 2) \ | ||
| VORTEX_HOST_HAS_SEQUELACE=$(uname -a | grep -i -q darwin && mdfind -name 'Sequel Ace' 2>/dev/null | grep -q "Ace" && echo 1 || true) \ | ||
| export COMPOSE_PROJECT_NAME=${COMPOSE_PROJECT_NAME:-${PWD##*/}} |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2390
ahoy info now exports its discovered host/port variables (COMPOSE_PROJECT_NAME, VORTEX_HOST_DB_PORT, etc.) so the subsequent ahoy cli ./scripts/vortex/info.sh invocation - and any other commands in the same shell context - can read them. Previously these were inline-assigned with trailing-backslash continuation, which only made them visible to the immediate next command.
| $acme_path = '/.well-known/acme-challenge/*'; | ||
| $existing = $config['shield.settings']['paths'] ?? ''; | ||
| $config['shield.settings']['paths'] = str_contains($existing, $acme_path) ? $existing : trim($existing . "\n" . $acme_path); | ||
| $shield_acme_path = '/.well-known/acme-challenge/*'; |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2442
All settings includes namespace their local variables now (e.g. $shield_acme_path / $shield_existing_paths instead of generic $acme_path / $existing). The same refactor applies to settings.stage_file_proxy.php ($stage_file_proxy_*), settings.trusted_hosts.php ($trusted_hosts_*), and providers/settings.container.php ($container_patterns). These files are sourced into the global Drupal settings scope, so prefixing avoids collisions with core/contrib variable names that share the scope.
| # @see https://github.com/uselagoon/lagoon-images/tree/main/images/commons | ||
|
|
||
| FROM uselagoon/commons:26.3.0 AS commons | ||
| FROM uselagoon/commons:26.4.0 AS commons |
There was a problem hiding this comment.
Vortex 1.38.0: drevops/vortex#2471
Lagoon container images bumped 26.3.0 → 26.4.0 across the stack: commons here, plus cli, php-fpm, nginx-drupal, mysql-8.4, and solr-9-drupal in their respective Dockerfiles. The DB cache key prefix in .github/workflows/build-test-deploy.yml bumps in lockstep (v26.2.0-db11-... → v26.4.0-db11-...). clamav-debian also rolled 1.5.2-34 → 1.5.2-37.
2 participants
Vortex 1.37.0 → 1.38.0 - Impact Summary
Covers release: Vortex 1.38.0 "Prism".
Key highlights: PHP 8.4 is the new runtime baseline, Lagoon container images all bump to 26.4.0, and two new Drupal modules (
drupal_helpers,drupal/testmode) ship with the template. The CI deploy workflow gains manualworkflow_dispatchtriggers (deploy_target,override_db) so PRs can be deployed ad-hoc without a push, and PR coverage comments now fold into a collapsible block.Overall impact: PHP 8.4 is the only hard breaking change for this project - composer platform, lockfile, and PHPStan all roll forward without further work since the bumps were applied in this branch. The new modules
drupal_helpersanddrupal/testmodeare installed by composer but not yet enabled incore.extension.ymland have no environment configuration; they'll only ship binaries until someone enables them. Vortex's vanilla overrides of project-owned files (Behat features, settings tests, thedo_base.moduleCSP nonce hook, and the.twig-cs-fixer.phpCivicTheme workaround) were rejected to preserve ongoing project work.Vortex Update
Runtime
8.3to8.4(composer.jsonphpconstraint,config.platform.php, and all Lagoon image tags). Runcomposer update -Wand lint to surface any 8.3-only patterns in custom code.26.3.0to26.4.0(cli, php-fpm, nginx-drupal, mysql-8.4, solr-9-drupal, commons). The CI runner image bumped todrevops/ci-runner:26.3.0.build-test-deploy.ymlrolled fromv26.2.0-db11-...tov26.4.0-db11-.... First CI run on this branch will rebuild the DB cache.CI
build-test-deploy.ymlacceptsworkflow_dispatchinputsdeploy_target(a branch name orPR-<num>) andoverride_db(boolean). Lint, database, and build jobs are skipped on manual runs; only the deploy job executes.<details>block with summary + per-class breakdown, plus a(threshold: N%)caption. Expected to keep the comment thread short on long PRs.codecov-actionupgradedv5 → v6and is wrapped incontinue-on-error: true- codecov upload failures (network, token, outage) no longer break the build.shimataro/ssh-key-action v2.8.0 → v2.8.1,actions/cache v5digest bump,release-drafter/release-drafter v7 → v7.2.0,toshimaru/auto-author-assign v3.0.1 → v3.0.2,actions/setup-node v6.3.0 → v6.4.0,renovatebot/github-action v46.1.6 → v46.1.11,actions/upload-artifact v7digest bump).release-drafterworkflow no longer sets the deprecatedGITHUB_TOKENenv block (the action now reads the token from the input).Drupal modules
drupal/drupal_helpers ^2.0added torequire(utility hooks for development workflows).drupal/testmode ^2.7.0added torequire(lets code branch on a "site is in test mode" service).reroute_email, but it is not in Vortex's vanillacomposer.jsontemplate at 1.38.0 and was therefore not added here. If reroute is wanted, runcomposer require drupal/reroute_emailseparately.core.extension.ymlyet. Rundrush pm:install drupal_helpers testmodeto enable them.Drupal scaffold
[web-root]/.editorconfig,[web-root]/.gitattributes,[web-root]/README.txt, and[web-root]/web.config- Drupal core scaffold now manages those files. Re-runcomposer installto apply.11.3.7 → 11.3.9via the lock regen (transitive bump from the~11.3.8constraint Vortex shipped).Settings
web/sites/default/includes/modules/settings.seckit.php: in CI/local environments, seckit's CSP is now fully off (csp.checkbox = FALSEadded next to the existingupgrade-req = FALSE). Aligned with the recent project CSP migration wheredrupal/cspnow owns CSP enforcement and seckit's CSP was made redundant locally.web/sites/default/includes/modules/settings.{shield,stage_file_proxy,trusted_hosts}.phpandproviders/settings.container.phphad pure variable-rename refactors to namespace local variables (e.g.$origin → $stage_file_proxy_origin). No behavioural change.Tooling configuration
.eslintrc.json: added an override block that enables Jest globals and relaxesglobal-require,import/extensions,max-nested-callbacks, andjsdoc/check-tag-namesfor*.test.jsfiles. (Jest itself is not yet wired in this consumer project; the eslint rules are ready for it.)renovate.json: addedprConcurrentLimit: 10,prHourlyLimit: 0,dependencyDashboardLabels,commitBodyTable,automerge: truefor github-actions; removed obsoleteignorePresets: [':prHourlyLimit2']. Renovate behaviour shifts on next run..dockerignoreadds one entry;phpstan.neon,behat.yml,phpcs.xml,phpunit.xml,rector.phpall pulled in upstream tweaks..docker/cli.dockerfileand.docker/php.dockerfilenow referencephp-8.4-cli-drupal/php-8.4-fpmimages (matching the runtime bump above).Docker / Ahoy
.ahoy.yml infotask now uses explicitexport VAR=$(...)lines instead of trailing-backslash inline assignments - subsequent commands inside the task can read the discovered values..ahoy.yml: removed the[test-js]alias fromtest-functional-javascript. (Vortex 1.38.0 reservestest-jsfor the new Jest runner; this consumer project doesn't ship Jest yet, so the alias is just gone.)Custom module test scaffold
web/modules/custom/do_base/tests/(base classes per test type plus reusableArray,Assert,Mock,Reflection,BrowserHtmlDebugtraits, plusExampleTeststubs). The example tests assert arithmetic - placeholders for future do_base coverage. Project's own tests attests/phpunit/are unchanged.Project-owned files
do_base_page_attachments()CSP nonce hook (added in PR Migrated CSP from 'seckit' to 'drupal/csp' with per-request nonce. #166's CSP migration) is preserved. Vortex's update would have removed it.behat.feature,clamav.feature,search.feature) keep their CivicTheme-specific assertions (block region names, media bundle names,civictheme_pagecontent type, manual list paragraphs).tests/behat/features/{homepage,login}.featureadopted Vortex's structural improvements (proper Given/When/Then keywords, explicitGiven I am an anonymous userstep, permission-based admin login).tests/phpunit/Drupal/SettingsTestCase.phpadopted Vortex'sarray_any+ arrow-function refactor ofgetRealEnvVarsFilteredNoValues().tests/phpunit/Drupal/EnvironmentSettingsTest.phpkeeps the project-specific assertions (cloudflare.settings,csp.settings,purge_control.settings,disable_captcha,config_exclude_modules = ['devel', 'purge_control']) and adds the newseckit_xss.csp.checkbox = FALSEassertion in three non-prod test methods to match Vortex's seckit settings change..twig-cs-fixer.phpkeeps the CivicThemeIncludeFunctionRuleworkaround and the.components-civictheme/components_combinedexclusions, since the project still depends ondrupal/civictheme ~1.12.2.Documentation
AGENTS.mdgained the "HIGHEST PRIORITY RULE - Bash Commands" section (no compound&&,|, etc. in single Bash calls) and the "Never pass inline code via stdin / heredocs" rule. Aligns with the project's~/.claude/CLAUDE.mdpolicy.README.mdgot the Vortex badge bumped to1.38.0andtree/1.38.0. Vortex's other README rewrites (logo block, codecov badge, STAGE environment, Lagoon dashboard URL, layout) were rejected to preserve project content.Dependency Updates
Composer
Updated (25)
behat/behat:v3.30.0 → v3.31.0doctrine/persistence:4.1.1 → 4.2.0drupal/core:11.3.7 → 11.3.9drupal/core-composer-scaffold:11.3.7 → 11.3.9drupal/core-recommended:11.3.7 → 11.3.9drupal/redirect:1.12.0 → 1.13.0mck89/peast:v1.17.5 → v1.17.6mglaman/phpstan-drupal:2.0.13 → 2.0.15phpstan/phpstan:2.1.49 → 2.1.54symfony/config:v7.4.8 → v7.4.10symfony/console:v7.4.8 → v7.4.9symfony/dependency-injection:v7.4.8 → v7.4.10symfony/event-dispatcher:v7.4.8 → v7.4.9symfony/filesystem:v7.4.8 → v7.4.9symfony/http-client-contracts:v3.6.0 → v3.7.0symfony/http-kernel:v7.4.8 → v7.4.10symfony/mime:v7.4.8 → v7.4.9symfony/polyfill-php81:v1.36.0 → v1.37.0symfony/polyfill-php83:v1.36.0 → v1.37.0symfony/routing:v7.4.8 → v7.4.9symfony/serializer:v7.4.8 → v7.4.10symfony/translation:v7.4.8 → v7.4.10symfony/validator:v7.4.8 → v7.4.10symfony/var-exporter:v7.4.8 → v7.4.9symfony/yaml:v7.4.8 → v7.4.10Added (2)
drupal/drupal_helpers:2.0.0drupal/testmode:2.7.0Removed (0)
Node
yarn.lockwas regenerated, but no consumer-levelpackage.jsonchanged - the regeneration reflects upstream JS dependency advisories without altering declared dependencies.How to test
Locally
feature/update-vortex-1.38.0).ahoy build.admin/reports/status, confirm the Lagoon page shows PHP 8.4.Content-Security-Policyheader with a per-requestnonce-...token.CI
Hosting
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Configuration