From bf8ea025942cf0fa141000c92d52759bc04185fb Mon Sep 17 00:00:00 2001 From: Enrico Battocchi Date: Tue, 28 Apr 2026 17:02:39 +0200 Subject: [PATCH 1/3] fix(rc-docs-sync): bump max-turns and add marker safety-net MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related fixes after the first real run on 27.6-RC1: 1. Bumped --max-turns from 50 to 100 in claude_args. The agent successfully triaged + authored + opened PR #393 but ran out of turns before posting its run-summary comment. 50 turns is too tight once authoring + git ops + gh pr create + comment posting are all counted; 100 gives realistic headroom and is still capped well below the 30-minute job timeout. 2. Added a new "Ensure marker comment exists (safety net)" step that runs `if: always()` after the agent step. It checks whether a `` marker comment already exists on the tracking issue for this (product, rc_tag) pair; if not, it posts one referencing the workflow run. This guarantees state always advances even if the agent fails for any reason, so the next scheduled run will not silently re-process the same RC and create a duplicate PR — which is exactly what would have happened without the manual marker that was just added to issue #390 by hand. The agent's normal run-summary comment is still preferred when it gets posted; the safety-net step skips when the agent has already posted. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/rc-docs-sync.yml | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rc-docs-sync.yml b/.github/workflows/rc-docs-sync.yml index ef50c403..92c758cb 100644 --- a/.github/workflows/rc-docs-sync.yml +++ b/.github/workflows/rc-docs-sync.yml @@ -353,5 +353,33 @@ jobs: } } claude_args: | - --max-turns 50 + --max-turns 100 --model claude-sonnet-4-6 + + - name: Ensure marker comment exists (safety net) + if: always() && steps.bundle.outputs.any_content == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + run: | + set -euo pipefail + issue='${{ matrix.item.tracking_issue }}' + product='${{ matrix.item.product }}' + rc_tag='${{ matrix.item.rc_tag }}' + display='${{ matrix.item.display_name }}' + + # Skip if the agent already posted its own marker for this (product, rc_tag). + if gh issue view "$issue" --json comments --jq '.comments[].body' \ + | grep -Eq ""; then + echo "Marker for ${product} ${rc_tag} already on issue #${issue}; nothing to do." + exit 0 + fi + + base_version="${rc_tag%-RC*}" + gh issue comment "$issue" --body " + + **${display} ${base_version}** (RC \`${rc_tag}\`) — agent step did not post its own summary; this is a safety-net marker so the next scheduled run does not re-process this RC. + + Workflow run: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + + Inspect the run logs and any PRs labeled \`rc/${rc_tag}\` to see what the agent produced before failing." From 91a51512eb7b8990e03f852537cb9977d09c52bc Mon Sep 17 00:00:00 2001 From: Enrico Battocchi Date: Wed, 29 Apr 2026 10:47:12 +0200 Subject: [PATCH 2/3] Add public-vs-internal surface discrimination to the agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Triggered by feedback on PR #393, where the agent documented two REST endpoints (yoast/v1/ai_content_planner/get_suggestions and get_outline) that may have been intended as internal admin-UI plumbing. The agent had no signal distinguishing public from internal surface — anything that looked like a public registration was fair game. This commit teaches the agent to discriminate, and documents the convention authors can use to override the heuristics. agent prompt (.github/claude-agent/run.md): adds Step 1.6 — Public vs. internal surface — between coverage-gap detection and authoring. The agent now applies, in order: 1. Authoritative override: `@internal` PHPDoc annotation on the registering method or class. If present, skip and log. 2. Permission-callback heuristic: `register_rest_route` whose permission_callback is `current_user_can('manage_options')` (or similar admin capability check) without an unauthenticated branch is likely internal. 3. Path/class-name heuristic: registrations under `**/admin/**`, `**/user-interface/**`, or named `*_Admin_*` / `*_Internal_*` default to internal. 4. When in doubt, don't document — false positives in the public direction are higher cost than false negatives. Items skipped under this rule appear in a new "Internal surface skipped" section of the run-summary comment, so the decision is visible per-RC and a maintainer can override in a follow-up. AGENT_MAP.md: adds a "Public vs. internal surface" section documenting both the heuristics (automatic) and the @internal override (authoritative), so source-repo authors know how to mark a route as internal without reading the agent prompt. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/claude-agent/run.md | 20 ++++++++++++++++++++ AGENT_MAP.md | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/.github/claude-agent/run.md b/.github/claude-agent/run.md index 41b73824..401f13f6 100644 --- a/.github/claude-agent/run.md +++ b/.github/claude-agent/run.md @@ -71,6 +71,25 @@ Emit these in the run summary in Step 4 under a heading **"Coverage gaps observe The gaps are informational; they do NOT block the run. A maintainer reviewing the tracking issue will decide whether to update `AGENT_MAP.md` in a separate PR. +### Step 1.6 — Public vs. internal surface + +Not every newly-added REST route, hook, or class is intended to be a public API. Some are internal admin-UI plumbing that the source plugin reserves the right to change without notice. Documenting an internal route in the developer portal effectively turns it into a public commitment, which is worse than failing to document a real public API (a missed-doc gap is a TODO; a falsely-public-promised internal API is a long-term support burden). + +For each candidate item that would otherwise produce a PR plan, apply this discrimination **before** adding it to a plan: + +1. **Authoritative override — `@internal` annotation.** If the registering call's surrounding PHPDoc, the registering method's docblock, or the registering class's docblock contains an `@internal` tag, treat the item as internal. Do NOT document. Note in the run summary's "Internal surface skipped" section (see Step 4). + +2. **Heuristic — permission callback.** For `register_rest_route(...)`, read the `permission_callback`. If it enforces a logged-in admin capability check (`current_user_can('manage_options')`, `current_user_can('edit_posts')`, `current_user_can('wpseo_manage_options')`, similar) without an unauthenticated branch, treat the route as **likely internal**. Don't document; flag in "Internal surface skipped" with the callback as evidence. + +3. **Heuristic — file-path/class-name signals.** Default-to-internal when the registration lives in any of these: + - File path contains `/admin/`, `/user-interface/`, `*-admin-*`, `*-internal-*`. + - Class name contains `Admin_`, `Internal_`, ends with `_Admin_Route` / `_UI_Route`. + - Registration is from a class that extends a known internal base (e.g. `Yoast\WP\SEO\Admin\...`). + +4. **When in doubt, don't document.** False positives in the public-API direction are higher cost than false negatives. If the signals are mixed (e.g. neutral path but no `@internal` annotation and a `current_user_can` callback), prefer to skip and flag, rather than confidently document. The maintainer can ask the source-repo team and reverse the decision in a follow-up. + +Items skipped under this rule must be listed in the run summary under a heading **"Internal surface skipped"**, with one bullet per item: source path, symbol/route, and which signal fired (`@internal`, permission-callback heuristic, path/class heuristic, or "mixed signals"). Omit the heading entirely if no items were skipped. + ### Step 2 — Authoring (only if PR plans exist) For each PR plan, in order: @@ -118,6 +137,7 @@ The body after the marker should contain: - One bullet per PR plan: area, title, PR link. - If zero PRs: a one-paragraph explanation of what the RC contained and why no doc changes are needed (cite the changelog entry and top-level diff areas). - A **"Coverage gaps observed"** section iff you flagged any in Step 1.5. Omit the heading entirely when there are none. +- An **"Internal surface skipped"** section iff you skipped any items in Step 1.6. Omit the heading entirely when there are none. **If you fail to post the comment with the marker, the next scheduled run will re-process this RC.** Posting the marker is the acknowledgement of completion. diff --git a/AGENT_MAP.md b/AGENT_MAP.md index 7d8f1b40..3a4440d8 100644 --- a/AGENT_MAP.md +++ b/AGENT_MAP.md @@ -296,6 +296,26 @@ These docs are only touched by the sync agent if the RC diff *explicitly* change - `docs/development/**` — contributor guides, tooling, standards. Changes to Yoast's internal development practices, not to the plugin's public surface. - `docs/overview.md` when the change is stylistic — only update on genuine information changes. +## Public vs. internal surface + +Not every new REST route, hook, or class in a plugin RC is intended to be public API. Documenting an internal admin-UI route in the developer portal effectively turns it into a long-term public commitment, which is worse than missing a real public API (a missed doc is a TODO; a falsely-public internal API is a support burden). + +The agent's prompt (`.github/claude-agent/run.md`, Step 1.6) implements this discrimination. Source-repo authors can mark a route or class as internal in either of two ways, in increasing order of authority: + +1. **Path/naming heuristics** are picked up automatically. Files under `*-admin-*`, `**/admin/**`, or `**/user-interface/**`, classes named `*_Admin_*` or `*_Internal_*`, and `register_rest_route` calls whose `permission_callback` enforces an admin capability check (`current_user_can('manage_options')` etc.) are treated as internal by default. + +2. **`@internal` PHPDoc annotation** is the unambiguous override. Add it to the registering method's or class's docblock: + ```php + /** + * @internal Used by the Yoast SEO admin UI; not part of the public REST API. + */ + ``` + The agent treats `@internal` as authoritative — don't document, regardless of what the heuristics say. + +When the agent skips an item under this rule, it lists the item (and which signal fired) in an **"Internal surface skipped"** section in the run-summary comment on the tracking issue. That gives a maintainer a clear audit trail: nothing is silently dropped; the decision is visible per RC. + +If the heuristics misfire and a route is *intentionally public* but matches an internal-looking path or callback, add `@public` to the docblock or move the registering code to a non-`/admin/`, non-`/user-interface/` path. (Adding `@public` is currently advisory — the agent treats absence of `@internal` as the public signal — but the convention may become authoritative if false-negatives become a recurring issue.) + ## Cross-product ripple rules When a public filter or class moves between products (e.g., promoted from Premium to Free, or vice versa), the agent must: From 4b9d1e6260e1d503b4a8ff948f662a237323e9ec Mon Sep 17 00:00:00 2001 From: Enrico Battocchi Date: Wed, 29 Apr 2026 10:55:42 +0200 Subject: [PATCH 3/3] Replace 'iff' with 'if' in run.md (less jargon-y, same meaning) --- .github/claude-agent/run.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/claude-agent/run.md b/.github/claude-agent/run.md index 401f13f6..ffa0e819 100644 --- a/.github/claude-agent/run.md +++ b/.github/claude-agent/run.md @@ -136,8 +136,8 @@ The body after the marker should contain: - Symbol index size, count of new symbols observed in diff. - One bullet per PR plan: area, title, PR link. - If zero PRs: a one-paragraph explanation of what the RC contained and why no doc changes are needed (cite the changelog entry and top-level diff areas). -- A **"Coverage gaps observed"** section iff you flagged any in Step 1.5. Omit the heading entirely when there are none. -- An **"Internal surface skipped"** section iff you skipped any items in Step 1.6. Omit the heading entirely when there are none. +- A **"Coverage gaps observed"** section if you flagged any in Step 1.5. Omit the heading entirely when there are none. +- An **"Internal surface skipped"** section if you skipped any items in Step 1.6. Omit the heading entirely when there are none. **If you fail to post the comment with the marker, the next scheduled run will re-process this RC.** Posting the marker is the acknowledgement of completion.