From 831352641b38b0facf9a17a2ac39aba263e89f0d Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Sat, 4 Apr 2026 22:56:05 +0200 Subject: [PATCH 1/5] docs: define conversation cwd ownership model --- ...orking-directory-ownership-architecture.md | 321 ++++++++++++++++++ 1 file changed, 321 insertions(+) create mode 100644 docs/2026-04-04-conversation-working-directory-ownership-architecture.md diff --git a/docs/2026-04-04-conversation-working-directory-ownership-architecture.md b/docs/2026-04-04-conversation-working-directory-ownership-architecture.md new file mode 100644 index 0000000..d2dc966 --- /dev/null +++ b/docs/2026-04-04-conversation-working-directory-ownership-architecture.md @@ -0,0 +1,321 @@ +--- +date: 2026-04-04 +author: Onur Solmaz +title: Conversation Working Directory Ownership Architecture +tags: [spritz, acp, conversation, cwd, architecture] +--- + +## Overview + +This document defines the production ownership model for conversation working +directories in Spritz. + +Goal in plain English: + +- keep the runtime or instance as the owner of the default working directory +- keep conversations responsible only for explicit user overrides +- keep the browser UI out of default resolution +- make reconnect and bootstrap behavior consistent across all clients + +The current model lets a saved conversation `cwd` behave like both: + +- an inherited instance default +- a user-selected override + +That conflation makes old values sticky and lets stale client state override a +correct runtime default. + +Spritz should converge on a model with three distinct concepts: + +- instance default working directory +- conversation override working directory +- effective working directory + +The server should resolve the effective value. Clients should render it and may +request an override, but they should not invent or replay defaults on their +own. + +## Problem + +Today Spritz stores `spec.cwd` on `SpritzConversation` and uses it in multiple +places: + +- conversation create and update accept `cwd` +- ACP bootstrap passes `conversation.Spec.CWD` into `session/new` and + `session/load` +- the browser ACP client passes `conversation.spec.cwd` into `session/load` + +That creates one ambiguous field with two incompatible meanings: + +- "what this instance should default to" +- "what this conversation explicitly overrode" + +This causes several failure modes: + +- a stale conversation value can override a correct runtime default +- reconnect behavior may differ across clients +- changing an instance default does not help conversations that copied the old + default into `spec.cwd` +- browser state and server state can disagree about the right path +- migration is hard because inherited defaults and real overrides are mixed + +The core issue is not that clients may send `cwd`. The issue is that Spritz +does not distinguish default from override. + +## Design Principles + +### 1. The instance owns the default + +The default working directory is runtime configuration. + +Examples: + +- repo-backed runtime starts in `/workspace/project` +- home-directory runtime starts in `/home/dev` + +That value belongs to the instance, preset, or runtime configuration layer. + +### 2. The conversation owns only explicit user intent + +If a user deliberately changes directory and wants the conversation to reopen +there, that is conversation state. + +That value is an override, not a default. + +### 3. The server resolves effective state + +The server should compute: + +`effectiveCwd = conversationOverrideCwd ?? instanceDefaultCwd` + +Clients should not reproduce this logic independently. + +### 4. Existing sessions own their current runtime state + +If a session already exists, the runtime should prefer the persisted session +working directory over any reconnect hint. + +Creating a session needs a resolved working directory. + +Loading an existing session should not depend on a client replaying the same +directory correctly. + +### 5. Spec stores intent, status stores resolved truth + +This follows the normal control-plane split: + +- `spec`: what the user or caller asked for +- `status` or bootstrap response: what Spritz resolved and is actually using + +## Target Model + +### Instance default working directory + +Each runtime instance should expose a canonical default working directory. + +Possible sources: + +- preset metadata +- resolved runtime binding data +- runtime environment configuration +- runtime capability metadata if Spritz later exposes it explicitly + +Spritz should resolve one canonical `instanceDefaultCwd` before session +creation or repair. + +### Conversation override working directory + +The conversation should store only an explicit override. + +There are two acceptable ways to model this: + +1. Pragmatic compatibility model: + keep `spec.cwd`, but define it as override-only semantics. +2. Clean schema model: + introduce `spec.cwdOverride` and treat `spec.cwd` as legacy input only. + +For the short term, Spritz can keep `spec.cwd` for compatibility and redefine +its meaning. The important part is the semantics, not the field rename. + +### Effective working directory + +Spritz should compute an effective working directory at bootstrap time and make +it visible to clients. + +Recommended surfaces: + +- `status.effectiveCwd` +- `bootstrap.effectiveCwd` + +If Spritz wants to minimize CRD churn, returning `effectiveCwd` from bootstrap +first is enough to fix reconnect behavior. Publishing it in `status` is still +useful because it makes the resolved state inspectable through standard API +reads. + +## Resolution Rules + +Spritz should resolve working directory with the following rules. + +### Create conversation + +- if request `cwd` is absent, create the conversation with no override +- if request `cwd` is present, validate and normalize it, then store it as an + explicit override + +### Update conversation + +- if request `cwd` is `null` or empty, clear the override +- if request `cwd` is non-empty, validate and normalize it and store it as the + override + +### Bootstrap and repair + +Bootstrap should compute: + +`effectiveCwd = normalizedOverrideCwd ?? resolvedInstanceDefaultCwd` + +Then: + +- if the conversation already has a live session, load it and prefer the + session's own stored cwd where the runtime can provide it +- if the session is missing and must be recreated, create the replacement + session with `effectiveCwd` +- return `effectiveCwd` in the bootstrap response + +### Reconnect + +Clients should use `effectiveCwd` returned by Spritz, not `spec.cwd`, when a +runtime still requires a `cwd` argument during replay. + +Longer term, `session/load` should not require callers to resend cwd for an +existing session. + +## API Contract + +### Conversation API + +The conversation API should treat `cwd` as optional override input. + +Required behavior: + +- no `cwd` means "inherit instance default" +- explicit `cwd` means "use this override" +- empty `cwd` means "clear override" + +### Bootstrap API + +Bootstrap should become the authoritative source for resolved ACP session +metadata. + +It should return: + +- `effectiveSessionId` +- `bindingState` +- `effectiveCwd` +- normalized agent info and capabilities + +That keeps client reconnect logic simple: + +- call bootstrap +- trust bootstrap response +- connect +- do not recompute cwd from local state + +### ACP runtime protocol hardening + +Spritz should also harden its ACP runtime expectations: + +- `session/new` requires a resolved cwd +- `session/load` should use the session's stored cwd when possible +- if `session/load` still accepts `cwd`, treat it as optional compatibility + input rather than the source of truth + +The built-in Codex ACP example already behaves this way for `session/load`: it +replays the stored session and does not need the caller's cwd to resolve it. +Spritz should make that the general expectation for runtimes. + +## Migration Strategy + +Spritz needs a compatibility path for conversations that currently store +copied defaults. + +### Read-time normalization + +As a first step, Spritz should normalize legacy values during bootstrap: + +- values known to be old inherited defaults +- values equal to the resolved instance default +- empty or whitespace-only values + +These should be treated as "no override". + +This read-time normalization fixes behavior before any data migration runs. + +### Data migration + +After read-time normalization exists, Spritz may run a one-time migration that: + +- scans existing conversations +- resolves instance defaults +- clears `spec.cwd` when it only mirrors the default +- preserves `spec.cwd` when it is a true explicit override + +This migration is optional for correctness once bootstrap normalization exists, +but it improves clarity and reduces future ambiguity. + +## Why This Is the Production Model + +This split gives each layer one clear responsibility: + +- instance or runtime config owns defaults +- conversation resource owns explicit user overrides +- server owns resolved truth +- UI owns presentation and override requests + +That gives Spritz the properties a production control plane should want: + +- one source of truth for effective state +- consistent behavior across browser, CLI, and future clients +- safer migrations when runtime defaults change +- less coupling between UI implementation details and runtime behavior +- better observability because resolved state is visible in one place + +## Validation + +The design should be considered correct when all of the following are true: + +- a new conversation with no override starts in the instance default +- an explicit override persists across refresh, reconnect, and second-client + access +- reconnect does not require the browser to rediscover or replay the default + cwd correctly +- changing an instance default affects only conversations without overrides +- repairing a missing ACP session recreates it with the same effective cwd +- old copied defaults no longer override the current instance default + +## Recommended Implementation Order + +### Phase 1 + +- change bootstrap to resolve and return `effectiveCwd` +- normalize copied defaults to "no override" during bootstrap +- update the UI to consume `effectiveCwd` instead of `spec.cwd` + +### Phase 2 + +- expose `status.effectiveCwd` on the conversation resource +- update create and update semantics so `cwd` clearly means override-only + +### Phase 3 + +- harden runtime expectations so `session/load` does not depend on client cwd +- optionally migrate existing conversation data to clear inherited defaults + +## References + +- `docs/2026-03-09-acp-port-and-agent-chat-architecture.md` +- `docs/2026-03-10-acp-adapter-and-runtime-target-architecture.md` +- `docs/2026-03-10-acp-conversation-storage-and-replay-model.md` +- `api/acp_conversations.go` +- `api/acp_bootstrap.go` +- `ui/src/lib/acp-client.ts` From 15272a5efc746a673244a87a59c45bacd44000ef Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Sat, 4 Apr 2026 23:08:49 +0200 Subject: [PATCH 2/5] fix(acp): resolve effective conversation cwd --- api/acp_bootstrap.go | 21 ++- api/acp_cwd.go | 164 ++++++++++++++++++ api/acp_helpers.go | 8 - api/acp_test.go | 131 ++++++++++++++ api/internal_debug_chat.go | 2 +- .../spritz.sh_spritzconversations.yaml | 2 + crd/spritz.sh_spritzconversations.yaml | 2 + .../crds/spritz.sh_spritzconversations.yaml | 2 + operator/api/v1/spritz_types.go | 1 + ui/src/lib/acp-client.test.ts | 24 ++- ui/src/lib/acp-client.ts | 10 +- ui/src/lib/use-chat-connection.ts | 29 +++- ui/src/pages/chat.test.tsx | 69 +++++++- ui/src/types/acp.ts | 1 + 14 files changed, 436 insertions(+), 30 deletions(-) create mode 100644 api/acp_cwd.go diff --git a/api/acp_bootstrap.go b/api/acp_bootstrap.go index 4b2232e..56fa82e 100644 --- a/api/acp_bootstrap.go +++ b/api/acp_bootstrap.go @@ -21,6 +21,7 @@ import ( type acpBootstrapResponse struct { Conversation *spritzv1.SpritzConversation `json:"conversation"` EffectiveSessionID string `json:"effectiveSessionId,omitempty"` + EffectiveCWD string `json:"effectiveCwd,omitempty"` BindingState string `json:"bindingState,omitempty"` Loaded bool `json:"loaded,omitempty"` Replaced bool `json:"replaced,omitempty"` @@ -417,10 +418,16 @@ func (s *server) bootstrapACPConversationBinding(ctx context.Context, conversati return nil, err } - return s.bootstrapACPConversationBindingWithClient(ctx, conversation, client, initResult) + return s.bootstrapACPConversationBindingWithClient(ctx, conversation, spritz, client, initResult) } -func (s *server) bootstrapACPConversationBindingWithClient(ctx context.Context, conversation *spritzv1.SpritzConversation, client *acpBootstrapInstanceClient, initResult *acpBootstrapInitializeResult) (*acpBootstrapResponse, error) { +func (s *server) bootstrapACPConversationBindingWithClient( + ctx context.Context, + conversation *spritzv1.SpritzConversation, + spritz *spritzv1.Spritz, + client *acpBootstrapInstanceClient, + initResult *acpBootstrapInitializeResult, +) (*acpBootstrapResponse, error) { if !initResult.AgentCapabilities.LoadSession { err := errors.New("agent does not support session/load") s.recordConversationBindingError(ctx, conversation.Namespace, conversation.Name, "", err) @@ -429,6 +436,7 @@ func (s *server) bootstrapACPConversationBindingWithClient(ctx context.Context, agentInfo := normalizeBootstrapAgentInfo(initResult) capabilities := normalizeBootstrapCapabilities(initResult) + effectiveCWD := resolveConversationEffectiveCWD(spritz, conversation) effectiveSessionID := strings.TrimSpace(conversation.Spec.SessionID) previousSessionID := "" bindingState := "active" @@ -438,12 +446,12 @@ func (s *server) bootstrapACPConversationBindingWithClient(ctx context.Context, var err error if effectiveSessionID != "" { - replayMessageCount, err = client.loadSession(ctx, effectiveSessionID, normalizeConversationCWD(conversation.Spec.CWD)) + replayMessageCount, err = client.loadSession(ctx, effectiveSessionID, effectiveCWD) if err != nil { var rpcErr *acpBootstrapRPCError if errors.As(err, &rpcErr) && rpcErr.missingSession() { previousSessionID = effectiveSessionID - effectiveSessionID, err = client.newSession(ctx, normalizeConversationCWD(conversation.Spec.CWD)) + effectiveSessionID, err = client.newSession(ctx, effectiveCWD) if err != nil { s.recordConversationBindingError(ctx, conversation.Namespace, conversation.Name, previousSessionID, err) return nil, err @@ -458,7 +466,7 @@ func (s *server) bootstrapACPConversationBindingWithClient(ctx context.Context, loaded = true } } else { - effectiveSessionID, err = client.newSession(ctx, normalizeConversationCWD(conversation.Spec.CWD)) + effectiveSessionID, err = client.newSession(ctx, effectiveCWD) if err != nil { s.recordConversationBindingError(ctx, conversation.Namespace, conversation.Name, "", err) return nil, err @@ -473,11 +481,13 @@ func (s *server) bootstrapACPConversationBindingWithClient(ctx context.Context, updatedConversation, err := s.updateConversationBinding(ctx, conversation.Namespace, conversation.Name, func(current *spritzv1.SpritzConversation) { now := metav1.Now() + current.Spec.CWD = normalizeConversationOverrideCWD(spritz, current.Spec.CWD) current.Spec.SessionID = effectiveSessionID current.Spec.AgentInfo = agentInfo current.Spec.Capabilities = capabilities current.Status.BoundSessionID = effectiveSessionID current.Status.BindingState = bindingState + current.Status.EffectiveCWD = effectiveCWD current.Status.PreviousSessionID = previousSessionID current.Status.LastBoundAt = &now current.Status.LastReplayMessageCount = replayMessageCount @@ -496,6 +506,7 @@ func (s *server) bootstrapACPConversationBindingWithClient(ctx context.Context, return &acpBootstrapResponse{ Conversation: updatedConversation, EffectiveSessionID: effectiveSessionID, + EffectiveCWD: effectiveCWD, BindingState: bindingState, Loaded: loaded, Replaced: replaced, diff --git a/api/acp_cwd.go b/api/acp_cwd.go new file mode 100644 index 0000000..8950cd9 --- /dev/null +++ b/api/acp_cwd.go @@ -0,0 +1,164 @@ +package main + +import ( + "net/url" + "path" + "strconv" + "strings" + + spritzv1 "spritz.sh/operator/api/v1" +) + +var legacyInheritedConversationCWDs = map[string]struct{}{ + defaultACPCWD: {}, + "/workspace": {}, +} + +// normalizeConversationCWD trims client input and preserves empty values so the +// conversation resource can distinguish "no override" from an explicit cwd. +func normalizeConversationCWD(value string) string { + return strings.TrimSpace(value) +} + +// resolveConversationEffectiveCWD resolves the cwd that should be used for ACP +// bootstrap and reconnect flows after accounting for explicit overrides, +// instance defaults, and legacy copied-default values. +func resolveConversationEffectiveCWD(spritz *spritzv1.Spritz, conversation *spritzv1.SpritzConversation) string { + defaultCWD := resolveSpritzDefaultCWD(spritz) + if conversation == nil { + return defaultCWD + } + if override := normalizeConversationOverrideCWD(spritz, conversation.Spec.CWD); override != "" { + return override + } + return defaultCWD +} + +// normalizeConversationOverrideCWD clears legacy copied defaults so bootstrap +// can distinguish a real override from inherited instance state. +func normalizeConversationOverrideCWD(spritz *spritzv1.Spritz, value string) string { + override := normalizeConversationCWD(value) + if override == "" { + return "" + } + + defaultCWD := resolveSpritzDefaultCWD(spritz) + if override == defaultCWD { + return "" + } + if _, ok := legacyInheritedConversationCWDs[override]; ok && override != defaultCWD { + return "" + } + return override +} + +// resolveSpritzDefaultCWD derives the runtime-owned default cwd from explicit +// env overrides first and falls back to the primary repo checkout directory. +func resolveSpritzDefaultCWD(spritz *spritzv1.Spritz) string { + if spritz == nil { + return defaultACPCWD + } + + for _, key := range []string{ + "SPRITZ_CONVERSATION_DEFAULT_CWD", + "SPRITZ_CODEX_WORKDIR", + "SPRITZ_CLAUDE_CODE_WORKDIR", + "SPRITZ_REPO_DIR", + } { + if value := spritzEnvValue(spritz, key); value != "" { + return value + } + } + + if repoDir := resolvePrimaryRepoDir(spritz); repoDir != "" { + return repoDir + } + return defaultACPCWD +} + +func spritzEnvValue(spritz *spritzv1.Spritz, key string) string { + if spritz == nil { + return "" + } + for i := len(spritz.Spec.Env) - 1; i >= 0; i-- { + env := spritz.Spec.Env[i] + if strings.TrimSpace(env.Name) != key { + continue + } + if value := strings.TrimSpace(env.Value); value != "" { + return value + } + } + return "" +} + +func resolvePrimaryRepoDir(spritz *spritzv1.Spritz) string { + if spritz == nil { + return "" + } + + repos := spritz.Spec.Repos + if len(repos) > 0 { + return repoDirForConversationDefault(repos[0], 0, len(repos)) + } + if spritz.Spec.Repo != nil && strings.TrimSpace(spritz.Spec.Repo.URL) != "" { + return repoDirForConversationDefault(*spritz.Spec.Repo, 0, 1) + } + return "" +} + +func repoDirForConversationDefault(repo spritzv1.SpritzRepo, index int, total int) string { + repoDir := strings.TrimSpace(repo.Dir) + if repoDir == "" { + if total > 1 { + repoDir = "/workspace/repo-" + strconv.Itoa(index+1) + } else if inferred := inferConversationRepoName(repo.URL); inferred != "" { + repoDir = path.Join("/workspace", inferred) + } else { + repoDir = "/workspace/repo" + } + } + if !strings.HasPrefix(repoDir, "/") { + repoDir = path.Join("/workspace", repoDir) + } + return path.Clean(repoDir) +} + +func inferConversationRepoName(raw string) string { + value := strings.TrimSpace(raw) + if value == "" { + return "" + } + pathPart := "" + if strings.Contains(value, "://") { + parsed, err := url.Parse(value) + if err != nil { + return "" + } + pathPart = parsed.Path + } else if strings.Contains(value, ":") { + parts := strings.SplitN(value, ":", 2) + if len(parts) == 2 { + pathPart = parts[1] + } else { + pathPart = value + } + } else { + pathPart = value + } + pathPart = strings.SplitN(pathPart, "?", 2)[0] + pathPart = strings.SplitN(pathPart, "#", 2)[0] + pathPart = strings.TrimSuffix(pathPart, "/") + if pathPart == "" { + return "" + } + base := path.Base(pathPart) + if base == "." || base == "/" { + return "" + } + base = strings.TrimSuffix(base, ".git") + if base == "" || base == "." || base == "/" { + return "" + } + return base +} diff --git a/api/acp_helpers.go b/api/acp_helpers.go index 447be35..408f23b 100644 --- a/api/acp_helpers.go +++ b/api/acp_helpers.go @@ -72,14 +72,6 @@ func normalizeConversationCapabilities(status *spritzv1.SpritzACPStatus) *spritz return capabilities } -func normalizeConversationCWD(value string) string { - trimmed := strings.TrimSpace(value) - if trimmed == "" { - return defaultACPCWD - } - return trimmed -} - func conversationDisplayTitle(conversation *spritzv1.SpritzConversation) string { if conversation == nil { return defaultACPConversationTitle diff --git a/api/acp_test.go b/api/acp_test.go index 5ba3b00..fe4c56e 100644 --- a/api/acp_test.go +++ b/api/acp_test.go @@ -82,6 +82,8 @@ type fakeACPBootstrapServer struct { server *httptest.Server mu sync.Mutex loadIDs []string + loadCWDs []string + newCWDs []string newCalls int initRequests []acpBootstrapInitializeRequest } @@ -140,12 +142,14 @@ func newFakeACPBootstrapServer(t *testing.T, options fakeACPBootstrapServerOptio case "session/load": var params struct { SessionID string `json:"sessionId"` + CWD string `json:"cwd"` } if err := json.Unmarshal(message.Params, ¶ms); err != nil { t.Fatalf("failed to decode load params: %v", err) } fakeServer.mu.Lock() fakeServer.loadIDs = append(fakeServer.loadIDs, params.SessionID) + fakeServer.loadCWDs = append(fakeServer.loadCWDs, params.CWD) fakeServer.mu.Unlock() if options.LoadError != nil { if err := conn.WriteJSON(map[string]any{ @@ -176,8 +180,15 @@ func newFakeACPBootstrapServer(t *testing.T, options fakeACPBootstrapServerOptio t.Fatalf("failed to write load result: %v", err) } case "session/new": + var params struct { + CWD string `json:"cwd"` + } + if err := json.Unmarshal(message.Params, ¶ms); err != nil { + t.Fatalf("failed to decode new session params: %v", err) + } fakeServer.mu.Lock() fakeServer.newCalls++ + fakeServer.newCWDs = append(fakeServer.newCWDs, params.CWD) fakeServer.mu.Unlock() sessionID := options.NewSessionID if sessionID == "" { @@ -371,6 +382,36 @@ func TestCreateACPConversationGeneratesIndependentConversationID(t *testing.T) { } } +func TestCreateACPConversationLeavesCWDUnsetWhenRequestOmitsIt(t *testing.T) { + spritz := readyACPSpritz("tidy-otter", "user-1") + s := newACPTestServer(t, spritz) + e := echo.New() + secured := e.Group("", s.authMiddleware()) + secured.POST("/api/acp/conversations", s.createACPConversation) + + body := strings.NewReader(`{"spritzName":"tidy-otter"}`) + req := httptest.NewRequest(http.MethodPost, "/api/acp/conversations", body) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Spritz-User-Id", "user-1") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusCreated { + t.Fatalf("expected status 201, got %d: %s", rec.Code, rec.Body.String()) + } + + var payload struct { + Status string `json:"status"` + Data spritzv1.SpritzConversation `json:"data"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("failed to decode conversation response: %v", err) + } + if payload.Data.Spec.CWD != "" { + t.Fatalf("expected empty cwd override, got %q", payload.Data.Spec.CWD) + } +} + func TestCreateACPConversationRejectsAgentsWithoutLoadSessionSupport(t *testing.T) { spritz := readyACPSpritz("tidy-otter", "user-1") spritz.Status.ACP.Capabilities.LoadSession = false @@ -461,6 +502,46 @@ func TestListAndPatchACPConversationsByID(t *testing.T) { } } +func TestPatchACPConversationClearsCWDOverrideWhenEmpty(t *testing.T) { + spritz := readyACPSpritz("tidy-otter", "user-1") + conversation := conversationFor("tidy-otter-new", "tidy-otter", "user-1", "Latest", metav1.Now()) + conversation.Spec.CWD = "/workspace/app" + + s := newACPTestServer(t, spritz, conversation) + e := echo.New() + secured := e.Group("", s.authMiddleware()) + secured.PATCH("/api/acp/conversations/:id", s.updateACPConversation) + secured.GET("/api/acp/conversations/:id", s.getACPConversation) + + patchReq := httptest.NewRequest(http.MethodPatch, "/api/acp/conversations/"+conversation.Name, strings.NewReader(`{"cwd":""}`)) + patchReq.Header.Set("Content-Type", "application/json") + patchReq.Header.Set("X-Spritz-User-Id", "user-1") + patchRec := httptest.NewRecorder() + e.ServeHTTP(patchRec, patchReq) + if patchRec.Code != http.StatusOK { + t.Fatalf("expected status 200 from patch, got %d: %s", patchRec.Code, patchRec.Body.String()) + } + + getReq := httptest.NewRequest(http.MethodGet, "/api/acp/conversations/"+conversation.Name, nil) + getReq.Header.Set("X-Spritz-User-Id", "user-1") + getRec := httptest.NewRecorder() + e.ServeHTTP(getRec, getReq) + if getRec.Code != http.StatusOK { + t.Fatalf("expected status 200 from get, got %d: %s", getRec.Code, getRec.Body.String()) + } + + var getPayload struct { + Status string `json:"status"` + Data spritzv1.SpritzConversation `json:"data"` + } + if err := json.Unmarshal(getRec.Body.Bytes(), &getPayload); err != nil { + t.Fatalf("failed to decode get response: %v", err) + } + if getPayload.Data.Spec.CWD != "" { + t.Fatalf("expected cleared cwd override, got %#v", getPayload.Data.Spec) + } +} + func TestListACPConversationsAllowsAdminToSeeAllOwners(t *testing.T) { now := metav1.Now() spritz := readyACPSpritz("tidy-otter", "user-1") @@ -610,11 +691,61 @@ func TestBootstrapACPConversationLoadsStoredSessionWithoutMutatingIdentity(t *te if len(fakeACP.loadIDs) != 1 || fakeACP.loadIDs[0] != "session-existing" { t.Fatalf("expected session/load for session-existing, got %#v", fakeACP.loadIDs) } + if len(fakeACP.loadCWDs) != 1 || fakeACP.loadCWDs[0] != "/home/dev" { + t.Fatalf("expected session/load cwd /home/dev, got %#v", fakeACP.loadCWDs) + } if fakeACP.newCalls != 0 { t.Fatalf("expected no session/new calls, got %d", fakeACP.newCalls) } } +func TestBootstrapACPConversationNormalizesLegacyHomeCWDToResolvedDefault(t *testing.T) { + spritz := readyACPSpritz("tidy-otter", "user-1") + spritz.Spec.Repo = &spritzv1.SpritzRepo{ + URL: "https://example.com/open/spritz.git", + Dir: "/workspace/platform", + } + conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now()) + conversation.Spec.SessionID = "session-existing" + conversation.Spec.CWD = "/home/dev" + fakeACP := newFakeACPBootstrapServer(t, fakeACPBootstrapServerOptions{}) + + s := newACPTestServer(t, spritz, conversation) + s.acp.instanceURL = func(namespace, name string) string { return fakeACP.url } + + e := echo.New() + secured := e.Group("", s.authMiddleware()) + secured.POST("/api/acp/conversations/:id/bootstrap", s.bootstrapACPConversation) + + req := httptest.NewRequest(http.MethodPost, "/api/acp/conversations/"+conversation.Name+"/bootstrap", nil) + req.Header.Set("X-Spritz-User-Id", "user-1") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", rec.Code, rec.Body.String()) + } + + var payload struct { + Status string `json:"status"` + Data struct { + EffectiveCWD string `json:"effectiveCwd"` + } `json:"data"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("failed to decode bootstrap response: %v", err) + } + if payload.Data.EffectiveCWD != "/workspace/platform" { + t.Fatalf("expected effective cwd /workspace/platform, got %q", payload.Data.EffectiveCWD) + } + + fakeACP.mu.Lock() + defer fakeACP.mu.Unlock() + if len(fakeACP.loadCWDs) != 1 || fakeACP.loadCWDs[0] != "/workspace/platform" { + t.Fatalf("expected session/load cwd /workspace/platform, got %#v", fakeACP.loadCWDs) + } +} + func TestBootstrapACPConversationRepairsMissingSessionExplicitly(t *testing.T) { spritz := readyACPSpritz("tidy-otter", "user-1") conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now()) diff --git a/api/internal_debug_chat.go b/api/internal_debug_chat.go index 2419c4e..6822a96 100644 --- a/api/internal_debug_chat.go +++ b/api/internal_debug_chat.go @@ -227,7 +227,7 @@ func (s *server) runInternalDebugChat(ctx context.Context, conversation *spritzv return nil, nil, err } - bootstrap, err := s.bootstrapACPConversationBindingWithClient(bootstrapCtx, conversation, client, initResult) + bootstrap, err := s.bootstrapACPConversationBindingWithClient(bootstrapCtx, conversation, spritz, client, initResult) if err != nil { return nil, nil, err } diff --git a/crd/generated/spritz.sh_spritzconversations.yaml b/crd/generated/spritz.sh_spritzconversations.yaml index 55db34f..4f64c13 100644 --- a/crd/generated/spritz.sh_spritzconversations.yaml +++ b/crd/generated/spritz.sh_spritzconversations.yaml @@ -133,6 +133,8 @@ spec: type: string boundSessionId: type: string + effectiveCwd: + type: string lastBoundAt: format: date-time type: string diff --git a/crd/spritz.sh_spritzconversations.yaml b/crd/spritz.sh_spritzconversations.yaml index 55db34f..4f64c13 100644 --- a/crd/spritz.sh_spritzconversations.yaml +++ b/crd/spritz.sh_spritzconversations.yaml @@ -133,6 +133,8 @@ spec: type: string boundSessionId: type: string + effectiveCwd: + type: string lastBoundAt: format: date-time type: string diff --git a/helm/spritz/crds/spritz.sh_spritzconversations.yaml b/helm/spritz/crds/spritz.sh_spritzconversations.yaml index 55db34f..4f64c13 100644 --- a/helm/spritz/crds/spritz.sh_spritzconversations.yaml +++ b/helm/spritz/crds/spritz.sh_spritzconversations.yaml @@ -133,6 +133,8 @@ spec: type: string boundSessionId: type: string + effectiveCwd: + type: string lastBoundAt: format: date-time type: string diff --git a/operator/api/v1/spritz_types.go b/operator/api/v1/spritz_types.go index af19e44..632f44c 100644 --- a/operator/api/v1/spritz_types.go +++ b/operator/api/v1/spritz_types.go @@ -281,6 +281,7 @@ type SpritzConversationStatus struct { // +kubebuilder:validation:Enum=pending;active;missing;replaced;error BindingState string `json:"bindingState,omitempty"` BoundSessionID string `json:"boundSessionId,omitempty"` + EffectiveCWD string `json:"effectiveCwd,omitempty"` PreviousSessionID string `json:"previousSessionId,omitempty"` LastBoundAt *metav1.Time `json:"lastBoundAt,omitempty"` LastReplayAt *metav1.Time `json:"lastReplayAt,omitempty"` diff --git a/ui/src/lib/acp-client.test.ts b/ui/src/lib/acp-client.test.ts index e6001fb..730863b 100644 --- a/ui/src/lib/acp-client.test.ts +++ b/ui/src/lib/acp-client.test.ts @@ -66,10 +66,11 @@ describe('createACPClient', () => { vi.restoreAllMocks(); }); - const CONVERSATION: ConversationInfo = { + const CONVERSATION = { metadata: { name: 'conv-1' }, - spec: { sessionId: 'sess-1' }, - }; + spec: { sessionId: 'sess-1', cwd: '/home/dev' }, + status: { effectiveCwd: '/workspace/platform' }, + } as unknown as ConversationInfo; function createTestClient(overrides: Partial = {}) { return createACPClient({ @@ -195,6 +196,23 @@ describe('createACPClient', () => { expect(onReplayStateChange).toHaveBeenNthCalledWith(2, false); }); + it('uses the backend-resolved effective cwd for session replay', async () => { + const client = createTestClient(); + const startPromise = client.start(); + lastWs.simulateOpen(); + + const initializeMessage = JSON.parse(lastWs.sent[0]); + lastWs.simulateMessage({ jsonrpc: '2.0', id: initializeMessage.id, result: {} }); + await flushMicrotasks(); + + const sessionLoadMessage = JSON.parse(lastWs.sent[1]); + expect(sessionLoadMessage.method).toBe('session/load'); + expect(sessionLoadMessage.params.cwd).toBe('/workspace/platform'); + + lastWs.simulateMessage({ jsonrpc: '2.0', id: sessionLoadMessage.id, result: {} }); + await startPromise; + }); + it('dispatches session/request_permission to onPermissionRequest', async () => { const onPermissionRequest = vi.fn(); await startClient({ onPermissionRequest }); diff --git a/ui/src/lib/acp-client.ts b/ui/src/lib/acp-client.ts index 887345c..591e12f 100644 --- a/ui/src/lib/acp-client.ts +++ b/ui/src/lib/acp-client.ts @@ -69,6 +69,7 @@ export function createACPClient(options: ACPClientOptions): ACPClient { let replaying = false; const pending = new Map void; reject: (e: Error) => void; method: string }>(); const sessionId = conversation?.spec?.sessionId || ''; + const effectiveCwd = String(conversation?.status?.effectiveCwd || '').trim(); function cleanupPending(error: Error) { pending.forEach(({ reject }) => reject(error)); @@ -182,11 +183,14 @@ export function createACPClient(options: ACPClientOptions): ACPClient { replaying = true; onReplayStateChange?.(true); try { - await requestRPC('session/load', { + const loadParams: Record = { sessionId, - cwd: conversation?.spec?.cwd || '/workspace', mcpServers: [], - }); + }; + if (effectiveCwd) { + loadParams.cwd = effectiveCwd; + } + await requestRPC('session/load', loadParams); } finally { replaying = false; onReplayStateChange?.(false); diff --git a/ui/src/lib/use-chat-connection.ts b/ui/src/lib/use-chat-connection.ts index 7b5d9a2..74f029e 100644 --- a/ui/src/lib/use-chat-connection.ts +++ b/ui/src/lib/use-chat-connection.ts @@ -129,6 +129,8 @@ export function useChatConnection({ if (forceBootstrap) return true; const sessionId = String(conv.spec?.sessionId || '').trim(); if (!sessionId) return true; + const effectiveCwd = String(conv.status?.effectiveCwd || '').trim(); + if (!effectiveCwd) return true; return String(conv.status?.bindingState || '').trim().toLowerCase() !== 'active'; } @@ -206,6 +208,7 @@ export function useChatConnection({ if (cancelled) return; const newSessionId = String(bootstrapData.effectiveSessionId || ''); + const newEffectiveCwd = String(bootstrapData.effectiveCwd || '').trim(); const replaced = Boolean(bootstrapData.replaced) || (effectiveSessionId && newSessionId && effectiveSessionId !== newSessionId); @@ -215,10 +218,30 @@ export function useChatConnection({ } effectiveSessionId = newSessionId; + const bootstrapConversation = bootstrapData.conversation as ConversationInfo | undefined; effectiveConversation = { - metadata: activeConversation.metadata, - spec: { ...activeConversation.spec, sessionId: effectiveSessionId }, - status: { ...activeConversation.status, bindingState: 'active' }, + metadata: { + ...activeConversation.metadata, + ...(bootstrapConversation?.metadata || {}), + }, + spec: { + ...activeConversation.spec, + ...(bootstrapConversation?.spec || {}), + sessionId: effectiveSessionId, + }, + status: { + ...activeConversation.status, + ...(bootstrapConversation?.status || {}), + bindingState: String( + bootstrapData.bindingState || + bootstrapConversation?.status?.bindingState || + 'active', + ), + effectiveCwd: + newEffectiveCwd || + String(bootstrapConversation?.status?.effectiveCwd || '').trim() || + String(activeConversation.status?.effectiveCwd || '').trim(), + }, }; onConversationUpdate(effectiveConversation); } diff --git a/ui/src/pages/chat.test.tsx b/ui/src/pages/chat.test.tsx index 40691f0..4070afc 100644 --- a/ui/src/pages/chat.test.tsx +++ b/ui/src/pages/chat.test.tsx @@ -7,6 +7,7 @@ import { createMockStorage } from '@/test/helpers'; import { ConfigProvider, config, resolveConfig, type RawSpritzConfig } from '@/lib/config'; import { NoticeProvider } from '@/components/notice-banner'; import { ChatPage } from './chat'; +import type { ConversationInfo } from '@/types/acp'; const { requestMock, @@ -257,16 +258,16 @@ vi.mock('@/components/ui/tooltip', () => ({ }) => <>{render || children}, })); -const CONVERSATIONS = [ +const CONVERSATIONS: ConversationInfo[] = [ { metadata: { name: 'conv-1' }, spec: { sessionId: 'sess-1', title: 'Conversation One', spritzName: 'covo' }, - status: { bindingState: 'active' }, + status: { bindingState: 'active', effectiveCwd: '/workspace/repo' }, }, { metadata: { name: 'conv-2' }, spec: { sessionId: 'sess-2', title: 'Conversation Two', spritzName: 'covo' }, - status: { bindingState: 'active' }, + status: { bindingState: 'active', effectiveCwd: '/workspace/repo' }, }, ]; @@ -310,10 +311,10 @@ function createSpritz( } function createConversation( - overrides: Partial<(typeof CONVERSATIONS)[number]> & { - metadata?: Partial<(typeof CONVERSATIONS)[number]['metadata']>; - spec?: Partial<(typeof CONVERSATIONS)[number]['spec']>; - status?: Partial<(typeof CONVERSATIONS)[number]['status'] & { lastActivityAt?: string }>; + overrides: Partial & { + metadata?: Partial; + spec?: Partial; + status?: Partial>; } = {}, ) { return { @@ -326,6 +327,7 @@ function createConversation( }, status: { bindingState: 'active', + effectiveCwd: '/workspace/repo', ...(overrides.status || {}), }, }; @@ -681,6 +683,59 @@ describe('ChatPage draft persistence', () => { }, { timeout: 4000 }); }); + it('bootstraps active conversations that still lack an effective cwd', async () => { + const conversation = createConversation({ + metadata: { name: 'conv-cwd' }, + spec: { sessionId: 'sess-cwd', title: 'Needs CWD', spritzName: 'covo' }, + status: { bindingState: 'active', effectiveCwd: '' }, + }); + setupRequestMock({ conversations: [conversation] as typeof CONVERSATIONS }); + requestMock.mockImplementation((path: string, options?: { method?: string }) => { + if (path === '/spritzes') { + return Promise.resolve({ items: [createSpritz()] }); + } + if (path === '/acp/conversations?spritz=covo') { + return Promise.resolve({ items: [conversation] }); + } + if (path === '/acp/conversations/conv-cwd/bootstrap' && options?.method === 'POST') { + return Promise.resolve({ + effectiveSessionId: 'sess-cwd', + effectiveCwd: '/workspace/platform', + conversation: createConversation({ + metadata: { name: 'conv-cwd' }, + spec: { sessionId: 'sess-cwd', title: 'Needs CWD', spritzName: 'covo' }, + status: { bindingState: 'active', effectiveCwd: '/workspace/platform' }, + }), + }); + } + if (path === '/acp/conversations/conv-cwd/connect-ticket' && options?.method === 'POST') { + return Promise.resolve({ + type: 'connect-ticket', + ticket: 'ticket-123', + expiresAt: '2026-03-30T12:34:56Z', + protocol: 'spritz-acp.v1', + connectPath: '/api/acp/conversations/conv-cwd/connect', + }); + } + return Promise.resolve({}); + }); + + renderChatPage('/c/covo/conv-cwd', { + apiBaseUrl: '/api', + auth: { + mode: 'bearer', + tokenStorageKeys: 'spritz-token', + }, + }); + + await waitFor(() => { + expect(countBootstrapCalls('conv-cwd')).toBe(1); + expect((getLastACPOptions()?.conversation as Record)?.status).toEqual( + expect.objectContaining({ effectiveCwd: '/workspace/platform' }), + ); + }); + }); + it('surfaces terminal bootstrap failures without retrying again in the background', async () => { setupBootstrapTerminalFailureMock('conv-terminal', 'Terminal Me'); diff --git a/ui/src/types/acp.ts b/ui/src/types/acp.ts index e92577a..7440639 100644 --- a/ui/src/types/acp.ts +++ b/ui/src/types/acp.ts @@ -64,6 +64,7 @@ export interface ConversationInfo { status?: { bindingState?: string; lastActivityAt?: string; + effectiveCwd?: string; }; } From 53f1bd1d33c0b2dafc737693d62f202ea6ab5102 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Sat, 4 Apr 2026 23:21:50 +0200 Subject: [PATCH 3/5] fix(acp): preserve explicit cwd overrides --- api/acp_bootstrap.go | 2 +- api/acp_config.go | 1 + api/acp_conversations.go | 11 +++++--- api/acp_cwd.go | 54 +++++++++++++++++++++++++++++--------- api/acp_helpers.go | 6 +++-- api/acp_test.go | 51 +++++++++++++++++++++++++++++++++-- ui/src/pages/chat.test.tsx | 6 +++++ ui/src/pages/chat.tsx | 26 +++++++++++++++++- 8 files changed, 135 insertions(+), 22 deletions(-) diff --git a/api/acp_bootstrap.go b/api/acp_bootstrap.go index 56fa82e..657a1d5 100644 --- a/api/acp_bootstrap.go +++ b/api/acp_bootstrap.go @@ -481,7 +481,7 @@ func (s *server) bootstrapACPConversationBindingWithClient( updatedConversation, err := s.updateConversationBinding(ctx, conversation.Namespace, conversation.Name, func(current *spritzv1.SpritzConversation) { now := metav1.Now() - current.Spec.CWD = normalizeConversationOverrideCWD(spritz, current.Spec.CWD) + setConversationCWDOverride(current, normalizeConversationOverrideCWD(spritz, current)) current.Spec.SessionID = effectiveSessionID current.Spec.AgentInfo = agentInfo current.Spec.Capabilities = capabilities diff --git a/api/acp_config.go b/api/acp_config.go index 175138f..01013a1 100644 --- a/api/acp_config.go +++ b/api/acp_config.go @@ -17,6 +17,7 @@ const ( acpConversationLabelValue = "true" acpConversationSpritzLabelKey = "spritz.sh/spritz-name" acpConversationOwnerLabelKey = ownerLabelKey + acpConversationExplicitCWDKey = "spritz.sh/acp-cwd-override-explicit" ) type acpConfig struct { diff --git a/api/acp_conversations.go b/api/acp_conversations.go index 23821f5..2a6da9b 100644 --- a/api/acp_conversations.go +++ b/api/acp_conversations.go @@ -165,9 +165,14 @@ func (s *server) updateACPConversation(c echo.Context) error { } changed = true } - if body.CWD != nil && conversation.Spec.CWD != normalizeConversationCWD(*body.CWD) { - conversation.Spec.CWD = normalizeConversationCWD(*body.CWD) - changed = true + if body.CWD != nil { + nextCWD := normalizeConversationCWD(*body.CWD) + nextExplicit := nextCWD != "" + currentExplicit := conversationHasExplicitCWDOverride(conversation) + if conversation.Spec.CWD != nextCWD || currentExplicit != nextExplicit { + setConversationCWDOverride(conversation, *body.CWD) + changed = true + } } if changed { if err := s.client.Update(c.Request().Context(), conversation); err != nil { diff --git a/api/acp_cwd.go b/api/acp_cwd.go index 8950cd9..c64257a 100644 --- a/api/acp_cwd.go +++ b/api/acp_cwd.go @@ -9,11 +9,6 @@ import ( spritzv1 "spritz.sh/operator/api/v1" ) -var legacyInheritedConversationCWDs = map[string]struct{}{ - defaultACPCWD: {}, - "/workspace": {}, -} - // normalizeConversationCWD trims client input and preserves empty values so the // conversation resource can distinguish "no override" from an explicit cwd. func normalizeConversationCWD(value string) string { @@ -28,25 +23,28 @@ func resolveConversationEffectiveCWD(spritz *spritzv1.Spritz, conversation *spri if conversation == nil { return defaultCWD } - if override := normalizeConversationOverrideCWD(spritz, conversation.Spec.CWD); override != "" { + if override := normalizeConversationOverrideCWD(spritz, conversation); override != "" { return override } return defaultCWD } -// normalizeConversationOverrideCWD clears legacy copied defaults so bootstrap -// can distinguish a real override from inherited instance state. -func normalizeConversationOverrideCWD(spritz *spritzv1.Spritz, value string) string { - override := normalizeConversationCWD(value) +// normalizeConversationOverrideCWD distinguishes an explicit override from an +// inherited instance default without guessing about ambiguous historical values. +func normalizeConversationOverrideCWD(spritz *spritzv1.Spritz, conversation *spritzv1.SpritzConversation) string { + if conversation == nil { + return "" + } + override := normalizeConversationCWD(conversation.Spec.CWD) if override == "" { return "" } defaultCWD := resolveSpritzDefaultCWD(spritz) - if override == defaultCWD { - return "" + if conversationHasExplicitCWDOverride(conversation) { + return override } - if _, ok := legacyInheritedConversationCWDs[override]; ok && override != defaultCWD { + if override == defaultCWD { return "" } return override @@ -162,3 +160,33 @@ func inferConversationRepoName(raw string) string { } return base } + +func conversationHasExplicitCWDOverride(conversation *spritzv1.SpritzConversation) bool { + if conversation == nil || conversation.Annotations == nil { + return false + } + value := strings.TrimSpace(conversation.Annotations[acpConversationExplicitCWDKey]) + switch strings.ToLower(value) { + case "1", "true", "yes", "on": + return true + default: + return false + } +} + +func setConversationCWDOverride(conversation *spritzv1.SpritzConversation, value string) { + if conversation == nil { + return + } + conversation.Spec.CWD = normalizeConversationCWD(value) + if conversation.Spec.CWD == "" { + if conversation.Annotations != nil { + delete(conversation.Annotations, acpConversationExplicitCWDKey) + } + return + } + if conversation.Annotations == nil { + conversation.Annotations = map[string]string{} + } + conversation.Annotations[acpConversationExplicitCWDKey] = "true" +} diff --git a/api/acp_helpers.go b/api/acp_helpers.go index 408f23b..0b8319c 100644 --- a/api/acp_helpers.go +++ b/api/acp_helpers.go @@ -122,7 +122,7 @@ func buildACPConversationResource(spritz *spritzv1.Spritz, requestedTitle, reque if title == "" { title = defaultACPConversationTitle } - return &spritzv1.SpritzConversation{ + conversation := &spritzv1.SpritzConversation{ TypeMeta: metav1.TypeMeta{ APIVersion: spritzv1.GroupVersion.String(), Kind: "SpritzConversation", @@ -153,7 +153,9 @@ func buildACPConversationResource(spritz *spritzv1.Spritz, requestedTitle, reque Status: spritzv1.SpritzConversationStatus{ BindingState: "pending", }, - }, nil + } + setConversationCWDOverride(conversation, requestedCWD) + return conversation, nil } func (s *server) getAuthorizedSpritz(ctx context.Context, principal principal, namespace, name string) (*spritzv1.Spritz, error) { diff --git a/api/acp_test.go b/api/acp_test.go index fe4c56e..18d306c 100644 --- a/api/acp_test.go +++ b/api/acp_test.go @@ -699,7 +699,7 @@ func TestBootstrapACPConversationLoadsStoredSessionWithoutMutatingIdentity(t *te } } -func TestBootstrapACPConversationNormalizesLegacyHomeCWDToResolvedDefault(t *testing.T) { +func TestBootstrapACPConversationUsesResolvedDefaultWhenOverrideMissing(t *testing.T) { spritz := readyACPSpritz("tidy-otter", "user-1") spritz.Spec.Repo = &spritzv1.SpritzRepo{ URL: "https://example.com/open/spritz.git", @@ -707,7 +707,7 @@ func TestBootstrapACPConversationNormalizesLegacyHomeCWDToResolvedDefault(t *tes } conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now()) conversation.Spec.SessionID = "session-existing" - conversation.Spec.CWD = "/home/dev" + conversation.Spec.CWD = "" fakeACP := newFakeACPBootstrapServer(t, fakeACPBootstrapServerOptions{}) s := newACPTestServer(t, spritz, conversation) @@ -746,6 +746,53 @@ func TestBootstrapACPConversationNormalizesLegacyHomeCWDToResolvedDefault(t *tes } } +func TestBootstrapACPConversationPreservesExplicitWorkspaceOverride(t *testing.T) { + spritz := readyACPSpritz("tidy-otter", "user-1") + spritz.Spec.Repo = &spritzv1.SpritzRepo{ + URL: "https://example.com/open/spritz.git", + Dir: "/workspace/platform", + } + conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now()) + conversation.Spec.SessionID = "session-existing" + setConversationCWDOverride(conversation, "/workspace") + fakeACP := newFakeACPBootstrapServer(t, fakeACPBootstrapServerOptions{}) + + s := newACPTestServer(t, spritz, conversation) + s.acp.instanceURL = func(namespace, name string) string { return fakeACP.url } + + e := echo.New() + secured := e.Group("", s.authMiddleware()) + secured.POST("/api/acp/conversations/:id/bootstrap", s.bootstrapACPConversation) + + req := httptest.NewRequest(http.MethodPost, "/api/acp/conversations/"+conversation.Name+"/bootstrap", nil) + req.Header.Set("X-Spritz-User-Id", "user-1") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", rec.Code, rec.Body.String()) + } + + var payload struct { + Status string `json:"status"` + Data struct { + EffectiveCWD string `json:"effectiveCwd"` + } `json:"data"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("failed to decode bootstrap response: %v", err) + } + if payload.Data.EffectiveCWD != "/workspace" { + t.Fatalf("expected explicit cwd override to be preserved, got %q", payload.Data.EffectiveCWD) + } + + fakeACP.mu.Lock() + defer fakeACP.mu.Unlock() + if len(fakeACP.loadCWDs) != 1 || fakeACP.loadCWDs[0] != "/workspace" { + t.Fatalf("expected session/load cwd /workspace, got %#v", fakeACP.loadCWDs) + } +} + func TestBootstrapACPConversationRepairsMissingSessionExplicitly(t *testing.T) { spritz := readyACPSpritz("tidy-otter", "user-1") conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now()) diff --git a/ui/src/pages/chat.test.tsx b/ui/src/pages/chat.test.tsx index 4070afc..fe640ff 100644 --- a/ui/src/pages/chat.test.tsx +++ b/ui/src/pages/chat.test.tsx @@ -734,6 +734,12 @@ describe('ChatPage draft persistence', () => { expect.objectContaining({ effectiveCwd: '/workspace/platform' }), ); }); + + fireEvent.click(screen.getByRole('button', { name: 'Needs CWD' })); + + await waitFor(() => { + expect(countBootstrapCalls('conv-cwd')).toBe(1); + }); }); it('surfaces terminal bootstrap failures without retrying again in the background', async () => { diff --git a/ui/src/pages/chat.tsx b/ui/src/pages/chat.tsx index dfc8793..076fba0 100644 --- a/ui/src/pages/chat.tsx +++ b/ui/src/pages/chat.tsx @@ -257,6 +257,30 @@ export function ChatPage() { ); }, []); + const applyConversationUpdate = useCallback((conversation: ConversationInfo) => { + setSelectedConversation(conversation); + setAgents((prev) => + prev.map((group) => { + const sameSpritz = group.spritz.metadata.name === (conversation.spec?.spritzName || ''); + const hasConversation = group.conversations.some((item) => item.metadata.name === conversation.metadata.name); + if (!sameSpritz && !hasConversation) { + return group; + } + const nextConversations = hasConversation + ? group.conversations.map((item) => + item.metadata.name === conversation.metadata.name ? conversation : item, + ) + : sameSpritz + ? [...group.conversations, conversation] + : group.conversations; + return { + ...group, + conversations: sortConversationsByRecency(nextConversations), + }; + }), + ); + }, []); + const { transcript, clientReady, @@ -270,7 +294,7 @@ export function ChatPage() { conversation: selectedConversation, apiBaseUrl: config.apiBaseUrl || '', websocketBaseUrl: config.websocketBaseUrl || '', - onConversationUpdate: setSelectedConversation, + onConversationUpdate: applyConversationUpdate, onConversationTitle: applyConversationTitle, }); From 5bb2663522a4ee90387b17955efe7689b438e96f Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Sat, 4 Apr 2026 23:30:51 +0200 Subject: [PATCH 4/5] fix(acp): rebind conversations after cwd changes --- api/acp_bootstrap.go | 6 ++- api/acp_conversations.go | 56 ++++++++++++++------- api/acp_cwd.go | 3 ++ api/acp_test.go | 102 +++++++++++++++++++++++++++++++++++++-- 4 files changed, 145 insertions(+), 22 deletions(-) diff --git a/api/acp_bootstrap.go b/api/acp_bootstrap.go index 657a1d5..5a4f3a9 100644 --- a/api/acp_bootstrap.go +++ b/api/acp_bootstrap.go @@ -538,18 +538,20 @@ func (s *server) updateConversationBinding(ctx context.Context, namespace, name } beforeSpec := current.Spec beforeStatus := current.Status + beforeAnnotations := cloneStringMap(current.Annotations) mutate(current) specChanged := !apiequality.Semantic.DeepEqual(beforeSpec, current.Spec) statusChanged := !apiequality.Semantic.DeepEqual(beforeStatus, current.Status) + annotationsChanged := !apiequality.Semantic.DeepEqual(beforeAnnotations, current.Annotations) desiredStatus := current.Status - if specChanged { + if specChanged || annotationsChanged { if err := s.client.Update(ctx, current); err != nil { return err } } if statusChanged { statusTarget := current - if specChanged { + if specChanged || annotationsChanged { statusTarget = &spritzv1.SpritzConversation{} if err := s.client.Get(ctx, clientKey(namespace, name), statusTarget); err != nil { return err diff --git a/api/acp_conversations.go b/api/acp_conversations.go index 2a6da9b..b38f5dd 100644 --- a/api/acp_conversations.go +++ b/api/acp_conversations.go @@ -9,6 +9,7 @@ import ( "github.com/labstack/echo/v4" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" spritzv1 "spritz.sh/operator/api/v1" @@ -157,29 +158,50 @@ func (s *server) updateACPConversation(c echo.Context) error { return writeError(c, http.StatusBadRequest, err.Error()) } - changed := false - if body.Title != nil && conversation.Spec.Title != strings.TrimSpace(*body.Title) { - conversation.Spec.Title = strings.TrimSpace(*body.Title) - if conversation.Spec.Title == "" { - conversation.Spec.Title = defaultACPConversationTitle - } - changed = true + if body.Title == nil && body.CWD == nil { + return writeJSON(c, http.StatusOK, conversation) } - if body.CWD != nil { + + updatedConversation, err := s.updateConversationBinding(c.Request().Context(), conversation.Namespace, conversation.Name, func(current *spritzv1.SpritzConversation) { + if body.Title != nil && current.Spec.Title != strings.TrimSpace(*body.Title) { + current.Spec.Title = strings.TrimSpace(*body.Title) + if current.Spec.Title == "" { + current.Spec.Title = defaultACPConversationTitle + } + } + if body.CWD == nil { + return + } + nextCWD := normalizeConversationCWD(*body.CWD) nextExplicit := nextCWD != "" - currentExplicit := conversationHasExplicitCWDOverride(conversation) - if conversation.Spec.CWD != nextCWD || currentExplicit != nextExplicit { - setConversationCWDOverride(conversation, *body.CWD) - changed = true + currentExplicit := conversationHasExplicitCWDOverride(current) + if current.Spec.CWD == nextCWD && currentExplicit == nextExplicit { + return } - } - if changed { - if err := s.client.Update(c.Request().Context(), conversation); err != nil { - return writeError(c, http.StatusInternalServerError, err.Error()) + + setConversationCWDOverride(current, *body.CWD) + + previousSessionID := strings.TrimSpace(current.Status.BoundSessionID) + if previousSessionID == "" { + previousSessionID = strings.TrimSpace(current.Spec.SessionID) } + current.Spec.SessionID = "" + current.Status.BindingState = "pending" + current.Status.BoundSessionID = "" + current.Status.EffectiveCWD = "" + current.Status.PreviousSessionID = previousSessionID + current.Status.LastBoundAt = nil + current.Status.LastReplayAt = nil + current.Status.LastReplayMessageCount = 0 + current.Status.LastError = "" + now := metav1.Now() + current.Status.UpdatedAt = &now + }) + if err != nil { + return writeError(c, http.StatusInternalServerError, err.Error()) } - return writeJSON(c, http.StatusOK, conversation) + return writeJSON(c, http.StatusOK, updatedConversation) } func decodeACPBody(c echo.Context, target any) error { diff --git a/api/acp_cwd.go b/api/acp_cwd.go index c64257a..4c5768b 100644 --- a/api/acp_cwd.go +++ b/api/acp_cwd.go @@ -47,6 +47,9 @@ func normalizeConversationOverrideCWD(spritz *spritzv1.Spritz, conversation *spr if override == defaultCWD { return "" } + if override == defaultACPCWD { + return "" + } return override } diff --git a/api/acp_test.go b/api/acp_test.go index 18d306c..60210f9 100644 --- a/api/acp_test.go +++ b/api/acp_test.go @@ -497,15 +497,21 @@ func TestListAndPatchACPConversationsByID(t *testing.T) { if err := json.Unmarshal(getRec.Body.Bytes(), &getPayload); err != nil { t.Fatalf("failed to decode get response: %v", err) } - if getPayload.Data.Spec.Title != "Renamed" || getPayload.Data.Spec.SessionID != "sess-tidy-otter-new" || getPayload.Data.Spec.CWD != "/workspace/app" { + if getPayload.Data.Spec.Title != "Renamed" || getPayload.Data.Spec.SessionID != "" || getPayload.Data.Spec.CWD != "/workspace/app" { t.Fatalf("expected patched conversation fields, got %#v", getPayload.Data.Spec) } + if getPayload.Data.Status.BindingState != "pending" || getPayload.Data.Status.BoundSessionID != "" || getPayload.Data.Status.EffectiveCWD != "" || getPayload.Data.Status.PreviousSessionID != "sess-tidy-otter-new" { + t.Fatalf("expected cwd patch to invalidate the binding, got %#v", getPayload.Data.Status) + } } func TestPatchACPConversationClearsCWDOverrideWhenEmpty(t *testing.T) { spritz := readyACPSpritz("tidy-otter", "user-1") conversation := conversationFor("tidy-otter-new", "tidy-otter", "user-1", "Latest", metav1.Now()) conversation.Spec.CWD = "/workspace/app" + conversation.Status.BindingState = "active" + conversation.Status.BoundSessionID = conversation.Spec.SessionID + conversation.Status.EffectiveCWD = "/workspace/app" s := newACPTestServer(t, spritz, conversation) e := echo.New() @@ -540,6 +546,12 @@ func TestPatchACPConversationClearsCWDOverrideWhenEmpty(t *testing.T) { if getPayload.Data.Spec.CWD != "" { t.Fatalf("expected cleared cwd override, got %#v", getPayload.Data.Spec) } + if getPayload.Data.Spec.SessionID != "" { + t.Fatalf("expected session id to be cleared after cwd change, got %#v", getPayload.Data.Spec) + } + if getPayload.Data.Status.BindingState != "pending" || getPayload.Data.Status.EffectiveCWD != "" { + t.Fatalf("expected conversation binding to require rebootstrap, got %#v", getPayload.Data.Status) + } } func TestListACPConversationsAllowsAdminToSeeAllOwners(t *testing.T) { @@ -699,7 +711,7 @@ func TestBootstrapACPConversationLoadsStoredSessionWithoutMutatingIdentity(t *te } } -func TestBootstrapACPConversationUsesResolvedDefaultWhenOverrideMissing(t *testing.T) { +func TestBootstrapACPConversationNormalizesLegacyHomeCWDToResolvedDefault(t *testing.T) { spritz := readyACPSpritz("tidy-otter", "user-1") spritz.Spec.Repo = &spritzv1.SpritzRepo{ URL: "https://example.com/open/spritz.git", @@ -707,7 +719,7 @@ func TestBootstrapACPConversationUsesResolvedDefaultWhenOverrideMissing(t *testi } conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now()) conversation.Spec.SessionID = "session-existing" - conversation.Spec.CWD = "" + conversation.Spec.CWD = "/home/dev" fakeACP := newFakeACPBootstrapServer(t, fakeACPBootstrapServerOptions{}) s := newACPTestServer(t, spritz, conversation) @@ -793,6 +805,90 @@ func TestBootstrapACPConversationPreservesExplicitWorkspaceOverride(t *testing.T } } +func TestBootstrapACPConversationPreservesExplicitHomeOverride(t *testing.T) { + spritz := readyACPSpritz("tidy-otter", "user-1") + spritz.Spec.Repo = &spritzv1.SpritzRepo{ + URL: "https://example.com/open/spritz.git", + Dir: "/workspace/platform", + } + conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now()) + conversation.Spec.SessionID = "session-existing" + setConversationCWDOverride(conversation, "/home/dev") + fakeACP := newFakeACPBootstrapServer(t, fakeACPBootstrapServerOptions{}) + + s := newACPTestServer(t, spritz, conversation) + s.acp.instanceURL = func(namespace, name string) string { return fakeACP.url } + + e := echo.New() + secured := e.Group("", s.authMiddleware()) + secured.POST("/api/acp/conversations/:id/bootstrap", s.bootstrapACPConversation) + + req := httptest.NewRequest(http.MethodPost, "/api/acp/conversations/"+conversation.Name+"/bootstrap", nil) + req.Header.Set("X-Spritz-User-Id", "user-1") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", rec.Code, rec.Body.String()) + } + + var payload struct { + Status string `json:"status"` + Data struct { + EffectiveCWD string `json:"effectiveCwd"` + } `json:"data"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("failed to decode bootstrap response: %v", err) + } + if payload.Data.EffectiveCWD != "/home/dev" { + t.Fatalf("expected explicit home cwd override to be preserved, got %q", payload.Data.EffectiveCWD) + } + + fakeACP.mu.Lock() + defer fakeACP.mu.Unlock() + if len(fakeACP.loadCWDs) != 1 || fakeACP.loadCWDs[0] != "/home/dev" { + t.Fatalf("expected session/load cwd /home/dev, got %#v", fakeACP.loadCWDs) + } +} + +func TestBootstrapACPConversationBackfillsExplicitCWDAnnotation(t *testing.T) { + spritz := readyACPSpritz("tidy-otter", "user-1") + spritz.Spec.Repo = &spritzv1.SpritzRepo{ + URL: "https://example.com/open/spritz.git", + Dir: "/workspace/platform", + } + conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now()) + conversation.Spec.SessionID = "session-existing" + conversation.Spec.CWD = "/workspace/custom" + conversation.Annotations = nil + fakeACP := newFakeACPBootstrapServer(t, fakeACPBootstrapServerOptions{}) + + s := newACPTestServer(t, spritz, conversation) + s.acp.instanceURL = func(namespace, name string) string { return fakeACP.url } + + e := echo.New() + secured := e.Group("", s.authMiddleware()) + secured.POST("/api/acp/conversations/:id/bootstrap", s.bootstrapACPConversation) + + req := httptest.NewRequest(http.MethodPost, "/api/acp/conversations/"+conversation.Name+"/bootstrap", nil) + req.Header.Set("X-Spritz-User-Id", "user-1") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d: %s", rec.Code, rec.Body.String()) + } + + stored := &spritzv1.SpritzConversation{} + if err := s.client.Get(context.Background(), clientKey("spritz-test", conversation.Name), stored); err != nil { + t.Fatalf("failed to reload conversation: %v", err) + } + if stored.Annotations[acpConversationExplicitCWDKey] != "true" { + t.Fatalf("expected explicit cwd annotation to be backfilled, got %#v", stored.Annotations) + } +} + func TestBootstrapACPConversationRepairsMissingSessionExplicitly(t *testing.T) { spritz := readyACPSpritz("tidy-otter", "user-1") conversation := conversationFor("tidy-otter-conv", "tidy-otter", "user-1", "Latest", metav1.Now()) From 561fadf0747cc4faa10eee887b3b12e0cc697420 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Sat, 4 Apr 2026 23:37:19 +0200 Subject: [PATCH 5/5] fix(slack-gateway): use resolved conversation cwd --- integrations/slack-gateway/backend_client.go | 17 ++- .../slack-gateway/backend_client_test.go | 102 ++++++++++++++++++ integrations/slack-gateway/gateway.go | 7 +- 3 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 integrations/slack-gateway/backend_client_test.go diff --git a/integrations/slack-gateway/backend_client.go b/integrations/slack-gateway/backend_client.go index 655bace..e8020fb 100644 --- a/integrations/slack-gateway/backend_client.go +++ b/integrations/slack-gateway/backend_client.go @@ -73,6 +73,7 @@ type spritzBootstrapResponse struct { Status string `json:"status"` Data struct { EffectiveSessionID string `json:"effectiveSessionId"` + EffectiveCWD string `json:"effectiveCwd"` Conversation struct { Metadata struct { Name string `json:"name"` @@ -81,6 +82,9 @@ type spritzBootstrapResponse struct { SessionID string `json:"sessionId"` CWD string `json:"cwd"` } `json:"spec"` + Status struct { + EffectiveCWD string `json:"effectiveCwd"` + } `json:"status"` } `json:"conversation"` } `json:"data"` } @@ -198,7 +202,6 @@ func (g *slackGateway) upsertChannelConversation(ctx context.Context, session ch "externalChannelId": strings.TrimSpace(event.Channel), "externalConversationId": strings.TrimSpace(externalConversationID), "title": fmt.Sprintf("Slack %s", strings.TrimSpace(event.Channel)), - "cwd": defaultConversationCWD, } var payload spritzConversationUpsertResponse if err := g.postSpritzJSON(ctx, http.MethodPost, "/api/channel-conversations/upsert", session.AccessToken, body, &payload, nil); err != nil { @@ -216,13 +219,17 @@ func (g *slackGateway) bootstrapConversation(ctx context.Context, serviceToken, if sessionID == "" { sessionID = strings.TrimSpace(payload.Data.Conversation.Spec.SessionID) } - cwd := strings.TrimSpace(payload.Data.Conversation.Spec.CWD) - if cwd == "" { - cwd = defaultConversationCWD - } + cwd := firstNonEmpty( + payload.Data.EffectiveCWD, + payload.Data.Conversation.Status.EffectiveCWD, + payload.Data.Conversation.Spec.CWD, + ) if sessionID == "" { return "", "", fmt.Errorf("bootstrap did not return a session id") } + if cwd == "" { + return "", "", fmt.Errorf("bootstrap did not return a cwd") + } return sessionID, cwd, nil } diff --git a/integrations/slack-gateway/backend_client_test.go b/integrations/slack-gateway/backend_client_test.go new file mode 100644 index 0000000..705bdac --- /dev/null +++ b/integrations/slack-gateway/backend_client_test.go @@ -0,0 +1,102 @@ +package main + +import ( + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + "time" +) + +func TestUpsertChannelConversationOmitsImplicitDefaultCWD(t *testing.T) { + var upsertPayload map[string]any + spritz := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/channel-conversations/upsert" { + t.Fatalf("unexpected spritz path %s", r.URL.Path) + } + if err := json.NewDecoder(r.Body).Decode(&upsertPayload); err != nil { + t.Fatalf("decode upsert payload: %v", err) + } + writeJSON(w, http.StatusCreated, map[string]any{ + "status": "success", + "data": map[string]any{ + "created": true, + "conversation": map[string]any{ + "metadata": map[string]any{"name": "conv-1"}, + }, + }, + }) + })) + defer spritz.Close() + + gateway := newSlackGateway(config{ + SpritzBaseURL: spritz.URL, + SpritzServiceToken: "spritz-service-token", + PrincipalID: "shared-slack-gateway", + HTTPTimeout: 5 * time.Second, + }, slog.New(slog.NewTextHandler(io.Discard, nil))) + + conversationID, err := gateway.upsertChannelConversation( + context.Background(), + channelSession{ + AccessToken: "spritz-access-token", + OwnerAuthID: "user-1", + Namespace: "spritz-staging", + InstanceID: "tidy-otter", + }, + slackEventInner{Channel: "D123"}, + "T123", + "", + "slack-thread-1", + ) + if err != nil { + t.Fatalf("upsert channel conversation: %v", err) + } + if conversationID != "conv-1" { + t.Fatalf("expected conversation id conv-1, got %q", conversationID) + } + if _, exists := upsertPayload["cwd"]; exists { + t.Fatalf("expected channel conversation upsert to omit cwd, got %#v", upsertPayload) + } +} + +func TestBootstrapConversationUsesEffectiveCWD(t *testing.T) { + spritz := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/acp/conversations/conv-1/bootstrap" { + t.Fatalf("unexpected spritz path %s", r.URL.Path) + } + writeJSON(w, http.StatusOK, map[string]any{ + "status": "success", + "data": map[string]any{ + "effectiveSessionId": "session-1", + "effectiveCwd": "/workspace/platform", + "conversation": map[string]any{ + "metadata": map[string]any{"name": "conv-1"}, + "spec": map[string]any{"sessionId": "session-1"}, + }, + }, + }) + })) + defer spritz.Close() + + gateway := newSlackGateway(config{ + SpritzBaseURL: spritz.URL, + SpritzServiceToken: "spritz-service-token", + PrincipalID: "shared-slack-gateway", + HTTPTimeout: 5 * time.Second, + }, slog.New(slog.NewTextHandler(io.Discard, nil))) + + sessionID, cwd, err := gateway.bootstrapConversation(context.Background(), "spritz-service-token", "spritz-staging", "conv-1") + if err != nil { + t.Fatalf("bootstrap conversation: %v", err) + } + if sessionID != "session-1" { + t.Fatalf("expected session id session-1, got %q", sessionID) + } + if cwd != "/workspace/platform" { + t.Fatalf("expected effective cwd /workspace/platform, got %q", cwd) + } +} diff --git a/integrations/slack-gateway/gateway.go b/integrations/slack-gateway/gateway.go index 7bb65aa..1e13222 100644 --- a/integrations/slack-gateway/gateway.go +++ b/integrations/slack-gateway/gateway.go @@ -13,10 +13,9 @@ import ( ) const ( - slackProvider = "slack" - slackWorkspaceScope = "workspace" - defaultSlackPresetID = "zeno" - defaultConversationCWD = "/home/dev" + slackProvider = "slack" + slackWorkspaceScope = "workspace" + defaultSlackPresetID = "zeno" ) type slackGateway struct {