Skip to content

feat: surface sign-in failures + add release-please automation - #1

Merged
nicknisi merged 9 commits into
mainfrom
dx-review-and-release-please
Jul 23, 2026
Merged

nicknisi merged 9 commits into
mainfrom
dx-review-and-release-please

Conversation

@claude

@claude claude Bot commented Jun 26, 2026

Copy link
Copy Markdown

Requested by Nick Nisi · Slack thread

A DX/correctness pass plus release automation for @workos/authkit-electron, ahead of its first publish.

Before / After

Sign-in failures (the headline change). Today, if a user denies the consent screen, closes the in-app sign-in window, or the authorization-code exchange fails, the callback completes with nothing to exchange — the SDK logs to the console and the renderer is left silently signed out, with no way to tell a failed attempt from "not signed in yet." A developer building a sign-in screen had no hook to show "Sign-in failed, try again."

After this change, every such failure is broadcast to the renderer as a safe { code, message } payload (never tokens). It surfaces as a new error field on useAuth():

const { error, signIn } = useAuth();
return (
  <>
    {error && <p role="alert">Sign-in failed: {error.message}</p>}
    <button onClick={() => signIn()}>Sign in</button>
  </>
);

error clears automatically on the next successful sign-in.

Sturdier registerProtocol(). Calling it more than once (easy to do across re-inits or HMR) previously wired a second set of open-url/second-instance listeners and leaked the first, so a single callback could be handled twice. It is now idempotent — the listeners are wired exactly once, and the JSDoc that wrongly claimed it "returns the deep-link cleanup" now matches its void return.

No more double failure signal. When the in-app auth window fails to load its page, the thrown error is now the single failure signal; the window-close handler no longer also emits a synthetic window_closed callback.

Cleaner release story. The package can now be cut and published with the same automation the rest of the AuthKit SDKs use, instead of having no release path at all (version was pinned at 0.0.0 with no changesets/release tooling).

How

  • A new SDK-owned IPC channel authkit:auth-error (main → renderer) carries an AuthErrorPayload ({ code, message } only). broadcastAuthError() mirrors the existing broadcastAuthChange() chokepoint, so the refresh-token confinement is untouched — no tokens cross this channel.
  • createAuthKit() emits an auth-error on a callback error param (including the window ceremony's window_closed cancellation) and on a failed completeCallback (state verification or code exchange).
  • The preload bridge gains onAuthError(); AuthKitProvider subscribes to it, exposes error via context, and clears it on the next successful auth change.
  • AuthErrorPayload is exported from the root, /react, and /preload entries; broadcastAuthError from /internals.
  • registerProtocol() guards on an internal protocolRegistered flag that cleanup() resets.
  • Comment scrub: removed leftover internal "Phase 1/2/3/4" roadmap wording from shipped source and specs (no behavior change).
  • Tests added/updated for: the auth-error broadcast (callback error param, failed exchange, unknown state), registerProtocol idempotency, the preload onAuthError subscription, and the useAuth().error surface + clear-on-success. Full suite: 136 tests, 15 files, green.

Release tooling

Mirrors authkit-nextjs exactly:

  • .github/workflows/release-please.yml — runs release-please on pushes to main, opening/maintaining a release PR; on merge it triggers the publish job.
  • .github/workflows/release.yml — reusable workflow that runs pnpm publish --access=public --provenance (correct for the scoped public @workos/... package; provenance via OIDC id-token: write).
  • .github/workflows/lint-pr-title.yml — conventional-commit PR-title lint (commits in this PR already follow the convention).
  • release-please-config.json — release-type: node, single "." package.
  • .release-please-manifest.json — pinned at 0.0.0; the first release computes the version from the conventional commits since.

First-release steps a maintainer must do (these are why this is a draft):

  1. GitHub App token — release-please.yml uses vars.SDK_BOT_APP_ID + secrets.SDK_BOT_PRIVATE_KEY (the same app authkit-nextjs uses). Confirm that app is installed on this repo, or swap in another token source.
  2. npm publish auth — the publish job relies on OIDC trusted publishing (--provenance, id-token: write). Configure npm trusted publishing for @workos/authkit-electron, or add an NPM_TOKEN/NODE_AUTH_TOKEN secret and a registry-auth step. (authkit-nextjs does not set NODE_AUTH_TOKEN explicitly, so it appears to use trusted publishing — confirm this matches the org's npm setup.)
  3. The first conventional commit after merge (this PR's feat: will count) drives the initial version release-please proposes.

Follow-ups (not done here — low risk, but out of scope for a tight diff)

  • getAccessToken runs a full validate/refresh (src/main/session-manager.ts): it delegates to getUser() (decrypt → validateAndRefresh → persist). Left as-is on purpose — that is the correct "refresh-on-read" guarantee, and trimming it risks returning a stale/expired token. Revisit only if profiling shows the extra refresh round-trips matter.
  • socket-tier1-analysis.yml (Socket reachability scan) exists in authkit-nextjs but was not ported: it requires the org-level secrets.SOCKET_API_KEY, which this task can't assume is provisioned for this repo. Add it separately once that secret is confirmed available.

Open questions (need a human/product decision — intentionally not changed)

  • npm scope. This package publishes as @workos/authkit-electron (like @workos/authkit-session), whereas the framework SDKs use @workos-inc/.... Left unchanged — confirm the intended scope before first publish, since it affects the publish config and trusted-publishing setup.
  • switchToOrganization while signed out. When called with no session it returns { user: null }, and the IPC handler then broadcasts a signed-out payload — which could sign other windows out. Possibly fine, possibly surprising; left as-is pending a product call.
  • as unknown as WorkOS / apiKey: '' widening at the authkit-session boundary (src/main/config.ts). Deliberate and documented; the real fix is upstream (authkit-session accepting a PublicWorkOS-compatible client and an optional apiKey). Left untouched — tracking as an upstream follow-up.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JT8sJBpwfAuRYKP7Lk53Hh


Generated by Claude Code

@nicknisi
nicknisi marked this pull request as ready for review June 26, 2026 14:51
@greptile-apps

greptile-apps Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR surfaces sign-in failures to the renderer via a new authkit:auth-error IPC channel, giving developers a stable useAuth().error field to display failure reasons instead of silently staying signed-out. It also makes registerProtocol() idempotent, fixes a double-error-signal in the window ceremony when loadURL throws, and adds full release automation (release-please, npm publish with OIDC provenance, PR-title lint).

  • Auth-error broadcast — a new broadcastAuthError() chokepoint (mirroring broadcastAuthChange) is called from every failure path: provider denial, window cancellation, failed code exchange, and ceremony open() failures. The payload is always { code, message } — no tokens cross the channel.
  • registerProtocol() idempotency — a protocolRegistered flag prevents duplicate open-url/second-instance listener registration across re-inits or HMR; cleanup() resets the flag.
  • Release automation — release-please.yml / release.yml / lint-pr-title.yml mirror the authkit-nextjs pattern; release-please-manifest.json pins version at 0.0.0 for the first conventional-commit-driven release.

Confidence Score: 5/5

Safe to merge; the auth-error broadcast is cleanly isolated and the token-safety invariant is preserved across all new paths.

The core changes — auth-error surfacing, registerProtocol idempotency, and window-ceremony double-signal fix — are all well-tested (136 tests, 15 files green). The new IPC channel carries no sensitive data, every broadcast path is covered by a unit test, and the existing refresh-token chokepoint is untouched. The release workflow is the only area worth a follow-up (no test step before publish), but that is a future-proofing concern rather than broken behavior.

release.yml — add a pnpm test step before Build project to guard against publishing a test-failing commit via workflow_dispatch.

Important Files Changed

Filename Overview
src/main/ipc-handlers.ts Adds broadcastAuthError and its companion sendToAllWindows helper; signIn handler now broadcasts an auth-error on !result.ok before returning the error result, closing the gap where a loadURL failure would leave useAuth().error empty. Token-isolation invariants are untouched.
src/main/create-auth-kit.ts Adds a broadcastError chokepoint mirroring broadcast, wires it into both the callback path (handleCallbackUrl) and the IPC handlers. Also introduces the protocolRegistered flag that makes registerProtocol() idempotent and properly reset by cleanup(). Logic is correct and well-tested.
src/main/ceremony/window.ts Sets captured = true before closing the window on a loadURL failure, preventing the closed handler from also emitting a synthetic window_closed callback. Eliminates the double-signal hazard cleanly without changing the happy path.
src/react/auth-kit-provider.tsx Adds a second useEffect subscription to onAuthError; correctly clears error on successful sign-in (payload.user !== null). Both subscriptions are cleaned up in the effect teardown. error is added to the useMemo dependency array.
src/shared/types.ts Introduces AuthErrorPayload { code, message } — a deliberate token-free shape. Comment scrub removes leftover roadmap wording from shipped types. No behavior changes to existing types.
.github/workflows/release.yml Publish workflow installs, builds, and publishes but never runs pnpm test. Supports workflow_dispatch, so a manual trigger can publish without test verification. Job key test naming is also misleading — the job publishes, not tests.
.github/workflows/release-please.yml Standard release-please wiring using a GitHub App token for the bot and calling the reusable release workflow on publish. Permissions are scoped appropriately; id-token: write at the workflow level is required for the reusable workflow's OIDC publish job.
src/preload/index.ts Adds onAuthError to the contextBridge bridge, wiring it to IPC_CHANNELS.authError with proper unsubscribe. Re-exports AuthErrorPayload from the preload entry point so downstream consumers can type the callback without deep path imports.
src/main/tests/create-auth-kit.spec.ts Updates existing tests to assert auth-error broadcast instead of silence for unknown-state / error-param callbacks; adds four new specs covering code-exchange failure, ceremony open failure, and registerProtocol idempotency. Coverage is thorough.

Sequence Diagram

sequenceDiagram
    participant R as Renderer (useAuth)
    participant P as Preload Bridge
    participant I as IPC Handler (main)
    participant SM as SessionManager
    participant C as Ceremony (window/system)
    participant CB as handleCallbackUrl

    R->>P: signIn()
    P->>I: ipcRenderer.invoke(authkit:sign-in)
    I->>SM: beginSignIn()
    SM->>C: ceremony.open(authUrl)

    alt loadURL fails (network error)
        C-->>SM: throws Error
        SM-->>I: throws Error
        I->>I: "toResult() → { ok: false, error }"
        I->>P: "broadcastAuthError({ code, message })"
        P->>R: onAuthError → error state set
        I-->>P: "return { ok: false }"
        P-->>R: signIn() rejects
    else loadURL succeeds, user cancels window
        C-->>SM: open() resolves (loadURL done)
        SM-->>I: resolves null
        I-->>P: "return { ok: true }"
        P-->>R: signIn() resolves
        C->>CB: "emit(redirectUri?error=window_closed)"
        CB->>P: "broadcastAuthError({ code: window_closed })"
        P->>R: onAuthError → error state set
    else "Provider denies (callback has ?error=)"
        C->>CB: "deep-link / onCallback URL with ?error="
        CB->>P: "broadcastAuthError({ code, message })"
        P->>R: onAuthError → error state set
    else Code exchange succeeds
        C->>CB: "deep-link / onCallback URL with ?code=&state="
        CB->>SM: completeCallback(code, state)
        SM-->>CB: AuthResult
        CB->>P: broadcastAuthChange(RendererAuthPayload)
        P->>R: onAuthChange → user set, error cleared
    else Code exchange fails
        CB->>SM: completeCallback(code, state)
        SM-->>CB: throws Error
        CB->>P: "broadcastAuthError({ code: err.name, message })"
        P->>R: onAuthError → error state set
    end
Loading

Reviews (4): Last reviewed commit: "docs: fix JSX syntax in error handling e..." | Re-trigger Greptile

claude added 4 commits July 23, 2026 10:24
Mirror the authkit-nextjs release setup: a release-please workflow that
opens/maintains a release PR and, on merge, calls a reusable release
workflow to publish the scoped public package to npm with provenance.
Also add conventional-commit PR-title linting.

- .github/workflows/release-please.yml: release-please-action + GitHub App
  token, triggers publish on release_created
- .github/workflows/release.yml: pnpm publish --access=public --provenance
- .github/workflows/lint-pr-title.yml: semantic PR title check
- release-please-config.json: release-type "node", "." package
- .release-please-manifest.json: pinned at 0.0.0 (first release computes
  the version from conventional commits)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JT8sJBpwfAuRYKP7Lk53Hh
A denied/cancelled sign-in or a failed code exchange previously only
logged to console; the renderer never learned of it and stayed silently
signed out. Add an auth-error broadcast path so consumers can react.

- new SDK-owned IPC channel `authkit:auth-error` (main -> renderer)
- AuthErrorPayload type carrying only a safe { code, message } — no
  tokens; refresh-token confinement is unchanged
- broadcastAuthError() mirrors broadcastAuthChange(); createAuthKit emits
  it on a callback `error` param and on a failed completeCallback
- preload bridge exposes onAuthError(); AuthKitProvider tracks it and
  surfaces `error` on useAuth(), cleared on the next successful sign-in
- exported AuthErrorPayload from the root, /react and /preload entries;
  broadcastAuthError from /internals

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JT8sJBpwfAuRYKP7Lk53Hh
Housekeeping ahead of the first release; no runtime behavior change.

- remove leftover "Phase 1/2/3/4" roadmap wording from shipped source and
  spec comments (neutral wording instead)
- .oxfmtrc.json: drop the stale `docs/` ignore (no docs/ dir exists)
- package.json: bump engines.node to >=22.11.0 to match the authkit
  house style and what CI actually runs (Node 22)
- README: document the new useAuth().error field and the auth-error
  failure-surfacing behavior

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JT8sJBpwfAuRYKP7Lk53Hh
Add bump-minor-pre-major so breaking changes bump the minor version
while < 1.0.0 instead of going to 1.0.0, matching the authkit-session
and authkit-tanstack-start SDK configs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JT8sJBpwfAuRYKP7Lk53Hh
@nicknisi
nicknisi force-pushed the dx-review-and-release-please branch from e0255e3 to e20149b Compare July 23, 2026 15:37
nicknisi and others added 4 commits July 23, 2026 10:41
The e2e helper hard-coded the __authkit_electron bridge global as a
dotted property access inside a Playwright evaluate callback, which
tripped oxlint's no-underscore-dangle rule. Import AUTHKIT_BRIDGE_KEY
from src/shared/ipc-channels.js and pass it into evaluate as an
argument with bracket access, matching the single-shared-constant
convention established in 574f7e3. No runtime behavior change.
oxlint exits 0 on warnings by default, so CI's `pnpm run lint` let
the no-underscore-dangle warning fixed in 81d881e pass unnoticed.
Add --deny-warnings so any warning fails the build instead of
silently accumulating.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JT8sJBpwfAuRYKP7Lk53Hh
When the sign-in ceremony fails to open (e.g. win.loadURL() rejects
because the auth server is unreachable), the failure only rejected the
renderer's signIn() promise — it never reached the auth-error broadcast,
so useAuth().error stayed empty while every other failure mode (consent
denied, window closed, failed code exchange) populated it.

The sign-in IPC handler now broadcasts the same safe { code, message }
payload on failure AND still returns { ok: false }, so signIn() keeps
throwing (backwards-compatible) while useAuth().error is populated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JT8sJBpwfAuRYKP7Lk53Hh
Comment thread README.md
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JT8sJBpwfAuRYKP7Lk53Hh
@nicknisi
nicknisi merged commit 06f042b into main Jul 23, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants