MetaMask / MetaMask/metamask-extension

[P1] A timed-out benchmark blocks CI as an opaque cancellation and is invisible to the quality gate

Open
#45,669 1 comment 0 reactions 0 assignees View on GitHub
INVALID-ISSUE-TEMPLATE ta-ai-fixable ta-needs-engineer-escalation ta-triaged team-extension-platform
Dominant language
TypeScript
Stars
13.2k
Forks
5.6k
Avg merge
2d 5h
Merged PRs (30d)
451

Description

**Parent Epic:** [#7588 Gate Verdicts and Output](https://github.com/MetaMask/MetaMask-planning/issues/7588)
**File:** `.github/workflows/run-benchmarks.yml`, `.github/workflows/ci-status-gate.yml`, `development/metamaskbot-build-announce/compare-benchmarks.ts`
**Size:** M | **Hours:** 4–8

---

## Problem

A benchmark shard that **times out** blocks CI in the one place that cannot explain why, and is invisible in the one place that could.

Both benchmark jobs carry `continue-on-error: true`, so a shard that *fails* is absorbed. A shard that exceeded the job's `timeout-minutes: 30` was **`cancelled`**, not failed, and `continue-on-error` does not cover cancellation. The cancellation propagates to the reusable-workflow call and lands on the aggregate.

### Observed on a release candidate

[Run 32370339725](https://github.com/MetaMask/metamask-extension/actions/runs/32370339725) on `release/13.45.0` at `c9e713c`, attempt 2. Of 119 jobs, exactly one was not `success`/`skipped`: `run-benchmarks / chrome-webpack-userJourneyTransactions`, which ran 14:45:39 → 15:15:58 — 30m19s against a 30-minute job timeout. The [aggregate](https://github.com/MetaMask/metamask-extension/actions/runs/32370339725/job/96477898718) failed with:

```
run-benchmarks did not succeed (result: cancelled)
```

### Failure and cancellation are treated differently, though both mean "no data"

| run | shard | `quality-gate` | `run-benchmarks` | `All jobs pass` |
|---|---|---|---|---|
| [32351197729](https://github.com/MetaMask/metamask-extension/actions/runs/32351197729) | **failure** (`chrome-webpack-pageLoadBenchmark`) | success | success | **success** |
| [32346566466](https://github.com/MetaMask/metamask-extension/actions/runs/32346566466) | failure | **failure** | failure | failure |
| [32338723978](https://github.com/MetaMask/metamask-extension/actions/runs/32338723978) | failure | **failure** | failure | failure |
| [32370339725](https://github.com/MetaMask/metamask-extension/actions/runs/32370339725) | **cancelled** | skipped | **cancelled** | **failure** |

`run-benchmarks` propagates `failure` only when the **quality gate itself** failed — which is the intended policy. Shard failures are already absorbed. Cancellation is the single leak.

### Step timeouts closed the cancellation path

[#45676 (prevent cascading benchmark timeouts)](https://github.com/MetaMask/metamask-extension/pull/45676), merged 2026-08-20, gave both benchmark steps a `timeout-minutes: 25` under the job's `timeout-minutes: 30`, so a hang now fails a step — which `continue-on-error: true` absorbs — rather than canceling the job: [`run-benchmarks.yml:79-82`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/.github/workflows/run-benchmarks.yml#L79-L82) and [`run-benchmarks.yml:171-174`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/.github/workflows/run-benchmarks.yml#L171-L174). That converts a loud, uninformative block into silence: the shard still produces no artifact, and nothing downstream records that its metrics went unmeasured. Both remaining items address that silence.

### The block carries no information, and its status text is wrong

`checkResult` resolves to `cancelled`, and [`ci-status-gate.yml:212-216`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/.github/workflows/ci-status-gate.yml#L212-L216) posts the description *"CI run was cancelled (queue reshuffle or manual cancellation)"*. Nothing was reshuffled and nobody canceled anything — a benchmark timed out. [`main.yml:587`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/.github/workflows/main.yml#L587) lists `run-benchmarks` as a **single** `needs` entry of `ci-status-gate`, so the aggregate cannot name the shard, the browser, or the metrics that lost coverage.

### Meanwhile the gate, which could explain it, never sees the gap

The timed-out shard uploaded **no artifact**. Every other shard's is present on the run, including `benchmark-firefox-webpack-userJourneyTransactions.json`; `benchmark-chrome-webpack-userJourneyTransactions.json` is absent.

[`compare-benchmarks.ts:55-69`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/development/metamaskbot-build-announce/compare-benchmarks.ts#L55-L69) enumerates its input with `await fs.readdir(dirPath)` and iterates whatever is there. The file declares no expected-benchmark manifest and no identifier containing `expected` — so a missing artifact is not a missing signal, it is simply a shorter list.

That matters here because `userJourneyTransactions` writes `sendTransactions` and `swap`, which between them own five entries in `GATED_METRICS` ([`gated-metrics.ts:24-73`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/gated-metrics.ts#L24-L73)):

```
METRIC.sendTransactions.cls
METRIC.sendTransactions.openSendPageFromHome
METRIC.swap.cls
METRIC.swap.total
METRIC.swap.fetchAndDisplaySwapQuotes
```

The firefox shard wrote the same two benchmark keys, so the gate still scored all five — on **one browser instead of two**, with nothing recording that chrome's coverage was lost.

### Why #45047 does not already cover this

[#45047 (a no-result benchmark blocks when it owns a gated metric)](https://github.com/MetaMask/metamask-extension/pull/45047) makes a no-result benchmark block when it owns a gated metric, and its description names timeouts explicitly. But it acts on an artifact whose contents are `{"error": "…"}`. A timeout produces **no artifact at all**, which is a different input and reaches none of that logic.

---

## Solution

**1. Give the gate an expected-benchmark manifest.** Derive the expected `--` set from the same source as the matrix, and treat an absent artifact exactly as #45047 treats an error artifact: block when the missing benchmark owns a metric in `GATED_METRICS`, report-only `ERROR` otherwise. A metric measured on one browser when two were expected is a coverage loss, not a pass. [#45670 (fail the quality gate on a benchmark that produced no artifact)](https://github.com/MetaMask/metamask-extension/pull/45670) is the open implementation.

**2. Correct the `cancelled`-status description** in `ci-status-gate.yml:212-216` so it stops asserting a queue reshuffle for what may be a timeout. The same assumption is written into the merge-queue skip at [`ci-status-gate.yml:281-286`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/.github/workflows/ci-status-gate.yml#L281-L286), which suppresses the merge-group outcome event whenever `CI_RESULT` is `cancelled`.

---

## Acceptance Criteria

- [ ] A benchmark shard that hangs past its step timeout leaves `run-benchmarks` resolving `success`, not `cancelled` — the step timeouts are in place as of #45676 (merged 2026-08-20); confirm against a run that actually reaches the 25-minute step limit
- [ ] `All jobs pass` is not failed by a benchmark shard timing out
- [ ] The quality gate fails when an expected benchmark artifact is absent and that benchmark owns a metric in `GATED_METRICS`
- [ ] The quality gate reports an absent non-gated benchmark as a report-only `ERROR`, consistent with #45047
- [ ] The gate's output names which browser/pageType combinations were expected and which were missing
- [ ] A metric scored on fewer browsers than expected is reported as reduced coverage rather than a clean pass
- [ ] The commit status posted for a `cancelled` aggregate no longer claims "queue reshuffle or manual cancellation" without evidence
- [ ] Regression test: an artifact directory missing one gated benchmark's file exits non-zero; the same directory complete exits zero

---

## Labels

`team-extension-platform`, `area-ci`, `type-bug`

---

## Dependencies

**Depends on:** #45047 (a no-result benchmark blocks when it owns a gated metric) — shares the missing-signal policy this extends from error artifacts to absent ones
**Addressed by:** #45670 (fail the quality gate on a benchmark that produced no artifact)
**Related:** #45450 (a gated benchmark that never ran scores as a pass), #45046 (`quality-gate` reds on `main` for a reason no branch caused)

Contributor guide

Open the contributing guide

Research direction

Start with .github/workflows/run-benchmarks.yml, .github/workflows/ci-status-gate.yml, and development/metamaskbot-build-announce/compare-benchmarks.ts. Review #45670, the open implementation, then inspect gated-metrics.ts and existing coverage tests. Done means missing artifacts are detected, gated coverage fails appropriately, non-gated gaps are reported, and cancellation text no longer assumes a manual cancellation.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, typescript
Domain
ci-cd, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.