From 3b4cbefe8dcc7e30da268b98554792a86443233d Mon Sep 17 00:00:00 2001 From: Eugene Manuilov Date: Sat, 8 Aug 2026 16:05:56 +0300 Subject: [PATCH 1/8] Add write-issue skill and workflow documentation for Site Kit issues. --- .agents/skills/write-issue/SKILL.md | 58 +++++++ .agents/workflows/write-issue.md | 50 ++++++ .claude/skills/write-issue/SKILL.md | 1 + .gemini/commands/write-issue.toml | 72 ++++++++ docs/context/workflow/write-issue.md | 239 +++++++++++++++++++++++++++ 5 files changed, 420 insertions(+) create mode 100644 .agents/skills/write-issue/SKILL.md create mode 100644 .agents/workflows/write-issue.md create mode 120000 .claude/skills/write-issue/SKILL.md create mode 100644 .gemini/commands/write-issue.toml create mode 100644 docs/context/workflow/write-issue.md diff --git a/.agents/skills/write-issue/SKILL.md b/.agents/skills/write-issue/SKILL.md new file mode 100644 index 00000000000..a285f4d3b43 --- /dev/null +++ b/.agents/skills/write-issue/SKILL.md @@ -0,0 +1,58 @@ +--- +name: write-issue +description: > + Write the Feature Description and Acceptance criteria of a Site Kit issue from a design doc or + from requirements given in the message. Use when the user asks to create, draft or write a + GitHub issue or ticket from a design doc or requirements, 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 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, or requirements]" +allowed-tools: Bash, Read, Grep, Glob, Edit, Write +--- + +# Write an issue + +Write the **Feature Description** and **Acceptance criteria** for **$ARGUMENTS** — a design doc +path, 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 source and the mode.** New issue → write both sections. Existing issue that + already has a Feature Description → 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. +3. **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, and the + sibling issues already written for the epic. +4. **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. Where the design doc and the code disagree, the code wins. +5. **Lay the issue out** per `.github/ISSUE_TEMPLATE/feature_request.md`, keeping the moderator + notice and the placeholder comments for the sections you are not writing. The title names the + deliverable; nothing sits between it and `## Feature Description`. A dependency on another + issue is a clause inside the description, by real GitHub issue number. +6. **Write the Feature Description** — prose. The gap today, what this adds, the halves of the + work, and the load-bearing decisions with their reasons. This is the only place rationale + belongs. +7. **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. +8. **Report separately** the decisions you had to make, code/design-doc discrepancies, anything + added beyond scope, and any ordering constraint between the issues. + +## Important + +- **Outcomes only in the criteria.** No rationale, no technique notes, no negative parentheticals + ("(not an `id`)"), no restating the event name, no "no X is needed", no out-of-scope or + known-limitation lists. Keep negatives only when the negative is the asserted outcome. +- **No links or paths to local design docs or specs** in either section — 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 Feature Description is untouchable** unless the user explicitly asks. +- **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-issue.md b/.agents/workflows/write-issue.md new file mode 100644 index 00000000000..b883850b430 --- /dev/null +++ b/.agents/workflows/write-issue.md @@ -0,0 +1,50 @@ +--- +title: Write an issue +description: Write the Feature Description and Acceptance criteria of a Site Kit issue from a design doc or from 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 source and the mode** — a design doc, 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 both sections; an **existing** issue that already has a Feature + Description 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. +2. **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; and the sibling issues already written for the + epic, which show where this issue's scope stops. +3. **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. Where the design doc and the code + disagree, the code wins. +4. **Lay the issue out** per `.github/ISSUE_TEMPLATE/feature_request.md` — moderator notice + intact, placeholder comments left in the sections you are not writing. The title names the + deliverable, and nothing sits between it and `## Feature Description` — no epic name, no + point estimate, no dependency line. A dependency on another issue is a clause inside the + description, by real GitHub issue number. +5. **Write the Feature Description** — prose only. 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. This is the only section where rationale belongs. +6. **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. Cut rationale, technique notes, + negative parentheticals, restated event names, work that isn't being done, and out-of-scope + or known-limitation lists. +7. **Report separately** the decisions you had to make, discrepancies against the code with + `file:line`, 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. + +**Two 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/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/commands/write-issue.toml b/.gemini/commands/write-issue.toml new file mode 100644 index 00000000000..473bb8ee3d6 --- /dev/null +++ b/.gemini/commands/write-issue.toml @@ -0,0 +1,72 @@ +description = "Writes the Feature Description and Acceptance criteria of a GitHub issue from a design doc or requirements, verified against the codebase" + +prompt = """ +You are writing the **Feature Description** and **Acceptance criteria** of a Site Kit issue for {{args}} — a path to a design doc, requirements stated in the user's message, or an existing GitHub issue in google/site-kit-wp that needs acceptance criteria. If nothing usable was given, ask for it before doing anything else. + +The canonical, tool-agnostic procedure is `docs/context/workflow/write-issue.md`. Read it and follow it exactly; the phases below are the same steps, and the playbook wins on any detail. + +--- CRITICAL INSTRUCTIONS --- + +- Write ONLY the Feature Description and Acceptance criteria. Leave Implementation Brief, Test Coverage, QA Brief and Changelog entry as the template's placeholder comments. +- If the issue ALREADY has a Feature Description, write ONLY the Acceptance criteria and leave the description byte-for-byte unchanged, unless the user explicitly asks you to change it. +- DO NOT run `gh issue create` or `gh issue edit`, and DO NOT post a comment, unless the user explicitly asks. Produce the markdown file and let the user place it. +- DO NOT name any class, method, hook, handle or path you have not opened and read. +- DO NOT put links or paths to local design docs or spec files in either section — they do not resolve on GitHub. +- DO NOT refer to a sibling issue by its position in the design doc ("issue 5", "the next issue"). That numbering is not GitHub's and will be wrong. Use a real issue number, or the name of the deliverable. + +--- PHASE 1: ESTABLISH THE SOURCE AND THE MODE --- + +The source is a design doc (a repo path or a hosted doc), requirements written in the user's message, or an existing issue (`!{gh issue view {{args}} --json title,body}`, or a local path the user supplies — never go looking for one). + +Two modes: a NEW issue gets both sections; an EXISTING issue that already has a Feature Description gets ONLY the criteria. + +STOP and ask the user if no source material was given, if the requirements are ambiguous or contradictory, 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. + +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 points. Do not re-slice it. + +--- PHASE 2: READ THE SOURCE MATERIAL END TO END --- + +Read the WHOLE design doc, not just this issue's section — the reasoning behind one issue's decisions routinely sits in Alternatives considered, Quality attributes, Technical debt or Dependencies. Read any sibling spec the design doc names as authoritative; where that spec and the design doc disagree on what is built, the spec wins. Read the sibling issues already written for the epic — they set the level of detail and show where this issue's scope stops. + +--- PHASE 3: VERIFY AGAINST THE CODE --- + +Open the files the issue will touch. Confirm every class, method, constant, hook, filter, option, script handle and path exists and is spelled exactly right; confirm the base class, registry or filter being extended has the shape the design doc claims; identify who CONSUMES the data the issue changes, which is what makes a "must stay unchanged" criterion real rather than imagined; and confirm that behavior the design doc treats as already existing actually exists. + +Where the code contradicts the design doc, the code wins. + +--- PHASE 4: LAY THE ISSUE OUT --- + +Follow `.github/ISSUE_TEMPLATE/feature_request.md` exactly, keeping the `Do not alter or remove anything below` moderator notice and the placeholder comments for the sections you are not writing. + +The title names the deliverable, not the activity — lead with the symbol or event produced, then the scope in a short clause. Nothing sits between the title and `## Feature Description` — no epic name, no point estimate, no dependency line. Where the issue depends on another, that dependency is a clause inside the Feature Description, by REAL GitHub issue number. + +--- PHASE 5: WRITE THE FEATURE DESCRIPTION --- + +Prose paragraphs. This is the ONLY section where rationale belongs. + +Open with the gap: what happens today and where it falls short, naming the concrete thing that does not cover the case. State what this issue adds in one sentence. Break the work into halves with bold run-in headings when it has more than one (server + frontend, two surfaces, markup + asset). Carry over the load-bearing decisions the design doc made, and their reasons — the constraint an acceptance criterion compresses into one line. + +No bullets of criteria, no implementation instructions, no file trees. + +--- PHASE 6: WRITE THE ACCEPTANCE CRITERIA --- + +A flat list of `*` bullets. Bold run-in group labels only when the issue has genuinely separate halves. One outcome per bullet; nest a second level when one outcome has an enumerable set of cases (the conditions a hook must not fire under, the values a param may take, the forms excluded from a match rule). Inline a table when a criterion enumerates a lookup that prose would obscure — never link out to it. + +Each bullet states an outcome that can be checked by reading the diff or exercising the feature. Lead with the precondition when the behavior is conditional. Name the real symbol — event name, config key, inline-data global, CSS selector, param names, hook, class. Give the values (`viewContext: 'mainDashboard'`, `dateRange: 'last-28-days'`, `limit: 3`). State cardinality when it matters ("at most once per request", "exactly one of the four states"). Say what must stay unchanged when the change touches shared code, naming the consumers. + +CUT from the criteria: +- rationale — no "because", "since", "so that", "the same reasoning as X"; +- technique — HOW to match, parse or store 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()`" is a technique note. Keep the outcome and its counter-example, drop the note; +- negative parentheticals — "(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 "the selector is named X"; +- work that is not being done — "no new setting is registered", "no Storybook story is added", "no migration needed". Silence says it; +- out-of-scope and known-limitation lists. + +KEEP the negatives that are themselves assertable outcomes: "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". + +--- PHASE 7: REPORT WHAT DID NOT GO IN THE ISSUE --- + +In your reply to the user — not in the issue — report the 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; and any ordering constraint, when one issue's text refers to another's deliverable by name. + +Begin with Phase 1. +""" diff --git a/docs/context/workflow/write-issue.md b/docs/context/workflow/write-issue.md new file mode 100644 index 00000000000..8127443a553 --- /dev/null +++ b/docs/context/workflow/write-issue.md @@ -0,0 +1,239 @@ +# Writing an Issue — Playbook + +This is the **single source of truth** for writing the **Feature Description** and **Acceptance +criteria** 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. + +These are the two *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 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 both the Feature Description and the Acceptance criteria. +- **Existing issue with a Feature Description 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. + +**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. + +## 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. + +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 + +Follow `.github/ISSUE_TEMPLATE/feature_request.md` exactly, including the moderator notice and +the placeholder comments for the sections you are not writing: + +```markdown +# + +## Feature Description + +<prose> + +--------------- + +_Do not alter or remove anything below. The following sections will be managed by moderators only._ + +## Acceptance criteria + +* <criterion> + +## Implementation Brief + +* [ ] <!-- One or more bullet points for how to technically implement the feature. Make sure to include changes to Storybook and visual regression tests where relevant. --> + +### Test Coverage + +* <!-- One or more bullet points for how to implement automated tests to verify the feature works. --> + +## QA Brief + +* <!-- One or more bullet points for how to test that the feature works as expected. --> + +## Changelog entry + +* <!-- One sentence summarizing the PR, to be used in the changelog. --> +``` + +**Title** — 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 ``. + +Nothing sits between the title and `## Feature Description` — no epic name, no point estimate, +no dependency line. Where the issue depends on another, that dependency is a clause inside the +Feature Description, by real GitHub issue number. + +## Referencing another issue — always ask for the number + +Every reference to another issue, in either section, 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 <number> --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 Feature Description + +This is the **only** section where rationale belongs. It answers "why does this exist, and what +problem does it solve" for someone who will never read the design doc. + +- **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. +- **Break out the halves** when the work has more than one (server + frontend, two surfaces, + markup + asset) using bold run-in headings. +- **Carry over the load-bearing decisions** the design doc made, 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. + +Do **not** include: + +- **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. Every criterion must be checkable by reading the diff or exercising the feature. + +### Shape + +- A flat list of `*` bullets. Add bold run-in group labels (`**Server**`, `**Frontend**`, + `**Both**`) only when the issue has genuinely separate halves. +- **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 event name, the config key, the inline-data global, the CSS + selector, the param names, the hook, the class. +- **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.** *How* to match, parse or store something 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()`" is a technique note — keep the outcome and its + counter-example ("an explicit `null` is preserved"), drop the note. +- **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 lists and known-limitation lists.** They describe what isn't built. A scope + boundary belongs in the Feature Description, in a clause, and only when it genuinely prevents + someone building the wrong thing. + +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". + +## 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: + +- **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. +- **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 + +- **Two sections only.** Leave Implementation Brief, Test Coverage, QA Brief and Changelog entry + as the template's placeholder comments. +- **An existing Feature Description is untouchable.** When asked to add criteria to an issue that + already has one, 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 either section, 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. From c9d970fa39b9aaee9870b857e9f80da82434158e Mon Sep 17 00:00:00 2001 From: Eugene Manuilov <eugene.manuilov@fueled.com> Date: Sat, 8 Aug 2026 16:06:07 +0300 Subject: [PATCH 2/8] Add implementation brief and test coverage writing playbook, skills, and commands. --- .../write-implementation-brief/SKILL.md | 55 ++++ .../workflows/write-implementation-brief.md | 39 +++ .../write-implementation-brief/SKILL.md | 1 + .gemini/commands/write-brief.toml | 91 +++++++ AGENTS.md | 24 ++ .../workflow/write-implementation-brief.md | 246 ++++++++++++++++++ 6 files changed, 456 insertions(+) create mode 100644 .agents/skills/write-implementation-brief/SKILL.md create mode 100644 .agents/workflows/write-implementation-brief.md create mode 120000 .claude/skills/write-implementation-brief/SKILL.md create mode 100644 .gemini/commands/write-brief.toml create mode 100644 docs/context/workflow/write-implementation-brief.md 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/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 <issue-number-or-path>` 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 <number> --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/.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/.gemini/commands/write-brief.toml b/.gemini/commands/write-brief.toml new file mode 100644 index 00000000000..052403a7f38 --- /dev/null +++ b/.gemini/commands/write-brief.toml @@ -0,0 +1,91 @@ +description = "Writes the Implementation Brief and Test Coverage sections of a GitHub issue, verified against the codebase" + +prompt = """ +You are writing the **Implementation Brief** and **Test Coverage** sections for {{args}} — either a GitHub issue number in the google/site-kit-wp repository, or a path the user supplied to a local issue file. If neither was given, ask for one before doing anything else; never go looking for the file yourself. + +The canonical, tool-agnostic procedure is `docs/context/workflow/write-implementation-brief.md`. Read it and follow it exactly; the phases below are the same steps, and the playbook wins on any detail. + +--- CRITICAL INSTRUCTIONS --- + +- Write ONLY the Implementation Brief and Test Coverage sections. Leave Feature Description, Acceptance criteria, QA Brief and Changelog entry untouched. +- DO NOT edit the GitHub issue or post a comment. Produce the text, or update the local issue file you were given, and let the user place it. +- DO NOT name any class, method, hook, handle or path you have not opened and read. +- DO NOT expand scope beyond the acceptance criteria. + +--- PHASE 1: READ THE ISSUE --- + +For a GitHub issue, fetch it using the GitHub MCP tools (`issue_read`). Fallback: `!{gh issue view {{args}} --json title,body,labels}`. For a local path, read the file. + +Extract: +1. **Feature Description** (between `## Feature Description` and the moderator notice) +2. **Acceptance criteria** (between `## Acceptance criteria` and `## Implementation Brief`) + +The acceptance criteria are the contract: everything you write must trace back to one, and nothing may quietly extend them. + +**STOP and ask the user** if the issue is not found, the body is empty, or the acceptance criteria are missing, ambiguous, or contradictory. + +--- PHASE 2: READ THE SOURCE MATERIAL --- + +- The design doc. Use the one the issue links, or the path the user gives you; otherwise ask the user for it, bundling the question with any Phase 1 ask. Many issues have no design doc — if there isn't one, say so and write the brief from the acceptance criteria and the code. Never stall on it, and never invent a path to one. +- The sibling issues in the same epic, which bound where this issue's scope stops and supply the numbers to cross-reference. + +--- PHASE 3: VERIFY AGAINST THE CODE --- + +Open every file the brief will touch. Confirm each class, method, constant, hook, filter, option and script handle exists and is spelled exactly right; confirm the pattern you will tell someone to follow actually looks that way; trace hook ordering to its `do_action()` / `apply_filters()` call site when the brief depends on it; and identify the existing tests, fixtures, snapshots and inline data the change will move. + +Where the code contradicts the design doc, the code wins. + +--- PHASE 4: LOAD THE CONVENTION DOCS --- + +Read only the `docs/context/{js,php}` docs the issue touches, using the scope map in `docs/context/workflow/implement-issue.md` Step 3. + +--- PHASE 5: WRITE THE IMPLEMENTATION BRIEF --- + +Group by path. Every group is headed by the path it changes, with the changes nested under it. Two shapes are in use — pick one and stay consistent within the issue: + +``` +* [ ] In `path/to/File.php`: + * <one change per bullet> +``` + +``` +In `path/to/directory/` (new directory): + +* [ ] <one change per bullet> +``` + +Head a group with 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, new classes, registration, build config, components, SCSS last. Nest a third level when one bullet covers several related behaviors, ending the parent in a colon ("Submit behavior:" → on click / on success / on failure). + +Every bullet is an instruction. Name the real thing wherever the codebase already decides it: classes, methods, hooks, constants, script handles, datapoint names (`GET:ctas`); selectors and actions with their signature (`getCTAs( { organizationID, publicationID } )`); components to reuse (`SpinnerButton`, `ProgressBar`, `Notice`), their props (`helperText`) and CSS class names (`mdc-text-field--error`); inline-data globals and keys; and user-facing strings quoted verbatim. Point at an existing exemplar ("following `publications.js`") instead of describing one. Cross-reference sibling issues inline (`(added in #12950)`, `(see #13005)`). Fence the scope in one line when an adjacent concern could be pulled in. For UI work, reference the design rather than restating measurements, spell out the interaction states (what disables the CTA, pending, success, error), and name the SCSS partial and the index file to import it into. + +Do NOT include: +- rationale, justification, trade-offs, or the benefits of a decision; +- work that is not needed ("no Storybook changes required", "no migration needed"); +- routine commands (lint, build, test, VRT) — `implement-issue.md` owns verification; +- links to local design docs — the brief is read on GitHub, where those paths do not resolve. External links (Figma, a hosted design doc) are fine; +- restated acceptance criteria, a recap of the Feature Description, or background prose. + +DO include, when they apply: the concrete shape of published data (array keys, JSON payload, inline-script global, selector signature); the exact insertion point when ordering matters (hook priority, array position, above/below an early return); and changes to existing files the criteria imply but do not name. + +--- PHASE 6: WRITE THE TEST COVERAGE --- + +Keep it short — a handful of bullets, not a test plan. Two shapes, both in use: + +``` +* Add tests for `<source file>` covering: + * <case> + +* JS tests in `<file>.test.js` covering the `<name>` action and the `<name>` selector. +``` + +One bullet per test file or area, cases nested under it. Name real paths (co-located `*.test.js` / `*.test.tsx` next to the source, `*Test.php` under the mirroring `tests/phpunit/integration/` path). Phrase each case as the behavior or outcome, not the mechanics. Cover every acceptance criterion including its negative cases. **Storybook stories belong here, not in the brief** — "Add a Story for `<Component>`", or list the states when there is more than one ("default, loading, error"). Name any new fixture or fake and where it goes. Call out existing tests the change will break ("Fix any failing tests/snapshots" when the blast radius isn't knowable up front). No commands. + +--- PHASE 7: REPORT WHAT DID NOT GO IN THE BRIEF --- + +In your reply to the user — not in the issue — report: +- 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 fail); +- anything left as an open decision, and the assumption the brief is written under. + +Begin with Phase 1. +""" diff --git a/AGENTS.md b/AGENTS.md index 732d9c5bdcc..5c6bdc512ae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,6 +3,30 @@ ## 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 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). Verify every class, +method, hook, handle and path against the code before naming it. Rationale belongs in the +**Feature Description**; the **Acceptance criteria** are checkable outcomes only — no rationale, +no technique notes, no negative parentheticals, no work that isn't being done. 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** those two sections, leave an existing Feature 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/write-implementation-brief.md b/docs/context/workflow/write-implementation-brief.md new file mode 100644 index 00000000000..aab907870eb --- /dev/null +++ b/docs/context/workflow/write-implementation-brief.md @@ -0,0 +1,246 @@ +# 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 <number> --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. Everything in the brief must trace back to one, +and nothing in the brief may quietly extend them. + +**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. + +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 <number> --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`: + * <one change per bullet> + * <one change per bullet> +``` + +``` +In `path/to/directory/` (new directory): + +* [ ] <one change per bullet> +* [ ] <one change per bullet> +``` + +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: + * <on click> + * <on success> + * <on failure> +``` + +### Content + +**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. + +**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. +- **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 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 `<source file>` covering: + * <case> + * <case> + +* JS tests in `<file>.test.js` covering the `<name>` action and the `<name>` 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 `*.test.js` / `*.test.tsx` next to the + source, `*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 `<Component>`", 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. +- **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. +- **Verify before naming.** Never reference a symbol, path or hook you have not opened. From d25ba193353a79391778f3c80d5d19d939381595 Mon Sep 17 00:00:00 2001 From: Eugene Manuilov <eugene.manuilov@fueled.com> Date: Sat, 8 Aug 2026 16:06:13 +0300 Subject: [PATCH 3/8] Enhance implementation playbook with guidance on referencing related issues before coding. --- docs/context/workflow/implement-issue.md | 33 +++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) 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 <number> --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.` From 36018c2a185c2641351447b253b6a2b6de61d526 Mon Sep 17 00:00:00 2001 From: Eugene Manuilov <eugene.manuilov@fueled.com> Date: Sat, 8 Aug 2026 17:04:32 +0300 Subject: [PATCH 4/8] Add settings.json for Claude plugin configuration and update .gitignore to include it. --- .claude/settings.json | 8 ++++++++ .gitignore | 1 + 2 files changed, 9 insertions(+) create mode 100644 .claude/settings.json diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000000..97e845d5935 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,8 @@ +{ + "enabledPlugins": { + "figma@claude-plugins-official": true, + "superpowers@claude-plugins-official": true, + "playwright@claude-plugins-official": true, + "chrome-devtools-mcp@claude-plugins-official": true + } +} 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/ From 5714fc2be0d39ffaefa00bba2b956defee9452a1 Mon Sep 17 00:00:00 2001 From: Eugene Manuilov <eugene.manuilov@fueled.com> Date: Thu, 13 Aug 2026 18:52:40 +0300 Subject: [PATCH 5/8] Enhance clarity and detail in issue writing guidelines. --- .../workflow/write-implementation-brief.md | 71 ++++++++++++++-- docs/context/workflow/write-issue.md | 83 ++++++++++++++++--- 2 files changed, 137 insertions(+), 17 deletions(-) diff --git a/docs/context/workflow/write-implementation-brief.md b/docs/context/workflow/write-implementation-brief.md index aab907870eb..55d79e46cbd 100644 --- a/docs/context/workflow/write-implementation-brief.md +++ b/docs/context/workflow/write-implementation-brief.md @@ -28,8 +28,11 @@ 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. Everything in the brief must trace back to one, -and nothing in the brief may quietly extend them. +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 @@ -64,6 +67,14 @@ confirm, by reading it: - 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). @@ -133,6 +144,33 @@ colon: ### 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. @@ -147,6 +185,14 @@ search for something the codebase has already decided. - 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. @@ -184,6 +230,13 @@ Do **not** include: 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 @@ -199,12 +252,14 @@ Keep it short — a handful of bullets, not a test plan. Two shapes, both in use * <case> * <case> -* JS tests in `<file>.test.js` covering the `<name>` action and the `<name>` selector. +* JS tests in `<file>.test.ts` covering the `<name>` action and the `<name>` 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 `*.test.js` / `*.test.tsx` next to the - source, `*Test.php` under the mirroring path in `tests/phpunit/integration/`. + 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". @@ -237,6 +292,12 @@ reply to the user — not in the issue: - **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, diff --git a/docs/context/workflow/write-issue.md b/docs/context/workflow/write-issue.md index 8127443a553..198133c1a4d 100644 --- a/docs/context/workflow/write-issue.md +++ b/docs/context/workflow/write-issue.md @@ -137,15 +137,46 @@ problem does it solve" for someone who will never read the design doc. - **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. -- **Break out the halves** when the work has more than one (server + frontend, two surfaces, - markup + asset) using bold run-in headings. -- **Carry over the load-bearing decisions** the design doc made, 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. +- **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. +### Write in plain, simple words + +This matters more than anything else in the section. 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: +- **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. @@ -157,7 +188,17 @@ Do **not** include: ## 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. Every criterion must be checkable by reading the diff or exercising the feature. +graded on them. Write them for the person who **verifies** the finished feature — 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. ### Shape @@ -175,8 +216,20 @@ 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 event name, the config key, the inline-data global, the CSS - selector, the param names, the hook, the class. +- **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 @@ -190,10 +243,11 @@ 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.** *How* to match, parse or store something 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()`" is a technique note — keep the outcome and its - counter-example ("an explicit `null` is preserved"), drop the note. +- **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 @@ -227,6 +281,11 @@ Everything you decided, cut or discovered belongs in your reply to the user — - **Two sections only.** Leave Implementation Brief, Test Coverage, QA Brief and Changelog entry as the template's placeholder comments. +- **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 Feature Description is untouchable.** When asked to add criteria to an issue that already has one, 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, From 0b0d9c5b1d5ae10356b27f3d2a19bccd8739e075 Mon Sep 17 00:00:00 2001 From: Eugene Manuilov <eugene.manuilov@fueled.com> Date: Mon, 17 Aug 2026 16:40:13 +0300 Subject: [PATCH 6/8] Address code review feedback. --- .agents/skills/write-issue/SKILL.md | 105 +++++--- .agents/workflows/write-issue.md | 94 ++++--- .gemini/commands/write-issue.toml | 70 ++++-- AGENTS.md | 29 ++- .../workflow/write-implementation-brief.md | 13 + docs/context/workflow/write-issue.md | 233 ++++++++++++------ 6 files changed, 373 insertions(+), 171 deletions(-) diff --git a/.agents/skills/write-issue/SKILL.md b/.agents/skills/write-issue/SKILL.md index a285f4d3b43..9d3db7bc7b5 100644 --- a/.agents/skills/write-issue/SKILL.md +++ b/.agents/skills/write-issue/SKILL.md @@ -1,58 +1,91 @@ --- name: write-issue description: > - Write the Feature Description and Acceptance criteria of a Site Kit issue from a design doc or - from requirements given in the message. Use when the user asks to create, draft or write a - GitHub issue or ticket from a design doc or requirements, 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 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, or requirements]" + 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 **Feature Description** and **Acceptance criteria** for **$ARGUMENTS** — a design doc -path, 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. +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 source and the mode.** New issue → write both sections. Existing issue that - already has a Feature Description → 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. -3. **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, and the - sibling issues already written for the epic. -4. **Verify against the code** before naming anything: every class, method, hook, constant, +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. Where the design doc and the code disagree, the code wins. -5. **Lay the issue out** per `.github/ISSUE_TEMPLATE/feature_request.md`, keeping the moderator - notice and the placeholder comments for the sections you are not writing. The title names the - deliverable; nothing sits between it and `## Feature Description`. A dependency on another + 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. -6. **Write the Feature Description** — prose. The gap today, what this adds, the halves of the - work, and the load-bearing decisions with their reasons. This is the only place rationale - belongs. -7. **Write the Acceptance criteria** — a flat `*` list, one checkable outcome per bullet, nested +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. -8. **Report separately** the decisions you had to make, code/design-doc discrepancies, anything - added beyond scope, and any ordering constraint between the issues. + 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 -- **Outcomes only in the criteria.** No rationale, no technique notes, no negative parentheticals - ("(not an `id`)"), no restating the event name, no "no X is needed", no out-of-scope or - known-limitation lists. Keep negatives only when the negative is the asserted outcome. -- **No links or paths to local design docs or specs** in either section — they don't resolve on - GitHub. Restate the constraint instead. External links are fine. +- **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 Feature Description is untouchable** unless the user explicitly asks. +- **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-issue.md b/.agents/workflows/write-issue.md index b883850b430..4fab661aaca 100644 --- a/.agents/workflows/write-issue.md +++ b/.agents/workflows/write-issue.md @@ -1,50 +1,80 @@ --- title: Write an issue -description: Write the Feature Description and Acceptance criteria of a Site Kit issue from a design doc or from requirements, verified against the codebase. +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 <design-doc-path | issue-number | requirements>` in the Agent chat. +Run with `/write-issue <design-doc-path | issue-number | bug report | requirements>` 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 source and the mode** — a design doc, requirements in the user's message, or - an existing issue (`gh issue view <number> --json title,body`, or a path the user supplies). - A **new** issue gets both sections; an **existing** issue that already has a Feature - Description 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. -2. **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; and the sibling issues already written for the - epic, which show where this issue's scope stops. -3. **Verify against the code** — every class, method, hook, constant, handle and path; the shape +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 <number> --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. Where the design doc and the code - disagree, the code wins. -4. **Lay the issue out** per `.github/ISSUE_TEMPLATE/feature_request.md` — moderator notice - intact, placeholder comments left in the sections you are not writing. The title names the - deliverable, and nothing sits between it and `## Feature Description` — no epic name, no - point estimate, no dependency line. A dependency on another issue is a clause inside the - description, by real GitHub issue number. -5. **Write the Feature Description** — prose only. 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. This is the only section where rationale belongs. -6. **Write the Acceptance criteria** — a flat `*` list, one checkable outcome per bullet, nested + 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. Cut rationale, technique notes, - negative parentheticals, restated event names, work that isn't being done, and out-of-scope - or known-limitation lists. -7. **Report separately** the decisions you had to make, discrepancies against the code with - `file:line`, anything added beyond the design doc's scope, and any ordering constraint between - the issues. + 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. -**Two 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. +**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/.gemini/commands/write-issue.toml b/.gemini/commands/write-issue.toml index 473bb8ee3d6..728a88f7273 100644 --- a/.gemini/commands/write-issue.toml +++ b/.gemini/commands/write-issue.toml @@ -1,72 +1,100 @@ -description = "Writes the Feature Description and Acceptance criteria of a GitHub issue from a design doc or requirements, verified against the codebase" +description = "Writes the description and Acceptance criteria of a GitHub issue — feature request or bug report — from a design doc, a bug report or requirements, verified against the codebase" prompt = """ -You are writing the **Feature Description** and **Acceptance criteria** of a Site Kit issue for {{args}} — a path to a design doc, requirements stated in the user's message, or an existing GitHub issue in google/site-kit-wp that needs acceptance criteria. If nothing usable was given, ask for it before doing anything else. +You are writing the **description** and the **Acceptance criteria** of a Site Kit issue for {{args}} — a path to a design doc, a bug report, requirements stated in the user's message, or an existing GitHub issue in google/site-kit-wp that needs acceptance criteria. If nothing usable was given, ask for it before doing anything else. The canonical, tool-agnostic procedure is `docs/context/workflow/write-issue.md`. Read it and follow it exactly; the phases below are the same steps, and the playbook wins on any detail. --- CRITICAL INSTRUCTIONS --- -- Write ONLY the Feature Description and Acceptance criteria. Leave Implementation Brief, Test Coverage, QA Brief and Changelog entry as the template's placeholder comments. -- If the issue ALREADY has a Feature Description, write ONLY the Acceptance criteria and leave the description byte-for-byte unchanged, unless the user explicitly asks you to change it. +- ASK THE USER whether they want a feature request or a bug report whenever their message does not make it clear, and WAIT for the answer before writing anything. The two use different templates and different description sections. +- Write ONLY the description sections for the type and the Acceptance criteria. Leave Implementation Brief, Test Coverage, QA Brief and Changelog entry as the template's placeholder comments. +- If the issue ALREADY has its description written — a Feature Description, or a Bug Description with its Steps to reproduce — write ONLY the Acceptance criteria and leave the description byte-for-byte unchanged, unless the user explicitly asks you to change it. - DO NOT run `gh issue create` or `gh issue edit`, and DO NOT post a comment, unless the user explicitly asks. Produce the markdown file and let the user place it. - DO NOT name any class, method, hook, handle or path you have not opened and read. -- DO NOT put links or paths to local design docs or spec files in either section — they do not resolve on GitHub. +- DO NOT put links or paths to local design docs or spec files in any section you write — they do not resolve on GitHub. - DO NOT refer to a sibling issue by its position in the design doc ("issue 5", "the next issue"). That numbering is not GitHub's and will be wrong. Use a real issue number, or the name of the deliverable. ---- PHASE 1: ESTABLISH THE SOURCE AND THE MODE --- +--- PHASE 1: ESTABLISH THE ISSUE TYPE --- -The source is a design doc (a repo path or a hosted doc), requirements written in the user's message, or an existing issue (`!{gh issue view {{args}} --json title,body}`, or a local path the user supplies — never go looking for one). +There are two templates, and the type decides which sections you write: -Two modes: a NEW issue gets both sections; an EXISTING issue that already has a Feature Description gets ONLY the criteria. +- 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. + +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. Say which way you read the request when you ask, so they can correct it 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. + +--- PHASE 2: ESTABLISH THE SOURCE AND THE MODE --- + +The source is a design doc (a repo path or a hosted doc), a bug report or requirements written in the user's message, or an existing issue (`!{gh issue view {{args}} --json title,body}`, or a local path the user supplies — never go looking for one). + +Two modes: a NEW issue gets the description sections and the criteria; an EXISTING issue whose description is already written gets ONLY the criteria. STOP and ask the user if no source material was given, if the requirements are ambiguous or contradictory, 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. 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 points. Do not re-slice it. ---- PHASE 2: READ THE SOURCE MATERIAL END TO END --- +--- PHASE 3: READ THE SOURCE MATERIAL END TO END --- Read the WHOLE design doc, not just this issue's section — the reasoning behind one issue's decisions routinely sits in Alternatives considered, Quality attributes, Technical debt or Dependencies. Read any sibling spec the design doc names as authoritative; where that spec and the design doc disagree on what is built, the spec wins. Read the sibling issues already written for the epic — they set the level of detail and show where this issue's scope stops. ---- PHASE 3: VERIFY AGAINST THE CODE --- +For a bug, read the whole report as the user gave it — plugin version, module state, browser, the exact values they saw — plus whatever it names as evidence: a support thread, a linked issue, or the pull request the user says introduced the problem. + +--- PHASE 4: VERIFY AGAINST THE CODE --- Open the files the issue will touch. Confirm every class, method, constant, hook, filter, option, script handle and path exists and is spelled exactly right; confirm the base class, registry or filter being extended has the shape the design doc claims; identify who CONSUMES the data the issue changes, which is what makes a "must stay unchanged" criterion real rather than imagined; and confirm that behavior the design doc treats as already existing actually exists. +For a bug, confirm that the wrong behavior is really what the code does today. Read until you can point at the lines that produce the symptom, and check which cases served by the same code still work. When you cannot find the symptom in the code, say so in Phase 8 instead of writing the issue around it. + Where the code contradicts the design doc, the code wins. ---- PHASE 4: LAY THE ISSUE OUT --- +--- PHASE 5: LAY THE ISSUE OUT --- + +Copy the template file for the type from Phase 1 — the file IS the layout, so do not reproduce it from memory or reorder its sections. Make two changes and no others: drop the YAML frontmatter (the `name:` and `about:` lines between the `---` markers), and put the title in its place as a single `# ` heading at the top. Everything else stays exactly as the file has it, including the `Do not alter or remove anything below` 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. + +On a bug report, Screenshots and Additional Context 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. Never invent a version number, an operating system or a device, and never describe a screenshot you have not seen. -Follow `.github/ISSUE_TEMPLATE/feature_request.md` exactly, keeping the `Do not alter or remove anything below` moderator notice and the placeholder comments for the sections you are not writing. +The title names the deliverable, not the activity — lead with the symbol or event produced, then the scope in a short clause. For a bug, name the symptom and where it happens, in the same shape. 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. -The title names the deliverable, not the activity — lead with the symbol or event produced, then the scope in a short clause. Nothing sits between the title and `## Feature Description` — no epic name, no point estimate, no dependency line. Where the issue depends on another, that dependency is a clause inside the Feature Description, by REAL GitHub issue number. +--- PHASE 6: WRITE THE DESCRIPTION --- ---- PHASE 5: WRITE THE FEATURE DESCRIPTION --- +Prose paragraphs. This is the ONLY place where rationale belongs. -Prose paragraphs. This is the ONLY section where rationale belongs. +FEATURE DESCRIPTION: open with the gap — what happens today and where it falls short, naming the concrete thing that does not cover the case. State what this issue adds in one sentence. Break the work into halves with bold run-in headings when it has more than one (server + frontend, two surfaces, markup + asset). Carry over the load-bearing decisions the design doc made, and their reasons — the constraint an acceptance criterion compresses into one line. -Open with the gap: what happens today and where it falls short, naming the concrete thing that does not cover the case. State what this issue adds in one sentence. Break the work into halves with bold run-in headings when it has more than one (server + frontend, two surfaces, markup + asset). Carry over the load-bearing decisions the design doc made, and their reasons — the constraint an acceptance criterion compresses into one line. +BUG DESCRIPTION: short prose, three things in order — what the plugin does today that is wrong, as someone sees it on the real surface (the widget, the page, the event, the API response, the option value), with the wrong value and the value expected instead; what should happen instead, in one sentence; and when it happens — which module is connected, which user role, which date range, which setting value, which screen size, which browser, and whether it happens every time. Add the cause in ONE clause only when you verified it in the code in Phase 4, and never prescribe the fix — that is the Implementation Brief's job. Say whether the same wrong behavior reaches anywhere else served by the same code, and whether this issue covers it. -No bullets of criteria, no implementation instructions, no file trees. +STEPS TO REPRODUCE: a numbered list a tester follows with nothing but the plugin. Start from a state they can reach (a fresh install, a connected module, a setting at a named value) and make that step 1 when the bug needs it. One action per step, with the real thing to click, type or paste — "Set the date range to `Last 28 days`", not "choose a date range". The last step is the observation: what the tester sees and what they should have seen instead. No mechanism — no "check the store", no breakpoints — unless the symptom cannot be seen any other way, in which case give the exact thing to open and the exact value to look at. ---- PHASE 6: WRITE THE ACCEPTANCE CRITERIA --- +No bullets of criteria, no implementation instructions, no file trees, in either type. + +--- PHASE 7: WRITE THE ACCEPTANCE CRITERIA --- A flat list of `*` bullets. Bold run-in group labels only when the issue has genuinely separate halves. One outcome per bullet; nest a second level when one outcome has an enumerable set of cases (the conditions a hook must not fire under, the values a param may take, the forms excluded from a match rule). Inline a table when a criterion enumerates a lookup that prose would obscure — never link out to it. Each bullet states an outcome that can be checked by reading the diff or exercising the feature. Lead with the precondition when the behavior is conditional. Name the real symbol — event name, config key, inline-data global, CSS selector, param names, hook, class. Give the values (`viewContext: 'mainDashboard'`, `dateRange: 'last-28-days'`, `limit: 3`). State cardinality when it matters ("at most once per request", "exactly one of the four states"). Say what must stay unchanged when the change touches shared code, naming the consumers. +ON A BUG REPORT the criteria are the corrected behavior, not the bug: write the outcome the reproduction steps 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. Then write the cases that already work and run through the same code, so the fix cannot trade one symptom for another. The cause and the fix stay out. + CUT from the criteria: - rationale — no "because", "since", "so that", "the same reasoning as X"; - technique — HOW to match, parse or store 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()`" is a technique note. Keep the outcome and its counter-example, drop the note; - negative parentheticals — "(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 "the selector is named X"; - work that is not being done — "no new setting is registered", "no Storybook story is added", "no migration needed". Silence says it; -- out-of-scope and known-limitation lists. +- "Out of scope" and "Known limitations" lists — the list form names work instead of behavior and cannot be graded. + +THE SCOPE BOUNDARY LIVES IN THE CRITERIA, because they are the contract the PR is graded against — not as a clause in the description, which nothing is graded against. Write it as the outcome at the edge: "the button renders on the single product template, and the cart and checkout pages render nothing", not "Out of scope: cart and checkout". A boundary with no observable edge is not one the issue needs. KEEP the negatives that are themselves assertable outcomes: "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". ---- PHASE 7: REPORT WHAT DID NOT GO IN THE ISSUE --- +--- PHASE 8: REPORT WHAT DID NOT GO IN THE ISSUE --- -In your reply to the user — not in the issue — report the 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; and any ordering constraint, when one issue's text refers to another's deliverable by name. +In your reply to the user — not in the issue — report which type you wrote when you had to ask; the 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; for a bug, 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; anything you added beyond the design doc's scope; and any ordering constraint, when one issue's text refers to another's deliverable by name. Begin with Phase 1. """ diff --git a/AGENTS.md b/AGENTS.md index 5c6bdc512ae..129991e2c78 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,17 +4,24 @@ 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 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). Verify every class, -method, hook, handle and path against the code before naming it. Rationale belongs in the -**Feature Description**; the **Acceptance criteria** are checkable outcomes only — no rationale, -no technique notes, no negative parentheticals, no work that isn't being done. 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** those two sections, leave an existing Feature 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.) +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** diff --git a/docs/context/workflow/write-implementation-brief.md b/docs/context/workflow/write-implementation-brief.md index 55d79e46cbd..66d992866a5 100644 --- a/docs/context/workflow/write-implementation-brief.md +++ b/docs/context/workflow/write-implementation-brief.md @@ -224,6 +224,17 @@ Do **not** include: - **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/<commit SHA>/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=<commit SHA> <path>` and adding the `#L<start>-L<end>` 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. @@ -304,4 +315,6 @@ reply to the user — not in the issue: 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/<branch>/` 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 index 198133c1a4d..d470ba982ec 100644 --- a/docs/context/workflow/write-issue.md +++ b/docs/context/workflow/write-issue.md @@ -1,17 +1,49 @@ # Writing an Issue — Playbook -This is the **single source of truth** for writing the **Feature Description** and **Acceptance -criteria** 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. +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. -These are the two *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. +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 source and the mode +## 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: @@ -22,10 +54,11 @@ Source material arrives one of three ways, and only the user can tell you which: There are two modes, and they decide what you are allowed to touch: -- **New issue** — write both the Feature Description and the Acceptance criteria. -- **Existing issue with a Feature Description already written** — write **only** the Acceptance +- **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. + 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 @@ -45,6 +78,9 @@ breakdown: one issue per row, with its title and its point estimate. Do not re-s 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 @@ -58,61 +94,56 @@ confirm, by reading them: 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 -Follow `.github/ISSUE_TEMPLATE/feature_request.md` exactly, including the moderator notice and -the placeholder comments for the sections you are not writing: - -```markdown -# <title> - -## Feature Description - -<prose> +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` -_Do not alter or remove anything below. The following sections will be managed by moderators only._ +Make two changes to the copy, and no others: -## Acceptance criteria +- **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. -* <criterion> +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). -## Implementation Brief +**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. -* [ ] <!-- One or more bullet points for how to technically implement the feature. Make sure to include changes to Storybook and visual regression tests where relevant. --> +**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 ``. -### Test Coverage - -* <!-- One or more bullet points for how to implement automated tests to verify the feature works. --> - -## QA Brief - -* <!-- One or more bullet points for how to test that the feature works as expected. --> - -## Changelog entry - -* <!-- One sentence summarizing the PR, to be used in the changelog. --> -``` - -**Title** — 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 ``. - -Nothing sits between the title and `## Feature Description` — no epic name, no point estimate, -no dependency line. Where the issue depends on another, that dependency is a clause inside the -Feature Description, by real GitHub issue number. +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 either section, 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". +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: @@ -129,10 +160,13 @@ from a nearby issue number, and never leave a placeholder to be filled in later: 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 Feature Description +## 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. -This is the **only** section where rationale belongs. It answers "why does this exist, and what -problem does it solve" for someone who will never read the design doc. +### 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. @@ -144,9 +178,45 @@ problem does it solve" for someone who will never read the design doc. 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 the section. The description is read by people: +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. @@ -166,7 +236,7 @@ English as a first language, and none of them should have to read a sentence twi 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: +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 @@ -188,7 +258,7 @@ Do **not** include: ## 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 feature — the QA engineer +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. @@ -200,10 +270,16 @@ the parsed `URL.hostname` against an allowlist" tells a tester nothing; "clickin `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. Add bold run-in group labels (`**Server**`, `**Frontend**`, - `**Both**`) only when the issue has genuinely separate halves. +- 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. @@ -254,23 +330,31 @@ A criterion states the outcome of implementing the brief, and nothing else. Cut: 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 lists and known-limitation lists.** They describe what isn't built. A scope - boundary belongs in the Feature Description, in a clause, and only when it genuinely prevents - someone building the wrong thing. - -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". +- **"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. @@ -279,18 +363,25 @@ Everything you decided, cut or discovered belongs in your reply to the user — ## Guardrails -- **Two sections only.** Leave Implementation Brief, Test Coverage, QA Brief and Changelog entry - as the template's placeholder comments. +- **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 Feature Description is untouchable.** When asked to add criteria to an issue that - already has one, read it and leave it exactly as it is unless explicitly told otherwise. +- **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 either section, and no indirect issue references** — every +- **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. From 09387f1e4ab98814e5a7c0053ec3d7364d95aa88 Mon Sep 17 00:00:00 2001 From: Eugene Manuilov <eugene.manuilov@fueled.com> Date: Mon, 17 Aug 2026 17:50:50 +0300 Subject: [PATCH 7/8] Remove gemini cli commands. --- .gemini/agents/code-reviewer.md | 67 --------------- .gemini/agents/documenter.md | 62 -------------- .gemini/agents/fixer.md | 53 ------------ .gemini/agents/implementer.md | 48 ----------- .gemini/commands/implement.toml | 137 ------------------------------ .gemini/commands/review-pr.toml | 106 ----------------------- .gemini/commands/write-brief.toml | 91 -------------------- .gemini/commands/write-issue.toml | 100 ---------------------- .gemini/styleguide.md | 85 ------------------ 9 files changed, 749 deletions(-) delete mode 100644 .gemini/agents/code-reviewer.md delete mode 100644 .gemini/agents/documenter.md delete mode 100644 .gemini/agents/fixer.md delete mode 100644 .gemini/agents/implementer.md delete mode 100644 .gemini/commands/implement.toml delete mode 100644 .gemini/commands/review-pr.toml delete mode 100644 .gemini/commands/write-brief.toml delete mode 100644 .gemini/commands/write-issue.toml delete mode 100644 .gemini/styleguide.md 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: - -- #<number> -``` - -Extract the first `#<number>` from the PR body and fetch the issue using the GitHub MCP tools (`issue_read`). -Fallback: `!{gh issue view <number> --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 (#<number>) 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/commands/write-brief.toml b/.gemini/commands/write-brief.toml deleted file mode 100644 index 052403a7f38..00000000000 --- a/.gemini/commands/write-brief.toml +++ /dev/null @@ -1,91 +0,0 @@ -description = "Writes the Implementation Brief and Test Coverage sections of a GitHub issue, verified against the codebase" - -prompt = """ -You are writing the **Implementation Brief** and **Test Coverage** sections for {{args}} — either a GitHub issue number in the google/site-kit-wp repository, or a path the user supplied to a local issue file. If neither was given, ask for one before doing anything else; never go looking for the file yourself. - -The canonical, tool-agnostic procedure is `docs/context/workflow/write-implementation-brief.md`. Read it and follow it exactly; the phases below are the same steps, and the playbook wins on any detail. - ---- CRITICAL INSTRUCTIONS --- - -- Write ONLY the Implementation Brief and Test Coverage sections. Leave Feature Description, Acceptance criteria, QA Brief and Changelog entry untouched. -- DO NOT edit the GitHub issue or post a comment. Produce the text, or update the local issue file you were given, and let the user place it. -- DO NOT name any class, method, hook, handle or path you have not opened and read. -- DO NOT expand scope beyond the acceptance criteria. - ---- PHASE 1: READ THE ISSUE --- - -For a GitHub issue, fetch it using the GitHub MCP tools (`issue_read`). Fallback: `!{gh issue view {{args}} --json title,body,labels}`. For a local path, read the file. - -Extract: -1. **Feature Description** (between `## Feature Description` and the moderator notice) -2. **Acceptance criteria** (between `## Acceptance criteria` and `## Implementation Brief`) - -The acceptance criteria are the contract: everything you write must trace back to one, and nothing may quietly extend them. - -**STOP and ask the user** if the issue is not found, the body is empty, or the acceptance criteria are missing, ambiguous, or contradictory. - ---- PHASE 2: READ THE SOURCE MATERIAL --- - -- The design doc. Use the one the issue links, or the path the user gives you; otherwise ask the user for it, bundling the question with any Phase 1 ask. Many issues have no design doc — if there isn't one, say so and write the brief from the acceptance criteria and the code. Never stall on it, and never invent a path to one. -- The sibling issues in the same epic, which bound where this issue's scope stops and supply the numbers to cross-reference. - ---- PHASE 3: VERIFY AGAINST THE CODE --- - -Open every file the brief will touch. Confirm each class, method, constant, hook, filter, option and script handle exists and is spelled exactly right; confirm the pattern you will tell someone to follow actually looks that way; trace hook ordering to its `do_action()` / `apply_filters()` call site when the brief depends on it; and identify the existing tests, fixtures, snapshots and inline data the change will move. - -Where the code contradicts the design doc, the code wins. - ---- PHASE 4: LOAD THE CONVENTION DOCS --- - -Read only the `docs/context/{js,php}` docs the issue touches, using the scope map in `docs/context/workflow/implement-issue.md` Step 3. - ---- PHASE 5: WRITE THE IMPLEMENTATION BRIEF --- - -Group by path. Every group is headed by the path it changes, with the changes nested under it. Two shapes are in use — pick one and stay consistent within the issue: - -``` -* [ ] In `path/to/File.php`: - * <one change per bullet> -``` - -``` -In `path/to/directory/` (new directory): - -* [ ] <one change per bullet> -``` - -Head a group with 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, new classes, registration, build config, components, SCSS last. Nest a third level when one bullet covers several related behaviors, ending the parent in a colon ("Submit behavior:" → on click / on success / on failure). - -Every bullet is an instruction. Name the real thing wherever the codebase already decides it: classes, methods, hooks, constants, script handles, datapoint names (`GET:ctas`); selectors and actions with their signature (`getCTAs( { organizationID, publicationID } )`); components to reuse (`SpinnerButton`, `ProgressBar`, `Notice`), their props (`helperText`) and CSS class names (`mdc-text-field--error`); inline-data globals and keys; and user-facing strings quoted verbatim. Point at an existing exemplar ("following `publications.js`") instead of describing one. Cross-reference sibling issues inline (`(added in #12950)`, `(see #13005)`). Fence the scope in one line when an adjacent concern could be pulled in. For UI work, reference the design rather than restating measurements, spell out the interaction states (what disables the CTA, pending, success, error), and name the SCSS partial and the index file to import it into. - -Do NOT include: -- rationale, justification, trade-offs, or the benefits of a decision; -- work that is not needed ("no Storybook changes required", "no migration needed"); -- routine commands (lint, build, test, VRT) — `implement-issue.md` owns verification; -- links to local design docs — the brief is read on GitHub, where those paths do not resolve. External links (Figma, a hosted design doc) are fine; -- restated acceptance criteria, a recap of the Feature Description, or background prose. - -DO include, when they apply: the concrete shape of published data (array keys, JSON payload, inline-script global, selector signature); the exact insertion point when ordering matters (hook priority, array position, above/below an early return); and changes to existing files the criteria imply but do not name. - ---- PHASE 6: WRITE THE TEST COVERAGE --- - -Keep it short — a handful of bullets, not a test plan. Two shapes, both in use: - -``` -* Add tests for `<source file>` covering: - * <case> - -* JS tests in `<file>.test.js` covering the `<name>` action and the `<name>` selector. -``` - -One bullet per test file or area, cases nested under it. Name real paths (co-located `*.test.js` / `*.test.tsx` next to the source, `*Test.php` under the mirroring `tests/phpunit/integration/` path). Phrase each case as the behavior or outcome, not the mechanics. Cover every acceptance criterion including its negative cases. **Storybook stories belong here, not in the brief** — "Add a Story for `<Component>`", or list the states when there is more than one ("default, loading, error"). Name any new fixture or fake and where it goes. Call out existing tests the change will break ("Fix any failing tests/snapshots" when the blast radius isn't knowable up front). No commands. - ---- PHASE 7: REPORT WHAT DID NOT GO IN THE BRIEF --- - -In your reply to the user — not in the issue — report: -- 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 fail); -- anything left as an open decision, and the assumption the brief is written under. - -Begin with Phase 1. -""" diff --git a/.gemini/commands/write-issue.toml b/.gemini/commands/write-issue.toml deleted file mode 100644 index 728a88f7273..00000000000 --- a/.gemini/commands/write-issue.toml +++ /dev/null @@ -1,100 +0,0 @@ -description = "Writes the description and Acceptance criteria of a GitHub issue — feature request or bug report — from a design doc, a bug report or requirements, verified against the codebase" - -prompt = """ -You are writing the **description** and the **Acceptance criteria** of a Site Kit issue for {{args}} — a path to a design doc, a bug report, requirements stated in the user's message, or an existing GitHub issue in google/site-kit-wp that needs acceptance criteria. If nothing usable was given, ask for it before doing anything else. - -The canonical, tool-agnostic procedure is `docs/context/workflow/write-issue.md`. Read it and follow it exactly; the phases below are the same steps, and the playbook wins on any detail. - ---- CRITICAL INSTRUCTIONS --- - -- ASK THE USER whether they want a feature request or a bug report whenever their message does not make it clear, and WAIT for the answer before writing anything. The two use different templates and different description sections. -- Write ONLY the description sections for the type and the Acceptance criteria. Leave Implementation Brief, Test Coverage, QA Brief and Changelog entry as the template's placeholder comments. -- If the issue ALREADY has its description written — a Feature Description, or a Bug Description with its Steps to reproduce — write ONLY the Acceptance criteria and leave the description byte-for-byte unchanged, unless the user explicitly asks you to change it. -- DO NOT run `gh issue create` or `gh issue edit`, and DO NOT post a comment, unless the user explicitly asks. Produce the markdown file and let the user place it. -- DO NOT name any class, method, hook, handle or path you have not opened and read. -- DO NOT put links or paths to local design docs or spec files in any section you write — they do not resolve on GitHub. -- DO NOT refer to a sibling issue by its position in the design doc ("issue 5", "the next issue"). That numbering is not GitHub's and will be wrong. Use a real issue number, or the name of the deliverable. - ---- PHASE 1: ESTABLISH THE ISSUE TYPE --- - -There are two templates, and the type decides which 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. - -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. Say which way you read the request when you ask, so they can correct it 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. - ---- PHASE 2: ESTABLISH THE SOURCE AND THE MODE --- - -The source is a design doc (a repo path or a hosted doc), a bug report or requirements written in the user's message, or an existing issue (`!{gh issue view {{args}} --json title,body}`, or a local path the user supplies — never go looking for one). - -Two modes: a NEW issue gets the description sections and the criteria; an EXISTING issue whose description is already written gets ONLY the criteria. - -STOP and ask the user if no source material was given, if the requirements are ambiguous or contradictory, 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. - -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 points. Do not re-slice it. - ---- PHASE 3: READ THE SOURCE MATERIAL END TO END --- - -Read the WHOLE design doc, not just this issue's section — the reasoning behind one issue's decisions routinely sits in Alternatives considered, Quality attributes, Technical debt or Dependencies. Read any sibling spec the design doc names as authoritative; where that spec and the design doc disagree on what is built, the spec wins. Read the sibling issues already written for the epic — they set the level of detail and show where this issue's scope stops. - -For a bug, read the whole report as the user gave it — plugin version, module state, browser, the exact values they saw — plus whatever it names as evidence: a support thread, a linked issue, or the pull request the user says introduced the problem. - ---- PHASE 4: VERIFY AGAINST THE CODE --- - -Open the files the issue will touch. Confirm every class, method, constant, hook, filter, option, script handle and path exists and is spelled exactly right; confirm the base class, registry or filter being extended has the shape the design doc claims; identify who CONSUMES the data the issue changes, which is what makes a "must stay unchanged" criterion real rather than imagined; and confirm that behavior the design doc treats as already existing actually exists. - -For a bug, confirm that the wrong behavior is really what the code does today. Read until you can point at the lines that produce the symptom, and check which cases served by the same code still work. When you cannot find the symptom in the code, say so in Phase 8 instead of writing the issue around it. - -Where the code contradicts the design doc, the code wins. - ---- PHASE 5: LAY THE ISSUE OUT --- - -Copy the template file for the type from Phase 1 — the file IS the layout, so do not reproduce it from memory or reorder its sections. Make two changes and no others: drop the YAML frontmatter (the `name:` and `about:` lines between the `---` markers), and put the title in its place as a single `# ` heading at the top. Everything else stays exactly as the file has it, including the `Do not alter or remove anything below` 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. - -On a bug report, Screenshots and Additional Context 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. Never invent a version number, an operating system or a device, and never describe a screenshot you have not seen. - -The title names the deliverable, not the activity — lead with the symbol or event produced, then the scope in a short clause. For a bug, name the symptom and where it happens, in the same shape. 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. - ---- PHASE 6: WRITE THE DESCRIPTION --- - -Prose paragraphs. This is the ONLY place where rationale belongs. - -FEATURE DESCRIPTION: open with the gap — what happens today and where it falls short, naming the concrete thing that does not cover the case. State what this issue adds in one sentence. Break the work into halves with bold run-in headings when it has more than one (server + frontend, two surfaces, markup + asset). Carry over the load-bearing decisions the design doc made, and their reasons — the constraint an acceptance criterion compresses into one line. - -BUG DESCRIPTION: short prose, three things in order — what the plugin does today that is wrong, as someone sees it on the real surface (the widget, the page, the event, the API response, the option value), with the wrong value and the value expected instead; what should happen instead, in one sentence; and when it happens — which module is connected, which user role, which date range, which setting value, which screen size, which browser, and whether it happens every time. Add the cause in ONE clause only when you verified it in the code in Phase 4, and never prescribe the fix — that is the Implementation Brief's job. Say whether the same wrong behavior reaches anywhere else served by the same code, and whether this issue covers it. - -STEPS TO REPRODUCE: a numbered list a tester follows with nothing but the plugin. Start from a state they can reach (a fresh install, a connected module, a setting at a named value) and make that step 1 when the bug needs it. One action per step, with the real thing to click, type or paste — "Set the date range to `Last 28 days`", not "choose a date range". The last step is the observation: what the tester sees and what they should have seen instead. No mechanism — no "check the store", no breakpoints — unless the symptom cannot be seen any other way, in which case give the exact thing to open and the exact value to look at. - -No bullets of criteria, no implementation instructions, no file trees, in either type. - ---- PHASE 7: WRITE THE ACCEPTANCE CRITERIA --- - -A flat list of `*` bullets. Bold run-in group labels only when the issue has genuinely separate halves. One outcome per bullet; nest a second level when one outcome has an enumerable set of cases (the conditions a hook must not fire under, the values a param may take, the forms excluded from a match rule). Inline a table when a criterion enumerates a lookup that prose would obscure — never link out to it. - -Each bullet states an outcome that can be checked by reading the diff or exercising the feature. Lead with the precondition when the behavior is conditional. Name the real symbol — event name, config key, inline-data global, CSS selector, param names, hook, class. Give the values (`viewContext: 'mainDashboard'`, `dateRange: 'last-28-days'`, `limit: 3`). State cardinality when it matters ("at most once per request", "exactly one of the four states"). Say what must stay unchanged when the change touches shared code, naming the consumers. - -ON A BUG REPORT the criteria are the corrected behavior, not the bug: write the outcome the reproduction steps 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. Then write the cases that already work and run through the same code, so the fix cannot trade one symptom for another. The cause and the fix stay out. - -CUT from the criteria: -- rationale — no "because", "since", "so that", "the same reasoning as X"; -- technique — HOW to match, parse or store 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()`" is a technique note. Keep the outcome and its counter-example, drop the note; -- negative parentheticals — "(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 "the selector is named X"; -- work that is not being done — "no new setting is registered", "no Storybook story is added", "no migration needed". Silence says it; -- "Out of scope" and "Known limitations" lists — the list form names work instead of behavior and cannot be graded. - -THE SCOPE BOUNDARY LIVES IN THE CRITERIA, because they are the contract the PR is graded against — not as a clause in the description, which nothing is graded against. Write it as the outcome at the edge: "the button renders on the single product template, and the cart and checkout pages render nothing", not "Out of scope: cart and checkout". A boundary with no observable edge is not one the issue needs. - -KEEP the negatives that are themselves assertable outcomes: "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". - ---- PHASE 8: REPORT WHAT DID NOT GO IN THE ISSUE --- - -In your reply to the user — not in the issue — report which type you wrote when you had to ask; the 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; for a bug, 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; anything you added beyond the design doc's scope; and any ordering constraint, when one issue's text refers to another's deliverable by name. - -Begin with Phase 1. -""" 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. From 6ed036c67ba14977144928c55c0f644465760048 Mon Sep 17 00:00:00 2001 From: Eugene Manuilov <eugene.manuilov@fueled.com> Date: Mon, 17 Aug 2026 17:51:33 +0300 Subject: [PATCH 8/8] Update enabled plugins for claude code. --- .claude/settings.json | 1 - 1 file changed, 1 deletion(-) diff --git a/.claude/settings.json b/.claude/settings.json index 97e845d5935..6dde0c4a214 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,7 +1,6 @@ { "enabledPlugins": { "figma@claude-plugins-official": true, - "superpowers@claude-plugins-official": true, "playwright@claude-plugins-official": true, "chrome-devtools-mcp@claude-plugins-official": true }