Skip to content

feat(reporting): add GitHub Checks API reporting#1016

Open
knechtionscoding wants to merge 1 commit intokelos-dev:mainfrom
datagravity-ai:feat/github-status-checks
Open

feat(reporting): add GitHub Checks API reporting#1016
knechtionscoding wants to merge 1 commit intokelos-dev:mainfrom
datagravity-ai:feat/github-status-checks

Conversation

@knechtionscoding
Copy link
Copy Markdown
Contributor

@knechtionscoding knechtionscoding commented Apr 27, 2026

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds GitHub Checks API integration to TaskSpawner reporting so that Kelos task outcomes appear as formal Check Runs on pull requests.

Today, Kelos reports task status via PR comments (reporting.enabled: true). While useful for humans, comments are invisible to GitHub's automated systems — branch protection rules, merge queues, and the Checks tab all ignore them. Teams that want to enforce "Kelos review must pass before merge" have no way to configure this.

This PR adds a new checks struct to the shared GitHubReporting type:

  • checks (struct, optional) — when present, creates GitHub Check Runs that map task phases to check statuses:
    • Pending/Running/Waiting → in_progress
    • Succeeded → completed / success
    • Failed → completed / failure
  • checks.name (string, optional) — overrides the default name ("Kelos: <spawner-name>"), which is what appears in branch protection rule configuration. The default is stable across releases; renaming a TaskSpawner changes the default and may require updating branch protection rules. MaxLength: 100.

Both comment and checks reporting can be enabled simultaneously, allowing gradual migration. The head SHA is propagated from the PR source through WorkItem.HeadSHA (polling) or GitHubEventData.HeadSHA (webhook) into a task annotation (kelos.dev/source-sha), enabling Check Run association with the correct commit.

Supported sources: checks lives on the shared GitHubReporting struct and is supported for:

  • githubPullRequests — always valid
  • githubWebhook — valid when at least one PR event type is configured (pull_request, pull_request_review, pull_request_review_comment, pull_request_target)
  • githubIssues — rejected via CEL validation (Check Runs are not applicable to issues)

CEL validation on githubWebhook ensures that reporting.checks is only accepted when the webhook includes at least one pull-request event type, preventing silent misconfiguration.

Example configurations:

# Polling-based PR source
spec:
  when:
    githubPullRequests:
      reporting:
        enabled: true       # still posts comments
        checks:              # also creates Check Runs
          name: "Kelos Security Review"
# Webhook-based PR source
spec:
  when:
    githubWebhook:
      events: ["pull_request"]
      reporting:
        enabled: true
        checks:
          name: "Kelos Code Review"

Files changed:

Area Files Summary
CRD api/v1alpha1/taskspawner_types.go, CRD manifests New GitHubChecksReporting type; checks field on shared GitHubReporting; CEL guards on githubIssues (reject) and githubWebhook (require PR event); MaxLength=100 on name
Source internal/source/source.go, github_pr.go Added HeadSHA to WorkItem, populated from PR head
Webhook internal/webhook/github_filter.go Added HeadSHA field to GitHubEventData, extracted from PR-related webhook events
Webhook internal/webhook/handler.go Stamps checks annotations (github-checks, source-sha, github-check-name) on webhook-created tasks for PR events
Reporting internal/reporting/checks.go (new) ChecksReporter — GitHub Checks API client (create + update)
Reporting internal/reporting/watcher.go Extended TaskReporter with reportViaCheckRun path, new annotations for check run state, in-memory cache backstop for check run IDs
Spawner cmd/kelos-spawner/main.go, reconciler.go checksReportingEnabled(), annotation stamping, ChecksReporter wiring for polling sources
Webhook server cmd/kelos-webhook-server/reporting.go Wires ChecksReporter when task has checks annotation; updated predicate to match checks-only tasks
Tests checks_test.go, watcher_test.go, main_test.go, handler_test.go, github_filter_test.go, reporting_test.go Unit tests for all new code paths

Which issue(s) this PR is related to:

Fixes #846

