diff --git a/.pi/README.md b/.pi/README.md new file mode 100644 index 000000000..98f1640b6 --- /dev/null +++ b/.pi/README.md @@ -0,0 +1,190 @@ +# .pi — Pi Coding Agent Integration + +This directory contains the **Pi adapter** for ECC — a thin extension that connects the +[@earendil-works/pi-coding-agent](https://github.com/earendil-works/pi-coding-agent) +terminal coding agent to ECC's canonical skills, prompts, and lifecycle hooks. + +## Design Principle + +ECC's canonical assets—skills, agents, commands, and hooks—**remain the single source of truth**. +This adapter contains **only the integration logic**. No copies, no duplication. + +## What This Provides + +- **ECC's skills** from `./skills/` — available in Pi as `/skill:` +- **ECC's commands** from `./commands/` — available in Pi as `/` +- **ECC's engineering rules** from `./rules/common/` — injected into Pi's system + prompt on every turn, so coding style, testing, security, git workflow, and + code-review standards apply in Pi as they do in other harnesses +- **Session lifecycle hooks** — ECC's SessionStart and SessionEnd hooks, run through ECC's own + `run-with-flags.js`, so `ECC_HOOK_PROFILE` and `ECC_DISABLED_HOOKS` keep working under Pi +- **Session context injection** — whatever ECC's SessionStart hook returns as + `additionalContext` is folded into Pi's system prompt for the next turn +- **`/ecc-doctor`** — diagnostic command to verify the integration + +Verified against Pi 0.84.1: a global install exposes 285 skills and 94 commands, resolved +directly from `skills/` and `commands/`, with no generated copies. + +## Installation + +### Option 1: Global Installation (Recommended) + +```bash +# Install ECC as a Pi package +pi install git:github.com/affaan-m/ECC + +# Or from a local checkout +pi install /path/to/ECC + +# Or project-local only +pi install -l /path/to/ECC + +# Verify +pi list +``` + +Then inside Pi, run `/ecc-doctor` to confirm skills, commands, and hooks are available. + +To uninstall: + +```bash +pi remove git:github.com/affaan-m/ECC +``` + +### Option 2: Zero-Install (Existing Claude Code Users) + +If you already have ECC installed for Claude Code, point Pi at the same canonical directories +from `~/.pi/agent/settings.json`: + +```json +{ + "skills": ["~/.claude/skills"], + "prompts": ["~/.claude/commands"] +} +``` + +This gives you skills and commands directly. It does **not** include the lifecycle hook adapter +or `/ecc-doctor` — use Option 1 for the full integration. + +## How It Works + +The `extensions/index.ts` file handles: + +1. **Skill and command mounting** — Pi reads `./skills` and `./commands` directly via the + `pi` key in `package.json`. No transformation is needed: ECC's `SKILL.md` files already + follow the Agent Skills standard Pi implements, and ECC's command frontmatter + (`description`, `argument-hint`) is already Pi's prompt-template format +2. **Lifecycle hooks** — Maps Pi's `session_start` to ECC's `session:start` hook + (`scripts/hooks/session-start.js`) and Pi's `session_shutdown` to ECC's `session:end:marker` + hook (`scripts/hooks/session-end-marker.js`), both invoked through + `scripts/hooks/run-with-flags.js` so ECC's profile and disable flags are honored +3. **Rule injection** — Reads ECC's portable engineering rules from the canonical + `rules/common/` directory at runtime and appends them to the system prompt inside an + `` block on every turn. Nothing is copied into `.pi/`. + `agents.md`, `hooks.md`, and `performance.md` are excluded on purpose: 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. Language-specific rules under `rules//` are not injected in this + first adapter. Set `ECC_PI_RULES` to `0`, `false`, `off`, `none`, or `disabled` to turn + injection off; `/ecc-doctor` reports the current state and the injected size +4. **Context injection** — Parses `hookSpecificOutput.additionalContext` from the SessionStart + hook and appends it to the system prompt on the next `before_agent_start`, wrapped in an + `` block. Non-JSON hook output is tolerated, not treated as an error +5. **Hook isolation** — Failing, missing, or slow hooks degrade to a warning and never + terminate the Pi session. Hook execution is bounded by a timeout and an output limit +6. **Package resolution** — Resolves hook scripts from the installed package via `__dirname`, + never from `process.cwd()`, so a global install works from any project directory. Hooks + still *run* in the user's project directory, so project detection stays correct + +All hook execution is non-shell (`execFile` without shell interpretation), so paths containing +spaces, tabs, or shell metacharacters are safe. + +## Scope + +Intentionally **out of scope** for this first adapter (to be added independently): + +- Subagent conversion and chains (need the `pi-subagents` companion package) +- Structured approval gates (need `@juicesharp/rpiv-ask-user-question`) +- Persistent todos (need `@juicesharp/rpiv-todo`) +- Profile-based resource filtering +- MCP translation — see below; no translation turned out to be necessary + +ECC works in Pi without any of these. Skills and commands are fully available today. + +These capabilities are provided by existing community Pi packages rather than by +anything ECC would need to write. This adapter deliberately does not bundle or +auto-install them: bundling would ship third-party code that executes with full +user permissions in every ECC install, and would make optional capabilities +mandatory. Install whichever you want yourself — `/ecc-doctor` reports which are +present and prints the exact `pi install` command for the ones that are not. + +### MCP + +Pi core has no MCP surface by design. The community `pi-mcp-adapter` package +adds one, and it 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: copying ECC's `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, alongside this adapter's own `/ecc-doctor`. No +translation layer is needed and no ECC change is required. + +```bash +pi install npm:pi-mcp-adapter +cp mcp-configs/mcp-servers.json /path/to/project/.mcp.json +``` + +ECC neither installs nor depends on that package. Two caveats: the adapter's +first run against a new config performs initialization that blocks in +non-interactive (`-p`) mode, so run it once interactively before using it +headless; and only server discovery was verified, not live tool invocation, +which needs real credentials for each server. + +## Security + +- Pi extensions run with the same OS permissions as the Pi process +- This adapter does **not** auto-commit, push, merge, or deploy +- Hooks are executed without a shell, preventing command injection +- Hook failures are isolated and cannot silently authorize blocked operations + +## Troubleshooting + +### Skills or commands not showing up + +**Cause:** the package's resources are disabled, or a project-local install has not been +trusted. Pi asks before trusting a project folder that carries its own `.pi/` resources. + +**Fix:** run `pi config` and confirm the ECC package's skills and prompts are enabled +(Tab switches between user and project scope). Then confirm the package itself is +registered with `pi list`. + +### `/ecc-doctor` not found or reports missing package root + +**Cause:** Extension not loaded or package installed incorrectly. + +**Fix:** +1. Run `pi list` to confirm ECC is registered +2. Restart Pi: exit and reopen the session +3. Run `/ecc-doctor` again + +`/ecc-doctor` prints the resolved package root, the skill and command counts it found, the +hook runner path, the active hook profile, and which optional companion packages are present. +A `NOT FOUND` line points at the specific path that failed to resolve. + +### Hooks not firing + +**Cause:** the extension is not loaded, or the hooks are gated off by an ECC hook profile. + +**Fix:** +1. Confirm `pi list` shows ECC and that `/ecc-doctor` reports the hook runner as found +2. Check `ECC_HOOK_PROFILE` and `ECC_DISABLED_HOOKS` — `/ecc-doctor` prints both. A hook + listed in `ECC_DISABLED_HOOKS` is skipped by design +3. Restart Pi so the extension reloads + +## Notes + +- The `.pi/extensions/` directory is the only place for adapter code +- Skills and commands are defined in the repo root (`skills/`, `commands/`) and referenced by Pi +- MCP is not bundled, but ECC's MCP configs load in Pi through the community `pi-mcp-adapter` — see [MCP](#mcp) above +- This adapter was tested against Pi v0.84.1 diff --git a/.pi/extensions/index.ts b/.pi/extensions/index.ts new file mode 100644 index 000000000..411791d72 --- /dev/null +++ b/.pi/extensions/index.ts @@ -0,0 +1,646 @@ +/** + * ECC adapter for the Pi coding agent. + * + * This is the ONLY adapter logic ECC ships for Pi. ECC's canonical assets stay + * the single source of truth: `skills/` and `commands/` are mounted directly by + * the `pi` manifest in the repo's root `package.json`. Nothing is copied or + * generated under `.pi/`. + * + * What this file adapts: + * - Pi lifecycle events -> ECC's existing hook runner (`run-with-flags.js`), + * so ECC hook profiles and disable flags keep working under Pi. + * - ECC's SessionStart `additionalContext` payload -> Pi's system prompt. + * - A `/ecc-doctor` command for install diagnostics. + * + * Design constraints (see .pi/README.md): + * - Hooks resolve relative to THIS file, never `process.cwd()`, so a global + * `pi install` works from any project directory. + * - Hooks execute via `execFile(process.execPath, [...])` with no shell, so + * paths containing spaces or shell metacharacters are safe. + * - Hook failures are isolated: a broken, missing, or slow hook degrades to a + * warning and never terminates the Pi session. + */ + +import { execFile } from "node:child_process" +import * as fs from "node:fs" +import * as os from "node:os" +import * as path from "node:path" + +/** + * Minimal structural types mirroring `@earendil-works/pi-coding-agent`. + * + * Declared locally on purpose: Pi loads extensions through jiti, which strips + * types without type-checking, so importing the package would add a dependency + * and a lockfile entry that buy nothing at runtime. Field names and signatures + * match the upstream `ExtensionAPI` / `ExtensionContext` declarations; install + * the package as a devDependency if you want editor-level checking. + */ +interface PiUiContext { + notify(message: string, type?: "info" | "warning" | "error"): void +} + +interface PiSessionManager { + getSessionId(): string + getSessionFile(): string | undefined +} + +interface ExtensionContext { + ui: PiUiContext + cwd: string + sessionManager: PiSessionManager +} + +interface SessionStartEvent { + reason: "startup" | "reload" | "new" | "resume" | "fork" +} + +interface SessionShutdownEvent { + reason: "quit" | "reload" | "new" | "resume" | "fork" +} + +interface BeforeAgentStartEvent { + systemPrompt: string +} + +interface BeforeAgentStartResult { + systemPrompt?: string +} + +interface ExtensionAPI { + on( + event: "session_start", + handler: (event: SessionStartEvent, ctx: ExtensionContext) => Promise | void + ): void + on( + event: "session_shutdown", + handler: (event: SessionShutdownEvent, ctx: ExtensionContext) => Promise | void + ): void + on( + event: "before_agent_start", + handler: ( + event: BeforeAgentStartEvent, + ctx: ExtensionContext + ) => Promise | BeforeAgentStartResult | void + ): void + registerCommand( + name: string, + options: { + description?: string + handler: (args: string, ctx: ExtensionContext) => Promise + } + ): void + sendMessage( + message: { customType: string; content: string; display: boolean; details?: unknown }, + options?: { triggerTurn?: boolean; deliverAs?: "steer" | "followUp" | "nextTurn" } + ): void +} + +/** + * ECC package root. This file lives at `/.pi/extensions/index.ts`, so the + * root is two levels up. Pi loads extensions via jiti in CommonJS mode, which + * is why `__dirname` is the correct primitive here rather than + * `import.meta.url` (verified against Pi 0.84.1). + */ +const ECC_ROOT = path.resolve(__dirname, "..", "..") + +/** ECC's universal hook runner. It applies hook-profile and disable flags. */ +const HOOK_RUNNER = path.join(ECC_ROOT, "scripts", "hooks", "run-with-flags.js") + +const HOOK_TIMEOUT_MS = 30_000 +const MAX_HOOK_OUTPUT_BYTES = 1024 * 1024 + +/** + * ECC rules injected into Pi's system prompt, read from the canonical + * `rules/common/` directory at runtime. Nothing is copied or generated. + * + * Excluded on purpose: `agents.md`, `hooks.md`, and `performance.md`. Those + * describe Claude Code primitives Pi does not have (Task/TodoWrite delegation, + * Claude hook event types, thinking-budget toggles), so injecting them would + * instruct the model to use tools that are not there. + */ +const PORTABLE_RULE_FILES = [ + "coding-style.md", + "testing.md", + "security.md", + "git-workflow.md", + "patterns.md", + "development-workflow.md", + "code-review.md", +] as const + +/** Upper bound on injected rule text, so a large edit cannot flood the prompt. */ +const MAX_RULES_BYTES = 32 * 1024 + +/** Values ECC treats as "off" across its existing environment switches. */ +const DISABLED_VALUES = new Set(["0", "false", "off", "none", "disabled"]) + +/** + * Optional Pi companion packages. ECC works without every one of these; they + * are reported by `/ecc-doctor` so users can see which extras are available. + */ +const COMPANION_PACKAGES = [ + "pi-subagents", + "@juicesharp/rpiv-ask-user-question", + "@juicesharp/rpiv-todo", +] as const + +interface HookSpec { + /** ECC hook id, used for profile gating and disable flags. */ + id: string + /** Hook script path relative to the ECC package root. */ + script: string + /** Hook profiles the hook participates in. */ + profiles: string +} + +/** Mirrors the SessionStart wiring in `hooks/hooks.json`. */ +const SESSION_START_HOOK: HookSpec = { + id: "session:start", + script: "scripts/hooks/session-start.js", + profiles: "minimal,standard,strict", +} + +/** Mirrors the SessionEnd wiring in `hooks/hooks.json`. */ +const SESSION_END_HOOK: HookSpec = { + id: "session:end:marker", + script: "scripts/hooks/session-end-marker.js", + profiles: "minimal,standard,strict", +} + +interface HookResult { + stdout: string + failure?: string +} + +/** + * Run an ECC hook through ECC's own runner. + * + * Never rejects: a missing runner, a non-zero exit, a timeout, or a spawn error + * all resolve to a `failure` string that the caller surfaces as a warning. + */ +function runEccHook( + spec: HookSpec, + payload: unknown, + env: NodeJS.ProcessEnv, + cwd: string +): Promise { + return new Promise(resolve => { + if (!fs.existsSync(HOOK_RUNNER)) { + resolve({ stdout: "", failure: `hook runner not found at ${HOOK_RUNNER}` }) + return + } + + const child = execFile( + process.execPath, + [HOOK_RUNNER, spec.id, spec.script, spec.profiles], + { + // Hooks inspect the user's project, so they run there. Only the script + // path is package-relative, and the runner resolves that from + // CLAUDE_PLUGIN_ROOT rather than from the working directory. + cwd, + env, + timeout: HOOK_TIMEOUT_MS, + maxBuffer: MAX_HOOK_OUTPUT_BYTES, + encoding: "utf8", + }, + (error, stdout) => { + const text = typeof stdout === "string" ? stdout : "" + if (error) { + resolve({ stdout: text, failure: `${spec.id}: ${error.message}` }) + return + } + resolve({ stdout: text }) + } + ) + + child.on("error", error => { + resolve({ stdout: "", failure: `${spec.id}: ${error.message}` }) + }) + + // stdin.end() writes asynchronously. A hook that exits, short-circuits, or + // is killed by the timeout before reading the payload makes the write fail + // with EPIPE, which Node reports as an `error` event rather than a throw. + // Without this listener that event is unhandled and would take the Pi + // session down, breaking the isolation guarantee documented above. + child.stdin?.on("error", error => { + resolve({ stdout: "", failure: `${spec.id}: could not write hook payload (${error.message})` }) + }) + + try { + child.stdin?.end(JSON.stringify(payload)) + } catch (error) { + resolve({ + stdout: "", + failure: `${spec.id}: could not write hook payload (${(error as Error).message})`, + }) + } + }) +} + +/** + * Working directory for hook execution: the user's project. Falls back to the + * ECC package root if Pi reports a directory that no longer exists, so a stale + * cwd degrades to a working hook rather than a spawn failure. + */ +function resolveHookCwd(ctx: ExtensionContext): string { + try { + if (ctx.cwd && fs.existsSync(ctx.cwd)) { + return ctx.cwd + } + } catch { + // Fall through to the package root. + } + return ECC_ROOT +} + +function readSessionId(ctx: ExtensionContext): string | undefined { + try { + return ctx.sessionManager.getSessionId() || undefined + } catch { + return undefined + } +} + +/** + * Build the environment ECC hooks expect. + * + * `CLAUDE_PLUGIN_ROOT` / `ECC_PLUGIN_ROOT` are how every ECC hook locates the + * package; setting them from `ECC_ROOT` is what makes a global install resolve + * correctly instead of probing the user's project. The `CLAUDE_*` session vars + * are the names ECC's shared hook scripts already read across harnesses. + */ +function buildHookEnv(ctx: ExtensionContext): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = { + ...process.env, + CLAUDE_PLUGIN_ROOT: ECC_ROOT, + ECC_PLUGIN_ROOT: ECC_ROOT, + CLAUDE_PROJECT_DIR: ctx.cwd, + } + + const sessionId = readSessionId(ctx) + if (sessionId) { + env.CLAUDE_SESSION_ID = sessionId + } + + return env +} + +/** + * Map Pi's session reason onto the `source` values ECC's SessionStart hook + * understands. Pi's `new` and `reload` have no Claude Code equivalent, so they + * report as a fresh startup. + */ +function mapSessionSource(reason: SessionStartEvent["reason"]): string { + switch (reason) { + case "resume": + case "fork": + return "resume" + default: + return "startup" + } +} + +/** + * Extract `hookSpecificOutput.additionalContext` from a hook's stdout. + * + * ECC hooks emit a JSON envelope, but the runner passes stdin straight through + * when a hook is disabled by profile, so non-JSON stdout is expected and must + * not be treated as an error. + */ +function extractAdditionalContext(stdout: string): string | undefined { + const trimmed = stdout.trim() + if (!trimmed.startsWith("{")) { + return undefined + } + + try { + const parsed = JSON.parse(trimmed) as { + hookSpecificOutput?: { additionalContext?: unknown } + } + const context = parsed.hookSpecificOutput?.additionalContext + return typeof context === "string" && context.trim() ? context : undefined + } catch { + return undefined + } +} + +function isDisabledByEnv(value: string | undefined): boolean { + return typeof value === "string" && DISABLED_VALUES.has(value.trim().toLowerCase()) +} + +/** Memoized so the rule files are read once per session, not once per turn. */ +let cachedRules: string | null | undefined + +/** + * How many of `PORTABLE_RULE_FILES` actually made it into `cachedRules`. + * + * Kept alongside the cache because `loadPortableRules` silently drops files it + * cannot read, files that are empty, and every file past the size cap — so the + * allowlist length would overstate a partial install in `/ecc-doctor`, which is + * the one place a user looks to find exactly that. + */ +let cachedRuleFileCount = 0 + +/** + * ECC's portable engineering rules, concatenated from the canonical + * `rules/common/` directory of the installed package. + * + * Returns null when disabled via `ECC_PI_RULES` or when no rule file could be + * read, so a partial install degrades to "no rules" instead of failing. + */ +function loadPortableRules(): string | null { + if (cachedRules !== undefined) { + return cachedRules + } + + if (isDisabledByEnv(process.env.ECC_PI_RULES)) { + cachedRules = null + cachedRuleFileCount = 0 + return cachedRules + } + + const sections: string[] = [] + let total = 0 + + for (const file of PORTABLE_RULE_FILES) { + let text: string + try { + text = fs.readFileSync(path.join(ECC_ROOT, "rules", "common", file), "utf8").trim() + } catch { + continue + } + + if (!text) { + continue + } + + if (total + text.length > MAX_RULES_BYTES) { + break + } + + total += text.length + sections.push(text) + } + + cachedRules = sections.length > 0 ? sections.join("\n\n---\n\n") : null + cachedRuleFileCount = sections.length + return cachedRules +} + +/** + * Pi's config directory, honoring the documented `PI_CODING_AGENT_DIR` override. + */ +function resolvePiConfigDir(): string { + const override = process.env.PI_CODING_AGENT_DIR + if (override && override.trim()) { + return override.trim() + } + return path.join(os.homedir(), ".pi", "agent") +} + +/** + * Package names Pi currently has installed, read from the same `packages` + * lists Pi itself uses: the user config directory plus the project-local + * `.pi/settings.json`. + * + * `require.resolve` cannot answer this. Pi installs packages under its own + * config directory (`/npm`, `/git`), which is not on Node's + * module resolution path from this file, so resolving would report every + * companion as missing no matter what the user has installed. + */ +function listInstalledPiPackages(projectDir: string): Set { + const names = new Set() + + const settingsFiles = [ + path.join(resolvePiConfigDir(), "settings.json"), + path.join(projectDir, ".pi", "settings.json"), + ] + + for (const file of settingsFiles) { + try { + const parsed = JSON.parse(fs.readFileSync(file, "utf8")) as { packages?: unknown } + if (!Array.isArray(parsed.packages)) { + continue + } + for (const entry of parsed.packages) { + const name = normalizePiPackageName(entry) + if (name) { + names.add(name) + } + } + } catch { + // Missing or unreadable settings are simply "nothing installed here". + } + } + + return names +} + +/** + * Reduce a `packages` entry to a bare package name. + * + * An entry is either the source string itself or an object carrying that + * string under `source` alongside resource filters (`{ source: "npm:x", + * skills: [] }`). Pi accepts both forms, and a filtered package is just as + * installed as a plain one, so both must resolve to the same name. + * + * Sources look like `npm:pi-subagents`, `npm:@scope/name@1.2.3`, a git source, + * or a filesystem path. Only npm sources carry a comparable package name. + */ +function normalizePiPackageName(entry: unknown): string | undefined { + const source = entry && typeof entry === "object" ? (entry as { source?: unknown }).source : entry + + if (typeof source !== "string" || !source.startsWith("npm:")) { + return undefined + } + + const spec = source.slice("npm:".length) + // Strip a trailing @version without breaking the leading @ of a scoped name. + const versionAt = spec.lastIndexOf("@") + return versionAt > 0 ? spec.slice(0, versionAt) : spec +} + +function countDirectories(dir: string): number { + try { + return fs.readdirSync(dir, { withFileTypes: true }).filter(entry => entry.isDirectory()).length + } catch { + return 0 + } +} + +function countMarkdownFiles(dir: string): number { + try { + return fs.readdirSync(dir).filter(name => name.endsWith(".md")).length + } catch { + return 0 + } +} + +function readEccVersion(): string { + try { + const manifest = JSON.parse(fs.readFileSync(path.join(ECC_ROOT, "package.json"), "utf8")) as { + version?: string + } + return manifest.version || "unknown" + } catch { + return "unknown" + } +} + +function describeRulesStatus(): string { + if (isDisabledByEnv(process.env.ECC_PI_RULES)) { + return "disabled via ECC_PI_RULES" + } + + const rules = loadPortableRules() + if (!rules) { + return `NOT FOUND (${path.join(ECC_ROOT, "rules", "common")})` + } + + const skipped = PORTABLE_RULE_FILES.length - cachedRuleFileCount + const shortfall = skipped > 0 ? ` (${skipped} unreadable, empty, or past the size cap)` : "" + return `${cachedRuleFileCount}/${PORTABLE_RULE_FILES.length} rule file(s), ${rules.length} chars, from rules/common/${shortfall}` +} + +function buildDoctorReport(ctx: ExtensionContext): string { + const skillsDir = path.join(ECC_ROOT, "skills") + const commandsDir = path.join(ECC_ROOT, "commands") + const skillCount = countDirectories(skillsDir) + const commandCount = countMarkdownFiles(commandsDir) + + const lines = [ + "ECC adapter for Pi", + "", + ` ECC version: ${readEccVersion()}`, + ` Package root: ${ECC_ROOT}`, + ` Project cwd: ${ctx.cwd}`, + "", + "Canonical resources", + ` skills/ ${skillCount > 0 ? `${skillCount} skill(s)` : "NOT FOUND"} (${skillsDir})`, + ` commands/ ${commandCount > 0 ? `${commandCount} command(s)` : "NOT FOUND"} (${commandsDir})`, + "", + "Engineering rules (injected into the system prompt)", + ` ${describeRulesStatus()}`, + "", + "Hook runner", + ` ${fs.existsSync(HOOK_RUNNER) ? "found" : "NOT FOUND"} (${HOOK_RUNNER})`, + ` profile: ${process.env.ECC_HOOK_PROFILE || "standard (default)"}`, + ` disabled: ${process.env.ECC_DISABLED_HOOKS || "none"}`, + "", + "Optional companion packages (from Pi's installed package list)", + ] + + const installed = listInstalledPiPackages(ctx.cwd) + for (const name of COMPANION_PACKAGES) { + const present = installed.has(name) + lines.push(` ${present ? "installed " : "not installed"} ${name}`) + if (!present) { + lines.push(` install with: pi install npm:${name}`) + } + } + + lines.push( + "", + "Companion packages are optional; ECC skills, commands, and session hooks", + "work without them. See .pi/README.md for what each one unlocks.", + "Detection reads Pi's `packages` list, so a companion vendored some other", + "way may work while reporting as not installed." + ) + + return lines.join("\n") +} + +export default function (pi: ExtensionAPI): void { + /** + * ECC's SessionStart hook returns context for the model, but Pi has no + * equivalent of Claude Code's `additionalContext` field. It is held here and + * folded into the system prompt on the next agent start, which is the + * documented Pi injection point that does not fabricate a user turn. + */ + let pendingContext: string | undefined + + pi.on("session_start", async (event, ctx) => { + const payload = { + hook_event_name: "SessionStart", + source: mapSessionSource(event.reason), + cwd: ctx.cwd, + session_id: readSessionId(ctx), + } + + // Drop any context captured by an earlier session start that has not been + // injected yet. Pi can start a new session (/new, /resume, /fork) before + // `before_agent_start` consumes the previous value, and replaying context + // built for a different session would describe the wrong project state. + pendingContext = undefined + + const result = await runEccHook( + SESSION_START_HOOK, + payload, + buildHookEnv(ctx), + resolveHookCwd(ctx) + ) + + if (result.failure) { + ctx.ui.notify(`ECC session-start hook skipped (${result.failure})`, "warning") + return + } + + pendingContext = extractAdditionalContext(result.stdout) + }) + + pi.on("before_agent_start", event => { + const additions: string[] = [] + + // Rules describe standing engineering policy, so they are re-applied on + // every turn. The session context is a one-shot handoff and is consumed. + const rules = loadPortableRules() + if (rules) { + additions.push(`\n${rules}\n`) + } + + if (pendingContext) { + additions.push(`\n${pendingContext}\n`) + pendingContext = undefined + } + + if (additions.length === 0) { + return + } + + return { systemPrompt: [event.systemPrompt, ...additions].join("\n\n") } + }) + + pi.on("session_shutdown", async (event, ctx) => { + const payload = { + hook_event_name: "SessionEnd", + reason: event.reason, + cwd: ctx.cwd, + session_id: readSessionId(ctx), + } + + const result = await runEccHook( + SESSION_END_HOOK, + payload, + buildHookEnv(ctx), + resolveHookCwd(ctx) + ) + + if (result.failure) { + ctx.ui.notify(`ECC session-end hook skipped (${result.failure})`, "warning") + } + }) + + pi.registerCommand("ecc-doctor", { + description: "Report ECC adapter status: package root, canonical resources, hooks, companions", + handler: async (_args, ctx) => { + pi.sendMessage( + { + customType: "ecc-doctor", + content: buildDoctorReport(ctx), + display: true, + }, + { deliverAs: "nextTurn" } + ) + }, + }) +} diff --git a/docs/architecture/harness-adapter-compliance.md b/docs/architecture/harness-adapter-compliance.md index 4d09a8301..09061190d 100644 --- a/docs/architecture/harness-adapter-compliance.md +++ b/docs/architecture/harness-adapter-compliance.md @@ -39,6 +39,7 @@ The matrix below is rendered from | Claude Code | Native | Claude plugin assets; skills; commands; hooks; MCP config; local rules; statusline-oriented workflows | Claude-native hooks do not imply parity in other harnesses | `./install.sh --profile minimal --target claude`; Claude plugin install | `npm run harness:audit -- --format json`; `node scripts/session-inspect.js --list-adapters` | Avoid loading every skill by default; keep hooks opt-in and inspectable. | | Codex | Instruction-backed | `AGENTS.md`; Codex plugin metadata; skills; MCP reference config; command patterns | Native hook enforcement and Claude slash-command semantics are not equivalent | `./install.sh --profile minimal --target codex`; repo-local `AGENTS.md` review | `npm run harness:audit -- --format json` | Treat hooks as policy text unless a native Codex hook surface exists. | | OpenCode | Adapter-backed | OpenCode package/plugin metadata; shared skills; MCP config; event adapter patterns | Event names, plugin packaging, and command dispatch differ from Claude Code | OpenCode package or plugin surface from this repo | `node tests/scripts/build-opencode.test.js`; `npm run harness:audit -- --format json` | Keep hook logic in shared scripts and adapt only event shape at the edge. | +| Pi | Adapter-backed | Pi package manifest; canonical ECC skills (skills/); canonical ECC commands as prompt templates (commands/); canonical ECC engineering rules (rules/common/) injected into the system prompt; session lifecycle hook adapter; /ecc-doctor diagnostics command | Subagents, chains, approval prompts, and persistent todos require companion Pi packages and are not part of this adapter; Pi core has no MCP surface, though ECC MCP configs load verbatim through the community pi-mcp-adapter package, which ECC neither installs nor depends on | `pi install git:github.com/affaan-m/ECC`; `pi install /path/to/ECC` from a local checkout | `node tests/pi/pi-package-manifest.test.js`; `node tests/pi/pi-extension-adapter.test.js`; `npm run harness:adapters -- --check` | Pi extensions execute with full user permissions, and hooks run without a shell and resolve from the installed package rather than the user project; Keep canonical skills and commands as the single source of truth, and never generate copies under .pi/ | | Cursor | Adapter-backed | Cursor rules; project-local skills; hook adapter; shared scripts | Cursor hook events and rule loading differ from Claude Code | `./install.sh --profile minimal --target cursor` | `node tests/lib/install-targets.test.js`; `npm run harness:audit -- --format json` | Cursor adapters must preserve existing project rules and avoid silent overwrite. | | Gemini | Instruction-backed | Gemini project-local instructions; shared skills; rules; compatibility docs | No full ECC hook parity; ecosystem ports must document drift from upstream ECC | `./install.sh --profile minimal --target gemini` | `node tests/lib/install-targets.test.js` | Treat Gemini ports as ecosystem adapters until validated end to end inside Gemini CLI. | | Zed | Adapter-backed | Zed project settings; flattened project rules; shared skills; commands; agents | Zed external agents and native Agent Panel permissions are not Claude hooks | `./install.sh --profile minimal --target zed` | `node tests/lib/install-targets.test.js`; `npm run harness:audit -- --format json` | Keep project settings conservative and do not copy BYOK/OpenRouter secrets into `.zed/`. | diff --git a/manifests/install-modules.json b/manifests/install-modules.json index c9eb0d232..8c0ea11d8 100644 --- a/manifests/install-modules.json +++ b/manifests/install-modules.json @@ -113,6 +113,7 @@ ".cursor", ".gemini", ".opencode", + ".pi", ".qwen", ".zed", "mcp-configs", diff --git a/package.json b/package.json index f047c6a66..55030f5f4 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ ".hermes/", ".kimi/", ".opencode/", + ".pi/", ".openclaw/", ".qwen/", ".zed/", @@ -470,6 +471,17 @@ "ajv": "8.20.0", "sql.js": "1.14.1" }, + "pi": { + "extensions": [ + "./.pi/extensions/index.ts" + ], + "skills": [ + "./skills" + ], + "prompts": [ + "./commands" + ] + }, "devDependencies": { "@eslint/js": "9.39.2", "@opencode-ai/plugin": "1.17.3", diff --git a/scripts/lib/harness-adapter-compliance.js b/scripts/lib/harness-adapter-compliance.js index 11bb363f2..e1aafd752 100644 --- a/scripts/lib/harness-adapter-compliance.js +++ b/scripts/lib/harness-adapter-compliance.js @@ -122,6 +122,40 @@ const ADAPTER_RECORDS = Object.freeze([ 'scripts/build-opencode.js', ], }, + { + id: 'pi', + harness: 'Pi', + state: 'Adapter-backed', + supported_assets: [ + 'Pi package manifest', + 'canonical ECC skills (skills/)', + 'canonical ECC commands as prompt templates (commands/)', + 'canonical ECC engineering rules (rules/common/) injected into the system prompt', + 'session lifecycle hook adapter', + '/ecc-doctor diagnostics command', + ], + unsupported_surfaces: [ + 'Subagents, chains, approval prompts, and persistent todos require companion Pi packages and are not part of this adapter', + 'Pi core has no MCP surface, though ECC MCP configs load verbatim through the community pi-mcp-adapter package, which ECC neither installs nor depends on', + ], + install_or_onramp: ['`pi install git:github.com/affaan-m/ECC`', '`pi install /path/to/ECC` from a local checkout'], + verification_commands: [ + '`node tests/pi/pi-package-manifest.test.js`', + '`node tests/pi/pi-extension-adapter.test.js`', + '`npm run harness:adapters -- --check`', + ], + risk_notes: [ + 'Pi extensions execute with full user permissions, and hooks run without a shell and resolve from the installed package rather than the user project', + 'Keep canonical skills and commands as the single source of truth, and never generate copies under .pi/', + ], + last_verified_at: '2026-08-10', + owner: 'ECC maintainers', + source_docs: [ + '.pi/extensions/index.ts', + '.pi/README.md', + 'package.json', + ], + }, { id: 'cursor', harness: 'Cursor', diff --git a/tests/pi/pi-extension-adapter.test.js b/tests/pi/pi-extension-adapter.test.js new file mode 100644 index 000000000..aff53a8c8 --- /dev/null +++ b/tests/pi/pi-extension-adapter.test.js @@ -0,0 +1,1352 @@ +/** + * Tests for the ECC <-> Pi coding agent thin adapter (.pi/extensions/index.ts). + * + * This adapter was rejected once already (PR #2352) for four defects: + * (a) resolving hook scripts from `process.cwd()` instead of the installed + * ECC package root, which breaks global installs; + * (b) running hooks through an interpolated shell string + * (`exec(\`node ${scriptPath}\`)`), which breaks on paths with spaces + * and is a shell-injection risk; + * (c) using the undocumented `app.events` bus instead of the documented + * `pi.on(...)` lifecycle API; + * (d) shipping with no compatibility tests at all. + * + * Group 1 below reads `.pi/extensions/index.ts` as text and asserts the + * source contract that keeps those defects from coming back. The file is + * TypeScript loaded by Pi through jiti at runtime, so it cannot be + * `require()`d or `import()`ed from a plain Node test — source inspection is + * the only option available without adding a build step or a new dependency. + * + * Group 2 exercises ECC's real hook runner (`scripts/hooks/run-with-flags.js`) + * with the exact argv/env shape the adapter builds, so the fix is proven by + * behavior, not just by grep. + * + * Group 3 covers three fixes a code review added on top of the above: EPIPE + * isolation on `child.stdin`, clearing stale `pendingContext` at session + * start, and reading companion-package installs from Pi's own settings files + * instead of `require.resolve`. Each fix gets a source-text assertion (so a + * regression is caught even if the behavioral mirror still passes) plus a + * real behavioral test wherever the fix is about runtime behavior rather + * than pure control flow. + * + * Group 4 covers the adapter's injection of ECC's canonical engineering rules + * into Pi's system prompt (`PORTABLE_RULE_FILES`, `loadPortableRules`, + * `isDisabledByEnv`, and the expanded `before_agent_start` handler). The core + * constraint under test is that rules are read at RUNTIME from the canonical + * `rules/common/` directory of the installed package — nothing is copied or + * generated into `.pi/`. Each test pairs a source-text assertion (so a + * regression in the real adapter fails even if a behavioral mirror still + * passes) with either a real-filesystem check against this repo's actual + * `rules/common/` files or a hand-copied mirror of the adapter's own logic. + */ + +const assert = require("assert") +const fs = require("fs") +const os = require("os") +const path = require("path") +const { spawnSync, execFile } = require("child_process") + +async function runTest(name, fn) { + try { + await fn() + console.log(` ✓ ${name}`) + return true + } catch (error) { + console.log(` ✗ ${name}`) + console.error(` ${error.message}`) + return false + } +} + +/** + * Strips `/* ... *\/` and `// ...` comments so the "never resolves from + * process.cwd()" check tests real behavior, not a doc comment. The adapter's + * own header comment explains the anti-pattern by naming it in backticks + * (`"never `process.cwd()`, so a global pi install works..."`), which is + * correct documentation, not a regression — the check must look past it. + */ +function stripComments(source) { + return source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, "") +} + +/** + * Mirrors the adapter's own hook invocation (`runEccHook` in + * .pi/extensions/index.ts): same binary (`process.execPath`), same argv + * shape, same stdin-JSON payload, same env keys. No shell is used anywhere. + */ +function runHookRunner(eccRoot, hookId, relScript, profiles, payload, extraEnv, cwd) { + const runner = path.join(eccRoot, "scripts", "hooks", "run-with-flags.js") + return spawnSync(process.execPath, [runner, hookId, relScript, profiles], { + input: JSON.stringify(payload), + encoding: "utf8", + cwd: cwd || eccRoot, + timeout: 30000, + env: { ...process.env, CLAUDE_PLUGIN_ROOT: eccRoot, ECC_PLUGIN_ROOT: eccRoot, ...extraEnv }, + }) +} + +/** + * Builds a minimal, standalone ECC package skeleton under a fresh temp + * directory so tests 8/9 can simulate a global install without touching the + * real repo. Only the files `run-with-flags.js` -> `session-end-marker.js` + * actually `require()` at runtime are copied. + */ +function buildEccSkeleton(repoRoot) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "ecc pi test-")) + const hooksDir = path.join(root, "scripts", "hooks") + fs.mkdirSync(hooksDir, { recursive: true }) + + for (const name of ["run-with-flags.js", "session-end-marker.js", "pretooluse-visible-output.js"]) { + fs.cpSync(path.join(repoRoot, "scripts", "hooks", name), path.join(hooksDir, name)) + } + fs.cpSync(path.join(repoRoot, "scripts", "lib"), path.join(root, "scripts", "lib"), { recursive: true }) + + return root +} + +/** + * Mirror of the adapter's `extractAdditionalContext` (same file, same six + * lines of logic) so the parsing contract can be exercised directly without + * importing the TypeScript source. This copy proves the *behavior* below is + * correct, but a copy cannot detect the real adapter's guards drifting out + * from under it. The source-text assertions in the "additionalContext + * extraction tolerates non-JSON hook passthrough" test below read the real + * `extractAdditionalContext` out of `.pi/extensions/index.ts` and pin its + * guards directly, so that kind of drift fails the test instead of passing + * silently against this mirror. + */ +function extractAdditionalContext(stdout) { + const trimmed = stdout.trim() + if (!trimmed.startsWith("{")) { + return undefined + } + try { + const parsed = JSON.parse(trimmed) + const context = parsed.hookSpecificOutput && parsed.hookSpecificOutput.additionalContext + return typeof context === "string" && context.trim() ? context : undefined + } catch { + return undefined + } +} + +/** + * Mirror of the adapter's `normalizePiPackageName` (same file, same handful of + * lines) so the object-form unwrapping and the trailing-@version stripping can + * be exercised directly without importing the TypeScript source. This copy + * proves the *behavior* below is correct, but a copy cannot detect the real + * adapter's guards drifting out from under it. The source-text assertions in + * the "companion package detection reads Pi's package list" test below read + * the real `normalizePiPackageName` text out of `.pi/extensions/index.ts` and + * pin its actual guards directly, so that kind of drift fails the test instead + * of passing silently against this mirror. + */ +function normalizePiPackageName(entry) { + const source = entry && typeof entry === "object" ? entry.source : entry + if (typeof source !== "string" || !source.startsWith("npm:")) { + return undefined + } + const spec = source.slice("npm:".length) + // Strip a trailing @version without breaking the leading @ of a scoped name. + const versionAt = spec.lastIndexOf("@") + return versionAt > 0 ? spec.slice(0, versionAt) : spec +} + +/** + * Mirror of the per-file body of the adapter's `listInstalledPiPackages` + * (same file, same read-parse-normalize-collect loop), applied to a single + * settings file so the "npm entries only, missing/malformed settings degrade + * to empty" contract can be exercised against a real temp file without + * importing the TypeScript source or touching a real `~/.pi/agent` + * directory. This copy proves the *behavior* below is correct, but a copy + * cannot detect the real adapter's guards drifting out from under it. The + * source-text assertions in the "companion package detection reads Pi's + * package list" test above read the real `listInstalledPiPackages` / + * `normalizePiPackageName` text out of `.pi/extensions/index.ts` and pin its + * actual guards directly, so that kind of drift fails the test instead of + * passing silently against this mirror. + */ +function readInstalledPackageNames(settingsFile) { + const names = new Set() + try { + const parsed = JSON.parse(fs.readFileSync(settingsFile, "utf8")) + if (!Array.isArray(parsed.packages)) { + return names + } + for (const entry of parsed.packages) { + const name = normalizePiPackageName(entry) + if (name) { + names.add(name) + } + } + } catch { + // Missing or unreadable settings are simply "nothing installed here". + } + return names +} + +/** + * Parses the `PORTABLE_RULE_FILES` array literal out of `.pi/extensions/index.ts` + * by text, so the real-filesystem-existence test and the `loadPortableRules` + * behavioral mirror below follow the constant instead of hardcoding the file + * list and silently drifting from it. + */ +function parsePortableRuleFiles(source) { + const constStart = source.indexOf("const PORTABLE_RULE_FILES") + if (constStart === -1) { + return [] + } + const constEnd = source.indexOf("]", constStart) + if (constEnd === -1) { + return [] + } + const constBody = source.slice(constStart, constEnd + 1) + return Array.from(constBody.matchAll(/["'`]([\w.-]+\.md)["'`]/g)).map(match => match[1]) +} + +/** + * Parses the numeric value of `MAX_RULES_BYTES` (e.g. `32 * 1024`) out of + * `.pi/extensions/index.ts`, so the cap assertion in the `loadPortableRules` + * behavioral mirror below follows the real constant instead of a hardcoded + * number. The captured expression is validated against a digits/operators + * whitelist before evaluation, so this never executes arbitrary source text. + */ +function parseMaxRulesBytes(source) { + const match = source.match(/const\s+MAX_RULES_BYTES\s*=\s*([0-9_ \t*/+-]+)/) + if (!match) { + return undefined + } + const expression = match[1].trim() + if (!expression || !/^[0-9_ \t*+]+$/.test(expression)) { + return undefined + } + + // Evaluate sums of products directly instead of through Function(): the + // constant is only ever a literal like `32 * 1024`, and a test helper has no + // business compiling code at runtime. + const total = expression + .replace(/_/g, "") + .split("+") + .reduce((sum, term) => { + const product = term.split("*").reduce((acc, factor) => acc * Number(factor.trim()), 1) + return sum + product + }, 0) + + return Number.isFinite(total) ? total : undefined +} + +/** + * Mirror of the adapter's `loadPortableRules` (same file, same + * read-trim-skip-cap-join loop over `rules/common/`, same + * `"\n\n---\n\n"` join). Deliberately omits the `ECC_PI_RULES` disable check + * and the `cachedRules` memoization, which are exercised separately (the + * disable check via `isDisabledByEnv` below; memoization is pure control + * flow with no behavior to mirror). This copy proves the *behavior* below is + * correct, but a copy cannot detect the real adapter's guards drifting out + * from under it. The source-text assertions in the "PORTABLE_RULE_FILES ..." + * and "engineering rules are read from rules/common ..." tests below read the + * real constant, the real `rules/common` path, and the real `MAX_RULES_BYTES` + * value out of `.pi/extensions/index.ts` and pin them directly, so that kind + * of drift fails those tests instead of passing silently against this mirror. + */ +function loadPortableRulesMirror(rootDir, ruleFiles, maxBytes) { + const sections = [] + let total = 0 + for (const file of ruleFiles) { + let text + try { + text = fs.readFileSync(path.join(rootDir, "rules", "common", file), "utf8").trim() + } catch { + continue + } + if (!text) { + continue + } + if (total + text.length > maxBytes) { + break + } + total += text.length + sections.push(text) + } + return sections.length > 0 ? sections.join("\n\n---\n\n") : null +} + +/** + * Mirror of the adapter's `isDisabledByEnv` (same `DISABLED_VALUES` set, same + * trim + lowercase normalization). This copy proves the *behavior* below is + * correct, but a copy cannot detect the real adapter's guard drifting out + * from under it. The source-text assertion in the "isDisabledByEnv ..." test + * below reads the real function and the real `ECC_PI_RULES` env var name out + * of `.pi/extensions/index.ts` and pins them directly. + */ +const DISABLED_VALUES_MIRROR = new Set(["0", "false", "off", "none", "disabled"]) +function isDisabledByEnvMirror(value) { + return typeof value === "string" && DISABLED_VALUES_MIRROR.has(value.trim().toLowerCase()) +} + +async function main() { + console.log("\n=== Testing .pi/extensions/index.ts (Pi thin adapter) ===\n") + + let passed = 0 + let failed = 0 + + const repoRoot = path.join(__dirname, "..", "..") + const extensionPath = path.join(repoRoot, ".pi", "extensions", "index.ts") + const extensionSource = fs.readFileSync(extensionPath, "utf8") + + const tests = [ + // ---- Group 1: source contract ------------------------------------- + + ["resolves the ECC package root from __dirname, never from process.cwd()", () => { + assert.ok( + extensionSource.includes("path.resolve(__dirname"), + "expected the adapter to derive its package root with path.resolve(__dirname, ...); " + + "resolving from __dirname is what makes a globally installed ECC find its own hooks " + + "regardless of which project the user opened Pi in" + ) + + const withoutComments = stripComments(extensionSource) + assert.ok( + !withoutComments.includes("process.cwd()"), + "found process.cwd() used as executable code in .pi/extensions/index.ts; " + + "resolving hook scripts from the working directory breaks global installs " + + "because it looks for ECC's hooks inside the user's project instead of the " + + "installed ECC package (this is the exact defect PR #2352 was rejected for)" + ) + }], + + ["executes hooks via execFile with no shell, so paths with spaces or metacharacters are safe", () => { + assert.ok( + extensionSource.includes("execFile("), + "expected the adapter to invoke hooks via child_process.execFile(...)" + ) + assert.ok( + extensionSource.includes("process.execPath"), + "expected hooks to be spawned with process.execPath, not a hardcoded 'node' string" + ) + + const shellExecPattern = /(? { + assert.ok( + extensionSource.includes(`pi.on("session_start"`), + "expected the adapter to register a session_start handler via pi.on(...)" + ) + assert.ok( + extensionSource.includes(`pi.on("session_shutdown"`), + "expected the adapter to register a session_shutdown handler via pi.on(...)" + ) + assert.ok( + extensionSource.includes(`pi.on("before_agent_start"`), + "expected the adapter to register a before_agent_start handler via pi.on(...)" + ) + assert.ok( + !extensionSource.includes("app.events"), + "found app.events in .pi/extensions/index.ts; app.events is an undocumented " + + "event-bus API that is not part of Pi's supported extension contract and can " + + "change or disappear without notice" + ) + assert.ok( + !extensionSource.includes(".events.on("), + "found a .events.on(...) subscription in .pi/extensions/index.ts; subscribing " + + "through an undocumented event bus instead of the documented pi.on(...) " + + "lifecycle is not part of Pi's supported extension contract" + ) + }], + + ["registers the ecc-doctor diagnostics command", () => { + assert.ok( + extensionSource.includes(`registerCommand("ecc-doctor"`), + "expected the adapter to register an 'ecc-doctor' command via pi.registerCommand(...) " + + "so users have an install-diagnostics entry point" + ) + }], + + ["bounds hook execution with a timeout and a maxBuffer", () => { + assert.ok( + extensionSource.includes("timeout"), + "expected the execFile(...) call options to include a timeout; an unbounded hook " + + "process can hang the Pi session forever on a stuck or misbehaving hook" + ) + assert.ok( + extensionSource.includes("maxBuffer"), + "expected the execFile(...) call options to include a maxBuffer; without it a " + + "runaway hook writing unbounded stdout can crash the adapter process" + ) + }], + + ["exports a default extension factory function", () => { + assert.ok( + extensionSource.includes("export default function"), + "expected .pi/extensions/index.ts to `export default function`, matching the " + + "shape Pi's extension loader expects" + ) + }], + + ["propagates the ECC package root to hooks via CLAUDE_PLUGIN_ROOT and ECC_PLUGIN_ROOT", () => { + assert.ok( + extensionSource.includes("CLAUDE_PLUGIN_ROOT"), + "expected the adapter to set CLAUDE_PLUGIN_ROOT in the hook environment; ECC's " + + "shared hook scripts read this to locate the package root" + ) + assert.ok( + extensionSource.includes("ECC_PLUGIN_ROOT"), + "expected the adapter to set ECC_PLUGIN_ROOT in the hook environment; this is " + + "the ECC-specific fallback the same hook scripts also read" + ) + }], + + // ---- Group 2: real hook-runner behavior --------------------------- + + ["GLOBAL INSTALL + SPACE IN PATH: hook execution succeeds from a package root whose path contains a space", () => { + const skeletonRoot = buildEccSkeleton(repoRoot) + try { + assert.ok( + skeletonRoot.includes(" "), + "test setup bug: the temp skeleton directory must contain a space to reproduce " + + "a global-install path (e.g. 'Application Support') — got: " + skeletonRoot + ) + + const result = runHookRunner( + skeletonRoot, + "session:end:marker", + "scripts/hooks/session-end-marker.js", + "minimal,standard,strict", + { hook_event_name: "SessionEnd", reason: "quit", cwd: skeletonRoot, session_id: "pi-adapter-test" } + ) + + assert.strictEqual( + result.error, + undefined, + "hook runner failed to spawn from a package root containing a space " + + `(${skeletonRoot}); this is exactly the shell-interpolation regression ` + + `PR #2352 was rejected for (error: ${result.error && result.error.message})` + ) + assert.strictEqual( + result.status, + 0, + "hook runner exited non-zero when invoked from a package root containing a " + + `space (${skeletonRoot}); a path with a space broke hook execution ` + + `(stderr: ${result.stderr})` + ) + } finally { + fs.rmSync(skeletonRoot, { recursive: true, force: true }) + } + }], + + ["hook resolution is package-relative, not cwd-relative: still succeeds when cwd points elsewhere", () => { + const skeletonRoot = buildEccSkeleton(repoRoot) + try { + const result = runHookRunner( + skeletonRoot, + "session:end:marker", + "scripts/hooks/session-end-marker.js", + "minimal,standard,strict", + { hook_event_name: "SessionEnd", reason: "quit", cwd: os.tmpdir(), session_id: "pi-adapter-test" }, + {}, + os.tmpdir() + ) + + assert.strictEqual( + result.error, + undefined, + "hook runner failed to spawn when cwd pointed away from the ECC package root; " + + "a globally installed ECC must resolve its own hooks regardless of which " + + `project directory the user is in (error: ${result.error && result.error.message})` + ) + assert.strictEqual( + result.status, + 0, + "hook runner exited non-zero when cwd pointed away from the ECC package root " + + `(cwd=${os.tmpdir()}, CLAUDE_PLUGIN_ROOT=${skeletonRoot}); this means hook ` + + "resolution is leaking cwd-dependence instead of being package-relative " + + `(stderr: ${result.stderr})` + ) + } finally { + fs.rmSync(skeletonRoot, { recursive: true, force: true }) + } + }], + + ["profile gating is honored: a disabled hook and a restrictive profile both degrade cleanly", () => { + // Uses the same isolated skeleton as tests 8/9 (not repoRoot) so that + // session-end-marker.js never executes against the real checkout: a + // real run can leave marker artifacts behind and would make this + // test's outcome depend on whatever state the repo happens to be in. + const skeletonRoot = buildEccSkeleton(repoRoot) + try { + const disabledResult = runHookRunner( + skeletonRoot, + "session:end:marker", + "scripts/hooks/session-end-marker.js", + "minimal,standard,strict", + { hook_event_name: "SessionEnd", reason: "quit", cwd: skeletonRoot, session_id: "pi-adapter-test" }, + { ECC_DISABLED_HOOKS: "session:end:marker" } + ) + + assert.strictEqual( + disabledResult.error, + undefined, + "hook runner failed to spawn when session:end:marker was listed in " + + `ECC_DISABLED_HOOKS (error: ${disabledResult.error && disabledResult.error.message})` + ) + assert.strictEqual( + disabledResult.status, + 0, + "hook runner exited non-zero for a hook disabled via ECC_DISABLED_HOOKS; a " + + "disabled hook must be skipped cleanly rather than crashing the Pi session " + + `(stderr: ${disabledResult.stderr})` + ) + + const minimalResult = runHookRunner( + skeletonRoot, + "session:end:marker", + "scripts/hooks/session-end-marker.js", + "minimal,standard,strict", + { hook_event_name: "SessionEnd", reason: "quit", cwd: skeletonRoot, session_id: "pi-adapter-test" }, + { ECC_HOOK_PROFILE: "minimal" } + ) + + assert.strictEqual( + minimalResult.error, + undefined, + "hook runner failed to spawn under ECC_HOOK_PROFILE=minimal " + + `(error: ${minimalResult.error && minimalResult.error.message})` + ) + assert.strictEqual( + minimalResult.status, + 0, + "hook runner exited non-zero under ECC_HOOK_PROFILE=minimal; hook-profile " + + `gating must degrade cleanly, not crash the session (stderr: ${minimalResult.stderr})` + ) + } finally { + fs.rmSync(skeletonRoot, { recursive: true, force: true }) + } + }], + + ["additionalContext extraction tolerates non-JSON hook passthrough", () => { + // ---- Behavioral assertions on the LOCAL MIRROR -------------------- + // extractAdditionalContext (defined above) is a hand-copied mirror of + // the real function in .pi/extensions/index.ts, kept because that file + // is TypeScript loaded via jiti and cannot be require()'d from a plain + // Node test. These assertions prove the mirror's behavior; they do NOT + // by themselves prove the shipped adapter still behaves this way. The + // source-text assertions further below read the real function's text + // out of .pi/extensions/index.ts and pin its actual guards, so that a + // real adapter regression fails here even though the mirror (and the + // assertions run against it) would keep passing unchanged. + assert.strictEqual( + extractAdditionalContext('{"hookSpecificOutput":{"additionalContext":"hello"}}'), + "hello", + "expected additionalContext to be extracted from a well-formed hook envelope" + ) + assert.strictEqual( + extractAdditionalContext("plain non-JSON stdout from a disabled hook"), + undefined, + "expected non-JSON stdout (the pass-through case for a disabled hook) to yield " + + "undefined instead of throwing or crashing the session_start handler" + ) + assert.strictEqual( + extractAdditionalContext('{"hookSpecificOutput": malformed'), + undefined, + "expected malformed JSON to yield undefined instead of throwing" + ) + assert.strictEqual( + extractAdditionalContext('{"unrelated":true}'), + undefined, + "expected valid JSON with no hookSpecificOutput.additionalContext field to yield undefined" + ) + assert.strictEqual( + extractAdditionalContext('{"hookSpecificOutput":{"additionalContext":""}}'), + undefined, + "expected an empty-string additionalContext to yield undefined rather than an " + + "empty block being spliced into the system prompt" + ) + + // ---- Source-text assertions on the REAL adapter ------------------- + // Isolate the real extractAdditionalContext function's text out of + // .pi/extensions/index.ts (up to the next top-level function + // declaration) and pin its actual guards. If the adapter's real + // startsWith("{") check, try/catch, hookSpecificOutput?.additionalContext + // read, or non-empty-string requirement ever changes, these fail + // regardless of what the mirror above still does. + const functionStart = extensionSource.indexOf("function extractAdditionalContext") + assert.ok( + functionStart !== -1, + "expected .pi/extensions/index.ts to define a function named extractAdditionalContext" + ) + const nextFunctionStart = extensionSource.indexOf("\nfunction ", functionStart + 1) + const extractContextSource = + nextFunctionStart === -1 + ? extensionSource.slice(functionStart) + : extensionSource.slice(functionStart, nextFunctionStart) + + assert.ok( + /if\s*\(\s*!\s*trimmed\.startsWith\(\s*["'`]\{["'`]\s*\)\s*\)\s*\{\s*return undefined/.test( + extractContextSource + ), + "expected extractAdditionalContext in .pi/extensions/index.ts to early-return " + + "undefined unless the trimmed stdout starts with '{'; this is what makes " + + "non-JSON stdout from a disabled hook a safe pass-through instead of a crash" + ) + assert.ok( + /try\s*\{[\s\S]*?JSON\.parse\(/.test(extractContextSource), + "expected extractAdditionalContext in .pi/extensions/index.ts to parse the " + + "trimmed stdout via JSON.parse(...) inside a try block" + ) + assert.ok( + /catch[^{]*\{\s*return undefined/.test(extractContextSource), + "expected extractAdditionalContext in .pi/extensions/index.ts to catch a " + + "JSON.parse failure and return undefined instead of throwing" + ) + assert.ok( + /hookSpecificOutput\?\.\s*additionalContext/.test(extractContextSource), + "expected extractAdditionalContext in .pi/extensions/index.ts to read " + + "hookSpecificOutput?.additionalContext from the parsed envelope" + ) + assert.ok( + /typeof\s+context\s*===\s*["'`]string["'`]\s*&&\s*context\.trim\(\)/.test(extractContextSource), + "expected extractAdditionalContext in .pi/extensions/index.ts to require a " + + 'non-empty string (typeof context === "string" && context.trim()) before ' + + "returning it, rejecting an empty-string additionalContext" + ) + }], + + // ---- Group 3: code-review fixes ----------------------------------- + + ["EPIPE isolation (source contract): child.stdin has an error listener, and the catch around child.stdin?.end(...) resolves rather than rethrows", () => { + const withoutComments = stripComments(extensionSource) + assert.ok( + withoutComments.includes('child.stdin?.on("error"'), + "expected runEccHook in .pi/extensions/index.ts to register an error listener on " + + 'child.stdin via child.stdin?.on("error", ...) as real code, not just described ' + + "in a comment; stdin.end() writes asynchronously, so a hook that exits before " + + "reading its payload raises an EPIPE `error` event that a try/catch around " + + "child.stdin?.end(...) cannot see, and an unhandled `error` event on a stream " + + "crashes the whole Pi session" + ) + + const runEccHookStart = extensionSource.indexOf("function runEccHook") + assert.ok( + runEccHookStart !== -1, + "expected .pi/extensions/index.ts to define a function named runEccHook" + ) + const nextFunctionStart = extensionSource.indexOf("\nfunction ", runEccHookStart + 1) + const runEccHookSource = + nextFunctionStart === -1 + ? extensionSource.slice(runEccHookStart) + : extensionSource.slice(runEccHookStart, nextFunctionStart) + + const catchMatch = runEccHookSource.match( + /try\s*\{\s*child\.stdin\?\.end\([\s\S]*?\)\)\s*\}\s*catch\s*\(error\)\s*\{([\s\S]*?)\n\s*\}\n/ + ) + assert.ok( + catchMatch, + "expected runEccHook in .pi/extensions/index.ts to wrap child.stdin?.end(...) in " + + "a try { ... } catch (error) { ... } block" + ) + const catchBody = catchMatch[1] + assert.ok( + /resolve\(/.test(catchBody), + "expected the catch around child.stdin?.end(...) in .pi/extensions/index.ts to " + + "call resolve(...); if it rethrows instead, a hook payload write failure " + + "escapes the Promise executor as an unhandled exception instead of degrading " + + "to a warning" + ) + assert.ok( + !/\bthrow\b/.test(catchBody), + "found a rethrow inside the catch around child.stdin?.end(...) in " + + ".pi/extensions/index.ts; this is the exact EPIPE-crashes-the-session " + + "regression the surrounding error handling exists to prevent" + ) + }], + + ["EPIPE isolation (real behavioral proof): a large stdin write to a child that exits without reading it survives as an `error` event or a clean resolution, never an uncaught exception", async () => { + // Mirrors the exact pattern in runEccHook: execFile + process.execPath, an + // `error` listener on child.stdin, and a try/catch around child.stdin.end(...). + // The child below exits immediately without ever reading stdin, so a payload + // larger than the OS pipe buffer (2MB) cannot be written synchronously and + // reliably reproduces the EPIPE this pattern exists to isolate. + const largePayload = "x".repeat(2 * 1024 * 1024) + const uncaughtExceptions = [] + const onUncaughtException = error => uncaughtExceptions.push(error) + process.on("uncaughtException", onUncaughtException) + + let outcome + try { + outcome = await new Promise((resolve, reject) => { + let stdinErrorSeen = false + let childErrorSeen = false + let writeThrew = false + // Safety net only, not a polling race: the assertions below depend on the + // uncaughtException listener, which fires synchronously with the offending + // event if it happens. This just stops the suite from hanging forever if + // the execFile callback never fires for an unrelated reason. + const safetyNet = setTimeout( + () => reject(new Error("execFile callback never fired within the 5.5s safety window")), + 5500 + ) + + const child = execFile( + process.execPath, + ["-e", "process.exit(0)"], + { timeout: 5000, maxBuffer: 1024 * 1024 }, + () => { + clearTimeout(safetyNet) + resolve({ stdinErrorSeen, childErrorSeen, writeThrew }) + } + ) + + child.on("error", () => { + childErrorSeen = true + }) + + child.stdin.on("error", () => { + stdinErrorSeen = true + }) + + try { + child.stdin.end(largePayload) + } catch { + writeThrew = true + } + }) + } finally { + process.off("uncaughtException", onUncaughtException) + } + + assert.strictEqual( + uncaughtExceptions.length, + 0, + "expected writing a 2MB payload to a child that exits before reading stdin to " + + "never raise an uncaughtException; this is exactly the " + + 'EPIPE-crashes-the-Pi-session regression the child.stdin?.on("error", ...) ' + + "listener in runEccHook exists to prevent" + ) + assert.ok( + outcome !== undefined, + "expected the execFile callback to fire and the parent process to survive " + + "writing to a child that never reads its stdin, instead of hanging or crashing" + ) + }], + + ["stale context is cleared at session_start before awaiting the hook, and again after injection in before_agent_start", () => { + const sessionStartIdx = extensionSource.indexOf('pi.on("session_start"') + assert.ok( + sessionStartIdx !== -1, + "expected .pi/extensions/index.ts to register a session_start handler via pi.on(...)" + ) + const beforeAgentStartIdx = extensionSource.indexOf('pi.on("before_agent_start"', sessionStartIdx) + assert.ok( + beforeAgentStartIdx !== -1 && beforeAgentStartIdx > sessionStartIdx, + "expected a before_agent_start handler registered after session_start in .pi/extensions/index.ts" + ) + const sessionShutdownIdx = extensionSource.indexOf('pi.on("session_shutdown"', beforeAgentStartIdx) + assert.ok( + sessionShutdownIdx !== -1 && sessionShutdownIdx > beforeAgentStartIdx, + "expected a session_shutdown handler registered after before_agent_start in .pi/extensions/index.ts" + ) + + const sessionStartSource = stripComments(extensionSource.slice(sessionStartIdx, beforeAgentStartIdx)) + const clearIdx = sessionStartSource.indexOf("pendingContext = undefined") + const hookCallIdx = sessionStartSource.indexOf("await runEccHook(") + assert.ok( + clearIdx !== -1, + "expected the session_start handler in .pi/extensions/index.ts to clear " + + "pendingContext = undefined; without this, a new session start can replay " + + "context captured for a previous session" + ) + assert.ok( + hookCallIdx !== -1, + "expected the session_start handler in .pi/extensions/index.ts to await runEccHook(...)" + ) + assert.ok( + clearIdx < hookCallIdx, + "expected pendingContext = undefined to run BEFORE `await runEccHook(...)` in " + + "the session_start handler; if the clear happens after (or is skipped when " + + "the hook fails), a new session start begun while a previous SessionStart " + + "hook is still running -- or one whose hook later fails -- can replay stale " + + "context captured for the wrong project state" + ) + + const beforeAgentStartSource = stripComments( + extensionSource.slice(beforeAgentStartIdx, sessionShutdownIdx) + ) + // Pin the guarantee (read the value, then clear it, then return) rather + // than one particular spelling of it. The handler injects the context + // inline inside its block instead of copying it to + // a local first; both orders are equivalent in a synchronous handler. + const captureIdx = beforeAgentStartSource.indexOf("") + const clearIdx2 = beforeAgentStartSource.indexOf("pendingContext = undefined") + const returnIdx = beforeAgentStartSource.indexOf("return {") + assert.ok( + captureIdx !== -1, + "expected the before_agent_start handler in .pi/extensions/index.ts to read " + + "pendingContext into an block before clearing it" + ) + assert.ok( + clearIdx2 !== -1, + "expected the before_agent_start handler in .pi/extensions/index.ts to still " + + "clear pendingContext = undefined after reading it for injection; without " + + "this, an already-injected context value would be replayed into a later agent turn" + ) + assert.ok( + returnIdx !== -1, + "expected the before_agent_start handler in .pi/extensions/index.ts to return " + + "an object with an injected systemPrompt" + ) + assert.ok( + captureIdx < clearIdx2, + "expected pendingContext to be read into the injected block BEFORE being " + + "cleared in before_agent_start; clearing first would lose the value before " + + "it can be injected into the system prompt" + ) + assert.ok( + clearIdx2 < returnIdx, + "expected pendingContext = undefined to run BEFORE the return statement in " + + "before_agent_start; if the clear is removed or moved past the return it " + + "never executes, and a later agent turn would replay the same context again" + ) + }], + + ["companion package detection reads Pi's package list (source contract): require.resolve is gone, PI_CODING_AGENT_DIR is honored, and normalizePiPackageName's version-stripping guard is pinned", () => { + // require.resolve is legitimately named in the doc comment above + // listInstalledPiPackages to explain why it was replaced (the same + // "documentation, not a regression" case stripComments exists for -- + // see its own jsdoc above). Strip comments first so this checks real + // code, not prose. + const withoutComments = stripComments(extensionSource) + assert.ok( + !withoutComments.includes("require.resolve"), + "found require.resolve(...) used as executable code in .pi/extensions/index.ts; " + + "Pi installs companion packages under its own config directory " + + "(~/.pi/agent/npm, overridable via PI_CODING_AGENT_DIR), which is not on " + + "Node's module resolution path from this file, so require.resolve reports " + + "every companion as missing no matter what the user actually installed -- " + + "this is the exact defect listInstalledPiPackages was introduced to replace" + ) + assert.ok( + extensionSource.includes("PI_CODING_AGENT_DIR"), + "expected .pi/extensions/index.ts to honor the documented PI_CODING_AGENT_DIR " + + "override when locating Pi's config directory" + ) + + const normalizeStart = extensionSource.indexOf("function normalizePiPackageName") + assert.ok( + normalizeStart !== -1, + "expected .pi/extensions/index.ts to define a function named normalizePiPackageName" + ) + const nextFunctionStart = extensionSource.indexOf("\nfunction ", normalizeStart + 1) + const normalizeSource = + nextFunctionStart === -1 + ? extensionSource.slice(normalizeStart) + : extensionSource.slice(normalizeStart, nextFunctionStart) + + assert.ok( + /typeof\s+entry\s*===\s*["'`]object["'`]\s*\?\s*\(entry\s+as\s*\{\s*source\?:\s*unknown\s*\}\)\.source/.test( + normalizeSource + ), + "expected normalizePiPackageName in .pi/extensions/index.ts to read `source` off " + + "an object entry before normalizing; Pi's settings accept both a bare source " + + 'string and an object carrying it ({ source: "npm:x", skills: [] }), and a ' + + "package filtered that way is just as installed as a plain one -- treating the " + + "object form as unrecognized makes /ecc-doctor report an installed companion as " + + "missing" + ) + assert.ok( + /typeof\s+source\s*!==\s*["'`]string["'`]\s*\|\|\s*!\s*source\.startsWith\(\s*["'`]npm:["'`]\s*\)/.test( + normalizeSource + ), + "expected normalizePiPackageName in .pi/extensions/index.ts to return undefined " + + "for any source that is not a string starting with 'npm:' (git sources and " + + "filesystem paths carry no comparable package name)" + ) + assert.ok( + /spec\s*=\s*source\.slice\(\s*["'`]npm:["'`]\.length\)/.test(normalizeSource), + 'expected normalizePiPackageName in .pi/extensions/index.ts to strip the "npm:" ' + + 'prefix via source.slice("npm:".length)' + ) + assert.ok( + /versionAt\s*=\s*spec\.lastIndexOf\(\s*["'`]@["'`]\s*\)/.test(normalizeSource), + "expected normalizePiPackageName in .pi/extensions/index.ts to locate a " + + 'trailing @version with spec.lastIndexOf("@")' + ) + assert.ok( + /versionAt\s*>\s*0\s*\?\s*spec\.slice\(0,\s*versionAt\)\s*:\s*spec/.test(normalizeSource), + "expected normalizePiPackageName in .pi/extensions/index.ts to only strip at " + + "versionAt when it is greater than 0 (versionAt > 0 ? ... : spec); a scoped " + + "package's leading '@' sits at index 0, so this is what keeps " + + "'@juicesharp/rpiv-todo@1.4.2' from being mangled into an empty name the way " + + 'a naive split("@")[0] would' + ) + }], + + ["companion package name normalization (behavioral mirror): strips a trailing version without breaking a scoped package name", () => { + assert.strictEqual( + normalizePiPackageName("npm:pi-subagents"), + "pi-subagents", + "expected a plain npm entry with no version to normalize to its bare package name" + ) + assert.strictEqual( + normalizePiPackageName("npm:pi-subagents@1.2.3"), + "pi-subagents", + "expected a plain npm entry with a version to have the version stripped" + ) + assert.strictEqual( + normalizePiPackageName("npm:@juicesharp/rpiv-todo"), + "@juicesharp/rpiv-todo", + "expected a versionless scoped npm entry to normalize to its full scoped name" + ) + assert.strictEqual( + normalizePiPackageName("npm:@juicesharp/rpiv-todo@1.4.2"), + "@juicesharp/rpiv-todo", + "expected a scoped npm entry WITH a version to strip only the trailing version " + + 'and keep the scope; a naive split("@")[0] gets this exact case wrong (it ' + + "would return an empty string because the scoped name's leading '@' is not " + + "the version separator)" + ) + assert.strictEqual( + normalizePiPackageName("git:https://github.com/example/pi-plugin.git"), + undefined, + "expected a git source to normalize to undefined; it carries no comparable npm package name" + ) + assert.strictEqual( + normalizePiPackageName("/Users/example/local-pi-plugin"), + undefined, + "expected a filesystem path entry to normalize to undefined" + ) + assert.strictEqual( + normalizePiPackageName(42), + undefined, + "expected a non-string entry to normalize to undefined instead of throwing" + ) + assert.strictEqual( + normalizePiPackageName(""), + undefined, + "expected an empty entry to normalize to undefined" + ) + }], + + ["companion package name normalization (behavioral mirror): an object entry with resource filters resolves to the same name as the bare source string", () => { + assert.strictEqual( + normalizePiPackageName({ source: "npm:pi-subagents", skills: [] }), + "pi-subagents", + "expected the object form Pi documents for filtered packages to resolve to the " + + "same name as the bare string; a user who narrows which resources pi-subagents " + + "contributes still has it installed, and /ecc-doctor exists to report exactly that" + ) + assert.strictEqual( + normalizePiPackageName({ source: "npm:@juicesharp/rpiv-todo@1.4.2", prompts: ["prompts/review.md"] }), + "@juicesharp/rpiv-todo", + "expected an object entry to go through the same version-stripping path as a " + + "string entry, scope intact" + ) + assert.strictEqual( + normalizePiPackageName({ source: "git:github.com/example/pi-plugin@v1" }), + undefined, + "expected an object entry wrapping a git source to stay unrecognized; the source " + + "type decides, not the entry shape" + ) + assert.strictEqual( + normalizePiPackageName({ extensions: ["extensions/*.ts"] }), + undefined, + "expected an object entry with no source field to normalize to undefined instead " + + "of throwing" + ) + assert.strictEqual( + normalizePiPackageName({ source: 42 }), + undefined, + "expected a non-string source to normalize to undefined instead of throwing" + ) + assert.strictEqual( + normalizePiPackageName(null), + undefined, + "expected a null entry to normalize to undefined; typeof null is \"object\", so " + + "this is the case an unguarded object branch would throw on" + ) + }], + + ["companion package detection reads Pi's settings.json (real filesystem): npm entries are recognized, path/git entries are ignored, missing/malformed settings degrade to an empty set", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pi-config-dir-test-")) + try { + const settingsFile = path.join(tmpDir, "settings.json") + fs.writeFileSync( + settingsFile, + JSON.stringify({ + packages: [ + "npm:pi-subagents@2.0.0", + "npm:@juicesharp/rpiv-todo@1.4.2", + "/Users/example/local-pi-plugin", + "git:https://github.com/example/pi-plugin.git", + ], + }) + ) + + const installed = readInstalledPackageNames(settingsFile) + assert.strictEqual( + installed.size, + 2, + "expected only the two npm: entries to be recognized out of a mixed packages " + + `list (got: ${[...installed].join(", ")})` + ) + assert.ok( + installed.has("pi-subagents"), + "expected the plain npm entry with a version to be recognized as pi-subagents" + ) + assert.ok( + installed.has("@juicesharp/rpiv-todo"), + "expected the scoped npm entry with a version to be recognized as @juicesharp/rpiv-todo" + ) + assert.ok( + !installed.has("/Users/example/local-pi-plugin"), + "expected the filesystem path entry to be ignored, not reported as an installed package" + ) + assert.ok( + ![...installed].some(name => name.startsWith("git:")), + "expected the git: source entry to be ignored, not reported as an installed package" + ) + + const missingFile = path.join(tmpDir, "does-not-exist.json") + assert.deepStrictEqual( + readInstalledPackageNames(missingFile), + new Set(), + "expected a missing settings.json to yield an empty set instead of throwing" + ) + + const malformedFile = path.join(tmpDir, "malformed.json") + fs.writeFileSync(malformedFile, "{ this is not valid json") + assert.deepStrictEqual( + readInstalledPackageNames(malformedFile), + new Set(), + "expected a malformed settings.json to yield an empty set instead of throwing" + ) + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }) + } + }], + + // ---- Group 4: engineering-rules injection ------------------------- + + ["PORTABLE_RULE_FILES lists exactly ECC's 7 Pi-portable rule files and excludes the 3 Claude-Code-only ones", () => { + const ruleFiles = parsePortableRuleFiles(extensionSource) + assert.ok( + ruleFiles.length > 0, + "expected to find and parse a PORTABLE_RULE_FILES array literal in .pi/extensions/index.ts" + ) + + assert.deepStrictEqual( + ruleFiles, + [ + "coding-style.md", + "testing.md", + "security.md", + "git-workflow.md", + "patterns.md", + "development-workflow.md", + "code-review.md", + ], + "expected PORTABLE_RULE_FILES in .pi/extensions/index.ts to contain exactly these " + + `7 files (got: ${ruleFiles.join(", ")}); a drift here silently changes which ECC ` + + "engineering rules get injected into Pi's system prompt" + ) + + for (const excluded of ["agents.md", "hooks.md", "performance.md"]) { + assert.ok( + !ruleFiles.includes(excluded), + `found ${excluded} in PORTABLE_RULE_FILES in .pi/extensions/index.ts; ${excluded} ` + + "describes Claude Code primitives Pi does not have (Task/TodoWrite delegation, " + + "Claude Code hook event types, or thinking-budget toggles like Option+T), so " + + "injecting it into Pi's system prompt would instruct the model to use tools " + + "and behaviors that do not exist in Pi" + ) + } + }], + + ["engineering rules are read from rules/common/ joined onto the package root at runtime, and nothing is copied into .pi/", () => { + const withoutComments = stripComments(extensionSource) + assert.ok( + /path\.join\(\s*ECC_ROOT\s*,\s*["'`]rules["'`]\s*,\s*["'`]common["'`]/.test(withoutComments), + "expected .pi/extensions/index.ts to build the rules directory via " + + 'path.join(ECC_ROOT, "rules", "common", ...); rules must be read at runtime from ' + + "the canonical rules/common/ directory of the installed ECC package, which is " + + "the entire point of this adapter feature, not from a path baked in some other way" + ) + + const piDir = path.join(repoRoot, ".pi") + assert.ok( + fs.existsSync(piDir), + `expected a .pi/ directory to exist at ${piDir} for this check to be meaningful` + ) + const piRulesDir = path.join(piDir, "rules") + assert.ok( + !fs.existsSync(piRulesDir), + `found ${piRulesDir} on disk; ECC's engineering rules must be read at runtime from ` + + "the canonical rules/common/ directory and never copied or generated into .pi/ -- " + + "a rules/ directory under .pi/ means that core constraint has been violated" + ) + }], + + ["every file named in PORTABLE_RULE_FILES actually exists under rules/common/ in this repo", () => { + const ruleFiles = parsePortableRuleFiles(extensionSource) + assert.ok( + ruleFiles.length > 0, + "expected to find and parse a PORTABLE_RULE_FILES array literal in .pi/extensions/index.ts" + ) + + const rulesCommonDir = path.join(repoRoot, "rules", "common") + for (const file of ruleFiles) { + const fullPath = path.join(rulesCommonDir, file) + assert.ok( + fs.existsSync(fullPath), + `expected ${fullPath} to exist because it is listed in PORTABLE_RULE_FILES; a ` + + "missing rule file makes loadPortableRules() silently skip it via its " + + "try/catch, so the adapter would inject less engineering-rule coverage into " + + "Pi's system prompt than intended, with no error or warning to notice it by" + ) + } + }], + + ["loadPortableRules() behavioral mirror: concatenates the real rules/common/ files, stays under the cap, and contains markers from several rule files", () => { + // Mirror of loadPortableRules() (read-trim-skip-cap-join loop), guarded by the + // source-text assertions in the two tests above (PORTABLE_RULE_FILES contents + // and the rules/common path) and by the parsed MAX_RULES_BYTES cap below, so a + // drift in the real function's shape fails those tests even if this mirror, + // run here against this repo's actual rule files, still looks correct. + const ruleFiles = parsePortableRuleFiles(extensionSource) + const maxRulesBytes = parseMaxRulesBytes(extensionSource) + assert.ok( + typeof maxRulesBytes === "number" && maxRulesBytes > 0, + "expected to parse a positive numeric MAX_RULES_BYTES constant out of .pi/extensions/index.ts" + ) + + const result = loadPortableRulesMirror(repoRoot, ruleFiles, maxRulesBytes) + + assert.ok( + typeof result === "string" && result.length > 0, + "expected loadPortableRules() to return a non-empty string when run against this " + + "repo's real rules/common/ files; an empty result means the " + + "block would be silently omitted from Pi's system prompt on every turn" + ) + assert.ok( + result.length < maxRulesBytes, + `expected the concatenated rules text (${result.length} chars) to stay under ` + + `MAX_RULES_BYTES (${maxRulesBytes} bytes); exceeding the cap means the ` + + "concatenation loop's stop-before-exceeding-cap guard is not doing its job, and a " + + "large rule-file edit could flood Pi's system prompt" + ) + + for (const marker of ["Immutability", "Minimum Test Coverage", "Secret Management"]) { + assert.ok( + result.includes(marker), + `expected the concatenated rules text to contain "${marker}" (a marker from one ` + + "of the real rules/common/ files); its absence means that file was skipped " + + "(missing, empty, or cut off by the cap) or its content changed in a way that " + + "dropped the section entirely" + ) + } + }], + + ["leakage guard: the text loadPortableRules() would inject contains no Claude-Code-only primitives Pi cannot use", () => { + const ruleFiles = parsePortableRuleFiles(extensionSource) + const maxRulesBytes = parseMaxRulesBytes(extensionSource) + const result = loadPortableRulesMirror(repoRoot, ruleFiles, maxRulesBytes) + assert.ok( + typeof result === "string" && result.length > 0, + "expected a non-empty mirrored rules result for this leakage check to be meaningful" + ) + + for (const leaked of ["TodoWrite", "Option+T", "PostToolUse", "alwaysThinkingEnabled"]) { + assert.ok( + !result.includes(leaked), + `found "${leaked}" in the text loadPortableRules() would inject into Pi's system ` + + "prompt; this is a Claude-Code-only primitive (a tool, hook event type, or " + + "thinking-budget toggle) that would instruct Pi's model to use something that " + + "does not exist in Pi -- exactly the leakage excluding agents.md/hooks.md/" + + "performance.md from PORTABLE_RULE_FILES exists to prevent" + ) + } + }], + + ["/ecc-doctor reports rule files actually loaded, not the allowlist length (source contract)", () => { + assert.ok( + /let\s+cachedRuleFileCount\s*=\s*0/.test(extensionSource), + "expected .pi/extensions/index.ts to track how many rule files actually loaded in a " + + "cachedRuleFileCount counter alongside cachedRules" + ) + assert.ok( + /cachedRuleFileCount\s*=\s*sections\.length/.test(extensionSource), + "expected loadPortableRules in .pi/extensions/index.ts to set cachedRuleFileCount " + + "from sections.length, which is what survived the read failures, the empty-file " + + "skip, and the MAX_RULES_BYTES break" + ) + + const disabledBranch = extensionSource.slice( + extensionSource.indexOf("isDisabledByEnv(process.env.ECC_PI_RULES)"), + extensionSource.indexOf("const sections: string[] = []") + ) + assert.ok( + /cachedRuleFileCount\s*=\s*0/.test(disabledBranch), + "expected the ECC_PI_RULES disable branch of loadPortableRules in " + + ".pi/extensions/index.ts to reset cachedRuleFileCount to 0, so the counter can " + + "never survive from a prior load into a disabled session" + ) + + const statusStart = extensionSource.indexOf("function describeRulesStatus") + assert.ok( + statusStart !== -1, + "expected .pi/extensions/index.ts to define a function named describeRulesStatus" + ) + const nextFunctionStart = extensionSource.indexOf("\nfunction ", statusStart + 1) + const statusSource = + nextFunctionStart === -1 + ? extensionSource.slice(statusStart) + : extensionSource.slice(statusStart, nextFunctionStart) + + assert.ok( + /\$\{cachedRuleFileCount\}\/\$\{PORTABLE_RULE_FILES\.length\}\s+rule file/.test(statusSource), + "expected describeRulesStatus in .pi/extensions/index.ts to report the loaded count " + + "over the allowlist length (`${cachedRuleFileCount}/${PORTABLE_RULE_FILES.length} " + + "rule file(s)`); loadPortableRules silently skips unreadable and empty files and " + + "breaks out of the loop at MAX_RULES_BYTES, so reporting the allowlist length " + + "alone makes an install that loaded 3 of 7 report 7 -- and /ecc-doctor is the one " + + "place a user looks to find a partial install" + ) + }], + + ["isDisabledByEnv() behavioral mirror: recognizes 0/false/off/none/disabled case- and whitespace-insensitively, and the real function reads ECC_PI_RULES", () => { + for (const disabledValue of ["0", "false", "off", "none", "disabled"]) { + assert.strictEqual( + isDisabledByEnvMirror(disabledValue), + true, + `expected isDisabledByEnv("${disabledValue}") to be true` + ) + assert.strictEqual( + isDisabledByEnvMirror(disabledValue.toUpperCase()), + true, + `expected isDisabledByEnv to be case-insensitive for "${disabledValue.toUpperCase()}"` + ) + assert.strictEqual( + isDisabledByEnvMirror(` ${disabledValue} `), + true, + `expected isDisabledByEnv to ignore surrounding whitespace for " ${disabledValue} "` + ) + } + + assert.strictEqual( + isDisabledByEnvMirror(" OFF "), + true, + 'expected isDisabledByEnv(" OFF ") to be true (mixed case AND surrounding whitespace ' + + "at once); a user pasting ECC_PI_RULES=\" OFF \" into a shell profile must still " + + "disable injection" + ) + + for (const enabledValue of [undefined, "", "1", "true", "on", "yes", "TRUE ISH"]) { + assert.strictEqual( + isDisabledByEnvMirror(enabledValue), + false, + `expected isDisabledByEnv(${JSON.stringify(enabledValue)}) to be false; treating an ` + + "unrecognized value as disabled would silently turn off rule injection for anyone " + + "who sets ECC_PI_RULES to something other than the 5 documented off-values" + ) + } + + const withoutComments = stripComments(extensionSource) + assert.ok( + withoutComments.includes("process.env.ECC_PI_RULES"), + "expected .pi/extensions/index.ts to read process.env.ECC_PI_RULES as the env var " + + "that turns rule injection off; a different or renamed env var would silently break " + + "anyone's existing ECC_PI_RULES=off configuration" + ) + }], + + ["before_agent_start wraps rules and context in their tags, consumes pendingContext but never the rules, and returns early with no override when there is nothing to add", () => { + const beforeAgentStartIdx = extensionSource.indexOf('pi.on("before_agent_start"') + assert.ok( + beforeAgentStartIdx !== -1, + "expected .pi/extensions/index.ts to register a before_agent_start handler via pi.on(...)" + ) + const sessionShutdownIdx = extensionSource.indexOf('pi.on("session_shutdown"', beforeAgentStartIdx) + assert.ok( + sessionShutdownIdx !== -1 && sessionShutdownIdx > beforeAgentStartIdx, + "expected a session_shutdown handler registered after before_agent_start in .pi/extensions/index.ts" + ) + + const handlerSource = stripComments(extensionSource.slice(beforeAgentStartIdx, sessionShutdownIdx)) + + assert.ok( + handlerSource.includes(""), + "expected the before_agent_start handler in .pi/extensions/index.ts to wrap " + + "injected rules in an tag" + ) + assert.ok( + handlerSource.includes(""), + "expected the before_agent_start handler in .pi/extensions/index.ts to wrap the " + + "session context in an tag" + ) + + const contextPushIdx = handlerSource.indexOf("") + const clearIdx = handlerSource.indexOf("pendingContext = undefined", contextPushIdx) + assert.ok( + contextPushIdx !== -1 && clearIdx !== -1 && clearIdx > contextPushIdx, + "expected before_agent_start to clear pendingContext = undefined after using it to " + + "build the block; without this, the same one-shot session " + + "context would be replayed into every later agent turn instead of being consumed once" + ) + + assert.ok( + !/\bcachedRules\s*=\s*(undefined|null)/.test(handlerSource) && + !/\brules\s*=\s*(undefined|null)/.test(handlerSource), + "found code in the before_agent_start handler that resets the loaded rules value; " + + "engineering rules describe standing policy and must be re-applied on EVERY turn " + + "(unlike the one-shot pendingContext), so nothing in this handler may consume or " + + "clear them the way pendingContext is consumed" + ) + + assert.ok( + /if\s*\(\s*additions\.length\s*===\s*0\s*\)\s*\{\s*return\s*\}/.test(handlerSource), + "expected before_agent_start to return early with a bare `return` (no systemPrompt " + + "override) when there is nothing to add; without this guard, a turn with no rules " + + "and no pending context would still return a rebuilt systemPrompt instead of " + + "leaving Pi's original systemPrompt untouched" + ) + + const earlyReturnIdx = handlerSource.indexOf("if (additions.length === 0)") + const overrideReturnIdx = handlerSource.indexOf("return { systemPrompt") + assert.ok( + earlyReturnIdx !== -1 && overrideReturnIdx !== -1 && earlyReturnIdx < overrideReturnIdx, + "expected the early-return-when-nothing-to-add guard to appear before the " + + "systemPrompt-override return in before_agent_start" + ) + }], + ] + + for (const [name, fn] of tests) { + if (await runTest(name, fn)) { + passed += 1 + } else { + failed += 1 + } + } + + console.log(`\nPassed: ${passed}`) + console.log(`Failed: ${failed}`) + process.exit(failed > 0 ? 1 : 0) +} + +main() diff --git a/tests/pi/pi-package-manifest.test.js b/tests/pi/pi-package-manifest.test.js new file mode 100644 index 000000000..35379e943 --- /dev/null +++ b/tests/pi/pi-package-manifest.test.js @@ -0,0 +1,335 @@ +/** + * 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()