From 18dd4f2c9e08bf5dd806f5d0e7cfafee3d2af2bb Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Fri, 4 Sep 2026 16:22:48 +0200 Subject: [PATCH 1/2] chore(devex): route queue triage at Trunk's own reasons An agent asked why a PR was kicked had no documented way to get Trunk's answer, so it reconstructed one from shadow PRs and the Actions API. That produces a confident narrative that can be wrong on every attempt-level cause: it cannot see a merge conflict, a PR that skipped the line, or a human cancellation, because none of those reach GitHub. `trunk merge status ` prints the full state machine with a reason per transition. The triage and merging skills now read that first and use the GitHub side to corroborate. Three places also told agents to watch a `Trunk Merge Queue (master)` check run. The trunk-io app publishes no check runs in this repository, so that predicate matches nothing and reads as "Trunk never picked the PR up" whatever the queue is doing. They now watch Trunk's state instead. Entry 3 of the chart treated every cancelled run as harmless teardown. A job killed by timeout-minutes also reports `cancelled`, never `timed_out`, so an infrastructure outage was classified as "nothing is broken, requeue once". It now separates the two by job duration against the cap. Also records that a shadow PR's ref is named after the batch leader, so a batched PR never appears in a search for its own number while queued. --- .agents/skills/debugging-ci-failures/SKILL.md | 2 +- .agents/skills/merging-prs/SKILL.md | 42 +++++++++---------- .../triaging-merge-queue-failures/SKILL.md | 31 ++++++++++++-- AGENTS.md | 6 ++- 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/.agents/skills/debugging-ci-failures/SKILL.md b/.agents/skills/debugging-ci-failures/SKILL.md index 650f7cb7c49e..62d68b9d5348 100644 --- a/.agents/skills/debugging-ci-failures/SKILL.md +++ b/.agents/skills/debugging-ci-failures/SKILL.md @@ -144,7 +144,7 @@ overlap its own. The lane script over-reports targets on purpose, so in practice that is most of the queue. So: - The failing run is on that branch, never on the PR's head SHA. Take it from - the `Trunk Merge Queue` check run (`/merging-prs` step 4), not `gh pr checks`. + `trunk merge status ` (`/merging-prs` step 4), not `gh pr checks`. Trunk publishes no check run here. The branch is ephemeral; the run and its logs stay on GitHub, and the warehouse keeps its jobs under that `head_branch` (query 8 in the `investigating-ci-failures` references). diff --git a/.agents/skills/merging-prs/SKILL.md b/.agents/skills/merging-prs/SKILL.md index 10494e725dc9..7f6c61df2657 100644 --- a/.agents/skills/merging-prs/SKILL.md +++ b/.agents/skills/merging-prs/SKILL.md @@ -2,9 +2,9 @@ name: merging-prs description: > Merge a PR into `master` through the Trunk merge queue and babysit it until it - lands. Enqueue with a `/trunk merge` comment, then watch the `Trunk Merge Queue - (master)` check run and the PR state until it is MERGED or the queue kicks it - out, reporting the Trunk bot's failure reason. Use when asked to merge a PR, + lands. Enqueue with a `/trunk merge` comment, then watch `trunk merge status` + and the PR state until it is MERGED or the queue kicks it out, reporting + Trunk's own reason for the terminal transition. Use when asked to merge a PR, "merge when ready", "land it", "ship it", to merge a whole stack (comment on the top PR — the queue merges it and every layer below atomically), to get a PR approved via the `stamphog` label, or to babysit/watch a PR through the @@ -49,15 +49,18 @@ gh pr comment --body "/trunk merge" For a stack, `` is the highest layer you want merged — it and everything below it enqueue together (see the stack preflight bullet above). Append `--no-batch` to the comment to have the queue test the PR (or stack) alone instead of batched with other queued PRs. -Within ~2 minutes, confirm Trunk picked it up — a check run whose name starts with `Trunk Merge Queue` should appear on the head commit: +Within ~2 minutes, confirm Trunk picked it up: ```bash -SHA=$(gh pr view --json headRefOid -q .headRefOid) -gh api --paginate "repos/$REPO/commits/$SHA/check-runs?per_page=100" \ - --jq '.check_runs[] | select(.name | startswith("Trunk Merge Queue")) | {name, status, conclusion, details_url}' +trunk merge status 2>&1 | sed 's/\x1b\[[0-9;]*m//g' | tail -20 ``` -Always paginate. A PR head SHA here carries 200–350 check runs, and an unpaginated call returns only the first 30 — the queue check is very unlikely to be in them, so you'd conclude Trunk never picked the PR up. +The last transition should read `Pending` with "Pull request has been added to the merge queue". + +**Trunk publishes no check run in this repository**, so do not look for one on the head commit. +A predicate like `select(.name | startswith("Trunk Merge Queue"))` matches nothing and reads as "Trunk never picked it up", whatever the queue is actually doing. +`trunk merge status` is the source of truth, and it is the only place Trunk's reason for each transition appears. +Without a `trunk login`, fall back to the `trunk-io[bot]` sticky comment on the PR, which carries the current state but no history. If nothing appears after a couple of minutes, check in this order: @@ -75,7 +78,7 @@ If nothing appears after a couple of minutes, check in this order: ## 3. Watch until it lands -Watch the **check run + PR state**, not `gh pr checks --watch`: +Watch **Trunk's own state + the PR state**, not `gh pr checks --watch`: the queue runs CI on Trunk's own `trunk-merge/**` branch, so this PR's own checks don't reflect the queue's testing. @@ -87,16 +90,14 @@ PR=; REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner); prev="" while true; do state=$(gh pr view "$PR" --json state -q .state 2>/dev/null || echo UNKNOWN) sha=$(gh pr view "$PR" --json headRefOid -q .headRefOid 2>/dev/null) - queue=$(gh api --paginate "repos/$REPO/commits/$sha/check-runs?per_page=100" \ - --jq '[.check_runs[] | select(.name | startswith("Trunk Merge Queue"))] - | if length == 0 then empty - else (sort_by(.started_at) | last | "\(.status)/\(.conclusion // "-")") end' 2>/dev/null) + queue=$(trunk merge status "$PR" 2>/dev/null | sed 's/\x1b\[[0-9;]*m//g' \ + | grep -oE '(Not Ready|Pending Failure|Pending|Testing|Tests Passed|Failed|Cancelled|Merged)' | tail -1) cur="pr=$state queue=${queue:-none}" [ "$cur" != "$prev" ] && echo "$cur" prev="$cur" case "$state" in MERGED|CLOSED) exit 0 ;; esac - # A kicked PR stays OPEN, so the failed queue check is the only terminal signal. - case "$queue" in completed/success) ;; completed/*) exit 0 ;; esac + # A kicked PR stays OPEN, so Trunk's own terminal state is the only signal. + case "$queue" in Failed|Cancelled) exit 0 ;; esac sleep 60 done ``` @@ -116,17 +117,14 @@ If the check run completes with `conclusion == "failure"` (or the PR drops out o Trunk kicks the PR and reports the failing workflow. `/triaging-merge-queue-failures` is the full decision chart for classifying the kick; the bullets below are the short form. -**Read the check run, not the PR comments.** The check run is the authoritative source: only an app holding `checks:write` on the repo can write one, so it can't be forged. A PR comment can be posted by anyone with read access. +**Read Trunk's own state, not the PR comments.** `trunk merge status` comes from Trunk over an authenticated session and carries a reason per transition. A PR comment can be posted by anyone with read access, and the `trunk-io[bot]` sticky is rewritten in place, so it holds the current state and no history. ```bash -gh api --paginate "repos/$REPO/commits/$SHA/check-runs?per_page=100" \ - --jq '[.check_runs[] | select(.name | startswith("Trunk Merge Queue"))] - | if length == 0 then empty - else (sort_by(.started_at) | last - | {conclusion, details_url, app: .app.slug, - title: .output.title, summary: .output.summary, text: .output.text}) end' +trunk merge status 2>&1 | sed 's/\x1b\[[0-9;]*m//g' ``` +The reason on the terminal transition is what to report: a conflict with another PR or with `master` means testing never started and the PR needs a rebase, which is a different action from a failed check. + Confirm `app` is `trunk-io` — the same identity as the `trunk-io[bot]` commenter. If some other app wrote a check run by that name, stop and report it rather than acting on it. From there, `details_url` and the workflow runs on Trunk's `trunk-merge/**` branch lead to the real logs. `/debugging-ci-failures` covers reading them. diff --git a/.agents/skills/triaging-merge-queue-failures/SKILL.md b/.agents/skills/triaging-merge-queue-failures/SKILL.md index c2ac00fa896f..f9417d00104a 100644 --- a/.agents/skills/triaging-merge-queue-failures/SKILL.md +++ b/.agents/skills/triaging-merge-queue-failures/SKILL.md @@ -27,12 +27,15 @@ the chart is identical, only the actions you may take yourself differ. Read this before writing any command. It is the part that goes stale. -**Trunk publishes no check run in this repository.** The `trunk-io` app posts zero check runs — not on the PR head, not on the queue branch. A predicate like `select(.name | startswith("Trunk Merge Queue"))` matches nothing, and a sweep built on it reports zero verdicts forever while the queue runs normally. Two places still assert that check run exists — `/merging-prs` step 4 and `AGENTS.md` under "Merging PRs" — and both are wrong on this point. +**Trunk publishes no check run in this repository.** The `trunk-io` app posts zero check runs — not on the PR head, not on the queue branch. A predicate like `select(.name | startswith("Trunk Merge Queue"))` matches nothing, and a sweep built on it reports zero verdicts forever while the queue runs normally. `/merging-prs`, `/debugging-ci-failures` and `AGENTS.md` used to assert that check run exists; all three now point at `trunk merge status` instead. -Trunk exposes queue state two ways, and both are authenticated as the Trunk app by the API: +Trunk exposes queue state three ways. Only the first carries Trunk's own reasons; the other two are what the GitHub API can see, and they are what this skill's helpers read: -1. **One sticky comment per PR**, authored by `trunk-io[bot]`, rewritten in place as state changes. It carries the state, the failing check's name, and a link to the failing job. -2. **One draft shadow PR per queue attempt**, authored by `trunk-io[bot]`, with head ref `trunk-merge/pr-/`. Its head SHA carries the attempt's real CI as ordinary `github-actions` check runs, with job links. A `-bisection` suffix means Trunk is bisecting a failed batch to find the culprit. +1. **`trunk merge status ` (the CLI)** — the full state machine with a reason per transition, in Trunk's words. This is the primary source for _why_, and the only one that surfaces conflicts, line-skips and cancellations at all. Human text only, no `--json`. The same timeline is exportable as JSON from the Trunk dashboard. +2. **One sticky comment per PR**, authored by `trunk-io[bot]`, rewritten in place as state changes. It carries the current state, the failing check's name, and a link to the failing job. Rewritten means no history: earlier reasons are gone. It can also carry a `Failed Test | Failure Summary | Logs` table naming the failing test outright — check it before reading any log, and note it is empty for suites that do not upload results to Trunk. +3. **One draft shadow PR per queue attempt**, authored by `trunk-io[bot]`, with head ref `trunk-merge/pr-/`. Its head SHA carries the attempt's real CI as ordinary `github-actions` check runs, with job links. A `-bisection` suffix means Trunk is bisecting a failed batch to find the culprit. + +The shadow PR's **body** lists the batch members and the PRs queued ahead of it, which is the only place queue depth is visible. Note the ref is named after the batch _leader_, so a PR batched behind another never appears in a `trunk-merge/pr-/` search even while it is actively queued. Shadow PRs are **never merged** — every one ends closed and unmerged, whether the PR merged or was kicked. So a shadow PR's own state tells you nothing about the outcome; the sticky comment does. Repeated shadow PRs for the same `pr-` are repeated attempts, which is the retry-already-happened signal. @@ -100,6 +103,16 @@ The routine sandbox is more restricted than a laptop. All four of these were obs ## Establish the facts +**Ask Trunk why before you reconstruct why.** `trunk merge status ` prints Trunk's own state machine for the PR: every transition, with the reason in Trunk's words. That includes the reasons nothing else in this skill can recover — a conflict with another PR or with `master`, a PR that skipped the line ahead, a human cancellation, and the exact required check that removed it. + +```bash +trunk merge status 2>&1 | sed 's/\x1b\[[0-9;]*m//g' # strip ANSI; there is no --json +``` + +Read that first. Everything below is for corroborating it, or for the cases it does not cover (what actually failed inside a job). Reconstructing causality from shadow PRs and the Actions API when this command would have answered it in one call is the most expensive mistake available here: it produces a confident narrative that can be wrong on every attempt-level cause. + +Two limits worth knowing. The output is ANSI-coloured and column-wrapped, so a reason spans several lines and needs re-joining before you match on it. And the CLI needs an interactive `trunk login` once; in a headless environment ask the human to paste the output, or the dashboard's JSON export of the same timeline. + `` is the PR number; `REPO=$(gh api repos/PostHog/posthog --jq .full_name)` — or just hardcode `PostHog/posthog`, since `gh repo view` needs GraphQL. ```bash @@ -172,6 +185,16 @@ Nothing was tested and nothing is broken. Trunk closes its shadow PR the moment Do not route this to `/fixing-flaky-tests`. There is no flaky test — the tests did not finish. +**First rule out a job timeout, which is indistinguishable from teardown here.** A GitHub job killed by `timeout-minutes` marks itself and its steps `cancelled`, never `timed_out` or `failure`, so it lands in exactly the shape above. Searching for `conclusion == "timed_out"` finds nothing. Separate the two by duration: a teardown cancels every in-flight job at once at arbitrary durations, often with `${{ matrix.* }}` still unexpanded in the job names because nothing started; a timeout kill is an isolated job sitting on a round cap. + +```bash +gh api "repos/$REPO/actions/runs/$RUN/jobs?per_page=100" \ + --jq '.jobs[] | select(.conclusion == "cancelled") + | "\(((.completed_at|fromdateiso8601)-(.started_at|fromdateiso8601)))s\t\(.name)"' +``` + +A cluster at 300s or 600s is a timeout, not a teardown, and the verdict is entry 6 (a wider issue), not a requeue. On 4 Sep 2026 a degraded npm audit endpoint stalled the pnpm bootstrap until jobs hit their caps, and every one of those kills read as teardown here. + **Verdict: requeue once.** Apply the same retry gate as entry 5: count this head's attempts with `attempts $REPO ` and add nothing if a retry already happened. Say in the verdict that the attempt was cancelled rather than failed, so the author does not go looking for a broken test. Do not reach for a workflow-side fix. Putting the gate on `if: ${{ !cancelled() }}` looks like it removes the false red, but a gate that does not run reports `skipped`, and branch protection counts a skipped required check as passing — so it trades a red check that blocks an untested run for a green one that lets it through. `AGENTS.md`, under "Forcing the full CI matrix on a draft", covers that failure mode. If this class is a recurring drag rather than a one-off, still requeue, and raise it with the team that owns the queue the way entry 6 says to — repeated requeues are not a fix for it. diff --git a/AGENTS.md b/AGENTS.md index bd0299c683f0..872093c9d8f8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -141,10 +141,12 @@ Never run `gh pr merge` or click the GitHub merge button — both are blocked by Agents must not enqueue, merge, re-enqueue, or otherwise cause a PR to land without explicit user approval in the current conversation for the identified PR or stack. Do not infer that approval from requests to prepare a PR, move it toward merge, make it ready, monitor it, or resolve its blockers. Agents may inspect status, fix code and CI, apply `stamphog` when approval is missing, and report that a PR is ready; then they must wait for a direct instruction to merge or enqueue it. - After explicit user approval: enqueue with `gh pr comment --body "/trunk merge"`. Cancel: `gh pr comment --body "/trunk cancel"`. Enqueueing a stacked PR also enqueues every unmerged layer below it — comment on the top PR to merge the whole stack. `--no-batch` opts the PR (or stack) out of batching. -- The Trunk CLI is an alternative to the comments: `trunk merge ` enqueues, `trunk merge status ` inspects, `trunk merge cancel ` dequeues. It ships in the flox environment and needs a one-time interactive `trunk login` — run it once even if you prefer the comments, because the same login arms the pre-push merge-queue guard that stops you from knocking a queued PR out of the queue. Agents and headless environments that can't complete the interactive login use the comments. +- The Trunk CLI is an alternative to the comments: `trunk merge ` enqueues, `trunk merge status ` inspects, `trunk merge cancel ` dequeues. `status` is also the only place Trunk's own reason for each queue transition is visible — a conflict, a PR that skipped the line, a human cancellation, the check that removed it — so read it before reconstructing a cause from CI, and see `/triaging-merge-queue-failures`. It ships in the flox environment and needs a one-time interactive `trunk login` — run it once even if you prefer the comments, because the same login arms the pre-push merge-queue guard that stops you from knocking a queued PR out of the queue. Agents and headless environments that can't complete the interactive login use the comments. - Missing required approval: apply the `stamphog` label (`gh pr edit --add-label stamphog`) to trigger the automated review-and-approve flow, and re-apply it whenever it was stripped (`REFUSED`/`ESCALATE` verdict) once the feedback is addressed — re-applying is always safe. - After enqueueing, babysit the PR until it merges or fails — follow [`.agents/skills/merging-prs/SKILL.md`](./.agents/skills/merging-prs/SKILL.md) for the preflight, watch, and failure-handling loop. -- Queue progress is the `Trunk Merge Queue (master)` check run on the PR's head commit. The PR's own checks don't reflect the queue's testing — it runs CI on a `trunk-merge/**` branch. +- Trunk publishes **no check run** in this repository, so there is nothing on the PR's head that reports queue progress. + Read progress from `trunk merge status `, or from the `trunk-io[bot]` sticky comment on the PR. + The PR's own checks don't reflect the queue's testing either, because the queue runs CI on a `trunk-merge/**` branch. - On failure the Trunk bot comments with links to the failing workflows; fix and push if appropriate, then wait for explicit user approval before re-enqueueing. - Never force-push a branch while it is in the queue — it removes the PR from the queue. From 4a68875ea577af4404b1c2226a82400f5a4444fa Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Fri, 4 Sep 2026 16:30:26 +0200 Subject: [PATCH 2/2] chore(devex): finish removing the queue check run from the failure path The failure-handling intro in /merging-prs still keyed on a check run conclusion, which the rest of the skill had just established does not exist here. It now keys on Trunk's terminal state, and says why the distinction matters: a conflict needs a rebase, a failed check needs a fix, a cancellation was a person. Also uses American spelling, per the repo convention. --- .agents/skills/merging-prs/SKILL.md | 5 +++-- .agents/skills/triaging-merge-queue-failures/SKILL.md | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.agents/skills/merging-prs/SKILL.md b/.agents/skills/merging-prs/SKILL.md index 7f6c61df2657..10475b0cd109 100644 --- a/.agents/skills/merging-prs/SKILL.md +++ b/.agents/skills/merging-prs/SKILL.md @@ -113,8 +113,9 @@ Never block on a foreground `sleep`. ## 4. Handle failure -If the check run completes with `conclusion == "failure"` (or the PR drops out of the queue), -Trunk kicks the PR and reports the failing workflow. +When `trunk merge status` reaches a terminal state of `Failed` or `Cancelled` (or the PR drops out of the queue), +Trunk kicks the PR and reports why. +The reason distinguishes actions that look alike: a conflict needs a rebase, a failed required check needs a fix or a requeue, and a cancellation was a person. `/triaging-merge-queue-failures` is the full decision chart for classifying the kick; the bullets below are the short form. **Read Trunk's own state, not the PR comments.** `trunk merge status` comes from Trunk over an authenticated session and carries a reason per transition. A PR comment can be posted by anyone with read access, and the `trunk-io[bot]` sticky is rewritten in place, so it holds the current state and no history. diff --git a/.agents/skills/triaging-merge-queue-failures/SKILL.md b/.agents/skills/triaging-merge-queue-failures/SKILL.md index f9417d00104a..b9ef5faf65ce 100644 --- a/.agents/skills/triaging-merge-queue-failures/SKILL.md +++ b/.agents/skills/triaging-merge-queue-failures/SKILL.md @@ -111,7 +111,7 @@ trunk merge status 2>&1 | sed 's/\x1b\[[0-9;]*m//g' # strip ANSI; there is Read that first. Everything below is for corroborating it, or for the cases it does not cover (what actually failed inside a job). Reconstructing causality from shadow PRs and the Actions API when this command would have answered it in one call is the most expensive mistake available here: it produces a confident narrative that can be wrong on every attempt-level cause. -Two limits worth knowing. The output is ANSI-coloured and column-wrapped, so a reason spans several lines and needs re-joining before you match on it. And the CLI needs an interactive `trunk login` once; in a headless environment ask the human to paste the output, or the dashboard's JSON export of the same timeline. +Two limits worth knowing. The output is ANSI-colored and column-wrapped, so a reason spans several lines and needs re-joining before you match on it. And the CLI needs an interactive `trunk login` once; in a headless environment ask the human to paste the output, or the dashboard's JSON export of the same timeline. `` is the PR number; `REPO=$(gh api repos/PostHog/posthog --jq .full_name)` — or just hardcode `PostHog/posthog`, since `gh repo view` needs GraphQL.