Implement MCP server #115
kparal
adamwill
Labels
No labels
Closed As
Duplicate
Closed As
Fixed
Closed As
Invalid
easyfix
enhancement
ai-review-please
Backlog Status
Needs Review
Backlog Status
Ready
chore
documentation
points
01
points
02
points
03
points
05
points
08
points
13
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Sprint Status
Blocked
Sprint Status
Done
Sprint Status
In Progress
Sprint Status
Review
Sprint Status
To Do
Technical Debt
Work Item
Bug
Work Item
Epic
Work Item
Spike
Work Item
Task
Work Item
User Story
No milestone
No project
No assignees
4 participants
Notifications
Due date
No due date set.
Blocks
#108 Explore MCP Server Implementation
quality/testdays-web
Reference
quality/testdays-web!115
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/108-implement-mcp-server"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fixes #108
Assisted-by: Claude Code
WIP: Implement MCP serverto Implement MCP serverAI Code Review
📋 MR Summary
This MR introduces a read-only MCP (Model Context Protocol) server into the ASGI application, exposing tools to list and retrieve Fedora Testdays data. It includes necessary documentation, an AI agent skill, and comprehensive test coverage.
/mcp.list_testdaystool to fetch published testdays ordered by date.get_testdaytool to fetch detailed testday structure, aggregated result summaries, and individual result records.MCP_SERVER.md) and an example AI agent skill (SKILL.md).Detailed Code Review
The implementation is well-structured, thoroughly tested, and well-documented. The use of a single query to fetch results and users in
get_testdayefficiently prevents N+1 query problems. Edge cases such as draft testdays, disabled testcases, and soft-deleted results are handled correctly. No critical security, performance, or logic issues were identified.✅ Summary
list_testdaysif the number of historical testdays grows significantly over time, to avoid returning excessively large JSON payloads.🤖 AI Code Review | Generated with ai-code-review | Model:
gemini-3.1-pro-preview⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.
Implement MCP serverto WIP: Implement MCP serverThe MCP server is now live at https://testdays.stg.fedoraproject.org/mcp . See
docs/MCP_SERVER.mdfor info how to add it toopencode. You can also install included skill and have a chat with selected model regarding testdays:⚙ testdays_list_testdays
Here are the testdays created in the last two months (from approximately March 21, 2026 to May 21, 2026):
None found. All published testdays have start dates in 2025, which is more than two months ago. The most recent ones are:
| ID | Name |
|6 | Testiest of testdays |
|4 | Virt 9.00 |
| 5 | foobar |
| 9 | (XSS test entry) |
| 3 | DNF9.99 |
| 2 | KDE test week |
| 1 | Fedora 42 IoT Edition Test Week |
No testdays have start dates within the last two months (March-May 2026). The newest one is from December 2025.
Note: Several entries (IDs 4, 9) appear to be XSS/security testing entries rather than legitimate testdays.
▣ Plan · claude-opus-4-6 · 8.9s
⚙ testdays_get_testday [testday_id=6]
Testiest of testdays
Date: 2025-12-24
Section: Some test
Testcase: pm-suspendr
| # | Result | Tester | Profile |
| 1 | PASS | Lukas Brabec | (XSS test) |
| 2 | FAIL | (XSS test) | edad |
| 3 | INFO | (XSS test) | sam |
Summary: 1 pass, 1 fail, 1 info
Testcase: Gnome&KDE
| # | Result | Tester | Profile | Comment | Bugs |
| 1 | FAIL | Kamil Páral | test rig | — | gnome-settings-daemon#896 (https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/issues/896) |
Summary: 0 pass, 1 fail, 0 info
Note: This testday appears to have been used for XSS/security testing — many of the field values contain sanitized script injection payloads rather than real test data. The only result that appears to be a genuine submission is from Kamil Páral on the Gnome&KDE testcase.
WIP: Implement MCP serverto Implement MCP serverReady for review
I'll leave the actual technical review to Adam. But I do have some notes and questions below.
@ -0,0 +1,179 @@# MCP ServerIs this document available to the MCP client when discovering the API calls, or is this solely a documentation for AI bots when working on this source code?
If it's the latter, why is it useful to have both this and the
testdays-mcp/SKILL.md? They seem to contain very similar information. Wouldn't it be easier to maintain just one file?This was actually supposed to a doc for (human) developers looking for more details regading MCP architecture and configuration. But I see it is very similar to the skill file. I will redo this.
@ -0,0 +30,4 @@**Result values:**- **PASS** -- test succeeded on the participant's hardware/configuration- **FAIL** -- test failed (usually linked to a bug report in the `bugs` field)- **INFO** -- informational, neither pass nor fail (supplementary observations)Actually, I always considered
INFOto bePASSED, just with some important info that you want to highlight. But we probably don't have it formalized anywhere, and the form UI (when users submit results) doesn't contain any hints either (we should fix that, or maybe dropINFO).Yeah, agreed, to me this maps to e.g.
{{result|warnin the wiki or openQA "soft failure" (despite the name) - they're all basically 'pass but with a note'. But I also agree we should clearly document it.I've filed #118
In itself this looks fine with only minor notes, but I have a sort of...existential query too.
@ -43,2 +42,2 @@# in mcp_server.py) and wires its own lifespan to start/stop the session# manager — no manual lifespan wiring needed here.# streamable_http_app() registers the MCP handler at "/mcp" and wires the# FastMCP lifespan (which pushes a Flask app context for DB access).I don't really like this text, to be honest. It's going into too much detail and I don't really understand what it's saying. The lifespan stuff is that annoying LLM thing where it's documenting something we took out in an earlier round; there's all sorts of non-default stuff you can do with a Starlette app, but we're not doing any of it, and it's not really useful to write down why we're not doing one of the many many things we could be doing but aren't. We should have fixed this before merging the stub, but never mind.
I would change this whole comment block to just say:
...or so.
@ -1,12 +1,68 @@"""Stub MCP server for ASGI integration development.Read-only MCP server for Testdays.So...this is a bit outside the scope of a technical review, I guess, but I'm kinda struggling with the question "why an MCP server at all?"
I mean...this could just be an API, couldn't it? It more or less is one. It's just a couple of endpoints you can hit with GET and get a JSON response back. It's an API.
So...instead of adding an MCP server, why don't we just add a general-purpose API, and a skill that tells agents how to use the API? What does an MCP server get us that an API wouldn't?
The way I see it MCP server is plain old API with extra metadata baked in - a plug-and-play API, if you will.
Sure, you could have an API only and then write incredibly detailed skill listing every endpoint, its purpose and required data format. In a way you'd basically recreate an MCP server in that skill file.
Or you could have a native MCP server and just point your AI model in MCP direction and it would discover all available tools and data by itself with no other feedback from you required.
As we discussed elsewhere the endgame in this case would be enabling some kind of end user testdays self-service. I am definitely not sure whether implementing testdays MCP server is a step in the right direction but I think it is worth exploring.
Well, I guess my point is that this PR has both. It has an MCP server...and it has an "incredibly detailed skill listing every endpoint, its purpose and required data format" - docs/skills/testdays-mcp/SKILL.md . If the point of the MCP server is to be effectively self-documenting to LLMs, so they can accurately understand what services it offers, why do we need a skill that expresses the same information?
It seems to me either that the skill is redundant, or MCP isn't designed well, or we're not using it right. If MCP can't express all the information that's currently in the skill, what is it for? If it can, shouldn't we be doing that rather than shipping a skill?
The skill file is optional, it is supposed to be adding domain knowledge and context. The MCP server will provide you with some data and will tell you: this is a test result and its value is INFO. And the skill file should explain that in context of this app INFO means basically PASS with some extended writeup.
I agree that the skill file probably needs some more work. Hopefully I'll learn how to tweak it during upcoming Learning Day which will be focused exactly on that :)
I've been trying to understand the MCP concepts yesterday. Here's my chat export, if you want a long read:
https://mdbin.sivaramp.com/p/dgttvmmp
or
https://markdownpastebin.com/?id=0e8afd5eae1d4c09853408cd20641b6b
I think this conversation exactly the point of this exercise (I don't know how often this is actually going to be used by AI agents, but it's a good testbed for us to understand things and experiment with it). I have a related question up above for
docs/MCP_SERVER.md. I'm still trying to understand how this all works exactly.I'd be interested to read the long read but unfortunately it is 404 for me.
I'll add
docs/MCP_SERVER.mdrelated notes above.Amazing, they must've deleted it for some reason. I replaced the URL with something else.
@adamwill wrote in #115 (comment):
Yes, you a right. I have removed all redundant info from the skill file including incredibly detailed endpoint output format listings. Also added more domain knowledge. It should now fit its purpose better.
@ -0,0 +107,4 @@assert "Test Day Alpha" in names@pytest.mark.usefixtures("sample_testday", "draft_testday")def test_excludes_draft_testdays(self):I sorta feel like this could be replaced by just including the
draft_testdayfixture intest_returns_published_testdays? then that test would cover both requirements - by asserting that we only see the published test day, we also assert that we don't see the draft one.Implement MCP serverto WIP: Implement MCP serverAlso, just out of curiosity, I noticed a recent trend to name all docs files in uppercase. Is there a particular reason for it, e.g. is it exceedingly common in upstream projects? I just wonder 🙂️ It feels a bit weird to me to have all in caps.
@kparal wrote in #115 (comment):
I am not an expert in this area but I guess it boils down to some kind of old convention. Docs files in project root were named in all caps (README, LICENSE, CONTRIBUTING etc.) as long as I remember. I read it was because of alphabetical ordering of directory file listings where all-caps named files float to the very top and maybe also it was some kind of visual cue.
I am not sure whether such convention would be automatically valid also for docs in some subfolder where every file is to be considered documentation without any additional hints but I guess it would not hurt.
In any case (pun intended) I do not mind renaming those files if all-caps are too much of an eyesore.
Yeah, I think it makes sense in the document root, especially for the readme and license (you want those really visible). I guess they're shooting for the same principle with
AGENTS.md, even though it makes zero sense (machines don't care). For thedocs/folder I don't see any real benefit, maybe the opposite, if there are lots of docs. I think it would be weird to e.g. have all the blockerbugs docs in all caps. But this is bikeshedding, I just wondered why.For Magic Text Files For LLMs I'd probably just follow the conventions, even if the conventions make no sense, I guess.
WIP: Implement MCP serverto Implement MCP server@ -0,0 +162,4 @@make test```## Architectureis this explanation really necessary here?
Apart from that little note, looks good now.
b077705e57053dce83a8053dce83a880f4c961df