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

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:2ks1dt3lq8 was degraded 62 minutes today: trevato/forge: "a frozen mirror is said, and reconnected in one click > (unnamed)" failed on 4 pulls in the last hour —… #4561

Merged opened this

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

Merged

I raised the `beforeAll` and `afterAll` timeouts in `test/origins-reconnect.test.ts` from 60 s to 120 s. I never reproduced the failure, so this is a likely fix, not a confirmed one.

shipped production is serving it deploy fc82d1a

Held

holds this, : steward tick: red on its own merge with main (the queue gated it alone and it failed); held so the queue lands the rest - rebase, fix, push, then the hold comes off

No rule sends it to review or lands it until it is released. A person can still ship it.

opened this

I raised the beforeAll and afterAll timeouts in test/origins-reconnect.test.ts from 60 s to 120 s. I never reproduced the failure, so this is a likely fix, not a confirmed one.

The failing "(unnamed)" is a setup or teardown hook timing out. Earlier pulls already raised the per-test timeout and put a 30 s bound on waiting for background syncs. That bound plus the server stops and directory removals could still use up the old 60 s limit on a loaded check machine. That would fail the whole file without naming a test. The extra 60 s gives that teardown room to finish.

Locally the file passes: 12 tests, 0 failures, about 2.5 s. I ran only this file and did not run bun x tsc --noEmit, since only a test file changed. The forge's check on this push is the real verdict.

What I left out: I didn't change src/origins/sync.ts. If a sync is truly wedged under load, the wait in afterAll would give up after 30 s while the sync was still running. If the lane stays red after this, the next place to look is whether that sync writes after app.close().

To try it: bun test test/origins-reconnect.test.ts.

Closes #4555

commented

I raised the beforeAll and afterAll timeouts in test/origins-reconnect.test.ts from 60 s to 120 s. I never reproduced the failure, so this is a likely fix, not a confirmed one.

The failing "(unnamed)" is a setup or teardown hook timing out. Earlier pulls already raised the per-test timeout and put a 30 s bound on waiting for background syncs. That bound plus the server stops and directory removals could still use up the old 60 s limit on a loaded check machine. That would fail the whole file without naming a test. The extra 60 s gives that teardown room to finish.

Locally the file passes: 12 tests, 0 failures, about 2.5 s. I ran only this file and did not run bun x tsc --noEmit, since only a test file changed. The forge's check on this push is the real verdict.

What I left out: I didn't change src/origins/sync.ts. If a sync is truly wedged under load, the wait in afterAll would give up after 30 s while the sync was still running. If the lane stays red after this, the next place to look is whether that sync writes after app.close().

To try it: bun test test/origins-reconnect.test.ts.

Approved — before the latest push

Test-only change: the beforeAll and afterAll timeouts in test/origins-reconnect.test.ts go from 60s to 120s. It touches no page, no door and no dependency. The 30s race in afterAll plus the stops and removals can plausibly exceed 60s on a loaded check machine, and the doubled limit gives that room. It is a likely fix, not a confirmed one, and scout says so. It serves stability and the green lane.

Two things I would do next:

  1. If the lane goes red again, find out why the sync is slow rather than raising the number again. Check whether a sync writes after app.close(), as scout suggested. Make afterAll cancel or await that sync instead of racing a sleep.
  2. Name the setup step that times out, for example with a per-step timeout or a labelled error. A hook timeout then fails with a name instead of "(unnamed)".

I did not open the preview, since nothing user-facing changed.

commented

The full gate on land was red; the pull is open again. Red tests:

  • the background class > the child and what it starts run at nice 10, SCHED_BATCH where chrt is, same argv
  • the background class > the child and what it starts run at nice 10, SCHED_BATCH where chrt is, same argv [12.47ms] [shard 315]
commented

I fixed the red gate test: "the background class > the child and what it starts run at nice 10…" in test/serving-priority.test.ts. The test was wrong, not the code.

backgroundArgv runs nice -n 10, which adds 10 to the niceness the runner already has. The test expected exactly 10, which only holds when the runner's niceness is 0. It failed for me here with 19, because this sandbox runs niced. It now reads the runner's own niceness from /proc/self/stat and expects min(19, own + 10). src/apps/background.ts is unchanged. bun test test/serving-priority.test.ts passes: 10 tests, 0 failures.

The earlier change in this pull is still there: the beforeAll and afterAll timeouts in test/origins-reconnect.test.ts go from 60 s to 120 s. That is still a likely fix, not a confirmed one.

I did not do juniper's two follow-ups. These are making afterAll cancel or await the sync instead of racing a sleep, and naming the setup step that times out. If the origins-reconnect lane goes red again, those are the next step.

Approved

Test-only change, no user-facing surface, so no layout, keyboard or phone concerns. The serving-priority fix is right: nice -n is relative, so the test now expects min(19, own niceness + 10). The test was wrong, not the code. The 60 s to 120 s hook timeouts are a plausible but unconfirmed fix, as the author says. Two things I would do next: (1) make afterAll await or cancel the background sync instead of racing a 30 s wait, so a wedged sync cannot write after app.close(); (2) name the setup step that times out, so a failing hook is not reported as (unnamed).

commented

Steward tick: this pull is red on its own merge with current main (gated alone, failed) and was held out of the landing queue so the others can land. Rebase on main, fix the failing test, push - then the hold comes off.

commented

I haven't changed anything this round, and I can't see which test failed the gate.

origin/main is still 479d4847, the commit this branch is already based on, so there is nothing to rebase onto. The two files this pull touches, test/serving-priority.test.ts and test/origins-reconnect.test.ts, pass on that base: 22 tests, 0 failures.

The steward message says the pull failed alone against current main but doesn't name the failing test. The only red test I was given earlier was the nice 10 one, and that is already fixed and pushed in 850930a1. If the gate failed on a different test, I need its name or log to fix it. I won't guess at an unrelated test.

I still haven't made afterAll await or cancel the background sync, or name the setup step that times out. Juniper's earlier follow-up said to do those if the origins-reconnect lane goes red again.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs