Skip to content

Label integration PRs when snapshot auto-recovery updates files#605

Closed
Copilot wants to merge 6 commits into
mainfrom
copilot/add-integration-snapshot-label
Closed

Label integration PRs when snapshot auto-recovery updates files#605
Copilot wants to merge 6 commits into
mainfrom
copilot/add-integration-snapshot-label

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 20, 2026

Integration PRs already include snapshot update details in the body, but they were missing an at-a-glance signal for reviewers. This change adds an explicit integration-only label when snapshot auto-recovery actually modifies snapshot files.

  • Workflow: apply snapshot-review label to generated integration PR only

    • Updated .github/workflows/integration-batch.yml in the Create or update PR integration -> main step.
    • After create/update, the workflow resolves the integration PR number and conditionally adds:
      • integration-needs-snapshot-review
    • Labeling is gated on non-empty snapshot updates from the existing batch summary JSON (snapshot_updates.files).
  • Behavior preserved

    • No change to candidate PR selection logic.
    • No label changes on source/batched PRs.
    • No label applied when snapshot updates are empty.
    • Existing compact JSON usage/output patterns are preserved.
  • Docs

    • Added integration-needs-snapshot-review to docs/INTEGRATION.md label semantics.
if echo "$snapshot_updates" | jq -e '.files | length > 0' >/dev/null; then
  gh pr edit "$integration_pr_number" --repo "$REPO" \
    --add-label integration-needs-snapshot-review
fi
Original prompt

Create a PR in rockcarver/frodo-cli to label the generated integration PR when snapshot auto-recovery occurred.

Context:

  • Integration PRs (integration -> main) already include sections listing snapshot patterns and snapshot files updated.
  • We want additional visibility via a label.

Requirements:

  1. Label name: integration-needs-snapshot-review.
  2. Apply ONLY to the generated integration PR (e.g., PR like integration #604), not to the original batched PRs.
  3. Update .github/workflows/integration-batch.yml so that after the integration PR is created/updated, if the batch summary indicates snapshot files were updated (non-empty list), add the label to the integration PR.
  4. If no snapshots were updated, do not add the label.
  5. Keep PR selection logic and all other workflow behavior unchanged.
  6. Ensure the workflow uses compact JSON outputs as currently required.

Deliverable: Open a PR implementing this label behavior.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: @copilot Accepted Confirmation: Confirm agent session

Assistant: I’ll open a new PR that fixes the snapshot-update path by ensuring frodo exists before running any targeted npm run test:update <pattern>.

data:
  - title: '[WIP] Fix integration batch snapshot update failures in GitHub Actions'
    repository: rockcarver/frodo-cli
    url: https://github.com/copilot/tasks/pull/PR_kwDOH1LPHs7Tx9HM?session_id=007ea63d-538a-4668-8f60-5ef6be9e8edd
    state: queued

What the PR will change

In the integration batching automation (the script that runs snapshot refreshes), only when snapshot patterns exist, it will run:

  1. npm ci
  2. npm run build:only
  3. npm i -g
  4. Add the global npm bin dir to PATH for subsequent steps (so frodo is discoverable)

Then it will run the existing targeted snapshot update commands.

This directly addresses the failure seen in run 24642018414 (/bin/sh: 1: frodo: not found) without slowing down normal batches that don’t touch snapshots.

Once the task produces a PR, merge it and re-run Integration Batch (dry-run is fine) to confirm snapshot conflicts now auto-heal cleanly.

User: PR is merged and the last dry-run of the integration-batch workflow ran really well: https://github.com/rockcarver/frodo-cli/actions/runs/24644174525

I would like to investigate options to include PRs with failed test runs but only if those failures are snapshot related. Analyze if that is possible and if we can auto-recover by updating the snapshots and have the tests succeed in the integration branch.

We should probably run all the tests against the integration branch after all PRs have been merged anyway to make sure the branch is ready for PR into main.

But let's discuss that idea and decide if that makes sense or if it's better to make the tests succeed in the original PR.

Assistant: It’s possible to include some PRs that have “failed tests” if (and only if) the failure is snapshot-only, and in many cases you can auto-recover on the integration branch by updating snapshots and re-running tests there. The key is: you need a reliable, machine-detectable signal that the PR failure is snapshot-related, otherwise you’ll accidentally batch genuinely broken PRs.

Also, I agree with your second point: running the full test suite on the integration branch after merging is the right safety net regardless of how strict PR preselection is.

Below are the options and tradeoffs, and what I think makes the most sense for frodo-cli.


1) Can we detect “snapshot-only failure” from a PR check run?

What you can do

During PR selection, you can call the Checks API for the PR head SHA (you’re already using check-runs), and for any check run with a failing conclusion, fetch its details and try to classify.

But: GitHub’s Checks API doesn’t inherently tell you “this failed because of snapshots”. You only get:

  • name, status/conclusion
  • a “summary”/“text” field (sometimes)
  • a details_url which points to the Actions job UI
  • annotations (sometimes; often limited)

So to do this robustly, you need one of these:

Best signals (recommended)

A. A dedicated “snapshot validation” check/job
Split CI so snapshot-only failure can be distinguished cleanly, e.g.

  • test (no snapshot updates allowed) — fails if snapshots differ
  • lint
  • etc.

Then selection logic can say:

  • PR is eligible if only the “snapshot validation” job failed and all other required checks are green.

This is the cleanest/most reliable.

B. A label / explicit opt-in
If the PR author (or maintainer) adds something like ok-to-autofix-snapshots, then integration-batch can include it even if tests failed, and attempt snapshot repair. This avoids risky heuristics.

Heuristic signals (possible, but fragile)

C. Parse logs for snapshot mismatch strings
You could fetch the failing job logs (via GitHub API) and look for patterns like:
-...

This pull request was created from Copilot chat.

Copilot AI changed the title [WIP] Add label for integration PRs needing snapshot review Label integration PRs when snapshot auto-recovery updates files Apr 20, 2026
Copilot AI requested a review from vscheuber April 20, 2026 12:44
@vscheuber
Copy link
Copy Markdown
Contributor

Superseded by #606

@vscheuber vscheuber closed this Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants