Skip to content

feat(land): hand back a refused fix push as a ready commit - #358

Merged
moq-bot[bot] merged 5 commits into
mainfrom
quest/m0/land-fixer/fork-commit
Nov 1, 2026
Merged

moq-bot[bot] merged 5 commits into
mainfrom
quest/m0/land-fixer/fork-commit

Conversation

@kixelated

@kixelated kixelated commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m0/land-fixer/fork-commit.md. The quest file is deleted, and its links are gone from the land fixer epic and from protected.md.

Summary

When GitHub refuses the fixer's push of a validated fix, the handback now carries the resolution, so a maintainer doesn't have to redo the merge. This happened on moq-dev/moq#7439 and moq-dev/moq#7467: the fixer's merge of the base brought in .github/workflows/alert.yml, and moq-bot has no workflows permission.

  • Daemon side (src/cli/land-fixer.ts, the push-refusal branch of collectJob): when the push fails, it records a Resolution in the job state, so the record survives a restart. The Resolution holds the PR head SHA, the base SHA, the resolved tree, and the fixer's edits as a gzipped, base64 patch against the merge's conflicted result. The handback comment gets a section naming those three SHAs, the command quest land apply <pr>, and the payload in a hidden <!-- quest land apply v1 … --> line. comment() gives the section whatever room is left under GitHub's 65,536-character limit. If the section doesn't fit, the handback is the plain one plus a sentence saying the resolution was too large.
  • quest land apply <pr> (new src/cli/land-apply.ts): reads the latest resolution posted by an App ([bot] login, with the fixer's handback marker). It exits 3 if the PR is closed or its head is no longer the recorded one. Otherwise it fetches that head and base, recomputes the conflicted result, applies the patch to a temporary index, and checks the tree hash. It then commits with the maintainer's own git identity: parents are the head and the base, or just the head when the base is already in it. It prints the commit and a shell-quoted push command. It pushes nothing and doesn't touch the working tree.
  • Deterministic conflicted result: both sides run git merge-tree --write-tree <head> <base> with full SHAs, so conflict labels match. They read attributes from the empty tree (--attr-source), so no merge driver or conversion from either checkout applies. merge.conflictStyle, renames, directory renames, and renormalize are pinned. The test runs the maintainer side with merge.conflictStyle=zdiff3 and still reaches the fixer's tree. If git versions disagree, the tree check catches it and the command refuses.
  • gitEnvironment() is pulled out of tryGit in src/cli/land.ts so the byte-safe git runner can reuse it. README, CONTRIBUTING, and the land serve help describe the new handback.

Tests (test/land.test.ts, "quest land apply") run a real fixer job against the fake forge. A pre-receive hook refuses workflow changes the way GitHub does. Then apply rebuilds the commit in a separate checkout and the maintainer's push fast-forwards the branch. Further cases: a moved head, a non-App comment, a forged tree, a hostile branch name in the push command, and the size fallback.

Choices made without asking

  • Every refused push of a validated fix gets the resolution, not only refusals that mention workflows. A fork's branch rules or a lost lease fail the same way, and a maintainer's own push may still pass. A lost lease means the head moved, and then apply refuses anyway. This avoids matching on GitHub's error text.
  • Which comments count: the poster's login must end in [bot], which only an App can have, and the comment must carry the fixer's handback marker. A contributor can't plant a resolution. Even a forged one would add nothing beyond what the contributor's own branch can already hold, and the maintainer reviews the commit before pushing.
  • Payload encoding: gzip plus base64 in an HTML comment, so CRLF, non-UTF-8, and binary content survive the trip through GitHub and the comment stays compact. The three SHAs also appear in the visible text.
  • The commit is left unpushed and unreferenced, and its SHA and push command are printed. It is one commit (parents: head and base) with the maintainer's identity, and signing when configured, rather than the fixer's original commits. The tree is what's verified.
  • The size limit is counted in JavaScript string length (UTF-16 code units). That is at least the character count GitHub enforces, so the check is conservative.

Suggested follow-ups

  • Teach /quest-iterate about ready commits. When a handback comment contains quest land apply, the skill (assets/skills/iterate.md) should run it, review the printed commit, push it to the PR branch, and re-pick, instead of redoing the merge. Today only the comment itself tells the session.
  • Keep the fixer's commits when the maintainer's checkout can reach them. apply compresses the resolution into one merge commit. Bundling the fixer's actual commits (a git bundle, base64, under the same size budget) would keep their messages and authorship. That only matters if the authorship of the fixer's commits matters to reviewers.

Review

  • Reviewer: Codex (quest review), gpt-6.1-sol, medium effort; session 01a19849-f6c3-7813-a84f-3a55162994bd (new session for the hand-resolved merge of main).
  • Round 1 on 6859aed: no findings.
  • Earlier rounds, before the merge (Codex quest review --deep, gpt-6-astra, medium; session 01a1974d-e979-7c41-ab9f-700068d33556): round 1 on b89cc07 found the unquoted branch in the printed push command, fixed in 99a1c91; round 2 on 99a1c91 found nothing.

Merge of main (6859aed): main gained moq-dev/quest#356, #360, #361 and #362. The one conflict was the land fixer epic's audit table: kept main's "fixed by #356" for the registry row and this PR's ready-commit row. workflows-permission.md (from #362) linked the deleted fork-commit quest; it now names this PR's quest land apply handback instead.

Local checks: just check and just test pass on 6859aed with CLAUDE_CONFIG_DIR unset. Inside a Claude Code session, that variable leaks into two test/agent.test.ts tests this PR does not touch.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 3 commits November 1, 2026 05:19
When GitHub refuses the fixer's push of a validated fix, such as a merged
base's workflow changes the App lacks the `workflows` permission for, the
handback comment carries the resolution: the PR head, the base, the
resolved tree, and the fixer's edits as a gzipped patch against the
merge's conflicted result, sized to GitHub's comment limit.

`quest land apply <pr>` refuses a moved head, recomputes that conflicted
result with pinned merge settings, applies the patch, checks the tree,
and prints an unpushed commit with its push command.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A contributor names the PR branch, so quote every argument of the push
command quest land apply prints.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title A refused fork push hands back a ready commit feat(land): hand back a refused fix push as a ready commit Nov 1, 2026
@kixelated
kixelated marked this pull request as ready for review November 1, 2026 13:34
@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 28b9f353-9294-4005-bea0-f3b0e51def4a
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated enabled auto-merge (squash) November 1, 2026 18:12
@moq-bot
moq-bot Bot disabled auto-merge November 1, 2026 18:25
@moq-bot moq-bot Bot removed the quest:merge label Nov 1, 2026
@moq-bot

moq-bot Bot commented Nov 1, 2026

Copy link
Copy Markdown

quest land removed this pull request from the queue: the batch check failed (exit 1).

     �[33m�[2m✓�[22m�[39m refuses a batch whose requeued changes during a check�[33m 526�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m a refused second merge ejects the remaining batch as merge_conflict and starts the fixer�[33m 1947�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m a refused second merge ejects the remaining batch as head_moved and starts the fixer�[33m 1899�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m retries a transient base modified refusal and then merges�[33m 946�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m retries a transient mergeability pending refusal and then merges�[33m 919�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m classifies a refused merge with false�[33m 853�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m classifies a refused merge with dirty�[33m 865�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m classifies a refused merge with squash disabled�[33m 755�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m classifies a refused merge with required check�[33m 1006�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m classifies a refused merge with closed�[33m 796�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m classifies a refused merge with merged�[33m 1165�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m classifies a refused merge with pending�[33m 1257�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m classifies a refused merge with base modified�[33m 1425�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m a base push before merge 1 stops the batch without ejecting or reporting tree mismatch�[33m 2001�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m a base push before merge 2 stops the batch without ejecting or reporting tree mismatch�[33m 2555�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m a tree mismatch logs CRITICAL, keeps landed PRs, and ejects only the unmerged batch suffix�[33m 2526�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m scopes each merge token to that PR's squash, preserving it through the queue token cache�[33m 2926�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m a lost merge response never re-merges or ejects the merged PR after restart�[33m 2997�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m a restart resolves only PRs left after successful API merges�[33m 4013�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m resumes all pending ejections after a restart during a failing batch�[33m 625�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m requests workflow write only when the selected batch touches workflows (false)�[33m 745�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m requests workflow write only when the selected batch touches workflows (true)�[33m 1075�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m fails a batch whose check outlives the timeout�[33m 958�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m bounds a base conflict's comment however many paths conflict�[33m 1166�[2mms�[22m�[39m
   �[32m✓�[39m asynchronous land fixer �[2m(46)�[22m
     �[33m�[2m✓�[22m�[39m requeues unchanged with its reason, and hands back a second ejection in the same pick�[33m 2997�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m collects a fixed conflict after a restart, pushes and pins the new Review head�[33m 2915�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m accepts a fix whose base gained protected changes before collection�[33m 4202�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m waits for GitHub to show the pushed fix before recording it�[33m 3102�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back handback without requeueing�[33m 2122�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back protected file without requeueing�[33m 2528�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back shell.nix without requeueing�[33m 2556�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back default.nix without requeueing�[33m 4288�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back .gitmodules without requeueing�[33m 4786�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back fork without requeueing�[33m 2127�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back crash without requeueing�[33m 1812�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back invalid result without requeueing�[33m 1824�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back symlink result without requeueing�[33m 1848�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m times out a running fixer without waiting for it�[33m 1874�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m survives an agent whose output outruns the launcher's reader�[33m 3081�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m requeues a PR that only shared the batch, and hands back the one that broke it�[33m 3324�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m names the signal that stopped a fixer and the daemon restart behind it�[33m 1107�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m waits for a fixer whose process it could not identify, and says when one stops unrecorded�[33m 999�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m bounds a fixer's import by what it added, not the history fetched at launch or its submodules�[33m 417�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m passes no host credentials to wrapper or agent, including inherited Git configuration�[33m 1444�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m starts the agent detached at the base, with the batch and PR branches�[33m 2072�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back a job after losing the enable-auto-merge response�[33m 2459�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m preserves a refused-push handback across a failed comment and restart�[33m 4401�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back deleted branch and keeps collection usable�[33m 2645�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back gitfile and keeps collection usable�[33m 2528�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back symlink and keeps collection usable�[33m 1926�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back corrupt objects and keeps collection usable�[33m 2109�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m hands back oversized and keeps collection usable�[33m 2084�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m does not import private objects through alternates�[33m 2064�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m ignores an alternate store when the fixed objects are self-contained�[33m 1234�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m collects an already finished job after an extended daemon outage�[33m 1141�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m only deduplicates the bot's own comments�[33m 1059�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m rereads the author's body immediately before recording the fix�[33m 3361�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m records the actual agent and explicit model in the result�[33m 345�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m rejects mixing individually approved protected files�[33m 1201�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m rejects new gitlinks anywhere in the tree�[33m 1710�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m rebuilds pack indexes and strictly validates packs (corrupt pack: false)�[33m 5476�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m rebuilds pack indexes and strictly validates packs (corrupt pack: true)�[33m 3472�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m uses the recorded agent and leaves the author's attribution alone�[33m 6063�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m compares protected paths at the new head against the pin and current base�[33m 1106�[2mms�[22m�[39m
   �[32m✓�[39m private repository reads �[2m(3)�[22m
     �[33m�[2m✓�[22m�[39m authenticates private Git reads without exposing credentials to child repositories (failed batch: false)�[33m 2068�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m authenticates private Git reads without exposing credentials to child repositories (failed batch: true)�[33m 1589�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m refreshes after expiry and retries a revoked read token once�[33m 909�[2mms�[22m�[39m
   �[32m✓�[39m quest land apply �[2m(6)�[22m
     �[33m�[2m✓�[22m�[39m rebuilds the fixer's commit from a refused workflow push, for the maintainer to push�[33m 1768�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m refuses a PR whose head moved since the handback�[33m 2990�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m trusts only an App's resolution�[33m 1356�[2mms�[22m�[39m
     �[33m�[2m✓�[22m�[39m refuses a resolution whose tree the rebuild does not reach�[33m 1141�[2mms�[22m�[39m

�[31m⎯⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Tests 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m

�[41m�[1m FAIL �[22m�[49m test/runner.test.ts�[2m > �[22mquest runner�[2m > �[22mpauses a large conflicted continuation with a bounded question and intact checkout
�[31m�[1mError�[22m: Test timed out in 5000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".�[39m
�[36m �[2m❯�[22m <anonymous> test/runner.test.ts:�[2m1523:7�[22m�[39m
    �[90m1521|�[39m     { remove: "old.txt" },
    �[90m1522|�[39m     {
    �[90m1523|�[39m      tool: "deliver",
    �[90m   |�[39m       �[31m^�[39m
    �[90m1524|�[39m      args: { summary: "Did the thing", title: "feat: thing", body: "Bo…
    �[90m1525|�[39m     },
�[90m �[2m❯�[22m processTicksAndRejections unknown:�[2m7:39�[22m�[39m

�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m


�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[2m | �[22m�[1m�[32m23 passed�[39m�[22m�[90m (24)�[39m
�[2m      Tests �[22m �[1m�[31m1 failed�[39m�[22m�[2m | �[22m�[1m�[32m741 passed�[39m�[22m�[2m | �[22m�[33m2 skipped�[39m�[90m (744)�[39m
�[2m   Start at �[22m 10:20:55
�[2m   Duration �[22m 254.42s�[2m (tests 87%, transform 9%, import 4%)�[22m

error: script "test" exited with code 1
error: recipe `_test` failed on line 33 with exit code 1
error: recipe `test` failed on line 30 with exit code 1

.

@moq-bot

moq-bot Bot commented Nov 1, 2026

Copy link
Copy Markdown

codex / Codex land fixer: PR was picked again while fixing; keeping the newer pick. Recommendation: run /quest-iterate before picking it again.

 succeeded in 0ms:
$ vitest run

 RUN  v5.0.5 /work


codex
The unchanged PR builds successfully and passes TypeScript, formatting, and document checks. The full test suite is running. Its runner test file matches the failed batch’s, so this run will also check the reported timeout.

exec
/bin/bash -lc 'tail -8 /tmp/pr-358-tests.log' in /work
 succeeded in 0ms:
$ vitest run

 RUN  v5.0.5 /work


exec
/bin/bash -lc "ps -eo pid,etime,comm | rg 'vitest|bun|node' | head -20" in /work
 succeeded in 0ms:
    417       01:14 bun
    418       01:14 node
    525       01:13 bun

exec
/bin/bash -lc 'tail -10 /tmp/pr-358-tests.log' in /work
 succeeded in 0ms:
$ vitest run

 RUN  v5.0.5 /work


codex
The test process is still running and has not reported a result yet. I’ve made no source changes; if the suite passes, I’ll requeue PR 358 unchanged and name the timeout test in the reason.

exec
/bin/bash -lc 'tail -10 /tmp/pr-358-tests.log' in /work
 succeeded in 0ms:
$ vitest run

 RUN  v5.0.5 /work


 succeeded in 221419ms:
$ bun build src/cli/bin.ts --target=bun --banner "#!/usr/bin/env bun" --outfile dist/quest.js && bun scripts/executable.mjs dist/quest.js
Bundled 226 modules in 80ms

  quest.js  0.93 MB  (entry point)

$ tsc && tsc -p src/core && prettier --check .
Checking formatting...
All matched files use Prettier code style!

exec
/bin/bash -lc 'tail -10 /tmp/pr-358-tests.log' in /work
 succeeded in 0ms:
$ vitest run

 RUN  v5.0.5 /work


 Test Files  24 passed (24)
      Tests  740 passed | 2 skipped (742)
   Start at  10:40:20
   Duration  212.73s (tests 85%, transform 10%, import 5%)


codex
All 24 test files passed: 740 tests passed and two were skipped. The reported timeout did not recur. I’ll record PR 358 as an unchanged requeue, with the flaky test named for follow-up.

exec
/bin/bash -lc "python3 - <<'PY'
import json
result = [{\"number\":358,\"action\":\"requeue\",\"reason\":\"Healthy unchanged; batch timeout appears flaky in test/runner.test.ts > quest runner > pauses a large conflicted continuation with a bounded question and intact checkout. Passed bun install --frozen-lockfile, bun run build, bun run check, bun run --silent quest check, bun run --silent quest --root examples/export check, and bun run test (24 files passed; 740 passed, 2 skipped).\"}]
with open('quest-land-result.json', 'w') as f:
    json.dump(result, f, indent=2)
    f.write('\\n')
PY
 git status --short && git rev-parse pr-358 && cat quest-land-result.json" in /work
 succeeded in 0ms:
?? quest-land-result.json
6859aedb39d4318ffc289a3d611abc26f35a35bf
[
  {
    "number": 358,
    "action": "requeue",
    "reason": "Healthy unchanged; batch timeout appears flaky in test/runner.test.ts > quest runner > pauses a large conflicted continuation with a bounded question and intact checkout. Passed bun install --frozen-lockfile, bun run build, bun run check, bun run --silent quest check, bun run --silent quest --root examples/export check, and bun run test (24 files passed; 740 passed, 2 skipped)."
  }
]

codex
PR 358 is healthy and marked for unchanged requeue. Build and checks passed; all 24 test files passed. The batch’s runner-test timeout appears flaky and is named in the reason.

Wrote [quest-land-result.json](/work/quest-land-result.json), left uncommitted. No branch changes.
tokens used
39,662
PR 358 is healthy and marked for unchanged requeue. Build and checks passed; all 24 test files passed. The batch’s runner-test timeout appears flaky and is named in the reason.

Wrote [quest-land-result.json](/work/quest-land-result.json), left uncommitted. No branch changes.
fixer result written

(written by codex / Codex)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant