From d866f85eeb1557d0acc162dd9a1dd0d1393a36b1 Mon Sep 17 00:00:00 2001 From: Peyman Date: Mon, 6 Jul 2026 01:27:11 +0200 Subject: [PATCH 1/4] feat(skill): add Anchor primitive, Task archetype, and two-mode operation; fix validator correctness Retarget the skill around the pipeline that consumes its output. Every story and task now names one machine-resolvable Anchor (file:line, file:symbol, module.function, METHOD /path -> status, or playwright:selector) that the held-out probe binds to. Acceptance must be runnable headless against the checkout and may not stub the thing under test; soaks and dashboards move to a Post-ship follow-up line. Add a lightweight Task archetype (Type: Bug/Chore/Spike/Ops -> Anchor / Problem / Verification) so a one-line bug is no longer inflated into a Story or filed as a one-issue Hardening ticket. Add a story Out-of-scope non-goal and a per-issue hardening Verify line. Split the flow into interactive and headless modes: headless drops the per-issue independent reviewer and the approval ceremony but keeps Gate 1 mandatory before every save (a save-hook enforces this at the tool boundary). Validator correctness: - epic-backlink demoted to WARN (Epic: none sentinel); it was the top fail-loop - labels-verify WARN deleted (undischargeable noise on every block) - vague-verb requires a metric adjacent to the verb, not anywhere on the line - code-fence and inline-code aware: quoted headings/identifiers no longer trip - --- splitting is fence-aware and merges title-less tails, so a mid-body rule can no longer drop the tail of an issue from validation - epic-story-count counts any child bullet; role-titled children exempt from em-dash - estimate is a save-time Linear field, not a body mandate; priority enum reconciled Ruff clean, 28 validator tests pass. --- CHANGELOG.md | 21 +- README.md | 21 +- skill/SKILL.md | 62 ++-- skill/prompts/brief-to-epic.md | 9 +- skill/prompts/hardening-issue.md | 8 +- skill/prompts/refactor-story.md | 10 +- skill/references/01-title-patterns.md | 15 +- skill/references/02-description-templates.md | 59 +++- skill/references/03-tone-guide.md | 7 +- skill/references/05-anti-patterns.md | 18 +- skill/references/06-canonical-examples.md | 53 +++- skill/review/issue-reviewer.md | 39 ++- skill/scripts/tests/test_validate.py | 269 +++++++++++++++- skill/scripts/validate_issue.py | 307 +++++++++++++++---- 14 files changed, 764 insertions(+), 134 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a92e03..0a0bd26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,26 @@ ## Unreleased -### Added +### Added (Anchor + validator hygiene) +- **Anchor field.** Every story and task now carries a `**Anchor:**` line: one machine-resolvable target (`file.ext:line`, `file.ext:symbol`, `module.function`, `METHOD /path -> status`, or `playwright:selector`). It is the interface the downstream held-out probe binds to; a probe cannot test a thing the issue never named. The validator WARNs on a missing or prose-shaped anchor; a save-hook (separate increment) promotes it to a create-only ERROR. +- **Task archetype** for a single bug, chore, spike, or ops change too small for the full story ceremony. Selected with `**Type:** Bug|Chore|Spike|Ops` (or a title prefix); shape is Anchor / Problem / Verification. Fixes the coverage gap where a one-line bug had to be inflated into a Story or mis-filed as a one-issue Hardening ticket. New title pattern (6) and canonical Example 5. +- **Compiler-input framing** in `SKILL.md` and README: the issue is read by an autonomous pipeline (planner, held-out probe, implementer, reviewer, merge gate), so concreteness is the product and prose is not. +- **Two-mode operation.** Interactive mode keeps the three gates and human approval. Headless / pipeline mode (a subagent filing a batch to the execution pipeline) drops the per-issue Gate 3 reviewer and the draft-approval ceremony, and relies on Gate 1 passing before every `save_issue` (enforced at the tool boundary by a save-hook). This is the path where 80 issues once shipped with the validator never run. +- Story template gains an `## Out of scope` non-goals section (the implementer agent gold-plates without a boundary) and a `## Post-ship follow-up` line for soak/dashboard checks that cannot run headless. Hardening issues gain a per-issue `**Verify:**` line (their only testable contract, since they have no Requirements section). +- New validator tests: fenced-code blindness, the `---` tail-drop split, a standalone story, an adjacent-metric false-negative, and the Anchor shape check. + +### Changed (validator correctness) +- **`epic-backlink` demoted to WARN.** A standalone or refactor story with no parent no longer hard-fails; write `**Epic:** none` to mark the omission deliberate. This was the single most common self-inflicted validator failure. +- **`labels-verify` WARN deleted.** It fired on every block every run and could never be discharged by the script, pure noise. +- **Vague-verb check now requires an adjacent metric.** The metric must sit within a few words of `improve`/`enhance`/`optimize`; a bare integer elsewhere on the line (a version, issue id, or port) no longer launders the verb. +- **Code-fence and inline-code awareness.** The em-dash, AI-tell, vague-verb, and section scans skip fenced code and inline-code spans, so a quoted `## Description`, `robust-parser.ts`, or a code sample no longer false-trips. +- **`---` splitting is fence-aware and merges title-less tails.** A `---` inside a code block, or one that only splits body prose, is content; it can no longer silently drop the tail of an issue (and everything after it) from validation. +- **`epic-story-count` counts any bullet** under "Stories under this epic" (verbatim child titles and ordered lists included), and role-prefixed child titles are exempt from the em-dash check, resolving the contradiction with "list child titles verbatim". +- **Estimate** is set as the Linear field at `save_issue` time, not mandated as a body line (no execution stage reads it); the reviewer treats a missing estimate as at most Minor. Priority enum reconciled to `Urgent / High / Medium / Low / No priority` with a validator WARN on off-enum values; `Urgent` is reserved for an active incident. +- The canonical examples carry Anchor lines and runnable acceptance closers; the 24h-soak closers moved to Post-ship follow-up, and Example 2's "operator" persona is annotated as a real workspace role (not the invented-persona anti-pattern). +- Portable validator invocation in `SKILL.md` (resolve from the skill dir), and the Gate-1 loophole is closed: time pressure and "too trivial to check" are not exemptions. + +### Added (original three-gate work) - `skill/review/issue-reviewer.md`: Gate 3, an independent issue-reviewer subagent template adapted from the code-review pattern. A fresh-context agent re-verifies every claim in a draft against the real repo and Linear workspace (file:line references, "already done" diagnoses, "change everywhere" sweeps, scope bundling, missing estimates, data-vs-code gaps, dedup and epic/child linkage) and returns graded findings with a File-as-is / Fix-before-filing / Decompose-first verdict. Grounded in real issue-creation failures (a false "there is no middleware.ts" diagnosis that would have shipped the wrong locale; a partial copy sweep that left the storefront and checkout disagreeing). - `skill/scripts/validate_issue.py`: a stdlib-only validator that mechanically enforces the house-style rules (em-dash in prose, vague verbs without a metric, AI tells, section names and order per archetype, hardening `Fix:` lines, epic story count, label shape, title rules). The skill runs it as a hard gate before any draft is shown. Tests in `skill/scripts/tests/`. - Validator support for the bundled-concept story variant (`### Concept` blocks replace the `## Solution` H2), so those drafts pass the gate; added a fixture. diff --git a/README.md b/README.md index 95ee7b5..16e7190 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ # linear-issue-craft -A portable Claude Code skill that captures a tight, repeatable Linear issue writing style — designed to be applied to any Linear workspace. +A portable Claude Code skill that captures a tight, repeatable Linear issue writing style, designed to be applied to any Linear workspace. -Every pattern in this repo is grounded in real shipped issues, not invented. The three canonical exemplars are quoted verbatim (with workspace identifiers anonymized) in `skill/references/06-canonical-examples.md`. +The issue is treated as a compiler input, not an essay: an autonomous execution pipeline reads it (a planner, a held-out probe that writes a frozen test from the title plus Requirements plus Evaluation, an implementer, a reviewer, a merge gate), so concreteness is the product. Every pattern in this repo is grounded in real shipped issues, not invented. The canonical exemplars are quoted verbatim (with workspace identifiers anonymized) in `skill/references/06-canonical-examples.md`. ## What's in the box @@ -10,12 +10,12 @@ Every pattern in this repo is grounded in real shipped issues, not invented. The skill/ ├── SKILL.md Main skill entry: decision flow, golden rules, three-gate output contract ├── references/ -│ ├── 01-title-patterns.md 5 title shapes with real examples +│ ├── 01-title-patterns.md 6 title shapes with real examples │ ├── 02-description-templates.md Body skeletons per archetype │ ├── 03-tone-guide.md Voice characteristics, hard "no" list │ ├── 04-labels-and-priority.md Layered labels + category-driven priority │ ├── 05-anti-patterns.md Common AI failures and their fixes -│ └── 06-canonical-examples.md Three full verbatim exemplars (Epic, Story, Hardening) +│ └── 06-canonical-examples.md Five full verbatim exemplars (Epic, Story, Hardening, Bundled, Task) ├── prompts/ │ ├── brief-to-epic.md Workflow: 1-paragraph brief → epic + N child stories │ ├── hardening-issue.md Workflow: list of defects in one file → Hardening ticket @@ -26,13 +26,16 @@ skill/ └── issue-reviewer.md Gate 3: independent reviewer subagent template ``` -## The three archetypes +## The archetypes | Archetype | Title shape | Body sections | |---|---|---| | **Epic** | `Epic: ` | Goal · Outcomes · Out of scope · Stories under this epic | -| **Story** | imperative verb + concrete deliverable, or ` — ` | (epic backlink) · User Story · Problem · Solution · Requirements · Evaluation | -| **Hardening** | `Harden : , , ` | Summary · Issues (numbered, each with **Fix:**) | +| **Story** | imperative verb + concrete deliverable, or ` — ` | (epic backlink) · Anchor · User Story · Problem · Solution · Out of scope · Requirements · Evaluation | +| **Hardening** | `Harden : , , ` | Summary · Issues (numbered, each with **Fix:** + **Verify:**) | +| **Task** | `Bug:` / `Chore:` / `Spike:` (or set `**Type:**`) | Anchor · Problem · Verification | + +Every **Story** and **Task** carries an **Anchor**: one machine-resolvable target (`file.ext:line`, `file.ext:symbol`, `module.function`, `METHOD /path -> status`, or `playwright:selector`) that the held-out probe binds to. ## How to use it @@ -52,7 +55,7 @@ You can also paste the contents of `skill/SKILL.md` and any relevant `references ### Filing the drafts -Drafts produced under this skill are designed to be fed into the Linear MCP server's `save_issue` tool. Before a draft is shown, it clears three gates: a mechanical validator (`scripts/validate_issue.py`), a judgment checklist, and an independent issue-reviewer subagent (`review/issue-reviewer.md`) that re-verifies every claim against the actual code and workspace (file:line references, "already done" diagnoses, "change everywhere" sweeps, scope, estimates, dedup). The skill requires explicit user approval before any `save_issue` call. +Drafts produced under this skill are designed to be fed into the Linear MCP server's `save_issue` tool. In interactive mode, before a draft is shown it clears three gates: a mechanical validator (`scripts/validate_issue.py`), a judgment checklist, and an independent issue-reviewer subagent (`review/issue-reviewer.md`) that re-verifies every claim against the actual code and workspace (file:line references, Anchor existence, "already done" diagnoses, "change everywhere" sweeps, scope, dedup). The skill requires explicit user approval before any `save_issue` call. In headless / pipeline mode (a subagent filing a batch straight to the execution pipeline) the per-issue reviewer and approval step are replaced by a save-hook that runs the validator at the tool boundary, so validation cannot be skipped under load. ## Why this exists @@ -64,7 +67,7 @@ This skill is the bridge. ## Source material -The patterns were distilled from 64 real shipped issues across one well-structured Linear workspace (Web / Mobile / API monorepo). The three canonical exemplars in `references/06-canonical-examples.md` are quoted verbatim with the workspace-specific identifiers anonymized to `PROJ-N`. +The patterns were distilled from real shipped issues across well-structured Linear workspaces. The canonical exemplars in `references/06-canonical-examples.md` are quoted verbatim with the workspace-specific identifiers anonymized to `PROJ-N` (Anchor lines and runnable acceptance closers reflect the current contract; the originals predate both). ## Extensions (not built yet) diff --git a/skill/SKILL.md b/skill/SKILL.md index 6d5e73d..69376ab 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -7,21 +7,26 @@ description: Use when creating, drafting, or rewriting Linear issues (epics, sto A portable issue-writing style. Every pattern in this skill is grounded in real shipped issues: see `references/06-canonical-examples.md` for verbatim source material. +**The issue is a compiler input, not an essay.** Downstream, an autonomous pipeline reads this text: a planner, a held-out probe that writes a frozen test from the title plus Requirements plus Evaluation alone (it never sees the repo), an implementer, a reviewer whose rubric is the Requirements section, and a merge gate. Concreteness is the product; prose is not. A named file, endpoint, or selector the probe can target is worth more than a paragraph of intent. When you are tempted to polish a sentence, name a thing instead. + ## When this skill applies Invoke when the user wants to create or polish a Linear issue and you have any signal that they want this style. Skip when the user explicitly asks for a different style or templates from another team. -## The three issue archetypes +## The issue archetypes -Every issue is one of three shapes. Identify the archetype first: it determines title pattern, body sections, and labels. +Every issue is one of these shapes. Identify the archetype first: it determines title pattern, body sections, and labels. | Archetype | When to use | Title shape | Body sections | |---|---|---|---| | **Epic** | Strategic surface, 2–8 child stories, shared infra/domain | `Epic: ` | Goal · Outcomes · Out of scope · Stories under this epic | -| **Story** | One PR, one acceptance test, one service/surface | imperative verb + concrete deliverable, or ` — ` | (epic backlink) · User Story · Problem · Solution · Requirements · Evaluation | +| **Story** | One PR, one acceptance test, one service/surface | imperative verb + concrete deliverable, or ` — ` | (epic backlink) · Anchor · User Story · Problem · Solution · Out of scope · Requirements · Evaluation | | **Hardening** | Multiple vulnerabilities/bugs in one file/service | `Harden : , , ` | Summary · Issues (numbered, each with **Fix:**) | +| **Task** | One bug, chore, spike, or ops change too small for the full story ceremony | `Bug: …` / `Chore: …` / `Spike: …`, or set `**Type:**` | Anchor · Problem · Verification | + +The **Story** and **Task** carry an **Anchor**: one machine-resolvable target (`file.ext:line`, `file.ext:symbol`, `module.function`, `METHOD /path -> status`, or `playwright:selector`). The held-out probe cannot test a thing you never named, so this is the single highest-leverage line in the issue. Epics and hardening tickets are inherently anchored (a domain, a file path in the title) and do not carry a separate Anchor. -If a request doesn't fit one of these three, **stop and decompose** before writing. A "let's also rework X while we're at it" is a sibling story, not a bolted-on requirement. +If a request doesn't fit one of these shapes, **stop and decompose** before writing. A "let's also rework X while we're at it" is a sibling story, not a bolted-on requirement. ## Decision flow @@ -43,19 +48,23 @@ If a request doesn't fit one of these three, **stop and decompose** before writi do NOT proceed. ``` +A single bug, chore, spike, or ops change that is too small for the full Story ceremony (no persona, no requirement-to-evaluation mapping) is a **Task**: `**Type:** Bug` (or `Chore`/`Spike`/`Ops`), an Anchor, a Problem, and a runnable Verification. Do not inflate a one-line bug into a Story, and do not file it as a one-issue Hardening ticket. + ## Golden rules (apply to every issue, every archetype) 1. **Concrete over abstract.** Name the file path, function, endpoint, table, env var. "Improve API performance" is rejected; "Add caching to `/search` endpoint (target p95 < 200ms on 10k events)" is accepted. -2. **Imperative verb first** on stories: Stand up · Wire · Harden · Build · Add · Generate · Refactor · Replace. Never passive ("Performance should be improved"). -3. **Diagnose current state before proposing the fix.** Every epic opens with "Today …" or "Currently …" naming the deficiency in concrete terms (missing files, missing headers, missing endpoints). Skip the diagnosis and the issue reads like a press release. -4. **Reference past work explicitly.** Link related tickets by identifier, prior PRs, prior incidents. Use Linear's `PROJ-XX` embed in Linear; plain `PROJ-XX` in plaintext. -5. **No em-dashes in prose.** The em-dash (`—`) is allowed in exactly one place: the separator in a role-prefixed story *title* (e.g., `Venue — Demand forecasting`). Everywhere else, including the "Stories under this epic" list (write `Story 1: …`), use a colon, a comma, or rewrite. `scripts/validate_issue.py` enforces this. -6. **Labels are layered, not flat.** Always pick one domain **plus** one or more capabilities (e.g., `ai`, `seo`, `compliance`, `tech-debt`, `ui-ux`). The domain set is whatever the target workspace already uses (`api`/`web`/`mobile`, or `api`/`frontend`, or per-service): run `list_issue_labels` and pick from the real list, never invent one. Don't use urgency or sprint labels. -7. **Priority maps to category, not urgency:** High = security / infra / epics. Medium = features. Low = exploratory / decision-support. No-priority = scoped-out or already done. +2. **Anchor every story and task.** State one machine-resolvable target as `**Anchor:**` (`file.ext:line`, `file.ext:symbol`, `module.function`, `METHOD /path -> status`, or `playwright:selector`). This is the interface the held-out probe binds to; a prose anchor ("the auth flow") is not addressable and gets flagged. Name the seam the change actually lands on. +3. **Imperative verb first** on stories: Stand up · Wire · Harden · Build · Add · Generate · Refactor · Replace. Never passive ("Performance should be improved"). +4. **Diagnose current state before proposing the fix.** Every epic opens with "Today …" or "Currently …" naming the deficiency in concrete terms (missing files, missing headers, missing endpoints). Skip the diagnosis and the issue reads like a press release. +5. **Acceptance must be runnable headless.** Every Evaluation item is a command or observation a headless stage can run against the repo checkout, with its expected output. No item may assert on a stubbed version of the thing under test. A soak, a dashboard read, or a staging check that needs prod credentials is a **Post-ship follow-up** line, never the acceptance. +6. **Reference past work explicitly.** Link related tickets by identifier, prior PRs, prior incidents. Use Linear's `PROJ-XX` embed in Linear; plain `PROJ-XX` in plaintext. Encode a hard ordering as a real Linear blocked-by relation, not only prose, so the planner's wave DAG is correct. +7. **No em-dashes in prose.** The em-dash (`—`) is allowed in exactly one place: the separator in a role-prefixed story *title* (e.g., `Venue — Demand forecasting`), including where that title is listed verbatim under "Stories under this epic". Everywhere else in prose, use a colon, a comma, or rewrite. `scripts/validate_issue.py` enforces this. +8. **Labels are layered, not flat.** Always pick one domain **plus** one or more capabilities (e.g., `ai`, `seo`, `compliance`, `tech-debt`, `ui-ux`). The domain set is whatever the target workspace already uses (`api`/`web`/`mobile`, or `api`/`frontend`, or per-service): run `list_issue_labels` and pick from the real list, never invent one. Don't use sprint labels. +9. **Priority maps to category, not urgency:** High = security / infra / epics. Medium = features. Low = exploratory / decision-support. No priority = scoped-out or already done. Urgent is reserved for an active incident (prod down, live regression), not for a feature the requester feels strongly about. ## How to use this skill -1. Identify the archetype (epic / story / hardening). When in doubt, ask the user. +1. Identify the archetype (epic / story / hardening / task). When in doubt, ask the user. 2. Open the matching workflow prompt in `prompts/`: - `prompts/brief-to-epic.md`: converts a one-paragraph brief into an epic + its child stories. - `prompts/hardening-issue.md`: converts a list of code issues in one file into a hardening ticket. @@ -75,27 +84,38 @@ If a request doesn't fit one of these three, **stop and decompose** before writi When this skill is active, every draft you produce must include: - A **title** that follows one of the patterns in `references/01-title-patterns.md` verbatim. -- A **body** that uses the section headings in `references/02-description-templates.md` for its archetype, in order, with no extra sections. +- A **body** that uses the section headings in `references/02-description-templates.md` for its archetype, in order. A story may add an `## Out of scope` non-goals section; do not invent other sections. +- An **Anchor** on every story and task: one machine-resolvable target (see golden rule 2). - A **labels** array (1 domain + ≥1 capability) following `references/04-labels-and-priority.md`. -- A **priority** name (`High` / `Medium` / `Low` / `No priority`) chosen by the category rule. Priority is category, not status: never use `No priority` to mean "deferred" or "verify-only". -- An **estimate** on every story (its point/size value). A story with no estimate is incomplete; a blank estimate pushes sizing onto a later planner. -- A short, conversational note to the user above the draft, summarizing why you chose this archetype, what alternative you considered, and any Critical or Important findings Gate 3 raised and how you resolved them. Keep it to 3 sentences max. +- A **priority** name (`Urgent` / `High` / `Medium` / `Low` / `No priority`) chosen by the category rule. Priority is category, not status: never use `No priority` to mean "deferred" or "verify-only", and reserve `Urgent` for an active incident. +- An **estimate**, set as the Linear estimate field at `save_issue` time (not a body line). No execution stage reads it; it is planner-facing, so set it when the workspace uses estimates and omit it silently when it does not. +- A short, conversational note to the user above the draft, summarizing why you chose this archetype, what alternative you considered, and any Critical or Important findings review raised and how you resolved them. Keep it to 3 sentences max. + +## Two modes: interactive and headless + +How this skill runs depends on who is downstream. Detect the mode from the invocation, not the mood of the request. + +- **Interactive mode** (a human is reading each draft): you draft one or a few issues, run the gates, and show the human a note plus the draft for approval before filing. All three gates apply, including the independent Gate 3 reviewer per issue. This is the writing-aid path. +- **Headless / pipeline mode** (default when this skill is invoked from inside a subagent, or when filing more than one issue in a batch to hand straight to the execution pipeline): there is no human reading each draft, so the per-issue draft-approval ceremony and the per-issue Gate 3 reviewer are **dropped**. What replaces them is enforcement that cannot be skipped under load: **Gate 1 must pass on every block before every `save_issue` call** (a save-hook enforces this at the tool boundary; see the repo's hook), and the batch as a whole may get one review pass, not one reviewer per issue. The failure this fixes is real: the headless path is where per-issue ceremony silently evaporated (one run filed 80 issues with the validator never executed). + +The rule that never relaxes in either mode: no `save_issue` without a clean Gate 1 on that issue's body. Everything else (the human note, the per-issue independent review) is interactive-mode ceremony that a batch handoff replaces with a boundary gate, not with trust. ## Validate before showing the draft -Three gates. Gate 1 is mechanical and non-negotiable. Gate 2 is judgment the script cannot make for you. Gate 3 is an independent reviewer that re-verifies your claims against the real code, because Gate 2 is you checking work you are biased to believe. +The gates below are the **interactive-mode** flow. Gate 1 is mechanical and non-negotiable in both modes. Gate 2 is judgment the script cannot make for you. Gate 3 is an independent reviewer that re-verifies your claims against the real code, because Gate 2 is you checking work you are biased to believe; in headless mode it is replaced by the save-hook plus an optional single batch review. ### Gate 1: run the validator (hard gate) -Run `scripts/validate_issue.py` (relative to this skill's directory) on the draft: +Run the validator on the draft. Resolve the path from this skill's own directory so it works from any working directory (the skill lives in a hashed plugin-cache dir, so a bare relative path can miss): ```bash -python3 scripts/validate_issue.py # or pipe the draft on stdin +python3 "$(dirname "$0")/scripts/validate_issue.py" # or pipe the draft on stdin +# from the skill root: python3 scripts/validate_issue.py ``` -It must exit 0. It enforces: em-dash in prose, vague verbs ("improve"/"enhance"/"optimize") without a target metric, AI tells and `Co-Authored-By`, section names and order per archetype (catches `Description`/`Acceptance Criteria` drift), hardening `**Fix:**` lines, epic story count (2 to 8), label shape, and trailing-period titles. Fix every ERROR before showing the draft. WARNs do not block, but read them. +It must exit 0. It enforces (ERROR): em-dash in prose (fenced code and role-titled child bullets exempt), vague verbs ("improve"/"enhance"/"optimize") with no adjacent target metric, AI tells and `Co-Authored-By`, section names and order per archetype (catches `Description`/`Acceptance Criteria` drift), hardening `**Fix:**` lines, epic story count (2 to 8), and label shape. It WARNs (does not block) on a missing or prose-shaped Anchor, a missing story non-goal, an unobservable acceptance closer, and an off-enum priority; read every WARN. Fix every ERROR before showing the draft. -This is a gate, not a mental note. Do not show the user a draft you have not run through the validator. If `python3` is unavailable, that is a blocker to resolve (the script is stdlib-only, no install needed), not a reason to skip the gate. +This is a gate, not a mental note. Do not show the user a draft you have not run through the validator. Time pressure and "this draft is too trivial to check" are not exemptions: the run is sub-second and the bulk/headless path is exactly where skipped validation shipped 80 unchecked issues. If `python3` is unavailable, that is a blocker to resolve (the script is stdlib-only, no install needed), not a reason to skip the gate. ### Gate 2: judgment checks (you, not the script) @@ -126,4 +146,4 @@ Act on the verdict: - **File as-is**: proceed to show the user, noting in your draft note what the review checked. - Push back if the reviewer is wrong, with the evidence (the `file:line` that proves the claim), the same way you would with a code reviewer. -This is a gate, not a mental note. Do not show the user a draft you have not had independently reviewed. The defects Gate 3 catches are exactly the ones that look correct to the author, so "I am confident" is not a reason to skip it. The only case that does not need Gate 3 is a pure title or label tweak on an already-filed issue. +This is an interactive-mode gate. In interactive mode, do not show the user a draft you have not had independently reviewed: the defects Gate 3 catches are exactly the ones that look correct to the author, so "I am confident" is not a reason to skip it. In headless / pipeline mode there is no per-issue reviewer; the save-hook (Gate 1 at the tool boundary) plus at most one batch review over the whole set stands in for it. The cases that never need Gate 3: a pure title or label tweak on an already-filed issue, and any issue filed in headless mode. diff --git a/skill/prompts/brief-to-epic.md b/skill/prompts/brief-to-epic.md index b8fa1aa..6dd945c 100644 --- a/skill/prompts/brief-to-epic.md +++ b/skill/prompts/brief-to-epic.md @@ -65,11 +65,11 @@ Use the Epic template from `references/02-description-templates.md`. Fill in: Use the Story template from `references/02-description-templates.md` for each child. Confirm: -- Each story has a `**Epic:** ` backlink and a `**Title:** ` line below it. -- The body is shaped as **User Story / Problem / Solution / Requirements / Evaluation** (or the bundled-concept variant if two primitives ship together). +- Each story has a `**Epic:** ` backlink, a `**Title:** ` line, and an `**Anchor:**` line (one machine-resolvable target the probe can bind to). +- The body is shaped as **User Story / Problem / Solution / Out of scope / Requirements / Evaluation** (or the bundled-concept variant if two primitives ship together). - Requirements list 3–6 grep-able actions with file paths. - Evaluation items map back to requirements with `**Validates R**:` prefixes. -- The last Evaluation item is a soak/integration check. +- Every Evaluation item is runnable headless against the checkout; the last item is a holistic acceptance check that exercises the real seam. Any soak, dashboard, or staging check goes in a `## Post-ship follow-up` line, never as the acceptance. ## Step 6: Show to user before filing @@ -100,6 +100,7 @@ I'm proposing ** child stories** under this epic. Archetype is ** +**Anchor:** status | playwright:selector> **Parent:** **Labels:** , **Priority:** @@ -111,7 +112,7 @@ I'm proposing ** child stories** under this epic. Archetype is ** **Fix:** +**Verify:** ### 2\. (lines ) <…> **Fix:** <…> +**Verify:** <…> (repeat for each numbered issue) ``` +A hardening ticket has no Requirements section, so the `**Verify:**` line is the per-issue testable contract the downstream reviewer and held-out probe read. Keep it runnable headless (a command plus expected output), not a soak or a manual click-through. + ## Step 5: Labels and priority - **Labels:** always include the domain label (`api` / `web` / `mobile`). Add `compliance` if any issue is security or privacy related. Add `tech-debt` if all issues are non-security defensive code. @@ -89,7 +93,7 @@ Drafted as a single Hardening ticket because all issues live in `` and ## Common mistakes to avoid -- **Missing per-issue `**Fix:**` line.** Every numbered issue must end with a Fix callout. Without it the ticket is just a bug list. +- **Missing per-issue `**Fix:**` or `**Verify:**` line.** Every numbered issue ends with a Fix callout and a runnable Verify line. Without the Fix it is just a bug list; without the Verify the downstream reviewer and probe have no acceptance to check. - **Mixing security and non-security issues without flagging.** If you bundle both, the Summary's severity calibration must call this out explicitly ("Issues 1–2 are security; issues 3–4 are defensive cleanup."). - **Generic symptom names.** "Bug in input handling" is rejected. "No upper-bound length check on text input" is accepted. - **Inventing line numbers.** Include `(lines X-Y)` in the issue header when the source gives them; reviewers scan for them. If the input has no line numbers, omit them rather than fabricating ranges. diff --git a/skill/prompts/refactor-story.md b/skill/prompts/refactor-story.md index 6e3f855..9b4f998 100644 --- a/skill/prompts/refactor-story.md +++ b/skill/prompts/refactor-story.md @@ -42,8 +42,9 @@ If the title sounds like a story to add a *feature*, you're not writing a refact Use the Story template. The shape is the same as a feature story, but the User Story has a particular flavor: ```markdown -**Epic:** +**Epic:** **Title:** +**Anchor:** ## User Story @@ -57,6 +58,10 @@ As an engineer working on , I want , so that +## Out of scope + +* + ## Requirements 1. @@ -68,7 +73,7 @@ As an engineer working on , I want , so that ` consumers import from the new module; `rg \"\"` returns zero hits."> 2. **Validates R3**: -3. **Validates all**: **No behavior change.** +3. **Validates all**: **No behavior change.** ``` ## Step 4: Labels and priority @@ -90,6 +95,7 @@ Skeleton: Drafted as a refactor story because the change preserves behavior and touches a single surface. Alternative was a hardening ticket; rejected because the issues here aren't defects in the current code: it just doesn't scale. **Title:** +**Anchor:** <the new file/function being introduced> **Labels:** <domain>, tech-debt **Priority:** Low diff --git a/skill/references/01-title-patterns.md b/skill/references/01-title-patterns.md index 3a91e3b..233d0bc 100644 --- a/skill/references/01-title-patterns.md +++ b/skill/references/01-title-patterns.md @@ -1,6 +1,6 @@ # Title patterns -Every issue title follows one of the five patterns below. Pick the matching pattern; never invent a new one. +Every issue title follows one of the patterns below. Pick the matching pattern; never invent a new one. ## 1. Epic: `Epic: <Domain>` @@ -66,6 +66,19 @@ Used when bootstrapping a multi-artifact infrastructure layer in one shot. Real example: - `SEO foundations: robots.ts, sitemap.ts, metadataBase, canonicals, Organization JSON-LD` +## 6. Task: imperative deliverable (Bug / Chore / Spike / Ops) + +A single small bug, chore, spike, or ops change uses a plain imperative title, with the kind set in the body's `**Type:**` header (or an optional `Bug:` / `Chore:` / `Spike:` prefix). Same concreteness rules as a story title. + +Real examples: +- `Fix drag reset on pointerup in the 3D graph` +- `Bump Node to 20 in the api Dockerfile and CI` +- `Spike: is the --strict-mcp-config envelope honored by the runner?` + +Avoid: +- `Fix the bug` (no target) +- `Cleanup` (no verb of action, no scope) + ## House rules for all titles - **Capitalization:** Title Case for the leading noun (`Epic`, `SEO`) and for trademarked tool names (`Schema.org`, `Trigger.dev`, `Helicone`); sentence case for the rest. diff --git a/skill/references/02-description-templates.md b/skill/references/02-description-templates.md index 33a57a7..cb4b9a9 100644 --- a/skill/references/02-description-templates.md +++ b/skill/references/02-description-templates.md @@ -45,8 +45,9 @@ Canonical example: see Example 1 in `06-canonical-examples.md`. Every story follows a four-beat mental model: **User Story**, then **Problem**, then **Solution**, then **Evaluation**. Persona and outcome (User Story). What is broken or missing today (Problem). What we are building and the chosen approach (Solution). How we will know it works (Evaluation). Even when a story is short, this scan-ability is the defining feature of the house style. A reader should be able to skim the section headers alone and answer "who, what's wrong, what's the fix, how do we verify." ```markdown -**Epic:** <parent epic title without the "Epic:" prefix> +**Epic:** <parent epic title without the "Epic:" prefix, or `none` for a deliberately standalone story> **Title:** <verbatim story title, matching the issue title exactly> +**Anchor:** <one machine-resolvable target: file.ext:line | file.ext:symbol | module.function | METHOD /path -> status | playwright:selector> ## User Story @@ -58,7 +59,11 @@ As a <role>, I want <capability>, so that <outcome>. ## Solution -<2–4 sentences. Name what we are building and the chosen approach. Reference the file/service being touched. End with a simplifying constraint or scope cap (e.g., "This is one config change, no business-logic refactor.").> +<2–4 sentences. Name what we are building and the chosen approach. Reference the file/service being touched. End with a simplifying constraint or scope cap (e.g., "This is one config change, no business-logic refactor."). Keep the sketch to ~10 lines; do not paste a design doc or prescribe a library unless it is a hard constraint. Over-specifying pre-commits the implementer.> + +## Out of scope + +* <One non-goal a reasonable implementer might otherwise pull in. Bounds the scope so the change stays one PR.> ## Requirements @@ -69,19 +74,25 @@ As a <role>, I want <capability>, so that <outcome>. ## Evaluation -1. **Validates R<n> [+ R<m>]**: <How to verify, with the exact command or observation. e.g., "A test call from `api/` shows up in the Helicone dashboard within 10s with model, tokens, and cost; `rg \"from 'openai'\"` outside `openai-client.ts` returns zero hits."> +1. **Validates R<n> [+ R<m>]**: <How to verify, with the exact command or observation. e.g., "`rg \"from 'openai'\"` outside `openai-client.ts` returns zero hits; a unit test asserts the client's baseURL."> 2. **Validates R<n>**: <…> 3. **Validates R<n>**: <…> -4. **Validates all**: <A holistic acceptance check that exercises the whole feature, e.g., "A 24h soak in staging shows zero 'fallback to direct OpenAI' log lines."> +4. **Validates all**: <A holistic acceptance check runnable headless against the checkout, e.g., "`pytest api/tests/test_openai_client.py` passes and the wrapper is the only importer of `openai`."> + +## Post-ship follow-up + +<Optional. Checks that cannot run headless (a 24h soak, a dashboard read, a prod-credential check). These are follow-ups, never the acceptance above.> ``` **Why each section:** -- **Epic** backlink keeps Linear's parent relation visible in PR descriptions, exports, and search. **Title** restates the issue title verbatim so the body remains self-identifying when copied out of Linear. +- **Epic** backlink keeps Linear's parent relation visible in PR descriptions, exports, and search; use `none` for a true standalone. **Title** restates the issue title verbatim so the body remains self-identifying when copied out of Linear. +- **Anchor** names the one thing the held-out probe binds to. It must be addressable (a path with a line or symbol, a dotted module.function, an HTTP `METHOD /path -> status`, or a `playwright:` selector), not prose. The probe cannot test what the issue never named. - **User Story** is the only place where "As X, I want Y, so Z" is allowed. Forces the writer to name the persona and the *so-that* outcome. The persona must be a role the project actually uses: confirm the term against the repo, its docs, or existing issues before writing it. A named-but-invented persona ("operator", "admin") looks concrete and slips past the abstract-noun check, so it is called out explicitly. If the project calls them "users", write "user". - **Problem** diagnoses current state. Always names the file/service/component being touched. Never leads with the fix. - **Solution** explains what we are building and the approach. Always ends with a simplifying constraint or scope cap so reviewers can see the boundary. -- **Requirements** are imperative. Each one is a single concrete action a reviewer can grep for in the diff. -- **Evaluation** items are pairings: each one explicitly cites which Requirement(s) it validates. The last item is always a soak / integration check that exercises the whole feature. +- **Out of scope** names at least one non-goal. The implementer is an autonomous agent that will gold-plate without an explicit boundary. +- **Requirements** are imperative. Each one is a single concrete action a reviewer can grep for in the diff. This section is the reviewer's rubric downstream, so keep each item checkable against a diff. +- **Evaluation** items are pairings: each one explicitly cites which Requirement(s) it validates. Every item is runnable headless against the checkout; the last item is a holistic acceptance check that exercises the real seam (not a stubbed version of the thing under test). Soaks, dashboards, and staging checks move to **Post-ship follow-up**. Canonical example: see Example 2 in `06-canonical-examples.md`. @@ -130,7 +141,7 @@ Use this variant only when both of the following are true: - The work delivers two or more clearly nameable primitives that share a single user story. - Splitting would create a real risk that one primitive ships without the other. -If either is false, file two sibling stories instead. +If either is false, file two sibling stories instead. For a backlog that will be run by the autonomous pipeline, prefer siblings: one issue is one branch and one held-out probe, and a single probe can pass on the finished half of a bundle. If you do bundle, give each concept its own acceptance item in Evaluation so no primitive is left unchecked. ### Epic-level story variant @@ -173,22 +184,50 @@ Then continue with Outcomes / Out of scope / Stories under this epic. <…> **Fix:** <…> +**Verify:** <One runnable check that proves the fix, with expected output. e.g., "`pytest -k fail_closed` asserts a blocked result when the API errors."> ### 3\. <Symptom name> <…> **Fix:** <…> +**Verify:** <…> ``` **Why each section:** - **Summary** = scope + calibration. The calibration sentence ("None are currently exploitable at scale, but…") is what separates a hardening ticket from a panicked CVE filing. It tells the reader why this matters *now* even though nothing is on fire. -- **Issues** are numbered. Each header includes the symptom and the line range. Each entry ends with a `**Fix:**` line so the reader can scan the whole ticket for proposed fixes in one pass. +- **Issues** are numbered. Each header includes the symptom and the line range. Each entry ends with a `**Fix:**` line so the reader can scan the whole ticket for proposed fixes in one pass, and a `**Verify:**` line so the downstream reviewer and probe have an observable acceptance per issue (a hardening ticket has no Requirements section, so the `**Verify:**` line is where its testable contract lives). Canonical example: see Example 3 in `06-canonical-examples.md`. --- +## Task body (bug / chore / spike / ops) + +A single bug, chore, spike, or ops change that does not need the full story ceremony. Select it with a `**Type:**` header. No persona, no requirement-to-evaluation mapping, but it still carries an Anchor and a runnable Verification. + +```markdown +**Type:** Bug <!-- or Chore / Spike / Ops --> +**Title:** <imperative deliverable, e.g., "Fix drag reset on pointerup in the 3D graph"> +**Anchor:** <file.ext:line | file.ext:symbol | module.function | METHOD /path -> status | playwright:selector> + +## Problem + +<1–3 sentences. For a bug: the current wrong behavior, named at the anchor (`graph-3d.tsx:657`). For a chore/spike: the concrete task and why now.> + +## Verification + +<One runnable check with expected output, e.g., "`npx playwright test drag-pin` shows the node stays pinned after release." A spike may instead state the concrete question to answer and where the answer gets recorded.> +``` + +**Why each section:** +- **Type** selects the lightweight shape so a one-line bug is not inflated into a Story or mis-filed as a one-issue Hardening ticket. +- **Anchor** and **Verification** are the minimum contract: name the seam, and give one command a headless stage can run. + +Canonical example: see Example 5 in `06-canonical-examples.md`. + +--- + ## Cross-references Always link related work. In Linear's editor, use `<issue id="<uuid>">PROJ-XX</issue>` (you can paste the ticket and Linear renders the embed). In plaintext drafts, write `PROJ-XX` and Linear will auto-link on paste. @@ -196,4 +235,4 @@ Always link related work. In Linear's editor, use `<issue id="<uuid>">PROJ-XX</i Always cross-reference: - The epic that contains a story (use the `**Epic:**` header line) - The story or PR that surfaced a hardening ticket -- Any blocking/blocked-by relationship (mention it inside Problem or Solution, not as a separate section) +- Any blocking/blocked-by relationship. Set it as a real Linear blocked-by relation (the planner builds its execution order from the relation, not from prose), and optionally mention it inside Problem or Solution. diff --git a/skill/references/03-tone-guide.md b/skill/references/03-tone-guide.md index 4a08a61..f9ca0fd 100644 --- a/skill/references/03-tone-guide.md +++ b/skill/references/03-tone-guide.md @@ -66,10 +66,11 @@ Imitate the rhythm: subject + must + comma-list of obligations. | Issue type | Body length | Sections | |---|---|---| | Epic | 200–500 words | Goal · Outcomes · Out of scope · Stories under this epic | -| Story (simple) | 100–250 words | Epic backlink · User Story · Problem · Solution · Requirements (3–4) · Evaluation (3–4) | +| Story (simple) | 100–250 words | Epic backlink · Anchor · User Story · Problem · Solution · Out of scope · Requirements (3–4) · Evaluation (3–4) | | Story (complex infra) | 250–500 words | Same sections, denser Requirements (5–6) | -| Story (bundled-concept) | 300–600 words | Epic backlink · User Story · Problem (intro) · Concept blocks (each with bolded **Problem:** + **Solution:**) · Why bundled · Requirements · Evaluation | -| Hardening | 200–400 words | Summary · Issues (3–5 numbered items) | +| Story (bundled-concept) | 300–600 words | Epic backlink · Anchor · User Story · Problem (intro) · Concept blocks (each with bolded **Problem:** + **Solution:**) · Why bundled · Requirements · Evaluation | +| Hardening | 200–400 words | Summary · Issues (3–5 numbered items, each with **Fix:** + **Verify:**) | +| Task (bug/chore/spike) | 40–120 words | Type · Anchor · Problem · Verification | Going past 500 words is a code smell: you're probably bundling two issues. diff --git a/skill/references/05-anti-patterns.md b/skill/references/05-anti-patterns.md index 2b0697f..64ed5b6 100644 --- a/skill/references/05-anti-patterns.md +++ b/skill/references/05-anti-patterns.md @@ -42,11 +42,11 @@ The most common ways AI-drafted Linear issues drift away from this style. Each r **In-style fix:** every Evaluation item names which Requirements it validates. > 1. **Validates R1 + R2**: A test call from `api/` shows up in the Helicone dashboard within 10s with model, tokens, and cost; `rg "from 'openai'"` outside `openai-client.ts` returns zero hits. -### Missing soak / integration check -**AI default:** All Evaluation items are unit-style. +### Unobservable acceptance closer +**AI default:** The last Evaluation item leans on something no headless stage can run: `A 24h soak in staging shows zero "fallback to direct OpenAI" log lines.` -**In-style fix:** the last item is always a holistic integration check. -> 4. **Validates all**: A 24h soak in staging shows zero "fallback to direct OpenAI" log lines. +**In-style fix:** the last item is a holistic check runnable headless against the checkout, exercising the real seam (not a stub of the thing under test). Push the soak to a `## Post-ship follow-up` line. +> 4. **Validates all**: `pytest api/tests/test_openai_client.py` passes and `rg "from 'openai'"` outside `openai-client.ts` returns zero hits. ### Section name drift ("Acceptance Criteria" instead of "Evaluation") **Signal:** Story ends with `## Acceptance Criteria` instead of `## Evaluation`. @@ -103,7 +103,15 @@ The most common ways AI-drafted Linear issues drift away from this style. Each r ### Story without "Epic:" backlink **Signal:** The story stands alone with no parent reference. -**Fix:** Add `**Epic:** <Epic name>` as the first line of the body. Linear's parent relation alone is not enough: PR descriptions and exports lose it. +**Fix:** Add `**Epic:** <Epic name>` as the first line of the body (Linear's parent relation alone is not enough: PR descriptions and exports lose it). If the story is genuinely standalone, write `**Epic:** none` so the omission is deliberate, not an oversight. The validator warns on a missing backlink; it does not block. + +### Missing Anchor +**Signal:** A story or task has no `**Anchor:**` line, or the anchor is prose ("the auth flow"). +**Fix:** Name one machine-resolvable target: `file.ext:line`, `file.ext:symbol`, `module.function`, `METHOD /path -> status`, or `playwright:selector`. The held-out probe binds to this; a probe cannot test a thing the issue never named. + +### Story with no Out-of-scope +**Signal:** A story lists Requirements but never says what it is *not* doing. +**Fix:** Add a `## Out of scope` bullet naming one non-goal. The implementer is an autonomous agent and will gold-plate past the intended scope without an explicit boundary. ## Labels / priority diff --git a/skill/references/06-canonical-examples.md b/skill/references/06-canonical-examples.md index 4e73660..bd7070a 100644 --- a/skill/references/06-canonical-examples.md +++ b/skill/references/06-canonical-examples.md @@ -1,8 +1,8 @@ # Canonical examples -Four verbatim shipped issues, one per archetype shape. Treat these as the gold standard: when in doubt, structure your draft to match the example whose archetype you're writing. +Five verbatim shipped issues, one per archetype shape. Treat these as the gold standard: when in doubt, structure your draft to match the example whose archetype you're writing. -Workspace-specific identifiers have been anonymized to `PROJ-N`. The prose, structure, and body content are otherwise unchanged from real shipped issues. +Workspace-specific identifiers have been anonymized to `PROJ-N`. The prose, structure, and body content are otherwise unchanged from real shipped issues, except that Anchor lines and runnable acceptance closers reflect the current contract (the originals predate both). --- @@ -58,6 +58,7 @@ Workspace-specific identifiers have been anonymized to `PROJ-N`. The prose, stru > **Epic:** AI Foundation / Platform infra > **Title:** Add Helicone proxy for all OpenAI calls with cost + latency dashboard +> **Anchor:** api/src/lib/openai-client.ts:getOpenAIClient > > ## User Story > @@ -71,6 +72,10 @@ Workspace-specific identifiers have been anonymized to `PROJ-N`. The prose, stru > > Add Helicone as a transparent proxy in front of OpenAI. The existing `api/src/services/openai-moderation.service.ts` and any future LLM caller imports a small `getOpenAIClient()` wrapper that points `baseURL` at Helicone with the org's API key. This is one config change, no business-logic refactor. > +> ## Out of scope +> +> * Changing any prompt or moderation logic; this story only reroutes the transport. +> > ## Requirements > > 1. Create `api/src/lib/openai-client.ts` exporting `getOpenAIClient()` that returns an OpenAI client configured with `baseURL: https://oai.helicone.ai/v1` and the Helicone auth header set from `HELICONE_API_KEY`. @@ -81,16 +86,21 @@ Workspace-specific identifiers have been anonymized to `PROJ-N`. The prose, stru > ## Evaluation > > 1. **Validates R1 + R2**: A test call from `api/` shows up in the Helicone dashboard within 10s with model, tokens, and cost; `rg "from 'openai'"` outside `openai-client.ts` returns zero hits. -> 2. **Validates R3**: Helicone dashboard "Properties" view groups calls by feature and shows distinct cost per feature. -> 3. **Validates R4**: The secret manager returns the key in dev; host secrets list shows it in staging and prod; the guide doc references it. -> 4. **Validates all**: A 24h soak in staging shows zero "fallback to direct OpenAI" log lines (i.e., the proxy stays healthy). +> 2. **Validates R3**: a unit test asserts each call carries a `Helicone-Property-Feature` header; `rg "Helicone-Property-Feature" api/src` shows one per LLM call site. +> 3. **Validates R4**: the secret manager returns the key in dev; `rg "HELICONE_API_KEY" docs/guides/AI_PLATFORM_GUIDE.md` returns a hit. +> 4. **Validates all**: `pytest api/tests/test_openai_client.py` passes and `rg "from 'openai'"` outside `openai-client.ts` returns zero hits. +> +> ## Post-ship follow-up +> +> * A 24h soak in staging should show zero "fallback to direct OpenAI" log lines. This is a follow-up watch, not the acceptance gate. **What to imitate:** -- Header lines (`**Epic:**`, `**Title:**`) repeat the parent link and title before the User Story. +- Header lines (`**Epic:**`, `**Title:**`, `**Anchor:**`) repeat the parent link, title, and probe target before the User Story. - **Problem** diagnoses the current-state gap before introducing the fix; it names the file paths and the failure surface. - **Solution** names the chosen mechanism and ends with a simplifying constraint ("This is one config change, no business-logic refactor."). - Each Requirement is one concrete grep-able action. -- Each Evaluation item explicitly cites which Requirements it validates; the final item is a soak/integration check. +- Each Evaluation item explicitly cites which Requirements it validates and is runnable headless against the checkout; the soak moves to Post-ship follow-up. +- The persona is "operator" because this workspace really uses that role. Verify the persona against the repo before reusing it; do not invent an "operator" or "admin" where the project says "user". --- @@ -153,6 +163,7 @@ Workspace-specific identifiers have been anonymized to `PROJ-N`. The prose, stru > **Epic:** AI Foundation / Platform infra > **Title:** AI rollout safety: prompt evals + feature flags +> **Anchor:** api/src/services/feature-flags.service.ts:isEnabled > > ## User Story > @@ -197,3 +208,31 @@ Workspace-specific identifiers have been anonymized to `PROJ-N`. The prose, stru - Each **Concept** has its own bolded `**Problem:**` and `**Solution:**` mini-frames; analogies and sentence fragments ("Slow and scary.", "Same mental model as `pnpm test`") are welcome here. - A `### Why bundled` block justifies the bundling and is the explicit signal that splitting would cause drift. - Requirements and Evaluation stay at H2 even with multiple concepts, with each Requirement still being one grep-able action. + +--- + +## Example 5: Task / Bug (PROJ-230) + +**Title:** `Fix drag reset on pointerup in the 3D graph` + +**Labels:** `web`, `ui-ux` +**Priority:** Medium (status: Done) + +**Body:** + +> **Type:** Bug +> **Title:** Fix drag reset on pointerup in the 3D graph +> **Anchor:** web/components/graph-3d.tsx:657 +> +> ## Problem +> +> `dragPinned` is set `true` on `pointerdown` but reset to `false` on `pointerup` (`graph-3d.tsx:657`), so a node the user drags to pin snaps back to the force layout the instant they release. The pin state never survives the gesture. +> +> ## Verification +> +> `npx playwright test drag-pin` drags a node, releases, and asserts the node stays at the dropped position (the force tick does not move it on the next frame). + +**What to imitate:** +- No persona, no Requirements-to-Evaluation mapping: a one-cause bug does not need the full Story ceremony. +- The Anchor is the exact `file:line` of the defect, so the held-out probe binds straight to it. +- The Problem names the wrong behavior at the anchor; the Verification is one runnable command with an expected observation. This is the shape that merged in minutes, not the padded one. diff --git a/skill/review/issue-reviewer.md b/skill/review/issue-reviewer.md index 9d6dda3..4783775 100644 --- a/skill/review/issue-reviewer.md +++ b/skill/review/issue-reviewer.md @@ -50,6 +50,19 @@ Task tool (general-purpose): copy string): flag it. It may live only in a runtime DB/CMS reachable via MCP, not in a file. The issue must say so, or an implementer will hunt for a file that does not exist. + - Verify the **Anchor** resolves. A story/task names one machine target + (`file.ext:line`, `file.ext:symbol`, `module.function`, `METHOD /path -> + status`, or `playwright:selector`). Open the repo and confirm the symbol, + line, or route actually exists. A prose or nonexistent Anchor is Critical: + the held-out probe binds to it, and a wrong Anchor makes the probe green + while testing nothing (or ImportError and burn the build loop). + - Verify the **acceptance is machine-checkable headless**. Each Evaluation + (or Task Verification, or hardening **Verify:**) item must be a command or + observation a headless stage can run against the checkout, with an expected + result. Flag as Important any item that (a) needs a soak, a dashboard, or + prod credentials, or (b) asserts against a stubbed version of the very + thing under test (a stub-seam eval passes while the real behavior is + never exercised). Those belong in a Post-ship follow-up, not the gate. ### 2. Scope and sizing - A story is one PR plus one acceptance test. Flag a story that bundles a @@ -61,8 +74,9 @@ Task tool (general-purpose): requirement. ### 3. Metadata and linkage - - Does each story carry an `estimate`? A missing estimate leaves sizing to a - later planner. Treat absent estimate as Important. + - Estimate is a Linear field set at save time, not body text, and no + execution stage reads it. Only flag a missing estimate (Minor) if the + target workspace actually uses estimates; never treat it as a blocker. - Is `priority` used for category (security/infra/epic = High, feature = Medium, exploratory = Low), NOT as a status marker? Flag `priority: No priority` used to mean "deferred" or "verify-only" — that belongs in status @@ -82,12 +96,16 @@ Task tool (general-purpose): ticket and no note? Flag it for a tracking note so the finding is not lost. ## Calibration (do not false-positive) - This style has three archetypes with intended shapes. Judge against the right one: + This style has four archetypes with intended shapes. Judge against the right one: - Epic: Goal / Outcomes / Out of scope / Stories under this epic. 2-8 children. - - Story: User Story / Problem / Solution / Requirements / Evaluation. One PR. - - Hardening: Summary / Issues (each numbered with a **Fix:**). Hardening tickets - INTENTIONALLY bundle 2-3 fixes in one file and put verification inside the - **Fix:** prose. Do NOT flag that as a sizing or acceptance-criteria defect. + - Story: Anchor / User Story / Problem / Solution / Out of scope / Requirements / + Evaluation. One PR. Standalone stories legitimately carry `**Epic:** none`. + - Hardening: Summary / Issues (each numbered with a **Fix:** and a **Verify:**). + Hardening tickets INTENTIONALLY bundle 2-3 fixes in one file; the per-issue + **Verify:** line is the acceptance. Do NOT flag the bundling as a sizing defect. + - Task: Type / Anchor / Problem / Verification. A one-cause bug or a chore. + It has no persona and no requirement-to-evaluation mapping BY DESIGN; do not + flag the missing User Story or Requirements as a defect. Do not invent problems to look thorough. A correct, well-scoped issue earns a short Strengths list and a "File as-is" verdict. @@ -104,9 +122,10 @@ Task tool (general-purpose): wrong behavior or sends an implementer to a file that does not exist.] #### Important (should fix before filing) - [Incomplete sweep, net-new work bundled into a small change, missing estimate, - undisclosed data-vs-code gap, filing without the requested approval, likely - duplicate, broken epic/child linkage.] + [Incomplete sweep, net-new work bundled into a small change, an acceptance + item that is not machine-checkable headless or that stubs the thing under + test, undisclosed data-vs-code gap, filing without the requested approval, + likely duplicate, broken epic/child linkage.] #### Minor (nice to fix) [Title polish, label refinement, weak deferral rationale, ordering.] diff --git a/skill/scripts/tests/test_validate.py b/skill/scripts/tests/test_validate.py index fb629a7..c59597d 100644 --- a/skill/scripts/tests/test_validate.py +++ b/skill/scripts/tests/test_validate.py @@ -9,6 +9,7 @@ GOOD_STORY = """**Epic:** AI Foundation / Platform infra **Title:** Add Helicone proxy for all OpenAI calls with cost + latency dashboard +**Anchor:** api/src/lib/openai-client.ts:getOpenAIClient **Labels:** `api`, `ai` **Priority:** No priority @@ -24,6 +25,10 @@ Add Helicone as a transparent proxy in front of OpenAI. The existing caller imports a small `getOpenAIClient()` wrapper. This is one config change, no business-logic refactor. +## Out of scope + +* The moderation service's own retry policy; that stays as-is. + ## Requirements 1. Create `api/src/lib/openai-client.ts` exporting `getOpenAIClient()` configured with `baseURL: https://oai.helicone.ai/v1`. @@ -32,7 +37,221 @@ ## Evaluation 1. **Validates R1**: A test call from `api/` shows up in the Helicone dashboard within 10s. -2. **Validates all**: A 24h soak in staging shows zero fallback log lines. +2. **Validates all**: `pytest api/tests/test_openai_client.py` passes and `rg "from 'openai'"` outside the wrapper returns zero hits. +""" + +# A story with no parent epic (a standalone refactor). The **Epic:** none +# sentinel is deliberate; it must not error. +STANDALONE_STORY = """**Epic:** none +**Title:** Refactor the pricing module in the api service +**Anchor:** api/src/pricing.ts:computePrice +**Labels:** `api`, `tech-debt` + +## User Story + +As a developer, I want the pricing module split by concern, so that changes stay local. + +## Problem + +Today `api/src/pricing.ts` mixes tax, discount, and rounding in one function. + +## Solution + +Split into three pure helpers behind the same signature. No behavior change. + +## Out of scope + +* New pricing rules; this is a structure-only refactor. + +## Requirements + +1. Extract `applyTax`, `applyDiscount`, `roundPrice` from `computePrice`. + +## Evaluation + +1. **Validates R1**: `pytest api/tests/test_pricing.py` passes with identical outputs. +""" + +# A lightweight Bug/Chore/Spike. Type header selects the minimal shape: no +# persona, no requirement->evaluation mapping, but an Anchor and a runnable +# Verification are still required. +GOOD_TASK = """**Type:** Bug +**Title:** Fix drag reset on pointerup in the 3D graph +**Anchor:** web/components/graph-3d.tsx:657 +**Labels:** `web`, `ui-ux` +**Priority:** Medium + +## Problem + +`dragPinned` is set true on pointerdown but reset to false on pointerup (graph-3d.tsx:657), so a pinned node snaps back. + +## Verification + +`npx playwright test drag-pin` shows the node stays pinned after release. +""" + +# Fenced code carrying a drift heading, an AI-tell word, and an em-dash. All of +# it is quoted source, so none of it may fire. +FENCE_NOISE = """**Epic:** Platform +**Title:** Stand up the notifications service in the api +**Anchor:** api/src/notify.ts:send +**Labels:** `api`, `tech-debt` + +## User Story + +As a user, I want notifications, so that I stay informed. + +## Problem + +Today there is no notifications service in `api/`. + +## Solution + +Build a small `send()` module. One file. + +## Out of scope + +* Delivery retries. + +## Requirements + +1. Create `api/src/notify.ts` exporting `send()`. + +```md +## Description +This robust, comprehensive layer — with an em-dash — is the OLD template. +``` + +## Evaluation + +1. **Validates R1**: `pytest -k notify` passes. +""" + +# A single story split by a mid-body `---`. The Requirements and Evaluation +# after the rule must stay in the same block, not be dropped from validation. +TAILDROP_STORY = """**Epic:** Platform +**Title:** Wire structured logging into the api service +**Anchor:** api/src/log.ts:logger +**Labels:** `api`, `tech-debt` + +## User Story + +As an operator, I want JSON logs, so that I can query them. + +## Problem + +Today `api/` logs unstructured text to stdout. + +## Solution + +Add a pino logger. One module, no behavior change. + +## Out of scope + +* Log rotation. + +--- + +## Requirements + +1. Add `api/src/log.ts` exporting a configured pino instance. + +## Evaluation + +1. **Validates R1**: `pytest -k logging` shows JSON on stdout. +""" + +# "Improve auth (blocks PROJ-19)": the digit 19 must not launder the vague verb. +VAGUE_WITH_ID = """**Epic:** Platform +**Title:** Improve the login throttle on the api +**Anchor:** api/src/login.ts:throttle +**Labels:** `api`, `compliance` + +## User Story + +As a user, I want protection, so that my account is safe. + +## Problem + +Improve auth (blocks PROJ-19) is the only note today. + +## Solution + +Add a limiter. One middleware. + +## Out of scope + +* Captcha. + +## Requirements + +1. Add `rateLimit()`. + +## Evaluation + +1. **Validates R1**: 100 rapid `POST /login` return 429. +""" + +# "Optimize ... p95 < 200ms": the metric is adjacent, so no vague-verb error. +GOOD_OPTIMIZE = """**Epic:** Platform +**Title:** Add an LRU cache to the search endpoint +**Anchor:** api/src/search.ts:handleSearch +**Labels:** `api`, `tech-debt` + +## User Story + +As a user, I want fast search, so that results feel instant. + +## Problem + +Today `/search` recomputes on every call. + +## Solution + +Optimize `/search` to p95 < 200ms on a 10k-event corpus with an LRU cache. One module. + +## Out of scope + +* Query-syntax changes. + +## Requirements + +1. Add an LRU cache to `handleSearch`. + +## Evaluation + +1. **Validates R1**: `pytest -k search_latency` asserts p95 < 200ms. +""" + +# A prose Anchor must be flagged (WARN) as not addressable. +ANCHOR_PROSE = """**Epic:** Platform +**Title:** Wire the rate limiter into the login route +**Anchor:** the whole auth flow +**Labels:** `api`, `compliance` + +## User Story + +As a user, I want brute-force protection, so that my account is safe. + +## Problem + +Today the login route has no throttle. + +## Solution + +Add a per-IP limiter. One middleware. + +## Out of scope + +* Captcha. + +## Requirements + +1. Add `rateLimit()` to the login route. + +## Evaluation + +1. **Validates R1**: 100 rapid `POST /login` return 429. """ GOOD_EPIC = """**Title:** Epic: SEO & Discoverability @@ -294,6 +513,16 @@ def codes(text): return {x.code for x in errors(text)} +def warn_codes(text): + v, _ = validate_text(text) + return {x.code for x in v if x.level == "WARN"} + + +def n_blocks(text): + _, n = validate_text(text) + return n + + def main(): passed = failed = 0 @@ -323,6 +552,44 @@ def check(name, cond): check("bad_epic_count flags epic-story-count", "epic-story-count" in codes(BAD_EPIC_COUNT)) check("bad_eval_mapping flags eval-mapping", "eval-mapping" in codes(BAD_EVAL_MAPPING)) + # New good drafts: zero errors. + for name, txt in [("standalone_story", STANDALONE_STORY), ("good_task", GOOD_TASK), + ("fence_noise", FENCE_NOISE), ("taildrop_story", TAILDROP_STORY), + ("good_optimize", GOOD_OPTIMIZE)]: + errs = errors(txt) + check(f"{name} has no errors", not errs) + if errs: + for e in errs: + print(f" unexpected: L{e.line} [{e.code}] {e.message}") + + # Standalone story: epic-backlink is a WARN, never an ERROR. + check("standalone_story does not error on epic-backlink", + "epic-backlink" not in codes(STANDALONE_STORY)) + + # Fence blindness: a drift heading / AI-tell / em-dash inside a code fence + # is quoted source and must not fire. + fence_codes = codes(FENCE_NOISE) + check("fence_noise ignores fenced section-drift", "section-drift" not in fence_codes) + check("fence_noise ignores fenced ai-tell", "ai-tell" not in fence_codes) + check("fence_noise ignores fenced em-dash", "em-dash" not in fence_codes) + + # Mid-body `---` must not split a single issue; the tail stays validated. + check("taildrop_story is one block", n_blocks(TAILDROP_STORY) == 1) + check("taildrop_story keeps its tail sections", + "section-missing" not in codes(TAILDROP_STORY)) + + # METRIC false-negative: a bare issue id must not launder a vague verb. + check("vague_with_id still flags vague-verb", "vague-verb" in codes(VAGUE_WITH_ID)) + check("good_optimize accepts an adjacent metric", "vague-verb" not in codes(GOOD_OPTIMIZE)) + + # Anchor: prose warns, a good-shaped anchor does not. + check("anchor_prose warns anchor-shape", "anchor-shape" in warn_codes(ANCHOR_PROSE)) + check("good_story has a clean anchor", "anchor-shape" not in warn_codes(GOOD_STORY)) + check("good_story does not warn anchor-missing", "anchor-missing" not in warn_codes(GOOD_STORY)) + + # The undischargeable labels-verify WARN is gone. + check("labels-verify WARN retired", "labels-verify" not in warn_codes(GOOD_STORY)) + print(f"\n{passed} passed, {failed} failed") return 1 if failed else 0 diff --git a/skill/scripts/validate_issue.py b/skill/scripts/validate_issue.py index d5fe5fe..a48410d 100644 --- a/skill/scripts/validate_issue.py +++ b/skill/scripts/validate_issue.py @@ -2,9 +2,12 @@ """Validate a linear-issue-craft draft against the mechanical house-style rules. This is the hard gate the skill runs on every draft before showing it to the -user. It checks only the rules a script can verify deterministically (em-dash -in prose, vague verbs, AI tells, section order, Fix lines, label shape, ...). -Judgment calls (is the persona real? is the scope one PR?) stay with the agent. +user (and, once welded, the gate a save-hook runs before every create). It +checks only the rules a script can verify deterministically: em-dash in prose, +vague verbs without an adjacent metric, AI tells, section order, Fix lines, +Anchor shape, label shape, and so on. Judgment calls (is the persona real? is +the scope one PR? does the named Anchor actually exist in the repo?) stay with +the agent and the pipeline, which can see the repo. Usage: python3 validate_issue.py draft.md # validate a file @@ -12,7 +15,10 @@ A draft may contain multiple issues (an epic plus its child stories) separated by lines that are exactly `---`; each block carrying a `**Title:**` line is -validated on its own. +validated on its own. A `---` inside a fenced code block, or one that only +splits body prose (the following text carries no new `**Title:**`), is treated +as content, not a separator, so it can never silently drop the tail of an issue +from validation. Exit code 0 when there are no ERRORs, 1 otherwise. WARNs never fail the gate. """ @@ -27,28 +33,86 @@ # Lines that may legitimately carry an em-dash: role-prefixed titles and the # header fields that echo a (possibly role-prefixed) title. The house style # puts the colon inside the bold markers (`**Title:**`), so tolerate both. -TITLE_FIELD = re.compile(r"^\s*\*\*\s*(Title|Parent|Epic|Type)\s*:?\s*\*\*", re.IGNORECASE) +TITLE_FIELD = re.compile(r"^\s*\*\*\s*(Title|Parent|Epic|Type|Anchor)\s*:?\s*\*\*", re.IGNORECASE) + +# A bullet under "Stories under this epic" lists a child story title verbatim; a +# role-prefixed child (`Venue — Demand forecasting`) legitimately carries the +# em-dash the title pattern mandates, so those lines are exempt from the em-dash +# scan too. +STORY_BULLET = re.compile(r"^\s*([-*]|\d+\.)\s+\S") VAGUE_VERBS = re.compile(r"\b(improv\w*|enhanc\w*|optimiz\w*)\b", re.IGNORECASE) -METRIC = re.compile(r"(\d|%|\bp\d{2}\b|\bms\b|\bms\.|\bseconds?\b|\bx\b)", re.IGNORECASE) +# A real target metric: a percentage, a pNN latency marker, a comparison bound, +# or a number bound to a unit. A bare integer (a version, issue id, port, or +# line number) is NOT a metric, so "Improve auth (blocks PROJ-19)" no longer +# slips through on the 19. +METRIC = re.compile( + r"(\bp\d{2,3}\b" + r"|\d+\s?%" + r"|[<>]=?\s?\d" + r"|\b\d+(\.\d+)?\s?(ms|s|sec|secs|seconds?|m|min|mins|minutes?|h|hours?" + r"|kb|mb|gb|rps|qps|x)\b)", + re.IGNORECASE, +) # AI tells: the skill's anti-patterns plus the operator's globally banned words. +# Matched on word boundaries against code-stripped prose so that identifiers +# (`robust-parser.ts`, `optimizePackageImports`) and quoted source do not trip. AI_TELLS = [ "generated with claude", "co-authored-by", "delve", "it's worth noting", "it is worth noting", "in conclusion", "navigate the landscape", "comprehensive", "robust", "seamless", "gracefully", "straightforward", "leverages", "leveraging", "this ensures", "for clarity", "this is critical", - "thoughtful", "production-ready", + "thoughtful", "production-ready", "elegant", ] +AI_TELL_RES = [(t, re.compile(r"(?<!\w)" + re.escape(t) + r"(?!\w)", re.IGNORECASE)) + for t in AI_TELLS] + +PRIORITIES = {"urgent", "high", "medium", "low", "no priority"} + +# The Anchor is the one new authoring primitive: a named, addressable thing the +# held-out probe can target. Accept exactly the shapes a machine can resolve; +# reject prose ("the auth flow"). Comma-separated anchors are allowed; each part +# must match one shape. +ANCHOR_RE = re.compile( + r"""^( + [\w./\-]+\.\w+:\d+ # file.ext:line + | [\w./\-]+\.\w+:[A-Za-z_]\w* # file.ext:symbol + | [A-Za-z_]\w*(\.[A-Za-z_]\w*){2,} # dotted.module.function (>=3 segments) + | (GET|POST|PUT|PATCH|DELETE|HEAD|OPTIONS)\s+/\S*\s*(->|→|returns?)\s*\d{3} # METHOD /path -> status + | playwright:\S.* # playwright:selector + )$""", + re.VERBOSE | re.IGNORECASE, +) + +# A last-line acceptance check should be runnable headless. Flag the closer when +# it leans on something no headless stage can observe (soak, dashboard, staging +# credentials) AND carries no runnable token. +UNOBSERVABLE = re.compile( + r"\b(soak|staging|dashboard|overnight|24\s?-?\s?h(our)?s?|prod(uction)?\b[^.]*\bcredential)", + re.IGNORECASE, +) +RUNNABLE = re.compile( + r"(`[^`]+`|\bcurl\b|\bpytest\b|\bnpx?\b|\bnpm\b|\bpnpm\b|\bplaywright\b|\brg\b|\bmake\b" + r"|\b(GET|POST|PUT|PATCH|DELETE)\b|https?://|\bexit\s?code\b|\breturns?\s+\d)", + re.IGNORECASE, +) ARCHETYPE_SECTIONS = { "epic": ["Goal", "Outcomes", "Out of scope", "Stories under this epic"], "story": ["User Story", "Problem", "Solution", "Requirements", "Evaluation"], "hardening": ["Summary", "Issues"], + # Lightweight shape for bugs, chores, spikes, ops tasks. The full story + # ceremony (persona, requirement->evaluation mapping) is optional here; the + # minimum contract (Anchor + a runnable Verification) still applies. + "task": ["Problem", "Verification"], } # Headings that signal the old/simple template drifted back in. DRIFT_HEADINGS = {"Description", "Acceptance Criteria"} +HEADING_RE = re.compile(r"^(#{2,3})\s+(.*\S)\s*$") +FENCE_RE = re.compile(r"^\s*(```|~~~)") + @dataclass class Violation: @@ -65,24 +129,63 @@ class Block: priority: str archetype: str start_line: int - lines: list # (abs_line_no, text) - headings: list # (abs_line_no, heading_text) for H2 + lines: list # (abs_line_no, text) + headings: list # (abs_line_no, heading_text) for H2/H3 outside fences + fenced: set # abs line numbers inside fenced code + story_bullets: set # abs line numbers of child-story list bullets + + +def strip_inline_code(s): + """Blank out `inline code` spans so identifiers and quoted source do not + trip the prose scanners.""" + return re.sub(r"`[^`]*`", " ", s) + + +def fenced_line_numbers(raw_lines, offset=0): + """Return the set of 1-based line numbers (offset + index) that sit inside a + fenced code block, fence markers included.""" + fenced, in_fence = set(), False + for i, ln in enumerate(raw_lines): + n = offset + i + 1 + if FENCE_RE.match(ln): + fenced.add(n) + in_fence = not in_fence + continue + if in_fence: + fenced.add(n) + return fenced def split_blocks(text): - """Split the draft on `---` separators into issue blocks with a Title.""" - raw_lines = text.splitlines() - blocks, cur, cur_start = [], [], 1 - for i, ln in enumerate(raw_lines, start=1): - if ln.strip() == "---": - if any("**Title:**" in x for _, x in cur): - blocks.append((cur_start, cur)) - cur, cur_start = [], i + 1 + """Split the draft into issue blocks on `---` separators, fence-aware. + + A `---` inside a fenced block is content. A `---` that splits off a tail + carrying no new `**Title:**` is a thematic break, so the tail is merged back + into the issue it belongs to instead of being silently dropped.""" + raw = text.splitlines() + fenced = fenced_line_numbers(raw) + # Cut into segments on bare `---` lines that are not inside a fence. + segments, cur, cur_start = [], [], 1 + for i, ln in enumerate(raw): + n = i + 1 + if ln.strip() == "---" and n not in fenced: + segments.append((cur_start, cur)) + cur, cur_start = [], n + 1 else: - cur.append((i, ln)) - if any("**Title:**" in x for _, x in cur): - blocks.append((cur_start, cur)) - return blocks + cur.append((n, ln)) + segments.append((cur_start, cur)) + # A segment with a Title starts a block; a title-less segment (a mid-body + # `---`, or trailing prose) attaches to the block before it. A title-less + # leading segment (preamble before the first issue) is dropped. + blocks = [] + for start, seg in segments: + has_title = any("**Title:**" in t for _, t in seg) + if has_title: + blocks.append([start, list(seg)]) + elif blocks: + blocks[-1][1].extend(seg) + return [(start, blk) for start, blk in blocks + if any("**Title:**" in t for _, t in blk)] def field(lines, name): @@ -102,8 +205,11 @@ def strip_code(s): return s.replace("`", "").strip() -def detect_archetype(title, heading_names): +def detect_archetype(title, heading_names, type_field): t = strip_code(title).lower() + ty = strip_code(type_field).lower() + if ty in ("bug", "chore", "spike", "task", "ops") or re.match(r"^(bug|chore|spike|ops)\b\s*[:\-]", t): + return "task" if t.startswith("epic:") or ("Goal" in heading_names and "Stories under this epic" in heading_names): return "epic" if t.startswith("harden") or ("Summary" in heading_names and "Issues" in heading_names): @@ -112,12 +218,27 @@ def detect_archetype(title, heading_names): def make_block(start_line, lines): - headings = [(n, m.group(1).strip()) + abs_nos = [n for n, _ in lines] + lo = min(abs_nos) if abs_nos else start_line + raw = [ln for _, ln in lines] + fenced = fenced_line_numbers(raw, offset=lo - 1) + headings = [(n, m.group(2).strip()) for n, ln in lines - for m in [re.match(r"^##\s+(.*\S)\s*$", ln)] if m] + for m in [HEADING_RE.match(ln)] if m and n not in fenced] heading_names = [h for _, h in headings] + # Child-story bullets under "Stories under this epic" carry verbatim titles. + story_bullets, in_stories = set(), False + for n, ln in lines: + if n in fenced: + continue + m = HEADING_RE.match(ln) + if m: + in_stories = m.group(2).strip() == "Stories under this epic" + continue + if in_stories and STORY_BULLET.match(ln): + story_bullets.add(n) title = strip_code(field(lines, "Title")) - archetype = detect_archetype(title, heading_names) + archetype = detect_archetype(title, heading_names, field(lines, "Type")) return Block( title=title, labels=parse_labels(field(lines, "Labels")), @@ -126,12 +247,16 @@ def make_block(start_line, lines): start_line=start_line, lines=lines, headings=headings, + fenced=fenced, + story_bullets=story_bullets, ) def check_em_dash(block, out): for n, ln in block.lines: - if EM_DASH in ln and not TITLE_FIELD.match(ln): + if n in block.fenced or TITLE_FIELD.match(ln) or n in block.story_bullets: + continue + if EM_DASH in strip_inline_code(ln): out.append(Violation("ERROR", n, "em-dash", "Em-dash in prose. Use a colon, comma, or rewrite " "(em-dash is allowed only in role-prefixed titles).")) @@ -139,18 +264,29 @@ def check_em_dash(block, out): def check_vague_verbs(block, out): for n, ln in block.lines: - if VAGUE_VERBS.search(ln) and not METRIC.search(ln): - verb = VAGUE_VERBS.search(ln).group(0) + if n in block.fenced: + continue + s = strip_inline_code(ln) + m = VAGUE_VERBS.search(s) + if not m: + continue + # The metric must sit near the verb (the next ~8 words), not anywhere on + # the line, so an unrelated number cannot launder a vague action. + window = " ".join(s[m.start():].split()[:9]) + if not METRIC.search(window): out.append(Violation("ERROR", n, "vague-verb", - f'Vague verb "{verb}" without a target metric. Name a ' - f'number (p95, %, ms, count) or use a concrete action verb.')) + f'Vague verb "{m.group(0)}" without an adjacent target metric. ' + f'Name a number with a unit (p95, %, ms, count) next to it, ' + f'or use a concrete action verb.')) def check_ai_tells(block, out): for n, ln in block.lines: - low = ln.lower() - for tell in AI_TELLS: - if tell in low: + if n in block.fenced: + continue + s = strip_inline_code(ln) + for tell, rex in AI_TELL_RES: + if rex.search(s): out.append(Violation("ERROR", n, "ai-tell", f'AI tell / banned phrase: "{tell.strip()}". Delete or rewrite.')) @@ -179,58 +315,104 @@ def check_sections(block, out): # Bundled-concept story variant: ### Concept blocks carry the solution, so # the ## Solution H2 is replaced by them and is not separately required. if block.archetype == "story" and any( - re.match(r"^###\s+(Concept\b|Why bundled\b)", ln) for _, ln in block.lines + n not in block.fenced and re.match(r"^###\s+(Concept\b|Why bundled\b)", ln) + for n, ln in block.lines ): expected = [s for s in expected if s != "Solution"] - # drift headings for n, h in block.headings: if h in DRIFT_HEADINGS: out.append(Violation("ERROR", n, "section-drift", f'Section "{h}" is the old template. House style uses ' f'Problem / Solution / Evaluation.')) - # required present for sec in expected: if sec not in present: out.append(Violation("ERROR", block.start_line, "section-missing", f'Missing required section "## {sec}" for {block.archetype}.')) - # relative order of the ones that are present idx = [present.index(s) for s in expected if s in present] if idx != sorted(idx): out.append(Violation("ERROR", block.start_line, "section-order", f"Sections out of order. Expected: {' -> '.join(expected)}.")) +def check_anchor(block, out): + """Every story/task must point at a machine-resolvable Anchor. WARN-only in + the skill validator (it sees a markdown draft, not a create-vs-edit call); + the save-hook promotes this to a create-only ERROR.""" + raw = field(block.lines, "Anchor") + if not raw: + out.append(Violation("WARN", block.start_line, "anchor-missing", + "No **Anchor:** line. Name the thing a probe can target: " + "file.ext:line, file.ext:symbol, module.function, " + "METHOD /path -> status, or playwright:selector.")) + return + parts = [p.strip() for p in raw.split(",") if p.strip()] + bad = [p for p in parts if not ANCHOR_RE.match(p)] + if bad: + out.append(Violation("WARN", block.start_line, "anchor-shape", + f'Anchor "{bad[0]}" is prose, not addressable. Use ' + f'file.ext:line, file.ext:symbol, module.function, ' + f'METHOD /path -> status, or playwright:selector.')) + + def check_story(block, out): - if not field(block.lines, "Epic"): - out.append(Violation("ERROR", block.start_line, "epic-backlink", - "Story is missing the **Epic:** backlink line.")) - # Evaluation items must cite a requirement - in_eval, saw_item = False, False + epic = field(block.lines, "Epic") + if not epic: + out.append(Violation("WARN", block.start_line, "epic-backlink", + "No **Epic:** backlink. Add the parent epic, or " + "**Epic:** none for a deliberately standalone story.")) + check_anchor(block, out) + headings = [h for _, h in block.headings] + if "Out of scope" not in headings and "Non-goals" not in headings: + out.append(Violation("WARN", block.start_line, "story-non-goals", + "No **## Out of scope** section. Name one non-goal so the " + "implementer does not gold-plate past the intended scope.")) + # Evaluation items must cite a requirement; the closer must be runnable. + in_eval, saw_item, last_item = False, False, None for n, ln in block.lines: - h = re.match(r"^##\s+(.*\S)\s*$", ln) + if n in block.fenced: + continue + h = HEADING_RE.match(ln) if h: - in_eval = h.group(1).strip() == "Evaluation" + in_eval = h.group(2).strip() == "Evaluation" continue if in_eval and re.match(r"^\s*\d+\.\s+\S", ln): - saw_item = True + saw_item, last_item = True, (n, ln) if "Validates R" not in ln and "Validates all" not in ln: out.append(Violation("ERROR", n, "eval-mapping", "Evaluation item must start with **Validates R<n>** " "or **Validates all**.")) - if "Evaluation" in [h for _, h in block.headings] and not saw_item: + if "Evaluation" in headings and not saw_item: out.append(Violation("WARN", block.start_line, "eval-empty", "Evaluation section has no numbered items.")) + if last_item: + n, ln = last_item + if UNOBSERVABLE.search(ln) and not RUNNABLE.search(ln): + out.append(Violation("WARN", n, "eval-unobservable", + "Final acceptance leans on a soak/dashboard/staging check " + "no headless stage runs. Make it a runnable command; move " + "the soak to a 'Post-ship follow-up' line.")) + + +def check_task(block, out): + check_anchor(block, out) + in_verify, runnable = False, False + for n, ln in block.lines: + if n in block.fenced: + continue + h = HEADING_RE.match(ln) + if h: + in_verify = h.group(2).strip() == "Verification" + continue + if in_verify and RUNNABLE.search(ln): + runnable = True + if "Verification" in [h for _, h in block.headings] and not runnable: + out.append(Violation("WARN", block.start_line, "verify-unrunnable", + "Verification has no runnable check (a command, endpoint, or " + "expected exit/return). A bug/chore still needs one.")) def check_epic(block, out): - in_stories, count = False, 0 - for _, ln in block.lines: - h = re.match(r"^##\s+(.*\S)\s*$", ln) - if h: - in_stories = h.group(1).strip() == "Stories under this epic" - continue - if in_stories and re.match(r"^\s*[-*]\s+Story\s+\d+", ln): - count += 1 + count = len(block.story_bullets) if not (2 <= count <= 8): out.append(Violation("ERROR", block.start_line, "epic-story-count", f"Epic lists {count} stories; must be 2-8 named in " @@ -241,9 +423,10 @@ def check_epic(block, out): def check_hardening(block, out): - # each ### issue needs a **Fix:** before the next ### issue_line, has_fix, n_issues = None, False, 0 for n, ln in block.lines: + if n in block.fenced: + continue if re.match(r"^###\s+\d", ln): if issue_line is not None and not has_fix: out.append(Violation("ERROR", issue_line, "fix-missing", @@ -265,9 +448,14 @@ def check_labels(block, out): if n < 2: out.append(Violation("ERROR", block.start_line, "labels-shape", f"{n} label(s); need 1 domain + >=1 capability.")) - out.append(Violation("WARN", block.start_line, "labels-verify", - "Verify the domain label exists in the target workspace " - "(run list_issue_labels); do not invent a taxonomy.")) + + +def check_priority(block, out): + p = block.priority.strip().lower() + if p and p not in PRIORITIES: + out.append(Violation("WARN", block.start_line, "priority-enum", + f'Priority "{block.priority}" is not one of ' + f'Urgent / High / Medium / Low / No priority.')) def validate_block(block): @@ -278,12 +466,15 @@ def validate_block(block): check_ai_tells(block, out) check_sections(block, out) check_labels(block, out) + check_priority(block, out) if block.archetype == "story": check_story(block, out) elif block.archetype == "epic": check_epic(block, out) elif block.archetype == "hardening": check_hardening(block, out) + elif block.archetype == "task": + check_task(block, out) return out From ab06da8fe96337f11d777b4421e1960005cb1155 Mon Sep 17 00:00:00 2001 From: Peyman <peyman.h.j@gmail.com> Date: Mon, 6 Jul 2026 01:39:06 +0200 Subject: [PATCH 2/4] feat(hooks): add create-only save-hook that welds Gate 1 to the tool boundary A headless subagent filing a batch can skip the skill's interactive Gate 1; this PreToolUse hook on mcp__linear__save_issue runs the same validator at the point of creation, and PreToolUse fires for subagent tool calls, so the batch path is covered. Create-only (updates with an id pass untouched, so the pre-Anchor corpus is never orphaned), build-lane scoped (governance labels exempt), observe by default and enforce on an operator flip via permissionDecision: deny. A missing or prose Anchor is promoted to a block on create. Fails open on bad input. Ships in the skill repo as a portable reference hook; the operator installs it and owns the enforce flip (gated on a clean corpus replay). test_hook.sh covers the 7-case decision matrix. --- CHANGELOG.md | 3 + hooks/README.md | 62 ++++++++++++ hooks/test_hook.sh | 46 +++++++++ hooks/validate-issue-on-save.sh | 172 ++++++++++++++++++++++++++++++++ skill/SKILL.md | 2 +- 5 files changed, 284 insertions(+), 1 deletion(-) create mode 100644 hooks/README.md create mode 100755 hooks/test_hook.sh create mode 100755 hooks/validate-issue-on-save.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a0bd26..271decd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +### Added (save-hook: weld the boundary) +- `hooks/validate-issue-on-save.sh`: a Claude Code PreToolUse hook for `mcp__linear__save_issue` that runs the Gate-1 validator at the tool boundary, so the check cannot be skipped on the headless batch-filing path (a PreToolUse hook fires for subagent tool calls too). Create-only (an update with an `id` is left alone, so the pre-Anchor corpus is never orphaned); build-lane scoped (governance labels `human`/`eval` exempt); `observe` by default (logs would-blocks, allows) and `enforce` on an operator flip (denies with the validator's re-fileable output via `permissionDecision: deny`). Fails open on any unexpected input. On a create, a missing or prose-shaped Anchor is promoted from WARN to a block. `hooks/README.md` documents install + the operator-owned enforce flip (gated on a clean corpus replay); `hooks/test_hook.sh` covers the decision matrix (7 cases). + ### Added (Anchor + validator hygiene) - **Anchor field.** Every story and task now carries a `**Anchor:**` line: one machine-resolvable target (`file.ext:line`, `file.ext:symbol`, `module.function`, `METHOD /path -> status`, or `playwright:selector`). It is the interface the downstream held-out probe binds to; a probe cannot test a thing the issue never named. The validator WARNs on a missing or prose-shaped anchor; a save-hook (separate increment) promotes it to a create-only ERROR. - **Task archetype** for a single bug, chore, spike, or ops change too small for the full story ceremony. Selected with `**Type:** Bug|Chore|Spike|Ops` (or a title prefix); shape is Anchor / Problem / Verification. Fixes the coverage gap where a one-line bug had to be inflated into a Story or mis-filed as a one-issue Hardening ticket. New title pattern (6) and canonical Example 5. diff --git a/hooks/README.md b/hooks/README.md new file mode 100644 index 0000000..c14c226 --- /dev/null +++ b/hooks/README.md @@ -0,0 +1,62 @@ +# Save-hook: validate a Linear issue at the tool boundary + +`validate-issue-on-save.sh` is a Claude Code **PreToolUse** hook for `mcp__linear__save_issue`. It runs the same validator as the skill's Gate 1, but at the point of creation, so the check cannot be skipped: a PreToolUse hook fires for tool calls made inside subagents too, which is exactly the headless batch-filing path where the interactive gates get bypassed. + +## What it does + +- Fires only on **create** calls (`save_issue` with no `id`). An update is left alone, so the existing corpus of pre-Anchor issues is never re-validated or orphaned. The Anchor requirement is therefore create-only. +- Reconstructs a validator-shaped document from the create payload (synthesizes the `**Title:**` / `**Labels:**` / `**Priority:**` header lines from the Linear fields when the description does not already carry them), then runs `skill/scripts/validate_issue.py`. +- Scopes to the **build lane**: an issue whose only labels are governance labels (default `human`, `eval`) is exempt. Decision and adjudication issues are not code and do not go through the build pipeline. +- Two modes: + - `observe` (default): logs a would-block on any validator ERROR (or, on a create, a missing/prose Anchor) and **allows**. This measures the gap without blocking filing. + - `enforce`: **denies** the create with the validator's re-fileable output as the reason (`permissionDecision: deny`). +- Fails **open** on anything unexpected (no `jq`/`python3`, unparseable payload, unreadable validator). A validation gate must never brick a session on its own bug. + +## Install (observe mode) + +Wire it into `~/.claude/settings.json` under `hooks.PreToolUse`, matched to the save tool by name: + +```json +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "mcp__linear__save_issue", + "hooks": [ + { "type": "command", + "command": "/ABSOLUTE/PATH/TO/linear-issue-craft/hooks/validate-issue-on-save.sh", + "timeout": 20 } + ] + } + ] + } +} +``` + +Config is env-only, so the script stays portable: + +| Env var | Default | Meaning | +|---|---|---| +| `LINEAR_ISSUE_GATE_MODE` | `observe` | `observe` or `enforce` | +| `LINEAR_ISSUE_GATE_VALIDATOR` | sibling `../skill/scripts/validate_issue.py` | validator path | +| `LINEAR_ISSUE_GATE_EXEMPT` | `human,eval` | governance labels that are never gated | +| `LINEAR_ISSUE_GATE_LOG` | `$TMPDIR/linear-issue-gate.log` | would-block / decision log | + +## Flipping to enforce (operator-owned) + +Do **not** flip to enforce on faith. The gate blocks real creates, so first prove the validator does not false-fail the workspace's existing issues: + +1. **Replay precondition.** Run the current validator over a representative sample of the workspace's existing issues, rendered in create-payload shape, and confirm **zero** create-blocking false-fails (a false-fail = a validator bug, not a genuine style violation). Only then is blocking safe. +2. Run `bash hooks/test_hook.sh` and confirm it passes. +3. Set `LINEAR_ISSUE_GATE_MODE=enforce` (e.g. in the hook's environment, or a wrapper) for **one** workspace first, watch the log for a week, and confirm `filed-count == intended-count` on a bulk run (a gap means a subagent got a `deny` and did not re-draft, the one blind spot this gate has). +4. Fan out only after the single-workspace bake is clean. + +The agent that authors issues does not flip this itself; the flip is a human step, the same stance as the held-out merge gate. + +## Test + +```bash +bash hooks/test_hook.sh +``` + +Covers: observe never denies, enforce denies a bad create, enforce allows a good create, updates are ignored, a create with no Anchor is blocked in enforce, governance-only labels are exempt, and observe logs the would-block. diff --git a/hooks/test_hook.sh b/hooks/test_hook.sh new file mode 100755 index 0000000..05896b9 --- /dev/null +++ b/hooks/test_hook.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# Tests for validate-issue-on-save.sh. Feeds PreToolUse stdin payloads and asserts +# the allow/deny decision. Run: bash hooks/test_hook.sh +set -uo pipefail +HOOK="$(cd "$(dirname "$0")" && pwd)/validate-issue-on-save.sh" +LOG="$(mktemp)" +export LINEAR_ISSUE_GATE_LOG="$LOG" +pass=0; fail=0 +ok() { pass=$((pass+1)); } +bad() { fail=$((fail+1)); printf ' FAIL: %s\n' "$1"; } + +# Returns "deny" if the hook emitted a deny decision, else "allow". +decide() { # $1 = mode, stdin = payload + local mode="$1" out + out=$(LINEAR_ISSUE_GATE_MODE="$mode" bash "$HOOK") + if printf '%s' "$out" | grep -q '"permissionDecision": *"deny"'; then + echo deny + else + echo allow + fi +} + +# Bodies with REAL newlines; jq --arg escapes them into valid JSON. +GOOD_BODY=$'**Epic:** none\n**Title:** Add an LRU cache to the search endpoint\n**Anchor:** api/src/search.ts:handleSearch\n\n## User Story\n\nAs a user, I want fast search, so results feel instant.\n\n## Problem\n\nToday /search recomputes each call.\n\n## Solution\n\nAdd an LRU cache. One module.\n\n## Out of scope\n\n* Query syntax.\n\n## Requirements\n\n1. Add cache to handleSearch.\n\n## Evaluation\n\n1. **Validates R1**: pytest -k search asserts p95 < 200ms.' +BAD_BODY=$'**Title:** Improve search performance\n\n## User Story\n\nbad' +NOANCHOR_BODY=$'## User Story\n\nAs an op, I want logs, so I can query.\n\n## Problem\n\nToday text logs.\n\n## Solution\n\nAdd pino. One module.\n\n## Out of scope\n\n* rotation\n\n## Requirements\n\n1. Add api/src/log.ts.\n\n## Evaluation\n\n1. **Validates R1**: pytest -k log passes.' + +good_create() { jq -nc --arg d "$GOOD_BODY" '{tool_name:"mcp__linear__save_issue",tool_input:{title:"Add an LRU cache to the search endpoint",description:$d,labels:["api","tech-debt"],priority:3}}'; } +bad_create() { jq -nc --arg d "$BAD_BODY" '{tool_name:"mcp__linear__save_issue",tool_input:{title:"Improve search performance",description:$d,labels:["api","tech-debt"],priority:3}}'; } +bad_update() { jq -nc --arg d "$BAD_BODY" '{tool_name:"mcp__linear__save_issue",tool_input:{id:"ISS-1",title:"Improve search performance",description:$d,labels:["api","tech-debt"]}}'; } +missing_anchor() { jq -nc --arg d "$NOANCHOR_BODY" '{tool_name:"mcp__linear__save_issue",tool_input:{title:"Wire structured logging into the api",description:$d,labels:["api","tech-debt"],priority:3}}'; } +governance() { jq -nc --arg d "$BAD_BODY" '{tool_name:"mcp__linear__save_issue",tool_input:{title:"Decide bourse: wire or decommission",description:$d,labels:["human","eval"]}}'; } + +[ "$(bad_create | decide observe)" = "allow" ] && ok || bad "observe allows bad create" +[ "$(bad_create | decide enforce)" = "deny" ] && ok || bad "enforce denies bad create" +[ "$(good_create | decide enforce)" = "allow" ] && ok || bad "enforce allows good create" +[ "$(bad_update | decide enforce)" = "allow" ] && ok || bad "enforce ignores updates" +[ "$(missing_anchor | decide enforce)" = "deny" ] && ok || bad "enforce blocks missing anchor on create" +[ "$(governance | decide enforce)" = "allow" ] && ok || bad "enforce exempts governance labels" + +bad_create | LINEAR_ISSUE_GATE_MODE=observe bash "$HOOK" >/dev/null +grep -q 'would-block(observe)' "$LOG" && ok || bad "observe logs would-block" + +printf '\n%d passed, %d failed\n' "$pass" "$fail" +rm -f "$LOG" +[ "$fail" -eq 0 ] diff --git a/hooks/validate-issue-on-save.sh b/hooks/validate-issue-on-save.sh new file mode 100755 index 0000000..c18df93 --- /dev/null +++ b/hooks/validate-issue-on-save.sh @@ -0,0 +1,172 @@ +#!/usr/bin/env bash +# PreToolUse(mcp__linear__save_issue): validate a Linear issue at the tool +# boundary, before it is created. This is the "weld the boundary" gate: the skill +# runs the same validator as Gate 1, but a headless subagent filing a batch can +# skip Gate 1, so this hook makes the check unskippable by construction (a +# PreToolUse hook fires for tool calls made inside subagents too). +# +# Scope: CREATE calls only (save_issue with no `id`). An UPDATE (id present) is +# left alone, so the existing corpus of pre-Anchor issues is never orphaned and +# routine edits are never blocked. The Anchor requirement is therefore create-only. +# +# Modes (env LINEAR_ISSUE_GATE_MODE, default observe): +# observe : run the validator, LOG a would-block on ERRORs (or, on create, a +# missing/prose Anchor), but ALLOW. Measures the gap without bricking +# filing before the validator is proven clean on the workspace corpus. +# enforce : DENY the create (permissionDecision: deny) with the validator's +# re-fileable output as the reason. Flip to this per workspace only +# after replaying the validator over the existing corpus shows zero +# false-fails. Operator-owned; the agent does not self-flip. +# +# Fail OPEN on anything unexpected (no jq, no python, unparseable payload, missing +# validator): allow. A validation gate must not brick a session on its own bug. +# +# Config (all optional, env-overridable so the script stays portable): +# LINEAR_ISSUE_GATE_MODE observe | enforce (default observe) +# LINEAR_ISSUE_GATE_VALIDATOR path to validate_issue.py (default: sibling skill copy) +# LINEAR_ISSUE_GATE_EXEMPT comma list of governance labels that are NOT +# build-lane and are never gated (default: human,eval) +# LINEAR_ISSUE_GATE_LOG would-block log file (default: $TMPDIR/linear-issue-gate.log) +set -uo pipefail + +allow() { exit 0; } +deny() { + # permissionDecision JSON on stdout, exit 0 (the documented deny path for a + # PreToolUse hook on an MCP tool; exit-2 is unreliable for non-Bash tools). + python3 - "$1" <<'PY' 2>/dev/null || { printf '%s\n' "$1" >&2; exit 2; } +import json, sys +print(json.dumps({"hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "deny", + "permissionDecisionReason": sys.argv[1], +}})) +PY + exit 0 +} + +command -v jq >/dev/null 2>&1 || allow +command -v python3 >/dev/null 2>&1 || allow + +input=$(cat 2>/dev/null) || allow +[ -n "$input" ] || allow + +tool=$(printf '%s' "$input" | jq -r '.tool_name // ""' 2>/dev/null) +case "$tool" in + *save_issue) : ;; # matcher should already scope this; double-check + *) allow ;; +esac + +# CREATE vs UPDATE: an update carries an id/issueId. Only gate creates. +iid=$(printf '%s' "$input" | jq -r '.tool_input.id // .tool_input.issueId // ""' 2>/dev/null) +[ -z "$iid" ] || allow + +MODE="${LINEAR_ISSUE_GATE_MODE:-observe}" +HOOK_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]:-$0}")" && pwd) +VALIDATOR="${LINEAR_ISSUE_GATE_VALIDATOR:-$HOOK_DIR/../skill/scripts/validate_issue.py}" +[ -r "$VALIDATOR" ] || allow +EXEMPT="${LINEAR_ISSUE_GATE_EXEMPT:-human,eval}" +LOG="${LINEAR_ISSUE_GATE_LOG:-${TMPDIR:-/tmp}/linear-issue-gate.log}" + +# Reconstruct a validator-shaped document from the create payload. Linear stores +# title/labels/priority as fields, not body lines, so synthesize the header lines +# the validator expects only when the description does not already carry them. +# The payload is passed as argv (a heredoc would capture python's stdin and shadow +# a piped payload). +doc=$(python3 - "$input" <<'PY' 2>/dev/null +import json, sys +try: + d = json.loads(sys.argv[1]) +except Exception: + sys.exit(0) +ti = d.get("tool_input") or {} +desc = ti.get("description") or ti.get("body") or "" +title = (ti.get("title") or "").strip() + +# Labels may arrive as names, as {name} objects, or as opaque ids. The validator +# only counts them (>=2), so any tokens preserve the shape. +labels = ti.get("labels") or ti.get("labelIds") or ti.get("labelNames") or [] +names = [] +for l in labels if isinstance(labels, list) else []: + if isinstance(l, str): + names.append(l) + elif isinstance(l, dict): + names.append(str(l.get("name") or l.get("id") or "label")) + else: + names.append("label") + +PRI = {0: "No priority", 1: "Urgent", 2: "High", 3: "Medium", 4: "Low"} +pri = ti.get("priority") +pri_name = ti.get("priorityLabel") or (PRI.get(pri) if isinstance(pri, int) else (pri or "")) + +head = [] +if title and "**Title:**" not in desc: + head.append(f"**Title:** {title}") +if names and "**Labels:**" not in desc: + head.append("**Labels:** " + ", ".join(f"`{n}`" for n in names)) +if pri_name and "**Priority:**" not in desc: + head.append(f"**Priority:** {pri_name}") + +# Emit the labels (comma-joined) on the first line for the shell's scope check. +low = ",".join(names).lower() +print("GATELABELS:" + low) +print("\n".join(head) + ("\n\n" if head else "") + desc) +PY +) +[ -n "$doc" ] || allow + +# Peel the label signal line, then the reconstructed document. +labels_line=$(printf '%s' "$doc" | sed -n '1p') +labels_low=${labels_line#GATELABELS:} +body=$(printf '%s' "$doc" | sed '1d') + +# Build-lane scoping: if the issue carries only governance labels (human/eval, +# decision/adjudication), it is out of scope for the build-lane gate. A build +# issue (api/web/build/infra/... or no governance label) is in scope. +in_scope=1 +if [ -n "$labels_low" ]; then + only_governance=1 + IFS=',' read -r -a _labels <<< "$labels_low" + for l in "${_labels[@]}"; do + l=$(printf '%s' "$l" | tr -d '[:space:]') + [ -n "$l" ] || continue + case ",$EXEMPT," in + *",$l,"*) : ;; # a governance label + *) only_governance=0 ;; # a real build/domain label -> in scope + esac + done + [ "$only_governance" = "1" ] && in_scope=0 +fi + +now=$(date -u +%FT%TZ 2>/dev/null || echo "?") +logline() { printf '%s\tmode=%s\t%s\n' "$now" "$MODE" "$1" >> "$LOG" 2>/dev/null || true; } + +[ "$in_scope" = "1" ] || { logline "governance-exempt labels=[$labels_low] ALLOW"; allow; } + +# Run the validator. Exit 1 means blocking ERRORs; capture the report for the reason. +report=$(printf '%s' "$body" | python3 "$VALIDATOR" - 2>/dev/null) +vstatus=$? + +# On a create, a missing or prose-shaped Anchor is promoted from WARN to a block. +anchor_block="" +printf '%s' "$report" | grep -Eq '\[anchor-(missing|shape)\]' && anchor_block=1 + +if [ "$vstatus" = "0" ] && [ -z "$anchor_block" ]; then + logline "clean title=$(printf '%s' "$body" | grep -m1 '\*\*Title:\*\*' | cut -c1-80) ALLOW" + allow +fi + +# Non-conforming create. +errs=$(printf '%s' "$report" | grep -E '^[[:space:]]+ERROR' | sed 's/^[[:space:]]*//' | head -12) +[ -n "$anchor_block" ] && errs=$(printf '%s\n%s' "$errs" " ANCHOR required on create: name a file.ext:line / file.ext:symbol / module.function / METHOD /path -> status / playwright:selector") +reason="linear-issue-craft gate: this create does not pass the house-style validator. Fix these, then re-file: +$errs + +(Run the validator locally: python3 $VALIDATOR <draft>. This gate is CREATE-only and build-lane scoped; governance issues (labels: $EXEMPT) are exempt.)" + +if [ "$MODE" = "enforce" ]; then + logline "BLOCK errs=$(printf '%s' "$errs" | tr '\n' ';' | cut -c1-160)" + deny "$reason" +fi + +logline "would-block(observe) errs=$(printf '%s' "$errs" | tr '\n' ';' | cut -c1-160)" +allow diff --git a/skill/SKILL.md b/skill/SKILL.md index 69376ab..7c4fbc6 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -96,7 +96,7 @@ When this skill is active, every draft you produce must include: How this skill runs depends on who is downstream. Detect the mode from the invocation, not the mood of the request. - **Interactive mode** (a human is reading each draft): you draft one or a few issues, run the gates, and show the human a note plus the draft for approval before filing. All three gates apply, including the independent Gate 3 reviewer per issue. This is the writing-aid path. -- **Headless / pipeline mode** (default when this skill is invoked from inside a subagent, or when filing more than one issue in a batch to hand straight to the execution pipeline): there is no human reading each draft, so the per-issue draft-approval ceremony and the per-issue Gate 3 reviewer are **dropped**. What replaces them is enforcement that cannot be skipped under load: **Gate 1 must pass on every block before every `save_issue` call** (a save-hook enforces this at the tool boundary; see the repo's hook), and the batch as a whole may get one review pass, not one reviewer per issue. The failure this fixes is real: the headless path is where per-issue ceremony silently evaporated (one run filed 80 issues with the validator never executed). +- **Headless / pipeline mode** (default when this skill is invoked from inside a subagent, or when filing more than one issue in a batch to hand straight to the execution pipeline): there is no human reading each draft, so the per-issue draft-approval ceremony and the per-issue Gate 3 reviewer are **dropped**. What replaces them is enforcement that cannot be skipped under load: **Gate 1 must pass on every block before every `save_issue` call** (a PreToolUse save-hook enforces this at the tool boundary, and it fires for subagent tool calls too; see `hooks/validate-issue-on-save.sh` and `hooks/README.md`), and the batch as a whole may get one review pass, not one reviewer per issue. The failure this fixes is real: the headless path is where per-issue ceremony silently evaporated (one run filed 80 issues with the validator never executed). The rule that never relaxes in either mode: no `save_issue` without a clean Gate 1 on that issue's body. Everything else (the human note, the per-issue independent review) is interactive-mode ceremony that a batch handoff replaces with a boundary gate, not with trust. From 271bce29d9b3967dbc1e74c65666decbd8731c9d Mon Sep 17 00:00:00 2001 From: Peyman <peyman.h.j@gmail.com> Date: Mon, 6 Jul 2026 01:50:55 +0200 Subject: [PATCH 3/4] fix(skill): address CodeRabbit review on PR #5 - refactor-story + brief-to-epic filing skeletons: restore the **Epic:** backlink line the validator reads (Major: authors had no slot for it in the skeleton) - README: Task row lists Ops; overview names the Task (Problem/Verification) path alongside Story, not just Story - save-hook: lowercase LINEAR_ISSUE_GATE_EXEMPT before matching so a mixed-case override still matches the (lowercased) label names - test_hook.sh: replace the SC2015 `A && ok || bad` pattern with explicit if/then/else via an assert helper --- README.md | 4 ++-- hooks/test_hook.sh | 18 +++++++++++------- hooks/validate-issue-on-save.sh | 4 +++- skill/prompts/brief-to-epic.md | 3 ++- skill/prompts/refactor-story.md | 1 + 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 16e7190..21f3592 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ A portable Claude Code skill that captures a tight, repeatable Linear issue writing style, designed to be applied to any Linear workspace. -The issue is treated as a compiler input, not an essay: an autonomous execution pipeline reads it (a planner, a held-out probe that writes a frozen test from the title plus Requirements plus Evaluation, an implementer, a reviewer, a merge gate), so concreteness is the product. Every pattern in this repo is grounded in real shipped issues, not invented. The canonical exemplars are quoted verbatim (with workspace identifiers anonymized) in `skill/references/06-canonical-examples.md`. +The issue is treated as a compiler input, not an essay: an autonomous execution pipeline reads it (a planner, a held-out probe that writes a frozen test from a Story's title plus Requirements and Evaluation, or a Task's Problem and Verification, an implementer, a reviewer, a merge gate), so concreteness is the product. Every pattern in this repo is grounded in real shipped issues, not invented. The canonical exemplars are quoted verbatim (with workspace identifiers anonymized) in `skill/references/06-canonical-examples.md`. ## What's in the box @@ -33,7 +33,7 @@ skill/ | **Epic** | `Epic: <Domain>` | Goal · Outcomes · Out of scope · Stories under this epic | | **Story** | imperative verb + concrete deliverable, or `<Role> — <Capability>` | (epic backlink) · Anchor · User Story · Problem · Solution · Out of scope · Requirements · Evaluation | | **Hardening** | `Harden <file.ts>: <symptom1>, <symptom2>, <symptom3>` | Summary · Issues (numbered, each with **Fix:** + **Verify:**) | -| **Task** | `Bug:` / `Chore:` / `Spike:` (or set `**Type:**`) | Anchor · Problem · Verification | +| **Task** | `Bug:` / `Chore:` / `Spike:` / `Ops:` (or set `**Type:**`) | Anchor · Problem · Verification | Every **Story** and **Task** carries an **Anchor**: one machine-resolvable target (`file.ext:line`, `file.ext:symbol`, `module.function`, `METHOD /path -> status`, or `playwright:selector`) that the held-out probe binds to. diff --git a/hooks/test_hook.sh b/hooks/test_hook.sh index 05896b9..32dd667 100755 --- a/hooks/test_hook.sh +++ b/hooks/test_hook.sh @@ -31,15 +31,19 @@ bad_update() { jq -nc --arg d "$BAD_BODY" '{tool_name:"mcp__linear__sav missing_anchor() { jq -nc --arg d "$NOANCHOR_BODY" '{tool_name:"mcp__linear__save_issue",tool_input:{title:"Wire structured logging into the api",description:$d,labels:["api","tech-debt"],priority:3}}'; } governance() { jq -nc --arg d "$BAD_BODY" '{tool_name:"mcp__linear__save_issue",tool_input:{title:"Decide bourse: wire or decommission",description:$d,labels:["human","eval"]}}'; } -[ "$(bad_create | decide observe)" = "allow" ] && ok || bad "observe allows bad create" -[ "$(bad_create | decide enforce)" = "deny" ] && ok || bad "enforce denies bad create" -[ "$(good_create | decide enforce)" = "allow" ] && ok || bad "enforce allows good create" -[ "$(bad_update | decide enforce)" = "allow" ] && ok || bad "enforce ignores updates" -[ "$(missing_anchor | decide enforce)" = "deny" ] && ok || bad "enforce blocks missing anchor on create" -[ "$(governance | decide enforce)" = "allow" ] && ok || bad "enforce exempts governance labels" +assert() { # $1 = expected, $2 = actual, $3 = name + if [ "$2" = "$1" ]; then ok; else bad "$3 (expected $1, got $2)"; fi +} + +assert allow "$(bad_create | decide observe)" "observe allows bad create" +assert deny "$(bad_create | decide enforce)" "enforce denies bad create" +assert allow "$(good_create | decide enforce)" "enforce allows good create" +assert allow "$(bad_update | decide enforce)" "enforce ignores updates" +assert deny "$(missing_anchor | decide enforce)" "enforce blocks missing anchor on create" +assert allow "$(governance | decide enforce)" "enforce exempts governance labels" bad_create | LINEAR_ISSUE_GATE_MODE=observe bash "$HOOK" >/dev/null -grep -q 'would-block(observe)' "$LOG" && ok || bad "observe logs would-block" +if grep -q 'would-block(observe)' "$LOG"; then ok; else bad "observe logs would-block"; fi printf '\n%d passed, %d failed\n' "$pass" "$fail" rm -f "$LOG" diff --git a/hooks/validate-issue-on-save.sh b/hooks/validate-issue-on-save.sh index c18df93..bbe23e4 100755 --- a/hooks/validate-issue-on-save.sh +++ b/hooks/validate-issue-on-save.sh @@ -64,7 +64,9 @@ MODE="${LINEAR_ISSUE_GATE_MODE:-observe}" HOOK_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]:-$0}")" && pwd) VALIDATOR="${LINEAR_ISSUE_GATE_VALIDATOR:-$HOOK_DIR/../skill/scripts/validate_issue.py}" [ -r "$VALIDATOR" ] || allow -EXEMPT="${LINEAR_ISSUE_GATE_EXEMPT:-human,eval}" +# Lowercased so a mixed-case override (e.g. "Human,Eval") still matches the +# label names, which are compared in lowercase. +EXEMPT=$(printf '%s' "${LINEAR_ISSUE_GATE_EXEMPT:-human,eval}" | tr '[:upper:]' '[:lower:]') LOG="${LINEAR_ISSUE_GATE_LOG:-${TMPDIR:-/tmp}/linear-issue-gate.log}" # Reconstruct a validator-shaped document from the create payload. Linear stores diff --git a/skill/prompts/brief-to-epic.md b/skill/prompts/brief-to-epic.md index 6dd945c..3c483fb 100644 --- a/skill/prompts/brief-to-epic.md +++ b/skill/prompts/brief-to-epic.md @@ -100,8 +100,9 @@ I'm proposing **<N> child stories** under this epic. Archetype is **<epic vs. pe ## Story 1 of N **Title:** <story title> +**Epic:** <parent epic name, the validator-readable backlink in the body> **Anchor:** <file.ext:line | file.ext:symbol | module.function | METHOD /path -> status | playwright:selector> -**Parent:** <Epic name> +**Parent:** <Epic name, the Linear parent field> **Labels:** <domain>, <capability(ies)> **Priority:** <High/Medium/Low> diff --git a/skill/prompts/refactor-story.md b/skill/prompts/refactor-story.md index 9b4f998..d441aee 100644 --- a/skill/prompts/refactor-story.md +++ b/skill/prompts/refactor-story.md @@ -95,6 +95,7 @@ Skeleton: Drafted as a refactor story because the change preserves behavior and touches a single surface. Alternative was a hardening ticket; rejected because the issues here aren't defects in the current code: it just doesn't scale. **Title:** <title> +**Epic:** <parent epic, or `none` for a deliberately standalone refactor> **Anchor:** <the new file/function being introduced> **Labels:** <domain>, tech-debt **Priority:** Low From 87e37bd7f770ef1fa31eec05b0eaee053e615435 Mon Sep 17 00:00:00 2001 From: Peyman <peyman.h.j@gmail.com> Date: Mon, 6 Jul 2026 01:54:29 +0200 Subject: [PATCH 4/4] fix(validator): vague-verb matches action verbs only, not noun forms A corpus replay over 100 real workspace issues surfaced the one validator-defect false-fail: the old improv\w*|enhanc\w*|optimiz\w* matched noun forms, so it fired on terms of art (self-improvement, optimizer, evaluator-optimizer) and blocked three otherwise-clean build-lane issues. Restrict to improve/enhance/optimize verb and gerund forms. The replay confirmed none of the Increment-1 changes introduced any false-fail; this was pre-existing. Regression test added. --- CHANGELOG.md | 1 + skill/scripts/tests/test_validate.py | 36 ++++++++++++++++++++++++++++ skill/scripts/validate_issue.py | 9 ++++++- 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 271decd..86db81a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - **`epic-backlink` demoted to WARN.** A standalone or refactor story with no parent no longer hard-fails; write `**Epic:** none` to mark the omission deliberate. This was the single most common self-inflicted validator failure. - **`labels-verify` WARN deleted.** It fired on every block every run and could never be discharged by the script, pure noise. - **Vague-verb check now requires an adjacent metric.** The metric must sit within a few words of `improve`/`enhance`/`optimize`; a bare integer elsewhere on the line (a version, issue id, or port) no longer launders the verb. +- **Vague-verb matches action verbs only, not their noun forms.** A corpus replay over 100 real issues found the old `improv\w*|optimiz\w*` firing on terms of art (`self-improvement`, `optimizer`, `evaluator-optimizer`), which blocked legitimate creates. The regex now matches only `improve`/`enhance`/`optimize` verb and gerund forms. (This was the one validator-defect false-fail the replay surfaced; none of the Increment-1 changes introduced any.) - **Code-fence and inline-code awareness.** The em-dash, AI-tell, vague-verb, and section scans skip fenced code and inline-code spans, so a quoted `## Description`, `robust-parser.ts`, or a code sample no longer false-trips. - **`---` splitting is fence-aware and merges title-less tails.** A `---` inside a code block, or one that only splits body prose, is content; it can no longer silently drop the tail of an issue (and everything after it) from validation. - **`epic-story-count` counts any bullet** under "Stories under this epic" (verbatim child titles and ordered lists included), and role-prefixed child titles are exempt from the em-dash check, resolving the contradiction with "list child titles verbatim". diff --git a/skill/scripts/tests/test_validate.py b/skill/scripts/tests/test_validate.py index c59597d..d58a37d 100644 --- a/skill/scripts/tests/test_validate.py +++ b/skill/scripts/tests/test_validate.py @@ -223,6 +223,38 @@ 1. **Validates R1**: `pytest -k search_latency` asserts p95 < 200ms. """ +# Noun compounds ("self-improvement", "evaluator-optimizer", "optimizer") are +# terms of art, not vague action verbs, and must NOT fire vague-verb. +NOUN_COMPOUND = """**Epic:** none +**Title:** Wire the self-improvement loop into the evaluator-optimizer +**Anchor:** app/loop.py:run_optimizer +**Labels:** `ai`, `infra` + +## User Story + +As an operator, I want the gated self-improvement loop wired in, so that the optimizer proposes changes safely. + +## Problem + +Today `app/loop.py` runs the evaluator-optimizer pattern but the self-improvement loop is ungated; the optimizer writes with no review. + +## Solution + +Gate the optimizer behind the reviewer. One module. + +## Out of scope + +* Unattended self-improvement; the gate stays. + +## Requirements + +1. Add a review gate to `run_optimizer` in `app/loop.py`. + +## Evaluation + +1. **Validates R1**: `pytest -k optimizer_gate` asserts the optimizer blocks on a failing review. +""" + # A prose Anchor must be flagged (WARN) as not addressable. ANCHOR_PROSE = """**Epic:** Platform **Title:** Wire the rate limiter into the login route @@ -582,6 +614,10 @@ def check(name, cond): check("vague_with_id still flags vague-verb", "vague-verb" in codes(VAGUE_WITH_ID)) check("good_optimize accepts an adjacent metric", "vague-verb" not in codes(GOOD_OPTIMIZE)) + # Noun compounds are terms of art, not vague action verbs. + check("noun_compound does not flag vague-verb", "vague-verb" not in codes(NOUN_COMPOUND)) + check("noun_compound has no errors", not errors(NOUN_COMPOUND)) + # Anchor: prose warns, a good-shaped anchor does not. check("anchor_prose warns anchor-shape", "anchor-shape" in warn_codes(ANCHOR_PROSE)) check("good_story has a clean anchor", "anchor-shape" not in warn_codes(GOOD_STORY)) diff --git a/skill/scripts/validate_issue.py b/skill/scripts/validate_issue.py index a48410d..9892ef0 100644 --- a/skill/scripts/validate_issue.py +++ b/skill/scripts/validate_issue.py @@ -41,7 +41,14 @@ # scan too. STORY_BULLET = re.compile(r"^\s*([-*]|\d+\.)\s+\S") -VAGUE_VERBS = re.compile(r"\b(improv\w*|enhanc\w*|optimiz\w*)\b", re.IGNORECASE) +# Vague ACTION verbs only, not their noun forms. "self-improvement", "optimizer", +# and "evaluator-optimizer" are terms of art, not vague actions, and must not fire +# (the old `improv\w*|optimiz\w*` matched the nouns and blocked real issues). +VAGUE_VERBS = re.compile( + r"\b(improve|improves|improving|enhance|enhances|enhancing" + r"|optimi[sz]e|optimi[sz]es|optimi[sz]ing)\b", + re.IGNORECASE, +) # A real target metric: a percentage, a pNN latency marker, a comparison bound, # or a number bound to a unit. A bare integer (a version, issue id, port, or # line number) is NOT a metric, so "Improve auth (blocks PROJ-19)" no longer