Skip to content

[Bug] CronJob rejects every plain callable it documents supporting — the Callable check is inverted - #1904

Merged
kyegomez merged 1 commit into
kyegomez:masterfrom
ayaangazali:fix/cronjob-plain-callable
Aug 20, 2026
Merged

[Bug] CronJob rejects every plain callable it documents supporting — the Callable check is inverted#1904
kyegomez merged 1 commit into
kyegomez:masterfrom
ayaangazali:fix/cronjob-plain-callable

Conversation

@ayaangazali

@ayaangazali ayaangazali commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

CronJob is documented as "A wrapper class that turns any callable (including Swarms agents) into a scheduled cron job" and typed agent: Optional[Union[Any, Callable]]. Only the Agent half works. The discriminator is inverted, so both non-Agent shapes fail — each one down the branch meant for the other.

Reproduce

def my_callable(task, **kw):
    return f"handled: {task}"

CronJob(agent=my_callable, interval="1second").run("do it")
                        BEFORE                                                      AFTER
plain function       -> CronJobExecutionError: 'function' object has no attribute 'run'    handled: do it
object with run()    -> CronJobExecutionError: 'HasRun' object is not callable             obj.run: do it
Agent                -> agent.run: do it                                                  agent.run: do it

Cause

if isinstance(self.agent, Callable):
    original_output = self.agent.run(task=task, **kwargs)
else:
    original_output = self.agent(task, **kwargs)

isinstance(x, Callable) tests for __call__, which is the wrong question here — it is true for plain functions and false for a plain object that only defines run(). So:

  • a function is Callable → takes the .run() branch → AttributeError
  • an object with run() but no __call__ is not Callable → takes the direct-call branch → TypeError

An Agent satisfies both, which is why the only shape that works is the one that hides the bug. The else branch is unreachable for every function, i.e. the documented "any callable" path never executed.

Fix

Ask the question the branch actually cares about — does this thing have a run() to call:

runner = getattr(self.agent, "run", None)
if callable(runner):
    original_output = runner(task=task, **kwargs)
else:
    original_output = self.agent(task, **kwargs)

Agent behaviour is unchanged (it has run, so it takes the same branch as before, with the same task=/**kwargs call shape). Callbacks and kwargs forwarding verified unaffected: handled: cb task [cb] | meta=1.

Same root cause as #1900 (create_agent_map used the same test to tell Agents from functions). Separate PR because it is a different module and a different symptom — that one silently returns an empty map, this one raises.

No test file: four-line change inside one branch.

Adjacent finding

Filed as #1905: batched_run(tasks) can only ever schedule the first task. run() ends in while True: time.sleep(1), so batched_run's loop never reaches iteration two and never returns. Verified — three tasks in, ['task-A'] scheduled, thread still alive.

@ayaangazali
ayaangazali requested a review from kyegomez as a code owner August 17, 2026 18:57
Copilot AI lite review requested due to automatic review settings August 17, 2026 18:57

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.

@ayaangazali

Copy link
Copy Markdown
Contributor Author

Still needed after #1920 — flagging it because the two land in the same method's neighbourhood and it would be easy to read one as covering the other.

#1920 fixed batched_run scheduling only the first task. This PR fixes a different line: _run_job discriminating on isinstance(self.agent, Callable), which is true for a plain function too, so every function is sent down the .run() path and the self.agent(task, ...) branch below it is unreachable.

Reproduced just now against master at 36d40c3 (with #1920 in):

plain callable -> CronJobExecutionError Task execution failed: 'function' object has no attribute 'run'

The docstring on that same method still advertises plain callables, and the class docstring shows one as an example, so the documented usage is the broken one.

@kyegomez
kyegomez merged commit d40b1d3 into kyegomez:master Aug 20, 2026
6 of 12 checks passed
@kyegomez

Copy link
Copy Markdown
Owner

Accepted and merged as d40b1d38. Thanks for this one.

Verified against master before merging rather than reading the diff. The two branches were exactly inverted, and only one agent shape happened to work:

Agent shape before after
object with .run(), no __call__ 'HasRunOnly' object is not callable ran:t
plain function (the documented case) 'function' object has no attribute 'run' fn:t
real Agent shape (.run() and __call__) ran:t ran:t

isinstance(x, Callable) is true for anything defining __call__, so plain functions took the .run() path and raised, while ordinary agent-like objects fell through to the else and were called as functions. Discriminating on hasattr(agent, "run") is the right test, and Callable is still needed in the file for the type hints so nothing was left unused.

I tested this on the merge result rather than the branch alone, since the branch predated #1920. Merged onto master it applies cleanly, both fixes coexist, and #1920's hourly-interval and batched_run behaviour still hold with its tests passing.

Worth noting this had already surfaced in practice: while reviewing #1920 I wrote a probe agent with .run() and no __call__ and it died with 'A' object is not callable. It is also why the MockAgent in tests/structs/test_cron_job.py had to define __call__ at all. That workaround is no longer necessary now that this has landed.

One thing for next time: seven lines of production change with no test. The three shapes in the table above are cheap to encode, and tests/structs/test_cron_job.py already exists. Not a blocker for a fix this small, but a test_run_job_dispatches_on_run_attribute would have pinned exactly the failure mode this fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants