Skip to content

remote/oidc: silent refresh + one-shot 401 retry on the attach path (T7) - #201

Merged
platypii merged 2 commits into
oidc-client-loginfrom
chunk5-attach-refresh
Jun 29, 2026
Merged

remote/oidc: silent refresh + one-shot 401 retry on the attach path (T7)#201
platypii merged 2 commits into
oidc-client-loginfrom
chunk5-attach-refresh

Conversation

@platypii

Copy link
Copy Markdown
Contributor

Chunk 5 of the OIDC client login (LLP 0046-0048), milestone 3. Stacked on #200 (base chunk4-login-command); review/merge after #197-#200.

What (LLP 0046 D5)

  • remote_verb.js resolves the bearer via resolveAccessJwt (not resolveToken), passing the origin-derived identity base; a stale cached JWT is refreshed + persisted before the call.
  • client.js tags 401/403 rejections (authError, status). On a live auth error for an oidc/file session, the attach path forces one refresh and retries once; an env override / static token is surfaced as-is.
  • A refresh that fails invalid_grant surfaces remote session expired - re-run 'hyp remote login <target>'.
  • resolveAccessJwt gains forceRefresh; deriveIdentityBase relocated to credentials.js as the single shared D6 definition (login command + attach path both import it).

Tests

remote-verb-oidc.test.js (4): stale-refresh-before-call, exactly-one mid-flight refresh+retry, invalid_grant -> re-login, static-not-retried. Full npm test green (1527 pass); typecheck clean.

🤖 Generated with Claude Code

@platypii
platypii changed the base branch from chunk4-login-command to oidc-client-loginJune 29, 2026 18:38
The query attach path becomes session-aware (LLP 0046 D5):
- remote_verb.js resolves the bearer via resolveAccessJwt (not
resolveToken), passing the origin-derived identity base. A stale cached
JWT is refreshed and persisted before the call.
- client.js tags a 401/403 rejection (authError + status) so the attach
path can recognize it. On a live auth error for an oidc/file session,
the attach path forces a single refresh and retries once; an env
override or static token is surfaced as-is (nothing to refresh).
- A refresh that fails invalid_grant surfaces "remote session expired -
re-run 'hyp remote login <target>'".
- resolveAccessJwt gains forceRefresh for the retry; deriveIdentityBase
moves to credentials.js as the shared home for the login command and
the attach path (single D6 definition).
Tests: a stale stored JWT refreshes + persists pre-call; a mid-flight
401 triggers exactly one refresh + retry; invalid_grant maps to
re-login guidance; a static 401 is not retried. Full suite green (1527).
@platypii

Copy link
Copy Markdown
ContributorAuthor

Review (independent agent review)

Verdict: request-changes → addressed. The silent-refresh and one-shot 401 retry are well-structured: retry is exactly once (no loop), isAuthError keys on the authError/status tag from client.js, refreshability is correctly gated to kind === 'oidc' && source === 'file' (env/static surfaced as-is), a tool-level isError is a normal return (never retried), and deriveIdentityBase has a single definition with no layering inversion.

Finding (major)

  • The initialresolveAccessJwt was not wrapped in try/catch. When an oidc target's stored JWT is already stale, the initial resolve does a pre-call refresh; if the refresh row is revoked that throws InvalidGrantError, which escaped runRemoteVerb (and verb_command.js) as an unhandled rejection instead of D5's clean re-login guidance — the canonical "you're logged out" case. The existing invalid_grant test used expiresAt: FUTURE, so it only exercised the mid-flight 401 path and missed this.

Addressed in this PR

  • The initial resolve is now wrapped and mapped exactly like the retry path, via a shared mapRefreshError(err, target) helper (invalid_grant → re-login + exit 2; other → generic + exit 1).
  • Added a unit test with expiresAt: PAST + a revoked refresh asserting exit 2 and the re-login message.

(This fix was originally delivered in the T8 follow-up; it has been relocated here so #201 is correct standalone.) 11 tests green, typecheck clean.

…01 retry)
Review follow-up (PR #201): the initial resolveAccessJwt can itself
refresh (and throw invalid_grant) when the stored JWT is already stale,
which escaped as an unhandled rejection. Wrap it and map via a shared
mapRefreshError helper, exactly like the mid-flight 401 retry path. Added
a stale-JWT pre-call-refresh-fails unit test.
@platypii
platypiiforce-pushed the chunk5-attach-refresh branch from e6211c1 to f6f80d9CompareJune 29, 2026 18:45
@platypii
platypii merged commit 197ce4a into oidc-client-loginJun 29, 2026
6 checks passed
@platypii
platypii deleted the chunk5-attach-refresh branch June 29, 2026 18:45
Sign up for freeto 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

@platypii