diff --git a/scarf/Packages/ScarfCore/Sources/ScarfCore/ACP/ACPClient.swift b/scarf/Packages/ScarfCore/Sources/ScarfCore/ACP/ACPClient.swift index e865b86..cc279a4 100644 --- a/scarf/Packages/ScarfCore/Sources/ScarfCore/ACP/ACPClient.swift +++ b/scarf/Packages/ScarfCore/Sources/ScarfCore/ACP/ACPClient.swift @@ -593,7 +593,30 @@ public enum ACPClientError: Error, LocalizedError { /// human-readable hint for the chat UI. Pattern-matches the most common /// fresh-install failure modes. Returns nil when no known pattern matches. public enum ACPErrorHint { - public static func classify(errorMessage: String, stderrTail: String) -> String? { + /// Result of a classifier hit. `hint` is the user-facing copy; when + /// the failure is an OAuth refresh-revocation, `oauthProvider` names + /// the affected provider (lowercase, matching `auth.json` keys) so + /// the UI can offer a one-click re-authenticate affordance. `nil` + /// `oauthProvider` means "we matched a non-OAuth failure mode, or + /// we matched OAuth but couldn't identify which provider." + public struct Classification: Sendable, Equatable { + public let hint: String + public let oauthProvider: String? + + public init(hint: String, oauthProvider: String? = nil) { + self.hint = hint + self.oauthProvider = oauthProvider + } + } + + /// Known OAuth-authed providers Hermes ships. Listed lowercase to + /// match `auth.json.providers.` and the values + /// `OAuthFlowController.start(provider:)` accepts. + private static let oauthProviders = [ + "nous", "claude", "anthropic", "qwen", "gemini", "google", "copilot", "github", + ] + + public static func classify(errorMessage: String, stderrTail: String) -> Classification? { let haystack = errorMessage + "\n" + stderrTail // SSH-level failures come first — they apply only to remote @@ -603,30 +626,55 @@ public enum ACPErrorHint { // all surface as opaque "ACP process terminated" / "request // timed out", and the user has no idea where to look. if haystack.contains("Connection refused") { - return "Couldn't reach the remote host — the SSH port is closed or the droplet is down. Check the host is running and reachable." + return Classification(hint: "Couldn't reach the remote host — the SSH port is closed or the droplet is down. Check the host is running and reachable.") } if haystack.localizedCaseInsensitiveContains("Operation timed out") || haystack.localizedCaseInsensitiveContains("Connection timed out") || haystack.contains("Network is unreachable") || haystack.contains("No route to host") { - return "Couldn't reach the remote host — the network connection timed out. Check the host is running and your network is up." + return Classification(hint: "Couldn't reach the remote host — the network connection timed out. Check the host is running and your network is up.") } if haystack.contains("Permission denied (publickey") || haystack.contains("Permission denied, please try again") { - return "SSH rejected the key. Make sure the right identity file is selected and that ssh-agent has the key loaded — open Terminal and run `ssh-add -l`." + return Classification(hint: "SSH rejected the key. Make sure the right identity file is selected and that ssh-agent has the key loaded — open Terminal and run `ssh-add -l`.") } if haystack.contains("Host key verification failed") || haystack.contains("REMOTE HOST IDENTIFICATION HAS CHANGED") { - return "The remote host's SSH key changed. If you just rebuilt the droplet, remove the old entry with `ssh-keygen -R `, then try again." + return Classification(hint: "The remote host's SSH key changed. If you just rebuilt the droplet, remove the old entry with `ssh-keygen -R `, then try again.") } if haystack.contains("Could not resolve hostname") || haystack.contains("Name or service not known") { - return "Couldn't resolve the host name. Check the host in this server's settings." + return Classification(hint: "Couldn't resolve the host name. Check the host in this server's settings.") } if haystack.localizedCaseInsensitiveContains("command not found") || haystack.contains("hermes: not found") || haystack.contains("exit 127") { - return "The remote shell couldn't find `hermes`. Either install Hermes on the remote (`pipx install hermes-agent`) or set an absolute binary path in this server's settings." + return Classification(hint: "The remote shell couldn't find `hermes`. Either install Hermes on the remote (`pipx install hermes-agent`) or set an absolute binary path in this server's settings.") + } + + // OAuth refresh-token revocation. Hermes prints + // "Refresh session has been revoked. Run `hermes model` to + // re-authenticate." to stderr/stdout when an OAuth-authed + // provider's refresh token can no longer mint access tokens + // (user revoked, server rotated keys, etc.). We can't drive + // `hermes model` interactively, but `hermes auth add + // --type oauth` is the same code path Scarf already drives via + // `OAuthFlowController` for first-time setup, so we surface a + // re-authenticate affordance instead. Checked BEFORE the + // generic "no credentials found" path because the message + // contains the word "credentials" via the surrounding context. + if haystack.localizedCaseInsensitiveContains("refresh session has been revoked") + || haystack.range(of: #"refresh.*revoked"#, options: [.regularExpression, .caseInsensitive]) != nil + || haystack.localizedCaseInsensitiveContains("re-authenticate") + || haystack.localizedCaseInsensitiveContains("reauthenticate") + || (haystack.contains("401") && oauthProvider(in: haystack) != nil) + || (haystack.localizedCaseInsensitiveContains("unauthorized") && oauthProvider(in: haystack) != nil) { + let provider = oauthProvider(in: haystack) + let suffix = provider.map { " (affected provider: \($0))." } ?? "." + return Classification( + hint: "Your OAuth session has expired or been revoked\(suffix) Click Re-authenticate below to sign in again.", + oauthProvider: provider + ) } if haystack.range(of: #"No\s+(Anthropic|OpenAI|OpenRouter|Gemini|Google|Groq|Mistral|XAI)?\s*credentials\s+found"#, @@ -635,7 +683,7 @@ public enum ACPErrorHint { || haystack.contains("ANTHROPIC_TOKEN") || haystack.contains("claude setup-token") || haystack.contains("claude /login") { - return "Hermes can't find your AI provider credentials. Set `ANTHROPIC_API_KEY` (or similar) in `~/.hermes/.env` or your shell profile, then restart Scarf." + return Classification(hint: "Hermes can't find your AI provider credentials. Set `ANTHROPIC_API_KEY` (or similar) in `~/.hermes/.env` or your shell profile, then restart Scarf.") } if let match = haystack.range(of: #"No such file or directory:\s*'([^']+)'"#, options: .regularExpression) { @@ -643,13 +691,31 @@ public enum ACPErrorHint { if let nameStart = matched.range(of: "'"), let nameEnd = matched.range(of: "'", range: nameStart.upperBound.. String? { + let lowered = haystack.lowercased() + for provider in oauthProviders { + // Whole-word match so substrings like "anthropicapi" don't + // false-trigger on "anthropic". + let pattern = "\\b" + NSRegularExpression.escapedPattern(for: provider) + "\\b" + if lowered.range(of: pattern, options: .regularExpression) != nil { + return provider + } } return nil } diff --git a/scarf/Packages/ScarfCore/Sources/ScarfCore/ViewModels/RichChatViewModel.swift b/scarf/Packages/ScarfCore/Sources/ScarfCore/ViewModels/RichChatViewModel.swift index ad0dcc3..1fc69ef 100644 --- a/scarf/Packages/ScarfCore/Sources/ScarfCore/ViewModels/RichChatViewModel.swift +++ b/scarf/Packages/ScarfCore/Sources/ScarfCore/ViewModels/RichChatViewModel.swift @@ -120,6 +120,12 @@ public final class RichChatViewModel { /// users can copy-paste the raw output into a bug report. public var acpErrorDetails: String? + /// Lowercase OAuth provider name (`"nous"`, `"claude"`, …) when the + /// most recent failure was an OAuth refresh-revocation Hermes asked + /// the user to fix via re-authentication. Drives the chat banner's + /// "Re-authenticate" button. Nil for any other failure mode. + public var acpErrorOAuthProvider: String? + /// Optional stderr-tail provider the controller can hook up when it /// creates the ACPClient. Used by `handlePromptComplete` to enrich /// the error banner on non-retryable stopReasons. The closure is @@ -134,6 +140,7 @@ public final class RichChatViewModel { acpError = nil acpErrorHint = nil acpErrorDetails = nil + acpErrorOAuthProvider = nil } /// Populate the error triplet from a thrown Error + the ACPClient @@ -154,10 +161,11 @@ public final class RichChatViewModel { } let msg = error.localizedDescription let stderrTail = await client?.recentStderr ?? "" - let hint = ACPErrorHint.classify(errorMessage: msg, stderrTail: stderrTail) + let cls = ACPErrorHint.classify(errorMessage: msg, stderrTail: stderrTail) acpError = msg - acpErrorHint = hint + acpErrorHint = cls?.hint acpErrorDetails = stderrTail.isEmpty ? nil : stderrTail + acpErrorOAuthProvider = cls?.oauthProvider } /// Populate the error triplet when `handlePromptComplete` sees a @@ -168,11 +176,11 @@ public final class RichChatViewModel { public func recordPromptStopFailure(stopReason: String, client: ACPClient?) async { let msg = "Prompt ended without a response (stopReason: \(stopReason))." let stderrTail = await client?.recentStderr ?? "" - let hint = ACPErrorHint.classify(errorMessage: msg, stderrTail: stderrTail) - ?? Self.fallbackHint(for: stopReason) + let cls = ACPErrorHint.classify(errorMessage: msg, stderrTail: stderrTail) acpError = msg - acpErrorHint = hint + acpErrorHint = cls?.hint ?? Self.fallbackHint(for: stopReason) acpErrorDetails = stderrTail.isEmpty ? nil : stderrTail + acpErrorOAuthProvider = cls?.oauthProvider } /// Same as `recordPromptStopFailure` but pulls stderr from the @@ -182,11 +190,11 @@ public final class RichChatViewModel { private func recordPromptStopFailureUsingProvider(stopReason: String) async { let msg = "Prompt ended without a response (stopReason: \(stopReason))." let stderrTail = await acpStderrProvider?() ?? "" - let hint = ACPErrorHint.classify(errorMessage: msg, stderrTail: stderrTail) - ?? Self.fallbackHint(for: stopReason) + let cls = ACPErrorHint.classify(errorMessage: msg, stderrTail: stderrTail) acpError = msg - acpErrorHint = hint + acpErrorHint = cls?.hint ?? Self.fallbackHint(for: stopReason) acpErrorDetails = stderrTail.isEmpty ? nil : stderrTail + acpErrorOAuthProvider = cls?.oauthProvider } private static func fallbackHint(for stopReason: String) -> String? { diff --git a/scarf/Packages/ScarfCore/Tests/ScarfCoreTests/M1ACPTests.swift b/scarf/Packages/ScarfCore/Tests/ScarfCoreTests/M1ACPTests.swift index 163de27..5d9d77a 100644 --- a/scarf/Packages/ScarfCore/Tests/ScarfCoreTests/M1ACPTests.swift +++ b/scarf/Packages/ScarfCore/Tests/ScarfCoreTests/M1ACPTests.swift @@ -265,19 +265,20 @@ import Foundation errorMessage: "No Anthropic credentials found", stderrTail: "" ) - #expect(noCreds?.contains("ANTHROPIC_API_KEY") == true) + #expect(noCreds?.hint.contains("ANTHROPIC_API_KEY") == true) + #expect(noCreds?.oauthProvider == nil) let missingBinary = ACPErrorHint.classify( errorMessage: "", stderrTail: "No such file or directory: 'npx'" ) - #expect(missingBinary?.contains("npx") == true) + #expect(missingBinary?.hint.contains("npx") == true) let rateLimit = ACPErrorHint.classify( errorMessage: "", stderrTail: "HTTP 429 Too Many Requests: rate limit" ) - #expect(rateLimit?.contains("rate-limit") == true) + #expect(rateLimit?.hint.contains("rate-limit") == true) let unknown = ACPErrorHint.classify( errorMessage: "weird thing", @@ -286,6 +287,53 @@ import Foundation #expect(unknown == nil) } + @Test func errorHintsClassifyOAuthRefreshRevoked() { + // Primary trigger — Hermes's verbatim message when an OAuth + // refresh token can't mint a new access token. Provider name + // appears alongside; classifier should extract it. + let revoked = ACPErrorHint.classify( + errorMessage: "", + stderrTail: "Refresh session has been revoked. Run `hermes model` to re-authenticate." + ) + #expect(revoked?.hint.contains("Re-authenticate") == true) + + // With provider context — surfaces the affected provider name + // so the chat banner can offer a one-click re-auth that targets + // the right OAuth flow. + let revokedWithProvider = ACPErrorHint.classify( + errorMessage: "", + stderrTail: "Provider claude: Refresh session has been revoked. Run `hermes model` to re-authenticate." + ) + #expect(revokedWithProvider?.oauthProvider == "claude") + + // 401 + OAuth provider name — broader catchall for providers + // that don't print the verbatim "revoked" string. + let unauthorized = ACPErrorHint.classify( + errorMessage: "", + stderrTail: "HTTP 401 Unauthorized from nous portal" + ) + #expect(unauthorized?.oauthProvider == "nous") + #expect(unauthorized?.hint.contains("OAuth") == true) + + // Unauthorized on a non-OAuth provider (API-key based) should + // NOT classify as OAuth revocation — no `oauthProvider` known + // to dispatch the re-auth flow against. + let unauthorizedNonOAuth = ACPErrorHint.classify( + errorMessage: "", + stderrTail: "HTTP 401 Unauthorized for groq" + ) + #expect(unauthorizedNonOAuth?.oauthProvider == nil) + + // Word-boundary check — "anthropicapi" must not false-trigger + // on "anthropic". Without word boundaries this catches the + // wrong cases. + let substringNoMatch = ACPErrorHint.classify( + errorMessage: "", + stderrTail: "401 unauthorized: anthropicapi.example.com" + ) + #expect(substringNoMatch?.oauthProvider != "anthropic") + } + // MARK: - Helpers /// Poll `predicate` every ~20ms up to `timeout` seconds. Fails if diff --git a/scarf/scarf/Features/Chat/ViewModels/ChatViewModel.swift b/scarf/scarf/Features/Chat/ViewModels/ChatViewModel.swift index 443fee6..28040c7 100644 --- a/scarf/scarf/Features/Chat/ViewModels/ChatViewModel.swift +++ b/scarf/scarf/Features/Chat/ViewModels/ChatViewModel.swift @@ -139,6 +139,10 @@ final class ChatViewModel { get { richChatViewModel.acpErrorDetails } set { richChatViewModel.acpErrorDetails = newValue } } + var acpErrorOAuthProvider: String? { + get { richChatViewModel.acpErrorOAuthProvider } + set { richChatViewModel.acpErrorOAuthProvider = newValue } + } /// True when `hasAnyAICredential()` returned false at last preflight. var missingCredentials: Bool = false diff --git a/scarf/scarf/Features/Chat/Views/ChatView.swift b/scarf/scarf/Features/Chat/Views/ChatView.swift index e6ca780..bb1d04c 100644 --- a/scarf/scarf/Features/Chat/Views/ChatView.swift +++ b/scarf/scarf/Features/Chat/Views/ChatView.swift @@ -116,6 +116,15 @@ struct ChatView: View { .lineLimit(showErrorDetails ? nil : 2) } Spacer() + if let provider = viewModel.acpErrorOAuthProvider { + Button("Re-authenticate") { + coordinator.pendingOAuthReauth = provider + coordinator.selectedSection = .credentialPools + } + .buttonStyle(.borderedProminent) + .controlSize(.small) + .help("Open Credential Pools and re-authenticate \(provider).") + } if viewModel.acpErrorDetails != nil { Button(showErrorDetails ? "Hide details" : "Show details") { showErrorDetails.toggle() diff --git a/scarf/scarf/Features/CredentialPools/Views/CredentialPoolsView.swift b/scarf/scarf/Features/CredentialPools/Views/CredentialPoolsView.swift index 5172a2f..9e97601 100644 --- a/scarf/scarf/Features/CredentialPools/Views/CredentialPoolsView.swift +++ b/scarf/scarf/Features/CredentialPools/Views/CredentialPoolsView.swift @@ -6,6 +6,14 @@ struct CredentialPoolsView: View { @State private var viewModel: CredentialPoolsViewModel @State private var showAddSheet = false @State private var pendingRemove: HermesCredential? + /// When non-nil, `AddCredentialSheet` opens pre-seeded with this + /// provider name + OAuth type — driven by the chat banner's + /// "Re-authenticate" button via `AppCoordinator.pendingOAuthReauth`, + /// or by clicking the per-row "Re-authenticate" button in this + /// view. Reset to nil when the sheet dismisses so the next plain + /// "Add Credential" press doesn't accidentally inherit it. + @State private var reauthInitialProvider: String? + @Environment(AppCoordinator.self) private var coordinator init(context: ServerContext) { _viewModel = State(initialValue: CredentialPoolsViewModel(context: context)) @@ -42,9 +50,15 @@ struct CredentialPoolsView: View { label: "Loading credentials…", isEmpty: viewModel.pools.isEmpty && viewModel.oauthProviders.isEmpty ) - .onAppear { viewModel.load() } - .sheet(isPresented: $showAddSheet) { - AddCredentialSheet(viewModel: viewModel) { + .onAppear { + viewModel.load() + consumePendingReauth() + } + .onChange(of: coordinator.pendingOAuthReauth) { _, _ in + consumePendingReauth() + } + .sheet(isPresented: $showAddSheet, onDismiss: { reauthInitialProvider = nil }) { + AddCredentialSheet(viewModel: viewModel, initialProvider: reauthInitialProvider) { showAddSheet = false } } @@ -64,6 +78,19 @@ struct CredentialPoolsView: View { } } + /// Drain any pending re-auth hand-off from the chat banner: the + /// banner's "Re-authenticate" button writes to + /// `coordinator.pendingOAuthReauth` and switches to this view; we + /// pick the value up here, seed the sheet's initial provider, and + /// clear the slot so navigating back to this view doesn't re-open + /// the sheet. + private func consumePendingReauth() { + guard let pending = coordinator.pendingOAuthReauth else { return } + reauthInitialProvider = pending + showAddSheet = true + coordinator.pendingOAuthReauth = nil + } + private var header: some View { ScarfPageHeader( "Credential Pools", @@ -166,13 +193,19 @@ struct CredentialPoolsView: View { } } Spacer() + Button("Re-authenticate") { + reauthInitialProvider = provider.provider + showAddSheet = true + } + .controlSize(.small) + .help("Run `hermes auth add \(provider.provider) --type oauth` again to refresh this provider's tokens.") } .padding(.horizontal, 12) .padding(.vertical, 6) .background(.quaternary.opacity(0.3)) } HStack { - Text("Managed by `hermes auth add ` — Scarf is read-only here.") + Text("Click Re-authenticate to refresh tokens. Removing or rotating providers is still done via `hermes auth …` in a terminal.") .font(.caption2) .foregroundStyle(.tertiary) Spacer() @@ -337,8 +370,25 @@ struct CredentialPoolsView: View { /// OAuth flow so the user can paste the authorization code back. private struct AddCredentialSheet: View { @Bindable var viewModel: CredentialPoolsViewModel + /// Optional pre-fill from the re-auth path. When non-nil, the sheet + /// opens with this provider name + OAuth selected, mirroring the + /// state the user would otherwise have to type. Plain "Add + /// Credential" presses leave it nil. + let initialProvider: String? let onDismiss: () -> Void + init( + viewModel: CredentialPoolsViewModel, + initialProvider: String? = nil, + onDismiss: @escaping () -> Void + ) { + self.viewModel = viewModel + self.initialProvider = initialProvider + self.onDismiss = onDismiss + _providerID = State(initialValue: initialProvider ?? "") + _authType = State(initialValue: initialProvider == nil ? .apiKey : .oauth) + } + enum AuthType: String, CaseIterable, Identifiable { case apiKey = "API Key" case oauth = "OAuth" @@ -352,8 +402,8 @@ private struct AddCredentialSheet: View { } } - @State private var providerID: String = "" - @State private var authType: AuthType = .apiKey + @State private var providerID: String + @State private var authType: AuthType @State private var apiKey: String = "" @State private var label: String = "" @State private var providers: [HermesProviderInfo] = [] diff --git a/scarf/scarf/Navigation/AppCoordinator.swift b/scarf/scarf/Navigation/AppCoordinator.swift index 7934c37..b6ecf58 100644 --- a/scarf/scarf/Navigation/AppCoordinator.swift +++ b/scarf/scarf/Navigation/AppCoordinator.swift @@ -108,4 +108,12 @@ final class AppCoordinator { /// session) — a new session needs a cwd override Scarf doesn't /// yet have an id for. var pendingProjectChat: String? + + /// Lowercase OAuth provider name to re-authenticate. Set by the + /// chat error banner's "Re-authenticate" button, consumed by + /// CredentialPoolsView, which auto-presents the OAuth sheet seeded + /// to this provider. Cleared by the consumer once handled. Sister + /// of `pendingProjectChat` — a hand-off slot, not a long-lived + /// state value. + var pendingOAuthReauth: String? }