diff --git a/.gitignore b/.gitignore index 1fedba8..622de74 100644 --- a/.gitignore +++ b/.gitignore @@ -65,3 +65,4 @@ CLAUDE.local.md # Chat history data/ +.codex diff --git a/internal/admin/handler_config_import.go b/internal/admin/handler_config_import.go index c238428..7decbde 100644 --- a/internal/admin/handler_config_import.go +++ b/internal/admin/handler_config_import.go @@ -56,26 +56,9 @@ func (h *Handler) configImport(w http.ResponseWriter, r *http.Request) { importedKeys = len(next.APIKeys) importedAccounts = len(next.Accounts) } else { - existingKeys := map[string]struct{}{} - for _, item := range next.APIKeys { - key := strings.TrimSpace(item.Key) - if key == "" { - continue - } - existingKeys[key] = struct{}{} - } - for _, item := range incoming.APIKeys { - key := strings.TrimSpace(item.Key) - if key == "" { - continue - } - if _, ok := existingKeys[key]; ok { - continue - } - existingKeys[key] = struct{}{} - next.APIKeys = append(next.APIKeys, item) - importedKeys++ - } + var changed int + next.APIKeys, changed = mergeAPIKeysPreferStructured(next.APIKeys, incoming.APIKeys) + importedKeys += changed existingAccounts := map[string]struct{}{} for _, acc := range next.Accounts { diff --git a/internal/admin/handler_config_write.go b/internal/admin/handler_config_write.go index ae696bc..1929f26 100644 --- a/internal/admin/handler_config_write.go +++ b/internal/admin/handler_config_write.go @@ -22,14 +22,7 @@ func (h *Handler) updateConfig(w http.ResponseWriter, r *http.Request) { if apiKeys, ok := toAPIKeys(req["api_keys"]); ok { c.APIKeys = apiKeys } else if keys, ok := toStringSlice(req["keys"]); ok { - legacy := make([]config.APIKey, 0, len(keys)) - for _, key := range keys { - if key == "" { - continue - } - legacy = append(legacy, config.APIKey{Key: key}) - } - c.APIKeys = legacy + c.Keys = keys } if accountsRaw, ok := req["accounts"].([]any); ok { existing := map[string]config.Account{} @@ -182,33 +175,22 @@ func (h *Handler) batchImport(w http.ResponseWriter, r *http.Request) { importedKeys, importedAccounts := 0, 0 err := h.Store.Update(func(c *config.Config) error { if apiKeys, ok := toAPIKeys(req["api_keys"]); ok { - existing := map[string]bool{} - for _, item := range c.APIKeys { - existing[item.Key] = true - } - for _, item := range apiKeys { - if item.Key == "" || existing[item.Key] { - continue - } - c.APIKeys = append(c.APIKeys, item) - existing[item.Key] = true - importedKeys++ - } + var changed int + c.APIKeys, changed = mergeAPIKeysPreferStructured(c.APIKeys, apiKeys) + importedKeys += changed } if keys, ok := req["keys"].([]any); ok { - existing := map[string]bool{} - for _, item := range c.APIKeys { - existing[item.Key] = true - } + legacy := make([]config.APIKey, 0, len(keys)) for _, k := range keys { key := strings.TrimSpace(fmt.Sprintf("%v", k)) - if key == "" || existing[key] { + if key == "" { continue } - c.APIKeys = append(c.APIKeys, config.APIKey{Key: key}) - existing[key] = true - importedKeys++ + legacy = append(legacy, config.APIKey{Key: key}) } + var changed int + c.APIKeys, changed = mergeAPIKeysPreferStructured(c.APIKeys, legacy) + importedKeys += changed } if accounts, ok := req["accounts"].([]any); ok { existing := map[string]bool{} diff --git a/internal/admin/handler_settings_test.go b/internal/admin/handler_settings_test.go index 9aeba35..e3ec356 100644 --- a/internal/admin/handler_settings_test.go +++ b/internal/admin/handler_settings_test.go @@ -271,6 +271,38 @@ func TestUpdateConfigPreservesStructuredAPIKeysWhenBothFieldsPresent(t *testing. } } +func TestUpdateConfigLegacyKeysPreserveStructuredMetadata(t *testing.T) { + h := newAdminTestHandler(t, `{ + "api_keys":[{"key":"legacy","name":"primary","remark":"prod"}], + "accounts":[] + }`) + + payload := map[string]any{ + "keys": []any{"legacy", "new-key"}, + } + b, _ := json.Marshal(payload) + req := httptest.NewRequest(http.MethodPost, "/admin/config", bytes.NewReader(b)) + rec := httptest.NewRecorder() + h.updateConfig(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + + snap := h.Store.Snapshot() + if len(snap.Keys) != 2 || snap.Keys[0] != "legacy" || snap.Keys[1] != "new-key" { + t.Fatalf("unexpected keys after legacy config update: %#v", snap.Keys) + } + if len(snap.APIKeys) != 2 { + t.Fatalf("unexpected api keys after legacy config update: %#v", snap.APIKeys) + } + if snap.APIKeys[0].Name != "primary" || snap.APIKeys[0].Remark != "prod" { + t.Fatalf("existing structured metadata was lost: %#v", snap.APIKeys[0]) + } + if snap.APIKeys[1].Key != "new-key" || snap.APIKeys[1].Name != "" || snap.APIKeys[1].Remark != "" { + t.Fatalf("new legacy key should remain metadata-free: %#v", snap.APIKeys[1]) + } +} + func TestUpdateSettingsPasswordInvalidatesOldJWT(t *testing.T) { hash := authn.HashAdminPassword("old-password") h := newAdminTestHandler(t, `{"admin":{"password_hash":"`+hash+`"}}`) @@ -386,6 +418,79 @@ func TestConfigImportMergePreservesStructuredAPIKeys(t *testing.T) { } } +func TestConfigImportMergeUpgradesLegacyAPIKeys(t *testing.T) { + h := newAdminTestHandler(t, `{ + "keys":["legacy"], + "accounts":[] + }`) + + merge := map[string]any{ + "mode": "merge", + "config": map[string]any{ + "api_keys": []any{ + map[string]any{"key": "legacy", "name": "primary", "remark": "prod"}, + map[string]any{"key": "new-key", "name": "secondary", "remark": "staging"}, + }, + }, + } + mergeBytes, _ := json.Marshal(merge) + mergeReq := httptest.NewRequest(http.MethodPost, "/admin/config/import?mode=merge", bytes.NewReader(mergeBytes)) + mergeRec := httptest.NewRecorder() + h.configImport(mergeRec, mergeReq) + if mergeRec.Code != http.StatusOK { + t.Fatalf("merge status=%d body=%s", mergeRec.Code, mergeRec.Body.String()) + } + + snap := h.Store.Snapshot() + if len(snap.Keys) != 2 || snap.Keys[0] != "legacy" || snap.Keys[1] != "new-key" { + t.Fatalf("unexpected keys after legacy import merge: %#v", snap.Keys) + } + if len(snap.APIKeys) != 2 { + t.Fatalf("unexpected api keys after legacy import merge: %#v", snap.APIKeys) + } + if snap.APIKeys[0].Name != "primary" || snap.APIKeys[0].Remark != "prod" { + t.Fatalf("legacy key metadata was not upgraded: %#v", snap.APIKeys[0]) + } + if snap.APIKeys[1].Name != "secondary" || snap.APIKeys[1].Remark != "staging" { + t.Fatalf("new structured metadata was not preserved: %#v", snap.APIKeys[1]) + } +} + +func TestBatchImportUpgradesLegacyAPIKeys(t *testing.T) { + h := newAdminTestHandler(t, `{ + "keys":["legacy"], + "accounts":[] + }`) + + payload := map[string]any{ + "keys": []any{"legacy", "new-key"}, + "api_keys": []any{ + map[string]any{"key": "legacy", "name": "primary", "remark": "prod"}, + }, + } + b, _ := json.Marshal(payload) + req := httptest.NewRequest(http.MethodPost, "/admin/import", bytes.NewReader(b)) + rec := httptest.NewRecorder() + h.batchImport(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + + snap := h.Store.Snapshot() + if len(snap.Keys) != 2 || snap.Keys[0] != "legacy" || snap.Keys[1] != "new-key" { + t.Fatalf("unexpected keys after batch import: %#v", snap.Keys) + } + if len(snap.APIKeys) != 2 { + t.Fatalf("unexpected api keys after batch import: %#v", snap.APIKeys) + } + if snap.APIKeys[0].Name != "primary" || snap.APIKeys[0].Remark != "prod" { + t.Fatalf("legacy key metadata was not upgraded: %#v", snap.APIKeys[0]) + } + if snap.APIKeys[1].Name != "" || snap.APIKeys[1].Remark != "" { + t.Fatalf("new batch-imported key should stay metadata-free: %#v", snap.APIKeys[1]) + } +} + func TestConfigImportAppliesTokenRefreshInterval(t *testing.T) { h := newAdminTestHandler(t, `{"keys":["k1"]}`) diff --git a/internal/admin/helpers.go b/internal/admin/helpers.go index 8411b3b..c7af36f 100644 --- a/internal/admin/helpers.go +++ b/internal/admin/helpers.go @@ -109,6 +109,78 @@ func toAPIKeys(v any) ([]config.APIKey, bool) { return out, true } +func normalizeAPIKeyForStorage(item config.APIKey) config.APIKey { + return config.APIKey{ + Key: strings.TrimSpace(item.Key), + Name: strings.TrimSpace(item.Name), + Remark: strings.TrimSpace(item.Remark), + } +} + +func apiKeyHasMetadata(item config.APIKey) bool { + return strings.TrimSpace(item.Name) != "" || strings.TrimSpace(item.Remark) != "" +} + +func mergeAPIKeysPreferStructured(existing, incoming []config.APIKey) ([]config.APIKey, int) { + if len(existing) == 0 && len(incoming) == 0 { + return nil, 0 + } + + merged := make([]config.APIKey, 0, len(existing)+len(incoming)) + index := make(map[string]int, len(existing)+len(incoming)) + for _, item := range existing { + item = normalizeAPIKeyForStorage(item) + if item.Key == "" { + continue + } + if _, ok := index[item.Key]; ok { + continue + } + index[item.Key] = len(merged) + merged = append(merged, item) + } + + imported := 0 + for _, item := range incoming { + item = normalizeAPIKeyForStorage(item) + if item.Key == "" { + continue + } + if idx, ok := index[item.Key]; ok { + keep := merged[idx] + next := mergeAPIKeyRecord(keep, item) + if next != keep { + merged[idx] = next + imported++ + } + continue + } + index[item.Key] = len(merged) + merged = append(merged, item) + imported++ + } + + if len(merged) == 0 { + return nil, imported + } + return merged, imported +} + +func mergeAPIKeyRecord(existing, incoming config.APIKey) config.APIKey { + existing = normalizeAPIKeyForStorage(existing) + incoming = normalizeAPIKeyForStorage(incoming) + if existing.Key == "" { + return incoming + } + if apiKeyHasMetadata(existing) { + return existing + } + if apiKeyHasMetadata(incoming) { + return incoming + } + return existing +} + func fieldString(m map[string]any, key string) string { v, ok := m[key] if !ok || v == nil { diff --git a/internal/chathistory/store.go b/internal/chathistory/store.go index 711b001..fcb7fdd 100644 --- a/internal/chathistory/store.go +++ b/internal/chathistory/store.go @@ -12,6 +12,8 @@ import ( "time" "github.com/google/uuid" + + "ds2api/internal/config" ) const ( @@ -379,7 +381,10 @@ func (s *Store) loadLocked() error { raw, err := os.ReadFile(s.path) if err != nil { if errors.Is(err, os.ErrNotExist) { - return s.saveLocked() + if saveErr := s.saveLocked(); saveErr != nil { + config.Logger.Warn("[chat_history] bootstrap write failed", "path", s.path, "error", saveErr) + } + return nil } return fmt.Errorf("read chat history index: %w", err) } @@ -390,7 +395,10 @@ func (s *Store) loadLocked() error { } if legacyOK { s.loadLegacyLocked(legacy) - return s.saveLocked() + if err := s.saveLocked(); err != nil { + config.Logger.Warn("[chat_history] legacy migration writeback failed", "path", s.path, "error", err) + } + return nil } var state File @@ -413,7 +421,10 @@ func (s *Store) loadLocked() error { s.details[item.ID] = detail } s.rebuildIndexLocked() - return s.saveLocked() + if saveErr := s.saveLocked(); saveErr != nil { + config.Logger.Warn("[chat_history] index rewrite failed", "path", s.path, "error", saveErr) + } + return nil } func (s *Store) loadLegacyLocked(legacy legacyFile) { diff --git a/internal/chathistory/store_test.go b/internal/chathistory/store_test.go index a5830bd..78e3e08 100644 --- a/internal/chathistory/store_test.go +++ b/internal/chathistory/store_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "os" "path/filepath" + "strings" "sync" "testing" ) @@ -335,6 +336,56 @@ func TestStoreAutoMigratesMetadataOnlyLegacyMonolith(t *testing.T) { } } +func TestStoreLegacyMigrationBestEffortWhenRewriteFails(t *testing.T) { + path := filepath.Join(t.TempDir(), "chat_history.json") + longID := "chat_" + strings.Repeat("x", 320) + legacy := legacyFile{ + Version: 1, + Limit: 20, + Items: []Entry{{ + ID: longID, + CreatedAt: 1, + UpdatedAt: 2, + Status: "success", + UserInput: "hello", + Content: "world", + }}, + } + body, err := json.MarshalIndent(legacy, "", " ") + if err != nil { + t.Fatalf("marshal legacy file failed: %v", err) + } + if err := os.WriteFile(path, body, 0o644); err != nil { + t.Fatalf("write legacy file failed: %v", err) + } + + store := New(path) + if err := store.Err(); err != nil { + t.Fatalf("expected store to stay usable after migration writeback failure, got %v", err) + } + if !store.Enabled() { + t.Fatal("expected store to remain enabled after best-effort migration") + } + + snapshot, err := store.Snapshot() + if err != nil { + t.Fatalf("snapshot failed: %v", err) + } + if len(snapshot.Items) != 1 || snapshot.Items[0].ID != longID { + t.Fatalf("unexpected snapshot after best-effort migration: %#v", snapshot.Items) + } + full, err := store.Get(longID) + if err != nil { + t.Fatalf("get migrated detail failed: %v", err) + } + if full.Content != "world" { + t.Fatalf("expected migrated content to stay in memory, got %#v", full) + } + if _, statErr := os.Stat(filepath.Join(store.DetailDir(), longID+".json")); statErr == nil { + t.Fatal("expected detail write to fail for overlong legacy id") + } +} + func TestStoreTransientPersistenceFailureDoesNotLatch(t *testing.T) { path := filepath.Join(t.TempDir(), "chat_history.json") store := New(path)