diff --git a/plan-ceo-review/sections/review-sections.md b/plan-ceo-review/sections/review-sections.md index f7aa4eb8f..1b82ecc1f 100644 --- a/plan-ceo-review/sections/review-sections.md +++ b/plan-ceo-review/sections/review-sections.md @@ -333,7 +333,7 @@ Continue through the blocks below in file order: 2. **Resolve remaining TODO choices:** use the selected mode's scope rules. 3. **Approval readiness:** check the ledger and record PASS before writing outputs. Its complete checklist is immediately after the TODO choices; no report or log is needed yet. 4. **Required Outputs:** follow the three stages below: prepare the plan body and summary, save and verify the terminal report, then publish the summary in chat. -5. **Cleanup and history:** perform permitted cleanup, attempt Review Log under the Artifact outcomes policy, then display the dashboard with the actual logging outcome. +5. **Cleanup and history:** archive old CEO plans only if the user approved it (otherwise skip cleanup), attempt Review Log under the Artifact outcomes policy, then display the dashboard with the actual logging outcome. 6. **Navigation:** choose Next Steps and any docs/designs promotion; queue the next skill. For a substantive answer, call 0D for only that change, repeat Approval readiness and Required Outputs, then repeat step 5. Resume navigation without asking settled choices again. Navigation alone does not reopen decisions. 7. **Learnings:** finish learning and brain write-back. Return to this skill's main `SKILL.md`, at **Section self-check**. Its EXIT gate verifies completed work and saved readiness without asking again. After a passing gate, refresh the cache, run telemetry last, then exit or return to the caller. diff --git a/plan-eng-review/SKILL.md b/plan-eng-review/SKILL.md index c7d76b2e8..005d8a926 100644 --- a/plan-eng-review/SKILL.md +++ b/plan-eng-review/SKILL.md @@ -50,7 +50,7 @@ Name the selected plan by its title or path; use "this draft" only for an untitl When no exception above applied: -1. Choose listed, enabled MCP AskUserQuestion, otherwise listed native. First tool call = AskUserQuestion (tool_use). Send this exact menu and wait. +1. Choose listed, enabled MCP AskUserQuestion, otherwise listed native. First tool call = AskUserQuestion (tool_use). Send this exact menu and wait; its Recommendation line advises the user, so do not inspect the repository to resolve it. 2. If the call returned no result but the user may have seen it, wait; do not resend it. If the tool is unavailable, disallowed (`--disallowedTools`) or failed before reaching the user, send the menu as plain prose and STOP, whatever the session type; the session type never chooses a target or approves work. Options start at column 0, without blockquotes. Never guess a target. What should I review? diff --git a/plan-eng-review/sections/review-sections.md b/plan-eng-review/sections/review-sections.md index bb7831b69..f3c376058 100644 --- a/plan-eng-review/sections/review-sections.md +++ b/plan-eng-review/sections/review-sections.md @@ -51,8 +51,8 @@ QA Test Plan/task JSONL keep discovery paths `$GSTACK_STATE_ROOT/projects/{slug} existing destination and preserve its content. For a new file, create permitted parent directories, then write that header, an unchanged copy of the original plan (for plan targets), and the scope record or ledger being saved. Recheck option 3's collision before -creation; use a suffix rather than overwrite. Do not add findings or fixes before -Scope Challenge C. Put records before an existing `## GSTACK REVIEW REPORT`, or +creation; use a suffix rather than overwrite. Findings and fixes first enter +through Scope Challenge C's ledger saves, never earlier. Put records before an existing `## GSTACK REVIEW REPORT`, or at EOF if absent; create that terminal report only at Plan File Review Report. **Write routes.** A forbidden write follows its row above. @@ -728,11 +728,11 @@ A proposal that fails the value bar becomes "extend " or is dropp Run the decision gate for this section's new or reopened choices. **STOP for each pending decision.** Wait for its answer before applying that remedy, moving to the next section or calling ExitPlanMode. -When these test and eval choices are resolved, write the Test Plan Artifact below. Its approved requirements should be specific enough to implement alongside the feature code. +Then write the Test Plan Artifact below, even while some choices are still unanswered (for example in a non-interactive run). Its approved requirements should be specific enough to implement alongside the feature code. #### Test Plan Artifact -After resolving the Test review decisions, record the approved test requirements in an artifact for `/qa` and `/qa-only`. List any unresolved choices separately as pending, not required implementation. Update this artifact if later approved decisions change the tests. Use the Review record and write policy above. +After the Test review decision gate, record the approved test requirements in an artifact for `/qa` and `/qa-only`. Write it even when choices are still pending; list any unresolved choices separately as pending, not required implementation. Update this artifact if later approved decisions change the tests. Use the Review record and write policy above. ```bash GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}" diff --git a/qa-only/SKILL.md b/qa-only/SKILL.md index 716606a28..7b1131632 100644 --- a/qa-only/SKILL.md +++ b/qa-only/SKILL.md @@ -536,7 +536,7 @@ Each proposed test carries a value card; propose it only when it passes this bar 1. Why does existing coverage not already catch that? Prefer adding a row to an existing table-driven test or shared fixture over a near-duplicate. 2. Does it need a production seam (export, flag, wrapper, injection hook) that no production caller needs? If yes, test at the real boundary instead. -Value card: `Value: protects=<...>; fails_when=<...>; why_new=<...>; seam=none` (seam: `none` or its name); each field at most 160 UTF-8 bytes here (clamp to 157 plus `...`; JSON keeps full values). Put it in the 8e.5 record (/qa) or under each proposed test (/qa-only). A missing upstream card never blocks: derive it; ignore unknown fields. +Value card: `Value: protects=<...>; fails_when=<...>; why_new=<...>; seam=none` (seam: `none` or its name); each field at most 160 UTF-8 bytes here (clamp to 157 plus `...`; JSON keeps full values). Put it under each proposed test. A missing upstream card never blocks: derive it; ignore unknown fields. Example: Value: protects=refundPayment rejects an empty reason; fails_when=the reason guard is removed or inverted; why_new=billing.test.ts covers processPayment only; seam=none Rejected (covered_elsewhere): "checkout renders"; checkout.e2e.ts:15 covers it, so extend that test. diff --git a/qa/SKILL.md b/qa/SKILL.md index 516535ea4..59cfc8155 100644 --- a/qa/SKILL.md +++ b/qa/SKILL.md @@ -629,7 +629,7 @@ and unclear contracts never authorize repair. 1. Why does existing coverage not already catch that? Prefer adding a row to an existing table-driven test or shared fixture over a near-duplicate. 2. Does it need a production seam (export, flag, wrapper, injection hook) that no production caller needs? If yes, test at the real boundary instead. -Value card: `Value: protects=<...>; fails_when=<...>; why_new=<...>; seam=none` (seam: `none` or its name); each field at most 160 UTF-8 bytes here (clamp to 157 plus `...`; JSON keeps full values). Put it in the 8e.5 record (/qa) or under each proposed test (/qa-only). A missing upstream card never blocks: derive it; ignore unknown fields. +Value card: `Value: protects=<...>; fails_when=<...>; why_new=<...>; seam=none` (seam: `none` or its name); each field at most 160 UTF-8 bytes here (clamp to 157 plus `...`; JSON keeps full values). Put it in the 8e.5 record. A missing upstream card never blocks: derive it; ignore unknown fields. Example: Value: protects=refundPayment rejects an empty reason; fails_when=the reason guard is removed or inverted; why_new=billing.test.ts covers processPayment only; seam=none Rejected (covered_elsewhere): "checkout renders"; checkout.e2e.ts:15 covers it, so extend that test. diff --git a/test/fixtures/context-budget.json b/test/fixtures/context-budget.json index fcecaf8a1..ecefa44c6 100644 --- a/test/fixtures/context-budget.json +++ b/test/fixtures/context-budget.json @@ -47,10 +47,10 @@ "plan-ceo-review": 20687, "plan-design-review": 20550, "plan-devex-review": 17492, - "plan-eng-review": 13073, + "plan-eng-review": 13096, "plan-tune": 14585, - "qa": 13285, - "qa-only": 11352, + "qa": 13273, + "qa-only": 11341, "retro": 18694, "review": 19288, "scrape": 6905,