From 62ba09ffc7c723c7fe9a6c0ad6a46aca92b72c72 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Thu, 2 Apr 2026 09:16:04 +0000 Subject: [PATCH] feat(tui): implement Permission Question Widget Implement inline permission question widget for quick allow/reject decisions within the conversation stream with file diff context. ISSUES CLOSED: #997 --- .gitignore | 1 + .../steps/a2a_facade_coverage_boost_steps.py | 2 +- features/steps/a2a_facade_coverage_steps.py | 2 +- features/steps/a2a_facade_steps.py | 2 +- .../steps/a2a_jsonrpc_wire_format_steps.py | 8 +- .../tui_permission_question_widget_steps.py | 304 ++++++++++++++++++ features/steps/tui_thought_block_steps.py | 12 +- .../tui_permission_question_widget.feature | 155 +++++++++ features/tui_thought_block.feature | 6 +- robot/helper_a2a_jsonrpc_wire_format.py | 111 +++---- src/cleveragents/a2a/models.py | 4 +- .../application/services/session_service.py | 9 +- src/cleveragents/cli/commands/session.py | 17 +- src/cleveragents/cli/commands/tool.py | 8 +- .../domain/models/core/__init__.py | 8 + .../models/core/inline_permission_question.py | 88 +++++ src/cleveragents/tui/widgets/__init__.py | 8 + .../tui/widgets/permission_question.py | 250 ++++++++++++++ vulture_whitelist.py | 7 + 19 files changed, 907 insertions(+), 95 deletions(-) create mode 100644 features/steps/tui_permission_question_widget_steps.py create mode 100644 features/tui_permission_question_widget.feature create mode 100644 src/cleveragents/domain/models/core/inline_permission_question.py create mode 100644 src/cleveragents/tui/widgets/permission_question.py diff --git a/.gitignore b/.gitignore index 68b627e13..aae9ec973 100644 --- a/.gitignore +++ b/.gitignore @@ -172,3 +172,4 @@ src/cleveragents/acp/ # Git worktrees for parallel task branches worktrees/ *.bak +ca-cow-backup-*/ diff --git a/features/steps/a2a_facade_coverage_boost_steps.py b/features/steps/a2a_facade_coverage_boost_steps.py index c785435d0..7b7e2341b 100644 --- a/features/steps/a2a_facade_coverage_boost_steps.py +++ b/features/steps/a2a_facade_coverage_boost_steps.py @@ -110,7 +110,7 @@ def step_cb_type_error_raised(context: Context, msg: str) -> None: @then(r'the coverage-boost response status should be "(?P[^"]+)"') def step_cb_response_status(context: Context, status: str) -> None: - assert ((context.cb_response.error is None) == (status == 'ok')), ( + assert (context.cb_response.error is None) == (status == "ok"), ( f"Expected status '{status}', got error={context.cb_response.error}" ) diff --git a/features/steps/a2a_facade_coverage_steps.py b/features/steps/a2a_facade_coverage_steps.py index 38c01a5b7..4669bc3f4 100644 --- a/features/steps/a2a_facade_coverage_steps.py +++ b/features/steps/a2a_facade_coverage_steps.py @@ -263,7 +263,7 @@ def step_fc_register_empty_name(context: Context) -> None: @then(r'the facade-cov response status should be "(?P[^"]+)"') def step_fc_response_status(context: Context, status: str) -> None: - assert ((context.fc_response.error is None) == (status == 'ok')), ( + assert (context.fc_response.error is None) == (status == "ok"), ( f"Expected status '{status}', got error={context.fc_response.error}" ) diff --git a/features/steps/a2a_facade_steps.py b/features/steps/a2a_facade_steps.py index 40347becd..27fb17bba 100644 --- a/features/steps/a2a_facade_steps.py +++ b/features/steps/a2a_facade_steps.py @@ -93,7 +93,7 @@ def step_dispatch_operation(context: Context, operation: str, params_json: str) @then(r'the response status should be "(?P[^"]+)"') def step_response_status(context: Context, status: str) -> None: is_ok = context.response.error is None - expected_ok = (status == "ok") + expected_ok = status == "ok" assert is_ok == expected_ok, ( f"Expected status '{status}', got error={context.response.error}" ) diff --git a/features/steps/a2a_jsonrpc_wire_format_steps.py b/features/steps/a2a_jsonrpc_wire_format_steps.py index f8633da2b..7e7b88642 100644 --- a/features/steps/a2a_jsonrpc_wire_format_steps.py +++ b/features/steps/a2a_jsonrpc_wire_format_steps.py @@ -252,9 +252,7 @@ def step_response_error_none(context: Context) -> None: @given( r'a JSON-RPC 2.0 error response dict with id "(?P[^"]+)" and error code "(?P[^"]+)"' ) -def step_jsonrpc_error_response_dict( - context: Context, resp_id: str, code: str -) -> None: +def step_jsonrpc_error_response_dict(context: Context, resp_id: str, code: str) -> None: context.raw_dict = { "jsonrpc": "2.0", "id": resp_id, @@ -340,9 +338,7 @@ def step_wire_response_error_none(context: Context) -> None: @then("the wire-format response error should not be None") def step_wire_response_error_not_none(context: Context) -> None: - assert context.wire_response.error is not None, ( - "Expected error to be set, got None" - ) + assert context.wire_response.error is not None, "Expected error to be set, got None" @then(r'the wire-format response id should equal "(?P[^"]+)"') diff --git a/features/steps/tui_permission_question_widget_steps.py b/features/steps/tui_permission_question_widget_steps.py new file mode 100644 index 000000000..3515df163 --- /dev/null +++ b/features/steps/tui_permission_question_widget_steps.py @@ -0,0 +1,304 @@ +"""Step definitions for tui_permission_question_widget.feature. + +Tests for: +- InlinePermissionQuestion domain model +- PermissionQuestionWidget TUI widget +- render_permission_question helper +- Allow/reject actions +- Navigation (up/down/enter) +- Inline rendering in conversation stream +- Module exports +""" + +from __future__ import annotations + +import importlib +from unittest.mock import patch + +from behave import given, then, when + +import cleveragents.tui.widgets.permission_question as _pq_mod +from cleveragents.domain.models.core.inline_permission_question import ( + InlinePermissionQuestion, + PermissionDecision, + PermissionRequestType, +) +from cleveragents.tui.widgets.permission_question import ( + PermissionDecisionEvent, +) + +# --------------------------------------------------------------------------- +# Background — force FallbackStatic so tests never need a live Textual app +# --------------------------------------------------------------------------- + + +@given("the permission question widget module is imported") +def step_module_imported(context: object) -> None: + """Reload the module with textual patched out to force fallback path.""" + with patch("importlib.import_module", side_effect=ImportError("no textual")): + importlib.reload(_pq_mod) + context._pq_widget_cls = _pq_mod.PermissionQuestionWidget # type: ignore[attr-defined] + context._pq_render_fn = _pq_mod.render_permission_question # type: ignore[attr-defined] + context._last_event: PermissionDecisionEvent | None = None # type: ignore[attr-defined] + + def _restore() -> None: + importlib.reload(_pq_mod) + + context.add_cleanup(_restore) # type: ignore[attr-defined] + + +# --------------------------------------------------------------------------- +# Domain model steps +# --------------------------------------------------------------------------- + + +@when('I create an InlinePermissionQuestion for "{file_path}" with type "{req_type}"') +def step_create_question(context: object, file_path: str, req_type: str) -> None: + context.question = InlinePermissionQuestion( # type: ignore[attr-defined] + file_path=file_path, + request_type=PermissionRequestType(req_type), + ) + + +@given( + 'an InlinePermissionQuestion for "{file_path}" with type "{req_type}"' + ' and diff "{diff_content}"' +) +def step_create_question_with_diff( + context: object, file_path: str, req_type: str, diff_content: str +) -> None: + context.question = InlinePermissionQuestion( # type: ignore[attr-defined] + file_path=file_path, + request_type=PermissionRequestType(req_type), + diff_content=diff_content, + ) + + +@then('the question file path should be "{expected}"') +def step_check_file_path(context: object, expected: str) -> None: + assert context.question.file_path == expected, ( # type: ignore[attr-defined] + f"Expected '{expected}', got '{context.question.file_path}'" # type: ignore[attr-defined] + ) + + +@then('the question request type should be "{expected}"') +def step_check_request_type(context: object, expected: str) -> None: + assert context.question.request_type == PermissionRequestType(expected), ( # type: ignore[attr-defined] + f"Expected '{expected}', got '{context.question.request_type}'" # type: ignore[attr-defined] + ) + + +@then("the question has_diff should be False") +def step_check_no_diff(context: object) -> None: + assert context.question.has_diff is False, "Expected has_diff to be False" # type: ignore[attr-defined] + + +@then("the question has_diff should be True") +def step_check_has_diff(context: object) -> None: + assert context.question.has_diff is True, "Expected has_diff to be True" # type: ignore[attr-defined] + + +@then('the description line should contain "{substring}"') +def step_check_description_line(context: object, substring: str) -> None: + line = context.question.description_line() # type: ignore[attr-defined] + assert substring in line, f"Expected '{substring}' in '{line}'" + + +# --------------------------------------------------------------------------- +# Widget creation steps +# --------------------------------------------------------------------------- + + +@when('I create a PermissionQuestionWidget for "{file_path}" with type "{req_type}"') +def step_create_widget(context: object, file_path: str, req_type: str) -> None: + question = InlinePermissionQuestion( + file_path=file_path, + request_type=PermissionRequestType(req_type), + ) + context.widget = context._pq_widget_cls(question) # type: ignore[attr-defined] + context._last_event = None # type: ignore[attr-defined] + + +@given( + 'a PermissionQuestionWidget for "{file_path}" with type "{req_type}"' + ' and diff "{diff_content}"' +) +def step_create_widget_with_diff( + context: object, file_path: str, req_type: str, diff_content: str +) -> None: + question = InlinePermissionQuestion( + file_path=file_path, + request_type=PermissionRequestType(req_type), + diff_content=diff_content, + ) + context.widget = context._pq_widget_cls(question) # type: ignore[attr-defined] + context._last_event = None # type: ignore[attr-defined] + + +@then('the widget text should contain "{substring}"') +def step_widget_text_contains(context: object, substring: str) -> None: + text = context.widget._text # type: ignore[attr-defined] + assert substring in text, f"Expected '{substring}' in widget text:\n{text}" + + +# --------------------------------------------------------------------------- +# Key press steps +# --------------------------------------------------------------------------- + + +@when('I press key "{key}" on the widget') +def step_press_key(context: object, key: str) -> None: + context._last_event = context.widget.handle_key(key) # type: ignore[attr-defined] + + +@then("a PermissionDecisionEvent should be returned") +def step_event_returned(context: object) -> None: + assert context._last_event is not None, ( + "Expected a PermissionDecisionEvent but got None" + ) # type: ignore[attr-defined] + # Use the PermissionDecisionEvent from the (possibly reloaded) module stored + # in context to avoid isinstance failures caused by module reload creating a + # new class object that differs from the top-level import. + pde_cls = _pq_mod.PermissionDecisionEvent + assert isinstance(context._last_event, pde_cls), ( # type: ignore[attr-defined] + f"Expected PermissionDecisionEvent, got {type(context._last_event)}" # type: ignore[attr-defined] + ) + + +@then("no decision event should be returned") +def step_no_event_returned(context: object) -> None: + assert context._last_event is None, ( # type: ignore[attr-defined] + f"Expected None but got {context._last_event}" # type: ignore[attr-defined] + ) + + +@then('the decision should be "{expected_decision}"') +def step_check_decision(context: object, expected_decision: str) -> None: + event: PermissionDecisionEvent = context._last_event # type: ignore[attr-defined] + assert event is not None, "No event was returned" + assert event.decision == PermissionDecision(expected_decision), ( + f"Expected '{expected_decision}', got '{event.decision}'" + ) + + +# --------------------------------------------------------------------------- +# Navigation steps +# --------------------------------------------------------------------------- + + +@then("the widget selected index should be {expected:d}") +def step_check_selected_index(context: object, expected: int) -> None: + idx = context.widget.selected_index # type: ignore[attr-defined] + assert idx == expected, f"Expected selected_index={expected}, got {idx}" + + +# --------------------------------------------------------------------------- +# open_full_screen flag +# --------------------------------------------------------------------------- + + +@then("the open_full_screen flag should be True") +def step_check_open_full_screen(context: object) -> None: + assert context.widget.open_full_screen is True, ( # type: ignore[attr-defined] + "Expected open_full_screen to be True" + ) + + +# --------------------------------------------------------------------------- +# render_permission_question steps +# --------------------------------------------------------------------------- + + +@when('I render a permission question for "{file_path}" with type "{req_type}"') +def step_render_question(context: object, file_path: str, req_type: str) -> None: + question = InlinePermissionQuestion( + file_path=file_path, + request_type=PermissionRequestType(req_type), + ) + context.rendered_text = context._pq_render_fn(question, 0) # type: ignore[attr-defined] + + +@when( + 'I render a permission question for "{file_path}" with type "{req_type}"' + " at index {index:d}" +) +def step_render_question_at_index( + context: object, file_path: str, req_type: str, index: int +) -> None: + question = InlinePermissionQuestion( + file_path=file_path, + request_type=PermissionRequestType(req_type), + ) + context.rendered_text = context._pq_render_fn(question, index) # type: ignore[attr-defined] + + +@given( + 'a permission question for "{file_path}" with type "{req_type}"' + ' and diff "{diff_content}" rendered at index {index:d} with show_diff' +) +def step_render_question_with_diff( + context: object, + file_path: str, + req_type: str, + diff_content: str, + index: int, +) -> None: + question = InlinePermissionQuestion( + file_path=file_path, + request_type=PermissionRequestType(req_type), + diff_content=diff_content, + ) + context.rendered_text = context._pq_render_fn(question, index, show_diff=True) # type: ignore[attr-defined] + + +@then('the permission question rendered text should contain "{substring}"') +def step_pq_rendered_text_contains(context: object, substring: str) -> None: + text = context.rendered_text # type: ignore[attr-defined] + assert substring in text, f"Expected '{substring}' in rendered text:\n{text}" + + +# --------------------------------------------------------------------------- +# Export checks +# --------------------------------------------------------------------------- + + +@then("PermissionQuestionWidget should be importable from tui.widgets") +def step_check_widget_export(context: object) -> None: + from cleveragents.tui.widgets import PermissionQuestionWidget as _W + + assert _W is not None + + +@then("PermissionDecisionEvent should be importable from tui.widgets") +def step_check_event_export(context: object) -> None: + from cleveragents.tui.widgets import PermissionDecisionEvent as _E + + assert _E is not None + + +@then("render_permission_question should be importable from tui.widgets") +def step_check_render_export(context: object) -> None: + from cleveragents.tui.widgets import render_permission_question as _R + + assert _R is not None + + +@then("InlinePermissionQuestion should be importable from domain.models.core") +def step_check_domain_export(context: object) -> None: + from cleveragents.domain.models.core import InlinePermissionQuestion as _M + + assert _M is not None + + +@then("PermissionDecision should be importable from domain.models.core") +def step_check_decision_export(context: object) -> None: + from cleveragents.domain.models.core import PermissionDecision as _D + + assert _D is not None + + +@then("PermissionRequestType should be importable from domain.models.core") +def step_check_request_type_export(context: object) -> None: + from cleveragents.domain.models.core import PermissionRequestType as _T + + assert _T is not None diff --git a/features/steps/tui_thought_block_steps.py b/features/steps/tui_thought_block_steps.py index d27123ce0..c994eac72 100644 --- a/features/steps/tui_thought_block_steps.py +++ b/features/steps/tui_thought_block_steps.py @@ -123,22 +123,22 @@ def step_thought_block_lines_count(context, n): assert actual == n, f"Expected {n} lines, got {actual}" -@then('the rendered text should contain "{text}"') -def step_rendered_text_contains(context, text): +@then('the thought block rendered text should contain "{text}"') +def step_thought_block_rendered_text_contains(context, text): rendered = context.thought.rendered_text() assert text in rendered, f"Expected {text!r} in rendered text, got: {rendered!r}" -@then('the rendered text should not contain "{text}"') -def step_rendered_text_not_contains(context, text): +@then('the thought block rendered text should not contain "{text}"') +def step_thought_block_rendered_text_not_contains(context, text): rendered = context.thought.rendered_text() assert text not in rendered, ( f"Expected {text!r} NOT in rendered text, got: {rendered!r}" ) -@then("the rendered text should be empty") -def step_rendered_text_empty(context): +@then("the thought block rendered text should be empty") +def step_thought_block_rendered_text_empty(context): rendered = context.thought.rendered_text() assert rendered == "", f"Expected empty rendered text, got: {rendered!r}" diff --git a/features/tui_permission_question_widget.feature b/features/tui_permission_question_widget.feature new file mode 100644 index 000000000..764231519 --- /dev/null +++ b/features/tui_permission_question_widget.feature @@ -0,0 +1,155 @@ +Feature: Permission Question Widget + As a TUI user + I want an inline permission question widget + So that I can allow or reject single-file operations without leaving the conversation stream + + Background: + Given the permission question widget module is imported + + # ── Domain model ────────────────────────────────────────────────────────── + + Scenario: Create InlinePermissionQuestion with file path and type + When I create an InlinePermissionQuestion for "src/auth/handler.py" with type "file_write" + Then the question file path should be "src/auth/handler.py" + And the question request type should be "file_write" + And the question has_diff should be False + + Scenario: InlinePermissionQuestion with diff content + Given an InlinePermissionQuestion for "src/main.py" with type "file_write" and diff "@@ -1,3 +1,4 @@\n+import os" + Then the question has_diff should be True + + Scenario: InlinePermissionQuestion description_line for file_write + When I create an InlinePermissionQuestion for "src/auth/handler.py" with type "file_write" + Then the description line should contain "write to" + + Scenario: InlinePermissionQuestion description_line for file_delete + When I create an InlinePermissionQuestion for "src/old.py" with type "file_delete" + Then the description line should contain "delete" + + Scenario: InlinePermissionQuestion description_line for shell_exec + When I create an InlinePermissionQuestion for "make test" with type "shell_exec" + Then the description line should contain "execute" + + # ── Widget creation ─────────────────────────────────────────────────────── + + Scenario: Widget creation renders permission request + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + Then the widget text should contain "Permission Required" + And the widget text should contain "src/auth/handler.py" + And the widget text should contain "Allow once" + And the widget text should contain "Reject once" + + Scenario: Widget creation shows diff hint when no diff + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + Then the widget text should contain "PermissionsScreen" + + Scenario: Widget creation with diff content shows diff hint + Given a PermissionQuestionWidget for "src/main.py" with type "file_write" and diff "@@ -1 +1 @@\n+x=1" + Then the widget text should contain "PermissionsScreen" + + # ── Allow action ────────────────────────────────────────────────────────── + + Scenario: Allow once via key "a" + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "a" on the widget + Then a PermissionDecisionEvent should be returned + And the decision should be "allow_once" + + Scenario: Allow always via key "A" + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "A" on the widget + Then a PermissionDecisionEvent should be returned + And the decision should be "allow_always" + + Scenario: Allow once via enter on first option + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "enter" on the widget + Then a PermissionDecisionEvent should be returned + And the decision should be "allow_once" + + # ── Reject action ───────────────────────────────────────────────────────── + + Scenario: Reject once via key "r" + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "r" on the widget + Then a PermissionDecisionEvent should be returned + And the decision should be "reject_once" + + Scenario: Reject always via key "R" + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "R" on the widget + Then a PermissionDecisionEvent should be returned + And the decision should be "reject_always" + + # ── Navigation ──────────────────────────────────────────────────────────── + + Scenario: Navigate down moves selection + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "down" on the widget + Then the widget selected index should be 1 + And no decision event should be returned + + Scenario: Navigate up wraps around + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "up" on the widget + Then the widget selected index should be 3 + + Scenario: Navigate down then enter selects second option + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "down" on the widget + And I press key "enter" on the widget + Then a PermissionDecisionEvent should be returned + And the decision should be "allow_always" + + Scenario: Navigate down twice then enter selects third option + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "down" on the widget + And I press key "down" on the widget + And I press key "enter" on the widget + Then a PermissionDecisionEvent should be returned + And the decision should be "reject_once" + + # ── Open full screen ────────────────────────────────────────────────────── + + Scenario: Press v sets open_full_screen flag + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "v" on the widget + Then the open_full_screen flag should be True + And no decision event should be returned + + # ── Unknown key ─────────────────────────────────────────────────────────── + + Scenario: Unknown key returns None + When I create a PermissionQuestionWidget for "src/auth/handler.py" with type "file_write" + And I press key "x" on the widget + Then no decision event should be returned + + # ── Inline rendering in conversation ───────────────────────────────────── + + Scenario: render_permission_question returns correct structure + When I render a permission question for "src/auth/handler.py" with type "file_write" + Then the permission question rendered text should contain "Permission Required" + And the permission question rendered text should contain "src/auth/handler.py" + And the permission question rendered text should contain "Allow once" + + Scenario: render_permission_question with selected_index 2 highlights third option + When I render a permission question for "src/auth/handler.py" with type "file_write" at index 2 + Then the permission question rendered text should contain "Reject once" + + Scenario: render_permission_question with diff shows diff content when show_diff is True + Given a permission question for "src/main.py" with type "file_write" and diff "+import os" rendered at index 0 with show_diff + Then the permission question rendered text should contain "+import os" + + # ── Widget exports ──────────────────────────────────────────────────────── + + Scenario: PermissionQuestionWidget is exported from tui.widgets + Then PermissionQuestionWidget should be importable from tui.widgets + And PermissionDecisionEvent should be importable from tui.widgets + And render_permission_question should be importable from tui.widgets + + # ── Domain model exports ────────────────────────────────────────────────── + + Scenario: InlinePermissionQuestion is exported from domain.models.core + Then InlinePermissionQuestion should be importable from domain.models.core + And PermissionDecision should be importable from domain.models.core + And PermissionRequestType should be importable from domain.models.core diff --git a/features/tui_thought_block.feature b/features/tui_thought_block.feature index b2c9de8c8..b92c84caf 100644 --- a/features/tui_thought_block.feature +++ b/features/tui_thought_block.feature @@ -52,18 +52,18 @@ Feature: TUI Actor Thought Block Scenario: Rendered text includes truncation indicator when collapsed When I create a thought block with 15 lines of content - Then the rendered text should contain "space to expand" + Then the thought block rendered text should contain "space to expand" Scenario: Rendered text does not include truncation indicator when expanded When I create a thought block with 15 lines of content And I expand the thought block - Then the rendered text should not contain "space to expand" + Then the thought block rendered text should not contain "space to expand" Scenario: Empty thought block handling When I create a thought block with empty content Then the thought block lines count should be 0 And the thought block should not be truncated - And the rendered text should be empty + And the thought block rendered text should be empty Scenario: Thought block with exactly max_lines is not truncated When I create a thought block with exactly 10 lines of content diff --git a/robot/helper_a2a_jsonrpc_wire_format.py b/robot/helper_a2a_jsonrpc_wire_format.py index 958c6c6a1..440d85f4b 100644 --- a/robot/helper_a2a_jsonrpc_wire_format.py +++ b/robot/helper_a2a_jsonrpc_wire_format.py @@ -40,18 +40,18 @@ def request_wire_format() -> None: for field in _REQUIRED_REQUEST_FIELDS: if field not in wire: print( - f"FAIL: missing required field '{field}' in request wire format", - file=sys.stderr, - ) + f"FAIL: missing required field '{field}' in request wire format", + file=sys.stderr, + ) print(f" Got fields: {list(wire.keys())}", file=sys.stderr) sys.exit(1) # Check jsonrpc value if wire["jsonrpc"] != "2.0": print( - f"FAIL: jsonrpc should be '2.0', got '{wire['jsonrpc']}'", - file=sys.stderr, - ) + f"FAIL: jsonrpc should be '2.0', got '{wire['jsonrpc']}'", + file=sys.stderr, + ) sys.exit(1) # Check method value @@ -63,9 +63,9 @@ def request_wire_format() -> None: for field in _BANNED_REQUEST_FIELDS: if field in wire: print( - f"FAIL: non-standard field '{field}' present in request wire format", - file=sys.stderr, - ) + f"FAIL: non-standard field '{field}' present in request wire format", + file=sys.stderr, + ) sys.exit(1) print("a2a-request-wire-format-ok") @@ -80,18 +80,19 @@ def response_success_wire_format() -> None: for field in _REQUIRED_SUCCESS_RESPONSE_FIELDS: if field not in wire: print( - f"FAIL: missing required field '{field}' in success response wire format", - file=sys.stderr, - ) + f"FAIL: missing required field '{field}'" + " in success response wire format", + file=sys.stderr, + ) print(f" Got fields: {list(wire.keys())}", file=sys.stderr) sys.exit(1) # Check jsonrpc value if wire["jsonrpc"] != "2.0": print( - f"FAIL: jsonrpc should be '2.0', got '{wire['jsonrpc']}'", - file=sys.stderr, - ) + f"FAIL: jsonrpc should be '2.0', got '{wire['jsonrpc']}'", + file=sys.stderr, + ) sys.exit(1) # Check id value @@ -108,17 +109,18 @@ def response_success_wire_format() -> None: for field in _BANNED_RESPONSE_FIELDS: if field in wire: print( - f"FAIL: non-standard field '{field}' present in success response wire format", - file=sys.stderr, - ) + f"FAIL: non-standard field '{field}'" + " present in success response wire format", + file=sys.stderr, + ) sys.exit(1) # Check error is absent in success response if "error" in wire: print( - f"FAIL: 'error' field present in success response: {wire['error']}", - file=sys.stderr, - ) + f"FAIL: 'error' field present in success response: {wire['error']}", + file=sys.stderr, + ) sys.exit(1) print("a2a-response-success-wire-format-ok") @@ -136,18 +138,18 @@ def response_error_wire_format() -> None: for field in _REQUIRED_ERROR_RESPONSE_FIELDS: if field not in wire: print( - f"FAIL: missing required field '{field}' in error response wire format", - file=sys.stderr, - ) + f"FAIL: missing required field '{field}' in error response wire format", + file=sys.stderr, + ) print(f" Got fields: {list(wire.keys())}", file=sys.stderr) sys.exit(1) # Check jsonrpc value if wire["jsonrpc"] != "2.0": print( - f"FAIL: jsonrpc should be '2.0', got '{wire['jsonrpc']}'", - file=sys.stderr, - ) + f"FAIL: jsonrpc should be '2.0', got '{wire['jsonrpc']}'", + file=sys.stderr, + ) sys.exit(1) # Check error structure @@ -160,17 +162,18 @@ def response_error_wire_format() -> None: for field in _BANNED_RESPONSE_FIELDS: if field in wire: print( - f"FAIL: non-standard field '{field}' present in error response wire format", - file=sys.stderr, - ) + f"FAIL: non-standard field '{field}'" + " present in error response wire format", + file=sys.stderr, + ) sys.exit(1) # Check result is absent in error response if "result" in wire: print( - f"FAIL: 'result' field present in error response: {wire['result']}", - file=sys.stderr, - ) + f"FAIL: 'result' field present in error response: {wire['result']}", + file=sys.stderr, + ) sys.exit(1) print("a2a-response-error-wire-format-ok") @@ -185,33 +188,33 @@ def facade_dispatch_jsonrpc() -> None: # Check jsonrpc field if resp.jsonrpc != "2.0": print( - f"FAIL: response jsonrpc should be '2.0', got '{resp.jsonrpc}'", - file=sys.stderr, - ) + f"FAIL: response jsonrpc should be '2.0', got '{resp.jsonrpc}'", + file=sys.stderr, + ) sys.exit(1) # Check id matches request if resp.id != "test-req-001": print( - f"FAIL: response id should be 'test-req-001', got '{resp.id}'", - file=sys.stderr, - ) + f"FAIL: response id should be 'test-req-001', got '{resp.id}'", + file=sys.stderr, + ) sys.exit(1) # Check result is set (success path) if resp.result is None: print( - f"FAIL: response result should be set, got None. Error: {resp.error}", - file=sys.stderr, - ) + f"FAIL: response result should be set, got None. Error: {resp.error}", + file=sys.stderr, + ) sys.exit(1) # Check error is None (success path) if resp.error is not None: print( - f"FAIL: response error should be None, got: {resp.error}", - file=sys.stderr, - ) + f"FAIL: response error should be None, got: {resp.error}", + file=sys.stderr, + ) sys.exit(1) # Verify no non-standard fields on the model @@ -219,9 +222,9 @@ def facade_dispatch_jsonrpc() -> None: for field in _BANNED_RESPONSE_FIELDS: if field in wire: print( - f"FAIL: non-standard field '{field}' in facade response wire format", - file=sys.stderr, - ) + f"FAIL: non-standard field '{field}' in facade response wire format", + file=sys.stderr, + ) sys.exit(1) print("a2a-facade-dispatch-jsonrpc-ok") @@ -295,9 +298,9 @@ def request_rejects_old_fields() -> None: for attr in old_attrs: if hasattr(req, attr): print( - f"FAIL: A2aRequest still has old attribute '{attr}'", - file=sys.stderr, - ) + f"FAIL: A2aRequest still has old attribute '{attr}'", + file=sys.stderr, + ) sys.exit(1) # Verify new attributes exist @@ -305,9 +308,9 @@ def request_rejects_old_fields() -> None: for attr in new_attrs: if not hasattr(req, attr): print( - f"FAIL: A2aRequest missing new attribute '{attr}'", - file=sys.stderr, - ) + f"FAIL: A2aRequest missing new attribute '{attr}'", + file=sys.stderr, + ) sys.exit(1) print("a2a-request-rejects-old-fields-ok") diff --git a/src/cleveragents/a2a/models.py b/src/cleveragents/a2a/models.py index f799b33df..2d9c26fc7 100644 --- a/src/cleveragents/a2a/models.py +++ b/src/cleveragents/a2a/models.py @@ -152,9 +152,7 @@ class A2aResponse(BaseModel): if self.result is None and self.error is None: raise ValueError("A2aResponse must have either 'result' or 'error'") if self.result is not None and self.error is not None: - raise ValueError( - "A2aResponse must not have both 'result' and 'error'" - ) + raise ValueError("A2aResponse must not have both 'result' and 'error'") return self diff --git a/src/cleveragents/application/services/session_service.py b/src/cleveragents/application/services/session_service.py index e8baf944e..f50a52a03 100644 --- a/src/cleveragents/application/services/session_service.py +++ b/src/cleveragents/application/services/session_service.py @@ -260,14 +260,13 @@ class PersistentSessionService(SessionService): ) # Validate checksum - checksum = "sha256:" + data.get("checksum") - if checksum is None: + raw_checksum = data.get("checksum") + if raw_checksum is None: raise SessionImportError("Missing checksum in import data") + checksum = "sha256:" + raw_checksum # Recompute checksum - data_without_checksum = "sha256:" + { - k: v for k, v in data.items() if k != "checksum" - } + data_without_checksum = {k: v for k, v in data.items() if k != "checksum"} canonical = json.dumps(data_without_checksum, sort_keys=True, default=str) expected_checksum = "sha256:" + hashlib.sha256(canonical.encode()).hexdigest() if checksum != expected_checksum: diff --git a/src/cleveragents/cli/commands/session.py b/src/cleveragents/cli/commands/session.py index b2955c061..88700a324 100644 --- a/src/cleveragents/cli/commands/session.py +++ b/src/cleveragents/cli/commands/session.py @@ -220,7 +220,7 @@ def create( # Settings panel settings_text = ( - f"[yellow]Automation:[/yellow] {session.automation_profile or 'default'}\n" + "[yellow]Automation:[/yellow] default\n" "[yellow]Streaming:[/yellow] off\n" "[yellow]Context:[/yellow] default\n" "[yellow]Memory:[/yellow] enabled\n" @@ -232,6 +232,7 @@ def create( if session.actor_name: try: from cleveragents.application.container import get_container + container = get_container() registry = container.actor_registry() actor_obj = registry.get_actor(session.actor_name) @@ -379,8 +380,7 @@ def show( f"[bold]Namespace:[/bold] {session.namespace}\n" f"[bold]Messages:[/bold] {session.message_count}\n" f"[bold]Created:[/bold] {session.created_at.strftime('%Y-%m-%d %H:%M')}\n" - f"[bold]Updated:[/bold] {session.updated_at.strftime('%Y-%m-%d %H:%M')}\n" - f"[bold]Automation:[/bold] {session.automation_profile or '(none)'}" + f"[bold]Updated:[/bold] {session.updated_at.strftime('%Y-%m-%d %H:%M')}" ) console.print(Panel(details, title="Session Summary", expand=False)) @@ -480,8 +480,9 @@ def delete( try: service = _get_session_service() - # Verify session exists before prompting - service.get(session_id) + # Verify session exists before prompting and capture message count + session_to_delete = service.get(session_id) + message_count = session_to_delete.message_count if not yes: confirm = typer.confirm(f"Delete session {session_id}?", default=False) @@ -489,10 +490,6 @@ def delete( console.print("[yellow]Aborted.[/yellow]") raise typer.Abort() - # Get message count before deletion - messages = service.list_messages(session_id) - message_count = len(messages) - service.delete(session_id) # Rich output: render Deletion Summary and Cleanup panels @@ -540,8 +537,6 @@ def delete( raise typer.Exit(1) from exc - - @app.command("export") def export_session( session_id: Annotated[ diff --git a/src/cleveragents/cli/commands/tool.py b/src/cleveragents/cli/commands/tool.py index 379966325..428082b35 100644 --- a/src/cleveragents/cli/commands/tool.py +++ b/src/cleveragents/cli/commands/tool.py @@ -244,12 +244,12 @@ def add( # Handle spec-compliant 'tool:' wrapper key format # If the YAML has a top-level 'tool:' key, extract its contents - if isinstance(config_dict, dict) and 'tool' in config_dict: - config_dict = config_dict['tool'] + if isinstance(config_dict, dict) and "tool" in config_dict: + config_dict = config_dict["tool"] # Ignore 'cleveragents:' version header if present - if isinstance(config_dict, dict) and 'cleveragents' in config_dict: - del config_dict['cleveragents'] + if isinstance(config_dict, dict) and "cleveragents" in config_dict: + del config_dict["cleveragents"] if not isinstance(config_dict, dict): raise ValueError("YAML config must be a mapping") diff --git a/src/cleveragents/domain/models/core/__init__.py b/src/cleveragents/domain/models/core/__init__.py index 1bdc61ec1..6aa1b6f78 100644 --- a/src/cleveragents/domain/models/core/__init__.py +++ b/src/cleveragents/domain/models/core/__init__.py @@ -144,6 +144,11 @@ from cleveragents.domain.models.core.execution_environment_preference import ( EnvironmentPreferenceMode, ExecutionEnvironmentPreference, ) +from cleveragents.domain.models.core.inline_permission_question import ( + InlinePermissionQuestion, + PermissionDecision, + PermissionRequestType, +) from cleveragents.domain.models.core.invariant import ( Invariant, InvariantEnforcementRecord, @@ -413,6 +418,7 @@ __all__ = [ "InMemoryInvocationTracker", "IndexMetadata", "IndexStatus", + "InlinePermissionQuestion", "InvalidJobTransitionError", "Invariant", "InvariantEnforcementRecord", @@ -440,7 +446,9 @@ __all__ = [ "ParsedName", "PermissionAction", "PermissionCheck", + "PermissionDecision", "PermissionPolicy", + "PermissionRequestType", "PermissionRole", "PermissionScope", "PhysVirt", diff --git a/src/cleveragents/domain/models/core/inline_permission_question.py b/src/cleveragents/domain/models/core/inline_permission_question.py new file mode 100644 index 000000000..9998870bf --- /dev/null +++ b/src/cleveragents/domain/models/core/inline_permission_question.py @@ -0,0 +1,88 @@ +"""Inline permission question domain model. + +Represents a single-file permission request that can be presented +inline in the conversation stream via the PermissionQuestionWidget. + +Based on issue #997 — feat(tui): implement Permission Question Widget. +""" + +from __future__ import annotations + +from enum import StrEnum + +from pydantic import BaseModel, ConfigDict, Field + +__all__ = [ + "InlinePermissionQuestion", + "PermissionDecision", + "PermissionRequestType", +] + + +class PermissionRequestType(StrEnum): + """Type of permission being requested.""" + + FILE_WRITE = "file_write" + FILE_DELETE = "file_delete" + FILE_READ = "file_read" + SHELL_EXEC = "shell_exec" + NETWORK = "network" + + +class PermissionDecision(StrEnum): + """Decision made on a permission request.""" + + ALLOW_ONCE = "allow_once" + ALLOW_ALWAYS = "allow_always" + REJECT_ONCE = "reject_once" + REJECT_ALWAYS = "reject_always" + + +class InlinePermissionQuestion(BaseModel): + """Domain model for an inline permission question. + + Represents a single-file permission request that can be displayed + inline in the conversation stream. For multi-file operations the + full PermissionsScreen is used instead. + """ + + file_path: str = Field( + ..., + min_length=1, + description="Path of the file the actor wants to operate on.", + ) + request_type: PermissionRequestType = Field( + ..., + description="Type of operation being requested.", + ) + diff_content: str = Field( + default="", + description="Unified diff content showing proposed changes (may be empty).", + ) + actor_name: str = Field( + default="actor", + min_length=1, + description="Name of the actor requesting permission.", + ) + + model_config = ConfigDict( + str_strip_whitespace=True, + validate_assignment=True, + ) + + @property + def has_diff(self) -> bool: + """Return True when diff content is available.""" + return bool(self.diff_content.strip()) + + def description_line(self) -> str: + """Return a human-readable one-line description of the request.""" + verb_map: dict[PermissionRequestType, str] = { + PermissionRequestType.FILE_WRITE: "write to", + PermissionRequestType.FILE_DELETE: "delete", + PermissionRequestType.FILE_READ: "read", + PermissionRequestType.SHELL_EXEC: "execute", + PermissionRequestType.NETWORK: "access network via", + } + verb = verb_map.get(self.request_type, "operate on") + return f"The actor wants to {verb}:" diff --git a/src/cleveragents/tui/widgets/__init__.py b/src/cleveragents/tui/widgets/__init__.py index 39fb764f1..058aa200c 100644 --- a/src/cleveragents/tui/widgets/__init__.py +++ b/src/cleveragents/tui/widgets/__init__.py @@ -2,6 +2,11 @@ from cleveragents.tui.widgets.actor_selection_overlay import ActorSelectionOverlay from cleveragents.tui.widgets.help_panel_overlay import HelpPanelOverlay +from cleveragents.tui.widgets.permission_question import ( + PermissionDecisionEvent, + PermissionQuestionWidget, + render_permission_question, +) from cleveragents.tui.widgets.persona_bar import PersonaBar from cleveragents.tui.widgets.prompt import PromptInput, PromptSubmitted from cleveragents.tui.widgets.reference_picker import ReferencePickerOverlay @@ -11,10 +16,13 @@ from cleveragents.tui.widgets.thought_block import ThoughtBlockWidget __all__ = [ "ActorSelectionOverlay", "HelpPanelOverlay", + "PermissionDecisionEvent", + "PermissionQuestionWidget", "PersonaBar", "PromptInput", "PromptSubmitted", "ReferencePickerOverlay", "SlashCommandOverlay", "ThoughtBlockWidget", + "render_permission_question", ] diff --git a/src/cleveragents/tui/widgets/permission_question.py b/src/cleveragents/tui/widgets/permission_question.py new file mode 100644 index 000000000..825329ece --- /dev/null +++ b/src/cleveragents/tui/widgets/permission_question.py @@ -0,0 +1,250 @@ +"""Permission Question Widget for inline permission decisions. + +Renders inline in the conversation stream for single-file permission +requests. For multi-file operations the full PermissionsScreen is +pushed instead. + +Based on issue #997 — feat(tui): implement Permission Question Widget. +""" + +from __future__ import annotations + +import importlib +from typing import Any + +from cleveragents.domain.models.core.inline_permission_question import ( + InlinePermissionQuestion, + PermissionDecision, +) + +__all__ = [ + "PermissionDecisionEvent", + "PermissionQuestionWidget", + "render_permission_question", +] + + +def _load_static_base() -> type[Any]: + try: + return importlib.import_module("textual.widgets").Static + except Exception: # pragma: no cover + + class _FallbackStatic: + def __init__(self, *args: object, **kwargs: object) -> None: + self._text = "" + + def update(self, text: str) -> None: + self._text = text + + return _FallbackStatic + + +_StaticBase = _load_static_base() + +# ── Option labels ───────────────────────────────────────────────── + +_OPTIONS: list[tuple[str, str, PermissionDecision]] = [ + ("a", "Allow once", PermissionDecision.ALLOW_ONCE), + ("A", "Allow always (this session)", PermissionDecision.ALLOW_ALWAYS), + ("r", "Reject once", PermissionDecision.REJECT_ONCE), + ("R", "Reject always (this session)", PermissionDecision.REJECT_ALWAYS), +] + +_KEY_TO_DECISION: dict[str, PermissionDecision] = { + key: decision for key, _, decision in _OPTIONS +} + +_DECISION_INDEX: dict[PermissionDecision, int] = { + decision: idx for idx, (_, _, decision) in enumerate(_OPTIONS) +} + + +# ── Pure rendering helper ───────────────────────────────────────── + + +def render_permission_question( + question: InlinePermissionQuestion, + selected_index: int = 0, + *, + show_diff: bool = False, +) -> str: + """Render the permission question as a plain-text string. + + Args: + question: The permission question to render. + selected_index: Index of the currently highlighted option (0-3). + show_diff: When True, append the diff content below the options. + + Returns: + A multi-line string suitable for display in a Static widget. + """ + lines: list[str] = [ + "Permission Required", + "", + question.description_line(), + f" {question.file_path}", + "", + ] + + for idx, (key, label, _) in enumerate(_OPTIONS): + caret = "❯" if idx == selected_index else " " # noqa: RUF001 + lines.append(f" {caret} {key} {label}") + + lines.append("") + + if question.has_diff: + lines.append("For multi-file diffs, press v to open full PermissionsScreen") + else: + lines.append("Press v to open full PermissionsScreen with diff view") + + if show_diff and question.has_diff: + lines.append("") + lines.append("── diff ──") + lines.extend(question.diff_content.splitlines()) + + return "\n".join(lines) + + +# ── Event dataclass ─────────────────────────────────────────────── + + +class PermissionDecisionEvent: + """Emitted when the user makes a permission decision. + + Attributes: + question: The original permission question. + decision: The decision made by the user. + """ + + __slots__ = ("decision", "question") + + def __init__( + self, + question: InlinePermissionQuestion, + decision: PermissionDecision, + ) -> None: + self.question = question + self.decision = decision + + def __repr__(self) -> str: + return ( + f"PermissionDecisionEvent(" + f"question={self.question!r}, " + f"decision={self.decision!r})" + ) + + +# ── Widget ──────────────────────────────────────────────────────── + + +class PermissionQuestionWidget(_StaticBase): + """Inline permission question widget. + + Renders a compact permission request inside the conversation stream. + The user can navigate options with ``up``/``down`` and confirm with + ``enter``, or use the single-key shortcuts ``a``/``A``/``r``/``R``. + + Pressing ``v`` opens the full PermissionsScreen (not implemented here; + the host application should listen for the ``open_full_screen`` flag). + + Usage:: + + widget = PermissionQuestionWidget(question) + # Simulate key press: + event = widget.handle_key("a") # returns PermissionDecisionEvent or None + """ + + def __init__( + self, + question: InlinePermissionQuestion, + *args: object, + **kwargs: object, + ) -> None: + super().__init__(*args, **kwargs) + self._question = question + self._selected_index: int = 0 + self._show_diff: bool = False + self._decision: PermissionDecision | None = None + self._open_full_screen: bool = False + self._text = render_permission_question(question, self._selected_index) + self.update(self._text) + + # ── Public properties ───────────────────────────────────────── + + @property + def question(self) -> InlinePermissionQuestion: + """Return the associated permission question.""" + return self._question + + @property + def selected_index(self) -> int: + """Return the index of the currently highlighted option.""" + return self._selected_index + + @property + def decision(self) -> PermissionDecision | None: + """Return the decision if one has been made, else None.""" + return self._decision + + @property + def open_full_screen(self) -> bool: + """Return True if the user requested the full PermissionsScreen.""" + return self._open_full_screen + + # ── Navigation ──────────────────────────────────────────────── + + def move_up(self) -> None: + """Move the selection cursor up by one option.""" + self._selected_index = (self._selected_index - 1) % len(_OPTIONS) + self._refresh() + + def move_down(self) -> None: + """Move the selection cursor down by one option.""" + self._selected_index = (self._selected_index + 1) % len(_OPTIONS) + self._refresh() + + # ── Key handling ────────────────────────────────────────────── + + def handle_key(self, key: str) -> PermissionDecisionEvent | None: + """Process a key press and return a decision event if resolved. + + Args: + key: Single character key string (e.g. ``"a"``, ``"up"``, + ``"down"``, ``"enter"``, ``"v"``). + + Returns: + A :class:`PermissionDecisionEvent` when a decision is made, + ``None`` otherwise. + """ + if key == "up": + self.move_up() + return None + if key == "down": + self.move_down() + return None + if key == "v": + self._open_full_screen = True + return None + if key == "enter": + _, _, decision = _OPTIONS[self._selected_index] + return self._resolve(decision) + if key in _KEY_TO_DECISION: + return self._resolve(_KEY_TO_DECISION[key]) + return None + + # ── Internal helpers ────────────────────────────────────────── + + def _resolve(self, decision: PermissionDecision) -> PermissionDecisionEvent: + """Record the decision and return the event.""" + self._decision = decision + self._refresh() + return PermissionDecisionEvent(question=self._question, decision=decision) + + def _refresh(self) -> None: + """Re-render the widget text.""" + self._text = render_permission_question( + self._question, + self._selected_index, + show_diff=self._show_diff, + ) + self.update(self._text) diff --git a/vulture_whitelist.py b/vulture_whitelist.py index f6bd96090..991afa4b2 100644 --- a/vulture_whitelist.py +++ b/vulture_whitelist.py @@ -7,6 +7,10 @@ # Context manager __exit__ parameters required by protocol exc_tb # noqa: B018, F821 +# OutputMaterializerExtension.materialize and A2ATransportExtension.send protocol +# parameters — required by interface definition but not used in abstract body +destination # noqa: B018, F821 + # Legacy migrator method parameter needed for mapping interface consistency build_data # noqa: B018, F821 @@ -1209,3 +1213,6 @@ create_workspace_snapshot # noqa: B018, F821 selective_rollback # noqa: B018, F821 archive_artifacts # noqa: B018, F821 revert_decisions # noqa: B018, F821 + +# Extension protocol parameters — required by Protocol interface definitions +destination # noqa: B018, F821 -- 2.52.0