feat(config): add --online flag for Mergify API simulator validation#1017
Conversation
Add an --online flag to `mergify config validate` that sends the configuration to the Mergify simulator API for deeper validation beyond JSON schema checks (e.g., condition syntax, action semantics). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Change-Id: Iccecbbc8b9a4511ccde0a3886d90e417c2c9d9c2 Claude-Session-Id: 35f710ae-3790-4349-b55b-82247979f4a8
|
This pull request is part of a stack:
|
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🔴 ⛓️ Depends-On RequirementsThis rule is failing.Requirement based on the presence of
🔴 👀 Review RequirementsThis rule is failing.
🔴 🔎 ReviewsThis rule is failing.
🟢 🤖 Continuous IntegrationWonderful, this rule succeeded.
🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 📕 PR descriptionWonderful, this rule succeeded.
|
There was a problem hiding this comment.
Pull request overview
Adds an optional “online” validation mode to mergify config validate, enabling deeper validation by sending the raw config YAML to the Mergify configuration simulator API in addition to the existing JSON schema validation.
Changes:
- Introduces
--onlineplus supporting CLI options (--token,--repository,--api-url) and wires them intoconfig validate. - Adds simulator API integration (
simulate_config) and raw file reading support in the config validation module. - Expands unit tests to cover online success/failure and missing-parameter scenarios.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
mergify_cli/config/cli.py |
Adds --online workflow and simulator call path to config validate. |
mergify_cli/config/validate.py |
Adds raw file reading and async simulator API function + result type. |
mergify_cli/tests/config/test_validate.py |
Adds tests for online simulator validation and new constants for URLs/args. |
Comments suppressed due to low confidence (1)
mergify_cli/tests/config/test_validate.py:57
- There’s no test ensuring that offline
config validate(without --online) does not attempt to resolve token/repository defaults (which may invokegh auth tokenor git). Adding a regression test that patches utils.get_default_token/get_default_repository and asserts they are not called when --online is absent would prevent reintroducing that side effect.
_SCHEMA_URL = "https://docs.mergify.com/mergify-configuration-schema.json"
_SIMULATOR_URL = "https://api.mergify.com/v1/repos/owner/repo/configuration-simulator"
_ONLINE_ARGS = [
"--online",
"--token",
"test-token",
"--repository",
"owner/repo",
]
def _write_config(tmp_path: pathlib.Path, content: str) -> str:
config_file = tmp_path / ".mergify.yml"
config_file.write_text(content)
return str(config_file)
def test_valid_config(tmp_path: pathlib.Path) -> None:
config_path = _write_config(tmp_path, "pull_request_rules: []\n")
with respx.mock:
respx.get(_SCHEMA_URL).mock(
return_value=Response(200, json=_MINIMAL_SCHEMA),
)
result = CliRunner().invoke(config, ["validate", "--config", config_path])
assert result.exit_code == 0, result.output
assert "is valid" in result.output
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| default=lambda: asyncio.run(utils.get_default_token()), | ||
| ) | ||
| @click.option( | ||
| "--repository", | ||
| "-r", | ||
| help="Repository full name owner/repo (for --online)", | ||
| required=False, | ||
| default=lambda: asyncio.run(utils.get_default_repository()), |
There was a problem hiding this comment.
The --token/--repository options have defaults that call asyncio.run(utils.get_default_token/get_default_repository). Click evaluates these defaults even when --online is not set, which can trigger gh auth token and git config remote.origin.url for a purely offline schema validation run (and may print errors or slow/fail in environments without gh/git). Consider making these options default to None and only resolving token/repository (via utils.get_default_*) inside the if online: branch when needed.
| default=lambda: asyncio.run(utils.get_default_token()), | |
| ) | |
| @click.option( | |
| "--repository", | |
| "-r", | |
| help="Repository full name owner/repo (for --online)", | |
| required=False, | |
| default=lambda: asyncio.run(utils.get_default_repository()), | |
| default=None, | |
| ) | |
| @click.option( | |
| "--repository", | |
| "-r", | |
| help="Repository full name owner/repo (for --online)", | |
| required=False, | |
| default=None, |
| import httpx as httpx_mod | ||
|
|
||
| try: | ||
| response = await client.post( | ||
| f"/v1/repos/{repository}/configuration-simulator", | ||
| json={"mergify_yml": mergify_yml}, | ||
| ) | ||
| except httpx_mod.HTTPStatusError as e: | ||
| if e.response.status_code == 422: | ||
| try: | ||
| data = e.response.json() | ||
| detail = data.get("detail", str(data)) | ||
| except ValueError: | ||
| detail = e.response.text | ||
| return SimulatorResult(success=False, message=str(detail)) | ||
| raise | ||
|
|
There was a problem hiding this comment.
simulate_config currently relies on the AsyncClient having a response hook that raises HTTPStatusError (utils.check_for_status). If a plain httpx.AsyncClient is passed, 4xx/5xx responses (including 422) won't raise and will be treated as success. Also, when using get_mergify_http_client, the 422 case will already have printed a generic API error before this function returns a user-friendly simulator message. Make this function self-contained by checking response.status_code/raising via response.raise_for_status() yourself (and handle 422 without the check_for_status hook printing).
| import httpx as httpx_mod | |
| try: | |
| response = await client.post( | |
| f"/v1/repos/{repository}/configuration-simulator", | |
| json={"mergify_yml": mergify_yml}, | |
| ) | |
| except httpx_mod.HTTPStatusError as e: | |
| if e.response.status_code == 422: | |
| try: | |
| data = e.response.json() | |
| detail = data.get("detail", str(data)) | |
| except ValueError: | |
| detail = e.response.text | |
| return SimulatorResult(success=False, message=str(detail)) | |
| raise | |
| response = await client.post( | |
| f"/v1/repos/{repository}/configuration-simulator", | |
| json={"mergify_yml": mergify_yml}, | |
| ) | |
| # Handle validation errors (422) explicitly to provide a user-friendly message | |
| if response.status_code == 422: | |
| try: | |
| data = response.json() | |
| detail = data.get("detail", str(data)) | |
| except ValueError: | |
| detail = response.text | |
| return SimulatorResult(success=False, message=str(detail)) | |
| # For all other non-success statuses, raise an error explicitly so that | |
| # this function does not rely on client-level response hooks. | |
| response.raise_for_status() |
jd
deleted the
devs/jd/feat/config-validate/Iccecbbc8b9a4511ccde0a3886d90e417c2c9d9c2
branch
🧪 CI InsightsHere's what we observed from your CI run for a042b70. 🟢 All jobs passed!But CI Insights is watching 👀 |
Add an --online flag to
mergify config validatethat sends theconfiguration to the Mergify simulator API for deeper validation
beyond JSON schema checks (e.g., condition syntax, action semantics).
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Depends-On: #1016