Skip to content

Commit 4f2009b

Browse files
committed
harden validation and Yarn PnP errors
1 parent f2f7104 commit 4f2009b

3 files changed

Lines changed: 192 additions & 20 deletions

File tree

packages/intent/src/commands/validate.ts

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { appendFileSync, existsSync, readFileSync } from 'node:fs'
2-
import { join, relative, resolve, sep } from 'node:path'
2+
import { basename, dirname, join, relative, resolve, sep } from 'node:path'
33
import { fail, isCliFailure } from '../cli-error.js'
44
import { printWarnings } from '../cli-support.js'
55
import {
@@ -13,10 +13,17 @@ interface ValidationError {
1313
message: string
1414
}
1515

16+
interface ValidationWarning {
17+
file: string
18+
message: string
19+
}
20+
1621
export interface ValidateCommandOptions {
1722
githubSummary?: boolean
1823
}
1924

25+
const agentSkillNamePattern = /^[a-z0-9]+(?:-[a-z0-9]+)*$/
26+
2027
function buildValidationFailure(
2128
errors: Array<ValidationError>,
2229
warnings: Array<string>,
@@ -83,6 +90,116 @@ function collectPackagingWarnings(context: ProjectContext): Array<string> {
8390
return warnings
8491
}
8592

93+
function formatWarning({ file, message }: ValidationWarning): string {
94+
return `${file}: ${message}`
95+
}
96+
97+
function isRecord(value: unknown): value is Record<string, unknown> {
98+
return !!value && typeof value === 'object' && !Array.isArray(value)
99+
}
100+
101+
function collectAgentSkillSpecWarnings({
102+
filePath,
103+
fm,
104+
rel,
105+
}: {
106+
filePath: string
107+
fm: Record<string, unknown>
108+
rel: string
109+
}): Array<ValidationWarning> {
110+
const warnings: Array<ValidationWarning> = []
111+
112+
if (typeof fm.name === 'string') {
113+
if (fm.name.length > 64) {
114+
warnings.push({
115+
file: rel,
116+
message: `Agent Skills spec warning: name exceeds 64 characters (${fm.name.length} chars)`,
117+
})
118+
}
119+
120+
for (const segment of fm.name.split('/')) {
121+
if (!agentSkillNamePattern.test(segment)) {
122+
warnings.push({
123+
file: rel,
124+
message:
125+
'Agent Skills spec warning: each name segment should use lowercase letters, numbers, and single hyphens only',
126+
})
127+
break
128+
}
129+
}
130+
131+
const parentDir = basename(dirname(filePath))
132+
if (!fm.name.includes('/') && fm.name !== parentDir) {
133+
warnings.push({
134+
file: rel,
135+
message:
136+
'Agent Skills spec warning: name should match the parent directory name',
137+
})
138+
}
139+
}
140+
141+
if (
142+
fm.license !== undefined &&
143+
(typeof fm.license !== 'string' || fm.license.trim().length === 0)
144+
) {
145+
warnings.push({
146+
file: rel,
147+
message: 'Agent Skills spec warning: license should be a non-empty string',
148+
})
149+
}
150+
151+
if (fm.compatibility !== undefined) {
152+
if (
153+
typeof fm.compatibility !== 'string' ||
154+
fm.compatibility.trim().length === 0
155+
) {
156+
warnings.push({
157+
file: rel,
158+
message:
159+
'Agent Skills spec warning: compatibility should be a non-empty string',
160+
})
161+
} else if (fm.compatibility.length > 500) {
162+
warnings.push({
163+
file: rel,
164+
message: `Agent Skills spec warning: compatibility exceeds 500 characters (${fm.compatibility.length} chars)`,
165+
})
166+
}
167+
}
168+
169+
if (fm.metadata !== undefined) {
170+
if (!isRecord(fm.metadata)) {
171+
warnings.push({
172+
file: rel,
173+
message: 'Agent Skills spec warning: metadata should be a mapping',
174+
})
175+
} else {
176+
const hasNonStringValue = Object.values(fm.metadata).some(
177+
(value) => typeof value !== 'string',
178+
)
179+
if (hasNonStringValue) {
180+
warnings.push({
181+
file: rel,
182+
message:
183+
'Agent Skills spec warning: metadata values should be strings',
184+
})
185+
}
186+
}
187+
}
188+
189+
if (
190+
fm['allowed-tools'] !== undefined &&
191+
typeof fm['allowed-tools'] !== 'string'
192+
) {
193+
warnings.push({
194+
file: rel,
195+
message:
196+
'Agent Skills spec warning: allowed-tools should be a space-separated string',
197+
})
198+
}
199+
200+
return warnings
201+
}
202+
86203
export async function runValidateCommand(
87204
dir?: string,
88205
options: ValidateCommandOptions = {},
@@ -206,6 +323,12 @@ async function runValidateCommandInternal(dir?: string): Promise<void> {
206323
})
207324
}
208325

326+
warnings.push(
327+
...collectAgentSkillSpecWarnings({ filePath, fm, rel }).map(
328+
formatWarning,
329+
),
330+
)
331+
209332
const lineCount = content.split(/\r?\n/).length
210333
if (lineCount > 500) {
211334
errors.push({

packages/intent/src/scanner.ts

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -100,26 +100,33 @@ function loadPnpApi(root: string): PnpApi | null {
100100
const pnpPath = findPnpFile(root)
101101
if (!pnpPath) return null
102102

103-
const moduleApi = requireFromHere('node:module') as {
104-
findPnpApi?: (lookupSource: string) => PnpApi | null
105-
}
106-
const foundApi = moduleApi.findPnpApi?.(root)
107-
if (foundApi) return foundApi
103+
try {
104+
const moduleApi = requireFromHere('node:module') as {
105+
findPnpApi?: (lookupSource: string) => PnpApi | null
106+
}
107+
const foundApi = moduleApi.findPnpApi?.(root)
108+
if (foundApi) return foundApi
108109

109-
const pnpModule = requireFromHere(pnpPath) as PnpApi
110-
if (typeof pnpModule.setup === 'function') {
111-
pnpModule.setup()
112-
}
110+
const pnpModule = requireFromHere(pnpPath) as PnpApi
111+
if (typeof pnpModule.setup === 'function') {
112+
pnpModule.setup()
113+
}
113114

114-
if (
115-
typeof pnpModule.getDependencyTreeRoots === 'function' &&
116-
typeof pnpModule.getPackageInformation === 'function'
117-
) {
118-
return pnpModule
119-
}
115+
if (
116+
typeof pnpModule.getDependencyTreeRoots === 'function' &&
117+
typeof pnpModule.getPackageInformation === 'function'
118+
) {
119+
return pnpModule
120+
}
120121

121-
const projectRequire = createRequire(join(dirname(pnpPath), 'package.json'))
122-
return projectRequire('pnpapi') as PnpApi
122+
const projectRequire = createRequire(join(dirname(pnpPath), 'package.json'))
123+
return projectRequire('pnpapi') as PnpApi
124+
} catch (err) {
125+
const msg = err instanceof Error ? err.message : String(err)
126+
throw new Error(
127+
`Yarn PnP project detected, but Intent could not load Yarn's PnP API from ${pnpPath}: ${msg}`,
128+
)
129+
}
123130
}
124131

125132
function getPnpLocatorKey(locator: PnpPackageLocator): string {

packages/intent/tests/cli.test.ts

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,46 @@ describe('cli commands', () => {
11271127
)
11281128
})
11291129

1130+
it('keeps nested Intent skill names valid without Agent Skills spec warnings', async () => {
1131+
const root = mkdtempSync(join(realTmpdir, 'intent-cli-validate-nested-'))
1132+
tempDirs.push(root)
1133+
1134+
writeSkillMd(join(root, 'skills', 'core', 'setup'), {
1135+
name: 'core/setup',
1136+
description: 'Core setup concepts',
1137+
})
1138+
1139+
process.chdir(root)
1140+
1141+
const exitCode = await main(['validate'])
1142+
const output = logSpy.mock.calls.flat().join('\n')
1143+
1144+
expect(exitCode).toBe(0)
1145+
expect(output).toContain('✅ Validated 1 skill files — all passed')
1146+
expect(output).not.toContain('Agent Skills spec warning')
1147+
})
1148+
1149+
it('warns but does not fail for Agent Skills spec-incompatible names', async () => {
1150+
const root = mkdtempSync(join(realTmpdir, 'intent-cli-validate-spec-'))
1151+
tempDirs.push(root)
1152+
1153+
writeSkillMd(join(root, 'skills', 'PDF-Processing'), {
1154+
name: 'PDF-Processing',
1155+
description: 'PDF processing concepts',
1156+
})
1157+
1158+
process.chdir(root)
1159+
1160+
const exitCode = await main(['validate'])
1161+
const output = logSpy.mock.calls.flat().join('\n')
1162+
1163+
expect(exitCode).toBe(0)
1164+
expect(output).toContain('✅ Validated 1 skill files — all passed')
1165+
expect(output).toContain(
1166+
'Agent Skills spec warning: each name segment should use lowercase letters, numbers, and single hyphens only',
1167+
)
1168+
})
1169+
11301170
it('validates package skills from repo root without root packaging warnings', async () => {
11311171
const root = mkdtempSync(join(realTmpdir, 'intent-cli-validate-mono-'))
11321172
tempDirs.push(root)
@@ -1361,7 +1401,7 @@ describe('cli commands', () => {
13611401
}
13621402
})
13631403

1364-
it('fails cleanly for unsupported yarn pnp projects', async () => {
1404+
it('fails cleanly when the Yarn PnP API cannot be loaded', async () => {
13651405
const root = mkdtempSync(join(realTmpdir, 'intent-cli-pnp-'))
13661406
tempDirs.push(root)
13671407
writeJson(join(root, 'package.json'), { name: 'app', private: true })
@@ -1372,7 +1412,9 @@ describe('cli commands', () => {
13721412

13731413
expect(exitCode).toBe(1)
13741414
expect(errorSpy).toHaveBeenCalledWith(
1375-
'Installed-package scanning in Yarn PnP projects without node_modules is not yet supported. `intent validate` can still validate workspace skills; for list/load/install, add `nodeLinker: node-modules` to .yarnrc.yml or use --global-only.',
1415+
expect.stringContaining(
1416+
'Yarn PnP project detected, but Intent could not load Yarn',
1417+
),
13761418
)
13771419
})
13781420

0 commit comments

Comments
 (0)