Skip to content

ci: gate merges on the test check, and verify the merge result - #238

Merged
eaitbrahim merged 2 commits into
mainfrom
ci/run-on-merge
Aug 11, 2026
Merged

ci: gate merges on the test check, and verify the merge result#238
eaitbrahim merged 2 commits into
mainfrom
ci/run-on-merge

Conversation

@eaitbrahim

Copy link
Copy Markdown
Contributor

Adds push: branches: [main] back to ci.yml, keeping workflow_dispatch.

on:
  push:
    branches: [main]
  workflow_dispatch:

Why this is not the redundant run it was called in #200

#200 removed this trigger on the grounds that it re-ran content the pull_request run had already passed. That reasoning held while pull_request still existed. It does not now — #234 removed it, so nothing automatic runs at all.

It was also incomplete even then. A pull_request run tests the PR head; a push run tests main after the merge landed. Those diverge whenever two independently-green branches conflict semantically — each passes alone, the merge of them does not. Only the post-merge run observes the tree that actually resulted.

What this does and does not buy

Does: every merge is verified within minutes, against the real post-merge tree. Given three PRs in this repo have merged mid-flight while an agent was still pushing to them (#192, #218, #235), time-to-discovery is the practical problem, and this addresses it.

Does not: block a merge. It runs after. Blocking needs two things this repo lacks, and they must move together:

  1. a pull_request trigger, and
  2. a ruleset that actually matches main and requires the test context.

The ruleset named main exists but matches no refs (ref_name.include is empty; GET /rules/branches/main returns []), and its required contexts ("Lint, Unit tests, Build", "Integration Tests") match no job here. Pointing it at main without fixing the contexts would block every PR forever on checks that can never report. Tracked in #236.

release.yml is unchanged and remains the gate that matters for money — it re-runs ruff + pytest before building an artifact, so a red main still cannot ship.

Concurrency

Unchanged (ci-${{ github.ref }}). The comment is updated: back-to-back merges to main now cancel the older run, which is correct — the newest main is the only one whose result still matters.

Validation

  • YAML parses; triggers read {'push': {'branches': ['main']}, 'workflow_dispatch': None} (checked via d[True], since PyYAML follows YAML 1.1 where bareword on is boolean).
  • actionlint clean, exit 0.
  • Job steps untouched — the diff is triggers and comments only.

A push to `main` is what a merge looks like from Actions' side, so every merged
PR now re-runs lint, the suite, and the build-identity check.

This is NOT the redundant run it was once called. A `pull_request` run tests the
PR head; a `push` run tests `main` after the merge landed. Those differ whenever
two independently-green branches conflict semantically -- each passes alone, the
merge of them does not, and only the post-merge run can see it.

It runs AFTER the merge and so cannot block one; that needs a `pull_request`
trigger plus a ruleset that actually matches `main` and requires the `test`
context. The comment records why both halves have to move together.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@eaitbrahim eaitbrahim added the ci CI/workflows (Docs, CI & tooling) label Aug 11, 2026
`pull_request` is restored because it is what makes this job report a status
named `test` on a PR, and the `main` ruleset now REQUIRES that context before a
merge is allowed (#236). The trigger and the ruleset are a matched pair.

So the two automatic triggers do different jobs: `pull_request` GATES the merge,
`push: [main]` VERIFIES the tree that resulted from it. The second is not
redundant -- two independently-green branches can conflict semantically, and
only the post-merge run observes that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@eaitbrahim eaitbrahim changed the title ci: run the suite on every merge to main ci: gate merges on the test check, and verify the merge result Aug 11, 2026
@eaitbrahim

Copy link
Copy Markdown
Contributor Author

Amended: added pull_request alongside push: [main].

on:
  pull_request:
  push:
    branches: [main]
  workflow_dispatch:

This PR is now half of the #236 fix and must merge first. The other half — repointing the main ruleset at the branch and swapping its two phantom contexts for test — can only be applied once this is on main, or the required check would never report and every PR (including this one) would deadlock.

The context string is test because the job is test: with no name:; confirmed against a real completed run on #220, which reported exactly test. That coupling is now written into the workflow comment, since renaming the job would silently break the ruleset.

actionlint clean; triggers parse as {'pull_request': None, 'push': {'branches': ['main']}, 'workflow_dispatch': None}. Concurrency comment corrected — PR runs key on refs/pull/<n>/merge, so they never cancel the post-merge main run.

@eaitbrahim
eaitbrahim merged commit 90e87a8 into main Aug 11, 2026
1 check passed
@eaitbrahim
eaitbrahim deleted the ci/run-on-merge branch August 11, 2026 17:26
eaitbrahim added a commit that referenced this pull request Aug 11, 2026
…the CTS scoring fix (#241)

A minor bump, not a patch, for three reasons that each require operator action or
change behaviour the deployment is currently relying on.

SCHEMA. `SCHEMA_VERSION` goes 9 -> 10 (#223). Both deployed databases are at 9 and
must be migrated before this build can use them.

BEHAVIOUR REQUIRING OPERATOR ACTION. #223 adds a second attested claim -- what
CONTRACT a venue listing is, not only what the underlying asset is. It fails closed
with no backfill, deliberately, so after this lands `keel assets screen` REJECTS
every product with `instrument_wrapper: UNATTESTED` until `keel assets
attest-instrument` is run once per product. Live trading is unaffected: rail 1 gates
buys on `config.allowlist`, not on the screen.

LIVE SCORING CHANGED. #227 fixed `is_round_number`, which returned True for every
2dp-quoted price and so handed BTC/ETH/PAXG a free CTS point on every bar. Scores on
those three assets are genuinely lower under this build than under 0.5.7.

Also ships: the Robinhood crypto adapter behind the broker port (#216/#218/#222/#229,
not wired to the live path), the TUI activity feed (#235/#237), the CTS factor
collinearity study (#224), `Preview.synthetic` at the confirm gate (#221), rail 9
seeing a bracket's own stop (#212), and CI gating merges on the `test` check
(#234/#238).

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI/workflows (Docs, CI & tooling)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant