Skip to content

fix: handle non-UTF-8 files in OpenClaw migration script - #8898

Closed
chancelu wants to merge 1 commit into
NousResearch:mainfrom
chancelu:fix/migration-unicode-decode-error
Closed

fix: handle non-UTF-8 files in OpenClaw migration script#8898
chancelu wants to merge 1 commit into
NousResearch:mainfrom
chancelu:fix/migration-unicode-decode-error

Conversation

@chancelu

Copy link
Copy Markdown
Contributor

Problem

On Windows systems with non-UTF-8 default encodings (e.g. GBK on Chinese Windows), hermes claw migrate crashes with UnicodeDecodeError when the source OpenClaw directory contains:

  • Binary files (.jpg, .ogg, .sqlite) in media/, memory/, or workspace/
  • Text files with mixed or non-UTF-8 encodings (e.g. log files written by Windows processes)

The error occurs during the preview phase, so the migration cannot even show what would be imported:

✗ Migration preview failed: 'utf-8' codec can't decode byte 0xb3 in position 23114: invalid start byte

Root Cause

Several file-reading functions in openclaw_to_hermes.py use path.read_text(encoding="utf-8") without error handling. When any file in the OpenClaw directory tree contains non-UTF-8 bytes, the entire migration aborts.

Fix

  • read_text(): add errors="replace" to gracefully substitute invalid bytes with U+FFFD instead of crashing
  • load_yaml_file(): catch UnicodeDecodeError alongside yaml.YAMLError
  • parse_env_file(): catch UnicodeDecodeError when reading .env files
  • load_openclaw_config(): catch UnicodeDecodeError alongside json.JSONDecodeError
  • migrate_daily_memory(): add per-file try/except to skip unreadable files instead of aborting the entire loop

Testing

Tested on Windows 11 Pro (Chinese locale, GBK default encoding) with an OpenClaw directory containing .jpg, .ogg, .sqlite, and mixed-encoding .md files. Before the fix, hermes claw migrate --dry-run crashed immediately. After the fix, it completes successfully and correctly identifies 44 items for migration.

@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #9145 — both fix the same UnicodeDecodeError in OpenClaw migration (#8901). #9145 was already triaged as a duplicate of this PR.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for addressing the Windows encoding failure. The current main premise is confirmed: read_text() is strict UTF-8 at optional-skills/migration/openclaw-migration/scripts/openclaw_to_hermes.py:315, and daily-memory migration calls it for each .md file at :1904.

Problems

  • The fix does not cover three remaining strict source JSON reads: :1211 (exec-approvals.json), :1346 (Telegram allowlist), and :1620 (auth-profiles.json). Any non-UTF-8 file there can still raise before the existing JSONDecodeError handling.
  • The diff adds no regression tests. Current daily-memory coverage is UTF-8-only at tests/skills/test_openclaw_migration.py:695-723.
  • Once a read uses errors="replace", its adjacent UnicodeDecodeError handler cannot receive a decoding failure. Please use one explicit recovery policy rather than retaining unreachable exception branches.

Suggested changes

  • Cover all source JSON/text readers and add invalid-byte tests that verify migration continues while valid neighboring items remain migratable.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 12, 2026
@chancelu

Copy link
Copy Markdown
Contributor Author

All three issues from the sweeper review are addressed in the latest commit (703949fe):

  • Added errors="replace" to the three remaining strict source reads flagged: exec-approvals.json, the Telegram allowlist file, and auth-profiles.json. Each now also has an explicit except OSError branch that records a migration error instead of raising.
  • Removed the now-unreachable UnicodeDecodeError except branches (in load_yaml_file, parse_env_file, the daily-memory loop, and load_openclaw_config) so there is one explicit recovery policy (errors="replace" + OSError for I/O failures) instead of dead exception handling.
  • Added 4 regression tests for invalid-byte scenarios covering command-allowlist, messaging-settings/Telegram, provider-keys, and daily-memory migrations. Each test injects an invalid UTF-8 byte into one record/file and asserts migration still completes and a valid neighboring item is correctly migrated.

Ran the full tests/skills/test_openclaw_migration.py suite locally: 34 passed, 2 failed. The 2 failures (test_cron_store_is_archived_without_config_cron_section, test_migrator_can_rename_conflicting_imported_skill) are pre-existing and unrelated to this fix — they fail on main too, caused by hardcoded forward-slash path assertions on Windows.

@chancelu
chancelu force-pushed the fix/migration-unicode-decode-error branch from 703949f to 77534ec Compare July 17, 2026 10:20
@chancelu

Copy link
Copy Markdown
Contributor Author

@teknium1 Thanks for the detailed review — rebased onto current main and resolved the conflict in tests/skills/test_openclaw_migration.py (upstream's new model-config tests kept intact; my four tests moved to the end of the file). Point by point:

1. Three remaining strict JSON reads — covered.

  • exec-approvals.json (migrate_command_allowlist): now reads with errors="replace" and records an OSError as a per-item migration error instead of aborting.
  • Telegram allowlist (migrate_messaging_settings): same treatment — errors="replace" plus an OSError branch that records "Could not read Telegram allowlist file: ...".
  • auth-profiles.json (migrate_provider_keys): same treatment.

I also swept the two other strict readers on the same path — load_yaml_file and parse_env_file now tolerate undecodable bytes and unreadable files, and load_openclaw_config catches OSError alongside JSONDecodeError.

2. Regression tests — added.

Four new tests at the end of tests/skills/test_openclaw_migration.py:

  • test_command_allowlist_handles_invalid_utf8_bytes
  • test_messaging_settings_handles_invalid_utf8_in_telegram_allowlist
  • test_provider_keys_handles_invalid_utf8_in_auth_profiles
  • test_daily_memory_skips_undecodable_file_but_merges_others

Each plants a single invalid byte (0xB3) inside an otherwise valid file and asserts the migration still completes and the cleanly-encoded values survive. Verified locally: all four fail on unpatched main and pass with this diff.

3. One explicit recovery policy — done.

Every source read now goes through a single policy: decode UTF-8 with errors="replace" (so UnicodeDecodeError cannot fire — noted in comments at each site), and treat OSError as the only read failure, recorded per-item so one bad file never aborts the run. No mixed replace/raise paths remain.

Could you re-review and approve the workflow run when you have a moment? Happy to adjust if you'd prefer a different recovery policy (e.g. skip-and-record instead of replace).

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #71078 — your commit(s) were cherry-picked onto current main with your authorship preserved in git log (rebase merge). This PR was part of the class-wide close-out of bare read_text/write_text calls: all 139 remaining sites now pass explicit encoding, and a new CI linter rule prevents regressions. Thanks for the contribution!

@teknium1 teknium1 closed this Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants