From 5118a566dfbea233d3ca515810d61eb13cacd9c5 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 5 Jun 2026 12:30:48 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=AD=20fix:=20Restore=20Empty=20Skill?= =?UTF-8?q?=20Allowlist=20Catalog=20(#13526)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/server/controllers/agents/v1.js | 5 ++-- api/server/controllers/agents/v1.spec.js | 27 +++++++++++++++++++ .../Input/__tests__/SkillsCommand.spec.tsx | 2 +- .../api/src/agents/__tests__/skills.test.ts | 4 +-- packages/api/src/agents/skills.ts | 8 +++--- 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index adb78d1b56..b7f7a9bd29 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -77,10 +77,9 @@ const sanitizeViewerSkillScope = (agent, accessibleSkillSet) => { const configuredSkills = Array.isArray(agent.skills) ? agent.skills : []; if (configuredSkills.length === 0) { + // Empty allowlist means the viewer's full accessible catalog. delete agent.skills; - if (accessibleSkillSet.size > 0) { - agent.skills_enabled = true; - } + agent.skills_enabled = true; return agent; } diff --git a/api/server/controllers/agents/v1.spec.js b/api/server/controllers/agents/v1.spec.js index f36152abec..fda2bdd616 100644 --- a/api/server/controllers/agents/v1.spec.js +++ b/api/server/controllers/agents/v1.spec.js @@ -1548,6 +1548,33 @@ describe('Agent Controllers - Mass Assignment Protection', () => { expect(response.data[0].skills_enabled).toBeUndefined(); }); + test('should preserve enabled skill scope for VIEW list callers with an empty allowlist', async () => { + await Agent.findByIdAndUpdate(agentA1._id, { + skills_enabled: true, + skills: [], + }); + + mockReq.user.id = userB.toString(); + mockReq.query.requiredPermission = String(PermissionBits.VIEW); + findAccessibleResources.mockImplementation(({ resourceType }) => { + if (resourceType === ResourceType.AGENT) { + return Promise.resolve([agentA1._id]); + } + if (resourceType === ResourceType.SKILL) { + return Promise.resolve([]); + } + return Promise.resolve([]); + }); + findPubliclyAccessibleResources.mockResolvedValue([]); + + await getListAgentsHandler(mockReq, mockRes); + + const response = mockRes.json.mock.calls[0][0]; + expect(response.data).toHaveLength(1); + expect(response.data[0].skills).toBeUndefined(); + expect(response.data[0].skills_enabled).toBe(true); + }); + test('should return raw skill configuration for EDIT list callers', async () => { const visibleSkillId = new mongoose.Types.ObjectId(); const hiddenSkillId = new mongoose.Types.ObjectId(); diff --git a/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx b/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx index 53ec6388f7..ecafb84586 100644 --- a/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx +++ b/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx @@ -320,7 +320,7 @@ describe('SkillsCommand', () => { isFetchingNextPage: false, }); mockUseAgentsMapContext.mockReturnValue({ - agent_1: { id: 'agent_1', skills_enabled: true }, + agent_1: { id: 'agent_1', skills: [], skills_enabled: true }, }); const textAreaRef = makeTextarea('$'); diff --git a/packages/api/src/agents/__tests__/skills.test.ts b/packages/api/src/agents/__tests__/skills.test.ts index df21c665b1..976c09eb85 100644 --- a/packages/api/src/agents/__tests__/skills.test.ts +++ b/packages/api/src/agents/__tests__/skills.test.ts @@ -253,9 +253,9 @@ describe('scopeSkillIds', () => { expect(scopeSkillIds(accessible, null)).toBe(accessible); }); - it('returns [] when agentSkills is an empty array (explicit none)', () => { + it('returns the full set when agentSkills is an empty array (no allowlist)', () => { const accessible = [makeId(), makeId()]; - expect(scopeSkillIds(accessible, [])).toEqual([]); + expect(scopeSkillIds(accessible, [])).toBe(accessible); }); it('returns intersection when agentSkills overlaps accessibleSkillIds', () => { diff --git a/packages/api/src/agents/skills.ts b/packages/api/src/agents/skills.ts index 102ea14dc3..32e3cb202b 100644 --- a/packages/api/src/agents/skills.ts +++ b/packages/api/src/agents/skills.ts @@ -101,9 +101,9 @@ export function isSkillPrimeMessage(msg: unknown): boolean { * * Semantics (pinned by unit tests): * - `undefined` / `null` → not configured, returns the full accessible catalog. - * - `[]` (empty array) → explicitly none, returns `[]`. A user who narrows their - * agent to a subset and then removes all entries is explicitly opting out of - * the full catalog fallback. + * - `[]` (empty array) → no allowlist, returns the full accessible catalog. + * Removing all selected skills in the builder restores the default full-catalog + * behavior while `skills_enabled` remains true. * - non-empty array of skill `_id` hex strings → intersection of accessible IDs * and agent-configured IDs. * @@ -120,7 +120,7 @@ export function scopeSkillIds( return accessibleSkillIds; } if (agentSkills.length === 0) { - return []; + return accessibleSkillIds; } const agentSet = new Set(agentSkills); return accessibleSkillIds.filter((oid) => agentSet.has(oid.toString()));