Github Auth Checks Failed#687
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@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. |
📝 WalkthroughWalkthroughGitHubAppInstallPanel adds a bounded retry mechanism after GitHub device authorization to detect delayed repository access, introducing an exported ChangesRepo Access Retry After Authorization
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.test.ts (1)
5-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider broadening coverage of
isGitHubAppRepoAccessPendingbranches.Current tests exercise only the exact
"not found"match and a non-matching error. The helper (per upstreamGitHubAppInstallPanel.tsx) also has substring branches ("repository not found","could not resolve to a repository") and guard conditions (relayConfigured: false,installed: true, non-"error"state) that returnfalseearly — none of these are covered.♻️ Example additional test cases
it("keeps non-propagation relay failures as check failures after authorization", () => { const status = makeStatus({ error: "GitHub App relay status check failed (500)" }); const view = statusView(status, false, true); expect(isGitHubAppRepoAccessPending(status)).toBe(false); expect(view.label).toBe("Check failed"); expect(view.description("arul28/ADE")).toBe("GitHub App relay status check failed (500)"); }); + + it("detects repository-not-found substring variants", () => { + expect(isGitHubAppRepoAccessPending(makeStatus({ error: "Repository not found." }))).toBe(true); + expect(isGitHubAppRepoAccessPending(makeStatus({ error: "Could not resolve to a Repository." }))).toBe(true); + }); + + it("does not treat pending access when relay is not configured or already installed", () => { + expect(isGitHubAppRepoAccessPending(makeStatus({ relayConfigured: false }))).toBe(false); + expect(isGitHubAppRepoAccessPending(makeStatus({ installed: true }))).toBe(false); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.test.ts` around lines 5 - 47, Expand the GitHubAppInstallPanel test coverage for isGitHubAppRepoAccessPending by adding cases for the other error-string branches in GitHubAppInstallPanel.tsx, including “repository not found” and “could not resolve to a repository,” plus the early-return guards for relayConfigured false, installed true, and a non-"error" state. Use makeStatus, isGitHubAppRepoAccessPending, and statusView to verify each branch returns the expected false or pending behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx`:
- Around line 82-95: The async cleanup in GitHubAppInstallPanel’s status refresh
logic should not use early returns from the finally path because Biome flags
this as noUnsafeFinally. Refactor the control flow around the mountedRef.current
and statusRequestSeqRef.current checks so the finally block only performs
guarded state updates (including setLoading(false)) without returning, while
keeping the existing behavior in the status-fetching routine and its
retryAfterAuthorization handling intact.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.test.ts`:
- Around line 5-47: Expand the GitHubAppInstallPanel test coverage for
isGitHubAppRepoAccessPending by adding cases for the other error-string branches
in GitHubAppInstallPanel.tsx, including “repository not found” and “could not
resolve to a repository,” plus the early-return guards for relayConfigured
false, installed true, and a non-"error" state. Use makeStatus,
isGitHubAppRepoAccessPending, and statusView to verify each branch returns the
expected false or pending behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dd8e3720-78f1-416a-aa70-50352c5fa34a
⛔ Files ignored due to path filters (1)
docs/features/onboarding-and-settings/README.mdis excluded by!docs/**
📒 Files selected for processing (2)
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.test.tsapps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
arul28
added
documentation
1 participant
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
New Features
Bug Fixes
Greptile Summary
This PR improves the GitHub App authorization status flow. The main changes are:
Checking accessstate.Confidence Score: 5/5
Safe to merge with minimal risk.
Changes are localized to UI status handling and helper logic, with targeted tests for the new behavior.
No files require special attention.
What T-Rex did
Important Files Changed
Checking accessstate.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant Panel as GitHubAppInstallPanel participant GitHub as GitHub device auth participant Relay as Hosted relay status User->>Panel: Authorize ADE Panel->>GitHub: start/poll device auth GitHub-->>Panel: authorized Panel->>Relay: getAppInstallationStatus(forceRefresh) Relay-->>Panel: repo access 404 Panel->>Panel: show Checking access loop Retry window Panel->>Relay: getAppInstallationStatus(forceRefresh) Relay-->>Panel: status result end Panel->>Panel: ignore stale responses via request sequence Panel-->>User: Configured, Checking access, or Check failed%%{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 User participant Panel as GitHubAppInstallPanel participant GitHub as GitHub device auth participant Relay as Hosted relay status User->>Panel: Authorize ADE Panel->>GitHub: start/poll device auth GitHub-->>Panel: authorized Panel->>Relay: getAppInstallationStatus(forceRefresh) Relay-->>Panel: repo access 404 Panel->>Panel: show Checking access loop Retry window Panel->>Relay: getAppInstallationStatus(forceRefresh) Relay-->>Panel: status result end Panel->>Panel: ignore stale responses via request sequence Panel-->>User: Configured, Checking access, or Check failedReviews (2): Last reviewed commit: "Address GitHub App status review feedbac..." | Re-trigger Greptile