fix(github-webhook): apply preview limit only to new previews - #5079
fix(github-webhook): apply preview limit only to new previews#5079NoiceHax wants to merge 1 commit into
Conversation
The preview deployment limit was checked before looking up whether the pull request already had a preview, so once an application reached the limit every `synchronize` event was dropped and existing previews went stale while GitHub still saw a 200. The check now runs inside the branch that creates a preview, uses the schema default of 3 when no limit is configured (instead of 0, which blocked every app without an explicit value) and compares with `>=` so the configured limit is inclusive. Closes Dokploy#4074
|
Update on the testing note in the description. I have now run the tests locally, so please ignore the "could not run the suite" line above.
Running the whole test/deploy folder gives 4 failing files (application.real.test.ts, github.test.ts, should-deploy.test.ts, soft-serve.test.ts). Those fail identically on canary without my patch, they need nixpacks and docker swarm which I do not have locally. Not related to this change.
One thing you may want to look at: the Pull Request and autofix.ci runs for this PR are sitting in action_required, so they never actually ran. They need approval before CI reports anything. |
The preview limit was checked before we looked up whether the pull request already had a preview. Once an app hit the limit, every
synchronizeevent was dropped, so open previews stopped rebuilding while GitHub still got a 200 back.I moved the check inside the branch that creates a new preview, so an update to an existing preview always goes through. Two smaller things in the same check. The fallback was
|| 0, so an app with no limit configured was treated as limit zero. It now uses?? 3to match the schema default. The comparison is>=instead of>, so a limit of 3 means 3 previews and not 4. Hitting the limit now logs a warning rather than dropping the event with no trace.Tests are in apps/dokploy/test/deploy/github-webhook-handler.test.ts. The three new cases fail on canary and pass with this change. I could not run the suite locally, so please check CI.
Closes #4074