Skip to content

fix(core): keep parked highlights when a forced refresh fails - #136

Merged
cameronapak merged 5 commits into
highlightsfrom
fix/queued-writes-survive-failed-refresh
Aug 14, 2026
Merged

fix(core): keep parked highlights when a forced refresh fails#136
cameronapak merged 5 commits into
highlightsfrom
fix/queued-writes-survive-failed-refresh

Conversation

@cameronapak

@cameronapak cameronapak commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Parked highlights stay painted when a forced token refresh fails. The sign-in prompt stays closed while auth is still loading.

Changes

  1. The drain keeps a parked write when getAccessToken({ force: true }) reports refresh-failed.
  2. The reader and the permission flow treat the token-loading window as not signed-out.
  3. getAccessToken accepts { force: true } so a caller can tell a mint from a leftover token.
  4. Core pins @youversion/platform-core at 2.6.2.
  5. The example app pins expo-application, and the docs match that.

Start here: change 1 — a leftover unexpired token is not a mint.

Flow

flowchart LR
  refuse[401 or 403] --> force["getAccessToken force"]
  force -->|ok| retry[Retry write]
  force -->|refresh-failed| keep[Backoff and keep paint]
  retry -->|refused again| drop[Drop and revert]
  retry -->|ok| land[Land write]
Loading
Under the hood

The drain used refreshNow() after a 401. That call swallows a failed refresh and leaves the old token in place. The next send uses the same stale token, the server refuses again, and the drain un-paints a real write.

getAccessToken({ force: true }) reports whether this call minted a token. refresh-failed takes ordinary backoff. The drain reverts only after a minted token is refused a second time.

Test plan

  • pnpm --filter @youversion/platform-react-native-expo-core exec jest src/auth/__tests__/auth-provider.test.tsx src/highlights/__tests__/highlight-queue-drain.test.ts src/highlights/__tests__/use-highlight-permission-flow.test.tsx --no-coverage (127 passed)
  • pnpm --filter @youversion/platform-react-native-expo-ui exec jest src/native/__tests__/bible-reader-highlights-prompts.test.tsx --no-coverage (18 passed)

Needs manual check: a parked write with a stale token and no network must stay painted after the drain wakes.

Made with Cursor

Greptile Summary

The PR preserves parked highlights when a forced token refresh fails and completes the earlier fix for highlight taps made during auth bootstrap.

  • Adds an outcome-reporting forced-token refresh path for queue retries.
  • Holds bootstrap-time highlight intents until auth becomes ready or definitively signed out.
  • Pins the updated platform-core and example-app Expo dependencies.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/core/src/auth/auth-provider.tsx Adds forced, outcome-reporting access-token refresh semantics while preserving transient failures as authenticated sessions.
packages/core/src/highlights/drain.ts Uses the forced token result to retry only after a confirmed mint and otherwise retains the parked write.
packages/core/src/highlights/use-highlight-permission-flow.ts Defers bootstrap-time applies until auth settles and safely abandons them on unmount or scope change.
packages/ui/src/native/bible-reader.tsx Holds highlight taps during auth bootstrap, then applies them for restored sessions or opens sign-in after confirmed sign-out.
packages/core/package.json Pins platform-core 2.6.2 with the lockfile updated consistently.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Tap[Highlight tap] --> Gate{Auth gate}
  Gate -->|settling| Hold[Hold intent]
  Hold -->|session restored| Apply[Apply highlight]
  Hold -->|signed out| Prompt[Open sign-in prompt]
  Gate -->|ready| Apply
  Gate -->|signed out| Prompt
  Refusal[Queue write receives 401/403] --> Force[Force token mint]
  Force -->|refresh failed| Keep[Keep parked write and paint]
  Force -->|mint succeeded| Retry[Retry write]
  Retry -->|refused again| Drop[Drop write and revert paint]
Loading

Reviews (4): Last reviewed commit: "fix(core): remint after a join without c..." | Re-trigger Greptile

Context used (3)

The drain treated a leftover token as a mint, then un-painted on the
second 401. getAccessToken({ force: true }) reports whether the mint
landed. Also skip the sign-in prompt while auth is loading, align
platform-core to 2.6.2, and pin expo-application in the example app.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cameronapak cameronapak self-assigned this Aug 14, 2026
cameronapak and others added 2 commits August 14, 2026 11:47
A type alias sat in the middle of the import block and tripped
import/first under --max-warnings=0.

Co-authored-by: Cursor <cursoragent@cursor.com>
A tap during the loading window skipped the sign-in sheet. When
bootstrap found no session the write reverted and the tap was lost.
Hold the intent until auth settles, then prompt or apply.

Co-authored-by: Cursor <cursoragent@cursor.com>

@bmanquen bmanquen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded — comments reposted below in Conventional Comments form.

@bmanquen bmanquen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two-axis review of highlights...HEAD, spec = this PR's description.

Standards: 2 hard (docs now contradict the code), 6 judgement calls.
Spec: 1 partial, 3 implementation concerns, 2 scope items.

Worst in each axis, kept separate rather than reranked against each other:

  • StandardsAGENTS.md:119 and docs/adr/0017:127 both quote the sign-in gate expression verbatim, and this branch changed it. AGENTS.md was edited here for getAccessToken({ force }), so the doc-sync habit was applied to core and skipped for UI.
  • Spec — change 2's loading-window guard sits behind the cached-grant short-circuit, so the exact case bc12822 describes still reverts silently for any consumer that isn't BibleReader.

Comments follow Conventional Comments. Two blocking, and 2 file-level comments posted alongside this review for findings whose lines fall outside the diff hunks.

Comment thread packages/core/src/highlights/use-highlight-permission-flow.ts Outdated
Comment thread packages/core/src/highlights/use-highlight-permission-flow.ts Outdated
Comment thread packages/core/src/auth/auth-provider.tsx Outdated
Comment thread packages/ui/src/native/bible-reader.tsx Outdated
Comment thread packages/ui/src/native/bible-reader.tsx Outdated
Comment thread packages/core/src/highlights/use-highlight-permission-flow.ts
Comment thread packages/core/src/highlights/use-highlight-permission-flow.ts
Comment thread packages/core/src/auth/auth-provider.tsx
Comment thread packages/core/src/auth/auth-provider.tsx Outdated
Comment thread packages/ui/src/native/bible-reader.tsx Outdated
Comment thread AGENTS.md
Comment thread docs/adr/0017-native-verse-action-sheet.md
cameronapak and others added 2 commits August 14, 2026 14:03
A returning grant is seeded before the token lands. Writing on that hint still reverts if bootstrap finds no session. Wait only while loading and unsigned, and remint after a force joins an in-flight refresh so the drain does not un-paint on a leftover.

Co-authored-by: Cursor <cursoragent@cursor.com>
The force path called refreshToken inside its own useCallback. ESLint treats that as access before declaration.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cameronapak
cameronapak merged commit 0961227 into highlights Aug 14, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants