emrg: remove TUI selector widgets from chat on confirm/cancel - #921
Conversation
All six interactive selectors (session/delete/project/model/rewind/task) kept their widget in the chat after selection: Esc/Enter only set active=False and widget=None, but never called chat.remove(), so the overlay list stayed visible (rant 2026-08-21T16:47:44 for /trigger). Add chat.remove(<sel>.widget) to every deactivation path (Esc-cancel and Enter-confirm for each selector, plus the direct /resume name path), and guard the tasks_list handler against stacking a second TaskSelector on re-trigger while one is already active.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 1214. Fixes rant 2026-08-21T16:47:44 (/trigger task list stays in chat): every selector deactivation path now calls chat.remove(.widget) before widget=None (session/delete/project/model/rewind/task × Esc-cancel + Enter-confirm, plus /resume name path), and the tasks_list handler drops a stale TaskSelector before stacking a new one. ChatHistory.remove is identity-based and no-ops on ValueError — safe in all paths. Local: pytest 994 passed + 1 skipped (995, doc-count guard green), GUI 249+8, import + CLI OK. CI test + test-windows PASS.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 1215. Independent re-review on head 9161006 (MERGEABLE, CI test + test-windows PASS):
- All 13 selector deactivation paths (session/delete/project/model/rewind/task × Esc-cancel + Enter-confirm, plus the /resume direct path) now call chat.remove(.widget) before widget=None — the overlay is removed from chat rows instead of lingering.
- ChatHistory.remove is identity-based and swallows ValueError (widgets.py:683), so calling it with a None/stale widget is a safe no-op — the /resume path's unconditional remove is defensive, not fragile.
- Duplicate-activation guard in the tasks_list handler drops a stale TaskSelector before stacking a new one on repeated /trigger.
- Local: pytest 994 passed + 1 skipped, GUI 249+8 (257), import + CLI OK.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle R2165. Independent review on head 9161006 (MERGEABLE, mergeStateStatus CLEAN; CI test + test-windows PASS):
- Diff audited: all 13 selector deactivation paths (session/delete/project/model/rewind/task × Esc-cancel + Enter-confirm, plus the /resume direct path) now call chat.remove(.widget) before widget=None — the missing widget-removal was the exact root cause of rant 2026-08-21T16:47:44 (/trigger task list stays in chat).
- Duplicate-activation guard in tasks_list handler correctly drops a stale pending TaskSelector before stacking a new one.
- ChatHistory.remove no-ops on ValueError, so removing an already-removed widget is safe.
- Local verification on this branch: pytest 994 passed + 1 skipped (995 collected); GUI 249 pass / 0 fail / 8 skipped; import + CLI OK. Merge-tree vs master: 0 conflict markers.
3 consecutive ✅ from different cycles — merging.
Summary
Fix TUI selector overlays staying in the chat after selection (host rant
2026-08-21T16:47:44—/triggerpopup: the task list remains visible in the chat after picking a task).Root cause
All six interactive selectors (
session_sel,delete_sel,project_sel,model_sel,rewind_sel,task_sel) shared the same latent bug: on Esc-cancel and Enter-confirm they only setactive = False/widget = Nonebut never calledchat.remove(widget), so the selector widget was never removed from the chat rows and stayed on screen. Only the autocomplete widget was being removed.Changes (
emrg/client/app.py, +18 lines)chat.remove(<sel>.widget)immediately beforewidget = Nonein:session_sel: Esc-cancel, Enter-confirm, and the direct/resume <name>pathdelete_sel: Esc-cancel and Enter-confirmproject_sel: Esc-cancel and Enter-confirmmodel_sel: Esc-cancel and Enter-confirmrewind_sel: Esc-cancel and Enter-confirmtask_sel: Esc-cancel and Enter-confirm (the/triggerrant target)tasks_listhandler — if a TaskSelector is already in the chat (duplicate/triggerwhile one is pending), drop it before stacking a new one instead of rendering two stacked lists.ChatHistory.remove(row)removes by widget identity and silently no-ops onValueError, so this is safe in every path.Verification
uv run pytest tests/ -v→ 994 passed + 1 skipped (995 collected; doc-count guard green, Agent.md stays at 995 on master)cd emrg/gui && npm test→ 249 passed + 8 skipped (257)uv run python -c "from emrg.client.app import run_client"→ OKuv run python -m emrg --help→ OK