Skip to content

Commit 579e093

Browse files
committed
test(agent-core): pin the tools and turn limit a prompt refresh must keep
Cover what refreshSystemPrompt is not allowed to touch: reloadSkills now asserts the active tool names and maxStepsPerTurn are unchanged, and a new case shows a skill saved into an empty root is invocable after the reload. Also replace the ?? 0 index fallback in the experiments call-order assertion with a non-null assertion, so a missing call reports the missing call rather than a comparison against a sentinel, and use a star re-export for PermissionModeBadge.
1 parent 722c011 commit 579e093

3 files changed

Lines changed: 64 additions & 5 deletions

File tree

apps/pythinker-code/test/tui/commands/experiments.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ describe('experimental feature command handlers', () => {
9191
// A flag can gate which skills exist, so rebuilding the command set before
9292
// the reload read the registry the reload was about to replace.
9393
expect(host.session.reloadSession.mock.invocationCallOrder[0]).toBeLessThan(
94-
host.refreshSkillCommands.mock.invocationCallOrder[0] ?? 0,
94+
host.refreshSkillCommands.mock.invocationCallOrder[0]!,
9595
);
9696
expect(host.reloadCurrentSessionView).toHaveBeenCalledWith(
9797
host.session,

apps/vscode/webview-ui/src/components/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@ export { InlineError } from "./InlineError";
2424
export { QuestionDialog } from "./QuestionDialog";
2525
export { PlanCard } from "./PlanCard";
2626
export { PlanModeButton } from "./PlanModeButton";
27-
export { PermissionModeBadge } from "./PermissionModeBadge";
27+
export * from "./PermissionModeBadge";
2828
export { BrailleSpinner } from "./BrailleSpinner";
2929
export { SilverSpinner } from "./SilverSpinner";

packages/agent-core/test/session/init.test.ts

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,14 +1181,19 @@ describe('AgentAPI.startBtw', () => {
11811181
homedir: sessionDir,
11821182
rpc: createSessionRpc([]),
11831183
skills: { explicitDirs: [skillsRoot] },
1184+
providerManager: testProviderManager(),
11841185
});
11851186

11861187
try {
11871188
const { agent: main } = await session.createAgent(
11881189
{ type: 'main' },
11891190
{ profile: skillListingProfile() },
11901191
);
1192+
main.config.update({ modelAlias: 'mock-model', thinkingLevel: 'off' });
11911193
expect(main.config.systemPrompt).not.toContain('audit-routes');
1194+
const toolsBefore = main.tools.loopTools.map((tool) => tool.name);
1195+
expect(toolsBefore).toEqual(['Read', 'Write']);
1196+
expect(main.config.maxStepsPerTurn).toBe(7);
11921197

11931198
await mkdir(join(skillsRoot, 'audit-routes'), { recursive: true });
11941199
await writeFile(
@@ -1204,14 +1209,66 @@ describe('AgentAPI.startBtw', () => {
12041209
// learns that the skill it was just told about exists.
12051210
expect(main.config.systemPrompt).toContain('audit-routes');
12061211
// Only the prompt. Re-applying the whole profile would reset the tools of
1207-
// an agent that is already running.
1212+
// an agent that is already running, and its turn limit with them.
12081213
expect(setActiveTools).not.toHaveBeenCalled();
1214+
expect(main.tools.loopTools.map((tool) => tool.name)).toEqual(toolsBefore);
1215+
expect(main.config.maxStepsPerTurn).toBe(7);
12091216
expect(main.config.profileName).toBe('skill-listing');
12101217
} finally {
12111218
await session.close();
12121219
}
12131220
});
12141221

1222+
it('a skill saved into an empty root is invocable after reloadSkills', async () => {
1223+
const workDir = await makeTempDir();
1224+
const sessionDir = await makeTempDir();
1225+
const skillsRoot = join(workDir, 'skills');
1226+
await mkdir(skillsRoot, { recursive: true });
1227+
1228+
const session = new Session({
1229+
id: 'test-reload-skills-tool',
1230+
kaos: testKaos.withCwd(workDir),
1231+
homedir: sessionDir,
1232+
rpc: createSessionRpc([]),
1233+
skills: { explicitDirs: [skillsRoot] },
1234+
providerManager: testProviderManager(),
1235+
});
1236+
1237+
try {
1238+
const { agent: main } = await session.createAgent(
1239+
{ type: 'main' },
1240+
{ profile: skillListingProfile(['Skill', 'Read']) },
1241+
);
1242+
main.config.update({ modelAlias: 'mock-model', thinkingLevel: 'off' });
1243+
1244+
// The builtin set is built once, and it only carries the Skill tool when
1245+
// a skill was already invocable. The user root is empty here, so what
1246+
// keeps the tool present is `loadSkills` registering the builtin skills
1247+
// before any agent is built — `createAgent` awaits that load. Were the
1248+
// tool to go missing, a saved workflow would be listed and uncallable.
1249+
expect(main.tools.loopTools.map((tool) => tool.name)).toContain('Skill');
1250+
1251+
await mkdir(join(skillsRoot, 'audit-routes'), { recursive: true });
1252+
await writeFile(
1253+
join(skillsRoot, 'audit-routes', 'SKILL.md'),
1254+
['---', 'name: audit-routes', 'description: Audit routes', '---', '', 'Body.'].join('\n'),
1255+
);
1256+
1257+
const setActiveTools = vi.spyOn(main.tools, 'setActiveTools');
1258+
await session.reloadSkills();
1259+
1260+
// The tool reads the registry as it runs, so the reload is all it needs
1261+
// to reach a skill written after the session opened.
1262+
expect(main.tools.loopTools.map((tool) => tool.name)).toContain('Skill');
1263+
expect(
1264+
(await session.listSkills()).map((skill) => skill.name),
1265+
).toContain('audit-routes');
1266+
expect(setActiveTools).not.toHaveBeenCalled();
1267+
} finally {
1268+
await session.close();
1269+
}
1270+
});
1271+
12151272
it('discovers sub-skills and builtins', async () => {
12161273
const workDir = await makeTempDir();
12171274
const sessionDir = await makeTempDir();
@@ -1305,7 +1362,7 @@ function testProfile(): ResolvedAgentProfile {
13051362
}
13061363

13071364
/** Renders the skill listing the way the real template's `PYTHINKER_SKILLS` does. */
1308-
function skillListingProfile(): ResolvedAgentProfile {
1365+
function skillListingProfile(tools: string[] = ['Read', 'Write']): ResolvedAgentProfile {
13091366
return {
13101367
name: 'skill-listing',
13111368
systemPrompt: (context) =>
@@ -1314,7 +1371,9 @@ function skillListingProfile(): ResolvedAgentProfile {
13141371
? context.skills
13151372
: (context.skills?.getModelSkillListing() ?? '')
13161373
}</skills>`,
1317-
tools: [],
1374+
tools,
1375+
// Non-default, so a refresh that resets the turn limit is visible.
1376+
maxTurns: 7,
13181377
};
13191378
}
13201379

0 commit comments

Comments
 (0)