spec-review.ts
297 lines · scripts/resolvers/spec-review.ts at 3f79d81
1/**
2 * Spec review loops, benefits-from, and the anti-shortcut clause.
3 *
4 * Moved from scripts/resolvers/review.ts.
5 */
6import { type TemplateContext } from './types';
7import { generateInvokeSkill } from './composition';
8import { CC_BACKGROUND_DEFAULT_SINCE, FOREGROUND_IF_AVAILABLE, BACKGROUND_RECOVERY } from './constants';
9import { DESIGN_DOC_DISCOVERY_BLOCK } from './design-doc-discovery';
11export function generateAntiShortcutClause(_ctx: TemplateContext): string {
12 if (_ctx.skillName === 'plan-ceo-review') return `**Anti-shortcut clause:** Analyze → resolve → apply for each section before advancing. The plan file records the interactive review; it cannot replace it. Do not prewrite the remaining sections or their implementation tasks and then walk through a fixed question list. Proposed findings are not accepted plan changes: mark them pending until their actual decisions are made. Ask once per unresolved or reopened issue, wait for the answer, and apply only the exact accepted choice and scope to the working plan. An earlier approach selection does not authorize unrelated choices. Keep established contracts, accepted decisions, and their evidence available to later sections; new material risks or changed remedies still need approval. Cross-referencing settled decisions never replaces the full review and terminal report. Follow the working review decisions below; never invent a question merely because a new section starts.`;
13 if (_ctx.skillName === 'plan-design-review') return `**Anti-shortcut clause:** Review every section and outside voice finding. The plan records the review; writing a finding into it is not approval. For each finding:
15- **New or reopened choice:** Ask once per independent decision, wait for the actual answer, then apply only its accepted scope. Present concrete new risks or changed assumptions that reopen an earlier choice.
16- **Work already approved:** Necessary code, tests and docs for an exact previously selected contract do not reopen it. Cite the selected answer and scope, retain the finding and proof, and disclose the follow-through. A broad approach or recommendation does not approve independent remedies or optional verification depth.
17- **Factual correction:** Correct descriptions against source evidence without authorizing behavior changes.
19Never skip sections or the terminal report. Do not invent a question merely because a finding came from another section or reviewer.`;
20 if (_ctx.skillName === 'plan-eng-review') return `**Anti-shortcut clause:** Use the decision gate for all four sections and outside voice. Retain findings and evidence. Ask only for new or reopened choices and apply their exact answers. Never prewrite unapproved remedies or skip sections or the terminal report.`;
22 if (_ctx.skillName === 'plan-devex-review') return `**Anti-shortcut clause:** Evaluate every section and outside voice finding through the decision gate below. The plan records the interactive review; writing findings into it never substitutes for approval. Ask once per new or reopened independent decision, wait for the actual answer, and apply only its accepted scope. Necessary code, tests and docs for an exact previously selected contract do not reopen it: cite that selected answer and scope, retain the finding and proof, and disclose the follow-through. Correct factual descriptions against source evidence without authorizing behavior changes. A broad approach or recommendation does not approve independent remedies or optional verification depth. Concrete new risks or changed assumptions may reopen a decision and must be presented. Never skip sections or the terminal report, or invent a question merely because a finding came from another section or reviewer.`;
23 return `**Anti-shortcut clause:** The plan file records the interactive review; it cannot replace it. Take each non-trivial finding through AskUserQuestion, wait for the answer, and apply only the accepted choice before writing it into the plan. Only a review with zero findings in every section reaches ExitPlanMode without asking. Never skip sections or the terminal report.`;
24}
26function generateOfficeHoursSpecReviewLoop(): string {
27 return `## Spec Review Loop
29Run an adversarial review before presenting the final document to the user.
30Follow the calling workflow's approval steps.
31The reviewer's saved JSON is the complete verdict. A prose summary is not a second
32finding inventory: the report helper preserves every problem/remedy and counts the
33records mechanically. Do not rewrite, condense, deduplicate, or recount its blocks.
35**Step 1: Prepare and dispatch the reviewer**
37Create a fresh review directory next to the design:
39\`\`\`bash
40mktemp -d "<design-path>.review.XXXXXX"
41\`\`\`
42Keep these evidence files with the design. Maximum 3 iterations total. Before EACH dispatch, generate the complete prompt using
43all preceding valid round files in order (omit them for round 1):
45\`\`\`bash
46~/.claude/skills/gstack/bin/gstack-office-hours-review prepare --design "<design-path>" --out-dir "<review-directory>" "<round-1.json if present>" "<round-2.json if present>"
47\`\`\`
49Omit absent arguments rather than passing placeholders. Users get 3 rounds; only
50when a caller sets a lower review round limit, add \`--max-rounds <N>\` with that
51same value to every prepare, check, and finalize command. The helper chooses the next
52round and writes \`round-N.prompt.md\`. It includes the full finding schema, all five
53review dimensions (Completeness, Consistency, Clarity, Scope, Feasibility), the
54office-hours coaching contract, and the COMPLETE preceding JSON verdict. It also
55saves the design as reviewed (\`round-N.design.md\`). Round 1 is a full review;
56rounds 2 and 3 are delta re-reviews of the exact design diff since the last round,
57so prepare every round in the same review directory and do not edit the design
58between prepare and its review.
60Use the Agent tool with ${FOREGROUND_IF_AVAILABLE} and its returned \`dispatch\`
61string unchanged as the prompt. ${BACKGROUND_RECOVERY} The reviewer must Read the entire prepared prompt
62file before reviewing the design. Do not recreate the prompt, copy selected fields,
63or summarize prior findings. A parent Read does not deliver the file to the reviewer.
64The reviewer has fresh context and cannot see the brainstorming conversation.
65Its prepared contract requires a complete JSON Write, sealed by the dispatch's
66\`Seal:\` command, and a one-line \`OFFICE_HOURS_VERDICT\` receipt as its entire response.
67It protects the required coaching and Assignment sections, distinguishes unknown
68customer facts from committed behavior, and requires evidence for every prior status.
70**Step 2: Check stop conditions, then fix and re-dispatch**
72After each verdict, BEFORE fixing any findings or dispatching again, validate the
73saved files with the helper. Pass the reviewer's entire response unchanged as the
74receipt (a lone \`OFFICE_HOURS_VERDICT\` line; with a quote, backtick, \`$\` or \`\\\` it is
75malformed) and list every completed round in order:
77\`\`\`bash
78~/.claude/skills/gstack/bin/gstack-office-hours-review check --receipt "<receipt line>" "<round-1.json>" "<round-2.json if present>" "<round-3.json if present>"
79\`\`\`
81A missing, malformed, or mismatched receipt fails the check: that attempt is a failed review.
83**Convergence guard and stopping rules:** Each finding is blocking or minor. Only
84blocking findings require another round. Read its stop reason:
85- PASS: no blocking findings remain. Any minor findings are recorded, not fixed,
86 and never justify another round; proceed to Step 3.
87- CONVERGENCE: the reviewer explicitly marked a blocking prior obligation
88 persisting with a concrete prior/current finding pair and document evidence.
89 Stop even if new findings appear. Shared topic labels or new refinements alone
90 are insufficient.
91- MAX_ITERATIONS: round 3 completed (or the caller's lower round limit); stop.
92- CONTINUE: fix only the blocking findings in the design, then return to Step 1
93 to prepare and dispatch the next review. Do not edit for minor findings
94 mid-loop: they stay recorded for the user, and new text only gives the next
95 diff more to review. Its reviewer checks every prior finding and raises new
96 blocking findings only for problems your changes introduced or exposed.
98On a stop, do not fix again or re-dispatch. Run the finalizer before approval:
100\`\`\`bash
101~/.claude/skills/gstack/bin/gstack-office-hours-review finalize --design "<design-path>" "<round-1.json>" "<round-2.json if present>" "<round-3.json if present>"
102\`\`\`
104It installs the complete \`## Reviewer Concerns\` section directly from the JSON.
105Recording concerns does not mark them fixed. Do not edit that generated section.
106At the cap (MAX_ITERATIONS or CONVERGENCE) it lists the unresolved blocking findings
107as numbered gaps. Never launch a fourth reviewer or re-review unchanged input. Either
108apply the listed fixes first and add \`--fixes <fixes.json>\`
109(\`[{"id":"<finding id>","applied_text":"<verbatim new excerpt>"}]\`): it records
110\`issues verified: k of N\` deterministically (new text present, snapshot lacks it),
111never a review verdict; or finalize without it and carry its
112\`N fixes unconfirmed — decision: spec not re-verified\` into the approval step, which
113stays blocked until the owner accepts it explicitly.
114Then proceed to Step 3 and the existing user approval.
116If the subagent fails, times out, or is unavailable — stop the loop and present the
117document unreviewed. Tell the user: "Spec review unavailable — presenting unreviewed doc."
118A missing or invalid verdict is an explicit review failure, never PASS. Preserve the
119failed output and its error. Finalize with \`--unreviewed "<actual failure cause>"\`
120and only the preceding valid round files (none if round 1 failed); their known
121concerns remain visible. Do not fabricate JSON or hide a completed verdict behind
122UNREVIEWED. The independent review remains a quality bonus, not an approval gate.
124**Step 3: Report and persist metrics**
126The finalizer prints the exact Spec Review block, quality score, and metrics. Tell the user the
127result using that block; link the design and saved verdicts for details. Confirmed
128resolutions require explicit later reviewer evidence; attempted fix rounds are never
129described as successful fixes.
131When writing a completion report, write its other sections normally, then run the
132same finalizer with \`--report "<report-path>"\` after the report exists. This installs
133its authoritative \`## Spec Review\` section and Disposition mechanically. Do not
134summarize or replace that section afterward; refer to it elsewhere instead of
135inventing duplicate counts. Preserve the Assignment, coaching, approval, and Handoff.
137Append the helper's actual metrics to the existing analytics log (telemetry is
138best-effort and must not block approval):
139\`\`\`bash
140GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "\${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}"
141mkdir -p "$GSTACK_STATE_ROOT/analytics"
142echo '{"skill":"office-hours","ts":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","iterations":ITERATIONS,"issues_found":FOUND,"issues_fixed":FIXED,"remaining":REMAINING,"quality_score":SCORE}' >> "$GSTACK_STATE_ROOT/analytics/spec-review.jsonl" 2>/dev/null || true
143\`\`\`
144Use iterations, issues_found, issues_fixed, remaining, and quality_score from the
145helper; its remaining_blocking and remaining_minor split the remaining count. FOUND counts finding observations across rounds; FIXED counts only
146reviewer-confirmed resolutions. An unavailable score is null, never invented.`;
147}
149export function generateSpecReviewLoop(_ctx: TemplateContext): string {
150 if (_ctx.skillName === 'office-hours') return generateOfficeHoursSpecReviewLoop();
151 const ceo = _ctx.skillName === 'plan-ceo-review';
152 return `${ceo ? '####' : '##'} Spec Review Loop
154Run an adversarial review before presenting the final document to the user.
155${ceo ? 'Use 0D for any new or reopened amendment discovered by the reviewer. The later 0H approval approves only the completed working plan and CEO summary, not unresolved amendments.' : "Follow the calling workflow's approval steps."}
157**Step 1: Dispatch reviewer subagent**
159${ceo ? `Read Agent's tool definition. Set \`run_in_background: false\` if that field is available; omit it otherwise. Launch one reviewer with both inputs below.
161If the result contains a completed review, consume it. If it returns a pending task, use the host's wait tool. With no wait tool, end this response and resume on its completion notification. While waiting, do not advance, edit either input or launch another reviewer.` : `Use Agent with JSON boolean \`run_in_background: false\`, never string \`"false"\`.
162Subagents default to background since ${CC_BACKGROUND_DEFAULT_SINCE}. Async launch metadata
163is not a verdict: wait for that agent's final review before continuing; do not launch a duplicate.
164The reviewer has fresh context: only the document, not the conversation.`}
166Prompt the subagent with:
167- ${ceo ? 'Both saved absolute paths, or both complete labeled texts if either input is not persisted: CEO scope summary and current amended working plan. No other conversation context.' : 'The file path of the document just written'}
168${ceo ? `- "Read both inputs in full. Evaluate them together on all five dimensions.
169 Flag contradictions, unsupported accepted expansions and required behavior
170 missing from both. Cite input and requirement for each finding. If either
171 input is unavailable or incomplete, report that failure instead of grading
172 partial input."` : `- "Read this document and review it on 5 dimensions. For each dimension, note PASS or
173 list specific issues with suggested fixes. At the end, output a quality score (1-10)
174 across all dimensions."`}
176**Dimensions:**
177${ceo ? `1. **Completeness** — requirements and edge cases.
1782. **Consistency** — no contradictions.
1793. **Clarity** — implementable without follow-up questions.
1804. **Scope** — no unapproved creep or YAGNI.
1815. **Feasibility** — buildable with the stated approach.` : `1. **Completeness** — Are all requirements addressed? Missing edge cases?
1822. **Consistency** — Do parts of the document agree with each other? Contradictions?
1833. **Clarity** — Could an engineer implement this without asking questions? Ambiguous language?
1844. **Scope** — Does the document creep beyond the original problem? YAGNI violations?
1855. **Feasibility** — Can this actually be built with the stated approach? Hidden complexity?`}
187The subagent should return:
188- A quality score (1-10)${ceo ? " across all dimensions" : ""}
189${ceo ? '- For each dimension, PASS or numbered issues with suggested fixes. Overall PASS only if all dimensions pass.' : '- PASS if no issues, or a numbered list of issues with dimension, description, and fix'}
191${ceo ? `**Step 2: Process the result**
193- **Unavailable:** If launch or review fails, times out, or cannot review both complete inputs, stop the loop. Say "Spec review unavailable — presenting unreviewed doc." Preserve the failure and all prior findings. Continue to Step 3 to record the unavailable outcome; a successful reviewer result is not required.
194- **PASS:** Stop the loop.
195- **Issues:** Stop after the third review, or when consecutive reviews repeat the same unresolved issues (the same requirements and problems). Otherwise use 0D for new or reopened choices, amend the working plan and CEO summary under the storage policy, Keep both consistent, and re-dispatch with both updated inputs and the same instructions.
197Make at most three reviewer launches. A missing score alone does not require another review. At the cap, list the unresolved issues as numbered gaps and never re-review unchanged input: verify each listed fix against the amended plan text and report \`issues verified: k of N\` (applied edits, not a verdict), or report \`N fixes unconfirmed\` and add a \`spec not re-verified\` decision to 0H. DONE_WITH_CONCERNS names the missing points next to the score.` : `**Step 2: Fix and re-dispatch**
199If the reviewer returns issues:
2001. Fix each issue in the document on disk (use Edit tool)
2012. Re-dispatch the reviewer subagent with the updated document
2023. Maximum 3 iterations total
204**Convergence guard:** If the reviewer returns the same issues on consecutive iterations
205(the fix didn't resolve them or the reviewer disagrees with the fix), stop the loop
206and persist those issues as "Reviewer Concerns" in the document rather than looping
207further.
209If the subagent fails, times out, or is unavailable — skip the review loop entirely.
210Tell the user: "Spec review unavailable — presenting unreviewed doc." The document is
211already written to disk; the review is a quality bonus, not a gate.`}
213**Step 3: Report and persist metrics**
215${ceo ? `Report the outcome and fields below. Show full reviewer output on request. List unresolved issues under "## Reviewer Concerns" in the CEO summary, citing the owning input.
217SCORE is the latest attempt's reported 1–10 grade after reviewing both full inputs. For an unavailable review or missing/invalid grade, use JSON \`null\` ("score unavailable"). Label earlier grades "prior review score".
219Recording the **0H spec-review metrics** is
220required when writing is permitted, even if the reviewer failed. Append the
221actual outcome below; failed mkdir or append stops the review. When writing is
222forbidden, show the actual fields as not persisted and continue without writing.
223If the reviewer fails, report that limit and continue after recording the outcome;
224if a required save fails, stop before claiming completion.` : `After the loop completes (PASS, max iterations, or convergence guard):
2261. Tell the user the result — summary by default:
227 "Your doc survived N rounds of adversarial review. M issues caught and fixed.
228 Quality score: X/10."
229 If they ask "what did the reviewer find?", show the full reviewer output.
2312. If issues remain after max iterations or convergence, add a "## Reviewer Concerns"
232 section to the document listing each unresolved issue as a numbered gap. Never
233 re-review unchanged input: record \`issues verified: k of N\` after checking each
234 listed fix in the text (not a verdict), or \`N fixes unconfirmed\` with a
235 \`spec not re-verified\` decision. DONE_WITH_CONCERNS names the missing points next
236 to the score. Downstream skills will see this.
2383. Append metrics:`}
239\`\`\`bash
240GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "\${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}"
241mkdir -p "$GSTACK_STATE_ROOT/analytics"${ceo ? ' || exit 1' : ''}
242echo '{"skill":"${_ctx.skillName}","ts":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","iterations":ITERATIONS,"issues_found":FOUND,"issues_fixed":FIXED,"remaining":REMAINING,"quality_score":SCORE}' >> "$GSTACK_STATE_ROOT/analytics/spec-review.jsonl"${ceo ? ' || exit 1' : ' 2>/dev/null || true'}
243\`\`\`
244${ceo ? 'ITERATIONS counts actual reviewer launches. FOUND, FIXED and REMAINING count reported issues, reviewer-confirmed fixes and reported unresolved issues. Use actual counts, never estimates.' : 'Replace ITERATIONS, FOUND, FIXED, REMAINING, SCORE with actual values from the review.'}`;
245}
247export function generateBenefitsFrom(ctx: TemplateContext): string {
248 if (!ctx.benefitsFrom || ctx.benefitsFrom.length === 0) return '';
250 const skillList = ctx.benefitsFrom.map(s => `\`/${s}\``).join(' or ');
251 const first = ctx.benefitsFrom[0];
253 // Reuse the INVOKE_SKILL resolver for the actual loading instructions
254 const invokeBlock = generateInvokeSkill(ctx, [first]);
256 return `## Prerequisite Skill Offer
258When the design doc check above prints "No design doc found," offer the prerequisite
259skill before proceeding.
261Skip the offer and proceed with the standard review when the preamble echoed \`SESSION_KIND\` \`spawned\` or \`headless\`.
263${ctx.skillName === 'plan-eng-review' ? 'Build the next full decision brief from these facts and options, using the preamble transport, numbering and format:' : 'Say to the user via AskUserQuestion:'}
265> "No design doc found for this branch. ${skillList} produces a structured problem
266> statement, premise challenge, and explored alternatives — it gives this review much
267> sharper input to work with. Takes about 10 minutes. The design doc is per-feature,
268> not per-product — it captures the thinking behind this specific change."
270Options:
271- A) Run /${first} now (we'll pick up the review right after)
272- B) Skip — proceed with standard review
274If they skip: "No worries — standard review. If you ever want sharper input, try
275/${first} first next time." Then proceed normally. Do not re-offer later in the session.
277If they choose A:
279Say: "Running /${first} inline. Once the design doc is ready, I'll pick up
280the review right where we left off."
282${invokeBlock}
284${ctx.skillName === 'plan-eng-review' ? `After /${first} completes, rerun the complete **Design Doc Check** block above.
285This is a fresh execution: the prerequisite may have created a design doc.
286Read the resulting doc if found; otherwise continue the standard review.
287Do not rerun the preamble or re-offer the prerequisite.` : `After /${first} completes, re-run the design doc check:
288\`\`\`bash
289setopt +o nomatch 2>/dev/null || true # zsh compat
290SLUG=$(~/.claude/skills/gstack/browse/bin/remote-slug 2>/dev/null || basename "$(git rev-parse --show-toplevel 2>/dev/null || pwd)")
291BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null | tr '/' '-' || echo 'no-branch')
292${DESIGN_DOC_DISCOVERY_BLOCK}
293\`\`\`
295If a design doc is now found, read it and continue the review.
296If none was produced (user may have cancelled), proceed with standard review.`}`;
297}