docs: request reviewers for real and lead ambient PRs with a feature summary

.agents/skills/create_pr/SKILL.md

@@ -109,10 +109,42 @@ Include the trailing slash on `destination` and the `statusCode`, matching the e
 
 ## PR Description Guidelines
 
-Structure your PR description with these sections:
+Structure your PR description with these sections, in this order. The feature summary comes first; everything else follows it.
+
+### What this feature does (required on drafting PRs)
+
+Open the body with a plain-language summary of what the feature does **for the user**. This is the first thing a reviewing engineer reads, so it must not be pipeline bookkeeping — which spec produced the draft, which workflow generated it, and which run it came from all belong further down. A reviewer who only reads this section should be able to tell whether the docs describe the right thing.
+
+End the summary with the shipped-in fact, not a forecast. Read the version and date from the release accessor the drift-watch gate already uses, rather than adding a second way to look up a release:
+
+```bash
+# Exits 10 when the current stable release was already processed, which is not an
+# error for this purpose — we only want the version and date it reports.
+python3 .agents/skills/missing_docs/scripts/check_new_release.py --json > /tmp/release.json || true
+python3 -c "import json; d=json.load(open('/tmp/release.json')); print(d['current_version'], d['release_date'])"
+```
+
+Write "shipped in `<version>` (`<date>`)". Do not write a target or predicted ship date: there is no trustworthy source for one, and a forecast in a merged PR body ages into a false claim.
+
+**Length budget: 75 words maximum**, ideally two to four sentences. Drafts are already too wordy; a summary that runs longer than a short paragraph has stopped being a summary. `check_pr_body.py` enforces the budget, the heading text, and the position.
+
+```markdown
+## What this feature does
+
+Workspace admin roles let a workspace owner delegate whole-workspace management — membership, billing, and cloud agent run visibility — to an admin without handing over ownership. Shipped in `v0.2026.08.18.02.52.stable_00` (`2026-08-18`).
+```
+
+Verify it before submitting, along with the other body checks:
+
+```bash
+python3 .agents/skills/create_pr/check_pr_body.py /tmp/pr-body.md \
+  --require-lead-section "## What this feature does"
+```
+
+The check fails if the section is missing, is not the first content in the body, is empty, or exceeds the word budget. Position is checked against content rather than headings, so a body cannot open with a few unheaded lines of spec/workflow/run-ID preamble and still pass. Omit the section — and the flag — only for the small corrections listed under "When a plan can be skipped": typos, link fixes, terminology sweeps, generated updates, and screenshot swaps have no feature to summarize.
 
 ### Summary
-Brief explanation of what the PR accomplishes and why.
+Brief explanation of what the PR accomplishes and why. This is where the pipeline detail goes: the source spec, the generating workflow, the new page path, and the sidebar entry.
 
 ### Changes
 Bulleted list of specific changes, organized by file or area:
@@ -203,8 +235,15 @@ Exit code 0 if PR exists, 1 if not.
 :::
 
 ```bash
-# 1. Write the description to a temp file using the create_file tool or a heredoc
+# 1. Write the description to a temp file using the create_file tool or a heredoc.
+#    The `## What this feature does` block is DRAFTING-PR ONLY - drop it (and the
+#    --require-lead-section flag in step 2) for typos, link fixes, terminology
+#    sweeps, generated updates, and screenshot swaps.
 cat > /tmp/pr-body.md << 'EOF'
+## What this feature does
+One short paragraph: what the feature does for the user, ending with
+shipped in `<version>` (`<date>`).
+
 ## Summary
 Description of changes
 
@@ -215,16 +254,97 @@ Description of changes
 Co-Authored-By: Oz <oz-agent@warp.dev>
 EOF
 
-# 2. Verify the body for corruption before submitting (exits non-zero on failure)
-python3 .agents/skills/create_pr/check_pr_body.py /tmp/pr-body.md
+# 2. Verify the body for corruption before submitting (exits non-zero on failure).
+python3 .agents/skills/create_pr/check_pr_body.py /tmp/pr-body.md \
+  --require-lead-section "## What this feature does"   # drafting PRs only
+
+# For a non-drafting correction, run the check without the flag:
+# python3 .agents/skills/create_pr/check_pr_body.py /tmp/pr-body.md
 
 # 3. Create the PR using the file (only if the check passed)
 gh pr create --title "docs: Add feature documentation" --body-file /tmp/pr-body.md
 
+# 4. REQUIRED: request the reviewer for real (see "Request reviewers" below).
+#    The PR is not complete until this has succeeded.
+
 # Open in browser to fill details
 gh pr create --web
 ```
 
+### Request reviewers (required)
+
+**Naming a reviewer in the body is not a review request.** A `/cc @engineer` mention notifies nobody through GitHub's review queue: the PR shows no requested reviewer, never appears in that engineer's "Review requested" filter, and quietly goes unreviewed. Every one of the four ambient-drafted docs PRs — #414, #415, #416, #417 — named reviewers in prose and received zero reviews; three had an empty requested-reviewers list and the fourth had a single reviewer added by hand.
+
+So the mention stays, and a real request is added alongside it. **A PR is not complete until `gh pr edit --add-reviewer` has succeeded and been verified.**
+
+A resolution failure must fall back, never no-op. When no owner resolves, assign `dannyneira`, matching the fallback the release docs workflow already uses (`.github/workflows/release-docs-update.yml`, "Assign last docs PR reviewer"). An unassignable reviewer is a problem to surface, not a reason to ship an unreviewed PR.
+
+Two details below are load-bearing, and getting either wrong reintroduces the silent drop this section exists to prevent:
+
+- **Request one reviewer per call.** `gh pr edit --add-reviewer a,b,c` sends a single atomic mutation, so one unassignable entry rejects the whole list. Since a resolution routinely mixes users with a team, and a team with no access to this repo cannot be requested here, a comma-joined call can fail wholesale and take every valid owner down with it.
+- **Verify against the resolved set, not against emptiness.** "Is the list non-empty?" passes when the real owner was dropped and only the fallback landed, which looks identical to success.
+
+```bash
+PR=123
+FALLBACK_REVIEWER=dannyneira
+
+# 1. Resolve the owning engineer(s). For missing_docs drift-watch runs, use the
+#    ownership resolver with the source files behind the change; see the
+#    missing_docs skill's "Reviewer routing" section for how to pick those files.
+#    Diagnostics go to stderr, so this captures only the reviewer list.
+REVIEWERS=$(python3 .agents/skills/missing_docs/scripts/suggest_reviewers.py \
+  --reviewers-only --warp ../warp --warp-server ../warp-server \
+  warp:app/src/settings/ssh.rs < /dev/null)
+
+# 2. Never let an empty resolution drop the request.
+if [[ -z "$REVIEWERS" ]]; then
+  echo "warning: no owner resolved - falling back to $FALLBACK_REVIEWER"
+  REVIEWERS="$FALLBACK_REVIEWER"
+fi
+
+# 3. Request each reviewer separately so one bad entry cannot drop the rest.
+IFS=',' read -ra WANT <<< "$REVIEWERS"
+GOT=()
+for R in "${WANT[@]}"; do
+  if gh pr edit "$PR" --repo warpdotdev/docs --add-reviewer "$R"; then
+    GOT+=("$R")
+  else
+    echo "warning: could not request $R on PR $PR"
+  fi
+done
+
+# 4. If nothing at all landed, fall back rather than ship an unreviewed PR.
+if (( ${#GOT[@]} == 0 )); then
+  gh pr edit "$PR" --repo warpdotdev/docs --add-reviewer "$FALLBACK_REVIEWER" &&
+    GOT+=("$FALLBACK_REVIEWER")
+fi
+
+# 5. Read back and compare against what was resolved. `gh` can exit 0 while
+#    skipping a reviewer, so the PR is the source of truth. Note the jq: teams
+#    have no .login, and `[.reviewRequests[].login // .reviewRequests[].name]`
+#    silently drops them from a mixed list.
+REQUESTED=$(gh pr view "$PR" --repo warpdotdev/docs \
+  --json reviewRequests --jq '[.reviewRequests[] | .login // .slug // .name] | join(",")')
+if (( ${#GOT[@]} < ${#WANT[@]} )); then
+  echo "warning: requested ${#GOT[@]}/${#WANT[@]} resolved reviewers on PR $PR"
+fi
+if [[ -z "$REQUESTED" ]]; then
+  echo "ERROR: no reviewer requested on PR $PR"
+  exit 1
+fi
+echo "Requested reviewers: $REQUESTED"
+```
+
+A partial result is a reportable outcome, not a pass: if some owners could not be requested, say which ones and why in the run output, so the gap is visible rather than buried. If even the fallback cannot be assigned, report the run as failed. Do not close out a PR whose requested-reviewers list is empty.
+
+:::caution
+A team handle resolved from `STAKEHOLDERS` or `CODEOWNERS` can only be requested on a repo that team has access to. `warpdotdev/oss-maintainers` is the root-rule owner in the warp client repo and therefore appears in most resolutions, but it has no access to `warpdotdev/docs`, so requesting it here fails. That is why step 3 requests one at a time.
+:::
+
+:::note
+Auto-requesting the review does not make it *block* merge. Whether an ambient docs PR should require that approval through branch protection is an open question for the docs owner, not something this skill decides.
+:::
+
 ### Update an existing PR
 
 When updating the body of an existing PR, make the **smallest** change rather than regenerating the whole description from memory — re-emitting a long body is what invites repetition-loop degeneration. Fetch the current body, apply a minimal or additive edit, verify it, then submit.
@@ -245,8 +365,12 @@ gh pr edit 123 --body-file /tmp/pr-body.md
 # Edit title only
 gh pr edit 123 --title "New title"
 
-# Add reviewers or labels
-gh pr edit 123 --add-reviewer username --add-label documentation
+# Add labels
+gh pr edit 123 --add-label documentation
+
+# Add reviewers - see "Request reviewers (required)" above; this is mandatory on a
+# new PR, not an optional extra.
+gh pr edit 123 --add-reviewer username
 ```
 
 ### View PR status
@@ -266,10 +390,11 @@ Co-Authored-By: Oz <oz-agent@warp.dev>
 
 ## After Opening the PR
 
-1. **Monitor for merge conflicts** - If main is updated, merge it into your branch
-2. **Respond to review comments** - Address feedback promptly
-3. **Re-run checks after changes** - Run `trunk check` and link checker after making updates
-4. **Verify Astro Starlight preview** - Astro Starlight automatically generates a preview for PRs; check that rendering looks correct
+1. **Confirm the review request landed** - Re-read `reviewRequests` on the PR. An empty list means the PR is not finished, whatever the body says. See "Request reviewers (required)".
+2. **Monitor for merge conflicts** - If main is updated, merge it into your branch
+3. **Respond to review comments** - Address feedback promptly
+4. **Re-run checks after changes** - Run `trunk check` and link checker after making updates
+5. **Verify Astro Starlight preview** - Astro Starlight automatically generates a preview for PRs; check that rendering looks correct
 
 ## Best Practices
 

.agents/skills/create_pr/check_pr_body.py

@@ -23,13 +23,22 @@
   * Duplicate heading  - the same Markdown heading text appearing more than once.
   * Required heading    - (optional) assert specific headings are present exactly
                          once, for skills that emit a fixed body template.
+  * Lead section        - (optional) assert a heading is the FIRST content in the
+                         body, has prose under it, and stays within a word budget.
+                         Drafting PRs must open with a plain-language summary of
+                         what the feature does for the user, so a reviewer learns
+                         that before any pipeline bookkeeping. Position is checked
+                         against content, not just headings, so a body cannot open
+                         with unheaded spec/workflow/run-ID preamble.
 
 Usage:
     python3 check_pr_body.py /tmp/pr-body.md
     cat /tmp/pr-body.md | python3 check_pr_body.py -
     python3 check_pr_body.py /tmp/pr-body.md \
         --require-heading "## Patterns addressed" \
         --require-heading "## Improvement targets"
+    python3 check_pr_body.py /tmp/pr-body.md \
+        --require-lead-section "## What this feature does"
 
 Exit codes:
     0  no issues found
@@ -52,6 +61,11 @@
 LONG_WINDOW = 80
 LONG_MIN_COUNT = 2
 
+# Word budget for the lead section. "Drafts are too wordy" is a standing complaint,
+# and a summary that runs past a short paragraph stops being a summary. Two to four
+# sentences fit comfortably under this cap.
+LEAD_SECTION_MAX_WORDS = 75
+
 
 def _strip_urls(text: str) -> str:
     """Remove URLs so repeated link targets don't cause false positives.
@@ -83,8 +97,14 @@ def find_repeated_span(text: str) -> Optional[Tuple[str, int]]:
 
 
 def _iter_non_code_lines(lines: List[str]):
-    """Yield (line_num, text) for lines outside fenced code blocks."""
+    """Yield (line_num, text) for lines outside fenced code blocks and HTML comments.
+
+    HTML comments are skipped for the same reason code fences are: a `##` line or a
+    stray backtick inside `<!-- ... -->` is commentary, not real body content. PR
+    bodies carry machine-managed comment banners, so this is a live case.
+    """
     fence: Optional[str] = None
+    in_comment = False
     for line_num, line in enumerate(lines, start=1):
         fence_match = re.match(r"^\s*(`{3,}|~{3,})", line)
         if fence is not None:
@@ -94,7 +114,55 @@ def _iter_non_code_lines(lines: List[str]):
         if fence_match:
             fence = fence_match.group(1)
             continue
-        yield line_num, line
+
+        visible, in_comment = _strip_html_comments(line, in_comment)
+        if not visible.strip():
+            # Either a genuinely blank line or a line that was entirely comment.
+            # Yield blanks so callers still see the line, but drop comment-only lines.
+            if in_comment or visible != line:
+                continue
+        yield line_num, visible
+
+
+def _strip_html_comments(line: str, in_comment: bool) -> Tuple[str, bool]:
+    """Remove HTML-comment spans from one line. Returns (visible_text, still_open)."""
+    out = []
+    rest = line
+    while rest:
+        if in_comment:
+            end = rest.find("-->")
+            if end == -1:
+                rest = ""
+                break
+            rest = rest[end + 3 :]
+            in_comment = False
+        else:
+            start = rest.find("<!--")
+            if start == -1:
+                out.append(rest)
+                break
+            out.append(rest[:start])
+            rest = rest[start + 4 :]
+            in_comment = True
+    return "".join(out), in_comment
+
+
+def _content_lines_before(lines: List[str], stop_line: int) -> List[Tuple[int, str]]:
+    """Return (line_num, text) for real content appearing before `stop_line`.
+
+    Blank lines and HTML comments do not count as content; anything else does,
+    including a fenced code block, which is exactly the kind of thing that must not
+    push the feature summary below the fold.
+    """
+    found: List[Tuple[int, str]] = []
+    in_comment = False
+    for line_num, line in enumerate(lines, start=1):
+        if line_num >= stop_line:
+            break
+        visible, in_comment = _strip_html_comments(line, in_comment)
+        if visible.strip():
+            found.append((line_num, visible.strip()))
+    return found
 
 
 def find_unbalanced_backticks(lines: List[str]) -> List[Tuple[int, str]]:
@@ -133,6 +201,67 @@ def check_required_headings(lines: List[str], required: List[str]) -> List[str]:
     return problems
 
 
+def check_lead_section(lines: List[str], heading: str) -> List[str]:
+    """Return messages if the lead section is missing, misplaced, empty, or too long.
+
+    The lead section is the plain-language answer to "what does this feature do for
+    the user?", and it only does that job if the reviewer hits it first. Ambient
+    drafts previously opened with pipeline bookkeeping (which spec, which workflow,
+    which run), so the check asserts position as well as presence.
+    """
+    wanted = heading.strip()
+    problems: List[str] = []
+
+    headings: List[Tuple[int, str]] = []
+    for line_num, line in _iter_non_code_lines(lines):
+        if re.match(r"^#{1,6}\s+\S", line):
+            headings.append((line_num, line.strip()))
+
+    matches = [ln for ln, text in headings if text == wanted]
+    if not matches:
+        return [f"missing required lead section: {wanted!r} (must be the first content in the body)"]
+    if len(matches) > 1:
+        problems.append(
+            f"lead section appears {len(matches)}x (expected once): {wanted!r}"
+        )
+
+    # Require the summary to be the first *content*, not merely the first heading.
+    # A body can open with several lines of unheaded bookkeeping -- the spec name,
+    # the generating workflow, the run ID -- and still have the summary as its first
+    # heading. That is the exact shape the lead section exists to prevent.
+    preceding = _content_lines_before(lines, matches[0])
+    if preceding:
+        line_num, text = preceding[0]
+        preview = text if len(text) <= 60 else text[:57] + "..."
+        kind = "heading" if re.match(r"^#{1,6}\s+\S", text) else "text"
+        problems.append(
+            f"lead section is not first: {kind} {preview!r} (line {line_num}) precedes "
+            f"{wanted!r} (line {matches[0]}); {len(preceding)} line(s) of content come "
+            "before it. The reader must get the feature summary before anything else."
+        )
+
+    # Collect the prose between the lead heading and the next heading.
+    start = matches[0]
+    body_words: List[str] = []
+    for line_num, line in _iter_non_code_lines(lines):
+        if line_num <= start:
+            continue
+        if re.match(r"^#{1,6}\s+\S", line):
+            break
+        body_words.extend(line.split())
+
+    if not body_words:
+        problems.append(f"lead section {wanted!r} has no content under it")
+    elif len(body_words) > LEAD_SECTION_MAX_WORDS:
+        problems.append(
+            f"lead section {wanted!r} is {len(body_words)} words "
+            f"(budget: {LEAD_SECTION_MAX_WORDS}). Cut it to a short paragraph: what the "
+            "feature does for the user, plus the shipped-in version and date."
+        )
+
+    return problems
+
+
 def main(argv: Optional[List[str]] = None) -> int:
     parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
     parser.add_argument("body", help="path to the PR body file, or '-' for stdin")
@@ -143,6 +272,14 @@ def main(argv: Optional[List[str]] = None) -> int:
         metavar="HEADING",
         help="assert this exact heading line is present exactly once (repeatable)",
     )
+    parser.add_argument(
+        "--require-lead-section",
+        metavar="HEADING",
+        help=(
+            "assert this exact heading is the FIRST content in the body, appears once, "
+            f"and carries 1-{LEAD_SECTION_MAX_WORDS} words of prose"
+        ),
+    )
     args = parser.parse_args(argv)
 
     if args.body == "-":
@@ -176,6 +313,9 @@ def main(argv: Optional[List[str]] = None) -> int:
 
     issues.extend(check_required_headings(lines, args.require_heading))
 
+    if args.require_lead_section:
+        issues.extend(check_lead_section(lines, args.require_lead_section))
+
     if issues:
         print("PR body integrity check FAILED:\n", file=sys.stderr)
         for issue in issues: