Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/claude-agent/run.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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.

Expand Down
30 changes: 29 additions & 1 deletion .github/workflows/rc-docs-sync.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<!--[[:space:]]*rc-docs-sync:v1[[:space:]]+product=${product}[[:space:]]+rc_tag=${rc_tag}[[:space:]]*-->"; 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 "<!-- rc-docs-sync:v1 product=${product} rc_tag=${rc_tag} -->

**${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."
20 changes: 20 additions & 0 deletions AGENT_MAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down