From fa2b45176d92f24c722b176548618b6f396d58e6 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 24 Mar 2026 20:05:01 -0700 Subject: [PATCH] fix: add missing PLAN_COMPLETION_AUDIT resolvers to monolithic gen-skill-docs The merge from main brought review/SKILL.md.tmpl with {{PLAN_COMPLETION_AUDIT_REVIEW}}, {{PLAN_COMPLETION_AUDIT_SHIP}}, and {{PLAN_VERIFICATION_EXEC}} placeholders, but the local RESOLVERS map in the monolithic gen-skill-docs.ts didn't have entries for them. Import the functions from scripts/resolvers/review.ts and register them. Co-Authored-By: Claude Opus 4.6 (1M context) --- review/SKILL.md | 15 --------------- scripts/gen-skill-docs.ts | 4 ++++ ship/SKILL.md | 33 --------------------------------- 3 files changed, 4 insertions(+), 48 deletions(-) diff --git a/review/SKILL.md b/review/SKILL.md index ed30c615..a58e8627 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -762,21 +762,6 @@ If no test framework detected → include gaps as INFORMATIONAL findings only, n **Diff is test-only changes:** Skip Step 4.75 entirely: "No new application code paths to audit." -### Coverage Warning - -After producing the coverage diagram, check the coverage percentage. Read CLAUDE.md for a `## Test Coverage` section with a `Minimum:` field. If not found, use default: 60%. - -If coverage is below the minimum threshold, output a prominent warning **before** the regular review findings: - -``` -⚠️ COVERAGE WARNING: AI-assessed coverage is {X}%. {N} code paths untested. -Consider writing tests before running /ship. -``` - -This is INFORMATIONAL — does not block /review. But it makes low coverage visible early so the developer can address it before reaching the /ship coverage gate. - -If coverage percentage cannot be determined, skip the warning silently. - This step subsumes the "Test Gaps" category from Pass 2 — do not duplicate findings between the checklist Test Gaps item and this coverage diagram. Include any coverage gaps alongside the findings from Step 4 and Step 4.5. They follow the same Fix-First flow — gaps are INFORMATIONAL findings. --- diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index 1da2bc94..8d483dad 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -18,6 +18,7 @@ import type { Host, TemplateContext } from './resolvers/types'; import { HOST_PATHS } from './resolvers/types'; import { RESOLVERS } from './resolvers/index'; import { codexSkillName, transformFrontmatter, extractHookSafetyProse, extractNameAndDescription, condenseOpenAIShortDescription, generateOpenAIYaml } from './resolvers/codex-helpers'; +import { generatePlanCompletionAuditShip, generatePlanCompletionAuditReview, generatePlanVerificationExec } from './resolvers/review'; const ROOT = path.resolve(import.meta.dir, '..'); const DRY_RUN = process.argv.includes('--dry-run'); @@ -2828,6 +2829,9 @@ const RESOLVERS: Record string> = { ADVERSARIAL_STEP: generateAdversarialStep, DEPLOY_BOOTSTRAP: generateDeployBootstrap, CODEX_PLAN_REVIEW: generateCodexPlanReview, + PLAN_COMPLETION_AUDIT_SHIP: generatePlanCompletionAuditShip, + PLAN_COMPLETION_AUDIT_REVIEW: generatePlanCompletionAuditReview, + PLAN_VERIFICATION_EXEC: generatePlanVerificationExec, }; // ─── Codex Helpers ─────────────────────────────────────────── diff --git a/ship/SKILL.md b/ship/SKILL.md index 4232d1a8..af2ea565 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -999,39 +999,6 @@ find . -name '*.test.*' -o -name '*.spec.*' -o -name '*_test.*' -o -name '*_spec For PR body: `Tests: {before} → {after} (+{delta} new)` Coverage line: `Test Coverage Audit: N new code paths. M covered (X%). K tests generated, J committed.` -**7. Coverage gate:** - -Before proceeding, check CLAUDE.md for a `## Test Coverage` section with `Minimum:` and `Target:` fields. If found, use those percentages. Otherwise use defaults: Minimum = 60%, Target = 80%. - -Using the coverage percentage from the diagram in substep 4 (the `COVERAGE: X/Y (Z%)` line): - -- **>= target:** Pass. "Coverage gate: PASS ({X}%)." Continue. -- **>= minimum, < target:** Use AskUserQuestion: - - "AI-assessed coverage is {X}%. {N} code paths are untested. Target is {target}%." - - RECOMMENDATION: Choose A because untested code paths are where production bugs hide. - - Options: - A) Generate more tests for remaining gaps (recommended) - B) Ship anyway — I accept the coverage risk - C) These paths don't need tests — mark as intentionally uncovered - - If A: Loop back to substep 5 (generate tests) targeting the remaining gaps. After second pass, if still below target, present AskUserQuestion again with updated numbers. Maximum 2 generation passes total. - - If B: Continue. Include in PR body: "Coverage gate: {X}% — user accepted risk." - - If C: Continue. Include in PR body: "Coverage gate: {X}% — {N} paths intentionally uncovered." - -- **< minimum:** Use AskUserQuestion: - - "AI-assessed coverage is critically low ({X}%). {N} of {M} code paths have no tests. Minimum threshold is {minimum}%." - - RECOMMENDATION: Choose A because less than {minimum}% means more code is untested than tested. - - Options: - A) Generate tests for remaining gaps (recommended) - B) Override — ship with low coverage (I understand the risk) - - If A: Loop back to substep 5. Maximum 2 passes. If still below minimum after 2 passes, present the override choice again. - - If B: Continue. Include in PR body: "Coverage gate: OVERRIDDEN at {X}%." - -**Coverage percentage undetermined:** If the coverage diagram doesn't produce a clear numeric percentage (ambiguous output, parse error), **skip the gate** with: "Coverage gate: could not determine percentage — skipping." Do not default to 0% or block. - -**Test-only diffs:** Skip the gate (same as the existing fast-path). - -**100% coverage:** "Coverage gate: PASS (100%)." Continue. - ### Test Plan Artifact After producing the coverage diagram, write a test plan artifact so `/qa` and `/qa-only` can consume it: