Skip to content

fix(core): accept string expires_in from the token endpoint - #366

Merged
Dustin-Kelley merged 5 commits into
mainfrom
fix/auth-token-expires-in-string
Aug 27, 2026
Merged

fix(core): accept string expires_in from the token endpoint#366
Dustin-Kelley merged 5 commits into
mainfrom
fix/auth-token-expires-in-string

Conversation

@Dustin-Kelley

@Dustin-Kelley Dustin-Kelley commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Sign-in after the YouVersion portal was completing the code redirect, then dropping the session so the user stayed signed out with ?code= still in the URL.

Root cause

#341 (e9840fc5) started validating /auth/token JSON with Zod and required expires_in to be a number.

Before that PR, the SDK JSON.parse’d the body and type-asserted it. A string like "3599" still worked at runtime ("3599" * 1000). After #341, Zod rejected the live payload, the exchange catch path cleared tokens, and the failure was easy to miss.

The live token endpoint (and the platform docs) still send expires_in as a string. Tests only used a numeric fixture, so CI did not catch it.

Solution

  • Coerce expires_in to a number on both the token-exchange and token-refresh schemas.
  • Surface a clear parse error instead of a raw Zod throw.
  • Point the token-exchange test fixture at the live string shape.
  • Log callback failures to the console so a failed exchange is visible.

Test plan

  • Sign in on http://localhost:5173 (not 127.0.0.1)
  • After clicking your name on the YouVersion portal, land signed in (name + Sign out, no ?code= in the URL)

Greptile Summary

The PR updates token exchange and refresh validation to accept the live endpoint’s string-shaped expires_in while retaining strict validation.

  • Adds a shared positive-integer expiry schema for numeric and digit-string values.
  • Improves token-exchange validation errors and development callback diagnostics.
  • Adds schema coverage and updates exchange fixtures to match the live response shape.
  • Adds coordinated patch changesets for core and React hooks.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/core/src/schemas/auth.ts Introduces a narrowly constrained positive-integer expiry schema shared by token exchange and refresh responses, resolving both prior findings.
packages/core/src/schemas/auth.test.ts Covers numeric and digit-string expiry values and rejects malformed, non-positive, and fractional inputs.
packages/core/src/Users.ts Uses safe parsing to provide a clearer token-exchange contract error while preserving validated token handling.
packages/hooks/src/context/YouVersionAuthProvider.tsx Adds development-only callback error logging without changing the provider’s existing error state behavior.
packages/core/src/tests/Users.test.ts Aligns token exchange fixtures and test typing with the live endpoint’s string expiry representation.
packages/hooks/src/context/YouVersionAuthProvider.test.tsx Updates callback failure coverage for the development-only logging behavior.
.changeset/auth-expires-in-string.md Records coordinated patch releases for the affected core and hooks packages.

Reviews (4): Last reviewed commit: "fix: require positive integer expires_in..." | Re-trigger Greptile

Context used (4)

#341 started validating the token JSON with Zod and required a number.
The live endpoint still sends a string, so sign-in parsed, threw, and
left the user signed out.

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

changeset-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: d2fd328

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@youversion/platform-core Patch
@youversion/platform-react-hooks Patch
@youversion/platform-react-ui Patch
vite-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8982b8b89c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/core/src/schemas/auth.ts Outdated
Comment thread packages/core/src/schemas/auth.ts Outdated
Reject null, booleans, and empty strings that z.coerce.number() turned into 0.

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

Copy link
Copy Markdown
Collaborator

Idk what greppy is crying about but I like this

Comment thread packages/core/src/schemas/auth.ts Outdated
0, "0", and negatives persisted an already-expired session.

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

@cameronapak cameronapak 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.

Spec

No Jira ticket. Spec is the PR body. #341 rejected string expires_in and sign-in dropped.

No must-fix. Counts: 0 must-fix. 1 question. 0 nits.
Worst issue: refresh still uses .parse() and wraps any Error. Exchange now has a dedicated invalid-response message. The spec asked for a clear parse error. It said "both" only for coerce.

Required items land. Both schemas share TokenExpiresInSchema. Exchange fixture is '3599'. Callback logs the failure. Patch changeset covers core and hooks. null, false, and '' now fail.

Standards

No must-fix. Counts: 0 must-fix. 2 questions. 1 nit.
Worst issue: console.error on the auth callback is unconditional in a published hooks package. Other SDK logs stay behind a dev gate.

The z.number() arm still admits 0 and negatives. Those persist an already-dead session. The digit-string arm does not.

Nit: exchange uses safeParse. Refresh still uses .parse(). Same lifetime field, different error shape.

Code Reviewer bot, sent on behalf of Cam.

Comment thread packages/hooks/src/context/YouVersionAuthProvider.tsx Outdated
Comment thread packages/core/src/schemas/auth.ts Outdated
Reject 0, negatives, and floats. Log auth callback failures only in
development, matching Users.ts and client.ts.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread packages/hooks/src/context/YouVersionAuthProvider.tsx
@Dustin-Kelley Dustin-Kelley self-assigned this Aug 27, 2026
@Dustin-Kelley
Dustin-Kelley merged commit 1f258fa into main Aug 27, 2026
13 checks passed
@Dustin-Kelley
Dustin-Kelley deleted the fix/auth-token-expires-in-string branch August 27, 2026 16:56
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.

3 participants