Automate integration-batch conflict recovery (union stub merges, snapshot refresh, lockfile regeneration)#599
Automate integration-batch conflict recovery (union stub merges, snapshot refresh, lockfile regeneration)#599
Conversation
Agent-Logs-Url: https://github.com/rockcarver/frodo-cli/sessions/ae77f243-0d6e-4e1b-950b-33899ada24d6 Co-authored-by: vscheuber <28791378+vscheuber@users.noreply.github.com>
Agent-Logs-Url: https://github.com/rockcarver/frodo-cli/sessions/ae77f243-0d6e-4e1b-950b-33899ada24d6 Co-authored-by: vscheuber <28791378+vscheuber@users.noreply.github.com>
Agent-Logs-Url: https://github.com/rockcarver/frodo-cli/sessions/ae77f243-0d6e-4e1b-950b-33899ada24d6 Co-authored-by: vscheuber <28791378+vscheuber@users.noreply.github.com>
Agent-Logs-Url: https://github.com/rockcarver/frodo-cli/sessions/ae77f243-0d6e-4e1b-950b-33899ada24d6 Co-authored-by: vscheuber <28791378+vscheuber@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the integration batching workflow to use a dedicated merge/conflict-recovery script, aiming to automatically resolve the most common conflict cases (stub command unions, snapshot updates, and lockfile regeneration) to keep batching moving reliably.
Changes:
- Added
.github/integration/scripts/merge_prs.shto perform merges and attempt deterministic auto-recovery for union/snapshot/lockfile conflicts, emitting a JSON summary. - Updated
integration-batch.ymlto invoke the script and use its JSON output for workflow outputs, PR body content, and labeling/commenting. - Documented the new auto-recovery behaviors and added empty allowlist/blocklist override files under
.github/integration/.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
docs/INTEGRATION.md |
Documents merge auto-recovery behavior (union, snapshots, lockfile regeneration) for the batching workflow. |
.github/workflows/integration-batch.yml |
Replaces inline merge logic with a scripted merge engine and consumes script JSON outputs for downstream steps. |
.github/integration/union-blocklist.txt |
Adds an empty manual blocklist file for union-merge auto-resolution overrides. |
.github/integration/union-allowlist-extra.txt |
Adds an empty manual allowlist extension file for union-merge auto-resolution overrides. |
.github/integration/scripts/merge_prs.sh |
Implements the merge engine, conflict classification, auto-resolution, snapshot refresh, lockfile regeneration, and JSON summary emission. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for pr in $(echo "$SELECTED_JSON" | jq -r '.[].number'); do | ||
| title="$(echo "$SELECTED_JSON" | jq -r ".[] | select(.number==$pr) | .title")" | ||
| git fetch origin "pull/$pr/head:pr-$pr" | ||
|
|
||
| if git merge --no-ff "pr-$pr" -m "Merge PR #$pr: $title"; then | ||
| merged="$(echo "$merged" | jq --argjson n "$pr" --arg t "$title" '. + [{"number":$n,"title":$t}]')" | ||
| continue | ||
| fi |
There was a problem hiding this comment.
merge_prs.sh is invoked via command substitution in the workflow and expected to emit only compact JSON on stdout, but this script runs git/npm commands that write progress/conflict output to stdout as well. That will pollute the captured summary and make jq -c . fail in the workflow. Redirect all non-JSON logging to stderr (or run git/npm in quiet mode), and ensure stdout prints exactly one JSON line (e.g., by sending JSON to a dedicated FD and exec 1>&2 for logs).
| fi | ||
| for pattern in "${deduped_patterns[@]}"; do | ||
| [ -z "$pattern" ] && continue | ||
| npm run test:update "$pattern" |
There was a problem hiding this comment.
npm run test:update "$pattern" will not reliably pass $pattern through to Jest; npm run requires -- to forward positional args to the underlying command. As written, the snapshot refresh is likely to update all snapshots (or ignore the pattern), which defeats the “targeted update” behavior described in the PR. Use npm run test:update -- "$pattern" (and ensure the Jest invocation uses the arg as a test path/pattern).
| npm run test:update "$pattern" | |
| npm run test:update -- "$pattern" |
| if [ -s "$snapshot_patterns_file" ]; then | ||
| mapfile -t deduped_patterns < <(sort -u "$snapshot_patterns_file") | ||
| if [ "${#deduped_patterns[@]}" -gt 0 ] && [ "$npm_ci_done" = 'false' ]; then | ||
| npm ci | ||
| npm_ci_done='true' | ||
| fi | ||
| for pattern in "${deduped_patterns[@]}"; do | ||
| [ -z "$pattern" ] && continue | ||
| npm run test:update "$pattern" | ||
| snapshot_patterns="$(echo "$snapshot_patterns" | jq --arg p "$pattern" '. + [$p]')" | ||
| done | ||
|
|
||
| mapfile -t snapshot_files_changed_arr < <( | ||
| { | ||
| git diff --name-only | ||
| git diff --name-only --cached | ||
| } | sort -u | grep -E '(^|/)(__snapshots__/|.*\.snap$)' || true | ||
| ) | ||
| if [ "${#snapshot_files_changed_arr[@]}" -gt 0 ]; then | ||
| git add -- "${snapshot_files_changed_arr[@]}" | ||
| git commit -m "test: update integration snapshots" | ||
| fi | ||
| fi | ||
|
|
||
| if [ "$lockfile_regen_commit_needed" = 'true' ]; then | ||
| if [ "$npm_ci_done" = 'false' ]; then | ||
| npm ci | ||
| npm_ci_done='true' | ||
| fi | ||
| npm i --package-lock-only |
There was a problem hiding this comment.
The script claims lockfile conflicts should “never fail batching”, but it runs npm ci before lockfile regeneration (both in the snapshot-update block and again in the lockfile-regeneration block). If package-lock.json is conflicted or temporarily inconsistent with merged package.json, npm ci can fail and abort the whole run due to set -e. Regenerate package-lock.json first (or skip npm ci entirely for the lockfile-regeneration path) so lockfile conflicts can’t break the batch.
| summary="$(bash .github/integration/scripts/merge_prs.sh --selected-json "$selected" --dry-run "$dry_run")" | ||
| summary="$(echo "$summary" | jq -c .)" |
There was a problem hiding this comment.
This step captures the script output into summary and immediately parses it as JSON. If the script writes any non-JSON logs to stdout (e.g., from git merge/npm), jq -c . will fail and the workflow will stop. Consider defensively capturing only the last line (or switching to a delimiter-based output from the script) in addition to updating the script to keep stdout JSON-only.
| summary="$(bash .github/integration/scripts/merge_prs.sh --selected-json "$selected" --dry-run "$dry_run")" | |
| summary="$(echo "$summary" | jq -c .)" | |
| summary_output="$(bash .github/integration/scripts/merge_prs.sh --selected-json "$selected" --dry-run "$dry_run")" | |
| summary="$(printf '%s\n' "$summary_output" | awk 'NF { line = $0 } END { print line }')" | |
| summary="$(printf '%s\n' "$summary" | jq -c .)" |
| - union auto-resolution is applied only to an allowlist of `src/cli/**` command files that instantiate `FrodoStubCommand` (excluding files that instantiate `FrodoCommand`), plus manual overrides from: | ||
| - `.github/integration/union-allowlist-extra.txt` | ||
| - `.github/integration/union-blocklist.txt` | ||
| - snapshot conflicts (`*.snap` and `__snapshots__/`) are auto-resolved enough to complete the merge, then targeted snapshot refresh runs via `npm run test:update <pattern>` where `<pattern>` is derived from conflicted snapshot filenames |
There was a problem hiding this comment.
The doc shows npm run test:update <pattern>, but npm run only forwards arguments to the underlying Jest command when they come after --. To reflect the actual invocation (and avoid confusion when running manually), document this as npm run test:update -- <pattern>.
| - snapshot conflicts (`*.snap` and `__snapshots__/`) are auto-resolved enough to complete the merge, then targeted snapshot refresh runs via `npm run test:update <pattern>` where `<pattern>` is derived from conflicted snapshot filenames | |
| - snapshot conflicts (`*.snap` and `__snapshots__/`) are auto-resolved enough to complete the merge, then targeted snapshot refresh runs via `npm run test:update -- <pattern>` where `<pattern>` is derived from conflicted snapshot filenames |
|
Superseded by #600” |
…r-package-lock Integration batch: auto-recover `package-lock.json` conflicts via lockfile regeneration (supersedes #599)
This PR moves integration batching merge/conflict handling into a dedicated script and adds deterministic auto-recovery for the two dominant conflict classes (stub-command files and snapshots), plus unconditional
package-lock.jsonregeneration on lockfile conflicts.Scripted merge engine for integration batching
.github/integration/scripts/merge_prs.shand replaced inline merge logic inintegration-batch.yml.integrationand emits a single-line JSON summary consumed by workflow outputs, PR body sections, and label/comment logic.Union conflict auto-resolution for stub command files
src/cli/**:const program = new FrodoStubCommand(const program = new FrodoCommand(.github/integration/union-allowlist-extra.txt.github/integration/union-blocklist.txt.git/info/attributes+merge.union.driver true.Snapshot conflict recovery with targeted updates
*.snap/__snapshots__/, resolves merge state enough to continue, derives test path patterns from snapshot stems, dedupes, and runs:npm run test:update <pattern>integrationfor focused maintainer review.Lockfile conflict policy: never fail batching
package-lock.jsonis conflicted, merge continues and lockfile is always regenerated with:npm i --package-lock-onlyintegration.Workflow/output/documentation updates
integration-batch.ymlnow consumes script JSON (merged,skipped, snapshot patterns/files, lockfile-regenerated flag) and uses it for summaries + non-dry-run labeling/commenting..github/integration/.docs/INTEGRATION.mdto document union/snapshot/lockfile auto-resolution behavior.Example invocation used by the workflow:
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/repos/rockcarver/frodo-cli/releases/opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --no-warnings --enable-source-maps --experimental-loader file:///home/REDACTED/work/frodo-cli/frodo-cli/dist/loader.cjs /home/REDACTED/work/frodo-cli/frodo-cli/dist/app.cjs shell ash credential.usernnode(http block)/opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --no-warnings --enable-source-maps --experimental-loader file:///home/REDACTED/work/frodo-cli/frodo-cli/dist/loader.cjs /home/REDACTED/work/frodo-cli/frodo-cli/dist/app.cjs shell p/bin/sh commit.gpgsign(http block)/opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --no-warnings --enable-source-maps --experimental-loader file:///home/REDACTED/work/frodo-cli/frodo-cli/dist/loader.cjs /home/REDACTED/work/frodo-cli/frodo-cli/dist/app.cjs shell tnet/tools/sh core.hooksPath(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
Create a PR in
rockcarver/frodo-cliimplementing scripted conflict resolution for the integration batching workflow, including automatic package-lock regeneration.Confirmed decisions:
.github/integration/scripts/so npm/binary packaging doesn't include them.package-lock.jsonconflicts should never fail batching; always auto-recover by regenerating lockfile withnpm i --package-lock-onlyand committing the result onintegration.Implement:
Add
.github/integration/scripts/merge_prs.sh:integration.src/cli/**:const program = new FrodoStubCommand(const program = new FrodoCommand(.github/integration/union-allowlist-extra.txt.github/integration/union-blocklist.txt.git/info/attributesandmerge.union.driver true.**/*.snapor__snapshots__) -> complete merge (resolve snapshot files enough to continue), then run targetednpm run test:update <pattern>where pattern is derived from snapshot filename stem; dedupe patterns.package-lock.jsonis conflicted -> always auto-recover by regenerating usingnpm i --package-lock-only(regardless of other conflict types, as long as remaining conflicts are auto-resolvable) and commit.Add empty override files:
.github/integration/union-allowlist-extra.txt.github/integration/union-blocklist.txtUpdate
.github/workflows/integration-batch.yml:Document briefly:
docs/INTEGRATION.md(or create if missing) to mention new auto-resolution behaviors: union for stub commands, snapshot auto-update, and automatic lockfile regeneration.Deliverable: Open a PR with these changes.
The following is the prior conversation context from the user's chat exploration (may be truncated):
User: Yes, implement your suggestions.
Assistant: I’m blocked from pushing the change via automation in this turn, but the fix is straightforward and you (or I, once tool access is available again) can apply it as a small edit to
.github/workflows/integration-batch.yml.What to change (selection criteria)
Right now the workflow likely does something like:
head_shastate == successThat misses GitHub Actions “checks”. Replace that with check-runs evaluation:
Drop-in replacement for the “Filter to PRs with successful …” step
In that step’s loop (per PR), replace the status call with:
And keep your existing “write outputs as compact JSON” lines, e.g.:
Why this fixes it
The PR UI “green checks” are based on check-runs, not n...
This pull request was created from Copilot chat.