Skip to content

Commit d061526

Browse files
adaltonclaude
andauthored
feat: implement per-repository autoPush configuration (#499)
Replace global autoPushOnComplete setting with per-repository autoPush field to enable granular control of git push behavior for agentic sessions. Each repository can now be configured independently with autoPush: true/false. ## Changes - Add autoPush field to SessionRepo type (backend, frontend, CRD) - Remove deprecated autoPushOnComplete from all components - Add type validation in runner to handle invalid autoPush values - Generate Git Push Instructions in system prompt for repos with autoPush=true - Add comprehensive test coverage (10 backend tests, 19 runner tests) - Update UI with clear help text explaining autoPush behavior ## Testing - All 351 backend tests pass - All 19 runner tests pass (including edge case type validation tests) - Frontend builds with 0 errors, 0 warnings - Manual testing verified correct behavior Co-authored-by: Claude Sonnet 4.5 <[email protected]>
1 parent 028affb commit d061526

File tree

11 files changed

+722
-34
lines changed

11 files changed

+722
-34
lines changed

components/backend/handlers/sessions.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,11 @@ func parseSpec(spec map[string]interface{}) types.AgenticSessionSpec {
179179
if branch, ok := m["branch"].(string); ok && strings.TrimSpace(branch) != "" {
180180
r.Branch = types.StringPtr(branch)
181181
}
182+
// Parse autoPush as optional boolean. Preserve nil to allow CRD default.
183+
// nil = use default (false), false = explicit no-push, true = explicit push
184+
if autoPush, ok := m["autoPush"].(bool); ok {
185+
r.AutoPush = types.BoolPtr(autoPush)
186+
}
182187
if strings.TrimSpace(r.URL) != "" {
183188
repos = append(repos, r)
184189
}
@@ -626,11 +631,6 @@ func CreateSession(c *gin.Context) {
626631
session["spec"].(map[string]interface{})["interactive"] = *req.Interactive
627632
}
628633

629-
// AutoPushOnComplete flag
630-
if req.AutoPushOnComplete != nil {
631-
session["spec"].(map[string]interface{})["autoPushOnComplete"] = *req.AutoPushOnComplete
632-
}
633-
634634
// Set multi-repo configuration on spec (simplified format)
635635
{
636636
spec := session["spec"].(map[string]interface{})
@@ -641,6 +641,9 @@ func CreateSession(c *gin.Context) {
641641
if r.Branch != nil {
642642
m["branch"] = *r.Branch
643643
}
644+
if r.AutoPush != nil {
645+
m["autoPush"] = *r.AutoPush
646+
}
644647
arr = append(arr, m)
645648
}
646649
spec["repos"] = arr
@@ -1296,8 +1299,9 @@ func AddRepo(c *gin.Context) {
12961299
}
12971300

12981301
var req struct {
1299-
URL string `json:"url" binding:"required"`
1300-
Branch string `json:"branch"`
1302+
URL string `json:"url" binding:"required"`
1303+
Branch string `json:"branch"`
1304+
AutoPush *bool `json:"autoPush,omitempty"`
13011305
}
13021306

13031307
if err := c.ShouldBindJSON(&req); err != nil {
@@ -1387,6 +1391,9 @@ func AddRepo(c *gin.Context) {
13871391
"url": req.URL,
13881392
"branch": req.Branch,
13891393
}
1394+
if req.AutoPush != nil {
1395+
newRepo["autoPush"] = *req.AutoPush
1396+
}
13901397
repos = append(repos, newRepo)
13911398
spec["repos"] = repos
13921399

components/backend/handlers/sessions_test.go

Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import (
2323
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2424
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2525
"k8s.io/apimachinery/pkg/runtime/schema"
26+
"k8s.io/client-go/dynamic"
27+
"k8s.io/client-go/kubernetes"
2628
)
2729

2830
var _ = Describe("Sessions Handler", Label(test_constants.LabelUnit, test_constants.LabelHandlers, test_constants.LabelSessions), func() {
@@ -521,6 +523,262 @@ var _ = Describe("Sessions Handler", Label(test_constants.LabelUnit, test_consta
521523
})
522524
})
523525
})
526+
527+
// AutoPush functionality tests
528+
Context("AutoPush Field Parsing", func() {
529+
var (
530+
testClientFactory *test_utils.TestClientFactory
531+
fakeClients *test_utils.FakeClientSet
532+
originalK8sClient kubernetes.Interface
533+
originalK8sClientMw kubernetes.Interface
534+
originalK8sClientProjects kubernetes.Interface
535+
originalDynamicClient dynamic.Interface
536+
)
537+
538+
BeforeEach(func() {
539+
logger.Log("Setting up AutoPush test")
540+
541+
// Save original state
542+
originalK8sClient = K8sClient
543+
originalK8sClientMw = K8sClientMw
544+
originalK8sClientProjects = K8sClientProjects
545+
originalDynamicClient = DynamicClient
546+
547+
// Create fake clients
548+
testClientFactory = test_utils.NewTestClientFactory()
549+
fakeClients = testClientFactory.GetFakeClients()
550+
551+
DynamicClient = fakeClients.GetDynamicClient()
552+
K8sClientProjects = fakeClients.GetK8sClient()
553+
K8sClient = fakeClients.GetK8sClient()
554+
K8sClientMw = fakeClients.GetK8sClient()
555+
})
556+
557+
AfterEach(func() {
558+
// Restore original state
559+
K8sClient = originalK8sClient
560+
K8sClientMw = originalK8sClientMw
561+
K8sClientProjects = originalK8sClientProjects
562+
DynamicClient = originalDynamicClient
563+
})
564+
565+
Describe("parseSpec", func() {
566+
It("Should parse autoPush=true from repo", func() {
567+
spec := map[string]interface{}{
568+
"repos": []interface{}{
569+
map[string]interface{}{
570+
"url": "https://github.com/owner/repo.git",
571+
"branch": "main",
572+
"autoPush": true,
573+
},
574+
},
575+
}
576+
577+
parsed := parseSpec(spec)
578+
Expect(parsed.Repos).To(HaveLen(1))
579+
Expect(parsed.Repos[0].URL).To(Equal("https://github.com/owner/repo.git"))
580+
Expect(parsed.Repos[0].Branch).NotTo(BeNil())
581+
Expect(*parsed.Repos[0].Branch).To(Equal("main"))
582+
Expect(parsed.Repos[0].AutoPush).NotTo(BeNil())
583+
Expect(*parsed.Repos[0].AutoPush).To(BeTrue())
584+
})
585+
586+
It("Should parse autoPush=false from repo", func() {
587+
spec := map[string]interface{}{
588+
"repos": []interface{}{
589+
map[string]interface{}{
590+
"url": "https://github.com/owner/repo.git",
591+
"branch": "develop",
592+
"autoPush": false,
593+
},
594+
},
595+
}
596+
597+
parsed := parseSpec(spec)
598+
Expect(parsed.Repos).To(HaveLen(1))
599+
Expect(parsed.Repos[0].AutoPush).NotTo(BeNil())
600+
Expect(*parsed.Repos[0].AutoPush).To(BeFalse())
601+
})
602+
603+
It("Should handle missing autoPush field", func() {
604+
spec := map[string]interface{}{
605+
"repos": []interface{}{
606+
map[string]interface{}{
607+
"url": "https://github.com/owner/repo.git",
608+
"branch": "main",
609+
},
610+
},
611+
}
612+
613+
parsed := parseSpec(spec)
614+
Expect(parsed.Repos).To(HaveLen(1))
615+
Expect(parsed.Repos[0].AutoPush).To(BeNil())
616+
})
617+
618+
It("Should handle multiple repos with mixed autoPush settings", func() {
619+
spec := map[string]interface{}{
620+
"repos": []interface{}{
621+
map[string]interface{}{
622+
"url": "https://github.com/owner/repo1.git",
623+
"autoPush": true,
624+
},
625+
map[string]interface{}{
626+
"url": "https://github.com/owner/repo2.git",
627+
"autoPush": false,
628+
},
629+
map[string]interface{}{
630+
"url": "https://github.com/owner/repo3.git",
631+
// No autoPush field
632+
},
633+
},
634+
}
635+
636+
parsed := parseSpec(spec)
637+
Expect(parsed.Repos).To(HaveLen(3))
638+
639+
// First repo: autoPush=true
640+
Expect(parsed.Repos[0].AutoPush).NotTo(BeNil())
641+
Expect(*parsed.Repos[0].AutoPush).To(BeTrue())
642+
643+
// Second repo: autoPush=false
644+
Expect(parsed.Repos[1].AutoPush).NotTo(BeNil())
645+
Expect(*parsed.Repos[1].AutoPush).To(BeFalse())
646+
647+
// Third repo: no autoPush field
648+
Expect(parsed.Repos[2].AutoPush).To(BeNil())
649+
})
650+
})
651+
652+
Describe("ExtractSessionContext", func() {
653+
It("Should extract autoPush from repo spec", func() {
654+
spec := map[string]interface{}{
655+
"repos": []interface{}{
656+
map[string]interface{}{
657+
"url": "https://github.com/owner/repo.git",
658+
"branch": "main",
659+
"autoPush": true,
660+
},
661+
},
662+
}
663+
664+
ctx := ExtractSessionContext(spec)
665+
Expect(ctx.Repos).To(HaveLen(1))
666+
Expect(ctx.Repos[0]["url"]).To(Equal("https://github.com/owner/repo.git"))
667+
Expect(ctx.Repos[0]["autoPush"]).To(BeTrue())
668+
})
669+
670+
It("Should handle repos without autoPush field", func() {
671+
spec := map[string]interface{}{
672+
"repos": []interface{}{
673+
map[string]interface{}{
674+
"url": "https://github.com/owner/repo.git",
675+
"branch": "main",
676+
},
677+
},
678+
}
679+
680+
ctx := ExtractSessionContext(spec)
681+
Expect(ctx.Repos).To(HaveLen(1))
682+
Expect(ctx.Repos[0]["autoPush"]).To(BeNil())
683+
})
684+
})
685+
686+
Describe("BoolPtr helper", func() {
687+
It("Should create pointer to true", func() {
688+
ptr := types.BoolPtr(true)
689+
Expect(ptr).NotTo(BeNil())
690+
Expect(*ptr).To(BeTrue())
691+
})
692+
693+
It("Should create pointer to false", func() {
694+
ptr := types.BoolPtr(false)
695+
Expect(ptr).NotTo(BeNil())
696+
Expect(*ptr).To(BeFalse())
697+
})
698+
})
699+
700+
Describe("Error handling", func() {
701+
It("Should handle invalid autoPush type gracefully", func() {
702+
spec := map[string]interface{}{
703+
"repos": []interface{}{
704+
map[string]interface{}{
705+
"url": "https://github.com/owner/repo.git",
706+
"autoPush": "invalid-string", // Should be bool
707+
},
708+
},
709+
}
710+
711+
parsed := parseSpec(spec)
712+
Expect(parsed.Repos).To(HaveLen(1))
713+
// Should skip invalid type and leave AutoPush as nil
714+
Expect(parsed.Repos[0].AutoPush).To(BeNil())
715+
})
716+
717+
It("Should handle autoPush in malformed repo gracefully", func() {
718+
spec := map[string]interface{}{
719+
"repos": []interface{}{
720+
"invalid-string-instead-of-map",
721+
},
722+
}
723+
724+
parsed := parseSpec(spec)
725+
Expect(parsed.Repos).To(BeEmpty())
726+
})
727+
})
728+
729+
Describe("Edge Cases in AddRepo Handler", func() {
730+
It("Should handle autoPush as null value", func() {
731+
spec := map[string]interface{}{
732+
"repos": []interface{}{
733+
map[string]interface{}{
734+
"url": "https://github.com/owner/repo.git",
735+
"branch": "main",
736+
"autoPush": nil,
737+
},
738+
},
739+
}
740+
741+
parsed := parseSpec(spec)
742+
Expect(parsed.Repos).To(HaveLen(1))
743+
// nil autoPush should be treated as not provided
744+
Expect(parsed.Repos[0].AutoPush).To(BeNil())
745+
})
746+
747+
It("Should skip autoPush with invalid type (string)", func() {
748+
spec := map[string]interface{}{
749+
"repos": []interface{}{
750+
map[string]interface{}{
751+
"url": "https://github.com/owner/repo.git",
752+
"branch": "main",
753+
"autoPush": "true", // String instead of bool
754+
},
755+
},
756+
}
757+
758+
parsed := parseSpec(spec)
759+
Expect(parsed.Repos).To(HaveLen(1))
760+
// Invalid type should be skipped, leaving AutoPush as nil
761+
Expect(parsed.Repos[0].AutoPush).To(BeNil())
762+
})
763+
764+
It("Should skip autoPush with invalid type (number)", func() {
765+
spec := map[string]interface{}{
766+
"repos": []interface{}{
767+
map[string]interface{}{
768+
"url": "https://github.com/owner/repo.git",
769+
"branch": "main",
770+
"autoPush": 1, // Number instead of bool
771+
},
772+
},
773+
}
774+
775+
parsed := parseSpec(spec)
776+
Expect(parsed.Repos).To(HaveLen(1))
777+
// Invalid type should be skipped, leaving AutoPush as nil
778+
Expect(parsed.Repos[0].AutoPush).To(BeNil())
779+
})
780+
})
781+
})
524782
})
525783

526784
// Helper functions

components/backend/types/session.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,9 @@ type AgenticSessionSpec struct {
2828

2929
// SimpleRepo represents a simplified repository configuration
3030
type SimpleRepo struct {
31-
URL string `json:"url"`
32-
Branch *string `json:"branch,omitempty"`
31+
URL string `json:"url"`
32+
Branch *string `json:"branch,omitempty"`
33+
AutoPush *bool `json:"autoPush,omitempty"`
3334
}
3435

3536
type AgenticSessionStatus struct {
@@ -53,7 +54,6 @@ type CreateAgenticSessionRequest struct {
5354
ParentSessionID string `json:"parent_session_id,omitempty"`
5455
// Multi-repo support
5556
Repos []SimpleRepo `json:"repos,omitempty"`
56-
AutoPushOnComplete *bool `json:"autoPushOnComplete,omitempty"`
5757
UserContext *UserContext `json:"userContext,omitempty"`
5858
EnvironmentVariables map[string]string `json:"environmentVariables,omitempty"`
5959
Labels map[string]string `json:"labels,omitempty"`

0 commit comments

Comments
 (0)