Skip to content

Fix GitHub webhook delivery + restore PR freshness in installed runtimes#690

Closed
arul28 wants to merge 2 commits into
mainfrom
ade/ok-start-skill-ade-desktop-c2abd496
Closed

Fix GitHub webhook delivery + restore PR freshness in installed runtimes#690
arul28 wants to merge 2 commits into
mainfrom
ade/ok-start-skill-ade-desktop-c2abd496

Conversation

@arul28

@arul28 arul28 commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

ADE   Open in ADE  ·  ade/ok-start-skill-ade-desktop-c2abd496 branch  ·  PR #690

Summary by CodeRabbit

  • New Features
    • Added a new “Checking access” state after GitHub authorization while repository access is still propagating.
    • The app now retries status checks for a short period before showing a failure, giving access time to become available.
  • Bug Fixes
    • Improved handling of repository-not-found errors so they’re treated as pending access when appropriate.
    • Prevented outdated status updates from overriding newer results during refreshes.
    • Updated action visibility so authorization prompts are hidden when access is already being checked.

Greptile Summary

This PR adds a stale-update prevention layer and a post-authorization retry window to the GitHub App installation panel, so GitHub's App-access propagation delay is shown as a friendly "Checking access" state rather than a misleading "Check failed" error.

  • Introduces isGitHubAppRepoAccessPending to detect GitHub 404/not-found error variants as transient access states, and statusView now returns a "Checking access" label for that condition when the user is already authorized.
  • loadStatus gains a statusRequestSeqRef + mountedRef guard that discards stale responses when a newer call supersedes them, and an optional retryAfterAuthorization path that re-polls up to four times (1.5 s → 3 s → 6 s delays) while access is still propagating.
  • The authorize button is hidden during the pending state and the Refresh button triggers the same retry path when the user is authorized.

Confidence Score: 4/5

The change is self-contained to a single UI component and introduces no new network surfaces or data mutations; the worst outcome of a bug here is a stale loading spinner or an incorrect status label, both recoverable by navigating away.

The sequence-number guard and mountedRef correctly discard stale async results in all observed scenarios, including React StrictMode double-invocation. The retry loop exits early as soon as access resolves, so the 10+ second wait only materialises when the repo truly isn't accessible yet. A minor timing window exists where the 'Waiting for repository access…' device message set inside the retry loop can briefly persist after a newer request takes over, but the succeeding request's finally block always corrects it. No state corruption or data loss paths were found.

The retry loop in GitHubAppInstallPanel.tsx (lines 47–59) is the densest part of the change and is worth a second read to confirm the stale-guard ordering matches expectations.

Important Files Changed

Filename Overview
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx Core change: adds sequence-number stale-guard, post-auth retry loop, isGitHubAppRepoAccessPending helper, and 'Checking access' UI state. Logic is generally sound with correct stale-check ordering throughout.
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.test.ts New unit tests covering the two exported helpers (isGitHubAppRepoAccessPending and statusView); all four scenarios are meaningful and assertions are correct.
docs/features/onboarding-and-settings/README.md Doc update accurately describes the new retry window and 'Checking access' state; no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User completes device auth
 result.status === 'authorized'] --> B[setDeviceMessage
'Checking repository access...']
    B --> C[loadStatus forceRefresh=true
retryAfterAuthorization=true
seq++, setLoading=true]
    C --> D[attempt 0: getAppInstallationStatus]
    D --> E{Stale?
mountedRef or seq mismatch}
    E -- Yes --> Z[return early
finally: skip setLoading]
    E -- No --> F[setStatus latestStatus]
    F --> G{isGitHubAppRepoAccessPending?}
    G -- No --> H[break loop]
    G -- Yes --> I[setDeviceMessage
'Waiting for repo access...']
    I --> J[await sleepMs
1500 / 3000 / 6000 ms]
    J --> K{Stale?}
    K -- Yes --> Z
    K -- No --> L{More attempts?}
    L -- Yes --> D
    L -- No --> H
    H --> M[finally: isCurrentRequest?]
    M -- No --> Z
    M -- Yes --> N[await getAppUserAuthStatus]
    N --> O{Still current?}
    O -- No --> Z
    O -- Yes --> P[setAppAuth
setDeviceMessage based on pending state
setLoading=false]
    P --> Q{Access was pending
after all retries?}
    Q -- Yes --> R[Show 'still warming up' message
'Checking access' status label]
    Q -- No --> S[Clear message
Show resolved status]
Loading
%%{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"}}}%%
flowchart TD
    A[User completes device auth
 result.status === 'authorized'] --> B[setDeviceMessage
'Checking repository access...']
    B --> C[loadStatus forceRefresh=true
retryAfterAuthorization=true
seq++, setLoading=true]
    C --> D[attempt 0: getAppInstallationStatus]
    D --> E{Stale?
mountedRef or seq mismatch}
    E -- Yes --> Z[return early
finally: skip setLoading]
    E -- No --> F[setStatus latestStatus]
    F --> G{isGitHubAppRepoAccessPending?}
    G -- No --> H[break loop]
    G -- Yes --> I[setDeviceMessage
'Waiting for repo access...']
    I --> J[await sleepMs
1500 / 3000 / 6000 ms]
    J --> K{Stale?}
    K -- Yes --> Z
    K -- No --> L{More attempts?}
    L -- Yes --> D
    L -- No --> H
    H --> M[finally: isCurrentRequest?]
    M -- No --> Z
    M -- Yes --> N[await getAppUserAuthStatus]
    N --> O{Still current?}
    O -- No --> Z
    O -- Yes --> P[setAppAuth
setDeviceMessage based on pending state
setLoading=false]
    P --> Q{Access was pending
after all retries?}
    Q -- Yes --> R[Show 'still warming up' message
'Checking access' status label]
    Q -- No --> S[Clear message
Show resolved status]
Loading

Reviews (1): Last reviewed commit: "Address GitHub App status review feedbac..." | Re-trigger Greptile

@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Jul 2, 2026 11:08pm

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 94be6276-88f7-4e0c-b4eb-b7d1fcec1c6d

📥 Commits

Reviewing files that changed from the base of the PR and between e15e974 and 2c9943b.

⛔ Files ignored due to path filters (1)
  • docs/features/onboarding-and-settings/README.md is excluded by !docs/**
📒 Files selected for processing (2)
  • apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.test.ts
  • apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.


📝 Walkthrough

Walkthrough

This PR adds a repo-access-pending detection mechanism to GitHubAppInstallPanel, including a retry delay schedule, request sequencing, polling logic after GitHub authorization, a new "Checking access" status view, UI button visibility adjustments, and accompanying unit tests.

Changes

Repo Access Pending Detection and Retry Flow

Layer / File(s) Summary
Detection helper and status view
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx
Adds retry delay constants, a sleepMs utility, and isGitHubAppRepoAccessPending helper that detects pending repo access from error patterns and relay/app state; statusView gains a dedicated "Checking access" branch.
Retry/polling and request sequencing
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx
loadStatus accepts retryAfterAuthorization, uses a request sequence ref and mountedRef to guard against stale updates, and polls getAppInstallationStatus with delays while access remains pending; unmount cleanup invalidates in-flight requests.
UI wiring and rendering
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx
On authorization, triggers retry-enabled loadStatus with a "checking repository access" message; refresh button passes retryAfterAuthorization; derives repoAccessPending to suppress the "Authorize ADE" button when appropriate.
Unit tests
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.test.ts
Adds makeStatus factory and test cases validating isGitHubAppRepoAccessPending and statusView across pending, failed, and guarded scenarios.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested labels: desktop

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade/ok-start-skill-ade-desktop-c2abd496

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@arul28

arul28 commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

Wrong head branch (stale lane record) — superseded by the PR from ade/pr-state-sync-webhook-fallback.

@arul28 arul28 closed this Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant