Repository navigation
Feat/mcp eval phases - #102
operetz-rh wants to merge 9 commits into
Conversation
- Move mcp-phase{1,2} tasks to pipeline/tasks/konflux/, rename to
mcp-phase1-static / mcp-phase2-conformance, and adopt the app.kubernetes.io
labels used on the deploy-pipeline branch (name == filename, no namespace).
- Trim redundant/duplicated comments in the Phase 1/2 scanners and gates;
keep only the WHY notes. No logic change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
IlonaShishov
left a comment
IlonaShishov
left a comment
There was a problem hiding this comment.
Great work @operetz-rh!
Please consider my suggestions bellow
| echo "Pipeline repo already cloned, skipping" | ||
| exit 0 | ||
| fi | ||
| git clone --depth 1 --branch "$(params.pipeline-repo-revision)" \ |
There was a problem hiding this comment.
Use Pre-built Container Image Instead of Runtime Git Clone
The current approach has the following issues:
- Performance: Each task clones the repo (30-60s) and installs tools (2+ min) on every run
- Anti-pattern: Dependencies should be baked into images at build time, not installed at runtime
- Inefficiency: Container images are pulled once per pipeline and cached on the node; git clones happen per step and can't be cached
- Version drift: pipeline-repo-revision: main means task bundle v0.1 could run different code on different days
- Inconsistent: We're not using Tekton's official git-clone task; we're doing ad-hoc bash cloning
For a cleaner approach, you could build a dedicated MCP eval image and use it as the tasks base image:
FROM registry.access.redhat.com/ubi9/python-311:9.6
# Copy ONLY what MCP needs
COPY abevalflow/mcp /opt/abevalflow/mcp
COPY abevalflow/gates /opt/abevalflow/gates
COPY abevalflow/schemas.py /opt/abevalflow/schemas.py
COPY abevalflow/observability /opt/abevalflow/observability
COPY scripts/mcp /opt/scripts/mcp
ENV PYTHONPATH=/opt
# Install tools + minimal Python deps
RUN curl ... | tar -xz gitleaks && \
pip install semgrep pydantic pyyaml && \
dnf install ruby && gem install licensee
There was a problem hiding this comment.
Ones you fix this, this step and params pipeline-repo-url and pipeline-repo-revision can be removed
| REPORTS_DIR="$(workspaces.source.path)/reports/$(params.submission-name)" | ||
| mkdir -p "$REPORTS_DIR" | ||
|
|
||
| echo "=== Installing gitleaks (secrets) ===" |
There was a problem hiding this comment.
Tools can be Pre-installed into Pre-built Container Image the become available when used as base image
There was a problem hiding this comment.
Moving the installation to image build stage during dev - eliminates installation errors at runtime that can redundantly fail the pipeline and are irrelevant to the MCP evaluation itself.
| results: | ||
| - name: phase1-passed | ||
| description: Whether all Phase 1 gates passed (true/false). | ||
| - name: secrets-passed |
There was a problem hiding this comment.
Why does the output include a result for each individual check? do they have a purpose in the next tasks?
If not report-path and phase1-passed is enough.
| results: | ||
| - name: phase2-passed | ||
| description: Whether Phase 2 passed (no check FAILED in block mode). | ||
| - name: not-evaluated-count |
|
Another review item: |
…arer layout - Add containers/mcp-eval/Containerfile baking scanners (gitleaks, semgrep, licensee) + deps; both Phase 1/2 tasks use it and drop the runtime git clone, tool installs, and pipeline-repo-url/revision params - Trim unused Tekton results (per-gate passes, not-evaluated-count); keep phase1/phase2-passed + report-path - Rename mcp_client.py -> _mcp_client.py and document entry points vs helpers - compass_fetch: degrade missing/malformed facts to not_evaluated instead of aborting Phase 2; pin image Python deps Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This branch has not been deployed
2 participants
Summary
Adds MCP-server evaluation Phase 1 (static / build-time) and Phase 2 (deterministic conformance) per the ADR (Approach 4), scoped to those two phases only — Phase 3 (behavioral) and pipeline orchestration are out of scope here. Targeted at the
mcp-evaluation-pipelinebranch so all MCP pipeline pieces land together before the combined PR tomain.Changes
abevalflow/mcp/phase1,scripts/mcp): secrets (gitleaks), no-user-code (semgrep, offline ruleset for Python/JS/TS/Go/Java), and license (licensee). Weighted 0–1 scoring withblock/warn/disabledmodes; a missing scan fails in block mode.abevalflow/mcp/phase2,scripts/mcp): black-box probe of a running server over Streamable HTTP (JSON-RPC 2.0), 10 checks, three-state model (pass/fail/not_evaluated— unreachable or undecidable never fails). No LLM. Two checks are consumed from existing Compass facts rather than re-probed.pipeline/tasks/konflux/:mcp-phase1-staticandmcp-phase2-conformance, following this branch's convention (name == filename,app.kubernetes.iolabels).coverageblock so a zero-finding pass shows what was actually scanned.Test plan
uv run pytest tests/test_mcp_phase1.py tests/test_mcp_phase2.py— 35 passed, 2 skipped (tool-gated integration tests; licensee not installed locally)generic-mock-mcp-serveropenshift-mcp-serverRelated
APPENG-6254