feat(harness): return failed output checks to the Agent

packages/harness/README.md

@@ -48,11 +48,14 @@ Both `Ok` and `Err` carry a `report`, so a failed run still reports its cost.
 A token count is `undefined` when the adapter does not report it.
 If the run fails before the Agent starts, `steps` is 0.
 `report.warnings` names each source file the Harness left out for size, each `onEvent` failure, and each cleanup problem.
-An `InvalidSkill` error lists each failed output check in `issues`.
+If the output fails a check, the Harness sends the issues back to the Agent in the same session, up to two times.
+Each repair turn adds its steps and usage to the report, and a warning names the issues.
+An `InvalidSkill` error lists each check that still fails in `issues`.
 
 The Harness checks the rules that the generation Skills state:
 
 - The frontmatter contains only `name` and `description`.
+- The description is one plain YAML line without double quotes, backticks, or `%`.
 - `SKILL.md` links every file under `references/`, by a Markdown link or an inline code path.
 - A package Skill keeps `SKILL.md` under 500 lines and writes at most eight reference files.
 - A project Skill points only at project files and gives at least one search command.

packages/harness/src/harness.ts

@@ -1,5 +1,5 @@
 import type { HarnessV1NetworkSandboxSession, HarnessV1Skill } from '@ai-sdk/harness'
-import type { SandboxSession } from './internal/output/collect.ts'
+import type { CollectedFile, SandboxSession } from './internal/output/collect.ts'
 import type { Result } from './internal/result.ts'
 import type { PreparedSource } from './internal/source/host.ts'
 import type { FetchClient } from './internal/source/npm.ts'
@@ -125,8 +125,23 @@ function toUsage(usage: AgentUsage): SkillRunUsage {
   }
 }
 
-function cancelled(): RunOutcome {
-  return err({ _tag: 'Cancelled', message: 'Skill run was cancelled.' })
+/** An unknown count in any turn keeps the total unknown. */
+function sumUsage(a: SkillRunUsage, b: SkillRunUsage): SkillRunUsage {
+  const sum = (x: number | undefined, y: number | undefined) => x === undefined || y === undefined ? undefined : x + y
+  return { inputTokens: sum(a.inputTokens, b.inputTokens), cachedInputTokens: sum(a.cachedInputTokens, b.cachedInputTokens), outputTokens: sum(a.outputTokens, b.outputTokens) }
+}
+
+/** Turns the Agent gets to fix output that failed the deterministic checks, in the same session. */
+const OUTPUT_REPAIR_TURNS = 2
+
+function repairRequest(outputPath: string, issues: ReadonlyArray<string>): string {
+  return [
+    `The Harness checked the output at \`${outputPath}\` and found these problems:`,
+    '',
+    ...issues.map(issue => `- ${issue}`),
+    '',
+    `Fix each problem in place. Write no files outside \`${outputPath}\`. Finish when the output passes these checks.`,
+  ].join('\n')
 }
 
 async function prepareCurrentSkill(destination: SkillDestination, policy: SkillOutputPolicy, signal?: AbortSignal): Promise<Result<PreparedSource | undefined, SkillRunError>> {
@@ -263,10 +278,14 @@ async function writePreparedSource(active: ActiveSandbox, prepared: PreparedRun,
   })
 }
 
