Add humanContactChannels to Agent Spec (not in-use yet) #56

balanceiskey · 2025-04-04T18:39:57Z

Add cool dudes like this:

apiVersion: kubechain.humanlayer.dev/v1alpha1
kind: Agent
metadata:
  name: agent-with-fetch
spec:
  llmRef:
    name: gpt-4o
  system: |
    You are a helpful assistant. Your job is to help the user with their tasks.
  mcpServers:
    - name: fetch
  humanContactChannels:
    - name: cool-friend

We'll get to actually putting this to work in the next PR.

Important

Add humanContactChannels to AgentSpec and AgentStatus, updating API, controller logic, and tests for future use.

API Changes:
- Add HumanContactChannels to AgentSpec and AgentStatus in agent_types.go.
- Update CRD in kubechain.humanlayer.dev_agents.yaml to include humanContactChannels.
Controller Logic:
- Add validateHumanContactChannels() in agent_controller.go to validate contact channels.
- Update Reconcile() in agent_controller.go to handle humanContactChannels.
Deepcopy Functions:
- Add deepcopy support for HumanContactChannels in zz_generated.deepcopy.go.
Testing:
- Update agent_controller_test.go to test humanContactChannels validation.
- Modify TestScopedAgent in objects_utils.go to include HumanContactChannels.

^{This description was created by}^{for 76f6d6f. It will automatically update as commits are pushed.}

ellipsis-dev

👍 Looks good to me! Reviewed everything up to 76f6d6f in 2 minutes and 25 seconds

More details

Looked at 512 lines of code in 9 files
Skipped 1 files when reviewing.
Skipped posting 11 drafted comments based on config settings.

1. kubechain/api/v1alpha1/agent_types.go:21

Draft comment:
New field HumanContactChannels is added correctly to AgentSpec. Ensure future documentation reflects its intended usage.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50%
None

2. kubechain/internal/controller/agent/agent_controller.go:125

Draft comment:
New function validateHumanContactChannels follows pattern of other validations. Ensure error messages (e.g. 'failed to get ContactChannel') consistently match testing expectations.
Reason this comment was not posted:
Confidence changes required: 30% <= threshold 50%
None

3. kubechain/config/crd/bases/kubechain.humanlayer.dev_agents.yaml:52

Draft comment:
CRD schema update for humanContactChannels and validHumanContactChannels looks correct. Ensure required fields are kept in sync with API changes.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50%
None

4. kubechain/internal/controller/agent/agent_controller_test.go:252

Draft comment:
Test cases for non-existent HumanContactChannel verify error handling; verify that error message substring matches actual output from validateHumanContactChannels.
Reason this comment was not posted:
Confidence changes required: 30% <= threshold 50%
None

5. kubechain/test/utils/objects_utils.go:43

Draft comment:
New helper logic for setting up HumanContactChannels in the TestScopedAgent is clear and aligns with the API; no issues found.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50%
None

6. kubechain/.gitignore:32

Draft comment:
MacOS specific system files (.DS_Store) are now ignored. This is standard and looks fine.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50%
None

7. kubechain/api/v1alpha1/agent_types.go:21

Draft comment:
Addition of HumanContactChannels in AgentSpec and ValidHumanContactChannels in AgentStatus looks good. Consider adding further validations (e.g. non-empty type) if future use cases require stricter constraints.
Reason this comment was not posted:
Confidence changes required: 33% <= threshold 50%
None

8. kubechain/config/crd/bases/kubechain.humanlayer.dev_agents.yaml:53

Draft comment:
CRD schema correctly reflects the new HumanContactChannels and ValidHumanContactChannels fields. Ensure consistency with the API definitions in future versions.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50%
None

9. kubechain/internal/controller/agent/agent_controller.go:125

Draft comment:
The new validateHumanContactChannels function is implemented correctly. Consider explicitly checking for NotFound errors (using errors.Is(err, apierrors.IsNotFound)) to differentiate not found cases from other errors, and note the repeated status-update blocks could be refactored for clarity.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
The comment addresses real patterns in the code - there is error handling that could be more specific, and there are repeated status update blocks. However, the error handling pattern is consistent with the rest of the codebase (validateLLM, validateTools, etc all handle errors the same way). The repeated status blocks are an existing pattern, not introduced by this PR. The PR only adds a new validateHumanContactChannels function following the existing patterns.
The suggestions would improve the code quality. Not distinguishing NotFound errors could make debugging harder. The repeated status blocks do make the code harder to maintain.
While valid suggestions, they address existing patterns in the codebase rather than issues with the changes made in this PR. The new code simply follows the established patterns.
Delete the comment. While the suggestions are valid, they are about existing code patterns rather than issues with the changes made in this PR.

10. kubechain/internal/controller/agent/agent_controller_test.go:252

Draft comment:
Test cases now cover successful validation and failures for invalid HumanContactChannel references. Consider adding an extra test for the scenario where a ContactChannel exists but isn’t ready.
Reason this comment was not posted:
Confidence changes required: 33% <= threshold 50%
None

11. kubechain/test/utils/objects_utils.go:43

Draft comment:
TestScopedAgent setup now correctly builds HumanContactChannels references. The approach is consistent with Tools. This looks good.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50%
None

Workflow ID: wflow_SfpfRG7PsLFxoyol

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

dexhorthy · 2025-04-04T21:43:36Z

kubechain/internal/controller/agent/agent_controller.go

 			statusUpdate.Status.Ready = false
 			statusUpdate.Status.Status = StatusError
 			statusUpdate.Status.StatusDetail = err.Error()
 			statusUpdate.Status.ValidTools = validTools


weird to me that we set all these things every time we reconcile

dexhorthy · 2025-04-04T21:44:28Z

kubechain/internal/controller/agent/agent_controller.go

+			statusUpdate.Status.Status = StatusError
+			statusUpdate.Status.StatusDetail = err.Error()
+			statusUpdate.Status.ValidTools = validTools
+			statusUpdate.Status.ValidMCPServers = validMCPServers


weird that this falls through here - like why don't we set it where we compute it? Right now, if I'm reading this code, I have to go scroll up and see where validMCPServers came from in order to know what's happening here.

like is this nil in this case? is this computed?

dexhorthy

approved but we need to clean up that status data stuff soon

Add humanContactChannels to Agent Spec (not in-use yet)

76f6d6f

balanceiskey requested review from allisoneer and dexhorthy April 4, 2025 18:40

ellipsis-dev bot reviewed Apr 4, 2025

View reviewed changes

dexhorthy reviewed Apr 4, 2025

View reviewed changes

dexhorthy approved these changes Apr 7, 2025

View reviewed changes

balanceiskey mentioned this pull request Apr 7, 2025

Add humanContactChannels to Agent Spec (not in-use yet) #62

Merged

balanceiskey closed this Apr 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add humanContactChannels to Agent Spec (not in-use yet) #56

Add humanContactChannels to Agent Spec (not in-use yet) #56

Uh oh!

balanceiskey commented Apr 4, 2025 •

edited by ellipsis-dev bot

Loading

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

dexhorthy Apr 4, 2025

Uh oh!

dexhorthy Apr 4, 2025

Uh oh!

dexhorthy left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add humanContactChannels to Agent Spec (not in-use yet) #56

Add humanContactChannels to Agent Spec (not in-use yet) #56

Uh oh!

Conversation

balanceiskey commented Apr 4, 2025 • edited by ellipsis-dev bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

dexhorthy Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

dexhorthy Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

dexhorthy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

balanceiskey commented Apr 4, 2025 •

edited by ellipsis-dev bot

Loading