Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
ApprovabilityVerdict: Needs Human Approval Macroscope's review found this PR requires human approval — This is a cross-cutting, user-facing monthly scheduling capability that extends shared contracts and MCP input, changes server execution and rescheduling, and adds web and mobile editors. Existing paths appear preserved and tests are included, but the production scope and new product behavior warrant human review. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Schedule contracts and capability packages/contracts/src/scheduledTask.ts, packages/contracts/src/scheduledTask.test.ts, packages/contracts/src/orchestratorMcp.ts, packages/contracts/src/environment.ts, packages/contracts/src/environment.test.ts, apps/server/src/environment/ServerEnvironment.ts, apps/server/src/environment/ServerEnvironment.test.ts |
The contracts accept monthly schedules with a day from 1 to 31. The environment capability schema adds optional monthlyScheduledTasks, and the server descriptor advertises it as supported. |
Web and mobile draft conversion apps/web/src/components/settings/scheduledTasksSettings.logic.ts, apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts, apps/mobile/src/features/settings/scheduledTaskDraft.ts, apps/mobile/src/features/settings/scheduledTaskDraft.test.ts |
Draft logic converts monthly schedules, validates the day and time against the capability, and detects changes to the selected day. |
Monthly run calculation and persistence apps/server/src/scheduledTasks/Schedule.ts, apps/server/src/scheduledTasks/Schedule.test.ts, apps/server/src/scheduledTasks/ScheduledTaskService.schedule.test.ts |
The server calculates monthly run dates, skips months without the selected day, compares monthly schedules, and applies the missed-run grace period. Tests cover date boundaries and schedule persistence. |
Web and mobile schedule controls apps/web/src/components/settings/ScheduledTasksSettings.tsx, apps/web/src/components/settings/ScheduledTasksSettings.browser.test.tsx, apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx, docs/user/project-settings.md |
The settings editors add a Monthly option and a day selector while retaining weekly controls. Unsupported monthly drafts remain visible but cannot be saved. Monthly summaries show the selected day and time, and the interface and documentation state that months without the selected day are skipped. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Possibly related issues
- scheduler: scheduled task times shown in UTC instead of local/system time (IST) #18632 — This issue requests local-time display for scheduled-task times. This change documents the environment time zone for calendar schedules but does not address the request for local-time display.
Suggested reviewers: macroscopeapp, juliusmarminge, t3dotgg
Merge Risk: 
Minimal · up to e3fcc
-
No actionable issue is established for this change; it is mergeable after normal checks.
Pre-merge checks: 
3 · 
1
-
Failed checks: 1 warning Passed checks: 3 passedFull details: Description check
-
Explanation
The description includes all required sections and provides clear problem, implementation, verification results, UI evidence, and untested scenarios. However, the change adds a new scheduling workflow and explicitly states that no maintainer approval was obtained, while the template requires explicit approval unless the focused configuration exception clearly applies.
Resolution
Obtain and link explicit maintainer approval for the change’s direction and scope, including the approval comment. Alternatively, provide a specific justification that this broader scheduling behavior qualifies for the focused configuration exception, consistent with CONTRIBUTING.md.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
b4bfe5c to
296249e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 296249eea1
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Heads-up for whoever reviews this: #26001 solves the same problem with a different design, so you can compare the two.
I'm happy to go with whichever direction maintainers prefer. Written with Claude Opus 5.5 ( |
|
@nacholibre I like your design, too. I think what I prefer from my approach is the separation to Weekly/Monthly tab, while your date picker is probably better than mine. Maybe a mix of the 2 PRs could yield the best result |
|
closing at @StiensWout's request under Provide evidence for the changed behavior. Adds a Monthly schedule tab, day-of-month controls and associated mobile/web editor states, but does not supply a pre-feature before screenshot. Both supplied comparison images already show Monthly selected and the day selector; they compare At a time versus Weekly wording and different draft values. The author expressly admits the missing baseline. This is an inadequate comparison for the actual UI feature, not an inaccessible artifact. submission evidence: "Captures show the label rename; Monthly is present in both. Browser timeouts prevented a pre-feature baseline capture." Attach a clear pre-feature task-editor baseline and matching after screenshots demonstrating the new Monthly controls; retain the existing screenshots as supplementary evidence. thanks for trying to help improve t3 code. if you think this closure was a mistake, please open a fresh pr addressing the requirement above, and link back to this one. |
Problem
A 30-day interval drifts across months. Scheduled tasks need a selectable calendar date.
Change
Add Monthly with a day from 1 to 31 and an environment-local time. Skip months without that date. Support web/desktop, mobile, and existing MCP tools; rename "At a time" to "Weekly".
Use the existing server capability system to block unsupported monthly saves while preserving drafts.
Scope and approval
Submitted under the focused configuration exception: this adds a recurrence choice within existing scheduled tasks, reusing their lifecycle and execution. Existing schedules and defaults stay unchanged. Maintainer approval is not claimed; eligibility remains their decision.
Verification
vp run check:changedpassed: 68 tests, affected typechecks, lint, formatting, and Knip. Coverage includes calendar boundaries, persistence, and compatibility gates.Captures show the label rename; Monthly is present in both. Browser timeouts prevented a pre-feature baseline capture.
Saved monthly task on day 31
Implementation and independent review: GPT-6.1 Sol through the Codex harness in T3 Code.