diff --git a/.agents/skills/write-implementation-brief/SKILL.md b/.agents/skills/write-implementation-brief/SKILL.md new file mode 100644 index 00000000000..19bb22036b4 --- /dev/null +++ b/.agents/skills/write-implementation-brief/SKILL.md @@ -0,0 +1,55 @@ +--- +name: write-implementation-brief +description: > + Write the Implementation Brief and Test Coverage sections of a Site Kit issue from its + acceptance criteria. Use when the user asks to write, draft, fill in, or groom an + implementation brief and/or test coverage for an issue (e.g. "write the implementation brief + for #12345", "fill in the brief and test coverage for this issue", "groom issue 12345") in the + google/site-kit-wp repo. +argument-hint: "[issue-number-or-path]" +allowed-tools: Bash, Read, Grep, Glob, Edit, Write +--- + +# Write an implementation brief + +Write the **Implementation Brief** and **Test Coverage** sections for **$ARGUMENTS** — either a +GitHub issue number, or a path the user supplied to a local issue file. If you were given +neither, ask for one before doing anything else; never go looking for the file yourself. Do not +reimplement the procedure here — read and follow the playbook, which is the single source of +truth shared with the other AI tools. + +## Procedure + +1. **Read the playbook** `docs/context/workflow/write-implementation-brief.md` and follow every + step: read the issue, read the design doc and sibling issues, verify every symbol against the + code, load only the relevant `docs/context/{js,php}` convention docs, write the two sections, + and report what you left out. +2. **Read the issue**: `gh issue view $ARGUMENTS --json title,body,labels` for an issue number, or + read the path the user supplied. Stop and ask the user if the acceptance criteria are missing, + ambiguous, or contradictory. +3. **Read the design doc**: use the one the issue links or the path the user gives you; otherwise + ask for it, bundling the question with any ask from step 2. Many issues have none — if there + isn't one, say so and work from the acceptance criteria and the code rather than stalling. +4. **Verify against the code** before naming anything. Open every file the brief will touch and + confirm each class, method, hook, handle and path exists as you describe it. Where the design + doc and the code disagree, the code wins. +5. **Write the two sections** in place. The brief is grouped by path, one instruction per bullet, + naming real symbols/components/strings and pointing at existing exemplars. Test Coverage is + short: one bullet per test file or area, cases nested and phrased as behaviors, Storybook + stories listed there rather than in the brief. +6. **Report separately** the discrepancies, unanticipated consequences, and assumptions that did + not belong in the brief. + +## Important + +- **Instructions only.** No rationale, no trade-off discussion, no restated acceptance criteria, + no background on how the existing system works. +- **Omit what isn't needed.** Never write "no Storybook changes required" or similar — silence + says it. +- **No routine commands.** Lint, build, test and VRT invocations belong to + `docs/context/workflow/implement-issue.md`, not the brief. +- **No links to local design docs** — the brief is read on GitHub where those paths don't + resolve. +- **Only the two sections.** Leave Feature Description, Acceptance criteria, QA Brief and + Changelog entry untouched, and do not edit the GitHub issue or post a comment unless the user + explicitly asks. diff --git a/.agents/skills/write-issue/SKILL.md b/.agents/skills/write-issue/SKILL.md new file mode 100644 index 00000000000..9d3db7bc7b5 --- /dev/null +++ b/.agents/skills/write-issue/SKILL.md @@ -0,0 +1,91 @@ +--- +name: write-issue +description: > + Write the description and Acceptance criteria of a Site Kit issue — a feature request or a bug + report — from a design doc, from a bug report, or from requirements given in the message. Use + when the user asks to create, draft or write a GitHub issue or ticket, to file a bug, to break + an epic's design doc into issues, or to write/add acceptance criteria for an existing issue + (e.g. "create an issue for the read_article event from this design doc", "write a bug report + for the audience tile showing no data", "write the remaining issues from design-doc.md", "add + acceptance criteria to #12345") in the google/site-kit-wp repo. +argument-hint: "[design-doc-path, issue-number, bug report, or requirements]" +allowed-tools: Bash, Read, Grep, Glob, Edit, Write +--- + +# Write an issue + +Write the **description** and the **Acceptance criteria** for **$ARGUMENTS** — a design doc path, +a bug report, requirements stated in the user's message, or an existing issue that needs criteria. +Do not reimplement the procedure here — read and follow the playbook, which is the single source +of truth shared with the other AI tools. + +## Procedure + +1. **Read the playbook** `docs/context/workflow/write-issue.md` and follow every step. +2. **Establish the issue type**, because it decides the template and the sections you write: + - **Feature request** — `.github/ISSUE_TEMPLATE/feature_request.md`: the **Feature + Description** and the **Acceptance criteria**. + - **Bug report** — `.github/ISSUE_TEMPLATE/bug_report.md`: the **Bug Description**, the + **Steps to reproduce** and the **Acceptance criteria**. + + **Ask the user which type they want whenever their message does not make it clear**, and wait + for the answer before writing anything. Say how you read the request when you ask. An existing + issue keeps the type it already has — never convert one into the other. A request to change + behavior that works as designed is a feature request, even when the user calls it a fix. +3. **Establish the source and the mode.** New issue → write the description sections and the + criteria. Existing issue whose description is already written → write **only** the criteria and + leave the description byte-for-byte alone. Ask the user if no source was given, the + requirements are ambiguous, or a design doc covers an epic without saying which issue to write. + Never invent a design-doc path or go hunting for a local issue file. +4. **Read the source material end to end** — the whole design doc (the rationale for one issue's + decisions often sits in another's section), any spec it names as authoritative, the sibling + issues already written for the epic, and, for a bug, the whole report plus whatever evidence it + names: a support thread, a linked issue, the pull request the user says introduced it. +5. **Verify against the code** before naming anything: every class, method, hook, constant, + handle and path, the shape of the base class or registry being extended, and who consumes the + data the issue changes. For a bug, confirm the wrong behavior is really what the code does + today and find the lines that produce it. Where the design doc and the code disagree, the code + wins. +6. **Lay the issue out** by copying the template file for the type — drop its YAML frontmatter, + put the title in its place as a single `# ` heading, and keep everything else as the file has + it, including the moderator notice and the placeholder comment in every section you are not + writing. The two templates word their placeholders differently, so copy them from the file for + the type you are writing rather than from the other one. On a bug report, Screenshots and + Additional Context stay as the template gives them, filled in only with environment facts the + user actually stated. The title names the deliverable, or for a bug the symptom and where it + happens; nothing sits between it and the first description heading. A dependency on another + issue is a clause inside the description, by real GitHub issue number. +7. **Write the description** — prose, and the only place rationale belongs. A **Feature + Description** gives the gap today, what this adds, the halves of the work, and the load-bearing + decisions with their reasons. A **Bug Description** gives the wrong behavior on the real + surface with its values, what should happen instead, and the conditions it needs — plus the + cause in one clause only when you verified it in the code, never the fix. **Steps to reproduce** + are a numbered list starting from a state the tester can reach, one action per step with real + values, ending in what the tester sees and what they should have seen. +8. **Write the Acceptance criteria** — a flat `*` list, one checkable outcome per bullet, nested + only for enumerable cases, tables inlined for lookups. Name real symbols and give real values. + On a bug, write the corrected behavior as the outcome it produces, plus the cases that must + keep working — never "no longer broken". +9. **Report separately** the type you wrote when you had to ask, the decisions you had to make, + code/design-doc discrepancies, the cause you found with its `file:line`, anything in a report + you could not confirm, anything added beyond scope, and any ordering constraint between issues. + +## Important + +- **Ask which type of issue it is** when the request does not make it clear. The wrong template + means the wrong sections and a rewrite. +- **Outcomes only in the criteria.** No rationale, no technique notes, no cause or fix, no + negative parentheticals ("(not an `id`)"), no restating the event name, no "no X is needed", no + "Out of scope" or "Known limitations" lists. Keep negatives only when the negative is the + asserted outcome. +- **The scope boundary lives in the criteria**, because that is what the PR is graded against — as + the outcome at the edge ("the cart and checkout pages render nothing"), never as a list of what + isn't built and never as a clause in the description. +- **No links or paths to local design docs or specs** in any section you write — they don't + resolve on GitHub. Restate the constraint instead. External links are fine. +- **Never reference a sibling by its design-doc position** ("issue 5", "the next issue") — that + numbering is not GitHub's. Use a real issue number, or the deliverable's name. +- **An existing description is untouchable** unless the user explicitly asks — a Feature + Description, or a Bug Description with its Steps to reproduce. +- **Don't publish.** No `gh issue create`, no `gh issue edit`, no comments, unless explicitly + asked. Produce the file and let the user place it. diff --git a/.agents/workflows/write-implementation-brief.md b/.agents/workflows/write-implementation-brief.md new file mode 100644 index 00000000000..f84a0cad478 --- /dev/null +++ b/.agents/workflows/write-implementation-brief.md @@ -0,0 +1,39 @@ +--- +title: Write an implementation brief +description: Write the Implementation Brief and Test Coverage sections of a Site Kit issue from its acceptance criteria, verified against the codebase. +--- + +# Write an implementation brief + +Run with `/write-implementation-brief ` in the Agent chat. + +The full procedure is the shared, tool-agnostic playbook +**`docs/context/workflow/write-implementation-brief.md`** (the single source of truth used by all +of this project's AI tools). Follow it exactly. Summary of the steps: + +1. **Read the issue** — from GitHub when given a number + (`gh issue view --json title,body,labels`), or from the path the user supplies for a + local issue file. Ask for one if you were given neither; never go looking for the file. The + Acceptance criteria are the contract — stop and ask if they are missing, ambiguous, or + contradictory. +2. **Read the source material** — the design doc, plus the sibling issues that bound this issue's + scope and supply the numbers to cross-reference. Use the design doc the issue links or the + user gives you a path to; otherwise ask for it, bundling the question with the Step 1 ask. + Many issues have none — if there isn't one, say so and work from the acceptance criteria and + the code rather than stalling. +3. **Verify against the code** — open every file the brief will touch and confirm each class, + method, hook, handle and path. Where the design doc and the code disagree, the code wins. +4. **Load only** the relevant `docs/context/{js,php}` convention docs (use the map in + `implement-issue.md` Step 3). +5. **Write the Implementation Brief** — grouped by path, one instruction per bullet. Name real + symbols, selectors, components, class names and user-facing strings; point at an existing + exemplar rather than describing one; cross-reference sibling issues inline; fence the scope + in one line. No rationale, no unneeded work, no routine commands, no local design-doc links. +6. **Write the Test Coverage** — short. One bullet per test file or area, cases nested and + phrased as behaviors, negative cases included, Storybook stories listed here (not in the + brief), plus any existing tests the change will break. +7. **Report separately** the discrepancies, unanticipated consequences, and assumptions that did + not belong in the brief. + +**Two sections only** — leave Feature Description, Acceptance criteria, QA Brief and Changelog +entry untouched, and do not edit the GitHub issue or post a comment unless explicitly asked. diff --git a/.agents/workflows/write-issue.md b/.agents/workflows/write-issue.md new file mode 100644 index 00000000000..4fab661aaca --- /dev/null +++ b/.agents/workflows/write-issue.md @@ -0,0 +1,80 @@ +--- +title: Write an issue +description: Write the description and Acceptance criteria of a Site Kit issue — a feature request or a bug report — from a design doc, a bug report or requirements, verified against the codebase. +--- + +# Write an issue + +Run with `/write-issue ` in the Agent +chat. + +The full procedure is the shared, tool-agnostic playbook +**`docs/context/workflow/write-issue.md`** (the single source of truth used by all of this +project's AI tools). Follow it exactly. Summary of the steps: + +1. **Establish the issue type**, which decides the template and the sections you write. A + **feature request** (`.github/ISSUE_TEMPLATE/feature_request.md`) gets the **Feature + Description** and the **Acceptance criteria**; a **bug report** + (`.github/ISSUE_TEMPLATE/bug_report.md`) gets the **Bug Description**, the **Steps to + reproduce** and the **Acceptance criteria**. **Ask the user which type they want whenever their + message does not make it clear**, and wait for the answer before writing anything — say how you + read the request when you ask. An existing issue keeps the type it already has; never convert + one into the other. A request to change behavior that works as designed is a feature request, + even when the user calls it a fix. +2. **Establish the source and the mode** — a design doc, a bug report or requirements in the + user's message, or an existing issue (`gh issue view --json title,body`, or a path the + user supplies). A **new** issue gets the description sections and the criteria; an **existing** + issue whose description is already written gets **only** the Acceptance criteria, with the + description left byte-for-byte alone. Ask when nothing was given, the requirements are + ambiguous, or a design doc covers an epic without saying which issue to write. A design doc's + work-estimate table *is* the breakdown — one issue per row; do not re-slice it. +3. **Read the source material end to end** — the whole design doc, not just this issue's section; + any spec the design doc names as authoritative; the sibling issues already written for the + epic, which show where this issue's scope stops; and, for a bug, the whole report as the user + gave it plus whatever evidence it names — a support thread, a linked issue, the pull request + the user says introduced the problem. +4. **Verify against the code** — every class, method, hook, constant, handle and path; the shape + of the base class or registry being extended; and who consumes the data the issue changes, + which is what makes a "must stay unchanged" criterion real. For a bug, confirm that the wrong + behavior is really what the code does today, find the lines that produce it, and check which + cases served by the same code still work. Where the design doc and the code disagree, the code + wins. +5. **Lay the issue out** by copying the template file for the type. Drop its YAML frontmatter and + put the title in its place as a single `# ` heading; everything else stays as the file has it — + moderator notice intact, the template's own placeholder comments left in the sections you are + not writing (the two templates word them differently, so copy from the right file), and on a + bug report Screenshots and + Additional Context left as the template gives them, filled in only with environment facts the + user actually stated. The title names the deliverable, or for a bug the symptom and where it + happens, and nothing sits between it and the first description heading — no epic name, no point + estimate, no dependency line. Where the issue depends on another, that dependency is a clause + inside the description, by real GitHub issue number. +6. **Write the description** — prose only, and the only place rationale belongs. A **Feature + Description**: the gap today and where it falls short, what this issue adds, the halves of the + work as bold run-in headings, and the load-bearing decisions with their reasons. A **Bug + Description**: the wrong behavior on the real surface with its values, what should happen + instead, and the conditions it needs — plus the cause in one clause, only when you verified it + in the code, and never the fix. **Steps to reproduce**: a numbered list that starts from a + state the tester can reach, one action per step with the real values to click or paste, ending + in what the tester sees and what they should have seen. +7. **Write the Acceptance criteria** — a flat `*` list, one checkable outcome per bullet, nested + one level for enumerable cases, tables inlined for lookups. Name real symbols, give real + values, state cardinality, and say what must stay unchanged. On a bug, write the corrected + behavior as the outcome it produces ("the tile shows the audience's 412 users"), never "no + longer broken", and add the cases that must keep working. The criteria are also where the + **scope boundary** lives, since they are what the PR is graded against — as the outcome at the + edge ("the cart and checkout pages render nothing"), never as an "Out of scope" list and never + as a clause in the description. Cut rationale, technique notes, the cause and the fix, negative + parentheticals, restated event names, and work that isn't being done. +8. **Report separately** the type you wrote when you had to ask, the decisions you had to make, + discrepancies against the code with `file:line`, the cause you found for a bug, anything in the + report you could not confirm, anything added beyond the design doc's scope, and any ordering + constraint between the issues. + +**No local design-doc paths or links, and no sibling references by design-doc position** ("issue +5", "the next issue") — that numbering is not GitHub's. Use a real issue number or the +deliverable's name. + +**The authoring sections only** — leave Implementation Brief, Test Coverage, QA Brief and +Changelog entry as placeholder comments, and do not create or edit a GitHub issue unless +explicitly asked. diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000000..6dde0c4a214 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,7 @@ +{ + "enabledPlugins": { + "figma@claude-plugins-official": true, + "playwright@claude-plugins-official": true, + "chrome-devtools-mcp@claude-plugins-official": true + } +} diff --git a/.claude/skills/write-implementation-brief/SKILL.md b/.claude/skills/write-implementation-brief/SKILL.md new file mode 120000 index 00000000000..5d895892a97 --- /dev/null +++ b/.claude/skills/write-implementation-brief/SKILL.md @@ -0,0 +1 @@ +../../../.agents/skills/write-implementation-brief/SKILL.md \ No newline at end of file diff --git a/.claude/skills/write-issue/SKILL.md b/.claude/skills/write-issue/SKILL.md new file mode 120000 index 00000000000..fe689a6f7e6 --- /dev/null +++ b/.claude/skills/write-issue/SKILL.md @@ -0,0 +1 @@ +../../../.agents/skills/write-issue/SKILL.md \ No newline at end of file diff --git a/.gemini/agents/code-reviewer.md b/.gemini/agents/code-reviewer.md deleted file mode 100644 index 6f33179c221..00000000000 --- a/.gemini/agents/code-reviewer.md +++ /dev/null @@ -1,67 +0,0 @@ ---- -name: code-reviewer -description: > - Reviews implemented code for quality, convention adherence, and acceptance - criteria compliance. Scores the implementation 0.0-1.0 and reports - specific violations with fix instructions. -tools: - - "read_file" - - "list_directory" - - "grep_search" - - "glob" -temperature: 0.2 -max_turns: 30 -timeout_mins: 10 ---- - -You are a strict code reviewer for the **google/site-kit-wp** project — a WordPress plugin with a PHP backend and React/JS frontend. - -## Your Task - -You will receive acceptance criteria, an implementation brief, and an implementation summary listing all created/modified/deleted files. Your job is to: - -1. Read ALL files listed in the implementation summary -2. Load relevant context docs to verify convention compliance -3. Score the implementation and report violations - -**CRITICAL**: Do NOT trust the implementation summary blindly. Read the actual source files and verify. - -## Review Standard - -Review against the shared checklist **`docs/context/workflow/review-checklist.md`** — the -single source of truth for Site Kit conventions and quality, shared across all of the -project's AI tools. It defines: - -- **Requirements adherence** — acceptance criteria, Implementation Brief, Test Coverage. -- **Convention adherence** — the JS and PHP principle areas, each citing the authoritative - `docs/context/{js,php}` doc. Load only the docs the change touches and verify against them. -- **Code quality** — structure, error handling, security, performance, docs, accessibility. -- **Verification** — lint/tests/build actually run and passing. -- **Scoring rubric** — 0.0–1.0, with **0.85** as the approval threshold. - -For each violation, report the principle, the context file + section that defines it, how the -code violates it, the fix required, and the affected files (as the checklist specifies). - -## Output Format - -Return this EXACT format: - -``` -CODE REVIEW RESULTS -=================== -Score: [0.0-1.0] | Status: [approved|needs_improvement] | Iteration: [n] - -REQUIREMENTS VIOLATIONS: [count] -[Violation #n: AC Point | Details | Fix Required] - -CONTEXT VIOLATIONS: [count] -[Violation #n: Principle | Context File+Section | Details | Fix Required | Affected Files] - -QUALITY RECOMMENDATIONS: [count] -[Rec #n: Category | Priority (critical/high/medium/low) | Issue | Suggestion | Affected Files] - -STRENGTHS: [positive aspects] -FILES REVIEWED: [count and list] -``` - -**IMPORTANT**: You have READ-ONLY access. Do not attempt to modify any files. diff --git a/.gemini/agents/documenter.md b/.gemini/agents/documenter.md deleted file mode 100644 index e3dd86fadb8..00000000000 --- a/.gemini/agents/documenter.md +++ /dev/null @@ -1,62 +0,0 @@ ---- -name: documenter -description: > - Creates architectural documentation in docs/concepts/ when new - patterns are introduced that are not covered by existing context docs. -tools: - - "read_file" - - "write_file" - - "list_directory" - - "grep_search" - - "glob" -temperature: 0.4 -max_turns: 15 -timeout_mins: 5 ---- - -You are a technical writer for the **google/site-kit-wp** project. - -## Your Task - -You will receive an implementation summary describing files that were created or modified. Your job is to determine if new architectural documentation is needed and create it if so. - -## Decision Criteria - -**Create documentation ONLY if** the implementation introduced NEW architectural patterns not already covered by existing docs in `docs/context/`. - -**Skip documentation if** only existing patterns were used. In this case, respond with: - -``` -DOCUMENTATION: Not needed — implementation uses only existing patterns. -``` - -## If Documentation Is Needed - -Create a `.md` file in `docs/concepts/` with these sections: - -1. **Overview** — What the pattern/concept is and why it exists -2. **Core Principles** — Key rules and constraints -3. **How It Works** — Technical explanation with architecture details -4. **Usage Examples** — Practical code examples -5. **Best Practices** — Do's and don'ts -6. **Common Pitfalls** — Mistakes to avoid -7. **Related Concepts** — Links to related docs - -Also update existing `docs/concepts/` files if the changes affect them. - -## Writing Guidelines - -- Write for human developers — clear language, not academic -- Focus on WHY and HOW, not just WHAT -- Include practical code examples from the actual implementation -- Keep it concise but complete - -## Output Format - -``` -DOCUMENTATION SUMMARY -===================== -Action: [created|updated|not needed] -Files: [paths if any] -Description: [what was documented and why] -``` diff --git a/.gemini/agents/fixer.md b/.gemini/agents/fixer.md deleted file mode 100644 index d8d08996edd..00000000000 --- a/.gemini/agents/fixer.md +++ /dev/null @@ -1,53 +0,0 @@ ---- -name: fixer -description: > - Fixes specific code violations identified during review. Addresses - all requirements violations, context violations, and critical - recommendations, then re-runs lint and tests. -tools: - - "*" -temperature: 0.3 -max_turns: 40 -timeout_mins: 20 ---- - -You are a developer fixing specific code violations in the **google/site-kit-wp** project — a WordPress plugin with a PHP backend and React/JS frontend. - -## Your Task - -You will receive a list of violations from a code review. Your job is to: - -1. Fix ALL requirements violations (non-negotiable) -2. Fix ALL context violations (non-negotiable) -3. Fix critical and high-priority quality recommendations -4. Re-run lint and tests to verify fixes -5. Return an updated implementation summary - -## Fix Process - -1. Read each violation carefully — note the affected files, the principle violated, and the - fix required. Consult the relevant `docs/context/{js,php}` doc (and - `docs/context/workflow/review-checklist.md`) to understand the convention behind it. -2. Read the affected files to understand the current code. -3. Apply **targeted** fixes — do NOT rewrite or refactor beyond what resolves the violation. -4. After all fixes, re-verify (lint, targeted tests, build) using the exact commands in - **`docs/context/workflow/implement-issue.md`** (Step 7). - -## Output Format - -Return this EXACT format: - -``` -FIXES APPLIED -============= -[Fix #n: Violation Reference | What Was Changed | Files Modified] - -UPDATED IMPLEMENTATION SUMMARY -=============================== -Files Created ([n]): [paths + descriptions] -Files Modified ([n]): [paths + descriptions] -Files Deleted ([n]): [paths + reasons] -Verification: Linting [pass/fail] | JS Tests [pass/fail] | PHP Tests [pass/fail] -``` - -**IMPORTANT**: DO NOT create commits, pull requests, or push to remote. All changes remain local. diff --git a/.gemini/agents/implementer.md b/.gemini/agents/implementer.md deleted file mode 100644 index 44a1981aa84..00000000000 --- a/.gemini/agents/implementer.md +++ /dev/null @@ -1,48 +0,0 @@ ---- -name: implementer -description: > - Implements a GitHub issue by loading relevant context documentation, - writing code following project conventions, writing tests, and - running lint/test verification. Returns a structured implementation summary. -tools: - - "*" -temperature: 0.3 -max_turns: 60 -timeout_mins: 30 ---- - -You are a senior developer implementing a GitHub issue for the **google/site-kit-wp** project — a WordPress plugin with a PHP backend and React/JS frontend. - -## Your Task - -You will receive an issue title, acceptance criteria, and implementation brief. Implement it -by following the shared playbook **`docs/context/workflow/implement-issue.md`** — the single -source of truth shared across all of the project's AI tools. The orchestrator has already -fetched and parsed the issue (the playbook's Step 1), so: - -1. Start at **Step 2 (Determine scope)** and continue through Step 7 (Verify). -2. Load **only** the `docs/context/{js,php}` convention docs the issue touches (use the map in - the playbook). The context files you read define MANDATORY conventions — any deviation is a - critical violation. -3. **Branch off `develop`** per the playbook before editing. -4. Implement with co-located tests and Storybook stories; cover ALL acceptance criteria and - Test Coverage items, including edge cases. -5. Self-review against **`docs/context/workflow/review-checklist.md`**, then verify (lint, - targeted tests, build) with the exact commands in the playbook's Step 7. - -## Output Format - -Return this EXACT format: - -``` -IMPLEMENTATION SUMMARY -====================== -Files Created ([n]): [paths + descriptions] -Files Modified ([n]): [paths + descriptions] -Files Deleted ([n]): [paths + reasons] -Key Features: [bullet list] -Tests: [files and edge cases covered] -Verification: Linting [pass/fail] | JS Tests [pass/fail] | PHP Tests [pass/fail] -``` - -**IMPORTANT**: DO NOT create commits, pull requests, or push to remote. All changes remain local. diff --git a/.gemini/commands/implement.toml b/.gemini/commands/implement.toml deleted file mode 100644 index 353a21babe6..00000000000 --- a/.gemini/commands/implement.toml +++ /dev/null @@ -1,137 +0,0 @@ -description = "Implements a GitHub issue locally with automated review and iteration using subagents" - -prompt = """ -You are orchestrating the implementation of GitHub issue #{{args}} from the google/site-kit-wp repository. You coordinate specialized subagents through a quality workflow. - -The canonical, tool-agnostic procedure is `docs/context/workflow/implement-issue.md` and the shared review rubric is `docs/context/workflow/review-checklist.md`. The subagents follow them; the phases below define how you orchestrate those subagents and the quality-gate loop. - ---- CRITICAL INSTRUCTIONS --- - -- This is a LOCAL implementation — DO NOT create commits, pull requests, or push to remote -- You are the orchestrator — delegate work to subagents, do not implement code yourself -- You control the iteration loop and data flow between agents - ---- PHASE 1: FETCH GITHUB ISSUE --- - -Fetch issue #{{args}} from `google/site-kit-wp` using GitHub MCP tools. -Fallback: `!{gh issue view {{args}} --json title,body}` - -Extract: -1. **Issue title** -2. **Acceptance criteria** (between `## Acceptance criteria` and `## Implementation Brief`) -3. **Implementation brief** (between `## Implementation Brief` and `## QA Brief`) - -**STOP and report** if: issue not found, API unavailable, body empty, or section markers missing. - ---- PHASE 2+3: IMPLEMENT --- - -Delegate to @implementer with the following message: - -"Implement GitHub issue #{{args}}: [issue title]. - -ACCEPTANCE CRITERIA: -[paste extracted acceptance criteria] - -IMPLEMENTATION BRIEF: -[paste extracted implementation brief]" - -Save the implementation summary returned by the agent. - ---- PHASE 4: CODE REVIEW --- - -Delegate to @code-reviewer with the following message: - -"Review the implementation of GitHub issue #{{args}}: [issue title]. -This is iteration [n] of the review. - -ACCEPTANCE CRITERIA: -[paste extracted acceptance criteria] - -IMPLEMENTATION BRIEF: -[paste extracted implementation brief] - -IMPLEMENTATION SUMMARY: -[paste summary from implementer or fixer]" - -Extract the **score** and **violations** from the review results. - ---- PHASE 5: ITERATION LOOP --- - -IF score >= 0.85 → skip to Phase 6. - -OTHERWISE, iterate (max 3 total review cycles): - -1. Delegate to @fixer: - - "Fix the following violations from review iteration [n] of issue #{{args}}. - - REVIEW RESULTS: - [paste the full CODE REVIEW RESULTS from the reviewer]" - -2. Save the updated implementation summary from fixer. - -3. Delegate to @code-reviewer again with the updated summary (same format as Phase 4, incrementing the iteration number). - -4. Extract the new score. - - Score >= 0.85 → proceed to Phase 6 - - Score < 0.85 and iterations < 3 → repeat from step 1 - - Score < 0.85 and iterations = 3 → graceful exit (below) - -**Graceful Exit** (score < 0.85 after 3 iterations): - -``` -IMPLEMENTATION INCOMPLETE - MANUAL INTERVENTION REQUIRED -========================================================= -Final Score: [score] | Iterations: 3 | Status: Needs manual review - -Files Created ([n]): [paths] -Files Modified ([n]): [paths] -Files Deleted ([n]): [paths] - -REMAINING ISSUES: -Context Violations ([n]): [list each with fix instructions] -Quality Issues ([n]): [critical and high priority list] - -TOP FIXES NEEDED: -1. [Fix description] — Files: [...] — Action: [...] — Reference: [context doc+section] -2. [Fix description] — Files: [...] — Action: [...] -[up to 5 items] - -WHAT WAS COMPLETED: [aspects that meet quality standards] - -NEXT STEPS: Review changes → fix violations → run tests (npm run test:js / composer run test) → lint → manual review -``` - -Stop after providing this summary. - ---- PHASE 6: DOCUMENTATION (score >= 0.85 only) --- - -Delegate to @documenter: - -"Review the implementation of issue #{{args}} and determine if new architectural documentation is needed. - -IMPLEMENTATION SUMMARY: -[paste final implementation summary]" - ---- FINAL SUMMARY --- - -If score >= 0.85: - -``` -IMPLEMENTATION COMPLETE ✓ -========================= -Issue #{{args}}: [title] | Score: [score] | Iterations: [n] - -Files Created ([n]): [paths + descriptions] -Files Modified ([n]): [paths + descriptions] -Files Deleted ([n]): [paths + reasons] - -Highlights: [key features and notable technical decisions] -Quality: All principles followed | Tests executed and passed | Linting passed -Documentation: [created/not needed] - -Next steps: Review changes → npm run test:js / composer run test → npm run lint → manual test → git commit → open PR -``` - -Begin with Phase 1: Fetch GitHub Issue #{{args}} -""" diff --git a/.gemini/commands/review-pr.toml b/.gemini/commands/review-pr.toml deleted file mode 100644 index d0ad7e5b486..00000000000 --- a/.gemini/commands/review-pr.toml +++ /dev/null @@ -1,106 +0,0 @@ -description = "Reviews a GitHub pull request against its linked issue, the project conventions, and the quality rubric" - -prompt = """ -You are reviewing GitHub pull request #{{args}} from the google/site-kit-wp repository. You orchestrate the read-only @code-reviewer subagent and synthesize the final review. - -The canonical, tool-agnostic procedure is `docs/context/workflow/review-pr.md` and the shared review rubric is `docs/context/workflow/review-checklist.md`. Follow them; the phases below define how you orchestrate the review. - ---- CRITICAL INSTRUCTIONS --- - -- This is a READ-ONLY review — DO NOT post comments, approve, request changes, or change the PR state on GitHub -- DO NOT create commits or push to remote -- The review is fundamentally a check of the diff AGAINST the linked issue, not just against conventions - ---- PHASE 1: FETCH THE PR --- - -Fetch PR #{{args}} from `google/site-kit-wp`: -- `!{gh pr view {{args}} --json number,title,body,author,baseRefName,headRefName,files,additions,deletions,commits}` -- `!{gh pr diff {{args}}}` - -To give the reviewer full-file context, check out the branch locally: `!{gh pr checkout {{args}}}`. - -**STOP and report** if: the PR is not found, the API is unavailable, or the diff is empty. - ---- PHASE 2: READ THE LINKED ISSUE --- - -Site Kit PRs use `.github/PULL_REQUEST_TEMPLATE.md`, whose **Summary** section links the issue under "Addresses issue:": - -``` -## Summary -Addresses issue: - -- # -``` - -Extract the first `#` from the PR body and fetch the issue using the GitHub MCP tools (`issue_read`). -Fallback: `!{gh issue view --json title,body,labels}`. - -Extract: -1. **Acceptance criteria** (between `## Acceptance criteria` and `## Implementation Brief`) -2. **Implementation Brief** (between `## Implementation Brief` and `### Test Coverage`) -3. **Test Coverage** (between `### Test Coverage` and `## QA Brief`) - -This is the spec the PR must satisfy. If the PR body has **no** linked issue (the `- #` line is empty), note it, treat requirements adherence as "not verifiable", and review conventions + code quality only — do not invent acceptance criteria. - ---- PHASE 3: DELEGATE THE REVIEW --- - -Delegate to @code-reviewer with the following message: - -"Review pull request #{{args}}: [PR title]. - -This is a PR review. Read the changed files (listed below, now checked out locally) and verify them against the acceptance criteria, Implementation Brief, and project conventions. - -ACCEPTANCE CRITERIA: -[paste extracted acceptance criteria, or 'No linked issue — review conventions and code quality only'] - -IMPLEMENTATION BRIEF: -[paste extracted Implementation Brief] - -CHANGED FILES: -[paste the `files` list from Phase 1] - -DIFF: -[paste the `gh pr diff` output]" - -Extract the **score**, **requirements violations**, **context violations**, and **quality recommendations** from the review results. - ---- PHASE 4: PRODUCE THE REVIEW --- - -Synthesize the reviewer's findings into the structured review defined by `docs/context/workflow/review-pr.md`. Omit any section with nothing to report. - -``` -### PR #{{args}} — [title] - -**Author**: [author] | **Branch**: `[head]` → `[base]` | **Size**: +[additions] / -[deletions] - -#### Summary -[One paragraph: what the PR does, what areas it touches, and the linked issue (#) or that none is linked.] - -#### Requirements Adherence -[Only when an issue is linked. For each Acceptance criterion and Implementation Brief checkbox: ✅ met / ⚠️ partial / ❌ missing, with the file:line that satisfies it or what's absent. Note any Test Coverage item without a corresponding test.] - -#### Principles Compliance -[Per area touched: ✅ / ⚠️ / ❌ with file:line and the context doc + section, from the reviewer's context violations.] - -#### Code Quality Issues -[Bugs, logic errors, edge cases with file:line.] - -#### Security Concerns -[XSS, capability checks, nonce verification, direct SQL, sanitization.] - -#### Performance -[Re-renders, memoization, expensive selectors, N+1 queries, large assets.] - -#### Test Coverage -[Changed logic lacking tests, or tests not updated for new behavior.] - -#### Minor / Nits -[Optional style/naming/docs improvements.] - -#### Verdict -**APPROVE** / **REQUEST CHANGES** / **NEEDS DISCUSSION** -[Brief justification. If requesting changes, list blocking issues as a numbered checklist.] -``` - -Begin with Phase 1: Fetch PR #{{args}}. -""" diff --git a/.gemini/styleguide.md b/.gemini/styleguide.md deleted file mode 100644 index 034e90ae68e..00000000000 --- a/.gemini/styleguide.md +++ /dev/null @@ -1,85 +0,0 @@ -# Site Kit by Google — Code Review Style Guide - -Site Kit is a mature, heavily reviewed WordPress plugin with strong CI (ESLint, Prettier, PHPCS, PHPStan, Jest, PHPUnit, and visual regression). -Formatting and style are already enforced by tooling — do not duplicate them. - -## Defer to the project's context documentation - -Before flagging anything related to conventions, naming, documentation, state management, data loading, or architecture, consult the project's context docs and defer to them. -They describe how this codebase is intentionally built: - -JavaScript (`docs/context/js/`): - -- `state-management.md` — Redux-style data stores, selectors, resolvers, and the data-loading lifecycle. -- `component-conventions.md` — React component structure and conventions. -- `hooks.md` — custom hooks and usage patterns. -- `jsdoc.md` — required JSDoc format and `@since` tagging. -- `module-architecture.md` — module registration and structure. -- `widgets.md` — the widget API and registration. -- `utils.md` — shared utilities. -- `feature-flags.md`, `feature-tours.md` — feature gating and tours. -- `notifications.md`, `event-tracking.md` — notifications and analytics events. -- `tests.md`, `storybook.md` — JS test and Storybook conventions. - -PHP (`docs/context/php/`): - -- `naming-conventions.md` — class, method, hook, and file naming. -- `dependency-injection.md`, `context-pattern.md` — DI and the Context object. -- `module-architecture.md`, `trait-composition.md` — module and trait structure. -- `storage-patterns.md`, `settings-management.md` — options and settings storage. -- `rest-api.md` — REST route registration and conventions. -- `asset-management.md`, `admin-features.md`, `prompts-and-dismissals.md` — assets, admin, and prompts. -- `phpunit.md` — PHP test conventions. - -If a suggestion would contradict one of these docs, do not make it. - -## Avoid speculative runtime guards - -Do not suggest adding defensive null/undefined/falsy guards when the value is already safe by construction. -In particular, do **not** flag a value as possibly-undefined when: - -- it is typed as non-nullable in TypeScript, or -- it has a destructuring default (e.g. `{ foo = '' }`), or -- it is produced by a data-store selector/resolver or data loader that already guarantees resolution before use (see `state-management.md`). - -Trust the type system and the data-loading lifecycle. -Treating type-guaranteed or loader-guaranteed values as untrusted at runtime produces unreachable, rejected suggestions. - -## Review only the actual change - -Review the diff and its real call sites. -Do not invent requirements, hypothesize feature variants, or request plumbing for use cases that are not present in the changeset. -If a concern depends on an assumption about how the code is called, verify it against the actual call sites before raising it — and assign severity based on demonstrated impact, not on a speculative scenario. - -## Consolidate repeated findings — but account for every instance - -When the same issue appears in multiple locations, post a single consolidated comment instead of repeating it on each occurrence. -In that comment, explicitly list **every** affected location as a `path:line` reference (or permalink) so each instance is individually accounted for and actionable. -Never silently aggregate — a reader must be able to find and fix all occurrences from the one comment. - -## User-facing copy comes from design - -Treat user-facing strings and i18n copy as authored against design (Figma) and product decisions. -Do not suggest wording, tone, or capitalization changes to display strings. - -## Respect the minimum supported versions - -Site Kit supports **WordPress 5.2+** and **PHP 7.4+** (see `readme.txt` and the `google-site-kit.php` header). -Code must run on those minimums, so flag usage of anything introduced in a later version unless it is properly feature-detected, version-gated, or polyfilled. - -Watch in particular for: - -- PHP language features newer than 7.4 — e.g. enums, `match`, named arguments, constructor property promotion, nullsafe `?->`, or 8.0+ standard-library functions (`str_contains`, `str_starts_with`, etc.). -- WordPress functions, hooks, classes, or arguments introduced after 5.2 used without a `function_exists()`/`method_exists()` guard, a version check, or a bundled polyfill. -- JavaScript or browser APIs that fall outside the project's supported targets without the appropriate polyfill or transpilation. - -Automated tooling (e.g. PHPCS compatibility checks) covers some of this but does not catch everything, so call out version-range risks even when CI is green — especially indirect cases like a newer function reached through a wrapper or passed as a callback. - -## What to prioritize - -Focus review effort where it has historically been most valuable on this project: - -- **Framework-API correctness**, especially Gutenberg Block API details (e.g. props no longer passed to `Edit` in API v2/v3, `supports` flags that silently drop attributes). -- **Cross-file duplication / DRY** — identical logic that should be extracted into a shared helper, trait, or utility. -- **Concrete PHP and CSS correctness** — e.g. `??` vs `?:` to avoid undefined-key warnings, direct array access on options that may be missing, redundant or conflicting style declarations. -- **Security** — capability checks, nonce verification, input sanitization, and output escaping, consistent with WordPress and project conventions. diff --git a/.gitignore b/.gitignore index 28d104bca12..88c8873b7f4 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ !.gemini/commands/** !.claude/ .claude/* +!.claude/settings.json !.claude/skills/ !.claude/skills/** !.claude/commands/ diff --git a/AGENTS.md b/AGENTS.md index 732d9c5bdcc..129991e2c78 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,6 +3,37 @@ ## Project Overview WordPress plugin providing Google services integration. PHP backend (`includes/`) + React frontend (`assets/js/`) with modular architecture for each Google service (Analytics, AdSense, Search Console, etc.). +## Writing an issue +When asked to create, draft, or write a GitHub issue from a design doc, from a bug report or from +requirements in the message, to break an epic's design doc into issues, or to write **Acceptance +criteria** for an existing issue, follow the shared, tool-agnostic playbook +**`docs/context/workflow/write-issue.md`** (the single source of truth). Establish the type first: +a **feature request** (`.github/ISSUE_TEMPLATE/feature_request.md`) gets a **Feature Description** +and **Acceptance criteria**; a **bug report** (`.github/ISSUE_TEMPLATE/bug_report.md`) gets a +**Bug Description**, **Steps to reproduce** and **Acceptance criteria** — **ask the user which one +when the request doesn't make it clear**. Verify every class, hook, handle and path against the +code before naming it, and for a bug confirm the wrong behavior is really what the code does +today. Rationale belongs in the description; the **Acceptance criteria** are checkable outcomes +only — no rationale, no technique notes, no cause or fix, no negative parentheticals, no work that +isn't being done. The **scope boundary** is itself a criterion, written as the outcome at the edge +("the cart and checkout pages render nothing"), never an "Out of scope" list and never a clause in +the description. Never link local design docs or refer to a sibling by its design-doc position +("issue 5") — that numbering isn't GitHub's. Touch **only** the type's authoring sections, leave +an existing description alone, and do not create or edit a GitHub issue unless explicitly asked. +(Claude Code exposes this as the `write-issue` skill; Gemini as `/write-issue`; Antigravity as the +`/write-issue` workflow.) + +## Writing an implementation brief +When asked to write, draft, fill in, or groom the **Implementation Brief** and **Test Coverage** +sections of an issue, follow the shared, tool-agnostic playbook +**`docs/context/workflow/write-implementation-brief.md`** (the single source of truth). Verify +every class, method, hook, handle and path against the code before naming it; where the design +doc and the code disagree, the code wins. Write **instructions only** — no rationale, no work +that isn't needed, no routine commands, no links to local design docs. Touch **only** those two +sections, and do not edit the GitHub issue or post a comment unless explicitly asked. (Claude +Code exposes this as the `write-implementation-brief` skill; Gemini as `/write-brief`; +Antigravity as the `/write-implementation-brief` workflow.) + ## Implementing a GitHub issue When asked to implement, build, or work on a GitHub issue by number, follow the shared, tool-agnostic playbook **`docs/context/workflow/implement-issue.md`** (the single source of diff --git a/docs/context/workflow/implement-issue.md b/docs/context/workflow/implement-issue.md index bc445093ad6..ec11ac49a53 100644 --- a/docs/context/workflow/implement-issue.md +++ b/docs/context/workflow/implement-issue.md @@ -33,6 +33,36 @@ not when the issue is created — expect it to be empty and don't rely on it. markers are missing, or the Implementation Brief is ambiguous/contradictory. Do not guess at requirements. +## Read every issue this one references — before writing any code + +Site Kit issues lean on their siblings: a brief says `(added in #12950)` for the store it +builds on, `(see #13005)` for the extension point it leaves open, "the create variant is +#13022" for the half that isn't yours. Those references carry requirements that the issue +in front of you does not repeat. + +Collect every issue referenced anywhere in the Feature Description, Acceptance criteria, +Implementation Brief and Test Coverage — `#12345`, a full `github.com/google/site-kit-wp/issues/…` +URL, the parent epic, and anything GitHub lists as linked — and read each one with +`gh issue view --json title,body,state,url` **before** you start implementing. From +each, take: + +- **What it delivered**, when it is closed — the classes, selectors, hooks and constants your + brief expects to already be there. Confirm they exist in the branch you are on; a closed + issue whose PR has not been merged into your base leaves you building on nothing. +- **What it will deliver**, when it is still open — the work that is *not* yours. Stop at that + boundary rather than implementing it, and don't duplicate a symbol it is going to add. +- **Its acceptance criteria**, where they constrain yours — a shared payload shape, a naming + scheme, an ordering guarantee. Where a referenced issue and your brief disagree, the code + wins if it is already written; otherwise ask. + +Read the referenced issues themselves, not their references — follow a second hop only when +the first leaves a requirement genuinely unclear. + +**Stop and ask the user** if: a referenced issue can't be fetched; your brief depends on work +that a referenced issue has not landed yet; or the issue refers to a sibling **indirectly** +("the next issue", "issue 5", "the issue that adds the store") rather than by number — ask for +the real number instead of guessing which issue is meant. + ## Step 2 — Determine scope Classify the work and identify the affected module: @@ -145,7 +175,8 @@ and verification results. - **Local only.** Do **not** commit, push, or open a pull request unless the user explicitly asks. The deliverable is a working, verified local change on the feature branch. - **No scope creep.** Implement what the brief specifies; flag anything underspecified rather - than inventing behavior. + than inventing behavior. A referenced issue is context, never a work item — never implement + a sibling's brief because your issue mentions it. - **Commit messages** (only when asked to commit) must satisfy `bin/check-commit-msg.php`: start with a capital letter, begin with a present-tense verb, contain more than one word, and end with a full stop — e.g. `Track learn more about conversion tracking link.` diff --git a/docs/context/workflow/write-implementation-brief.md b/docs/context/workflow/write-implementation-brief.md new file mode 100644 index 00000000000..66d992866a5 --- /dev/null +++ b/docs/context/workflow/write-implementation-brief.md @@ -0,0 +1,320 @@ +# Writing an Implementation Brief — Playbook + +This is the **single source of truth** for writing the **Implementation Brief** and **Test +Coverage** sections of a Site Kit issue. Every AI coding tool (Gemini CLI, Antigravity, Claude +Code) points at this file through a thin per-tool adapter, so the procedure stays identical +everywhere. When you update the process, update it **here** — not in the adapters. + +The brief you write is the spec that `implement-issue.md` executes and `review-pr.md` grades +against. Write it so an engineer who has never read the design doc can implement the issue from +the brief plus the codebase alone. + +--- + +## Step 1 — Read the issue + +The issue comes to you one of two ways, and only the user can tell you which: + +- **An issue number** — fetch it with `gh issue view --json title,body,labels`. +- **A path to a local issue file** — read that path. Do not go looking for it: such a file has no + fixed home, so the user must give you the path. + +If the user gave you **neither** a number nor a path, ask for one before doing anything else. + +Parse whichever you get using the section map in `implement-issue.md` Step 1. + +You are writing exactly two sections: + +- **Implementation Brief** — between `## Implementation Brief` and `### Test Coverage`. +- **Test Coverage** — between `### Test Coverage` and `## QA Brief`. + +The **Acceptance criteria** are the contract, and they describe observable behavior only: what a +tester can see by using the feature. The mechanism that produces that behavior is the brief's to +specify, so expect to add detail the criteria do not carry. Everything in the brief must serve a +criterion, and the brief may not add behavior a user or a tester would notice that no criterion +asks for. + +**Stop and ask the user** if the acceptance criteria are missing, ambiguous, or contradict each +other. A brief written over guessed criteria is worse than no brief. Writing the criteria is a +separate job with its own playbook, `write-issue.md` — offer it rather than filling the gap here. + +Feature Description and Acceptance criteria sit under a "managed by moderators only" notice in +the issue template — do not edit them. QA Brief and Changelog entry are filled in later by other +roles; leave them alone unless asked. + +## Step 2 — Read the source material + +- **The design doc.** It carries the rationale, the rejected alternatives, and the constraints + that an acceptance criterion compresses into a single line, so read it whenever one exists. + Use it when the issue links one or the user gives you a path; otherwise **ask the user for + it** — if you already have to ask for the issue itself (Step 1), ask for both at once. Plenty + of issues have no design doc: when the answer is "there isn't one", say so and write the brief + from the acceptance criteria and the code. Never stall on a design doc, and never invent a + path to one. +- **Sibling issues in the same epic** — they tell you where this issue's scope stops and the + next one begins. Ask the user for their numbers so you can cross-reference them (see + *Referencing another issue*, below). + +## Step 3 — Verify everything against the code + +Do not write a brief from the issue text alone. Open every file the brief will touch and +confirm, by reading it: + +- Every class, method, constant, hook, filter, option and script handle you name exists and is + spelled exactly right. +- The pattern you tell someone to follow (e.g. "mirror `X::register_script()`") actually looks + the way you describe. +- Hook ordering and firing conditions, whenever the brief depends on them — trace the hook to + its `do_action()` / `apply_filters()` call site rather than trusting the design doc. +- Which existing tests, fixtures, snapshots or inline data the change will move. +- **That the build can compile the kind of file you are about to name.** Open the build config + that will include the new file, plus the lint config and `tsconfig.json`. A bundle can define + its own rules instead of using the shared `createRules()`: + `assets/webpack/frontendModules.config.js` defines a `babel-loader` rule that matches `.js` + only, so the first `.ts` file in that bundle needs a new rule, and adding that rule is part of + the brief. Check the globals the new file reads in the same pass: `_googlesitekit` has no + declaration in `assets/js/types/globals.d.ts`, so a TypeScript file that reads it fails + `npm run typecheck` until a declaration is added. + +Where the code contradicts the design doc, the code wins: write the brief against reality and +report the discrepancy (Step 7). + +## Step 4 — Load the relevant convention docs + +Read **only** what the issue touches, using the scope map in `implement-issue.md` Step 3. The +brief should not restate those conventions — it should be written so that following it produces +conforming code. + +## Referencing another issue — always ask for the number + +Every reference to another issue is a real GitHub number (`#12345`) — prerequisites, siblings in +the same epic, the extension point this brief leaves open, the follow-up that lands the rest. +There is no acceptable indirect form: not "issue 5" from a design doc's ordering, not "the next +issue", not "the issue that adds the datastore". + +**Ask the user for the number.** Never infer it from a design doc's ordering, never guess it from +a nearby issue number, and never leave a placeholder to be filled in later: + +- **The issue is already in GitHub** — ask the user for its number, then confirm it is the issue + you mean with `gh issue view --json title` before citing it. +- **The issue only exists locally** — a markdown file that has not been filed yet. Say so, and + ask the user to create it in GitHub and give you the new number. Wait for it: a cross-reference + is the one thing you cannot write around. +- **The user has no number to give** — drop the cross-reference and name the deliverable instead + (``the store introduced with `getAudienceSettings` ``). Report the dropped reference in Step 7. + +Work out every issue you will need to cite before you start writing, and ask for all of the +numbers in one message — together with the issue and design doc you already have to ask for +(Steps 1 and 2) — rather than stopping repeatedly. + +## Step 5 — Write the Implementation Brief + +### Structure + +**Group by path.** Every group is headed by the path it changes; the changes hang off it as +nested bullets. Two shapes are in use — pick one and stay consistent within an issue: + +``` +* [ ] In `path/to/File.php`: + * + * +``` + +``` +In `path/to/directory/` (new directory): + +* [ ] +* [ ] +``` + +A group can be headed by a file, a directory, or a path plus a parenthetical note — +`(new file)`, `(new directory)`, ``(new file, combined into the store in `index.js`)``. Order +the groups the way someone would work through them: shared/base changes, then new classes, then +registration, then build config, then components, with SCSS last. + +Nest a third level when one bullet covers several related behaviors, ending the parent in a +colon: + +``` + * Submit behavior: + * + * + * +``` + +### Content + +**Write in plain, simple words — this matters more than anything else in this section.** The +brief is read by people: the engineer who implements it, the reviewer who grades the PR against +it, and a moderator who has never read the design doc. Many of them do not speak English as a +first language. Every sentence must be clear on the first reading: + +- **Use the simplest word that is still accurate.** Short sentences. One idea per sentence. No + decoration, no "notably", "crucially", "simply", "of course", "elegant", "robust". +- **No idioms, metaphors, or cultural references.** Not "standard furniture", "dead weight", + "burns the flag", "under the hood", "out of the box", "for free", "a natural no-op". Write what + actually happens instead: "the listener finds no matching elements, so it does nothing". +- **Explain a technical term the first time you use it, or do not use it.** "the value lands in + an opaque `pathname`" says nothing to the reader. "`hostname` is empty and the whole recipient + stays in `pathname` as one unsplit string with no leading slash — `tel:+15551234567` gives + `pathname` `+15551234567`" says all of it. +- **Finish every sentence.** "`https://notwa.me/1555` must not classify" leaves out *as what*. + Write "…is not a contact link, so nothing is emitted". +- **Name who does what to what.** "the classifier short-circuits `outbound_link_click`" can be + read in two opposite directions. "when a link is classified as a contact link, the listener + emits `contact_link_click` and returns, so the outbound handler never receives that anchor" + can be read in only one. +- **List the members of any set you name.** If you write "an allowlist", "the excluded hosts", + "the usual guards", write out what is in it, inline. A set the reader has to guess at is the + same as no set at all. + +Read each bullet once more before you finish, and rewrite every sentence that needs a second +reading. This applies to your reply to the user (Step 7) as much as to the issue text. + +**Say what to do, and nothing else.** Every bullet is an instruction. Leave a genuine +implementation choice open rather than inventing a detail, but never leave the implementer to +search for something the codebase has already decided. + +**Name the real thing.** Wherever a symbol already exists, name it exactly: + +- classes, methods, hooks, filters, constants, script handles, datapoint names (`GET:ctas`); +- datastore selectors and actions with their signature + (`getCTAs( { organizationID, publicationID } )`), and the form/store constants they use; +- components to reuse (`SpinnerButton`, `ProgressBar`, `Notice`), their props (`helperText`), + and CSS class names (`mdc-text-field--error`); +- inline-data globals and keys (`wpPrivacyURL` on `_googlesitekitBaseData`); +- user-facing strings, quoted verbatim — headings, descriptions, CTA labels, error text. + +**Spell out every name you invent.** A brief names things that do not exist yet: constants, config +keys, event params, methods, variables, types. Write each of them in full words — no abbreviations, +no truncations, no initials. `readThresholdPct` and `readMinSeconds` each cost the reader a guess: +what does `Pct` stand for, and is `Min` "minutes" or "minimum"? `readTimeThresholdPercent` and +`minimumReadTimeSeconds` cost nothing and can only be read one way. The short forms that stay are +the ones already used across the codebase (`ID`, `URL`, `HTML`) and any existing name, which you +copy exactly as it is. See `docs/context/php/naming-conventions.md`. + +**Point at an exemplar instead of describing one.** "following the pattern in +`includes/Modules/Analytics_4/Datapoints/`", "following `publications.js`", "following how +`Analytics_4` registers `POST:create-audience`". One reference beats a paragraph of description. + +**Cross-reference sibling issues inline** wherever the work meets theirs — `(added in #12950)`, +`(see #13005)`, "the create variant is #13022". Use it for prerequisites the brief depends on +and for extension points it deliberately leaves open. Every number comes from the user, never +from your own inference — see *Referencing another issue*, above. + +**Fence the scope in one line** when an adjacent concern could reasonably be pulled in: "No +express-flow routing/gating changes in this step." "The live API wiring lands in the API +integration work." "Keep shared stepper/layout styles untouched." + +**For UI work**, reference the design (a Figma node link, or "per the design") rather than +restating measurements, and spell out the interaction states: what disables the CTA, the pending +state, what happens on success, what renders on error. Name the SCSS partial to create or extend +and the index file to import it into. + +Do **not** include: + +- **Rationale.** No "because", no "this keeps X out of Y", no trade-off discussion, no + restatement of a decision's benefits. The design doc and the acceptance criteria hold the + *why*; the brief holds the *what*. +- **Work that isn't needed.** No "no Storybook changes required", "no migration needed", "no new + settings". Silence already says that. +- **Routine commands.** No lint, build, test or VRT invocations — `implement-issue.md` Step 6 + owns verification, and every engineer already knows to build. Mention a command only when it + is unusual and specific to this issue. +- **Links to local design docs.** They don't resolve for someone reading the issue on GitHub; + restate the constraint in one clause instead. External links — Figma, a hosted design doc — + are fine. +- **Plain-text line numbers.** No `includes/Core/Modules/Modules.php:212`, no "see line 212", no + `file:line` anywhere in the two sections. The number is wrong as soon as anyone edits the file, + and a reader on GitHub cannot click it. When a bullet has to point at an exact place in the + code, use a GitHub permalink instead — a URL pinned to a commit SHA, with the line or range on + the end: + `https://github.com/google/site-kit-wp/blob//includes/Core/Modules/Modules.php#L212-L218`. + Get one by opening the file on GitHub and pressing `y`, or with + `gh browse --no-browser --commit= ` and adding the `#L-L` yourself. + A URL built on a branch name (`/blob/main/`, `/blob/develop/`) is **not** a permalink — it points + at different lines next week — so never use one. Naming the symbol is still better than linking + to it: link only when there is no name to give, such as an unnamed block inside a long function. +- **Restated acceptance criteria**, a recap of the Feature Description, or background on how the + existing system works. +- Prose paragraphs. + +Do include, when they apply: + +- **The right extension on every new frontend file.** New files on the JavaScript side are + written in TypeScript: `.ts`, or `.tsx` when the file returns JSX, with co-located tests as + `.test.ts` / `.test.tsx`. Never ask for a new `.js` or `.jsx` file. Name the types the new file + exports the same way you name any other symbol — the union of allowed values, the interface a + table row must match, the function signature with its return type + (`classifyContactLink( anchor: HTMLAnchorElement ): ContactLinkType | null`). An existing `.js` + file that the issue only edits stays as it is, unless the issue asks for a rename. +- The concrete shape of any data the change publishes — array keys, JSON payload, inline-script + global, selector signature. +- The exact insertion point when ordering matters: hook priority, array position, above or below + an early return. +- Changes to existing files that the acceptance criteria imply but do not name. + +## Step 6 — Write the Test Coverage + +Keep it short — a handful of bullets, not a test plan. Two shapes, both in use: + +``` +* Add tests for `` covering: + * + * + +* JS tests in `.test.ts` covering the `` action and the `` selector. +``` + +- One bullet per test file or area, with the cases nested under it. Name the test file when it + exists or its name follows from the source: co-located next to the source — `*.test.ts` / + `*.test.tsx` for every new test file, `*.test.js` only when you are adding cases to a JavaScript + test file that already exists — and `*Test.php` under the mirroring path in + `tests/phpunit/integration/`. +- Phrase each case as the behavior or outcome, not the mechanics — "the privacy policy field + falls back to the WordPress privacy policy URL when the publication value is missing", not + "mock the store and assert the input value". +- Cover every acceptance criterion at least once, including the negative cases: precondition + unmet, feature disabled, invalid input, empty or error response. +- **Storybook stories belong here, not in the brief.** "Add a Story for ``", or list + the states when there is more than one: "(default, loading, error)". +- Name any new fixture, fake or mock the coverage needs, and the directory it belongs in. +- Call out **existing** tests the change will break, so the work is budgeted rather than + discovered mid-implementation — "Fix any failing tests/snapshots" when the blast radius isn't + knowable up front. +- No commands. + +## Step 7 — Report what did not go in the brief + +Everything you cut for brevity, and everything you learned while verifying, belongs in your +reply to the user — not in the issue: + +- Discrepancies between the design doc and the code, with the `file:line` you checked. +- Consequences the acceptance criteria did not anticipate: a metric that will shift, a default + that changes for every install, an existing test that will start failing. +- Anything you left as an open decision, and the assumption the brief is written under. +- Every issue number you were given and what you cited it for, any cross-reference you dropped + because no number was available, and any locally-written issue that still has to be filed + before the brief's text is final. + +--- + +## Guardrails + +- **Write only the two sections.** Leave Feature Description, Acceptance criteria, QA Brief and + Changelog entry untouched. +- **Plain, simple words.** No idioms, no metaphors, no unexplained jargon, no half sentences. + Name who does what to what, and list the members of every set you name — see Step 5. +- **New frontend files are TypeScript.** `.ts` / `.tsx` sources, `.test.ts` / `.test.tsx` tests, + and a build-config change whenever the bundle cannot compile them yet. +- **Names are full words.** Every constant, key, param, method and type the brief invents is + spelled out — no `Pct`, no `Min`, no initials — see Step 5. +- **Don't publish.** Do not edit the GitHub issue or post a comment unless the user explicitly + asks. Produce the text — or update the local issue file you were given — and let them place it. +- **Don't expand scope.** If a change looks necessary but no acceptance criterion covers it, + raise it in Step 7 instead of adding it silently. +- **No indirect issue references.** Every cross-reference is a real GitHub number you asked the + user for, never a design-doc position, a relative pointer or a placeholder. +- **No plain-text line numbers.** Every pointer into the code is a GitHub permalink pinned to a + commit SHA, never `path/to/File.php:212` and never a `/blob//` URL — see Step 5. +- **Verify before naming.** Never reference a symbol, path or hook you have not opened. diff --git a/docs/context/workflow/write-issue.md b/docs/context/workflow/write-issue.md new file mode 100644 index 00000000000..d470ba982ec --- /dev/null +++ b/docs/context/workflow/write-issue.md @@ -0,0 +1,389 @@ +# Writing an Issue — Playbook + +This is the **single source of truth** for writing the **description** and the **Acceptance +criteria** of a Site Kit issue, whether that issue is a feature request or a bug report. Every AI +coding tool (Gemini CLI, Antigravity, Claude Code) points at this file through a thin per-tool +adapter, so the procedure stays identical everywhere. When you update the process, update it +**here** — not in the adapters. + +Which sections make up the description depends on the type of issue: a **Feature Description** +for a feature request, a **Bug Description** and **Steps to reproduce** for a bug report. Step 1 +settles which type you are writing. + +These are the *authoring* sections. The Implementation Brief and Test Coverage are written later, +from the criteria you produce here, by `write-implementation-brief.md`. Write the criteria so that +playbook has a contract it can execute against without re-reading the design doc. + +--- + +## Step 1 — Establish the issue type, the source and the mode + +### The type + +There are two issue templates, and they have different description sections: + +- **Feature request** — `.github/ISSUE_TEMPLATE/feature_request.md`. You write the **Feature + Description** and the **Acceptance criteria**. +- **Bug report** — `.github/ISSUE_TEMPLATE/bug_report.md`. You write the **Bug Description**, the + **Steps to reproduce** and the **Acceptance criteria**. + +Take the type from what the user asked for. A design doc, a new event, a new component, a new +setting, "add", "support" — feature request. A wrong value, a value that is never sent, a console +error, a state the plugin renders when it should not, something that used to work and no longer +does, "broken", "fix" — bug report. + +**Ask the user which type they want whenever it is not clear from their message**, and wait for +the answer before you write anything. One question costs less than writing the issue against the +wrong template. When you ask, say which way you read the request and why, so the user can correct +the reading in one word. + +A request to change behavior that works the way it was designed to work is a feature request, not +a bug, even when the user calls it a fix. Point that out when you ask. + +When the source is an existing issue, its type is already decided: read the issue and match it. +Do not convert one type into the other. + +### The source and the mode + +Source material arrives one of three ways, and only the user can tell you which: + +- **A design doc** — a path in the repo, or a hosted doc. +- **Requirements written directly in the user's message.** +- **An existing issue** — a GitHub issue number (`gh issue view --json title,body`) or a + path to a local issue file, when only the criteria are wanted. + +There are two modes, and they decide what you are allowed to touch: + +- **New issue** — write the description sections for the type and the Acceptance criteria. +- **Existing issue whose description is already written** — write **only** the Acceptance + criteria. Read the description to write against it and leave it byte-for-byte alone, unless + the user explicitly asks you to change it. This covers a Feature Description on a feature + request and a Bug Description with its Steps to reproduce on a bug report alike. + +**Stop and ask the user** if no source material was given, if the requirements are ambiguous or +contradict each other, or if the design doc covers an epic and the user has not said which issue +to write. Never invent a path to a design doc, and never go looking for a local issue file — it +has no fixed home, so the user must give you the path. + +If the design doc has a **work-estimate table of proposed issues**, that table *is* the +breakdown: one issue per row, with its title and its point estimate. Do not re-slice it. + +## Step 2 — Read the source material end to end + +- **The whole design doc**, not only the section for this issue. The reasoning behind one + issue's decisions is routinely somewhere else — Alternatives considered, Quality attributes, + Technical debt, Dependencies. +- **Any sibling spec the design doc names as authoritative** (a locked event spec, a data + contract, a PRD). Where that spec and the design doc disagree on *what* is built, the spec + wins; where the design doc and the code disagree on *how*, the code wins (Step 3). +- **Sibling issues already written for the same epic.** They set the level of detail and show + where this issue's scope stops and the next one begins. +- **For a bug report, the whole report as the user gave it** — the plugin version, the module + state, the browser, the exact values they saw. Read whatever the report names as evidence: a + support thread, a linked issue, or the pull request the user says introduced the problem. + +## Step 3 — Verify everything against the code + +Do not write criteria from the design doc alone. Open the files the issue will touch and +confirm, by reading them: + +- Every class, method, constant, hook, filter, option, script handle and path you name exists + and is spelled exactly right. +- The base class, registry or filter the issue extends has the shape the design doc claims. +- Who **consumes** the data the issue changes — a new key in a shared array, a new entry in a + registry, a new item in a walk. That is what tells you which "must stay unchanged" criteria + are real rather than imagined. +- That behavior the design doc treats as already existing actually exists. +- **For a bug report, that the wrong behavior is really what the code does today**, and where. + Read until you can point at the lines that produce the symptom, and check whether the same + code serves other cases that still work. A bug written from a guess sends the implementer to + the wrong file, and its criteria describe a fix for something that was never broken. When you + cannot find the symptom in the code, say so (Step 7) instead of writing the issue around it. + +Where the code contradicts the design doc, the code wins: write the criteria against reality and +report the discrepancy (Step 7). + +## Step 4 — Lay out the issue + +Open the template for the type you settled in Step 1 and copy it. The file *is* the layout — do +not reproduce it from memory, and do not reorder or rename its sections: + +- **Feature request** — `.github/ISSUE_TEMPLATE/feature_request.md` +- **Bug report** — `.github/ISSUE_TEMPLATE/bug_report.md` + +Make two changes to the copy, and no others: + +- **Drop the YAML frontmatter** — the `name:` and `about:` lines between the two `---` markers. + They belong to GitHub's template picker, not to the issue. +- **Put the title in its place**, as a single `# ` heading at the top of the file. + +Everything else stays exactly as the template has it: the headings in the template's order, the +`---------------` rule, the moderator notice byte-for-byte, and the placeholder comment in every +section you are not writing. The two templates word their placeholders differently — the feature +request's brief says "implement the feature" where the bug report's says "resolve the issue" — so +copy them from the file rather than carrying them over from the other type. Replace a placeholder +with your own content only in the sections the type gives you (Step 1). + +**Screenshots** and **Additional Context** on a bug report are the reporter's evidence, not yours. +Leave both as the template gives them, and fill in only the environment facts the user actually +stated — a plugin version they named, the browser they used. Never invent a version number, an +operating system or a device, and never describe a screenshot you have not seen. + +**Title** — for a feature request, name the deliverable, not the activity. Lead with the symbol or +event the issue produces, then the scope in a short clause: `` `AudienceTile` — no-data and +partially-available states ``. For a bug report, name the symptom and where it happens, in the +same shape: `` `AudienceTile` — no-data state renders for an audience that has data ``. + +Nothing sits between the title and the first description heading — no epic name, no point +estimate, no dependency line. Where the issue depends on another, that dependency is a clause +inside the description, by real GitHub issue number. + +## Referencing another issue — always ask for the number + +Every reference to another issue, in any section you write, is a real GitHub number (`#12345`). +This applies to dependencies, prerequisites, siblings in the same epic and follow-ups alike. There +is no acceptable indirect form — not "issue 5" from the design doc's ordering, not "the next +issue", not "the issue that adds the store". + +**Ask the user for the number.** Never infer it from the design doc's ordering, never guess it +from a nearby issue number, and never leave a placeholder to be filled in later: + +- **The issue is already in GitHub** — ask the user for its number, then confirm it is the issue + you mean with `gh issue view --json title` before citing it. +- **The issue only exists locally** — a markdown file you or the user wrote that has not been + filed yet. Say so, and ask the user to create it in GitHub and give you the new number. Wait + for it: a cross-reference is the one thing you cannot write around. +- **The user has no number to give** — drop the cross-reference and name the deliverable instead + (``the store introduced with `getAudienceSettings` ``). Report the dropped reference in + Step 7. + +Work out every issue you will need to cite before you start writing, and ask for all of the +numbers in one message rather than stopping repeatedly. + +## Step 5 — Write the description + +The description is the **only** place where rationale belongs. It answers "why does this issue +exist, and what problem does it solve" for someone who will never read the design doc or the +support thread. Write the sections that belong to the type you settled in Step 1. + +### A feature request: the Feature Description + +- **Open with the gap.** What happens today, and where it falls short. Name the concrete thing + that doesn't cover the case — the existing component, the class, the API behavior. +- **State what the issue adds**, in one sentence. +- **Split the description** when the work has more than one part (server + frontend, two + surfaces, markup + asset), using bold run-in headings. +- **Carry over the decisions the issue depends on**, and why — the constraint that an acceptance + criterion compresses into a single line. "Why the value is stored per-user rather than + per-site" lives here. +- **Prose paragraphs.** No bullets of criteria, no implementation instructions, no file trees. + +### A bug report: the Bug Description and Steps to reproduce + +The **Bug Description** is prose, and it is short. Three things, in this order: + +- **What the plugin does today that is wrong.** The symptom as someone sees it, on the real + surface: the widget, the page, the event, the API response, the option value. Give the wrong + value and the value that was expected instead. +- **What should happen instead**, in one sentence. +- **When it happens.** The conditions the symptom needs and the ones it does not survive: which + module is connected, which user role, which date range, which setting value, which screen size, + which browser. Say plainly when the symptom appears every time. + +Add the cause in one clause **only when you verified it in the code** in Step 3 — "the value is +read before the settings are loaded, so it is always the default". Do not prescribe the fix; that +is the Implementation Brief's job. When you did not find the cause, leave it out rather than +guessing at it. + +Name the other places the same wrong behavior reaches — another widget served by the same code, +another module with the same shape. Whether this issue covers them is a scope decision, so it is +settled in the criteria (Step 6): the ones it covers get their corrected outcome, and the ones it +leaves get the outcome that still holds at the edge. Report the split in your reply (Step 7) so the +user can move the line. + +The **Steps to reproduce** are the numbered list a tester follows with nothing but the plugin: + +- **Start from a state the tester can reach.** A fresh install, a connected module, a setting at a + named value. Make the starting state step 1 when the bug needs one. +- **One action per step**, with the real thing to click, type or paste — the page, the menu item, + the URL, the value in the field. "Set the date range to `Last 28 days`", not "choose a date + range". +- **End with the observation.** The last step is what the tester sees and what they should have + seen instead: "The tile shows `Gathering data`, though the audience returned 412 users." +- **No mechanism.** No "check the store", no "read the network response", no breakpoints — unless + the symptom genuinely cannot be seen any other way, in which case give the exact thing to open + and the exact value to look at. + +### Write in plain, simple words + +This matters more than anything else in these sections. The description is read by people: +reviewers, moderators, engineers picking the issue up months later. Many of them do not speak +English as a first language, and none of them should have to read a sentence twice. + +- **Use the simplest word that is still accurate.** Short sentences, one idea each. No + decoration, no "notably", "crucially", "simply", "of course", "elegant", "robust". +- **No idioms, metaphors, or cultural references.** "A WhatsApp button is now standard furniture + on business sites" is a phrase a reviewer has to decode; "A WhatsApp button is common on + business sites" is not. The same goes for "under the hood", "out of the box", "for free", + "dead weight", "poisoned", "leaks into". +- **Explain a technical term the first time you use it, or leave it out.** +- **Finish every sentence, and name who does what to what.** "the classifier short-circuits + `outbound_link_click`" can be read in two opposite directions. "when a link is classified as a + contact link, the listener emits `contact_link_click` and stops, so the same click is never + also reported as an outbound click" can be read in only one. +- **Cut the sentence that only sounds good.** If removing it loses no information, remove it. + +Read the finished description once more and rewrite every sentence that needs a second reading. +The same rules apply to the acceptance criteria (Step 6) and to your reply to the user (Step 7). + +Do **not** include, in either type of description: + +- **Rationale for the shape of the code.** The description says why the feature *behaves* the way + it does, for someone who will use it or verify it. Why one listener serves two events, why a + check sits in one place rather than two, what the code would have looked like otherwise — that + is the Implementation Brief's material, and only where the implementer needs it. "This is a + second consumer of one resolution — not a second `closest()` call" and "the short-circuit exists + for that privacy reason, not for tidiness" leave the reader with nothing they can act on. Write + the behavior and its reason in ordinary words instead: "when a link is already a contact link it + sends `contact_link_click` and nothing else, so the phone number in its address is never sent to + GA". +- **Links or paths to local design docs and specs.** They don't resolve for someone reading the + issue on GitHub. Restate the constraint in a clause instead. External links — Figma, a hosted + doc, public vendor documentation — are fine. +- **References to sibling issues by their position in the design doc** — "issue 5", "the next + issue", "issue 3 of this epic". The design doc's numbering is not GitHub's, and will be wrong + the moment the issues are filed. Cite the sibling by the real GitHub number you asked the user + for, or name its deliverable when there is no number to be had. + +## Step 6 — Write the Acceptance criteria + +The criteria are the contract: the Implementation Brief is written against them and the PR is +graded on them. Write them for the person who **verifies** the finished work — the QA engineer +who will click through the site, and the reviewer checking that nothing is missing. That person +does not need to know how the code works, so each criterion states something they can observe by +using the feature: what the user does, what the plugin then does, and what the result contains. + +**Keep the mechanism out.** Which class holds the logic, that one delegated listener serves every +click, that a lookup table drives the matching, which browser API parses the URL — that is the +Implementation Brief's job, and none of it can be checked by using the feature. "Matching uses +the parsed `URL.hostname` against an allowlist" tells a tester nothing; "clicking a link to +`https://notwa.me/1555` sends no event" tells them exactly what to try and what to expect. When +the criteria drop a rule the implementer still needs, put it in the brief. + +**On a bug report** the criteria are the corrected behavior, not the bug. Write the outcome the +steps in the description must produce once the issue is done — "the tile shows the audience's 412 +users" — never "the tile no longer shows `Gathering data` incorrectly", which cannot be graded +without the reader deciding what "incorrectly" meant. Then write the cases that already work and +run through the same code, so the fix is not allowed to trade one symptom for another. The cause +and the fix stay out of the criteria, in the same way technique does for a feature. + +### Shape + +- A flat list of `*` bullets. +- **One outcome per bullet.** Nest a second level when a single outcome has an enumerable set of + cases — the conditions under which a hook must not fire, the values a param may take, the + forms excluded from a match rule. +- A **table** when a criterion enumerates a lookup that prose would obscure (each widget slug + and the module that owns it). Inline it; never link out to it. + +### Content + +Each bullet states an outcome: + +- **Lead with the precondition** when the behavior is conditional — "When the module is + connected and the user has view access (#12345), …". +- **Name the real thing** — the parts of it that appear in the outcome: the event name and its + params, the config key, the inline-data global, the user-facing string, the page it renders on. + Name a class, hook or filter when the outcome itself is server behavior someone verifies on the + server; do not name the internals that merely produce a visible result. Names the issue invents + are spelled out in full words — `character_count`, never `char_cnt`. +- **Give the input that produces the outcome**, in the form a tester can paste or click: the + exact `href`, URL, setting value or sequence of clicks. `https://wa.me/15551234567` is + verifiable; "a WhatsApp link with a recipient" is not. +- **State the whole outcome, not half of it.** "`https://notwa.me/1555` must not classify" leaves + out *as what*; "clicking it sends no event" is complete. A criterion the reader has to + interpret cannot be graded. +- **List every case you refer to as a group.** "Share links are excluded" is not checkable until + the criterion writes out the share links it means. Give the members inline, or in a table the + criterion carries. +- **Give the values**: `viewContext: 'mainDashboard'`, `dateRange: 'last-28-days'`, `limit: 3`, + a 300-second cache TTL. +- **State cardinality** when it matters: "at most once per request", "exactly once per page + view", "exactly one of the four states". +- **Say what must stay unchanged** when the change touches shared code, naming the consumers + that would otherwise regress. + +### Keep them compact + +A criterion states the outcome of implementing the brief, and nothing else. Cut: + +- **Rationale.** No "because", "since", "so that", "— the same reasoning as X". If a bullet + explains itself, the explanation belongs in the Feature Description. +- **Technique and mechanism.** *How* the plugin matches, parses, stores or listens is the brief's + job. "The value falls back to the site default when the user setting is unset" is an outcome; + "read with `array_key_exists()` rather than `isset()`", "a single delegated listener on + `document`", "driven by one lookup table keyed by scheme and host" are mechanism — keep the + outcome and its counter-example ("an explicit `null` is preserved"), drop the rest. +- **Negative parentheticals.** No "(not an `id`)", "(not the raw response)", "rather than + assumed to be the first entry". State what it *is*. +- **Restatements.** Name the thing once, in the bullet that defines when it happens. Never add + a separate "the selector is named `X`" bullet, and never re-justify the name. +- **Work that isn't being done.** No "no new setting is registered", "no Storybook story is + added", "no migration needed". Silence already says it. +- **"Out of scope" and "Known limitations" lists.** The list form names work instead of behavior, + and nobody can grade it. A boundary that matters is a criterion like any other: write it as the + outcome at the edge. "The button renders on the single product template, and the cart and + checkout pages render nothing" is assertable; "Out of scope: cart and checkout" is not. + +The criteria are the contract, so **the scope boundary lives here**, in that form — not as a +clause in the description, which no PR is graded against. Keep the negatives that **are** outcomes +and that a test can assert: "the notification is not rendered when …", "an unconnected module +contributes nothing", "the legacy option is not written", "the payload carries the slug and +nothing else", "nothing is enqueued when the feature flag is off". A boundary with no observable +edge — nothing a tester could do that would show the difference — is not one the issue needs. + +## Step 7 — Report what did not go in the issue + +Everything you decided, cut or discovered belongs in your reply to the user — not in the issue: + +- **The type you wrote**, when the user did not name it and you asked — one line saying which + template you used, so a wrong answer costs one reply rather than a review cycle. +- **Decisions you had to make** because the design doc was silent, flagged as decisions so the + user can overrule them. +- **Discrepancies** between the design doc and the code, with the `file:line` you checked. +- **Anything you added** beyond the design doc's scope for this issue, so the user can drop it. +- **On a bug report**: the cause you found, with its `file:line`; anything in the report you could + not confirm in the code; and the environment facts you left blank because the user did not + state them. +- **Cross-references** — every issue number you were given and what you cited it for, any + reference you dropped because no number was available, and any locally-written issue that still + has to be filed before this issue's text is final. + +--- + +## Guardrails + +- **Ask which type of issue it is** when the user's message does not make it clear, and wait for + the answer. A feature request and a bug report use different templates and different description + sections — guessing wrong means writing the issue twice. +- **The authoring sections only.** A feature request: the Feature Description and the Acceptance + criteria. A bug report: the Bug Description, the Steps to reproduce and the Acceptance criteria. + Leave Implementation Brief, Test Coverage, QA Brief and Changelog entry as the template's + placeholder comments, and leave a bug report's Screenshots and Additional Context as the + template gives them, except for environment facts the user actually stated. +- **Plain, simple words.** No idioms, no metaphors, no unexplained jargon, no half sentences. + Name who does what to what, and list the members of every set you name — see Steps 5 and 6. +- **Criteria are observable behavior.** Written for whoever verifies the feature by using it, not + for whoever writes the code. Anything that can only be checked by reading the source belongs in + the Implementation Brief. +- **An existing description is untouchable.** When asked to add criteria to an issue that already + has a Feature Description, or a Bug Description with its Steps to reproduce, read it and leave it + exactly as it is unless explicitly told otherwise. +- **Don't publish.** Never run `gh issue create` or `gh issue edit`, and never post a comment, + unless the user explicitly asks. Produce the markdown file and let them place it. +- **No local paths anywhere in the sections you write, and no indirect issue references** — every + cross-reference is a real GitHub number you asked the user for, never a design-doc position, + a relative pointer or a placeholder. +- **Verify before naming.** Never reference a symbol, path or hook you have not opened. +- **One issue per unit of work.** If the source material describes more, write them as separate + files rather than merging them.