Skip to content

Commit 72b4f90

Browse files
authored
fix(ci): verify regenerated patch & report resolution honestly (#187)
## Why While reviewing the auto-generated patch PRs (#184, #185, #186) they *looked* like failed AI resolutions — identical patch content, body claiming "conflicts were resolved with AI". Investigation showed the opposite: the existing patches applied **cleanly**, the AI step was **skipped**, and the byte-identical output is correct (every source file the patches touch is unchanged between the old and new Node releases). The real defects were **visibility and missing guards**, not the resolver: - The PR body *always* said conflicts "were resolved with `actions/ai-inference@v2`", even when AI never ran — making legitimate no-op version bumps look bogus. - A quota-exhausted / rate-limited AI response (empty body) surfaced as a vague `SEARCH block not found` instead of a clear diagnosis. - A fully-upstreamed patch could regenerate to an empty diff and ship an unpatched build. ## Changes `.github/workflows/patch-node.yml` - **Honest PR body** — reports whether AI actually ran vs. clean apply, whether the patch changed vs. is an identical carry-forward, and the sanity-check result. Uses a random heredoc delimiter so a body line can't terminate the env block early. - **Empty-diff abort** — if the regenerated patch is zero bytes (e.g. every hunk landed upstream), abort instead of shipping an unpatched build. - **"Sanity-check generated patch" step** — re-applies the generated diff to a pristine tree. This is a *regression guard on the diff-generation plumbing* (`--src-prefix`/`-p1`), **not** proof of semantic correctness: any tree round-trips through `git diff`/`git apply`, so only the build (`test-patch.yml`) validates that a resolution is actually right. `.github/scripts/ai_resolver.py` - Empty AI response while conflicts are outstanding now fails with an explicit **"GitHub Models quota exhausted or rate-limited"** message and exits 1, so PR creation is skipped. ## Verification - `python3 -m py_compile` + YAML parse pass. - Empty-response guard tested locally: exits 1 with the quota diagnostic. - Existing safe-abort behaviour (incomplete AI resolution → PR skipped) is unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent d183913 commit 72b4f90

2 files changed

Lines changed: 89 additions & 13 deletions

File tree

.github/scripts/ai_resolver.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,20 @@ def cmd_apply(node_dir: str, response_path: str) -> None:
288288
initial_rejects = find_reject_files(node_dir)
289289
total = len(initial_rejects)
290290

291+
# An empty model response while conflicts are still outstanding almost
292+
# always means the GitHub Models quota is exhausted or the request was
293+
# rate-limited — the action can return a 200 with no content. Fail loudly
294+
# with that diagnosis instead of letting it surface downstream as a vague
295+
# "SEARCH block not found", and never let an empty resolution reach a PR.
296+
if total and not response.strip():
297+
print("❌ AI response is empty — the model returned no content.")
298+
print(" Likely cause: GitHub Models quota exhausted or request rate-limited.")
299+
print("CONFLICTS_RESOLVED=0")
300+
print(f"TOTAL_CONFLICTS={total}")
301+
print("HAS_UNRESOLVED=True")
302+
print(f"FAILED_FILES={' '.join(sorted(os.path.relpath(r[:-4], node_dir) for r in initial_rejects))}")
303+
sys.exit(1)
304+
291305
# Count the hunks the AI must resolve per file BEFORE applying anything.
292306
# Hunks upstream already landed are excluded — the file already carries
293307
# their post-image, so no SEARCH/REPLACE block is expected (or possible)

.github/workflows/patch-node.yml

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -118,36 +118,101 @@ jobs:
118118
run: |
119119
set -eo pipefail
120120
# Python exits non-zero on unresolved conflicts; pipefail+set -e
121-
# ensures the whole step fails so the PR step is skipped.
121+
# ensures the whole step fails so the PR step is skipped. The summary
122+
# is saved to a temp file that the verify step folds into the PR body.
122123
python3 .github/scripts/ai_resolver.py apply ../node "${{ steps.ai.outputs.response-file }}" \
123124
| tee "$RUNNER_TEMP/resolution_output.txt"
124-
{
125-
echo "RESOLUTION_OUTPUT<<EOF"
126-
cat "$RUNNER_TEMP/resolution_output.txt"
127-
echo "EOF"
128-
} >> "$GITHUB_ENV"
129125
130126
- name: Generate new patch & update patches.json
131127
run: |
132128
set -e
133129
130+
NEW_PATCH="patches/node.v${{ inputs.nodeVersion }}.cpp.patch"
131+
134132
cd ../node
135133
git add -A
136134
git diff --staged --src-prefix=node/ --dst-prefix=node/ \
137-
> "../pkg-fetch/patches/node.v${{ inputs.nodeVersion }}.cpp.patch"
135+
> "../pkg-fetch/$NEW_PATCH"
138136
139137
cd ../pkg-fetch
138+
139+
# A zero-byte diff means the resolved tree carries no change at all —
140+
# the patch would be a no-op and the build would be unpatched. This
141+
# often means every hunk has landed upstream and the patch is no
142+
# longer needed; either way a human should look, so never open a PR.
143+
if [ ! -s "$NEW_PATCH" ]; then
144+
echo "❌ Regenerated patch is empty — the resolved tree produced no diff."
145+
echo " Every hunk may have landed upstream in v${{ inputs.nodeVersion }}; investigate manually. Aborting."
146+
exit 1
147+
fi
148+
149+
# Record whether the regenerated patch is byte-identical to the source
150+
# patch. That is legitimate (the touched source files were unchanged
151+
# between releases) but worth surfacing in the PR body so a no-op
152+
# carry-forward is not mistaken for a failed resolution.
153+
if [ -f "$PATCH_FILE" ] && cmp -s "$PATCH_FILE" "$NEW_PATCH"; then
154+
echo "PATCH_CHANGED=false" >> "$GITHUB_ENV"
155+
else
156+
echo "PATCH_CHANGED=true" >> "$GITHUB_ENV"
157+
fi
158+
echo "NEW_PATCH=$NEW_PATCH" >> "$GITHUB_ENV"
159+
140160
# Replace the old version entry with the new one. If a `patchFile`
141161
# input was supplied, $PATCH_VERSION refers to that source patch.
142162
sed -i \
143163
"s/\"v$PATCH_VERSION\": \[\"node.v$PATCH_VERSION.cpp.patch\"\]/\"v${{ inputs.nodeVersion }}\": \[\"node.v${{ inputs.nodeVersion }}.cpp.patch\"\]/" \
144164
patches/patches.json
145165
146166
# Remove the old patch file unless we just overwrote it in place.
147-
if [ "$PATCH_FILE" != "patches/node.v${{ inputs.nodeVersion }}.cpp.patch" ]; then
167+
if [ "$PATCH_FILE" != "$NEW_PATCH" ]; then
148168
rm -f "$PATCH_FILE"
149169
fi
150170
171+
- name: Sanity-check generated patch & summarize
172+
run: |
173+
set -e
174+
175+
# Regression guard on the diff-generation flags above: re-apply the
176+
# generated patch to a pristine v${{ inputs.nodeVersion }} tree. This is a cheap check that
177+
# the `--src-prefix`/`-p1` plumbing still lines up — it does NOT prove
178+
# the resolution is semantically correct (any tree round-trips through
179+
# `git diff`/`git apply`); only the build (test-patch.yml) does that.
180+
cd ../node
181+
git reset --hard HEAD >/dev/null
182+
git clean -fd >/dev/null # drop leftover .rej/.orig
183+
if git apply --check "../pkg-fetch/$NEW_PATCH"; then
184+
VERIFY="✅ Generated patch re-applies to a pristine v${{ inputs.nodeVersion }} tree (build still validates semantics)."
185+
else
186+
echo "❌ Generated patch does NOT re-apply to a pristine v${{ inputs.nodeVersion }} tree — diff-generation plumbing is broken. Aborting before PR."
187+
exit 1
188+
fi
189+
cd ../pkg-fetch
190+
191+
# Compose an honest PR body. Never claim the AI ran when it did not.
192+
if [ "${{ steps.apply.outputs.needs_ai }}" = "true" ]; then
193+
MODE="🤖 Conflicts were resolved with \`actions/ai-inference@v2\` (GitHub Models)."
194+
if [ -f "$RUNNER_TEMP/resolution_output.txt" ]; then
195+
MODE="$MODE"$'\n\n'"$(cat "$RUNNER_TEMP/resolution_output.txt")"
196+
fi
197+
else
198+
MODE="✅ The existing v$PATCH_VERSION patch applied cleanly to v${{ inputs.nodeVersion }} — no AI resolution was needed."
199+
fi
200+
201+
if [ "$PATCH_CHANGED" = "false" ]; then
202+
CHANGED="ℹ️ The regenerated patch is byte-identical to v$PATCH_VERSION — the source files it touches did not change between the two releases."
203+
else
204+
CHANGED="ℹ️ The patch was updated to match the v${{ inputs.nodeVersion }} source tree."
205+
fi
206+
207+
# Random delimiter so a body line that happens to equal the marker
208+
# can't terminate the heredoc early.
209+
DELIM="RESOLUTION_EOF_$(uuidgen)"
210+
{
211+
echo "RESOLUTION_OUTPUT<<$DELIM"
212+
printf '%s\n\n%s\n\n%s\n' "$MODE" "$CHANGED" "$VERIFY"
213+
echo "$DELIM"
214+
} >> "$GITHUB_ENV"
215+
151216
- name: Create Pull Request
152217
uses: peter-evans/create-pull-request@v8
153218
with:
@@ -157,12 +222,9 @@ jobs:
157222
body: |
158223
## Node.js Patch Update to v${{ inputs.nodeVersion }}
159224
160-
This PR updates the Node.js patch to version ${{ inputs.nodeVersion }}.
161-
162-
Patch conflicts (if any) were resolved with `actions/ai-inference@v2`
163-
using GitHub Models — no third-party API key required.
225+
This PR updates the Node.js patch from v${{ env.PATCH_VERSION }} to v${{ inputs.nodeVersion }}.
164226
165-
${{ env.RESOLUTION_OUTPUT && '### AI Resolution Details' || '' }}
227+
### Resolution
166228
${{ env.RESOLUTION_OUTPUT }}
167229
branch: "nodejs-v${{ inputs.nodeVersion }}"
168230
base: "main"

0 commit comments

Comments
 (0)