feat(review): add read-only mode, ask before stopping other engines

plugins/ddev/skills/ci-failure/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: ci-failure
-description: Diagnose a failing or intermittent CI job in a DDEV repository (a git remote under github.com/ddev, a fork of one, or a DDEV add-on) from a GitHub Actions URL, a Buildkite URL, or a downloaded log. Find the first real error, decide whether it is a regression, intermittent, platform-specific, or upstream, and fix the cause. Use only in such a repository or for a run under github.com/ddev, when the user gives a failing run, or says "this test fails", "is it intermittent or not?", "fails periodically", "works locally but fails in CI", "see failed test in [PR], fix it", or "let's fix it once and for all". Not for other repositories.
+description: Diagnose a failing or intermittent CI job in a DDEV repository (a git remote under github.com/ddev, a fork of one, or a DDEV add-on) from a GitHub Actions URL, a Buildkite URL, or a downloaded log. Find the first real error, decide whether it is a regression, intermittent, platform-specific, or upstream, and fix the cause. Use only in such a repository or for a run under github.com/ddev, when the user gives a failing run, or says "this test fails", "is it intermittent?", "works locally but fails in CI", or "fix the failing test in [PR]". Not for other repositories.
 ---
 
 # DDEV CI failure
@@ -95,15 +95,17 @@ ask before running anything that looks wrong.
    single-test commands in `AGENTS.md`. In an add-on, run
    `bats ./tests/test.bats --filter '<test name>'`. Never run the whole
    suite to check one test.
-2. Use the same engine and versions as the failing job. The user's machine
-   usually matches CI, so don't build a replica environment without asking.
-   Other sessions may use Docker at the same time, so take the lock that
-   `/ddev:review` uses before tests that start containers, and release it
-   when they finish. Use the name of the log directory as the label:
+2. Use the same engine and versions as the failing job. If this machine
+   doesn't match it, such as a macOS job on a Linux machine, say so, and
+   ask before you build a replica environment. Other sessions may use
+   Docker at the same time, so take the lock that `/ddev:review` uses
+   before tests that start containers, and release it when they finish.
+   Use the name of the log directory as the label:
    `<skill dir>/../review/review-lock.sh acquire ci-<repo or pipeline>-<run>`
-   and `release` with the same label. With the lock held, run
-   `<skill dir>/../review/poweroff-others.sh <context>`, because other local
-   engines running DDEV take the same ports.
+   and `release` with the same label. Other local engines running DDEV take
+   the same ports. Before the lock, list them with
+   `<skill dir>/../review/poweroff-others.sh <context>`, and ask before you
+   run it again with `--poweroff` under the lock.
 3. Fix the cause once. Search for the same pattern in other tests, and in
    other official add-ons for an add-on, and reuse an existing helper, such
    as one for Docker and Podman differences, before you add one.

plugins/ddev/skills/commit/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: commit
-description: Write, amend, or reword a commit message, a PR title and description, or an issue in a DDEV repository (a git remote under github.com/ddev, a fork of one, or a DDEV add-on), run the repository's checks, and commit locally. Use only in such a repository, when the user says "commit", "make a branch and commit", "amend", "squash into one commit", "rename the commit", "good commit name?", "open a PR", "update the PR body", "update the PR description", "the commit body is too long", or asks to draft an issue. Not for other repositories.
+description: Write, amend, or reword a commit message, a PR title and description, or an issue in a DDEV repository (a git remote under github.com/ddev, a fork of one, or a DDEV add-on), run the repository's checks, and commit locally. Use only in such a repository, when the user says "commit", "make a branch and commit", "amend", "squash into one commit", "reword the commit message", "open a PR", "update the PR description", or asks to draft an issue. Not for other repositories.
 ---
 
 # DDEV commit
@@ -132,8 +132,6 @@ from [ASD-STE100](https://en.wikipedia.org/wiki/Simplified_Technical_English):
 - One connective per sentence, such as so, which, or because. None reads
   choppy, and more read as dense prose.
 
-Then read it aloud, as if to a teammate.
-
 End with the attribution from the organization `AGENTS.md`. The PR
 description is the same text without the `Co-Authored-By` trailer.
 

plugins/ddev/skills/review/SKILL.md

@@ -1,12 +1,12 @@
 ---
 name: review
-description: Review pull requests, local branches, individual commits, and follow-up changes in DDEV repositories, meaning those with a git remote under github.com/ddev, forks of them, and DDEV add-ons. Compare behavior with a baseline build, report evidence and testing gaps, and draft review comments the user posts under their own name, as a pending review only when asked. Use only in such a repository, when the user says "let's review [PR]", "any bugs, problems? are you happy?", "review again", "happy now?", "review the current branch / last commit", or asks what to write as a review comment. Not for other repositories.
+description: Review pull requests, local branches, individual commits, and follow-up changes in DDEV repositories, meaning those with a git remote under github.com/ddev, forks of them, and DDEV add-ons. Compare behavior with a baseline build, report evidence and testing gaps, and draft review comments the user posts under their own name, as a pending review only when asked. Use only in such a repository, when the user says "let's review [PR]", "review again", "review the current branch / last commit", or asks what to write as a review comment. Not for other repositories.
 ---
 
 # DDEV review
 
-Every review answers **"are you happy with it?"** with evidence you collected
-yourself, by running the changed behavior when feasible.
+Every review ends in a verdict backed by evidence you collected yourself, by
+running the changed behavior when feasible.
 
 This skill is only for DDEV repositories: one with a remote under
 `github.com/ddev/`, a fork of one, or a DDEV add-on with an `install.yaml` at
@@ -18,14 +18,17 @@ and review without it.
 ## Modes
 
 - **PR** (number or URL): all the steps below.
-- **Re-review** ("review again", "happy now?", or the user already reviewed
-  it): check that each earlier point was fixed or answered, then review and
-  retest the diff from the last reviewed commit (the review's `commit=`) to
-  the head. If that commit is unknown, say so and review the full diff.
+- **Re-review** ("review again", or the user already reviewed it): check
+  that each earlier point was fixed or answered, then review and retest the
+  diff from the last reviewed commit (the review's `commit=`) to the head.
+  If that commit is unknown, say so and review the full diff.
 - **Local branch or last commit:** use the local build as the candidate.
   Compare a branch with its intended target; compare the last commit with its
   first parent unless the user gives another base. Check that the commit body
   describes the resulting behavior, not the steps that led to it.
+- **Read only** (the user asks only to read the diff, or for a quick look):
+  steps 1 and 3, without builds or Docker. Say in the answer that nothing
+  ran.
 - **Review comments on the user's own PR:** check each comment against the
   current code, fix the ones that hold up, and draft the replies. Resolve
   threads only when asked.
@@ -120,15 +123,18 @@ review back to the user. Take it again if you need Docker later.
 ```
 
 Local engines, such as Docker and Podman, or OrbStack and Docker Desktop,
-compete for the same ports. With the lock held, and before starting ddev on
-an engine, power off DDEV on every other local engine that is running it.
-Run this with the user's own ddev on `PATH`, not a review build, and tell the
-user what it stopped:
+compete for the same ports. Before you take the lock, list the other local
+engines that run DDEV. Run this with the user's own ddev on `PATH`, not a
+review build:
 
 ```bash
 <skill dir>/poweroff-others.sh <context>   # the one you are about to use
 ```
 
+If it lists any, ask the user before you power them off, because one may run
+a project they are working in. Then, with the lock held, run it again with
+`--poweroff` before you start ddev, and tell the user what it stopped.
+
 ### Builds
 
 In ddev/ddev, compare the candidate with a baseline build, and record both
@@ -220,8 +226,9 @@ impact and evidence, and rank it by how many users are likely to hit it.
 
 ## 4. Answer the user
 
-Open with "I'm happy with it", "Not yet", or "I can't conclude yet", plus
-one line why. Use the last when missing verification prevents a verdict. Then:
+Open with a verdict, "Ready to merge", "Not yet", or "Can't conclude yet",
+plus one line why. Use the last when missing verification prevents a verdict.
+Then:
 
 1. **What I checked:** the head and baseline revisions, and evidence for
    each claim: a `file:line` link, or the command run on both builds and how
@@ -250,7 +257,7 @@ it, not like AI output.
 - **Plain:** short sentences. Avoid:
     - headings or bold lead-ins
     - restating the PR or a praise paragraph
-    - vague hedging, and em dashes; state any concrete uncertainty
+    - vague hedging; state any concrete uncertainty
     - the organization's list of words, which the check below reports
 - **Suggest, don't lecture:** "Maybe show only the names?" A small code
   suggestion beats a paragraph.
@@ -265,7 +272,7 @@ it, not like AI output.
   [Claude Code](https://claude.ai/code) or
   [Codex](https://openai.com/codex/) as appropriate.
 
-A draft the user cut from five paragraphs to this:
+For example:
 
 > This prints `web-environment` values, which the full listing hides since
 > #6002 because people keep tokens there (our Platform.sh, Acquia and
@@ -330,6 +337,6 @@ prune.
 - Docker images and containers this review created, unless other projects
   use them
 
-If you're happy with the PR, clean up without asking and list what you
+If the verdict is "Ready to merge", clean up without asking and list what you
 removed. If not, list what's left and ask before deleting anything. Leave the
 user's checked-out branch as it was.

plugins/ddev/skills/review/poweroff-others.sh

@@ -1,11 +1,14 @@
 #!/usr/bin/env bash
-# Power off DDEV on every local Docker context except the one a review is
-# about to use: local engines compete for the same ports.
-# Usage: poweroff-others.sh <context>
-# Run it with the user's own ddev on PATH, not a review build.
+# List, or power off, DDEV on every local Docker context except the one a
+# review is about to use: local engines compete for the same ports.
+# Usage: poweroff-others.sh [--poweroff] <context>
+# Without --poweroff it only lists them. Run it with the user's own ddev on
+# PATH, not a review build.
 set -euo pipefail
 
-keep=${1:?usage: poweroff-others.sh <context>}
+poweroff=
+if [ "${1:-}" = --poweroff ]; then poweroff=1; shift; fi
+keep=${1:?usage: poweroff-others.sh [--poweroff] <context>}
 # DOCKER_HOST overrides DOCKER_CONTEXT, and the review's config isn't the user's.
 unset DOCKER_HOST DDEV_XDG_CONFIG_HOME
 
@@ -23,6 +26,7 @@ docker context ls --format '{{.Name}} {{.DockerEndpoint}}' |
     [ "$name" != "$keep" ] || continue
     [ -z "$keep_id" ] || [ "$(engine_id "$name")" != "$keep_id" ] || continue
     [ -n "$(DOCKER_CONTEXT=$name quick docker ps -q --filter label=com.ddev.platform=ddev 2>/dev/null)" ] || continue
+    if [ -z "$poweroff" ]; then echo "ddev is running on $name"; continue; fi
     echo "powering off ddev on $name"
     DOCKER_CONTEXT=$name ddev poweroff </dev/null
   done