fix(config): surface an invalid simulate config as a config error#1679
Conversation
|
This pull request is part of a Mergify stack:
|
Merge Protections🟢 All 7 merge protections satisfied — ready to merge. Show 7 satisfied protections🟢 ⛓️ Depends-On RequirementsRequirement based on the presence of
🟢 🤖 Continuous Integration
🟢 👀 Review Requirements
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 🔎 Reviews
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
987a30e to
f18b064
Compare
9ef5865 to
62bec7d
Compare
Revision history
|
f18b064 to
23baf94
Compare
62bec7d to
0dd17d0
Compare
`queue show` mapped a PR that isn't in the merge queue to exit 6
(Mergify API error) with empty stdout — so `--json` consumers got
nothing parseable for the most common case, and scripts saw an
API-failure code for a routine state.
Report it on stdout and exit 0 instead: a `{"number": N, "queued":
false}` document under `--json`, the `PR #N is not in the merge
queue` line in human mode.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Change-Id: I6f7ca3cf3cffb83ba129a1c48d2aab218dabd989
A schema-invalid config uploaded by `config simulate` came back as the simulator's HTTP 422 and was reported as a Mergify API failure (exit 6) — even though the same fixture fails `config validate` client-side with exit 8. The drift meant a local-config problem looked like a service problem to scripts. Add `HttpClient::post_classifying`, which lets a caller remap a terminal status before the default flavor mapping, and use it in `simulate` to map the simulator's 422 to a configuration error (exit 8). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: Ie77d8f689eef92cd71201a67dc793742ec7fe52b
0dd17d0 to
f2561e6
Compare
23baf94 to
75d611c
Compare
There was a problem hiding this comment.
Pull request overview
This PR adjusts HTTP error classification so config simulate reports a schema-invalid local config as a configuration error (exit 8) instead of a Mergify API failure (exit 6), and also updates queue show to treat “PR not in merge queue” (404) as a normal state with stdout output and exit 0.
Changes:
- Add
HttpClient::post_classifyingto allow callers to remap specific terminal HTTP statuses beforeApiFlavorerror mapping. - Use
post_classifyinginconfig simulateto map simulator HTTP 422 toCliError::Configuration(exit 8), with regression tests. - Change
queue show404 behavior to emit a not-queued message/document on stdout and return success, updating unit + live smoke tests accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
crates/mergify-queue/src/show.rs |
Treat 404 “not queued” as a normal result; emit stdout output in human/JSON modes; update tests. |
crates/mergify-core/src/http.rs |
Introduce post_classifying and a classifier hook into the retry/execution path; add unit tests. |
crates/mergify-config/src/simulate.rs |
Map simulator 422 responses to configuration errors using the new HTTP helper; add regression test. |
crates/mergify-cli/tests/live_smoke.rs |
Update pinned contract for queue show not-queued case (stdout + exit 0). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Queued — the merge queue status continues in this comment ↓. |
Merge Queue Status
This pull request spent 8 minutes 14 seconds in the queue, including 7 minutes 44 seconds running CI. Required conditions to merge
|
A schema-invalid config uploaded by
config simulatecame back asthe simulator's HTTP 422 and was reported as a Mergify API failure
(exit 6) — even though the same fixture fails
config validateclient-side with exit 8. The drift meant a local-config problem
looked like a service problem to scripts.
Add
HttpClient::post_classifying, which lets a caller remap aterminal status before the default flavor mapping, and use it in
simulateto map the simulator's 422 to a configuration error(exit 8).
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Depends-On: #1678