Label integration PRs when snapshot auto-recovery updates files#605
Closed
Copilot wants to merge 6 commits into
Closed
Label integration PRs when snapshot auto-recovery updates files#605Copilot wants to merge 6 commits into
Copilot wants to merge 6 commits into
Conversation
Agent-Logs-Url: https://github.com/rockcarver/frodo-cli/sessions/8f69d39c-9bb8-4dd7-97ac-de057fae5ee7 Co-authored-by: vscheuber <[email protected]>
…ling Agent-Logs-Url: https://github.com/rockcarver/frodo-cli/sessions/8f69d39c-9bb8-4dd7-97ac-de057fae5ee7 Co-authored-by: vscheuber <[email protected]>
Agent-Logs-Url: https://github.com/rockcarver/frodo-cli/sessions/8f69d39c-9bb8-4dd7-97ac-de057fae5ee7 Co-authored-by: vscheuber <[email protected]>
Agent-Logs-Url: https://github.com/rockcarver/frodo-cli/sessions/8f69d39c-9bb8-4dd7-97ac-de057fae5ee7 Co-authored-by: vscheuber <[email protected]>
Agent-Logs-Url: https://github.com/rockcarver/frodo-cli/sessions/8f69d39c-9bb8-4dd7-97ac-de057fae5ee7 Co-authored-by: vscheuber <[email protected]>
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
Contributor
|
Superseded by #606 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
.github/workflows/integration-batch.ymlin theCreate or update PR integration -> mainstep.integration-needs-snapshot-reviewsnapshot_updates.files).Behavior preserved
Docs
integration-needs-snapshot-reviewtodocs/INTEGRATION.mdlabel semantics.Original prompt
Create a PR in
rockcarver/frodo-clito label the generated integration PR when snapshot auto-recovery occurred.Context:
Requirements:
integration-needs-snapshot-review..github/workflows/integration-batch.ymlso 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.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
frodoexists before running any targetednpm run test:update <pattern>.What the PR will change
In the integration batching automation (the script that runs snapshot refreshes), only when snapshot patterns exist, it will run:
npm cinpm run build:onlynpm i -gfrodois 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:
details_urlwhich points to the Actions job UISo 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 differlintThen selection logic can say:
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.