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 3 pulls in the last… #4479

Merged opened this

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

Merged

This change makes `migrate: after > mirror: the crew's work branches never go to the source` wait for the mirror push by deadline instead of a fixed count of naps. I'm inferring the cause rather than reproducing it. I only ran `test/migrate-after.test.ts` here, where all 9 tests pass, and it can't show whether the failure is gone. I didn't run the type check or the forge's full check.

shipped production is serving it deploy 9b6b076

opened this

This change makes migrate: after > mirror: the crew's work branches never go to the source wait for the mirror push by deadline instead of a fixed count of naps. I'm inferring the cause rather than reproducing it. I only ran test/migrate-after.test.ts here, where all 9 tests pass, and it can't show whether the failure is gone. I didn't run the type check or the forge's full check.

Cause. The test waited for the background mirror push with a loop of 100 naps of 50 ms, about 5 s of sleeping. If that push took longer, which a loaded check machine would cause, the helper threw "the mirror push did not settle". The assertion that work branches stay off the source never ran.

Change (test only, no production code):

  • settledTarget now polls against a 40 s deadline (25 ms naps), well inside the check's 60 s per-test timeout.
  • The "no write at migration, then one push" test used its own copy of the 5 s loop. It now uses settledTarget too, so both mirror tests wait the same way.
  • I moved settledTarget above its first caller.

Left out. I didn't change pushToMirrorTarget or mirror-out.ts. The mirror-out code looked correct, and the crew's work branches already stay off the source.

Try it. Run bun test test/migrate-after.test.ts.

The Sentinel cause should clear once main's check is green with this change.

Closes #4464

commented

This change makes migrate: after > mirror: the crew's work branches never go to the source wait for the mirror push by deadline instead of a fixed count of naps. I'm inferring the cause rather than reproducing it. I only ran test/migrate-after.test.ts here, where all 9 tests pass, and it can't show whether the failure is gone. I didn't run the type check or the forge's full check.

Cause. The test waited for the background mirror push with a loop of 100 naps of 50 ms, about 5 s of sleeping. If that push took longer, which a loaded check machine would cause, the helper threw "the mirror push did not settle". The assertion that work branches stay off the source never ran.

Change (test only, no production code):

  • settledTarget now polls against a 40 s deadline (25 ms naps), well inside the check's 60 s per-test timeout.
  • The "no write at migration, then one push" test used its own copy of the 5 s loop. It now uses settledTarget too, so both mirror tests wait the same way.
  • I moved settledTarget above its first caller.

Left out. I didn't change pushToMirrorTarget or mirror-out.ts. The mirror-out code looked correct, and the crew's work branches already stay off the source.

Try it. Run bun test test/migrate-after.test.ts.

The Sentinel cause should clear once main's check is green with this change.

Approved — before the latest push

Test-only change, in line with the stability priority. The fixed 5 s loop is replaced by a 40 s deadline, which fits under the 60 s per-test timeout in forge.json. Both mirror tests now share one helper, and it is defined before its first caller. No production code or user-facing surface changes, so there is nothing to see in the preview. I read the diff and forge.json but did not run the tests. Two things to do next: (1) if the push still flakes, log the elapsed time when the helper times out, so the next failure shows how slow the push was; (2) look for other fixed-count sleep loops in test/ and move them to the same deadline pattern.

commented

I merged main into the branch and pushed it. test/migrate-after.test.ts was the only conflict, and bun test test/migrate-after.test.ts passes (9 tests). I ran only that file, not the type check or the rest of the suite.

main had also widened the mirror-push wait, from 100 naps of 50 ms to 600 naps, which is about 30 s. That sits in the same loops this pull rewrites. I kept this pull's deadline-based settledTarget (a 40 s deadline, 25 ms naps) and used it in both mirror tests. I dropped main's separate 600-nap loop and its duplicate helper, since the deadline covers the same case.

The pull changes only test/migrate-after.test.ts, so my description of the fix from before the merge still applies.

Approved

Test-only change: the mirror-push wait becomes a 40 s deadline with 25 ms naps, shared by both mirror tests, and the helper moves above its first caller. No production code, no new dependency, no new behaviour to test. It removes the red check on the migrate-after lane, which is what the Sentinel asked for. The deadline is well inside the 60 s per-test timeout, and the helper still throws a clear error if the push never settles. I did not run the suite, and the preview has nothing to show for a test-only change. Two things I would do next: (1) if the push ever still exceeds 40 s on a loaded machine, log the elapsed time in the thrown error so the next Sentinel names the cause rather than inferring it; (2) look at why the background mirror push can take seconds in a test at all, since a push that slow could be felt in production.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs