Document automation friction + fix skill submodule resolution

README.md

@@ -125,6 +125,8 @@ pnpm --filter asil-runners auto grind --max-tasks 3
 
 See [`examples/quickstart.md`](examples/quickstart.md) for a five-minute integration walkthrough.
 
+Lessons from running these skills unsupervised (what fought automation, fuzzy boundaries under load) are in [`docs/automation-friction.md`](docs/automation-friction.md).
+
 ## What it looks like
 
 ```

docs/automation-friction.md

@@ -0,0 +1,78 @@
+# Lessons from wiring agent-skills into an autonomous loop
+
+Reply draft for [addyosmani/agent-skills#97](https://github.com/addyosmani/agent-skills/issues/97) — what fought automation, and where boundaries went fuzzy under load. Grounded in production use on an 80+ agent platform and in the ASIL open-source extract.
+
+---
+
+## Draft reply (ready to paste)
+
+@addyosmani Glad the write-up was useful — here is the gold you asked for. After running 14 skills + the 3 personas unsupervised at production scale, these are the places the skills fought the automation or a boundary went fuzzy.
+
+### 1. Personas assumed a human merge step (biggest friction)
+
+`agents/code-reviewer.md`, `security-auditor.md`, and `test-engineer.md` are excellent for human-in-the-loop review. Unattended, they over-reject.
+
+- The code reviewer audited the *file*, not the *diff* — pre-existing issues in unchanged lines became blockers.
+- The security auditor treated dead-code / unused-export cleanups as attack-surface changes.
+- The test engineer demanded new tests for pure deletions and visibility tweaks.
+
+**Fix that stuck:** a hard scoping preamble on every persona prompt — "review the DIFF only; pre-existing issues are out of scope; calibrate scrutiny to change risk." Same preamble on the adversarial (Codex) gate. Without it, trivial tasks never cleared the gate.
+
+### 2. Section title mismatch silently dropped the anti-rationalization tables
+
+Your skills title the table **Common Rationalizations**. Our executor originally looked for **Anti-Rationalization Table**. Under automation that meant the tables — the piece we called out as most important — were never injected into the enforcement block. Agents fell through to a generic one-liner fallback.
+
+**Fix:** accept both headers. (Shipped in this repo's skill loader.)
+
+### 3. Skill path / name layout was not automation-friendly
+
+Submodule layout is `skills/<name>/SKILL.md`. Early ASIL expected `skills/<name>.md`, and several category/thinker names did not match upstream folders 1:1 (`security-review` vs `security-and-hardening`, `testing-strategy` / `test-coverage-improvement` vs `test-driven-development`, etc.). Missing files failed open into inline fallbacks — so the loop "worked" while ignoring the submodule.
+
+**Fix:** resolve flat file → `SKILL.md` → alias chain, in that order.
+
+### 4. Skills written for feature work fought cleanup categories
+
+| Skill / persona | Fought when… |
+|---|---|
+| `test-driven-development` | Applied to dead-code / simplification — insisted on RED-GREEN for no behavior change |
+| `security-and-hardening` | Applied to one-line visibility changes — invented auth concerns |
+| `code-simplification` | Fine alone; combined with TDD persona → thrash on "simplify but also add tests" |
+| `incremental-implementation` | Wanted thin slices; the scanner queued whole-file TODO clusters |
+
+**Pattern:** skills encode *how a human should work a feature*. Autonomous categories are often *mechanical cleanup*. Mapping category → skill needs an explicit "no behavior change → skip TDD ceremony" delta, not just "load the skill."
+
+### 5. Priority queues let one skill monopolize the run
+
+Strict priority (`test-failure` → … → `documentation`) meant a noisy test suite starved security / type-error work for an entire grind. Round-robin across categories fixed starvation without abandoning severity ordering inside a category.
+
+### 6. Scanner false-positives became skill false-confidence
+
+Dead-code skill + shallow "no in-repo references" detection = the loop confidently proposed deleting public API surface from package entry points. The skill did its job; the *task* was wrong. We had to exempt entry points and put honest uncertainty in the task description before the skill ran.
+
+### 7. What held up under load
+
+- **Common Rationalizations / Red Flags** — once actually injected, they were the highest-leverage content. Unsupervised agents rationalize exactly the rows in those tables.
+- **Fail-closed JSON** from personas and the adversarial gate (unparseable → reject).
+- **Cost checkpoints around the review fan-out** — three personas + Codex after a fat executor call is where budgets die; force-check before the fan-out.
+- **Domain-question markers** — the cleanest "skill stops here" boundary we found. Skills should say when to emit one instead of inventing domain logic.
+
+Happy to go deeper on any of these. The reference implementation is [TelivityAI/asil](https://github.com/TelivityAI/asil); the skill-loader / rationalization fixes from this write-up live there too.
+
+---
+
+## Mapping: ASIL names → upstream skills
+
+| ASIL category / thinker | Primary name | Upstream alias |
+|---|---|---|
+| vulnerability | `security-review` | `security-and-hardening` |
+| test-failure / coverage-gap | `test-coverage-improvement` | `test-driven-development` |
+| test-strategist thinker | `test-driven-development` | (`testing-strategy` still aliases) |
+| security thinker | `security-and-hardening` | (`security-review` still aliases) |
+| dead-code | `dead-code-removal` | `code-simplification` |
+| dependency-update | `dependency-update` | `deprecation-and-migration` |
+| documentation | `documentation-generation` | `documentation-and-adrs` |
+| todo-resolution | `todo-resolution` | `incremental-implementation`, `debugging-and-error-recovery` |
+| type-error / complexity | `code-simplification` | (exact match) |
+| planner / spec-writer | `planning-and-task-breakdown` / `spec-driven-development` | (exact match) |
+
+Resolution order for every name: `skills/<name>.md` → `skills/<name>/SKILL.md` → same for each alias.

examples/quickstart.md

@@ -39,13 +39,22 @@ REPO_ROOT=/absolute/path/to/your/repo
 
 ## 3. Drop in the runner skills
 
-ASIL's thinkers expect Markdown skill files at `<REPO_ROOT>/.asil/skills/`. The simplest setup:
+ASIL's thinkers and executor load Markdown skills from `ASIL_SKILLS_PATH` (default `<REPO_ROOT>/.asil/skills`). Two layouts are supported:
+
+```bash
+# A) Flat files (early ASIL installs / hand-copied skills)
+.asil/skills/skills/planning-and-task-breakdown.md
+
+# B) Upstream agent-skills layout (git submodule or clone)
+.asil/skills/skills/planning-and-task-breakdown/SKILL.md
+```
 
 ```bash
 mkdir -p .asil/skills
-# Copy or symlink any Markdown skills (e.g. from a public skill library) you want to use.
-# A minimal install is fine — the thinkers fall back to inline defaults
-# if a skill file is missing.
+# Option: submodule an upstream skill library, then point ASIL_SKILLS_PATH at it.
+# Historical ASIL names (security-review, testing-strategy, …) alias onto
+# upstream folder names (security-and-hardening, test-driven-development, …).
+# Thinkers fall back to inline defaults if a skill file is missing.
 ```
 
 Override the location with `ASIL_SKILLS_PATH` if you want them somewhere else.

packages/asil-improvement-loop/src/__tests__/executor.test.ts

@@ -456,6 +456,23 @@ describe('executor', () => {
     it('returns empty string when the section is missing', () => {
       expect(extractAntiRationalization('# Just a skill\n\nNo table here.')).toBe('');
     });
+
+    it('also extracts upstream "Common Rationalizations" sections', () => {
+      const skill = [
+        '# Skill',
+        '',
+        '## Common Rationalizations',
+        '| Rationalization | Reality |',
+        '| Tests later | Write them first |',
+        '',
+        '## Red Flags',
+        '- x',
+      ].join('\n');
+      const section = extractAntiRationalization(skill);
+      expect(section).toMatch(/Common Rationalizations/);
+      expect(section).toMatch(/Tests later/);
+      expect(section).not.toMatch(/Red Flags/);
+    });
   });
 
   describe('buildExecutionPrompt', () => {

packages/asil-improvement-loop/src/__tests__/skill-loader.test.ts

@@ -0,0 +1,93 @@
+import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { describe, expect, it } from 'vitest';
+import {
+  extractAntiRationalization,
+  loadSkillMarkdown,
+  skillCandidatePaths,
+} from '../skill-loader.js';
+
+describe('skillCandidatePaths', () => {
+  it('lists flat .md then SKILL.md, then aliases', () => {
+    expect(skillCandidatePaths('security-review')).toEqual([
+      join('skills', 'security-review.md'),
+      join('skills', 'security-review', 'SKILL.md'),
+      join('skills', 'security-and-hardening.md'),
+      join('skills', 'security-and-hardening', 'SKILL.md'),
+    ]);
+  });
+});
+
+describe('loadSkillMarkdown', () => {
+  it('loads upstream agent-skills layout via alias', () => {
+    const root = mkdtempSync(join(tmpdir(), 'asil-skills-'));
+    const dir = join(root, 'skills', 'security-and-hardening');
+    mkdirSync(dir, { recursive: true });
+    writeFileSync(join(dir, 'SKILL.md'), '# Security\n\nHardening rules.\n');
+
+    expect(loadSkillMarkdown(root, 'security-review')).toContain('Hardening');
+  });
+
+  it('prefers flat primary name when present', () => {
+    const root = mkdtempSync(join(tmpdir(), 'asil-skills-'));
+    mkdirSync(join(root, 'skills'), { recursive: true });
+    writeFileSync(
+      join(root, 'skills', 'security-review.md'),
+      '# Flat security review\n',
+    );
+    const dir = join(root, 'skills', 'security-and-hardening');
+    mkdirSync(dir, { recursive: true });
+    writeFileSync(join(dir, 'SKILL.md'), '# Upstream\n');
+
+    expect(loadSkillMarkdown(root, 'security-review')).toContain('Flat security');
+  });
+
+  it('returns empty string when nothing matches', () => {
+    expect(loadSkillMarkdown('/nonexistent', 'nope')).toBe('');
+  });
+});
+
+describe('extractAntiRationalization', () => {
+  it('extracts Anti-Rationalization Table sections', () => {
+    const skill = [
+      '# Skill',
+      '',
+      '## Anti-Rationalization Table',
+      '| Rationalization | Reality |',
+      '| --- | --- |',
+      '| Skip tests | Never |',
+      '',
+      '## Red Flags',
+      '- x',
+    ].join('\n');
+    const section = extractAntiRationalization(skill);
+    expect(section).toMatch(/Anti-Rationalization/i);
+    expect(section).toMatch(/Skip tests/);
+    expect(section).not.toMatch(/Red Flags/);
+  });
+
+  it('extracts Common Rationalizations sections from upstream skills', () => {
+    const skill = [
+      '# Test-Driven Development',
+      '',
+      '## Common Rationalizations',
+      '| Rationalization | Reality |',
+      '| --- | --- |',
+      '| Tests later | Tests first |',
+      '',
+      '## Red Flags',
+      '- shipping without tests',
+    ].join('\n');
+    const section = extractAntiRationalization(skill);
+    expect(section).toMatch(/Common Rationalizations/);
+    expect(section).toMatch(/Tests later/);
+    expect(section).not.toMatch(/Red Flags/);
+  });
+
+  it('returns empty string when the section is missing', () => {
+    expect(extractAntiRationalization('# Just a skill\n\nNo table here.')).toBe(
+      '',
+    );
+  });
+});

packages/asil-improvement-loop/src/executor.ts

@@ -9,9 +9,20 @@ import type {
   LLMCaller,
   LLMResponse,
 } from './types.js';
+import {
+  extractAntiRationalization,
+  loadSkillMarkdown,
+} from './skill-loader.js';
 import { CATEGORY_SKILL_MAP } from './types.js';
 import type { CommandRunner } from './scanner.js';
 
+export { extractAntiRationalization } from './skill-loader.js';
+export {
+  loadSkillMarkdown,
+  skillCandidatePaths,
+  SKILL_NAME_ALIASES,
+} from './skill-loader.js';
+
 /**
  * Structured logger contract. Defaults to stderr via `console.error`,
  * but tests (and the CLI) can inject a spy/mock to capture events.
@@ -467,11 +478,7 @@ async function defaultSkillLoader(
   skillsPath: string,
   skillName: string,
 ): Promise<string> {
-  try {
-    return readFileSync(join(skillsPath, 'skills', `${skillName}.md`), 'utf8');
-  } catch {
-    return '';
-  }
+  return loadSkillMarkdown(skillsPath, skillName);
 }
 
 function defaultReadFile(absPath: string): string | null {
@@ -482,27 +489,6 @@ function defaultReadFile(absPath: string): string | null {
   }
 }
 
-/** Extracts the anti-rationalization table — a section commonly titled
- *  "Anti-Rationalization Table" in the configured skills. Returns the skill
- *  content unchanged if we can't locate it. */
-export function extractAntiRationalization(skill: string): string {
-  if (!skill) return '';
-  const lines = skill.split(/\r?\n/);
-  const startIdx = lines.findIndex((l) =>
-    /^#{1,6}\s*anti[- ]?rationalization/i.test(l),
-  );
-  if (startIdx === -1) return '';
-  const afterStart = lines.slice(startIdx);
-  const nextHeaderIdx = afterStart
-    .slice(1)
-    .findIndex((l) => /^#{1,6}\s+/.test(l));
-  const slice =
-    nextHeaderIdx === -1
-      ? afterStart
-      : afterStart.slice(0, nextHeaderIdx + 1);
-  return slice.join('\n').trim();
-}
-
 export function buildExecutionPrompt(
   skillContent: string,
   antiRationalization: string,

packages/asil-improvement-loop/src/index.ts

@@ -51,6 +51,11 @@ export {
   type FileBlock,
   type FileFetcher,
 } from './executor.js';
+export {
+  loadSkillMarkdown,
+  skillCandidatePaths,
+  SKILL_NAME_ALIASES,
+} from './skill-loader.js';
 export { selfReview } from './self-review.js';
 export {
   adversarialReview,

packages/asil-improvement-loop/src/skill-loader.ts

@@ -0,0 +1,83 @@
+/**
+ * Resolve Markdown skill content from either a flat file layout or the
+ * agent-skills directory layout (`skills/<name>/SKILL.md`), with aliases
+ * for historical ASIL names that map onto upstream skill names.
+ */
+import { readFileSync } from 'node:fs';
+import { join } from 'node:path';
+
+/**
+ * When ASIL's category / thinker names don't match upstream skill folder
+ * names 1:1, try these fallbacks in order after the primary name.
+ */
+export const SKILL_NAME_ALIASES: Readonly<Record<string, readonly string[]>> = {
+  'security-review': ['security-and-hardening'],
+  'testing-strategy': ['test-driven-development'],
+  'test-coverage-improvement': ['test-driven-development'],
+  'dead-code-removal': ['code-simplification'],
+  'dependency-update': ['deprecation-and-migration'],
+  'documentation-generation': ['documentation-and-adrs'],
+  'todo-resolution': [
+    'incremental-implementation',
+    'debugging-and-error-recovery',
+  ],
+};
+
+/** Candidate relative paths under `skillsPath` for a single skill name. */
+export function skillCandidatePaths(skillName: string): string[] {
+  const names = [skillName, ...(SKILL_NAME_ALIASES[skillName] ?? [])];
+  const paths: string[] = [];
+  for (const name of names) {
+    // Flat layout used by early ASIL installs and symlinked copies.
+    paths.push(join('skills', `${name}.md`));
+    // Upstream agent-skills submodule / clone layout.
+    paths.push(join('skills', name, 'SKILL.md'));
+  }
+  return paths;
+}
+
+/**
+ * Load skill Markdown from disk. Returns empty string when nothing matches
+ * so callers can fall back to inline defaults.
+ */
+export function loadSkillMarkdown(
+  skillsPath: string,
+  skillName: string,
+): string {
+  for (const rel of skillCandidatePaths(skillName)) {
+    try {
+      return readFileSync(join(skillsPath, rel), 'utf8');
+    } catch {
+      // try next candidate
+    }
+  }
+  return '';
+}
+
+/**
+ * Extracts the anti-/common-rationalization section from a skill.
+ *
+ * Upstream agent-skills title this "## Common Rationalizations"; some
+ * forks use "## Anti-Rationalization Table". Both must be recognized —
+ * otherwise the autonomous executor silently drops the tables that were
+ * the whole point of importing the skills.
+ */
+export function extractAntiRationalization(skill: string): string {
+  if (!skill) return '';
+  const lines = skill.split(/\r?\n/);
+  const startIdx = lines.findIndex((l) =>
+    /^#{1,6}\s*(?:anti[- ]?rationalization|common\s+rationalizations)\b/i.test(
+      l,
+    ),
+  );
+  if (startIdx === -1) return '';
+  const afterStart = lines.slice(startIdx);
+  const nextHeaderIdx = afterStart
+    .slice(1)
+    .findIndex((l) => /^#{1,6}\s+/.test(l));
+  const slice =
+    nextHeaderIdx === -1
+      ? afterStart
+      : afterStart.slice(0, nextHeaderIdx + 1);
+  return slice.join('\n').trim();
+}