Skip to content

fix(catalogs): close resource-consumption gaps in agent catalog access - #333

Open
bradhe wants to merge 1 commit into
developfrom
fix/catalog-agent-resource-limits
Open

fix(catalogs): close resource-consumption gaps in agent catalog access#333
bradhe wants to merge 1 commit into
developfrom
fix/catalog-agent-resource-limits

Conversation

@bradhe

@bradhe bradhe commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

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 xs came back with truncated: false while 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 against serde_json in 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_show listed every table with no limits into model context. The MCP path now runs the listing under the agent ceilings and reports tables_truncated; the CLI show stays 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 file mcp_catalogs.feature does not run in CI. It is gated on TOWER_TEST_CATALOG plus a real TOWER_URL because 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.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: aaa15696-4e52-483f-8a13-344cfef01b40

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/catalog-agent-resource-limits

Comment @coderabbitai help to get the list of available commands.

…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
bradhe force-pushed the fix/catalog-agent-resource-limits branch from ac7310f to 32cf149 Compare July 27, 2026 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant