Skip to content

Implement skipped Copilot sync idempotency and lock tests #46

@studert

Description

@studert

Problem

Three integration-test placeholders for the sync framework's most important invariants are still it.todo:

  • Idempotency: re-running a sync must not duplicate billed_costs.
  • Update-on-change: re-running with a different source amount must update the existing row.
  • Mutual exclusion: a second sync on the same source_type must reject while the first is still in progress.

These are the exact invariants that prevent double-billing and silent data corruption — the highest-impact unit/integration tests we could land for the sync layer.

Evidence

tests/integration/sync/copilot-idempotent.test.ts:1-7:

import { describe, it } from \"vitest\";

describe(\"Copilot billing sync idempotency\", () => {
  it.todo(\"does not create duplicate billed_costs on re-run\");
  it.todo(\"updates billed_costs.amount_cents when source amount changes\");
});

tests/integration/sync/lock.test.ts:22:

// Full mutual exclusion test requires DB connection
it.todo(\"rejects concurrent syncs on same source (requires DB)\");

Underlying code under test:

  • Sync framework: src/lib/sync/framework.ts (advisory locks via pg_try_advisory_xact_lock + hashSourceType).
  • Copilot billing path: src/lib/sync/sources/github-copilot.ts (or whichever file in src/lib/sync/sources/ maps to the billing snapshot).

Proposed approach

copilot-idempotent.test.ts

  1. Stub the GitHub API client (src/lib/github.ts) to return a deterministic billing fixture.
  2. Test A — no duplicates:
    • Run the Copilot billing sync once → assert one row in billed_costs for the period.
    • Run it again with the same fixture → assert still exactly one row (and created event count = 0, updated count = 0 or 1 depending on framework semantics).
  3. Test B — update on change:
    • Run with amount = X → row exists with amount_cents = X*100.
    • Run with amount = X+5 → same row, amount_cents = (X+5)*100, no extra row.

lock.test.ts

Replace the it.todo with a test that:

  1. Acquires a transaction and calls withSyncLock(\"invoice_period_matching\", async () => { await sleep(...); }) in a held promise.
  2. While holding, calls withSyncLock again on the same source type from a second connection.
  3. Expects the second call to throw "Sync already in progress" (or whatever the framework's exact error is — read framework.ts to confirm).
  4. After the first promise resolves, a third call should succeed.

Use tests/integration/sync/ patterns from existing passing tests in this folder.

Acceptance criteria

  • Zero it.todo calls remain in copilot-idempotent.test.ts and lock.test.ts.
  • All three tests pass against a Neon test branch under pnpm test:integration.
  • Mutating either invariant (e.g. removing the unique-on-conflict logic in the sync source, or removing pg_try_advisory_xact_lock) makes the corresponding test fail.
  • pnpm lint && pnpm typecheck pass.

Verification

  1. pnpm test:integration tests/integration/sync/copilot-idempotent.test.ts tests/integration/sync/lock.test.ts is green locally.
  2. Comment out the conflict resolution in the Copilot billing source → both idempotency tests fail. Revert.
  3. Replace pg_try_advisory_xact_lock with true → the lock test fails. Revert.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:syncSync framework / cron / ingestionpriority:highShip soontestsTest coverage / disabled tests

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions