fix(onboarding): /setup/tasks must not re-create an existing schedule (chat#1889 row 12)#1898
fix(onboarding): /setup/tasks must not re-create an existing schedule (chat#1889 row 12)#1898sweetmantech wants to merge 1 commit into
Conversation
chat#1889 matrix row 12. FirstTaskStep pre-ran a fresh weekly report and scheduled it on every visit. A return visit, or a re-click of the welcome email's step 4 link, therefore billed a duplicate LLM run and left the account with a second schedule. - New findExistingWeeklyReportTask picks the account's enabled task; a paused schedule does not count, so the step still offers to set one up. - New ExistingWeeklyReportPanel surfaces the existing schedule with its next and most recent run instead. - FirstTaskStep waits for the tasks read before deciding, so the pre-run can't fire while tasks are still loading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✨ Finishing Touches🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f0a282062
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // run and left a second schedule behind. Wait for the read before deciding, so | ||
| // the pre-run isn't kicked off while tasks are still loading. | ||
| const existingTask = findExistingWeeklyReportTask(tasksQuery.data); | ||
| if (tasksQuery.isLoading) { |
There was a problem hiding this comment.
isLoading only covers the initial fetch: it is false while React Query background-refetches cached data and after a request ultimately errors. If the cache contains an old empty list, or /api/tasks fails while other endpoints still work, existingTask remains null and FirstTaskReportRun immediately sends the billed prompt before the current schedule state is known; a returning user can therefore still incur the duplicate run and, after confirming, create another schedule. Proceed only after a successful, settled task read and show an error/retry state otherwise.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 4 files
Confidence score: 2/5
- In
components/Onboarding/FirstTaskStep.tsx, the pre-run can startFirstTaskReportRunwhile task data is still stale/empty or after a failed read, so users may trigger onboarding report creation before the no-schedule guard is actually validated; gate this path on a completed successful tasks fetch before launching the run. - In
components/Onboarding/ExistingWeeklyReportPanel.tsx, the existing-task check appears too broad, so any unrelated enabled scheduled task can be misclassified as the onboarding weekly report and block step completion; scope the lookup to the onboarding weekly-report task type (and relevant metadata) to avoid false positives. components/Onboarding/FirstTaskStep.tsxhas grown past the readability/single-responsibility size guideline, which raises regression risk around guard ordering and async edge cases in this flow; split the new early-return/guard logic into focused helpers or subcomponents to make behavior easier to verify and maintain.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="components/Onboarding/ExistingWeeklyReportPanel.tsx">
<violation number="1" location="components/Onboarding/ExistingWeeklyReportPanel.tsx:24">
P1: Accounts with an unrelated enabled scheduled task are told their weekly report already exists and cannot complete this onboarding step. Filter the existing-task lookup to the onboarding weekly-report task (and ideally the selected artist) before rendering this panel.</violation>
</file>
<file name="components/Onboarding/FirstTaskStep.tsx">
<violation number="1" location="components/Onboarding/FirstTaskStep.tsx:43">
P1: Returning with a cached empty task list while its refetch is in flight, or after the tasks read fails, still starts `FirstTaskReportRun` before this guard verifies no schedule exists. Gate the pre-run on a completed successful read (and active fetches) so these paths cannot bill and schedule a duplicate.</violation>
<violation number="2" location="components/Onboarding/FirstTaskStep.tsx:43">
P2: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
This file now totals 117 lines, exceeding the 100-line limit for readability and single-responsibility. The newly added early-return blocks for `tasksQuery.isLoading` and `existingTask` pushed the component over the threshold. Consider extracting these into smaller sub-components (e.g., `FirstTaskLoadingState` and `ExistingWeeklyReportPanel` already exists) or moving the conditional orchestration into a wrapper hook to keep this file under 100 lines.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| <CalendarClock className="size-10 text-muted-foreground" /> | ||
| <div> | ||
| <h1 className="text-2xl font-semibold text-foreground"> | ||
| Your weekly report is already set up |
There was a problem hiding this comment.
P1: Accounts with an unrelated enabled scheduled task are told their weekly report already exists and cannot complete this onboarding step. Filter the existing-task lookup to the onboarding weekly-report task (and ideally the selected artist) before rendering this panel.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/ExistingWeeklyReportPanel.tsx, line 24:
<comment>Accounts with an unrelated enabled scheduled task are told their weekly report already exists and cannot complete this onboarding step. Filter the existing-task lookup to the onboarding weekly-report task (and ideally the selected artist) before rendering this panel.</comment>
<file context>
@@ -0,0 +1,45 @@
+ <CalendarClock className="size-10 text-muted-foreground" />
+ <div>
+ <h1 className="text-2xl font-semibold text-foreground">
+ Your weekly report is already set up
+ </h1>
+ <p className="mt-1 text-sm text-muted-foreground">
</file context>
| // run and left a second schedule behind. Wait for the read before deciding, so | ||
| // the pre-run isn't kicked off while tasks are still loading. | ||
| const existingTask = findExistingWeeklyReportTask(tasksQuery.data); | ||
| if (tasksQuery.isLoading) { |
There was a problem hiding this comment.
P1: Returning with a cached empty task list while its refetch is in flight, or after the tasks read fails, still starts FirstTaskReportRun before this guard verifies no schedule exists. Gate the pre-run on a completed successful read (and active fetches) so these paths cannot bill and schedule a duplicate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/FirstTaskStep.tsx, line 43:
<comment>Returning with a cached empty task list while its refetch is in flight, or after the tasks read fails, still starts `FirstTaskReportRun` before this guard verifies no schedule exists. Gate the pre-run on a completed successful read (and active fetches) so these paths cannot bill and schedule a duplicate.</comment>
<file context>
@@ -31,6 +35,28 @@ const FirstTaskStep = () => {
+ // run and left a second schedule behind. Wait for the read before deciding, so
+ // the pre-run isn't kicked off while tasks are still loading.
+ const existingTask = findExistingWeeklyReportTask(tasksQuery.data);
+ if (tasksQuery.isLoading) {
+ return (
+ <div className="mx-auto flex w-full max-w-3xl flex-col gap-4 px-4 py-8">
</file context>
| if (tasksQuery.isLoading) { | |
| if (tasksQuery.isFetching || !tasksQuery.isSuccess) { |
| // run and left a second schedule behind. Wait for the read before deciding, so | ||
| // the pre-run isn't kicked off while tasks are still loading. | ||
| const existingTask = findExistingWeeklyReportTask(tasksQuery.data); | ||
| if (tasksQuery.isLoading) { |
There was a problem hiding this comment.
P2: Custom agent: Code Structure and Size Limits for Readability and Single Responsibility
This file now totals 117 lines, exceeding the 100-line limit for readability and single-responsibility. The newly added early-return blocks for tasksQuery.isLoading and existingTask pushed the component over the threshold. Consider extracting these into smaller sub-components (e.g., FirstTaskLoadingState and ExistingWeeklyReportPanel already exists) or moving the conditional orchestration into a wrapper hook to keep this file under 100 lines.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/FirstTaskStep.tsx, line 43:
<comment>This file now totals 117 lines, exceeding the 100-line limit for readability and single-responsibility. The newly added early-return blocks for `tasksQuery.isLoading` and `existingTask` pushed the component over the threshold. Consider extracting these into smaller sub-components (e.g., `FirstTaskLoadingState` and `ExistingWeeklyReportPanel` already exists) or moving the conditional orchestration into a wrapper hook to keep this file under 100 lines.</comment>
<file context>
@@ -31,6 +35,28 @@ const FirstTaskStep = () => {
+ // run and left a second schedule behind. Wait for the read before deciding, so
+ // the pre-run isn't kicked off while tasks are still loading.
+ const existingTask = findExistingWeeklyReportTask(tasksQuery.data);
+ if (tasksQuery.isLoading) {
+ return (
+ <div className="mx-auto flex w-full max-w-3xl flex-col gap-4 px-4 py-8">
</file context>
1 participant
Matrix row 12 in chat#1889. Independent of the other row-12 PRs (#1896, #1897).
Why
FirstTaskSteppre-ran a fresh weekly report and then scheduled it — on every visit, unconditionally. So a user who returned to/setup/tasks, or simply re-clicked the welcome email's step 4 link, got:This is the same duplicate-schedule class of bug the task-email audit has hit before, reached through onboarding this time.
What changed
findExistingWeeklyReportTask— returns the account'senabledtask. A paused schedule deliberately does not count, so a user who disabled theirs can still set one up.ExistingWeeklyReportPanel— surfaces the existing schedule with its next run and most recent run, linking to/tasks, instead of running anything.FirstTaskStepwaits for the tasks read before deciding, so the pre-run cannot fire whileuseScheduledActionsis still loading — otherwise the race would re-introduce the duplicate on a slow read.Verification
TDD, red → green:
```
RED — module not found
Test Files 1 failed (1)
Tests no tests
GREEN
pnpm exec vitest run lib/onboarding components/Onboarding
Test Files 18 passed (18)
Tests 72 passed (72)
```
Unit tests cover: enabled task returned, disabled ignored, empty/undefined → null (first-run still pre-runs), and first-enabled-wins with a mixed list.
pnpm exec tsc --noEmitclean for the touched files. Preview click-through pending (needs an authed account with an existing enabled task).Tracked in chat#1889 (matrix row 12).
Summary by cubic
Fixes onboarding so
/setup/tasksno longer re-runs and re-schedules the weekly report when one already exists. Prevents duplicate billed LLM runs and multiple schedules (chat#1889, row 12)./tasks.Written for commit 2f0a282. Summary will update on new commits.