RLCR methodology feedback: defect-class audits, review-phase format, reviewer capabilities, and hook hardening (macOS sed stall)

[xuw]

Context

Sanitized methodology review of one completed RLCR session:

• 5 rounds: 3 implementation, 2 code-review.
• Round 0 delivered the full plan; later rounds only closed gaps.
• Findings decreased every round, with no stagnation, false positives or scope drift.
• Every fix shipped with a regression test that was shown to fail on the old code.

Overall the loop worked well. The suggestions below address inefficiencies and tooling friction.

Observations

• One defect class was found piece by piece. 4 of 10 findings were the same class: stale asynchronous replies or state crossing a context change. They surfaced one at a time in rounds 0, 2 and 4, and the class as a whole was never audited.
• Code-review-phase output was terse. Each finding was printed twice, with no AC or task mapping and no required regression. By contrast, the implementation-phase reviews (AC, reproducing probe, required fix, required regression) were very actionable.
• The reviewer could not run the heavier tests. Its sandbox could not bind sockets or launch a browser, so browser and container suites were trusted from the implementer's evidence. One unit test failed every round for environmental reasons, which added noise.
• "Complete" came before the code review finished. The implementation-phase review declared completion, and the code-review phase then found two more real cross-context races.
• Setup order was not enforced. The goal tracker and round-0 contract were created after implementation had begun.

Tooling friction

• The stop hook stalls silently on macOS.
  • A GNU-only sed expression (nested braces without a trailing ;) runs under set -euo pipefail.
  • It only runs when a background-pending marker exists.
  • BSD sed rejects it, and the hook exits 1 with no stderr.
  • The marker's cleanup code sits after the failing line, so the loop stays stuck until the marker is deleted by hand.
• Session binding misses chained setup commands. It recognizes the setup command only when it is the first command in the shell invocation, so chaining it after other commands silently leaves the session unbound.
• A missing review result was easy to overlook. One review run exited 0 without writing its result file. Its findings survived only in the log.

Suggestions

1. Defect-class audits. When a finding is one instance of a broader class, the review should name the class and require a sweep of every similar site in the same round. The round summary should list the sites that were audited.
2. Enforce initialization order. Setup should create the goal tracker and round-0 contract, or the loop should warn loudly until they exist.
3. Make code-review-phase output match the implementation phase. Use finding ID, related AC/task, required fix and required regression, with no duplicated text.
4. Declare reviewer capabilities up front. List which verification layers the reviewer can execute (unit, DOM, browser, container). Exclude known environmental failures from the gate comparison, and label checks it could not run as "trusted from implementer evidence".
5. Treat a missing review result as a failure. Retry, or extract the findings from the log automatically, instead of relying on the implementer to notice.
6. Harden the hooks.
   • Use portable text-processing syntax, and test on both macOS and Linux.
   • Surface hook failures visibly instead of exiting silently.
   • Expire or clean up state markers once their condition is gone.
   • Detect the setup command anywhere in a chained invocation, or warn when a session is left unbound.
7. Add an optional pre-completion stress pass. Before declaring "complete", run a brief targeted check for the risk classes the plan names, such as concurrency or navigation races.
8. Prompt for a lesson on repeated findings. When the same finding class appears in two or more rounds, the lesson step should ask for a written lesson, even if each instance was fixed within one round.