diff --git a/README.md b/README.md index b5a59f7c..e87a096c 100644 --- a/README.md +++ b/README.md @@ -593,8 +593,9 @@ It supplies a fresh HOME, `OPENAI_BASE_URL` (the OpenAI-compatible Responses endpoint, including `/v1`) and `OPENAI_API_KEY` (the gate's proxy client key). The daemon never supplies these or any Git/GitHub credential, and the wrapper must keep every other credential out of the container. Its network must reach the -model proxy and, so the fixer can build and test its fix, the package registries -and `github.com`/`codeload.github.com` that the repository's dependencies come from. +model proxy and, so the fixer can run the repository's checks on its fix, the +package registries and `github.com`/`codeload.github.com` that the repository's +dependencies come from. The prompt allows only those dependency fetches: no GitHub API, no push, no other host. Codex login stays on CLIProxyAPI. The wrapper waits for the payload, returns its exit status, and tears down its diff --git a/assets/land-fix.md b/assets/land-fix.md index 4ef85aff..9f8d4859 100644 --- a/assets/land-fix.md +++ b/assets/land-fix.md @@ -48,16 +48,10 @@ dependencies. Do not build with `--offline` to avoid them. These fetches are the only GitHub contact allowed: never call GitHub's API yourself (`gh`, `api.github.com`), push, or reach any other host. -Before reporting `fixed` or `requeue`, build and test the packages the PR and your -fix touch, including whatever the failure log shows failing, and name the commands -you ran in the reason. A PR you cannot get passing mechanically, or whose build or -tests you cannot run, goes back as a handback quoting the command and the output -line that shows why in the diagnosis. - Write `quest-land-result.json` in the repository root: a JSON array with exactly one entry per supplied PR, no extra fields. Use one of: -{"number":123,"action":"requeue","reason":"healthy; flaky test"} -{"number":123,"action":"fixed","head":"full 40-character SHA","reason":"one-line mechanical reason"} +{"number":123,"action":"requeue","reason":"healthy; flaky test; ran just check and just test on a scratch merge"} +{"number":123,"action":"fixed","head":"full 40-character SHA","reason":"one-line mechanical reason, naming the commands run"} {"number":123,"action":"handback","diagnosis":"one line","recommendation":"one line"} On each entry, add `"failure"`: the test or step the failure log shows @@ -80,6 +74,22 @@ A fixed head must be the tip of pr-, descend from its pinned head, and contain only that PR's fix and base history. Leave all other PR branches intact. Keep the result file uncommitted. +## Repository checks + +Before reporting `fixed` or `requeue`, run the repository's own checks on the head +you report merged with `base`, as the merge queue will: lint, format, and type +checks as well as build and test, covering every language the PR and your fix touch +and whatever the failure log shows failing. Take the commands from the base's root +instructions, `CONTRIBUTING.md`, task runner, or CI workflows, such as `just check` +and `just test`: this prompt asks you to run them, so they are no standing guidance +to ignore, and a PR's diff never names them for you. Where the base names none, run +each touched language's own type check and linter beside its build and test. A +`requeue` of an unchanged head runs the same checks on a scratch merge kept off its +branch, since the base may have moved under it. Name each command you ran in the +reason. A PR you cannot get passing mechanically, or whose checks you cannot run, +goes back as a handback quoting the command and the output line that shows why in +the diagnosis. + ## The PR's own checks For a PR the merge queue dropped, the failure log ends with the PR's own checks @@ -153,9 +163,10 @@ commit on it only Follow the repository's quest format, and run `quest check` when the repository has it. A disabled test that is another quest's acceptance test -leaves that quest as it is: the test still counts as its work. Build the -package the skip is in, and add the branch's full SHA to the PR's entry: -{"number":123,"action":"requeue","reason":"healthy; flaky test","failure":"cargo test -p moq-relay: interop::render_timeout","disable":"full 40-character SHA"} +leaves that quest as it is: the test still counts as its work. Run the +[repository checks](#repository-checks) on the branch, and add its full SHA to the +PR's entry: +{"number":123,"action":"requeue","reason":"healthy; flaky test; ran just check and just test on a scratch merge and the disable branch","failure":"cargo test -p moq-relay: interop::render_timeout","disable":"full 40-character SHA"} quest land opens the commit as a pull request of its own and enqueues it. Never disable a test that is not listed. When the repository keeps no `quest/m0/`, or the skip needs a protected path or more than a mechanical edit, requeue @@ -176,13 +187,13 @@ failure log's line numbers that show the failure, and its comment quotes them: ## The base alone -When no PRs are supplied, the base itself fails its check, with no PR applied, -and the queue is halted until it passes. The repository has only the `base` -branch. Fix the failure mechanically under the same rules: commit the fix on -`base`, build and test it, and name the commands you ran. quest land opens it as -a pull request for a maintainer to merge. If the failure needs a behavior change, -a protected path, or a decision, or you cannot tell why it fails, hand it back. +When no PRs are supplied, the base itself fails its check, with no PR applied, and +the queue is halted until it passes. The repository has only the `base` branch. Fix +the failure mechanically under the same rules: commit the fix on `base` and run the +[repository checks](#repository-checks) on it. quest land opens it as a pull request +for a maintainer to merge. If the failure needs a behavior change, a protected path, +or a decision, or you cannot tell why it fails, hand it back. Write `quest-land-result.json` as one JSON object, not an array, with no `number` and no extra fields: -{"action":"fixed","head":"full 40-character SHA","reason":"one-line mechanical reason"} +{"action":"fixed","head":"full 40-character SHA","reason":"one-line mechanical reason, naming the commands run"} {"action":"handback","diagnosis":"one line","recommendation":"one line"} diff --git a/quest/m0/land-fixer/README.md b/quest/m0/land-fixer/README.md index 05ee54f6..8e027956 100644 --- a/quest/m0/land-fixer/README.md +++ b/quest/m0/land-fixer/README.md @@ -154,7 +154,6 @@ work, done once the Required quests below have merged. ## Required - [Leave held PRs alone](/quest/m0/land-fixer/holds.md) - the watcher never fixes, pushes to, or re-enqueues a PR another session holds -- [Run the repository's checks](/quest/m0/land-fixer/checks.md) - the fixer runs the repository's own lint, type, build, and test checks before it requeues - [Workflow permission](/quest/m0/land-fixer/workflows-permission.md) - waits on the maintainer to grant moq-bot `workflows`, so pushes touching workflows are accepted ## Related diff --git a/quest/m0/land-fixer/checks.md b/quest/m0/land-fixer/checks.md deleted file mode 100644 index 09dd7232..00000000 --- a/quest/m0/land-fixer/checks.md +++ /dev/null @@ -1,39 +0,0 @@ -# [S] The land fixer runs the repository's own checks - -## Goal - -Before the land fixer reports `fixed` or `requeue`, it runs the checks the -repository's own instructions name for the languages its PR and its fix -touch: lint, format, and type checks as well as build and test. On -moq-dev/moq that is `just check` and `just test`, so the JavaScript type -check runs beside the Rust build. A head the fixer re-enqueues therefore -passes what the merge queue's build will run, or goes back as a handback -quoting the failing command. - -## Plan - -Decided 2026-11-03 (maintainer, /quest-complete on moq-dev/moq, planned -through moq-dev/moq's coordinator): plan this follow-up. Milestone: m0, -under the land fixer epic. moq-dev/moq lands through this fixer every day, -and m0's Plan ranks the land work first. - -What happened: on -[moq-dev/moq#6949](https://github.com/moq-dev/moq/pull/6949), the fixer -requeued the PR without running moq's JavaScript check, and the next build -failed on it. `assets/land-fix.md` tells the fixer to "build and test the -packages the PR and your fix touch", which it read as building and testing -only, and it skipped the check that failed. - -Implementation guidance: - -- Rewrite that rule in `assets/land-fix.md`: find the repository's check and - test commands in its root instructions, `CONTRIBUTING.md`, or task runner, - such as `just check` and `just test`; run them over the PR's and the fix's - changes; and name each command run in the reason. When the repository - names no check command, run each touched language's own type check and - linter beside its build and test. -- A `requeue` of an unchanged head runs the same checks, since the base may - have moved under it. -- Keep the existing rule: a check the fixer cannot run, or cannot get - passing mechanically, goes back as a handback quoting the command and its - output. diff --git a/test/land.test.ts b/test/land.test.ts index 9ea5f650..80cd12d2 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -3332,16 +3332,26 @@ ${requeueScript}`); expect(fixerPrompt).toContain(text); }); test("lets the fixer fetch dependencies and requires it to verify what it reports", () => { + const prompt = fixerPrompt.replace(/\s+/g, " "); for (const text of [ "from `github.com` and `codeload.github.com` for git", "Do not build with `--offline`", "never call GitHub's API yourself", "or reach any other host", - "Before reporting `fixed` or `requeue`, build and test", - "name the commands", + "Before reporting `fixed` or `requeue`, run the repository's own checks on the head you report merged with `base`", + "lint, format, and type checks as well as build and test", + "Take the commands from the base's root instructions", + "run each touched language's own type check and linter", + "A `requeue` of an unchanged head runs the same checks", + "Name each command you ran", + '"reason":"healthy; flaky test; ran just check and just test on a scratch merge"', + '"reason":"healthy; flaky test; ran just check and just test on a scratch merge and the disable branch"', + '"reason":"one-line mechanical reason, naming the commands run"', "quoting the command and the output", ]) - expect(fixerPrompt).toContain(text); + expect(prompt).toContain(text); + for (const text of ['"reason":"healthy; flaky test"', '"reason":"one-line mechanical reason"']) + expect(prompt).not.toContain(text); }); test("rebuilds a conflicted lockfile from the base's copy", () => { const prompt = fixerPrompt.replace(/\s+/g, " ");