Skip to content

fix(github): evaluate watch paths against push diff#10938

Open
wottpal wants to merge 1 commit into
coollabsio:nextfrom
wottpal:agent/fix-github-watch-path-push-diff
Open

fix(github): evaluate watch paths against push diff#10938
wottpal wants to merge 1 commit into
coollabsio:nextfrom
wottpal:agent/fix-github-watch-path-push-diff

Conversation

@wottpal

@wottpal wottpal commented Jul 15, 2026

Copy link
Copy Markdown

Changes

  • Evaluate GitHub App push watch paths against the net files changed between the event's before and after SHAs instead of unioning every pushed commit's file arrays.
  • Fall back to webhook file arrays when the Compare API is unavailable, the push creates or deletes a branch, or the comparison reaches GitHub's 300-file response limit.
  • Only request a comparison when at least one matching application has non-empty watch paths.
  • Add focused regression coverage for transient merge-commit paths, empty comparisons, API fallback, and the 300-file limit.

Issues

Category

  • Bug fix
  • Improvement
  • New feature
  • Adding new one click service
  • Fixing or updating existing one click service

Preview

Not applicable; this changes backend webhook filtering behavior and adds regression tests.

AI Assistance

  • AI was NOT used to create this PR
  • AI was used (please describe below)

If AI was used:

  • Tools used: OpenAI Codex
  • How extensively: Root-cause analysis, implementation, focused regression tests, and PR preparation. The resulting diff and public description were reviewed before submission.

Testing

  • vendor/bin/pest tests/Unit/GithubPushChangedFilesTest.php — 4 tests passed, 5 assertions.
  • vendor/bin/pint app/Http/Controllers/Webhook/Github.php bootstrap/helpers/github.php tests/Unit/GithubPushChangedFilesTest.php — passed.
  • php -l for all three changed PHP files — passed.
  • git diff --check — passed.
  • The broader tests/Feature/GithubWebhookTest.php was attempted in the standalone checkout; three manual-webhook cases passed, while the normal-webhook case could not boot because the in-memory SQLite test database lacked the instance_settings table. The failure occurs before the changed push-filtering path.

Contributor Agreement

Important

  • I have read and understood the contributor guidelines. If I have failed to follow any guideline, I understand that this PR may be closed without review.
  • I have searched existing issues and pull requests (including closed ones) to ensure this isn't a duplicate.
  • I have tested all the changes thoroughly with a local development instance of Coolify and I am confident that they will work as expected when a maintainer tests them.

@wottpal
wottpal marked this pull request as ready for review July 15, 2026 07:26
@wottpal

wottpal commented Jul 15, 2026

Copy link
Copy Markdown
Author

Addendum: Did a full manual testing suite now (successfully) ✅ – Obviously, with the help of an LLM (Sol 5.6 xhigh) but human-steered and verified!

Manual testing

Local setup

  1. Cloned this branch at 0f76109d4c1abf8236d6cef41510791948e452de.
  2. Built and started the development stack from source with docker compose -f docker-compose.yml -f docker-compose.dev.yml up -d --build using fresh project volumes.
  3. Waited for the PostgreSQL migrations and seeders to finish, checked that /api/health returned OK, and logged in at http://localhost:18080.
  4. Used the seeded GitHub App source for coollabsio/coolify-examples and configured an application on branch v4.x with:
    • Build pack: Dockerfile
    • Base directory: /dockerfile
    • Dockerfile: /Dockerfile
    • Exposed port: 80

Reproducing the false deployment on the parent commit

  1. Set the application's watch path to dockerfile/** in the Coolify UI.
  2. For the payload, I used this real two-commit range from coollabsio/coolify-examples:
    • before: ef9c0d19e42f1f4bb2b431ff9ba303321d459824
    • commit 1: c157a1c5f2948c702a0e848fcd5dd78e6300e73e
    • commit 2 / after: 30964e962b4830213ac631944850ad64196b41d3
  3. Both commits list dockerfile/Dockerfile as modified, but the second commit restores the original content. GitHub's Compare API for ef9c0d1...30964e9 returns an empty files array.
  4. Checked out the PR's parent (05dc2c65f6134f6491d51d0fcd04cd5645260264) in a detached worktree and restarted the Coolify app container with the parent versions of the changed controller/helper mounted, while keeping the same database and application configuration.
  5. Sent a signed push payload for that range to POST /webhooks/source/github/events, including the real repository ID, branch, before/after SHAs, both commit SHAs, and modified: ["dockerfile/Dockerfile"] on each commit.
  6. The parent code queued deployment drugqtw9s8cqjgairkkdhkmg. It finished successfully in 48 seconds, the application became running:healthy, and the deployed container returned Hello, World! over HTTP. That's the unwanted deployment this PR is meant to prevent.

Verifying this PR with the same payload

  1. Restarted the Coolify app container on this PR's code and confirmed the exact same application/watch-path configuration was still present.
  2. Re-sent the identical signed webhook payload.
  3. The response was Changed files do not match watch paths. Ignoring deployment. with changed_files: [] and watch_paths: ["dockerfile/**"].
  4. The deployment queue count was unchanged and no deployment row was created.

Positive control: a real net change still deploys

  1. Changed the watch path in the UI to bun/**.
  2. Sent a signed push payload for the real range 02812a9d4ce48809d367d4cb62c106367cd21eb6...42592e3a5c3fcb3535b5769a947a886ca836096f. GitHub's Compare API returns exactly bun/README.md for this range.
  3. Coolify queued deployment 4pgma5evoqjusq6hrlb9axgn.
  4. The deployment finished successfully in 14 seconds, the application was running:healthy, and the deployed container returned Hello, World 2! over HTTP.

Compare API failure fallback

  1. Set the watch path to nodejs/**.
  2. Sent a signed push payload whose repository ID still selected the test application, but whose repository name deliberately produced a Compare API 404. The helper returned null, and the payload contained modified: ["nodejs/package.json"].
  3. Coolify fell back to the webhook file list and queued deployment 5beqoubchnfwns3tiyta5mgk.
  4. That deployment finished successfully in 19 seconds and the application remained running:healthy.

Additional checks

  • Set watch_paths to null in the local test application and replayed the push payload. The application still queued and completed normally (zg5aoxvq3kbnzjut23tdkpxp).
  • php artisan test --compact tests/Unit/GithubPushChangedFilesTest.php: 4 passed, 5 assertions.
  • ./vendor/bin/pint --test app/Http/Controllers/Webhook/Github.php bootstrap/helpers/github.php tests/Unit/GithubPushChangedFilesTest.php: passed.
  • php -l on both changed PHP source files: no syntax errors.

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