Skip to content

Commit 90d77d9

Browse files
author
codejunkie99
committed
harden episodic writes: fcntl lock + fix pi skills orphan sync
Two pre-existing infrastructure bugs flagged during the PR #17 cross-model review, fixed here against master because they predate that PR and affect every harness. ## 1. Concurrent writes to AGENT_LEARNINGS.jsonl post_execution.py and on_failure.py both did plain `open(EPISODIC, "a")` → `f.write(json.dumps(entry) + "\n")`. POSIX O_APPEND makes single `write(2)` calls atomic only up to PIPE_BUF (4 KB on Linux/macOS). In practice most entries stay under that ceiling and the unlocked code never corrupts, but the `reflection` field is uncapped in log_execution and can easily exceed 4 KB on high-importance failure logs. Every downstream reader (auto_dream.py, cluster.py, context_budget.py, show.py) skips `json.JSONDecodeError` lines silently — so one over-PIPE_BUF interleave = one episodic entry gone with no signal. Fix: new `_episodic_io.append_jsonl()` helper that opens in append- binary mode (no Python text-mode buffering quirks) and wraps the write in `fcntl.flock(LOCK_EX)`. Shared by both writers. On platforms without fcntl (native Windows Python) behavior falls back to the pre-fix unlocked append; WSL, git-bash/Cygwin, macOS, Linux all have fcntl. Verified: 40 concurrent writers × 500 entries × 2 KB reflection each → 20,000 parseable lines, zero corruption. ## 2. pi install.sh silently leaves stale skills on re-install `ln -sfn src dest` where `dest` is a REAL directory (e.g. from an earlier copy-fallback install) silently creates `dest/<basename-of-src>` INSIDE the dir and exits 0. The existing `if ln -sfn; then` branch took the success path, the `rm -rf + cp` fallback never ran, and orphans stuck around forever. Verified on macOS, confirmed the symlink-inside-dir behavior. Fix: check `-L` (symlink) and `-d` (real dir) explicitly before calling `ln -sfn`, mirror the pattern used by the codex adapter (PR #16 follow-up). Existing symlink → cheap repoint. Real directory → rsync --delete when available, rm+cp otherwise. Non-existent → symlink or copy fallback. Same three-branch shape, no more silent wrong behavior. Verified: re-install after orphan-skill was added to a real-dir `.pi/skills` → rsync --delete removes the orphan.
1 parent 63bbbd9 commit 90d77d9

4 files changed

Lines changed: 72 additions & 15 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
"""Cross-platform locked append for episodic JSONL writes.
2+
3+
POSIX `write(2)` in O_APPEND mode is atomic for payloads up to PIPE_BUF
4+
(4 KB on Linux, 512 B minimum per POSIX). Most episodic entries fit,
5+
but failure entries with reflection + context + detail can exceed that,
6+
and two harness hooks writing from the same process (or from two Pi
7+
sessions on the same repo) can interleave bytes mid-line. Silent
8+
corruption is worse than a visible error because every downstream
9+
reader (`auto_dream.py`, `cluster.py`, `context_budget.py`,
10+
`show.py`) skips `JSONDecodeError` lines without surfacing the loss.
11+
12+
This module serializes appends with `fcntl.flock(LOCK_EX)` on POSIX.
13+
On platforms without `fcntl` (native Windows Python) the lock is a
14+
no-op and behavior matches the pre-lock baseline. WSL, git-bash via
15+
Cygwin, macOS, and Linux all provide `fcntl`.
16+
"""
17+
import json
18+
import os
19+
20+
try:
21+
import fcntl # POSIX
22+
_HAVE_FLOCK = True
23+
except ImportError:
24+
_HAVE_FLOCK = False
25+
26+
27+
def append_jsonl(path: str, entry: dict) -> dict:
28+
"""Serialize `entry` to one JSON line and append to `path`.
29+
30+
Uses `open(..., "ab")` (append-binary) to bypass Python's text-mode
31+
buffering and guarantee a single `write(2)` per call. `fcntl.flock`
32+
provides cross-process mutual exclusion on POSIX.
33+
"""
34+
payload = (json.dumps(entry) + "\n").encode("utf-8")
35+
os.makedirs(os.path.dirname(path), exist_ok=True)
36+
with open(path, "ab") as f:
37+
if _HAVE_FLOCK:
38+
fcntl.flock(f.fileno(), fcntl.LOCK_EX)
39+
try:
40+
f.write(payload)
41+
f.flush()
42+
finally:
43+
if _HAVE_FLOCK:
44+
fcntl.flock(f.fileno(), fcntl.LOCK_UN)
45+
return entry

