Send the WorkOS refresh once and stop re-sending a refused token

[realtonyyoung]

Closes #867 — AI-2691

What & why

The shared single-use WorkOS refresh token was being re-sent after WorkOS had already consumed it, which trips AuthKit reuse-detection and revokes the whole token family — the "I have to run kcap login every few minutes" bug. The per-profile file lock already serialized concurrent refreshers correctly; the gap was re-sending a consumed token. Two changes close it: the refresh POST is now single-shot (a single-use token must be at-most-once on the wire — a retry re-presents a token a lost/timed-out first attempt may already have consumed), and its outcome is classified so a refusal is distinguishable from a blip. A refused refresh token (4xx) is terminal: the daemon's proactive loop logs one clear "run kcap login" and backs off an hour instead of re-sending the dead token every few minutes; a transient 5xx/408/429 stays a retryable transport failure. The proactive window drops from 5 minutes to 2 so a token minted seconds ago at login is not refreshed on the spot.

Where to look

WorkOSClient.RefreshAsync returns a typed WorkOSRefreshResult (Rotated / Rejected / TransportFailed); the login-flow callers keep their old behavior by reading .Response. RefreshIfExpiringAsync threads the WorkOS classification through a captured-outcome callback, mirroring the existing onLockContended pattern, so a null result plus a Rejected outcome is a genuine refusal (a peer-refresh or still-fresh re-read returns non-null without calling the delegate).

