Fix GitHub webhook delivery + restore PR freshness in installed runtimes#691
Conversation
…snapshot precedence - Start GitHub PR polling at project startup (prs.polling_start) so the fallback feeds Work/Lanes/PRs via prs-updated without requiring a PR tab read first; allow ADE_ENABLE_PR_POLLING and ADE_ENABLE_AUTOMATION_INGRESS in startup stability mode. - prsGetForLane now ensures polling is running (Work tab lane reads). - Lane PR tag precedence: a stale non-terminal GitHub snapshot state can no longer override a terminal ADE linked PR state. - GitHubTab reconciles linked local terminal state (merged/closed) over stale open GitHub list items for filtering, counts, selection, render. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…times Hosted relay (apps/webhook-relay): - Add POST /github/repos/:owner/:name/webhook/heal (repo-admin gated): re-syncs the GitHub App's webhook secret to the worker's current GITHUB_WEBHOOK_SECRET via PATCH /app/hook/config. Recovers from the secret drift that made GitHub sign every delivery with a stale secret (all deliveries 401-rejected since 2026-06-30). - Add GET /github/repos/:owner/:name/webhook/deliveries (push+ gated): proxies the GitHub App delivery log filtered to the repo so delivery failures are observable without the GitHub App settings UI. - assertGitHubRepoAuthorized gains an access level (write|admin) and returns the repository id for delivery filtering. App-side consumption (desktop + daemon): - automationIngressService now accepts a null automationService: the GitHub relay poll and prService.ingestGithubWebhook (PR freshness) run even where the automations feature is unavailable (packaged builds, installed daemons); rule dispatch, the local webhook server, and ingress status stay gated. Cursor persistence falls back to an injected kv-backed store. - ade-cli daemon: actually start the ingress (it was constructed but never started) and add the missing prPollingService so runtime-bound windows get background GitHub polling + prs-updated events. Polling had been dead in installed builds since PR reads moved off the in-process IPC path (~May 21). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- prPollingService: throttle zero-tracked-PR discovery (a forced full repo snapshot fetch) to a 10-minute cadence instead of every 60s tick; read-driven surfaces still discover instantly on their own. - automationIngressService: treat a missing GitHub App user token as a quiet auth-pending state — skip hosted relay polls for 5 minutes, log the transition once at info, report status "disabled" instead of warning every tick. pollNow() bypasses the cooldown so post-authorize checks are instant. Keeps the pre-webhook read-driven freshness path (renderer coalesced reads + 120s snapshot TTL) untouched as the default for repos without the GitHub App. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ldown status, dedupe shared helpers - relay deliveries endpoint drops repo-scoped rows when the repo id cannot be proven to match the authorized repo (fail closed). - Relay poll skips before the "polling" status write during the auth-pending cooldown so the "disabled" status stays accurate. - createKvIngressCursorStore owns the kv cursor key template once (desktop main + daemon bootstrap both consume it). - isTerminalPrState hoisted to renderer/lib/prState.ts (was duplicated in lanePageModel.ts and GitHubTab.tsx). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g row, docs parity - prAsync.test.ts: pin the empty-cache discovery throttle (first tick discovers, 60s ticks inside the 10-min window do not, the crossing tick does; no tracked-PR refresh and a single prs-updated on the empty path). - relay.test.ts: delivery-log filtering drops a repo-scoped row whose repository_id cannot be parsed (fail closed). - docs: pull-requests README documents daemon-owned background polling, the discovery throttle, and the shared terminal-state precedence rule (GitHubTab reconciliation + lane PR tag); automations README documents the ingress PR-freshness-only mode, kv cursor store, auth-pending cooldown, and the relay webhook heal/deliveries routes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lane chips shouldPreferGithubTag was a copy of the desktop logic with the same bug: a stale non-terminal GitHub snapshot row could override a merged/closed ADE PR for the SAME PR, flipping a lane chip back to "open" after merge. Mirror the desktop rule (terminal ADE state is never superseded by a non-terminal GitHub state for a matched PR; unmatched behavior unchanged) and add the regression case to ADETests. Verified with a full simulator xcodebuild (BUILD SUCCEEDED). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (16)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@copilot review but do not make fixes |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- Deliveries endpoint always fetches the max GitHub page (per_page=100) and applies the caller's limit to the repo-filtered output, so busy sibling repos cannot starve the requested repo out of a small page. - automationIngressService throws at construction when automationService is unavailable and no ingressCursorStore is provided — a silent in-memory cursor would reset on every restart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d58c2bcb10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // only leaves via an explicit reopen), so surfaces treat a terminal local | ||
| // state as authoritative over a stale non-terminal GitHub snapshot row. | ||
| export function isTerminalPrState(state: PrSummary["state"]): boolean { | ||
| return state === "merged" || state === "closed"; |
There was a problem hiding this comment.
When a PR is reopened on GitHub, the repo snapshot can legitimately show the linked item as open while ADE's cached PrSummary still says closed. Including closed here makes reconcileLinkedPrState and the lane tag logic rewrite that fresh snapshot back to closed, so the PR can disappear from the Open filter and keep a closed lane badge until a separate background refresh fixes the local row. Only merged is irreversible, or the closed-vs-open case needs a freshness check before overriding the GitHub snapshot.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Considered and deliberately kept closed in the terminal set. Tradeoff: dropping it would make a lane badge / list row flip back to "open" for up to ~2 minutes after every user-initiated close (a very common ADE action, and the exact stale-flicker class this PR fixes), to protect the rare external-reopen case. That reopen case is transient and now self-heals fast: pull_request.reopened is a subscribed webhook event (marks the PR hot → refresh within seconds), and the daemon polling loop bounds it to ≤2 min even without webhooks. A timestamp-freshness comparison was considered and rejected — it introduces cross-source clock/format comparison fragility for a rare, bounded case.
Snapshot-derived filter counts now apply the reconciliation delta, so a stale open row that displays under Merged is also counted under Merged (Greptile P1). Regression assertions added to the stale-open test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf7158fbd0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // only leaves via an explicit reopen), so surfaces treat a terminal local | ||
| // state as authoritative over a stale non-terminal GitHub snapshot row. | ||
| export function isTerminalPrState(state: PrSummary["state"]): boolean { | ||
| return state === "merged" || state === "closed"; |
There was a problem hiding this comment.
When ADE has a PR locally as closed and that PR is reopened on GitHub, the fresh GitHub row is open, but this helper classifies closed as terminal, so reconcileLinkedPrState and shouldPreferGithubPrTag keep showing the stale closed row instead of the reopened PR. That hides the PR from the open tab/lane chip until a separate local refresh catches up; only merged is irreversible, while closed can transition back via GitHub's reopen action. The mirrored Swift lane helper needs the same distinction.
Useful? React with 👍 / 👎.
| // after the repo filter — otherwise busy sibling repos could starve the | ||
| // requested repo out of a small page. | ||
| const limit = Math.min(100, parseLimit(new URL(request.url))); | ||
| const response = await fetch(`${githubApiBaseUrl(env)}/app/hook/deliveries?per_page=100`, { |
There was a problem hiding this comment.
When the GitHub App has more than 100 newer deliveries for other repositories, the requested repo's first matching delivery can be on a later cursor page. This endpoint fetches only the first app-wide page and then filters locally, so it can report no failures even though matching deliveries exist; it should keep following the delivery API cursor/Link pages until the per-repo limit is filled or pagination is exhausted.
Useful? React with 👍 / 👎.
1 participant
Summary
PR #687 showed as merged in one panel but open everywhere else, and GitHub-backed PR state had silently degraded since webhooks were added. Three stacked root causes, all fixed here plus the already-deployed relay-side heal:
GITHUB_WEBHOOK_SECRETon 2026-06-30; every delivery since 401'd. The worker gainsPOST /github/repos/:owner/:name/webhook/heal(repo-admin gated; re-syncs the App's webhook secret to the worker's own viaPATCH /app/hook/config— converge-only, idempotent) andGET .../webhook/deliveries(push+ gated, repo-filtered fail-closed delivery-log proxy). Heal was executed against production and a livepull_requestdelivery verified end-to-end (GitHub → worker 202 → D1 → repo events route).prPollingServiceat all, and packaged builds nulled the whole ingress stack via the automations feature gate. The ingress now runs in a PR-freshness-only mode when automations are unavailable (relay poll +prService.ingestGithubWebhook; rule dispatch/local webhook server/status stay gated), with kv-backed cursor persistence, and the daemon starts both loops.shouldPreferGithubPrTag), the GitHub tab (reconcileLinkedPrStateoverdisplayedItemsfor list/filter/counts/selection), and the same inherited bug in the iOS lane chips (LaneHelpers.shouldPreferGithubTag, full simulator build verified).Cheap-by-default guarantees for machines and rate limits: zero-tracked-PR discovery (a forced full-snapshot fetch) is throttled to a 10-minute cadence; missing GitHub App authorization puts the relay poll into a quiet 5-minute auth-pending cooldown (one info log, status
disabled,pollNow()bypasses); poll ticks make zero GitHub calls when rows are fresh. The pre-webhook read-driven freshness path (renderer coalesced reads + 120s snapshot TTL) is untouched and remains the default for repos without the GitHub App.Validation
🤖 Generated with Claude Code
Greptile Summary
This PR fixes three root causes behind stale/invisible PR state: a drifted webhook secret causing delivery 401s since 2026-06-30 (fixed with a new relay heal endpoint + deliveries proxy), the daemon never running ingress or PR polling loops in production (fixed in both
bootstrap.tsandmain.ts), and stale GitHub snapshot rows overriding terminal local PR states in the UI.relay.ts): AddsPOST /github/repos/:owner/:name/webhook/heal(admin-gated, idempotent secret re-sync viaPATCH /app/hook/config) andGET .../webhook/deliveries(push-gated, repo-filtered delivery log proxy that fails closed on unresolvablerepository_id).bootstrap.ts,main.ts,automationIngressService.ts,prPollingService.ts):automationIngressServiceis now always constructed with a PR-freshness-only mode (relay poll +ingestGithubWebhook) whenautomationServiceis null; construction enforcesingressCursorStorepresence in that mode. Auth-pending cooldown replaces per-tick errors.prPollingServiceis added to both the daemon and the desktop default background task set, with a 10-minute throttle on zero-tracked-PR discovery.GitHubTab.tsx,lanePageModel.ts,LaneHelpers.swift,prState.ts): A new sharedisTerminalPrStatehelper powersreconcileLinkedPrState(GitHub tab list/filter/counts/selection) and fixesshouldPreferGithubPrTag(Lanes + iOS chips) so terminal ADE states always win over stale non-terminal GitHub snapshots.Confidence Score: 5/5
Safe to merge; all three root causes are addressed with matching test coverage, and no regressions were found in the changed paths.
The PR addresses targeted, well-understood bugs with clear before/after behavior. The relay endpoints are properly auth-gated and idempotent. The daemon ingress and polling startup paths are tested end-to-end. The stale-state precedence fix is correctly symmetric across TypeScript (Lanes + GitHub tab) and Swift (iOS). No incorrect data paths, missing required side effects, or broken contracts were found in the diff.
No files require special attention. The
if (automationIngressService)guard inapps/desktop/src/main/main.tsaroundscheduleBackgroundProjectTaskis always-true dead code now that the service is unconditionally constructed — noted in the previous review cycle and left for a follow-up cleanup.Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant GH as GitHub participant Relay as Cloudflare Worker participant Daemon as ADE Daemon (bootstrap.ts) participant Ingress as automationIngressService participant PR as prService participant UI as Desktop Renderer GH->>Relay: POST /github/webhook (pull_request event) Relay->>Relay: HMAC verify + D1 store loop Every ~60s (automationIngressService relay poll) Daemon->>Ingress: pollGithubRelayOnce() Ingress->>Relay: "GET /github/repos/:owner/:repo/events?after=cursor" Relay-->>Ingress: "{ events, nextCursor }" Ingress->>PR: ingestGithubWebhook(event) Ingress->>Ingress: setIngressCursor(kv store) end loop Every ~60s (prPollingService) Daemon->>PR: listAll() / discoverLanePullRequests() PR-->>Daemon: PR list with updated states Daemon->>UI: emit prs-updated event end UI->>UI: reconcileLinkedPrState(githubItem, adesLocalPr) Note over UI: terminal ADE state wins over stale GitHub snapshot%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant GH as GitHub participant Relay as Cloudflare Worker participant Daemon as ADE Daemon (bootstrap.ts) participant Ingress as automationIngressService participant PR as prService participant UI as Desktop Renderer GH->>Relay: POST /github/webhook (pull_request event) Relay->>Relay: HMAC verify + D1 store loop Every ~60s (automationIngressService relay poll) Daemon->>Ingress: pollGithubRelayOnce() Ingress->>Relay: "GET /github/repos/:owner/:repo/events?after=cursor" Relay-->>Ingress: "{ events, nextCursor }" Ingress->>PR: ingestGithubWebhook(event) Ingress->>Ingress: setIngressCursor(kv store) end loop Every ~60s (prPollingService) Daemon->>PR: listAll() / discoverLanePullRequests() PR-->>Daemon: PR list with updated states Daemon->>UI: emit prs-updated event end UI->>UI: reconcileLinkedPrState(githubItem, adesLocalPr) Note over UI: terminal ADE state wins over stale GitHub snapshotComments Outside Diff (1)
apps/desktop/src/main/main.ts, line 3772-3784 (link)automationIngressServiceis now always constructed (nevernull), so theif (automationIngressService)guard is dead code — the branch is always taken. The surroundingscheduleBackgroundProjectTaskcall is still correct becauseADE_ENABLE_AUTOMATION_INGRESSgates execution, but the conditional wrapper adds noise and could mislead future readers into thinking the service can still be absent here.Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (3): Last reviewed commit: "Address review: reconciled rows move in ..." | Re-trigger Greptile