feat: self-mint app tokens and classify failures in ensure-pr-passes-merge-queue#754
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the ensure-pr-passes-merge-queue composite action to support long monitoring windows by self-minting and refreshing GitHub App installation tokens, while also improving outcome classification and reporting for downstream alerting.
Changes:
- Add optional
app-id/private-keyinputs to mint + refresh installation tokens inmonitor-merge-queue.sh(while keepingapp-tokenfor backward compatibility). - Classify consecutive API failures as
api_failure(instead oftimeout) and add a “final recheck” path for timeout/API-failure conclusions. - Add new outputs (
queue-state,queue-position,merge-state-status) and update documentation/examples accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ensure-pr-passes-merge-queue/README.md | Documents new auth modes, new outputs, and clarifies timeout vs api_failure semantics. |
| ensure-pr-passes-merge-queue/monitor-merge-queue.sh | Implements token mint/refresh, new outputs, API-failure classification, and final recheck logic (currently not applied to all non-merged outcomes). |
| ensure-pr-passes-merge-queue/action.yml | Adds new inputs and wires them into the composite action environment; exposes new outputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [ "$PR_STATE" = "CLOSED" ]; then | ||
| echo "PR was closed without merging" | ||
| echo "result=closed" >> "$GITHUB_OUTPUT" | ||
| write_outputs "closed" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Fixed in 8c7ef4f — the CLOSED branch now concludes via conclude_not_merged "closed", so the final fresh-token recheck applies (a closed-then-merged race reports merged).
| if [ $EVICTION_COUNT -ge "$MAX_EVICTIONS" ]; then | ||
| echo "PR evicted $MAX_EVICTIONS times - giving up" | ||
| echo "result=evicted" >> "$GITHUB_OUTPUT" | ||
| write_outputs "evicted" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Fixed in 8c7ef4f — the eviction-limit path now concludes via conclude_not_merged "evicted", matching the documented recheck-before-any-non-merged-outcome behavior. Harness covers both new paths (17 cases, incl. closed/evicted-but-recheck-finds-merged).
Closes #753.
What
app-id+private-key: the monitor script mints GitHub App installation tokens itself (JWT via openssl, per the documented bash pattern) and re-mints at 50 minutes or immediately on HTTP 401 — monitoring windows beyond the 1-hour installation-token TTL now survive.app-tokenremains supported for backward compatibility (short windows only).result=api_failureinstead oftimeout.merged(this exact false-alarm hit camunda/camunda-platform-helm 14.7.0, see ensure-pr-passes-merge-queue: installation token expires mid-monitor, API failures mislabeled as timeout #753).queue-state,queue-position,merge-state-status(last observed) so callers can build informative alerts.result.Testing
--external-sources).gh/curl/sleep, real RSA key so the JWT/openssl path runs for real): 14 cases covering validation, static-token mode, self-mint mode, 401 storms with re-mint, timeout-with-recheck rescue, and all eviction paths (re-queue + merge, eviction limit, auto-merge re-enable failure) — all pass, each asserting exactly oneresult=write.app-token; camunda/camunda-platform-helm will switch both its shepherd workflows toapp-id/private-key(Merge-queue shepherd jobs fail with expired app token after 60 minutes camunda-platform-helm#6634).