Skip to content

fix(mobile): stop sign-in retry loop on terminal auth failures - #6624

Merged
iscekic merged 3 commits into
mainfrom
kwf/req-auth-loop-c5c2
Sep 24, 2026
Merged

iscekic merged 3 commits into
mainfrom
kwf/req-auth-loop-c5c2

Conversation

@iscekic

@iscekic iscekic commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Changelog for users

  • The app stops retrying a refused sign-in and returns you to the sign-in screen.
  • A credential the server rejects is cleared instead of being reused on every foreground.
  • A throttled sign-in waits at least as long as the server asks before trying again.
  • A refused refresh from an earlier session no longer signs out a session you started afterward.

Changelog for maintainers

  • apps/mobile/src/lib/auth/credentials.ts:226 — accepted: a rejected keychain delete was downgraded to a retryable refresh; the clear now uses Promise.allSettled and still reports the terminal refusal.
  • apps/mobile/src/lib/auth/device-auth-poll.ts:145 — accepted: a retry wait used the whole budget, letting a large Retry-After double the timeout; the wait is now capped by the time left, and the budget check is inclusive.
  • apps/mobile/src/lib/auth/auth-context.tsx:638 — accepted: a refusal from an older session could sign out a newer one; refusals now carry their sessionVersion and sign out only while it is current.
  • apps/mobile/src/lib/auth/credentials.ts:104 — accepted: the 401 clear dropped the in-memory owner before sign-out's remote cleanup, so the revoke and push unregister ran unauthenticated. The clear now deletes only the stored pair; sign-out's teardown still clears the owner after cleanup.
  • A shared classifier maps native auth POSTs to success, terminal, or retry; a 401 and any 4xx without retry guidance are terminal and never retried. Terminal failures report once per process with the fingerprint [auth-terminal, route, status]. Start review at auth-response-class.ts, then the epoch-guarded clear in credentials.ts.
  • Only a refresh-route 401 clears the stored bearer pair; a native/token or passkey 401 is terminal for that attempt without clearing a healthy session.
  • The proactive refresh path signs the person out when a refresh is refused and leaves a transient refresh alone; refresh exposes retryAfterMs parsed from Retry-After on 429/5xx, and the device-auth poll waits max(its backoff, Retry-After) capped by the remaining budget.
  • The earlier stub-run excerpts and the failed CLI test excerpt are removed as superseded; the Android emulator device-auth run appended in the proof section replaces them and shows the poll wait capped by the remaining budget. The sign-in-POST 429 wait is not claimed live.

E2E proof

e1-timed-out

Owner request

Surface: mobile-app

The app retries a failed sign-in call forever instead of stopping and asking the
user to sign in again.

The evidence (production)

  • KILO-APP-JF, 141 events: POST https://api.kilo.ai/api/auth/native/token -> 401
  • KILO-APP-K9, 87 events: POST https://api.kilo.ai/api/auth/native/otp -> 429
  • KILO-APP-18E: POST https://api.kilo.ai/api/auth/native/refresh -> 401

A 401 means the credential is gone. A 429 means the client is asking too often.
Both repeat on one client, so the retry loop is the defect: the app treats a
terminal sign-in failure as a retryable one.

What to build

  1. Classify the sign-in responses. A 401 on native/token or native/refresh
    is terminal: clear the stored credential, stop the loop, and send the user to
    sign in.
  2. Back off on a 429 and honour Retry-After. Do not retry a 4xx that carries
    no retry guidance.
  3. Report each terminal failure once, with a stable fingerprint, instead of once
    per retry.

Proof

One must-run scenario that signs in, invalidates the credential, and shows the
app stopping the loop and reaching the sign-in screen. Quote the decisive log
lines in the pull request body, including the response code and the retry stop.

E2E proof — log excerpts

[e1] device sign-in: a 429 with Retry-After larger than the remaining poll budge -> pass :: Android emulator-5554: with a stub answering the device-auth poll, e1-stub.log records one poll 'POST /api/device-auth/token -> 429 Retry-After: 600' and no second poll, and the app then rendered 'Sign-in timed out. Please try again.' at the 5-minute budget (e1-digest.txt), so the wait was capped by the remaining budget rather than waiting out the 600 s Retry-After; screenshots e1-pending.png/e1-timed-out.png are for the visual reviewer; no UX defect observed.

@iscekic
iscekic marked this pull request as draft September 23, 2026 06:12
Comment thread apps/mobile/src/lib/auth/credentials.ts
Comment thread apps/mobile/src/lib/auth/device-auth-poll.ts Outdated
Comment thread apps/mobile/src/lib/auth/auth-context.tsx Outdated
@kilo-code-bot

kilo-code-bot Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit 7a13bc7 makes the device-auth foreground poll honor the server's Retry-After cooldown through a throttledUntil deadline in apps/mobile/src/lib/auth/device-auth-poll.ts, and adds a regression test that foregrounds inside the cooldown; the deadline is always at or before the retry timer it was scheduled with, so it cannot strand pollNow, and the earlier terminal-auth findings remain addressed. No new issues were found in the changed lines.

Files Reviewed (2 files)
  • apps/mobile/src/lib/auth/device-auth-poll.ts
  • apps/mobile/src/lib/auth/device-auth-poll.test.ts

All other files in the PR diff are unchanged since the previous review and were not re-flagged. Existing findings are fixed at this commit: the 401 clear reports the terminal refusal via Promise.allSettled while leaving the in-memory owner for sign-out's cleanup, the retry wait is capped by the remaining poll budget, and refusals are epoch-scoped before sign-out.

Previous Review Summaries (3 snapshots, latest commit cc84825)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit cc84825)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental changes since the previous review are correct and well-tested: the refresh-route 401 clear is epoch-scoped and best-effort (Promise.allSettled) while still returning refused: true, the in-memory owner keeps serving until sign-out's teardown clears it, the device-auth poll wait is capped by the remaining budget with an inclusive boundary check, refusals carry their sessionVersion and sign out only while current, and the new shared classifier reports each terminal failure once per process; no new issues were found in the changed lines.

Files Reviewed (13 files)
  • apps/mobile/src/lib/auth/auth-context.lifecycle.test.tsx
  • apps/mobile/src/lib/auth/auth-context.test.ts
  • apps/mobile/src/lib/auth/auth-context.tsx
  • apps/mobile/src/lib/auth/auth-fetch.test.ts
  • apps/mobile/src/lib/auth/auth-fetch.ts
  • apps/mobile/src/lib/auth/auth-response-class.test.ts
  • apps/mobile/src/lib/auth/auth-response-class.ts
  • apps/mobile/src/lib/auth/credentials.ts
  • apps/mobile/src/lib/auth/device-auth-poll.test.ts
  • apps/mobile/src/lib/auth/device-auth-poll.ts
  • apps/mobile/src/lib/auth/poll-response.test.ts
  • apps/mobile/src/lib/auth/poll-response.ts
  • apps/mobile/src/lib/auth/refresh-terminal.test.ts

Previous review (commit d1f212e)

Status: No Issues Found | Recommendation: Merge

Executive Summary

All three prior findings on the mobile terminal-auth changes are fixed at HEAD — the refresh clear is best-effort yet still reports the terminal refusal, the device-auth retry wait is capped by the remaining poll budget, and refusals are epoch-scoped before sign-out — and no new issues were found in the changed code.

Files Reviewed (7 files)
  • apps/mobile/src/lib/auth/auth-context.tsx
  • apps/mobile/src/lib/auth/auth-context.lifecycle.test.tsx
  • apps/mobile/src/lib/auth/auth-context.test.ts
  • apps/mobile/src/lib/auth/credentials.ts
  • apps/mobile/src/lib/auth/device-auth-poll.ts
  • apps/mobile/src/lib/auth/device-auth-poll.test.ts
  • apps/mobile/src/lib/auth/refresh-terminal.test.ts

Previous review (commit a9571af)

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

The terminal-auth classification is sound, but a failed credential delete in doRefresh can silently downgrade a terminal 401 back into the retryable loop this PR removes; two lower-severity issues concern poll-budget and epoch handling.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/lib/auth/credentials.ts 226 A rejected SecureStore delete is caught by doRefresh's outer catch and returned as { ok: false, refused: false }, so a terminal 401 is reported as transient, the dead pair is kept, and the retry loop resumes.

SUGGESTION

File Line Issue
apps/mobile/src/lib/auth/device-auth-poll.ts 145 Retry wait is capped at the full POLL_OVERALL_TIMEOUT_MS rather than the remaining budget, so a large Retry-After can push the timeout past the intended deadline (up to ~2x budget).
apps/mobile/src/lib/auth/auth-context.tsx 638 The added signOut(true) on a refused proactive refresh is not gated on the captured epoch; combined with single-flight performRefresh, a refused outcome from an older session can tear down a newer sign-in.
Files Reviewed (11 files)
  • apps/mobile/src/lib/auth/auth-context.tsx - 1 issue
  • apps/mobile/src/lib/auth/auth-fetch.ts - no issues
  • apps/mobile/src/lib/auth/auth-fetch.test.ts - no issues
  • apps/mobile/src/lib/auth/auth-response-class.ts - no issues
  • apps/mobile/src/lib/auth/auth-response-class.test.ts - no issues
  • apps/mobile/src/lib/auth/credentials.ts - 1 issue
  • apps/mobile/src/lib/auth/device-auth-poll.ts - 1 issue
  • apps/mobile/src/lib/auth/device-auth-poll.test.ts - no issues
  • apps/mobile/src/lib/auth/poll-response.ts - no issues
  • apps/mobile/src/lib/auth/poll-response.test.ts - no issues
  • apps/mobile/src/lib/auth/refresh-terminal.test.ts - no issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic force-pushed the kwf/req-auth-loop-c5c2 branch from df7e9b4 to d1f212e Compare September 23, 2026 07:53
@iscekic
iscekic marked this pull request as ready for review September 23, 2026 08:16
@iscekic

iscekic commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

kilo-review — independent audit of the published diff.

Status: 1 Issues

File Line Issue
apps/mobile/src/lib/auth/credentials.ts 104 The new clear deletes the stored pair and empties the in-memory token owner before the refusal-triggered signOut runs, so sign-out's runLogoutCleanup (revokeCurrentDeviceSession, unregisterPushToken) is sent with no Authorization header and fails, where before the change the owner still served the access token to that cleanup.

@iscekic
iscekic marked this pull request as draft September 23, 2026 08:47
@iscekic

iscekic commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Accepted. Pushed 2f83fc7 (kwf kwf-fix-review-7270).

What changed:

  • apps/mobile/src/lib/auth/credentials.ts:226 — accepted: a rejected keychain delete was downgraded to a retryable refresh; the clear now uses Promise.allSettled and still reports the terminal refusal.
  • apps/mobile/src/lib/auth/device-auth-poll.ts:145 — accepted: a retry wait used the whole budget, letting a large Retry-After double the timeout; the wait is now capped by the time left, and the budget check is inclusive.
  • apps/mobile/src/lib/auth/auth-context.tsx:638 — accepted: a refusal from an older session could sign out a newer one; refusals now carry their sessionVersion and sign out only while it is current.
  • apps/mobile/src/lib/auth/credentials.ts:104 — accepted: the 401 clear dropped the in-memory owner before sign-out's remote cleanup, so the revoke and push unregister ran unauthenticated. The clear now deletes only the stored pair; sign-out's teardown still clears the owner after cleanup.
  • A shared classifier maps native auth POSTs to success, terminal, or retry; a 401 and any 4xx without retry guidance are terminal and never retried. Terminal failures report once per process with the fingerprint [auth-terminal, route, status]. Start review at auth-response-class.ts, then the epoch-guarded clear in credentials.ts.
  • Only a refresh-route 401 clears the stored bearer pair; a native/token or passkey 401 is terminal for that attempt without clearing a healthy session.
  • The proactive refresh path signs the person out when a refresh is refused; a transient refresh leaves the session alone.
  • Refresh exposes retryAfterMs parsed from Retry-After on 429/5xx; the device-auth poll waits max(its backoff, Retry-After) capped by the remaining budget.

@iscekic
iscekic force-pushed the kwf/req-auth-loop-c5c2 branch from 2f83fc7 to cc84825 Compare September 23, 2026 09:45
@iscekic
iscekic marked this pull request as ready for review September 23, 2026 10:06
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 23, 2026
@iscekic iscekic self-assigned this Sep 23, 2026
Comment thread apps/mobile/src/lib/auth/device-auth-poll.ts Outdated
@iscekic
iscekic enabled auto-merge (squash) September 24, 2026 14:10
@iscekic
iscekic merged commit 8d07bf5 into main Sep 24, 2026
29 checks passed
@iscekic
iscekic deleted the kwf/req-auth-loop-c5c2 branch September 24, 2026 14:12
iscekic pushed a commit to Kilo-Org/kilocode that referenced this pull request Sep 26, 2026
…4554)

## Automated docs sync — 2026-09-25

