From 37b724f4902eeaeb2b79e5d2197790b9bcd6db38 Mon Sep 17 00:00:00 2001 From: yujiezhang-ops Date: Wed, 19 Aug 2026 15:23:32 +0800 Subject: [PATCH] fix: migrate configs saved with a BOM, and don't stop at the first failure A user could not migrate from OneAgent: "migrate C:\Users\...\.openclaw\ openclaw.json: hujson: line 1, column 1: invalid character '\ufeff' at start of value". That character is a UTF-8 BOM, which Windows Notepad writes by default and every editor that does renders invisible. Neither parser accepts it -- hujson rejects it as above, go-toml as "invalid character at start of key" -- so the whole migration failed for someone whose only mistake was opening a config in an editor. The mark is now stripped after reading and dropped rather than written back, because JSON and TOML both define it as invalid content: preserving it would mean rewriting a file we just proved neither parser will read. The failure was worse than one file. The six configs belong to different Agents and nothing links them, but the loop returned on the first error, so a BOM in openclaw.json also left .kimi-code and .zcode unmigrated -- and the message named only openclaw, so nothing hinted that later files had been skipped. Failures are collected and joined now, and each file is attempted regardless of what happened to the ones before it. The notice that carries this message also gets a bound. It renders as a flex sibling of .page-scaffold, so its height comes out of the page's; the scaffold already uses `flex: 1` for this reason, but it pairs that with `min-height: 0` and so keeps yielding to a sibling with no limit of its own. Capping the notice with `overflow: auto` keeps the whole message reachable while giving the page a real floor, and `overflow-wrap: anywhere` stops a path like C:\Users\11811\.openclaw\openclaw.json -- which has no spaces to break at -- from forcing the line wider than the column. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/components/AppWindow.tsx | 2 +- frontend/src/styles/app.css | 24 ++++++++ internal/config/migrate.go | 38 +++++++++++-- internal/config/migrate_test.go | 82 +++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 5 deletions(-) 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) + } +}