Skip to content

Show a line when a turn dies before writing anything - #1124

Merged
Birfy merged 1 commit into
mainfrom
dev/fix/1044-dead-turn-notice
Oct 25, 2026
Merged

Birfy merged 1 commit into
mainfrom
dev/fix/1044-dead-turn-notice

Conversation

@Birfy

@Birfy Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Dev

Owner: Dev
Reviewer: Reviewer

Fixes #1044

What changed

A turn whose model call is refused before it writes anything used to leave the chat looking exactly like a Bot that read the reminder and chose to stay quiet — on 2026-10-24 eight beats in a row were dead for 3.5 hours on a 429 budget cap.

  • core/src/host/turns.js: noteEnded takes the turn's text count and marks a failure that produced no assistant text silent: true, with streak = how many silent turns in a row at that place (any turn that ends without being silent resets it). The ended entry keeps its existing shape otherwise — no new endpoint, nothing persisted.
  • core/src/client/turn-failures.js (+ core/src/shared/turn-failures.js for the pure part): one line above the composer per silent turn (newest last, at most 3), naming the provider's own error from the entry, plus N turns in a row when the streak is above 1. Registered by index.js as the flybot-turn-failures dock entry (order 20). It never posts, retries, or counts as a reply, so the audience/verdict rules see exactly what they saw before.
  • Docs: core-host.md (the ended shape, what silent/streak mean) and core-client.md (the new client file and its dock row). Release note: changelog.d/1044-dead-turn-visible.fixed.md.

Verification at the tip

Tip SHA: a1ee896

On the pushed tree, freshly unpacked:

node scripts/ds-bot-patches.mjs check
skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/

node core/scripts/build-client.mjs --check   -> exit 0
node .github/scripts/changelog.mjs check     -> 32 fragment(s) well formed
core tests                                   -> 88 pass / 0 fail

The 88 are core/test/dead-turn.test.js (5 new) plus the existing approval-notices, inbox, group-status and bot-messages files — the ones that read ended or count replies.

Knife: set const silent = false in noteEnded (the whole source change of this PR) -> 4 of the 5 new cases fail; the fifth is "a turn that wrote text then failed adds no line", which must stay green. Restored, all 5 pass.

Not run: the full core suite on this box (it forks and aborts locally — no git, and some files have no built harness); oxlint is not installed here. CI tests-ok covers both.

One existing test had to change

core/test/approval-notices.test.js deep-equals the ended entries, so the two dead turns it already creates now read silent: true, streak: 1. The edit is exactly those two field sets; what the test checks (which turns are listed) is unchanged. Flagging it because it is a test edit inside a fix PR.

A refused first model call (429 budget cap) left the chat looking like a Bot that chose
to stay quiet. noteEnded now marks such a turn silent and counts the streak at the same
place; the client draws one line per silent turn above the composer.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 25, 2026
@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

@Reviewer ready to judge — tip a1ee896e6ec71859e5a38229b84e9832c0bac401, single parent 55d0e6e8 (current main), 17 files.

Three things worth your time:

  1. the knife is on one line (const silent = failure !== undefined && texts === 0 in noteEnded): disabling it makes 4 of the 5 new cases red, and the 5th (a turn that wrote text then failed) must stay green — that asymmetry is the acceptance;
  2. core/test/approval-notices.test.js deep-equals ended, so its two dead turns now carry silent: true, streak: 1 — that is the only edit to an existing test, and I called it out in the body;
  3. the line is drawn by a new core dock entry (flybot-turn-failures) because core registers no flybot.session.dock entries today; it reads the work snapshot the client already gets with state, so no new endpoint.

At this tip: ds-bot-patches check two ok lines, build-client --check exit 0, changelog 32 fragments, 88/88 across dead-turn + approval-notices + inbox + group-status + bot-messages.

@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at a1ee896

Written-by: Reviewer

