fix(nightly): test-merge bot PRs locally instead of filtering the lazy mergeable field - #898
Conversation
…KNOWN as clean Closes #897
tend-agent
left a comment
There was a problem hiding this comment.
The lazy-mergeable diagnosis is right and the retry loop settles it correctly — I ran the recipe verbatim from the repo root and it exits on the first pass with a warm cache, writes to sanitized paths, and reports #856 as CONFLICTING, matching the PR body.
One gap: the redirect makes a failed query indistinguishable from a clean one, which is the same shape as the bug being fixed. > "$out" truncates the file before gh writes, so any non-zero gh pr list (rate limit, transient 5xx, wrong working directory) leaves $out empty. jq on empty input emits nothing, so $(...) is the empty string and [ "" -eq 0 ] errors with integer expression expected instead of breaking — the loop burns all five iterations, then both reporting jq calls print nothing. The step then reads as "every PR settled, none conflicting" and gets skipped. I reproduced this by running the recipe with gh unable to resolve the repo: 2m30s per author, no output, exit 0.
[] (3 bytes) is non-empty while a failed query leaves 0 bytes, so [ -s "$out" ] separates "this author has no open PRs" from "the query never landed" without an extra call. Suggestion inline.
| gh pr list --author "$author" --json number,title,mergeable,headRefName,author \ | ||
| --jq '.[] | select(.mergeable == "CONFLICTING")' | ||
| out="/tmp/prs-${author//\//-}.json" # `app/dependabot` has a slash; strip it | ||
| for _ in 1 2 3 4 5; do |
There was a problem hiding this comment.
is there a way to do this with a function of "wait for the answer", rather than repeating for 5 steps?
There was a problem hiding this comment.
There isn't one on the GitHub side — mergeable has no blocking read, and the REST docs say so explicitly: "If the value is null, then GitHub has started a background job to compute the mergeability. After giving the job time to complete, resubmit the request." Polling is the documented interface.
But we don't have to ask GitHub. git merge-tree --write-tree performs the same merge locally and returns the answer synchronously, so the loop disappears rather than getting wrapped in a function. Pushed in 988b4c1.
Checked against GitHub's own answer on all 30 open bot PRs in this repo, with the cache warm so its values are authoritative: exact agreement, only #856 CONFLICTING. Whole step now takes ~2s across all three authors, no sleeps.
The one thing it does need is the PR heads locally — one batched git fetch of refs/pull/N/head (covers fork PRs too, --force since bot branches get force-pushed). fetch-depth: 0 is already set by the shared checkout macro, so every generated nightly job has the history for it.
Failure stays distinguishable from clean, which was the point of the previous round: git merge-tree exits non-zero both for a real conflict and for a ref it couldn't resolve, but a conflict prints the tree OID and conflicted paths on stdout while an error prints nothing — so the recipe tests the captured output, not the status alone. I exercised both branches directly; an unfetched ref reports "merge test never ran, conflicts unverified" rather than passing as clean.
git merge-tree answers the same question synchronously, so the retry loop goes away entirely.
tend-agent
left a comment
There was a problem hiding this comment.
Dropping mergeable for a local test-merge is the right call, and this run happens to demonstrate why: gh pr list --json mergeable returned UNKNOWN for 24 of the 26 open tend-agent PRs on a cold read, while the recipe answered synchronously and flagged #856 and #809 as conflicting. #809 is the case the old filter drops silently — cold mergeable says UNKNOWN, the local merge says CONFLICTING. The error/conflict discrimination holds up too: an unresolvable ref exits non-zero with empty stdout, so [ -n "$tree" ] is load-bearing exactly as the comment claims.
Two gaps, both in the same silent-miss class the PR is closing:
gh pr listhas no--limit, so it silently truncates at 30.review-reviewers's SKILL.md already carries this as a rule ("--limit 100is load-bearing —gh pr listdefaults to 30 and truncates silently"), and this repo is at 26 open bot PRs today; on a dependabot-heavy adopter the 31st PR onward is never test-merged and the step reads as clean for it.- The output line drops the author. The old snippet emitted the whole JSON object, so
.authorwas there; the two sub-steps that consume this list branch on exactly that field —app/dependabot/app/renovateroute to Upstream dependency bots: trigger the bot's own rebase (whose table is keyed on the--author (PR list)value),$BOT_LOGINroutes to the manual worktree resolution.CONFLICTING: #809 fix(report-failure): …doesn't say which, and title text isn't a reliable proxy.
Suggestions inline.
…t one by position (#899) A session verifying its own skill edit closed a live maintainer-facing issue and posted a false status comment. The bot caught it and self-healed within ~2.5 minutes, but the close/reopen is permanent in the timeline and the deleted comment had already fired a workflow run. The guidance that produced the behaviour is still unguarded, so the next session hits the same shape. ## What happened Run [31257558939](https://github.com/max-sixty/tend/actions/runs/31257558939) (`tend-mention`, `repository_dispatch`) was answering a review question on #898 — a change to `plugins/tend-ci-runner/skills/nightly/SKILL.md`. To verify the edit it extracted a code block from that file **by fence ordinal** and ran it: ````bash awk '/^```bash$/{n++} n==3 && !/^```/{print} /^```$/{if(n==3) exit}' \ plugins/tend-ci-runner/skills/nightly/SKILL.md > /tmp/step3.sh bash /tmp/step3.sh ```` The third `bash` fence in that file is not the step-3 recipe it was aiming at. It is the drift-issue closer: ```bash gh issue list --state open --author '@me' \ --search '"configuration drift" in:title' \ --json number --jq '.[].number' \ | xargs -r -I {} gh issue close {} --comment 'tend check now passes.' ``` Issue #822 (`tend check: configuration drift on max-sixty/tend`) matched. It was closed at `12:38:56Z` with the comment `tend check now passes.` — false; the check still fails. The session noticed, reopened at `12:39:04Z`, deleted the comment, and [left a correction on the thread](#822 (comment)) plus a flag in its PR reply. Its own words from the session log: *"My mistake — I ran an extracted block without reading it first."* What did not recover: `#822`'s timeline still shows `closed by tend-agent` / `reopened by tend-agent`, and the deleted comment had already fired `issue_comment`, so run [31257716552](https://github.com/max-sixty/tend/actions/runs/31257716552) exists and always will. ## Root cause Two things compose, and only the second is a slip: 1. **Structural — the verification mandate has no write carve-out.** [`running-in-ci`'s "Verifying external-tool behavior"](https://github.com/max-sixty/tend/blob/03f8e0d/plugins/tend-ci-runner/skills/running-in-ci/SKILL.md#L610) says *"verify by running the code"* and ranks running the tool above reading the source, with an example scolding a session for trusting docs instead of running the thing. That instruction is right, and the session was obeying it. Nothing anywhere says the recipes it points at include `gh issue close`, `gh pr comment`, and `git push`, or that "run it" needs to mean something different when the recipe writes. Every adopter's `nightly` skill ships that same closer, so this is a bundled gap, not a tend-local one. 2. **Stochastic — selecting the block by ordinal and not reading it.** A different session might have read it. But the ordinal is a trap independent of judgment: it moves with every edit to the file, so the block that runs is by construction not guaranteed to be the one under test. ## The change One paragraph plus a two-line example in the section that created the pressure. It names the write surface, kills positional extraction, and gives the read-half recipe. No new section, no reorganisation. ## Gate assessment - **Evidence level: Critical, 1 occurrence** — this is the gate table's own example, *"clearly wrong outcome (closed wrong issue)"*, which acts at 1. It is also genuinely new: every one of the 124 prior windows in the evidence log records `0 reopened issue` under negative signals, so there is no historical count to add to. - **Classification: structural pressure, stochastic trigger.** I'm not claiming the failure is fully structural — a session that read the block first would have avoided it. What is structural is the instruction that sent it there with no mention of side effects, and the ordinal extraction that makes "the block I meant" unknowable. - **Magnitude: targeted fix.** Gate 2 would put a *new* section at the 3+ bar. This is a guard attached to an existing instruction that demonstrably produced a public wrong write, held to one paragraph to stay proportionate. Flagging the tension rather than hiding it: if you read this as a new-paragraph change wanting 3 occurrences, the counter-argument is that the three occurrences would each be another wrongly-closed issue. - **Dedup:** no open or closed issue/PR covers it. The three open `running-in-ci` PRs (#837, #870, #876) are all the CI-monitor poll, a different section. ## Verified, not inferred - Both claims in the new text were checked against this incident rather than assumed: `#822`'s events API shows the close and the reopen both retained; run `31257716552` fired from the since-deleted comment and is still listed. - The extraction command and `bash /tmp/step3.sh` are read verbatim from the session log's `tool_use` entries, not reconstructed from the bot's summary. - The third-fence block is reproduced by re-running the same `awk` against `nightly/SKILL.md` at `03f8e0d`. Evidence log: https://gist.github.com/e08f6e62d6478163cb425a75648eb7e4 --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Keep the batch fetch and the failed-query guard (both measured to earn their lines); drop the temp file and the three-way status split, whose only distinct output already reaches stderr. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Automated nightly regeneration of tend's workflow files. **tend version:** 0.1.14 → 0.1.15 **Notable changes** - **Rate-limit handling**: a maintainer can approve past the spike limit (max-sixty/tend#874), and `tend-mention` now skips comments on `tend-rate-limit`-labelled issues as well as `tend-outage` ones — the same self-trigger loop guard, widened to every issue tend files about its own health. - **Review flow**: a push is queued behind an examined-HEAD gate instead of cancelling the in-flight review (max-sixty/tend#903), the review is submitted before a fix is pushed (max-sixty/tend#834), a force-push triggers a re-review rather than trusting the re-anchored SHA (max-sixty/tend#884), and the `/code-review` second pass is unconditional (max-sixty/tend#937). - **CI monitoring**: both the check poll and the `gh run rerun --failed` poll now end terminally when the cap is hit instead of reading as done (max-sixty/tend#876, max-sixty/tend#951), and a failed GitHub-status probe no longer reads as "no incident" (max-sixty/tend#913). - **Nightly**: conflicted bot PRs are test-merged locally instead of filtered on the lazy `mergeable` field (max-sixty/tend#898), and mention runs skip the bot's own review and a third party's content-free approval (max-sixty/tend#916, max-sixty/tend#955). - **`tend check`**: an unreadable ruleset bypass list is reported as unknown rather than ungated (max-sixty/tend#825), environment names are read one per line and addressed encoded (max-sixty/tend#879), and `credential-environments` no longer points at the setting it rejects (max-sixty/tend#900) — this repo currently `SKIP`s that check, so its wording should improve here. Full comparison: max-sixty/tend@0.1.14...0.1.15 Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
Problem
Step 3 of the bundled
nightlyskill found conflicted bot PRs with a singlegh pr listwhose result was filtered onselect(.mergeable == "CONFLICTING"), followed by "Skip the rest of this step if none of the queries return anything."mergeableis not stored — GitHub computes it lazily. The first query after the base branch moves returnsUNKNOWNand enqueues the computation; a later query returns the real value.select(.mergeable == "CONFLICTING")dropsUNKNOWNsilently, so a cold cache was indistinguishable from a clean one, and the skip line turned "I don't know" into "there are no conflicts" — skipping the whole step, including the Bot-authored PRs: resolve manually subagent dispatch that exists to rebase the bot's own conflicted PRs.Reported in #897 with session-log evidence from ten sampled nightly runs.
Reproduction
Confirmed live against
nodejs/node, two queries seconds apart with nothing touched in between:UNKNOWNCONFLICTINGMERGEABLEThe cold read reported zero conflicted PRs; the settled read revealed one that was genuinely
CONFLICTING.Solution
Don't read
mergeableat all — compute the merge locally withgit merge-tree --write-tree, which is synchronous and authoritative, so there is no cache to wait on and no retry loop. There is no blocking read on the GitHub side: the REST docs prescribe resubmitting the request untilmergeableis non-null.The recipe fetches every PR head in one batched
git fetch(refs/pull/N/head, so fork PRs are covered too,--forcebecause bot branches get force-pushed), then test-merges each againstorigin/main. Failure stays distinguishable from clean at both steps: an emptygh pr listoutput file (0 bytes, versus 3 for[]) reports as unverified, andgit merge-treeexits non-zero both for a real conflict and for a ref it couldn't resolve — the conflict prints the tree OID and conflicted paths on stdout, the error prints nothing, so the recipe tests the captured output rather than the exit status alone.The output path is
/tmp/prs-${author//\//-}.json—app/dependabotandapp/renovatecontain a slash, so the un-sanitized form suggested in the issue would redirect into a nonexistent/tmp/prs-app/directory and fail.Testing
Ran the recipe verbatim from the repo root against this repo's own PRs: it agrees exactly with GitHub's own (warm, therefore authoritative) answer across all 30 open bot PRs, flagging only #856 as
CONFLICTING, and finishes in ~2s for all three authors with no sleeps. Both failure branches were exercised directly — agh pr listthat can't resolve the repo reports "query never landed", and an unfetched head ref reports "merge test never ran" — neither reads as clean.select(.mergeableappears at no other site in the bundled skills; the only othermergeablementions are workflow comments about merge-ref materialization, which are unrelated.fetch-depth: 0is set by the shared checkout macro ingenerator/src/tend/templates/macros.yaml.j2, so every generated nightly job has the history the local merge needs.Closes #897 — automated triage