From e39b6e4fa781429c4007453691395d54919b9cda Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Thu, 20 Aug 2026 13:15:41 +0300 Subject: [PATCH 1/3] mcp: add lab (runtime) queries Add list_labs, backed by the dashboard /metrics/ endpoint, returning the labs reporting to KernelCI with their build, boot and test counts for the last N days. Those names feed a new lab filter on list_builds, list_boots and list_tests, matching the top-level 'lab' field on boots and tests and 'misc.lab'/'misc.runtime' on builds. The dashboard has no server-side lab filter, so the filter narrows the fetched page rather than the request. Keep the 'labs' key in the compact get_summary response, which already aggregates per lab, and document data.runtime and data.platform in the list_nodes docstring. Reported-by: Yogesh Lal Signed-off-by: Denys Fedoryshchenko Signed-off-by: Ben Copeland --- docs/mcp.md | 25 ++++++++ kcidev/api.py | 10 +++ kcidev/libs/dashboard.py | 12 ++++ kcidev/mcp/tools_dashboard.py | 94 +++++++++++++++++++++------ kcidev/mcp/tools_maestro.py | 26 +++++--- tests/test_mcp_tools_dashboard.py | 101 ++++++++++++++++++++++++++++++ 6 files changed, 240 insertions(+), 28 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index eed0e3e..98a927e 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -59,3 +59,28 @@ compact aggregates unless `detail=true` is passed, and the list tools paginate (default `limit` of 20) and accept a `fields` list to return only the named keys per entry. Prefer `status`/`arch` filters, small limits and field projection when exploring large trees. + +## Querying a single lab + +To look at one lab (test runtime) rather than a whole tree, start from +`list_labs`, which returns the labs reporting to KernelCI with their +build, boot and test counts for the last N days. Those names are then +usable as: + +- the `lab` filter of `list_builds`, `list_boots` and `list_tests`, + which narrows a commit's results to that lab; +- the `data.runtime` filter of `list_nodes`, for example + `list_nodes(filters=["kind=job", "data.runtime=lava-collabora", + "data.platform__re=^qcom"])`. Maestro applies this filter server-side, + so it is the cheapest way to ask what one lab is doing with a family + of boards. + +For the status of a lab rather than its individual results, `get_summary` +and `get_hardware_summary` both carry a per-lab breakdown of pass/fail +counts under `summary.
.labs`, which answers "how is this tree or +platform doing in lab X" in a single call. + +The dashboard has no server-side lab filter, so the `lab` option of the +list tools is applied to the fetched page after the request. It shrinks +the response, not the query: `total` counts entries before filtering and +`matched` after. diff --git a/kcidev/api.py b/kcidev/api.py index d269b85..ccfd030 100644 --- a/kcidev/api.py +++ b/kcidev/api.py @@ -39,6 +39,7 @@ dashboard_fetch_issue_list, dashboard_fetch_issue_tests, dashboard_fetch_issues_extra, + dashboard_fetch_metrics, dashboard_fetch_summary, dashboard_fetch_test, dashboard_fetch_tests, @@ -735,6 +736,15 @@ def get_tree_list(self, origin, days=7): days, ) + def get_metrics(self, start_days_ago=7, end_days_ago=0): + return self._dashboard_request( + "Dashboard metrics request failed", + dashboard_fetch_metrics, + True, + start_days_ago, + end_days_ago, + ) + def get_hardware_list(self, origin): return self._dashboard_request( "Dashboard hardware list request failed", diff --git a/kcidev/libs/dashboard.py b/kcidev/libs/dashboard.py index 054d92e..9114263 100644 --- a/kcidev/libs/dashboard.py +++ b/kcidev/libs/dashboard.py @@ -316,6 +316,18 @@ def dashboard_fetch_tree_list(origin, use_json, days=7): return dashboard_api_fetch("tree", params, use_json) +def dashboard_fetch_metrics(use_json, start_days_ago=7, end_days_ago=0): + """Fetch global KernelCI metrics, including per-lab result counts.""" + params = { + "start_days_ago": start_days_ago, + "end_days_ago": end_days_ago, + } + logging.info( + f"Fetching metrics for days {start_days_ago} to {end_days_ago} days ago" + ) + return dashboard_api_fetch("metrics/", params, use_json) + + def dashboard_fetch_hardware_list(origin, use_json): # TODO: add date filter now = datetime.today() diff --git a/kcidev/mcp/tools_dashboard.py b/kcidev/mcp/tools_dashboard.py index 7f05426..5529c0b 100644 --- a/kcidev/mcp/tools_dashboard.py +++ b/kcidev/mcp/tools_dashboard.py @@ -16,13 +16,27 @@ def _current_client(): return _active_client.get() or KernelCIClient() -def _page(data, key, status, limit, offset, fields=None): +def _entry_labs(item): + """Return the lab/runtime names an entry reports, lowercased. + + Boots and tests carry a top-level 'lab'; builds report the same + information as 'misc.lab' and 'misc.runtime'. + """ + misc = item.get("misc") or {} + names = (item.get("lab"), misc.get("lab"), misc.get("runtime")) + return {name.lower() for name in names if isinstance(name, str)} + + +def _page(data, key, status, limit, offset, fields=None, lab=None): check_page_bounds(limit, offset) items = data[key] if isinstance(data, dict) else data total = len(items) if status: status_filter = StatusFilter(checked_status(status)) items = [item for item in items if status_filter.matches(item)] + if lab: + wanted = lab.lower() + items = [item for item in items if wanted in _entry_labs(item)] page = items[offset : offset + limit] if fields: page = [{k: item[k] for k in fields if k in item} for item in page] @@ -46,7 +60,13 @@ def list_trees(origin: str = "maestro", days: int = 7): return _current_client().get_tree_list(origin, days) -_COMPACT_SUMMARY_KEYS = ("status", "architectures", "issues", "failed_platforms") +_COMPACT_SUMMARY_KEYS = ( + "status", + "architectures", + "labs", + "issues", + "failed_platforms", +) @tool_errors @@ -137,16 +157,20 @@ def list_builds( start_date: str | None = None, end_date: str | None = None, status: str | None = None, + lab: str | None = None, limit: int = 20, offset: int = 0, fields: list[str] | None = None, ): """List kernel builds for one commit of a tree. - Optional filters: arch (e.g. 'arm64'), tree name, ISO date range, and - status ('pass', 'fail', 'inconclusive' or 'all'). Results are paginated with - limit/offset; the response carries 'total' (before status filtering) - and 'matched' counts so you know whether to fetch further pages; + Optional filters: arch (e.g. 'arm64'), tree name, ISO date range, + status ('pass', 'fail', 'inconclusive' or 'all'), and lab, the lab or + runtime that produced the build (builds report this as 'misc.lab' + and 'misc.runtime', for example 'maestro' or 'k8s-all'); use + list_labs to find valid names. Results are paginated with + limit/offset; the response carries 'total' (before filtering) and + 'matched' counts so you know whether to fetch further pages; fields projects each entry to only those keys. Returns build entries with ids usable with get_build. """ @@ -154,7 +178,7 @@ def list_builds( data = _current_client().get_builds( origin, giturl, branch, commit, arch, tree, start_date, end_date ) - return _page(data, "builds", status, limit, offset, fields) + return _page(data, "builds", status, limit, offset, fields, lab) @tool_errors @@ -169,24 +193,28 @@ def list_boots( end_date: str | None = None, boot_origin: str | None = None, status: str | None = None, + lab: str | None = None, limit: int = 20, offset: int = 0, fields: list[str] | None = None, ): """List boot test results for one commit of a tree. - Optional filters: arch, tree name, ISO date range, boot origin, and - status ('pass', 'fail', 'inconclusive' or 'all'). Results are paginated with - limit/offset; the response carries 'total' (before status filtering) - and 'matched' counts so you know whether to fetch further pages; - fields projects each entry to only those keys. + Optional filters: arch, tree name, ISO date range, boot origin, + status ('pass', 'fail', 'inconclusive' or 'all'), and lab, the lab or + runtime that ran the boot (for example 'lava-collabora'); use + list_labs to find valid names, or get_summary, whose per-section + 'labs' counts show which labs ran this commit at all. Results are + paginated with limit/offset; the response carries 'total' (before + filtering) and 'matched' counts so you know whether to fetch further + pages; fields projects each entry to only those keys. Returns boot entries with ids usable with get_test. """ check_page_args(status, limit, offset) data = _current_client().get_boots( origin, giturl, branch, commit, arch, tree, start_date, end_date, boot_origin ) - return _page(data, "boots", status, limit, offset, fields) + return _page(data, "boots", status, limit, offset, fields, lab) @tool_errors @@ -200,25 +228,29 @@ def list_tests( start_date: str | None = None, end_date: str | None = None, status: str | None = None, + lab: str | None = None, limit: int = 20, offset: int = 0, fields: list[str] | None = None, ): """List test results for one commit of a tree. - Optional filters: arch, tree name, ISO date range, and status ('pass', - 'fail', 'inconclusive' or 'all'). A full commit can carry tens of thousands - of tests, so filter by status and paginate with limit/offset; the - response carries 'total' (before status filtering) and 'matched' - counts so you know whether to fetch further pages; fields projects - each entry to only those keys. + Optional filters: arch, tree name, ISO date range, status ('pass', + 'fail', 'inconclusive' or 'all'), and lab, the lab or runtime that ran the + test (for example 'lava-collabora'); use list_labs to find valid + names, or get_summary, whose per-section 'labs' counts show which + labs ran this commit at all. A full commit can carry tens of + thousands of tests, so filter by lab and status and paginate with + limit/offset; the response carries 'total' (before filtering) and + 'matched' counts so you know whether to fetch further pages; fields + projects each entry to only those keys. Returns test entries with ids usable with get_test. """ check_page_args(status, limit, offset) data = _current_client().get_tests( origin, giturl, branch, commit, arch, tree, start_date, end_date ) - return _page(data, "tests", status, limit, offset, fields) + return _page(data, "tests", status, limit, offset, fields, lab) @tool_errors @@ -285,6 +317,24 @@ def get_build_issues(build_id: str): return _current_client().get_build_issues(build_id) +def list_labs(days: int = 7): + """List the labs (test runtimes) reporting to KernelCI. + + Returns each lab name with how many builds, boots and tests it + reported over the last N days, so you can pick a valid lab name + without scanning result listings. The names are usable as the 'lab' + filter of list_builds, list_boots and list_tests, and as the + 'data.runtime' filter of list_nodes. Counts cover all origins and + trees; for the labs that ran one specific tree or platform, use the + per-section 'labs' counts of get_summary or get_hardware_summary. + """ + data = _current_client().get_metrics(start_days_ago=days) + labs = data.get("lab_maps") if isinstance(data, dict) else None + if not isinstance(labs, dict): + raise KciDevError("dashboard metrics response carried no lab data") + return {"labs": labs, "days": days} + + @tool_errors def list_hardware(origin: str = "maestro"): """List hardware platforms with results over the last 7 days. @@ -299,6 +349,9 @@ def get_hardware_summary(name: str, origin: str = "maestro"): """Get the build/boot/test summary for one hardware platform. Covers the last 7 days. Use list_hardware to find platform names. + Each build/boot/test section carries a 'labs' breakdown of status + counts per lab, so this answers "how is this platform doing in lab + X" in one call, without listing and filtering individual results. """ return _current_client().get_hardware_summary(name, origin) @@ -381,6 +434,7 @@ def get_issue_tests( get_log, get_test_issues, get_build_issues, + list_labs, list_hardware, get_hardware_summary, list_issues, diff --git a/kcidev/mcp/tools_maestro.py b/kcidev/mcp/tools_maestro.py index d6f9185..9a8ae1c 100644 --- a/kcidev/mcp/tools_maestro.py +++ b/kcidev/mcp/tools_maestro.py @@ -36,14 +36,24 @@ def list_nodes( """List Maestro nodes, oldest first, optionally filtered. Filters are 'field=value' strings, for example 'name=checkout', - 'state=done', 'result=fail' or 'treeid='. Matching is - exact; append '__re' to a field for a regex match, for example - 'name__re=baseline' matches all baseline job variants. Results - are returned oldest first, so to reach recent nodes window the - query with a filter such as 'created__gt=2026-07-01' rather - than paginating from the start. Use limit and offset to - paginate within the window; full nodes are large, so use - fields to project each node to only those keys. + 'state=done', 'result=fail' or 'treeid='. Nested node + fields are addressed with a dot, most usefully + 'data.runtime=' to restrict results to one lab or runtime + (for example 'data.runtime=lava-collabora') and + 'data.platform=' for one board; both are applied by + the server, so prefer them over listing everything and + filtering afterwards. Matching is exact; append '__re' to a + field for a regex match, for example 'name__re=baseline' + matches all baseline job variants and + 'data.platform__re=sc7180' all sc7180 boards. Filters combine, + so 'data.runtime=lava-collabora' with + 'data.platform__re=^qcom' answers "what is this lab doing with + qcom boards" in one query. Results are returned oldest first, + so to reach recent nodes window the query with a filter such as + 'created__gt=2026-07-01' rather than paginating from the start. + Use limit and offset to paginate within the window; full nodes + are large, so use fields to project each node to only those + keys. """ check_page_bounds(limit, offset) nodes = client.get_nodes( diff --git a/tests/test_mcp_tools_dashboard.py b/tests/test_mcp_tools_dashboard.py index 87c9cd6..ab4d59e 100644 --- a/tests/test_mcp_tools_dashboard.py +++ b/tests/test_mcp_tools_dashboard.py @@ -196,6 +196,7 @@ def test_get_summary_compact_by_default(monkeypatch): assert result["summary"]["builds"] == { "status": {"PASS": 10, "FAIL": 1}, "architectures": {"x86_64": {"PASS": 5}}, + "labs": {"lab-1": {}}, "issues": [], } assert result["summary"]["boots"] == { @@ -356,3 +357,103 @@ def test_issue_tools_validate_before_any_request(monkeypatch): with pytest.raises(ToolExecutionError): tools_dashboard.get_issue_tests("maestro:i1", status="borked") get.assert_not_called() +def test_list_labs_returns_lab_counts(monkeypatch): + get = _mock_get( + monkeypatch, + { + "n_builds": 100, + "lab_maps": { + "lava-collabora": {"builds": 161, "boots": 987, "tests": 100105}, + "opentest-ti": {"builds": 4, "boots": 80, "tests": 0}, + }, + }, + ) + result = tools_dashboard.list_labs(days=3) + url = get.call_args[0][0] + assert "metrics/" in url + assert "start_days_ago=3" in url + assert result["days"] == 3 + assert result["labs"]["opentest-ti"] == {"builds": 4, "boots": 80, "tests": 0} + assert "n_builds" not in result + + +def test_list_labs_without_lab_data_errors(monkeypatch): + _mock_get(monkeypatch, {"n_builds": 100}) + with pytest.raises(ToolExecutionError, match="lab data"): + tools_dashboard.list_labs() + + +def test_list_boots_filters_by_lab(monkeypatch): + _mock_get( + monkeypatch, + { + "boots": [ + {"id": "b1", "status": "PASS", "lab": "lava-collabora"}, + {"id": "b2", "status": "FAIL", "lab": "opentest-ti"}, + {"id": "b3", "status": "FAIL", "lab": "lava-collabora"}, + {"id": "b4", "status": "PASS", "lab": None}, + ] + }, + ) + result = tools_dashboard.list_boots( + giturl="https://git.example.org/linux.git", + branch="master", + commit="deadbeef", + lab="LAVA-Collabora", + ) + assert result["total"] == 4 + assert result["matched"] == 2 + assert [b["id"] for b in result["boots"]] == ["b1", "b3"] + + +def test_list_tests_combines_lab_and_status_filters(monkeypatch): + _mock_get( + monkeypatch, + { + "tests": [ + {"id": "t1", "status": "FAIL", "lab": "lava-collabora"}, + {"id": "t2", "status": "PASS", "lab": "lava-collabora"}, + {"id": "t3", "status": "FAIL", "lab": "maestro"}, + ] + }, + ) + result = tools_dashboard.list_tests( + giturl="https://git.example.org/linux.git", + branch="master", + commit="deadbeef", + status="fail", + lab="lava-collabora", + ) + assert result["total"] == 3 + assert result["matched"] == 1 + assert [t["id"] for t in result["tests"]] == ["t1"] + + +def test_list_builds_matches_lab_in_misc(monkeypatch): + _mock_get( + monkeypatch, + { + "builds": [ + {"id": "b1", "status": "PASS", "misc": {"lab": "maestro"}}, + {"id": "b2", "status": "PASS", "misc": {"runtime": "k8s-all"}}, + {"id": "b3", "status": "PASS", "misc": None}, + {"id": "b4", "status": "PASS"}, + ] + }, + ) + result = tools_dashboard.list_builds( + giturl="https://git.example.org/linux.git", + branch="master", + commit="deadbeef", + lab="k8s-all", + ) + assert result["matched"] == 1 + assert [b["id"] for b in result["builds"]] == ["b2"] + + +def test_get_summary_keeps_lab_breakdown(monkeypatch): + _mock_get(monkeypatch, SUMMARY_PAYLOAD) + result = tools_dashboard.get_summary( + giturl="https://git.example.org/linux.git", branch="master", commit="deadbeef" + ) + assert result["summary"]["builds"]["labs"] == {"lab-1": {}} From 516908e14628c6856c158f070e8e8f54bf1f06dc Mon Sep 17 00:00:00 2001 From: Ben Copeland Date: Thu, 27 Aug 2026 08:32:27 +0100 Subject: [PATCH 2/3] mcp: bound list_labs and explain an unmatched lab days was passed through unchecked to a metrics aggregation that gets slower as its window grows. Measured against production: 3 days answers in under 10s cold, 7 days takes around 56s, and 14 and 30 days always exceed the 60s read timeout and come back as a bare "Dashboard metrics request failed" after a full minute. Cap the window at 7 days, so a caller cannot ask for one that cannot succeed. An unknown lab returned matched=0, which is indistinguishable from a lab that ran nothing on that commit. Return the labs the fetched entries actually report, so a mistyped name shows up without a second call. Validating up front is not possible: lab names are an open set and the only source is the metrics call itself. Drop 'maestro' as the example lab for builds. Every build from that origin reports it as misc.lab, so filtering by it matches everything; the runtime cluster is the value that discriminates. Signed-off-by: Ben Copeland --- kcidev/mcp/tools_dashboard.py | 63 +++++++++++++++++++++---------- kcidev/mcp/validation.py | 13 +++++++ tests/test_mcp_tools_dashboard.py | 42 +++++++++++++++++++++ 3 files changed, 98 insertions(+), 20 deletions(-) diff --git a/kcidev/mcp/tools_dashboard.py b/kcidev/mcp/tools_dashboard.py index 5529c0b..601ef23 100644 --- a/kcidev/mcp/tools_dashboard.py +++ b/kcidev/mcp/tools_dashboard.py @@ -7,7 +7,12 @@ from kcidev.api import KciDevError, KernelCIClient from kcidev.libs.filters import StatusFilter from kcidev.mcp.errors import tool_errors -from kcidev.mcp.validation import check_page_args, check_page_bounds, checked_status +from kcidev.mcp.validation import ( + check_page_args, + check_page_bounds, + checked_days, + checked_status, +) _active_client = ContextVar("dashboard_tool_client", default=None) @@ -34,19 +39,25 @@ def _page(data, key, status, limit, offset, fields=None, lab=None): if status: status_filter = StatusFilter(checked_status(status)) items = [item for item in items if status_filter.matches(item)] + candidates = items if lab: wanted = lab.lower() items = [item for item in items if wanted in _entry_labs(item)] page = items[offset : offset + limit] if fields: page = [{k: item[k] for k in fields if k in item} for item in page] - return { + result = { key: page, "total": total, "matched": len(items), "limit": limit, "offset": offset, } + if lab and not items: + result["labs_present"] = sorted( + {name for item in candidates for name in _entry_labs(item)} + ) + return result @tool_errors @@ -165,13 +176,17 @@ def list_builds( """List kernel builds for one commit of a tree. Optional filters: arch (e.g. 'arm64'), tree name, ISO date range, - status ('pass', 'fail', 'inconclusive' or 'all'), and lab, the lab or - runtime that produced the build (builds report this as 'misc.lab' - and 'misc.runtime', for example 'maestro' or 'k8s-all'); use - list_labs to find valid names. Results are paginated with - limit/offset; the response carries 'total' (before filtering) and - 'matched' counts so you know whether to fetch further pages; - fields projects each entry to only those keys. + status ('pass', 'fail', 'inconclusive' or 'all'), and lab, the lab + or runtime that produced the build (builds report this as + 'misc.lab' and 'misc.runtime'; 'misc.lab' is effectively the + origin, so the runtime cluster such as 'k8s-all' is the value that + discriminates); use list_labs to find valid names. Results are + paginated with limit/offset; the response carries 'total' (before + filtering) and 'matched' counts so you know whether to fetch + further pages, and a lab matching nothing returns 'labs_present', + the labs the entries actually report, so a mistyped name shows up + without a second call; fields projects each entry to only those + keys. Returns build entries with ids usable with get_build. """ check_page_args(status, limit, offset) @@ -201,13 +216,16 @@ def list_boots( """List boot test results for one commit of a tree. Optional filters: arch, tree name, ISO date range, boot origin, - status ('pass', 'fail', 'inconclusive' or 'all'), and lab, the lab or - runtime that ran the boot (for example 'lava-collabora'); use + status ('pass', 'fail', 'inconclusive' or 'all'), and lab, the lab + or runtime that ran the boot (for example 'lava-collabora'); use list_labs to find valid names, or get_summary, whose per-section 'labs' counts show which labs ran this commit at all. Results are paginated with limit/offset; the response carries 'total' (before - filtering) and 'matched' counts so you know whether to fetch further - pages; fields projects each entry to only those keys. + filtering) and 'matched' counts so you know whether to fetch + further pages, and a lab matching nothing returns 'labs_present', + the labs the entries actually report, so a mistyped name shows up + without a second call; fields projects each entry to only those + keys. Returns boot entries with ids usable with get_test. """ check_page_args(status, limit, offset) @@ -236,14 +254,16 @@ def list_tests( """List test results for one commit of a tree. Optional filters: arch, tree name, ISO date range, status ('pass', - 'fail', 'inconclusive' or 'all'), and lab, the lab or runtime that ran the - test (for example 'lava-collabora'); use list_labs to find valid - names, or get_summary, whose per-section 'labs' counts show which - labs ran this commit at all. A full commit can carry tens of + 'fail', 'inconclusive' or 'all'), and lab, the lab or runtime that + ran the test (for example 'lava-collabora'); use list_labs to find + valid names, or get_summary, whose per-section 'labs' counts show + which labs ran this commit at all. A full commit can carry tens of thousands of tests, so filter by lab and status and paginate with limit/offset; the response carries 'total' (before filtering) and - 'matched' counts so you know whether to fetch further pages; fields - projects each entry to only those keys. + 'matched' counts so you know whether to fetch further pages, and a + lab matching nothing returns 'labs_present', the labs the entries + actually report, so a mistyped name shows up without a second call; + fields projects each entry to only those keys. Returns test entries with ids usable with get_test. """ check_page_args(status, limit, offset) @@ -317,6 +337,7 @@ def get_build_issues(build_id: str): return _current_client().get_build_issues(build_id) +@tool_errors def list_labs(days: int = 7): """List the labs (test runtimes) reporting to KernelCI. @@ -327,8 +348,10 @@ def list_labs(days: int = 7): 'data.runtime' filter of list_nodes. Counts cover all origins and trees; for the labs that ran one specific tree or platform, use the per-section 'labs' counts of get_summary or get_hardware_summary. + The window is capped at 7 days; wider windows time out in the + dashboard's metrics aggregation. """ - data = _current_client().get_metrics(start_days_ago=days) + data = _current_client().get_metrics(start_days_ago=checked_days(days)) labs = data.get("lab_maps") if isinstance(data, dict) else None if not isinstance(labs, dict): raise KciDevError("dashboard metrics response carried no lab data") diff --git a/kcidev/mcp/validation.py b/kcidev/mcp/validation.py index 6c4d26a..afc6bd1 100644 --- a/kcidev/mcp/validation.py +++ b/kcidev/mcp/validation.py @@ -5,6 +5,8 @@ STATUS_CHOICES = ("all", "pass", "fail", "inconclusive") +MAX_LAB_DAYS = 7 + def checked_status(status): normalised = status.strip().lower() @@ -43,3 +45,14 @@ def check_page_args(status, limit, offset): check_page_bounds(limit, offset) if status: checked_status(status) + + +def checked_days(days, maximum=MAX_LAB_DAYS): + if days < 1: + raise KciDevError(f"Invalid days {days}: must be one or greater") + if days > maximum: + raise KciDevError( + f"Invalid days {days}: must be at most {maximum}. Wider windows " + "time out in the dashboard metrics aggregation" + ) + return days diff --git a/tests/test_mcp_tools_dashboard.py b/tests/test_mcp_tools_dashboard.py index ab4d59e..2a2c931 100644 --- a/tests/test_mcp_tools_dashboard.py +++ b/tests/test_mcp_tools_dashboard.py @@ -357,6 +357,8 @@ def test_issue_tools_validate_before_any_request(monkeypatch): with pytest.raises(ToolExecutionError): tools_dashboard.get_issue_tests("maestro:i1", status="borked") get.assert_not_called() + + def test_list_labs_returns_lab_counts(monkeypatch): get = _mock_get( monkeypatch, @@ -457,3 +459,43 @@ def test_get_summary_keeps_lab_breakdown(monkeypatch): giturl="https://git.example.org/linux.git", branch="master", commit="deadbeef" ) assert result["summary"]["builds"]["labs"] == {"lab-1": {}} + + +def test_list_labs_rejects_non_positive_days(monkeypatch): + get = _mock_get(monkeypatch, {"lab_maps": {}}) + with pytest.raises(ToolExecutionError): + tools_dashboard.list_labs(days=0) + get.assert_not_called() + + +def test_list_labs_rejects_days_above_cap(monkeypatch): + get = _mock_get(monkeypatch, {"lab_maps": {}}) + with pytest.raises(ToolExecutionError) as excinfo: + tools_dashboard.list_labs(days=90) + assert "90" in str(excinfo.value) + get.assert_not_called() + + +def test_unmatched_lab_reports_the_labs_that_are_present(monkeypatch): + _mock_get( + monkeypatch, + { + "tests": [ + {"id": "t1", "status": "PASS", "lab": "lava-collabora"}, + {"id": "t2", "status": "PASS", "lab": "lava-broonie"}, + ] + }, + ) + result = tools_dashboard.list_tests(**_tree_args(lab="lava-colabora")) + assert result["matched"] == 0 + assert result["labs_present"] == ["lava-broonie", "lava-collabora"] + + +def test_matched_lab_omits_the_labs_present_hint(monkeypatch): + _mock_get( + monkeypatch, + {"tests": [{"id": "t1", "status": "PASS", "lab": "lava-collabora"}]}, + ) + result = tools_dashboard.list_tests(**_tree_args(lab="lava-collabora")) + assert result["matched"] == 1 + assert "labs_present" not in result From 5d5fd44f3102929a6a005e6a94e8b17836758539 Mon Sep 17 00:00:00 2001 From: Ben Copeland Date: Fri, 4 Sep 2026 11:11:27 +0100 Subject: [PATCH 3/3] mcp: list labs present regardless of the status filter labs_present was collected after the status filter, so a real lab with no entries in the requested status was left out of it: asking for status=fail on a lab whose tests all passed returned no results and no mention of the lab, which is exactly the mistyped-name case the hint exists to rule out. Collect it from the commit's entries before any status filter, so it answers "does this lab appear here at all" rather than "does it appear with this status". Signed-off-by: Ben Copeland --- kcidev/mcp/tools_dashboard.py | 43 +++++++++++++++++++------------ tests/test_mcp_tools_dashboard.py | 19 ++++++++++++++ 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/kcidev/mcp/tools_dashboard.py b/kcidev/mcp/tools_dashboard.py index 601ef23..26eb3c3 100644 --- a/kcidev/mcp/tools_dashboard.py +++ b/kcidev/mcp/tools_dashboard.py @@ -36,10 +36,10 @@ def _page(data, key, status, limit, offset, fields=None, lab=None): check_page_bounds(limit, offset) items = data[key] if isinstance(data, dict) else data total = len(items) + candidates = items if status: status_filter = StatusFilter(checked_status(status)) items = [item for item in items if status_filter.matches(item)] - candidates = items if lab: wanted = lab.lower() items = [item for item in items if wanted in _entry_labs(item)] @@ -184,8 +184,9 @@ def list_builds( paginated with limit/offset; the response carries 'total' (before filtering) and 'matched' counts so you know whether to fetch further pages, and a lab matching nothing returns 'labs_present', - the labs the entries actually report, so a mistyped name shows up - without a second call; fields projects each entry to only those + every lab the entries report before any status filter, so a mistyped + name shows up without a second call and a real lab with no matching + status is still listed; fields projects each entry to only those keys. Returns build entries with ids usable with get_build. """ @@ -223,8 +224,9 @@ def list_boots( paginated with limit/offset; the response carries 'total' (before filtering) and 'matched' counts so you know whether to fetch further pages, and a lab matching nothing returns 'labs_present', - the labs the entries actually report, so a mistyped name shows up - without a second call; fields projects each entry to only those + every lab the entries report before any status filter, so a mistyped + name shows up without a second call and a real lab with no matching + status is still listed; fields projects each entry to only those keys. Returns boot entries with ids usable with get_test. """ @@ -261,8 +263,9 @@ def list_tests( thousands of tests, so filter by lab and status and paginate with limit/offset; the response carries 'total' (before filtering) and 'matched' counts so you know whether to fetch further pages, and a - lab matching nothing returns 'labs_present', the labs the entries - actually report, so a mistyped name shows up without a second call; + lab matching nothing returns 'labs_present', every lab the entries + report before any status filter, so a mistyped name shows up without + a second call and a real lab with no matching status is still listed; fields projects each entry to only those keys. Returns test entries with ids usable with get_test. """ @@ -339,17 +342,23 @@ def get_build_issues(build_id: str): @tool_errors def list_labs(days: int = 7): - """List the labs (test runtimes) reporting to KernelCI. - - Returns each lab name with how many builds, boots and tests it - reported over the last N days, so you can pick a valid lab name - without scanning result listings. The names are usable as the 'lab' - filter of list_builds, list_boots and list_tests, and as the - 'data.runtime' filter of list_nodes. Counts cover all origins and + """List the labs (test runtimes) that ran boots or tests on KernelCI. + + Returns each lab name with how many builds, boots and tests are + associated with it over the last N days, so you can pick a valid lab + name without scanning result listings. The names are usable as the + 'lab' filter of list_boots and list_tests, and as the 'data.runtime' + filter of list_nodes. + + The figures come from the dashboard's test metrics, so they are + test-derived: the 'builds' count is builds referenced by those tests, + not builds a lab produced, and a lab that only produces builds and + runs no tests may be absent. Treat this as boot/test lab discovery, + not a complete lab list for list_builds. Counts cover all origins and trees; for the labs that ran one specific tree or platform, use the - per-section 'labs' counts of get_summary or get_hardware_summary. - The window is capped at 7 days; wider windows time out in the - dashboard's metrics aggregation. + per-section 'labs' counts of get_summary or get_hardware_summary. The + window is capped at 7 days; wider windows time out in the dashboard's + metrics aggregation. """ data = _current_client().get_metrics(start_days_ago=checked_days(days)) labs = data.get("lab_maps") if isinstance(data, dict) else None diff --git a/tests/test_mcp_tools_dashboard.py b/tests/test_mcp_tools_dashboard.py index 2a2c931..70b0c9c 100644 --- a/tests/test_mcp_tools_dashboard.py +++ b/tests/test_mcp_tools_dashboard.py @@ -499,3 +499,22 @@ def test_matched_lab_omits_the_labs_present_hint(monkeypatch): result = tools_dashboard.list_tests(**_tree_args(lab="lava-collabora")) assert result["matched"] == 1 assert "labs_present" not in result + + +def test_labs_present_ignores_the_status_filter(monkeypatch): + _mock_get( + monkeypatch, + { + "tests": [ + {"id": "p1", "status": "PASS", "lab": "lava-collabora"}, + {"id": "f1", "status": "FAIL", "lab": "lava-broonie"}, + ] + }, + ) + + result = tools_dashboard.list_tests( + **_tree_args(status="fail", lab="lava-collabora") + ) + + assert result["matched"] == 0 + assert "lava-collabora" in result["labs_present"]