[gjkim42]

What type of PR is this?

/kind api

What this PR does / why we need it:

Add requestedReviewer to v1alpha2 GitHub webhook filters so TaskSpawner and SessionSpawner can select review requests for a particular user before creating work. It matches requested_reviewer.login exactly and case-sensitively. On pull_request and pull_request_target events, set action: review_requested to match only new review requests; review_request_removed also carries the reviewer. Events without a requested reviewer, including team review requests, never match.

The criterion supports both accepting filters and excludeFilters rules that omit event. Includes generated CRDs, reference documentation, a usage example, and tests for matching, validation, and conversion.

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

The field is added only to v1alpha2. Exclusion rules preserve it through a v1alpha1 round-trip; accepting filters follow the existing pullRequestAuthor behavior and do not preserve it through that round-trip.

Validation passed:

• make verify
• env -u CODEX_HOME -u CODEX_AUTH_JSON make test (unset inherited variables that interfere with unrelated entrypoint tests)
• make test-integration TEST_FLAGS='./test/integration/... -ginkgo.focus=githubWebhook.excludeFilters'

Does this PR introduce a user-facing change?

GitHub webhook filters in v1alpha2 TaskSpawner and SessionSpawner now support requestedReviewer to match an individual requested reviewer's login exactly and case-sensitively. The field also supports exclusion rules without an event scope; team review requests and events without a requested reviewer do not match.

---

Summary by cubic

Adds requestedReviewer to v1alpha2 GitHub webhook filters so TaskSpawner and SessionSpawner can spawn work only when a review is requested from a specific user. The criterion matches requested_reviewer.login exactly and case-sensitively and works on pull_request and pull_request_target events with actions review_requested and review_request_removed; events without a requested reviewer, including team review requests, never match. It works in accepting filters and in excludeFilters rules that omit event.

Notes

• Adds the field only to v1alpha2, with matching, validation, and conversion tests.
• excludeFilters preserve the field through a v1alpha1 round-trip; accepting filters drop it, matching the existing pullRequestAuthor behavior.
• Updates generated CRDs, reference docs, and the integration guide with a usage example.

^{Written for commit aa07ed9. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 13 files

_{View guided diff | Turn on auto-fix | Re-trigger cubic}

[gjkim42]

/kelos api-review

[kelos-bot]

🤖 Kelos Task Status

Task kelos-api-reviewer-issue-comment-62360c7fc41c has succeeded. ✅

[kelos-bot]

🤖 Kelos API Reviewer Agent @gjkim42

API Design Review

Verdict: APPROVE
Scope: Adds an optional requestedReviewer string criterion (MaxLength 64) to v1alpha2 GitHubWebhookFilter, usable in both filters[] and unscoped excludeFilters[] for TaskSpawner and SessionSpawner. The change is additive and v1alpha2-only, and the generated CRDs and docs are updated. Reviewed at 1b90597.

Findings

Documentation contract (P2, non-blocking)

• api/v1alpha2/taskspawner_types.go:557-559, docs/reference.md:1189: The godoc and the reference row don't say which events or actions carry requested_reviewer. GitHub sets it on pull_request and pull_request_target deliveries for both review_requested and review_request_removed, and internal/webhook/github_filter_test.go:3013 ("criterion also matches a removed review request") confirms this behavior is intended. So an accepting filter {event: pull_request, requestedReviewer: alice} without action also starts work when alice's review request is removed. docs/integration.md shows the safe pairing with action: review_requested, but kubectl explain and the reference table don't. Suggested godoc addition: "GitHub sets it on pull_request and pull_request_target events with action review_requested or review_request_removed; set Action to review_requested to react only to new requests." Then run make update and mirror the sentence in docs/reference.md.

Compatibility (P3, follow-up)

• internal/conversion/taskspawner_test.go:1196: A v1alpha1 read-modify-write drops requestedReviewer from accepting filters. This widens the filter: {action: review_requested, requestedReviewer: alice} becomes "every review request". The PR follows the existing pullRequestAuthor / conclusion / checkName precedent, documents the behavior, and in-tree clients use v1alpha2, so this isn't blocking here. As more v1alpha2-only accepting criteria are added, consider a follow-up that preserves them through the round-trip, the same way excludeFilters already are, since losing an accepting criterion widens the match just as losing an exclusion does.

Naming / docs nit (P3)

• docs/integration.md:159: The example uses a personal-looking handle (gunju-kim). A neutral placeholder such as octocat or my-reviewer would be better, so readers don't copy a real account into their manifests.

Strengths

• The name mirrors GitHub's requested_reviewer payload field exactly, so a future requestedTeam (for requested_team) can be added without renaming. Team requests are correctly documented as never matching.
• A scalar, exact, case-sensitive match is consistent with author and pullRequestAuthor. Multiple reviewers can be expressed with OR across filter entries, so a list type isn't needed.
• The "safe unscoped" classification is correct. The check sits outside the per-event switch (internal/webhook/github_filter.go:478), and a missing value never matches, so an unscoped exclusion rule can't reject unrelated events.
• The MaxLength=64 bound is mirrored in githubwebhook_criteria.go, pinned by TestMirroredBoundsMatchMarkers, and re-checked on the excludeFilters restore path. The "at least one criterion" CEL rule includes the new field.
• The field is added only to the latest version (v1alpha2). Generated CRDs for TaskSpawner and SessionSpawner (chart and install-crd.yaml) are included, and no existing field or validation is tightened, so existing manifests still apply.

Suggestions (optional)

• If you adopt the P2 wording, consider listing the events in the integration table too: pull_request_target inherits requestedReviewer via "Same as pull_request", but saying it explicitly would help readers scanning for the field.

/kelos needs-input

[cubic-dev-ai]

1 issue found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/manifests/install-crd.yaml">

<violation number="1" location="internal/manifests/install-crd.yaml:15539">
P3: The CRD names this as `Action`, but the serialized field users must configure is `action`; use the lowercase API key here to avoid suggesting an invalid manifest field.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The CRD names this as Action, but the serialized field users must configure is action; use the lowercase API key here to avoid suggesting an invalid manifest field.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At internal/manifests/install-crd.yaml, line 15539:

<comment>The CRD names this as `Action`, but the serialized field users must configure is `action`; use the lowercase API key here to avoid suggesting an invalid manifest field.</comment>

<file context>
@@ -15535,6 +15535,9 @@ spec:
                                 payload. Matching is exact and case-sensitive. Events without a requested
                                 reviewer, including team review requests, never match this criterion.
+                                Available on pull_request and pull_request_target events with action
+                                review_requested or review_request_removed. Set Action to review_requested
+                                to match only new review requests.
                               maxLength: 64
</file context>

[gjkim42]

/kelos review

[kelos-bot]

🤖 Kelos Task Status

Task kelos-reviewer-issue-comment-c7becec38a31 has succeeded. ✅

[kelos-bot]

🤖 Kelos Reviewer Agent @gjkim42

Review Summary

Verdict: APPROVE
Overall correctness: patch is correct
Scope: Adds an optional requestedReviewer criterion to v1alpha2 GitHubWebhookFilter (TaskSpawner and SessionSpawner). It matches requested_reviewer.login exactly and works in both filters and unscoped excludeFilters. Generated CRDs, docs, and tests are included. Reviewed at 2bbeb47.

Findings Overview

Priority	Count	File:Line	Summary
P0	0	—	none
P1	0	—	none
P2	0	—	none
P3	0	—	none

Findings

No qualifying issues found.

What I checked:

• Parsing (internal/webhook/github_filter.go:166, :313-315): pull_request reads the reviewer through PullRequestEvent.GetRequestedReviewer().GetLogin(), which is nil-safe. pull_request_target has no typed case arm, so it falls through to the raw-JSON branch, which pulls requested_reviewer.login with checked type assertions. A null reviewer, a missing login, or a team-only request leaves the field empty, so the filter cannot match.
• Matching (internal/webhook/github_filter.go:478): The check sits outside the per-event switch and an empty event value never equals a non-empty filter value. That makes requestedReviewer safe in an excludeFilters rule without event, so it belongs in gitHubWebhookExcludeFilterSafeUnscopedCriteria. TestMatchesCriteria_TypeSpecificCriteriaSkippedOutsideTheirArm checks this against a push event.
• Validation and conversion: The "at least one criterion" CEL rule includes the new field. MaxLength=64 is copied into GitHubWebhookFilterRequestedReviewerMaxLength, checked against the kubebuilder marker by TestMirroredBoundsMatchMarkers, and re-checked in validGitHubWebhookExcludeFilter on the annotation restore path. criteriaWithValues and the coverage test both use reflection, so they pick up the field automatically.
• Tests: The table test runs every payload in both filters and excludeFilters mode. It covers the happy path, a sender or PR author that differs from the reviewer, case sensitivity, a near-miss login, team requests, null, missing, and empty logins, review_request_removed, pull_request_target, and an unrelated event. The action: review_requested pairing has its own test. Integration tests cover admission, the length bound in both lists, and the v1alpha1 round-trip (preserved in exclusions, dropped in accepting filters).
• Docs: docs/reference.md and docs/integration.md match the implementation, including the caveat that omitting action also matches review_request_removed. The SessionSpawner reference section refers to the TaskSpawner filter fields, so no separate row is needed.
• Conventions: Generated CRD artifacts are included for both CRDs (chart templates and install-crd.yaml). The field is added only to v1alpha2. Commit messages contain no PR links. The PR body follows the template with /kind api and a release note.

Key takeaways

• The change follows the existing pullRequestAuthor pattern end to end: matcher, safe-unscoped classification, mirrored bound, restore-path check, and docs. It adds no new failure modes.
• If a v1alpha1 client rewrites the object, the accepting-filter criterion is dropped, which widens the match. The PR documents this, and it follows the existing precedent for pullRequestAuthor, conclusion, and checkName. It is not blocking here.