Repository navigation
Confidence 8/10 · 1 issue to address
🟢 Confidence 8/10 · likely safe to merge. The lifecycle logic is sound and well tested; the one gap is the test meant to pin the exact-advisory-id preference, which passes either way.
1 warning −1 · tests partial −1 · risk medium · 1/1 new units observable
Adds the dependency scanner's three Postgres tables, their migration and DependencyScanStore: a scan stores its matches and a tracked-branch head scan reconciles findings. The lifecycle, tenant and ordering guarantees hold; one test does not pin the fix it was added for.
completeScanstores a scan's matches and settles it in one transactionreconcileFindingsopens, reopens and fixes findings from a tracked-branch head scankeepKnownAdvisoryIdkeeps a finding's id and triage when OSV relinks an advisory- New
dependency_scans,dependency_scan_matches,dependency_findingstables, purged with their org
Before merge
- Migration · Check it is safe on production data and with the code still deployed ·
packages/db/drizzle/20261031225742_dependency_scans/migration.sql
Findings
🟠 Warning · F4 · The exact-advisory-id test passes with or without the fix it covers
tests · packages/backend/src/services/dependency-scan/DependencyScanStore.test.ts:215-221
Both findings stay open under the old keepKnownAdvisoryId too: whichever row the unordered query returns first is renamed, and namedByMatch then rescues the other because the renamed match's aliases hold its id. The assertion cannot fail, so a regression of the exact-id preference in DependencyScanStore.ts:236 goes green.
Also assert the scan's stored matches: after scan `b`, `scanMatches` must hold two rows with advisory ids `GO-2026-1` and `GHSA-bbbb` — the rename collapses them into one.
Production impact
Telemetry this change adds and removes (3)
- ➕ span name
DependencyScanStore.startScan·packages/backend/src/services/dependency-scan/DependencyScanStore.ts:434 - ➕ span name
DependencyScanStore.failScan·packages/backend/src/services/dependency-scan/DependencyScanStore.ts:562 - ➕ span name
DependencyScanStore.listScans·packages/backend/src/services/dependency-scan/DependencyScanStore.ts:667
What was checked
- The settle path takes the scan row
forUpdateand a per-repository advisory lock, so an older scan reads a newer one's committed row (DependencyScanStore.ts:477-531) vcs_repositoriesids are globally unique (tables/vcs.ts:161), so the org-less unique finding key cannot collide across tenants- The new bulk readers dedupe their
in_lists and short-circuit an empty list (DependencyScanStore.ts:586,:754,:775)
Observability coverage: 1 of 1 changes observable
| Change | Kind | Observable | Evidence |
|---|---|---|---|
| DependencyScanStore (12 methods, Postgres reads and writes) | service | yes | Each method is an Effect.fn("DependencyScanStore.<name>") span and every query goes through makeDbExecute, which logs the failure with the span name (db-execute.ts:69-77) |
c20c149 · Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
Annotations
maple-review-bot / Maple / review
tests: The exact-advisory-id test passes with or without the fix it covers
Both findings stay `open` under the old `keepKnownAdvisoryId` too: whichever row the unordered query returns first is renamed, and `namedByMatch` then rescues the other because the renamed match's aliases hold its id. The assertion cannot fail, so a regression of the exact-id preference in `DependencyScanStore.ts:236` goes green.