Skip to content

Commit 7530916

Browse files
dcramerclaude
andauthored
refactor: rename requiredSkills to skills for semantic clarity (#673)
Rename `requiredSkills` to `skills` across all tool definitions to better reflect the relationship: tools *belong* to skill categories rather than *requiring* them. This makes the authorization model clearer - a tool is enabled when ANY of its associated skills are granted. Also rename `hasRequiredSkills` to `isEnabledBySkills` for consistency. Co-authored-by: Claude Opus 4.5 <[email protected]>
1 parent c8d5068 commit 7530916

27 files changed

+61
-66
lines changed

packages/mcp-core/scripts/generate-definitions.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,12 @@ async function generateSkillDefinitions() {
131131
const t = tool as {
132132
name: string;
133133
description: string;
134-
requiredSkills: string[];
134+
skills: string[];
135135
requiredScopes: string[];
136136
};
137137

138138
// Check if this tool is enabled by this skill
139-
if (
140-
Array.isArray(t.requiredSkills) &&
141-
t.requiredSkills.includes(skill.id)
142-
) {
139+
if (Array.isArray(t.skills) && t.skills.includes(skill.id)) {
143140
skillTools.push({
144141
name: t.name,
145142
description: t.description,

packages/mcp-core/scripts/validate-skills-mapping.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ function getToolsForSkill(skill: Skill): string[] {
3636
const enabledTools: string[] = [];
3737

3838
for (const [toolName, tool] of Object.entries(tools)) {
39-
if (tool.requiredSkills.includes(skill)) {
39+
if (tool.skills.includes(skill)) {
4040
enabledTools.push(toolName);
4141
}
4242
}
@@ -88,8 +88,8 @@ function findOrphanedTools(): string[] {
8888
continue;
8989
}
9090

91-
// Check if tool has no required skills (orphaned)
92-
if (tool.requiredSkills.length === 0) {
91+
// Check if tool has no skills (orphaned)
92+
if (tool.skills.length === 0) {
9393
orphanedTools.push(toolName);
9494
}
9595
}

packages/mcp-core/src/server.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import { logIssue, type LogIssueOptions } from "./telem/logging";
3737
import { formatErrorForUser } from "./internal/error-handling";
3838
import { LIB_VERSION } from "./version";
3939
import { MCP_SERVER_NAME } from "./constants";
40-
import { hasRequiredSkills, type Skill } from "./skills";
40+
import { isEnabledBySkills, type Skill } from "./skills";
4141
import {
4242
getConstraintParametersToInject,
4343
getConstraintKeysToFilter,
@@ -155,21 +155,21 @@ function configureServer({
155155
* ==========================
156156
*
157157
* Tools are filtered at registration time based on grantedSkills.
158-
* Tool must have non-empty `requiredSkills` array to be exposed.
159-
* Empty `requiredSkills: []` means intentionally excluded from skills system.
158+
* Tool must have non-empty `skills` array to be exposed.
159+
* Empty `skills: []` means intentionally excluded from skills system.
160160
*
161161
* In agent mode, authorization is skipped - use_sentry handles it internally.
162162
*
163163
* ## Examples:
164164
* ```typescript
165-
* // Tool available in "triage" skill only:
166-
* { requiredSkills: ["triage"] }
165+
* // Tool belongs to "triage" skill only:
166+
* { skills: ["triage"] }
167167
*
168-
* // Tool available to ALL skills (foundational tool like whoami):
169-
* { requiredSkills: ALL_SKILLS }
168+
* // Tool belongs to ALL skills (foundational tool like whoami):
169+
* { skills: ALL_SKILLS }
170170
*
171171
* // Tool excluded from skills system (like use_sentry in agent mode):
172-
* { requiredSkills: [] }
172+
* { skills: [] }
173173
* ```
174174
*/
175175
let allowed = false;
@@ -178,12 +178,12 @@ function configureServer({
178178
if (agentMode) {
179179
allowed = true;
180180
}
181-
// Skills system: tool must have non-empty requiredSkills to be exposed
181+
// Skills system: tool must have non-empty skills to be exposed
182182
else if (grantedSkills) {
183-
if (tool.requiredSkills && tool.requiredSkills.length > 0) {
184-
allowed = hasRequiredSkills(grantedSkills, tool.requiredSkills);
183+
if (tool.skills && tool.skills.length > 0) {
184+
allowed = isEnabledBySkills(grantedSkills, tool.skills);
185185
}
186-
// Empty requiredSkills means NOT exposed via skills system
186+
// Empty skills means NOT exposed via skills system
187187
}
188188

189189
// Skip tool if not allowed by active authorization system

packages/mcp-core/src/skills.test.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
DEFAULT_SKILLS,
55
isValidSkill,
66
parseSkills,
7-
hasRequiredSkills,
7+
isEnabledBySkills,
88
type Skill,
99
} from "./skills";
1010

@@ -113,31 +113,31 @@ describe("skills module", () => {
113113
});
114114
});
115115

116-
describe("hasRequiredSkills", () => {
117-
it("returns true when any required skill is granted", () => {
116+
describe("isEnabledBySkills", () => {
117+
it("returns true when any tool skill is granted", () => {
118118
const grantedSkills = new Set<Skill>(["inspect", "docs"]);
119-
expect(hasRequiredSkills(grantedSkills, ["inspect"])).toBe(true);
120-
expect(hasRequiredSkills(grantedSkills, ["docs"])).toBe(true);
121-
expect(hasRequiredSkills(grantedSkills, ["inspect", "triage"])).toBe(
119+
expect(isEnabledBySkills(grantedSkills, ["inspect"])).toBe(true);
120+
expect(isEnabledBySkills(grantedSkills, ["docs"])).toBe(true);
121+
expect(isEnabledBySkills(grantedSkills, ["inspect", "triage"])).toBe(
122122
true,
123123
);
124124
});
125125

126-
it("returns false when no required skills are granted", () => {
126+
it("returns false when no tool skills are granted", () => {
127127
const grantedSkills = new Set<Skill>(["inspect", "docs"]);
128-
expect(hasRequiredSkills(grantedSkills, ["triage"])).toBe(false);
128+
expect(isEnabledBySkills(grantedSkills, ["triage"])).toBe(false);
129129
expect(
130-
hasRequiredSkills(grantedSkills, ["triage", "project-management"]),
130+
isEnabledBySkills(grantedSkills, ["triage", "project-management"]),
131131
).toBe(false);
132132
});
133133

134134
it("returns false when grantedSkills is undefined", () => {
135-
expect(hasRequiredSkills(undefined, ["inspect"])).toBe(false);
135+
expect(isEnabledBySkills(undefined, ["inspect"])).toBe(false);
136136
});
137137

138-
it("returns false when requiredSkills is empty", () => {
138+
it("returns false when toolSkills is empty", () => {
139139
const grantedSkills = new Set<Skill>(["inspect"]);
140-
expect(hasRequiredSkills(grantedSkills, [])).toBe(false);
140+
expect(isEnabledBySkills(grantedSkills, [])).toBe(false);
141141
});
142142
});
143143
});

packages/mcp-core/src/skills.ts

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ export async function getSkillsArrayWithCounts(): Promise<SkillDefinition[]> {
8282

8383
// Count tools for each skill
8484
for (const tool of Object.values(tools)) {
85-
if (Array.isArray(tool.requiredSkills)) {
86-
for (const skill of tool.requiredSkills) {
85+
if (Array.isArray(tool.skills)) {
86+
for (const skill of tool.skills) {
8787
counts.set(skill as Skill, (counts.get(skill as Skill) || 0) + 1);
8888
}
8989
}
@@ -108,13 +108,13 @@ export function isValidSkill(skill: string): skill is Skill {
108108
return skill in SKILLS;
109109
}
110110

111-
// Check if tool is enabled by skills
112-
export function hasRequiredSkills(
111+
// Check if tool is enabled by granted skills (ANY match = enabled)
112+
export function isEnabledBySkills(
113113
grantedSkills: Set<Skill> | undefined,
114-
requiredSkills: Skill[],
114+
toolSkills: Skill[],
115115
): boolean {
116-
if (!grantedSkills || requiredSkills.length === 0) return false;
117-
return requiredSkills.some((skill) => grantedSkills.has(skill));
116+
if (!grantedSkills || toolSkills.length === 0) return false;
117+
return toolSkills.some((skill) => grantedSkills.has(skill));
118118
}
119119

120120
// Parse and validate skills from input
@@ -160,10 +160,8 @@ export async function getScopesForSkills(
160160

161161
// Iterate through all tools and collect required scopes for tools enabled by granted skills
162162
for (const tool of Object.values(tools)) {
163-
// Check if any of the tool's required skills are granted
164-
const toolEnabled = tool.requiredSkills.some((reqSkill) =>
165-
grantedSkills.has(reqSkill),
166-
);
163+
// Check if any of the tool's skills are granted
164+
const toolEnabled = tool.skills.some((skill) => grantedSkills.has(skill));
167165

168166
// If tool is enabled by granted skills, add its required scopes
169167
if (toolEnabled) {

packages/mcp-core/src/tools/analyze-issue-with-seer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525

2626
export default defineTool({
2727
name: "analyze_issue_with_seer",
28-
requiredSkills: ["seer"], // Only available in seer skill
28+
skills: ["seer"], // Only available in seer skill
2929
requiredScopes: [], // No Sentry API scopes required - authorization via 'seer' skill
3030
description: [
3131
"Use Seer to analyze production errors and get detailed root cause analysis with specific code fixes.",

packages/mcp-core/src/tools/create-dsn.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111

1212
export default defineTool({
1313
name: "create_dsn",
14-
requiredSkills: ["project-management"], // Only available in project-management skill
14+
skills: ["project-management"], // Only available in project-management skill
1515
requiredScopes: ["project:write"],
1616
description: [
1717
"Create an additional DSN for an EXISTING project.",

packages/mcp-core/src/tools/create-project.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414

1515
export default defineTool({
1616
name: "create_project",
17-
requiredSkills: ["project-management"], // Only available in project-management skill
17+
skills: ["project-management"], // Only available in project-management skill
1818
requiredScopes: ["project:write", "team:read"],
1919
description: [
2020
"Create a new project in Sentry (includes DSN automatically).",

packages/mcp-core/src/tools/create-team.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { ParamOrganizationSlug, ParamRegionUrl } from "../schema";
77

88
export default defineTool({
99
name: "create_team",
10-
requiredSkills: ["project-management"], // Only available in project-management skill
10+
skills: ["project-management"], // Only available in project-management skill
1111
requiredScopes: ["team:write"],
1212
description: [
1313
"Create a new team in Sentry.",

packages/mcp-core/src/tools/find-dsns.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010

1111
export default defineTool({
1212
name: "find_dsns",
13-
requiredSkills: ["project-management"], // DSN management is part of project setup
13+
skills: ["project-management"], // DSN management is part of project setup
1414
requiredScopes: ["project:read"],
1515
description: [
1616
"List all Sentry DSNs for a specific project.",

0 commit comments

Comments
 (0)