Skip to content

fix(agents): gate SWARM/hybrid transfer worker registration behind requiredWorkers; fix sub-agent tool domain routing - #459

Open
kowser-orkes wants to merge 3 commits into
mainfrom
fix/swarm-transfer-worker-registration-gate
Open

fix(agents): gate SWARM/hybrid transfer worker registration behind requiredWorkers; fix sub-agent tool domain routing#459
kowser-orkes wants to merge 3 commits into
mainfrom
fix/swarm-transfer-worker-registration-gate

Conversation

@kowser-orkes

Copy link
Copy Markdown
Contributor

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • WHOSUSING.md
  • Other (please describe):

Changes in this PR

Bug: a stateful Strategy.SWARM agent never reaches a terminal state against conductor-oss
past rc.16 (#1363). Two independent causes:

  • _register_swarm_transfer_workers / _register_hybrid_transfer_workers always registered a
    TaskDef for every {source}_transfer_to_{peer} name, ignoring the requiredWorkers gate every
    other system-worker registration respects. These names are compiler-owned control signals, never
    real dispatched work — the PUT /api/metadata/taskdefs 404s.
  • ToolRegistry.register_tool_workers only applied the resolved worker domain when the
    current (sub-)agent itself was stateful=True. The server domain-routes every tool in the
    whole tree once any agent is stateful, so a SWARM member's real @tool task lands in the
    domain-scoped queue while its own worker polls the domain-less queue — task sits SCHEDULED
    forever. This is the actual hang.

Fix:

  • Gate SWARM/hybrid transfer-worker registration behind requiredWorkers (per-name, via a
    server_needs predicate); server_needs=None keeps the old register-everything fallback for
    servers that don't return requiredWorkers.
  • Drop the per-agent agent_stateful re-check in register_tool_workers; always pass the
    already tree-wide-resolved domain through.
  • New tests: per-name gating, full-skip, server_needs=None fallback parity, domain-routing pin.

Validated: full unit suite green (2613 passed, 1 skipped, 0 failed). Live-validated the
registration gate without an LLM (real /api/agent/deploy call, zero registration attempts for
_transfer_to_ names). Live LLM re-run of the repro against the domain fix is the one open item.

Paired with: conductor-oss PR mapping toolType="handoff"INLINE. Fixing either alone
doesn't fully close #1363.

Issue #1363

Alternatives considered

  • Gate at the call site instead of per-name inside the loop — rejected: a swarm can have the
    server list some peer-pair transfer names as needed and not others; a single call-site check
    can't express that.
  • Rely on task_runner.py's PUTPOST fallback to paper over the bad registration — rejected:
    that's the existing non-stateful behavior and doesn't explain the stateful-specific hang; better
    to stop the wasted call entirely.

Kowser and others added 3 commits July 30, 2026 09:20
…rkers

_register_swarm_transfer_workers and _register_hybrid_transfer_workers
unconditionally registered (and PUT-taskdef'd) a worker for every
{source}_transfer_to_{peer} name, bypassing the _server_needs gate every
other system-worker registration in _register_workers respects. These
names are compiler-owned control signals (toolType="handoff", INLINE
after the paired conductor-oss fix) that are never dispatched as real
tasks -- the unconditional PUT is exactly what 404s against a server
that (correctly) never created a TaskDef for them (conductor-oss #1363).

- both functions take an optional server_needs predicate and skip
  worker_task(...) per-name when the server doesn't list it; None
  preserves the old register-everything fallback for older servers
- drop the "always register anyway" bypass comments -- the nested/
  sub-workflow gap they cited doesn't apply to this name category,
  since _collect_worker_names already recurses independently of the
  server for genuinely-declared @tool workers
- new unit tests: per-name gating (registers only what's needed),
  full-skip when server needs nothing, and server_needs=None fallback
  parity with prior behavior
ToolRegistry.register_tool_workers() only honored *domain* when the
*current* agent (or the individual tool) was itself marked
stateful=True. For a SWARM/hybrid/sequential parent whose sub-agents own
their own @tool workers, each sub-agent's own .stateful is False even
though the whole compiled graph is domain-routed once any agent in the
tree is stateful -- confirmed server-side, AgentConfig.java's
collectDeclaredWorkerTaskNames() puts every "worker"-type tool name into
taskToDomain unconditionally whenever runId is set, with no per-tool
gate. The client's extra local re-check left sub-agent tool workers
polling the domain-less queue while their tasks sat in the domain-scoped
queue forever -- a live stateful SWARM run hung on the very first tool
call past the conductor-oss #1363 fix.

domain is already None unless AgentRuntime._resolve_worker_domain /
_has_stateful_tools determined (once, tree-wide) that this execution is
stateful, so re-deriving it locally was redundant and wrong for nested
agents. Drop the agent_stateful param entirely and always pass domain
through.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ARM transfer-worker registration fix

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/conductor/ai/agents/runtime/runtime.py 51.59% <100.00%> (+1.11%) ⬆️
src/conductor/ai/agents/runtime/tool_registry.py 86.11% <ø> (+66.66%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kowser-orkes
kowser-orkes requested a review from v1r3n July 30, 2026 16:56
ling-senpeng13 added a commit to conductor-oss/conductor that referenced this pull request Jul 30, 2026
The entries had grown into full root-cause write-ups — 2.2KB for one
reason string. That analysis belongs in the tracking issue and the fixing
PR, where it can be discussed and closed out; duplicated here it just goes
stale silently, which is exactly what happened to the suite14 entry.

Each value is now a short statement of what fails plus a tracking link.
_README says to keep it that way. File is 2227 bytes, down from 4781.

Kept the operational parts, which are not history and prevent real
mistakes: the E2E_MIN_PASSED coupling, and the warning that a run:false
xfail can never XPASS so it must be un-listed by hand.

Also repoints suite14 at conductor-oss/python-sdk#459, which supersedes
the now-closed #455.

Verified: plugin still reports both entries matching exactly one test,
no MATCHED NOTHING.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant