Proactively refresh auth tokens in the daemon ahead of expiry

[realtonyyoung]

What & why

Auth tokens were refreshed only lazily — TokenStore.GetValidTokensAsync() refreshes only once the access token is already expired, on the next call that needs it. Nothing kept the refresh credential warm ahead of time, so after an idle period the next hook hit a 401 and the user was forced to re-run kcap login.

The daemon now runs a low-frequency loop that refreshes the active profile's token ahead of expiry. For WorkOS's sliding inactivity window, periodic refresh keeps the session alive for as long as the daemon runs, pushing forced re-logins out to the absolute session max instead of the shorter inactivity timeout.

How

• TokenStore.RefreshIfExpiringAsync(window) (Core) — refreshes the active profile's token when it's within window of expiry, through the same profile-scoped cross-process lock as GetValidTokensAsync (rotation-safe re-read under the lock, so it can't race a hook/watcher/MCP refresh or double-spend a rotated WorkOS refresh token). No-op for the None provider and when no tokens are stored. The decision is factored into a pure, fully unit-tested DecideProactiveRefresh. Resolves the active profile once and threads it through the read + lock (tighter than the reactive path).
• TokenRefreshLoop (Daemon) — mirrors DaemonHeartbeatLoop: a total (never-throwing) tick behind an IProactiveTokenRefreshPort. It rate-limits attempts to at most one per interval, so a failing refresh (dead/rotated token, server down) or a token whose lifetime is shorter than the window can't hammer the refresh endpoint every tick.
• Wired into AgentOrchestrator alongside the existing heartbeat loops (60s tick, 5-min window, 5-min min attempt interval), disposed on shutdown.

Acceptance criteria

• Daemon refreshes the active profile's token ahead of expiry on a timer, via the cross-process lock.
• No measurable increase in refresh-endpoint traffic — refresh fires only inside the window, and the loop rate-limits attempts so a failing/short-lived token can't retry-storm.
• A continuously-running daemon keeps a WorkOS session alive up to its absolute lifetime without a manual kcap login.
• No-op for the None auth provider and when no tokens are stored.

Testing

• New unit tests: ProactiveTokenRefreshDecisionTests (all decision branches), RefreshIfExpiringTests (offline no-op paths), TokenRefreshLoopTests (outcome logging, totality, and rate-limiting on failed/successful/short-lived refresh).
• Full unit suite green (2089 tests). dotnet publish -c Release clean — no IL3050/IL2026 AOT warnings on CLI or daemon.

No README/CLI-surface change: this is internal daemon behavior (no new command, flag, default, or prerequisite).

Closes #182
Linear: AI-992

[qodo-code-review]

PR Summary by Qodo

Daemon: proactively refresh auth tokens ahead of expiry

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Add proactive, window-based token refresh to keep sessions alive during daemon idle
• Reuse the existing cross-process lock to avoid refresh-token rotation races
• Introduce a rate-limited daemon loop and unit tests for decisions and backoff behavior

Diagram

graph TD
  A["AgentOrchestrator"] --> B["PeriodicTimer (60s)"] --> C["TokenRefreshLoop"] --> D["IProactiveTokenRefreshPort"] --> E["TokenStore.RefreshIfExpiringAsync"] --> F["Cross-process lock"]
  E --> S[("Token files")]
  E --> X{{"Refresh endpoints"}}
  subgraph Legend
    direction LR
    _svc["Component"] ~~~ _db[("Storage")] ~~~ _ext{{"External"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Schedule a one-shot refresh at (ExpiresAt - window)

• ➕ Avoids periodic token-file reads when tokens are long-lived
• ➕ Naturally refreshes at the earliest eligible moment
• ➖ More complex lifecycle management (reschedule on profile switch / token updates)
• ➖ Harder to reason about with cross-process updates unless you still poll/re-read

2. Fold proactive refresh into existing daemon heartbeat loop

• ➕ Fewer background tasks/timers to manage
• ➕ Centralizes periodic maintenance in one loop
• ➖ Conflates unrelated concerns and logging/telemetry
• ➖ Harder to unit-test rate limiting and outcomes in isolation

3. Refresh only on demand (keep lazy refresh) with better retry/backoff

• ➕ No daemon-driven behavior; minimal background work
• ➕ All refreshes remain tightly coupled to actual usage
• ➖ Still fails after idle periods (401 on next hook), forcing re-login
• ➖ Does not keep WorkOS sliding inactivity sessions alive while daemon runs

Recommendation: The PR’s approach is the best tradeoff: a low-frequency tick with explicit window gating plus post-attempt rate limiting keeps refresh traffic bounded, while reusing the existing profile-scoped cross-process lock preserves refresh-token rotation safety. The one-shot scheduling alternative could reduce periodic reads, but would add significant complexity around rescheduling and cross-process token changes.

Files changed (6) +579 / -5

Enhancement (3) +237 / -5

TokenStore.csAdd proactive refresh decision + RefreshIfExpiringAsync under cross-process lock +89/-5

Add proactive refresh decision + RefreshIfExpiringAsync under cross-process lock

• Introduces a pure DecideProactiveRefresh decision function and a new RefreshIfExpiringAsync(window) entrypoint returning a ProactiveRefreshOutcome. Extends the existing RefreshWithCrossProcessLockAsync to accept a needsRefresh predicate so proactive refresh can re-check “within window” under the lock, preventing rotation races and redundant refreshes.

src/Capacitor.Cli.Core/Auth/TokenStore.cs

AgentOrchestrator.csWire a periodic proactive token refresh loop into daemon lifecycle +38/-0

Wire a periodic proactive token refresh loop into daemon lifecycle

• Adds a 60s PeriodicTimer-driven background loop that runs TokenRefreshLoop with a 5-minute refresh window and 5-minute minimum attempt interval. Ensures the new timer is started alongside heartbeat loops and disposed during orchestrator shutdown.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

TokenRefreshLoop.csIntroduce TokenRefreshLoop with rate limiting and port abstraction +110/-0

Introduce TokenRefreshLoop with rate limiting and port abstraction

• Adds IProactiveTokenRefreshPort plus a TokenStore-backed implementation for production. Implements a total TickAsync that logs outcomes and rate-limits refresh attempts after both success and failure to prevent endpoint hammering.

src/Capacitor.Cli.Daemon/Services/TokenRefreshLoop.cs

Tests (3) +342 / -0

TokenRefreshLoopTests.csUnit-test proactive refresh loop logging, totality, and rate limiting +172/-0

Unit-test proactive refresh loop logging, totality, and rate limiting

• Adds tests using a fake port and injected clock to validate debug/trace/warning logging and that TickAsync never throws (except expected outer cancellation). Verifies backoff behavior after failed and successful attempts, and that NotDue does not arm the rate limiter.

test/Capacitor.Cli.Tests.Unit/Daemon/TokenRefreshLoopTests.cs

ProactiveTokenRefreshDecisionTests.csCover DecideProactiveRefresh branches for windowing and provider gating +101/-0

Cover DecideProactiveRefresh branches for windowing and provider gating

• Adds unit tests for all decision outcomes: no tokens, not due yet, WorkOS refresh, GitHub refresh, inclusive window boundary, and unsupported cases (missing WorkOS credentials or provider None). Ensures proactive refresh only triggers when within the configured expiry window.

test/Capacitor.Cli.Tests.Unit/ProactiveTokenRefreshDecisionTests.cs

RefreshIfExpiringTests.csVerify RefreshIfExpiringAsync offline no-op behaviors +69/-0

Verify RefreshIfExpiringAsync offline no-op behaviors

• Adds non-parallel filesystem tests that validate RefreshIfExpiringAsync is a no-op when no tokens exist, when tokens are comfortably valid, and when provider is None even inside the window. Ensures these paths do not mutate persisted tokens or require network access.

test/Capacitor.Cli.Tests.Unit/RefreshIfExpiringTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 Proactively refresh auth tokens in the daemon to reduce forced re-logins

1. Profile-switch refresh miswrite ✓ Resolved 🐞 Bug ≡ Correctness

Description

RefreshIfExpiringAsync locks and refreshes a specific resolved profile, but the refresh
implementations persist via SaveAsync(StoredTokens) which re-resolves the active profile from disk.
If active_profile changes mid-refresh, refreshed tokens for profile A can be written into profile B
without holding B’s lock, corrupting B’s credentials and leaving A unrefreshed.

Code

src/Capacitor.Cli.Core/Auth/TokenStore.cs[R307-320]

+    public static async Task<ProactiveRefreshOutcome> RefreshIfExpiringAsync(TimeSpan window) {
+        // Resolve the active profile once and thread it through both the read and the lock, so
+        // a profile switch mid-call can't make us refresh one profile's token under another's
+        // lock (GetValidTokensAsync re-resolves; this is deliberately tighter).
+        var profile = await ResolveActiveProfileAsync();
+        var tokens  = await LoadAsync(profile);
+
+        // Re-evaluate the window under the lock too (via this predicate): a peer may refresh
+        // between our read here and our acquiring the lock, leaving the re-read token fresh.
+        bool ExpiringWithinWindow(StoredTokens t) => DateTimeOffset.UtcNow >= t.ExpiresAt - window;
+
+        return DecideProactiveRefresh(tokens, DateTimeOffset.UtcNow, window) switch {
+            RefreshDecision.RefreshWorkOS => ToOutcome(await RefreshWithCrossProcessLockAsync(profile, tokens!, RefreshWorkOSAsync, ExpiringWithinWindow)),
+            RefreshDecision.RefreshGitHub => ToOutcome(await RefreshWithCrossProcessLockAsync(profile, tokens!, RefreshGitHubAsync, ExpiringWithinWindow)),

Evidence

The proactive path resolves a specific profile and passes it into the lock/refresh call, but the
underlying refresh implementations persist via the overload that re-reads the active profile from
config, so a concurrent profile switch can redirect the write to a different profile file (and
bypass its lock).

src/Capacitor.Cli.Core/Auth/TokenStore.cs[307-322]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[204-207]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[413-505]
src/Capacitor.Cli.Core/Config/AppConfig.cs[247-284]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RefreshIfExpiringAsync` resolves `profile` once and refreshes under that profile’s cross-process lock, but `RefreshWorkOSAsync`/`RefreshGitHubAsync` call `SaveAsync(refreshed)` (the legacy overload) which resolves the active profile again from config.

This allows a mid-refresh profile switch to persist refreshed tokens into the *wrong* profile file (and without that profile’s lock), corrupting token storage.

## Issue Context
- `SaveAsync(StoredTokens)` resolves the active profile from disk each call.
- The proactive refresh path explicitly tries to avoid “refresh under the wrong profile lock”, but persistence currently undermines that guarantee.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[307-328]
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[204-207]
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[413-505]

## Suggested direction
- Ensure the refresh path persists using `SaveAsync(profile, refreshed)` for the *same* `profile` used to acquire the lock.
- One robust option: change `RefreshWithCrossProcessLockAsync` so `refresh(...)` returns refreshed tokens *without persisting*, and have `RefreshWithCrossProcessLockAsync` do the `SaveAsync(profile, refreshed)` under the lock.
- Alternatively: parameterize the refresh delegates to accept `profile` or a “save” callback bound to `profile`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Legacy tokens not refreshed ✓ Resolved 🐞 Bug ☼ Reliability

Description

RefreshIfExpiringAsync reads only the per-profile token file via LoadAsync(profile) and never falls
back to the legacy tokens.json. On upgraded installs that still only have tokens.json, the daemon
will treat this as “no tokens” and skip proactive refresh until another code path migrates tokens to
the per-profile store.

Code

src/Capacitor.Cli.Core/Auth/TokenStore.cs[R311-313]

+        var profile = await ResolveActiveProfileAsync();
+        var tokens  = await LoadAsync(profile);
+

Evidence

The proactive path uses the per-profile LoadAsync(profile) only, while the existing non-profile
LoadAsync() explicitly falls back to reading LegacyTokenPath when the per-profile file is missing.

src/Capacitor.Cli.Core/Auth/TokenStore.cs[307-322]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[186-201]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RefreshIfExpiringAsync` loads tokens with `LoadAsync(profile)`, which does not perform the legacy `tokens.json` fallback that `LoadAsync()` provides.

This makes proactive refresh ineffective for users who still have only the legacy store (common immediately after upgrade), until some other path refreshes/saves and deletes the legacy file.

## Issue Context
`LoadAsync()` implements the intended legacy fallback behavior when the per-profile file is genuinely missing; `RefreshIfExpiringAsync` should mirror that behavior for correctness/consistency.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[186-201]
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[307-322]

## Suggested direction
- Implement a profile-aware load in `RefreshIfExpiringAsync` that mimics `LoadAsync()`’s fallback logic for the resolved active profile (i.e., if the per-profile file is Missing, read `LegacyTokenPath`).
- Optionally, if legacy tokens are successfully refreshed, persist them into the resolved profile store to complete migration (and delete legacy), matching existing migration behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Lock timeout treated as failure ✓ Resolved 🐞 Bug ☼ Reliability

Description

When the cross-process lock can’t be acquired before the deadline and the token is still considered
“due” by needsRefresh (e.g., within the proactive window), RefreshWithCrossProcessLockAsync returns
null. RefreshIfExpiringAsync maps that null to Failed, causing the daemon to warn/back off even
though no endpoint refresh was attempted (it was lock contention).

Code

src/Capacitor.Cli.Core/Auth/TokenStore.cs[R359-363]

                if (DateTime.UtcNow >= deadline) {
                    var latest = await LoadAsync(profile);

-                    return latest is { IsExpired: false } ? latest : null;
+                    return latest is not null && !needsRefresh(latest) ? latest : null;
                }

Evidence

The lock-timeout branch returns null when the token is still considered due; proactive refresh maps
null to Failed, and the daemon loop logs a warning/backoff on Failed, even though this path may not
have executed the refresh delegate at all.

src/Capacitor.Cli.Core/Auth/TokenStore.cs[334-366]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[324-328]
src/Capacitor.Cli.Daemon/Services/TokenRefreshLoop.cs[83-96]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
On lock-acquisition timeout, `RefreshWithCrossProcessLockAsync` returns `null` when `needsRefresh(latest)` is true. In the proactive path, `null` is interpreted as `ProactiveRefreshOutcome.Failed`, which triggers warning logs and rate limiting.

This conflates lock contention with an actual refresh attempt failure, producing misleading warnings and potentially delaying the next proactive attempt.

## Issue Context
The proactive loop’s backoff is meant to prevent retry storms against the refresh endpoint; lock timeout does not imply an endpoint hit.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[355-366]
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[324-328]
- src/Capacitor.Cli.Daemon/Services/TokenRefreshLoop.cs[83-96]

## Suggested direction
- Distinguish lock-timeout/contended from refresh-attempt-failed. Options:
 - Add a separate `ProactiveRefreshOutcome` (e.g., `Contended`/`Skipped`) and have the loop log at Debug/Trace and apply a lighter/no backoff.
 - Or change the timeout return contract to return the latest tokens (even if still within window) and have `RefreshIfExpiringAsync` treat it as `NotDue` (since no refresh attempt occurred).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[realtonyyoung]

Independent review (Codex) — fix commit 783b65b

No blocking findings; the fix looks sound. Verified the two earlier findings are addressed:

• Refreshed tokens are now persisted via SaveAsync(profile, refreshed) while holding that same profile's lock (TokenStore.cs in RefreshWithCrossProcessLockAsync); the provider delegates no longer self-save. rg confirms RefreshWorkOSAsync/RefreshGitHubAsync are only reached through RefreshWithCrossProcessLockAsync.
• The peer-refresh short-circuit does not suppress a needed reactive refresh: if a peer wrote a different-but-still-expired token, !latest.IsExpired is false, so the default reactive predicate (IsExpired) still refreshes. Returning latest early only applies to changed + still-valid tokens, avoiding the double proactive rotation without opening a same-token double-spend window.
• The 15s lock-deadline fallback is unchanged and still sound (returns a peer-written token only if the predicate says it's not due, else null → proactive backs off / reactive fails closed).

Verification: full TUnit suite 2093/2093; git diff --check clean. AOT: no IL3050/IL2026 warnings observed; the native publish itself failed in the reviewer's sandbox on an environmental MSBuild task-host error (ComputeManagedAssemblies couldn't spawn the arm64 task host), not from this change — the CI AOT publish check jobs are the source of truth here.