Main

[knechtionscoding]

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR is related to:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

[greptile-apps]

Greptile Summary

This PR introduces three main features: (1) an agentConfigRefs plural field on TaskSpec/TaskTemplate that allows merging multiple AgentConfig resources in order, (2) new PodOverrides fields (volumes, volumeMounts, podSecurityContext, containerSecurityContext) for PSS-restricted namespace support, and (3) a generic webhook source for arbitrary HTTP POST triggers with JSONPath field mapping. It also fixes a Helm chart YAML-splitting bug caused by rw-rw---- in CRD descriptions matching the naive --- separator, and hardens the CI stale-run detection to avoid blocking on an eventually-consistent workflow listing.

Confidence Score: 4/5

PR is safe to merge; all findings are P2 style/quality suggestions with no functional impact on the changed paths.

No P0 or P1 issues found. Two P2 observations: a shallow-copy inconsistency in the single-element merge fast path and a missing comment on the poll-interval display function's intentionally limited scope. The core logic (merge, volume validation, FSGroup preservation, CI run-supersession check) is well-tested and correct.

internal/controller/agentconfig_merge.go (shallow copy fast path)

Sequence Diagram

sequenceDiagram
    participant User as User/TaskSpawner
    participant TC as TaskController
    participant K8s as Kubernetes API
    participant JB as JobBuilder

    User->>K8s: Create Task (agentConfigRefs: [base, role])
    TC->>TC: ResolveAgentConfigRefs(task.Spec)
    loop For each AgentConfigRef
        TC->>K8s: Get AgentConfig by name
        K8s-->>TC: AgentConfigSpec
    end
    TC->>TC: MergeAgentConfigs([base.Spec, role.Spec])
    Note over TC: agentsMD concatenated, plugins/skills appended,
    Note over TC: mcpServers: later wins on collision
    TC->>JB: Build(task, workspace, mergedConfig, prompt)
    JB->>JB: validateUserVolumes (reserved name check)
    JB->>JB: Apply PodSecurityContext (preserve FSGroup)
    JB->>JB: Apply ContainerSecurityContext
    JB-->>TC: batchv1.Job
    TC->>K8s: Create Job

Loading

Comments Outside Diff (2)

1. internal/controller/agentconfig_merge.go, line 1587-1590 (link)

   Shallow copy in single-config fast path

   The single-element path returns &result where result is a copy of configs[0]. Since Go copies slices by header (pointer + len + cap), the returned Plugins, Skills, and MCPServers slices share their underlying arrays with the original. The multi-element path below always allocates fresh slices. Any caller that appends to the returned spec would silently mutate the original AgentConfigSpec. Consider allocating fresh slices for consistency:

   if len(configs) == 1 {
       result := configs[0]
       merged := kelosv1alpha1.AgentConfigSpec{AgentsMD: result.AgentsMD}
       merged.Plugins = append(merged.Plugins, result.Plugins...)
       merged.Skills = append(merged.Skills, result.Skills...)
       merged.MCPServers = append(merged.MCPServers, result.MCPServers...)
       return &merged
   }

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/controller/agentconfig_merge.go
   Line: 1587-1590
   
   Comment:
   **Shallow copy in single-config fast path**
   
   The single-element path returns `&result` where `result` is a copy of `configs[0]`. Since Go copies slices by header (pointer + len + cap), the returned `Plugins`, `Skills`, and `MCPServers` slices share their underlying arrays with the original. The multi-element path below always allocates fresh slices. Any caller that appends to the returned spec would silently mutate the original `AgentConfigSpec`. Consider allocating fresh slices for consistency:
   
   ```go
   if len(configs) == 1 {
       result := configs[0]
       merged := kelosv1alpha1.AgentConfigSpec{AgentsMD: result.AgentsMD}
       merged.Plugins = append(merged.Plugins, result.Plugins...)
       merged.Skills = append(merged.Skills, result.Skills...)
       merged.MCPServers = append(merged.MCPServers, result.MCPServers...)
       return &merged
   }
   ```
   
   How can I resolve this? If you propose a fix, please make it concise.

   

2. internal/cli/printer.go, line 1330-1340 (link)

   effectivePollInterval silently drops LinearWebhook/GenericWebhook/Cron sources

   The function only checks GitHubIssues, GitHubPullRequests, and Jira for per-source overrides. If a new polling source with its own PollInterval is added later, the display will silently show the deprecated top-level value. Adding a comment marking the fall-through as intentional would prevent future confusion.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/cli/printer.go
   Line: 1330-1340
   
   Comment:
   **`effectivePollInterval` silently drops LinearWebhook/GenericWebhook/Cron sources**
   
   The function only checks `GitHubIssues`, `GitHubPullRequests`, and `Jira` for per-source overrides. If a new polling source with its own `PollInterval` is added later, the display will silently show the deprecated top-level value. Adding a comment marking the fall-through as intentional would prevent future confusion.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

This is a comment left during a code review.
Path: internal/controller/agentconfig_merge.go
Line: 1587-1590

Comment:
**Shallow copy in single-config fast path**

The single-element path returns `&result` where `result` is a copy of `configs[0]`. Since Go copies slices by header (pointer + len + cap), the returned `Plugins`, `Skills`, and `MCPServers` slices share their underlying arrays with the original. The multi-element path below always allocates fresh slices. Any caller that appends to the returned spec would silently mutate the original `AgentConfigSpec`. Consider allocating fresh slices for consistency:

```go
if len(configs) == 1 {
    result := configs[0]
    merged := kelosv1alpha1.AgentConfigSpec{AgentsMD: result.AgentsMD}
    merged.Plugins = append(merged.Plugins, result.Plugins...)
    merged.Skills = append(merged.Skills, result.Skills...)
    merged.MCPServers = append(merged.MCPServers, result.MCPServers...)
    return &merged
}
```

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

---

This is a comment left during a code review.
Path: internal/cli/printer.go
Line: 1330-1340

Comment:
**`effectivePollInterval` silently drops LinearWebhook/GenericWebhook/Cron sources**

The function only checks `GitHubIssues`, `GitHubPullRequests`, and `Jira` for per-source overrides. If a new polling source with its own `PollInterval` is added later, the display will silently show the deprecated top-level value. Adding a comment marking the fall-through as intentional would prevent future confusion.

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

_{Reviews (1): Last reviewed commit: "Merge pull request #1041 from JustinElst..." | Re-trigger Greptile}