deft-directive-pre-pr

name	deft-directive-pre-pr
description	Iterative pre-PR quality improvement loop. Use before pushing a branch for PR creation -- after completing implementation but before task check. Cycles through Read-Write-Lint-Diff until a full pass produces zero changes.

Deft Directive Pre-PR -- Read, Write, Lint, Diff, Loop

Structured self-review loop agents run before submitting a PR. Catches inconsistencies, missing enforcement markers, incomplete acceptance criteria, scope creep, and unintended changes before they reach the reviewer.

Legend (from RFC2119): !=MUST, ~=SHOULD, ≉=SHOULD NOT, ⊗=MUST NOT, ?=MAY.

See also: deft-directive-review-cycle | deft-directive-build | RWLDL tool

Formerly deft-rwldl -- renamed to clearly communicate the skill's purpose (iterative pre-PR quality loop).

Branch-Protection Policy Guard

! Before entering Phase 1 (Read), run the skill-level branch-policy guard documented in scripts/policy.py / scripts/preflight_branch.py (#746 / #747). Halt before any state mutation (the Phase 2 Write phase, the Phase 3 Lint phase that may touch files) when the policy is unresolvable AND no env-var bypass is active:

uv run python scripts/preflight_branch.py --project-root . --quiet || exit 1

or invoke task verify:branch. Pre-PR is the last gate before push, so a stale / unresolvable policy here is the highest-leverage place to catch the bug before it reaches the bot reviewer.

Deterministic Questions Contract

! Every numbered-menu prompt rendered in this skill (per-finding decision menus in Phase 4 Diff, the Phase 5 Loop restart-vs-exit gate) MUST follow ../../contracts/deterministic-questions.md: the final two numbered options MUST be Discuss and Back, in that order. The Discuss-pause semantic is documented verbatim there -- implicit resumption is forbidden.

Ordered-plan target gate (#2402)

! Before opening or pushing a PR, when an ordered-plan sequence may be active, verify the work unit matches the current authorized entry:

1. Resolve the current entry kind and id:

task plan-sequence:current

Use task plan-sequence:current -- --json when you need machine-readable kind / id fields from the current entry.

2. Verify with the entry kind (not hardcoded pr — story / issue entries are common):

task verify:plan-sequence -- --target-kind <entry-kind> --target <entry-id>

• ! Exit non-zero → fail closed; do not open an unauthorized PR.
• ! Exit 0 with "skipped (no active ordered-plan sequence)" → proceed under normal pre-PR rules.
• ⊗ Hardcode --target-kind pr when the current entry is kind=story or kind=issue — the verifier matches entry kind exactly (#2662).
• ! After the PR's review cycle completes successfully, run task plan-sequence:advance so "next" resolves to the following entry (or exhausted).
• ⊗ Treat skill-chaining or "what's next?" as permission to open a PR outside the current sequence entry.

Gate throughput — iteration fast lane vs merge chokepoint (#1704)

Invariant: every change MUST pass the full gate at least once before merge. Pre-PR is the merge chokepoint — NOT every iteration commit.

• ! Iteration lane (Phases 2–3 loop): use affected/static gates on changed paths — vitest run --coverage <paths>, relevant verify:* on touched files, task coverage:hotspots — instead of full task check on every RWLDL iteration.
• ! Merge chokepoint (Phase 3 Lint exit + final confirm): run full task check once before push/PR; Phase 3c targeted coverage precedes but does not replace the full gate.
• ! Escape-rate safety (#1703 Tier-1): before recommending fleet-wide fast-lane tightening, cite #1703 Tier-1 telemetry (helped/crud-metrics.jsonl) and task eval:health — do NOT invent a separate escape-rate surface.
• ~ In-engine incrementality (#1713): content-hash cache + runner-delegated affected selection are sibling work (#1713).
• ⊗ Treat Phase 3c targeted coverage alone as PR-ready without full task check at the merge chokepoint.

When to Use

• ! Before pushing a branch for PR creation
• ! After completing implementation but before the final merge-chokepoint task check (#1704)
• ~ After addressing bot reviewer findings (run one RWLDL pass before pushing the fix batch)
• ? During mid-implementation checkpoints on large changes

Loop Phases

Each iteration proceeds through all phases in order. Do NOT skip phases or reorder them.

Phase 1 -- Read

! Re-read each changed file end-to-end (git diff master --name-only to get the list).

• ! Read every changed file in full -- do not skim or skip sections
• ! Compare each file against its scope xBRIEF acceptance criteria in xbrief/active/
• ~ If changed files include xbrief/PROJECT-DEFINITION.xbrief.json, a configured codebase-map provider artifact, or .planning/codebase/MAP.md, read the MAP and canonical metadata together. The MAP is orientation; plan.architecture.codeStructure and provider artifacts remain authoritative.
• ! When adding a ! or ⊗ rule that prohibits a specific command, pattern, or behavior, search the same file for any ~, ≉, or prose that recommends or permits the same command/pattern -- resolve all contradictions in the same commit before pushing
• ! When strengthening a rule (e.g. upgrading ~ to !), grep for the term in the full file and verify no weaker-strength duplicate remains
• ~ Note any inconsistencies, missing RFC2119 markers, stale cross-references, or incomplete sections
• ~ Check that CHANGELOG.md entries match the actual changes made

Phase 2 -- Write

! Fix any issues found in the Read phase.

• ! Fix inconsistencies, add missing RFC2119 enforcement markers (!, ~, ⊗)
• ! Complete any incomplete acceptance criteria or missing content
• ! Update stale cross-references
• ~ Improve clarity where intent is ambiguous
• ⊗ Make changes beyond the scope of the current task -- if you notice unrelated issues, file them as ideas or future work, do not fix them now

Phase 3 -- Lint

! Run the merge-chokepoint gate and fix any failures (#1704).

• ! Run full task check (fmt + lint + typecheck + tests + coverage + verify:*) — the merge chokepoint, not every RWLDL iteration
• ! Fix all failures before proceeding to Phase 3b
• ~ If a lint fix requires changing a file, that counts as a change for the Loop phase

~ Windows + Grok Build (#1353): Avoid |, >, or 2>&1 in run_terminal_command strings -- use Python pathlib/subprocess or plain task commands instead.

Phase 3c -- Coverage headroom (#2683)

! Before the full task check coverage gate, run targeted coverage on changed modules and verify headroom above the project floor.

• ! Run targeted coverage first: vitest run --coverage <changed-paths> (or language equivalent) — not only full task check.
• ! Exercise both sides of new branches (ternary / early-return / catch / default switch / || / ??).
• ! Treat barely ≥ floor as insufficient — aim for ≥ floor + 0.3–0.5pp headroom on the branch metric relative to your project's vitest/coverage floor (may differ from 85%).
• ! Run task coverage:hotspots / deft coverage:hotspots to locate uncovered branches before opening a PR; complements deft verify:forward-coverage (#1310) and --allow-coverage-debt=#N (#2573).

Phase 3b -- Auto-Render Exports

! If xbrief/specification.xbrief.json exists, refresh rendered exports before the diff check:

• ! Run task prd:render if PRD.md already exists in the project root
• ! Run task spec:render if SPECIFICATION.md already exists and does not contain <!-- deft:deprecated-redirect -->
• ! If plan.architecture.codeStructure, a configured codebase-map provider artifact, or .planning/codebase/MAP.md changed, run task codebase:map and task verify:codebase-map-fresh so the generated orientation projection matches its canonical inputs.
• ⊗ Create export files that don't already exist -- only refresh existing ones
• ⊗ Hand-edit .planning/codebase/MAP.md to satisfy review; update canonical metadata or provider artifacts, then regenerate it

Phase 4 -- Diff

! Review the full diff against the base branch for unintended changes.

git --no-pager diff master

• ! Verify no files outside the task scope were modified
• ! Check for scope creep -- changes that go beyond the spec task acceptance criteria
• ! Verify no debug code, TODO comments, or temporary scaffolding remains
• ! Confirm no unintended whitespace-only changes or formatting drift
• ! Run task pr:check-closing-keywords -- --pr <N> (or pass --body-file / --commits-file for offline checking) before opening the PR; refuse to push if findings (#737). The lint scans both the PR body AND every commit message for closing-keyword tokens (close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved) followed by #\d+ in negation / quotation / example / code-block contexts. The recurrence record is the Layer 1 / Layer 2 / Layer 3 stack: #167 (post-merge close-verify), #697 / #698 (negation-context substring match), #401 / #700 (persistent closingIssuesReferences link), #735 (squash body containing DOES NOT CLOSE #734 auto-closed #734). When the lint surfaces a known-safe occurrence (e.g. test fixtures that legitimately exercise the trigger token), pass --allow-known-false-positives <issue-numbers> to suppress -- DO NOT silently delete the lint invocation
• ~ Verify the diff tells a coherent story -- a reviewer reading it top-to-bottom should understand the change
• ~ If the PR adds or moves any documentation, verify each new doc is reachable from the AGENTS.md reference chain -- an orphan doc is discovered <10% of the time yet still costs context when found (the reference-chain contract, #644 / #647). Add a pointer or fold it in rather than leaving it stranded.

Phase 5 -- Loop

! Decide whether to restart or exit.

• ! If ANY fixes were made in Phase 2 (Write) or Phase 3 (Lint): restart from Phase 1 (Read)
• ~ Phase 3b auto-renders are intentional output refreshes; they do NOT trigger a loop restart
• ! If a full Read-Write-Lint-Diff cycle produced zero changes: exit the loop
• ~ Track iteration count -- if you exceed 3 iterations, pause and assess whether you are oscillating between competing fixes

Exit Condition

! Exit when a complete Read-Write-Lint-Diff cycle produces zero changes -- no file edits in Write, no lint fixes in Lint, and no scope issues in Diff.

After exiting:

• ! Run task check one final time to confirm clean state
• ~ The branch is now ready for push and PR creation

Anti-Patterns

• ⊗ Submit a PR without running the RWLDL loop -- every PR branch should pass at least one full cycle
• ⊗ Exit the loop after the Lint phase without completing the Diff phase -- Diff catches scope creep and unintended changes that Lint cannot detect
• ⊗ Skip the Read phase and jump directly to Lint -- Read catches semantic issues (missing content, wrong RFC2119 markers, incomplete acceptance criteria) that linters do not check
• ⊗ Make out-of-scope fixes during Write -- this introduces scope creep that Diff will flag, forcing another iteration
• ⊗ Ignore the iteration count -- more than 3 iterations usually indicates oscillating fixes or an unclear spec task
• ⊗ Add a prohibition (! or ⊗) without scanning the same file for conflicting softer-strength rules (~, ≉) that reference the same term
• ⊗ Skip task pr:check-closing-keywords (#737) before pushing a PR. The negation-context substring match is the Layer 0 (prevention) gate that prevents the recurring auto-close of umbrella / staying-OPEN issues observed in #697 (closed #642), #401 (closed #642), #700 (closed #233), and #735 (closed #734) -- each incident required manual reopen and downstream cleanup. The lint's three-state exit (0 clean / 1 hits found / 2 config error) MUST be treated as a hard refusal: rewrite the PR body / commit messages until clean, OR pass --allow-known-false-positives ONLY for legitimately-quoted occurrences (test fixtures, documentation that discusses the trigger token literally). See skills/deft-directive-swarm/SKILL.md Phase 6 Step 1 for the corresponding Layer 3 (recovery) pr:check-protected-issues rule (#701)