fix: npm 429 retries

[epipav]

No description provided.

[cursor]

PR Summary

Medium Risk
Changes Temporal retry timing and batch replay semantics for npm metadata ingest; incorrect skip logic could leave purls un-ingested, but the query is scoped to the same batch and activity schedule time.

Overview
Improves npm metadata ingest behavior when the registry returns 429 rate limits, so Temporal retries wait out npm’s penalty window instead of retrying too soon.

FetchErrorKind is now an enum (call sites updated across fetch clients). Packument 429 responses parse Retry-After into optional retryAfterSec on FetchError. ingestOne fails 429s with ApplicationFailure and nextRetryDelay derived from that header (default ~300s, clamped), rather than using short exponential backoff.

On ingestNpmPackageBatch activity retries, purls already marked scanned since the activity was first scheduled are skipped via new getNpmPurlsScannedSince, avoiding duplicate fetches and wasted rate budget.

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

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[github-actions]

⚠️ Jira Issue Key Missing

Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability.

Example:

• feat: add user authentication (CM-123)
• feat: add user authentication (IN-123)

Projects:

• CM: Community Data Platform
• IN: Insights

Please add a Jira issue key to your PR title.

[Copilot]

Pull request overview

Improves npm ingestion retries by honoring Retry-After and skipping packages completed during earlier Temporal attempts.

Changes:

• Propagates npm rate-limit delay metadata.
• Overrides Temporal retry delays for HTTP 429 responses.
• Skips packages already processed during retried batches.

Review note: Retry-After date values and delays over 900 seconds are not honored. The PR title also lacks the required JIRA key.

Reviewed changes

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

File	Description
npmPackageState.ts	Finds packages scanned since activity scheduling.
types.ts	Adds retry-delay metadata to fetch errors.
fetchPackument.ts	Reads Retry-After from 429 responses.
activities.ts	Customizes retries and skips completed work.

---

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

[epipav]

@cursor review

[Copilot]

Pull request overview

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

Suppressed comments (2)

services/apps/packages_worker/src/npm/activities.ts:126

• The one-hour cap can schedule a retry before the server-stated penalty ends when Retry-After exceeds 3600 seconds. Since this activity has no schedule-to-close timeout and only five attempts, repeated early retries can exhaust the lane while it is still rate-limited. Remove the cap so this branch actually honors the header.

      const delaySec = Math.min(Math.max(packumentResult.retryAfterSec ?? 300, 30), 3600) + 5

services/apps/packages_worker/src/npm/fetchPackument.ts:77

• 0 is a valid Retry-After delta, but this converts it to undefined, causing the caller to use the 300-second fallback instead of its 30-second minimum. Preserve zero here so a server that permits an immediate retry does not stall the lane for five minutes.

  if (Number.isFinite(seconds)) return seconds > 0 ? Math.ceil(seconds) : undefined

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

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

[Copilot]

Pull request overview

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

Suppressed comments (2)

services/apps/packages_worker/src/npm/fetchPackument.ts:77

• Retry-After: 0 is a valid delta-seconds value, but this maps it to undefined; the caller then substitutes 300 seconds rather than applying its 30-second safety floor. Preserve zero so this valid response does not cause an unnecessary five-minute pause.

  if (Number.isFinite(seconds)) return seconds > 0 ? Math.ceil(seconds) : undefined

services/apps/packages_worker/src/npm/activities.ts:126

• The 3600-second cap can schedule the next request before the server's Retry-After window expires, which defeats this retry path and can trigger another 429. Keep the 30-second floor, but do not shorten a valid server-provided delay.

      const delaySec = Math.min(Math.max(packumentResult.retryAfterSec ?? 300, 30), 3600) + 5

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (1)

services/apps/packages_worker/src/npm/fetchPackument.ts:78

• Retry-After: 0 is a valid delta-seconds value, but this converts it to undefined. ingestOne then substitutes the 300-second fallback, producing a 305-second retry instead of applying its 30-second safety floor. Preserve zero so the nullish fallback does not replace an explicit server value.

  if (Number.isFinite(seconds)) return seconds > 0 ? Math.ceil(seconds) : undefined