Skip to content

Commit 13f51bb

Browse files
ericallamericallam
committed
fix(cli): stop chat.agent skills silently disappearing from trigger dev
The dev CLI ran a separate skill-discovery indexer pass with a bare process.env, while the actual worker indexer ran with the full execution env. Task files that read process.env at module top level imported cleanly in the worker pass and threw in the skill pass — the latter swallowed the error and skipped skill copying, so skill.local() failed at runtime with ENOENT. Drop the duplicate pass. The skill registry is already part of the worker manifest, so copy skill folders from there after initialize. A bad SKILL.md now surfaces as a startup error instead of silently disappearing skills.
1 parent 9d47608 commit 13f51bb

4 files changed

Lines changed: 116 additions & 80 deletions

File tree

‎.changeset/bundle-skills-single-pass.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"trigger.dev": patch
3+
---
4+
5+
Fix `chat.agent` skills silently missing in `trigger dev` for projects whose task files read `process.env` at module top level (e.g. a third-party SDK client initialized at import). Skill folders now bundle into `.trigger/skills/` reliably regardless of which env vars are set when the CLI launches.

‎packages/cli-v3/src/build/bundleSkills.ts‎

Lines changed: 85 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import { createHash } from "node:crypto";
21
import { readFile } from "node:fs/promises";
3-
import { dirname, isAbsolute, join, resolve as resolvePath } from "node:path";
2+
import { isAbsolute, join, resolve as resolvePath } from "node:path";
43
import type { BuildManifest, SkillManifest } from "@trigger.dev/core/v3/schemas";
54
import { copyDirectoryRecursive } from "@trigger.dev/build/internal";
65
import { indexWorkerManifest } from "../indexing/indexWorkerManifest.js";
@@ -21,13 +20,84 @@ export type BundleSkillsResult = {
2120
skills: SkillManifest[];
2221
};
2322

23+
export type CopySkillFoldersOptions = {
24+
skills: SkillManifest[];
25+
/** Root where `{destinationRoot}/{id}/` folders will be created. */
26+
destinationRoot: string;
27+
/** Used to resolve relative `filePath` references in skill manifests. */
28+
workingDir: string;
29+
/** Only `debug` is used. `BuildLogger` and the cli `logger` both satisfy this shape. */
30+
logger: { debug: (...args: unknown[]) => void };
31+
};
32+
33+
/**
34+
* Copy each skill's source folder to `{destinationRoot}/{id}/`. Validates
35+
* that `SKILL.md` exists and has the required frontmatter. Pure file IO —
36+
* no indexer subprocess, no env handling.
37+
*
38+
* Used by the dev path (driven by the main worker indexer's skills list)
39+
* and indirectly by the deploy path (via `bundleSkills` which discovers
40+
* skills via its own indexer pass first, then delegates here).
41+
*/
42+
export async function copySkillFolders(
43+
options: CopySkillFoldersOptions
44+
): Promise<SkillManifest[]> {
45+
const { skills, destinationRoot, workingDir, logger } = options;
46+
47+
if (skills.length === 0) {
48+
return [];
49+
}
50+
51+
for (const skill of skills) {
52+
const callerDir = skill.filePath
53+
? resolvePath(workingDir, skill.filePath, "..")
54+
: workingDir;
55+
const sourcePath = isAbsolute(skill.sourcePath)
56+
? skill.sourcePath
57+
: resolvePath(callerDir, skill.sourcePath);
58+
const skillMdPath = join(sourcePath, "SKILL.md");
59+
60+
let skillMd: string;
61+
try {
62+
skillMd = await readFile(skillMdPath, "utf8");
63+
} catch {
64+
throw new Error(
65+
`Skill "${skill.id}": SKILL.md not found at ${skillMdPath}. ` +
66+
`Registered via skills.define({ id: "${skill.id}", path: "${skill.sourcePath}" }) ` +
67+
`at ${skill.filePath}.`
68+
);
69+
}
70+
71+
if (!/^---\r?\n[\s\S]*?\r?\n---/.test(skillMd)) {
72+
throw new Error(
73+
`Skill "${skill.id}": SKILL.md at ${skillMdPath} is missing a frontmatter block.`
74+
);
75+
}
76+
if (!/\bname:\s*\S/.test(skillMd) || !/\bdescription:\s*\S/.test(skillMd)) {
77+
throw new Error(
78+
`Skill "${skill.id}": SKILL.md at ${skillMdPath} frontmatter must include both \`name\` and \`description\`.`
79+
);
80+
}
81+
82+
const skillDest = join(destinationRoot, skill.id);
83+
logger.debug(`[copySkillFolders] Copying ${sourcePath} → ${skillDest}`);
84+
await copyDirectoryRecursive(sourcePath, skillDest);
85+
}
86+
87+
return [...skills].sort((a, b) => a.id.localeCompare(b.id));
88+
}
89+
2490
/**
2591
* Built-in skill bundler — not an extension. Runs the indexer locally
26-
* against the bundled worker output to discover `ai.defineSkill(...)`
92+
* against the bundled worker output to discover `skills.define(...)`
2793
* registrations, validates each skill's `SKILL.md`, and copies the
2894
* folder into `{outputPath}/.trigger/skills/{id}/` so the deploy image
2995
* picks it up via the existing Dockerfile `COPY`.
3096
*
97+
* Used by the deploy path. The dev path uses `copySkillFolders` directly,
98+
* driven by the main worker indexer that already runs in `BackgroundWorker.initialize` —
99+
* no duplicate indexer pass needed there.
100+
*
31101
* No `trigger.config.ts` changes required — discovery is side-effect
32102
* based, same mechanism as task/prompt registration.
33103
*/
@@ -71,65 +141,20 @@ export async function bundleSkills(
71141
return { buildManifest, skills: [] };
72142
}
73143

