Skip to content

Security hardening, test coverage, and deployment docs#5

Merged
nathanialhenniges merged 24 commits intomainfrom
dev
Mar 26, 2026
Merged

Security hardening, test coverage, and deployment docs#5
nathanialhenniges merged 24 commits intomainfrom
dev

Conversation

@nathanialhenniges
Copy link
Copy Markdown
Member

@nathanialhenniges nathanialhenniges commented Mar 15, 2026

Summary

  • CRITICAL: Enforce ALLOWED_TWITCH_IDS allowlist — was defined/documented but never checked; any Twitch user could log in regardless. Now blocked at account creation via databaseHooks.user.create.before.
  • Security hardening — timer transition enum validation, command alias input bounds, Twitch token revocation on bot disconnect, and security headers (HSTS, CSP-adjacent, X-Frame-Options).
  • Test coverage — 90+ new tests across 8 new/expanded test files covering all previously untested routers (task, timer, user, overlay, config aliases) and auth allowlist parsing. Total: 175 tests (up from ~85).
  • Deployment docs — Added missing nixpacks.toml (referenced in deployment guide but didn't exist) and CONTRIBUTING.md.

Security Fixes

Fix File Description
Enforce allowlist packages/auth/src/index.ts databaseHooks.user.create.before checks ALLOWED_TWITCH_IDS
Enum validation packages/api/src/routers/timer.ts z.string()z.enum([...]) on transition status
Alias bounds packages/api/src/routers/config.ts Key max 50 chars, value max 100, max 50 aliases
Token revocation packages/api/src/routers/user.ts Calls Twitch revoke endpoint before deleting bot account
Security headers apps/web/next.config.ts HSTS, X-Content-Type-Options, X-Frame-Options, Referrer-Policy, Permissions-Policy

Test plan

  • bun run check-types — no type errors
  • bun run test — 175 tests passing across 16 files
  • bun run build — production build succeeds
  • Manual: attempt login with Twitch ID not in allowlist (when set) — should be rejected
  • Manual: call timer.transition with invalid status — should get Zod error
  • Manual: verify security headers in browser Network tab

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added OAuth CSRF protection for bot authorization.
    • Introduced configurable default cycles for timer settings.
  • Bug Fixes

    • Fixed task promotion logic to only activate next task when completing an active task.
    • Corrected task ordering queries and ETA time formatting.
    • Resolved bot token expiration handling during auth refresh.
  • Security

    • Added bot owner authorization checks for bot control operations.
    • Configured Docker to run as non-root user.
    • Updated Twitch API scopes for improved permissions.
  • Documentation

    • Migrated build system from pnpm to Bun.
    • Updated Docker pipeline configuration.

nathanialhenniges and others added 19 commits February 14, 2026 10:44
- Add LICENSE file (MIT)
- Use next/link for Privacy Policy and Terms of Service links in docs
  footer so basePath is prepended correctly on GitHub Pages

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Enable the Sponsor button on the repository by adding FUNDING.yml
with the nathanialhenniges GitHub Sponsors profile.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tings

DW-2: Configure GitHub repo settings
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace Nixpacks with Dockerfile-based builds to fix Coolify deployment
failures caused by missing DATABASE_URL during build. Unify Prisma config
to use process.env with fallback, and make migrations non-fatal so the
app starts even if the database isn't immediately reachable.

- Switch Dockerfile base to node:24-alpine with pnpm cache mounts
- Delete nixpacks.toml (no longer needed)
- Merge prisma.docker.config.ts into prisma.config.ts using process.env fallback
- Copy full packages/db into production image for Prisma auto-discovery
- Update entrypoint: cd into packages/db, use npx, non-fatal migrations

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…timer controls

Replace disabled:pointer-events-none with disabled:cursor-not-allowed on input, button, and select primitives so users see a not-allowed cursor on disabled fields. Add data-disabled:cursor-not-allowed to slider. Extract TimerProvider context and show "Stop the timer to change settings" hint when timer is running. Includes dashboard layout, theme center, and bot settings refinements.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Overlay tokens are now blurred by default to prevent accidental exposure.
A single Eye/EyeOff toggle in the card header reveals both URLs. Copy
button works regardless of blur state since it reads from the variable.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace overlay URL blur with password field toggle
- Add scrollable task list (max-h-400px) on dashboard
- Add missing style editor controls (task border width, checkbox margins)
- Implement active task status with auto-activation and glow animation
- Add Twitch bot service with twurple (chat connect, command handling, ban/timeout)
- Add bot commands: task (!task, !done, !edit, !remove, !focus, !check, !next, !help, !clear) and timer (!timer start/pause/resume/skip/reset/eta)
- Add bot tRPC router (status/start/stop) with dashboard status pill and controls
- Auto-start bot on server boot via Next.js instrumentation
- Stop bot on disconnect, reload config on settings save
- Update OAuth scopes to include channel:moderate and user:read:chat

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rewrite chat-commands.mdx to match actual bot implementation (fix !clear
subcommands, add !focus, correct !edit/!remove signatures, update timer
commands). Add DOCS_URL optional env var so the bot can link viewers to
the commands reference page. Add always-enabled !dwhelp and !dwcommands
meta-commands that respond with the docs link or an inline summary.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…yout

- Replace pnpm/Prisma with Bun workspaces and Drizzle ORM (schema-as-code)
- Add Twitch bot runtime with active task state and !dwhelp/!dwcommands commands
- Refactor bot settings page: combine toggle cards, tabbed message editors, full-width commands reference
- Update CI/CD workflows, Dockerfile, and docker-entrypoint for Bun-based build
- Update docs (deployment, troubleshooting) and README

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Consolidate bot settings components by extracting BotMessagesCard,
inlining command alias and variable reference into commands-reference,
and removing unused checkbox component. Add unit tests for bot command
parsing and matching logic.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Enforce ALLOWED_TWITCH_IDS allowlist on account creation (was defined but never checked)
- Restrict timer.transition status to enum instead of arbitrary string
- Add length/count limits to command alias input validation
- Revoke Twitch token on bot disconnect before deleting row
- Add security headers (HSTS, X-Content-Type-Options, X-Frame-Options, etc.)
- Add router tests for task, timer, user, overlay, and config aliases (90 new tests)
- Add auth allowlist parsing tests and expand bot command tests
- Add nixpacks.toml for Coolify deployment and CONTRIBUTING.md

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 15, 2026

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

This PR consolidates updates across documentation, Docker configuration, and API/database code. Changes include converting package manager from pnpm to Bun in README, enhancing OAuth authorization with CSRF nonce verification, refining task completion logic to conditionally promote pending tasks, adding a defaultCycles timer configuration parameter, introducing authorization checks for bot operations, updating Twitch OAuth scopes, and adjusting Docker user permissions and environment validation handling.

Changes

Cohort / File(s) Summary
Documentation & Deployment
README.md, CLAUDE.md, Dockerfile, docker-entrypoint.sh
Updated package manager references from pnpm to Bun, changed Drizzle migration documentation, added non-root user execution with ownership/permission fixes, and enabled environment validation skip at startup.
OAuth Security
apps/web/src/app/(app)/api/bot/authorize/route.ts, apps/web/src/app/(app)/api/bot/callback/twitch/route.ts
Added cryptographic nonce generation, CSRF state payload encoding, httpOnly cookie storage, and timing-safe nonce verification in callback; updated Twitch OAuth scopes from chat moderation to read/write chat focus.
Bot Service & Operations
apps/web/instrumentation.ts, packages/api/src/bot/index.ts, packages/api/src/bot/commands.ts, packages/api/src/routers/bot.ts
Enhanced logging with userId, added getOwnerId() public method, fixed token expiration fallback logic, adjusted task ordering to descending, updated ETA locale formatting, and introduced authorization check restricting bot stop operations to owner.
Timer Configuration
packages/api/src/routers/timer-logic.ts, packages/api/src/routers/timer.ts, packages/api/src/routers/__tests__/timer-logic.test.ts
Added defaultCycles parameter to timer config defaults (value: 4), extended TimerConfigInput interface, updated getTimerConfig to return the new field, and modified timerRouter.start to use config default instead of hardcoded fallback.
Task Management
packages/api/src/routers/task.ts
Refined markDone logic to query existing task status and only promote pending tasks to active when the completed task previously had active status, changing from unconditional promotion.
Config Management
packages/api/src/routers/config.ts
Added ensureUserConfig initialization calls across multiple update endpoints and simplified not-found error messages from instructional to generic.
Database Schema
packages/db/src/schema/app.ts
Updated botAccount.scopes default from chat moderation array (["chat:read", "chat:edit", "channel:moderate", "user:read:chat"]) to read/write chat array (["user:read:chat", "user:write:chat"]).
Frontend
apps/web/src/app/(app)/dashboard/bot/bot-settings-page.tsx
Added router to useEffect dependency list for bot connection callback query parameter handling.

Sequence Diagram

sequenceDiagram
    actor User
    participant WebApp as Web App
    participant TwitchOAuth as Twitch OAuth
    participant Callback as Callback Route
    participant DB as Database/Cookies

    User->>WebApp: Initiate bot authorization
    WebApp->>WebApp: Generate nonce & state (base64url)
    WebApp->>DB: Set httpOnly cookie with nonce
    WebApp->>TwitchOAuth: Redirect with state (includes nonce)
    
    User->>TwitchOAuth: Authorize scopes
    TwitchOAuth->>Callback: Redirect with auth code & state
    
    Callback->>DB: Read bot_oauth_nonce cookie
    Callback->>Callback: Decode state, extract nonce
    
    alt Nonce Verification Pass
        Callback->>TwitchOAuth: Exchange code for token
        TwitchOAuth-->>Callback: Access token & user info
        Callback->>DB: Upsert botAccount
        Callback-->>User: Redirect success
    else Nonce Verification Fail
        Callback-->>User: Redirect error (CSRF rejection)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A nonce hops in, a cookie sets secure,
Tasks complete with wisdom—only promote the sure!
Timers count their cycles, config keeps the tune,
OAuth guards with timing-safe salutes 'neath moon.
Scopes reshape from chat to write, a rabbit's delight! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.86% 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 title 'Security hardening, test coverage, and deployment docs' directly aligns with the main objectives: security improvements, expanded test coverage, and deployment documentation updates across multiple files.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev

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.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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: 19

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (14)
README.md (8)

141-141: ⚠️ Potential issue | 🟠 Major

Update database start command from pnpm to bun.

📦 Proposed fix
-   pnpm db:start
+   bun db:start
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 141, Update the README entry that currently shows the
command string "pnpm db:start" to use Bun instead: replace the literal "pnpm
db:start" with "bun db:start" so docs reflect the new startup approach; search
for the exact token "pnpm db:start" in README.md and update it wherever present
(e.g., the command example shown on the docs line).

158-164: ⚠️ Potential issue | 🟠 Major

Update Development Scripts from pnpm to bun.

The Development Scripts section inconsistently uses pnpm (lines 158-164) and bun run (lines 165-168). All commands should use bun to align with the Bun workspaces migration indicated in line 100.

📦 Proposed fix to standardize on bun
-- `pnpm dev` - Start all apps (web on port 3001, docs on port 4000)
-- `pnpm build` - Build all apps for production
-- `pnpm check-types` - Run TypeScript type checking
-- `pnpm test` - Run unit tests across all packages
-- `pnpm dev:web` - Start the web app only
-- `pnpm db:start` - Start PostgreSQL via Docker
-- `pnpm db:stop` - Stop PostgreSQL
+- `bun dev` - Start all apps (web on port 3001, docs on port 4000)
+- `bun build` - Build all apps for production
+- `bun check-types` - Run TypeScript type checking
+- `bun test` - Run unit tests across all packages
+- `bun dev:web` - Start the web app only
+- `bun db:start` - Start PostgreSQL via Docker
+- `bun db:stop` - Stop PostgreSQL
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 158 - 164, The README's Development Scripts list
still references pnpm for commands (`pnpm dev`, `pnpm build`, `pnpm
check-types`, `pnpm test`, `pnpm dev:web`, `pnpm db:start`, `pnpm db:stop`);
update each of these to use bun (e.g., replace `pnpm` with `bun run` or the
appropriate `bun` invocation) so the Development Scripts section is consistent
with the Bun workspaces migration and the other commands already using `bun`.

176-176: ⚠️ Potential issue | 🟠 Major

Update test command from pnpm to bun.

📦 Proposed fix
-- Run with `pnpm test`
+- Run with `bun test`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 176, Update the test command text that currently reads
"Run with `pnpm test`" to use bun instead: replace the string "Run with `pnpm
test`" with "Run with `bun test`" (locate the exact sentence "Run with `pnpm
test`" in the README and change it to "Run with `bun test`").

147-147: ⚠️ Potential issue | 🟠 Major

Update schema push command from pnpm to bun.

📦 Proposed fix
-   pnpm db:push
+   bun db:push
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 147, The README shows the outdated command string "pnpm
db:push"; update the documentation to use the correct runtime command by
replacing the token "pnpm db:push" with "bun db:push" so examples and setup
instructions reflect the current use of Bun (search for the literal "pnpm
db:push" in README.md and update that occurrence).

153-153: ⚠️ Potential issue | 🟠 Major

Update dev server command from pnpm to bun.

📦 Proposed fix
-   pnpm dev
+   bun dev
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 153, The README currently instructs to run the dev server
with "pnpm dev"; update the command to "bun dev" wherever the dev server start
command appears (replace the "pnpm dev" line in README.md with "bun dev") so the
documentation matches the new dev workflow.

124-124: ⚠️ Potential issue | 🟠 Major

Update Setup install command from pnpm to bun.

📦 Proposed fix
-   pnpm install
+   bun install
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 124, Update the README install command by replacing the
existing "pnpm install" instruction with "bun install"; search for the "pnpm
install" string in the README (and any other docs) and change it to "bun
install" to ensure the documented setup steps match the project's package
manager.

107-107: ⚠️ Potential issue | 🟠 Major

Update prerequisite from pnpm to Bun.

The Prerequisites section lists pnpm 10+, but line 100 indicates the project uses "Bun workspaces". This inconsistency will confuse new users and contributors.

📦 Proposed fix to align with Bun migration
-- pnpm 10+
+- Bun 1.0+
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 107, The Prerequisites section currently says "pnpm 10+"
which conflicts with the project using "Bun workspaces"; update the README to
replace the "pnpm 10+" entry with a Bun prerequisite (e.g., "Bun 1+"), add a
brief note that the repo uses Bun workspaces, and optionally include a one-line
install or link reference so new contributors know to install Bun before running
workspace commands.

39-44: ⚠️ Potential issue | 🟠 Major

Update Getting Started commands from pnpm to bun.

The Getting Started section uses pnpm commands, but line 100 indicates the project has migrated to "Bun workspaces". These commands should be updated to use bun.

📦 Proposed fix to align with Bun migration
-2. Install dependencies with `pnpm install`
+2. Install dependencies with `bun install`
 3. Configure your `apps/web/.env` file
-4. Start PostgreSQL with `pnpm db:start`
-5. Push the database schema with `pnpm db:push`
-6. Start the dev server with `pnpm dev`
+4. Start PostgreSQL with `bun db:start`
+5. Push the database schema with `bun db:push`
+6. Start the dev server with `bun dev`
 7. Open `http://localhost:3001` and sign in with Twitch
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 39 - 44, The Getting Started commands in README.md
still use pnpm; update them to Bun equivalents: replace "pnpm install" with "bun
install", and replace script invocations "pnpm db:start", "pnpm db:push", and
"pnpm dev" with "bun run db:start", "bun run db:push", and "bun run dev" (or
"bun db:start" etc. if your repo uses Bun shortcuts); keep the rest of the steps
(config .env, URL, Twitch sign-in) unchanged and ensure the exact strings "pnpm
install", "pnpm db:start", "pnpm db:push", and "pnpm dev" are removed/updated in
README.md.
apps/web/src/app/(app)/dashboard/bot/bot-settings-page.tsx (1)

115-125: ⚠️ Potential issue | 🟡 Minor

Missing router in useEffect dependency array.

The effect uses router.replace() but router is not included in the dependency array. While this may work due to router stability, it can cause lint warnings and is technically incorrect.

🔧 Proposed fix
   }, [searchParams]);
+  }, [searchParams, router]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/app/`(app)/dashboard/bot/bot-settings-page.tsx around lines 115
- 125, The useEffect block that reads searchParams and calls router.replace
(inside the useEffect defined around the bot status handling) is missing router
from its dependency array; update the effect to include router in the dependency
array (e.g., change the deps to [searchParams, router]) so that the hook
correctly declares all external values it uses (references: useEffect,
searchParams.get, router.replace, toast.success/toast.error).
Dockerfile (1)

27-42: ⚠️ Potential issue | 🟠 Major

Drop root in the runtime stage.

The runner never switches away from the default root user, so server.js and docker-entrypoint.sh execute with full container privileges. Docker recommends setting USER for services that do not require elevated access. (docs.docker.com)

🔒 Proposed hardening
 FROM node:24-alpine AS runner
 RUN apk add --no-cache libc6-compat curl
 WORKDIR /app
 ENV NODE_ENV=production
 ENV HOSTNAME="0.0.0.0"
 ENV PORT=3000
 
 COPY --from=build /app/apps/web/.next/standalone ./
 COPY --from=build /app/apps/web/.next/static ./apps/web/.next/static
 COPY --from=build /app/packages/db/migrate.js ./packages/db/migrate.js
 COPY --from=build /app/packages/db/drizzle ./packages/db/drizzle
 COPY docker-entrypoint.sh ./
+RUN chown -R node:node /app && chmod +x /app/docker-entrypoint.sh
+USER node
 
 EXPOSE 3000
 ENTRYPOINT ["./docker-entrypoint.sh"]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 27 - 42, The runtime Dockerfile uses the root user
in the runner stage so docker-entrypoint.sh and server processes run as root;
create a non-root user/group in the runner stage (e.g., addgroup/adduser or use
the existing node user), chown the application files and the
docker-entrypoint.sh to that user (ensure the script stays executable), and set
USER to that non-root account before ENTRYPOINT; update symbols in this file:
the runner stage, COPY lines (copied files and docker-entrypoint.sh), and
ENTRYPOINT to apply the ownership change and user switch.
apps/web/src/app/(app)/api/bot/callback/twitch/route.ts (1)

22-35: ⚠️ Potential issue | 🟠 Major

Generate a cryptographically random state per authorization request and verify it server-side.

The state parameter is only a base64url-encoded userId, which is deterministic and forgeable. An attacker who knows the victim's user ID can craft a valid state and bypass the CSRF protection that state is meant to provide. Generate a random nonce in the authorize route, persist it (server-side or in a signed httpOnly cookie), and verify an exact match in the callback before exchanging the code. See RFC 6819 § 5.2.4.1.

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

In `@apps/web/src/app/`(app)/api/bot/callback/twitch/route.ts around lines 22 -
35, The callback currently trusts a base64url-encoded userId in state; change
the flow so the authorize route creates a cryptographically secure random nonce
(per request), stores it server-side or in a signed httpOnly cookie tied to the
user's session, and sends that nonce as the OAuth state; then in route.ts
replace the JSON.parse(Buffer.from(state,...)) logic with verification that the
received state exactly equals the stored/signed nonce (using the same session
from auth.api.getSession or the signed cookie) and call errorRedirect(request,
...) on mismatch; ensure the nonce generation uses a secure RNG and that the
storage/retrieval logic is implemented where the authorize handler and the
callback handler (this route) share access.
CLAUDE.md (2)

215-215: ⚠️ Potential issue | 🟡 Minor

Inconsistent command reference: pnpm test should be bun run test.

The file documents the migration from pnpm to Bun (lines 35-48), but this line still references pnpm test. Update for consistency.

-Vitest unit tests across `packages/api` and `apps/web`. Run with `pnpm test`.
+Vitest unit tests across `packages/api` and `apps/web`. Run with `bun run test`.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` at line 215, Update the inconsistent test command in the CLAUDE.md
entry that currently reads "Vitest unit tests across `packages/api` and
`apps/web`. Run with `pnpm test`" to instead state "Run with `bun run test`";
locate the exact string in the file and replace `pnpm test` with `bun run test`
so the documented command matches the Bun migration described earlier.

