feat(mcp): add --output-max-size for output dir eviction#41021
Open
Skn0tt wants to merge 12 commits into
Open
Conversation
Adds an integer-byte budget for the MCP/CLI output directory. Before each tracked write, the oldest evictable artifacts are deleted to keep the total under the cap. Session logs and per-tab log files are pinned (they still count toward the budget, but are never evicted). Trace artifacts are resolved through the cap but intentionally never report their size, since a live trace grows unbounded. Config surface: `outputMaxSize` field, `--output-max-size <bytes>` CLI flag, and `PLAYWRIGHT_MCP_OUTPUT_MAX_SIZE` env var. Internally, `Context.outputFile` now returns an `OutputFile` (resolved through the new per-context `OutputDir`), so every cap-tracked write flows through a single chokepoint: `OutputFile.write`, `.append`, and `.trackSize`. Workspace-branch files resolved via `resolveClientFile` are pinned non-evictable so user files are never auto-deleted.
…ir files as evictable - In Response.resolveClientFile, files whose suggested filename lands inside outputDir are now evictable (workspace files remain pinned). - Collapse the spec down to two focused tests with helpers inlined.
Collapse parallel _files/_entries maps into one Map<path, OutputFile>; insertion order encodes age. Single _evict loop replaces _evictForBytes plus _oldestEvictable. Each write/append/trackSize tracks the file's own size and re-inserts itself to mark "newest". Also: Response no longer special-cases suggested filenames; OutputDir.resolve defaults `evictable` based on whether the resolved path lives inside outputDir.
OutputFile.write/append no longer mkdir on every call; OutputDir.resolve is now async and ensures the parent dir exists once. Drop the duplicate mkdir from context.outputFile.
Log resolve, write, append, and evict actions with sizes through the 'pw:mcp:file' channel; drop the redundant log in context.outputFile.
Size 0 means we haven't accounted for the file's bytes, so unlinking it could delete something we don't own.
This comment has been minimized.
This comment has been minimized.
Contributor
Test results for "tests 1"3 failed 3 flaky43922 passed, 861 skipped Merge workflow run. |
Member
pavelfeldman
left a comment
pavelfeldman
left a comment
There was a problem hiding this comment.
I made some random comments, but I don't think the approach is quite right. I would examine response for the new output folder entries and would remove the oldest entries instead.
Per review on microsoft#41021. We'll file an issue if we want this later.
No caller passes it. Workspace files are classified non-evictable by OutputDir.resolve via isPathInside; outputDir files are evictable. Removing the param avoids suggesting client files could be pinned.
Skn0tt
added a commit
to Skn0tt/playwright
that referenced
this pull request
May 28, 2026
Per review on microsoft#41021. The previous "We don't own writing so we cannot track file size for this" comment meant traces were pinned outside the budget, so a single trace could blow the cap unboundedly. Drop `evictable: false` from the traces directory and, on tracingStop, walk the produced trace files via OutputDir.trackTree to register their sizes. Old traces become evictable like any other artifact. We still can't account for the trace mid-recording (no hooks), but post-stop accounting is enough to keep multi-trace sessions under cap.
Skn0tt
force-pushed
the
feat-output-max-size
branch
from
aa6654c to
1883162
Compare
Member
Author
Is #41031 what you have in mind? I put that up as an alternative PR and updated this one to address your comments. Here's the main differences I see between the approaches:
|
Contributor
Test results for "MCP"1 failed 7232 passed, 1113 skipped Merge workflow run. |
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
2 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.
Summary
--output-max-size <bytes>option (CLI / config / env) for the MCP output directory.