From 2f102624b764f3d4c8a03d15acd2f22cc3ceedfd Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 2 Nov 2026 18:54:40 -0800 Subject: [PATCH 1/2] chore(quest): claim quest/m0/land-fixer/lockfile.md Quest-Claim: ca26a9bbdb6a0e36 From 08b8fa27f425d05372cb1d9617529a44459d928a Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 2 Nov 2026 18:59:24 -0800 Subject: [PATCH 2/2] feat(land): rebuild a conflicted lockfile from the base's copy The fixer's only lockfile rule was "regenerate lockfiles", with no side to start from, so a fixer merge on moq-dev/moq#7318 rebuilt bun.lock from the PR and reverted the base's advisory fix. The prompt now gives a Lockfiles procedure: take the base's copy, run the package manager's minimal resolve against the merged manifests, reapply the PR's own lockfile-only bumps with a pinned update, check every change traces to the PR, and hand back when it cannot. Co-Authored-By: Claude Opus 5.5 --- assets/land-fix.md | 42 +++++++++++++++++++- quest/m0/README.md | 2 +- quest/m0/land-fixer/README.md | 8 ++-- quest/m0/land-fixer/lockfile.md | 69 --------------------------------- test/land.test.ts | 22 +++++++++++ 5 files changed, 69 insertions(+), 74 deletions(-) delete mode 100644 quest/m0/land-fixer/lockfile.md diff --git a/assets/land-fix.md b/assets/land-fix.md index 60ca5033..39bcc494 100644 --- a/assets/land-fix.md +++ b/assets/land-fix.md @@ -28,7 +28,8 @@ For each PR, requeue it unchanged if healthy (if the failure was flaky, name the flaky test in its `failure` so it can be followed up), commit a mechanical fix on its own branch, or hand it back with diagnosis and recommendation. Allowed fixes: preserve both sides' intent in conflicts, format/lint, regenerate -lockfiles/generated files, or follow a base rename. Merge base where practical. +generated files, rebuild lockfiles as [Lockfiles](#lockfiles) says, or follow a +base rename. Merge base where practical. Never edit flake.nix, flake.lock, shell.nix, default.nix, nix/, .envrc, .github/, .gitmodules, or gitlinks (submodule entries) anywhere. Merging the base may carry the base's own changes to those paths: that is allowed when they come out exactly as @@ -74,6 +75,45 @@ 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. +## Lockfiles + +When merging the base leaves a lockfile (`bun.lock`, `package-lock.json`, +`pnpm-lock.yaml`, `yarn.lock`, `Cargo.lock`, `uv.lock`, `poetry.lock`, `go.sum`, +and the like) conflicted or out of date with the merged manifests, rebuild it from +the base's copy, never from the PR's side: the base's copy carries dependency +changes the base made, such as a security bump, that the PR's copy would revert. + +1. Resolve the manifests first (`package.json`, `Cargo.toml`, `pyproject.toml`, + `go.mod`, and the like), preserving both sides' intent. +2. Take the base's lockfile: `git checkout base -- `. +3. Run the package manager's minimal resolve against the merged manifests, such + as `bun install`, `npm install`, `cargo update --workspace` or a `cargo build`, + `uv lock`, or `go mod tidy`. Never run `cargo generate-lockfile`, `bun update`, + `uv lock --upgrade`, a `cargo update` without `--workspace`, or another update + that moves versions neither side asked for. +4. Reapply any lockfile change the PR made without a manifest change, such as a + `cargo update -p`, with a pinned targeted update such as + `cargo update -p --precise `. Where the base also moved that + package, keep the base's version if it is at or past the PR's. +5. Compare the result with the base's copy (`git diff base -- `) and + with the PR's own changes (`git diff base...pr- -- `): + every lockfile change, and any manifest change the package manager made itself + (such as `go mod tidy` editing `go.mod`), must trace to a dependency or import + change in the PR. + +Name the lockfiles you rebuilt in the reason. Hand back instead of taking either +side's copy, quoting the command's output or the offending diff in the diagnosis, +when: + +- the package manager cannot rebuild the lockfile, or cannot reapply the PR's own + lockfile change with a pinned update; +- a change does not trace to the PR; +- the PR's manifest no longer admits a version the base moved to, such as a pin + below the base's security bump, or the base moved a package below the PR's + version: keeping either version is a behavior decision, not a mechanical fix. + +`flake.lock` is a protected path: this section does not let you edit it. + ## Blame The supplied PRs stay queued while you work. First decide which of them caused diff --git a/quest/m0/README.md b/quest/m0/README.md index aa77a9c2..dc66d90a 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -84,7 +84,7 @@ ownership-manifest design: - [Land through the merge queue with an async fixer](/quest/m0/queue-watcher/README.md) - GitHub's merge queue lands one PR at a time, and a host watcher fixes and re-enqueues each PR it drops - [Merge through a host gate](/quest/m0/host-land/README.md) - the host's fixer containers, and the frozen batched gate for the repositories not yet switched -- [The land fixer resolves every mechanical failure](/quest/m0/land-fixer/README.md) - lockfile merges, protected edits as ready commits, workflow pushes, and the PR's own CI as context +- [The land fixer resolves every mechanical failure](/quest/m0/land-fixer/README.md) - protected edits as ready commits, workflow pushes, and the PR's own CI as context - [Current landing status](/quest/m0/status-landing.md) - `quest status` drops a stale `landing-stopped` line and ignores superseded cancelled checks - [Agents don't stall](/quest/m0/agent-stalls.md) - long waits block in the same turn, pre-existing failures don't stop review, and reviewer outages become waits - [Follow-ups as agents report](/quest/m0/follow-ups-as-reports.md) - coordinators plan follow-ups as reports arrive, fewer of them, with standing milestone rules diff --git a/quest/m0/land-fixer/README.md b/quest/m0/land-fixer/README.md index b539ce2e..33c3e087 100644 --- a/quest/m0/land-fixer/README.md +++ b/quest/m0/land-fixer/README.md @@ -125,8 +125,11 @@ Decision prompts, selected options marked ✅: closed unmerged by the merge-queue switch below. - Amended 2026-11-01 (maintainer, /quest-plan in moq-dev/moq): a fixer merge on [moq-dev/moq#7318](https://github.com/moq-dev/moq/pull/7318) rebuilt - `bun.lock` from the PR's side and undid the base's advisory fix; see - [Lockfile merges](/quest/m0/land-fixer/lockfile.md). + `bun.lock` from the PR's side and undid the base's advisory fix. + [moq-dev/quest#410](https://github.com/moq-dev/quest/pull/410) gave + `assets/land-fix.md` a Lockfiles procedure: rebuild from the base's copy with + the package manager's minimal resolve, and hand back any change that does not + trace to the PR. - Superseded 2026-11-01 (maintainer, /quest-plan; see the [queue watcher](/quest/m0/queue-watcher/README.md)): repositories land @@ -152,7 +155,6 @@ work, done once the Required quests below have merged. ## Required - [Workflow permission](/quest/m0/land-fixer/workflows-permission.md) - waits on the maintainer to grant moq-bot `workflows`, so pushes touching workflows are accepted -- [Lockfile merges](/quest/m0/land-fixer/lockfile.md) - a conflicted lockfile is rebuilt from the base's copy, so a fix never reverts the base's dependency changes - [Protected edits as ready commits](/quest/m0/land-fixer/protected-commit.md) - a fixer edit `validateFix` rejects for touching protected paths comes back as a commit the maintainer applies - [PR CI as a signal](/quest/m0/land-fixer/pr-signal.md) - `/quest-merge` and the fixer read a PR's own CI without gating on it; public repos may run it, private ones need not diff --git a/quest/m0/land-fixer/lockfile.md b/quest/m0/land-fixer/lockfile.md deleted file mode 100644 index f37fd547..00000000 --- a/quest/m0/land-fixer/lockfile.md +++ /dev/null @@ -1,69 +0,0 @@ -# [S] The fixer rebuilds a conflicted lockfile from the base's copy - -## Goal - -When the fixer resolves a lockfile while merging the base (`bun.lock`, -`Cargo.lock`, `uv.lock`, `go.sum`, and the like), it starts -from the base's copy and runs the package manager, so the result adds only -the PR's own dependency changes. A fixer merge never reverts a dependency -change the base made, such as a security bump. - -Non-goals: running `bun audit` or another advisory scan in a repository's -land check; that is each repository's CI choice. - -## Plan - -Decided 2026-11-01 (maintainer, /quest-plan in moq-dev/moq). After `quest land` -removed [moq-dev/moq#7318](https://github.com/moq-dev/moq/pull/7318) with -`merge_conflict`, the fixer pushed its own merge of main, -[229973492b](https://github.com/moq-dev/moq/commit/229973492ba403cd414161c69c98e67680907e23), -which rebuilt `bun.lock` from the PR's side instead of main's. That undid -[moq-dev/moq#7443](https://github.com/moq-dev/moq/pull/7443)'s glob/minimatch -advisory fix: `glob@10.5.0` (with `jackspeak` and `path-scurry@1.11.1`) and -`minimatch@9.0.9` came back, and `bun audit` reported two moderate advisories, -GHSA-3g3m-2fjr-6pw4 and GHSA-g3hv-75wg-m249. The land check missed it because -moq runs `bun audit` only in Nightly. Redoing the merge from main's `bun.lock` -plus `bun install` added only the PR's packages and audited clean -([evidence](https://github.com/moq-dev/moq/pull/7318#issuecomment-6558969580)). - -- ✅ Plan it: start from the base's lockfile and regenerate, size S, in this - epic (m0) - -The fixer's merge resolution is Codex following `assets/land-fix.md`, whose -only lockfile rule is "regenerate lockfiles/generated files"; nothing says -which side to start from. Replace it with a procedure: for each lockfile the -merge touches, check out the `base` branch's copy (`git checkout base -- -`), apply the PR's manifest changes (merged `package.json`, -`Cargo.toml`, and so on), and run the package manager's minimal resolve -(`bun install`, a `cargo` build or `cargo update --workspace`, `uv lock`, -`go mod tidy`; never `cargo generate-lockfile` or an unpinned update). Then -compare the result with the base's copy and the merged manifests: every -lockfile change, and any manifest change the package manager itself made -(such as `go mod tidy` editing `go.mod`), must trace to a dependency or import -change in the PR. The fix reason names the lockfiles it rebuilt. - -Hand back, quoting the command's output or the offending diff, instead of -taking either side's copy when: - -- the package manager cannot rebuild the lockfile; -- a change does not trace to the PR; -- the PR's manifest no longer admits a version the base moved to, such as a - pin below the base's security bump. Keeping either version is a behavior - decision, not a mechanical fix. - -The prompt already lets build tools fetch from package registries (from -[moq-dev/quest#356](https://github.com/moq-dev/quest/pull/356)), which -regeneration needs. `flake.lock` stays a protected path in `validateFix`, and -this quest does not open it: a fixer-written `flake.lock` comes back as a -ready commit through -[Protected edits as ready commits](/quest/m0/land-fixer/protected-commit.md), -and that edit follows the same rule, rebuilt from the base's copy with -`nix flake lock`. Both quests edit `assets/land-fix.md`, so whichever lands -second merges the other's wording. - -Cover the rule in `test/land.test.ts`, which already asserts the prompt's -contents. - -## Related - -- [Protected edits as ready commits](/quest/m0/land-fixer/protected-commit.md) - returns a fixer-written `flake.lock` as a ready commit, built by this quest's rule diff --git a/test/land.test.ts b/test/land.test.ts index 6cde2067..07fd201b 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -3259,6 +3259,28 @@ ${requeueScript}`); ]) expect(fixerPrompt).toContain(text); }); + test("rebuilds a conflicted lockfile from the base's copy", () => { + const prompt = fixerPrompt.replace(/\s+/g, " "); + expect(prompt).not.toContain("regenerate lockfiles"); + for (const text of [ + "rebuild lockfiles as [Lockfiles](#lockfiles) says", + "## Lockfiles", + "rebuild it from the base's copy, never from the PR's side", + "`git checkout base -- `", + "minimal resolve against the merged manifests", + "Never run `cargo generate-lockfile`", + "`cargo update -p --precise `", + "keep the base's version if it is at or past the PR's", + "must trace to a dependency or import change in the PR", + "Name the lockfiles you rebuilt in the reason.", + "Hand back instead of taking either side's copy", + "the package manager cannot rebuild the lockfile", + "a change does not trace to the PR", + "below the base's security bump", + "`flake.lock` is a protected path", + ]) + expect(prompt).toContain(text); + }); test("has the fixer put its comments in the result instead of posting them", () => { for (const text of [ "You have no GitHub credential and must not seek one.",