fix(live-tests): move tests show onto the admin token - #1798
Open
jd wants to merge 1 commit into
Open
Conversation
Contributor
Merge Protections🔴 2 of 6 protections blocking · waiting on 👀 reviews
🔴 👀 Review RequirementsWaiting for
This rule is failing.
🔴 🔎 ReviewsWaiting for
This rule is failing.
Show 4 satisfied protections🟢 🤖 Continuous Integration
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
There was a problem hiding this comment.
🟢 Approval recommended
The changes align the live tests and docs with the intended authorization model and are low-risk (token selection + documentation only).
Pull request overview
This PR updates the live functional smoke tests and supporting documentation to reflect that CI-Insights test health endpoints are intentionally not accessible with the CI-scoped application key (403 by design), and must be exercised with the admin-scoped token instead.
Changes:
- Switch
tests_show_no_matchlive smoke test from the CI token to the admin token. - Clarify in the live test token helper docs that the admin token covers both queue-management endpoints and CI-Insights test reads.
- Update the
mergify-ciskill documentation to state thattests showand quarantine mutations require an admin key (or PAT), and that CI keys are rejected.
File summaries
| File | Description |
|---|---|
| skills/mergify-ci/SKILL.md | Documents that CI-Insights test reads and quarantine mutations require admin-scoped auth (CI key 403s). |
| crates/mergify-cli/tests/live_smoke.rs | Moves tests show live smoke coverage onto the admin token and updates token-scope rationale. |
| .github/workflows/func-tests-live.yaml | Updates workflow commentary to include tests show among admin-token-covered endpoints. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
MRGFY-9001 cut the `ci` application key to what a CI job actually
needs, and `GET /ci/{owner}/repositories/{repo}/search/tests` lost
`ci_application_key` along with it. `tests_show_no_match` still
passed the CI-scoped token, so every live run since has failed with
a 403 on that one test — on pull requests that touch nothing near
it.
Reading test health is a developer inspecting a repository, not
something a CI job does, so the 403 is the intended answer and the
test was asserting an obsolete contract. It moves to the admin
token, under the admin banner with the queue and freeze tests that
were separated for the same reason.
The narrowing also took the two quarantine mutations the CLI makes
(`tests quarantines add` and `remove`); no live test covers those,
so they broke silently. Three doc surfaces promised any token
worked, and AGENTS.md makes README and `--help` the owners of the
token contract, so all three now say which key each command needs:
- `skills/mergify-ci/SKILL.md` grows one `## Authentication`
section glossing the two key classes, replacing the per-command
repetition the sibling merge-protections skill already avoids.
It also states the rule for the commands that keep working, so
the silence on `quarantines list`/`get` is readable rather than
looking like an oversight.
- README documents the split at group altitude, next to the
existing token resolution order.
- `--help` for the three affected commands says it inline. Those
doc comments feed `cli_schema.rs`, which the docs site renders,
so the golden snapshot moves with them.
While in that section: the `tests show` exit-code table claimed `1`
= flaky and `6` = broken, but `tests_show::run` returns
`ExitCode::Success` on every rendered path — pinned by
`unhealthy_tests_still_exit_success`. A CI gate written from that
table never fired, and it read this commit's own 403 (exit 6, a
`CliError`) as "a broken test". Corrected to what the command does.
Nothing in `Mergifyio/mergify-ci-integrations` is affected: its
whole API surface is `traces`, `quarantines` (list),
`flaky-detection-context` and `test-selection`, all of which the
narrowing kept.
Verified on CI: run 33880789655 reports `tests_show_no_match ... ok`
with 12/12 green, so the admin key does carry CI-Insights read
scope. `search/tests` is the only route the suite covers —
`tests/{test_id}` is narrowed the same way but the no-match query
returns before the details fetch, which the helper's doc now says.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JBz3hxMUDCuWftT6qAHtnn
Change-Id: I4e4c3e308adb7dbd68052ef9b03c2e42381dd8ce
jd
force-pushed
the
devs/jd/fix/live-tests-tests-show-admin-token/move-tests-show-onto-admin-token--4e4c3e30
branch
from
September 4, 2026 14:39
4b37722 to
c0055cb
Compare
Member
Author
|
This pull request is part of a Mergify stack:
|
Member
Author
Revision history
|
sileht
approved these changes
Sep 5, 2026
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
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.
MRGFY-9001 cut the
ciapplication key to what a CI job actuallyneeds, and
GET /ci/{owner}/repositories/{repo}/search/testslostci_application_keyalong with it.tests_show_no_matchstillpassed the CI-scoped token, so every live run since has failed with
a 403 on that one test — on pull requests that touch nothing near
it.
Reading test health is a developer inspecting a repository, not
something a CI job does, so the 403 is the intended answer and the
test was asserting an obsolete contract. It moves to the admin
token, under the admin banner with the queue and freeze tests that
were separated for the same reason.
The narrowing also took the two quarantine mutations the CLI makes
(
tests quarantines addandremove); no live test covers those,so they broke silently. Three doc surfaces promised any token
worked, and AGENTS.md makes README and
--helpthe owners of thetoken contract, so all three now say which key each command needs:
skills/mergify-ci/SKILL.mdgrows one## Authenticationsection glossing the two key classes, replacing the per-command
repetition the sibling merge-protections skill already avoids.
It also states the rule for the commands that keep working, so
the silence on
quarantines list/getis readable rather thanlooking like an oversight.
existing token resolution order.
--helpfor the three affected commands says it inline. Thosedoc comments feed
cli_schema.rs, which the docs site renders,so the golden snapshot moves with them.
While in that section: the
tests showexit-code table claimed1= flaky and
6= broken, buttests_show::runreturnsExitCode::Successon every rendered path — pinned byunhealthy_tests_still_exit_success. A CI gate written from thattable never fired, and it read this commit's own 403 (exit 6, a
CliError) as "a broken test". Corrected to what the command does.Nothing in
Mergifyio/mergify-ci-integrationsis affected: itswhole API surface is
traces,quarantines(list),flaky-detection-contextandtest-selection, all of which thenarrowing kept.
Verified on CI: run 33880789655 reports
tests_show_no_match ... okwith 12/12 green, so the admin key does carry CI-Insights read
scope.
search/testsis the only route the suite covers —tests/{test_id}is narrowed the same way but the no-match queryreturns before the details fetch, which the helper's doc now says.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01JBz3hxMUDCuWftT6qAHtnn