diff --git a/.github/claude-agent/run.md b/.github/claude-agent/run.md index 41b73824..ffa0e819 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: @@ -117,7 +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. +- 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. 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." 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: