Skip to content

fix: Make action attempt error and result nullable - #1007

Open
razor-x wants to merge 3 commits into
mainfrom
claude/js-sdk-audit-6x115l-h6-action-attempt-types
Open

fix: Make action attempt error and result nullable#1007
razor-x wants to merge 3 commits into
mainfrom
claude/js-sdk-audit-6x115l-h6-action-attempt-types

Conversation

@razor-x

@razor-x razor-x commented Aug 24, 2026

Copy link
Copy Markdown
Member

Problem

SDK audit finding H6 (high): the generated ActionAttempt type collapses the API's per-status variants (21 action types × 3 statuses) and claims non-null error and result on every variant:

const aa = await seam.locks.unlockDoor({device_id}, {waitForActionAttempt: false})
aa.result.was_confirmed_by_device  // typechecks — TypeError at runtime: result is null on pending

SucceededActionAttempt<T> = T & {status:'success'} was an intersection that recovered nothing, and SeamActionAttemptFailedError read actionAttempt.error.message unguarded.

Fix

Root cause is upstream: @seamapi/blueprint's createActionAttempts merges the per-status OpenAPI variants and deliberately keeps the non-nullable error/result definitions — the JS templates already render isNullable correctly. So the fix is a codegen normalization (mirroring seamapi/php#479, which faced the same blueprint limitation and also kept action_type-discriminated variants): mark error and result nullable on every action-attempt variant and append the same doc sentence PHP uses — "Null while the action attempt is pending or when this value does not apply."

Regeneration touches exactly one generated file, src/lib/resources/action-attempt.ts (21 variants × error+result).

Hand-written follow-through in resolve-action-attempt.ts:

  • SucceededActionAttempt<T> asserts result: NonNullable<T['result']>; error: null, and FailedActionAttempt<T> the mirror — so (await seam.locks.unlockDoor(...)).result.was_confirmed_by_device keeps compiling and is now actually safe.
  • SeamActionAttemptFailedError falls back to 'Action attempt failed' / code 'unknown' for a failed attempt without an error object instead of crashing on a null dereference.

Breaking-change note

Compile-time-only tightening that fixes a type lie — runtime behavior of well-formed responses is unchanged (the API always sent null). Code reading aa.error.x/aa.result.x on an unnarrowed attempt stops compiling, which is the point; narrow via resolveActionAttempt/waitForActionAttempt or a status check. PHP shipped its analog as a feat: in a minor.

Tests

  • Type-level assertions: unnarrowed error/result reads are @ts-expect-error; SucceededActionAttempt/FailedActionAttempt narrowing compiles. On reverted source these produce 4 type errors — proving the old types lied.
  • Runtime: a failed attempt with error: null rejects with SeamActionAttemptFailedError('Action attempt failed', code 'unknown'); on reverted source this is a bare TypeError: Cannot read properties of null (reading 'message').

Full suite (127 tests), lint, typecheck green. npm run generate diff verified to touch only the expected sites.

Part of applying the rev-3 SDK audit (one PR per finding). Related: #1002#1006.

🤖 Generated with Claude Code

https://claude.ai/code/session_01B8xeJm2Hd923k8uo6eoFd2


Generated by Claude Code

The generated ActionAttempt type collapsed the per-status API variants
and claimed non-null error and result on every variant, so reading
result on a pending attempt or error on a successful one typechecked
and crashed at runtime. The API sends null for result unless the
status is success and null for error unless the status is error.

The blueprint merges the per-status variants and keeps the non-nullable
property definitions, so normalize the action attempt properties during
generation: error and result are nullable with the null semantics
documented. SucceededActionAttempt and FailedActionAttempt now assert
the resolved shape, so narrowing through resolveActionAttempt or a
status check keeps result and error reads compiling. Guard
SeamActionAttemptFailedError against a failed attempt without an error
object instead of crashing with a null dereference.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8xeJm2Hd923k8uo6eoFd2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants