Uh oh!
There was an error while loading. Please reload this page.
fix(sdk): make the 0.2.0 SDK release safe to publish - #6616
Conversation
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).
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Versioning: Python correctness: Tests cover cancelled/failed/paused outcomes and ISO vs unparseable reset headers. Reviewed by Cursor Bugbot for commit 679049a. Configure here. |
Greptile SummaryThe PR prepares the TypeScript and Python SDKs for their breaking 0.2.0 release and corrects Python synchronous-status and rate-limit-reset handling.
Confidence Score: 5/5The PR appears safe to merge, with the release metadata, SDK behavior, documentation, and regression tests aligned. The changed status mapping covers the complete synchronous v2 status contract, expected rate-limit reset formats degrade safely, and the authoritative package versions consumed by publishing are consistently set to 0.2.0.
|
| Filename | Overview |
|---|---|
| packages/python-sdk/simstudio/init.py | Correctly adds terminal status reporting, aligns success classification with the v2 contract, and safely parses expected ISO and legacy rate-limit reset formats. |
| packages/python-sdk/tests/test_client.py | Adds focused regression tests for completed, paused, cancelled, and failed executions and for ISO or malformed reset headers. |
| packages/python-sdk/pyproject.toml | Advances the authoritative Python package version to the unpublished breaking-release boundary. |
| packages/ts-sdk/package.json | Advances the TypeScript SDK to 0.2.0 so existing 0.1.x caret ranges do not absorb breaking API changes. |
| packages/ts-sdk/src/index.test.ts | Extends coverage for cancelled and paused status mapping and the v2 ISO reset-header format. |
| packages/python-sdk/README.md | Documents v2 server requirements, migration steps, result semantics, and the intentional Python failure-delivery behavior. |
| packages/ts-sdk/README.md | Documents the breaking v2 migration, server feature-gate requirement, and changed synchronous failure semantics. |
Reviews (1): Last reviewed commit: "fix(sdk): make the 0.2.0 SDK release saf..." | Re-trigger Greptile
Summary
Three release-correctness defects in the two SDK packages, all of which would have shipped badly on the next publish.
1. The TypeScript SDK carried five breaking changes but was versioned
0.1.3.The v2 migration already landed on this branch's base. It changed, in the published API:
executeWorkflowmoved toPOST /api/v2/workflows/{id}/execute, with the workflow input nested underinputandasync/executionTimeoutSecondsmoved from theX-Execution-Mode/X-Execution-Timeout-Secondsheaders into the bodyAsyncExecutionResult.jobIdrenamed torunIdexecutionIdremoved fromAsyncExecutionResultgetJobStatussuperseded bygetWorkflowRun(workflowId, runId){ success: false }At
0.1.3every existing consumer would have taken all five automatically: npm expands^0.1.2to>=0.1.2 <0.2.0-0, so0.1.3is in range and0.2.0is not. Verified with the resolver rather than by eye:Now
0.2.0. Minor rather than major because the package is pre-1.0 and 0.x minors are where breaking changes belong under semver —1.0.0would additionally assert a stability commitment the package has not made.0.1.3was never published (npm has0.1.0,0.1.1,0.1.2), so nothing is being reused or overwritten.2. The Python SDK was rewritten onto
/api/v2with no version bump, so it was silently unpublishable.pyproject.tomlstill said0.1.2and PyPI already has0.1.2. The publish job's existence check therefore matched, the job skipped, and it skipped green — so the repo and PyPI diverged with no failing signal. Every v2 change made since has been sitting unpublished. Now0.2.0.3. A cancelled run reported
success=Truein Python andFalsein TypeScript.Python derived
success = status != 'failed', which countscancelledas a success. TypeScript derivesstatus === 'completed' || status === 'paused'. Python was the wrong side and now matches. The predicate is a whitelist on purpose: a status added to the API later defaults to "not successful" rather than silently reportingTrue.A new
WorkflowExecutionResult.statusfield carries the server's terminal status verbatim, so a caller can tell a cancelled run from a failed one — both reportsuccess=False. The two SDKs now agree on the predicate for all four terminal states and differ only in delivery, which is documented on both sides:completedsuccess: truesuccess=Truepausedsuccess: truesuccess=Truecancelledsuccess: falsesuccess=FalsefailedSimStudioErrorsuccess=False,status='failed',errorsetAlso fixed, found while verifying the above: Python parsed
X-RateLimit-Resetwith a bareint(). The v2 API sends that header as an ISO 8601 timestamp (resetAt.toISOString()), so everyexecute_workflowcall against v2 would have raisedValueErrorout of the rate-limit bookkeeping — the Python0.2.0would have been dead on arrival. It now mirrors the TypeScript parser exactly (all-digit kept as-is, otherwise parsed to epoch ms, otherwise 0) and degrades rather than raising, because a quota hint must never take down the call it rode in on.Both READMEs get a migration guide. A breaking major without one is the actual defect.
Publish implications — read before merging
This is a breaking release of two packages to two registries that cannot be un-published cleanly. Versions can never be reused.
publish-ts-sdk.ymlandpublish-python-sdk.ymltrigger onpushtomainonly, path-filtered to their package. The publish fires when staging is promoted to main, not on this merge.packages/ts-sdk/package.json; the PyPI job reads[project].versionfrompackages/python-sdk/pyproject.toml. Both are0.2.0here, and neither version exists on its registry, so both existence checks miss and both will publish. Each also cuts a GitHub release tag.packages/python-sdk/setup.pystill says0.1.1and is dead. The publish job does not read it, and because[project].versionis static, setuptools ignores theversion=kwarg passed tosetup(). It does not affect this release. Left alone deliberately — see follow-ups.0.2.0:^0.1.2andsimstudio-sdk<0.2both stay on the old line, and upgrading becomes a deliberate act.Server compatibility claim
The branch originally asserted a minimum server version of "Sim v0.7.69 or later" in both READMEs. I removed that number rather than shipping it, because a published README cannot be corrected without cutting another release and the claim did not hold up:
v0.7.69tag exists — only-beta/-stagingprereleases. The latest stable tag isv0.7.68.stagingand not onmain, sov0.7.68does not contain it and a stablev0.7.69cut before that promotion would not either./api/v2surface sits behind thev2-apigate, which answers 404 when off. A self-hosted deployment serves it only when the operator enablesV2_API, regardless of which version it runs.The READMEs now describe the requirement in terms the reader can actually check — the endpoint, the
V2_APIsetting, and the 404 they will see — with no version number to go stale.Type of Change
Testing
packages/python-sdk: 40 passed. Run withPYTHONPATH=. python -m pytest tests/test_client.py -q. Note that PR CI does not cover this suite — the Python SDK is not a JS workspace, soturbo run testskips it and the tests run only inside the publish job on main. Run locally when reviewing.packages/ts-sdk: 38 passed (bunx vitest run src/index.test.ts). This one is covered by PR CI viaturbo run test.successtostatus != 'failed'turnstest_sync_execution_cancelled_is_not_successred, and restoring it turns it green with a clean tree. Same for the ISO reset parser — reverting toint(reset)fails three tests withValueError.bun run type-checkand biome clean on the changed TypeScript.bun.lockchanged by exactly one line: thepackages/ts-sdkworkspace version propagating. No dependency, resolution, or integrity change.Explicitly out of scope
A 404-fallback / dual-endpoint compatibility shim was considered and rejected. The legacy 202 body's
statusUrlpoints at/api/jobs/{jobId}, so mappingjobIdontorunIdwould hand the caller arunIdthat this SDK's owngetWorkflowRuncannot resolve — a silently broken poll is worse than an honest 404.Follow-ups, not this PR
RateLimitInfo.resetcarries epoch seconds for the legacy integer header and epoch milliseconds for the ISO one, in both SDKs. That inconsistency is faithfully mirrored from TypeScript rather than fixed here; normalizing both to milliseconds is a separate breaking change to a separate field.0.1.xAPI (job_id/jobId/get_job_status/getJobStatus) and become wrong the moment0.2.0publishes. This is broader than it first looks: 20 pages across 5 locales —{de,es,fr,ja,zh}×{api-reference,sdks}×{python,typescript}. The Englishapi-referencepages are already current. Separately, there is noen/sdks/directory at all, so the ten*/sdks/*.mdxpages are orphaned translations of a source page that no longer exists and no English-side edit will ever propagate to them.packages/python-sdk/setup.pyshould probably be deleted outright rather than bumped. It is a wholly redundant second metadata declaration — name, description, classifiers,install_requires,python_requiresall duplicated frompyproject.tomland none of it consulted. Bumping it to0.2.0would make it look live and guarantee the same drift recurs at0.3.0. Deleting it removes the trap: ifversionwere ever moved todynamic, setup.py's stale value would silently become authoritative.WorkflowExecutionResulthas nostatusfield, so "a resolvedsuccess: falsemeans it was cancelled" is an inference the caller cannot verify from the type. It holds today only becausefailedthrows. Python fixed exactly this; addingstatusto TypeScript would be another breaking-ish change and belongs on its own.Checklist