Skip to content

Commit 53f1bd1

Browse files
committed
fix(acp): preserve explicit cwd overrides
1 parent 15272a5 commit 53f1bd1

8 files changed

Lines changed: 135 additions & 22 deletions

File tree

api/acp_bootstrap.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,7 @@ func (s *server) bootstrapACPConversationBindingWithClient(
481481

482482
updatedConversation, err := s.updateConversationBinding(ctx, conversation.Namespace, conversation.Name, func(current *spritzv1.SpritzConversation) {
483483
now := metav1.Now()
484-
current.Spec.CWD = normalizeConversationOverrideCWD(spritz, current.Spec.CWD)
484+
setConversationCWDOverride(current, normalizeConversationOverrideCWD(spritz, current))
485485
current.Spec.SessionID = effectiveSessionID
486486
current.Spec.AgentInfo = agentInfo
487487
current.Spec.Capabilities = capabilities

api/acp_config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const (
1717
acpConversationLabelValue = "true"
1818
acpConversationSpritzLabelKey = "spritz.sh/spritz-name"
1919
acpConversationOwnerLabelKey = ownerLabelKey
20+
acpConversationExplicitCWDKey = "spritz.sh/acp-cwd-override-explicit"
2021
)
2122

2223
type acpConfig struct {

api/acp_conversations.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,14 @@ func (s *server) updateACPConversation(c echo.Context) error {
165165
}
166166
changed = true
167167
}
168-
if body.CWD != nil && conversation.Spec.CWD != normalizeConversationCWD(*body.CWD) {
169-
conversation.Spec.CWD = normalizeConversationCWD(*body.CWD)
170-
changed = true
168+
if body.CWD != nil {
169+
nextCWD := normalizeConversationCWD(*body.CWD)
170+
nextExplicit := nextCWD != ""
171+
currentExplicit := conversationHasExplicitCWDOverride(conversation)
172+
if conversation.Spec.CWD != nextCWD || currentExplicit != nextExplicit {
173+
setConversationCWDOverride(conversation, *body.CWD)
174+
changed = true
175+
}
171176
}
172177
if changed {
173178
if err := s.client.Update(c.Request().Context(), conversation); err != nil {

api/acp_cwd.go

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,6 @@ import (
99
spritzv1 "spritz.sh/operator/api/v1"
1010
)
1111

12-
var legacyInheritedConversationCWDs = map[string]struct{}{
13-
defaultACPCWD: {},
14-
"/workspace": {},
15-
}
16-
1712
// normalizeConversationCWD trims client input and preserves empty values so the
1813
// conversation resource can distinguish "no override" from an explicit cwd.
1914
func normalizeConversationCWD(value string) string {
@@ -28,25 +23,28 @@ func resolveConversationEffectiveCWD(spritz *spritzv1.Spritz, conversation *spri
2823
if conversation == nil {
2924
return defaultCWD
3025
}
31-
if override := normalizeConversationOverrideCWD(spritz, conversation.Spec.CWD); override != "" {
26+
if override := normalizeConversationOverrideCWD(spritz, conversation); override != "" {
3227
return override
3328
}
3429
return defaultCWD
3530
}
3631

37-
// normalizeConversationOverrideCWD clears legacy copied defaults so bootstrap
38-
// can distinguish a real override from inherited instance state.
39-
func normalizeConversationOverrideCWD(spritz *spritzv1.Spritz, value string) string {
40-
override := normalizeConversationCWD(value)
32+
// normalizeConversationOverrideCWD distinguishes an explicit override from an
33+
// inherited instance default without guessing about ambiguous historical values.
34+
func normalizeConversationOverrideCWD(spritz *spritzv1.Spritz, conversation *spritzv1.SpritzConversation) string {
35+
if conversation == nil {
36+
return ""
37+
}
38+
override := normalizeConversationCWD(conversation.Spec.CWD)
4139
if override == "" {
4240
return ""
4341
}
4442

4543
defaultCWD := resolveSpritzDefaultCWD(spritz)
46-
if override == defaultCWD {
47-
return ""
44+
if conversationHasExplicitCWDOverride(conversation) {
45+
return override
4846
}
49-
if _, ok := legacyInheritedConversationCWDs[override]; ok && override != defaultCWD {
47+
if override == defaultCWD {
5048
return ""
5149
}
5250
return override
@@ -162,3 +160,33 @@ func inferConversationRepoName(raw string) string {
162160
}
163161
return base
164162
}
163+
164+
func conversationHasExplicitCWDOverride(conversation *spritzv1.SpritzConversation) bool {
165+
if conversation == nil || conversation.Annotations == nil {
166+
return false
167+
}
168+
value := strings.TrimSpace(conversation.Annotations[acpConversationExplicitCWDKey])
169+
switch strings.ToLower(value) {
170+
case "1", "true", "yes", "on":
171+
return true
172+
default:
173+
return false
174+
}
175+
}
176+
177+
func setConversationCWDOverride(conversation *spritzv1.SpritzConversation, value string) {
178+
if conversation == nil {
179+
return
180+
}
181+
conversation.Spec.CWD = normalizeConversationCWD(value)
182+
if conversation.Spec.CWD == "" {
183+
if conversation.Annotations != nil {
184+
delete(conversation.Annotations, acpConversationExplicitCWDKey)
185+
}
186+
return
187+
}
188+
if conversation.Annotations == nil {
189+
conversation.Annotations = map[string]string{}
190+
}
191+
conversation.Annotations[acpConversationExplicitCWDKey] = "true"
192+
}

api/acp_helpers.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func buildACPConversationResource(spritz *spritzv1.Spritz, requestedTitle, reque
122122
if title == "" {
123123
title = defaultACPConversationTitle
124124
}
125-
return &spritzv1.SpritzConversation{
125+
conversation := &spritzv1.SpritzConversation{
126126
TypeMeta: metav1.TypeMeta{
127127
APIVersion: spritzv1.GroupVersion.String(),
128128
Kind: "SpritzConversation",
@@ -153,7 +153,9 @@ func buildACPConversationResource(spritz *spritzv1.Spritz, requestedTitle, reque
153153
Status: spritzv1.SpritzConversationStatus{
154154
BindingState: "pending",
155155
},
156-
}, nil
156+
}
157+
setConversationCWDOverride(conversation, requestedCWD)
158+
return conversation, nil
157159
}
158160

159161
func (s *server) getAuthorizedSpritz(ctx context.Context, principal principal, namespace, name string) (*spritzv1.Spritz, error) {

api/acp_test.go

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -699,15 +699,15 @@ func TestBootstrapACPConversationLoadsStoredSessionWithoutMutatingIdentity(t *te
699699
}
700700
}
701701

702-
func TestBootstrapACPConversationNormalizesLegacyHomeCWDToResolvedDefault(t *testing.T) {
702+
func TestBootstrapACPConversationUsesResolvedDefaultWhenOverrideMissing(t *testing.T) {
703703
spritz := readyACPSpritz("tidy-otter", "user-1")
704704
spritz.Spec.Repo = &spritzv1.SpritzRepo{
705705
URL: "https://example.com/open/spritz.git",
706706
Dir: "/workspace/platform",
707707
}
708708
conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now())
709709
conversation.Spec.SessionID = "session-existing"
710-
conversation.Spec.CWD = "/home/dev"
710+
conversation.Spec.CWD = ""
711711
fakeACP := newFakeACPBootstrapServer(t, fakeACPBootstrapServerOptions{})
712712

713713
s := newACPTestServer(t, spritz, conversation)
@@ -746,6 +746,53 @@ func TestBootstrapACPConversationNormalizesLegacyHomeCWDToResolvedDefault(t *tes
746746
}
747747
}
748748

749+
func TestBootstrapACPConversationPreservesExplicitWorkspaceOverride(t *testing.T) {
750+
spritz := readyACPSpritz("tidy-otter", "user-1")
751+
spritz.Spec.Repo = &spritzv1.SpritzRepo{
752+
URL: "https://example.com/open/spritz.git",
753+
Dir: "/workspace/platform",
754+
}
755+
conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now())
756+
conversation.Spec.SessionID = "session-existing"
757+
setConversationCWDOverride(conversation, "/workspace")
758+
fakeACP := newFakeACPBootstrapServer(t, fakeACPBootstrapServerOptions{})
759+
760+
s := newACPTestServer(t, spritz, conversation)
761+
s.acp.instanceURL = func(namespace, name string) string { return fakeACP.url }
762+
763+
e := echo.New()
764+
secured := e.Group("", s.authMiddleware())
765+
secured.POST("/api/acp/conversations/:id/bootstrap", s.bootstrapACPConversation)
766+
767+
req := httptest.NewRequest(http.MethodPost, "/api/acp/conversations/"+conversation.Name+"/bootstrap", nil)
768+
req.Header.Set("X-Spritz-User-Id", "user-1")
769+
rec := httptest.NewRecorder()
770+
e.ServeHTTP(rec, req)
771+
772+
if rec.Code != http.StatusOK {
773+
t.Fatalf("expected status 200, got %d: %s", rec.Code, rec.Body.String())
774+
}
775+
776+
var payload struct {
777+
Status string `json:"status"`
778+
Data struct {
779+
EffectiveCWD string `json:"effectiveCwd"`
780+
} `json:"data"`
781+
}
782+
if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil {
783+
t.Fatalf("failed to decode bootstrap response: %v", err)
784+
}
785+
if payload.Data.EffectiveCWD != "/workspace" {
786+
t.Fatalf("expected explicit cwd override to be preserved, got %q", payload.Data.EffectiveCWD)
787+
}
788+
789+
fakeACP.mu.Lock()
790+
defer fakeACP.mu.Unlock()
791+
if len(fakeACP.loadCWDs) != 1 || fakeACP.loadCWDs[0] != "/workspace" {
792+
t.Fatalf("expected session/load cwd /workspace, got %#v", fakeACP.loadCWDs)
793+
}
794+
}
795+
749796
func TestBootstrapACPConversationRepairsMissingSessionExplicitly(t *testing.T) {
750797
spritz := readyACPSpritz("tidy-otter", "user-1")
751798
conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now())

ui/src/pages/chat.test.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,12 @@ describe('ChatPage draft persistence', () => {
734734
expect.objectContaining({ effectiveCwd: '/workspace/platform' }),
735735
);
736736
});
737+
738+
fireEvent.click(screen.getByRole('button', { name: 'Needs CWD' }));
739+
740+
await waitFor(() => {
741+
expect(countBootstrapCalls('conv-cwd')).toBe(1);
742+
});
737743
});
738744

739745
it('surfaces terminal bootstrap failures without retrying again in the background', async () => {

ui/src/pages/chat.tsx

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,30 @@ export function ChatPage() {
257257
);
258258
}, []);
259259

260+
const applyConversationUpdate = useCallback((conversation: ConversationInfo) => {
261+
setSelectedConversation(conversation);
262+
setAgents((prev) =>
263+
prev.map((group) => {
264+
const sameSpritz = group.spritz.metadata.name === (conversation.spec?.spritzName || '');
265+
const hasConversation = group.conversations.some((item) => item.metadata.name === conversation.metadata.name);
266+
if (!sameSpritz && !hasConversation) {
267+
return group;
268+
}
269+
const nextConversations = hasConversation
270+
? group.conversations.map((item) =>
271+
item.metadata.name === conversation.metadata.name ? conversation : item,
272+
)
273+
: sameSpritz
274+
? [...group.conversations, conversation]
275+
: group.conversations;
276+
return {
277+
...group,
278+
conversations: sortConversationsByRecency(nextConversations),
279+
};
280+
}),
281+
);
282+
}, []);
283+
260284
const {
261285
transcript,
262286
clientReady,
@@ -270,7 +294,7 @@ export function ChatPage() {
270294
conversation: selectedConversation,
271295
apiBaseUrl: config.apiBaseUrl || '',
272296
websocketBaseUrl: config.websocketBaseUrl || '',
273-
onConversationUpdate: setSelectedConversation,
297+
onConversationUpdate: applyConversationUpdate,
274298
onConversationTitle: applyConversationTitle,
275299
});
276300

0 commit comments

Comments
 (0)