Skip to content

fix(repo): pick the pre-push base by history, never a local main - #1192

Merged
khoaguin merged 6 commits into
mainfrom
OME-1443-pre-push-base-fallback
Oct 1, 2026
Merged

khoaguin merged 6 commits into
mainfrom
OME-1443-pre-push-base-fallback

Conversation

@khoaguin

@khoaguin khoaguin commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

TLDR

The pre-push hook (.githooks/pre-push) runs before every git push. It works out which stacks the branch changed and runs each one's full checks: ruff, pyright, and the whole test suite with coverage. It finds "what changed" by comparing the branch against a base, the main branch it was cut from.

The rule: the base must be the remote's current main. Otherwise commits already on main get counted as this branch's changes.

Where it broke:

  • The hook looked for origin/main. A clone whose remote is named upstream has none, so the hook fell back to the local main branch.
  • Local main lags the remote. On 2026-10-01 it was 8 commits behind, and those commits touched the engine, aigateway and aigateway-ui. A docs-only push then ran all three stacks' checks: the engine's ~4,650-test suite with coverage, aigateway's suite, and an npm build.

The change: the hook picks the base by history, not by remote name. Among every <remote>/main, it uses the one the branch has the fewest commits on top of, which is the main it was cut from. Example: a docs-only branch is 1 commit ahead of sc-remote/main but 41 ahead of a fork's stale origin/main, so sc-remote/main wins and no stack is checked. The checks then get the commit the branch was cut from, not that remote main's latest commit: the append-only test check compares its base with your working copy, so a test main changed after you branched would otherwise read as your edit and go red. It stops with a fix-it message when there is no remote main, and never uses a local branch. Blast radius: one shell hook plus its new test; no app code changes, and a clone with a single up-to-date origin behaves exactly as before.

Closes OME-1443.

Before / After

flowchart TB
  subgraph TODAY["TODAY — a docs-only push runs three stacks' full test suites"]
    direction LR
    a1["👤 push a branch that only edits docs"] -->|no origin/main, falls back| a2["⚠️ compares against local main<br/>8 commits behind upstream"]
    a2 -->|upstream's commits look like this branch's| a3["engine + aigateway + aigateway-ui<br/>checks run, minutes per push"]
  end
  subgraph AFTER["AFTER — only stacks the branch really changed are checked"]
    direction LR
    b1["👤 push a branch that only edits docs"] -->|any remote name| b2["compares against the remote main<br/>the branch was cut from"]
    b2 -->|no stack changed| b3["✅ no checks run<br/>this branch's push took 4 s"]
  end
  TODAY ~~~ AFTER
  classDef bad   fill:#7f1d2b,stroke:#e5484d,color:#ffe8ea
  classDef good  fill:#14532d,stroke:#30a46c,color:#dcfce7
  classDef stage fill:#1e3a8a,stroke:#4a7fd4,color:#dbeafe
  classDef plain fill:#374151,stroke:#9ca3af,color:#f3f4f6
  class a1,b1,b2 stage
  class a2 bad
  class a3 plain
  class b3 good
Loading

For the developer, a push that touches no stack goes from minutes to seconds. A push that really touches the engine still runs the engine's full suite, as designed.

Don't regress

  • A branch that changes the engine still gates exactly the engine, now with --base upstream/main (pinned by a test).
  • A clone whose remote is origin keeps working unchanged (pinned by a test).
  • A stale second remote never wins because of its name or its sort order (pinned by two tests).
  • main moving on after you branch never makes its test changes look like yours (pinned by a test).

Architecture / Design

Skipped: the mechanism is one loop in the hook. It carries one "because": the base is chosen by counting commits, not by remote name, because names differ between clones (origin, upstream, sc-remote) and a fork can have two remotes where one is stale. Counting the commits between each remote main and the branch picks the main the branch was really cut from, whatever it's called.

Known limitations of this design

  • A branch cut from another branch is still compared with main. Example: B is cut from A, A changed the engine, B only edits docs; B's push re-runs the engine checks. Slower, never misses, and it's how the hook behaved before. A per-branch override can come later if stacked PRs make it annoying.
  • A clone with no remote main at all has its push stopped with a message telling you to fetch. Accepted: guessing from a local branch is the bug this removes.
  • The hook never runs the repo stack's own checks (the scripts under .claude/scripts/), because repo isn't in its stack list. That was already true before this change, and it's out of scope here.

Review order

  1. .githooks/pre-push: the whole change. Check that only refs/remotes/*/main are considered, so a local branch can never be picked; that the fewest commits ahead wins; that "no remote main" exits 1 before any check runs; and that the checks get git merge-base <remote>/main HEAD, not the remote main itself.
  2. .claude/scripts/tests/test_pre_push.py: runs the real hook in throwaway git repos with a stub uv that records which stacks would be checked. Eight cases: remotes named upstream, origin and sc-remote; a stale second remote named upstream, and one named origin that sorts first (only counting commits gets that one right; a hook that took the first remote main failed it); a real engine change, checked against the branch point; main adding an engine test after the cut; no remote at all. The upstream docs-only test failed on the old hook, with screamingface-engine main.
  3. Skim: .claude/sdlc.local.md registers the test under the repo stack's checks; the ledger and the docs/tasks mirror.

🤖 Generated with Claude Code

… a local main

A clone whose remote is named upstream has no origin/main, so the hook fell back to the
local main branch. A local main that lags the remote made every remote commit since look
like this branch's change, and a docs-only push ran the gates of every stack those commits
touched. The hook now uses upstream/main, then origin/main, and stops with a fix-it message
when neither exists.
@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Preview: disabled

  • Revision: 28252ccfde78d229bdcb4be307731c3f887389ac
  • Components: none
  • Images: none

Among every <remote>/main, the hook now compares against the one this branch has the
fewest commits on top of, instead of looking for upstream/main or origin/main by name. A
remote called sc-remote works, and a stale second remote no longer wins by its name.
@khoaguin khoaguin changed the title fix(repo): compare the pre-push hook against the remote's main, never a local main fix(repo): pick the pre-push base by history, never a local main Oct 1, 2026
…main's latest commit

run_gates.py's append-only test check diffs its base against the working tree, so a test
that main changed after the branch was cut read as this branch's edit and went red. The
hook now passes git merge-base <remote>/main HEAD.
@khoaguin
khoaguin merged commit 575aa96 into main Oct 1, 2026
21 checks passed
@khoaguin
khoaguin deleted the OME-1443-pre-push-base-fallback branch October 1, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant