100%

review-section-skill

Goal

Author the /review-section tool skill + scripts/review_section_runtime/ that drives the single-section intel-grounded review, reusing the shared review_plan_runtime backend, status_flip, editor hook, and S1 primitives.

Dependency

Needs S1 (restructuring primitives) + S2 (intel_reviewed_hash field + compute_intel_package_hash helper) — state.py records intel_reviewed_hash on close, which the S2 field + hash helper must already provide.

S1 + S2 ordering is enforced by linear-walk key order (v7 plans gate serve on advisory hint_needs, not a depends_on field); S1 (s-004ec797) and S2 (s-feba50ad) are already complete, so the order is satisfied.

Implementation Sketch

  • Scaffold scripts/review_section_runtime/ on the verify_tpr_runtime template (api.py / gates.py / dispatch.py / state.py / __main__.py / next_action.schema.json / tests). role: tool, NO AskUserQuestion.
  • Entry gates: section readable + no parallel review_in_flight + structural-audit-passed.
  • dispatch.py drives the single-section review (the steps-2-9 content scope) through review_plan_runtime.backend.resolve_backend: backend.enter_review() (entry) / backend.mark_reviewed_clean(...) (close). The backend dispatches the close to V7Backend.mark_reviewed_clean -> mark_section_reviewed_v7 (writes plan.json) on the v7 path and to MarkdownBackend.mark_reviewed_clean -> the status_flip CAS flip on the markdown path. NEVER call the raw status_flip.flip_from_in_review_clean_with_marker directly — it early-returns _v7_route_noop on a v7 plan (status_flip.py:1128), so a direct call would silently skip the close + hash write.
  • state.py delegates restructuring to S1 + existing move_work_item / abandon_work_item / append_work_item / section_create; the intel_reviewed_hash is recorded on close through backend.mark_reviewed_clean (v7 -> mark_section_reviewed_v7 records it in plan.json).

intel_reviewed_hash write is clean-close-only (soundness pin)

state.py records intel_reviewed_hash ONLY on a clean close, NEVER on abort or reversal — mirroring the S2 writer-contract (intel-reviewed-hash-field--s-feba50ad.md): S3’s review_section_runtime state.py is the WRITER, recording the sha256 envelope hash on the null -> hash clean-close transition through resolve_backend().mark_reviewed_clean(...) only. On the v7 path that lands in plan.json via mark_section_reviewed_v7; the raw markdown flip_from_in_review_clean_with_marker no-ops on v7 (status_flip.py:1128) and is the markdown-backend path only. S3 MUST EXTEND the writer thread — mark_section_reviewed_v7 (review_phase.py:815, no intel_reviewed_hash param today) AND V7Backend.mark_reviewed_clean (backend.py:275, threads only reviewed_evidence + reviewer, silently dropping other kwargs) — to accept + persist intel_reviewed_hash into plan.json; the hash write is a real deliverable, not a presumed-existing capability. record_review_abort / record_review_reversal MUST leave the field untouched (the null never-reviewed value survives an abort/reversal). The reopen/reset path — reset_section_to_not_started (route_v7.py:654-667), which pops completion/tpr/close_out/superseded_by + sets reviewed:false but does NOT pop intel_reviewed_hash today — MUST be extended to clear intel_reviewed_hash to null on reopen, else a stale hash falsely suppresses the next review of the reopened section. A hash recorded on a non-clean close, or surviving a reopen, would falsely suppress the next review against a dossier the section was never validated against.

Declared exit_reason enum (S5 consumption surface)

review_section_runtime emits a CLOSED exit_reason set, mirroring the verify_tpr_runtime / next_action.schema.json shape, so S5 (orchestrator-wiring--s-d1501645.md) knows the routing contract it consumes:

  • review_clean — section reviewed against current intel, no restructuring needed; intel_reviewed_hash recorded.
  • structural_changes_applied — restructuring enacted through the sanctioned primitives; intel_reviewed_hash recorded on clean close.
  • file_not_found — target section path unreadable (entry gate failure).
  • structural_audit_failed — entry structural-audit gate rejected the section.
  • review_in_flight_blocked — a parallel review holds the in-flight lock; no flip performed.

Backend-aware routing note

review_section_runtime routes EVERY review state transition through review_plan_runtime.backend.resolve_backend (enter_review / mark_reviewed_clean / abort / reversal) so the v7-plan vs markdown split is honored by the backend, never by a hand-picked status_flip function: in the v7 environment reviewed: true + intel_reviewed_hash land in plan.json via V7Backend.mark_reviewed_clean -> mark_section_reviewed_v7 (the raw markdown status_flip flips are _v7_route_noop no-ops there), whereas MarkdownBackend.mark_reviewed_clean performs the CAS flip on the markdown backend. Calling a raw status_flip.flip_* function directly from dispatch.py / state.py is BANNED — it bypasses the backend dispatch and silently no-ops on v7.

Spec References

scripts/verify_tpr_runtime/ (template); scripts/review_plan_runtime/backend.py + status_flip.py; .claude/rules/skill-control-contract.md (tool role); .claude/rules/script-first.md §4.

Work Items

  • Scaffold scripts/review_section_runtime/ (api/gates/dispatch/state/main/next_action.schema.json/tests) on the verify_tpr_runtime template.
  • dispatch.py drives the single-section review (steps 2-9 content scope) through review_plan_runtime.backend.resolve_backend (enter_review / mark_reviewed_clean), never a raw status_flip function directly.
  • state.py delegates restructuring to S1 primitives + existing move/abandon/append/section_create; records intel_reviewed_hash on close via resolve_backend().mark_reviewed_clean only (v7 -> mark_section_reviewed_v7 writes plan.json). EXTEND the writer thread as part of this deliverable: add an intel_reviewed_hash parameter to mark_section_reviewed_v7 (review_phase.py) + thread it through V7Backend.mark_reviewed_clean (backend.py, which today drops non-reviewed_evidence/reviewer kwargs), and extend reset_section_to_not_started (route_v7.py) to pop intel_reviewed_hash to null on reopen. Test directive: pin v7 + markdown parity — on a v7 plan mark_reviewed_clean records the sha256 hash in plan.json (a direct flip_from_in_review_clean_with_marker call no-ops, proving the bypass would silently skip it); record_review_abort / record_review_reversal leave intel_reviewed_hash untouched on a never-clean close; AND a reopen via reset_section_to_not_started clears intel_reviewed_hash to null so the reopened section re-reviews against the current dossier.
  • Author .claude/skills/review-section/SKILL.md (role: tool) with @-includes + structured exit state; structured exit emits the closed exit_reason enum (review_clean / structural_changes_applied / file_not_found / structural_audit_failed / review_in_flight_blocked) per the Implementation Sketch routing contract.
  • Author tests + —self-test; import-boundary audit clean. The import-boundary audit (scripts/skill_audit/import_boundary_audit.py per .claude/rules/script-first.md §4 api.py surface) is part of the deliverable gate; the corresponding success criterion is the python -m scripts.review_section_runtime --self-test pin, which lives in plan.json (engine-written, owned by s-94d8f5fa), not editor-authored in this body.
  • Document move_work_item + abandon_work_item reuse in the review_section_runtime api surface (no new code).