Skip to content

fix(knowledge): store connector sync schedules at millisecond precision - #7966

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/member-sync-timestamp-precision
Sep 18, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
fix/member-sync-timestamp-precision

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • The connector scheduler reads next_member_sync_at into a JavaScript Date and claims the run with an equality match on that value. Date carries milliseconds; PostgreSQL stored microseconds. A schedule written in SQL rather than by the application became permanently unmatchable once it landed on a fractional millisecond — the connector stayed due forever, every claim was refused, and its members never synced.
  • Narrow next_member_sync_at and next_sync_at to timestamp(3) so both ends compare the same value. Both columns move in one ALTER TABLE so the rewrite is a single pass. The rewrite rounds the stored values, so an already-wedged row recovers on the next scheduler tick — no backfill needed.
  • knowledge_connector_member.next_attempt_at is written by the same backfill but is only ever range-compared, never claimed by equality, so it is deliberately left alone. The migration header says so.
  • Stop describeUnacceptedMemberSync from asserting "a member sync is already queued or running" when it cannot see one. It now reads the lock token and reports plainly when no condition explains the refusal — the misleading fallback is what let this read as ordinary contention.
  • New PostgreSQL contract test pins the storage invariant: a sub-millisecond schedule written as a SQL literal cannot survive in either column. Verified red without the migration, green with it.
  • Extracted the disposable-schema fixture and the migration statement-splitter that this test would otherwise have copied verbatim from the retired-columns suite, and moved that suite onto it. The splitter was already duplicated across the package with a divergent empty-statement guard; the shared copy keeps the guard.

Type of Change

  • Bug fix

Testing

check:migrations ✓, lint ✓, check:audits (46) ✓, docs-manifest:check ✓, type-check (apps/sim + packages/db) ✓. Both migration-contract suites pass against a local PostgreSQL; the new test was confirmed to fail with the migration stubbed out. member-queue and queue unit suites pass (51).

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

The scheduler round-trips next_member_sync_at through a JavaScript Date and
claims the run by matching the value back exactly. Date carries milliseconds
while PostgreSQL stored microseconds, so a schedule written in SQL rather than
by the application became unmatchable the moment it landed on a fractional
millisecond: the connector stayed permanently due, every claim was refused, and
its members never synced.

Narrow both connector schedule columns to timestamp(3) so the two ends compare
the same value. The rewrite also rounds the stored values, so an already-wedged
row recovers on the next scheduler tick.

Also stop the claim diagnosis from asserting a queued run it cannot see — it now
reads the lock token and says so plainly when no condition explains the refusal,
which is what let this hide as ordinary contention.
@vercel

vercel Bot commented Sep 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs Ready Ready Preview Sep 18, 2026 7:09am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the previously identified recovery-test gap is resolved and no new actionable defects remain.

Summary

This PR aligns connector schedule storage with JavaScript Date precision and improves diagnostics for unexplained member-sync claim failures.

  • Narrows next_member_sync_at and next_sync_at to millisecond precision, repairing existing fractional-millisecond schedules during migration.
  • Updates the Drizzle schema and migration metadata to preserve the new precision contract.
  • Adds an isolated PostgreSQL contract test covering both existing-row recovery and subsequent writes.
  • Extracts shared disposable-schema and migration-replay helpers for database contract suites.
  • Expands CI to run both migration contract suites.
  • Distinguishes actual member-sync lock contention from otherwise unexplained claim refusals.

Reviews (2) · Last reviewed commit: "test(knowledge): pin the schedule rewrit..."

Comment thread packages/db/scripts/connector-sync-schedule-precision.postgres.test.ts Outdated

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 9 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

…rows

Scope the column-precision query to the test schema. It matched on table name
alone, so in CI — where the database already holds the migrated public table —
it saw four columns instead of two and failed. A local run passed because the
throwaway schema was the only place that table existed.

Write the sub-millisecond schedule before applying the migration rather than
after, so the test pins the rewrite rounding an already-wedged row, not just the
narrowed column refusing new ones. Without the migration it now fails on exactly
that claim.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 9 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 4d64b37 into staging Sep 18, 2026
34 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/member-sync-timestamp-precision branch September 18, 2026 07:12

This branch was successfully deployed

1 active deployment
Preview 99d91d5c Deployed Sep 18, 2026 by vercel[bot]
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.

1 participant