74-
// Destination layout differs between dev and deploy:
75-
// - Dev: the worker runs with cwd = workingDir, so skills must live at
76-
// {workingDir}/.trigger/skills/{id}/ for skill.local() to find them.
77-
// - Deploy: the Dockerfile COPY picks up everything under outputPath into
78-
// /app, so we target {outputPath}/.trigger/skills/{id}/ and the
79-
// container's cwd (/app) resolves correctly.
80-
const destinationRoot =
81-
buildManifest.target === "dev"
82-
? join(workingDir, ".trigger", "skills")
83-
: join(buildManifest.outputPath, ".trigger", "skills");
144+
// Deploy target: the Dockerfile COPY picks up everything under outputPath
145+
// into /app, so we target {outputPath}/.trigger/skills/{id}/ and the
146+
// container's cwd (/app) resolves correctly.
147+
const destinationRoot = join(buildManifest.outputPath, ".trigger", "skills");
84148

85-
for (const skill of skills) {
86-
// Resolve the skill's source folder relative to the file that called
87-
// `skills.define(...)`. Absolute paths are honored as-is.
88-
const callerDir = skill.filePath
89-
? dirname(resolvePath(workingDir, skill.filePath))
90-
: workingDir;
91-
const sourcePath = isAbsolute(skill.sourcePath)
92-
? skill.sourcePath
93-
: resolvePath(callerDir, skill.sourcePath);
94-
const skillMdPath = join(sourcePath, "SKILL.md");
95-
96-
let skillMd: string;
97-
try {
98-
skillMd = await readFile(skillMdPath, "utf8");
99-
} catch {
100-
throw new Error(
101-
`Skill "${skill.id}": SKILL.md not found at ${skillMdPath}. ` +
102-
`Registered via ai.defineSkill({ id: "${skill.id}", path: "${skill.sourcePath}" }) ` +
103-
`at ${skill.filePath}.`
104-
);
105-
}
106-
107-
if (!/^---\r?\n[\s\S]*?\r?\n---/.test(skillMd)) {
108-
throw new Error(
109-
`Skill "${skill.id}": SKILL.md at ${skillMdPath} is missing a frontmatter block.`
110-
);
111-
}
112-
if (!/\bname:\s*\S/.test(skillMd) || !/\bdescription:\s*\S/.test(skillMd)) {
113-
throw new Error(
114-
`Skill "${skill.id}": SKILL.md at ${skillMdPath} frontmatter must include both \`name\` and \`description\`.`
115-
);
116-
}
117-
118-
const skillDest = join(destinationRoot, skill.id);
119-
logger.debug(`[bundleSkills] Copying ${sourcePath} → ${skillDest}`);
120-
await copyDirectoryRecursive(sourcePath, skillDest);
121-
}
122-
123-
// Sort by id for deterministic manifest output
124-
skills = [...skills].sort((a, b) => a.id.localeCompare(b.id));
125-
126-
// Content hash is derived from each SKILL.md's content for cache invalidation
127-
// downstream (dashboard persistence in Phase 2). Not used in Phase 1.
128-
void createHash;
129-
void dirname;
149+
const sortedSkills = await copySkillFolders({
150+
skills,
151+
destinationRoot,
152+
workingDir,
153+
logger,
154+
});
130155

131156
return {
132-
buildManifest: { ...buildManifest, skills },
133-
skills,
157+
buildManifest: { ...buildManifest, skills: sortedSkills },
158+
skills: sortedSkills,
134159
};
135160
}

‎packages/cli-v3/src/dev/devSession.ts‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
logBuildFailure,
1010
logBuildWarnings,
1111
} from "../build/bundle.js";
12-
import { bundleSkills } from "../build/bundleSkills.js";
1312
import {
1413
createBuildContext,
1514
notifyExtensionOnBuildComplete,
@@ -119,25 +118,12 @@ export async function startDevSession({
119118
bundle.metafile
120119
);
121120

122-
// Built-in skill bundling — copies registered skill folders into
123-
// `.trigger/skills/{id}/` so `skill.local()` works at dev runtime.
124-
try {
125-
const buildManifestPath = join(
126-
workerDir?.path ?? destination.path,
127-
"build.json"
128-
);
129-
await writeJSONFile(buildManifestPath, buildManifest);
130-
const skillsResult = await bundleSkills({
131-
buildManifest,
132-
buildManifestPath,
133-
workingDir: rawConfig.workingDir,
134-
env: process.env,
135-
logger: buildContext.logger,
136-
});
137-
buildManifest = skillsResult.buildManifest;
138-
} catch (err) {
139-
logger.warn("Skill bundling failed during dev rebuild", err);
140-
}
121+
// Skill folder copying happens after the main worker indexer runs in
122+
// `BackgroundWorker.initialize` — that pass already discovers skills
123+
// via the resource catalog and reports them on `workerManifest.skills`,
124+
// so we don't need a duplicate indexer here (which historically ran
125+
// with a bare `process.env` and silently dropped skills on projects
126+
// whose task files read CLI-injected vars at module top level).
141127

142128
buildManifest = await notifyExtensionOnBuildComplete(buildContext, buildManifest);
143129

‎packages/cli-v3/src/dev/devSupervisor.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { eventBus } from "../utilities/eventBus.js";
1818
import { logger } from "../utilities/logger.js";
1919
import { resolveSourceFiles } from "../utilities/sourceFiles.js";
2020
import { BackgroundWorker } from "./backgroundWorker.js";
21+
import { copySkillFolders } from "../build/bundleSkills.js";
2122
import { WorkerRuntime } from "./workerRuntime.js";
2223
import { chalkTask, cliLink, prettyError } from "../utilities/cliOutput.js";
2324
import { DevRunController } from "../entryPoints/dev-run-controller.js";
@@ -331,6 +332,25 @@ class DevSupervisor implements WorkerRuntime {
331332
throw new Error("Could not initialize worker");
332333
}
333334

335+
// Copy registered skill folders into `${workingDir}/.trigger/skills/{id}/`
336+
// so `skill.local()` can read them at runtime. The main indexer already
337+
// discovered skills; we just do the file IO here.
338+
const discoveredSkills = backgroundWorker.manifest.skills ?? [];
339+
if (discoveredSkills.length > 0) {
340+
try {
341+
await copySkillFolders({
342+
skills: discoveredSkills,
343+
destinationRoot: join(this.options.config.workingDir, ".trigger", "skills"),
344+
workingDir: this.options.config.workingDir,
345+
logger,
346+
});
347+
} catch (err) {
348+
prettyError("Skill bundling failed", (err as Error).message);
349+
stop();
350+
return;
351+
}
352+
}
353+
334354
const validationIssue = validateWorkerManifest(backgroundWorker.manifest);
335355

336356
if (validationIssue) {

0 commit comments

Comments
 (0)