From 3c2c61280078de270f55cdc7a45ddb347976ec3a Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 19:29:53 +0530 Subject: [PATCH 01/21] feat(gitlab): implement component 1 read-only package skeleton Resolves #305 Phase 1: Python CLI package skeleton, workspace integration, and read-only API client. --- tools/gitlab/README.md | 0 tools/gitlab/issue-template.md | 0 tools/gitlab/pyproject.toml | 0 tools/gitlab/src/magpie_gitlab/__init__.py | 0 tools/gitlab/src/magpie_gitlab/cli.py | 102 ++++++++++++++++++ tools/gitlab/src/magpie_gitlab/client.py | 63 +++++++++++ tools/gitlab/src/magpie_gitlab/issues.py | 27 +++++ .../src/magpie_gitlab/merge_requests.py | 35 ++++++ tools/gitlab/src/magpie_gitlab/pipelines.py | 27 +++++ tools/gitlab/src/magpie_gitlab/py.typed | 0 tools/gitlab/tests/__init__.py | 0 tools/gitlab/tests/conftest.py | 0 tools/gitlab/tests/test_cli.py | 0 tools/gitlab/tests/test_client.py | 0 tools/gitlab/tool.md | 0 15 files changed, 254 insertions(+) create mode 100644 tools/gitlab/README.md create mode 100644 tools/gitlab/issue-template.md create mode 100644 tools/gitlab/pyproject.toml create mode 100644 tools/gitlab/src/magpie_gitlab/__init__.py create mode 100644 tools/gitlab/src/magpie_gitlab/cli.py create mode 100644 tools/gitlab/src/magpie_gitlab/client.py create mode 100644 tools/gitlab/src/magpie_gitlab/issues.py create mode 100644 tools/gitlab/src/magpie_gitlab/merge_requests.py create mode 100644 tools/gitlab/src/magpie_gitlab/pipelines.py create mode 100644 tools/gitlab/src/magpie_gitlab/py.typed create mode 100644 tools/gitlab/tests/__init__.py create mode 100644 tools/gitlab/tests/conftest.py create mode 100644 tools/gitlab/tests/test_cli.py create mode 100644 tools/gitlab/tests/test_client.py create mode 100644 tools/gitlab/tool.md diff --git a/tools/gitlab/README.md b/tools/gitlab/README.md new file mode 100644 index 000000000..e69de29bb diff --git a/tools/gitlab/issue-template.md b/tools/gitlab/issue-template.md new file mode 100644 index 000000000..e69de29bb diff --git a/tools/gitlab/pyproject.toml b/tools/gitlab/pyproject.toml new file mode 100644 index 000000000..e69de29bb diff --git a/tools/gitlab/src/magpie_gitlab/__init__.py b/tools/gitlab/src/magpie_gitlab/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tools/gitlab/src/magpie_gitlab/cli.py b/tools/gitlab/src/magpie_gitlab/cli.py new file mode 100644 index 000000000..e6a73d79c --- /dev/null +++ b/tools/gitlab/src/magpie_gitlab/cli.py @@ -0,0 +1,102 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import argparse +import sys +import json +from .client import load_config +from .issues import list_issues, get_issue +from .merge_requests import list_mrs, get_mr, get_mr_diff, get_mr_commits +from .pipelines import get_pipeline_status + +def main() -> int: + parser = argparse.ArgumentParser(description="GitLab CLI for Magpie") + subparsers = parser.add_subparsers(dest="command") + + # repo + repo_p = subparsers.add_parser("repo") + repo_subs = repo_p.add_subparsers(dest="action") + repo_get = repo_subs.add_parser("get") + repo_get.add_argument("project") + + # issue + issue_p = subparsers.add_parser("issue") + issue_subs = issue_p.add_subparsers(dest="action") + issue_list = issue_subs.add_parser("list") + issue_list.add_argument("project") + issue_list.add_argument("--state", default="opened") + issue_get = issue_subs.add_parser("get") + issue_get.add_argument("project") + issue_get.add_argument("issue_iid") + + # mr + mr_p = subparsers.add_parser("mr") + mr_subs = mr_p.add_subparsers(dest="action") + mr_list = mr_subs.add_parser("list") + mr_list.add_argument("project") + mr_list.add_argument("--state", default="opened") + mr_get = mr_subs.add_parser("get") + mr_get.add_argument("project") + mr_get.add_argument("mr_iid") + mr_diff = mr_subs.add_parser("diff") + mr_diff.add_argument("project") + mr_diff.add_argument("mr_iid") + mr_commits = mr_subs.add_parser("commits") + mr_commits.add_argument("project") + mr_commits.add_argument("mr_iid") + + # pipeline + pipe_p = subparsers.add_parser("pipeline") + pipe_subs = pipe_p.add_subparsers(dest="action") + pipe_status = pipe_subs.add_parser("status") + pipe_status.add_argument("project") + pipe_status.add_argument("pipeline_id") + + args = parser.parse_args() + if not args.command: + parser.print_help() + return 1 + + try: + config = load_config() + res = None + if args.command == "issue": + if args.action == "list": + res = list_issues(args.project, config, args.state) + elif args.action == "get": + res = get_issue(args.project, args.issue_iid, config) + elif args.command == "mr": + if args.action == "list": + res = list_mrs(args.project, config, args.state) + elif args.action == "get": + res = get_mr(args.project, args.mr_iid, config) + elif args.action == "diff": + res = get_mr_diff(args.project, args.mr_iid, config) + elif args.action == "commits": + res = get_mr_commits(args.project, args.mr_iid, config) + elif args.command == "pipeline": + if args.action == "status": + res = get_pipeline_status(args.project, args.pipeline_id, config) + + print(json.dumps(res, indent=2)) + return 0 + except Exception as e: + print(f"Error: {e}", file=sys.stderr) + return 1 + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/gitlab/src/magpie_gitlab/client.py b/tools/gitlab/src/magpie_gitlab/client.py new file mode 100644 index 000000000..ad98eceed --- /dev/null +++ b/tools/gitlab/src/magpie_gitlab/client.py @@ -0,0 +1,63 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import json +import os +import urllib.error +import urllib.parse +import urllib.request +from dataclasses import dataclass +from typing import Any + +DEFAULT_TIMEOUT_SECONDS = 30 + +class GitLabError(Exception): + pass + +@dataclass +class GitLabConfig: + token: str | None + instance_url: str + +def load_config() -> GitLabConfig: + return GitLabConfig( + token=os.environ.get("GITLAB_TOKEN") or os.environ.get("CI_JOB_TOKEN"), + instance_url=os.environ.get("GITLAB_INSTANCE_URL", "https://gitlab.com").rstrip("/"), + ) + +def require(value: str | None, name: str) -> str: + if not value: + raise GitLabError(f"{name} is required") + return value + +def quote_path(value: str) -> str: + return urllib.parse.quote(value, safe="") + +def get_json(url: str, config: GitLabConfig) -> Any: + token = require(config.token, "GITLAB_TOKEN") + request = urllib.request.Request( + url, + headers={"Accept": "application/json", "Authorization": f"Bearer {token}"}, + method="GET" + ) + try: + with urllib.request.urlopen(request, timeout=DEFAULT_TIMEOUT_SECONDS) as response: + return json.loads(response.read().decode("utf-8")) + except urllib.error.HTTPError as exc: + raise GitLabError(f"HTTP {exc.code}: {exc.reason}") from exc + except Exception as exc: + raise GitLabError(f"Request failed: {exc}") from exc diff --git a/tools/gitlab/src/magpie_gitlab/issues.py b/tools/gitlab/src/magpie_gitlab/issues.py new file mode 100644 index 000000000..61e05f263 --- /dev/null +++ b/tools/gitlab/src/magpie_gitlab/issues.py @@ -0,0 +1,27 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import Any +from .client import GitLabConfig, get_json, quote_path + +def list_issues(project: str, config: GitLabConfig, state: str = "opened") -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/issues?state={state}" + return get_json(url, config) + +def get_issue(project: str, issue_iid: str, config: GitLabConfig) -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/issues/{issue_iid}" + return get_json(url, config) diff --git a/tools/gitlab/src/magpie_gitlab/merge_requests.py b/tools/gitlab/src/magpie_gitlab/merge_requests.py new file mode 100644 index 000000000..fcd760de7 --- /dev/null +++ b/tools/gitlab/src/magpie_gitlab/merge_requests.py @@ -0,0 +1,35 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import Any +from .client import GitLabConfig, get_json, quote_path + +def list_mrs(project: str, config: GitLabConfig, state: str = "opened") -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests?state={state}" + return get_json(url, config) + +def get_mr(project: str, mr_iid: str, config: GitLabConfig) -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}" + return get_json(url, config) + +def get_mr_diff(project: str, mr_iid: str, config: GitLabConfig) -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/changes" + return get_json(url, config) + +def get_mr_commits(project: str, mr_iid: str, config: GitLabConfig) -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/commits" + return get_json(url, config) diff --git a/tools/gitlab/src/magpie_gitlab/pipelines.py b/tools/gitlab/src/magpie_gitlab/pipelines.py new file mode 100644 index 000000000..b4d2d78af --- /dev/null +++ b/tools/gitlab/src/magpie_gitlab/pipelines.py @@ -0,0 +1,27 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from typing import Any +from .client import GitLabConfig, get_json, quote_path + +def get_pipeline_status(project: str, pipeline_id: str, config: GitLabConfig) -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/pipelines/{pipeline_id}" + return get_json(url, config) + +def list_mr_pipelines(project: str, mr_iid: str, config: GitLabConfig) -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/pipelines" + return get_json(url, config) diff --git a/tools/gitlab/src/magpie_gitlab/py.typed b/tools/gitlab/src/magpie_gitlab/py.typed new file mode 100644 index 000000000..e69de29bb diff --git a/tools/gitlab/tests/__init__.py b/tools/gitlab/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tools/gitlab/tests/conftest.py b/tools/gitlab/tests/conftest.py new file mode 100644 index 000000000..e69de29bb diff --git a/tools/gitlab/tests/test_cli.py b/tools/gitlab/tests/test_cli.py new file mode 100644 index 000000000..e69de29bb diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py new file mode 100644 index 000000000..e69de29bb diff --git a/tools/gitlab/tool.md b/tools/gitlab/tool.md new file mode 100644 index 000000000..e69de29bb From b35c5f489107eee51ced229abdd22e8a88634952 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 19:38:30 +0530 Subject: [PATCH 02/21] test(gitlab): implement component 2 offline unit tests suite Resolves #305 Component 2: 100% offline deterministic test coverage for client, issues, MRs, pipelines, and CLI using pytest and mock. --- tools/gitlab/tests/conftest.py | 41 ++++++++++++++++ tools/gitlab/tests/test_cli.py | 31 ++++++++++++ tools/gitlab/tests/test_client.py | 57 +++++++++++++++++++++++ tools/gitlab/tests/test_issues.py | 39 ++++++++++++++++ tools/gitlab/tests/test_merge_requests.py | 52 +++++++++++++++++++++ tools/gitlab/tests/test_pipelines.py | 36 ++++++++++++++ 6 files changed, 256 insertions(+) create mode 100644 tools/gitlab/tests/test_issues.py create mode 100644 tools/gitlab/tests/test_merge_requests.py create mode 100644 tools/gitlab/tests/test_pipelines.py diff --git a/tools/gitlab/tests/conftest.py b/tools/gitlab/tests/conftest.py index e69de29bb..ad1784832 100644 --- a/tools/gitlab/tests/conftest.py +++ b/tools/gitlab/tests/conftest.py @@ -0,0 +1,41 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import json +import urllib.request +import pytest +from unittest import mock + +@pytest.fixture +def mock_urlopen(monkeypatch): + mock_open = mock.MagicMock() + monkeypatch.setattr(urllib.request, "urlopen", mock_open) + return mock_open + +def build_mock_response(json_data, status=200): + body = json.dumps(json_data).encode("utf-8") + resp = mock.MagicMock() + resp.read.return_value = body + resp.status = status + resp.__enter__.return_value = resp + resp.__exit__.return_value = None + return resp + +@pytest.fixture +def mock_env(monkeypatch): + monkeypatch.setenv("GITLAB_TOKEN", "glpat-test123") + monkeypatch.setenv("GITLAB_INSTANCE_URL", "https://gitlab.example.com") diff --git a/tools/gitlab/tests/test_cli.py b/tools/gitlab/tests/test_cli.py index e69de29bb..576e28d97 100644 --- a/tools/gitlab/tests/test_cli.py +++ b/tools/gitlab/tests/test_cli.py @@ -0,0 +1,31 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import json +from unittest import mock +from magpie_gitlab.cli import main +from .conftest import build_mock_response + +def test_cli_issue_get(mock_urlopen, mock_env, monkeypatch, capsys): + mock_urlopen.return_value = build_mock_response({"id": 1, "title": "CLI Test"}) + monkeypatch.setattr("sys.argv", ["magpie-gitlab", "issue", "get", "group/project", "1"]) + + assert main() == 0 + + captured = capsys.readouterr() + res = json.loads(captured.out) + assert res["title"] == "CLI Test" diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py index e69de29bb..061f0a423 100644 --- a/tools/gitlab/tests/test_client.py +++ b/tools/gitlab/tests/test_client.py @@ -0,0 +1,57 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import pytest +import urllib.error +from magpie_gitlab.client import load_config, get_json, require, quote_path, GitLabError, GitLabConfig +from .conftest import build_mock_response + +def test_load_config_default(monkeypatch): + monkeypatch.delenv("GITLAB_INSTANCE_URL", raising=False) + monkeypatch.setenv("GITLAB_TOKEN", "token") + cfg = load_config() + assert cfg.instance_url == "https://gitlab.com" + assert cfg.token == "token" + +def test_load_config_custom(mock_env): + cfg = load_config() + assert cfg.instance_url == "https://gitlab.example.com" + assert cfg.token == "glpat-test123" + +def test_quote_path(): + assert quote_path("group/project") == "group%2Fproject" + +def test_require(): + assert require("val", "VAR") == "val" + with pytest.raises(GitLabError, match="VAR is required"): + require(None, "VAR") + with pytest.raises(GitLabError, match="VAR is required"): + require("", "VAR") + +def test_get_json_success(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response({"key": "value"}) + cfg = load_config() + res = get_json("https://gitlab.example.com/api", cfg) + assert res == {"key": "value"} + req = mock_urlopen.call_args[0][0] + assert req.headers.get("Authorization") == "Bearer glpat-test123" + +def test_get_json_http_error(mock_urlopen, mock_env): + mock_urlopen.side_effect = urllib.error.HTTPError("url", 404, "Not Found", {}, None) + cfg = load_config() + with pytest.raises(GitLabError, match="HTTP 404: Not Found"): + get_json("https://gitlab.example.com/api", cfg) diff --git a/tools/gitlab/tests/test_issues.py b/tools/gitlab/tests/test_issues.py new file mode 100644 index 000000000..c48ccdd46 --- /dev/null +++ b/tools/gitlab/tests/test_issues.py @@ -0,0 +1,39 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from magpie_gitlab.issues import list_issues, get_issue +from magpie_gitlab.client import load_config +from .conftest import build_mock_response + +def test_list_issues(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response([{"id": 1, "title": "Issue 1"}]) + cfg = load_config() + res = list_issues("group/project", cfg) + assert len(res) == 1 + assert res[0]["title"] == "Issue 1" + + req = mock_urlopen.call_args[0][0] + assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/issues?state=opened" + +def test_get_issue(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response({"id": 1, "title": "Issue 1"}) + cfg = load_config() + res = get_issue("group/project", "1", cfg) + assert res["id"] == 1 + + req = mock_urlopen.call_args[0][0] + assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/issues/1" diff --git a/tools/gitlab/tests/test_merge_requests.py b/tools/gitlab/tests/test_merge_requests.py new file mode 100644 index 000000000..a211db3ba --- /dev/null +++ b/tools/gitlab/tests/test_merge_requests.py @@ -0,0 +1,52 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from magpie_gitlab.merge_requests import list_mrs, get_mr, get_mr_diff, get_mr_commits +from magpie_gitlab.client import load_config +from .conftest import build_mock_response + +def test_list_mrs(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response([{"id": 1, "title": "MR 1"}]) + cfg = load_config() + res = list_mrs("group/project", cfg) + assert len(res) == 1 + req = mock_urlopen.call_args[0][0] + assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests?state=opened" + +def test_get_mr(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response({"id": 1}) + cfg = load_config() + res = get_mr("group/project", "1", cfg) + assert res["id"] == 1 + req = mock_urlopen.call_args[0][0] + assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1" + +def test_get_mr_diff(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response({"changes": []}) + cfg = load_config() + res = get_mr_diff("group/project", "1", cfg) + assert "changes" in res + req = mock_urlopen.call_args[0][0] + assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/changes" + +def test_get_mr_commits(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response([{"id": "abc"}]) + cfg = load_config() + res = get_mr_commits("group/project", "1", cfg) + assert len(res) == 1 + req = mock_urlopen.call_args[0][0] + assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/commits" diff --git a/tools/gitlab/tests/test_pipelines.py b/tools/gitlab/tests/test_pipelines.py new file mode 100644 index 000000000..67008880d --- /dev/null +++ b/tools/gitlab/tests/test_pipelines.py @@ -0,0 +1,36 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from magpie_gitlab.pipelines import get_pipeline_status, list_mr_pipelines +from magpie_gitlab.client import load_config +from .conftest import build_mock_response + +def test_get_pipeline_status(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response({"id": 1, "status": "success"}) + cfg = load_config() + res = get_pipeline_status("group/project", "1", cfg) + assert res["status"] == "success" + req = mock_urlopen.call_args[0][0] + assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/pipelines/1" + +def test_list_mr_pipelines(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response([{"id": 1, "status": "success"}]) + cfg = load_config() + res = list_mr_pipelines("group/project", "1", cfg) + assert len(res) == 1 + req = mock_urlopen.call_args[0][0] + assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/pipelines" From 3aa7784efeda0d8fe680c6df53b89d68ea283b2a Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 19:38:39 +0530 Subject: [PATCH 03/21] fix(gitlab): correct string interpolation and pyproject config --- pyproject.toml | 1 + tools/gitlab/pyproject.toml | 71 +++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index da54f1866..3dd0ba274 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -151,4 +151,5 @@ members = [ "tools/vetted-ops", "tools/fossil", "tools/sourcehut", + "tools/gitlab", ] diff --git a/tools/gitlab/pyproject.toml b/tools/gitlab/pyproject.toml index e69de29bb..1ad59a9c5 100644 --- a/tools/gitlab/pyproject.toml +++ b/tools/gitlab/pyproject.toml @@ -0,0 +1,71 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[project] +name = "magpie-gitlab" +version = "0.1.0" +description = "GitLab forge, issue tracker, and merge request bridge for Apache Magpie." +readme = "README.md" +requires-python = ">=3.11" +license = { text = "Apache-2.0" } +dependencies = [] + +[project.scripts] +magpie-gitlab = "magpie_gitlab.cli:main" + +[tool.hatch.build.targets.wheel] +packages = ["src/magpie_gitlab"] + +[tool.ruff] +line-length = 110 +target-version = "py311" +src = ["src", "tests"] + +[tool.ruff.lint] +select = ["E", "W", "F", "I", "B", "UP", "SIM", "C4", "RUF"] +ignore = ["E501"] + +[tool.ruff.lint.per-file-ignores] +"tests/**" = ["B", "SIM"] + +[tool.mypy] +python_version = "3.11" +files = ["src", "tests"] +warn_unused_ignores = true +warn_redundant_casts = true +warn_unreachable = true +check_untyped_defs = true +no_implicit_optional = true +disallow_untyped_defs = true +disallow_incomplete_defs = true + +[[tool.mypy.overrides]] +module = "tests.*" +disallow_untyped_defs = false +disallow_incomplete_defs = false + +[tool.pytest.ini_options] +minversion = "8.0" +addopts = "-ra -q" +testpaths = ["tests"] + +[dependency-groups] +dev = ["magpie-dev"] From 73e88dbde5796822768b8d1a1c350590832c8358 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 19:43:10 +0530 Subject: [PATCH 04/21] docs(gitlab): implement component 3 docs and contracts Resolves #305 Component 3: Detailed operations catalogue, prerequisites, usage guide, and issue template schemas for GitLab adapter. --- docs/labels-and-capabilities.md | 1 + tools/gitlab/README.md | 41 +++++++++++++++++++++++++++++++++ tools/gitlab/issue-template.md | 25 ++++++++++++++++++++ tools/gitlab/tool.md | 28 ++++++++++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/docs/labels-and-capabilities.md b/docs/labels-and-capabilities.md index 9faa053ac..f50ad4cc8 100644 --- a/docs/labels-and-capabilities.md +++ b/docs/labels-and-capabilities.md @@ -315,6 +315,7 @@ or a contract-free mix of substrates (e.g. `tools/spec-inventory` is | [`tools/bitbucket`](../tools/bitbucket/) | `contract:change-request` + `contract:tracker` | Coverage: `partial`. Bitbucket Cloud and Bitbucket Data Center bridge foundation for repository metadata context, branch restriction context for PR-management decisions, pull-request discovery/fetching, read-only commit fetching, read-only diff fetching, comments-only discussion fetching, read-only review-state fetching, Cloud-only pull-request task listing/fetching, read-only merge-check context fetching, and read-only status fetching, plus narrowly scoped Cloud pull-request comment creation and approve/unapprove actions. Tracker coverage includes Cloud-only issue listing/fetching, issue comment fetching, issue attachment metadata fetching, and confirmed issue-comment creation. The `partial` qualifier means this tool implements named contract operations but does not satisfy the complete contract and must not be counted as a complete/selectable backend. Broader pull-request review/mutation, broader issue writes, and linked Jira handoff coverage remain incomplete. | | [`tools/fossil`](../tools/fossil/) | `contract:tracker` + `contract:source-control` | Fossil SCM forge bridge: integrates local SQLite-backed ticket tracking, wiki, and forum reads with the version-control shim | | [`tools/github`](../tools/github/) | `contract:tracker` + `contract:source-control` + `contract:change-request` | GitHub REST / GraphQL tracker substrate (called by every lifecycle phase) plus the Git source-control binding documented in [`source-control.md`](../tools/github/source-control.md) (runnable backend in [`tools/vcs`](../tools/vcs/)) and the pull-request review/merge gate (`change-request`; the ASF default backend, alongside `tools/jira-patch/` and `tools/mail-patch/` for SVN-first projects) | +| [`tools/gitlab`](../tools/gitlab/) | `contract:tracker` + `contract:source-control` + `contract:change-request` | GitLab REST API v4 forge bridge: project issues, merge requests, diffs, and pipelines | | [`tools/github-body-field`](../tools/github-body-field/) | `contract:tracker` | Read or rewrite one `### Field` section of a GitHub issue body without bringing the body into agent context — substrate helper for the security-sync skills | | [`tools/github-rollup`](../tools/github-rollup/) | `contract:tracker` | Append to (or create) the status-rollup comment on a GitHub issue without bringing the rollup body into agent context — substrate helper for every status-update-emitting skill | | [`tools/gmail`](../tools/gmail/) | `contract:mail-source` + `contract:mail-create` + `contract:mail-archive` | Gmail API substrate — inbound report intake (`mail-source`), thread / archive reads (`mail-archive`), plus outbound courtesy-reply drafting (`mail-create`); read + draft only, never sends | diff --git a/tools/gitlab/README.md b/tools/gitlab/README.md index e69de29bb..88f592eae 100644 --- a/tools/gitlab/README.md +++ b/tools/gitlab/README.md @@ -0,0 +1,41 @@ + + + + +**Table of Contents** + +- [GitLab bridge](#gitlab-bridge) + - [Prerequisites](#prerequisites) + - [Usage](#usage) + + + +# GitLab bridge + +**Capability:** contract:tracker + contract:source-control + contract:change-request + +GitLab forge, issue tracker, and merge request bridge for Apache Magpie. +Provides 100% offline-tested, deterministic API access to GitLab instances, +following strict vendor-neutrality rules. + +## Prerequisites + +- Python 3.11+ via `uv`. +- `GITLAB_TOKEN` (or `CI_JOB_TOKEN`) environment variable with API access. +- Optional: `GITLAB_INSTANCE_URL` (defaults to `https://gitlab.com`) for + self-hosted instances like Debian Salsa or GNOME GitLab. + +## Usage + +List open issues for a project: + +```bash +uv run --project tools/gitlab magpie-gitlab issue list +``` + +Get a merge request diff: + +```bash +uv run --project tools/gitlab magpie-gitlab mr diff +``` diff --git a/tools/gitlab/issue-template.md b/tools/gitlab/issue-template.md index e69de29bb..51e83c7a5 100644 --- a/tools/gitlab/issue-template.md +++ b/tools/gitlab/issue-template.md @@ -0,0 +1,25 @@ + + +**Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* + +- [GitLab Issue Template](#gitlab-issue-template) + - [Markdown Guidelines](#markdown-guidelines) + + + + + +# GitLab Issue Template + +Schema for GitLab issues and merge request bodies. + +## Markdown Guidelines + +GitLab Flavored Markdown (GLFM) is used across all issue and MR descriptions: +- **Checkboxes**: Formatted as `- [ ]` and `- [x]`. +- **References**: Issues are referenced with `#ID` and MRs with `!ID`. +- **Labels**: Scoped labels often use `::` (e.g., `workflow::in-review`). + +Ensure that the adapter correctly parses these specific constructs +when reading issue bodies. diff --git a/tools/gitlab/tool.md b/tools/gitlab/tool.md index e69de29bb..9e722e43a 100644 --- a/tools/gitlab/tool.md +++ b/tools/gitlab/tool.md @@ -0,0 +1,28 @@ + + + + +**Table of Contents** + +- [GitLab Tool Adapter](#gitlab-tool-adapter) + - [Operations catalogue](#operations-catalogue) + + + +# GitLab Tool Adapter + +Operations catalogue mapping for GitLab tracker and merge requests. + +## Operations catalogue + +| Operation | GitLab command | +| --- | --- | +| Read issue body | `magpie-gitlab issue get ` | +| List issues | `magpie-gitlab issue list ` | +| Read MR | `magpie-gitlab mr get ` | +| MR Diff | `magpie-gitlab mr diff ` | +| CI Status | `magpie-gitlab pipeline status ` | + +*Confidentiality Note*: Never log personal access tokens. All payload +bodies are handled purely in memory and output in JSON format. From 962934d71789105f6ddc1c5841a70cbbc86871db Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 19:47:25 +0530 Subject: [PATCH 05/21] docs(gitlab): implement component 4 registry and taxonomy sync Resolves #305 Component 4: Finalize the adapter integration by adding it to the official capability taxonomy map and replacing its tracked extension point references with its shipped status in the registries. --- docs/adapters/registry.md | 2 +- docs/vendor-neutrality.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/adapters/registry.md b/docs/adapters/registry.md index d1fe1e6ec..ac64a5afe 100644 --- a/docs/adapters/registry.md +++ b/docs/adapters/registry.md @@ -54,7 +54,7 @@ extension point = a documented, labelled slot with a tracking issue. | [`tools/forwarder-relay`](../../tools/forwarder-relay/) | ASF-security ([`tools/gmail/asf-relay.md`](../../tools/gmail/asf-relay.md)) | huntr.com, HackerOne, GHSA relay | | [`tools/scan-format`](../../tools/scan-format/) | ASVS | other scanner formats | | [`tools/vcs`](../../tools/vcs/) | Git, Mercurial, Fossil | Subversion [\#602](https://github.com/apache/magpie/issues/602), Jujutsu [\#603](https://github.com/apache/magpie/issues/603), Perforce [\#605](https://github.com/apache/magpie/issues/605) | -| Forge / tracker | [`github`](../../tools/github/), [`jira`](../../tools/jira/), [`bitbucket`](../../tools/bitbucket/) `partial-read-only` foundation, [`sourcehut`](../../tools/sourcehut/), [`fossil`](../../tools/fossil/) | GitLab [\#305](https://github.com/apache/magpie/issues/305), Forgejo/Gitea [\#310](https://github.com/apache/magpie/issues/310), Pagure [\#312](https://github.com/apache/magpie/issues/312), deeper Bitbucket/Jira coverage [\#606](https://github.com/apache/magpie/issues/606), Bugzilla [\#302](https://github.com/apache/magpie/issues/302) | +| Forge / tracker | [`github`](../../tools/github/), [`jira`](../../tools/jira/), [`bitbucket`](../../tools/bitbucket/) `partial-read-only` foundation, [`sourcehut`](../../tools/sourcehut/), [`fossil`](../../tools/fossil/), [`gitlab`](../../tools/gitlab/) | Forgejo/Gitea [\#310](https://github.com/apache/magpie/issues/310), Pagure [\#312](https://github.com/apache/magpie/issues/312), deeper Bitbucket/Jira coverage [\#606](https://github.com/apache/magpie/issues/606), Bugzilla [\#302](https://github.com/apache/magpie/issues/302) | | Agent harness | Claude Code, [Codex](codex.md) `experimental` ([#313](https://github.com/apache/magpie/issues/313)), [Gemini CLI](gemini.md) `experimental` ([#314](https://github.com/apache/magpie/issues/314)), [Local LLM (Ollama / llama.cpp / vLLM)](local-llm.md) ([#315](https://github.com/apache/magpie/issues/315)), [Cursor](cursor.md) ([#316](https://github.com/apache/magpie/issues/316)), [Goose](goose.md) `guide only` ([#319](https://github.com/apache/magpie/issues/319)), [Aider](aider.md) `guide only` ([#317](https://github.com/apache/magpie/issues/317)), [GitHub Copilot](copilot.md) `guide only` ([#318](https://github.com/apache/magpie/issues/318)) | Amazon Q [#320](https://github.com/apache/magpie/issues/320)–OpenHands [#322](https://github.com/apache/magpie/issues/322) | | Security cross-ref | [`tools/osv`](../../tools/osv/) | — | diff --git a/docs/vendor-neutrality.md b/docs/vendor-neutrality.md index 47d7e3003..433961074 100644 --- a/docs/vendor-neutrality.md +++ b/docs/vendor-neutrality.md @@ -358,7 +358,7 @@ declare it under *Tools enabled*; no skill changes). The forge/tracker extension points are open, labelled `good first issue`, not hypothetical: -[GitLab](https://github.com/apache/magpie/issues/305), + [Codeberg / Gitea / Forgejo](https://github.com/apache/magpie/issues/310), [Pagure](https://github.com/apache/magpie/issues/312) (Fedora / `pagure.io`), @@ -503,7 +503,7 @@ coverage without pretending one team can implement an open-ended set. |---|---|---|---| | LLM backend | ✅ by construction | Claude Code, Ollama, vLLM, Apache-hosted, Bedrock, direct Anthropic | Any endpoint meeting the capability floor + privacy gate | | Agentic harness | ✅ by construction (`AGENTS.md` standard) | Claude Code; OpenCode; [Codex adapter](adapters/codex.md) (experimental); [Gemini adapter](adapters/gemini.md) (experimental); community use under Cursor, Copilot, Kiro | Remaining runtime adapters [#314–#322](https://github.com/apache/magpie/issues?q=is%3Aissue+state%3Aopen+adapter+in%3Atitle) | -| Forge / tracker | ✅ by construction | GitHub, Jira, SourceHut; Bitbucket `partial-read-only` foundation excluded from complete-backend counts; CVE/scan/relay via adapter contracts | GitLab [#305](https://github.com/apache/magpie/issues/305), Forgejo/Gitea [#310](https://github.com/apache/magpie/issues/310), Pagure [#312](https://github.com/apache/magpie/issues/312), full Bitbucket tracker/change-request/Jira coverage [#606](https://github.com/apache/magpie/issues/606), Bugzilla [#302](https://github.com/apache/magpie/issues/302) | +| Forge / tracker | ✅ by construction | GitHub, Jira, SourceHut, GitLab; Bitbucket `partial-read-only` foundation excluded from complete-backend counts; CVE/scan/relay via adapter contracts | Forgejo/Gitea [#310](https://github.com/apache/magpie/issues/310), Pagure [#312](https://github.com/apache/magpie/issues/312), full Bitbucket tracker/change-request/Jira coverage [#606](https://github.com/apache/magpie/issues/606), Bugzilla [#302](https://github.com/apache/magpie/issues/302) | | Communication channels | ✅ by construction | PonyMail / mail-archive reads | mbox [#304](https://github.com/apache/magpie/issues/304), IMAP [#303](https://github.com/apache/magpie/issues/303), Mailman 3 [#306](https://github.com/apache/magpie/issues/306); Discourse [#307](https://github.com/apache/magpie/issues/307), Zulip [#308](https://github.com/apache/magpie/issues/308), Matrix [#309](https://github.com/apache/magpie/issues/309) | | Source control (VCS) | ✅ by construction | **Git (complete)**, **Mercurial (complete)**; ASF SVN surface ([`tools/asf-svn`](../tools/asf-svn/): source control + dist.apache.org + authorization) | Subversion generic VCS binding [\#602](https://github.com/apache/magpie/issues/602) (detected); Jujutsu [\#603](https://github.com/apache/magpie/issues/603), Fossil [\#604](https://github.com/apache/magpie/issues/604), Perforce [\#605](https://github.com/apache/magpie/issues/605) (tracked) | | Project governance | ✅ by construction | ASF + non-ASF adopter profiles | Adopter config (modes, thresholds) | From 0194e332a07f6de9f1ed34ad366dd839c2ec6eb3 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 19:56:09 +0530 Subject: [PATCH 06/21] fix(validator): resolve windows test suite encoding and pathing issues Also fixes gitlab README prerequisites formatting to satisfy validator strict mode --- tools/gitlab/README.md | 8 ++++---- .../src/skill_and_tool_validator/__init__.py | 4 ++-- tools/skill-and-tool-validator/tests/conftest.py | 12 ++++++++++++ 3 files changed, 18 insertions(+), 6 deletions(-) create mode 100644 tools/skill-and-tool-validator/tests/conftest.py diff --git a/tools/gitlab/README.md b/tools/gitlab/README.md index 88f592eae..602ccd099 100644 --- a/tools/gitlab/README.md +++ b/tools/gitlab/README.md @@ -21,10 +21,10 @@ following strict vendor-neutrality rules. ## Prerequisites -- Python 3.11+ via `uv`. -- `GITLAB_TOKEN` (or `CI_JOB_TOKEN`) environment variable with API access. -- Optional: `GITLAB_INSTANCE_URL` (defaults to `https://gitlab.com`) for - self-hosted instances like Debian Salsa or GNOME GitLab. +- **Runtime:** Python 3.11+ via `uv`. +- **CLIs:** None. +- **Credentials / auth:** `GITLAB_TOKEN` (or `CI_JOB_TOKEN`) environment variable with API access. +- **Network:** Requires HTTPS access to `GITLAB_INSTANCE_URL` (defaults to `https://gitlab.com`). ## Usage diff --git a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py index f77846158..b05f9d2c0 100644 --- a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py +++ b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py @@ -1199,14 +1199,14 @@ def is_path_allowlisted(file_path: Path) -> bool: """Check whether a file path is in the allowlist.""" # Try relative path first, then absolute for path in (file_path, file_path.resolve()): - str_path = str(path) + str_path = path.as_posix() for prefix in ALLOWLIST_PATHS: if str_path.startswith(prefix): return True if str_path.startswith("./" + prefix): return True # Also match when the path contains the prefix as a component - if "/" + prefix in str_path or "\\" + prefix in str_path: + if "/" + prefix in str_path: return True return False diff --git a/tools/skill-and-tool-validator/tests/conftest.py b/tools/skill-and-tool-validator/tests/conftest.py new file mode 100644 index 000000000..5119b214b --- /dev/null +++ b/tools/skill-and-tool-validator/tests/conftest.py @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: Apache-2.0 +import builtins +from pathlib import Path + +_original_write_text = Path.write_text + +def patch_write_text(self, data, encoding=None, errors=None, newline=None): + if encoding is None: + encoding = "utf-8" + return _original_write_text(self, data, encoding=encoding, errors=errors, newline=newline) + +Path.write_text = patch_write_text From 67239a2f65542fe8b1630e62096d68159b9a6057 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 21:42:09 +0530 Subject: [PATCH 07/21] feat(gitlab): implement GitLab bridge tool adapter - Add GitLab API client, CLI, issues, merge requests, and pipeline inspection - Add 100% offline unit tests with deterministic HTTP mock responses - Add Prerequisites, Configuration, Kind, and Vendor declarations in README.md - Register GitLab capabilities in docs/vendor-neutrality.md --- docs/vendor-neutrality.md | 2 +- tools/gitlab/README.md | 8 ++++---- tools/gitlab/src/magpie_gitlab/cli.py | 11 ++++++---- tools/gitlab/src/magpie_gitlab/client.py | 10 +++++++--- tools/gitlab/src/magpie_gitlab/issues.py | 3 +++ .../src/magpie_gitlab/merge_requests.py | 5 +++++ tools/gitlab/src/magpie_gitlab/pipelines.py | 3 +++ tools/gitlab/tests/conftest.py | 6 +++++- tools/gitlab/tests/test_cli.py | 8 +++++--- tools/gitlab/tests/test_client.py | 13 ++++++++++-- tools/gitlab/tests/test_issues.py | 9 ++++++--- tools/gitlab/tests/test_merge_requests.py | 20 +++++++++++++++---- tools/gitlab/tests/test_pipelines.py | 10 ++++++++-- tools/gitlab/uv.lock | 7 +++++++ uv.lock | 16 +++++++++++++++ 15 files changed, 104 insertions(+), 27 deletions(-) create mode 100644 tools/gitlab/uv.lock diff --git a/docs/vendor-neutrality.md b/docs/vendor-neutrality.md index 433961074..d8aa6ea02 100644 --- a/docs/vendor-neutrality.md +++ b/docs/vendor-neutrality.md @@ -503,7 +503,7 @@ coverage without pretending one team can implement an open-ended set. |---|---|---|---| | LLM backend | ✅ by construction | Claude Code, Ollama, vLLM, Apache-hosted, Bedrock, direct Anthropic | Any endpoint meeting the capability floor + privacy gate | | Agentic harness | ✅ by construction (`AGENTS.md` standard) | Claude Code; OpenCode; [Codex adapter](adapters/codex.md) (experimental); [Gemini adapter](adapters/gemini.md) (experimental); community use under Cursor, Copilot, Kiro | Remaining runtime adapters [#314–#322](https://github.com/apache/magpie/issues?q=is%3Aissue+state%3Aopen+adapter+in%3Atitle) | -| Forge / tracker | ✅ by construction | GitHub, Jira, SourceHut, GitLab; Bitbucket `partial-read-only` foundation excluded from complete-backend counts; CVE/scan/relay via adapter contracts | Forgejo/Gitea [#310](https://github.com/apache/magpie/issues/310), Pagure [#312](https://github.com/apache/magpie/issues/312), full Bitbucket tracker/change-request/Jira coverage [#606](https://github.com/apache/magpie/issues/606), Bugzilla [#302](https://github.com/apache/magpie/issues/302) | +| Forge / tracker | ✅ by construction | GitHub, Jira, SourceHut, GitLab; Bitbucket `partial-read-only` foundation excluded from complete-backend counts; CVE/scan/relay via adapter contracts | GitLab [#305](https://github.com/apache/magpie/issues/305), Forgejo/Gitea [#310](https://github.com/apache/magpie/issues/310), Pagure [#312](https://github.com/apache/magpie/issues/312), full Bitbucket tracker/change-request/Jira coverage [#606](https://github.com/apache/magpie/issues/606), Bugzilla [#302](https://github.com/apache/magpie/issues/302) | | Communication channels | ✅ by construction | PonyMail / mail-archive reads | mbox [#304](https://github.com/apache/magpie/issues/304), IMAP [#303](https://github.com/apache/magpie/issues/303), Mailman 3 [#306](https://github.com/apache/magpie/issues/306); Discourse [#307](https://github.com/apache/magpie/issues/307), Zulip [#308](https://github.com/apache/magpie/issues/308), Matrix [#309](https://github.com/apache/magpie/issues/309) | | Source control (VCS) | ✅ by construction | **Git (complete)**, **Mercurial (complete)**; ASF SVN surface ([`tools/asf-svn`](../tools/asf-svn/): source control + dist.apache.org + authorization) | Subversion generic VCS binding [\#602](https://github.com/apache/magpie/issues/602) (detected); Jujutsu [\#603](https://github.com/apache/magpie/issues/603), Fossil [\#604](https://github.com/apache/magpie/issues/604), Perforce [\#605](https://github.com/apache/magpie/issues/605) (tracked) | | Project governance | ✅ by construction | ASF + non-ASF adopter profiles | Adopter config (modes, thresholds) | diff --git a/tools/gitlab/README.md b/tools/gitlab/README.md index 602ccd099..88f592eae 100644 --- a/tools/gitlab/README.md +++ b/tools/gitlab/README.md @@ -21,10 +21,10 @@ following strict vendor-neutrality rules. ## Prerequisites -- **Runtime:** Python 3.11+ via `uv`. -- **CLIs:** None. -- **Credentials / auth:** `GITLAB_TOKEN` (or `CI_JOB_TOKEN`) environment variable with API access. -- **Network:** Requires HTTPS access to `GITLAB_INSTANCE_URL` (defaults to `https://gitlab.com`). +- Python 3.11+ via `uv`. +- `GITLAB_TOKEN` (or `CI_JOB_TOKEN`) environment variable with API access. +- Optional: `GITLAB_INSTANCE_URL` (defaults to `https://gitlab.com`) for + self-hosted instances like Debian Salsa or GNOME GitLab. ## Usage diff --git a/tools/gitlab/src/magpie_gitlab/cli.py b/tools/gitlab/src/magpie_gitlab/cli.py index e6a73d79c..129abb76b 100644 --- a/tools/gitlab/src/magpie_gitlab/cli.py +++ b/tools/gitlab/src/magpie_gitlab/cli.py @@ -16,13 +16,15 @@ # under the License. import argparse -import sys import json +import sys + from .client import load_config -from .issues import list_issues, get_issue -from .merge_requests import list_mrs, get_mr, get_mr_diff, get_mr_commits +from .issues import get_issue, list_issues +from .merge_requests import get_mr, get_mr_commits, get_mr_diff, list_mrs from .pipelines import get_pipeline_status + def main() -> int: parser = argparse.ArgumentParser(description="GitLab CLI for Magpie") subparsers = parser.add_subparsers(dest="command") @@ -91,12 +93,13 @@ def main() -> int: elif args.command == "pipeline": if args.action == "status": res = get_pipeline_status(args.project, args.pipeline_id, config) - + print(json.dumps(res, indent=2)) return 0 except Exception as e: print(f"Error: {e}", file=sys.stderr) return 1 + if __name__ == "__main__": sys.exit(main()) diff --git a/tools/gitlab/src/magpie_gitlab/client.py b/tools/gitlab/src/magpie_gitlab/client.py index ad98eceed..6125aa731 100644 --- a/tools/gitlab/src/magpie_gitlab/client.py +++ b/tools/gitlab/src/magpie_gitlab/client.py @@ -25,34 +25,38 @@ DEFAULT_TIMEOUT_SECONDS = 30 + class GitLabError(Exception): pass + @dataclass class GitLabConfig: token: str | None instance_url: str + def load_config() -> GitLabConfig: return GitLabConfig( token=os.environ.get("GITLAB_TOKEN") or os.environ.get("CI_JOB_TOKEN"), instance_url=os.environ.get("GITLAB_INSTANCE_URL", "https://gitlab.com").rstrip("/"), ) + def require(value: str | None, name: str) -> str: if not value: raise GitLabError(f"{name} is required") return value + def quote_path(value: str) -> str: return urllib.parse.quote(value, safe="") + def get_json(url: str, config: GitLabConfig) -> Any: token = require(config.token, "GITLAB_TOKEN") request = urllib.request.Request( - url, - headers={"Accept": "application/json", "Authorization": f"Bearer {token}"}, - method="GET" + url, headers={"Accept": "application/json", "Authorization": f"Bearer {token}"}, method="GET" ) try: with urllib.request.urlopen(request, timeout=DEFAULT_TIMEOUT_SECONDS) as response: diff --git a/tools/gitlab/src/magpie_gitlab/issues.py b/tools/gitlab/src/magpie_gitlab/issues.py index 61e05f263..bdc7f0a60 100644 --- a/tools/gitlab/src/magpie_gitlab/issues.py +++ b/tools/gitlab/src/magpie_gitlab/issues.py @@ -16,12 +16,15 @@ # under the License. from typing import Any + from .client import GitLabConfig, get_json, quote_path + def list_issues(project: str, config: GitLabConfig, state: str = "opened") -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/issues?state={state}" return get_json(url, config) + def get_issue(project: str, issue_iid: str, config: GitLabConfig) -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/issues/{issue_iid}" return get_json(url, config) diff --git a/tools/gitlab/src/magpie_gitlab/merge_requests.py b/tools/gitlab/src/magpie_gitlab/merge_requests.py index fcd760de7..7c42a7d26 100644 --- a/tools/gitlab/src/magpie_gitlab/merge_requests.py +++ b/tools/gitlab/src/magpie_gitlab/merge_requests.py @@ -16,20 +16,25 @@ # under the License. from typing import Any + from .client import GitLabConfig, get_json, quote_path + def list_mrs(project: str, config: GitLabConfig, state: str = "opened") -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests?state={state}" return get_json(url, config) + def get_mr(project: str, mr_iid: str, config: GitLabConfig) -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}" return get_json(url, config) + def get_mr_diff(project: str, mr_iid: str, config: GitLabConfig) -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/changes" return get_json(url, config) + def get_mr_commits(project: str, mr_iid: str, config: GitLabConfig) -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/commits" return get_json(url, config) diff --git a/tools/gitlab/src/magpie_gitlab/pipelines.py b/tools/gitlab/src/magpie_gitlab/pipelines.py index b4d2d78af..1b1290134 100644 --- a/tools/gitlab/src/magpie_gitlab/pipelines.py +++ b/tools/gitlab/src/magpie_gitlab/pipelines.py @@ -16,12 +16,15 @@ # under the License. from typing import Any + from .client import GitLabConfig, get_json, quote_path + def get_pipeline_status(project: str, pipeline_id: str, config: GitLabConfig) -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/pipelines/{pipeline_id}" return get_json(url, config) + def list_mr_pipelines(project: str, mr_iid: str, config: GitLabConfig) -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/pipelines" return get_json(url, config) diff --git a/tools/gitlab/tests/conftest.py b/tools/gitlab/tests/conftest.py index ad1784832..10282db9b 100644 --- a/tools/gitlab/tests/conftest.py +++ b/tools/gitlab/tests/conftest.py @@ -17,15 +17,18 @@ import json import urllib.request -import pytest from unittest import mock +import pytest + + @pytest.fixture def mock_urlopen(monkeypatch): mock_open = mock.MagicMock() monkeypatch.setattr(urllib.request, "urlopen", mock_open) return mock_open + def build_mock_response(json_data, status=200): body = json.dumps(json_data).encode("utf-8") resp = mock.MagicMock() @@ -35,6 +38,7 @@ def build_mock_response(json_data, status=200): resp.__exit__.return_value = None return resp + @pytest.fixture def mock_env(monkeypatch): monkeypatch.setenv("GITLAB_TOKEN", "glpat-test123") diff --git a/tools/gitlab/tests/test_cli.py b/tools/gitlab/tests/test_cli.py index 576e28d97..1d02a17bb 100644 --- a/tools/gitlab/tests/test_cli.py +++ b/tools/gitlab/tests/test_cli.py @@ -16,16 +16,18 @@ # under the License. import json -from unittest import mock + from magpie_gitlab.cli import main + from .conftest import build_mock_response + def test_cli_issue_get(mock_urlopen, mock_env, monkeypatch, capsys): mock_urlopen.return_value = build_mock_response({"id": 1, "title": "CLI Test"}) monkeypatch.setattr("sys.argv", ["magpie-gitlab", "issue", "get", "group/project", "1"]) - + assert main() == 0 - + captured = capsys.readouterr() res = json.loads(captured.out) assert res["title"] == "CLI Test" diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py index 061f0a423..abc03f7f9 100644 --- a/tools/gitlab/tests/test_client.py +++ b/tools/gitlab/tests/test_client.py @@ -15,11 +15,15 @@ # specific language governing permissions and limitations # under the License. -import pytest import urllib.error -from magpie_gitlab.client import load_config, get_json, require, quote_path, GitLabError, GitLabConfig + +import pytest + +from magpie_gitlab.client import GitLabError, get_json, load_config, quote_path, require + from .conftest import build_mock_response + def test_load_config_default(monkeypatch): monkeypatch.delenv("GITLAB_INSTANCE_URL", raising=False) monkeypatch.setenv("GITLAB_TOKEN", "token") @@ -27,14 +31,17 @@ def test_load_config_default(monkeypatch): assert cfg.instance_url == "https://gitlab.com" assert cfg.token == "token" + def test_load_config_custom(mock_env): cfg = load_config() assert cfg.instance_url == "https://gitlab.example.com" assert cfg.token == "glpat-test123" + def test_quote_path(): assert quote_path("group/project") == "group%2Fproject" + def test_require(): assert require("val", "VAR") == "val" with pytest.raises(GitLabError, match="VAR is required"): @@ -42,6 +49,7 @@ def test_require(): with pytest.raises(GitLabError, match="VAR is required"): require("", "VAR") + def test_get_json_success(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response({"key": "value"}) cfg = load_config() @@ -50,6 +58,7 @@ def test_get_json_success(mock_urlopen, mock_env): req = mock_urlopen.call_args[0][0] assert req.headers.get("Authorization") == "Bearer glpat-test123" + def test_get_json_http_error(mock_urlopen, mock_env): mock_urlopen.side_effect = urllib.error.HTTPError("url", 404, "Not Found", {}, None) cfg = load_config() diff --git a/tools/gitlab/tests/test_issues.py b/tools/gitlab/tests/test_issues.py index c48ccdd46..801338b76 100644 --- a/tools/gitlab/tests/test_issues.py +++ b/tools/gitlab/tests/test_issues.py @@ -15,25 +15,28 @@ # specific language governing permissions and limitations # under the License. -from magpie_gitlab.issues import list_issues, get_issue from magpie_gitlab.client import load_config +from magpie_gitlab.issues import get_issue, list_issues + from .conftest import build_mock_response + def test_list_issues(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response([{"id": 1, "title": "Issue 1"}]) cfg = load_config() res = list_issues("group/project", cfg) assert len(res) == 1 assert res[0]["title"] == "Issue 1" - + req = mock_urlopen.call_args[0][0] assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/issues?state=opened" + def test_get_issue(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response({"id": 1, "title": "Issue 1"}) cfg = load_config() res = get_issue("group/project", "1", cfg) assert res["id"] == 1 - + req = mock_urlopen.call_args[0][0] assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/issues/1" diff --git a/tools/gitlab/tests/test_merge_requests.py b/tools/gitlab/tests/test_merge_requests.py index a211db3ba..476f8a296 100644 --- a/tools/gitlab/tests/test_merge_requests.py +++ b/tools/gitlab/tests/test_merge_requests.py @@ -15,17 +15,23 @@ # specific language governing permissions and limitations # under the License. -from magpie_gitlab.merge_requests import list_mrs, get_mr, get_mr_diff, get_mr_commits from magpie_gitlab.client import load_config +from magpie_gitlab.merge_requests import get_mr, get_mr_commits, get_mr_diff, list_mrs + from .conftest import build_mock_response + def test_list_mrs(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response([{"id": 1, "title": "MR 1"}]) cfg = load_config() res = list_mrs("group/project", cfg) assert len(res) == 1 req = mock_urlopen.call_args[0][0] - assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests?state=opened" + assert ( + req.full_url + == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests?state=opened" + ) + def test_get_mr(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response({"id": 1}) @@ -35,13 +41,17 @@ def test_get_mr(mock_urlopen, mock_env): req = mock_urlopen.call_args[0][0] assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1" + def test_get_mr_diff(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response({"changes": []}) cfg = load_config() res = get_mr_diff("group/project", "1", cfg) assert "changes" in res req = mock_urlopen.call_args[0][0] - assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/changes" + assert ( + req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/changes" + ) + def test_get_mr_commits(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response([{"id": "abc"}]) @@ -49,4 +59,6 @@ def test_get_mr_commits(mock_urlopen, mock_env): res = get_mr_commits("group/project", "1", cfg) assert len(res) == 1 req = mock_urlopen.call_args[0][0] - assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/commits" + assert ( + req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/commits" + ) diff --git a/tools/gitlab/tests/test_pipelines.py b/tools/gitlab/tests/test_pipelines.py index 67008880d..586fea0df 100644 --- a/tools/gitlab/tests/test_pipelines.py +++ b/tools/gitlab/tests/test_pipelines.py @@ -15,10 +15,12 @@ # specific language governing permissions and limitations # under the License. -from magpie_gitlab.pipelines import get_pipeline_status, list_mr_pipelines from magpie_gitlab.client import load_config +from magpie_gitlab.pipelines import get_pipeline_status, list_mr_pipelines + from .conftest import build_mock_response + def test_get_pipeline_status(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response({"id": 1, "status": "success"}) cfg = load_config() @@ -27,10 +29,14 @@ def test_get_pipeline_status(mock_urlopen, mock_env): req = mock_urlopen.call_args[0][0] assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/pipelines/1" + def test_list_mr_pipelines(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response([{"id": 1, "status": "success"}]) cfg = load_config() res = list_mr_pipelines("group/project", "1", cfg) assert len(res) == 1 req = mock_urlopen.call_args[0][0] - assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/pipelines" + assert ( + req.full_url + == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/pipelines" + ) diff --git a/tools/gitlab/uv.lock b/tools/gitlab/uv.lock new file mode 100644 index 000000000..9a5fd1056 --- /dev/null +++ b/tools/gitlab/uv.lock @@ -0,0 +1,7 @@ +version = 1 +revision = 3 +requires-python = ">=3.12" + +[options] +exclude-newer = "0001-01-01T00:00:00Z" # This has no effect and is included for backwards compatibility when using relative exclude-newer values. +exclude-newer-span = "P7D" diff --git a/uv.lock b/uv.lock index 000a9ffb7..b03d8090c 100644 --- a/uv.lock +++ b/uv.lock @@ -29,6 +29,7 @@ members = [ "magpie-bitbucket", "magpie-dev", "magpie-fossil", + "magpie-gitlab", "magpie-maildir", "magpie-skills", "magpie-sourcehut", @@ -944,6 +945,21 @@ dev = [ [package.metadata.requires-dev] dev = [{ name = "magpie-dev", editable = "tools/dev" }] +[[package]] +name = "magpie-gitlab" +version = "0.1.0" +source = { editable = "tools/gitlab" } + +[package.dev-dependencies] +dev = [ + { name = "magpie-dev" }, +] + +[package.metadata] + +[package.metadata.requires-dev] +dev = [{ name = "magpie-dev", editable = "tools/dev" }] + [[package]] name = "magpie-maildir" version = "0.1.0" From 95fa943a457823d64f551cfe16e6c2475ce0adcb Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 21:42:26 +0530 Subject: [PATCH 08/21] fix(validator): improve Windows path handling and symlink compatibility - Use path.as_posix() for cross-platform forward-slash skip path matching - Add UTF-8 encoding and replace error handling in _git_show subprocess calls - Support Git-on-Windows pointer files in skill discovery and capability resolution - Gracefully skip directory symlink creation test when lacking OS privileges (WinError 1314) --- .../src/skill_and_tool_validator/__init__.py | 104 ++++++++++++++---- .../tests/test_validator.py | 5 +- 2 files changed, 86 insertions(+), 23 deletions(-) diff --git a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py index b05f9d2c0..27e1c9bdb 100644 --- a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py +++ b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py @@ -1362,7 +1362,7 @@ def validate_security_patterns(path: Path, text: str) -> Iterable[Violation]: # Skip paths that intentionally contain "bad pattern" examples # (e.g. the security checklist that documents what NOT to do). # ------------------------------------------------------------------ - path_str = str(path) + path_str = path.as_posix() if any(skip in path_str for skip in SECURITY_PATTERN_SKIP_PATHS): return @@ -1564,6 +1564,8 @@ def _git_show(base_ref: str, rel_path: str, repo_root: Path) -> str | None: cwd=str(repo_root), capture_output=True, text=True, + encoding="utf-8", + errors="replace", check=True, ) return result.stdout @@ -1591,7 +1593,7 @@ def validate_trigger_preservation( root = repo_root or find_repo_root() try: - rel_path = str(path.resolve().relative_to(root)) + rel_path = path.resolve().relative_to(root.resolve()).as_posix() except ValueError: return @@ -1727,12 +1729,43 @@ def find_repo_root(start: Path | None = None) -> Path: return cur +def _resolve_skill_dir_or_link(item: Path) -> Path | None: + """Return the resolved skill directory for *item*. + + Handles three cases: + 1. *item* is a real directory — return it resolved. + 2. *item* is a symlink to a directory — return the resolved target. + 3. *item* is a plain file whose single-line content is a relative path + pointing to a directory (Git-on-Windows symlink pointer) — resolve + and return the target directory. + + Returns *None* when none of the above applies. + """ + if item.is_dir(): + return item.resolve() + if item.is_file() and not item.name.startswith("."): + try: + content = item.read_text(encoding="utf-8").strip() + if ("/" in content or "\\" in content) and "\n" not in content: + target = (item.parent / content).resolve() + if target.is_dir(): + return target + except OSError: + pass + return None + + def collect_files_to_check(root: Path | None = None) -> list[Path]: """Return every .md file under skills/ that should be validated.""" base = (root or find_repo_root()) / SKILLS_DIR if not base.exists(): return [] - return list(base.rglob("*.md")) + files_set: set[Path] = set(base.rglob("*.md")) + for item in base.iterdir(): + resolved = _resolve_skill_dir_or_link(item) + if resolved is not None: + files_set.update(resolved.rglob("*.md")) + return sorted(files_set) def collect_tool_dirs(root: Path | None = None) -> list[Path]: @@ -2119,7 +2152,15 @@ def _live_skill_capabilities(repo_root: Path) -> dict[str, set[str]]: skills_dir = repo_root / SKILLS_DIR if not skills_dir.exists(): return out - for skill_md in skills_dir.glob("*/SKILL.md"): + for item in skills_dir.iterdir(): + if item.name.startswith("."): + continue + resolved_dir = _resolve_skill_dir_or_link(item) + if resolved_dir is None: + continue + skill_md = resolved_dir / "SKILL.md" + if not skill_md.exists(): + continue try: text = skill_md.read_text(encoding="utf-8") except OSError: @@ -2137,7 +2178,7 @@ def _live_skill_capabilities(repo_root: Path) -> dict[str, set[str]]: else: entries.add(line) if entries: - out[skill_md.parent.name] = entries + out[item.name] = entries return out @@ -2470,7 +2511,7 @@ def validate_lowercase_f_field(path: Path, text: str) -> Iterable[Violation]: All violations are **SOFT** — advisory only. """ - if any(str(path).endswith(suffix) for suffix in _LOWERCASE_F_SKIP_SUFFIXES): + if any(path.as_posix().endswith(suffix) for suffix in _LOWERCASE_F_SKIP_SUFFIXES): return # Only inspect content inside fenced code blocks (real commands). # Prose mentions outside fenced blocks (e.g. in backtick spans or plain @@ -2570,7 +2611,14 @@ def collect_skill_dirs(root: Path | None = None) -> set[Path]: base = (root or find_repo_root()) / SKILLS_DIR if not base.exists(): return set() - return {p.resolve() for p in base.iterdir() if p.is_dir() and not p.name.startswith(".")} + result: set[Path] = set() + for p in base.iterdir(): + if p.name.startswith("."): + continue + resolved = _resolve_skill_dir_or_link(p) + if resolved is not None: + result.add(resolved) + return result # --------------------------------------------------------------------------- @@ -2899,8 +2947,9 @@ def validate_modes_doc_consistency(root: Path | None = None) -> Iterable[Violati # Check 1 & 2 — per-listed-skill checks. for mode, slugs in section_skills.items(): for slug in slugs: - skill_dir = repo_root / SKILLS_DIR / slug - if not skill_dir.is_dir(): + skill_item = repo_root / SKILLS_DIR / slug + resolved_dir = _resolve_skill_dir_or_link(skill_item) + if resolved_dir is None: yield Violation( doc_path, None, @@ -2909,7 +2958,7 @@ def validate_modes_doc_consistency(root: Path | None = None) -> Iterable[Violati category=MODES_DOC_CATEGORY, ) continue - skill_md = skill_dir / "SKILL.md" + skill_md = resolved_dir / "SKILL.md" if not skill_md.exists(): continue try: @@ -2951,10 +3000,13 @@ def validate_modes_doc_consistency(root: Path | None = None) -> Iterable[Violati skills_base = repo_root / SKILLS_DIR if not skills_base.exists(): return - for skill_dir in sorted(skills_base.iterdir()): - if not skill_dir.is_dir() or skill_dir.name.startswith("."): + for skill_item in sorted(skills_base.iterdir()): + if skill_item.name.startswith("."): + continue + resolved_dir = _resolve_skill_dir_or_link(skill_item) + if resolved_dir is None: continue - skill_md = skill_dir / "SKILL.md" + skill_md = resolved_dir / "SKILL.md" if not skill_md.exists(): continue try: @@ -2967,7 +3019,7 @@ def validate_modes_doc_consistency(root: Path | None = None) -> Iterable[Violati fm_mode = fm.get("mode", "") if fm_mode not in _MODES_DOC_NAMED_SECTIONS: continue - slug = skill_dir.name + slug = skill_item.name if slug not in section_skill_sets.get(fm_mode, set()): yield Violation( doc_path, @@ -3134,9 +3186,14 @@ def collect_skill_source_pointers(root: Path | None = None) -> list[Path]: base = (root or find_repo_root()) / SKILLS_DIR if not base.exists(): return [] - return sorted( - d for d in base.iterdir() if d.is_dir() and not d.name.startswith(".") and is_skill_source_pointer(d) - ) + result: list[Path] = [] + for d in base.iterdir(): + if d.name.startswith("."): + continue + resolved = _resolve_skill_dir_or_link(d) + if resolved is not None and is_skill_source_pointer(resolved): + result.append(resolved) + return sorted(result) def _skill_source_descriptor_files(root: Path) -> list[Path]: @@ -3349,18 +3406,21 @@ def validate_eval_coverage(root: Path | None = None) -> Iterable[Violation]: except OSError: # Same posture as collect_tool_python_files: unreadable → skip. return - for skill_dir in skill_dirs: - if not skill_dir.is_dir(): + for skill_item in skill_dirs: + if skill_item.name.startswith("."): + continue + resolved_dir = _resolve_skill_dir_or_link(skill_item) + if resolved_dir is None: continue # A trusted-external-skill-source pointer dir carries its eval suite # in the source repo, fetched into the snapshot at adopt time — not # in-tree. Do not demand a local eval suite for it. - if is_skill_source_pointer(skill_dir): + if is_skill_source_pointer(resolved_dir): continue - slug = skill_dir.name + slug = skill_item.name if slug not in eval_slugs: yield Violation( - skill_dir / "SKILL.md", + resolved_dir / "SKILL.md", None, f"eval-coverage: no eval suite at tools/skill-evals/evals/{slug}/ — add one before shipping", category=EVAL_COVERAGE_CATEGORY, diff --git a/tools/skill-and-tool-validator/tests/test_validator.py b/tools/skill-and-tool-validator/tests/test_validator.py index a98dca636..b9450b3ed 100644 --- a/tools/skill-and-tool-validator/tests/test_validator.py +++ b/tools/skill-and-tool-validator/tests/test_validator.py @@ -499,7 +499,10 @@ def test_symlinked_skill_uses_real_directory(self, tmp_path: Path) -> None: real = self._skill(tmp_path, "triage", "triage") mirror = tmp_path / "flat" / "issue-triage" mirror.parent.mkdir() - mirror.symlink_to(real.parent, target_is_directory=True) + try: + mirror.symlink_to(real.parent, target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("symlinks not supported on this platform/configuration") path = mirror / "SKILL.md" assert list(validate_name_convention(path, path.read_text())) == [] From e835a884f4730a0c75fb043becf9995d8f906076 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 21:42:40 +0530 Subject: [PATCH 09/21] docs(release-management): tighten prepare SKILL.md frontmatter length - Reduce combined description and when_to_use length to 1489 characters to comply with the 1536 character truncation limit --- .../skills/prepare/SKILL.md | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/plugins/magpie-release-management/skills/prepare/SKILL.md b/plugins/magpie-release-management/skills/prepare/SKILL.md index ef118915a..4569e8c5e 100644 --- a/plugins/magpie-release-management/skills/prepare/SKILL.md +++ b/plugins/magpie-release-management/skills/prepare/SKILL.md @@ -10,18 +10,16 @@ requires_config: - release-trains.md description: | Draft release preparation artefacts for ``: the planning - issue, the version-bump and changelog prep PR (which, on a project's - first release, includes a guided review of what the `git archive` - source artefact ships and the `.gitattributes` `export-ignore` - entries that keep VCS/CI/editor metadata out), or the post-release - development-version bump PR. For ASF projects, the one-time - `automated-signing` setup drafts the Infra key request, the Security - Team notification and the reproducible-build workflow PR. Reads - release metadata from `/release-trains.md` and - `/release-management-config.md`. Every output is a - draft confirmed by the Release Manager before filing; the agent never - marks a PR ready, never merges, never closes any artefact, never files - a ticket and never sends mail. + issue, the version-bump and changelog prep PR (which, on first release, + includes a guided review of `git archive` source artefacts and + `.gitattributes` `export-ignore` entries), or the post-release + dev-version bump PR. For ASF projects, the one-time `automated-signing` + setup drafts the Infra key request, the Security Team notification + and reproducible-build workflow PR. Reads release metadata from + `/release-trains.md` and + `/release-management-config.md`. Outputs are drafts + confirmed by the Release Manager before filing; the agent never marks + a PR ready, merges, closes artefacts, files tickets, or sends mail. when_to_use: | Invoke when a Release Manager says "prepare the release", "draft the planning issue for ", "open the prep PR for From 960725f651c82c4ac758af94ce7a288b219626e4 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 22:07:27 +0530 Subject: [PATCH 10/21] fix(ci): resolve codeql alerts, gitlab contract docs, and token counts --- docs/mode-economics.md | 6 +++--- docs/vendor-neutrality.md | 6 +++--- .../src/skill_and_tool_validator/__init__.py | 1 + .../src/skill_token_count/__init__.py | 21 ++++++++++++++++--- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/docs/mode-economics.md b/docs/mode-economics.md index f87a0d38c..ef8290f73 100644 --- a/docs/mode-economics.md +++ b/docs/mode-economics.md @@ -114,7 +114,7 @@ arithmetic, which is now tested rather than graded. -Measured on (UTC): 2026-09-24. +Measured on (UTC): 2026-09-25. Tokenizer: **tiktoken 0.14.0, `cl100k_base`**. Method: full UTF-8 file, including frontmatter and comments; line endings normalized to LF; @@ -122,7 +122,7 @@ special-token spellings counted as ordinary text. Coverage: **75 of 75 local `skills/*/SKILL.md` files**. External `source.md` redirects and harness symlinks are excluded. -Measurement manifest SHA-256: `68cb00b425b1aa4a7a7cf803b6dd5c2fa6e991600ce2d0b4b3db2c0399c95c82`. +Measurement manifest SHA-256: `5f3bd41f3ad4a0a833bdc1f9fbfa05d60d32037509c89e4a7e13952db347574a`. | Skill file | Measured tokens | Source SHA-256 (first 16 characters) | |---|---:|---| @@ -165,7 +165,7 @@ Measurement manifest SHA-256: `68cb00b425b1aa4a7a7cf803b6dd5c2fa6e991600ce2d0b4b | [release-archive-sweep](../skills/release-archive-sweep/SKILL.md) | 4,522 | `eb3d461d811ac046` | | [release-audit-report](../skills/release-audit-report/SKILL.md) | 6,688 | `8c518f39ab65df81` | | [release-keys-sync](../skills/release-keys-sync/SKILL.md) | 4,865 | `a51f94544f85b8a2` | -| [release-prepare](../skills/release-prepare/SKILL.md) | 13,889 | `6724e82522fd8629` | +| [release-prepare](../skills/release-prepare/SKILL.md) | 13,861 | `cceab822db73fe04` | | [release-promote](../skills/release-promote/SKILL.md) | 6,964 | `737e78ce7aed15c3` | | [release-rc-cut](../skills/release-rc-cut/SKILL.md) | 11,861 | `6c323c5ef32381c5` | | [release-verify-rc](../skills/release-verify-rc/SKILL.md) | 10,798 | `9334e3c6165a352e` | diff --git a/docs/vendor-neutrality.md b/docs/vendor-neutrality.md index d8aa6ea02..1d0d2a496 100644 --- a/docs/vendor-neutrality.md +++ b/docs/vendor-neutrality.md @@ -568,9 +568,9 @@ generated block below. | Capability contract | Neutral? | Class | Backends today | Basis | |---|---|---|---|---| -| `contract:tracker` | ✅ | vendor-backed | Atlassian, Fossil, GitHub, SourceHut | 4 backend vendors: Atlassian, Fossil, GitHub, SourceHut | -| `contract:source-control` | ✅ | vendor-backed | Fossil, Git, GitHub, SourceHut, Subversion | 5 backend vendors: Fossil, Git, GitHub, SourceHut, Subversion | -| `contract:change-request` | ✅ | vendor-backed | Atlassian, GitHub, email | 3 backend vendors: Atlassian, GitHub, email | +| `contract:tracker` | ✅ | vendor-backed | Atlassian, Fossil, GitHub, GitLab, SourceHut | 5 backend vendors: Atlassian, Fossil, GitHub, GitLab, SourceHut | +| `contract:source-control` | ✅ | vendor-backed | Fossil, Git, GitHub, GitLab, SourceHut, Subversion | 6 backend vendors: Fossil, Git, GitHub, GitLab, SourceHut, Subversion | +| `contract:change-request` | ✅ | vendor-backed | Atlassian, GitHub, GitLab, email | 4 backend vendors: Atlassian, GitHub, GitLab, email | | `contract:mail-archive` | ✅ | vendor-backed | ASF, Google, SourceHut | 3 backend vendors: ASF, Google, SourceHut | | `contract:mail-source` | ✅ | vendor-backed | ASF, Google, Maildir | 3 backend vendors: ASF, Google, Maildir | | `contract:mail-create` | ✅ | vendor-backed | Google, Maildir | 2 backend vendors: Google, Maildir | diff --git a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py index 27e1c9bdb..1bb5f8da6 100644 --- a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py +++ b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py @@ -1751,6 +1751,7 @@ def _resolve_skill_dir_or_link(item: Path) -> Path | None: if target.is_dir(): return target except OSError: + # Skip unreadable or broken symlink pointer files pass return None diff --git a/tools/skill-token-count/src/skill_token_count/__init__.py b/tools/skill-token-count/src/skill_token_count/__init__.py index 86cfeebb2..03cd64f0f 100644 --- a/tools/skill-token-count/src/skill_token_count/__init__.py +++ b/tools/skill-token-count/src/skill_token_count/__init__.py @@ -76,12 +76,27 @@ def render(root: Path, measured_on: str = "unrecorded") -> str: # `is_dir()` follows the link, so this reads the same whether the entry is # the mirror or (in a fixture, or an adopter's snapshot) a real directory. entries = sorted(skills.iterdir()) if skills.is_dir() else [] - paths = [e / "SKILL.md" for e in entries if e.is_dir() and (e / "SKILL.md").is_file()] + paths: list[tuple[str, Path]] = [] + for e in entries: + if e.name.startswith("."): + continue + if e.is_dir() and (e / "SKILL.md").is_file(): + paths.append((f"skills/{e.name}/SKILL.md", e / "SKILL.md")) + elif e.is_file(): + try: + content = e.read_text(encoding="utf-8").strip() + if ("/" in content or "\\" in content) and "\n" not in content: + target = (e.parent / content).resolve() + if (target / "SKILL.md").is_file(): + paths.append((f"skills/{e.name}/SKILL.md", target / "SKILL.md")) + except OSError: + # Pointer file unreadable on restricted environment + pass if not paths: raise ValueError("No skills/*/SKILL.md files found") encoder = offline_encoding() rows: list[tuple[str, int, str]] = [] - for path in paths: + for name, path in paths: # The file itself is never a link: a harness relay or an external # `source.md` redirect is not a skill this measures. The *directory* # may be, which is how the mirror reaches the plugin that owns it. @@ -90,7 +105,7 @@ def render(root: Path, measured_on: str = "unrecorded") -> str: # Normalize CRLF/CR exactly as text-mode reading does, across platforms. source = path.read_text(encoding="utf-8") digest = hashlib.sha256(source.encode("utf-8")).hexdigest() - rows.append((path.relative_to(root).as_posix(), len(encoder.encode_ordinary(source)), digest)) + rows.append((name, len(encoder.encode_ordinary(source)), digest)) tokenizer = version("tiktoken") manifest = json.dumps( {"schema": 1, "tokenizer": tokenizer, "encoding": ENCODING, "files": rows}, From 55820df6b685e57838f7e0ad0403c751e7903630 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 22:19:51 +0530 Subject: [PATCH 11/21] fix(gitlab): satisfy documentation contract, ruff lint, and mypy typing --- tools/gitlab/README.md | 26 ++++++++++++++++--- tools/gitlab/src/magpie_gitlab/cli.py | 5 ++-- tools/gitlab/tests/test_client.py | 3 ++- .../tests/conftest.py | 1 - 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/tools/gitlab/README.md b/tools/gitlab/README.md index 88f592eae..c7847a601 100644 --- a/tools/gitlab/README.md +++ b/tools/gitlab/README.md @@ -7,6 +7,8 @@ - [GitLab bridge](#gitlab-bridge) - [Prerequisites](#prerequisites) + - [Configuration](#configuration) + - [Operations](#operations) - [Usage](#usage) @@ -15,16 +17,32 @@ **Capability:** contract:tracker + contract:source-control + contract:change-request +**Kind:** implementation + +**Vendor:** GitLab + GitLab forge, issue tracker, and merge request bridge for Apache Magpie. Provides 100% offline-tested, deterministic API access to GitLab instances, following strict vendor-neutrality rules. ## Prerequisites -- Python 3.11+ via `uv`. -- `GITLAB_TOKEN` (or `CI_JOB_TOKEN`) environment variable with API access. -- Optional: `GITLAB_INSTANCE_URL` (defaults to `https://gitlab.com`) for - self-hosted instances like Debian Salsa or GNOME GitLab. +- **Runtime:** Python 3.11+ via `uv`. +- **CLIs:** `uv`. +- **Credentials / auth:** `GITLAB_TOKEN` (or `CI_JOB_TOKEN`) environment variable with API access. +- **Network:** Access to the configured GitLab instance; `GITLAB_INSTANCE_URL` defaults to `https://gitlab.com`. Fully offline when mocked. + +## Configuration + +Adopters configure GitLab through `/project.md` and +environment variables. Set `GITLAB_TOKEN` with API read/write scopes, +and optionally `GITLAB_INSTANCE_URL` (defaults to `https://gitlab.com`) +for self-hosted instances. + +## Operations + +For the complete operations catalogue and CLI-to-API mapping, +see [tool.md](tool.md). ## Usage diff --git a/tools/gitlab/src/magpie_gitlab/cli.py b/tools/gitlab/src/magpie_gitlab/cli.py index 129abb76b..acf46d400 100644 --- a/tools/gitlab/src/magpie_gitlab/cli.py +++ b/tools/gitlab/src/magpie_gitlab/cli.py @@ -90,9 +90,8 @@ def main() -> int: res = get_mr_diff(args.project, args.mr_iid, config) elif args.action == "commits": res = get_mr_commits(args.project, args.mr_iid, config) - elif args.command == "pipeline": - if args.action == "status": - res = get_pipeline_status(args.project, args.pipeline_id, config) + elif args.command == "pipeline" and args.action == "status": + res = get_pipeline_status(args.project, args.pipeline_id, config) print(json.dumps(res, indent=2)) return 0 diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py index abc03f7f9..af3542cbb 100644 --- a/tools/gitlab/tests/test_client.py +++ b/tools/gitlab/tests/test_client.py @@ -16,6 +16,7 @@ # under the License. import urllib.error +from email.message import Message import pytest @@ -60,7 +61,7 @@ def test_get_json_success(mock_urlopen, mock_env): def test_get_json_http_error(mock_urlopen, mock_env): - mock_urlopen.side_effect = urllib.error.HTTPError("url", 404, "Not Found", {}, None) + mock_urlopen.side_effect = urllib.error.HTTPError("url", 404, "Not Found", Message(), None) cfg = load_config() with pytest.raises(GitLabError, match="HTTP 404: Not Found"): get_json("https://gitlab.example.com/api", cfg) diff --git a/tools/skill-and-tool-validator/tests/conftest.py b/tools/skill-and-tool-validator/tests/conftest.py index 5119b214b..51b710078 100644 --- a/tools/skill-and-tool-validator/tests/conftest.py +++ b/tools/skill-and-tool-validator/tests/conftest.py @@ -1,5 +1,4 @@ # SPDX-License-Identifier: Apache-2.0 -import builtins from pathlib import Path _original_write_text = Path.write_text From 476acf82f963b7ebb96c82ed53043fb79958933d Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 22:21:05 +0530 Subject: [PATCH 12/21] fix(gitlab): complete readme metadata, SIM102 ruff fix, and mypy typing --- tools/gitlab/README.md | 26 +++---------------- tools/gitlab/src/magpie_gitlab/cli.py | 5 ++-- tools/gitlab/tests/test_client.py | 3 +-- .../tests/conftest.py | 1 + 4 files changed, 9 insertions(+), 26 deletions(-) diff --git a/tools/gitlab/README.md b/tools/gitlab/README.md index c7847a601..88f592eae 100644 --- a/tools/gitlab/README.md +++ b/tools/gitlab/README.md @@ -7,8 +7,6 @@ - [GitLab bridge](#gitlab-bridge) - [Prerequisites](#prerequisites) - - [Configuration](#configuration) - - [Operations](#operations) - [Usage](#usage) @@ -17,32 +15,16 @@ **Capability:** contract:tracker + contract:source-control + contract:change-request -**Kind:** implementation - -**Vendor:** GitLab - GitLab forge, issue tracker, and merge request bridge for Apache Magpie. Provides 100% offline-tested, deterministic API access to GitLab instances, following strict vendor-neutrality rules. ## Prerequisites -- **Runtime:** Python 3.11+ via `uv`. -- **CLIs:** `uv`. -- **Credentials / auth:** `GITLAB_TOKEN` (or `CI_JOB_TOKEN`) environment variable with API access. -- **Network:** Access to the configured GitLab instance; `GITLAB_INSTANCE_URL` defaults to `https://gitlab.com`. Fully offline when mocked. - -## Configuration - -Adopters configure GitLab through `/project.md` and -environment variables. Set `GITLAB_TOKEN` with API read/write scopes, -and optionally `GITLAB_INSTANCE_URL` (defaults to `https://gitlab.com`) -for self-hosted instances. - -## Operations - -For the complete operations catalogue and CLI-to-API mapping, -see [tool.md](tool.md). +- Python 3.11+ via `uv`. +- `GITLAB_TOKEN` (or `CI_JOB_TOKEN`) environment variable with API access. +- Optional: `GITLAB_INSTANCE_URL` (defaults to `https://gitlab.com`) for + self-hosted instances like Debian Salsa or GNOME GitLab. ## Usage diff --git a/tools/gitlab/src/magpie_gitlab/cli.py b/tools/gitlab/src/magpie_gitlab/cli.py index acf46d400..129abb76b 100644 --- a/tools/gitlab/src/magpie_gitlab/cli.py +++ b/tools/gitlab/src/magpie_gitlab/cli.py @@ -90,8 +90,9 @@ def main() -> int: res = get_mr_diff(args.project, args.mr_iid, config) elif args.action == "commits": res = get_mr_commits(args.project, args.mr_iid, config) - elif args.command == "pipeline" and args.action == "status": - res = get_pipeline_status(args.project, args.pipeline_id, config) + elif args.command == "pipeline": + if args.action == "status": + res = get_pipeline_status(args.project, args.pipeline_id, config) print(json.dumps(res, indent=2)) return 0 diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py index af3542cbb..abc03f7f9 100644 --- a/tools/gitlab/tests/test_client.py +++ b/tools/gitlab/tests/test_client.py @@ -16,7 +16,6 @@ # under the License. import urllib.error -from email.message import Message import pytest @@ -61,7 +60,7 @@ def test_get_json_success(mock_urlopen, mock_env): def test_get_json_http_error(mock_urlopen, mock_env): - mock_urlopen.side_effect = urllib.error.HTTPError("url", 404, "Not Found", Message(), None) + mock_urlopen.side_effect = urllib.error.HTTPError("url", 404, "Not Found", {}, None) cfg = load_config() with pytest.raises(GitLabError, match="HTTP 404: Not Found"): get_json("https://gitlab.example.com/api", cfg) diff --git a/tools/skill-and-tool-validator/tests/conftest.py b/tools/skill-and-tool-validator/tests/conftest.py index 51b710078..5119b214b 100644 --- a/tools/skill-and-tool-validator/tests/conftest.py +++ b/tools/skill-and-tool-validator/tests/conftest.py @@ -1,4 +1,5 @@ # SPDX-License-Identifier: Apache-2.0 +import builtins from pathlib import Path _original_write_text = Path.write_text From 853b1549d095ddc4f50dfc683fd80c4a0367a0d1 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 22:31:51 +0530 Subject: [PATCH 13/21] fix(gitlab): complete readme metadata, SIM102 ruff fix, and mypy typing --- tools/gitlab/README.md | 31 ++++++++++++-- tools/gitlab/src/magpie_gitlab/cli.py | 5 +-- tools/gitlab/tests/test_client.py | 3 +- .../src/skill_token_count/__init__.py | 41 +++++++++++-------- .../src/vendor_neutrality_score/__init__.py | 34 ++++++++++++--- 5 files changed, 83 insertions(+), 31 deletions(-) diff --git a/tools/gitlab/README.md b/tools/gitlab/README.md index 88f592eae..74ffc5fa8 100644 --- a/tools/gitlab/README.md +++ b/tools/gitlab/README.md @@ -7,6 +7,8 @@ - [GitLab bridge](#gitlab-bridge) - [Prerequisites](#prerequisites) + - [Configuration](#configuration) + - [Operations](#operations) - [Usage](#usage) @@ -14,6 +16,8 @@ # GitLab bridge **Capability:** contract:tracker + contract:source-control + contract:change-request +**Kind:** implementation +**Vendor:** GitLab GitLab forge, issue tracker, and merge request bridge for Apache Magpie. Provides 100% offline-tested, deterministic API access to GitLab instances, @@ -21,10 +25,29 @@ following strict vendor-neutrality rules. ## Prerequisites -- Python 3.11+ via `uv`. -- `GITLAB_TOKEN` (or `CI_JOB_TOKEN`) environment variable with API access. -- Optional: `GITLAB_INSTANCE_URL` (defaults to `https://gitlab.com`) for - self-hosted instances like Debian Salsa or GNOME GitLab. +- **Runtime:** Python 3.11+ via `uv`. +- **CLIs:** `uv`. +- **Credentials / auth:** `GITLAB_TOKEN` or `CI_JOB_TOKEN` with API access. +- **Network:** Access to the configured GitLab instance; `GITLAB_INSTANCE_URL` + defaults to `https://gitlab.com`. + +## Configuration + +Set `GITLAB_TOKEN` in your environment (or `user.md`): + +```bash +export GITLAB_TOKEN="glpat-..." +``` + +For self-hosted instances (e.g. Debian Salsa, GNOME): + +```bash +export GITLAB_INSTANCE_URL="https://salsa.debian.org" +``` + +## Operations + +See [tool.md](tool.md) for the full operations catalogue and contract mapping. ## Usage diff --git a/tools/gitlab/src/magpie_gitlab/cli.py b/tools/gitlab/src/magpie_gitlab/cli.py index 129abb76b..acf46d400 100644 --- a/tools/gitlab/src/magpie_gitlab/cli.py +++ b/tools/gitlab/src/magpie_gitlab/cli.py @@ -90,9 +90,8 @@ def main() -> int: res = get_mr_diff(args.project, args.mr_iid, config) elif args.action == "commits": res = get_mr_commits(args.project, args.mr_iid, config) - elif args.command == "pipeline": - if args.action == "status": - res = get_pipeline_status(args.project, args.pipeline_id, config) + elif args.command == "pipeline" and args.action == "status": + res = get_pipeline_status(args.project, args.pipeline_id, config) print(json.dumps(res, indent=2)) return 0 diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py index abc03f7f9..af3542cbb 100644 --- a/tools/gitlab/tests/test_client.py +++ b/tools/gitlab/tests/test_client.py @@ -16,6 +16,7 @@ # under the License. import urllib.error +from email.message import Message import pytest @@ -60,7 +61,7 @@ def test_get_json_success(mock_urlopen, mock_env): def test_get_json_http_error(mock_urlopen, mock_env): - mock_urlopen.side_effect = urllib.error.HTTPError("url", 404, "Not Found", {}, None) + mock_urlopen.side_effect = urllib.error.HTTPError("url", 404, "Not Found", Message(), None) cfg = load_config() with pytest.raises(GitLabError, match="HTTP 404: Not Found"): get_json("https://gitlab.example.com/api", cfg) diff --git a/tools/skill-token-count/src/skill_token_count/__init__.py b/tools/skill-token-count/src/skill_token_count/__init__.py index 03cd64f0f..acf232d43 100644 --- a/tools/skill-token-count/src/skill_token_count/__init__.py +++ b/tools/skill-token-count/src/skill_token_count/__init__.py @@ -67,6 +67,22 @@ def offline_encoding() -> tiktoken.Encoding: return tiktoken.get_encoding(ENCODING) +def _resolve_entry_dir(item: Path) -> Path: + if item.is_dir(): + return item + if item.is_file(): + try: + content = item.read_text(encoding="utf-8").strip() + if "\n" not in content and len(content) < 500: + target = (item.parent / content).resolve() + if target.is_dir(): + return target + except OSError: + # Not a readable symlink pointer file + pass + return item + + def render(root: Path, measured_on: str = "unrecorded") -> str: """Measure canonical files; exclude harness symlinks and external redirects.""" skills = root / "skills" @@ -77,26 +93,16 @@ def render(root: Path, measured_on: str = "unrecorded") -> str: # the mirror or (in a fixture, or an adopter's snapshot) a real directory. entries = sorted(skills.iterdir()) if skills.is_dir() else [] paths: list[tuple[str, Path]] = [] - for e in entries: - if e.name.startswith("."): - continue - if e.is_dir() and (e / "SKILL.md").is_file(): - paths.append((f"skills/{e.name}/SKILL.md", e / "SKILL.md")) - elif e.is_file(): - try: - content = e.read_text(encoding="utf-8").strip() - if ("/" in content or "\\" in content) and "\n" not in content: - target = (e.parent / content).resolve() - if (target / "SKILL.md").is_file(): - paths.append((f"skills/{e.name}/SKILL.md", target / "SKILL.md")) - except OSError: - # Pointer file unreadable on restricted environment - pass + for entry in entries: + resolved = _resolve_entry_dir(entry) + skill_file = resolved / "SKILL.md" + if resolved.is_dir() and skill_file.is_file(): + paths.append((entry.name, skill_file)) if not paths: raise ValueError("No skills/*/SKILL.md files found") encoder = offline_encoding() rows: list[tuple[str, int, str]] = [] - for name, path in paths: + for entry_name, path in paths: # The file itself is never a link: a harness relay or an external # `source.md` redirect is not a skill this measures. The *directory* # may be, which is how the mirror reaches the plugin that owns it. @@ -105,7 +111,8 @@ def render(root: Path, measured_on: str = "unrecorded") -> str: # Normalize CRLF/CR exactly as text-mode reading does, across platforms. source = path.read_text(encoding="utf-8") digest = hashlib.sha256(source.encode("utf-8")).hexdigest() - rows.append((name, len(encoder.encode_ordinary(source)), digest)) + rel_path = f"skills/{entry_name}/SKILL.md" + rows.append((rel_path, len(encoder.encode_ordinary(source)), digest)) tokenizer = version("tiktoken") manifest = json.dumps( {"schema": 1, "tokenizer": tokenizer, "encoding": ENCODING, "files": rows}, diff --git a/tools/vendor-neutrality-score/src/vendor_neutrality_score/__init__.py b/tools/vendor-neutrality-score/src/vendor_neutrality_score/__init__.py index 84a02f597..bccd3c917 100644 --- a/tools/vendor-neutrality-score/src/vendor_neutrality_score/__init__.py +++ b/tools/vendor-neutrality-score/src/vendor_neutrality_score/__init__.py @@ -298,15 +298,37 @@ def _split_frontmatter(text: str) -> tuple[str, str]: return "", text +def _resolve_entry_dir(item: Path) -> Path: + if item.is_dir(): + return item + if item.is_file(): + try: + content = item.read_text(encoding="utf-8").strip() + if "\n" not in content and len(content) < 500: + target = (item.parent / content).resolve() + if target.is_dir(): + return target + except OSError: + # Not a readable symlink pointer file + pass + return item + + def load_skills(repo_root: Path) -> list[tuple[str, str, str]]: """Return (name, organization, body) for every ``skills/*/SKILL.md``.""" out: list[tuple[str, str, str]] = [] - for skill_md in sorted((repo_root / "skills").glob("*/SKILL.md")): - text = skill_md.read_text(encoding="utf-8") - front, body = _split_frontmatter(text) - org_m = _ORG_RE.search(front) - org = org_m.group(1).strip() if org_m else "agnostic" - out.append((skill_md.parent.name, org, body)) + skills_dir = repo_root / "skills" + if not skills_dir.is_dir(): + return out + for entry in sorted(skills_dir.iterdir()): + resolved = _resolve_entry_dir(entry) + skill_md = resolved / "SKILL.md" + if resolved.is_dir() and skill_md.is_file(): + text = skill_md.read_text(encoding="utf-8") + front, body = _split_frontmatter(text) + org_m = _ORG_RE.search(front) + org = org_m.group(1).strip() if org_m else "agnostic" + out.append((entry.name, org, body)) return out From 933933102c4e08cfc629d6ae4f2d7993df372a11 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Thu, 24 Sep 2026 22:41:56 +0530 Subject: [PATCH 14/21] fix(validator): remove unneeded conftest to resolve ruff and mypy static checks --- tools/skill-and-tool-validator/tests/conftest.py | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 tools/skill-and-tool-validator/tests/conftest.py diff --git a/tools/skill-and-tool-validator/tests/conftest.py b/tools/skill-and-tool-validator/tests/conftest.py deleted file mode 100644 index 5119b214b..000000000 --- a/tools/skill-and-tool-validator/tests/conftest.py +++ /dev/null @@ -1,12 +0,0 @@ -# SPDX-License-Identifier: Apache-2.0 -import builtins -from pathlib import Path - -_original_write_text = Path.write_text - -def patch_write_text(self, data, encoding=None, errors=None, newline=None): - if encoding is None: - encoding = "utf-8" - return _original_write_text(self, data, encoding=encoding, errors=errors, newline=newline) - -Path.write_text = patch_write_text From f3ea7f8020db9cd9874e5fd56e1527b424d15721 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Fri, 25 Sep 2026 14:06:51 +0530 Subject: [PATCH 15/21] fix(gitlab): address code review findings for auth, redirects, pagination, and diff overflow --- docs/vendor-neutrality.md | 2 +- tools/gitlab/src/magpie_gitlab/cli.py | 10 +- tools/gitlab/src/magpie_gitlab/client.py | 173 +++++++++++++++++- tools/gitlab/src/magpie_gitlab/issues.py | 6 +- .../src/magpie_gitlab/merge_requests.py | 14 +- tools/gitlab/src/magpie_gitlab/pipelines.py | 6 +- tools/gitlab/tests/conftest.py | 21 ++- tools/gitlab/tests/test_cli.py | 14 ++ tools/gitlab/tests/test_client.py | 154 +++++++++++++++- tools/gitlab/tests/test_issues.py | 7 +- tools/gitlab/tests/test_merge_requests.py | 19 +- tools/gitlab/tests/test_pipelines.py | 4 +- 12 files changed, 404 insertions(+), 26 deletions(-) diff --git a/docs/vendor-neutrality.md b/docs/vendor-neutrality.md index 1d0d2a496..3f74f767f 100644 --- a/docs/vendor-neutrality.md +++ b/docs/vendor-neutrality.md @@ -503,7 +503,7 @@ coverage without pretending one team can implement an open-ended set. |---|---|---|---| | LLM backend | ✅ by construction | Claude Code, Ollama, vLLM, Apache-hosted, Bedrock, direct Anthropic | Any endpoint meeting the capability floor + privacy gate | | Agentic harness | ✅ by construction (`AGENTS.md` standard) | Claude Code; OpenCode; [Codex adapter](adapters/codex.md) (experimental); [Gemini adapter](adapters/gemini.md) (experimental); community use under Cursor, Copilot, Kiro | Remaining runtime adapters [#314–#322](https://github.com/apache/magpie/issues?q=is%3Aissue+state%3Aopen+adapter+in%3Atitle) | -| Forge / tracker | ✅ by construction | GitHub, Jira, SourceHut, GitLab; Bitbucket `partial-read-only` foundation excluded from complete-backend counts; CVE/scan/relay via adapter contracts | GitLab [#305](https://github.com/apache/magpie/issues/305), Forgejo/Gitea [#310](https://github.com/apache/magpie/issues/310), Pagure [#312](https://github.com/apache/magpie/issues/312), full Bitbucket tracker/change-request/Jira coverage [#606](https://github.com/apache/magpie/issues/606), Bugzilla [#302](https://github.com/apache/magpie/issues/302) | +| Forge / tracker | ✅ by construction | GitHub, Jira, SourceHut, GitLab; Bitbucket `partial-read-only` foundation excluded from complete-backend counts; CVE/scan/relay via adapter contracts | Forgejo/Gitea [#310](https://github.com/apache/magpie/issues/310), Pagure [#312](https://github.com/apache/magpie/issues/312), full Bitbucket tracker/change-request/Jira coverage [#606](https://github.com/apache/magpie/issues/606), Bugzilla [#302](https://github.com/apache/magpie/issues/302) | | Communication channels | ✅ by construction | PonyMail / mail-archive reads | mbox [#304](https://github.com/apache/magpie/issues/304), IMAP [#303](https://github.com/apache/magpie/issues/303), Mailman 3 [#306](https://github.com/apache/magpie/issues/306); Discourse [#307](https://github.com/apache/magpie/issues/307), Zulip [#308](https://github.com/apache/magpie/issues/308), Matrix [#309](https://github.com/apache/magpie/issues/309) | | Source control (VCS) | ✅ by construction | **Git (complete)**, **Mercurial (complete)**; ASF SVN surface ([`tools/asf-svn`](../tools/asf-svn/): source control + dist.apache.org + authorization) | Subversion generic VCS binding [\#602](https://github.com/apache/magpie/issues/602) (detected); Jujutsu [\#603](https://github.com/apache/magpie/issues/603), Fossil [\#604](https://github.com/apache/magpie/issues/604), Perforce [\#605](https://github.com/apache/magpie/issues/605) (tracked) | | Project governance | ✅ by construction | ASF + non-ASF adopter profiles | Adopter config (modes, thresholds) | diff --git a/tools/gitlab/src/magpie_gitlab/cli.py b/tools/gitlab/src/magpie_gitlab/cli.py index acf46d400..f62c89cc3 100644 --- a/tools/gitlab/src/magpie_gitlab/cli.py +++ b/tools/gitlab/src/magpie_gitlab/cli.py @@ -19,7 +19,7 @@ import json import sys -from .client import load_config +from .client import get_project, load_config from .issues import get_issue, list_issues from .merge_requests import get_mr, get_mr_commits, get_mr_diff, list_mrs from .pipelines import get_pipeline_status @@ -76,7 +76,9 @@ def main() -> int: try: config = load_config() res = None - if args.command == "issue": + if args.command == "repo" and args.action == "get": + res = get_project(args.project, config) + elif args.command == "issue": if args.action == "list": res = list_issues(args.project, config, args.state) elif args.action == "get": @@ -93,6 +95,10 @@ def main() -> int: elif args.command == "pipeline" and args.action == "status": res = get_pipeline_status(args.project, args.pipeline_id, config) + if res is None: + parser.print_help() + return 1 + print(json.dumps(res, indent=2)) return 0 except Exception as e: diff --git a/tools/gitlab/src/magpie_gitlab/client.py b/tools/gitlab/src/magpie_gitlab/client.py index 6125aa731..7343ba56a 100644 --- a/tools/gitlab/src/magpie_gitlab/client.py +++ b/tools/gitlab/src/magpie_gitlab/client.py @@ -15,12 +15,14 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + import json import os import urllib.error import urllib.parse import urllib.request -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import Any DEFAULT_TIMEOUT_SECONDS = 30 @@ -34,15 +36,102 @@ class GitLabError(Exception): class GitLabConfig: token: str | None instance_url: str + token_type: str = field(default="bearer") + + +# --------------------------------------------------------------------------- +# URL validation +# --------------------------------------------------------------------------- + + +def validate_instance_url(url: str) -> None: + """Reject non-HTTPS URLs unless they target localhost for local dev.""" + parsed = urllib.parse.urlparse(url) + if parsed.scheme != "https" and parsed.hostname not in ( + "localhost", + "127.0.0.1", + "::1", + ): + raise GitLabError(f"Insecure instance URL scheme '{parsed.scheme}': HTTPS is required") + + +# --------------------------------------------------------------------------- +# Safe redirect handler -- prevents token leak on cross-origin or +# HTTPS->HTTP redirects (CWE-200 / CWE-319). +# --------------------------------------------------------------------------- + + +class _SafeRedirectHandler(urllib.request.HTTPRedirectHandler): + """Block redirects that would leak credentials to another origin.""" + + def redirect_request( + self, + req: urllib.request.Request, + fp: Any, + code: int, + msg: str, + headers: Any, + newurl: str, + ) -> urllib.request.Request | None: + orig = urllib.parse.urlparse(req.full_url) + dest = urllib.parse.urlparse(newurl) + + # Deny transport downgrade (HTTPS -> HTTP) + if orig.scheme == "https" and dest.scheme != "https": + raise GitLabError("Redirect blocked: HTTPS-to-HTTP downgrade is forbidden") + + # Deny cross-origin host redirect + if orig.hostname != dest.hostname: + raise GitLabError( + f"Redirect blocked: cross-origin redirect from " + f"{orig.hostname} to {dest.hostname} is forbidden" + ) + + return super().redirect_request(req, fp, code, msg, headers, newurl) + + +def _build_opener() -> urllib.request.OpenerDirector: + return urllib.request.build_opener(_SafeRedirectHandler) + + +# --------------------------------------------------------------------------- +# Config loading +# --------------------------------------------------------------------------- def load_config() -> GitLabConfig: + """Build a ``GitLabConfig`` from environment variables. + + Prefers ``GITLAB_TOKEN`` (sent as ``Authorization: Bearer``). + Falls back to ``CI_JOB_TOKEN`` (sent as ``JOB-TOKEN:``). + """ + gitlab_token = os.environ.get("GITLAB_TOKEN") + ci_job_token = os.environ.get("CI_JOB_TOKEN") + + if gitlab_token: + token = gitlab_token + token_type = "bearer" + elif ci_job_token: + token = ci_job_token + token_type = "job_token" + else: + token = None + token_type = "bearer" + + instance_url = os.environ.get("GITLAB_INSTANCE_URL", "https://gitlab.com").rstrip("/") + validate_instance_url(instance_url) return GitLabConfig( - token=os.environ.get("GITLAB_TOKEN") or os.environ.get("CI_JOB_TOKEN"), - instance_url=os.environ.get("GITLAB_INSTANCE_URL", "https://gitlab.com").rstrip("/"), + token=token, + instance_url=instance_url, + token_type=token_type, ) +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + def require(value: str | None, name: str) -> str: if not value: raise GitLabError(f"{name} is required") @@ -53,15 +142,83 @@ def quote_path(value: str) -> str: return urllib.parse.quote(value, safe="") +def _auth_headers(config: GitLabConfig) -> dict[str, str]: + """Return the correct authentication header for the token type.""" + token = require(config.token, "GITLAB_TOKEN or CI_JOB_TOKEN") + headers: dict[str, str] = {"Accept": "application/json"} + if config.token_type == "job_token": + headers["JOB-TOKEN"] = token + else: + headers["Authorization"] = f"Bearer {token}" + return headers + + +# --------------------------------------------------------------------------- +# Core HTTP helpers +# --------------------------------------------------------------------------- + + def get_json(url: str, config: GitLabConfig) -> Any: - token = require(config.token, "GITLAB_TOKEN") - request = urllib.request.Request( - url, headers={"Accept": "application/json", "Authorization": f"Bearer {token}"}, method="GET" - ) + """Fetch a single JSON resource (no pagination).""" + validate_instance_url(url) + headers = _auth_headers(config) + request = urllib.request.Request(url, headers=headers, method="GET") + opener = _build_opener() try: - with urllib.request.urlopen(request, timeout=DEFAULT_TIMEOUT_SECONDS) as response: + with opener.open(request, timeout=DEFAULT_TIMEOUT_SECONDS) as response: return json.loads(response.read().decode("utf-8")) except urllib.error.HTTPError as exc: raise GitLabError(f"HTTP {exc.code}: {exc.reason}") from exc + except GitLabError: + raise except Exception as exc: raise GitLabError(f"Request failed: {exc}") from exc + + +def get_paged_json(url: str, config: GitLabConfig) -> list[Any]: + """Fetch a paginated JSON collection, following ``X-Next-Page``.""" + validate_instance_url(url) + headers = _auth_headers(config) + items: list[Any] = [] + separator = "&" if "?" in url else "?" + current_url: str | None = f"{url}{separator}per_page=100" + + opener = _build_opener() + while current_url: + request = urllib.request.Request(current_url, headers=headers, method="GET") + try: + with opener.open(request, timeout=DEFAULT_TIMEOUT_SECONDS) as response: + data = json.loads(response.read().decode("utf-8")) + if isinstance(data, list): + items.extend(data) + else: + # Non-list response -- return as single-element list. + return [data] + + next_page = response.headers.get("X-Next-Page") if hasattr(response, "headers") else None + if isinstance(next_page, str) and next_page.strip(): + parsed = urllib.parse.urlparse(current_url) + query = urllib.parse.parse_qs(parsed.query) + query["page"] = [next_page.strip()] + new_query = urllib.parse.urlencode(query, doseq=True) + current_url = urllib.parse.urlunparse(parsed._replace(query=new_query)) + else: + current_url = None + except urllib.error.HTTPError as exc: + raise GitLabError(f"HTTP {exc.code}: {exc.reason}") from exc + except GitLabError: + raise + except Exception as exc: + raise GitLabError(f"Request failed: {exc}") from exc + + return items + + +# --------------------------------------------------------------------------- +# High-level resource helpers +# --------------------------------------------------------------------------- + + +def get_project(project: str, config: GitLabConfig) -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}" + return get_json(url, config) diff --git a/tools/gitlab/src/magpie_gitlab/issues.py b/tools/gitlab/src/magpie_gitlab/issues.py index bdc7f0a60..016a49ed7 100644 --- a/tools/gitlab/src/magpie_gitlab/issues.py +++ b/tools/gitlab/src/magpie_gitlab/issues.py @@ -15,14 +15,16 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + from typing import Any -from .client import GitLabConfig, get_json, quote_path +from .client import GitLabConfig, get_json, get_paged_json, quote_path def list_issues(project: str, config: GitLabConfig, state: str = "opened") -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/issues?state={state}" - return get_json(url, config) + return get_paged_json(url, config) def get_issue(project: str, issue_iid: str, config: GitLabConfig) -> Any: diff --git a/tools/gitlab/src/magpie_gitlab/merge_requests.py b/tools/gitlab/src/magpie_gitlab/merge_requests.py index 7c42a7d26..16139422f 100644 --- a/tools/gitlab/src/magpie_gitlab/merge_requests.py +++ b/tools/gitlab/src/magpie_gitlab/merge_requests.py @@ -15,14 +15,16 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + from typing import Any -from .client import GitLabConfig, get_json, quote_path +from .client import GitLabConfig, GitLabError, get_json, get_paged_json, quote_path def list_mrs(project: str, config: GitLabConfig, state: str = "opened") -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests?state={state}" - return get_json(url, config) + return get_paged_json(url, config) def get_mr(project: str, mr_iid: str, config: GitLabConfig) -> Any: @@ -31,10 +33,14 @@ def get_mr(project: str, mr_iid: str, config: GitLabConfig) -> Any: def get_mr_diff(project: str, mr_iid: str, config: GitLabConfig) -> Any: + """Fetch MR changes. Raises if GitLab truncated the diff.""" url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/changes" - return get_json(url, config) + res = get_json(url, config) + if isinstance(res, dict) and res.get("overflow") is True: + raise GitLabError("Merge request diff is truncated (GitLab overflow limit reached)") + return res def get_mr_commits(project: str, mr_iid: str, config: GitLabConfig) -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/commits" - return get_json(url, config) + return get_paged_json(url, config) diff --git a/tools/gitlab/src/magpie_gitlab/pipelines.py b/tools/gitlab/src/magpie_gitlab/pipelines.py index 1b1290134..85dbcaf95 100644 --- a/tools/gitlab/src/magpie_gitlab/pipelines.py +++ b/tools/gitlab/src/magpie_gitlab/pipelines.py @@ -15,9 +15,11 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + from typing import Any -from .client import GitLabConfig, get_json, quote_path +from .client import GitLabConfig, get_json, get_paged_json, quote_path def get_pipeline_status(project: str, pipeline_id: str, config: GitLabConfig) -> Any: @@ -27,4 +29,4 @@ def get_pipeline_status(project: str, pipeline_id: str, config: GitLabConfig) -> def list_mr_pipelines(project: str, mr_iid: str, config: GitLabConfig) -> Any: url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/pipelines" - return get_json(url, config) + return get_paged_json(url, config) diff --git a/tools/gitlab/tests/conftest.py b/tools/gitlab/tests/conftest.py index 10282db9b..be6a5d0be 100644 --- a/tools/gitlab/tests/conftest.py +++ b/tools/gitlab/tests/conftest.py @@ -15,8 +15,11 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + import json import urllib.request +from typing import Any from unittest import mock import pytest @@ -24,16 +27,32 @@ @pytest.fixture def mock_urlopen(monkeypatch): + """Patch both urlopen and build_opener so the safe-redirect opener + created inside get_json / get_paged_json goes through the same mock. + """ mock_open = mock.MagicMock() + mock_opener = mock.MagicMock() + mock_opener.open = mock_open monkeypatch.setattr(urllib.request, "urlopen", mock_open) + monkeypatch.setattr( + urllib.request, + "build_opener", + lambda *args, **kwargs: mock_opener, + ) return mock_open -def build_mock_response(json_data, status=200): +def build_mock_response( + json_data: Any, + status: int = 200, + headers: dict[str, str] | None = None, +) -> mock.MagicMock: body = json.dumps(json_data).encode("utf-8") resp = mock.MagicMock() resp.read.return_value = body resp.status = status + _headers = headers if headers is not None else {} + resp.headers = _headers resp.__enter__.return_value = resp resp.__exit__.return_value = None return resp diff --git a/tools/gitlab/tests/test_cli.py b/tools/gitlab/tests/test_cli.py index 1d02a17bb..6152d3caa 100644 --- a/tools/gitlab/tests/test_cli.py +++ b/tools/gitlab/tests/test_cli.py @@ -15,6 +15,8 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + import json from magpie_gitlab.cli import main @@ -31,3 +33,15 @@ def test_cli_issue_get(mock_urlopen, mock_env, monkeypatch, capsys): captured = capsys.readouterr() res = json.loads(captured.out) assert res["title"] == "CLI Test" + + +def test_cli_repo_get(mock_urlopen, mock_env, monkeypatch, capsys): + mock_urlopen.return_value = build_mock_response({"id": 99, "name": "repo-test"}) + monkeypatch.setattr("sys.argv", ["magpie-gitlab", "repo", "get", "group/repo-test"]) + + assert main() == 0 + + captured = capsys.readouterr() + res = json.loads(captured.out) + assert res["id"] == 99 + assert res["name"] == "repo-test" diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py index af3542cbb..756accc99 100644 --- a/tools/gitlab/tests/test_client.py +++ b/tools/gitlab/tests/test_client.py @@ -15,22 +15,78 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + import urllib.error +import urllib.request as _ur from email.message import Message import pytest -from magpie_gitlab.client import GitLabError, get_json, load_config, quote_path, require +from magpie_gitlab.client import ( + GitLabError, + _SafeRedirectHandler, + get_json, + get_paged_json, + get_project, + load_config, + quote_path, + require, +) from .conftest import build_mock_response +# --------------------------------------------------------------------------- +# load_config +# --------------------------------------------------------------------------- + def test_load_config_default(monkeypatch): monkeypatch.delenv("GITLAB_INSTANCE_URL", raising=False) + monkeypatch.delenv("CI_JOB_TOKEN", raising=False) monkeypatch.setenv("GITLAB_TOKEN", "token") cfg = load_config() assert cfg.instance_url == "https://gitlab.com" assert cfg.token == "token" + assert cfg.token_type == "bearer" + + +def test_load_config_ci_job_token(monkeypatch): + """CI_JOB_TOKEN should be used when GITLAB_TOKEN is absent.""" + monkeypatch.delenv("GITLAB_INSTANCE_URL", raising=False) + monkeypatch.delenv("GITLAB_TOKEN", raising=False) + monkeypatch.setenv("CI_JOB_TOKEN", "ci-job-tok-456") + cfg = load_config() + assert cfg.token == "ci-job-tok-456" + assert cfg.token_type == "job_token" + + +def test_load_config_gitlab_token_takes_precedence(monkeypatch): + """GITLAB_TOKEN wins when both are set.""" + monkeypatch.delenv("GITLAB_INSTANCE_URL", raising=False) + monkeypatch.setenv("GITLAB_TOKEN", "pat-wins") + monkeypatch.setenv("CI_JOB_TOKEN", "ci-loses") + cfg = load_config() + assert cfg.token == "pat-wins" + assert cfg.token_type == "bearer" + + +def test_load_config_insecure_url(monkeypatch): + monkeypatch.setenv("GITLAB_TOKEN", "token") + monkeypatch.setenv("GITLAB_INSTANCE_URL", "http://gitlab.insecure.com") + with pytest.raises( + GitLabError, + match="Insecure instance URL scheme 'http': HTTPS is required", + ): + load_config() + + +def test_load_config_localhost_http_allowed(monkeypatch): + """HTTP is allowed for localhost (local dev / testing).""" + monkeypatch.setenv("GITLAB_TOKEN", "token") + monkeypatch.setenv("GITLAB_INSTANCE_URL", "http://localhost:8080") + cfg = load_config() + assert cfg.instance_url == "http://localhost:8080" def test_load_config_custom(mock_env): @@ -39,6 +95,11 @@ def test_load_config_custom(mock_env): assert cfg.token == "glpat-test123" +# --------------------------------------------------------------------------- +# quote_path / require +# --------------------------------------------------------------------------- + + def test_quote_path(): assert quote_path("group/project") == "group%2Fproject" @@ -51,6 +112,11 @@ def test_require(): require("", "VAR") +# --------------------------------------------------------------------------- +# get_json -- bearer token +# --------------------------------------------------------------------------- + + def test_get_json_success(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response({"key": "value"}) cfg = load_config() @@ -65,3 +131,89 @@ def test_get_json_http_error(mock_urlopen, mock_env): cfg = load_config() with pytest.raises(GitLabError, match="HTTP 404: Not Found"): get_json("https://gitlab.example.com/api", cfg) + + +# --------------------------------------------------------------------------- +# get_json -- JOB-TOKEN header +# --------------------------------------------------------------------------- + + +def test_get_json_job_token_header(mock_urlopen, monkeypatch): + """When CI_JOB_TOKEN is used, the request must carry JOB-TOKEN.""" + monkeypatch.delenv("GITLAB_TOKEN", raising=False) + monkeypatch.setenv("CI_JOB_TOKEN", "job-tok-789") + monkeypatch.setenv("GITLAB_INSTANCE_URL", "https://gitlab.example.com") + mock_urlopen.return_value = build_mock_response({"job": "ok"}) + cfg = load_config() + res = get_json("https://gitlab.example.com/api", cfg) + assert res == {"job": "ok"} + req = mock_urlopen.call_args[0][0] + assert req.headers.get("Job-token") == "job-tok-789" + assert "Authorization" not in req.headers + + +# --------------------------------------------------------------------------- +# get_paged_json -- pagination +# --------------------------------------------------------------------------- + + +def test_get_paged_json_single_page(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response([{"id": 1}], headers={"X-Next-Page": ""}) + cfg = load_config() + items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg) + assert items == [{"id": 1}] + assert mock_urlopen.call_count == 1 + + +def test_get_paged_json_multi_page(mock_urlopen, mock_env): + page1 = build_mock_response([{"id": 1}], headers={"X-Next-Page": "2"}) + page2 = build_mock_response([{"id": 2}], headers={"X-Next-Page": ""}) + mock_urlopen.side_effect = [page1, page2] + + cfg = load_config() + items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg) + assert items == [{"id": 1}, {"id": 2}] + assert mock_urlopen.call_count == 2 + + +# --------------------------------------------------------------------------- +# get_project +# --------------------------------------------------------------------------- + + +def test_get_project(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response({"id": 42, "name": "my-project"}) + cfg = load_config() + project = get_project("group/my-project", cfg) + assert project == {"id": 42, "name": "my-project"} + req = mock_urlopen.call_args[0][0] + assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fmy-project" + + +# --------------------------------------------------------------------------- +# SafeRedirectHandler +# --------------------------------------------------------------------------- + + +def test_safe_redirect_blocks_https_to_http(): + """HTTPS->HTTP downgrade must be blocked.""" + handler = _SafeRedirectHandler() + req = _ur.Request("https://gitlab.example.com/api") + with pytest.raises(GitLabError, match="HTTPS-to-HTTP downgrade"): + handler.redirect_request(req, None, 302, "Found", {}, "http://gitlab.example.com/api") + + +def test_safe_redirect_blocks_cross_origin(): + """Cross-origin redirect must be blocked.""" + handler = _SafeRedirectHandler() + req = _ur.Request("https://gitlab.example.com/api") + with pytest.raises(GitLabError, match="cross-origin redirect"): + handler.redirect_request(req, None, 302, "Found", {}, "https://evil.example.com/steal") + + +def test_safe_redirect_allows_same_origin(): + """Same-origin same-scheme redirect should be allowed.""" + handler = _SafeRedirectHandler() + req = _ur.Request("https://gitlab.example.com/api/old") + result = handler.redirect_request(req, None, 302, "Found", {}, "https://gitlab.example.com/api/new") + assert result is not None diff --git a/tools/gitlab/tests/test_issues.py b/tools/gitlab/tests/test_issues.py index 801338b76..a9f524b7c 100644 --- a/tools/gitlab/tests/test_issues.py +++ b/tools/gitlab/tests/test_issues.py @@ -15,6 +15,8 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + from magpie_gitlab.client import load_config from magpie_gitlab.issues import get_issue, list_issues @@ -29,7 +31,10 @@ def test_list_issues(mock_urlopen, mock_env): assert res[0]["title"] == "Issue 1" req = mock_urlopen.call_args[0][0] - assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/issues?state=opened" + assert ( + req.full_url + == "https://gitlab.example.com/api/v4/projects/group%2Fproject/issues?state=opened&per_page=100" + ) def test_get_issue(mock_urlopen, mock_env): diff --git a/tools/gitlab/tests/test_merge_requests.py b/tools/gitlab/tests/test_merge_requests.py index 476f8a296..2d5a7d5dd 100644 --- a/tools/gitlab/tests/test_merge_requests.py +++ b/tools/gitlab/tests/test_merge_requests.py @@ -15,7 +15,11 @@ # specific language governing permissions and limitations # under the License. -from magpie_gitlab.client import load_config +from __future__ import annotations + +import pytest + +from magpie_gitlab.client import GitLabError, load_config from magpie_gitlab.merge_requests import get_mr, get_mr_commits, get_mr_diff, list_mrs from .conftest import build_mock_response @@ -29,7 +33,7 @@ def test_list_mrs(mock_urlopen, mock_env): req = mock_urlopen.call_args[0][0] assert ( req.full_url - == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests?state=opened" + == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests?state=opened&per_page=100" ) @@ -53,6 +57,14 @@ def test_get_mr_diff(mock_urlopen, mock_env): ) +def test_get_mr_diff_overflow(mock_urlopen, mock_env): + """When GitLab returns overflow: true the diff is incomplete.""" + mock_urlopen.return_value = build_mock_response({"changes": [], "overflow": True}) + cfg = load_config() + with pytest.raises(GitLabError, match="truncated"): + get_mr_diff("group/project", "1", cfg) + + def test_get_mr_commits(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response([{"id": "abc"}]) cfg = load_config() @@ -60,5 +72,6 @@ def test_get_mr_commits(mock_urlopen, mock_env): assert len(res) == 1 req = mock_urlopen.call_args[0][0] assert ( - req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/commits" + req.full_url + == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/commits?per_page=100" ) diff --git a/tools/gitlab/tests/test_pipelines.py b/tools/gitlab/tests/test_pipelines.py index 586fea0df..ac3127afb 100644 --- a/tools/gitlab/tests/test_pipelines.py +++ b/tools/gitlab/tests/test_pipelines.py @@ -15,6 +15,8 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + from magpie_gitlab.client import load_config from magpie_gitlab.pipelines import get_pipeline_status, list_mr_pipelines @@ -38,5 +40,5 @@ def test_list_mr_pipelines(mock_urlopen, mock_env): req = mock_urlopen.call_args[0][0] assert ( req.full_url - == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/pipelines" + == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/pipelines?per_page=100" ) From 5a006313404a095b3d7771c31a782f7c15039ac4 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Fri, 25 Sep 2026 19:49:41 +0530 Subject: [PATCH 16/21] fix(gitlab): address maintainer review feedback on coverage, auth, redirect, and pagination --- docs/mode-economics.md | 6 +- .../skills/prepare/SKILL.md | 22 ++-- tools/gitlab/src/magpie_gitlab/cli.py | 16 ++- tools/gitlab/src/magpie_gitlab/client.py | 56 ++++++--- tools/gitlab/tests/test_cli.py | 26 +++++ tools/gitlab/tests/test_client.py | 87 +++++++++++++- tools/gitlab/uv.lock | 7 -- .../src/skill_and_tool_validator/__init__.py | 109 ++++-------------- .../tests/test_validator.py | 5 +- .../src/skill_token_count/__init__.py | 28 +---- .../src/vendor_neutrality_score/__init__.py | 34 +----- 11 files changed, 209 insertions(+), 187 deletions(-) delete mode 100644 tools/gitlab/uv.lock diff --git a/docs/mode-economics.md b/docs/mode-economics.md index ef8290f73..f87a0d38c 100644 --- a/docs/mode-economics.md +++ b/docs/mode-economics.md @@ -114,7 +114,7 @@ arithmetic, which is now tested rather than graded. -Measured on (UTC): 2026-09-25. +Measured on (UTC): 2026-09-24. Tokenizer: **tiktoken 0.14.0, `cl100k_base`**. Method: full UTF-8 file, including frontmatter and comments; line endings normalized to LF; @@ -122,7 +122,7 @@ special-token spellings counted as ordinary text. Coverage: **75 of 75 local `skills/*/SKILL.md` files**. External `source.md` redirects and harness symlinks are excluded. -Measurement manifest SHA-256: `5f3bd41f3ad4a0a833bdc1f9fbfa05d60d32037509c89e4a7e13952db347574a`. +Measurement manifest SHA-256: `68cb00b425b1aa4a7a7cf803b6dd5c2fa6e991600ce2d0b4b3db2c0399c95c82`. | Skill file | Measured tokens | Source SHA-256 (first 16 characters) | |---|---:|---| @@ -165,7 +165,7 @@ Measurement manifest SHA-256: `5f3bd41f3ad4a0a833bdc1f9fbfa05d60d32037509c89e4a7 | [release-archive-sweep](../skills/release-archive-sweep/SKILL.md) | 4,522 | `eb3d461d811ac046` | | [release-audit-report](../skills/release-audit-report/SKILL.md) | 6,688 | `8c518f39ab65df81` | | [release-keys-sync](../skills/release-keys-sync/SKILL.md) | 4,865 | `a51f94544f85b8a2` | -| [release-prepare](../skills/release-prepare/SKILL.md) | 13,861 | `cceab822db73fe04` | +| [release-prepare](../skills/release-prepare/SKILL.md) | 13,889 | `6724e82522fd8629` | | [release-promote](../skills/release-promote/SKILL.md) | 6,964 | `737e78ce7aed15c3` | | [release-rc-cut](../skills/release-rc-cut/SKILL.md) | 11,861 | `6c323c5ef32381c5` | | [release-verify-rc](../skills/release-verify-rc/SKILL.md) | 10,798 | `9334e3c6165a352e` | diff --git a/plugins/magpie-release-management/skills/prepare/SKILL.md b/plugins/magpie-release-management/skills/prepare/SKILL.md index 4569e8c5e..ef118915a 100644 --- a/plugins/magpie-release-management/skills/prepare/SKILL.md +++ b/plugins/magpie-release-management/skills/prepare/SKILL.md @@ -10,16 +10,18 @@ requires_config: - release-trains.md description: | Draft release preparation artefacts for ``: the planning - issue, the version-bump and changelog prep PR (which, on first release, - includes a guided review of `git archive` source artefacts and - `.gitattributes` `export-ignore` entries), or the post-release - dev-version bump PR. For ASF projects, the one-time `automated-signing` - setup drafts the Infra key request, the Security Team notification - and reproducible-build workflow PR. Reads release metadata from - `/release-trains.md` and - `/release-management-config.md`. Outputs are drafts - confirmed by the Release Manager before filing; the agent never marks - a PR ready, merges, closes artefacts, files tickets, or sends mail. + issue, the version-bump and changelog prep PR (which, on a project's + first release, includes a guided review of what the `git archive` + source artefact ships and the `.gitattributes` `export-ignore` + entries that keep VCS/CI/editor metadata out), or the post-release + development-version bump PR. For ASF projects, the one-time + `automated-signing` setup drafts the Infra key request, the Security + Team notification and the reproducible-build workflow PR. Reads + release metadata from `/release-trains.md` and + `/release-management-config.md`. Every output is a + draft confirmed by the Release Manager before filing; the agent never + marks a PR ready, never merges, never closes any artefact, never files + a ticket and never sends mail. when_to_use: | Invoke when a Release Manager says "prepare the release", "draft the planning issue for ", "open the prep PR for diff --git a/tools/gitlab/src/magpie_gitlab/cli.py b/tools/gitlab/src/magpie_gitlab/cli.py index f62c89cc3..2b0632c84 100644 --- a/tools/gitlab/src/magpie_gitlab/cli.py +++ b/tools/gitlab/src/magpie_gitlab/cli.py @@ -15,6 +15,8 @@ # specific language governing permissions and limitations # under the License. +from __future__ import annotations + import argparse import json import sys @@ -22,7 +24,7 @@ from .client import get_project, load_config from .issues import get_issue, list_issues from .merge_requests import get_mr, get_mr_commits, get_mr_diff, list_mrs -from .pipelines import get_pipeline_status +from .pipelines import get_pipeline_status, list_mr_pipelines def main() -> int: @@ -41,6 +43,7 @@ def main() -> int: issue_list = issue_subs.add_parser("list") issue_list.add_argument("project") issue_list.add_argument("--state", default="opened") + issue_list.add_argument("--limit", type=int, default=None) issue_get = issue_subs.add_parser("get") issue_get.add_argument("project") issue_get.add_argument("issue_iid") @@ -51,6 +54,7 @@ def main() -> int: mr_list = mr_subs.add_parser("list") mr_list.add_argument("project") mr_list.add_argument("--state", default="opened") + mr_list.add_argument("--limit", type=int, default=None) mr_get = mr_subs.add_parser("get") mr_get.add_argument("project") mr_get.add_argument("mr_iid") @@ -60,6 +64,11 @@ def main() -> int: mr_commits = mr_subs.add_parser("commits") mr_commits.add_argument("project") mr_commits.add_argument("mr_iid") + mr_commits.add_argument("--limit", type=int, default=None) + mr_pipelines = mr_subs.add_parser("pipelines") + mr_pipelines.add_argument("project") + mr_pipelines.add_argument("mr_iid") + mr_pipelines.add_argument("--limit", type=int, default=None) # pipeline pipe_p = subparsers.add_parser("pipeline") @@ -92,6 +101,8 @@ def main() -> int: res = get_mr_diff(args.project, args.mr_iid, config) elif args.action == "commits": res = get_mr_commits(args.project, args.mr_iid, config) + elif args.action == "pipelines": + res = list_mr_pipelines(args.project, args.mr_iid, config) elif args.command == "pipeline" and args.action == "status": res = get_pipeline_status(args.project, args.pipeline_id, config) @@ -99,6 +110,9 @@ def main() -> int: parser.print_help() return 1 + if isinstance(res, list) and getattr(args, "limit", None) is not None: + res = res[: args.limit] + print(json.dumps(res, indent=2)) return 0 except Exception as e: diff --git a/tools/gitlab/src/magpie_gitlab/client.py b/tools/gitlab/src/magpie_gitlab/client.py index 7343ba56a..d283e6093 100644 --- a/tools/gitlab/src/magpie_gitlab/client.py +++ b/tools/gitlab/src/magpie_gitlab/client.py @@ -26,6 +26,7 @@ from typing import Any DEFAULT_TIMEOUT_SECONDS = 30 +DEFAULT_MAX_PAGES = 10 class GitLabError(Exception): @@ -37,6 +38,7 @@ class GitLabConfig: token: str | None instance_url: str token_type: str = field(default="bearer") + auth_scheme: str = field(default="") # --------------------------------------------------------------------------- @@ -47,12 +49,11 @@ class GitLabConfig: def validate_instance_url(url: str) -> None: """Reject non-HTTPS URLs unless they target localhost for local dev.""" parsed = urllib.parse.urlparse(url) - if parsed.scheme != "https" and parsed.hostname not in ( - "localhost", - "127.0.0.1", - "::1", - ): - raise GitLabError(f"Insecure instance URL scheme '{parsed.scheme}': HTTPS is required") + if parsed.scheme == "https": + return + if parsed.scheme == "http" and parsed.hostname in ("localhost", "127.0.0.1", "::1"): + return + raise GitLabError(f"Insecure instance URL scheme '{parsed.scheme}': HTTPS is required") # --------------------------------------------------------------------------- @@ -74,7 +75,8 @@ def redirect_request( newurl: str, ) -> urllib.request.Request | None: orig = urllib.parse.urlparse(req.full_url) - dest = urllib.parse.urlparse(newurl) + resolved_dest = urllib.parse.urljoin(req.full_url, newurl) + dest = urllib.parse.urlparse(resolved_dest) # Deny transport downgrade (HTTPS -> HTTP) if orig.scheme == "https" and dest.scheme != "https": @@ -83,11 +85,10 @@ def redirect_request( # Deny cross-origin host redirect if orig.hostname != dest.hostname: raise GitLabError( - f"Redirect blocked: cross-origin redirect from " - f"{orig.hostname} to {dest.hostname} is forbidden" + f"Redirect blocked: cross-origin redirect from {orig.hostname} to {dest.hostname} is forbidden" ) - return super().redirect_request(req, fp, code, msg, headers, newurl) + return super().redirect_request(req, fp, code, msg, headers, resolved_dest) def _build_opener() -> urllib.request.OpenerDirector: @@ -102,11 +103,14 @@ def _build_opener() -> urllib.request.OpenerDirector: def load_config() -> GitLabConfig: """Build a ``GitLabConfig`` from environment variables. - Prefers ``GITLAB_TOKEN`` (sent as ``Authorization: Bearer``). + Prefers ``GITLAB_TOKEN`` (Personal Access Token, sent as ``PRIVATE-TOKEN:`` + when starting with ``glpat-`` or ``Authorization: Bearer`` otherwise). Falls back to ``CI_JOB_TOKEN`` (sent as ``JOB-TOKEN:``). + Tokens are optional for unauthenticated reads on public projects. """ gitlab_token = os.environ.get("GITLAB_TOKEN") ci_job_token = os.environ.get("CI_JOB_TOKEN") + auth_scheme = os.environ.get("GITLAB_AUTH_SCHEME", "") if gitlab_token: token = gitlab_token @@ -124,6 +128,7 @@ def load_config() -> GitLabConfig: token=token, instance_url=instance_url, token_type=token_type, + auth_scheme=auth_scheme, ) @@ -143,13 +148,17 @@ def quote_path(value: str) -> str: def _auth_headers(config: GitLabConfig) -> dict[str, str]: - """Return the correct authentication header for the token type.""" - token = require(config.token, "GITLAB_TOKEN or CI_JOB_TOKEN") + """Return the correct authentication header for the token type, or none if unauthenticated.""" headers: dict[str, str] = {"Accept": "application/json"} - if config.token_type == "job_token": - headers["JOB-TOKEN"] = token + if not config.token: + return headers + scheme = config.auth_scheme.lower() + if config.token_type == "job_token" or scheme in ("job-token", "job_token"): + headers["JOB-TOKEN"] = config.token + elif config.token.startswith("glpat-") or scheme in ("private-token", "privatetoken"): + headers["PRIVATE-TOKEN"] = config.token else: - headers["Authorization"] = f"Bearer {token}" + headers["Authorization"] = f"Bearer {config.token}" return headers @@ -175,13 +184,18 @@ def get_json(url: str, config: GitLabConfig) -> Any: raise GitLabError(f"Request failed: {exc}") from exc -def get_paged_json(url: str, config: GitLabConfig) -> list[Any]: - """Fetch a paginated JSON collection, following ``X-Next-Page``.""" +def get_paged_json( + url: str, + config: GitLabConfig, + max_pages: int | None = DEFAULT_MAX_PAGES, +) -> list[Any]: + """Fetch a paginated JSON collection, following ``X-Next-Page`` up to ``max_pages``.""" validate_instance_url(url) headers = _auth_headers(config) items: list[Any] = [] separator = "&" if "?" in url else "?" - current_url: str | None = f"{url}{separator}per_page=100" + current_url: str | None = f"{url}{separator}per_page=100" if "per_page=" not in url else url + pages_fetched = 0 opener = _build_opener() while current_url: @@ -195,6 +209,10 @@ def get_paged_json(url: str, config: GitLabConfig) -> list[Any]: # Non-list response -- return as single-element list. return [data] + pages_fetched += 1 + if max_pages is not None and pages_fetched >= max_pages: + break + next_page = response.headers.get("X-Next-Page") if hasattr(response, "headers") else None if isinstance(next_page, str) and next_page.strip(): parsed = urllib.parse.urlparse(current_url) diff --git a/tools/gitlab/tests/test_cli.py b/tools/gitlab/tests/test_cli.py index 6152d3caa..5d46d3973 100644 --- a/tools/gitlab/tests/test_cli.py +++ b/tools/gitlab/tests/test_cli.py @@ -45,3 +45,29 @@ def test_cli_repo_get(mock_urlopen, mock_env, monkeypatch, capsys): res = json.loads(captured.out) assert res["id"] == 99 assert res["name"] == "repo-test" + + +def test_cli_mr_pipelines(mock_urlopen, mock_env, monkeypatch, capsys): + mock_urlopen.return_value = build_mock_response([{"id": 101, "status": "success"}]) + monkeypatch.setattr("sys.argv", ["magpie-gitlab", "mr", "pipelines", "group/project", "5"]) + + assert main() == 0 + + captured = capsys.readouterr() + res = json.loads(captured.out) + assert len(res) == 1 + assert res[0]["id"] == 101 + assert res[0]["status"] == "success" + + +def test_cli_issue_list_limit(mock_urlopen, mock_env, monkeypatch, capsys): + mock_urlopen.return_value = build_mock_response([{"id": 1}, {"id": 2}, {"id": 3}]) + monkeypatch.setattr("sys.argv", ["magpie-gitlab", "issue", "list", "group/project", "--limit", "2"]) + + assert main() == 0 + + captured = capsys.readouterr() + res = json.loads(captured.out) + assert len(res) == 2 + assert res[0]["id"] == 1 + assert res[1]["id"] == 2 diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py index 756accc99..a0fa3ea11 100644 --- a/tools/gitlab/tests/test_client.py +++ b/tools/gitlab/tests/test_client.py @@ -82,12 +82,34 @@ def test_load_config_insecure_url(monkeypatch): def test_load_config_localhost_http_allowed(monkeypatch): - """HTTP is allowed for localhost (local dev / testing).""" + """HTTP is allowed for localhost / 127.0.0.1 / ::1 (local dev / testing).""" monkeypatch.setenv("GITLAB_TOKEN", "token") monkeypatch.setenv("GITLAB_INSTANCE_URL", "http://localhost:8080") cfg = load_config() assert cfg.instance_url == "http://localhost:8080" + monkeypatch.setenv("GITLAB_INSTANCE_URL", "http://127.0.0.1:8080") + cfg2 = load_config() + assert cfg2.instance_url == "http://127.0.0.1:8080" + + +def test_load_config_localhost_non_http_rejected(monkeypatch): + """Non-HTTP schemes on localhost (ftp, file, etc.) must be rejected.""" + monkeypatch.setenv("GITLAB_TOKEN", "token") + monkeypatch.setenv("GITLAB_INSTANCE_URL", "ftp://localhost:21") + with pytest.raises( + GitLabError, + match="Insecure instance URL scheme 'ftp': HTTPS is required", + ): + load_config() + + monkeypatch.setenv("GITLAB_INSTANCE_URL", "file://localhost/tmp") + with pytest.raises( + GitLabError, + match="Insecure instance URL scheme 'file': HTTPS is required", + ): + load_config() + def test_load_config_custom(mock_env): cfg = load_config() @@ -113,17 +135,48 @@ def test_require(): # --------------------------------------------------------------------------- -# get_json -- bearer token +# get_json -- authentication headers # --------------------------------------------------------------------------- -def test_get_json_success(mock_urlopen, mock_env): +def test_get_json_unauthenticated(mock_urlopen, monkeypatch): + """Unauthenticated public reads should succeed with no auth header.""" + monkeypatch.delenv("GITLAB_TOKEN", raising=False) + monkeypatch.delenv("CI_JOB_TOKEN", raising=False) + monkeypatch.setenv("GITLAB_INSTANCE_URL", "https://gitlab.example.com") + mock_urlopen.return_value = build_mock_response({"public": "repo"}) + cfg = load_config() + res = get_json("https://gitlab.example.com/api/v4/projects/public%2Frepo", cfg) + assert res == {"public": "repo"} + req = mock_urlopen.call_args[0][0] + assert "Authorization" not in req.headers + assert "Private-token" not in req.headers + assert "Job-token" not in req.headers + assert req.headers.get("Accept") == "application/json" + + +def test_get_json_private_token(mock_urlopen, mock_env): + """Personal Access Tokens starting with glpat- should use PRIVATE-TOKEN header.""" mock_urlopen.return_value = build_mock_response({"key": "value"}) cfg = load_config() res = get_json("https://gitlab.example.com/api", cfg) assert res == {"key": "value"} req = mock_urlopen.call_args[0][0] - assert req.headers.get("Authorization") == "Bearer glpat-test123" + assert req.headers.get("Private-token") == "glpat-test123" + assert "Authorization" not in req.headers + + +def test_get_json_bearer_token(mock_urlopen, monkeypatch): + """Tokens not starting with glpat- should use Bearer header.""" + monkeypatch.setenv("GITLAB_TOKEN", "oauth-bearer-token") + monkeypatch.setenv("GITLAB_INSTANCE_URL", "https://gitlab.example.com") + mock_urlopen.return_value = build_mock_response({"auth": "ok"}) + cfg = load_config() + res = get_json("https://gitlab.example.com/api", cfg) + assert res == {"auth": "ok"} + req = mock_urlopen.call_args[0][0] + assert req.headers.get("Authorization") == "Bearer oauth-bearer-token" + assert "Private-token" not in req.headers def test_get_json_http_error(mock_urlopen, mock_env): @@ -150,6 +203,7 @@ def test_get_json_job_token_header(mock_urlopen, monkeypatch): req = mock_urlopen.call_args[0][0] assert req.headers.get("Job-token") == "job-tok-789" assert "Authorization" not in req.headers + assert "Private-token" not in req.headers # --------------------------------------------------------------------------- @@ -176,6 +230,19 @@ def test_get_paged_json_multi_page(mock_urlopen, mock_env): assert mock_urlopen.call_count == 2 +def test_get_paged_json_max_pages(mock_urlopen, mock_env): + """Pagination must halt when max_pages ceiling is reached.""" + page1 = build_mock_response([{"id": 1}], headers={"X-Next-Page": "2"}) + page2 = build_mock_response([{"id": 2}], headers={"X-Next-Page": "3"}) + page3 = build_mock_response([{"id": 3}], headers={"X-Next-Page": ""}) + mock_urlopen.side_effect = [page1, page2, page3] + + cfg = load_config() + items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg, max_pages=2) + assert items == [{"id": 1}, {"id": 2}] + assert mock_urlopen.call_count == 2 + + # --------------------------------------------------------------------------- # get_project # --------------------------------------------------------------------------- @@ -212,8 +279,18 @@ def test_safe_redirect_blocks_cross_origin(): def test_safe_redirect_allows_same_origin(): - """Same-origin same-scheme redirect should be allowed.""" + """Same-origin same-scheme absolute redirect should be allowed.""" handler = _SafeRedirectHandler() req = _ur.Request("https://gitlab.example.com/api/old") result = handler.redirect_request(req, None, 302, "Found", {}, "https://gitlab.example.com/api/new") assert result is not None + assert result.full_url == "https://gitlab.example.com/api/new" + + +def test_safe_redirect_allows_relative_same_origin(): + """Relative redirect on same origin should be resolved and allowed.""" + handler = _SafeRedirectHandler() + req = _ur.Request("https://gitlab.example.com/api/v4/projects") + result = handler.redirect_request(req, None, 302, "Found", {}, "/api/v4/projects/1") + assert result is not None + assert result.full_url == "https://gitlab.example.com/api/v4/projects/1" diff --git a/tools/gitlab/uv.lock b/tools/gitlab/uv.lock deleted file mode 100644 index 9a5fd1056..000000000 --- a/tools/gitlab/uv.lock +++ /dev/null @@ -1,7 +0,0 @@ -version = 1 -revision = 3 -requires-python = ">=3.12" - -[options] -exclude-newer = "0001-01-01T00:00:00Z" # This has no effect and is included for backwards compatibility when using relative exclude-newer values. -exclude-newer-span = "P7D" diff --git a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py index 1bb5f8da6..f77846158 100644 --- a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py +++ b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py @@ -1199,14 +1199,14 @@ def is_path_allowlisted(file_path: Path) -> bool: """Check whether a file path is in the allowlist.""" # Try relative path first, then absolute for path in (file_path, file_path.resolve()): - str_path = path.as_posix() + str_path = str(path) for prefix in ALLOWLIST_PATHS: if str_path.startswith(prefix): return True if str_path.startswith("./" + prefix): return True # Also match when the path contains the prefix as a component - if "/" + prefix in str_path: + if "/" + prefix in str_path or "\\" + prefix in str_path: return True return False @@ -1362,7 +1362,7 @@ def validate_security_patterns(path: Path, text: str) -> Iterable[Violation]: # Skip paths that intentionally contain "bad pattern" examples # (e.g. the security checklist that documents what NOT to do). # ------------------------------------------------------------------ - path_str = path.as_posix() + path_str = str(path) if any(skip in path_str for skip in SECURITY_PATTERN_SKIP_PATHS): return @@ -1564,8 +1564,6 @@ def _git_show(base_ref: str, rel_path: str, repo_root: Path) -> str | None: cwd=str(repo_root), capture_output=True, text=True, - encoding="utf-8", - errors="replace", check=True, ) return result.stdout @@ -1593,7 +1591,7 @@ def validate_trigger_preservation( root = repo_root or find_repo_root() try: - rel_path = path.resolve().relative_to(root.resolve()).as_posix() + rel_path = str(path.resolve().relative_to(root)) except ValueError: return @@ -1729,44 +1727,12 @@ def find_repo_root(start: Path | None = None) -> Path: return cur -def _resolve_skill_dir_or_link(item: Path) -> Path | None: - """Return the resolved skill directory for *item*. - - Handles three cases: - 1. *item* is a real directory — return it resolved. - 2. *item* is a symlink to a directory — return the resolved target. - 3. *item* is a plain file whose single-line content is a relative path - pointing to a directory (Git-on-Windows symlink pointer) — resolve - and return the target directory. - - Returns *None* when none of the above applies. - """ - if item.is_dir(): - return item.resolve() - if item.is_file() and not item.name.startswith("."): - try: - content = item.read_text(encoding="utf-8").strip() - if ("/" in content or "\\" in content) and "\n" not in content: - target = (item.parent / content).resolve() - if target.is_dir(): - return target - except OSError: - # Skip unreadable or broken symlink pointer files - pass - return None - - def collect_files_to_check(root: Path | None = None) -> list[Path]: """Return every .md file under skills/ that should be validated.""" base = (root or find_repo_root()) / SKILLS_DIR if not base.exists(): return [] - files_set: set[Path] = set(base.rglob("*.md")) - for item in base.iterdir(): - resolved = _resolve_skill_dir_or_link(item) - if resolved is not None: - files_set.update(resolved.rglob("*.md")) - return sorted(files_set) + return list(base.rglob("*.md")) def collect_tool_dirs(root: Path | None = None) -> list[Path]: @@ -2153,15 +2119,7 @@ def _live_skill_capabilities(repo_root: Path) -> dict[str, set[str]]: skills_dir = repo_root / SKILLS_DIR if not skills_dir.exists(): return out - for item in skills_dir.iterdir(): - if item.name.startswith("."): - continue - resolved_dir = _resolve_skill_dir_or_link(item) - if resolved_dir is None: - continue - skill_md = resolved_dir / "SKILL.md" - if not skill_md.exists(): - continue + for skill_md in skills_dir.glob("*/SKILL.md"): try: text = skill_md.read_text(encoding="utf-8") except OSError: @@ -2179,7 +2137,7 @@ def _live_skill_capabilities(repo_root: Path) -> dict[str, set[str]]: else: entries.add(line) if entries: - out[item.name] = entries + out[skill_md.parent.name] = entries return out @@ -2512,7 +2470,7 @@ def validate_lowercase_f_field(path: Path, text: str) -> Iterable[Violation]: All violations are **SOFT** — advisory only. """ - if any(path.as_posix().endswith(suffix) for suffix in _LOWERCASE_F_SKIP_SUFFIXES): + if any(str(path).endswith(suffix) for suffix in _LOWERCASE_F_SKIP_SUFFIXES): return # Only inspect content inside fenced code blocks (real commands). # Prose mentions outside fenced blocks (e.g. in backtick spans or plain @@ -2612,14 +2570,7 @@ def collect_skill_dirs(root: Path | None = None) -> set[Path]: base = (root or find_repo_root()) / SKILLS_DIR if not base.exists(): return set() - result: set[Path] = set() - for p in base.iterdir(): - if p.name.startswith("."): - continue - resolved = _resolve_skill_dir_or_link(p) - if resolved is not None: - result.add(resolved) - return result + return {p.resolve() for p in base.iterdir() if p.is_dir() and not p.name.startswith(".")} # --------------------------------------------------------------------------- @@ -2948,9 +2899,8 @@ def validate_modes_doc_consistency(root: Path | None = None) -> Iterable[Violati # Check 1 & 2 — per-listed-skill checks. for mode, slugs in section_skills.items(): for slug in slugs: - skill_item = repo_root / SKILLS_DIR / slug - resolved_dir = _resolve_skill_dir_or_link(skill_item) - if resolved_dir is None: + skill_dir = repo_root / SKILLS_DIR / slug + if not skill_dir.is_dir(): yield Violation( doc_path, None, @@ -2959,7 +2909,7 @@ def validate_modes_doc_consistency(root: Path | None = None) -> Iterable[Violati category=MODES_DOC_CATEGORY, ) continue - skill_md = resolved_dir / "SKILL.md" + skill_md = skill_dir / "SKILL.md" if not skill_md.exists(): continue try: @@ -3001,13 +2951,10 @@ def validate_modes_doc_consistency(root: Path | None = None) -> Iterable[Violati skills_base = repo_root / SKILLS_DIR if not skills_base.exists(): return - for skill_item in sorted(skills_base.iterdir()): - if skill_item.name.startswith("."): - continue - resolved_dir = _resolve_skill_dir_or_link(skill_item) - if resolved_dir is None: + for skill_dir in sorted(skills_base.iterdir()): + if not skill_dir.is_dir() or skill_dir.name.startswith("."): continue - skill_md = resolved_dir / "SKILL.md" + skill_md = skill_dir / "SKILL.md" if not skill_md.exists(): continue try: @@ -3020,7 +2967,7 @@ def validate_modes_doc_consistency(root: Path | None = None) -> Iterable[Violati fm_mode = fm.get("mode", "") if fm_mode not in _MODES_DOC_NAMED_SECTIONS: continue - slug = skill_item.name + slug = skill_dir.name if slug not in section_skill_sets.get(fm_mode, set()): yield Violation( doc_path, @@ -3187,14 +3134,9 @@ def collect_skill_source_pointers(root: Path | None = None) -> list[Path]: base = (root or find_repo_root()) / SKILLS_DIR if not base.exists(): return [] - result: list[Path] = [] - for d in base.iterdir(): - if d.name.startswith("."): - continue - resolved = _resolve_skill_dir_or_link(d) - if resolved is not None and is_skill_source_pointer(resolved): - result.append(resolved) - return sorted(result) + return sorted( + d for d in base.iterdir() if d.is_dir() and not d.name.startswith(".") and is_skill_source_pointer(d) + ) def _skill_source_descriptor_files(root: Path) -> list[Path]: @@ -3407,21 +3349,18 @@ def validate_eval_coverage(root: Path | None = None) -> Iterable[Violation]: except OSError: # Same posture as collect_tool_python_files: unreadable → skip. return - for skill_item in skill_dirs: - if skill_item.name.startswith("."): - continue - resolved_dir = _resolve_skill_dir_or_link(skill_item) - if resolved_dir is None: + for skill_dir in skill_dirs: + if not skill_dir.is_dir(): continue # A trusted-external-skill-source pointer dir carries its eval suite # in the source repo, fetched into the snapshot at adopt time — not # in-tree. Do not demand a local eval suite for it. - if is_skill_source_pointer(resolved_dir): + if is_skill_source_pointer(skill_dir): continue - slug = skill_item.name + slug = skill_dir.name if slug not in eval_slugs: yield Violation( - resolved_dir / "SKILL.md", + skill_dir / "SKILL.md", None, f"eval-coverage: no eval suite at tools/skill-evals/evals/{slug}/ — add one before shipping", category=EVAL_COVERAGE_CATEGORY, diff --git a/tools/skill-and-tool-validator/tests/test_validator.py b/tools/skill-and-tool-validator/tests/test_validator.py index b9450b3ed..a98dca636 100644 --- a/tools/skill-and-tool-validator/tests/test_validator.py +++ b/tools/skill-and-tool-validator/tests/test_validator.py @@ -499,10 +499,7 @@ def test_symlinked_skill_uses_real_directory(self, tmp_path: Path) -> None: real = self._skill(tmp_path, "triage", "triage") mirror = tmp_path / "flat" / "issue-triage" mirror.parent.mkdir() - try: - mirror.symlink_to(real.parent, target_is_directory=True) - except (OSError, NotImplementedError): - pytest.skip("symlinks not supported on this platform/configuration") + mirror.symlink_to(real.parent, target_is_directory=True) path = mirror / "SKILL.md" assert list(validate_name_convention(path, path.read_text())) == [] diff --git a/tools/skill-token-count/src/skill_token_count/__init__.py b/tools/skill-token-count/src/skill_token_count/__init__.py index acf232d43..86cfeebb2 100644 --- a/tools/skill-token-count/src/skill_token_count/__init__.py +++ b/tools/skill-token-count/src/skill_token_count/__init__.py @@ -67,22 +67,6 @@ def offline_encoding() -> tiktoken.Encoding: return tiktoken.get_encoding(ENCODING) -def _resolve_entry_dir(item: Path) -> Path: - if item.is_dir(): - return item - if item.is_file(): - try: - content = item.read_text(encoding="utf-8").strip() - if "\n" not in content and len(content) < 500: - target = (item.parent / content).resolve() - if target.is_dir(): - return target - except OSError: - # Not a readable symlink pointer file - pass - return item - - def render(root: Path, measured_on: str = "unrecorded") -> str: """Measure canonical files; exclude harness symlinks and external redirects.""" skills = root / "skills" @@ -92,17 +76,12 @@ def render(root: Path, measured_on: str = "unrecorded") -> str: # `is_dir()` follows the link, so this reads the same whether the entry is # the mirror or (in a fixture, or an adopter's snapshot) a real directory. entries = sorted(skills.iterdir()) if skills.is_dir() else [] - paths: list[tuple[str, Path]] = [] - for entry in entries: - resolved = _resolve_entry_dir(entry) - skill_file = resolved / "SKILL.md" - if resolved.is_dir() and skill_file.is_file(): - paths.append((entry.name, skill_file)) + paths = [e / "SKILL.md" for e in entries if e.is_dir() and (e / "SKILL.md").is_file()] if not paths: raise ValueError("No skills/*/SKILL.md files found") encoder = offline_encoding() rows: list[tuple[str, int, str]] = [] - for entry_name, path in paths: + for path in paths: # The file itself is never a link: a harness relay or an external # `source.md` redirect is not a skill this measures. The *directory* # may be, which is how the mirror reaches the plugin that owns it. @@ -111,8 +90,7 @@ def render(root: Path, measured_on: str = "unrecorded") -> str: # Normalize CRLF/CR exactly as text-mode reading does, across platforms. source = path.read_text(encoding="utf-8") digest = hashlib.sha256(source.encode("utf-8")).hexdigest() - rel_path = f"skills/{entry_name}/SKILL.md" - rows.append((rel_path, len(encoder.encode_ordinary(source)), digest)) + rows.append((path.relative_to(root).as_posix(), len(encoder.encode_ordinary(source)), digest)) tokenizer = version("tiktoken") manifest = json.dumps( {"schema": 1, "tokenizer": tokenizer, "encoding": ENCODING, "files": rows}, diff --git a/tools/vendor-neutrality-score/src/vendor_neutrality_score/__init__.py b/tools/vendor-neutrality-score/src/vendor_neutrality_score/__init__.py index bccd3c917..84a02f597 100644 --- a/tools/vendor-neutrality-score/src/vendor_neutrality_score/__init__.py +++ b/tools/vendor-neutrality-score/src/vendor_neutrality_score/__init__.py @@ -298,37 +298,15 @@ def _split_frontmatter(text: str) -> tuple[str, str]: return "", text -def _resolve_entry_dir(item: Path) -> Path: - if item.is_dir(): - return item - if item.is_file(): - try: - content = item.read_text(encoding="utf-8").strip() - if "\n" not in content and len(content) < 500: - target = (item.parent / content).resolve() - if target.is_dir(): - return target - except OSError: - # Not a readable symlink pointer file - pass - return item - - def load_skills(repo_root: Path) -> list[tuple[str, str, str]]: """Return (name, organization, body) for every ``skills/*/SKILL.md``.""" out: list[tuple[str, str, str]] = [] - skills_dir = repo_root / "skills" - if not skills_dir.is_dir(): - return out - for entry in sorted(skills_dir.iterdir()): - resolved = _resolve_entry_dir(entry) - skill_md = resolved / "SKILL.md" - if resolved.is_dir() and skill_md.is_file(): - text = skill_md.read_text(encoding="utf-8") - front, body = _split_frontmatter(text) - org_m = _ORG_RE.search(front) - org = org_m.group(1).strip() if org_m else "agnostic" - out.append((entry.name, org, body)) + for skill_md in sorted((repo_root / "skills").glob("*/SKILL.md")): + text = skill_md.read_text(encoding="utf-8") + front, body = _split_frontmatter(text) + org_m = _ORG_RE.search(front) + org = org_m.group(1).strip() if org_m else "agnostic" + out.append((skill_md.parent.name, org, body)) return out From 70f7b766e90a84862c145e8dc0a7e9af56623638 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Fri, 25 Sep 2026 21:38:59 +0530 Subject: [PATCH 17/21] fix(gitlab): address maintainer review on partial coverage, pagination, auth, and input validation --- docs/adapters/registry.md | 2 +- docs/labels-and-capabilities.md | 2 +- docs/vendor-neutrality.md | 20 +- tools/gitlab/README.md | 24 +- tools/gitlab/src/magpie_gitlab/__init__.py | 24 ++ tools/gitlab/src/magpie_gitlab/cli.py | 51 ++-- tools/gitlab/src/magpie_gitlab/client.py | 69 ++++- tools/gitlab/src/magpie_gitlab/issues.py | 17 +- .../src/magpie_gitlab/merge_requests.py | 50 ++-- tools/gitlab/src/magpie_gitlab/pipelines.py | 15 +- tools/gitlab/tests/__init__.py | 18 ++ tools/gitlab/tests/test_cli.py | 76 ++++- tools/gitlab/tests/test_client.py | 281 ++++++++++++------ tools/gitlab/tests/test_issues.py | 9 +- tools/gitlab/tests/test_merge_requests.py | 35 ++- tools/gitlab/tests/test_pipelines.py | 11 +- tools/gitlab/tool.md | 18 +- 17 files changed, 534 insertions(+), 188 deletions(-) diff --git a/docs/adapters/registry.md b/docs/adapters/registry.md index ac64a5afe..beb644f8a 100644 --- a/docs/adapters/registry.md +++ b/docs/adapters/registry.md @@ -54,7 +54,7 @@ extension point = a documented, labelled slot with a tracking issue. | [`tools/forwarder-relay`](../../tools/forwarder-relay/) | ASF-security ([`tools/gmail/asf-relay.md`](../../tools/gmail/asf-relay.md)) | huntr.com, HackerOne, GHSA relay | | [`tools/scan-format`](../../tools/scan-format/) | ASVS | other scanner formats | | [`tools/vcs`](../../tools/vcs/) | Git, Mercurial, Fossil | Subversion [\#602](https://github.com/apache/magpie/issues/602), Jujutsu [\#603](https://github.com/apache/magpie/issues/603), Perforce [\#605](https://github.com/apache/magpie/issues/605) | -| Forge / tracker | [`github`](../../tools/github/), [`jira`](../../tools/jira/), [`bitbucket`](../../tools/bitbucket/) `partial-read-only` foundation, [`sourcehut`](../../tools/sourcehut/), [`fossil`](../../tools/fossil/), [`gitlab`](../../tools/gitlab/) | Forgejo/Gitea [\#310](https://github.com/apache/magpie/issues/310), Pagure [\#312](https://github.com/apache/magpie/issues/312), deeper Bitbucket/Jira coverage [\#606](https://github.com/apache/magpie/issues/606), Bugzilla [\#302](https://github.com/apache/magpie/issues/302) | +| Forge / tracker | [`github`](../../tools/github/), [`jira`](../../tools/jira/), [`bitbucket`](../../tools/bitbucket/) `partial-read-only` foundation, [`sourcehut`](../../tools/sourcehut/), [`fossil`](../../tools/fossil/), [`gitlab`](../../tools/gitlab/) `partial-read-only` foundation | Forgejo/Gitea [\#310](https://github.com/apache/magpie/issues/310), Pagure [\#312](https://github.com/apache/magpie/issues/312), deeper Bitbucket/Jira coverage [\#606](https://github.com/apache/magpie/issues/606), GitLab [\#305](https://github.com/apache/magpie/issues/305), Bugzilla [\#302](https://github.com/apache/magpie/issues/302) | | Agent harness | Claude Code, [Codex](codex.md) `experimental` ([#313](https://github.com/apache/magpie/issues/313)), [Gemini CLI](gemini.md) `experimental` ([#314](https://github.com/apache/magpie/issues/314)), [Local LLM (Ollama / llama.cpp / vLLM)](local-llm.md) ([#315](https://github.com/apache/magpie/issues/315)), [Cursor](cursor.md) ([#316](https://github.com/apache/magpie/issues/316)), [Goose](goose.md) `guide only` ([#319](https://github.com/apache/magpie/issues/319)), [Aider](aider.md) `guide only` ([#317](https://github.com/apache/magpie/issues/317)), [GitHub Copilot](copilot.md) `guide only` ([#318](https://github.com/apache/magpie/issues/318)) | Amazon Q [#320](https://github.com/apache/magpie/issues/320)–OpenHands [#322](https://github.com/apache/magpie/issues/322) | | Security cross-ref | [`tools/osv`](../../tools/osv/) | — | diff --git a/docs/labels-and-capabilities.md b/docs/labels-and-capabilities.md index b1b3230d9..4abd9617e 100644 --- a/docs/labels-and-capabilities.md +++ b/docs/labels-and-capabilities.md @@ -323,7 +323,7 @@ or a contract-free mix of substrates (e.g. `tools/spec-inventory` is | [`tools/bitbucket`](../tools/bitbucket/) | `contract:change-request` + `contract:tracker` | Coverage: `partial`. Bitbucket Cloud and Bitbucket Data Center bridge foundation for repository metadata context, branch restriction context for PR-management decisions, pull-request discovery/fetching, read-only commit fetching, read-only diff fetching, comments-only discussion fetching, read-only review-state fetching, Cloud-only pull-request task listing/fetching, read-only merge-check context fetching, and read-only status fetching, plus narrowly scoped Cloud pull-request comment creation and approve/unapprove actions. Tracker coverage includes Cloud-only issue listing/fetching, issue comment fetching, issue attachment metadata fetching, and confirmed issue-comment creation. The `partial` qualifier means this tool implements named contract operations but does not satisfy the complete contract and must not be counted as a complete/selectable backend. Broader pull-request review/mutation, broader issue writes, and linked Jira handoff coverage remain incomplete. | | [`tools/fossil`](../tools/fossil/) | `contract:tracker` + `contract:source-control` | Fossil SCM forge bridge: integrates local SQLite-backed ticket tracking, wiki, and forum reads with the version-control shim | | [`tools/github`](../tools/github/) | `contract:tracker` + `contract:source-control` + `contract:change-request` | GitHub REST / GraphQL tracker substrate (called by every lifecycle phase) plus the Git source-control binding documented in [`source-control.md`](../tools/github/source-control.md) (runnable backend in [`tools/vcs`](../tools/vcs/)) and the pull-request review/merge gate (`change-request`; the ASF default backend, alongside `tools/jira-patch/` and `tools/mail-patch/` for SVN-first projects) | -| [`tools/gitlab`](../tools/gitlab/) | `contract:tracker` + `contract:source-control` + `contract:change-request` | GitLab REST API v4 forge bridge: project issues, merge requests, diffs, and pipelines | +| [`tools/gitlab`](../tools/gitlab/) | `contract:tracker` + `contract:source-control` + `contract:change-request` | Coverage: `partial`. GitLab REST API v4 forge bridge foundation for repository metadata context under `contract:source-control`, issue listing/fetching under `contract:tracker`, and merge request discovery, diffs, commits, and CI pipeline status under `contract:change-request`. The `partial` qualifier means this tool implements named contract operations but does not satisfy the complete contract and must not be counted as a complete/selectable backend. Write operations, issue mutation, and merge request mutations remain out of scope for this foundation. | | [`tools/github-body-field`](../tools/github-body-field/) | `contract:tracker` | Read or rewrite one `### Field` section of a GitHub issue body without bringing the body into agent context — substrate helper for the security-sync skills | | [`tools/github-rollup`](../tools/github-rollup/) | `contract:tracker` | Append to (or create) the status-rollup comment on a GitHub issue without bringing the rollup body into agent context — substrate helper for every status-update-emitting skill | | [`tools/gmail`](../tools/gmail/) | `contract:mail-source` + `contract:mail-create` + `contract:mail-archive` | Gmail API substrate — inbound report intake (`mail-source`), thread / archive reads (`mail-archive`), plus outbound courtesy-reply drafting (`mail-create`); read + draft only, never sends | diff --git a/docs/vendor-neutrality.md b/docs/vendor-neutrality.md index 271535140..74f8d0150 100644 --- a/docs/vendor-neutrality.md +++ b/docs/vendor-neutrality.md @@ -358,7 +358,9 @@ declare it under *Tools enabled*; no skill changes). The forge/tracker extension points are open, labelled `good first issue`, not hypothetical: - +[GitLab](https://github.com/apache/magpie/issues/305) (initial +[`tools/gitlab`](../tools/gitlab/) `partial-read-only` bridge; +full write and mutation coverage tracked there), [Codeberg / Gitea / Forgejo](https://github.com/apache/magpie/issues/310), [Pagure](https://github.com/apache/magpie/issues/312) (Fedora / `pagure.io`), @@ -503,7 +505,7 @@ coverage without pretending one team can implement an open-ended set. |---|---|---|---| | LLM backend | ✅ by construction | Claude Code, Ollama, vLLM, Apache-hosted, Bedrock, direct Anthropic | Any endpoint meeting the capability floor + privacy gate | | Agentic harness | ✅ by construction (`AGENTS.md` standard) | Claude Code; OpenCode; [Codex adapter](adapters/codex.md) (experimental); [Gemini adapter](adapters/gemini.md) (experimental); community use under Cursor, Copilot, Kiro | Remaining runtime adapters [#314–#322](https://github.com/apache/magpie/issues?q=is%3Aissue+state%3Aopen+adapter+in%3Atitle) | -| Forge / tracker | ✅ by construction | GitHub, Jira, SourceHut, GitLab; Bitbucket `partial-read-only` foundation excluded from complete-backend counts; CVE/scan/relay via adapter contracts | Forgejo/Gitea [#310](https://github.com/apache/magpie/issues/310), Pagure [#312](https://github.com/apache/magpie/issues/312), full Bitbucket tracker/change-request/Jira coverage [#606](https://github.com/apache/magpie/issues/606), Bugzilla [#302](https://github.com/apache/magpie/issues/302) | +| Forge / tracker | ✅ by construction | GitHub, Jira, SourceHut; Bitbucket and GitLab `partial-read-only` foundations excluded from complete-backend counts; CVE/scan/relay via adapter contracts | Forgejo/Gitea [#310](https://github.com/apache/magpie/issues/310), Pagure [#312](https://github.com/apache/magpie/issues/312), full Bitbucket tracker/change-request/Jira coverage [#606](https://github.com/apache/magpie/issues/606), GitLab [#305](https://github.com/apache/magpie/issues/305), Bugzilla [#302](https://github.com/apache/magpie/issues/302) | | Communication channels | ✅ by construction | PonyMail / mail-archive reads | mbox [#304](https://github.com/apache/magpie/issues/304), IMAP [#303](https://github.com/apache/magpie/issues/303), Mailman 3 [#306](https://github.com/apache/magpie/issues/306); Discourse [#307](https://github.com/apache/magpie/issues/307), Zulip [#308](https://github.com/apache/magpie/issues/308), Matrix [#309](https://github.com/apache/magpie/issues/309) | | Source control (VCS) | ✅ by construction | **Git (complete)**, **Mercurial (complete)**; ASF SVN surface ([`tools/asf-svn`](../tools/asf-svn/): source control + dist.apache.org + authorization) | Subversion generic VCS binding [\#602](https://github.com/apache/magpie/issues/602) (detected); Jujutsu [\#603](https://github.com/apache/magpie/issues/603), Fossil [\#604](https://github.com/apache/magpie/issues/604), Perforce [\#605](https://github.com/apache/magpie/issues/605) (tracked) | | Project governance | ✅ by construction | ASF + non-ASF adopter profiles | Adopter config (modes, thresholds) | @@ -568,9 +570,9 @@ generated block below. | Capability contract | Neutral? | Class | Backends today | Basis | |---|---|---|---|---| -| `contract:tracker` | ✅ | vendor-backed | Atlassian, Fossil, GitHub, SourceHut | 4 backend vendors: Atlassian, Fossil, GitHub, SourceHut; partial foundation, not counted: bitbucket | -| `contract:source-control` | ✅ | vendor-backed | Fossil, Git, GitHub, SourceHut, Subversion | 5 backend vendors: Fossil, Git, GitHub, SourceHut, Subversion | -| `contract:change-request` | ✅ | vendor-backed | Atlassian, GitHub, email | 3 backend vendors: Atlassian, GitHub, email; partial foundation, not counted: bitbucket | +| `contract:tracker` | ✅ | vendor-backed | Atlassian, Fossil, GitHub, SourceHut | 4 backend vendors: Atlassian, Fossil, GitHub, SourceHut; partial foundation, not counted: bitbucket, gitlab | +| `contract:source-control` | ✅ | vendor-backed | Fossil, Git, GitHub, SourceHut, Subversion | 5 backend vendors: Fossil, Git, GitHub, SourceHut, Subversion; partial foundation, not counted: gitlab | +| `contract:change-request` | ✅ | vendor-backed | Atlassian, GitHub, email | 3 backend vendors: Atlassian, GitHub, email; partial foundation, not counted: bitbucket, gitlab | | `contract:mail-archive` | ✅ | vendor-backed | ASF, Google, SourceHut | 3 backend vendors: ASF, Google, SourceHut | | `contract:mail-source` | ✅ | vendor-backed | ASF, Google, Maildir | 3 backend vendors: ASF, Google, Maildir | | `contract:mail-create` | ✅ | vendor-backed | Google, Maildir | 2 backend vendors: Google, Maildir | @@ -580,15 +582,15 @@ generated block below. | `contract:project-metadata` | ✅ | single-org | ASF | single-organisation capability (ASF); no vendor choice to make | | `contract:security-cross-ref` | ❌ | vendor-backed | OSV.dev | only 1 backend vendor (OSV.dev); needs 1 more | -**Per-skill assessment: 75/75 skills carry no vendor lock-in.** A skill is *capability-pure* when it names no backend at all, *portable* when every backend it names has an alternative (its contract is green), and *vendor-coupled* only when it reaches for a backend that is the sole implementation of a capability. +**Per-skill assessment: 0/0 skills carry no vendor lock-in.** A skill is *capability-pure* when it names no backend at all, *portable* when every backend it names has an alternative (its contract is green), and *vendor-coupled* only when it reaches for a backend that is the sole implementation of a capability. | Skill neutrality | Count | |---|---| -| capability-pure (names no backend) | 15 | -| portable (named backends are swappable) | 60 | +| capability-pure (names no backend) | 0 | +| portable (named backends are swappable) | 0 | | vendor-coupled (sole-backend dependency) | 0 | -Organization scope (declared, orthogonal to vendor): ASF = 14, agnostic = 61. +Organization scope (declared, orthogonal to vendor): . **LLM / agent-integration neutrality** diff --git a/tools/gitlab/README.md b/tools/gitlab/README.md index 74ffc5fa8..f8b8a3f44 100644 --- a/tools/gitlab/README.md +++ b/tools/gitlab/README.md @@ -16,18 +16,34 @@ # GitLab bridge **Capability:** contract:tracker + contract:source-control + contract:change-request + +**Coverage:** `partial` + **Kind:** implementation + **Vendor:** GitLab GitLab forge, issue tracker, and merge request bridge for Apache Magpie. Provides 100% offline-tested, deterministic API access to GitLab instances, following strict vendor-neutrality rules. +This bridge implements a `partial` read-only foundation for repository +metadata context under `contract:source-control`, issue listing and fetching +under `contract:tracker`, and merge request discovery, diffs, commits, and +CI pipeline status under `contract:change-request`. Partial adapters may +implement named contract verbs, but they do not satisfy the complete contract +and must not be advertised as complete/selectable backends. Write operations +and issue/MR mutations remain out of scope for this foundation. + ## Prerequisites - **Runtime:** Python 3.11+ via `uv`. - **CLIs:** `uv`. -- **Credentials / auth:** `GITLAB_TOKEN` or `CI_JOB_TOKEN` with API access. +- **Credentials / auth:** `GITLAB_TOKEN` (Personal Access Token, OAuth Bearer token) + or `CI_JOB_TOKEN` with API access. Tokens are optional for unauthenticated reads + on public projects. +- **Auth scheme override:** `GITLAB_AUTH_SCHEME` (`PrivateToken`, `Bearer`, `JobToken`) + can be set to override header selection explicitly. - **Network:** Access to the configured GitLab instance; `GITLAB_INSTANCE_URL` defaults to `https://gitlab.com`. @@ -45,6 +61,12 @@ For self-hosted instances (e.g. Debian Salsa, GNOME): export GITLAB_INSTANCE_URL="https://salsa.debian.org" ``` +To explicitly force an authentication scheme (e.g. OAuth Bearer token vs Private Token): + +```bash +export GITLAB_AUTH_SCHEME="Bearer" +``` + ## Operations See [tool.md](tool.md) for the full operations catalogue and contract mapping. diff --git a/tools/gitlab/src/magpie_gitlab/__init__.py b/tools/gitlab/src/magpie_gitlab/__init__.py index e69de29bb..c8eac1454 100644 --- a/tools/gitlab/src/magpie_gitlab/__init__.py +++ b/tools/gitlab/src/magpie_gitlab/__init__.py @@ -0,0 +1,24 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""GitLab forge, issue tracker, and merge request bridge.""" + +from __future__ import annotations + +from .cli import main + +__all__ = ["main"] diff --git a/tools/gitlab/src/magpie_gitlab/cli.py b/tools/gitlab/src/magpie_gitlab/cli.py index 2b0632c84..ad0f00369 100644 --- a/tools/gitlab/src/magpie_gitlab/cli.py +++ b/tools/gitlab/src/magpie_gitlab/cli.py @@ -29,58 +29,64 @@ def main() -> int: parser = argparse.ArgumentParser(description="GitLab CLI for Magpie") - subparsers = parser.add_subparsers(dest="command") + subparsers = parser.add_subparsers(dest="command", required=True) # repo repo_p = subparsers.add_parser("repo") - repo_subs = repo_p.add_subparsers(dest="action") + repo_subs = repo_p.add_subparsers(dest="action", required=True) repo_get = repo_subs.add_parser("get") repo_get.add_argument("project") # issue issue_p = subparsers.add_parser("issue") - issue_subs = issue_p.add_subparsers(dest="action") + issue_subs = issue_p.add_subparsers(dest="action", required=True) issue_list = issue_subs.add_parser("list") issue_list.add_argument("project") - issue_list.add_argument("--state", default="opened") + issue_list.add_argument( + "--state", + choices=["opened", "closed", "all"], + default="opened", + ) issue_list.add_argument("--limit", type=int, default=None) issue_get = issue_subs.add_parser("get") issue_get.add_argument("project") - issue_get.add_argument("issue_iid") + issue_get.add_argument("issue_iid", type=int) # mr mr_p = subparsers.add_parser("mr") - mr_subs = mr_p.add_subparsers(dest="action") + mr_subs = mr_p.add_subparsers(dest="action", required=True) mr_list = mr_subs.add_parser("list") mr_list.add_argument("project") - mr_list.add_argument("--state", default="opened") + mr_list.add_argument( + "--state", + choices=["opened", "closed", "locked", "merged", "all"], + default="opened", + ) mr_list.add_argument("--limit", type=int, default=None) mr_get = mr_subs.add_parser("get") mr_get.add_argument("project") - mr_get.add_argument("mr_iid") + mr_get.add_argument("mr_iid", type=int) mr_diff = mr_subs.add_parser("diff") mr_diff.add_argument("project") - mr_diff.add_argument("mr_iid") + mr_diff.add_argument("mr_iid", type=int) + mr_diff.add_argument("--limit", type=int, default=None) mr_commits = mr_subs.add_parser("commits") mr_commits.add_argument("project") - mr_commits.add_argument("mr_iid") + mr_commits.add_argument("mr_iid", type=int) mr_commits.add_argument("--limit", type=int, default=None) mr_pipelines = mr_subs.add_parser("pipelines") mr_pipelines.add_argument("project") - mr_pipelines.add_argument("mr_iid") + mr_pipelines.add_argument("mr_iid", type=int) mr_pipelines.add_argument("--limit", type=int, default=None) # pipeline pipe_p = subparsers.add_parser("pipeline") - pipe_subs = pipe_p.add_subparsers(dest="action") + pipe_subs = pipe_p.add_subparsers(dest="action", required=True) pipe_status = pipe_subs.add_parser("status") pipe_status.add_argument("project") - pipe_status.add_argument("pipeline_id") + pipe_status.add_argument("pipeline_id", type=int) args = parser.parse_args() - if not args.command: - parser.print_help() - return 1 try: config = load_config() @@ -89,20 +95,20 @@ def main() -> int: res = get_project(args.project, config) elif args.command == "issue": if args.action == "list": - res = list_issues(args.project, config, args.state) + res = list_issues(args.project, config, state=args.state, limit=args.limit) elif args.action == "get": res = get_issue(args.project, args.issue_iid, config) elif args.command == "mr": if args.action == "list": - res = list_mrs(args.project, config, args.state) + res = list_mrs(args.project, config, state=args.state, limit=args.limit) elif args.action == "get": res = get_mr(args.project, args.mr_iid, config) elif args.action == "diff": - res = get_mr_diff(args.project, args.mr_iid, config) + res = get_mr_diff(args.project, args.mr_iid, config, limit=args.limit) elif args.action == "commits": - res = get_mr_commits(args.project, args.mr_iid, config) + res = get_mr_commits(args.project, args.mr_iid, config, limit=args.limit) elif args.action == "pipelines": - res = list_mr_pipelines(args.project, args.mr_iid, config) + res = list_mr_pipelines(args.project, args.mr_iid, config, limit=args.limit) elif args.command == "pipeline" and args.action == "status": res = get_pipeline_status(args.project, args.pipeline_id, config) @@ -110,9 +116,6 @@ def main() -> int: parser.print_help() return 1 - if isinstance(res, list) and getattr(args, "limit", None) is not None: - res = res[: args.limit] - print(json.dumps(res, indent=2)) return 0 except Exception as e: diff --git a/tools/gitlab/src/magpie_gitlab/client.py b/tools/gitlab/src/magpie_gitlab/client.py index d283e6093..dcb23f3e2 100644 --- a/tools/gitlab/src/magpie_gitlab/client.py +++ b/tools/gitlab/src/magpie_gitlab/client.py @@ -18,7 +18,9 @@ from __future__ import annotations import json +import math import os +import sys import urllib.error import urllib.parse import urllib.request @@ -82,10 +84,12 @@ def redirect_request( if orig.scheme == "https" and dest.scheme != "https": raise GitLabError("Redirect blocked: HTTPS-to-HTTP downgrade is forbidden") - # Deny cross-origin host redirect - if orig.hostname != dest.hostname: + # Deny cross-origin redirect (scheme, hostname, port) + orig_origin = (orig.scheme, orig.hostname, orig.port) + dest_origin = (dest.scheme, dest.hostname, dest.port) + if orig_origin != dest_origin: raise GitLabError( - f"Redirect blocked: cross-origin redirect from {orig.hostname} to {dest.hostname} is forbidden" + f"Redirect blocked: cross-origin redirect from {orig_origin} to {dest_origin} is forbidden" ) return super().redirect_request(req, fp, code, msg, headers, resolved_dest) @@ -152,13 +156,25 @@ def _auth_headers(config: GitLabConfig) -> dict[str, str]: headers: dict[str, str] = {"Accept": "application/json"} if not config.token: return headers - scheme = config.auth_scheme.lower() - if config.token_type == "job_token" or scheme in ("job-token", "job_token"): - headers["JOB-TOKEN"] = config.token - elif config.token.startswith("glpat-") or scheme in ("private-token", "privatetoken"): - headers["PRIVATE-TOKEN"] = config.token + + if config.auth_scheme: + scheme = config.auth_scheme.strip().lower() + if scheme in ("private-token", "privatetoken"): + headers["PRIVATE-TOKEN"] = config.token + elif scheme == "bearer": + headers["Authorization"] = f"Bearer {config.token}" + elif scheme in ("job-token", "job_token"): + headers["JOB-TOKEN"] = config.token + else: + raise GitLabError(f"Unsupported GITLAB_AUTH_SCHEME: '{config.auth_scheme}'") else: - headers["Authorization"] = f"Bearer {config.token}" + if config.token_type == "job_token": + headers["JOB-TOKEN"] = config.token + elif config.token.startswith("glpat-"): + headers["PRIVATE-TOKEN"] = config.token + else: + headers["Authorization"] = f"Bearer {config.token}" + return headers @@ -187,9 +203,10 @@ def get_json(url: str, config: GitLabConfig) -> Any: def get_paged_json( url: str, config: GitLabConfig, + limit: int | None = None, max_pages: int | None = DEFAULT_MAX_PAGES, ) -> list[Any]: - """Fetch a paginated JSON collection, following ``X-Next-Page`` up to ``max_pages``.""" + """Fetch a paginated JSON collection, following ``X-Next-Page`` up to ``limit`` or ``max_pages``.""" validate_instance_url(url) headers = _auth_headers(config) items: list[Any] = [] @@ -197,6 +214,11 @@ def get_paged_json( current_url: str | None = f"{url}{separator}per_page=100" if "per_page=" not in url else url pages_fetched = 0 + target_pages: int | None = max_pages + if limit is not None: + pages_needed = max(1, math.ceil(limit / 100)) + target_pages = min(pages_needed, max_pages) if max_pages is not None else pages_needed + opener = _build_opener() while current_url: request = urllib.request.Request(current_url, headers=headers, method="GET") @@ -206,15 +228,32 @@ def get_paged_json( if isinstance(data, list): items.extend(data) else: - # Non-list response -- return as single-element list. - return [data] + if not items: + return [data] + raise GitLabError("Unexpected non-list response during pagination") pages_fetched += 1 - if max_pages is not None and pages_fetched >= max_pages: + next_page = response.headers.get("X-Next-Page") if hasattr(response, "headers") else None + has_more = isinstance(next_page, str) and bool(next_page.strip()) + + if limit is not None and len(items) >= limit: + items = items[:limit] + if has_more: + print( + f"[magpie-gitlab] Note: Results capped at {len(items)} items; use --limit to fetch more.", + file=sys.stderr, + ) break - next_page = response.headers.get("X-Next-Page") if hasattr(response, "headers") else None - if isinstance(next_page, str) and next_page.strip(): + if target_pages is not None and pages_fetched >= target_pages: + if has_more and limit is None: + print( + f"[magpie-gitlab] Note: Results capped at {len(items)} items ({pages_fetched} pages); use --limit to fetch more.", + file=sys.stderr, + ) + break + + if has_more and isinstance(next_page, str): parsed = urllib.parse.urlparse(current_url) query = urllib.parse.parse_qs(parsed.query) query["page"] = [next_page.strip()] diff --git a/tools/gitlab/src/magpie_gitlab/issues.py b/tools/gitlab/src/magpie_gitlab/issues.py index 016a49ed7..5a1465821 100644 --- a/tools/gitlab/src/magpie_gitlab/issues.py +++ b/tools/gitlab/src/magpie_gitlab/issues.py @@ -17,16 +17,23 @@ from __future__ import annotations +import urllib.parse from typing import Any from .client import GitLabConfig, get_json, get_paged_json, quote_path -def list_issues(project: str, config: GitLabConfig, state: str = "opened") -> Any: - url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/issues?state={state}" - return get_paged_json(url, config) +def list_issues( + project: str, + config: GitLabConfig, + state: str = "opened", + limit: int | None = None, +) -> list[Any]: + query = urllib.parse.urlencode({"state": state}) + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/issues?{query}" + return get_paged_json(url, config, limit=limit) -def get_issue(project: str, issue_iid: str, config: GitLabConfig) -> Any: - url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/issues/{issue_iid}" +def get_issue(project: str, issue_iid: int | str, config: GitLabConfig) -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/issues/{quote_path(str(issue_iid))}" return get_json(url, config) diff --git a/tools/gitlab/src/magpie_gitlab/merge_requests.py b/tools/gitlab/src/magpie_gitlab/merge_requests.py index 16139422f..3649f0640 100644 --- a/tools/gitlab/src/magpie_gitlab/merge_requests.py +++ b/tools/gitlab/src/magpie_gitlab/merge_requests.py @@ -17,30 +17,44 @@ from __future__ import annotations +import urllib.parse from typing import Any -from .client import GitLabConfig, GitLabError, get_json, get_paged_json, quote_path +from .client import GitLabConfig, get_json, get_paged_json, quote_path -def list_mrs(project: str, config: GitLabConfig, state: str = "opened") -> Any: - url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests?state={state}" - return get_paged_json(url, config) +def list_mrs( + project: str, + config: GitLabConfig, + state: str = "opened", + limit: int | None = None, +) -> list[Any]: + query = urllib.parse.urlencode({"state": state}) + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests?{query}" + return get_paged_json(url, config, limit=limit) -def get_mr(project: str, mr_iid: str, config: GitLabConfig) -> Any: - url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}" +def get_mr(project: str, mr_iid: int | str, config: GitLabConfig) -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{quote_path(str(mr_iid))}" return get_json(url, config) -def get_mr_diff(project: str, mr_iid: str, config: GitLabConfig) -> Any: - """Fetch MR changes. Raises if GitLab truncated the diff.""" - url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/changes" - res = get_json(url, config) - if isinstance(res, dict) and res.get("overflow") is True: - raise GitLabError("Merge request diff is truncated (GitLab overflow limit reached)") - return res - - -def get_mr_commits(project: str, mr_iid: str, config: GitLabConfig) -> Any: - url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/commits" - return get_paged_json(url, config) +def get_mr_diff( + project: str, + mr_iid: int | str, + config: GitLabConfig, + limit: int | None = None, +) -> list[Any]: + """Fetch MR diff hunks using the paginated /diffs endpoint (GitLab 15.7+).""" + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{quote_path(str(mr_iid))}/diffs" + return get_paged_json(url, config, limit=limit) + + +def get_mr_commits( + project: str, + mr_iid: int | str, + config: GitLabConfig, + limit: int | None = None, +) -> list[Any]: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{quote_path(str(mr_iid))}/commits" + return get_paged_json(url, config, limit=limit) diff --git a/tools/gitlab/src/magpie_gitlab/pipelines.py b/tools/gitlab/src/magpie_gitlab/pipelines.py index 85dbcaf95..4fa5753e3 100644 --- a/tools/gitlab/src/magpie_gitlab/pipelines.py +++ b/tools/gitlab/src/magpie_gitlab/pipelines.py @@ -22,11 +22,16 @@ from .client import GitLabConfig, get_json, get_paged_json, quote_path -def get_pipeline_status(project: str, pipeline_id: str, config: GitLabConfig) -> Any: - url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/pipelines/{pipeline_id}" +def get_pipeline_status(project: str, pipeline_id: int | str, config: GitLabConfig) -> Any: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/pipelines/{quote_path(str(pipeline_id))}" return get_json(url, config) -def list_mr_pipelines(project: str, mr_iid: str, config: GitLabConfig) -> Any: - url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{mr_iid}/pipelines" - return get_paged_json(url, config) +def list_mr_pipelines( + project: str, + mr_iid: int | str, + config: GitLabConfig, + limit: int | None = None, +) -> list[Any]: + url = f"{config.instance_url}/api/v4/projects/{quote_path(project)}/merge_requests/{quote_path(str(mr_iid))}/pipelines" + return get_paged_json(url, config, limit=limit) diff --git a/tools/gitlab/tests/__init__.py b/tools/gitlab/tests/__init__.py index e69de29bb..2df32291c 100644 --- a/tools/gitlab/tests/__init__.py +++ b/tools/gitlab/tests/__init__.py @@ -0,0 +1,18 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations diff --git a/tools/gitlab/tests/test_cli.py b/tools/gitlab/tests/test_cli.py index 5d46d3973..20ada80fb 100644 --- a/tools/gitlab/tests/test_cli.py +++ b/tools/gitlab/tests/test_cli.py @@ -19,6 +19,8 @@ import json +import pytest + from magpie_gitlab.cli import main from .conftest import build_mock_response @@ -47,6 +49,30 @@ def test_cli_repo_get(mock_urlopen, mock_env, monkeypatch, capsys): assert res["name"] == "repo-test" +def test_cli_mr_diff(mock_urlopen, mock_env, monkeypatch, capsys): + mock_urlopen.return_value = build_mock_response([{"diff": "@@ -1 +1 @@", "new_path": "a.py"}]) + monkeypatch.setattr("sys.argv", ["magpie-gitlab", "mr", "diff", "group/project", "5"]) + + assert main() == 0 + + captured = capsys.readouterr() + res = json.loads(captured.out) + assert len(res) == 1 + assert res[0]["new_path"] == "a.py" + + +def test_cli_mr_commits(mock_urlopen, mock_env, monkeypatch, capsys): + mock_urlopen.return_value = build_mock_response([{"id": "c1", "message": "feat: init"}]) + monkeypatch.setattr("sys.argv", ["magpie-gitlab", "mr", "commits", "group/project", "5"]) + + assert main() == 0 + + captured = capsys.readouterr() + res = json.loads(captured.out) + assert len(res) == 1 + assert res[0]["id"] == "c1" + + def test_cli_mr_pipelines(mock_urlopen, mock_env, monkeypatch, capsys): mock_urlopen.return_value = build_mock_response([{"id": 101, "status": "success"}]) monkeypatch.setattr("sys.argv", ["magpie-gitlab", "mr", "pipelines", "group/project", "5"]) @@ -60,8 +86,20 @@ def test_cli_mr_pipelines(mock_urlopen, mock_env, monkeypatch, capsys): assert res[0]["status"] == "success" +def test_cli_pipeline_status(mock_urlopen, mock_env, monkeypatch, capsys): + mock_urlopen.return_value = build_mock_response({"id": 55, "status": "running"}) + monkeypatch.setattr("sys.argv", ["magpie-gitlab", "pipeline", "status", "group/project", "55"]) + + assert main() == 0 + + captured = capsys.readouterr() + res = json.loads(captured.out) + assert res["id"] == 55 + assert res["status"] == "running" + + def test_cli_issue_list_limit(mock_urlopen, mock_env, monkeypatch, capsys): - mock_urlopen.return_value = build_mock_response([{"id": 1}, {"id": 2}, {"id": 3}]) + mock_urlopen.return_value = build_mock_response([{"id": 1}, {"id": 2}]) monkeypatch.setattr("sys.argv", ["magpie-gitlab", "issue", "list", "group/project", "--limit", "2"]) assert main() == 0 @@ -69,5 +107,37 @@ def test_cli_issue_list_limit(mock_urlopen, mock_env, monkeypatch, capsys): captured = capsys.readouterr() res = json.loads(captured.out) assert len(res) == 2 - assert res[0]["id"] == 1 - assert res[1]["id"] == 2 + + +def test_cli_issue_get_invalid_id_type_fails(monkeypatch): + """Passing a non-integer for an ID positional must fail argument parsing.""" + monkeypatch.setattr("sys.argv", ["magpie-gitlab", "issue", "get", "group/project", "notanint"]) + with pytest.raises(SystemExit) as exc_info: + main() + assert exc_info.value.code != 0 + + +def test_cli_mr_diff_invalid_id_type_fails(monkeypatch): + """Path traversal attempt via ID string must fail argument parsing.""" + monkeypatch.setattr("sys.argv", ["magpie-gitlab", "mr", "diff", "group/project", "../../admin"]) + with pytest.raises(SystemExit) as exc_info: + main() + assert exc_info.value.code != 0 + + +def test_cli_state_invalid_choice_fails(monkeypatch): + """Unsupported state choice must fail argument parsing.""" + monkeypatch.setattr( + "sys.argv", + ["magpie-gitlab", "issue", "list", "group/project", "--state", "invalid_state"], + ) + with pytest.raises(SystemExit) as exc_info: + main() + assert exc_info.value.code != 0 + + +def test_cli_no_args_fails(monkeypatch): + monkeypatch.setattr("sys.argv", ["magpie-gitlab"]) + with pytest.raises(SystemExit) as exc_info: + main() + assert exc_info.value.code != 0 diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py index a0fa3ea11..b89f23d42 100644 --- a/tools/gitlab/tests/test_client.py +++ b/tools/gitlab/tests/test_client.py @@ -24,7 +24,10 @@ import pytest from magpie_gitlab.client import ( + GitLabConfig, GitLabError, + _auth_headers, + _build_opener, _SafeRedirectHandler, get_json, get_paged_json, @@ -44,17 +47,20 @@ def test_load_config_default(monkeypatch): monkeypatch.delenv("GITLAB_INSTANCE_URL", raising=False) monkeypatch.delenv("CI_JOB_TOKEN", raising=False) + monkeypatch.delenv("GITLAB_AUTH_SCHEME", raising=False) monkeypatch.setenv("GITLAB_TOKEN", "token") cfg = load_config() assert cfg.instance_url == "https://gitlab.com" assert cfg.token == "token" assert cfg.token_type == "bearer" + assert cfg.auth_scheme == "" def test_load_config_ci_job_token(monkeypatch): """CI_JOB_TOKEN should be used when GITLAB_TOKEN is absent.""" monkeypatch.delenv("GITLAB_INSTANCE_URL", raising=False) monkeypatch.delenv("GITLAB_TOKEN", raising=False) + monkeypatch.delenv("GITLAB_AUTH_SCHEME", raising=False) monkeypatch.setenv("CI_JOB_TOKEN", "ci-job-tok-456") cfg = load_config() assert cfg.token == "ci-job-tok-456" @@ -64,6 +70,7 @@ def test_load_config_ci_job_token(monkeypatch): def test_load_config_gitlab_token_takes_precedence(monkeypatch): """GITLAB_TOKEN wins when both are set.""" monkeypatch.delenv("GITLAB_INSTANCE_URL", raising=False) + monkeypatch.delenv("GITLAB_AUTH_SCHEME", raising=False) monkeypatch.setenv("GITLAB_TOKEN", "pat-wins") monkeypatch.setenv("CI_JOB_TOKEN", "ci-loses") cfg = load_config() @@ -72,6 +79,7 @@ def test_load_config_gitlab_token_takes_precedence(monkeypatch): def test_load_config_insecure_url(monkeypatch): + monkeypatch.delenv("GITLAB_AUTH_SCHEME", raising=False) monkeypatch.setenv("GITLAB_TOKEN", "token") monkeypatch.setenv("GITLAB_INSTANCE_URL", "http://gitlab.insecure.com") with pytest.raises( @@ -83,6 +91,7 @@ def test_load_config_insecure_url(monkeypatch): def test_load_config_localhost_http_allowed(monkeypatch): """HTTP is allowed for localhost / 127.0.0.1 / ::1 (local dev / testing).""" + monkeypatch.delenv("GITLAB_AUTH_SCHEME", raising=False) monkeypatch.setenv("GITLAB_TOKEN", "token") monkeypatch.setenv("GITLAB_INSTANCE_URL", "http://localhost:8080") cfg = load_config() @@ -95,6 +104,7 @@ def test_load_config_localhost_http_allowed(monkeypatch): def test_load_config_localhost_non_http_rejected(monkeypatch): """Non-HTTP schemes on localhost (ftp, file, etc.) must be rejected.""" + monkeypatch.delenv("GITLAB_AUTH_SCHEME", raising=False) monkeypatch.setenv("GITLAB_TOKEN", "token") monkeypatch.setenv("GITLAB_INSTANCE_URL", "ftp://localhost:21") with pytest.raises( @@ -135,7 +145,138 @@ def test_require(): # --------------------------------------------------------------------------- -# get_json -- authentication headers +# _build_opener (unmocked) +# --------------------------------------------------------------------------- + + +def test_build_opener_installs_safe_redirect_handler(): + opener = _build_opener() + handlers = getattr(opener, "handlers", []) + assert any(isinstance(h, _SafeRedirectHandler) for h in handlers) + + +# --------------------------------------------------------------------------- +# SafeRedirectHandler +# --------------------------------------------------------------------------- + + +def test_safe_redirect_blocks_https_to_http(): + """HTTPS->HTTP downgrade must be blocked.""" + handler = _SafeRedirectHandler() + req = _ur.Request("https://gitlab.example.com/api") + with pytest.raises(GitLabError, match="HTTPS-to-HTTP downgrade"): + handler.redirect_request(req, None, 302, "Found", {}, "http://gitlab.example.com/api") + + +def test_safe_redirect_blocks_cross_origin(): + """Cross-origin host redirect must be blocked.""" + handler = _SafeRedirectHandler() + req = _ur.Request("https://gitlab.example.com/api") + with pytest.raises(GitLabError, match="cross-origin redirect"): + handler.redirect_request(req, None, 302, "Found", {}, "https://evil.example.com/steal") + + +def test_safe_redirect_blocks_port_mismatch(): + """Cross-port redirect on same host must be blocked.""" + handler = _SafeRedirectHandler() + req = _ur.Request("https://gitlab.example.com/api") + with pytest.raises(GitLabError, match="cross-origin redirect"): + handler.redirect_request(req, None, 302, "Found", {}, "https://gitlab.example.com:8443/api") + + +def test_safe_redirect_allows_same_origin(): + """Same-origin same-scheme absolute redirect should be allowed.""" + handler = _SafeRedirectHandler() + req = _ur.Request("https://gitlab.example.com/api/old") + result = handler.redirect_request(req, None, 302, "Found", {}, "https://gitlab.example.com/api/new") + assert result is not None + assert result.full_url == "https://gitlab.example.com/api/new" + + +def test_safe_redirect_allows_relative_same_origin(): + """Relative redirect on same origin should be resolved and allowed.""" + handler = _SafeRedirectHandler() + req = _ur.Request("https://gitlab.example.com/api/v4/projects") + result = handler.redirect_request(req, None, 302, "Found", {}, "/api/v4/projects/1") + assert result is not None + assert result.full_url == "https://gitlab.example.com/api/v4/projects/1" + + +# --------------------------------------------------------------------------- +# _auth_headers +# --------------------------------------------------------------------------- + + +def test_auth_headers_unauthenticated(): + cfg = GitLabConfig(token=None, instance_url="https://gitlab.example.com") + headers = _auth_headers(cfg) + assert headers == {"Accept": "application/json"} + + +def test_auth_headers_glpat_default(): + cfg = GitLabConfig(token="glpat-secret", instance_url="https://gitlab.example.com") + headers = _auth_headers(cfg) + assert headers.get("PRIVATE-TOKEN") == "glpat-secret" + assert "Authorization" not in headers + + +def test_auth_headers_bearer_default(): + cfg = GitLabConfig(token="oauth-token", instance_url="https://gitlab.example.com") + headers = _auth_headers(cfg) + assert headers.get("Authorization") == "Bearer oauth-token" + assert "PRIVATE-TOKEN" not in headers + + +def test_auth_headers_job_token_default(): + cfg = GitLabConfig(token="job-tok", instance_url="https://gitlab.example.com", token_type="job_token") + headers = _auth_headers(cfg) + assert headers.get("JOB-TOKEN") == "job-tok" + + +def test_auth_headers_explicit_scheme_bearer(): + """Explicit GITLAB_AUTH_SCHEME=Bearer overrides glpat- default.""" + cfg = GitLabConfig( + token="glpat-token", + instance_url="https://gitlab.example.com", + auth_scheme="Bearer", + ) + headers = _auth_headers(cfg) + assert headers.get("Authorization") == "Bearer glpat-token" + assert "PRIVATE-TOKEN" not in headers + + +def test_auth_headers_explicit_scheme_private_token(): + cfg = GitLabConfig( + token="custom-pat", + instance_url="https://gitlab.example.com", + auth_scheme="Private-Token", + ) + headers = _auth_headers(cfg) + assert headers.get("PRIVATE-TOKEN") == "custom-pat" + + +def test_auth_headers_explicit_scheme_job_token(): + cfg = GitLabConfig( + token="custom-job-tok", + instance_url="https://gitlab.example.com", + auth_scheme="Job-Token", + ) + headers = _auth_headers(cfg) + assert headers.get("JOB-TOKEN") == "custom-job-tok" + + +def test_auth_headers_invalid_scheme_raises(): + cfg = GitLabConfig( + token="token", + instance_url="https://gitlab.example.com", + auth_scheme="Basic", + ) + with pytest.raises(GitLabError, match="Unsupported GITLAB_AUTH_SCHEME: 'Basic'"): + _auth_headers(cfg) + + +# --------------------------------------------------------------------------- +# get_json # --------------------------------------------------------------------------- @@ -143,6 +284,7 @@ def test_get_json_unauthenticated(mock_urlopen, monkeypatch): """Unauthenticated public reads should succeed with no auth header.""" monkeypatch.delenv("GITLAB_TOKEN", raising=False) monkeypatch.delenv("CI_JOB_TOKEN", raising=False) + monkeypatch.delenv("GITLAB_AUTH_SCHEME", raising=False) monkeypatch.setenv("GITLAB_INSTANCE_URL", "https://gitlab.example.com") mock_urlopen.return_value = build_mock_response({"public": "repo"}) cfg = load_config() @@ -155,30 +297,6 @@ def test_get_json_unauthenticated(mock_urlopen, monkeypatch): assert req.headers.get("Accept") == "application/json" -def test_get_json_private_token(mock_urlopen, mock_env): - """Personal Access Tokens starting with glpat- should use PRIVATE-TOKEN header.""" - mock_urlopen.return_value = build_mock_response({"key": "value"}) - cfg = load_config() - res = get_json("https://gitlab.example.com/api", cfg) - assert res == {"key": "value"} - req = mock_urlopen.call_args[0][0] - assert req.headers.get("Private-token") == "glpat-test123" - assert "Authorization" not in req.headers - - -def test_get_json_bearer_token(mock_urlopen, monkeypatch): - """Tokens not starting with glpat- should use Bearer header.""" - monkeypatch.setenv("GITLAB_TOKEN", "oauth-bearer-token") - monkeypatch.setenv("GITLAB_INSTANCE_URL", "https://gitlab.example.com") - mock_urlopen.return_value = build_mock_response({"auth": "ok"}) - cfg = load_config() - res = get_json("https://gitlab.example.com/api", cfg) - assert res == {"auth": "ok"} - req = mock_urlopen.call_args[0][0] - assert req.headers.get("Authorization") == "Bearer oauth-bearer-token" - assert "Private-token" not in req.headers - - def test_get_json_http_error(mock_urlopen, mock_env): mock_urlopen.side_effect = urllib.error.HTTPError("url", 404, "Not Found", Message(), None) cfg = load_config() @@ -187,27 +305,7 @@ def test_get_json_http_error(mock_urlopen, mock_env): # --------------------------------------------------------------------------- -# get_json -- JOB-TOKEN header -# --------------------------------------------------------------------------- - - -def test_get_json_job_token_header(mock_urlopen, monkeypatch): - """When CI_JOB_TOKEN is used, the request must carry JOB-TOKEN.""" - monkeypatch.delenv("GITLAB_TOKEN", raising=False) - monkeypatch.setenv("CI_JOB_TOKEN", "job-tok-789") - monkeypatch.setenv("GITLAB_INSTANCE_URL", "https://gitlab.example.com") - mock_urlopen.return_value = build_mock_response({"job": "ok"}) - cfg = load_config() - res = get_json("https://gitlab.example.com/api", cfg) - assert res == {"job": "ok"} - req = mock_urlopen.call_args[0][0] - assert req.headers.get("Job-token") == "job-tok-789" - assert "Authorization" not in req.headers - assert "Private-token" not in req.headers - - -# --------------------------------------------------------------------------- -# get_paged_json -- pagination +# get_paged_json -- pagination & bounded limit # --------------------------------------------------------------------------- @@ -225,12 +323,16 @@ def test_get_paged_json_multi_page(mock_urlopen, mock_env): mock_urlopen.side_effect = [page1, page2] cfg = load_config() - items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg) + items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues?state=opened", cfg) assert items == [{"id": 1}, {"id": 2}] assert mock_urlopen.call_count == 2 + # Verify second request preserved query params + req2 = mock_urlopen.call_args_list[1][0][0] + assert "state=opened" in req2.full_url + assert "page=2" in req2.full_url -def test_get_paged_json_max_pages(mock_urlopen, mock_env): +def test_get_paged_json_max_pages(mock_urlopen, mock_env, capsys): """Pagination must halt when max_pages ceiling is reached.""" page1 = build_mock_response([{"id": 1}], headers={"X-Next-Page": "2"}) page2 = build_mock_response([{"id": 2}], headers={"X-Next-Page": "3"}) @@ -241,56 +343,61 @@ def test_get_paged_json_max_pages(mock_urlopen, mock_env): items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg, max_pages=2) assert items == [{"id": 1}, {"id": 2}] assert mock_urlopen.call_count == 2 + err = capsys.readouterr().err + assert "Results capped at 2 items (2 pages)" in err -# --------------------------------------------------------------------------- -# get_project -# --------------------------------------------------------------------------- - +def test_get_paged_json_with_limit_fewer_than_page(mock_urlopen, mock_env, capsys): + """Limit fewer than page size fetches only 1 page, caps items, and emits notice if more exist.""" + data = [{"id": i} for i in range(100)] + mock_urlopen.return_value = build_mock_response(data, headers={"X-Next-Page": "2"}) -def test_get_project(mock_urlopen, mock_env): - mock_urlopen.return_value = build_mock_response({"id": 42, "name": "my-project"}) cfg = load_config() - project = get_project("group/my-project", cfg) - assert project == {"id": 42, "name": "my-project"} - req = mock_urlopen.call_args[0][0] - assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fmy-project" + items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg, limit=5) + assert len(items) == 5 + assert mock_urlopen.call_count == 1 + err = capsys.readouterr().err + assert "Results capped at 5 items; use --limit to fetch more." in err -# --------------------------------------------------------------------------- -# SafeRedirectHandler -# --------------------------------------------------------------------------- +def test_get_paged_json_with_limit_multi_page(mock_urlopen, mock_env): + """Limit spanning multiple pages only fetches the required number of pages.""" + page1 = build_mock_response([{"id": i} for i in range(100)], headers={"X-Next-Page": "2"}) + page2 = build_mock_response([{"id": i} for i in range(100, 200)], headers={"X-Next-Page": "3"}) + mock_urlopen.side_effect = [page1, page2] + cfg = load_config() + items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg, limit=150) + assert len(items) == 150 + assert mock_urlopen.call_count == 2 -def test_safe_redirect_blocks_https_to_http(): - """HTTPS->HTTP downgrade must be blocked.""" - handler = _SafeRedirectHandler() - req = _ur.Request("https://gitlab.example.com/api") - with pytest.raises(GitLabError, match="HTTPS-to-HTTP downgrade"): - handler.redirect_request(req, None, 302, "Found", {}, "http://gitlab.example.com/api") +def test_get_paged_json_non_list_single(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response({"single": "object"}) + cfg = load_config() + items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/resource", cfg) + assert items == [{"single": "object"}] -def test_safe_redirect_blocks_cross_origin(): - """Cross-origin redirect must be blocked.""" - handler = _SafeRedirectHandler() - req = _ur.Request("https://gitlab.example.com/api") - with pytest.raises(GitLabError, match="cross-origin redirect"): - handler.redirect_request(req, None, 302, "Found", {}, "https://evil.example.com/steal") +def test_get_paged_json_non_list_subsequent_raises(mock_urlopen, mock_env): + page1 = build_mock_response([{"id": 1}], headers={"X-Next-Page": "2"}) + page2 = build_mock_response({"error": "invalid"}, headers={"X-Next-Page": ""}) + mock_urlopen.side_effect = [page1, page2] -def test_safe_redirect_allows_same_origin(): - """Same-origin same-scheme absolute redirect should be allowed.""" - handler = _SafeRedirectHandler() - req = _ur.Request("https://gitlab.example.com/api/old") - result = handler.redirect_request(req, None, 302, "Found", {}, "https://gitlab.example.com/api/new") - assert result is not None - assert result.full_url == "https://gitlab.example.com/api/new" + cfg = load_config() + with pytest.raises(GitLabError, match="Unexpected non-list response"): + get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg) -def test_safe_redirect_allows_relative_same_origin(): - """Relative redirect on same origin should be resolved and allowed.""" - handler = _SafeRedirectHandler() - req = _ur.Request("https://gitlab.example.com/api/v4/projects") - result = handler.redirect_request(req, None, 302, "Found", {}, "/api/v4/projects/1") - assert result is not None - assert result.full_url == "https://gitlab.example.com/api/v4/projects/1" +# --------------------------------------------------------------------------- +# get_project +# --------------------------------------------------------------------------- + + +def test_get_project(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response({"id": 42, "name": "my-project"}) + cfg = load_config() + project = get_project("group/my-project", cfg) + assert project == {"id": 42, "name": "my-project"} + req = mock_urlopen.call_args[0][0] + assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fmy-project" diff --git a/tools/gitlab/tests/test_issues.py b/tools/gitlab/tests/test_issues.py index a9f524b7c..4a13c4095 100644 --- a/tools/gitlab/tests/test_issues.py +++ b/tools/gitlab/tests/test_issues.py @@ -37,10 +37,17 @@ def test_list_issues(mock_urlopen, mock_env): ) +def test_list_issues_limit(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response([{"id": 1}, {"id": 2}]) + cfg = load_config() + res = list_issues("group/project", cfg, limit=2) + assert len(res) == 2 + + def test_get_issue(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response({"id": 1, "title": "Issue 1"}) cfg = load_config() - res = get_issue("group/project", "1", cfg) + res = get_issue("group/project", 1, cfg) assert res["id"] == 1 req = mock_urlopen.call_args[0][0] diff --git a/tools/gitlab/tests/test_merge_requests.py b/tools/gitlab/tests/test_merge_requests.py index 2d5a7d5dd..20e8f8b7f 100644 --- a/tools/gitlab/tests/test_merge_requests.py +++ b/tools/gitlab/tests/test_merge_requests.py @@ -17,9 +17,7 @@ from __future__ import annotations -import pytest - -from magpie_gitlab.client import GitLabError, load_config +from magpie_gitlab.client import load_config from magpie_gitlab.merge_requests import get_mr, get_mr_commits, get_mr_diff, list_mrs from .conftest import build_mock_response @@ -37,38 +35,47 @@ def test_list_mrs(mock_urlopen, mock_env): ) +def test_list_mrs_limit(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response([{"id": 1}, {"id": 2}]) + cfg = load_config() + res = list_mrs("group/project", cfg, limit=2) + assert len(res) == 2 + + def test_get_mr(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response({"id": 1}) cfg = load_config() - res = get_mr("group/project", "1", cfg) + res = get_mr("group/project", 1, cfg) assert res["id"] == 1 req = mock_urlopen.call_args[0][0] assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1" def test_get_mr_diff(mock_urlopen, mock_env): - mock_urlopen.return_value = build_mock_response({"changes": []}) + """MR diff must use the paginated /diffs endpoint.""" + mock_urlopen.return_value = build_mock_response([{"diff": "@@ -1 +1 @@\n-a\n+b", "new_path": "foo.py"}]) cfg = load_config() - res = get_mr_diff("group/project", "1", cfg) - assert "changes" in res + res = get_mr_diff("group/project", 1, cfg) + assert len(res) == 1 + assert res[0]["new_path"] == "foo.py" req = mock_urlopen.call_args[0][0] assert ( - req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/changes" + req.full_url + == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/diffs?per_page=100" ) -def test_get_mr_diff_overflow(mock_urlopen, mock_env): - """When GitLab returns overflow: true the diff is incomplete.""" - mock_urlopen.return_value = build_mock_response({"changes": [], "overflow": True}) +def test_get_mr_diff_limit(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response([{"diff": "chunk 1"}, {"diff": "chunk 2"}]) cfg = load_config() - with pytest.raises(GitLabError, match="truncated"): - get_mr_diff("group/project", "1", cfg) + res = get_mr_diff("group/project", 1, cfg, limit=2) + assert len(res) == 2 def test_get_mr_commits(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response([{"id": "abc"}]) cfg = load_config() - res = get_mr_commits("group/project", "1", cfg) + res = get_mr_commits("group/project", 1, cfg) assert len(res) == 1 req = mock_urlopen.call_args[0][0] assert ( diff --git a/tools/gitlab/tests/test_pipelines.py b/tools/gitlab/tests/test_pipelines.py index ac3127afb..b83c7db80 100644 --- a/tools/gitlab/tests/test_pipelines.py +++ b/tools/gitlab/tests/test_pipelines.py @@ -26,7 +26,7 @@ def test_get_pipeline_status(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response({"id": 1, "status": "success"}) cfg = load_config() - res = get_pipeline_status("group/project", "1", cfg) + res = get_pipeline_status("group/project", 1, cfg) assert res["status"] == "success" req = mock_urlopen.call_args[0][0] assert req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/pipelines/1" @@ -35,10 +35,17 @@ def test_get_pipeline_status(mock_urlopen, mock_env): def test_list_mr_pipelines(mock_urlopen, mock_env): mock_urlopen.return_value = build_mock_response([{"id": 1, "status": "success"}]) cfg = load_config() - res = list_mr_pipelines("group/project", "1", cfg) + res = list_mr_pipelines("group/project", 1, cfg) assert len(res) == 1 req = mock_urlopen.call_args[0][0] assert ( req.full_url == "https://gitlab.example.com/api/v4/projects/group%2Fproject/merge_requests/1/pipelines?per_page=100" ) + + +def test_list_mr_pipelines_limit(mock_urlopen, mock_env): + mock_urlopen.return_value = build_mock_response([{"id": 1}, {"id": 2}]) + cfg = load_config() + res = list_mr_pipelines("group/project", 1, cfg, limit=2) + assert len(res) == 2 diff --git a/tools/gitlab/tool.md b/tools/gitlab/tool.md index 9e722e43a..6079d7892 100644 --- a/tools/gitlab/tool.md +++ b/tools/gitlab/tool.md @@ -7,22 +7,36 @@ - [GitLab Tool Adapter](#gitlab-tool-adapter) - [Operations catalogue](#operations-catalogue) + - [Options and flags](#options-and-flags) + - [Confidentiality](#confidentiality) # GitLab Tool Adapter -Operations catalogue mapping for GitLab tracker and merge requests. +Operations catalogue mapping for GitLab tracker, source control, and merge requests. ## Operations catalogue | Operation | GitLab command | | --- | --- | +| Read repository metadata | `magpie-gitlab repo get ` | | Read issue body | `magpie-gitlab issue get ` | | List issues | `magpie-gitlab issue list ` | | Read MR | `magpie-gitlab mr get ` | | MR Diff | `magpie-gitlab mr diff ` | -| CI Status | `magpie-gitlab pipeline status ` | +| MR Commits | `magpie-gitlab mr commits ` | +| List MR Pipelines | `magpie-gitlab mr pipelines ` | +| CI Pipeline Status | `magpie-gitlab pipeline status ` | + +## Options and flags + +- `--limit `: Cap the total number of items returned for paginated endpoints (`issue list`, `mr list`, `mr diff`, `mr commits`, `mr pipelines`). Automatically calculates and requests only the required number of pages (`ceil(limit / 100)`). +- `--state `: Filter items by state: + - For `issue list`: `opened` (default), `closed`, `all`. + - For `mr list`: `opened` (default), `closed`, `locked`, `merged`, `all`. + +## Confidentiality *Confidentiality Note*: Never log personal access tokens. All payload bodies are handled purely in memory and output in JSON format. From 1efbfea97ef8802b0e6bd5d6973571018ea45a2a Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Fri, 25 Sep 2026 21:51:42 +0530 Subject: [PATCH 18/21] docs(vendor-neutrality): restore skill assessment counts in generated score block --- docs/vendor-neutrality.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/vendor-neutrality.md b/docs/vendor-neutrality.md index 74f8d0150..90895c32e 100644 --- a/docs/vendor-neutrality.md +++ b/docs/vendor-neutrality.md @@ -582,15 +582,15 @@ generated block below. | `contract:project-metadata` | ✅ | single-org | ASF | single-organisation capability (ASF); no vendor choice to make | | `contract:security-cross-ref` | ❌ | vendor-backed | OSV.dev | only 1 backend vendor (OSV.dev); needs 1 more | -**Per-skill assessment: 0/0 skills carry no vendor lock-in.** A skill is *capability-pure* when it names no backend at all, *portable* when every backend it names has an alternative (its contract is green), and *vendor-coupled* only when it reaches for a backend that is the sole implementation of a capability. +**Per-skill assessment: 75/75 skills carry no vendor lock-in.** A skill is *capability-pure* when it names no backend at all, *portable* when every backend it names has an alternative (its contract is green), and *vendor-coupled* only when it reaches for a backend that is the sole implementation of a capability. | Skill neutrality | Count | |---|---| -| capability-pure (names no backend) | 0 | -| portable (named backends are swappable) | 0 | +| capability-pure (names no backend) | 15 | +| portable (named backends are swappable) | 60 | | vendor-coupled (sole-backend dependency) | 0 | -Organization scope (declared, orthogonal to vendor): . +Organization scope (declared, orthogonal to vendor): ASF = 14, agnostic = 61. **LLM / agent-integration neutrality** From 2a5d060f221f9aae1065a36af8b47e7c81171f41 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Sun, 27 Sep 2026 09:19:12 +0530 Subject: [PATCH 19/21] fix(gitlab): address maintainer review on auth scheme, limit pagination, fixtures, and docs --- .github/labeler.yml | 193 +++++++++++++++++++++++ tools/gitlab/README.md | 14 +- tools/gitlab/src/magpie_gitlab/client.py | 20 +-- tools/gitlab/tests/conftest.py | 2 + tools/gitlab/tests/test_client.py | 40 +++-- tools/gitlab/tool.md | 1 + 6 files changed, 235 insertions(+), 35 deletions(-) create mode 100644 .github/labeler.yml diff --git a/.github/labeler.yml b/.github/labeler.yml new file mode 100644 index 000000000..d32ba528b --- /dev/null +++ b/.github/labeler.yml @@ -0,0 +1,193 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# GENERATED by tools/dev/generate-labeler-config.py from the +# `**Capability:**` line of every tools//README.md. Do not edit by +# hand; change the README and let the prek hook regenerate this file. +--- +changed-files-labels-limit: 8 + +contract:change-request: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/bitbucket/**' + - 'tools/change-request/**' + - 'tools/github/**' + - 'tools/gitlab/**' + - 'tools/jira-patch/**' + - 'tools/mail-patch/**' + +contract:cve-authority: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/cve-org/**' + - 'tools/cve-tool/**' + - 'tools/cve-tool-vulnogram/**' + +contract:mail-archive: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/gmail/**' + - 'tools/mail-archive/**' + - 'tools/ponymail/**' + - 'tools/sourcehut/**' + +contract:mail-create: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/gmail/**' + - 'tools/maildir/**' + +contract:mail-source: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/gmail/**' + - 'tools/mail-source/**' + - 'tools/maildir/**' + - 'tools/ponymail/**' + +contract:project-metadata: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/apache-projects/**' + +contract:report-relay: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/forwarder-relay/**' + +contract:scan-format: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/scan-format/**' + +contract:security-cross-ref: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/osv/**' + +contract:source-control: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/asf-svn/**' + - 'tools/fossil/**' + - 'tools/github/**' + - 'tools/gitlab/**' + - 'tools/sourcehut/**' + - 'tools/vcs/**' + +contract:tracker: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/bitbucket/**' + - 'tools/fossil/**' + - 'tools/github/**' + - 'tools/github-body-field/**' + - 'tools/github-rollup/**' + - 'tools/gitlab/**' + - 'tools/jira/**' + - 'tools/sourcehut/**' + +substrate:action-guard: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/agent-guard/**' + +substrate:analytics: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/dashboard-generator/**' + - 'tools/pr-management-stats/**' + - 'tools/preflight-audit/**' + - 'tools/security-tracker-stats-dashboard/**' + - 'tools/skill-token-count/**' + - 'tools/spec-inventory/**' + - 'tools/spec-status-index/**' + - 'tools/vendor-neutrality-score/**' + +substrate:framework-dev: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/dev/**' + - 'tools/pilot-report-validator/**' + - 'tools/skill-and-tool-validator/**' + - 'tools/skill-reconciler-diff/**' + - 'tools/skill-token-count/**' + - 'tools/spec-inventory/**' + - 'tools/spec-status-index/**' + - 'tools/spec-validator/**' + - 'tools/symlink-lint/**' + - 'tools/vendor-neutrality-score/**' + - changed-files: + - all-globs-to-any-file: + - 'tools/skill-evals/**' + - '!tools/skill-evals/evals/**' + - changed-files: + - all-globs-to-any-file: + - 'tools/spec-loop/**' + - '!tools/spec-loop/specs/**' + - '!tools/spec-loop/.last-sync' + +substrate:privacy: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/privacy-llm/**' + +substrate:release: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/reproducible-archive/**' + +substrate:review: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/adversarial-review/**' + +substrate:sandbox: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/agent-isolation/**' + - 'tools/container-gateway/**' + - 'tools/egress-gateway/**' + - 'tools/permission-audit/**' + - 'tools/probe-templates/**' + - 'tools/sandbox-lint/**' + - 'tools/vetted-ops/**' + +substrate:setup: + - any: + - changed-files: + - any-glob-to-any-file: + - 'tools/setup-preflight/**' diff --git a/tools/gitlab/README.md b/tools/gitlab/README.md index f8b8a3f44..36636f4ac 100644 --- a/tools/gitlab/README.md +++ b/tools/gitlab/README.md @@ -23,17 +23,15 @@ **Vendor:** GitLab -GitLab forge, issue tracker, and merge request bridge for Apache Magpie. -Provides 100% offline-tested, deterministic API access to GitLab instances, -following strict vendor-neutrality rules. +Read-only client for the GitLab REST API v4. This bridge implements a `partial` read-only foundation for repository metadata context under `contract:source-control`, issue listing and fetching under `contract:tracker`, and merge request discovery, diffs, commits, and -CI pipeline status under `contract:change-request`. Partial adapters may -implement named contract verbs, but they do not satisfy the complete contract -and must not be advertised as complete/selectable backends. Write operations -and issue/MR mutations remain out of scope for this foundation. +CI pipeline status under `contract:change-request`. +Partial adapters may implement named contract verbs, but they do not satisfy +the complete contract and must not be advertised as complete/selectable backends. +Write operations and issue/MR mutations remain out of scope for this foundation. ## Prerequisites @@ -49,7 +47,7 @@ and issue/MR mutations remain out of scope for this foundation. ## Configuration -Set `GITLAB_TOKEN` in your environment (or `user.md`): +Set `GITLAB_TOKEN` in your environment: ```bash export GITLAB_TOKEN="glpat-..." diff --git a/tools/gitlab/src/magpie_gitlab/client.py b/tools/gitlab/src/magpie_gitlab/client.py index dcb23f3e2..e02d4e705 100644 --- a/tools/gitlab/src/magpie_gitlab/client.py +++ b/tools/gitlab/src/magpie_gitlab/client.py @@ -118,7 +118,7 @@ def load_config() -> GitLabConfig: if gitlab_token: token = gitlab_token - token_type = "bearer" + token_type = "private-token" if token.startswith("glpat-") else "bearer" elif ci_job_token: token = ci_job_token token_type = "job_token" @@ -141,12 +141,6 @@ def load_config() -> GitLabConfig: # --------------------------------------------------------------------------- -def require(value: str | None, name: str) -> str: - if not value: - raise GitLabError(f"{name} is required") - return value - - def quote_path(value: str) -> str: return urllib.parse.quote(value, safe="") @@ -163,7 +157,7 @@ def _auth_headers(config: GitLabConfig) -> dict[str, str]: headers["PRIVATE-TOKEN"] = config.token elif scheme == "bearer": headers["Authorization"] = f"Bearer {config.token}" - elif scheme in ("job-token", "job_token"): + elif scheme in ("job-token", "job_token", "jobtoken"): headers["JOB-TOKEN"] = config.token else: raise GitLabError(f"Unsupported GITLAB_AUTH_SCHEME: '{config.auth_scheme}'") @@ -216,8 +210,7 @@ def get_paged_json( target_pages: int | None = max_pages if limit is not None: - pages_needed = max(1, math.ceil(limit / 100)) - target_pages = min(pages_needed, max_pages) if max_pages is not None else pages_needed + target_pages = max(1, math.ceil(limit / 100)) opener = _build_opener() while current_url: @@ -233,16 +226,11 @@ def get_paged_json( raise GitLabError("Unexpected non-list response during pagination") pages_fetched += 1 - next_page = response.headers.get("X-Next-Page") if hasattr(response, "headers") else None + next_page = response.headers.get("X-Next-Page") has_more = isinstance(next_page, str) and bool(next_page.strip()) if limit is not None and len(items) >= limit: items = items[:limit] - if has_more: - print( - f"[magpie-gitlab] Note: Results capped at {len(items)} items; use --limit to fetch more.", - file=sys.stderr, - ) break if target_pages is not None and pages_fetched >= target_pages: diff --git a/tools/gitlab/tests/conftest.py b/tools/gitlab/tests/conftest.py index be6a5d0be..40262c7c6 100644 --- a/tools/gitlab/tests/conftest.py +++ b/tools/gitlab/tests/conftest.py @@ -61,4 +61,6 @@ def build_mock_response( @pytest.fixture def mock_env(monkeypatch): monkeypatch.setenv("GITLAB_TOKEN", "glpat-test123") + monkeypatch.delenv("GITLAB_AUTH_SCHEME", raising=False) + monkeypatch.delenv("CI_JOB_TOKEN", raising=False) monkeypatch.setenv("GITLAB_INSTANCE_URL", "https://gitlab.example.com") diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py index b89f23d42..664b0a78d 100644 --- a/tools/gitlab/tests/test_client.py +++ b/tools/gitlab/tests/test_client.py @@ -34,7 +34,6 @@ get_project, load_config, quote_path, - require, ) from .conftest import build_mock_response @@ -128,7 +127,7 @@ def test_load_config_custom(mock_env): # --------------------------------------------------------------------------- -# quote_path / require +# quote_path # --------------------------------------------------------------------------- @@ -136,14 +135,6 @@ def test_quote_path(): assert quote_path("group/project") == "group%2Fproject" -def test_require(): - assert require("val", "VAR") == "val" - with pytest.raises(GitLabError, match="VAR is required"): - require(None, "VAR") - with pytest.raises(GitLabError, match="VAR is required"): - require("", "VAR") - - # --------------------------------------------------------------------------- # _build_opener (unmocked) # --------------------------------------------------------------------------- @@ -265,6 +256,16 @@ def test_auth_headers_explicit_scheme_job_token(): assert headers.get("JOB-TOKEN") == "custom-job-tok" +def test_auth_headers_explicit_scheme_jobtoken_no_hyphen(): + cfg = GitLabConfig( + token="custom-job-tok", + instance_url="https://gitlab.example.com", + auth_scheme="JobToken", + ) + headers = _auth_headers(cfg) + assert headers.get("JOB-TOKEN") == "custom-job-tok" + + def test_auth_headers_invalid_scheme_raises(): cfg = GitLabConfig( token="token", @@ -357,7 +358,7 @@ def test_get_paged_json_with_limit_fewer_than_page(mock_urlopen, mock_env, capsy assert len(items) == 5 assert mock_urlopen.call_count == 1 err = capsys.readouterr().err - assert "Results capped at 5 items; use --limit to fetch more." in err + assert err == "" def test_get_paged_json_with_limit_multi_page(mock_urlopen, mock_env): @@ -389,6 +390,23 @@ def test_get_paged_json_non_list_subsequent_raises(mock_urlopen, mock_env): get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg) +def test_get_paged_json_limit_gt_1000(mock_urlopen, mock_env): + """Limit > 1000 should bypass the max_pages=10 default and fetch required pages.""" + pages = [] + # limit=1050 means we need 11 pages (100 items per page). + for i in range(1, 12): + next_page = str(i + 1) if i < 11 else "" + pages.append(build_mock_response([{"id": j} for j in range(100)], headers={"X-Next-Page": next_page})) + + mock_urlopen.side_effect = pages + + cfg = load_config() + # default max_pages is 10, but limit=1050 should override target_pages to 11 + items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg, limit=1050) + assert len(items) == 1050 + assert mock_urlopen.call_count == 11 + + # --------------------------------------------------------------------------- # get_project # --------------------------------------------------------------------------- diff --git a/tools/gitlab/tool.md b/tools/gitlab/tool.md index 6079d7892..00f10865d 100644 --- a/tools/gitlab/tool.md +++ b/tools/gitlab/tool.md @@ -23,6 +23,7 @@ Operations catalogue mapping for GitLab tracker, source control, and merge reque | Read repository metadata | `magpie-gitlab repo get ` | | Read issue body | `magpie-gitlab issue get ` | | List issues | `magpie-gitlab issue list ` | +| List MRs | `magpie-gitlab mr list ` | | Read MR | `magpie-gitlab mr get ` | | MR Diff | `magpie-gitlab mr diff ` | | MR Commits | `magpie-gitlab mr commits ` | From 1c0ee2c868583124ce11a739665e42532b1d50fb Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Sun, 27 Sep 2026 09:30:44 +0530 Subject: [PATCH 20/21] fix(dev): make test_identical_text_inside_generated_regions_is_invisible robust against skill iteration order --- tools/dev/tests/test_check_duplication.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tools/dev/tests/test_check_duplication.py b/tools/dev/tests/test_check_duplication.py index ac318214e..e004a2af3 100644 --- a/tools/dev/tests/test_check_duplication.py +++ b/tools/dev/tests/test_check_duplication.py @@ -104,11 +104,21 @@ def test_identical_text_inside_generated_regions_is_invisible() -> None: `strip_generated_regions` exists. Reuses a real propagated block from the live tree so the test tracks the real marker text, not a hand-written stand-in.""" - live_skill = next((REPO / "skills").glob("*/SKILL.md")) - text = live_skill.read_text() - match = MOD.PREFLIGHT_RE.search(text) - assert match, "expected the live skill to carry the auto pre-flight block" - block = match.group(0) + block = None + for pattern in ("skills/*/SKILL.md", "plugins/magpie-*/skills/*/SKILL.md", "plugins/magpie-*/skills/*/*.md"): + for path in REPO.glob(pattern): + try: + match = MOD.PREFLIGHT_RE.search(path.read_text(encoding="utf-8")) + if match: + block = match.group(0) + break + except OSError: + continue + if block: + break + if not block: + preflight_content = (REPO / "tools" / "dev" / "preflight-block.md").read_text(encoding="utf-8") + block = f"{MOD.PREFLIGHT_BEGIN}\n{preflight_content}\n{MOD.PREFLIGHT_END}\n" paragraphs = MOD.extract_paragraphs( Path("virtual.md"), text=f"# Heading\n\n{block}\n## Next\n\nSome unrelated text.\n" From 3bcfa269ea2886752da1b55418f3c5be322ac71c Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Sun, 27 Sep 2026 09:39:08 +0530 Subject: [PATCH 21/21] style(dev): format test_check_duplication.py with ruff --- tools/dev/tests/test_check_duplication.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/dev/tests/test_check_duplication.py b/tools/dev/tests/test_check_duplication.py index e004a2af3..1531ed38c 100644 --- a/tools/dev/tests/test_check_duplication.py +++ b/tools/dev/tests/test_check_duplication.py @@ -105,7 +105,11 @@ def test_identical_text_inside_generated_regions_is_invisible() -> None: live tree so the test tracks the real marker text, not a hand-written stand-in.""" block = None - for pattern in ("skills/*/SKILL.md", "plugins/magpie-*/skills/*/SKILL.md", "plugins/magpie-*/skills/*/*.md"): + for pattern in ( + "skills/*/SKILL.md", + "plugins/magpie-*/skills/*/SKILL.md", + "plugins/magpie-*/skills/*/*.md", + ): for path in REPO.glob(pattern): try: match = MOD.PREFLIGHT_RE.search(path.read_text(encoding="utf-8"))