fix(catalog)+perf(compressor): api_key headers for http MCP servers; post-compression trim (#70782 split-salvage) - #77356
Merged
kshitijk4poor merged 3 commits intoAug 3, 2026
Conversation
kshitijk4poor
enabled auto-merge (rebase)
August 3, 2026 04:51
alt-glitch
added
type/bug
kshitijk4poor
added
ci-reviewed
When an optional-mcps manifest declares transport.type=http with
auth.type=api_key, install_entry() prompts for the key and saves it to
.env, but _build_server_config() only handled the oauth case — the
api_key case produced a bare url entry with no headers, so every
request to the server was unauthenticated (-> 401).
Reuse _bearer_auth_headers(entry.name) from mcp_config.py so the
catalog path emits the same 'Authorization: Bearer ${MCP_..._API_KEY}'
template as the manual 'hermes mcp add --url' path.
Salvaged from NousResearch#70782 (production hunk applied clean; tests re-anchored
onto current main). Credit: JonthanaHanh.
A successful compaction frees the largest allocation a long session ever drops (the compressed-away message dicts), but Python's arena allocator keeps those pages in the heap — RSS retains the pre-compaction high-water mark until exit. NousResearch#76905's trim_memory lifecycle covers the gateway/TUI housekeeping loops but not the CLI compression path. Call trim_memory(reason='post-compression') at the compression-success point in ContextCompressor.compress(), following the house pattern (lazy import in try, debug-level log on failure). The helper is glibc-gated, config-gated and rate-limited, so it is a safe no-op on other platforms and cannot fail compression. Re-expresses the intent of NousResearch#70782 (JonthanaHanh), which reached for a bare gc.collect(); trim_memory is the house mechanism and already wraps a collect.
…v key Simplify-pass follow-up on the NousResearch#70782 salvage: _bearer_auth_headers hard-emits ${MCP_<NAME>_API_KEY} but install_entry only persists auth.env-declared vars — a manifest naming its key differently (the shipped n8n style) would install cleanly yet send a literal-placeholder header at connect time (silent 401, the NousResearch#37792 bug class). Enforce the naming contract at parse time. Also pins the secret-stays-in-.env property in the install test (raw config.yaml carries the template, never the secret). Mutation-checked: validation disabled -> guard test fails.
kshitijk4poor
force-pushed
the
salvage/70782-split
branch
from
August 3, 2026 17:19
292b754 to
5248833
Compare
19 tasks
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
3 participants
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.
Split-salvage of #70782 by @JonthanaHanh — the PR bundled 3 fixes; per-piece verification found 1 already on main, 2 live. Both live pieces land here, the contributor's fix under their authorship via cherry-pick-style commit.
Piece 1 (JonthanaHanh): MCP catalog api_key auth headers — live bug
_build_server_config only handled oauth for http transports: an http + api_key manifest yielded a bare url with NO auth headers, while install_entry dutifully prompted for and saved the key — every connect then 401s. The fix wires _bearer_auth_headers(entry.name) (the same helper the interactive
hermes mcp addflow uses, keeping CLI/Dashboard byte-equivalence). Latent-but-real: no current catalog manifest uses http+api_key, but third-party/future manifests would hit it immediately.Piece 2 (kshitij, crediting the PR's intent): post-compression memory trim
The PR's gc.collect()-after-compression intent was valid — main's compression path releases nothing, and the #76905 trim lifecycle points (gateway tick, TUI turn, agent close) don't cover mid-session CLI compression. Re-expressed via that merged infrastructure: trim_memory(reason="post-compression") after the compression-success point — superior to a bare gc.collect (adds malloc_trim, 60s cooldown coalescing for thrash scenarios, config gating).
Piece 3 (skipped): display_metadata string parsing — already on main (parseDisplayMetadata, commit 6ec319f, more general than the PR's version).
Review follow-up (simplify finding folded)
The api_key fix exposed a latent contract gap: bearer_auth_headers hard-emits ${MCP_API_KEY}, but install_entry only persists auth.env-DECLARED vars — a manifest naming its key differently (the shipped n8n style) would install cleanly yet send a literal-placeholder header (silent 401). Now validated at parse time with a clear CatalogError. Also pinned the secret-stays-in-.env property (raw config.yaml carries the template, never the secret).
Verification
Closes #70782.