Twinned from https://github.com/cfsh/ninefold/issues/737 by tools/beads/import_github.py's reconcile pass.
Suggestions from ninefold-reviewer's review of #734. Both are below the severity floor for that PR's gate — neither blocked it, and both are small.
---
--reconcile-only reports a failure when the reconcile succeededtools/pr/review_now.py — conclusion()
decide() returns at steps 1–2 (dispatch.py:675-694) before the reconcile_only short-circuit at :695, and sweep_one publishes the check from a finally on every path regardless. So review_now.py <n> --reconcile-only on a rev:off PR, or one where review was never requested, produces:
``` #7: skip (rev:off — reviewer muted for this PR) #7: check -> #7 FAILURE Needs a human approval
ⓘ rev:off — the reviewer is muted for this PR. --force overrides it for this one run.
exit status: 1
```
The check was republished — the only thing the caller asked for — but the summary names neither that nor a next step for it, exits 1, and points at --force. A session that then drops the flag and runs review_now.py <n> --force spends the xhigh run this whole PR exists to avoid, on a PR explicitly muted from review. (--force --reconcile-only together still reconciles rather than spawning, so the advice is inert in the one combination it is printed for — which is its own small confusion.)
This is the mirror of #734's D1, not a sequel to it: D1 was a failure reported as a success; this is a success reported as a failure. The D1 arm ordering is correct and should not move. The fix is for conclusion() to know the mode — conclusion(out, number, reconcile_only=args.reconcile_only) — and under that flag report the republished check as the outcome while still printing the skip reason.
notes/reviewer.md, as #734 leaves it, invites the filing directly: *"If one prints something you cannot act on, that is a bug worth filing."*
Test shape: extend ReconcileOnlyConclusionTest with the two skip lines coexisting with a check line, asserting ok is True and that the message names both facts.
---
--reconcile-onlytools/pr/review_now.py:3 still reads:
``` ./tools/pr/review_now.py <pr> [--force] [--no-self-sync] ```
That line is the first thing a session reading the file sees, and it presents itself as the complete option list. #734's stated aim is making the cheap action visible where an agent will actually see it; the --help epilog, the flag help, notes/reviewer.md and notes/merging.md all carry it — the file's own header does not.
tools/reviewer/README.md:248 has the same gap (../pr/review_now.py <pr> [--force]), and already omitted --no-self-sync before #734.
None.
No comments.