Twinned from https://github.com/cfsh/ninefold/issues/884 by tools/beads/import_github.py's reconcile pass.
Suggestions from ninefold-reviewer's review of #883. None of these blocked that PR — they are the advisory residuals left at CONVERGED. Unclaimed.
1. merge() has no fallback when mergeCommit is unreadable.
tools/pr/merge_pr.py (merge()): sha = (merged.get("mergeCommit") or {}).get("oid", ""). A gh hiccup, or a mergeCommit that has not materialised yet, leaves sha == "" — and the gate then takes the *full-sweep* path at an unusable sha: _gate_worktree("") fails, gate_worker records skipped, and --check-gate reports FAILING for a merge that was fine. Before the F4 fix the sha came from a local git rev-parse origin/main, which effectively always answers; it now comes from a network read with no fallback. sync_main() still computes origin/main on the very next line and its return is discarded — sha = oid or sync_main(number) keeps the authoritative value when it exists and a correct one (in the non-racing case, which is nearly all of them) when it does not.
2. sync_main()'s docstring is now stale.
tools/pr/merge_pr.py:952-954 still says *"Returns the sha origin/main now points at (best-effort; "" if it could not be read) — #728 needs it to name what the UI gate is about to sweep."* Nothing in production reads that return value any more. DEV_PRINCIPLES C6 — present tense, what the code does now. Either wire it back in per (1) or drop the claim and the -> str.
3. _record_empty_gate never steals a provably-stale lock.
The other half of review F7. A lock orphaned by a crashed worker raises FileExistsError forever, so every later empty-selection merge in that worktree prints *"another run in this worktree is mid-sweep"* and skips the ledger write — reporting the opposite of what happened. _acquire_gate_lock steals a dead-pid or 45-minute-old holder precisely because these locks do get orphaned. Fail-safe in outcome (--check-gate reads MISSING, never a false green), which is why it is a follow-up and not a gate item.
4. Test seams in tools/tests/test_merge_pr.py.
- Every gate-level test stubs M.changed_paths, so nothing drives a real before..sha diff end to end — the selection is exercised, the plumbing that feeds it is not (DEV_PRINCIPLES F1/F2). Carried from the 850bf58 pass.
- _record_empty_gate's new non-FileExistsError OSError arm has no case.
- ProbeGateShaTest cannot catch a wrong --json field name: _T1Gh.pr_view returns the whole _pr dict regardless of which fields were requested — the same shape as the #736 lesson recorded in nflib/gh.py's pr_list docstring. (The real gh pr view <n> --json mergeCommit was checked by hand during this review and does return {"oid": "..."}, so the call itself is correct today.)
None.
No comments.