Skip to content

Coalesce a burst of the user's messages into one turn - #682

Merged
Birfy merged 3 commits into
mainfrom
feat/642-settle-window
Oct 22, 2026
Merged

Birfy merged 3 commits into
mainfrom
feat/642-settle-window

Conversation

@Birfy

@Birfy Birfy commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #642

Tip SHA: a3cbbcdc848afa1b21abfce0d9c15061669b205a

Reproduce

Send three short lines to one Bot in its own chat, a few hundred milliseconds apart. Each send reaches POST /api/bot on its own (the composer posts one card per line) and the host opened a turn per line: three prompts, three turns, so the Bot starts on the first line and then takes the second and third as separate work. core/test/settle.test.js, "three lines inside the window open one turn, in arrival order", reproduces it: with core/src/host/channel.js from main it records three prompts; with this change it records one.

What changed

Update at a3cbbcdc — the #682 INC: a held burst is no longer dropped when the prompt fails. flushSettled() takes the lines, and on an error puts them back at the front (a line that arrived meanwhile stays behind them in order), re-arms the window while the failures are below SETTLE_RETRIES (3), and rethrows. The send path that was waiting for an answer therefore gets the error (bot/refused, as main does) instead of ok(true), so a line the host could not deliver is never reported as sent; past the retries the kept lines go out with the user's next line instead of retrying forever. Two cases pin it: a failed flush delivers nothing and the line goes out on the retry, and a failed flush on the send path answers ok: false with bot/refused while the kept line still goes first afterwards.

  • core/src/host/channel.js: SETTLE_MS (1500 ms) and one settle timer per Session. In a Bot's own chat (rt.roomOf(sessionId) === undefined) a user line arriving while no turn runs is held, further user lines join it, and the timer opens one turn whose text is those lines joined by \n in arrival order — no sender labels, no "read N messages as one" notice, no new setting, no i18n. A line arriving while a turn runs is not held: the held lines are flushed first (so the queued items keep arrival order) and that line lands as its own queued item, as on main. A Bot-to-Bot deliver never goes through the window (its own queue and Bot-to-bot reports duplicate and pile up because cross-line delivery only goes through the parent chain #641 dedupe stay its own). A group room keeps its round rules — its line must reach the Session at once so a running round can drain it (Face rooms: a plain line should be answered by the Bot(s) it is for, not by every Bot (follow-up to #295) #299) — so the window is only reached when the Session has no room. The pending timer is cleared on dispose.
  • core/test/settle.test.js (new, 5 cases) and one line in core/test/install.test.js's registration-order snapshot (effect ds-bot: settle window).
  • docs/subsystems/core-host.md: new section "A burst of the user's lines (A burst of messages opens one turn per message; there is no receive buffer #642)".
  • changelog.d/642-settle-window.changed.md.
  • patches/ds-bot/ regenerated for the two upstream files (src/host/channel.js in 0005-social-seams, test/install.test.js in 0001-core-fixes) and the new test as files/test/settle.test.js.

What I ran

At a3cbbcdc:

  • TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/settle.test.js → 7/7 pass (the 5 acceptance cases plus the two INC cases).
  • The same command with test/rounds.test.js test/install.test.js → 49/49 pass (rounds 38, install 4, settle 7).
  • Rebuilt the series from ds-bot/ + patches/ds-bot/files/ + all 10 patches and compared it byte for byte with core/ → core/ equals ds-bot/ + patches/ds-bot/ (the check half of ds-bot-patches.mjs; this box has no git, and ds-bot/ is untouched, so the pin-tree check is CI's).
  • node .github/scripts/changelog.mjs check → 10 fragments well formed.
  • node --check on both changed JS files.

Red without the source change

Restore core/src/host/channel.js to 9393d558 and keep the new tests: test/settle.test.js → 3 of 7 fail. The headline case records 3 prompts instead of 1; a single line opens its turn before the window; a burst that contains a Bot-to-Bot line opens its prompt during the burst. The four that still pass cover behaviour this issue keeps unchanged or adds no failure path (a line after the window is its own turn, a line arriving while a turn runs is its own item, and the two INC cases cannot fail where nothing is ever held).

Restore only this PR's settle change to the previous tip eff9f5ab (#682's subject) and keep the new tests: test/settle.test.js → 2 of 7 fail, exactly the two #682 cases — the failed flush delivered nothing but the line never went out on the retry, and the send path answered ok: true for a line it had lost.

Not run

  • npm run verify in full: this box has no core/node_modules (no npm ci) and no git, so oxlint, arch-docs (cannot diff against HEAD), build-client --check and the plugin suites cannot run here. CI's tests-ok and package jobs run them.
  • The smoke test (dev/bench.sh): not run — the bench needs a pnpm profile install this box cannot do, and CI does not run it. The issue's acceptance cases are covered by the deterministic-clock unit tests above.
  • No client file changed and no client.js rebuild, so build-client --check is unaffected.

Risk

  • T3 by .github/agent/risk-rules.json (core/src/host/channel.js is on the T3 path list); the risk-tier job summary is authoritative.
  • User-visible timing change: one line sent into a Bot's own chat now starts its turn up to 1500 ms later (the owner's design decision in the issue). Three lines consume one turn instead of three.
  • A line held when the host is disposed inside the window is dropped with the cleared timer — the process is going away and the window is 1.5 s; nothing is written to state before a turn opens.
  • A held burst whose prompt fails is retried for up to three windows (1.5 s each) and then waits for the user's next line; the send that triggered a failing flush is answered bot/refused rather than ok, which is a visible change only in that failure case.
  • Ordering when a turn starts inside the window is explicit: held lines are flushed (queued) before the new line, so arrival order is kept.
  • Scope kept narrow: group rooms, Bot-to-Bot delivery, scheduled runs, face rooms and the client are untouched.

A burst typed in a row opened one turn per line, so the Bot started on the
first line and then took the next ones as separate work (#642). In a Bot's
own chat the host now holds the user's lines for 1500 ms and opens one turn
with them in arrival order, each on its own line. A line that arrives while
a turn runs still lands as its own queued item, a Bot-to-Bot delivery never
waits behind the window, and a group room keeps its round rules.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 22, 2026
@Birfy Birfy self-assigned this Oct 22, 2026
Lint: Identifier 'randomUUID' is imported but never used.
@github-actions github-actions Bot added the risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff label Oct 22, 2026
@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: INC at eff9f5a
Written-by: Reviewer

Reviewed at this tip only. All 9 checks are completed/success, mergeable: clean, 0 commits behind main. settle.test.js + rounds.test.js + install.test.js are 47/47 here, and reverting core/src/host/channel.js to 9393d558 turns 3 of the 5 new cases red, as the PR body says.

Blocker: the window can report a delivered turn while the user's line is thrown away

openSettledTurn() catches everything from prompt() and only logs it (core/src/host/channel.js:43-56). flushSettled() deletes the held lines before calling it (:57-63), and send returns ok(true) as soon as settleLine reported a hold (:521). So when the queued prompt fails, the user's text is gone, send claims success, and no turn ever runs. Before this PR the same failure reached the caller as an error.

Probe (same stub of ctx.sessionController.prompt raising the host's own "no model set up" refusal, so the only difference is this PR):

tip  eff9f5ab: send -> {"ok":true,"value":true} | prompts: 0 | log: "ds-bot: held user lines were not delivered: no model set up"
base 9393d558: send -> {"ok":false,"error":{"code":"bot/refused","message":"no model set up"}}

This is a user-visible regression in the direction the PR is meant to improve: a Bot whose model is not set up (or whose capability install throws) silently eats what the user typed, and the UI shows the line as accepted. The case where a turn starts inside the window is the same defect with a caller available — settleLine awaits flushSettled() and returns false (:65-68), yet the held lines' failure is swallowed there too.

Fix

  • Keep the lines when the hand-off fails: only settling.delete(sessionId) after prompt() resolved, so a failed flush can be retried by the next line or the next send, and log it as "still held", not as "lost".
  • And where a caller is waiting (the turn-began-inside-the-window path), let the error travel: await the flush without the catch, so send returns bot/refused like base does, instead of ok(true).
  • Add the case: a prompt that throws must leave the user's line either delivered later or reported as an error — never ok: true with zero prompts.

Non-blocking

  • ctx.effect clears the settle timers on dispose and drops whatever is held (:79-83), the same silent loss on shutdown. Flushing instead of clearing would be better; it is a rarer window and no test covers it.
  • After a flush, a Bot line that arrives first is delivered ahead of the user's earlier input (test 4 pins that order deliberately), so "keeps input order" holds between the user's own lines only. Worth one sentence in the PR body.

Risk: T3 (core/src/host/channel.js is a T3 path), so the fixed tip still needs a maintainer APPROVED review. No awaiting-approval while it is INC.

The settle window answered ok(true) and swallowed a prompt error, so a line
the host could not deliver disappeared while the chat showed it as sent, and
the held lines were dropped before the prompt ran. flushSettled() now keeps
them, re-arms the window a few times, and rethrows so the send path answers
the caller the error instead of ok; the lines go out with the next line after
the retries (#682).
@Birfy

Birfy commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at a3cbbcd
Written-by: Reviewer

Ran:

Not run:

  • a live UI burst against a running app — the settle window is driven by fake timers in the tests

Reviewed at this tip only. All 9 checks are completed/success, mergeable: clean. settle.test.js + rounds.test.js + install.test.js are 49/49 here, and patches/ds-bot/files/test/settle.test.js is byte-identical to its core/test/ counterpart.

The INC at eff9f5ab is fixed: openSettledTurn() no longer swallows the prompt error, a failed flush puts the lines back with unshift and re-arms its own window up to SETTLE_RETRIES (core/src/host/channel.js:45-92), and the send path that is still waiting gets the error instead of ok(true). The two new tests pin exactly the two halves:

  • a failed flush delivers nothing, keeps the line, and the next window sends it in order;
  • a send whose flush fails answers ok: false with bot/refused and no prompt, and the kept line goes out ahead of the next one.

Red without the source change: with channel.js reverted to eff9f5ab those two are the only failures in settle.test.js (5/7).

Non-blocking

  • After SETTLE_RETRIES the lines sit in settling with no timer and go out only with the user's next line. If the user never writes again they are never delivered and only a warn records it. A UI hint, or one final retry on a longer timer, would close that.
  • In the turn-began-inside-the-window path the new line is not added to the held list before the flush, so when the flush fails that line is dropped too (the sender does get the error). Appending it to held.lines first would keep both.
  • ctx.effect still clears the timers on dispose and drops whatever is held (:109-113); a flush there would be better.

Risk: T3 (core/src/host/channel.js), so this needs a maintainer APPROVED review at this tip. Added awaiting-approval; not enqueued.

@Birfy Birfy added the awaiting-approval PR is green and waits for the maintainer's approval (T2/T3) or a review PASS at its tip label Oct 22, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit a41f860 Oct 22, 2026
9 checks passed
@Birfy
Birfy deleted the feat/642-settle-window branch October 22, 2026 06:21
@Birfy Birfy removed awaiting-approval PR is green and waits for the maintainer's approval (T2/T3) or a review PASS at its tip agent-pr Pull request opened by an agent labels Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A burst of messages opens one turn per message; there is no receive buffer

1 participant