fix(pi): rewrite adapter — inline TS hook, formula crash, decay tz bug - #24
Merged
Merged
Conversation
Fixes four distinct issues found after PR codejunkie99#17 was merged: ## 1. ModuleNotFoundError crash on brew install (Formula) Formula/agentic-stack.rb pkgshare.install was missing harness_manager/. install.sh dispatches to `python3 -m harness_manager.cli` but the module was never copied into the cellar, causing an immediate crash for every brew user regardless of adapter. ## 2. memory-hook.ts — complete rewrite (approach was wrong) PR codejunkie99#17's hook used: - fileURLToPath(import.meta.url) → jiti leaves this undefined in CJS-transform mode, silently breaking HOOK_SCRIPT path resolution - spawn(python3, [pi_post_tool.py]) per tool_result → subprocess overhead + timeout complexity for every single tool call - No pre-filtering → Python was spawned for read/find/ls/grep too - Private cc._is_success / cc._importance etc. imports → breaks silently on any refactor of claude_code_post_tool.py New approach (matches the working life-international reference setup): - process.cwd() for all path resolution — no import.meta.url - All importance / pain_score / action / reflection logic is inline TypeScript — no Python subprocess per tool call - Direct fs.appendFileSync to AGENT_LEARNINGS.jsonl - Pre-filter: bash / edit / write only (mirrors Claude Code's "^(Bash|Edit|Write)$" matcher). Low-importance bash successes (imp <= 3) are also skipped to keep the log signal-rich. - Typed inputs via isBashToolResult / isEditToolResult / isWriteToolResult type guards (exported from pi's public API) - cachedSha typed as string | undefined so the git cache actually works - session_shutdown handler runs auto_dream.py on quit/new/resume, mirroring Claude Code's Stop hook. cron is now an optional fallback. ## 3. adapter.json — remove from_stack pi_post_tool.py entry The hook is self-contained TypeScript. pi_post_tool.py is no longer deployed as an explicitly-managed adapter file. It remains in the brain template for standalone/debug use. ## 4. decay.py — timezone-aware vs naive datetime crash auto_dream.py → decay_old_entries() compared a timezone-aware cutoff (datetime.now()) against naive timestamps from AGENT_LEARNINGS.jsonl, crashing with TypeError. Fixed by using datetime.now(timezone.utc) and normalising naive timestamps to UTC before comparison. Tested: fresh install on lumen-review, 41/41 tests pass, doctor green.
aliirz
marked this pull request as draft
April 24, 2026 20:43
aliirz
marked this pull request as ready for review
April 24, 2026 21:18
…c dream cycle Follow-up fixes for PR codejunkie99#24 surfaced by independent verification (memory-hook type cross-reference + codex review). Each fix is annotated with the failure mode it addresses. P0 — session_shutdown filter rejected every event Pi's `SessionShutdownEvent` is `{ type: "session_shutdown" }` with no `reason` field (verified in pi-coding-agent's types.d.ts and the emit site at agent-session.js:1638). The hook's `DREAM_REASONS.has(event.reason)` filter therefore evaluated `has(undefined) === false` for every event, so `auto_dream.py` never ran. Drop the filter; shutdown only fires once on process exit. Add a re-entrancy guard for defence in depth. P1 — edit reflection always lost the diff Hook accessed `event.input.edits[0]` but Pi's `EditToolInput` is flat `{ path, oldText, newText }` (no `edits` array — that's MultiEdit on Claude Code). Reflections silently degraded to `Edited <path>` with no old/new content for the dream-cycle clusterer to grip. Use the flat fields directly. P1 — naive-local Python timestamps + UTC decay = silent drift Decay's "naive == UTC" assumption is correct only if writers emit UTC. They didn't: post_execution, on_failure, learn, graduate, promote, review_state, render_lessons all wrote naive-local. Switch every writer to `datetime.now(timezone.utc).isoformat()` and teach every reader (salience, show._human_age / _daily_counts / failing_skills / last_dream_cycle, on_failure._count_recent_failures, review_state._age_factor, archive) to normalise naive timestamps to UTC before comparing. P1 — one bad user regex disabled every user pattern Pre-fix: build one combined RegExp per list, catch any error, return null for both. A single typo in `hook_patterns.json` silently dropped all user rules. Now: validate per-fragment, incremental merge — same posture as `claude_code_post_tool.py`'s `_filter_valid` / `_build_with_fallback`. P1 — auto_dream lost entries that landed mid-cycle Original PR fixed the truncate-before-lock race in `_write_entries` but not the read-modify-write window: an `append_jsonl()` between `_load_entries()` and `_write_entries(kept)` would be truncated away. Hold a single LOCK_EX on the episodic log across the entire cycle via `_episodic_locked()`. Mutually exclusive with `_episodic_io.append_jsonl` (same flock target). P1 — salience over-scored future-skewed legacy rows Legacy naive-local timestamps re-interpreted as UTC can read as a few hours in the future during the migration window. `timedelta.days` then went negative and `recency = 10 - age*0.3` exceeded the intended cap. Floor age at 0; clamp recency to ≤ 10. P2 — _cachedSha went stale across `git commit` inside a session Cache for the lifetime of pi was a perf win but recorded the pre-commit SHA on every entry after a mid-session commit. Invalidate on bash commands matching `git <subcmd>` for HEAD-moving subcommands. `[^|;&]*?` allows option flags between `git` and the subcommand (`git -c key=val checkout main`, `git -C path switch dev`). Includes `switch` which an earlier draft missed. P3 — test_pi_install_creates_symlink_and_syncs_hook misnamed Asserted that `pi_post_tool.py` was synced via from_stack — the entry PR codejunkie99#24 removed. Rename the test, drop the obsolete assertion, document in the comment that the .py still ships in the brain template for standalone use. P3 — decay archive filename used local date while cutoff was UTC Asymmetric. `archive_{date}.jsonl` now uses UTC date. Plus tests/test_decay_timezone.py — six tests pinning the shapes decay must handle: aware UTC old/recent, naive old, mixed mid-stream, malformed, archive-filename-uses-UTC. Catches the original crash and the migration-era silent-drift cases. Test suite: 47 / 47 pass (was 41 before; +6 new). Lock semantics verified manually (open-then-flock-then-ftruncate; full read-modify-write window held under a single fd). SHA-invalidation regex spot-checked against 12 git command shapes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
Four distinct issues found after PR #17 was merged.
1.
ModuleNotFoundError: No module named 'harness_manager'(the crash)File:
Formula/agentic-stack.rbpkgshare.installwas missingharness_manager/.install.shdispatches topython3 -m harness_manager.clibut the module was never copied into the cellar, soagentic-stack picrashes immediately for every brew user before doing anything at all.Fix: add
harness_managertopkgshare.install.2.
memory-hook.ts— complete rewritePR #17's approach had four problems:
fileURLToPath(import.meta.url)for path resolutionimport.meta.urlundefined in CJS-transform mode →HOOK_SCRIPTpath silently breaks → no logging at allspawn(python3, [pi_post_tool.py])pertool_resultread,ls,grepread/find/ls/grep— pure noisecc._is_success,cc._importanceetc.New approach (matches the working reference setup from a live project):
process.cwd()for all paths — noimport.meta.urlfs.appendFileSynctoAGENT_LEARNINGS.jsonlbash/edit/writeonly — mirrors Claude Code's"^(Bash|Edit|Write)$"matcherimp <= 3) also skipped to keep the log signal-richisBashToolResult/isEditToolResult/isWriteToolResult(pi's own exported type guards)cachedShatypedstring | undefinedso the git cache actually workssession_shutdownhandler runsauto_dream.pyonquit/new/resume— mirrors Claude Code'sStophook. No more silent dream cycle gap between harnesses.3.
adapter.json— dropfrom_stackpi_post_tool.pyentryThe hook is now self-contained TypeScript.
pi_post_tool.pyis no longer deployed as an explicitly-managed adapter file. It remains in the brain template for standalone/debug use.4.
decay.py— timezone-aware vs naive datetime crashauto_dream.py → decay_old_entries()compared a timezone-awarecutoffagainst naivetimestampstrings fromAGENT_LEARNINGS.jsonl, crashing with:This crash was latent before but our new
session_shutdownhook surfaces it on every clean pi exit.Fix: use
datetime.now(timezone.utc)for the cutoff and normalise any naive timestamps to UTC before comparison.Testing
./install.sh pion a clean project:.pi/extensions/memory-hook.tsinstalled,.pi/skillssymlinked,install.jsoncorrect./install.sh doctor→ green./install.sh status→ correct outputpython3 .agent/memory/auto_dream.pyruns without error41/41existing tests passCloses #13 (properly this time).