Time updates a PR branch that fell behind its base; a conflict goes to the worker as a revision #190

Merged
time merged 4 commits from time/card-123 into main 2026-10-19 15:11:37 +00:00
Collaborator

In land(), an approved PR with mergeable: false (or a 409 from the merge call) now gets git merge origin/<base> pushed by Time as one commit, card #N: merge <base>. The base is the PR's own base.ref. Time posts an updated beat. CI runs on the new head and landing follows on a later beat.

A conflict is not guessed at. The merge is aborted and the branch is left as it was. The conflicting files and trimmed <<<<<<< blocks are posted on the card and on the PR (reviewer_feedback reads the PR's comments). The PR gets the needs-human label and the card gets a needs-human beat marked conflict, so the conflict is reported once. /time retry sends the worker back to the branch with those hunks as the reviewer's text.

Also changed: ci-dispatched and updated beats now reach land() in tick(). The tuple only listed pr, needs-human, no-checks and checks-failed, so a card whose last beat was ci-dispatched never got to land.

Checks: python3 .github/time/test_time.py passes, including the new check 123. It covers:

  • an unmergeable approved PR gets one merge commit and no merge call that tick;
  • a clean merge lands on the next tick;
  • a conflict posts the hunks and labels needs-human, and the next beat says nothing more;
  • /time retry dispatches the worker with the hunks in its prompt;
  • an already-mergeable PR is left alone;
  • a 409 on the merge call takes the same path;
  • update_branch runs against a real bare git repo (clean merge by Time, up to date, conflict aborted with the branch untouched).

I have not seen CI run on the PR.

Known limit: the worker's write_file of a resolved file is a plain commit, not the merge. A retry may therefore conflict again on Time's next try. A human sees it each time, because it goes back to needs-human. A fix would let the worker's hands commit the merge. I noted this in the code and the CHANGELOG.

Not handled: fork PRs, because Time clones the head branch from this repo.

Closes #123

worker: Claude Sonnet (Anthropic) via Time

In `land()`, an approved PR with `mergeable: false` (or a 409 from the merge call) now gets `git merge origin/<base>` pushed by Time as one commit, `card #N: merge <base>`. The base is the PR's own `base.ref`. Time posts an `updated` beat. CI runs on the new head and landing follows on a later beat. A conflict is not guessed at. The merge is aborted and the branch is left as it was. The conflicting files and trimmed `<<<<<<<` blocks are posted on the card and on the PR (`reviewer_feedback` reads the PR's comments). The PR gets the `needs-human` label and the card gets a `needs-human` beat marked `conflict`, so the conflict is reported once. `/time retry` sends the worker back to the branch with those hunks as the reviewer's text. Also changed: `ci-dispatched` and `updated` beats now reach `land()` in `tick()`. The tuple only listed `pr`, `needs-human`, `no-checks` and `checks-failed`, so a card whose last beat was `ci-dispatched` never got to land. Checks: `python3 .github/time/test_time.py` passes, including the new check 123. It covers: - an unmergeable approved PR gets one merge commit and no merge call that tick; - a clean merge lands on the next tick; - a conflict posts the hunks and labels `needs-human`, and the next beat says nothing more; - `/time retry` dispatches the worker with the hunks in its prompt; - an already-mergeable PR is left alone; - a 409 on the merge call takes the same path; - `update_branch` runs against a real bare git repo (clean merge by Time, up to date, conflict aborted with the branch untouched). I have not seen CI run on the PR. Known limit: the worker's `write_file` of a resolved file is a plain commit, not the merge. A retry may therefore conflict again on Time's next try. A human sees it each time, because it goes back to `needs-human`. A fix would let the worker's hands commit the merge. I noted this in the code and the CHANGELOG. Not handled: fork PRs, because Time clones the head branch from this repo. Closes #123 <sub>worker: Claude Sonnet (Anthropic) via Time</sub>
time added 4 commits 2026-10-19 14:13:54 +00:00
card #123: kept .github/time/time.py (generated)
time/worker-run cat > /tmp/blk.py <<'EOF' # 123 an approved PR that no longer merges is brought up to date by Time;
c2ebb2375f
card #123: kept .github/time/test_time.py (generated)
time/worker-run sed -i 's/len(ub_calls) == 1 and len(w.posted) == 1, "the next beat does not merge or post again"/le
36b2b0563a
card #123: kept .github/time/test_time.py (generated)
time/worker-run python3 - <<'EOF' p = "CHANGELOG.md" s = open(p).read() line = ("- Time brings a PR branch up to dat
90609cea81
card #123: kept CHANGELOG.md (generated)
ci / test (pull_request) Successful in 38s
heartbeat / tick (pull_request_review) Canceled after 0s
332484bd24
haiku approved these changes 2026-10-19 15:07:58 +00:00
haiku left a comment
Collaborator

Approve

The diff does what card #123 asks, the finish line holds (ci / test is success on the head, and the new check 123 covers the cases the card lists), and the conflict path is cautious: it aborts, leaves the branch alone, and reports once. Non-blocking notes:

  • .github/time/time.py ~L1140-1178 (update_branch): a non-conflict merge failure is re-run with check=True, so it raises a RuntimeError. I could not confirm from the diff that tick() isolates per-card exceptions. If it does not, one git or network error stops the whole run. Worth a quick check.
  • .github/time/time.py ~L1163-1166: the hunk text is cut at 1200 and 3500 characters. The cut can leave a ``` fence unbalanced in the posted comment. Cosmetic.
  • .github/time/time.py ~L969-973 (the 409 branch): matching "409" in the exception text is loose. It works for the Gitea error format, but it is a string match.
  • .github/time/time.py ~L1154: when update_branch returns ("", "") (nothing to merge) but the PR still reports mergeable: false, update_branch_of returns silently every tick. This is the intended "silent" case, but it can hide a stuck PR. A log line would help.
  • .github/time/time.py ~L1682 (tick): adding ci-dispatched and updated to the land() tuple is beyond the card's text. The PR body explains it, and it is needed for the new updated beat to progress, so I accept it.
  • Known limit (already disclosed): a worker's write_file of a resolved file is a plain commit, so a retry can conflict again. That goes back to needs-human each time, so it is visible to a human.

review by haiku · claude-haiku-5-5 · via Time

**Approve** The diff does what card #123 asks, the finish line holds (`ci / test` is success on the head, and the new check 123 covers the cases the card lists), and the conflict path is cautious: it aborts, leaves the branch alone, and reports once. Non-blocking notes: - `.github/time/time.py` ~L1140-1178 (`update_branch`): a non-conflict merge failure is re-run with `check=True`, so it raises a `RuntimeError`. I could not confirm from the diff that `tick()` isolates per-card exceptions. If it does not, one git or network error stops the whole run. Worth a quick check. - `.github/time/time.py` ~L1163-1166: the hunk text is cut at 1200 and 3500 characters. The cut can leave a `` ``` `` fence unbalanced in the posted comment. Cosmetic. - `.github/time/time.py` ~L969-973 (the 409 branch): matching `"409"` in the exception text is loose. It works for the Gitea error format, but it is a string match. - `.github/time/time.py` ~L1154: when `update_branch` returns `("", "")` (nothing to merge) but the PR still reports `mergeable: false`, `update_branch_of` returns silently every tick. This is the intended "silent" case, but it can hide a stuck PR. A log line would help. - `.github/time/time.py` ~L1682 (`tick`): adding `ci-dispatched` and `updated` to the `land()` tuple is beyond the card's text. The PR body explains it, and it is needed for the new `updated` beat to progress, so I accept it. - Known limit (already disclosed): a worker's `write_file` of a resolved file is a plain commit, so a retry can conflict again. That goes back to `needs-human` each time, so it is visible to a human. <sub>review by haiku · claude-haiku-5-5 · via Time</sub>
time merged commit 7156f0023c into main 2026-10-19 15:11:37 +00:00
time deleted branch time/card-123 2026-10-19 15:11:37 +00:00
Sign in to join this conversation.