Skip to content

fix(v2): close the defects live probing found - #6681

Merged
waleedlatif1 merged 3 commits into
stagingfrom
fix/v2-live-findings
Aug 13, 2026
Merged

fix(v2): close the defects live probing found#6681
waleedlatif1 merged 3 commits into
stagingfrom
fix/v2-live-findings

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Staging finally deployed the merged release, so the v2 surface could be exercised for real. Every fix already shipped held up — cursor scope ordering, NUL rejection, CORS exposeHeaders, the enrichment-group P1, predicate cursor ordering, cost/duration refines, workflowGroupId rejection, and tag-filter value coercion all verified live. These are the defects only live traffic surfaced, plus the ones a static sweep had found and left.

Every finding below was reproduced against staging before any code changed, and each fix was verified red-then-green.

P1 — a cursor named a position without naming the sequence

cursorScopeKey hashed only the caller's filters, so any two lists filtering on nothing but workspaceId produced one fingerprint and accepted each other's tokens. Seven of seven same-sort pairs returned 200, and a tables cursor replayed against /v2/knowledge silently skipped a row. Table rows never reached that check at all, so a cursor from one table paged another — and POST /query shared the defect whenever no predicate was present.

Identity now comes from the route's own contract — method plus resolved path — rather than a per-route literal, because a hand-written name is the step an author forgets and forgetting it is invisible. An unresolved path placeholder throws rather than fingerprinting the template, so a misconfigured route fails on every request instead of an unlucky one. A source sweep now enforces that every route calling cursorScopeKey passes its contract — closing the gap that let this hide, since list-pagination.test.ts reads contracts and never the route's actual call.

Every token minted before this is refused with an accurate message. They are single-walk and unpersisted.

P0 — the two knowledge surfaces answered different questions

Search grouped same-tag filters by slot and joined them with OR; the list conjoined every filter. So gte 9 AND lte 2 on one tag returned 0 documents from the list and a full billed page from search.

Search now conjoins. The OR grouping replaced an explicit |OR| mechanism that was deleted outright, was never documented in any contract, and cost the ability to express a range on a single tag — the union it provided is still reachable as separate searches. The search body also accepted an unbounded query that was billed and then silently truncated to the embedding window, and ignored the tag-filter cap the list enforces.

A body over 10 MiB was reported as malformed JSON

Proven by sending an 11 MB body whose JSON terminated at 9 MB with trailing padding — it parsed, so the body is silently truncated, not stream-errored. The cause is Next's proxyClientMaxBodySize default of 10 MiB, well under this app's 50 MB ceiling, leaving the size branch unreachable. The ceiling is clamped to what the proxy passes. (Math.min is load-bearing: the env default is hard-coded to 50 MB, so editing the constant alone would have been inert.)

Also fixed

  • A group's outputColumns accepted a workflowGroupId and discarded it — the sibling of the create-body fix, via an omit()-derived nested shape the top-level .strict() doesn't bind
  • An enrichment group could never gain an output: a new coordinate demanded workflow metadata a group with no workflow cannot have
  • newOutputColumns alone reported success and created nothing
  • A saved view stored layout references to columns that do not exist, while refusing the same name in a filter
  • MCP: retries: 0 stored as 3; an explicit authType overridden by oauthClientId; a disabled server answering tool discovery with an unclassified 500; a header rotation leaving the server reading connected; and the authType PATCH default that revokes a stored OAuth grant when an SDK materializes it
  • A run whose workflow was deleted reported folderPath: "/" while also reporting deleted: true (9,642 production rows have workflow_id IS NULL)

Where the honest fix was out of reach, the contract was corrected instead

The polled run resource rebuilds error.code by matching the persisted message, so it can never report the two codes needing block attribution — it now says so, and the PR records the exact three-file delta for the real fix (no migration needed; the trace spans already carry the block). OUTPUT_TOO_LARGE is removed — no path ever emitted it.

One finding deliberately not fixed

triggers reads as a closed enum, but production holds 43 distinct values — a webhook run stores its provider id, including both microsoft-teams and microsoftteams. Pinning the enum would refuse legitimate history that a diagnostic log search exists to find. The description now states the vocabulary is open and matching is case-sensitive.

