Comfy-Org / Comfy-Org/ComfyUI_frontend
Coverage Slack notifier is a one-way ratchet: regressions are unpostable, and it posts into #p-deprecated-frontend-automated-testing
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 704
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 512
Description
`coverage-slack-notify.yaml` posts a coverage update to Slack on every successful `CI: Tests Unit` run on `main`. Two problems, one structural:
1. **It posts only when coverage goes up.** A regression is literally unpostable.
2. **It posts into `#p-deprecated-frontend-automated-testing`** — a channel whose name declares the project dead.
## 1. The one-way ratchet
`scripts/coverage-slack-notify.ts`:
```ts
if (!unitImproved && !e2eImproved) { process.exit(0) }
```
The early return fires whenever coverage is flat or down, so the only message the workflow can ever emit is the improvement case — `const payload = { text: 'Coverage improved!', blocks }`. That is why every message in the channel's history reads "Coverage improved!": it is true by construction, not by measurement.
The effect: the org's only ambient coverage telemetry presents an unbroken record of improvement, and a coverage regression on `main` produces **silence** — which is indistinguishable from "the workflow didn't run", "CI was red so it was skipped", and "coverage was flat". Anyone reading the channel as a health signal is reading a ratchet.
This is the "an aggregate reads green while a component regressed" shape, with the reporting channel itself as the aggregate.
## 2. The E2E row can vanish silently
The workflow downloads the e2e coverage artifact with `continue-on-error` plus `if_no_artifact_found: warn`. If the e2e coverage job stops producing an artifact, `e2eCurrent` is `null` → `e2eDelta` is `0` → the E2E row is simply **absent** from the Slack post, with no error and no mention that it is missing. A reader sees a well-formed message about unit coverage and has no way to notice that half the report stopped existing.
Relevant history: on 2026-08-07 @DrJKL posted "e2e ded" in this channel. That claim stood unrebutted for two weeks and is **no longer true** — `ci-tests-e2e-coverage.yaml` is running and succeeding (runs 32532324443, 32531238187, 32531228798, all `success`, 2026-08-21). But the notify workflow is built so that if e2e dies *again*, nothing in the channel will say so.
## 3. The destination channel
```yaml
# Channel: #p-deprecated-frontend-automated-testing
BODY=$(echo "$SLACK_PAYLOAD" | jq --arg ch "C0AP09LKRDZ" '. + {channel: $ch}')
```
Hardcoded channel ID. The channel has had exactly two human messages in the last three weeks; the rest is this bot, ~40 posts. The workflow itself is not cheap — full `checkout` + `setup-frontend` (pnpm install) + 4 artifact downloads, ~1m per run, 10+ runs/day — so this is live CI spend delivering a structurally-positive-only metric into a channel named "deprecated".
## Suggested fix
Pick one of two coherent end states rather than leaving it half-alive:
**Keep it** — then make it honest:
- Delete the `process.exit(0)` early return so flat and regressed both post. A regression is the only message with real information content.
- Repoint `C0AP09LKRDZ` at a live channel (`#frontend` or `#frontend-tech-improvements`).
- Make a missing e2e artifact render as an explicit "E2E: no data" row instead of dropping the row.
**Or turn it off** — delete the workflow and the script. A metric nobody can act on, in a channel nobody reads, is worth less than the CI minutes.
The first option is probably right, but the choice should be explicit; the current state is the result of drift, not a decision.
Found by a random-walk sweep crossing `#p-deprecated-frontend-automated-testing` against this repo — the permutation was chosen because the channel name asserts the project is dead, and the question was whether its infrastructure had actually been torn down. It had not.
Contributor guide
Assessment
This issue has not been assessed yet.