Skip to content

Add git-merge-pull-request LLM merge gate to gc-toolkit pack (tk-9obwui)#80

Open
zook-bot wants to merge 5 commits into
mainfrom
polecat/tk-9obwui
Open

Add git-merge-pull-request LLM merge gate to gc-toolkit pack (tk-9obwui)#80
zook-bot wants to merge 5 commits into
mainfrom
polecat/tk-9obwui

Conversation

@zook-bot

Copy link
Copy Markdown
Contributor

Summary

Fold the operator's "merge this PR" LLM validation into the gc-toolkit pack as a Claude skill.

DELIVERABLE: add skills/git-merge-pull-request/SKILL.md to the gc-toolkit pack.
The authored content is ready at this absolute path -- copy it VERBATIM (do not
rewrite; the wording is the deliberate output of a mayor-thread design session):
/home/zook/loomington/.gc/agents/mayor-thread/gc-toolkit.mayor-thread-adhoc-1c3ea2db48/git-merge-pull-request.SKILL.md

WHAT IT IS: an LLM merge gate. The merge actor is an LLM that, immediately before
merging and against the EXACT head being merged, re-validates: (1) mergeable + not
draft, (2) CI green, (3) human approval is on this head, (4) codex review resolved
on this head, (5) title + description still accurately describe the final diff. On
title/description drift it fixes the wording and merges (fix-and-merge -- safe; it
only makes wording match what was already approved to ship). Squash-only.
Human-invoked ("merge this PR").

WHY: GitHub owns approval + CI; Gas City owns "is this correct to ship." The
refinery (mol-refinery-patrol.toml) already hands off mr-mode PRs at "PR ready"
(opens draft, dispatches codex review, closes the work bead) and never auto-merges,
so NO formula behavior change is needed -- this skill fills the unguarded space
after "PR ready."

STEPS:

  1. Branch from origin/main (e.g. feat/tk-9obwui-merge-gate-skill).
  2. Copy the authored file to skills/git-merge-pull-request/SKILL.md verbatim.
  3. SKILL FILE ONLY -- do NOT modify mol-refinery-patrol.toml (no behavior change).
  4. Commit (signed), push, open a PR to main on zookanalytics/gc-toolkit with a body summarizing the above.
  5. Request Codex review. Do NOT self-merge.

Implementation notes

Implemented: added skills/git-merge-pull-request/SKILL.md (LLM merge gate) verbatim from the mayor-thread design. Skill file only; mol-refinery-patrol.toml unchanged. merge_strategy=pr so refinery opens a draft PR to main and dispatches codex review (no self-merge). Branch polecat/tk-9obwui @ 1c3fbc8, signed.

Refinery handoff

  • Issue: tk-9obwui (task, P2)
  • Source branch: polecat/tk-9obwui
  • Target: main
  • Rebased on main via Gastown Refinery.

Add skills/git-merge-pull-request/SKILL.md to the gc-toolkit pack: a
human-invoked LLM merge gate for "merge this PR".

The merge actor is an LLM that, immediately before merging and against the
exact head being merged, re-validates: mergeable + not draft, CI green,
human approval on this head, codex review resolved on this head, and that
the title + description still accurately describe the final diff. On
title/description drift it fixes the wording and squash-merges
(fix-and-merge — safe; only makes wording match what was already approved
to ship). Squash-only.

GitHub owns approval + CI; Gas City owns "is this correct to ship." The
refinery (mol-refinery-patrol) already hands off mr-mode PRs at "PR ready"
(opens draft, dispatches codex review, closes the work bead) and never
auto-merges, so no formula behavior change is needed — this skill fills the
unguarded space after "PR ready." Skill file only; mol-refinery-patrol.toml
is unchanged.
@zook-bot

Copy link
Copy Markdown
Contributor Author

Codex review verdict: REQUEST_CHANGES

Findings:

  • Blocking: skills/git-merge-pull-request/SKILL.md:62 treats COMPLETED as an acceptable green CI state. For GitHub check runs, COMPLETED only means the run has reached a terminal state; failed, cancelled, timed-out, and action-required runs are also completed, with the actual pass/fail result carried by conclusion. Because this skill is the last merge gate, following the wording literally can allow a PR with failed CI to be squash-merged. Please make the gate require StatusContext state == SUCCESS and CheckRun conclusion == SUCCESS or legitimate SKIPPED, and explicitly block any completed check run with a non-success conclusion.

Verification:

  • Confirmed the PR diff adds only skills/git-merge-pull-request/SKILL.md.
  • Compared the PR file to the provided mayor-thread source path; no diff.
  • Ran git diff --check origin/main...origin/polecat/tk-9obwui; no whitespace errors.
  • gh pr checks 80 reports no checks on the branch.

Note: GitHub would not accept a formal request-changes review from zook-bot because this PR is authored by the same account, so this verdict is posted as a PR comment instead of a review object.

…(tk-9obwui)

The git-merge-pull-request CI-green check (step 2) treated `COMPLETED` as a
pass. For GitHub CheckRuns, `COMPLETED` is only the terminal status — failed,
cancelled, timed-out, and action-required runs are also COMPLETED, with the
real result carried in `conclusion`. As the last merge gate, following the
literal wording could let a PR with failed CI squash-merge.

Rewrite step 2 to gate per node type:
- CheckRun: require `conclusion == SUCCESS` (or legitimate SKIPPED); block any
  completed run with a non-success conclusion; in-progress runs fail closed.
- StatusContext: require `state == SUCCESS`.

Adds a runnable jq probe that lists any non-green check so the gate is clear
only when it prints nothing.

Addresses codex review on PR#80 (fix bead tk-j7oacl).

@zook-bot zook-bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking review finding; posting as COMMENT because GitHub does not allow zook-bot to request changes on its own PR.

Finding:

  • skills/git-merge-pull-request/SKILL.md:111 - The codex-review gate only checks for currently open/in_progress review beads. If no review bead exists for the PR, or if the only review bead was closed against an older head, this command returns [] and the skill can proceed even though the stated requirement is "codex review resolved on HEAD_SHA." Since this skill is the final merge gate, that lets an unreviewed or stale-reviewed PR pass the gate. Please make this step require evidence of a codex review for the current HEAD_SHA (for example by checking GitHub review commit_id or durable review metadata) and treat no matching current-head review as blocking.

Verification: git diff --check origin/main...origin/polecat/tk-9obwui passed.

Step 4 of the git-merge-pull-request gate only listed open/in_progress
review beads and proceeded when empty. The codex reviewer closes its own
bead after posting, so that query is empty in the common case — and is
indistinguishable from "never reviewed" and "reviewed an older head
(stale)". As the final merge gate, this let an unreviewed or stale-reviewed
PR pass.

Bind the check to positive current-head evidence: cross-reference the
durable review_id recorded on the review bead against the PR's GitHub
reviews and require a codex review whose commit_id == HEAD_SHA. Fail closed
when no review_id was recorded (require a PR review on HEAD_SHA plus a
dispatched review bead) and on any missing/stale evidence. Add a
rationalizations row codifying the lesson.

Addresses codex review 4395643265 on PR#80 (tk-2ugkzj).

@zook-bot zook-bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking review finding; posting as COMMENT because GitHub does not allow zook-bot to request changes on its own PR.

Finding:

  • skills/git-merge-pull-request/SKILL.md:205 - The final merge command validates a pinned HEAD_SHA but does not pass --match-head-commit "$HEAD_SHA" to gh pr merge. If the PR head moves after the gate captures and validates HEAD_SHA but before the merge command runs, GitHub can squash-merge the new unvalidated head. That bypasses the exact-head CI, approval, codex-review, and title/body checks this skill is meant to enforce. Please bind the merge command to the pinned SHA, e.g. gh pr merge <number> --squash --delete-branch --match-head-commit "$HEAD_SHA", and treat a mismatch as a signal to restart from "Pin the head."

Verification:

  • Reviewed the full single-file diff in skills/git-merge-pull-request/SKILL.md.
  • git diff --check origin/main...HEAD passed.
  • gh pr checks 80 reports no checks on the branch.
  • gc lint . was attempted but fails before validating this skill because the worktree pack import resolves //.gc/system/packs/gastown under the worktree and the existing keeper prompt references a missing watch-dispatched-work template.

…k-9obwui)

The final merge command validated a pinned HEAD_SHA but never passed it to
`gh pr merge`, so a head landing in the gap between the last check and the
merge could be squash-merged unvalidated — bypassing the CI, approval,
codex-review, and title/body gates the skill exists to enforce.

Capture HEAD_SHA as a real shell variable in "Pin the head" and bind the
merge with `--match-head-commit "$HEAD_SHA"`. On a head-mismatch rejection,
restart from "Pin the head" and re-validate rather than retrying the new
head. Documents the new failure mode in the troubleshooting list and the
rationalizations table.

Addresses codex review finding on PR#80.

@zook-bot zook-bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex review verdict: COMMENT

No blocking findings on the current head 9e49e38731d6e230acbb71b1d12ba80149161bca.

Verification:

  • Reviewed the full single-file diff in skills/git-merge-pull-request/SKILL.md.
  • Confirmed the prior blocking issue is addressed: the final merge command now uses --match-head-commit "$HEAD_SHA", with instructions to re-pin and re-validate on a head mismatch.
  • git diff --check origin/main...HEAD passed.
  • gh pr checks 80 reports no checks on the branch.
  • gc lint . was attempted but still fails before validating this skill because the worktree pack import resolves /.gc/system/packs/gastown under the worktree and the existing keeper prompt references a missing watch-dispatched-work template.

@zook-bot
zook-bot marked this pull request as ready for review May 30, 2026 17:08

@zook-bot zook-bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking review finding; posting as COMMENT because GitHub does not allow zook-bot to request changes on its own PR.

Finding:

  • skills/git-merge-pull-request/SKILL.md:150 - The codex-review gate only proves that a recorded codex review_id exists on HEAD_SHA; it never checks the review bead's metadata.verdict or the GitHub review state. A codex review can close with verdict=REQUEST_CHANGES and, for bot-authored PRs, still appear on GitHub as a COMMENTED review because GitHub rejects formal self request-changes. Under the current wording, that review still satisfies "a recorded codex review_id must appear with commit_id == HEAD_SHA", so the merge gate can treat blocking codex findings as resolved and proceed once the PR is undrafted. Please require the matching current-head codex review bead to have a non-blocking verdict (APPROVE or non-blocking COMMENT) and block any matching REQUEST_CHANGES/CHANGES_REQUESTED verdict or unresolved fix bead.

Verification: git diff --check origin/main...origin/pr/80 passed; the documented CI jq probe ran cleanly on PR #80; gc lint . is still blocked by existing pack import/template issues unrelated to this single-file change.

@zook-bot
zook-bot marked this pull request as draft May 31, 2026 01:22
…k-9obwui)

The codex-review gate (step 4) only confirmed a recorded review_id landed
on HEAD_SHA; it never checked the review's verdict. A REQUEST_CHANGES
verdict on a bot-authored PR is downgraded by GitHub to a COMMENTED review
(GitHub rejects a formal self request-changes), so a blocking codex review
satisfied the old "a review_id appears with commit_id == HEAD_SHA" check and
the merge gate could ship unresolved blocking findings.

Bind the gate to the durable bead record, not the GitHub state:
- require the matching current-head review bead's metadata.verdict to be
  non-blocking (APPROVE or COMMENT) on HEAD_SHA;
- block any REQUEST_CHANGES/CHANGES_REQUESTED verdict (even when its
  github_review_state shows the downgraded COMMENTED), the GitHub
  CHANGES_REQUESTED state (human-authored PRs), or any open/in_progress
  fix bead (metadata.source_review_bead) still working the findings;
- fail closed when no verdict is recorded; add a rationalizations row.

Addresses codex finding on PR#80 (fix bead tk-nh5r2i, source review tk-h13zwe).

@zook-bot zook-bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex review verdict: COMMENT

No blocking findings on the current head f223fcb62afc78d36b972f1d3ce5a715ccbdeb05.

Confirmed the four prior codex findings are resolved:

  • CI gate now distinguishes CheckRun conclusion from status and blocks non-success terminal conclusions.
  • Codex review gate no longer treats an empty open/in-progress bead query as evidence of review; it requires a current-head review record.
  • Final squash merge is bound to the pinned HEAD_SHA with --match-head-commit.
  • Codex review gate now requires a non-blocking bead verdict on the reviewed head, blocks REQUEST_CHANGES/CHANGES_REQUESTED and open fix beads, and fails closed when verdict evidence is missing.

Verification:

  • Reviewed the full added file skills/git-merge-pull-request/SKILL.md and the latest f223fcb patch.
  • git diff --check origin/main...HEAD passed.
  • gh pr checks 80 reports no checks on the branch.
  • gc lint . still fails before validating this skill due existing unrelated pack import/template issues: missing worktree-local .gc/system/packs/gastown/pack.toml and undefined watch-dispatched-work template.

@zook-bot
zook-bot marked this pull request as ready for review May 31, 2026 10:43

@zook-bot zook-bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex review verdict: COMMENT

No blocking findings on the current head f223fcb62afc78d36b972f1d3ce5a715ccbdeb05.

Confirmed the final skills/git-merge-pull-request/SKILL.md still addresses the prior merge-gate defects:

  • CI gate distinguishes CheckRun conclusion from terminal status and blocks non-success terminal results.
  • Codex review gate requires current-head evidence instead of treating an empty open-review query as success.
  • Final squash merge is bound to the pinned HEAD_SHA with --match-head-commit.
  • Codex review gate requires a non-blocking durable bead verdict and blocks request-changes/fix-bead states.

Verification:

  • Reviewed the full added file and the final five-commit diff for PR #80.
  • Verified the current PR head matches the review bead metadata: f223fcb62afc78d36b972f1d3ce5a715ccbdeb05.
  • git diff --check origin/main...origin/pr/80 passed.
  • gh pr checks 80 reports no checks on the branch.
  • gc lint . is still blocked before validating this skill by existing unrelated pack issues: missing worktree-local .gc/system/packs/gastown/pack.toml and undefined watch-dispatched-work template.

@johnzook johnzook left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This skill will pollute all contexts. tk-24gdht to address

zook-bot added a commit that referenced this pull request Jul 21, 2026
…one (tk-vpx62)

Second arm of tk-vpx62 (mayor scope extension, 2026-07-21). The first arm
handled a PR whose anchor is stranded; this one handles a PR whose anchor is
GONE.

Every close-on-land path — merge-skill.sh, this reconciler, the refinery patrol
— enumerates from the BEAD and reads its pr_number. Sound while the bead
outlives the PR, which close-on-land guarantees. But a PR whose bead is CLOSED
is not merely unhandled, it is unseen: no scan starts anywhere that reaches it,
so it never lands, never escalates, never times out. It does not read as broken,
it reads as absent. The pre-#163 close-on-publish model created that state by
design, and the 7 PRs it stranded (#171 #170 #166 #108 #88 #80 #77) sat
untouched for weeks — found only by a human cross-checking `gh pr list` against
the ledger by hand.

So reconcile from the other side too: enumerate open PRs, subtract every PR
number any LIVE bead references, report the remainder. Verified against the live
ledger: flags exactly those 7 and correctly excludes in-flight PR#195.

Detection and surfacing ONLY, per the mayor's scope note — it never merges,
closes, or reopens an anchorless PR. Disposition stays an operator call.

Convergence, since a patrol pass runs on every idle wake:
- Escalate once, bounded by an anchorless_flagged marker written to the closed
  bead BEFORE the mail (stamp-first, mirroring the merged path's close-first).
  A failed stamp means no bound, so it does not mail — a delayed escalation is
  recoverable, a mail storm is not.
- A PR with NO bead in any state is logged but never mailed: nothing durable
  exists to record that we already said it.
- A failed ledger read is empty, not "[]", and the scan fails CLOSED on it —
  treating "could not read" as "nothing is tracked" would flag every open PR at
  once.
- Anchor resolution picks the OLDEST bead carrying merge_result, so the operator
  is pointed at the bead that OPENED the PR rather than a review bead or a later
  "address findings" child (several closed beads routinely name one PR); the
  mail lists the whole set regardless.

Zero gating anchors is no longer an early exit — that was precisely the state
where this scan matters most (no live anchors WITH open PRs is the stranded
shape).

Tests: 24 new hermetic assertions (62 total, all pass); verified 13 of them FAIL
against the pre-arm script while all 38 pre-existing ones still pass. Sibling
suites green (merge-skill 36, one-anchor-per-pr 13, pre-open-resolve 16,
signoff-anchor-resolution 14, signoff-anchor-failclosed 14,
reconcile-graduated-convoys 14, reconcile-rig-checkouts 8, husk-guard 16).
bash -n + shellcheck clean at warning severity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants