diff --git a/docs/api-reference.md b/docs/api-reference.md index a5e7e96..1691279 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -74,8 +74,8 @@ Scope requirements: ## Write Tools (Write Mode Required) -- `create_issue` (`owner`, `repo`, `title`, optional `body`, `labels`, `assignees`) -- `update_issue` (`owner`, `repo`, `issue_number`, one or more of `title`, `body`, `state`) +- `create_issue` (`owner`, `repo`, `title`, optional `body`, `labels`, `assignees`, `milestone`) +- `update_issue` (`owner`, `repo`, `issue_number`, one or more of `title`, `body`, `state`, `milestone`) - `create_issue_comment` (`owner`, `repo`, `issue_number`, `body`) - `create_pr_comment` (`owner`, `repo`, `pull_number`, `body`) - `add_labels` (`owner`, `repo`, `issue_number`, `labels` by name) @@ -96,6 +96,12 @@ management. Note: `create_issue`, `add_labels`, and `remove_labels` accept label **names**; the server resolves them to Gitea label ids and returns a clear error for unknown labels. +Note: the `milestone` argument on `create_issue`/`update_issue` accepts either a numeric +milestone **id** or a milestone **title** (resolved case-insensitively against open and +closed milestones; unknown titles return a clear error). On `update_issue`, `milestone: 0` +clears the issue's milestone. Gitea Projects (Kanban boards) are intentionally unsupported: +the Gitea REST API exposes no project endpoints. + ## Validation and Limits - All tool argument schemas reject unknown fields. diff --git a/docs/observability.md b/docs/observability.md index 8203702..392134d 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -6,6 +6,26 @@ - Request correlation via `X-Request-ID`. - Security events and policy denials are audit logged. +### Structured event helpers + +`logging_utils` exposes reusable helpers so endpoints emit consistent, +secret-safe structured events instead of ad-hoc inline logging: + +- `log_event(logger, level, event, **context)` — emit a named event with a + `context` mapping; keys in `SENSITIVE_CONTEXT_KEYS` (e.g. `token`, + `authorization`, `password`) are masked as `***`. +- `log_nullable_field(logger, event, field, value)` — record whether a parsed + response field is `None` and its runtime type, without dumping its contents. +- `sanitize_context(context)` — the masking primitive used by the above. + +The `context` mapping is serialized into the JSON log payload under a `context` +key. These run at `DEBUG`, so they are silent unless `LOG_LEVEL=DEBUG`. + +`get_issue` is instrumented with these helpers (`get_issue.start`, +`get_issue.payload_shape`, `get_issue.field_check`) to make nullable-field +parsing failures diagnosable. The same pattern can be reused for other +parsing-heavy endpoints (`get_pull_request`, `list_issues`, `get_commit_diff`). + ## Metrics Prometheus-compatible endpoint: `GET /metrics`. diff --git a/docs/write-mode.md b/docs/write-mode.md index 9e764ed..11b7a58 100644 --- a/docs/write-mode.md +++ b/docs/write-mode.md @@ -13,14 +13,26 @@ Write mode introduces mutation risk (issue/PR changes, metadata updates). Risks ## Supported Write Tools -- `create_issue` -- `update_issue` +- `create_issue` (optional `milestone` id or title) +- `update_issue` (optional `milestone`; `0` clears it) - `create_issue_comment` - `create_pr_comment` +- `edit_issue_comment` - `add_labels` +- `remove_labels` - `assign_issue` +- `create_label` +- `update_label` +- `create_pull_request` +- `create_release` +- `edit_release` +- `create_branch` +- `create_milestone` -Not supported (explicitly forbidden): merge actions, branch deletion, force push. +Not supported (explicitly forbidden): merge actions, branch/label/release deletion, +force push, repo/admin management, and repository content writes (file create/edit, +commits). Gitea Projects (Kanban boards) are unsupported because the Gitea REST API +exposes no project endpoints. ## Enablement Steps diff --git a/src/aegis_gitea_mcp/gitea_client.py b/src/aegis_gitea_mcp/gitea_client.py index 0ed08b0..d4e3f6c 100644 --- a/src/aegis_gitea_mcp/gitea_client.py +++ b/src/aegis_gitea_mcp/gitea_client.py @@ -621,6 +621,41 @@ class GiteaClient: ) return ids + async def _resolve_milestone_id( + self, owner: str, repo: str, milestone: int | str, *, correlation_id: str + ) -> int: + """Resolve a milestone id or title to a numeric milestone id. + + Gitea's issue API requires a numeric milestone id. An integer is used + as-is (``0`` clears the milestone); a string is resolved + case-insensitively against the repository's milestones (open or closed) + and raises a clear error when no title matches. + """ + if isinstance(milestone, int): + return milestone + title = milestone.strip() + existing = await self._request( + "GET", + f"/api/v1/repos/{quote(owner, safe='')}/{quote(repo, safe='')}/milestones", + params={"state": "all", "limit": 100}, + correlation_id=correlation_id, + ) + by_title: dict[str, int] = {} + if isinstance(existing, list): + for item in existing: + if isinstance(item, dict): + m_title = str(item.get("title", "")) + m_id = item.get("id") + if m_title and isinstance(m_id, int): + by_title[m_title.lower()] = m_id + match = by_title.get(title.lower()) + if match is None: + raise GiteaError( + f"Unknown milestone for {owner}/{repo}: {title}. " + "Create it first with create_milestone." + ) + return match + async def create_issue( self, owner: str, @@ -630,6 +665,7 @@ class GiteaClient: body: str, labels: list[str] | None = None, assignees: list[str] | None = None, + milestone: int | str | None = None, ) -> dict[str, Any]: """Create repository issue.""" correlation_id = str( @@ -642,6 +678,10 @@ class GiteaClient: ) if assignees: payload["assignees"] = assignees + if milestone is not None: + payload["milestone"] = await self._resolve_milestone_id( + owner, repo, milestone, correlation_id=correlation_id + ) result = await self._request( "POST", f"/api/v1/repos/{quote(owner, safe='')}/{quote(repo, safe='')}/issues", @@ -659,8 +699,12 @@ class GiteaClient: title: str | None = None, body: str | None = None, state: str | None = None, + milestone: int | str | None = None, ) -> dict[str, Any]: """Update issue fields.""" + correlation_id = str( + self.audit.log_tool_invocation(tool_name="update_issue", result_status="pending") + ) payload: dict[str, Any] = {} if title is not None: payload["title"] = title @@ -668,13 +712,15 @@ class GiteaClient: payload["body"] = body if state is not None: payload["state"] = state + if milestone is not None: + payload["milestone"] = await self._resolve_milestone_id( + owner, repo, milestone, correlation_id=correlation_id + ) result = await self._request( "PATCH", f"/api/v1/repos/{quote(owner, safe='')}/{quote(repo, safe='')}/issues/{index}", json_body=payload, - correlation_id=str( - self.audit.log_tool_invocation(tool_name="update_issue", result_status="pending") - ), + correlation_id=correlation_id, ) return result if isinstance(result, dict) else {} diff --git a/src/aegis_gitea_mcp/logging_utils.py b/src/aegis_gitea_mcp/logging_utils.py index 89d7cf3..309f187 100644 --- a/src/aegis_gitea_mcp/logging_utils.py +++ b/src/aegis_gitea_mcp/logging_utils.py @@ -5,16 +5,22 @@ from __future__ import annotations import json import logging from datetime import datetime, timezone +from typing import Any from aegis_gitea_mcp.request_context import get_request_id +# Context keys whose values must never be written to logs verbatim. +SENSITIVE_CONTEXT_KEYS = frozenset( + {"token", "authorization", "cookie", "password", "secret", "api_key"} +) + class JsonLogFormatter(logging.Formatter): """Format log records as JSON documents.""" def format(self, record: logging.LogRecord) -> str: """Serialize a log record to JSON.""" - payload = { + payload: dict[str, Any] = { "timestamp": datetime.now(timezone.utc).isoformat(), "level": record.levelname, "logger": record.name, @@ -22,6 +28,10 @@ class JsonLogFormatter(logging.Formatter): "request_id": get_request_id(), } + context = getattr(record, "context", None) + if isinstance(context, dict) and context: + payload["context"] = context + if record.exc_info: # Security decision: include only exception type to avoid stack leakage. exception_type = record.exc_info[0] @@ -46,3 +56,55 @@ def configure_logging(level: str) -> None: stream_handler = logging.StreamHandler() stream_handler.setFormatter(JsonLogFormatter()) logger.addHandler(stream_handler) + + +def sanitize_context(context: dict[str, Any]) -> dict[str, Any]: + """Redact sensitive values from a logging context mapping. + + Keys whose lower-cased name is in ``SENSITIVE_CONTEXT_KEYS`` have their + value replaced with ``"***"`` so credentials never reach the log sink. + + Args: + context: Arbitrary key/value pairs intended for structured logging. + + Returns: + A new mapping with sensitive values masked. + """ + return { + key: ("***" if key.lower() in SENSITIVE_CONTEXT_KEYS else value) + for key, value in context.items() + } + + +def log_event(logger: logging.Logger, level: int, event: str, **context: Any) -> None: + """Emit a structured log event with a sanitized context payload. + + Args: + logger: Logger to emit on. + level: Standard ``logging`` level (e.g. ``logging.DEBUG``). + event: Stable, machine-friendly event name (e.g. ``get_issue.start``). + **context: Extra structured fields; sensitive keys are masked. + """ + logger.log(level, event, extra={"context": sanitize_context(context)}) + + +def log_nullable_field(logger: logging.Logger, event: str, field_name: str, value: Any) -> None: + """Log whether a parsed response field is ``None`` and its runtime type. + + Makes null/nullable field failures (such as a ``null`` ``labels`` array) + obvious in logs without dumping the field's full contents. + + Args: + logger: Logger to emit on. + event: Stable event name for the field check. + field_name: Name of the field being inspected. + value: The parsed value to characterize. + """ + log_event( + logger, + logging.DEBUG, + event, + field=field_name, + is_none=value is None, + value_type=(type(value).__name__ if value is not None else None), + ) diff --git a/src/aegis_gitea_mcp/mcp_protocol.py b/src/aegis_gitea_mcp/mcp_protocol.py index 9e35d8f..079956d 100644 --- a/src/aegis_gitea_mcp/mcp_protocol.py +++ b/src/aegis_gitea_mcp/mcp_protocol.py @@ -464,6 +464,10 @@ AVAILABLE_TOOLS: list[MCPTool] = [ "body": {"type": "string", "default": ""}, "labels": {"type": "array", "items": {"type": "string"}, "default": []}, "assignees": {"type": "array", "items": {"type": "string"}, "default": []}, + "milestone": { + "type": ["integer", "string"], + "description": "Milestone id or title to assign the issue to", + }, }, "required": ["owner", "repo", "title"], "additionalProperties": False, @@ -472,7 +476,7 @@ AVAILABLE_TOOLS: list[MCPTool] = [ ), _tool( "update_issue", - "Update issue title/body/state (write-mode only).", + "Update issue title/body/state/milestone (write-mode only).", { "type": "object", "properties": { @@ -482,6 +486,10 @@ AVAILABLE_TOOLS: list[MCPTool] = [ "title": {"type": "string"}, "body": {"type": "string"}, "state": {"type": "string", "enum": ["open", "closed"]}, + "milestone": { + "type": ["integer", "string"], + "description": "Milestone id or title to assign; 0 clears the milestone", + }, }, "required": ["owner", "repo", "issue_number"], "additionalProperties": False, diff --git a/src/aegis_gitea_mcp/tools/arguments.py b/src/aegis_gitea_mcp/tools/arguments.py index d39edde..4d934fc 100644 --- a/src/aegis_gitea_mcp/tools/arguments.py +++ b/src/aegis_gitea_mcp/tools/arguments.py @@ -4,7 +4,14 @@ from __future__ import annotations from typing import Annotated, Literal -from pydantic import AfterValidator, BaseModel, ConfigDict, Field, model_validator +from pydantic import ( + AfterValidator, + BaseModel, + BeforeValidator, + ConfigDict, + Field, + model_validator, +) _REPO_PART_PATTERN = r"^[A-Za-z0-9._-]{1,100}$" @@ -45,6 +52,33 @@ def _validate_git_ref(value: str) -> str: GitRef = Annotated[str, AfterValidator(_validate_git_ref)] +def _validate_milestone(value: object) -> int | str: + """Validate a milestone reference supplied as a numeric id or a title. + + An integer is treated as a milestone id (``0`` clears the milestone on + update); a string is treated as a milestone title to resolve. Runs as a + ``BeforeValidator`` so ``bool`` (a subclass of ``int`` that Pydantic would + otherwise coerce to ``1``/``0``) is rejected on the raw input. + """ + if isinstance(value, bool): + raise ValueError("milestone must be a milestone id or title") + if isinstance(value, int): + if value < 0: + raise ValueError("milestone id must be >= 0") + return value + if isinstance(value, str): + title = value.strip() + if not title: + raise ValueError("milestone title must not be empty") + if len(title) > 256: + raise ValueError("milestone title must not exceed 256 characters") + return title + raise ValueError("milestone must be a milestone id or title") + + +MilestoneRef = Annotated[int | str, BeforeValidator(_validate_milestone)] + + class StrictBaseModel(BaseModel): """Strict model base that rejects unexpected fields.""" @@ -174,6 +208,9 @@ class CreateIssueArgs(RepositoryArgs): body: str = Field(default="", max_length=20_000) labels: list[str] = Field(default_factory=list, max_length=20) assignees: list[str] = Field(default_factory=list, max_length=20) + milestone: MilestoneRef | None = Field( + default=None, description="Milestone id or title to assign the issue to" + ) class UpdateIssueArgs(RepositoryArgs): @@ -183,12 +220,20 @@ class UpdateIssueArgs(RepositoryArgs): title: str | None = Field(default=None, min_length=1, max_length=256) body: str | None = Field(default=None, max_length=20_000) state: Literal["open", "closed"] | None = Field(default=None) + milestone: MilestoneRef | None = Field( + default=None, description="Milestone id or title to assign; 0 clears the milestone" + ) @model_validator(mode="after") def require_change(self) -> UpdateIssueArgs: """Require at least one mutable field in update payload.""" - if self.title is None and self.body is None and self.state is None: - raise ValueError("At least one of title, body, or state must be provided") + if ( + self.title is None + and self.body is None + and self.state is None + and self.milestone is None + ): + raise ValueError("At least one of title, body, state, or milestone must be provided") return self diff --git a/src/aegis_gitea_mcp/tools/read_tools.py b/src/aegis_gitea_mcp/tools/read_tools.py index 594112a..34d44b2 100644 --- a/src/aegis_gitea_mcp/tools/read_tools.py +++ b/src/aegis_gitea_mcp/tools/read_tools.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging from typing import Any from aegis_gitea_mcp.gitea_client import ( @@ -10,6 +11,7 @@ from aegis_gitea_mcp.gitea_client import ( GiteaClient, GiteaError, ) +from aegis_gitea_mcp.logging_utils import log_event, log_nullable_field from aegis_gitea_mcp.response_limits import limit_items, limit_text from aegis_gitea_mcp.tools.arguments import ( CommitDiffArgs, @@ -38,6 +40,8 @@ from aegis_gitea_mcp.tools.arguments import ( SearchCodeArgs, ) +logger = logging.getLogger(__name__) + async def search_code_tool(gitea: GiteaClient, arguments: dict[str, Any]) -> dict[str, Any]: """Search repository code and return bounded result snippets.""" @@ -265,16 +269,34 @@ async def list_issues_tool(gitea: GiteaClient, arguments: dict[str, Any]) -> dic async def get_issue_tool(gitea: GiteaClient, arguments: dict[str, Any]) -> dict[str, Any]: """Get issue details.""" parsed = IssueArgs.model_validate(arguments) + log_event( + logger, + logging.DEBUG, + "get_issue.start", + owner=parsed.owner, + repo=parsed.repo, + issue_number=parsed.issue_number, + ) try: issue = await gitea.get_issue(parsed.owner, parsed.repo, parsed.issue_number) + log_event( + logger, + logging.DEBUG, + "get_issue.payload_shape", + top_level_keys=sorted(issue.keys()) if issue else None, + ) + # Surface nullable collections that previously broke parsing (see #13). + log_nullable_field(logger, "get_issue.field_check", "labels", issue.get("labels")) + log_nullable_field(logger, "get_issue.field_check", "assignees", issue.get("assignees")) + log_nullable_field(logger, "get_issue.field_check", "user", issue.get("user")) return { "number": issue.get("number", 0), "title": limit_text(str(issue.get("title", ""))), "body": limit_text(str(issue.get("body", ""))), "state": issue.get("state", ""), - "author": issue.get("user", {}).get("login", ""), - "labels": [label.get("name", "") for label in issue.get("labels", [])], - "assignees": [assignee.get("login", "") for assignee in issue.get("assignees", [])], + "author": (issue.get("user") or {}).get("login", ""), + "labels": [label.get("name", "") for label in (issue.get("labels") or [])], + "assignees": [assignee.get("login", "") for assignee in (issue.get("assignees") or [])], "created_at": issue.get("created_at", ""), "updated_at": issue.get("updated_at", ""), "url": issue.get("html_url", ""), diff --git a/src/aegis_gitea_mcp/tools/write_tools.py b/src/aegis_gitea_mcp/tools/write_tools.py index a68fdde..686fd5b 100644 --- a/src/aegis_gitea_mcp/tools/write_tools.py +++ b/src/aegis_gitea_mcp/tools/write_tools.py @@ -30,6 +30,14 @@ from aegis_gitea_mcp.tools.arguments import ( ) +def _milestone_title(issue: dict[str, Any]) -> str: + """Extract the milestone title from an issue payload, or '' if unset.""" + milestone = issue.get("milestone") + if isinstance(milestone, dict): + return limit_text(str(milestone.get("title", ""))) + return "" + + async def create_label_tool(gitea: GiteaClient, arguments: dict[str, Any]) -> dict[str, Any]: """Create a repository label in write mode.""" parsed = CreateLabelArgs.model_validate(arguments) @@ -119,11 +127,13 @@ async def create_issue_tool(gitea: GiteaClient, arguments: dict[str, Any]) -> di body=parsed.body, labels=parsed.labels, assignees=parsed.assignees, + milestone=parsed.milestone, ) return { "number": issue.get("number", 0), "title": limit_text(str(issue.get("title", ""))), "state": issue.get("state", ""), + "milestone": _milestone_title(issue), "url": issue.get("html_url", ""), } except (GiteaAuthenticationError, GiteaAuthorizationError): @@ -145,11 +155,13 @@ async def update_issue_tool(gitea: GiteaClient, arguments: dict[str, Any]) -> di title=parsed.title, body=parsed.body, state=parsed.state, + milestone=parsed.milestone, ) return { "number": issue.get("number", parsed.issue_number), "title": limit_text(str(issue.get("title", ""))), "state": issue.get("state", ""), + "milestone": _milestone_title(issue), "url": issue.get("html_url", ""), } except (GiteaAuthenticationError, GiteaAuthorizationError): diff --git a/tests/test_gitea_client.py b/tests/test_gitea_client.py index 061c95c..279a20a 100644 --- a/tests/test_gitea_client.py +++ b/tests/test_gitea_client.py @@ -272,6 +272,76 @@ async def test_resolve_label_ids_rejects_unknown_label() -> None: await client._resolve_label_ids("o", "r", ["ghost"], correlation_id="c") +@pytest.mark.asyncio +async def test_resolve_milestone_id_passes_through_integer() -> None: + """An integer milestone reference is used as a Gitea milestone id as-is.""" + client = GiteaClient(token="user-token") + client._request = AsyncMock() # type: ignore[method-assign] + assert await client._resolve_milestone_id("o", "r", 7, correlation_id="c") == 7 + # Integer ids must not trigger a milestone lookup. + client._request.assert_not_called() + + +@pytest.mark.asyncio +async def test_resolve_milestone_id_maps_title_case_insensitively() -> None: + """A milestone title is resolved to its id regardless of case.""" + client = GiteaClient(token="user-token") + + async def fake_request(method: str, endpoint: str, **kwargs): + return [{"id": 11, "title": "Sprint 1"}, {"id": 12, "title": "Backlog"}] + + client._request = AsyncMock(side_effect=fake_request) # type: ignore[method-assign] + resolved = await client._resolve_milestone_id("o", "r", "sprint 1", correlation_id="c") + assert resolved == 11 + + +@pytest.mark.asyncio +async def test_resolve_milestone_id_rejects_unknown_title() -> None: + """An unknown milestone title raises a clear error.""" + client = GiteaClient(token="user-token") + + async def fake_request(method: str, endpoint: str, **kwargs): + return [{"id": 11, "title": "Sprint 1"}] + + client._request = AsyncMock(side_effect=fake_request) # type: ignore[method-assign] + with pytest.raises(GiteaError, match="Unknown milestone"): + await client._resolve_milestone_id("o", "r", "Sprint 2", correlation_id="c") + + +@pytest.mark.asyncio +async def test_create_issue_resolves_milestone_title() -> None: + """create_issue resolves a milestone title to an id in the POST payload.""" + client = GiteaClient(token="user-token") + captured: dict = {} + + async def fake_request(method: str, endpoint: str, **kwargs): + if endpoint.endswith("/milestones") and method == "GET": + return [{"id": 11, "title": "Sprint 1"}] + if endpoint.endswith("/issues") and method == "POST": + captured["payload"] = kwargs.get("json_body") + return {"number": 1, "title": "Issue", "state": "open"} + return {} + + client._request = AsyncMock(side_effect=fake_request) # type: ignore[method-assign] + await client.create_issue("o", "r", title="Issue", body="", milestone="Sprint 1") + assert captured["payload"]["milestone"] == 11 + + +@pytest.mark.asyncio +async def test_update_issue_clears_milestone_with_zero() -> None: + """update_issue forwards milestone id 0 verbatim to clear the milestone.""" + client = GiteaClient(token="user-token") + captured: dict = {} + + async def fake_request(method: str, endpoint: str, **kwargs): + captured["payload"] = kwargs.get("json_body") + return {"number": 1, "title": "Issue", "state": "open"} + + client._request = AsyncMock(side_effect=fake_request) # type: ignore[method-assign] + await client.update_issue("o", "r", 1, milestone=0) + assert captured["payload"]["milestone"] == 0 + + @pytest.mark.asyncio async def test_add_labels_resolves_names_to_ids() -> None: """add_labels translates names to ids before POSTing to Gitea.""" diff --git a/tests/test_logging_utils.py b/tests/test_logging_utils.py new file mode 100644 index 0000000..f5a62fa --- /dev/null +++ b/tests/test_logging_utils.py @@ -0,0 +1,120 @@ +"""Tests for structured logging helpers and get_issue instrumentation (#14).""" + +from __future__ import annotations + +import json +import logging + +import pytest + +from aegis_gitea_mcp.config import reset_settings +from aegis_gitea_mcp.logging_utils import ( + JsonLogFormatter, + log_event, + log_nullable_field, + sanitize_context, +) +from aegis_gitea_mcp.tools.read_tools import get_issue_tool + +READ_TOOLS_LOGGER = "aegis_gitea_mcp.tools.read_tools" + + +@pytest.fixture(autouse=True) +def tool_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Provide minimal settings environment for response limit helpers.""" + reset_settings() + monkeypatch.setenv("GITEA_URL", "https://gitea.example.com") + monkeypatch.setenv("GITEA_TOKEN", "test-token") + monkeypatch.setenv("MCP_API_KEYS", "a" * 64) + monkeypatch.setenv("ENVIRONMENT", "test") + + +def test_sanitize_context_masks_sensitive_keys() -> None: + """Sensitive keys are masked case-insensitively; others pass through.""" + cleaned = sanitize_context( + {"owner": "acme", "Token": "abc", "password": "x", "issue_number": 7} + ) + + assert cleaned["owner"] == "acme" + assert cleaned["issue_number"] == 7 + assert cleaned["Token"] == "***" + assert cleaned["password"] == "***" + + +def test_json_formatter_includes_context() -> None: + """The formatter serializes a record's context mapping into the payload.""" + record = logging.LogRecord( + "test", logging.DEBUG, __file__, 1, "get_issue.field_check", None, None + ) + record.context = {"field": "labels", "is_none": True} + + payload = json.loads(JsonLogFormatter().format(record)) + + assert payload["message"] == "get_issue.field_check" + assert payload["context"] == {"field": "labels", "is_none": True} + + +def test_log_event_emits_sanitized_context(caplog: pytest.LogCaptureFixture) -> None: + """log_event records the event name and masks sensitive context values.""" + logger = logging.getLogger("test.log_event") + with caplog.at_level(logging.DEBUG, logger="test.log_event"): + log_event(logger, logging.DEBUG, "evt", owner="acme", token="secret") + + record = caplog.records[-1] + assert record.getMessage() == "evt" + assert record.context == {"owner": "acme", "token": "***"} + + +def test_log_nullable_field_characterizes_value(caplog: pytest.LogCaptureFixture) -> None: + """log_nullable_field reports None-ness and runtime type without dumping data.""" + logger = logging.getLogger("test.nullable") + with caplog.at_level(logging.DEBUG, logger="test.nullable"): + log_nullable_field(logger, "evt", "labels", None) + log_nullable_field(logger, "evt", "labels", [1, 2]) + + assert caplog.records[0].context == { + "field": "labels", + "is_none": True, + "value_type": None, + } + assert caplog.records[1].context == { + "field": "labels", + "is_none": False, + "value_type": "list", + } + + +class _NullIssueGitea: + """Minimal stub returning an issue with null collection fields.""" + + async def get_issue(self, owner: str, repo: str, index: int) -> dict[str, object]: + return { + "number": index, + "title": "Issue", + "body": "Body", + "state": "open", + "labels": None, + "assignees": None, + "user": None, + } + + +@pytest.mark.asyncio +async def test_get_issue_tool_emits_debug_events(caplog: pytest.LogCaptureFixture) -> None: + """get_issue_tool emits start/shape/field-check debug events and still parses.""" + with caplog.at_level(logging.DEBUG, logger=READ_TOOLS_LOGGER): + result = await get_issue_tool( + _NullIssueGitea(), {"owner": "acme", "repo": "app", "issue_number": 7} + ) + + events = [r.getMessage() for r in caplog.records] + assert "get_issue.start" in events + assert "get_issue.payload_shape" in events + + field_checks = [r.context for r in caplog.records if r.getMessage() == "get_issue.field_check"] + assert {"field": "labels", "is_none": True, "value_type": None} in field_checks + assert {"field": "user", "is_none": True, "value_type": None} in field_checks + + # Logging must not change behaviour: null collections still parse to empties. + assert result["labels"] == [] + assert result["assignees"] == [] diff --git a/tests/test_tools_extended.py b/tests/test_tools_extended.py index 8828c1c..0b9df0a 100644 --- a/tests/test_tools_extended.py +++ b/tests/test_tools_extended.py @@ -3,7 +3,7 @@ import pytest from aegis_gitea_mcp.config import reset_settings -from aegis_gitea_mcp.gitea_client import GiteaError +from aegis_gitea_mcp.gitea_client import GiteaAuthenticationError, GiteaError from aegis_gitea_mcp.tools.read_tools import ( compare_refs_tool, get_branch_tool, @@ -140,11 +140,21 @@ class StubGitea: async def list_repo_topics(self, owner, repo): return ["python", "mcp"] - async def create_issue(self, owner, repo, *, title, body, labels=None, assignees=None): - return {"number": 1, "title": title, "state": "open"} + async def create_issue( + self, owner, repo, *, title, body, labels=None, assignees=None, milestone=None + ): + result = {"number": 1, "title": title, "state": "open"} + if milestone is not None: + result["milestone"] = {"id": 4, "title": str(milestone)} + return result - async def update_issue(self, owner, repo, index, *, title=None, body=None, state=None): - return {"number": index, "title": title or "Issue", "state": state or "open"} + async def update_issue( + self, owner, repo, index, *, title=None, body=None, state=None, milestone=None + ): + result = {"number": index, "title": title or "Issue", "state": state or "open"} + if milestone is not None: + result["milestone"] = {"id": 4, "title": str(milestone)} + return result async def create_issue_comment(self, owner, repo, index, body): return {"id": 1, "body": body} @@ -265,6 +275,34 @@ async def test_extended_read_tools_failure_mode() -> None: await list_commits_tool(ErrorGitea(), {"owner": "acme", "repo": "app"}) +@pytest.mark.asyncio +async def test_get_issue_tolerates_null_collections() -> None: + """Regression for #13: Gitea may return null for labels/assignees/user. + + `.get(key, [])` returns None when the key is present with a null value, so + iterating the result raised `'NoneType' object is not iterable`. + """ + + class NullFieldsGitea(StubGitea): + async def get_issue(self, owner, repo, index): + return { + "number": index, + "title": "Issue", + "body": "Body", + "state": "open", + "user": None, + "labels": None, + "assignees": None, + } + + result = await get_issue_tool( + NullFieldsGitea(), {"owner": "acme", "repo": "app", "issue_number": 1} + ) + assert result["author"] == "" + assert result["labels"] == [] + assert result["assignees"] == [] + + @pytest.mark.asyncio @pytest.mark.parametrize( "tool,args,expected_key", @@ -374,3 +412,104 @@ def test_create_label_args_reject_invalid_color() -> None: with pytest.raises(pydantic.ValidationError): CreateLabelArgs(owner="o", repo="r", name="bug", color="red") + + +@pytest.mark.asyncio +async def test_create_issue_returns_assigned_milestone_title() -> None: + """create_issue surfaces the assigned milestone title in its response.""" + result = await create_issue_tool( + StubGitea(), + {"owner": "acme", "repo": "app", "title": "Issue", "milestone": "Sprint 1"}, + ) + assert result["milestone"] == "Sprint 1" + + +@pytest.mark.asyncio +async def test_update_issue_accepts_milestone_only() -> None: + """update_issue may change only the milestone (no title/body/state needed).""" + result = await update_issue_tool( + StubGitea(), + {"owner": "acme", "repo": "app", "issue_number": 1, "milestone": 4}, + ) + assert result["milestone"] == "4" + + +def test_update_issue_args_require_a_changed_field() -> None: + """An update with no mutable field (incl. milestone) is rejected.""" + import pydantic + + from aegis_gitea_mcp.tools.arguments import UpdateIssueArgs + + with pytest.raises(pydantic.ValidationError): + UpdateIssueArgs(owner="o", repo="r", issue_number=1) + # Supplying only a milestone satisfies the change requirement. + assert UpdateIssueArgs(owner="o", repo="r", issue_number=1, milestone=0).milestone == 0 + + +def test_issue_args_reject_boolean_milestone() -> None: + """A boolean is rejected as a milestone reference (it subclasses int).""" + import pydantic + + from aegis_gitea_mcp.tools.arguments import CreateIssueArgs + + with pytest.raises(pydantic.ValidationError): + CreateIssueArgs(owner="o", repo="r", title="x", milestone=True) + + +# (tool, valid_args) for every write tool, used to exercise error branches. +WRITE_TOOL_ERROR_CASES = [ + (create_issue_tool, {"owner": "acme", "repo": "app", "title": "Issue"}), + (update_issue_tool, {"owner": "acme", "repo": "app", "issue_number": 1, "title": "x"}), + (create_issue_comment_tool, {"owner": "acme", "repo": "app", "issue_number": 1, "body": "c"}), + (create_pr_comment_tool, {"owner": "acme", "repo": "app", "pull_number": 1, "body": "c"}), + (add_labels_tool, {"owner": "acme", "repo": "app", "issue_number": 1, "labels": ["bug"]}), + (assign_issue_tool, {"owner": "acme", "repo": "app", "issue_number": 1, "assignees": ["al"]}), + (create_label_tool, {"owner": "acme", "repo": "app", "name": "bug", "color": "#ff0000"}), + (update_label_tool, {"owner": "acme", "repo": "app", "name": "bug", "new_name": "defect"}), + (remove_labels_tool, {"owner": "acme", "repo": "app", "issue_number": 1, "labels": ["bug"]}), + ( + create_pull_request_tool, + {"owner": "acme", "repo": "app", "title": "PR", "head": "feature", "base": "main"}, + ), + (create_release_tool, {"owner": "acme", "repo": "app", "tag_name": "v1.0.0"}), + (edit_release_tool, {"owner": "acme", "repo": "app", "release_id": 3, "name": "x"}), + (create_branch_tool, {"owner": "acme", "repo": "app", "new_branch_name": "feature/x"}), + (create_milestone_tool, {"owner": "acme", "repo": "app", "title": "M1"}), + (edit_issue_comment_tool, {"owner": "acme", "repo": "app", "comment_id": 5, "body": "e"}), +] + + +class _WriteBackendErrorGitea: + """Stub whose every method raises a generic Gitea backend error.""" + + def __getattr__(self, name: str): + async def _raise(*args: object, **kwargs: object) -> object: + raise GiteaError("backend failure") + + return _raise + + +class _WriteAuthErrorGitea: + """Stub whose every method raises an authentication error.""" + + def __getattr__(self, name: str): + async def _raise(*args: object, **kwargs: object) -> object: + raise GiteaAuthenticationError("token expired") + + return _raise + + +@pytest.mark.asyncio +@pytest.mark.parametrize("tool,args", WRITE_TOOL_ERROR_CASES) +async def test_write_tools_wrap_backend_errors(tool, args) -> None: + """Every write tool wraps a backend GiteaError as a RuntimeError.""" + with pytest.raises(RuntimeError): + await tool(_WriteBackendErrorGitea(), args) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("tool,args", WRITE_TOOL_ERROR_CASES) +async def test_write_tools_propagate_auth_errors(tool, args) -> None: + """Every write tool lets auth failures surface for re-authorization.""" + with pytest.raises(GiteaAuthenticationError): + await tool(_WriteAuthErrorGitea(), args)