From 00cefcafb1f4e6b3c931bb0798af19950eaaade9 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 19 Mar 2026 01:36:26 -0500 Subject: [PATCH] feat: review chaining + commit hash staleness tracking (v0.8.3) (#206) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: review chaining + commit hash staleness tracking Each plan review skill now suggests the next review via AskUserQuestion: - CEO review → eng review (required gate) + design review (if UI scope) - Design review → eng review + CEO review (if product gaps) - Eng review → design review (if UI changes) + CEO review (soft suggestion) Reviews now track HEAD commit hash in JSONL entries for deterministic staleness detection. Dashboard compares stored hash against current HEAD and reports drift. Respects skip_eng_review config in chaining logic. Also adds commit tracking to design-review-lite entries. * chore: regenerate SKILL.md files for review chaining * chore: bump version and changelog (v0.8.3) Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- CHANGELOG.md | 9 ++++++ VERSION | 2 +- plan-ceo-review/SKILL.md | 26 +++++++++++++++++- plan-ceo-review/SKILL.md.tmpl | 18 +++++++++++- plan-design-review/SKILL.md | 26 +++++++++++++++++- plan-design-review/SKILL.md.tmpl | 18 +++++++++++- plan-eng-review/SKILL.md | 28 ++++++++++++++++++- plan-eng-review/SKILL.md.tmpl | 20 +++++++++++++- review/SKILL.md | 4 +-- scripts/gen-skill-docs.ts | 14 ++++++++-- ship/SKILL.md | 12 ++++++-- test/gen-skill-docs.test.ts | 47 ++++++++++++++++++++++++++++++++ 12 files changed, 210 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20735e5d13f999802a98ab6ab3d278ee9c6f73c5..a43363d742485e8596d2ad71cc77c32e23ded41a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## [0.8.3] - 2026-03-19 + +### Added + +- **Plan reviews now guide you to the next step.** After running `/plan-ceo-review`, `/plan-eng-review`, or `/plan-design-review`, you get a recommendation for what to run next — eng review is always suggested as the required shipping gate, design review is suggested when UI changes are detected, and CEO review is softly mentioned for big product changes. No more remembering the workflow yourself. +- **Reviews know when they're stale.** Each review now records the commit it was run at. The dashboard compares that against your current HEAD and tells you exactly how many commits have elapsed — "eng review may be stale — 13 commits since review" instead of guessing. +- **`skip_eng_review` respected everywhere.** If you've opted out of eng review globally, the chaining recommendations won't nag you about it. +- **Design review lite now tracks commits too.** The lightweight design check that runs inside `/review` and `/ship` gets the same staleness tracking as full reviews. + ## [0.8.2] - 2026-03-19 ### Added diff --git a/VERSION b/VERSION index 100435be135a32ae8974fe4dd281c4d3a9d62e02..ee94dd834b5395f973d3c7992f661d306320aec2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.8.2 +0.8.3 diff --git a/plan-ceo-review/SKILL.md b/plan-ceo-review/SKILL.md index 5101ea76db3f9518dc45687ba83170ddc4aa134f..008a4e6346b8e42d0a8519687712899043a5033c 100644 --- a/plan-ceo-review/SKILL.md +++ b/plan-ceo-review/SKILL.md @@ -794,7 +794,7 @@ After producing the Completion Summary above, persist the review result: ```bash eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) mkdir -p ~/.gstack/projects/$SLUG -echo '{"skill":"plan-ceo-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +echo '{"skill":"plan-ceo-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE","commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl ``` Before running this command, substitute the placeholder values from the Completion Summary you just produced: @@ -803,6 +803,7 @@ Before running this command, substitute the placeholder values from the Completi - **unresolved**: number from "Unresolved decisions" in the summary - **critical_gaps**: number from "Failure modes: ___ CRITICAL GAPS" in the summary - **MODE**: the mode the user selected (SCOPE_EXPANSION / SELECTIVE_EXPANSION / HOLD_SCOPE / SCOPE_REDUCTION) +- **COMMIT**: output of `git rev-parse --short HEAD` ## Review Readiness Dashboard @@ -813,6 +814,8 @@ eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) cat ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl 2>/dev/null || echo "NO_REVIEWS" echo "---CONFIG---" ~/.claude/skills/gstack/bin/gstack-config get skip_eng_review 2>/dev/null || echo "false" +echo "---HEAD---" +git rev-parse --short HEAD 2>/dev/null || echo "unknown" ``` Parse the output. Find the most recent entry for each skill (plan-ceo-review, plan-eng-review, plan-design-review, design-review-lite, codex-review). Ignore entries with timestamps older than 7 days. For Design Review, show whichever is more recent between `plan-design-review` (full visual audit) and `design-review-lite` (code-level check). Append "(FULL)" or "(LITE)" to the status to distinguish. Display: @@ -844,6 +847,27 @@ Parse the output. Find the most recent entry for each skill (plan-ceo-review, pl - CEO, Design, and Codex reviews are shown for context but never block shipping - If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED +**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale: +- Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash +- For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review" +- For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection" +- If all reviews match the current HEAD, do not display any staleness notes + +## Next Steps — Review Chaining + +After displaying the Review Readiness Dashboard, recommend the next review(s) based on what this CEO review discovered. Read the dashboard output to see which reviews have already been run and whether they are stale. + +**Recommend /plan-eng-review if eng review is not skipped globally** — check the dashboard output for `skip_eng_review`. If it is `true`, eng review is opted out — do not recommend it. Otherwise, eng review is the required shipping gate. If this CEO review expanded scope, changed architectural direction, or accepted scope expansions, emphasize that a fresh eng review is needed. If an eng review already exists in the dashboard but the commit hash shows it predates this CEO review, note that it may be stale and should be re-run. + +**Recommend /plan-design-review if UI scope was detected** — specifically if Section 11 (Design & UX Review) was NOT skipped, or if accepted scope expansions included UI-facing features. If an existing design review is stale (commit hash drift), note that. In SCOPE REDUCTION mode, skip this recommendation — design review is unlikely relevant for scope cuts. + +**If both are needed, recommend eng review first** (required gate), then design review. + +Use AskUserQuestion to present the next step. Include only applicable options: +- **A)** Run /plan-eng-review next (required gate) +- **B)** Run /plan-design-review next (only if UI scope detected) +- **C)** Skip — I'll handle reviews manually + ## docs/designs Promotion (EXPANSION and SELECTIVE EXPANSION only) At the end of the review, if the vision produced a compelling feature direction, offer to promote the CEO plan to the project repo. AskUserQuestion: diff --git a/plan-ceo-review/SKILL.md.tmpl b/plan-ceo-review/SKILL.md.tmpl index 09189af5f553ad09fe6af0a4f113681cbb148403..4f927880aeaa7c360f8abad99ef124b89686eaca 100644 --- a/plan-ceo-review/SKILL.md.tmpl +++ b/plan-ceo-review/SKILL.md.tmpl @@ -641,7 +641,7 @@ After producing the Completion Summary above, persist the review result: ```bash eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) mkdir -p ~/.gstack/projects/$SLUG -echo '{"skill":"plan-ceo-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +echo '{"skill":"plan-ceo-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE","commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl ``` Before running this command, substitute the placeholder values from the Completion Summary you just produced: @@ -650,9 +650,25 @@ Before running this command, substitute the placeholder values from the Completi - **unresolved**: number from "Unresolved decisions" in the summary - **critical_gaps**: number from "Failure modes: ___ CRITICAL GAPS" in the summary - **MODE**: the mode the user selected (SCOPE_EXPANSION / SELECTIVE_EXPANSION / HOLD_SCOPE / SCOPE_REDUCTION) +- **COMMIT**: output of `git rev-parse --short HEAD` {{REVIEW_DASHBOARD}} +## Next Steps — Review Chaining + +After displaying the Review Readiness Dashboard, recommend the next review(s) based on what this CEO review discovered. Read the dashboard output to see which reviews have already been run and whether they are stale. + +**Recommend /plan-eng-review if eng review is not skipped globally** — check the dashboard output for `skip_eng_review`. If it is `true`, eng review is opted out — do not recommend it. Otherwise, eng review is the required shipping gate. If this CEO review expanded scope, changed architectural direction, or accepted scope expansions, emphasize that a fresh eng review is needed. If an eng review already exists in the dashboard but the commit hash shows it predates this CEO review, note that it may be stale and should be re-run. + +**Recommend /plan-design-review if UI scope was detected** — specifically if Section 11 (Design & UX Review) was NOT skipped, or if accepted scope expansions included UI-facing features. If an existing design review is stale (commit hash drift), note that. In SCOPE REDUCTION mode, skip this recommendation — design review is unlikely relevant for scope cuts. + +**If both are needed, recommend eng review first** (required gate), then design review. + +Use AskUserQuestion to present the next step. Include only applicable options: +- **A)** Run /plan-eng-review next (required gate) +- **B)** Run /plan-design-review next (only if UI scope detected) +- **C)** Skip — I'll handle reviews manually + ## docs/designs Promotion (EXPANSION and SELECTIVE EXPANSION only) At the end of the review, if the vision produced a compelling feature direction, offer to promote the CEO plan to the project repo. AskUserQuestion: diff --git a/plan-design-review/SKILL.md b/plan-design-review/SKILL.md index e0e3a8396e9e8d033276e818d7b6068b7a233d61..8b85f2dbd9b62397c04c714b82f680169ffccb9e 100644 --- a/plan-design-review/SKILL.md +++ b/plan-design-review/SKILL.md @@ -424,7 +424,7 @@ After producing the Completion Summary above, persist the review result: ```bash eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) mkdir -p ~/.gstack/projects/$SLUG -echo '{"skill":"plan-design-review","timestamp":"TIMESTAMP","status":"STATUS","overall_score":N,"unresolved":N,"decisions_made":N}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +echo '{"skill":"plan-design-review","timestamp":"TIMESTAMP","status":"STATUS","overall_score":N,"unresolved":N,"decisions_made":N,"commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl ``` Substitute values from the Completion Summary: @@ -433,6 +433,7 @@ Substitute values from the Completion Summary: - **overall_score**: final overall design score (0-10) - **unresolved**: number of unresolved design decisions - **decisions_made**: number of design decisions added to the plan +- **COMMIT**: output of `git rev-parse --short HEAD` ## Review Readiness Dashboard @@ -443,6 +444,8 @@ eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) cat ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl 2>/dev/null || echo "NO_REVIEWS" echo "---CONFIG---" ~/.claude/skills/gstack/bin/gstack-config get skip_eng_review 2>/dev/null || echo "false" +echo "---HEAD---" +git rev-parse --short HEAD 2>/dev/null || echo "unknown" ``` Parse the output. Find the most recent entry for each skill (plan-ceo-review, plan-eng-review, plan-design-review, design-review-lite, codex-review). Ignore entries with timestamps older than 7 days. For Design Review, show whichever is more recent between `plan-design-review` (full visual audit) and `design-review-lite` (code-level check). Append "(FULL)" or "(LITE)" to the status to distinguish. Display: @@ -474,6 +477,27 @@ Parse the output. Find the most recent entry for each skill (plan-ceo-review, pl - CEO, Design, and Codex reviews are shown for context but never block shipping - If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED +**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale: +- Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash +- For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review" +- For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection" +- If all reviews match the current HEAD, do not display any staleness notes + +## Next Steps — Review Chaining + +After displaying the Review Readiness Dashboard, recommend the next review(s) based on what this design review discovered. Read the dashboard output to see which reviews have already been run and whether they are stale. + +**Recommend /plan-eng-review if eng review is not skipped globally** — check the dashboard output for `skip_eng_review`. If it is `true`, eng review is opted out — do not recommend it. Otherwise, eng review is the required shipping gate. If this design review added significant interaction specifications, new user flows, or changed the information architecture, emphasize that eng review needs to validate the architectural implications. If an eng review already exists but the commit hash shows it predates this design review, note that it may be stale and should be re-run. + +**Consider recommending /plan-ceo-review** — but only if this design review revealed fundamental product direction gaps. Specifically: if the overall design score started below 4/10, if the information architecture had major structural problems, or if the review surfaced questions about whether the right problem is being solved. AND no CEO review exists in the dashboard. This is a selective recommendation — most design reviews should NOT trigger a CEO review. + +**If both are needed, recommend eng review first** (required gate). + +Use AskUserQuestion to present the next step. Include only applicable options: +- **A)** Run /plan-eng-review next (required gate) +- **B)** Run /plan-ceo-review (only if fundamental product gaps found) +- **C)** Skip — I'll handle reviews manually + ## Formatting Rules * NUMBER issues (1, 2, 3...) and LETTERS for options (A, B, C...). * Label with NUMBER + LETTER (e.g., "3A", "3B"). diff --git a/plan-design-review/SKILL.md.tmpl b/plan-design-review/SKILL.md.tmpl index 73e383b68f13fd6de9424a68b33180acd4d23d96..256666280f16dde4dc24d3d6907930e1e2caaaf3 100644 --- a/plan-design-review/SKILL.md.tmpl +++ b/plan-design-review/SKILL.md.tmpl @@ -271,7 +271,7 @@ After producing the Completion Summary above, persist the review result: ```bash eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) mkdir -p ~/.gstack/projects/$SLUG -echo '{"skill":"plan-design-review","timestamp":"TIMESTAMP","status":"STATUS","overall_score":N,"unresolved":N,"decisions_made":N}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +echo '{"skill":"plan-design-review","timestamp":"TIMESTAMP","status":"STATUS","overall_score":N,"unresolved":N,"decisions_made":N,"commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl ``` Substitute values from the Completion Summary: @@ -280,9 +280,25 @@ Substitute values from the Completion Summary: - **overall_score**: final overall design score (0-10) - **unresolved**: number of unresolved design decisions - **decisions_made**: number of design decisions added to the plan +- **COMMIT**: output of `git rev-parse --short HEAD` {{REVIEW_DASHBOARD}} +## Next Steps — Review Chaining + +After displaying the Review Readiness Dashboard, recommend the next review(s) based on what this design review discovered. Read the dashboard output to see which reviews have already been run and whether they are stale. + +**Recommend /plan-eng-review if eng review is not skipped globally** — check the dashboard output for `skip_eng_review`. If it is `true`, eng review is opted out — do not recommend it. Otherwise, eng review is the required shipping gate. If this design review added significant interaction specifications, new user flows, or changed the information architecture, emphasize that eng review needs to validate the architectural implications. If an eng review already exists but the commit hash shows it predates this design review, note that it may be stale and should be re-run. + +**Consider recommending /plan-ceo-review** — but only if this design review revealed fundamental product direction gaps. Specifically: if the overall design score started below 4/10, if the information architecture had major structural problems, or if the review surfaced questions about whether the right problem is being solved. AND no CEO review exists in the dashboard. This is a selective recommendation — most design reviews should NOT trigger a CEO review. + +**If both are needed, recommend eng review first** (required gate). + +Use AskUserQuestion to present the next step. Include only applicable options: +- **A)** Run /plan-eng-review next (required gate) +- **B)** Run /plan-ceo-review (only if fundamental product gaps found) +- **C)** Skip — I'll handle reviews manually + ## Formatting Rules * NUMBER issues (1, 2, 3...) and LETTERS for options (A, B, C...). * Label with NUMBER + LETTER (e.g., "3A", "3B"). diff --git a/plan-eng-review/SKILL.md b/plan-eng-review/SKILL.md index a98fa4f2053cc13ba359d16ed73d87ad5a56178a..bbab36d106fcf3e7c5389c00141b6ccb0edcadc3 100644 --- a/plan-eng-review/SKILL.md +++ b/plan-eng-review/SKILL.md @@ -395,7 +395,7 @@ After producing the Completion Summary above, persist the review result: ```bash eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) mkdir -p ~/.gstack/projects/$SLUG -echo '{"skill":"plan-eng-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +echo '{"skill":"plan-eng-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE","commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl ``` Substitute values from the Completion Summary: @@ -404,6 +404,7 @@ Substitute values from the Completion Summary: - **unresolved**: number from "Unresolved decisions" count - **critical_gaps**: number from "Failure modes: ___ critical gaps flagged" - **MODE**: FULL_REVIEW / SCOPE_REDUCED +- **COMMIT**: output of `git rev-parse --short HEAD` ## Review Readiness Dashboard @@ -414,6 +415,8 @@ eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) cat ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl 2>/dev/null || echo "NO_REVIEWS" echo "---CONFIG---" ~/.claude/skills/gstack/bin/gstack-config get skip_eng_review 2>/dev/null || echo "false" +echo "---HEAD---" +git rev-parse --short HEAD 2>/dev/null || echo "unknown" ``` Parse the output. Find the most recent entry for each skill (plan-ceo-review, plan-eng-review, plan-design-review, design-review-lite, codex-review). Ignore entries with timestamps older than 7 days. For Design Review, show whichever is more recent between `plan-design-review` (full visual audit) and `design-review-lite` (code-level check). Append "(FULL)" or "(LITE)" to the status to distinguish. Display: @@ -445,5 +448,28 @@ Parse the output. Find the most recent entry for each skill (plan-ceo-review, pl - CEO, Design, and Codex reviews are shown for context but never block shipping - If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED +**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale: +- Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash +- For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review" +- For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection" +- If all reviews match the current HEAD, do not display any staleness notes + +## Next Steps — Review Chaining + +After displaying the Review Readiness Dashboard, check if additional reviews would be valuable. Read the dashboard output to see which reviews have already been run and whether they are stale. + +**Suggest /plan-design-review if UI changes exist and no design review has been run** — detect from the test diagram, architecture review, or any section that touched frontend components, CSS, views, or user-facing interaction flows. If an existing design review's commit hash shows it predates significant changes found in this eng review, note that it may be stale. + +**Mention /plan-ceo-review if this is a significant product change and no CEO review exists** — this is a soft suggestion, not a push. CEO review is optional. Only mention it if the plan introduces new user-facing features, changes product direction, or expands scope substantially. + +**Note staleness** of existing CEO or design reviews if this eng review found assumptions that contradict them, or if the commit hash shows significant drift. + +**If no additional reviews are needed** (or `skip_eng_review` is `true` in the dashboard config, meaning this eng review was optional): state "All relevant reviews complete. Run /ship when ready." + +Use AskUserQuestion with only the applicable options: +- **A)** Run /plan-design-review (only if UI scope detected and no design review exists) +- **B)** Run /plan-ceo-review (only if significant product change and no CEO review exists) +- **C)** Ready to implement — run /ship when done + ## Unresolved decisions If the user does not respond to an AskUserQuestion or interrupts to move on, note which decisions were left unresolved. At the end of the review, list these as "Unresolved decisions that may bite you later" — never silently default to an option. diff --git a/plan-eng-review/SKILL.md.tmpl b/plan-eng-review/SKILL.md.tmpl index e7120f60656e288b3face68ab5a66a445c090570..a864324e5a42081b2a6bf1ef5d1756dd6a31a698 100644 --- a/plan-eng-review/SKILL.md.tmpl +++ b/plan-eng-review/SKILL.md.tmpl @@ -259,7 +259,7 @@ After producing the Completion Summary above, persist the review result: ```bash eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) mkdir -p ~/.gstack/projects/$SLUG -echo '{"skill":"plan-eng-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +echo '{"skill":"plan-eng-review","timestamp":"TIMESTAMP","status":"STATUS","unresolved":N,"critical_gaps":N,"mode":"MODE","commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl ``` Substitute values from the Completion Summary: @@ -268,8 +268,26 @@ Substitute values from the Completion Summary: - **unresolved**: number from "Unresolved decisions" count - **critical_gaps**: number from "Failure modes: ___ critical gaps flagged" - **MODE**: FULL_REVIEW / SCOPE_REDUCED +- **COMMIT**: output of `git rev-parse --short HEAD` {{REVIEW_DASHBOARD}} +## Next Steps — Review Chaining + +After displaying the Review Readiness Dashboard, check if additional reviews would be valuable. Read the dashboard output to see which reviews have already been run and whether they are stale. + +**Suggest /plan-design-review if UI changes exist and no design review has been run** — detect from the test diagram, architecture review, or any section that touched frontend components, CSS, views, or user-facing interaction flows. If an existing design review's commit hash shows it predates significant changes found in this eng review, note that it may be stale. + +**Mention /plan-ceo-review if this is a significant product change and no CEO review exists** — this is a soft suggestion, not a push. CEO review is optional. Only mention it if the plan introduces new user-facing features, changes product direction, or expands scope substantially. + +**Note staleness** of existing CEO or design reviews if this eng review found assumptions that contradict them, or if the commit hash shows significant drift. + +**If no additional reviews are needed** (or `skip_eng_review` is `true` in the dashboard config, meaning this eng review was optional): state "All relevant reviews complete. Run /ship when ready." + +Use AskUserQuestion with only the applicable options: +- **A)** Run /plan-design-review (only if UI scope detected and no design review exists) +- **B)** Run /plan-ceo-review (only if significant product change and no CEO review exists) +- **C)** Ready to implement — run /ship when done + ## Unresolved decisions If the user does not respond to an AskUserQuestion or interrupts to move on, note which decisions were left unresolved. At the end of the review, list these as "Unresolved decisions that may bite you later" — never silently default to an option. diff --git a/review/SKILL.md b/review/SKILL.md index 557259f179d32dd1bf9395c16413f87aa4ce710d..b93ea79c2f275b4fb36546fda60fd5ca35447d91 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -296,10 +296,10 @@ eval $(~/.claude/skills/gstack/bin/gstack-diff-scope 2>/dev/null) ```bash eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) mkdir -p ~/.gstack/projects/$SLUG -echo '{"skill":"design-review-lite","timestamp":"TIMESTAMP","status":"STATUS","findings":N,"auto_fixed":M}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +echo '{"skill":"design-review-lite","timestamp":"TIMESTAMP","status":"STATUS","findings":N,"auto_fixed":M,"commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl ``` -Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "issues_found", N = total findings, M = auto-fixed count. +Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "issues_found", N = total findings, M = auto-fixed count, COMMIT = output of `git rev-parse --short HEAD`. Include any design findings alongside the findings from Step 4. They follow the same Fix-First flow in Step 5 — AUTO-FIX for mechanical CSS fixes, ASK for everything else. diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index 9f5460a356186645ac225293fa5e8d120f180e99..08136b49b4d164efe24230461457a9080a60b2a1 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -592,10 +592,10 @@ eval $(~/.claude/skills/gstack/bin/gstack-diff-scope 2>/dev/null) \`\`\`bash eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) mkdir -p ~/.gstack/projects/$SLUG -echo '{"skill":"design-review-lite","timestamp":"TIMESTAMP","status":"STATUS","findings":N,"auto_fixed":M}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +echo '{"skill":"design-review-lite","timestamp":"TIMESTAMP","status":"STATUS","findings":N,"auto_fixed":M,"commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl \`\`\` -Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "issues_found", N = total findings, M = auto-fixed count.`; +Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "issues_found", N = total findings, M = auto-fixed count, COMMIT = output of \`git rev-parse --short HEAD\`.`; } // NOTE: design-checklist.md is a subset of this methodology for code-level detection. @@ -944,6 +944,8 @@ eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) cat ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl 2>/dev/null || echo "NO_REVIEWS" echo "---CONFIG---" ~/.claude/skills/gstack/bin/gstack-config get skip_eng_review 2>/dev/null || echo "false" +echo "---HEAD---" +git rev-parse --short HEAD 2>/dev/null || echo "unknown" \`\`\` Parse the output. Find the most recent entry for each skill (plan-ceo-review, plan-eng-review, plan-design-review, design-review-lite, codex-review). Ignore entries with timestamps older than 7 days. For Design Review, show whichever is more recent between \`plan-design-review\` (full visual audit) and \`design-review-lite\` (code-level check). Append "(FULL)" or "(LITE)" to the status to distinguish. Display: @@ -973,7 +975,13 @@ Parse the output. Find the most recent entry for each skill (plan-ceo-review, pl - **CLEARED**: Eng Review has >= 1 entry within 7 days with status "clean" (or \\\`skip_eng_review\\\` is \\\`true\\\`) - **NOT CLEARED**: Eng Review missing, stale (>7 days), or has open issues - CEO, Design, and Codex reviews are shown for context but never block shipping -- If \\\`skip_eng_review\\\` config is \\\`true\\\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED`; +- If \\\`skip_eng_review\\\` config is \\\`true\\\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED + +**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale: +- Parse the \\\`---HEAD---\\\` section from the bash output to get the current HEAD commit hash +- For each review entry that has a \\\`commit\\\` field: compare it against the current HEAD. If different, count elapsed commits: \\\`git rev-list --count STORED_COMMIT..HEAD\\\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review" +- For entries without a \\\`commit\\\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection" +- If all reviews match the current HEAD, do not display any staleness notes`; } function generateTestBootstrap(_ctx: TemplateContext): string { diff --git a/ship/SKILL.md b/ship/SKILL.md index 97c8e79e00ac529033f072a52f1b78db262f2678..7bc9d75844d24ce2e466693af451d359a07909e8 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -217,6 +217,8 @@ eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) cat ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl 2>/dev/null || echo "NO_REVIEWS" echo "---CONFIG---" ~/.claude/skills/gstack/bin/gstack-config get skip_eng_review 2>/dev/null || echo "false" +echo "---HEAD---" +git rev-parse --short HEAD 2>/dev/null || echo "unknown" ``` Parse the output. Find the most recent entry for each skill (plan-ceo-review, plan-eng-review, plan-design-review, design-review-lite, codex-review). Ignore entries with timestamps older than 7 days. For Design Review, show whichever is more recent between `plan-design-review` (full visual audit) and `design-review-lite` (code-level check). Append "(FULL)" or "(LITE)" to the status to distinguish. Display: @@ -248,6 +250,12 @@ Parse the output. Find the most recent entry for each skill (plan-ceo-review, pl - CEO, Design, and Codex reviews are shown for context but never block shipping - If \`skip_eng_review\` config is \`true\`, Eng Review shows "SKIPPED (global)" and verdict is CLEARED +**Staleness detection:** After displaying the dashboard, check if any existing reviews may be stale: +- Parse the \`---HEAD---\` section from the bash output to get the current HEAD commit hash +- For each review entry that has a \`commit\` field: compare it against the current HEAD. If different, count elapsed commits: \`git rev-list --count STORED_COMMIT..HEAD\`. Display: "Note: {skill} review from {date} may be stale — {N} commits since review" +- For entries without a \`commit\` field (legacy entries): display "Note: {skill} review from {date} has no commit tracking — consider re-running for accurate staleness detection" +- If all reviews match the current HEAD, do not display any staleness notes + If the Eng Review is NOT "CLEAR": 1. **Check for a prior override on this branch:** @@ -708,10 +716,10 @@ eval $(~/.claude/skills/gstack/bin/gstack-diff-scope 2>/dev/null) ```bash eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null) mkdir -p ~/.gstack/projects/$SLUG -echo '{"skill":"design-review-lite","timestamp":"TIMESTAMP","status":"STATUS","findings":N,"auto_fixed":M}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +echo '{"skill":"design-review-lite","timestamp":"TIMESTAMP","status":"STATUS","findings":N,"auto_fixed":M,"commit":"COMMIT"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl ``` -Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "issues_found", N = total findings, M = auto-fixed count. +Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "issues_found", N = total findings, M = auto-fixed count, COMMIT = output of `git rev-parse --short HEAD`. Include any design findings alongside the code review findings. They follow the same Fix-First flow below. diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index b53ebc173d18235f108648b0abe8b9ebd81e48ad..7396933c9d010ab6e4f9103b19fbf7a5a61c4529 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -374,4 +374,51 @@ describe('REVIEW_DASHBOARD resolver', () => { expect(content).toContain('Design Review'); expect(content).toContain('skip_eng_review'); }); + + test('dashboard bash block includes git HEAD for staleness detection', () => { + const content = fs.readFileSync(path.join(ROOT, 'plan-ceo-review', 'SKILL.md'), 'utf-8'); + expect(content).toContain('git rev-parse --short HEAD'); + expect(content).toContain('---HEAD---'); + }); + + test('dashboard includes staleness detection prose', () => { + const content = fs.readFileSync(path.join(ROOT, 'plan-ceo-review', 'SKILL.md'), 'utf-8'); + expect(content).toContain('Staleness detection'); + expect(content).toContain('commit'); + }); + + for (const skill of REVIEW_SKILLS) { + test(`${skill} contains review chaining section`, () => { + const content = fs.readFileSync(path.join(ROOT, skill, 'SKILL.md'), 'utf-8'); + expect(content).toContain('Review Chaining'); + }); + + test(`${skill} Review Log includes commit field`, () => { + const content = fs.readFileSync(path.join(ROOT, skill, 'SKILL.md'), 'utf-8'); + expect(content).toContain('"commit"'); + }); + } + + test('plan-ceo-review chaining mentions eng and design reviews', () => { + const content = fs.readFileSync(path.join(ROOT, 'plan-ceo-review', 'SKILL.md'), 'utf-8'); + expect(content).toContain('/plan-eng-review'); + expect(content).toContain('/plan-design-review'); + }); + + test('plan-eng-review chaining mentions design and ceo reviews', () => { + const content = fs.readFileSync(path.join(ROOT, 'plan-eng-review', 'SKILL.md'), 'utf-8'); + expect(content).toContain('/plan-design-review'); + expect(content).toContain('/plan-ceo-review'); + }); + + test('plan-design-review chaining mentions eng and ceo reviews', () => { + const content = fs.readFileSync(path.join(ROOT, 'plan-design-review', 'SKILL.md'), 'utf-8'); + expect(content).toContain('/plan-eng-review'); + expect(content).toContain('/plan-ceo-review'); + }); + + test('ship does NOT contain review chaining', () => { + const content = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8'); + expect(content).not.toContain('Review Chaining'); + }); });