--- name: tickets-review-loop description: Pre-dispatch review loop for the ticket SET written by /tickets. Runs single-backend × 5-lens fan-out (Slicer / DependencyAuditor / Granularity / AcceptanceCriteria / Coverage) over every pending ticket file, plus a deterministic `## Blocked by` parser preflight (hard gate — overrides 3-of-5 progression). Loop until a deterministic 3-of-5 Progression Check + parserPass hard gate exits. Use before /tickets applies the ready-for-agent triage label. Trigger: "tickets-review-loop", "/tickets-review-loop". allowed-tools: Bash, Read, Write, Edit, Skill, Workflow --- # Tickets Review Loop Drives an adversarial multi-lens review of the ticket SET written by `/tickets` before those tickets are labelled `ready-for-agent` and picked up by `/run-all-tickets`. Workflow position: ``` /tickets step 5 writes ticket files → /tickets-review-loop → user final approval → /tickets step 6 applies triage labels ^^^ THIS SKILL ^^^ ``` This skill never publishes anything or applies labels. It loops review-and-revise on the ticket files in place, then hands back to `/tickets` (or the user) to label. `/tickets-review-loop` is the sibling of `/spec-review-loop`: same architectural shape (single backend × N-lens fan-out + 3-of-5 progression check + persisted wont-fix ledger), specialized for ticket-set defects (vertical-slice violations, undeclared dependencies, granularity drift, under-specified Acceptance Criteria, spec-coverage gaps). It adds one extra **deterministic hard gate**: every pending ticket file's `## Blocked by` section must pass the `/run-all-tickets` parser — no amount of LLM consensus can override this gate, because an unparseable ticket silently jams the entire `/run-all-tickets` drain. ## Input — two accepted forms The skill accepts the ticket set in one of two forms. Resolve which via this priority order: 1. **Feature slug** — `slug` arg or `/tickets-review-loop ` invocation (e.g. `/tickets-review-loop sessions-list-by-last-event-at`). Skill resolves to `~/.claude/matt/features//tickets/` per `~/.claude/matt/ticket-tracker.md` conventions; reads and edits ticket files in place. After EXIT, if any pending ticket is already labelled `ready-for-agent` in the parent `/tickets` flow, prompt the user to confirm the label remains valid (sync drift guard). 2. **Feature directory path** — `featureDir` arg or `/tickets-review-loop ~/.claude/matt/features/` invocation. Same behavior as form (1) — the skill computes `ticketsDir = /tickets` and `specPath = /spec.md`. There is intentionally **no inline form**. Tickets are files on disk by construction (they ship via `/run-all-tickets`'s file-iteration loop); a pre-file draft isn't a meaningful unit to review. If parent invokes the skill without either form, ask once which to use. The skill computes from the resolved path: - `ticketsDir` — directory containing pending `NN-*.md` ticket files - `specPath` — `/spec.md` (read if exists; absence is allowed and silently drops the Coverage lens) - `wontfixPath` — `/.tickets-review.wontfix.md` (sibling of `tickets/`, **not** inside it — placing the ledger inside `tickets/` would violate `/run-all-tickets`'s `^(done-)?[0-9]{2,}-[a-z0-9-]+\.md$` filename invariant) ## Architecture ``` Context preflight → List pending ticket files → Glob project context → Build wont-fix ledger │ ▼ For each round: ┌─ Workflow(review.workflow.js, mode=tickets, lensRoster=5) ────────────────────┐ │ step 1: deterministic `## Blocked by` parser preflight (HARD GATE) │ │ → parserFailures[] │ │ step 2: auto-drop Coverage lens if no PARENT_SPEC in contextFiles │ │ step 3: lens × ticket-file fan-out (5 lenses × N pending tickets, chunked) │ │ Slicer | DependencyAuditor | Granularity | AcceptanceCriteria │ │ [+ Coverage when spec injected] │ │ step 4: merge by (ticketFile, anchor) → verdict │ │ parserFailures!=0 → REJECT regardless of LLM consensus │ └───────────────────────────────────────────────────────────────────────────────┘ │ ▼ Parent edits ticket files in place (split / merge / fix Blocked-by / sharpen AC / move non-code gates to ## Hold) or marks a finding wont-fix → persists to ledger │ ▼ Workflow(progression-check.workflow.js) 3-of-4 deterministic/semi-deterministic counted criteria + 1 parent-confirmed precondition (coverage) + parserPass hard gate (independent — must be true for any EXIT path) │ ▼ EXIT? → done. CONTINUE? → next round (re-read ticket files, re-load wont-fix ledger, re-run parser). ``` ## Flow ### Step 0 — Context preflight (run ONCE per loop invocation, before round 1) Resolve `ticketsDir`, `specPath`, `wontfixPath` from the input. Then run in parallel: ```bash # Required: pending ticket files PENDING_FILES=$(find "$TICKETS_DIR" -maxdepth 1 -name '*.md' ! -name 'done-*' | sort) [ -z "$PENDING_FILES" ] && echo "❌ No pending ticket files in $TICKETS_DIR" && exit 1 # Optional: parent spec (required by Coverage lens; absence drops that lens silently) CTX_FILES=() [ -f "$PRD_PATH" ] && CTX_FILES+=("PARENT_SPEC:$PRD_PATH") # Optional: project context (same as /spec-review-loop) [ -f "$PWD/GRILLCOMMITMENTS.md" ] && CTX_FILES+=("GRILLCOMMITMENTS:$PWD/GRILLCOMMITMENTS.md") [ -f "$PWD/CONTEXT.md" ] && CTX_FILES+=("GLOSSARY:$PWD/CONTEXT.md") [ -d "$PWD/docs/adr" ] && for f in "$PWD/docs/adr"/*.md; do [ -f "$f" ] && CTX_FILES+=("ADR:$f") done # NO sibling-spec injection — ticket review is scoped to ONE feature's ticket set. # Build ONE framed context-bundle FILE (deterministic cat — keeps file CONTENT # out of the parent context AND out of per-round Workflow args; the workflow's # lenses `cat` this path). PARENT_SPEC / GRILLCOMMITMENTS / GLOSSARY full; ADR head -200. CTX_BUNDLE="" if [ ${#CTX_FILES[@]} -gt 0 ]; then CTX_BUNDLE=$(mktemp -t tickets-ctx-bundle.XXXXXX) printf 'PROJECT CONTEXT (injected — reviewer MUST cross-check tickets against these):\n' >> "$CTX_BUNDLE" for entry in "${CTX_FILES[@]}"; do label="${entry%%:*}"; path="${entry#*:}" printf '\n--- %s (%s) ---\n' "$label" "$path" >> "$CTX_BUNDLE" case "$label" in ADR) head -200 "$path" >> "$CTX_BUNDLE" ;; *) cat "$path" >> "$CTX_BUNDLE" ;; esac done printf '\n--- end project context ---\n' >> "$CTX_BUNDLE" fi # Emit path-only contextFiles + the bundle path. NOTHING in stdout contains file # CONTENT — only labels/paths — so the parent never holds (or re-passes) bodies. ctx_json=$(for e in "${CTX_FILES[@]:-}"; do [ -n "$e" ] && printf '%s\t%s\n' "${e%%:*}" "${e#*:}"; done \ | jq -R 'split("\t")|{label:.[0],path:.[1]}' | jq -s .) jq -n --argjson cf "$ctx_json" --arg bp "$CTX_BUNDLE" '{contextFiles:$cf, contextBundlePath:$bp}' ``` The bundle build above applies the tickets-mode truncation policy: `PARENT_SPEC` — **full** (Coverage lens needs the entire User Stories + Acceptance Criteria sections; truncating would silently weaken the lens); `GRILLCOMMITMENTS` / `GLOSSARY` — full; `ADR` — `head -200` per file. Capture the bash's JSON stdout: `contextFiles` (an array of `{label, path}` — **path-only, no `content`**) and `contextBundlePath` (the bundle file path, or `""` when no context). **Do NOT Read the context files into the parent** — their bodies live only in the bundle file. Remember `contextBundlePath` for cleanup in Step 3, and pass both fields to the workflow every round. If `PARENT_SPEC` is absent, log `tickets-review-loop: no PARENT_SPEC at — Coverage lens will be skipped, other 4 lenses run` and proceed. ### Step 1 — Load wont-fix ledger Look for `/.tickets-review.wontfix.md`. Schema is a markdown list: ```markdown # Wont-fix ledger for ticket review ## [W-001] Required — Slicer round 2 - **TicketFile**: 04-balance-fetch.md - **Anchor**: AC-3 - **Rationale**: AC-3 intentionally only covers the happy path; failure-mode AC lives in ticket 05-balance-fetch-failure-handling.md (referenced via ## Blocked by from 05). - **Decided round**: 2 ## [W-002] Suggestion — Granularity round 1 - **TicketFile**: GLOBAL - **Anchor**: MATRIX-SUMMARY - **Rationale**: Set-wide observation about ticket count vs. spec Story count — accepted as informational; not actionable until next spec revision. - **Decided round**: 1 ``` Parse into `wontfixLedger: [{id, severity, source, rationale, decidedRoundN, ticketFile, anchor}]`. **Both `ticketFile` and `anchor` must be present** — entries missing `anchor` are silently ineffective (the workflow returns them in `stats.ledgerEntriesSkipped` so you can see and re-author them). If the file does not exist, `wontfixLedger = []`. Pass to both the workflow and the progression-check. The ledger lives at `/.tickets-review.wontfix.md` (sibling of `tickets/`, **not** inside it). Inside `tickets/`, any non-`(done-)?NN-slug.md` file would crash `/run-all-tickets` preflight step 3 — the dot-prefixed ledger file would fail that regex. Locating it one level up keeps it completely out of `/run-all-tickets`'s scrutiny scope. ### Step 2 — Review round loop For round N (starting at 1): #### 2a. Invoke the tickets review workflow ``` Workflow({ scriptPath: '/home/fei/.claude/skills/review-with-agent/review.workflow.js', args: { mode: 'tickets', backend: 'codex', // single backend (same rationale as /spec-review-loop) ticketsDir: '', contextFiles: , // PARENT_SPEC optional; GRILLCOMMITMENTS / GLOSSARY / ADR optional contextBundlePath: , wontfixLedger: , lensRoster: ['Slicer', 'DependencyAuditor', 'Granularity', 'AcceptanceCriteria', 'Coverage'], // Workflow auto-drops Coverage from the dispatched roster when no PARENT_SPEC in contextFiles. }, }) ``` The workflow returns `{verdict, ticketFilesReviewed, parserFailures, findings, lensResults, stats}`. Findings are already merged across lenses with the `lenses: [...]` array showing which lenses flagged each one. `parserFailures` is populated by the deterministic `## Blocked by` regex preflight — these are NOT findings from LLM lenses; they are hard mechanical failures. #### 2b. Triage findings — parent decides per finding For each finding, decide one of: - **Revise** — edit the ticket file in place to address the finding (Edit tool on the ticket file). Possible edits include: | Decision | Mechanics | Round-N+1 effect on signals | |---|---|---| | Split ticket | Create new `NN+1-.md` (renumber subsequent files if needed), move content, update any `## Blocked by` references that pointed to the original | Bumps ticket-file count → trips `positionStability` to false | | Merge two tickets | Delete one, fold content into the other, rewrite `## Blocked by` of every dependent that pointed to the removed one | Bumps `|removed|` → trips `positionStability` | | Fix `## Blocked by` line | Replace malformed bullet with backtick form `` - `NN-slug.md` `` or hash form `- #NN` | May flip `parserPass` from false → true | | Move non-code gate to `## Hold` | Cut the offending bullet out of `## Blocked by`, paste into a new `## Hold` section in the same ticket (or out of the ticket entirely) | Flips `parserPass` to true for that file | | Inline body edit (sharpen AC, rewrite `## What to build`) | Edit bullets / prose in place | Body-byte delta only; no file churn | - **Wont-fix** — append a new entry to `/.tickets-review.wontfix.md` with `id = W-` (next available), the finding's `ticketFile` + `anchor` (BOTH required), source = the lens(es) that flagged it, current round, and parent's structured rationale. **Persisted to disk** — survives loop / session / cross-day. Prioritize multi-lens findings (`lenses.length >= 2`) — they are by construction the highest-confidence signal. **Treat `parserFailures` specially**: these are NOT triageable as wont-fix. The author MUST fix them (edit the offending `## Blocked by` line per the parser regex, OR move the non-code gate to `## Hold`). The loop cannot EXIT while `parserFailures.length > 0`. #### 2c. Compute progression Capture round-state for the progression check: ```bash # Pending ticket file list (after edits) ISSUE_FILES=$(find "$TICKETS_DIR" -maxdepth 1 -name '*.md' ! -name 'done-*' -exec basename {} \; | sort) # Sum of non-whitespace bytes across all pending tickets — null-safe in empty dir TOTAL_BYTES=$(find "$TICKETS_DIR" -maxdepth 1 -type f -name '*.md' ! -name 'done-*' -print0 \ | xargs -0 cat 2>/dev/null | tr -d '[:space:]' | wc -c | tr -d ' ') ``` The `find ... -print0 | xargs -0 cat` form is null-safe — `xargs -0` with no input is a no-op (does not invoke `cat`), so the pipeline returns 0 bytes cleanly when the directory is empty. The bare `cat $(find ...)` form is unsafe: an empty `find` makes it `cat` (no args) which blocks on stdin forever. ``` Workflow({ scriptPath: '/home/fei/.claude/skills/tickets-review-loop/progression-check.workflow.js', args: { findings: , roundNum: , priorRoundFindingCount: , priorRoundAnchors: ::">, wontfixLedger: , thisRoundTicketFiles: , priorRoundTicketFiles: , thisRoundTotalBytes: , priorRoundTotalBytes: , parserFailureCount: , escapeHatchAlreadyUsed: , parentConfirmsCoverage: , }, }) ``` **Coverage confirmation**: workflow returns `parentMustConfirm.coverage` listing `::` keys still open this round and NOT in wont-fix. Parent inspects: - If every prior-round anchor was either edited away OR moved to wont-fix → set `parentConfirmsCoverage: true` and re-invoke the progression check. - Otherwise → loop back to 2a (still-open anchors will resurface). #### 2d. Act on verdict - `verdict === 'EXIT'` → proceed to Step 3 (Final). - `verdict === 'EXIT_VIA_ESCAPE_HATCH'` → permitted **only once per loop invocation**, **only at roundNum ≥ 3**, **only when zero effective Blockers remain**, AND **only when `hardGates.parserPass === true`** (the parser gate has no escape). Coverage is NOT a hard precondition for the escape hatch — its purpose is to let the parent exit when remaining tickets are judged not worth fixing (which often includes ambiguity about whether prior anchors are "fixed"). Parent MUST emit a structured rationale (`{remainingIssues: [...], rationaleForExitAnyway: '...'}`) and log it to the final summary. Set `escapeHatchAlreadyUsed = true` for the rest of this loop. Proceed to Step 3. - `verdict === 'CONTINUE'` → loop back to 2a (round N+1). Persist `thisRoundTicketFiles / TotalBytes / FindingCount / Anchors` as next round's prior state. #### 2e. Per-round trace The progression-check workflow returns `trace: {round, verdict, criteriaFired, criteriaState, preconditions, hardGates}`. At the end of every round, **print this trace verbatim to the user** so the EXIT decision is auditable: ``` tickets-review-loop round 2 → CONTINUE criteriaFired: [diminishingSeverity, minimumRounds] (counted 2 of 4) criteriaState: {diminishingSeverity: true, positionStability: false, minimumRounds: true, reviewerAcknowledgment: false} preconditions: {coverage: null, parserPass: false} ← either being false blocks normal EXIT hardGates: {parserPass: false} ← THIS also blocks the escape hatch effective findings this round: 4 (1 Blocking, 2 Required, 1 Suggestion) parser failures this round: 1 (03-cleanup.md: Blocked by line "- Phase 4 rollout signal" is unparseable prose) re-raised despite wont-fix: 0 ``` ### Step 3 — Final summary First, clean up the Step 0 context bundle (a tmpfile, no longer needed): `rm -f "$CTX_BUNDLE"` (skip when `contextBundlePath` was `""`). Run this BEFORE emitting the summary so the summary stays the final message. Emit the loop summary as the standalone final message (no other content before or after): ``` tickets-review-loop: EXIT after N rounds Feature: Pending tickets reviewed: (.md filenames>) Wont-fix ledger: entries (persisted at /.tickets-review.wontfix.md) Parser preflight: PASS (0 failures) Exit path: normal | escape-hatch (rationale: ...) Remaining unresolved findings: Context cross-checked: , or "none" Coverage lens: enabled (spec: ) | skipped (no PARENT_SPEC) Per-round trace: round 1: criteriaFired=[...] preconditions={coverage,parserPass} hardGates={parserPass:true|false} round 2: criteriaFired=[...] preconditions={coverage,parserPass} hardGates={parserPass:true|false} ... Next step: user reviews the tickets, then either re-invokes /tickets step 6 to apply triage labels (ready-for-agent / ready-for-human) or requests further changes. ``` After this summary, the loop is done. Do NOT apply triage labels; do NOT modify any `Status:` line inside an ticket file. That is `/tickets` step 6's job. ## Rules - **Manual only.** Never invoke automatically. - **Parent does the editing.** This skill never modifies ticket file content; it only orchestrates reviews and surfaces findings + parser failures. The parent uses Edit/Write on ticket files between rounds (split / merge / fix Blocked-by / sharpen AC / move non-code gates). - **Single backend × multi-lens.** Use one backend (default `codex`) with the 5-lens roster (4 lenses if no PARENT_SPEC — Coverage drops). Same rationale as `/spec-review-loop`: orthogonal lens perspectives outperform same-prompt dual-backend voting at proportional cost. - **`parserPass` is a HARD GATE — no LLM override.** Per `~/.claude/injected-rules/model-judgment-only.md`, the `## Blocked by` regex is deterministic and runs in `review.workflow.js` Bash, not as an LLM lens. The loop CANNOT EXIT (normal OR escape-hatch) while any pending ticket file fails the parser. Even 4-of-4 counted criteria + coverage passing does not override this gate. - **Won't-fix is persisted to disk.** `/.tickets-review.wontfix.md` survives sessions. Entries never deleted; rationales become the audit trail for the ticket review. **`parserFailures` are NOT wont-fix-able** — they must be physically fixed (edit the offending line or move it to `## Hold`). **Wont-fix entries must include both `ticketFile` and `anchor`** — entries missing `anchor` are silently ineffective; the workflow surfaces them in `stats.ledgerEntriesSkipped`. - **No new ticket files between rounds without rationale.** Splitting an ticket is a legitimate response to a Granularity / Slicer finding — but the new file must be backtick-referenced from any dependent's `## Blocked by`, and the next-round `parserPass` check must still pass. - **No code-only steps.** Unlike `/finalize`, there is no `/simplify` or `/e2e-verify` here — tickets are documents (specifications), not running code. The verification is structural (parser + lens checklist + progression criteria), not behavioral. - **Escape hatch is bounded.** At most one use per loop, only at round ≥ 3, only with zero unresolved Blockers, only with `parserPass === true`, only with structured rationale logged. ## Why these criteria + preconditions + hard gate Adapted from `~/.claude/skills/spec-review-loop/progression-check.workflow.js` (which inherits the 3-of-5 pattern from `~/.claude/skills/finalize/progression-check.workflow.js`), but specialized for ticket sets and split into three groups so the EXIT contract is unambiguous: **Counted criteria** (4 — 3-of-4 must be satisfied): 1. **Diminishing severity** — deterministic. Effective-finding Blocker ratio < 0.2 (or zero findings). 2. **Position stability** — semi-deterministic. **Ticket-file churn ≤ 1 added/removed AND total body-byte delta < 30%.** This differs from spec-loop (which checks H2 section churn): for tickets, the structural signal is the ticket SET shape — a split/merge between rounds is real position churn even though it's the right response to a Granularity finding. Null on round 1. 3. **Minimum rounds** — deterministic. `roundNum >= 2` OR (`roundNum === 1` AND zero effective findings AND `parserPass === true` — first-round-LGTM shortcut, only fires when the initial ticket set was clean on both axes). 4. **Reviewer acknowledgment** — deterministic. Aggregated effective Blocker count across lenses === 0. **Preconditions** (BOTH must hold for normal EXIT): 5. **Coverage** — parent-confirmed. Every prior-round `::` either fixed or moved to wont-fix. Workflow surfaces still-open anchors as evidence. 6. **parserPass** — deterministic hard gate. Every pending ticket file's `## Blocked by` section accepts under `/run-all-tickets`'s parser (backtick form / hash form / `[Nn]one` shortcut; referenced blocker NNs must exist). Normal **EXIT** requires `satisfied >= 3` of the 4 counted criteria **AND** `coverage === true` **AND** `parserPass === true`. **Escape hatch** (`EXIT_VIA_ESCAPE_HATCH`) — at most one use per loop, only at roundNum ≥ 3, only with zero unresolved Blockers, only with `parserPass === true`, and parent MUST log a structured rationale. **Coverage is NOT a hard precondition for the escape hatch** — its whole purpose is to let the parent exit when remaining tickets are judged not worth fixing (which often includes ambiguity around whether prior-round anchors are "fixed"); the rationale is the audit trail. ### Why `parserPass` is a hard gate, not the 5th counted criterion `parserPass` is structurally different from the 4 counted criteria: - The counted criteria measure **review progress** — are the findings shrinking? Did churn settle? Did the reviewer give an LGTM? - `parserPass` measures a **downstream hazard** — will `/run-all-tickets` accept this file? If `parserPass` were the 5th counted criterion in a 3-of-5 vote, the loop could EXIT with `parserPass === false` as long as 3 of the other 4 are satisfied. That would be wrong: any unparseable `## Blocked by` line silently stops the entire `/run-all-tickets` drain at preflight step 8 (per `/run-all-tickets` SKILL.md:28). One bad bullet in one ticket file blocks every queued ticket from that point. The cost of letting `parserPass === false` slip past EXIT vastly exceeds the cost of forcing one more round to fix the bullet. So: `parserPass` is computed deterministically and surfaced separately as `hardGates.parserPass` (also visible in `preconditions.parserPass`). It must be `true` for any EXIT path. There is no "the LLM judges this parser failure spurious" override — the parser is the source of truth `/run-all-tickets` actually uses, so it is the source of truth here too. ### Why `coverage` is a precondition for normal EXIT but not for the escape hatch `coverage` is parent-confirmed (the workflow can never auto-judge "did the author fix the prior anchor?"). For the normal EXIT path, requiring confirmation prevents premature exit while real findings sit open. For the escape hatch, requiring confirmation would defeat its purpose: the hatch exists precisely for cases where the parent has stared at remaining findings and decided they're not worth another round — and that decision often spans "I'm not 100% sure I 'fixed' that anchor, but I'm sure it's not worth iterating". The structured rationale documents the call.