feat(skills): /execute — issue → prime → plan → implement, gated (#207) - #237
Conversation
Add .claude/skills/execute/SKILL.md, a thin conductor that chains the existing pipeline for ONE issue: /prime (load only) -> grill-me -> /plan -> [HARD GATE: approve] -> /implement. Every stage boundary is a blocking confirmation gate; nothing auto-advances and nothing implements without an explicit approve. References the chained commands and skill by invocation name; re-implements none of their logic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean addition. The /execute skill is a well-designed thin conductor — it references /prime, grill-me, /plan, and /implement by invocation name without reimplementing any of their logic, the three hard gates are internally consistent with explicit default-deny semantics, and the 0/1/≥2 input-token routing is correct. No parity concern (tooling-only change), no dead code, no workarounds, no hard-rule violations. Dependency on #206 is called out clearly. Good to merge after #206 lands.
There was a problem hiding this comment.
Code Review: PR #237
Scope: PR #237 — feat(skills): /execute — issue → prime → plan → implement, gated
Recommendation: NEEDS WORK
Summary
PR #237 adds `.claude/skills/execute/SKILL.md` — a thin conductor skill that chains `/prime` → `grill-me` → `/plan` → `/implement` with a hard blocking gate at every stage boundary. The design intent is sound and the gate logic is well-specified. Two Medium findings need resolution: `grill-me` is not present as a registered skill in the current skills directory (making Stage 2 unresolvable unless it ships with or before this PR), and the GATE 2 exit condition is ambiguously specified in the prose vs the response table. All other dimensions are N/A or clean.
Findings
Critical
None.
High
None.
Medium
[MEDIUM] grill-me skill does not exist in the skills directory
· dimension: Correctness (#1)
· location: `.claude/skills/execute/SKILL.md:45-47`
· issue: Stage 2 instructs the model to "invoke the `grill-me` skill against the primed issue context," but `grill-me` does not appear in `.claude/skills/`. The current registered skills are `prime`, `plan`, `implement`, `validate`, `pr-review`, `add-`, `create-`, `android-generate`, and `prd-interactive`. The PR body states a dependency on #206 for migrating `grill-me` to skill form, but #206 does not resolve as an open PR in this repo — the recent merge #236 (which migrated slash-commands to skills) does not appear to have included a `grill-me` skill file.
· risk: Any invocation of `/execute` that reaches Stage 2 will stall or confuse the model. The core differentiating value of `/execute` (gated grilling before planning) is broken from day one.
· fix: Either (a) ship a `grill-me` skill at `.claude/skills/grill-me/SKILL.md` in this PR or confirm it landed in #236, (b) update Stage 2 to reference the correct registered name if it was renamed, or (c) inline a minimal grill interaction directly in `/execute` as a fallback. Update the PR description to confirm the dependency is satisfied before merge.
· reference: CLAUDE.md rule 1 (root-cause, not workaround); CLAUDE.md rule 8 (surface errors at boundaries — a missing dependency is a hard boundary failure)
[MEDIUM] GATE 2 exit condition: prose and table contradict each other
· dimension: Correctness (#1)
· location: `.claude/skills/execute/SKILL.md:47, 85-92`
· issue: Prose says "The user exits the grill loop explicitly. That exit is GATE 2" — implying exiting `grill-me` automatically constitutes GATE 2 `proceed`. But the GATE 2 response table then requires an explicit `proceed` / `more` / `abort`. These are contradictory: if finishing grilling is the gate, `more` in the table is unreachable; if the gate requires an explicit `proceed`, the prose is wrong.
· risk: The model may interpret finishing grilling as automatic `proceed` and skip the gate — violating the hard-gate, default-deny invariant that is the design's core guarantee. A user who expects to review the grilling summary before planning gets no checkpoint.
· fix: Align prose and table. Recommended: change the prose to "When `grill-me` ends, `/execute` presents the resolved-decisions summary as GATE 2 and waits for an explicit response." Remove the ambiguous "That exit is GATE 2" sentence.
· reference: CLAUDE.md rule 1 (the ambiguity is in the spec — fix it there)
Low / Info
[INFO] Invocation style inconsistency — `grill-me` vs `/slash` prefix
· dimension: Correctness (#1)
· location: `.claude/skills/execute/SKILL.md:45-47`
· issue: `/prime`, `/plan`, and `/implement` are referenced with their slash prefix throughout. `grill-me` appears without a slash. Skills registered in `.claude/skills/` are invoked via the Skill tool. An inconsistent reference style (once as bare name, once as slash-prefixed) may cause the model to use a different invocation path than intended.
· risk: Low — cosmetic unless `grill-me` has a different invocation mechanism than the other three.
· fix: Decide on `grill-me` vs `/grill-me` and use it consistently. Match whatever invocation style the skill itself documents.
· reference: Internal consistency
[INFO] Stage 4 "(path-based mode)" annotation is correct — no action needed
· dimension: Correctness (#1)
· location: `.claude/skills/execute/SKILL.md:59`
· issue: The annotation "(path-based mode)" on the `/implement` call is accurate: `.claude/plans/issue-.plan.md` is a single non-numeric whitespace-delimited argument, so implement's mode detection correctly selects path-based (0 numeric tokens). Note for reviewers only.
· risk: None.
· fix: No action required.
· reference: Cross-check against `/implement` mode detection table
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | N/A — change is under `.claude/skills/`, not `apps/*` |
| i18n-syncer | N/A — no user-facing strings added |
| contract-aligner | N/A — no types, endpoints, or DTOs changed |
| security-reviewer | N/A — no `orbit-api` code changed |
Validation
| Check | Result |
|---|---|
| Lint | N/A — `.md` file; no TypeScript/C# to lint |
| Type check | N/A — no source files changed |
| Tests | N/A — skills carry no unit-test harness (noted in PR body) |
| Build (api) | N/A — no API changes |
What's good
- Thin conductor by design. Imports no logic from the four commands it chains and explicitly forbids re-describing them. Changes to `/prime`, `/plan`, etc. apply automatically.
- Hard gate, default-deny throughout. All three gates specify that no response or ambiguous response causes a re-ask, never an auto-advance. GATE 3's explicit `approve`-only entry to implement is especially clean.
- Input validation table is precise. The 0 / 1 / ≥2 token table with the STOP for multi-issue correctly routes to the existing parallel flow without trying to replicate it.
- No double push gate. Phase 7 of `/implement` is the final checkpoint; `/execute` deliberately delegates it rather than duplicating.
- Frontmatter is complete. `name`, `description`, and `argument-hint` are all present and formatted for skill-menu discoverability.
- Abort handling at every gate. Each gate specifies the stopped stage and how to resume — good UX for a multi-stage pipeline.
Recommendation
Two Medium findings need resolution before merge:
-
Confirm `grill-me` exists as a registered skill (or ship it in this PR). If it was supposed to land in #236 and didn't, a `.claude/skills/grill-me/SKILL.md` file is the fix. If it was deferred, the Stage 2 description must gracefully handle its absence.
-
Resolve the GATE 2 prose vs table contradiction. Make the gate always explicit and update the prose to match the table.
The Low/Info items are optional cleanup. Once the two Mediums are addressed, this is clean to merge.
🤖 Generated with Claude Code
|
1 participant
What
Adds
.claude/skills/execute/SKILL.md— the/execute <issue>orchestrator. A thin conductor that drives one issue from its number to a PR by chaining the existing pipeline, with a hard confirmation gate at every stage boundary.Design
/prime,grill-me,/plan, and/implementby invocation name only — re-implements none of their logic. The only hardcoded coupling is the stable single-issue plan path.claude/plans/issue-<N>.plan.md./primestays load-only (AC3). Stage 1 stops at the prime summary; the only path forward is GATE 1. Cites the standingfeedback_prime_loads_context_only.mdrule.approve—revise <feedback>loops back to/plan, never falls through to implement./executeowns gates 1–3; the push/PR confirmation inside/implement(its Phase 7) stays the final checkpoint and is not duplicated here.≥2numeric tokens → STOP and redirect to the existing/prime <N…>parallel multi-issue flow;0→ ask for an issue number.Frontmatter
SKILL-style
name: execute+description:(so it lists in the skill menu and is invocable via the Skill tool), plus anargument-hint: <issue-number>line for discoverability.Dependency & merge order
Depends on #206 (which migrates
/prime,/plan,/implement, andgrill-meinto skill form). This PR references those by their stable invocation names and should merge after #206.Validation
No automated tests — skills carry no unit-test harness and the integration/E2E suites were removed. Validated by inspection: file at the mandated path,
name: execute, frontmatter parses, references the pipeline by invocation name (no.claude/commands/path refs), and a coherent 3-gate flow that maps 1:1 to AC1/AC2/AC3. A manual dry-run on a small real issue is recommended.Closes #207
🤖 Generated with Claude Code