Skip to content

simplify SkillReference, remove index field, calculate based on array index #1947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
dimaMachina wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

simplify SkillReference, remove index field, calculate based on array index #1947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a075799
upd
dimaMachina Feb 12, 2026
c25b6e6
upd
dimaMachina Feb 12, 2026
fa45f72
upd
dimaMachina Feb 12, 2026
0a8cd1d
upd
dimaMachina Feb 12, 2026
bc01bea
simplify SkillReference
dimaMachina Feb 12, 2026
7843a2e
simplify SkillReference
dimaMachina Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions agents-cli/src/commands/pull-v3/components/__tests__/sub-agent-generator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,31 @@ describe('Sub-Agent Generator', () => {
expect(definition).not.toContain('legacyAgent2,');
});

it('should generate skill references without index and keep index-based ordering', () => {

@dimaMachina dimaMachina Feb 12, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('should generate skill references without index and keep index-based ordering', () => {
it('should generate skill references and keep index-based ordering', () => {

const skillsData = {
name: 'Skills Agent',
skills: [
{ id: 'weather-safety-guardrails', alwaysLoaded: true },
{ id: 'structured-itinerary-responses', alwaysLoaded: false },
],
};

const definition = generateSubAgentDefinition(
'skills-agent',
skillsData,
undefined,
mockRegistry
);

expect(definition).toContain('skills: () => [');
expect(definition).toContain("{ id: 'weather-safety-guardrails', alwaysLoaded: true },");
expect(definition).toContain("{ id: 'structured-itinerary-responses' },");
expect(definition).not.toContain('index:');
expect(definition.indexOf("{ id: 'weather-safety-guardrails'")).toBeLessThan(
definition.indexOf("{ id: 'structured-itinerary-responses'")
);
});

it('should not generate stopWhen without stepCountIs', () => {
const noStepCountData = {
name: 'No Step Count Agent',
Expand Down
1 change: 0 additions & 1 deletion agents-cli/src/commands/pull-v3/components/sub-agent-generator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ export function generateSubAgentDefinition(
lines.push(`${indentation}skills: () => [`);
for (const skill of agentData.skills) {
const parts = [`id: ${formatString(skill.id, q)}`];
parts.push(`index: ${skill.index}`);
if (skill.alwaysLoaded) {
parts.push(`alwaysLoaded: ${skill.alwaysLoaded}`);
}
Expand Down
4 changes: 2 additions & 2 deletions ...kbook/template-projects/activities-planner-advanced/agents/activities-planner-advanced.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ const activitiesPlanner = subAgent({
'tz'
)}. When the user asks about activities in a given location, first ask the coordinates agent for the coordinates, and then pass those coordinates to the weather forecast agent to get the weather forecast. Then based on the weather forecast, ask the websearch MCP tool to search the web for good activities given the weather. Once you have the activities, use the calculate-activity-score tool to calculate a score for one of the activities based on the weather conditions- and then show the user the activity score in your response. When you receive web search results, create citation artifacts to document your sources.`,
skills: () => [
{ id: 'weather-safety-guardrails', index: 0 },
{ id: 'structured-itinerary-responses', index: 1 },
{ id: 'weather-safety-guardrails', alwaysLoaded: true },
{ id: 'structured-itinerary-responses' },
],
canDelegateTo: () => [weatherForecaster, coordinatesAgent],
canUse: () => [calculateActivityScore, exaMcpTool.with({ selectedTools: ['web_search_exa'] })],
Expand Down
19 changes: 7 additions & 12 deletions agents-docs/content/typescript-sdk/skills.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ const activitiesPlanner = subAgent({
description: 'Plans activities based on weather conditions',
prompt: 'Help users plan activities based on current weather.',
skills: () => [
{ id: 'weather-safety-guardrails', index: 0 },
{ id: 'structured-itinerary-responses', index: 1 },
{ id: 'weather-safety-guardrails' },
{ id: 'structured-itinerary-responses' },
],
});
```
Expand All @@ -118,9 +118,10 @@ const activitiesPlanner = subAgent({
| Field | Type | Required | Description |
|-------|------|----------|-------------|
| `id` | string | Yes | The skill identifier (matches the `name` in SKILL.md). |
| `index` | number | No | Controls ordering. Higher index = higher priority in the prompt. |
| `alwaysLoaded` | boolean | No | If `true`, skill content is always included. If `false` (default), loaded on demand. |

Skill order and priority follow the order of items in the `skills` array.
Comment on lines 122 to +123

@claude claude Bot Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 Consider Clarify priority direction

Issue: The statement "Skill order and priority follow the order of items in the skills array" doesn't specify the priority direction.

Why: The system prompt instruction in PromptConfig.ts (line 267) says "Apply skills by index; later entries weigh more." This implies later array positions = higher priority, which should be explicitly documented for developers.

Fix: Consider clarifying the priority semantics:

Suggested change
Skill order and priority follow the order of items in the `skills` array.
Skill order and priority follow the order of items in the `skills` array. Later entries in the array take higher priority when skills conflict.

Refs:


### Attaching Skills

Define attached skills to subagents:
Expand All @@ -130,13 +131,7 @@ const mySubAgent = subAgent({
id: 'my-sub-agent',
name: 'My Sub Agent',
prompt: 'You are a helpful assistant.',
skills: () => [
{
id: 'inline-skill',
index: 0,
alwaysLoaded: true,
},
],
skills: () => [{ id: 'inline-skill', alwaysLoaded: true } ],
});
```

Expand Down Expand Up @@ -212,8 +207,8 @@ const activitiesPlanner = subAgent({
description: 'Plans activities considering weather conditions',
prompt: 'Help users find activities based on weather forecasts.',
skills: () => [
{ id: 'weather-safety-guardrails', index: 0 },
{ id: 'structured-itinerary-responses', index: 1 },
{ id: 'weather-safety-guardrails' },
{ id: 'structured-itinerary-responses' },
],
});

Expand Down
4 changes: 2 additions & 2 deletions packages/agents-sdk/src/__tests__/builder/agent.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,8 @@ describe('Agent', () => {
description: 'Agent with skills',
prompt: 'Use skills when needed.',
skills: () => [
{ id: 'weather-safety-guardrails', index: 0 },
{ id: 'structured-itinerary-responses', index: 1, alwaysLoaded: true },
{ id: 'weather-safety-guardrails' },
{ id: 'structured-itinerary-responses', alwaysLoaded: true },
],
});

Expand Down
24 changes: 10 additions & 14 deletions packages/agents-sdk/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
FullAgentDefinition,
McpTransportConfig,
ModelSettings,
SkillSelect,
StatusUpdateSettings,
SubAgentApiInsert,
ToolInsert,
Expand Down Expand Up @@ -81,21 +82,16 @@ export interface ToolResult {
error?: string;
}

export interface SkillDefinition {
export type SkillDefinition = Pick<
SkillSelect,
'id' | 'name' | 'description' | 'content' | 'metadata' | 'createdAt' | 'updatedAt'
>;

export interface SkillReference {
id: string;
name: string;
description: string;
content: string;
metadata: Record<string, string> | null;
createdAt?: string;
updatedAt?: string;
/** @default false */
alwaysLoaded?: boolean;
}
Comment on lines +90 to 94

@claude claude Bot Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 MAJOR Breaking type contract change

Issue: The SkillReference type was changed from a union type supporting 4 input forms to a simple interface:

Before:

export type SkillReference =
  | string
  | { id: string; index?: number; alwaysLoaded?: boolean }
  | { skillId: string; index?: number; alwaysLoaded?: boolean }
  | (SkillDefinition & { index?: number; alwaysLoaded?: boolean });

After:

export interface SkillReference {
  id: string;
  alwaysLoaded?: boolean;
}

Why: This is a breaking change for TypeScript consumers of @inkeep/agents-sdk who use any of the removed forms:

  • skills: () => ['my-skill'] (string shorthand)
  • skills: () => [{ skillId: 'x' }] (skillId variant)
  • skills: () => [{ id: 'x', index: 0 }] (explicit index)

The runtime implementation in subAgent.ts (lines 247-272) still handles all legacy formats, so JavaScript users won't break, but TypeScript users will get compilation errors after upgrading.

Fix: Either:

  1. If intentionally breaking: Create a changeset with pnpm bump minor --pkg agents-sdk "Simplify SkillReference type: remove index field and legacy input forms. Skills now ordered by array position." and add migration guidance
  2. If preserving compat: Keep the union type and mark deprecated variants with JSDoc @deprecated comments

Refs:

@claude claude Bot Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MINOR Type asymmetry between input and output

Issue: The input type SkillReference no longer has index, but SubAgentInterface.getSkills() return type (lines 311-316) still includes index?: number. This creates a conceptual mismatch.

Why: Users cannot specify index when defining skills, but the returned type suggests index might be absent (optional). In practice, the implementation always calculates and populates index from array position.

Fix: Consider making the return type's index required (index: number) since it's always computed, or document that index is derived from array position:

getSkills(): Array<{
  id: string;
  index: number;  // Always calculated from array position
  alwaysLoaded?: boolean;
  skill?: SkillDefinition;
}>;

Refs:


export type SkillReference =
| string
| { id: string; index?: number; alwaysLoaded?: boolean }
| { skillId: string; index?: number; alwaysLoaded?: boolean }
| (SkillDefinition & { index?: number; alwaysLoaded?: boolean });
export type AllDelegateInputInterface =
| SubAgentInterface
| subAgentExternalAgentInterface
Expand Down Expand Up @@ -350,7 +346,7 @@ export type subAgentTeamAgentInterface = {

export interface AgentInterface {
init(): Promise<void>;
setConfig(tenantId: string, projectId: string, apiUrl: string, skills?: SkillDefinition[]): void;
setConfig(tenantId: string, projectId: string, apiUrl: string): void;
getId(): string;
getName(): string;
getDescription(): string | undefined;
Expand Down
Loading