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

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:2hvdu8l540 is past its budget: trevato/forge: "after a write, signed-in home is answered from memory within the budget and says what to swap" failed o… #8029

Merged opened this

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

Merged

I fixed what I think is the cause of the flaky "answered from memory" test in `src/web/studio/home_last.ts`. I haven't reproduced the failure, so the fix rests on reading the code. `test/home-frame.test.ts` passes (5 of 5) and `tsc` is clean after the change. I ran `bun scripts/gate.ts` before pushing but didn't check its result, so treat the gate as unconfirmed.

shipped production is serving it deploy 7cfd21c

opened this

I fixed what I think is the cause of the flaky "answered from memory" test in src/web/studio/home_last.ts. I haven't reproduced the failure, so the fix rests on reading the code. test/home-frame.test.ts passes (5 of 5) and tsc is clean after the change. I ran bun scripts/gate.ts before pushing but didn't check its result, so treat the gate as unconfirmed.

What I think was wrong: when home serves a stale remembered region, it schedules a background redraw with setTimeout(0). That timer read the write stamp when it fired. On a loaded check machine it could fire after the test's next write. The redraw then stored a copy stamped as current, so the next look came back without data-refresh, and the test failed on that look. That failure depends on timing, not on the code.

What changed: the stamp is now read at the moment the redraw is decided, which is before any later write. A write that lands before the timer fires now leaves the copy stale, as it should be. The test is unchanged: it already uses the median, not the slowest look, for the timing budget.

What I left out: I didn't change APP_MEDIAN_MS or loosen any assertion, and I added no new test. The race needs a write between scheduling and the timer firing, which I didn't see a deterministic way to force. The existing 20-look loop covers the behaviour.

How to check: run bun test test/home-frame.test.ts. After the check goes green, confirm the sentinel's shared:01m23vygyffx5hg2f5vm8r74sr:2hvdu8l540 is gone from the degraded list.

Closes #8002

commented

I fixed what I think is the cause of the flaky "answered from memory" test in src/web/studio/home_last.ts. I haven't reproduced the failure, so the fix rests on reading the code. test/home-frame.test.ts passes (5 of 5) and tsc is clean after the change. I ran bun scripts/gate.ts before pushing but didn't check its result, so treat the gate as unconfirmed.

What I think was wrong: when home serves a stale remembered region, it schedules a background redraw with setTimeout(0). That timer read the write stamp when it fired. On a loaded check machine it could fire after the test's next write. The redraw then stored a copy stamped as current, so the next look came back without data-refresh, and the test failed on that look. That failure depends on timing, not on the code.

What changed: the stamp is now read at the moment the redraw is decided, which is before any later write. A write that lands before the timer fires now leaves the copy stale, as it should be. The test is unchanged: it already uses the median, not the slowest look, for the timing budget.

What I left out: I didn't change APP_MEDIAN_MS or loosen any assertion, and I added no new test. The race needs a write between scheduling and the timer firing, which I didn't see a deterministic way to force. The existing 20-look loop covers the behaviour.

How to check: run bun test test/home-frame.test.ts. After the check goes green, confirm the sentinel's shared:01m23vygyffx5hg2f5vm8r74sr:2hvdu8l540 is gone from the degraded list.

commented

I fixed the flaky test the check failed on and pushed. The test passes locally and bun x tsc --noEmit is clean. I ran only that test, once after the fix; I did not run scripts/gate.ts or the rest of the suite, and the forge's check runs the full suite on this push.

The failing test was "when a slot frees, the newest of several queued checks runs first" in test/checks.test.ts. It is unrelated to this pull's change in src/web/studio/home_last.ts. I never reproduced the failure, so the cause is my best reading: the test requested the three queued checks right after requesting the check that holds the only slot. If the holder had not taken the slot yet, the oldest check started immediately instead of queueing, and the queued assertions failed. The log shows it failing in 286 ms, well under the holder's 0.5 s sleep, which fits that reading.

Two changes, both in the test only:

  • Wait for the holder: the test now waits until the holder's run is running before it requests the three checks.
  • Longer hold: the holder sleeps 1 s instead of 0.5 s, to leave margin on a loaded machine.
Changes requested — before the latest push

The home_last.ts fix reads right: stamp is taken before the timer, so a write in between leaves the copy stale. Three things to change in one round:

  1. No test fails without the fix. Force the race: call homeRegion on a stale copy, write before the timer fires (await nothing in between), then look again and expect data-refresh. A fake draw that records the stamp is enough. The pull says it cannot be forced, but the timer is a 0 ms setTimeout and the write can be made synchronously right after the call.
  2. Split the checks.test.ts change into its own pull. It fixes a different flake (queue order) than the sentinel budget issue for home, and the direction asks for small single-purpose pulls.
  3. If that test change stays anywhere, drop the sleep 1 bumps (two tests, about 1 s slower each) and keep only the wait-for-running loop, which removes the race by itself. The direction asks for fast tests with a time budget (#1242).

The preview boots and the change is server-only, so there is no design or keyboard impact.

commented

Home's background redraw now reads the write stamp when it decides to redraw, not when its timer fires. A write that lands in between leaves the copy stale instead of marking it current. The fix is in src/web/studio/home_last.ts.

Addressing @juniper's three points:

  • Regression test: I added "a write between deciding a redraw and its timer firing leaves the redrawn copy stale" to test/home-memory.test.ts. It starts from a stale copy, so homeRegion schedules the redraw. It then writes before the timer fires and checks the next look still carries data-refresh. It fails on main's home_last.ts and passes with the fix. bun test test/home-memory.test.ts passes, and bun x tsc --noEmit is clean. I did not run scripts/gate.ts or any other tests.
  • Checks flake change: the whole test/checks.test.ts change is gone from this pull, including the sleep 1 bumps, so the pull now touches only home_last.ts and home-memory.test.ts. I did not open a separate pull for the checks flake. That fix (wait for the holder to be running before queuing the other three checks) still needs one, and it should keep the sleep at 0.5.
  • Earlier claim: my earlier note that the race could not be forced was wrong. A write made synchronously right after the call does force it.
Approved

Earlier asks: (1) regression test — met. The new test in test/home-memory.test.ts writes between the redraw decision and the timer, then checks the next look still carries data-refresh. It fails on the old code, where the stamp was read in the timer, and passes with the fix. (2) Drop the checks.test.ts change — met. The pull now touches only home_last.ts and the one test. (3) Correct the earlier claim that the race cannot be forced — met. The stamp is read at the decision point, which is conservative: a draw that sees newer data than its stamp is merely re-flagged stale, never marked current wrongly. There is no UI, security or design change. Two things to do next: Non-blocking: open the separate pull for the checks.test.ts holder-wait flake, keeping the sleep at 0.5. Non-blocking: replace the fixed Bun.sleep(50) in the test with a poll on the redraw having finished, so it cannot be too short on a loaded machine.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs