artifact: add MCP context bundle reference example#209
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new artifact bundle example and a generator script, along with associated tests. The review feedback suggests enhancing the test suite by implementing more robust error handling and type validation when loading JSON files, as well as replacing brittle string-based checks with comprehensive key-set validation for the artifact's bundle object.
|
|
||
|
|
||
| def test_mcp_context_bundle_ref_example_references_mcp_output_without_dumping_payload() -> None: | ||
| artifact = json.loads(MCP_REF_ARTIFACT_PATH.read_text(encoding="utf-8")) |
There was a problem hiding this comment.
When loading JSON files, the repository guidelines require handling FileNotFoundError and JSONDecodeError by raising a RuntimeError that includes the repo-relative path. Additionally, the decoded payload should be validated as the expected type (e.g., a dictionary) using RuntimeError instead of assertions.
try:
artifact = json.loads(MCP_REF_ARTIFACT_PATH.read_text(encoding="utf-8"))
except (FileNotFoundError, json.JSONDecodeError) as exc:
raise RuntimeError(f"Failed to load {MCP_REF_ARTIFACT_PATH}: {exc}") from exc
if not isinstance(artifact, dict):
raise RuntimeError(f"Decoded payload from {MCP_REF_ARTIFACT_PATH} must be a dictionary")References
- When loading JSON files, handle FileNotFoundError and JSONDecodeError by raising a RuntimeError that includes a repo-relative path. Additionally, validate that the decoded payload is of the expected type (e.g., a dictionary) using RuntimeError instead of assertions.
| bundle = artifact["bundle"] | ||
| assert bundle["branch"] == "feat/mcp-context-bundle-ref-example" | ||
| assert bundle["mcp_context_output_ref"] == MCP_CONTEXT_OUTPUT_REF | ||
| assert bundle["ok"] is True | ||
| assert bundle["result"] == "PASS" | ||
| assert bundle["safe_pr_gate"]["ok"] is True | ||
|
|
||
| artifact_text = MCP_REF_ARTIFACT_PATH.read_text(encoding="utf-8") | ||
| assert "prompt_context" not in artifact_text | ||
| assert "replay_payload" not in artifact_text | ||
| assert "dependency_chains" not in artifact_text |
There was a problem hiding this comment.
The current test uses brittle string-based checks to verify that certain fields are not present in the artifact. A more robust approach is to validate the set of keys in the bundle object. This ensures that only the expected fields are present and provides better coverage by also verifying changed_files.
| bundle = artifact["bundle"] | |
| assert bundle["branch"] == "feat/mcp-context-bundle-ref-example" | |
| assert bundle["mcp_context_output_ref"] == MCP_CONTEXT_OUTPUT_REF | |
| assert bundle["ok"] is True | |
| assert bundle["result"] == "PASS" | |
| assert bundle["safe_pr_gate"]["ok"] is True | |
| artifact_text = MCP_REF_ARTIFACT_PATH.read_text(encoding="utf-8") | |
| assert "prompt_context" not in artifact_text | |
| assert "replay_payload" not in artifact_text | |
| assert "dependency_chains" not in artifact_text | |
| bundle = artifact["bundle"] | |
| assert set(bundle) == { | |
| "branch", | |
| "changed_files", | |
| "mcp_context_output_ref", | |
| "ok", | |
| "result", | |
| "safe_pr_gate", | |
| "validation_evidence", | |
| } | |
| assert bundle["branch"] == "feat/mcp-context-bundle-ref-example" | |
| assert bundle["changed_files"] == [ | |
| "artifacts/mcp_context_bundle_ref_example.json", | |
| "scripts/generate_mcp_context_bundle_ref_example.py", | |
| "tests/test_agent_artifact_bundle.py", | |
| ] | |
| assert bundle["mcp_context_output_ref"] == MCP_CONTEXT_OUTPUT_REF | |
| assert bundle["ok"] is True | |
| assert bundle["result"] == "PASS" | |
| assert bundle["safe_pr_gate"]["ok"] is True |
Adds a lightweight deterministic agent artifact bundle example that references MCP context output by repo-relative artifact path.
Validation: