Skip to content

fix(tools): bound internal request serialization - #7639

Open
BillLeoutsakosvl346 wants to merge 2 commits into
stagingfrom
fix/platform-request-admission
Open

fix(tools): bound internal request serialization#7639
BillLeoutsakosvl346 wants to merge 2 commits into
stagingfrom
fix/platform-request-admission

Conversation

@BillLeoutsakosvl346

@BillLeoutsakosvl346 BillLeoutsakosvl346 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Enforce the existing 10 MiB internal-operation request limit during native JSON serialization, before the complete oversized JSON string is allocated.
  • Preserve native traversal, escaping, omissions, boxed primitive conversion, and single getter/toJSON invocation; retain the final exact UTF-8 check and existing user-facing size error.
  • Preserve structured oversized-request diagnostics using an observed byte lower bound and configured limit, without logging payload contents.
  • Leave external HTTP and MCP request checks unchanged. Allocations performed inside user getters/toJSON hooks are outside this guard.

Validation

  • 166 focused tests passed across request-size behavior and shared tool execution.
  • App type-check, changed-file Biome, API validation, and git diff --check passed.
  • Focused regression proves oversized input aborts before later getters and the internal handler execute.
  • Controlled local browser regression passed through Start → File Write → File Read: temporary synthetic oversized internal input produced the existing 10 MB error in the workflow terminal and notification; after removing the temporary fixture, the same workflow successfully returned file output. No fixture or test-only route is included in this PR.

Four scoped files changed; no generated artifacts or authentication changes.

@vercel

vercel Bot commented Sep 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 9, 2026 1:36am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

The PR appears safe to merge, with a non-blocking observability regression in the oversized internal-request path.

Findings

  1. P2 Size telemetry is lost

Summary

  • Adds incremental accounting for JSON strings, primitives, arrays, objects, boxed primitives, and raw JSON.
  • Converts incremental limit failures to the existing user-facing request-size error before invoking internal handlers.
  • Adds focused tests for native serialization parity, hook invocation counts, error behavior, and early traversal termination.
  • The implementation preserves behavior, but the replacement path drops the structured size measurements previously logged for oversized requests.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Declared internal operation] --> B[Build operation input]
    B --> C[Bounded native JSON traversal]
    C -->|Budget exceeded| D[Translate to existing 10 MiB error]
    D --> E[Generic execution error handling]
    C -->|Within budget| F[Exact serialized UTF-8 check]
    F --> G[Execute internal operation handler]
Loading

Comment thread apps/sim/tools/index.ts
Comment on lines +2692 to +2696
try {
stringifyRequestWithinLimit(operationInput, MAX_REQUEST_BODY_SIZE_BYTES)
} catch (error) {
if (isPayloadSizeLimitError(error)) throw new Error(BODY_SIZE_LIMIT_ERROR_MESSAGE)
throw error

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Size telemetry is lost

Oversized internal-operation inputs now skip the structured log fields that recorded the observed body size and configured limit. The generic execution error is still logged, but without these measurements, request-limit incidents are harder to diagnose and quantify. Please preserve equivalent size-limit telemetry in the new bounded serialization path.

Knowledge Base Used: Integrations, connectors, and tools

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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