Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 19 additions & 10 deletions internal/command/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -1080,14 +1080,16 @@ func runWebServer(parent context.Context, port int, host string, openBrowser boo
var err error
ag, err = createAgent(providerName, modelName)
if err != nil {
if _, isCloud := cloud.ParseCloudProviderRef(providerName); !isCloud {
return nil, fmt.Errorf("error creating agent: %w", err)
}
// Keep the Desktop control plane usable when a previously
// selected Cloud model is temporarily unavailable. We never
// silently execute on a local model; the empty agent blocks
// sends until the user reconnects or explicitly selects one.
config.Logger().Printf("[cloud] selected model unavailable at startup: %v", err)
// Model availability must not gate the Desktop control plane. In
// particular, managed accounts that require reauthentication need
// the settings UI served by this same process in order to recover.
// Keep the selected provider/model and let the Engine lazily retry
// agent creation before the next send; it never falls back to a
// different model silently.
config.Logger().Printf(
"[web] selected model %s/%s unavailable while building task engine; control plane remains available: %v",
providerName, modelName, err,
)
}
}

Expand Down Expand Up @@ -1273,8 +1275,15 @@ func startWebServer(runtime webServerRuntime) error {
return
}
config.Logger().Printf("[wechat] inbound message from %s: %s", from, text)
if !srv.SubmitMessage(text, "wechat") {
_ = runtime.wechatClient.SendText(channel.BusyMessage())
accepted, submitErr := srv.SubmitMessage(text, "wechat")
if submitErr != nil {
if sendErr := runtime.wechatClient.SendText(channel.DoneMessage("", submitErr)); sendErr != nil {
config.Logger().Printf("[wechat] failed to send submission error: %v", sendErr)
}
} else if !accepted {
if sendErr := runtime.wechatClient.SendText(channel.BusyMessage()); sendErr != nil {
config.Logger().Printf("[wechat] failed to send busy response: %v", sendErr)
}
}
})
defer func() {
Expand Down
127 changes: 127 additions & 0 deletions internal/command/web_unavailable_model_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
package command

import (
"bytes"
"context"
"fmt"
"io"
"net"
"net/http"
"os"
"path/filepath"
"strings"
"testing"
"time"
)

func managedReauthHome(t *testing.T) string {
t.Helper()
home := t.TempDir()
configDir := filepath.Join(home, ".jcode")
if err := os.MkdirAll(configDir, 0o700); err != nil {
t.Fatalf("mkdir config: %v", err)
}
configJSON := `{
"model": "xai/grok-4.6",
"providers": {
"xai": {
"auth": {"method": "xai_oauth", "account_id": "account-1"},
"custom_models": [
{"id": "grok-4.6", "managed": true, "protocol": "responses"}
]
}
},
"memory": {"enabled": false}
}`
if err := os.WriteFile(filepath.Join(configDir, "config.json"), []byte(configJSON), 0o600); err != nil {
t.Fatalf("write config: %v", err)
}
authJSON := `{
"version": 1,
"methods": {
"xai_oauth": {
"accounts": {
"account-1": {
"id": "account-1",
"login": "reauth@example.test",
"secret": "expired-refresh-token",
"authenticated_at": "2026-08-09T00:00:00Z",
"requires_reauth": true
}
},
"default_account_id": "account-1"
}
}
}`
if err := os.WriteFile(filepath.Join(configDir, "provider-auth.json"), []byte(authJSON), 0o600); err != nil {
t.Fatalf("write provider auth: %v", err)
}
return home
}

// Regression: a selected managed account that needs reauthentication must not
// abort the sidecar before /api/health and the provider-auth recovery endpoints
// are reachable. Chat remains fail-closed until reauthentication succeeds.
func TestRunWebServerManagedReauthKeepsControlPlaneAvailable(t *testing.T) {
t.Setenv("HOME", managedReauthHome(t))

ln, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("reserve port: %v", err)
}
port := ln.Addr().(*net.TCPAddr).Port
_ = ln.Close()

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
errCh := make(chan error, 1)
go func() { errCh <- runWebServer(ctx, port, "127.0.0.1", false, "") }()

baseURL := fmt.Sprintf("http://127.0.0.1:%d", port)
requireHealthOK(t, baseURL, errCh)
client := &http.Client{Timeout: 10 * time.Second}

statusResp, err := client.Get(baseURL + "/api/provider-auth/xai_oauth") //nolint:gosec // loopback test server
if err != nil {
t.Fatalf("get provider auth status: %v", err)
}
statusBody, readErr := io.ReadAll(io.LimitReader(statusResp.Body, 1<<20))
_ = statusResp.Body.Close()
if readErr != nil {
t.Fatalf("read provider auth status: %v", readErr)
}
if statusResp.StatusCode != http.StatusOK || !bytes.Contains(statusBody, []byte(`"requires_reauth":true`)) {
t.Fatalf("provider auth status=%d body=%s", statusResp.StatusCode, statusBody)
}

// A failed lazy rebuild must release the task's running claim. The second
// request should retry authentication and return the same actionable error,
// not get stuck behind a false "already processing" conflict.
for attempt := 1; attempt <= 2; attempt++ {
chatResp, err := client.Post( //nolint:gosec // loopback test server
baseURL+"/api/chat", "application/json", strings.NewReader(`{"message":"hello"}`),
)
if err != nil {
t.Fatalf("post chat attempt %d: %v", attempt, err)
}
chatBody, readErr := io.ReadAll(io.LimitReader(chatResp.Body, 1<<20))
_ = chatResp.Body.Close()
if readErr != nil {
t.Fatalf("read chat response attempt %d: %v", attempt, readErr)
}
if chatResp.StatusCode != http.StatusServiceUnavailable ||
!bytes.Contains(chatBody, []byte("requires reauthentication")) {
t.Fatalf("chat attempt %d status=%d body=%s", attempt, chatResp.StatusCode, chatBody)
}
}

cancel()
select {
case runErr := <-errCh:
if runErr != nil {
t.Fatalf("runWebServer returned error: %v", runErr)
}
case <-time.After(15 * time.Second):
t.Fatal("web server did not shut down after context cancel")
}
}
7 changes: 6 additions & 1 deletion internal/web/approval.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,12 @@ func (s *Server) handleSetGoal(w http.ResponseWriter, r *http.Request) {
// will pick the goal up after the current run finishes. Targets the active
// task.
if eng.running.CompareAndSwap(false, true) {
s.submitMessage(eng, tools.GoalKickoffPrompt(objective), eng.curMode(), req.Source, req.TaskID, nil)
if _, err := s.submitMessage(
eng, tools.GoalKickoffPrompt(objective), eng.curMode(), req.Source, req.TaskID, nil,
); err != nil {
writeJSON(w, http.StatusServiceUnavailable, map[string]string{"error": err.Error()})
return
}
}
}
writeJSON(w, http.StatusOK, g)
Expand Down
5 changes: 4 additions & 1 deletion internal/web/automation_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,10 @@ func (s *Server) runAutomation(ctx context.Context, a *automation.Automation, ki
s.deleteEngine(sid)
return sid, fmt.Errorf("engine busy")
}
_ = s.submitMessage(eng, a.Prompt, mode, "automation", sid, nil)
if _, err := s.submitMessage(eng, a.Prompt, mode, "automation", sid, nil); err != nil {
s.deleteEngine(sid)
return sid, err
}
s.stampAutomationMeta(sid, a, kind)

var runErr error
Expand Down
83 changes: 60 additions & 23 deletions internal/web/chat.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ func (s *Server) handleChat(w http.ResponseWriter, r *http.Request) {
return
}

sessionID := s.submitMessage(eng, req.Message, modeStr, req.Source, req.SessionID, req.Images)
sessionID, err := s.submitMessage(eng, req.Message, modeStr, req.Source, req.SessionID, req.Images)
if err != nil {
writeJSON(w, http.StatusServiceUnavailable, map[string]string{"error": err.Error()})
return
}
writeJSON(w, http.StatusAccepted, map[string]string{"status": "processing", "session_id": sessionID})
}

Expand Down Expand Up @@ -145,17 +149,22 @@ type chatImage struct {
}

// SubmitMessage submits a message for agent processing from an external source
// (e.g. WeChat inbound message). Returns false if the agent is busy.
func (s *Server) SubmitMessage(message, source string) bool {
// (e.g. WeChat inbound message). accepted is false only when no active engine is
// available or the targeted engine is already busy; agent recovery failures are
// returned separately so channel callers can surface the real error.
func (s *Server) SubmitMessage(message, source string) (accepted bool, err error) {
eng := s.activeEngine()
if eng == nil {
return false
return false, nil
}
if !eng.running.CompareAndSwap(false, true) {
return false
return false, nil
}
_, err = s.submitMessage(eng, message, eng.curMode(), source, "", nil)
if err != nil {
return false, err
}
s.submitMessage(eng, message, eng.curMode(), source, "", nil)
return true
return true, nil
}

// submitMessage is the shared implementation for starting an agent run.
Expand All @@ -165,8 +174,15 @@ func (s *Server) SubmitMessage(message, source string) bool {
// correct session instead of creating a new one.
// images is an optional list of base64-encoded images to include in the message.
// The caller must have already set eng.running to true (via CompareAndSwap).
// Returns the session_id of the recorder used.
func (s *Server) submitMessage(eng *Engine, message, mode, source, sessionID string, images []chatImage) string {
// Returns the session_id of the recorder used. If a degraded Engine is
// still unavailable, agent construction is retried before any history or
// recorder mutation and the caller's running claim is released.
func (s *Server) submitMessage(eng *Engine, message, mode, source, sessionID string, images []chatImage) (string, error) {
if err := eng.ensureAgentAvailable(); err != nil {
eng.running.Store(false)
return "", err
}

// Slash command rewrite: if the original message starts with "/", check for
// skill slash commands and rewrite to load_skill instruction (same pattern as
// ACP/TUI). This must happen BEFORE the plan-mode prefix is applied, otherwise
Expand Down Expand Up @@ -238,31 +254,52 @@ func (s *Server) submitMessage(eng *Engine, message, mode, source, sessionID str
eng.toolOverrideMu.Lock()
eng.emu.Lock()
if eng.recorder == nil {
rec, _ := session.NewRecorder(eng.pwd, eng.providerName, eng.modelName)
if rec != nil {
rec.SetAgent(eng.agentRole)
rec, err := session.NewRecorder(eng.pwd, eng.providerName, eng.modelName)
if err != nil {
eng.emu.Unlock()
eng.toolOverrideMu.Unlock()
eng.running.Store(false)
return "", fmt.Errorf("create session recorder: %w", err)
}
if rec != nil && sessionID != "" {
if rec == nil {
eng.emu.Unlock()
eng.toolOverrideMu.Unlock()
eng.running.Store(false)
return "", fmt.Errorf("create session recorder: returned nil recorder")
}
rec.SetAgent(eng.agentRole)
if sessionID != "" {
rec.SetUUID(sessionID)
}
if rec != nil && eng.recorderInit != nil {
if eng.recorderInit != nil {
eng.recorderInit(rec)
}
eng.recorder = rec
// sessionID == "" means the recorder just minted a new UUID.
stampNew = rec != nil && sessionID == ""
stampNew = sessionID == ""
} else if sessionID != "" && eng.recorder.UUID() != sessionID {
// Client is continuing a session that doesn't match the current recorder.
// Resume the client's session to keep all messages together.
eng.recorder.Close()
rec, _ := session.NewRecorder(eng.pwd, eng.providerName, eng.modelName)
if rec != nil {
rec.SetAgent(eng.agentRole)
rec.SetUUID(sessionID)
// Build the replacement before closing the current recorder so a creation
// failure cannot discard the live session.
rec, err := session.NewRecorder(eng.pwd, eng.providerName, eng.modelName)
if err != nil {
eng.emu.Unlock()
eng.toolOverrideMu.Unlock()
eng.running.Store(false)
return "", fmt.Errorf("create recorder for session %s: %w", sessionID, err)
}
if rec == nil {
eng.emu.Unlock()
eng.toolOverrideMu.Unlock()
eng.running.Store(false)
return "", fmt.Errorf("create recorder for session %s: returned nil recorder", sessionID)
}
if rec != nil && eng.recorderInit != nil {
rec.SetAgent(eng.agentRole)
rec.SetUUID(sessionID)
if eng.recorderInit != nil {
eng.recorderInit(rec)
}
eng.recorder.Close()
eng.recorder = rec
}
recorder := eng.recorder
Expand Down Expand Up @@ -381,7 +418,7 @@ func (s *Server) submitMessage(eng *Engine, message, mode, source, sessionID str
}
}()

return recorder.UUID()
return recorder.UUID(), nil
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// --- Stop handler ---
Expand Down
40 changes: 40 additions & 0 deletions internal/web/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,46 @@ func (e *Engine) modelSnapshot() (provider, model, modeStr string) {
return e.providerName, e.modelName, e.mode
}

// ensureAgentAvailable lazily repairs an Engine whose selected model could not
// be constructed when the task engine was built. This keeps the control plane
// reachable for account reauthentication and provider/model changes without
// ever executing on a silent fallback. Rebuilds are serialized with the normal
// model/mode paths so a successful recovery cannot overwrite a concurrent user
// selection.
func (e *Engine) ensureAgentAvailable() error {
e.emu.Lock()
if e.agent != nil {
e.emu.Unlock()
return nil
}
e.emu.Unlock()

if e.createAgent == nil {
return fmt.Errorf("selected model agent is unavailable")
}

e.rebuildMu.Lock()
defer e.rebuildMu.Unlock()

e.emu.Lock()
if e.agent != nil {
e.emu.Unlock()
return nil
}
provider, modelName := e.providerName, e.modelName
e.emu.Unlock()

ag, err := e.createAgent(provider, modelName)
if err != nil {
return fmt.Errorf("create agent for selected model %s/%s: %w", provider, modelName, err)
}
if ag == nil {
return fmt.Errorf("create agent for selected model %s/%s returned nil", provider, modelName)
}
e.applyModelSwitch(ag, provider, modelName)
return nil
}

// agentBuildSnapshot captures the inputs and revision for an asynchronous
// agent rebuild. Call installAgentIfRevision with the returned revision: a
// concurrent model/mode/skill change must win instead of being overwritten by
Expand Down
Loading
Loading