Python: clear function invocation budget state from session on failed turn

docs/superpowers/plans/2026-11-06-issue-9677-tool-approval-budget-cleanup.md

@@ -0,0 +1,95 @@
+# Tool Approval Budget State Cleanup on Turn Failure Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Ensure `ToolApprovalMiddleware` per-attempt function-invocation budget state in `session.state` is cleaned up after provider errors, timeouts, and cancellations, preventing budget exhaustion on retry turns while preserving resumable pending approvals.
+
+**Architecture:** In `_tools.py`, integrate `_clear_budget_state_from_session` into exception, task cancellation, and generator exit paths of the tool execution loop. The cleanup will check `invocation_session.state[_TOOL_APPROVAL_STATE_KEY]`: if active approval requests or pending pause batches are waiting for human resolution, the budget is preserved; otherwise, stale per-turn iteration and duration counts are purged.
+
+**Tech Stack:** Python 3.10+, `pytest`, `pytest-asyncio`, `agent_framework`.
+
+**Spec:** GitHub Issue #9677 (https://github.com/microsoft/agent-framework/issues/9677), triage planning directive by maintainer `eavanvalkenburg`.
+
+## Global Constraints
+
+- Never clear budget state when a run pauses for human-in-the-loop approval (`_PENDING_APPROVAL_REQUESTS_KEY` or `_PENDING_MIXED_PAUSE_BATCH_KEY`).
+- Maintain exact streaming and non-streaming parity.
+- Must not touch public agent APIs, middleware interfaces, or `AgentSession` contracts.
+- Tests must use mocked LLM chat clients without cloud credentials or network access.
+
+## Review Focus
+
+- Turn retries immediately following a transient provider exception.
+- Turn retries following an `asyncio.CancelledError` or client timeout.
+- Preservation of pending approval requests across session serialization.
+- Streaming generator early-break / generator close cleanup.
+- Preservation of legitimate cross-turn multi-agent memory in `session.state`.
+
+---
+
+### Task 1: Add Reproducing Regression Tests for Issue #9677
+
+**Files:**
+- Modify/Create: `python/packages/core/tests/core/test_tool_approval_budget.py`
+
+**Interfaces:**
+- Consumes: `Agent`, `AgentSession`, `ToolApprovalMiddleware`, mock chat client.
+- Produces: Test functions validating budget clearance on failure and budget retention on approval pause.
+
+- [ ] **Step 1: Write reproducing unit tests**
+
+```python
+import pytest
+from agent_framework import Agent, AgentSession, Message
+from agent_framework._harness._tool_approval import ToolApprovalMiddleware
+from agent_framework._tools import _FUNCTION_INVOCATION_BUDGET_STATE_KEY
+
+
+@pytest.mark.asyncio
+async def test_failed_turn_clears_budget_state_on_session(mock_failing_chat_client) -> None:
+    session = AgentSession()
+    agent = Agent(
+        client=mock_failing_chat_client,
+        middleware=[ToolApprovalMiddleware()],
+    )
+
+    with pytest.raises(RuntimeError, match="transient provider error"):
+        await agent.run("call tool", session=session)
+
+    # Budget key must be cleared from session after failure
+    assert _FUNCTION_INVOCATION_BUDGET_STATE_KEY not in session.state
+```
+
+- [ ] **Step 2: Run test to verify failure on current main**
+
+Run: `uv run pytest packages/core/tests/core/test_tool_approval_budget.py`
+Expected: FAIL with `assert '_function_invocation_budget_state' not in session.state` failing.
+
+---
+
+### Task 2: Implement Unwinding Budget Cleanup in `_tools.py`
+
+**Files:**
+- Modify: `python/packages/core/agent_framework/_tools.py`
+
+**Interfaces:**
+- Consumes: `_clear_budget_state_from_session(invocation_session, budget_state)`
+- Produces: Hardened error and cancellation handlers in runtime invocation loops.
+
+- [ ] **Step 1: Update non-streaming tool invocation loop error handling**
+
+Ensure that on any caught unhandled exception, task cancellation, or abnormal loop termination, `_clear_budget_state_from_session(invocation_session, budget_state)` is invoked in a `finally` or exception boundary.
+
+- [ ] **Step 2: Update streaming generator termination handlers**
+
+Ensure that streaming generator cancellation and generator `aclose()` trigger the same `_clear_budget_state_from_session` check.
+
+- [ ] **Step 3: Run regression tests to verify pass**
+
+Run: `uv run pytest packages/core/tests/core/test_tool_approval_budget.py`
+Expected: PASS.
+
+- [ ] **Step 4: Run full core test suite**
+
+Run: `uv run pytest packages/core/tests/core/test_tools.py`
+Expected: All tests pass.

docs/superpowers/plans/2026-11-06-issue-9819-windows-shell-parsing.md

@@ -0,0 +1,160 @@
+# Windows Command Line Tokenization for LocalShellTool Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Fix Windows shell path and argument tokenization in `LocalShellTool` and `AGENT_FRAMEWORK_SHELL` by replacing POSIX `shlex.split` with a Windows-native `CommandLineToArgvW` tokenizer when running on or targeting Windows.
+
+**Architecture:** Implement a specialized `_split_windows_command_line(cmd: str) -> list[str]` tokenizer in `_resolve.py` that preserves literal backslashes, double-quoted substrings with embedded whitespace, escaped quotes (`\"`), and explicit empty arguments (`""`). Route string-based shell resolution through this tokenizer when `sys.platform == "win32"` while preserving standard POSIX `shlex.split` behavior on Unix platforms.
+
+**Tech Stack:** Python 3.10+, `pytest`, `agent_framework_tools.shell`.
+
+**Spec:** GitHub Issue #9819 (https://github.com/microsoft/agent-framework/issues/9819), maintainer review on closed PR #9858.
+
+## Global Constraints
+
+- Never alter POSIX shell parsing on Linux or macOS (`shlex.split` remains active).
+- Never add new external dependencies; use standard library parsing logic.
+- Must satisfy `CommandLineToArgvW` rules: $2N$ backslashes before a quote yield $N$ backslashes without escaping the quote; $2N+1$ backslashes before a quote yield $N$ backslashes plus a literal quote. Backslashes elsewhere are literal.
+- Explicit empty quoted arguments (e.g. `""`) must produce an empty string element `""` in argv.
+- Regression tests must run deterministically on Linux/macOS CI using `monkeypatch.setattr(resolve_module.sys, "platform", "win32")`.
+
+## Review Focus
+
+- Unquoted Windows paths containing spaces and backslashes (e.g., `C:\Program Files\PowerShell\7\pwsh.exe`).
+- Adjacent quoted and unquoted fragments (e.g., `--rcfile="C:\Program Files\bash.rc"`).
+- Explicit empty argument tokens (e.g., `--flag "" -v`).
+- Escaped quotation marks within quoted parameters (e.g., `--opt="val\"ue"`).
+- Network UNC paths with consecutive leading backslashes (e.g., `\\server\share\tool.exe`).
+
+---
+
+### Task 1: Add Reproducing Regression Tests in `test_shell_resolve.py`
+
+**Files:**
+- Modify: `python/packages/tools/tests/test_shell_resolve.py`
+
+**Interfaces:**
+- Consumes: `agent_framework_tools.shell._resolve.resolve_shell`
+- Produces: Test functions validating Windows path preservation and argument tokenization.
+
+- [ ] **Step 1: Write the failing tests**
+
+```python
+def test_win32_shell_override_preserves_backslashes(monkeypatch: pytest.MonkeyPatch) -> None:
+    import agent_framework_tools.shell._resolve as resolve_module
+
+    monkeypatch.setattr(resolve_module.sys, "platform", "win32")
+    path = r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe"
+    argv = resolve_shell(path, interactive=False)
+    assert argv == [path, "-Command"]
+
+
+def test_win32_env_shell_override_preserves_backslashes(monkeypatch: pytest.MonkeyPatch) -> None:
+    import agent_framework_tools.shell._resolve as resolve_module
+
+    monkeypatch.setattr(resolve_module.sys, "platform", "win32")
+    path = r"C:\msys64\usr\bin\bash.exe"
+    monkeypatch.setenv("AGENT_FRAMEWORK_SHELL", path)
+    argv = resolve_shell(None, interactive=False)
+    assert argv == [path, "-c"]
+
+
+def test_win32_quoted_shell_with_spaces_and_empty_arg(monkeypatch: pytest.MonkeyPatch) -> None:
+    import agent_framework_tools.shell._resolve as resolve_module
+
+    monkeypatch.setattr(resolve_module.sys, "platform", "win32")
+    cmd = r'"C:\Program Files\PowerShell\7\pwsh.exe" -NoProfile "" --opt="val\"ue"'
+    argv = resolve_shell(cmd, interactive=False)
+    assert argv[0] == r"C:\Program Files\PowerShell\7\pwsh.exe"
+    assert argv[1] == "-NoProfile"
+    assert argv[2] == ""
+    assert argv[3] == r'--opt=val"ue'
+    assert argv[-1] == "-Command"
+```
+
+- [ ] **Step 2: Run test to verify failure**
+
+Run: `uv run pytest packages/tools/tests/test_shell_resolve.py -k "win32"`
+Expected: FAIL with backslashes stripped or tokenization mismatch.
+
+---
+
+### Task 2: Implement Windows `CommandLineToArgvW` Tokenizer in `_resolve.py`
+
+**Files:**
+- Modify: `python/packages/tools/agent_framework_tools/shell/_resolve.py`
+
+**Interfaces:**
+- Consumes: `shell: str | Sequence[str] | None`, `AGENT_FRAMEWORK_SHELL`
+- Produces: `_split_windows_command_line(cmd: str) -> list[str]` and updated `resolve_shell` routing.
+
+- [ ] **Step 1: Write `_split_windows_command_line` implementation**
+
+```python
+def _split_windows_command_line(cmd: str) -> list[str]:
+    """Parse a Windows command line string into argv following CommandLineToArgvW rules."""
+    args: list[str] = []
+    buf: list[str] = []
+    in_quotes = False
+    has_token = False
+    i = 0
+    n = len(cmd)
+
+    while i < n:
+        c = cmd[i]
+        if c in " \t" and not in_quotes:
+            if has_token:
+                args.append("".join(buf))
+                buf = []
+                has_token = False
+            i += 1
+            continue
+
+        if c == "\\":
+            num_slashes = 0
+            while i < n and cmd[i] == "\\":
+                num_slashes += 1
+                i += 1
+            if i < n and cmd[i] == '"':
+                buf.append("\\" * (num_slashes // 2))
+                if num_slashes % 2 == 1:
+                    buf.append('"')
+                    has_token = True
+                    i += 1
+                else:
+                    in_quotes = not in_quotes
+                    has_token = True
+                    i += 1
+            else:
+                buf.append("\\" * num_slashes)
+                has_token = True
+        elif c == '"':
+            in_quotes = not in_quotes
+            has_token = True
+            i += 1
+        else:
+            buf.append(c)
+            has_token = True
+            i += 1
+
+    if has_token:
+        args.append("".join(buf))
+    return args
+```
+
+- [ ] **Step 2: Update `resolve_shell` and `is_powershell` in `_resolve.py`**
+
+In `_split_shell_override(value: str) -> list[str]`:
+Route to `_split_windows_command_line(value)` if `sys.platform == "win32"`, else `shlex.split(value)`.
+In `is_powershell`:
+Normalize backslashes via `argv[0].replace("\\", "/")` before `os.path.basename` to ensure win32 paths detect properly when tested on POSIX platforms.
+
+- [ ] **Step 3: Run tests to verify they pass**
+
+Run: `uv run pytest packages/tools/tests/test_shell_resolve.py`
+Expected: ALL PASS.
+
+- [ ] **Step 4: Run repo linting and typing checks**
+
+Run: `uv run ruff check packages/tools` and `uv run pyright packages/tools`
+Expected: 0 errors.

python/packages/core/agent_framework/_tools.py

@@ -6548,20 +6548,27 @@ def get_response(
 
         # Dispatch to the shape-specific loop; both loops follow the same approval -> model -> execution phases.
         if not stream:
-            return self._get_response_with_function_invocation(
-                super_get_response=super_get_response,
-                messages=messages,
-                options=mutable_options,
-                request_kwargs=request_kwargs,
-                compaction_strategy=compaction_strategy,
-                tokenizer=tokenizer,
-                execute_function_calls=execute_function_calls,
-                invocation_session=invocation_session,
-                approval_session_is_authoritative=approval_session_is_authoritative,
-                budget_state=budget_state,
-                max_errors=max_errors,
-                middleware_pipeline=function_middleware_pipeline,
-            )
+
+            async def _run_non_streaming() -> ChatResponse[Any]:
+                try:
+                    return await self._get_response_with_function_invocation(
+                        super_get_response=super_get_response,
+                        messages=messages,
+                        options=mutable_options,
+                        request_kwargs=request_kwargs,
+                        compaction_strategy=compaction_strategy,
+                        tokenizer=tokenizer,
+                        execute_function_calls=execute_function_calls,
+                        invocation_session=invocation_session,
+                        approval_session_is_authoritative=approval_session_is_authoritative,
+                        budget_state=budget_state,
+                        max_errors=max_errors,
+                        middleware_pipeline=function_middleware_pipeline,
+                    )
+                finally:
+                    _clear_budget_state_from_session(invocation_session, budget_state)
+
+            return _run_non_streaming()
 
         response_format = mutable_options.get("response_format")
         invalidation_error: list[ResponseInvalidatedException] = []
@@ -6571,22 +6578,36 @@ def finalize_stream(updates: Sequence[ChatResponseUpdate]) -> ChatResponse[Any]:
                 raise invalidation_error[0]
             return self._finalize_function_invocation_updates(updates, response_format=response_format)
 
+        async def _run_streaming() -> AsyncIterable[ChatResponseUpdate]:
+            updates: AsyncIterable[ChatResponseUpdate] | None = None
+            try:
+                updates = self._stream_response_with_function_invocation(
+                    super_get_response=super_get_response,
+                    messages=messages,
+                    options=mutable_options,
+                    request_kwargs=request_kwargs,
+                    compaction_strategy=compaction_strategy,
+                    tokenizer=tokenizer,
+                    execute_function_calls=execute_function_calls,
+                    invocation_session=invocation_session,
+                    approval_session_is_authoritative=approval_session_is_authoritative,
+                    budget_state=budget_state,
+                    max_errors=max_errors,
+                    middleware_pipeline=function_middleware_pipeline,
+                    invalidation_error=invalidation_error,
+                )
+                async for update in updates:
+                    yield update
+            finally:
+                try:
+                    close = getattr(updates, "aclose", None)
+                    if close is not None:
+                        await close()
+                finally:
+                    _clear_budget_state_from_session(invocation_session, budget_state)
+
         return ResponseStream(
-            self._stream_response_with_function_invocation(
-                super_get_response=super_get_response,
-                messages=messages,
-                options=mutable_options,
-                request_kwargs=request_kwargs,
-                compaction_strategy=compaction_strategy,
-                tokenizer=tokenizer,
-                execute_function_calls=execute_function_calls,
-                invocation_session=invocation_session,
-                approval_session_is_authoritative=approval_session_is_authoritative,
-                budget_state=budget_state,
-                max_errors=max_errors,
-                middleware_pipeline=function_middleware_pipeline,
-                invalidation_error=invalidation_error,
-            ),
+            _run_streaming(),
             finalizer=finalize_stream,
         )