Repository navigation
feat: cut standalone /plan-eng-review; /autoplan keeps its eng phase (GST-1698) - #4108
Merged
Merged
Conversation
…(GST-1698) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
|
😎 This pull request was merged. |
… pins measured beside /autoplan Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
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>
…g claim to ./setup Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
…on (journey-plan-eng) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This was referenced Nov 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
/plan-eng-reviewno longer installs as a standalone skill on any host. Its methodology stays inplan-eng-review/and still runs as/autoplan's Phase 3, byte for byte.scripts/discover-skills.tsexportsRETIRED_SKILLS = ['plan-eng-review'].setup,bin/gstack-relinkandbin/gstack-install-registry.shtreat it as disabled and can't re-enable it. The next./setupor upgrade removes a gstack-ownedplan-eng-review/gstack-plan-eng-reviewlink, and leaves a user's own same-name skill alone.disabled_skillsoutput and counts show only what the user disabled.gstack-autoplan/plan-eng-review.mdinstead of a nestedSKILL.md, because those hosts register every nestedSKILL.md. It has noagents/openai.yamland no hoisted preamble. On all 11 external hosts the file is byte-identical to main'sgstack-plan-eng-review/SKILL.md.composition.tsandgstack-autoplan-snapshotresolve that path. Kiro installs copy files one by one, sosetup's Kiro loop now also carries generated top-level siblings ofSKILL.md, which putsplan-eng-review.mdbesidegstack-autoplan/SKILL.mdthere too (a /ship review fix, 692c8e6)./autoplan: the router,CLAUDE.md,gstack-skill-startrouting, the office-hours handoff, and the CEO, design and devex chaining (which no longer says "required gate"). So do the qa, retro, spec and test-audit mentions, the review dashboard's Eng Review row, and/ship's over-200-lines hint./autoplanadds "Next Steps — Review Chaining" to its skip list so it doesn't recurse. Its guard-failure fallback now names/plan-ceo-reviewthen/plan-devex-review.journey-plan-eng(periodic, kind rule) failed twice at 8cadbdc with 0 Skill calls./autoplan's description now names the eng phase's architecture, edge cases, failure modes and tests, and its template adds the triggers "review the architecture" and "engineering review". The wording is shorter, soautoplan/SKILL.mddoesn't grow: the 12gstack-autoplanpins intest/fixtures/skill-md-size-allowlist.jsonwent down (11 hosts by 2 chars, hermes by 51). The renders andgstack/llms.txtare regenerated, and the changelog fragment gained one clause.docs/skills.mdreplaces the section with a "cut" note that carries the eval numbers. README, AGENTS.md, hosts.md, PROJECT_STRUCTURE, OPENCLAW, voice-input, troubleshooting and ARCHITECTURE are updated, and there's a CHANGELOG fragment, which now says./setupno longer installs the skill on any host. TODOS.md files the review's follow-ups.Why
Two evals found no gain from the standalone skill. In the seeded-flaw panel, plain Opus 5.5 found the planted flaws in 15 of 16 reviews. In the wave 1 outcome test, it built 22 of 22 features, and a session told to use the skill cost 7.1x a plain one (GST-1218, gstack-evals#253). Garry approved the cut on GEVA-2's R41 list ("A1 cut /plan-eng-review"). On GST-1698 he chose options A plus B, which mean an unregistered methodology source, retirement through
disabled_skills, and an external copy besidegstack-autoplan.Blast radius
/autoplanPhase 3 is unchanged. In a fixed-plan fixture (gstack-autoplan next --full --ui --developer-toolon the simulated Capy host), the engmethodology.mdsha539f595b…is identical on main and this branch. The only difference inmethodology.jsonis the envelope byte offsets, because the checkout root paths differ by 17 bytes. The per-source sha values are equal. The CEO, design and devex methodologies differ only in the redirected chaining and dashboard lines./shipgating is unchanged. Eng Review still acceptsreviewor autoplan'splan-eng-reviewlog row, and a missing standalone skill never blocks.plan-eng-review/from the source tree, so the methodology cases keep testing what Phase 3 loads.shipat −21 bytes. No ratchet, budget or threshold was loosened../setup --host kiroin a fixture HOME left~/.kiro/skills/gstack-autoplan/with onlySKILL.mdandsections/, so/autoplanPhase 3 pointed at a missing file. After it, the same install hasplan-eng-review.mdlinked to the render (exploratory QA, captures below). The new loop uses the same ownership guards as the existingsections/loop, so a user's own file or a foreign link next toSKILL.mdis kept.Tests
test/retired-skills.test.tscovers stale-link removal by relink (plain and prefixed) and by setup on external hosts, a user-owned same-name skill left alone, the shared retired list in setup/relink and the generator, and the external render path. It fails first on main and passes on this branch.Approvals
Approved: yes (record r-9500a191f09b). The full approval record stays private: this repository is public or its visibility is unknown.
Summary
Two evals showed the standalone
/plan-eng-reviewadded nothing, so it stops installing as a skill and/autoplankeeps running its methodology as Phase 3, byte for byte. Everything that used to send people to it (the router, the office-hours handoff, the CEO, design and devex chaining, the dashboard and/ship's big-diff hint) now points at/autoplan, and existing installs lose the old entry on the next setup. The /ship review caught that Kiro copies skills file by file and would have lost the Phase 3 file, so that is fixed here with a failing-first test.Test Coverage
test/retired-skills.test.tscovers stale-link removal by relink (plain and prefixed) and by setup on external hosts, a user-owned same-name skill left alone, the shared retired list in setup/relink and the generator, and the external render path. It fails first on main and passes on this branch.test/setup-kiro-native.test.tsnow seeds a generatedgstack-autoplan/plan-eng-review.mdand asserts the Kiro install carries it. Regression proof: fails at 0124a2c (0 pass, 2 fail) · passes after fix (with retired-skills and setup-prune-stale-generated: 12 pass, 0 fail).Tests: 1746 → 1746 (+0 new files; 1 existing test extended)
Coverage: 33% value-weighted (67% including 1 weakly covered path)
Test value: 0 tests written, 1 rejected by the authoring gate, 1 existing test extended, 1 path weakly covered (weak = ★, gate-failing or unrated).
Proposed tests (filed in TODOS.md):
generateLlmsTxtlists noplan-eng-reviewentry (protects: a retired skill never appears in llms.txt; fails when the RETIRED_SKILLS filter is removed);HYBRID_SKILLSholds none ofRETIRED_SKILLS(protects: OpenClaw hybrid mode never registers a retired skill). Rejected:lib/ship-preflight.tsDIFF hint (prose-only string change).Tests ran: test carried from CI (free-tests.yml at the final head; see the receipt)
Pre-Landing Review
Pre-Landing Review: 8 issues — 1 fixed (CRITICAL
setup:4018Kiro installs lacked /autoplan's Phase 3 file; fixed in 692c8e6, fix-check clean), 2 skipped, 1 deferred, 4 noted.lib/harness-register.ts:236dots/generic harness still register the retired skill (re-raised in pass 2 and deferred below);scripts/gen-skill-docs.ts:888external-host preamble pointer names the pruned directory (fixing it changes Phase 3 bytes, which this PR pins byte-identical to main).REVIEW_LOOP: passes=2 blocking_found=2 deferred=1 noted=0 fixed=1 scope=frozen ledgers_read=3 fix_checks=1 dispatches=2
Gate edits: none detected (52 listed, 35 read; patterns: RH-1,RH-3,RH-4,RH-12,RH-13,RH-14,RH-15,RH-16; unlisted idioms: none)
Outside review: openai/gpt-6.1-sol (host subagent), completed: request changes for the dots/generic harness registration (labelled blocking). The lead kept its CRITICAL label; review scope classified it outside the diff and deferred it, and by the owner's blocking floor it fails no test, breaks no install and loses no data, because the retired skill's files still exist and still work. Native adversarial: ship as-is, four non-blocking notes.
Deferred findings
Blocking findings in code this PR did not change (review scope: frozen); filed as issues and not closed by this PR.
lib/harness-register.ts:236dots and generic (claude-render) harness targets still register and, in all mode, install the retired plan-eng-review skill. Not filed as a GitHub issue (unattended run; crew rules keep gh writes off garrytan's identity); tracked in TODOS.md "P2: /plan-eng-review retirement follow-ups".Review notes
Below the blocking floor (advisory, informational, test-gap or wording items); recorded here and not fixed in this PR.
lib/harness-register.ts:236dots and generic harness targets still register plan-eng-review (same as outside-voice#1); Blocks: noscripts/gen-skill-docs.ts:888external-host plan-eng-review.md shared-preamble pointer names the pruned gstack-plan-eng-review directorybin/gstack-doctor-components.sh:1gstack-doctor --for plan-eng-review now exits 2 unknown skill with no redirect to /autoplanhosts/qm.ts:1QM edition still builds gstack-plan from plan-eng-review sources (product call)autoplan/SKILL.md.tmpl:10(scoped re-check of 6cfe8b6) the new "review the architecture" and "engineering review" trigger phrases sit only in the template's long description; the rendered frontmatter keeps the first sentence, so routing rests on its "eng (architecture, edge cases, failure modes, tests)" wording, whichjourney-plan-engpassed at 6cfe8b6; Blocks: noExploratory QA
Surface: functional (
./setup --host kiroin a fixture HOME). Pass 1 at 0124a2c: the install held onlySKILL.mdandsections/besidegstack-autoplan(PHASE3_MISSING), confirming the finding; after the fix the replay linkedplan-eng-review.mdto the render, and a second run kept it. That pass ended inconclusive because its pre-fix captures were superseded. Pass 2 at 692c8e6 on a fresh fixture: pass (nogstack-plan-eng-reviewentry,plan-eng-review.mdpresent). Not probed: other external hosts, which link whole skill directories and are covered bytest/retired-skills.test.ts.Design Review
No frontend files changed — design review skipped.
Eval Results
Evals: none run here. The change moves skill prose and generated renders; /autoplan Phase 3 bytes are unchanged (see Blast radius), and paid evals are the Captain's gate.
journey-plan-eng(skill-routing-e2e, periodic, kind rule): FAIL 0/1 twice at 8cadbdc (session completed with 0 Skill calls, so the routing check failed), then PASS 1/1 at 6cfe8b6 after the routing fix ($0.06, reported by the lane). Command:bun run scripts/test-paid-shards.ts --tier periodic --case journey-plan-eng.Review bot comments
No review bot comments.
Scope Drift
Scope Check: CLEAN
Plan Completion
Plan completion audit: not run (no plan is bound to this branch and no docs/designs/ file matches). Fix: add "Plan: " to the PR body, or run /autoplan.
TODOS
No TODO items completed in this PR. Added "P2:
/plan-eng-reviewretirement follow-ups" (five items from this review).Documentation
Status: current. No authored-doc edits were needed.
Scope: base e325b6d through head 692c8e6 (3 commits, GST-1698, which cuts standalone /plan-eng-review), plus the unstaged changes to TODOS.md and the changelog fragment. Discovery covered tracked and new .md, .tmpl and .txt docs, searched for every remaining /plan-eng-review reference.
Documentation health:
Debt: no coverage gaps or diagram drift found. The open retirement follow-ups are already filed in TODOS.md (P2/P3).
Test plan
bun test --timeout=30000 test/setup-kiro-native.test.ts test/retired-skills.test.ts test/setup-prune-stale-generated.test.ts: 12 pass, 0 failautoplan/SKILL.mdandgstack/llms.txtas regenerated outputs, the carry packet dropped those two hunks to 28 lines (8ca04233), every line of which was in the packet the reader cleared. Its focused checks: skill-md-size-guard, catalog-budget, context-budget-ratchet, eng-review-routing, catalog-trim, instruction-facts and retired-skills all green (65 tests, no failures), and parity-suite green (2 tests);gen:skill-docs --dry-runfresh.bun run test: carried from CI at the final head (free-tests.yml; run id in the receipt)🤖 Generated with Claude Code