PR #407 follow-ups: off-main report prose, gate path resolution, comm collation nf-liin ← Beads

closed priority 2 task unassigned activesession:a

Twinned from https://github.com/cfsh/ninefold/issues/412 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #407 (result=CONVERGED at dbb110f). None of these blocked the merge; they are the residue left after F1/F2/F3 and four earlier residuals were all fixed on the branch.

1. The off-main NEW block corrects its banner but not the prose under it

tools/pr/probe_gate.sh:241–248 correctly swaps the headline to ████ FAILING HERE, PASSING IN MAIN'S BASELINE ████ when record_ok=0. But :250–257 then prints unconditionally:

Run from f5/integration during a genuine divergence — the exact scenario #387 documents — that hands you a one-keystroke public issue asserting that main regressed at an f5 sha. F1's data-integrity half is properly closed (the baseline can no longer be poisoned); this is the leftover copy. Gate the prose on record_ok the same way the banner already is, and derive the title's branch name from $here.

2. merge_pr.sh resolves the gate by a CWD-relative path

tools/pr/merge_pr.sh:117 tests [ ! -f tools/pr/probe_gate.sh ] and :122 runs bash tools/pr/probe_gate.sh. merge_pr.sh never cds to the repo toplevel and doesn't otherwise need to — every other command in it is gh or git, both CWD-agnostic. Invoked from anywhere but the repo root, the gate silently no-ops with note: tools/pr/probe_gate.sh missing — UI gate not run, which reads as "the file is absent" rather than "you are in the wrong directory" — a quieter version of the "nothing runs it" failure #389 exists to end.

"$(dirname "$0")/probe_gate.sh" for both the test and the invocation closes it.

3. LC_ALL=C is applied to sort but not to comm

The fix at probe_gate.sh:172 and :188 makes both input streams codepoint-ordered, which is right. But GNU comm compares lines with xmemcoll (locale collation) rather than memcmp whenever LC_COLLATE is not C, so :179–181 and :189–190 still *merge* two C-sorted streams under locale collation. That reopens the same dial-reset / dialogue mis-diff the comment at :167–171 describes as closed, and it would also produce a spurious comm: file 1 is not in sorted order on stderr.

Simplest fix: set LC_ALL=C once near the top of the script rather than per-command.

4. --dry-run and the REPORT ONLY line

--dry-run does not clear record_ok, so the REPORT ONLY — this sweep will not be recorded line at :203–208 never prints on a dry run even though nothing will be recorded. Conversely, when record_ok=0 because of the branch check at :97–99, that line says *"(reason above)"* — but the branch check is silent, and the reason is only printed below, at :282. Cosmetic, but it is the one line whose job is to tell you how to read the table directly under it.

5. probe_gate.sh has no regression guard of its own (declined on #407, recorded here)

The seven scenarios in #407's test plan were driven against a throwaway repo and nothing is committed. Scenario 7 in particular — a failed dotnet build reporting every standing red as ✓ FIXED, a false all-clear from the thing meant to prevent one — is the costly one to re-break silently. The #407 reply declined this as out of scope, correctly: the repo has no shell-test harness today and adding one is a bigger call than that PR should make. Parked here rather than lost.

Dependencies

None.

Comments

No comments.

Add a comment