PR #378 follow-ups: base_ref could ride the head fetch, push-path HEAD, two verdict matchers nf-l511 ← Beads

closed priority 2 task unassigned

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

Suggestions from ninefold-reviewer's review of #378. All advisory — the PR reached result=CONVERGED with these deliberately left out rather than folded back in. None of them break anything.

1. dispatch.sh:111 — the third API call is avoidable, which is the same argument #377 item 3 made. delta_check() fetches .base.ref with its own gh api repos/$repo/pulls/$n. But the only path that reaches it has already run gh pr view "$n" --json headRefOid at :398 to obtain live_head — and baseRefName rides along on that call for free (poll.sh:40 already fetches both together). Adding the field there and passing it in as a 4th arg makes the delta check genuinely two API calls, and the docs this PR just corrected up to "three" would go back to "two", truthfully this time.

2. rebuild_f5.sh:119git push -f origin f5/integration is the one remaining exit that leaves HEAD parked. Same defect class item 2 just closed, and explicitly outside #377's stated scope ("guarding the two verify failures is what's needed"), so this is a note, not a miss. It is also much less likely to bite: a failed push is retried immediately, whereas a failed build is where you wander off. || bail_to_start (with the message reworded off "verify failed") would close it.

3. rebuild_f5.sh:96 — a failed hand-back is near-silent. git checkout -q "$START_REF" && echo "→ Back on …" — if the checkout fails, && short-circuits and the only signal is git's own stderr. An else arm ("⚠ still on f5/integration — check out your branch before editing") makes the foot-gun explicit at exactly the moment the guard failed to prevent it.

4. poll.sh now carries two matchers for the same line. test("(?m)^REVIEWER-VERDICT") at :78 and the #341 split form (split("\n") | map(select(startswith("REVIEWER-VERDICT")))) at :199. Both are correct — I re-measured the new one live against gh's gojq: test("^…";"m")false, test("(?m)^…")true — but a reader of this file now has to hold two idioms for one job. Cosmetic; converging on the split form (or on (?m)) at some later touch is enough.

5. What the (?m) widening also admits, noted so a future format change does not trip on it. Any bot post containing a column-0 REVIEWER-VERDICT line now counts as the newest verdict — including one that merely *quotes* an older verdict. Nothing posted today has that shape (the charter renders quoted verdicts inside ledger table cells, which are not column 0), and the widening is exactly what makes GAPS-REPEAT escalations parse. Recording it so a later ledger-format change gets checked against it.

Dependencies

None.

Comments

No comments.

Add a comment