Skip to content

INTERNAL Fix CI pipeline: run lint and phpstan without the private workspace repo - #341

Merged
TLabutis merged 1 commit into
feature/react-admin-settingsfrom
INTERNAL/ci-drop-private-workspace
Jul 29, 2026
Merged

INTERNAL Fix CI pipeline: run lint and phpstan without the private workspace repo#341
TLabutis merged 1 commit into
feature/react-admin-settingsfrom
INTERNAL/ci-drop-private-workspace

Conversation

@TLabutis

Copy link
Copy Markdown
Contributor

Self-Checks

  • I have performed a self-review of my code.
  • I have updated/added necessary technical documentation in the README file.

JIRA task link

SL-382

Summary

The CI/CD pipeline has been dead since around May 2026, so lint and phpstan have not run on this module for months while the admin redesign landed. This makes them run again without depending on the private ps-module-workspace repository, and fixes the two findings that turning them back on exposed.

QA Checklist Labels

  • Bug fix?
  • New feature?
  • Improvement?
  • Technical debt?
  • Reusable?
  • Covered by tests?

QA Checklist

Blocks the 2.1.0 release together with #330 (React bundle built in CI) and #339 (Composer advisory release-build fix).

Additional Context

Root cause

Every run died in the very first real step:

Identity added: (stdin) (marijus@Marijuss-MacBook-Pro.local)
Cloning into '.../workspace'...
ERROR: Repository not found.
##[error]Process completed with exit code 128.

ssh-add succeeds, so the secret PS_MODULE_WORKSPACE_SAFERPAY_PRIVATE_KEY is intact. The key is a personal key (note the identity comment) and the account it belongs to is no longer on the collaborator list of Invertus/ps-module-workspace, so GitHub authenticates the key and then reports the private repo as not found. Last green run was 2025-11-21; failures start 2026-05-08.

Restoring a credential would mean either a read-only deploy key on ps-module-workspace (needs admin on that repo) or another personal token with the same expiry and offboarding fragility. So the dependency is removed instead.

What changed

lint and phpstan need the PrestaShop sources on disk and the matching PHP version. They do not need a running shop, a database or a healthcheck. The public prestashop/prestashop:1.7.8-7.4 image already ships the sources, so it is now started idle with the checked-out module mounted in place:

docker run -d --name ps-lint \
  -v ${{ github.workspace }}:/var/www/html/modules/saferpayofficial \
  --entrypoint tail prestashop/prestashop:${{ matrix.PS }} -f /dev/null

That drops seven steps: the workspace clone, docker compose up, the healthcheck, the swap-space allocation, and the in-container git clone plus branch checkout of this module. As a side effect the job now tests the actual checked-out code instead of re-cloning the branch from GitHub inside the container, which also means it works for pull requests from forks. git and composer are installed into the container since the base image ships neither.

fail-fast: false added on the matrix, so a red lint leg no longer cancels phpstan.

Findings this exposed, both fixed here

  • src/Service/SaferPayOrderStatusService.php - three unused imports (Customer, PendingNotification, ControllerName), each referenced nowhere but its own use line. php-cs-fixer error 8.
  • controllers/admin/AdminSaferPayOfficialSettingsController.php - postProcess() had bare return; statements and no return at the end, while the inherited signature is bool|ObjectModel. Now returns false on each rejected request and true after dispatching. Behaviour is unchanged: ajaxResponse() ends in sendJsonResponse() which calls die(), so those returns are unreachable defensive paths.

Verification

Ran the exact steps of the new workflow locally against prestashop/prestashop:1.7.8-7.4 on a clean clone of the release branch.

Before the fixes, both suites reported real failures, which is the proof they had genuinely stopped running:

make ci-lint     -> Error 8, 1) src/Service/SaferPayOrderStatusService.php
make ci-phpstan  -> [ERROR] Found 1 error
                    postProcess() should return bool|ObjectModel but return statement is missing

After:

make ci-lint                             -> exit=0, Checked all files
make ci-phpstan ps_version_tag=1.7.8-7.4 -> exit=0, Detected PS version 1.7.8.11, [OK] No errors (267 files)

composer install and the on-the-fly composer require phpstan/phpstan both succeeded, which incidentally re-confirms the advisory policy fix from #339 works during dependency resolution.

Follow-up worth considering

Other Invertus modules use the same ps-module-workspace harness with their own personal-key secrets, so they will break the same way whenever those accounts are offboarded. Either move them to read-only deploy keys, or apply this self-contained pattern.

Frontend Changes

None.

The pipeline had been failing at "Set up workspace" since around May 2026
with "ERROR: Repository not found" while cloning Invertus/ps-module-workspace.
The SSH key in PS_MODULE_WORKSPACE_SAFERPAY_PRIVATE_KEY loads fine but
belongs to a personal account that no longer has access to that repository,
so lint and phpstan have not run on this module for months.

lint and phpstan need the PrestaShop sources and the matching PHP version,
not a running shop, so the public prestashop/prestashop image is now started
idle with the checked-out module mounted in place. No private repository, no
SSH secret, no database and no healthcheck.

Also fixes the two findings this exposed: three unused imports in
SaferPayOrderStatusService and the missing return in the settings
controller postProcess().

Sets fail-fast to false so one red matrix leg no longer hides the other.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@TLabutis
TLabutis merged commit eb84340 into feature/react-admin-settings Jul 29, 2026
3 checks passed
@TLabutis
TLabutis deleted the INTERNAL/ci-drop-private-workspace branch July 29, 2026 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant