fix: dedup advisory_affected_ranges via diff-based upsert (CM-1258)

[themarolt]

Summary

Fixes two duplication bugs in advisory_affected_ranges for the OSV/deps.dev sync: zombie rows from hard-delete+reinsert not propagating deletes to Tinybird, and OSV/deps.dev rows for the same range never colliding.

Changes

• Add deleted_at soft-delete column to advisory_affected_ranges (migration).
• Replace the OSV worker's hard-delete + reinsert sweep with a diff-based upsert (reconcileOsvRanges): unchanged tuples are left untouched (no write, no Sequin/Tinybird event), new tuples are upserted, stale tuples are soft-deleted instead of hard-deleted.
• Add supersedeDepsDevRanges to soft-delete live deps.dev raw rows once OSV writes structured ranges for the same advisory_package, per the existing "OSV overwrites deps.dev over time" source-of-truth rule.
• Add deletedAt to the Tinybird advisoryAffectedRanges datasource and filter it in ossPackages_enriched.pipe's join — FINAL alone doesn't collapse two distinct PKs claiming the same real-world range.
• Record the decided strategy in docs/adr/0001-oss-packages-design-decisions.md.

Type of change

• Bug fix
• New feature
• Refactor / cleanup
• Performance improvement
• Chore / dependency update
• Documentation

JIRA ticket

CM-1258

[cursor]

PR Summary

Medium Risk
Changes advisory vulnerability range persistence, OSV/deps.dev concurrency locking, and Tinybird open-vuln scoring; incorrect rollout without the documented Tinybird resnapshot could leave false-positive ranges downstream.

Overview
CM-1258 stops duplicate advisory_affected_ranges rows in Postgres and Tinybird by replacing OSV’s hard-delete + reinsert sweep with diff-based reconciliation and deleted_at soft-delete.

Postgres adds deleted_at plus a partial live index. reconcileOsvRanges upserts new tuples, leaves unchanged OSV rows alone (no updated_at churn), and tombstones dropped tuples; supersedeDepsDevRanges retires live deps.dev raw rows once OSV owns the package. The OSV worker calls these instead of deleteOsvOnlyRanges / per-row inserts, reconciles advisory packages dropped from the payload with an empty range set, and uses ordered FOR UPDATE on advisory_packages to avoid races with deps.dev. The deps.dev merge skips packages that already have live OSV structured ranges, can revive soft-deleted raw tuples, and runs a matching lock SQL ahead of merge.

API/SQL paths and getRangesForPackages ignore soft-deleted ranges. Tinybird gains deletedAt on the datasource and deletedAt IS NULL in ossPackages_enriched. ADR-0001 documents the strategy and notes a one-time Tinybird resnapshot for existing zombie rows.

^{Reviewed by Cursor Bugbot for commit a9b7a98. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Copilot]

Pull request overview

Fixes duplicate and stale advisory ranges across PostgreSQL and Tinybird.

Changes:

• Introduces soft deletion and diff-based OSV range reconciliation.
• Supersedes deps.dev ranges when OSV data exists.
• Filters deleted ranges in Tinybird and documents the strategy.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
ossPackages_enriched.pipe	Excludes deleted ranges.
advisoryAffectedRanges.datasource	Adds deletedAt.
packages/osv.ts	Reconciles and soft-deletes ranges.
osv/upsertAdvisory.ts	Uses the new reconciliation flow.
0001-oss-packages-design-decisions.md	Records the strategy.
V1784567585__advisory_affected_ranges_soft_delete.sql	Adds soft-delete schema support.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

services/apps/packages_worker/src/osv/upsertAdvisory.ts:52

• Mandatory: add database-backed coverage for this new reconciliation flow. The existing upsertAdvisory.test.ts only tests dedupeRanges, so regressions in the SQL/orchestration would go undetected. Cover unchanged tuples producing no update, stale tuples being soft-deleted, a deleted tuple being reactivated, and deps.dev raw rows being superseded while readers exclude deleted rows.

    await reconcileOsvRanges(
      qx,
      advisoryPackageId,
      dedupeRanges(entry.ranges).map((range) => ({

docs/adr/0001-oss-packages-design-decisions.md:367

• This newly recorded architecture decision does not follow the repository's ADR contract. .claude/rules/adr-format.md:18-28 requires ## Decision, ## Alternatives Considered, and ## Consequences (plus an allowed ADR status), but this is added as another subsection in the legacy living document without those sections. Record the decision as the next standalone ADR and add it to docs/adr/README.md, or bring ADR-0001 into the required structure.

### `advisory_affected_ranges` delete/dedup strategy

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

docs/adr/0001-oss-packages-design-decisions.md:367

• This new decision is appended as an ad-hoc subsection, but .claude/rules/adr-format.md:18-28 requires numbered ADRs to include the standard Context, Decision, Alternatives Considered, and Consequences sections and an allowed status. Please record this as a conforming ADR (and update the ADR index), or bring ADR-0001 into the required format while editing it.

### `advisory_affected_ranges` delete/dedup strategy

services/apps/packages_worker/src/osv/upsertAdvisory.ts:53

• The core behavior introduced here is not exercised by the existing upsertAdvisory tests, which only cover dedupeRanges. Please add database-backed coverage for unchanged tuples producing no update, stale tuples being soft-deleted, deleted tuples being revived, and deps.dev rows being superseded; otherwise regressions in the SQL that implements this bug fix will pass the current suite.

    await reconcileOsvRanges(
      qx,
      advisoryPackageId,
      dedupeRanges(entry.ranges).map((range) => ({
        advisoryPackageId,

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit d89f9fd. Configure here.}

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

services/apps/packages_worker/src/osv/upsertAdvisory.ts:68

• Sorting here is too late to establish a canonical lock order. Each earlier upsertAdvisoryPackage executes ON CONFLICT ... DO UPDATE (services/libs/data-access-layer/src/packages/osv.ts:132-135), which already takes and retains an advisory_packages row lock in OSV payload order. A concurrent deps.dev transaction can therefore lock the lower ID, wait on OSV's higher ID, while OSV's next upsert waits on the lower ID—the same deadlock this sort is intended to prevent. Acquire/lock the existing rows in ID order before any updating upsert, or add reliable deadlock retries around both transactions.

  entries.sort((a, b) => a.advisoryPackageId - b.advisoryPackageId)

services/apps/packages_worker/src/osv/upsertAdvisory.ts:82

• The new database reconciliation paths are untested: this module's current suite only exercises dedupeRanges, so regressions in unchanged-row no-ops, stale-row soft deletion, tombstone revival, deps.dev supersession, or transaction ordering would not be detected. Add database-backed scenarios for these state transitions, especially because the implementation relies on ON CONFLICT predicates and cross-writer locking.

    await reconcileOsvRanges(qx, advisoryPackageId, ranges)
    await supersedeDepsDevRanges(qx, advisoryPackageId)

docs/adr/0001-oss-packages-design-decisions.md:375

• This records a new architectural decision in an ADR that does not satisfy the repository's current ADR contract: .claude/rules/adr-format.md:18-28 requires an allowed status plus ## Decision, ## Alternatives Considered, and ## Consequences sections. ADR-0001 still uses the unsupported living status and lacks the required alternatives/consequences structure. Record CM-1258 as a new sequential ADR using the required template (and add it to docs/adr/README.md), or migrate this file to the required format.

**Decided fix — diff-based upsert with soft-delete, not sweep-and-reinsert:**

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

services/apps/packages_worker/src/osv/upsertAdvisory.ts:68

• Sorting at this point does not establish the claimed lock order. Each preceding upsertAdvisoryPackage executes INSERT ... ON CONFLICT DO UPDATE, which already row-locks an existing advisory_packages row in OSV payload order. For example, OSV can lock id 2 first while deps.dev locks id 1 first, after which each waits for the other's next row and the same deadlock remains. Acquire/update these rows in canonical order before any DO UPDATE, or add an idempotent 40P01 retry (including the deps.dev merge, whose activity has maximumAttempts: 1).

  entries.sort((a, b) => a.advisoryPackageId - b.advisoryPackageId)

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

services/apps/packages_worker/src/osv/upsertAdvisory.ts:68

• The canonical lock ordering happens too late. Each preceding upsertAdvisoryPackage executes ON CONFLICT ... DO UPDATE (services/libs/data-access-layer/src/packages/osv.ts:122-139), which already row-locks existing advisory_packages in arbitrary OSV payload order. A concurrent deps.dev transaction can therefore still lock a lower ID first while OSV holds a higher ID, recreating the deadlock this sort is intended to prevent; deps.dev then fails because its activity has maximumAttempts: 1. Acquire/update the existing rows in canonical order before any conflicting upsert, or add a deadlock retry around the transaction.

    })
  }

  // Packages OSV reported in a prior sync but that are missing from this payload
  // (parseOsvRecord drops a package once it has no usable ranges left, and a

services/apps/packages_worker/src/osv/upsertAdvisory.ts:70

• This only reconciles packages still present in the current OSV payload. If OSV removes an advisory/package entirely, its old structured ranges are never passed to reconcileOsvRanges, so they remain live; the new deps.dev NOT EXISTS guard then permanently treats that package as OSV-owned and cannot revive the raw row as documented. Reconcile the advisory's previously stored package set and tombstone omitted packages (or persist source ownership separately) before using live ranges as the ownership marker.

  // so their previously live OSV ranges would otherwise sit forever as false

services/apps/packages_worker/src/osv/tests/reconcileOsvRanges.integration.test.ts:143

• The packages-db connection registers TIMESTAMP/TIMESTAMPTZ parsers that return strings (services/libs/database/src/connection.ts:75-79), so updated_at is not a Date at runtime and this throws getTime is not a function whenever the DB-backed test actually runs. Compare the timestamp strings directly (and type the row accordingly).

    expect(afterUpdatedAt).toBe(beforeUpdatedAt)

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

services/apps/packages_worker/src/osv/upsertAdvisory.ts:91

• Sorting here does not establish the claimed canonical lock order. The preceding upsertAdvisoryPackage calls execute ON CONFLICT ... DO UPDATE (services/libs/data-access-layer/src/packages/osv.ts:132-139), so they already acquire and retain locks on existing advisory_packages rows in arbitrary OSV payload order. OSV can therefore lock a higher ID first while deps.dev locks the lower ID first, recreating the deadlock this block is intended to prevent; deps.dev's maximumAttempts: 1 then makes the merge fail. Acquire/update these rows in a shared canonical order before any lock-taking upsert, or retry the whole idempotent transaction on 40P01.

  entries.sort((a, b) => a.advisoryPackageId - b.advisoryPackageId)

services/apps/packages_worker/src/osv/upsertAdvisory.ts:80

• The newly added absent-package reconciliation path is not covered by the integration test: that test calls reconcileOsvRanges directly and never exercises findRemovedAdvisoryPackageIds through upsertAdvisoryBatch. A regression for an OSV record whose packages becomes empty would therefore leave prior ranges live without failing tests. Add a DB-backed workflow test that syncs a package, resyncs the advisory without it, and verifies the old OSV ranges are tombstoned while deps.dev rows remain eligible.

  const removedIds = await findRemovedAdvisoryPackageIds(
    qx,
    advisoryId,
    entries.map((e) => e.advisoryPackageId),
  )

docs/adr/0001-oss-packages-design-decisions.md:367

• This edit leaves ADR-0001 outside the mandatory ADR format. .claude/rules/adr-format.md:18-28 permits only proposed, accepted, deprecated, or superseded by ADR-NNNN status and requires ## Decision, ## Alternatives Considered, and ## Consequences; this file still uses Status: living and a ## Decisions layout. Bring it into the required format or move the living design log outside docs/adr/ before extending it.

### `advisory_affected_ranges` delete/dedup strategy

[joanagmaia]

LGTM! I think it addresses both issues as it should. As for Tinybird yeah I would say the easiest way woulld be to truncate the existing datasource, and then trigger a backfill from Sequin.
You can check this documentation https://github.com/linuxfoundation/crowd.dev/blob/main/services/libs/tinybird/README.md on how to achieve both