Skip to content

fix(nightly): test-merge bot PRs locally instead of filtering the lazy mergeable field - #898

Merged
max-sixty merged 5 commits into
mainfrom
fix/issue-897
Aug 11, 2026
Merged

fix(nightly): test-merge bot PRs locally instead of filtering the lazy mergeable field#898
max-sixty merged 5 commits into
mainfrom
fix/issue-897

Conversation

@tend-agent

@tend-agent tend-agent commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Problem

Step 3 of the bundled nightly skill found conflicted bot PRs with a single gh pr list whose result was filtered on select(.mergeable == "CONFLICTING"), followed by "Skip the rest of this step if none of the queries return anything."

mergeable is not stored — GitHub computes it lazily. The first query after the base branch moves returns UNKNOWN and enqueues the computation; a later query returns the real value. select(.mergeable == "CONFLICTING") drops UNKNOWN silently, 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:

1st query 2nd query
UNKNOWN 36 0
CONFLICTING 0 1
MERGEABLE 24 59

The cold read reported zero conflicted PRs; the settled read revealed one that was genuinely CONFLICTING.

Solution

Don't read mergeable at all — compute the merge locally with git 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 until mergeable is non-null.

The recipe fetches every PR head in one batched git fetch (refs/pull/N/head, so fork PRs are covered too, --force because bot branches get force-pushed), then test-merges each against origin/main. Failure stays distinguishable from clean at both steps: an empty gh pr list output file (0 bytes, versus 3 for []) reports as unverified, and git merge-tree exits 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//\//-}.jsonapp/dependabot and app/renovate contain 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 — a gh pr list that can't resolve the repo reports "query never landed", and an unfetched head ref reports "merge test never ran" — neither reads as clean.

select(.mergeable appears at no other site in the bundled skills; the only other mergeable mentions are workflow comments about merge-ref materialization, which are unrelated.

fetch-depth: 0 is set by the shared checkout macro in generator/src/tend/templates/macros.yaml.j2, so every generated nightly job has the history the local merge needs.


Closes #897 — automated triage

@tend-agent tend-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

Comment thread plugins/tend-ci-runner/skills/nightly/SKILL.md Outdated
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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

is there a way to do this with a function of "wait for the answer", rather than repeating for 5 steps?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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 tend-agent changed the title fix(nightly): settle mergeable before filtering, and stop treating UNKNOWN as clean fix(nightly): test-merge bot PRs locally instead of filtering the lazy mergeable field Aug 8, 2026

@tend-agent tend-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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 list has no --limit, so it silently truncates at 30. review-reviewers's SKILL.md already carries this as a rule ("--limit 100 is load-bearing — gh pr list defaults 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 .author was there; the two sub-steps that consume this list branch on exactly that field — app/dependabot / app/renovate route to Upstream dependency bots: trigger the bot's own rebase (whose table is keyed on the --author (PR list) value), $BOT_LOGIN routes to the manual worktree resolution. CONFLICTING: #809 fix(report-failure): … doesn't say which, and title text isn't a reliable proxy.

Suggestions inline.

Comment thread plugins/tend-ci-runner/skills/nightly/SKILL.md Outdated
Comment thread plugins/tend-ci-runner/skills/nightly/SKILL.md Outdated
Comment thread plugins/tend-ci-runner/skills/nightly/SKILL.md Outdated
max-sixty pushed a commit that referenced this pull request Aug 11, 2026
…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>
@max-sixty
max-sixty merged commit f5d4e51 into main Aug 11, 2026
@max-sixty
max-sixty deleted the fix/issue-897 branch August 11, 2026 23:35
max-sixty pushed a commit to max-sixty/worktrunk that referenced this pull request Aug 13, 2026
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>
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.

nightly: conflict detection reads mergeable once, so a lazily-computed UNKNOWN is silently treated as no conflict

2 participants