From 350e6f54ff1cce5480125c322bf5f5551e775195 Mon Sep 17 00:00:00 2001 From: Trip T Date: Thu, 12 Mar 2026 20:40:39 -0400 Subject: [PATCH 1/2] fix: prevent "Event loop is closed" on repeated Gitea API calls The httpx AsyncClient was cached across asyncio.run() boundaries. Each asyncio.run() creates and closes a new event loop, leaving the cached client's connections on a dead loop. Second+ calls would fail with "Event loop is closed". Fix: create a fresh client per request and close it in a finally block. No more cross-loop client reuse. Co-Authored-By: Claude Opus 4.6 --- src/infrastructure/hands/gitea.py | 33 ++++++++++++++++++------------- tests/test_hands_gitea.py | 12 +++++------ 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/infrastructure/hands/gitea.py b/src/infrastructure/hands/gitea.py index c25cf28..252db76 100644 --- a/src/infrastructure/hands/gitea.py +++ b/src/infrastructure/hands/gitea.py @@ -72,7 +72,6 @@ class GiteaHand: self._token = token or _resolve_token() self._repo = repo or settings.gitea_repo self._timeout = timeout or settings.gitea_timeout - self._client = None if not self._token: logger.warning( @@ -92,20 +91,24 @@ class GiteaHand: return bool(settings.gitea_enabled and self._token and self._repo) def _get_client(self): - """Lazy-initialise the async HTTP client.""" + """Create a fresh async HTTP client for the current event loop. + + Always creates a new client rather than caching, because tool + functions call us via ``asyncio.run()`` which creates a new loop + each time — a cached client from a previous loop would raise + "Event loop is closed". + """ import httpx - if self._client is None or self._client.is_closed: - self._client = httpx.AsyncClient( - base_url=self._base_url, - headers={ - "Authorization": f"token {self._token}", - "Accept": "application/json", - "Content-Type": "application/json", - }, - timeout=self._timeout, - ) - return self._client + return httpx.AsyncClient( + base_url=self._base_url, + headers={ + "Authorization": f"token {self._token}", + "Accept": "application/json", + "Content-Type": "application/json", + }, + timeout=self._timeout, + ) async def _request(self, method: str, path: str, **kwargs) -> GiteaResult: """Make an API request with full error handling.""" @@ -119,8 +122,8 @@ class GiteaHand: error="Gitea not configured (missing token or repo)", ) + client = self._get_client() try: - client = self._get_client() resp = await client.request(method, path, **kwargs) latency = (time.time() - start) * 1000 @@ -155,6 +158,8 @@ class GiteaHand: error=str(exc), latency_ms=latency, ) + finally: + await client.aclose() # ── Issue operations ───────────────────────────────────────────────── diff --git a/tests/test_hands_gitea.py b/tests/test_hands_gitea.py index 9335e3f..833899b 100644 --- a/tests/test_hands_gitea.py +++ b/tests/test_hands_gitea.py @@ -199,7 +199,7 @@ async def test_create_issue_success(): mock_client = AsyncMock() mock_client.request = AsyncMock(return_value=mock_response) - mock_client.is_closed = False + mock_client.aclose = AsyncMock() with patch("infrastructure.hands.gitea.settings") as mock_settings: mock_settings.gitea_enabled = True @@ -208,7 +208,7 @@ async def test_create_issue_success(): mock_settings.gitea_repo = "owner/repo" mock_settings.gitea_timeout = 30 hand = GiteaHand(token="test-token") - hand._client = mock_client + hand._get_client = MagicMock(return_value=mock_client) result = await hand.create_issue("Test issue", "Body text") assert result.success is True @@ -231,7 +231,7 @@ async def test_create_issue_handles_http_error(): mock_client = AsyncMock() mock_client.request = AsyncMock(return_value=mock_response) - mock_client.is_closed = False + mock_client.aclose = AsyncMock() with patch("infrastructure.hands.gitea.settings") as mock_settings: mock_settings.gitea_enabled = True @@ -240,7 +240,7 @@ async def test_create_issue_handles_http_error(): mock_settings.gitea_repo = "owner/repo" mock_settings.gitea_timeout = 30 hand = GiteaHand(token="test-token") - hand._client = mock_client + hand._get_client = MagicMock(return_value=mock_client) result = await hand.create_issue("Test issue") assert result.success is False @@ -254,7 +254,7 @@ async def test_create_issue_handles_connection_error(): mock_client = AsyncMock() mock_client.request = AsyncMock(side_effect=ConnectionError("refused")) - mock_client.is_closed = False + mock_client.aclose = AsyncMock() with patch("infrastructure.hands.gitea.settings") as mock_settings: mock_settings.gitea_enabled = True @@ -263,7 +263,7 @@ async def test_create_issue_handles_connection_error(): mock_settings.gitea_repo = "owner/repo" mock_settings.gitea_timeout = 30 hand = GiteaHand(token="test-token") - hand._client = mock_client + hand._get_client = MagicMock(return_value=mock_client) result = await hand.create_issue("Test issue") assert result.success is False From 41d6ebaf6a205f9ee3ac4c3c04b823d96be6afbe Mon Sep 17 00:00:00 2001 From: Trip T Date: Thu, 12 Mar 2026 20:55:56 -0400 Subject: [PATCH 2/2] feat: CLI session persistence + tool confirmation gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Chat sessions persist across `timmy chat` invocations via Agno SQLite (session_id="cli"), fixing context amnesia between turns - Dangerous tools (shell, write_file, etc.) now prompt for approval in CLI instead of silently exiting — uses typer.confirm() + Agno continue_run - --new flag starts a fresh conversation when needed - Improved _maybe_file_issues prompt for engineer-quality issue bodies (what's happening, expected behavior, suggested fix, acceptance criteria) - think/status commands also pass session_id for continuity Co-Authored-By: Claude Opus 4.6 --- src/timmy/cli.py | 98 ++++++++++++++++++- src/timmy/thinking.py | 9 +- tests/timmy/test_cli.py | 203 +++++++++++++++++++++++++++++++++++----- 3 files changed, 284 insertions(+), 26 deletions(-) diff --git a/src/timmy/cli.py b/src/timmy/cli.py index ae07ea0..681466c 100644 --- a/src/timmy/cli.py +++ b/src/timmy/cli.py @@ -1,12 +1,20 @@ +import logging import subprocess import typer from timmy.agent import create_timmy from timmy.prompts import STATUS_PROMPT +from timmy.tool_safety import format_action_description, get_impact_level + +logger = logging.getLogger(__name__) app = typer.Typer(help="Timmy — sovereign AI agent") +# Stable session ID — Agno persists conversation history in SQLite keyed on this. +# Every `timmy chat` invocation reuses the same session so context carries over. +_CLI_SESSION_ID = "cli" + # Shared option definitions (reused across commands for consistency). _BACKEND_OPTION = typer.Option( None, @@ -22,6 +30,64 @@ _MODEL_SIZE_OPTION = typer.Option( ) +def _handle_tool_confirmation(agent, run_output, session_id: str): + """Prompt user to approve/reject dangerous tool calls. + + When Agno pauses a run because a tool requires confirmation, this + function displays the action, asks for approval via stdin, and + resumes or rejects the run accordingly. + + Returns the final RunOutput after all confirmations are resolved. + """ + max_rounds = 10 # safety limit + for _ in range(max_rounds): + status = getattr(run_output, "status", None) + is_paused = status == "PAUSED" or str(status) == "RunStatus.paused" + if not is_paused: + return run_output + + reqs = getattr(run_output, "active_requirements", None) or [] + if not reqs: + return run_output + + for req in reqs: + if not getattr(req, "needs_confirmation", False): + continue + + te = req.tool_execution + tool_name = getattr(te, "tool_name", "unknown") + tool_args = getattr(te, "tool_args", {}) or {} + + description = format_action_description(tool_name, tool_args) + impact = get_impact_level(tool_name) + + typer.echo() + typer.echo(typer.style("Tool confirmation required", bold=True)) + typer.echo(f" Impact: {impact.upper()}") + typer.echo(f" {description}") + typer.echo() + + approved = typer.confirm("Allow this action?", default=False) + if approved: + req.confirm() + logger.info("CLI: approved %s", tool_name) + else: + req.reject(note="User rejected from CLI") + logger.info("CLI: rejected %s", tool_name) + + # Resume the run so the agent sees the confirmation result + try: + run_output = agent.continue_run( + run_response=run_output, stream=False, session_id=session_id + ) + except Exception as exc: + logger.error("CLI: continue_run failed: %s", exc) + typer.echo(f"Error resuming: {exc}", err=True) + return run_output + + return run_output + + @app.command() def tick( prompt: str | None = typer.Argument( @@ -48,7 +114,7 @@ def think( ): """Ask Timmy to think carefully about a topic.""" timmy = create_timmy(backend=backend, model_size=model_size) - timmy.print_response(f"Think carefully about: {topic}", stream=True) + timmy.print_response(f"Think carefully about: {topic}", stream=True, session_id=_CLI_SESSION_ID) @app.command() @@ -56,10 +122,34 @@ def chat( message: str = typer.Argument(..., help="Message to send"), backend: str | None = _BACKEND_OPTION, model_size: str | None = _MODEL_SIZE_OPTION, + new_session: bool = typer.Option( + False, + "--new", + "-n", + help="Start a fresh conversation (ignore prior context)", + ), ): - """Send a message to Timmy.""" + """Send a message to Timmy. + + Conversation history persists across invocations. Use --new to start fresh. + """ + import uuid + + session_id = str(uuid.uuid4()) if new_session else _CLI_SESSION_ID timmy = create_timmy(backend=backend, model_size=model_size) - timmy.print_response(message, stream=True) + + # Use agent.run() so we can intercept paused runs for tool confirmation. + run_output = timmy.run(message, stream=False, session_id=session_id) + + # Handle paused runs — dangerous tools need user approval + run_output = _handle_tool_confirmation(timmy, run_output, session_id) + + # Print the final response + content = run_output.content if hasattr(run_output, "content") else str(run_output) + if content: + from timmy.session import _clean_response + + typer.echo(_clean_response(content)) @app.command() @@ -69,7 +159,7 @@ def status( ): """Print Timmy's operational status.""" timmy = create_timmy(backend=backend, model_size=model_size) - timmy.print_response(STATUS_PROMPT, stream=False) + timmy.print_response(STATUS_PROMPT, stream=False, session_id=_CLI_SESSION_ID) @app.command() diff --git a/src/timmy/thinking.py b/src/timmy/thinking.py index 8e65166..d4f077f 100644 --- a/src/timmy/thinking.py +++ b/src/timmy/thinking.py @@ -422,8 +422,15 @@ class ThinkingEngine: "Rules:\n" "- Only include things that could become a real code fix or feature\n" "- Skip vague reflections, philosophical musings, or repeated themes\n" - "- Each item needs a specific, descriptive title and body\n" "- Category must be one of: bug, feature, suggestion, maintenance\n\n" + "For each item, write an ENGINEER-QUALITY issue:\n" + '- "title": A clear, specific title (e.g. "[Memory] MEMORY.md timestamp not updating")\n' + '- "body": A detailed body with these sections:\n' + " **What's happening:** Describe the current (broken) behavior.\n" + " **Expected behavior:** What should happen instead.\n" + " **Suggested fix:** Which file(s) to change and what the fix looks like.\n" + " **Acceptance criteria:** How to verify the fix works.\n" + '- "category": One of bug, feature, suggestion, maintenance\n\n' "Return ONLY a JSON array of objects with keys: " '"title", "body", "category"\n' "Return [] if nothing is actionable.\n\n" diff --git a/tests/timmy/test_cli.py b/tests/timmy/test_cli.py index 15ad5cf..4122dc5 100644 --- a/tests/timmy/test_cli.py +++ b/tests/timmy/test_cli.py @@ -2,12 +2,17 @@ from unittest.mock import MagicMock, patch from typer.testing import CliRunner -from timmy.cli import app +from timmy.cli import _CLI_SESSION_ID, _handle_tool_confirmation, app from timmy.prompts import STATUS_PROMPT runner = CliRunner() +# --------------------------------------------------------------------------- +# status command +# --------------------------------------------------------------------------- + + def test_status_uses_status_prompt(): """status command must pass STATUS_PROMPT to the agent.""" mock_timmy = MagicMock() @@ -15,7 +20,9 @@ def test_status_uses_status_prompt(): with patch("timmy.cli.create_timmy", return_value=mock_timmy): runner.invoke(app, ["status"]) - mock_timmy.print_response.assert_called_once_with(STATUS_PROMPT, stream=False) + mock_timmy.print_response.assert_called_once_with( + STATUS_PROMPT, stream=False, session_id=_CLI_SESSION_ID + ) def test_status_does_not_use_inline_string(): @@ -29,14 +36,9 @@ def test_status_does_not_use_inline_string(): assert call_args[0][0] != "Brief status report — one sentence." -def test_chat_sends_message_to_agent(): - """chat command must pass the user message to the agent with streaming.""" - mock_timmy = MagicMock() - - with patch("timmy.cli.create_timmy", return_value=mock_timmy): - runner.invoke(app, ["chat", "Hello Timmy"]) - - mock_timmy.print_response.assert_called_once_with("Hello Timmy", stream=True) +# --------------------------------------------------------------------------- +# think command +# --------------------------------------------------------------------------- def test_think_sends_topic_to_agent(): @@ -47,20 +49,12 @@ def test_think_sends_topic_to_agent(): runner.invoke(app, ["think", "Bitcoin self-custody"]) mock_timmy.print_response.assert_called_once_with( - "Think carefully about: Bitcoin self-custody", stream=True + "Think carefully about: Bitcoin self-custody", + stream=True, + session_id=_CLI_SESSION_ID, ) -def test_chat_passes_backend_option(): - """chat --backend airllm must forward the backend to create_timmy.""" - mock_timmy = MagicMock() - - with patch("timmy.cli.create_timmy", return_value=mock_timmy) as mock_create: - runner.invoke(app, ["chat", "test", "--backend", "airllm"]) - - mock_create.assert_called_once_with(backend="airllm", model_size=None) - - def test_think_passes_model_size_option(): """think --model-size 70b must forward the model size to create_timmy.""" mock_timmy = MagicMock() @@ -69,3 +63,170 @@ def test_think_passes_model_size_option(): runner.invoke(app, ["think", "topic", "--model-size", "70b"]) mock_create.assert_called_once_with(backend=None, model_size="70b") + + +# --------------------------------------------------------------------------- +# chat command — session persistence +# --------------------------------------------------------------------------- + + +def test_chat_uses_session_id(): + """chat command must pass the stable CLI session_id to agent.run().""" + mock_run_output = MagicMock() + mock_run_output.content = "Hello there!" + mock_run_output.status = "COMPLETED" + mock_run_output.active_requirements = [] + + mock_timmy = MagicMock() + mock_timmy.run.return_value = mock_run_output + + with patch("timmy.cli.create_timmy", return_value=mock_timmy): + result = runner.invoke(app, ["chat", "Hello Timmy"]) + + mock_timmy.run.assert_called_once_with("Hello Timmy", stream=False, session_id=_CLI_SESSION_ID) + assert result.exit_code == 0 + assert "Hello there!" in result.output + + +def test_chat_new_session_uses_unique_id(): + """chat --new must use a unique session_id, not the stable one.""" + mock_run_output = MagicMock() + mock_run_output.content = "Fresh start!" + mock_run_output.status = "COMPLETED" + mock_run_output.active_requirements = [] + + mock_timmy = MagicMock() + mock_timmy.run.return_value = mock_run_output + + with patch("timmy.cli.create_timmy", return_value=mock_timmy): + runner.invoke(app, ["chat", "Hello", "--new"]) + + call_args = mock_timmy.run.call_args + used_session_id = call_args[1]["session_id"] + assert used_session_id != _CLI_SESSION_ID # Must be unique + + +def test_chat_passes_backend_option(): + """chat --backend airllm must forward the backend to create_timmy.""" + mock_run_output = MagicMock() + mock_run_output.content = "OK" + mock_run_output.status = "COMPLETED" + mock_run_output.active_requirements = [] + + mock_timmy = MagicMock() + mock_timmy.run.return_value = mock_run_output + + with patch("timmy.cli.create_timmy", return_value=mock_timmy) as mock_create: + runner.invoke(app, ["chat", "test", "--backend", "airllm"]) + + mock_create.assert_called_once_with(backend="airllm", model_size=None) + + +def test_chat_cleans_response(): + """chat must clean tool-call artifacts from the response.""" + raw = '{"name": "python", "parameters": {"code": "1+1"}} The answer is 2.' + mock_run_output = MagicMock() + mock_run_output.content = raw + mock_run_output.status = "COMPLETED" + mock_run_output.active_requirements = [] + + mock_timmy = MagicMock() + mock_timmy.run.return_value = mock_run_output + + with patch("timmy.cli.create_timmy", return_value=mock_timmy): + result = runner.invoke(app, ["chat", "what is 1+1"]) + + # The JSON tool call should be stripped + assert '"name": "python"' not in result.output + assert "The answer is 2." in result.output + + +# --------------------------------------------------------------------------- +# Tool confirmation gate +# --------------------------------------------------------------------------- + + +def _make_paused_run(tool_name="shell", tool_args=None): + """Create a mock paused RunOutput with one requirement.""" + tool_args = tool_args or {"command": "ls -la"} + + mock_te = MagicMock() + mock_te.tool_name = tool_name + mock_te.tool_args = tool_args + + mock_req = MagicMock() + mock_req.needs_confirmation = True + mock_req.tool_execution = mock_te + + mock_run = MagicMock() + mock_run.status = "RunStatus.paused" + mock_run.active_requirements = [mock_req] + + return mock_run, mock_req + + +def test_handle_tool_confirmation_approve(): + """Approving a tool should call req.confirm() and agent.continue_run().""" + paused_run, mock_req = _make_paused_run() + + completed_run = MagicMock() + completed_run.status = "COMPLETED" + completed_run.active_requirements = [] + completed_run.content = "Done." + + mock_agent = MagicMock() + mock_agent.continue_run.return_value = completed_run + + # Simulate user typing "y" at the prompt + with patch("timmy.cli.typer.confirm", return_value=True): + result = _handle_tool_confirmation(mock_agent, paused_run, "cli") + + mock_req.confirm.assert_called_once() + mock_agent.continue_run.assert_called_once() + assert result.content == "Done." + + +def test_handle_tool_confirmation_reject(): + """Rejecting a tool should call req.reject() and agent.continue_run().""" + paused_run, mock_req = _make_paused_run() + + completed_run = MagicMock() + completed_run.status = "COMPLETED" + completed_run.active_requirements = [] + completed_run.content = "Action rejected." + + mock_agent = MagicMock() + mock_agent.continue_run.return_value = completed_run + + with patch("timmy.cli.typer.confirm", return_value=False): + _handle_tool_confirmation(mock_agent, paused_run, "cli") + + mock_req.reject.assert_called_once() + mock_agent.continue_run.assert_called_once() + + +def test_handle_tool_confirmation_not_paused(): + """Non-paused runs should pass through unchanged.""" + completed_run = MagicMock() + completed_run.status = "COMPLETED" + completed_run.active_requirements = [] + + mock_agent = MagicMock() + result = _handle_tool_confirmation(mock_agent, completed_run, "cli") + + assert result is completed_run + mock_agent.continue_run.assert_not_called() + + +def test_handle_tool_confirmation_continue_error(): + """Errors in continue_run should be handled gracefully.""" + paused_run, mock_req = _make_paused_run() + + mock_agent = MagicMock() + mock_agent.continue_run.side_effect = Exception("connection lost") + + with patch("timmy.cli.typer.confirm", return_value=True): + result = _handle_tool_confirmation(mock_agent, paused_run, "cli") + + # Should return the original paused run, not crash + assert result is paused_run