Skip to content

fix(webapp): precise S2 record cap + CORS 413 on session append#3720

Merged
matt-aitken merged 2 commits into
mainfrom
fix/realtime-append-precise-cap
May 23, 2026
Merged

fix(webapp): precise S2 record cap + CORS 413 on session append#3720
matt-aitken merged 2 commits into
mainfrom
fix/realtime-append-precise-cap

Conversation

@ericallam
Copy link
Copy Markdown
Member

Summary

Two improvements to session .in/append:

  • Oversize-body 413 responses now carry CORS headers, so browser fetches see a readable status instead of an opaque TypeError: Failed to fetch. App-side retry-on-disconnect loops no longer spin forever on a permanently-rejected payload.
  • The per-record cap is now computed precisely against S2's actual ceiling instead of the conservative 512 KiB floor. Legitimate ~600-900 KiB tool outputs (search results, file content) now succeed; pathological all-quote content that would double under JSON escape still rejects cleanly.

Design

S2 enforces a per-record metered size of 8 + 2*H + Σ(header name + value) + body ≤ 1048576 bytes. With no record headers (our case), the budget reduces to body ≤ 1048568. Verified empirically against cloud S2 — append succeeds at metered=1048576 and 422s at 1048577 with record must have metered size less than 1 MiB.

The old MAX_APPEND_BODY_BYTES = 512 KiB was derived by assuming worst-case JSON escape doubling (every byte becomes \" or \\), giving (1 MiB - overhead) / 2. Safe, but rejects ~half the legitimate input space.

The new flow:

  1. Pre-cap the HTTP body at 1 MiB (DoS guard against reading arbitrary garbage before we can compute the wrap).
  2. After reading, S2RealtimeStreams.#appendPartByName computes Buffer.byteLength(JSON.stringify({data: part, id: partId}), "utf8") + 8 and throws S2RecordTooLargeError (a ServiceValidationError with status 413) if it would exceed S2's ceiling. The route's existing error branch maps the throw to a 413 with a descriptive message.

The 413 CORS fix is a single-line change in apiBuilder.server.tswrapResponse was being skipped on the body-too-large branch; every other error branch wraps; the 413 was the exception.

Test plan

  • Empirically verified against cloud S2 with a boundary scan across [1048568, 1048569, ..., 1048576] and across H ∈ {0, 1×5 hdr bytes, 1×14 hdr bytes} — the formula matches exactly
  • Browser-side fetch on a 700 KiB POST now resolves with a readable status: 413 (no TypeError: Failed to fetch)
  • A 900 KiB ASCII tool output now passes (would have 413'd at 512 KiB pre-fix)

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 23, 2026

⚠️ No Changeset found

Latest commit: 62aaeef

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 23, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8f9f46c4-3832-4256-9d77-5458a44cbb2d

📥 Commits

Reviewing files that changed from the base of the PR and between 7b7a600 and 62aaeef.

📒 Files selected for processing (1)
  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead

Files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use zod for validation in packages/core and apps/webapp

Files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

**/*.{ts,tsx,js,jsx}: Prefer static imports over dynamic imports. Only use dynamic import() when circular dependencies cannot be resolved otherwise, code splitting is needed for performance, or the module must be loaded conditionally at runtime.
Import from @trigger.dev/core using subpaths only - never import from the root.
When writing Trigger.dev tasks, always import from @trigger.dev/sdk. Never use @trigger.dev/sdk/v3 or deprecated client.defineJob.
Add agentcrumbs markers (// @Crumbs or `#region `@crumbs) as you write code, not just when debugging. They stay on the branch throughout development and are stripped by agentcrumbs strip before merge.

Files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)

**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries

Files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: Access environment variables through the env export of env.server.ts instead of directly accessing process.env
Use subpath exports from @trigger.dev/core package instead of importing from the root @trigger.dev/core path

Use named constants for sentinel/placeholder values (e.g. const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons

Files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}

📄 CodeRabbit inference engine (AGENTS.md)

Code formatting must be enforced using Prettier before committing

Files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
🧠 Learnings (5)
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).

Applied to files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.

Applied to files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.

Applied to files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.

Applied to files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.

Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.

Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.

Applied to files:

  • apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts
🔇 Additional comments (2)
apps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.ts (2)

2-2: LGTM!

Also applies to: 17-17


82-104: LGTM!


Walkthrough

This PR adds per-record metered-size enforcement to realtime session append operations. The S2 service layer now exports size constants and a S2RecordTooLargeError class, validates records by JSON-encoding and computing UTF-8 byte length plus overhead, and rejects records exceeding the 1 MiB limit with HTTP 413. Route-level HTTP body caps are increased from 512 KiB to 1 MiB as a DoS pre-guard. Error responses are wrapped with CORS handling to respect configured CORS strategies. The changelog documents the transition from opaque fetch errors to readable 413 responses.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the two main improvements: precise S2 record cap and CORS 413 handling on session append, matching the core changes in the changeset.
Description check ✅ Passed The description covers all required template sections: it includes a summary, design rationale, test plan, and changelog. The author provides comprehensive context on the changes, design decisions, and verification approach.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/realtime-append-precise-cap

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@matt-aitken matt-aitken merged commit c0365d3 into main May 23, 2026
29 of 30 checks passed
@matt-aitken matt-aitken deleted the fix/realtime-append-precise-cap branch May 23, 2026 15:08
pull Bot pushed a commit to Dustin4444/trigger.dev that referenced this pull request May 23, 2026
…act (triggerdotdev#3721)

## Summary

Updates the AI chat docs to match the slim-wire + field-level merge
behavior shipped in triggerdotdev#3719 and the precise `.in/append` cap +
CORS-readable 413 shipped in triggerdotdev#3720. No behavior changes here — code is
correct in `main`; the docs were lagging on three patterns customers
copy out of the page.

## What changed

- **`hydrateMessages` examples upsert by id** (in `lifecycle-hooks.mdx`,
`patterns/database-persistence.mdx`, and
`patterns/persistence-and-replay.mdx`). The previous
`stored.push(newMsg)` pattern duplicated the assistant id on HITL
continuations and caused the LLM to receive a tool call with no
`arguments`. The new examples include the rationale inline.
- **`onValidateMessages` example filters to user messages**
(`lifecycle-hooks.mdx`). The previous example called
`validateUIMessages({ messages, tools })` directly, which now throws on
HITL slim wires (the AI SDK schema requires `input` on resolved tool
parts). New example shows the filter pattern, with a Warning callout
explaining why.
- **Merge contract description updated** (`lifecycle-hooks.mdx`). The
old wording said incoming messages are "auto-merged" / "replaced"; the
new description explains the actual field-level overlay (state advances
only).
- **Approval-responded wire example slimmed** (`client-protocol.mdx`).
Shows the minimum shape the agent reads — `state` + `approval` (or
`output` / `errorText` for HITL). Notes that the built-in transports
ship this slim shape by default and that fuller shapes are still
accepted.
- **`/in/append` 413 row and FAQ updated** (`client-protocol.mdx`,
`patterns/trusted-edge-signals.mdx`). Reflects the new precise S2 cap
and the CORS-readable 413.
- **New changelog entry** at the top of `changelog.mdx` covering all of
the above.

The historical `## 512 KiB ceiling removed` entry further down the
changelog is left as-is (it's a snapshot of the prior transition), and
the v4.5 upgrade-guide section is skipped — the merge contract is
backwards compatible.

## Test plan

- Mintlify dev preview renders cleanly with no broken anchors
- Linked references resolve (`/ai-chat/lifecycle-hooks#hydratemessages`,
`/ai-chat/lifecycle-hooks#onvalidatemessages`,
`/ai-chat/patterns/database-persistence#alternative-hydratemessages`,
`/ai-chat/client-protocol#step-3-send-messages-stops-and-actions`,
`/ai-chat/patterns/large-payloads`)
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.

2 participants