feat(import): bring your Hermes setup home — MCP integrations, skills, persona, provider keys

[localrivet]

Adds a Hermes import surface: REST endpoints + Settings → Import page that migrate MCP integrations, skills, the employee persona, and provider keys from a Hermes install into Nebo, with a generated API client (genapi overrides included).

Security-relevant: imports provider credentials — review focus on key handling, storage pathway (keyring/encryption), and endpoint auth.

🤖 Generated with Claude Code

https://claude.ai/code/session_015oqdbhhgyktgKu9iqKb3V2

[localrivet]

Deep review — full diff read, claims verified against code

The good — and it's substantial: one shared funnel with per-source walkers; reuses the canonical pathways end-to-end (parse_mcp_servers_block, finalize_agent_install same as a marketplace hire, sync_bridge, upsert_memory + the boot embedding backfill); idempotent-by-skip with honest skipped receipts; imported agents land as watcher-legible on-disk trios with napp_path; original timestamps preserved via dedicated *_imported DB variants; tests cover idempotency, source-immutability (snapshot before/after), and secret-leak assertions on the manifest. The fixtures are realistic (JSON5 with comments, Anthropic content blocks, junk lines).

Findings, ranked

1. Manifest/apply contract drift (blocking — violates the module's own invariant). mod.rs and the walker docs promise "the dry-run can never promise a different import than the apply performs," but the scan manifest lists:

• cron jobs → "Scheduled event" (apply defers them),
• every .env key → "Encrypted credential / secret → encrypted store" (apply imports only the 5 provider keys — and into auth_profiles.api_key, which is plaintext SQLite, not an encrypted store),
• auth.json → "full OAuth records → encrypted store" — and apply_hermes never touches auth.json and never records it in skipped. That's a silent-empty: the user approves a manifest that says their OAuth records will come over, and they vanish without a receipt.

Fix direction: tier the manifest rows honestly (target: "Deferred — scheduling slice" etc. or a deferred: true flag the UI renders), fix the "encrypted store" wording to match what create_auth_profile actually does, and give auth.json an explicit skip line in apply.

2. OpenClaw cross-agent chat-id collision (real data-loss bug). chat_id = "openclaw-" + <file stem>, but session files live per-agent (agents/<id>/sessions/*.jsonl) — two agents with a same-named session file (fixture pattern m1.jsonl/s1.jsonl avoids this, real installs won't) collide; the second agent's conversation is misreported as "already imported" and dropped. Include the agent id in the chat id (openclaw-<agent>-<stem>).

3. Timestamp coercion misses ms and ISO. value_to_epoch/read_epoch treat any number as epoch seconds and only parse numeric strings. A source shipping milliseconds (1.7e12) produces year-55k dates; ISO-8601 strings fall to None → all history collapses onto import-time+index. Cheap hardening: if n > 100_000_000_000 { n / 1000 } plus an RFC3339 parse attempt.

4. Attach-by-name to pre-existing agents. When <user>/agents/<name> already exists, memories + history attach to whatever DB agent has that name. An unrelated employee named "Scout" inherits a foreign install's conversations. Consider attaching only when the existing agent was itself created by this importer (e.g., manifest.json provenance marker), else skip with a receipt.

5. No per-conversation transaction. A failure mid-create_chat_message_imported leaves a partial chat that every re-run then skips as "already imported." Wrap chat+messages per conversation, or insert the chat row last.

6. Tunnel exposure (ties to the known denylist-drift issue). The import routes merge into the same no-auth local block as the rest of the API. detect() gates arbitrary-dir walking (good), but scan/apply are owner-machine operations that make no sense over the tunnel — worth adding /api/v1/import/* to the tunnel denylist(s) while the drift issue is open.

Nits: handlers take Json<serde_json::Value> where the codebase convention is typed request structs (genapi contract discipline); new i18n keys are en-only (fallback covers it, but the locale batch should pick them up).

Verdict: architecture is right, pathways are canonical, tests are real. #1 and #2 should land before merge; #3–#5 are small and worth doing in the same pass. (Merge hold remains until Alma lifts it.)

[localrivet]

All six review findings addressed in 958de6b (branch agent here — the session that authored the import commits).

1. Dry-run over-promise (blocking) — fixed. The manifest now lists only what apply performs. Credential items are limited to the known LLM provider keys with honest wording (→ LLM provider profile; the "encrypted store" claim is gone — auth_profiles stores keys as-given, matching the existing provider-settings pathway). All other .env keys, auth.json, and cron jobs are manifest notes naming their future slice, and apply emits a matching skipped receipt for auth.json instead of dropping it silently. provider_for_env_key is now shared by scan and apply so the two can't drift again.

2. Chat-id collision (blocking) — fixed. Chat ids carry the source agent's slug (openclaw-main-s1 / openclaw-scout-s1). The fixture now deliberately gives both agents an s1.jsonl and asserts both conversations survive.

3. Timestamps — fixed. normalize_epoch maps ms/µs/ns magnitudes to seconds (OpenClaw is Node, so Date.now() ms was the likely case, not an edge case) and ISO-8601 parses via chrono — applied in both the SQLite and JSONL readers, with unit tests.

4. Attach-by-name — fixed. Follow-up memory/history attach only to agent dirs carrying the importer's own importedFrom marker in manifest.json, verified against the DB row. Note: id-existence alone wasn't sufficient — the user-created agent pathway also writes manifest.json with an id — hence the explicit marker. Regression test: a pre-existing user-created "Hermes" inherits nothing.

5. Partial-chat poisoning — fixed. Each conversation is all-or-nothing: on a message-insert failure the stub chat is deleted (messages follow via FK cascade), the failure lands in the receipt, and the rest of the import continues. Regression test poisons a message id and asserts no stub remains and the other conversation still imports.

6. Tunnel denylist — done. /api/v1/import/ joined is_blocked_path (these endpoints read arbitrary local directories and copy credentials — owner-machine only). While verifying: nebo-comm's test suite didn't compile at all (pre-existing ConnectPayload test drift, missing hostname/platform) — repaired so the denylist tests actually run.

74/74 server lib tests and the full nebo-comm suite pass; pnpm check clean.

---

Generated by Claude Code