This PR keeps kilo.ai/docs in sync with features merged to [Kilo-Org/cloud](https://github.com/Kilo-Org/cloud) and [Kilo-Org/kilocode](https://github.com/Kilo-Org/kilocode). Every change below links to the merged PR it documents.

- Window: `2026-09-24T07:08:33.639Z` → `2026-09-25T07:05:29.302Z`
- Verification (docs build + tests): **passing**

### Surface: `cloud-mobile`

- Assignees / requested reviewers: @iscekic and @eshurakov
- Derivation: Derived from the repository layout. A product surface is a package under packages/ that ships a distinct client, plugin, backend, or hosted service: cli = packages/opencode/ + packages/tui/ + packages/server/ + packages/sdk/ + packages/plugin/; vscode = packages/kilo-vscode/ + packages/kilo-web-ui/ + packages/kilo-ui/; jetbrains = packages/kilo-jetbrains/; gateway = packages/kilo-gateway/; web = packages/kilo-console/ + packages/kilo-indexing/ + packages/kilo-memory/ + packages/kilo-sandbox/. Docs route from the IA tree packages/kilo-docs/pages/ plus docs/jetbrains-vscode-settings-parity.md: each surface lists the pages sections that document it, and the per-platform pages under packages/kilo-docs/pages/code-with-ai/platforms/ map to the matching extension surface (the vscode/ directory to vscode, jetbrains.md to jetbrains). A doc path belongs to the surface with the longest matching prefix; a path that matches none of those prefixes falls to `other` (the explicit other prefixes are listed under other.docs). The cloud surfaces are derived the same way from the Kilo-Org/cloud layout: cloud-mobile = apps/mobile/, cloud-web = apps/web/, cloud-extension = apps/extension/, and cloud-agent = the cloud-agent packages under packages/ (packages/cloud-agent-sdk/ + packages/cloud-agent-profile/). A cloud source names its repository while a bare string still means this repository. The pages under packages/kilo-docs/pages/collaborate/ document the cloud web app (app.kilo.ai: teams dashboard, billing, SSO, adoption dashboard), so they route to cloud-web. No page under packages/kilo-docs/pages/ documents the browser side-panel extension yet, so cloud-extension lists no docs prefix.
- Map: `.github/docs-sync/surfaces.json`
- Surface map: `cli`, `vscode`, `jetbrains`, `gateway`, `web`, `cloud-mobile`, `cloud-web`, `cloud-extension`, `cloud-agent`, `other`
- Source prefixes: `apps/mobile/` (Kilo-Org/cloud)
- Doc prefixes: `packages/kilo-docs/pages/code-with-ai/platforms/mobile.md`
- Paths that fall to `other`: `packages/kilo-docs/pages/community/`, `packages/kilo-docs/pages/kiloclaw/`, `packages/kilo-docs/pages/contributing/`, `packages/kilo-docs/LEARNINGS.md`, `docs/`
- Reviewers are ranked from `Kilo-Org/cloud`; the workflow needs a token with `contents: read` on that repository (repository secret `CROSS_REPO_ACCESS_TOKEN`, exposed to the upsert step as `CLOUD_REPO_TOKEN`).
- How the two were computed: Reviewers for `cloud-mobile` are ranked from `Kilo-Org/cloud` git history over `apps/mobile/` (a commit 180 days old counts half as much, half-life 180 days). Bots (author type "Bot" or a login matching /\[bot\]$/i) and people without admin, write, or maintain permission are excluded.

### Changes

<!-- docs-sync:changes:start -->
| Docs change | Source |
| --- | --- |
| updated pages/code-with-ai/platforms/mobile.md | [Kilo-Org/cloud#6386](Kilo-Org/cloud#6386) |
| updated pages/ai-providers/openai-chatgpt-plus-pro.md | [Kilo-Org/cloud#6702](Kilo-Org/cloud#6702) |
| updated pages/code-with-ai/platforms/cloud-agent.md | [Kilo-Org/cloud#6683](Kilo-Org/cloud#6683) |
| updated pages/getting-started/byok.md | [Kilo-Org/cloud#6692](Kilo-Org/cloud#6692) |
<!-- docs-sync:changes:end -->

### Pending — will retry

<!-- docs-sync:pending:start -->
_None._
<!-- docs-sync:pending:end -->

### Considered, no docs change needed

<!-- docs-sync:skipped:start -->
| PR | Reason |
| --- | --- |
| [Kilo-Org/cloud#6658](Kilo-Org/cloud#6658) | Internal sandbox lifecycle fix with no user-visible workflow or setting. |
| [Kilo-Org/cloud#6673](Kilo-Org/cloud#6673) | Internal container CA trust plumbing, no user-facing behavior. |
| [Kilo-Org/cloud#6672](Kilo-Org/cloud#6672) | Internal sandbox launch/recovery fix with no documented workflow change. |
| [Kilo-Org/cloud#6660](Kilo-Org/cloud#6660) | Internal cloud-agent queue delivery fix; no new command, setting, or workflow for users. |
| [Kilo-Org/cloud#6226](Kilo-Org/cloud#6226) | Internal gateway alias-routing change, not user-visible. |
| [#14490](#14490) | Tool-call animation and streaming UI polish; users do not need to learn a new workflow. |
| [#14530](#14530) | Bug fix restoring intended worktree-pool behavior, no doc change needed. |
| [#14529](#14529) | Bug fix restoring tab/panel state across project switches. |
| [#14531](#14531) | Reconnect recovery bug fix, restores already-documented behavior. |
| [#14532](#14532) | Bug fix keeping session tab title in sync on rename. |
| [Kilo-Org/cloud#6088](Kilo-Org/cloud#6088) | Removes internal/admin model-experiment surfaces, not public product docs. |
| [Kilo-Org/cloud#6682](Kilo-Org/cloud#6682) | Internal control-socket reconnect race fix, no user-facing change. |
| [#14534](#14534) | Transcript re-render performance bug fix. |
| [#14535](#14535) | Bug fix preserving the loaded browser page across context switches. |
| [Kilo-Org/cloud#6684](Kilo-Org/cloud#6684) | Reverted by Kilo-Org/cloud#6685. |
| [Kilo-Org/cloud#6678](Kilo-Org/cloud#6678) | Dead-code constant removal, no user-visible effect. |
| [Kilo-Org/cloud#6687](Kilo-Org/cloud#6687) | Removes internal model-experiment maintenance and retains tables, no user-facing change. |
| [#14515](#14515) | JetBrains plugin unload crash fix, no documented behavior change. |
| [#14520](#14520) | JetBrains transcript/list rendering performance work. |
| [Kilo-Org/cloud#6614](Kilo-Org/cloud#6614) | Mobile PR Review header and session title bug fix, no doc change needed. |
| [Kilo-Org/cloud#6625](Kilo-Org/cloud#6625) | Internal mobile secure-store error-handling refactor. |
| [Kilo-Org/cloud#6624](Kilo-Org/cloud#6624) | Mobile auth bug fix that stops a retry loop; restores expected sign-in behavior with no new setting or workflow. |
| [#14310](#14310) | Contributor/CI fix making the kilo-v2 checkout installable; not user-visible product behavior. |
| [Kilo-Org/cloud#6611](Kilo-Org/cloud#6611) | Mobile notification-tap fix that selects the session's organization; restores correct behavior rather than adding a learnable feature. |
| [Kilo-Org/cloud#6644](Kilo-Org/cloud#6644) | Mobile sign-in layout/alignment polish; no change to what a user must do. |
| [Kilo-Org/cloud#6601](Kilo-Org/cloud#6601) | Mobile layout fix keeping empty states clear of the tab bar; purely visual. |
| [#14543](#14543) | CI/release infrastructure adding Windows binary code signing; no public docs impact. |
| [Kilo-Org/cloud#6616](Kilo-Org/cloud#6616) | Mobile visual defect fixes and a session-title fallback; no new user workflow or setting. |
| [Kilo-Org/cloud#6630](Kilo-Org/cloud#6630) | Reports an edge-case partial worktree restore; failure-path plumbing with no new user-facing workflow, target setting, or config. |
| [Kilo-Org/cloud#6699](Kilo-Org/cloud#6699) | Cloud Agent e2e stabilization plus internal idle-sandbox capacity handling; not user-visible. |
| [#14545](#14545) | Automated JetBrains release/changelog PR; underlying user-facing changes are triaged from their own PRs. |
| [Kilo-Org/cloud#6708](Kilo-Org/cloud#6708) | Internal AI-gateway request-logging policy change in the admin panel; no existing public docs surface and no change to how users run Kilo Code. |
| [#14533](#14533) | Documentation already shipped with the merged PR. The experimental.task_model_selection flag is gone from the current source, and pages/code-with-ai/agents/model-selection.md, pages/code-with-ai/agents/context-mentions.md, and pages/getting-started/settings/index.md already describe per-task selection as default-on with no stale experiment references. |
| [#14510](#14510) | Documentation already shipped with the merged PR. Marketplace companion-skill support is present in the current source (packages/opencode/src/kilocode/marketplace/companions.ts and installer), and pages/customize/marketplace.md already documents installing, publishing, and removing MCP servers with companion skills. |
<!-- docs-sync:skipped:end -->

---

(bot) Generated by the docs-sync workflow. Humans review and merge; while this PR stays open, the next daily run appends new changes here. Branch: `docs/auto-sync-2026-09-25`.
<!-- docs-sync: processed-through 2026-09-25T07:05:29.302Z -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants