diff --git a/tests/test_interactive_pane_js.py b/tests/test_interactive_pane_js.py index 71578107..b7d27aca 100644 --- a/tests/test_interactive_pane_js.py +++ b/tests/test_interactive_pane_js.py @@ -409,3 +409,23 @@ def test_pane_handles_cross_user_409() -> None: assert "r.status === 409" in body assert 'status: "cross_user_interjection"' in body assert 'data.status === "cross_user_interjection"' in body + + +def test_sync_approval_state_prunes_orphan_cycles() -> None: + """``_syncApprovalState`` prunes cycles whose block elements are no longer + in the living DOM (``.isConnected === false``). This covers the rare case + where an ``approve_request`` event is processed between a DOM wipe + (``clear_ui`` / ``replay_truncated`` / ``replaceChildren``) and the + refetch-restore — the cycle card lives in a detached subtree, the matching + ``approval_resolved`` never arrives, and the send button stays disabled + forever without this guard. The pin guards against a future refactor that + drops the orphan prune but doesn't otherwise break ``_syncApprovalState``.""" + body = _INTERACTIVE.read_text(encoding="utf-8") + fn_start = body.index("_syncApprovalState() {") + assert "entry.blockEls && !entry.blockEls.some((el) => el.isConnected)" in body, ( + "orphan pruning must check .isConnected on block elements" + ) + tail = body[fn_start : body.index("_oldestCycleId()", fn_start)] + assert "this.approvalCycles.delete(cid);" in tail, ( + "orphan pruning must delete the cycle from the Map" + ) diff --git a/turnstone/shared_static/interactive.js b/turnstone/shared_static/interactive.js index adbe1d3a..1a7f1674 100644 --- a/turnstone/shared_static/interactive.js +++ b/turnstone/shared_static/interactive.js @@ -541,6 +541,18 @@ class Pane { // live. _syncApprovalState() { + // Prune orphaned cycles whose block elements are no longer in the DOM. + // A clear_ui / replay_truncated / re-render that wipes the conversation + // subtree (messagesEl.replaceChildren()) also clears approvalCycles via + // _resetStreamingRefs. But if an approve_request event is processed + // between the wipe and the refetch, its cycle card is in a detached + // subtree and the matching approval_resolved may never arrive — leaving + // pendingApproval=true and the send button disabled forever. + for (const [cid, entry] of this.approvalCycles) { + if (entry.blockEls && !entry.blockEls.some((el) => el.isConnected)) { + this.approvalCycles.delete(cid); + } + } const first = this.approvalCycles.values().next(); const active = first.done ? null : first.value; this.pendingApproval = this.approvalCycles.size > 0; @@ -2952,8 +2964,17 @@ class Pane { ), }); block.appendChild(actions); + } + + // Append BEFORE registering the approval cycle — the orphan-prune in + // _syncApprovalState checks that no blockEls are connected + // (!blockEls.some(el => el.isConnected)), so a freshly-built block + // (not yet in the DOM) would be mistaken for an orphan and immediately + // pruned if we registered before appending. + if (!announced) this.messagesEl.appendChild(block); + if (!autoApproved) { this._registerApprovalCycle(cycleId, [block], items); - const fb = actions.querySelector(".conv-feedback"); + const fb = block.querySelector(".conv-feedback"); // Focus the feedback field only for the FIRST (oldest) live cycle — // a sibling card arriving while the user is typing into another // cycle's field must not steal focus mid-word. @@ -2963,8 +2984,6 @@ class Pane { }); } } - - if (!announced) this.messagesEl.appendChild(block); this._relinkAgentCards(items); this.scrollToBottom(stick); }