# Pre-Merge Independent Review Report: PR #32 (`fix/issue-25`)

- **Job ID**: `41f07cde`
- **Target Agent**: `agy` (Session: `herdr:reviewer-agy-01`)
- **Role**: Reviewer
- **Target Worktree Context**:
  - **Loop ID**: `loop-20260921T150212Z-97d845`
  - **Base Commit**: `6fc9194f896eb2e13e123b8ebe7a9089b629764d` (v4.7.0)
  - **Head Commit**: `1e826515cc2a07ea1f8da0ff1259e1ea4dde86fd`
  - **Tree SHA**: `4fd82ad1c2682d12c3f5114818f97291af28a032`
  - **Verification Path**: `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20260921T150212Z-97d845/1`

---

## 1. Executive Summary

PR #32 resolves Issue #25 ("승격된 계획서 재사용을 --plan-file 로만 허용 — 암묵적 자동 로드 제거") across 5 commits and 4 files (+486 / −31):
- `1e82651` fix(issue-25): a relative --plan-file is taken against the control root, as its usage line says
- `5fa5923` docs(issue-25): the loop skill documents --plan-file and the empty-report handling
- `a334083` test(issue-25): plan reuse is explicit only and empty plan reports are not silent (P1-P11)
- `e551e3b` fix(issue-25): one plan-text check for every load; an empty planner report stops the loop, an empty refine keeps the plan
- `a28711c` fix(issue-25): reuse a promoted plan only with --plan-file; drop the implicit newest-plan load

All checklist criteria have been verified in the isolated verification path. All 17 new tests in `test_issue25_plan_reuse.py` and all 115 baseline regression tests passed with zero regressions.

---

## 2. Detailed Verification Against Checklist Items

### (1) Removal of Implicit Plan Loading
- **Python Runner (`lib_py/loop/runner.py`)**:
  - Removed `import glob` and the directory scan `glob.glob(str(self.workspace_root / ".agents" / "reports" / planner_session / "plan-*.md"))`.
  - In Phase 1 Self-Planning mode, if `--plan-file` was not supplied, `self.current_plan` remains empty and the runner logs: `"No plan loaded; the Creator plans by itself (reuse one with --plan-file <path>)."`.
- **Legacy Shell Runner (`scripts/run_loop.sh`)**:
  - Removed `EXISTING_PLAN_FILE=$(ls -t ".agents/reports/$PLANNER_SESSION"/plan-*.md 2>/dev/null | head -n 1 || true)`.
  - In Self-Planning mode, logs `"No plan loaded; the Creator plans by itself."`.
- **Parity & Search Check**:
  - Test `test_p8_neither_loop_implementation_globs_for_a_plan_any_more` confirms the string pattern `plan-*.md` does not appear anywhere in `runner.py` or `run_loop.sh`.
  - Automatic planner resolution (`resolve_planner_session`) is not called or consulted to retrieve stored plans in Phase 1 self-planning mode.

### (2) `--plan-file <path>` CLI Option Semantics
- **Control Root Resolution**:
  - Relative paths are anchored against `self.workspace_root` via `(self.workspace_root / Path(self.plan_file).expanduser()).resolve()`. This guarantees identical path resolution whether the command is run from the repository root, a subdirectory, or an isolated worktree (`test_p3c`).
  - Absolute paths and user home expansion (`~`) remain untouched and work as expected (`test_p3d`).
- **Fail-Closed Early Validation**:
  - Evaluated in `parse_args()` before any agent delegation or job submission occurs (`test_p3`, `test_p3b`).
  - If the path is non-existent, a directory, unreadable, or contains whitespace-only text, the runner logs an error naming the resolved path and immediately exits with rc=1.
- **Mutual Exclusion & Option Combinations**:
  - Combining `--plan-file` with `--plan` exits 1 with a clear mutual exclusion error (`test_p4`).
  - Specifying `--plan-file` without an argument exits 1 (`test_p4`).
  - When combined with `--review-only`, the runner logs a warning and ignores the plan (`self.current_plan = ""`), because Phase 2 implementation is skipped (`test_p5`, `test_p5b`).
- **Legacy Shell Rejection**:
  - When invoked with `MAM_LOOP_PY=0`, `run_loop.sh` explicitly detects `--plan-file` and terminates with code 1 before the loop marker/guard is created (`test_p7b`).

### (3) Centralized Plan Text Validation (`e551e3b`)
- **Shared Validation Method**:
  - `LoopRunner._plan_text(path: Path) -> Optional[str]` enforces:
    1. Regular file check (`path.is_file()`).
    2. Safe decoding with error replacement (`read_text(encoding="utf-8", errors="replace")`).
    3. Stripped text check (`text if text.strip() else None`).
