Skip to content

fix: npm repository url gap (CM-1305)#4334

Open
ulemons wants to merge 2 commits into
mainfrom
fix/npm-repository-url-gap
Open

fix: npm repository url gap (CM-1305)#4334
ulemons wants to merge 2 commits into
mainfrom
fix/npm-repository-url-gap

Conversation

@ulemons

@ulemons ulemons commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the npm repository_url normalization gap (Gap D): canonicalizeRepoUrl failed to parse scp-style SSH URLs written under the ssh:// scheme (e.g. ssh://git@github.com:owner/repo.git), returning null and leaving repository_url empty even though a usable repo URL was declared. Adds an offline backfill so existing npm rows can be recomputed from the already-stored declared_repository_url without re-fetching the registry.

Changes

  • src/utils/canonicalizeRepoUrl.ts: handle ssh://git@host:owner/repo (colon before the path) before the existing ssh://git@host/owner/repo (slash) case — new URL() throws on the colon form because the segment after the host looks like a port, so it's rewritten to https:// first.
  • src/utils/__tests__/canonicalizeRepoUrl.test.ts: added cases for the scp-colon form and a digit-leading owner name (1inch/...), plus extra unparseable-input cases.
  • services/libs/data-access-layer/src/packages/packages.ts: new listNpmPackagesForRepoUrlRecompute / updateNpmRepositoryUrls DAL functions (keyset-paginated scan by id, split clear-vs-set UPDATE to avoid NULLs inside a text[] literal), modeled directly on the existing Maven equivalents.
  • src/npm/backfillRepositoryUrl.ts (new): recomputes repository_url for every npm row from its stored declared_repository_url — no packument re-fetch. Keeps package_repos consistent: prunes the stale source='declared' link and re-links via the same getOrCreateRepoByUrl/upsertPackageRepo pair the ingest path uses. Wrapped in withDeadlockRetry (duplicated from Maven's helper — see Notes) since the UPDATE + prune + relink must commit atomically per batch.
  • src/bin/npm-repo-url-backfill.ts (new): CLI entrypoint, mirrors bin/maven-repo-url-backfill.ts (--dry-run, --critical-only, graceful SIGINT/SIGTERM, NPM_REPO_URL_BACKFILL_BATCH_SIZE env var).
  • package.json: added backfill:npm-repo-url / backfill:npm-repo-url:local scripts.

Type of change

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

JIRA ticket

CM-1305


Note

Medium Risk
Batch writes touch packages, repos, and package_repos at scale; transactions and idempotent updates limit inconsistency, but a full backfill run still mutates core package metadata.

Overview
Fixes npm repository_url gaps where canonicalizeRepoUrl returned null for scp-style SSH declarations (ssh://git@github.com:owner/repo), leaving canonical URLs empty despite a stored declared_repository_url.

canonicalizeRepoUrl now rewrites ssh://git@host:path (colon before the path) to https:// before URL parsing, while still treating a numeric segment after the colon as a real SSH port. Tests cover digit-leading owners, git+https, and additional invalid inputs.

Adds an offline npm repo-url backfill (Maven-style): keyset scan of npm rows with a declared URL, recompute repository_url without packument fetches, and keep package_repos in sync via atomic batch transactions (update, prune stale declared links, relink) with deadlock retry. New DAL helpers listNpmPackagesForRepoUrlRecompute / updateNpmRepositoryUrls, CLI npm-repo-url-backfill (--dry-run, --critical-only, graceful shutdown), and backfill:npm-repo-url scripts.

Reviewed by Cursor Bugbot for commit 79e3552. Bugbot is set up for automated code reviews on this repo. Configure here.

@ulemons ulemons self-assigned this Jul 13, 2026
Copilot AI review requested due to automatic review settings July 13, 2026 16:06
@ulemons ulemons added the Bug Created by Linear-GitHub Sync label Jul 13, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Fix All in Cursor

❌ 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 a9be6d4. Configure here.

Comment thread services/apps/packages_worker/src/npm/backfillRepositoryUrl.ts

Copilot AI 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.

Pull request overview

Fixes npm repository_url normalization for scp-style SSH URLs under ssh:// and introduces an offline backfill to recompute packages.repository_url (and keep package_repos consistent) from already-stored declared_repository_url without re-fetching npm registry data.

Changes:

  • Extend canonicalizeRepoUrl to handle ssh://git@host:owner/repo(.git) inputs and add unit tests for the new cases.
  • Add DAL helpers to keyset-scan npm package rows and batch-update repository_url (including NULL clears) while bumping last_synced_at.
  • Add an npm repo-url backfill worker + CLI entrypoint and expose it via package.json scripts.

Reviewed changes

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

Show a summary per file
File Description
services/libs/data-access-layer/src/packages/packages.ts Adds npm-specific scan/update helpers for recomputing repository_url in batches.
services/apps/packages_worker/src/utils/canonicalizeRepoUrl.ts Adds normalization for ssh://git@host:owner/repo scp-colon form.
services/apps/packages_worker/src/utils/tests/canonicalizeRepoUrl.test.ts Adds regression tests for new canonicalization inputs.
services/apps/packages_worker/src/npm/backfillRepositoryUrl.ts New offline backfill: recompute URLs and sync package_repos in atomic batches.
services/apps/packages_worker/src/bin/npm-repo-url-backfill.ts New CLI wrapper with --dry-run / --critical-only and batch-size env var.
services/apps/packages_worker/package.json Adds backfill:npm-repo-url scripts.

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

Comment thread services/apps/packages_worker/src/utils/canonicalizeRepoUrl.ts
Comment thread services/apps/packages_worker/src/npm/backfillRepositoryUrl.ts Outdated
Comment thread services/apps/packages_worker/src/npm/backfillRepositoryUrl.ts
Comment thread services/apps/packages_worker/src/npm/backfillRepositoryUrl.ts
Comment thread services/apps/packages_worker/src/npm/backfillRepositoryUrl.ts Outdated
Copilot AI review requested due to automatic review settings July 13, 2026 16:22
ulemons added 2 commits July 13, 2026 18:24
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
@ulemons ulemons force-pushed the fix/npm-repository-url-gap branch from 18cffab to 79e3552 Compare July 13, 2026 16:24

Copilot AI 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.

Pull request overview

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

Comment on lines +111 to +113
// declaredRepositoryUrl is guaranteed non-null by the query's IS NOT NULL filter.
const canonical = canonicalizeRepoUrl(row.declaredRepositoryUrl as string)
const desired = canonical?.url ?? null
Copilot AI review requested due to automatic review settings July 13, 2026 16:28

Copilot AI 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.

Pull request overview

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

// segment before the next `/` is a real port (e.g. `ssh://git@host:2222/owner/repo`),
// not an scp-style owner — leave those for the generic case, which URL parses fine.
const sshScp = s.match(/^ssh:\/\/git@([^/:]+):(.+)$/)
const sshScpIsPort = sshScp ? /^\d+$/.test(sshScp[2].split('/')[0]) : false
// so the inconsistency would never be repaired.
await withDeadlockRetry(() =>
qx.tx(async (t: QueryExecutor) => {
await updateNpmRepositoryUrls(t, updates)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Created by Linear-GitHub Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants