fix: annotate deleted files + retry on transient upstream errors#29
Merged
fix: annotate deleted files + retry on transient upstream errors#29
Conversation
Two related fixes that surfaced during the data-pathways v2.3.7 release (see flowcore-io/data-pathways#89), where the validator repeatedly tried to read files that had been deleted in the PR and eventually tripped the OpenCode agent's loop detector. 1. `scripts/validate.sh` — `generate_diff_summary` now emits a `Status` field per file (ADDED / MODIFIED / DELETED / RENAMED / COPIED / TYPE-CHANGED) derived from `git diff --name-status`. DELETED entries get an explicit "do NOT attempt to read" note with a pointer to `git show HEAD^:path` for inspecting prior contents. Modified-range extraction is skipped for deleted files. Rename detection handles the `old => new` numstat notation so status lookup matches. 2. `scripts/validate.sh` — `run_opencode` and `run_gemini` now retry on a broader set of transient upstream signals: 504, 530, "provider returned error", "unmapped", ECONNRESET, EAI_AGAIN, "socket hang up", "deadline exceeded", plus the existing 429/503/timeout/rate-limit set. Previously these caused a single-shot failure. Grep is now case-insensitive. 3. `system-prompt.md` — adds a "DELETED files — STOP condition" section that explicitly tells the LLM to trust the Status annotation, never retry bare reads on deleted paths, and use `git show origin/<BASE>:path` if it actually needs the former contents. Updates the example workflow to include a DELETED row so the pattern is concrete. Tested locally against the data-pathways v2.3.6→v2.3.7 diff: both deleted handler files are now annotated `Status: DELETED` with the read-warning note, and modification-only diffs still render cleanly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
generate_diff_summarynow annotates every file with a Status field (ADDED / MODIFIED / DELETED / RENAMED / COPIED / TYPE-CHANGED) derived fromgit diff --name-status. DELETED entries get an explicit "do NOT attempt to read" note and skip modified-range extraction.run_opencode/run_gemininow retry on a broader set of transient upstream signals: 504, 530, "provider returned error", "unmapped", ECONNRESET, EAI_AGAIN, "socket hang up", "deadline exceeded" (in addition to the existing 429/503/timeout/rate-limit). Grep is now case-insensitive.system-prompt.mdadds a "DELETED files — STOP condition" section that tells the LLM to trust the Status annotation and NEVER retry bare reads on deleted paths. Example workflow updated to include a DELETED row.Context
Surfaced during the data-pathways v2.3.7 release (flowcore-io/data-pathways#89). A PR that deleted two handler files caused the validator to:
File not foundwhen it tried toReadthe deleted paths (the diff summary listed them as normal changed files).permission requested: doom_loop (read); auto-rejecting.security-standardsjob, OpenRouter returned 504 / 530 upstream errors that weren't in the retry pattern, so the validator failed single-shot instead of retrying.Between the two bugs, that PR's validation had to be manually rerun 4 times before it passed.
Test plan
generate_diff_summaryagainstv2.3.6..v2.3.7on data-pathways correctly annotates both deleted handler files asStatus: DELETEDwith the read-warning note.v2.3.5..v2.3.6) still renders cleanly withStatus: MODIFIED.bash -n scripts/validate.sh).🤖 Generated with Claude Code