Skip to content

Commit 25be5dc

Browse files
committed
feat(agent-core-v2): watch user-level skill roots so the catalog stays fresh
1 parent 350e7d9 commit 25be5dc

6 files changed

Lines changed: 273 additions & 7 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-code": minor
3+
---
4+
5+
Refresh the skill catalog automatically when user-level skills are created, changed, or deleted while Pythinker Code is running.

packages/agent-core-v2/src/features/skill/catalog/userFileSkillSource.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
1+
import { existsSync } from 'node:fs';
2+
3+
import { join } from 'pathe';
14
import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation';
2-
import { Disposable } from '#/_base/di/lifecycle';
5+
import { Disposable, DisposableStore } from '#/_base/di/lifecycle';
36
import { Emitter, type Event } from '#/_base/event';
7+
import { TimeoutTimer } from '#/_base/utils/timer';
8+
import { subtreeWatchFilter } from '#/_base/utils/paths';
49
import { LifecycleScope } from '#/app/scopes';
510
import { ScopeActivation, registerScopedService } from '#/_base/di/scope';
611
import { IBootstrapService } from '#/app/bootstrap/bootstrap';
712
import { IConfigService } from '#/app/config/config';
13+
import { IHostFsWatchService } from '#/os/interface/hostFsWatch';
814

