A run never moves its branch off the head the forge's branch holds, and a run whose push fails keeps its head on the forge #170

Merged
agra merged 6 commits from task/3852-a-run-that-rewrites-a-commit-the-runner into master 2026-10-27 07:31:35 +02:00
Owner

Tracker: #3852

A run that amended, rebased or reset a commit the runner had already pushed ended failed push_failed at its end, and its finished work stayed only in the run's work tree (Implementer run 1113 of SX #3848). This PR:

  • Refuses the rewrite when it happens. The runner writes a reference-transaction hook into the hooks directory of the run's home, beside commit-msg. It refuses to move the run's branch to a commit that does not hold the head named in hooks/pushed: the remote's head of the branch as the runner knows it, from the checkout's start when the remote has the branch, then each head the runner pushes (named before the push, named back when the push fails). git commit --amend, git rebase or git reset off a pushed commit fails at once, and the hook's message tells the agent to add a commit instead. The harness's git takes gc.packRefs=false: packing refs prunes each loose ref in a transaction of its own, which the hook reads as a move of the branch to the zero id, so git gc would otherwise exit 128. An explicit git pack-refs is still refused.
  • Says so in the instructions. Every run on a branch gets a ## Pushed commits part in sak's instructions: the runner pushes the branch when it opens the PR and when the run ends, a commit it pushed is never rewritten, and the run's git refuses to move the branch off the forge's head.
  • Keeps the head of a run whose last push fails. When the push of the branch fails at the run's end, the runner pushes the head to refs/sak/run-<id> on the forge and reports it as kept_head on POST /api/runs/:id/complete. sak stores it (migration 50 adds kept_head, resume_ref and resume_head to run_request), shows it on the run and on its ended history event. The runner keeps the head whatever reason the run ends with (a run killed idle_timeout whose end push fails keeps it too), and a run queued again after a run that kept a head carries resume_ref and resume_head, whatever that run's reason; a request queued again for its account's limit resumes the head it kept from its own ref. A run that pushed its branch and kept no head leaves no head to resume, so the run after it starts from the forge's branch; one that pushed nothing and kept nothing leaves the head it resumed. The resumed run's runner fetches the kept head and starts the branch from it (a kept head the remote lacks ends the run resume_missing), and its prompt's ## The run before says so. When the forge's branch moved past the kept head, the hook makes the resumed run merge it before it commits, so its push is a fast-forward.

docs/api.md and runner/README.md describe the hook, the ## Pushed commits part, kept_head, resume_ref/resume_head and resume_missing. The web's RunRequest type carries the three new fields, with what they hold; nothing renders them, so no golden moves.

Regression

tests/runner_core.sx check_kept_head walks the repro on a flow run against the stub forge. Its first step runs the stub harness's new diverge mode: it commits, says something until the runner pushes the commit as it opens the PR, then runs git gc and git commit --amend. It then moves the forge's branch past the pushed head, as a push from elsewhere would, commits again and concludes. The checks:

  • the claim's instructions carry ## Pushed commits;
  • its git runs git gc with the branch guarded;
  • the amend after it is refused, with the hook's message;
  • the run ends failed push_failed, its kept_head is its head, and the forge holds it as refs/sak/run-<id>;
  • the run queued again carries resume_ref/resume_head, and its prompt names them;
  • that run starts at the kept head; its git refuses a commit until the branch holds the forge's head; once it merges that head it ends done, and the forge's branch holds its work and the kept head;
  • a run that ends failed idle_timeout with a kept head is queued again on that head, and its prompt names it;
  • a run that ends account_limited with a kept head is queued again to resume that head from refs/sak/run-<its id>;
  • that request's next run, limited before it pushed or kept a head, leaves it that head to resume; its next, limited once it pushed its branch and kept no head, leaves it none;
  • a run resuming a kept head that ends failed idle_timeout is queued again on that head when it pushed nothing and kept nothing, and on the forge's branch when it pushed its branch and kept no head.

