Skip to content

Fix(OTP Login): fresh-install 500s; load settings from documents table - #1076

Merged
lane711 merged 9 commits into
mainfrom
merge-pr-1056-fix-plugin-otp
Sep 4, 2026
Merged

Fix(OTP Login): fresh-install 500s; load settings from documents table#1076
lane711 merged 9 commits into
mainfrom
merge-pr-1056-fix-plugin-otp

Conversation

@lane711

@lane711 lane711 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Same-repo mirror of #1056 (by @LukeSeers) so CI/CD runs without the fork authorize gate.

Summary

Fixes OTP login 500s on fresh installs. The legacy plugins/otp_codes tables were dropped when plugin settings moved to documents (#858), but the OTP plugin still read from plugins → every /auth/otp/* request 500'd (no such table: plugins), and code storage needed otp_codes.

  • OTP settings now load from the documents table via PluginService.getPlugin('otp-login').
  • Restore otp_codes table (migration 0005_otp_codes.sql) required by OTPService.
  • Regenerate migrations-bundle.ts (the actual source MigrationService reads) → 5 migrations. This was missing from Fix(OTP Login): on fresh installs and load settings from the documents table #1056, so 0005 never applied and migrations.test.ts would have failed CI.
  • E2E regression spec retagged @smoke @auth so CI selects it.

Attribution

Maintainer changes

  • 1b8b6dd02 regenerate migrations bundle for 0005 + retag OTP e2e @smoke + fix stale docstring
  • 2af7ec4ce load settings via PluginService.getPlugin (drop hand-rolled document SQL + dead legacy plugins fallback)
  • merged latest main

Verification

  • 709/709 unit tests pass (migrations + plugin); migrations.test.ts green
  • Type-check clean (only pre-existing missing-dep noise locally)

🤖 Generated with Claude Code

LukeSeers and others added 8 commits August 14, 2026 16:40
Migration 0005_otp_codes.sql was added but the migrations bundle
(migrations-bundle.ts) — the actual source MigrationService reads, not
the .sql files — was not regenerated. On a fresh install 0005 never
applied: otp_codes was never created and /auth/otp/request 500'd for a
real user with `no such table: otp_codes`, and migrations.test.ts failed
CI (bundle stopped at 0004 while the test expects 0005).

Regenerate the bundle to include 0005, and retag the OTP regression spec
@smoke so CI actually selects it (the migrations/otp change never
triggers the @auth-only selection). Also correct the stale spec
docstring (settings now come from the documents table, not `plugins`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
loadOtpSettings hand-rolled raw SQL against the documents table and a
dead fallback to the dropped legacy `plugins` table (R4 duplication).
PluginService.getPlugin already reads the same plugin document
(type_id='plugin', slug='otp-login') and returns its settings. Reuse it
and drop the dead legacy path. Behavior unchanged: never throws, missing
document or unparseable settings resolves to DEFAULT_SETTINGS.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts:
#	packages/core/src/db/migrations-bundle.ts
@lane711
lane711 merged commit ebd48c9 into main Sep 4, 2026
2 of 5 checks passed
@lane711
lane711 deleted the merge-pr-1056-fix-plugin-otp branch September 4, 2026 22:44
lane711 added a commit that referenced this pull request Sep 4, 2026
Resolve migration-inventory conflicts with main's PR #1076 (OTP login plugin,
migration 0005_otp_codes.sql):

- migrations.test.ts: union both inventories — 0005 (otp_codes) from main plus
  0006 (two_factor_lockout) / 0007 (two_factor_required) from this branch.
  Renumber the applied_at fixtures sequentially and bump appliedMigrations to 7.
  Drop the stale comment claiming 0005 was reserved for the FTS5 search PR.
- migrations-bundle.ts: regenerated via `npm run generate:migrations` (R9); the
  my-sonicjs-app/migrations/ copies are byte-identical to packages/core/migrations/.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to 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.

2 participants