Repository navigation
Feat/mcp eval phases - #102
Open
operetz-rh wants to merge 12 commits into
Open
operetz-rh wants to merge 12 commits into
operetz-rh wants to merge 12 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
requested changes
Oct 5, 2026
IlonaShishov
left a comment
IlonaShishov
left a comment
There was a problem hiding this comment.
Great work @operetz-rh!
Please consider my suggestions bellow
|
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>
IlonaShishov
requested changes
Oct 8, 2026
IlonaShishov
left a comment
IlonaShishov
left a comment
There was a problem hiding this comment.
Great work on the Refactor, please see a few more minor comments.
We are almost there!
…per, scanner hardening Phase 2 probe / client: - Parse all SSE data frames and select the one whose id matches the request (with a string-id fallback), so an interleaved notification no longer masquerades as the response. - Paginate tools/list via nextCursor (list_all_tools, with cursor dedup) so schema/annotation checks see tools beyond page one. - Guard a non-object JSON-RPC "error" (e.g. a bare string) instead of crashing. - Tighten _schema_is_object: require the declared type to permit "object" (array types like ["object","null"] are accepted) and reject malformed object keywords, while still accepting a typeless node with object keywords. Phase 1 scanners: - license_scan: licensee exits non-zero when it finds no license; evaluate its JSON output as a license-missing finding instead of a tool error. A non-zero exit with empty/unparseable output is still treated as a genuine tool error. - no_user_code_scan: disable inline nosem suppression (--disable-nosem) and the phone-home version check (--disable-version-check); flag a committed .semgrepignore as a high-severity finding (semgrep honors it, so its presence fails a blocking gate). Tekton tasks / image: - Extract the duplicated gate step into scripts/mcp/run_gate.sh (baked in the image) and call it from both phases; a missing/invalid summary now fails loudly rather than writing an empty Tekton result. - Drop the SEMGREP_ENABLE_VERSION_CHECK env var from the phase 1 task (now a scanner flag) and pin the licensee gem version in the Containerfile. Docs/tests: - Correct the Phase 1/2 implementation note in abevalflow/mcp docstring. - Add unit tests for SSE id-matching, pagination, schema shape (incl. array types), non-object errors, licensee non-zero exits, and .semgrepignore. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make entry-points-vs-helpers legible by filename, per PR review. Convention: run_* = Tekton-invoked entry point (python -m scripts.mcp.<name> or run_gate.sh), _* = imported helper. No bare names. Renamed (git-tracked moves): secrets_scan.py -> run_secrets_scan.py no_user_code_scan.py -> run_no_user_code_scan.py license_scan.py -> run_license_scan.py phase2_probe.py -> run_phase2_probe.py compass_fetch.py -> run_compass_fetch.py Updated the python -m paths in both Konflux task YAMLs, aliased the new names in the phase 1/2 test imports (usage sites unchanged), and fixed docstring/ comment path references in abevalflow/mcp. Added scripts/mcp/README.md documenting the convention with an entry-point -> phase -> emits table and the helper list. Behavior unchanged. Verify: ruff check + format clean; 47 passed, 2 skipped; live mock and openshift-mcp-server runs identical to prior baseline (P1 FAIL 0.75; P2 mock PASS 1.00, openshift FAIL 0.88 catching error-unknown-method). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This branch has not been deployed
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
2 participants
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.
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