[calvin-archastro]

Review on ArchCode

Task: tsk_034jsXbpOVaZqQO9kGlka2 (blocks tsk_00P3MwVmlsfLC0KnJpssNu, thread-computer SDK release)

Problem

• scripts/regenerate_sdk.sh in archastro-js emits a TS SDK that fails npm run build with TS2393 in src/channels/api_artifact_channel.ts.
• Cause: the spec's Artifact channel (x-channels) has a client-to-server message event named leave.
  1. TS emitter always writes lifecycle async leave(): Promise<void>.
  2. It then writes the leave event as async leave(payload) (tsChannelMessageMethodName = camelCase(event)).
  3. Two members, same name: tsc fails; CI builds before tests, so no SDK can release.
• Python has the same collision but worse: the second async def leave silently replaces the lifecycle leave(), so channel.leave() would push a leave event instead of leaving.
• Not caused by thread computers; any spec with this event triggers it.

What changed

• Message-method naming (the single source for SDK, samples and contract tests) now suffixes Message / _message when the name equals a member the class defines itself.
  • TS reserved: leave, channel, joinResponse, constructor. Result: leaveMessage(payload).
  • Python reserved: leave, join, topic, join_response. Result: leave_message(payload).
• Contract-test emitters (contract-tests/channel-emitter.ts, channel-emitter-python.ts) now call the same helpers instead of duplicating the naming.
• Wire event is unchanged: this.channel.push("leave", payload).
• Unchanged: lifecycle leave() stays the public leave API; every other message/push name is identical.

Scope

Generator only (packages/sdk-generator). No generated output hand-edited, no spec change.

Risk

Low. Only event names that collide with a class member change; in the current spec that is just Artifact leave. Compatibility: no SDK ever shipped a working leave(payload) (TS did not compile; Python's was shadowed), so nothing can depend on the old name.

Testing

• Unit: new tests in __tests__/backends/typescript.test.ts ("does not emit a message method that collides with leave()") and python.test.ts ("does not let a leave event replace the lifecycle leave()"). vitest run: 12 files / 428 tests pass. npm run lint, npm run build: clean.
• Canonical end-to-end proof (manual, local; scratch checkout of archastro-js at 1163b25 v0.20.0, this repo's specs/platform-openapi.json):
  1. Published @archastro/sdk-generator@latest via scripts/regenerate_sdk.sh --local <this repo>, then tsc --noEmit in packages/sdk: TS2393 at api_artifact_channel.ts lines 31 and 51 (reproduced).
  2. Same spec with this branch's built dist/index.js (--lang typescript and --lang contract-tests-ts): tsc --noEmit exits clean; emitted async leave() plus async leaveMessage(payload).
• Automatic coverage: the unit tests run in ci.yml on PR via npm test --workspaces. The generate-then-tsc check has no automatic path in this repo (it needs the archastro-js checkout); archastro-js CI builds the regenerated SDK after release. Not proven here: Python end-to-end generation (unit test only); Swift/Go/Rust.

Follow-ups

• Release a new @archastro/sdk-generator version (release.yml), then regenerate archastro-js / archastro-python with it. Not done here.
• Swift emitter also emits leave() and uses uniqueSwiftMemberNames for message names; likely the same collision. Go/Rust not checked.

🤖 Generated with Claude Code