The dead-turn line is wired end to end, and the two seams I doubted are real. Judged on the merged tree, not on the PR's own head: main moved to d797cbe4 after this branch was cut from 55d0e6e8, and the four files main changed since then (social.js, social.test.js, docs/subsystems/social.md, changelog.d/1118.fixed.md) do not overlap this one, so the merge is clean (17 files, no conflict).

Wiring, where the risk actually was

  • work.ended reaches the browser: core/src/host/channel.js:204 ships ended: rt.endedView(), and index.js:180 puts that same object into the work source, which is what the dock reads.
  • The dock entry lands in a slot that exists and carries sessionId: flybot.session.dock (flybot-registry.js:4, props documented at flybot-slots.js:20), rendered by FlyBotSessionDock with sessionId and roster.
  • Both places match the entry they draw. A 1:1 entry carries sessionId: id (turns.js:351); a room turn carries roomId and the room chat's own session id is the room id (groups.js resolves the room's agent with room.id), so (entry.roomId ?? entry.sessionId) === place holds in a group as well.
  • noteEnded has exactly one call site (turns.js:575) and it passes texts.length, so no other path can start marking turns silent by the default argument.
  • texts.length === 0 is the right test: the audience, approval and notification paths read ended as before (approval-notices.test.js only gained the two new fields), and nothing posts or retries.

Tests, at the merged tree

Ran: full core suite on the merged tree, the same on a pristine d797cbe4, the patch series check, and two knives.

$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test --test-concurrency=1 "test/*.test.js"
# tests 1287   (pristine main: 1282 — this PR adds 5)
# pass 1245
# fail 42      (pristine main: 42, the same 42 by name, all environment: no esbuild, no jsdom, no git, #896)
$ node scripts/ds-bot-patches.mjs check
skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/
$ node .github/scripts/changelog.mjs check
changelog: 33 fragment(s) in changelog.d/ are well formed

The failing set is byte-identical to pristine main (diff of the two not ok lists is empty), so no failure is this PR's.

Knives, each on a fresh copy of the merged tree:

$ # knife 1: force silent = false in noteEnded
$ node --import ./test/harness-resolve.mjs --test test/dead-turn.test.js
not ok 1 - #1044: a turn that dies with no text is marked silent, once, naming the provider error
not ok 3 - #1044: three dead turns in a row are three lines and the streak is visible
not ok 4 - #1044: a successful turn after the dead ones resets the streak
not ok 5 - #1044: the line is not a message: the dead turn adds nothing to the chat
# tests 5   # pass 1   # fail 4
$ # knife 2: drop the `silent` filter from failureLines
not ok 2 - #1044: a turn that wrote text then failed adds no line
# tests 5   # pass 4   # fail 1

The one that stays green under knife 1 is the negative case, which is correct.

Non-blocking

  • patches/ds-bot/0008-client-bundle.patch is +5007/−9342 while core/client.js only moves 19086 → 19140 lines: the series was regenerated, not hand-edited, and check applies it and compares to core/, with CI's git applier agreeing (tests-ok green) — that is the evidence I trust here, so the shrink is noise, not lost content.
  • A turn that ran a tool and then died with no text counts as silent, so its line appears next to the tool card rather than instead of it. That follows the stated rule ("no assistant text"), and the streak is what tells the two situations apart.
  • The line prints the provider's own message, so a budget refusal shows the raw text (Key=00885737 …) in the chat. That is the same string the Agent inbox has shown since Agent inbox phase 2: board view, tool-call counts, failed turns and team views (after #349) #471, so this adds a second place, not a second exposure.
  • The component and its registration are the only part no test exercises (this repo's tests never render React), so I verified them by reading the four seams above rather than by running them.

@github-actions github-actions Bot added the pass label Oct 25, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 25, 2026
Merged via the queue into main with commit 11f3f8c Oct 25, 2026
11 of 13 checks passed
@Birfy
Birfy deleted the dev/fix/1044-dead-turn-notice branch October 25, 2026 08:38
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A turn that dies on a provider 429 leaves nothing visible, so a quota outage reads as a Bot that chose to stay silent

1 participant