fix: stop sub-goals being overridden, plus repo audit and Workers perf - #92
Merged
nathanialhenniges merged 11 commits intoJul 29, 2026
Merged
Conversation
Two independent data-exposure/data-loss bugs found while tracing the sub-goal override report. Hidden goals reached public chat. `goalsValue` and the `!wolfathon` "goal" segment indexed `data.goals[data.currentIndex]` on the RAW document. `currentIndex` counts hidden goals, and `stripNotes` — the only place that filters them — is never called on the bot path, so a goal marked hidden via the eye toggle was announced by name (and by target) the moment it became the next locked goal. Add `nextGoalIndex` / `nextVisibleGoal` to state.ts and route the bot through the latter. The pointer was previously reimplemented in four places; `recompute`, `stripNotes` and the goal editor now share it. Disconnecting Twitch destroyed the chat-bot account. `twitch.disconnect` rebuilt the row from a hand-written literal that omitted `doc.bot` in both branches, so the separately-connected bot's OAuth grant went with the broadcaster's — silently, recoverable only by re-authorizing from a second Twitch login. Extract `disconnectedDoc`, which allowlists what survives (a plain spread would keep the broadcaster's tokens instead), and switch the write from blind `writeTwitch` to `mutateTwitch` so the `getAppToken` + `deleteSubscriptions` network window can't clobber a concurrent `recentEventIds` append or bot token refresh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reported symptom: delete a channel-point reward on Twitch, create a new one, then hit Save on the Timer tab and the new reward's title and minutes revert to the old ones. Three faults compounded. `channelPoints` was in the Timer tab's draft contract even though it is server-owned — created and removed by dedicated procedures that write straight through to Twitch and D1. Worse, `ChannelRewards` patched the returned rules into the draft via `setDraft`, which never touches `savedRef`, so the tab went permanently dirty and `useDraft`'s dirty gate then refused every later re-seed. The doc comment claiming this was handled was wrong. `channelPoints` now leaves both the dirty diff and the Save payload; `timer.setConfig` merges instead of replacing wholesale, preserving stored rules for any payload that omits the key. A backup restore carries the key, so it still replaces them. Rule matching preferred the stored `rewardId` per-rule, so deleting a reward on Twitch and recreating it — same title, fresh id — matched nothing and silently added zero minutes. `findChannelPointRule` now tries the id first and falls back to a case-insensitive title, and `reconcileRewardId` re-points the stale id from inside the CAS apply that already runs, so the first redemption both adds time and repairs the rule at no extra read or write. Goals had the same shape: the Rewards tab always sent `currentSubs` from a page-load snapshot, rewinding every sub Twitch counted since. It now sends the field only when the operator actually moved it, and the server's existing `existing.currentSubs` fallback does the rest. Also fixes two `useDraft` bugs behind all of this: a revision arriving while dirty was marked seen without being applied, stranding the tab on a stale base with nothing left to trigger a re-seed; and there was no polling at all, so a draft could rot indefinitely with no signal. Adds `stale` + `saved` to the draft contract, a 15s operator poll, and an amber DirtyBar warning. New `channelPointsEnabled` toggle parks the integration without deleting the rewards from Twitch. Removal stays live while parked so rewards can't be stranded on the channel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A goal whose target the count had already passed but which wasn't unlocked yet was still a raise candidate — and because the raise runs a floor forward through the list, moving it pushed every goal after it too. So working on goal #5 could rewrite the targets of #2, #3 and #4. Add `freezeMetTargets` to the tracker document, on by default. A met-but- locked goal now holds the floor and returns untouched, which also stops the cascade originating from one. Genuinely out-of-order targets — below an already-unlocked goal's target — are still repairable with the existing opt-in "Raise past goals" button. Once every goal is unlocked the raise count is zero and the button disappears on its own. `recompute` rebuilds an explicit literal and runs on both sides of every `mutateState`, so the new key is named there (with its pre-flag default) or it would be erased on the next read or write. Called out in a comment and pinned by a test, because the next field to be added will hit the same trap. Also fixes the invisible write target in the editor: a locked row's own sub target used to disappear once that goal became "next", leaving the prominent banner stepper as the only way to edit it — so an operator aiming at one goal could be nudging another. Every locked row now shows its own target, and the banner edits the same number. Drops `currentIndex` from the `state.replace` input: `recompute` derives it from the unlocked flags unconditionally, so accepting it was misleading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Measured before: three OBS sources cost ~110k D1 round-trips a day, every overlay poll paid two sequential waves, and every single chat message cost a D1 read. Skip no-op writes. `mutateDoc` now compares the serialized apply result against the row it read and skips the UPDATE when they match — D1 counts a same-value UPDATE as a change, so previously every `!command` in chat rewrote the entire giveaway doc (up to 5000 entrants), every stream.online/offline rewrote the timer with auto-pause off, and every cooldown-blocked bot reply rewrote the bot doc. Content compare rather than object identity, because the read-boundary normalizers always hand back a fresh object. The contract this implies — a no-op apply returns the value it read — is documented at the call site. One wave per overlay poll. The `?t=` token check was awaited before the docs it gates; both now go out together and the gate still runs before anything is returned. Zero D1 reads for the chat firehose. The EventSub webhook read the twitch doc before the signature check, so every chat message (and every unsigned POST) cost a read. The webhook secret is now cached per isolate, with a re-read-once retry on verify failure so a rotation self-heals; the doc itself is loaded lazily and only for deliveries that survive the pre-filter. The webhook is also excluded from the request logger, which at raid volume was thousands of lines a minute for 204s. Parallelise independent writes: the timer and state mutations inside `applyTimerEventAndBumpSubs` hit different rows with no ordering between them, as do the two readTwitch/readTimer pairs and the four writes in `resetForNextSubathon`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rom prod `compatibilityDate` was unset on both Workers, so production inherited whatever the installed miniflare reported — meaning a routine `bun update` could silently change runtime semantics. Pinned to the value Alchemy resolves today, so this deploy is a no-op and future moves are deliberate. Smart Placement on the API Worker only. It is D1-bound, not client-bound: an overlay poll is one D1 wave and a gift delivery several, against tiny responses. Not enabled on the web Worker, which is interactive and Access-gated. `libsql` + `@libsql/client` were runtime dependencies of packages/db but only drizzle-kit uses them — nothing imports them, and the app is D1-only. Moved to devDependencies with the other tooling so native prebuilt binaries can't be resolved into a Worker bundle. `db:generate` verified. Also adds a tsconfig + check-types to packages/infra: the deploy program was the one TypeScript in the repo CI never typechecked, which is a poor place to have no safety net. Not done, and why: the audit flagged the four Google font families in the root layout as overlay bloat, but Poppins and Inter are already `preload: false` and all four are selectable from the overlay theme picker — the current setup is correct. Splitting the tRPC client module was also dropped: the operator client is a few hundred bytes of closure and `sonner` arrives via the shared queryClient either way, so the split would be churn for no measurable win. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The 50-id idempotency ring buffer lived inside the `twitch` JSON document, which caused three problems. Every actionable delivery wrote the single hottest row, so a sub train serialised through one CAS chain. The window was a hard 50 events, so a Twitch retry arriving after a burst could be reprocessed and double-count time. And that row also holds the OAuth tokens — so the event firehose raced the bot's token refresh, and losing that race discards an already-rotated refresh token and kills the bot until someone reconnects by hand. `eventsub_seen(message_id PRIMARY KEY, seen_at)` replaces it. The claim is `INSERT … ON CONFLICT DO NOTHING`: distinct ids never contend, the window is unbounded, and the credential row is out of the hot path entirely. Expired rows are swept from `waitUntil` on ~1% of deliveries — the table is correct whether or not that ever runs. `seenInLegacyRing` is a one-release shim so a retry spanning the deploy is still recognised from the old in-document ring. Drop it, and `TwitchDoc.recentEventIds`, one release after this ships. Exhausting the CAS retry loop now throws a typed CONFLICT with operator-facing copy instead of a bare Error naming an internal function. This is reachable during a big gift bomb, and a dropped write there means lost Wolfathon time. Deploy note: the migration must land before the code that reads the table. Alchemy applies migrationsDir on deploy, so a single `bun run deploy` does both in the right order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two browser tabs on the same panel could still clobber each other: both load, both edit, second Save wins and the first operator's work vanishes with no signal. P1 fixed the single-tab case by making server-owned fields server-owned; this covers the fields a human legitimately edits. `goalsRev` and `configRev` are bumped by `mutateState` / `mutateTimer` whenever their guarded region actually changes, so it is automatic for `state.import`, `resetForNextSubathon`, `createChannelReward` and anything added later. A save sends the revision it was built from and is rejected with CONFLICT on a mismatch. The check runs INSIDE the CAS apply — a throw escapes the retry loop uncaught, so a rejected save writes nothing at all. Scoped per region, not per document, and that distinction is the whole design: `mutateTimer` runs on every timer event and `mutateState` on every counted sub, so a whole-document revision would make both tabs conflict constantly during any live stream. Guarding only `goals` and `config` means a save is rejected when someone edited the same thing, and never because Twitch moved the clock. Comparison is by value, so rebuilding an identical array (which the panel does constantly) doesn't bump. No migration — the field lives in the JSON document, which is the house idiom. Resolution is two explicit choices in the DirtyBar, no merge UI: Load latest (take theirs) or Save mine anyway (re-issue against the freshly refetched revision). A save carrying no revision opts out entirely, so backup restores and scripts are unaffected. Also closes the last piece of the channel-point write-through: creating or removing a reward now seeds the tab's saved baseline via `onDocChanged`, so it adopts the `configRev` that write just bumped rather than conflicting on the operator's own action. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Wheel colour picker wrote on every pointer move. React maps `onChange` to the DOM `input` event, so one colour drag fired hundreds of `upsertSlot` mutations — hundreds of D1 writes, each of which also clears a pending spin, so picking a colour could kill an armed wheel mid-animation. Commits on blur now, matching the label and weight fields beside it. Overlay unlock celebration could stick on the OBS source. The 3.2s timeout was returned as the effect cleanup, but the effect re-runs on every `data` change — so a poll landing inside the window cleared it and then early-returned without scheduling a replacement, leaving "Unlocked: X" on stream until the next unlock. Held in a ref, cleared only on unmount. A bot token refresh losing a race falsely reported an expired token. Concurrent waitUntil sends share one snapshot and refresh with the same refresh token; Twitch rotates it on use, so all but one get a 4xx. Those are lost races, not revoked grants. The flag is now only raised when the STORED refresh token is still the one that failed. Clipboard copy had no catch. A denied permission or non-secure context was a silent no-op plus an unhandled rejection, with the operator left thinking they had copied the overlay URL. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The README and the public landing page both advertise the wheel "auto-spinning every N counted subs (default 10, configurable)" with a cadence control on the Wheel tab. No such feature exists anywhere in the codebase — an operator would hunt the panel for a setting that was never built. Removed from both. The rewards-overlay table was inverted in a way that could burn a surprise: it claimed future goals are "hidden entirely" when the "Coming up" row renders the next few upcoming reward names, and described a dimmed unlocked row that doesn't exist. Corrected, including that only the next goal's target is ever exposed and hidden goals never reach the browser. Poll intervals were documented as 2 seconds; they are 5s (timer), 10s (rewards) and 3s (wheel). And `start_url` opened `/`, which stopped being the panel when the landing page shipped — installing the PWA opened marketing instead of the dashboard. Accessibility: two `aria-live` regions wrapped one-second countdowns, so a screen reader re-announced the full remaining time every second on the Timer tab, and re-read the entire claim panel every second for the whole five-minute giveaway window. The live regions now wrap the status text only. The two `role="radiogroup"` controls had no roving tabIndex and no arrow-key handler — they announced as radiogroups but every option was a separate tab stop and the arrows did nothing. Three inputs (the JSON import textarea, the bot triggers field, the bot reply text) had no accessible name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lugins Removed, each verified to have zero callers: `writeState` / `writeTimer` / `writeGiveaway` / `writeWheel` / `writeBot` (every write path goes through the CAS helpers), the entire `goals` sub-router (`unlockNext` / `add` / `remove` / `reorder` — the editor does all four in the draft), `state.adjustSubs` and `state.setSubs`, `EXAMPLE_JSON` / `TIMER_EXAMPLE_JSON`, `OverlayId`, and the five unused sub-router type exports. Merged the two byte-identical blob-download helpers into one `downloadFile` in control/util.ts. `toPublicTimer` substituted the eight default emoji for an EMPTY list, so an operator who deliberately cleared every emoji got the full wolf set bursting back — contradicting the panel's own "None — overlay falls back to 🐺" copy and making the overlay's single-wolf fallback unreachable. A row that predates the field is already backfilled by `withTimerConfigDefaults`, so empty here can only mean deliberate. Added eslint-plugin-react-hooks and eslint-plugin-jsx-a11y, neither of which was running — in a codebase whose recent bugs were mostly hook-dependency and a11y bugs. Configured rather than blanket-disabled: this codebase labels controls by nesting them, and the nested control is usually one of our own primitives, so the rule is told about both; the shared <Label> passthrough has nothing to associate and is scoped out; and the three roving-tabindex containers are disabled individually with the reason, since focus belongs on their children by design. Lint is now error-free with the new rules on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
WalkthroughWolfathon updates backend concurrency and EventSub idempotency, adds frozen goal targets and save-conflict handling, expands timer channel-point controls, improves control-panel accessibility, adjusts overlay behavior, and refreshes documentation and deployment configuration. ChangesBackend reliability and persistence
Goal state and save conflicts
Timer configuration
Control-panel and overlay UX
Documentation and deployment
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Operator
participant RewardsTab
participant ProtectedRouter
participant TrackerStore
Operator->>RewardsTab: edit goals
RewardsTab->>ProtectedRouter: save with baseGoalsRev
ProtectedRouter->>TrackerStore: CAS update
TrackerStore-->>ProtectedRouter: saved document or CONFLICT
ProtectedRouter-->>RewardsTab: response
RewardsTab-->>Operator: saved state or conflict actions
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
`showNext` gated rendering only. The public payload still carried every upcoming reward name, so turning "Next rewards" off looked like it hid them and didn't — anyone holding the `?t=` URL, or anyone opening devtools on the OBS browser source, could read the whole ladder. The README promised these were "hidden entirely". `stripNotes` now slices to what the overlay can actually draw: everything up to and including the current goal, plus the "Coming up" window when that toggle is on. The kept prefix always contains every unlocked goal (they unlock top-to-bottom), so `currentIndex` still indexes correctly and the overlay's unlock-celebration tracker still sees each goal the moment it flips — no client change needed. `hidden` keeps its stronger meaning: a hidden goal never reaches the browser even when it is the very next reward. `showNext` is now a privacy toggle too, and the Customizer hint and the type doc say so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/api/src/routers/twitch.ts (1)
94-114: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
unsubscribeddefaults totrueeven when the delete was never attempted (missing credentials).
disconnectnever callsrequireCreds, sohasCredscan befalsewhiledoc.subscriptionIdsis non-empty. In that case thetry/catchblock is skipped entirely, leavingunsubscribedat its initialtrue— indistinguishable from a real, successfuldeleteSubscriptions()call. That value now feeds directly into the CAS write viadisconnectedDoc(cur, unsubscribed)(Line 112), which per its own docstring should treat "delete not attempted" the same as "delete failed" (unsubscribed: false) so the webhook secret and subscription ids survive for later reconciliation. As written, missing credentials silently discards that state, leaving any still-live Twitch subscriptions un-reconcilable and unverifiable by the webhook going forward.🛡️ Proposed fix: treat "no credentials" like a failed delete
- let unsubscribed = true; - if (hasCreds && doc.subscriptionIds?.length) { - try { - const appToken = await getAppToken(clientId!, clientSecret!); - await deleteSubscriptions(clientId!, appToken, doc.subscriptionIds); - } catch { - unsubscribed = false; - } - } + // Nothing to unsubscribe from is trivially "clean". Otherwise, only a + // successful `deleteSubscriptions` call counts as unsubscribed — missing + // credentials mean we never attempted the delete, so treat it like a + // failed delete (see `disconnectedDoc`) rather than discarding state + // we'd need to reconcile once credentials are restored. + let unsubscribed = !doc.subscriptionIds?.length; + if (doc.subscriptionIds?.length) { + if (hasCreds) { + try { + const appToken = await getAppToken(clientId!, clientSecret!); + await deleteSubscriptions(clientId!, appToken, doc.subscriptionIds); + unsubscribed = true; + } catch { + unsubscribed = false; + } + } else { + unsubscribed = false; + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/routers/twitch.ts` around lines 94 - 114, Initialize unsubscribed as false when credentials are unavailable, while preserving true only for an actually attempted and successful deleteSubscriptions call. Update the disconnect mutation’s hasCreds/subscriptionIds flow so skipped deletion and caught deletion failures both pass false to disconnectedDoc, while the existing successful deletion behavior remains unchanged.
🧹 Nitpick comments (2)
apps/web/src/components/control/bot-panel.tsx (1)
306-315: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared
Labelprimitive for both fields.The associations are correct, but these new native
<label>elements bypass the existingpackages/uiprimitive. Replace both withLabelwhile preservinghtmlFor/id.As per coding guidelines,
apps/web/src/components/**/*.{ts,tsx}should reusepackages/uiprimitives;giveaway-tab.tsxalready demonstrates the sharedLabelusage.Also applies to: 370-383
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/control/bot-panel.tsx` around lines 306 - 315, Replace both native label elements in the trigger and association field sections with the shared Label primitive, preserving each existing htmlFor value and matching Input id. Ensure Label is imported from the established packages/ui primitives used by bot-panel.tsx.Source: Coding guidelines
apps/web/src/components/control/timer-config-panel.tsx (1)
551-566: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNew toggle uses the shared
Checkbox, but two sibling toggles in the same file still hand-roll<input type="checkbox">.The new "Channel-point rewards add time" toggle correctly uses
@wolfathon/ui'sCheckbox, but "Name who added the time" (~406-411) and "Auto-pause when the stream goes offline" (~423-428) still use a raw<input type="checkbox">. Consider migrating those toCheckboxtoo for visual/behavioral consistency in the same panel.As per coding guidelines, "Use
packages/uiprimitives andcninstead of duplicating UI controls or constructing class names with ternary template literals."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/control/timer-config-panel.tsx` around lines 551 - 566, In the timer configuration panel, migrate the “Name who added the time” and “Auto-pause when the stream goes offline” toggles from raw input elements to the shared Checkbox component, matching the existing channelPointsEnabled toggle’s checked and onCheckedChange behavior while preserving their current state updates and labels.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/page.tsx`:
- Line 49: Update the descriptive body text in the page metadata to clarify that
the wheel is hidden until it lands by default, while acknowledging the existing
“Keep wheel on screen” option that can keep it visible.
In `@apps/web/src/components/control/giveaway-tab.tsx`:
- Around line 758-766: Update the live-region content in the claim status block
around the status div to include the pending winner’s name for the active drawn
state, so initial draws and redraws are announced. Keep countdown values outside
the live region, and preserve the existing claimLapsed announcement behavior.
In `@apps/web/src/components/control/timer-tab.tsx`:
- Around line 106-134: Update the TimerConfigPanel onDocChanged handler to patch
only the server-owned channelPoints and configRev fields from selectConfig(doc)
into the existing draft, instead of calling seed with the full returned config.
Preserve all unrelated unsaved draft edits while updating the saved baseline and
revision through the existing draft state flow.
In `@packages/api/src/state.test.ts`:
- Around line 130-156: Update the Data fixture literals in
packages/api/src/state.test.ts (lines 130-156) and both targeted fixtures in
packages/api/src/bot.test.ts (lines 93-108 and 110-124) to include goalsRev: 0
and provide properly typed freezeMetTargets values instead of relying on as
Data; preserve the existing optional-default handling in the affected functions.
---
Outside diff comments:
In `@packages/api/src/routers/twitch.ts`:
- Around line 94-114: Initialize unsubscribed as false when credentials are
unavailable, while preserving true only for an actually attempted and successful
deleteSubscriptions call. Update the disconnect mutation’s
hasCreds/subscriptionIds flow so skipped deletion and caught deletion failures
both pass false to disconnectedDoc, while the existing successful deletion
behavior remains unchanged.
---
Nitpick comments:
In `@apps/web/src/components/control/bot-panel.tsx`:
- Around line 306-315: Replace both native label elements in the trigger and
association field sections with the shared Label primitive, preserving each
existing htmlFor value and matching Input id. Ensure Label is imported from the
established packages/ui primitives used by bot-panel.tsx.
In `@apps/web/src/components/control/timer-config-panel.tsx`:
- Around line 551-566: In the timer configuration panel, migrate the “Name who
added the time” and “Auto-pause when the stream goes offline” toggles from raw
input elements to the shared Checkbox component, matching the existing
channelPointsEnabled toggle’s checked and onCheckedChange behavior while
preserving their current state updates and labels.
🪄 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 Plus
Run ID: ff236e94-2cc8-484d-b582-9dc943e3c161
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (54)
README.mdapps/server/src/index.tsapps/web/src/app/dashboard/page.tsxapps/web/src/app/manifest.tsapps/web/src/app/page.tsxapps/web/src/components/control/backup-tab.tsxapps/web/src/components/control/bot-panel.tsxapps/web/src/components/control/dirty-bar.tsxapps/web/src/components/control/example.tsapps/web/src/components/control/giveaway-tab.tsxapps/web/src/components/control/goal-editor.tsxapps/web/src/components/control/import-export-panel.tsxapps/web/src/components/control/rewards-tab.tsxapps/web/src/components/control/theme-tab.tsxapps/web/src/components/control/timer-config-panel.tsxapps/web/src/components/control/timer-example.tsapps/web/src/components/control/timer-panel.tsxapps/web/src/components/control/timer-tab.tsxapps/web/src/components/control/twitch-panel.tsxapps/web/src/components/control/use-control-doc.tsapps/web/src/components/control/use-draft.tsapps/web/src/components/control/use-save-conflict.tsapps/web/src/components/control/util.tsapps/web/src/components/control/wheel-tab.tsxapps/web/src/components/overlay/overlay-view.tsxapps/web/src/utils/constants.tseslint.config.mjspackage.jsonpackages/api/src/bot.test.tspackages/api/src/bot.tspackages/api/src/routers/bot.tspackages/api/src/routers/giveaway.tspackages/api/src/routers/protected.tspackages/api/src/routers/public.tspackages/api/src/routers/timer.tspackages/api/src/routers/twitch.tspackages/api/src/routers/wheel.tspackages/api/src/state.test.tspackages/api/src/state.tspackages/api/src/store.test.tspackages/api/src/store.tspackages/api/src/timer.test.tspackages/api/src/timer.tspackages/api/src/twitch.test.tspackages/api/src/twitch.tspackages/db/package.jsonpackages/db/src/migrations/0001_flashy_red_ghost.sqlpackages/db/src/migrations/meta/0001_snapshot.jsonpackages/db/src/migrations/meta/_journal.jsonpackages/db/src/schema/index.tspackages/infra/alchemy.run.tspackages/infra/package.jsonpackages/infra/tsconfig.jsonpackages/ui/src/hooks/use-copy-to-clipboard.ts
💤 Files with no reviewable changes (3)
- packages/api/src/routers/bot.ts
- packages/api/src/routers/wheel.ts
- packages/api/src/routers/giveaway.ts
| icon: Disc3, | ||
| title: "Wheel of dares", | ||
| body: "Spin Howlwheel on stream for a random dare — on demand, or auto-spun every few subs. Hidden until it lands so nobody sees it coming.", | ||
| body: "Spin Howlwheel on stream for a random dare. Hidden until it lands so nobody sees it coming.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify the wheel visibility claim.
Line [49] says the wheel is hidden until it lands, but README.md documents a Keep wheel on screen option that keeps it visible. Make the default behavior explicit.
Suggested wording
- body: "Spin Howlwheel on stream for a random dare. Hidden until it lands so nobody sees it coming.",
+ body: "Spin Howlwheel on stream for a random dare. Hidden by default until it lands so nobody sees it coming.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| body: "Spin Howlwheel on stream for a random dare. Hidden until it lands so nobody sees it coming.", | |
| body: "Spin Howlwheel on stream for a random dare. Hidden by default until it lands so nobody sees it coming.", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/page.tsx` at line 49, Update the descriptive body text in
the page metadata to clarify that the wheel is hidden until it lands by default,
while acknowledging the existing “Keep wheel on screen” option that can keep it
visible.
Comment on lines
+758
to
766
| {/* The live region is the STATE line only. It used to wrap the whole | ||
| block, which contains a countdown re-rendering every second — so a | ||
| screen reader re-read the entire claim panel once a second for the | ||
| full five-minute window. */} | ||
| <div role="status" aria-live="polite" className="text-xs text-muted-foreground"> | ||
| {claimLapsed | ||
| ? "Didn’t claim in time — redraw a new winner." | ||
| : "Drawn — waiting for them to type !claim in chat."} | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include the pending winner in the live announcement.
The live region only contains invariant state text. On an initial draw or redraw, pending.name changes while that text stays the same, so screen-reader users may not hear which winner must claim; the countdown reset is also silenced. Include the winner’s name in the live-region text while keeping countdown updates disabled.
Also applies to: 768-775
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/control/giveaway-tab.tsx` around lines 758 - 766,
Update the live-region content in the claim status block around the status div
to include the pending winner’s name for the active drawn state, so initial
draws and redraws are announced. Keep countdown values outside the live region,
and preserve the existing claimLapsed announcement behavior.
Comment on lines
+106
to
+134
| {draft && ( | ||
| <TimerConfigPanel | ||
| config={draft.config} | ||
| onChange={(config) => setDraft((d) => d && { ...d, config })} | ||
| // A reward create/remove already wrote through to Twitch and D1, so | ||
| // adopt the returned doc as the new SAVED baseline rather than an | ||
| // edit — otherwise the tab reads as dirty and its `configRev` (which | ||
| // the write just bumped) would conflict on the operator's next save. | ||
| onDocChanged={(doc) => seed(selectConfig(doc))} | ||
| /> | ||
| )} | ||
| </> | ||
| )} | ||
| <DirtyBar | ||
| dirty={dirty} | ||
| saving={setConfig.isPending} | ||
| onSave={save} | ||
| onDiscard={discard} | ||
| onSave={() => save()} | ||
| onDiscard={() => { | ||
| clear(); | ||
| discard(); | ||
| }} | ||
| summary="timer settings" | ||
| stale={stale} | ||
| conflict={conflict} | ||
| onLoadLatest={() => { | ||
| clear(); | ||
| discard(); | ||
| }} | ||
| onForceSave={() => save(true)} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
onDocChanged clobbers unrelated unsaved edits on every reward create/remove.
seed(selectConfig(doc)) fully replaces the draft with the server's returned config. If the operator has unsaved edits to any other field (e.g. startMinutes, the channelPointsEnabled toggle, sub tiers) and then creates/removes a channel-point reward (triggered from ChannelRewards in timer-config-panel.tsx), those edits are silently discarded — the draft resets to the server's last-saved values for everything, not just channelPoints. This contradicts the adjacent comment's intent of adopting only the server-owned bits as the new baseline.
Since configKey already excludes channelPoints/configRev from the dirty diff, patch just those two fields into the existing draft instead of a full reseed.
🐛 Proposed fix: patch instead of full reseed
{draft && (
<TimerConfigPanel
config={draft.config}
onChange={(config) => setDraft((d) => d && { ...d, config })}
- // A reward create/remove already wrote through to Twitch and D1, so
- // adopt the returned doc as the new SAVED baseline rather than an
- // edit — otherwise the tab reads as dirty and its `configRev` (which
- // the write just bumped) would conflict on the operator's next save.
- onDocChanged={(doc) => seed(selectConfig(doc))}
+ // A reward create/remove already wrote through to Twitch and D1.
+ // Patch just the server-owned `channelPoints` + the bumped
+ // `configRev` into the current draft (not a full reseed), so any
+ // other in-progress, unsaved edits survive.
+ onDocChanged={(doc) =>
+ setDraft(
+ (d) =>
+ d && {
+ ...d,
+ configRev: doc.configRev ?? 0,
+ config: { ...d.config, channelPoints: doc.config.channelPoints },
+ },
+ )
+ }
/>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {draft && ( | |
| <TimerConfigPanel | |
| config={draft.config} | |
| onChange={(config) => setDraft((d) => d && { ...d, config })} | |
| // A reward create/remove already wrote through to Twitch and D1, so | |
| // adopt the returned doc as the new SAVED baseline rather than an | |
| // edit — otherwise the tab reads as dirty and its `configRev` (which | |
| // the write just bumped) would conflict on the operator's next save. | |
| onDocChanged={(doc) => seed(selectConfig(doc))} | |
| /> | |
| )} | |
| </> | |
| )} | |
| <DirtyBar | |
| dirty={dirty} | |
| saving={setConfig.isPending} | |
| onSave={save} | |
| onDiscard={discard} | |
| onSave={() => save()} | |
| onDiscard={() => { | |
| clear(); | |
| discard(); | |
| }} | |
| summary="timer settings" | |
| stale={stale} | |
| conflict={conflict} | |
| onLoadLatest={() => { | |
| clear(); | |
| discard(); | |
| }} | |
| onForceSave={() => save(true)} | |
| {draft && ( | |
| <TimerConfigPanel | |
| config={draft.config} | |
| onChange={(config) => setDraft((d) => d && { ...d, config })} | |
| // A reward create/remove already wrote through to Twitch and D1. | |
| // Patch just the server-owned `channelPoints` + the bumped | |
| // `configRev` into the current draft (not a full reseed), so any | |
| // other in-progress, unsaved edits survive. | |
| onDocChanged={(doc) => | |
| setDraft( | |
| (d) => | |
| d && { | |
| ...d, | |
| configRev: doc.configRev ?? 0, | |
| config: { ...d.config, channelPoints: doc.config.channelPoints }, | |
| }, | |
| ) | |
| } | |
| /> | |
| )} | |
| </> | |
| )} | |
| <DirtyBar | |
| dirty={dirty} | |
| saving={setConfig.isPending} | |
| onSave={() => save()} | |
| onDiscard={() => { | |
| clear(); | |
| discard(); | |
| }} | |
| summary="timer settings" | |
| stale={stale} | |
| conflict={conflict} | |
| onLoadLatest={() => { | |
| clear(); | |
| discard(); | |
| }} | |
| onForceSave={() => save(true)} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/control/timer-tab.tsx` around lines 106 - 134, Update
the TimerConfigPanel onDocChanged handler to patch only the server-owned
channelPoints and configRev fields from selectConfig(doc) into the existing
draft, instead of calling seed with the full returned config. Preserve all
unrelated unsaved draft edits while updating the saved baseline and revision
through the existing draft state flow.
Comment on lines
+130
to
+156
| test("nextVisibleGoal skips a hidden next goal (chat must never name a secret reward)", () => { | ||
| const data: Data = { | ||
| goals: [ | ||
| { id: "a", reward: "Q&A", unlocked: true, target: 5 }, | ||
| { id: "b", reward: "Secret", unlocked: false, target: 8, hidden: true }, | ||
| { id: "c", reward: "Onesie", unlocked: false, target: 10 }, | ||
| ], | ||
| currentIndex: 1, // raw pointer lands ON the hidden goal — that's the trap | ||
| currentSubs: 7, | ||
| theme: defaultOverlayTheme(), | ||
| }; | ||
| expect(data.goals[data.currentIndex]?.reward).toBe("Secret"); | ||
| expect(nextVisibleGoal(data)?.reward).toBe("Onesie"); | ||
| }); | ||
|
|
||
| test("nextVisibleGoal is undefined when every visible goal is unlocked", () => { | ||
| const data: Data = { | ||
| goals: [ | ||
| { id: "a", reward: "Q&A", unlocked: true }, | ||
| { id: "b", reward: "Secret", unlocked: false, hidden: true }, | ||
| ], | ||
| currentIndex: 1, | ||
| currentSubs: 0, | ||
| theme: defaultOverlayTheme(), | ||
| }; | ||
| expect(nextVisibleGoal(data)).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching state/bot test and source:"
git ls-files | rg '(^|/)(state\.(ts|test\.ts)|bot\.(ts|test\.ts))$|packages/api/src/(state|bot)(\..*)?\.ts$' || true
echo
echo "state.ts outline and Data type:"
ast-grep outline packages/api/src/state.ts --view expanded || true
rg -n "interface Data|type Data|freezeMetTargets|goalsRev" packages/api/src/state.ts packages/api/src/state.test.ts packages/api/src/bot.test.ts
echo
echo "Relevant state.test.ts sections:"
sed -n '30,70p;115,165p;235,255p' packages/api/src/state.test.ts
echo
echo "Relevant bot.test.ts sections:"
sed -n '1,25p;75,130p' packages/api/src/bot.test.ts
echo
echo "Relevant bot.ts imports/functions:"
rg -n "function goalsValue|const goalsValue|function wolfathonValue|const wolfathonValue|nextVisibleGoal|goalsRev|freezeMetTargets" packages/api/src/bot.ts packages/api/src/*.tsRepository: MrDemonWolf/wolfathon
Length of output: 15226
Make these test fixtures satisfy Data’s required fields. Data.goalsRev is required, and the targeted Data literals pass freezeMetTargets/goalsRev as unknown even though those are still handled as optional defaults in the functions. Add goalsRev: 0 to the fixture literals in packages/api/src/state.test.ts and packages/api/src/bot.test.ts instead of relying on as Data.
📍 Affects 2 files
packages/api/src/state.test.ts#L130-L156(this comment)packages/api/src/bot.test.ts#L93-L108packages/api/src/bot.test.ts#L110-L124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/state.test.ts` around lines 130 - 156, Update the Data
fixture literals in packages/api/src/state.test.ts (lines 130-156) and both
targeted fixtures in packages/api/src/bot.test.ts (lines 93-108 and 110-124) to
include goalsRev: 0 and provide properly typed freezeMetTargets values instead
of relying on as Data; preserve the existing optional-default handling in the
affected functions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The reported bug
One bug shape with three faces. Every operator tab held a whole-document draft snapshotted at page load and PUT it back with no base-version check. Server-side CAS only protects server-vs-server writers; on retry it re-applied the same stale client array.
For channel-point rewards specifically:
channelPointswas in the Timer tab's draft even though the server owns it.setDraft, which never updates the saved baseline — so the tab went permanently dirty, and the dirty gate then refused every later re-seed. The doc comment claiming this was handled was false.setConfigreplaced the whole config, never readingprev.config.So Save wrote a page-load snapshot over the reward you had just recreated.
Compounding it, rule matching preferred the stored
rewardId. Delete on Twitch + recreate keeps the title but mints a new id, so a redemption matched nothing and silently added zero minutes.Verified end to end against the real code path: recreate on Twitch → Save an unrelated field → title and minutes survive; redeem → time is added and the stale id self-heals; backup restore still replaces rules.
Goals stay put
A goal whose target the count had passed but which wasn't unlocked yet was still a raise candidate — and the raise cascades, so working on goal #5 rewrote #2, #3 and #4.
freezeMetTargets(on by default) pins met goals and kills the cascade at its source.Also fixed the invisible write target: a locked row's target field vanished once it became "next", leaving the banner stepper editing a row the operator wasn't looking at.
Two new operator toggles, both requested: Keep met targets and Channel-point rewards add time.
Two security / data-loss bugs found in passing
!goalsand!wolfathonleaked hidden goals to public chat. They indexeddata.goals[data.currentIndex]on the raw document;stripNotesis the only hidden-goal filter and never ran on that path. The secret reward name and its target were posted.disconnectrebuilt the row from a literal that omitteddoc.bot, silently taking a separate account's OAuth grant with it.Cloudflare Workers
Measured before: ~110k D1 round-trips/day, two sequential waves per overlay poll, one D1 read per chat message.
!commandrewrote the whole giveaway doc (up to 5000 entrants), every stream state change rewrote the timer, every cooldown-blocked reply rewrote the bot doc.?t=check no longer blocks the docs it gates.compatibilityDatepinned (was unset, sobun updatecould change prod runtime semantics), Smart Placement on the API Worker,libsqlout of the production dependency graph, andpackages/infratypechecked in CI for the first time.Two-tab protection
goalsRev/configRevscoped per region, not per document —mutateTimerfires on every timer event andmutateStateon every sub, so a doc-level revision would conflict constantly mid-stream. A stale save is rejected with Load latest / Save mine anyway instead of silently winning. Saves carrying no revision (restores, scripts) opt out.Audit cleanup
Four more latent bugs: the wheel colour picker wrote per pointer-move (hundreds of D1 writes per drag, each clearing an armed spin); the overlay unlock celebration could stick on the OBS source; a bot token refresh losing a race falsely reported an expired token; clipboard copy had no catch.
Docs that were actively wrong: the README and landing page advertised wheel auto-spin every N subs — that feature does not exist. The rewards-overlay table claimed future goals are "hidden entirely" when they render in "Coming up". Poll intervals were documented as 2s (really 5/10/3). The PWA opened the marketing page instead of the dashboard.
Plus dead-code removal (all verified zero-caller), two shared helpers, and eslint-plugin-react-hooks + jsx-a11y — neither was running, in a codebase whose bugs were mostly hook-dependency and a11y bugs. Configured rather than blanket-disabled.
Overlay payload no longer leaks the reward ladder
showNextgated rendering only — the payload still carried every upcomingreward name, so turning "Next rewards" off looked like it hid them and didn't.
Anyone with the
?t=URL, or anyone opening devtools on the OBS browser source,could read the whole ladder.
stripNotesnow slices to what the overlay canactually draw.
hiddenkeeps its stronger meaning: never sent, even when it'sthe very next reward.
Deliberately not done
mutateDocis the seam.versioncolumn for CAS — its value is mostly as an enabler for a zero-staleness overlay read cache, so it belongs with that work. Note its CAS token must carry both version and data, because of the no-op write-skip added here.preload: falseand all four families are overlay-selectable;sonnerarrives via the shared queryClient regardless.Verification
202 tests pass (was 167), 0 type errors, 0 lint errors, format clean.
Deploy note: one new migration (
eventsub_seen). Alchemy appliesmigrationsDiron deploy, so a singlebun run deployorders it correctly. KeepseenInLegacyRingfor exactly one release so a retry spanning the deploy is still recognised, then drop it andTwitchDoc.recentEventIds.🤖 Generated with Claude Code
Summary by CodeRabbit