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 PR target matches the current authorized entry:

task verify:plan-sequence -- --target-kind pr --target <issue-or-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.
• ! 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.

When to Use

• ! Before pushing a branch for PR creation
• ! After completing implementation but before the final task check gate
• ~ 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 task check and fix any failures.

• ! Run task check (fmt + lint + typecheck + tests + coverage)
• ! 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 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)