dont override with undefined

[deathbyknowledge]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 2139cd7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[claude]

Claude Code Review

Summary: The fix correctly addresses the issue of undefined values overwriting existing tool outputs.

Issue Identified:
The bug occurs when updateDynamicToolPart and updateToolPart unconditionally assign optional fields from the discriminated union options, even when those fields aren't present in the current state variant. This overwrites values previously set by _applyToolResult with undefined.

Solution Assessment:
The fix uses "field" in anyOptions checks to conditionally update fields only when explicitly provided. This preserves existing values while allowing legitimate updates.

Concerns:

1. Inconsistent handling in update path (line 1343):

   anyPart.input = anyOptions.input;  // Always assigned

   Unlike the optional fields, input is assigned unconditionally even in the update path. This could theoretically cause the same bug if a state transition provides undefined for input. However, reviewing the discriminated union types (lines 1379-1411), input is always required in all state variants, so this appears safe—but the inconsistency is confusing.

2. Missing guards in creation path:
   When creating new parts (lines 1361-1375, 1446-1462), fields like rawInput are only added conditionally. This is correct, but there's a subtle issue: if a field exists in anyOptions with value undefined, it will still be added to newPart. The check "field" in anyOptions is true even if the value is undefined. Consider:

   if ("rawInput" in anyOptions) newPart.rawInput = anyOptions.rawInput;
   // If anyOptions.rawInput === undefined, this adds { rawInput: undefined }

   This might be intentional, but it's worth verifying the type system prevents this.

3. No test coverage:
   This bug fix lacks automated tests. A regression test should verify that tool outputs set by _applyToolResult aren't overwritten by subsequent state transitions.

Recommendation:
The fix is fundamentally correct and should be merged. Consider adding a test case for the specific bug scenario to prevent regression.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/agents@750

commit: 2139cd7

[uuouter]

@deathbyknowledge seeing as this didn't affect my issue, not sure if needed. I did check it out. I noticed there were some new exposed undefined attributes, such as title errorText. Don't know if this is new with V6 just a FYI.