Looking for a critical review of my QA Orchestrator MCP server

[Rbkmen]

🏷️ Discussion Type

Product Feedback

Body

Looking for a critical review of my QA Orchestrator MCP server

Hi GitHub Community,

I built QA Orchestrator, a local FastMCP service for coordinating multi-stage QA reviews. It handles deterministic routing and bounded session state, while the host agent retains the code, evidence, prompts, model execution, and final QA decision.

I’d appreciate a first-pass technical review, especially:

Is the boundary between the host agent and orchestrator clear and trustworthy?

Are fixed review bundles and risk-based escalation useful, or too complex?

Are the MCP state transitions and setup docs understandable on first read?

What would stop you from trying or adopting it?

Please be candid; concrete concerns and suggestions are most helpful. If the project seems useful to you, a ⭐ is welcome, but honest feedback is my main goal.

[Simo-J-E]

Looks pretty good overall. The host/orchestrator split is clear and I like that the orchestrator does not get access to the code or make the final decision.

My main issue is that it feels a bit complex for normal PR reviews. I would keep the simple review as the default and only use the bigger review bundles when the risk is higher.

I would also add one really simple example showing:

normal QA review vs QA Orchestrator review

That would make it much easier to understand why someone should use this instead of just putting the same rules into AGENTS.md or prompts.

Also WSL2 being required on Windows might stop some people from trying it.

[Rbkmen]

Thanks, Simo — this is useful, concrete feedback. I agree the lightweight path for routine, narrow PRs should be clearer. One compatibility profile is already supported; fixed bundles are for broader or cross-concern changes. I will make this path and a comparison more visible in the README.

For example, for a small Ruby guard change:

• Ordinary review: the host runs one reviewer prompt and tracks the result in the conversation.
• With QA Orchestrator: the host selects ruby_reviewer; the MCP validates the route and session transition.

For a broader PR touching a Rails endpoint, background job, and tests, the host can choose the fixed ruby_backend bundle (code_explorer → ruby_reviewer → pr_test_analyzer). The host sends structured risk signals after primary review; the orchestrator applies fixed rules for optional deep review. AGENTS.md can describe the same process, while the MCP adds a validated transition contract and bounded progress state.

One clarification: the host selects the initial route from the task and diff; risk signals do not automatically choose the initial bundle. The host still owns evidence, model calls, and the final decision. Windows currently requires WSL2 because the launcher uses POSIX facilities; native Windows support is separate portability work. Thanks for pointing out these barriers.

[Rbkmen]

Big update =)

[vishalhabib99]

I ran it through mcp-doctor (my static MCP linter) and read the README, server.py and the tool contract. Candid answers to your four questions:

Scan first: all 9 tools found, Quality A (93%), Security A (100%). The only warnings are three read-only getters (get_qa_orchestration_catalog, get_qa_orchestration_model_policy, list_qa_orchestrations) with no tool-level try/except, which matters little for getters. One caveat: contracts.py uses 3.12 syntax that my local run (3.11) couldn't parse, so that file wasn't checked. The tool surface is better than most servers I scan: readOnlyHint/destructiveHint are set correctly, run_id has a pattern constraint, and the field description says "do not invent or transform it".

1. Is the boundary clear and trustworthy?
Clear, yes. Trustworthy is a different property, and it's worth naming. The orchestrator enforces the order the host reports, not the execution. completed_profile and risk_signals are claims from the host. A host that skips pr_test_analyzer and just sends the next completed_profile passes every check. A host that under-reports risk never triggers deep review. That's an inherent limit of a content-free design, not a bug, but readers will assume more than the tool guarantees. You already say this about models ("cannot verify that the host executed that model"); the same applies to roles. Two cheap ways to close part of the gap without taking any content:

• An opt-in, content-free transition log (run_id, step, profile, timestamp, outcome), so a human can audit afterwards that every role ran and how long each took. Today nothing persists once a session finishes or expires, so the trust story ends with the session.
• One line under "Responsibility boundary": "The orchestrator validates the sequence the host reports; it cannot confirm the host performed each review."

2. Fixed bundles and risk-based escalation: useful or too complex?
The escalation rules are the most useful part. Three readable rules turn "should we dig deeper?" into something consistent across runs and people. The bundles are where I'd trim: 9 bundles plus 7 single profiles is a lot of choice at triage, and autotest's order (TypeScript reviewer last) is surprising. I'd ship 3–4 bundles (ordinary, security, one per stack you actually use) and let people add more through config. I agree with @Simo-J-E: make the single-profile path the default and bundles the opt-in.

3. Are the state transitions and setup docs understandable on first read?
The state machine is. The README reads as a reference doc, not a first-read doc: 300+ lines, and model-policy setup comes before the reader has seen a single run.

• Put a ~10-line transcript of one real run right after the intro: the tool calls and responses for a small Ruby change. That one example answers "why not just AGENTS.md" better than the comparison table.
• Ship the host rules with the server. FastMCP(name="qa-orchestrator") sets no instructions, so, as the README says, registering the server doesn't load the rules. Passing the core of client-rules/generic/QA_ORCHESTRATOR_INSTRUCTIONS.md as instructions= (or exposing it as an MCP prompt) gives clients that support it the rules with no copy-paste step.
• Move the bundle/profile tables, deep-review rules and full tool contract into docs/. Keep the README to what, why, quick start and one example.

4. What would stop me from trying or adopting it?

• Setup asks me to choose provider, model and reasoning for 4 stages before I've seen it work. A "use whatever the host is running" default, with per-stage models as an advanced option, removes that step.
• The defaults are OpenAI models, so a Claude Code user has to go through the wizard just to start.
• The point from Welcome to GitHub product feedback #1: for a QA tool, "can I trust that the review actually happened?" is the adoption question.
• Smaller: Python 3.12 + uv + a checkout, and nine tools' descriptions in every session's context. A pinned uvx one-liner as the main install path would help.

Net: the design instinct (orchestrator stays content-free, host owns decisions) is right, and the code quality shows. The gap is between "validated workflow" and "verified review": close it or name it, and cut the first run down to one command and one example.

[NavidZamanKhan]

Hi! I went through your README and orchestration documentation. The separation between the host agent and MCP server is well thought out, especially keeping source code and model execution on the host side.

A few suggestions:

1. The workflow involves several MCP calls, even for simple reviews. I'd recommend measuring the additional latency to show whether the benefits justify that overhead.

2. Since the orchestrator relies on host-reported progress and risk signals, it cannot independently confirm that reviews actually happened. This is an important limitation worth highlighting.

3. A practical demo showing what happens when a review fails or gets interrupted would help developers understand the recovery process.

4. I'd also love to see a comparison of review accuracy and execution time with and without the orchestrator.

The architecture looks promising. For me, measurable improvements in review quality and reliability would be the biggest reasons to adopt it.

Nice work, and good luck with the project!

[Rbkmen]

Thanks, Navid — these are useful, concrete suggestions.

I have made the trust boundary explicit in the server instructions and README: the orchestrator validates the sequence and risk signals reported by the host, but cannot confirm that a review actually ran or that a signal is backed by evidence. The README now starts with a short Ruby walkthrough and a pinned uvx quick start, and routine low-risk changes can use a single review profile.

I agree that latency and quality need measurement before making any effectiveness claims. I have not run a controlled with/without comparison yet, so I am not claiming the extra MCP calls improve accuracy or pay for their latency. A useful evaluation would run matched changes with the same host, model, and settings, record end-to-end time, and independently compare findings.

The docs explain session recovery, but they do not yet show a concrete interrupted or failed run. That is the clearest remaining documentation gap. Thanks for calling it out.