Conversation
Last git-cluster port: tools/git/batch-resolve-pr-threads.{sh→ts}.
Slice 13 (push-with-retry) + slice 20 (batch-resolve-pr-threads)
together complete the git-cluster.
Behavioural improvements over bash original (deliberate, not drift):
- jq pipelines replaced with native JSON parse + array operations;
drops jq from runtime deps (only `gh` required now).
- Pattern classification via TypeScript pattern arrays + `.some()`
rather than bash `for pat in ; do [[ ]]; done` — same semantics,
more type-safe with `Classification = "dangling-ref" |
"name-attribution" | "unknown"` discriminated union.
- ResolveError discriminated record with `stage: "reply" | "resolve"`
so failure mode is visible in error messages.
Byte-equivalence verified on this repo state:
- Three argument-validation paths byte-equivalent (no args / bad
pr-number / unknown second arg) — same exit code + same message.
- Live dry-run on PR #902 (4 unresolved threads) byte-equivalent —
empty diff against bash original.
- Apply mode not exercised (would mutate live PR state); code path
verified by inspection.
All bash safety rails from PR #199 (Copilot/Codex) preserved:
positive-integer validation, exact-`--apply` second-arg check,
GraphQL errors array inspection, null-pullRequest detection,
paginated thread fetch (50 per page) + paginated per-thread comments
(50 per thread with truncation warning), positional -F args
(avoids parameter-expansion-quote pitfall), printf-not-echo for
review-comment bodies (handled via TypeScript string operations).
Bucket B 2 → 1 (only tools/pr-preservation/archive-pr.sh 674L
remains; bash+Python mix — slice 21).
Composes with:
- tools/git/push-with-retry.ts (slice 13, #892) — git-cluster siblings
- docs/trajectories/typescript-bun-migration/RESUME.md updated
- docs/trajectories/typescript-bun-migration/slice-audits.md slice 20 audit
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c0b0c28ac
ℹ️ 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".
| if (result.status !== 0) { | ||
| return { | ||
| error: `error: GraphQL fetch failed for PR #${String(args.prNumber)}`, | ||
| exitCode: 1, |
There was a problem hiding this comment.
Propagate gh stderr on GraphQL fetch failures
When gh api graphql exits non-zero, this path returns only a generic message and drops result.stderr. Since spawnSync pipes child stderr by default, auth/scope/rate-limit diagnostics from gh are swallowed here, which makes automation failures much harder to triage than the previous shell behavior that surfaced those errors directly. Include the captured stderr (or use inherited stderr) so operators can distinguish environment/token issues from script defects.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Ports the last remaining “git-cluster” script from Bash to TypeScript+Bun as part of the ongoing TS/Bun migration (B-0086), preserving the PR-thread classification + optional resolve workflow while dropping the runtime jq dependency.
Changes:
- Added
tools/git/batch-resolve-pr-threads.tsas a TS/Bun port of the bash script, including GraphQL pagination, thread classification, and apply-mode mutations. - Appended Slice 20 audit notes to the TS/Bun migration slice audit log.
- Updated the TS/Bun migration trajectory dashboard to reflect Slice 20 being in flight and Bucket B’s updated remaining work.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tools/git/batch-resolve-pr-threads.ts | New TS/Bun implementation of batch PR-thread classification + optional resolution via gh api graphql. |
| docs/trajectories/typescript-bun-migration/slice-audits.md | Adds Slice 20 audit entry describing the port and equivalence notes. |
| docs/trajectories/typescript-bun-migration/RESUME.md | Updates trajectory status/milestone text to reflect Slice 20 in-flight and the remaining Bucket B work. |
| // eslint-disable-next-line sonarjs/no-os-command-from-path | ||
| const result = spawnSync("gh", ["repo", "view", "--json", "owner,name"], { | ||
| encoding: "utf8", | ||
| maxBuffer: SPAWN_MAX_BUFFER, | ||
| }); | ||
| if (result.status !== 0) { | ||
| return { | ||
| error: "error: could not detect repo via 'gh repo view'. Run inside a repo with a GitHub remote.", | ||
| exitCode: 1, | ||
| }; | ||
| } | ||
| const parsed = JSON.parse(result.stdout) as RepoViewResponse; | ||
| const owner = parsed.owner?.login ?? ""; | ||
| const name = parsed.name ?? ""; | ||
| if (owner.length === 0 || name.length === 0) { | ||
| return { error: "error: could not parse repo owner/name from gh repo view", exitCode: 1 }; | ||
| } | ||
| return { owner, name }; | ||
| } |
| // eslint-disable-next-line sonarjs/no-os-command-from-path | ||
| const result = spawnSync("gh", ghArgs, { | ||
| encoding: "utf8", | ||
| maxBuffer: SPAWN_MAX_BUFFER, | ||
| }); | ||
| if (result.status !== 0) { | ||
| return { | ||
| error: `error: GraphQL fetch failed for PR #${String(args.prNumber)}`, | ||
| exitCode: 1, | ||
| }; | ||
| } | ||
| return JSON.parse(result.stdout) as ReviewThreadsResponse; | ||
| } |
| 'Acknowledged and accepted during Phase 1 queue-drain (per the "merge over invent" operational-gap-assessment direction from the 2026-04-23 round). Referenced artifacts are in-flight across adjacent PRs; cross-PR dangling refs are a known side-effect of stacked-PR state and self-heal as the queue drains. Resolving to unblock merge; opportunistic cleanup of any permanent refs in follow-up tick if gaps remain visible after queue drain.'; | ||
|
|
||
| const REPLY_NAME_ATTRIBUTION = | ||
| "Acknowledged; the name appearance here is legitimate per the named-agents-get-attribution policy (see `memory/CURRENT-aaron.md` attribution table + `docs/EXPERT-REGISTRY.md` persona roster). Named personas are factory-level attribution surfaces; their names in ADRs / config / collaborator registries are the factory's structural record of who contributed what. Resolving; the name-attribution rule applies to personal human names outside persona-scope, not to persona names in structural attribution contexts."; |
|
|
||
| ### Code-pattern audit (per-port) | ||
|
|
||
| - **`batch-resolve-pr-threads.ts`** (390 → 415 lines): bash GraphQL pagination loop preserved 1:1 — same `first: 50, after: $cursor` shape, same `pageInfo.hasNextPage`/`endCursor` termination. Bash `gh api graphql -F owner=$x -F name=$y ...` shape preserved verbatim via `spawnSync("gh", ["api", "graphql", "-F", "owner=...", ...])` — positional `-F` args avoid the bash parameter-expansion-quote pitfall and the TS shape avoids the same string-concat-into-GraphQL footgun. jq pipelines (`[.comments.nodes[].body] | join("\n---\n")`) become typed `commentNodes.map(c => c.body ?? "").join("\n---\n")`. Pattern classification splits into three pattern arrays (DANGLING_REF_PATTERNS / NAME_ATTRIBUTION_DIRECT_PATTERNS / NAME_ATTRIBUTION_FUZZY_NAME × NAME_ATTRIBUTION_FUZZY_RULE) — same shape as the bash `for pat in "..." "..." ; do [[ ]]; done` loops; the fuzzy-combination check uses `.some()` × 2 rather than the bash `&&` of two `||`-OR groups. Same conservative semantics: unknown threads left unresolved. |
| return { | ||
| error: "error: could not detect repo via 'gh repo view'. Run inside a repo with a GitHub remote.", |
…Bun migration (#908) * ts(B-0086): port 1 pr-preservation script (.sh→.ts) — slice 21 of TS/Bun migration Last Bucket B file: tools/pr-preservation/archive-pr.{sh→ts}. After this PR merges, **Bucket B is empty** — every file flagged for TS port has been ported. The trajectory transitions from "porting" phase to "soak + bash retirement" phase. Behavioural improvements over bash original (deliberate, not drift): - Drops Python from runtime deps entirely. Bash original was 217 lines bash + ~457 lines embedded Python (GraphQL fetcher + Markdown formatter); TS port collapses both into single Bun runtime. No more bash/Python boundary, no more mktemp + trap cleanup, no `set +e` to capture Python exit code. - Native JSON parsing replaces all jq/Python json shells. - Generic `paginateTopLevel<T>` helper with type-safe extractor handles the cursor loop for all 3 top-level connections (reviewThreads/reviews/comments). `paginateThreadComments` handles the per-thread case. - `detectFenceMarker` preserves CommonMark §4.5 fence rules strictly: leading-space-count ≤ 3 + no tab in prefix; closing fence same marker char + length ≥ opener. This matches the Python original's nuanced fence detection (Otto-241 etc). - `yamlQuote` post-processes `JSON.stringify` output to escape non-ASCII codepoints as \uXXXX, matching Python's `json.dumps(ensure_ascii=True)` wire-format default. Without this, titles with → / — would diverge from bash output. Byte-equivalence verified on this repo state: - 2 argument-validation paths byte-equivalent (no args / bad PR number) — same exit code 1 + same message. - Live archive run on PR #902 (4 threads, 2 reviews, 0 comments) byte-equivalent EXCEPT `archived_at` (timestamp) + `archive_tool` (.sh vs .ts — deliberate self-reference). Title with non-ASCII chars escapes correctly via the yamlQuote fix. All bash safety rails preserved: positive-integer PR validation, GH_REPO env-var preference + `gh repo view` fallback, 2/3-segment NWO parsing with Enterprise HOST validation (dot required), slash-injection defence on owner/name, paginated GraphQL fetch (top-level + per-thread), GraphQL `errors` array inspection, null-pullRequest detection, idempotent archive path via PR-<NNNN>-* glob (Otto-235), CommonMark §4.5 fence detection (Otto-241), trailing-newline-only rstrip (preserves two-space markdown hard-line-breaks). Bucket B 1 → 0. Bucket C: 2 (gh-api-heavy scripts pending maintainer decision). Bucket A: 14 (stays bash by design). Composes with: - tools/git/batch-resolve-pr-threads.ts (slice 20, #907) — same GraphQL pagination shape - docs/trajectories/typescript-bun-migration/RESUME.md updated - docs/trajectories/typescript-bun-migration/slice-audits.md slice 21 audit appended Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(ts-bun-audits): MD038 — rephrase to avoid leading-space inside code spans * fix(ts-bun-audits): address #908 review threads — line counts + ensure_ascii framing + usage-line carve-out Round-1 fixes for PR #908 (4 Copilot P1 threads): - Line-count drift (Copilot P1): I claimed "674 → 590" but actual is "674 → 806". Updated and added a one-line note explaining why TS is larger than bash (explicit type interfaces replace Python's untyped dict navigation). - ensure_ascii=True framing was wrong (Copilot P1, twice): Python's json.dumps with ensure_ascii=True does NOT emit literal `→`/`—` — it emits \uXXXX escapes. My audit prose said "Python escapes to `→`" which contradicted both the byte-equivalence goal and what yamlQuote actually implements. Rewrote to clearly say Python emits \uXXXX form (e.g. → for right-arrow, — for em-dash) and the TS yamlQuote post-processes JSON.stringify to match. - Usage-line not byte-equivalent (Copilot P1, twice): bash echoes `$0` showing the actual `./tools/...sh` path; TS hard-codes `bun tools/...ts` so the user sees the form they should run. Reframed equivalence claim to be honest: same exit code + same error-body, but usage-line script-path is intentionally NOT byte-equivalent — same carve-out as the `archive_tool` YAML self-reference. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Slice 20 of the TS/Bun migration (B-0086). Ports
tools/git/batch-resolve-pr-threads.sh(390 lines bash) →tools/git/batch-resolve-pr-threads.ts(415 lines TS). Last git-cluster port — slice 13 (push-with-retry) + slice 20 together complete the cluster.Behavioural improvements (deliberate, not drift)
JSON.parsereplaces all jq pipelines). Now onlyghis probed..some()— same semantics as bashfor pat in ; do [[ ]]; done, with type-safeClassification = "dangling-ref" | "name-attribution" | "unknown"discriminated union.ResolveErrordiscriminated record —stage: "reply" | "resolve"makes failure mode visible in error messages.All bash safety rails from PR #199 review-cycle preserved: positive-integer pr-number validation, exact-
--applysecond-arg check, GraphQL errors array inspection, null-pullRequest detection, paginated thread fetch (50 per page) + paginated per-thread comments (50 per thread with truncation warning), positional -F args, byte-equivalent reply templates.Verification
diff <(bun ...) <(./...sh)emptybun --bun tsc --noEmit -p tsconfig.jsoncleanTest plan
lint (tsc tools)gate passesComposes with
tools/git/push-with-retry.ts(slice 13, ts(B-0086): port 1 git script (.sh→.ts) — slice 13 of TS/Bun migration #892) — git-cluster siblingsdocs/trajectories/typescript-bun-migration/RESUME.md— updated to reflect Bucket B 2 → 1docs/trajectories/typescript-bun-migration/slice-audits.md— slice 20 audit appendedtools/pr-preservation/archive-pr.sh(674L, bash+Python mix — last Bucket B file)🤖 Generated with Claude Code