From b62aa581aa014fc6e43f3d5f1acb2a01863bf7c3 Mon Sep 17 00:00:00 2001 From: v0 Date: Mon, 13 Jul 2026 15:41:55 +0000 Subject: [PATCH 1/2] fix: close-before-report ordering defect from fourth field run (FN-027/FN-028) Root cause correction for FN-023: done decided success by the gate's return code alone, but the gate can fail AFTER the task was closed and committed (late stages query 'the current active task', which no longer exists). done now decides by facts - if the task flipped to [x], the full ledger runs even on gate failure, with an explicit GATE INCONSISTENCY note. Closeout context (--next, acceptance verdicts, verify results) is snapshotted to gitignored .coderail/pending_close.json BEFORE anything mutates; progress --repair restores the real parameters from a surviving snapshot. finish_task reports 'done (WITH ERRORS)' instead of 'blocked' when the close actually happened; the rerun hint only appears for genuinely open tasks. done ends with the same audit as coderail progress as a fuse. Three end-to-end regression tests against the real flow, no mocks (76 total). Co-authored-by: v0 --- CHANGELOG.md | 21 ++++++ scripts/coderail.py | 144 +++++++++++++++++++++++++++++++++++++--- scripts/finish_task.py | 15 ++++- scripts/init_project.py | 3 +- tests/test_structure.py | 77 +++++++++++++++++++++ 5 files changed, 248 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb7509e..a881b47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,26 @@ # Changelog +## Unreleased + +Close-before-report ordering fix: closes the fourth round of field findings (FN-027/FN-028) and corrects the root-cause analysis of FN-023. The theme: the closeout ledger runs from a snapshot taken before anything mutates, and every verdict printed matches what actually happened on disk. + +### Root cause correction (FN-027, supersedes the FN-023 analysis) + +- The v0.8.2 FN-023 fix targeted the wrong branch: it assumed warning paths skipped the ledger, but the fourth field run (T-191, zero warnings) proved the real defect is ordering - `done` decided success purely by the gate's return code, and the gate can return failure AFTER the task was already closed and committed (its late stages query "the current active task", which by then no longer exists). The old rc-only branch then skipped the whole ledger and printed a misleading "run done again". +- `done` now decides by facts, not return codes: after the gate runs, it checks whether the task actually flipped to `[x]` in TASKS.md. A closed-and-committed task gets its full ledger (journal entry, on-disk report, deferred queueing) even when the gate reports failure, with an explicit `GATE INCONSISTENCY` note instead of silence. +- The "run done again" hint is only printed when the task is genuinely still open; a failure on an already-closed task points to `coderail progress --repair` instead. +- `finish_task`'s Finish Task Report matches reality: when checks fail after the task was marked done and committed, it reports `done (WITH ERRORS)` with an explicit do-not-rerun note, never a bare "blocked". +- Built-in fuse: `done` ends by running the same audit as `coderail progress` and hard-fails with `LEDGER ERROR` if the journal entry it just claimed to write is not actually on disk. + +### Closeout snapshot (FN-028) + +- Before any state-mutating step, `done` persists the full closeout context to `.coderail/pending_close.json` (gitignored): task id, display id, title, `--next` text, acceptance items and verdicts, manual-acceptance note, verify results. No ledger step depends on "the current active task" any more. +- The snapshot is deleted only after the ledger is fully written. If a close is interrupted mid-ledger, `progress --repair` reads the surviving snapshot and restores the REAL `--next` text, per-item acceptance verdicts, and verify evidence into the retroactive entry - not default copy. + +### Tests + +- Lessons applied from FN-023's failed fix: the three new regression tests are end-to-end against the real `done` flow, no mocked report layer. They assert (a) two consecutive closes each leave all four artifacts at once (journal entry with verbatim `--next`, on-disk report with a non-blocked Done Gate, closed TASKS entry, task commit) plus a clean audit; (b) a sabotaged journal keeps the snapshot and `--repair` restores the original parameters verbatim; (c) the rerun hint only appears for genuinely open tasks (76 total). + ## v0.8.3 Ledger integrity and gate coherence: closes the third round of field findings (FN-020..FN-024) from the timebuild run. The theme: closing a task and recording that close are one transaction, and both ends of a task's life apply the same rules. diff --git a/scripts/coderail.py b/scripts/coderail.py index 06bf986..95338f5 100644 --- a/scripts/coderail.py +++ b/scripts/coderail.py @@ -13,6 +13,7 @@ from __future__ import annotations import argparse +import json import re import subprocess import sys @@ -862,6 +863,34 @@ def cmd_check(args) -> int: BOILERPLATE_VERIFY = "Manually confirm the result works as intended." +# FN-027/FN-028: everything the closeout ledger needs is snapshotted BEFORE +# any state-mutating step runs, and persisted to disk so a crash or a +# mid-flow "task not found" can never lose it. progress --repair reads it. + +def pending_close_path(root: Path) -> Path: + return root / ".coderail" / "pending_close.json" + + +def write_pending_close(root: Path, snapshot: dict) -> None: + path = pending_close_path(root) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(snapshot, ensure_ascii=False, indent=2), encoding="utf-8") + + +def load_pending_close(root: Path) -> dict: + try: + return json.loads(pending_close_path(root).read_text(encoding="utf-8")) + except (FileNotFoundError, ValueError, OSError): + return {} + + +def clear_pending_close(root: Path) -> None: + try: + pending_close_path(root).unlink() + except (FileNotFoundError, OSError): + pass + + def cmd_done(args) -> int: root = Path(args.target).resolve() @@ -943,6 +972,29 @@ def cmd_done(args) -> int: for w in tdd_warnings: print(f"WARNING: {w}") + # FN-027/FN-028: snapshot the full closeout context to disk BEFORE the + # gate runs. From here on, no ledger step may depend on "the current + # active task" - the task will stop being active mid-flow by design. + snapshot_title = "" + if task_before: + for t in list_tasks(tasks_text_before): + if t["id"] == task_before: + snapshot_title = t["title"] + break + write_pending_close(root, { + "task": task_before, + "display_id": meta.get("display_id", ""), + "title": snapshot_title, + "next_hint": (args.next_hint or "").strip(), + "accept_items": accept_items, + "accept_statuses": accept_statuses, + "manual_acceptance": args.manual_acceptance or "", + "verify_results": [ + {"cmd": r["cmd"], "exit": r["exit"]} for r in verify_results + ], + "stamp": datetime.now(timezone.utc).strftime("%Y%m%d-%H%M%S"), + }) + extra = [] # FN-017-1: ALWAYS pass the resolved task explicitly, so the gate chain # and the reporting chain are guaranteed to close the SAME task. @@ -973,7 +1025,24 @@ def cmd_done(args) -> int: print(" which PASSED.") print() - if rc in (0, 3): + # FN-027: decide by FACTS, not by the gate's return code alone. The field + # run proved rc can be 1 while the task WAS closed and committed - and the + # old rc-only branch then skipped the entire ledger and told the user to + # "run done again" (which can only yield "no active task"). + after_by_id_fact = {t["id"]: t for t in list_tasks(read_tasks(root))} + task_closed_fact = bool( + task_before + and after_by_id_fact.get(task_before, {}).get("status") == "[x]" + and {t["id"]: t for t in list_tasks(tasks_text_before)}.get( + task_before, {}).get("status") != "[x]" + ) + if rc in (0, 3) or task_closed_fact: + if rc not in (0, 3): + print(f"GATE INCONSISTENCY: the gate reported failure (rc={rc}) but") + print(f"{shown} WAS closed in docs/TASKS.md. Trusting the file: writing") + print("the ledger now. Do NOT rerun done. Gate output is in the report;") + print("investigate it, but the close itself stands.") + print() # FN-023: rc==3 (continuous mode) is ALSO a successful close - the # gate closed the task and committed. The ledger steps below must run # for every successful close, and each is individually guarded so a @@ -1077,6 +1146,15 @@ def cmd_done(args) -> int: except Exception as e: # noqa: BLE001 ledger_errors.append(f"deferred task queueing (docs/TASKS.md): {e}") + # FN-027 fuse: audit the ledger with the same logic as + # `coderail progress` before declaring victory. If the entry we just + # claimed to write is not actually on disk, that is a hard error. + if task_before and not ledger_errors: + gap_ids = {g[0] for g in ledger_gaps(root)} + if task_before in gap_ids or closed_id in gap_ids: + ledger_errors.append( + "post-close audit: the PROGRESS entry claimed above is NOT on disk") + if ledger_errors: print() print("LEDGER ERROR: the task WAS closed and committed, but these") @@ -1084,8 +1162,12 @@ def cmd_done(args) -> int: for e in ledger_errors: print(f" - {e}") print("Repair with: coderail progress --repair") + print("(the closeout snapshot is kept in .coderail/pending_close.json)") return 1 + # Ledger complete: the snapshot has served its purpose. + clear_pending_close(root) + print_blueprint_notice(root) print_next_recommendation(root) if task_before: @@ -1093,13 +1175,25 @@ def cmd_done(args) -> int: if rc == 3: print("This project runs in continuous mode: keep going with the next task.") return 3 + return 0 else: if not args.verbose: print(gate_output) # on failure, details ARE the point if task_before: bump_spin_state(root, task_before) + # FN-027: only tell the user to rerun done if the task is genuinely + # still open - "run done again" on a closed task can only produce + # "no active task" and is actively misleading. + still_open = bool( + task_before + and after_by_id_fact.get(task_before, {}).get("status") in ("[~]", "[ ]") + ) print("Not finished yet - one or more checks did not pass (details above).") - print("Fix what it points out, then run: coderail done again.") + if still_open or not task_before: + print("Fix what it points out, then run: coderail done again.") + else: + print(f"NOTE: {shown} is no longer open in docs/TASKS.md. Do NOT rerun") + print("done. Audit the ledger instead: coderail progress --repair") text = read_tasks(root) print_spin_report(root, active_task_id(text), list_tasks(text)) return rc @@ -1107,11 +1201,10 @@ def cmd_done(args) -> int: # ---------------------------------------------------------------- progress -def cmd_progress(args) -> int: - """FN-023: audit the ledger - every closed task must have a PROGRESS - entry. --repair writes honest retroactive entries for any that are missing - (e.g. closes that predate the transactional fix).""" - root = Path(args.target).resolve() +def ledger_gaps(root: Path) -> list[tuple[str, str, dict]]: + """Every closed task must have a PROGRESS.md entry. Returns the ones that + do not, as (task_id, title, meta). Shared by `coderail progress` and the + post-close audit fuse inside `done` (FN-027).""" progress_file = root / "docs" / "PROGRESS.md" progress_text = progress_file.read_text(encoding="utf-8") if progress_file.exists() else "" @@ -1126,6 +1219,15 @@ def cmd_progress(args) -> int: or (display and f"({display}" in progress_text): continue missing.append((tid, t["title"], m)) + return missing + + +def cmd_progress(args) -> int: + """FN-023: audit the ledger - every closed task must have a PROGRESS + entry. --repair writes honest retroactive entries for any that are missing + (e.g. closes that predate the transactional fix).""" + root = Path(args.target).resolve() + missing = ledger_gaps(root) if not missing: print("Ledger is complete: every closed task has a PROGRESS.md entry.") @@ -1138,14 +1240,28 @@ def cmd_progress(args) -> int: print("Write retroactive entries with: coderail progress --repair") return 1 + # FN-028: if the interrupted close left its snapshot behind, restore the + # REAL --next text, acceptance verdicts, and verify evidence from it + # instead of falling back to default copy. + pending = load_pending_close(root) + for tid, title, m in missing: shown = fmt_id(tid, m) + snap = pending if pending.get("task") == tid else {} # Point at the on-disk done report if one survived the original close. reports = sorted((root / ".coderail" / "reports").glob("done-*.md")) \ if (root / ".coderail" / "reports").is_dir() else [] safe = re.sub(r"[^A-Za-z0-9_-]", "_", shown) matching = [p for p in reports if safe in p.name or tid in p.name] - if m.get("verify"): + + if snap.get("verify_results"): + checked = ("retroactive entry - verify results recovered from the " + "closeout snapshot: " + + "; ".join(f"`{r['cmd']}` exit {r['exit']}" + for r in snap["verify_results"])) + elif snap.get("manual_acceptance"): + checked = f"retroactive entry - manual check: {snap['manual_acceptance']}" + elif m.get("verify"): checked = ("retroactive entry - verify commands were registered (" + "; ".join(f"`{c}`" for c in m["verify"]) + "); original console evidence was lost to a ledger bug") @@ -1153,10 +1269,18 @@ def cmd_progress(args) -> int: checked = "retroactive entry - no verify commands were registered" if matching: checked += f"; surviving report: {matching[-1].relative_to(root)}" - append_progress(root, shown, title, checked, - "decide with the user", + + next_hint = snap.get("next_hint") or "decide with the user" + accepted = list(zip(snap.get("accept_items", []), + snap.get("accept_statuses", []))) \ + if snap.get("accept_statuses") else [] + append_progress(root, shown, title, checked, next_hint, + accepted=accepted, warnings=["this entry was written by progress --repair, " "after the close itself skipped the journal"]) + if snap: + clear_pending_close(root) + pending = {} print(f" repaired: {shown}") print("Ledger repaired. Review docs/PROGRESS.md and commit it.") return 0 diff --git a/scripts/finish_task.py b/scripts/finish_task.py index 7ad1368..be3bbf2 100644 --- a/scripts/finish_task.py +++ b/scripts/finish_task.py @@ -168,6 +168,7 @@ def main(argv=None) -> int: failures += bool(run("Trace Index", "trace_index.py", root)) done_rc = 0 + task_marked_done = False # FN-027: track the actual close, for honest reporting if args.task_result == "done": done_evidence = list(evidence_args) if not args.harness_result and verification: @@ -183,6 +184,8 @@ def main(argv=None) -> int: if task_id and not set_task_status(root, task_id, "[x]"): print(f"Could not mark {task_id} done in docs/TASKS.md", file=sys.stderr) failures += 1 + else: + task_marked_done = bool(task_id) # Drive is evaluated from the task state. Closeout remains the authority for # changed-file scope and performs the final task-scoped commit below. @@ -225,7 +228,17 @@ def main(argv=None) -> int: print("\n" + drive_check.render_human(decision)) print("\n# Finish Task Report\n") - print(f"Closeout state: {'blocked' if failures else args.task_result}") + # FN-027: the verdict must match what actually happened. If the task WAS + # closed (and possibly committed), never report it as simply "blocked" - + # that leads callers to "run done again", which can only produce + # "no active task" once the task is [x]. + if failures and task_marked_done: + print(f"Closeout state: {args.task_result} (WITH ERRORS)") + print(f"NOTE: {task_id} WAS marked done in docs/TASKS.md and the commit") + print("step ran. Do NOT rerun done for this task. Investigate the failed") + print("check(s) above, then audit the ledger: coderail progress") + else: + print(f"Closeout state: {'blocked' if failures else args.task_result}") print(f"May stop: {'yes' if decision['may_stop'] and not failures else 'no'}") print(f"Next task mode: {decision['next_task_mode']}") print(f"Activated task: {activated_task or 'none'}") diff --git a/scripts/init_project.py b/scripts/init_project.py index 50bd5f4..d755611 100644 --- a/scripts/init_project.py +++ b/scripts/init_project.py @@ -97,7 +97,8 @@ def install_local_entry(target: Path, force: bool = False) -> None: # Local working state (spin counter, done reports, machine-local home # override) must stay out of git. gitignore = target / ".gitignore" - ignore_lines = [".coderail/spin.json", ".coderail/reports/", ".coderail/config.local.json"] + ignore_lines = [".coderail/spin.json", ".coderail/reports/", ".coderail/config.local.json", + ".coderail/pending_close.json"] existing = gitignore.read_text(encoding="utf-8", errors="ignore") if gitignore.exists() else "" missing = [l for l in ignore_lines if l not in existing] if missing: diff --git a/tests/test_structure.py b/tests/test_structure.py index c97f949..ee8c020 100644 --- a/tests/test_structure.py +++ b/tests/test_structure.py @@ -1268,6 +1268,83 @@ def test_done_next_flag_sets_journal_next(): f'--next not honoured: {progress}') +def test_done_produces_all_four_artifacts_end_to_end(): + # FN-027: the real done flow (no mocks) must leave all four artifacts at + # once - PROGRESS entry, on-disk report, TASKS closed, commit made - and + # the captured Done Gate Report must not be blocked. Two consecutive + # tasks, per the field acceptance criteria. + with tempfile.TemporaryDirectory() as td: + root, cr = _lifecycle_env(td) + for n, (biz, title) in enumerate([('T-201', 'First artifact task'), + ('T-202', 'Second artifact task')], 1): + r = cr('start', f'{biz} {title}', '--verify', 'true') + check(r.returncode == 0, r.stdout) + r = cr('done', '--next', f'next step after {biz}') + check(r.returncode == 0, r.stdout) + progress = (root/'docs/PROGRESS.md').read_text(encoding='utf-8') + check(title in progress, f'(1/4) PROGRESS missing {biz}: {progress}') + check(f'next step after {biz}' in progress, f'--next lost for {biz}: {progress}') + reports = list((root/'.coderail/reports').glob('done-*.md')) + check(len(reports) == n, f'(2/4) report count {len(reports)} != {n}') + body = sorted(reports)[-1].read_text(encoding='utf-8') + check('Status: blocked' not in body, + f'(FN-027b) Done Gate blocked inside a passing close: {body}') + tasks = (root/'docs/TASKS.md').read_text(encoding='utf-8') + check(tasks.count('Status: [x]') == n, f'(3/4) TASKS close count != {n}: {tasks}') + log = subprocess.run(['git', 'log', '--oneline'], cwd=td, + capture_output=True, text=True).stdout + check(f'chore({biz}/' in log, f'(4/4) commit missing for {biz}: {log}') + check(not (root/'.coderail/pending_close.json').exists(), + 'snapshot must be cleared after a fully-ledgered close') + r = cr('progress') + check(r.returncode == 0 and 'complete' in r.stdout, + f'built-in audit disagrees with artifacts: {r.stdout}') + + +def test_snapshot_survives_ledger_failure_and_repair_restores_params(): + # FN-028: a close whose ledger step fails must keep the snapshot on disk, + # and progress --repair must restore the REAL --next text and per-item + # acceptance verdicts from it - not default copy. + with tempfile.TemporaryDirectory() as td: + root, cr = _lifecycle_env(td) + r = cr('start', 'Snapshot recovery task', '--verify', 'true', + '--accept', 'first criterion', '--accept', 'second criterion') + check(r.returncode == 0, r.stdout) + (root/'docs/PROGRESS.md').unlink(missing_ok=True) + (root/'docs/PROGRESS.md').mkdir() # sabotage: journal unwritable + r = cr('done', '--next', 'switch harness to real compiler output', + '--accept-status', '1=done', '--accept-status', '2=deferred') + check(r.returncode == 1 and 'LEDGER ERROR' in r.stdout, r.stdout) + check((root/'.coderail/pending_close.json').exists(), + 'snapshot must survive a failed ledger (FN-028)') + (root/'docs/PROGRESS.md').rmdir() + r = cr('progress', '--repair') + check(r.returncode == 0, r.stdout) + progress = (root/'docs/PROGRESS.md').read_text(encoding='utf-8') + check('- Next: switch harness to real compiler output' in progress, + f'real --next text not restored (FN-028): {progress}') + check('[done]' in progress and 'first criterion' in progress, + f'acceptance verdicts not restored: {progress}') + check('[deferred]' in progress and 'second criterion' in progress, + f'deferred verdict not restored: {progress}') + check(not (root/'.coderail/pending_close.json').exists(), + 'snapshot must be consumed by repair') + + +def test_failed_done_on_open_task_still_says_rerun(): + # FN-027: "run done again" is only printed when the task is genuinely + # still open (here: verify fails, task stays [~]). + with tempfile.TemporaryDirectory() as td: + root, cr = _lifecycle_env(td) + cr('start', 'Failing verify task', '--verify', 'false') + r = cr('done') + check(r.returncode == 1, r.stdout) + check('coderail done' in r.stdout and 'again' in r.stdout, + f'open-task failure should still suggest rerun: {r.stdout}') + tasks = (root/'docs/TASKS.md').read_text(encoding='utf-8') + check('Status: [~]' in tasks, 'task must remain open after failed verify') + + def run_all(): tests = [v for k, v in globals().items() if k.startswith('test_')] for t in tests: From 68f826f0f626c52280b05f2d1350abde716685e4 Mon Sep 17 00:00:00 2001 From: v0 Date: Mon, 13 Jul 2026 15:43:04 +0000 Subject: [PATCH 2/2] chore: bump to v0.8.4 Co-authored-by: v0 --- .claude-plugin/plugin.json | 2 +- .codex-plugin/plugin.json | 2 +- CHANGELOG.md | 2 +- README.md | 2 +- VERSION | 2 +- package.json | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 09a04b2..9be8061 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "coderail", "description": "Blueprint Gate, North-Star, Coordinate Contract Draft, TDD Gate, Done Gate, Auto Commit Gate, CI Gate, Inspect, and traceable governance for AI coding agents.", - "version": "0.8.3", + "version": "0.8.4", "author": { "name": "CodeRail" }, diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index b62a9f3..41f36d3 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "coderail", - "version": "0.8.3", + "version": "0.8.4", "description": "Blueprint Gate, North-Star, Coordinate Contract Draft, TDD Gate, Done Gate, Auto Commit Gate, CI Gate, Inspect, and traceable governance for AI coding agents.", "author": { "name": "CodeRail" diff --git a/CHANGELOG.md b/CHANGELOG.md index a881b47..4e89a19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## Unreleased +## v0.8.4 Close-before-report ordering fix: closes the fourth round of field findings (FN-027/FN-028) and corrects the root-cause analysis of FN-023. The theme: the closeout ledger runs from a snapshot taken before anything mutates, and every verdict printed matches what actually happened on disk. diff --git a/README.md b/README.md index fcfb6e0..fe1890a 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # CodeRail — Convergent Coding -![version](https://img.shields.io/badge/version-v0.8.3-2f80ed) +![version](https://img.shields.io/badge/version-v0.8.4-2f80ed) ![license](https://img.shields.io/badge/license-MIT-27ae60) ![python](https://img.shields.io/badge/python-3.x-ffd43b) ![agent](https://img.shields.io/badge/agent--ready-Codex%20%7C%20Claude-8e44ad) diff --git a/VERSION b/VERSION index ee94dd8..b60d719 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.8.3 +0.8.4 diff --git a/package.json b/package.json index e58d83f..3b225d7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "coderail", - "version": "0.8.3", + "version": "0.8.4", "description": "Blueprint Gate, North-Star, Coordinate Contract Draft, TDD Gate, Done Gate, Auto Commit Gate, CI Gate, Inspect, and traceable governance for AI coding agents", "license": "MIT", "private": false,