Skip to content

refactor(codemode): dispatch all tool calls positionally#1547

Merged
mattzcarey merged 2 commits into
mainfrom
refactor/codemode-positional-dispatch
May 18, 2026
Merged

refactor(codemode): dispatch all tool calls positionally#1547
mattzcarey merged 2 commits into
mainfrom
refactor/codemode-positional-dispatch

Conversation

@mattzcarey
Copy link
Copy Markdown
Contributor

@mattzcarey mattzcarey commented May 18, 2026

Summary

Simplify codemode tool dispatch by treating every sandbox tool call as positional.

Previously providers could opt into positionalArgs, which made the sandbox proxy capture (...args) and made the dispatcher spread those args into the host function. Providers without the flag used a separate single-object path.

This PR removes that split. The sandbox proxy now always captures an argument list and the dispatcher always calls:

fn(...args)

Object-style calls still work naturally because they are just one positional argument:

await codemode.foo({ a, b }) // fn({ a, b })
await shell.writeFile("path", "contents") // fn("path", "contents")

Changes

  • Remove positionalArgs from ResolvedProvider and ToolProvider
  • Remove ToolDispatcher.positionalArgs
  • Use one dispatch path in both Dynamic Worker and iframe runtimes
  • Update shell/browser/codemode call sites that used positionalArgs: true
  • Rename the TanStack codemode test fixture from .backup.ts to the real test file and merge the existing small TanStack test into it

Verification

npm test --workspace @cloudflare/codemode -- --run
npm run build
npm run check

Open in Devin Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 18, 2026

🦋 Changeset detected

Latest commit: 141ea4e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cloudflare/codemode Patch

Not sure what this means? Click here to learn what changesets are.

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

@mattzcarey mattzcarey force-pushed the refactor/codemode-positional-dispatch branch 2 times, most recently from a97a7c2 to 118c63f Compare May 18, 2026 09:36
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment thread packages/codemode/src/messages.ts Outdated
typeof provider.name === "string" &&
(provider.positionalArgs === undefined ||
typeof provider.positionalArgs === "boolean")
isRecord(provider) && typeof provider.name === "string" && true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Residual && true in type guard from incomplete cleanup

When removing the positionalArgs validation from isExecuteRequestMessageShape, the old check (provider.positionalArgs === undefined || typeof provider.positionalArgs === "boolean") was replaced with just true instead of being removed entirely. This leaves a dead && true in the .every() predicate at packages/codemode/src/messages.ts:101, which is a no-op but clearly an artifact of an incomplete transformation.

Suggested change
isRecord(provider) && typeof provider.name === "string" && true
isRecord(provider) && typeof provider.name === "string"
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 18, 2026

Open in StackBlitz

agents

npm i https://pkg.pr.new/agents@1547

@cloudflare/ai-chat

npm i https://pkg.pr.new/@cloudflare/ai-chat@1547

@cloudflare/codemode

npm i https://pkg.pr.new/@cloudflare/codemode@1547

hono-agents

npm i https://pkg.pr.new/hono-agents@1547

@cloudflare/shell

npm i https://pkg.pr.new/@cloudflare/shell@1547

@cloudflare/think

npm i https://pkg.pr.new/@cloudflare/think@1547

@cloudflare/voice

npm i https://pkg.pr.new/@cloudflare/voice@1547

@cloudflare/worker-bundler

npm i https://pkg.pr.new/@cloudflare/worker-bundler@1547

commit: 141ea4e

Comment on lines -228 to -229
const args = argsJson ? parseForCodemode(argsJson) : {};
const result = await fn(args);
Copy link
Copy Markdown
Contributor

@cjol cjol May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very slightly behaviour-changing in edge cases, I think. If args was undefined, we previously sent an empty object, but will now send undefined. Does that matter?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm happy with this change. feel more correct if everything is positional.

Copy link
Copy Markdown
Contributor Author

@mattzcarey mattzcarey May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the ai sdk libs would have normalised this anyway. It would only be weird for people doing custom implementation in which case this was broken before so I think I'm actually fine with this being a patch.

@mattzcarey mattzcarey force-pushed the refactor/codemode-positional-dispatch branch 2 times, most recently from bc0b81e to 7598dc9 Compare May 18, 2026 15:11
@mattzcarey mattzcarey force-pushed the refactor/codemode-positional-dispatch branch from 7598dc9 to 265b41c Compare May 18, 2026 15:24
@mattzcarey mattzcarey merged commit f739ec9 into main May 18, 2026
4 checks passed
@mattzcarey mattzcarey deleted the refactor/codemode-positional-dispatch branch May 18, 2026 15:45
@github-actions github-actions Bot mentioned this pull request May 18, 2026
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.

3 participants