Skip to content

fix(sdk): sanitize malformed-success decode errors - #1700

Open
samuv wants to merge 1 commit into
plan/sdk-malformed-success-causesfrom
impl/sdk-malformed-success-causes
Open

samuv wants to merge 1 commit into
plan/sdk-malformed-success-causesfrom
impl/sdk-malformed-success-causes

Conversation

@samuv

@samuv samuv commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Remove runtime JSON/protobuf decoder exceptions from malformed successful unary HTTP and ordinary SSE ProtocolError values.
  • Retain safe HTTP status and request-ID metadata while preserving existing body-read, server, authentication, fetch/network, abort, cancellation, and SSE-reader causes.
  • Add red-green regression coverage for JSON, well-known-type normalization, and actual protobuf-es failures on both unary and streaming paths.

Development stage

  • Plan / Interface — Bounded/Architectural behavioral and exact-interface contract; no implementation
  • Implementation — based on the exact Plan / Interface commit below under the directing human's explicit pre-merge stacking waiver
  • Combined — compact one-task Bounded/Architectural exception; no separate plan PR
  • Spike / Routine — acceptance-plan spine exempt

Contract linkage

  • Work classification: Architectural
  • Classification rationale: changes the durable public diagnostic and security policy at the TypeScript SDK HTTP successful-response boundary
  • Decision record: docs/adr/0348-typescript-sdk-malformed-success-decoding.md
  • Human waiver of spine: Yes — the directing user explicitly authorized this Implementation PR as the next gh stack layer without waiting for the Plan / Interface PR to merge; neither PR is authorized to merge here
  • Acceptance plan: docs/acceptance/sdk-malformed-success-decoding.md
  • Human decisions resolved and recorded: Yes
  • Plan / Interface PR: plan(sdk): harden malformed-success decode errors #1698
  • Approved commit baseline: b6e05820685a85bf3cf027ed42da4e269060e6ed under the recorded stacking waiver; it is not yet merged authority
  • Combined/exemption rationale: N/A — delivery remains Split

Interface conformance

Matches the exact committed plan contract. There are no protobuf, Go API, tool, CLI/config, event, persistence, or public TypeScript signature changes.

Issue relationship

Closes #1694

Type of change

  • Behavioral/interface plan
  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation/process
  • Other (describe):

Test plan

Baseline checks

  • Acceptance-plan checker
  • Linting (task lint)
  • Offline test suite (task test) — host Xcode/macOS 27 linker rejects its own arm64e.x1-* SDK architectures across CGO-linked test binaries; one unrelated TestCarryoverHandoff also timed out during the degraded run. CI must supply this proof.
  • Offline demo (CGO_ENABLED=0 go run ./cmd/mecademo)
  • Markdown changed: docs generation/link checks (CGO_ENABLED=0 task docs)
  • User docs/user-facing behavior changed: site build (task site:build)
  • Guarded engine API affected: N/A — no engine API change
  • Intentional engine API change: N/A
  • Landed plan: strict acceptance trace — plan remains an in-progress stacked candidate until the full CI gate passes
  • Final implementation review: /panel-review

Additional SDK gates:

  • task sdk:lint sdk:typecheck sdk:test sdk:build sdk:api:check sdk:docs:check
  • 56 Vitest files and 306 tests pass with the complete decoder-stage cases; the focused suite passes all 3 tests.
  • Panel result after the reviewer-requested true protobuf-es fixtures: Spec 0, Standards 0, Test adequacy 0, Domain 0.

Changes

File Change
sdk/typescript/src/http.ts Single cause-free malformed-success constructor and split body acquisition/JSON parsing
sdk/typescript/test/http-decoding-errors.test.ts Unary/SSE decode and neighboring-error regression matrix
sdk/typescript/src/errors.ts, generated API reference Public diagnostic contract
TypeScript SDK guide and living architecture/design notes Safe logging and boundary documentation
Acceptance plan/index Stacked candidate status and exact waived baseline

User-facing change

Applications receive the same ProtocolError class, code, message, transport, and status for malformed successful HTTP/SSE payloads, plus a response request ID when present. These errors no longer expose the decoder exception through cause.

Special notes for reviewers

Unary body acquisition is intentionally separate from JSON.parse(): a response.text() failure retains the existing caused ProtocolError. SSE reader.read() failures remain outside decoder catches. The final CI test result is the only pending acceptance-plan gate; this PR does not claim the plan is landed.

Omit decoder causes at the successful HTTP and SSE payload boundary while retaining safe metadata and preserving body-read, server, authentication, transport, and cancellation causes.

Relates to #1694

Co-authored-by: Codex <codex@openai.com>
Signed-off-by: Samuele Verzi <samu@stacklok.com>
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.

SDK: sanitize malformed-success HTTP decoding causes

1 participant