Skip to content

feat: single-user architecture with first-time setup flow#11

Open
nathanialhenniges wants to merge 3 commits intomainfrom
claude/nostalgic-lewin-6249f4
Open

feat: single-user architecture with first-time setup flow#11
nathanialhenniges wants to merge 3 commits intomainfrom
claude/nostalgic-lewin-6249f4

Conversation

@nathanialhenniges
Copy link
Copy Markdown
Member

@nathanialhenniges nathanialhenniges commented Apr 17, 2026

Summary

  • True singleton tables: dropped userId FK from all config/state tables (timerState, timerConfig, timerStyle, taskStyle, botConfig, botAccount); each uses id='singleton' as the primary key
  • New instanceConfig table: holds overlayTimerToken and overlayTasksToken (moved off user table), provisioned automatically on first login
  • Owner gate: databaseHooks.user.create.before counts existing users — first signup sets isOwner=true, any subsequent attempt throws FORBIDDEN; ALLOWED_TWITCH_IDS env var removed entirely
  • /setup route: server component calls notFound() if an owner already exists; client shows "Claim this instance" UI with Twitch sign-in; home page shows a toast on ?error=instance_claimed
  • Event bus: dropped :userId suffix from all event constants (timerStateChange, taskListChange, botConfigChange)
  • tRPC routers: all queries unscoped (no userId/ownerId filters); overlay token lookup via instanceConfig; ensureUserConfig renamed ensureSingletons
  • Bot service/commands: start(db) signature (no userId); broadcaster check via unscoped user.findFirst(); task queries drop ownerId
  • DB migration: replaced two existing migrations with a single fresh 0000_init.sql (app not yet released — clean slate)
  • drizzle-orm added to @dirework/auth dependencies (new import for hasOwner() count query)
  • Docs: README Getting Started updated to bun commands and /setup claim step; CLAUDE.md env var section cleaned up

Test plan

  • Fresh install: bun run db:start && bun run db:push && bun run dev:web → visit http://localhost:3001 → redirects to /setup
  • Click "Claim with Twitch" → OAuth → lands on /dashboard; DB shows user.isOwner=true, all singleton rows exist, instanceConfig has tokens
  • Sign out, visit /setup → 404
  • Second Twitch account OAuth attempt → redirects to /?error=instance_claimed → toast shown
  • Original owner re-login → succeeds
  • OBS overlay tokens load from instanceConfig; SSE pushes work on timer/task mutations
  • bun run check-types → 0 errors
  • bun run test → 175 tests passing
  • SKIP_ENV_VALIDATION=true bun run next build → all 15 routes compiled

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added instance setup page for first-time ownership claiming with Twitch account
  • Bug Fixes

    • Improved error handling and redirect flow for instance ownership validation
  • Documentation

    • Updated getting started guide to use bun instead of pnpm
    • Clarified setup flow in setup documentation
  • Chores

    • Removed legacy allowlist configuration option

nathanialhenniges and others added 2 commits April 17, 2026 11:59
Convert Dirework from multi-user to single-owner-per-instance model.
Removes all per-user FK columns from config/state tables (true singletons),
adds instanceConfig table for overlay tokens, gates signup to one owner,
and introduces a /setup route that self-disables once claimed.

Key changes:
- Schema: drop userId FK from all singleton tables (timerState, timerConfig,
  timerStyle, taskStyle, botConfig, botAccount); add instanceConfig singleton
  with overlayTimerToken/overlayTasksToken; add isOwner flag to user table;
  drop ownerId from task (keep authorTwitchId/authorDisplayName); fresh init migration
- Auth: replace allowlist with owner-gate hook (count-based, first signup wins,
  sets isOwner=true); session hook provisions all singletons on first login;
  export hasOwner() helper; add drizzle-orm as explicit dependency
- Event bus: drop :userId suffix from event constants (TIMER_STATE_CHANGE,
  TASK_LIST_CHANGE, BOT_CONFIG_CHANGE)
- tRPC routers: remove all userId/ownerId scoping from queries; overlay token
  lookup via instanceConfig; rename ensureUserConfig→ensureSingletons
- Bot service/commands: remove userId parameter; load config unscoped;
  broadcaster check via unscoped user.findFirst()
- Setup route: /setup server component calls notFound() if owner exists;
  client UI with Twitch claim button; home page shows toast on instance_claimed error
- Remove ALLOWED_TWITCH_IDS env var entirely
- Tests: update fixtures to singleton schema; fix vi.hoisted() for mock ordering

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- README: fix pnpm → bun commands in Getting Started; update step 7
  to explain /setup claim flow on first run
- CLAUDE.md: remove ALLOWED_TWITCH_IDS from env var docs (removed in refactor)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 17, 2026

Warning

Rate limit exceeded

@nathanialhenniges has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 43 minutes and 8 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 8 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fd26e962-566e-4f86-9982-dcdf0962f17d

📥 Commits

Reviewing files that changed from the base of the PR and between b8fcc17 and c6fd2c4.

📒 Files selected for processing (7)
  • apps/web/src/app/(app)/home-content.tsx
  • apps/web/src/app/(app)/setup/setup-content.tsx
  • packages/api/src/routers/user.ts
  • packages/db/drizzle/0001_add-single-owner-index.sql
  • packages/db/drizzle/meta/0001_snapshot.json
  • packages/db/drizzle/meta/_journal.json
  • packages/db/src/schema/auth.ts

Walkthrough

Architectural migration from multi-user to single-owner instance model: removes per-user scoped configurations and overlay tokens, consolidates config tables to singleton pattern with shared instanceConfig, replaces user-scoped events with global event constants, implements single-owner authentication constraint, and updates all API routers and database schema accordingly.

Changes

Cohort / File(s) Summary
Documentation & Configuration
.env.example, CLAUDE.md, README.md, packages/env/src/server.ts
Removed ALLOWED_TWITCH_IDS environment variable; updated setup instructions from pnpm to bun commands and refined first-run Twitch login guidance.
Database Schema & Migrations
packages/db/src/schema/auth.ts, packages/db/src/schema/app.ts, packages/db/drizzle/0000_init.sql, packages/db/drizzle/0001_pretty_jocasta.sql, packages/db/drizzle/meta/...
Removed user-scoped overlay tokens from user table and userId columns from config/state tables; introduced singleton-pattern instanceConfig table for overlay tokens; changed multi-row config tables to singleton defaults; removed owner-scoped foreign keys and indexes; replaced per-owner task indexes with global status/priority/author indexes.
Schema Type & Index Exports
packages/db/src/index.ts, packages/db/src/schema/index.ts
Added InstanceConfig type export; removed relation exports for app tables from user relations; simplified userRelations to exclude singleton table references.
Authentication & Authorization
packages/auth/src/index.ts, packages/auth/src/__tests__/allowlist.test.ts, packages/auth/package.json
Removed allowlist validation; implemented single-owner constraint enforcing exactly one user creation; changed session hooks to provision singleton config rows and new instanceConfig; replaced allowlist tests with ownership tests; added drizzle-orm dependency.
Event System
packages/api/src/events.ts, packages/api/src/__tests__/events.test.ts
Exported event name constants (TIMER_STATE_CHANGE, TASK_LIST_CHANGE, BOT_CONFIG_CHANGE); removed per-user scoped event patterns; updated test suite to use constants instead of hardcoded strings.
Bot Service & Commands
packages/api/src/bot/index.ts, packages/api/src/bot/commands.ts, apps/web/instrumentation.ts
Removed userId parameter from botService.start(); simplified bot account lookup to unscoped query; removed ownerId from MessageContext; updated all message handlers to use global event constants; changed timer/task state access from user-filtered to singleton queries.
API Routers
packages/api/src/routers/bot.ts, packages/api/src/routers/config.ts, packages/api/src/routers/user.ts
Removed user-scoped filtering from bot account queries; replaced per-user ensureUserConfig() with singleton ensureSingletons(); removed getByOverlayToken procedure; changed bot stop authorization to unscoped deletion; moved overlay token management from user to instanceConfig.
Timer & Task Routers
packages/api/src/routers/timer.ts, packages/api/src/routers/task.ts, packages/api/src/routers/overlay.ts
Removed all user-id-based where clauses; changed upsert conflict targets from userId to id; removed ownerId filtering from task queries; replaced per-user event emissions with global event constants; updated overlay token resolution to use instanceConfig singleton; simplified config loading to direct table queries without user scoping.
Router Tests
packages/api/src/routers/__tests__/config.test.ts, packages/api/src/routers/__tests__/config-roundtrip.test.ts
Updated test fixtures to use singleton id value and removed userId field assignments from config mocks.
Web UI & Pages
apps/web/src/app/(app)/page.tsx, apps/web/src/app/(app)/home-content.tsx, apps/web/src/app/(app)/setup/page.tsx, apps/web/src/app/(app)/setup/setup-content.tsx
Converted home page from client-side to server-side auth flow with redirects; added new SetupPage and SetupContent for first-time instance claiming; implemented server-side owner/session checks and redirects to /dashboard or /setup.

Sequence Diagram(s)

sequenceDiagram
    participant User as User/Browser
    participant Server as Next.js Server
    participant Auth as Auth Service
    participant DB as Database
    participant SetupUI as Setup UI

    User->>Server: GET /
    Server->>Auth: getSession(headers)
    Auth->>DB: Query user (check existence)
    DB-->>Auth: user row or null
    
    alt No users exist (instance unclaimed)
        Auth-->>Server: session = null
        Server->>DB: hasOwner()
        DB-->>Server: false
        Server-->>User: Render HomeContent (Sign in link)
        User->>Auth: signIn.social('twitch')
        Auth-->>User: Redirect to /setup
    else Users exist (instance claimed)
        alt User is authenticated
            Auth-->>Server: session with user
            Server-->>User: Redirect to /dashboard
        else User not authenticated
            Auth-->>Server: session = null
            Server->>DB: hasOwner()
            DB-->>Server: true
            Server-->>User: Render HomeContent
        end
    end

    User->>Server: GET /setup
    Server->>Auth: getSession(headers)
    Server->>DB: hasOwner()
    alt Instance already owned
        DB-->>Server: true
        Server-->>User: 404 Not Found
    else Instance not owned
        alt User authenticated
            Auth-->>Server: session with user
            Server->>DB: Create user (isOwner=true)
            DB-->>Server: user created
            Server->>DB: Provision instanceConfig
            DB-->>Server: config created
            Server-->>User: Redirect to /dashboard
        else User not authenticated
            Auth-->>Server: session = null
            Server-->>SetupUI: Render SetupContent
            SetupUI-->>User: Display "Claim with Twitch" button
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Poem

🐰 Singleton hops through the fields so bright,
One owner claims the instance tonight!
No more scattered tokens, no scopes to divide,
Global events flow with unified stride. 🎉
From multi to singular, the DireWork takes flight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely summarizes the primary architectural change: transitioning to a single-owner-per-instance model with a new setup/claim flow.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/nostalgic-lewin-6249f4

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (10)
apps/web/src/app/(app)/setup/page.tsx (1)

1-3: Consolidate next/navigation imports.

redirect and notFound come from the same module; merge into one import.

♻️ Proposed fix
-import { redirect } from "next/navigation";
 import { headers } from "next/headers";
-import { notFound } from "next/navigation";
+import { notFound, redirect } from "next/navigation";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/app/`(app)/setup/page.tsx around lines 1 - 3, Consolidate the
duplicate import from "next/navigation" by combining the two separate imports
into a single import statement that includes both redirect and notFound (leave
headers imported from "next/headers" as-is); update the file-level imports so
only one import from "next/navigation" exports redirect and notFound to remove
redundancy and keep imports tidy.
packages/api/src/routers/timer.ts (1)

89-91: Consider an explicit .where() on singleton updates.

ctx.db.update(schema.timerState).set(data).returning() has no predicate; it updates every row. That is correct today because the table is a singleton, but an explicit .where(eq(schema.timerState.id, "singleton")) documents intent and guards against accidental drift if the invariant changes. Same applies to the other update sites in this file (lines 113, 126, 144, 186, 194).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routers/timer.ts` around lines 89 - 91, The update calls
like ctx.db.update(schema.timerState).set(data).returning() currently lack a
predicate and will update all rows; add an explicit WHERE predicate such as
.where(eq(schema.timerState.id, "singleton")) to each update call to document
intent and guard the singleton invariant. Locate the update expressions (e.g.,
ctx.db.update(schema.timerState).set(data).returning()) and add
.where(eq(schema.timerState.id, "singleton")) (import eq if necessary), and
apply the same change to the other update sites in this file (the similar update
calls referenced in the review).
packages/db/src/schema/auth.ts (1)

14-14: isOwner column looks good; single-owner invariant relies on app-level enforcement.

The column is notNull with a safe default. Enforcement that only one user can exist lives in the Better Auth user.create.before hook in packages/auth/src/index.ts (counts rows and throws FORBIDDEN). If you want defense-in-depth at the DB level, consider adding a partial unique index (e.g. CREATE UNIQUE INDEX user_single_owner_idx ON "user" ((is_owner)) WHERE is_owner = true) in a future migration — not required for this PR.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/db/src/schema/auth.ts` at line 14, isOwner column is correctly
not-null with default false and the single-owner invariant is enforced in the
Better Auth hook user.create.before in packages/auth/src/index.ts; if you want
DB-level defense-in-depth add a migration that creates a partial unique index
(e.g. name it user_single_owner_idx) on the "user" table for rows WHERE is_owner
= true so the database will prevent more than one owner in addition to the
application-level check.
apps/web/src/app/(app)/home-content.tsx (1)

3-5: Nit: merge the two react imports.

Suspense and useEffect are imported from "react" on two separate lines. Consolidate to a single import for readability.

♻️ Proposed fix
-import { Suspense } from "react";
+import { Suspense, useEffect } from "react";
 import { useSearchParams } from "next/navigation";
-import { useEffect } from "react";
 import {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/app/`(app)/home-content.tsx around lines 3 - 5, Merge the two
separate React imports into a single import statement by combining Suspense and
useEffect into one import from "react" (replace the separate imports of Suspense
and useEffect with a single line importing both) and remove the now-unused
duplicate import; ensure the import includes both Suspense and useEffect in the
same curly-braced import.
packages/api/src/bot/index.ts (1)

174-186: Minor: log when reloadConfig fails to load a config row.

If the insert succeeds but findFirst returns undefined (e.g., due to an unexpected schema issue or a transaction anomaly), configCache stays null and every chat message silently short-circuits at onMessage (line 78). That's very hard to diagnose from outside. A single logger.warn/logger.error when botConfigRow is falsy would make the failure mode observable.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/bot/index.ts` around lines 174 - 186, In reloadConfig, add a
log when the inserted-but-not-found case occurs: after awaiting
this.db.query.botConfig.findFirst() check if botConfigRow is falsy and call a
logger (e.g., this.logger.warn or this.logger.error) with a descriptive message
including context like "reloadConfig: botConfig row missing after insert" and
any available diagnostics (e.g., this.db transaction id, schema version, or the
inserted result) before returning so that the unexpected state where configCache
remains null is observable; update references in reloadConfig, botConfigRow, and
configCache accordingly.
packages/auth/src/__tests__/allowlist.test.ts (1)

1-71: Nit: rename file now that allowlist logic is gone.

All parseAllowedTwitchIds coverage was removed and this file now exclusively tests hasOwner(). Consider renaming to has-owner.test.ts (or owner.test.ts) so the filename reflects the suite and future grep/navigation matches intent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/auth/src/__tests__/allowlist.test.ts` around lines 1 - 71, Test file
name no longer matches its contents: rename the test file from allowlist.test.ts
to has-owner.test.ts (or owner.test.ts) so it reflects the single exported
function under test (hasOwner) and improves discoverability; ensure any CI/test
patterns or docs referencing the old filename are updated and keep the existing
describe/it blocks as-is since they already target hasOwner.
packages/api/src/bot/commands.ts (1)

170-171: Hot-path: avoid a DB lookup for the owner's twitchId on every !task/!next.

handleTaskAdd (line 170) and handleTaskNext (line 430) each execute db.query.user.findFirst(...) per chat message purely to determine isBroadcaster. In a single-owner instance the owner's twitchId is effectively immutable for the lifetime of the bot; fetching it on every message is wasted I/O on the chat hot path.

Consider caching it once at bot startup (e.g. store ownerTwitchId on TwitchBotService next to channelName, loaded in start() from the same db.query.user.findFirst that already runs there) and passing it through MessageContext so these handlers can do a plain string compare. Same semantics, no per-message query.

Also applies to: 430-431

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/bot/commands.ts` around lines 170 - 171, The handlers
handleTaskAdd and handleTaskNext are doing a DB lookup per message to compute
isBroadcaster; instead, add a cached ownerTwitchId property on TwitchBotService
(loaded once in start() using the existing db.query.user.findFirst call) and
propagate that value into MessageContext so the handlers can compare strings
(ownerTwitchId === userInfo.twitchId) instead of calling db.query.user.findFirst
each time; update TwitchBotService.start to set ownerTwitchId, adjust
MessageContext shape to include ownerTwitchId, and change handleTaskAdd and
handleTaskNext to read from context and perform a plain string compare.
packages/api/src/routers/config.ts (1)

378-380: Scope updates to the singleton row defensively.

All six db.update(schema.X).set(...).returning() calls here omit a where clause, so they update every row in the table. That's the intended behavior today because these tables hold a single id = 'singleton' row, but the invariant is implicit and easy to violate (a bad migration, a manual DB insert, or a future feature that adds a second row will silently corrupt every other record on the next save).

Consider adding an explicit .where(eq(schema.X.id, "singleton")) to every mutation. It encodes the assumption in the code and costs nothing at runtime.

Also applies to: 391-393, 404-406, 457-494, 513-523, 537-539

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routers/config.ts` around lines 378 - 380, The update calls
like ctx.db.update(schema.timerConfig).set(input).returning() must be scoped to
the singleton row; add an explicit .where(eq(schema.timerConfig.id,
"singleton")) to that call and to the other five similar mutation calls (e.g.,
where you call ctx.db.update(schema.<Table>).set(...).returning()). Also ensure
the eq symbol is imported/available in the file so you can reference
eq(schema.<Table>.id, "singleton") for each update. This encodes the singleton
invariant and prevents accidental multi-row updates.
packages/db/drizzle/meta/0000_snapshot.json (1)

1-1695: Clean-slate migration: confirm no existing deployments need an upgrade path.

This snapshot (and the replaced 0000_init.sql per the PR description) represents a reset of the migration history — any existing database from the previous multi-user schema will not migrate cleanly with drizzle-kit migrate and would need to be dropped/recreated. That's consistent with the pre-release single-owner pivot, but worth confirming:

  • No production/staging DB holds data that must be preserved.
  • Contributors with local dev databases will need to drop them (mention in README/CLAUDE.md if not already covered).

If confirmed, the snapshot contents themselves (singleton defaults, new instance_config, is_owner on user, updated task indexes with removed owner_id) match the authored schema in packages/db/src/schema/app.ts.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/db/drizzle/meta/0000_snapshot.json` around lines 1 - 1695, This
change replaces migration history with a clean-slate snapshot
(0000_snapshot.json + replaced 0000_init.sql) so confirm there are no
production/staging databases or any contributor local DBs that must be
preserved, and update documentation (README or CLAUDE.md) with explicit
instructions to drop/recreate local dev databases before running drizzle-kit
migrate; also verify the snapshot schema (instance_config table, user.is_owner
column, task index changes) exactly matches packages/db/src/schema/app.ts and
mention the required manual upgrade/drop steps in the PR description or docs so
users are not surprised.
packages/api/src/routers/overlay.ts (1)

53-115: Optional: deduplicate the initial-yield and change-loop read blocks.

