Tail remote MCP logs before downloading - #1797
Merged
Merged
Conversation
Contributor
ApprovabilityVerdict: Approved Small performance optimization to the You can customize Macroscope's approvability policy. Learn more. |
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.
Overview
Bound MCP server failure-log collection at the runtime boundary so diagnostics no longer transfer and decode an entire remote log just to keep its suffix.
Why
log_tailpreviously calledruntime.read(log)and only appliedlimitafter the complete file reached the host and was decoded. A large server log could therefore consume hundreds of MiB and block the event loop while the launcher was already trying to report a startup failure. In remote runtimes, the unnecessary file transfer adds another unbounded cost.Change
tail -c <limit> <log>inside the runtime with separate, argv-safe arguments.tailsucceeds while preserving missing/unreadable log behavior as an empty diagnostic.[-0:]behavior that returned the entire stripped log.log_tail; successful server launches and their control flow are unchanged.Performance impact
A three-round median benchmark over a 256 MiB log measured:
Wall time used
perf_counter, loop stall used a 1 ms asyncio heartbeat, and host allocation usedtracemalloc. The new measurement includes process startup fortail.Tradeoff
Tiny failure logs may pay a remote command-startup cost. This path only runs while enriching launch failures, and bounding the transferred data prevents error reporting from downloading an arbitrarily large log.
Note
Low Risk
Change is confined to error-enrichment on failed server launch and assumes standard
tailin runtimes; no auth or rollout success-path behavior changes.Overview
log_tailno longer pulls the full remote log into the host and slices it locally. It runstail -c <limit>inside the runtime so failure diagnostics stay bounded (default 2000 bytes), which avoids huge transfers and decode stalls when MCP startup fails on large logs.Nonpositive
limitnow returns""immediately instead of relying on Python slicing that could return the entire stripped log forlimit == 0. Successful launches are unchanged; this only affects log snippets attached toToolsetErrorpaths inserve_in_runtime.Reviewed by Cursor Bugbot for commit 65c4f52. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Use
tail -cto read last N bytes of remote MCP logs before downloadingUpdates
log_tailin launch.py to invoketail -c <limit>viaruntime.runinstead of reading and slicing the full log file in memory. Returns an empty string for non-positive limits or non-zero exit codes, and no longer strips whitespace or applies error-replacement decoding.Macroscope summarized 65c4f52.