deftai

deft-directive-pre-pr

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.

deftai 31 14 Updated 1w ago
GitHub

Install

npx skillscat add deftai/directive/deft-directive-pre-pr

Install via the SkillsCat registry.

SKILL.md

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 and scoped to PR readiness. For the broader iterative quality improvement loop (micro/macro review), see tools/RWLDL.md.

Branch-Protection Policy Guard

! Before entering Phase 1 (Read), run the skill-level branch-policy guard documented in task policy:show / task verify:branch (#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:

task verify:branch

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.

  1. Verify with the entry kind (not hardcoded prstory / 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 pre-PR 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 pre-PR 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
  • ! If the change alters user-visible behavior, apply coding/docs.md (#447): update the matching user-facing surface (CHANGELOG when user/operator-visible, CLI help / commands.md for new or changed verbs/flags, README/getting-started for install/first-run, skill/strategy triggers when entry points change) in this PR
  • ⊗ Claim documentation was updated without those files in the diff
  • ~ Pure internal refactors with no user-visible behavior change do not require user-facing doc updates

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 pre-PR 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
  • ! Closing-keyword policy (#3015 class D / #737 Layer 0): Default PR body uses Tracking: #N / Related: #N / Refs #N only. Use Closes / Fixes / Resolves #N only when the full issue DoD is met (not Phase A partial / multi-phase mid-stream).
  • ⊗ Write Closes #N Phase A, only if, partial, or other conditional English around a real closing keyword — GitHub ignores the condition and auto-closes (#3015 enterprize#29).
  • ! Run task pr:check-closing-keywords before opening the PR (default mode both = FP #737 + intent #3015). Offline: --body-file / --commits-file. Scans body and commits for closing-keyword tokens in negation / quotation / example / code-block contexts (FP) and for any unallowlisted real close (intent). When full DoD close is intentional, pass --allow-close <N,M> (CLI allowlist only — body trailers are not an authorization path). FP-only: --mode fp. Known-safe quoted tokens: --allow-known-false-positives. Recurrence stack: #167, #697 / #698, #401 / #700, #735 (class A), #3015 (class D enterprize#29).
  • ~ 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.
  • ! Gate integrity (#3156): If the branch cleared a red gate, confirm the fix changed the product/process/test under test — not the gate definition, verifier, reward, required check, coverage floor, policy flag, or eval fixture that was failing. Deliberate gate changes need issue/PR rationale (constitution-tier disposal under #3164). Doctrine: docs/gate-integrity.md. Refine-internal SkillOpt region stays on #2436 — do not re-implement it here.

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

Budget-aware pre-PR depth (#3266)

When session effort-budget is hard-capped (DEFT_MAX_TURNS / DEFT_MAX_BUDGET / session:start effort_budget):

  • ! Complete the merge-chokepoint path that proves stated AC (full task check once, stated AC evidence) before optional polish that only deepens beyond the bar.
  • ! If remaining budget cannot fund another full Read-Write-Lint-Diff pass and a fix for a new finding, stop after the stated-pass chokepoint and record deepening_skipped=true with reason (#1006).
  • ! Dual-stop pre-PR defaults (max 3 polish / 2 no-diff) still apply; under a hard cap, prefer fewer polish iterations once AC + check are green.
  • ⊗ Start an unbounded self-imposed verification suite that exceeds stated AC while the hard budget is nearly exhausted (#3266).

Product-first done-gate (#3284) / literal AC (#3267)

  • ! Before Phase 3 Lint exit / push, when the active scope xBRIEF has acceptance commands (plan.acceptance or #3267 metadata/narratives), run task verify:ac -- <active-xbrief> and fail closed on exit 1. (verify:literal-ac is the mechanism alias.)
  • ! Commands must be the exact strings from the task statement — same flags and cwd. Self-chosen verification is supplementary only.
  • ! Full task check already runs verify:ac first; still run it explicitly before push when AC is stated so failures are visible without the full hygiene suite.
  • ⊗ Skip AC run on rapid/minimal ceremony dial — rapid = AC-only; AC never degrades when commands exist (#3284 / #3156).

Product-oracle gate integrity (#3322 / #3156)

A red product verification may be resolved only by a product change or an independently re-derived oracle (both sides rebuilt from scratch, different method). In-place repair of the failing comparison then pass is not a pass — it is an unresolved discrepancy.

  • ! When a product oracle is red, resolve it by changing the product or by independently re-deriving the oracle, and record independent_rederivation on the run-summary verification event.
  • ! Emit a run-summary verification event {check_id, method_fingerprint, outcome} for each product-oracle attempt when DEFT_RUN_SUMMARY_PATH is set. fail then a different method_fingerprint then pass on one check id is machine-flagged.
  • ! task verify:ac treats comparison-method mutation as unresolved (exit non-zero) unless independent re-derivation is recorded. Lead the done report with any unresolved discrepancy (#1006).
  • ⊗ Self-adjudicate a red product oracle by editing the comparison (reference file, diff invocation, one-sided regenerate) and shipping the new pass as success.

Probe-then-fill remote claims (#3120)

! Before filling any remote handoff field (PR URL, PR number, commit/HEAD SHA, CI green/success, review score) or claiming status: pass / ship/gate done, MUST probe then fill:

  1. Run same-turn git + forge probes (examples: git rev-parse HEAD, gh api repos/<owner>/<repo>/pulls/<N>, task pr:watch -- <N> --one-shot, checks API).
  2. Copy IDs / URLs / SHAs / scores only from that probe JSON/text into the evidence block.
  3. Set proof_status: bound and attach short raw probe snippets (command + snippet) for each remote claim.

! Handoff evidence axes: work (local) / ship (pushed branch or PR) / gate (CI/review on HEAD). proof_status is bound | unbound | n/a-no-remote-claim.
! Legal partial: local work done + ship not_started / blocked without PR/SHA/CI/review fields and proof_status: n/a-no-remote-claim (or status: partial) is valid — do not invent ship state.
! Fail ranking: invented-done (false/unbound remote artifacts under pass) is stricter than empty-done. Unbound remote claims → invalid evidence (fail), not pass-with-notes.
! Machine check: validateHandoffEvidence in packages/core/src/handoff-evidence/ (see templates/agent-prompt-preamble.md §11).
⊗ Fill PR / SHA / CI / review fields from recollection, narration, or prior-turn memory.
⊗ Claim status: pass with remote fields when proof_status is not bound or probes are missing (#3120).

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

Significant decision log (#1396)

! Before push, if this PR encodes a significant choice not already recorded, run task decision:write (significance filter: architecture, product behavior, security, boundaries, data model, topology, hard-to-reverse process).

~ Check task decision:list for related process policy so the PR does not silently reverse a dogfood decision.

⊗ Invent revisit-free rationale in the PR body as a substitute for a decision record when the choice is significant.
⊗ Noise the log with trivial scope completions.

Docs: docs/decision-log.md.

Anti-Patterns

  • ⊗ Submit a PR without running this pre-PR loop (Read-Write-Lint-Diff) -- 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. Intent mode (#3015) also refuses bare/conditional real Closes #N without --allow-close. 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)
  • ⊗ Invent remote PR/SHA/CI/review claims in handoff evidence without same-turn probe binding — invented-done (#3120)
  • ⊗ Fill remote ship/gate fields from memory when only local work completed; legal partial omits PR fields (#3120)
  • ⊗ Clear a failing gate by editing the gate (definition, verifier, reward, required check, coverage floor, policy, eval fixture) instead of the work under test — gate integrity (#3156); see docs/gate-integrity.md
  • ⊗ Clear a red product oracle by editing the comparison method then treating the new pass as a pass — record independent re-derivation or fix the product (#3322 / #3156)
  • ⊗ Under a hard turn/cost budget, gold-plate pre-PR polish past the stated bar until the budget expires (#3266)
  • ⊗ Exit pre-PR after skipping deepen-for-budget without naming the skip in the summary (#3266 / #1006)