From 27d62611705eb57b2c25e012adf565f37579e405 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 12 Nov 2026 17:23:26 -0500 Subject: [PATCH 1/3] Show shared command forms before preparation and own complete confirmation --- .verification/p189-command-forms.plan.json | 161 ++++++++++++++++++ .../p189-confirm-current-draft.plan.json | 47 +++++ .../p189-confirm-loading-form.plan.json | 35 ++++ .../p189-preparation-failures.plan.json | 47 +++++ src/toad/thread_actions.py | 11 +- src/toad/widgets/comms_command_dialog.py | 115 +++++++++---- src/toad/worker_group.py | 14 +- 7 files changed, 389 insertions(+), 41 deletions(-) create mode 100644 .verification/p189-command-forms.plan.json create mode 100644 .verification/p189-confirm-current-draft.plan.json create mode 100644 .verification/p189-confirm-loading-form.plan.json create mode 100644 .verification/p189-preparation-failures.plan.json diff --git a/.verification/p189-command-forms.plan.json b/.verification/p189-command-forms.plan.json new file mode 100644 index 000000000..5ad3950c0 --- /dev/null +++ b/.verification/p189-command-forms.plan.json @@ -0,0 +1,161 @@ +{ + "recipes": [ + { + "recipe_id": "shared-command-form-preparation-and-confirmation", + "operations": [ + { + "operation": "ensure_import", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "import_source": "from python_introspect.choices import LabelledChoices" + }, + { + "operation": "ensure_import", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "import_source": "from textual.worker import Worker" + }, + { + "operation": "ensure_import", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "import_source": "from toad.worker_group import WorkerGroup" + }, + { + "operation": "ensure_import", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "import_source": "from agent_comms.cli_verbs.cli_command import TargetAction" + }, + { + "operation": "ensure_import", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "import_source": "from agent_comms.errors import RouteChangedError" + }, + { + "operation": "ensure_import", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "import_source": "from agent_comms.execution.native_failures import NativePiUnavailable" + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "PreparedChoices", + "replacements": [ + { + "old_source": "class PreparedChoices(AnnotationChoices, DeclaredMetadata):", + "new_source": "class PreparedChoices(LabelledChoices, DeclaredMetadata):" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "PreparedChoices", + "replacements": [ + { + "old_source": " def choices(self):\n return tuple(value for _, value in self.parameter.choices)\n\n def label(self, choice):\n return next((label for label, value in self.parameter.choices if value == choice), 'Choose\u2026')", + "new_source": " def labelled_choices(self):\n return self.parameter.choices" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "ApplyCommandAction.apply", + "replacements": [ + { + "old_source": " await context.confirm()\n if context.query_one(ApplyCommandButton).disabled:\n return\n arguments = context.arguments()\n confirmed = context.query_one(CommandConfirmed).value\n context.query_one(ApplyCommandButton).disabled = True\n await context.app.preparation.run_thread(\n lambda: context.definition.edited(arguments).with_confirmation(confirmed))\n if context.is_attached and arguments == context.arguments():\n context.dismiss(arguments)", + "new_source": " await context.confirm()" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.confirm", + "replacements": [ + { + "old_source": " self.form.finish()", + "new_source": " self.form.finish()\n if self.query_one(ApplyCommandButton).disabled:\n return\n arguments = self.arguments()\n confirmed = self.query_one(CommandConfirmed).value\n self.query_one(ApplyCommandButton).disabled = True\n await self.app.preparation.run_thread(\n lambda: self.definition.edited(arguments).with_confirmation(confirmed))\n if self.is_attached and arguments == self.arguments():\n self.dismiss(arguments)" + } + ] + }, + { + "operation": "insert_before_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog", + "source": "class CommandPreparation(WorkerGroup):\n \"\"\"One preparation follows the current draft without restarting on each edit.\"\"\"\n\n\n" + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.__init__", + "replacements": [ + { + "old_source": "definition: PreparedTargetAction", + "new_source": "definition: TargetAction" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.on_mount", + "replacements": [ + { + "old_source": " self.update_confirmation()", + "new_source": " self.prepare_current()\n self.update_confirmation()" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.draft_changed", + "replacements": [ + { + "old_source": " self.update_confirmation()\n if paths & self.choices.keys():\n self.run_worker(self.refresh_catalogs(self.arguments()),\n group='command-catalogs', exclusive=True)", + "new_source": " self.prepare_current()\n self.update_confirmation()" + } + ] + }, + { + "operation": "insert_before_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.arguments", + "source": " def prepare_current(self) -> None:\n if not CommandPreparation.running_on(self):\n CommandPreparation.run_on(self, self.refresh_catalogs(self.arguments()))\n\n @on(Worker.StateChanged)\n def preparation_changed(self, event: Worker.StateChanged) -> None:\n if (event.worker.node is self and event.worker.group == CommandPreparation.group\n and event.worker.is_finished and self.is_attached):\n self.update_confirmation()\n\n" + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.refresh_catalogs", + "replacements": [ + { + "old_source": " self.query_one(ApplyCommandButton).disabled = True\n edited = self.definition.edited(arguments)\n fields = (await self.app.preparation.run_thread(\n lambda: self.core.call(CommsPrepareTargetActionTool(action=edited)))).fields\n if not self.is_attached or arguments != self.arguments():\n return\n for parameter in fields:\n if parameter.name in self.choices:\n self.choices[parameter.name].parameter = parameter\n self.form.values[parameter.name].editor.paint()\n self.update_confirmation()\n", + "new_source": " self.query_one(ApplyCommandButton).disabled = True\n try:\n while self.is_attached:\n edited = self.definition.edited(arguments)\n prepared = await self.app.preparation.run_thread(\n lambda: self.core.call(CommsPrepareTargetActionTool(action=edited)))\n if not self.is_attached:\n return\n current = self.arguments()\n if arguments != current:\n arguments = current\n continue\n self.definition = prepared\n for parameter in prepared.fields:\n if parameter.name in self.choices:\n self.choices[parameter.name].parameter = parameter\n self.form.values[parameter.name].editor.paint()\n break\n except (NativePiUnavailable, RouteChangedError) as error:\n self.app.show_error(error, self.definition.label)\n if self.is_attached:\n self.update_confirmation()\n" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.update_confirmation", + "replacements": [ + { + "old_source": " arguments = self.arguments()\n async def prepare():", + "new_source": " arguments = self.arguments()\n if CommandPreparation.running_on(self):\n self.query_one(CommandConfirmation).update(\"Loading choices\u2026\")\n confirmed.display = False\n return\n async def prepare():" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/thread_actions.py", + "target_qualname": "ThreadAction.collect", + "replacements": [ + { + "old_source": " try:\n prepared = await ctx.app.preparation.run_thread(\n lambda: ctx.observed.core.call(CommsPrepareTargetActionTool(action=definition)))\n ctx.current()\n except (NativePiUnavailable, RouteChangedError) as error:\n ctx.app.show_error(error, definition.label)\n return\n if ctx.app.screen is screen:\n ctx.app.push_screen(\n CommandDialog(prepared, ctx.title, prepared.fields, ctx.observed.core), accepted)", + "new_source": " fields = await ctx.app.preparation.run_thread(lambda: definition.editable_fields)\n ctx.current()\n if ctx.app.screen is screen:\n ctx.app.push_screen(\n CommandDialog(definition, ctx.title, fields, ctx.observed.core), accepted)" + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/.verification/p189-confirm-current-draft.plan.json b/.verification/p189-confirm-current-draft.plan.json new file mode 100644 index 000000000..d0e545cee --- /dev/null +++ b/.verification/p189-confirm-current-draft.plan.json @@ -0,0 +1,47 @@ +{ + "recipes": [ + { + "recipe_id": "confirm-the-current-draft-through-its-worker-owner", + "operations": [ + { + "operation": "ensure_import", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/worker_group.py", + "import_source": "import asyncio" + }, + { + "operation": "insert_before_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/worker_group.py", + "target_qualname": "WorkerGroup.running_on", + "source": " @classmethod\n def workers_on(cls, node: DOMNode) -> tuple[Worker, ...]:\n return tuple(worker for worker in node.workers\n if worker.node is node and worker.group == cls.group)\n\n @classmethod\n async def wait_on(cls, node: DOMNode) -> None:\n \"\"\"Wait for the node's actual work; its original error still refuses the caller.\"\"\"\n await asyncio.gather(*(worker.wait() for worker in cls.workers_on(node)))\n\n" + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/worker_group.py", + "target_qualname": "WorkerGroup.running_on", + "replacements": [ + { + "old_source": "return any(worker.node is node and worker.group == cls.group and not worker.is_finished\n for worker in node.workers)", + "new_source": "return any(not worker.is_finished for worker in cls.workers_on(node))" + } + ] + }, + { + "operation": "ensure_import", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "import_source": "from agent_comms.cli_support.cli_failures import CLI_FAILURES" + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.confirm", + "replacements": [ + { + "old_source": " self.form.finish()\n if self.query_one(ApplyCommandButton).disabled:\n return\n arguments = self.arguments()\n confirmed = self.query_one(CommandConfirmed).value\n self.query_one(ApplyCommandButton).disabled = True\n await self.app.preparation.run_thread(\n lambda: self.definition.edited(arguments).with_confirmation(confirmed))\n if self.is_attached and arguments == self.arguments():\n self.dismiss(arguments)\n", + "new_source": " button = self.query_one(ApplyCommandButton)\n if button.disabled:\n return\n self.form.finish()\n try:\n await CommandPreparation.wait_on(self)\n except WorkerFailed:\n # The preparation's own handler shows the original failure.\n return\n if not self.is_attached or any(view.editor.feedback for view in self.form.values.values()):\n return\n arguments = self.arguments()\n confirmed = self.query_one(CommandConfirmed).value\n button.disabled = True\n try:\n await self.app.preparation.run_thread(\n lambda: self.definition.edited(arguments).with_confirmation(confirmed))\n except CLI_FAILURES as error:\n self.app.show_error(error, self.definition.label)\n self.update_confirmation()\n return\n if self.is_attached and arguments == self.arguments():\n self.dismiss(arguments)\n" + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/.verification/p189-confirm-loading-form.plan.json b/.verification/p189-confirm-loading-form.plan.json new file mode 100644 index 000000000..ad12bca9b --- /dev/null +++ b/.verification/p189-confirm-loading-form.plan.json @@ -0,0 +1,35 @@ +{ + "recipes": [ + { + "recipe_id": "the-confirm-operation-owns-its-preparation-wait", + "operations": [ + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.update_confirmation", + "replacements": [ + { + "old_source": " if CommandPreparation.running_on(self):\n self.query_one(CommandConfirmation).update(\"Loading choices\u2026\")\n confirmed.display = False\n return", + "new_source": " button = self.query_one(ApplyCommandButton)\n button.loading = CommandPreparation.running_on(self)\n if button.loading:\n self.query_one(CommandConfirmation).update(\"Loading choices\u2026\")\n confirmed.display = False\n # Confirm owns the wait, including an edit committed by this click.\n button.disabled = False\n return" + }, + { + "old_source": "if not self.is_attached or arguments != self.arguments():", + "new_source": "if not self.is_attached or arguments != self.arguments() or CommandPreparation.running_on(self):" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.confirm", + "replacements": [ + { + "old_source": " self.form.finish()\n try:\n await CommandPreparation.wait_on(self)", + "new_source": " self.form.finish()\n button.disabled = True\n button.loading = True\n try:\n await CommandPreparation.wait_on(self)" + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/.verification/p189-preparation-failures.plan.json b/.verification/p189-preparation-failures.plan.json new file mode 100644 index 000000000..de060cbb8 --- /dev/null +++ b/.verification/p189-preparation-failures.plan.json @@ -0,0 +1,47 @@ +{ + "recipes": [ + { + "recipe_id": "keep-preparation-errors-owned-by-the-worker", + "operations": [ + { + "operation": "ensure_import", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "import_source": "from textual.worker import WorkerFailed" + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.prepare_current", + "replacements": [ + { + "old_source": "CommandPreparation.run_on(self, self.refresh_catalogs(self.arguments()))", + "new_source": "CommandPreparation.run_on(self, self.refresh_catalogs(self.arguments()), exit_on_error=False)" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.preparation_changed", + "replacements": [ + { + "old_source": " def preparation_changed(self, event: Worker.StateChanged) -> None:\n if (event.worker.node is self and event.worker.group == CommandPreparation.group\n and event.worker.is_finished and self.is_attached):\n self.update_confirmation()", + "new_source": " async def preparation_changed(self, event: Worker.StateChanged) -> None:\n if (event.worker.node is self and event.worker.group == CommandPreparation.group\n and event.worker.is_finished and self.is_attached):\n try:\n await event.worker.wait()\n except WorkerFailed as error:\n self.query_one(CommandConfirmation).update(\"Could not load choices\")\n self.app.show_error(error.error, self.definition.label)\n else:\n self.update_confirmation()" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.refresh_catalogs", + "replacements": [ + { + "old_source": " try:\n while self.is_attached:\n edited = self.definition.edited(arguments)\n prepared = await self.app.preparation.run_thread(\n lambda: self.core.call(CommsPrepareTargetActionTool(action=edited)))\n if not self.is_attached:\n return\n current = self.arguments()\n if arguments != current:\n arguments = current\n continue\n self.definition = prepared\n for parameter in prepared.fields:\n if parameter.name in self.choices:\n self.choices[parameter.name].parameter = parameter\n self.form.values[parameter.name].editor.paint()\n break\n except (NativePiUnavailable, RouteChangedError) as error:\n self.app.show_error(error, self.definition.label)\n", + "new_source": " while self.is_attached:\n edited = self.definition.edited(arguments)\n prepared = await self.app.preparation.run_thread(\n lambda: self.core.call(CommsPrepareTargetActionTool(action=edited)))\n if not self.is_attached:\n return\n current = self.arguments()\n if arguments != current:\n arguments = current\n continue\n self.definition = prepared\n for parameter in prepared.fields:\n if parameter.name in self.choices:\n self.choices[parameter.name].parameter = parameter\n self.form.values[parameter.name].editor.paint()\n break\n" + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/src/toad/thread_actions.py b/src/toad/thread_actions.py index c3ffd114b..7f02248c3 100644 --- a/src/toad/thread_actions.py +++ b/src/toad/thread_actions.py @@ -137,16 +137,11 @@ def prepare(): async def prepare(): if definition.editable_fields or definition.confirmation: screen = ctx.app.screen - try: - prepared = await ctx.app.preparation.run_thread( - lambda: ctx.observed.core.call(CommsPrepareTargetActionTool(action=definition))) - ctx.current() - except (NativePiUnavailable, RouteChangedError) as error: - ctx.app.show_error(error, definition.label) - return + fields = await ctx.app.preparation.run_thread(lambda: definition.editable_fields) + ctx.current() if ctx.app.screen is screen: ctx.app.push_screen( - CommandDialog(prepared, ctx.title, prepared.fields, ctx.observed.core), accepted) + CommandDialog(definition, ctx.title, fields, ctx.observed.core), accepted) else: await accepted({}) ctx.app.run_worker(prepare, name='command-fields') diff --git a/src/toad/widgets/comms_command_dialog.py b/src/toad/widgets/comms_command_dialog.py index 0f3aa5bcc..5732b9dce 100644 --- a/src/toad/widgets/comms_command_dialog.py +++ b/src/toad/widgets/comms_command_dialog.py @@ -5,16 +5,20 @@ from dataclasses import dataclass, field, make_dataclass from typing import Annotated, get_args, get_origin -from agent_comms.cli_verbs.cli_command import PreparedTargetAction +from agent_comms.cli_support.cli_failures import CLI_FAILURES +from agent_comms.cli_verbs.cli_command import PreparedTargetAction, TargetAction from agent_comms.cli_verbs.target_fields import ChoiceTargetField, TargetField from agent_comms.config_options import ModelConfigOption +from agent_comms.errors import RouteChangedError from agent_comms.execution.login import run_login +from agent_comms.execution.native_failures import NativePiUnavailable from agent_comms.nominal.field_codec import compile_decoder from agent_comms.transport.request import CoreAccess from agent_comms.transport.tools.sidebar import CommsPrepareTargetActionTool from formbridge.forms.declared_fields import DeclaredMetadata, Multiline from objectstate import ObjectState from python_introspect import AnnotationChoices, FieldLabel +from python_introspect.choices import LabelledChoices from textual import on, work from textual.app import ComposeResult from textual.containers import Horizontal, VerticalScroll @@ -23,6 +27,7 @@ from textual.events import Mount, Unmount from textual.widgets import Button, Static +from textual.worker import Worker, WorkerFailed from toad.forms.editors import MultilineEditor, ValueEditor from toad.forms.form_view import InlineFormView from toad.native_style import AutoWidth, EllipsisOverflow, SideMargin @@ -37,18 +42,16 @@ from toad.widgets.acting_button import ConfirmButton, Confirms, DismissButton from toad.widgets.checkbox import Checkbox from toad.widgets.model_selection import ModelSelection +from toad.worker_group import WorkerGroup @dataclass(eq=False) -class PreparedChoices(AnnotationChoices, DeclaredMetadata): +class PreparedChoices(LabelledChoices, DeclaredMetadata): """The catalog last prepared for a draft, not another copy of its value.""" parameter: ChoiceTargetField - def choices(self): - return tuple(value for _, value in self.parameter.choices) - - def label(self, choice): - return next((label for label, value in self.parameter.choices if value == choice), 'Choose…') + def labelled_choices(self): + return self.parameter.choices def declares(self): """The prepared parameter names its own editors first; any choices editor follows.""" @@ -151,15 +154,6 @@ class ApplyCommandAction(ViewAction): async def apply(self, context: CommandDialog) -> None: await context.confirm() - if context.query_one(ApplyCommandButton).disabled: - return - arguments = context.arguments() - confirmed = context.query_one(CommandConfirmed).value - context.query_one(ApplyCommandButton).disabled = True - await context.app.preparation.run_thread( - lambda: context.definition.edited(arguments).with_confirmation(confirmed)) - if context.is_attached and arguments == context.arguments(): - context.dismiss(arguments) class CommandBox(ModalPanel): @@ -206,8 +200,12 @@ class CommandConfirmed(Checkbox): """The user's confirmation of that operation.""" +class CommandPreparation(WorkerGroup): + """One preparation follows the current draft without restarting on each edit.""" + + class CommandDialog(Confirms, ShadedScreen, CenteredScreen, DismissibleScreen[dict[str, str | None]], actions=(ApplyCommandAction,)): - def __init__(self, definition: PreparedTargetAction, target: str, fields: tuple[TargetField, ...], + def __init__(self, definition: TargetAction, target: str, fields: tuple[TargetField, ...], core: CoreAccess): super().__init__() self.definition, self.target, self.core = definition, target, core @@ -239,11 +237,36 @@ def compose(self) -> ComposeResult: yield CancelCommandButton('Cancel') async def confirm(self) -> None: + button = self.query_one(ApplyCommandButton) + if button.disabled: + return self.form.finish() + button.disabled = True + button.loading = True + try: + await CommandPreparation.wait_on(self) + except WorkerFailed: + # The preparation's own handler shows the original failure. + return + if not self.is_attached or any(view.editor.feedback for view in self.form.values.values()): + return + arguments = self.arguments() + confirmed = self.query_one(CommandConfirmed).value + button.disabled = True + try: + await self.app.preparation.run_thread( + lambda: self.definition.edited(arguments).with_confirmation(confirmed)) + except CLI_FAILURES as error: + self.app.show_error(error, self.definition.label) + self.update_confirmation() + return + if self.is_attached and arguments == self.arguments(): + self.dismiss(arguments) @on(Mount) def on_mount(self): self.state.on_resolved_changed(self.draft_changed) + self.prepare_current() self.update_confirmation() @on(Unmount) @@ -251,33 +274,63 @@ def on_unmount(self): self.state.off_resolved_changed(self.draft_changed) def draft_changed(self, paths): + self.prepare_current() self.update_confirmation() - if paths & self.choices.keys(): - self.run_worker(self.refresh_catalogs(self.arguments()), - group='command-catalogs', exclusive=True) + def prepare_current(self) -> None: + if not CommandPreparation.running_on(self): + CommandPreparation.run_on(self, self.refresh_catalogs(self.arguments()), exit_on_error=False) + + @on(Worker.StateChanged) + async def preparation_changed(self, event: Worker.StateChanged) -> None: + if (event.worker.node is self and event.worker.group == CommandPreparation.group + and event.worker.is_finished and self.is_attached): + try: + await event.worker.wait() + except WorkerFailed as error: + self.query_one(CommandConfirmation).update("Could not load choices") + self.app.show_error(error.error, self.definition.label) + else: + self.update_confirmation() + def arguments(self): return {name: self.state.get_resolved_value(name) for name in self.form.values} async def refresh_catalogs(self, arguments): self.query_one(ApplyCommandButton).disabled = True - edited = self.definition.edited(arguments) - fields = (await self.app.preparation.run_thread( - lambda: self.core.call(CommsPrepareTargetActionTool(action=edited)))).fields - if not self.is_attached or arguments != self.arguments(): - return - for parameter in fields: - if parameter.name in self.choices: - self.choices[parameter.name].parameter = parameter - self.form.values[parameter.name].editor.paint() - self.update_confirmation() + while self.is_attached: + edited = self.definition.edited(arguments) + prepared = await self.app.preparation.run_thread( + lambda: self.core.call(CommsPrepareTargetActionTool(action=edited))) + if not self.is_attached: + return + current = self.arguments() + if arguments != current: + arguments = current + continue + self.definition = prepared + for parameter in prepared.fields: + if parameter.name in self.choices: + self.choices[parameter.name].parameter = parameter + self.form.values[parameter.name].editor.paint() + break + if self.is_attached: + self.update_confirmation() def update_confirmation(self): confirmed = self.query_one(CommandConfirmed) confirmed.value = False self.query_one(ApplyCommandButton).disabled = True arguments = self.arguments() + button = self.query_one(ApplyCommandButton) + button.loading = CommandPreparation.running_on(self) + if button.loading: + self.query_one(CommandConfirmation).update("Loading choices…") + confirmed.display = False + # Confirm owns the wait, including an edit committed by this click. + button.disabled = False + return async def prepare(): def read(): for name, choices in self.choices.items(): @@ -285,7 +338,7 @@ def read(): return f'Choose {name.replace("_", " ")}', False return self.definition.edited(arguments).confirmation, True confirmation, valid = await self.app.preparation.run_thread(read) - if not self.is_attached or arguments != self.arguments(): + if not self.is_attached or arguments != self.arguments() or CommandPreparation.running_on(self): return self.query_one(CommandConfirmation).update(confirmation) confirmed.display = valid and bool(confirmation) diff --git a/src/toad/worker_group.py b/src/toad/worker_group.py index c94991274..ba4d24701 100644 --- a/src/toad/worker_group.py +++ b/src/toad/worker_group.py @@ -2,6 +2,7 @@ from __future__ import annotations +import asyncio from collections.abc import Callable from typing import ClassVar @@ -41,10 +42,19 @@ def cancel_all(cls, node: DOMNode) -> list[Worker]: """Every worker of this kind on the node, including ones a newer start superseded.""" return node.workers.cancel_group(node, cls.group) + @classmethod + def workers_on(cls, node: DOMNode) -> tuple[Worker, ...]: + return tuple(worker for worker in node.workers + if worker.node is node and worker.group == cls.group) + + @classmethod + async def wait_on(cls, node: DOMNode) -> None: + """Wait for the node's actual work; its original error still refuses the caller.""" + await asyncio.gather(*(worker.wait() for worker in cls.workers_on(node))) + @classmethod def running_on(cls, node: DOMNode) -> bool: - return any(worker.node is node and worker.group == cls.group and not worker.is_finished - for worker in node.workers) + return any(not worker.is_finished for worker in cls.workers_on(node)) class InputHistoryRecord(WorkerGroup): From d811706b92bd4397212a1bc0d7c8126281e87d3f Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 12 Nov 2026 17:36:30 -0500 Subject: [PATCH 2/3] Keep preparing confirmation clickable and retain the first-click control --- .../p189-first-click-control.plan.json | 15 ++++++ ...keep-preparing-confirm-clickable.plan.json | 31 +++++++++++ src/toad/widgets/comms_command_dialog.py | 4 +- ..._application_actions_in_the_running_app.py | 53 +++++++++++++++++++ 4 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 .verification/p189-first-click-control.plan.json create mode 100644 .verification/p189-keep-preparing-confirm-clickable.plan.json diff --git a/.verification/p189-first-click-control.plan.json b/.verification/p189-first-click-control.plan.json new file mode 100644 index 000000000..8fea3cff6 --- /dev/null +++ b/.verification/p189-first-click-control.plan.json @@ -0,0 +1,15 @@ +{ + "recipes": [ + { + "recipe_id": "exercise-held-preparation-and-first-mouse-confirm", + "operations": [ + { + "operation": "insert_after_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/tests/textual/test_application_actions_in_the_running_app.py", + "target_qualname": "test_new_session_button_opens_creation_and_tab_close_keeps_the_original_draft", + "source": "\n\nasync def test_command_form_opens_before_preparation_and_first_mouse_confirm_submits(workspace):\n from threading import Event\n\n from agent_comms.cli_verbs.cli_command import TargetAction\n from agent_comms.cli_verbs.new import NewThreadCliCommand\n from agent_comms.projects.checkouts import Checkout\n from agent_comms.transport.request import CoreAccess\n from toad.widgets.comms_command_dialog import ApplyCommandButton, CommandDialog, CommandPreparation\n\n entered, released = Event(), Event()\n\n class HeldPreparation(CoreAccess):\n def __init__(self, original):\n self.original = original\n\n def call(self, request):\n entered.set()\n if not released.wait(30):\n raise TimeoutError(\"The test did not release command preparation\")\n return self.original.call(request)\n\n async with running_session(workspace) as (ui, app, config, path):\n project = Checkout(workspace.project)\n command, = NewThreadCliCommand.project_bindings(workspace.comms, project)\n action = TargetAction((command,), (), NewThreadCliCommand)\n observed = await app.preparation.run_thread(app.coordination_access.observation.observed)\n accepted = []\n dialog = CommandDialog(action, str(project), action.editable_fields, HeldPreparation(observed.core))\n app.push_screen(dialog, accepted.append)\n try:\n await ui.until(entered.is_set, why=\"preparation to have entered its held request\")\n assert app.screen is dialog and not released.is_set()\n assert dialog.form.is_mounted\n released.set()\n await ui.until(lambda: not CommandPreparation.running_on(dialog), why=\"the real catalogs to arrive\")\n button = dialog.query_one(ApplyCommandButton)\n await ui.until(lambda: not button.disabled, why=\"the command values to be accepted\")\n editor = dialog.form.values[\"name\"].editor\n editor.scroll_visible(animate=False, immediate=True)\n await ui.until_clickable(editor, why=\"the name editor to be visible\")\n assert await ui.textual[1].click(editor)\n await ui.type(\"created-from-first-click\")\n # Do not blur first: this click must commit the focused editor and submit.\n button.scroll_visible(animate=False, immediate=True)\n await ui.until_clickable(button, why=\"the confirm button to be visible\")\n assert await ui.textual[1].click(button)\n await ui.until(lambda: bool(accepted), why=\"the first mouse confirmation to submit the form\")\n assert accepted[0][\"name\"] == \"created-from-first-click\"\n assert app.screen is app.workspace_screen\n finally:\n released.set()\n" + } + ] + } + ] +} \ No newline at end of file diff --git a/.verification/p189-keep-preparing-confirm-clickable.plan.json b/.verification/p189-keep-preparing-confirm-clickable.plan.json new file mode 100644 index 000000000..2aa84a1c6 --- /dev/null +++ b/.verification/p189-keep-preparing-confirm-clickable.plan.json @@ -0,0 +1,31 @@ +{ + "recipes": [ + { + "recipe_id": "pending-work-must-not-remove-the-confirm-buttons-event-handler", + "operations": [ + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.update_confirmation", + "replacements": [ + { + "old_source": " button.loading = CommandPreparation.running_on(self)\n if button.loading:\n", + "new_source": " if CommandPreparation.running_on(self):\n" + } + ] + }, + { + "operation": "patch_target", + "file_path": "/home/ts/wt/codex-parallel--P189--toad/src/toad/widgets/comms_command_dialog.py", + "target_qualname": "CommandDialog.confirm", + "replacements": [ + { + "old_source": " button.loading = True\n", + "new_source": "" + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/src/toad/widgets/comms_command_dialog.py b/src/toad/widgets/comms_command_dialog.py index 5732b9dce..1cf9c1f82 100644 --- a/src/toad/widgets/comms_command_dialog.py +++ b/src/toad/widgets/comms_command_dialog.py @@ -242,7 +242,6 @@ async def confirm(self) -> None: return self.form.finish() button.disabled = True - button.loading = True try: await CommandPreparation.wait_on(self) except WorkerFailed: @@ -324,8 +323,7 @@ def update_confirmation(self): self.query_one(ApplyCommandButton).disabled = True arguments = self.arguments() button = self.query_one(ApplyCommandButton) - button.loading = CommandPreparation.running_on(self) - if button.loading: + if CommandPreparation.running_on(self): self.query_one(CommandConfirmation).update("Loading choices…") confirmed.display = False # Confirm owns the wait, including an edit committed by this click. diff --git a/tests/textual/test_application_actions_in_the_running_app.py b/tests/textual/test_application_actions_in_the_running_app.py index 5726939ab..9744f149a 100644 --- a/tests/textual/test_application_actions_in_the_running_app.py +++ b/tests/textual/test_application_actions_in_the_running_app.py @@ -136,6 +136,59 @@ async def test_new_session_button_opens_creation_and_tab_close_keeps_the_origina assert ui.draft() == draft and prompt.prompt_text_area.document is document +async def test_command_form_opens_before_preparation_and_first_mouse_confirm_submits(workspace): + from threading import Event + + from agent_comms.cli_verbs.cli_command import TargetAction + from agent_comms.cli_verbs.new import NewThreadCliCommand + from agent_comms.projects.checkouts import Checkout + from agent_comms.transport.request import CoreAccess + from toad.widgets.comms_command_dialog import ApplyCommandButton, CommandDialog, CommandPreparation + + entered, released = Event(), Event() + + class HeldPreparation(CoreAccess): + def __init__(self, original): + self.original = original + + def call(self, request): + entered.set() + if not released.wait(30): + raise TimeoutError("The test did not release command preparation") + return self.original.call(request) + + async with running_session(workspace) as (ui, app, config, path): + project = Checkout(workspace.project) + command, = NewThreadCliCommand.project_bindings(workspace.comms, project) + action = TargetAction((command,), (), NewThreadCliCommand) + observed = await app.preparation.run_thread(app.coordination_access.observation.observed) + accepted = [] + dialog = CommandDialog(action, str(project), action.editable_fields, HeldPreparation(observed.core)) + app.push_screen(dialog, accepted.append) + try: + await ui.until(entered.is_set, why="preparation to have entered its held request") + assert app.screen is dialog and not released.is_set() + assert dialog.form.is_mounted + released.set() + await ui.until(lambda: not CommandPreparation.running_on(dialog), why="the real catalogs to arrive") + button = dialog.query_one(ApplyCommandButton) + await ui.until(lambda: not button.disabled, why="the command values to be accepted") + editor = dialog.form.values["name"].editor + editor.scroll_visible(animate=False, immediate=True) + await ui.until_clickable(editor, why="the name editor to be visible") + assert await ui.textual[1].click(editor) + await ui.type("created-from-first-click") + # Do not blur first: this click must commit the focused editor and submit. + button.scroll_visible(animate=False, immediate=True) + await ui.until_clickable(button, why="the confirm button to be visible") + assert await ui.textual[1].click(button) + await ui.until(lambda: bool(accepted), why="the first mouse confirmation to submit the form") + assert accepted[0]["name"] == "created-from-first-click" + assert app.screen is app.workspace_screen + finally: + released.set() + + async def test_a_settings_file_that_cannot_be_written_is_reported_kept_dirty_and_saved_on_retry(workspace): async with running_session(workspace) as (ui, app, config, path): record_notifications(app) From 9f202b46de1e38b45b26ff74abf58c3b36faf7c3 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 12 Nov 2026 20:52:08 -0500 Subject: [PATCH 3/3] Let submission alone disable its confirmation button --- src/toad/widgets/comms_command_dialog.py | 61 ++++++++++--------- ..._application_actions_in_the_running_app.py | 4 ++ 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/toad/widgets/comms_command_dialog.py b/src/toad/widgets/comms_command_dialog.py index 1cf9c1f82..4e0e383cc 100644 --- a/src/toad/widgets/comms_command_dialog.py +++ b/src/toad/widgets/comms_command_dialog.py @@ -243,24 +243,27 @@ async def confirm(self) -> None: self.form.finish() button.disabled = True try: - await CommandPreparation.wait_on(self) - except WorkerFailed: - # The preparation's own handler shows the original failure. - return - if not self.is_attached or any(view.editor.feedback for view in self.form.values.values()): - return - arguments = self.arguments() - confirmed = self.query_one(CommandConfirmed).value - button.disabled = True - try: - await self.app.preparation.run_thread( - lambda: self.definition.edited(arguments).with_confirmation(confirmed)) - except CLI_FAILURES as error: - self.app.show_error(error, self.definition.label) - self.update_confirmation() - return - if self.is_attached and arguments == self.arguments(): - self.dismiss(arguments) + try: + await CommandPreparation.wait_on(self) + except WorkerFailed: + # Preparation reports its original error through its own handler. + return + if not self.is_attached or any(view.editor.feedback for view in self.form.values.values()): + return + arguments = self.arguments() + confirmed = self.query_one(CommandConfirmed).value + try: + await self.app.preparation.run_thread( + lambda: self.validated_action(arguments).with_confirmation(confirmed)) + except CLI_FAILURES as error: + self.app.show_error(error, self.definition.label) + self.update_confirmation() + return + if self.is_attached and arguments == self.arguments(): + self.dismiss(arguments) + finally: + if self.is_attached: + button.disabled = False @on(Mount) def on_mount(self): @@ -293,11 +296,16 @@ async def preparation_changed(self, event: Worker.StateChanged) -> None: else: self.update_confirmation() + def validated_action(self, arguments) -> TargetAction: + """The same declared catalog constraints guard the preview and submission.""" + for name in self.choices: + self.form.values[name].field.validate(arguments[name]) + return self.definition.edited(arguments) + def arguments(self): return {name: self.state.get_resolved_value(name) for name in self.form.values} async def refresh_catalogs(self, arguments): - self.query_one(ApplyCommandButton).disabled = True while self.is_attached: edited = self.definition.edited(arguments) prepared = await self.app.preparation.run_thread( @@ -320,27 +328,20 @@ async def refresh_catalogs(self, arguments): def update_confirmation(self): confirmed = self.query_one(CommandConfirmed) confirmed.value = False - self.query_one(ApplyCommandButton).disabled = True arguments = self.arguments() - button = self.query_one(ApplyCommandButton) if CommandPreparation.running_on(self): self.query_one(CommandConfirmation).update("Loading choices…") confirmed.display = False - # Confirm owns the wait, including an edit committed by this click. - button.disabled = False return async def prepare(): def read(): - for name, choices in self.choices.items(): - if arguments[name] not in choices.choices(): - return f'Choose {name.replace("_", " ")}', False - return self.definition.edited(arguments).confirmation, True + try: + return self.validated_action(arguments).confirmation, True + except (TypeError, ValueError) as error: + return str(error), False confirmation, valid = await self.app.preparation.run_thread(read) if not self.is_attached or arguments != self.arguments() or CommandPreparation.running_on(self): return self.query_one(CommandConfirmation).update(confirmation) confirmed.display = valid and bool(confirmation) - self.query_one(ApplyCommandButton).disabled = not valid or any( - arguments[name] != choices.parameter.editor_default - for name, choices in self.choices.items()) self.run_worker(prepare, group='command-confirmation', exclusive=True) diff --git a/tests/textual/test_application_actions_in_the_running_app.py b/tests/textual/test_application_actions_in_the_running_app.py index 9744f149a..cc9fa98bd 100644 --- a/tests/textual/test_application_actions_in_the_running_app.py +++ b/tests/textual/test_application_actions_in_the_running_app.py @@ -1,4 +1,5 @@ """Textual-facing: application actions (help, settings screen, command palette, sidebar buttons, tab close) run through Textual screens and keep the session, its draft and the saved settings.""" +from toad.forms.editors import ChoicesEditor from toad.launch_binding import AgentAttached from contextlib import asynccontextmanager @@ -14,6 +15,7 @@ from toad.preferences import SETTINGS from toad.screens.keymap import KeymapScreen from toad.screens.settings import SettingsScreen +from toad.widgets.comms_command_dialog import CatalogField from toad.widgets.comms_sidebar import CommsSidebar, NewSessionButton from toad.widgets.footer import ToadFooter from toad.widgets.session_tabs import SessionTabClose @@ -169,6 +171,8 @@ def call(self, request): await ui.until(entered.is_set, why="preparation to have entered its held request") assert app.screen is dialog and not released.is_set() assert dialog.form.is_mounted + assert isinstance(dialog.form.values["model"].editor, CatalogField) + assert isinstance(dialog.form.values["thinking_level"].editor, ChoicesEditor) released.set() await ui.until(lambda: not CommandPreparation.running_on(dialog), why="the real catalogs to arrive") button = dialog.query_one(ApplyCommandButton)