Skip to content

remote/oidc: identity client + browser opener + login orchestrator (T3, T5) - #198

Merged
platypii merged 2 commits into
oidc-client-loginfrom
chunk2-identity-oidc-login
Jun 29, 2026
Merged

remote/oidc: identity client + browser opener + login orchestrator (T3, T5)#198
platypii merged 2 commits into
oidc-client-loginfrom
chunk2-identity-oidc-login

Conversation

@platypii

Copy link
Copy Markdown
Contributor

Chunk 2 of the OIDC client login (LLP 0046-0048), completing milestone-1 primitives. Stacked on #197 (base is chunk1-pkce-loopback); review/merge after #197.

What

  • identity_client.js - exchangeCode / refreshSession over an injectable fetch against <origin>/v1/identity/token; a 401 invalid_grant becomes a typed InvalidGrantError. Field is access_jwt, expires_at is ISO; no external JWKS (LLP 0047 wire contract).
  • open_browser.js - platform opener (open / xdg-open / cmd start), detached; returns whether an opener was found (LLP 0046 D8).
  • oidc_login.js - loginWithBrowser() composes PKCE + random state + loopback + /login/start + exchange, returns the session (no persistence) (LLP 0046 D2/D3).
  • types.d.ts - OidcSession / RefreshedAccess.

Tests

3 new test files, 13 tests green; npm run typecheck clean.

🤖 Generated with Claude Code

Base automatically changed from chunk1-pkce-loopback to oidc-client-loginJune 29, 2026 18:33
… T5)
Completes milestone-1 local primitives, composing chunk 1's PKCE +
loopback:
- identity_client.js: exchangeCode / refreshSession over an injectable
fetch against <origin>/v1/identity/token; a 401 invalid_grant surfaces
a typed InvalidGrantError. Response field is access_jwt, expires_at is
ISO. No external JWKS on the client.
- open_browser.js: platform opener (open / xdg-open / cmd start),
detached; returns whether an opener was found (LLP 0046 D8).
- oidc_login.js: loginWithBrowser() orchestrates PKCE + a random state,
starts the loopback receiver, builds the /login/start URL, opens the
browser (or prints the URL), awaits the code, and exchanges it for the
session. No persistence; the caller stores it (LLP 0046 D2/D3).
- types.d.ts: OidcSession / RefreshedAccess shared interfaces.
Unit tests cover the token request bodies + response mapping, the
invalid_grant typing, each platform opener, and the full
PKCE->loopback->exchange orchestration including --no-browser and
loopback cleanup on failure.
@platypii

Copy link
Copy Markdown
ContributorAuthor

Review (independent agent review)

Verdict: request-changes → addressed. The identity client and login orchestrator are faithful to the LLP 0047 wire contract (correct grants, access_jwt/expires_at parsing, typed invalid_grant only on 401, no secret logging, clean try/finally loopback cleanup, proper URL/S256 start-URL building). Conventions clean.

Findings

  • blocker — open_browser.js crashed on a missing opener.spawn delivers a missing-binary failure (ENOENT) as an async 'error' event, not a synchronous throw. With no listener attached, that became an uncaught exception that crashed the CLI — exactly the D8 "no opener, print the URL" path. Verified empirically by the reviewer. The failure test was tautological (threw synchronously, which real spawn never does), masking it.
  • minor — Windows & in the URL via cmd /c start "" <url> (unquoted query separators).
  • minor — expires_at validated only as a non-empty string, not a parseable ISO timestamp.

Addressed in this PR

  • open_browser.js now attaches child.on('error', () => {}) so the async failure can never crash the process; the boolean return is documented as best-effort (a silently-failed launcher still returns true), with the loopback timeout + printed URL as the real backstops.
  • The login orchestrator now always prints the URL as a fallback even on the "opened" path, so a silently-failed opener never strands the user.
  • The failure test now models spawn's async 'error' emission (an EventEmitter child) and asserts the process does not crash; a separate test keeps the synchronous-throw path.

The expires_at ISO-parse hardening is effectively covered downstream: the attach-path freshness check (PR #201) treats an unparseable expiresAt as stale and refreshes. The Windows &-quoting nit is noted as a follow-up. 14 tests green, typecheck clean.

Review follow-ups (PR #198):
- spawn delivers a missing-opener failure as an async 'error' event, not
a synchronous throw. Without a listener it became an uncaught exception
that crashed the CLI on the D8 no-opener path. Attach child.on('error')
to swallow it; document the boolean return as best-effort.
- oidc_login now always prints the URL as a fallback even when the opener
reported success, so a silently-failed launcher never strands the user.
- test now models spawn's async 'error' emission (EventEmitter child) and
asserts no process crash; keep the synchronous-throw case too.
@platypii
platypiiforce-pushed the chunk2-identity-oidc-login branch from d0ea83e to a60d2d6CompareJune 29, 2026 18:36
@platypii
platypii merged commit c4c4ca4 into oidc-client-loginJun 29, 2026
6 checks passed
@platypii
platypii deleted the chunk2-identity-oidc-login branch June 29, 2026 18:36
@platypii
platypii restored the chunk2-identity-oidc-login branch June 29, 2026 18:38
@platypii
platypii deleted the chunk2-identity-oidc-login branch June 29, 2026 18:39
bgmcmullen added a commit that referenced this pull request Aug 19, 2026
`hyp init` no longer starts an AI client on its closing question. It may
have been invoked by an installer or from a home, temp, or unrelated
directory, and handing that directory to a new agent session is not
setup's call to make. It prints the questions and names the verb; the
launch waits for an explicit `hyp ask` run from a directory the user
chose.
LLP 0198 rewritten to the printed-list decision, with #re-runnable
re-argued: it justified `hyp ask` as the durable form of a wizard menu
that no longer exists.
LLP 0203's ordering rested on the deleted launch ("may take the terminal
for good"), in #context, #offer, and #child-process. Re-argued on the
reason that survives: a prompt must not land under a block the reader has
started scrolling past. Its #frame why-not rejected a second framed
block; the frame now belongs to `hyp ask`'s menu, so the rejection is
that framing is not a wizard idiom at all.
LLP 0135's forward-ref claimed this "extends" #privacy's closing-words
rule. It narrows it: the narration stays the last thing said about
privacy, not the last thing on screen. 0198 records the cost.
Also:
- One `writeSuggestedPrompts({ stdout, footer, hasRows })` replaces three
renderers whose bodies were literal string copies, so the empty-cache
sentence cannot drift out of agreement with itself.
- `first_ask` on `wizard.finish` was derivable from `pathway` and
`cancelled`. It now carries `listed-empty`, making the rate of installs
finishing with an empty cache the signal setup contributes.
- `RunWizardFirstAskOptions.cwd` was set by nothing but a test, in a
change whose whole rationale is working directories. Dropped from the
wizard-level type; `launchClient` keeps it, with its own test.
- The four questions rewritten: one subject, one criterion, a closing
clause asking for the mechanism rather than restating the subject.
`which` for things in the recorded history, `what` for things being
proposed. The `Based on the hypaware logs.` fragment becomes a `From my
HypAware history,` clause, which names the product rather than a
dataset the user has never seen.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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