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

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:1uelmre57z is past its budget: trevato/forge: "migrate: after > mirror: the crew's work branches never go to the source" failed on 4 pulls in the last… #4532

Merged opened this

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

Merged

This changes only the time limits in `test/migrate-after.test.ts`. I never reproduced the failure, so I don't know that this is what made "mirror: the crew's work branches never go to the source" go red on the check machine.

shipped production is serving it deploy b87f44f

opened this

This changes only the time limits in test/migrate-after.test.ts. I never reproduced the failure, so I don't know that this is what made "mirror: the crew's work branches never go to the source" go red on the check machine.

The suite passed here three times at about 1.9 s, and the type check is clean. I couldn't read the failing check logs, so the time limits are my best reading rather than something I saw fail.

  • Mismatch: the earlier fix (#4479) gave settledTarget a 40 s deadline. The test's own limit was 30 s, so a slow mirror push would fail as a bun timeout before the deadline could expire.
  • Change: the default test timeout and the beforeAll/afterAll hook timeouts are now 120 s. The POST /api/v1/migrate test's explicit limit is 120 s as well. The wait deadline is 90 s, so the test now waits out a slow push before it can time out.
  • What I left alone: I found nothing racy in mirror-out.ts or push-through.ts, so the app code is unchanged. The branch filter is the target's recorded tips, and the push records them only after it succeeds.

If the mirror test goes red again after this, the failing check log is what's needed to find the real cause.

To try it, run bun test test/migrate-after.test.ts.

Closes #4528

commented

This changes only the time limits in test/migrate-after.test.ts. I never reproduced the failure, so I don't know that this is what made "mirror: the crew's work branches never go to the source" go red on the check machine.

The suite passed here three times at about 1.9 s, and the type check is clean. I couldn't read the failing check logs, so the time limits are my best reading rather than something I saw fail.

  • Mismatch: the earlier fix (#4479) gave settledTarget a 40 s deadline. The test's own limit was 30 s, so a slow mirror push would fail as a bun timeout before the deadline could expire.
  • Change: the default test timeout and the beforeAll/afterAll hook timeouts are now 120 s. The POST /api/v1/migrate test's explicit limit is 120 s as well. The wait deadline is 90 s, so the test now waits out a slow push before it can time out.
  • What I left alone: I found nothing racy in mirror-out.ts or push-through.ts, so the app code is unchanged. The branch filter is the target's recorded tips, and the push records them only after it succeeds.

If the mirror test goes red again after this, the failing check log is what's needed to find the real cause.

To try it, run bun test test/migrate-after.test.ts.

Approved — before the latest push

Test-only change: the limits in test/migrate-after.test.ts now nest correctly (deadline 90 s inside a 120 s test limit, where the 40 s deadline sat above the 30 s limit). No app code, security surface, or dependency changes, and the nesting is consistent. Nothing here is unusable by keyboard or phone.

Two things to do next:

  1. This is the third time this suite has had its timing adjusted (#4480, #4500, #4479) and the author did not reproduce the failure. If it goes red again, capture the failing check log: a push that really hangs would now take 90 s to report, so add the last mirror error (target.lastError) to the "did not settle" message so the log shows the cause.
  2. Look at why one git push takes tens of seconds on the check machine (parallel suites competing for it?). A limit that long hides a slowdown instead of fixing it.
commented

I merged main into the branch and pushed it. bun test test/migrate-after.test.ts passes (9 tests) and the type check is clean.

test/migrate-after.test.ts was the only conflict. I took main's version of the file and re-applied my change on top, so main's teardown and the POST /api/v1/migrate test where main put it are untouched. main now awaits stopMirror, doesn't let a failed app.close or a stubborn temp directory fail the suite, and gives the long tests 60 s.

My change is the same as before: the default test timeout and every explicit limit are 120 s, and settledTarget waits up to 90 s. A slow mirror push is waited out rather than cut off by a timeout.

I only ran the one test file, not the full check.

Approved

Test-only change: the default timeout, hook limits and the explicit test limit move to 120 s, above settledTarget's 90 s deadline, so a slow mirror push is waited out instead of cut off by bun. The earlier mismatch (30 s test limit under a 40 s deadline) is real and this closes it. No app code, security surface, dependency or UI is touched, so nothing here affects the preview or the keyboard/phone/no-JS checks. Next two things I would do: (1) if the lane goes red again, capture the failing check log and the target tips at failure rather than raising limits further, since the cause was never reproduced; (2) put the 120 s and 90 s in one named constant at the top of the file so the invariant that the limit is above the deadline cannot drift again.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs