fix(api): restore migrated endpoint and SDK compatibility - #6564
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview Knowledge restores owner-only access to legacy personal knowledge bases (and their child resources) without weakening workspace-scoped auth, including usage admission, search billing, and shared audit projection. Tables again support unbounded filter updates/deletes via bounded keyset batches with concurrent-change revalidation, return missing rows as 404, preserve saved group output coordinates, and expose plan-derived Files switch nested write/move paths to canonical folder helpers with fail-fast segment limits and map folder/move conflicts to 409. Also restores legacy status codes for table name conflicts (400) and storage quota (402), strips encrypted service-account material from credential create responses, treats MCP paused HITL runs as successful tool results, allows nullable pause Reviewed by Cursor Bugbot for commit e0a5c39. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThe PR restores API and SDK compatibility across error projection, file operations, table mutations, workflow execution, Knowledge authorization, and response serialization.
Confidence Score: 5/5The PR appears safe to merge because the previously reported issues are fixed and no blocking failure remains. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/table/rows/service.ts | Restores no-limit filter mutations using bounded keyset pages, per-page transactions, locked revalidation, and bounded side-effect dispatch. |
| packages/ts-sdk/src/index.ts | Restores synchronous execution failure throwing and compatibility fields for execution identifiers and timing metadata. |
| packages/ts-sdk/src/index.test.ts | Adds compiler-checked coverage for synchronous workflow failures and restored execution metadata. |
| apps/sim/app/api/credentials/route.ts | Validates credential-creation responses through the public contract so encrypted service-account material is omitted. |
| apps/sim/lib/workspace-files/application/workspace-file-folders.ts | Coordinates canonical nested-folder creation and rollback behavior for workspace-file operations. |
| apps/sim/lib/knowledge/application/authorization.ts | Restores legacy personal-knowledge owner access while preserving workspace-scoped authorization behavior. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Caller[API or SDK caller] --> Contract[Shared request/response contract]
Contract --> UseCase[Authorized application use case]
UseCase --> Domain[Files, Tables, Knowledge, or Workflows]
Domain --> Policy[Surface-specific error policy]
Policy --> Response[Legacy or v2 response envelope]
Reviews (8): Last reviewed commit: "fix(auth): project legacy knowledge audi..." | Re-trigger Greptile
|
@cursor review |
|
@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 9d7c60f. Configure here.
…r-projection # Conflicts: # apps/sim/app/api/mcp/servers/route.test.ts # apps/sim/app/api/mcp/servers/route.ts # apps/sim/app/api/table/utils.test.ts # apps/sim/app/api/v1/tables/route.test.ts # apps/sim/app/api/v2/files/folders/route.test.ts # apps/sim/app/api/v2/lib/response.ts # apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts # apps/sim/lib/api/contracts/workflows.ts # apps/sim/lib/uploads/archive.test.ts # apps/sim/lib/uploads/archive.ts # apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.test.ts # apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts # apps/sim/lib/workspace-files/application/workspace-file-folders.ts
|
@cursor review |
|
@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 6976393. Configure here.
|
@cursor review |
…r-projection # Conflicts: # apps/sim/app/api/tools/file/manage/route.test.ts
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
2 issues from previous reviews remain unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1858bad. Configure here.
|
@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 e0a5c39. Configure here.
The v2 SDK migration (#5273, #6564) shipped five breaking changes in both SDKs but got the release mechanics wrong in three separate ways, and left one of the two rewrites unable to complete a single successful call. Versions. packages/ts-sdk/package.json read 0.1.3 -- a patch digit added inside an unrelated compatibility commit, never deliberated. npm expands ^0.1.2 to >=0.1.2 <0.2.0, so every existing consumer would have picked the break up on a lockfile refresh: AsyncExecutionResult.jobId renamed to runId, executionId dropped from that interface, a failed sync run now throwing instead of resolving {success:false}, the request body reshaped, and the endpoint moved to /api/v2 with no fallback. 0.2.0 excludes every existing range, so the upgrade becomes opt-in. packages/python-sdk carries the identical break and was never bumped at all, so its publish job would have skipped green at the "version already exists" gate and left the repo and PyPI silently divergent; it moves 0.1.2 -> 0.2.0 in lockstep, along with the __version__ string in simstudio/__init__.py, which tracks pyproject and would otherwise have started lying. setup.py is left at 0.1.1: it is unchanged from main and demonstrably unread (0.1.2 published from pyproject while setup.py already said 0.1.1). It wants deleting, in its own commit. A 404 fallback was considered and rejected. The legacy 202 body's statusUrl points at /api/jobs/{jobId}, so mapping jobId onto runId would hand the caller an id that getWorkflowRun cannot resolve against that same old server -- a successful execute followed by an inexplicable failure on the next call is a worse contract than a clean 404. Both READMEs instead state the minimum server version and name the endpoint to check for. Cancelled runs. packages/python-sdk computed success as status != 'failed', so a run cancelled out of band reported success=True. The TypeScript SDK uses a closed whitelist and reports False, and before the migration both SDKs read the server's own value, which was False -- so this was a Python regression, not merely an inconsistency. Fixed by mirroring the whitelist. The v2 contract enumerates exactly completed|failed|paused|cancelled, so narrowing the blacklist to a whitelist cannot drop a live value, and a status added later now defaults to "not successful" rather than silently reporting True. WorkflowExecutionResult gains a status field because Python, unlike TypeScript, does not throw on 'failed' -- so success=False alone is ambiguous there in a way it is not in the TypeScript SDK, which is why status is not added to both. Rate-limit header. Found while auditing the two SDKs for further divergence, and the reason the Python bump could not have shipped as it stood: every authenticated v2 response now carries X-RateLimit-Reset as an ISO 8601 timestamp (recorded by v2RateLimits.publicApi, stamped by withRouteHandler). The Python SDK parsed it with int(), raising a bare ValueError that no handler in execute_workflow catches -- so every successful v2 execution raised instead of returning. None of the legacy endpoints the SDK previously called record a rate-limit snapshot, which is why the latent int() survived until the v2 move. The TypeScript SDK already branches on the format; _parse_reset_header mirrors it, including degrading an unrecognised value to 0, because a quota hint must not take down the call it rode in on. Timing metadata. The v2 rewrite stopped forwarding startedAt/endedAt, which main passed through and the TypeScript SDK still reports; restored under the same startTime/endTime keys the TypeScript SDK uses. Tests: cancelled/failed/paused status coverage, the ISO reset header, and the restored metadata keys, each verified red against the unfixed line first. The TypeScript suite gains matching cancelled/paused and ISO-reset pins -- they pass against today's source by design, and were confirmed to fail against a deliberately degraded copy so they are not toothless. Deliberately not included: a CI guard failing a PR that changes SDK source without a version bump. It would have caught this twice over, but it is a new script and workflow rather than a fix to the defect at hand. Review revision. bun.lock recorded packages/ts-sdk at 0.1.3 and was left stale by the first pass, so the repo asserted two versions for the same workspace package -- in a change whose whole thesis is that the version strings had diverged. It does not break CI (bun 1.3.14 accepts the mismatch under --frozen-lockfile, confirmed here), but 092311e bumped the lock in lockstep with package.json, and the next unfrozen install would otherwise drop the line into an unrelated PR. _parse_reset_header gated the numeric branch on str.isdigit(), which accepts characters int() rejects ('²'.isdigit() is True, int('²') raises) -- and that int() sits outside the try, so the one function added to stop a quota hint raising could still raise, contradicting its own docstring. str.isdecimal() is exactly the set int() accepts. The tolerates-unparseable test is parametrized over both forms and was confirmed red on '²' against isdigit. Docs and docstrings: apps/docs api-reference/python.mdx mirrors the README's dataclass block and was the only copy left without the new status field. RateLimitInfo now names its units, because reset is epoch seconds for the legacy integer and milliseconds for the ISO form that v2 sends. execute_workflow's Args entry still described the pre-v2 body shape ("spread at root level"); every input is nested under input now, and this is the commit that ships that help() text to PyPI. The "declared last so positional construction keeps working" sentence was a maintainer's note that belongs in this message, not in every user's help(WorkflowExecutionResult).

Summary
executionId/ timing compatibility, and bump the SDK to 0.1.3Error model
Application and domain code now throws classified errors at the source. Shared route builders project those errors into each API surface's declared envelope, including typed
HttpErrorfailures. Unknown errors still fail closed as generic 500 responses without leaking implementation details.Security and authorization notes
Validation
Notes
stagingand includes staging through merge commit1858bad382(origin/stagingatcb2809001e)