249-249: ⚠️ Potential issue | 🟡 Minor

Stale reference to Prisma in Docker build documentation.

The project migrated from Prisma to Drizzle ORM (documented throughout this file), but this line still references "generate Prisma client". Update to reflect Drizzle-based workflow.

-- Docker build: install deps → generate Prisma client → build Next.js → copy static assets
+- Docker build: install deps → run Drizzle migrations → build Next.js → copy static assets
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` at line 249, Update the stale Docker build step text that reads
"install deps → generate Prisma client → build Next.js → copy static assets" to
reflect the Drizzle ORM workflow; replace "generate Prisma client" with a
Drizzle-appropriate step such as "run Drizzle migrations / generate Drizzle
client (if applicable)" or "run Drizzle migrations" so the Docker build sequence
correctly documents installing dependencies, applying Drizzle migrations (or
generating any Drizzle artifacts), building Next.js, and copying static assets.
packages/api/src/routers/timer.ts (1)

108-130: ⚠️ Potential issue | 🟠 Major

transition() can still write impossible timer states.

A "paused" transition here never fills pausedFromStatus or pausedWithRemaining, timed statuses are allowed without durationMs, and the truthy if (input.durationMs) check also drops an explicit 0. That leaves rows the pause/resume/countdown logic can't interpret safely.

🤖 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 108 - 130, The transition
mutation can produce invalid timer rows; fix it by first fetching the current
row (select from schema.timerState where userId = ctx.session.user.id) then
validate and populate fields deterministically: treat durationMs present only
when input.durationMs !== undefined (so 0 is accepted); for timed statuses
("starting","work","break","longBreak") require durationMs and set targetEndTime
= new Date(Date.now() + input.durationMs) and clear
pausedFromStatus/pausedWithRemaining; for "paused" set pausedFromStatus to the
fetched current.status and set pausedWithRemaining to the remaining ms computed
from fetched.targetEndTime - Date.now() (or use input.durationMs if provided),
and clear targetEndTime; for "idle" and "finished" clear targetEndTime,
pausedFromStatus and pausedWithRemaining; finally use
ctx.db.update(schema.timerState).set(...) with these validated values and return
the result. Ensure you reference the existing transition protectedProcedure,
schema.timerState, pausedFromStatus, pausedWithRemaining, and targetEndTime
symbols when making changes.
🟡 Minor comments (2)
packages/api/src/bot/commands.ts-104-106 (1)

104-106: ⚠️ Potential issue | 🟡 Minor

Semantic mismatch: Timer permission denial uses task message template.

When a non-mod user attempts a !timer command, the response uses config.task.notMod instead of a timer-specific message. This could cause confusing user feedback if task and timer permission messages differ.

💡 Suggestion

Consider adding a notMod field to config.timer for consistency, or document that config.task.notMod is the shared permission-denied message.

🤖 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 104 - 106, The
permission-denied message for the !timer command incorrectly uses
config.task.notMod; update the handler that checks userInfo.isMod (the branch
calling say(interpolate(...))) to use a timer-specific message key (e.g.,
config.timer.notMod), and if you add config.timer.notMod ensure you fall back to
config.task.notMod when the timer key is absent so behavior remains
backward-compatible; locate the check around userInfo.isMod in the !timer
command handler in commands.ts and replace or augment the
interpolate(config.task.notMod, vars) call accordingly.
apps/web/instrumentation.ts-7-15 (1)

7-15: ⚠️ Potential issue | 🟡 Minor

Clarify behavior when multiple bot accounts exist.

findFirst without ordering returns an arbitrary bot account. If multiple users have connected bot accounts, the auto-started bot may not be deterministic across restarts. Consider:

  1. Adding explicit ordering (e.g., by createdAt)
  2. Logging which user's bot is being started
  3. Documenting that only one bot instance runs globally
🔍 Suggested logging improvement
     if (botAccount && !botService.isRunning()) {
       await botService.start(db, botAccount.userId);
-      console.log("[Instrumentation] Bot auto-started");
+      console.log(`[Instrumentation] Bot auto-started for user ${botAccount.userId}`);
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/instrumentation.ts` around lines 7 - 15, The current use of
db.query.botAccount.findFirst returns an arbitrary botAccount; change it to
select a deterministic record (e.g., use db.query.botAccount.findFirst with an
explicit orderBy: { createdAt: "asc" } or similar) so the same bot is chosen
across restarts, then log the chosen userId before/after calling
botService.start (reference botAccount and botService.start) so callers see
which user's bot was auto-started, and add a short comment/docstring near this
block clarifying that only one global bot instance is started.
🧹 Nitpick comments (18)
apps/web/src/index.css (1)

