feat: Import OpenClaw memory, history and settings

[nickpismenkov]

Summary

Change Type

• Bug fix
• New feature
• Refactor
• Documentation
• CI/Infrastructure
• Security
• Dependencies

Linked Issue

Validation

• cargo fmt
• cargo clippy --all --benches --tests --examples --all-features
• Relevant tests pass:
• Manual testing:

Security Impact

Database Impact

Blast Radius

Rollback Plan

---

Review track:

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[zmanian]

Review: Import OpenClaw memory, history, and settings

Good feature -- clean reader design, solid test coverage (6 test files with real SQLite fixtures), proper feature gating. Two blocking issues:

Blocking

1. [Security] API keys stored as plain String with Debug derive

src/import/openclaw/reader.rs -- OpenClawLlmConfig and OpenClawEmbeddingsConfig store api_key: Option<String> and derive Debug. Any debug log or panic backtrace leaks keys in cleartext. The keys are wrapped in SecretString later in extract_credentials, but the intermediate structs are exposed.

Fix: Use SecretString for the api_key fields and implement Debug manually with redaction, or at minimum remove the Debug derive and add a manual impl.

2. [Database Safety] No transaction wrapping for multi-step import

src/import/openclaw/mod.rs -- import() performs settings, credentials, workspace, chunk, and conversation writes as individual calls. Per CLAUDE.md: "Multi-step database operations MUST be wrapped in a transaction." A crash mid-import leaves partial state with no way to detect or recover. Especially problematic for conversations where the conversation is created, then messages are added one by one.

Fix: Wrap each conversation + its messages in a transaction at minimum. Ideally wrap the entire import.

Non-blocking

3. super:: imports in settings.rs tests -- use crate::import::openclaw::reader:: per project style
4. Dead code: credentials.rs::import_credentials() is defined but never called (logic inlined in mod.rs)
5. No idempotency: Re-running import creates duplicate conversations (no check for openclaw_conversation_id already imported)
6. Dead #[cfg(not(feature = "import"))] fallbacks: The entire module is feature-gated, so the not-feature stubs are unreachable
7. Dry-run undercounts: Messages within conversations are not counted in dry-run mode
8. Dependencies: json5 brings pest (4 transitive crates), rusqlite bundled compiles SQLite from source. Both behind feature gate so normal builds unaffected. Acceptable.
9. list_agent_dbs nondeterministic order: Filesystem iteration order makes test assertions fragile. Sort by name.

Looks good

• Feature gating keeps deps out of default builds
• No .unwrap()/.expect() in production code
• Credentials wrapped in SecretString before reaching SecretsStore
• Reader is read-only (never modifies source data)
• Tests are substantive -- real SQLite fixtures, error paths, edge cases, UTF-8, idempotency
• tempfile used correctly in all tests
• Dry-run separation is clean

[zmanian]

Follow-up Review (post review-fixes commits)

The two blocking issues from my previous review have been addressed:

1. API keys as SecretString -- Fixed. OpenClawLlmConfig and OpenClawEmbeddingsConfig now use SecretString for api_key fields with manual Debug impls that redact values. Tests verify redaction. Good.

2. Transaction safety -- Partially addressed. The Database trait doesn't expose transaction control, so full wrapping isn't possible without trait changes. The current approach (read-all-first, then write in grouped phases) is a reasonable mitigation. The import_conversation_atomic function validates messages before creating the conversation, which helps. The code comments honestly document the limitation. Acceptable for now.

Remaining issues (non-blocking, should be tracked)

3. Dead import_conversation function -- The old non-atomic import_conversation in history.rs is marked #[allow(dead_code)] and deprecated. It should just be deleted rather than kept around. No callers remain.

4. unwrap_or("unknown") in list_agent_dbs (reader.rs:212) -- This is on file_stem().and_then(|s| s.to_str()).unwrap_or("unknown"). While unlikely to cause issues in practice, this silently processes a file whose name couldn't be decoded as UTF-8. Consider logging a warning or skipping the file instead of silently renaming it to "unknown".

5. #[allow(dead_code)] on LLM code -- The merge commit adds #[allow(dead_code)] to UnsupportedParam, strip_unsupported_completion_params, and strip_unsupported_tool_params in src/llm/provider.rs. These were added in a separate PR (#809) and should already be wired up. The allow(dead_code) annotations suggest a merge conflict resolution that lost the integration wiring. This needs investigation -- if the code from #809 that actually calls these functions was dropped during the merge into this branch, those providers will silently send unsupported parameters again.

6. No idempotency for conversations -- Re-importing creates duplicate conversations. The openclaw_conversation_id is stored in metadata but never checked before creating. The import_conversation_atomic doc comments acknowledge this as a known limitation. Fine for v1 but should be tracked.

7. Emojis in CLI output -- cli/import.rs uses emoji in println! output. Per project convention (CLAUDE.md emoji policy), these should be removed or replaced with plain text markers.

8. unwrap_or_else in cli/import.rs -- Lines 70 and 74 use unwrap_or_else for HOME and user_id respectively. The HOME one is fine (fallback to "."). The user_id one is fine (defaulting to "default"). These are acceptable CLI-level defaults.

Verdict

The review fixes are solid. The security issue is properly resolved, the transaction safety has a documented pragmatic mitigation, dead code was cleaned up (except the old import_conversation), and super::super:: was fixed. The #[allow(dead_code)] on the LLM provider code from the merge is the most concerning item -- please verify the unsupported-param filtering from #809 is still wired up in the providers after the merge.

No blocking issues remain. The items above can be addressed in follow-up work.

[nickpismenkov]

@zmanian sorry, I accidentally merged main into the branch, now it should be fixed, please check again. thanks!

[zmanian]

Re-review after fix commits (6739426, c3646db, 4d6fbba)

The new commits address the issues flagged in my previous two reviews. Here is the status of each:

Previously flagged items -- now resolved

1. Dead import_conversation function -- Deleted in 4d6fbba. Only import_conversation_atomic remains. Good.

2. unwrap_or("unknown") in list_agent_dbs -- Replaced with match that logs a tracing::warn! and skips the file. Correct behavior.

3. Emojis in CLI output -- The two main ones were fixed: "Import Complete" (was checkmark emoji) and "[DRY RUN] No data was written." (was warning emoji). Good.

4. #[allow(dead_code)] on LLM provider code -- Resolved by the merge from staging (c3646db), which brought in the wiring from PR #809. The strip_unsupported_completion_params and strip_unsupported_tool_params functions are now properly called from rig_adapter.rs. No dead code annotations remain.

5. Dry-run message counting -- Fixed in 6739426. The dry-run path now iterates conversations and sums conv.messages.len().

6. super::super:: imports -- No longer present. Remaining super:: usage is single-level within the same package (sibling module refs like use super::reader::OpenClawConfig), which is acceptable.

Minor remaining items (non-blocking, can be follow-up)

1. Remaining emoji on line 76 of cli/import.rs: println!("magnifying glass OpenClaw Import") still has a magnifying glass emoji. The other two were fixed but this one was missed.

2. Unreachable #[cfg(not(feature = "import"))] in reader.rs:187: The entire openclaw module is gated behind #[cfg(feature = "import")] in src/import/mod.rs, so the #[cfg(not(feature = "import"))] block inside reader.rs::read_config() is dead code -- it can never compile. Harmless but should be cleaned up.

3. No conversation import idempotency: Re-importing still creates duplicate conversations. The code documents this limitation clearly with a TODO in import_conversation_atomic. Fine for v1.

4. credentials.rs is test-only: The module contains only tests (the production logic is inlined in mod.rs). Consider renaming or documenting this to avoid confusion, or moving the tests elsewhere.

Verdict

All blocking and significant non-blocking issues from the previous reviews have been addressed. The remaining items are cosmetic. Approving.

[zmanian]

Re-review after fix commits (6739426, c3646db, 4d6fbba)

The new commits address the issues flagged in my previous two reviews. Here is the status of each:

Previously flagged items -- now resolved

1. Dead import_conversation function -- Deleted in 4d6fbba. Only import_conversation_atomic remains. Good.

2. unwrap_or(unknown) in list_agent_dbs -- Replaced with match that logs a tracing::warn! and skips the file. Correct behavior.

3. Emojis in CLI output -- The two main ones were fixed: Import Complete (was checkmark emoji) and [DRY RUN] No data was written (was warning emoji). Good.

4. allow(dead_code) on LLM provider code -- Resolved by the merge from staging (c3646db), which brought in the wiring from PR #809. The strip_unsupported_completion_params and strip_unsupported_tool_params functions are now properly called from rig_adapter.rs. No dead code annotations remain.

5. Dry-run message counting -- Fixed in 6739426. The dry-run path now iterates conversations and sums conv.messages.len().

6. super::super:: imports -- No longer present. Remaining super:: usage is single-level within the same package (sibling module refs like use super::reader::OpenClawConfig), which is acceptable.

Minor remaining items (non-blocking, can be follow-up)

1. Remaining emoji on line 76 of cli/import.rs: The magnifying glass emoji in the OpenClaw Import header was not removed along with the other two.

2. Unreachable cfg(not(feature = import)) in reader.rs:187: The entire openclaw module is gated behind cfg(feature = import) in src/import/mod.rs, so the cfg(not(feature = import)) block inside reader.rs::read_config() is dead code -- it can never compile. Harmless but should be cleaned up.

3. No conversation import idempotency: Re-importing still creates duplicate conversations. The code documents this limitation clearly with a TODO in import_conversation_atomic. Fine for v1.

4. credentials.rs is test-only: The module contains only tests (the production logic is inlined in mod.rs). Consider renaming or documenting this to avoid confusion, or moving the tests elsewhere.

Verdict

All blocking and significant non-blocking issues from the previous reviews have been addressed. The remaining items are cosmetic. Approving.