diff --git a/quest/m0/README.md b/quest/m0/README.md index 8c146ba2..9322d412 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -40,6 +40,11 @@ ownership-manifest design: ## Required - [Merge through a host gate](/quest/m0/host-land/README.md) - inline merge picks, small batches landed from the maintainer's host, and a Codex fixer instead of bisection +- [Landing priority](/quest/m0/land-priority.md) - cheap checks wait a bounded time and the land daemon's check runs ahead of agent builds +- [Local mergeability](/quest/m0/land-mergeability.md) - `quest land` merges with the quest driver instead of ejecting on GitHub's conflict flag +- [Accurate head_moved](/quest/m0/land-head-moved.md) - the gate's own base-merge push and batch-mate failures are not reported as a moved head +- [Queue-time checks](/quest/m0/land-queue-checks.md) - queued PRs are checked against each other and `quest check` runs per PR before batching +- [Drop emptied Required headings](/quest/m0/required-heading.md) - a clean driver merge that empties a `Required` list passes `quest check` - [Codex cloud off](/quest/m0/codex-cloud-off.md) - automatic Codex cloud reviews stop once no open PR relies on them - [Review handles expired and environment logins](/quest/m0/review-login.md) - `quest review` says to log in again instead of failing with a 401, and reviews with credentials from the environment - [Live sandbox test](/quest/m0/review-sandbox.md) - the real reviewer CLIs run nothing a reviewed head plants diff --git a/quest/m0/host-land/README.md b/quest/m0/host-land/README.md index fb0ad4b3..f2bc84cf 100644 --- a/quest/m0/host-land/README.md +++ b/quest/m0/host-land/README.md @@ -87,8 +87,13 @@ can hand off instead. PR. The healthy PR lands after the requeue, the mechanical one lands after its fix, and the other is handed back. `CONTRIBUTING.md` and the README's landing section describe the host gate. +- Amended 2026-10-31 (user): the fixer triages a failing batch before + anything is ejected, and only the PR it blames is ejected. See + [batch triage](/quest/m0/host-land/triage.md). ## Required - [Gate host](/quest/m0/host-land/host.md) - kixelated/nixos runs the daemon as its own user, checks in nspawn containers, and retires the Actions runner - [Fixer proxy access](/quest/m0/host-land/fixer.md) - connect the nixos fixer container to CLIProxyAPI with the gate client key and maintainer login +- [Fixer survives ejection](/quest/m0/host-land/fixer-survives.md) - the fixer finishes after every ejection and requeues collateral PRs itself +- [Batch triage](/quest/m0/host-land/triage.md) - the fixer blames the failing PR before ejection, and a red base halts the queue diff --git a/quest/m0/host-land/fixer-survives.md b/quest/m0/host-land/fixer-survives.md new file mode 100644 index 00000000..96174772 --- /dev/null +++ b/quest/m0/host-land/fixer-survives.md @@ -0,0 +1,37 @@ +# [S] The fixer survives an ejection and recognises collateral PRs + +## Goal + +After every batch ejection on moq-dev/moq, the fixer runs to completion and +records a result for each PR. A PR that was ejected only because it shared a +batch with the failing one is recognised as collateral and requeued, so no +maintainer has to hand it back. When the fixer does fail, its handback says +why, using its exit status and the end of its log. + +## Plan + +Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq +`/quest-complete` run). On 2026-10-31 the fixer crashed on every ejection, so +the maintainer handed collateral PRs back by hand +([moq-dev/moq#7416](https://github.com/moq-dev/moq/pull/7416), +[moq-dev/moq#7413](https://github.com/moq-dev/moq/pull/7413)). The failures +seen: + +- Exit 101, after a flood of `cargo_cyclonedx` "Hash not found in + Cargo.lock" errors. The fixer ran a check the container cannot complete, + or one that does not apply to the failure. +- Exit 143, which means it was killed by SIGTERM: the timeout in + `collectJob` (`src/cli/land-fixer.ts`), or the host stopping the unit. +- "fixer disappeared before recording an exit status; run /quest-iterate" + (`collectJob`). That text hides which of the two happened. Record the + exit status, or that the daemon restarted, and the tail of + `fixerLog`. + +Find each crash's cause in the host's per-job logs before changing anything. +Fix what is quest's here, and what is the host's wrapper or container in +kixelated/nixos as part of the [gate host](/quest/m0/host-land/host.md). +Cover a crashing fixer and a collateral requeue in the fixer tests. + +## Related + +- [Fixer proxy access](/quest/m0/host-land/fixer.md) - the fixer container's model access diff --git a/quest/m0/host-land/triage.md b/quest/m0/host-land/triage.md new file mode 100644 index 00000000..b8f9a2f3 --- /dev/null +++ b/quest/m0/host-land/triage.md @@ -0,0 +1,58 @@ +# [M] The fixer triages a failing batch before anything is ejected + +## Goal + +When a batch's check fails, the fixer reads the failure first and names the +PR that caused it. `quest land` ejects only that PR and keeps the rest +queued. When the fixer cannot tell, it checks the base tip alone. If the +base is red, the queue halts and says so, ejecting nothing. If the base is +green, every PR in the batch is ejected, as today. Bisection stays gone, as +the [epic](/quest/m0/host-land/README.md) decided. + +## Plan + +Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq +`/quest-complete` run). On 2026-10-31 moq's gate ejected whole batches at +02:38, 03:01, 05:40, 06:21, 07:10, and 09:23. The fixer later found most of +those PRs healthy and requeued them. The user asked: "could the fixer not +figure it out? It should run on each batch before ejecting, instead of +wasting CI on bisection?" + +- Goal, restated as "the fixer runs on each failing batch before anything is + ejected, and blames the PR that caused the failure": + - ✅ Yes, as restated + - Keep ejecting the whole batch, and only speed up the requeue + - Bring back bisection +- The user's refinement: "The fixer reads the failure, tries to identify the + PR that caused it, and only ejects that PR. If it can't figure it out, it + ejects all PRs". +- Repairing the blamed PR does not change: one attempt, after it is ejected. +- Blame is unclear, so check the base tip alone. A red base halts the queue + with one visible notice instead of ejecting PRs that did nothing wrong + ([moq-dev/moq#7196](https://github.com/moq-dev/moq/pull/7196), + [moq-dev/moq#7085](https://github.com/moq-dev/moq/pull/7085)). The queue + resumes once the base passes again. +- A new gate that fails on content already on the base blames the PR that + adds the gate. [moq-dev/moq#7317](https://github.com/moq-dev/moq/pull/7317) + added a zizmor check that failed on the base's `pullfrog.yml`, and six + unrelated PRs were ejected with it. +- The ejection comment keeps the failing module's own output. Today + `serveBatch` in `src/cli/land-serve.ts` keeps only the log's last 100 + lines, so a failure early in the log is cut off. The cap is + `EXCERPT` in the same file. Prefer the fixer's diagnosis and the failing + step's own lines over a plain tail. +- Today a failed check builds one `Failure` whose `ejections` cover every + link in the chain (`src/cli/land-serve.ts`). `startFixer` in + `src/cli/land-fixer.ts` then runs on PRs that are already ejected. Triage + moves the fixer's run before those ejections. Its result needs a blame + field beside `requeue`, `fixed`, and `handback` in `src/cli/land-fix.ts`. +- Keep the fixer's untrusted-input rules: a PR's content cannot make the + fixer blame another PR or skip the base check. + +## Required + +- [Fixer survives ejection](/quest/m0/host-land/fixer-survives.md) - the fixer has to finish before its triage can be trusted + +## Related + +- [Fixer proxy access](/quest/m0/host-land/fixer.md) - how the fixer reaches its model diff --git a/quest/m0/land-head-moved.md b/quest/m0/land-head-moved.md new file mode 100644 index 00000000..ef2e0e2f --- /dev/null +++ b/quest/m0/land-head-moved.md @@ -0,0 +1,29 @@ +# [S] quest land reports head_moved only when a writer moved the head + +## Goal + +`quest land` ejects a PR as `head_moved` only when someone other than the +gate pushed to it after it was queued. The gate's own base-merge push is +never counted as a moved head. A PR stopped because a batch-mate conflicted +or moved is ejected with that reason, naming the batch-mate. + +## Plan + +Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq +`/quest-complete` run). PRs were wrongly ejected as `head_moved`: +[moq-dev/moq#7407](https://github.com/moq-dev/moq/pull/7407), +[moq-dev/moq#7416](https://github.com/moq-dev/moq/pull/7416), +[moq-dev/moq#7353](https://github.com/moq-dev/moq/pull/7353), +[moq-dev/moq#7413](https://github.com/moq-dev/moq/pull/7413). + +- Own push. `prepareHead` (`src/cli/land-head.ts`) pushes a base merge to a + PR's head, and `isPrepared` recognises that head from a private record. Land + still reported `head_moved` after that push, so one of the checks in + `resolveQueue` (`src/cli/land.ts`), `prepareHead`, or `mergePinned` + (`src/cli/land-serve.ts`) missed it. + Find the case the record misses, such as GitHub lagging, a restart, or + `forgetHeads` running too early, and cover it with a test. +- Batch-mates. When one link fails to merge, `serveBatch` + (`src/cli/land-serve.ts`) ejects every remaining link with that link's + reason and detail. Give the other PRs their own reason, one that names + the PR that stopped the batch. diff --git a/quest/m0/land-mergeability.md b/quest/m0/land-mergeability.md new file mode 100644 index 00000000..acaa9d56 --- /dev/null +++ b/quest/m0/land-mergeability.md @@ -0,0 +1,22 @@ +# [XS] quest land merges locally instead of trusting GitHub's mergeability + +## Goal + +`quest land` never ejects a PR as `merge_conflict` because of GitHub's +mergeability alone. When the squash API refuses a PR that GitHub marks as +conflicting, the gate merges the base into it locally with the quest merge +driver, pushes that merge, and lands the PR. It ejects only when the local +merge also conflicts. + +## Plan + +Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq +`/quest-complete` run). GitHub reported conflicts on +[moq-dev/moq#7407](https://github.com/moq-dev/moq/pull/7407), which merges +cleanly, and on [moq-dev/moq#7382](https://github.com/moq-dev/moq/pull/7382), +whose only conflict was in quest link lists, which the driver resolves. + +`mergePinned` (`src/cli/land-serve.ts`) returns `merge_conflict` when +`pull.mergeable === false` or `mergeable_state === "dirty"`. Hand that case +to `prepareHead` (`src/cli/land-head.ts`), which already merges with the +driver through `mergeBase` (`src/cli/land.ts`), instead of ejecting. diff --git a/quest/m0/land-priority.md b/quest/m0/land-priority.md new file mode 100644 index 00000000..3663d4cd --- /dev/null +++ b/quest/m0/land-priority.md @@ -0,0 +1,34 @@ +# [S] Landing and cheap checks are admitted ahead of agent load + +## Goal + +On a loaded host, a cheap check such as `quest check` or a formatter waits +at most a short bounded time in `quest gate`, and the land daemon's batch +check is admitted ahead of agent builds. The queue keeps landing even while +agents load the host. + +## Plan + +Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq +`/quest-complete` run). With load between 33 and 67, cheap checks waited an +hour behind `quest gate` +([moq-dev/moq#7250](https://github.com/moq-dev/moq/pull/7250), +[moq-dev/moq#7257](https://github.com/moq-dev/moq/pull/7257)). On +2026-10-31 the land daemon sat idle from 05:07Z until after 06:45Z, with +about 80 PRs queued, while agent gates kept the host loaded. + +- Cheap checks: admit them after a bounded wait, or let them bypass the + gate. Decide how a command qualifies: a flag the caller passes, or a + duration it declares. Do not keep a list of command names. +- Landing priority: the daemon's check comes before agent work. `admit` in + `src/cli/gate.ts` takes no priority today, and the ledger is first come, + first served. Find out whether the batch check reaches the gate through the + host's sandbox launcher, then add a priority class or reserve a slot for + it. +- Keep the gate's purpose: an unbounded bypass would let agent builds starve + the host again. + +## Related + +- [Gate admission on CI](/quest/m0/gate-ci-admission.md) - admission when load sits high for other reasons +- [Gate bench](/quest/m0/gate-bench.md) - admission measurements on an idle host diff --git a/quest/m0/land-queue-checks.md b/quest/m0/land-queue-checks.md new file mode 100644 index 00000000..97de226b --- /dev/null +++ b/quest/m0/land-queue-checks.md @@ -0,0 +1,35 @@ +# [S] quest land checks queued PRs before batching them + +## Goal + +Problems that one PR can be blamed for are caught before that PR joins a +batch, so they never eject the whole batch. When a PR is queued, `quest +land` reports which other queued PRs it conflicts with. Before batching, +each PR passes `quest check` against the current base, and a PR that fails +is ejected alone with that output. + +## Plan + +Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq +`/quest-complete` run). + +- Queued PRs that conflict with each other: + [moq-dev/moq#6994](https://github.com/moq-dev/moq/pull/6994) and + [moq-dev/moq#7196](https://github.com/moq-dev/moq/pull/7196) both edited + `drafts/draft-lcurley-moq-lite.md`. Report the overlap at queue time, so + the later PR's author can fix it before its turn comes. `build` in + `src/cli/land.ts` already merges each PR onto the chain with the quest + driver, and can run that merge pairwise. +- Dangling links: [moq-dev/moq#7135](https://github.com/moq-dev/moq/pull/7135) + linked to a quest the base had already finished, which failed its batch. + Run `quest check` on each PR merged onto the current base, a cheap step, + before the full batch check. +- [Sibling PRs through a server-side merge](/quest/m0/helpers/queue-siblings.md) + owns the same dead-link failure on gates that are not `quest land`, and + may instead make `quest check` tolerate links to finished quests. Whichever + lands first, the other quest drops what is already covered. + +## Related + +- [Sibling PRs through a server-side merge](/quest/m0/helpers/queue-siblings.md) - the dead-link failure on other gates +- [Batch triage](/quest/m0/host-land/triage.md) - blame for failures these checks miss diff --git a/quest/m0/required-heading.md b/quest/m0/required-heading.md new file mode 100644 index 00000000..b3ed7bdd --- /dev/null +++ b/quest/m0/required-heading.md @@ -0,0 +1,25 @@ +# [XS] The merge driver drops a Required heading it empties + +## Goal + +After a merge where the `quest resolve` driver removes the last entry from +an epic's `Required` list, the `## Required` heading is gone too, and +`quest check` passes on the merged tree. + +## Plan + +Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq +`/quest-complete` run). [moq-dev/moq#7251](https://github.com/moq-dev/moq/pull/7251) +merged cleanly, then failed `quest check`. The driver had emptied a +`Required` list and left its heading, but the guide says an empty `Required` +section is deleted. + +`resolveLists` (`src/core/merge.ts`) merges only the conflicting entries, +so it never sees the heading. After resolving, drop a `Required` heading +whose section is now empty. Leave other empty sections to `quest check`, which +should report them. Test it with both sides each removing one of the last two +entries. + +## Related + +- [Visible merge-driver fallback](/quest/m0/helpers/merge-driver.md) - the driver's other gap diff --git a/quest/m0/review-login.md b/quest/m0/review-login.md index 9dc82e7e..83923c06 100644 --- a/quest/m0/review-login.md +++ b/quest/m0/review-login.md @@ -4,8 +4,9 @@ When a reviewer CLI's login has expired or been revoked, `quest review` fails with the same "`` is not logged in; run ``" error it gives a -signed-out CLI, instead of a raw 401 from the reviewer's turn. A reviewer -whose only credentials come from the environment still reviews. +signed-out CLI, instead of a raw 401 from the reviewer's turn. The error +names the Claude profile it checked. A reviewer whose only credentials come +from the environment still reviews. ## Plan @@ -38,3 +39,15 @@ Decided 2026-10-30 (user, /quest-plan of the follow-ups from a moq-dev/moq variable instead of saying "not logged in". - Test with a fake reviewer that authenticates only through an environment variable. + +Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq +`/quest-complete` run): the "not logged in" error names the +`CLAUDE_CONFIG_DIR` profile it checked, or the default when unset. Sessions +running with `~/.claude-personal` were told "claude is not logged in" with no +hint of which profile was read +([moq-dev/moq#7245](https://github.com/moq-dev/moq/pull/7245), +[moq-dev/moq#7210](https://github.com/moq-dev/moq/pull/7210)). The error is +raised in `src/cli/review.ts`'s status check. +[moq-dev/quest#316](https://github.com/moq-dev/quest/pull/316) (preserve +reviewer connection credentials) covers the environment-credential part +above and removes it from this quest when it lands. diff --git a/quest/m1/README.md b/quest/m1/README.md index 9e879137..238d1cd7 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -15,6 +15,7 @@ additional login providers. This milestone does not gate the first release. - [Donor isolation](/quest/m1/donor-isolation.md) - stronger boundaries for untrusted shared jobs - [ChatGPT hosted runs](/quest/m1/chatgpt-hosted.md) - conditional on provider approval and current terms - [ChatGPT approval](/quest/m1/chatgpt-waitlist.md) - resolve hosted-use approval before implementation +- [Current landing status](/quest/m1/status-landing.md) - `quest status` drops a stale `landing-stopped` line and ignores superseded cancelled checks - [Worker dev setup](/quest/m1/worker-dev.md) - per-server cookies, seeded raw output, static assets, one fake GitHub, each test run alone - [Gate ownership everywhere](/quest/m1/gate-ownership.md) - delegated sub-cgroups for nested gates and Windows Job Objects - [Fewer git processes on Windows](/quest/m1/windows-git.md) - batched git lookups in the CLI and reused fixture repositories in tests diff --git a/quest/m1/status-landing.md b/quest/m1/status-landing.md new file mode 100644 index 00000000..8a2a6405 --- /dev/null +++ b/quest/m1/status-landing.md @@ -0,0 +1,22 @@ +# [XS] quest status reports landing and checks as they are now + +## Goal + +`quest status` stops printing a `landing-stopped` line once the PR has been +queued again, including through `quest merge --land`. A required check whose +newest run passed counts as passed, even if an older run of it was +cancelled. + +## Plan + +Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq +`/quest-complete` run). + +- `summarize` in `src/cli/status.ts` takes the last removal, auto-merge + disable, or ejection comment from the timeline, even if auto-merge was + turned on again later. Drop the line when auto-merge's `enabledAt`, or a + merge-queue entry, is newer than that event. +- Checks are classified one context at a time in `summarize` (`classifyCheck`). + A cancelled run and a newer passing run of the same check both count, so + the check still shows as failing. Count only the newest run of each check + name.