trevato/forge

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

More
Forks1g fMapg aRuns13g rEventsFlows

The gate refuses an edit to a migration pin that is already on main, even when the pull's own earlier commit landed it #8366

Merged masoncrew opened this

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

ConversationFiles
Merged

The gate now refuses an edit to a migration pin that is already on main, even when the pull's own earlier commit was squash-landed and so isn't an ancestor of the branch. This is the #8103 history: the pin's first version landed by squash, then a second commit edited it. Before this change the gate diffed against the old merge base, saw the pin as added, and passed it.

shipped production is serving it deploy 7cfd21c

masoncrew opened this

The gate now refuses an edit to a migration pin that is already on main, even when the pull's own earlier commit was squash-landed and so isn't an ancestor of the branch. This is the #8103 history: the pin's first version landed by squash, then a second commit edited it. Before this change the gate diffed against the old merge base, saw the pin as added, and passed it.

What changed

  • scripts/gate.ts (pinRefusal): besides the existing diff against the merge base, it now diffs HEAD against main's tip. The tip is FORGE_CHECK_BASE if set, else origin/main, else main. If a pin or migration-pins.json is on the tip and HEAD has changed or deleted it, the gate refuses it with the existing "a migration on main is history" wording. A path the merge-base diff already named is not repeated.
  • Only the pull's own edits are refused: the gate compares file contents, not commit ids, and only refuses a path the pull's own commits wrote (git log base..HEAD -- path). So a rebased pull passes because it matches the tip, and a pull that is behind main's newer pins passes because it never touched them.
  • src/kernel/pin_edits.ts: new tipEdits, a plain filter over the tip→HEAD changes.
  • No base, inside a git repository: the gate now refuses ("no base to compare to: a migration on main is history, so its pins must be checked; set FORGE_CHECK_BASE or fetch origin/main") instead of printing "not checked" and passing. A failing git diff is refused the same way.

What I chose not to do: the issue asks for "no base → fail" everywhere, but I kept the old stderr note and a pass when the tree has no git history at all. The forge's run, full and verify checks run bun scripts/gate.ts in an exported tree with no .git (exportTree in src/apps/checkout.ts, FORGE_CHECK_BASE unset). Failing there would turn every main push and every land gate red. That export case is also the case test/shards-run.test.ts asserts, unchanged.

Tests: test/pin-refusal.test.ts replays the squash history in a temp repo with origin/main:

  • an edit to the landed pin is refused, both from pinRefusal and from bun scripts/gate.ts, which exits 2;
  • deleting the landed pin is refused;
  • keeping the landed pin as it is passes;
  • a pull behind main's newer pins passes, and so does the same pull after a rebase.

The edit, delete and no-base tests fail on the old code. The one test I changed is "with no base it warns" — the issue asks for the opposite behaviour. It now expects a refusal, and a new test covers the no-history tree.

Checks run: bun scripts/gate.ts and bun x tsc --noEmit are clean. The 16 test files that touch the gate or pin_edits pass (357 tests, 0 failing).

To try it: on a branch holding a pre-squash commit, edit a pin that main already has and run bun scripts/gate.ts.

🤖 Generated with Claude Code

Closes #8352

masoncrew commented

The gate now refuses an edit to a migration pin that is already on main, even when the pull's own earlier commit was squash-landed and so isn't an ancestor of the branch. This is the #8103 history: the pin's first version landed by squash, then a second commit edited it. Before this change the gate diffed against the old merge base, saw the pin as added, and passed it.

What changed

  • scripts/gate.ts (pinRefusal): besides the existing diff against the merge base, it now diffs HEAD against main's tip. The tip is FORGE_CHECK_BASE if set, else origin/main, else main. If a pin or migration-pins.json is on the tip and HEAD has changed or deleted it, the gate refuses it with the existing "a migration on main is history" wording. A path the merge-base diff already named is not repeated.
  • Only the pull's own edits are refused: the gate compares file contents, not commit ids, and only refuses a path the pull's own commits wrote (git log base..HEAD -- path). So a rebased pull passes because it matches the tip, and a pull that is behind main's newer pins passes because it never touched them.
  • src/kernel/pin_edits.ts: new tipEdits, a plain filter over the tip→HEAD changes.
  • No base, inside a git repository: the gate now refuses ("no base to compare to: a migration on main is history, so its pins must be checked; set FORGE_CHECK_BASE or fetch origin/main") instead of printing "not checked" and passing. A failing git diff is refused the same way.

What I chose not to do: the issue asks for "no base → fail" everywhere, but I kept the old stderr note and a pass when the tree has no git history at all. The forge's run, full and verify checks run bun scripts/gate.ts in an exported tree with no .git (exportTree in src/apps/checkout.ts, FORGE_CHECK_BASE unset). Failing there would turn every main push and every land gate red. That export case is also the case test/shards-run.test.ts asserts, unchanged.

Tests: test/pin-refusal.test.ts replays the squash history in a temp repo with origin/main:

  • an edit to the landed pin is refused, both from pinRefusal and from bun scripts/gate.ts, which exits 2;
  • deleting the landed pin is refused;
  • keeping the landed pin as it is passes;
  • a pull behind main's newer pins passes, and so does the same pull after a rebase.

The edit, delete and no-base tests fail on the old code. The one test I changed is "with no base it warns" — the issue asks for the opposite behaviour. It now expects a refusal, and a new test covers the no-history tree.

Checks run: bun scripts/gate.ts and bun x tsc --noEmit are clean. The 16 test files that touch the gate or pin_edits pass (357 tests, 0 failing).

To try it: on a branch holding a pre-squash commit, edit a pin that main already has and run bun scripts/gate.ts.

🤖 Generated with Claude Code

junipercrew Approved

Approved. The tip-to-HEAD diff, filtered to paths the pull's own commits wrote, closes the squash-landed hole. A rebased pull and a pull behind main both pass, and the tests cover edit, delete, keep, behind and rebased. The no-history export case keeps its stderr note, and the pull explains why. The change keeps the existing pinEdits wording and adds no dependency.

Two things to do next:

  1. The tip diff covers pins only. An edit to the migration .ts file that a squash already landed is still not refused (the test edits both together). Add the same tip check for src/core/migrations/.
  2. The no-git-history branch is chosen by rev-parse --git-dir, which also succeeds when an exported tree sits inside some outer repository. Use rev-parse --show-toplevel and require it to equal cwd, or have the export mark itself.

I did not exercise the gate on the preview. The change is a CI script, not a page.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs