diff --git a/docs/connectors_list.md b/docs/connectors_list.md index 02bf4d2..4b8a0ff 100644 --- a/docs/connectors_list.md +++ b/docs/connectors_list.md @@ -56,3 +56,43 @@ CLI flow: 3. Run `octopal connector auth google` 4. Run `octopal connector status` 5. Restart Octopal if needed + +## GitHub + +Current supported services: +- Repositories +- Issues +- Pull requests + +What it can do today: +- inspect the authenticated GitHub account +- list repositories visible to that account +- inspect repository metadata +- list repository issues +- read a single issue +- create issues +- update issues +- list issue and pull request conversation comments +- create issue and pull request conversation comments +- update issue and pull request conversation comments +- list pull requests +- read a single pull request +- list pull request reviews +- list inline review comments on pull requests +- create pull request reviews including comment/approve/request changes +- list changed files in pull requests, including patch hunks when GitHub provides them +- list commits included in pull requests +- list commit comments for specific commit SHAs +- summarize pull request merge readiness without merging + +What it does not do yet: +- create or merge pull requests +- read GitHub Actions runs +- read code contents through the connector + +CLI flow: +1. Run `octopal configure` +2. Enable any needed GitHub services such as `Repositories`, `Issues`, and/or `Pull Requests` +3. Run `octopal connector auth github` +4. Run `octopal connector status` +5. Restart Octopal if needed diff --git a/src/octopal/cli/configure.py b/src/octopal/cli/configure.py index 1b99ddc..fe27710 100644 --- a/src/octopal/cli/configure.py +++ b/src/octopal/cli/configure.py @@ -589,7 +589,8 @@ def _configure_connectors(config: OctopalConfig, prompter) -> None: "Connectors (experimental)", [ "Connectors allow Octo to link with external services through explicit CLI setup.", - "Google connector support currently covers Gmail and Calendar.", + "Google connector support covers Gmail, Calendar, and Drive.", + "GitHub connector support covers repositories, issues, and pull requests.", "After saving, Octopal will tell you which CLI command to run next for authorization.", ], ) @@ -600,6 +601,11 @@ def _configure_connectors(config: OctopalConfig, prompter) -> None: label="Google", hint="Integrate with Gmail and Google Calendar. More Google services can land on the same connector flow later.", ), + WizardSelectOption( + value="github", + label="GitHub", + hint="Inspect repositories, issues, and pull requests with a personal access token.", + ), ] initial_values = [ @@ -649,6 +655,26 @@ def _configure_connectors(config: OctopalConfig, prompter) -> None: ) ) config.connectors.instances[name].enabled_services = selected_google + if name == "github" and is_enabled: + github_services = [ + WizardSelectOption(value="repos", label="Repositories"), + WizardSelectOption(value="issues", label="Issues"), + WizardSelectOption(value="pull_requests", label="Pull Requests"), + ] + + current_github_services = config.connectors.instances[name].enabled_services or ["repos"] + current_github_services = [ + service for service in current_github_services if service in {"repos", "issues", "pull_requests"} + ] or ["repos"] + + selected_github = prompter.multiselect( + WizardMultiSelectParams( + message="Select specific GitHub services to enable", + initial_values=current_github_services, + options=github_services, + ) + ) + config.connectors.instances[name].enabled_services = selected_github def _build_sections(config: OctopalConfig, prompter) -> list[WizardSection]: @@ -850,6 +876,29 @@ def _collect_connector_next_steps(config: OctopalConfig, previous_config: Octopa " [magenta]octopal restart[/magenta] - Reload Octopal after connector authorization." ) + github = config.connectors.instances.get("github") + if github and github.enabled: + current_services = set(_enabled_services(config, "github")) + previous_services = set(_enabled_services(previous_config, "github")) + authorized_services = set(_authorized_services(config, "github")) + has_token = bool(github.auth.access_token) + previous_github = previous_config.connectors.instances.get("github") + + needs_auth = not has_token or not current_services.issubset(authorized_services) + services_added = sorted(current_services - previous_services) + newly_enabled = previous_github is None or not previous_github.enabled + + if needs_auth or services_added or newly_enabled: + lines.append( + " [magenta]octopal connector auth github[/magenta] - Authorize GitHub for the enabled services." + ) + lines.append( + " [magenta]octopal connector status[/magenta] - Verify connector status after authorization." + ) + lines.append( + " [magenta]octopal restart[/magenta] - Reload Octopal after connector authorization." + ) + return lines diff --git a/src/octopal/cli/main.py b/src/octopal/cli/main.py index c1c18ca..ee2ae80 100644 --- a/src/octopal/cli/main.py +++ b/src/octopal/cli/main.py @@ -1087,11 +1087,11 @@ def _connector_disconnect_message(name: str, *, forget_credentials: bool) -> str if forget_credentials: return ( f"[bold yellow]{name} disconnected.[/bold yellow] " - "Stored client credentials were removed." + "Stored credentials were removed." ) return ( f"[bold yellow]{name} disconnected.[/bold yellow] " - "Stored client credentials were kept so you can re-authorize later." + "Stored credentials were kept so you can re-authorize later." ) @@ -1122,6 +1122,18 @@ def _print_google_headless_auth_help(auth_url: str) -> None: console.print() +def _print_github_auth_setup_help() -> None: + console.print() + console.print("[bold]GitHub token setup[/bold]") + console.print("You need a GitHub personal access token for the account Octopal should use.") + console.print("Fine-grained tokens are preferred when they cover the repositories you want.") + console.print(" 1. Open [cyan]https://github.com/settings/personal-access-tokens[/cyan]") + console.print(" 2. Create a fine-grained token or a classic token if your setup needs it") + console.print(" 3. Grant read access to repository metadata, issues, and pull requests as needed") + console.print(" 4. Paste the token here when prompted") + console.print() + + @connector_app.command("status") def connector_status(json_output: bool = typer.Option(False, "--json", help="Print machine-readable JSON.")) -> None: """Show connector status and any required next action.""" @@ -1160,6 +1172,7 @@ def connector_auth( name: str = typer.Argument(..., help="Connector name to authorize."), client_id: str | None = typer.Option(None, "--client-id", help="OAuth client ID to use."), client_secret: str | None = typer.Option(None, "--client-secret", help="OAuth client secret to use."), + token: str | None = typer.Option(None, "--token", help="Access token to use for token-based connectors."), ) -> None: """Authorize a configured connector via CLI.""" settings = load_settings() @@ -1178,22 +1191,28 @@ def connector_auth( if name == "google": _print_google_auth_setup_help() - resolved_client_id = client_id or typer.prompt( - "Your Google OAuth Desktop App client ID" - ) - resolved_client_secret = client_secret or typer.prompt( - "Your Google OAuth Desktop App client secret", - hide_input=True, - ) - - asyncio.run( - connector.configure( - { - "client_id": resolved_client_id, - "client_secret": resolved_client_secret, - } + resolved_client_id = client_id or typer.prompt( + "Your Google OAuth Desktop App client ID" ) - ) + resolved_client_secret = client_secret or typer.prompt( + "Your Google OAuth Desktop App client secret", + hide_input=True, + ) + asyncio.run( + connector.configure( + { + "client_id": resolved_client_id, + "client_secret": resolved_client_secret, + } + ) + ) + elif name == "github": + _print_github_auth_setup_help() + resolved_token = token or typer.prompt( + "Your GitHub personal access token", + hide_input=True, + ) + asyncio.run(connector.configure({"token": resolved_token})) result = asyncio.run(connector.authorize()) if result.get("status") == "manual_required" and name == "google": diff --git a/src/octopal/infrastructure/connectors/github.py b/src/octopal/infrastructure/connectors/github.py new file mode 100644 index 0000000..b9d9c8d --- /dev/null +++ b/src/octopal/infrastructure/connectors/github.py @@ -0,0 +1,289 @@ +from __future__ import annotations + +import sys +from typing import Any + +import httpx +import structlog + +from octopal.infrastructure.connectors.base import Connector + +logger = structlog.get_logger(__name__) + + +class GitHubConnector(Connector): + _SUPPORTED_SERVICES = ("repos", "issues", "pull_requests") + _SERVER_ID = "github-core" + + def __init__(self, manager: Any): + self.manager = manager + + @property + def name(self) -> str: + return "github" + + def _get_config(self): + return self.manager.config.instances.get(self.name) + + def supported_services(self) -> list[str]: + return list(self._SUPPORTED_SERVICES) + + def managed_server_ids(self) -> list[str]: + return [self._SERVER_ID] if self._get_enabled_services() else [] + + def _get_enabled_services(self) -> list[str]: + config = self._get_config() + if not config: + return [] + enabled = [str(service).strip().lower() for service in config.enabled_services if str(service).strip()] + deduped: list[str] = [] + for service in enabled: + if service not in deduped: + deduped.append(service) + return deduped + + def _get_authorized_services(self) -> list[str]: + config = self._get_config() + if not config: + return [] + return [ + str(service).strip().lower() + for service in config.auth.authorized_services + if str(service).strip() + ] + + def _unsupported_enabled_services(self) -> list[str]: + supported = set(self.supported_services()) + return [service for service in self._get_enabled_services() if service not in supported] + + def _validate_classic_token_scopes( + self, + *, + enabled_services: list[str], + scopes_header: str, + ) -> dict[str, Any]: + scopes = [scope.strip() for scope in scopes_header.split(",") if scope.strip()] + scope_set = set(scopes) + warnings: list[str] = [] + + if any(service in {"issues", "pull_requests"} for service in enabled_services): + if "repo" not in scope_set and "public_repo" not in scope_set: + return { + "ok": False, + "scopes": scopes, + "error": ( + "GitHub token is valid, but it does not include write access for issues or pull requests. " + "Classic PATs need `public_repo` for public repositories or `repo` for private repositories." + ), + } + if "public_repo" in scope_set and "repo" not in scope_set: + warnings.append( + "Classic PAT includes `public_repo` but not `repo`, so writes will work only on public repositories." + ) + + return {"ok": True, "scopes": scopes, "warnings": warnings} + + async def get_status(self) -> dict[str, Any]: + config = self._get_config() + if not config or not config.enabled: + return {"status": "disabled"} + + enabled_services = self._get_enabled_services() + unsupported_services = self._unsupported_enabled_services() + if unsupported_services: + return { + "status": "unsupported_service_configuration", + "message": ( + "GitHub connector is configured with unsupported services: " + f"{', '.join(unsupported_services)}. Re-run `octopal configure` and keep only supported GitHub services." + ), + "services": enabled_services, + "supported_services": self.supported_services(), + } + + if not enabled_services: + return { + "status": "misconfigured", + "message": "GitHub connector is enabled but no supported services are selected.", + "services": [], + "supported_services": self.supported_services(), + } + + if not config.auth.access_token: + return { + "status": "needs_auth", + "message": "Connector configured but needs a GitHub personal access token.", + "services": enabled_services, + "supported_services": self.supported_services(), + } + + authorized_services = set(self._get_authorized_services()) + enabled_service_set = set(enabled_services) + if not enabled_service_set.issubset(authorized_services): + missing = sorted(enabled_service_set - authorized_services) + return { + "status": "needs_reauth", + "message": ( + "GitHub connector needs re-authorization for newly enabled services: " + f"{', '.join(missing)}" + ), + "services": enabled_services, + "supported_services": self.supported_services(), + } + + return { + "status": "ready", + "message": f"GitHub connector is ready with services: {', '.join(enabled_services)}", + "services": enabled_services, + "supported_services": self.supported_services(), + } + + async def configure(self, settings: dict[str, Any]) -> None: + config = self._get_config() + if not config: + raise RuntimeError( + "GitHub connector is not enabled. Run `octopal configure` and enable GitHub first." + ) + if not config.enabled: + raise RuntimeError( + "GitHub connector is disabled. Re-run `octopal configure` to enable it before authorizing." + ) + + token = settings.get("token") + if token is not None: + config.auth.access_token = str(token).strip() or None + if config.auth.access_token is None: + config.auth.authorized_services = [] + self.manager.save_config() + + async def authorize(self) -> dict[str, Any]: + config = self._get_config() + if not config: + return {"error": "GitHub connector is not enabled. Run `octopal configure` first."} + if not config.enabled: + return {"error": "GitHub connector is disabled. Run `octopal configure` to enable it first."} + + unsupported_services = self._unsupported_enabled_services() + if unsupported_services: + return { + "error": ( + "Unsupported GitHub services are enabled: " + f"{', '.join(unsupported_services)}. Re-run `octopal configure` and keep only supported GitHub services." + ) + } + + enabled_services = self._get_enabled_services() + if not enabled_services: + return {"error": "No supported GitHub services are enabled. Re-run `octopal configure`."} + + token = str(config.auth.access_token or "").strip() + if not token: + return {"error": "Missing GitHub personal access token."} + + try: + async with httpx.AsyncClient(base_url="https://api.github.com", timeout=20.0) as client: + response = await client.get( + "/user", + headers={ + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }, + ) + if response.status_code == 401: + return {"error": "GitHub rejected the token. Check that the personal access token is valid."} + if response.is_error: + detail = response.text.strip() or response.reason_phrase + return {"error": f"GitHub authorization check failed: {detail}"} + + scopes_header = response.headers.get("X-OAuth-Scopes", "").strip() + auth_note = "" + if scopes_header: + validation = self._validate_classic_token_scopes( + enabled_services=enabled_services, + scopes_header=scopes_header, + ) + if not validation["ok"]: + return {"error": validation["error"]} + warnings = validation.get("warnings") or [] + if warnings: + auth_note = " " + " ".join(warnings) + else: + auth_note = ( + " GitHub did not expose classic scope headers for this token. " + "If this is a fine-grained token, repository permissions will still be enforced per repository at runtime." + ) + except Exception as exc: + logger.exception("Failed to authorize GitHub connector") + return {"error": f"Failed to authorize GitHub connector: {exc}"} + + config.auth.authorized_services = enabled_services + config.auth.last_error = None + self.manager.save_config() + + if self.manager.mcp_manager is not None: + await self.start() + + return { + "status": "success", + "message": f"GitHub connector authorized for {', '.join(enabled_services)}.{auth_note}", + } + + async def start(self) -> None: + if self.manager.mcp_manager is None: + return + + config = self._get_config() + if not config or not config.enabled: + return + + enabled_services = self._get_enabled_services() + if not enabled_services or not config.auth.access_token: + return + + from octopal.infrastructure.mcp.manager import MCPServerConfig + + github_cfg = MCPServerConfig( + id=self._SERVER_ID, + name="GitHub Connector", + command=sys.executable, + args=["-m", "octopal.mcp_servers.github"], + env={ + "GITHUB_TOKEN": config.auth.access_token, + "GITHUB_ENABLED_SERVICES": ",".join(enabled_services), + }, + transport="stdio", + ) + await self.manager.mcp_manager.connect_server(github_cfg) + + async def disconnect(self, *, forget_credentials: bool = False) -> dict[str, Any]: + config = self._get_config() + if not config: + return {"status": "noop", "message": "GitHub connector is not configured."} + + if self.manager.mcp_manager is not None: + for server_id in self.managed_server_ids(): + try: + await self.manager.mcp_manager.disconnect_server(server_id, intentional=True) + except Exception: + logger.warning( + "Failed to disconnect MCP server for GitHub connector", + server_id=server_id, + exc_info=True, + ) + + config.auth.authorized_services = [] + config.auth.last_error = None + if forget_credentials: + config.auth.refresh_token = None + config.auth.access_token = None + self.manager.save_config() + + return { + "status": "success", + "message": ( + "GitHub connector disconnected and stored access credentials were removed." + if forget_credentials + else "GitHub connector disconnected. Stored access credentials were kept." + ), + } diff --git a/src/octopal/infrastructure/connectors/manager.py b/src/octopal/infrastructure/connectors/manager.py index 16e1766..f0174ea 100644 --- a/src/octopal/infrastructure/connectors/manager.py +++ b/src/octopal/infrastructure/connectors/manager.py @@ -5,6 +5,7 @@ import structlog from octopal.infrastructure.config.models import ConnectorsConfig +from octopal.infrastructure.connectors.github import GitHubConnector from octopal.infrastructure.connectors.google import GoogleConnector if TYPE_CHECKING: @@ -24,7 +25,8 @@ def __init__( self.mcp_manager = mcp_manager self.octo_config = octo_config self.connectors = { - "google": GoogleConnector(self) + "github": GitHubConnector(self), + "google": GoogleConnector(self), } def get_connector(self, name: str): diff --git a/src/octopal/mcp_servers/github.py b/src/octopal/mcp_servers/github.py new file mode 100644 index 0000000..95cc02a --- /dev/null +++ b/src/octopal/mcp_servers/github.py @@ -0,0 +1,1131 @@ +from __future__ import annotations + +import asyncio +import json +import os +from typing import Any + +import httpx +from mcp.server import FastMCP + +_GITHUB_API_BASE_URL = "https://api.github.com" +_DEFAULT_HEADERS = { + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", +} + + +class GitHubConfigError(RuntimeError): + """Raised when required GitHub MCP configuration is missing.""" + + +class GitHubApiError(RuntimeError): + """Raised when GitHub returns a structured error response.""" + + def __init__( + self, + *, + status_code: int, + message: str, + details: dict[str, Any] | None = None, + method: str | None = None, + path: str | None = None, + ) -> None: + self.status_code = status_code + self.details = details or {} + self.method = method + self.path = path + super().__init__(f"GitHub API {status_code}: {message}") + + +_MAX_LIST_PER_PAGE = 50 + + +def _clamp_per_page(per_page: int) -> int: + return max(1, min(per_page, _MAX_LIST_PER_PAGE)) + + +def _format_permission_hint(method: str, path: str, response: httpx.Response) -> str | None: + accepted_permissions = response.headers.get("X-Accepted-GitHub-Permissions", "").strip() + accepted_suffix = f" GitHub reports accepted permissions: {accepted_permissions}." if accepted_permissions else "" + + if response.status_code == 403: + if method == "POST" and "/issues/" in path and path.endswith("/comments"): + return ( + "GitHub denied write access for issue or pull request conversation comments. " + "Fine-grained PATs usually need Issues: write or Pull requests: write. " + "Classic PATs need public_repo for public repos or repo for private repos." + f"{accepted_suffix}" + ) + if method == "PATCH" and "/issues/comments/" in path: + return ( + "GitHub denied write access for updating an issue or pull request conversation comment. " + "Fine-grained PATs usually need Issues: write or Pull requests: write. " + "Classic PATs need public_repo for public repos or repo for private repos." + f"{accepted_suffix}" + ) + if method == "POST" and "/pulls/" in path and path.endswith("/reviews"): + return ( + "GitHub denied write access for creating a pull request review. " + "Fine-grained PATs need Pull requests: write. " + "Classic PATs need public_repo for public repos or repo for private repos." + f"{accepted_suffix}" + ) + if method == "POST" and path.endswith("/issues"): + return ( + "GitHub denied write access for creating an issue. " + "Fine-grained PATs need Issues: write. " + "Classic PATs need public_repo for public repos or repo for private repos." + f"{accepted_suffix}" + ) + if method == "PATCH" and "/issues/" in path and "/comments/" not in path: + return ( + "GitHub denied write access for updating an issue. " + "Fine-grained PATs need Issues: write. " + "Classic PATs need public_repo for public repos or repo for private repos." + f"{accepted_suffix}" + ) + return ( + "GitHub denied access for this request. " + "For fine-grained PATs, confirm the token has repository access to this repo and the required repository permission." + f"{accepted_suffix}" + ) + + if response.status_code == 404 and path.startswith("/repos/"): + return ( + "GitHub returned 404 for this repository resource. " + "Check the owner/repo slug and, for fine-grained PATs, confirm the token includes this repository under Repository access." + ) + + return None + + +def _parse_github_api_error(response: httpx.Response, *, method: str, path: str) -> GitHubApiError: + payload: dict[str, Any] = {} + try: + payload = response.json() + except json.JSONDecodeError: + text = response.text.strip() or response.reason_phrase + hint = _format_permission_hint(method, path, response) + return GitHubApiError( + status_code=response.status_code, + message=hint or text, + details={}, + method=method, + path=path, + ) + + message = str(payload.get("message") or response.reason_phrase or "Unknown GitHub API error").strip() + hint = _format_permission_hint(method, path, response) + details = dict(payload) + accepted_permissions = response.headers.get("X-Accepted-GitHub-Permissions", "").strip() + oauth_scopes = response.headers.get("X-OAuth-Scopes", "").strip() + if accepted_permissions: + details["accepted_permissions"] = accepted_permissions + if oauth_scopes: + details["oauth_scopes"] = oauth_scopes + return GitHubApiError( + status_code=response.status_code, + message=hint or message, + details=details, + method=method, + path=path, + ) + + +def _normalize_user(user: dict[str, Any] | None) -> dict[str, Any] | None: + if not isinstance(user, dict): + return None + return { + "login": user.get("login"), + "id": user.get("id"), + "type": user.get("type"), + "site_admin": user.get("site_admin", False), + "html_url": user.get("html_url"), + } + + +def _normalize_repo(repo: dict[str, Any]) -> dict[str, Any]: + owner = _normalize_user(repo.get("owner")) + return { + "id": repo.get("id"), + "name": repo.get("name"), + "full_name": repo.get("full_name"), + "private": repo.get("private", False), + "default_branch": repo.get("default_branch"), + "description": repo.get("description"), + "html_url": repo.get("html_url"), + "clone_url": repo.get("clone_url"), + "ssh_url": repo.get("ssh_url"), + "visibility": repo.get("visibility"), + "language": repo.get("language"), + "fork": repo.get("fork", False), + "archived": repo.get("archived", False), + "disabled": repo.get("disabled", False), + "open_issues_count": repo.get("open_issues_count"), + "stargazers_count": repo.get("stargazers_count"), + "watchers_count": repo.get("watchers_count"), + "forks_count": repo.get("forks_count"), + "updated_at": repo.get("updated_at"), + "pushed_at": repo.get("pushed_at"), + "owner": owner, + } + + +def _normalize_issue(issue: dict[str, Any]) -> dict[str, Any]: + assignees = issue.get("assignees") or [] + labels = issue.get("labels") or [] + return { + "id": issue.get("id"), + "number": issue.get("number"), + "title": issue.get("title"), + "state": issue.get("state"), + "state_reason": issue.get("state_reason"), + "html_url": issue.get("html_url"), + "comments": issue.get("comments"), + "created_at": issue.get("created_at"), + "updated_at": issue.get("updated_at"), + "closed_at": issue.get("closed_at"), + "body": issue.get("body"), + "user": _normalize_user(issue.get("user")), + "assignees": [_normalize_user(assignee) for assignee in assignees], + "labels": [ + { + "name": label.get("name"), + "color": label.get("color"), + "description": label.get("description"), + } + for label in labels + if isinstance(label, dict) + ], + "pull_request": issue.get("pull_request"), + } + + +def _normalize_issue_comment(comment: dict[str, Any]) -> dict[str, Any]: + return { + "id": comment.get("id"), + "node_id": comment.get("node_id"), + "html_url": comment.get("html_url"), + "body": comment.get("body"), + "created_at": comment.get("created_at"), + "updated_at": comment.get("updated_at"), + "author_association": comment.get("author_association"), + "user": _normalize_user(comment.get("user")), + } + + +def _normalize_pull_request(pr: dict[str, Any]) -> dict[str, Any]: + head = pr.get("head") or {} + base = pr.get("base") or {} + requested_reviewers = pr.get("requested_reviewers") or [] + requested_teams = pr.get("requested_teams") or [] + return { + "id": pr.get("id"), + "number": pr.get("number"), + "title": pr.get("title"), + "state": pr.get("state"), + "draft": pr.get("draft", False), + "merged": pr.get("merged"), + "mergeable": pr.get("mergeable"), + "mergeable_state": pr.get("mergeable_state"), + "rebaseable": pr.get("rebaseable"), + "html_url": pr.get("html_url"), + "created_at": pr.get("created_at"), + "updated_at": pr.get("updated_at"), + "closed_at": pr.get("closed_at"), + "merged_at": pr.get("merged_at"), + "body": pr.get("body"), + "comments": pr.get("comments"), + "review_comments": pr.get("review_comments"), + "commits": pr.get("commits"), + "additions": pr.get("additions"), + "deletions": pr.get("deletions"), + "changed_files": pr.get("changed_files"), + "user": _normalize_user(pr.get("user")), + "requested_reviewers": [_normalize_user(user) for user in requested_reviewers], + "requested_teams": [ + { + "id": team.get("id"), + "name": team.get("name"), + "slug": team.get("slug"), + } + for team in requested_teams + if isinstance(team, dict) + ], + "head": { + "label": head.get("label"), + "ref": head.get("ref"), + "sha": head.get("sha"), + "repo": _normalize_repo(head.get("repo")) if isinstance(head.get("repo"), dict) else None, + }, + "base": { + "label": base.get("label"), + "ref": base.get("ref"), + "sha": base.get("sha"), + "repo": _normalize_repo(base.get("repo")) if isinstance(base.get("repo"), dict) else None, + }, + } + + +def _normalize_pull_review(review: dict[str, Any]) -> dict[str, Any]: + return { + "id": review.get("id"), + "node_id": review.get("node_id"), + "user": _normalize_user(review.get("user")), + "body": review.get("body"), + "state": review.get("state"), + "html_url": review.get("html_url"), + "submitted_at": review.get("submitted_at"), + "commit_id": review.get("commit_id"), + "author_association": review.get("author_association"), + } + + +def _normalize_pull_review_comment(comment: dict[str, Any]) -> dict[str, Any]: + return { + "id": comment.get("id"), + "node_id": comment.get("node_id"), + "path": comment.get("path"), + "position": comment.get("position"), + "line": comment.get("line"), + "side": comment.get("side"), + "commit_id": comment.get("commit_id"), + "body": comment.get("body"), + "html_url": comment.get("html_url"), + "created_at": comment.get("created_at"), + "updated_at": comment.get("updated_at"), + "author_association": comment.get("author_association"), + "user": _normalize_user(comment.get("user")), + } + + +def _normalize_pull_file(file: dict[str, Any]) -> dict[str, Any]: + return { + "sha": file.get("sha"), + "filename": file.get("filename"), + "status": file.get("status"), + "additions": file.get("additions"), + "deletions": file.get("deletions"), + "changes": file.get("changes"), + "blob_url": file.get("blob_url"), + "raw_url": file.get("raw_url"), + "contents_url": file.get("contents_url"), + "patch": file.get("patch"), + "previous_filename": file.get("previous_filename"), + } + + +def _normalize_commit(commit_payload: dict[str, Any]) -> dict[str, Any]: + commit = commit_payload.get("commit") or {} + author = commit.get("author") or {} + committer = commit.get("committer") or {} + verification = commit.get("verification") or {} + parents = commit_payload.get("parents") or [] + return { + "sha": commit_payload.get("sha"), + "html_url": commit_payload.get("html_url"), + "message": commit.get("message"), + "author": { + "name": author.get("name"), + "email": author.get("email"), + "date": author.get("date"), + "user": _normalize_user(commit_payload.get("author")), + }, + "committer": { + "name": committer.get("name"), + "email": committer.get("email"), + "date": committer.get("date"), + "user": _normalize_user(commit_payload.get("committer")), + }, + "parents": [{"sha": parent.get("sha"), "url": parent.get("url")} for parent in parents if isinstance(parent, dict)], + "comment_count": commit.get("comment_count"), + "verification": { + "verified": verification.get("verified"), + "reason": verification.get("reason"), + "verified_at": verification.get("verified_at"), + }, + } + + +def _normalize_commit_comment(comment: dict[str, Any]) -> dict[str, Any]: + return { + "id": comment.get("id"), + "node_id": comment.get("node_id"), + "path": comment.get("path"), + "position": comment.get("position"), + "line": comment.get("line"), + "commit_id": comment.get("commit_id"), + "body": comment.get("body"), + "html_url": comment.get("html_url"), + "created_at": comment.get("created_at"), + "updated_at": comment.get("updated_at"), + "author_association": comment.get("author_association"), + "user": _normalize_user(comment.get("user")), + } + + +class GitHubApiClient: + def __init__(self) -> None: + self._token = self._load_token() + self._client = httpx.AsyncClient(base_url=_GITHUB_API_BASE_URL, timeout=30.0) + + def _load_token(self) -> str: + token = os.getenv("GITHUB_TOKEN", "").strip() + if not token: + raise GitHubConfigError("Missing GitHub MCP credentials. Expected GITHUB_TOKEN.") + return token + + async def close(self) -> None: + await self._client.aclose() + + async def _request( + self, + method: str, + path: str, + *, + params: dict[str, Any] | None = None, + json_body: dict[str, Any] | None = None, + ) -> Any: + response = await self._client.request( + method, + path, + params=params, + json=json_body, + headers={ + **_DEFAULT_HEADERS, + "Authorization": f"Bearer {self._token}", + }, + ) + if response.is_error: + raise _parse_github_api_error(response, method=method, path=path) + if not response.content: + return {} + return response.json() + + async def get_authenticated_user(self) -> dict[str, Any]: + payload = await self._request("GET", "/user") + return { + "login": payload.get("login"), + "id": payload.get("id"), + "name": payload.get("name"), + "email": payload.get("email"), + "company": payload.get("company"), + "html_url": payload.get("html_url"), + "avatar_url": payload.get("avatar_url"), + } + + async def list_repositories( + self, + *, + visibility: str | None = None, + affiliation: str | None = None, + sort: str = "updated", + direction: str = "desc", + per_page: int = 20, + page: int = 1, + ) -> dict[str, Any]: + params: dict[str, Any] = { + "sort": sort, + "direction": direction, + "per_page": _clamp_per_page(per_page), + "page": max(1, page), + } + if visibility: + params["visibility"] = visibility + if affiliation: + params["affiliation"] = affiliation + payload = await self._request("GET", "/user/repos", params=params) + return {"repositories": [_normalize_repo(repo) for repo in payload]} + + async def get_repository(self, *, owner: str, repo: str) -> dict[str, Any]: + payload = await self._request("GET", f"/repos/{owner}/{repo}") + return _normalize_repo(payload) + + async def list_issues( + self, + *, + owner: str, + repo: str, + state: str = "open", + labels: str | None = None, + since: str | None = None, + per_page: int = 20, + page: int = 1, + ) -> dict[str, Any]: + params: dict[str, Any] = { + "state": state, + "per_page": _clamp_per_page(per_page), + "page": max(1, page), + } + if labels: + params["labels"] = labels + if since: + params["since"] = since + payload = await self._request("GET", f"/repos/{owner}/{repo}/issues", params=params) + return { + "owner": owner, + "repo": repo, + "issues": [_normalize_issue(issue) for issue in payload], + } + + async def get_issue(self, *, owner: str, repo: str, issue_number: int) -> dict[str, Any]: + payload = await self._request("GET", f"/repos/{owner}/{repo}/issues/{issue_number}") + return _normalize_issue(payload) + + async def create_issue( + self, + *, + owner: str, + repo: str, + title: str, + body: str | None = None, + assignees: list[str] | None = None, + labels: list[str] | None = None, + milestone: int | None = None, + ) -> dict[str, Any]: + request_body: dict[str, Any] = {"title": title} + if body is not None: + request_body["body"] = body + if assignees is not None: + request_body["assignees"] = assignees + if labels is not None: + request_body["labels"] = labels + if milestone is not None: + request_body["milestone"] = milestone + payload = await self._request("POST", f"/repos/{owner}/{repo}/issues", json_body=request_body) + return _normalize_issue(payload) + + async def update_issue( + self, + *, + owner: str, + repo: str, + issue_number: int, + title: str | None = None, + body: str | None = None, + assignees: list[str] | None = None, + labels: list[str] | None = None, + milestone: int | None = None, + state: str | None = None, + state_reason: str | None = None, + ) -> dict[str, Any]: + request_body: dict[str, Any] = {} + if title is not None: + request_body["title"] = title + if body is not None: + request_body["body"] = body + if assignees is not None: + request_body["assignees"] = assignees + if labels is not None: + request_body["labels"] = labels + if milestone is not None: + request_body["milestone"] = milestone + if state is not None: + request_body["state"] = state + if state_reason is not None: + request_body["state_reason"] = state_reason + if not request_body: + raise ValueError("At least one mutable issue field must be provided for update.") + payload = await self._request( + "PATCH", + f"/repos/{owner}/{repo}/issues/{issue_number}", + json_body=request_body, + ) + return _normalize_issue(payload) + + async def list_issue_comments( + self, + *, + owner: str, + repo: str, + issue_number: int, + per_page: int = 30, + page: int = 1, + ) -> dict[str, Any]: + payload = await self._request( + "GET", + f"/repos/{owner}/{repo}/issues/{issue_number}/comments", + params={"per_page": _clamp_per_page(per_page), "page": max(1, page)}, + ) + return { + "owner": owner, + "repo": repo, + "issue_number": issue_number, + "comments": [_normalize_issue_comment(comment) for comment in payload], + } + + async def create_issue_comment( + self, + *, + owner: str, + repo: str, + issue_number: int, + body: str, + ) -> dict[str, Any]: + payload = await self._request( + "POST", + f"/repos/{owner}/{repo}/issues/{issue_number}/comments", + json_body={"body": body}, + ) + return _normalize_issue_comment(payload) + + async def update_issue_comment( + self, + *, + owner: str, + repo: str, + comment_id: int, + body: str, + ) -> dict[str, Any]: + payload = await self._request( + "PATCH", + f"/repos/{owner}/{repo}/issues/comments/{comment_id}", + json_body={"body": body}, + ) + return _normalize_issue_comment(payload) + + async def list_pull_requests( + self, + *, + owner: str, + repo: str, + state: str = "open", + head: str | None = None, + base: str | None = None, + sort: str = "updated", + direction: str = "desc", + per_page: int = 20, + page: int = 1, + ) -> dict[str, Any]: + params: dict[str, Any] = { + "state": state, + "sort": sort, + "direction": direction, + "per_page": _clamp_per_page(per_page), + "page": max(1, page), + } + if head: + params["head"] = head + if base: + params["base"] = base + payload = await self._request("GET", f"/repos/{owner}/{repo}/pulls", params=params) + return { + "owner": owner, + "repo": repo, + "pull_requests": [_normalize_pull_request(pr) for pr in payload], + } + + async def get_pull_request(self, *, owner: str, repo: str, pull_number: int) -> dict[str, Any]: + payload = await self._request("GET", f"/repos/{owner}/{repo}/pulls/{pull_number}") + return _normalize_pull_request(payload) + + async def list_pull_reviews( + self, + *, + owner: str, + repo: str, + pull_number: int, + per_page: int = 30, + page: int = 1, + ) -> dict[str, Any]: + payload = await self._request( + "GET", + f"/repos/{owner}/{repo}/pulls/{pull_number}/reviews", + params={"per_page": _clamp_per_page(per_page), "page": max(1, page)}, + ) + return { + "owner": owner, + "repo": repo, + "pull_number": pull_number, + "reviews": [_normalize_pull_review(review) for review in payload], + } + + async def list_pull_review_comments( + self, + *, + owner: str, + repo: str, + pull_number: int, + per_page: int = 30, + page: int = 1, + ) -> dict[str, Any]: + payload = await self._request( + "GET", + f"/repos/{owner}/{repo}/pulls/{pull_number}/comments", + params={"per_page": _clamp_per_page(per_page), "page": max(1, page)}, + ) + return { + "owner": owner, + "repo": repo, + "pull_number": pull_number, + "comments": [_normalize_pull_review_comment(comment) for comment in payload], + } + + async def create_pull_review( + self, + *, + owner: str, + repo: str, + pull_number: int, + body: str | None = None, + event: str | None = None, + commit_id: str | None = None, + comments: list[dict[str, Any]] | None = None, + ) -> dict[str, Any]: + request_body: dict[str, Any] = {} + if body is not None: + request_body["body"] = body + if event is not None: + request_body["event"] = event + if commit_id is not None: + request_body["commit_id"] = commit_id + if comments is not None: + request_body["comments"] = comments + payload = await self._request( + "POST", + f"/repos/{owner}/{repo}/pulls/{pull_number}/reviews", + json_body=request_body, + ) + return _normalize_pull_review(payload) + + async def list_pull_files( + self, + *, + owner: str, + repo: str, + pull_number: int, + per_page: int = 30, + page: int = 1, + ) -> dict[str, Any]: + payload = await self._request( + "GET", + f"/repos/{owner}/{repo}/pulls/{pull_number}/files", + params={"per_page": _clamp_per_page(per_page), "page": max(1, page)}, + ) + return { + "owner": owner, + "repo": repo, + "pull_number": pull_number, + "files": [_normalize_pull_file(item) for item in payload], + } + + async def list_pull_commits( + self, + *, + owner: str, + repo: str, + pull_number: int, + per_page: int = 30, + page: int = 1, + ) -> dict[str, Any]: + payload = await self._request( + "GET", + f"/repos/{owner}/{repo}/pulls/{pull_number}/commits", + params={"per_page": _clamp_per_page(per_page), "page": max(1, page)}, + ) + return { + "owner": owner, + "repo": repo, + "pull_number": pull_number, + "commits": [_normalize_commit(item) for item in payload], + } + + async def list_commit_comments( + self, + *, + owner: str, + repo: str, + commit_sha: str, + per_page: int = 30, + page: int = 1, + ) -> dict[str, Any]: + payload = await self._request( + "GET", + f"/repos/{owner}/{repo}/commits/{commit_sha}/comments", + params={"per_page": _clamp_per_page(per_page), "page": max(1, page)}, + ) + return { + "owner": owner, + "repo": repo, + "commit_sha": commit_sha, + "comments": [_normalize_commit_comment(item) for item in payload], + } + + async def get_pull_merge_readiness( + self, + *, + owner: str, + repo: str, + pull_number: int, + ) -> dict[str, Any]: + pr = await self.get_pull_request(owner=owner, repo=repo, pull_number=pull_number) + reviews_payload = await self.list_pull_reviews(owner=owner, repo=repo, pull_number=pull_number) + reviews = reviews_payload.get("reviews") or [] + + latest_by_user: dict[str, dict[str, Any]] = {} + for review in reviews: + user = review.get("user") or {} + login = str(user.get("login") or "").strip().lower() + if login: + latest_by_user[login] = review + + state_counts: dict[str, int] = {} + for review in reviews: + state = str(review.get("state") or "UNKNOWN").upper() + state_counts[state] = state_counts.get(state, 0) + 1 + + blocking_reviews = [ + review for review in latest_by_user.values() if str(review.get("state") or "").upper() == "CHANGES_REQUESTED" + ] + approvals = [ + review for review in latest_by_user.values() if str(review.get("state") or "").upper() == "APPROVED" + ] + + return { + "owner": owner, + "repo": repo, + "pull_number": pull_number, + "pull_request": pr, + "review_summary": { + "total_reviews": len(reviews), + "latest_reviews_by_user": list(latest_by_user.values()), + "state_counts": state_counts, + "approvals": len(approvals), + "changes_requested": len(blocking_reviews), + "comment_only_reviews": state_counts.get("COMMENTED", 0), + }, + "merge_readiness": { + "draft": pr.get("draft", False), + "mergeable": pr.get("mergeable"), + "mergeable_state": pr.get("mergeable_state"), + "rebaseable": pr.get("rebaseable"), + "requested_reviewers": pr.get("requested_reviewers", []), + "requested_teams": pr.get("requested_teams", []), + "blocking_reviews": blocking_reviews, + }, + } + + +mcp = FastMCP( + name="Octopal GitHub", + instructions=( + "Use these tools to inspect the connected GitHub account, repositories, issues, and pull requests. " + "Prefer list tools to discover repositories or PR numbers before fetching a single item." + ), + log_level="ERROR", +) + +_github_client: GitHubApiClient | None = None + + +def _client() -> GitHubApiClient: + global _github_client + if _github_client is None: + _github_client = GitHubApiClient() + return _github_client + + +@mcp.tool(name="get_authenticated_user") +async def get_authenticated_user() -> dict[str, Any]: + """Return basic information about the connected GitHub user.""" + return await _client().get_authenticated_user() + + +@mcp.tool(name="list_repositories") +async def list_repositories( + visibility: str | None = None, + affiliation: str | None = None, + sort: str = "updated", + direction: str = "desc", + per_page: int = 20, + page: int = 1, +) -> dict[str, Any]: + """List repositories visible to the authenticated GitHub user.""" + return await _client().list_repositories( + visibility=visibility, + affiliation=affiliation, + sort=sort, + direction=direction, + per_page=per_page, + page=page, + ) + + +@mcp.tool(name="get_repository") +async def get_repository(owner: str, repo: str) -> dict[str, Any]: + """Return metadata for a specific GitHub repository.""" + return await _client().get_repository(owner=owner, repo=repo) + + +@mcp.tool(name="list_issues") +async def list_issues( + owner: str, + repo: str, + state: str = "open", + labels: str | None = None, + since: str | None = None, + per_page: int = 20, + page: int = 1, +) -> dict[str, Any]: + """List issues for a repository.""" + return await _client().list_issues( + owner=owner, + repo=repo, + state=state, + labels=labels, + since=since, + per_page=per_page, + page=page, + ) + + +@mcp.tool(name="get_issue") +async def get_issue(owner: str, repo: str, issue_number: int) -> dict[str, Any]: + """Return a single issue by number.""" + return await _client().get_issue(owner=owner, repo=repo, issue_number=issue_number) + + +@mcp.tool(name="create_issue") +async def create_issue( + owner: str, + repo: str, + title: str, + body: str | None = None, + assignees: list[str] | None = None, + labels: list[str] | None = None, + milestone: int | None = None, +) -> dict[str, Any]: + """Create a GitHub issue.""" + return await _client().create_issue( + owner=owner, + repo=repo, + title=title, + body=body, + assignees=assignees, + labels=labels, + milestone=milestone, + ) + + +@mcp.tool(name="update_issue") +async def update_issue( + owner: str, + repo: str, + issue_number: int, + title: str | None = None, + body: str | None = None, + assignees: list[str] | None = None, + labels: list[str] | None = None, + milestone: int | None = None, + state: str | None = None, + state_reason: str | None = None, +) -> dict[str, Any]: + """Update mutable fields on a GitHub issue.""" + return await _client().update_issue( + owner=owner, + repo=repo, + issue_number=issue_number, + title=title, + body=body, + assignees=assignees, + labels=labels, + milestone=milestone, + state=state, + state_reason=state_reason, + ) + + +@mcp.tool(name="list_issue_comments") +async def list_issue_comments( + owner: str, + repo: str, + issue_number: int, + per_page: int = 30, + page: int = 1, +) -> dict[str, Any]: + """List issue comments for an issue or pull request conversation.""" + return await _client().list_issue_comments( + owner=owner, + repo=repo, + issue_number=issue_number, + per_page=per_page, + page=page, + ) + + +@mcp.tool(name="create_issue_comment") +async def create_issue_comment(owner: str, repo: str, issue_number: int, body: str) -> dict[str, Any]: + """Create an issue comment. This also works for pull request conversation comments.""" + return await _client().create_issue_comment(owner=owner, repo=repo, issue_number=issue_number, body=body) + + +@mcp.tool(name="update_issue_comment") +async def update_issue_comment(owner: str, repo: str, comment_id: int, body: str) -> dict[str, Any]: + """Update an existing issue comment.""" + return await _client().update_issue_comment(owner=owner, repo=repo, comment_id=comment_id, body=body) + + +@mcp.tool(name="list_pull_requests") +async def list_pull_requests( + owner: str, + repo: str, + state: str = "open", + head: str | None = None, + base: str | None = None, + sort: str = "updated", + direction: str = "desc", + per_page: int = 20, + page: int = 1, +) -> dict[str, Any]: + """List pull requests for a repository.""" + return await _client().list_pull_requests( + owner=owner, + repo=repo, + state=state, + head=head, + base=base, + sort=sort, + direction=direction, + per_page=per_page, + page=page, + ) + + +@mcp.tool(name="get_pull_request") +async def get_pull_request(owner: str, repo: str, pull_number: int) -> dict[str, Any]: + """Return a single pull request by number.""" + return await _client().get_pull_request(owner=owner, repo=repo, pull_number=pull_number) + + +@mcp.tool(name="list_pull_reviews") +async def list_pull_reviews( + owner: str, + repo: str, + pull_number: int, + per_page: int = 30, + page: int = 1, +) -> dict[str, Any]: + """List review submissions for a pull request.""" + return await _client().list_pull_reviews( + owner=owner, + repo=repo, + pull_number=pull_number, + per_page=per_page, + page=page, + ) + + +@mcp.tool(name="list_pull_review_comments") +async def list_pull_review_comments( + owner: str, + repo: str, + pull_number: int, + per_page: int = 30, + page: int = 1, +) -> dict[str, Any]: + """List inline review comments for a pull request.""" + return await _client().list_pull_review_comments( + owner=owner, + repo=repo, + pull_number=pull_number, + per_page=per_page, + page=page, + ) + + +@mcp.tool(name="create_pull_review") +async def create_pull_review( + owner: str, + repo: str, + pull_number: int, + body: str | None = None, + event: str | None = None, + commit_id: str | None = None, + comments: list[dict[str, Any]] | None = None, +) -> dict[str, Any]: + """Create a pull request review, optionally with APPROVE, REQUEST_CHANGES, or COMMENT event.""" + return await _client().create_pull_review( + owner=owner, + repo=repo, + pull_number=pull_number, + body=body, + event=event, + commit_id=commit_id, + comments=comments, + ) + + +@mcp.tool(name="list_pull_files") +async def list_pull_files( + owner: str, + repo: str, + pull_number: int, + per_page: int = 30, + page: int = 1, +) -> dict[str, Any]: + """List changed files in a pull request, including patch hunks when GitHub provides them.""" + return await _client().list_pull_files( + owner=owner, + repo=repo, + pull_number=pull_number, + per_page=per_page, + page=page, + ) + + +@mcp.tool(name="list_pull_commits") +async def list_pull_commits( + owner: str, + repo: str, + pull_number: int, + per_page: int = 30, + page: int = 1, +) -> dict[str, Any]: + """List commits included in a pull request.""" + return await _client().list_pull_commits( + owner=owner, + repo=repo, + pull_number=pull_number, + per_page=per_page, + page=page, + ) + + +@mcp.tool(name="list_commit_comments") +async def list_commit_comments( + owner: str, + repo: str, + commit_sha: str, + per_page: int = 30, + page: int = 1, +) -> dict[str, Any]: + """List commit comments for a specific commit SHA.""" + return await _client().list_commit_comments( + owner=owner, + repo=repo, + commit_sha=commit_sha, + per_page=per_page, + page=page, + ) + + +@mcp.tool(name="get_pull_merge_readiness") +async def get_pull_merge_readiness(owner: str, repo: str, pull_number: int) -> dict[str, Any]: + """Summarize pull request review state and merge readiness without merging.""" + return await _client().get_pull_merge_readiness(owner=owner, repo=repo, pull_number=pull_number) + + +def main() -> None: + try: + mcp.run() + finally: + try: + if _github_client is not None: + asyncio.run(_github_client.close()) + except Exception: + pass + + +if __name__ == "__main__": + main() diff --git a/src/octopal/runtime/octo/router.py b/src/octopal/runtime/octo/router.py index 34c7287..bfa4453 100644 --- a/src/octopal/runtime/octo/router.py +++ b/src/octopal/runtime/octo/router.py @@ -45,6 +45,15 @@ _MIN_TOOL_COUNT_ON_OVERFLOW = 12 _CATALOG_TOOL_EXPANSION_LIMIT = 12 _MANDATORY_OCTO_TOOL_NAMES = { + "octo_context_health", + "check_schedule", + "tool_catalog_search", + "list_workers", + "start_worker", + "get_worker_status", + "list_active_workers", + "get_worker_result", + "stop_worker", "fs_list", "fs_read", "fs_write", diff --git a/src/octopal/runtime/workers/runtime.py b/src/octopal/runtime/workers/runtime.py index 6d0e822..5d2c27b 100644 --- a/src/octopal/runtime/workers/runtime.py +++ b/src/octopal/runtime/workers/runtime.py @@ -123,32 +123,36 @@ async def run_task( mcp_tools_data = [] known_server_ids = list(self.mcp_manager.sessions.keys()) if self.mcp_manager else [] - # 1. Add explicitly requested MCP tools + # 1. Add explicitly requested MCP-backed tools, including connector aliases. for tool_name in requested_tool_names: - if tool_name.startswith("mcp_"): - # Find the tool spec - spec_found = next((t for t in all_tools if t.name == tool_name), None) - if spec_found: - server_id = getattr(spec_found, "server_id", None) - remote_tool_name = getattr(spec_found, "remote_tool_name", None) - if not server_id or not remote_tool_name: - server_id, remote_tool_name = _extract_mcp_tool_identity( - spec_found.name, known_server_ids - ) - mcp_tools_data.append( - { - "name": spec_found.name, - "description": spec_found.description, - "parameters": spec_found.parameters, - "permission": spec_found.permission, - "is_async": spec_found.is_async, - "server_id": server_id, - "remote_tool_name": remote_tool_name, - } - ) + # Find the tool spec + spec_found = next((t for t in all_tools if t.name == tool_name), None) + if spec_found is None: + continue + + server_id = getattr(spec_found, "server_id", None) + remote_tool_name = getattr(spec_found, "remote_tool_name", None) + if (not server_id or not remote_tool_name) and str(tool_name).startswith("mcp_"): + server_id, remote_tool_name = _extract_mcp_tool_identity( + spec_found.name, known_server_ids + ) + if not server_id or not remote_tool_name: + continue + + mcp_tools_data.append( + { + "name": spec_found.name, + "description": spec_found.description, + "parameters": spec_found.parameters, + "permission": spec_found.permission, + "is_async": spec_found.is_async, + "server_id": server_id, + "remote_tool_name": remote_tool_name, + } + ) # Global MCP tools are intentionally NOT auto-injected. - # Workers only receive MCP tools explicitly listed in task_request/tools or template available_tools. + # Workers only receive MCP-backed tools explicitly listed in task_request/tools or template available_tools. # Resolve worker LLM configuration llm_config = self._resolve_worker_llm_config(template, task_request) diff --git a/src/octopal/tools/catalog.py b/src/octopal/tools/catalog.py index c12a1ef..abfef3b 100644 --- a/src/octopal/tools/catalog.py +++ b/src/octopal/tools/catalog.py @@ -35,6 +35,7 @@ from octopal.tools.connectors.calendar import get_calendar_connector_tools from octopal.tools.connectors.drive import get_drive_connector_tools from octopal.tools.connectors.gmail import get_gmail_connector_tools +from octopal.tools.connectors.github import get_github_connector_tools from octopal.tools.connectors.status import get_connector_status_tools from octopal.tools.filesystem.download import download_file from octopal.tools.filesystem.files import fs_delete, fs_list, fs_move, fs_read, fs_write @@ -53,6 +54,114 @@ logger = structlog.get_logger(__name__) +def _resolve_mcp_manager(ctx: dict[str, Any], fallback: Any) -> Any: + octo = (ctx or {}).get("octo") + if octo is not None and getattr(octo, "mcp_manager", None) is not None: + return octo.mcp_manager + return fallback + + +async def _tool_github_review_bundle(args, ctx) -> str: + manager = _resolve_mcp_manager(ctx, ctx.get("mcp_manager")) + if manager is None: + return json.dumps( + { + "status": "error", + "message": "GitHub review bundle is unavailable because no MCP manager is active.", + }, + ensure_ascii=False, + ) + + owner = str((args or {}).get("owner", "") or "").strip() + repo = str((args or {}).get("repo", "") or "").strip() + pull_number = int((args or {}).get("pull_number", 0) or 0) + file_limit = max(1, min(int((args or {}).get("file_limit", 100) or 100), 100)) + commit_limit = max(1, min(int((args or {}).get("commit_limit", 100) or 100), 100)) + review_limit = max(1, min(int((args or {}).get("review_limit", 100) or 100), 100)) + comment_limit = max(1, min(int((args or {}).get("comment_limit", 100) or 100), 100)) + include_commit_comments = bool((args or {}).get("include_commit_comments", True)) + + if not owner or not repo or pull_number <= 0: + return json.dumps( + { + "status": "error", + "message": "owner, repo, and pull_number are required.", + }, + ensure_ascii=False, + ) + + async def _call(tool_name: str, tool_args: dict[str, Any]) -> Any: + result = await manager.call_tool( + "github-core", + tool_name, + tool_args, + allow_name_fallback=True, + ) + content_items = getattr(result, "content", None) + if not content_items: + return result + if len(content_items) == 1: + text = getattr(content_items[0], "text", None) + if isinstance(text, str): + return json.loads(text) + normalized: list[Any] = [] + for item in content_items: + text = getattr(item, "text", None) + if isinstance(text, str): + normalized.append(json.loads(text)) + else: + normalized.append(item.model_dump() if hasattr(item, "model_dump") else str(item)) + return normalized + + base_args = {"owner": owner, "repo": repo, "pull_number": pull_number} + pr_payload, readiness_payload, reviews_payload, review_comments_payload, files_payload, commits_payload, convo_payload = await asyncio.gather( + _call("get_pull_request", base_args), + _call("get_pull_merge_readiness", base_args), + _call("list_pull_reviews", {**base_args, "per_page": review_limit}), + _call("list_pull_review_comments", {**base_args, "per_page": comment_limit}), + _call("list_pull_files", {**base_args, "per_page": file_limit}), + _call("list_pull_commits", {**base_args, "per_page": commit_limit}), + _call("list_issue_comments", {"owner": owner, "repo": repo, "issue_number": pull_number, "per_page": comment_limit}), + ) + + commit_comments: dict[str, Any] = {} + if include_commit_comments: + commits = (commits_payload or {}).get("commits") or [] + for commit in commits[:commit_limit]: + sha = str((commit or {}).get("sha", "") or "").strip() + if not sha: + continue + try: + commit_comments[sha] = await _call( + "list_commit_comments", + {"owner": owner, "repo": repo, "commit_sha": sha, "per_page": comment_limit}, + ) + except Exception as exc: + commit_comments[sha] = {"status": "error", "message": str(exc)} + + payload = { + "status": "ok", + "owner": owner, + "repo": repo, + "pull_number": pull_number, + "pull_request": pr_payload, + "merge_readiness": (readiness_payload or {}).get("merge_readiness"), + "review_summary": (readiness_payload or {}).get("review_summary"), + "conversation_comments": (convo_payload or {}).get("comments", []), + "reviews": (reviews_payload or {}).get("reviews", []), + "review_comments": (review_comments_payload or {}).get("comments", []), + "files": (files_payload or {}).get("files", []), + "commits": (commits_payload or {}).get("commits", []), + "commit_comments": commit_comments, + "hints": [ + "Use files.patch and review_comments together when drafting code review feedback.", + "Use merge_readiness.blocking_reviews and requested_reviewers to understand review state before commenting.", + "Conversation comments are issue comments on the PR thread; inline code feedback lives in review_comments.", + ], + } + return json.dumps(payload, ensure_ascii=False) + + def _tool_catalog_search(args, ctx) -> str: query = str((args or {}).get("query", "") or "").strip().lower() category_filter = str((args or {}).get("category", "") or "").strip().lower() @@ -320,6 +429,31 @@ def get_tools(mcp_manager=None) -> list[ToolSpec]: permission="self_control", handler=_tool_catalog_search, ), + ToolSpec( + name="github_review_bundle", + description=( + "Collect a pull request review bundle from the GitHub connector: PR metadata, merge readiness, " + "reviews, review comments, changed files, commits, and commit comments." + ), + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "pull_number": {"type": "integer", "minimum": 1}, + "file_limit": {"type": "integer", "minimum": 1, "maximum": 100}, + "commit_limit": {"type": "integer", "minimum": 1, "maximum": 100}, + "review_limit": {"type": "integer", "minimum": 1, "maximum": 100}, + "comment_limit": {"type": "integer", "minimum": 1, "maximum": 100}, + "include_commit_comments": {"type": "boolean"}, + }, + "required": ["owner", "repo", "pull_number"], + "additionalProperties": False, + }, + permission="mcp_exec", + handler=_tool_github_review_bundle, + is_async=True, + ), ToolSpec( name="octo_opportunity_scan", description="Generate proactive opportunity cards (impact/effort/confidence/next_action) for the active chat.", @@ -1333,6 +1467,7 @@ def get_tools(mcp_manager=None) -> list[ToolSpec]: tools.extend(get_calendar_connector_tools(mcp_manager)) tools.extend(get_drive_connector_tools(mcp_manager)) tools.extend(get_gmail_connector_tools(mcp_manager)) + tools.extend(get_github_connector_tools(mcp_manager)) tools.extend(_get_mcp_management_tools()) if mcp_manager: mcp_tools = mcp_manager.get_all_tools() diff --git a/src/octopal/tools/connectors/github.py b/src/octopal/tools/connectors/github.py new file mode 100644 index 0000000..c5fe7f3 --- /dev/null +++ b/src/octopal/tools/connectors/github.py @@ -0,0 +1,477 @@ +from __future__ import annotations + +import json +from typing import Any + +from octopal.tools.metadata import ToolMetadata +from octopal.tools.registry import ToolSpec + +_GITHUB_SERVER_ID = "github-core" + + +def _extract_mcp_payload(result: Any) -> Any: + content_items = getattr(result, "content", None) + if not content_items: + return result + + if len(content_items) == 1: + item = content_items[0] + text = getattr(item, "text", None) + if isinstance(text, str): + try: + return json.loads(text) + except json.JSONDecodeError: + return text + if hasattr(item, "model_dump"): + return item.model_dump() + return str(item) + + normalized: list[Any] = [] + for item in content_items: + text = getattr(item, "text", None) + if isinstance(text, str): + try: + normalized.append(json.loads(text)) + except json.JSONDecodeError: + normalized.append(text) + continue + if hasattr(item, "model_dump"): + normalized.append(item.model_dump()) + continue + normalized.append(str(item)) + return normalized + + +def _resolve_mcp_manager(ctx: dict[str, Any], fallback: Any) -> Any: + octo = (ctx or {}).get("octo") + if octo is not None and getattr(octo, "mcp_manager", None) is not None: + return octo.mcp_manager + return fallback + + +async def _github_mcp_proxy( + remote_tool_name: str, + args: dict[str, Any], + ctx: dict[str, Any], + *, + fallback_manager: Any, +) -> Any: + manager = _resolve_mcp_manager(ctx, fallback_manager) + if manager is None: + return { + "ok": False, + "error": "GitHub tools are unavailable because no MCP manager is active.", + "hint": "Restart Octopal after authorizing the GitHub connector.", + } + + try: + result = await manager.call_tool( + _GITHUB_SERVER_ID, + remote_tool_name, + args or {}, + allow_name_fallback=True, + ) + except Exception as exc: + return { + "ok": False, + "error": str(exc), + "server_id": _GITHUB_SERVER_ID, + "tool": remote_tool_name, + "hint": "Check connector status and confirm the GitHub MCP server is connected.", + } + + return _extract_mcp_payload(result) + + +def _github_tool( + *, + name: str, + remote_tool_name: str, + description: str, + parameters: dict[str, Any], + fallback_manager: Any, + capabilities: tuple[str, ...], +) -> ToolSpec: + return ToolSpec( + name=name, + description=description, + parameters=parameters, + permission="mcp_exec", + handler=lambda args, ctx, _remote=remote_tool_name, _manager=fallback_manager: _github_mcp_proxy( + _remote, + args, + ctx, + fallback_manager=_manager, + ), + is_async=True, + server_id=_GITHUB_SERVER_ID, + remote_tool_name=remote_tool_name, + metadata=ToolMetadata( + category="connectors", + risk="safe", + profile_tags=("research", "engineering"), + capabilities=capabilities, + ), + ) + + +def get_github_connector_tools(mcp_manager: Any = None) -> list[ToolSpec]: + if mcp_manager is None: + return [] + + return [ + _github_tool( + name="github_get_authenticated_user", + remote_tool_name="get_authenticated_user", + description="Get the connected GitHub user profile to confirm which account is active.", + parameters={"type": "object", "properties": {}, "additionalProperties": False}, + fallback_manager=mcp_manager, + capabilities=("github_read", "connector_use"), + ), + _github_tool( + name="github_list_repositories", + remote_tool_name="list_repositories", + description="List repositories visible to the connected GitHub account.", + parameters={ + "type": "object", + "properties": { + "visibility": {"type": "string"}, + "affiliation": {"type": "string"}, + "sort": {"type": "string"}, + "direction": {"type": "string"}, + "per_page": {"type": "integer", "minimum": 1, "maximum": 50}, + "page": {"type": "integer", "minimum": 1}, + }, + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_read", "connector_use"), + ), + _github_tool( + name="github_get_repository", + remote_tool_name="get_repository", + description="Read metadata for a specific GitHub repository by owner and repo name.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + }, + "required": ["owner", "repo"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_read", "connector_use"), + ), + _github_tool( + name="github_list_issues", + remote_tool_name="list_issues", + description="List issues for a repository, optionally filtered by state, labels, or time.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "state": {"type": "string"}, + "labels": {"type": "string"}, + "since": {"type": "string"}, + "per_page": {"type": "integer", "minimum": 1, "maximum": 50}, + "page": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_issue_read", "connector_use"), + ), + _github_tool( + name="github_get_issue", + remote_tool_name="get_issue", + description="Read a GitHub issue by owner, repo, and issue number.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "issue_number": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo", "issue_number"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_issue_read", "connector_use"), + ), + _github_tool( + name="github_create_issue", + remote_tool_name="create_issue", + description="Create a GitHub issue in a repository.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "title": {"type": "string"}, + "body": {"type": "string"}, + "assignees": {"type": "array", "items": {"type": "string"}}, + "labels": {"type": "array", "items": {"type": "string"}}, + "milestone": {"type": "integer", "minimum": 0}, + }, + "required": ["owner", "repo", "title"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_issue_write", "connector_use"), + ), + _github_tool( + name="github_update_issue", + remote_tool_name="update_issue", + description="Update mutable fields on a GitHub issue, including body, labels, assignees, or state.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "issue_number": {"type": "integer", "minimum": 1}, + "title": {"type": "string"}, + "body": {"type": "string"}, + "assignees": {"type": "array", "items": {"type": "string"}}, + "labels": {"type": "array", "items": {"type": "string"}}, + "milestone": {"type": "integer", "minimum": 0}, + "state": {"type": "string"}, + "state_reason": {"type": "string"}, + }, + "required": ["owner", "repo", "issue_number"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_issue_write", "connector_use"), + ), + _github_tool( + name="github_list_issue_comments", + remote_tool_name="list_issue_comments", + description="List conversation comments for a GitHub issue or pull request.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "issue_number": {"type": "integer", "minimum": 1}, + "per_page": {"type": "integer", "minimum": 1, "maximum": 50}, + "page": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo", "issue_number"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_issue_read", "connector_use"), + ), + _github_tool( + name="github_create_issue_comment", + remote_tool_name="create_issue_comment", + description="Create a conversation comment on a GitHub issue or pull request.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "issue_number": {"type": "integer", "minimum": 1}, + "body": {"type": "string"}, + }, + "required": ["owner", "repo", "issue_number", "body"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_issue_write", "connector_use"), + ), + _github_tool( + name="github_update_issue_comment", + remote_tool_name="update_issue_comment", + description="Update an existing GitHub issue or pull request conversation comment.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "comment_id": {"type": "integer", "minimum": 1}, + "body": {"type": "string"}, + }, + "required": ["owner", "repo", "comment_id", "body"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_issue_write", "connector_use"), + ), + _github_tool( + name="github_list_pull_requests", + remote_tool_name="list_pull_requests", + description="List pull requests for a repository, optionally filtered by state, base, or head branch.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "state": {"type": "string"}, + "head": {"type": "string"}, + "base": {"type": "string"}, + "sort": {"type": "string"}, + "direction": {"type": "string"}, + "per_page": {"type": "integer", "minimum": 1, "maximum": 50}, + "page": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_pr_read", "connector_use"), + ), + _github_tool( + name="github_get_pull_request", + remote_tool_name="get_pull_request", + description="Read a GitHub pull request by owner, repo, and pull request number.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "pull_number": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo", "pull_number"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_pr_read", "connector_use"), + ), + _github_tool( + name="github_list_pull_reviews", + remote_tool_name="list_pull_reviews", + description="List submitted reviews for a GitHub pull request.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "pull_number": {"type": "integer", "minimum": 1}, + "per_page": {"type": "integer", "minimum": 1, "maximum": 50}, + "page": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo", "pull_number"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_pr_read", "connector_use"), + ), + _github_tool( + name="github_list_pull_review_comments", + remote_tool_name="list_pull_review_comments", + description="List inline review comments for a GitHub pull request.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "pull_number": {"type": "integer", "minimum": 1}, + "per_page": {"type": "integer", "minimum": 1, "maximum": 50}, + "page": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo", "pull_number"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_pr_read", "connector_use"), + ), + _github_tool( + name="github_create_pull_review", + remote_tool_name="create_pull_review", + description="Create a GitHub pull request review with COMMENT, APPROVE, or REQUEST_CHANGES.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "pull_number": {"type": "integer", "minimum": 1}, + "body": {"type": "string"}, + "event": {"type": "string"}, + "commit_id": {"type": "string"}, + "comments": {"type": "array", "items": {"type": "object"}}, + }, + "required": ["owner", "repo", "pull_number"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_pr_write", "connector_use"), + ), + _github_tool( + name="github_list_pull_files", + remote_tool_name="list_pull_files", + description="List changed files in a GitHub pull request, including patch hunks when available.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "pull_number": {"type": "integer", "minimum": 1}, + "per_page": {"type": "integer", "minimum": 1, "maximum": 50}, + "page": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo", "pull_number"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_pr_read", "connector_use"), + ), + _github_tool( + name="github_list_pull_commits", + remote_tool_name="list_pull_commits", + description="List commits included in a GitHub pull request.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "pull_number": {"type": "integer", "minimum": 1}, + "per_page": {"type": "integer", "minimum": 1, "maximum": 50}, + "page": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo", "pull_number"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_pr_read", "connector_use"), + ), + _github_tool( + name="github_list_commit_comments", + remote_tool_name="list_commit_comments", + description="List commit comments for a specific commit SHA in a repository.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "commit_sha": {"type": "string"}, + "per_page": {"type": "integer", "minimum": 1, "maximum": 50}, + "page": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo", "commit_sha"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_pr_read", "connector_use"), + ), + _github_tool( + name="github_get_pull_merge_readiness", + remote_tool_name="get_pull_merge_readiness", + description="Summarize pull request review state and merge readiness without merging.", + parameters={ + "type": "object", + "properties": { + "owner": {"type": "string"}, + "repo": {"type": "string"}, + "pull_number": {"type": "integer", "minimum": 1}, + }, + "required": ["owner", "repo", "pull_number"], + "additionalProperties": False, + }, + fallback_manager=mcp_manager, + capabilities=("github_pr_read", "connector_use"), + ), + ] diff --git a/tests/test_cli_connectors.py b/tests/test_cli_connectors.py index ff6f2a0..c1f54e1 100644 --- a/tests/test_cli_connectors.py +++ b/tests/test_cli_connectors.py @@ -75,6 +75,15 @@ def test_connector_next_action_maps_needs_auth_to_auth_command() -> None: assert action == "run `octopal connector auth google`" +def test_connector_next_action_maps_github_needs_auth_to_auth_command() -> None: + action = _connector_next_action( + "github", + {"status": "needs_auth"}, + ) + + assert action == "run `octopal connector auth github`" + + def test_connector_auth_requires_enabled_connector(tmp_path, monkeypatch) -> None: (tmp_path / "config.json").write_text(json.dumps({}), encoding="utf-8") monkeypatch.chdir(tmp_path) @@ -306,3 +315,38 @@ def test_connector_disconnect_clears_auth_state(tmp_path, monkeypatch) -> None: assert instance["auth"]["access_token"] is None assert instance["auth"]["authorized_services"] == [] assert instance["credentials"]["client_id"] == "client-id" + + +def test_connector_auth_success_uses_github_cli_flow(tmp_path, monkeypatch) -> None: + (tmp_path / "config.json").write_text( + json.dumps( + { + "connectors": { + "instances": { + "github": { + "enabled": True, + "enabled_services": ["repos"], + } + } + } + } + ), + encoding="utf-8", + ) + monkeypatch.chdir(tmp_path) + monkeypatch.setattr("octopal.cli.main.typer.prompt", lambda *args, **kwargs: "ghp_test") + + async def fake_authorize(self): + config = self._get_config() + assert config.auth.access_token == "ghp_test" + return {"status": "success", "message": "GitHub connector authorized for repos."} + + monkeypatch.setattr( + "octopal.infrastructure.connectors.github.GitHubConnector.authorize", + fake_authorize, + ) + + result = runner.invoke(app, ["connector", "auth", "github"]) + + assert result.exit_code == 0 + assert "authorized for repos" in result.stdout diff --git a/tests/test_connector_tools.py b/tests/test_connector_tools.py index 139ac93..3c9a8c1 100644 --- a/tests/test_connector_tools.py +++ b/tests/test_connector_tools.py @@ -10,6 +10,7 @@ from octopal.tools.connectors.calendar import get_calendar_connector_tools from octopal.tools.connectors.drive import get_drive_connector_tools from octopal.tools.connectors.gmail import get_gmail_connector_tools +from octopal.tools.connectors.github import get_github_connector_tools from octopal.tools.connectors.status import connector_status_read from octopal.tools.registry import ToolSpec @@ -20,6 +21,7 @@ def test_catalog_includes_read_only_connector_status_tool() -> None: assert "connector_status" in names assert "send_file_to_user" in names + assert "github_review_bundle" in names def test_connector_status_tool_reads_status_from_octo_context() -> None: @@ -68,7 +70,7 @@ def get_all_tools(self): assert "gmail_get_message" in names -def test_octo_budget_keeps_connector_alias_tools() -> None: +def test_octo_budget_keeps_system_baseline_tools() -> None: class _Manager: def get_all_tools(self): return [ @@ -87,9 +89,15 @@ def get_all_tools(self): active = _budget_tool_specs(tools, max_count=64) names = {tool.name for tool in active} - assert "gmail_list_messages" in names - assert "gmail_search_messages" in names - assert "gmail_get_unread_count" in names + assert "octo_context_health" in names + assert "check_schedule" in names + assert "tool_catalog_search" in names + assert "list_workers" in names + assert "start_worker" in names + assert "get_worker_status" in names + assert "list_active_workers" in names + assert "get_worker_result" in names + assert "stop_worker" in names def test_catalog_includes_first_class_calendar_tools_when_mcp_manager_is_present() -> None: @@ -131,6 +139,28 @@ def get_all_tools(self): assert "drive_update_from_workspace" in names +def test_catalog_includes_first_class_github_tools_when_mcp_manager_is_present() -> None: + class _Manager: + def get_all_tools(self): + return [] + + tools = get_tools(mcp_manager=_Manager()) + names = {tool.name for tool in tools} + + assert "github_list_repositories" in names + assert "github_get_repository" in names + assert "github_list_issues" in names + assert "github_list_pull_requests" in names + assert "github_create_issue" in names + assert "github_create_issue_comment" in names + assert "github_list_pull_reviews" in names + assert "github_create_pull_review" in names + assert "github_list_pull_files" in names + assert "github_list_pull_commits" in names + assert "github_list_commit_comments" in names + assert "github_get_pull_merge_readiness" in names + + def test_gmail_connector_tool_proxies_and_parses_json_payload() -> None: class _Text: def __init__(self, text: str) -> None: @@ -486,3 +516,151 @@ async def call_tool(self, server_id, tool_name, args, allow_name_fallback=False) assert payload["ok"] is True assert payload["content"] == "# Hello" + + +def test_github_connector_tool_proxies_and_parses_json_payload() -> None: + class _Text: + def __init__(self, text: str) -> None: + self.text = text + + class _Result: + def __init__(self, text: str) -> None: + self.content = [_Text(text)] + + class _Manager: + async def call_tool(self, server_id, tool_name, args, allow_name_fallback=False): + assert server_id == "github-core" + assert tool_name == "list_repositories" + assert args == {"per_page": 1} + assert allow_name_fallback is True + return _Result('{"repositories":[{"id":1,"full_name":"octo/demo"}]}') + + tools = {tool.name: tool for tool in get_github_connector_tools(_Manager())} + payload = asyncio.run(tools["github_list_repositories"].handler({"per_page": 1}, {})) + + assert payload["repositories"][0]["full_name"] == "octo/demo" + + +def test_github_create_pull_review_tool_proxies_and_parses_json_payload() -> None: + class _Text: + def __init__(self, text: str) -> None: + self.text = text + + class _Result: + def __init__(self, text: str) -> None: + self.content = [_Text(text)] + + class _Manager: + async def call_tool(self, server_id, tool_name, args, allow_name_fallback=False): + assert server_id == "github-core" + assert tool_name == "create_pull_review" + assert args == { + "owner": "octo", + "repo": "demo", + "pull_number": 7, + "body": "Looks good overall", + "event": "COMMENT", + } + assert allow_name_fallback is True + return _Result('{"id":55,"state":"COMMENTED","body":"Looks good overall"}') + + tools = {tool.name: tool for tool in get_github_connector_tools(_Manager())} + payload = asyncio.run( + tools["github_create_pull_review"].handler( + { + "owner": "octo", + "repo": "demo", + "pull_number": 7, + "body": "Looks good overall", + "event": "COMMENT", + }, + {}, + ) + ) + + assert payload["id"] == 55 + assert payload["state"] == "COMMENTED" + + +def test_github_list_pull_files_tool_proxies_and_parses_json_payload() -> None: + class _Text: + def __init__(self, text: str) -> None: + self.text = text + + class _Result: + def __init__(self, text: str) -> None: + self.content = [_Text(text)] + + class _Manager: + async def call_tool(self, server_id, tool_name, args, allow_name_fallback=False): + assert server_id == "github-core" + assert tool_name == "list_pull_files" + assert args == { + "owner": "octo", + "repo": "demo", + "pull_number": 9, + } + assert allow_name_fallback is True + return _Result('{"files":[{"filename":"src/app.py","patch":"@@ -1 +1 @@"}]}') + + tools = {tool.name: tool for tool in get_github_connector_tools(_Manager())} + payload = asyncio.run( + tools["github_list_pull_files"].handler( + { + "owner": "octo", + "repo": "demo", + "pull_number": 9, + }, + {}, + ) + ) + + assert payload["files"][0]["filename"] == "src/app.py" + assert payload["files"][0]["patch"] == "@@ -1 +1 @@" + + +def test_github_review_bundle_collects_pr_context() -> None: + class _Text: + def __init__(self, text: str) -> None: + self.text = text + + class _Result: + def __init__(self, text: str) -> None: + self.content = [_Text(text)] + + class _Manager: + def get_all_tools(self): + return [] + + async def call_tool(self, server_id, tool_name, args, allow_name_fallback=False): + assert server_id == "github-core" + assert allow_name_fallback is True + responses = { + "get_pull_request": '{"number":7,"title":"Improve GitHub connector"}', + "get_pull_merge_readiness": '{"merge_readiness":{"mergeable":true},"review_summary":{"approvals":1}}', + "list_pull_reviews": '{"reviews":[{"id":1,"state":"APPROVED"}]}', + "list_pull_review_comments": '{"comments":[{"id":2,"path":"src/x.py"}]}', + "list_pull_files": '{"files":[{"filename":"src/x.py","patch":"@@ -1 +1 @@"}]}', + "list_pull_commits": '{"commits":[{"sha":"abc123","message":"refactor"}]}', + "list_issue_comments": '{"comments":[{"id":3,"body":"general feedback"}]}', + "list_commit_comments": '{"comments":[{"id":4,"body":"nit"}]}', + } + return _Result(responses[tool_name]) + + tools = {tool.name: tool for tool in get_tools(mcp_manager=_Manager())} + payload = json.loads( + asyncio.run( + tools["github_review_bundle"].handler( + {"owner": "octo", "repo": "demo", "pull_number": 7}, + {"mcp_manager": _Manager()}, + ) + ) + ) + + assert payload["status"] == "ok" + assert payload["pull_request"]["title"] == "Improve GitHub connector" + assert payload["merge_readiness"]["mergeable"] is True + assert payload["review_summary"]["approvals"] == 1 + assert payload["files"][0]["filename"] == "src/x.py" + assert payload["commits"][0]["sha"] == "abc123" + assert payload["commit_comments"]["abc123"]["comments"][0]["body"] == "nit" diff --git a/tests/test_connectors.py b/tests/test_connectors.py index 2696527..0b7e0a7 100644 --- a/tests/test_connectors.py +++ b/tests/test_connectors.py @@ -6,6 +6,7 @@ import pytest +from octopal.infrastructure.connectors import github as github_connector_module from octopal.cli.configure import _collect_connector_next_steps from octopal.infrastructure.config.models import ConnectorInstanceConfig, OctopalConfig from octopal.infrastructure.connectors.google import _oauthlib_insecure_transport_for_localhost @@ -25,6 +26,15 @@ def test_google_connector_configure_requires_cli_enabled_connector() -> None: asyncio.run(connector.configure({"client_id": "id", "client_secret": "secret"})) +def test_github_connector_configure_requires_cli_enabled_connector() -> None: + config = OctopalConfig() + manager = _build_manager(config) + connector = manager.get_connector("github") + + with pytest.raises(RuntimeError, match="not enabled"): + asyncio.run(connector.configure({"token": "ghp_test"})) + + def test_google_connector_status_rejects_unknown_services() -> None: config = OctopalConfig() config.connectors.instances["google"] = ConnectorInstanceConfig( @@ -106,6 +116,53 @@ def test_collect_connector_next_steps_prompts_for_auth_when_google_added() -> No assert any("octopal restart" in line for line in lines) +def test_collect_connector_next_steps_prompts_for_auth_when_github_added() -> None: + previous = OctopalConfig() + current = OctopalConfig() + current.connectors.instances["github"] = ConnectorInstanceConfig( + enabled=True, + enabled_services=["repos"], + ) + + lines = _collect_connector_next_steps(current, previous) + + assert any("octopal connector auth github" in line for line in lines) + assert any("octopal connector status" in line for line in lines) + assert any("octopal restart" in line for line in lines) + + +def test_github_connector_status_accepts_repo_service() -> None: + config = OctopalConfig() + config.connectors.instances["github"] = ConnectorInstanceConfig( + enabled=True, + enabled_services=["repos"], + auth={"authorized_services": ["repos"], "access_token": "ghp_test"}, + ) + manager = _build_manager(config) + connector = manager.get_connector("github") + + status = asyncio.run(connector.get_status()) + + assert status["status"] == "ready" + assert status["services"] == ["repos"] + + +def test_github_connector_status_requires_reauth_for_newly_enabled_services() -> None: + config = OctopalConfig() + config.connectors.instances["github"] = ConnectorInstanceConfig( + enabled=True, + enabled_services=["repos", "issues"], + auth={"authorized_services": ["repos"], "access_token": "ghp_test"}, + ) + manager = _build_manager(config) + connector = manager.get_connector("github") + + status = asyncio.run(connector.get_status()) + + assert status["status"] == "needs_reauth" + assert "issues" in status["message"] + + def test_google_connector_disconnect_clears_auth_state_but_keeps_client_credentials() -> None: config = OctopalConfig() config.connectors.instances["google"] = ConnectorInstanceConfig( @@ -132,6 +189,168 @@ def test_google_connector_disconnect_clears_auth_state_but_keeps_client_credenti assert instance.credentials.client_secret == "client-secret" +def test_github_connector_disconnect_keeps_access_token_by_default() -> None: + config = OctopalConfig() + config.connectors.instances["github"] = ConnectorInstanceConfig( + enabled=True, + enabled_services=["repos"], + auth={ + "authorized_services": ["repos"], + "access_token": "ghp_test", + }, + ) + manager = _build_manager(config) + connector = manager.get_connector("github") + + result = asyncio.run(connector.disconnect()) + + assert result["status"] == "success" + instance = config.connectors.instances["github"] + assert instance.auth.access_token == "ghp_test" + assert instance.auth.authorized_services == [] + + +def test_github_connector_disconnect_forgets_access_token_when_requested() -> None: + config = OctopalConfig() + config.connectors.instances["github"] = ConnectorInstanceConfig( + enabled=True, + enabled_services=["repos"], + auth={ + "authorized_services": ["repos"], + "access_token": "ghp_test", + }, + ) + manager = _build_manager(config) + connector = manager.get_connector("github") + + result = asyncio.run(connector.disconnect(forget_credentials=True)) + + assert result["status"] == "success" + instance = config.connectors.instances["github"] + assert instance.auth.access_token is None + assert instance.auth.authorized_services == [] + + +def test_github_connector_authorize_rejects_classic_token_without_write_scopes(monkeypatch) -> None: + config = OctopalConfig() + config.connectors.instances["github"] = ConnectorInstanceConfig( + enabled=True, + enabled_services=["issues"], + auth={"access_token": "ghp_test"}, + ) + manager = _build_manager(config) + connector = manager.get_connector("github") + + class _FakeClient: + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return None + + async def get(self, path, headers): + return github_connector_module.httpx.Response( + 200, + json={"login": "pmbstyle"}, + headers={"X-OAuth-Scopes": "read:org"}, + ) + + monkeypatch.setattr(github_connector_module.httpx, "AsyncClient", lambda *args, **kwargs: _FakeClient()) + + result = asyncio.run(connector.authorize()) + + assert "public_repo" in result["error"] + + +def test_github_connector_authorize_accepts_fine_grained_token_without_scope_headers(monkeypatch) -> None: + config = OctopalConfig() + config.connectors.instances["github"] = ConnectorInstanceConfig( + enabled=True, + enabled_services=["pull_requests"], + auth={"access_token": "ghp_test"}, + ) + manager = _build_manager(config) + connector = manager.get_connector("github") + + class _FakeClient: + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return None + + async def get(self, path, headers): + return github_connector_module.httpx.Response(200, json={"login": "pmbstyle"}) + + monkeypatch.setattr(github_connector_module.httpx, "AsyncClient", lambda *args, **kwargs: _FakeClient()) + + result = asyncio.run(connector.authorize()) + + assert result["status"] == "success" + assert "fine-grained token" in result["message"] + + +def test_connector_manager_reconciles_ready_github_connector_into_running_mcp() -> None: + config = OctopalConfig() + config.connectors.instances["github"] = ConnectorInstanceConfig( + enabled=True, + enabled_services=["repos"], + auth={"authorized_services": ["repos"], "access_token": "ghp_test"}, + ) + + class _MCP: + def __init__(self) -> None: + self.connected: list[str] = [] + self.disconnected: list[str] = [] + + async def connect_server(self, mcp_config): + self.connected.append(mcp_config.id) + return [] + + async def disconnect_server(self, server_id: str, *, intentional: bool = True): + self.disconnected.append(server_id) + + mcp = _MCP() + manager = ConnectorManager(config=config.connectors, mcp_manager=mcp, octo_config=config) + + asyncio.run(manager.load_and_start_all()) + + assert mcp.connected == ["github-core"] + assert mcp.disconnected == [] + + +def test_connector_manager_uses_internal_github_mcp_server_and_env_names() -> None: + config = OctopalConfig() + config.connectors.instances["github"] = ConnectorInstanceConfig( + enabled=True, + enabled_services=["repos", "issues"], + auth={"authorized_services": ["repos", "issues"], "access_token": "ghp_test"}, + ) + + class _MCP: + def __init__(self) -> None: + self.configs = [] + + async def connect_server(self, mcp_config): + self.configs.append(mcp_config) + return [] + + async def disconnect_server(self, server_id: str, *, intentional: bool = True): + return None + + mcp = _MCP() + manager = ConnectorManager(config=config.connectors, mcp_manager=mcp, octo_config=config) + + asyncio.run(manager.load_and_start_all()) + + assert len(mcp.configs) == 1 + github_cfg = mcp.configs[0] + assert github_cfg.command == sys.executable + assert github_cfg.args == ["-m", "octopal.mcp_servers.github"] + assert github_cfg.env["GITHUB_TOKEN"] == "ghp_test" + assert github_cfg.env["GITHUB_ENABLED_SERVICES"] == "repos,issues" + + def test_connector_manager_reconciles_ready_google_connector_into_running_mcp() -> None: config = OctopalConfig() config.connectors.instances["google"] = ConnectorInstanceConfig( diff --git a/tests/test_github_mcp_server.py b/tests/test_github_mcp_server.py new file mode 100644 index 0000000..80a8a5d --- /dev/null +++ b/tests/test_github_mcp_server.py @@ -0,0 +1,227 @@ +from __future__ import annotations + +import httpx +import pytest + +from octopal.mcp_servers.github import ( + GitHubApiClient, + _normalize_commit, + _normalize_commit_comment, + _normalize_issue, + _normalize_issue_comment, + _normalize_pull_file, + _normalize_pull_request, + _normalize_pull_review, + _normalize_pull_review_comment, + _normalize_repo, + _parse_github_api_error, +) + + +def test_normalize_repo_keeps_expected_fields() -> None: + normalized = _normalize_repo( + { + "id": 1, + "name": "demo", + "full_name": "octo/demo", + "private": False, + "default_branch": "main", + "html_url": "https://github.com/octo/demo", + "owner": { + "login": "octo", + "id": 9, + "type": "User", + "site_admin": False, + "html_url": "https://github.com/octo", + }, + } + ) + + assert normalized["full_name"] == "octo/demo" + assert normalized["owner"]["login"] == "octo" + + +def test_normalize_issue_keeps_labels_and_user() -> None: + normalized = _normalize_issue( + { + "id": 2, + "number": 17, + "title": "Fix connector", + "state": "open", + "user": {"login": "alice", "id": 10, "type": "User", "site_admin": False}, + "labels": [{"name": "bug", "color": "d73a4a", "description": "Something is broken"}], + } + ) + + assert normalized["number"] == 17 + assert normalized["user"]["login"] == "alice" + assert normalized["labels"][0]["name"] == "bug" + + +def test_normalize_pull_request_keeps_base_and_head_refs() -> None: + normalized = _normalize_pull_request( + { + "id": 3, + "number": 8, + "title": "Add connector", + "state": "open", + "mergeable_state": "clean", + "requested_reviewers": [{"login": "carol", "id": 12, "type": "User", "site_admin": False}], + "head": {"label": "octo:feature", "ref": "feature", "sha": "abc123"}, + "base": {"label": "octo:main", "ref": "main", "sha": "def456"}, + "user": {"login": "bob", "id": 11, "type": "User", "site_admin": False}, + } + ) + + assert normalized["number"] == 8 + assert normalized["head"]["ref"] == "feature" + assert normalized["base"]["ref"] == "main" + assert normalized["mergeable_state"] == "clean" + assert normalized["requested_reviewers"][0]["login"] == "carol" + + +def test_normalize_issue_comment_keeps_body_and_author() -> None: + normalized = _normalize_issue_comment( + { + "id": 4, + "body": "Looks good", + "user": {"login": "dana", "id": 13, "type": "User", "site_admin": False}, + } + ) + + assert normalized["id"] == 4 + assert normalized["body"] == "Looks good" + assert normalized["user"]["login"] == "dana" + + +def test_normalize_pull_review_keeps_state_and_user() -> None: + normalized = _normalize_pull_review( + { + "id": 5, + "state": "APPROVED", + "body": "Approved", + "user": {"login": "erin", "id": 14, "type": "User", "site_admin": False}, + } + ) + + assert normalized["state"] == "APPROVED" + assert normalized["user"]["login"] == "erin" + + +def test_normalize_pull_review_comment_keeps_path_and_body() -> None: + normalized = _normalize_pull_review_comment( + { + "id": 6, + "path": "src/app.py", + "body": "Please rename this", + "user": {"login": "frank", "id": 15, "type": "User", "site_admin": False}, + } + ) + + assert normalized["path"] == "src/app.py" + assert normalized["body"] == "Please rename this" + assert normalized["user"]["login"] == "frank" + + +def test_normalize_pull_file_keeps_patch_and_stats() -> None: + normalized = _normalize_pull_file( + { + "sha": "abc123", + "filename": "src/app.py", + "status": "modified", + "additions": 5, + "deletions": 2, + "changes": 7, + "patch": "@@ -1,2 +1,5 @@", + } + ) + + assert normalized["filename"] == "src/app.py" + assert normalized["changes"] == 7 + assert normalized["patch"] == "@@ -1,2 +1,5 @@" + + +def test_normalize_commit_keeps_message_and_sha() -> None: + normalized = _normalize_commit( + { + "sha": "def456", + "html_url": "https://github.com/octo/demo/commit/def456", + "commit": { + "message": "Improve worker flow", + "comment_count": 1, + "author": {"name": "Alice", "email": "alice@example.com", "date": "2026-04-06T10:00:00Z"}, + "committer": {"name": "Alice", "email": "alice@example.com", "date": "2026-04-06T10:00:00Z"}, + "verification": {"verified": True, "reason": "valid", "verified_at": "2026-04-06T10:00:01Z"}, + }, + } + ) + + assert normalized["sha"] == "def456" + assert normalized["message"] == "Improve worker flow" + assert normalized["verification"]["verified"] is True + + +def test_normalize_commit_comment_keeps_path_and_body() -> None: + normalized = _normalize_commit_comment( + { + "id": 7, + "path": "src/app.py", + "body": "Nit: rename this helper", + "user": {"login": "gina", "id": 16, "type": "User", "site_admin": False}, + } + ) + + assert normalized["id"] == 7 + assert normalized["path"] == "src/app.py" + assert normalized["user"]["login"] == "gina" + + +def test_parse_github_api_error_prefers_message_from_json_payload() -> None: + response = httpx.Response( + 403, + json={"message": "Resource not accessible by personal access token", "documentation_url": "https://docs.github.com"}, + ) + + error = _parse_github_api_error(response, method="GET", path="/repos/octo/demo") + + assert error.status_code == 403 + assert "GitHub denied access for this request." in str(error) + + +def test_parse_github_api_error_for_pull_review_write_is_actionable() -> None: + response = httpx.Response( + 403, + json={"message": "Resource not accessible by personal access token"}, + headers={"X-Accepted-GitHub-Permissions": "pull_requests=write"}, + ) + + error = _parse_github_api_error( + response, + method="POST", + path="/repos/pmbstyle/Octopal/pulls/31/reviews", + ) + + assert "Pull requests: write" in str(error) + assert error.details["accepted_permissions"] == "pull_requests=write" + + +@pytest.mark.asyncio +async def test_list_pull_requests_clamps_per_page_to_internal_limit(monkeypatch) -> None: + client = GitHubApiClient.__new__(GitHubApiClient) + + async def _fake_request(method: str, path: str, *, params=None, json_body=None): + assert method == "GET" + assert path == "/repos/pmbstyle/Octopal/pulls" + assert params["per_page"] == 50 + return [] + + client._request = _fake_request # type: ignore[attr-defined] + + payload = await GitHubApiClient.list_pull_requests( + client, + owner="pmbstyle", + repo="Octopal", + per_page=999, + ) + + assert payload["pull_requests"] == [] diff --git a/tests/test_runtime_mcp_integration.py b/tests/test_runtime_mcp_integration.py index 1cb4355..3d90291 100644 --- a/tests/test_runtime_mcp_integration.py +++ b/tests/test_runtime_mcp_integration.py @@ -196,6 +196,88 @@ async def _fake_run(spec, approval_requester=None): assert spec.mcp_tools[0]["server_id"] == "demo" +def test_runtime_includes_connector_alias_tools_for_workers(tmp_path: Path) -> None: + template = WorkerTemplateRecord( + id="worker", + name="Worker", + description="Test worker", + system_prompt="Do work", + available_tools=["github_list_repositories"], + required_permissions=["network"], + model=None, + max_thinking_steps=3, + default_timeout_seconds=30, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + class _Store: + def get_worker_template(self, worker_id: str): + return template + + class _Policy: + def grant_capabilities(self, capabilities): + return [Capability(type="network", scope="worker")] + + class _MCP: + def __init__(self) -> None: + self.sessions = {"github-core": object()} + self.ensure_calls: list[object] = [] + + async def ensure_configured_servers_connected(self, server_ids=None): + self.ensure_calls.append(server_ids) + return {"github-core": "connected"} + + def get_all_tools(self): + return [] + + mcp_manager = _MCP() + runtime = WorkerRuntime( + store=_Store(), + policy=_Policy(), + workspace_dir=tmp_path, + launcher=object(), + mcp_manager=mcp_manager, + settings=Settings(), + ) + + captured: dict[str, object] = {} + + async def _fake_run(spec, approval_requester=None): + captured["spec"] = spec + return WorkerResult(summary="ok") + + runtime.run = _fake_run # type: ignore[method-assign] + + request = TaskRequest(worker_id="worker", task="inspect repos") + asyncio.run(runtime.run_task(request)) + + spec = captured["spec"] + assert "github_list_repositories" in spec.available_tools + assert spec.mcp_tools == [ + { + "name": "github_list_repositories", + "description": "List repositories visible to the connected GitHub account.", + "parameters": { + "type": "object", + "properties": { + "visibility": {"type": "string"}, + "affiliation": {"type": "string"}, + "sort": {"type": "string"}, + "direction": {"type": "string"}, + "per_page": {"type": "integer", "minimum": 1, "maximum": 50}, + "page": {"type": "integer", "minimum": 1}, + }, + "additionalProperties": False, + }, + "permission": "mcp_exec", + "is_async": True, + "server_id": "github-core", + "remote_tool_name": "list_repositories", + } + ] + + def test_runtime_launch_env_includes_workspace_dir(tmp_path: Path) -> None: template = WorkerTemplateRecord( id="worker",