Skip to content

[Bug] arun() drops its own *args with a TypeError, and awaits a synchronous error handler (#1853 item 3) - #1871

Merged
kyegomez merged 1 commit into
kyegomez:masterfrom
ayaangazali:fix/arun-arg-forwarding
Aug 11, 2026
Merged

[Bug] arun() drops its own *args with a TypeError, and awaits a synchronous error handler (#1853 item 3)#1871
kyegomez merged 1 commit into
kyegomez:masterfrom
ayaangazali:fix/arun-arg-forwarding

Conversation

@ayaangazali

Copy link
Copy Markdown
Contributor

Addresses item 3 in #1853, plus a second defect in the same three lines.

1. arun(task, img, extra) raises instead of running

arun forwarded to run like this:

return await asyncio.to_thread(
    self.run, task=task, img=img, *args, **kwargs,
)

asyncio.to_thread(func, *a, **kw) calls func(*a, **kw), so the splatted *args arrive positionally while task is also passed by keyword — and run's first positional parameter is task. Any extra positional argument therefore collides:

TypeError: run() got multiple values for argument 'task'

arun declares *args specifically to accept those arguments, so the one documented way to use them was the one way that failed.

Fixed by forwarding positionally, in run's own parameter order, which makes arun(t, i, x) behave exactly like run(t, i, x):

return await asyncio.to_thread(self.run, task, img, *args, **kwargs)

2. await on a synchronous method

except Exception as error:
    await self._handle_run_error(error)  # Ensure this is also async if needed

_handle_run_error is a plain def whose last statement is raise error. It is not a coroutine function, and the seven other call sitesagent.py:1679, :1688, :2060, :3114, llm_manager.py:889, :899, autonomous_loop.py:954 — all call it without await.

This only ever worked by accident: the method raises before returning, so the await never evaluates its operand. The moment _handle_run_error stops raising unconditionally, this line becomes await NoneTypeError, on the error path, where it is hardest to notice. Dropped the await and the stale "Ensure this is also async if needed" comment.

Tests

Appended to tests/structs/test_agent.py as TestArunForwarding — no new file. Neither test constructs a model or makes an LLM call; run is replaced on a bare instance, so they exercise arun's plumbing only.

  • test_extra_positional_args_reach_runarun("T", "I", "EXTRA") reaches run as ("T", "I", "EXTRA")
  • test_error_path_does_not_await_a_sync_handler — asserts _handle_run_error is not a coroutine function, that arun's source no longer awaits it, and that the original ValueError surfaces rather than a TypeError
master source + these tests:   2 failed
this branch:                   2 passed
black --check --line-length 70, ruff:  clean

The rest of test_agent.py is red on master before and after this change (missing provider credentials); this diff does not move those numbers.

Red checks are the repo-wide pre-existing ones, addressed in #1812.

@ayaangazali
ayaangazali requested a review from kyegomez as a code owner August 10, 2026 20:26
Copilot AI lite review requested due to automatic review settings August 10, 2026 20:26

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown

Hello there, thank you for opening an PR ! 🙏🏻 The team was notified and they will get back to you asap.

@kyegomez
kyegomez merged commit 433954c into kyegomez:master Aug 11, 2026
6 of 12 checks passed
ayaangazali added a commit to ayaangazali/swarms that referenced this pull request Aug 23, 2026
test_extra_positional_args_reach_run came in with kyegomez#1871 and asserts that
arun(task, img, "EXTRA") forwards "EXTRA" positionally to run(). That is
the behaviour this PR removes, so the test locked the defect in.

It only ever passed because its stub is declared
`fake_run(*args, **kwargs)`, which accepts anything positionally and so
cannot observe where the argument actually lands. Against the real
signature it lands on `imgs`:

    >>> inspect.signature(Agent.run).bind_partial(None, "T", "I", "EXTRA")
    {'task': 'T', 'img': 'I', 'imgs': 'EXTRA'}

`imgs` is a List[str] of image paths, not run()'s own *args.

Replaced with two tests that use a stub with the real parameter names,
so a mis-bind is visible:

  - task/img/kwargs reach run() as themselves
  - a third positional raises TypeError at the arun boundary instead of
    being silently bound to imgs

The second fails on master's agent.py and passes here, which is the
whole point of the change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ayaangazali added a commit to ayaangazali/swarms that referenced this pull request Aug 23, 2026
test_extra_positional_args_reach_run came in with kyegomez#1871 and asserts that
arun(task, img, "EXTRA") forwards "EXTRA" positionally to run(). That is
the behaviour this PR removes, so the test locked the defect in.

It only ever passed because its stub is declared
`fake_run(*args, **kwargs)`, which accepts anything positionally and so
cannot observe where the argument actually lands. Against the real
signature it lands on `imgs`:

    >>> inspect.signature(Agent.run).bind_partial(None, "T", "I", "EXTRA")
    {'task': 'T', 'img': 'I', 'imgs': 'EXTRA'}

`imgs` is a List[str] of image paths, not run()'s own *args.

Replaced with two tests that use a stub with the real parameter names,
so a mis-bind is visible:

  - task/img/kwargs reach run() as themselves
  - a third positional raises TypeError at the arun boundary instead of
    being silently bound to imgs

The second fails on master's agent.py and passes here, which is the
whole point of the change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ayaangazali added a commit to ayaangazali/swarms that referenced this pull request Aug 27, 2026
test_extra_positional_args_reach_run came in with kyegomez#1871 and asserts that
arun(task, img, "EXTRA") forwards "EXTRA" positionally to run(). That is
the behaviour this PR removes, so the test locked the defect in.

It only ever passed because its stub is declared
`fake_run(*args, **kwargs)`, which accepts anything positionally and so
cannot observe where the argument actually lands. Against the real
signature it lands on `imgs`:

    >>> inspect.signature(Agent.run).bind_partial(None, "T", "I", "EXTRA")
    {'task': 'T', 'img': 'I', 'imgs': 'EXTRA'}

`imgs` is a List[str] of image paths, not run()'s own *args.

Replaced with two tests that use a stub with the real parameter names,
so a mis-bind is visible:

  - task/img/kwargs reach run() as themselves
  - a third positional raises TypeError at the arun boundary instead of
    being silently bound to imgs

The second fails on master's agent.py and passes here, which is the
whole point of the change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants