coilyco-gaming/eco-app

Watch

ci: gate pull requests with the same jobs main runs #308

Merged

coilyco-ops merged 1 commit from aos/claude/bk79-eco-pr-ci into main

2026-08-16 07:06:26 +00:00

coilyco-ops commented

2026-08-16 07:05:11 +00:00

Owner

Why

eco-app had no pre-merge gate. build-publish.yml triggered only on push to main and workflow_dispatch, so a pull request ran nothing and the first CI signal arrived after the merge.

That cost us one already. #306 merged and turned main red — run 215, job test, on ward exec lint:

Would reformat: src/eco_mcp_app/server.py
1 file would be reformatted, 122 files already formatted
exit status 1

A missing blank line before a top-level def. ward exec lint catches it and CI already runs ward exec lint — it just ran too late to stop the merge. This PR fixes that line and closes the window that let it land.

What

One workflow, both triggers. Adding pull_request: to the existing workflow rather than writing a PR-only copy means the gate a PR passes is byte-for-byte the gate main runs. A second file would drift.

That leaves the publishing jobs, which must not fire on a PR:

build-image and publish-mod now check github.event_name == 'push' && github.ref == 'refs/heads/main'.
test and frontend were already trigger-agnostic and now run on both.

build-image alone would technically be enough — publish-mod needs it, and a skipped need skips the dependent — but each job stating its own rule survives someone later adding always() to a needs chain. That is why sirens-echo's publish-observed carries both.

This mirrors coilyco-gaming/sirens-echo ci.yml, which is the house shape already: push: [main] + pull_request:, publish jobs gated on the event.

Alert channel

The Telegram steps are keyed on failure() && github.ref == 'refs/heads/main'. A PR ref is refs/pull/N/merge, so PR failures never reach the alert channel. No change needed.

Verification

ward exec lint, ward exec test, ward exec smoke and pre-commit run --all-files all pass locally on this branch, with no hook rewrites left in the tree. The workflow YAML parses and the job graph is what it claims:

test           runs-on=docker  needs=None                 if=NO
frontend       runs-on=docker  needs=None                 if=NO
build-image    runs-on=deploy  needs=[test, frontend]     if=yes
publish-mod    runs-on=deploy  needs=[build-image]        if=yes

This PR is its own test. It is the first eco-app PR that runs CI, and its own run is the evidence that test and frontend fire while the two deploy jobs stay skipped. Worth reading the run before merging rather than taking the diff's word for it.

Two things I found but did not fix

The alert never fired. Run 215's log shows telegram alert missing required secret — TELEGRAM_BOT_TOKEN or TELEGRAM_RED_CHAT_ID is unset for this repo, so main went red silently. Setting repo secrets is Kai's, not mine.

CI does not run pre-commit. sirens-echo runs pre-commit run --all-files in its test job; eco-app runs only test/lint/smoke, so the catalog hooks (doc caps, trifecta, layout) are enforced at commit time but not in CI. Deliberately left out of this PR: it would make PR CI stricter than main CI, which is a separate change, and hook installation needs egress I cannot verify without pushing and watching.

Refs #306

## Why eco-app had **no pre-merge gate**. `build-publish.yml` triggered only on `push` to main and `workflow_dispatch`, so a pull request ran nothing and the first CI signal arrived after the merge. That cost us one already. **#306 merged and turned main red** — run [215](https://forgejo.coilysiren.me/coilyco-gaming/eco-app/actions/runs/215), job `test`, on `ward exec lint`: ``` Would reformat: src/eco_mcp_app/server.py 1 file would be reformatted, 122 files already formatted exit status 1 ``` A missing blank line before a top-level `def`. `ward exec lint` catches it and CI already runs `ward exec lint` — it just ran too late to stop the merge. This PR fixes that line and closes the window that let it land. ## What **One workflow, both triggers.** Adding `pull_request:` to the existing workflow rather than writing a PR-only copy means the gate a PR passes is byte-for-byte the gate main runs. A second file would drift. That leaves the publishing jobs, which must not fire on a PR: * `build-image` and `publish-mod` now check `github.event_name == 'push' && github.ref == 'refs/heads/main'`. * `test` and `frontend` were already trigger-agnostic and now run on both. `build-image` alone would technically be enough — `publish-mod` needs it, and a skipped need skips the dependent — but each job stating its own rule survives someone later adding `always()` to a `needs` chain. That is why sirens-echo's `publish-observed` carries both. This mirrors `coilyco-gaming/sirens-echo` `ci.yml`, which is the house shape already: `push: [main]` + `pull_request:`, publish jobs gated on the event. ## Alert channel The Telegram steps are keyed on `failure() && github.ref == 'refs/heads/main'`. A PR ref is `refs/pull/N/merge`, so PR failures never reach the alert channel. No change needed. ## Verification `ward exec lint`, `ward exec test`, `ward exec smoke` and `pre-commit run --all-files` all pass locally on this branch, with no hook rewrites left in the tree. The workflow YAML parses and the job graph is what it claims: ``` test runs-on=docker needs=None if=NO frontend runs-on=docker needs=None if=NO build-image runs-on=deploy needs=[test, frontend] if=yes publish-mod runs-on=deploy needs=[build-image] if=yes ``` **This PR is its own test.** It is the first eco-app PR that runs CI, and its own run is the evidence that `test` and `frontend` fire while the two `deploy` jobs stay skipped. Worth reading the run before merging rather than taking the diff's word for it. ## Two things I found but did not fix **The alert never fired.** Run 215's log shows `telegram alert missing required secret` — `TELEGRAM_BOT_TOKEN` or `TELEGRAM_RED_CHAT_ID` is unset for this repo, so main went red silently. Setting repo secrets is Kai's, not mine. **CI does not run pre-commit.** sirens-echo runs `pre-commit run --all-files` in its test job; eco-app runs only test/lint/smoke, so the catalog hooks (doc caps, trifecta, layout) are enforced at commit time but not in CI. Deliberately left out of this PR: it would make PR CI *stricter* than main CI, which is a separate change, and hook installation needs egress I cannot verify without pushing and watching. Refs #306

coilyco-ops added 1 commit

2026-08-16 07:05:11 +00:00

ci: gate pull requests with the same jobs main runs

build-publish / frontend (pull_request) Successful in 23s

Details

build-publish / test (pull_request) Successful in 41s

Details

build-publish / build-image (pull_request) Has been skipped

Details

build-publish / publish-eco-jobs-tracker (pull_request) Has been skipped

Details

build-publish / publish-eco-replay (pull_request) Has been skipped

Details

build-publish / publish-eco-store-exporter (pull_request) Has been skipped

Details

build-publish / publish-eco-telemetry (pull_request) Has been skipped

Details

e06fd54032

eco-app had no pre-merge gate. Its only automatic trigger was push-to-main,
so a PR could be merged on nothing and the failure surfaced after landing.
That is not hypothetical: #306 merged green-looking and turned main red on
`ward exec lint` (run 215), for a missing blank line this file now fixes.

The same workflow serves both triggers rather than a second PR-only copy,
so the gate a PR passes is the gate main runs and the two cannot drift.
The verifying jobs (test, frontend) were already trigger-agnostic. The two
that push bytes to a registry now check the event themselves. build-image
alone would be enough, since publish-mod needs it and a skipped need skips
the dependent, but each job stating its own rule survives someone later
adding `always()` to a needs chain.

The Telegram alert steps are keyed on `refs/heads/main`, so a PR ref never
matches and PR failures stay off the alert channel. No change needed there.

Refs coilyco-gaming/eco-app#306

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-16 07:05:11 +00:00

coilyco-ops referenced this pull request

2026-08-16 07:05:47 +00:00

Main went red and the alert never fired: Telegram secrets are unset for this repo #309

coilyco-ops referenced this pull request

2026-08-16 07:06:09 +00:00

CI does not run pre-commit, so the catalog hooks are enforced only on a developer's machine #310

coilyco-ops merged commit 7ac3904b92 into main

2026-08-16 07:06:26 +00:00

coilyco-ops referenced this pull request from a commit

2026-08-16 07:06:28 +00:00