diff --git a/apps/agent-orchestrator/src/agent/graph.test.ts b/apps/agent-orchestrator/src/agent/graph.test.ts index eb9e2b5..4f825da 100644 --- a/apps/agent-orchestrator/src/agent/graph.test.ts +++ b/apps/agent-orchestrator/src/agent/graph.test.ts @@ -783,6 +783,73 @@ describe("buildAgentGraph session-scoped active skill (ADR 0012)", () => { expect(deps.skillFitChecker.fits).not.toHaveBeenCalled(); expect(deps.skillStore.query).toHaveBeenCalled(); }); + + it("falls back to full retrieval when the turn names a capability outside the active skill's own toolIds, even though the fit-check says it still fits", async () => { + // Regression: the fit-checker only judges topic continuity, so "use your + // kubectl access to debug this" reads as a continuation of the same + // debugging conversation and passes the fit-check -- but the active + // skill's toolIds could never call kubectl, so it must not be reused. + const kubectlTool: ToolDescriptor = { + id: "kubectl-readonly", + name: "kubectl-readonly", + description: "Runs read-only kubectl commands against the cluster", + allowedRoles: ["reader"], + jobTemplate: { image: "example.com/kubectl-readonly:latest", namespace: "default", serviceAccountName: "sa" }, + }; + const deps = baseDeps({ + vectorStore: { + upsert: vi.fn(), + delete: vi.fn(), + query: vi.fn().mockResolvedValue([{ tool: kubectlTool, score: 0.8 }]), + getByIds: vi.fn().mockResolvedValue([ + { tool: scraperTool, score: 1 }, + { tool: publisherTool, score: 1 }, + ]), + }, + }); + const graph = buildAgentGraph(deps); + + const final = await graph.invoke({ + request: "use your kubectl access to debug it", + authToken: "tok", + activeSkillId: skill.id, + sessionSubject: "alice", + }); + + expect(final.error).toBeUndefined(); + expect(deps.skillFitChecker.fits).toHaveBeenCalledWith("use your kubectl access to debug it", skill); + expect(deps.toolFitChecker.fits).toHaveBeenCalledWith("use your kubectl access to debug it", kubectlTool); + // Fell through to full retrieval + selection instead of reusing the skill as-is. + expect(deps.skillStore.query).toHaveBeenCalled(); + expect(deps.skillSelector.select).toHaveBeenCalled(); + }); + + it("keeps the active skill when the only vector-store candidates are already within its toolIds", async () => { + const deps = baseDeps({ + vectorStore: { + upsert: vi.fn(), + delete: vi.fn(), + query: vi.fn().mockResolvedValue([{ tool: scraperTool, score: 0.9 }]), + getByIds: vi.fn().mockResolvedValue([ + { tool: scraperTool, score: 1 }, + { tool: publisherTool, score: 1 }, + ]), + }, + }); + const graph = buildAgentGraph(deps); + + const final = await graph.invoke({ + request: "yes, publish it", + authToken: "tok", + activeSkillId: skill.id, + sessionSubject: "alice", + }); + + expect(final.error).toBeUndefined(); + expect(final.selectedSkill?.id).toBe(skill.id); + expect(deps.toolFitChecker.fits).not.toHaveBeenCalled(); + expect(deps.skillStore.query).not.toHaveBeenCalled(); + }); }); describe("buildAgentGraph tool continuation tokens (ADR 0017)", () => { diff --git a/apps/agent-orchestrator/src/agent/graph.ts b/apps/agent-orchestrator/src/agent/graph.ts index f92a0b8..3f1a625 100644 --- a/apps/agent-orchestrator/src/agent/graph.ts +++ b/apps/agent-orchestrator/src/agent/graph.ts @@ -1195,6 +1195,29 @@ async function selectFallbackTool( return { tool, toolArgs: planned.toolArgs, ...(planned.toolInstanceKey ? { toolInstanceKey: planned.toolInstanceKey } : {}) }; } +/** + * Guards `checkActiveSkill`'s fit-check (docs/adr/0012) against a failure mode + * the fit-checker's own prompt can't see: it only judges topic continuity + * ("is this still the same task?"), so a turn that explicitly asks for a + * DIFFERENT capability mid-task (e.g. "use your kubectl access to debug + * this" while still inside skill-web-search) reads as "still fits" even + * though the active skill's own `toolIds` could never satisfy it — the turn + * then gets a flat "I can't do that" from the model instead of ever reaching + * a tool that could. Reuses the same narrow `toolFitChecker` the fallback + * path (`selectFallbackTool`) already trusts for exactly this judgment, + * scoped to candidates NOT already in the skill's own `toolIds`. A hit means + * this turn needs full retrieval, not the tools already loaded for the + * active skill. + */ +async function hasOutOfScopeToolMatch(state: AgentState, skill: SkillDescriptor, deps: AgentGraphDeps): Promise { + if (!state.identity) return false; + const candidates = await deps.vectorStore.query(state.request, { callerRoles: state.identity.roles }, deps.fallbackToolTopK ?? 3); + const outOfScope = candidates.filter((c) => !skill.toolIds.includes(c.tool.id)); + if (outOfScope.length === 0) return false; + const fitFlags = await Promise.all(outOfScope.map((c) => deps.toolFitChecker.fits(state.request, c.tool))); + return fitFlags.some(Boolean); +} + /** * Calls `bestEffortResponder` for the raw request, streaming deltas through * `progressListener` (as "agent-text" content, the same convention @@ -1307,6 +1330,11 @@ export function buildAgentGraph(deps: AgentGraphDeps) { if (!skill) return {}; const fits = await deps.skillFitChecker.fits(state.request, skill); if (!fits) return {}; + // A "yes, still the same task" verdict can still be wrong if this turn + // names a capability outside the skill's own toolIds (see + // hasOutOfScopeToolMatch) -- don't reuse the active skill in that case, + // fall through to full retrieval instead. + if (await hasOutOfScopeToolMatch(state, skill, deps)) return {}; return { selectedSkill: skill }; }) .addNode("checkPendingIdentityLink", async (state) => {