Verification

  • Full suite: 1,859 files / 24,760 tests passed, 0 failed
  • type-check 23/23, biome, check:audits 26/26, check:api-validation — all clean
  • OpenAPI regenerated across 5 documents

Staging finally deployed the merged release, so the surface could be
exercised for real. Every fix already shipped held up. These are the
defects only live traffic surfaced, plus the ones a static sweep had
found and left.

A cursor named a position in a sequence without naming the sequence.
`cursorScopeKey` hashed only the caller's filters, so any two lists
filtering on nothing but `workspaceId` produced one fingerprint and
accepted each other's tokens: a tables cursor replayed against the
knowledge list answered 200 and silently skipped a row. Table rows never
reached that check at all, so a cursor from one table paged another.
Identity now comes from the route's own contract — method plus resolved
path — because a hand-written name is the step an author forgets, and
forgetting it is invisible. An unresolved path placeholder throws rather
than fingerprinting the template, so a misconfigured route fails on every
request instead of an unlucky one. Every token minted before this is
refused with an accurate message; they are single-walk and unpersisted.

Knowledge search and the document list answered different questions.
Search grouped same-tag filters by slot and joined them with OR while the
list conjoined every filter, so `gte 9` and `lte 2` on one tag returned
nothing from the list and a full billed page from search. Search now
conjoins. The OR grouping replaced an explicit `|OR|` mechanism that was
deleted outright, was never documented in any contract, and cost the
ability to express a range on a single tag; the union it gave is still
reachable as separate searches. The search body also accepted an
unbounded query that was billed and then silently truncated to the
embedding model's window, and ignored the tag-filter cap the list
enforces.

A body over ten mebibytes was reported as malformed JSON. Next's proxy
truncates there, well under this app's fifty-megabyte ceiling, so the
parse failed on a body the caller sent whole and the size branch was
unreachable. The ceiling is now clamped to what the proxy will pass.

Also: a group's output columns accepted a `workflowGroupId` and discarded
it; an enrichment group could never gain an output, because a new output
coordinate demanded workflow metadata a group with no workflow cannot
have; `newOutputColumns` alone reported success and created nothing; a
saved view stored layout references to columns that do not exist while
refusing the same name in a filter; an MCP server stored `retries: 0` as
three and overrode an explicit auth type; a disabled server answered tool
discovery with an unclassified fault; rotating a header server's headers
left it reading connected; and a run whose workflow was deleted reported
the root folder path while also reporting the workflow deleted.

Where the honest fix was out of reach, the contract was corrected instead
of half-fixing the code: the polled run resource rebuilds `error.code` by
matching the persisted message, so it can never report the two codes that
need block attribution, and now says so. `OUTPUT_TOO_LARGE` is removed —
no path ever emitted it.

`triggers` was left alone deliberately. It reads as a closed enum but
production holds 43 distinct values, because a webhook run stores its
provider id; pinning the enum would refuse legitimate history a log
search exists to find. The description now says the vocabulary is open.
@vercel

vercel Bot commented Aug 13, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 13, 2026 10:55pm

Request Review

@cursor

cursor Bot commented Aug 13, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Wide v2 cursor behavior change invalidates existing pagination tokens and alters knowledge search filtering and billed scan semantics; remaining changes are mostly validation, documentation, and targeted MCP/table fixes.

Overview
Pagination cursors now fingerprint the list route (contract method + resolved path params), not filters alone. Tokens minted before this change are rejected with a clear 400. Table row and query cursors are scoped per tableId so one table’s cursor cannot page another.

Knowledge search conjoins every tag filter (including duplicate tags on the same slot), matching document list behavior; same-tag OR grouping is removed. The search body enforces query max length (embedding ceiling) and at most 10 tag filters.

JSON bodies are clamped to the 10 MiB proxy ceiling so oversize requests return 413 instead of malformed JSON from truncated streams.

Tables / views: workflow group output column schemas are strict (reject client workflowGroupId); enrichment groups can extend outputs via the registry without workflow resolution; v2 view writes can reject unknown layout column refs like filter/sort.

MCP: retries: 0 is stored correctly; explicit authType is not overridden by oauthClientId; tool discovery on a disabled server returns conflict; header rotation on a headers-auth server resets connection status.

Workflows / logs: OUTPUT_TOO_LARGE is removed from the public error enum; polled run errors document missing block attribution; includeOutput vs selectedOutputs / blockOutputs is clarified; deleted-workflow log reads return null folder path instead of /; log triggers filter is documented as open and case-sensitive.

OpenAPI and contract descriptions are updated across knowledge, logs, resources, tables, and workflows.

Reviewed by Cursor Bugbot for commit 6dafb52. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR addresses defects found while exercising the v2 API against staging.

  • Binds pagination cursors to route contracts and resolved paths.
  • Aligns knowledge search filtering and request bounds with list behavior.
  • Corrects body-size reporting, table group and saved-view validation, MCP lifecycle behavior, and deleted-workflow log projection.
  • Updates contracts, generated OpenAPI documents, and regression coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/api/server/validation.ts Clamps default and route-specific JSON body limits to the proxy ceiling so declared oversized requests return an accurate 413 response.
apps/sim/lib/api/server/validation.test.ts Adds boundary coverage for default, explicit, below-ceiling, optional-body, and malformed-JSON parsing behavior.
apps/sim/lib/api/cursor-binding.ts Strengthens cursor scope identity to include each route contract and resolved path.
apps/sim/lib/knowledge/search/queries.ts Makes repeated knowledge tag filters conjunctive to match document-list semantics.
apps/sim/lib/mcp/application/use-cases.ts Corrects MCP registration and update defaults, authentication selection, retries, and credential lifecycle behavior.
apps/sim/lib/table/application/groups.ts Corrects output-column validation and enrichment-group output update behavior.
apps/sim/lib/table/views/service.ts Validates saved-view layout references against existing table columns.

Reviews (2): Last reviewed commit: "fix(v2): close the two holes the first r..." | Re-trigger Greptile

Comment thread apps/sim/lib/api/server/validation.ts
Comment thread apps/sim/lib/table/views/service.ts Outdated
Comment thread apps/sim/lib/table/application/groups.ts
The previous commit clamped the default JSON body cap but left explicit
per-route overrides alone, so a route declaring a larger `maxBodyBytes`
still fell into the truncation it was meant to report: the four inline
workspace-file routes at 70 MB and the deployed-chat route at 220 MB.

Next attaches `proxyClientMaxBodySize` to every request and clones the
body unconditionally for any non-GET method on a matched path, pushing
EOF at ten mebibytes with only a warning, so the handler reads a
truncated prefix. Those routes therefore already fail above that size —
as a malformed-JSON 400. Clamping the effective limit inside the two
body readers makes the same request fail as payload-too-large, quoting
the limit actually in force.

One existing test asserted the unreachable case, allowing a sixty-mebibyte
base64 body; it now asserts what the proxy will forward intact.

The inline-file path still advertises fifty mebibytes and cannot exceed
the proxy ceiling until that ceiling is raised, which changes buffering
for every route and belongs in its own change.
Both are places where a fix in this branch shut one door and left a
smaller one open in the same wall.

Letting an enrichment group gain an output meant skipping workflow
resolution — but that resolution was the only thing validating a new
output, so a PATCH began storing coordinates the runner can never fill.
It fills a cell from `result[out.outputId]` and skips an output with no
`outputId` at all, while the writer diffs on that same id and the sidebar
reads and writes by it; the contract leaves it optional. The regression
test added with that fix was itself asserting such a dead coordinate.
Create's registry checks are now two shared helpers both paths call, and
on update an output is exempt only when an identical binding already
existed, so renaming a group whose enrichment has since changed still
works while anything added or repointed must name a real output.
`mappingUpdates` on an enrichment group now says it is inexpressible
rather than resolving an empty workflow id into a missing workflow.

The layout-reference check was handed the tolerant column set, so a
placeholder minted to keep a dangling filter writable also whitelisted a
brand-new layout reference — storing an entry the next read discards,
which is the inconsistency the check was added to remove. Layout now
resolves against the live columns, which is exactly what pruning keeps,
while filters and sorts keep the exemption they need.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 6dafb52. Configure here.

@waleedlatif1
waleedlatif1 merged commit ab8a64f into staging Aug 13, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/v2-live-findings branch August 13, 2026 23:12
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