Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:2ks1dt3lq8 is past its budget: trevato/forge: "a frozen mirror is said, and reconnected in one click > (unnamed)" failed on 3 pulls in the last hour —… #4475
Check · Review · Land · Live done — Landed and live in production.
This change should stop the frozen-mirror suite failing, but I couldn't reproduce the failure here, so the cause is my best reading. The suite passed all 12 tests on every local run, three of them after the change. I haven't seen the forge's check log either, so whether this clears the sentinel is only known once the check runs on this push.
shipped production is serving it deploy 7189963
This change should stop the frozen-mirror suite failing, but I couldn't reproduce the failure here, so the cause is my best reading. The suite passed all 12 tests on every local run, three of them after the change. I haven't seen the forge's check log either, so whether this clears the sentinel is only known once the check runs on this push.
The failure is reported as "(unnamed)", which suggests a setup or teardown step rather than a named test. I think the cause is the background syncs that connecting an origin or reconnecting a mirror starts. They kept running while the next test disconnected the origin, and while afterAll closed the app and removed its data directories. A sync writing into a directory being deleted can make the removal fail, or fail a test, depending on how busy the machine is.
src/origins/sync.ts: newsyncsSettled()resolves once every origin sync currently running has finished, however it ended.test/origins-reconnect.test.ts: the suite awaitssyncsSettled()after each wait for refs, before the next disconnect, and at the start ofafterAll. Directory removal now also retries.
I added no new test. The change is to the existing suite's waits, so the existing 12 tests are what exercise it.
To try it, run bun test test/origins-reconnect.test.ts and bun x tsc --noEmit. Both pass here.
The direction is right: the sentinel is a flaky suite, and awaiting background syncs before teardown is a plausible fix. One thing misses the list. syncsSettled() is new exported behaviour in src/ and nothing tests it. The scout also did not reproduce the failure, so nothing shows it is the cause. Fix first: add a small test (for example in test/origins-sync) that starts a syncOrigin held open on a deferred, asserts syncsSettled() is still pending, releases it, and asserts it resolves, including when the sync rejects. It would fail without the function. Second: syncsSettled snapshots inFlight once, so a sync a reconnect starts after the call is not awaited. Either loop until inFlight is empty or say in the comment that it is a snapshot. I did not read the diff beyond the two files; this is a test-only reliability change with no page surface, so there is nothing in the preview to judge.
I've pushed both fixes to the branch. I still haven't reproduced the original failure, so the fix is a plausible cause rather than a confirmed one.
- Loop:
syncsSettled()insrc/origins/sync.tsnow loops whileinFlightis non-empty. A sync that starts while it waits, such as one a reconnect chains, is awaited too. - Tests: the new
test/origins-sync-settled.test.tsholds syncs open behind a gate, and its three cases cover the following.
- It stays pending while a sync is held, then resolves once the sync ends. - It resolves rather than rejects when the sync rejects. I force the rejection by closing the app while the sync is held. - It stays pending after a first sync ends if a second started meanwhile, and resolves only once that one ends too.
- Check that the tests bite: with the loop swapped back to a single
if, the third test fails. Without the function the file doesn't compile. - Test runs: the new tests and
origins-reconnectpass together (15 tests), andtsc --noEmitis clean. - Still unproven: the forge's check log is the only evidence that the sentinel clears.
Approved. syncsSettled() is small and correct: inFlight entries are removed in .finally, allSettled means a rejected sync cannot reject the waiter, and the loop catches syncs chained during the wait. The three new tests cover pending, rejection and chained-start, and the reconnect suite now waits before disconnect and teardown. No user-facing surface, no security or dependency change, within the stability priority. Two things I would do next: (1) read the forge check log on this push, since the cause of the sentinel failure is still a reading, not reproduced; if it recurs, capture which hook is unnamed. (2) In the new test afterEach, replace the empty try/catch around app.close() with an explicit guard so a real close failure is not swallowed.
This change should stop the frozen-mirror suite failing, but I couldn't reproduce the failure here, so the cause is my best reading. The suite passed all 12 tests on every local run, three of them after the change. I haven't seen the forge's check log either, so whether this clears the sentinel is only known once the check runs on this push.
The failure is reported as "(unnamed)", which suggests a setup or teardown step rather than a named test. I think the cause is the background syncs that connecting an origin or reconnecting a mirror starts. They kept running while the next test disconnected the origin, and while
afterAllclosed the app and removed its data directories. A sync writing into a directory being deleted can make the removal fail, or fail a test, depending on how busy the machine is.src/origins/sync.ts: newsyncsSettled()resolves once every origin sync currently running has finished, however it ended.test/origins-reconnect.test.ts: the suite awaitssyncsSettled()after each wait for refs, before the next disconnect, and at the start ofafterAll. Directory removal now also retries.I added no new test. The change is to the existing suite's waits, so the existing 12 tests are what exercise it.
To try it, run
bun test test/origins-reconnect.test.tsandbun x tsc --noEmit. Both pass here.Closes #4456