fix(pr-fix): keep the item URL as the PR URL when CI evidence re-enqueues it

[itsmiso-ai]

Closes #1098
Closes #1118

Problem

metadataPatch in src/lib/pr-fix-queue.ts overwrote a queued item's url on every re-enqueue, and CI-failure enqueues passed the failing job URL. The item's URL flipped between the PR URL and a CI job URL, and next-task handed the flipped value out as pullRequest.url via createFollowupPrTask.

Fix

Two layers, per the issue and grooming guidance:

1. Queue-level write-once guard (src/lib/pr-fix-queue.ts) — url is removed from metadataPatch (it still refreshes issue/branch/title/headSha/author). The item URL is now set at first enqueue and backfilled only while empty (null/""); no re-enqueue — fresh attempt or otherwise — can overwrite it.
2. Ingestion identity fix (src/lib/pr-followup-ingestion.ts + both pr-followup routes) — PrFollowupEvent.url now always means the PR URL. check_run events carry the job URL in a new optional checkRunUrl, which feeds the existing "Full log:" line in the feedback text (evidence, where job links belong). The webhook builder uses the associated pull request's API URL, falling back to "" (never a job URL, which the guard would freeze); the next sync enqueue backfills the real PR URL. ingestCheckRunEvent accepts checkRunUrl and documents the contract.

Tests

• Acceptance test from the issue: enqueue a review item, then a CI-failure item for the same PR — item url stays the PR URL while feedback/evidenceKeys still append (the job URL shows up in the evidence).
• URL backfill only while empty (undefined and empty-string shapes).
• Ingestion: check_run with checkRunUrl → item URL is the PR URL and the job URL is in the "Full log:" feedback; legacy shape (no checkRunUrl) keeps the old behavior via fallback.
• Webhook: payload whose first associated PR lacks url → event url is "", job URL in checkRunUrl.

Full suite green locally: 3491 passed / 16 skipped / 2 todo; lint and typecheck clean (one pre-existing unrelated lint warning). CI: all 13 checks green.

Notes

• Rows already flipped to a job URL are repaired (fix(pr-fix-queue): repair items whose URL was already poisoned with a CI job URL #1118): the write-once guard treats a stored Actions run URL as empty so it heals on the next enqueue, an enqueue never stores a job URL as the item URL (legacy callers included), and migration 20261004000000_repair_pr_fix_item_urls rewrites existing job-URL rows to the PR URL derived from repo + pr. Verified against Postgres: only the job-URL row changed; API URLs, web /pull/ URLs and NULLs are untouched.
• Workers never merge; this PR is left open for human review.

[itsmiso-ai]

Status: ready for human review. All 13 CI checks are green (Tests, Coverage, Database integration, Typecheck, Lint, Build, Docker Build x2, npm audit, smoke, review, workflow lint). Independent review pass ran before push; its follow-ups (webhook job-URL fallback replaced with an empty-string backfill path, ingestCheckRunEvent contract, EnqueuePrFixInput.url doc, empty-string backfill test) are included in commit 0acbc86. Legacy poisoned rows are tracked separately in #1118.

[its-saffron]

AI Automated Review (incremental)

Incremental review: reviewed the changes since the last managed review; unresolved findings from that review are carried forward.

Analysis engine: glm-5.3-flash@https://litellm.jory.dev/v1 (openai) — routed smart (risk match: db_or_migration_changes)

Recommendation: approve. The delta correctly completes the PR 1098/PR 1118 fix: a targeted, well-scoped data-repair migration plus the enqueue-time heal guard and tests for both. No data-loss risk identified; CI (including the Database migrations check) is green.

Change-by-change findings

• prisma/migrations/20261004000000_repair_pr_fix_item_urls/migration.sql (new) — Single targeted UPDATE that rewrites url to the PR API URL derived from repo + pr only where the stored value matches the Actions run/job URL shape (^https://github\.com/[^/]+/[^/]+/actions/runs/). The WHERE clause is anchored and narrow; no rows are deleted, and legitimate API-format, web /pull/, and NULL URLs are untouched (matching the regex semantics of ACTIONS_RUN_URL in pr-fix-queue.ts). This satisfies the must_check review migration for data loss risk: the only mutation is replacing a known-poisoned identity value with the correct one, which is the intent of PR 1118. One gap (minor): the regex — like isActionsRunUrl — does not match the legacy check-run html_url shape https://github.com/<org>/<repo>/runs/<id>, which is precisely the shape the old webhook path stored (this PR's own updated fixture uses .../runs/321). A row poisoned with that shape stays frozen by the write-once guard and is skipped by the migration. Issue PR 1118 scopes the repair to the Actions run/job shape and the known poisoned example matches it, so this is an edge case, not a blocker — but the guard and migration should ideally cover both shapes or the discrepancy should be documented.
• src/lib/pr-fix-queue.ts — The heal logic is correct: (!existing.url || isActionsRunUrl(existing.url)) && itemUrlFromInput(input) backfills only when the stored value is empty or a known-poisoned job URL, and itemUrlFromInput prevents a job URL from ever being written on create or backfill (legacy callers included). The url removal from metadataPatch is preserved. Tests added in the delta cover poisoned-row repair on next enqueue, no-touch of a legitimate stored URL, and never storing a job URL on create or re-enqueue — matching the tests PR 1118 asked for.
• src/lib/pr-fix-queue.test.ts (delta) — Three new tests as above; they pin the write-once invariant with observable enqueue/read assertions.
• src/app/api/pr-followup/{sync,webhook}/route.ts + ingestion (from the earlier commits, unchanged in this delta) — url now always carries the PR URL and the job URL moves to checkRunUrl → "Full log:" feedback evidence, with a legacy-shape fallback. Webhook test covers the PR-without-url → "" case.

Must-check items

• Review migration for data loss risk — Done (above). The migration is a narrow, idempotent-shaped repair; the only overwritten values are rows whose URL is already known-bad, and the replacement is derivable from repo + pr per the issue.
• Test migration on a copy of production schema — The CI "Database migrations" check passed on the head SHA, confirming the migration applies cleanly. The PR body additionally claims a manual Postgres verification ("only the job-URL row changed; API URLs, web /pull/ URLs and NULLs are untouched"); this is self-reported and I could not independently reproduce it, but it is consistent with the regex semantics I verified against the diff.

Standards Compliance

• Lint/typecheck block CI and must pass (AGENTS.md): CI shows Typecheck and Lint success on the head commit — satisfied.
• Tokens/secrets never logged or persisted: no secrets or token material appear in the diff — satisfied.
• Prisma conventions: the migration follows the existing prisma/migrations/<timestamp>_<name>/migration.sql pattern used by 27 prior migrations; no schema change, so no schema.prisma drift.
• Validation before DB operations: enqueue input validation is unchanged; the migration's repo/pr values come from the database and the host in the constructed URL is fixed, so this is data-integrity only, not an injection or SSRF surface.

Linked Issue Fit

• PR 1098 (keep the item URL as the PR URL when CI evidence re-enqueues): fully implemented — write-once guard, job URL relocated to evidence/feedback, and the issue's named acceptance test (review enqueue then CI-failure enqueue; URL stays the PR URL, job URL in evidence) exists in pr-fix-queue.test.ts.
• PR 1118 (repair already-poisoned rows): implemented as a one-time SQL migration plus the enqueue-time heal, with the requested tests for poisoned-row repair and no-touch of legit URLs. The issue suggested "a script or backfill on the update path"; a migration is a reasonable equivalent for a one-time normalization and is consistent with how this repo ships data repairs (cf. 20260516010000_reconcile_lane_classification, 20260824000000_repair_current_lane_indexes).

Tool Harness Findings

8 tool calls ran. repo_contents for the new migration file and the direct gh_api contents fetch both failed (404/preflight), but the migration's full content was verified via the PR files API patch and the incremental delta, which agree byte-for-byte. pr-fix-queue.ts and .github/ai-review-rules.md were read successfully; PR PR 1117 metadata, the files list, and issues PR 1098/PR 1118 were fetched successfully. The per-rule standards file confirms the review tone policy (flag only real defects; prefer approve for reasonable PRs).

Unknowns or Needs Verification

• The manual Postgres verification of the migration's row selection is self-reported in the PR body; the CI migration check confirms applicability but not row-level selection on production data. If the deployment's poisoned rows include the legacy /runs/<id> shape (see minor finding), they will remain unhealed — worth a quick production query (SELECT count(*) FROM "PrFixQueueItem" WHERE "url" ~ '^https://github\.com/[^/]+/[^/]+/runs/') before or after deploy.
• Downstream consumers of pullRequest.url receive either API-format or web-format PR URLs depending on row history (the migration writes API format; older legitimate rows may hold web format). Both resolve to the same PR and the codebase already handles both shapes in tests, but no corpus evidence pins every consumer.

Sources: PR PR 1117 diff and files API (head ad33042b), issues PR 1098 and PR 1118, .github/ai-review-rules.md, AGENTS.md, CI check results for ad33042b, repository migration history.

[its-saffron]

Minor: Both the migration's WHERE regex and isActionsRunUrl only match the /actions/runs/ shape, so a row poisoned with the legacy check-run html_url shape (https://github.com///runs/ — the exact shape the old webhook path stored, per this PR's own updated fixture) is neither repaired nor treated as empty by the heal guard.

Automated finding from AI PR review.

[its-saffron]

Info: itemUrlFromInput blocklists only the Actions-run URL shape rather than positively validating the URL matches the item's repo+pr, so any other non-PR URL from an ingestion caller could still become the write-once identity; acceptable given callers are internal, but worth noting.

Automated finding from AI PR review.

[its-saffron]

Info: The job-URL pattern is duplicated between the migration SQL and ACTIONS_RUN_URL in pr-fix-queue.ts with no test asserting the two predicates stay in sync; consider a comment cross-reference on both sides.

Automated finding from AI PR review.