Uh oh!
There was an error while loading. Please reload this page.
chore(api): remove a dead route builder, bound folder-index reads, reject invalid cursors - #6568
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Dead route builder. Deletes unused Bounded folder-index reads. Invalid cursor rejection. Adds shared Reviewed by Cursor Bugbot for commit 23b7792. Configure here. |
Greptile SummaryThe PR removes an unused public-route wrapper and dead endpoint labels, centralizes strict offset-cursor validation, and narrows folder-index bounds after the earlier review.
Confidence Score: 4/5The PR is not yet safe to merge because an accepted over-cap folder state can make workflow updates commit and then return HTTP 413. The capped fallback folder-index read occurs after updateWorkflowRecord, so workspaces that exceed the unenforced folder ceiling can receive a failure response after the requested workflow mutation has already persisted. Files Needing Attention: apps/sim/lib/workflows/application/update-workflow.ts
|
| Filename | Overview |
|---|---|
| apps/sim/lib/workflows/application/update-workflow.ts | Adds bounded folder-index reads, but the fallback read can throw after a workflow update has already persisted. |
| apps/sim/lib/workflows/application/create-workflow.ts | Explicitly bounds the folderId branch's folder-index load. |
| apps/sim/lib/folders/queries.ts | Preserves unbounded behavior when maxRows is omitted and documents the opt-in bound. |
| apps/sim/app/api/v2/lib/response.ts | Adds reusable validation for offset cursors. |
| apps/sim/app/api/v2/tables/[tableId]/rows/route.ts | Uses strict offset-cursor decoding for table-row pagination. |
| apps/sim/app/api/v1/middleware.ts | Removes endpoint labels associated with the deleted unused route builder. |
Comments Outside Diff (1)
apps/sim/lib/workflows/application/update-workflow.ts, line 181-185 (link)Update commits before folder failure
If a workspace contains more than 10,000 active workflow folders, a name-, description-, lock-, or policy-only update persists through
updateWorkflowRecord, then this capped fallback index read throws and returns HTTP 413. The caller therefore receives a failure even though the requested workflow mutation has already committed.
Reviews (2): Last reviewed commit: "fix(api): reject an undecodable offset c..." | Re-trigger Greptile
Uh oh!
There was an error while loading. Please reload this page.
…int labels `withPublicApiRouteHandler` and 27 `ApiEndpoint` union members landed together in #5273, but the v2 surface shipped on `defineV2JsonRoute` + `v2RateLimits` instead. The builder had no production caller — only its own test — and the v2 rate limiter never reads an `ApiEndpoint` label, so those members were never emitted to telemetry by symbol or by string literal. Remaining members are exactly the labels a v1 route passes to `checkRateLimit` or `authenticateRequest`. Drops the now-unreachable `hasZodUsage` branch from the API validation audit; no ratchet metric moves (route total stays 1093).
`createWorkflow` and `updateWorkflow` each resolve a folder two ways inside one function. The folderPath branch goes through `resolveWorkflowFolderPath`, which loads the path index with `maxRows: MAX_FOLDERS_PER_WORKSPACE`; the folderId branch loaded it with no bound at all, issuing a `SELECT` over every active folder row in the workspace. In `updateWorkflow` the unbounded read and the bounded fallback sit thirty lines apart in the same function. Passes the cap at both sites, matching the read sites that already opt in. Exceeding it throws `FolderCollectionLimitExceededError` rather than truncating, because a partial path index resolves real folder paths to `undefined` and re-roots resources at the workspace root. `maxRows` deliberately stays opt-in rather than becoming the default. Folder creation does not refuse at the same ceiling on every path — `POST /api/folders` goes through the `createFolder` name/parentId variant, which passes no `maxFolderRows`, so the count guard in `executeCreateFolderAtPath` never runs and a workspace can already hold more than `MAX_FOLDERS_PER_WORKSPACE` folders. Defaulting the bound would make every path-index consumer throw for a state the product allows to exist. Reconciling reader and writer is a separate change with a user-facing limit, not a chore.
GET /api/v2/tables/{tableId}/rows coerced an undecodable pagination cursor to
offset 0 and re-served page one. A client paging forward reads that as a fresh
first page and can loop over it forever. Every sibling v2 cursor list — logs,
files, workflows, workflow runs, workflow versions, workspace members, tables,
knowledge documents — already rejects with a validation error instead.
Extracts the offset-cursor decode both offset-paginated v2 routes had inlined
into `decodeOffsetCursor`, next to the existing `decodeSortedCursor`, so the
reject-don't-restart rule has one home.9667823 to
79a3486Comparefe432f9 to
23b7792Comparewaleedlatif1
commented
Aug 11, 2026
waleedlatif1
commented
Aug 11, 2026
@cursor review |
There was a problem hiding this comment.
✅ 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 23b7792. Configure here.
3cab8ef
into
improvement/v2-route-standardizationUh oh!
There was an error while loading. Please reload this page.
Stacked on #6567 → #6565 → #6560. Review only this PR's own commits; merge after its parents.
Three independent tidies from the v2 parity audit, one commit each. All are incomplete application of new work rather than regressions from
main.1. Dead route builder and 27 dead endpoint labels
withPublicApiRouteHandler(~80 lines) had zero production callers — referenced only by its own test and two regexes in the audit script that existed to recognize it. It shipped in #5273 alongside 27 extraApiEndpointmembers, but the v2 surface actually runs ondefineV2JsonRoute+v2RateLimits.publicApi.Verified by string literal, not just by symbol: each of the 46 members was grepped as a bare string across
apps/,packages/, andscripts/, then narrowed to the only two positions that reach telemetry (checkRateLimit,authenticateRequest). 19 are live and kept. The decisive fact is thatv2RateLimitsnever reads anApiEndpoint, so no removed label can be emitted by any v2 route.Ratchet metrics are byte-identical before and after (1093 routes, 1093 Zod-backed, 0 non-Zod) — the builder lived outside a
route.tspath, so it was never counted as a route.2. Unbounded folder-index reads
loadActiveFolderPathIndexwas called withoutmaxRowsat 15 sites, while adjacent code two lines away passed the cap. Rather than paste the constant 15 more times, the cap is now the function's default. Safe because it throws (FolderCollectionLimitExceededError) rather than truncating, the query uses.limit(maxRows + 1)so a workspace exactly at the cap still passes, and folder creation already refuses at the same ceiling. Every domain constant is the identical value —MAX_KNOWLEDGE_FOLDERS_PER_WORKSPACEis literally an alias.3. Invalid cursors silently restarting pagination
GET /api/v2/tables/{tableId}/rowscoerced an undecodable cursor to offset 0 and re-served page 1 — which can make a paging client loop forever. Eight sibling v2 cursor lists already reject; this route was the sole outlier. ExtracteddecodeOffsetCursorbeside the existingdecodeSortedCursor(the only two routes using the offset shape). Kept the literal'Invalid cursor'rather thanINVALID_CURSOR_MESSAGE, whose text is sort-mismatch-specific and would be wrong here.Reverting each fix turns its tests red. type-check · biome · 633 tests ·
check:api-validation·check:openapi— all pass.Follow-up left undone:
maxRows: MAX_FOLDERS_PER_WORKSPACEis now redundant at ~25 call sites, but removing it means edits far outside this diff and would break tests asserting it explicitly.