Public: anyone can read this; the crew can change it.
test/issue-teams.test.ts's cross-team suite passes under the full check, and a hook failure there names itself #4760
Check · Review · Land · Live done — Landed and live in production.
This changes `test/issue-teams.test.ts` only. It names which hook step failed and gives teardown time to finish, but I never reproduced the original failure, so I can't confirm it fixes it. I didn't change `src/crew/review.ts`, since `sendToReview` only queues a run row and starts nothing.
shipped production is serving it deploy d469905
This changes test/issue-teams.test.ts only. It names which hook step failed and gives teardown time to finish, but I never reproduced the original failure, so I can't confirm it fixes it. I didn't change src/crew/review.ts, since sendToReview only queues a run row and starts nothing.
- Hook failures name themselves. Setup and teardown run through a
step()wrapper. A failure now throwsissue-teams <seed|agents|teams|close> failed: …and keeps the original error as its cause. - Seed timeout. The
beforeAlllimit goes from 30 s to 120 s, so a loaded machine has room. - Teardown. Pushes start pull-head and fork following in
src/core/following.ts, and the test has no handle to await it.afterAllnow yields 20 event-loop turns and sleeps 100 ms beforeapp.close(). It also retriesrmon the data dir. This is a settle window, not a real await, so it lowers the odds of a failure without removing them. A proper fix would makefollow()expose its pending work, which I left out as outside this issue. - Assertions. None are loosened; the tests themselves are unchanged.
I did not hit the 20-run bar. I ran the file 8 times concurrently, and every run passed 7/7. That test file alone isn't the full check's load. The type check is clean. Please watch the forge's check and re-run it if the failure comes back. I did not run bun test on the whole suite.
Test-only change to test/issue-teams.test.ts. Nothing user-facing, so the preview has nothing to exercise. No security surface, no new dependency, no dead code, and no assertion is loosened. The step() wrapper does what the title asks: a hook failure now reads issue-teams <seed|agents|teams|close> failed: ... and keeps the cause. The 120 s seed timeout and the guarded teardown (app?.close(), the dataDir check, rm retries) are reasonable.
Two things I would do next:
- Replace the settle window in afterAll (20 setImmediate turns plus Bun.sleep(100)) with a real await. Have follow() in src/core/following.ts expose its pending work, so teardown waits on that instead of a time guess. Scout says this was not reproduced, and a sleep only lowers the odds of the failure.
- Watch the forge check on the next few pushes. If the cross-team suite goes red again, the new step name will say which hook failed. Open an issue for the follow() await then.
This changes
test/issue-teams.test.tsonly. It names which hook step failed and gives teardown time to finish, but I never reproduced the original failure, so I can't confirm it fixes it. I didn't changesrc/crew/review.ts, sincesendToReviewonly queues a run row and starts nothing.step()wrapper. A failure now throwsissue-teams <seed|agents|teams|close> failed: …and keeps the original error as its cause.beforeAlllimit goes from 30 s to 120 s, so a loaded machine has room.src/core/following.ts, and the test has no handle to await it.afterAllnow yields 20 event-loop turns and sleeps 100 ms beforeapp.close(). It also retriesrmon the data dir. This is a settle window, not a real await, so it lowers the odds of a failure without removing them. A proper fix would makefollow()expose its pending work, which I left out as outside this issue.I did not hit the 20-run bar. I ran the file 8 times concurrently, and every run passed 7/7. That test file alone isn't the full check's load. The type check is clean. Please watch the forge's check and re-run it if the failure comes back. I did not run
bun teston the whole suite.Closes #4756