fix: let bare Tools compete directly in delegate selection (not just as a last resort) - #195
fix: let bare Tools compete directly in delegate selection (not just as a last resort)#195imaustink wants to merge 2 commits into
Conversation
"Is airvinyl running okay" matched claude-code-swe-agent -- the only
Agent in the catalog, description broad enough ("runs bash...") to
loosely overlap via embedding similarity -- instead of the ssh tool,
and hung on that agent's Claude identity-link gate (a known class of
pre-existing bug in this repo: fix/claude-auth-submit-hang et al.).
Nothing about that path is specific to ssh; any bare Tool with no
wrapping Skill is exposed to the same mis-routing risk once a broad
Agent exists in the catalog.
Adds ssh-skill (mirroring cluster-debug-skill's shape: toolRefs: [ssh],
no allowedRoles of its own per ADR 0011), so selectDelegate's combined
skill+agent choice sees a strong, specific match for SSH-shaped
requests and picks it over the vague agent match. Its markdown also
gives authored guidance for turning an open-ended "is it healthy?"
into a concrete sequence of read-only diagnostic calls (uptime, df -h,
free -m, then a named service if implied) -- separately addressing
why a vague request was declining at the ActionPlanner stage even when
the ssh tool WAS the right fallback candidate.
Verified with a real kind cluster + kubectl apply --dry-run=server.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh
…tion Fixes the actual root cause behind ssh-skill being needed at all: a bare Tool with no wrapping Skill was only ever considered as a last resort (noMatchFallback's selectFallbackTool), invoked only once skillCandidates AND agentCandidates both came up empty. Any Agent whose description loosely overlapped a request via embedding similarity alone -- not because it was actually the better fit, but because a Tool never got the chance to compete at all -- would win by existing as a candidate. That's what let claude-code-swe-agent (the only Agent in the catalog) absorb "SSH into X" requests instead of the ssh Tool, hanging on its identity-link gate. Adds a `retrieveTools` graph node (embedding query + ToolFitChecker, reusing selectFallbackTool's own two-stage relevance gate) alongside retrieveSkills/retrieveAgents, and extends DelegateSelector to a three-way choice among skills/agents/tools in one combined decision (docs/adr/0036). Guarded on deps.delegateSelector being configured, so non-NATS deployments pay no extra cost and keep the exact old skill-only + selectFallbackTool behavior. ssh-skill (this branch's other commit) stays -- it still adds value a bare Tool alone can't: authored guidance for turning an open-ended "is it healthy?" into a concrete sequence of diagnostic calls. This fixes the starvation itself so no *future* unwrapped Tool needs a Skill just to be reachable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh
imaustink
changed the title
|
🤖 Starting work on this now. |
|
🤖 Watch live or take over the session here: https://claude.ai/code/session_01Bn6S7LNrrgd7nfGXtVrddZ |
|
Something went wrong processing this: /invoke/96006fd2-e3e7-4e21-a8af-75f0de005d07 poll failed: 404 |
![k5s-bot[bot]](https://avatars.githubusercontent.com/in/1052865?s=60&v=4){kind=small}
There was a problem hiding this comment.
Solid, well-scoped fix — retrieveTools correctly reuses the existing embedding query + ToolFitChecker gate, the three-way selector change is clean, and the tests cover the important routing cases. Two issues before merge: one user-facing correctness bug on the new tool branch, and one drift risk in the un-templated skill markdown. (No CI checks are configured on this branch, so nothing was running to gate this review.)
| return { | ||
| selectedTool: planned.tool, | ||
| toolArgs: planned.toolArgs, | ||
| ...(planned.toolInstanceKey ? { toolInstanceKey: planned.toolInstanceKey } : {}), | ||
| wasFallback: true, | ||
| }; |
There was a problem hiding this comment.
Setting wasFallback: true here is wrong for a deliberately selected tool. wasFallback is the signal that a catalog search found no proper match and the request was handled ad-hoc — it appends the SELF_IMPROVEMENT_FOOTER (chat-completions.ts:69: "No existing skill or agent matched this request, so it was handled ad-hoc...") at graph.ts:1981/2085. But this branch is reached precisely because delegateSelector.select picked this tool as the best fit in the three-way comparison — a first-class match, exactly like the agent branch (line 1616) and skill branch (line 1630), neither of which sets wasFallback. As written, every SSH-shaped request that this PR successfully routes to the ssh tool ends its reply telling the user nothing matched and offering to author a skill — contradicting the feature's own premise. The !planned fall-through at 1620-1621 already routes genuine no-matches through noMatchFallback, which sets the flag correctly.
| return { | |
| selectedTool: planned.tool, | |
| toolArgs: planned.toolArgs, | |
| ...(planned.toolInstanceKey ? { toolInstanceKey: planned.toolInstanceKey } : {}), | |
| wasFallback: true, | |
| }; | |
| return { | |
| selectedTool: planned.tool, | |
| toolArgs: planned.toolArgs, | |
| ...(planned.toolInstanceKey ? { toolInstanceKey: planned.toolInstanceKey } : {}), | |
| }; |
|
|
||
| ## Write capability | ||
|
|
||
| This deployment's `ssh` tool is NOT restricted to read-only commands -- |
There was a problem hiding this comment.
This markdown hardcodes deployment-specific facts that the ssh Tool template derives from values, so the two drift apart on any deployment that isn't the author's. The Tool's description is templated on $wideOpen (eq .Values.sshTool.allowedCommands "*") and it enforces a read-only command allowlist when not wide-open — but this section unconditionally tells the model it "can also restart services, edit or delete files." On a read-only sshTool deployment (the chart default guidance) the model will attempt writes the Tool then rejects. Same class of drift for the fixed host list at line 45 vs. the operator-configured sshTool.sshConfig/allowedHosts. Both should be rendered from .Values.sshTool rather than baked in.
🤖 Prompt to fix this with an AI agent
In imaustink/agent-controller on PR #195, edit charts/community-components/templates/skill-ssh.yaml so the skill markdown is derived from the same values as templates/tool-ssh.yaml instead of hardcoded. Specifically: (1) gate the "## Write capability" section (around line 66) on {{ eq .Values.sshTool.allowedCommands "*" }} so it only claims write/restart/delete ability when the ssh Tool is actually wide-open, and describes read-only-only behavior otherwise; (2) render the "Known targets" host list (around line 45) from .Values.sshTool.sshConfig / .Values.sshTool.allowedHosts rather than the literal home/bastion/console/printcam/airvinyl/airbuddy list. Verify with `helm template` against values.yaml, values-production.yaml, and values-ci-all.yaml that the rendered skill markdown matches the corresponding ssh Tool's configured commands and hosts in each. Do not change unrelated behavior.
1 participant
Summary
Follow-up to #192. After that PR deployed, "Can you SSH into airvinyl and see if it's running all good?" in Open WebUI got a generic LLM fallback with no tool use, and a follow-up with a specific command ("SSH into airvinyl and run uptime") got stuck showing "found 1 agent candidate" indefinitely.
Root cause, confirmed against the live cluster:
claude-code-swe-agentis the onlyAgentin the whole catalog, and its broad description ("Performs software-engineering work on GitHub end-to-end. Runs the Claude Code CLI headless — which has bash, file-read/write, grep, and glob tools...") loosely matched via embedding similarity.selectDelegate(apps/agent-orchestrator/src/agent/graph.ts) only ever considered a bareTool(likessh, with no wrappingSkill) as a last resort — reachable only onceskillCandidatesANDagentCandidatesboth came up empty. Since an Agent candidate always existed, thesshTool never got a chance to compete at all, and dispatching to the agent hung on its Claude identity-link gate (a known, pre-existing bug class in this repo —fix/claude-auth-submit-hanget al. — unrelated to #192's code).Fix (two parts)
1.
ssh-skill(charts/community-components/templates/skill-ssh.yaml) — mirrorscluster-debug-skill's shape (toolRefs: [ssh]). GivesselectDelegatea strong, specific match for SSH-shaped requests so it wins over the vague agent overlap, and its authoredmarkdowngives explicit guidance for turning an open-ended "is it healthy?" into a concrete sequence of read-only diagnostic calls.2. The actual root cause (
docs/adr/0036-tools-compete-directly-in-delegate-selection.md) — a bare Tool was starved out categorically, not because it lost a comparison, but because it was never in the comparison. Any future unwrapped Tool would hit the identical problem the moment a broad Agent (or Skill) exists in the catalog. Fixed by:retrieveToolsnode (apps/agent-orchestrator/src/agent/graph.ts), inserted betweenretrieveAgentsandselectDelegate: runs the same embedding query +ToolFitCheckertwo-stage relevance gateselectFallbackToolalready used, reused rather than reinvented. Guarded ondeps.delegateSelectorbeing configured, so non-NATS deployments pay no extra cost and keep the exact old behavior.DelegateSelector.select(apps/agent-orchestrator/src/agent/delegate-selector.ts) now takes a thirdtoolsparameter and makes one combined three-way choice (skill/agent/tool), with an explicit preference order in its prompt: skill (authored guidance) > bare tool (single well-defined action) > agent (open-ended/multi-step work).selectDelegate's tool branch reuses a new shared helper (planFallbackToolCall, extracted fromselectFallbackTool's own tail) to construct the actual tool call, falling back to the oldnoMatchFallback/selectFallbackToolsafety net if the planner declines or the combined choice comes back empty.ssh-skillis kept alongside the core fix rather than superseded by it — it still adds value a bare Tool alone can't (the authored multi-step diagnostic guidance).Test plan
npm run typecheck --workspace=agent-orchestratornpm run build --workspace=agent-orchestratornpm run test --workspace=agent-orchestrator— 579/579 passing, including 4 new tests covering: tool candidates reach the delegate selector already fit-checked, a tool wins over an agent candidate when the selector says so (agent never launched), a tool that failsToolFitCheckeris excluded from what's offered to the selector, and thenoMatchFallbacksafety net still works when the combined selector picks nothing.helm lint charts/community-componentshelm templateagainstvalues.yamldefaults,values-production.yaml, andvalues-ci-all.yaml(verified againstvalidate-crds.yml's literal "every template rendered" assertion).kindcluster +kubectl apply --dry-run=serveragainst the full rendered catalog —skill.core.controller-agent.dev/ssh-skill created (server dry run).🤖 Generated with Claude Code
https://claude.ai/code/session_01Fg8b9pPWm91nLbDnB6ECJh