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

Red after landing: 1b5f2d4 broke test/timers-named.test.ts #11993

Merged opened this in answer to

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

Merged

This fixes the four red tests. They weren't all caused by 1b5f2d4: its diff touches none of the flagged files. The failures came from four separate landings, so I fixed each one where it came from. `bun scripts/gate.ts` and `bun x tsc --noEmit` pass. The four tests named in the issue pass, as do `runs-log-buffered` and `spawn-stopped`, which cover the files I edited. I did not run the full suite;…

shipped production is serving it deploy d20ce1b

opened this

This fixes the four red tests. They weren't all caused by 1b5f2d4: its diff touches none of the flagged files. The failures came from four separate landings, so I fixed each one where it came from. bun scripts/gate.ts and bun x tsc --noEmit pass. The four tests named in the issue pass, as do runs-log-buffered and spawn-stopped, which cover the files I edited. I did not run the full suite; the forge's check does that on this push.

What changed

  • test/timers-named.test.ts: src/runs/log.ts (the 6032969 landing) and src/spawn/stopped.ts (the #11976 landing) had timers that ran outside during. Both are now wrapped, as during("run-log-flush", …) and during("stopped-restart", …).
  • test/flow-clock.test.ts: src/flow/pipeline.ts read Date.now() in the body. The two reads now go through a small wall(now = Date.now()) helper, so the clock is a parameter default as the test requires.
  • test/unused-exports.test.ts: mergedPullClosingScan, pinCrawlLag and bufferRunLogs are exported for tests only. I added them to scripts/unused-exports-allow.json rather than un-exporting them.
  • test/stall-scans.test.ts: the test was wrong, so I changed it. The assigned-sweep query now probes run through the covering index run_agent_issue_latest, which is still an index probe, not a scan. The test only accepted run_issue, so I widened its regex.

What I left out

  • I did not revert 1b5f2d4. Its own change (the three run indexes and the terminal refresh throttle) is not what broke these tests.
  • I did not rework the wall helper beyond what flow-clock needs.

To try it: run bun test test/timers-named.test.ts test/stall-scans.test.ts test/unused-exports.test.ts test/flow-clock.test.ts.

🤖 Generated with Claude Code

Closes #11988

commented

This fixes the four red tests. They weren't all caused by 1b5f2d4: its diff touches none of the flagged files. The failures came from four separate landings, so I fixed each one where it came from. bun scripts/gate.ts and bun x tsc --noEmit pass. The four tests named in the issue pass, as do runs-log-buffered and spawn-stopped, which cover the files I edited. I did not run the full suite; the forge's check does that on this push.

What changed

  • test/timers-named.test.ts: src/runs/log.ts (the 6032969 landing) and src/spawn/stopped.ts (the #11976 landing) had timers that ran outside during. Both are now wrapped, as during("run-log-flush", …) and during("stopped-restart", …).
  • test/flow-clock.test.ts: src/flow/pipeline.ts read Date.now() in the body. The two reads now go through a small wall(now = Date.now()) helper, so the clock is a parameter default as the test requires.
  • test/unused-exports.test.ts: mergedPullClosingScan, pinCrawlLag and bufferRunLogs are exported for tests only. I added them to scripts/unused-exports-allow.json rather than un-exporting them.
  • test/stall-scans.test.ts: the test was wrong, so I changed it. The assigned-sweep query now probes run through the covering index run_agent_issue_latest, which is still an index probe, not a scan. The test only accepted run_issue, so I widened its regex.

What I left out

  • I did not revert 1b5f2d4. Its own change (the three run indexes and the terminal refresh throttle) is not what broke these tests.
  • I did not rework the wall helper beyond what flow-clock needs.

To try it: run bun test test/timers-named.test.ts test/stall-scans.test.ts test/unused-exports.test.ts test/flow-clock.test.ts.

🤖 Generated with Claude Code

commented

The deploy for #11993 did not fail on my change, so I pushed nothing new.

The log you quoted ends with ready, serving and done at https://lab--11993--forge-trevato-apps.open-platform.sh, and the app answered 1.5 s after it started. "No tests declared, none run" only says the preview deploy runs no tests. forge.json has no test entry under app and I did not touch it, so the same note applies to every preview of this repo. The tests run in the check section instead, on every push to the pull (fast, run and verify).

The WARN about two unknown migrations (20261114000000-estimate-done, 20261114000100-release-how) comes from the preview copying production data that is already ahead of this branch's base. It left them as they are and the app booted, so it is not a failure either.

Before I pushed, bun scripts/gate.ts, bun x tsc --noEmit and the six test files that cover my change passed locally. The forge's own check on the push is what decides the landing, and I have no failing log from it to act on.

If the forge shows this preview as failed, the status line is probably misreading that "no tests declared" note. I did not add a dummy test command to forge.json to silence it, because that would change how the app is run to suit a message. Say so if you want it added.

commented

I merged main into the branch and pushed it; the earlier gate message was from before the rewrap.

  • Conflict: the only one was test/stall-scans.test.ts. main had widened the assigned-sweep plan regex to accept the covering index run_agent_issue_latest, the same fix I had made. I kept main's version of that line.
  • Gate: the merge exposed one new failure, my comment above the wall helper in src/flow/pipeline.ts was over 100 columns. I rewrapped it and committed that separately.
  • Checks: after the rewrap, bun scripts/gate.ts is clean and bun x tsc --noEmit is clean. test/flow-clock.test.ts passes after the rewrap. Before it, the four tests named in the issue (timers-named, stall-scans, unused-exports, flow-clock) passed on the merge. I did not re-run timers-named, stall-scans and unused-exports after the rewrap. The forge's check runs the full suite on this push.

The branch is at 06c04526b. The pull's content is unchanged:

  • The two timers in src/runs/log.ts and src/spawn/stopped.ts now run inside during.
  • The src/flow/pipeline.ts clock reads go through a default parameter.
  • Three test-only exports are allowed in scripts/unused-exports-allow.json.

🤖 Generated with Claude Code

commented

I merged main into this branch and pushed it. The only conflict was in src/flow/pipeline.ts, and the branch merges cleanly again.

What changed:

  • Conflict: main made waitingTally take the instant as a now parameter, read from the callers' now.getTime(). My side read the clock inside the function through wall(), in two places. I kept main's version.
  • Removed helper: wall() was left with no callers, so I deleted it along with its comment. The "one clock read" intent now sits with the callers, as on main.

Checks run before the push:

  • bun x tsc --noEmit and bun scripts/gate.ts were clean.
  • Six pipeline and stall test files pass (32 tests, 0 fail): flow-waiting, flow-waiting-offthread, flow-pipeline, flow-clock, stall-scans and flow-approved-wait.

What I didn't run:

  • scripts/fast.ts exited 2 because the merge changed scripts/unused-exports-allow.json, so it could not name the covering tests. I chose those six files myself, and the forge's full check runs on this push.
  • A first attempt passed the error text to bun test and started a broad run. I stopped it and ran only the six files above.
  • Sandbox denials on outbound hosts like *.forge.test appeared in that stopped run, not in the six-file run.
commented

The check failed because src/runs/log.ts imported during twice, which is the TS2300 error from the check log. I deleted the second import and pushed.

The duplicate came from the earlier merge of main: the same import was added on both sides. Resolving that merge's conflicts in src/runs/log.ts and src/spawn/stopped.ts kept main's wording: void during(...) and the spawn-restart timer name.

bun x tsc --noEmit and bun scripts/gate.ts are clean, and main hasn't moved since. These five test files pass (30 tests, 0 fail): timers-named, spawn-stopped, run-log-audience, events-run-log and unused-exports. The forge's fast-tier check runs on this push.

Approved

Approved. The diff is one import moved in src/runs/log.ts, with no behaviour change and no UI, so the preview and screenshots are unaffected. At this head test/timers-named.test.ts passes (8 tests), and the run-log flush timer is wrapped in during("run-log-flush"). Nothing here touches security, the terminal, docs or dependencies. Two things I would do next: 1) Move the import back below the node: imports, or keep it where it is on purpose. The change only reorders it, and a reorder alone should not be a pull. 2) Make the name-every-timer rule fail at the push that adds the timer, for example by running timers-named in the fast tier, so a main that lands red like this one is caught earlier.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs