fix: skip onboarding when insights project was soft-deleted (CM-1138)

[ulemons]

Summary

gerritcodereview/gerrit (and any other repo whose insightsProjects row was
deliberately soft-deleted in the 2026-04 cleanup) failed re-onboarding with an opaque
Segment creation returned HTTP 500: Internal Server Error. The insights project's
slug unique index can't be made partial on deletedAt (three FKs from
securityInsights* tables reference it), so the soft-deleted row still owns the slug
and blocks creating a new one. Per Joana's decision, we don't resurrect these
automatically — we skip onboarding, track a reason, and leave the list for manual
review one by one.

Changes

• New projectCatalog.skipReason column (migration) — kept separate from
  onboardingError (a skip isn't an error) and from evaluationReason (these rows
  have evaluationResult = 'true', so a non-onboarding reason there would be
  contradictory).
• New findInsightsProjectBySlugIncludingDeleted DAL lookup — deliberately does not
  apply injectSoftDeletionCriteria and doesn't touch the existing
  queryInsightsProjects (blast-radius avoidance).
• New guarded writer markProjectCatalogOnboardingSkipped, same guard as
  markProjectCatalogOnboardingFailed (action = 'onboard' AND onboardedAt IS NULL)
  so a concurrent manual action always wins; clears any stale onboardingError from a
  prior failed attempt.
• onboardAndUpdateProject pre-checks for a soft-deleted insights project before
  calling the onboarding API; on a match it skips the HTTP call entirely and returns a
  typed 'skipped' outcome, counted separately from succeeded/failed in the batch
  log.
• Cross-cutting: onboarder HTTP error messages now include the response body
  (truncated to 500 chars), so future failures are diagnosable from
  onboardingError alone instead of needing the Slack error-reporter.
• Unit tests per ADR-0007/ADR-0008: pure-function coverage for the skip-reason builder
  and error-body truncation (passing), plus Postgres-backed DAL tests for the new
  lookup and writer (written, not yet run in this environment — local test DB port
  5434 is occupied by an unrelated container).

Post-merge / verification (manual, on staging/prod)

1. Run the migration, confirm skipReason exists.
2. Requeue gerrit:

   UPDATE "projectCatalog" SET action = 'onboard', "onboardingError" = NULL, "updatedAt" = NOW()
   WHERE "repoUrl" = 'https://github.com/gerritcodereview/gerrit';

3. Confirm gerrit lands in action = 'skip' with a skipReason citing the deletion
   date, and no segment/subproject gets created.
4. Monitoring query for the manual-review backlog:

   SELECT "repoUrl", action, COALESCE("skipReason", "evaluationReason") AS reason, "updatedAt"
   FROM "projectCatalog"
   WHERE action = 'skip' AND "skipReason" IS NOT NULL
   ORDER BY "updatedAt" DESC;

Type of change

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

JIRA ticket

https://linuxfoundation.atlassian.net/browse/CM-1138

[cursor]

PR Summary

Medium Risk
Changes onboarding workflow and project-catalog state transitions; guarded updates limit races, but wrong slug/deletion detection could skip repos that should onboard.

Overview
Prevents automatic onboarding from repeatedly failing when an insightsProjects row still holds the slug after soft deletion (segment creation would 500). The worker now detects that case before calling the onboarding API, marks the catalog row as action = 'skip' with a human-readable skipReason, and treats it as a non-failure in the batch.

Adds projectCatalog.skipReason (migration + DAL types/update path) and markProjectCatalogOnboardingSkipped, guarded like the existing failed marker so concurrent manual/onboarded state wins. findInsightsProjectBySlugIncludingDeleted looks up slugs without soft-delete filtering. onboardAndUpdateProject returns typed outcomes (onboarded, skipped, already-onboarded, catalog-changed); the workflow logs skipped and racedOut counts separately from successes.

Onboarder HTTP errors now append a truncated response body via readErrorBody for easier diagnosis in onboardingError. Unit/DB tests cover skip-reason formatting, error-body truncation, the new DAL helpers, and the skip writer.

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

[Copilot]

Pull request overview

Prevents onboarding when a matching insights project was soft-deleted, preserving it for manual review.

Changes:

• Adds persisted skip reasons and guarded skip transitions.
• Detects deleted insights projects before onboarding.
• Improves HTTP error diagnostics and test coverage.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
project-catalog/types.ts	Adds skip-reason types.
project-catalog/projectCatalog.ts	Persists and updates skip reasons.
project-catalog/projectCatalog.test.ts	Tests guarded skip transitions.
collections/index.ts	Adds deleted-inclusive slug lookup.
collections/index.test.ts	Tests slug lookup behavior.
workflows/onboardProjects.ts	Tracks skipped outcomes.
automatic_onboarding_worker/src/types.ts	Defines onboarding outcomes.
onboarder/onboarder.ts	Includes response bodies in errors.
onboarder/onboarder.test.ts	Tests error-body handling.
activities/insightsProjectSkip.ts	Builds skip reasons.
activities/insightsProjectSkip.test.ts	Tests reason formatting.
activities/activities.ts	Adds the deleted-project pre-check.
V1788516039__skip-reason-to-project-catalog.sql	Adds the database column.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

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

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

services/libs/data-access-layer/src/project-catalog/types.ts:63

• IDbProjectCatalogCreate now advertises that callers can persist skipReason, but insertProjectCatalog, both bulk paths, and upsertProjectCatalog omit that field from their SQL/value maps, so it is silently discarded. Either wire the field through every create path or, since this PR only writes it during a transition, remove it from the create contract.

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

services/libs/data-access-layer/src/project-catalog/projectCatalog.ts:579

• This line only narrates the visible SQL assignment and is not needed to preserve the non-obvious concurrency constraint. Remove it to follow the repository's comments convention while retaining the guard rationale.

services/apps/automatic_onboarding_worker/src/onboarder/onboarder.test.ts:21

• A Response constructed with a null body has no stream to disturb, so the second text() call can return '' normally and this test does not exercise readErrorBody's catch branch. Use a non-empty body before consuming it once.

    const response = new Response(null)