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

The tty registry, the cell watcher and the app runtime run their timers inside `during`, and a test finds any server timer outside it #11175

Merged opened this

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

Merged

The four timers named in #11167 now run inside `during`. If one of them stalls the loop, the stall blame names it instead of `unknown`. The timer test now names the file and line of any timer that isn't wrapped.

shipped production is serving it deploy 7bd4f86

opened this

The four timers named in #11167 now run inside during. If one of them stalls the loop, the stall blame names it instead of unknown. The timer test now names the file and line of any timer that isn't wrapped.

  • Wrapped timers:

- src/apps/tty_registry.ts: the app terminal's idle check runs as app-tty-idle. - src/apps/contain.ts: the namespace probe deadline runs as cell-probe-deadline, and the delayed kill of the tree's root runs as cell-kill-root. - src/apps/runtime.ts: the restart after backoff runs as apps-restart. Its if (this.stopped) return guard moved inside the wrapper.

No timing, ordering or return values change.

  • test/timers-named.test.ts: the test from #11127 already did the scan this issue asks for, so I extended it rather than adding a second one.

- All four timers are off its allowlist. - A failure prints file:line: call(callback…. - A new case checks that each of the four callbacks is wrapped under its name. - The scan of src/ runs once per test run instead of once per test, which brings the file from about 5.5 s to about 3 s.

I unwrapped the tty timer by hand to check: two tests went red, one printing src/apps/tty_registry.ts:79. I then restored the file.

I left the test as a TypeScript syntax parse rather than switching it to a plain text scan. It still loads no app modules, and the syntax parse is what stops a timer inside a template string (the toolbar's browser script, a scaffolded app) from counting as a timer. It does not meet the issue's one-second target: the scan alone takes over a second. Outside the files #11117 named, the test still checks only setInterval and not setTimeout. Those are one-shot deadlines and delays, and wrapping each would add one job-watch entry per request. The allowlist keeps the per-connection timers (SSE heartbeat, talk poll, shell idle, git progress, capped fetch) and the toolbar's page deadline, each with its reason.

Checks: bun x tsc --noEmit passes, and so do test/timers-named.test.ts, test/app-tty-sessions.test.ts, test/contain.test.ts, test/tty-stop-tree.test.ts and test/app-runtime-root.test.ts. The sandbox skipped the 30 cell and unix-socket tests in those files; the forge's check runs them. scripts/gate.ts printed nothing, so its result isn't confirmed.

To try it, run bun test test/timers-named.test.ts, then remove one of the four during(…) wrappers and run it again; it fails and names that file and line.

🤖 Generated with Claude Code

Closes #11167

commented

The four timers named in #11167 now run inside during. If one of them stalls the loop, the stall blame names it instead of unknown. The timer test now names the file and line of any timer that isn't wrapped.

  • Wrapped timers:

- src/apps/tty_registry.ts: the app terminal's idle check runs as app-tty-idle. - src/apps/contain.ts: the namespace probe deadline runs as cell-probe-deadline, and the delayed kill of the tree's root runs as cell-kill-root. - src/apps/runtime.ts: the restart after backoff runs as apps-restart. Its if (this.stopped) return guard moved inside the wrapper.

No timing, ordering or return values change.

  • test/timers-named.test.ts: the test from #11127 already did the scan this issue asks for, so I extended it rather than adding a second one.

- All four timers are off its allowlist. - A failure prints file:line: call(callback…. - A new case checks that each of the four callbacks is wrapped under its name. - The scan of src/ runs once per test run instead of once per test, which brings the file from about 5.5 s to about 3 s.

I unwrapped the tty timer by hand to check: two tests went red, one printing src/apps/tty_registry.ts:79. I then restored the file.

I left the test as a TypeScript syntax parse rather than switching it to a plain text scan. It still loads no app modules, and the syntax parse is what stops a timer inside a template string (the toolbar's browser script, a scaffolded app) from counting as a timer. It does not meet the issue's one-second target: the scan alone takes over a second. Outside the files #11117 named, the test still checks only setInterval and not setTimeout. Those are one-shot deadlines and delays, and wrapping each would add one job-watch entry per request. The allowlist keeps the per-connection timers (SSE heartbeat, talk poll, shell idle, git progress, capped fetch) and the toolbar's page deadline, each with its reason.

Checks: bun x tsc --noEmit passes, and so do test/timers-named.test.ts, test/app-tty-sessions.test.ts, test/contain.test.ts, test/tty-stop-tree.test.ts and test/app-runtime-root.test.ts. The sandbox skipped the 30 cell and unix-socket tests in those files; the forge's check runs them. scripts/gate.ts printed nothing, so its result isn't confirmed.

To try it, run bun test test/timers-named.test.ts, then remove one of the four during(…) wrappers and run it again; it fails and names that file and line.

🤖 Generated with Claude Code

commented

The preview failure didn't come from this change, so I made no code change and pushed nothing. The pull below is as it was pushed.

Why I think the deploy failure isn't from this change:

  • The preview at https://lab--11175--forge-trevato-apps.open-platform.sh is up. /healthz answers ok, / answers 200, and every path in forge.json's looks answers 200 or 303 in under 1.6 s.
  • The deploy log tail you sent is routine INFO lines (the SQL-by-caller summaries and the process count) with no error.
  • The status line itself reads "running at … no tests declared, none run", which sounds like a deploy that came up rather than one that failed.
  • The pull's check is green at head 98ee018ea.
  • Booted locally on a loopback port, the app answers /healthz 200 and / 200 almost at once, well inside the 90 s ready limit, and logs no errors.

The forge's API has no route for the deploy record, so I couldn't read why it was marked failed. Re-running the preview, or looking at that record on the forge's side, should settle it. I didn't push an empty commit just to trigger a redeploy.

---

The four timers named in #11167 now run inside during, so a loop stall in any of them is blamed on a name instead of unknown:

  • src/apps/tty_registry.ts: the app terminal's idle check runs as app-tty-idle.
  • src/apps/contain.ts: the sandbox probe deadline runs as cell-probe-deadline, and the delayed kill of a process tree's root runs as cell-kill-root.
  • src/apps/runtime.ts: the restart after backoff runs as apps-restart.

Timing, ordering and return values are unchanged.

The test. The scan already existed in test/timers-named.test.ts (from #11127), with these four timers on its allowlist. I extended it instead of adding a second test:

  • The four entries are off the allowlist.
  • A failure now prints file:line: call(callback….
  • A new case checks that each of the four callbacks is wrapped under its name.
  • The scan of src/ runs once per test run instead of once per test, which brings the file from about 5.5 s to about 3 s.

When I removed the during from the tty timer by hand, two tests failed, one naming src/apps/tty_registry.ts:79.

Not done:

  • The test still parses each file with the TypeScript parser rather than scanning plain text. It loads no app modules, and the parse is what keeps a timer written inside a page script or app template from counting. The scan alone still takes over a second, so the issue's one-second target isn't met.
  • Outside the files #11117 names, setTimeout is still not checked: those are one-shot deadlines and delays, and wrapping each would add a job-watch entry per request.
  • The per-connection timers (SSE heartbeat, talk poll, shell idle check, git fetch stall check, capped-fetch size check) and the toolbar's page deadline stay on the allowlist, each with its reason.

Tested: bun x tsc --noEmit passes, and so do test/timers-named.test.ts, test/app-tty-sessions.test.ts, test/contain.test.ts, test/tty-stop-tree.test.ts and test/app-runtime-root.test.ts. 30 tests in those files need unix sockets or cells and were skipped here; the forge's check runs them. scripts/gate.ts printed nothing when run.

To try it: run bun test test/timers-named.test.ts, then remove one of the four during(…) wrappers and run it again; it fails and names that file and line.

🤖 Generated with Claude Code

commented

The app's tty idle check, the cell start-probe deadline, the cell root kill and the app restart now each run inside during, so the job-slices table measures them. The timer scan in test/timers-named.test.ts now names the file and line of each timer it flags.

  • src/apps/tty_registry.ts: the idle check runs as app-tty-idle.
  • src/apps/contain.ts: the probe deadline runs as cell-probe-deadline, and the kill of a cell's root process after its grace period runs as cell-kill-root.
  • src/apps/runtime.ts: the delayed redeploy after an app exits runs as apps-restart.
  • test/job-slices.test.ts: these four names are in UNTIMED, each with its reason. The test fixture has no apps, cells or app terminals, so none of them can be timed there. The last check failed because the names were missing from that list.

The previous check also failed "a scaffolded app signs a person in through the forge > OpenID Connect, code flow with PKCE". That test starts its own scaffolded server and does not import any of the files this pull changes. It passed in each of three runs here, so it looks like a load flake on the check machine. I did not change it.

The gate, bun x tsc --noEmit, test/job-slices.test.ts, test/timers-named.test.ts and test/scaffold.test.ts pass locally. To try it: bun test test/job-slices.test.ts test/timers-named.test.ts.

🤖 Generated with Claude Code

Changes requested — before the latest push

The four timers are wrapped correctly (app-tty-idle, cell-probe-deadline, cell-kill-root, apps-restart), job-slices UNTIMED is updated, and the test reports file:line. No UI is touched, so the preview shows nothing new, and nothing is left on the phone or keyboard side. One miss against the issue, so I am requesting changes.

Fix first: box 2 asks for a test that fails on any setInterval/setTimeout under src/ whose callback is not wrapped in during. In test/timers-named.test.ts scan() checks setTimeout only in the NAMED files and setInterval everywhere else. A new unwrapped setTimeout in any other server file passes silently. The pull says it left this out because wrapping each one would add a job-watch entry per request, but the issue did not ask to wrap them all. It asked for a scan that finds them, with a short named allowlist, each entry with its reason, for the one-shot timers that are deliberately not wrapped. Scan setTimeout in every file, move the existing one-shot setTimeouts (including the toolbar deadline) onto ALLOWED with reasons, and keep the AST parse so template-string code (toolbar client script, scaffold templates) is not counted. If the allowlist then grows too long to be short, list the genuine reason for each and say so in the pull rather than skipping the call.

commented

The timer scan in test/timers-named.test.ts now fails on any setInterval or setTimeout anywhere in server code whose callback doesn't run inside during. It no longer limits setTimeout to the files #11117 named. It still parses the code, so timers inside template strings (the toolbar's browser script, scaffold templates) are not counted. A failure names file:line.

Earlier in this pull: the tty idle check, the cell probe deadline, the cell root kill and the app restart run inside during. Their four job names are in UNTIMED in test/job-slices.test.ts, which is what the last check failed on.

Reading every file turned up 91 unwrapped setTimeouts. Some were real background work, so I wrapped those rather than allowlist them:

  • Loops: bucket-watch (snapshot lanes), routines (the routine runner), early-watch (a run's early look), release-prove.
  • Deferred jobs, at most one per burst or key: route-timing-flush, usage-count-flush, access-flush, egress-refused-flush, nightly-output-flush, home-refresh, home-last, disk-walk.

Each new job name is in UNTIMED in test/job-slices.test.ts with its reason.

The scan now also accepts a counter bump (looks++) and a set forgetting a timer (timers.delete(timer)) as bookkeeping next to a during call. Without that, the existing probes and nightly timers counted as unwrapped.

The allowlist is long, about 80 entries, so it is not short as the issue asked. Almost all are one-shots of three kinds, each with one shared reason:

  • a sleep that only resolves a promise the caller awaits (24),
  • a deadline that settles, aborts or stops one call in flight, including the toolbar's (24),
  • a kill timer that signals one child process (16).

None of these does work on the event loop, so a span around each would only fill the 256-span ring. The rest have their own reasons:

  • Timers that exist once per open connection: SSE, talk, device-flow streams, ssh auth, shell resize, git idle deadlines.
  • The loop-lag meter itself: wrapping it would blame its own lateness on itself.
  • The flow cache's per-slice scheduler: one span per slice would fill the ring in a single refresh.
  • The prover's scheduler seam: its caller already wraps the callback.
  • probes' track(): the round it holds already runs inside during.
  • Three tiny clears: the perf capture drop, the image loader's flag, and the cgroup rmdir retry.

The per-entry check now requires each entry to match exactly as many timers as it is listed, so the two identical flow-stream polls each need their own line.

Two places to look:

  • src/web/studio/home_cache.ts and src/web/studio/home_last.ts only gain the during wrap; the design lint is clean.
  • The push hook asked for a "how to try it" commit because those files are under src/web/, so there is an empty Try: commit.

The type check, the gate, both timer test files, and the tests for each module I touched (about 230) pass locally; I didn't run the full suite. scripts/fast.ts named 2226 files for this diff, so I picked those tests by hand.

Try it: run bun test test/timers-named.test.ts test/job-slices.test.ts, then add setTimeout(() => f(), 1) to any server file and run it again. The scan names that line.

🤖 Generated with Claude Code

Approved

The four timers run inside during under stable names, and during returns fn's value and rethrows, so the try/catch and .finally chains keep their behaviour. The scan now reads setTimeout in every server file and names file:line, which closes the gap Juniper raised. The pull also wraps 11 more real background timers, with their names in UNTIMED, and the allowlist grew to about 80 entries, which is not the short list the issue asked for. Every entry has a shared reason and an exact-count check, so I accept it. Not blocking: the release-prove check greps the file's text with toContain, and short starts keys like "r" and "fn" are fragile. I found no security finding. No UI changed.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs