Fixed eight small issues across ahoy commands, installer runners, and CI workflows.#2533
Conversation
…rencing missing scripts.
…h 'ahoy update-videos installer'.
…rwritten by raw exit code.
…no longer in the template.
…n 'vortex-test-installer.yml'.
WalkthroughThis PR updates Ahoy ChangesInstaller command consolidation and deployment updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 @.ahoy.yml:
- Line 146: The download-db2 script currently exports only
VORTEX_DOWNLOAD_DB_FORCE when --fresh is passed, but the router and provider
scripts check indexed variables like VORTEX_DOWNLOAD_DB2_FRESH and
VORTEX_DOWNLOAD_DB2_FORCE; update the --fresh handling in the download-db2
invocation (the case that matches " --fresh ") to export both
VORTEX_DOWNLOAD_DB2_FRESH=1 and VORTEX_DOWNLOAD_DB2_FORCE=1 (i.e., follow the
same pattern used by download-db but using the indexed names derived from
VORTEX_DB_INDEX), so the router (.vortex/tooling/src/download-db) and provider
scripts (.vortex/tooling/src/download-db-lagoon / download-db-acquia) will
correctly honor --fresh for db2.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0446c0c9-62a0-49af-a53c-7fd58ddefc5b
⛔ Files ignored due to path filters (11)
.vortex/installer/tests/Fixtures/handler_process/_baseline/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_acquia/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_ftp/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_lagoon/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_s3/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_url/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/provision_profile/.ahoy.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (9)
.ahoy.yml.github/workflows/vortex-test-installer.yml.vortex/.ahoy.yml.vortex/docs/content/contributing/maintenance/installer.mdx.vortex/docs/content/contributing/maintenance/release.mdx.vortex/installer/CLAUDE.md.vortex/installer/src/Prompts/Handlers/Services.php.vortex/installer/src/Runner/CommandRunner.php.vortex/installer/src/Runner/ProcessRunner.php
💤 Files with no reviewable changes (4)
- .vortex/.ahoy.yml
- .vortex/installer/src/Runner/CommandRunner.php
- .vortex/installer/src/Runner/ProcessRunner.php
- .vortex/installer/src/Prompts/Handlers/Services.php
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
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 @@
## main #2533 +/- ##
==========================================
- Coverage 86.49% 86.11% -0.39%
==========================================
Files 94 87 -7
Lines 4674 4501 -173
Branches 47 3 -44
==========================================
- Hits 4043 3876 -167
+ Misses 631 625 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ORTEX_DOWNLOAD_DB2_*' flags.
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
|
Code coverage (threshold: 90%) Per-class coverage |
Summary
Bundles eight verified findings from an exploratory assessment of the Vortex codebase. Each commit is a focused, independent fix - small enough to review individually but grouped here because they share a theme of cleaning up small bugs, dead code, and CI waste surfaced by the same pass.
Changes
Ahoy command bugs and broken doc references
download-db --freshno-op on default sources (.ahoy.yml). The--freshflag setVORTEX_DOWNLOAD_DB_FRESH=1, but thedownload-dbscript readsVORTEX_DOWNLOAD_DB_FORCE. Only the Acquia and Lagoon downloader variants happen to also read_FRESH, so on the defaulturl/ftp/s3paths--freshsilently did nothing. Fix: the sameexportstatement now also setsVORTEX_DOWNLOAD_DB_FORCE=1, keeping both variables for any code that reads either.download-db2 --freshwas missing the indexed env vars (.ahoy.yml). Same class of bug as above but for the migration DB. The router resolves env vars with the index suffix (VORTEX_DOWNLOAD_DB${_db_index}_FORCE, i.e.VORTEX_DOWNLOAD_DB2_FORCEwhenVORTEX_DB_INDEX=2); the Acquia/Lagoon downloaders gate fresh behavior onVORTEX_DOWNLOAD_DB2_FRESH. Thedownload-db2 --freshinvocation exported the un-indexedVORTEX_DOWNLOAD_DB_FORCEinstead, so neither the cache-skip nor the fresh dump path was honored. Fix: exportVORTEX_DOWNLOAD_DB2_FRESH=1 VORTEX_DOWNLOAD_DB2_FORCE=1.ahoy test-workflowandahoy test-deployment(.vortex/.ahoy.yml). Both pointed at shell scripts that no longer exist - the work moved to PHPUnit tests under.vortex/tests/phpunit/Functional/and neither command is referenced anywhere in CI. Removed.ahoy update-installer-videoreferences (.vortex/installer/CLAUDE.md,.vortex/docs/content/contributing/maintenance/installer.mdx,.vortex/docs/content/contributing/maintenance/release.mdx). Three files instructed maintainers to run a command that does not exist. Replaced with the actual command,ahoy update-videos installer.Dead code in the installer
matchblock in runners (.vortex/installer/src/Runner/ProcessRunner.php,.vortex/installer/src/Runner/CommandRunner.php). Both runners had amatch ($exit_code) { ... }writing an aliased value into$this->exitCode, immediately followed by$this->exitCode = $exit_code;. The match was 100% no-op. Removed both blocks and the now-unuseduse Symfony\Component\Console\Command\Command;import in each file. TheRunnerInterface::EXIT_*constants are kept - they are public API surface referenced by mock callbacks inBuildCommandTestandCheckRequirementsCommandTest..vortex/installer/src/Prompts/Handlers/Services.php). TwoFile::removeLineInFile()calls targeted backslash-continuation forms of the Solr port line in.ahoy.yml(one had// @todo Remove after 25.10.0 release.). The current template line has no\\continuation, so neither pattern matches anything. Also, that whole line is already wrapped in#;< SERVICE_SOLR ... #;> SERVICE_SOLRand gets removed viaFile::removeTokenAsync('SERVICE_SOLR')earlier in the same handler. The two calls were doubly dead. Removed both.CI waste in
vortex-test-installer.ymlcancel-in-progressguard.vortex-test-common.ymlhas aconcurrencyblock;vortex-test-installer.ymldid not. Rapid force-pushes queued multiple ~15 min runs. Copied the sameconcurrencyblock over.if: matrix.php-versions == '8.2', matching the existing guard already on the PHAR upload step.Auto-regenerated fixtures
20
.ahoy.ymlfixture files under.vortex/installer/tests/Fixtures/handler_process/propagate thedownload-dbanddownload-db2env-var changes. Generated byahoy update-snapshots.Before / After
The
--freshenv-var flow on the defaulturlsource for the primary DB:The same flow for the migration DB, via the indexed lookup (
_db_index="2"):