diff --git a/go.mod b/go.mod index b67187b73f1c7144d452364fa90578a7bb7337da..19df7e97ece12a2d1a1f0243e3513d2563e94cca 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 2f9bbc3608391ad3c74c59dc3e6b22464cfb858f..c6878a43aba265a3c9a5a19e2bdb6c59c8525597 100644 --- a/go.sum +++ b/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= diff --git a/internal/agent/coordinator.go b/internal/agent/coordinator.go index 3dea024a844f20ecb48dca5c6efae3842bf1de32..749db4572775e646e6ebb318e13f84b48be1fd6f 100644 --- a/internal/agent/coordinator.go +++ b/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() diff --git a/internal/agent/coordinator_test.go b/internal/agent/coordinator_test.go index 1dd90d91e75421f1e8a6d3e3b54f12a009643c1c..c1d5ede005e4d3f69cf9482c38d99c2ca8565d98 100644 --- a/internal/agent/coordinator_test.go +++ b/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", diff --git a/internal/backend/backend.go b/internal/backend/backend.go index f14e5b7229939f5b9af11047cec7bb68a5e59cab..370bdbdcf706f2b25f17bfb53e54c041e473b586 100644 --- a/internal/backend/backend.go +++ b/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), } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index b9ba4e8241100383492d20c3e64aefe5c0a5b286..0c02064507caa544100f57953ab8a6dde6f1ff4f 100644 --- a/internal/cmd/root.go +++ b/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. diff --git a/internal/config/atomicwrite.go b/internal/config/atomicwrite.go new file mode 100644 index 0000000000000000000000000000000000000000..7e981fa11e89550184002fa1232104b846bf65c8 --- /dev/null +++ b/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 +} diff --git a/internal/config/atomicwrite_test.go b/internal/config/atomicwrite_test.go new file mode 100644 index 0000000000000000000000000000000000000000..089ee5c78eae7fecdb5bc3cd3d8f74713098fc38 --- /dev/null +++ b/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()) +} diff --git a/internal/config/store.go b/internal/config/store.go index 5501bdddafd206c4294a22666b59399338db7503..3e55509b7132e38805830818fca1e5265b7e03f9 100644 --- a/internal/config/store.go +++ b/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. diff --git a/internal/proto/version.go b/internal/proto/version.go index b728a8b966068a7810f86aae74cfcc6f57e03d39..4f837ce2eb561e1e16586e86b891aa0a1dc3dba8 100644 --- a/internal/proto/version.go +++ b/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"` } diff --git a/internal/version/version.go b/internal/version/version.go index 3eb4f74139a752c1567986a8b9344913d55f08b1..99b06243f176cf7e078a14ec3eae867738d48cbc 100644 --- a/internal/version/version.go +++ b/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) }