The Promise.all([timerState, timerConfig, timerStyle]) fetch and its mapping to the yielded shape appears three times across this file (initial + loop in onTimerState, plus getTimerState), and the task equivalent three times as well. Extracting small helpers like readTimerSnapshot(ctx) / readTaskSnapshot(ctx) would keep all yields in lockstep and reduce drift risk if the shape changes later.

♻️ Sketch
async function readTimerSnapshot(ctx: Ctx) {
  const [timerState, timerConfigRow, timerStyleRow] = await Promise.all([
    ctx.db.query.timerState.findFirst(),
    ctx.db.query.timerConfig.findFirst(),
    ctx.db.query.timerStyle.findFirst(),
  ]);
  return {
    timerState: timerState ?? null,
    timerConfig: timerConfigRow ? buildTimerConfig(timerConfigRow) : null,
    timerStyles: timerStyleRow ? buildTimerStylesConfig(timerStyleRow) : null,
  };
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routers/overlay.ts` around lines 53 - 115, Duplicate DB-read
+ mapping logic in onTimerState, onTaskList (and the separate getTimerState
path) should be extracted into small helpers to avoid drift; add async functions
like readTimerSnapshot(ctx) and readTaskSnapshot(ctx) that perform the
Promise.all queries (timerState/timerConfig/timerStyle and
task/findMany/taskStyle respectively) and return the mapped shape (using
buildTimerConfig/buildTimerStylesConfig and buildTaskStylesConfig). Replace the
initial-yield blocks and the for-await loop bodies in onTimerState and
onTaskList (and call sites such as getTimerState) to invoke these helpers and
yield their result, ensuring all yields share the same mapping logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/src/app/`(app)/setup/setup-content.tsx:
- Around line 57-63: The OAuth errorCallbackURL currently hardcodes
error=instance_claimed for all Twitch sign-in failures, which mislabels
unrelated failures; update the authClient.signIn.social call (provider:
"twitch") to use a generic error code such as error=signin_failed (or a distinct
error_type param) instead of instance_claimed, or add a more specific parameter
(e.g., error_type=instance_claimed) only when you can deterministically detect
the FORBIDDEN/owner-claimed case; adjust any home page toast logic to read the
new generic key and display appropriate copy for generic sign-in failures vs.
the specific instance-claimed case.

In `@packages/api/src/routers/user.ts`:
- Around line 30-41: The mutate path regenerateOverlayToken can cause an
uninitialized instanceConfig row to be created with the "other" token due to the
schema $defaultFn when using insert(...).onConflictDoUpdate; to prevent that,
call ensureSingletons() first (the routine that provisions instanceConfig) or
replace the insert-onConflict pattern with an explicit update targeting the
singleton row (use
ctx.db.update(schema.instanceConfig).set({...}).where(eq(schema.instanceConfig.id,
SINGLETON_ID))) so rotating one token does not implicitly mint the other; update
references: regenerateOverlayToken, schema.instanceConfig, ensureSingletons,
eq(...), and SINGLETON_ID.

In `@packages/auth/src/index.ts`:
- Around line 36-44: The current create.before hook in
packages/auth/src/index.ts (the async "before" handler that checks db.select
count on schema.user and throws APIError) is racy; add a DB-level partial unique
index to enforce a single owner (e.g. CREATE UNIQUE INDEX user_single_owner_idx
ON "user"(is_owner) WHERE is_owner = true) via a migration, and also catch the
DB unique-constraint failure when inserting a second owner and translate it into
the same APIError ("FORBIDDEN" / "This instance is already claimed. Single-user
only.") so concurrent requests get a friendly error; keep the existing before
hook for the common-path short-circuit but rely on the DB constraint plus the
insert error handling to guarantee correctness.

---

Nitpick comments:
In `@apps/web/src/app/`(app)/home-content.tsx:
- Around line 3-5: Merge the two separate React imports into a single import
statement by combining Suspense and useEffect into one import from "react"
(replace the separate imports of Suspense and useEffect with a single line
importing both) and remove the now-unused duplicate import; ensure the import
includes both Suspense and useEffect in the same curly-braced import.

In `@apps/web/src/app/`(app)/setup/page.tsx:
- Around line 1-3: Consolidate the duplicate import from "next/navigation" by
combining the two separate imports into a single import statement that includes
both redirect and notFound (leave headers imported from "next/headers" as-is);
update the file-level imports so only one import from "next/navigation" exports
redirect and notFound to remove redundancy and keep imports tidy.

In `@packages/api/src/bot/commands.ts`:
- Around line 170-171: The handlers handleTaskAdd and handleTaskNext are doing a
DB lookup per message to compute isBroadcaster; instead, add a cached
ownerTwitchId property on TwitchBotService (loaded once in start() using the
existing db.query.user.findFirst call) and propagate that value into
MessageContext so the handlers can compare strings (ownerTwitchId ===
userInfo.twitchId) instead of calling db.query.user.findFirst each time; update
TwitchBotService.start to set ownerTwitchId, adjust MessageContext shape to
include ownerTwitchId, and change handleTaskAdd and handleTaskNext to read from
context and perform a plain string compare.

In `@packages/api/src/bot/index.ts`:
- Around line 174-186: In reloadConfig, add a log when the
inserted-but-not-found case occurs: after awaiting
this.db.query.botConfig.findFirst() check if botConfigRow is falsy and call a
logger (e.g., this.logger.warn or this.logger.error) with a descriptive message
including context like "reloadConfig: botConfig row missing after insert" and
any available diagnostics (e.g., this.db transaction id, schema version, or the
inserted result) before returning so that the unexpected state where configCache
remains null is observable; update references in reloadConfig, botConfigRow, and
configCache accordingly.

In `@packages/api/src/routers/config.ts`:
- Around line 378-380: The update calls like
ctx.db.update(schema.timerConfig).set(input).returning() must be scoped to the
singleton row; add an explicit .where(eq(schema.timerConfig.id, "singleton")) to
that call and to the other five similar mutation calls (e.g., where you call
ctx.db.update(schema.<Table>).set(...).returning()). Also ensure the eq symbol
is imported/available in the file so you can reference eq(schema.<Table>.id,
"singleton") for each update. This encodes the singleton invariant and prevents
accidental multi-row updates.

In `@packages/api/src/routers/overlay.ts`:
- Around line 53-115: Duplicate DB-read + mapping logic in onTimerState,
onTaskList (and the separate getTimerState path) should be extracted into small
helpers to avoid drift; add async functions like readTimerSnapshot(ctx) and
readTaskSnapshot(ctx) that perform the Promise.all queries
(timerState/timerConfig/timerStyle and task/findMany/taskStyle respectively) and
return the mapped shape (using buildTimerConfig/buildTimerStylesConfig and
buildTaskStylesConfig). Replace the initial-yield blocks and the for-await loop
bodies in onTimerState and onTaskList (and call sites such as getTimerState) to
invoke these helpers and yield their result, ensuring all yields share the same
mapping logic.

In `@packages/api/src/routers/timer.ts`:
- Around line 89-91: The update calls like
ctx.db.update(schema.timerState).set(data).returning() currently lack a
predicate and will update all rows; add an explicit WHERE predicate such as
.where(eq(schema.timerState.id, "singleton")) to each update call to document
intent and guard the singleton invariant. Locate the update expressions (e.g.,
ctx.db.update(schema.timerState).set(data).returning()) and add
.where(eq(schema.timerState.id, "singleton")) (import eq if necessary), and
apply the same change to the other update sites in this file (the similar update
calls referenced in the review).

In `@packages/auth/src/__tests__/allowlist.test.ts`:
- Around line 1-71: Test file name no longer matches its contents: rename the
test file from allowlist.test.ts to has-owner.test.ts (or owner.test.ts) so it
reflects the single exported function under test (hasOwner) and improves
discoverability; ensure any CI/test patterns or docs referencing the old
filename are updated and keep the existing describe/it blocks as-is since they
already target hasOwner.

In `@packages/db/drizzle/meta/0000_snapshot.json`:
- Around line 1-1695: This change replaces migration history with a clean-slate
snapshot (0000_snapshot.json + replaced 0000_init.sql) so confirm there are no
production/staging databases or any contributor local DBs that must be
preserved, and update documentation (README or CLAUDE.md) with explicit
instructions to drop/recreate local dev databases before running drizzle-kit
migrate; also verify the snapshot schema (instance_config table, user.is_owner
column, task index changes) exactly matches packages/db/src/schema/app.ts and
mention the required manual upgrade/drop steps in the PR description or docs so
users are not surprised.

In `@packages/db/src/schema/auth.ts`:
- Line 14: isOwner column is correctly not-null with default false and the
single-owner invariant is enforced in the Better Auth hook user.create.before in
packages/auth/src/index.ts; if you want DB-level defense-in-depth add a
migration that creates a partial unique index (e.g. name it
user_single_owner_idx) on the "user" table for rows WHERE is_owner = true so the
database will prevent more than one owner in addition to the application-level
check.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 217cd40d-1fdb-49d7-a3ab-30974787bdc8

📥 Commits

Reviewing files that changed from the base of the PR and between 08f705b and b8fcc17.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (34)
  • .env.example
  • CLAUDE.md
  • README.md
  • apps/web/instrumentation.ts
  • apps/web/src/app/(app)/api/bot/callback/twitch/route.ts
  • apps/web/src/app/(app)/home-content.tsx
  • apps/web/src/app/(app)/page.tsx
  • apps/web/src/app/(app)/setup/page.tsx
  • apps/web/src/app/(app)/setup/setup-content.tsx
  • packages/api/src/__tests__/events.test.ts
  • packages/api/src/bot/commands.ts
  • packages/api/src/bot/index.ts
  • packages/api/src/events.ts
  • packages/api/src/routers/__tests__/config-roundtrip.test.ts
  • packages/api/src/routers/__tests__/config.test.ts
  • packages/api/src/routers/bot.ts
  • packages/api/src/routers/config.ts
  • packages/api/src/routers/overlay.ts
  • packages/api/src/routers/task.ts
  • packages/api/src/routers/timer.ts
  • packages/api/src/routers/user.ts
  • packages/auth/package.json
  • packages/auth/src/__tests__/allowlist.test.ts
  • packages/auth/src/index.ts
  • packages/db/drizzle/0000_init.sql
  • packages/db/drizzle/0001_pretty_jocasta.sql
  • packages/db/drizzle/meta/0000_snapshot.json
  • packages/db/drizzle/meta/0001_snapshot.json
  • packages/db/drizzle/meta/_journal.json
  • packages/db/src/index.ts
  • packages/db/src/schema/app.ts
  • packages/db/src/schema/auth.ts
  • packages/db/src/schema/index.ts
  • packages/env/src/server.ts
💤 Files with no reviewable changes (5)
  • CLAUDE.md
  • .env.example
  • packages/env/src/server.ts
  • packages/db/drizzle/0001_pretty_jocasta.sql
  • packages/db/drizzle/meta/0001_snapshot.json

Comment thread apps/web/src/app/(app)/setup/setup-content.tsx
Comment thread packages/api/src/routers/user.ts
Comment thread packages/auth/src/index.ts
- OAuth error codes: replace hardcoded error=instance_claimed with
  error=signin_failed on both setup and home sign-in buttons; add
  toast handler for signin_failed on home page. The instance_claimed
  code is reserved for future server-side detection; generic failures
  now get a neutral message instead of a misleading claim toast.

- regenerateOverlayToken: replace insert...onConflictDoUpdate with a
  bare update(...).where(eq(id, 'singleton')) to avoid silently
  minting the other token via schema $defaultFn when instanceConfig
  hasn't been provisioned yet.

- Single-owner TOCTOU: add partial unique index
  user_single_owner_idx ON "user"(is_owner) WHERE is_owner = true
  to enforce the single-owner invariant at the DB level. The app-level
  count check still runs for a friendly FORBIDDEN in the common path;
  the DB constraint handles concurrent races.

Co-Authored-By: Claude Sonnet 4.6 <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.

1 participant