Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 36 additions & 3 deletions .claude/hooks/lib/worktree-utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,45 @@ checkWorktreeTTL() {
return 1
fi

# Get creation time from worktree directory metadata
local created_epoch
created_epoch=$(stat -f "%B" "$wt_dir" 2>/dev/null || stat -c "%W" "$wt_dir" 2>/dev/null || echo "0")
# Get creation time from worktree directory metadata.
#
# Cross-platform AND CI-safe:
# * macOS/BSD stat uses `-f <fmt>`: %B = birth (create) time, %m = mtime.
# * GNU/Linux stat uses `-c <fmt>`: %W = birth time, %Y = mtime.
#
# Two failures this guards against (both seen on Ubuntu CI, never on macOS):
# 1. The old `stat -f "%B"` was tried FIRST on Linux, where `-f` means
# "filesystem mode" — GNU stat then printed multi-line, NON-NUMERIC
# filesystem info into created_epoch, and `$(( now - created_epoch ))`
# aborted the function under `set -uo pipefail` -> EMPTY stdout ->
# JSONDecodeError in the tests.
# 2. Many Linux filesystems (ext4/overlayfs on CI) report birth time as 0
# (`%W` == 0). created_epoch=0 made a FRESH worktree look ~29M minutes
# old -> expired:true. We fall back to mtime when birth time is 0/empty.
local created_epoch=""
if stat -c '%W' / >/dev/null 2>&1; then
# GNU/Linux stat
created_epoch=$(stat -c '%W' "$wt_dir" 2>/dev/null || echo "")
if [[ -z "$created_epoch" || ! "$created_epoch" =~ ^[0-9]+$ || "$created_epoch" -eq 0 ]]; then
# Birth time unavailable on this fs -> use mtime (always populated).
created_epoch=$(stat -c '%Y' "$wt_dir" 2>/dev/null || echo "")
fi
else
# macOS/BSD stat
created_epoch=$(stat -f '%B' "$wt_dir" 2>/dev/null || echo "")
if [[ -z "$created_epoch" || ! "$created_epoch" =~ ^[0-9]+$ || "$created_epoch" -eq 0 ]]; then
created_epoch=$(stat -f '%m' "$wt_dir" 2>/dev/null || echo "")
fi
fi
# Final guard: never let a non-numeric value reach the arithmetic below
# (a non-numeric $(( )) aborts the function under set -e and yields empty stdout).
[[ "$created_epoch" =~ ^[0-9]+$ ]] || created_epoch=$(date +%s)

local now_epoch
now_epoch=$(date +%s)
local elapsed_minutes=$(( (now_epoch - created_epoch) / 60 ))
# Clamp negative drift (clock skew between birth time and now) to 0.
(( elapsed_minutes < 0 )) && elapsed_minutes=0
local expired="false"

if [[ "$elapsed_minutes" -ge "$ttl_minutes" ]]; then
Expand Down
6 changes: 3 additions & 3 deletions .claude/hooks/session-end-handoff.sh
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ gc_cleanup() {
member_count=$(find "$team_dir" -type f -name "member-*.json" 2>/dev/null | wc -l | tr -d ' ')
if [[ "$member_count" -eq 0 ]]; then
rm -rf "$team_dir" 2>/dev/null && {
((cleaned_teams++))
cleaned_teams=$((cleaned_teams + 1))
log "GC: Removed empty team dir: $team_dir" >> "$gc_log"
}
fi
Expand All @@ -321,7 +321,7 @@ gc_cleanup() {
dir_date=$(basename "$task_dir" | grep -oE '[0-9]{8}' || echo "")
if [[ -n "$dir_date" ]] && [[ "$dir_date" -lt "$cutoff_date" ]]; then
rm -rf "$task_dir" 2>/dev/null && {
((cleaned_tasks++))
cleaned_tasks=$((cleaned_tasks + 1))
log "GC: Removed old task dir: $task_dir" >> "$gc_log"
}
fi
Expand All @@ -338,7 +338,7 @@ gc_cleanup() {
sort -n | head -n "$to_delete" | cut -d' ' -f2- | \
while IFS= read -r file; do
rm -f "$file" 2>/dev/null && {
((cleaned_handoffs++))
cleaned_handoffs=$((cleaned_handoffs + 1))
log "GC: Removed old handoff: $file" >> "$gc_log"
}
done
Expand Down
8 changes: 7 additions & 1 deletion .claude/hooks/vault-writeback.sh
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,13 @@ for i in $(seq 0 $((QUEUE_LENGTH - 1)) 2>/dev/null); do
fi

# Sanitize summary (no secrets, no absolute paths)
SAFE_SUMMARY=$(echo "$SUMMARY" | sed "s|${HOME}/|~/|g" | tr -cd ' a-zA-Z0-9_.:/-()[]{}?!,;' | head -c 2000)
# NOTE: the literal '-' MUST be last in the tr set. Inside '...:/-()...' the
# substring '/-(' is parsed by GNU tr (Linux/CI) as a range from '/' (0x2F)
# to '(' (0x28) — a reversed range — and GNU tr aborts with a non-zero exit
# ("range-endpoints ... in reverse collating sequence order"). Under
# `set -euo pipefail` that killed the whole hook (rc=1) on Ubuntu while BSD
# tr on macOS tolerated it. Trailing '-' is a literal dash on both.
SAFE_SUMMARY=$(echo "$SUMMARY" | sed "s|${HOME}/|~/|g" | tr -cd ' a-zA-Z0-9_.:/()[]{}?!,;-' | head -c 2000)

# Create draft wiki article
NOW=$(date -u +"%Y-%m-%dT%H:%M:%SZ")
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ jobs:
- name: Run pytest
run: |
if [ -d "tests" ]; then
python -m pytest tests/ -v --tb=short || true
python -m pytest tests/ -v --tb=short

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Seed required layer files before gating pytest

In this workflow the pytest tests/ step runs after only checkout and pip install, with no W2.2/layer setup. Several tests in tests/layers/test_layer_stack.py assert that ~/.ralph/layers/L0_identity.md and ~/.ralph/layers/L1_essential.md already exist, and a fresh GitHub-hosted runner will not have those home-directory files, so removing || true makes the CI test job fail for every PR before the later harness step. Please either create the required fixtures/setup in CI or fix/skip those environment-dependent tests before making this command gating.

Useful? React with 👍 / 👎.

fi

- name: Run harness tests
Expand Down
77 changes: 77 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import sys
import tempfile
import shutil
from pathlib import Path
import pytest

# Add project root to path
Expand Down Expand Up @@ -292,3 +293,79 @@ def _validate(skill_path: str) -> dict:
return result

return _validate


# ============================================================
# CI-safe isolation fixtures (PR #28: make tests/ fail-loud-able)
# ============================================================

@pytest.fixture
def isolated_home(tmp_path, monkeypatch):
"""Isolate $HOME to a temp dir seeded with the minimal Ralph layout.

Redirects BOTH ``$HOME`` (for shell hooks that read ``${HOME}``) and
``Path.home()`` (for Python code), so a test never touches the developer's
real ~/.claude or ~/.ralph. The repo's hooks are symlinked in so they
resolve to the real, versioned scripts under the temp HOME.
"""
import json

home = tmp_path / "home"
for sub in (
".claude/state", ".claude/teams", ".claude/tasks", ".claude/skills", ".claude/scripts",
".ralph/logs", ".ralph/state", ".ralph/handoffs", ".ralph/ledgers",
".ralph/config", ".ralph/temp", ".ralph/markers",
):
(home / sub).mkdir(parents=True, exist_ok=True)

# Point ~/.claude/hooks at the repo's real hooks (no duplication, no dev HOME).
repo_hooks = Path(PROJECT_ROOT) / ".claude" / "hooks"
hooks_link = home / ".claude" / "hooks"
if repo_hooks.is_dir() and not hooks_link.exists():
hooks_link.symlink_to(repo_hooks)

# Minimal valid config so settings/feature readers find a non-empty file.
(home / ".claude" / "settings.json").write_text(
json.dumps({"hooks": {}}), encoding="utf-8"
)
(home / ".ralph" / "config" / "features.json").write_text(
json.dumps({"RALPH_ENABLE_HANDOFF": True}), encoding="utf-8"
)

monkeypatch.setenv("HOME", str(home))
monkeypatch.setattr(Path, "home", lambda: home)
return home


@pytest.fixture
def requires_tool():
"""Return a helper that skips the test when a named CLI tool is absent.

Usage: ``def test_x(requires_tool): requires_tool("rsync")``.
"""
def _require(name):
if shutil.which(name) is None:
pytest.skip(f"required tool not available on PATH: {name}")
return _require


@pytest.fixture
def git_repo(tmp_path):
"""Create a throwaway git repo for hook/worktree tests (skips if git absent)."""
import subprocess

if shutil.which("git") is None:
pytest.skip("git not available on PATH")

repo = tmp_path / "repo"
repo.mkdir()
env = {**os.environ, "GIT_CONFIG_GLOBAL": os.devnull, "GIT_CONFIG_SYSTEM": os.devnull}
run = lambda *a: subprocess.run(a, cwd=repo, check=True, env=env,
capture_output=True, text=True)
run("git", "init", "-q")
run("git", "config", "user.email", "test@example.com")
run("git", "config", "user.name", "ralph-test")
(repo / "README.md").write_text("test\n", encoding="utf-8")
run("git", "add", "-A")
run("git", "commit", "-qm", "init")
return repo
37 changes: 27 additions & 10 deletions tests/layers/test_layer_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,20 @@ def sample_rules_json(tmp_path: Path) -> Path:
class TestLayer0:
"""Tests for the identity layer (L0)."""

def test_exists_returns_true_for_real_file(self, real_l0_path: Path):
"""L0.exists() returns True when the real identity file exists."""
layer = Layer0(path=real_l0_path)
def test_exists_returns_true_for_real_file(self, tmp_path: Path):
"""L0.exists() returns True when an identity file is present.

CI-safe: build the artifact in tmp_path instead of depending on the
developer's ``~/.ralph/layers/L0_identity.md`` (absent on CI runners).
"""
l0_path = tmp_path / "L0_identity.md"
l0_path.write_text(
"# Ralph Identity\n\n## Principles\nBuilt for the test.\n",
encoding="utf-8",
)
layer = Layer0(path=l0_path)
assert layer.exists() is True, (
f"L0 identity file missing at {real_l0_path}. "
"Run W2.2 setup to create ~/.ralph/layers/L0_identity.md"
f"L0 identity file should exist after writing it at {l0_path}"
)

def test_exists_returns_false_for_missing_file(self, tmp_layers_dir: Path):
Expand Down Expand Up @@ -230,12 +238,21 @@ def test_load_raises_when_file_missing(self, tmp_layers_dir: Path):
with pytest.raises(FileNotFoundError):
layer.load()

def test_real_l1_exists(self, real_l1_path: Path):
"""Real L1_essential.md was built by W2.2 setup."""
layer = Layer1(path=real_l1_path)
def test_real_l1_exists(self, tmp_path: Path, sample_rules_json: Path, monkeypatch):
"""Layer1.build() produces an L1_essential.md that exists().

CI-safe: build the artifact in tmp_path from the sample rules fixture
(same pattern as test_build_creates_md_file) instead of asserting on the
developer's ``~/.ralph/layers/L1_essential.md`` (absent on CI runners).
"""
l1_path = tmp_path / "L1_essential.md"
_patch_rules_json(monkeypatch, sample_rules_json)

layer = Layer1(path=l1_path, rule_count=15)
layer.build()

assert layer.exists() is True, (
f"L1 file missing at {real_l1_path}. "
"Run: python3 .claude/lib/layers.py --build-l1"
f"L1 file should exist after build() at {l1_path}"
)

def test_real_l1_has_substantive_rules(self, real_l1_path: Path):
Expand Down
Loading
Loading