- **Initial Plan Failure**:
  - In `--plan` mode, if the initial planner report is empty or whitespace-only, the runner logs `the planner's report has no plan text: <path>` and exits with code 1 (`test_p9`). This eliminates the regression where an empty plan silently demoted the run to self-planning.
- **Refinement Retention (Debate & Review Escalation)**:
  - In Phase 1 interactive debate (`--plan-talk`) and Phase 3 review escalation (`[ESCALATE: PLANNER]`), an empty refinement report logs a warning and preserves the previously valid plan (`test_p10`, `test_p11`).
  - This prevents prompt corruption in `correction_prompt = f"갱신된 다음 계획서에 입각하여 ... 계획서:\n{self.current_plan}"`.

### (4) Checkpoint Integrity
- `LoopRunner` persists `current_plan` to `.mam/loop-checkpoint.json` for diagnostic and monitoring visibility during active loops.
- `LoopRunner` never reads `.mam/loop-checkpoint.json` at startup to populate `self.current_plan`. `self.current_plan` is always initialized cleanly (`self.current_plan = ""`).
- Existing checkpoint files in the workspace (such as unrelated historical plan text from prior issues) cannot contaminate new loop runs.

### (5) Baseline Behavior & Creator Prompts
- In self-planning mode without `--plan-file`, the initial Creator prompt retains the exact contract:
  `"계획서가 존재하지 않으므로, 작업자(Creator)의 판단하에 스스로 구현 계획 및 설계를 수립한 뒤, 이를 바탕으로 코드를 구현하고 다음 작업 목표를 완성해주세요. 작업 목표: {self.task}"`.
- With `--plan-file <path>`, the prompt receives:
  `"이미 수립된 다음 계획서에 입각하여 작업자의 판단하에 코드를 구현하고 작업 목표를 완성해주세요. 계획서:\n{self.current_plan}\n\n작업 목표: {self.task}"`.
- Subsequent review corrective rounds strictly provide reviewer feedback without repeating the initial plan (`test_p2`).
- Promotion of newly planned tasks via `--plan` to `.agents/reports/<planner>/plan-<id>.md` remains operational and protected (`test_p6`).

### (6) Test Suite Execution & Isolation (P1–P11)
- All 17 test cases in `tests/test_issue25_plan_reuse.py` operate strictly within isolated `tmp_path` fixtures with stubbed delegations.
- Real `.mam/jobs`, live daemon processes, and global agent sessions remain completely untouched.
- Execution timing: 17 tests passed in 2.02 seconds.

### (7) Documentation Integrity
- `.agents/skills/multi-agent-mux-loop/SKILL.md` was thoroughly updated:
  - Completely purged the stale statement ("기존에 승격된 계획서가 있다면 이를 로드").
  - Updated Sequence Diagram and Phase 1 documentation to reflect explicit `--plan-file` reuse.
  - Added a dedicated section: `### 계획서가 비어 있을 때 (Empty plan reports)`.
  - Updated CLI Option mapping table and Workflow examples.
- No dangling references to the old implicit loading mechanism exist across the repository.

### (8) Versioning & Merge Readiness
- Version lockstep bump to `v4.7.1` is reserved for the orchestrator release commit in accordance with release procedures.
- Formatting and git diff checks (`git diff 6fc9194...HEAD --check`) reported 0 issues.
- No merge blockers, regression risks, or side-effects detected.

---

## 3. Test Execution Summary

| Test Suite | Result | Duration | Notes |
| :--- | :--- | :--- | :--- |
| `tests/test_issue25_plan_reuse.py` | **17 / 17 PASS** | 2.02s | P1–P11 test matrix covering all reuse/empty-plan conditions |
| `tests/test_loop_cli.py` | **9 / 9 PASS** | 38.97s | CLI interface and argument validation regressions |
| `tests/test_issue5_phase2a_loop.py` | **91 / 91 PASS** | 2.91s | Core loop state machine, dispatching, and verdict handling |
| `tests/test_issue5_phase3_loop.py` | **12 / 12 PASS** | 0.54s | Runner orchestration, timeouts, and signal handling |
| `tests/test_version_consistency.py` | **3 / 3 PASS** | 0.21s | Version consistency checks |
| `git diff 6fc9194...HEAD --check` | **0 errors** | 0.05s | Zero trailing whitespace / formatting errors |

---

[VERDICT: PASS]