mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🔐 chore: Skills Permissions Housekeeping, Reachable Admin Dialog + Defaults Tests (#12766)
* 🔐 chore: Skills permissions housekeeping — reachable admin dialog + defaults tests
Phase 9 housekeeping pass. Skills was already gated on `PermissionTypes.SKILLS`
(seeded from `interface.skills`) and `AgentCapabilities.skills` everywhere it
matters, but two smaller parity gaps with Prompts/Memory/MCP remained:
- The skills admin settings dialog had no UI entry point. The only mount was
inside an unused `FilterSkills` component, so admins had no way to reach the
role-permissions editor for skills. Mounted it in `SkillsAccordion` gated on
`SystemRoles.ADMIN`, matching the `PromptsAccordion` pattern.
- No regression lock on skill permission defaults. `roles.spec.ts` asserted
structural completeness but not the specific shape — a future refactor
could silently flip ADMIN's `USE/CREATE/SHARE/SHARE_PUBLIC` to false or
drop SKILLS from USER defaults without failing. Added explicit Skills
assertions for both roles.
- No lock on `AgentCapabilities.skills` being in `defaultAgentCapabilities`.
Added an assertion in `endpoints.spec.ts`.
* 🩹 fix: Remove duplicate `const appConfig` in Responses createResponse
The Skills polish commit (#12760) added `const appConfig = req.config;` at
line 381 inside the try block of `createResponse`, without noticing that
the earlier drive-by fix (2463b6acd) already declared it at the function
top (line 283). The second `const` creates a new block-scoped binding
inside the try, so earlier references within the same block (e.g.
line 348's `appConfig?.endpoints?.[EModelEndpoint.agents]?.allowedProviders`)
now hit the TDZ instead of the outer binding and throw
`ReferenceError: Cannot access 'appConfig' before initialization` —
which the outer try/catch then swallows into a generic 500.
This surfaced as all six token-usage tests in
`api/server/controllers/agents/__tests__/responses.unit.spec.js` failing
with `mockRecordCollectedUsage` never being called (because the throw
skips past the `recordCollectedUsage(...)` call).
Dropping the inner re-declaration restores the full control flow. All 11
tests in the file pass again.
* 🧹 refactor: Address review nits on Phase 9 housekeeping
- Move the `defaultAgentCapabilities` regression test out of the
`createEndpointsConfigService` describe block and into its own
top-level describe. It tests a module constant and has no relationship
to the service factory; nesting was misleading and made it easier to
accidentally drop if the service tests are later restructured.
- Re-order local imports in `SkillsAccordion.tsx` longest-to-shortest
per AGENTS.md convention (`SkillsSidePanel` 48 chars before
`useAuthContext` 41 chars).
This commit is contained in:
parent
91cd3f7b7c
commit
ac913aa886
4 changed files with 41 additions and 1 deletions
|
|
@ -378,7 +378,6 @@ const createResponse = async (req, res) => {
|
|||
getSkillByName: db.getSkillByName,
|
||||
};
|
||||
|
||||
const appConfig = req.config;
|
||||
const enabledCapabilities = new Set(
|
||||
appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -1,9 +1,18 @@
|
|||
import { SystemRoles } from 'librechat-data-provider';
|
||||
import { AdminSettings } from '~/components/Skills/buttons';
|
||||
import SkillsSidePanel from './SkillsSidePanel';
|
||||
import { useAuthContext } from '~/hooks';
|
||||
|
||||
export default function SkillsAccordion() {
|
||||
const { user } = useAuthContext();
|
||||
return (
|
||||
<div className="flex h-auto w-full flex-col">
|
||||
<SkillsSidePanel className="h-auto border-r-0" />
|
||||
{user?.role === SystemRoles.ADMIN && (
|
||||
<div className="flex w-full items-center justify-end px-4 pb-2">
|
||||
<AdminSettings />
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -238,3 +238,9 @@ describe('createEndpointsConfigService', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('defaultAgentCapabilities', () => {
|
||||
it('includes AgentCapabilities.skills so skills are enabled by default', () => {
|
||||
expect(defaultAgentCapabilities).toContain(AgentCapabilities.skills);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -130,4 +130,30 @@ describe('roleDefaults', () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('SKILLS permission defaults', () => {
|
||||
it('grants ADMIN all four skill permissions by default', () => {
|
||||
const adminSkills = roleDefaults[SystemRoles.ADMIN].permissions[
|
||||
PermissionTypes.SKILLS
|
||||
] as Record<string, boolean>;
|
||||
expect(adminSkills).toEqual({
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: true,
|
||||
[Permissions.SHARE_PUBLIC]: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('grants USER USE+CREATE but no sharing by default', () => {
|
||||
const userSkills = roleDefaults[SystemRoles.USER].permissions[
|
||||
PermissionTypes.SKILLS
|
||||
] as Record<string, boolean>;
|
||||
expect(userSkills).toEqual({
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue