Skip to content

remote/oidc: hermetic smoke + LLP follow-ups (T8) - #202

Merged
platypii merged 2 commits into
oidc-client-loginfrom
chunk6-smoke-llp
Jun 29, 2026
Merged

remote/oidc: hermetic smoke + LLP follow-ups (T8)#202
platypii merged 2 commits into
oidc-client-loginfrom
chunk6-smoke-llp

Conversation

@platypii

Copy link
Copy Markdown
Contributor

Chunk 6 (final) of the OIDC client login (LLP 0046-0048), milestone 4. Stacked on #201 (base chunk5-attach-refresh); review/merge after #197-#201.

What

  • remote_oidc_login smoke: one in-process server plays both the identity surface (/v1/identity/login/start + /token, signing real per-call tokens) and the MCP endpoint (/mcp). A scripted opener drives the real loopback redirect. Asserts the user-visible result and the remote-oidcsmoke_step telemetry across: browser login -> session stored kind: 'oidc' -> query attaches the JWT -> forced expiry -> silent refresh + persist -> revoked refresh -> re-login message.
  • Bug the smoke caught: when the stored JWT is already stale, the initialresolveAccessJwt refreshes pre-call and could throw invalid_grant outside the 401-retry try/catch. remote_verb now maps a refresh failure to re-login guidance on both the initial-resolve and mid-flight-401 paths (shared mapRefreshError); added a unit test.
  • LLP 0033 updated to note the kind discriminator + silent-refresh/401-retry (-> LLP 0046 D4/D5). LLP 0046/0047/0048 promoted Draft -> Accepted. All @refs resolve (ref-check clean).

Verification

npm test green (1528 pass), npm run smoke -- remote_oidc_login green, npm run lint clean, typecheck clean.

🤖 Generated with Claude Code

@platypii
platypii changed the base branch from chunk5-attach-refresh to oidc-client-loginJune 29, 2026 18:38
- remote_oidc_login smoke: one in-process server plays both the identity
surface (/v1/identity/login/start + /token, signing real per-call
tokens) and the MCP endpoint (/mcp, accepting only the current JWT). A
scripted opener drives the real loopback redirect. Asserts the
user-visible result and the remote-oidc smoke_step telemetry across:
browser login -> session stored kind: 'oidc' -> query attaches the JWT
-> forced expiry drives a silent refresh + persist -> a revoked refresh
row drives the re-login message.
- Fix surfaced by the smoke: when the stored JWT is already stale, the
initial resolveAccessJwt refreshes pre-call and can throw invalid_grant
outside the 401-retry try/catch. remote_verb now maps a refresh failure
to the re-login guidance on both the initial-resolve and the
mid-flight-401 paths (shared mapRefreshError). Added a unit test for
the stale-initial-resolve path.
- LLP 0033: its credential-store and attach sections now note the kind
discriminator and the silent-refresh + 401-retry behavior, pointing at
LLP 0046 D4/D5.
- Promote LLP 0046/0047/0048 Draft -> Accepted now that the milestones
land. All @refs resolve (ref-check clean).
@platypii

Copy link
Copy Markdown
ContributorAuthor

Review (independent agent review)

Verdict: approve-with-nits. Correct and well-tested final chunk. Both server roles are modeled faithfully (identity /login/start 302 → loopback redirect with code+state; /token serving both grants with fresh jwt-N; /mcp accepting only the current JWT). Assertions are authoritative: "exactly one refresh" is proven by state.refreshCalls, and the revoked path returns the re-login message with exit 2. forceExpiry round-trips the normalized record. Log-Driven Development honored with smoke_step markers + hyp_component assertions. LLP edits accurate; #d4/#d5 anchors resolve.

Note: after the stack reorg, the mapRefreshError fix now lands in #201 (its proper home); this PR is purely the smoke + LLP follow-ups.

Findings

  • minor — inline import('node:http').IncomingMessage type violated the repo's hard "no inline import() types" rule.
  • nits — runQuery took an unused cmd param; obs.shutdown() sat outside the try/finally; the stub relied on socket idle timeouts to close.

Addressed in this PR

  • Hoisted IncomingMessage to the top-level @import block.
  • Dropped the unused runQuery param; moved obs.shutdown() into the finally; added server.closeAllConnections() before close.

Smoke green, typecheck + lint clean.

Review follow-ups (PR #202):
- hoist the inline import('node:http').IncomingMessage type to the
top-level @import block (repo hard rule: no inline import() types).
- drop runQuery's unused cmd param.
- move obs.shutdown() into the finally and force-close keep-alive sockets
(closeAllConnections) so the server does not wait on idle timeouts.
@platypii
platypii merged commit a83a3ec into oidc-client-loginJun 29, 2026
0 of 6 checks passed
@platypii
platypii deleted the chunk6-smoke-llp branch June 29, 2026 18:48
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