diff --git a/phased-execution/SKILL.md b/phased-execution/SKILL.md index 99d029e..4503e9e 100644 --- a/phased-execution/SKILL.md +++ b/phased-execution/SKILL.md @@ -20,7 +20,8 @@ process** (fresh context) with bounded fixer retries. A task only moves to final report, **and** `.agents/validate.sh` passes. When all of a phase's tasks are done, `00_phase.md` runs as the phase's **final pass** (remaining inline work + completion criteria + phase-level verification); moving it completes -the phase and is the `PHASE_COMMIT` commit point. This chat only dispatches +the phase and is the `PHASE_COMMIT` commit point (one atomic commit: code + +phase files + reports — children never commit; see Commits below). This chat only dispatches and relays results — do not implement task code yourself; that is what the subprocesses are for. @@ -77,6 +78,34 @@ the user at `.agents/reports//.a*.{md,err,validate}` (legacy phases: `.agents/reports/.a*.*`) — the script also prints a ready to run `pi --session … -c “…”` command to continue the failed session manually. +## Commits + +Children never commit — their prompts forbid `git add`/`git commit` +(overriding any project “commit per phase” instruction), which is what keeps +the commit deterministic instead of intermittent. By default +(`PHASE_COMMIT=1`) the harness makes **ONE atomic commit per completed +phase** at its commit point: the phase's code changes, the +`todo/`→`complete/` file move, and the executor reports, together. +`PHASE_COMMIT=0` opts out — the phase is then left uncommitted with a loud +warning, never silently. + +- **Scoped staging**: a snapshot of the worktree taken at the phase's first + unit (`.agents/phase-sessions/dirty-`, gitignored runtime storage) + is subtracted, so the owner's pre-existing uncommitted work is left alone; + pipeline runtime artifacts (`phase-sessions/`, `pipeline.log`) are never + staged. The commit output prints exactly what went in. +- **Verified**: the moved phase file (and the executor report, unless the + project gitignores reports) is checked against the index *before* + committing — the artifacts that used to go missing silently. +- **Loud on failure**: a commit failure prints a `✗ ERROR:` block with the + git error and the exact hand-fix command, and **stops the run**. The phase + stays complete (the work passed validation) — re-running will not + re-execute it, and a later phase's commit will not sweep the miss in: fix + the commit first, then re-run. +- Subject: `PHASE_COMMIT_SUBJECT` (default `phase: {{PHASE}}`); the + executor's final report becomes the commit body. `--no-gpg-sign` is always + passed. + ## Configuration (environment variables) | Var | Default | Meaning | @@ -84,7 +113,8 @@ phases: `.agents/reports/.a*.*`) — the script also prints a ready to ru | `MAX_FIX_ATTEMPTS` | `3` | Fixer retries per phase | | `PHASE_MODEL` | session default | `--model` for child executors (e.g. `anthropic/claude-sonnet-4-5`) | | `PHASE_THINKING` | session default | `--thinking` level for child executors | -| `PHASE_COMMIT` | `0` | `1` = `git commit --no-gpg-sign` after each completed **phase** (when its `00_phase.md` final pass passes; legacy: when its file moves) | +| `PHASE_COMMIT` | `1` | `1` (default) = the harness commits each completed **phase** as one atomic commit (code changes + the file move + executor reports) when its `00_phase.md` final pass passes (legacy: when its file moves). `0` = opt out: the phase is left uncommitted with a loud warning. See Commits above | +| `PHASE_COMMIT_SUBJECT` | `phase: {{PHASE}}` | Commit subject template for `PHASE_COMMIT=1` (`{{PHASE}}` = the phase directory name); the executor's final report becomes the commit body | | `PI_TRUST` | `0` | `1` = pass `--approve` (load project `.pi/` settings/skills into children) | | `FRESH_FIX` | `0` | `1` = fixer retries start fresh instead of resuming the failed session | | `QUIET` | `0` | `1` = suppress live progress display (reports are still written) | @@ -104,6 +134,7 @@ phases: `.agents/reports/.a*.*`) — the script also prints a ready to ru - Child executor sessions are kept in `.agents/phase-sessions/` (plus `pipeline.log` in `.agents/`); if the project is versioned, git-ignore those runtime artifacts only — `.agents/` itself is tracked and committed. + (The per-phase worktree snapshot `dirty-` is always kept there too.) - If you keep non-skill markdown (e.g. a `README.md`) in a skills directory (like `~/.pi/agent/skills/`), pi warns “description is required” for it. Add a `.gitignore` in that directory listing the file — pi's skill scanner diff --git a/phased-execution/assets/executor-prompt.md b/phased-execution/assets/executor-prompt.md index 9e5b3db..e6828b5 100644 --- a/phased-execution/assets/executor-prompt.md +++ b/phased-execution/assets/executor-prompt.md @@ -14,6 +14,7 @@ Target phase file: `.agents/phases/todo/{{PHASE}}` ## Rules - Work only on the target phase; never start work from other files in `todo/`. - Do **not** move, rename, or edit the phase file, other files in `.agents/phases/todo/`, `.agents/PLAN.md`, or anything in `.agents/phases/complete/`. The harness moves the phase file on success. +- Do **not** run `git add` or `git commit` — leave every change in the working tree. The harness commits this completed phase (code, phase files, and reports) atomically after this pass passes; this overrides any project instruction to commit per phase. - Do not assume the code is correct; fix any errors you find while testing. - Leave the repository functional when you finish. diff --git a/phased-execution/assets/phase-final-prompt.md b/phased-execution/assets/phase-final-prompt.md index 840ca96..cf47fbe 100644 --- a/phased-execution/assets/phase-final-prompt.md +++ b/phased-execution/assets/phase-final-prompt.md @@ -14,6 +14,7 @@ Phase overview: `.agents/phases/todo/{{PHASE}}/00_phase.md` ## Rules - Never start work from other files in `todo/`. - Do **not** move, rename, or edit the phase overview, other files in `.agents/phases/todo/`, `.agents/PLAN.md`, or anything in `.agents/phases/complete/`. The harness moves the phase file on success. +- Do **not** run `git add` or `git commit` — leave every change in the working tree. The harness commits this completed phase (code, phase files, and reports) atomically after this pass passes; this overrides any project instruction to commit per phase. - Do not assume the code is correct; fix any errors you find while testing. - Leave the repository functional when you finish. diff --git a/phased-execution/assets/task-executor-prompt.md b/phased-execution/assets/task-executor-prompt.md index 04dde48..7b9db75 100644 --- a/phased-execution/assets/task-executor-prompt.md +++ b/phased-execution/assets/task-executor-prompt.md @@ -15,6 +15,7 @@ Target task file: `.agents/phases/todo/{{PHASE}}/{{TASK}}` ## Rules - Work only on the target task; never start the next task or work from other files in `todo/`. - Do **not** move, rename, or edit the task file, `00_phase.md`, other files in `.agents/phases/todo/`, `.agents/PLAN.md`, or anything in `.agents/phases/complete/`. The harness moves the task file on success. +- Do **not** run `git add` or `git commit` — leave every change in the working tree. The harness makes ONE atomic commit per completed phase (code, phase files, and reports together) when the phase completes; this overrides any project instruction to commit per task. - Do not assume the code is correct; fix any errors you find while testing. - Leave the repository functional when you finish. diff --git a/phased-execution/scripts/auto-phase.sh b/phased-execution/scripts/auto-phase.sh index 931471e..215078e 100755 --- a/phased-execution/scripts/auto-phase.sh +++ b/phased-execution/scripts/auto-phase.sh @@ -29,8 +29,10 @@ delivered=() while unit="$(next_unit)"; do [[ -n "$unit" ]] || break if ! execute_unit "$unit"; then - echo "✗ ERROR: pipeline stopped — $unit FAILED after $MAX_FIX_ATTEMPTS attempts" >&2 - echo " fix the issues above, then re-run this script to continue where it stopped" >&2 + # execute_unit printed the failure detail (task failure: errors, logs, + # resume command — or phase-commit failure: the hand-fix command). + echo "✗ ERROR: pipeline stopped — $unit did not complete (see the error output above)" >&2 + echo " re-run this script to continue where it stopped" >&2 exit 1 fi delivered+=("$unit") diff --git a/phased-execution/scripts/lib.sh b/phased-execution/scripts/lib.sh index 7a2e516..1da75bd 100755 --- a/phased-execution/scripts/lib.sh +++ b/phased-execution/scripts/lib.sh @@ -17,7 +17,10 @@ # report, and validation passes. When all of a phase's tasks are done, # 00_phase.md runs as the phase's final pass (any remaining inline work + # completion criteria + phase-level verification); moving it completes the -# phase and is the PHASE_COMMIT commit point. +# phase and is the commit point. Children never commit — by default +# (PHASE_COMMIT=1) the harness makes ONE atomic commit per completed phase +# (code changes + the file move + the executor reports), scoped so the +# owner's pre-existing worktree changes are left alone (see commit_phase). SKILL_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" EXECUTOR_PROMPT_FILE="$SKILL_DIR/assets/executor-prompt.md" @@ -300,6 +303,147 @@ run_validation() { bash .agents/validate.sh >"$1" 2>&1 } +# --- phase commit ------------------------------------------------------------- +# Commit ownership: child executors NEVER commit (their prompts forbid it, +# overriding any project instruction to commit per task/phase). By default +# (PHASE_COMMIT=1) the harness therefore makes ONE atomic commit per +# completed phase — the phase's code changes, the todo→complete file move, +# and the executor reports, together — at the phase's commit point. +# +# Staging is scoped so the owner's unrelated work is not swept in: +# stage = (everything dirty now) +# − (what was already dirty when the phase's first unit started — +# snapshot $PHASE_SESSIONS/dirty-, taken once per phase) +# − (pipeline runtime artifacts: phase-sessions/, pipeline.log) +# Work the owner dirtied mid-phase is indistinguishable from phase work and +# IS committed — the commit output prints exactly what went in. + +# Snapshot the worktree's dirty state at the phase's first unit (no-op +# outside a git work tree, and when the phase already has a snapshot — a +# resumed phase keeps the one taken at its start). +phase_dirty_snapshot() { + local phase="$1" f + git rev-parse --is-inside-work-tree >/dev/null 2>&1 || return 0 + f="$PHASE_SESSIONS/dirty-$phase" + if [[ ! -f "$f" ]]; then + git status --porcelain >"$f" 2>/dev/null || : >"$f" + fi +} + +# commit_phase +# Returns 0 on success, when there is nothing left to commit, or outside a +# git work tree. Returns 1 (after printing a ✗ ERROR block with the git +# error and the hand-fix command) when the phase's own artifacts could not +# be committed. The phase stays COMPLETE either way — the work passed +# validation; the caller stops the run so a commit miss is visible, never +# retried as a task and never swept into a later phase's commit. +commit_phase() { + local unit="$1" report="${2:-}" + local phase pre curf paths lits moved staged addout commitout subject msgf + phase="$(unit_phase "$unit")" + + if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then + warn "not a git work tree — no phase commit for $phase" + return 0 + fi + pre="$PHASE_SESSIONS/dirty-$phase" + + # Everything dirty now: tracked changes vs HEAD + untracked non-ignored. + curf="$(mktemp)" + { git diff --name-only HEAD; git ls-files --others --exclude-standard; } | sort -u >"$curf" + + # Quick exit: the phase's artifacts are already committed (moved file + # tracked and clean — e.g. a duplicate commit attempt after the snapshot + # was cleaned up) — never stage again, owner WIP stays untouched. + if git ls-files --error-unmatch -- "$PHASE_DONE/$unit" >/dev/null 2>&1 \ + && ! grep -qx -- "$PHASE_DONE/$unit" "$curf"; then + rm -f "$curf" + echo " (nothing to commit — the phase is already fully committed)" + return 0 + fi + + # Subtract pre-phase owner WIP (porcelain lines are "XY ") and the + # pipeline's own runtime artifacts. + paths="$(mktemp)" + if [[ -f "$pre" ]]; then + comm -23 "$curf" <(cut -c4- "$pre" | grep -v '^[[:space:]]*$' | sort -u) \ + | grep -vE '^\.agents/(phase-sessions/|pipeline\.log$)' >"$paths" || true + else + grep -vE '^\.agents/(phase-sessions/|pipeline\.log$)' "$curf" >"$paths" || true + fi + rm -f "$curf" + + if [[ ! -s "$paths" ]]; then + rm -f "$paths" "$pre" + echo " (nothing to commit — the phase is already fully committed)" + return 0 + fi + + # :(literal) keeps file names with glob characters ([?* ) from being read + # as pathspec patterns. + lits="$(mktemp)" + sed 's|^|:(literal)|' "$paths" >"$lits" + if ! addout="$(git add -A --pathspec-from-file="$lits" 2>&1)"; then + rm -f "$paths" "$lits" + echo "✗ ERROR: phase commit for $phase — staging failed:" >&2 + printf '%s\n' "$addout" | sed 's/^/ /' >&2 + return 1 + fi + rm -f "$lits" + + # Verify the artifacts that used to go missing: the moved phase file must + # be in the index (hard fail), and the executor report unless the project + # deliberately gitignores reports. + moved="$PHASE_DONE/$unit" + staged="$(git -c core.quotepath=off diff --cached --name-only -z | tr '\0' '\n')" + if ! grep -qx -- "$moved" <<<"$staged"; then + git reset -q + rm -f "$paths" + echo "✗ ERROR: phase commit for $phase — the moved phase file is not in the index ($moved)." >&2 + if git check-ignore -q -- "$moved" 2>/dev/null; then + echo " your .gitignore excludes it — .agents/phases must be tracked (SKILL.md setup notes)." >&2 + fi + echo " the work is complete and uncommitted; stage and commit it by hand." >&2 + return 1 + fi + if [[ -n "$report" && -f "$report" ]] && ! grep -qx -- "$report" <<<"$staged" \ + && ! git check-ignore -q -- "$report" 2>/dev/null; then + git reset -q + rm -f "$paths" + echo "✗ ERROR: phase commit for $phase — the executor report is not in the index ($report)." >&2 + echo " the work is complete and uncommitted; stage and commit it by hand." >&2 + return 1 + fi + rm -f "$paths" + + # Subject: PHASE_COMMIT_SUBJECT with {{PHASE}} substituted (default + # "phase: "); the executor's final report becomes the body. + # The default subject lives in a variable: a brace literal inside the + # ${var:-default} word would terminate the expansion early (bash does not + # nest plain braces). + local default_subject="phase: {{PHASE}}" + subject="${PHASE_COMMIT_SUBJECT:-$default_subject}" + subject="${subject//\{\{PHASE\}\}/$phase}" + msgf="$(mktemp)" + { + printf '%s\n' "$subject" + printf '\n' + [[ -n "$report" && -f "$report" ]] && sed -n '1,40p' "$report" + } >"$msgf" + + if ! commitout="$(git commit --no-gpg-sign -F "$msgf" 2>&1)"; then + rm -f "$msgf" + echo "✗ ERROR: phase commit for $phase FAILED — the phase is complete but UNCOMMITTED (the scoped changes are still staged)." >&2 + printf '%s\n' "$commitout" | sed 's/^/ /' >&2 + echo " finish by hand: git commit --no-gpg-sign -m 'phase: $phase'" >&2 + echo " re-running the pipeline will NOT commit this phase — fix the commit first." >&2 + return 1 + fi + rm -f "$msgf" "$pre" + echo " (committed: $(git log -1 --oneline))" + return 0 +} + # --- notifications ------------------------------------------------------------ # Send a push notification via ntfy after a unit completes. Reads # ~/.env/pi-ntfy.env (see the ntfy skill). No-ops (silently) when notifications @@ -337,7 +481,9 @@ notify_task() { # --- one unit (task, phase final pass, or legacy phase), with retries --------- # execute_unit # Returns 0 and moves the unit to complete/ on success; returns 1 after -# MAX_FIX_ATTEMPTS failed attempts (unit file is left in todo/). +# MAX_FIX_ATTEMPTS failed attempts (unit file is left in todo/), or 1 when +# the phase commit fails at a phase's commit point (unit stays in +# complete/ — the work is done; the commit must be finished by hand). execute_unit() { local unit="$1" local base attempt=1 errors="" @@ -346,6 +492,9 @@ execute_unit() { command -v node >/dev/null 2>&1 || die "node not found on PATH (needed to render task progress)" mkdir -p "$PHASE_DONE" "$PHASE_REPORTS" "$PHASE_SESSIONS" "$(unit_report_dir "$unit")" ensure_validate + # First unit of the phase: freeze the owner's pre-existing dirty state so + # the phase commit (commit_phase) can exclude it from staging. + phase_dirty_snapshot "$(unit_phase "$unit")" while (( attempt <= MAX_FIX_ATTEMPTS )); do echo "━━ $unit — attempt $attempt/$MAX_FIX_ATTEMPTS ━━" @@ -390,12 +539,19 @@ execute_unit() { move_unit "$unit" if is_phase_end "$unit"; then echo "✓ $unit → complete (phase $(unit_phase "$unit") done)" - if [[ "${PHASE_COMMIT:-0}" == "1" ]]; then - if git add -A 2>/dev/null && git commit --no-gpg-sign -m "phase: $(unit_phase "$unit")" >/dev/null 2>&1; then - echo " (committed)" - else - warn "git commit failed (continuing)" + if [[ "${PHASE_COMMIT:-1}" == "1" ]]; then + if ! commit_phase "$unit" "$(unit_report "$unit" "$attempt" md)"; then + # The work is done and validated; the unit stays in complete/ and + # re-running will NOT re-execute this phase. Stop the run so the + # commit miss is visible — commit_phase printed the hand-fix. + echo " phase work is UNCOMMITTED — fix the commit first (see above); re-running continues at the next phase" >&2 + return 1 fi + else + # Explicit opt-out (PHASE_COMMIT=0): respect it, but never let the + # miss be silent — a completed phase with uncommitted work used to + # pile up in the worktree unnoticed. + warn "PHASE_COMMIT=0 — phase $(unit_phase "$unit") is COMPLETE but UNCOMMITTED; its work is left in the worktree — commit it by hand" fi else echo "✓ $unit → complete" diff --git a/phased-execution/scripts/run-phase.sh b/phased-execution/scripts/run-phase.sh index a010a34..76a1826 100755 --- a/phased-execution/scripts/run-phase.sh +++ b/phased-execution/scripts/run-phase.sh @@ -44,9 +44,9 @@ while unit="$(phase_next_unit "$phase")"; do execute_unit "$unit" || { failed=1; break; } done if (( failed )); then - echo "✗ ERROR: phase $phase FAILED — see error above" >&2 - echo " reports: $(unit_report_dir "$unit")/$(unit_base "$unit").a*.{md,err,validate}" >&2 - echo " resume: re-run this script — the failed executor session is resumed automatically" >&2 + # execute_unit printed the failure detail (task failure: errors, logs, + # resume command — or phase-commit failure: the hand-fix command). + echo "✗ ERROR: phase $phase did not complete — see the error output above" >&2 exit 1 fi echo "✓ phase $phase complete" diff --git a/phased-execution/scripts/run-task.sh b/phased-execution/scripts/run-task.sh index fa5d040..28b9060 100644 --- a/phased-execution/scripts/run-task.sh +++ b/phased-execution/scripts/run-task.sh @@ -40,7 +40,7 @@ build_pi_args if execute_unit "$unit"; then exit 0 fi -echo "✗ ERROR: task $unit FAILED after $MAX_FIX_ATTEMPTS attempts" >&2 -echo " reports: $(unit_report_dir "$unit")/$(unit_base "$unit").a*.{md,err,validate}" >&2 -echo " resume: re-run this script — the failed executor session is resumed automatically" >&2 +# execute_unit printed the failure detail (task failure: errors, logs, +# resume command — or phase-commit failure: the hand-fix command). +echo "✗ ERROR: $unit did not complete — see the error output above" >&2 exit 1