Add Brew Trust to CLI Distribution · Entire

Review: #1462 — Add entire trail merge subcommand

Verdict: clean, well-structured, and the API-interaction correctness is solid. My main finding is a safety/UX inconsistency with its sibling command, plus a test gap on the orchestration. Approve-with-comments.

✅ What's done well

• resolveTrailBySelector extraction is a genuine dedup — runTrailShow now shares it, and the "never nil trail with nil error" invariant holds across all return paths (callers can rely on a non-nil result).
• Doesn't trust a 2xx — mergeTrailByNumber verifies the server's ok:true before reporting success, mirroring deleteTrailByNumber. Correct and consistent.
• Checks mergeability before merging, and the TOCTOU window (state changing between check and merge) degrades safely — the server re-gates on the merge endpoint, and checkTrailResponse + !res.OK surface that. Good that it doesn't rely solely on the client-side check.
• describeMergeBlockers lists blockers in server-gate order with a defensive fallback for the "not mergeable but no recognized blocker" case.
• Dry-run semantics are correct: the !mergeable gate sits before the dry-run branch, so --dry-run returns a non-zero error on an un-mergeable trail (CI-gatable) and exits 0 with a "mergeable" message otherwise.
• Helper-level test coverage is good (fetch + server-error, merge + ok:false, blocker text across states, summary rendering).

🟡 Main finding — no confirmation prompt, unlike trail delete

entire trail delete prompts for confirmation unless --force is passed ("Deletion is permanent; you are prompted to confirm unless --force is passed"). entire trail merge has no such prompt — a real run merges the branch into base immediately once gates pass.

Merging into the base branch is a comparably consequential, remote-state-changing, not-trivially-reversible operation. The mergeability gate (approvals + CI) limits what can be merged, but it doesn't protect against merging the wrong trail — e.g. running entire trail merge from the wrong branch, or a fat-fingered invocation, merges the current branch's trail with no "Merge trail #N into main? [y/N]". gh pr merge prompts by default for the same reason.

Recommendation: match the delete pattern — interactive confirm on a TTY, --force/--yes to skip for automation (dry-run unaffected). Or, if merge is intentionally prompt-free for the automation-first use case, document that choice. This is the one thing I'd want decided before merge.

🟡 Test gap — runTrailMerge orchestration is untested

The tests cover the four helpers individually (fetchTrailMergeability, mergeTrailByNumber, describeMergeBlockers, printTrailMergeability), but there's no test for runTrailMerge itself — the part with the actual behavior contract:

• --dry-run stops after the check and returns non-zero when not mergeable (the CI-gating promise in the PR body),
• an un-mergeable trail returns an error before attempting the merge,
• the check-then-merge sequencing.

That orchestration is exactly what's most likely to regress (e.g. someone reorders the dry-run branch above the gate). Worth a table test driving runTrailMerge with a fake client across mergeable/not-mergeable × dry-run/real.

🟢 Minor

• On a non-mergeable trail, the summary (with ✗ marks) prints to stdout and an error listing the same blockers goes to stderr — slight redundancy, but the separation is actually helpful for CI (parse stdout, gate on exit code). Fine as-is.

Recommendation

Approve-with-comments, with the confirmation-prompt parity (🟡 #1) as the one real decision, and the runTrailMerge test (🟡 #2) as a should-add.