feat(task-board): push PR checks and preview url from a GitHub webhook - #7039
pedrofrxncx wants to merge 6 commits into
Conversation
…serving #7033 gave the pending reads a zero HIT window, which only starts a background refresh — the poll still served the previous read, so a card assembled from it needed a second poll to show green, and never moved at all if that one background write failed. Give the same reads a zero STALE CEILING so a pending read is a miss and asks GitHub on the spot. Dialog polls a pending card at 10s.
A PR card's CI state and preview url were poll-only, so they landed 1-3 minutes after GitHub had them: the card cache, the raw reads and the dialog poll each held a window, and the windows stacked. GitHub already emits what we need and the receiver already exists — the `deco-cms` App subscribes to `issue_comment`/`push` (and now `check_suite`), and `/api/_github/webhook` has been HMAC-verifying push deliveries for warm pools. It just had no consumer for the PR-card events, and prod has no `GITHUB_WEBHOOK_SECRET`, so the route answers 503. - `check_suite` / `issue_comment` deliveries resolve the PR to its cards (new `findPrLinks` + an index for it — the table had none for a repo/number lookup), re-read GitHub bypassing the read cache (the event IS the invalidation), write the card cache, and emit `task-board.item.prs.updated` on the org's `/watch` stream. - The open dialog writes those cards straight into its query cache, so checks and the Preview button land in about a second instead of on a poll. - Preview urls are now also read off a deploy CHECK, not only a bot comment: the check-run scan was limited to Workers Builds' version id, and any deploy check that prints its url in `output` or `details_url` now counts. Several providers never comment one. - Polling stays as the fallback for repos the App isn't installed on. Bounded: an event for a repo no card links costs one indexed lookup. The webhook is optional — without the secret the route still 503s and everything falls back to the polling behavior it has today.
# Conflicts: # apps/api/src/tools/task-board/prs-get.ts # apps/web/src/hooks/use-task-board-item-prs.ts
…#7094) Reverts the polling escalation from #7033 and #7037. Together they turned a pending-CI card into a live GitHub read on EVERY poll, at a 10s poll: - #7037 gave the CI reads a zero STALE CEILING, so a pending read was a MISS and blocked on the provider instead of being served from KV. - #7033 gave the same reads a zero HIT window (a detached live read per poll), widened the card cache's zero window from "pending AND no preview" to any pending card, and cut the dialog poll from 60s to 15s; #7037 cut it to 10s. Six polls a minute x a live detail read each, per open dialog, against an App installation floor of 5,000/hr - which is the rate limit we are now hitting in production. Back to the pre-#7033 windows: 55s reads / 30s cards / 60s poll. The freshness this bought is coming back off webhooks in #7039, which costs no polling at all. Co-authored-by: Pedro França <pedrofrxncx@deco.cx>
|
Superseded by #7100. This branch is stacked on #7037, which #7094 reverts — we were rate-limited against GitHub in production, and the 10s poll here plus #7037's zero stale ceiling was the cause. It also no longer applies: #7022 replaced the #7100 is the same feature ported onto current main, on top of the revert, with two changes:
It also settles the open question in this PR's description: One thing that did not survive the port: the full delivery→SSE→dialog e2e walk. It stubbed GitHub as an |
Ports #7039 onto post-#7022 main, on top of the #7094 revert. Checks go green and the deploy bot posts a preview url, but the card shows neither for 1-3 minutes. Polling harder is not the fix - it is the incident #7094 just reverted. Use the events GitHub already sends instead: - `/api/_github/webhook` gains `check_suite` / `issue_comment` consumers; they reverse-look-up the PR's cards (new `findPrLinks`, indexed by migration 207), re-read the provider bypassing the read cache, write the card cache and emit `task-board.item.prs.updated` on the org's SSE stream. - The open dialog writes those cards straight into its query cache. - A deploy check that prints its url in `summary` counts as a preview source, so a repo whose bot never comments still gets a Preview button. Changes from #7039, both about not re-creating the rate limit: - The dialog poll stays at 60s. #7039 kept a 10s tier for in-flight cards, which is what #7094 reverted, and the webhook is the freshness path now. A repo the App is not installed on falls back to the minute. - `check_suite` is narrowed to `action: "completed"`. A suite fires three times per commit per app and every refresh is an UNCACHED provider read, so the other two tripled this path's cost to learn "pending" - which the card already shows. Rebased onto the git-providers abstraction: `updatedAt` is now on the neutral `ChangeRequest` (both adapters), the `fresh` bypass is a flag on the read cache rather than a zero-stale-ceiling predicate, and the obsolete `get_status` / `get_check_runs` window overrides are gone with the MCP read path. Dormant until deployed: no `GITHUB_WEBHOOK_SECRET` means 503 and today's polling, unchanged.
Ports #7039 onto post-#7022 main, on top of the #7094 revert. Checks go green and the deploy bot posts a preview url, but the card shows neither for 1-3 minutes. Polling harder is not the fix - it is the incident #7094 just reverted. Use the events GitHub already sends instead: - `/api/_github/webhook` gains `check_suite` / `issue_comment` consumers; they reverse-look-up the PR's cards (new `findPrLinks`, indexed by migration 207), re-read the provider bypassing the read cache, write the card cache and emit `task-board.item.prs.updated` on the org's SSE stream. - The open dialog writes those cards straight into its query cache. - A deploy check that prints its url in `summary` counts as a preview source, so a repo whose bot never comments still gets a Preview button. Changes from #7039, both about not re-creating the rate limit: - The dialog poll stays at 60s. #7039 kept a 10s tier for in-flight cards, which is what #7094 reverted, and the webhook is the freshness path now. A repo the App is not installed on falls back to the minute. - `check_suite` is narrowed to `action: "completed"`. A suite fires three times per commit per app and every refresh is an UNCACHED provider read, so the other two tripled this path's cost to learn "pending" - which the card already shows. Rebased onto the git-providers abstraction: `updatedAt` is now on the neutral `ChangeRequest` (both adapters), the `fresh` bypass is a flag on the read cache rather than a zero-stale-ceiling predicate, and the obsolete `get_status` / `get_check_runs` window overrides are gone with the MCP read path. Dormant until deployed: no `GITHUB_WEBHOOK_SECRET` means 503 and today's polling, unchanged.
#7100) Ports #7039 onto post-#7022 main, on top of the #7094 revert. Checks go green and the deploy bot posts a preview url, but the card shows neither for 1-3 minutes. Polling harder is not the fix - it is the incident #7094 just reverted. Use the events GitHub already sends instead: - `/api/_github/webhook` gains `check_suite` / `issue_comment` consumers; they reverse-look-up the PR's cards (new `findPrLinks`, indexed by migration 207), re-read the provider bypassing the read cache, write the card cache and emit `task-board.item.prs.updated` on the org's SSE stream. - The open dialog writes those cards straight into its query cache. - A deploy check that prints its url in `summary` counts as a preview source, so a repo whose bot never comments still gets a Preview button. Changes from #7039, both about not re-creating the rate limit: - The dialog poll stays at 60s. #7039 kept a 10s tier for in-flight cards, which is what #7094 reverted, and the webhook is the freshness path now. A repo the App is not installed on falls back to the minute. - `check_suite` is narrowed to `action: "completed"`. A suite fires three times per commit per app and every refresh is an UNCACHED provider read, so the other two tripled this path's cost to learn "pending" - which the card already shows. Rebased onto the git-providers abstraction: `updatedAt` is now on the neutral `ChangeRequest` (both adapters), the `fresh` bypass is a flag on the read cache rather than a zero-stale-ceiling predicate, and the obsolete `get_status` / `get_check_runs` window overrides are gone with the MCP read path. Dormant until deployed: no `GITHUB_WEBHOOK_SECRET` means 503 and today's polling, unchanged. Co-authored-by: Pedro França <pedrofrxncx@deco.cx>
|
Closing as stale: this PR sat past the bot's 48h merge window, main has moved on, and its CI results no longer reflect the current base. This is a housekeeping close, not a rejection of the change — if the underlying problem still exists, the bot will find it again and open a fresh, rebased PR. [studio-bot:stale-close] |
The problem
Checks go green on GitHub and the deploy bot posts a preview url, but the card
shows neither for 1–3 minutes. Nothing is stuck — every layer is one window
behind, and the windows stack:
DECOCMS_PR_CARDS)get_status,get_check_runs,get_comments)#7033 zeroed the first two while CI is pending, which is the wrong half of the
timeline: the preview comment lands after the checks turn green, and at that
instant every layer reverts to its full window.
Turning the polling up further is not the fix — at a 10s poll one open dialog on
a mid-flight PR costs ~30 GitHub calls/min, and an App installation's floor is
5,000/hr (~83/min). Three open dialogs would exceed it, against the same limit
review-sweeper.tssays already took the App out once.The fix: use the events GitHub already sends
Most of this was already built and dormant:
/api/_github/webhookhas been HMAC-verifyingpushdeliveries for tenant warm pools. It had no consumer for PR-card events.
deco-cms(ownerdeco-cx) subscribes toissue_commentandpush;check_suitewas added for this (it coversCloudflare Pages / Workers Builds, which Actions-only
workflow_rundoes not)./api/:org/watch, same hub the board already usesfor
task-board.item.updated.What this PR adds:
check_suite/issue_commentdeliveries resolve the PR → its cards via anew
findPrLinks, plus an index for that lookup (migration 204 — thetable's only index was
(organization_id, task_board_item_id)and its PK(task_board_item_id, url), so a repo/number lookup was a full scan on apath that fires for every repo the App is installed on).
invalidation, and a 55s-old read is exactly what it supersedes — writes the
result to the card cache, and emits
task-board.item.prs.updated.the Preview button land in ~1s.
Also: a preview url that only the deploy check knows
Reported alongside: "sometimes the bot doesn't comment the preview url, but the
info is available on the deploy check." True —
extractPreviewUrlFromCheckRunsonly understood
Workers Builds: <name>+Version ID:. Now any deploy checkthat prints its url in
output.summary/output.text/output.title/details_urlcounts, successful runs first, still gated throughisTrustedPreviewHost. The exact Workers Builds path still wins when present.Bounded / reversible
today's polling. Nothing here is load-bearing until
GITHUB_WEBHOOK_SECRETisdeployed.
case in prod.
to 10 min past the PR's
updated_at(a repo that never publishes a previewstops costing reads).
Deploy steps (not code)
GITHUB_WEBHOOK_SECRETinto AWS SMprod/studio/application(us-west-2) —ESO syncs it into
deco-studio-secretswithin 1h. Confirmed missing today:POST https://studio.decocms.com/api/_github/webhook→503 {"error":"github webhook not configured"}./api/_github/webhook.Side effect worth knowing: this switches on the warm-pool push acceleration
that has been dormant since it shipped, for the same reason (no secret).
Testing
Unit (
bun test, 91 pass in the touched files):prRefsFromGithubEventpayload shapes (check_suite fan-out + dedupe, PR comment vs plain-issue comment,
wrong shape); the generalized check-run preview scan (output, details_url,
success-preferred, untrusted host rejected, Workers Builds still wins);
isCardNotReadyincluding its bound.E2E —
packages/e2e/tests/task-board-pr-webhook.spec.tswalks the wholechain black-box, with GitHub as a local MCP server (the card path reads GitHub
through an
mcp-githubconnection, not the REST stub) whose answers flipmid-test from "CI running, no preview" to "CI passed, url in the deploy check's
output" — and never a bot comment:
HMAC-signed delivery → reverse lookup → fresh read → card cache write → SSE push
→ the already-open dialog showing Checks passing and a preview control.
Plus: bad signature → 400, unhandled event → 200 ignored, unlinked PR →
refreshed: 0,issue_commenton a plain issue → not treated as a PR.Migration 204 verified against real Postgres (
task_board_item_prs_repo_idxpresent after
bun run migrate).bun run fmt,bun run lint(0 errors) andapps/apitypecheck clean. The 10failing task-board unit tests are the Postgres-backed ones and fail identically
on
mainin this shell; the 2 remainingapps/webtype errors are thepre-existing duplicate-prosemirror ones.
One open decision
ciMaxStaleMs(from the commit before this one) gives a pending read a zerostale ceiling, so a rate-limited
get_statusreturnsnull→checksStatus: null→prReadyForReviewpasses → the reviewer gets handed a PRwhose CI is still running. With this webhook path in place that blocking read
buys much less; I'd give it a short ceiling instead of 0, or gate the reconciles
on a positively-live checks answer. Left as-is here — say the word and it's a
two-line follow-up.
Summary by cubic
Task-board PR cards were poll-only, so CI results and preview URLs could appear 1–3 minutes late. This adds GitHub webhook refreshes plus SSE updates so open dialogs receive changes in about a second, while polling remains the fallback.
New Features
check_suiteandissue_commentevents and refreshes linked cards with uncached GitHub reads; failures are logged and the delivery returns 200, so GitHub doesn't retry transient errors, and one failing card doesn't block the others on the same PR.task-board.item.prs.updatedto update the open dialog immediately.Migration
task_board_item_prs_repo_idx.GITHUB_WEBHOOK_SECRETin AWS Secrets Manager and use the same value for the GitHub App webhook at/api/_github/webhook.Written for commit aa0a028. Summary will update on new commits.