[21379] [Bug fix] Todoist bug fix#21421
Conversation
…ng tasks
Describe the bug
The Todoist New Completed Task source (todoist-completed-task) does not emit an event when a recurring task is marked done. Non-recurring (one-off) task completions still emit correctly, so the trigger looks like it works, but every recurring completion is silently missed.
To Reproduce
Reproduced live against the Todoist API v1 (verified 2026-07-14). Using an authenticated Todoist account:
Create a recurring task: POST /api/v1/tasks with {"content": "recurring test", "due_string": "every day"}. Response confirms due.is_recurring: true.
Create a non-recurring control task: POST /api/v1/tasks with {"content": "normal test"} (due: null).
Complete both: POST /api/v1/tasks/{id}/close for each.
Re-fetch each task:
Recurring (GET /api/v1/tasks/{id}): checked: false, completed_at: null, completed_count: 1, and due.date has advanced to the next day. The task is NOT in a completed state — it was rescheduled.
Control (GET /api/v1/tasks/{id}): checked: true, completed_at set.
Query the exact endpoint the source polls: GET /api/v1/tasks/completed/by_completion_date?since=<today>&until=<tomorrow>.
Result: items contains only the control task. The recurring completion is absent, even though completed_count incremented.
For completeness, GET /api/v1/tasks/completed/by_due_date?since=...&until=... over the occurrence's due date also returns items: [] for the recurring completion.
Net: the recurring task was completed (Todoist recorded it), but it appears in neither v1 REST completed-task endpoint, so the source never emits it.
Expected behavior
Marking a recurring Todoist task as done should emit a "New Completed Task" event, matching the pre-v1-migration behavior.
Screenshots
N/A (verified via raw API responses; can attach the request/response bodies if helpful).
Additional context
Add any other context about the problem here.
Changes in the completed task for supporting recurring task completion
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
dannyroosevelt
moved this to Ready for PR Review
in Component (Source and Action) Backlog
📝 WalkthroughWalkthroughThe Todoist completed-task source now detects recurring completions through the Activity Log, combines them with standard completed tasks, caps and emits events, and advances polling state only after successful emission. Shared constants and a REST client method support the polling flow. ChangesTodoist completion event flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Version update
…to Todoist-bug-fix
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@components/todoist/sources/completed-task/completed-task.mjs`:
- Around line 94-97: Update the catch block around getActivityLogs in the
completed-task source to return an empty list only for the documented
premium-plan 403 error. Re-throw network failures, authentication errors, rate
limits, and other unexpected errors so they remain visible to the polling flow,
while preserving the existing premium-restriction message for the handled case.
- Around line 99-145: Update getSyncResult to sort allResults by ascending
timestamp before applying MAX_RESULTS_PER_POLL, so capped retains the oldest
events in the window. Keep _nextLastDate based on the last capped event,
ensuring the watermark advances only past returned events and newer results
remain available for the next poll.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ea72e9d5-f168-4de4-8d7e-844f56b5907b
📒 Files selected for processing (4)
components/todoist/common/constants.mjscomponents/todoist/package.jsoncomponents/todoist/sources/completed-task/completed-task.mjscomponents/todoist/todoist.app.mjs
code cleanup and adding correct comments
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/todoist/sources/completed-task/completed-task.mjs (1)
138-145: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winWatermark gets stuck at the last event's timestamp unless the cap is reached.
If
capped.length > 0but we processed all available results (i.e.allResults.length <= MAX_RESULTS_PER_POLL), setting the watermark to the newest event's timestamp (capped[capped.length - 1].ts) prevents the watermark from advancing to the end of the polling window (dateTo). On the next poll,dateFromwill start at that old event's timestamp again, continually re-fetching it. This causes the polling window (dateTo - dateFrom) to grow indefinitely during inactive periods, which can eventually violate API date-range limits and cause redundant data fetching.To maintain continuity (as described in the
constants.mjscomments), the watermark should advance todateTowhen all events in the window have been processed, and should only be held back at an event's timestamp if the batch was capped (leaving unprocessed events behind).🐛 Proposed fix: only hold the watermark back if the cap was exceeded
- this._nextLastDate = capped.length + this._nextLastDate = allResults.length > MAX_RESULTS_PER_POLL ? new Date(capped[capped.length - 1].ts).toISOString() : dateTo;🤖 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 `@components/todoist/sources/completed-task/completed-task.mjs` around lines 138 - 145, Update the _nextLastDate assignment in the completed-task polling flow to use dateTo whenever allResults.length is at or below MAX_RESULTS_PER_POLL, including when capped contains events. Only use the newest capped event timestamp when the result set exceeded the cap and unprocessed events remain.
🤖 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.
Outside diff comments:
In `@components/todoist/sources/completed-task/completed-task.mjs`:
- Around line 138-145: Update the _nextLastDate assignment in the completed-task
polling flow to use dateTo whenever allResults.length is at or below
MAX_RESULTS_PER_POLL, including when capped contains events. Only use the newest
capped event timestamp when the result set exceeded the cap and unprocessed
events remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e2dcf0c4-d786-4441-a282-c29f673a3bed
📒 Files selected for processing (3)
components/todoist/common/constants.mjscomponents/todoist/package.jsoncomponents/todoist/sources/completed-task/completed-task.mjs
Review comment changes
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
components/todoist/sources/completed-task/completed-task.mjs (3)
91-91: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winBound the recurring dedupe ID to 64 characters.
The constructed
${entry.object_id}-${entry.event_date}ID has no length guard or hash. Enforce the source contract with a stable bounded-ID helper so deduplication cannot silently stop working for longer API values.🤖 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 `@components/todoist/sources/completed-task/completed-task.mjs` at line 91, Update the dedupeId construction in the completed-task source to use the project’s stable bounded-ID helper, ensuring the resulting ID is limited to 64 characters while remaining deterministic for the same entry.object_id and entry.event_date values.Source: Path instructions
45-45: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winUse
completed_athere. The/tasks/completed/by_completion_datepayload exposescompleted_at;task.date_completedis undefined, sotsbecomesNaNand the poll can fail when the watermark is written.🤖 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 `@components/todoist/sources/completed-task/completed-task.mjs` at line 45, Update the timestamp assignment in the completed-task mapping to parse task.completed_at instead of task.date_completed, ensuring the completed-task poll writes a valid watermark.Source: Path instructions
47-47: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude
date_completedin the non-recurring dedupe ID.${task.id}-completedkeys off the task, so reopening and re-completing the same task can collapse later completions. Usetask.id+date_completedfor the event identity.🤖 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 `@components/todoist/sources/completed-task/completed-task.mjs` at line 47, Update the dedupeId construction in the completed-task mapping to include both task.id and task.date_completed, while retaining the completed-event context, so separate completions after reopening receive distinct identities.Source: Path instructions
♻️ Duplicate comments (1)
components/todoist/sources/completed-task/completed-task.mjs (1)
135-155:⚠️ Potential issue | 🔴 CriticalFilter eligible projects before capping and advancing the watermark.
The oldest-first sort fixes the previous newest-cap loss, but
filterResults()still runs after the cap. If the first 20 events belong to unselected projects and a selected-project completion is 21st, the source emits nothing for that project while advancinglastDatepast it, permanently skipping the event.🤖 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 `@components/todoist/sources/completed-task/completed-task.mjs` around lines 135 - 155, Update the completed-task fetch flow so project eligibility filtering occurs before sorting, capping, and calculating this._nextLastDate. Reuse the logic from filterResults() or extract it into a shared helper, ensuring only selected-project completions influence MAX_RESULTS_PER_POLL and the watermark while preserving the existing emitResults() flow.
🤖 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.
Outside diff comments:
In `@components/todoist/sources/completed-task/completed-task.mjs`:
- Line 91: Update the dedupeId construction in the completed-task source to use
the project’s stable bounded-ID helper, ensuring the resulting ID is limited to
64 characters while remaining deterministic for the same entry.object_id and
entry.event_date values.
- Line 45: Update the timestamp assignment in the completed-task mapping to
parse task.completed_at instead of task.date_completed, ensuring the
completed-task poll writes a valid watermark.
- Line 47: Update the dedupeId construction in the completed-task mapping to
include both task.id and task.date_completed, while retaining the
completed-event context, so separate completions after reopening receive
distinct identities.
---
Duplicate comments:
In `@components/todoist/sources/completed-task/completed-task.mjs`:
- Around line 135-155: Update the completed-task fetch flow so project
eligibility filtering occurs before sorting, capping, and calculating
this._nextLastDate. Reuse the logic from filterResults() or extract it into a
shared helper, ensuring only selected-project completions influence
MAX_RESULTS_PER_POLL and the watermark while preserving the existing
emitResults() flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b206f40-7cd7-4d5f-bfa5-f7f096aad58a
📒 Files selected for processing (1)
components/todoist/sources/completed-task/completed-task.mjs
|
|
||
| return allTasks.map((task) => ({ | ||
| projectId: task.project_id, | ||
| ts: Date.parse(task.date_completed), |
There was a problem hiding this comment.
Coderabbit was correct here. It should be task.completed_at.
Use completed_at here. The /tasks/completed/by_completion_date payload exposes completed_at; task.date_completed is undefined, so ts becomes NaN and the poll can fail when the watermark is written.
| projectId: task.project_id, | ||
| ts: Date.parse(task.date_completed), | ||
| summary: `Completed task: ${task.content ?? task.id}`, | ||
| dedupeId: `${task.id}-completed`, |
There was a problem hiding this comment.
Per coderabbit:
Include date_completed in the non-recurring dedupe ID. ${task.id}-completed keys off the task, so reopening and re-completing the same task can collapse later completions. Use task.id + date_completed for the event identity.
| projectId: entry.parent_project_id, | ||
| ts: Date.parse(entry.event_date), | ||
| summary: `Completed task: ${entry.extra_data?.content ?? entry.object_id}`, | ||
| dedupeId: `${entry.object_id}-${entry.event_date}`, |
There was a problem hiding this comment.
Per coderabbit:
Bound the recurring dedupe ID to 64 characters.
The constructed ${entry.object_id}-${entry.event_date} ID has no length guard or hash. Enforce the source contract with a stable bounded-ID helper so deduplication cannot silently stop working for longer API values.
4 participants
Summary
closes: #21379
Describe the bug
The Todoist New Completed Task source (todoist-completed-task) does not emit an event when a recurring task is marked done. Non-recurring (one-off) task completions still emit correctly, so the trigger looks like it works, but every recurring completion is silently missed.
To Reproduce
Reproduced live against the Todoist API v1 (verified 2026-07-14). Using an authenticated Todoist account:
Create a recurring task: POST /api/v1/tasks with {"content": "recurring test", "due_string": "every day"}. Response confirms due.is_recurring: true.
Create a non-recurring control task: POST /api/v1/tasks with {"content": "normal test"} (due: null).
Complete both: POST /api/v1/tasks/{id}/close for each.
Re-fetch each task:
Recurring (GET /api/v1/tasks/{id}): checked: false, completed_at: null, completed_count: 1, and due.date has advanced to the next day. The task is NOT in a completed state — it was rescheduled.
Control (GET /api/v1/tasks/{id}): checked: true, completed_at set.
Query the exact endpoint the source polls: GET /api/v1/tasks/completed/by_completion_date?since=&until=.
Result: items contains only the control task. The recurring completion is absent, even though completed_count incremented.
For completeness, GET /api/v1/tasks/completed/by_due_date?since=...&until=... over the occurrence's due date also returns items: [] for the recurring completion.
Net: the recurring task was completed (Todoist recorded it), but it appears in neither v1 REST completed-task endpoint, so the source never emits it.
Expected behavior
Marking a recurring Todoist task as done should emit a "New Completed Task" event, matching the pre-v1-migration behavior.
Screenshots
N/A (verified via raw API responses; can attach the request/response bodies if helpful).
Additional context
Add any other context about the problem here.
Checklist
Please check the following items before your PR can be reviewed:
Versioning
0.0.1for new ones)package.json's version updatedNew app
If this is a new app, please submit an app integration request - the PR will only be reviewed after the app is integrated.
CodeRabbit review
After the PR is opened, and if new changes are pushed, CodeRabbit will automatically review it. Do not 'mark as resolved' CodeRabbit's comments, but reply to them instead, whether you agree (and update the PR accordingly) or disagree.
Summary by CodeRabbit