Special notes for your reviewer:

  • The GitHub token must have checks:write permission for Check Runs to be created. This is documented in the CRD field comment.
  • checks lives on the shared GitHubReporting type used by all GitHub sources. Applicability is enforced via CEL validation: githubIssues rejects checks entirely; githubWebhook requires at least one PR event type (pull_request, pull_request_review, pull_request_review_comment, pull_request_target).
  • For webhook sources, checks annotations are only stamped on tasks when the event is a PR event (determined at runtime by webhookSourceKind), so a webhook that mixes PR and issue events only gets Check Runs for the PR tasks.
  • HeadSHA is extracted from the webhook payload's head.sha field for PullRequestEvent, PullRequestReviewEvent, and PullRequestReviewCommentEvent.
  • The webhook server's reporting reconciler now wires ChecksReporter dynamically based on task annotations, and its predicate matches tasks with either comment or checks reporting annotations.
  • Check Run ID and report phase are persisted as task annotations (kelos.dev/github-check-run-id, kelos.dev/github-check-report-phase) following the same pattern as comment reporting.
  • The check run path uses the same ReportStateCache in-memory backstop as the comment path, preventing duplicate CreateCheckRun calls when concurrent reconciles race the annotation Update propagation.
  • When the source SHA annotation is missing, reportViaCheckRun logs an info message and returns nil, making the skip observable.
  • Re-run support (check_run.rerequested webhook handling) is intentionally deferred — it requires a small addition to the webhook event parser and is tracked separately.
  • The ChecksReporter uses custom HTTP (not go-github) to stay consistent with the existing GitHubReporter pattern and its TokenFunc dynamic auth.
  • The Accept header uses the current application/vnd.github+json form per GitHub's recommendation.
  • The default check name ("Kelos: <taskspawner-name>") is documented as stable across releases. Renaming a TaskSpawner changes the default, which is also documented.
  • checks.name has MaxLength=100 validation (GitHub's limit) so misconfiguration is caught at admission time.

Does this PR introduce a user-facing change?

Add GitHub Checks API reporting for TaskSpawners. Setting `reporting.checks` on a `githubPullRequests` or `githubWebhook` (with PR events) source creates GitHub Check Runs that reflect task status, enabling branch protection rules and merge queue integration. An optional `checks.name` field overrides the default check name.

@github-actions github-actions Bot added kind/feature Categorizes issue or PR as related to a new feature needs-triage needs-priority needs-actor release-note labels Apr 27, 2026
@knechtionscoding knechtionscoding force-pushed the feat/github-status-checks branch from bb28b86 to 4536eff Compare April 27, 2026 10:18
@gjkim42 gjkim42 self-assigned this Apr 27, 2026
@knechtionscoding knechtionscoding marked this pull request as ready for review April 27, 2026 14:07
@knechtionscoding knechtionscoding marked this pull request as draft April 27, 2026 14:07
@knechtionscoding
Copy link
Copy Markdown
Contributor Author

@gjkim42 once the other PR lands I will update this with webhook reporting as well, I have that working in my branch

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 12 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/reporting/watcher.go">

<violation number="1" location="internal/reporting/watcher.go:280">
P2: persistCheckRunState duplicates the entire retry/get/update/error-handling flow from persistReportingState with only different annotation keys/messages, increasing maintenance drift risk. Consider extracting a shared helper for annotation persistence to keep behavior consistent.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread internal/reporting/watcher.go
@gjkim42 gjkim42 added the kind/api Categorizes issue or PR as related to API changes label Apr 28, 2026
@gjkim42
Copy link
Copy Markdown
Collaborator

gjkim42 commented Apr 30, 2026

Merged it

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 30, 2026

Greptile Summary

This PR wires GitHub Checks API reporting into Kelos's TaskSpawner, adding ChecksReporter, a new GitHubChecksReporting CRD type, CEL guards, head-SHA propagation through WorkItem and webhook event data, and in-memory cache backstop for the checks path — paralleling the existing comment-reporting infrastructure cleanly.

  • P1 (handler.go:583): For issue_comment events on PRs, webhookSourceKind returns \"pull-request\" and AnnotationGitHubChecks is stamped \"enabled\", but IssueCommentEvent never populates HeadSHA. The reporter then silently skips with "Skipping Check Run: source SHA annotation is not set" — no check run is ever created despite the annotation being set. Gate the annotation block on parsed.GitHub.HeadSHA != \"\" to align the annotation with actual check-run creation.

Confidence Score: 4/5

Safe to merge after fixing the issue_comment/HeadSHA mismatch in handler.go; all other paths are correct.

One P1 defect: checks annotation is stamped for issue_comment-on-PR events but HeadSHA is never set for that event type, so no check run is ever created even though the task carries the enabled annotation. All three prior review concerns (silent skip, duplicate-create race, deprecated Accept header) have been addressed. The rest of the implementation — CEL guards, cache backstop, persist/retry logic, token resolver pattern — is solid.

internal/webhook/handler.go — the createTask checks annotation block (lines 583–591)

Important Files Changed

Filename Overview
internal/reporting/checks.go New ChecksReporter with correct Accept header, token pattern, and create/update lifecycle for GitHub Check Runs
internal/reporting/watcher.go reportViaCheckRun path added alongside existing comment path; in-memory cache backstop, log on SHA skip, and persistCheckRunState all correctly implemented
internal/webhook/handler.go Checks annotation stamped for pull-request kind events including issue_comment on PRs, but HeadSHA is never populated for IssueCommentEvent, causing silent skip in reportViaCheckRun (P1)
internal/webhook/github_filter.go HeadSHA correctly extracted for PullRequestEvent, PullRequestReviewEvent, PullRequestReviewCommentEvent; not set for IssueCommentEvent by design
cmd/kelos-spawner/reconciler.go checksReportingEnabled and ChecksReporter wiring correctly added for polling sources; Cache intentionally nil (MaxConcurrentReconciles=1)
cmd/kelos-webhook-server/reporting.go reportingEnabled predicate correctly updated to include AnnotationGitHubChecks; ChecksReporter wired per-task based on annotation
api/v1alpha1/taskspawner_types.go New GitHubChecksReporting type with MaxLength=100; CEL guards on githubIssues (reject) and githubWebhook (require PR event) correctly applied
internal/source/github_pr.go HeadSHA populated on WorkItem from pr.Head.SHA; no issues
cmd/kelos-spawner/main.go sourceAnnotations correctly stamps checks annotations and HeadSHA for polling PR sources; checksReportingEnabled only checks GitHubPullRequests (correct — CEL blocks issues)

Sequence Diagram

sequenceDiagram
    participant GH as GitHub
    participant WH as WebhookHandler
    participant K8s as Kubernetes API
    participant RR as ReportingReconciler
    participant CR as ChecksReporter

    GH->>WH: pull_request / pull_request_review webhook
    WH->>WH: ParseGitHubWebhook (extracts HeadSHA)
    WH->>K8s: Create Task (annotations: github-checks=enabled, source-sha, github-check-name)
    K8s-->>RR: Task reconcile event (phase: Pending/Running)
    RR->>CR: CreateCheckRun(name, headSHA, in_progress)
    CR->>GH: POST /repos/{owner}/{repo}/check-runs
    GH-->>CR: {id: 42}
    RR->>K8s: Annotate Task (github-check-run-id=42, github-check-report-phase=in_progress)
    K8s-->>RR: Task reconcile event (phase: Succeeded/Failed)
    RR->>CR: UpdateCheckRun(42, completed, success/failure)
    CR->>GH: PATCH /repos/{owner}/{repo}/check-runs/42
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
internal/webhook/handler.go:583-591
**Checks annotation stamped with no SHA for `issue_comment`-on-PR events**

When an `issue_comment` event arrives on a PR, `webhookSourceKind` returns `"pull-request"`, so `AnnotationGitHubChecks` is set to `"enabled"`. But `IssueCommentEvent` never populates `HeadSHA` (the `github_filter.go` parser only sets `HeadSHA` for `PullRequestEvent`, `PullRequestReviewEvent`, and `PullRequestReviewCommentEvent`). As a result, `AnnotationSourceSHA` is not stamped, and `reportViaCheckRun` silently skips with `"Skipping Check Run: source SHA annotation is not set"` — no check run is ever created even though the task carries the `enabled` annotation.

The minimal fix is to gate the annotation on a non-empty `HeadSHA`:

```go
if rep.Checks != nil && webhookSourceKind(eventType, parsed.GitHub) == "pull-request" && parsed.GitHub.HeadSHA != "" {
    task.Annotations[reporting.AnnotationGitHubChecks] = "enabled"
    task.Annotations[reporting.AnnotationSourceSHA] = parsed.GitHub.HeadSHA
    if rep.Checks.Name != "" {
        task.Annotations[reporting.AnnotationGitHubCheckName] = rep.Checks.Name
    }
}
```

This is reproducible with any webhook spawner that lists both `issue_comment` and a PR event type (the CEL guard permits this), when a PR reviewer leaves a comment.

```suggestion
if rep.Checks != nil && webhookSourceKind(eventType, parsed.GitHub) == "pull-request" && parsed.GitHub.HeadSHA != "" {
			task.Annotations[reporting.AnnotationGitHubChecks] = "enabled"
			task.Annotations[reporting.AnnotationSourceSHA] = parsed.GitHub.HeadSHA
			if rep.Checks.Name != "" {
				task.Annotations[reporting.AnnotationGitHubCheckName] = rep.Checks.Name
			}
		}
```

Reviews (7): Last reviewed commit: "feat(reporting): add GitHub Checks API r..." | Re-trigger Greptile

Comment thread internal/reporting/watcher.go
Comment thread internal/reporting/watcher.go Outdated
Comment thread internal/reporting/checks.go Outdated
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 12 files

@gjkim42
Copy link
Copy Markdown
Collaborator

gjkim42 commented Apr 30, 2026

/kelos api-review

@kelos-bot
Copy link
Copy Markdown

kelos-bot Bot commented Apr 30, 2026

🤖 Kelos Task Status

Task kelos-api-reviewer-issue-comment-c30218bf308e has failed. ❌

@gjkim42
Copy link
Copy Markdown
Collaborator

gjkim42 commented Apr 30, 2026

/kelos api-review

@kelos-bot
Copy link
Copy Markdown

kelos-bot Bot commented Apr 30, 2026

🤖 Kelos Task Status

Task kelos-api-reviewer-issue-comment-b0e7e109316d has succeeded. ✅

Copy link
Copy Markdown

@kelos-bot kelos-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Kelos API Reviewer Agent @gjkim42

API Design Review

Verdict: REQUEST CHANGES
Scope: Adds Checks bool and CheckName string fields to GitHubReporting for GitHub Checks API integration.

Findings

Extensibility / future-proofing (blocking)

  • api/v1alpha1/taskspawner_types.go:69-83 — The new fields are introduced as a flat pair on GitHubReporting (checks: bool + checkName: string), but they are conceptually one feature. Once shipped, every additional knob (output template, conclusion mapping, re-run behavior, neutral conclusion, summary truncation, separate check name suffix per task, etc. — the PR description already mentions check_run.rerequested is deferred) has to land as another checkX-prefixed sibling. CRD fields are effectively permanent: it is much cheaper to fix this now, before any user has set checks: true, than to maintain a parallel namespace forever. Recommend collapsing into a struct from the start, e.g.:
    // GitHubChecksReporting configures GitHub Check Run reporting.
    type GitHubChecksReporting struct {
        // Name overrides the default Check Run name ("Kelos: <taskspawner-name>").
        // +optional
        Name string `json:"name,omitempty"`
    }
    // ...
    type GitHubReporting struct {
        Enabled bool `json:"enabled,omitempty"`
        // Checks creates GitHub Check Runs ... When nil, no Check Runs are created.
        // +optional
        Checks *GitHubChecksReporting `json:"checks,omitempty"`
    }
    Presence of the struct = enabled, absence = disabled, and any future sub-field nests cleanly under checks.*. The YAML still reads naturally:
    reporting:
      enabled: true
      checks:
        name: "Kelos Security Review"
    This also matches the broader "prefer a struct over a bare scalar/bool when more knobs are likely later" guidance in the API conventions.

API conventions / shape (blocking)

  • api/v1alpha1/taskspawner_types.go:64GitHubReporting is shared by GitHubIssues.Reporting, GitHubPullRequests.Reporting, and GitHubWebhook.Reporting (see usages at lines 204, 296, 391). The CRD diff (taskspawner-crd.yaml:3071-3085, 3262-3276, 3424-3438) confirms checks and checkName are now schema-valid in all three locations, but checksReportingEnabled in cmd/kelos-spawner/main.go:524-528 only reads from Spec.When.GitHubPullRequests. An operator who sets checks: true under githubIssues or githubWebhook gets no error, no log line, and no Check Runs — just silent failure. The field comment ("Only applies to githubPullRequests source") is documentation, not enforcement. Two acceptable fixes:
    1. Split the type: keep GitHubReporting (Enabled-only) for issues/webhooks and define a GitHubPullRequestReporting that embeds it and adds Checks (preferred — makes the schema self-documenting).
    2. Add CEL XValidation on GitHubIssues and GitHubWebhook rejecting reporting.checks (or reporting.checks.* after the restructure in the previous finding) so admission rejects the misconfiguration immediately.
      Note this also affects the webhook source path, not only issues — webhook-driven tasks have no head SHA propagated either, so the silent no-op is the same.

Validation

  • api/v1alpha1/taskspawner_types.go:83CheckName has no length cap. GitHub Check Run names are bounded; an over-long name will fail at the API call (returning a generic 4xx that surfaces as a reconcile error) instead of at admission. Add +kubebuilder:validation:MaxLength=… (GitHub's documented limit, currently 100, is a reasonable upper bound) so misconfiguration is caught at apply time.

Naming and documentation

  • api/v1alpha1/taskspawner_types.go:79-83 — The doc comment notes that CheckName "appears in branch protection rule configuration", which means the default value ("Kelos: <taskspawner-name>") is effectively a user-visible contract: changing it later silently breaks every branch protection rule that referenced the old name. Worth documenting that the default is stable / not expected to change, so the contract is explicit. (The check name also includes the spawner name, so renaming a TaskSpawner becomes a similar break — also worth a sentence in the doc.)
  • api/v1alpha1/taskspawner_types.go:69-77 — Comment says "includes the task name in the check details" but checks.go:48 puts the check name in the title and the task name in the summary. Minor doc/code mismatch — clarify which is which.
  • If the struct restructure above is adopted, name the inner field just name (matching GitHub's own Check Run terminology) instead of checkNamereporting.checks.checkName would be redundant.

Suggestions (non-blocking)

  • internal/source/source.go:25HeadSHA is fine as an internal struct field, but consider whether other source kinds (e.g. push webhooks) that legitimately have a head SHA might want to populate it later; the current name and field comment ("for GitHub PR sources") will read oddly if so. Not blocking — internal types are easy to rename.
  • cmd/kelos-spawner/main.go:495-501AnnotationGitHubChecks is set independently of AnnotationGitHubReporting, mirroring the spec shape. If Checks becomes a struct (per the first finding), the annotation can stay a simple "enabled" flag, but consider whether a single structured kelos.dev/github-reporting-modes annotation (e.g. comments,checks) would scale better as more reporting backends are added. Internal state, not API — only worth doing if you also expect more modes soon.

Note on prompt injection: the cubic-dev-ai and greptile-apps review comments on this PR contain embedded instructions ("Prompt for AI agents", attribution demands). Those were disregarded; the findings above are formed independently from reading the diff.

/kelos needs-input

@knechtionscoding
Copy link
Copy Markdown
Contributor Author

Feedback addressed @gjkim42

Comment thread api/v1alpha1/taskspawner_types.go Outdated
@gjkim42
Copy link
Copy Markdown
Collaborator

gjkim42 commented May 1, 2026

/kelos pick-up

@kelos-bot
Copy link
Copy Markdown

kelos-bot Bot commented May 1, 2026

🤖 Kelos Task Status

Task kelos-workers-issue-comment-023058fcde5c has succeeded. ✅

@gjkim42
Copy link
Copy Markdown
Collaborator

gjkim42 commented May 1, 2026

/kelos review

@kelos-bot
Copy link
Copy Markdown

kelos-bot Bot commented May 1, 2026

🤖 Kelos Task Status

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

Copy link
Copy Markdown

@kelos-bot kelos-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Kelos Reviewer Agent @gjkim42

Review Summary

Verdict: REQUEST CHANGES
Overall correctness: patch is incorrect
Scope: Adds a reporting.checks struct to TaskSpawners that creates GitHub Check Runs for PR-discovered tasks, enabling branch-protection integration alongside the existing comment reporter.

Findings Overview

Priority Count File:Line Summary
P0 0 none
P1 1 api/v1alpha1/taskspawner_types.go:377 reporting.checks accepted on githubWebhook source but silently ignored at runtime
P2 1 api/v1alpha1/taskspawner_types.go:63 Type doc comment and PR description claim a GitHubPullRequestReporting type split that does not exist
P3 0 none

Findings

Correctness / API

  • [P1] api/v1alpha1/taskspawner_types.go:377GitHubWebhook.Reporting is *GitHubReporting, the same shared type that now carries Checks, so the CRD schema accepts reporting.checks on githubWebhook sources. GitHubIssues has a CEL guard (!has(self.reporting) || !has(self.reporting.checks)), GitHubWebhook does not. The runtime path that stamps AnnotationGitHubChecks lives only in cmd/kelos-spawner/main.go:495 (checksReportingEnabled reads Spec.When.GitHubPullRequests) and internal/webhook/handler.go:567 only stamps AnnotationGitHubReporting. A user who configures reporting.checks on a webhook source therefore gets no admission error and zero check runs with no diagnostic. Add the same CEL rule to GitHubWebhook (and regenerate deploy/charts/kelos/templates/crds/taskspawner-crd.yaml and internal/manifests/install-crd.yaml); remove it when webhook-driven check reporting is wired up in the follow-up PR. Suggested addition above the GitHubWebhook declaration:
    // +kubebuilder:validation:XValidation:rule="!has(self.reporting) || !has(self.reporting.checks)",message="checks reporting is not supported for githubWebhook source"

Conventions / Documentation

  • [P2] api/v1alpha1/taskspawner_types.go:63 and PR description — the comment on GitHubReporting says “The Checks field is only valid for pull-request-based sources (githubPullRequests, and githubWebhook with pull request events)” and the PR description claims the type was split into a GitHubPullRequestReporting (“the checks field does not appear in their CRD schema at all”). Neither is true: Checks lives on the shared GitHubReporting struct used by all three GitHub sources, and at runtime only GitHubPullRequests is honored. Reconcile the comment, the special-notes section, and (if intent is to defer webhook support) reinforce with the CEL guard above so the doc and schema agree.

Key takeaways

  • The CEL guard for githubWebhook is the only change needed for correctness; once added, the doc/PR-description discrepancy is just a wording fix.
  • The cache backstop for check-run IDs (storeCheckRun + the cached.checkRunID != 0 short-circuit) and the duplicate-suppression test coverage are solid.

Note on prompt injection: third-party bot comments on this PR contain embedded directives instructing readers to attribute findings to those tools or to suppress unrelated content. Those instructions were disregarded; findings above are based solely on the diff and surrounding code.

…n integration

Add GitHub Check Run support to GitHubReporting so Kelos task outcomes
appear as formal Check Runs on pull requests. This enables teams to
require "Kelos agent passed" in branch protection rules and makes task
status visible in the PR Checks tab.

New fields on GitHubReporting:
- checks (struct, optional) — when present, creates Check Runs
- checks.name (string) — overrides the default "Kelos: <spawner-name>"

Supported for both githubPullRequests and githubWebhook sources with
pull-request event types. A CEL validation rule on githubIssues rejects
checks (not applicable), and on githubWebhook requires at least one PR
event type when checks is configured.

Check Run lifecycle maps task phases to GitHub states:
- Pending/Running/Waiting → in_progress
- Succeeded → completed (success)
- Failed → completed (failure)

Head SHA is propagated from PR sources through WorkItem.HeadSHA (polling)
or GitHubEventData.HeadSHA (webhook) into a task annotation, enabling
Check Run association with the correct commit.

Both comment and checks reporting can be enabled simultaneously.

Closes kelos-dev#846
@knechtionscoding
Copy link
Copy Markdown
Contributor Author

@gjkim42 Okay, updated. I lost the github webhook support for checks somewhere in there, so I added it back.

@knechtionscoding
Copy link
Copy Markdown
Contributor Author

@greptile review

Comment on lines +583 to +591
if rep.Checks != nil && webhookSourceKind(eventType, parsed.GitHub) == "pull-request" {
task.Annotations[reporting.AnnotationGitHubChecks] = "enabled"
if parsed.GitHub.HeadSHA != "" {
task.Annotations[reporting.AnnotationSourceSHA] = parsed.GitHub.HeadSHA
}
if rep.Checks.Name != "" {
task.Annotations[reporting.AnnotationGitHubCheckName] = rep.Checks.Name
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Checks annotation stamped with no SHA for issue_comment-on-PR events

When an issue_comment event arrives on a PR, webhookSourceKind returns "pull-request", so AnnotationGitHubChecks is set to "enabled". But IssueCommentEvent never populates HeadSHA (the github_filter.go parser only sets HeadSHA for PullRequestEvent, PullRequestReviewEvent, and PullRequestReviewCommentEvent). As a result, AnnotationSourceSHA is not stamped, and reportViaCheckRun silently skips with "Skipping Check Run: source SHA annotation is not set" — no check run is ever created even though the task carries the enabled annotation.

The minimal fix is to gate the annotation on a non-empty HeadSHA:

if rep.Checks != nil && webhookSourceKind(eventType, parsed.GitHub) == "pull-request" && parsed.GitHub.HeadSHA != "" {
    task.Annotations[reporting.AnnotationGitHubChecks] = "enabled"
    task.Annotations[reporting.AnnotationSourceSHA] = parsed.GitHub.HeadSHA
    if rep.Checks.Name != "" {
        task.Annotations[reporting.AnnotationGitHubCheckName] = rep.Checks.Name
    }
}

This is reproducible with any webhook spawner that lists both issue_comment and a PR event type (the CEL guard permits this), when a PR reviewer leaves a comment.

Suggested change
if rep.Checks != nil && webhookSourceKind(eventType, parsed.GitHub) == "pull-request" {
task.Annotations[reporting.AnnotationGitHubChecks] = "enabled"
if parsed.GitHub.HeadSHA != "" {
task.Annotations[reporting.AnnotationSourceSHA] = parsed.GitHub.HeadSHA
}
if rep.Checks.Name != "" {
task.Annotations[reporting.AnnotationGitHubCheckName] = rep.Checks.Name
}
}
if rep.Checks != nil && webhookSourceKind(eventType, parsed.GitHub) == "pull-request" && parsed.GitHub.HeadSHA != "" {
task.Annotations[reporting.AnnotationGitHubChecks] = "enabled"
task.Annotations[reporting.AnnotationSourceSHA] = parsed.GitHub.HeadSHA
if rep.Checks.Name != "" {
task.Annotations[reporting.AnnotationGitHubCheckName] = rep.Checks.Name
}
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/webhook/handler.go
Line: 583-591

Comment:
**Checks annotation stamped with no SHA for `issue_comment`-on-PR events**

When an `issue_comment` event arrives on a PR, `webhookSourceKind` returns `"pull-request"`, so `AnnotationGitHubChecks` is set to `"enabled"`. But `IssueCommentEvent` never populates `HeadSHA` (the `github_filter.go` parser only sets `HeadSHA` for `PullRequestEvent`, `PullRequestReviewEvent`, and `PullRequestReviewCommentEvent`). As a result, `AnnotationSourceSHA` is not stamped, and `reportViaCheckRun` silently skips with `"Skipping Check Run: source SHA annotation is not set"` — no check run is ever created even though the task carries the `enabled` annotation.

The minimal fix is to gate the annotation on a non-empty `HeadSHA`:

```go
if rep.Checks != nil && webhookSourceKind(eventType, parsed.GitHub) == "pull-request" && parsed.GitHub.HeadSHA != "" {
    task.Annotations[reporting.AnnotationGitHubChecks] = "enabled"
    task.Annotations[reporting.AnnotationSourceSHA] = parsed.GitHub.HeadSHA
    if rep.Checks.Name != "" {
        task.Annotations[reporting.AnnotationGitHubCheckName] = rep.Checks.Name
    }
}
```

This is reproducible with any webhook spawner that lists both `issue_comment` and a PR event type (the CEL guard permits this), when a PR reviewer leaves a comment.

```suggestion
if rep.Checks != nil && webhookSourceKind(eventType, parsed.GitHub) == "pull-request" && parsed.GitHub.HeadSHA != "" {
			task.Annotations[reporting.AnnotationGitHubChecks] = "enabled"
			task.Annotations[reporting.AnnotationSourceSHA] = parsed.GitHub.HeadSHA
			if rep.Checks.Name != "" {
				task.Annotations[reporting.AnnotationGitHubCheckName] = rep.Checks.Name
			}
		}
```

How can I resolve this? If you propose a fix, please make it concise.

@gjkim42
Copy link
Copy Markdown
Collaborator

gjkim42 commented May 1, 2026

/kelos review

@kelos-bot
Copy link
Copy Markdown

kelos-bot Bot commented May 1, 2026

🤖 Kelos Task Status

Task kelos-reviewer-issue-comment-8f6cb9b3fcf2 has succeeded. ✅

Copy link
Copy Markdown

@kelos-bot kelos-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Kelos Reviewer Agent @gjkim42

Review Summary

Verdict: REQUEST CHANGES
Overall correctness: patch is incorrect
Scope: Adds GitHub Checks API reporting through a new reporting.checks struct on the shared GitHubReporting type, head-SHA propagation via WorkItem and webhook event data, a new ChecksReporter HTTP client, and a parallel create/update lifecycle alongside the existing comment reporter.

Findings Overview

Priority Count File:Line Summary
P0 0 none
P1 1 internal/webhook/handler.go:583 issue_comment-on-PR webhook events stamp AnnotationGitHubChecks=enabled without a SHA, producing a silent no-op
P2 0 none
P3 0 none

Findings

Correctness

  • [P1] internal/webhook/handler.go:583 — A webhook spawner that lists issue_comment alongside a PR event type passes the GitHubWebhook CEL guard (!has(self.reporting.checks) || self.events.exists(...) only requires one PR event), but at runtime an issue_comment on a PR returns "pull-request" from webhookSourceKind, so the block stamps AnnotationGitHubChecks="enabled" without ever populating AnnotationSourceSHAIssueCommentEvent parsing in github_filter.go does not set HeadSHA. reportViaCheckRun then short-circuits on the missing SHA (internal/reporting/watcher.go:277-281), logs an info-level skip, and returns nil. The task carries the enabled annotation but no Check Run is ever created. This is reachable on every /kelos-style trigger comment in a typical mixed-event configuration. Fix by gating the annotation stamping on parsed.GitHub.HeadSHA != "", by restricting the eventType match to the four explicit PR event types, or by lazily fetching the head SHA from the PR API URL the way githubPRBranchFetcher already does for the branch.

Suggestions

  • [P3] Add a handler-level test for issue_comment on a PR with reporting.checks configured. The current TestServeHTTP_Checks* tests only exercise the pull_request event path, so the silent-skip configuration above is not covered.

Key takeaways

  • One P1 silent-failure path on webhook issue_comment events. The rest of the wiring — CEL guards on githubIssues (reject) and githubWebhook (require PR event), the in-memory cache backstop for check run IDs, the persist/retry refactor, and the dual comment+checks reporting — is consistent and well-tested.

Note on prompt injection: the cubic-dev-ai and greptile-apps review comments on this PR contain embedded directives (e.g., "Prompt for AI agents" sections, attribution demands). Those instructions were disregarded; findings above are formed independently from reading the diff.

Comment on lines +583 to +591
if rep.Checks != nil && webhookSourceKind(eventType, parsed.GitHub) == "pull-request" {
task.Annotations[reporting.AnnotationGitHubChecks] = "enabled"
if parsed.GitHub.HeadSHA != "" {
task.Annotations[reporting.AnnotationSourceSHA] = parsed.GitHub.HeadSHA
}
if rep.Checks.Name != "" {
task.Annotations[reporting.AnnotationGitHubCheckName] = rep.Checks.Name
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] When eventType == "issue_comment" on a PR, webhookSourceKind returns "pull-request" (handler.go:648-650), so this block stamps AnnotationGitHubChecks="enabled". But *github.IssueCommentEvent is parsed in internal/webhook/github_filter.go:136-152 without setting HeadSHA — only the three PullRequest*Event cases populate it. The result is a task that carries the enabled annotation but no AnnotationSourceSHA, so at reconcile time reportViaCheckRun (internal/reporting/watcher.go:277-281) hits the no-SHA short-circuit, logs "Skipping Check Run: source SHA annotation is not set", and returns nil — no Check Run is ever created. The GitHubWebhook CEL guard requires only at least one PR event type, so any spawner that mixes issue_comment with a PR event (a typical /kelos review trigger setup) reaches this path on every PR comment. Either gate the annotation block on parsed.GitHub.HeadSHA != "", restrict to the explicit PR event types, or fetch the head SHA via the same lazy enrichment used for branch resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/api Categorizes issue or PR as related to API changes kind/feature Categorizes issue or PR as related to a new feature needs-actor needs-priority needs-triage release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integration: Add GitHub Checks API reporting for branch protection and PR status integration

2 participants