Fail-before, from git archive arms with one part of the fix taken out, each running tests/runner_core.sx on m5 with the gates' sx 49e83601. First, arms of ab88fd1e:

  • ab88fd1e as is: RUNNER CORE OK, 488 ok.
  • No reference-transaction hook (write_hooks writes only commit-msg): RUNNER CORE FAIL (5 checks). Failing: "its git refuses to amend the commit the runner pushed as it opened the PR", "its branch starts at the kept head, and its git refuses a commit until the branch holds the forge's head", and the three exact listings of a run's home, which now hold hooks/reference-transaction.
  • No kept head (finish does not call keep after push_failed): RUNNER CORE FAIL (4 checks). Failing: "a run whose push the forge refuses ends failed push_failed, its head kept on the forge under its own ref", "the run queued again resumes the kept head, and its prompt says so", "its branch starts at the kept head, …", "once it merges the forge's branch its work is pushed, the kept head with it".

Then, for the review's two findings, arms of 9aecb4e5:

  • 9aecb4e5 as is: RUNNER CORE OK, 491 ok.
  • ab88fd1e's code with 9aecb4e5's tests: RUNNER CORE FAIL (6 checks). Failing: "its git collects garbage with the branch guarded", "a run ended failed idle_timeout with a kept head is queued again on that head, and its prompt says so", "a run whose account reached its limit with a kept head is queued again to resume that head from its own ref", and the three checks of a run's exact environment, one per harness, which now holds gc.packRefs=false.
  • The kept head named only after push_failed (run_before as at ab88fd1e): RUNNER CORE FAIL (1 checks), the idle_timeout check.
  • No resume on an account-limited requeue: RUNNER CORE FAIL (1 checks), the account_limited check.
  • No gc.packRefs=false (git_env as at ab88fd1e): RUNNER CORE FAIL (4 checks), the gc check and the three environment checks.

Then, for the second review's finding (a run's resume outliving a run that pushed its work), arms of 4aaa319e's tests, run side by side on m5:

  • 4aaa319e as is: RUNNER CORE OK, 495 ok.
  • 9aecb4e5's code: RUNNER CORE FAIL (2 checks). Failing: "its next run, limited once it pushed its branch and kept no head, leaves it to resume none" and "one ended failed idle_timeout once it pushed its branch and kept no head is queued again on the forge's branch".
  • Only the account-limited requeue fixed (queue_again copies the resume as at 9aecb4e5): RUNNER CORE FAIL (1 checks), the idle_timeout one.
  • Only queue_again fixed (the limited UPDATE as at 9aecb4e5): RUNNER CORE FAIL (1 checks), the account-limited one.
  • Every run that keeps no head clears the resume: RUNNER CORE FAIL (2 checks), the two whose run pushed nothing and kept nothing.

The ## Pushed commits check fails on master, which composes no such part.

Migration 50's undo is added to the three tests that take a migrated database back by hand (tests/tools.sx to 40, tests/utf8_text.sx to 44, tests/flows.sx to 48), and the schema-head asserts move to 50.

Gates

At the gates' sx pin 49e83601:

  • Run 1187's full #1, gate run 608, at 4aaa319e (this head, clean tree), on m5: passed. Every tests/*.sx file, scripts/release.sh and step9_release, the deploy contract, the main Playwright config (771 passed, 1 skipped) and the runner config (10 passed).
  • Run 1177's full #1, gate run 602, at 9aecb4e5, on m5: passed.
  • Run 1163's full #3, gate run 597, at ab88fd1e, on m5: passed.
  • Run 1163's fast #1, gate run 593, and full #2, gate run 594, at 0ffdfffe: passed. ab88fd1e changes only tests/runner_core.sx and the stream-json stub harness script, written while #2's tests loop ran.
Tracker: [#3852](https://sak.swipelab.com/tasks/3852) A run that amended, rebased or reset a commit the runner had already pushed ended `failed` `push_failed` at its end, and its finished work stayed only in the run's work tree (Implementer run 1113 of SX #3848). This PR: - **Refuses the rewrite when it happens.** The runner writes a `reference-transaction` hook into the hooks directory of the run's home, beside `commit-msg`. It refuses to move the run's branch to a commit that does not hold the head named in `hooks/pushed`: the remote's head of the branch as the runner knows it, from the checkout's start when the remote has the branch, then each head the runner pushes (named before the push, named back when the push fails). `git commit --amend`, `git rebase` or `git reset` off a pushed commit fails at once, and the hook's message tells the agent to add a commit instead. The harness's git takes `gc.packRefs=false`: packing refs prunes each loose ref in a transaction of its own, which the hook reads as a move of the branch to the zero id, so `git gc` would otherwise exit 128. An explicit `git pack-refs` is still refused. - **Says so in the instructions.** Every run on a branch gets a `## Pushed commits` part in sak's instructions: the runner pushes the branch when it opens the PR and when the run ends, a commit it pushed is never rewritten, and the run's git refuses to move the branch off the forge's head. - **Keeps the head of a run whose last push fails.** When the push of the branch fails at the run's end, the runner pushes the head to `refs/sak/run-<id>` on the forge and reports it as `kept_head` on `POST /api/runs/:id/complete`. sak stores it (migration 50 adds `kept_head`, `resume_ref` and `resume_head` to `run_request`), shows it on the run and on its `ended` history event. The runner keeps the head whatever reason the run ends with (a run killed `idle_timeout` whose end push fails keeps it too), and a run queued again after a run that kept a head carries `resume_ref` and `resume_head`, whatever that run's reason; a request queued again for its account's limit resumes the head it kept from its own ref. A run that pushed its branch and kept no head leaves no head to resume, so the run after it starts from the forge's branch; one that pushed nothing and kept nothing leaves the head it resumed. The resumed run's runner fetches the kept head and starts the branch from it (a kept head the remote lacks ends the run `resume_missing`), and its prompt's `## The run before` says so. When the forge's branch moved past the kept head, the hook makes the resumed run merge it before it commits, so its push is a fast-forward. docs/api.md and runner/README.md describe the hook, the `## Pushed commits` part, `kept_head`, `resume_ref`/`resume_head` and `resume_missing`. The web's `RunRequest` type carries the three new fields, with what they hold; nothing renders them, so no golden moves. ## Regression `tests/runner_core.sx` `check_kept_head` walks the repro on a flow run against the stub forge. Its first step runs the stub harness's new `diverge` mode: it commits, says something until the runner pushes the commit as it opens the PR, then runs `git gc` and `git commit --amend`. It then moves the forge's branch past the pushed head, as a push from elsewhere would, commits again and concludes. The checks: - the claim's instructions carry `## Pushed commits`; - its git runs `git gc` with the branch guarded; - the amend after it is refused, with the hook's message; - the run ends `failed` `push_failed`, its `kept_head` is its head, and the forge holds it as `refs/sak/run-<id>`; - the run queued again carries `resume_ref`/`resume_head`, and its prompt names them; - that run starts at the kept head; its git refuses a commit until the branch holds the forge's head; once it merges that head it ends `done`, and the forge's branch holds its work and the kept head; - a run that ends `failed` `idle_timeout` with a kept head is queued again on that head, and its prompt names it; - a run that ends `account_limited` with a kept head is queued again to resume that head from `refs/sak/run-<its id>`; - that request's next run, limited before it pushed or kept a head, leaves it that head to resume; its next, limited once it pushed its branch and kept no head, leaves it none; - a run resuming a kept head that ends `failed` `idle_timeout` is queued again on that head when it pushed nothing and kept nothing, and on the forge's branch when it pushed its branch and kept no head. Fail-before, from `git archive` arms with one part of the fix taken out, each running `tests/runner_core.sx` on m5 with the gates' sx 49e83601. First, arms of ab88fd1e: - **ab88fd1e as is:** `RUNNER CORE OK`, 488 ok. - **No reference-transaction hook** (`write_hooks` writes only `commit-msg`): `RUNNER CORE FAIL (5 checks)`. Failing: "its git refuses to amend the commit the runner pushed as it opened the PR", "its branch starts at the kept head, and its git refuses a commit until the branch holds the forge's head", and the three exact listings of a run's home, which now hold `hooks/reference-transaction`. - **No kept head** (`finish` does not call `keep` after `push_failed`): `RUNNER CORE FAIL (4 checks)`. Failing: "a run whose push the forge refuses ends failed push_failed, its head kept on the forge under its own ref", "the run queued again resumes the kept head, and its prompt says so", "its branch starts at the kept head, …", "once it merges the forge's branch its work is pushed, the kept head with it". Then, for the review's two findings, arms of 9aecb4e5: - **9aecb4e5 as is:** `RUNNER CORE OK`, 491 ok. - **ab88fd1e's code with 9aecb4e5's tests:** `RUNNER CORE FAIL (6 checks)`. Failing: "its git collects garbage with the branch guarded", "a run ended failed idle_timeout with a kept head is queued again on that head, and its prompt says so", "a run whose account reached its limit with a kept head is queued again to resume that head from its own ref", and the three checks of a run's exact environment, one per harness, which now holds `gc.packRefs=false`. - **The kept head named only after `push_failed`** (`run_before` as at ab88fd1e): `RUNNER CORE FAIL (1 checks)`, the idle_timeout check. - **No resume on an account-limited requeue:** `RUNNER CORE FAIL (1 checks)`, the account_limited check. - **No `gc.packRefs=false`** (`git_env` as at ab88fd1e): `RUNNER CORE FAIL (4 checks)`, the gc check and the three environment checks. Then, for the second review's finding (a run's resume outliving a run that pushed its work), arms of 4aaa319e's tests, run side by side on m5: - **4aaa319e as is:** `RUNNER CORE OK`, 495 ok. - **9aecb4e5's code:** `RUNNER CORE FAIL (2 checks)`. Failing: "its next run, limited once it pushed its branch and kept no head, leaves it to resume none" and "one ended failed idle_timeout once it pushed its branch and kept no head is queued again on the forge's branch". - **Only the account-limited requeue fixed** (`queue_again` copies the resume as at 9aecb4e5): `RUNNER CORE FAIL (1 checks)`, the idle_timeout one. - **Only `queue_again` fixed** (the limited UPDATE as at 9aecb4e5): `RUNNER CORE FAIL (1 checks)`, the account-limited one. - **Every run that keeps no head clears the resume:** `RUNNER CORE FAIL (2 checks)`, the two whose run pushed nothing and kept nothing. The `## Pushed commits` check fails on master, which composes no such part. Migration 50's undo is added to the three tests that take a migrated database back by hand (`tests/tools.sx` to 40, `tests/utf8_text.sx` to 44, `tests/flows.sx` to 48), and the schema-head asserts move to 50. ## Gates At the gates' sx pin 49e83601: - Run 1187's `full` #1, gate run 608, at 4aaa319e (this head, clean tree), on m5: passed. Every `tests/*.sx` file, `scripts/release.sh` and `step9_release`, the deploy contract, the main Playwright config (771 passed, 1 skipped) and the runner config (10 passed). - Run 1177's `full` #1, gate run 602, at 9aecb4e5, on m5: passed. - Run 1163's `full` #3, gate run 597, at ab88fd1e, on m5: passed. - Run 1163's `fast` #1, gate run 593, and `full` #2, gate run 594, at 0ffdfffe: passed. ab88fd1e changes only `tests/runner_core.sx` and the stream-json stub harness script, written while #2's tests loop ran.
agra changed title from A run never rewrites a commit the runner pushed, and a run whose push fails keeps its head on the forge to A run never moves its branch off the head the forge's branch holds, and a run whose push fails keeps its head on the forge 2026-10-27 04:33:20 +02:00
The runner keeps the head of any run whose end push fails, whatever
reason the run ends with, and sak already queued the run again on that
head for every retried reason. The run before's kept head is now named
in `## The run before` for every reason, not only push_failed, and a
request queued again for its account's limit resumes the head it kept
from its own ref.
Packing refs prunes each loose ref in a transaction of its own, which
the reference-transaction hook reads as a move of the run's branch to
the zero id and refuses, so `git gc` in the run's directory exited 128.
The harness's git now takes gc.packRefs=false; an amend off the pushed
head is still refused.
agra merged commit 70afcae087 into master 2026-10-27 07:31:35 +02:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
sx/sak!170
No description provided.