Repository navigation
Park a server whose credential was rejected instead of retrying it forever - #20
Merged
M3gA-Mind merged 1 commit intoSep 22, 2026
Conversation
…rever An MCP server answering 401 was penalised on the backoff curve like any other failure. The curve caps at BACKOFF_MAX, so the server was re-dialled every five minutes for the life of the session — 18+ attempts in one reported session, each writing a warning, against an endpoint that had already said no (openhuman#6412). Waiting does not turn a 401 into a 200. Only the user supplying a working credential does, and that arrives through the store, not through time. This is the same shape as a missing runtime, which the supervisor already parks with exactly that reasoning, so route a rejected credential into the same terminal path rather than inventing a second one. The existing park machinery then covers the rest of the report for free: a parked server is reported once and stays quiet, so the log noise stops at one line; `terminal` is already cleared when a server is disabled, so toggling it off and on after setting a token is the way back, the same gesture that already clears a backoff penalty; and `ServerStatus::Unauthorized` already reaches the host, so the state is visible rather than merely silent.
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Ready for maintainer review Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Before mergeNone. How this fits togetherflowchart LR
n0["Supervisor<br/>changed"]:::changed
n1["new"]:::impacted
n2["install"]:::impacted
n3["insert_server"]:::impacted
n4["supervisor"]:::impacted
n5["connected_to"]:::impacted
n2 -->|calls| n1
n4 -->|uses| n0
n4 -->|calls| n1
n5 -->|calls| n1
n5 -->|calls| n2
n5 -->|calls| n3
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Comment |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0052 · 101,047 in / 4,152 out · 26,251 cached (26%) · flash, ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 347 embedded
critique: $0.0018 · 30,992 in / 1,120 out · 2,119 cached (7%) · gpt-5.6-luna
security: $0.0017 · 30,504 in / 858 out · 1,875 cached (6%) · gpt-5.6-luna
tests: $0.0007 · 26,807 in / 728 out · 18,872 cached (70%) · deepseek/deepseek-v4-flash
description: $0.0005 · 5,391 in / 78 out · 0 cached (0%) · deepseek/deepseek-v4-flash
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
1 participant
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.
What changed and why
An MCP server answering HTTP 401 was penalised on the backoff curve like any other failure. The curve caps at
BACKOFF_MAX, so the server was re-dialled every five minutes for the life of the session — 18+ attempts in one reported session, each writing a warning, against an endpoint that had already said no.Waiting does not turn a 401 into a 200. Only the user supplying a working credential does, and that arrives through the store, not through time.
The supervisor already has a terminal/park path, used for
Error::MissingRuntime, whose comment states exactly this reasoning: "a penalty says 'wait, then try again', and there is nothing to wait for". A rejected credential is the same shape, so this routes it into that existing path rather than inventing a second mechanism.Error::is_unauthorized()already existed and is already tested (transport/http/test.rs,a_401_becomes_a_typed_unauthorized_error); it simply was never consulted here.Public API / behavior changes
Behavior: a server whose credential is rejected now emits
SupervisorEvent::Parkedon the first attempt instead ofReconnectFailedon every attempt forever. It is skipped by subsequent ticks until the user disables and re-enables it.Public API: None.
Deliberate consequence worth review: a server that answers 401 transiently — say, an expired token its own operator rotates server-side without the user touching anything — will now stay parked until toggled, where before it would eventually reconnect by itself. I judged that the right trade: the reported case (a credential that was never supplied) is the common one, and the toggle is discoverable because the status is visible. If you would rather 401 kept a bounded number of retries before parking, that is a one-line change to make here and I am happy to make it.
What this gets for free
These fall out of the existing park machinery rather than needing new code:
a_parked_server_is_reported_once_and_then_stays_quietalready covers this.terminalis already cleared on disable, so toggling after setting a token reconnects; the same gesture that already clears a backoff penalty.ServerStatus::Unauthorizedalready flows to the host.Validation
Run from
crates/tinymcp's workspace root, in the order the repository's CLAUDE.md specifies:cargo fmt --all -- --checkcargo test --all-featurescargo clippy --all-targets --all-features -- -D warningsClippy fails on
error: unknown lint: clippy::unused_async_trait_impl. I verified this is inherited rather than mine by reverting both changed files to the pinned commit withgit checkout --and re-running: byte-identical error, same exit 101, with none of my changes present. My toolchain isclippy 0.1.96 (31fca3adb2 2026-06-26), which does not know that lint; a newer one does. Nothing in this PR touches a lint declaration.Test
a_401_is_terminal_and_earns_no_backoff_penalty, modelled on the existinga_missing_runtime_is_terminal_and_earns_no_backoff_penaltybeside it, with a loopback axum route that answers every request 401.Written before the fix, and it failed on the assertion that names the defect:
After the fix it passes. It asserts the server is parked not penalised, that it is still parked after
BACKOFF_MAX * 2(so a penalised server would certainly have been retried by then), and that disabling clears the verdict.Related
tinyhumansai/openhuman#6412openhumangitlink bump forvendor/tinymcp; that is separate work and not part of this PR.