Skip to content

fix(passport): silent refresh on expired access — restore the 90-day re-verify UX#23

Merged
vvillait88 merged 9 commits intomainfrom
0.1.1
May 7, 2026
Merged

fix(passport): silent refresh on expired access — restore the 90-day re-verify UX#23
vvillait88 merged 9 commits intomainfrom
0.1.1

Conversation

@vvillait88
Copy link
Copy Markdown
Contributor

Summary

Two related bugs in the silent-refresh path were forcing verify-URL prompts every 24h. Confirmed live by reproducing the user-visible flow against martin-estate, fixed, and re-verified end-to-end.

Bug Aattach.ts:48 short-circuited and returned kind: 'expired' BEFORE attempting to use the still-valid refresh_token. Pay's caller (pay.ts) then drove bootstrap reauth via the verify-URL flow, even though a silent exchange would have worked.

Bug BREFRESH_THRESHOLD_MS = 60 * 1000 (60 seconds). Proactive refresh window was unreachably short on a 24h access TTL — only fired if pay happened to be invoked in the last 60 seconds of the access token's lifetime.

Fix

Restructured attachPassport to evaluate accessExpired and accessNearExpiry up-front, then attempt refresh in either case when refresh_token is still valid. Only after refresh has been tried (or skipped) do we surface kind: 'expired', and only when access is genuinely expired post-attempt — the path that drives bootstrap reauth.

Refresh failures are now graceful when access is still valid: a proactive failure with 30s remaining attaches the existing token rather than driving eager bootstrap. A reactive failure with already-expired access surfaces 'expired' and bootstraps as before.

REFRESH_THRESHOLD_MS bumped from 60s to 5 minutes (clock-skew + on-the-wire-latency headroom for proactive refreshes).

Downstream effects (also in this PR)

expiringSoon predicate — after silent refresh works, every refresh issues a 24h access token, so the existing predicate (access_remaining < 5d) was always true. The "Passport expires soon — run passport login to renew" warning would print on every pay call telling users to do something they don't need to. Tightened the predicate to gate on refresh state: expiringSoon = true only when refresh isn't going to bail us out (no refresh_token, or refresh itself within the 5-day window).

passport status output — was reporting only access-token expiry. Agents reading expires_in_days: 0 would think they had to act when actually 89 days of effective life remained via refresh. Extended the output with silent_refresh_available, refresh_expires_at, refresh_expires_in_days (additive — existing fields unchanged). Same shape extension to passport login output via a shared buildPassportSummary helper.

agent-guide — golden_path step 1 now describes the access + refresh lifecycle explicitly. Auxiliary passport status step describes the new fields and gives an unambiguous instruction: do not surface expires_in_days: 0 as actionable when silent_refresh_available: true.

README.mdpassport status row mentions the new fields.

Tests

tests/passport-attach.test.ts rewritten:

  • Reactive path — silently refreshes when access already expired but refresh_token still valid (the dominant real-world case)
  • Proactive path — refreshes within the 5-min threshold of expiry
  • Refresh failure paths — proactive failure with valid access stays attached; reactive failure with expired access surfaces 'expired'
  • Legacy / refresh-expired / skipRefresh paths unchanged
  • expiringSoon=false with short-lived access + valid refresh (the post-fix case that would've spammed warnings)
  • expiringSoon=true when both access AND refresh are within the 5d window (the genuinely-actionable case)

Old tests pinned the broken 60s window and the early short-circuit; new tests cover both paths.

Live verification

End-to-end against the published agents.martinestate.com API:

$ # Force-set passport access expired 1h ago, refresh still valid for 89 days
$ ./dist/index.js pay POST https://agents.martinestate.com/purchase ... --dry-run --json

# stderr: empty (no "Stored Passport has expired" prompt)
# Tokens rotated: opc_old → opc_new, prt_old → prt_new
# silent_refresh_available stays true

Pre-fix: this would have printed Open this URL to renew: and forced a browser click. Post-fix: silent rotation happens transparently.

Test plan

  • bun run typecheck clean
  • bun run lint clean
  • bunx vitest run — 438+ tests pass (4 new on the silent-refresh + expiringSoon paths; existing tests rewritten for the new contract)
  • Lefthook pre-commit + pre-push pass
  • Live smoke against agents.martinestate.com: forced expiry → silent refresh fires → tokens rotated → no user prompt
  • CI green
  • Tag v0.1.1 after merge → npm publish to latest dist-tag (no - in version, publish workflow takes the stable branch)

Cross-repo follow-up

Mintlify docs in agentscore/core (docs/integrations/pay-cli.mdx line 141, docs/passport.mdx lines 60 + 76) had pre-existing inaccuracies about the token TTL ("Tokens default to a 30-day TTL" — wrong; was always 24h access + 90d refresh). Will be addressed in a separate core PR.

🤖 Generated with Claude Code

vvillait88 and others added 9 commits May 7, 2026 08:16
Two related bugs in the silent-refresh path were silently forcing
verify-URL prompts after every 24h of agent inactivity, defeating
the 90-day refresh_token UX the system was designed for.

Bug A (the one users hit). When the access token had expired, the
attach handler short-circuited and returned `kind: 'expired'` BEFORE
attempting to use the still-valid refresh_token. The caller then drove
bootstrap reauth via the verify-URL flow, even though a silent
exchange would have worked.

Bug B (latent). The proactive refresh window was 60 seconds —
unreachably short on a 24h access TTL. Bumped to 5 minutes (covers
clock-skew + on-the-wire-latency without over-rotating).

Restructured `attachPassport` to evaluate accessExpired vs
accessNearExpiry up-front, then attempt refresh in either case when
the refresh_token is still valid. Only after refresh has been tried
(or skipped) do we surface `kind: 'expired'`, and only when access is
genuinely expired post-attempt — the path that drives bootstrap.

Refresh failures are now graceful when access is still valid:
proactive failure with 30s remaining attaches the existing token
rather than driving eager bootstrap. Reactive failure with already-
expired access surfaces 'expired' and bootstraps as before.

Tests rewritten. Old tests pinned the broken 60s window and the early
short-circuit; new tests cover both reactive and proactive paths plus
the failure modes (revoked refresh, legacy passport without refresh,
refresh_token expired).

Net UX after this fix: re-verify needed approximately every 90 days
(when refresh_token TTL expires), not every 24h.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tatus output, agent-guide

Three follow-on changes the silent-refresh fix forced.

1. expiringSoon predicate

After silent refresh works, every refresh issues a 24h access token,
so `(access_remaining < 5d)` is always true on a freshly-refreshed
passport — the misleading `Passport expires soon — run \`passport
login\` to renew` warning would print on every pay call. Tightened
the predicate to gate on refresh state: expiringSoon=true only when
refresh isn't going to bail us out (no refresh_token, or refresh
itself within the 5-day window). The warning now fires only when the
user genuinely needs to re-verify in browser.

2. passport status output

Was reporting only access-token expiry — agents reading
`expires_in_days: 0` would think they had to act when actually 89
days of effective life remain via refresh. Extended the output with
`silent_refresh_available`, `refresh_expires_at`,
`refresh_expires_in_days`. Same shape extension to passport login
output (via a shared `buildPassportSummary` helper).

3. agent-guide

The auxiliary section on `passport status` now describes the new
fields explicitly, plus an unambiguous instruction: do not surface
"expires in 0 days" as an actionable warning when
silent_refresh_available is true. Step 1 already gained the
refresh-token lifecycle clarification in the prior commit on this
branch.

Tests: two new cases on expiringSoon behavior — false with
short-lived access + valid refresh, true when both are within the
5d window. Existing passport-commands tests still pass against the
extended status shape (the new fields are additive).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Patch release for the silent-refresh fix + downstream effects.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reflects the extended PassportStatusOutput shape from this release —
silent_refresh_available, refresh_expires_at, refresh_expires_in_days.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two changes:

1. New `passport_login_required` envelope: when a stored Passport's access
   token has expired AND silent refresh did not succeed, non-TTY callers
   (agents in --json, MCP, scripted contexts) now get a structured
   { code: passport_login_required, next_steps.action: passport_login }
   envelope instead of blocking up to an hour on the inline browser-redirect.
   Humans at a terminal still get the inline bootstrap.

   Aligns with the API's own /v1/sessions/refresh failure message ("Drive
   the inline reauth flow to mint a new Passport").

2. MCP descriptions across the passport group + `pay` now mention the
   24h+90d lifecycle and silent rotation, so an agent reading tool docs
   (rather than `agent-guide`) still gets the picture. agent-guide step 1
   + identity_error_recovery catalog updated with the new envelope.

Tests: 4 new in pay-passport-expired.test.ts (442 total, +4).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same UX-cliff fix as the expired-access path: when a merchant returns 403
with bootstrap fields (verify_url + session_id + poll_secret) and the agent
has no usable Passport, non-TTY callers now get a structured
{ code: passport_required_by_merchant, next_steps.action: passport_login }
envelope with the merchant URL in extra, instead of blocking ~1h on the
inline browser flow. Humans at a terminal still drive bootstrap inline.

Verified shape against the canonical commerce SDK denial body — every
node-commerce + python-commerce middleware variant (express, hono, fastify,
nextjs, web, fastapi, flask, django, aiohttp, sanic, asgi) routes through
the shared denialReasonToBody / denial_reason_to_body which emits
verify_url + session_id + poll_secret at the top level. Pay's
detectMerchantBootstrap reads top-level fields — matches all 11.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rchant

Both pay-thrown codes (added in this PR) are now in the README's identity
error-codes section with extra-field shape and recovery actions. Distinct
from the SDK-thrown table since these fire from the `pay <url>` command
itself, not from the SDK identity wrappers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vvillait88 vvillait88 merged commit 37787eb into main May 7, 2026
6 checks passed
@vvillait88 vvillait88 deleted the 0.1.1 branch May 7, 2026 16:44
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.

1 participant