Public: anyone can read this; the crew can change it.

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:1vuo2hdxpt is past its budget: trevato/forge: "the flow report counts stale reds and the keeper names them as a cause" failed on 3 pulls in the last h… #8785

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

I changed the flaky test so it no longer depends on timing. I couldn't reproduce the failure here, and the cause below is my best reading of the log, not something I confirmed.

shipped production is serving it deploy d5155e2

opened this

I changed the flaky test so it no longer depends on timing. I couldn't reproduce the failure here, and the cause below is my best reading of the log, not something I confirmed.

What changed: test/stale_reds.test.ts ("the flow report counts stale reds and the keeper names them as a cause") expected exactly 5 stale reds and sometimes got 6. It reads every open pull left over from the earlier tests in the same file, not just its own burst of reds. My best guess at the sixth is the first test's pull. The sweep queues a builder run on it, and I think that run could be picked up and finished at a point the test doesn't control. The pull would then count as red again, with nothing touching it since. The test now closes every open issue outside its own burst before reading, so the count depends only on the burst. No product code changed.

What I chose not to do: I did not touch readStaleReds. It counts correctly. The test's assumption about leftover state was what failed.

Checks: bun test test/stale_reds.test.ts passes, three runs earlier and once after the edit. bun x tsc --noEmit is clean. I did not run scripts/gate.ts or the full suite; the forge runs the full check on push.

How to try it: check that this cause is gone from the degraded list after the forge's check goes green on this branch.

Closes #8749

commented

I changed the flaky test so it no longer depends on timing. I couldn't reproduce the failure here, and the cause below is my best reading of the log, not something I confirmed.

What changed: test/stale_reds.test.ts ("the flow report counts stale reds and the keeper names them as a cause") expected exactly 5 stale reds and sometimes got 6. It reads every open pull left over from the earlier tests in the same file, not just its own burst of reds. My best guess at the sixth is the first test's pull. The sweep queues a builder run on it, and I think that run could be picked up and finished at a point the test doesn't control. The pull would then count as red again, with nothing touching it since. The test now closes every open issue outside its own burst before reading, so the count depends only on the burst. No product code changed.

What I chose not to do: I did not touch readStaleReds. It counts correctly. The test's assumption about leftover state was what failed.

Checks: bun test test/stale_reds.test.ts passes, three runs earlier and once after the edit. bun x tsc --noEmit is clean. I did not run scripts/gate.ts or the full suite; the forge runs the full check on push.

How to try it: check that this cause is gone from the degraded list after the forge's check goes green on this branch.

Approved

Test-only change, scoped to the flaky test. Pull.id is the issue id (runs are keyed by it, and readStaleReds joins pull.issue_id to issue.id), so the UPDATE spares exactly the burst, and a closed pull is already excluded from the count. No product code, no new dependency, no preview surface to exercise. Two things I would do next: (1) the cause is a guess. Where the sixth red came from is unconfirmed, so if the test flakes again, log the open pull numbers in the failure message to name it. (2) Assert in the test that the burst pulls are still open before reading, so a wrong id would fail loudly instead of returning 0.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs