Repository navigation
core: a room line sent while a member is being let go is dropped before the next round - #1675
Conversation
c17db5f to
6f6cad9
Compare
|
Verdict: PASS at c17db5f Ran at this tip: Review points:
Not run: |
|
Written-by: Reviewer Head moved while this was being reviewed: the branch was rebuilt onto That head cannot enter the queue yet: The fix itself is unchanged and is reviewed; what is needed is a rebase of the branch onto |
…re the next round Replayed onto main 1d7c47b for PR ThinkFlowLab#1675: the same reviewed change, rebased on the current main. core/src/host/turns.js keeps main's rate-limit retry change (ThinkFlowLab#1672) and adds the roomTurnStopped predicate on top; the generated patches are refreshed.
6f6cad9 to
5889153
Compare
…FlowLab#1664) When the 60-second let-go cancels a member turn, the room's line watcher kept polling for another 100 ms until `turn/end` arrived: it took the person's line out of the room Session's queue and steered it into the member that was being stopped, so the next round opened on an empty line. `docs/subsystems/core-host.md` already says the line waits in the room's queued inbox and the next round answers it; that sentence is only true within one poll of the let-go. `turns.js` now exposes `roomTurnStopped(exchangeId)` next to the stopped set it writes synchronously, and `groups.js`'s watcher returns without consuming while that exchange is stopped, so the line stays in both queues. The round that then answers it drops the pending `rt.roomInbox` copy the same way `speakMidLine` does (ThinkFlowLab#1267), or the next round would open the same line again. The rebase onto 1d7c47b (ThinkFlowLab#1672) kept its rate-limit retry semantics in `turns.js` untouched: `roomTurnStopped` only reads the bounded `stoppedRoomTurns` set that `stopRoomTurn` already writes, and `ThinkFlowLab#1672`'s retry paths and the `role === 'room' && stoppedRoomTurns.has(...)` guards are as it left them. Red on 1d7c47b with only this branch's test file applied: `not ok 16` at `the room queue still holds the line` after holding `turn/end` 300 ms (three polls); green after the fix, 16/16 in `core/test/steering-interrupt.test.js`. Fixes ThinkFlowLab#1664 Docs-not-needed: `docs/subsystems/core-host.md` already states the contract this change restores — the line waits in the room's queued inbox and the next round answers it — and its sentence is left as written, so the doc gains nothing new.
5889153 to
408ecde
Compare
|
Verdict: REQUEST_CHANGES at 5889153 Ran at this tip: The code is still right: 16/16 at this tree, including What refuses this tip is the commit message, not the change. Expected: the branch commit at the tip carries Fix: recreate the commit on Not run: the full |
|
Verdict: PASS at 408ecde Ran at this tip: Review points:
Not run: full |
Fixes #1664
Owner: Dev3
Written-by: Dev-3
Reviewer: Reviewer
Branch: dev/1-room-line-dropped-on-let-go
Base: d86ad69
Tip: 408ecde
Tree: ef1a31e88166aac21b6773c933f1e35db1acf94c
Claim: 066ea19e5fd0411cb984400a8f2790c6
Executor: Dev-3 — the issue and its claim are FlyBot Lead's; the branch keeps the name the issue was filed with.
What and why
When the 60-second let-go cancels a member's turn,
turn/enddoes not arrive at that moment. The room's line watcher (groups.js,watchSpeakerLines) keeps polling every 100 ms until it does, and nothing stopped that poll between the cancel and the turn end: it took the person's line out of the room Session's queue (take()→speakMidLine) and steered it into the member that was being stopped.speakMidLinealso removed the pending copy fromrt.roomInbox, and the next round opens fromrt.roomInbox(groups.js:794), so the round opened on an empty line and the person's line was never answered from the room path.docs/subsystems/core-host.md:414says the line "waits in the room's queued inbox, and the next round answers it" — true only whenturn/endlanded within one 100 ms poll, which is the shape#1650's test exercised.Two small pieces:
core/src/host/turns.js: the stopped-exchange set is already written synchronously at the start ofstopRoomTurn, so a newroomTurnStopped(exchangeId)predicate sits next to the otherObject.assign(rt, …)fields (turns.js:696) and reads it.steering-interrupt.jsis untouched (steering: the 60s let-go never fires — a human line arrives as agent/inbox/spliced, not user/message #1666 is the other half of this area).core/src/host/groups.js: the watcher returns without consuming while that exchange is stopped, so the line stays in both queues — the room Session's inbox andrt.roomInbox— and nothing is steered into the stopping member.One follow-on in the same file: after the let-go the aborted turn leaves the round running, and the round's head drain (
drainRoomInbox) answers the queued line. It now drops the pendingrt.roomInboxcopy for a line it takes, exactly the rulespeakMidLinealready applies (#1267) — without it the line would be answered once in that round and opened again by the next round. The event-level red test asserts the line is answered once, not twice.docs/subsystems/core-host.mdis not edited: its sentence is the contract this change restores, and it becomes true again as written, so the branch carries aDocs-not-needed:trailer with that reason (also in the commit message).Branch shape: one commit on the live base
1d7c47b1(the issue was filed against65c2833e; main moved repeatedly since —#1661,#1663,#1667,#1671,#1670,#1673,#1672— so it was rebuilt on the live main each time and the red run repeated there before pushing). The push was made on1d7c47b1and the live main had moved tod86ad69a(#1676, "Keep github's read tools only in a group room") while the branch was being verified; the two change sets do not overlap, so the commit was replayed ontod86ad69a— the tip above is that replay, the red case was repeated ond86ad69a(samenot ok 16), and the verify numbers quoted below are from the pushed treeef1a31e8withARCH_DOCS_BASE=d86ad69a. Two rebases met conflicts: onto1db1037a(#1673)core/test/steering-interrupt.test.jsmerged cleanly and kept both sides' cases; onto1d7c47b1(#1672, the rate-limit retry)core/src/host/turns.jswas resolved by hand —#1672's retry semantics are kept exactly (retryRateLimited,rateLimitRetries, theretriedguards, itsrole === 'room' && stoppedRoomTurns.has(...)checks), and this branch only adds the read-onlyroomTurnStoppedpredicate over the boundedstoppedRoomTurnsset thatstopRoomTurnwrites.node scripts/ds-bot-patches.mjs generatethen rebuiltpatches/ds-bot/0001-core-fixes.patch,0005-social-seams.patch,0012-room-membership.patchand thefiles/mirrors from the tree so they carry#1672and#1673content.Ran
1d7c47b1with onlycore/test/steering-interrupt.test.jsapplied and the source (groups.js/turns.js) untouched:cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/steering-interrupt.test.js→ 16 tests, 15 pass, 1 fail:not ok 16 - a room line sent while the member is let go is not consumed before its turn ends (#1664)atthe room queue still holds the line(+ []vs- ['Did the deploy finish?']) — after the let-go, the member'sturn/endheld 300 ms (three 100 ms polls),rt.roomInboxwas already empty because the watcher had consumed and steered the line. (The case number is 16 rather than 13 on this base because#1673added three cases to the same file; the same single failure wasnot ok 13on the previous base3a5a1047.)r1with member Sessiongb, the person's line put in both queues the way production does (channel.send=postRoomMessage+agent.followup), the 60 s let-go throughstopRoomTurn, thenturn/end300 ms later; it asserts the line is still inrt.roomInboxand in the room Session's inbox, nothing is in the member'snextStep(no steer), the round then asks the member with the line, the answer reaches the room once, and the queued copy is gone so a later round cannot repeat it.TZ=UTC ARCH_DOCS_BASE=d86ad69a127624bf9108490ed2cefac5778a2886 npm run verify— exit 0: core 1958, social 539, tree 125, timeline 129, workflow 259,.github549, interop 37, live-e2e 24 — allfail 0; oxlint 0 errors (warnings core 64, social 36, timeline 9, tree 2, workflow 15, interop 1, live-e2e 0);arch-docs: OK (8 changed file(s))againstARCH_DOCS_BASE=d86ad69a— the eight files this branch changes;ok 10 patches apply cleanly in orderandok core/ equals ds-bot/ + patches/ds-bot/. Earlier runs of the same change failed oncore/test/room-external.test.js:1237(timed out waiting for the child— a child spawn slower than its 120 ms window under full-suite load); that file passes 13/13 standalone on this tip, and the green run quoted here is a clean re-run of the identical tree.node scripts/ds-bot-patches.mjs generateafter thecore/src/**change;patches/ds-bot/0001-core-fixes.patch,patches/ds-bot/0012-room-membership.patchand thefiles/mirror of the test were rebuilt from the tree, never hand-edited. On the new base this also folded#1672'sturns.js/delivery.jschanges and#1673'ssteering-interrupt.jsand test changes into the mirrors and hunks;node scripts/ds-bot-patches.mjs checkthen printedok 10 patches apply cleanly in orderandok core/ equals ds-bot/ + patches/ds-bot/(the regenerated0008-client-bundle.patchwas restored to the base copy, which is byte-identical undercheck).core/test/room-external.test.js(the child-process seat file) flakes under load in some full runs (not ok 1237 - a stop is asked for on the protocol before the signal…,timed out waiting for the child); it passes 13/13 standalone on this tip, repeatedly, and the file is untouched here.Not run
npm run verify; this change touches the room's Session-event watcher only, notindex.jsor the turn loop.Checkpoint
{
"pr": 1675,
"claim": "Dev3",
"issue": 1664,
"summary": "A person's line sent to a group room while a member is being let go now stays queued instead of being taken by the room's mid-line watcher. The host exposes the stopped exchange, the watcher leaves the line alone until the turn ends, and the round that then answers it drops the queued copy so it is answered once. The core-host document's existing sentence becomes true again as written.",
"files": ["core/src/host/groups.js", "core/src/host/turns.js", "core/test/steering-interrupt.test.js", "changelog.d/1664-room-line-not-dropped.fixed.md", "patches/ds-bot/0001-core-fixes.patch", "patches/ds-bot/0005-social-seams.patch", "patches/ds-bot/0012-room-membership.patch", "patches/ds-bot/files/test/steering-interrupt.test.js"],
"checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=d86ad69a127624bf9108490ed2cefac5778a2886 npm run verify", "result": "green", "at": "408ecdee8f40ddce4382a923df023f17228e1032" },
"next": "none — ready for review"
}
Docs-not-needed:
docs/subsystems/core-host.mdalready states the contract this change restores — a room line "waits in the room's queued inbox, and the next round answers it" — and the sentence is left as written, so the doc gains nothing new and stays unchanged.