fix(catalogs): close resource-consumption gaps in agent catalog access - #333
Open
bradhe wants to merge 1 commit into
Open
fix(catalogs): close resource-consumption gaps in agent catalog access#333bradhe wants to merge 1 commit into
bradhe wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…ccess This commit addresses four ways an agent query could spend more than the ceilings intended: The byte ceiling now counts the exact serialized size of each value, including quotes, escapes, and separators. The old estimate counted an array of empty strings as roughly zero bytes, so a nested value like list_transform(range(10000000), x -> '') serialized to 100+ MB while passing a 1 MiB ceiling. The MCP layer also serializes catalog results compactly now, since pretty-printing an array of arrays multiplies its size several-fold past the ceiling the rows were admitted under. Agent-facing error messages are bounded to 4 KiB. A DuckDB error can echo the offending value (a failed CAST reproduces the whole string it was given), which made the error channel an unbounded output path around the result ceilings. Full errors still go to the debug log. DuckDB sessions now run inside a process-wide two-slot budget, and a dropped MCP request interrupts its running query instead of leaving it to burn its ceilings on a thread nobody is waiting on. Without this, concurrent agent calls multiplied the 1 GiB memory / 2 GiB spill ceilings per call. Catalog discovery through MCP runs under the agent ceilings too, with a tables_truncated flag, so a huge catalog cannot flood a model's context through SHOW ALL TABLES. The CLI show path stays unbounded, since a person asked for the listing.
bradhe
force-pushed
the
fix/catalog-agent-resource-limits
branch
from
July 27, 2026 21:23
ac7310f to
32cf149
Compare
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.
This PR addresses resource-consumption feedback on the DuckDB agent query path (follow-up to #328, supersedes #332 which was closed by a branch rename). The ceilings on agent catalog access did not all hold in practice; each gap below let a query spend or return more than intended.
Nested JSON walked under the 1 MiB response cap. The byte counter approximated serialized size and ignored quotes, escapes, and separators, so an array of empty strings counted as roughly zero bytes.
SELECT list_transform(range(10000000), x -> '') AS xscame back withtruncated: falsewhile serializing to 120 MB, which defeats the point of having a response cap at all. The counter now computes the exact compact-JSON size (asserted againstserde_jsonin tests), the budget check runs per value so an oversized row stops converting early, and the MCP layer serializes catalog results compactly so pretty-print whitespace cannot outgrow the ceiling the rows were admitted under.Errors were an unbounded output channel. A DuckDB error can echo the offending value:
SELECT CAST(repeat('x', 2000000) AS INTEGER)produced a 2 MB error containing the whole string, so an aggregate over a catalog column could exfiltrate past the result caps through the error message. Agent-facing errors are now bounded to 4 KiB, redacted before bounding so truncation cannot leave the token intact, with the full redacted message kept in the debug log.Parallel calls multiplied the per-query limits. Every MCP request opened its own session, each entitled to 1 GiB of engine memory and 2 GiB of spill, and cancelling a request left the query running. Sessions now run inside a process-wide two-slot semaphore, so the worst case is bounded at twice the per-session ceilings instead of scaling with request count. I considered fully serializing sessions, but one slow analytical scan would then block every other call, and two slots keeps that from happening while staying bounded. A dropped request future now interrupts its running query via a cancel handle, and a request cancelled while queued never starts.
Discovery was unbounded.
tower_catalogs_showlisted every table with no limits into model context. The MCP path now runs the listing under the agent ceilings and reportstables_truncated; the CLIshowstays unbounded because a person asked for the listing.On CI coverage: the Rust tests, including the adversarial suite and the new regression tests here, run on every PR via
cargo test --all-features. The behave e2e filemcp_catalogs.featuredoes not run in CI. It is gated onTOWER_TEST_CATALOGplus a realTOWER_URLbecause it exercises a live attach against a real storage catalog, and CI only has the mock server. That gating is intentional, but it means those scenarios only run when someone points them at a real environment.