diff --git a/frontend/src/components/AppWindow.tsx b/frontend/src/components/AppWindow.tsx index 2964785b..42de7da8 100644 --- a/frontend/src/components/AppWindow.tsx +++ b/frontend/src/components/AppWindow.tsx @@ -12,7 +12,7 @@ export function AppWindow({ children }: PropsWithChildren) {
- {state.status?.migrationNotice ?

{state.status.migrationNotice}

: null} + {state.status?.migrationNotice ?

{state.status.migrationNotice}

: null} {children}
diff --git a/frontend/src/styles/app.css b/frontend/src/styles/app.css index ed9d48bd..d00a2d1f 100644 --- a/frontend/src/styles/app.css +++ b/frontend/src/styles/app.css @@ -2124,6 +2124,30 @@ line-height: 1.6; } +/* The migration notice sits above .page-scaffold as its flex sibling, so its + height is taken out of the page's. It carries a backend error containing a + filesystem path and a parser message, which on a narrow window wrapped to + several lines and pushed the footer -- and the buttons in it -- past the + bottom edge. .page-scaffold already uses `flex: 1` for this reason, but + `flex` alone cannot help: it also sets `min-height: 0`, so the scaffold + keeps yielding space to a sibling that has no bound of its own. + + The cap is what makes the page's floor real, and `overflow: auto` keeps the + whole message reachable rather than clipping it. `overflow-wrap: anywhere` + is for the path: `C:\Users\...\openclaw.json` has no spaces to break at, so + without it a long one forces the line wider than the column. */ +.app-migration-notice { + flex: 0 0 auto; + max-height: 5.5em; + overflow: auto; + overflow-wrap: anywhere; + scrollbar-gutter: stable; + margin: 14px 42px 0; + color: var(--text-tertiary); + font-size: 11px; + line-height: 1.6; +} + .profile-key { display: inline-flex; align-items: center; diff --git a/internal/config/migrate.go b/internal/config/migrate.go index 40bfc70f..d03c62f8 100644 --- a/internal/config/migrate.go +++ b/internal/config/migrate.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -30,27 +31,56 @@ func MigrateLegacyAgentConfigs(ctx context.Context, home string, filesystem secu {filepath.Join(home, ".kimi-code", "config.toml"), migrateLegacyKimi}, {filepath.Join(home, ".zcode", "v2", "config.json"), migrateLegacyZCode}, } + var failures []error for _, migration := range migrations { data, err := os.ReadFile(migration.path) if os.IsNotExist(err) { continue } if err != nil { - return fmt.Errorf("read %s: %w", migration.path, err) + failures = append(failures, fmt.Errorf("read %s: %w", migration.path, err)) + continue } - updated, changed, err := migration.migrate(data) + updated, changed, err := migration.migrate(stripBOM(data)) if err != nil { - return fmt.Errorf("migrate %s: %w", migration.path, err) + // Collected rather than returned: these files belong to different + // Agents and nothing links them, so one unreadable config used to + // abandon every migration listed after it. A user with a hand-edited + // openclaw.json silently kept an unmigrated .zcode config as well, + // and the reported error named only the first file. + failures = append(failures, fmt.Errorf("migrate %s: %w", migration.path, err)) + continue } if changed { if _, err := filesystem.AtomicWrite(ctx, migration.path, updated, true); err != nil { - return err + failures = append(failures, err) } } } + if len(failures) > 0 { + return errors.Join(failures...) + } return nil } +// utf8BOM is the byte order mark Windows editors prepend when saving as UTF-8. +// Notepad writes it by default, and it is invisible in every editor that does. +const utf8BOM = "\ufeff" + +// stripBOM removes a leading UTF-8 BOM so a config saved by a Windows editor can +// be parsed. +// +// Neither parser tolerates it: hujson reports `invalid character '\ufeff' at +// start of value` and go-toml reports `invalid character at start of key`. Since +// a migration failure named only the file, the whole feature looked broken to a +// user whose only mistake was opening a config in Notepad. The mark is dropped +// rather than preserved on write, because JSON and TOML both define it as invalid +// content -- keeping it would mean writing back a file we just proved neither +// parser will read. +func stripBOM(data []byte) []byte { + return []byte(strings.TrimPrefix(string(data), utf8BOM)) +} + func migrateLegacyCodex(data []byte) ([]byte, bool, error) { var parsed map[string]any if err := toml.Unmarshal(data, &parsed); err != nil { diff --git a/internal/config/migrate_test.go b/internal/config/migrate_test.go index 46a30304..8129b6a4 100644 --- a/internal/config/migrate_test.go +++ b/internal/config/migrate_test.go @@ -63,3 +63,85 @@ model = "m"`, } } } + +// A UTF-8 BOM is what Windows Notepad writes by default, and it is invisible in +// every editor that does. Both parsers reject it outright -- hujson with +// `invalid character '\ufeff' at start of value`, go-toml with `invalid +// character at start of key` -- so a user whose only mistake was opening a +// config in Notepad saw the whole migration fail. +func TestMigrateAcceptsConfigsSavedWithABOM(t *testing.T) { + home := t.TempDir() + files := map[string]string{ + ".codex/config.toml": "\ufeff[model_providers.oneagent]\nbase_url = \"https://x\"\n", + ".config/opencode/opencode.json": "\ufeff" + `{"provider":{"oneagent":{"x":1}},"model":"oneagent/m"}`, + ".openclaw/openclaw.json": "\ufeff" + `{"models":{"providers":{"oneagent":{"y":2}}}}`, + ".kimi-code/config.toml": "\ufeff[providers.oneagent]\nbase_url = \"https://x\"\n", + ".zcode/v2/config.json": "\ufeff" + `{"provider":{"old":{"name":"OneAgent - PPIO"}}}`, + } + for name, content := range files { + path := filepath.Join(home, name) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + fs := securefs.New(securefs.Options{OS: "linux", BackupRoot: filepath.Join(home, ".bootagent", "backup")}) + if err := MigrateLegacyAgentConfigs(context.Background(), home, fs); err != nil { + t.Fatalf("migration of BOM-prefixed configs failed: %v", err) + } + for name := range files { + data, err := os.ReadFile(filepath.Join(home, name)) + if err != nil { + t.Fatal(err) + } + if strings.Contains(strings.ToLower(string(data)), "oneagent") { + t.Errorf("%s still contains oneagent: %s", name, data) + } + // Dropped rather than preserved: JSON and TOML both define the mark as + // invalid content, so writing it back would produce a file neither parser + // will read on the next run. + if strings.HasPrefix(string(data), "\ufeff") { + t.Errorf("%s was rewritten with the BOM still on it", name) + } + } +} + +// The six configs belong to different Agents and nothing links them. One that +// cannot be parsed used to abandon every migration listed after it, and the +// reported error named only the first file -- so a single hand-edited config +// silently left later ones on the old provider name. +func TestMigrateContinuesPastAFileItCannotParse(t *testing.T) { + home := t.TempDir() + // openclaw is listed before zcode, so a broken openclaw is what used to stop + // zcode from being migrated at all. + broken := filepath.Join(home, ".openclaw", "openclaw.json") + zcode := filepath.Join(home, ".zcode", "v2", "config.json") + for path, content := range map[string]string{ + broken: `{"models":{"providers":{"oneagent":`, + zcode: `{"provider":{"old":{"name":"OneAgent - PPIO"}}}`, + } { + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + fs := securefs.New(securefs.Options{OS: "linux", BackupRoot: filepath.Join(home, ".bootagent", "backup")}) + err := MigrateLegacyAgentConfigs(context.Background(), home, fs) + if err == nil { + t.Fatal("expected the unparsable config to be reported") + } + if !strings.Contains(err.Error(), ".openclaw") { + t.Errorf("error does not name the file that failed: %v", err) + } + data, readErr := os.ReadFile(zcode) + if readErr != nil { + t.Fatal(readErr) + } + if strings.Contains(strings.ToLower(string(data)), "oneagent") { + t.Errorf("zcode was skipped because an earlier file failed: %s", data) + } +}