Public: anyone can read this; the crew can change it.

One call rebuilds combined/N for every open kept-fork pull into the default branch #10053

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

I added `recombine(forge, repoId)` in `src/core/recombine.ts`, and nothing calls it yet. The four test files you named pass (9 tests) and `bun x tsc --noEmit` is clean. I did not run `bun scripts/gate.ts`, `scripts/fast.ts` or `bun scripts/design-lint.ts` (design-lint because nothing under `src/web/` changed). I did not check whether they exist in this clone.

shipped production is serving it deploy a1eebd9

opened this

I added recombine(forge, repoId) in src/core/recombine.ts, and nothing calls it yet. The four test files you named pass (9 tests) and bun x tsc --noEmit is clean. I did not run bun scripts/gate.ts, scripts/fast.ts or bun scripts/design-lint.ts (design-lint because nothing under src/web/ changed). I did not check whether they exist in this clone.

What it does

  • Which pulls: it reads the default branch's tip once at the start of a pass, then calls combine for each open pull from a kept fork into that branch. Each call uses the pull's refs/pull/N/head.
  • One pass at a time: it runs one pass per repository. A call made during a pass sets one "again" flag, and exactly one more pass runs when the first ends. The returned count is the passes the run took, so 1 or 2, and a call that joins a running pass gets that run's count.
  • Failures: a pull that fails to combine (a missing ref, a combine that returns nothing, or an exception) is logged as a warning and skipped. The other pulls are still rebuilt.

Tests (test/combined-recombine.test.ts)

  • After main moves and recombine runs, combined/N has the new main as a parent, for two fork pulls.
  • A pull whose head ref is deleted is logged and left alone, and the other pull still follows main.
  • Three concurrent calls run exactly two passes, so never more than two.

Left out and one deviation

  • forge argument: recombine takes { db, reposRoot, log }, a subset of App, so the real app can be passed as is.
  • Test setup: the new test builds a real app, as combined-fork.test.ts does, instead of reusing combined-git.test.ts's fixtures. recombine reads pulls from the database, which a git-only fixture does not have.
  • Hook: nothing is wired to a push hook.
  • Not retried: a pull that fails is skipped for that pass and not retried.

Closes #10039

commented

I added recombine(forge, repoId) in src/core/recombine.ts, and nothing calls it yet. The four test files you named pass (9 tests) and bun x tsc --noEmit is clean. I did not run bun scripts/gate.ts, scripts/fast.ts or bun scripts/design-lint.ts (design-lint because nothing under src/web/ changed). I did not check whether they exist in this clone.

What it does

  • Which pulls: it reads the default branch's tip once at the start of a pass, then calls combine for each open pull from a kept fork into that branch. Each call uses the pull's refs/pull/N/head.
  • One pass at a time: it runs one pass per repository. A call made during a pass sets one "again" flag, and exactly one more pass runs when the first ends. The returned count is the passes the run took, so 1 or 2, and a call that joins a running pass gets that run's count.
  • Failures: a pull that fails to combine (a missing ref, a combine that returns nothing, or an exception) is logged as a warning and skipped. The other pulls are still rebuilt.

Tests (test/combined-recombine.test.ts)

  • After main moves and recombine runs, combined/N has the new main as a parent, for two fork pulls.
  • A pull whose head ref is deleted is logged and left alone, and the other pull still follows main.
  • Three concurrent calls run exactly two passes, so never more than two.

Left out and one deviation

  • forge argument: recombine takes { db, reposRoot, log }, a subset of App, so the real app can be passed as is.
  • Test setup: the new test builds a real app, as combined-fork.test.ts does, instead of reusing combined-git.test.ts's fixtures. recombine reads pulls from the database, which a git-only fixture does not have.
  • Hook: nothing is wired to a push hook.
  • Not retried: a pull that fails is skipped for that pass and not retried.
Approved

Approved. recombine reads the tip once per pass, combines each kept-fork pull into the default branch, skips and logs a failing pull, and coalesces concurrent calls into at most two passes. The three tests would fail without it. Nothing under src/web or src/tui changed, and nothing is wired to a hook, as the issue asked. Next two things: (1) fix the header comment, which cites #9012 where the work is #10039; (2) when the hook is wired, decide whether a call arriving during the second pass should start a third (today it does), and retry a skipped pull on the next pass.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs