fix(a2a): address PR #11053 review blockers and CI failures

- Fix Python syntax error in a2a_facade_coverage_steps.py line 376:
  use single-quote outer string for regex containing escaped double-quotes
- Fix Robot Framework integration test: replace hardcoded /tmp/fix-repo/
  library paths with ${CURDIR}/helper_session_id_validation_integration.py
- Refactor helper module to expose top-level snake_case keyword functions
  instead of class-based assertions (Robot Framework auto-maps these)
- Update consolidated_misc.feature and m6_autonomy_acceptance.feature to
  expect error status for empty session_id (regression guard from fix)

CHANGELOG.md

@@ -122,6 +122,8 @@ Changed `wf10_batch.robot` to be less likely to create files, and
 - **Plan Rollback Command** (#8557): Implemented `agents plan rollback <plan-id> [<checkpoint-id>]` for checkpoint-based plan state restoration in Epic #8493. The command restores a plan's sandbox to the state captured at a given checkpoint, discarding all decisions made after that checkpoint. The checkpoint can be specified as an optional positional second argument or via the `--to-checkpoint` named option. Supports `--yes/-y` flag to skip confirmation prompts and `--format/-f` for output format selection (rich/plain/json/yaml). Included with comprehensive BDD test coverage (>= 97%) and spec-aligned output formatting showing rollback summary, changes reverted, impact analysis, and post-rollback state panels.
 
 ### Fixed
+- **a2a `_handle_session_close` validates `session_id` before devcontainer cleanup** (#9094): Moved the `session_id` validation check to the very top of `_handle_session_close` in `src/cleveragents/a2a/facade.py`. Previously, when no `session_service` was wired, the handler skipped directly to `_cleanup_session_devcontainers(session_id)` without checking whether `session_id` was non-empty. This allowed callers to bypass input validation by omitting the service dependency. The fix ensures `ValueError("session_id is required")` is raised regardless of service availability, closing a validation bypass path while preserving all devcontainer cleanup and deletion behaviour.
+
 - **Guard cleanup_stale against execute/processing and execute/complete plans** (#11121):
   ``_create_sandbox_for_plan()`` in ``src/cleveragents/cli/commands/plan.py`` now
   skips ``GitWorktreeSandbox.cleanup_stale()`` when the plan is in
@@ -291,6 +293,7 @@ Changed `wf10_batch.robot` to be less likely to create files, and
 
 ### Fixed
 
+
 - **Error suppression removed from `reactive_registry_adapter.py`** (#9060): Removed two `try...except Exception:` blocks in `register_registry_agents()` that silently suppressed errors, violating the CONTRIBUTING.md fail-fast policy. Exceptions from `actor_registry.list_actors()` and the route bridge refresh now propagate to the caller instead of being swallowed. Added Behave scenarios verifying RuntimeError, AttributeError, and TypeError propagation.
 
 - **Implementation Supervisor PR Compliance Checklist** (#9824): Added a mandatory
@@ -402,6 +405,7 @@ _ALL_DATA_COLUMNS + ") " "SELECT " + _ALL_DATA_COLUMNS + " FROM v3_plans"`.
 
 ### Fixed
 
+
 - **`invariant_enforced` decisions not propagated to child plans on subplan spawn** (#9131):
   Fixed `SubplanService.spawn()` to propagate all `invariant_enforced` decisions from the
   parent plan's decision tree to each child plan's decision tree. Previously, child plans
@@ -860,6 +864,7 @@ Documentation Report (Cycle N)` issues every 10 cycles (~3.3 hours). The manager
 
 ### Fixed
 
+
 - **Plan Concurrency Race Condition** (#7989): Fixed critical race condition in `execute_plan()` and
   `apply_plan()` where concurrent CLI/worker sessions could simultaneously modify the same plan,
   corrupting plan state. `LockService` is now wired into the plan lifecycle with plan-level advisory
@@ -973,6 +978,7 @@ iteration` and data corruption under concurrent plan execution. All public
 
 ### Fixed
 
+
 - **CLI (`agents actor remove`)** (#6491): Restores output parity with the
   other actor commands by honoring `--format`/`-f` for JSON/YAML/plain/Rich
   envelopes. Adds a Robot Framework regression test to assert the JSON

CONTRIBUTORS.md

@@ -39,6 +39,7 @@ Below are some of the specific details of various contributions.
 * HAL 9000 has contributed comprehensive milestone documentation for v3.6.0 (Advanced Concepts & Deferred Features) and v3.7.0 (TUI Implementation) (PR #9903): split into sub-documents covering context strategies, LLM backends, resource types, A2A rename, container tool execution, scope chain resolution, cost/safety budgets, E2E workflow tests, code review examples, plugin architecture, TUI layout, persona system, reference/command input, session management, configuration, and TuiMaterializer integration.
 * HAL 9000 has contributed the LLMTraceRepository data-integrity fix (PR #8185 / issue #7505): replaced the unconditional `session.commit()` in `LLMTraceRepository.save()` with a dual-path implementation that respects the UnitOfWork pattern — flushing only when an external session is provided, and flushing + committing + closing when operating standalone. This eliminates premature transaction commits, loss of rollback capability, and a docstring/implementation mismatch.
 * HAL 9000 has contributed the ACMS Index Data Model and File Traversal Engine (PR #9664 / issue #9579): foundational data structures for indexed context entries with hot/warm/cold/archive storage tier classification, tag system, and a timeout-safe chunked file traversal engine for large projects with 10,000+ files.
+* HAL 9000 has contributed the A2A session close `session_id` validation fix (PR #11053 / issue #9094): moved input validation to the entry of `_handle_session_close` so that empty/null session IDs are rejected before any devcontainer cleanup or service operations, preventing callers from bypassing validation by omitting the `session_service` dependency. Added BDD coverage for empty and missing session_id error paths with Robot Framework integration tests.
 * HAL 9000 has contributed the error-suppression removal fix (PR #9247 / issue #9060): removed both `try...except Exception:` blocks in `register_registry_agents()` that silently suppressed errors from `actor_registry.list_actors()` and the route bridge refresh, enabling exceptions to propagate per CONTRIBUTING.md fail-fast policy. Added three Behave scenarios verifying RuntimeError, AttributeError, and TypeError propagation.
 * HAL 9000 has contributed the Strategize phase full context snapshot fix (issue #9056): added `_build_strategize_context_snapshot()` helper to `PlanLifecycleService`, updated `_try_record_decision()` to accept and forward a `ContextSnapshot` parameter, and added BDD test coverage verifying all four `ContextSnapshot` fields (`hot_context_hash`, `hot_context_ref`, `actor_state_ref`, `relevant_resources`) are populated during the Strategize phase.
 * HAL 9000 has contributed the ACMS context path matching fix (PR #10975 / issue #10972): corrects `_path_matches()` and `_matches_pattern()` to properly match absolute fragment paths against relative glob patterns by auto-prefixing with `**/` before calling `PurePath.full_match()`, preventing silent inefficacy of include/exclude filters for absolute paths in fragment metadata.

features/a2a_facade_coverage.feature

@@ -19,11 +19,19 @@ Feature: A2A local facade coverage — uncovered handler and edge-case paths
     Then the facade-cov response status should be "ok"
     And the facade-cov response data key "status" should equal "closed"
 
-  Scenario: Session close with empty session_id and no service
+  @tdd_issue @tdd_issue_9094
+  Scenario: Session close with empty session_id and no service raises validation error
     Given a facade-cov facade with no services
-    When I dispatch facade-cov operation "session.close" with params {"session_id": ""}
-    Then the facade-cov response status should be "ok"
-    And the facade-cov response data key "status" should equal "closed"
+    When I dispatch facade-cov operation "session.close" with params {"session_id": ""} expecting error response
+    Then the facade-cov response status should be "error"
+    And the facade-cov response error message should contain "session_id is required"
+
+  @tdd_issue @tdd_issue_9094
+  Scenario: Session close with missing session_id key and no service raises validation error
+    Given a facade-cov facade with no services
+    When I dispatch facade-cov operation "session.close" with params {} expecting error response
+    Then the facade-cov response status should be "error"
+    And the facade-cov response error message should contain "session_id is required"
 
   # -------------------------------------------------------------------
   # Plan cancel — with service wired (lines 501-502)

features/consolidated_misc.feature

@@ -40,11 +40,11 @@ Feature: Consolidated Misc
     And response data key "status" equals "created"
 
 
-  Scenario: Dispatch session.close returns status closed
+  @tdd_issue @tdd_issue_9094
+  Scenario: Dispatch session.close with empty session_id returns validation error
     Given a new A2aLocalFacade with no services
     When I dispatch operation "session.close" with params {}
-    Then the response status should be "ok"
-    And response data key "status" equals "closed"
+    Then the response status should be "error"
 
 
   Scenario: Dispatch plan.create returns plan_id and status

features/m6_autonomy_acceptance.feature

@@ -34,10 +34,10 @@ Feature: M6 autonomy acceptance smoke tests
     And the m6 smoke response data should contain key "session_id"
     And the m6 smoke response data should contain key "status"
 
-  Scenario: M6 smoke A2A session close returns closed status
+  @tdd_issue @tdd_issue_9094
+  Scenario: M6 smoke A2A session close with empty session_id returns validation error
     When I m6 smoke dispatch "session.close" with params {}
-    Then the m6 smoke response status should be "ok"
-    And the m6 smoke response data "status" should equal "closed"
+    Then the m6 smoke response status should be "error"
 
   # --- A2A facade plan operations (AC-1: plan lifecycle) ---
 

features/steps/a2a_facade_coverage_steps.py

@@ -22,7 +22,7 @@ Targets uncovered lines in ``src/cleveragents/a2a/facade.py``:
 | 169-170   | dispatch TypeError for non-request                         |
 | 212-220   | register_service + cache invalidation                      |
 | 315-317   | session create with service                                |
-| 329-335   | session close with service (empty + valid session_id)      |
+| 329-341   | session close — all validation/error paths                 |
 | 486-490   | event subscribe with wired queue                           |
 | 441-445   | registry list tools with service                           |
 | 451-462   | registry list resources with service                       |
@@ -242,6 +242,22 @@ def step_fc_dispatch_non_request(context: Context) -> None:
         context.fc_caught_error = exc
 
 
+@when(
+    r'I dispatch facade-cov operation "(?P<operation>[^"]+)" '
+    r'with params (?P<params_json>.+) expecting error response'
+)
+def step_fc_dispatch_expect_error(context: Context, operation: str, params_json: str) -> None:
+    """Dispatch and always capture the A2aResponse (errors are returned, not raised).
+
+    This is the robust variant used by session-id validation scenarios that expect
+    an error response rather than a caught exception — _dispatch catches internal
+    exceptions and converts them to error responses.
+    """
+    params: dict[str, Any] = json.loads(params_json)
+    request = A2aRequest(method=operation, params=params)
+    context.fc_response = context.fc_facade.dispatch(request)
+
+
 @when(r'I register a facade-cov service "(?P<name>[^"]+)" with a mock')
 def step_fc_register_service(context: Context, name: str) -> None:
     context.fc_facade.register_service(name, _build_mock_session_service())
@@ -263,6 +279,7 @@ def step_fc_register_empty_name(context: Context) -> None:
 
 @then(r'the facade-cov response status should be "(?P<status>[^"]+)"')
 def step_fc_response_status(context: Context, status: str) -> None:
+    assert context.fc_response is not None, "fc_response was not set before status check"
     assert (context.fc_response.error is None) == (status == "ok"), (
         f"Expected status '{status}', got error={context.fc_response.error}"
     )
@@ -272,18 +289,21 @@ def step_fc_response_status(context: Context, status: str) -> None:
     r'the facade-cov response data key "(?P<key>[^"]+)" should equal "(?P<value>[^"]+)"'
 )
 def step_fc_data_key_equals(context: Context, key: str, value: str) -> None:
+    assert context.fc_response is not None, "fc_response was not set before data check"
     actual = context.fc_response.result.get(key)
     assert str(actual) == value, f"Expected data['{key}'] = '{value}', got '{actual}'"
 
 
 @then(r'the facade-cov response data key "(?P<key>[^"]+)" should be true')
 def step_fc_data_key_true(context: Context, key: str) -> None:
+    assert context.fc_response is not None, "fc_response was not set before data check"
     actual = context.fc_response.result.get(key)
     assert actual is True, f"Expected data['{key}'] to be True, got {actual!r}"
 
 
 @then(r'the facade-cov response data should contain key "(?P<key>[^"]+)"')
 def step_fc_data_has_key(context: Context, key: str) -> None:
+    assert context.fc_response is not None, "fc_response was not set before data check"
     assert key in context.fc_response.result, (
         f"Expected key '{key}' in response data, "
         f"got: {list(context.fc_response.result.keys())}"
@@ -292,6 +312,7 @@ def step_fc_data_has_key(context: Context, key: str) -> None:
 
 @then(r'the facade-cov response data should not contain key "(?P<key>[^"]+)"')
 def step_fc_data_no_key(context: Context, key: str) -> None:
+    assert context.fc_response is not None, "fc_response was not set before check"
     assert key not in context.fc_response.result, (
         f"Expected key '{key}' not in response data, but found it"
     )
@@ -299,12 +320,14 @@ def step_fc_data_no_key(context: Context, key: str) -> None:
 
 @then(r"the facade-cov response error should not be None")
 def step_fc_error_not_none(context: Context) -> None:
+    assert context.fc_response is not None, "fc_response was not set before error check"
     assert context.fc_response.error is not None, "Expected an error in the response"
 
 
 @then(r"the facade-cov response should have timing_ms set")
 def step_fc_timing_set(context: Context) -> None:
     # timing_ms removed from wire format per JSON-RPC 2.0 compliance
+    assert context.fc_response is not None, "fc_response was not set"
     assert context.fc_response.result is not None, "Expected result to be set"
 
 
@@ -329,23 +352,38 @@ def step_fc_value_error_raised(context: Context) -> None:
 def step_fc_session_create_used_service(context: Context) -> None:
     # After registering a new service and dispatching again,
     # the response should reflect the mock service return value
+    assert context.fc_response is not None
     assert context.fc_response.error is None
     assert context.fc_response.result["session_id"] == "MOCK-SESSION-001"
 
 
 @then(r"the facade-cov response tools list should have (?P<count>\d+) items")
 def step_fc_tools_count(context: Context, count: str) -> None:
+    assert context.fc_response is not None
     tools = context.fc_response.result.get("tools", [])
     assert len(tools) == int(count), f"Expected {count} tools, got {len(tools)}"
 
 
 @then(r"the facade-cov response resources list should have (?P<count>\d+) items")
 def step_fc_resources_count(context: Context, count: str) -> None:
+    assert context.fc_response is not None
     resources = context.fc_response.result.get("resources", [])
     assert len(resources) == int(count), (
         f"Expected {count} resources, got {len(resources)}"
     )
 
 
+@then(r'the facade-cov response error message should contain (?P<msg>[^\"]+)')
+def step_fc_error_message(context: Context, msg: str) -> None:
+    """Assert that the error response includes a message matching `msg`."""
+    assert context.fc_response is not None
+    assert context.fc_response.error is not None, "Expected an error in the response"
+    # The A2aResponse wraps exceptions; check repr for the expected message.
+    error_text = str(context.fc_response.error)
+    assert msg in error_text, (
+        f"Expected error message containing '{msg}', got '{error_text}'"
+    )
+
+
 # Reset step matcher to parse (default) so subsequent step files are not affected
 use_step_matcher("parse")

robot/helper_session_id_validation_integration.py

@@ -0,0 +1,95 @@
+"""Robot keyword helper for session_id_validation_integration.robot.
+
+Exposes top-level functions that Robot Framework auto-imports as keywords
+(ROBOT_LISTENER_API_VERSION=2).  Module-level state persists across
+keyword calls within a single test, keeping helpers thin while allowing
+each robot test case to share a facade / response context.
+
+Keywords used by session_id_validation_integration.robot:
+    A2a Local Facade Should Have No Services (line 21)
+    A2a Local Facade With Mock Session Service (line 34)
+    Dispatch Facade Operation (line 56, returns ${response})
+    Response Status Should Be (line 78)
+    Response Error Should Not Be None (line 86)
+    Response Error Message Should Contain (line 92)
+    Response Data Key Should Equal (line 103)
+"""
+
+from __future__ import annotations
+
+import json
+from typing import Any
+
+# ---------------------------------------------------------------------------
+# Module-level shared state for keyword callers (set & read by helpers).
+# ---------------------------------------------------------------------------
+_robot_facade: Any = None
+_robot_response: Any = None
+
+
+def a2a_local_facade_should_have_no_services() -> None:
+    """Create an A2aLocalFacade with no services wired."""
+    global _robot_facade  # noqa: PLW0603
+    from cleveragents.a2a.facade import A2aLocalFacade
+
+    _robot_facade = A2aLocalFacade()
+
+
+def a2a_local_facade_with_mock_session_service() -> None:
+    """Create an A2aLocalFacade with a mock SessionService wired in."""
+    global _robot_facade  # noqa: PLW0603
+    from cleveragents.a2a.facade import A2aLocalFacade
+    from unittest.mock import MagicMock
+
+    class _MockSession:
+        session_id = "MOCK-SESSION-001"
+
+    svc = MagicMock()
+    svc.create.return_value = _MockSession()
+    svc.delete.return_value = None
+
+    _robot_facade = A2aLocalFacade(services={"session_service": svc})
+
+
+def dispatch_facade_operation(operation: str, params_json: str) -> Any:
+    """Dispatch an operation and return the response.
+
+    The facade is read from module-level state set by a setup keyword.
+    """
+    global _robot_response  # noqa: PLW0603
+    from cleveragents.a2a.models import A2aRequest
+
+    params: dict[str, Any] = json.loads(params_json)
+    request = A2aRequest(method=operation, params=params)
+    _robot_response = _robot_facade.dispatch(request)  # type: ignore[arg-type]
+    return _robot_response
+
+
+# ---------------------------------------------------------------------------
+# Response assertion keywords (Robot maps snake_case to Title Case With Spaces).
+# ---------------------------------------------------------------------------
+
+
+def response_status_should_be(expected: str) -> None:
+    """Assert the dispatch response status is ``expected``."""
+    assert (_robot_response.error is None) == (expected == "ok"), (
+        f"Expected status '{expected}', got error={_robot_response.error}"  # type: ignore[union-attr]
+    )
+
+
+def response_error_should_not_be_none() -> None:
+    """Assert the dispatch response contains an error."""
+    assert _robot_response.error is not None, "Expected an error in the response"  # type: ignore[union-attr]
+
+
+def response_error_message_should_contain(text: str) -> None:
+    """Assert the error message contains ``text``."""
+    assert _robot_response.error is not None, "Expected an error in the response"  # type: ignore[union-attr]
+    error_text = str(_robot_response.error)
+    assert text in error_text, f"Expected error message containing '{text}', got '{error_text}'"
+
+
+def response_data_key_should_equal(key: str, value: str) -> None:
+    """Assert ``response.result[key]`` equals ``value``."""
+    actual = _robot_response.result.get(key)  # type: ignore[union-attr]
+    assert str(actual) == value, f"Expected data['{key}'] = '{value}', got '{actual!r}'"

robot/session_id_validation_integration.robot

@@ -0,0 +1,52 @@
+*** Settings ***
+Documentation    Robot Framework integration tests for A2A session close session_id validation.
+
+    Verifies that _handle_session_close validates session_id BEFORE any service checks
+    or devcontainer cleanup, preventing callers from bypassing input validation by
+    omitting the session_service dependency (issue #9094).
+
+Library    ${CURDIR}/helper_session_id_validation_integration.py
+
+*** Variables ***
+${SESSION_ID_VALIDATION_TIMEOUT}=    30
+
+*** Test Cases ***
+A2A Session Close With Empty Session ID Returns Error Response
+    [Documentation]
+        Verifies that closing a session with an empty string session_id returns
+        an error response, because validation happens at entry before any
+        service checks.
+    A2a Local Facade Should Have No Services
+    ${response}=    Dispatch Facade Operation    session.close    {"session_id": ""}
+    Response Status Should Be    error
+    Response Error Should Not Be None
+    Response Error Message Should Contain    session_id is required
+
+A2A Session Close With Missing Session ID Returns Error Response
+    [Documentation]
+        Verifies that closing a session without providing a session_id key at all
+        also returns an error response with the same validation message.
+    A2a Local Facade Should Have No Services
+    ${response}=    Dispatch Facade Operation    session.close    {}
+    Response Status Should Be    error
+    Response Error Should Not Be None
+    Response Error Message Should Contain    session_id is required
+
+A2A Session Close With Valid Session ID Succeeds Without Service
+    [Documentation]
+        Verifies that with a valid (non-empty) session_id the close operation
+        succeeds even without a wired SessionService, because the validation
+        guard at entry passes and cleanup runs as best-effort.
+    A2a Local Facade Should Have No Services
+    ${response}=    Dispatch Facade Operation    session.close    {"session_id": "orphan-test-sess-001"}
+    Response Status Should Be    ok
+    Response Data Key Should Equal    status    closed
+
+A2A Session Close With Valid Session ID Succeeds With Service
+    [Documentation]
+        Verifies that the normal happy path — valid session_id with a wired
+        SessionService — still works correctly after the validation fix.
+    A2a Local Facade With Mock Session Service
+    ${response}=    Dispatch Facade Operation    session.close    {"session_id": "SESS-WIRED-001"}
+    Response Status Should Be    ok
+    Response Data Key Should Equal    status    closed

src/cleveragents/a2a/facade.py

@@ -354,6 +354,12 @@ class A2aLocalFacade:
     def _handle_session_close(self, params: dict[str, Any]) -> dict[str, Any]:
         session_id = params.get("session_id", "")
 
+        # Validate session_id at entry — before any service checks or
+        # devcontainer cleanup — so callers cannot bypass validation by
+        # omitting the session_service dependency.
+        if not session_id:
+            raise ValueError("session_id is required")
+
         svc = self._session_service
         if svc is None:
             # R7-F4 fix: still run container cleanup even without a
@@ -361,8 +367,6 @@ class A2aLocalFacade:
             self._cleanup_session_devcontainers(session_id)
             return {"status": "closed"}
 
-        if not session_id:
-            raise ValueError("session_id is required")
         svc.delete(session_id)
 
         # R7-F4 fix: run container cleanup after session deletion.