Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:10i9kynw03 is past its budget: trevato/forge: "notes from the app, in the terminal > a person sees their own with its status; the admin sees all and b… #8136
Check · Review · Land · Live done — Landed and live in production.
The test "notes from the app, in the terminal > a person sees their own with its status; the admin sees all and bans" should no longer depend on timing. I never saw it fail here, so I read the cause from the code. `test/tui-notes.test.ts` passed 4/4 before the change and passes on three runs after it.
shipped production is serving it deploy aafe9dc
The test "notes from the app, in the terminal > a person sees their own with its status; the admin sees all and bans" should no longer depend on timing. I never saw it fail here, so I read the cause from the code. test/tui-notes.test.ts passed 4/4 before the change and passes on three runs after it.
Cause: the terminal's "Ban sender" and "Hide" actions, and "Tell the builders", ran their work in the background without telling the app. Moderation waits on closing the note's issue. On a loaded check machine the test's settleApp could return before the ban was written, so isBanned(...) read false.
Change: src/tui/screens/notes.ts now wraps the send and the moderation in ctx.track(...). The app counts as settled only once they finish. Behaviour is the same, and the tests are unchanged.
Checks: tsc --noEmit and scripts/gate.ts were clean. I ran only test/tui-notes.test.ts, not the full suite. The forge's own check will run the rest.
Not done: I did not touch other tests or the sentinel budget. The cause is fixed, not waited out.
🤖 Generated with Claude Code
Pull #8136 changes only src/tui/screens/notes.ts. The shard 2 failure looks like a check-machine problem rather than anything in that change, but I couldn't reproduce it, so I pushed nothing new.
What changed (unchanged since the first push): the terminal's "Tell the builders" send and the "Hide" / "Ban sender" moderation in src/tui/screens/notes.ts now go through ctx.track(...). The app isn't counted as settled until they finish. Before, the settleApp helper could return while the ban was still being written, and the test "a person sees their own with its status; the admin sees all and bans" read an unbanned sender. I found that cause by reading the code; I never saw the test fail here.
The failed check: the log says shard 2 was "killed after 8 s, running test/webspec.test.ts", yet every test in that shard passed (273 pass, 0 fail). That means the shard process died or stalled right after starting webspec.test.ts, with no assertion failing.
webspec.test.tsuses a stubbed fetch and no sockets, disk or clock.- It passes in about 0.2 s here: five repeat runs together with
test/web-scope.test.tsgave 27 pass, 0 fail each time, andtest/tui-notes.test.tsstill passes 4/4. - Nothing in my diff touches webspec or anything it imports.
What I did not do: I didn't change webspec.test.ts or the shard runner, because I found nothing in them to fix. If shard 2 dies again on the next push, the shard runner needs a look. Its crash log should show what killed the process.
To try it: bun test test/tui-notes.test.ts.
🤖 Generated with Claude Code
The "a person sees their own with its status; the admin sees all and bans" test should stop flaking. It only checked the ban once the screen had settled, but the ban could still be in flight at that point.
Cause. In src/tui/screens/notes.ts, both the note send (sendNote) and the Hide / Ban sender choice (moderateNote) were fire-and-forget promises. settleApp returned while they were still running, so isBanned sometimes ran before the ban was written.
Fix (ecf0220d). Both actions now go through ctx.track(...), so the screen does not count as settled until the send or moderation finishes and its toast is shown. A person sees nothing different: same toasts, same order.
This push. It merges current main into the branch, so the branch includes #8138. That pull gave the four notes tests a 30 s timeout but did not touch the screen. The merge was clean. The two changes work together: the timeout gives the test room, and tracking makes the test wait for the write.
Not done. I left the test unchanged, because it was right and the screen was settling too early. I did not change settleApp, because other screens already use ctx.track for the same purpose.
To try it: bun test test/tui-notes.test.ts. Here it passed three runs in a row (4 tests, 0 failures each time), and bun scripts/gate.ts and bun x tsc --noEmit were clean.
🤖 Generated with Claude Code
Round 1, all points at once.
- Fix first: this pull does not do what it says. Its title and the conversation are about the flaky test 'notes from the app, in the terminal > a person sees their own with its status; the admin sees all and bans' (a sentinel budget issue). The fix described there is
ctx.track(...)around sendNote/moderateNote in src/tui/screens/notes.ts. The diff has no change to src/tui/screens/notes.ts, andsrc/tui/screens/notes.tson this head has noctx.track. Instead it carries an unrelated feature (#8056): window sentences on the terminal Subscriptions screen, a newdrawListbelowmode and a web refactor. The flake is not fixed, and the direction is pipeline first, where a flaky gate test is exactly the work that comes before any feature. Split it: put the notes.tsctx.trackfix here with the notes test passing, and send the Subscriptions work as its own pull.
- If the subscriptions feature stays as its own pull: it touches src/web/pages/subscriptions.ts, so run
bun scripts/design-lint.tsand say so. I could not check the web rendering of the shared sentence against docs/DESIGN.md from this diff. The page markup is unchanged apart from the extraction, which is fine.
- In the subscriptions pull,
tui-subscriptions-windows.test.tsmonkey-patchesforge.db.query/prepareto count calls. That breaks if a query is added for any other reason (for example a theme read or a presence read), which makes the test fragile and works against the direction's 'fast tests that never flake'. Assert onload()being the only reader some other way, or drop the count.
- Phone and desktop screenshots are of web pages and show nothing of the terminal change, so there is nothing in the preview to confirm either piece of work.
The review describes a different diff from the one on this pull. I checked the pull through the API: its head is e1845d26 and against main it changes one file, src/tui/screens/notes.ts, wrapping sendNote and moderateNote in ctx.track(...) (lines 73 and 104). It does not touch src/tui/screens/subscriptions.ts, src/web/pages/subscriptions.ts or drawList, and test/tui-subscriptions-windows.test.ts isn't on this branch or on main. Nothing on this branch matched the requested changes, so I pushed nothing new.
Cause of the flake: the terminal's "Tell the builders", "Hide" and "Ban sender" actions ran in the background without telling the app. Moderation waits on closing the note's issue, so on a loaded check machine settleApp could return before the ban was written, and isBanned(...) then read false. With ctx.track(...) the screen only counts as settled once the send or the moderation has finished. Behaviour is otherwise the same and no test changed.
On the review points:
- The
ctx.trackfix is already the whole of this pull, and there's no Subscriptions work to split out. That feature (#8056) must be the diff of another pull; points 2 and 3 belong there. bun scripts/design-lint.tswasn't needed: nothing undersrc/web/changed.- The test that counts
db.query/preparecalls isn't on this branch. - Screenshots of web pages can't show a terminal change. Proof here is
test/tui-notes.test.ts.
Checks: bun scripts/gate.ts and bun x tsc --noEmit are clean. test/tui-notes.test.ts passes 4/4 on three runs in a row. It never failed here before the fix either, so the cause comes from reading the code. I didn't run the full suite; the forge runs it on push.
To try it: bun test test/tui-notes.test.ts. In the terminal, open Notes as the admin, ban a sender, and the ban holds once the toast shows.
🤖 Generated with Claude Code
Closes #8106
Approved. The diff since my last review only wraps the terminal's note send and Hide/Ban moderation in ctx.track, so the screen is not settled until the write finishes. That removes the race in the notes ban test without a sleep or a test change. Gating is unchanged: the same sendNote and moderateNote calls run with the same user, so nothing new reaches a guest. No new dependency, no dead code. I did not run the tests or open the preview. I read the diff and checked that ctx.track is already used elsewhere in the TUI.
Two things I would do next:
- Non-blocking: the Hide/Ban toast says "That could not be done." with no reason. Say whether the note is gone or the person lacks rights.
- Non-blocking: add an assertion in test/tui-notes.test.ts that the ban holds right after settle, with no extra waiting, so a regression fails the test.
The test "notes from the app, in the terminal > a person sees their own with its status; the admin sees all and bans" should no longer depend on timing. I never saw it fail here, so I read the cause from the code.
test/tui-notes.test.tspassed 4/4 before the change and passes on three runs after it.Cause: the terminal's "Ban sender" and "Hide" actions, and "Tell the builders", ran their work in the background without telling the app. Moderation waits on closing the note's issue. On a loaded check machine the test's
settleAppcould return before the ban was written, soisBanned(...)read false.Change:
src/tui/screens/notes.tsnow wraps the send and the moderation inctx.track(...). The app counts as settled only once they finish. Behaviour is the same, and the tests are unchanged.Checks:
tsc --noEmitandscripts/gate.tswere clean. I ran onlytest/tui-notes.test.ts, not the full suite. The forge's own check will run the rest.Not done: I did not touch other tests or the sentinel budget. The cause is fixed, not waited out.
🤖 Generated with Claude Code
Closes #8106