fix(typescript): cancel and bound background streams

[mikemikimike]

Fixes #662
Closes #662
Resolves #662

This change cancels background stream consumption when the returned output closes and applies bounds to stream chunks, stream bytes, and accumulated response text. It also adds regression coverage for cancellation, completion, limits, and provider errors.

Issue: #662

Validation:

• TypeScript build
• TypeScript tests
• TypeScript type check
• ESLint

[brnaba-aws]

@claude review the change

[claude]

Claude finished @brnaba-aws's task in 3m 15s —— View job

---

Claude finished reviewing this PR

Todo list:

• Gather context (PR body, changed files)
• Check for repo CLAUDE.md (none found)
• Review typescript/src/orchestrator.ts changes
• Review typescript/src/utils/helpers.ts changes
• Review new test file typescript/tests/streamingCancellation.test.ts
• Review typescript/tests/utils/helpers.test.ts changes
• Post review feedback

---

Summary

Overall this is a well-structured fix for #662. The cancellation wiring in processStreamInBackground (listening for close on the output, racing iterator.next() against a cancellation promise, calling iterator.return(), and awaiting that cleanup in finally) is a sound pattern, and the new streamingCancellation.test.ts covers cancellation, completion, byte/chunk limits, and provider errors well. I traced the write-backpressure logic (accumulatorTransform.write(...) callback + accepted flag) carefully — it relies on Node's writable-stream internals deferring the write callback via process.nextTick when the write completes synchronously, so the accepted variable is correctly populated before the callback reads it. Not a bug.

Bug: reasoning/"thinking" stream chunks are now silently dropped

typescript/src/utils/helpers.ts AccumulatorTransform._transform (lines 22-47):

const text = this.extractFromChunk(chunk);
if (typeof text === "string" && text.length > 0) {
  ...
  this.push(text);
}
callback();

Previously this was if (text) { ... push(text) } (truthy check). extractFromChunk (lines 49-59) has:

} else if (chunk.thinking) {
  return chunk; // returns the whole object, not a string
}

bedrockLLMAgent.ts (around line 441-446) yields exactly this shape when Bedrock reasoning/thinking is enabled during streaming:

const thinking = { thinking: true, content: chunk.contentBlockDelta.delta.reasoningContent.text };
yield thinking;

Under the old truthy check, this object chunk was still forwarded to the consumer via this.push(text) (even though accumulation via += on an object was already dubious). With the new typeof text === "string" guard, the whole if block is skipped for these object chunks — they are now neither pushed to the output stream nor accumulated. Streaming consumers of a reasoning-enabled Bedrock agent will silently lose all "thinking" deltas with this change, and no existing test (including the ones added/modified in this PR) covers extractFromChunk's chunk.thinking branch, so this regression isn't caught by CI.

This looks like an unintended side effect of tightening the guard to satisfy Buffer.byteLength(text, "utf8") (which requires a string) — the ui widget-chunk case was given an explicit pass-through at the top of _transform (this.push(chunk); return;) but the thinking case wasn't given the same treatment.

Suggested fix: give chunk.thinking the same explicit pass-through treatment as the ui widget case (forward chunk via this.push(chunk) without subjecting it to the byte-accounted text path), and add a regression test asserting a { thinking: true, content: ... } chunk is still forwarded.

Fix this →

Minor observations (not blocking)

• orchestrator.ts getChunkByteLength uses JSON.stringify(chunk) to size non-string chunks for the MAX_STREAM_BYTES check — this is an approximation of the wire size and can throw/behave oddly for chunks with toJSON() quirks or undefined values, but it's already guarded with a try/catch fallback, so this is fine as-is.
• The finally block in processStreamInBackground only awaits iteratorClosePromise if closeIterator() was actually invoked; on the normal-completion path the iterator naturally reports done, so no explicit .return() call is needed there — this is correct, just noting it since it's easy to misread as a missed cleanup path.

Validation note

I wasn't able to run the TypeScript build/tests/lint myself — network access (npm install / git fetch) is blocked in this sandboxed environment. The review above is based on a full manual read of the single-commit diff (git show HEAD) plus tracing the runtime code paths, including cross-referencing bedrockLLMAgent.ts to confirm the thinking chunk shape is real and reachable.
· Branch: fix/issue-662-stream-cancellation