From be28ac34dde2a01986420f1318977c43891d3b2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Wr=C3=B3blewski?= Date: Sat, 12 Sep 2026 22:55:54 +0200 Subject: [PATCH] A preview is not started, or not left, for a pull request that has closed (#111) Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 66 +++++++++++++++++++++++++++++++++++++--- docs/dev-environment.md | 2 +- tasks/plan.md | 12 ++++++-- 3 files changed, 73 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e336b3c..53d1ec9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -555,6 +555,8 @@ jobs: permissions: contents: read packages: read + # #111: to ask whether the pull request is still open. Read only. + pull-requests: read steps: - name: Is the instance wired up yet? id: wired @@ -567,10 +569,37 @@ jobs: else echo "wired=true" >> "$GITHUB_OUTPUT" fi - - uses: actions/checkout@v7 + # #111: a preview that comes up after its pull request has closed stays + # until someone removes it by hand — the cleanup (preview-cleanup.yml) ran + # on `closed`, before there was anything to remove — and two of those + # block every later preview at the cap. Found on 09.09.2026: `pr-77` up for + # eight hours after its merge. The gate on `main` (#61) has since made a + # merge wait for e2e-full, which outlasts this job, but a pull request can + # still be CLOSED unmerged mid-run, a slow start (the database copy, #113) + # can outlast the tests, and this job can be re-run on a closed pull request + # — though a re-run reads the workflow at its own commit, so runs from + # before this check do not have it. + # + # A failed read counts as open: previews then behave as they did before + # this check, rather than stopping because the API hiccuped. + - name: Is the pull request still open? + id: open if: steps.wired.outputs.wired == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR: ${{ github.event.pull_request.number }} + run: | + state=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR" --jq .state) || state=unknown + if [ "$state" = closed ]; then + echo "::notice::Pull request #$PR is closed — no preview for it (#111)." + echo "open=false" >> "$GITHUB_OUTPUT" + else + echo "open=true" >> "$GITHUB_OUTPUT" + fi + - uses: actions/checkout@v7 + if: steps.wired.outputs.wired == 'true' && steps.open.outputs.open == 'true' - name: Authorize this run against the instance - if: steps.wired.outputs.wired == 'true' + if: steps.wired.outputs.wired == 'true' && steps.open.outputs.open == 'true' env: KEY: ${{ secrets.DEV_SSH_KEY }} KNOWN_HOSTS: ${{ secrets.DEV_SSH_KNOWN_HOSTS }} @@ -581,7 +610,7 @@ jobs: echo "$KNOWN_HOSTS" > ~/.ssh/known_hosts chmod 600 ~/.ssh/known_hosts - name: Ship the preview scripts - if: steps.wired.outputs.wired == 'true' + if: steps.wired.outputs.wired == 'true' && steps.open.outputs.open == 'true' env: HOST: ${{ secrets.DEV_SSH_HOST }} run: | @@ -592,7 +621,7 @@ jobs: # `main`, through deploy-dev. scp -i ~/.ssh/id_deploy deploy/preview-up.sh deploy/preview-down.sh "$HOST:/opt/platform-lite/" - name: Start the preview - if: steps.wired.outputs.wired == 'true' + if: steps.wired.outputs.wired == 'true' && steps.open.outputs.open == 'true' env: HOST: ${{ secrets.DEV_SSH_HOST }} GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -611,3 +640,32 @@ jobs: cat preview.log exit 1 fi + # The other half of the race. Closed while the preview was starting, the + # pull request has had its cleanup already — before there was anything to + # remove — so nothing else will ever take this preview down. Looked at + # again once it is up; closing after this line is safe, because the + # cleanup then runs after the preview exists. Not skipped when the start + # failed, nor when the run was cancelled or timed out mid-start: by the + # health wait at the end of preview-up.sh the container is already + # running, its trap does not remove it, and a half-started preview of a + # closed pull request is the same orphan (review). preview-down.sh is safe + # to run for anything; a run cancelled by a newer push reads `open` here + # and does nothing. + - name: Take the preview down if the pull request closed meanwhile + if: >- + always() && + steps.wired.outputs.wired == 'true' && steps.open.outputs.open == 'true' + env: + HOST: ${{ secrets.DEV_SSH_HOST }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR: ${{ github.event.pull_request.number }} + run: | + state=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR" --jq .state) || state=unknown + if [ "$state" = closed ]; then + echo "::notice::Pull request #$PR closed while its preview was starting — removing it (#111)." + ssh -i ~/.ssh/id_deploy "$HOST" "PR='$PR' bash /opt/platform-lite/preview-down.sh" + elif [ "$state" = unknown ]; then + # Not a failure — the preview is up and may well be wanted — but this + # is the one read a missed orphan would hide behind, so say it (review). + echo "::warning::Could not read whether pull request #$PR is still open (#111). If it has closed, remove its preview by hand: PR=$PR bash /opt/platform-lite/preview-down.sh" + fi diff --git a/docs/dev-environment.md b/docs/dev-environment.md index 350a097..f2132dc 100644 --- a/docs/dev-environment.md +++ b/docs/dev-environment.md @@ -189,7 +189,7 @@ at one: afterwards is not there, and anything added in the preview — an account, a work — exists only there and goes away with it. The objects those uploads put in the bucket do NOT: they stay under `pr-/` with nothing naming - them (the same tail as #34 and #111). + them (the same tail as #34). - **You have to sign in to a preview.** Its copy is restored with `sessions` and `verifications` emptied: a session copied out of dev would stay valid in the copy after it was revoked on dev, and nothing could reach in to end it. diff --git a/tasks/plan.md b/tasks/plan.md index 1f1704f..4373cd9 100644 --- a/tasks/plan.md +++ b/tasks/plan.md @@ -91,8 +91,16 @@ in under 5 minutes (manual walkthrough); e2e green. dies with it. The copy is taken with a dump rather than `create database ... template`, which PostgreSQL refuses while anything is connected to the source — dev's own container always is. -- [#111](https://github.com/Devski/platform-lite/issues/111) Preview cleanup loses the race - with a CI run still in flight, and the orphan blocks the two-preview cap (`bug`). +- ~~[#111](https://github.com/Devski/platform-lite/issues/111) Preview cleanup loses the race + with a CI run still in flight, and the orphan blocks the two-preview cap~~ — **done + 12.09.2026**: the preview job asks GitHub whether its pull request is still open before it + starts anything, and again once the preview is up — taking it down itself if the pull + request closed meanwhile, because the cleanup ran before there was anything to remove. A + merge had already stopped winning the race once #61 made it wait for e2e-full; a close + without a merge, a slow start and a re-run on a closed pull request had not. The issue's + second kind of orphan — rows in dev's database naming dead preview prefixes — ended with + #113. The objects under `pr-/` stay with #34, which must delete by row and not by + prefix: 735 of dev's rows still name objects under such prefixes. - ~~[#172](https://github.com/Devski/platform-lite/issues/172) The database has no deadlines~~ — **done 12.09.2026**. Found in the security review of #66: the pool was built with pg's defaults, so `connectionTimeoutMillis` was 0 — a request waiting for a free connection