Not in scope (follow-ups, AI-2691's analysis Fix 4/5): a cross-process "auth broken" marker so hooks/app also stop on a refusal, and making the daemon the sole refresher. This PR closes the primary re-send and the daemon-loop escalation.

Verification

• Core Auth suite green incl. new tests: refused refresh → Rejected and sent exactly once (no retry); a 200 → Rotated; a 503 → TransportFailed; RefreshIfExpiringAsync 400 → ProactiveRefreshOutcome.Rejected; the 2-minute window boundary.
• Daemon TokenRefreshLoopTests: a Rejected tick logs once and arms the hour-long backoff.
• Both AOT publishes (CLI and daemon) clean.

[qodo-code-review]

PR Summary by Qodo

Prevent repeated WorkOS refresh-token reuse

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Sends each single-use WorkOS refresh token once and classifies terminal versus transient failures.
• Backs off rejected daemon refreshes while directing users to re-authenticate.
• Narrows proactive refresh timing and adds outcome-focused regression coverage.

Diagram

graph TD
    LOOP["Daemon loop"] --> STORE["Token store"] --> CLIENT["WorkOS client"] --> API["WorkOS API"] --> OUTCOME{"Refresh outcome"}
    OUTCOME -->|Rotated| SAVE["Persist tokens"]
    OUTCOME -->|Rejected| BACKOFF["Reauth warning"]
    OUTCOME -->|Transient| RETRY["Normal cadence"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Persist rejected authentication state

• ➕ Stops every CLI, hook, app, and daemon process from retrying a known-dead token.
• ➕ Avoids the daemon's remaining hourly probe after a terminal rejection.
• ➖ Requires durable marker lifecycle and invalidation when login replaces credentials.
• ➖ Broadens the change across all authentication consumers and cross-process state handling.

2. Make the daemon the sole refresher

• ➕ Centralizes token rotation and eliminates competing refresh implementations.
• ➕ Provides one place to enforce retry, locking, and terminal-state policies.
• ➖ Requires IPC and clear behavior when the daemon is unavailable.
• ➖ Creates a larger architectural migration for hooks and interactive CLI flows.

Recommendation: Keep the PR's single-shot request and typed outcome as the immediate fix: retrying a rotating token is intrinsically unsafe, and explicit classification lets existing callers remain compatible while the daemon handles terminal rejection correctly. A durable rejected-state marker is the strongest follow-up for suppressing retries across all processes; daemon-only refresh is valuable but substantially broader.

Files changed (11) +227 / -43

Enhancement (1) +15 / -0

WorkOSRefreshResult.csDefine typed WorkOS refresh outcomes +15/-0

Define typed WorkOS refresh outcomes

• Introduces WorkOSRefreshOutcome and WorkOSRefreshResult so callers can distinguish token rotation, terminal refusal, and transport failure while accessing a response only after successful rotation.

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

Bug fix (4) +79 / -32

TokenStore.csPropagate terminal WorkOS refresh rejection +31/-9

Propagate terminal WorkOS refresh rejection

• Adds Rejected to proactive refresh outcomes and captures the underlying WorkOS classification when the refresh delegate runs. Successful peer or local refreshes remain Refreshed, while terminal refusals are separated from retryable failures.

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

WorkOSClient.csMake WorkOS refresh single-shot and classified +30/-20

Make WorkOS refresh single-shot and classified

• Replaces retrying refresh requests with one POST and returns Rotated, Rejected, or TransportFailed. Client errors are terminal rejections, while 5xx, 408, 429, exceptions, and unreadable successful bodies remain transient failures.

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

AgentOrchestrator.csNarrow the proactive refresh window +4/-3

Narrow the proactive refresh window

• Reduces the proactive token refresh window from five minutes to two, preventing newly issued short-lived tokens from being refreshed immediately while retaining margin over the daemon tick and reactive expiry threshold.

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

TokenRefreshLoop.csBack off terminal refresh rejections +14/-0

Back off terminal refresh rejections

• Handles Rejected outcomes with a one-hour backoff and a warning directing the user to run kcap login. This prevents the daemon from repeatedly submitting a dead refresh token.

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

Refactor (1) +2 / -1

OnboardingFacade.csAdapt onboarding refresh to typed WorkOS results +2/-1

Adapt onboarding refresh to typed WorkOS results

• Updates the organizationless onboarding refresh delegate to unwrap the successful response from WorkOSRefreshResult, preserving its existing nullable-response contract.

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

Tests (5) +131 / -10

OAuthFlowTests.csAdapt OAuth tests to typed refresh responses +4/-4

Adapt OAuth tests to typed refresh responses

• Updates existing successful and transport-failure tests to inspect the Response property returned by the new WorkOSRefreshResult contract.

test/Capacitor.Cli.Core.Tests.Unit/Auth/OAuthFlowTests.cs

ProactiveTokenRefreshDecisionTests.csCover the two-minute proactive window +22/-0

Cover the two-minute proactive window

• Adds boundary-oriented tests confirming that tokens 2m30s from expiry remain untouched while tokens 1m30s from expiry are selected for refresh.

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

RefreshIfExpiringTests.csVerify rejected refresh propagation +34/-6

Verify rejected refresh propagation

• Adds a local WorkOS stub returning invalid_grant and verifies that TokenStore reports ProactiveRefreshOutcome.Rejected rather than a generic failure.

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

WorkOSClientTests.csTest WorkOS refresh classifications and request count +51/-0

Test WorkOS refresh classifications and request count

• Covers successful rotation, terminal 400 rejection, and transient 503 handling. The rejection test also proves the single-use token is posted exactly once.

test/Capacitor.Cli.Core.Tests.Unit/Auth/WorkOSClientTests.cs

TokenRefreshLoopTests.csVerify terminal rejection warning and backoff +20/-0

Verify terminal rejection warning and backoff

• Confirms a rejected daemon tick logs one warning mentioning kcap login and suppresses another refresh attempt thirty minutes later.

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

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Token refresh can hang for 100 seconds ✓ Resolved 🐞 Bug ☼ Reliability

Description

RefreshAsync replaces the five-second PostWithRetryAsync budget with PostFormAsync, whose bare
HttpClient.SendAsync has no per-request deadline. Because the WorkOS named client does not
configure Timeout and proactive refresh supplies CancellationToken.None, an unreachable endpoint
can block token resolution or the daemon loop until the default client timeout instead of failing
after five seconds.

Code

src/Capacitor.Cli.Core/Auth/WorkOSClient.cs[L23-24]

-    // Short, so a hook never blocks for the default budget when WorkOS is unreachable.
-    static readonly TimeSpan RetryBudget = TimeSpan.FromSeconds(5);

Relevance

●●● Strong

Recent reliability reviews accept missing HTTP bounds and defensive handling around refresh
requests.

PR-#171
PR-#259

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR removes RetryBudget, while the replacement helper calls SendAsync directly. The WorkOS
client registration only configures redirects, and the repository documents that an unconfigured
HttpClient.Timeout defaults to 100 seconds; the proactive caller passes no cancellation token.

src/Capacitor.Cli.Core/Auth/WorkOSClient.cs[142-155]
src/Capacitor.Cli.Core/Http/CapacitorHttpServices.cs[160-168]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[59-68]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[489-501]

Agent prompt

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

## Issue description
Removing the retry helper also removed the refresh operation's five-second wall-clock budget. The replacement remains single-shot but can now wait for the default HTTP client timeout.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Auth/WorkOSClient.cs[34-60]
- src/Capacitor.Cli.Core/Auth/WorkOSClient.cs[142-155]
- src/Capacitor.Cli.Core/Http/CapacitorHttpServices.cs[160-168]

## Recommended Fix
Restore a five-second linked cancellation deadline around the single `SendAsync` call without adding retries. Preserve caller cancellation separately and map only expiration of the internal deadline to the appropriate refresh outcome.

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

---

2. Lost replies can revoke token families ✗ Dismissed 🐞 Bug ≡ Correctness

Description

RefreshAsync classifies unreadable successful responses and every non-cancellation exception as
TransportFailed, even though WorkOS may already have received the request and rotated the
single-use token. RefreshIfExpiringAsync maps that outcome to Failed, so the daemon retries the
unchanged credential after its normal interval and a lost or malformed success response can trigger
reuse detection.

Code

src/Capacitor.Cli.Core/Auth/WorkOSClient.cs[R57-60]

+                ? new(WorkOSRefreshOutcome.TransportFailed, null)
+                : new(WorkOSRefreshOutcome.Rotated, body);
+        } catch when (!ct.IsCancellationRequested) {
+            return new(WorkOSRefreshOutcome.TransportFailed, null);

Relevance

●●● Strong

Accepted precedents prioritize distinguishing refresh transport failures and preventing unsafe
retries of consumed credentials.

PR-#257
PR-#259

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The client itself documents that a lost response may mean WorkOS already spent the token, but lines
56-60 classify both unreadable successes and caught request failures as retryable transport
failures. TokenStore converts every non-rejected null result to Failed, and the daemon schedules
Failed outcomes on the ordinary retry interval while leaving the stored token unchanged.

src/Capacitor.Cli.Core/Auth/WorkOSClient.cs[23-32]
src/Capacitor.Cli.Core/Auth/WorkOSClient.cs[53-60]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[503-513]
src/Capacitor.Cli.Daemon/Services/TokenRefreshLoop.cs[89-103]

Agent prompt

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

## Issue description
A lost response, malformed successful response, or transport exception after dispatch does not prove that WorkOS left the refresh token unused. Classifying these cases as retryable causes later refresh paths to resend a potentially consumed token.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Auth/WorkOSClient.cs[23-60]
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[489-513]
- src/Capacitor.Cli.Daemon/Services/TokenRefreshLoop.cs[89-111]

## Recommended Fix
Represent ambiguous post-dispatch outcomes as terminal or indeterminate rather than retryable. Durably mark a refresh token as attempted before sending it under the profile lock, replace that marker only after a valid rotated response, and ensure both proactive and reactive paths cannot resend the marked token.

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

---

3. Two refresh comments preserve history ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

RefreshWorkOSAsync calls the non-rotated mapping “unchanged,” while the window-boundary test calls
the two-minute window “shrunk,” so both comments depend on prior implementation state. Future
changes can leave these qualifiers stale and make readers infer historical constraints that the
current mapping and boundary do not require.

Code

src/Capacitor.Cli.Core/Auth/TokenStore.cs[701]

+    // retry. The non-Rotated → null mapping is unchanged: every other caller ignores the outcome.

Relevance

●●● Strong

Recent history accepts removing historical comment qualifiers and shortening narrative comments.

PR-#638
PR-#693

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2897915 prohibits comments that narrate implementation history. The added comments use
unchanged and shrunk, making their meaning depend on prior behavior rather than the current code
alone.

Rule 2897915: Avoid time-sensitive or process-reference metadata in code comments
src/Capacitor.Cli.Core/Auth/TokenStore.cs[699-701]
test/Capacitor.Cli.Core.Tests.Unit/Auth/ProactiveTokenRefreshDecisionTests.cs[67-70]

Agent prompt

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

## Issue description
Two refresh comments describe how behavior changed by calling the mapping `unchanged` and the window `shrunk`, rather than documenting only current behavior and intent.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[699-701]
- test/Capacitor.Cli.Core.Tests.Unit/Auth/ProactiveTokenRefreshDecisionTests.cs[67-70]

## Recommended Fix
Rewrite both comments without historical qualifiers. Describe the current non-rotated-to-null mapping and the boundary enforced by the two-minute refresh window.

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

---

4. A refresh outcome lacks its own file 📘 Rule violation ⚙ Maintainability

Description

WorkOSRefreshResult.cs declares both public top-level WorkOSRefreshOutcome and
WorkOSRefreshResult, although only the record matches the filename. Neither type forms an allowed
extension, implementation hierarchy, or descriptor exception, so the file no longer provides a
one-to-one map to its public types.

Code

src/Capacitor.Cli.Core/Auth/WorkOSRefreshResult.cs[8]

+public enum WorkOSRefreshOutcome {

Relevance

● Weak

Recent same-rule precedent rejects splitting related public types into separate files when the
design remains coherent.

PR-#865
PR-#817

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 3162234 requires one primary top-level type per file with a matching filename. The new file
declares a public enum and a separate public record struct, and this pairing does not match any
permitted exception.

Rule 3162234: One primary type per file, with only narrow documented exceptions
src/Capacitor.Cli.Core/Auth/WorkOSRefreshResult.cs[8-15]

Agent prompt

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

## Issue description
The new source file contains two public top-level types, while the one-primary-type convention requires each public type to have a matching file unless a documented exception applies.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Auth/WorkOSRefreshResult.cs[8-15]

## Recommended Fix
Move `WorkOSRefreshOutcome` into a new `WorkOSRefreshOutcome.cs` file in the same namespace, leaving only `WorkOSRefreshResult` in the existing file.

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

---

Context sources

✅ Compliance rules (platform): 64 rules

✅ Cross-repo context — repo relationships

Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 17/18, lines 270/200; both must reach the floor). Router rationale: This security-sensitive single-use token refresh change spans client classification, persistence/locking, daemon backoff, and multiple caller paths, creating several independent failure modes that benefit from redundant review.

Tip of the day

💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[realtonyyoung]

Ran a Codex review of this branch (codex exec review --base main). One finding, now addressed:

• Refresh could hang for the client's 100 s default (P1). Switching to the single-shot POST dropped the old 5 s budget, so a stalled WorkOS would hold auth and every peer's refresh (a refresh runs under the cross-process lock) for up to 100 s. Fixed by wrapping the single attempt in a linked CancellationTokenSource with a 5 s deadline (injectable for tests); the caller's cancellation stays separate, and a deadline hit reads as TransportFailed. Qodo raised the same issue independently.

Qodo's other two comments were also addressed in this push: an unreadable WorkOS success now maps to Rejected rather than a retryable failure, and two history-narrating comments were rewritten. The residual post-dispatch ambiguity (a reply lost after WorkOS processed it) is left as TransportFailed deliberately and noted as the durable-marker follow-up.