ci(pr): skip pr-lockfile artifact download when policy didn't regenerate it (BLO-21118)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work.
• The CI suite is the safety net for every PR entering the merge queue.
• Every job that could consume a regenerated pnpm-lock.yaml unconditionally ran actions/download-artifact@v4 for pr-lockfile, even on the overwhelming majority of PRs that never touch a manifest and so never trigger an upload.
• continue-on-error: true keeps the missing artifact from failing the job, but the download step still emits an "Artifact not found for name: pr-lockfile" annotation on every one of those runs.
• That annotation reads as failure signal when triaging CI, and was flagged twice (BLO-18991, then BLO-21118) as noise polluting real flake investigations.
• The policy job already exposes outputs.lockfile_regenerated; gating each download step on it removes the false-signal annotation with no behavior change on the manifest-changing path.

Linked Issues or Issue Description

BLO-21118 — CI: three distinct server-shard flakes fail unrelated PRs, successor to the BLO-18991 deadlock. This PR resolves the pr-lockfile artifact-noise finding called out in that issue's acceptance criteria.

What Changed

• Added if: needs.policy.outputs.lockfile_regenerated == '1' to the "Restore regenerated PR lockfile (if policy uploaded one)" step in all 7 jobs that reference it: typecheck_release_registry, worktree_install, general_tests, build, verify_serialized_server, canary_dry_run, e2e.
• No change to behavior when the policy job did regenerate the lockfile (manifest-touching PRs) — the download still runs and still restores it.

Verification

• .github/workflows/pr.yml parses as valid YAML.
• No existing workflow-wiring test (scripts/__tests__/*.test.mjs) references these download steps or the lockfile_regenerated output, so nothing exercises this path outside the workflow itself — CI running this PR is the verification.
• Confirmed via gh api .../check-runs/{id}/annotations across recent failed pr.yml runs that "Artifact not found for name: pr-lockfile" is currently appearing regularly (non-manifest PRs are the overwhelming majority), which this removes.

Risks

Low risk, CI-only change. Worst case if the condition is ever wrong: the download step is skipped when it shouldn't be, which only matters on manifest-changing PRs and would surface immediately as a pnpm install --frozen-lockfile failure in that job (loud, not silent).

Model Used

Claude Sonnet 5 (claude-sonnet-5[1m]), agentic coding session with repository inspection and CI log analysis.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either linked an existing issue or described the issue in-PR
• I have considered and documented risks above

[allyblockcast]

🔗 Paperclip issue: BLO-18991
🔗 Paperclip issue: BLO-21118

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 2533dc6

Looks good. The seven consumers all declare needs: policy, and the new condition matches both the policy job's explicit 1/0 output and the artifact upload predicate. This removes expected missing-artifact noise without changing the manifest-regeneration path.

Critical Issues (0)

Important Issues (0)

Suggestions (1)

• [tests] .github/workflows/pr.yml:221 — This non-manifest PR exercises the regenerated=0 skip path; a future manifest-changing PR or merge-group run should confirm the regenerated=1 upload/download path end to end.

Strengths

• The condition is applied consistently to all seven artifact consumers.
• Failed or cancelled policy jobs still prevent dependent jobs from running, so the change does not weaken failure propagation.
• No untrusted expression interpolation or new security boundary is introduced.

Recommended Action

1. Merge after required CI completes.

This PR is authored by app/allyblockcast, so the App cannot formally approve its own PR. The exact head must be reopened under an independent author before an App approval is possible; the shared User token is not substituted for the App review.

[allyblockcast]

Thanks for the review. Responding to the one suggestion:

This non-manifest PR exercises the regenerated=0 skip path; a future manifest-changing PR or merge-group run should confirm the regenerated=1 upload/download path end to end.

Agreed this path deserves explicit confirmation, but it's not something this PR can exercise on its own diff (this PR doesn't touch a manifest, so it can't self-trigger regenerated=1) and it's not a defect in the change as shipped — the if: condition is a pure narrowing of an existing unconditional download, so the regenerated=1 branch is unchanged from current behavior. I'm not pushing a commit for it. Leaving a note on BLO-21118 to watch the next manifest-touching PR's pr.yml run and confirm the restore step still fires and populates the lockfile correctly — that's the real verification for the regenerated=1 path and doesn't require any code change here.

No critical/important findings, all 20 checks green. Leaving merge/approval to a human or independent reviewer per the note above about self-approval.

[kkroo]

Superseded by #1104, which carries the exact same head under independent PR author kkroo so Ally can provide the required App approval.