From c50bdd0e7e4920fd7cc7b208f3e35c3ae8fa8c00 Mon Sep 17 00:00:00 2001 From: Shane Neuville Date: Wed, 1 Apr 2026 09:10:06 -0500 Subject: [PATCH] fix: set sentinel on auth success to prevent lazy Keychain reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After startup, _resolvedGitHubToken stays null when no env var is set and the server self-authenticates. The lazy Keychain path in CheckAuthStatusAsync is guarded by (_resolvedGitHubToken == null), so any later transient auth failure triggers ResolveGitHubTokenForServer → TryReadCopilotKeychainToken → 3 service names × 3s timeout each = 3 macOS "allow copilot-cli" password dialogs. Fix: set _resolvedGitHubToken to string.Empty when auth succeeds. This sentinel means "server can self-authenticate, no Keychain needed." ServerManager.StartServerAsync uses !string.IsNullOrEmpty() so the empty sentinel is never forwarded as an env var. Confirmed via process monitoring: parent PID of /usr/bin/security calls is the PolyPilot .NET process (dotnet exec), not external copilot binaries. Tests: 3064/3064 pass ✅ (2 flaky timing tests pass on retry) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .claude/skills/auth-token-safety/SKILL.md | 7 ++++++- PolyPilot/Services/CopilotService.Utilities.cs | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.claude/skills/auth-token-safety/SKILL.md b/.claude/skills/auth-token-safety/SKILL.md index 4732b64073..1352741148 100644 --- a/.claude/skills/auth-token-safety/SKILL.md +++ b/.claude/skills/auth-token-safety/SKILL.md @@ -70,9 +70,14 @@ Do NOT re-read from Keychain on every recovery cycle. - ❌ Auth polling loop calling `ResolveGitHubTokenForServer()` on every auth detection - ✅ Only `ReauthenticateAsync` (explicit user action) re-reads Keychain - ✅ `TryRecoverPersistentServerAsync` and polling use the cached `_resolvedGitHubToken` +- ✅ `CheckAuthStatusAsync` sets `_resolvedGitHubToken ??= string.Empty` on auth success + so later transient failures don't trigger the lazy Keychain path **Why:** Each Keychain read = another password dialog. The polling loop runs every 10s. -If it re-reads Keychain, users get prompted every 10 seconds. +If it re-reads Keychain, users get prompted every 10 seconds. The sentinel (`""`) on +auth success prevents the lazy path from firing when the server can self-authenticate — +without it, `_resolvedGitHubToken` stays null after startup (no env var set), and any +transient auth failure triggers 3 Keychain reads (3 service names × 3s timeout = 3 dialogs). ### INV-A3: Never clear `_resolvedGitHubToken` on automatic recovery diff --git a/PolyPilot/Services/CopilotService.Utilities.cs b/PolyPilot/Services/CopilotService.Utilities.cs index 826b80866d..446ee4d045 100644 --- a/PolyPilot/Services/CopilotService.Utilities.cs +++ b/PolyPilot/Services/CopilotService.Utilities.cs @@ -865,6 +865,11 @@ private async Task CheckAuthStatusAsync() if (status.IsAuthenticated) { StopAuthPolling(); + // Mark that the server can self-authenticate — no Keychain read needed. + // Without this, _resolvedGitHubToken stays null after startup (no env var), + // and any later transient auth failure triggers the lazy Keychain path + // (3 service names × 3s timeout each = 3 macOS password dialogs). + _resolvedGitHubToken ??= string.Empty; InvokeOnUI(() => { AuthNotice = null;