fix: handle expired gh auth during runner start#67
Merged
Peyton-Spencer merged 1 commit intomainfrom Mar 26, 2026
Merged
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between ff16286dd5de98c8a3b0e51b005f91cebaa09c54 and 1f33fe3. 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR refactors GitHub CLI authentication handling by introducing a new Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Runner Operation
participant Manager as RunnerManager
participant Service as GHCLIService
participant Process as gh auth status
participant UI as MenuBar/UI
Runner->>Manager: addRunner() / startRunner()
activate Manager
Manager->>Manager: validateGitHubAuth()
activate Manager
Manager->>Service: validateAuth()
activate Service
Service->>Process: runGH(["auth","status"])
Process-->>Service: stdout/stderr/exitCode
Service->>Service: GitHubAuthState.fromProcessResult()
Service-->>Manager: GitHubAuthState
deactivate Service
alt isAuthenticated == false
Manager->>Manager: Set gitHubAuthIssue
Manager-->>Runner: throw GHError.authFailed
else isAuthenticated == true
Manager->>Manager: Clear gitHubAuthIssue
end
deactivate Manager
deactivate Manager
Manager->>UI: Publish gitHubAuthIssue
UI->>UI: Render auth error message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/Views/MenuBarView.swift (1)
560-564: Use one auth snapshot for the Settings refresh.Lines 561-563 run
gh auth statusthree times (checkAuth(),authStatus(), thenrefreshGitHubAuthStatus()). If the CLI state changes between calls, the GitHub tab and footer can disagree. FetchvalidateAuth()once here and derive all three UI fields from that result.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Views/MenuBarView.swift` around lines 560 - 564, Replace the three separate CLI calls with a single auth snapshot fetch: call GHCLIService.shared.validateAuth() once, store its result, set isAuthenticated from that snapshot and set authStatusText from the snapshot's status string, then call runnerManager.refreshGitHubAuthStatus(...) using the same snapshot (or add a refresh overload that accepts the snapshot) so all UI state (isAuthenticated, authStatusText, and runner refresh) derive from the same validateAuth() result instead of calling checkAuth(), authStatus(), and refreshGitHubAuthStatus() separately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Services/GHCLIService.swift`:
- Around line 92-105: The current validateAuth() uses try? which swallows launch
errors from runGH and returns a generic "auth expired" message; change
validateAuth() to use a do-catch around runGH(["auth", "status"]) so you can
inspect thrown errors (e.g., GHError.processLaunchFailed) and return a distinct
GitHubAuthState when the gh binary failed to launch (with a recoveryMessage
advising to check the gh installation/path), while keeping the existing
GitHubAuthState.fromProcessResult(...) for normal process results; update the
error branch to preserve and surface launch-failure details rather than always
suggesting "gh auth login".
In `@Sources/Services/RunnerManager.swift`:
- Around line 1104-1113: The recoveryMessage from ghService.validateAuth can
contain embedded newlines, so normalize it to a single-line string before using
it: in validateGitHubAuth(for:operation:) call ghService.validateAuth(), then
create a sanitized variable (e.g., normalizedRecovery =
authState.recoveryMessage.replacingOccurrences(of: "\n", with: "
").trimmingCharacters(in: .whitespacesAndNewlines)) and assign gitHubAuthIssue,
error, pass normalizedRecovery into logRunnerEvent(for:message:) and throw
GHError.authFailed(normalizedRecovery) so runner.log receives one timestamped
line and formatting stays intact.
---
Nitpick comments:
In `@Sources/Views/MenuBarView.swift`:
- Around line 560-564: Replace the three separate CLI calls with a single auth
snapshot fetch: call GHCLIService.shared.validateAuth() once, store its result,
set isAuthenticated from that snapshot and set authStatusText from the
snapshot's status string, then call runnerManager.refreshGitHubAuthStatus(...)
using the same snapshot (or add a refresh overload that accepts the snapshot) so
all UI state (isAuthenticated, authStatusText, and runner refresh) derive from
the same validateAuth() result instead of calling checkAuth(), authStatus(), and
refreshGitHubAuthStatus() separately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e7486c2-fb83-4e6c-8cc6-eaa93440a664
📥 Commits
Reviewing files that changed from the base of the PR and between c708907 and ff16286dd5de98c8a3b0e51b005f91cebaa09c54.
📒 Files selected for processing (5)
Sources/Services/CLIHandler.swiftSources/Services/GHCLIService.swiftSources/Services/RunnerManager.swiftSources/Views/MenuBarView.swiftTests/MacRunnerTests/MacRunnerTests.swift
Comment on lines
+92
to
+105
| func validateAuth() async -> GitHubAuthState { | ||
| guard let result = try? await runGH(["auth", "status"]) else { | ||
| return false | ||
| return GitHubAuthState( | ||
| isAuthenticated: false, | ||
| statusMessage: "gh CLI not found or not authenticated", | ||
| recoveryMessage: "GitHub authentication expired or is invalid. Run: gh auth login" | ||
| ) | ||
| } | ||
| return result.exitCode == 0 | ||
|
|
||
| return GitHubAuthState.fromProcessResult( | ||
| exitCode: result.exitCode, | ||
| stdout: result.stdout, | ||
| stderr: result.stderr | ||
| ) |
There was a problem hiding this comment.
Preserve gh launch failures instead of rewriting them as auth expiry.
Line 93 uses try?, so GHError.processLaunchFailed(...) gets flattened into the same “authentication expired” recovery text. On machines where gh is missing or the configured path is wrong, the suggested fix (gh auth login) is incorrect.
💡 Suggested fix
func validateAuth() async -> GitHubAuthState {
- guard let result = try? await runGH(["auth", "status"]) else {
- return GitHubAuthState(
- isAuthenticated: false,
- statusMessage: "gh CLI not found or not authenticated",
- recoveryMessage: "GitHub authentication expired or is invalid. Run: gh auth login"
- )
- }
-
- return GitHubAuthState.fromProcessResult(
- exitCode: result.exitCode,
- stdout: result.stdout,
- stderr: result.stderr
- )
+ do {
+ let result = try await runGH(["auth", "status"])
+ return GitHubAuthState.fromProcessResult(
+ exitCode: result.exitCode,
+ stdout: result.stdout,
+ stderr: result.stderr
+ )
+ } catch let GHError.processLaunchFailed(detail) {
+ return GitHubAuthState(
+ isAuthenticated: false,
+ statusMessage: "Failed to launch gh CLI at \(ghPath)",
+ recoveryMessage: "GitHub CLI is unavailable at \(ghPath). Verify the path or install `gh`, then retry.\n\(detail)"
+ )
+ } catch {
+ return GitHubAuthState(
+ isAuthenticated: false,
+ statusMessage: "gh CLI not found or not authenticated",
+ recoveryMessage: "GitHub authentication expired or is invalid. Run: gh auth login"
+ )
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Services/GHCLIService.swift` around lines 92 - 105, The current
validateAuth() uses try? which swallows launch errors from runGH and returns a
generic "auth expired" message; change validateAuth() to use a do-catch around
runGH(["auth", "status"]) so you can inspect thrown errors (e.g.,
GHError.processLaunchFailed) and return a distinct GitHubAuthState when the gh
binary failed to launch (with a recoveryMessage advising to check the gh
installation/path), while keeping the existing
GitHubAuthState.fromProcessResult(...) for normal process results; update the
error branch to preserve and surface launch-failure details rather than always
suggesting "gh auth login".
Comment on lines
+1104
to
+1113
| private func validateGitHubAuth(for runner: Runner, operation: String) async throws { | ||
| let authState = await ghService.validateAuth() | ||
| guard authState.isAuthenticated else { | ||
| gitHubAuthIssue = authState.recoveryMessage | ||
| error = authState.recoveryMessage | ||
| logRunnerEvent( | ||
| for: runner, | ||
| message: "GitHub auth check failed during \(operation): \(authState.recoveryMessage)" | ||
| ) | ||
| throw GHError.authFailed(authState.recoveryMessage) |
There was a problem hiding this comment.
Normalize multiline recovery text before writing it to runner.log.
Line 1111 logs recoveryMessage verbatim, but that string now intentionally includes embedded newlines. logRunnerEvent(...) writes one timestamped line per call, so the detailed auth text will spill into unprefixed lines and break log formatting.
💡 Suggested fix
guard authState.isAuthenticated else {
gitHubAuthIssue = authState.recoveryMessage
error = authState.recoveryMessage
+ let logMessage = authState.recoveryMessage.replacingOccurrences(of: "\n", with: " | ")
logRunnerEvent(
for: runner,
- message: "GitHub auth check failed during \(operation): \(authState.recoveryMessage)"
+ message: "GitHub auth check failed during \(operation): \(logMessage)"
)
throw GHError.authFailed(authState.recoveryMessage)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private func validateGitHubAuth(for runner: Runner, operation: String) async throws { | |
| let authState = await ghService.validateAuth() | |
| guard authState.isAuthenticated else { | |
| gitHubAuthIssue = authState.recoveryMessage | |
| error = authState.recoveryMessage | |
| logRunnerEvent( | |
| for: runner, | |
| message: "GitHub auth check failed during \(operation): \(authState.recoveryMessage)" | |
| ) | |
| throw GHError.authFailed(authState.recoveryMessage) | |
| private func validateGitHubAuth(for runner: Runner, operation: String) async throws { | |
| let authState = await ghService.validateAuth() | |
| guard authState.isAuthenticated else { | |
| gitHubAuthIssue = authState.recoveryMessage | |
| error = authState.recoveryMessage | |
| let logMessage = authState.recoveryMessage.replacingOccurrences(of: "\n", with: " | ") | |
| logRunnerEvent( | |
| for: runner, | |
| message: "GitHub auth check failed during \(operation): \(logMessage)" | |
| ) | |
| throw GHError.authFailed(authState.recoveryMessage) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Services/RunnerManager.swift` around lines 1104 - 1113, The
recoveryMessage from ghService.validateAuth can contain embedded newlines, so
normalize it to a single-line string before using it: in
validateGitHubAuth(for:operation:) call ghService.validateAuth(), then create a
sanitized variable (e.g., normalizedRecovery =
authState.recoveryMessage.replacingOccurrences(of: "\n", with: "
").trimmingCharacters(in: .whitespacesAndNewlines)) and assign gitHubAuthIssue,
error, pass normalizedRecovery into logRunnerEvent(for:message:) and throw
GHError.authFailed(normalizedRecovery) so runner.log receives one timestamped
line and formatting stays intact.
Peyton-Spencer
force-pushed
the
feat/53-gh-auth-expiry
branch
from
ff16286 to
4baba28
Compare
Validate gh auth before fetching registration tokens, publish actionable auth failures in the menubar UI, and stop restart paths from failing silently when credentials expire. Add focused tests around the new auth-state messaging so the recovery guidance stays explicit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Peyton-Spencer
force-pushed
the
feat/53-gh-auth-expiry
branch
from
4baba28 to
1f33fe3
Compare
|
🎉 This PR is included in version 1.14.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
gh auth statusbefore runner registration work so add/start/restart paths fail with explicit recovery guidance instead of silently breaking when credentials expireTesting
git diff --checkswift test(not runnable in this container:swift: command not found)Notes
ghauth by marking the runner with an actionable auth failure instead of repeatedly retrying blindSummary by CodeRabbit
New Features
Bug Fixes
Tests