Skip to content

Fix Notion sync checkpoint handling - #684

Open
iamakulov wants to merge 1 commit into
mainfrom
codex/fix-notion-sync-checkpoint
Open

Fix Notion sync checkpoint handling#684
iamakulov wants to merge 1 commit into
mainfrom
codex/fix-notion-sync-checkpoint

Conversation

@iamakulov

@iamakulov iamakulov commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

This PR fixes a Notion sync bug where, when one or more items would fail to sync, we’d still treat the sync as successful and advance the checkpoint. The result of this would be that the failed items would stay outdated but would be treated as fresh (until you edit them again).

QAed by blocking one of the requests that the plugin makes to the Notion API to fetch a record.

@iamakulov
iamakulov marked this pull request as ready for review August 10, 2026 16:46
Copilot AI lite review requested due to automatic review settings August 10, 2026 16:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes Notion sync checkpoint advancement so that collections don’t record a successful sync checkpoint when one or more items failed to import, preventing those failed items from being incorrectly treated as up-to-date.

Changes:

  • Capture a conservative syncStartedAt timestamp at the beginning of the sync to use as the next checkpoint on full success.
  • Only persist PLUGIN_KEYS.LAST_SYNCED when there are zero per-item sync errors; otherwise keep the previous checkpoint and surface a sync failure.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread plugins/notion/src/data.ts
@iamakulov
iamakulov force-pushed the codex/fix-notion-sync-checkpoint branch from c188722 to 578b26a Compare August 10, 2026 17:39
@iamakulov
iamakulov requested a review from kaloyanvi August 10, 2026 18:05

@kaloyanvi kaloyanvi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! Left two nits

status: "partial"
succeeded: number
failed: number
total: number

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: do we need all 3, we can derive the total right?


if (syncErrors.length > 0) {
return {
status: "partial",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: it's bit strange that if all items error we still have "partial" status even though none succeeded

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.

3 participants