GodopuandClaude Fable 5.1
6b7198f73c
Issue #25: a promoted plan is reused only with --plan-file; the implicit newest-plan load is gone in both runners, and an empty planner report no longer passes silently. VERSIONS.md carries the entry; the four merge-side review reports and their identity sidecars (base6fc9194-> head1e82651, evidence review-tree) are archived under .agents/reports/<session>/. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
33 lines
3.8 KiB
Markdown
33 lines
3.8 KiB
Markdown
# PR #32 independent review
|
||
|
||
Reviewer: planner-reviewer-codex-01. Job: 08808795.
|
||
|
||
Reviewed the exact detached review-identity path specified by the brief:
|
||
`/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20260921T150212Z-97d845/1`, at `1e826515`, for the five commits after base `6fc9194`. No checkout, source file, session, monitor, or real registry state was modified.
|
||
|
||
## Findings
|
||
|
||
The explicit plan-reuse change is coherent and closes the implicit-load defect.
|
||
|
||
- With neither `--plan` nor `--plan-file`, Python self-planning logs that no plan is loaded and does not glob `.agents/reports/<planner>/plan-*.md`. The legacy shell runner does the same. `resolve_planner_session` may still identify a planner for other loop bookkeeping, but it no longer supplies a plan through that path.
|
||
- `--plan-file` is parsed by both wrappers. Python resolves relative paths against the control root, preserves absolute and `~` paths, requires a readable regular file with non-whitespace content, and rejects bad inputs before registering jobs. It is mutually exclusive with `--plan`; `--planner` still requires `--plan`; review-only accepts a valid file but warns and ignores it because no implementation prompt is produced. The legacy shell runner rejects the option explicitly when `MAM_LOOP_PY=0`, before the loop lock.
|
||
- `_plan_text()` is the shared check for explicit files, initial planner reports, discussion refinements, and escalation refinements. An empty initial planner report stops the run rather than silently falling back to self-planning. An empty refine report preserves the existing plan. Correction prompts therefore never claim to carry an empty plan.
|
||
- Checkpoint `current_plan` is not a plan-discovery source. It is only recorded for the current run; a later no-option run starts planless, and a named `--plan-file` is read explicitly. Promotion still writes the plan only for an actual `--plan` run.
|
||
- Relative plan paths with spaces, missing paths, directories, empty/whitespace files, unreadable files, `--plan` conflicts, review-only handling, and legacy-shell rejection are covered by P1–P11. The tests run in temporary control roots and use isolated reports; they do not write the real `.mam` or `.agents/reports`.
|
||
- The loop skill’s old implicit-reuse wording is removed. The CLI usage, workflow examples, option table, and explicit-reuse description agree. No `.claude/skills` deployment copy exists in this checkout, so there is no second copy to synchronize.
|
||
|
||
The known release prerequisite remains outside this PR: the orchestrator must perform the 4.7.1 lockstep version bump and release entry before merge.
|
||
|
||
## Verification
|
||
|
||
All tests used `PYTHONDONTWRITEBYTECODE=1`, the interpreter from the brief, pytest cache disabled, and a basetemp outside the review path.
|
||
|
||
- `tests/test_issue25_plan_reuse.py`: **17 passed** in 1.70 s.
|
||
- `tests/test_issue25_plan_reuse.py`, `tests/test_loop_cli.py`, `tests/test_issue5_phase3_loop.py`, and `tests/test_version_consistency.py`: **39 passed, 2 failed** in 49.25 s when run exactly from the identity path. Both failures occurred before their assertions because `run_loop.sh` could not find `.venv` in the detached snapshot and fell back to system Python, which lacked `paho` (`RuntimeError: Failed to eager load lib_py.loop.autocomplete: No module named 'paho'`). This is a snapshot/setup issue, not a change in the plan-reuse code.
|
||
- Re-running the two failed compatibility cases with `AGENT_PYTHON_BIN` pinned to the brief’s main interpreter: **2 passed** in 0.32 s. This reaches and verifies the expected session-registration checks.
|
||
- `git diff --check 6fc9194...HEAD`: passed.
|
||
|
||
The focused plan tests passed without modification, and the only broader failures disappeared when the required interpreter was explicitly supplied. I found no additional merge-blocking defect beyond the known version gate.
|
||
|
||
[VERDICT: PASS]
|