From b64b80c813e455df3e118e1805c4ed6422c69dea Mon Sep 17 00:00:00 2001 From: Hovhannes Tsakanyan Date: Thu, 23 Apr 2026 00:17:28 +0400 Subject: [PATCH 1/3] feat: add pi tool-result hook and windows installer --- .agent/harness/hooks/pi_post_tool.py | 144 +++++++++++++++++++++++++++ README.md | 2 +- adapters/pi/AGENTS.md | 4 +- adapters/pi/README.md | 21 +++- adapters/pi/memory-hook.ts | 110 ++++++++++++++++++++ docs/per-harness/pi.md | 17 +++- install.ps1 | 34 ++++++- install.sh | 3 + 8 files changed, 328 insertions(+), 7 deletions(-) create mode 100644 .agent/harness/hooks/pi_post_tool.py create mode 100644 adapters/pi/memory-hook.ts diff --git a/.agent/harness/hooks/pi_post_tool.py b/.agent/harness/hooks/pi_post_tool.py new file mode 100644 index 0000000..60c5500 --- /dev/null +++ b/.agent/harness/hooks/pi_post_tool.py @@ -0,0 +1,144 @@ +#!/usr/bin/env python3 +"""Process Pi Coding Agent tool_result events into episodic memory entries. + +Pi exposes a project-local extension system. The adapter installs +`.pi/extensions/memory-hook.ts`, which forwards every `tool_result` event to +this script via stdin. We normalize Pi's event shape into the same +tool_input/tool_response structure used by the Claude Code hook so the +importance scoring, reflection generation, and success classification stay +consistent across harnesses. +""" +import json +import os +import sys + +HERE = os.path.dirname(os.path.abspath(__file__)) +AGENT_ROOT = os.path.abspath(os.path.join(HERE, "..", "..")) + +sys.path.insert(0, os.path.join(AGENT_ROOT, "harness")) +sys.path.insert(0, os.path.join(AGENT_ROOT, "tools")) + +from hooks.post_execution import log_execution # noqa: E402 +from hooks.on_failure import on_failure # noqa: E402 +import hooks.claude_code_post_tool as cc # noqa: E402 + + +_PI_TO_CANONICAL = { + "bash": "Bash", + "edit": "Edit", + "write": "Write", + "read": "Read", + "grep": "Grep", + "find": "Find", + "ls": "LS", + "task": "Task", + "todowrite": "TodoWrite", + "webfetch": "WebFetch", +} + + +def _tool_name(name: str) -> str: + if not isinstance(name, str): + return "Unknown" + lowered = name.strip().lower() + return _PI_TO_CANONICAL.get(lowered, name[:1].upper() + name[1:]) + + +def _extract_text(content) -> str: + if isinstance(content, str): + return content[:500] + if not isinstance(content, list): + return "" + texts = [] + for item in content: + if isinstance(item, dict) and item.get("type") == "text": + text = item.get("text") + if isinstance(text, str) and text: + texts.append(text) + return " ".join(texts)[:500] + + +def _normalize_response(event: dict) -> dict: + """Map Pi tool_result shape to the Claude hook's response schema.""" + resp: dict = {"is_error": bool(event.get("isError", False))} + details = event.get("details") + content = event.get("content") + + if isinstance(details, dict): + if isinstance(details.get("output"), str): + resp["output"] = details["output"][:500] + if isinstance(details.get("stdout"), str): + resp["stdout"] = details["stdout"][:500] + if isinstance(details.get("stderr"), str): + resp["stderr"] = details["stderr"][:300] + if isinstance(details.get("error"), str): + resp["error"] = details["error"][:300] + if isinstance(details.get("text"), str): + resp["text"] = details["text"][:500] + if "exitCode" in details: + resp["exit_code"] = details.get("exitCode") + if "cancelled" in details: + resp["interrupted"] = bool(details.get("cancelled")) + if "truncated" in details: + resp["truncated"] = bool(details.get("truncated")) + resp["details"] = details + + text = _extract_text(content) + if text: + resp.setdefault("output", text) + resp["content"] = [{"type": "text", "text": text}] + + if not any(k in resp for k in ("output", "stdout", "text", "content")): + if isinstance(details, str): + resp["output"] = details[:500] + elif details is not None: + resp["output"] = json.dumps(details, default=str)[:500] + + return resp + + +def main() -> None: + try: + raw = sys.stdin.read() + payload = json.loads(raw) if raw.strip() else {} + except (json.JSONDecodeError, OSError): + payload = {} + + tool_name = _tool_name(payload.get("tool_name") or "Unknown") + tool_input = payload.get("tool_input") or {} + if not isinstance(tool_input, dict): + tool_input = {"raw": str(tool_input)} + tool_response = _normalize_response(payload) + + success = cc._is_success(tool_name, tool_input, tool_response) + importance = cc._importance(tool_name, json.dumps(tool_input)) + action = cc._action_label(tool_name, tool_input) + reflection = cc._reflection(tool_name, tool_input, tool_response, success) + detail = cc._detail(tool_name, tool_input, tool_response, success) + + pscore = cc._pain_score(importance, success) + if success: + log_execution( + skill_name="pi", + action=action, + result=detail, + success=True, + reflection=reflection, + importance=importance, + confidence=0.7, + pain_score=pscore, + ) + else: + on_failure( + skill_name="pi", + action=action, + error=reflection, + context=detail, + confidence=0.7, + importance=importance, + pain_score=pscore, + ) + + +if __name__ == "__main__": + main() diff --git a/README.md b/README.md index 46a00dd..d3db66b 100644 --- a/README.md +++ b/README.md @@ -224,7 +224,7 @@ verify_codex_fixes.py # v0.8.0 regression checks (33 checks) | **OpenCode** | `AGENTS.md` + `opencode.json` | partial (permission rules) | | **OpenClaw** | `AGENTS.md` (auto-injected) + per-project `openclaw agents add --workspace` | varies by fork | | **Hermes Agent** | `AGENTS.md` (agentskills.io compatible) | partial (own memory) | -| **Pi Coding Agent** | `AGENTS.md` + `.pi/skills/` | no (extension system) | +| **Pi Coding Agent** | `AGENTS.md` + `.pi/skills/` + `.pi/extensions/` | yes (`tool_result` event) | | **Standalone Python** | `run.py` (any LLM) | yes (full control) | | **Antigravity** | `ANTIGRAVITY.md` | yes (system context) | diff --git a/adapters/pi/AGENTS.md b/adapters/pi/AGENTS.md index 71d6145..dfe4a0b 100644 --- a/adapters/pi/AGENTS.md +++ b/adapters/pi/AGENTS.md @@ -45,4 +45,6 @@ them. - System prompt override: put `.pi/SYSTEM.md` at project root if you want to replace pi's default system prompt entirely. - Prompt templates go in `.pi/prompts/`. -- TypeScript extensions go in `.pi/extensions/` (advanced). +- TypeScript extensions go in `.pi/extensions/` (advanced). This adapter + installs `memory-hook.ts`, which logs `tool_result` events to episodic + memory automatically. diff --git a/adapters/pi/README.md b/adapters/pi/README.md index add43a5..69fedd1 100644 --- a/adapters/pi/README.md +++ b/adapters/pi/README.md @@ -10,6 +10,11 @@ on top so you keep one knowledge base even if you later swap harnesses. ./install.sh pi ``` +Or on Windows PowerShell: +```powershell +.\install.ps1 pi C:\path\to\your-project +``` + Then install pi itself: ```bash npm install -g @mariozechner/pi-coding-agent @@ -23,8 +28,11 @@ npm install -g @mariozechner/pi-coding-agent - `.pi/skills/` → symlink to `.agent/skills/`. Pi scans this path at startup. Symlink means there's one source of truth; customize under `.agent/skills/` and pi sees it immediately. -- `.pi/` directory is created even if empty — ready for optional pi - extensions, prompt templates, and `.pi/SYSTEM.md` overrides. +- `.pi/extensions/memory-hook.ts` — project-local extension that listens + to Pi's `tool_result` event and appends episodic entries via the shared + agentic-stack hook path. +- `.pi/` directory is created for skills, extensions, prompt templates, + and optional `.pi/SYSTEM.md` overrides. ## Coexisting with other adapters Pi, hermes, and opencode all read `AGENTS.md`. You can install any @@ -35,6 +43,15 @@ subsequent installs are no-ops on that file. In pi: ask "what's in my LESSONS file?" — it should read `.agent/memory/semantic/LESSONS.md`. +Run one tool call, then inspect the episodic log: + +```bash +tail -1 .agent/memory/episodic/AGENT_LEARNINGS.jsonl +``` + +You should see a `skill` of `pi` and an `action` derived from the tool +that just ran. + ## Optional If pi's default system prompt doesn't fit your workflow, drop a `.pi/SYSTEM.md` at project root. Pi uses it as a complete override. diff --git a/adapters/pi/memory-hook.ts b/adapters/pi/memory-hook.ts new file mode 100644 index 0000000..4f3633a --- /dev/null +++ b/adapters/pi/memory-hook.ts @@ -0,0 +1,110 @@ +import type { ExtensionAPI } from "@mariozechner/pi-coding-agent"; +import { existsSync } from "node:fs"; +import { spawn } from "node:child_process"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const EXTENSION_DIR = path.dirname(fileURLToPath(import.meta.url)); +const PROJECT_ROOT = path.resolve(EXTENSION_DIR, "..", ".."); +const HOOK_SCRIPT = path.join( + PROJECT_ROOT, + ".agent", + "harness", + "hooks", + "pi_post_tool.py", +); + +let warnedMissingHook = false; +let warnedMissingPython = false; +let warnedHookFailure = false; + +type PythonCandidate = { + command: string; + args: string[]; +}; + +function pythonCandidates(): PythonCandidate[] { + const envPy = process.env.AGENT_PYTHON?.trim(); + const out: PythonCandidate[] = []; + if (envPy) out.push({ command: envPy, args: [] }); + out.push({ command: "python3", args: [] }); + out.push({ command: "python", args: [] }); + out.push({ command: "py", args: ["-3"] }); + return out; +} + +function tryRun( + candidate: PythonCandidate, + payload: Record, +): Promise<"ok" | "spawn-error" | "hook-failure"> { + return new Promise((resolve) => { + const child = spawn( + candidate.command, + [...candidate.args, HOOK_SCRIPT], + { + cwd: PROJECT_ROOT, + stdio: ["pipe", "ignore", "ignore"], + }, + ); + child.on("error", () => resolve("spawn-error")); + child.on("spawn", () => { + child.stdin.end(JSON.stringify(payload)); + }); + child.on("close", (code) => { + resolve(code === 0 ? "ok" : "hook-failure"); + }); + }); +} + +async function runHook(payload: Record) { + if (!existsSync(HOOK_SCRIPT)) return "missing-hook"; + for (const candidate of pythonCandidates()) { + const result = await tryRun(candidate, payload); + if (result === "ok") return "ok"; + if (result === "hook-failure") return "hook-failure"; + } + return "missing-python"; +} + +export default function (pi: ExtensionAPI) { + pi.on("tool_result", async (event, ctx) => { + const payload = { + tool_name: event.toolName, + tool_input: event.input ?? {}, + content: event.content ?? [], + details: event.details ?? {}, + isError: event.isError ?? false, + }; + + try { + const result = await runHook(payload); + if (result === "missing-hook" && !warnedMissingHook) { + warnedMissingHook = true; + ctx.ui.notify( + "agentic-stack pi memory hook missing; automatic episodic logging disabled.", + "warning", + ); + } else if (result === "missing-python" && !warnedMissingPython) { + warnedMissingPython = true; + ctx.ui.notify( + "agentic-stack pi memory hook: python3/python not found; automatic episodic logging disabled.", + "warning", + ); + } else if (result === "hook-failure" && !warnedHookFailure) { + warnedHookFailure = true; + ctx.ui.notify( + "agentic-stack pi memory hook failed; continuing without automatic episodic logging.", + "warning", + ); + } + } catch { + if (!warnedHookFailure) { + warnedHookFailure = true; + ctx.ui.notify( + "agentic-stack pi memory hook failed; continuing without automatic episodic logging.", + "warning", + ); + } + } + }); +} diff --git a/docs/per-harness/pi.md b/docs/per-harness/pi.md index d2653aa..51ff9e4 100644 --- a/docs/per-harness/pi.md +++ b/docs/per-harness/pi.md @@ -11,6 +11,8 @@ and a TypeScript extension system. Our adapter layers the portable - `.pi/` directory - `.pi/skills` symlinked to `.agent/skills` (falls back to copy on platforms without symlinks, e.g. Windows without developer mode) +- `.pi/extensions/memory-hook.ts`, auto-discovered by pi at startup and + wired to the `tool_result` event for episodic logging ## Install ```bash @@ -19,18 +21,31 @@ npm install -g @mariozechner/pi-coding-agent pi ``` +On Windows PowerShell: +```powershell +.\install.ps1 pi C:\path\to\your-project +npm install -g @mariozechner/pi-coding-agent +pi +``` + ## How it works - Pi loads `AGENTS.md` (or `CLAUDE.md`) from `~/.pi/agent/` and walks the current directory up to the filesystem root, aggregating context. - Skills at `.pi/skills//SKILL.md` use the same frontmatter-plus -body shape as agentskills.io and our `.agent/skills/` layout. -- Pi extensions live in `.pi/extensions/` (TypeScript, optional). +- Pi extensions live in `.pi/extensions/` (TypeScript). The adapter's + `memory-hook.ts` listens to `tool_result` and forwards each tool result + to `.agent/harness/hooks/pi_post_tool.py`, which reuses the same + scoring / reflection logic as the Claude Code hook. ## Troubleshooting - If pi doesn't see your skills, run `pi skills list` — it should print entries from `.pi/skills/`. If the directory is a broken symlink, re-run `./install.sh pi` to rebuild. +- If episodic logging stays empty, make sure Python is available as + `python3`, `python`, or via `AGENT_PYTHON`, since the extension shells + out to `.agent/harness/hooks/pi_post_tool.py`. - On Windows without symlink support, the installer copies `.agent/skills/` instead. Changes to `.agent/skills/` won't propagate — re-run the installer to sync. diff --git a/install.ps1 b/install.ps1 index 2207705..b80c467 100644 --- a/install.ps1 +++ b/install.ps1 @@ -1,6 +1,6 @@ # install.ps1 — Windows PowerShell installer (parallel to install.sh) # Usage: .\install.ps1 [target-dir] [-Yes] [-Reconfigure] [-Force] -# adapter-name: claude-code | cursor | windsurf | opencode | openclaw | hermes | standalone-python | antigravity +# adapter-name: claude-code | cursor | windsurf | opencode | openclaw | hermes | pi | standalone-python | antigravity # target-dir: where your project lives (default: current dir) # -Yes accept all wizard defaults (safe for CI) # -Reconfigure re-run the wizard on an existing project @@ -24,7 +24,7 @@ $Here = Split-Path -Parent $MyInvocation.MyCommand.Path $ValidAdapters = @( 'claude-code', 'cursor', 'windsurf', - 'opencode', 'openclaw', 'hermes', + 'opencode', 'openclaw', 'hermes', 'pi', 'standalone-python', 'antigravity' ) if ($Adapter -notin $ValidAdapters) { @@ -140,6 +140,36 @@ switch ($Adapter) { 'hermes' { Copy-Item (Join-Path $Src 'AGENTS.md') (Join-Path $TargetDir 'AGENTS.md') -Force } + 'pi' { + $agentsMd = Join-Path $TargetDir 'AGENTS.md' + if (Test-Path $agentsMd -PathType Leaf) { + Write-Host " ~ $agentsMd already exists — skipping (pi reads whatever is there)" + } else { + Copy-Item (Join-Path $Src 'AGENTS.md') $agentsMd -Force + Write-Host " + AGENTS.md" + } + + $piDir = Join-Path $TargetDir '.pi' + New-Item -ItemType Directory -Path $piDir -Force | Out-Null + + $skillsSrc = Join-Path $TargetAgent 'skills' + $skillsDst = Join-Path $piDir 'skills' + if (Test-Path $skillsDst) { + Remove-Item -LiteralPath $skillsDst -Recurse -Force + } + try { + New-Item -ItemType SymbolicLink -Path $skillsDst -Target $skillsSrc -ErrorAction Stop | Out-Null + Write-Host " + .pi/skills -> $skillsSrc" + } catch { + Copy-Item -Path $skillsSrc -Destination $skillsDst -Recurse + Write-Host " + .pi/skills (copy; symlink not supported here)" + } + + $extensionsDir = Join-Path $piDir 'extensions' + New-Item -ItemType Directory -Path $extensionsDir -Force | Out-Null + Copy-Item (Join-Path $Src 'memory-hook.ts') (Join-Path $extensionsDir 'memory-hook.ts') -Force + Write-Host " + .pi/extensions/memory-hook.ts" + } 'standalone-python' { Copy-Item (Join-Path $Src 'run.py') (Join-Path $TargetDir 'run.py') -Force } diff --git a/install.sh b/install.sh index b14ca78..0d5f414 100755 --- a/install.sh +++ b/install.sh @@ -144,6 +144,9 @@ case "$ADAPTER" in cp -R "$SKILLS_SRC" "$TARGET/.pi/skills" echo " + .pi/skills (copy; symlink not supported here)" fi + mkdir -p "$TARGET/.pi/extensions" + cp "$SRC/memory-hook.ts" "$TARGET/.pi/extensions/memory-hook.ts" + echo " + .pi/extensions/memory-hook.ts" ;; standalone-python) cp "$SRC/run.py" "$TARGET/run.py" From d7907699cbeb75cba0631c2d922afbdb7970e7bc Mon Sep 17 00:00:00 2001 From: codejunkie99 Date: Thu, 23 Apr 2026 13:22:54 +0530 Subject: [PATCH 2/3] review: address pi tool-result hook review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-model review (Claude + Codex adversarial) flagged 4 issues that would either lose user data on Windows or silently degrade episodic memory quality. 1. install.ps1:157-159 — DATA LOSS on Windows re-install. `Remove-Item -LiteralPath $skillsDst -Recurse -Force` on PowerShell 5.1 (default Windows shell) traverses INTO a symlink target and deletes its contents before removing the link. A second `install.ps1 pi` run would wipe .agent/skills/. Detect ReparsePoint via Get-Item.Attributes BEFORE Remove-Item; use .NET Directory.Delete($path, false) on links so only the link is removed, never the target. 2. adapters/pi/memory-hook.ts — no subprocess timeout, hangs Pi. `await runHook(payload)` is awaited inside Pi's tool_result handler; a stuck Python child blocks Pi's event loop forever. Add 3s default timeout (overridable via $AGENT_HOOK_TIMEOUT_MS), kill the child on timeout, surface `timeout` as a separate result kind. 3. adapters/pi/memory-hook.ts — stderr was dropped, failures undiagnosable. Switch stdio to capture stderr (bounded to 4KB to avoid memory blowup on a wedged hook) and surface the first line in the failure notification. 4. .agent/harness/hooks/pi_post_tool.py — Pi sends tool_input with camelCase keys (filePath, oldString, newString), but the shared cc.* helpers (action_label, reflection, importance) expect Claude Code's snake_case keys. Without normalization, every Edit/Write logged by Pi degraded to "edit: ?" / "Edited ?" with empty detail. Add a Pi→canonical input key map applied in _normalize_input(). 5. .agent/harness/hooks/pi_post_tool.py — fail-open on malformed payload was logging bogus "Unknown success" entries (Codex's High #1). If Pi ever changes the event shape or sends invalid JSON, episodic memory got polluted with noise instead of a real signal. Add a _emit_malformed() path that records an explicit `hook:malformed_payload` failure entry with a 200-char excerpt of the offending payload — visible in AGENT_LEARNINGS.jsonl as a real error, not noise. Smoke-tested: - empty payload → `hook:malformed_payload | empty payload` - malformed JSON → `hook:malformed_payload | json decode error: ...` - Pi camelCase Edit (filePath/oldString/newString) → produces correct `edit: /tmp/x.txt` action label and `Edited /tmp/x.txt: replaced 'a' with 'b'` reflection (was: `edit: ?` / `Edited ?`) - well-formed bash success → unchanged behavior Codex's concurrent-write concern (Codex #3) and the rsync-style sync for .pi/skills (Codex #5) are NOT addressed here — they apply to pre-existing infrastructure (post_execution.py write semantics, pi's existing symlink path) and are scoped as separate follow-ups. --- .agent/harness/hooks/pi_post_tool.py | 88 ++++++++++++++++++++++++-- adapters/pi/memory-hook.ts | 95 ++++++++++++++++++++++++---- install.ps1 | 17 ++++- 3 files changed, 180 insertions(+), 20 deletions(-) diff --git a/.agent/harness/hooks/pi_post_tool.py b/.agent/harness/hooks/pi_post_tool.py index 60c5500..005c8f3 100644 --- a/.agent/harness/hooks/pi_post_tool.py +++ b/.agent/harness/hooks/pi_post_tool.py @@ -33,7 +33,31 @@ "ls": "LS", "task": "Task", "todowrite": "TodoWrite", + "todo_write": "TodoWrite", "webfetch": "WebFetch", + "web_fetch": "WebFetch", +} + +# Pi tends to use camelCase for tool_input keys; the shared +# claude_code_post_tool helpers (cc._action_label, cc._reflection, +# cc._importance) expect the snake_case keys Claude Code sends. +# Normalize a known set so action labels and reflections come out +# meaningful instead of degrading to "edit: ?" / "Edited ?". +_PI_INPUT_KEY_MAP = { + "filePath": "file_path", + "filepath": "file_path", + "path": "file_path", + "oldString": "old_string", + "oldstring": "old_string", + "newString": "new_string", + "newstring": "new_string", + "content": "content", + "command": "command", + "todos": "todos", + "todo": "todos", + "url": "url", + "pattern": "pattern", + "query": "query", } @@ -41,7 +65,23 @@ def _tool_name(name: str) -> str: if not isinstance(name, str): return "Unknown" lowered = name.strip().lower() - return _PI_TO_CANONICAL.get(lowered, name[:1].upper() + name[1:]) + if lowered in _PI_TO_CANONICAL: + return _PI_TO_CANONICAL[lowered] + if "_" in name: + return "".join(part[:1].upper() + part[1:] for part in name.split("_") if part) + return name[:1].upper() + name[1:] + + +def _normalize_input(raw) -> dict: + """Map Pi tool_input keys to the snake_case shape cc.* helpers expect.""" + if not isinstance(raw, dict): + if raw is None: + return {} + return {"raw": str(raw)} + out: dict = {} + for k, v in raw.items(): + out[_PI_INPUT_KEY_MAP.get(k, k)] = v + return out def _extract_text(content) -> str: @@ -97,17 +137,51 @@ def _normalize_response(event: dict) -> dict: return resp +def _emit_malformed(reason: str, raw_excerpt: str) -> None: + """Record an explicit failure entry instead of silently logging a bogus + 'Unknown success'. If Pi changes payload shape or sends malformed + JSON, this surfaces in AGENT_LEARNINGS.jsonl as a real signal. + """ + excerpt = raw_excerpt[:200] if isinstance(raw_excerpt, str) else "" + on_failure( + skill_name="pi", + action="hook:malformed_payload", + error=f"pi tool_result payload malformed: {reason}", + context=excerpt, + confidence=0.95, + importance="medium", + pain_score=2, + ) + + def main() -> None: + raw = "" try: raw = sys.stdin.read() - payload = json.loads(raw) if raw.strip() else {} - except (json.JSONDecodeError, OSError): - payload = {} + except OSError as e: + _emit_malformed(f"stdin read failed: {e}", "") + return + + if not raw or not raw.strip(): + _emit_malformed("empty payload", "") + return + + try: + payload = json.loads(raw) + except json.JSONDecodeError as e: + _emit_malformed(f"json decode error: {e.msg}", raw) + return + + if not isinstance(payload, dict): + _emit_malformed(f"payload is {type(payload).__name__}, expected object", raw) + return + + if "tool_name" not in payload: + _emit_malformed("missing tool_name", raw) + return tool_name = _tool_name(payload.get("tool_name") or "Unknown") - tool_input = payload.get("tool_input") or {} - if not isinstance(tool_input, dict): - tool_input = {"raw": str(tool_input)} + tool_input = _normalize_input(payload.get("tool_input")) tool_response = _normalize_response(payload) success = cc._is_success(tool_name, tool_input, tool_response) diff --git a/adapters/pi/memory-hook.ts b/adapters/pi/memory-hook.ts index 4f3633a..1b1c108 100644 --- a/adapters/pi/memory-hook.ts +++ b/adapters/pi/memory-hook.ts @@ -14,15 +14,31 @@ const HOOK_SCRIPT = path.join( "pi_post_tool.py", ); +// Timeout for the Python child. If the hook hangs (bad import, stuck I/O), +// Pi's tool_result handler stays blocked because the extension awaits +// runHook(). Override via $AGENT_HOOK_TIMEOUT_MS for slow machines. +const HOOK_TIMEOUT_MS = (() => { + const raw = process.env.AGENT_HOOK_TIMEOUT_MS?.trim(); + const n = raw ? Number.parseInt(raw, 10) : NaN; + return Number.isFinite(n) && n > 0 ? n : 3000; +})(); + let warnedMissingHook = false; let warnedMissingPython = false; let warnedHookFailure = false; +let warnedHookTimeout = false; type PythonCandidate = { command: string; args: string[]; }; +type HookResult = + | { kind: "ok" } + | { kind: "spawn-error" } + | { kind: "hook-failure"; stderr: string; exitCode: number | null } + | { kind: "timeout" }; + function pythonCandidates(): PythonCandidate[] { const envPy = process.env.AGENT_PYTHON?.trim(); const out: PythonCandidate[] = []; @@ -36,32 +52,77 @@ function pythonCandidates(): PythonCandidate[] { function tryRun( candidate: PythonCandidate, payload: Record, -): Promise<"ok" | "spawn-error" | "hook-failure"> { +): Promise { return new Promise((resolve) => { + let settled = false; + const settle = (r: HookResult) => { + if (settled) return; + settled = true; + resolve(r); + }; + + let stderrBuf = ""; const child = spawn( candidate.command, [...candidate.args, HOOK_SCRIPT], { cwd: PROJECT_ROOT, - stdio: ["pipe", "ignore", "ignore"], + // Capture stderr so a "hook-failure" notification can include + // the actual error instead of being undiagnosable. + stdio: ["pipe", "ignore", "pipe"], }, ); - child.on("error", () => resolve("spawn-error")); + + const timer = setTimeout(() => { + try { child.kill("SIGKILL"); } catch { /* already dead */ } + settle({ kind: "timeout" }); + }, HOOK_TIMEOUT_MS); + + child.on("error", () => { + clearTimeout(timer); + settle({ kind: "spawn-error" }); + }); child.on("spawn", () => { - child.stdin.end(JSON.stringify(payload)); + try { + child.stdin.end(JSON.stringify(payload)); + } catch { + // stdin closed before we could write — handled by close/error + } }); + if (child.stderr) { + child.stderr.setEncoding("utf8"); + child.stderr.on("data", (chunk: string) => { + // bound stderr buffer to avoid memory blowup on a wedged hook + if (stderrBuf.length < 4096) stderrBuf += chunk; + }); + } child.on("close", (code) => { - resolve(code === 0 ? "ok" : "hook-failure"); + clearTimeout(timer); + if (code === 0) { + settle({ kind: "ok" }); + } else { + settle({ kind: "hook-failure", stderr: stderrBuf.trim(), exitCode: code }); + } }); }); } -async function runHook(payload: Record) { +async function runHook( + payload: Record, +): Promise< + | "ok" + | "missing-hook" + | "missing-python" + | "timeout" + | { kind: "hook-failure"; stderr: string; exitCode: number | null } +> { if (!existsSync(HOOK_SCRIPT)) return "missing-hook"; for (const candidate of pythonCandidates()) { const result = await tryRun(candidate, payload); - if (result === "ok") return "ok"; - if (result === "hook-failure") return "hook-failure"; + if (result.kind === "ok") return "ok"; + if (result.kind === "timeout") return "timeout"; + if (result.kind === "hook-failure") return result; + // spawn-error → try the next python candidate } return "missing-python"; } @@ -90,10 +151,22 @@ export default function (pi: ExtensionAPI) { "agentic-stack pi memory hook: python3/python not found; automatic episodic logging disabled.", "warning", ); - } else if (result === "hook-failure" && !warnedHookFailure) { + } else if (result === "timeout" && !warnedHookTimeout) { + warnedHookTimeout = true; + ctx.ui.notify( + `agentic-stack pi memory hook timed out (>${HOOK_TIMEOUT_MS}ms); subsequent calls may be skipped. Override with $AGENT_HOOK_TIMEOUT_MS.`, + "warning", + ); + } else if ( + typeof result === "object" && + result.kind === "hook-failure" && + !warnedHookFailure + ) { warnedHookFailure = true; + // Surface the first line of stderr so the failure is diagnosable. + const firstLine = result.stderr.split(/\r?\n/, 1)[0] || `(exit ${result.exitCode})`; ctx.ui.notify( - "agentic-stack pi memory hook failed; continuing without automatic episodic logging.", + `agentic-stack pi memory hook failed: ${firstLine}`, "warning", ); } @@ -101,7 +174,7 @@ export default function (pi: ExtensionAPI) { if (!warnedHookFailure) { warnedHookFailure = true; ctx.ui.notify( - "agentic-stack pi memory hook failed; continuing without automatic episodic logging.", + "agentic-stack pi memory hook errored unexpectedly; continuing without automatic episodic logging.", "warning", ); } diff --git a/install.ps1 b/install.ps1 index b80c467..8d7064e 100644 --- a/install.ps1 +++ b/install.ps1 @@ -154,8 +154,21 @@ switch ($Adapter) { $skillsSrc = Join-Path $TargetAgent 'skills' $skillsDst = Join-Path $piDir 'skills' - if (Test-Path $skillsDst) { - Remove-Item -LiteralPath $skillsDst -Recurse -Force + + # CRITICAL: detect symlink BEFORE Remove-Item. On PowerShell 5.1 + # (Windows default), `Remove-Item -Recurse -Force` on a symlink + # traverses INTO the target and deletes its contents. Re-running + # the installer would silently wipe .agent/skills via the link. + # Use IsLink detection + .NET Delete (link only, not target). + $skillsDstItem = Get-Item -LiteralPath $skillsDst -Force -ErrorAction SilentlyContinue + if ($skillsDstItem) { + $isLink = ($skillsDstItem.Attributes -band [System.IO.FileAttributes]::ReparsePoint) -eq [System.IO.FileAttributes]::ReparsePoint + if ($isLink) { + try { [System.IO.Directory]::Delete($skillsDst, $false) } + catch { [System.IO.File]::Delete($skillsDst) } + } else { + Remove-Item -LiteralPath $skillsDst -Recurse -Force + } } try { New-Item -ItemType SymbolicLink -Path $skillsDst -Target $skillsSrc -ErrorAction Stop | Out-Null From 0f4e7804d4a53b93e86380c58a7c1360c8c2e8ad Mon Sep 17 00:00:00 2001 From: codejunkie99 Date: Thu, 23 Apr 2026 15:39:21 +0530 Subject: [PATCH 3/3] review: fix P2 findings from codex pre-merge review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final codex review surfaced two P2s that would break upgrades and downstream salience. 1. install.sh:148 / install.ps1 — the top-level `.agent/` copy is skipped when the target already has one (line 39-42 / 45-48). So on an upgrade install of an older agentic-stack project, the pi extension's `memory-hook.ts` gets dropped, but the Python hook it invokes (`.agent/harness/hooks/pi_post_tool.py`) never arrives. Every `tool_result` then fires `missing-hook` forever. Fix: sync `pi_post_tool.py` explicitly in the pi case, both installers. 2. pi_post_tool.py:152 — `_emit_malformed()` was writing `importance="medium"` (string). Downstream `salience_score()` does `importance / 10.0`, which raises TypeError on a string and would crash context_budget.py, show.py, and auto_dream.py readers of AGENT_LEARNINGS.jsonl. Fix: use int 5 (middle of the 1-10 scale). Smoke-tested: - empty payload → entry has `importance: 5` (int), salience_score runs to 1.000 without error - upgrade install on a project with old .agent/ (no pi_post_tool.py) → "synced for upgrades" line fires, file now present --- .agent/harness/hooks/pi_post_tool.py | 5 ++++- install.ps1 | 7 +++++++ install.sh | 7 +++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.agent/harness/hooks/pi_post_tool.py b/.agent/harness/hooks/pi_post_tool.py index 005c8f3..4e2f991 100644 --- a/.agent/harness/hooks/pi_post_tool.py +++ b/.agent/harness/hooks/pi_post_tool.py @@ -143,13 +143,16 @@ def _emit_malformed(reason: str, raw_excerpt: str) -> None: JSON, this surfaces in AGENT_LEARNINGS.jsonl as a real signal. """ excerpt = raw_excerpt[:200] if isinstance(raw_excerpt, str) else "" + # importance MUST be numeric — downstream salience_score() does + # `importance / 10.0` and a string would crash context_budget, + # show.py, and auto_dream readers. 5 ≈ "medium" on the 1-10 scale. on_failure( skill_name="pi", action="hook:malformed_payload", error=f"pi tool_result payload malformed: {reason}", context=excerpt, confidence=0.95, - importance="medium", + importance=5, pain_score=2, ) diff --git a/install.ps1 b/install.ps1 index 8d7064e..4cf47a0 100644 --- a/install.ps1 +++ b/install.ps1 @@ -182,6 +182,13 @@ switch ($Adapter) { New-Item -ItemType Directory -Path $extensionsDir -Force | Out-Null Copy-Item (Join-Path $Src 'memory-hook.ts') (Join-Path $extensionsDir 'memory-hook.ts') -Force Write-Host " + .pi/extensions/memory-hook.ts" + # Upgrade path: the .agent copy higher up is skipped when .agent + # already exists, but the pi extension calls this python hook, + # so sync it explicitly. + $hooksDir = Join-Path $TargetAgent 'harness/hooks' + New-Item -ItemType Directory -Path $hooksDir -Force | Out-Null + Copy-Item (Join-Path $Here '.agent/harness/hooks/pi_post_tool.py') (Join-Path $hooksDir 'pi_post_tool.py') -Force + Write-Host " + .agent/harness/hooks/pi_post_tool.py (synced for upgrades)" } 'standalone-python' { Copy-Item (Join-Path $Src 'run.py') (Join-Path $TargetDir 'run.py') -Force diff --git a/install.sh b/install.sh index 0d5f414..b897ac0 100755 --- a/install.sh +++ b/install.sh @@ -147,6 +147,13 @@ case "$ADAPTER" in mkdir -p "$TARGET/.pi/extensions" cp "$SRC/memory-hook.ts" "$TARGET/.pi/extensions/memory-hook.ts" echo " + .pi/extensions/memory-hook.ts" + # Upgrade path: the top-level `.agent/` copy at line 39-42 is skipped + # when .agent already exists, but the pi extension calls this python + # hook, so sync it explicitly for installs on older agentic-stack + # projects that don't already have it. + mkdir -p "$TARGET/.agent/harness/hooks" + cp "$HERE/.agent/harness/hooks/pi_post_tool.py" "$TARGET/.agent/harness/hooks/pi_post_tool.py" + echo " + .agent/harness/hooks/pi_post_tool.py (synced for upgrades)" ;; standalone-python) cp "$SRC/run.py" "$TARGET/run.py"