learn: gh --json merged is invalid; zsh word-split silently breaks id loops

[d-morrison]

Records two verified, cross-repo technical facts measured live on 2026-08-09 while working Morrison-Lab/wai#57.

memories/github.md

gh pr view --json merged fails with Unknown JSON field: "merged" (reproduced here against Morrison-Lab/wai#57 with gh 2.96.0). This was already documented in the existing bullet at line 100 (state/mergedAt as the fix under gh --json), so this is an update to that entry, not a new one.

The addition: the merged boolean itself is real, just not under gh --json's field spelling. Verified independently:

• gh api repos/Morrison-Lab/wai/pulls/57 --jq '{merged, merged_at, merged_by: .merged_by.login, state}' returns {"merged":true, ...} -- REST's single-PR endpoint carries a genuine merged boolean.
• memories/github-mcp-tools.md already documents that the GitHub MCP tool's pull_request_read get method reports merged correctly too (only list_pull_requests is the broken one, always false).

Cross-referenced the two entries so a reader hitting the gh --json error knows the field exists elsewhere.

memories/tools.md

Extends the existing "The Bash tool runs zsh here, and zsh does not word-split unquoted expansions" entry with a second variant. The existing entry's case is "nothing errors, every row of a report reads the same wrong value" (a [ "$2" = "MERGED" ] test going uniformly false). This is the sharper case: a stricter consumer (a GraphQL mutation expecting one node id) rejects the unsplit, newline-joined string outright and errors -- but a resolve-and-echo pattern inside the loop body can make that single failed call read as several partial successes in the transcript, since the error message itself named every id on its own line.

Reproduced the underlying mechanism directly in this session's own shell:

$ echo $SHELL
/bin/zsh
$ IDS=$'AAA\nBBB\nCCC'; n=0; for id in $IDS; do n=$((n+1)); echo "iter $n: [$id]"; done
iter 1: [AAA
BBB
CCC]

One iteration over the whole concatenated string, confirming the same root cause the existing entry already names.

Where each landed and why

• memories/github.md -- updates the existing gh pr view --json field-list entry (dupe-checked via grep -rniE "gh pr view.*--json|--json merged|Unknown JSON field|mergedAt|mergeCommit" across memories/ and shared/).
• memories/tools.md -- extends the existing zsh word-split entry (dupe-checked via grep -rniE "word.split|word-split|IFS\b|zsh.*split|unquoted.*expansion|for id in \$" across memories/ and shared/; found the exact section at memories/tools.md:570).

Both files checked against scripts/check-memory-file-size.py's 1200-line ceiling (github.md: 1075, tools.md: 1052 -- both well under; re-derived at head 6838e31 after the round-1 prose fix, which moved github.md by 3 lines).

Pre-push checks run (after committing, three-dot range against origin/main, per this corpus's own diff-scoped-check ordering)

• check-new-line-breaks.py (from a local d-morrison/gha checkout, since it was retired from this repo's own scripts/ per ai-config#703): flagged one compound sentence in github.md, fixed, re-ran clean.
• Banned-punctuation scan (em/en-dash, curly quotes, multiplication sign) over added lines: 0 hits.
• Deleted-lines review: 0 -- purely additive.
• python3 scripts/check-memory-file-size.py: no memory file exceeds 1200 lines.
• Column-1 # heading-collision scan over added lines: 0 hits.
• python3 scripts/validate-skills.py: clean (181 skills, 181 Codex wrappers in sync).
• python3 scripts/check-links.py: 0 broken relative links (confirmed the new [github-mcp-tools.md](github-mcp-tools.md) link resolves).
• npx markdownlint-cli2@0.22.1: 0 errors.

What in the framing turned out to be wrong when checked

Nothing in the framing was wrong. One thing was worth independently re-verifying rather than trusting as reported (per this corpus's own metacognitive-monitoring conventions): I reproduced both underlying mechanisms myself rather than only transcribing the report -- the gh --json merged error directly against wai#57, the REST merged boolean directly against the same PR, and the zsh non-word-splitting directly in this session's own shell. All three matched what was reported. I also confirmed wai#57 is real (docs/offline-agent-corrections, merged) rather than assuming the PR number cited in the framing was accurate.

[d-morrison]

Working on this --- paws off until I'm done.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Claude finished review — View run

---

Based on my independent verification of this re-review round, here is my complete review.

Code Review: PR #1371

Context: This is round 2 of review. The prior round (posted 2026-08-10T01:42:49Z) found the diff technically sound but flagged one non-blocking ambiguity: the pronoun "it" in "the GraphQL field it wraps" (memories/github.md, then at L101-L103) had no unambiguous antecedent. Verdict was Ready for merge with that one nit noted.

What changed since: A new commit (6838e31, pushed 2 minutes after the review was posted) directly addresses that finding — commit message: "Review finding on #1371: 'it' had no unambiguous antecedent... Per shared/writing/ambiguous-reference.md, the remedy is to name the referent rather than reword around the pronoun."

Verification of the fix:
The sentence now reads:

REST's GET /repos/{owner}/{repo}/pulls/{number} and the GraphQL PullRequest.merged field each carry a genuine merged boolean, verified 2026-08-09 against Morrison-Lab/wai#57: ... returns true, and ... returns true as well.

This names both referents explicitly instead of using "it" to bridge them, which resolves the flagged ambiguity — the "wrap" framing that made the pronoun's antecedent unclear is dropped entirely rather than reworded around, per the author's own note that the "wraps" claim about gh's internals wasn't load-bearing and hadn't been verified. That's the correct fix per this repo's own shared/writing/ambiguous-reference.md: "The remedy: replace the pronoun with the noun." No new ambiguity was introduced by the fix.

I independently re-verified the substantive technical claims in the full diff (not just the changed sentence), since a prose fix can accidentally break something adjacent:

• gh pr view --json merged still fails with Unknown JSON field: "merged", and the printed field list matches exactly (state, mergedAt, mergedBy, mergeCommit, closed, closedAt, no bare merged) — reproduced live.
• Morrison-Lab/wai#57 is real and merged; confirmed via public REST API that .merged is true for that PR.
• The cross-reference to memories/github-mcp-tools.md's claim that list_pull_requests always reports merged: false while pull_request_read's get method reports it correctly — confirmed accurate against the current file (line 284's measured table).
• The memories/tools.md:570 dupe-check citation is accurate — that's exactly the heading of the existing "Bash tool runs zsh here" entry being extended.
• The PRRT_ GraphQL node-ID prefix used in the illustrative error message matches this corpus's own documentation of real GitHub GraphQL thread IDs (memories/github-mcp-tools.md), so it's not a fabricated ID format.
• No banned punctuation (em/en-dash, curly quotes, multiplication sign) anywhere in the diff — confirmed with a regex scan.
• The [github-mcp-tools.md](github-mcp-tools.md) relative link resolves.
• No duplication found elsewhere in the corpus for the new "resolve-and-echo"/zsh-loop content.
• Both files stay well under the 1200-line check-memory-file-size.py ceiling (confirmed: base is 1069/995 lines, PR adds 6/57, so 1075/1052 post-merge — both comfortably under 1200; the PR body's self-reported "1072" for github.md is off by a few lines relative to my count, almost certainly a stale figure from an earlier round before the intervening fixup: commit, but the substantive claim it supports — "well under 1200" — is still true either way, and this is a PR-description detail rather than shipped corpus content).

No new findings from this round's fix, and nothing else in the diff (memories/tools.md, untouched by the fix commit) needed re-checking since it was already verified clean in the prior round.

Verdict

Ready for merge — the one non-blocking finding from the prior review round has been correctly addressed (pronoun replaced with named referents, per this repo's own ambiguous-reference guidance), and no new issues were introduced by the fix or found elsewhere in the diff.

[github-actions]

💰 Cost: $7.7023 (review) — run