refactor: raise error on auth failure and clean up meeks-ai fallback
This commit is contained in:
@@ -5,7 +5,7 @@ You are an autonomous AI Software Engineer working on the `meeks` organization's
|
||||
|
||||
### 🎯 SCOPE & BOUNDARIES
|
||||
- **Organization**: You ONLY work on repositories under the `meeks` organization (e.g., `meeks/ai-electronbun-todo-app`).
|
||||
- **DO NOT work on**: `meeks-ai`, `michael`, or any other organization/personal repos.
|
||||
- **DO NOT work on**: any other organization/personal repos.
|
||||
- **DO NOT create new repositories**. The repo already exists. It is cloned locally in the workspace (which is your current working directory).
|
||||
- **DO NOT edit `.git` files** unless explicitly asked to resolve a git conflict or rebase issue.
|
||||
|
||||
|
||||
+23
-13
@@ -22,7 +22,6 @@ from gitea.models import CommentModel, PullRequestFileModel, PullRequestModel, I
|
||||
|
||||
logger: logging.Logger = logging.getLogger("agent-dispatcher")
|
||||
|
||||
AGENT_USERNAMES: frozenset[str] = frozenset({"meeks-ai", "agent-bot"})
|
||||
|
||||
CLOSE_KEYWORDS_PATTERN: re.Pattern[str] = re.compile(
|
||||
rf"\b(?:close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved)\s+#(\d+)\b",
|
||||
@@ -58,7 +57,7 @@ def _find_issues_for_pr_helper(pr_body: str) -> list[int]:
|
||||
return list(set(int(m) for m in matches))
|
||||
|
||||
|
||||
def _is_awaiting_reply_helper(comments: list[CommentModel]) -> bool:
|
||||
def _is_awaiting_reply_helper(comments: list[CommentModel], ai_username: str) -> bool:
|
||||
"""Return True if the agent's most recent comment contains the
|
||||
awaiting-reply marker AND no human has commented after it.
|
||||
"""
|
||||
@@ -66,8 +65,9 @@ def _is_awaiting_reply_helper(comments: list[CommentModel]) -> bool:
|
||||
return False
|
||||
# Find the last agent comment index
|
||||
last_agent_idx: int = -1
|
||||
agent_usernames = {ai_username, "agent-bot"}
|
||||
for i, c in enumerate(comments):
|
||||
if c.user and c.user.login in AGENT_USERNAMES:
|
||||
if c.user and c.user.login in agent_usernames:
|
||||
last_agent_idx = i
|
||||
if last_agent_idx == -1:
|
||||
return False
|
||||
@@ -78,7 +78,7 @@ def _is_awaiting_reply_helper(comments: list[CommentModel]) -> bool:
|
||||
return False
|
||||
# Check if any human replied AFTER the last agent comment
|
||||
for c in comments[last_agent_idx + 1:]:
|
||||
if c.user and c.user.login not in AGENT_USERNAMES:
|
||||
if c.user and c.user.login not in agent_usernames:
|
||||
return False # Human replied — we can proceed
|
||||
return True # Agent signalled wait, no human replied yet
|
||||
|
||||
@@ -344,7 +344,7 @@ class PRTaskProcessor(TaskProcessor):
|
||||
pr_comments = self.client.get_pull_request_comments(self.owner, self.repo_name, self.item.task_number)
|
||||
except Exception:
|
||||
pass
|
||||
if _is_awaiting_reply_helper(pr_comments):
|
||||
if _is_awaiting_reply_helper(pr_comments, self.ai_username):
|
||||
logger.info(f"PR #{self.item.task_number}: agent asked a question and is awaiting a human reply. Skipping.")
|
||||
return f"SKIP: Awaiting human reply on PR #{self.item.task_number}."
|
||||
|
||||
@@ -482,7 +482,7 @@ class IssueTaskProcessor(TaskProcessor):
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
if _is_awaiting_reply_helper(issue_comments) or _is_awaiting_reply_helper(pr_comments):
|
||||
if _is_awaiting_reply_helper(issue_comments, self.ai_username) or _is_awaiting_reply_helper(pr_comments, self.ai_username):
|
||||
logger.info(f"Issue #{self.item.task_number}: awaiting human reply. Skipping.")
|
||||
return f"SKIP: Awaiting human reply on issue #{self.item.task_number} or PR."
|
||||
|
||||
@@ -678,13 +678,14 @@ class AgentDispatcher:
|
||||
|
||||
results: list[str] = []
|
||||
# Get authenticated username for reviewer filter
|
||||
ai_username = "meeks-ai"
|
||||
try:
|
||||
user = self._client.get_authenticated_user()
|
||||
if user:
|
||||
except Exception as e:
|
||||
raise RuntimeError("No authenticated user found.") from e
|
||||
|
||||
if not user or not user.login:
|
||||
raise RuntimeError("No authenticated user found.")
|
||||
ai_username = user.login
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
for item in work_items:
|
||||
processor: TaskProcessor
|
||||
@@ -722,19 +723,25 @@ class AgentDispatcher:
|
||||
return _find_pr_for_issue_helper(self._client, repo_full_name, issue_number)
|
||||
|
||||
def _is_awaiting_reply(self, comments: list[CommentModel]) -> bool:
|
||||
return _is_awaiting_reply_helper(comments)
|
||||
user = self._client.get_authenticated_user()
|
||||
if not user or not user.login:
|
||||
raise RuntimeError("No authenticated user found.")
|
||||
return _is_awaiting_reply_helper(comments, user.login)
|
||||
|
||||
def _build_pr_mission(self, item: WorkItem) -> str:
|
||||
pr_info = item.task_info
|
||||
if not isinstance(pr_info, PullRequestModel):
|
||||
raise TypeError("Expected task_info to be a PullRequestModel")
|
||||
user = self._client.get_authenticated_user()
|
||||
if not user or not user.login:
|
||||
raise RuntimeError("No authenticated user found.")
|
||||
processor = PRTaskProcessor(
|
||||
client=self._client,
|
||||
tools=self._tools,
|
||||
model_name=self._model_name,
|
||||
repo=item.repo_full_name,
|
||||
item=item,
|
||||
ai_username="meeks-ai",
|
||||
ai_username=user.login,
|
||||
)
|
||||
return processor._build_pr_mission(pr_info, is_own_pr=False)
|
||||
|
||||
@@ -742,12 +749,15 @@ class AgentDispatcher:
|
||||
issue_info = item.task_info
|
||||
if not isinstance(issue_info, IssueModel):
|
||||
raise TypeError("Expected task_info to be an IssueModel")
|
||||
user = self._client.get_authenticated_user()
|
||||
if not user or not user.login:
|
||||
raise RuntimeError("No authenticated user found.")
|
||||
processor = IssueTaskProcessor(
|
||||
client=self._client,
|
||||
tools=self._tools,
|
||||
model_name=self._model_name,
|
||||
repo=item.repo_full_name,
|
||||
item=item,
|
||||
ai_username="meeks-ai",
|
||||
ai_username=user.login,
|
||||
)
|
||||
return processor._build_issue_mission(issue_info, "dummy-branch")
|
||||
|
||||
+2
-2
@@ -52,14 +52,14 @@ class GiteaClient(IssuesClient, PullRequestsClient, FilesClient, RefsClient, Rep
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
def get_authenticated_user(self) -> UserModel | None:
|
||||
def get_authenticated_user(self) -> UserModel:
|
||||
try:
|
||||
response = self.client.get(f"{self.base_url}/api/v1/user")
|
||||
response.raise_for_status()
|
||||
return UserModel(**response.json())
|
||||
except Exception as e:
|
||||
logger.error(f"Error getting authenticated user: {e}", exc_info=True)
|
||||
return None
|
||||
raise RuntimeError(f"Could not get authenticated user: {e}") from e
|
||||
|
||||
def list_all_user_repos(self) -> list[RepositoryModel]:
|
||||
try:
|
||||
|
||||
@@ -104,13 +104,13 @@ async def test_dispatch_planning_and_coding_phases(mock_planning_class: MagicMoc
|
||||
mock_client.get_issue_comments.return_value = []
|
||||
|
||||
from gitea.models import UserModel
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="meeks-ai")
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
|
||||
mock_pr = PullRequestModel(
|
||||
number=42,
|
||||
title="fix bug",
|
||||
body="bug details",
|
||||
user=UserModel(login="meeks-ai")
|
||||
user=UserModel(login="unknown-ai")
|
||||
)
|
||||
mock_client.get_pull_request.return_value = mock_pr
|
||||
mock_client.get_pull_request_diff.return_value = "diff"
|
||||
|
||||
@@ -122,3 +122,14 @@ def test_gitea_client_list_unread_notifications() -> None:
|
||||
assert kwargs.get("params") == {"all": "false", "since": "2026-06-30T21:41:16+02:00"}
|
||||
|
||||
|
||||
import pytest
|
||||
|
||||
def test_gitea_client_get_authenticated_user_failure() -> None:
|
||||
client: GiteaClient = GiteaClient()
|
||||
with patch("httpx.Client.get") as mock_get:
|
||||
mock_get.side_effect = Exception("Connection error")
|
||||
with pytest.raises(RuntimeError, match="Could not get authenticated user"):
|
||||
client.get_authenticated_user()
|
||||
|
||||
|
||||
|
||||
|
||||
+55
-22
@@ -107,6 +107,7 @@ async def test_build_pr_mission_injects_issue_context(mock_agent_class: MagicMoc
|
||||
mock_agent_instance.run_with_tools = AsyncMock(return_value="PR Reviewed.")
|
||||
mock_agent_class.return_value = mock_agent_instance
|
||||
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
dispatcher = AgentDispatcher(client=mock_client, tools=mock_tools)
|
||||
|
||||
work_item = WorkItem(
|
||||
@@ -186,12 +187,12 @@ async def test_dispatch_skips_already_reviewed_pr() -> None:
|
||||
mock_tools: MagicMock = MagicMock(spec=GiteaTools)
|
||||
|
||||
from gitea.models import UserModel
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="meeks-ai")
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
pr = PullRequestModel(
|
||||
number=104,
|
||||
title="already reviewed PR",
|
||||
body="closes #42",
|
||||
user=UserModel(login="meeks-ai")
|
||||
user=UserModel(login="unknown-ai")
|
||||
)
|
||||
mock_client.get_pull_request.return_value = pr
|
||||
mock_client.get_pull_request_diff.return_value = "diff"
|
||||
@@ -222,29 +223,35 @@ def _make_comment(login: str, body: str) -> CommentModel:
|
||||
return CommentModel(id=1, body=body, user=user)
|
||||
|
||||
|
||||
def _make_dispatcher_for_reply_tests() -> AgentDispatcher:
|
||||
mock_client = MagicMock()
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
return AgentDispatcher(client=mock_client, tools=MagicMock())
|
||||
|
||||
|
||||
def test_is_awaiting_reply_no_comments() -> None:
|
||||
dispatcher = AgentDispatcher(client=MagicMock(), tools=MagicMock())
|
||||
dispatcher = _make_dispatcher_for_reply_tests()
|
||||
assert dispatcher._is_awaiting_reply([]) is False
|
||||
|
||||
|
||||
def test_is_awaiting_reply_no_question() -> None:
|
||||
dispatcher = AgentDispatcher(client=MagicMock(), tools=MagicMock())
|
||||
comments = [_make_comment("meeks-ai", "I will fix this now.")]
|
||||
dispatcher = _make_dispatcher_for_reply_tests()
|
||||
comments = [_make_comment("unknown-ai", "I will fix this now.")]
|
||||
assert dispatcher._is_awaiting_reply(comments) is False
|
||||
|
||||
|
||||
def test_is_awaiting_reply_agent_question_no_human_reply() -> None:
|
||||
dispatcher = AgentDispatcher(client=MagicMock(), tools=MagicMock())
|
||||
dispatcher = _make_dispatcher_for_reply_tests()
|
||||
body = "Should I use approach A or B?\n<!-- agent:awaiting-reply -->"
|
||||
comments = [_make_comment("meeks-ai", body)]
|
||||
comments = [_make_comment("unknown-ai", body)]
|
||||
assert dispatcher._is_awaiting_reply(comments) is True
|
||||
|
||||
|
||||
def test_is_awaiting_reply_agent_question_human_replied() -> None:
|
||||
dispatcher = AgentDispatcher(client=MagicMock(), tools=MagicMock())
|
||||
dispatcher = _make_dispatcher_for_reply_tests()
|
||||
body = "Should I use approach A or B?\n<!-- agent:awaiting-reply -->"
|
||||
comments = [
|
||||
_make_comment("meeks-ai", body),
|
||||
_make_comment("unknown-ai", body),
|
||||
_make_comment("michael", "Use approach A please."),
|
||||
]
|
||||
assert dispatcher._is_awaiting_reply(comments) is False
|
||||
@@ -252,8 +259,8 @@ def test_is_awaiting_reply_agent_question_human_replied() -> None:
|
||||
|
||||
def test_is_awaiting_reply_no_marker_not_detected() -> None:
|
||||
"""Agent asked a question but forgot the marker — should NOT be skipped."""
|
||||
dispatcher = AgentDispatcher(client=MagicMock(), tools=MagicMock())
|
||||
comments = [_make_comment("meeks-ai", "Should I use approach A or B?")]
|
||||
dispatcher = _make_dispatcher_for_reply_tests()
|
||||
comments = [_make_comment("unknown-ai", "Should I use approach A or B?")]
|
||||
assert dispatcher._is_awaiting_reply(comments) is False
|
||||
|
||||
|
||||
@@ -264,7 +271,7 @@ async def test_dispatch_proposes_plan(mock_coord_class: MagicMock) -> None:
|
||||
|
||||
mock_client.list_repo_pull_requests.return_value = []
|
||||
mock_client.get_issue_comments.return_value = []
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="meeks-ai")
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
|
||||
async def mock_decide(mission: str, planning_tools: list, coord_tools) -> str:
|
||||
coord_tools.propose_plan(plan="- Add endpoint\n<!-- agent:plan-proposal -->\n<!-- agent:awaiting-reply -->", issue_number=42)
|
||||
@@ -295,7 +302,7 @@ async def test_dispatch_answers_question(mock_coord_class: MagicMock) -> None:
|
||||
|
||||
mock_client.list_repo_pull_requests.return_value = []
|
||||
mock_client.get_issue_comments.return_value = []
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="meeks-ai")
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
|
||||
async def mock_decide(mission: str, planning_tools: list, coord_tools) -> str:
|
||||
coord_tools.answer_question(answer="X works by doing Y.\n<!-- agent:question-response -->\n<!-- agent:awaiting-reply -->", issue_number=42)
|
||||
@@ -325,11 +332,11 @@ async def test_dispatch_closes_issue_on_satisfaction(mock_coord_class: MagicMock
|
||||
mock_tools: MagicMock = MagicMock(spec=GiteaTools)
|
||||
|
||||
mock_client.list_repo_pull_requests.return_value = []
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="meeks-ai")
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
|
||||
# Human comments indicating satisfaction after our answer
|
||||
mock_client.get_issue_comments.return_value = [
|
||||
_make_comment("meeks-ai", "Here is the answer.\n<!-- agent:question-response -->\n<!-- agent:awaiting-reply -->"),
|
||||
_make_comment("unknown-ai", "Here is the answer.\n<!-- agent:question-response -->\n<!-- agent:awaiting-reply -->"),
|
||||
_make_comment("michael", "Yes, thanks! That makes sense.")
|
||||
]
|
||||
|
||||
@@ -364,9 +371,9 @@ async def test_dispatch_executes_approved_plan_and_creates_wip_pr(mock_coord_cla
|
||||
mock_tools: MagicMock = MagicMock(spec=GiteaTools)
|
||||
|
||||
mock_client.list_repo_pull_requests.return_value = []
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="meeks-ai")
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
mock_client.get_issue_comments.return_value = [
|
||||
_make_comment("meeks-ai", "### Proposed Plan\n<!-- agent:plan-proposal -->\n<!-- agent:awaiting-reply -->"),
|
||||
_make_comment("unknown-ai", "### Proposed Plan\n<!-- agent:plan-proposal -->\n<!-- agent:awaiting-reply -->"),
|
||||
_make_comment("michael", "looks good, go ahead")
|
||||
]
|
||||
|
||||
@@ -414,7 +421,7 @@ async def test_dispatch_resumes_wip_pr(mock_coord_class: MagicMock, mock_coding_
|
||||
mock_client: MagicMock = MagicMock(spec=GiteaClient)
|
||||
mock_tools: MagicMock = MagicMock(spec=GiteaTools)
|
||||
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="meeks-ai")
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
|
||||
# Existing WIP PR addressing issue #42
|
||||
wip_pr = PullRequestModel(
|
||||
@@ -427,7 +434,7 @@ async def test_dispatch_resumes_wip_pr(mock_coord_class: MagicMock, mock_coding_
|
||||
mock_client.get_pr_reviews.return_value = []
|
||||
|
||||
mock_client.get_issue_comments.return_value = [
|
||||
_make_comment("meeks-ai", "### Proposed Plan\n<!-- agent:plan-proposal -->\n<!-- agent:awaiting-reply -->"),
|
||||
_make_comment("unknown-ai", "### Proposed Plan\n<!-- agent:plan-proposal -->\n<!-- agent:awaiting-reply -->"),
|
||||
_make_comment("michael", "looks good, go ahead")
|
||||
]
|
||||
mock_client.get_pull_request_comments.return_value = []
|
||||
@@ -462,7 +469,7 @@ async def test_dispatch_skips_pr_if_not_requested_reviewer() -> None:
|
||||
mock_client: MagicMock = MagicMock(spec=GiteaClient)
|
||||
mock_tools: MagicMock = MagicMock(spec=GiteaTools)
|
||||
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="meeks-ai")
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
|
||||
# PR authored by michael, requested reviewers is empty (agent not requested)
|
||||
pr_detail = PullRequestModel(
|
||||
@@ -511,7 +518,7 @@ async def test_dispatch_uses_coordinator_tool_calling(mock_coord_class: MagicMoc
|
||||
|
||||
mock_client.list_repo_pull_requests.return_value = []
|
||||
mock_client.get_issue_comments.return_value = []
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="meeks-ai")
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
|
||||
# Mock agent invoking propose_plan tool
|
||||
async def mock_decide(mission: str, planning_tools: list, coord_tools) -> str:
|
||||
@@ -549,7 +556,7 @@ async def test_dispatcher_raises_type_error_for_invalid_task_info() -> None:
|
||||
# Mock return values for methods called prior to the isinstance check
|
||||
mock_client.list_repo_pull_requests.return_value = []
|
||||
mock_client.get_issue_comments.return_value = []
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="meeks-ai")
|
||||
mock_client.get_authenticated_user.return_value = UserModel(login="unknown-ai")
|
||||
|
||||
dispatcher = AgentDispatcher(client=mock_client, tools=mock_tools)
|
||||
|
||||
@@ -580,4 +587,30 @@ async def test_dispatcher_raises_type_error_for_invalid_task_info() -> None:
|
||||
dispatcher._build_issue_mission(work_item_invalid_issue)
|
||||
|
||||
|
||||
async def test_dispatch_fails_if_no_authenticated_user() -> None:
|
||||
mock_client: MagicMock = MagicMock(spec=GiteaClient)
|
||||
mock_tools: MagicMock = MagicMock(spec=GiteaTools)
|
||||
|
||||
# Simulate get_authenticated_user returning None
|
||||
mock_client.get_authenticated_user.return_value = None
|
||||
|
||||
dispatcher = AgentDispatcher(client=mock_client, tools=mock_tools)
|
||||
work_item = WorkItem(
|
||||
repo_full_name="meeks/repo1",
|
||||
task_type="issue",
|
||||
task_number=42,
|
||||
task_info=IssueModel(number=42, title="add X", body=""),
|
||||
priority=0
|
||||
)
|
||||
|
||||
with pytest.raises(RuntimeError, match="No authenticated user found."):
|
||||
await dispatcher.dispatch("meeks/repo1", [work_item])
|
||||
|
||||
# Simulate get_authenticated_user raising an Exception
|
||||
mock_client.get_authenticated_user.side_effect = Exception("API error")
|
||||
with pytest.raises(RuntimeError, match="No authenticated user found."):
|
||||
await dispatcher.dispatch("meeks/repo1", [work_item])
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user