Merge remote-tracking branch 'origin/main' into fast-ui

Christian Rocha created

Change summary

go.mod                              |  2 
go.sum                              |  4 +-
internal/agent/coordinator.go       | 41 +++++++++++++++++++++++------
internal/agent/coordinator_test.go  |  2 
internal/backend/backend.go         |  1 
internal/cmd/root.go                | 11 +++++--
internal/config/atomicwrite.go      | 38 +++++++++++++++++++++++++++
internal/config/atomicwrite_test.go | 43 ++++++++++++++++++++++++++++++
internal/config/store.go            | 40 ++++++++++++++++++++-------
internal/proto/version.go           |  1 
internal/version/version.go         | 40 ++++++++++++++++++++++++----
11 files changed, 189 insertions(+), 34 deletions(-)

Detailed changes

go.mod 🔗

@@ -7,7 +7,7 @@ require (
 	charm.land/bubbletea/v2 v2.0.6
 	charm.land/catwalk v0.40.0
 	charm.land/fang/v2 v2.0.1
-	charm.land/fantasy v0.23.2
+	charm.land/fantasy v0.24.0
 	charm.land/glamour/v2 v2.0.0
 	charm.land/lipgloss/v2 v2.0.3
 	charm.land/log/v2 v2.0.0

go.sum 🔗

@@ -6,8 +6,8 @@ charm.land/catwalk v0.40.0 h1:Ldqr2YzTqk2QdXAFFa5W/EiFoxNGMQc7KXekze5Z+Cw=
 charm.land/catwalk v0.40.0/go.mod h1:LmMFJdRqF5F7qKa+xqD9SBq7tph7L98GU3ZFa1TxftA=
 charm.land/fang/v2 v2.0.1 h1:zQCM8JQJ1JnQX/66B5jlCYBUxL2as5JXQZ2KJ6EL0mY=
 charm.land/fang/v2 v2.0.1/go.mod h1:S1GmkpcvK+OB5w9caywUnJcsMew45Ot8FXqoz8ALrII=
-charm.land/fantasy v0.23.2 h1:9gUknrENwv0lV379Hs7m/GK2c/9m+gWXyfbox1tVHrg=
-charm.land/fantasy v0.23.2/go.mod h1:8QrWUzIcKwZQP+aAnC9vLu3iID6hu9/Jt+rPMiieBkc=
+charm.land/fantasy v0.24.0 h1:/q35T2TvrqTYqMnFOBHEqp8IWoKMxh05Y7r5wsF/6F4=
+charm.land/fantasy v0.24.0/go.mod h1:8QrWUzIcKwZQP+aAnC9vLu3iID6hu9/Jt+rPMiieBkc=
 charm.land/glamour/v2 v2.0.0 h1:IDBoqLEy7Hdpb9VOXN+khLP/XSxtJy1VsHuW/yF87+U=
 charm.land/glamour/v2 v2.0.0/go.mod h1:kjq9WB0s8vuUYZNYey2jp4Lgd9f4cKdzAw88FZtpj/w=
 charm.land/lipgloss/v2 v2.0.3 h1:yM2zJ4Cf5Y51b7RHIwioil4ApI/aypFXXVHSwlM6RzU=

internal/agent/coordinator.go 🔗

@@ -62,6 +62,15 @@ var (
 	errSmallModelNotFound              = errors.New("small model not found in provider config")
 )
 
+// Copilot models that use the Responses API instead of Chat Completions.
+var copilotResponsesModels = map[string]bool{
+	"gpt-5.2":       true,
+	"gpt-5.2-codex": true,
+	"gpt-5.3-codex": true,
+	"gpt-5.4-mini":  true,
+	"gpt-5-mini":    true,
+}
+
 type Coordinator interface {
 	// INFO: (kujtim) this is not used yet we will use this when we have multiple agents
 	// SetMainAgent(string)
@@ -268,7 +277,7 @@ func getProviderOptions(model Model, providerCfg config.ProviderConfig) fantasy.
 	switch providerCfg.Type {
 	case openai.Name, azure.Name:
 		_, hasReasoningEffort := mergedOptions["reasoning_effort"]
-		if !hasReasoningEffort && model.ModelCfg.ReasoningEffort != "" {
+		if !hasReasoningEffort && model.ModelCfg.ReasoningEffort != "" && model.CatwalkCfg.CanReason {
 			mergedOptions["reasoning_effort"] = model.ModelCfg.ReasoningEffort
 		}
 		if openai.IsResponsesModel(model.CatwalkCfg.ID) {
@@ -292,7 +301,7 @@ func getProviderOptions(model Model, providerCfg config.ProviderConfig) fantasy.
 			_, hasThink  = mergedOptions["thinking"]
 		)
 		switch {
-		case !hasEffort && model.ModelCfg.ReasoningEffort != "":
+		case !hasEffort && model.ModelCfg.ReasoningEffort != "" && model.CatwalkCfg.CanReason:
 			mergedOptions["effort"] = model.ModelCfg.ReasoningEffort
 		case !hasThink && model.ModelCfg.Think:
 			mergedOptions["thinking"] = map[string]any{"budget_tokens": 2000}
@@ -346,13 +355,18 @@ func getProviderOptions(model Model, providerCfg config.ProviderConfig) fantasy.
 			options[google.Name] = parsed
 		}
 	case openaicompat.Name, hyper.Name:
+		extraBody := make(map[string]any)
+
 		_, hasReasoningEffort := mergedOptions["reasoning_effort"]
-		if !hasReasoningEffort && model.ModelCfg.ReasoningEffort != "" {
-			mergedOptions["reasoning_effort"] = model.ModelCfg.ReasoningEffort
+		if !hasReasoningEffort && model.ModelCfg.ReasoningEffort != "" && model.CatwalkCfg.CanReason {
+			switch providerCfg.ID {
+			case string(catwalk.InferenceProviderIoNet):
+				extraBody["reasoning"] = map[string]string{"effort": model.ModelCfg.ReasoningEffort}
+			default:
+				mergedOptions["reasoning_effort"] = model.ModelCfg.ReasoningEffort
+			}
 		}
 
-		extraBody := make(map[string]any)
-
 		// "reasoning effort" is a standard OpenAI field, but "thinking" is not.
 		// Setting it in the right way for each provider.
 		// TODO: Abstract this in Fantasy somehow?
@@ -361,8 +375,12 @@ func getProviderOptions(model Model, providerCfg config.ProviderConfig) fantasy.
 		case hyper.Name:
 			extraBody["thinking"] = model.ModelCfg.Think
 		case string(catwalk.InferenceProviderIoNet):
-			extraBody["chat_template_kwargs"] = map[string]any{
-				"thinking": model.ModelCfg.Think,
+			if _, ok := extraBody["reasoning"]; !ok && model.CatwalkCfg.CanReason {
+				if model.ModelCfg.Think {
+					extraBody["reasoning"] = map[string]string{"effort": "medium"}
+				} else {
+					extraBody["reasoning"] = map[string]string{"effort": "none"}
+				}
 			}
 		case string(catwalk.InferenceProviderZAI), string(catwalk.InferenceProviderDeepSeek):
 			if model.ModelCfg.Think {
@@ -722,7 +740,12 @@ func (c *coordinator) buildOpenaiCompatProvider(baseURL, apiKey string, headers
 	// Set HTTP client based on provider and debug mode.
 	var httpClient *http.Client
 	if providerID == string(catwalk.InferenceProviderCopilot) {
-		opts = append(opts, openaicompat.WithUseResponsesAPI())
+		opts = append(opts,
+			openaicompat.WithUseResponsesAPI(),
+			openaicompat.WithResponsesAPIFunc(func(modelID string) bool {
+				return copilotResponsesModels[modelID]
+			}),
+		)
 		httpClient = copilot.NewClient(isSubAgent, c.cfg.Config().Options.Debug)
 	} else if c.cfg.Config().Options.Debug {
 		httpClient = log.NewHTTPClient()

internal/agent/coordinator_test.go 🔗

@@ -399,7 +399,7 @@ func TestGetProviderOptionsReasoningEffort(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			model := Model{
-				CatwalkCfg: catwalk.Model{ID: "claude-opus-4-7"},
+				CatwalkCfg: catwalk.Model{ID: "claude-opus-4-7", CanReason: true},
 				ModelCfg: config.SelectedModel{
 					Provider:        "test",
 					ReasoningEffort: "max",

internal/backend/backend.go 🔗

@@ -174,6 +174,7 @@ func (b *Backend) VersionInfo() proto.VersionInfo {
 	return proto.VersionInfo{
 		Version:   version.Version,
 		Commit:    version.Commit,
+		BuildID:   version.BuildID,
 		GoVersion: runtime.Version(),
 		Platform:  fmt.Sprintf("%s/%s", runtime.GOOS, runtime.GOARCH),
 	}

internal/cmd/root.go 🔗

@@ -614,12 +614,15 @@ func restartIfStale(cmd *cobra.Command, hostURL *url.URL) (restarted bool, err e
 	if err != nil {
 		return false, err
 	}
-	if vi.Version == version.Version {
+	if vi.Version == version.Version && vi.BuildID == version.BuildID {
 		return false, nil
 	}
-	slog.Info("Server version mismatch, restarting",
-		"server", vi.Version,
-		"client", version.Version,
+	slog.Info(
+		"Server version mismatch, restarting",
+		"server_version", vi.Version,
+		"client_version", version.Version,
+		"server_build_id", vi.BuildID,
+		"client_build_id", version.BuildID,
 	)
 	_ = c.ShutdownServer(cmd.Context())
 	// Give the old process a moment to release the socket.

internal/config/atomicwrite.go 🔗

@@ -0,0 +1,38 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+)
+
+// atomicWriteFile writes data to a file atomically by writing to a unique
+// temporary file in the same directory and renaming it into place. This
+// prevents concurrent readers from observing a partially-written file.
+func atomicWriteFile(path string, data []byte, perm os.FileMode) error {
+	path = filepath.Clean(path)
+	dir := filepath.Dir(path)
+	f, err := os.CreateTemp(dir, filepath.Base(path)+".*.tmp")
+	if err != nil {
+		return err
+	}
+	tmp := f.Name()
+	if _, err := f.Write(data); err != nil {
+		f.Close()
+		os.Remove(tmp)
+		return err
+	}
+	if err := f.Chmod(perm); err != nil {
+		f.Close()
+		os.Remove(tmp)
+		return err
+	}
+	if err := f.Close(); err != nil {
+		os.Remove(tmp)
+		return err
+	}
+	if err := os.Rename(tmp, path); err != nil {
+		os.Remove(tmp)
+		return err
+	}
+	return nil
+}

internal/config/atomicwrite_test.go 🔗

@@ -0,0 +1,43 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestAtomicWriteFile(t *testing.T) {
+	t.Parallel()
+	dir := t.TempDir()
+	path := filepath.Join(dir, "test.json")
+
+	require.NoError(t, atomicWriteFile(path, []byte(`{"key":"value"}`), 0o600))
+
+	data, err := os.ReadFile(path)
+	require.NoError(t, err)
+	require.Equal(t, `{"key":"value"}`, string(data))
+
+	// No temp files should linger.
+	entries, err := os.ReadDir(dir)
+	require.NoError(t, err)
+	require.Len(t, entries, 1)
+	require.Equal(t, "test.json", entries[0].Name())
+}
+
+func TestAtomicWriteFile_PermissionsApplied(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Windows does not support Unix file permissions")
+	}
+	t.Parallel()
+	dir := t.TempDir()
+	path := filepath.Join(dir, "test.json")
+
+	require.NoError(t, atomicWriteFile(path, []byte(`{}`), 0o600))
+
+	info, err := os.Stat(path)
+	require.NoError(t, err)
+	require.Equal(t, os.FileMode(0o600), info.Mode().Perm())
+}

internal/config/store.go 🔗

@@ -158,7 +158,7 @@ func (s *ConfigStore) SetConfigFields(scope Scope, kv map[string]any) error {
 	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
 		return fmt.Errorf("failed to create config directory %q: %w", path, err)
 	}
-	if err := os.WriteFile(path, []byte(newValue), 0o600); err != nil {
+	if err := atomicWriteFile(path, []byte(newValue), 0o600); err != nil {
 		return fmt.Errorf("failed to write config file: %w", err)
 	}
 
@@ -193,7 +193,7 @@ func (s *ConfigStore) RemoveConfigField(scope Scope, key string) error {
 	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
 		return fmt.Errorf("failed to create config directory %q: %w", path, err)
 	}
-	if err := os.WriteFile(path, []byte(newValue), 0o600); err != nil {
+	if err := atomicWriteFile(path, []byte(newValue), 0o600); err != nil {
 		return fmt.Errorf("failed to write config file: %w", err)
 	}
 
@@ -302,7 +302,10 @@ func (s *ConfigStore) SetProviderAPIKey(scope Scope, providerID string, apiKey a
 // RefreshOAuthToken refreshes the OAuth token for the given provider.
 // Before making an external refresh request, it checks the config file on
 // disk to see if another Crush session has already refreshed the token. If
-// a newer token is found, it is used instead of refreshing.
+// a newer, unexpired token is found, it is used instead of refreshing. If
+// the exchange fails (e.g. because another session already rotated the
+// refresh token), the disk is re-checked to recover the other session's
+// token.
 func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, providerID string) error {
 	providerConfig, exists := s.config.Providers.Get(providerID)
 	if !exists {
@@ -318,15 +321,9 @@ func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, provid
 	newToken, err := s.loadTokenFromDisk(scope, providerID)
 	if err != nil {
 		slog.Warn("Failed to read token from config file, proceeding with refresh", "provider", providerID, "error", err)
-	} else if newToken != nil && newToken.AccessToken != providerConfig.OAuthToken.AccessToken {
+	} else if newToken != nil && !newToken.IsExpired() && newToken.AccessToken != providerConfig.OAuthToken.AccessToken {
 		slog.Info("Using token refreshed by another session", "provider", providerID)
-		providerConfig.OAuthToken = newToken
-		providerConfig.APIKey = newToken.AccessToken
-		if providerID == string(catwalk.InferenceProviderCopilot) {
-			providerConfig.SetupGitHubCopilot()
-		}
-		s.config.Providers.Set(providerID, providerConfig)
-		return nil
+		return s.applyToken(providerConfig, newToken, providerID)
 	}
 
 	var refreshedToken *oauth.Token
@@ -340,6 +337,16 @@ func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, provid
 		return fmt.Errorf("OAuth refresh not supported for provider %s", providerID)
 	}
 	if refreshErr != nil {
+		// The exchange may have failed because another session already
+		// rotated the refresh token. Re-read the config file and use the
+		// other session's token if available.
+		if diskToken, diskErr := s.loadTokenFromDisk(scope, providerID); diskErr == nil &&
+			diskToken != nil &&
+			!diskToken.IsExpired() &&
+			diskToken.AccessToken != providerConfig.OAuthToken.AccessToken {
+			slog.Info("Using token refreshed by another session after exchange failure", "provider", providerID)
+			return s.applyToken(providerConfig, diskToken, providerID)
+		}
 		return fmt.Errorf("failed to refresh OAuth token for provider %s: %w", providerID, refreshErr)
 	}
 
@@ -364,6 +371,17 @@ func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, provid
 	return nil
 }
 
+// applyToken updates the in-memory provider config with the given token.
+func (s *ConfigStore) applyToken(providerConfig ProviderConfig, token *oauth.Token, providerID string) error {
+	providerConfig.OAuthToken = token
+	providerConfig.APIKey = token.AccessToken
+	if providerID == string(catwalk.InferenceProviderCopilot) {
+		providerConfig.SetupGitHubCopilot()
+	}
+	s.config.Providers.Set(providerID, providerConfig)
+	return nil
+}
+
 // loadTokenFromDisk reads the OAuth token for the given provider from the
 // config file on disk. Returns nil if the token is not found or matches the
 // current in-memory token.

internal/proto/version.go 🔗

@@ -4,6 +4,7 @@ package proto
 type VersionInfo struct {
 	Version   string `json:"version"`
 	Commit    string `json:"commit"`
+	BuildID   string `json:"build_id"`
 	GoVersion string `json:"go_version"`
 	Platform  string `json:"platform"`
 }

internal/version/version.go 🔗

@@ -1,12 +1,21 @@
 package version
 
-import "runtime/debug"
+import (
+	"os"
+	"runtime/debug"
+	"strconv"
+)
 
 // Build-time parameters set via -ldflags.
 
 var (
 	Version = "devel"
 	Commit  = "unknown"
+	// BuildID is a unique identifier for this build. For release builds it
+	// equals Commit; for development builds (go run / go build without
+	// ldflags) it is derived from the executable's modification time, which
+	// changes on every recompilation.
+	BuildID = ""
 )
 
 // A user may install crush using `go install github.com/charmbracelet/crush@latest`.
@@ -15,11 +24,30 @@ var (
 // is only set for `go install` and not for `go build`).
 func init() {
 	info, ok := debug.ReadBuildInfo()
-	if !ok {
-		return
+	if ok {
+		mainVersion := info.Main.Version
+		if mainVersion != "" && mainVersion != "(devel)" {
+			Version = mainVersion
+		}
+	}
+
+	// Derive BuildID when not set via ldflags.
+	if BuildID == "" {
+		BuildID = deriveBuildID()
+	}
+}
+
+// deriveBuildID uses the running executable's modification time as a unique
+// build fingerprint. This changes on every recompilation (including `go run`),
+// making it reliable for detecting stale servers during development.
+func deriveBuildID() string {
+	exe, err := os.Executable()
+	if err != nil {
+		return "unknown"
 	}
-	mainVersion := info.Main.Version
-	if mainVersion != "" && mainVersion != "(devel)" {
-		Version = mainVersion
+	fi, err := os.Stat(exe)
+	if err != nil {
+		return "unknown"
 	}
+	return strconv.FormatInt(fi.ModTime().UnixNano(), 36)
 }