[RMANOV]

Problem

An accepted reflection candidate could be applied after another session had changed the target task. reflect_decide stored only the decision, author and time; reflect_apply then called apply_task_mutation without the reviewed values or versions. Before/after snapshots recorded the mutation after the fact but did not stop it.

Reproduced on main (b758842) with a temporary SQLite database and separate connections:

1. create an overdue task and a stale_overdue_tasks candidate;
2. reflect_decide(..., "accept");
3. another connection sets status=in_progress and new notes;
4. reflect_apply with the old approval.

Result on main: applied=1, no refusal, the resumed task becomes archived, and an apply snapshot is written.

Fix

• When a candidate is created, the full tasks row and its ordered task_field_versions are frozen in reviewed_state_json. reflect_review shows that snapshot; accept never refreshes it.
• reflect_apply takes SQLite's writer lock (BEGIN IMMEDIATE for the MCP tool; savepoint plus a no-op UPDATE for direct DAO/autocommit callers). Under the lock it re-reads the decision, idempotency marker, run state, refusal marker and the current task state.
• Any mismatch leaves the task untouched: applied=0, skipped.reason=stale_review, and a durable apply_error_json (reason, actor, time, requires_new_review=true). The human decision is kept, the candidate cannot be re-accepted, and a new run plus a new explicit approval are required.
• The mutation, its ledger/field versions and the apply snapshot share one savepoint; a failure rolls them back together and leaves the caller's outer transaction intact.
• Idempotent migration adds nullable reviewed_state_json and apply_error_json. Legacy candidates without a snapshot fail closed with missing_review_state; nothing is re-approved automatically.

Changed: reflection_dao.py, reflection_apply.py, intel_server.py, schema.py. Added: tests/test_reflection_freshness.py. No existing test is modified.

Verification

Run on this PR's commits: 76787c7 (fix) and 4c9800b (compact review listings).

• GitHub CI (Linux, Python 3.11):
  • 4c9800b: full suite, ruff, manifest gate and registry smoke pass.
  • 76787c7: full suite 2093 passed, 30 skipped.
• Baseline proof on main with only the new test file (Windows, Python 3.12.13): test_reopened_task_survives_old_approval fails (in_progress → archived) and test_unchanged_approval_applies_once passes.
• Windows, Python 3.12.13 / SQLite 3.53.1:
  • tests/test_reflection_freshness.py: 24 passed.
  • Targeted set from the package README: 324 passed, 2 skipped.
  • Full suite on 76787c7: 2095 passed, 9 failed. Each failure also fails on main, is flaky, or is Windows-specific.
• Windows, Python 3.14.6 / SQLite 3.50.4: tests/test_reflection*.py 168 passed.
• Copy of a real 2,834-task database:
  • The frozen state round-trips for every task, so no task is permanently refused with a false stale_review.
  • Unchanged approvals apply, and local housekeeping writers did not invalidate fresh approvals.
  • The 4c9800b fix came from this check. With 76787c7, the default reflect_review grew from 15.5k to 499k characters, over MCP client output caps. With 4c9800b it is 17.8k characters, and the largest row is 944 characters.

The commits went through the owner's ruff format pre-commit hook. 76787c7 is AST- and comment-identical to the reviewed patch (sha256 987ff013…b3e1): reflection_apply.py and reflection_dao.py differ only in line wrapping, and the other three files are byte-identical.

The original patch package, run on 13 Oct 2026 on Linux with Python 3.12.14, reported the following. These are the package's numbers, not re-run here:

• 23 new cases passed;
• 325 targeted tests passed;
• an independent review ran 119 tests plus probes and found no blocking issue in scope.

Not run yet, deferred: adversarial locking probes on WAL, mutation testing of the guards, and a consumer audit.

Limits

• Full-row comparison is conservative: a housekeeping-only change to the task also requires a new review.
• The guarantee covers the target task; other rows used by duplicate/orphan predicates are not re-validated.
• Canonical ABA is detected through field versions; raw SQL that restores every value and version without history is indistinguishable from no change.
• A caller's already-stale deferred read transaction can get database is locked on upgrade; it makes no mutation and must retry. The MCP path starts with BEGIN IMMEDIATE.
• Not deployed to any running installation by this PR.
• reflect_review lists reviewed_state_sha256 instead of the full frozen state; get_candidate (used by apply) still returns the full state.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Hdie6u9nXibq7PSu6xv9RT

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-14T11:15:39.678666Z	4c9800b	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[Copilot]

🔵 Needs a closer look

It changes transaction/locking and compare-and-mutate semantics on a critical data-mutation path, which warrants human verification despite the thorough test suite.

1 open finding

• Annotate conn parameter in record_apply_refusal · New

What changed in this PR

This PR fixes a correctness bug in the Phase‑1 reflection pipeline where an accepted reflection candidate could be applied even after another SQLite session had changed the target task, silently clobbering concurrent work. The fix freezes the full target‑task state at candidate creation, then re‑validates it under SQLite's writer lock at apply time and fails closed (with a durable, human‑visible refusal) on any mismatch, legacy, or missing snapshot — requiring a fresh run and explicit re‑approval.

Changes:

• Freeze the tasks row + ordered task_field_versions into reviewed_state_json on candidate creation; decide never refreshes it, and listings carry only a sha256 digest to stay within MCP output caps.
• Guard apply behind a savepoint + no-op UPDATE writer lock (and BEGIN IMMEDIATE for the MCP tool), re-reading decision/idempotency/freshness atomically; mismatches record apply_error_json and skip with stale_review/missing_review_state.
• Idempotent migration adds nullable reviewed_state_json and apply_error_json; comprehensive new freshness/concurrency tests added.

File	Description
reflection_dao.py	Adds candidate_write_transaction, task_review_state, record_apply_refusal, snapshot freezing in add_candidate, and apply-error gating in decide_candidate.
reflection_apply.py	Replaces after-the-fact snapshotting with _apply_candidate that re-validates frozen state under the writer lock and applies atomically.
intel_server.py	Marks reflect_apply as write=True (BEGIN IMMEDIATE) and documents the freshness/fail-closed contract.
schema.py	Adds reviewed_state_json / apply_error_json columns plus idempotent guarded migrations.
tests/​test_reflection_freshness.py	New tests covering staleness, ABA, legacy/malformed state, atomic rollback, concurrency, and the MCP-tool path.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.