915
import {
1016
MERGE_ALL_AVAILABLE_SKILLS_SECTION,
@@ -21,28 +27,38 @@ export interface IUserFileSkillSource extends ISkillSource {
2127
export const IUserFileSkillSource: ServiceIdentifier<IUserFileSkillSource> =
2228
createDecorator<IUserFileSkillSource>('userFileSkillSource');
2329

30+
const WATCH_DEBOUNCE_MS = 200;
31+
2432
export class UserFileSkillSource extends Disposable implements IUserFileSkillSource {
2533
declare readonly _serviceBrand: undefined;
2634

2735
readonly id = 'user';
2836
readonly priority = SKILL_SOURCE_PRIORITY.user;
2937
private readonly onDidChangeEmitter = this._register(new Emitter<void>());
3038
readonly onDidChange: Event<void> = this.onDidChangeEmitter.event;
39+
private readonly watchDebounce = this._register(new TimeoutTimer());
40+
private readonly watchResources = this._register(new DisposableStore());
41+
private watchReady: Promise<void> = Promise.resolve();
3142

3243
constructor(
3344
@ISkillDiscovery private readonly discovery: ISkillDiscovery,
3445
@IBootstrapService private readonly bootstrap: IBootstrapService,
3546
@IConfigService private readonly config: IConfigService,
47+
@IHostFsWatchService private readonly fsWatch: IHostFsWatchService,
3648
) {
3749
super();
3850
this._register(
3951
this.config.onDidSectionChange((event) => {
4052
if (event.domain === MERGE_ALL_AVAILABLE_SKILLS_SECTION) this.onDidChangeEmitter.fire();
4153
}),
4254
);
55+
if ((this.bootstrap.args.skillDirs?.length ?? 0) === 0) {
56+
this.watchUserSkillRoots();
57+
}
4358
}
4459

4560
async load(): Promise<SkillContribution> {
61+
await this.watchReady;
4662
if ((this.bootstrap.args.skillDirs?.length ?? 0) > 0) {
4763
return { skills: [] };
4864
}
@@ -53,6 +69,36 @@ export class UserFileSkillSource extends Disposable implements IUserFileSkillSou
5369
await userRoots(this.bootstrap.homeDir, this.bootstrap.osHomeDir, { mergeAllAvailableSkills }),
5470
);
5571
}
72+
73+
private watchUserSkillRoots(): void {
74+
const candidatesByBase = new Map<string, string[]>();
75+
for (const [base, root] of [
76+
[this.bootstrap.homeDir, join(this.bootstrap.homeDir, 'skills')],
77+
[this.bootstrap.osHomeDir, join(this.bootstrap.osHomeDir, '.agents', 'skills')],
78+
] as const) {
79+
const existing = candidatesByBase.get(base);
80+
if (existing === undefined) candidatesByBase.set(base, [root]);
81+
else existing.push(root);
82+
}
83+
const ready: Promise<void>[] = [];
84+
for (const [base, candidates] of candidatesByBase) {
85+
if (!existsSync(base)) continue;
86+
const handle = this.fsWatch.watch(base, {
87+
ignored: subtreeWatchFilter(base, candidates),
88+
signal: true,
89+
});
90+
this.watchResources.add(handle);
91+
this.watchResources.add(
92+
handle.onDidChange(() => {
93+
this.watchDebounce.cancelAndSet(() => {
94+
this.onDidChangeEmitter.fire();
95+
}, WATCH_DEBOUNCE_MS);
96+
}),
97+
);
98+
ready.push(handle.ready);
99+
}
100+
this.watchReady = Promise.allSettled(ready).then(() => undefined);
101+
}
56102
}
57103

58104
registerScopedService(

packages/agent-core-v2/src/os/backends/node-local/hostFsWatchService.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,13 @@ class SignalWatchHandle implements IHostFsWatchHandle {
184184
this.fireInvalidation();
185185
return;
186186
}
187-
onUnexpectedError(error);
187+
if (error.code === 'ENOENT') {
188+
this.readiness.resolve();
189+
} else {
190+
onUnexpectedError(error);
191+
this.fireInvalidation();
192+
}
188193
this.recovering = true;
189-
this.fireInvalidation();
190194
const delay = Math.min(NATIVE_RETRY_BASE_MS * 2 ** this.retryAttempts, NATIVE_RETRY_MAX_MS);
191195
this.retryAttempts += 1;
192196
this.retry?.dispose();

packages/agent-core-v2/src/program/program.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ export class Program {
298298
const extraAgentProfiles = own(new ExtraAgentProfileLoaderService(this.dependencies.config, this.context, this.dependencies.bootstrap, runtime.fs!, this.dependencies.log, userAgentProfiles, this.dependencies.agentProfiles));
299299
const agentProfiles = own(new WorkspaceAgentProfileLoaderService(this.context, runtime.fs!, this.dependencies.log, userAgentProfiles, runtime.watch!, this.dependencies.agentProfiles));
300300
const skillDiscovery = new RuntimeSkillDiscovery(this.dependencies.log, runtime.fs!);
301-
const userSkills = own(new UserFileSkillSource(skillDiscovery, this.dependencies.bootstrap, this.dependencies.config));
301+
const userSkills = own(new UserFileSkillSource(skillDiscovery, this.dependencies.bootstrap, this.dependencies.config, runtime.watch!));
302302
const explicitSkills = new ExplicitFileSkillSource(skillDiscovery, this.context, this.dependencies.bootstrap);
303303
const extraSkills = own(new ExtraFileSkillSource(skillDiscovery, this.dependencies.config, this.context, this.dependencies.bootstrap));
304304
const workspaceSkills = own(new WorkspaceRootSkillSource(skillDiscovery, this.context, this.dependencies.config, this.dependencies.bootstrap, runtime.watch!));
@@ -329,7 +329,7 @@ export class Program {
329329
retired: false,
330330
};
331331
} catch (error) {
332-
for (const disposable of disposables.reverse()) void disposable.dispose();
332+
for (const disposable of disposables.toReversed()) void disposable.dispose();
333333
lease.dispose();
334334
throw error;
335335
}
@@ -368,7 +368,7 @@ export class Program {
368368
private releaseGeneration(generation: ProgramGeneration): void {
369369
generation.references -= 1;
370370
if (generation.references !== 0 || !generation.retired) return;
371-
for (const disposable of [...generation.disposables].reverse()) void disposable.dispose();
371+
for (const disposable of [...generation.disposables].toReversed()) void disposable.dispose();
372372
generation.lease.dispose();
373373
}
374374

packages/agent-core-v2/test/app/bootstrap/stubs.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export function stubBootstrap(
1616
homeDir = '/tmp/pythinker-home',
1717
env: NodeJS.ProcessEnv = {},
1818
args: HostArgsInput = {},
19+
osHomeDir = '/home/test',
1920
): IBootstrapService {
2021
const scopes: Record<PersistenceScopeName, string> = {
2122
config: '',
@@ -31,7 +32,7 @@ export function stubBootstrap(
3132
platform: 'linux',
3233
arch: 'x64',
3334
cwd: '/tmp',
34-
osHomeDir: '/home/test',
35+
osHomeDir,
3536
homeDir,
3637
configPath: `${homeDir}/config.toml`,
3738
configKey: 'config.toml',

packages/agent-core-v2/test/features/skill/workspace/skillCatalog.test.ts

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,32 @@ function fsWatchStub(
164164
};
165165
}
166166

167+
function recordingWatchService(): {
168+
service: IHostFsWatchService;
169+
calls: { path: string; ignored: ((path: string) => boolean) | undefined }[];
170+
handles: { disposed: boolean }[];
171+
} {
172+
const calls: { path: string; ignored: ((path: string) => boolean) | undefined }[] = [];
173+
const handles: { disposed: boolean }[] = [];
174+
const service: IHostFsWatchService = {
175+
_serviceBrand: undefined,
176+
watch: (path, options) => {
177+
calls.push({ path, ignored: options?.ignored });
178+
const handle: IHostFsWatchHandle & { disposed: boolean } = {
179+
ready: Promise.resolve(),
180+
onDidChange: Event.None as Event<HostFsChange>,
181+
disposed: false,
182+
dispose: () => {
183+
handle.disposed = true;
184+
},
185+
};
186+
handles.push(handle);
187+
return handle;
188+
},
189+
};
190+
return { service, calls, handles };
191+
}
192+
167193
function makeHost(
168194
store: ISkillDiscovery,
169195
ws: IWorkspaceContext,
@@ -1096,4 +1122,188 @@ describe('WorkspaceSkillCatalogService', () => {
10961122
await rm(workDir, { recursive: true, force: true });
10971123
}
10981124
}, 15000);
1125+
it('watches both user-level skill roots and prunes unrelated paths', async () => {
1126+
const homeDir = await mkdtemp(join(tmpdir(), 'skill-user-home-'));
1127+
const osHomeDir = await mkdtemp(join(tmpdir(), 'skill-user-os-'));
1128+
await mkdir(join(homeDir, 'skills'), { recursive: true });
1129+
await mkdir(join(osHomeDir, '.agents', 'skills'), { recursive: true });
1130+
const { service, calls } = recordingWatchService();
1131+
const host = createScopedTestHost([
1132+
stubPair(IFlagService, stubFlag(true)),
1133+
stubPair(IBootstrapService, stubBootstrap(homeDir, {}, {}, osHomeDir)),
1134+
stubPair(IConfigService, configStub()),
1135+
stubPair(IPluginService, pluginStub()),
1136+
stubPair(ILogService, stubLog()),
1137+
stubPair(IHostFsWatchService, service),
1138+
stubPair(ISkillDiscovery, new FileSkillDiscovery(stubLog())),
1139+
]);
1140+
1141+
try {
1142+
const source = host.app.accessor.get(IUserFileSkillSource);
1143+
await source.load();
1144+
1145+
const home = calls.find((call) => call.path === homeDir);
1146+
const osHome = calls.find((call) => call.path === osHomeDir);
1147+
expect(home).toBeDefined();
1148+
expect(osHome).toBeDefined();
1149+
expect(home?.ignored?.(join(homeDir, 'skills/demo/SKILL.md'))).toBe(false);
1150+
expect(home?.ignored?.(join(homeDir, 'sessions/s1/state.json'))).toBe(true);
1151+
expect(osHome?.ignored?.(join(osHomeDir, '.agents/skills/demo/SKILL.md'))).toBe(false);
1152+
expect(osHome?.ignored?.(join(osHomeDir, 'Downloads/x.zip'))).toBe(true);
1153+
} finally {
1154+
host.dispose();
1155+
await rm(homeDir, { recursive: true, force: true });
1156+
await rm(osHomeDir, { recursive: true, force: true });
1157+
}
1158+
});
1159+
1160+
it('merges both skill-root candidates into one watch when homeDir equals osHomeDir', async () => {
1161+
const homeDir = await mkdtemp(join(tmpdir(), 'skill-user-same-'));
1162+
await mkdir(join(homeDir, 'skills'), { recursive: true });
1163+
await mkdir(join(homeDir, '.agents', 'skills'), { recursive: true });
1164+
const { service, calls } = recordingWatchService();
1165+
const host = createScopedTestHost([
1166+
stubPair(IFlagService, stubFlag(true)),
1167+
stubPair(IBootstrapService, stubBootstrap(homeDir, {}, {}, homeDir)),
1168+
stubPair(IConfigService, configStub()),
1169+
stubPair(IPluginService, pluginStub()),
1170+
stubPair(ILogService, stubLog()),
1171+
stubPair(IHostFsWatchService, service),
1172+
stubPair(ISkillDiscovery, new FileSkillDiscovery(stubLog())),
1173+
]);
1174+
1175+
try {
1176+
const source = host.app.accessor.get(IUserFileSkillSource);
1177+
await source.load();
1178+
1179+
const homeCalls = calls.filter((call) => call.path === homeDir);
1180+
expect(homeCalls).toHaveLength(1);
1181+
const ignored = homeCalls[0]?.ignored;
1182+
expect(ignored?.(join(homeDir, 'skills/demo/SKILL.md'))).toBe(false);
1183+
expect(ignored?.(join(homeDir, '.agents/skills/demo/SKILL.md'))).toBe(false);
1184+
expect(ignored?.(join(homeDir, 'sessions/s1/state.json'))).toBe(true);
1185+
} finally {
1186+
host.dispose();
1187+
await rm(homeDir, { recursive: true, force: true });
1188+
}
1189+
});
1190+
1191+
it('does not watch the user skill roots when explicit skillDirs are set', async () => {
1192+
const homeDir = await mkdtemp(join(tmpdir(), 'skill-user-explicit-'));
1193+
await mkdir(join(homeDir, 'skills'), { recursive: true });
1194+
const { service, calls } = recordingWatchService();
1195+
const host = createScopedTestHost([
1196+
stubPair(IFlagService, stubFlag(true)),
1197+
stubPair(IBootstrapService, stubBootstrap(homeDir, {}, { skillDirs: ['/explicit'] })),
1198+
stubPair(IConfigService, configStub()),
1199+
stubPair(IPluginService, pluginStub()),
1200+
stubPair(ILogService, stubLog()),
1201+
stubPair(IHostFsWatchService, service),
1202+
stubPair(ISkillDiscovery, new FileSkillDiscovery(stubLog())),
1203+
]);
1204+
1205+
try {
1206+
const source = host.app.accessor.get(IUserFileSkillSource);
1207+
await source.load();
1208+
expect(calls.map((call) => call.path)).toEqual([]);
1209+
} finally {
1210+
host.dispose();
1211+
await rm(homeDir, { recursive: true, force: true });
1212+
}
1213+
});
1214+
1215+
it('disposes the user root watches when the app scope is disposed', async () => {
1216+
const homeDir = await mkdtemp(join(tmpdir(), 'skill-user-dispose-'));
1217+
await mkdir(join(homeDir, 'skills'), { recursive: true });
1218+
const { service, handles } = recordingWatchService();
1219+
const host = createScopedTestHost([
1220+
stubPair(IFlagService, stubFlag(true)),
1221+
stubPair(IBootstrapService, stubBootstrap(homeDir)),
1222+
stubPair(IConfigService, configStub()),
1223+
stubPair(IPluginService, pluginStub()),
1224+
stubPair(ILogService, stubLog()),
1225+
stubPair(IHostFsWatchService, service),
1226+
stubPair(ISkillDiscovery, new FileSkillDiscovery(stubLog())),
1227+
]);
1228+
1229+
const source = host.app.accessor.get(IUserFileSkillSource);
1230+
await source.load();
1231+
expect(handles.length).toBeGreaterThan(0);
1232+
1233+
host.dispose();
1234+
expect(handles.every((handle) => handle.disposed)).toBe(true);
1235+
await rm(homeDir, { recursive: true, force: true });
1236+
});
1237+
1238+
it('rescans the user source when skills appear, change and disappear under the user roots', async () => {
1239+
const homeDir = await mkdtemp(join(tmpdir(), 'skill-user-watch-'));
1240+
const osHomeDir = await mkdtemp(join(tmpdir(), 'skill-os-watch-'));
1241+
const host = createScopedTestHost([
1242+
stubPair(IFlagService, stubFlag(true)),
1243+
stubPair(IBootstrapService, stubBootstrap(homeDir, {}, {}, osHomeDir)),
1244+
stubPair(IConfigService, configStub()),
1245+
stubPair(IPluginService, pluginStub()),
1246+
stubPair(ILogService, stubLog()),
1247+
stubPair(ISkillDiscovery, new FileSkillDiscovery(stubLog())),
1248+
stubPair(IHostFsWatchService, new HostFsWatchService()),
1249+
]);
1250+
const workspace = host.child('program', 'w1', [
1251+
stubPair(IWorkspaceContext, workspaceContextStub('/work')),
1252+
]);
1253+
const writeSkill = (dir: string, description: string) =>
1254+
writeFile(
1255+
join(dir, 'SKILL.md'),
1256+
`---\nname: watched-user-skill\ndescription: ${description}\n---\nbody`,
1257+
'utf8',
1258+
);
1259+
1260+
try {
1261+
const catalog = workspace.accessor.get(IWorkspaceSkillCatalog);
1262+
await catalog.load();
1263+
expect(catalog.catalog.getSkill('watched-user-skill')).toBeUndefined();
1264+
1265+
const waitForUserChange = (): Promise<string> => {
1266+
const refreshed = new Promise<string>((resolvePromise) => {
1267+
const d = catalog.onDidChange((sourceId) => {
1268+
if (sourceId !== 'user') return;
1269+
d.dispose();
1270+
resolvePromise(sourceId);
1271+
});
1272+
});
1273+
const timedOut = new Promise<never>((_resolve, reject) => {
1274+
setTimeout(() => reject(new Error('user watch refresh timed out')), 10000);
1275+
});
1276+
return Promise.race([refreshed, timedOut]);
1277+
};
1278+
1279+
const created = waitForUserChange();
1280+
const skillDir = join(homeDir, 'skills', 'watched-user-skill');
1281+
await mkdir(skillDir, { recursive: true });
1282+
await writeSkill(skillDir, 'v1');
1283+
await created;
1284+
expect(catalog.catalog.getSkill('watched-user-skill')?.description).toBe('v1');
1285+
1286+
const modified = waitForUserChange();
1287+
await writeSkill(skillDir, 'v2');
1288+
await modified;
1289+
expect(catalog.catalog.getSkill('watched-user-skill')?.description).toBe('v2');
1290+
1291+
const deleted = waitForUserChange();
1292+
await rm(skillDir, { recursive: true, force: true });
1293+
await deleted;
1294+
expect(catalog.catalog.getSkill('watched-user-skill')).toBeUndefined();
1295+
1296+
const osCreated = waitForUserChange();
1297+
const osSkillDir = join(osHomeDir, '.agents', 'skills', 'watched-user-skill');
1298+
await mkdir(osSkillDir, { recursive: true });
1299+
await writeSkill(osSkillDir, 'os');
1300+
await osCreated;
1301+
expect(catalog.catalog.getSkill('watched-user-skill')?.description).toBe('os');
1302+
} finally {
1303+
host.dispose();
1304+
await rm(homeDir, { recursive: true, force: true });
1305+
await rm(osHomeDir, { recursive: true, force: true });
1306+
}
1307+
}, 20000);
1308+
10991309
});

0 commit comments

Comments
 (0)