feat(mcp): read-only MCP server — FOREMAN as an open node (F7) - #16
Merged
Conversation
Expose the reasoners' stored insights to third parties (simulation, ERP, energy management) via a read-only Model-Context-Protocol server (FastMCP, Streamable HTTP) — the "platform, not app" pillar. New layer src/foreman/mcp/. This is an interface layer over the existing read paths; it adds no reasoning. What: - 11 read-only tools mapped onto the existing reasoner read paths: machines + composed status, alarms (since/severity), drift status, failure predictions, worker recommendations, event-chain explanations, semantic note search, aggregated readings. No write tool, no reasoner/LLM trigger over MCP. - MCP-owned read layer (reads.py): the reasoner read logic lived inline in the HTTP routers with no reusable service method; MCP gets its own injected, testable, SELECT-only read layer (decision confirmed in review) rather than refactoring six existing routers. - Honest AI-Act transparency wrapper (Art. 50(2)): every AI-stemming output carries ai_generated / generated_by="foreman-ai" / requires_human_review / model_version (+ validation_status / data_regime / validation_caveat for predictions and recommendations). Non-AI data (master data, readings, alarms) carries no AI flags — enforced structurally by a model validator. - Dedicated read-only token auth (FOREMAN_MCP_, SecretStr, constant-time compare, fail-closed, production fail-fast) + token-bucket rate limit; standalone ASGI app with its own /health and /metrics. - PII: only HMAC tokens / NER-masked text leave MCP; tokens are never resolved; no embedding vector, no users fields. - IP wording: no internal vocabulary in tool names/descriptions/schemas (hidden-term scan as an acceptance test); SHAP is exposed as "contribution". - foreman_mcp_* metrics (tool, result, latency). Why: second differentiator pillar; fulfils AI-Act measure §10.5(2) — the MCP transparency flag moves from "documented" to "built". Tested (tests/mcp/): transparency honesty, tool correctness + read-only (no side effects), auth reject (401) + rate limit (429), PII safety, real SDK handshake (tool registry), structural no-actuation + hidden-term scan. MCP layer coverage 96%. mypy --strict 0, ruff + ruff format clean. Note: the F-PRED shap/lightgbm suite is verified in CI (crashes locally on Python 3.13); its module is untouched by this change. Docs (same commit): GROUND_TRUTH §4 (MCP interface), §10.5 (measure 2 "built"), new §17 (MCP contract) + §18 Privacy & Compliance + §19 Security; WALKTHROUGH F7 section; eu-ai-act-assessment measure 2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughDas PR implementiert das vollständige F7-MCP-Interface als neues ChangesF7 MCP-Schnittstelle
Sequence Diagram(s)sequenceDiagram
participant Client as Externes System
participant Middleware as McpAuthMiddleware
participant Server as Starlette/FastMCP
participant Tools as tools.py
participant Reads as reads.py
participant DB as PostgreSQL
rect rgba(100, 150, 200, 0.5)
note over Client,Middleware: Authentifizierung & Rate-Limiting
Client->>Middleware: POST /mcp (Authorization: Bearer token)
Middleware->>Middleware: extract_bearer()
Middleware->>Middleware: verify_mcp_token() [hmac.compare_digest]
Middleware->>Middleware: TokenBucket.allow()
alt Ungültiger Token
Middleware-->>Client: 401 WWW-Authenticate: Bearer
else Rate-Limit überschritten
Middleware-->>Client: 429 Too Many Requests
else Token gültig & Limit ok
Middleware->>Server: Request weiterleiten
end
end
rect rgba(100, 200, 150, 0.5)
note over Server,DB: Tool-Ausführung (read-only)
Server->>Tools: Tool-Dispatch (z.B. get_failure_prediction)
Tools->>Tools: _read_session() [kein commit]
Tools->>Reads: SELECT-Funktion (get_prediction)
Reads->>DB: parameterisierte SQL-Query
DB-->>Reads: ORM-Records
Reads-->>Tools: Sequence[FailurePredictionRecord]
Tools->>Tools: ai_transparency(model_version, caveat)
Tools-->>Server: FailurePredictionOut (+ AiTransparency)
Server-->>Client: JSON (ai_generated/requires_human_review)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
GROUND_TRUTH.md (1)
5-5: 💤 Low valueStatuszeile F7: Prägnant und vollständig, aber sehr dicht.
Die Zusammenfassung der drei Invarianten, der Architektur-Entscheidungen (ASGI, Token, Metriken, AI-Act-Maßnahme 2) ist komplett und richtig. Zur Lesbarkeit wäre eine Auflösung in Stichpunkte nach dem ersten Satz („read-only) hilfreich — die aktuelle Inline-Liste ist verständlich, aber bei einer zukünftigen Aktualisierung schnell fehleranfällig (jeder Punkt sollte einzeln editierbar bleiben). Optional: den Satz ab „Drei Invarianten" ggf. in eine nummerierte Liste (I/II/III mit jeweils einer kurzen Erklär-Klammer) umwandeln, damit Punkt-Updates leichter fallen.
📋 Vorschlag (optional): Gliederung der Invarianten
Anstelle von:
Drei Invarianten, strukturell verankert: **(I) read-only** — … ; **(II) AI-Act-Transparenz** … ; **(III) IP-Wording** —Könnte man schreiben:
Drei tragende Invarianten (strukturell verankert): 1. **Read-only, keine Aktorik, kein Reasoner-/LLM-Trigger** — keine Schreib-Pfade, MCP-eigene Read-Schicht `reads.py`. 2. **AI-Act-Transparenz an jedem KI-Output (Art. 50(2))** — gemeinsamer Wrapper `AiTransparency`, Validator erzwingt Ehrlichkeit. 3. **IP-Wording** — kein internes Vokabular in Tool-Namen/-Beschreibungen/-Schemata; Hidden-Term-Scan als Akzeptanzkriterium.Dies macht zukünftige Aktualisierungen granularer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@GROUND_TRUTH.md` at line 5, The section "Drei Invarianten, strukturell verankert" in GROUND_TRUTH.md (line 5) is currently formatted as a dense inline list with semicolons separating three invariants labeled (I) read-only, (II) AI-Act-Transparenz, and (III) IP-Wording. To improve readability and maintainability for future updates, refactor this section into a clearly numbered list (1, 2, 3) where each invariant becomes its own separate line item with a brief descriptive label and explanation. Break the long inline definitions into individual, self-contained points so that each invariant can be independently understood and edited without the risk of punctuation or syntax errors that come from managing a complex single-sentence structure.src/foreman/mcp/tools.py (1)
270-285: 💤 Low valueEingabevalidierung vor Session-Erwerb würde unnötige Ressourcen-Allokation vermeiden.
Die Severity-Validierung (Zeile 277) erfolgt erst nach dem Betreten beider Context Manager. Bei ungültiger Eingabe wurde die Session bereits erworben. Eine frühere Validierung vor
async withwürde dies vermeiden.♻️ Optionale Optimierung
async def get_alarms( machine_id: int | None = None, since: datetime | None = None, severity: str | None = None, ) -> AlarmListOut: """Liest Alarme (inkl. Drift-Warnungen), optional gefiltert nach Maschine/Zeit/Schwere.""" + if severity is not None and severity not in _ALLOWED_SEVERITIES: + raise ValueError( + f"Unbekannte severity '{severity}'. Erlaubt: {sorted(_ALLOWED_SEVERITIES)}." + ) async with _measured("get_alarms"), _read_session() as session: - if severity is not None and severity not in _ALLOWED_SEVERITIES: - raise ValueError( - f"Unbekannte severity '{severity}'. Erlaubt: {sorted(_ALLOWED_SEVERITIES)}." - ) alarms = await reads.list_alarms( session, machine_id=machine_id, since=since, severity=severity )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/foreman/mcp/tools.py` around lines 270 - 285, The severity parameter validation in the get_alarms function occurs inside the async with block after the session is already acquired, which wastes resources if invalid input is provided. Move the severity validation check (the condition checking if severity is not None and severity not in _ALLOWED_SEVERITIES) to execute before entering the async with statement containing _measured and _read_session, so that invalid inputs are rejected without acquiring unnecessary resources.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Around line 60-62: The mcp dependency constraint currently uses only a lower
bound (mcp>=1.9), which allows automatic upgrades to version 2.0.0 when it
becomes stable, introducing breaking changes to the project. Update the mcp
dependency constraint to include an upper bound that prevents major version
upgrades by changing it from mcp>=1.9 to mcp>=1.25,<2 (or minimally
mcp>=1.9,<2), ensuring the project stays within the stable 1.x release series
while also updating to a more recent stable version within that series as
recommended by the official SDK.
In `@src/foreman/mcp/reads.py`:
- Around line 224-252: The load_readings function accepts a limit parameter that
defaults to MAX_READING_POINTS but does not enforce this as an upper bound,
allowing callers to request larger result sets than intended. Add validation in
the load_readings function to cap the limit parameter to not exceed
MAX_READING_POINTS, either by clamping the value to the maximum or by raising an
error if the caller attempts to exceed it, ensuring the documented limit is
actually enforced regardless of what value is passed by the caller.
In `@tests/mcp/security/test_ip_wording.py`:
- Around line 40-43: The test function
test_tool_strings_carry_no_internal_vocabulary is missing a validation check to
ensure tools are actually registered before verifying their content. After
calling await server.list_tools() and storing the result in tool_list, add an
assertion to confirm that tool_list is not empty. This prevents the security
test from passing trivially if the tool registry happens to be empty, ensuring
the verification of tool strings is actually meaningful.
In `@tests/mcp/security/test_no_actuation.py`:
- Around line 37-45: The pattern matching in the
test_mcp_source_has_no_write_or_reasoner_trigger function performs a
case-sensitive check when comparing patterns against the source code (line 43:
`if pattern in source`). This allows SQL write strings in lowercase (like
`insert into`) to bypass the security check. Make the pattern comparison
case-insensitive by converting the source text to lowercase before checking if
each pattern from _FORBIDDEN_PATTERNS exists within it, ensuring the security
scan catches forbidden patterns regardless of their case.
- Around line 48-56: Add explicit pytest async markers to both test files for
clarity and robustness. In tests/mcp/security/test_no_actuation.py (lines
48-56): add `import pytest` at the top of the file if not already present, then
add the `@pytest.mark.asyncio` decorator immediately before the
`test_every_registered_tool_is_read_only` function. In
tests/mcp/security/test_ip_wording.py (lines 40-57): add `import pytest` at the
top of the file if not already present, then add the `@pytest.mark.asyncio`
decorator immediately before the
`test_tool_strings_carry_no_internal_vocabulary` function.
---
Nitpick comments:
In `@GROUND_TRUTH.md`:
- Line 5: The section "Drei Invarianten, strukturell verankert" in
GROUND_TRUTH.md (line 5) is currently formatted as a dense inline list with
semicolons separating three invariants labeled (I) read-only, (II)
AI-Act-Transparenz, and (III) IP-Wording. To improve readability and
maintainability for future updates, refactor this section into a clearly
numbered list (1, 2, 3) where each invariant becomes its own separate line item
with a brief descriptive label and explanation. Break the long inline
definitions into individual, self-contained points so that each invariant can be
independently understood and edited without the risk of punctuation or syntax
errors that come from managing a complex single-sentence structure.
In `@src/foreman/mcp/tools.py`:
- Around line 270-285: The severity parameter validation in the get_alarms
function occurs inside the async with block after the session is already
acquired, which wastes resources if invalid input is provided. Move the severity
validation check (the condition checking if severity is not None and severity
not in _ALLOWED_SEVERITIES) to execute before entering the async with statement
containing _measured and _read_session, so that invalid inputs are rejected
without acquiring unnecessary resources.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 65877bd5-6419-4ea4-baa2-ab2273d87cd0
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
GROUND_TRUTH.mddocs/WALKTHROUGH.mddocs/compliance/eu-ai-act-assessment.mdpyproject.tomlsrc/foreman/mcp/__init__.pysrc/foreman/mcp/auth.pysrc/foreman/mcp/reads.pysrc/foreman/mcp/schemas.pysrc/foreman/mcp/server.pysrc/foreman/mcp/tools.pysrc/foreman/mcp/transparency.pysrc/foreman/observability/metrics.pytests/integration/__init__.pytests/mcp/conftest.pytests/mcp/security/test_ip_wording.pytests/mcp/security/test_no_actuation.pytests/mcp/test_auth.pytests/mcp/test_pii.pytests/mcp/test_server.pytests/mcp/test_tools.pytests/mcp/test_transparency.py
- pyproject: pin mcp>=1.9,<2 — mcp 2.0 (July 2026) is breaking and moves transport config to streamable_http_app(); avoid an unguarded auto-upgrade. - reads.load_readings: clamp limit to MAX_READING_POINTS so the documented load cap is enforced regardless of caller (defense-in-depth). - tools.get_alarms: validate severity before acquiring the read session. - tests/mcp/security/test_no_actuation: scan source case-insensitively so a lowercase write/SQL pattern can't slip the read-only gate. - tests/mcp/security/test_ip_wording: assert a non-empty tool registry so the hidden-term scan can't pass trivially. Declined (with reasoning, see PR threads): - explicit @pytest.mark.asyncio on the two security tests — the whole suite runs under asyncio_mode=auto without markers; adding them to 2 of N async tests is inconsistent and gives no real protection. - bulletizing the GROUND_TRUTH status line — the dense single-paragraph **Status:** line is the established format across all phases. mypy --strict 0, ruff + format clean, 47 MCP tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
F7 — MCP-Server (FOREMAN als offener Knoten)
Eine neue Schicht
src/foreman/mcp/macht FOREMAN zum read-only Model-Context-Protocol-Server (Anthropic SDK / FastMCP, Streamable HTTP): die aggregierten Reasoner-Erkenntnisse werden als saubere, maschinenlesbare Tools an Drittsysteme (Simulation / ERP / Energiemanagement) gereicht. Schnittstellen-Schicht über dem schon Gebauten — keine neue Logik.Was
list_machines/get_machine(+ komponierter Status),get_drift_status,get_alarms(machine_id?, since?, severity?),list_failure_predictions/get_failure_prediction,get_worker_recommendation,list_event_chains/get_event_chain,search_notes,get_readings. Kein Schreib-Tool, kein Reasoner-/LLM-Trigger über MCP.reads.py): die Read-Logik der Reasoner lag bisher inline in den HTTP-Routern ohne wiederverwendbare Service-Methode. Statt sechs Router zu refactoren, bekommt MCP eine eigene, injizierte, testbare Read-Schicht (ausschließlich SELECT) — chirurgisch. (Entscheidung im Review geklärt.)ai_generated/generated_by="foreman-ai"/requires_human_review/model_version(Vorhersage/Empfehlung zusätzlichvalidation_status/data_regime/validation_caveat). Nicht-KI-Daten tragen keine KI-Flags — ein Validator erzwingt die Ehrlichkeit strukturell.auth.py): dedizierterFOREMAN_MCP_-Token (SecretStr, zeitkonstant, Fail-Closed, Produktions-Fail-Fast), Token-Bucket-Rate-Limit; eigenständige ASGI-App mit eigener/health//metrics.users-Felder.contribution.foreman_mcp_*-Metriken.Warum
Zweiter Differenzierungs-Pfeiler „Plattform statt App"; erfüllt zugleich AI-Act-Maßnahme §10.5(2) — das MCP-Transparenz-Flag von „dokumentiert" auf „gebaut".
Getestet
tests/mcp/— Transparenz-Ehrlichkeit, Tool-Korrektheit + Read-only (keine Seiteneffekte), Auth-Reject (401) + Rate-Limit (429), PII-Schutz, echter SDK-Handshake (Tool-Registry), struktureller No-Actuation- + Hidden-Term-Scan.mypy --strict0 (109 Dateien) ·ruff+ruff format --checkcleanDoku (selber Commit)
GROUND_TRUTH §4 (MCP-Schnittstelle), §10.5 (Maßnahme 2 „gebaut"), neue §17 (MCP-Vertrag) + §18 Privacy & Compliance + §19 Security; WALKTHROUGH F7-Abschnitt; AI-Act-Assessment Maßnahme 2.
Hinweise für den Reviewer
.env.example:FOREMAN_MCP_TOKEN=<32+ Byte>muss manuell ergänzt werden (der Hook blockt.env*-Zugriffe). Ohne Token läuft der Server Fail-Closed; in Produktion brichtrequire_secure_tokenden Start ab.tests/mcp/bewusst ohne__init__.py: ein Test-Paket namensmcpwürde sonst das SDK-Paketmcpüberschatten. Dafür wurdetests/integration/__init__.pyergänzt, um einetest_auth-Basenamen-Kollision aufzulösen.tests/mcp/security/test_ip_wording.py).🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation