refactor: remove GiteaTools facade, use focused tool classes directly
- Removed gitea/tools/gitea_tools.py (useless facade layer) - Removed tests/test_gitea_tools.py (tests for removed facade) - Updated core/dispatcher.py to use IssueTools, PRTools, FileTools, GitTools directly - Updated core/orchestrator.py to use individual tool instances - Updated main.py to create individual tool instances This eliminates the triple layer of indirection (Issue 2.2 from bad_code.md) where GiteaTools just delegated to IssueTools/PRTools/etc with zero added value.
This commit is contained in:
+68
-28
@@ -11,10 +11,16 @@ from core.queue import WorkQueue, WorkItem
|
||||
from core.dispatcher import AgentDispatcher
|
||||
from gitea.client import GiteaClient
|
||||
from gitea.models import IssueModel, PullRequestModel, RepositoryModel
|
||||
from gitea.tools.gitea_tools import GiteaTools
|
||||
from gitea.tools.issue_tools import IssueTools
|
||||
from gitea.tools.pr_tools import PRTools
|
||||
from gitea.tools.file_tools import FileTools
|
||||
from gitea.tools.git_tools import GitTools
|
||||
from gitea.config import AGENT_MODEL_ID, AGENT_MAX_RETRIES
|
||||
from gitea.workspace import WorkspaceManager
|
||||
from core.notification_agent import NotificationReaderAgent, NotificationNoToolCalledError
|
||||
from core.notification_agent import (
|
||||
NotificationReaderAgent,
|
||||
NotificationNoToolCalledError,
|
||||
)
|
||||
from core.notification_tools import NotificationTools
|
||||
|
||||
logger: logging.Logger = logging.getLogger("agent-orchestrator")
|
||||
@@ -26,19 +32,32 @@ class AgentOrchestrator:
|
||||
def __init__(
|
||||
self,
|
||||
client: GiteaClient,
|
||||
tools: GiteaTools,
|
||||
issue_tools: IssueTools,
|
||||
pr_tools: PRTools,
|
||||
file_tools: FileTools,
|
||||
git_tools: GitTools,
|
||||
model_name: str = AGENT_MODEL_ID,
|
||||
max_retries: int = AGENT_MAX_RETRIES,
|
||||
) -> None:
|
||||
self._client = client
|
||||
self._tools = tools
|
||||
self._issue_tools = issue_tools
|
||||
self._pr_tools = pr_tools
|
||||
self._file_tools = file_tools
|
||||
self._git_tools = git_tools
|
||||
self._model_name = model_name
|
||||
self._work_queue = WorkQueue()
|
||||
self._dispatcher = AgentDispatcher(client, tools, model_name, max_retries)
|
||||
self._dispatcher = AgentDispatcher(
|
||||
client,
|
||||
issue_tools,
|
||||
pr_tools,
|
||||
file_tools,
|
||||
git_tools,
|
||||
model_name,
|
||||
max_retries,
|
||||
)
|
||||
self._notification_reader = NotificationReaderAgent(model_name)
|
||||
self._max_retries = max_retries
|
||||
|
||||
|
||||
def _get_state_file_path(self) -> Path:
|
||||
"""Get the path to the persistent state file."""
|
||||
return Path(__file__).parent.parent / "agent_state.json"
|
||||
@@ -67,7 +86,9 @@ class AgentOrchestrator:
|
||||
async def poll_and_dispatch(self) -> None:
|
||||
"""Poll Gitea unread notifications, enqueue them, and dispatch to agent."""
|
||||
last_checked = self._read_last_checked()
|
||||
logger.info(f"Polling unread notifications since: {last_checked or 'beginning'}")
|
||||
logger.info(
|
||||
f"Polling unread notifications since: {last_checked or 'beginning'}"
|
||||
)
|
||||
|
||||
notifications = self._client.list_unread_notifications(since=last_checked)
|
||||
|
||||
@@ -80,10 +101,10 @@ class AgentOrchestrator:
|
||||
# Filter and enqueue tasks from notifications
|
||||
latest_timestamp = last_checked
|
||||
inspection_tools = [
|
||||
self._tools.get_issue,
|
||||
self._tools.get_pull_request,
|
||||
self._tools.get_issue_comments,
|
||||
self._tools.get_pull_request_comments,
|
||||
self._issue_tools.get_issue,
|
||||
self._pr_tools.get_pull_request,
|
||||
self._issue_tools.get_issue_comments,
|
||||
self._pr_tools.get_pull_request_comments,
|
||||
]
|
||||
|
||||
for n in notifications:
|
||||
@@ -106,7 +127,9 @@ class AgentOrchestrator:
|
||||
try:
|
||||
task_number = int(subj_url.rstrip("/").split("/")[-1])
|
||||
except (ValueError, IndexError):
|
||||
logger.warning(f"Could not parse task number from subject URL: {subj_url}")
|
||||
logger.warning(
|
||||
f"Could not parse task number from subject URL: {subj_url}"
|
||||
)
|
||||
continue
|
||||
|
||||
# Run NotificationReaderAgent to pre-screen the notification
|
||||
@@ -125,21 +148,27 @@ class AgentOrchestrator:
|
||||
attempt += 1
|
||||
try:
|
||||
await self._notification_reader.decide_notification(
|
||||
mission,
|
||||
inspection_tools,
|
||||
notification_tools
|
||||
mission, inspection_tools, notification_tools
|
||||
)
|
||||
success = True
|
||||
break
|
||||
except NotificationNoToolCalledError as e:
|
||||
logger.error(f"Notification reader error on notification {notification_id} (attempt {attempt}/{attempt_limit}): {e}")
|
||||
logger.error(
|
||||
f"Notification reader error on notification {notification_id} (attempt {attempt}/{attempt_limit}): {e}"
|
||||
)
|
||||
|
||||
if not success or notification_tools.action == "SKIP":
|
||||
reason = notification_tools.arguments.get("reason", "Failed to call routing tool / default skip")
|
||||
logger.info(f"Skipping notification {notification_id} for {repo_full_name}#{task_number}. Reason: {reason}")
|
||||
reason = notification_tools.arguments.get(
|
||||
"reason", "Failed to call routing tool / default skip"
|
||||
)
|
||||
logger.info(
|
||||
f"Skipping notification {notification_id} for {repo_full_name}#{task_number}. Reason: {reason}"
|
||||
)
|
||||
if notification_id is not None:
|
||||
self._client.mark_notification_as_read(notification_id)
|
||||
logger.info(f"Marked skipped Gitea notification thread {notification_id} as read.")
|
||||
logger.info(
|
||||
f"Marked skipped Gitea notification thread {notification_id} as read."
|
||||
)
|
||||
continue
|
||||
|
||||
# Route based on decided action
|
||||
@@ -147,7 +176,9 @@ class AgentOrchestrator:
|
||||
try:
|
||||
issue = self._client.get_issue(owner, repo_name, task_number)
|
||||
if issue.repository is None:
|
||||
issue = issue.model_copy(update={"repository": RepositoryModel(**repo_info)})
|
||||
issue = issue.model_copy(
|
||||
update={"repository": RepositoryModel(**repo_info)}
|
||||
)
|
||||
|
||||
item = WorkItem(
|
||||
repo_full_name=repo_full_name,
|
||||
@@ -155,16 +186,20 @@ class AgentOrchestrator:
|
||||
task_number=task_number,
|
||||
task_info=issue,
|
||||
notification_id=notification_id,
|
||||
priority=0
|
||||
priority=0,
|
||||
)
|
||||
self._work_queue.enqueue(item)
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to fetch issue #{task_number} for notification: {e}")
|
||||
logger.error(
|
||||
f"Failed to fetch issue #{task_number} for notification: {e}"
|
||||
)
|
||||
elif notification_tools.action == "PROCESS_PR":
|
||||
try:
|
||||
pr = self._client.get_pull_request(owner, repo_name, task_number)
|
||||
if pr.repository is None:
|
||||
pr = pr.model_copy(update={"repository": RepositoryModel(**repo_info)})
|
||||
pr = pr.model_copy(
|
||||
update={"repository": RepositoryModel(**repo_info)}
|
||||
)
|
||||
|
||||
item = WorkItem(
|
||||
repo_full_name=repo_full_name,
|
||||
@@ -172,12 +207,13 @@ class AgentOrchestrator:
|
||||
task_number=task_number,
|
||||
task_info=pr,
|
||||
notification_id=notification_id,
|
||||
priority=0
|
||||
priority=0,
|
||||
)
|
||||
self._work_queue.enqueue(item)
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to fetch PR #{task_number} for notification: {e}")
|
||||
|
||||
logger.error(
|
||||
f"Failed to fetch PR #{task_number} for notification: {e}"
|
||||
)
|
||||
|
||||
# Process enqueued work
|
||||
if not self._work_queue.is_empty:
|
||||
@@ -212,7 +248,11 @@ class AgentOrchestrator:
|
||||
|
||||
for i, result in enumerate(results):
|
||||
item = work_items[i]
|
||||
logger.info(f"Completed {item.task_type} #{item.task_number}: {result[:200]}")
|
||||
logger.info(
|
||||
f"Completed {item.task_type} #{item.task_number}: {result[:200]}"
|
||||
)
|
||||
if item.notification_id is not None:
|
||||
self._client.mark_notification_as_read(item.notification_id)
|
||||
logger.info(f"Marked Gitea notification thread {item.notification_id} as read.")
|
||||
logger.info(
|
||||
f"Marked Gitea notification thread {item.notification_id} as read."
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user