Skip to content

Commit 42720c8

Browse files
committed
feat(skills): auto-bundle declared skill dependencies in cloud runs
1 parent aad38ac commit 42720c8

13 files changed

Lines changed: 381 additions & 4 deletions

File tree

apps/code/src/renderer/di/bindings.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ import {
5959
type BundleLocalSkill,
6060
CLOUD_ARTIFACT_BUNDLE_LOCAL_SKILL,
6161
CLOUD_ARTIFACT_READ_FILE_AS_BASE64,
62+
CLOUD_ARTIFACT_RESOLVE_SKILL_DEPENDENCIES,
63+
type ResolveSkillBundleDependencies,
6264
} from "@posthog/core/sessions/cloudArtifactIdentifiers";
6365
import {
6466
LOCAL_HANDOFF_DIALOG,
@@ -272,6 +274,7 @@ export interface RendererBindings {
272274
[REVERT_HUNK_SERVICE]: RevertHunkService;
273275
[SKILLS_WORKSPACE_CLIENT]: SkillsWorkspaceClient;
274276
[CLOUD_ARTIFACT_BUNDLE_LOCAL_SKILL]: BundleLocalSkill;
277+
[CLOUD_ARTIFACT_RESOLVE_SKILL_DEPENDENCIES]: ResolveSkillBundleDependencies;
275278
[CLOUD_ARTIFACT_READ_FILE_AS_BASE64]: ReadFileAsBase64;
276279
[LLM_GATEWAY_SERVICE]: LlmGatewayService;
277280
[TITLE_GENERATOR_FILE_READ_CLIENT]: FileReadClient;

apps/code/src/renderer/di/container.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import type { LlmMessage } from "@posthog/core/llm-gateway/schemas";
3232
import {
3333
CLOUD_ARTIFACT_BUNDLE_LOCAL_SKILL,
3434
CLOUD_ARTIFACT_READ_FILE_AS_BASE64,
35+
CLOUD_ARTIFACT_RESOLVE_SKILL_DEPENDENCIES,
3536
} from "@posthog/core/sessions/cloudArtifactIdentifiers";
3637
import {
3738
LOCAL_HANDOFF_DIALOG,
@@ -393,6 +394,11 @@ container
393394
.toConstantValue((skillBundleRef) =>
394395
hostTrpcClient.skills.bundleLocal.query(skillBundleRef),
395396
);
397+
container
398+
.bind(CLOUD_ARTIFACT_RESOLVE_SKILL_DEPENDENCIES)
399+
.toConstantValue((skillBundleRefs) =>
400+
hostTrpcClient.skills.resolveDependencies.query(skillBundleRefs),
401+
);
396402
container.bind(LLM_GATEWAY_SERVICE).toConstantValue({
397403
prompt: (
398404
messages: LlmMessage[],

packages/core/src/sessions/cloudArtifactIdentifiers.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ export type BundleLocalSkill = (
6767
skillBundleRef: CloudSkillBundleRef,
6868
) => Promise<LocalSkillBundle>;
6969

70+
/**
71+
* Expand tagged skill refs to include their transitively-declared dependency
72+
* skills so a cloud run gets every skill it needs, not just the tagged one.
73+
*/
74+
export type ResolveSkillBundleDependencies = (
75+
skillBundleRefs: CloudSkillBundleRef[],
76+
) => Promise<CloudSkillBundleRef[]>;
77+
7078
export const CLOUD_ARTIFACT_SERVICE = Symbol.for(
7179
"posthog.core.sessions.cloudArtifactService",
7280
);
@@ -76,3 +84,6 @@ export const CLOUD_ARTIFACT_READ_FILE_AS_BASE64 = Symbol.for(
7684
export const CLOUD_ARTIFACT_BUNDLE_LOCAL_SKILL = Symbol.for(
7785
"posthog.core.sessions.cloudArtifactBundleLocalSkill",
7886
);
87+
export const CLOUD_ARTIFACT_RESOLVE_SKILL_DEPENDENCIES = Symbol.for(
88+
"posthog.core.sessions.cloudArtifactResolveSkillDependencies",
89+
);

packages/core/src/sessions/cloudArtifactService.test.ts

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest";
22
import type {
33
BundleLocalSkill,
44
CloudArtifactClient,
5+
ResolveSkillBundleDependencies,
56
} from "./cloudArtifactIdentifiers";
67
import {
78
CLOUD_ATTACHMENT_MAX_SIZE_BYTES,
@@ -18,6 +19,8 @@ function makeClient(): CloudArtifactClient {
1819
};
1920
}
2021

22+
const passthroughDeps: ResolveSkillBundleDependencies = async (refs) => refs;
23+
2124
const bundleLocalSkill: BundleLocalSkill = vi.fn(async (skillBundleRef) => {
2225
const contentBase64 = btoa("skill-bundle");
2326
return {
@@ -34,7 +37,11 @@ const bundleLocalSkill: BundleLocalSkill = vi.fn(async (skillBundleRef) => {
3437

3538
describe("CloudArtifactService", () => {
3639
it("returns empty ids when no file paths are provided", async () => {
37-
const service = new CloudArtifactService(vi.fn(), bundleLocalSkill);
40+
const service = new CloudArtifactService(
41+
vi.fn(),
42+
bundleLocalSkill,
43+
passthroughDeps,
44+
);
3845
expect(
3946
await service.uploadRunAttachments(makeClient(), "t", "r", []),
4047
).toEqual([]);
@@ -46,6 +53,7 @@ describe("CloudArtifactService", () => {
4653
const service = new CloudArtifactService(
4754
vi.fn().mockResolvedValue(base64),
4855
bundleLocalSkill,
56+
passthroughDeps,
4957
);
5058

5159
await expect(
@@ -61,6 +69,7 @@ describe("CloudArtifactService", () => {
6169
const service = new CloudArtifactService(
6270
vi.fn().mockResolvedValue(base64),
6371
bundleLocalSkill,
72+
passthroughDeps,
6473
);
6574

6675
await expect(
@@ -76,6 +85,7 @@ describe("CloudArtifactService", () => {
7685
const service = new CloudArtifactService(
7786
vi.fn().mockResolvedValue(null),
7887
bundleLocalSkill,
88+
passthroughDeps,
7989
);
8090

8191
await expect(
@@ -93,6 +103,7 @@ describe("CloudArtifactService", () => {
93103
const service = new CloudArtifactService(
94104
vi.fn().mockResolvedValue(base64),
95105
bundleLocalSkill,
106+
passthroughDeps,
96107
);
97108

98109
const client = makeClient();
@@ -127,7 +138,11 @@ describe("CloudArtifactService", () => {
127138
const fetchMock = vi
128139
.spyOn(globalThis, "fetch")
129140
.mockResolvedValue({ ok: true } as Response);
130-
const service = new CloudArtifactService(vi.fn(), bundleLocalSkill);
141+
const service = new CloudArtifactService(
142+
vi.fn(),
143+
bundleLocalSkill,
144+
passthroughDeps,
145+
);
131146
const client = makeClient();
132147

133148
(
@@ -173,4 +188,52 @@ describe("CloudArtifactService", () => {
173188
);
174189
fetchMock.mockRestore();
175190
});
191+
192+
it("uploads dependency skills the resolver adds to a tagged skill", async () => {
193+
const fetchMock = vi
194+
.spyOn(globalThis, "fetch")
195+
.mockResolvedValue({ ok: true } as Response);
196+
const resolveDeps: ResolveSkillBundleDependencies = vi.fn(async (refs) => [
197+
...refs,
198+
{ name: "dep-skill", source: "user", path: "/tmp/dep-skill" },
199+
]);
200+
const service = new CloudArtifactService(
201+
vi.fn(),
202+
bundleLocalSkill,
203+
resolveDeps,
204+
);
205+
const client = makeClient();
206+
207+
(
208+
client.prepareTaskRunArtifactUploads as ReturnType<typeof vi.fn>
209+
).mockImplementation((_taskId, _runId, uploads: unknown[]) =>
210+
uploads.map((_upload, index) => ({
211+
id: `prep-${index}`,
212+
name: `skill-${index}.zip`,
213+
type: "skill_bundle",
214+
size: 12,
215+
presigned_post: { url: "https://s3/upload", fields: { key: "k" } },
216+
})),
217+
);
218+
(
219+
client.finalizeTaskRunArtifactUploads as ReturnType<typeof vi.fn>
220+
).mockResolvedValue([{ id: "primary-1" }, { id: "dep-1" }]);
221+
222+
const ids = await service.uploadRunAttachments(
223+
client,
224+
"task-1",
225+
"run-1",
226+
[],
227+
[{ name: "primary-skill", source: "user", path: "/tmp/primary-skill" }],
228+
);
229+
230+
expect(resolveDeps).toHaveBeenCalledWith([
231+
{ name: "primary-skill", source: "user", path: "/tmp/primary-skill" },
232+
]);
233+
expect(bundleLocalSkill).toHaveBeenCalledWith(
234+
expect.objectContaining({ name: "dep-skill" }),
235+
);
236+
expect(ids).toEqual(["primary-1", "dep-1"]);
237+
fetchMock.mockRestore();
238+
});
176239
});

packages/core/src/sessions/cloudArtifactService.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import {
55
type BundleLocalSkill,
66
CLOUD_ARTIFACT_BUNDLE_LOCAL_SKILL,
77
CLOUD_ARTIFACT_READ_FILE_AS_BASE64,
8+
CLOUD_ARTIFACT_RESOLVE_SKILL_DEPENDENCIES,
89
type CloudArtifactClient,
910
type CloudArtifactUploadRequest,
1011
type CloudSkillBundleRef,
1112
type FinalizedCloudArtifact,
1213
type PreparedCloudArtifact,
14+
type ResolveSkillBundleDependencies,
1315
} from "./cloudArtifactIdentifiers";
1416

1517
const ATTACHMENT_SOURCE = "posthog_code";
@@ -123,6 +125,8 @@ export class CloudArtifactService {
123125
private readonly readFileAsBase64: ReadFileAsBase64,
124126
@inject(CLOUD_ARTIFACT_BUNDLE_LOCAL_SKILL)
125127
private readonly bundleLocalSkill: BundleLocalSkill,
128+
@inject(CLOUD_ARTIFACT_RESOLVE_SKILL_DEPENDENCIES)
129+
private readonly resolveSkillBundleDependencies: ResolveSkillBundleDependencies,
126130
) {}
127131

128132
async uploadTaskStagedAttachments(
@@ -225,8 +229,15 @@ export class CloudArtifactService {
225229
private async loadCloudSkillBundles(
226230
skillBundleRefs: CloudSkillBundleRef[],
227231
): Promise<LoadedCloudAttachment[]> {
232+
if (skillBundleRefs.length === 0) {
233+
return [];
234+
}
235+
// Pull in dependency skills the tagged ones declare, so a skill that needs
236+
// another arrives in the sandbox together with it.
237+
const expandedRefs =
238+
await this.resolveSkillBundleDependencies(skillBundleRefs);
228239
return Promise.all(
229-
skillBundleRefs.map(async (skillBundleRef) => {
240+
expandedRefs.map(async (skillBundleRef) => {
230241
const bundle = await this.bundleLocalSkill(skillBundleRef);
231242
const bytes = base64ToUint8Array(bundle.contentBase64);
232243
if (bytes.byteLength !== bundle.size) {

packages/host-router/src/routers/skills.router.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import {
1414
readSkillFileInput,
1515
readSkillFileOutput,
1616
renameSkillFileInput,
17+
resolveSkillDependenciesInput,
18+
resolveSkillDependenciesOutput,
1719
saveSkillFileInput,
1820
saveSkillManifestInput,
1921
skillContentsInput,
@@ -44,6 +46,14 @@ export const skillsRouter = router({
4446
.query(({ ctx, input }) =>
4547
ctx.container.get<SkillsService>(SKILLS_SERVICE).bundleLocalSkill(input),
4648
),
49+
resolveDependencies: publicProcedure
50+
.input(resolveSkillDependenciesInput)
51+
.output(resolveSkillDependenciesOutput)
52+
.query(({ ctx, input }) =>
53+
ctx.container
54+
.get<SkillsService>(SKILLS_SERVICE)
55+
.resolveSkillBundleDependencies(input),
56+
),
4757
contents: publicProcedure
4858
.input(skillContentsInput)
4959
.output(skillContentsOutput)

packages/ui/src/features/sessions/sessionServiceHost.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ const mockTrpcFs = vi.hoisted(() => ({
4747
const mockTrpcSkills = vi.hoisted(() => ({
4848
list: { query: vi.fn() },
4949
bundleLocal: { query: vi.fn() },
50+
resolveDependencies: { query: vi.fn() },
5051
}));
5152

5253
const mockTrpcHandoff = vi.hoisted(() => ({
@@ -427,6 +428,10 @@ describe("SessionService", () => {
427428
mockTrpcSkills.bundleLocal.query.mockRejectedValue(
428429
new Error("Unexpected skill bundle upload"),
429430
);
431+
// Dependency resolution is a no-op passthrough by default (no declared deps).
432+
mockTrpcSkills.resolveDependencies.query.mockImplementation(
433+
async (refs: unknown) => refs,
434+
);
430435
mockTrpcHandoff.preflightToCloud.query.mockResolvedValue({
431436
canHandoff: true,
432437
});

packages/ui/src/features/sessions/sessionServiceHost.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ function buildSessionServiceDeps(): SessionServiceDeps {
6262
const cloudArtifactService = new CloudArtifactService(
6363
(filePath) => trpc.fs.readFileAsBase64.query({ filePath }),
6464
(skillBundleRef) => trpc.skills.bundleLocal.query(skillBundleRef),
65+
(skillBundleRefs) => trpc.skills.resolveDependencies.query(skillBundleRefs),
6566
);
6667

6768
return {
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { describe, expect, it } from "vitest";
2+
import { parseSkillDependencies } from "./parse-skill-frontmatter";
3+
4+
describe("parseSkillDependencies", () => {
5+
it.each([
6+
["absent", `---\nname: a\ndescription: d\n---\nbody`, []],
7+
[
8+
"block sequence",
9+
`---\nname: a\ndependencies:\n - one\n - two\n---\nbody`,
10+
["one", "two"],
11+
],
12+
[
13+
"flow sequence",
14+
`---\nname: a\ndependencies: [one, two]\n---\nbody`,
15+
["one", "two"],
16+
],
17+
[
18+
"quoted entries",
19+
`---\nname: a\ndependencies:\n - "one"\n - 'two'\n---\nbody`,
20+
["one", "two"],
21+
],
22+
["no frontmatter", `just a body`, []],
23+
])("parses %s", (_label, content, expected) => {
24+
expect(parseSkillDependencies(content)).toEqual(expected);
25+
});
26+
});

packages/workspace-server/src/services/skills/parse-skill-frontmatter.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,64 @@ export function parseSkillFrontmatter(
2121
return { name, description };
2222
}
2323

24+
/**
25+
* Extracts the optional `dependencies` list from a SKILL.md frontmatter — the
26+
* names of other skills this skill needs. Supports flow (`dependencies: [a, b]`)
27+
* and block (`dependencies:\n - a\n - b`) sequences. Returns [] when absent.
28+
*/
29+
export function parseSkillDependencies(content: string): string[] {
30+
const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---/);
31+
if (!match) return [];
32+
return extractYamlList(match[1], "dependencies");
33+
}
34+
35+
function unquoteYamlScalar(value: string): string {
36+
if (
37+
(value.startsWith("'") && value.endsWith("'")) ||
38+
(value.startsWith('"') && value.endsWith('"'))
39+
) {
40+
return value.slice(1, -1);
41+
}
42+
return value;
43+
}
44+
45+
function extractYamlList(yaml: string, key: string): string[] {
46+
const lines = yaml.split("\n");
47+
// Match `key:` at the start of the line with a literal prefix rather than a
48+
// RegExp built from the (caller-supplied) key — avoids a metacharacter
49+
// footgun if this helper is ever reused with a key like `my.key`.
50+
const prefix = `${key}:`;
51+
52+
for (let i = 0; i < lines.length; i++) {
53+
if (!lines[i].startsWith(prefix)) continue;
54+
55+
const inline = lines[i].slice(prefix.length).trim();
56+
// Flow sequence or comma list: `[a, b]` / `a, b`
57+
if (inline) {
58+
const stripped = inline.replace(/^\[/, "").replace(/\]$/, "");
59+
return stripped
60+
.split(",")
61+
.map((entry) => unquoteYamlScalar(entry.trim()))
62+
.filter((entry) => entry.length > 0);
63+
}
64+
65+
// Block sequence: subsequent `- item` lines
66+
const items: string[] = [];
67+
for (let j = i + 1; j < lines.length; j++) {
68+
const itemMatch = lines[j].match(/^\s*-\s+(.*)$/);
69+
if (!itemMatch) {
70+
if (/^\s*$/.test(lines[j])) continue;
71+
break;
72+
}
73+
const value = unquoteYamlScalar(itemMatch[1].trim());
74+
if (value) items.push(value);
75+
}
76+
return items;
77+
}
78+
79+
return [];
80+
}
81+
2482
function extractYamlValue(yaml: string, key: string): string | null {
2583
const lines = yaml.split("\n");
2684

0 commit comments

Comments
 (0)