You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#242 explicitly asked to "update cargo xtask sdk-schema-vendor to invoke cratestack generate-typescript and vendor the output into the Node SDK."#245 did not do that, and did not record it as a scope cut. sdks/node/vsms-sdk-node/src/types.ts is entirely hand-written instead.
Severity: P2 — the types match the schema today (checked by inspection). The problem is that nothing will notice when they stop.
Why this is structurally different from the Rust SDK
sdks/rust/vsms-sdk-rust macro-expands the real schema through include_client_schema!, so a schema change that invalidates the SDK is a compile error. sdks/node/vsms-sdk-node/src/types.ts is a second, hand-maintained shadow of schema/schema.cstack with no compile-time or CI tie to it at all — and per #249, no CI job even typechecks the package today.
So the Node SDK can silently ship types describing a version of the API that no longer exists. An integrator would find out at runtime, from a response that does not match its declared shape.
.xtask/src/sdk_schema.rs is untouched by #245 and still knows only sdks/rust/vsms-sdk-rust/schema.cstack.
Options
Do what Implement official Node.js SDK (@vsms/sdk) to improve DX #242 asked — generate the TypeScript types from the schema and vendor them, with an xtask check that fails on drift, matching how sdk-schema-check already guards the Rust SDK's vendored copy.
Keep them hand-written but tie them down — a check that the hand-written types still match a generated reference. Cheaper, weaker, and still better than nothing.
Decide hand-written is correct and say so in the module doc, with the reasoning. A deliberate, documented choice is fine; an undocumented one that contradicts the issue that asked for it is the thing worth avoiding.
Note packages/sms-client — the existing generated TypeScript client — is separately unusable for reasons documented in docs/design/frontend-package-audit.md and filed upstream as cratestack#610 (its runtime discards the Response, so it cannot do ETag/If-Match). That does not block option 1, which needs only the generated types, not the generated client runtime — but it is context worth having before reaching for the generated client wholesale.
Also, minor and unrelated
docs/roadmap.md's "Already built, ahead of its milestone → Integration surface" bullet names sdks/rust/vsms-sdk-rust but not the new Node SDK. That section exists so infrastructure landing ahead of its milestone is not rebuilt later — a one-line addition is warranted.
Provenance
Review of #245 (merged as 17ec2bb), conventions-and-packaging pass.
Summary
#242 explicitly asked to "update
cargo xtask sdk-schema-vendorto invokecratestack generate-typescriptand vendor the output into the Node SDK." #245 did not do that, and did not record it as a scope cut.sdks/node/vsms-sdk-node/src/types.tsis entirely hand-written instead.Severity: P2 — the types match the schema today (checked by inspection). The problem is that nothing will notice when they stop.
Why this is structurally different from the Rust SDK
sdks/rust/vsms-sdk-rustmacro-expands the real schema throughinclude_client_schema!, so a schema change that invalidates the SDK is a compile error.sdks/node/vsms-sdk-node/src/types.tsis a second, hand-maintained shadow ofschema/schema.cstackwith no compile-time or CI tie to it at all — and per #249, no CI job even typechecks the package today.So the Node SDK can silently ship types describing a version of the API that no longer exists. An integrator would find out at runtime, from a response that does not match its declared shape.
.xtask/src/sdk_schema.rsis untouched by #245 and still knows onlysdks/rust/vsms-sdk-rust/schema.cstack.Options
xtaskcheck that fails on drift, matching howsdk-schema-checkalready guards the Rust SDK's vendored copy.Note
packages/sms-client— the existing generated TypeScript client — is separately unusable for reasons documented indocs/design/frontend-package-audit.mdand filed upstream as cratestack#610 (its runtime discards theResponse, so it cannot do ETag/If-Match). That does not block option 1, which needs only the generated types, not the generated client runtime — but it is context worth having before reaching for the generated client wholesale.Also, minor and unrelated
docs/roadmap.md's "Already built, ahead of its milestone → Integration surface" bullet namessdks/rust/vsms-sdk-rustbut not the new Node SDK. That section exists so infrastructure landing ahead of its milestone is not rebuilt later — a one-line addition is warranted.Provenance
Review of #245 (merged as
17ec2bb), conventions-and-packaging pass.