Skip to content

fix(install): configure and verify AuthKit application URLs - #244

Open
nicknisi wants to merge 5 commits into
mainfrom
fix/authkit-application-setup
Open

nicknisi wants to merge 5 commits into
mainfrom
fix/authkit-application-setup

Conversation

@nicknisi

@nicknisi nicknisi commented Sep 19, 2026

Copy link
Copy Markdown
Member

Summary

Rebased onto origin/main at 7f6eac2, which includes merged #243 and @workos/skills@0.7.3. The SDK/dependency upgrade is no longer part of this PR's diff. The skills explain the required application settings; this PR makes the installer create the sign-in route and configure supported settings itself, rather than relying on its restricted agent to run management commands.

  • Include the shared AuthKit setup reference directly in the Next.js agent prompt only. Keep the agent focused on app code and explicitly prohibit bypassing its shell restrictions.
  • Require a dedicated Next.js /sign-in route using the SDK, separate from the callback. Include missing Next.js requirements in agent retries and fail validation instead of reporting incomplete code as installed. Other frameworks keep their existing advisory completeness checks; their security/build checks are unchanged.
  • Configure Next.js callback, default Sign-out URI, and Initiate login URI through native installer code. Match the app client ID to a unique sandbox environment and its default application, preserve existing URLs/defaults, validate changes, and verify saved settings by reading them back.
  • Preserve an existing homepage unless --homepage-url explicitly requests a change. Register and verify the callback before optional URL updates. Conflicting sign-out defaults or initiate-login URLs are left unchanged without blocking the callback or unrelated settings.
  • Carry application setup results through the state machine into human and JSON completion output. Distinguish installed code, verified settings, and browser flows still needing tests.

Authentication and scope

  • With a usable dashboard session, all three URL settings use the same client-ID-matched sandbox application. This step does not launch login.
  • Without a dashboard session, or when the current team's catalog has no match for the app client ID, API-key-only and unclaimed installs register the callback using their sandbox API key, then return immediately without any dashboard writes. The API key is the sole write target in this path. A failed callback registration fails the install instead of leaving sign-in broken behind a success result.
  • The two write paths are exclusive. Dashboard failures, including partial writes, never fall back to API-key mutations. This avoids splitting one run's settings across environments without removing callback registration for logged-out users.
  • API-only results report callbackRegistered: true but verified: false. Sign-out URI, Initiate login URI, and any requested homepage still need dashboard setup; browser flows remain untested.
  • Every successful Next.js setup must have a confirmed callback. Missing credentials, ambiguous targets, failed callback writes, and callback read-back failures stop the install. Remaining optional settings may be reported as incomplete only after the callback is confirmed.
  • Production is read-only: an already registered callback permits installation with manual setup guidance; a missing production callback fails without mutation. Non-default applications are not configured automatically.
  • Dashboard targeting uses the already-fetched environment ID, without a second catalog lookup or changes to stored profiles.
  • Native application setup is Next.js App Router-only. The SDK documents App Router support, not a Pages Router equivalent. CLI choices and human/JSON help now advertise only --router app; there is no Pages Router option in the interactive picker. Known Pages-only projects are declined before credential provisioning, including with --force. Explicit --router pages is rejected during argument parsing; internal router options are also narrowed to app. A shared runtime guard rejects unexpected values that bypass those type/parser checks, including before provisioning in an empty project. Preflight and installer use the same detection, so pages/api/ alone is not mistaken for a Pages Router app. Mixed-router projects use App Router without modifying the Pages tree.
  • Route-group directories are supported for sign-in and callback routes. A different URL such as /account/sign-in does not satisfy /sign-in. Existing pages at /sign-in, including within route groups, are refused before provisioning rather than overwritten; this installer currently requires that URL to be a dedicated route handler.
  • The Next.js post-agent validation still runs with --no-validate so the installer does not configure a missing route.
  • This does not change logout or the installer's initial credential-selection behavior.

Verification

  • bun run test: 2,848 tests passed across 163 files with published skills 0.7.3 installed.
  • bun run typecheck and bun run lint passed.
  • Formatting passed for all changed files. The whole-directory local check flagged one unrelated untracked .pi/artifacts/cli-0-22-0-ship-message.html; it was left untouched and is not part of the PR.
  • bun run build passed.
  • Compiled workos internal verify-assets --json passed and reported bundled skills 0.7.3.
  • Tests cover no pre-agent mutations from mixed credentials, exclusive API-key/dashboard write paths, logged-out callback registration and duplicate handling, fatal callback failures, production/unknown-key refusal, single-application dashboard targeting, early Pages Router refusal, preserving settings, rejected dry runs, concurrent edits, partial writes, read-back mismatches, route validation, and completion reporting.
  • Nine runtime-router regression cases failed before the latest guard and pass now, covering unsupported strings/falsy values, preflight with/without a manifest, and refusal before credential/API writes.
  • Earlier regression tests reproduced the missing logged-out callback and unsupported router advertising before their fixes. Compiled help advertises only app, and compiled argument parsing rejects --router pages.
  • Compiled offline command smoke passed, including install, integrate, and dashboard refusing Pages Router, plus an existing route-group sign-in page being preserved before provisioning.
  • New regression tests reproduced non-Next.js blocking/retry errors, wrong-team callback omission, and stale-setting callback omission before the fixes. Tests cover callback-first ordering, read-back failures, production read-only behavior, route groups, collision refusal, and immutable installer options.
  • One full-suite rerun hit an intermittent skills-refresh test failure; the unchanged full-suite retry passed. No unrelated test code was changed.

Live smoke evidence

  • On d17ce80, a fresh Next.js 16.3.5 project with isolated file-backed credentials completed the full compiled install with exit 0, no CLI login, and callbackRegistered: true.
  • A live WorkOS API GET returned 200 and confirmed the callback URI was registered. Clicking the generated Sign in link reached the hosted AuthKit email form in the browser.
  • After the callback-first fixes in 5274ae8 (rebased as 13e5b3d), the callback setup helper was rerun against that same isolated unclaimed environment: callback registration passed, the CLI remained logged out, and full application URL verification correctly remained false. Existing account environments were not used.

End-to-end browser sign-in/sign-out and password-reset/invitation flows have not been verified for this PR. Unit tests use mocked dashboard responses; they do not prove a live dashboard configuration succeeds.

@nicknisi
nicknisi added this pull request to stack #245 September 19, 2026 15:43
@greptile-apps

greptile-apps Bot commented Sep 19, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the remaining runtime-router finding is fixed and no new actionable issues were identified.

Summary

This PR makes Next.js AuthKit installation App Router-only, validates the required sign-in and callback routes, and configures application URLs through exclusive dashboard or API-key paths with read-back verification.

  • Rejects Pages Router projects and unsupported runtime router values before provisioning or project writes.
  • Adds callback-first AuthKit application configuration while preserving existing settings and avoiding cross-environment writes.
  • Carries verified and incomplete application-setup state into human and structured completion output.
  • Strengthens Next.js route validation, including route groups and existing /sign-in page collisions.
  • The latest change fully addresses the remaining runtime-router finding without widening the public router type or CLI choices.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Install command] --> B[Preflight router and route checks]
  B --> C[Resolve credentials]
  C --> D[Write local environment]
  D --> E[Agent installs Next.js code]
  E --> F[Validate sign-in and callback routes]
  F --> G{Dashboard environment uniquely matches client ID?}
  G -->|Yes| H[Register callback and configure optional URLs]
  H --> I[Read back and verify settings]
  G -->|No, sandbox API key available| J[Register callback through API]
  J --> K[Report remaining dashboard setup as unverified]
  I --> L[Structured completion]
  K --> L
Loading

Reviews (5) · Last reviewed commit: "fix(install): reject unsupported runtime..."

Comment thread src/lib/agent-runner.ts Outdated
Comment thread src/lib/run-with-core.ts Outdated
Comment thread src/lib/run-with-core.ts
@nicknisi

Copy link
Copy Markdown
Member Author

@greptileai Please re-review the latest commit d17ce80. It restores sandbox API-key-only callback registration after validation, keeps API-key and dashboard write paths exclusive, fails installation on callback registration failure, and removes unsupported Pages Router choices from CLI help and the picker. All CI checks pass; the current 3/5 summary still refers to 3366b2a.

@gjtorikian gjtorikian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified locally on d17ce80 with skills 0.7.3: bun run test (163 files / 2,824), typecheck, and lint all pass as described.

The dashboard path (read → dry-run → recheck → write → read-back, sandbox-only, no cross-target writes) is careful and well-tested. Two regressions undercut the PR's own goal, though:

  1. The new validation gate in agent-runner.ts is not scoped to Next.js. The other rulesets were advisory and are stale; a correct modern TanStack Start install (per the bundled skill) now hard-fails.
  2. With a dashboard session present, any precondition failure leaves the callback unregistered behind exit 0 — logged in to a different team than the API key, production env, stale sign-out/initiate-login values from another project, or any dashboard error. All of these worked pre-PR.

Both fixes are small and local; details inline, plus a few MEDIUM/LOW items.

Comment thread src/lib/agent-runner.ts Outdated
Comment thread src/lib/agent-runner.ts Outdated
Comment thread src/lib/authkit-application-setup.ts Outdated
Comment thread src/lib/authkit-application-setup.ts Outdated
Comment thread src/lib/authkit-application-setup.ts Outdated
Comment thread src/lib/preflight-authkit.ts Outdated
Comment thread src/utils/types.ts Outdated
Comment thread src/lib/run-with-core.ts Outdated
Comment thread src/lib/agent-runner.ts Outdated
Comment thread src/integrations/nextjs/utils.ts Outdated
Base automatically changed from fix/workos-sdk-v10-upgrade to main September 21, 2026 17:27
Comment thread src/integrations/nextjs/utils.ts
@nicknisi
nicknisi force-pushed the fix/authkit-application-setup branch from 5274ae8 to f540e89 Compare September 22, 2026 14:12
@nicknisi

Copy link
Copy Markdown
Member Author

@greptileai Please re-review f540e89. The branch is now rebased onto the latest origin/main (7f6eac2), removing the already-merged SDK/dependency changes from the diff. The remaining runtime-router finding is fixed without widening the app-only type or CLI choices: unsupported supplied values fail before detection, provisioning, or writes, including in empty projects. Nine new regression cases fail before the fix and pass now; all 2,848 tests, typecheck, lint, changed-file formatting, build, and compiled smoke checks pass. All previously addressed review threads have been resolved.

@nicknisi
nicknisi requested a review from gjtorikian September 22, 2026 14:43

This branch has not been deployed

No deployments
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