-
Notifications
You must be signed in to change notification settings - Fork 12
Description
π€ Kelos Strategist Agent @gjkim42
Summary
When a PR receives new commits after a Kelos task has completed, the spawner does not automatically re-trigger the task β even though the PR content has fundamentally changed. This is a significant friction point for PR review workflows where agents should re-review after the author pushes fixes, but currently require manual /kelos review comments to restart.
This proposal adds a retriggerOnPush field to GitHubPullRequests that automatically re-triggers completed tasks when the PR's head SHA changes.
Problem
Current retrigger mechanism only handles comments and reviews
The spawner's retrigger logic (cmd/kelos-spawner/main.go:266-277) compares item.TriggerTime against existing.Status.CompletionTime. But TriggerTime is only populated from two sources (internal/source/github_pr.go:430-436):
- Comment trigger time β when a matching trigger comment (e.g.,
/kelos review) is posted - Review state trigger time β when the review state changes (only when
reviewStatefilter is notany)
When neither is configured, TriggerTime is zero, and the !item.TriggerTime.IsZero() guard prevents any retrigger. Even when a trigger comment is configured, pushing new commits does not reset the comment state β the old /kelos needs-input exclude comment remains the most recent matching command.
Real-world friction in kelos-reviewer
The kelos-reviewer spawner (self-development/kelos-reviewer.yaml) illustrates this perfectly:
- Maintainer comments
/kelos reviewβ reviewer task runs β reviewer posts/kelos needs-input(exclude comment) - PR author pushes commits addressing review feedback
- Nothing happens β the exclude comment is still the most recent, and pushing commits doesn't produce a new trigger
- Maintainer must manually comment
/kelos reviewagain to re-trigger
Step 4 is unnecessary manual toil. The reviewer should automatically re-engage when the PR is updated with new commits, since the code it reviewed has changed.
TTL-based workaround is too coarse
Setting ttlSecondsAfterFinished provides eventual re-triggering (old task expires β PR rediscovered as new), but:
- It retriggers even when no commits were pushed (wasting resources)
- The timing is arbitrary (TTL expiry doesn't correlate with push events)
- Short TTLs cause unnecessary retriggers; long TTLs cause delays after real pushes
Proposed API Change
Add retriggerOnPush to GitHubPullRequests:
type GitHubPullRequests struct {
// ... existing fields ...
// RetriggerOnPush enables automatic re-triggering of completed tasks
// when the pull request's head SHA changes (new commits are pushed).
// When enabled, a SHA change is treated as an implicit trigger event
// whose timestamp is the PR's updated_at time. This means:
// - If the PR has both a trigger comment and exclude comments, the
// push-based trigger competes with the most recent exclude comment
// based on chronological order (push time vs. exclude comment time).
// - If no comment policy is configured, the push-based trigger time
// alone is used for the retrigger time comparison.
//
// Best suited for read-only workflows (reviews, analysis) where the
// agent does not push commits to the PR branch. For agent-modifier
// workflows, use comment-based retrigger instead to avoid the agent's
// own commits causing re-triggers.
//
// +optional
RetriggerOnPush bool `json:"retriggerOnPush,omitempty"`
}YAML example (kelos-reviewer)
apiVersion: kelos.dev/v1alpha1
kind: TaskSpawner
metadata:
name: kelos-reviewer
spec:
when:
githubPullRequests:
state: open
draft: false
retriggerOnPush: true # β NEW: auto-retrigger on new commits
commentPolicy:
triggerComment: /kelos review
excludeComments:
- /kelos needs-input
allowedUsers:
- gjkim42
- kelos-bot[bot]
reporting:
enabled: true
pollInterval: 1m
maxConcurrency: 3
taskTemplate:
# ... (unchanged)With this config, the reviewer workflow becomes:
- Maintainer comments
/kelos reviewβ task runs β reviewer posts/kelos needs-input - PR author pushes new commits (HEAD SHA changes, PR
updated_atrefreshes) - Spawner detects SHA change β treats as implicit trigger at
updated_attime - Since
updated_at(push time) >/kelos needs-inputcomment time β trigger wins β task re-triggered automatically - Reviewer re-reviews the updated PR β posts
/kelos needs-inputagain - Cycle repeats on next push
If the maintainer posts /kelos needs-input AFTER the push (overriding the auto-trigger), the exclude comment wins chronologically and the PR stays excluded until the next push or manual /kelos review.
Implementation Plan
1. Store head SHA on spawned tasks
Add annotation kelos.dev/source-head-sha to tasks spawned from githubPullRequests:
// cmd/kelos-spawner/main.go β in sourceAnnotations()
if ts.Spec.When.GitHubPullRequests != nil {
annotations["kelos.dev/source-head-sha"] = item.HeadSHA
}2. Parse updated_at and head.sha from GitHub PR API
Extend githubPullRequest struct in internal/source/github_pr.go:
type githubPullRequest struct {
// ... existing fields ...
UpdatedAt string `json:"updated_at"` // NEW
}Add HeadSHA to WorkItem in internal/source/source.go:
type WorkItem struct {
// ... existing fields ...
HeadSHA string // PR head SHA for push-based retrigger
}3. Inject push-based trigger into TriggerTime calculation
In GitHubPullRequestSource.Discover(), when retriggerOnPush is enabled, use updated_at as an additional trigger time source:
// In the Discover() loop, after comment evaluation:
if s.RetriggerOnPush && pr.Head.SHA != "" {
pushTime := parsePRUpdatedAt(pr.UpdatedAt)
if pushTime.After(item.TriggerTime) {
item.TriggerTime = pushTime
}
}
item.HeadSHA = pr.Head.SHA4. Add SHA comparison to retrigger check
In cmd/kelos-spawner/main.go, extend the retrigger logic:
// Existing retrigger check (trigger time based):
if !item.TriggerTime.IsZero() && isCompleted && item.TriggerTime.After(completionTime) {
// delete and retrigger
}
// Additional retrigger check for push-based (when TriggerTime-based retrigger didn't fire):
if retriggerOnPush && isCompleted && item.HeadSHA != "" {
storedSHA := existing.Annotations["kelos.dev/source-head-sha"]
if storedSHA != "" && storedSHA != item.HeadSHA {
// SHA changed since task was created β retrigger
}
}The SHA comparison serves as a safety net: even if the updated_at time is stale or unparseable, a SHA mismatch definitively indicates the PR was updated.
5. Propagate config through source builder
Wire the new field through buildSource() in cmd/kelos-spawner/main.go:
// In the githubPullRequests branch:
return &source.GitHubPullRequestSource{
// ... existing fields ...
RetriggerOnPush: gh.RetriggerOnPush,
}, nilFiles changed
| File | Change |
|---|---|
api/v1alpha1/taskspawner_types.go |
Add RetriggerOnPush bool to GitHubPullRequests |
internal/source/source.go |
Add HeadSHA string to WorkItem |
internal/source/github_pr.go |
Add UpdatedAt to PR struct, RetriggerOnPush to source, wire into Discover() |
cmd/kelos-spawner/main.go |
Store SHA annotation, add SHA-based retrigger check, wire config |
internal/source/github_pr_test.go |
Test push-based trigger time injection |
cmd/kelos-spawner/main_test.go |
Test SHA-based retrigger with completed tasks |
Generated files
After API type changes: make update to regenerate CRD manifests and deepcopy.
Design consideration: agent-created commits
When retriggerOnPush is enabled on a workflow where the agent pushes commits (e.g., kelos-pr-responder), the agent's own commits would change the HEAD SHA, potentially causing an unwanted retrigger cycle.
Recommended usage pattern: retriggerOnPush is designed for read-only agent workflows (reviews, analysis, linting) where the agent does not modify the PR branch. For modifier workflows, continue using comment-based retrigger (/kelos pick-up).
This is analogous to how reviewState: approved is meaningful for review-gated workflows but not for the agents doing the work. The API field is opt-in, so users choose the appropriate retrigger strategy for each spawner.
A future enhancement could add retriggerOnPush.excludeAuthors to ignore commits from specific users (e.g., kelos-bot[bot]), enabling push-based retrigger even on modifier workflows. This is left for a follow-up to keep the initial implementation minimal.
Backward compatibility
retriggerOnPushis optional and defaults tofalse(current behavior preserved)- The
kelos.dev/source-head-shaannotation is additive and ignored by existing logic - The
HeadSHAWorkItem field has zero value when unset (empty string), which the SHA comparison skips - No CRD version bump needed (additive field in v1alpha1)
Related issues
| Issue | Relationship |
|---|---|
| #432 (immediate re-triggering) | Foundation β implemented the TriggerTime-based retrigger mechanism this builds on |
| #401 (lifecycle callbacks) | Complementary β reporting can notify when a push-based retrigger occurs |
| #749 (onCompletion hooks) | Complementary β hooks could fire on retrigger events |
| #498 (CEL matchExpressions) | Orthogonal β CEL filters which PRs enter the pipeline; retriggerOnPush controls re-engagement |