164-173: Use CSS custom properties for theme-aware glow colors.

The hardcoded rgba(59, 130, 246, ...) values are inconsistent with the established theming pattern throughout this file, which uses CSS custom properties for all colors. This prevents the glow from adapting to light/dark themes.

♻️ Proposed fix to use theme-aware colors

Add color variables to the theme definitions:

 :root {
   --background: oklch(0.97 0.005 260);
   --foreground: oklch(0.18 0.02 260);
+  --active-glow: oklch(0.58 0.18 260 / 0.3);
+  --active-glow-strong: oklch(0.58 0.18 260 / 0.6);
   --card: oklch(1 0 0 / 60%);

And in the .dark theme:

 .dark {
   --background: oklch(0.13 0.01 260);
   --foreground: oklch(0.95 0.01 260);
+  --active-glow: oklch(0.68 0.18 260 / 0.3);
+  --active-glow-strong: oklch(0.68 0.18 260 / 0.6);
   --card: oklch(1 0 0 / 6%);

Then update the animation:

 `@keyframes` active-glow {
   0%, 100% {
-    box-shadow: 0 0 4px 1px rgba(59, 130, 246, 0.3);
+    box-shadow: 0 0 4px 1px var(--active-glow);
   }
   50% {
-    box-shadow: 0 0 12px 4px rgba(59, 130, 246, 0.6);
+    box-shadow: 0 0 12px 4px var(--active-glow-strong);
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/index.css` around lines 164 - 173, The keyframes animation
active-glow uses hardcoded rgba(59, 130, 246, ...) which breaks theme-aware
styling; add a CSS custom property (e.g. --color-glow or --color-glow-rgb) to
your theme root and .dark theme definitions and then replace the hardcoded rgba
values inside `@keyframes` active-glow with the custom property (using either
rgba(var(--color-glow-rgb), alpha) or color-mix/hex+alpha) so the glow color
adapts to light/dark themes; update the theme definitions where other color
variables are declared and modify `@keyframes` active-glow to reference the new
variable.
apps/web/src/components/timer-controls.tsx (1)

386-387: Consider handling empty input edge case.

When the input is cleared, Number(e.target.value) returns 0, which would result in 0 * 60000 = 0 being saved for durations. The HTML min={1} only prevents submitting values below 1 via form validation, but onChange still fires with empty string or 0.

This could cause unexpected behavior if a user clears the field and tabs away (triggering onBlur). You might want to validate before saving.

💡 Optional: Add validation guard in onBlur
         onChange={(e) => setWorkMin(Number(e.target.value))}
-        onBlur={() => saveConfig({ workDuration: minutesToMs(workMin) })}
+        onBlur={() => workMin >= 1 && saveConfig({ workDuration: minutesToMs(workMin) })}

Apply similar guards to other inputs.

Also applies to: 406-407, 426-427, 446-447, 466-467

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

In `@apps/web/src/components/timer-controls.tsx` around lines 386 - 387, The
onBlur currently saves durations using minutesToMs(workMin) even when the input
was cleared or invalid (e.g., empty string -> Number(...) -> 0); add a
validation guard in the onBlur handlers (where saveConfig is called) to only
call saveConfig with minutesToMs(...) when the corresponding state (e.g.,
workMin for the work input) is a positive integer (>=1), otherwise skip saving
or restore a default/previous value; update the same pattern for the other
inputs mentioned (the handlers using setShortBreakMin/saveConfig,
setLongBreakMin/saveConfig, etc.) so empty or non-positive values do not produce
0ms durations.
apps/web/src/components/theme-center/font-select.tsx (1)

44-53: Prefer useId()-backed IDs to avoid duplicate DOM ids.

font-${label...} can collide when the same label appears more than once on a page.

♻️ Suggested patch
+"use client";
+
+import { useId } from "react";
 import {
   Select,
   SelectContent,
@@
 export function FontSelect({
@@
 }) {
-  const id = `font-${label.toLowerCase().replace(/\s+/g, "-")}`;
+  const reactId = useId();
+  const id = `font-${label.toLowerCase().replace(/\s+/g, "-")}-${reactId}`;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/components/theme-center/font-select.tsx` around lines 44 - 53,
Replace the manual id generation that uses the label string with a stable,
unique id generated from React's useId() to avoid DOM id collisions: import and
call useId() inside the FontSelect component, combine that id with the label
suffix (e.g., `${baseId}-font-${sanitizedLabel}`) and use that resulting id for
the Label htmlFor and SelectTrigger id (the current id variable), ensuring you
still sanitize or normalize the label if needed and preserve the existing
props/value handling in the component.
apps/web/src/components/header.tsx (1)

71-88: Optional: add aria-controls linkage for the mobile menu button.

You already expose aria-expanded; adding aria-controls on the button and an id on the mobile <nav> would improve SR context for the controlled region.

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

In `@apps/web/src/components/header.tsx` around lines 71 - 88, Add an accessible
linkage between the mobile toggle button and the mobile nav by giving the <nav>
element a stable id (e.g., "mobile-menu" or "mobile-nav") and adding
aria-controls="{that id}" to the Button that uses setMobileMenuOpen and
mobileMenuOpen; ensure aria-controls matches the nav id and remains present when
the nav is rendered so screen readers can associate aria-expanded on the Button
with the controlled <nav>.
apps/web/src/components/bot-settings/message-editor.tsx (1)

56-189: Consider extracting shared editor logic to reduce duplication.

TaskMessageEditor and TimerMessageEditor have nearly identical structures (search input, disabled message, filtered grid rendering). A generic MessageEditor component could accept fields, messages, onChange, disabled, and an idPrefix prop.

♻️ Optional refactor sketch
// Generic editor that both task and timer editors could use
function MessageEditor<T extends Record<string, string>>({
  fields,
  messages,
  onChange,
  disabled,
  idPrefix,
  disabledMessage,
}: {
  fields: { key: keyof T; label: string; placeholder: string }[];
  messages: T;
  onChange: (messages: T) => void;
  disabled?: boolean;
  idPrefix: string;
  disabledMessage: string;
}) {
  const [search, setSearch] = useState("");
  const filteredFields = fields.filter((f) =>
    f.label.toLowerCase().includes(search.toLowerCase())
  );
  // ... rest of shared logic
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/components/bot-settings/message-editor.tsx` around lines 56 -
189, Both TaskMessageEditor and TimerMessageEditor duplicate the same UI/logic;
extract a generic MessageEditor component to remove duplication. Create a
reusable MessageEditor<T> that accepts fields, messages, onChange, disabled,
idPrefix, and disabledMessage props, move the shared search state, filtering
(use fields.filter... and label matching), grid rendering,
label/input/placeholder rendering and handleChange into it, then replace
TaskMessageEditor and TimerMessageEditor to call MessageEditor with their
specific fields (taskMessageFields/timerMessageFields), messages, and idPrefix
("task"/"timer") while keeping the existing handleChange signature used by
parent components.
packages/db/src/migrate.ts (1)

9-11: Prefer natural process exit over process.exit(0).

On Line 11, forcing exit can prematurely terminate cleanup/flush paths. Use try/catch and process.exitCode instead.

Proposed refactor
-await migrate(db, { migrationsFolder });
-console.log("Migrations applied successfully");
-process.exit(0);
+try {
+  await migrate(db, { migrationsFolder });
+  console.log("Migrations applied successfully");
+} catch (error) {
+  console.error("Migration failed", error);
+  process.exitCode = 1;
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/db/src/migrate.ts` around lines 9 - 11, The current code calls
process.exit(0) after await migrate(db, { migrationsFolder }) which can abort
pending flush/cleanup; wrap the migration call in a try/catch, set
process.exitCode = 0 on success (instead of process.exit(0)), and in the catch
set process.exitCode = 1 and log the error via console.error (or the project
logger) so any finally/cleanup and async I/O can complete; ensure you reference
the migrate call and the migrationsFolder/db variables when editing this block.
packages/auth/src/index.ts (1)

9-21: Move the parser into a tiny helper module.

Because parseAllowedTwitchIds is pure, keeping it in index.ts forces the test to mock the entire auth bootstrap just to import one helper. Extracting it to something like allowlist.ts would make both the auth config and its tests simpler.

As per coding guidelines, "Extract new pure functions into testable modules (not inline in components/routers) and add corresponding Vitest tests".

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

In `@packages/auth/src/index.ts` around lines 9 - 21, Extract the pure helper
parseAllowedTwitchIds out of index.ts into a new module (e.g., allowlist.ts)
that exports the function, update any imports in packages/auth to import
parseAllowedTwitchIds from the new module (replace references in index.ts or
auth bootstrap), and add a small Vitest unit test file for allowlist.ts covering
empty/undefined, comma-separated values, trimming, and filtering empty entries;
ensure the exported function signature and behavior (Set<string> return) remain
unchanged.
packages/api/package.json (1)

11-12: Prefer explicit deep exports over a catch-all pattern.

"./*/*" makes every two-level file under src/ part of the package contract. That broadens the semver surface area for internals like routers and bot helpers; explicit subpath exports are easier to evolve safely.

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

In `@packages/api/package.json` around lines 11 - 12, Replace the catch-all deep
export pattern ("./*/*": { "default": "./src/*/*.ts" }) with explicit subpath
exports for only the public modules your package intends to expose; remove the
"./*/*" entry and add individual entries mapping each public subpath (e.g.,
"./foo": "./src/foo/index.ts", "./bar/utils": "./src/bar/utils.ts") so internal
routers/helpers remain non-exported, and ensure the corresponding build outputs
and type files match those explicit paths.
CLAUDE.md (1)

22-22: Add language specification to fenced code block.

Static analysis flagged this code block as missing a language tag. Since it shows directory structure, use text or leave empty but explicit.

-```
+```text
 apps/web           → Next.js 16 app (frontend + API), port 3001
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` at line 22, The fenced code block in CLAUDE.md is missing a
language tag; update that fence to include a language specifier (e.g., change
the opening ``` to ```text) so the block explicitly reads as text (for example:
```text followed by "apps/web           → Next.js 16 app (frontend + API), port
3001" and then the closing ```).
packages/api/src/routers/__tests__/user.test.ts (2)

59-66: Token generation test validates crypto API behavior, not router logic.

This test verifies that crypto.randomUUID() produces valid UUIDs, which is testing Node.js/browser API behavior rather than application code. Consider focusing tests on the router's usage of UUID generation instead.

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

In `@packages/api/src/routers/__tests__/user.test.ts` around lines 59 - 66, The
test "token generation" currently asserts Node's crypto.randomUUID behavior
(crypto.randomUUID()), which tests the platform API instead of your router;
remove or replace this test so it verifies how the router uses UUIDs (e.g.,
stub/mock crypto.randomUUID and assert the router/handler that generates tokens
calls it and stores/returns the resulting token). Update the test in
packages/api/src/routers/__tests__/user.test.ts to either delete the
describe("token generation") block or rewrite it to spy on crypto.randomUUID and
assert the router function responsible for token creation invokes it and handles
the returned value correctly.

4-8: Consider importing schemas from the actual router for tighter coupling.

The test defines local copies of overlayTokenSchema and regenerateSchema instead of importing them from the router. This means tests could pass even if the actual router schemas diverge.

💡 Suggestion

Export the schemas from user.ts and import them here:

// In user.ts
export const overlayTokenSchema = z.object({
  token: z.string(),
  type: z.enum(["timer", "tasks"]),
});

// In user.test.ts
import { overlayTokenSchema, regenerateSchema } from "../user";

This ensures tests validate the actual schemas used by the router.

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

In `@packages/api/src/routers/__tests__/user.test.ts` around lines 4 - 8, The test
defines local overlayTokenSchema and regenerateSchema which can drift from the
router's definitions; export the schemas (overlayTokenSchema and
regenerateSchema) from the router module (the user router) and replace the local
schema declarations in the test with imports of those exported symbols so the
tests validate the actual router schemas used at runtime.
packages/api/src/bot/commands.ts (1)

698-700: toLocaleTimeString may produce inconsistent output across server environments.

Without an explicit locale and timezone, the output format depends on the server's locale settings, which can vary between development and production environments.

💡 Proposed fix for consistent formatting
-      const timeStr = etaDate.toLocaleTimeString([], { hour: "2-digit", minute: "2-digit" });
+      const timeStr = etaDate.toLocaleTimeString("en-US", { hour: "2-digit", minute: "2-digit", hour12: true });

Or use a library like date-fns for more predictable formatting.

🤖 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 698 - 700, toLocaleTimeString
is used without explicit locale/timezone which can vary across environments;
update the formatting of etaDate (where timeStr is created) to use a
deterministic formatter—either replace toLocaleTimeString with
Intl.DateTimeFormat providing an explicit locale (e.g., "en-US" or "en-GB") and
an explicit timeZone (e.g., "UTC" or server/user timezone), or switch to a
stable library like date-fns and format(etaDate, "HH:mm" or "hh:mm a") so the
output of timeStr is consistent across environments.
packages/api/src/routers/__tests__/config-aliases.test.ts (1)

4-10: Same schema duplication concern applies here.

The commandAliasesSchema mirrors the production schema. Consider exporting and importing from config.ts to prevent drift.

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

In `@packages/api/src/routers/__tests__/config-aliases.test.ts` around lines 4 -
10, The test declares a duplicated schema (commandAliasesSchema) that should be
the canonical one: export the production schema from the config router module
(e.g., the schema object used in config.ts) and import it into this test instead
of redefining commandAliasesSchema; update the test to import the exported
schema (the same named export used in the router/config code) so the test always
uses the authoritative schema and avoids drift.
packages/api/src/routers/__tests__/task.test.ts (2)

6-27: Schema definitions are duplicated from production code.

Same recommendation as other test files: consider exporting schemas from task.ts to avoid drift between tests and production validation.

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

In `@packages/api/src/routers/__tests__/task.test.ts` around lines 6 - 27, The
test file duplicates production zod schemas (createSchema, markDoneSchema,
removeSchema, activateSchema, removeByViewerSchema, editSchema,
moderateEditSchema); instead export the canonical schemas from the production
module (task.ts) and import them into this test to avoid drift—update task.ts to
export the existing zod schemas and replace the inline definitions in
packages/api/src/routers/__tests__/task.test.ts with imports of those exported
symbols so tests validate against the same schemas as runtime.

177-192: Priority logic tests provide minimal value.

Testing that 0 < 1 and 0 === 0 doesn't validate any production code behavior. If these constants exist in production code, consider importing and testing them directly. Otherwise, these assertions are tautological.

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

In `@packages/api/src/routers/__tests__/task.test.ts` around lines 177 - 192, The
tests in task.test.ts are tautological because they assert literal values
(BROADCASTER_PRIORITY and VIEWER_PRIORITY) rather than exercising production
behavior; replace these hard-coded assertions by importing the actual priority
constants from the production module (e.g., import BROADCASTER_PRIORITY,
VIEWER_PRIORITY from their source) and assert their values and ordering, or
remove the tests if those constants aren’t defined in prod and the behavior is
covered elsewhere; update the describe block to reference the imported symbols
(BROADCaster_PRIORITY, VIEWER_PRIORITY) and, if appropriate, add a test that
verifies the sorting behavior using the function or comparator that consumes
those priorities.
packages/db/src/index.ts (1)

6-7: Consider adding pool configuration for production.

The pool uses only the connection string with default settings. For production deployments, you may want to configure pool size, idle timeout, and connection timeout.

♻️ Example configuration
-const pool = new Pool({ connectionString: env.DATABASE_URL });
+const pool = new Pool({
+  connectionString: env.DATABASE_URL,
+  max: 10,
+  idleTimeoutMillis: 30000,
+  connectionTimeoutMillis: 2000,
+});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/db/src/index.ts` around lines 6 - 7, The current Pool is created
only with connectionString (Pool) which uses defaults; update Pool instantiation
to include production-tunable options (e.g., max pool size, idle timeout,
connection timeout) and ideally read values from environment variables (e.g.,
DB_MAX_POOL_SIZE, DB_IDLE_TIMEOUT_MS, DB_CONN_TIMEOUT_MS) so you can tune for
production, then pass that configured pool into drizzle(pool, { schema }) as
before; reference Pool, env.DATABASE_URL, drizzle, and schema when making the
change.
packages/api/src/routers/__tests__/overlay.test.ts (1)

30-56: Response shape tests provide minimal value.

These tests validate static objects created within the test itself rather than actual router responses. Lines 52-55 (const response = null; expect(response).toBeNull()) are tautological.

Consider either:

  1. Removing these tests entirely
  2. Converting them to integration tests that call the actual router procedures
  3. Defining response schemas and validating those
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routers/__tests__/overlay.test.ts` around lines 30 - 56, The
tests in the "overlay response shape" suite are asserting against locally
constructed objects (tautological), especially the null case in the "null
response for invalid token (documents behavior)" test; remove the tautological
assertions or replace them with real integration checks by invoking the overlay
router procedures (e.g., call the procedure used to fetch overlay data or the
token-validation helper) and assert the actual returned value or error;
alternatively, implement and use a shared response schema (e.g., a Zod schema)
and validate the router output against that schema in the test instead of
asserting on hand-made objects — update the tests named "timer state response
has expected keys when present", "task list response has expected keys when
present", and "null response for invalid token (documents behavior)"
accordingly.
packages/api/src/routers/__tests__/timer.test.ts (1)

4-17: Consider importing the schema from the production code to avoid drift.

The timerStatusEnum and transitionSchema are duplicated here. If the production schema in timer.ts changes, these tests could silently become out of sync. Importing and re-exporting the schema from the router module would ensure tests always validate against the actual input shape.

♻️ Suggested approach

Export the schema from the router and import it in tests:

// In timer.ts
export const timerStatusEnum = z.enum([...]);
export const transitionSchema = z.object({...});

// In timer.test.ts
import { timerStatusEnum, transitionSchema } from "../timer";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routers/__tests__/timer.test.ts` around lines 4 - 17, Remove
the duplicated zod definitions from the test and import the canonical schemas
from the production module: export timerStatusEnum and transitionSchema from the
timer router (ensure the timer module exports these symbols) and then update the
test to import timerStatusEnum and transitionSchema instead of redefining them
so tests always validate against the real production schema.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 00dcf447-6dff-47bb-9e4d-daf314d3f961

📥 Commits

Reviewing files that changed from the base of the PR and between d0a25c2 and 018af7f.

⛔ Files ignored due to path filters (2)
  • bun.lock is excluded by !**/*.lock
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (87)
  • .env.example
  • .github/FUNDING.yml
  • .github/workflows/ci.yml
  • .github/workflows/deploy-docs-to-pages.yml
  • .npmrc
  • CLAUDE.md
  • CONTRIBUTING.md
  • Dockerfile
  • README.md
  • apps/fumadocs/content/docs/chat-commands.mdx
  • apps/fumadocs/content/docs/deployment.mdx
  • apps/fumadocs/content/docs/troubleshooting.mdx
  • apps/fumadocs/src/app/global.css
  • apps/web/instrumentation.ts
  • apps/web/next.config.ts
  • apps/web/package.json
  • apps/web/src/app/(app)/api/bot/authorize/route.ts
  • apps/web/src/app/(app)/api/bot/callback/twitch/route.ts
  • apps/web/src/app/(app)/dashboard/bot/bot-settings-page.tsx
  • apps/web/src/app/(app)/dashboard/dashboard.tsx
  • apps/web/src/app/(app)/dashboard/styles/styles-page.tsx
  • apps/web/src/components/bot-settings/bot-messages-card.tsx
  • apps/web/src/components/bot-settings/command-alias-editor.tsx
  • apps/web/src/components/bot-settings/commands-reference.tsx
  • apps/web/src/components/bot-settings/message-editor.tsx
  • apps/web/src/components/bot-settings/variable-reference.tsx
  • apps/web/src/components/header.tsx
  • apps/web/src/components/mode-toggle.tsx
  • apps/web/src/components/task-list-display.tsx
  • apps/web/src/components/task-manager.tsx
  • apps/web/src/components/theme-center/color-input.tsx
  • apps/web/src/components/theme-center/font-select.tsx
  • apps/web/src/components/theme-center/style-preview-panel.tsx
  • apps/web/src/components/theme-center/task-style-editor.tsx
  • apps/web/src/components/theme-center/timer-style-editor.tsx
  • apps/web/src/components/timer-controls.tsx
  • apps/web/src/components/ui/button.tsx
  • apps/web/src/components/ui/checkbox.tsx
  • apps/web/src/components/ui/input.tsx
  • apps/web/src/components/ui/slider.tsx
  • apps/web/src/index.css
  • docker-entrypoint.sh
  • nixpacks.toml
  • package.json
  • packages/api/package.json
  • packages/api/src/bot/__tests__/commands.test.ts
  • packages/api/src/bot/commands.ts
  • packages/api/src/bot/index.ts
  • packages/api/src/context.ts
  • packages/api/src/events.ts
  • packages/api/src/routers/__tests__/config-aliases.test.ts
  • packages/api/src/routers/__tests__/overlay.test.ts
  • packages/api/src/routers/__tests__/task.test.ts
  • packages/api/src/routers/__tests__/timer.test.ts
  • packages/api/src/routers/__tests__/user.test.ts
  • packages/api/src/routers/bot.ts
  • packages/api/src/routers/config.ts
  • packages/api/src/routers/index.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/api/vitest.config.ts
  • packages/auth/package.json
  • packages/auth/src/__tests__/allowlist.test.ts
  • packages/auth/src/index.ts
  • packages/auth/vitest.config.ts
  • packages/db/drizzle.config.ts
  • packages/db/drizzle/0000_wooden_zarek.sql
  • packages/db/drizzle/meta/0000_snapshot.json
  • packages/db/drizzle/meta/_journal.json
  • packages/db/package.json
  • packages/db/prisma.config.ts
  • packages/db/prisma/migrations/0001_init/migration.sql
  • packages/db/prisma/migrations/0002_bot_settings_overhaul/migration.sql
  • packages/db/prisma/migrations/migration_lock.toml
  • packages/db/prisma/schema/app.prisma
  • packages/db/prisma/schema/auth.prisma
  • packages/db/prisma/schema/schema.prisma
  • packages/db/src/index.ts
  • packages/db/src/migrate.ts
  • packages/db/src/schema/app.ts
  • packages/db/src/schema/auth.ts
  • packages/db/src/schema/index.ts
  • packages/env/package.json
  • packages/env/src/server.ts
  • pnpm-workspace.yaml
💤 Files with no reviewable changes (12)
  • packages/db/prisma/schema/schema.prisma
  • packages/db/prisma/schema/app.prisma
  • .npmrc
  • apps/web/src/components/bot-settings/command-alias-editor.tsx
  • packages/db/prisma/migrations/0002_bot_settings_overhaul/migration.sql
  • apps/web/src/components/ui/checkbox.tsx
  • apps/web/src/components/bot-settings/variable-reference.tsx
  • packages/db/prisma/schema/auth.prisma
  • packages/db/prisma/migrations/migration_lock.toml
  • packages/db/prisma.config.ts
  • packages/db/prisma/migrations/0001_init/migration.sql
  • pnpm-workspace.yaml

Comment on lines 44 to +48
disconnectBot: protectedProcedure.mutation(async ({ ctx }) => {
await ctx.prisma.botAccount.deleteMany({
where: { userId: ctx.session.user.id },
// Stop the bot if running
if (botService.isRunning()) {
await botService.stop();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Bot stop may affect another user's running instance.

The botService.stop() is called unconditionally if any bot is running, before verifying the current user owns the running bot. This could stop another user's bot instance.

Consider checking if the running bot belongs to the disconnecting user before stopping.

🛡️ Suggested fix
   disconnectBot: protectedProcedure.mutation(async ({ ctx }) => {
-    // Stop the bot if running
-    if (botService.isRunning()) {
+    // Stop the bot only if it belongs to this user
+    if (botService.isRunning() && botService.getOwnerId?.() === ctx.session.user.id) {
       await botService.stop();
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routers/user.ts` around lines 44 - 48, The disconnectBot
mutation currently calls botService.stop() whenever any bot is running, which
may terminate another user's bot; change the logic to first verify that the
running bot belongs to the requesting user (use ctx.session.user.id or
ctx.userId) before stopping. Specifically, in disconnectBot
(protectedProcedure.mutation) replace the unconditional stop with a conditional:
if (botService.isRunning()) { const ownerId = await botService.getOwner?.() ||
botService.getRunningUserId?.(); if (ownerId === ctx.session.user.id) await
botService.stop(); } and if no owner API exists, add or use an existing method
on botService (e.g., getOwner/getRunningUserId/currentOwner) to determine
ownership; also handle missing session/user id gracefully. Ensure you reference
the functions botService.isRunning(), botService.stop(), and the mutation
disconnectBot when making the change.

nathanialhenniges and others added 4 commits March 18, 2026 14:41
# Conflicts:
#	.github/workflows/deploy-docs-to-pages.yml
#	apps/web/instrumentation.ts
#	apps/web/src/app/(app)/api/bot/callback/twitch/route.ts
#	apps/web/src/app/(app)/dashboard/bot/bot-settings-page.tsx
#	apps/web/src/components/bot-settings/commands-reference.tsx
#	apps/web/src/components/bot-settings/message-editor.tsx
#	packages/api/src/bot/index.ts
#	packages/db/drizzle.config.ts
#	packages/db/drizzle/meta/_journal.json
#	packages/db/src/migrate.ts
#	packages/db/src/schema/auth.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Critical: fix task order calculation bug (asc→desc) in bot commands,
security hardening (OAuth CSRF nonce, bot stop ownership check, Docker
non-root user), config mutation safety (ensureUserConfig before writes),
task promotion guard (only promote if completed task was active), timer
defaultCycles from config, OAuth scopes cleanup, and README pnpm→bun.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nathanialhenniges nathanialhenniges merged commit 9a01f4f into main Mar 26, 2026
3 of 4 checks passed
@nathanialhenniges nathanialhenniges deleted the dev branch March 26, 2026 15:59
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