Skip to content

Commit 9e72e3c

Browse files
committed
fix: never downgrade repo link provenance from packagist
Signed-off-by: anilb <epipav@gmail.com>
1 parent 5b609ca commit 9e72e3c

3 files changed

Lines changed: 53 additions & 10 deletions

File tree

services/apps/packages_worker/src/packagist/__tests__/persistPackageInfo.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
removeDeclaredPackageRepo,
77
updatePackagistPackageStats,
88
upsertPackageMaintainers,
9-
upsertPackageRepo,
9+
upsertPackageRepoPreserveProvenance,
1010
} from '@crowd/data-access-layer/src/packages'
1111
import type { QueryExecutor } from '@crowd/data-access-layer/src/queryExecutor'
1212

@@ -17,15 +17,15 @@ vi.mock('@crowd/data-access-layer/src/packages', () => ({
1717
updatePackagistPackageStats: vi.fn(),
1818
upsertPackageMaintainers: vi.fn().mockResolvedValue([]),
1919
getOrCreateRepoByUrl: vi.fn(),
20-
upsertPackageRepo: vi.fn().mockResolvedValue([]),
20+
upsertPackageRepoPreserveProvenance: vi.fn().mockResolvedValue([]),
2121
removeDeclaredPackageRepo: vi.fn().mockResolvedValue([]),
2222
logAuditFieldChanges: vi.fn(),
2323
}))
2424

2525
const mockUpdate = vi.mocked(updatePackagistPackageStats)
2626
const mockMaintainers = vi.mocked(upsertPackageMaintainers)
2727
const mockRepoGet = vi.mocked(getOrCreateRepoByUrl)
28-
const mockRepoLink = vi.mocked(upsertPackageRepo)
28+
const mockRepoLink = vi.mocked(upsertPackageRepoPreserveProvenance)
2929
const mockRepoRemove = vi.mocked(removeDeclaredPackageRepo)
3030
const mockAudit = vi.mocked(logAuditFieldChanges)
3131

services/apps/packages_worker/src/packagist/upsertPackageInfo.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
removeDeclaredPackageRepo,
55
updatePackagistPackageStats,
66
upsertPackageMaintainers,
7-
upsertPackageRepo,
7+
upsertPackageRepoPreserveProvenance,
88
} from '@crowd/data-access-layer/src/packages'
99
import type { QueryExecutor } from '@crowd/data-access-layer/src/queryExecutor'
1010

@@ -61,14 +61,18 @@ export async function persistPackagistPackageInfo(
6161
changedFields.push(...result.changedFields)
6262

6363
// Step 2: Link the repo for ALL packages — 'declared'/0.8 is the manifest-declared
64-
// convention shared by npm/pypi/maven/cargo. When there's no trusted repo (removed
65-
// from the manifest, or no longer canonicalizable to a known host), or it now
66-
// resolves to a different repo, clear any previously-declared link that no longer
67-
// applies — package_repos' unique key is (package_id, repo_id), not (package_id,
68-
// source), so upserting the new link alone would leave a stale one dangling.
64+
// convention shared by npm/pypi/maven/cargo. Preserve-provenance: a higher-confidence
65+
// link another pipeline (manual/deps_dev) already owns for this repo must not be
66+
// downgraded to 'declared'/0.8 by a routine weekly refresh — matches
67+
// upsertMavenPackageRepo/cargo's established GREATEST-confidence, source-preserving
68+
// pattern. When there's no trusted repo (removed from the manifest, or no longer
69+
// canonicalizable to a known host), or it now resolves to a different repo, clear any
70+
// previously-declared link that no longer applies — package_repos' unique key is
71+
// (package_id, repo_id), not (package_id, source), so upserting the new link alone
72+
// would leave a stale one dangling.
6973
if (trustedRepo) {
7074
const repo = await getOrCreateRepoByUrl(t, trustedRepo.url, trustedRepo.host)
71-
const linkChanged = await upsertPackageRepo(t, id, repo.id, 'declared', 0.8)
75+
const linkChanged = await upsertPackageRepoPreserveProvenance(t, id, repo.id, 'declared', 0.8)
7276
const removedFields = await removeDeclaredPackageRepo(t, id, repo.id)
7377
changedFields.push(...repo.changedFields, ...linkChanged, ...removedFields)
7478
} else {

services/libs/data-access-layer/src/packages/repos.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,42 @@ export async function upsertPackageRepo(
8787
)
8888
return row.changed_fields
8989
}
90+
91+
// Same shape as upsertPackageRepo, but never lets this write downgrade an existing
92+
// link: `source` is left untouched on conflict (a manual/deps_dev link keeps its
93+
// provenance instead of being reassigned to this caller's source) and `confidence`
94+
// only ever moves up via GREATEST. Matches the established pattern in
95+
// upsertMavenPackageRepo (osspckgs/repos.ts) and cargo/enrich.ts's inline equivalent —
96+
// added here as a separate function rather than changing upsertPackageRepo itself,
97+
// which pypi/npm/go/nuget/rubygems also call and rely on staying an unconditional set.
98+
export async function upsertPackageRepoPreserveProvenance(
99+
qx: QueryExecutor,
100+
packageId: string,
101+
repoId: string,
102+
source: string,
103+
confidence: number,
104+
): Promise<string[]> {
105+
const row: { changed_fields: string[] } = await qx.selectOne(
106+
`WITH old AS (
107+
SELECT source, confidence FROM package_repos
108+
WHERE package_id = $(packageId)::bigint AND repo_id = $(repoId)::bigint
109+
),
110+
ins AS (
111+
INSERT INTO package_repos (package_id, repo_id, source, confidence, created_at)
112+
VALUES ($(packageId)::bigint, $(repoId)::bigint, $(source), $(confidence), NOW())
113+
ON CONFLICT (package_id, repo_id) DO UPDATE SET
114+
confidence = GREATEST(EXCLUDED.confidence, package_repos.confidence),
115+
verified_at = NOW()
116+
RETURNING source, confidence
117+
)
118+
SELECT array_remove(ARRAY[
119+
CASE WHEN o.source IS NULL THEN 'package_repos.repo_id' END,
120+
CASE WHEN o.source IS NULL THEN 'package_repos.source' END,
121+
CASE WHEN o.source IS NULL
122+
OR o.confidence IS DISTINCT FROM ins.confidence THEN 'package_repos.confidence' END
123+
], NULL) AS changed_fields
124+
FROM ins LEFT JOIN old o ON true`,
125+
{ packageId, repoId, source, confidence },
126+
)
127+
return row.changed_fields
128+
}

0 commit comments

Comments
 (0)