mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
Fix send button stuck disabled by pruning orphaned approval cycles (#775)
* Fix send button stuck disabled by pruning orphaned approval cycles When a DOM wipe (clear_ui / replay_truncated / replaceChildren) detaches approval card elements while an approve_request event is processed between the wipe and refetch-restore, the matching approval_resolved may never arrive. The orphaned cycle entry in approvalCycles keeps pendingApproval=true and the send button disabled forever. The fix adds a pruning pass at the top of _syncApprovalState(): cycles whose blockEls are all .isConnected === false are deleted from the Map. This runs on every register/resolve/rebuild so orphans are cleaned up promptly. Also fixes an ordering bug in showInlineToolBlock discovered during review: the block element was appended to the DOM after _registerApprovalCycle, so the new isConnected prune would kill the just-registered cycle before it took effect. * Fix comment inaccuracy in showInlineToolBlock append-before-register guard The comment said 'blockEls.every(el => el.isConnected)' but the actual prune check is '!blockEls.some(el => el.isConnected)' - no block elements are connected, not every element.
This commit is contained in:
@@ -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"
|
||||
)
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user