From 3742ee9527615b0fa325402d6c02498cc7fc19fa Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 6 Nov 2026 20:17:35 -0800 Subject: [PATCH] feat: cut standalone /plan-eng-review; /autoplan keeps its eng phase (GST-1698) (#4108) * feat: cut standalone /plan-eng-review; /autoplan keeps its eng phase (GST-1698) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: lower shrunk SKILL.md size pins; keep the retired methodology's pins measured beside /autoplan Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: carry /autoplan's plan-eng-review.md into Kiro installs Kiro's per-skill loop copied only SKILL.md and sections, so ~/.kiro/skills/gstack-autoplan lacked the Phase 3 methodology file its SKILL.md reads. Generated sibling *.md files now travel with SKILL.md under the same ownership guards. Review-Finding: p1/specialist-batched#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: file /plan-eng-review retirement follow-ups; scope the changelog claim to ./setup Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: route architecture-review requests to /autoplan by its description (journey-plan-eng) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- AGENTS.md | 1 - ARCHITECTURE.md | 6 +- BROWSER.md | 2 +- CLAUDE.md | 2 +- README.md | 9 +- SKILL.md | 3 +- SKILL.md.tmpl | 2 +- TODOS.md | 8 + autoplan/SKILL.md | 7 +- autoplan/SKILL.md.tmpl | 17 +- autoplan/bin/guard-reasons.ts | 2 +- bin/gstack-autoplan-snapshot.ts | 2 +- bin/gstack-doctor-components.sh | 8 +- bin/gstack-install-registry.sh | 7 + bin/gstack-relink | 8 +- bin/gstack-skill-start | 2 +- .../capy-gst-1698-cut-plan-eng-review.md | 1 + codex/SKILL.md | 2 +- devex-review/SKILL.md | 2 +- docs/OPENCLAW.md | 6 +- docs/PROJECT_STRUCTURE.md | 2 +- docs/autoplan-guard-troubleshooting.md | 3 +- docs/hosts.md | 2 +- docs/skills.md | 72 ++------ docs/test-value-bar.md | 2 +- docs/troubleshooting.md | 4 +- docs/voice-input.md | 4 +- gstack/llms.txt | 3 +- lib/doctor-components.ts | 3 +- lib/harness-register.ts | 2 +- lib/ship-preflight.ts | 2 +- office-hours/SKILL.md.tmpl | 2 +- office-hours/sections/design-and-handoff.md | 12 +- .../sections/design-and-handoff.md.tmpl | 12 +- plan-ceo-review/sections/review-sections.md | 8 +- .../sections/review-sections.md.tmpl | 6 +- .../sections/review-sections.md | 8 +- .../sections/review-sections.md.tmpl | 6 +- plan-devex-review/SKILL.md | 2 +- plan-devex-review/SKILL.md.tmpl | 2 +- plan-devex-review/sections/review-sections.md | 8 +- .../sections/review-sections.md.tmpl | 6 +- qa/SKILL.md | 2 +- qa/SKILL.md.tmpl | 2 +- retro/SKILL.md | 2 +- retro/SKILL.md.tmpl | 2 +- scripts/discover-skills.ts | 10 ++ scripts/gen-llms-txt.ts | 4 +- scripts/gen-skill-docs.ts | 11 +- scripts/preflight-codex-overlap.ts | 3 +- scripts/resolvers/composition.ts | 5 + scripts/resolvers/review-dashboard.ts | 4 +- scripts/resolvers/voice-triggers.ts | 4 +- setup | 27 ++- ship/SKILL.md | 4 +- ship/SKILL.md.tmpl | 2 +- spec/SKILL.md | 3 +- spec/SKILL.md.tmpl | 3 +- test-audit/SKILL.md | 2 +- test-audit/SKILL.md.tmpl | 2 +- test/autoplan-bounded-journal.test.ts | 4 +- test/autoplan-guard-reasons.test.ts | 2 +- test/autoplan-methodology-names.test.ts | 14 +- test/autoplan-publication-hook.test.ts | 2 +- test/autoplan-publication-payload.test.ts | 2 +- test/autoplan-review-discovery.test.ts | 15 +- test/disabled-plan-review-evidence.test.ts | 6 +- test/doctor-components.test.ts | 3 +- test/eng-review-routing.test.ts | 2 +- test/external-host-carve.test.ts | 2 +- test/fixtures/golden/agy-ship-SKILL.md | 4 +- test/fixtures/golden/claude-ship-SKILL.md | 4 +- test/fixtures/golden/codex-ship-SKILL.md | 4 +- test/fixtures/golden/factory-ship-SKILL.md | 4 +- test/fixtures/skill-md-size-allowlist.json | 60 +++---- test/gen-skill-docs-checks.test.ts | 2 +- test/gen-skill-docs.test.ts | 15 +- .../helpers/autoplan-preconfigured-fixture.ts | 2 +- test/helpers/plan-review-cases.ts | 3 +- test/outside-voice-routing.test.ts | 3 +- test/plan-review-cases.test.ts | 6 +- test/retired-skills.test.ts | 156 ++++++++++++++++++ test/setup-kiro-native.test.ts | 2 + test/shared-libs-rendering.test.ts | 3 +- .../capy-gst-1698-cut-plan-eng-review.json | 4 + test/skill-md-size-guard.test.ts | 6 + test/skill-routing-e2e.test.ts | 4 +- test/skill-validation.test.ts | 3 + 88 files changed, 442 insertions(+), 255 deletions(-) create mode 100644 changelog.d/capy-gst-1698-cut-plan-eng-review.md create mode 100644 test/retired-skills.test.ts create mode 100644 test/size-ledger/ship/capy-gst-1698-cut-plan-eng-review.json diff --git a/AGENTS.md b/AGENTS.md index a572ea733..7a9f75c16 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,7 +16,6 @@ CLIs (`gstack-ios-qa-daemon`, `gstack-ios-qa-mint`, `gstack-ios-qa-regen`) are i |-------|------| | `/office-hours` | plan | | `/plan-ceo-review` | plan | -| `/plan-eng-review` | plan | | `/plan-design-review` | plan | | `/plan-devex-review` | plan | | `/plan-tune` | plan | diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 7dc9c3546..14dea0a77 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -345,12 +345,12 @@ Templates contain the workflows, tips, and examples that require human judgment. | `{{QA_METHODOLOGY}}` | `resolvers/utility.ts` | Browser-only QA methodology, conditionally loaded by /qa and /qa-only | | `{{QA_SCOPE}}, {{QA_EXPLORATORY}}, {{QA_FUNCTIONAL}}, {{QA_RESOURCE}}, {{QA_METHOD_READS}}, {{QA_REVIEW}}` | `resolvers/qa.ts` | Surface selection, checkpointed native/exploratory QA, direct conditional method reads, installed-asset references and bounded review/ship callers | | `{{DESIGN_METHODOLOGY}}` | `gen-skill-docs.ts` | Shared design audit methodology for /plan-design-review and /design-review | -| `{{SHARED_LIBS_RUBRIC}}` | `resolvers/shared-libs.ts` | Shared-code criteria for /deslop-shared-libs, /plan-eng-review, and /review: verified callers, existing helpers, compatibility, tests, and total savings | +| `{{SHARED_LIBS_RUBRIC}}` | `resolvers/shared-libs.ts` | Shared-code criteria for /deslop-shared-libs, /autoplan's eng phase, and /review: verified callers, existing helpers, compatibility, tests, and total savings | | `{{REVIEW_DASHBOARD}}` | `gen-skill-docs.ts` | Review Readiness Dashboard for /ship pre-flight | -| `{{TEST_VALUE_BAR:}}` | `resolvers/test-value.ts` | Shared test value bar (authoring gate, value card, X/Y coverage, red-first proof, low-value catalog) for /qa and /qa-only (`qa`) and /test-audit (`audit`); /plan-eng-review and /ship embed it through the coverage audit | +| `{{TEST_VALUE_BAR:}}` | `resolvers/test-value.ts` | Shared test value bar (authoring gate, value card, X/Y coverage, red-first proof, low-value catalog) for /qa and /qa-only (`qa`) and /test-audit (`audit`); /autoplan's eng phase and /ship embed it through the coverage audit | | `{{TEST_VALUE_MESSAGE:}}` | `resolvers/test-value.ts` | One degraded-mode message (problem, consequence, fix, docs anchor) from the shared constants | | `{{TEST_BOOTSTRAP}}` | `resolvers/testing.ts` | Test framework detection, bootstrap, CI/CD setup for /ship and /design-review | -| `{{CODEX_PLAN_REVIEW}}` | `resolvers/outside-voice-steps.ts` | Optional outside plan review for /plan-ceo-review and /plan-eng-review: Claude Code on Codex, Codex on other supported harnesses, with the caller's native subagent fallback | +| `{{CODEX_PLAN_REVIEW}}` | `resolvers/outside-voice-steps.ts` | Optional outside plan review for /plan-ceo-review and /autoplan's eng phase: Claude Code on Codex, Codex on other supported harnesses, with the caller's native subagent fallback | | `{{DESIGN_SETUP}}` | `resolvers/design.ts` | Discovery pattern for `$D` design binary, mirrors `{{BROWSE_SETUP}}` | | `{{DESIGN_DETECTOR}}` | `resolvers/design.ts` | Probe block + sentinel reading for the user-installed impeccable engine (`bin/gstack-design-detect.ts`); `:phase0` renders design-review's mechanical scan, `:gate` design-html's bounded slop gate | | `{{DESIGN_MD_CHECK}}` | `resolvers/design.ts` | Open DESIGN.md format check through `bin/gstack-design-md.ts`, with the one-time conversion offer persisted in the file; `:calibrate` renders the tokens-as-calibration form for /design-review | diff --git a/BROWSER.md b/BROWSER.md index ce0ab80bb..adbd71f8b 100644 --- a/BROWSER.md +++ b/BROWSER.md @@ -99,7 +99,7 @@ through CDP before `goto`, as the cookbook does), and no `file://` navigation. | `/scrape` | Prototype the extraction with `aside repl`, hand back the table, list, or prices as structured data. Read-only. | | `/devex-review` | Walk the real onboarding flow and time it, carrying the cookbook inline. | | `/ship`, `/spec`, `/land-and-deploy`, `/setup-deploy`, `/office-hours` | Third-party web actions (vendor dashboards, API keys, webhooks) offered as an Aside drive across your real sessions, with the one-question consent gate for anything mutating. | -| `/plan-ceo-review`, `/plan-eng-review`, `/plan-devex-review`, `/design-consultation`, `/review`, `/cso`, `/office-hours` | Web research runs through `aside exec` in your real browser (`{{ASIDE_RESEARCH}}`), one read-only request per question, answers treated as untrusted content. No Aside → the same queries go to the host's WebSearch tool; no WebSearch either → "Search unavailable" once and the skill proceeds on in-distribution knowledge. | +| `/plan-ceo-review`, `/plan-devex-review`, `/design-consultation`, `/review`, `/cso`, `/office-hours` | Web research runs through `aside exec` in your real browser (`{{ASIDE_RESEARCH}}`), one read-only request per question, answers treated as untrusted content. No Aside → the same queries go to the host's WebSearch tool; no WebSearch either → "Search unavailable" once and the skill proceeds on in-distribution knowledge. | ### Local-HTML rendering diff --git a/CLAUDE.md b/CLAUDE.md index 37fb8db4d..dc9249489 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -547,7 +547,7 @@ When the user's request matches an available skill, invoke it via the Skill tool Key routing rules: - Product ideas/brainstorming → invoke /office-hours - Strategy/scope → invoke /plan-ceo-review -- Architecture → invoke /plan-eng-review +- Architecture → invoke /autoplan - Design system/plan review → invoke /design-consultation or /plan-design-review - Full review pipeline → invoke /autoplan - Bugs/errors → invoke /investigate, unless one read-only check confirms the cause and a narrow fix (then fix it and run the focused test) diff --git a/README.md b/README.md index af8d2d013..d50b3570a 100644 --- a/README.md +++ b/README.md @@ -111,8 +111,8 @@ Claude: I'm going to push back on the framing. You said "daily briefing You: /plan-ceo-review [reads the design doc, challenges scope, runs 10-section review] -You: /plan-eng-review - [ASCII diagrams for data flow, state machines, error paths] +You: /autoplan + [eng phase: ASCII diagrams for data flow, state machines, error paths] [test matrix, failure modes, security concerns] You: Approve plan. Exit plan mode. @@ -136,13 +136,12 @@ gstack is a process, not a collection of tools. The skills run in the order a sp **Think → Plan → Build → Review → Test → Ship → Reflect** -Each skill feeds into the next. `/office-hours` writes a design doc that `/plan-ceo-review` reads. `/plan-eng-review` writes a test plan that `/qa` picks up. `/review` catches bugs that `/ship` verifies are fixed. Nothing falls through the cracks because every step knows what came before it. +Each skill feeds into the next. `/office-hours` writes a design doc that `/plan-ceo-review` reads. `/autoplan`'s eng phase writes a test plan that `/qa` picks up. `/review` catches bugs that `/ship` verifies are fixed. Nothing falls through the cracks because every step knows what came before it. | Skill | Your specialist | What they do | |-------|----------------|--------------| | `/office-hours` | **YC Office Hours** | Start here. Six forcing questions that reframe your product before you write code. Pushes back on your framing, challenges premises, generates implementation alternatives. Design doc feeds into every downstream skill. | | `/plan-ceo-review` | **CEO / Founder** | Rethink the problem. Find the 10-star product hiding inside the request. Four modes: Expansion, Selective Expansion, Hold Scope, Reduction. | -| `/plan-eng-review` | **Eng Manager** | Lock in architecture, data flow, diagrams, edge cases, and tests. Forces hidden assumptions into the open. | | `/plan-design-review` | **Senior Designer** | Rates each design dimension 0-10, explains what a 10 looks like, then edits the plan to get there. AI Slop detection. Interactive — one AskUserQuestion per design choice. | | `/plan-devex-review` | **Developer Experience Lead** | Interactive DX review: explores developer personas, benchmarks against competitors' TTHW, designs your magical moment, traces friction points step by step. Scores error messages for the agents that read them ([agent-ready errors](docs/agent-ready-errors.md)). Three modes: DX EXPANSION, DX POLISH, DX TRIAGE. 20-45 forcing questions. | | `/design-consultation` | **Design Partner** | Build a complete design system from scratch. Researches the landscape, proposes creative risks, generates realistic product mockups. Writes `DESIGN.md` in the open DESIGN.md format, so impeccable, Google Stitch, and any tool that reads it share one file. | @@ -189,7 +188,7 @@ The same `/qa` and `/qa-only` commands test CLIs, APIs, jobs, workers and webhoo |-----------------|--------------------------|----------------------------| | **End users** (UI, web app, mobile) | `/plan-design-review` | `/design-review` | | **Developers** (API, CLI, SDK, docs) | `/plan-devex-review` | `/devex-review` | -| **Architecture** (data flow, perf, tests) | `/plan-eng-review` | `/review` | +| **Architecture** (data flow, perf, tests) | `/autoplan` (eng phase) | `/review` | | **All of the above** | `/autoplan` (runs CEO → design → DX → eng, auto-detects which apply; eng always last) | — | ### Power tools diff --git a/SKILL.md b/SKILL.md index 19892de70..dbc31bf86 100644 --- a/SKILL.md +++ b/SKILL.md @@ -194,7 +194,7 @@ quality gates that produce better results than answering inline. - User describes a new idea, asks "is this worth building", brainstorms, pitches a concept → invoke `/office-hours` - User asks to spec something out, file an issue, write up a ticket, "turn this into a GitHub issue", "backlog item" → invoke `/spec` - User asks about strategy, scope, ambition, "think bigger", "what should we build" → invoke `/plan-ceo-review` -- User asks to review architecture, lock in the plan, "does this design make sense" → invoke `/plan-eng-review` +- User asks to review architecture, lock in the plan, "does this design make sense" → invoke `/autoplan` - User asks about design system, brand, visual identity, "how should this look" → invoke `/design-consultation` - User asks to review design of a plan → invoke `/plan-design-review` - User asks about developer experience of a plan, API/CLI/SDK design → invoke `/plan-devex-review` @@ -256,7 +256,6 @@ Generic “second opinion”, “outside review”, or “cross-model review” - Voice trigger "show me the browser" → invoke `/open-gstack-browser` - Voice trigger "pair agent", "connect agent", "share my browser", "remote browser access" → invoke `/pair-agent` - Voice trigger "dx review", "developer experience review", "devex review", "devex audit", "API design review", "onboarding review" → invoke `/plan-devex-review` -- Voice trigger "tech review", "technical review", "plan engineering review" → invoke `/plan-eng-review` - Voice trigger "quality check", "test the app", "run QA" → invoke `/qa` - Voice trigger "bug report", "just check for bugs" → invoke `/qa-only` diff --git a/SKILL.md.tmpl b/SKILL.md.tmpl index 8e8a15302..f354a83be 100644 --- a/SKILL.md.tmpl +++ b/SKILL.md.tmpl @@ -70,7 +70,7 @@ quality gates that produce better results than answering inline. - User describes a new idea, asks "is this worth building", brainstorms, pitches a concept → invoke `/office-hours` - User asks to spec something out, file an issue, write up a ticket, "turn this into a GitHub issue", "backlog item" → invoke `/spec` - User asks about strategy, scope, ambition, "think bigger", "what should we build" → invoke `/plan-ceo-review` -- User asks to review architecture, lock in the plan, "does this design make sense" → invoke `/plan-eng-review` +- User asks to review architecture, lock in the plan, "does this design make sense" → invoke `/autoplan` - User asks about design system, brand, visual identity, "how should this look" → invoke `/design-consultation` - User asks to review design of a plan → invoke `/plan-design-review` - User asks about developer experience of a plan, API/CLI/SDK design → invoke `/plan-devex-review` diff --git a/TODOS.md b/TODOS.md index 0962de65e..4ce5715c8 100644 --- a/TODOS.md +++ b/TODOS.md @@ -2,6 +2,14 @@ ## NEXT PRIORITY +### P2: `/plan-eng-review` retirement follow-ups (filed 2026-11-06, GST-1698 /ship review of #4108) + +- **Stop the dots and generic harness targets from registering the retired skill.** `sourceSkills()` in `lib/harness-register.ts` lists every committed Claude-render skill and filters out only `gstack`, so `gstack-harness register --target dots|generic` still lists `plan-eng-review` in its router and gives it a folder in `all` mode. Filter `RETIRED_SKILLS` there (the Claude-render /autoplan reads Phase 3 from `~/.claude/skills/gstack/plan-eng-review/SKILL.md`, not from the harness payload) and add a case to `test/retired-skills.test.ts`. Raised by the outside voice (labelled blocking) and the native adversarial pass (non-blocking); below the blocking floor because nothing fails and the skill still works. **Effort:** S. **Priority:** P2. +- **Point the external-host methodology file's preamble Read at `gstack-autoplan`.** Every external host's `gstack-autoplan/plan-eng-review.md` says to Read `sections/preamble.md` relative to the installed `gstack-plan-eng-review` directory, which no longer exists. Normally the matching preamble id skips the reread, but after compaction late in a run the agent follows a missing path. Fixing it changes Phase 3 bytes, so it needs its own byte-pin update. **Effort:** S. **Priority:** P3. +- **Redirect `gstack-doctor --for plan-eng-review`.** It now exits 2 with "unknown skill"; map the retired name to `/autoplan`'s requirements and print a one-line redirect, so saved lane scripts keep working. **Effort:** S. **Priority:** P3. +- **Decide whether the QM edition keeps `gstack-plan`.** `hosts/qm.ts` and `qm/edition.json` still build `gstack-plan` from the `plan-eng-review` sources under another name, so the retired-name guards don't see it. Product call for Garry. **Effort:** S. **Priority:** P3. +- **Pin the retirement in two more tests.** Assert that `generateLlmsTxt` output lists no `plan-eng-review` entry (`test/llms-txt-shape.test.ts`), and that `HYBRID_SKILLS` holds none of `RETIRED_SKILLS` (`test/harness-register.test.ts`). **Effort:** S. **Priority:** P3. + ### P3: clarity-judge removal follow-ups (filed 2026-11-04, GST-1371) - **Delete the four orphaned judge-calibration corpora.** `test/fixtures/judge-calibration/{qa-workflow,qa-health-rubric,workflow-default,cookie-workflow}/` serve no configuration since the skill-prose clarity judges were removed; deleting them in the removal PR put its diff over the /ship review ceiling. Delete them alone once main is unfrozen (or after `review_exclude_paths` lands), and drop their rows from the README's results tables if the record is no longer wanted. **Effort:** S. **Priority:** P3. diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index 5413fe13a..50437330f 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -2,7 +2,7 @@ name: autoplan preamble-tier: 3 version: 1.0.0 -description: Auto-review pipeline — reads the full CEO, design, eng, and DX review skills from disk and runs them sequentially with auto-decisions using 6 decision principles. (gstack) +description: Auto-review pipeline — runs the CEO, design, eng (architecture, edge cases, failure modes, tests) and DX reviews in order, auto-deciding by 6 principles. (gstack) triggers: - run all reviews - automatic review pipeline @@ -472,7 +472,7 @@ Read skills/sections only at their triggers, never prefetch future phases. Load the tasks aggregator at Phase 4. Run every applicable skill and lazy section fully. **Section skip list — when following a loaded skill file, SKIP these sections -(they are already handled by /autoplan):** +(/autoplan handles them):** - Preamble (run first) - Scope gate (the plan under review is already the target) - AskUserQuestion Format @@ -482,7 +482,8 @@ the tasks aggregator at Phase 4. Run every applicable skill and lazy section ful - Step 0: Detect platform and base branch - Review Readiness Dashboard - Plan File Review Report -- Prerequisite Skill Offer (BENEFITS_FROM) +- Prerequisite Skill Offer +- Next Steps — Review Chaining - Outside Voice — Independent Plan Challenge - Design Outside Voices (independent) diff --git a/autoplan/SKILL.md.tmpl b/autoplan/SKILL.md.tmpl index 6fa40c659..d6047855e 100644 --- a/autoplan/SKILL.md.tmpl +++ b/autoplan/SKILL.md.tmpl @@ -3,12 +3,12 @@ name: autoplan preamble-tier: 3 version: 1.0.0 description: | - Auto-review pipeline — reads the full CEO, design, eng, and DX review skills from disk - and runs them sequentially with auto-decisions using 6 decision principles. Surfaces - taste decisions (close approaches, borderline scope, outside-review disagreements) at a final - approval gate. One command, fully reviewed plan out. - Use when asked to "auto review", "autoplan", "run all reviews", "review this plan - automatically", or "make the decisions for me". (gstack) + Auto-review pipeline — runs the CEO, design, eng (architecture, edge cases, failure + modes, tests) and DX reviews in order, auto-deciding by 6 principles. Surfaces + taste decisions (close approaches, borderline scope, outside-review disagreements) at + one approval gate. Use when asked to "auto review", "autoplan", "run all reviews", + "review this plan automatically", "review the architecture", "engineering review", or + "make the decisions for me". (gstack) codex_default_prompt: "Use $gstack-autoplan to run the CEO, design, DX and eng reviews over this plan in one pass, decide the routine questions itself, and stop at a single approval gate." voice-triggers: - "auto plan" @@ -254,7 +254,7 @@ Read skills/sections only at their triggers, never prefetch future phases. Load the tasks aggregator at Phase 4. Run every applicable skill and lazy section fully. **Section skip list — when following a loaded skill file, SKIP these sections -(they are already handled by /autoplan):** +(/autoplan handles them):** - Preamble (run first) - Scope gate (the plan under review is already the target) - AskUserQuestion Format @@ -264,7 +264,8 @@ the tasks aggregator at Phase 4. Run every applicable skill and lazy section ful - Step 0: Detect platform and base branch - Review Readiness Dashboard - Plan File Review Report -- Prerequisite Skill Offer (BENEFITS_FROM) +- Prerequisite Skill Offer +- Next Steps — Review Chaining - Outside Voice — Independent Plan Challenge - Design Outside Voices (independent) diff --git a/autoplan/bin/guard-reasons.ts b/autoplan/bin/guard-reasons.ts index ad143c2d5..fde59f000 100644 --- a/autoplan/bin/guard-reasons.ts +++ b/autoplan/bin/guard-reasons.ts @@ -16,7 +16,7 @@ export type Disposition = 'fallback' | 'corrective' | 'transient' | 'unverified' export type Detail = { phase?: string | number; keys?: string[]; cause?: string; size?: string; limit?: string }; interface Reason { disposition: Disposition; text: (d: Detail) => string } -export const FALLBACK = 'Fallback: run /plan-ceo-review, then /plan-devex-review, then /plan-eng-review by hand; ' + +export const FALLBACK = 'Fallback: run /plan-ceo-review, then /plan-devex-review by hand; ' + 'or run /context-save, start a new Claude Code session (not --resume), run /context-restore, then /autoplan .'; const PUBLISH_SEPARATELY = (d: Detail) => `Publish the filled Phase ${d.phase} report as your own parent assistant text in a message ` + `whose only tool call is the Bash no-op \`true autoplan-published \`, then make this phase-entry call in a later message.`; diff --git a/bin/gstack-autoplan-snapshot.ts b/bin/gstack-autoplan-snapshot.ts index 7cfbf958f..9ae6effa4 100644 --- a/bin/gstack-autoplan-snapshot.ts +++ b/bin/gstack-autoplan-snapshot.ts @@ -516,7 +516,7 @@ function phaseName(phase: string): string { function methodologyContent(phase: string, skillFile: string) { phaseName(phase); const skill = `plan-${phase === 'dx' ? 'devex' : phase}-review`; - if (!isAbsolute(skillFile) || basename(skillFile) !== 'SKILL.md') throw new Error('Expected an absolute installed SKILL.md path'); + if (!isAbsolute(skillFile) || (basename(skillFile) !== 'SKILL.md' && basename(skillFile) !== `${skill}.md`)) throw new Error('Expected an absolute installed SKILL.md path'); const readPart = (file: string) => { const resolved = realpathSync(file); if (!statSync(resolved).isFile()) throw new Error('Methodology source must be a regular file'); diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index 81677601c..3b8d2def1 100644 --- a/bin/gstack-doctor-components.sh +++ b/bin/gstack-doctor-components.sh @@ -8,7 +8,7 @@ GSTACK_DOCTOR_COMPONENTS="claude codex patch codex-cli browse-bundle browser cso GSTACK_DOCTOR_CORE="runtime pins state-root privacy cores revision floor wrapper" GSTACK_DOCTOR_HOSTS="claude codex" GSTACK_DOCTOR_OPTIONAL="codex-cli cso" -GSTACK_DOCTOR_SKILLS="autoplan plan-ceo-review plan-eng-review plan-design-review plan-devex-review plan-tune office-hours spec review ship investigate codex cso browse qa qa-only design-review design-consultation design-html design-motion devex-review scrape skillify canary benchmark make-pdf diagram pair-agent open-gstack-browser setup-browser-cookies connect-chrome land-and-deploy benchmark-models captain careful claude-code context-restore context-save design-shotgun deslop-shared-libs document-generate document-release eval-plan freeze gstack-upgrade guard health ios-clean ios-design-review ios-fix ios-qa ios-sync landing-report learn write-skill modernize retro runaway-stopper setup-deploy setup-gbrain sync-gbrain test-audit ubicloud unfreeze verify review-voice" +GSTACK_DOCTOR_SKILLS="autoplan plan-ceo-review plan-design-review plan-devex-review plan-tune office-hours spec review ship investigate codex cso browse qa qa-only design-review design-consultation design-html design-motion devex-review scrape skillify canary benchmark make-pdf diagram pair-agent open-gstack-browser setup-browser-cookies connect-chrome land-and-deploy benchmark-models captain careful claude-code context-restore context-save design-shotgun deslop-shared-libs document-generate document-release eval-plan freeze gstack-upgrade guard health ios-clean ios-design-review ios-fix ios-qa ios-sync landing-report learn write-skill modernize retro runaway-stopper setup-deploy setup-gbrain sync-gbrain test-audit ubicloud unfreeze verify review-voice" GSTACK_BROWSER_LAZY_REASON="lazy; gstack-browser-ensure installs on first use" GSTACK_DOCTOR_HELPERS="gstack-pregate gstack-plan-reality gstack-artifact gstack-autoplan" GSTACK_DOCTOR_NOT_NEEDED="none" @@ -65,7 +65,7 @@ gstack_doctor_component_kind() { # gstack_doctor_skill_requires SKILL — the non-core ids the skill needs; returns 1 for an unknown skill. gstack_doctor_skill_requires() { case "$1" in - autoplan|plan-ceo-review|plan-eng-review|plan-design-review|plan-devex-review|plan-tune|office-hours|spec|review|ship|investigate) printf '%s' 'claude codex patch' ;; + autoplan|plan-ceo-review|plan-design-review|plan-devex-review|plan-tune|office-hours|spec|review|ship|investigate) printf '%s' 'claude codex patch' ;; codex) printf '%s' 'claude codex patch codex-cli' ;; cso) printf '%s' 'claude cso' ;; browse|qa|qa-only|design-review|design-consultation|design-html|design-motion|devex-review|scrape|skillify|canary|benchmark|make-pdf|diagram|pair-agent|open-gstack-browser|setup-browser-cookies|connect-chrome|land-and-deploy) printf '%s' 'claude codex patch browse-bundle browser' ;; @@ -78,7 +78,7 @@ gstack_doctor_skill_requires() { # gstack_doctor_skill_optional SKILL — ids that SKIP when absent; returns 1 for an unknown skill. gstack_doctor_skill_optional() { case "$1" in - autoplan|plan-ceo-review|plan-eng-review|plan-design-review|plan-devex-review|plan-tune|office-hours|spec|review|ship|investigate) printf '%s' 'codex-cli' ;; + autoplan|plan-ceo-review|plan-design-review|plan-devex-review|plan-tune|office-hours|spec|review|ship|investigate) printf '%s' 'codex-cli' ;; codex) printf '%s' '' ;; cso) printf '%s' '' ;; browse|qa|qa-only|design-review|design-consultation|design-html|design-motion|devex-review|scrape|skillify|canary|benchmark|make-pdf|diagram|pair-agent|open-gstack-browser|setup-browser-cookies|connect-chrome|land-and-deploy) printf '%s' 'codex-cli' ;; @@ -91,7 +91,7 @@ gstack_doctor_skill_optional() { # gstack_doctor_skill_pins_advisory SKILL — true for a planning-only workflow (a repo test-pin mismatch SKIPs pins). gstack_doctor_skill_pins_advisory() { case "$1" in - autoplan|plan-ceo-review|plan-eng-review|plan-design-review|plan-devex-review|office-hours) return 0 ;; + autoplan|plan-ceo-review|plan-design-review|plan-devex-review|office-hours) return 0 ;; *) return 1 ;; esac } diff --git a/bin/gstack-install-registry.sh b/bin/gstack-install-registry.sh index 27ed91c57..282387789 100644 --- a/bin/gstack-install-registry.sh +++ b/bin/gstack-install-registry.sh @@ -339,3 +339,10 @@ gstack_disabled_normalize() { done printf '%s' "$out" } + +# GSTACK_RETIRED_SKILLS — skills gstack keeps off every host whatever +# disabled_skills says; an existing gstack-owned entry is removed like a +# disabled one. The source stays for the skill that reads it: plan-eng-review +# is /autoplan's eng-phase methodology (GST-1698). Same list as RETIRED_SKILLS +# in scripts/discover-skills.ts (test/retired-skills.test.ts). +GSTACK_RETIRED_SKILLS="plan-eng-review" diff --git a/bin/gstack-relink b/bin/gstack-relink index f7443deba..7479a46b3 100755 --- a/bin/gstack-relink +++ b/bin/gstack-relink @@ -72,6 +72,10 @@ if [ -f "$SCRIPT_DIR/gstack-install-registry.sh" ]; then . "$SCRIPT_DIR/gstack-install-registry.sh" DISABLED="$(gstack_disabled_normalize "$("$GSTACK_CONFIG" get disabled_skills 2>/dev/null || true)")" fi +# Retired skills (GSTACK_RETIRED_SKILLS) lose their entry the same way; only +# the user's DISABLED list is reported and passed to the router. +OFF="$DISABLED" +[ -z "${GSTACK_RETIRED_SKILLS:-}" ] || OFF="$(gstack_disabled_normalize "$GSTACK_RETIRED_SKILLS $DISABLED")" DISABLED_COUNT=0 # #2569: rendered :user variants (brain-aware blocks) live in an UNTRACKED @@ -385,11 +389,11 @@ for skill_dir in "$INSTALL_DIR"/*/; do # A directory is a skill iff it carries a SKILL.md (#2263): a name list # dropped the real browse skill. Support dirs have no SKILL.md. [ -f "$skill_dir/SKILL.md" ] || continue - case "$DISABLED" in + case "$OFF" in *" ${skill#gstack-} "*) _cleanup_skill_entry "$SKILLS_DIR/$skill" "$skill" case "$skill" in gstack-*) ;; *) _cleanup_skill_entry "$SKILLS_DIR/gstack-$skill" "$skill" ;; esac - DISABLED_COUNT=$((DISABLED_COUNT + 1)) + case "$DISABLED" in *" ${skill#gstack-} "*) DISABLED_COUNT=$((DISABLED_COUNT + 1)) ;; esac continue ;; esac diff --git a/bin/gstack-skill-start b/bin/gstack-skill-start index 29dc7ce42..2daa50d8b 100755 --- a/bin/gstack-skill-start +++ b/bin/gstack-skill-start @@ -699,7 +699,7 @@ When the user's request matches an available skill, invoke it via the Skill tool Key routing rules: - Product ideas/brainstorming → invoke /office-hours - Strategy/scope → invoke /plan-ceo-review -- Architecture → invoke /plan-eng-review +- Architecture → invoke /autoplan - Design system/plan review → invoke /design-consultation or /plan-design-review - Full review pipeline → invoke /autoplan - Bugs/errors → invoke /investigate, unless one read-only check confirms the cause and a narrow fix (then fix it and run the focused test) diff --git a/changelog.d/capy-gst-1698-cut-plan-eng-review.md b/changelog.d/capy-gst-1698-cut-plan-eng-review.md new file mode 100644 index 000000000..e0db4392b --- /dev/null +++ b/changelog.d/capy-gst-1698-cut-plan-eng-review.md @@ -0,0 +1 @@ +- **`/plan-eng-review` is cut; `/autoplan` keeps its eng phase (GST-1698).** Two evals found no gain from the standalone skill. In the seeded-flaw panel, plain Opus 5.5 with no gstack found the planted engineering flaws in 15 of 16 reviews. In the wave 1 outcome test, it built 22 of 22 planned features correctly, and a session told to use the skill cost 7.1 times a plain one (GST-1218, gstack-evals#253). `./setup` no longer installs `/plan-eng-review` on any host, and the next `./setup` or upgrade removes an existing gstack-owned `plan-eng-review` / `gstack-plan-eng-review` entry. A skill you wrote yourself under that name is left alone. The entry is retired through the `disabled_skills` path and can't be re-enabled. Its methodology stays in `plan-eng-review/` as `/autoplan`'s Phase 3, byte for byte. On Codex, Kiro and the other external hosts, that methodology now ships beside `gstack-autoplan/SKILL.md` as `plan-eng-review.md`, because those hosts register every nested `SKILL.md`. To review a plan's architecture and tests, run `/autoplan`; its description now names architecture and engineering-review requests, so they route there. The router, the office-hours handoff and the CEO, design and devex review chaining now point there and no longer call eng review a "required gate". `/autoplan` also skips that chaining step inside its own run. `/ship` still accepts `review` or autoplan's `plan-eng-review` log row for Eng Review, and its over-200-lines hint now names only `/autoplan`. diff --git a/codex/SKILL.md b/codex/SKILL.md index 8a914356d..8d76a43ab 100644 --- a/codex/SKILL.md +++ b/codex/SKILL.md @@ -444,7 +444,7 @@ Produce this markdown table: |--------|---------|-----|------|--------|----------| | CEO Review | \`/plan-ceo-review\` | Scope & strategy | {runs} | {status} | {findings} | | Outside Review | {recorded provider and trigger} | Independent 2nd opinion | {runs} | {outside_status} | {findings} | -| Eng Review | \`/plan-eng-review\` | Architecture & tests (required) | {runs} | {status} | {findings} | +| Eng Review | \`/autoplan\` | Architecture & tests (required) | {runs} | {status} | {findings} | | Design Review | \`/plan-design-review\` | UI/UX gaps | {runs} | {status} | {findings} | | DX Review | \`/plan-devex-review\` | Developer experience gaps | {runs} | {status} | {findings} | \`\`\` diff --git a/devex-review/SKILL.md b/devex-review/SKILL.md index 1dc425e3c..d22a5cea1 100644 --- a/devex-review/SKILL.md +++ b/devex-review/SKILL.md @@ -821,7 +821,7 @@ Produce this markdown table: |--------|---------|-----|------|--------|----------| | CEO Review | \`/plan-ceo-review\` | Scope & strategy | {runs} | {status} | {findings} | | Outside Review | {recorded provider and trigger} | Independent 2nd opinion | {runs} | {outside_status} | {findings} | -| Eng Review | \`/plan-eng-review\` | Architecture & tests (required) | {runs} | {status} | {findings} | +| Eng Review | \`/autoplan\` | Architecture & tests (required) | {runs} | {status} | {findings} | | Design Review | \`/plan-design-review\` | UI/UX gaps | {runs} | {status} | {findings} | | DX Review | \`/plan-devex-review\` | Developer experience gaps | {runs} | {status} | {findings} | \`\`\` diff --git a/docs/OPENCLAW.md b/docs/OPENCLAW.md index b3fe8af55..ae2ca5a44 100644 --- a/docs/OPENCLAW.md +++ b/docs/OPENCLAW.md @@ -181,8 +181,8 @@ The agent should: 4. Verify with a test spawn **Native skills or ACP.** `./setup --host openclaw` (or `./setup` run inside an -OpenClaw session) gives OpenClaw's own agent gstack's skills: the five task -skills (review, investigate, qa, ship, plan-eng-review) listed directly and one +OpenClaw session) gives OpenClaw's own agent gstack's skills: the four task +skills (review, investigate, qa, ship) listed directly and one `gstack` router skill for the rest in `$OPENCLAW_STATE_DIR/skills`, rendered with OpenClaw's tool names and pointing at the installed gstack, so the agent runs gstack's workflows and helpers itself. Use ACP when the work should happen in a Claude @@ -190,7 +190,7 @@ Code session with the full install (hooks, Claude tool names); use native skills when the OpenClaw agent should do it. Both can be installed together. OpenClaw serves a registered skill's `SKILL.md` whole or refuses it, and its limit is 32,768 bytes when it does not know the model's context window -(32,000 characters with a 100K-200K-token one), so a skill over 32,000 bytes (review, investigate, ship and plan-eng-review among the five) is +(32,000 characters with a 100K-200K-token one), so a skill over 32,000 bytes (review, investigate and ship among the four) is registered as a short entry that tells the agent to read the skill's full `SKILL.md` in `gstack/skills/`, a plain copy of every skill that OpenClaw reads in pages (`.gstack-skills/` under `--skills all`). diff --git a/docs/PROJECT_STRUCTURE.md b/docs/PROJECT_STRUCTURE.md index dabf9c54e..c89d0879b 100644 --- a/docs/PROJECT_STRUCTURE.md +++ b/docs/PROJECT_STRUCTURE.md @@ -58,7 +58,7 @@ gstack/ ├── eval-plan/ # Preregistered paid evals: prereg, arms, $0 dry run, priced pilot (bin/gstack-eval-plan) ├── captain/ # /captain (also /crew): one controller, many lanes; templates/ holds CREW.md, AGENTS.md, NOTES.md, DIGEST.md, the flaky ledger, the lane report, the READY block, slot call, the lane brief, asks and the daily automation prompt ├── plan-ceo-review/ # /plan-ceo-review skill -├── plan-eng-review/ # /plan-eng-review skill +├── plan-eng-review/ # /autoplan's eng-phase methodology (retired as a skill, GST-1698) ├── autoplan/ # /autoplan skill (auto-review pipeline: CEO → design → DX → eng, eng always last) ├── benchmark/ # /benchmark skill (performance regression detection) ├── runaway-stopper/ # /runaway-stopper skill (runaway thread/subagent detect, stop, rotate, prevent; bin/gstack-runaway, templates/) diff --git a/docs/autoplan-guard-troubleshooting.md b/docs/autoplan-guard-troubleshooting.md index 4e7feaaa7..c30bfc455 100644 --- a/docs/autoplan-guard-troubleshooting.md +++ b/docs/autoplan-guard-troubleshooting.md @@ -22,8 +22,7 @@ Each code has one of four dispositions: **The fallback** (printed in every fallback denial): -1. Run the reviews by hand: `/plan-ceo-review`, then `/plan-devex-review`, - then `/plan-eng-review`. +1. Run the reviews by hand: `/plan-ceo-review`, then `/plan-devex-review`. 2. Or move to a fresh session and keep your work: `/context-save`, start a new Claude Code session in the project (not `--resume`), `/context-restore`, then `/autoplan `. diff --git a/docs/hosts.md b/docs/hosts.md index c5a5bf05c..337213637 100644 --- a/docs/hosts.md +++ b/docs/hosts.md @@ -61,7 +61,7 @@ characters, the part of a description Hermes's skills index shows. | Harness | What gstack registers | Install | Verify | Remove | |---|---|---|---|---| -| OpenClaw | default `--skills hybrid`: `gstack-review`, `gstack-investigate`, `gstack-qa`, `gstack-ship` and `gstack-plan-eng-review` as top-level skills plus one `gstack` router skill whose body lists every other skill and its path, in `${OPENCLAW_STATE_DIR:-~/.openclaw}/skills`. With OpenClaw 2026.10.6's 19 eligible bundled skills, hybrid's skills section is 10,290 characters, full format, under its 18,000-character budget; `--skills all` (17,971, compact format, descriptions cut) lists every skill as its own folder, `--skills router` (6,701) only the router. OpenClaw serves a registered skill's `SKILL.md` whole or refuses it ("Skill instructions cannot be partially served"), at most 32,768 bytes when its model catalog has no context window for the model and 32,000 characters with a 100K-200K-token window, so any registered skill over 32,000 bytes (four of the five in hybrid, 26 of 63 in `all` today) is a short entry `SKILL.md` with the same name and description that sends the agent to its full `SKILL.md` in a plain copy of every skill (the router's `gstack/skills/`, or `.gstack-skills/` under `--skills all`, a folder OpenClaw's skill scan skips), which OpenClaw reads in pages. A context window under 100K tokens lowers that budget to 16,000 characters, which this limit does not cover | `./setup --host openclaw`, or `./setup` inside OpenClaw | `openclaw skills list --json` shows `gstack` and the five task skills (source `openclaw-managed`, eligible) | `gstack-harness unregister openclaw` | +| OpenClaw | default `--skills hybrid`: `gstack-review`, `gstack-investigate`, `gstack-qa` and `gstack-ship` as top-level skills plus one `gstack` router skill whose body lists every other skill and its path, in `${OPENCLAW_STATE_DIR:-~/.openclaw}/skills`. With OpenClaw 2026.10.6's 19 eligible bundled skills, hybrid's skills section stays in full format under its 18,000-character budget; `--skills all` (17,971, compact format, descriptions cut) lists every skill as its own folder, `--skills router` (6,701) only the router. OpenClaw serves a registered skill's `SKILL.md` whole or refuses it ("Skill instructions cannot be partially served"), at most 32,768 bytes when its model catalog has no context window for the model and 32,000 characters with a 100K-200K-token window, so any registered skill over 32,000 bytes (three of the four in hybrid, 25 of 62 in `all` today) is a short entry `SKILL.md` with the same name and description that sends the agent to its full `SKILL.md` in a plain copy of every skill (the router's `gstack/skills/`, or `.gstack-skills/` under `--skills all`, a folder OpenClaw's skill scan skips), which OpenClaw reads in pages. A context window under 100K tokens lowers that budget to 16,000 characters, which this limit does not cover | `./setup --host openclaw`, or `./setup` inside OpenClaw | `openclaw skills list --json` shows `gstack` and the four task skills (source `openclaw-managed`, eligible) | `gstack-harness unregister openclaw` | | Hermes | every skill, as `gstack-` in one `gstack` category, in `${HERMES_HOME:-~/.hermes}/skills/gstack`. Hermes's skills index lists only skills registered as folders, so a router would leave the rest out of the listing (`skill_view` still loads an unlisted skill by name, but nothing tells the model it exists) | `./setup --host hermes`, or `./setup` inside Hermes | `hermes skills list` shows the `gstack-*` skills in category `gstack` | `gstack-harness unregister hermes` | | dots (feder-cr) | the `gstack` router in `/home/dot/skills/gstack`, which the Dot lists every turn | `./setup` on the Dot's computer, or `gstack-harness register dots` | the Dot's Skills page, or `GET /skills` | `gstack-harness unregister dots` | | QM | three skills (`gstack-investigate`, `gstack-review`, `gstack-plan`) in QM's own skill format, from `qm/sandbox/skills/` ([qm/README.md](../qm/README.md)). `gstack-harness register qm --yes` still stops before sending anything: QM imports a skill pack by cloning the whole repository and caps it at 32 MiB and 5,000 files, and gstack's repository is about 73 MiB ([why](troubleshooting.md#harness-qm-pack-too-large)) | copy `qm/sandbox/skills/gstack-*` into `/sandbox/skills/` and run `qm up` | ask it for a gstack skill | remove the folders and run `qm up` again | diff --git a/docs/skills.md b/docs/skills.md index 11698b319..ee7779dd9 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -7,7 +7,6 @@ Detailed guides for every gstack skill — philosophy, workflow, and examples. | [`/office-hours`](#office-hours) | **YC Office Hours** | Start here. Six forcing questions that reframe your product before you write code. Pushes back on your framing, challenges premises, generates implementation alternatives. Design doc feeds into every downstream skill. | | [`/spec`](#spec) | **Spec Author** | Turn vague intent into a precise, executable spec in five phases. Backlog-ready output that downstream skills can pick up. Optional agent spawn at the end. | | [`/plan-ceo-review`](#plan-ceo-review) | **CEO / Founder** | Rethink the problem. Find the 10-star product hiding inside the request. Four modes: Expansion, Selective Expansion, Hold Scope, Reduction. | -| [`/plan-eng-review`](#plan-eng-review) | **Eng Manager** | Lock in architecture, data flow, diagrams, edge cases, and tests. Forces hidden assumptions into the open. | | [`/plan-design-review`](#plan-design-review) | **Senior Designer** | Interactive plan-mode design review. Rates each dimension 0-10, explains what a 10 looks like, fixes the plan. Works in plan mode. | | [`/design-consultation`](#design-consultation) | **Design Partner** | Build a complete design system from scratch. Knows the landscape, proposes creative risks, generates realistic product mockups. Design at the heart of all other phases. | | [`/review`](#review) | **Staff Engineer** | Find the bugs that pass CI but blow up in production. Auto-fixes the obvious ones. Flags completeness gaps. Advisory simplification lens flags over-built code — never blocks, never auto-applies. | @@ -127,7 +126,7 @@ Recommends A because you learn from real usage. CRM data comes naturally in week ### The design doc -Both modes end with a design doc written to `~/.gstack/projects/` — and that doc feeds directly into `/plan-ceo-review` and `/plan-eng-review`. The full lifecycle is now: `office-hours → plan → implement → review → QA → ship → retro`. +Both modes end with a design doc written to `~/.gstack/projects/` — and that doc feeds directly into `/plan-ceo-review` and `/autoplan`. The full lifecycle is now: `office-hours → plan → implement → review → QA → ship → retro`. After the design doc is approved, `/office-hours` reflects on what it noticed about how you think — not generic praise, but specific callbacks to things you said during the session. The observations appear in the design doc too, so you re-encounter them when you re-read later. @@ -185,62 +184,11 @@ Visions and decisions are persisted to `~/.gstack/projects/` so they survive bey --- -## `/plan-eng-review` +## `/plan-eng-review` (cut) -This is my **eng manager mode**. +`/plan-eng-review` was cut on 2026-11-06 (GST-1698) after two evals found no gain. In the seeded-flaw panel, plain Opus 5.5 with no gstack found the planted engineering flaws in 15 of 16 reviews. In the wave 1 outcome test, plain Opus 5.5 built 22 of 22 planned features correctly, and a session told to use the skill cost 7.1 times a plain one. The evidence is in GST-1218 and gstack-evals#253. -Once the product direction is right, I want a different kind of intelligence entirely. I do not want more sprawling ideation. I do not want more "wouldn't it be cool if." I want the model to become my best technical lead. - -This mode should nail: - -* architecture -* system boundaries -* data flow -* state transitions -* failure modes -* edge cases -* trust boundaries -* test coverage - -And one surprisingly big unlock for me: **diagrams**. - -LLMs get way more complete when you force them to draw the system. Sequence diagrams, state diagrams, component diagrams, data-flow diagrams, even test matrices. Diagrams force hidden assumptions into the open. They make hand-wavy planning much harder. - -So `/plan-eng-review` is where I want the model to build the technical spine that can carry the product vision. - -### Example - -Take the same listing app example. - -Let's say `/plan-ceo-review` already did its job. We decided the real feature is not just photo upload. It is a smart listing flow that: - -* uploads photos -* identifies the product -* enriches the listing from the web -* drafts a strong title and description -* suggests the best hero image - -Now `/plan-eng-review` takes over. - -Now I want the model to answer questions like: - -* What is the architecture for upload, classification, enrichment, and draft generation? -* Which steps happen synchronously, and which go to background jobs? -* Where are the boundaries between app server, object storage, vision model, search/enrichment APIs, and the listing database? -* What happens if upload succeeds but enrichment fails? -* What happens if product identification is low-confidence? -* How do retries work? -* How do we prevent duplicate jobs? -* What gets persisted when, and what can be safely recomputed? - -And this is where I want diagrams — architecture diagrams, state models, data-flow diagrams, test matrices. Diagrams force hidden assumptions into the open. They make hand-wavy planning much harder. - -That is `/plan-eng-review`. - -Not "make the idea smaller." -**Make the idea buildable.** - -One note on invocation: in plan mode, the skill skips the "what should I review?" scope question and reviews your active plan automatically, announcing its pick in one line ("Scope gate: plan mode — auto-selected B") so you can redirect it. Name a target explicitly ("review PLAN.md") and your choice wins in any mode. Outside plan mode with nothing named, it asks first — that gate is a hard stop. +The standalone skill no longer installs, and `./setup` removes an existing gstack-owned `/plan-eng-review` entry on every host. Its review methodology stays as `/autoplan`'s eng phase (Phase 3), unchanged. To review a plan's architecture, data flow, edge cases and tests, run `/autoplan`. `/review` covers the diff before `/ship`. ### Review Readiness Dashboard @@ -260,7 +208,7 @@ Every review (CEO, Eng, Design) logs its result. At the end of each review, you +====================================================================+ ``` -Eng Review is the only required gate (disable with `gstack-config set skip_eng_review true`). CEO and Design are informational — recommended for product and UI changes respectively. +Eng Review is the only required gate; `/review` on the diff or `/autoplan`'s eng phase satisfies it (disable with `gstack-config set skip_eng_review true`). CEO and Design are informational — recommended for product and UI changes respectively. Diff reviews use the `review_freshness` grade computed by `gstack-review-read`, shared by `/ship` and `/land-and-deploy`. CURRENT requires a clean, reviewer-reported completed and converged pass whose captured start and finish fingerprints match the current working-tree content. Tracked edits and non-ignored untracked source both count; an identical commit hash or zero commits since review is not a fallback. @@ -270,7 +218,7 @@ Plan-file reviews retain their existing seven-day freshness handling and optiona ### Plan-to-QA flow -When `/plan-eng-review` finishes the test review section, it writes a test plan artifact to `~/.gstack/projects/`. When you later run `/qa`, it picks up that test plan automatically — your engineering review feeds directly into QA testing with no manual copy-paste. +When an eng review writes a test plan artifact to `~/.gstack/projects/`, a later `/qa` picks up that test plan automatically — your engineering review feeds directly into QA testing with no manual copy-paste. --- @@ -282,7 +230,7 @@ Most plans describe what the backend does but never specify what the user actual `/plan-design-review` catches all of this during planning, when it's cheap to fix. -It works like `/plan-ceo-review` and `/plan-eng-review` — interactive, one issue at a time, with the **STOP + AskUserQuestion** pattern. It rates each design dimension 0-10, explains what a 10 looks like, then edits the plan to get there. The rating drives the work: rate low = lots of fixes, rate high = quick pass. Like `/plan-eng-review`, it skips the "what should I review?" scope question in plan mode and targets your active plan automatically (announced in one line so you can redirect); an explicitly named target wins in any mode. +It works like `/plan-ceo-review` — interactive, one issue at a time, with the **STOP + AskUserQuestion** pattern. It rates each design dimension 0-10, explains what a 10 looks like, then edits the plan to get there. The rating drives the work: rate low = lots of fixes, rate high = quick pass. It skips the "what should I review?" scope question in plan mode and targets your active plan automatically (announced in one line so you can redirect); an explicitly named target wins in any mode. Seven passes over the plan: information architecture, interaction state coverage, user journey, AI slop risk, design system alignment, responsive/accessibility, and unresolved design decisions. For each pass, it finds gaps and either fixes them directly (obvious ones) or asks you to make a design choice (genuine tradeoffs). @@ -892,7 +840,7 @@ bounded scan and discloses inaccessible history or incomplete coverage. It reads raw uncommitted source without running project hooks or filters. It never edits code, runs project tests, saves a report, or creates issues or PRs. -`/plan-eng-review` applies the same criteria to the plan and proposed callers. +`/autoplan`'s eng phase applies the same criteria to the plan and proposed callers. `/review` checks the diff and related callers even on tiny changes. These scoped checks do not run the history audit. Optional extractions are advisory and require approval; they do not block a clean review or reduce its score. Actual defects @@ -934,7 +882,7 @@ gstack's own templates and for any skill folder you own. ## `/test-audit` Find existing tests that cost more than they protect. `/review`, `/ship`, `/qa` and -`/plan-eng-review` apply the same [test value bar](test-value-bar.md) to tests in a +`/autoplan`'s eng phase apply the same [test value bar](test-value-bar.md) to tests in a diff; `/test-audit` sweeps the tests that already exist. ```text @@ -1290,7 +1238,7 @@ English in, diagram out. Describe the diagram (or paste mermaid source) and you This is my **review autopilot mode**. -Running `/plan-ceo-review`, then `/plan-design-review`, then `/plan-eng-review` individually means answering 15-30 intermediate questions. Each question is valuable, but sometimes you want the gauntlet to run without stopping for every decision. +Running `/plan-ceo-review`, `/plan-design-review` and `/plan-devex-review` individually means answering 15-30 intermediate questions. Each question is valuable, but sometimes you want the gauntlet to run without stopping for every decision. `/autoplan` reads the review skills from disk and runs them sequentially: CEO → Design (if UI scope) → DX (if developer-facing scope) → Eng, always last — the required shipping gate reviews the final amended plan, not a stale one. It makes decisions automatically: each question resolves to its recommended option by default, with six encoded principles (prefer completeness, match existing patterns, choose reversible options, prefer the option the user chose for similar past decisions, defer ambiguous items, and escalate security) breaking ties and deciding questions that carry no recommendation. Taste decisions (close approaches, borderline scope expansions, cross-model disagreements) get saved and presented at a final approval gate. diff --git a/docs/test-value-bar.md b/docs/test-value-bar.md index 5cd916e0d..5233b4550 100644 --- a/docs/test-value-bar.md +++ b/docs/test-value-bar.md @@ -2,7 +2,7 @@ gstack's test workflows share one rule: a test earns its place by protecting behavior that a real regression would break. Test count is not a goal. The bar is -embedded in `/plan-eng-review`, `/review`, `/qa`, `/qa-only` and `/ship`, so every +embedded in `/autoplan`'s eng phase, `/review`, `/qa`, `/qa-only` and `/ship`, so every new or changed test in a diff meets it without a separate step. `/test-audit` applies the same bar to tests that already exist. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 5876b9a68..f1de2e14b 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -709,7 +709,7 @@ plan; the audit is `not run`. ### `PERSISTED: no` / `PERSIST: gstack-review-log '{...}'` / `no saved Eng Review: run the PERSIST: line ...` -**Meaning.** /plan-eng-review ran where the caller forbids writes (for example +**Meaning.** The eng review methodology (`/autoplan`'s eng phase) ran where the caller forbids writes (for example a delegating agent keeps the reviewer read-only, so it cannot edit the plan it judges). The review completed: it ends `DONE` with `PERSISTED: no`, records telemetry `success`, and returns the full report and decision ledger in its @@ -727,7 +727,7 @@ review-log command with this review's values. ~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"plan-eng-review",...}' ``` -Or rerun /plan-eng-review where it may write its report and log. +Or rerun /autoplan where it may write its report and log. **Expected result.** `gstack-review-read` lists the `plan-eng-review` entry and /ship's dashboard shows the Eng Review row (CLEAR when its status is `clean`). diff --git a/docs/voice-input.md b/docs/voice-input.md index 3401077f6..5cfdae809 100644 --- a/docs/voice-input.md +++ b/docs/voice-input.md @@ -1,8 +1,8 @@ # Voice input (AquaVoice, Whisper, etc.) Many gstack skills list speech-to-text aliases (`voice-triggers:` in their -templates): "security check" for `/cso`, "run QA" or "test the app" for `/qa`, -"tech review" for `/plan-eng-review`. Say one of those naturally and the right +templates): "security check" for `/cso`, "run QA" or "test the app" for `/qa`. +Say one of those naturally and the right skill can be picked without the slash command name. On Claude Code the aliases are routing lines in the root `/gstack` router; on Codex and the other hosts they are appended to each skill's description, which is what those hosts match diff --git a/gstack/llms.txt b/gstack/llms.txt index 4f7180eed..22ab954bf 100644 --- a/gstack/llms.txt +++ b/gstack/llms.txt @@ -10,7 +10,7 @@ Conventions: ## Skills -- [/autoplan](autoplan/SKILL.md): Auto-review pipeline — reads the full CEO, design, eng, and DX review skills from disk and runs them sequentially with auto-decisions using 6 decision principles. +- [/autoplan](autoplan/SKILL.md): Auto-review pipeline — runs the CEO, design, eng (architecture, edge cases, failure modes, tests) and DX reviews in order, auto-deciding by 6 principles. - [/benchmark](benchmark/SKILL.md): Web performance regression detection. - [/benchmark-models](benchmark-models/SKILL.md): Cross-model benchmark for gstack skills. - [/browse](browse/SKILL.md): Drive a real browser through Aside: open a page, read it, click through a flow, take screenshots, check console errors. @@ -54,7 +54,6 @@ Conventions: - [/plan-ceo-review](plan-ceo-review/SKILL.md): CEO/founder-mode plan review. - [/plan-design-review](plan-design-review/SKILL.md): Designer's eye plan review — interactive, like CEO and Eng review. - [/plan-devex-review](plan-devex-review/SKILL.md): Interactive developer experience plan review. -- [/plan-eng-review](plan-eng-review/SKILL.md): Eng manager-mode plan review. - [/plan-tune](plan-tune/SKILL.md): Self-tuning question sensitivity + developer psychographic for gstack (v1: observational). - [/qa](qa/SKILL.md): Use when you test the site, API or CLI and fix bugs. - [/qa-only](qa-only/SKILL.md): Report browser/API/CLI/job/worker/webhook bugs. diff --git a/lib/doctor-components.ts b/lib/doctor-components.ts index e2ee00ab3..81e24aecd 100644 --- a/lib/doctor-components.ts +++ b/lib/doctor-components.ts @@ -134,7 +134,6 @@ export function capyBootstrapCommand(skill: string, root: string = CAPY_INSTALL_ export const SKILL_REQUIREMENTS: Readonly> = { autoplan: PLANNING, 'plan-ceo-review': PLANNING, - 'plan-eng-review': PLANNING, 'plan-design-review': PLANNING, 'plan-devex-review': PLANNING, 'plan-tune': PLANNING, @@ -208,7 +207,7 @@ export const SKILL_REQUIREMENTS: Readonly> = { * required. */ export const PINS_ADVISORY_SKILLS: readonly string[] = [ - 'autoplan', 'plan-ceo-review', 'plan-eng-review', 'plan-design-review', 'plan-devex-review', 'office-hours', + 'autoplan', 'plan-ceo-review', 'plan-design-review', 'plan-devex-review', 'office-hours', ]; /** Rows always evaluated and required, in print order. */ diff --git a/lib/harness-register.ts b/lib/harness-register.ts index 8744f0977..10a46a093 100644 --- a/lib/harness-register.ts +++ b/lib/harness-register.ts @@ -37,7 +37,7 @@ export const BLOCK_END = ''; export const DOT_SKILLS_DIR = '/home/dot/skills'; export const ROUTER_DESCRIPTION = 'Use to review a diff or PR, debug a bug or failing test, QA test a site, plan or review architecture, ship a PR, audit security or write release docs. Load first for any coding task: lists every gstack skill and where to read it.'; /** OpenClaw hybrid mode: these task skills are listed as skills of their own; the router carries the rest. */ -export const HYBRID_SKILLS = ['review', 'investigate', 'qa', 'ship', 'plan-eng-review']; +export const HYBRID_SKILLS = ['review', 'investigate', 'qa', 'ship']; /** * OpenClaw serves a registered skill's SKILL.md whole or refuses it ("Skill instructions cannot be partially * served"), with maxBytes from resolveAdaptiveReadMaxBytes: 32,768 (DEFAULT_READ_PAGE_MAX_BYTES) when the model's diff --git a/lib/ship-preflight.ts b/lib/ship-preflight.ts index 90f57312f..c54d4a226 100644 --- a/lib/ship-preflight.ts +++ b/lib/ship-preflight.ts @@ -85,7 +85,7 @@ function integrate(cwd: string, base: string, run: string | undefined, r: Report const stat = git(cwd, ['diff', forkPoint(cwd, base), '--shortstat']).stdout.trim(); const lines = [...stat.matchAll(/(\d+) (insertion|deletion)/g)].reduce((n, m) => n + Number(m[1]), 0); const commits = git(cwd, ['rev-list', '--count', `origin/${base}..HEAD`]).stdout.trim(); - r.add(`DIFF: ${stat || 'no changes'}; commits=${commits || 0}${lines > 200 ? '; over 200 lines: recommend /plan-eng-review or /autoplan' : ''}`); + r.add(`DIFF: ${stat || 'no changes'}; commits=${commits || 0}${lines > 200 ? '; over 200 lines: recommend /autoplan' : ''}`); const pr = hostedOrigin(cwd) ? gh(cwd, ['pr', 'list', '--head', branch, '--state', 'open', '--json', 'number', '--jq', '.[0].number // empty']) : null; const n = pr?.stdout.trim() ?? ''; if (!pr) r.add('PR: not checked (origin is a local path, not a hosted remote gh can map; Step 0 uses git-native commands)'); diff --git a/office-hours/SKILL.md.tmpl b/office-hours/SKILL.md.tmpl index de7f761c0..fb80d82fe 100644 --- a/office-hours/SKILL.md.tmpl +++ b/office-hours/SKILL.md.tmpl @@ -13,7 +13,7 @@ description: | a new product idea, asks whether something is worth building, wants to think through design decisions for something that doesn't exist yet, or is exploring a concept before any code is written. - Use before /plan-ceo-review or /plan-eng-review. (gstack) + Use before /plan-ceo-review or /autoplan. (gstack) codex_default_prompt: "Use $gstack-office-hours to think this idea through before any code: challenge the premise, find the narrowest wedge, and save a design doc the later skills read." allowed-tools: - Bash diff --git a/office-hours/sections/design-and-handoff.md b/office-hours/sections/design-and-handoff.md index e201e0c85..5071a96f9 100644 --- a/office-hours/sections/design-and-handoff.md +++ b/office-hours/sections/design-and-handoff.md @@ -46,7 +46,7 @@ are empty or that restate what's already settled. No page cap — extra length must come from genuinely open questions, not template completeness. After writing, tell the user: -**"Design doc saved to: {repo path if written, else ~/.gstack path}{when both: ' (cross-session copy in ~/.gstack)'}. Other skills (/plan-ceo-review, /plan-eng-review) will find it automatically."** +**"Design doc saved to: {repo path if written, else ~/.gstack path}{when both: ' (cross-session copy in ~/.gstack)'}. Other skills (/plan-ceo-review, /autoplan) will find it automatically."** ### Startup mode design doc template: @@ -656,13 +656,13 @@ If E: proceed to next-skill recommendations. Don't just list options. Offer to launch the next review NOW so the design doc flows straight into a structured review. Map the design-doc mode to the recommended option -(default `/plan-eng-review` when ambiguous — it has the broadest real-world use and the -strongest retention). +(default `/autoplan` when ambiguous — it runs the CEO, design, DX and eng reviews in one +pass). **If `PROACTIVE` is `false` or `off`, OR `CONDUCTOR_SESSION: true`:** do NOT auto-launch. Recommend in one line and stop; the user invokes: - EXPANSION / ambitious → "Next: `/plan-ceo-review` to pressure-test scope and find the 10-star product." -- well-scoped → "Next: `/plan-eng-review` to lock architecture, tests, and edge cases." +- well-scoped → "Next: `/autoplan` to lock architecture, tests, and edge cases." - visual/UX-heavy → "Next: `/plan-design-review` for a visual/UX pass." **Otherwise**, offer via AskUserQuestion (D format from the preamble): @@ -673,11 +673,11 @@ ELI10: You just wrote a design doc. The natural next step is a structured review catches scope and architecture problems before you build. I can launch it right now, or you can run it later yourself. Stakes if we pick wrong: skipping review means problems surface mid-build, costing rework. -Recommendation: the mode-mapped option (`/plan-eng-review` if unsure) because it locks the +Recommendation: the mode-mapped option (`/autoplan` if unsure) because it locks the plan before any code is written. Completeness: A=10/10, B=9/10, C=8/10, D=3/10 Pros / cons: -A) Run /plan-eng-review now (recommended) +A) Run /autoplan now (recommended) ✅ Locks architecture, tests, and edge cases before a line of code is written ❌ Adds ~15 min CC now (human: 1-2 hrs of review compressed) B) Run /plan-ceo-review now diff --git a/office-hours/sections/design-and-handoff.md.tmpl b/office-hours/sections/design-and-handoff.md.tmpl index e9d57fe7a..91d5439be 100644 --- a/office-hours/sections/design-and-handoff.md.tmpl +++ b/office-hours/sections/design-and-handoff.md.tmpl @@ -43,7 +43,7 @@ are empty or that restate what's already settled. No page cap — extra length must come from genuinely open questions, not template completeness. After writing, tell the user: -**"Design doc saved to: {repo path if written, else ~/.gstack path}{when both: ' (cross-session copy in ~/.gstack)'}. Other skills (/plan-ceo-review, /plan-eng-review) will find it automatically."** +**"Design doc saved to: {repo path if written, else ~/.gstack path}{when both: ' (cross-session copy in ~/.gstack)'}. Other skills (/plan-ceo-review, /autoplan) will find it automatically."** ### Startup mode design doc template: @@ -497,13 +497,13 @@ If E: proceed to next-skill recommendations. Don't just list options. Offer to launch the next review NOW so the design doc flows straight into a structured review. Map the design-doc mode to the recommended option -(default `/plan-eng-review` when ambiguous — it has the broadest real-world use and the -strongest retention). +(default `/autoplan` when ambiguous — it runs the CEO, design, DX and eng reviews in one +pass). **If `PROACTIVE` is `false` or `off`, OR `CONDUCTOR_SESSION: true`:** do NOT auto-launch. Recommend in one line and stop; the user invokes: - EXPANSION / ambitious → "Next: `/plan-ceo-review` to pressure-test scope and find the 10-star product." -- well-scoped → "Next: `/plan-eng-review` to lock architecture, tests, and edge cases." +- well-scoped → "Next: `/autoplan` to lock architecture, tests, and edge cases." - visual/UX-heavy → "Next: `/plan-design-review` for a visual/UX pass." **Otherwise**, offer via AskUserQuestion (D format from the preamble): @@ -514,11 +514,11 @@ ELI10: You just wrote a design doc. The natural next step is a structured review catches scope and architecture problems before you build. I can launch it right now, or you can run it later yourself. Stakes if we pick wrong: skipping review means problems surface mid-build, costing rework. -Recommendation: the mode-mapped option (`/plan-eng-review` if unsure) because it locks the +Recommendation: the mode-mapped option (`/autoplan` if unsure) because it locks the plan before any code is written. Completeness: A=10/10, B=9/10, C=8/10, D=3/10 Pros / cons: -A) Run /plan-eng-review now (recommended) +A) Run /autoplan now (recommended) ✅ Locks architecture, tests, and edge cases before a line of code is written ❌ Adds ~15 min CC now (human: 1-2 hrs of review compressed) B) Run /plan-ceo-review now diff --git a/plan-ceo-review/sections/review-sections.md b/plan-ceo-review/sections/review-sections.md index 9736d5bfb..981e17a0f 100644 --- a/plan-ceo-review/sections/review-sections.md +++ b/plan-ceo-review/sections/review-sections.md @@ -898,7 +898,7 @@ Produce this markdown table: |--------|---------|-----|------|--------|----------| | CEO Review | `/plan-ceo-review` | Scope & strategy | {runs} | {status} | {findings} | | Outside Review | {recorded provider and trigger} | Independent 2nd opinion | {runs} | {outside_status} | {findings} | -| Eng Review | `/plan-eng-review` | Architecture & tests (required) | {runs} | {status} | {findings} | +| Eng Review | `/autoplan` | Architecture & tests (required) | {runs} | {status} | {findings} | | Design Review | `/plan-design-review` | UI/UX gaps | {runs} | {status} | {findings} | | DX Review | `/plan-devex-review` | Developer experience gaps | {runs} | {status} | {findings} | ``` @@ -1086,14 +1086,14 @@ explicitly, never as CLEAR. Display a fresh `clean` result as CLEAR and 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 /autoplan for the 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, /autoplan runs the eng review as its last phase. 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. +**If both are needed, recommend /autoplan**: it also runs the design review when it detects UI scope. Use AskUserQuestion to present the next step. Include only applicable options: -- **A)** Run /plan-eng-review next (required gate) +- **A)** Run /autoplan next (eng review) - **B)** Run /plan-design-review next (only if UI scope detected) - **C)** Skip — I'll handle reviews manually diff --git a/plan-ceo-review/sections/review-sections.md.tmpl b/plan-ceo-review/sections/review-sections.md.tmpl index fd32eeb6f..601d20ea3 100644 --- a/plan-ceo-review/sections/review-sections.md.tmpl +++ b/plan-ceo-review/sections/review-sections.md.tmpl @@ -568,14 +568,14 @@ Substitute `SCOPE_SUMMARY` (e.g. "accepted 4 of 6 proposals", "held scope" or 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 /autoplan for the 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, /autoplan runs the eng review as its last phase. 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. +**If both are needed, recommend /autoplan**: it also runs the design review when it detects UI scope. Use AskUserQuestion to present the next step. Include only applicable options: -- **A)** Run /plan-eng-review next (required gate) +- **A)** Run /autoplan next (eng review) - **B)** Run /plan-design-review next (only if UI scope detected) - **C)** Skip — I'll handle reviews manually diff --git a/plan-design-review/sections/review-sections.md b/plan-design-review/sections/review-sections.md index 256f8a17a..5318555d7 100644 --- a/plan-design-review/sections/review-sections.md +++ b/plan-design-review/sections/review-sections.md @@ -449,7 +449,7 @@ Produce this markdown table: |--------|---------|-----|------|--------|----------| | CEO Review | \`/plan-ceo-review\` | Scope & strategy | {runs} | {status} | {findings} | | Outside Review | {recorded provider and trigger} | Independent 2nd opinion | {runs} | {outside_status} | {findings} | -| Eng Review | \`/plan-eng-review\` | Architecture & tests (required) | {runs} | {status} | {findings} | +| Eng Review | \`/autoplan\` | Architecture & tests (required) | {runs} | {status} | {findings} | | Design Review | \`/plan-design-review\` | UI/UX gaps | {runs} | {status} | {findings} | | DX Review | \`/plan-devex-review\` | Developer experience gaps | {runs} | {status} | {findings} | \`\`\` @@ -676,14 +676,14 @@ SLUG=$(~/.claude/skills/gstack/bin/gstack-slug --get SLUG 2>/dev/null) || true From the dashboard output, recommend the next review(s): -- **/plan-eng-review** unless `skip_eng_review` is `true`: it is the required shipping gate. Stress it when this review added flows or changed the information architecture; an eng review whose commit predates this review is stale and should be re-run. -- **/plan-ceo-review** only for fundamental product gaps (initial score below 4/10, major IA problems, or doubt about the problem itself) and no CEO review exists. Most design reviews should not trigger it. If both apply, eng review first. +- **/autoplan** unless `skip_eng_review` is `true`: its eng phase reviews architecture and tests. Stress it when this review added flows or changed the information architecture; an eng review whose commit predates this review is stale and should be re-run. +- **/plan-ceo-review** only for fundamental product gaps (initial score below 4/10, major IA problems, or doubt about the problem itself) and no CEO review exists. Most design reviews should not trigger it. If both apply, /autoplan covers both. - **/design-shotgun** for visual issues worth exploring; **/design-html** when approved mockups need HTML. Both produce design artifacts, so they fit plan mode. Use AskUserQuestion to present the next step. Always include the manual/stop option E; offer only applicable follow-on skills. If the user chooses manual, finish without starting another skill: -- **A)** Run /plan-eng-review next (required gate) +- **A)** Run /autoplan next (eng review) - **B)** Run /plan-ceo-review (only if fundamental product gaps found) - **C)** Run /design-shotgun — explore visual design variants for issues found - **D)** Run /design-html — generate Pretext-native HTML from approved mockups diff --git a/plan-design-review/sections/review-sections.md.tmpl b/plan-design-review/sections/review-sections.md.tmpl index 9f86e6a9d..c8a5b66c4 100644 --- a/plan-design-review/sections/review-sections.md.tmpl +++ b/plan-design-review/sections/review-sections.md.tmpl @@ -260,14 +260,14 @@ Substitute values from the Completion Summary: From the dashboard output, recommend the next review(s): -- **/plan-eng-review** unless `skip_eng_review` is `true`: it is the required shipping gate. Stress it when this review added flows or changed the information architecture; an eng review whose commit predates this review is stale and should be re-run. -- **/plan-ceo-review** only for fundamental product gaps (initial score below 4/10, major IA problems, or doubt about the problem itself) and no CEO review exists. Most design reviews should not trigger it. If both apply, eng review first. +- **/autoplan** unless `skip_eng_review` is `true`: its eng phase reviews architecture and tests. Stress it when this review added flows or changed the information architecture; an eng review whose commit predates this review is stale and should be re-run. +- **/plan-ceo-review** only for fundamental product gaps (initial score below 4/10, major IA problems, or doubt about the problem itself) and no CEO review exists. Most design reviews should not trigger it. If both apply, /autoplan covers both. - **/design-shotgun** for visual issues worth exploring; **/design-html** when approved mockups need HTML. Both produce design artifacts, so they fit plan mode. Use AskUserQuestion to present the next step. Always include the manual/stop option E; offer only applicable follow-on skills. If the user chooses manual, finish without starting another skill: -- **A)** Run /plan-eng-review next (required gate) +- **A)** Run /autoplan next (eng review) - **B)** Run /plan-ceo-review (only if fundamental product gaps found) - **C)** Run /design-shotgun — explore visual design variants for issues found - **D)** Run /design-html — generate Pretext-native HTML from approved mockups diff --git a/plan-devex-review/SKILL.md b/plan-devex-review/SKILL.md index b9e9f11ab..19ca31daf 100644 --- a/plan-devex-review/SKILL.md +++ b/plan-devex-review/SKILL.md @@ -383,7 +383,7 @@ Before proceeding, read the plan and infer the developer product type from conte If NONE of the above: the plan has no developer-facing surface. Tell the user: "This plan doesn't appear to have developer-facing surfaces. /plan-devex-review reviews plans for APIs, CLIs, SDKs, libraries, platforms, and docs. Consider -/plan-eng-review or /plan-design-review instead." Exit gracefully. +/autoplan or /plan-design-review instead." Exit gracefully. If detected: State your classification and ask for confirmation. Do not ask from scratch. "I'm reading this as a CLI Tool plan. Correct?" diff --git a/plan-devex-review/SKILL.md.tmpl b/plan-devex-review/SKILL.md.tmpl index 18a919793..8696d9838 100644 --- a/plan-devex-review/SKILL.md.tmpl +++ b/plan-devex-review/SKILL.md.tmpl @@ -135,7 +135,7 @@ Before proceeding, read the plan and infer the developer product type from conte If NONE of the above: the plan has no developer-facing surface. Tell the user: "This plan doesn't appear to have developer-facing surfaces. /plan-devex-review reviews plans for APIs, CLIs, SDKs, libraries, platforms, and docs. Consider -/plan-eng-review or /plan-design-review instead." Exit gracefully. +/autoplan or /plan-design-review instead." Exit gracefully. If detected: State your classification and ask for confirmation. Do not ask from scratch. "I'm reading this as a CLI Tool plan. Correct?" diff --git a/plan-devex-review/sections/review-sections.md b/plan-devex-review/sections/review-sections.md index 166aa0b67..b42159ac9 100644 --- a/plan-devex-review/sections/review-sections.md +++ b/plan-devex-review/sections/review-sections.md @@ -764,7 +764,7 @@ Produce this markdown table: |--------|---------|-----|------|--------|----------| | CEO Review | \`/plan-ceo-review\` | Scope & strategy | {runs} | {status} | {findings} | | Outside Review | {recorded provider and trigger} | Independent 2nd opinion | {runs} | {outside_status} | {findings} | -| Eng Review | \`/plan-eng-review\` | Architecture & tests (required) | {runs} | {status} | {findings} | +| Eng Review | \`/autoplan\` | Architecture & tests (required) | {runs} | {status} | {findings} | | Design Review | \`/plan-design-review\` | UI/UX gaps | {runs} | {status} | {findings} | | DX Review | \`/plan-devex-review\` | Developer experience gaps | {runs} | {status} | {findings} | \`\`\` @@ -984,9 +984,9 @@ SLUG=$(~/.claude/skills/gstack/bin/gstack-slug --get SLUG 2>/dev/null) || true After displaying the Review Readiness Dashboard, recommend next reviews: -**Recommend /plan-eng-review if eng review is not skipped globally** — DX issues often +**Recommend /autoplan if eng review is not skipped globally** — DX issues often have architectural implications. If this DX review found API design problems, error -handling gaps, or CLI ergonomics issues, eng review should validate the fixes. +handling gaps, or CLI ergonomics issues, /autoplan's eng phase should validate the fixes. **Suggest /plan-design-review if user-facing UI exists** — DX review focuses on developer-facing surfaces; design review covers end-user-facing UI. @@ -997,7 +997,7 @@ out. This is where the competitive benchmark pays off: you have a concrete targe measure against. Use AskUserQuestion with applicable options: -- **A)** Run /plan-eng-review next (required gate) +- **A)** Run /autoplan next (eng review) - **B)** Run /plan-design-review (only if UI scope detected) - **C)** Ready to implement, run /devex-review after shipping - **D)** Skip, I'll handle next steps manually diff --git a/plan-devex-review/sections/review-sections.md.tmpl b/plan-devex-review/sections/review-sections.md.tmpl index 7ad879c6a..1c0d4d6a0 100644 --- a/plan-devex-review/sections/review-sections.md.tmpl +++ b/plan-devex-review/sections/review-sections.md.tmpl @@ -427,9 +427,9 @@ TIMESTAMP = current ISO 8601 datetime; STATUS = "clean" if score 8+ AND 0 unreso After displaying the Review Readiness Dashboard, recommend next reviews: -**Recommend /plan-eng-review if eng review is not skipped globally** — DX issues often +**Recommend /autoplan if eng review is not skipped globally** — DX issues often have architectural implications. If this DX review found API design problems, error -handling gaps, or CLI ergonomics issues, eng review should validate the fixes. +handling gaps, or CLI ergonomics issues, /autoplan's eng phase should validate the fixes. **Suggest /plan-design-review if user-facing UI exists** — DX review focuses on developer-facing surfaces; design review covers end-user-facing UI. @@ -440,7 +440,7 @@ out. This is where the competitive benchmark pays off: you have a concrete targe measure against. Use AskUserQuestion with applicable options: -- **A)** Run /plan-eng-review next (required gate) +- **A)** Run /autoplan next (eng review) - **B)** Run /plan-design-review (only if UI scope detected) - **C)** Ready to implement, run /devex-review after shipping - **D)** Skip, I'll handle next steps manually diff --git a/qa/SKILL.md b/qa/SKILL.md index 5c8b6ab91..d9b5dd54d 100644 --- a/qa/SKILL.md +++ b/qa/SKILL.md @@ -307,7 +307,7 @@ Prefer the richer of recent project test plans and plans in conversation over gi SLUG=$(~/.claude/skills/gstack/bin/gstack-slug --get SLUG 2>/dev/null) ls -t "$GSTACK_STATE_ROOT"/projects/$SLUG/*-test-plan-*.md 2>/dev/null | head -1 ``` -2. **Conversation context:** Prior `/plan-eng-review` or `/plan-ceo-review` test plans. +2. **Conversation context:** Prior `/autoplan` or `/plan-ceo-review` test plans. 3. Fall back to git diff only if neither exists. --- diff --git a/qa/SKILL.md.tmpl b/qa/SKILL.md.tmpl index 7bc6706b1..359b35ff1 100644 --- a/qa/SKILL.md.tmpl +++ b/qa/SKILL.md.tmpl @@ -120,7 +120,7 @@ Prefer the richer of recent project test plans and plans in conversation over gi {{SLUG_EVAL}} ls -t "$GSTACK_STATE_ROOT"/projects/$SLUG/*-test-plan-*.md 2>/dev/null | head -1 ``` -2. **Conversation context:** Prior `/plan-eng-review` or `/plan-ceo-review` test plans. +2. **Conversation context:** Prior `/autoplan` or `/plan-ceo-review` test plans. 3. Fall back to git diff only if neither exists. --- diff --git a/retro/SKILL.md b/retro/SKILL.md index 336dee513..bf5fc75b3 100644 --- a/retro/SKILL.md +++ b/retro/SKILL.md @@ -371,7 +371,7 @@ Conditional rows (skip each when its input is absent or empty in the window): If eureka moments exist, list them: ``` EUREKA /office-hours (branch: garrytan/auth-rethink): "Session tokens don't need server storage — browser crypto API makes client-side JWT validation viable" - EUREKA /plan-eng-review (branch: garrytan/cache-layer): "Redis isn't needed here — Bun's built-in LRU cache handles this workload" + EUREKA /autoplan (branch: garrytan/cache-layer): "Redis isn't needed here — Bun's built-in LRU cache handles this workload" ``` ### Step 3: Commit Time Distribution diff --git a/retro/SKILL.md.tmpl b/retro/SKILL.md.tmpl index 5db2f220c..80f170230 100644 --- a/retro/SKILL.md.tmpl +++ b/retro/SKILL.md.tmpl @@ -216,7 +216,7 @@ Conditional rows (skip each when its input is absent or empty in the window): If eureka moments exist, list them: ``` EUREKA /office-hours (branch: garrytan/auth-rethink): "Session tokens don't need server storage — browser crypto API makes client-side JWT validation viable" - EUREKA /plan-eng-review (branch: garrytan/cache-layer): "Redis isn't needed here — Bun's built-in LRU cache handles this workload" + EUREKA /autoplan (branch: garrytan/cache-layer): "Redis isn't needed here — Bun's built-in LRU cache handles this workload" ``` ### Step 3: Commit Time Distribution diff --git a/scripts/discover-skills.ts b/scripts/discover-skills.ts index 2122c534d..d1bf507b5 100644 --- a/scripts/discover-skills.ts +++ b/scripts/discover-skills.ts @@ -9,6 +9,16 @@ import type { HostConfig } from './host-config'; const SKIP = new Set(['node_modules', '.git', 'dist']); +/** + * Retired skills: the source stays because another skill reads it, but no host + * registers it. plan-eng-review is /autoplan's eng-phase methodology (GST-1698). + * External hosts get that methodology beside gstack-autoplan/SKILL.md as + * .md, since Codex and Copilot register every nested SKILL.md. setup and + * gstack-relink read the same list from GSTACK_RETIRED_SKILLS in + * bin/gstack-install-registry.sh. + */ +export const RETIRED_SKILLS = ['plan-eng-review']; + /** The generator and its coverage checks use the same include-minus-skip rule. */ export function includesSkill(host: HostConfig, skillDir: string): boolean { const { includeSkills, skipSkills } = host.generation; diff --git a/scripts/gen-llms-txt.ts b/scripts/gen-llms-txt.ts index 176e720f6..66070d51a 100644 --- a/scripts/gen-llms-txt.ts +++ b/scripts/gen-llms-txt.ts @@ -24,7 +24,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { mkdirpSync } from '../lib/fs-utils'; -import { discoverTemplates, includesSkill } from './discover-skills'; +import { discoverTemplates, includesSkill, RETIRED_SKILLS } from './discover-skills'; import claudeHost from '../hosts/claude'; import { COMMAND_DESCRIPTIONS as BROWSE_COMMANDS } from '../browse/src/commands'; @@ -145,7 +145,7 @@ export async function generateLlmsTxt(opts: GenerateOptions = {}): Promise(); for (const template of discoverTemplates(source)) { const dir = path.dirname(template.tmpl); const name = externalSkillName(dir, extractNameAndDescription(fs.readFileSync(path.join(source, template.tmpl), 'utf8')).name); + if (RETIRED_SKILLS.includes(dir)) continue; sourceNames.add(name); if (!includesSkill(config, dir)) continue; names.add(name); diff --git a/scripts/resolvers/composition.ts b/scripts/resolvers/composition.ts index 19b9be512..acf9ed896 100644 --- a/scripts/resolvers/composition.ts +++ b/scripts/resolvers/composition.ts @@ -2,6 +2,7 @@ import { toShellPath, type TemplateContext } from './types'; import * as path from 'path'; import { getHostConfig } from '../../hosts'; import { runtimeRootPrelude } from './runtime-root'; +import { RETIRED_SKILLS } from '../discover-skills'; /** Claude's scoped hook enforces the parent publication boundary during /autoplan. */ export function generateAutoplanPublicationHook(ctx: TemplateContext, args?: string[]): string { @@ -119,6 +120,10 @@ export function generateAutoplanReviewFile(ctx: TemplateContext, args?: string[] if (ctx.host === 'claude') return `\`${ctx.paths.skillRoot}/${skill}/SKILL.md\``; const host = getHostConfig(ctx.host); + if (RETIRED_SKILLS.includes(skill)) { + const own = `gstack-autoplan/${skill}.md`; + return `the file \`${skill}.md\` in the installed \`/autoplan\` SKILL.md directory (local: \`${path.posix.dirname(host.localSkillRoot)}/${own}\`; global: \`~/${path.posix.dirname(host.globalRoot)}/${own}\`${ctx.host === 'codex' ? ', or the corresponding skills directory under CODEX_HOME when configured' : ''})`; + } const file = `gstack-${skill}/SKILL.md`; const local = `${path.posix.dirname(host.localSkillRoot)}/${file}`; const global = `~/${path.posix.dirname(host.globalRoot)}/${file}`; diff --git a/scripts/resolvers/review-dashboard.ts b/scripts/resolvers/review-dashboard.ts index 4b2e58237..9873da53a 100644 --- a/scripts/resolvers/review-dashboard.ts +++ b/scripts/resolvers/review-dashboard.ts @@ -58,7 +58,7 @@ and outside_status; retain partial coverage. These details do not clear Eng Revi **3. Choose the historical verdict.** CLEARED requires the selected Eng Review to be \`clean\`, within 7 days and fresh under step 2. Otherwise report NOT CLEARED -and its missing, stale or open-issue reason.${ctx.skillName === 'ship' ? ` With no Eng Review record, say "no saved Eng Review: run the \`PERSIST:\` line a read-only /plan-eng-review printed, or rerun it" (docs/troubleshooting.md#plan-eng-review-not-persisted).` : ''} If \`skip_eng_review\` is true, show +and its missing, stale or open-issue reason.${ctx.skillName === 'ship' ? ` With no Eng Review record, say "no saved Eng Review: run the \`PERSIST:\` line a read-only eng review printed, or rerun /autoplan" (docs/troubleshooting.md#plan-eng-review-not-persisted).` : ''} If \`skip_eng_review\` is true, show "SKIPPED (global)" for Eng Review and CLEARED for this dashboard. ${ctx.skillName === 'ship' ? 'This verdict never skips Step 9 or its finding, approval and convergence gates. Continue Step 1 even when history is NOT CLEARED.' : 'Eng Review is required by default; `gstack-config set skip_eng_review true` disables that requirement.'} @@ -164,7 +164,7 @@ ${conditionalWrites ? 'Display `clean` as CLEAR and `issues_open` as ISSUES OPEN |--------|---------|-----|------|--------|----------| | CEO Review | \\\`/plan-ceo-review\\\` | Scope & strategy | {runs} | {status} | {findings} | | Outside Review | {recorded provider and trigger} | Independent 2nd opinion | {runs} | {outside_status} | {findings} | -| Eng Review | \\\`/plan-eng-review\\\` | Architecture & tests (required) | {runs} | {status} | {findings} | +| Eng Review | \\\`${ctx.skillName === 'plan-eng-review' ? '/plan-eng-review' : '/autoplan'}\\\` | Architecture & tests (required) | {runs} | {status} | {findings} | | Design Review | \\\`/plan-design-review\\\` | UI/UX gaps | {runs} | {status} | {findings} | | DX Review | \\\`/plan-devex-review\\\` | Developer experience gaps | {runs} | {status} | {findings} | \\\`\\\`\\\` diff --git a/scripts/resolvers/voice-triggers.ts b/scripts/resolvers/voice-triggers.ts index da652833b..e2875025c 100644 --- a/scripts/resolvers/voice-triggers.ts +++ b/scripts/resolvers/voice-triggers.ts @@ -10,7 +10,7 @@ import * as fs from 'fs'; import * as path from 'path'; import type { TemplateContext } from './types'; -import { discoverTemplates, includesSkill } from '../discover-skills'; +import { discoverTemplates, includesSkill, RETIRED_SKILLS } from '../discover-skills'; import { getHostConfig } from '../../hosts/index'; const ROOT = path.resolve(import.meta.dir, '..', '..'); @@ -51,7 +51,7 @@ export function generateVoiceTriggerRouting(ctx: TemplateContext): string { // Sorted so the render doesn't depend on filesystem iteration order. const skillDirs = discoverTemplates(ROOT).map(t => path.dirname(t.tmpl)).sort(); for (const skillDir of skillDirs) { - if (skillDir === '.' || !includesSkill(hostConfig, skillDir)) continue; + if (skillDir === '.' || RETIRED_SKILLS.includes(skillDir) || !includesSkill(hostConfig, skillDir)) continue; const content = fs.readFileSync(path.join(ROOT, skillDir, 'SKILL.md.tmpl'), 'utf-8').replace(/\r\n/g, '\n'); const triggers = extractVoiceTriggers(content); if (triggers.length === 0) continue; diff --git a/setup b/setup index c267679b5..eadcf8943 100755 --- a/setup +++ b/setup @@ -1111,6 +1111,7 @@ _skill_source_exists() { # render from the template's frontmatter `name:` when that differs from the # directory, so both the directory and every frontmatter name count. local g="$1" n="$2" base="${2#gstack-}" + case " ${GSTACK_RETIRED_SKILLS:-} " in *" $base "*) return 1 ;; esac [ -f "$g/$base/SKILL.md.tmpl" ] && return 0 [ -f "$g/$n/SKILL.md.tmpl" ] && return 0 grep -qsE "^name:[[:space:]]*(gstack-)?${base}[[:space:]]*$" "$g"/*/SKILL.md.tmpl 2>/dev/null && return 0 @@ -1323,8 +1324,8 @@ _setup_print_summary() { echo "" echo "Install summary:" printf '%s' "$_SETUP_ROWS" | gstack_install_registry_render - if [ "$_DISABLED_SKILLS" != " " ] && [ -n "${_DISABLED_SKILLS:-}" ]; then - echo " disabled skills (not registered on any host):$_DISABLED_SKILLS" + if [ "${_USER_DISABLED_SKILLS:- }" != " " ]; then + echo " disabled skills (not registered on any host):$_USER_DISABLED_SKILLS" echo " re-enable: gstack-config set disabled_skills \"\"" fi } @@ -1442,11 +1443,14 @@ fi # it keep working. Missing or empty = every skill enabled (#1206). Linkers test # membership inline with "${_DISABLED_SKILLS:- }", so a linker extracted on its # own (tests) behaves as "nothing disabled". -_DISABLED_SKILLS="$(gstack_disabled_normalize "$("$GSTACK_CONFIG" get disabled_skills 2>/dev/null || true)")" +_USER_DISABLED_SKILLS="$(gstack_disabled_normalize "$("$GSTACK_CONFIG" get disabled_skills 2>/dev/null || true)")" +# Retired skills (GSTACK_RETIRED_SKILLS) take the same no-entry path on every +# host; only the user's own list is reported and passed to the router. +_DISABLED_SKILLS="$(gstack_disabled_normalize "$GSTACK_RETIRED_SKILLS $_USER_DISABLED_SKILLS")" # Every render of the router (committed-path renders of env-var hosts and the # per-install renders) omits disabled skills (C8); the default Claude router is # rendered in place by gstack-relink. -_DISABLED_CSV="$(printf '%s' "$_DISABLED_SKILLS" | xargs | tr ' ' ',')" +_DISABLED_CSV="$(printf '%s' "$_USER_DISABLED_SKILLS" | xargs | tr ' ' ',')" # plan_mode_preamble=off (#3168): every render below omits the plan-mode # preamble; the default Claude install then serves a render instead of the # committed files (bin/gstack-render-claude.sh). @@ -4021,6 +4025,21 @@ if [ "$INSTALL_KIRO" -eq 1 ]; then fi _copy_skill_md "$skill_dir/SKILL.md" "$target_dir/SKILL.md" _record_skill_copies "$target_dir" || true + # Generated siblings of SKILL.md (gstack-autoplan/plan-eng-review.md, the + # Phase 3 methodology) travel with it; user files next to them survive. + for extra_file in "$skill_dir"/*.md; do + [ -f "$extra_file" ] || continue + [ "$(basename "$extra_file")" = "SKILL.md" ] && continue + extra_dest="$target_dir/$(basename "$extra_file")" + if [ -L "$extra_dest" ]; then + extra_target="$(_gstack_link_target_abs "$extra_dest")" + _gstack_target_is_ours "$extra_target" "$SOURCE_GSTACK_DIR" || continue + elif [ -e "$extra_dest" ] && ! _gstack_generated_header "$extra_dest"; then + echo " kept $skill_name/$(basename "$extra_file"): existing file is not gstack-managed" >&2 + continue + fi + _link_or_copy "$extra_file" "$extra_dest" + done # Native sections already contain Kiro paths/provider markers. Refresh # generated files individually so user assets next to them survive. if [ -d "$skill_dir/sections" ]; then diff --git a/ship/SKILL.md b/ship/SKILL.md index 847810c4e..688f0b977 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -395,7 +395,7 @@ and outside_status; retain partial coverage. These details do not clear Eng Revi **3. Choose the historical verdict.** CLEARED requires the selected Eng Review to be `clean`, within 7 days and fresh under step 2. Otherwise report NOT CLEARED -and its missing, stale or open-issue reason. With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only /plan-eng-review printed, or rerun it" (docs/troubleshooting.md#plan-eng-review-not-persisted). If `skip_eng_review` is true, show +and its missing, stale or open-issue reason. With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only eng review printed, or rerun /autoplan" (docs/troubleshooting.md#plan-eng-review-not-persisted). If `skip_eng_review` is true, show "SKIPPED (global)" for Eng Review and CLEARED for this dashboard. This verdict never skips Step 9 or its finding, approval and convergence gates. Continue Step 1 even when history is NOT CLEARED. @@ -423,7 +423,7 @@ VERDICT: {CLEARED or NOT CLEARED} — {reason} An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. -Over 200 lines, `DIFF:` recommends `/plan-eng-review` or `/autoplan`. `SCOPE: frontend=true` (from +Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from `gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index a18af4768..c96726c0d 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -153,7 +153,7 @@ act on each `STOP:` and `NEXT:` line. An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. -Over 200 lines, `DIFF:` recommends `/plan-eng-review` or `/autoplan`. `SCOPE: frontend=true` (from +Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from `gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/spec/SKILL.md b/spec/SKILL.md index 722483a88..4307940e1 100644 --- a/spec/SKILL.md +++ b/spec/SKILL.md @@ -622,8 +622,7 @@ Add to the standard template: route them to `/office-hours` first. `/spec` is for work that has already passed the "is this worth building" bar. - **After `/spec`:** if the spec describes architectural or design risk that - needs review before implementation starts, suggest `/plan-eng-review` (or - `/autoplan` for the full review gauntlet). + needs review before implementation starts, suggest `/autoplan`. - **For implementation:** the issue itself is the handoff. The implementer can open it and execute without re-asking the user. **Implementation model:** relay model/source from `"$HOME/.claude/skills/gstack/bin/gstack-models" resolve --role implementation --provider anthropic`. gstack cannot change this session. Recommend only; no spawn or config edits unless asked. On error, relay its repair, not a model. [Policy setup](https://github.com/garrytan/gstack/blob/main/docs/model-policy.md). diff --git a/spec/SKILL.md.tmpl b/spec/SKILL.md.tmpl index aad712c22..58b8eb7d2 100644 --- a/spec/SKILL.md.tmpl +++ b/spec/SKILL.md.tmpl @@ -474,8 +474,7 @@ Add to the standard template: route them to `/office-hours` first. `/spec` is for work that has already passed the "is this worth building" bar. - **After `/spec`:** if the spec describes architectural or design risk that - needs review before implementation starts, suggest `/plan-eng-review` (or - `/autoplan` for the full review gauntlet). + needs review before implementation starts, suggest `/autoplan`. - **For implementation:** the issue itself is the handoff. The implementer can open it and execute without re-asking the user. {{IMPLEMENTATION_MODEL_HANDOFF}} diff --git a/test-audit/SKILL.md b/test-audit/SKILL.md index 3072ea8cf..03c444f6f 100644 --- a/test-audit/SKILL.md +++ b/test-audit/SKILL.md @@ -102,7 +102,7 @@ telemetry — it never blocks the workflow. Find existing tests that cost more than they protect, prove it with evidence, and retire them only in approved batches. Optimize for confidence, not deletion count; a few well-evidenced candidates beat a large speculative list, and none is a valid -result. `/review`, `/ship`, `/qa` and `/plan-eng-review` apply the same bar to new +result. `/review`, `/ship`, `/qa` and `/autoplan` apply the same bar to new tests in a diff; this skill is the whole-repo sweep for tests that already exist. Usage: `/test-audit [path ...] [--since ] [--max-candidates N]` (default: whole diff --git a/test-audit/SKILL.md.tmpl b/test-audit/SKILL.md.tmpl index b063cd16d..4c66b7b7c 100644 --- a/test-audit/SKILL.md.tmpl +++ b/test-audit/SKILL.md.tmpl @@ -26,7 +26,7 @@ allowed-tools: Find existing tests that cost more than they protect, prove it with evidence, and retire them only in approved batches. Optimize for confidence, not deletion count; a few well-evidenced candidates beat a large speculative list, and none is a valid -result. `/review`, `/ship`, `/qa` and `/plan-eng-review` apply the same bar to new +result. `/review`, `/ship`, `/qa` and `/autoplan` apply the same bar to new tests in a diff; this skill is the whole-repo sweep for tests that already exist. Usage: `/test-audit [path ...] [--since ] [--max-candidates N]` (default: whole diff --git a/test/autoplan-bounded-journal.test.ts b/test/autoplan-bounded-journal.test.ts index 768d50c2a..087a75009 100644 --- a/test/autoplan-bounded-journal.test.ts +++ b/test/autoplan-bounded-journal.test.ts @@ -77,7 +77,7 @@ describe('A2: the guard decides on a journal larger than its shrunk bounds', () const output = await bounded(() => f.hook(input), RECORD, 4 * 1024); expect(codeOf(output)).toBe('oversized_invocation'); expect(denial(output)).toContain('retained-data bound'); - expect(denial(output)).toContain('/plan-ceo-review, then /plan-devex-review, then /plan-eng-review'); + expect(denial(output)).toContain('/plan-ceo-review, then /plan-devex-review by hand'); expect(f.log().at(-1)).toMatchObject({ decision: 'deny', disposition: 'fallback', code: 'oversized_invocation' }); await bounded(() => expect(ownedRetainedLimit()).toBe(OWNED_RETAINED_MAX_BYTES), RECORD, OWNED_RETAINED_MAX_BYTES * 4); }); @@ -157,7 +157,7 @@ describe('A2: append-tolerant reads with prefix identity', () => { test('a journal that shrinks between two reads is an identity denial', async () => { const { output } = await twoReads(file => fs.truncateSync(file, Math.floor(fs.statSync(file).size / 2))); expect(codeOf(output)).toBe('identity'); - expect(denial(output)).toContain('/plan-ceo-review, then /plan-devex-review, then /plan-eng-review'); + expect(denial(output)).toContain('/plan-ceo-review, then /plan-devex-review by hand'); }); test('the reader itself: a changed prefix is rewritten, fewer bytes than the prefix is identity', () => { diff --git a/test/autoplan-guard-reasons.test.ts b/test/autoplan-guard-reasons.test.ts index 6154d918f..39963056b 100644 --- a/test/autoplan-guard-reasons.test.ts +++ b/test/autoplan-guard-reasons.test.ts @@ -30,7 +30,7 @@ describe('guard reason inventory', () => { if (reason.disposition === 'unverified') expect(text).toContain('Phase-report enforcement was skipped for this call'); if (reason.disposition === 'corrective') expect(text).not.toContain(FALLBACK); } - expect(FALLBACK).toContain('/plan-ceo-review, then /plan-devex-review, then /plan-eng-review'); + expect(FALLBACK).toContain('/plan-ceo-review, then /plan-devex-review by hand'); expect(FALLBACK).toContain('/context-save'); expect(FALLBACK).toContain('/context-restore, then /autoplan '); }); diff --git a/test/autoplan-methodology-names.test.ts b/test/autoplan-methodology-names.test.ts index cdd3e0118..deda76857 100644 --- a/test/autoplan-methodology-names.test.ts +++ b/test/autoplan-methodology-names.test.ts @@ -4,6 +4,7 @@ import { chmodSync, cpSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rm import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { createSnapshot, prepareMethodology } from '../bin/gstack-autoplan-snapshot'; +import { RETIRED_SKILLS } from '../scripts/discover-skills'; const ROOT = resolve(import.meta.dir, '..'); const owned: string[] = []; @@ -22,13 +23,16 @@ function fixture(phase: string, layout: string) { }); expect(generated.status, generated.stderr).toBe(0); } - const source = layout === 'inlineCodex' ? join(dir, '.agents/skills', `gstack-${skill}`) : join(ROOT, skill); - const target = join(dir, skill); + // A retired skill's external render is a methodology file beside /autoplan. + const retiredInline = layout === 'inlineCodex' && RETIRED_SKILLS.includes(skill); + const source = layout === 'inlineCodex' ? join(dir, '.agents/skills', retiredInline ? 'gstack-autoplan' : `gstack-${skill}`) : join(ROOT, skill); + const fileName = retiredInline ? `${skill}.md` : 'SKILL.md'; + const target = join(dir, retiredInline ? 'gstack-autoplan' : skill); mkdirSync(target); - cpSync(join(source, 'SKILL.md'), join(target, 'SKILL.md')); + cpSync(join(source, fileName), join(target, fileName)); // C4: Codex carves plan-ceo-review too, so an install that has sections/ copies it. // Every tier 2+ skill ships sections/preamble.md; carving is the review section. - const carved = existsSync(join(source, 'sections', 'review-sections.md')); + const carved = !retiredInline && existsSync(join(source, 'sections', 'review-sections.md')); if (existsSync(join(source, 'sections'))) cpSync(join(source, 'sections'), join(target, 'sections'), { recursive: true }); if (layout === 'patchedClaude') { const result = spawnSync('bash', [join(ROOT, 'bin/gstack-patch-names'), dir, 'true'], { encoding: 'utf8', timeout: 10_000 }); @@ -39,7 +43,7 @@ function fixture(phase: string, layout: string) { const restore = join(dir, 'restore.md'); writeFileSync(active, '## Implementation plan\nBuild the widget.\n## Review record\n'); writeFileSync(restore, 'Restore point.\n'); - return { dir, file: join(target, 'SKILL.md'), active, restore, skill, carved }; + return { dir, file: join(target, fileName), active, restore, skill, carved }; } describe('installed methodology identities', () => { diff --git a/test/autoplan-publication-hook.test.ts b/test/autoplan-publication-hook.test.ts index 9635e38fa..da97d11ee 100644 --- a/test/autoplan-publication-hook.test.ts +++ b/test/autoplan-publication-hook.test.ts @@ -156,7 +156,7 @@ describe('Autoplan hook journal-root verdicts', () => { const reason: string = output.hookSpecificOutput.permissionDecisionReason; expect(reason).toContain(`code ${code}`); expect(reason).toContain('Claude Code 2.1.284'); - expect(reason).toContain('/plan-ceo-review, then /plan-devex-review, then /plan-eng-review'); + expect(reason).toContain('/plan-ceo-review, then /plan-devex-review by hand'); expect(reason).toContain('docs/autoplan-guard-troubleshooting.md'); expect(reason).not.toContain('no missing-publication conclusion'); const records = fs.readFileSync(s.log, 'utf8').trim().split('\n').map(line => JSON.parse(line)); diff --git a/test/autoplan-publication-payload.test.ts b/test/autoplan-publication-payload.test.ts index 0515e426f..471649705 100644 --- a/test/autoplan-publication-payload.test.ts +++ b/test/autoplan-publication-payload.test.ts @@ -156,7 +156,7 @@ describe('Publication flush: a report counts only once a later journaled record f.report(f.nextMessage()); f.journal(); const output = await f.hook(next(f)); expect(codeOf(output)).toBe('publication_repeat'); - expect(denial(output)).toContain('/plan-ceo-review, then /plan-devex-review, then /plan-eng-review'); + expect(denial(output)).toContain('/plan-ceo-review, then /plan-devex-review by hand'); }); test('a denied call after the report proves the report was journaled, so the next attempt counts it', async () => { diff --git a/test/autoplan-review-discovery.test.ts b/test/autoplan-review-discovery.test.ts index 7e40a5959..24215662c 100644 --- a/test/autoplan-review-discovery.test.ts +++ b/test/autoplan-review-discovery.test.ts @@ -8,9 +8,12 @@ import { ALL_HOST_CONFIGS } from '../hosts'; import { generateAutoplanReviewFile } from '../scripts/resolvers/composition'; import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types'; import { E2E_TOUCHFILES } from './helpers/touchfiles'; +import { RETIRED_SKILLS } from '../scripts/discover-skills'; const ROOT = path.resolve(import.meta.dir, '..'); const REVIEWS = ['plan-ceo-review', 'plan-design-review', 'plan-devex-review', 'plan-eng-review']; +/** External hosts: a retired review ships beside gstack-autoplan/SKILL.md as .md (GST-1698). */ +const externalRef = (review: string) => RETIRED_SKILLS.includes(review) ? `${review}.md` : `../gstack-${review}/SKILL.md`; let owned: string; let rendered: string; @@ -75,7 +78,11 @@ describe('autoplan reads installed host methodology', () => { // Read the path actually emitted in the entrypoint. Resolving it from // the installed entrypoint works for copied installs and symlinked ones. const refs = [...body.matchAll(/`(\.\.\/gstack-plan-[a-z-]+\/SKILL\.md)`/g)].map(match => match[1]!); - expect([...new Set(refs)].sort()).toEqual(REVIEWS.map(name => `../gstack-${name}/SKILL.md`).sort()); + expect([...new Set(refs)].sort()).toEqual(REVIEWS.filter(name => !RETIRED_SKILLS.includes(name)).map(externalRef).sort()); + for (const name of REVIEWS.filter(review => RETIRED_SKILLS.includes(review))) { + expect(body).toContain(`the file \`${name}.md\` in the installed \`/autoplan\` SKILL.md directory`); + expect(body).not.toContain(`gstack-${name}/SKILL.md`); + } for (const review of REVIEWS) expect(body).not.toContain(`$GSTACK_ROOT/${review}/SKILL.md`); expect(body.replace(/\s+/g, ' ')).toContain("Use /autoplan's installed registry; resolve siblings from its discovered SKILL.md"); } @@ -88,14 +95,14 @@ describe('autoplan reads installed host methodology', () => { installFile(generatedEntry, entry, mode); for (const review of REVIEWS) { const reviewName = host.name === 'claude' ? review : `gstack-${review}`; - const generatedReview = path.join(generatedRoot, reviewName, 'SKILL.md'); - installFile(generatedReview, path.join(registry, reviewName, 'SKILL.md'), mode); + const reference = host.name === 'claude' ? `../${review}/SKILL.md` : externalRef(review); + const generatedReview = path.resolve(path.join(generatedRoot, entryName), reference); + installFile(generatedReview, path.resolve(path.dirname(entry), reference), mode); // A runtime root may be an unrelated checkout. Its old canonical // review file must never be selected instead of the installed host. const stale = path.join(registry, 'gstack', review, 'SKILL.md'); fs.mkdirSync(path.dirname(stale), { recursive: true }); fs.writeFileSync(stale, 'STALE FOREIGN HARNESS SKILL'); - const reference = host.name === 'claude' ? `../${review}/SKILL.md` : [...body.matchAll(/`(\.\.\/gstack-plan-[a-z-]+\/SKILL\.md)`/g)].find(match => match[1] === `../gstack-${review}/SKILL.md`)![1]!; const loaded = fs.readFileSync(path.resolve(path.dirname(entry), reference), 'utf8'); expect(loaded).toBe(fs.readFileSync(generatedReview, 'utf8')); expect(loaded).not.toContain('STALE FOREIGN HARNESS SKILL'); diff --git a/test/disabled-plan-review-evidence.test.ts b/test/disabled-plan-review-evidence.test.ts index 66448eef3..d94a9cbbc 100644 --- a/test/disabled-plan-review-evidence.test.ts +++ b/test/disabled-plan-review-evidence.test.ts @@ -219,10 +219,10 @@ describe('disabled outside-plan live oracle', () => { const unexpectedWrite = join(repo, 'unexpected-review-write'); writeFileSync(join(brokenRuntime, 'bin/gstack-review-log'), '#!/bin/sh\nprintf invoked > "$UNEXPECTED_REVIEW_WRITE"\n', { mode: 0o755 }); for (const [skill, id, phase] of [ - ['gstack-plan-eng-review', 'codex-plan-review', 'plan-review'], - ['gstack-document-release', 'codex-doc-review', 'documentation'], + ['gstack-autoplan/plan-eng-review.md', 'codex-plan-review', 'plan-review'], + ['gstack-document-release/SKILL.md', 'codex-doc-review', 'documentation'], ]) { - const instructions = readFileSync(join(rendered, '.agents', 'skills', skill, 'SKILL.md'), 'utf8'); + const instructions = readFileSync(join(rendered, '.agents', 'skills', skill), 'utf8'); const fence = [...instructions.matchAll(/```bash\n([\s\S]*?)\n```/g)] .map(match => match[1]).find(command => command.includes(`"skill":"${id}"`) && command.includes('"outside_status":"disabled"')); expect(fence, skill).toBeDefined(); diff --git a/test/doctor-components.test.ts b/test/doctor-components.test.ts index 3bcb854a0..66969b290 100644 --- a/test/doctor-components.test.ts +++ b/test/doctor-components.test.ts @@ -14,6 +14,7 @@ import { } from '../lib/doctor-components'; import { RESULT_CODES } from '../lib/result-codes'; import { DOCTOR_COMPONENTS_RELPATH } from '../scripts/gen-doctor-components'; +import { RETIRED_SKILLS } from '../scripts/discover-skills'; const ROOT = path.resolve(import.meta.dir, '..'); const bash = (script: string) => spawnSync('bash', ['-c', `. "${path.join(ROOT, DOCTOR_COMPONENTS_RELPATH)}" && ${script}`], { encoding: 'utf8', timeout: 20_000 }); @@ -34,7 +35,7 @@ describe('doctor component table', () => { }); test('every skill directory in the repo is classified, and no classified skill is missing (install profiles are the only other rows)', () => { - const skills = fs.readdirSync(ROOT).filter(d => fs.existsSync(path.join(ROOT, d, 'SKILL.md.tmpl'))).sort(); + const skills = fs.readdirSync(ROOT).filter(d => fs.existsSync(path.join(ROOT, d, 'SKILL.md.tmpl')) && !RETIRED_SKILLS.includes(d)).sort(); expect(skills.length).toBeGreaterThan(40); expect(Object.keys(SKILL_REQUIREMENTS).filter(s => !INSTALL_PROFILES.includes(s)).sort()).toEqual(skills); expect(INSTALL_PROFILES).toEqual(['review-voice']); diff --git a/test/eng-review-routing.test.ts b/test/eng-review-routing.test.ts index 6c8372d7d..92215bfd9 100644 --- a/test/eng-review-routing.test.ts +++ b/test/eng-review-routing.test.ts @@ -158,7 +158,7 @@ describe('engineering review routing contracts', () => { test('#3106: an unpersisted review gets no saved credit; /ship names the PERSIST: line or a rerun', () => { const ctx = (skillName: string) => ({ skillName, tmplPath: `${skillName}/SKILL.md.tmpl`, host: 'claude', paths: HOST_PATHS.claude } as TemplateContext); const ship = compact(generateReviewDashboard(ctx('ship'))); - expect(ship).toContain('With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only /plan-eng-review printed, or rerun it"'); + expect(ship).toContain('With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only eng review printed, or rerun /autoplan"'); expect(ship).toContain('docs/troubleshooting.md#plan-eng-review-not-persisted'); expect(generateReviewDashboard(ctx('plan-eng-review'))).not.toContain('PERSIST:'); const gate = compact(generateExitPlanModeGate(ctx('plan-eng-review'))); diff --git a/test/external-host-carve.test.ts b/test/external-host-carve.test.ts index 665e655fe..46d394027 100644 --- a/test/external-host-carve.test.ts +++ b/test/external-host-carve.test.ts @@ -93,7 +93,7 @@ describe('C4: ship, plan-ceo-review, office-hours and design-review are carved o } expect(offenders).toEqual([]); const codex = ALL_HOST_CONFIGS.find(c => c.name === 'codex')!; - expect(fs.readFileSync(path.join(skillDir(codex, renders.all, 'plan-eng-review'), 'SKILL.md'), 'utf8')) + expect(fs.readFileSync(path.join(skillDir(codex, renders.all, 'autoplan'), 'plan-eng-review.md'), 'utf8')) .toContain('Read the `review-sections` section inlined in this SKILL.md'); const html = fs.readFileSync(path.join(skillDir(codex, renders.all, 'design-html'), 'SKILL.md'), 'utf8'); expect(html).not.toContain('detector-install-offer.md'); diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index e240b0923..fa4786bfe 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -396,7 +396,7 @@ and outside_status; retain partial coverage. These details do not clear Eng Revi **3. Choose the historical verdict.** CLEARED requires the selected Eng Review to be `clean`, within 7 days and fresh under step 2. Otherwise report NOT CLEARED -and its missing, stale or open-issue reason. With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only /plan-eng-review printed, or rerun it" (docs/troubleshooting.md#plan-eng-review-not-persisted). If `skip_eng_review` is true, show +and its missing, stale or open-issue reason. With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only eng review printed, or rerun /autoplan" (docs/troubleshooting.md#plan-eng-review-not-persisted). If `skip_eng_review` is true, show "SKIPPED (global)" for Eng Review and CLEARED for this dashboard. This verdict never skips Step 9 or its finding, approval and convergence gates. Continue Step 1 even when history is NOT CLEARED. @@ -424,7 +424,7 @@ VERDICT: {CLEARED or NOT CLEARED} — {reason} An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. -Over 200 lines, `DIFF:` recommends `/plan-eng-review` or `/autoplan`. `SCOPE: frontend=true` (from +Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from `gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 847810c4e..688f0b977 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -395,7 +395,7 @@ and outside_status; retain partial coverage. These details do not clear Eng Revi **3. Choose the historical verdict.** CLEARED requires the selected Eng Review to be `clean`, within 7 days and fresh under step 2. Otherwise report NOT CLEARED -and its missing, stale or open-issue reason. With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only /plan-eng-review printed, or rerun it" (docs/troubleshooting.md#plan-eng-review-not-persisted). If `skip_eng_review` is true, show +and its missing, stale or open-issue reason. With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only eng review printed, or rerun /autoplan" (docs/troubleshooting.md#plan-eng-review-not-persisted). If `skip_eng_review` is true, show "SKIPPED (global)" for Eng Review and CLEARED for this dashboard. This verdict never skips Step 9 or its finding, approval and convergence gates. Continue Step 1 even when history is NOT CLEARED. @@ -423,7 +423,7 @@ VERDICT: {CLEARED or NOT CLEARED} — {reason} An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. -Over 200 lines, `DIFF:` recommends `/plan-eng-review` or `/autoplan`. `SCOPE: frontend=true` (from +Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from `gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index f30493591..d1ba0868e 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -395,7 +395,7 @@ and outside_status; retain partial coverage. These details do not clear Eng Revi **3. Choose the historical verdict.** CLEARED requires the selected Eng Review to be `clean`, within 7 days and fresh under step 2. Otherwise report NOT CLEARED -and its missing, stale or open-issue reason. With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only /plan-eng-review printed, or rerun it" (docs/troubleshooting.md#plan-eng-review-not-persisted). If `skip_eng_review` is true, show +and its missing, stale or open-issue reason. With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only eng review printed, or rerun /autoplan" (docs/troubleshooting.md#plan-eng-review-not-persisted). If `skip_eng_review` is true, show "SKIPPED (global)" for Eng Review and CLEARED for this dashboard. This verdict never skips Step 9 or its finding, approval and convergence gates. Continue Step 1 even when history is NOT CLEARED. @@ -423,7 +423,7 @@ VERDICT: {CLEARED or NOT CLEARED} — {reason} An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. -Over 200 lines, `DIFF:` recommends `/plan-eng-review` or `/autoplan`. `SCOPE: frontend=true` (from +Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from `gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index 83930fe0a..babfb52cb 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -397,7 +397,7 @@ and outside_status; retain partial coverage. These details do not clear Eng Revi **3. Choose the historical verdict.** CLEARED requires the selected Eng Review to be `clean`, within 7 days and fresh under step 2. Otherwise report NOT CLEARED -and its missing, stale or open-issue reason. With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only /plan-eng-review printed, or rerun it" (docs/troubleshooting.md#plan-eng-review-not-persisted). If `skip_eng_review` is true, show +and its missing, stale or open-issue reason. With no Eng Review record, say "no saved Eng Review: run the `PERSIST:` line a read-only eng review printed, or rerun /autoplan" (docs/troubleshooting.md#plan-eng-review-not-persisted). If `skip_eng_review` is true, show "SKIPPED (global)" for Eng Review and CLEARED for this dashboard. This verdict never skips Step 9 or its finding, approval and convergence gates. Continue Step 1 even when history is NOT CLEARED. @@ -425,7 +425,7 @@ VERDICT: {CLEARED or NOT CLEARED} — {reason} An outside review with status `unverified` or `unavailable` is missing coverage, never a pass. -Over 200 lines, `DIFF:` recommends `/plan-eng-review` or `/autoplan`. `SCOPE: frontend=true` (from +Over 200 lines, `DIFF:` recommends `/autoplan`. `SCOPE: frontend=true` (from `gstack-diff-scope `) with no design review: mention "Design Review not run — Step 9 includes the lite check; consider /design-review for a full visual audit." Continue to Step 2 without asking; Step 9 applies the review gates. diff --git a/test/fixtures/skill-md-size-allowlist.json b/test/fixtures/skill-md-size-allowlist.json index 6212b3f5c..57c3f72a8 100644 --- a/test/fixtures/skill-md-size-allowlist.json +++ b/test/fixtures/skill-md-size-allowlist.json @@ -2,55 +2,55 @@ "_comment": "Generated SKILL.md files over the limit, pinned at their exact character count (#3199). test/skill-md-size-guard.test.ts fails when a pinned skill grows past its pin or shrinks below it: lower the pin when a skill shrinks and delete it once the skill is within the limit. Raising a pin is an owner decision. The shared-preamble hoist (GSTA-96) moved every tier 2+ skill's shared preamble into sections/preamble.md; the pins below are what remains.", "limit": 100000, "pins": { - "agy/gstack-autoplan": 109640, - "agy/gstack-plan-design-review": 109107, - "agy/gstack-plan-devex-review": 114948, + "agy/gstack-autoplan": 109601, + "agy/gstack-plan-design-review": 109100, + "agy/gstack-plan-devex-review": 114928, "agy/gstack-plan-eng-review": 143581, "agy/gstack-review": 104752, - "codex/gstack-autoplan": 108383, - "codex/gstack-plan-design-review": 110464, - "codex/gstack-plan-devex-review": 116456, + "codex/gstack-autoplan": 108344, + "codex/gstack-plan-design-review": 110457, + "codex/gstack-plan-devex-review": 116436, "codex/gstack-plan-eng-review": 144746, "codex/gstack-review": 104275, - "copilot/gstack-autoplan": 109083, - "copilot/gstack-plan-design-review": 109373, - "copilot/gstack-plan-devex-review": 114410, + "copilot/gstack-autoplan": 109044, + "copilot/gstack-plan-design-review": 109366, + "copilot/gstack-plan-devex-review": 114390, "copilot/gstack-plan-eng-review": 143746, "copilot/gstack-review": 104333, - "cursor/gstack-autoplan": 108315, - "cursor/gstack-plan-design-review": 107915, - "cursor/gstack-plan-devex-review": 113680, + "cursor/gstack-autoplan": 108276, + "cursor/gstack-plan-design-review": 107908, + "cursor/gstack-plan-devex-review": 113660, "cursor/gstack-plan-eng-review": 142336, "cursor/gstack-review": 122805, - "factory/gstack-autoplan": 110593, - "factory/gstack-plan-design-review": 111013, - "factory/gstack-plan-devex-review": 117043, + "factory/gstack-autoplan": 110554, + "factory/gstack-plan-design-review": 111006, + "factory/gstack-plan-devex-review": 117023, "factory/gstack-plan-eng-review": 145444, "factory/gstack-review": 126578, - "gbrain/gstack-autoplan": 108722, + "gbrain/gstack-autoplan": 108683, "gbrain/gstack-plan-eng-review": 124963, - "gemini/gstack-autoplan": 108873, - "gemini/gstack-plan-design-review": 109201, - "gemini/gstack-plan-devex-review": 114238, + "gemini/gstack-autoplan": 108834, + "gemini/gstack-plan-design-review": 109194, + "gemini/gstack-plan-devex-review": 114218, "gemini/gstack-plan-eng-review": 143579, "gemini/gstack-review": 104155, - "hermes/gstack-autoplan": 108725, + "hermes/gstack-autoplan": 108637, "hermes/gstack-plan-eng-review": 124999, - "kiro/gstack-autoplan": 108089, - "kiro/gstack-plan-design-review": 107761, - "kiro/gstack-plan-devex-review": 113532, + "kiro/gstack-autoplan": 108050, + "kiro/gstack-plan-design-review": 107754, + "kiro/gstack-plan-devex-review": 113512, "kiro/gstack-plan-eng-review": 142198, "kiro/gstack-review": 122629, - "openclaw/gstack-autoplan": 108545, + "openclaw/gstack-autoplan": 108506, "openclaw/gstack-plan-eng-review": 123478, - "opencode/gstack-autoplan": 109064, - "opencode/gstack-plan-design-review": 109229, - "opencode/gstack-plan-devex-review": 114288, + "opencode/gstack-autoplan": 109025, + "opencode/gstack-plan-design-review": 109222, + "opencode/gstack-plan-devex-review": 114268, "opencode/gstack-plan-eng-review": 143605, "opencode/gstack-review": 123427, - "slate/gstack-autoplan": 108202, - "slate/gstack-plan-design-review": 107838, - "slate/gstack-plan-devex-review": 113606, + "slate/gstack-autoplan": 108163, + "slate/gstack-plan-design-review": 107831, + "slate/gstack-plan-devex-review": 113586, "slate/gstack-plan-eng-review": 142267, "slate/gstack-review": 122717 } diff --git a/test/gen-skill-docs-checks.test.ts b/test/gen-skill-docs-checks.test.ts index 8a93103c2..d0caf3ec2 100644 --- a/test/gen-skill-docs-checks.test.ts +++ b/test/gen-skill-docs-checks.test.ts @@ -67,7 +67,7 @@ describe('generator artifact and dry-run contract', () => { const skills = generated.artifacts.filter(a => a.kind === 'skill' && pointerId.test(fs.readFileSync(path.join(render, a.relativePath), 'utf-8'))); expect(skills.length).toBeGreaterThan(40 * ALL_HOST_NAMES.length); expect(preambles.map(a => `${a.host}:${path.dirname(path.dirname(a.relativePath))}`).sort()) - .toEqual(skills.map(a => `${a.host}:${path.dirname(a.relativePath)}`).sort()); + .toEqual([...new Set(skills.map(a => `${a.host}:${path.dirname(a.relativePath)}`))].sort()); for (const host of ALL_HOST_NAMES) { const bodies = new Set(preambles.filter(a => a.host === host).map(a => fs.readFileSync(path.join(render, a.relativePath), 'utf-8'))); expect(bodies.size, host).toBe(1); diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index ef4cf899c..b4bcee604 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -2,6 +2,7 @@ import { describe, test, expect, afterAll } from 'bun:test'; import { assertSinglePreamble, extractCodexDefaultPrompt, generateOpenAIYaml } from '../scripts/gen-skill-docs'; import { validateSkillFrontmatter } from '../scripts/skill-check'; import { externalHostPathLeaks } from './helpers/skill-parser'; +import { RETIRED_SKILLS } from '../scripts/discover-skills'; import { COMMAND_DESCRIPTIONS } from '../browse/src/commands'; import { SNAPSHOT_FLAGS } from '../browse/src/snapshot'; import * as fs from 'fs'; @@ -916,13 +917,13 @@ describe('REVIEW_DASHBOARD resolver', () => { }); } - test('plan-ceo-review chaining mentions eng and design reviews', () => { + test('plan-ceo-review chaining mentions eng (via /autoplan) and design reviews', () => { // Carved skill: the chaining prose lives in sections/*.md. (It used to // pass against the skeleton only because the preamble's routing-injection // rules incidentally named these skills — that prose moved into // bin/gstack-skill-start in token-reduction Phase 2.) const content = readSkillUnion('plan-ceo-review'); - expect(content).toContain('/plan-eng-review'); + expect(content).toContain('Run /autoplan next (eng review)'); expect(content).toContain('/plan-design-review'); }); @@ -932,9 +933,9 @@ describe('REVIEW_DASHBOARD resolver', () => { expect(content).toContain('/plan-ceo-review'); }); - test('plan-design-review chaining mentions eng, ceo, and design skills', () => { + test('plan-design-review chaining mentions eng (via /autoplan), ceo, and design skills', () => { const content = readSkillUnion('plan-design-review'); - expect(content).toContain('/plan-eng-review'); + expect(content).toContain('Run /autoplan next (eng review)'); expect(content).toContain('/plan-ceo-review'); expect(content).toContain('/design-shotgun'); expect(content).toContain('/design-html'); @@ -1302,7 +1303,7 @@ describe('PLAN_FILE_REVIEW_REPORT resolver', () => { expect(content).toContain('Findings'); expect(content).toContain('VERDICT'); expect(content).toContain('/plan-ceo-review'); - expect(content).toContain('/plan-eng-review'); + expect(content).toContain('| Eng Review | `/autoplan` |'); expect(content).toContain('/plan-design-review'); expect(content).toContain('/codex review'); }); @@ -2890,6 +2891,7 @@ describe('Codex generation (--host codex)', () => { for (const entry of fs.readdirSync(ROOT, { withFileTypes: true })) { if (!entry.isDirectory() || entry.name.startsWith('.') || entry.name === 'node_modules') continue; if (entry.name === 'codex') continue; // /codex is excluded from Codex output + if (RETIRED_SKILLS.includes(entry.name)) continue; if (!fs.existsSync(path.join(ROOT, entry.name, 'SKILL.md.tmpl'))) continue; const codexName = entry.name.startsWith('gstack-') ? entry.name : `gstack-${entry.name}`; skills.push({ dir: entry.name, codexName }); @@ -3348,6 +3350,7 @@ describe('Factory generation (--host factory)', () => { for (const entry of fs.readdirSync(ROOT, { withFileTypes: true })) { if (!entry.isDirectory() || entry.name.startsWith('.') || entry.name === 'node_modules') continue; if (entry.name === 'codex') continue; + if (RETIRED_SKILLS.includes(entry.name)) continue; if (!fs.existsSync(path.join(ROOT, entry.name, 'SKILL.md.tmpl'))) continue; const factoryName = entry.name.startsWith('gstack-') ? entry.name : `gstack-${entry.name}`; skills.push({ dir: entry.name, factoryName }); @@ -4531,7 +4534,7 @@ describe('voice-triggers processing', () => { test('Claude root router routes every skill voice trigger the catalog trim drops', () => { const routing = between(fs.readFileSync(path.join(ROOT, 'SKILL.md'), 'utf-8'), '**Routing rules', '**Route only to skills'); - const skills = fs.readdirSync(ROOT).filter(d => !fs.lstatSync(path.join(ROOT, d)).isSymbolicLink() && fs.existsSync(path.join(ROOT, d, 'SKILL.md.tmpl'))); + const skills = fs.readdirSync(ROOT).filter(d => !fs.lstatSync(path.join(ROOT, d)).isSymbolicLink() && fs.existsSync(path.join(ROOT, d, 'SKILL.md.tmpl')) && !RETIRED_SKILLS.includes(d)); let routed = 0; for (const skill of skills) { const triggers = extractVoiceTriggers(fs.readFileSync(path.join(ROOT, skill, 'SKILL.md.tmpl'), 'utf-8').replace(/\r\n/g, '\n')); diff --git a/test/helpers/autoplan-preconfigured-fixture.ts b/test/helpers/autoplan-preconfigured-fixture.ts index 6e2b4cb36..f181a145a 100644 --- a/test/helpers/autoplan-preconfigured-fixture.ts +++ b/test/helpers/autoplan-preconfigured-fixture.ts @@ -9,7 +9,7 @@ When the user's request matches an available skill, invoke it via the Skill tool Key routing rules: - Product ideas/brainstorming → invoke /office-hours - Strategy/scope → invoke /plan-ceo-review -- Architecture → invoke /plan-eng-review +- Architecture → invoke /autoplan - Design system/plan review → invoke /design-consultation or /plan-design-review - Full review pipeline → invoke /autoplan - Bugs/errors → invoke /investigate, unless one read-only check confirms the cause and a narrow fix (then fix it and run the focused test) diff --git a/test/helpers/plan-review-cases.ts b/test/helpers/plan-review-cases.ts index 0e66fe70b..340ee464b 100644 --- a/test/helpers/plan-review-cases.ts +++ b/test/helpers/plan-review-cases.ts @@ -67,7 +67,8 @@ export function pickPlanReviewQuestion(question: NativeQuestion): number { const labels = question.options.map(option => planReviewOptionLabel(option.label)); const run = (label: string) => /^(?:Run )?\/plan-(?:ceo|eng|design|devex)-review(?: (?:next|first))?(?:\s*\((?:required gate|only if UI scope detected(?: and no design review exists)?|only if fundamental product gaps found|only if significant product change and no CEO review exists)\))?$/i.test(label) || /^(?:Run )?\/design-shotgun(?: to explore visual variants| for visual variants| after adding an OpenAI key|\s*[—–-]\s*explore visual design variants for issues found)?$/i.test(label) - || /^(?:Run )?\/design-html(?:\s*[—–-]\s*generate Pretext-native HTML from approved mockups)?$/i.test(label); + || /^(?:Run )?\/design-html(?:\s*[—–-]\s*generate Pretext-native HTML from approved mockups)?$/i.test(label) + || /^(?:Run )?\/autoplan(?: next)?(?:\s*\(eng review\))?$/i.test(label); // A bare Skip declines only an offered, recognized follow-up in this handoff. const offersFollowUp = labels.some(run); const manual = (label: string) => /^Skip\s*[,:;.—–-]\s*(?:I(?:['’]ll| will)\s+)?handle (?:reviews|next steps) manually$/i.test(label) diff --git a/test/outside-voice-routing.test.ts b/test/outside-voice-routing.test.ts index 2dddae92e..257d55067 100644 --- a/test/outside-voice-routing.test.ts +++ b/test/outside-voice-routing.test.ts @@ -5,6 +5,7 @@ import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { ALL_HOST_CONFIGS } from '../hosts'; import { ALL_MODEL_NAMES } from '../scripts/models'; +import { RETIRED_SKILLS } from '../scripts/discover-skills'; import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types'; import { outsideVoiceFor, outsideVoiceCommand } from '../scripts/resolvers/outside-voice'; import { RESOLVERS } from '../scripts/resolvers'; @@ -81,7 +82,7 @@ describe('generated automatic review coverage', () => { for (const skill of skills) { test(`Codex /${skill} invokes Claude Code, including lazy sections`, () => { const host = ALL_HOST_CONFIGS.find(h => h.name === 'codex')!; - const text = readUnion(skillDir(host, skill)); + const text = RETIRED_SKILLS.includes(skill) ? readFileSync(join(skillDir(host, 'autoplan'), `${skill}.md`), 'utf8') : readUnion(skillDir(host, skill)); expect(text).toContain('gstack-claude-code'); // Prose about the CLI is permitted; executable codex review commands are not. expect(shellLines(text).filter(line => /\bcodex\s+(?:exec|review)\b/.test(line))).toEqual([]); diff --git a/test/plan-review-cases.test.ts b/test/plan-review-cases.test.ts index 6d0f66e13..14917fd58 100644 --- a/test/plan-review-cases.test.ts +++ b/test/plan-review-cases.test.ts @@ -265,12 +265,12 @@ describe('plan report persistence precedes completion logging', () => { || artifact.relativePath.endsWith(`/gstack-${skill}/sections/review-sections.md`)); const carved = new Set(generated.artifacts.filter(isSection).map(a => `${a.host}:${a.relativePath.replace(/\/sections\/review-sections\.md$/, '')}`)); const carriers = generated.artifacts.filter(artifact => isSection(artifact) - || (artifact.kind === 'skill' && plans.some(skill => artifact.relativePath.endsWith(`/gstack-${skill}/SKILL.md`)) + || (artifact.kind === 'skill' && plans.some(skill => artifact.relativePath.endsWith(`/gstack-${skill}/SKILL.md`) || artifact.relativePath.endsWith(`/gstack-autoplan/${skill}.md`)) && !carved.has(`${artifact.host}:${artifact.relativePath.replace(/\/SKILL\.md$/, '')}`))); expect(carriers).toHaveLength(plans.length * ALL_HOST_CONFIGS.length); for (const carrier of carriers) { const content = readFileSync(join(outputRoot, carrier.relativePath), 'utf8'); - if (carrier.relativePath.includes('plan-eng-review/')) { + if (carrier.relativePath.includes('plan-eng-review')) { const dispatch = compactProse(content.slice(content.indexOf('**Pending-record checkpoint.**'), content.indexOf('## Scope Challenge'))); ordered(dispatch, ['AskUserQuestion({ questions: [currentDecision] })', /stop until the actual answer arrives/i, '### Record the answer', 'For the next choice']); } @@ -280,7 +280,7 @@ describe('plan report persistence precedes completion logging', () => { const dashboard = content.indexOf('\n## Review Readiness Dashboard\n'); expect({ carrier: carrier.relativePath, ordered: 0 < report && report < readback && readback < log && log < dashboard }).toMatchObject({ ordered: true }); expect(content.slice(report, readback)).toContain('current Completion Summary'); - expect(content.slice(readback, log)).toMatch(carrier.relativePath.includes('plan-eng-review/') + expect(content.slice(readback, log)).toMatch(carrier.relativePath.includes('plan-eng-review') ? /follow \*\*Blocked outcome\*\* before Review Log or decision logging/i : /stop before Review Log or decision logging/i); } diff --git a/test/retired-skills.test.ts b/test/retired-skills.test.ts new file mode 100644 index 000000000..d4f18c5d1 --- /dev/null +++ b/test/retired-skills.test.ts @@ -0,0 +1,156 @@ +/** + * GST-1698: /plan-eng-review is cut. Its source stays as /autoplan's eng-phase + * methodology, and no host registers it. setup and gstack-relink treat it as a + * disabled skill that config cannot re-enable (GSTACK_RETIRED_SKILLS), so an + * existing gstack-owned entry is removed on the next setup or upgrade and a + * user's own same-name skill stays. External hosts carry the methodology beside + * gstack-autoplan/SKILL.md as plan-eng-review.md (Codex and Copilot register + * every nested SKILL.md). + */ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { RETIRED_SKILLS } from '../scripts/discover-skills'; +import { runGeneration } from '../scripts/gen-skill-docs'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const REGISTRY = path.join(ROOT, 'bin', 'gstack-install-registry.sh'); +const BINS = ['gstack-config', 'gstack-relink', 'gstack-patch-names', 'gstack-skill-name.sh', 'gstack-state-root.sh', 'gstack-install-registry.sh']; + +test('setup and gstack-relink retire the same skills as the generator', () => { + const r = spawnSync('bash', ['-c', '. "$1" && printf %s "$GSTACK_RETIRED_SKILLS"', '_', REGISTRY], { encoding: 'utf8', timeout: 10_000 }); + expect(r.status, r.stderr).toBe(0); + expect(r.stdout.split(/\s+/).filter(Boolean)).toEqual(RETIRED_SKILLS); + expect(RETIRED_SKILLS).toEqual(['plan-eng-review']); +}); + +describe('gstack-relink on an install that still has /plan-eng-review', () => { + let tmp: string; + let install: string; + let skills: string; + const relink = () => spawnSync('bash', [path.join(install, 'bin', 'gstack-relink')], { + encoding: 'utf8', + timeout: 60_000, + env: { + ...process.env, GSTACK_HOME: path.join(tmp, 'state'), GSTACK_STATE_ROOT: '', GSTACK_STATE_DIR: '', + GSTACK_INSTALL_DIR: install, GSTACK_SKILLS_DIR: skills, GSTACK_SETUP_RUNNING: '', + }, + }); + const setRegistry = (retired: string) => { + const src = fs.readFileSync(REGISTRY, 'utf8'); + fs.writeFileSync(path.join(install, 'bin', 'gstack-install-registry.sh'), src.replace(/^GSTACK_RETIRED_SKILLS=.*$/m, `GSTACK_RETIRED_SKILLS="${retired}"`)); + }; + + beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-retired-')); + install = path.join(tmp, 'gstack'); + skills = path.join(tmp, 'skills'); + fs.mkdirSync(path.join(install, 'bin'), { recursive: true }); + for (const b of BINS) { + fs.copyFileSync(path.join(ROOT, 'bin', b), path.join(install, 'bin', b)); + fs.chmodSync(path.join(install, 'bin', b), 0o755); + } + for (const skill of ['plan-eng-review', 'ship']) { + fs.mkdirSync(path.join(install, skill)); + fs.writeFileSync(path.join(install, skill, 'SKILL.md'), `---\nname: ${skill}\ndescription: test\n---\n# ${skill}\n`); + } + }); + afterEach(() => fs.rmSync(tmp, { recursive: true, force: true })); + + test('an upgrade removes the old gstack-owned entry and keeps the methodology source', () => { + setRegistry(''); + const before = relink(); + expect(before.status, before.stderr).toBe(0); + expect(fs.existsSync(path.join(skills, 'plan-eng-review', 'SKILL.md'))).toBe(true); + + setRegistry('plan-eng-review'); + const after = relink(); + expect(after.status, after.stderr).toBe(0); + expect(fs.lstatSync(path.join(skills, 'plan-eng-review'), { throwIfNoEntry: false })).toBeUndefined(); + expect(fs.existsSync(path.join(skills, 'ship', 'SKILL.md'))).toBe(true); + expect(fs.existsSync(path.join(install, 'plan-eng-review', 'SKILL.md'))).toBe(true); + expect(after.stdout).not.toContain('Disabled (gstack-config disabled_skills'); + }); + + test('a gstack-plan-eng-review entry from a prefixed install goes too', () => { + fs.mkdirSync(path.join(skills, 'gstack-plan-eng-review'), { recursive: true }); + fs.symlinkSync(path.join(install, 'plan-eng-review', 'SKILL.md'), path.join(skills, 'gstack-plan-eng-review', 'SKILL.md')); + const r = relink(); + expect(r.status, r.stderr).toBe(0); + expect(fs.lstatSync(path.join(skills, 'gstack-plan-eng-review'), { throwIfNoEntry: false })).toBeUndefined(); + }); + + test("a user's own plan-eng-review skill is left alone", () => { + fs.mkdirSync(path.join(skills, 'plan-eng-review'), { recursive: true }); + const own = '---\nname: plan-eng-review\ndescription: mine\n---\nmy own review\n'; + fs.writeFileSync(path.join(skills, 'plan-eng-review', 'SKILL.md'), own); + const r = relink(); + expect(r.status, r.stderr).toBe(0); + expect(fs.readFileSync(path.join(skills, 'plan-eng-review', 'SKILL.md'), 'utf8')).toBe(own); + }); +}); + +describe('setup prunes external-host entries for a retired skill whose source still exists', () => { + const SETUP_SRC = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8'); + const extractFn = (name: string) => { + const start = SETUP_SRC.indexOf(`${name}() {`); + const end = SETUP_SRC.indexOf('\n}\n', start); + if (start < 0 || end < 0) throw new Error(`Could not locate ${name}() in setup`); + return SETUP_SRC.slice(start, end + 2); + }; + + test('the old gstack-plan-eng-review render and its host link are removed, live skills stay', () => { + const t = fs.mkdtempSync(path.join(os.tmpdir(), 'retired-prune-')); + try { + const src = path.join(t, 'src'), gen = path.join(t, 'gen'), host = path.join(t, 'host'); + for (const s of ['plan-eng-review', 'autoplan']) { + fs.mkdirSync(path.join(src, s), { recursive: true }); + fs.writeFileSync(path.join(src, s, 'SKILL.md.tmpl'), `---\nname: ${s}\n---\nx\n`); + } + for (const g of ['gstack-plan-eng-review', 'gstack-autoplan']) { + fs.mkdirSync(path.join(gen, g), { recursive: true }); + fs.writeFileSync(path.join(gen, g, 'SKILL.md'), `\n# ${g}\n`); + } + fs.mkdirSync(host); + for (const g of ['gstack-plan-eng-review', 'gstack-autoplan']) fs.symlinkSync(path.join(gen, g) + '/', path.join(host, g)); + const script = [ + 'set -e', 'log() { echo "$@"; }', `. "${REGISTRY}"`, + ...['_gstack_link_target_abs', '_gstack_target_is_ours', '_gstack_generated_header', '_backup_skill_md', '_cleanup_weak_dir', + '_owned_for_windows_refresh', '_skill_source_exists', '_prune_stale_generated'].map(extractFn), + `_prune_stale_generated "${src}" "${gen}" "${host}"`, + ].join('\n'); + const r = spawnSync('bash', ['-c', script], { encoding: 'utf-8', timeout: 30_000 }); + expect(r.status, r.stderr).toBe(0); + expect(r.stdout).toContain('pruned retired skill: gstack-plan-eng-review'); + expect(fs.readdirSync(gen)).toEqual(['gstack-autoplan']); + expect(fs.readdirSync(host)).toEqual(['gstack-autoplan']); + } finally { + fs.rmSync(t, { recursive: true, force: true }); + } + }); +}); + +test('no host renders a plan-eng-review skill; external hosts get the methodology beside gstack-autoplan', async () => { + const out = fs.mkdtempSync(path.join(os.tmpdir(), 'retired-gen-')); + try { + const result = await runGeneration({ host: 'all', outputRoot: out }); + expect(result.diagnostics.filter(d => d.kind === 'error'), JSON.stringify(result.diagnostics.filter(d => d.kind === 'error'))).toEqual([]); + const paths = result.artifacts.map(a => a.relativePath); + expect(paths.filter(p => /gstack-plan-eng-review\//.test(p))).toEqual([]); + expect(paths).toContain('plan-eng-review/SKILL.md'); + const external = result.artifacts.filter(a => a.host && a.host !== 'claude' && a.kind === 'skill'); + const hosts = new Set(external.map(a => a.host)); + for (const host of hosts) { + const copy = external.find(a => a.host === host && a.relativePath.endsWith('/skills/gstack-autoplan/plan-eng-review.md')); + expect(copy, host).toBeDefined(); + expect(fs.readFileSync(path.join(out, copy!.relativePath), 'utf8')).toMatch(/^---\nname: (gstack-)?plan-eng-review\n/); + } + const router = fs.readFileSync(path.join(out, 'SKILL.md'), 'utf8'); + expect(router).not.toContain('/plan-eng-review'); + expect(fs.readFileSync(path.join(out, 'gstack', 'llms.txt'), 'utf8')).not.toContain('plan-eng-review'); + } finally { + fs.rmSync(out, { recursive: true, force: true }); + } +}, 300_000); diff --git a/test/setup-kiro-native.test.ts b/test/setup-kiro-native.test.ts index 9b2da70d0..4b02dde95 100644 --- a/test/setup-kiro-native.test.ts +++ b/test/setup-kiro-native.test.ts @@ -39,6 +39,7 @@ describe.skipIf(process.platform === 'win32')('native Kiro setup installation', if (name !== 'gstack') put(path.join(root, name.replace(/^gstack-/, ''), 'SKILL.md.tmpl'), `name: ${name}`); } put(path.join(generated, 'gstack-autoplan/sections/ceo.md'), `${banner}\nKiro native phase`); + put(path.join(generated, 'gstack-autoplan/plan-eng-review.md'), `${banner}\nKiro Phase 3 methodology`); put(path.join(installed, 'gstack-autoplan/SKILL.md'), skill('gstack-autoplan', 'old Codex output')); put(path.join(installed, 'gstack-autoplan/sections/ceo.md'), `${banner}\nold Codex phase`); put(path.join(installed, 'gstack-autoplan/sections/notes.md'), 'user notes'); @@ -68,6 +69,7 @@ describe.skipIf(process.platform === 'win32')('native Kiro setup installation', expect(fs.readFileSync(path.join(installed, 'gstack/office-hours/SKILL.md'), 'utf8')).toContain('kiro native'); expect(fs.readFileSync(path.join(installed, 'gstack-autoplan/sections/ceo.md'), 'utf8')).toContain('Kiro native phase'); expect(fs.readFileSync(path.join(installed, 'gstack-autoplan/sections/notes.md'), 'utf8')).toBe('user notes'); + expect(fs.readFileSync(path.join(installed, 'gstack-autoplan/plan-eng-review.md'), 'utf8')).toContain('Kiro Phase 3 methodology'); expect(fs.readFileSync(path.join(installed, 'gstack-review/SKILL.md'), 'utf8')).toBe('a foreign review skill'); expect(fs.existsSync(path.join(installed, 'gstack/bin/tool'))).toBe(true); expect(fs.existsSync(path.join(installed, 'gstack/lib/helper.ts'))).toBe(true); diff --git a/test/shared-libs-rendering.test.ts b/test/shared-libs-rendering.test.ts index 07bde9b55..0a246d7b6 100644 --- a/test/shared-libs-rendering.test.ts +++ b/test/shared-libs-rendering.test.ts @@ -4,7 +4,7 @@ import { mkdtempSync, readFileSync, readdirSync, existsSync, rmSync } from 'node import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { ALL_HOST_CONFIGS } from '../hosts'; -import { discoverTemplates } from '../scripts/discover-skills'; +import { discoverTemplates, RETIRED_SKILLS } from '../scripts/discover-skills'; import { sharedLibsPlanExcerpt } from './helpers/shared-libs-plan-excerpt'; import { readSkillEffective } from './helpers/skill-startup'; @@ -12,6 +12,7 @@ const ROOT = resolve(import.meta.dir, '..'); let output: string; function rendered(host: (typeof ALL_HOST_CONFIGS)[number], skill: string): string { + if (host.name !== 'claude' && RETIRED_SKILLS.includes(skill)) return readFileSync(join(output, host.hostSubdir, 'skills', 'gstack-autoplan', `${skill}.md`), 'utf8'); const dir = host.name === 'claude' ? join(output, skill) : join(output, host.hostSubdir, 'skills', `gstack-${skill}`); const sections = join(dir, 'sections'); diff --git a/test/size-ledger/ship/capy-gst-1698-cut-plan-eng-review.json b/test/size-ledger/ship/capy-gst-1698-cut-plan-eng-review.json new file mode 100644 index 000000000..b0b85bdee --- /dev/null +++ b/test/size-ledger/ship/capy-gst-1698-cut-plan-eng-review.json @@ -0,0 +1,4 @@ +{ + "bytes": -21, + "why": "GST-1698: /plan-eng-review cut; the preflight DIFF hint and the no-Eng-Review line name /autoplan" +} diff --git a/test/skill-md-size-guard.test.ts b/test/skill-md-size-guard.test.ts index 44d095442..09ce7a164 100644 --- a/test/skill-md-size-guard.test.ts +++ b/test/skill-md-size-guard.test.ts @@ -15,6 +15,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { ALL_HOST_CONFIGS } from '../hosts/index'; +import { RETIRED_SKILLS } from '../scripts/discover-skills'; const ROOT = path.resolve(import.meta.dir, '..'); const ALLOWLIST_FILE = 'test/fixtures/skill-md-size-allowlist.json'; @@ -82,6 +83,11 @@ function renderAllHosts(outDir: string): Record { if (host.name === 'claude' && !fs.existsSync(`${file}.tmpl`)) continue; sizes[`${host.name}/${dir === '.' ? 'gstack' : dir}`] = countChars(fs.readFileSync(file, 'utf-8')); } + // A retired skill's methodology ships beside /autoplan; it keeps its pin. + for (const skill of RETIRED_SKILLS) { + const file = path.join(skillsRoot, 'gstack-autoplan', `${skill}.md`); + if (host.name !== 'claude' && fs.existsSync(file)) sizes[`${host.name}/gstack-${skill}`] = countChars(fs.readFileSync(file, 'utf-8')); + } } return sizes; } diff --git a/test/skill-routing-e2e.test.ts b/test/skill-routing-e2e.test.ts index 2bdc68c31..9fc2d5d72 100644 --- a/test/skill-routing-e2e.test.ts +++ b/test/skill-routing-e2e.test.ts @@ -73,7 +73,7 @@ if (evalsEnabled && process.env.EVALS_TIER) { function installSkills(tmpDir: string) { const skillDirs = [ '', // root gstack SKILL.md - 'qa', 'qa-only', 'ship', 'review', 'plan-ceo-review', 'plan-eng-review', + 'qa', 'qa-only', 'ship', 'review', 'plan-ceo-review', 'autoplan', 'plan-design-review', 'design-review', 'design-consultation', 'retro', 'document-release', 'investigate', 'office-hours', 'browse', 'setup-browser-cookies', 'gstack-upgrade', 'humanizer', @@ -241,7 +241,7 @@ describeE2E('Skill Routing E2E — Developer Journey', () => { spawnSync('git', ['commit', '-m', 'initial'], { cwd: tmpDir, stdio: 'pipe', timeout: 5000 }); const testName = 'journey-plan-eng'; - const expectedSkill = 'plan-eng-review'; + const expectedSkill = 'autoplan'; const result = await runSkillTest({ prompt: "I wrote up a plan for the waitlist app in plan.md. Can you take a look at the architecture and make sure I'm not missing any edge cases or failure modes before I start coding?", workingDirectory: tmpDir, diff --git a/test/skill-validation.test.ts b/test/skill-validation.test.ts index 8504339ba..748dd429d 100644 --- a/test/skill-validation.test.ts +++ b/test/skill-validation.test.ts @@ -10,6 +10,7 @@ import { expectMentions } from './helpers/prompt-structure'; import { generateFailureRouting } from '../scripts/resolvers/preamble/generate-test-failure-triage'; import { readShippedSkillRouting } from './helpers/shipped-skill-routing'; import { readSkillStartup } from './helpers/skill-startup'; +import { RETIRED_SKILLS } from '../scripts/discover-skills'; const ROOT = path.resolve(import.meta.dir, '..'); @@ -1873,6 +1874,7 @@ describe('Doc inventory cross-check', () => { if (!entry.isDirectory()) continue; if (entry.name.startsWith('.')) continue; if (DOC_INVENTORY_EXCLUDE.has(entry.name)) continue; + if (RETIRED_SKILLS.includes(entry.name)) continue; const tmplPath = path.join(ROOT, entry.name, 'SKILL.md.tmpl'); if (fs.existsSync(tmplPath)) dirs.push(entry.name); } @@ -1916,6 +1918,7 @@ describe('Codex skill validation', () => { if (!entry.isDirectory() || entry.name.startsWith('.') || entry.name === 'node_modules') continue; if (entry.name === 'codex') continue; // Unavailable on Codex if (entry.name === 'claude-code') continue; // Unavailable on Claude Code + if (RETIRED_SKILLS.includes(entry.name)) continue; if (fs.existsSync(path.join(ROOT, entry.name, 'SKILL.md.tmpl'))) { skills.push(entry.name); }