fix(issue-25): reuse a promoted plan only with --plan-file; drop the implicit newest-plan load
Without --plan, Phase 1 resolved a planner session automatically and globbed <workspace_root>/.agents/reports/<planner>/plan-*.md for the newest file by mtime, handing it to the Creator as "the plan already agreed on". Both keys are wrong: the report directory belongs to a session, not to this task, so other issues' plans pile up in it, and right after a clone or checkout mtime is checkout order. 47 such plans are committed in this repository, so an ordinary run could silently implement against an unrelated design. Reuse is explicit now. --plan-file <path> is the only way to load an existing plan; it is resolved against the control root, checked before a single job exists, and refused if the file is missing, not a regular file, unreadable or empty (an empty file would quietly turn into a "no plan" prompt). It cannot be combined with --plan, and --review-only warns and ignores it because no implementation job is delegated there. The implementation prompt itself is unchanged, so the Creator's contract does not move. Phase 1 now says which plan it loaded and where from, or that it loaded none. The legacy shell loop (MAM_LOOP_PY=0) loses the same auto-load and rejects --plan-file rather than duplicating the check on a path that is on its way out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
6fc9194f89
commit
a28711cccf
2 files changed
+67
-18
No files matched your search
@@ -9,7 +9,6 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import argparse
|
import argparse
|
||||||
import atexit
|
import atexit
|
||||||
import glob
|
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
@@ -98,6 +97,7 @@ class LoopRunner:
|
|||||||
self.creator: str = ""
|
self.creator: str = ""
|
||||||
self.task: str = ""
|
self.task: str = ""
|
||||||
self.plan_mode: bool = False
|
self.plan_mode: bool = False
|
||||||
|
self.plan_file: Optional[str] = None
|
||||||
self.planner_override: Optional[str] = None
|
self.planner_override: Optional[str] = None
|
||||||
self.plan_talk_turns: int = 1
|
self.plan_talk_turns: int = 1
|
||||||
self.reviewer_list: Optional[str] = None
|
self.reviewer_list: Optional[str] = None
|
||||||
@@ -116,6 +116,7 @@ class LoopRunner:
|
|||||||
" --plan Enable Planner agent intervention & design phase",
|
" --plan Enable Planner agent intervention & design phase",
|
||||||
" --planner <name> Planner session (requires --plan; default: auto-resolve)",
|
" --planner <name> Planner session (requires --plan; default: auto-resolve)",
|
||||||
" --plan-talk N Planner-Creator discussion limit turns (default: 1)",
|
" --plan-talk N Planner-Creator discussion limit turns (default: 1)",
|
||||||
|
" --plan-file <path> Reuse an existing plan file, relative to the control root (not with --plan)",
|
||||||
" --reviewer \"A,B\" Targeted reviewer session name list (comma-separated)",
|
" --reviewer \"A,B\" Targeted reviewer session name list (comma-separated)",
|
||||||
" --all-reviewer Enforce PASS verdict from all active reviewer sessions",
|
" --all-reviewer Enforce PASS verdict from all active reviewer sessions",
|
||||||
" --max-loop N Max execution-review corrective loop runs (default: 3)",
|
" --max-loop N Max execution-review corrective loop runs (default: 3)",
|
||||||
@@ -162,6 +163,12 @@ class LoopRunner:
|
|||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
self.planner_override = args[i + 1]
|
self.planner_override = args[i + 1]
|
||||||
i += 2
|
i += 2
|
||||||
|
elif arg == "--plan-file":
|
||||||
|
if i + 1 >= len(args):
|
||||||
|
self.print_usage()
|
||||||
|
sys.exit(1)
|
||||||
|
self.plan_file = args[i + 1]
|
||||||
|
i += 2
|
||||||
elif arg == "--plan-talk":
|
elif arg == "--plan-talk":
|
||||||
if i + 1 >= len(args) or not re.match(r"^[0-9]+$", args[i + 1]):
|
if i + 1 >= len(args) or not re.match(r"^[0-9]+$", args[i + 1]):
|
||||||
print("ERROR: --plan-talk requires a positive integer.", file=sys.stderr)
|
print("ERROR: --plan-talk requires a positive integer.", file=sys.stderr)
|
||||||
@@ -240,6 +247,28 @@ class LoopRunner:
|
|||||||
print(f' run_loop.sh --creator <creator> --plan --planner <planner> --task "..."', file=sys.stderr)
|
print(f' run_loop.sh --creator <creator> --plan --planner <planner> --task "..."', file=sys.stderr)
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
|
|
||||||
|
# Issue #25: a promoted plan is reused only when it is named (D2). The
|
||||||
|
# file is read here, before a single job exists, so a bad path cannot
|
||||||
|
# surface halfway through a run (D3).
|
||||||
|
if self.plan_file is not None:
|
||||||
|
if self.plan_mode:
|
||||||
|
print("ERROR: --plan-file cannot be combined with --plan.", file=sys.stderr)
|
||||||
|
print("Use --plan to have the Planner write a new plan, or --plan-file to reuse one:", file=sys.stderr)
|
||||||
|
print(' run_loop.sh --creator <creator> --plan-file <path> --task "..."', file=sys.stderr)
|
||||||
|
sys.exit(1)
|
||||||
|
resolved = Path(self.plan_file).expanduser().resolve()
|
||||||
|
text = self._plan_text(resolved)
|
||||||
|
if text is None:
|
||||||
|
log_error(f"--plan-file {resolved}: not a readable file with plan text in it.")
|
||||||
|
sys.exit(1)
|
||||||
|
self.plan_file = str(resolved)
|
||||||
|
if self.review_only:
|
||||||
|
# D6: --review-only delegates no implementation job, so there is
|
||||||
|
# nothing for a plan to steer.
|
||||||
|
log_warn(f"--plan-file is ignored with --review-only: {resolved}")
|
||||||
|
else:
|
||||||
|
self.current_plan = text
|
||||||
|
|
||||||
if self.all_reviewers and self.reviewer_list:
|
if self.all_reviewers and self.reviewer_list:
|
||||||
log_warn(f"--all-reviewer takes precedence; ignoring --reviewer list ('{self.reviewer_list}').")
|
log_warn(f"--all-reviewer takes precedence; ignoring --reviewer list ('{self.reviewer_list}').")
|
||||||
|
|
||||||
@@ -266,6 +295,21 @@ class LoopRunner:
|
|||||||
if not self._context_active() and any(a in self.raw_argv for a in ("--review-isolation", "--remote-review")):
|
if not self._context_active() and any(a in self.raw_argv for a in ("--review-isolation", "--remote-review")):
|
||||||
log_warn("--review-isolation/--remote-review only apply with --worktree or --base; ignored.")
|
log_warn("--review-isolation/--remote-review only apply with --worktree or --base; ignored.")
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _plan_text(path: Path) -> Optional[str]:
|
||||||
|
"""Issue #25 (D3): the one check every plan load shares - a plan is a
|
||||||
|
readable regular file that has text in it. None means 'no plan here';
|
||||||
|
each caller decides what that means (D13 stops the run, D14 keeps the
|
||||||
|
plan it already has). A file that cannot be read is not told apart from
|
||||||
|
an empty one: either way the loop would carry no plan text."""
|
||||||
|
try:
|
||||||
|
if not path.is_file():
|
||||||
|
return None
|
||||||
|
text = path.read_text(encoding="utf-8", errors="replace")
|
||||||
|
except OSError:
|
||||||
|
return None
|
||||||
|
return text if text.strip() else None
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _git(cwd: Path, *args: str) -> subprocess.CompletedProcess:
|
def _git(cwd: Path, *args: str) -> subprocess.CompletedProcess:
|
||||||
return subprocess.run(["git", "-C", str(cwd), *args], capture_output=True, text=True)
|
return subprocess.run(["git", "-C", str(cwd), *args], capture_output=True, text=True)
|
||||||
@@ -1002,11 +1046,15 @@ class LoopRunner:
|
|||||||
log_success("Interactive planning completed. Plan finalized.")
|
log_success("Interactive planning completed. Plan finalized.")
|
||||||
else:
|
else:
|
||||||
log_info("=== Phase 1: Self-Planning Mode (Direct Execution) ===")
|
log_info("=== Phase 1: Self-Planning Mode (Direct Execution) ===")
|
||||||
if planner_session:
|
# Issue #25 (D1): no plan is loaded unless one was named. This used
|
||||||
plan_files = sorted(glob.glob(str(self.workspace_root / ".agents" / "reports" / planner_session / "plan-*.md")), key=os.path.getmtime, reverse=True)
|
# to take the newest promoted plan in the planner's report dir,
|
||||||
if plan_files:
|
# keyed on mtime (checkout order) and on a planner session that says
|
||||||
log_info(f"Found existing promoted plan at '{plan_files[0]}'. Loading plan...")
|
# nothing about this task - so it injected other issues' plans.
|
||||||
self.current_plan = Path(plan_files[0]).read_text(encoding="utf-8", errors="replace")
|
if self.current_plan:
|
||||||
|
mtime = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime(os.path.getmtime(self.plan_file)))
|
||||||
|
log_info(f"Using the plan given with --plan-file: {self.plan_file} (modified {mtime} UTC)")
|
||||||
|
else:
|
||||||
|
log_info("No plan loaded; the Creator plans by itself (reuse one with --plan-file <path>).")
|
||||||
|
|
||||||
# =======================================================================
|
# =======================================================================
|
||||||
# PHASE 2: IMPLEMENTATION (CREATION)
|
# PHASE 2: IMPLEMENTATION (CREATION)
|
||||||
|
|||||||
@@ -32,6 +32,7 @@ TARGET_AGENT=""
|
|||||||
TASK=""
|
TASK=""
|
||||||
REVIEWER_LIST=""
|
REVIEWER_LIST=""
|
||||||
PLANNER_SESSION_OVERRIDE=""
|
PLANNER_SESSION_OVERRIDE=""
|
||||||
|
LOOP_PLAN_FILE=""
|
||||||
LOOP_WORKTREE=""
|
LOOP_WORKTREE=""
|
||||||
LOOP_BASE=""
|
LOOP_BASE=""
|
||||||
LOOP_REVIEW_ONLY=0
|
LOOP_REVIEW_ONLY=0
|
||||||
@@ -46,6 +47,7 @@ usage() {
|
|||||||
echo " --plan Enable Planner agent intervention & design phase"
|
echo " --plan Enable Planner agent intervention & design phase"
|
||||||
echo " --planner <name> Planner session (requires --plan; default: auto-resolve)"
|
echo " --planner <name> Planner session (requires --plan; default: auto-resolve)"
|
||||||
echo " --plan-talk N Planner-Creator discussion limit turns (default: 1)"
|
echo " --plan-talk N Planner-Creator discussion limit turns (default: 1)"
|
||||||
|
echo " --plan-file <path> Reuse an existing plan file, relative to the control root (not with --plan)"
|
||||||
echo " --reviewer \"A,B\" Targeted reviewer session name list (comma-separated)"
|
echo " --reviewer \"A,B\" Targeted reviewer session name list (comma-separated)"
|
||||||
echo " --all-reviewer Enforce PASS verdict from all active reviewer sessions"
|
echo " --all-reviewer Enforce PASS verdict from all active reviewer sessions"
|
||||||
echo " --max-loop N Max execution-review corrective loop runs (default: 3)"
|
echo " --max-loop N Max execution-review corrective loop runs (default: 3)"
|
||||||
@@ -92,6 +94,7 @@ while [[ "$#" -gt 0 ]]; do
|
|||||||
exit 1
|
exit 1
|
||||||
;;
|
;;
|
||||||
--planner) PLANNER_SESSION_OVERRIDE="$2"; shift 2 ;;
|
--planner) PLANNER_SESSION_OVERRIDE="$2"; shift 2 ;;
|
||||||
|
--plan-file) LOOP_PLAN_FILE="$2"; shift 2 ;;
|
||||||
--task) TASK="$2"; shift 2 ;;
|
--task) TASK="$2"; shift 2 ;;
|
||||||
--worktree) LOOP_WORKTREE="$2"; shift 2 ;;
|
--worktree) LOOP_WORKTREE="$2"; shift 2 ;;
|
||||||
--base) LOOP_BASE="$2"; shift 2 ;;
|
--base) LOOP_BASE="$2"; shift 2 ;;
|
||||||
@@ -121,11 +124,14 @@ fi
|
|||||||
|
|
||||||
# Issue #21 (D12): the legacy shell loop assumes a cwd-relative .mam/jobs and
|
# Issue #21 (D12): the legacy shell loop assumes a cwd-relative .mam/jobs and
|
||||||
# diffs the checkout it runs in; the worktree/base options are Python-only.
|
# diffs the checkout it runs in; the worktree/base options are Python-only.
|
||||||
|
# Issue #25 (D9): --plan-file is too - its fail-closed check lives in the
|
||||||
|
# runner and is not duplicated in a path that is on its way out.
|
||||||
# Checked before the freeze and the loop lock (log_* are not defined yet).
|
# Checked before the freeze and the loop lock (log_* are not defined yet).
|
||||||
if [ "${MAM_LOOP_PY:-1}" = "0" ] && { [ -n "$LOOP_WORKTREE" ] || [ -n "$LOOP_BASE" ] \
|
if [ "${MAM_LOOP_PY:-1}" = "0" ] && { [ -n "$LOOP_WORKTREE" ] || [ -n "$LOOP_BASE" ] \
|
||||||
|| [ "$LOOP_REVIEW_ONLY" = "1" ] || [ -n "${MAM_LOOP_WORKTREE:-}" ] \
|
|| [ "$LOOP_REVIEW_ONLY" = "1" ] || [ -n "${MAM_LOOP_WORKTREE:-}" ] \
|
||||||
|| [ -n "$LOOP_REVIEW_ISOLATION" ] || [ -n "$LOOP_REMOTE_REVIEW" ]; }; then
|
|| [ -n "$LOOP_REVIEW_ISOLATION" ] || [ -n "$LOOP_REMOTE_REVIEW" ] \
|
||||||
echo "ERROR: --worktree/--base/--review-only/--review-isolation/--remote-review require the Python loop (MAM_LOOP_PY=1)" >&2
|
|| [ -n "$LOOP_PLAN_FILE" ]; }; then
|
||||||
|
echo "ERROR: --worktree/--base/--review-only/--review-isolation/--remote-review/--plan-file require the Python loop (MAM_LOOP_PY=1)" >&2
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
@@ -560,16 +566,11 @@ if [ "$PLAN_MODE" = true ]; then
|
|||||||
log_success "Interactive planning completed. Plan finalized."
|
log_success "Interactive planning completed. Plan finalized."
|
||||||
else
|
else
|
||||||
log_info "=== Phase 1: Self-Planning Mode (Direct Execution) ==="
|
log_info "=== Phase 1: Self-Planning Mode (Direct Execution) ==="
|
||||||
EXISTING_PLAN_FILE=""
|
# Issue #25 (D1/D9): no plan is loaded here. This used to take the newest
|
||||||
if [ -n "$PLANNER_SESSION" ]; then
|
# promoted plan of the auto-resolved planner, keyed on mtime and on a session
|
||||||
# Promoted plans are saved as plan-<job-id>.md by the Phase 3 promotion
|
# that says nothing about this task. Reuse is explicit and Python-only now
|
||||||
# step below, not report-final.md; pick the most recently promoted one.
|
# (--plan-file), and this path rejects that option above.
|
||||||
EXISTING_PLAN_FILE=$(ls -t ".agents/reports/$PLANNER_SESSION"/plan-*.md 2>/dev/null | head -n 1 || true)
|
log_info "No plan loaded; the Creator plans by itself."
|
||||||
fi
|
|
||||||
if [ -n "$EXISTING_PLAN_FILE" ] && [ -f "$EXISTING_PLAN_FILE" ]; then
|
|
||||||
log_info "Found existing promoted plan at '$EXISTING_PLAN_FILE'. Loading plan..."
|
|
||||||
CURRENT_PLAN=$(cat "$EXISTING_PLAN_FILE")
|
|
||||||
fi
|
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# ===========================================================================
|
# ===========================================================================
|
||||||
|
|||||||
Reference in new issue
Block a user