.agent/harness/hooks/on_failure.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Failures are learning. High pain score + rewrite flag after repeat offenses."""
22
import json, datetime, os
33
from ._provenance import build_source
4+
from ._episodic_io import append_jsonl
45

56
ROOT = os.path.join(os.path.dirname(__file__), "..", "..")
67
EPISODIC = os.path.join(ROOT, "memory/episodic/AGENT_LEARNINGS.jsonl")
@@ -69,7 +70,4 @@ def on_failure(skill_name, action, error, context="", confidence=0.9,
6970
f"Flag for rewrite."
7071
)
7172
entry["pain_score"] = 10
72-
os.makedirs(os.path.dirname(EPISODIC), exist_ok=True)
73-
with open(EPISODIC, "a") as f:
74-
f.write(json.dumps(entry) + "\n")
75-
return entry
73+
return append_jsonl(EPISODIC, entry)

.agent/harness/hooks/post_execution.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Runs after every action. Appends a structured entry to episodic memory."""
2-
import json, datetime, os
2+
import datetime, os
33
from ._provenance import build_source
4+
from ._episodic_io import append_jsonl
45

56
ROOT = os.path.join(os.path.dirname(__file__), "..", "..")
67
EPISODIC = os.path.join(ROOT, "memory/episodic/AGENT_LEARNINGS.jsonl")
@@ -15,7 +16,6 @@ def log_execution(skill_name, action, result, success, reflection="",
1516
a higher value (e.g. 5) for high-importance successful operations so
1617
recurring patterns cross the dream-cycle promotion threshold (7.0).
1718
"""
18-
os.makedirs(os.path.dirname(EPISODIC), exist_ok=True)
1919
if pain_score is None:
2020
pain_score = 2 if success else 7
2121
entry = {
@@ -31,6 +31,4 @@ def log_execution(skill_name, action, result, success, reflection="",
3131
"source": build_source(skill_name),
3232
"evidence_ids": list(evidence_ids) if evidence_ids else [],
3333
}
34-
with open(EPISODIC, "a") as f:
35-
f.write(json.dumps(entry) + "\n")
36-
return entry
34+
return append_jsonl(EPISODIC, entry)

install.sh

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,31 @@ case "$ADAPTER" in
133133
echo " + AGENTS.md"
134134
fi
135135
mkdir -p "$TARGET/.pi"
136-
# symlink .pi/skills -> .agent/skills so pi sees the one true skill tree.
137-
# ln -sfn atomically replaces an existing symlink; fall back to cp -R
138-
# on filesystems that don't support symlinks (e.g. Windows without dev mode).
136+
# Keep .pi/skills in sync with .agent/skills (the one true skill tree).
137+
# Handle three shapes explicitly: `ln -sfn src dest` against a REAL
138+
# directory silently creates `dest/<basename-of-src>` INSIDE the dir
139+
# on macOS/Linux and exits 0, which would leave stale orphans forever.
140+
# Check -L before -d so existing symlinks take the cheap repoint path,
141+
# and reserve the rsync path for real dirs left from a prior copy
142+
# fallback install.
139143
SKILLS_SRC="$(cd "$TARGET/.agent/skills" && pwd)"
140-
if ln -sfn "$SKILLS_SRC" "$TARGET/.pi/skills" 2>/dev/null; then
144+
SKILLS_DEST="$TARGET/.pi/skills"
145+
if [[ -L "$SKILLS_DEST" ]]; then
146+
ln -sfn "$SKILLS_SRC" "$SKILLS_DEST"
147+
echo " + .pi/skills -> $SKILLS_SRC"
148+
elif [[ -d "$SKILLS_DEST" ]]; then
149+
if command -v rsync >/dev/null 2>&1; then
150+
rsync -a --delete "$SKILLS_SRC/" "$SKILLS_DEST/"
151+
echo " ~ synced .agent/skills → .pi/skills (rsync --delete)"
152+
else
153+
rm -rf "$SKILLS_DEST"
154+
cp -R "$SKILLS_SRC" "$SKILLS_DEST"
155+
echo " ~ replaced .pi/skills with current .agent/skills (no rsync)"
156+
fi
157+
elif ln -sfn "$SKILLS_SRC" "$SKILLS_DEST" 2>/dev/null; then
141158
echo " + .pi/skills -> $SKILLS_SRC"
142159
else
143-
rm -rf "$TARGET/.pi/skills"
144-
cp -R "$SKILLS_SRC" "$TARGET/.pi/skills"
160+
cp -R "$SKILLS_SRC" "$SKILLS_DEST"
145161
echo " + .pi/skills (copy; symlink not supported here)"
146162
fi
147163
;;

0 commit comments

Comments
 (0)