feat(scripts/pr-comments-mgmt): expand review-thread and PR-write CLI#149
Merged
feat(scripts/pr-comments-mgmt): expand review-thread and PR-write CLI#149
Conversation
Wraps `gh api graphql` to manage PR review threads: * resolve / hide-comment / hide-thread mutations * list-threads pagination over reviewThreads and inner comments * current-pr lookup for the open PR on the current branch Subcommands default to the current repo and PR via gh, validate ReportedContentClassifiers via an allowlist, and surface partial GraphQL errors instead of swallowing them.
Bad3r
added
type(enhancement)
Bad3r
added
type(enhancement)
This comment has been minimized.
This comment has been minimized.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new bash script, pr-comments-mgmt.sh, which provides a suite of tools for managing GitHub pull request review threads and comments using the GraphQL API. Key features include resolving threads, minimizing comments with specific reasons, and listing threads with full pagination support. The review feedback suggests a more robust validation check in the hide_thread function to ensure the target node is indeed a PullRequestReviewThread before attempting to access its comments.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12b0deecf4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Wrap err/log in {level, prog, message} envelopes via a manual escaper
(no jq bootstrap dependency), and route every gh call through `_gh_run`
so gh's own stderr is captured and re-emitted as JSON instead of leaking
plain text. Stdout from list-threads and current-pr remains a single
pretty-printed JSON document; stderr becomes NDJSON. The help menu
(via --help / -h / no-args) is intentionally left as plain text per spec.
Verified via a 30-case test runner (help, bad option/reason, unknown
subcommand, missing/empty/fake ids per subcommand, list-threads with
0/1/2/3 args and bad shapes, current-pr ok/extra/--reason): every
non-empty stdout line parses as one JSON document and every non-empty
stderr line parses as a JSON object.
This comment has been minimized.
This comment has been minimized.
Replaces the loose `type == "object"` check, which only catches the "node id does not exist" case. When an id resolves to a different Node type (issue comment, discussion, etc.), GraphQL returns a populated `data.node` whose inline fragment didn't match, so the old check passed silently and the loop emitted a confusing follow-on error from `resolve_thread`. Asks GraphQL for `__typename` and asserts equality to `PullRequestReviewThread`, surfacing the actual type in the error message.
If GraphQL ever returns `pullRequest: null` without populating the top-level `errors` array, the previous code treated it as success: `page` became `null`, the inner loop iterated zero times, and the accumulator emitted `[]`, making a nonexistent PR indistinguishable from a PR with no review threads. Asserts `data.repository.pullRequest != null` after each page fetch and exits 2 with a contextual error message.
The usage block advertised `current-pr` as printing the *open* PR for the current branch, but `gh pr view` resolves any PR for the branch regardless of state (open/merged/closed). The same ambiguity leaked into `list-threads` no-args, where a closed PR's threads would silently be listed. Both code paths now request the PR `state` from gh and bail out with a contextual error if it is not `OPEN`. The `state` field is stripped from `current-pr` output to keep the documented schema.
`jq -c '.errors // empty'` only triggered the alternative for
`null`/missing — an empty array `[]` flowed through verbatim and
tripped the `[[ -n ${errors} ]]` check, failing the call with
`graphql errors: []`. GitHub doesn't currently emit `errors: []`,
but the new guard rejects the response only when it actually
carries entries.
The pure-bash `_json_string` only handled the named escapes (`\b \f \n \r \t \\ \"`); anything else in U+0000-U+001F flowed through verbatim, producing JSON that strict parsers reject per RFC 8259 §7. The realistic trigger today is captured `gh` stderr that contains ANSI escape sequences leaking into NDJSON consumers. Delegates encoding to `jq -Rn --arg s "$1" '$s'`, which covers the full control-character range plus surrogate-pair edge cases. Adds an explicit early `command -v jq` probe (with a plain-text error) before the trap is installed so a missing `jq` no longer recurses through `err -> _json_string -> jq`. Drops the now-redundant `require_cmd jq` from `main`.
The previous loop walked every thread in the outer page just to find the few whose inner comments were paginated, doing a `jq` sub-shell per index for `length`, the slice, and `hasNextPage`. A single `to_entries[] | select(...) | .key` pass over the page extracts only the indices that actually need follow-up fetches, and the page-info reads at the bottom share one decode. Output is byte-identical against the old loop (verified on PR #149). Trims several jq sub-shells per outer page, which adds up on PRs with many threads.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The `command -v jq` probe at the top of the script intentionally emits plain text instead of NDJSON because `_json_string` itself depends on `jq`. Adds a one-line comment acknowledging the deliberate inconsistency so a strict NDJSON consumer doesn't mistake it for an oversight.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The list-* verbs already covered "fetch + filter" but consumers still
had to pipe to jq for the recurring shapes — "latest review with body",
"last 5 reviews summary line", "last 3 comments full block",
"chronological preview". Move the sort + limit + per-item formatting
into the wrapper so those workflows do not require external jq.
- `--sort newest|oldest`: sort by the per-verb natural timestamp
(`submittedAt` for reviews, `createdAt` for comments, the first
comment's `createdAt` for threads). Implemented as
`sort_by(<expr>) [| reverse]` in the new `_apply_view`, applied
after the per-verb filter pass and before the format pass.
- `--limit N`: positive integer, takes the first N items after `--sort`.
`--limit 0`, negative, and non-numeric values die `1`.
- `--format text`: one short summary line per item, with the relevant
per-verb fields:
- reviews: `[<time>] <author> (<state>) body=<N> chars`
- comments: `[<time>] <author> [minimized:<reason>]? body=<N> chars`
- threads: `[<path>:<line>] <first-author> resolved=.. outdated=.. comments=<N>`
- `--format full`: header + body block per item, with per-verb headers:
- reviews: `=== [<time>] <author> (<state>) ===\n<body>`
- comments: `=== [<time>] <author> [minimized:<reason>]? ===\n<body>`
- threads: `=== [<path>:<line>] <first-author> resolved=.. outdated=.. ===\n<first-body>`
- `_format_array` now takes a kind arg ("threads"|"reviews"|"comments")
to pick the right text/full template; json/ndjson/ids paths are
unchanged. Each list_* function passes its kind.
- SUBCOMMAND_FLAGS extended for the three list verbs to accept `sort`
and `limit`; non-list verbs reject these via `_assert_flags_for`.
Verified against PR #149 that the four real-world workflows (latest
review full, last 5 reviews text, last 3 comments full, all comments
text preview) collapse to single CLI invocations with no external jq.
Edge cases: bad sort/limit values die 1 with the right message,
sort/limit composes with the existing --author / --minimized / etc.
filters, and existing json/ndjson/ids consumers see no change.
This comment has been minimized.
This comment has been minimized.
The recurring "thread audit" query — id-keyed, status flags, total +
unminimized comment counts per row — still required raw `gh api` +
`jq` because the existing `text` format is prose-style and lacks
unminimized counts and id, while `ndjson` is one-document-per-line
with no per-verb column projection. Add a tab-separated tabular
format with per-verb columns so audit/automation work fits awk, cut,
column, and Excel without external jq.
Per-verb columns (no header — leave that to the caller so cut -f<n>
and awk -F'\t' do not have to skip a row):
reviews: id, submittedAt, author, state, body_len, url
comments: id, createdAt, author, isMinimized, minimizedReason,
body_len, url
threads: id, isResolved, isOutdated, path, line, first_author,
comments, unminimized
The threads row mirrors the audit query exactly: id-first, with the
`unminimized` count derived from `comments.nodes | map(select(...)) |
length` so triage can spot threads with unfinished hide-comment work
in a single pass.
Verified against PR #149: tsv emits one row per item across all three
list verbs, composes with the existing --sort / --limit / --author /
--minimized / --unresolved filters, and renders cleanly through
`column -t -s $'\t'` for visual columns. Existing json/ndjson/ids/
text/full consumers see no change.
This comment has been minimized.
This comment has been minimized.
…d; clarify --sort/--limit help
Round 8 of automated review (claude on `92e6f93`) flagged three
items in the new `--sort` / `--limit` / format-dispatch surface that
were nits rather than bugs:
- `_format_text` / `_format_full` / `_format_tsv` silently emitted
nothing when called with a kind not in their `case`, instead of
failing loudly. Today the only callsite is `_format_array`, which
controls the kinds, but a future list verb that forgets to register
in one of the three formatters would silently return empty output
rather than tripping the diagnostic. Add `*) die 1 "_format_*:
unknown kind '\$1'" ;;` arms, matching the same "missing entry fails
loudly" pattern as `_assert_flags_for`.
- `--limit N` help read as if `--sort` were required ("Keep the first
N items after --sort"). The parser actually allows `--limit` alone
and keeps the first N items in cursor-pagination order. Reword to
document the standalone behavior.
- `--sort newest` documented as sorting by the natural timestamp, but
did not call out that `null` timestamps (PENDING reviews) sort to
the tail. So `list-reviews --sort newest --limit 5` never surfaces
a pending review while any submitted review exists. Defensible
semantics, but worth recording so a future reader does not file it
as a bug.
- One inline doc comment on `_format_full` records that the `threads`
arm renders the thread-opener's body only; full reply chains belong
to `get-thread <id>`. This matches the round-8 reviewer's preferred
resolution (document the trim rather than expand `full` into a
thread dump).
Verified end-to-end against PR #149: `list-threads --format=text`,
`list-reviews --format=tsv`, and `list-comments --format=full` all
still produce the expected per-verb shapes.
This comment has been minimized.
This comment has been minimized.
Round 9 nit: the file-level `OUTPUT_FORMAT` state-variable comment listed only the original five formats (`json`/`ndjson`/`ids`/`text`/ `full`); `tsv` was added to the validator, the help block, and `_format_array`'s docstring in `522f8d0` but not here. Sync the docstring so the global is documented identically to the call site.
…nt/reply
Round 9 nit (carry-over from round 8): `set-title` rejects empty
titles locally and `dismiss-review` rejects empty messages, but
`set-body`, `comment`, and `reply` happily round-tripped empty bodies
through `gh`:
- `set-body --body=''` silently cleared the PR description via
`gh pr edit --body ""` — almost certainly not what the caller
meant; if they actually want to clear the body, the bare `gh`
command or the GitHub UI is the right escape hatch.
- `comment --body=''` posted (or got rejected by `gh`'s voice rather
than this CLI's), losing the structured NDJSON error shape.
- `reply --body=''` had the same problem against
`addPullRequestReviewThreadReply`.
Fix: add `[[ -n ${body} ]] || die 1 "<verb>: body cannot be empty"`
guards in each dispatch arm before the API round-trip, matching
`set-title` (1731) and `dismiss-review` (1711). Update the help
block to document the new contract for `reply`, `set-body`, and
`comment`. `review --approve` still permits an empty body
(documented at the existing `review` entry); the others now match
the rest of the file's guard style.
Verified all three diagnostics fire correctly:
$ ./scripts/gh-cli/pr-comments-mgmt.sh --pr 149 set-body --body=''
{"level":"error",...,"message":"set-body: body cannot be empty"}
$ ./scripts/gh-cli/pr-comments-mgmt.sh --pr 149 comment --body=''
{"level":"error",...,"message":"comment: body cannot be empty"}
$ ./scripts/gh-cli/pr-comments-mgmt.sh --pr 149 reply <id> --body=''
{"level":"error",...,"message":"reply: body cannot be empty"}
… -> visible_comments Round 9 nit: the 8th column of `--format=tsv` for `list-threads` counts comments where `isMinimized` is false, i.e. comments currently visible in the GitHub UI. The original name `unminimized` read as either "comments that have been *unminimized* (action history)" or "comments not currently minimized (current state)"; only the latter matches the code. `visible_comments` is unambiguous and matches the spelling readers expect when asking "how many comments would I see if I scrolled through the thread?" Update both the help block and the inline column comment in `_format_tsv` so they match. The TSV output itself is unchanged (no header is emitted), so this is doc-only — no caller has to update parsing.
Owner
Author
Round 9 / 10 follow-upTwo automated review passes landed back-to-back. The round-9 review (claude bot at Accepted
Carried (no action this round)
Carry-over status (rounds 1 - 8)All previously-documented dispositions remain unchanged:
Validation
HousekeepingOlder round-1 - round-6 status / acknowledgement comments and the now-superseded round-9 review have been minimized via Net unresolved threads: 0. Net unminimized superseded comments: 0. |
This comment has been minimized.
This comment has been minimized.
…_comments rename Round 11 nit: the `--format=tsv` example in the help block injects an `awk` header for the threads output, but the 8th-column label still read `unmin` — carrying the same action-history-vs-current- state ambiguity that motivated the `unminimized → visible_comments` rename in `e893871`. Update the example header to `visible` so the documentation is consistent end-to-end. The change is to a user-supplied awk literal embedded in the help example, so no caller's parsing is affected — TSV output still emits no header row.
Contributor
|
Claude finished @Bad3r's task in 1m 13s —— View job Code Review: PR #149 —
|
Owner
Author
Round 11 / 12 finalRound 11 ( Round 12 (
Conversation tab is now compacted: every superseded round-1-through-round-11 status / acknowledgement / review comment and every resolved review thread has been minimized via |
`--format=text` lines are now `<id>\t<rest>` and `--format=full` headers are now `=== <id> <rest> ===`, so the operator can pipe either format through `cut -f1` (text) or grep the headers (full) without re-querying the same items via `--format=ids`. The text and full layouts remain non-contract; downstream parsers should keep using `ndjson` or `tsv`. Validated via shellcheck, shfmt -i 2 -s, and live runs of list-comments / list-reviews / list-threads against PR #149 in text and full formats.
Adds a body-only output mode to list-threads, list-reviews, and list-comments that dumps `.body` per item with no headers or separators (opener body for threads, matching `full`). Best paired with `--limit=1` to read a single item; multi-item runs concatenate without delimiters and should prefer `--format=full` instead. Implementation: new `_format_body` helper, dispatcher case in `_format_array`, and the `--format` validator (both `--format X` and `--format=X` argv forms) extended to accept `body`. text/full /body remain non-contract; downstream parsers should keep using ndjson or tsv. Validated via shellcheck, shfmt -i 2 -s, and live runs of list-comments / list-reviews / list-threads against PR #149, including a negative test that an unknown --format value still errors with the updated allowlist.
…author for list-reviews
Adds two composable filters to list-comments and list-reviews so the
"find older same-author items and hide/dismiss them" workflow can be
expressed as one pipe instead of a hand-rolled GraphQL+jq cycle:
--superseded Keep items where some other item by the same
author has a strictly later timestamp
(createdAt for comments, submittedAt for
reviews). PENDING reviews (no submittedAt)
are excluded from both sides of the
comparison.
--similar-prefix N Modifier on --superseded: also require the
newer same-author item to share the first N
bytes of `.body`. Lets `claude` round-N
status comments be separated from substantive
one-off comments by the same author.
`--superseded` runs after `--author`/`--minimized` inside the
filter helper, so `--minimized=false --superseded` reads as
"from what is still visible, drop the newest per author and keep
the rest" and drives the iterative `hide-comment` loop to zero.
`--similar-prefix` without `--superseded` is rejected at dispatch
time.
Also extends `--author` to list-reviews (already supported on
list-comments and list-threads) so `--superseded --author <login>`
is composable for the review path; a new `_apply_review_filters`
mirrors `_apply_comment_filters` and is wired into `list_reviews`
before `_apply_view`.
Validated via shellcheck, shfmt -i 2 -s, and live runs against
PR #149: superseded counts match (9 claude reviews → 8 superseded;
older `### Code Review: PR #149 — round` claude comments match
under `--similar-prefix 30`); `--minimized=false --superseded`
returns empty after a prior hide round (the iterative-loop
contract); `--similar-prefix` without `--superseded` and bad
integer values are rejected with the expected error messages.
Mirrors `get-thread` for issue-level (top-level) PR comments. Takes
exactly one comment node id (e.g. `IC_kwDO...`) and emits a single
JSON object with the same field shape as one element of
`list-comments` (id, databaseId, author, body, createdAt, updatedAt,
url, isMinimized, minimizedReason, viewerCan{Minimize,Update,Delete}).
Validates `__typename == "IssueComment"`; on mismatch emits
`get-comment: <id> is <X>, expected IssueComment` and exits 2.
Accepts `--pr` for surface consistency with `get-thread` even
though node ids are globally unique.
Validated via shellcheck, shfmt -i 2 -s, and live calls against
PR #149: a valid IssueComment node returns the expected object,
a PullRequestReviewThread id is rejected with the typename
mismatch error, an invalid id surfaces gh's "could not resolve"
error, and zero / multiple positional ids both die at the
dispatch precondition.
Contributor
|
Claude finished @Bad3r's task in 3m 42s —— View job Code Review: PR #149 —
|
| Case | Expected | Observed |
|---|---|---|
| 3 same-author items + 1 solo (other author) | newest by author dropped, solo dropped | a, b retained ✅ |
| Tied timestamps for two same-author items, plus one strictly newer | both tied items retained | a, b retained ✅ |
Same-author status comments + one substantive ad-hoc, --similar-prefix 30 |
r1, r2 superseded by r3 by prefix; ad-hoc retained | r1, r2 retained ✅ |
| Same-author status comments + one substantive ad-hoc, no prefix gate | all three older items superseded | r1, r2, adhoc retained ✅ |
Reviews with one PENDING (submittedAt == null) |
r1 superseded by r2, PENDING excluded from both sides | r1 retained ✅ |
Dispatch interlocks all fire correctly:
list-comments --similar-prefix 30(no--superseded) →--similar-prefix requires --superseded✅list-reviews --similar-prefix 30 --superseded(zero/non-numeric) →--similar-prefix must be a positive integer✅list-threads --superseded→list-threads: --superseded is not applicable(becauselist-threadsis not inSUBCOMMAND_FLAGSfor these flags — the_assert_flags_forallowlist correctly rejects it) ✅
Two notes worth recording:
-
Cost. The supersession check is O(n²) per filter (one
anywalk per item). For PR-scale volumes (≤ few hundred items per PR) this is fine. For an archive sweep across thousands of items, agroup_by(.author.login) | map(sort_by(.createdAt) | .[0:-1]) | flattenshape would be O(n log n), but it would also complicate the prefix gate (the "newest item to compare against" stops being unique once you also condition on prefix-equality). The current shape is the right trade-off for the target volume; flag this only if a future caller wants to run it across years of comments at once. -
Filter-order docstring claim verified. The docstring at
L564-L569claims--minimized=false --supersededreads as "from what is still visible, drop the newest per author." The order in the function does match (author → minimized → superseded), so the iterativehide-commentloop will indeed converge to zero per the documented contract.
0866ea3 — get-comment <id> ✅
L1552-L1594. Mirrors get-thread exactly:
__typenamevalidation withIssueCommentguard (round-3 hardening pattern, applied uniformly).return 2on both API failure paths (matches the file-wide convention: 1 = user error, 2 = API/typename error).--praccepted but never read (node ids are globally unique) — same surface asget-thread.- Field set is the full
list-commentsper-item shape:id, databaseId, author, body, createdAt, updatedAt, url, isMinimized, minimizedReason, viewerCan{Minimize,Update,Delete}. Matches. - Dispatch checks 1 positional arg; both 0 and 2 args die with the expected message.
- Registered in
SUBCOMMAND_FLAGSas"quiet pr"so unrelated flags fail loudly via_assert_flags_for.
Lints
bash -n scripts/gh-cli/pr-comments-mgmt.sh✅shellcheck scripts/gh-cli/pr-comments-mgmt.sh✅ (clean)shfmtnot available in this CI sandbox; the author confirmedshfmt -i 2 -sclean per each commit message.
Findings
None blocking. One optional observation logged inline above (the O(n²) vs group_by trade-off in supersession), explicitly not flagged as actionable — the current shape is the right call for the target scale and keeps the prefix-gate semantics simple.
Praise
- The
_apply_review_filtersfunction is a near-mirror of_apply_comment_filtersbut the docstring atL601-L607explicitly calls out the one substantive divergence (PENDING reviews on both sides of the comparison). That's the right discipline for "near-duplicate but not really" code: don't pretend the duplication is incidental, name the difference and why it has to exist. get-commentaccepting--preven though the node id is globally unique is the right surface choice for muscle-memory consistency withget-thread. The docstring atL1554-L1556records the rationale so a future reader doesn't try to "tighten" the surface and break ergonomics.- Commit messages on all four continue to be exemplary: scope, rationale, validation matrix, and explicit non-contract caveats where relevant.
f0d8783in particular is a textbook example — declares the workflow it enables, lists every guard, lists what it deliberately doesn't change, and pins the live-PR validation count.
Net: nothing left to do. Carry-overs (bats for _glob_to_regex, body_len jq-codepoint trivia) remain appropriately deferred per rounds 9-12.
· branch feat/pr-comments-mgmt-cli
Drop the `_json_string` helper and the `printf`-based JSON assembly in `err`, `log`, and `_bulk_summary` so every diagnostic record is built end-to-end by `jq -cn`. `_bulk_summary` keeps `ok`/`failed` numeric via `--argjson`. The bootstrap `jq` check stays plain text since it has to predate any jq invocation.
Contributor
|
Claude finished @Bad3r's task in 2m 23s —— View job Code Review: PR #149 —
|
Doc drift: the file-level `OUTPUT_FORMAT` state-variable comment still listed only six formats (`json`/`ndjson`/`ids`/`text`/`full`/`tsv`) and named only `text/full/tsv` as per-verb dispatched. `body` was added to the validator, the help block, and `_format_array`'s "one of seven shapes" docstring in `36b36b9` but the global was missed. Same shape fix as `19fee8e` (which fixed the matching `tsv` drift after `522f8d0`): sync the docstring so the global is documented identically to the call site. Validation: bash -n, shellcheck, shfmt -i 2 -d all clean. No code behaviour changes; the only dispatch change is in the docstring text.
Owner
Author
Round 13 / 14 follow-up plus pre-emptive doc-drift fixRound 13 ( An independent re-audit of the script against the help-block / docstring drift class surfaced one item the bot did not flag in rounds 13 / 14: the file-level Accepted
Carried (no action this round)
Validation
Carry-over statusAll previously-documented dispositions remain unchanged. No new inline review threads opened in rounds 13 / 14, and the existing 11 threads remain resolved. Net unresolved threads: 0. Net new findings flagged by round 14: 0. Net findings landed by round 15 follow-up: 1 (the |
Contributor
|
Claude finished @Bad3r's task in 2m 20s —— View job Code Review: PR #149 —
|
| Site | Line | Enumeration | Status |
|---|---|---|---|
Global OUTPUT_FORMAT docstring |
L54-L59 |
json, ndjson, ids, text, full, tsv, body | ✅ this commit |
Help block --format synopsis |
L203 |
json|ndjson|ids|text|full|tsv|body |
✅ landed in 36b36b9 |
Help block body semantics paragraph |
L220-L227 |
full description with multi-item caveat | ✅ landed in 36b36b9 |
_format_array header docstring |
L678-L693 |
"one of seven shapes" + per-kind dispatch list text/full/tsv/body |
✅ landed in 36b36b9 |
_format_array dispatch |
L695-L703 |
all 7 cases handled | ✅ |
--format <X> validator |
L1669-L1675 |
accepts all 7; error lists all 7 | ✅ |
--format=<X> validator |
L1678-L1685 |
accepts all 7; error lists all 7 | ✅ |
| Formatter implementations | _format_text L706 / _format_full L724 / _format_tsv L742 / _format_body L775 |
all 4 per-kind formatters defined | ✅ |
Worth noting that the new dispatch sentence at L58-59 enumerates text/full/tsv/body (the per-kind dispatched set), while the help block at L228-L231 enumerates text/full/body (the unstable-contract set, deliberately excluding tsv). These are different concepts (dispatch vs. stability), so they correctly enumerate different subsets. Not drift.
Drift sweep
Beyond the canonical sites listed above, no other format-name enumerations exist in the file:
$ grep -nE '"text"|"full"|"tsv"|"body"|"json"|"ndjson"|"ids"' scripts/gh-cli/pr-comments-mgmt.sh
returns only the L54-60 docstring + assignment. No second source of truth to keep in sync. Counting words also clean: the only "seven|six|..." reference left is the _format_array "one of seven shapes" (correct).
Lints
bash -n scripts/gh-cli/pr-comments-mgmt.sh✅shellcheck scripts/gh-cli/pr-comments-mgmt.sh✅ (clean)shfmtnot available in this CI sandbox; the author confirmedshfmt -i 2 -dclean per the commit message.
Findings
None. No new actionable items.
Carry-overs (still appropriately deferred)
batssmoke suite for_glob_to_regex— pinned by help-block semantics + placeholder-ordering comment; defer until the function gains a second consumer.body_lenjq-codepoint-vs-byte trivia —lengthon a JSON string is the right semantic for the audit-grep use casetsvis meant for.
Praise
- The author's own re-audit caught a doc-drift item my round-13 and round-14 reviews missed. The discipline of not relying on the bot to find every drift, and instead doing an independent sweep against a known drift class ("any new format added must be mirrored at the global docstring AND the help block AND the dispatcher AND the validator AND
_format_array's docstring"), is exactly the right shape for a 2087-line file with seven canonical sites that have to agree. - Commit message on
52b2202is again exemplary: names the previous matching fix (19fee8e), names the originating drift commit (36b36b9), and explicitly scopes the change as docstring-only with no behaviour delta. Five lines that close out a doc-drift round with no ambiguity.
Net: nothing left to do. Carry-over status from rounds 9-14 unchanged. Ship it.
· branch feat/pr-comments-mgmt-cli
Bad3r
added a commit
that referenced
this pull request
May 10, 2026
…#149) * feat(scripts): add pr-comments-mgmt cli helper Wraps `gh api graphql` to manage PR review threads: * resolve / hide-comment / hide-thread mutations * list-threads pagination over reviewThreads and inner comments * current-pr lookup for the open PR on the current branch Subcommands default to the current repo and PR via gh, validate ReportedContentClassifiers via an allowlist, and surface partial GraphQL errors instead of swallowing them. * refactor(scripts/pr-comments-mgmt): emit ndjson for all non-help output Wrap err/log in {level, prog, message} envelopes via a manual escaper (no jq bootstrap dependency), and route every gh call through `_gh_run` so gh's own stderr is captured and re-emitted as JSON instead of leaking plain text. Stdout from list-threads and current-pr remains a single pretty-printed JSON document; stderr becomes NDJSON. The help menu (via --help / -h / no-args) is intentionally left as plain text per spec. Verified via a 30-case test runner (help, bad option/reason, unknown subcommand, missing/empty/fake ids per subcommand, list-threads with 0/1/2/3 args and bad shapes, current-pr ok/extra/--reason): every non-empty stdout line parses as one JSON document and every non-empty stderr line parses as a JSON object. * fix(scripts/pr-comments-mgmt): assert __typename in hide-thread Replaces the loose `type == "object"` check, which only catches the "node id does not exist" case. When an id resolves to a different Node type (issue comment, discussion, etc.), GraphQL returns a populated `data.node` whose inline fragment didn't match, so the old check passed silently and the loop emitted a confusing follow-on error from `resolve_thread`. Asks GraphQL for `__typename` and asserts equality to `PullRequestReviewThread`, surfacing the actual type in the error message. * fix(scripts/pr-comments-mgmt): fail loudly on missing PR in list-threads If GraphQL ever returns `pullRequest: null` without populating the top-level `errors` array, the previous code treated it as success: `page` became `null`, the inner loop iterated zero times, and the accumulator emitted `[]`, making a nonexistent PR indistinguishable from a PR with no review threads. Asserts `data.repository.pullRequest != null` after each page fetch and exits 2 with a contextual error message. * fix(scripts/pr-comments-mgmt): enforce open PR contract The usage block advertised `current-pr` as printing the *open* PR for the current branch, but `gh pr view` resolves any PR for the branch regardless of state (open/merged/closed). The same ambiguity leaked into `list-threads` no-args, where a closed PR's threads would silently be listed. Both code paths now request the PR `state` from gh and bail out with a contextual error if it is not `OPEN`. The `state` field is stripped from `current-pr` output to keep the documented schema. * fix(scripts/pr-comments-mgmt): treat empty errors array as success `jq -c '.errors // empty'` only triggered the alternative for `null`/missing — an empty array `[]` flowed through verbatim and tripped the `[[ -n ${errors} ]]` check, failing the call with `graphql errors: []`. GitHub doesn't currently emit `errors: []`, but the new guard rejects the response only when it actually carries entries. * fix(scripts/pr-comments-mgmt): escape all JSON control characters The pure-bash `_json_string` only handled the named escapes (`\b \f \n \r \t \\ \"`); anything else in U+0000-U+001F flowed through verbatim, producing JSON that strict parsers reject per RFC 8259 §7. The realistic trigger today is captured `gh` stderr that contains ANSI escape sequences leaking into NDJSON consumers. Delegates encoding to `jq -Rn --arg s "$1" '$s'`, which covers the full control-character range plus surrogate-pair edge cases. Adds an explicit early `command -v jq` probe (with a plain-text error) before the trap is installed so a missing `jq` no longer recurses through `err -> _json_string -> jq`. Drops the now-redundant `require_cmd jq` from `main`. * refactor(scripts/pr-comments-mgmt): collapse jq passes per outer page The previous loop walked every thread in the outer page just to find the few whose inner comments were paginated, doing a `jq` sub-shell per index for `length`, the slice, and `hasNextPage`. A single `to_entries[] | select(...) | .key` pass over the page extracts only the indices that actually need follow-up fetches, and the page-info reads at the bottom share one decode. Output is byte-identical against the old loop (verified on PR #149). Trims several jq sub-shells per outer page, which adds up on PRs with many threads. * docs(scripts/pr-comments-mgmt): note bootstrap jq probe is plain text The `command -v jq` probe at the top of the script intentionally emits plain text instead of NDJSON because `_json_string` itself depends on `jq`. Adds a one-line comment acknowledging the deliberate inconsistency so a strict NDJSON consumer doesn't mistake it for an oversight. * chore(scripts/pr-comments-mgmt): document _gh_run success-path stderr Successful gh invocations occasionally emit progress hints or deprecation notices on stderr; those do not belong in this script's structured stdout stream. Capture the existing intent in a comment so a future reader does not "fix" the silent drop. * refactor(scripts/pr-comments-mgmt): per-subcommand applicable-flags map Replaces the ad-hoc `[[ ${reason_set} == true ]] && die ...` lines with a single `SUBCOMMAND_FLAGS` allowlist consulted by `_assert_flags_for`. Existing behavior is preserved: `--reason` still rejects on `resolve`, `current-pr`, and `list-threads`. Future flags inherit the same scaffolding without touching every case branch, and a missing allowlist entry now fails loudly so new subcommands cannot forget to register. * feat(scripts/pr-comments-mgmt): add --pr global flag and pr_resolve helper Replaces the per-subcommand `[<owner/repo>] [<pr-number>]` positional shape with a uniform `--pr <number>` or `--pr <owner/repo>#<number>` global flag. The new `pr_resolve` helper exports `PR_OWNER_REPO` and `PR_NUMBER` either from `--pr` (no state assertion: the user has named a PR explicitly) or from a `gh repo view` + `gh pr view` fallback on the current branch (still asserts OPEN). `list-threads` and `current-pr` consume the resolved globals; the legacy positional `list-threads` form is removed per the project's no-backward-compatibility rule. Help text and examples are updated to match. * feat(scripts/pr-comments-mgmt): add stdin-id and bulk-summary scaffolding Adds three shared helpers that later commits build on: - `_read_body` sources a body from --body-file (with `-` meaning stdin), --body, or a single positional argument, with a "exactly one source" contract. - `_collect_ids` returns positional ids when present, otherwise reads them from stdin one-per-line, skipping blank and `# ...` comment lines. - `_bulk_summary` emits a final NDJSON record `{"verb":...,"ok":N, "failed":M}` on stderr, regardless of --quiet. Wires `_collect_ids` and `_bulk_summary` into the existing `resolve`, `hide-comment`, and `hide-thread` branches so every bulk verb shares one input and reporting contract before new bulk verbs land. Adds the global `--body` and `--body-file` parsers (rejected via the flags allowlist until a consumer subcommand opts in). Behavior change worth noting: bulk verbs no longer die on empty input; they exit 0 with `ok=0,failed=0`. This is required for stdin-driven pipelines such as `list-threads --format=ndjson | jq -r .id | resolve` to no-op cleanly when the upstream filter yields nothing. * feat(scripts/pr-comments-mgmt): include comment bodies and diff context in list-threads Triage workflows need the actual review text and diff context, not just ids and resolution state. Extends the `list-threads` selection set: - Per thread: `subjectType`, `isCollapsed`, `resolvedBy { login }`, `viewerCanResolve`, `viewerCanUnresolve`, `viewerCanReply`. Combined with the existing `isOutdated` and `isResolved`, this is enough to filter by status without a follow-up call. - Per comment: `body`, `diffHunk`, `originalLine`, `originalStartLine`, `subjectType`. The follow-up pagination query in `_fetch_thread_comments_page` is updated in lockstep so first-page and subsequent-page comments share one schema. * feat(scripts/pr-comments-mgmt): expose richer current-pr payload Adds the fields a triage workflow asks "should I act on this PR?" without a second call: `id` (the PR node id, required by future GraphQL writes), `url`, `state`, `headRefName`, `baseRefName`, `author`, `isDraft`, `mergeable`, and `mergeStateStatus`. Drops the `del(.state)` finalizer so state is preserved on output. * feat(scripts/pr-comments-mgmt): add get-thread subcommand Single-thread fetcher for callers iterating from previously-saved ids. Output shape matches one element of `list-threads`: same fields, same `comments.nodes` schema, same pagination merge through `_paginate_thread_comments`. Asserts the resolved node is a `PullRequestReviewThread` (mirrors the validation in `hide-thread`) and fails with exit code 2 on type mismatch. * feat(scripts/pr-comments-mgmt): add list-reviews subcommand Mirrors `list-threads` plumbing against `pullRequest.reviews`. Returns one record per review with id, databaseId, state, body, author { login }, submittedAt, url, and commit { oid } - the fields a triage workflow needs to find Codex/Gemini/Claude summary text without a separate gh call. Uses `pr_resolve` for PR targeting and the same `--pr`/no-arg conventions as `list-threads`. * feat(scripts/pr-comments-mgmt): add --format=ndjson to list-* verbs Adds a `--format=json|ndjson` global flag (default `json`) routed through `_format_array` and applied to `list-threads` and `list-reviews`. NDJSON output emits one row per top-level node, which composes cleanly with shell pipelines such as `jq -c | while read` or `xargs`. The flag is rejected on every other verb via the `SUBCOMMAND_FLAGS` allowlist. * feat(scripts/pr-comments-mgmt): add client-side filters to list-threads Adds five composable filters that apply as a jq pipeline against the already-fetched thread array (no extra round-trips): - `--unresolved` keeps threads with `isResolved == false`. - `--outdated` keeps threads with `isOutdated == true`. - `--author <login>` keeps threads whose first comment was authored by the given login (the thread starter). - `--path <glob>` keeps threads whose path matches the supplied fnmatch-style glob (`*` and `?` are honored, every other regex meta-char is escaped via the new `_glob_to_regex` helper). - `--minimized=true` keeps threads where every comment is minimized; `--minimized=false` keeps threads with at least one non-minimized comment. Filters compose with logical AND; absent flags are no-ops. Each filter is rejected on every other subcommand via the `SUBCOMMAND_FLAGS` allowlist. * feat(scripts/pr-comments-mgmt): add reply via addPullRequestReviewThreadReply Adds `reply <thread-id> [body|--body|--body-file]` for posting threaded replies on the diff (the verb that triggered the original issue: `gh pr comment` posts a top-level conversation comment, not a threaded reply). Uses `addPullRequestReviewThreadReply(pullRequestReviewThreadId, body)`, which is cleaner than `addPullRequestReviewComment(inReplyTo, ...)` because the thread node id is already the input - no first-comment lookup or PR id round-trip is required. Body sourcing flows through the shared `_read_body` helper. The new `raw:` prefix on `graphql_call` arguments forces `gh -f` (raw string), guarding against bodies that happen to look like `true`, `null`, or a number from being silently coerced by gh's typed-parsing. * feat(scripts/pr-comments-mgmt): add unresolve and unhide-comment Adds the inverse mutations so a typo on a glob is recoverable from this tool alone: - `unresolve <thread-id>...` calls `unresolveReviewThread` and asserts the response has `isResolved == false`. - `unhide-comment <comment-node-id>...` calls `unminimizeComment` and asserts `isMinimized == false`. Both are bulk verbs: positional ids, fall back to stdin, and emit the shared `_bulk_summary` record on completion. * feat(scripts/pr-comments-mgmt): add PR write verbs Closes the symmetry gap with the existing read verbs by wrapping `gh pr edit / gh pr comment / gh pr review` behind one CLI surface so scripts can reach for one tool and one output convention: - `set-title <text>` and `set-body <body|--body|--body-file>` rewrite the title and body via `gh pr edit`. - `add-label <name>...` and `remove-label <name>...` are bulk verbs that feed `--add-label` / `--remove-label`, accept stdin, and emit the shared `_bulk_summary` record. - `set-labels <name>...` reads the current labels, computes the diff with jq, and issues a single `gh pr edit` with the appropriate `--add-label` / `--remove-label` flags. No-op when the diff is empty. - `comment <body|--body|--body-file>` posts an issue-level comment via `gh pr comment` (the threaded counterpart is `reply`). - `review --approve|--request-changes|--comment --body|--body-file <body>` submits a PR review. `--approve` permits an empty body; `--request-changes` and `--comment` require a non-empty body. `--pr` is now accepted on every subcommand for uniformity (silently ignored by node-id verbs that do not need PR context). * docs(scripts/pr-comments-mgmt): refresh usage block Rewrites the help heredoc to reflect the expanded surface. Subcommands are grouped into read / thread mutation / PR write sections; the options block lists every flag including the list-threads filters and review event flags; the bulk-verb stdin and summary contract is called out explicitly; examples cover every category, including the canonical `list-threads --format=ndjson | jq | resolve` pipeline. * fix(scripts/pr-comments-mgmt): use jq tostring for boolean response checks \`jq -r '.x // \"null\"'\` returns the fallback when \`.x\` is \`false\` (per jq's \`//\` semantics: both null and false trigger the alternative). This caused \`unresolve\` and \`unhide-comment\` to misclassify successful responses (where \`isResolved\` / \`isMinimized\` is \`false\`) as malformed and report \`failed=1\` despite the API call succeeding. Switching to \`(.x | tostring)\` produces the literal strings \`"true"\` / \`"false"\` / \`"null"\`, which the case statements match correctly. The same idiom is applied to \`resolve\` and \`hide-comment\` so the boolean check is uniform across the four mutation paths; those two were not symptomatic because their success values are \`true\`. * fix(scripts/pr-comments-mgmt): refuse set-labels with empty desired set Previously, `set-labels` with no positional args and an empty stdin computed `diff = {add: [], remove: <every existing label>}` and silently issued `gh pr edit --remove-label` for every label on the PR. That conflicts with CLAUDE.md's "Never make changes irrecoverable" stance and is almost always an upstream pipe bug, not an intent. Refuse the empty-desired-set call early with a clear error pointing at `remove-label` for explicit drops. * fix(scripts/pr-comments-mgmt): die on bulk verb with no ids `resolve`, `unresolve`, `hide-comment`, `unhide-comment`, `hide-thread`, `add-label`, and `remove-label` previously looped zero times when both positional args and stdin were empty, then printed `{"ok":0,"failed":0}` and exited 0. That collapses two distinct outcomes ("nothing to do" vs "input pipe closed unexpectedly") into a single "success" signal. Add `_assert_processed` so each bulk verb dies 1 with a clear message when the loop never ran. The summary record is still emitted on the mutation paths. * fix(scripts/pr-comments-mgmt): reject empty set-title `set-title ""` previously passed the `((${#args[@]} == 1))` guard and called `gh pr edit --title ""`. GitHub would reject the empty title, but only after a network round-trip and with a less obvious error. Match the body-source guards in `_read_body` and fail fast locally. * fix(scripts/pr-comments-mgmt): handle globstar in --path filter The previous translation mapped `*` and `?` to greedy `.*` / `.`, which crossed `/` and made `modules/**/*.nix` translate to a regex that could not match `modules/foo.nix` (the inner `*.nix` was eaten by the leading greedy match) and equally failed user expectations for multi-level paths. Switch `_glob_to_regex` to gitignore-style semantics: * -> [^/]* (within a path segment) ? -> [^/] **/ -> (?:[^/]+/)* (zero or more directory levels) /** -> (?:/[^/]+)+ (one or more trailing levels) ** -> .* Globstar tokens are extracted via NUL-byte placeholders before the single-char rules run, so the tightened `*` does not eat their inner stars. Verified against 25 cases including `*.sh`, `modules/**/*.nix`, `**/foo.sh`, `a/**`, `src/**/test_*.py`, etc., and the usage block now documents the supported wildcards. * docs(scripts/pr-comments-mgmt): refresh current-pr fields and clarify --comment Two doc tweaks against the latest review: - `current-pr`: list every field the JSON now carries (id, number, title, body, state, url, headRefName, baseRefName, author, isDraft, mergeable, mergeStateStatus, labels), so the help text stops drifting from `gh pr view`'s actual JSON keys. - `comment` / `review --comment`: spell out that the standalone `comment` subcommand and `review`'s `--comment` event flag are distinct verbs. `_assert_flags_for` already rejects the cross-use, but the help text now signposts the duality. * docs(scripts/pr-comments-mgmt): note that --path glob has no escape syntax Per round-4 review nit on `_glob_to_regex`: `\*` translates to `^\\[^/]*...`, i.e., a literal backslash followed by any non-slash run, not a literal `*`. The previous usage block didn't promise escape support but didn't disclaim it either, so a reader coming from gitignore could reach for `\*` and silently get a regex that does not match what they intended. Add a one-line note to the `--path` help and a longer comment in the function explaining the deliberate omission. No code change; review paths realistically never contain glob meta-characters and escape support would complicate the placeholder ordering for no real-world coverage. * fix(scripts/pr-comments-mgmt): explain blank/comment-only stdin in bulk error Round-4 review nit: when stdin contains only blank or `#`-comment lines, `_collect_ids` filters them out and the bulk verb dies with "no ids supplied (positional or stdin)" — technically correct but it reads as if the pipe were closed. Differentiate the two failure modes: - "no ids supplied (positional empty, stdin empty)" when nothing was read. - "no ids supplied (read N stdin line(s), all blank/comment)" when `_collect_ids` drained N lines but every one matched the filter. Implementation: `_collect_ids` writes its pre-filter line count to ${BULK_READ_COUNT_FILE} (set per dispatch via `_bulk_count_file_init` / `_bulk_count_file_done`), and `_assert_processed` reads it to pick the right detail string. Verified against all three paths on PR #149's `resolve`; the happy path is unchanged. * fix(scripts/pr-comments-mgmt): clean up bulk count tempfile on die path Round-5 review caught a resource leak: `_bulk_count_file_init` creates the per-dispatch `BULK_READ_COUNT_FILE` via `mktemp`, but on the `_assert_processed` -> `die` path the explicit `_bulk_count_file_done` call after `_assert_processed` is never reached and the file leaks. Verified 5 successive `printf '' | resolve --pr 149` runs accumulated five files in /tmp before the fix. Install an `EXIT` trap inside `_bulk_count_file_init` so cleanup runs on every exit path (success, partial-failure summary, and the no-ids die). The success path's explicit `_bulk_count_file_done` call still runs first; the trap then no-ops because `_bulk_count_file_done` guards on `${BULK_READ_COUNT_FILE:-}`. Verified post-fix: zero `/tmp` delta after 5 die-path runs and 3 blank/comment-path runs plus a success run. There is no other `EXIT` trap on this script; only the `ERR` trap at line 82, which uses a different signal and is unaffected. * feat(scripts/pr-comments-mgmt): add list-comments The script's read surface previously covered review threads (`list-threads`), review submissions (`list-reviews`), and a single thread (`get-thread`), but had no verb for the issue-level (top-level) PR conversation comments. Every other channel the CLI can write to via `comment` it should be able to read from too — without that symmetry, managing the conversation tab (e.g., enumerating ids for `hide-comment`) requires falling back to `gh api repos/.../issues/<n>/ comments` directly, which defeats the point of having a single CLI surface. Add `list-comments`, mirroring `list-reviews`: - Paginated GraphQL `pullRequest.comments` walk (100 per page, cursor-driven). - Per comment: id, databaseId, author { login }, body, createdAt, updatedAt, url, isMinimized, minimizedReason, viewerCanMinimize, viewerCanUpdate, viewerCanDelete. - Shares `_format_array` with the other list verbs (`json` default, `--format ndjson` for one document per line). - Same flag allowlist shape (`quiet pr format`). - `_assert_flags_for` rejects flags that don't apply, matching every other subcommand. Verified against PR #149: 13 total comments, 12 minimized, 1 visible (the most recent LGTM); the missing-PR path returns the same graphql-failure NDJSON pair as `list-reviews`; positional args reject with `takes no positional arguments (use --pr)`. Usage example added showing the "list -> filter -> hide" pipeline that was previously impossible without `gh api` fallback. * feat(scripts/pr-comments-mgmt): add --format ids and list-comments filters Closes the second half of the read-side gap that motivated the list-comments addition: enumerating ids for `hide-comment` (or any other bulk verb) still required a `jq -r '... | .id'` fallback even with `list-comments` available, because the script had no native id-only output and `list-comments` had no filters. - `--format ids` on `list-threads`, `list-reviews`, and `list-comments` emits one `.id` per line (skipping null/missing ids), suitable for piping straight into a bulk verb. - `_apply_comment_filters` mirrors `_apply_thread_filters` for the `list-comments` shape (each element has `.author.login` and `.isMinimized` directly, not via nested `.comments.nodes[]`). `list-comments` accepts `--author <login>` and `--minimized true|false` with the same semantics as their list-threads counterparts. This collapses the cleanup workflow that previously required `gh api` + `jq` plumbing into a single end-to-end pipeline: list-comments --pr N --minimized=false --format=ids | hide-comment --pr N --reason RESOLVED Verified against PR #149: every format/filter combination produces the expected counts, the bad-format-value path dies 1 with the updated allowlist message, and `_assert_flags_for` correctly accepts `--author`/`--minimized` on `list-comments` (parallel to `list-threads`). * feat(scripts/pr-comments-mgmt): add dismiss-review `list-reviews` had no write counterpart in the script; PullRequestReview shells (gemini-code-assist's "Code Review" summary, codex's "About" info box, every claude review-submission stub) sit in the conversation timeline with no `hide-comment`-equivalent path because the `Minimizable` interface does not include `PullRequestReview`. Add `dismiss-review` wrapping the `dismissPullRequestReview` mutation: - Bulk-shape (positional ids or stdin), mirroring resolve / hide-comment / unresolve / unhide-comment. - Message via --body / --body-file (positional args are review ids, so no positional body); both an explicit "--body or --body-file is required" guard and the empty-message guard fire before the API round-trip. - `dismiss_review` validates the response state is `DISMISSED` and emits the standard `dismissed: <id>` log line; the bulk loop wires through `_assert_processed` + `_bulk_summary` like every other bulk verb. - Wired into the `_assert_flags_for` allowlist (`quiet pr body body-file`). GitHub's runtime constraint (discovered empirically against PR #149's all-COMMENTED reviews): only APPROVED and CHANGES_REQUESTED reviews are dismissable; COMMENTED and PENDING reviews are rejected with "Can not dismiss a commented pull request review" even though the input type takes any review id. The usage block and the function comment both spell this out so users do not waste a round-trip. Verified against PR #149: missing-body, empty-body, no-ids, and disallowed-flag paths all die `1` with the right message before any API call; the API-rejection path against COMMENTED reviews surfaces the GitHub error verbatim and reports `failed=N` via the bulk summary. The dismissed-state code path is unverified empirically because no APPROVED / CHANGES_REQUESTED reviews exist on this PR; it follows the same shape as `resolve_thread` / `unresolve_thread` etc., which are verified. * refactor(scripts/pr-comments-mgmt): scope name local in set-labels arm Every other bulk-handler arm (`add-label`, `remove-label`, `resolve`, `unresolve`, `hide-comment`, `unhide-comment`, `hide-thread`, `dismiss-review`) declares `local name` (or `local id`) before the read loop. The `set-labels` arm did not, so `name` leaked into the outer `main()` scope. Benign today (nothing in `main()` references `name` afterward) but trivially fixed and matches the rest of the dispatch table. Resolves PR #149 thread PRRT_kwDOPeLwm85_Il3r. * fix(scripts/pr-comments-mgmt): pipe merge JSON via stdin to avoid execve(2) MAX_ARG_STRLEN The four `jq -n --argjson <name> "${large_json}" ...` merge sites (`_paginate_thread_comments`, the per-page replace inside `list_threads`, and the `${all_*} + ${page}` accumulators in `list_threads`, `list_reviews`, `list_comments`) blow up once the merged JSON crosses Linux's per-argv-string limit. execve(2) caps a single argv element at MAX_ARG_STRLEN = 32 * PAGE_SIZE (128 KiB on a 4 KiB-page x86_64 box) regardless of the headroom in ARG_MAX (typically 2 MiB). Each `--argjson` value is one argv element, so a single PR with enough threads / large diffHunks pushes one of these sites past 128 KiB and `jq` exits before it ever runs: pr-comments-mgmt.sh: line N: /path/to/jq: Argument list too long Reproduced empirically on PR #149 itself (11 threads, 185 KiB merged page): `list-threads` returned exit 0 with empty stdout and only the "Argument list too long" message on stderr — silent data loss for any caller piping the output (`| resolve`, `| hide-comment`, etc.). Fix: feed both inputs via stdin and use `jq -s` to slurp them as `.[0]` / `.[1]`. Stdin has no per-element size cap, so the limit becomes RLIMIT_DATA / available memory rather than an arbitrary 128 KiB. Other small `--argjson` callers (the int `idx` in the per-thread page rewrite, the small label arrays in `set_labels`) keep the original shape since they're nowhere near the cap. Verified end-to-end against PR #149 after the fix: $ ./scripts/gh-cli/pr-comments-mgmt.sh list-threads --pr 149 \ --format=ids | wc -l 11 $ ./scripts/gh-cli/pr-comments-mgmt.sh list-reviews --pr 149 \ --format=ids | wc -l 15 $ ./scripts/gh-cli/pr-comments-mgmt.sh list-comments --pr 149 \ --format=ids | wc -l 16 $ ./scripts/gh-cli/pr-comments-mgmt.sh get-thread --pr 149 \ PRRT_kwDOPeLwm85_Il2V | jq '.id' "PRRT_kwDOPeLwm85_Il2V" Pre-fix all four invocations returned exit 0 with empty stdout. * refactor(scripts/pr-comments-mgmt): drop unnecessary export of BULK_READ_COUNT_FILE `_collect_ids` is only ever invoked through process substitution `< <(_collect_ids "${args[@]}")`, which runs in a bash subshell. Bash subshells inherit unexported variables for free; `export` only matters for child processes started via `execve(2)`, which this code path does not use. Drop the `export`, document the rationale in the function header so the next reader does not re-add it "for safety", and re-validate the bulk loop's empty-stdin / blank-only-stdin diagnostics still flow through correctly: $ printf '' | resolve --pr 149 {"level":"error",...,"message":"resolve: no ids supplied (positional empty, stdin empty)"} $ printf '# x\n\n # y\n' | resolve --pr 149 {"level":"error",...,"message":"resolve: no ids supplied (read 3 stdin line(s), all blank/comment)"} Both unchanged from pre-edit behavior. * docs(scripts/pr-comments-mgmt): clarify cursor sentinel and hide_thread first-page validation Two adjacent comments inside `hide_thread`, which is the first function in the file that establishes both conventions: 1. The `local cursor="null"` sentinel: `gh -F cursor=null` parses the value as JSON-ish and forwards it as the GraphQL `null` literal, which the `$cursor: String` parameter accepts as "no cursor / first page". Any other sentinel string ("", "false", etc.) would be rejected by GitHub as `Argument 'cursor' has an invalid value`. This same convention is reused by `list_threads`, `list_reviews`, and `list_comments`. 2. The `validated=false` first-page-only `__typename` check: a node id ↔ type mapping is stable across pages on GitHub's side (the same id cannot be a `PullRequestReviewThread` on page 1 and a different type on page 2), so re-checking on every page would be wasted round-trips. Documenting this so a future reader doesn't move the check inside the loop "for safety". Both are non-obvious choices that already work; this just records the "why" so the next reader doesn't have to reverse-engineer it. * refactor(scripts/pr-comments-mgmt): unique desired labels in set-labels `jq -n --args '$ARGS.positional'` preserves duplicates as the user typed them. `(desired - current)` is array-difference, which keeps duplicates within `desired`, so `set-labels foo foo` (when `foo` is not on the PR) emitted `--add-label foo --add-label foo`. `gh pr edit` is idempotent on label adds, so this was benign in practice, but it produced ugly bulk-summary numbers and a confusing `gh` argv. `unique` in the desired-side jq pipeline normalises before the diff and costs nothing. Verified empirically: $ jq -nc --argjson c '["bar"]' \ --argjson d "$(jq -n --args '$ARGS.positional | unique' foo foo)" \ '{add: ($d - $c), remove: ($c - $d)}' {"add":["foo"],"remove":["bar"]} * feat(scripts/pr-comments-mgmt): add --sort, --limit, text/full formats The list-* verbs already covered "fetch + filter" but consumers still had to pipe to jq for the recurring shapes — "latest review with body", "last 5 reviews summary line", "last 3 comments full block", "chronological preview". Move the sort + limit + per-item formatting into the wrapper so those workflows do not require external jq. - `--sort newest|oldest`: sort by the per-verb natural timestamp (`submittedAt` for reviews, `createdAt` for comments, the first comment's `createdAt` for threads). Implemented as `sort_by(<expr>) [| reverse]` in the new `_apply_view`, applied after the per-verb filter pass and before the format pass. - `--limit N`: positive integer, takes the first N items after `--sort`. `--limit 0`, negative, and non-numeric values die `1`. - `--format text`: one short summary line per item, with the relevant per-verb fields: - reviews: `[<time>] <author> (<state>) body=<N> chars` - comments: `[<time>] <author> [minimized:<reason>]? body=<N> chars` - threads: `[<path>:<line>] <first-author> resolved=.. outdated=.. comments=<N>` - `--format full`: header + body block per item, with per-verb headers: - reviews: `=== [<time>] <author> (<state>) ===\n<body>` - comments: `=== [<time>] <author> [minimized:<reason>]? ===\n<body>` - threads: `=== [<path>:<line>] <first-author> resolved=.. outdated=.. ===\n<first-body>` - `_format_array` now takes a kind arg ("threads"|"reviews"|"comments") to pick the right text/full template; json/ndjson/ids paths are unchanged. Each list_* function passes its kind. - SUBCOMMAND_FLAGS extended for the three list verbs to accept `sort` and `limit`; non-list verbs reject these via `_assert_flags_for`. Verified against PR #149 that the four real-world workflows (latest review full, last 5 reviews text, last 3 comments full, all comments text preview) collapse to single CLI invocations with no external jq. Edge cases: bad sort/limit values die 1 with the right message, sort/limit composes with the existing --author / --minimized / etc. filters, and existing json/ndjson/ids consumers see no change. * feat(scripts/pr-comments-mgmt): add --format tsv The recurring "thread audit" query — id-keyed, status flags, total + unminimized comment counts per row — still required raw `gh api` + `jq` because the existing `text` format is prose-style and lacks unminimized counts and id, while `ndjson` is one-document-per-line with no per-verb column projection. Add a tab-separated tabular format with per-verb columns so audit/automation work fits awk, cut, column, and Excel without external jq. Per-verb columns (no header — leave that to the caller so cut -f<n> and awk -F'\t' do not have to skip a row): reviews: id, submittedAt, author, state, body_len, url comments: id, createdAt, author, isMinimized, minimizedReason, body_len, url threads: id, isResolved, isOutdated, path, line, first_author, comments, unminimized The threads row mirrors the audit query exactly: id-first, with the `unminimized` count derived from `comments.nodes | map(select(...)) | length` so triage can spot threads with unfinished hide-comment work in a single pass. Verified against PR #149: tsv emits one row per item across all three list verbs, composes with the existing --sort / --limit / --author / --minimized / --unresolved filters, and renders cleanly through `column -t -s $'\t'` for visual columns. Existing json/ndjson/ids/ text/full consumers see no change. * refactor(scripts/pr-comments-mgmt): fail loudly on unknown format kind; clarify --sort/--limit help Round 8 of automated review (claude on `5c37b4f`) flagged three items in the new `--sort` / `--limit` / format-dispatch surface that were nits rather than bugs: - `_format_text` / `_format_full` / `_format_tsv` silently emitted nothing when called with a kind not in their `case`, instead of failing loudly. Today the only callsite is `_format_array`, which controls the kinds, but a future list verb that forgets to register in one of the three formatters would silently return empty output rather than tripping the diagnostic. Add `*) die 1 "_format_*: unknown kind '\$1'" ;;` arms, matching the same "missing entry fails loudly" pattern as `_assert_flags_for`. - `--limit N` help read as if `--sort` were required ("Keep the first N items after --sort"). The parser actually allows `--limit` alone and keeps the first N items in cursor-pagination order. Reword to document the standalone behavior. - `--sort newest` documented as sorting by the natural timestamp, but did not call out that `null` timestamps (PENDING reviews) sort to the tail. So `list-reviews --sort newest --limit 5` never surfaces a pending review while any submitted review exists. Defensible semantics, but worth recording so a future reader does not file it as a bug. - One inline doc comment on `_format_full` records that the `threads` arm renders the thread-opener's body only; full reply chains belong to `get-thread <id>`. This matches the round-8 reviewer's preferred resolution (document the trim rather than expand `full` into a thread dump). Verified end-to-end against PR #149: `list-threads --format=text`, `list-reviews --format=tsv`, and `list-comments --format=full` all still produce the expected per-verb shapes. * docs(scripts/pr-comments-mgmt): include tsv in OUTPUT_FORMAT docstring Round 9 nit: the file-level `OUTPUT_FORMAT` state-variable comment listed only the original five formats (`json`/`ndjson`/`ids`/`text`/ `full`); `tsv` was added to the validator, the help block, and `_format_array`'s docstring in `2b52dac` but not here. Sync the docstring so the global is documented identically to the call site. * fix(scripts/pr-comments-mgmt): reject empty bodies for set-body/comment/reply Round 9 nit (carry-over from round 8): `set-title` rejects empty titles locally and `dismiss-review` rejects empty messages, but `set-body`, `comment`, and `reply` happily round-tripped empty bodies through `gh`: - `set-body --body=''` silently cleared the PR description via `gh pr edit --body ""` — almost certainly not what the caller meant; if they actually want to clear the body, the bare `gh` command or the GitHub UI is the right escape hatch. - `comment --body=''` posted (or got rejected by `gh`'s voice rather than this CLI's), losing the structured NDJSON error shape. - `reply --body=''` had the same problem against `addPullRequestReviewThreadReply`. Fix: add `[[ -n ${body} ]] || die 1 "<verb>: body cannot be empty"` guards in each dispatch arm before the API round-trip, matching `set-title` (1731) and `dismiss-review` (1711). Update the help block to document the new contract for `reply`, `set-body`, and `comment`. `review --approve` still permits an empty body (documented at the existing `review` entry); the others now match the rest of the file's guard style. Verified all three diagnostics fire correctly: $ ./scripts/gh-cli/pr-comments-mgmt.sh --pr 149 set-body --body='' {"level":"error",...,"message":"set-body: body cannot be empty"} $ ./scripts/gh-cli/pr-comments-mgmt.sh --pr 149 comment --body='' {"level":"error",...,"message":"comment: body cannot be empty"} $ ./scripts/gh-cli/pr-comments-mgmt.sh --pr 149 reply <id> --body='' {"level":"error",...,"message":"reply: body cannot be empty"} * docs(scripts/pr-comments-mgmt): rename tsv threads column unminimized -> visible_comments Round 9 nit: the 8th column of `--format=tsv` for `list-threads` counts comments where `isMinimized` is false, i.e. comments currently visible in the GitHub UI. The original name `unminimized` read as either "comments that have been *unminimized* (action history)" or "comments not currently minimized (current state)"; only the latter matches the code. `visible_comments` is unambiguous and matches the spelling readers expect when asking "how many comments would I see if I scrolled through the thread?" Update both the help block and the inline column comment in `_format_tsv` so they match. The TSV output itself is unchanged (no header is emitted), so this is doc-only — no caller has to update parsing. * docs(scripts/pr-comments-mgmt): align tsv example header with visible_comments rename Round 11 nit: the `--format=tsv` example in the help block injects an `awk` header for the threads output, but the 8th-column label still read `unmin` — carrying the same action-history-vs-current- state ambiguity that motivated the `unminimized → visible_comments` rename in `d5ce9c4`. Update the example header to `visible` so the documentation is consistent end-to-end. The change is to a user-supplied awk literal embedded in the help example, so no caller's parsing is affected — TSV output still emits no header row. * feat(scripts/pr-comments-mgmt): include id in text and full headers `--format=text` lines are now `<id>\t<rest>` and `--format=full` headers are now `=== <id> <rest> ===`, so the operator can pipe either format through `cut -f1` (text) or grep the headers (full) without re-querying the same items via `--format=ids`. The text and full layouts remain non-contract; downstream parsers should keep using `ndjson` or `tsv`. Validated via shellcheck, shfmt -i 2 -s, and live runs of list-comments / list-reviews / list-threads against PR #149 in text and full formats. * feat(scripts/pr-comments-mgmt): add --format=body for raw body emission Adds a body-only output mode to list-threads, list-reviews, and list-comments that dumps `.body` per item with no headers or separators (opener body for threads, matching `full`). Best paired with `--limit=1` to read a single item; multi-item runs concatenate without delimiters and should prefer `--format=full` instead. Implementation: new `_format_body` helper, dispatcher case in `_format_array`, and the `--format` validator (both `--format X` and `--format=X` argv forms) extended to accept `body`. text/full /body remain non-contract; downstream parsers should keep using ndjson or tsv. Validated via shellcheck, shfmt -i 2 -s, and live runs of list-comments / list-reviews / list-threads against PR #149, including a negative test that an unknown --format value still errors with the updated allowlist. * feat(scripts/pr-comments-mgmt): superseded/similar-prefix filters; --author for list-reviews Adds two composable filters to list-comments and list-reviews so the "find older same-author items and hide/dismiss them" workflow can be expressed as one pipe instead of a hand-rolled GraphQL+jq cycle: --superseded Keep items where some other item by the same author has a strictly later timestamp (createdAt for comments, submittedAt for reviews). PENDING reviews (no submittedAt) are excluded from both sides of the comparison. --similar-prefix N Modifier on --superseded: also require the newer same-author item to share the first N bytes of `.body`. Lets `claude` round-N status comments be separated from substantive one-off comments by the same author. `--superseded` runs after `--author`/`--minimized` inside the filter helper, so `--minimized=false --superseded` reads as "from what is still visible, drop the newest per author and keep the rest" and drives the iterative `hide-comment` loop to zero. `--similar-prefix` without `--superseded` is rejected at dispatch time. Also extends `--author` to list-reviews (already supported on list-comments and list-threads) so `--superseded --author <login>` is composable for the review path; a new `_apply_review_filters` mirrors `_apply_comment_filters` and is wired into `list_reviews` before `_apply_view`. Validated via shellcheck, shfmt -i 2 -s, and live runs against PR #149: superseded counts match (9 claude reviews → 8 superseded; older `### Code Review: PR #149 — round` claude comments match under `--similar-prefix 30`); `--minimized=false --superseded` returns empty after a prior hide round (the iterative-loop contract); `--similar-prefix` without `--superseded` and bad integer values are rejected with the expected error messages. * feat(scripts/pr-comments-mgmt): add get-comment <id> Mirrors `get-thread` for issue-level (top-level) PR comments. Takes exactly one comment node id (e.g. `IC_kwDO...`) and emits a single JSON object with the same field shape as one element of `list-comments` (id, databaseId, author, body, createdAt, updatedAt, url, isMinimized, minimizedReason, viewerCan{Minimize,Update,Delete}). Validates `__typename == "IssueComment"`; on mismatch emits `get-comment: <id> is <X>, expected IssueComment` and exits 2. Accepts `--pr` for surface consistency with `get-thread` even though node ids are globally unique. Validated via shellcheck, shfmt -i 2 -s, and live calls against PR #149: a valid IssueComment node returns the expected object, a PullRequestReviewThread id is rejected with the typename mismatch error, an invalid id surfaces gh's "could not resolve" error, and zero / multiple positional ids both die at the dispatch precondition. * refactor(scripts/pr-comments-mgmt): build all NDJSON via jq directly Drop the `_json_string` helper and the `printf`-based JSON assembly in `err`, `log`, and `_bulk_summary` so every diagnostic record is built end-to-end by `jq -cn`. `_bulk_summary` keeps `ok`/`failed` numeric via `--argjson`. The bootstrap `jq` check stays plain text since it has to predate any jq invocation. * docs(scripts/pr-comments-mgmt): include body in OUTPUT_FORMAT docstring Doc drift: the file-level `OUTPUT_FORMAT` state-variable comment still listed only six formats (`json`/`ndjson`/`ids`/`text`/`full`/`tsv`) and named only `text/full/tsv` as per-verb dispatched. `body` was added to the validator, the help block, and `_format_array`'s "one of seven shapes" docstring in `0135c57` but the global was missed. Same shape fix as `fb61237` (which fixed the matching `tsv` drift after `2b52dac`): sync the docstring so the global is documented identically to the call site. Validation: bash -n, shellcheck, shfmt -i 2 -d all clean. No code behaviour changes; the only dispatch change is in the docstring text.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Expands
scripts/gh-cli/pr-comments-mgmt.shfrom a small thread-resolverinto a complete CLI surface for managing GitHub PR review threads and PR
metadata. Read verbs now return body, diff context, and thread metadata
required for triage; new write verbs cover threaded replies, the inverse
mutations, PR title/body edits, label management, issue-level comments,
and review submissions. A uniform
--prflag plus stdin-driven bulkpipelines and an NDJSON output mode replace the legacy positional
argument shapes.
New subcommands
Read:
get-thread <id>returns one thread with the same shape as oneelement of
list-threads.list-reviewsreturns paginated review summaries (state,body,author,submittedAt,url,commit { oid }).Thread mutations:
reply <thread-id> <body|--body|--body-file>posts a threaded replyvia
addPullRequestReviewThreadReply(cleaner thanaddPullRequestReviewComment: no first-comment lookup or PR idround-trip required).
unresolve <thread-id>...is the inverse ofresolveviaunresolveReviewThread.unhide-comment <comment-node-id>...is the inverse ofhide-commentvia
unminimizeComment.PR write:
set-title <text>andset-body <body|--body|--body-file>wrapgh pr edit.add-label,remove-label,set-labelscover label management;set-labelscomputes the add/remove diff against current labels andissues a single
gh pr edit.comment <body|--body|--body-file>posts an issue-level conversationcomment (the threaded counterpart is
reply).review --approve|--request-changes|--comment --body|--body-filesubmits a PR review;
--approvepermits an empty body, the othersrequire a non-empty body.
Enriched payloads
list-threadsnow includesbody,diffHunk,originalLine,originalStartLine, andsubjectTypeper comment, plussubjectType,isCollapsed,resolvedBy { login },viewerCanResolve,viewerCanUnresolve, andviewerCanReplyperthread.
current-prreturnsid,number,title,body,state,url,headRefName,baseRefName,author,isDraft,mergeable,mergeStateStatus, andlabelsflattened to a name array.New flags
--pr <number>or--pr <owner/repo>#<number>. Falls back tothe current branch's PR via
ghwhen omitted. Replaces the legacypositional
list-threads [<owner/repo>] [<pr-number>]form.--format json|ndjsononlist-threadsandlist-reviews(defaultjson). NDJSON emits one row per top-level node.list-threadsfilters:--unresolved,--outdated,--author <login>,--path <glob>,--minimized true|false. Allclient-side jq, no extra API round-trips. The
--pathglob accepts*(within a path segment),?(one non-/char), and**(zeroor more directory levels via
**/, one or more trailing levels via/**).resolve,unresolve,hide-comment,unhide-comment,hide-thread,add-label,remove-label) accept ids on stdin whenno positional ids are given, ignoring blank and
# ...commentlines, and emit a final NDJSON summary
{\"verb\":...,\"ok\":N,\"failed\":M}on stderr. They die withexit 1 when no ids land (positional + stdin both empty), so a closed
upstream pipe does not silently masquerade as success.
--body <text>and--body-file <path|->for any verb taking abody (
-reads stdin).Safety guards
set-labelsrefuses an empty desired set: clearing every label isirrecoverable from this CLI's perspective and is almost always an
upstream-pipe bug, not an intent. Use
remove-labelto drop labelsexplicitly.
set-titlerejects an empty title locally rather than round-trippingthe rejection through GitHub.
Non-goals / explicitly rejected
--dry-runfor mutating verbs. Will not be added. Each mutationis a single GraphQL or
ghround-trip whose effect is described bythe verb name and its arguments, the
set-labelsadd/remove diff isalready logged before the edit, and the inverse verbs (
unresolve,unhide-comment,gh pr edit --add-labelto undo a remove) coverrecovery for every destructive surface this CLI exposes. Threading
--dry-runthrough every mutation path would double the dispatchsurface and split the test matrix without removing a real failure
mode. Surface-level fail-fast guards (the
set-labelsempty-setrefusal,
set-titleempty-string rejection,_assert_processedonbulk verbs) cover the actual irrecoverable footguns.
Internal cleanups
SUBCOMMAND_FLAGS) replacesthe ad-hoc
--reasonrejection lines. Unknown flags fail with thematching subcommand name; missing allowlist entries fail loudly so
new subcommands cannot forget to register.
_gh_rundocuments that successful invocations intentionally dropgh's stderr to keep stdout JSON-clean._read_body,_collect_ids, and_bulk_summaryhelpersnormalize body sourcing, bulk-id input, and bulk-action reporting
across every verb.
graphql_callgains araw:argument prefix that forwards viagh -f(raw string) instead ofgh -F(typed), guarding bodiesthat happen to look like
true,null, or a number from beingsilently coerced.
(.x | tostring)rather than.x // \"null\"to avoid jq's//swallowingfalse.Test plan
bash -n scripts/gh-cli/pr-comments-mgmt.shshellcheck scripts/gh-cli/pr-comments-mgmt.shshfmt -i 2 -s -d scripts/gh-cli/pr-comments-mgmt.shtreefmt scripts/gh-cli/pr-comments-mgmt.sh--helprenders all new subcommands and flags.current-prreturns the full field list above.list-threadsreturns the enriched per-comment and per-threadfields against PR feat(scripts/pr-comments-mgmt): expand review-thread and PR-write CLI #149 (9 threads).
list-threads --format=ndjsonemits 9 rows;--unresolvednarrows to 2;
--outdatedto 1;--author Bad3rto 0;--author gemini-code-assistto 1;--path '*.sh'to 9;--path 'no/such/*'to 0;--minimized=falseto 9.list-reviews --format=jsonreturns an array of length 9.get-thread <id>against a known thread id matches onelist-threadselement shape.reply <id> --body 'ack'posts the reply, then a follow-uphide-comment <new-id>minimizes it for cleanup.unresolve <id>thenresolve <id>round-trip on a resolvedthread (after the
tostringfix).unhide-comment <id>thenhide-comment <id>round-trip on aminimized comment (after the
tostringfix).set-title,set-body --body-file -,add-label,remove-label,set-labels(no-op + drop+restore),comment --body 'smoke', andreview --comment --body 'smoke'againstPR feat(scripts/pr-comments-mgmt): expand review-thread and PR-write CLI #149. Title, body, and labels restored to original values
after the smoke run.
list-threads --format=ndjson | jq -r '.id' | resolvereportsok=7,failed=0against the 7 already-resolved threads onPR feat(scripts/pr-comments-mgmt): expand review-thread and PR-write CLI #149; empty stdin / closed pipe dies with exit 1
(
no ids supplied); comments and blank lines on stdin areskipped.
set-labelswith an empty desired set (no positionals, emptystdin) dies with exit 1 instead of silently dropping every
existing label.
set-title ""dies with exit 1 instead of round-tripping theempty title through
gh pr edit._glob_to_regexhandles**: 25 cases includingmodules/**/*.nix(matchesmodules/foo.nixthroughmodules/a/b/foo.nix, rejectsother/foo.nix),**/foo.sh(matches at any depth), and the path-segmenttightening for bare
*and?.