mirror of
https://github.com/affaan-m/ECC.git
synced 2026-08-17 21:15:40 +02:00
* feat: add thin Pi adapter mounting ECC's canonical skills and commands Adds first-class Pi (@earendil-works/pi-coding-agent) support as a thin adapter layer, following the maintainer review on #2352. ECC's canonical assets stay the single source of truth: nothing is copied or generated under .pi/. The `pi` manifest in package.json points Pi directly at `skills/` and `commands/`. No transformation is needed — ECC's SKILL.md files already follow the Agent Skills standard Pi implements, and ECC's command frontmatter is already Pi's prompt-template format. .pi/extensions/index.ts is the only adapter logic. It: - uses Pi's documented `pi.on(...)` lifecycle, not an undocumented event bus - resolves hook scripts from the installed package via `__dirname`, never `process.cwd()`, so global installs work from any project directory - runs hooks with `execFile(process.execPath, [...])` and no shell, so paths containing spaces or shell metacharacters are safe - invokes hooks through ECC's own `run-with-flags.js`, so `ECC_HOOK_PROFILE` and `ECC_DISABLED_HOOKS` keep gating hooks under Pi - runs hooks in the user's project directory so project detection stays correct, while resolving the scripts themselves package-relative - injects the SessionStart hook's `additionalContext` into the system prompt on the next `before_agent_start` - isolates hook failures behind a timeout and an output limit - registers `/ecc-doctor` for install diagnostics Registers `.pi` in the platform-configs install module and adds a Pi row to the harness adapter compliance matrix. Verified against Pi 0.84.1: a global `pi install` exposes 285 skills and 94 commands resolved from `skills/` and `commands/`, plus `/ecc-doctor`, with no generated copies. Scope deliberately excludes subagents, chains, approval gates, todos, profiles, and MCP; ECC works in Pi without any companion package. * fix: address review findings on the Pi adapter Bot review on #2759 surfaced two real runtime defects and several hardening gaps. Runtime fixes: - Attach an `error` listener to the hook child's stdin. `stdin.end()` writes asynchronously, so a hook that exits, short-circuits, or is killed by the timeout before reading the payload raises EPIPE as an `error` event that the surrounding try/catch cannot see. Unhandled, that event would terminate the Pi session and break the isolation guarantee the adapter documents. - Clear `pendingContext` at the top of the `session_start` handler. Pi can start a new session (/new, /resume, /fork) before `before_agent_start` consumes the previous value; if the newer hook then failed, the next agent start received context describing a different session's project state. - Replace `require.resolve` companion detection with a read of Pi's own `packages` list, honoring `PI_CODING_AGENT_DIR`. Pi installs packages under its config directory, which is not on Node's module resolution path from the extension, so the previous check reported every companion as missing no matter what was installed. Compliance matrix: remove internal semicolons and a trailing period from the Pi record's list entries. The renderer joins entries with "; ", so those characters split one entry into several in the rendered cell. Tests: run profile gating against the temp skeleton instead of the real checkout so it cannot leave marker artifacts behind; count files under .pi/ by walking disk rather than git, so untracked copies cannot bypass the regression guard; allow negated phrasing in the README heuristic; pin the adapter's real parser guards with source assertions so the local mirrors cannot silently diverge; add coverage for EPIPE isolation, stale context clearing, and companion detection. * docs: point users at existing companion Pi packages instead of bundling them Every capability listed as out of scope is already provided by a maintained community Pi package: pi-subagents, @juicesharp/rpiv-ask-user-question, @juicesharp/rpiv-todo, and pi-mcp-adapter for MCP. Pi supports pulling other pi packages in via dependencies plus bundledDependencies, but this adapter deliberately does not. Bundling would ship third-party code that executes with full user permissions in every ECC install, turn optional capabilities into mandatory ones, and add four fast-moving pins to maintain. Instead /ecc-doctor now prints the exact `pi install npm:<name>` command for each companion it does not find, so adopting one stays a deliberate user choice. Also corrects the MCP claim: Pi core has no MCP surface by design, but the community pi-mcp-adapter package adds one. This adapter neither installs nor verifies it, and ECC's MCP reference configs are not known to be compatible. * docs: ECC's MCP configs work in Pi through pi-mcp-adapter, verbatim Tested rather than assumed. The community pi-mcp-adapter package reads the standard mcpServers format from .mcp.json and ~/.config/mcp/mcp.json, which is exactly the format ECC already uses in .mcp.json and mcp-configs/mcp-servers.json. Verified against pi-mcp-adapter 2.21.2 in an isolated PI_CODING_AGENT_DIR: copying mcp-configs/mcp-servers.json to a project's .mcp.json registers Pi's `mcp` tool and `/mcp` command with all 35 ECC servers discovered, coexisting with this adapter's /ecc-doctor. No translation layer and no ECC change are needed, so this stops being a limitation and becomes documentation. Recorded caveats: the adapter's first run against a new config performs initialization that blocks in non-interactive mode, and only discovery was verified, not live tool invocation. ECC still neither installs nor depends on the package. * feat: inject ECC's canonical engineering rules into Pi's system prompt ECC's rules were the one durable asset the adapter did not deliver: skills and commands reached Pi in full, but the 122 rule files that carry ECC's coding style, testing, security, git workflow, and code-review standards did not, so ECC in Pi was a library of skills rather than a set of enforced standards. Rules are read at runtime from the canonical rules/common/ directory of the installed package and appended to the system prompt inside an <ecc-engineering-rules> block. Nothing is copied or generated under .pi/, which keeps the single-source-of-truth constraint this PR exists to satisfy. Injection reuses the before_agent_start path already built for session context, so no new lifecycle mapping is introduced. Rules are re-applied every turn because they are standing policy, while the session context stays one-shot and is consumed on first use. agents.md, hooks.md, and performance.md are excluded: they describe Claude Code primitives Pi does not have (Task/TodoWrite delegation, Claude hook event types, thinking-budget toggles), so injecting them would point the model at tools that are not there. A test asserts they stay excluded, and a leakage test asserts none of those primitives appear in the injected text. Language-specific rules under rules/<language>/ are out of scope for this first adapter. Injection is bounded by MAX_RULES_BYTES and can be disabled with ECC_PI_RULES, following ECC's existing off-switch convention. /ecc-doctor reports the state and injected size. Measured on this repo: 7 files, 12,361 characters, roughly 3k tokens. Also replaces a Function() call in the test helper with direct arithmetic, and repins a stale assertion that pinned one spelling of the context handoff rather than the guarantee (read before clear, clear before return). * fix: /ecc-doctor misreported filtered packages and partial rule installs Two reporting defects in /ecc-doctor, the command whose whole job is telling a user what is actually installed. Pi's settings accept a `packages` entry in two shapes: the bare source string ("npm:pi-subagents") and an object carrying that source alongside resource filters ({ source: "npm:pi-subagents", skills: [] }). normalizePiPackageName only recognized the string, so a user who narrowed which resources a companion contributes was told the companion was not installed, along with an install command for something already present. The source type still decides whether a name is comparable, so an object wrapping a git source or a path stays unrecognized exactly as before. loadPortableRules drops rule files it cannot read, drops empty ones, and stops at MAX_RULES_BYTES, but describeRulesStatus reported PORTABLE_RULE_FILES.length regardless. A partial install that loaded 3 of 7 files reported "7 rule file(s)" to the one command a user runs to find a partial install. The loaded count is now tracked next to the cache and reported as a ratio, with the shortfall named. Also reconciles the Notes bullet in .pi/README.md, which still called MCP out of scope after the MCP section landed documenting that ECC's configs load in Pi through pi-mcp-adapter. Both defects were reported by CodeRabbit and verified against Pi's own packages.md before fixing. Adapter tests go from 24 to 26; the two source contracts that pinned the previous spellings now pin the new guards, so the object-form unwrapping and the loaded-count reporting cannot be silently reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
336 lines
14 KiB
JavaScript
336 lines
14 KiB
JavaScript
/**
|
|
* Tests for the Pi coding agent package manifest (`pi` key in package.json)
|
|
* and the `.pi/` adapter directory.
|
|
*
|
|
* This is the regression guard for PR #2352, which generated ~440 copied
|
|
* files (skills/agents/prompts/commands) under `.pi/`. The Pi integration
|
|
* must stay a thin adapter: `.pi/` holds only adapter code, and the `pi`
|
|
* manifest points directly at ECC's canonical `skills/` and `commands/`
|
|
* directories rather than at duplicated copies.
|
|
*/
|
|
|
|
const assert = require("assert")
|
|
const fs = require("fs")
|
|
const path = require("path")
|
|
const { execFileSync } = require("child_process")
|
|
|
|
function runTest(name, fn) {
|
|
try {
|
|
fn()
|
|
console.log(` ✓ ${name}`)
|
|
return true
|
|
} catch (error) {
|
|
console.log(` ✗ ${name}`)
|
|
console.error(` ${error.message}`)
|
|
return false
|
|
}
|
|
}
|
|
|
|
function extractFrontmatter(content) {
|
|
const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---/)
|
|
return match ? match[1] : null
|
|
}
|
|
|
|
/**
|
|
* Manual recursive file walk. Node 18 (the repo's minimum supported version,
|
|
* see `engines` in package.json) does not support
|
|
* `fs.readdirSync(dir, { recursive: true })` — that option was only added in
|
|
* Node 20 — so this walk is done by hand instead.
|
|
*/
|
|
function walkFiles(dir) {
|
|
let files = []
|
|
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
|
const fullPath = path.join(dir, entry.name)
|
|
if (entry.isDirectory()) {
|
|
files = files.concat(walkFiles(fullPath))
|
|
} else if (entry.isFile()) {
|
|
files.push(fullPath)
|
|
}
|
|
}
|
|
return files
|
|
}
|
|
|
|
const COPY_OR_GENERATE_WORD = /\b(copy|copies|copying|generate|generates|generated|generating)\b/i
|
|
const PATH_UNDER_PI = /\.pi\//
|
|
const NEGATION_WORD = /\b(no|not|never|nothing|without|isn't|aren't|don't|doesn't)\b/i
|
|
const COPY_INTO_PI_SHAPE = /\b(copy|copies|copying|generate|generates|generating)\b[^.!?\n]*\.pi\//i
|
|
|
|
/**
|
|
* Splits markdown text into sentence-ish chunks: paragraphs first, then each
|
|
* paragraph on sentence-ending punctuation. Good enough for this heuristic —
|
|
* it does not need to be a real sentence parser, only to stop treating an
|
|
* entire multi-sentence paragraph as one unit.
|
|
*/
|
|
function splitIntoSentences(text) {
|
|
return text
|
|
.split(/\n\s*\n/)
|
|
.flatMap((paragraph) => paragraph.split(/(?<=[.!?])\s+/))
|
|
.map((sentence) => sentence.trim())
|
|
.filter(Boolean)
|
|
}
|
|
|
|
/**
|
|
* Detects an actual imperative instruction to copy or generate files into
|
|
* `.pi/` (e.g. "Copy your skills into .pi/skills/ before installing."),
|
|
* while explicitly allowing negated phrasing that documents the opposite
|
|
* (e.g. "no generated copies", "Nothing is copied or generated under .pi/").
|
|
* A naive "copy/generate word AND .pi/ path in the same paragraph" proximity
|
|
* check flags that legitimate negated documentation as a violation; this
|
|
* requires copy/generate word and .pi/ path to appear in the same sentence
|
|
* with no negation word, which is what an actual instruction looks like.
|
|
*/
|
|
function findImperativeCopyIntoPiInstruction(text) {
|
|
return splitIntoSentences(text).some((sentence) => {
|
|
if (!COPY_OR_GENERATE_WORD.test(sentence) || !PATH_UNDER_PI.test(sentence)) {
|
|
return false
|
|
}
|
|
if (NEGATION_WORD.test(sentence)) {
|
|
return false
|
|
}
|
|
return COPY_INTO_PI_SHAPE.test(sentence)
|
|
})
|
|
}
|
|
|
|
function main() {
|
|
console.log("\n=== Testing Pi package manifest (pi key + .pi/ adapter) ===\n")
|
|
|
|
let passed = 0
|
|
let failed = 0
|
|
|
|
const repoRoot = path.join(__dirname, "..", "..")
|
|
const packageJson = JSON.parse(
|
|
fs.readFileSync(path.join(repoRoot, "package.json"), "utf8")
|
|
)
|
|
|
|
const tests = [
|
|
["package.json pi key has exactly extensions, skills, prompts (not agents or chains)", () => {
|
|
assert.ok(
|
|
packageJson.pi && typeof packageJson.pi === "object",
|
|
"package.json must have a top-level `pi` key for Pi coding agent integration"
|
|
)
|
|
const keys = Object.keys(packageJson.pi).sort()
|
|
assert.deepStrictEqual(
|
|
keys,
|
|
["extensions", "prompts", "skills"],
|
|
`pi manifest must contain exactly extensions, prompts, skills — got: ${keys.join(", ")}`
|
|
)
|
|
assert.ok(
|
|
!("agents" in packageJson.pi),
|
|
"pi.agents is not supported by Pi's core manifest — subagent conversion belongs to the pi-subagents companion package and would be silently ignored if placed here"
|
|
)
|
|
assert.ok(
|
|
!("chains" in packageJson.pi),
|
|
"pi.chains is not supported by Pi's core manifest — chains belong to the pi-subagents companion package and would be silently ignored if placed here"
|
|
)
|
|
}],
|
|
|
|
["pi.extensions is exactly the single ECC adapter entry file, and it exists on disk", () => {
|
|
assert.deepStrictEqual(
|
|
packageJson.pi.extensions,
|
|
["./.pi/extensions/index.ts"],
|
|
`pi.extensions must be exactly ["./.pi/extensions/index.ts"] — got ${JSON.stringify(packageJson.pi.extensions)}`
|
|
)
|
|
const extensionPath = path.join(repoRoot, ".pi", "extensions", "index.ts")
|
|
assert.ok(
|
|
fs.existsSync(extensionPath),
|
|
`${extensionPath} does not exist, but pi.extensions references it — Pi would fail to load the adapter`
|
|
)
|
|
}],
|
|
|
|
["pi.skills and pi.prompts point at ECC's canonical top-level directories, never at .pi/", () => {
|
|
assert.deepStrictEqual(
|
|
packageJson.pi.skills,
|
|
["./skills"],
|
|
`pi.skills must be exactly ["./skills"] (ECC's canonical skills directory) — got ${JSON.stringify(packageJson.pi.skills)}`
|
|
)
|
|
assert.deepStrictEqual(
|
|
packageJson.pi.prompts,
|
|
["./commands"],
|
|
`pi.prompts must be exactly ["./commands"] (ECC's canonical commands directory) — got ${JSON.stringify(packageJson.pi.prompts)}`
|
|
)
|
|
for (const entry of [...packageJson.pi.skills, ...packageJson.pi.prompts]) {
|
|
assert.ok(
|
|
!entry.startsWith("./.pi") && !entry.includes(".pi/"),
|
|
`pi.skills/pi.prompts entry "${entry}" must not point under .pi/ — Pi must mount ECC's canonical assets directly, never a copy generated into the adapter directory`
|
|
)
|
|
}
|
|
}],
|
|
|
|
["REGRESSION GUARD: .pi/ contains no generated resource directories (PR #2352 regenerated this)", () => {
|
|
const forbiddenDirs = [".pi/skills", ".pi/agents", ".pi/prompts", ".pi/chains", ".pi/commands", ".pi/rules"]
|
|
for (const relativeDir of forbiddenDirs) {
|
|
const fullPath = path.join(repoRoot, relativeDir)
|
|
assert.ok(
|
|
!fs.existsSync(fullPath),
|
|
`${relativeDir} must not exist — .pi/ may contain adapter code only; a generated resource directory here means canonical skills/agents/prompts were copied instead of referenced by the pi manifest (the PR #2352 regression)`
|
|
)
|
|
}
|
|
}],
|
|
|
|
["REGRESSION GUARD: fewer than 10 files exist on disk under .pi/ (adapter code only)", () => {
|
|
// Authoritative check: walk .pi/ on disk so untracked files (e.g.
|
|
// regenerated skill copies that were never `git add`ed) cannot bypass
|
|
// this guard the way a git-only check would.
|
|
const piDir = path.join(repoRoot, ".pi")
|
|
const onDiskFiles = walkFiles(piDir)
|
|
assert.ok(
|
|
onDiskFiles.length < 10,
|
|
".pi/ must contain only adapter code, never copies of canonical assets " +
|
|
`(skills/agents/prompts) — found ${onDiskFiles.length} files on disk: ` +
|
|
`${onDiskFiles.map((file) => path.relative(repoRoot, file)).join(", ")}`
|
|
)
|
|
|
|
// Additional signal only, not authoritative: git ls-files reports what
|
|
// is tracked, which is useful corroborating evidence but is silently
|
|
// bypassed by untracked files, so it never replaces the on-disk walk above.
|
|
let trackedFiles
|
|
try {
|
|
const output = execFileSync("git", ["ls-files", ".pi"], {
|
|
cwd: repoRoot,
|
|
encoding: "utf8",
|
|
})
|
|
trackedFiles = output.split("\n").filter(Boolean)
|
|
} catch (error) {
|
|
console.log(` (git signal skipped: git unavailable or \`git ls-files .pi\` failed: ${error.message})`)
|
|
}
|
|
if (trackedFiles) {
|
|
assert.ok(
|
|
trackedFiles.length < 10,
|
|
`.pi/ must contain only adapter code, never copies of canonical assets (skills/agents/prompts) — found ${trackedFiles.length} tracked files: ${trackedFiles.join(", ")}`
|
|
)
|
|
}
|
|
}],
|
|
|
|
["package.json files array ships the .pi/ adapter and the canonical assets the manifest depends on", () => {
|
|
const files = packageJson.files
|
|
assert.ok(Array.isArray(files), "package.json must have a `files` array to control what npm publishes")
|
|
assert.ok(
|
|
files.includes(".pi/"),
|
|
"package.json files array must include \".pi/\" so the Pi adapter ships in the published npm package"
|
|
)
|
|
assert.ok(
|
|
files.includes("commands/"),
|
|
"package.json files array must include \"commands/\" — pi.prompts (\"./commands\") depends on this canonical directory being published"
|
|
)
|
|
assert.ok(
|
|
files.some((entry) => entry.startsWith("skills/")),
|
|
"package.json files array must include at least one skills/... entry — pi.skills (\"./skills\") depends on the canonical skills directory being published"
|
|
)
|
|
}],
|
|
|
|
["canonical commands/ is Pi-compatible without transformation (prompt-template format)", () => {
|
|
const commandsDir = path.join(repoRoot, "commands")
|
|
const commandFiles = fs.readdirSync(commandsDir).filter((name) => name.endsWith(".md"))
|
|
assert.ok(
|
|
commandFiles.length >= 50,
|
|
`commands/ must contain at least 50 .md files for Pi's prompt-template format — found ${commandFiles.length}`
|
|
)
|
|
|
|
const planCommandPath = path.join(commandsDir, "plan.md")
|
|
const planCommand = fs.readFileSync(planCommandPath, "utf8")
|
|
const planFrontmatter = extractFrontmatter(planCommand)
|
|
assert.ok(
|
|
planFrontmatter !== null,
|
|
`${planCommandPath} must start with a --- YAML frontmatter block for Pi to parse it as a prompt template`
|
|
)
|
|
assert.ok(
|
|
/^description:/m.test(planFrontmatter),
|
|
`${planCommandPath} frontmatter must contain a description: field — Pi's prompt-template format requires it`
|
|
)
|
|
}],
|
|
|
|
["canonical skills/ is Pi-compatible without transformation (Agent Skills standard)", () => {
|
|
const skillsDir = path.join(repoRoot, "skills")
|
|
const skillDirNames = fs.readdirSync(skillsDir, { withFileTypes: true })
|
|
.filter((entry) => entry.isDirectory())
|
|
.map((entry) => entry.name)
|
|
const skillDirsWithManifest = skillDirNames.filter((name) =>
|
|
fs.existsSync(path.join(skillsDir, name, "SKILL.md"))
|
|
)
|
|
assert.ok(
|
|
skillDirsWithManifest.length >= 100,
|
|
`skills/ must contain at least 100 subdirectories with a SKILL.md for Pi's Agent Skills implementation — found ${skillDirsWithManifest.length}`
|
|
)
|
|
|
|
const sampleSkillPath = path.join(skillsDir, "frontend-patterns", "SKILL.md")
|
|
const sampleSkill = fs.readFileSync(sampleSkillPath, "utf8")
|
|
const sampleFrontmatter = extractFrontmatter(sampleSkill)
|
|
assert.ok(
|
|
sampleFrontmatter !== null,
|
|
`${sampleSkillPath} must start with a --- YAML frontmatter block for Pi to parse it as an Agent Skill`
|
|
)
|
|
assert.ok(
|
|
/^name:/m.test(sampleFrontmatter),
|
|
`${sampleSkillPath} frontmatter must contain a name: field — the Agent Skills standard Pi implements requires it`
|
|
)
|
|
assert.ok(
|
|
/^description:/m.test(sampleFrontmatter),
|
|
`${sampleSkillPath} frontmatter must contain a description: field — the Agent Skills standard Pi implements requires it`
|
|
)
|
|
}],
|
|
|
|
[".pi/README.md documents the single-source-of-truth principle without instructing copies into .pi/", () => {
|
|
const readmePath = path.join(repoRoot, ".pi", "README.md")
|
|
assert.ok(
|
|
fs.existsSync(readmePath),
|
|
`${readmePath} must exist to document the adapter's single-source-of-truth design principle`
|
|
)
|
|
const readme = fs.readFileSync(readmePath, "utf8")
|
|
assert.ok(
|
|
readme.includes("skills/"),
|
|
".pi/README.md must mention skills/ as the canonical directory Pi mounts directly"
|
|
)
|
|
assert.ok(
|
|
readme.includes("commands/"),
|
|
".pi/README.md must mention commands/ as the canonical directory Pi mounts directly"
|
|
)
|
|
|
|
// Detects actual imperative instructions to copy/generate into .pi/,
|
|
// not mere word proximity — a naive "copy/generate word + .pi/ path in
|
|
// the same paragraph" check would flag legitimate negated documentation
|
|
// (e.g. "no generated copies", "Nothing is copied or generated under
|
|
// .pi/") as a violation. Verified against the current .pi/README.md
|
|
// content below (must pass) and the detector's own behavior further down.
|
|
assert.ok(
|
|
!findImperativeCopyIntoPiInstruction(readme),
|
|
".pi/README.md must not instruct users to copy or generate files into .pi/ " +
|
|
"— that documentation would reintroduce the PR #2352 regression"
|
|
)
|
|
|
|
// Sanity-check the detector itself so the assertion above is not
|
|
// vacuously true: it must still catch a real instruction...
|
|
assert.ok(
|
|
findImperativeCopyIntoPiInstruction("Copy your skills into .pi/skills/ before installing."),
|
|
"the copy-into-.pi/ detector must flag an actual instruction to copy files " +
|
|
"into .pi/ (this checks the detector, not .pi/README.md itself)"
|
|
)
|
|
// ...and it must explicitly allow the negated phrasing named in the
|
|
// PR #2352 regression-guard rationale, rather than flagging it.
|
|
assert.ok(
|
|
!findImperativeCopyIntoPiInstruction("This adapter ships with no generated copies under `.pi/`."),
|
|
'the copy-into-.pi/ detector must not flag negated phrasing (e.g. "no generated ' +
|
|
'copies... .pi/") as an instruction (this checks the detector, not .pi/README.md itself)'
|
|
)
|
|
assert.ok(
|
|
!findImperativeCopyIntoPiInstruction("Nothing is copied or generated under `.pi/`."),
|
|
'the copy-into-.pi/ detector must not flag negated phrasing (e.g. "Nothing is copied ' +
|
|
'or generated under .pi/") as an instruction (this checks the detector, not .pi/README.md itself)'
|
|
)
|
|
}],
|
|
]
|
|
|
|
for (const [name, fn] of tests) {
|
|
if (runTest(name, fn)) {
|
|
passed += 1
|
|
} else {
|
|
failed += 1
|
|
}
|
|
}
|
|
|
|
console.log(`\nPassed: ${passed}`)
|
|
console.log(`Failed: ${failed}`)
|
|
process.exit(failed > 0 ? 1 : 0)
|
|
}
|
|
|
|
main()
|