shahar1 commented on PR #69381: URL: https://github.com/apache/airflow/pull/69381#issuecomment-4887126538
> IIUC, we could achieve the same purpose and functionally by adding a new "rule" on AGENTS.md to route the query with Airflow CTL by `breeze exec ... airflowctl` or (even just naive `docker exec ... airflowctl ...`). Few issues with that: 1. It introduces friction around building the command string, escaping/quoting it correctly, and parsing the output back into something usable - a typed MCP tool sidesteps all three; params go in typed, the response comes back typed. 2. Authentication has to be handled explicitly on the CLI side (login step, token storage, keyring) - the MCP server takes care of that behind the scenes. 3. `airflowctl` is still missing some commands we rely on (TIs, logs). Worth noting logs aren't in the legacy airflow CLI either - there's no airflow tasks logs command; fetching task logs has only ever been possible through the REST API (or the web UI), which is exactly what this MCP wraps. > MCP for Airflow API server in Breeze is definitely another direction to achieve it but _might_ introduce more maintenance effort. I don't think it's too much maintenance in practice: - The surface is already small on purpose, and narrowly scoped to the most common methods. - Unit tests already run as their own CI job. - There's a contract test that checks every endpoint we call against Airflow's own OpenAPI spec, so if something renames/changes, CI just fails and tells us exactly where. - The only new dependency is fastmcp, and everything it pulls in (mcp, httpx, pydantic, etc.) we already ship elsewhere anyway. So it's a bounded cost, not something that grows on us over time. > IMHO. How about waiting for the eval framework that @RoyLee1224 working in #69308 to settle down first before directly introducing MCP for Breeze. WDYT? I wouldn't like to gate on that, if it's ok - this is an opt-in feature that doesn't touch any other code areas, so I think we can run independently and evaluate/adjust along the way rather than block on it. I still see agentic skills and MCP as complementary here. In fact, we'll even need a dedicated skill for using the MCP, covering the consent/risk wording around write/delete operations discussed with @Dev-iL (separate from the other breeze-related skills you're developing as part of your current efforts). I'd be happy if Roy wants to contribute this part, once the eval framework settles. > Additionally, @shahar1 would you mind to provide more context or even concrete setup for `I've empirically tested the resolving 2*** GitHub issues by subagents, where each issue is resolved independently by 2 subagents with the same model (A/B testing):` this part you mentioned when you have a moment? Thanks. > From my perspective, If we miss context of some the server-side runtime, we should improve the coverage of Airflow CTL, leverage the existing feature at much as possible instead of introducing new deps in the first place. (IMO, CTL should be able to represent the all the Core API context) For each issue I ran two headless agents (claude -p), same model (Sonnet), each in its own git worktree off the same main commit, unaware of each other, given the identical task prompt (reproduce, root-cause, fix, and add a regression test). The only difference was MCP access - one arm had the proposed MCP, the other was hard-denied it. I checked ground truth myself: each arm's regression test passes with its fix and fails without it. I evaluted some metrics from a JSON output (wall-clock, turns, tokens, cost) and parsed each transcript (pytest runs, live API/MCP calls). I used this method on two issues: - [#42790](https://redirect.github.com/apache/airflow/issues/42790) (Well-specified / low-API) - Both arms produced a correct, tested fix; the MCP arm was no faster (slightly slower) and used more tokens - no benefit where the fix is clear from the issue text. - [#39801](https://redirect.github.com/apache/airflow/issues/39801) (Hard / API-heavy) - Both arms again reached a correct, tested fix with the same root cause - but the MCP arm did it in ~9 min / 46 turns / 8 test runs vs ~28 min / 86 turns / 24 test runs. The gain (probably) came from ~2 cheap typed calls to observe live task-instance state early, cutting the trial-and-error roughly in half. > From my perspective, If we miss context of some the server-side runtime, we should improve the coverage of Airflow CTL, > leverage the existing feature at much as possible instead of introducing new deps in the first place. (IMO, CTL should be > able to represent the all the Core API context) Agreed on the direction, but: 1. `airflowctl` genuinely doesn't cover this yet (as I stated above) 2. "new deps" overstates it - fastmcp is the only new dependency, everything else is already shipped. 3. Everything from my first reply still stands regardless - auth handled server-side, typed calls vs. CLI parsing, etc. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
