Fixed code artifact permissions not being preserved. #2010
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe CI workflow was updated to gate artifact export on matrix instance and branch name, switch artifact packaging from a directory to a tarball in /tmp/artifacts, adjust upload/download paths, and add explicit unpack steps in both build and deploy phases to restore code into /tmp/workspace. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Job as Build/Test Job
participant Art as /tmp/artifacts
participant WS as /tmp/workspace
participant Store as Actions Artifacts
GH->>Job: Run matrix job(s)
alt Export gated
Note over Job: matrix.instance == 0<br/>AND contains(VORTEX_DEPLOY_TYPES,'artifact')<br/>AND NOT startsWith(head_ref||ref_name,'deps/')
Job->>WS: Prepare code in workspace
Job->>Art: Create /tmp/artifacts
Job->>Art: tar -C /tmp/workspace -cpf /tmp/artifacts/code.tar code
Job->>Store: Upload code.tar
else Skipped
Note over Job: No artifact export
end
GH->>Job: Deploy stage
alt Artifact-based deploy
Job->>Store: Download code.tar -> /tmp/artifacts
Job->>WS: mkdir -p /tmp/workspace
Job->>WS: tar -xpf /tmp/artifacts/code.tar -C /tmp/workspace
Note over WS,Job: Code restored to /tmp/workspace/code
else Non-artifact deploy
Note over Job: Existing deploy path
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (13)
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1).github/workflows/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
🔇 Additional comments (2)
✨ Finishing touches🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
scripts/vortex/deploy-artifact.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
scripts/vortex/**
📄 CodeRabbit inference engine (CLAUDE.md)
Do not modify core Vortex scripts under scripts/vortex
Files:
scripts/vortex/deploy-artifact.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
scripts/vortex/deploy-artifact.sh
Outdated
Comment on lines
100
to
104
| cd "${VORTEX_DEPLOY_ARTIFACT_SRC}" | ||
| git config core.filemode | ||
| git config core.filemode true | ||
| cd - | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
set -e will abort if core.filemode is unset
git config core.filemode exits with 1 when the key isn’t set; with set -e that terminates the deployment. Also no need to cd/cd -; prefer git -C and scope to --local.
Apply:
-cd "${VORTEX_DEPLOY_ARTIFACT_SRC}"
-git config core.filemode
-git config core.filemode true
-cd -
+git -C "${VORTEX_DEPLOY_ARTIFACT_SRC}" config --get core.filemode >/dev/null 2>&1 || true
+git -C "${VORTEX_DEPLOY_ARTIFACT_SRC}" config --local core.filemode trueCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/vortex/deploy-artifact.sh around lines 100 to 104, calling `git
config core.filemode` without a value can exit with status 1 under set -e and
abort the script; also avoid unnecessary cd/cd -; update to run git with -C
against "${VORTEX_DEPLOY_ARTIFACT_SRC}" and set the value locally, e.g. replace
the two config lines and the cd/cd - with a single command that runs `git -C
"${VORTEX_DEPLOY_ARTIFACT_SRC}" config --local core.filemode true` so the key is
set (no failing status) and no directory changes are required.
🧹 Nitpick (assertive)
Add a short log for traceability
Helps explain why we touch repo config during deploy.
+info "Ensuring Git tracks exec bit in artifact source (core.filemode=true)."
git -C "${VORTEX_DEPLOY_ARTIFACT_SRC}" config --get core.filemode >/dev/null 2>&1 || true
git -C "${VORTEX_DEPLOY_ARTIFACT_SRC}" config --local core.filemode trueCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/vortex/deploy-artifact.sh around lines 100 to 104, the script changes
git config core.filemode without any trace; add a short trace log immediately
before the git config calls that states the repository directory and why
filemode is being set (e.g., "Temporarily disabling filemode checks in
${VORTEX_DEPLOY_ARTIFACT_SRC} to avoid permission-only changes during deploy"),
then run the two git config commands as-is; keep the log simple, use the same
logging mechanism used elsewhere in the script (echo/printf or logger) and
ensure it prints to stdout/stderr for deploy logs.
💡 Verification agent
❓ Verification inconclusive
Do not modify core Vortex scripts under scripts/vortex/
This change violates the repo guideline. Please move this logic to the calling CI step/wrapper before invoking this script (or contribute it to git-artifact if broadly needed).
Revert changes to core Vortex script; do not modify scripts/vortex
File: scripts/vortex/deploy-artifact.sh — lines 100-104
cd "${VORTEX_DEPLOY_ARTIFACT_SRC}"
git config core.filemode
git config core.filemode true
cd -
This violates the repo guideline against editing scripts/vortex. Move this logic to the calling CI step/wrapper (or contribute it to git-artifact if broadly needed).
🤖 Prompt for AI Agents
scripts/vortex/deploy-artifact.sh lines 100-104 currently add git config
commands inside the core Vortex script which violates the repo guideline against
modifying scripts/vortex; remove those three lines from this file and instead
add equivalent git configuration in the CI job or wrapper that invokes this
script (e.g., run git config core.filemode true in the pipeline step prior to
calling deploy-artifact.sh), or contribute the functionality to the shared
git-artifact tool if it should be broadly available.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2010 +/- ##
========================================
Coverage 78.02% 78.02%
========================================
Files 89 89
Lines 5542 5542
Branches 44 44
========================================
Hits 4324 4324
Misses 1218 1218 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-test-deploy.yml(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**
📄 CodeRabbit inference engine (CLAUDE.md)
Store GitHub Actions workflows under .github/workflows when using GitHub Actions for CI/CD
Files:
.github/workflows/build-test-deploy.yml
🪛 GitHub Check: vortex-test-actions
.github/workflows/build-test-deploy.yml
[failure] 332-332:
332:1 [empty-lines] too many blank lines (2 > 1)
[failure] 326-326:
326:1 [empty-lines] too many blank lines (2 > 1)
[failure] 342-342:
342:1 [empty-lines] too many blank lines (2 > 1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-docs
Comment on lines
261
to
267
| - name: Check perms | ||
| run: | | ||
| docker compose exec $(env | cut -f1 -d= | sed 's/^/-e /') -T cli bash -c " \ | ||
| if [ -n \"${PACKAGE_TOKEN:-}\" ]; then export COMPOSER_AUTH='{\"github-oauth\": {\"github.com\": \"${PACKAGE_TOKEN-}\"}}'; fi && \ | ||
| COMPOSER_MEMORY_LIMIT=-1 composer --ansi install --prefer-dist" | ||
| docker compose exec $(env | cut -f1 -d= | sed 's/^/-e /') -T cli bash -c "yarn install --frozen-lockfile" | ||
| - name: Validate Composer configuration is normalized | ||
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: docker compose exec -T cli composer normalize --dry-run | ||
| continue-on-error: ${{ vars.VORTEX_CI_COMPOSER_NORMALIZE_IGNORE_FAILURE == '1' }} | ||
|
|
||
| #;< TOOL_PHPCS | ||
| - name: Lint code with PHPCS | ||
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: docker compose exec -T cli vendor/bin/phpcs | ||
| continue-on-error: ${{ vars.VORTEX_CI_PHPCS_IGNORE_FAILURE == '1' }} | ||
| #;> TOOL_PHPCS | ||
|
|
||
| #;< TOOL_PHPSTAN | ||
| - name: Lint code with PHPStan | ||
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: docker compose exec -T cli vendor/bin/phpstan | ||
| continue-on-error: ${{ vars.VORTEX_CI_PHPSTAN_IGNORE_FAILURE == '1' }} | ||
| #;> TOOL_PHPSTAN | ||
|
|
||
| #;< TOOL_RECTOR | ||
| - name: Lint code with Rector | ||
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: docker compose exec -T cli vendor/bin/rector --clear-cache --dry-run | ||
| continue-on-error: ${{ vars.VORTEX_CI_RECTOR_IGNORE_FAILURE == '1' }} | ||
| #;> TOOL_RECTOR | ||
|
|
||
| #;< TOOL_PHPMD | ||
| - name: Lint code with PHPMD | ||
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: docker compose exec -T cli vendor/bin/phpmd . text phpmd.xml | ||
| continue-on-error: ${{ vars.VORTEX_CI_PHPMD_IGNORE_FAILURE == '1' }} | ||
| #;> TOOL_PHPMD | ||
|
|
||
| - name: Lint code with Twig CS Fixer | ||
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: docker compose exec -T cli vendor/bin/twig-cs-fixer | ||
| continue-on-error: ${{ vars.VORTEX_CI_TWIG_CS_FIXER_IGNORE_FAILURE == '1' }} | ||
|
|
||
| #;< TOOL_BEHAT | ||
| - name: Lint code with Gherkin Lint | ||
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: docker compose exec -T cli vendor/bin/gherkinlint lint tests/behat/features | ||
| continue-on-error: ${{ vars.VORTEX_CI_GHERKIN_LINT_IGNORE_FAILURE == '1' }} | ||
| #;> TOOL_BEHAT | ||
|
|
||
| - name: Lint module code with NodeJS linters | ||
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: docker compose exec -T cli bash -c "yarn run lint" | ||
| continue-on-error: ${{ vars.VORTEX_CI_NODEJS_LINT_IGNORE_FAILURE == '1' }} | ||
|
|
||
| #;< DRUPAL_THEME | ||
| - name: Lint theme code with NodeJS linters | ||
| if: ${{ (matrix.instance == 0 || strategy.job-total == 1) && vars.VORTEX_FRONTEND_BUILD_SKIP != '1' }} | ||
| run: docker compose exec -T cli bash -c "yarn --cwd=\${WEBROOT}/themes/custom/\${DRUPAL_THEME} run lint" | ||
| continue-on-error: ${{ vars.VORTEX_CI_NODEJS_LINT_IGNORE_FAILURE == '1' }} | ||
| #;> DRUPAL_THEME | ||
|
|
||
| - name: Provision site | ||
| run: | | ||
| if [ -f .data/db.sql ]; then | ||
| docker compose exec cli mkdir -p .data | ||
| docker compose cp -L .data/db.sql cli:/app/.data/db.sql | ||
| fi | ||
| docker compose exec $(env | cut -f1 -d= | sed 's/^/-e /') -T cli ./scripts/vortex/provision.sh | ||
| timeout-minutes: 30 | ||
|
|
||
| #;< TOOL_PHPUNIT | ||
| - name: Test with PHPUnit | ||
| run: docker compose exec -T cli vendor/bin/phpunit | ||
| continue-on-error: ${{ vars.VORTEX_CI_PHPUNIT_IGNORE_FAILURE == '1' }} | ||
| #;> TOOL_PHPUNIT | ||
|
|
||
| #;< TOOL_BEHAT | ||
| - name: Test with Behat | ||
| run: | | ||
| # shellcheck disable=SC2170 | ||
| if [ ${{ strategy.job-total }} -gt 1 ]; then export VORTEX_CI_BEHAT_PROFILE="${VORTEX_CI_BEHAT_PROFILE:-p${{ strategy.job-index }}}"; fi | ||
| echo "Running with ${VORTEX_CI_BEHAT_PROFILE:-default} profile" | ||
| docker compose exec -T cli php -d memory_limit=-1 vendor/bin/behat --colors --strict --profile="${VORTEX_CI_BEHAT_PROFILE:-default}" || \ | ||
| docker compose exec -T cli php -d memory_limit=-1 vendor/bin/behat --colors --strict --rerun --profile="${VORTEX_CI_BEHAT_PROFILE:-default}" | ||
| env: | ||
| VORTEX_CI_BEHAT_PROFILE: ${{ vars.VORTEX_CI_BEHAT_PROFILE }} | ||
| continue-on-error: ${{ vars.VORTEX_CI_BEHAT_IGNORE_FAILURE == '1' }} | ||
| timeout-minutes: 30 | ||
| #;> TOOL_BEHAT | ||
|
|
||
| - name: Process test logs and artifacts | ||
| if: always() | ||
| run: | | ||
| mkdir -p ".logs" | ||
| if docker compose ps --services --filter "status=running" | grep -q cli && docker compose exec cli test -d /app/.logs; then | ||
| docker compose cp cli:/app/.logs/. ".logs/" | ||
| fi | ||
| - name: Upload test artifacts | ||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 | ||
| if: always() | ||
| with: | ||
| name: test-artifacts-${{ matrix.instance }} | ||
| path: .logs | ||
| include-hidden-files: true | ||
| if-no-files-found: error | ||
|
|
||
| - name: Upload coverage report to Codecov | ||
| uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 # v5 | ||
| if: ${{ env.CODECOV_TOKEN != '' }} | ||
| with: | ||
| directory: .logs/coverage | ||
| fail_ci_if_error: true | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
| env: | ||
| CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} | ||
| umask | ||
| echo "In container" | ||
| docker compose exec cli bash -c "ls -al /app/hooks/library" | ||
| echo "On host" | ||
| ls -al /tmp/workspace/code/hooks/library |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve permission diagnostics (optional): show octal modes and git index modes.
This gives unambiguous exec-bit visibility beyond ls -l.
You can extend both checks:
umask
+ echo "Octal modes (container):"
+ docker compose exec -T cli bash -lc 'command -v stat >/dev/null && stat -c "%A %a %n" /app/hooks/library/* || true'
+ echo "Git index modes (container):"
+ docker compose exec -T cli bash -lc 'cd /app && git ls-files -s hooks/library 2>/dev/null || true'
echo "In container"And in deploy:
umask
- if [ -d /tmp/workspace/code/hooks/library ]; then ls -al /tmp/workspace/code/hooks/library; else echo "hooks/library not found"; fi
+ if [ -d /tmp/workspace/code/hooks/library ]; then
+ ls -al /tmp/workspace/code/hooks/library
+ command -v stat >/dev/null && stat -c "%A %a %n" /tmp/workspace/code/hooks/library/* || true
+ (cd /tmp/workspace/code && git ls-files -s hooks/library 2>/dev/null) || true
+ else
+ echo "hooks/library not found"
+ fiAlso applies to: 328-331
Gate the "Check perms" step to instance 0 and artifact deploy; otherwise matrix instance 1 will fail.
This step assumes exported code exists at /tmp/workspace/code, but export runs only on instance 0. On instance 1, ls will fail and can break the job.
Apply:
- - name: Check perms
- run: |
+ - name: Check perms
+ if: ${{ matrix.instance == 0 && contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }}
+ run: |
umask
echo "In container"
- docker compose exec cli bash -c "ls -al /app/hooks/library"
+ docker compose exec cli bash -lc 'test -d /app/hooks/library && ls -al /app/hooks/library || echo "hooks/library not found"'
echo "On host"
- ls -al /tmp/workspace/code/hooks/library
+ if [ -d /tmp/workspace/code/hooks/library ]; then ls -al /tmp/workspace/code/hooks/library; else echo "hooks/library not found"; 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.
Suggested change
| - name: Check perms | |
| run: | | |
| docker compose exec $(env | cut -f1 -d= | sed 's/^/-e /') -T cli bash -c " \ | |
| if [ -n \"${PACKAGE_TOKEN:-}\" ]; then export COMPOSER_AUTH='{\"github-oauth\": {\"github.com\": \"${PACKAGE_TOKEN-}\"}}'; fi && \ | |
| COMPOSER_MEMORY_LIMIT=-1 composer --ansi install --prefer-dist" | |
| docker compose exec $(env | cut -f1 -d= | sed 's/^/-e /') -T cli bash -c "yarn install --frozen-lockfile" | |
| - name: Validate Composer configuration is normalized | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: docker compose exec -T cli composer normalize --dry-run | |
| continue-on-error: ${{ vars.VORTEX_CI_COMPOSER_NORMALIZE_IGNORE_FAILURE == '1' }} | |
| #;< TOOL_PHPCS | |
| - name: Lint code with PHPCS | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: docker compose exec -T cli vendor/bin/phpcs | |
| continue-on-error: ${{ vars.VORTEX_CI_PHPCS_IGNORE_FAILURE == '1' }} | |
| #;> TOOL_PHPCS | |
| #;< TOOL_PHPSTAN | |
| - name: Lint code with PHPStan | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: docker compose exec -T cli vendor/bin/phpstan | |
| continue-on-error: ${{ vars.VORTEX_CI_PHPSTAN_IGNORE_FAILURE == '1' }} | |
| #;> TOOL_PHPSTAN | |
| #;< TOOL_RECTOR | |
| - name: Lint code with Rector | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: docker compose exec -T cli vendor/bin/rector --clear-cache --dry-run | |
| continue-on-error: ${{ vars.VORTEX_CI_RECTOR_IGNORE_FAILURE == '1' }} | |
| #;> TOOL_RECTOR | |
| #;< TOOL_PHPMD | |
| - name: Lint code with PHPMD | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: docker compose exec -T cli vendor/bin/phpmd . text phpmd.xml | |
| continue-on-error: ${{ vars.VORTEX_CI_PHPMD_IGNORE_FAILURE == '1' }} | |
| #;> TOOL_PHPMD | |
| - name: Lint code with Twig CS Fixer | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: docker compose exec -T cli vendor/bin/twig-cs-fixer | |
| continue-on-error: ${{ vars.VORTEX_CI_TWIG_CS_FIXER_IGNORE_FAILURE == '1' }} | |
| #;< TOOL_BEHAT | |
| - name: Lint code with Gherkin Lint | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: docker compose exec -T cli vendor/bin/gherkinlint lint tests/behat/features | |
| continue-on-error: ${{ vars.VORTEX_CI_GHERKIN_LINT_IGNORE_FAILURE == '1' }} | |
| #;> TOOL_BEHAT | |
| - name: Lint module code with NodeJS linters | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: docker compose exec -T cli bash -c "yarn run lint" | |
| continue-on-error: ${{ vars.VORTEX_CI_NODEJS_LINT_IGNORE_FAILURE == '1' }} | |
| #;< DRUPAL_THEME | |
| - name: Lint theme code with NodeJS linters | |
| if: ${{ (matrix.instance == 0 || strategy.job-total == 1) && vars.VORTEX_FRONTEND_BUILD_SKIP != '1' }} | |
| run: docker compose exec -T cli bash -c "yarn --cwd=\${WEBROOT}/themes/custom/\${DRUPAL_THEME} run lint" | |
| continue-on-error: ${{ vars.VORTEX_CI_NODEJS_LINT_IGNORE_FAILURE == '1' }} | |
| #;> DRUPAL_THEME | |
| - name: Provision site | |
| run: | | |
| if [ -f .data/db.sql ]; then | |
| docker compose exec cli mkdir -p .data | |
| docker compose cp -L .data/db.sql cli:/app/.data/db.sql | |
| fi | |
| docker compose exec $(env | cut -f1 -d= | sed 's/^/-e /') -T cli ./scripts/vortex/provision.sh | |
| timeout-minutes: 30 | |
| #;< TOOL_PHPUNIT | |
| - name: Test with PHPUnit | |
| run: docker compose exec -T cli vendor/bin/phpunit | |
| continue-on-error: ${{ vars.VORTEX_CI_PHPUNIT_IGNORE_FAILURE == '1' }} | |
| #;> TOOL_PHPUNIT | |
| #;< TOOL_BEHAT | |
| - name: Test with Behat | |
| run: | | |
| # shellcheck disable=SC2170 | |
| if [ ${{ strategy.job-total }} -gt 1 ]; then export VORTEX_CI_BEHAT_PROFILE="${VORTEX_CI_BEHAT_PROFILE:-p${{ strategy.job-index }}}"; fi | |
| echo "Running with ${VORTEX_CI_BEHAT_PROFILE:-default} profile" | |
| docker compose exec -T cli php -d memory_limit=-1 vendor/bin/behat --colors --strict --profile="${VORTEX_CI_BEHAT_PROFILE:-default}" || \ | |
| docker compose exec -T cli php -d memory_limit=-1 vendor/bin/behat --colors --strict --rerun --profile="${VORTEX_CI_BEHAT_PROFILE:-default}" | |
| env: | |
| VORTEX_CI_BEHAT_PROFILE: ${{ vars.VORTEX_CI_BEHAT_PROFILE }} | |
| continue-on-error: ${{ vars.VORTEX_CI_BEHAT_IGNORE_FAILURE == '1' }} | |
| timeout-minutes: 30 | |
| #;> TOOL_BEHAT | |
| - name: Process test logs and artifacts | |
| if: always() | |
| run: | | |
| mkdir -p ".logs" | |
| if docker compose ps --services --filter "status=running" | grep -q cli && docker compose exec cli test -d /app/.logs; then | |
| docker compose cp cli:/app/.logs/. ".logs/" | |
| fi | |
| - name: Upload test artifacts | |
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 | |
| if: always() | |
| with: | |
| name: test-artifacts-${{ matrix.instance }} | |
| path: .logs | |
| include-hidden-files: true | |
| if-no-files-found: error | |
| - name: Upload coverage report to Codecov | |
| uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 # v5 | |
| if: ${{ env.CODECOV_TOKEN != '' }} | |
| with: | |
| directory: .logs/coverage | |
| fail_ci_if_error: true | |
| token: ${{ secrets.CODECOV_TOKEN }} | |
| env: | |
| CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} | |
| umask | |
| echo "In container" | |
| docker compose exec cli bash -c "ls -al /app/hooks/library" | |
| echo "On host" | |
| ls -al /tmp/workspace/code/hooks/library | |
| - name: Check perms | |
| if: ${{ matrix.instance == 0 && contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }} | |
| run: | | |
| umask | |
| echo "In container" | |
| docker compose exec cli bash -lc 'test -d /app/hooks/library && ls -al /app/hooks/library || echo "hooks/library not found"' | |
| echo "On host" | |
| if [ -d /tmp/workspace/code/hooks/library ]; then ls -al /tmp/workspace/code/hooks/library; else echo "hooks/library not found"; fi |
Comment on lines
326
to
331
|
|
||
| - name: Check perms after download | ||
| run: | | ||
| umask | ||
| ls -al /tmp/workspace/code/hooks/library | ||
There was a problem hiding this comment.
Make "Check perms after download" conditional and resilient when artifact or path is absent.
Without a guard, this runs even when artifact deploy is disabled; ls will fail if the directory is missing.
Apply:
- - name: Check perms after download
- run: |
+ - name: Check perms after download
+ if: ${{ contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }}
+ run: |
umask
- ls -al /tmp/workspace/code/hooks/library
+ if [ -d /tmp/workspace/code/hooks/library ]; then ls -al /tmp/workspace/code/hooks/library; else echo "hooks/library not found"; 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.
Suggested change
| - name: Check perms after download | |
| run: | | |
| umask | |
| ls -al /tmp/workspace/code/hooks/library | |
| - name: Check perms after download | |
| if: ${{ contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }} | |
| run: | | |
| umask | |
| if [ -d /tmp/workspace/code/hooks/library ]; then ls -al /tmp/workspace/code/hooks/library; else echo "hooks/library not found"; fi |
🧰 Tools
🪛 GitHub Check: vortex-test-actions
[failure] 326-326:
326:1 [empty-lines] too many blank lines (2 > 1)
🤖 Prompt for AI Agents
In .github/workflows/build-test-deploy.yml around lines 326-331, the "Check
perms after download" step unconditionally runs ls and fails when the artifact
or directory is missing; make the step conditional and resilient by either
adding an if: expression so it only runs when artifact download/deploy is
enabled or succeeded (e.g. reference the download step's output or an ENV flag),
and change the run block to guard the commands with a directory check (e.g. if [
-d /tmp/workspace/code/hooks/library ]; then umask; ls -al
/tmp/workspace/code/hooks/library; fi) so it safely no-ops when the path is
absent.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/build-test-deploy.yml (3)
260-266: Harden "Check perms": guard paths; use non-interactive exec; optionally limit to instance 0.Prevents failures if hooks/library is absent and avoids TTY issues; cuts duplicate logs on matrix runs.
- - name: Check perms - run: | + - name: Check perms + if: ${{ matrix.instance == 0 }} + run: | umask echo "In container" - docker compose exec cli bash -c "ls -al /app/hooks/library" + docker compose exec -T cli bash -lc 'test -d /app/hooks/library && ls -al /app/hooks/library || echo "hooks/library not found"' echo "On host" - ls -al /tmp/workspace/code/hooks/library + if [ -d /tmp/workspace/code/hooks/library ]; then + ls -al /tmp/workspace/code/hooks/library + else + echo "hooks/library not found" + fiOptionally extend diagnostics (octal + git index) for exec-bit clarity:
echo "On host" if [ -d /tmp/workspace/code/hooks/library ]; then ls -al /tmp/workspace/code/hooks/library + command -v stat >/dev/null && stat -c "%A %a %n" /tmp/workspace/code/hooks/library/* || true + (cd /tmp/workspace/code && git ls-files -s hooks/library 2>/dev/null) || true else echo "hooks/library not found" fi
324-328: Make "Check perms after download" resilient to missing artifact/path.Avoid step failure when directory is absent; add guarded checks and richer diagnostics.
- - name: Check perms after download - run: | - umask - ls -al /tmp/workspace/code/hooks/library + - name: Check perms after download + run: | + umask + if [ -d /tmp/workspace/code/hooks/library ]; then + ls -al /tmp/workspace/code/hooks/library + command -v stat >/dev/null && stat -c "%A %a %n" /tmp/workspace/code/hooks/library/* || true + (cd /tmp/workspace/code && git ls-files -s hooks/library 2>/dev/null) || true + else + echo "hooks/library not found" + fi
323-329: Fix workflow lint: remove extra blank lines.CI flags “too many blank lines” at Lines 323 and 329.
with: name: code-artifact path: "/tmp/workspace/code" - - - - name: Check perms after download + - name: Check perms after download run: | umask ls -al /tmp/workspace/code/hooks/library -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-test-deploy.yml(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**
📄 CodeRabbit inference engine (CLAUDE.md)
Store GitHub Actions workflows under .github/workflows when using GitHub Actions for CI/CD
Files:
.github/workflows/build-test-deploy.yml
🪛 GitHub Check: vortex-test-actions
.github/workflows/build-test-deploy.yml
[failure] 329-329:
329:1 [empty-lines] too many blank lines (2 > 1)
[failure] 323-323:
323:1 [empty-lines] too many blank lines (2 > 1)
[failure] 339-339:
339:1 [empty-lines] too many blank lines (2 > 1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.2)
AlexSkrypnyk
force-pushed
the
feature/fix-artifact-exec-bit
branch
from
a383322 to
84f54e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.github/workflows/build-test-deploy.yml (2)
249-257: Gate and harden “Check perms”; add richer diagnostics.
- Gate to instance 0 and only when artifact deploy is in play to avoid false failures on the other matrix job.
- Don’t fail if the directory isn’t present; guard with
test -d.- Add octal modes and git index modes for unambiguous exec-bit visibility. This mirrors prior feedback.
- - name: Check perms - if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} + - name: Check perms + if: ${{ matrix.instance == 0 && contains(env.VORTEX_DEPLOY_TYPES, 'artifact') }} run: | umask - echo "In container" - docker compose exec cli bash -c "ls -al /app/hooks/library" - echo "On host" - ls -al /tmp/workspace/code/hooks/library + echo "In container" + docker compose exec -T cli bash -lc 'if [ -d /app/hooks/library ]; then \ + ls -al /app/hooks/library; \ + command -v stat >/dev/null && stat -c "%A %a %n" /app/hooks/library/* || true; \ + (cd /app && git ls-files -s hooks/library 2>/dev/null) || true; \ + else echo "hooks/library not found"; fi' + echo "On host" + if [ -d /tmp/workspace/code/hooks/library ]; then + ls -al /tmp/workspace/code/hooks/library + command -v stat >/dev/null && stat -c "%A %a %n" /tmp/workspace/code/hooks/library/* || true + (cd /tmp/workspace/code && git ls-files -s hooks/library 2>/dev/null) || true + else + echo "hooks/library not found" + fi
319-323: Guard and enrich “Check perms after download”.
- Make the step conditional on the download success, and don’t fail when the path is absent.
- Add octal and git-index visibility for clarity (same as earlier comment).
- - name: Check perms after download - run: | - umask - ls -al /tmp/workspace/code/hooks/library + - name: Check perms after download + if: ${{ steps.download.outcome == 'success' }} + run: | + umask + if [ -d /tmp/workspace/code/hooks/library ]; then + ls -al /tmp/workspace/code/hooks/library + command -v stat >/dev/null && stat -c "%A %a %n" /tmp/workspace/code/hooks/library/* || true + (cd /tmp/workspace/code && git ls-files -s hooks/library 2>/dev/null) || true + else + echo "hooks/library not found" + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-test-deploy.yml(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**
📄 CodeRabbit inference engine (CLAUDE.md)
Store GitHub Actions workflows under .github/workflows when using GitHub Actions for CI/CD
Files:
.github/workflows/build-test-deploy.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-docs
🔇 Additional comments (1)
.github/workflows/build-test-deploy.yml (1)
259-263: Artifact upload is correctly scoped and paths look right.Pinned action, explicit path to
code.tar, and retention are all good.
Comment on lines
242
to
261
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: | | ||
| mkdir -p "/tmp/workspace/code" | ||
| mkdir -p "/tmp/workspace/code" "/tmp/artifacts" | ||
| docker compose cp -L cli:"/app/." "/tmp/workspace/code" | ||
| du -sh "/tmp/workspace/code" | ||
| - name: Install development dependencies | ||
| run: | | ||
| docker compose exec $(env | cut -f1 -d= | sed 's/^/-e /') -T cli bash -c " \ | ||
| if [ -n \"${PACKAGE_TOKEN:-}\" ]; then export COMPOSER_AUTH='{\"github-oauth\": {\"github.com\": \"${PACKAGE_TOKEN-}\"}}'; fi && \ | ||
| COMPOSER_MEMORY_LIMIT=-1 composer --ansi install --prefer-dist" | ||
| docker compose exec $(env | cut -f1 -d= | sed 's/^/-e /') -T cli bash -c "yarn install --frozen-lockfile" | ||
| - name: Validate Composer configuration is normalized | ||
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: docker compose exec -T cli composer normalize --dry-run | ||
| continue-on-error: ${{ vars.VORTEX_CI_COMPOSER_NORMALIZE_IGNORE_FAILURE == '1' }} | ||
|
|
||
| #;< TOOL_PHPCS | ||
| - name: Lint code with PHPCS | ||
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | ||
| run: docker compose exec -T cli vendor/bin/phpcs | ||
| continue-on-error: ${{ vars.VORTEX_CI_PHPCS_IGNORE_FAILURE == '1' }} | ||
| #;> TOOL_PHPCS | ||
| tar -C /tmp/workspace -cpf /tmp/artifacts/code.tar code | ||
| du -sh "/tmp/artifacts" | ||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Preserving exec bits via tar looks good; confirm symlink dereference is intentional.
- Using
tar -cpfis correct to keep modes. - You’re copying with
docker compose cp -L …which dereferences symlinks; this will turn symlinks into regular files in the artifact. If the downstream expects symlinks, drop-L. If you intended to flatten symlinks for deploy, keep as-is.
Optional hardening:
- run: |
- mkdir -p "/tmp/workspace/code" "/tmp/artifacts"
+ run: |
+ set -euo pipefail
+ mkdir -p "/tmp/workspace/code" "/tmp/artifacts"
docker compose cp -L cli:"/app/." "/tmp/workspace/code"
tar -C /tmp/workspace -cpf /tmp/artifacts/code.tar code
du -sh "/tmp/artifacts"📝 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.
Suggested change
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: | | |
| mkdir -p "/tmp/workspace/code" | |
| mkdir -p "/tmp/workspace/code" "/tmp/artifacts" | |
| docker compose cp -L cli:"/app/." "/tmp/workspace/code" | |
| du -sh "/tmp/workspace/code" | |
| - name: Install development dependencies | |
| run: | | |
| docker compose exec $(env | cut -f1 -d= | sed 's/^/-e /') -T cli bash -c " \ | |
| if [ -n \"${PACKAGE_TOKEN:-}\" ]; then export COMPOSER_AUTH='{\"github-oauth\": {\"github.com\": \"${PACKAGE_TOKEN-}\"}}'; fi && \ | |
| COMPOSER_MEMORY_LIMIT=-1 composer --ansi install --prefer-dist" | |
| docker compose exec $(env | cut -f1 -d= | sed 's/^/-e /') -T cli bash -c "yarn install --frozen-lockfile" | |
| - name: Validate Composer configuration is normalized | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: docker compose exec -T cli composer normalize --dry-run | |
| continue-on-error: ${{ vars.VORTEX_CI_COMPOSER_NORMALIZE_IGNORE_FAILURE == '1' }} | |
| #;< TOOL_PHPCS | |
| - name: Lint code with PHPCS | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: docker compose exec -T cli vendor/bin/phpcs | |
| continue-on-error: ${{ vars.VORTEX_CI_PHPCS_IGNORE_FAILURE == '1' }} | |
| #;> TOOL_PHPCS | |
| tar -C /tmp/workspace -cpf /tmp/artifacts/code.tar code | |
| du -sh "/tmp/artifacts" | |
| if: ${{ matrix.instance == 0 || strategy.job-total == 1 }} | |
| run: | | |
| set -euo pipefail | |
| mkdir -p "/tmp/workspace/code" "/tmp/artifacts" | |
| docker compose cp -L cli:"/app/." "/tmp/workspace/code" | |
| tar -C /tmp/workspace -cpf /tmp/artifacts/code.tar code | |
| du -sh "/tmp/artifacts" |
🤖 Prompt for AI Agents
.github/workflows/build-test-deploy.yml lines 242-248: the workflow currently
uses `docker compose cp -L` which dereferences symlinks and will convert them
into regular files in the tar artifact; if downstream consumers require symlinks
preserved, remove the `-L` flag from the `docker compose cp` invocation so
symlinks are copied as links, otherwise keep `-L` if flattening was intentional;
update the workflow to reflect the chosen behavior and add a one-line comment
explaining why symlinks are being dereferenced or preserved for future readers.
Comment on lines
312
to
444
| path: "/tmp/artifacts" | ||
|
|
||
| - name: Unpack downloaded exported codebase | ||
| run: | | ||
| mkdir -p /tmp/workspace | ||
| tar -xpf /tmp/artifacts/code.tar -C /tmp/workspace | ||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make unpack conditional on successful artifact download.
If the artifact is missing or renamed, tar -xpf will fail. Add an id to the download step and gate unpack on its outcome.
- - name: Download exported codebase as an artifact
- uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5
+ - name: Download exported codebase as an artifact
+ id: download
+ uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5
with:
name: code-artifact
path: "/tmp/artifacts"
- - name: Unpack downloaded exported codebase
- run: |
+ - name: Unpack downloaded exported codebase
+ if: ${{ steps.download.outcome == 'success' }}
+ run: |
mkdir -p /tmp/workspace
tar -xpf /tmp/artifacts/code.tar -C /tmp/workspace📝 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.
Suggested change
| path: "/tmp/artifacts" | |
| - name: Unpack downloaded exported codebase | |
| run: | | |
| mkdir -p /tmp/workspace | |
| tar -xpf /tmp/artifacts/code.tar -C /tmp/workspace | |
| - name: Download exported codebase as an artifact | |
| id: download | |
| uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5 | |
| with: | |
| name: code-artifact | |
| path: "/tmp/artifacts" | |
| - name: Unpack downloaded exported codebase | |
| if: ${{ steps.download.outcome == 'success' }} | |
| run: | | |
| mkdir -p /tmp/workspace | |
| tar -xpf /tmp/artifacts/code.tar -C /tmp/workspace |
🤖 Prompt for AI Agents
.github/workflows/build-test-deploy.yml around lines 312 to 318: the unpack step
always runs and will fail if the artifact download step didn't produce
/tmp/artifacts/code.tar; add an id to the artifact download step (e.g., id:
download_artifact) and change the unpack step to run only when that step
succeeded by adding a conditional like if: steps.download_artifact.outcome ==
'success' so the tar extraction is gated on a successful download.
84f54e5 to
dffc1cf
Compare
AlexSkrypnyk
changed the title
AlexSkrypnyk
force-pushed
the
feature/fix-artifact-exec-bit
branch
from
dffc1cf to
dcb10ce
Compare
AlexSkrypnyk
added
the
Confirmed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit