Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ comes last, and a missing value prints as `-`:
| `checks` | `failure` if any check fails, else `pending` if any is pending, else `success`, and `passed=`, `failing=`, `pending=` counts, all from each check's latest run; `-` when the head has no checks. |
| `check` | `failing` or `pending`, `required` or `optional`, the result, details URL, and name. |
| `review` | `current` or `stale` against the head, state, reviewer, time, and URL of each reviewer's latest review. |
| `checkpoint` | Each bot reviewer that approves or requests changes, such as one whose approval the base requires, its round, and its review's URL, when that review is of the head and its latest verdict there, ignoring plain comments, is neither an approval nor a dismissal, at the end of its third, sixth, ... round: [`/quest-review`'s design checkpoint](#review-locally) for an online reviewer. A round is one commit it reviewed; a dismissed review counts as one that approves or requests changes. Only printed when due. |
| `reacted:<content>` | Actor, time, and the PR or review reacted to, as `quest wait` names reactions. |
| `thread` | Node ID, `current` or `outdated`, last commenter, latest comment URL, and `path:line` of each unresolved thread. |
| `hold` | Label, acquisition token, acquisition time, verified GitHub event API URL, and age; `hold -` when no label holds it. Unknown acquisition facts print as `-` and `unknown age`. |
Expand All @@ -518,7 +519,7 @@ own rollup state counts the superseded runs too. A required check
is one branch protection or a ruleset requires for this PR. `--json` prints the
same facts as one object with the fields `url`, `state`, `draft`, `head`,
`mergeable`, `mergeState`, `reviewDecision`, `autoMerge`, `mergeQueue`,
`landingStopped`, `checks` (`state`, `passed`, `failing`, `pending`), `reviews`, `reactions`,
`landingStopped`, `checks` (`state`, `passed`, `failing`, `pending`), `reviews`, `checkpoints` (`reviewer`, `round`, `url`), `reactions`,
`threads`, `holds` (`label`, `token`, `takenAt`, `age`, `eventUrl`), and `starterHold` (`lastActive`, `age`, `url`, or null). Ownership uses the state commands' shared reader without changing labels, and event links match the opaque acquisition token against GitHub's event listing. Long lists are paged; reactions are the newest 100 per PR or review,
as in `quest wait`. Any PR state exits 0, invalid arguments exit 2, and other
failures exit 1.
Expand Down Expand Up @@ -751,11 +752,13 @@ up to floor(log2 n) further PRs that merge cleanly in that order, resolving
quest links with `quest resolve`. A failed batch halves the next batch's size,
down to one PR, and each batch that lands grows it back by one; the order and
size persist in the state directory. Conflicting
additions stay queued. A PR whose required checks are still running stays queued
and sits out batches until they pass; only each check's latest run counts, as in
`quest status`. If any are still pending two hours after
additions stay queued. A PR whose required checks are still running, or that
still needs an approving review the base requires, stays queued and sits out
batches until they pass and it is approved; only each check's latest run counts,
as in `quest status`. If it still waits two hours after
its pick (twice the default `--timeout`), the daemon comments once on that pick,
naming them, since a required check that never reports would hold it silently. The sandbox checks the tip once, without
naming what it waits for, since a check that never reports or an approval nobody
gives would hold it silently. The sandbox checks the tip once, without
bisection: success lands one commit per PR, as `--method` says. A failure, or a base that
conflicts with the batch's first PR, parks those PRs for the fixer: they keep their
auto-merge and place, get no comment, and sit out batches while its
Expand Down Expand Up @@ -1023,9 +1026,10 @@ watcher is the base's only writer outside an admin's emergency bypass.

Its picks are those `quest land serve` reads: open PRs whose auto-merge
`quest merge --land` armed, pinned to their head, in the order they were armed,
with the same ejections for a moved head, a missing pin, a blocking review, or
a failing required check. A pick still waiting on its required checks keeps its
place, and one another session holds waits, armed, until its hold clears.
with the same ejections for a moved head, a missing pin, a requested change, or
a failing required check. A pick still waiting on its required checks or a
required approval keeps its place, and one another session holds waits, armed,
until its hold clears.

For the oldest pick it merges the base into the picked head with the quest
merge driver, in the daemon's private repository, and pushes that merge to
Expand Down
4 changes: 3 additions & 1 deletion assets/CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ Merge the base only when the branch conflicts with it or a landing step, such as
Before each push to a PR's branch, confirm the PR is still open, since a push after it merges restores the deleted branch.

Before a PR is marked ready for review, a model other than its latest writer reviews the committed branch locally, as `/quest-review` describes; a later push needs a new review unless `/quest-merge` exempts its changes.
Online reviewers such as CodeRabbit are optional, and anything AI-attributed is a suggestion, not a maintainer's decision: address findings already posted on the head, never wait for or request a review, and reply to feedback you disagree with.
Anything AI-attributed, such as an online reviewer's findings, is a suggestion, not a maintainer's decision: address findings already posted on the head, reply to feedback you disagree with, and never wait for an online review, since the queue waits for any approval the base requires.
Request a review only from a reviewer whose approval the base requires and that never ran on the head.
Once you fix or refute an automated reviewer's finding, say so in its thread and resolve it, since such a reviewer may approve only after its threads are resolved.

The PR body's `## Models` section records its size as `Size: M` (its quest's, or for a skill's own PR, as a quest is sized) and one `Implementer: Codex, gpt-6.1-sol, high effort` line per implementing session, moved last whenever it pushes; write the model's ID, and `unknown` for whatever the harness doesn't report.

Expand Down
1 change: 1 addition & 0 deletions assets/skills/iterate.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Read the PR, its linked issues and quest, the reviews and comments posted on it,
`quest status <pr>` reports its checks, reviews by head, and unresolved threads in one call.
Judge the approach before touching anything: would you take a different one?
`/quest-review`'s design checkpoint counts the rounds of each reviewer's session on the branch, including those earlier agents ran, so it may fall due in this one.
At a `checkpoint` line in `quest status`, an online reviewer's every third round with findings, stop before fixing them and take `/quest-review`'s design checkpoint.

Settle every decision this PR needs with the grill skill's interview, starting from the user's feedback: those its `## Waiting` section lists, those your judgment of the approach or the review findings raise, and any you find along the way.
Record the settled decisions in this PR description's `## Decisions` section as the paper trail, then implement them.
Expand Down
4 changes: 3 additions & 1 deletion assets/skills/merge.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ The head is landable when:
- the PR is not a draft;
- a review counts for it, as below;
- every review finding posted on it, including automated reviewers' such as CodeRabbit or Codex, is fixed on the head or answered with a refutation, and no decision is open;
- no check failed on it, required or not, its required checks passed, other than the gate's own `quest land` check, which never reports on a head, and it does not conflict with its base.
- no check failed on it, required or not, its required checks passed, other than the gate's own `quest land` check, which never reports on a head, and it does not conflict with its base;
- no review requests changes; an approval its base requires may still be to come, since the queue waits for it.

While its required checks are only pending, report that the pick stands until they pass; do not wait for them.
A change request that stands once its findings are fixed or refuted and their threads resolved is the user's call.
Any other head needs `/quest-iterate`: run it, saying what the head needs, such as a review, fixes, a failed check fixed or rerun, flaky ones included, or decisions, then run this skill again once it finishes; if it stops for the user instead, report that.
Run it in this session, where its interview can ask the user; under a coordinator such as `/quest-run`, return the PR to the loop, which dispatches it in a sub-agent.
A PR that adds follow-ups also needs the pick round's answers recorded before queueing, as the guide's Creation section describes: ask about each follow-up that round has not asked, in a question of its own, then run `/quest-iterate` with the answers unless the head already records them.
Expand Down
2 changes: 1 addition & 1 deletion assets/skills/review.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Refute a finding only with evidence the reviewer cannot rebut: a `path:line`, a
A finding the reviewer keeps against such a refutation is a decision for the user.
Write each round's `--responses` file in the checkout's `.tmp/`.

At a `next checkpoint` line, which a reviewer's every third round prints when it reports findings, stop before fixing them and reconsider the approach.
At a `next checkpoint` line, which a reviewer's every third round prints when it reports findings, stop before fixing them and reconsider the approach; `quest status <pr>` prints a `checkpoint` line when an online reviewer's round is due the same way.
Findings that keep coming usually mean the approach is wrong, not the last fix: brainstorm other designs that would avoid the whole class of findings, favoring simpler ones, and compare them with the current one.
Report the alternatives and what remains, recommending to continue, rethink the approach, merge as is, or drop the PR, and let the user decide.
Record each design change, including a rethink the user picks, in a PR comment that starts with `Design change:`, and start the next review with `quest review --new`, which restarts the count.
Expand Down
4 changes: 2 additions & 2 deletions assets/skills/run.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Run each `/quest-merge` inline; dispatch each `/quest-iterate` and `/quest-revie
Dispatch `/quest-iterate` without asking for a PR that needs work but no decision, such as a conflict, failing checks, unaddressed findings, or no review that counts for its head as `/quest-merge` defines.
A PR with open decisions, its `## Waiting` section's or an issue you would solve differently, goes to the user: settle them with the grill skill's rounds, then dispatch `/quest-iterate` with the answers in its brief, since a sub-agent cannot prompt.
A PR you would skip or delete rather than fix, such as a superseded one, gets that question instead.
Leave a PR whose required checks are still running for a later round; an optional automated reviewer that has not posted, such as one rate-limited or skipping drafts, is not waited on.
Leave a PR whose required checks are still running for a later round; an automated reviewer is not waited on, since the queue waits for an approval the base requires.

A PR whose author has write access and that changes no user-facing behavior carries a standing `/quest-merge` pick, unless the repository's `CONTRIBUTING.md` opts out: run `/quest-merge` on it without asking, and name it as auto-picked in the summary.
That covers only tests, benchmarks, CI, internal docs or comments, and behavior-preserving code. Anything users or other repositories get, such as CLI behavior or output, shipped skills or rules, user-facing docs, dependencies, or release artifacts, needs the user's pick, as does adding or expanding quests.
Expand All @@ -57,7 +57,7 @@ A `/quest-merge` pick covers the PR until it lands or needs the user: when `/que
Order merges that would otherwise conflict.
A PR that left the queue unmerged shows why under `landing-stopped` in `quest status <pr>`.
Where the queue runs a fixer, such as `quest land`, it hands back only a decision: ask the user, with its handback comment's diagnosis and recommendation.
Without one, after `merge_conflict` or `failed_checks` the pick holds, even from an earlier session: dispatch `/quest-iterate`. After `checks_timed_out`, run `/quest-merge` again once; a second timeout of the same head is the user's call.
Without one, after `merge_conflict`, `failed_checks`, `changes_requested`, or `unresolved_conversations` the pick holds, even from an earlier session: dispatch `/quest-iterate`. After `checks_timed_out`, run `/quest-merge` again once; a second timeout of the same head is the user's call.
Any other reason, such as a person removing it, is the user's call.

## Loop
Expand Down
18 changes: 16 additions & 2 deletions src/cli/land-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ interface Shown {
id: number;
status: View["status"];
title: string;
summary: string;
url?: string;
}

Expand Down Expand Up @@ -166,6 +167,7 @@ class Queue {
shown?.head === head &&
shown.status === view.status &&
shown.title === view.title &&
shown.summary === view.summary &&
shown.url === view.url
)
return;
Expand Down Expand Up @@ -200,6 +202,7 @@ class Queue {
id,
status: view.status,
title: view.title,
summary: view.summary,
...(view.url ? { url: view.url } : {}),
};
save(shownPath(this.queue.state), this.shown);
Expand Down Expand Up @@ -714,8 +717,19 @@ export async function ownPass(
},
);
}
// A pick waiting on its own required checks keeps the check it last showed.
for (const waiting of plan.waiting ?? []) open.add(waiting.number);
// A pick waiting on its own required checks or an approval shows what, so the wait is never silent.
for (const waiting of plan.waiting ?? []) {
open.add(waiting.number);
const waits = [
...waiting.pending.map((name) => `the required check \`${name}\` to pass`),
...(waiting.approval ? [`an approving review, which ${base} requires`] : []),
];
await own.show(waiting.number, waiting.head, {
status: "queued",
title: "Waiting",
summary: `It keeps its place and joins the queue once nothing is left to wait for: ${waits.join(", ")}.`,
Comment thread
pullfrog[bot] marked this conversation as resolved.
Resolved
});
}
await own.leave(open, "The pick ended: its auto-merge was disabled, or it closed.");
if (fix)
own.lines.push(
Expand Down
21 changes: 13 additions & 8 deletions src/cli/land-serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,15 +313,15 @@ async function reportOverlaps(github: GitHub, plan: Plan, state: string, overlap
}

/**
* How long after its pick a PR may wait on pending required checks before its author is told which.
* Twice the default check deadline (`--timeout`), so a slow but running CI rarely trips it.
* How long after its pick a PR may wait on pending required checks or an approval before its author is told
* what it waits for. Twice the default check deadline (`--timeout`), so a slow but running CI rarely trips it.
*/
export const STALLED_AFTER = 2 * 60 * 60_000;

/**
* Comment once per pick on each PR still waiting on required checks `STALLED_AFTER` its pick, naming
* them: a check that never reports, such as one whose workflow skips the PR, would otherwise hold it
* in the queue without a word.
* Comment once per pick on each PR still waiting `STALLED_AFTER` its pick, naming what it waits for: a check
* that never reports, such as one whose workflow skips the PR, or an approval nobody gives would otherwise
* hold it in the queue without a word.
*/
async function reportStalled(github: GitHub, plan: Plan, state: string, now: number): Promise<string[]> {
const path = join(state, "stalled.json");
Expand All @@ -335,15 +335,20 @@ async function reportStalled(github: GitHub, plan: Plan, state: string, now: num
const output: string[] = [];
for (const waiting of plan.waiting ?? []) {
if (kept[waiting.number] === waiting.enabledAt || now - Date.parse(waiting.enabledAt) < STALLED_AFTER) continue;
const list = waiting.pending.map((name) => `- <code>${excerpt(name)}</code>`).join("\n");
const list = [
...waiting.pending.map((name) => `- the required check <code>${excerpt(name)}</code>`),
...(waiting.approval ? [`- an approving review, which ${plan.base.ref} requires`] : []),
].join("\n");
await github.request(`/repos/${plan.repository}/issues/${waiting.number}/comments`, {
body: {
body: `quest land: this pull request is queued, but ${waiting.pending.length === 1 ? "a required check is" : "required checks are"} still pending on its head ${STALLED_AFTER / 3_600_000} hours after it was picked:\n\n${list}\n\nIt keeps its place and joins a batch once they pass. A check that never reports, such as one whose workflow does not run on this pull request, holds it here: run that check, or ask a maintainer to change the checks ${plan.base.ref} requires.`,
body: `quest land: this pull request is queued, but its head still waits ${STALLED_AFTER / 3_600_000} hours after it was picked for:\n\n${list}\n\nIt keeps its place and joins a batch once nothing is left. A check that never reports, such as one whose workflow does not run on this pull request, or an approval nobody gives holds it here: run that check or get that review, or ask a maintainer to change what ${plan.base.ref} requires.`,
},
});
kept[waiting.number] = waiting.enabledAt;
save(path, kept);
output.push(`stalled ${waiting.number} ${waiting.pending.join(",")}`);
output.push(
`stalled ${waiting.number} ${[...waiting.pending, ...(waiting.approval ? ["approval"] : [])].join(",")}`,
);
}
save(path, kept);
return output;
Expand Down
Loading
Loading