Repository navigation
[finding] plugins/uberdev/lib/review-consolidate.sh — 3 findings #771
Description
@TheFJK — /premerge flagged a blocker finding on PR #768.
Origin: 2c6db44177351a84156c12e838aceecd76f8713b
Agent: premerge.review.code_review
File: plugins/uberdev/lib/review-consolidate.sh:175
Severity: blocker (Phase premerge)
Disposition: DEFERRED (premerge-repair-budget-exhausted)
Tier: BLOCKER
Blocks: #768
Findings (3)
1. plugins/uberdev/lib/review-consolidate.sh:175 — blocker / BLOCKER / DEFERRED (premerge-repair-budget-exhausted) — review_consolidate_survivors still ends with jq ... 2>/dev/null || printf '[]', mapping a crashed producer to a definite
Lenses: premerge.review.code_review
The three lines above it were rewritten so rc 2 means UNKNOWN and never 'none', but the final jq is untouched. A truncated or half-written candidates.json makes that jq exit non-zero; stderr is discarded, [] is printed, and the function returns 0. Phase 0 then reads 'no survivors' as a fact and the /premerge run reports there are no PRs to pack - every open PR silently dropped from the stack, with no diagnostic anywhere. Same defect class, same function, one line further down.
2. plugins/uberdev/lib/review-consolidate.sh:729 — blocker / BLOCKER / DEFERRED (premerge-repair-budget-exhausted) — The rewritten staged-set guard reads git diff --cached --name-only without --no-renames, so a staged rename hides the de
Lenses: premerge.review.code_review
Verified locally: with a.txt renamed to b.txt and staged, git diff --cached --name-only prints only b.txt, while --no-renames prints both. Guard asserts staged is a subset of allowed. If anything between the conflict capture and review_consolidate_continue stages a delete of an unconflicted X plus an add of a conflicted Y with similar content, rename detection collapses the pair to Y alone, comm -13 finds no extra, and git merge --continue commits the deletion of X into the stack. The same PR adds --no-renames to all three premerge scope guards citing this exact defect; the twin guard it rewrites in the same diff keeps the bypassable form.
3. plugins/uberdev/lib/review-consolidate.sh:730 — blocker / BLOCKER / DEFERRED (premerge-repair-budget-exhausted) — The staged half is C-quoted by git's default core.quotePath while the allowed half's conflict list arrives NUL-delimited
Lenses: premerge.review.code_review
conflicts-<N>.txt is produced from git status --porcelain -z (raw bytes, no quoting) and read back with tr. staged_raw comes from git diff --cached --name-only, which with default core.quotePath=true renders a non-ASCII name in octal-escaped quoted form. For a conflicted path with any non-ASCII byte the two forms never match, comm -13 names it as extra, review_consolidate_continue returns 2, and the candidate is abandoned from the stack even though its staged set was exactly right. The same PR adds GIT_CONFIG_COUNT/KEY_0/VALUE_0 core.quotePath=false to all three premerge scope guards citing precisely this; this guard, rewritten in the same diff for the same byte-equality reason, does not get it.
To resolve: address the findings above in code and close this issue. Closing resolves exactly the findings listed in this body and the findings added by a later comment on it — one index records both, and closing resolves that recorded set and nothing else. A finding in this file that this issue never recorded is filed as a NEW issue rather than suppressed, so closing never mutes the file. Before closing, apply finding:true-positive if it was a real defect or finding:false-positive if it was not — that label is the eval ground truth (RFC 0018).
Activity