From eb4970265169fec82371c92f615e2e133d875e27 Mon Sep 17 00:00:00 2001 From: Renan Olovics Date: Wed, 12 Aug 2026 18:41:33 -0400 Subject: [PATCH] feat: thin Pi adapter mounting ECC's canonical skills and commands (#2759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add thin Pi adapter mounting ECC's canonical skills and commands Adds first-class Pi (@earendil-works/pi-coding-agent) support as a thin adapter layer, following the maintainer review on #2352. ECC's canonical assets stay the single source of truth: nothing is copied or generated under .pi/. The `pi` manifest in package.json points Pi directly at `skills/` and `commands/`. No transformation is needed — ECC's SKILL.md files already follow the Agent Skills standard Pi implements, and ECC's command frontmatter is already Pi's prompt-template format. .pi/extensions/index.ts is the only adapter logic. It: - uses Pi's documented `pi.on(...)` lifecycle, not an undocumented event bus - resolves hook scripts from the installed package via `__dirname`, never `process.cwd()`, so global installs work from any project directory - runs hooks with `execFile(process.execPath, [...])` and no shell, so paths containing spaces or shell metacharacters are safe - invokes hooks through ECC's own `run-with-flags.js`, so `ECC_HOOK_PROFILE` and `ECC_DISABLED_HOOKS` keep gating hooks under Pi - runs hooks in the user's project directory so project detection stays correct, while resolving the scripts themselves package-relative - injects the SessionStart hook's `additionalContext` into the system prompt on the next `before_agent_start` - isolates hook failures behind a timeout and an output limit - registers `/ecc-doctor` for install diagnostics Registers `.pi` in the platform-configs install module and adds a Pi row to the harness adapter compliance matrix. Verified against Pi 0.84.1: a global `pi install` exposes 285 skills and 94 commands resolved from `skills/` and `commands/`, plus `/ecc-doctor`, with no generated copies. Scope deliberately excludes subagents, chains, approval gates, todos, profiles, and MCP; ECC works in Pi without any companion package. * fix: address review findings on the Pi adapter Bot review on #2759 surfaced two real runtime defects and several hardening gaps. Runtime fixes: - Attach an `error` listener to the hook child's stdin. `stdin.end()` writes asynchronously, so a hook that exits, short-circuits, or is killed by the timeout before reading the payload raises EPIPE as an `error` event that the surrounding try/catch cannot see. Unhandled, that event would terminate the Pi session and break the isolation guarantee the adapter documents. - Clear `pendingContext` at the top of the `session_start` handler. Pi can start a new session (/new, /resume, /fork) before `before_agent_start` consumes the previous value; if the newer hook then failed, the next agent start received context describing a different session's project state. - Replace `require.resolve` companion detection with a read of Pi's own `packages` list, honoring `PI_CODING_AGENT_DIR`. Pi installs packages under its config directory, which is not on Node's module resolution path from the extension, so the previous check reported every companion as missing no matter what was installed. Compliance matrix: remove internal semicolons and a trailing period from the Pi record's list entries. The renderer joins entries with "; ", so those characters split one entry into several in the rendered cell. Tests: run profile gating against the temp skeleton instead of the real checkout so it cannot leave marker artifacts behind; count files under .pi/ by walking disk rather than git, so untracked copies cannot bypass the regression guard; allow negated phrasing in the README heuristic; pin the adapter's real parser guards with source assertions so the local mirrors cannot silently diverge; add coverage for EPIPE isolation, stale context clearing, and companion detection. * docs: point users at existing companion Pi packages instead of bundling them Every capability listed as out of scope is already provided by a maintained community Pi package: pi-subagents, @juicesharp/rpiv-ask-user-question, @juicesharp/rpiv-todo, and pi-mcp-adapter for MCP. Pi supports pulling other pi packages in via dependencies plus bundledDependencies, but this adapter deliberately does not. Bundling would ship third-party code that executes with full user permissions in every ECC install, turn optional capabilities into mandatory ones, and add four fast-moving pins to maintain. Instead /ecc-doctor now prints the exact `pi install npm:` command for each companion it does not find, so adopting one stays a deliberate user choice. Also corrects the MCP claim: Pi core has no MCP surface by design, but the community pi-mcp-adapter package adds one. This adapter neither installs nor verifies it, and ECC's MCP reference configs are not known to be compatible. * docs: ECC's MCP configs work in Pi through pi-mcp-adapter, verbatim Tested rather than assumed. The community pi-mcp-adapter package reads the standard mcpServers format from .mcp.json and ~/.config/mcp/mcp.json, which is exactly the format ECC already uses in .mcp.json and mcp-configs/mcp-servers.json. Verified against pi-mcp-adapter 2.21.2 in an isolated PI_CODING_AGENT_DIR: copying mcp-configs/mcp-servers.json to a project's .mcp.json registers Pi's `mcp` tool and `/mcp` command with all 35 ECC servers discovered, coexisting with this adapter's /ecc-doctor. No translation layer and no ECC change are needed, so this stops being a limitation and becomes documentation. Recorded caveats: the adapter's first run against a new config performs initialization that blocks in non-interactive mode, and only discovery was verified, not live tool invocation. ECC still neither installs nor depends on the package. * feat: inject ECC's canonical engineering rules into Pi's system prompt ECC's rules were the one durable asset the adapter did not deliver: skills and commands reached Pi in full, but the 122 rule files that carry ECC's coding style, testing, security, git workflow, and code-review standards did not, so ECC in Pi was a library of skills rather than a set of enforced standards. Rules are read at runtime from the canonical rules/common/ directory of the installed package and appended to the system prompt inside an block. Nothing is copied or generated under .pi/, which keeps the single-source-of-truth constraint this PR exists to satisfy. Injection reuses the before_agent_start path already built for session context, so no new lifecycle mapping is introduced. Rules are re-applied every turn because they are standing policy, while the session context stays one-shot and is consumed on first use. agents.md, hooks.md, and performance.md are excluded: they describe Claude Code primitives Pi does not have (Task/TodoWrite delegation, Claude hook event types, thinking-budget toggles), so injecting them would point the model at tools that are not there. A test asserts they stay excluded, and a leakage test asserts none of those primitives appear in the injected text. Language-specific rules under rules// are out of scope for this first adapter. Injection is bounded by MAX_RULES_BYTES and can be disabled with ECC_PI_RULES, following ECC's existing off-switch convention. /ecc-doctor reports the state and injected size. Measured on this repo: 7 files, 12,361 characters, roughly 3k tokens. Also replaces a Function() call in the test helper with direct arithmetic, and repins a stale assertion that pinned one spelling of the context handoff rather than the guarantee (read before clear, clear before return). * fix: /ecc-doctor misreported filtered packages and partial rule installs Two reporting defects in /ecc-doctor, the command whose whole job is telling a user what is actually installed. Pi's settings accept a `packages` entry in two shapes: the bare source string ("npm:pi-subagents") and an object carrying that source alongside resource filters ({ source: "npm:pi-subagents", skills: [] }). normalizePiPackageName only recognized the string, so a user who narrowed which resources a companion contributes was told the companion was not installed, along with an install command for something already present. The source type still decides whether a name is comparable, so an object wrapping a git source or a path stays unrecognized exactly as before. loadPortableRules drops rule files it cannot read, drops empty ones, and stops at MAX_RULES_BYTES, but describeRulesStatus reported PORTABLE_RULE_FILES.length regardless. A partial install that loaded 3 of 7 files reported "7 rule file(s)" to the one command a user runs to find a partial install. The loaded count is now tracked next to the cache and reported as a ratio, with the shortfall named. Also reconciles the Notes bullet in .pi/README.md, which still called MCP out of scope after the MCP section landed documenting that ECC's configs load in Pi through pi-mcp-adapter. Both defects were reported by CodeRabbit and verified against Pi's own packages.md before fixing. Adapter tests go from 24 to 26; the two source contracts that pinned the previous spellings now pin the new guards, so the object-form unwrapping and the loaded-count reporting cannot be silently reverted. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- .pi/README.md | 190 +++ .pi/extensions/index.ts | 646 ++++++++ .../harness-adapter-compliance.md | 1 + manifests/install-modules.json | 1 + package.json | 12 + scripts/lib/harness-adapter-compliance.js | 34 + tests/pi/pi-extension-adapter.test.js | 1352 +++++++++++++++++ tests/pi/pi-package-manifest.test.js | 335 ++++ 8 files changed, 2571 insertions(+) create mode 100644 .pi/README.md create mode 100644 .pi/extensions/index.ts create mode 100644 tests/pi/pi-extension-adapter.test.js create mode 100644 tests/pi/pi-package-manifest.test.js 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()