这是indexloc提供的服务，不要输入任何密码
Skip to content

When rejected, stamp status comment to trtc #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 2, 2025
Merged

When rejected, stamp status comment to trtc #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions kubechain/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
bin/*
Dockerfile.cross

# Developer playground samples
config/samples/*.play.yaml

# Test binary, built with `go test -c`
*.test

Expand Down
4 changes: 2 additions & 2 deletions kubechain/config/manager/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
images:
- name: controller
newName: ghcr.io/humanlayer/smallchain
newTag: v0.1.13
newName: controller
newTag: "202504012152"
10 changes: 7 additions & 3 deletions kubechain/internal/controller/taskruntoolcall/taskruntoolcall_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ func (r *TaskRunToolCallReconciler) setStatusError(ctx context.Context, trtcStat
return ctrl.Result{}, nil, true
}

func (r *TaskRunToolCallReconciler) updateTRTCStatus(ctx context.Context, trtc *kubechainv1alpha1.TaskRunToolCall, trtcStatusType kubechainv1alpha1.TaskRunToolCallStatusType, trtcStatusPhase kubechainv1alpha1.TaskRunToolCallPhase, statusDetail string) (ctrl.Result, error, bool) {
func (r *TaskRunToolCallReconciler) updateTRTCStatus(ctx context.Context, trtc *kubechainv1alpha1.TaskRunToolCall, trtcStatusType kubechainv1alpha1.TaskRunToolCallStatusType, trtcStatusPhase kubechainv1alpha1.TaskRunToolCallPhase, statusDetail string, result string) (ctrl.Result, error, bool) {
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot Apr 2, 2025

Choose a reason for hiding this comment

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

Document the new result parameter in updateTRTCStatus. Update function comment to note that result is used only when trtcStatusType indicates rejection.

Copy link
Contributor

@dexhorthy dexhorthy Apr 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

the signature of this method is getting really big - 6 params, 5 of them strings coming through now...is this really better than passing in a struct or doing it inline?

Copy link
Contributor

@dexhorthy dexhorthy Apr 2, 2025

Choose a reason for hiding this comment

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

not a blocker just something to think about

Copy link
Contributor Author

@balanceiskey balanceiskey Apr 2, 2025

Choose a reason for hiding this comment

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

Yeah I had that thought as I was rolling through, will peek at this on incoming updates.

Copy link
Contributor Author

@balanceiskey balanceiskey Apr 2, 2025

Choose a reason for hiding this comment

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

Was leaning struct, doing inline is definitely an option though, will judge based on how it reads.

logger := log.FromContext(ctx)

trtcDeepCopy := trtc.DeepCopy()
Expand All @@ -712,6 +712,10 @@ func (r *TaskRunToolCallReconciler) updateTRTCStatus(ctx context.Context, trtc *
trtcDeepCopy.Status.StatusDetail = statusDetail
trtcDeepCopy.Status.Phase = trtcStatusPhase

if trtcStatusType == kubechainv1alpha1.TaskRunToolCallStatusTypeToolCallRejected {
trtcDeepCopy.Status.Result = result
}

if err := r.Status().Update(ctx, trtcDeepCopy); err != nil {
logger.Error(err, "Failed to update status")
return ctrl.Result{}, err, true
Expand Down Expand Up @@ -789,12 +793,12 @@ func (r *TaskRunToolCallReconciler) handlePendingApproval(ctx context.Context, t
return r.updateTRTCStatus(ctx, trtc,
kubechainv1alpha1.TaskRunToolCallStatusTypeReadyToExecuteApprovedTool,
kubechainv1alpha1.TaskRunToolCallPhasePending,
"Ready to execute approved tool")
"Ready to execute approved tool", "")
} else {
return r.updateTRTCStatus(ctx, trtc,
kubechainv1alpha1.TaskRunToolCallStatusTypeToolCallRejected,
kubechainv1alpha1.TaskRunToolCallPhaseFailed,
"Tool execution rejected")
"Tool execution rejected", status.GetComment())
}
}

Expand Down
6 changes: 5 additions & 1 deletion kubechain/internal/controller/taskruntoolcall/taskruntoolcall_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ var _ = Describe("TaskRunToolCall Controller", func() {
})
})

// Tests for approval workflow
// Tests for MCP tools with approval requirement
Context("Pending -> AwaitingHumanApproval (MCP Tool, Slack Contact Channel)", func() {
It("transitions to AwaitingHumanApproval when MCPServer has approval channel", func() {
// Note setupTestApprovalResources sets up the MCP server, MCP tool, and TaskRunToolCall
Expand Down Expand Up @@ -406,11 +406,14 @@ var _ = Describe("TaskRunToolCall Controller", func() {
NeedsApproval: true,
}

rejectionComment := "You know what, I strongly disagree with this tool call and feel it should not be be given permission to execute. I, by the powers granted to me by The System, hereby reject it. If you too feel strongly, you can try again. I will reject it a second time, but with greater vigor."

reconciler.HLClientFactory = &humanlayer.MockHumanLayerClientFactory{
ShouldFail: false,
StatusCode: 200,
ReturnError: nil,
ShouldReturnRejection: true,
StatusComment: rejectionComment,
}

result, err := reconciler.Reconcile(ctx, reconcile.Request{
Expand All @@ -434,6 +437,7 @@ var _ = Describe("TaskRunToolCall Controller", func() {
Expect(updatedTRTC.Status.Phase).To(Equal(kubechainv1alpha1.TaskRunToolCallPhaseFailed))
Expect(updatedTRTC.Status.Status).To(Equal(kubechainv1alpha1.TaskRunToolCallStatusTypeToolCallRejected))
Expect(updatedTRTC.Status.StatusDetail).To(ContainSubstring("Tool execution rejected"))
Expect(updatedTRTC.Status.Result).To(Equal(rejectionComment))
})
})

Expand Down
2 changes: 2 additions & 0 deletions kubechain/internal/humanlayer/mock_hlclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type MockHumanLayerClientFactory struct {
LastRunID string
LastFunction string
LastArguments map[string]interface{}
StatusComment string
}

// MockHumanLayerClientWrapper implements HumanLayerClientWrapper for testing
Expand Down Expand Up @@ -104,6 +105,7 @@ func (m *MockHumanLayerClientWrapper) GetFunctionCallStatus(ctx context.Context)
RequestedAt: *humanlayerapi.NewNullableTime(&now),
RespondedAt: *humanlayerapi.NewNullableTime(&now),
Approved: *humanlayerapi.NewNullableBool(&approved),
Comment: *humanlayerapi.NewNullableString(&m.parent.StatusComment),
})
return &humanlayerapi.FunctionCallOutput{
Status: *status,
Expand Down