-function toAgentError(cause: unknown, signal?: AbortSignal): RunOutcome {
+function agentError(cause: unknown, signal?: AbortSignal): SkillRunError {
   return signal?.aborted
-    ? cancelled()
-    : err({ _tag: 'AgentFailed', message: 'Harness Agent failed during the Skill run.', cause })
+    ? { _tag: 'Cancelled', message: 'Skill run was cancelled.' }
+    : { _tag: 'AgentFailed', message: 'Harness Agent failed during the Skill run.', cause }
+}
+
+function toAgentError(cause: unknown, signal?: AbortSignal): RunOutcome {
+  return err(agentError(cause, signal))
 }
 
 export function createSkillHarness(options: CreateSkillHarnessOptions): SkillHarness {
@@ -352,44 +371,70 @@ export function createSkillHarness(options: CreateSkillHarnessOptions): SkillHar
       const currentSkillPath = posix.join(active.workDir, 'input/current-skill')
       const outputPath = posix.join(active.workDir, 'skilld-output', prepared.outputName)
       const prompt = renderRequest(requestContent(skill), sourcePath, currentSkillPath, outputPath, prepared.outputName)
+      const outputSandbox = active.sandbox
+      // Steps count across turns, so a repair turn continues the numbering.
       let step = 0
-      const generated = await agent.generate({
-        session,
-        prompt,
-        abortSignal: signal,
-        onStepStart: (event) => {
-          step = event.stepNumber
-          log.emit({ _tag: 'StepStart', step })
-        },
-        onToolExecutionStart: ({ toolCall }) => {
-          log.emit({ _tag: 'ToolCall', step, toolName: toolCall.toolName, toolCallId: toolCall.toolCallId, input: toolCall.input })
-        },
-        onStepEnd: (result) => {
-          log.emit({ _tag: 'StepFinish', step: result.stepNumber, finishReason: result.finishReason, usage: toUsage(result.usage) })
-        },
-      }).then(ok, cause => err(cause))
-      if (generated._tag === 'Err')
-        return toAgentError(generated.error, signal)
-      log.settle(toUsage(generated.value.totalUsage), generated.value.steps.length)
-      if (signal?.aborted)
-        return cancelled()
+      let stepOffset = 0
+      let usage: SkillRunUsage | undefined
+      const turn = async (text: string): Promise<Result<void, SkillRunError>> => {
+        const generated = await agent.generate({
+          session,
+          prompt: text,
+          abortSignal: signal,
+          onStepStart: (event) => {
+            step = stepOffset + event.stepNumber
+            log.emit({ _tag: 'StepStart', step })
+          },
+          onToolExecutionStart: ({ toolCall }) => {
+            log.emit({ _tag: 'ToolCall', step, toolName: toolCall.toolName, toolCallId: toolCall.toolCallId, input: toolCall.input })
+          },
+          onStepEnd: (result) => {
+            log.emit({ _tag: 'StepFinish', step: stepOffset + result.stepNumber, finishReason: result.finishReason, usage: toUsage(result.usage) })
+          },
+        }).then(ok, cause => err(cause))
+        if (generated._tag === 'Err')
+          return err(agentError(generated.error, signal))
+        const turnUsage = toUsage(generated.value.totalUsage)
+        usage = usage ? sumUsage(usage, turnUsage) : turnUsage
+        stepOffset += generated.value.steps.length
+        log.settle(usage, stepOffset)
+        return signal?.aborted ? err({ _tag: 'Cancelled', message: 'Skill run was cancelled.' }) : ok(undefined)
+      }
+      const check = async (): Promise<Result<ReadonlyArray<CollectedFile>, SkillRunError>> => {
+        const collected = await collectSandboxOutput(outputSandbox, outputPath, policy, signal)
+        if (collected._tag === 'Err')
+          return collected
+        const validated = prepared.skillName === 'review-skill'
+          ? validateSkillReview(collected.value)
+          : validateGeneratedSkill(
+              prepared.outputName,
+              collected.value,
+              prepared.skillName === 'generate-project-skill'
+                ? { _tag: 'ProjectSkill', projectPaths: prepared.source.files.map(file => file.path) }
+                : { _tag: 'PackageSkill' },
+            )
+        return validated._tag === 'Err' ? validated : ok(collected.value)
+      }
 
-      const collected = await collectSandboxOutput(active.sandbox, outputPath, policy, signal)
-      if (collected._tag === 'Err')
-        return collected
+      const first = await turn(prompt)
+      if (first._tag === 'Err')
+        return first
+      let checked = await check()
+      // A failed check goes back to the Agent, which still holds its context, instead of failing the run.
+      for (let repair = 1; repair <= OUTPUT_REPAIR_TURNS && checked._tag === 'Err' && checked.error._tag === 'InvalidSkill'; repair++) {
+        log.warn(`Output checks failed; repair turn ${repair} of ${OUTPUT_REPAIR_TURNS}: ${checked.error.issues.join(' ')}`)
+        const repaired = await turn(repairRequest(outputPath, checked.error.issues))
+        if (repaired._tag === 'Err')
+          return repaired
+        checked = await check()
+      }
+      if (checked._tag === 'Err')
+        return checked
+      const collected = checked
 
       if (prepared.skillName === 'review-skill')
         return validateSkillReview(collected.value)
 
-      const validated = validateGeneratedSkill(
-        prepared.outputName,
-        collected.value,
-        prepared.skillName === 'generate-project-skill'
-          ? { _tag: 'ProjectSkill', projectPaths: prepared.source.files.map(file => file.path) }
-          : { _tag: 'PackageSkill' },
-      )
-      if (validated._tag === 'Err')
-        return validated
       if (!prepared.destination)
         return err({ _tag: 'InvalidInput', message: 'Skill destination is required.' })
       const promoted = await promoteSkill(

packages/harness/src/internal/output/validate.ts

@@ -1,7 +1,7 @@
 import type { SkillReview, SkillReviewFinding, SkillRunError } from '../../types.ts'
 import type { Result } from '../result.ts'
 import type { CollectedFile } from './collect.ts'
-import { parseDocument } from 'yaml'
+import { isScalar, parseDocument, Scalar } from 'yaml'
 import { isSkillName, normalizeOutputPath } from '../paths.ts'
 import { checkProjectSkill } from './project.ts'
 import { err, ok } from '../result.ts'
@@ -15,6 +15,12 @@ const packageSkillLineLimit = 500
 /** `generate-package-skill`: "Write at most eight reference files." */
 const packageSkillReferenceLimit = 8
 
+/**
+ * Skill loaders and viewers parse frontmatter with different YAML parsers.
+ * A plain one-line description without these characters reads the same in all of them.
+ */
+const riskyDescription = /["`%]/
+
 /**
  * The Skill that produced the output decides which of its written rules apply.
  * A project Skill also carries the project paths its pointers must resolve to.
@@ -125,6 +131,9 @@ export const validateGeneratedSkill = (
     issues.push('Frontmatter name is invalid.')
   if (typeof frontmatter.description !== 'string' || frontmatter.description.trim().length === 0 || frontmatter.description.length > 1024)
     issues.push('Frontmatter description must contain 1 to 1024 characters.')
+  const description = document.get('description', true)
+  if (typeof frontmatter.description === 'string' && (!isScalar(description) || description.type !== Scalar.PLAIN || riskyDescription.test(frontmatter.description)))
+    issues.push('Frontmatter description must be one plain line without double quotes, backticks, or %. Name symptoms in plain words.')
   issues.push(...structureIssues(source, files, contract))
 
   return issues.length === 0 ? ok(undefined) : invalid(issues)

packages/harness/test/e2e/harness.test.ts

@@ -1,4 +1,4 @@
-import { mkdir, mkdtemp, readFile, symlink, writeFile } from 'node:fs/promises'
+import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { createSkillHarness } from '../../src/index.ts'
@@ -237,6 +237,8 @@ describe('createSkillHarness', () => {
       async onPrompt({ workDir }) {
         const outputDir = join(workDir, 'skilld-output/example-project')
         await mkdir(outputDir, { recursive: true })
+        // Every repair turn links the file again.
+        await rm(join(outputDir, 'SKILL.md'), { force: true })
         await symlink(outside, join(outputDir, 'SKILL.md'))
       },
     })

packages/harness/test/e2e/output-checks.test.ts

@@ -58,6 +58,49 @@ function linking(paths: ReadonlyArray<string>): string {
   return paths.map(path => `- [${path}](${path})`).join('\n')
 }
 
+const promptText = (prompt: unknown): string => typeof prompt === 'string' ? prompt : JSON.stringify(prompt)
+
+/** Runs a package Skill whose Agent writes `outputs[n]` on its nth turn, repeating the last one. */
+async function runTurns(outputs: ReadonlyArray<Output>) {
+  const packageDir = await mkdtemp(join(tmpdir(), 'skilld-checks-'))
+  await writeFile(join(packageDir, 'package.json'), '{"name":"example-package"}\n')
+  const destinationRoot = await mkdtemp(join(tmpdir(), 'skilld-checks-out-'))
+  let turn = 0
+  const fake = createFakeHarness({
+    async onPrompt({ sandbox, workDir }) {
+      const output = outputs[Math.min(turn++, outputs.length - 1)]!
+      for (const [path, content] of Object.entries(output))
+        await sandbox.writeTextFile({ path: join(workDir, 'skilld-output/example-package', path), content })
+    },
+  })
+  const result = await createSkillHarness({ harness: fake.harness, sandbox: createFakeSandboxProvider() }).run({
+    _tag: 'PackageSkill',
+    source: { _tag: 'LocalPackage', rootDir: packageDir, packageDir: '.' },
+    destination: { rootDir: destinationRoot, name: 'example-package' },
+  })
+  return { result, prompts: fake.capture.prompts.map(prompt => promptText(prompt.prompt)) }
+}
+
+describe('output check feedback', () => {
+  const invalid = { 'SKILL.md': `${frontmatter('example-package', 'license: MIT\n')}# Example\n` }
+  const valid = { 'SKILL.md': `${frontmatter('example-package')}# Example\n` }
+
+  it('sends failed checks back to the Agent in the same session and promotes the repaired Skill', async () => {
+    const { result, prompts } = await runTurns([invalid, valid])
+
+    expect(result).toMatchObject({ _tag: 'Ok', value: { _tag: 'GeneratedSkill' } })
+    expect(prompts).toHaveLength(2)
+    expect(prompts[1]).toContain('Frontmatter field is not supported: license')
+  })
+
+  it('returns the remaining issues after two repair turns', async () => {
+    const { result, prompts } = await runTurns([invalid])
+
+    expect(issues(result)).toContain('Frontmatter field is not supported: license')
+    expect(prompts).toHaveLength(3)
+  })
+})
+
 describe('generated Skill output checks', () => {
   it('promotes a package Skill that links eight references', async () => {
     const files = references(8)
@@ -115,6 +158,17 @@ describe('generated Skill output checks', () => {
     expect(result).toMatchObject({ _tag: 'Ok' })
   })
 
+  it.each([
+    ['Use it when a task reports "Nuxt not detected".', 'quotes'],
+    ['Use it when %siteName prints in the title.', 'percent'],
+    ['Use it when `useHead` throws.', 'backtick'],
+    ['>\n  Use it for head tags.', 'block'],
+  ])('rejects a description a Skill loader can misread (%s)', async (description) => {
+    const result = await runPackageSkill({ 'SKILL.md': `---\nname: example-package\ndescription: ${description}\n---\n\n# Example\n` })
+
+    expect(issues(result)).toContain('Frontmatter description must be one plain line without double quotes, backticks, or %. Name symptoms in plain words.')
+  })
+
   it('applies the frontmatter and link rules to a project Skill', async () => {
     const result = await runProjectSkill({
       'SKILL.md': `${frontmatter('example-project', 'license: MIT\n')}${projectSkillBody()}`,

packages/harness/test/e2e/run-events.test.ts

@@ -159,7 +159,7 @@ describe('failed skill run report', () => {
     })
   })
 
-  it('reports usage and steps when the output check fails', async () => {
+  it('sums usage and steps across repair turns when the output check still fails', async () => {
     const { harness } = createFakeHarness({
       async onPrompt({ sandbox, workDir }) {
         await sandbox.writeTextFile({
@@ -179,7 +179,14 @@ describe('failed skill run report', () => {
     expect(result).toMatchObject({
       _tag: 'Err',
       error: { _tag: 'InvalidSkill' },
-      report: { steps: 1, usage: { outputTokens: 30 }, warnings: [] },
+      report: {
+        steps: 3,
+        usage: { outputTokens: 90 },
+        warnings: [
+          'Output checks failed; repair turn 1 of 2: Frontmatter name must match the Skill directory name.',
+          'Output checks failed; repair turn 2 of 2: Frontmatter name must match the Skill directory name.',
+        ],
+      },
     })
   })
 

packages/harness/test/unit/extract-blocks.test.ts

@@ -0,0 +1,75 @@
+import { spawnSync } from 'node:child_process'
+import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import { join, resolve } from 'node:path'
+
+const script = resolve(import.meta.dirname, '../../../../skills/generate-package-skill/scripts/extract-blocks.mjs')
+
+const skill = [
+  '# Example',
+  '',
+  '```ts',
+  '// server/api/search.ts',
+  'export default defineEventHandler(() => fetch(\'https://example.com/api\'))',
+  '```',
+  '',
+  '````md',
+  '```ts',
+  'nested',
+  '```',
+  '````',
+  '',
+  '```vue',
+  '<!-- app/pages/index.vue -->',
+  '<template><p>Hi</p></template>',
+  '```',
+  '',
+  '```sh',
+  'pnpm add retriv',
+  '```',
+  '',
+  '```ts',
+  '// ../outside.ts',
+  'export {}',
+  '```',
+  '',
+].join('\n')
+
+describe('extract-blocks script', () => {
+  let dir: string
+
+  beforeEach(async () => {
+    dir = await mkdtemp(join(tmpdir(), 'extract-blocks-'))
+    await writeFile(join(dir, 'SKILL.md'), skill)
+  })
+
+  afterEach(async () => {
+    await rm(dir, { recursive: true, force: true })
+  })
+
+  it('writes each block to its own directory under the path its first line names', async () => {
+    const out = join(dir, 'blocks')
+    const run = spawnSync(process.execPath, [script, join(dir, 'SKILL.md'), out, '--replace', 'https://example.com=http://localhost:4000'], { encoding: 'utf8' })
+
+    expect(await readFile(join(out, '1/server/api/search.ts'), 'utf8')).toBe('// server/api/search.ts\nexport default defineEventHandler(() => fetch(\'http://localhost:4000/api\'))\n')
+    expect(await readFile(join(out, '2/block.md'), 'utf8')).toBe('```ts\nnested\n```\n')
+    expect(await readFile(join(out, '3/app/pages/index.vue'), 'utf8')).toContain('<template>')
+    expect(await readFile(join(out, '4/block.sh'), 'utf8')).toBe('pnpm add retriv\n')
+    const manifest = JSON.parse(await readFile(join(out, 'blocks.json'), 'utf8'))
+    expect(manifest.map((entry: { block: number, line: number, file: string }) => [entry.block, entry.line, entry.file])).toEqual([
+      [1, 4, '1/server/api/search.ts'],
+      [2, 9, '2/block.md'],
+      [3, 15, '3/app/pages/index.vue'],
+      [4, 20, '4/block.sh'],
+    ])
+    expect(run.status).toBe(1)
+    expect(run.stderr).toContain('block 5 line 24: path ../outside.ts leaves the output directory; skipped')
+  })
+
+  it('refuses a replacement without a target', () => {
+    const run = spawnSync(process.execPath, [script, join(dir, 'SKILL.md'), join(dir, 'out'), '--replace', 'https://example.com'], { encoding: 'utf8' })
+
+    expect(run.status).toBe(2)
+    expect(run.stderr).toContain('--replace needs FROM=TO.')
+  })
+})