fix: tool_calls double-encoding on import (#68856)

* nix: add `cage` to devShell

* test(desktop): add pre-filled sessions support

Exports createSandbox, writeMockProviderConfig, writeEnvFile,
buildAppEnv, findElectron, and launchDesktop from fixtures.ts so
specs can compose their own seeded-backend fixtures without duplicating
the sandbox/config/launch logic.

* test(desktop): auto-fail e2e tests on error banner

Adds a shared test fixture (e2e/test.ts) that wraps @playwright/test's
page with an error-banner guard. When any [role="alert"] element
(error notification toast) appears in the DOM during a test, the test
fails with the error message text.

The guard uses:
- A MutationObserver (injected via addInitScript) that watches for
  [role="alert"] elements appearing at any point during the test
- A final DOM scan in afterEach for alerts still visible at teardown
- Deduplication so the same error text only fires once

All existing e2e specs updated to import { test, expect } from './test'
instead of '@playwright/test'. No per-spec setup needed — the guard is
auto-installed on every page via the extended fixture.

This catches issues like the "resume failed" error banner that can
appear during session loading — previously the test would pass while
an error toast was silently visible on screen.

* fix(state): parse tool_calls JSON string before re-serializing

_insert_message_rows and append_message both do json.dumps(tool_calls)
to serialize the field for SQLite storage. But when tool_calls arrives
as a JSON string (from import_sessions / export_session, which store it
as TEXT), json.dumps double-encodes it — wrapping the already-serialized
string in quotes and escaping the inner quotes.

When _rows_to_conversation later does json.loads(row['tool_calls']),
the double-encoded string parses back to a plain string (not a list).
_history_to_messages then iterates this string character-by-character,
calling tc.get('function', {}) on each char — 'str' object has no
attribute 'get'.

This was a pre-existing bug (on main), but only triggered by the
import_sessions path (the live agent always passes tool_calls as a
Python list). The e2e error-banner guard caught it via the 'Resume
failed' notification toast.

Fix: in both append_message and _insert_message_rows, parse tool_calls
with json.loads first if it's a string, then re-serialize.

* fix(desktop): exempt boot-failure from error guard

- boot-failure: add allowErrorBanners() beforeEach — these tests
  deliberately trigger boot errors, so error toasts are expected
- test.ts: export allowErrorBanners() opt-out + reset flag in afterEach

apps/desktop/e2e/boot-failure.spec.ts

@@ -9,7 +9,7 @@
  * Prerequisite: `npm run build` must have been run so dist/ exists.
  */
 
-import { test } from '@playwright/test'
+import { allowErrorBanners, test } from './test'
 
 import {
   type DeadBackendFixture,
@@ -26,6 +26,12 @@ test.afterAll(async () => {
 })
 
 test.describe('boot failure with dead backend', () => {
+  test.beforeEach(() => {
+    // These tests deliberately trigger boot errors — error banners
+    // (notifyError → [role="alert"]) are expected, not failures.
+    allowErrorBanners()
+  })
+
   test('app shows error state', async () => {
     // Inject a fake boot error so the backend resolution "fails" with a
     // controlled error message. This is the only reliable way to trigger

apps/desktop/e2e/boot.spec.ts

@@ -11,7 +11,7 @@
  * Run from the nix devshell:
  *   npm exec playwright test e2e/boot.spec.ts --reporter=list
  */
-import { expect, test } from '@playwright/test'
+import { expect, test } from './test'
 
 import {
   type MockBackendFixture,

apps/desktop/e2e/chat.spec.ts

@@ -8,7 +8,7 @@
  * Prerequisite: `npm run build` must have been run so dist/ exists.
  */
 
-import { test } from '@playwright/test'
+import { test } from './test'
 
 import {
   type MockBackendFixture,

apps/desktop/e2e/fixtures.ts

@@ -28,6 +28,7 @@ import * as path from 'node:path'
 import { _electron, type ElectronApplication, type Page } from '@playwright/test'
 
 import { startMockServer } from './mock-server'
+import { installErrorBannerGuard } from './test'
 
 const DESKTOP_ROOT = path.resolve(import.meta.dirname, '..')
 const REPO_ROOT = path.resolve(DESKTOP_ROOT, '..', '..')
@@ -96,7 +97,7 @@ export interface Sandbox {
   cleanup: () => void
 }
 
-function createSandbox(prefix: string): Sandbox {
+export function createSandbox(prefix: string): Sandbox {
   const root = fs.mkdtempSync(path.join(os.tmpdir(), `hermes-e2e-${prefix}-${Math.random()}`))
   const hermesHome = path.join(root, 'hermes-home')
   const userDataDir = path.join(root, 'electron-user-data')
@@ -140,7 +141,7 @@ function createSandbox(prefix: string): Sandbox {
  * mock inference server. The provider is set as the active model provider so
  * the desktop app skips onboarding and boots straight to the chat UI.
  */
-function writeMockProviderConfig(hermesHome: string, mockUrl: string): void {
+export function writeMockProviderConfig(hermesHome: string, mockUrl: string): void {
   const configPath = path.join(hermesHome, 'config.yaml')
 
   const config = `# Auto-generated by E2E test fixtures
@@ -165,7 +166,7 @@ providers:
  * Write a minimal .env with the mock API key. The key_env in config.yaml
  * references MOCK_API_KEY, so the backend resolves credentials from here.
  */
-function writeEnvFile(hermesHome: string, apiKey = 'e2e-mock-key'): void {
+export function writeEnvFile(hermesHome: string, apiKey = 'e2e-mock-key'): void {
   const envPath = path.join(hermesHome, '.env')
   fs.writeFileSync(envPath, `MOCK_API_KEY=${apiKey}\n`, 'utf8')
 }
@@ -193,7 +194,7 @@ function writeEmptyConfig(hermesHome: string): void {
  *  - HERMES_DESKTOP_APP_NAME → unique-ish per test (avoids single-instance lock)
  *  - XDG_RUNTIME_DIR → ensure Electron has a writable runtime dir on Linux
  */
-function buildAppEnv(sandbox: Sandbox, extra: Record<string, string> = {}): Record<string, string> {
+export function buildAppEnv(sandbox: Sandbox, extra: Record<string, string> = {}): Record<string, string> {
   const clean = stripCredentials(process.env)
 
   // XDG_RUNTIME_DIR is needed for Electron on Linux when running in a
@@ -252,7 +253,7 @@ function assertDistBuilt(): void {
  * Find the Electron binary. In the nix devshell, `electron` is on PATH.
  * As a fallback, use the node_modules/.bin/electron from the desktop package.
  */
-function findElectron(): string {
+export function findElectron(): string {
   // In dev mode, we use the `electron` binary directly (not the packaged app).
   // The dev:electron script in package.json does exactly this: `electron .`
   // after building. We replicate that here.
@@ -283,7 +284,7 @@ function findElectron(): string {
  * @param env      - the process environment (already has HERMES_HOME etc.)
  * @returns the ElectronApplication + first Page
  */
-async function launchDesktop(
+export async function launchDesktop(
   env: Record<string, string>,
 ): Promise<{ app: ElectronApplication; page: Page }> {
   assertDistBuilt()
@@ -305,6 +306,10 @@ async function launchDesktop(
 
   const page = await app.firstWindow()
 
+  // Install the error-banner guard so any [role="alert"] that appears
+  // during a test is collected and surfaced in afterEach.
+  installErrorBannerGuard(page)
+
   return { app, page }
 }
 
@@ -514,6 +519,7 @@ export async function setupPackagedApp(): Promise<PackagedAppFixture> {
   })
 
   const page = await app.firstWindow()
+  installErrorBannerGuard(page)
 
   return {
     app,

apps/desktop/e2e/launch-packaged-app.spec.ts

@@ -1,4 +1,4 @@
-import { expect, test } from '@playwright/test'
+import { expect, test } from './test'
 
 import {
   PACKAGED_BINARY_PATH,

apps/desktop/e2e/mock-backend-setup.spec.ts

@@ -15,7 +15,7 @@
  * Prerequisite: `npm run build` must have been run so dist/ exists.
  */
 
-import { expect, test } from '@playwright/test'
+import { expect, test } from './test'
 
 import {
   type MockBackendFixture,

apps/desktop/e2e/onboarding.spec.ts

@@ -9,7 +9,7 @@
  * Prerequisite: `npm run build` must have been run so dist/ exists.
  */
 
-import { expect, test } from '@playwright/test'
+import { expect, test } from './test'
 
 import {
   type NoProviderFixture,

apps/desktop/e2e/scripts/seed_session_db.py

@@ -0,0 +1,56 @@
+#!/usr/bin/env python3
+"""Seed a Hermes state.db with a session exported from a real conversation.
+
+Usage: seed_session_db.py <state_db_path> <fixture_json_path>
+
+Creates the database with the full SessionDB schema (if it doesn't exist)
+and imports the session from the JSON fixture. Uses the real
+SessionDB.import_sessions() so the data shape matches what the desktop
+backend expects.
+"""
+import json
+import sys
+from pathlib import Path
+
+# Add the repo root to sys.path so we can import hermes_state.
+# The script is invoked from apps/desktop/e2e/ — repo root is ../../..
+repo_root = Path(__file__).resolve().parents[3]
+sys.path.insert(0, str(repo_root))
+
+from hermes_state import SessionDB  # noqa: E402
+
+
+def main():
+    if len(sys.argv) != 3:
+        print(f"Usage: {sys.argv[0]} <state_db_path> <fixture_json_path>", file=sys.stderr)
+        sys.exit(1)
+
+    db_path = Path(sys.argv[1])
+    fixture_path = Path(sys.argv[2])
+
+    db_path.parent.mkdir(parents=True, exist_ok=True)
+
+    with open(fixture_path, "r", encoding="utf-8") as f:
+        session_data = json.load(f)
+
+    db = SessionDB(db_path=db_path)
+    result = db.import_sessions([session_data])
+
+    if not result.get("ok"):
+        print(f"Import failed: {result}", file=sys.stderr)
+        sys.exit(1)
+
+    imported = result.get("imported", 0)
+    skipped = result.get("skipped", 0)
+    errors = result.get("errors", [])
+
+    if errors:
+        print(f"Import had errors: {errors}", file=sys.stderr)
+        sys.exit(1)
+
+    print(f"Seeded {imported} session(s), skipped {skipped} → {db_path}")
+    db.close()
+
+
+if __name__ == "__main__":
+    main()

apps/desktop/e2e/test.ts

@@ -0,0 +1,166 @@
+/**
+ * Extended Playwright test fixture that auto-fails any test if an error
+ * banner (notification toast with role="alert") appears in the DOM.
+ *
+ * The desktop app surfaces errors as `[data-slot="alert"][role="alert"]`
+ * elements (see components/notifications.tsx). When one appears during a
+ * test, it means something went wrong (resume failed, boot error, etc.)
+ * — the test should fail with the error message, not silently pass while
+ * an error toast is visible on screen.
+ *
+ * Usage: import { test, expect } from './test' instead of
+ * '@playwright/test'. The guard is auto-installed on every page — no
+ * per-spec setup needed.
+ */
+
+import { test as base, expect, type Page, type ElectronApplication, _electron } from '@playwright/test'
+
+// Track error messages per test so afterEach can assert + report.
+const seenErrors: string[] = []
+let activePage: Page | null = null
+// When true, the afterEach guard skips the error-banner check.
+// Set by tests that deliberately trigger error states (e.g. boot-failure).
+let errorBannersAllowed = false
+
+/**
+ * Opt out of the error-banner guard for the current test. Call in
+ * test.beforeEach or at the top of a test body when error banners are
+ * expected (e.g. boot-failure tests that deliberately trigger errors).
+ */
+export function allowErrorBanners(): void {
+  errorBannersAllowed = true
+}
+
+/**
+ * Install the error-banner guard on a page. Watches for `[role="alert"]`
+ * elements appearing in the DOM. When one is found, records its text
+ * content for the afterEach assertion.
+ *
+ * Exported so e2e fixture functions (which create pages via _electron.launch)
+ * can install the guard on their custom pages — the default Playwright `page`
+ * fixture override only catches pages created by Playwright itself, not
+ * pages created by the test's own Electron launch.
+ */
+export function installErrorBannerGuard(page: Page): void {
+  activePage = page
+
+  // Clear any errors from a previous test when a new page is created.
+  seenErrors.length = 0
+
+  // Use a MutationObserver to catch error banners as they appear.
+  // We inject this via addInitScript so it runs before any app code.
+  page.addInitScript(() => {
+    const seen: string[] = []
+    ;(window as unknown as { __ERROR_BANNER_GUARD__?: string[] }).__ERROR_BANNER_GUARD__ = seen
+
+    const observer = new MutationObserver(() => {
+      const alerts = document.querySelectorAll('[role="alert"]')
+
+      for (const alert of alerts) {
+        const text = (alert.textContent ?? '').trim()
+
+        if (text && !seen.includes(text)) {
+          seen.push(text)
+        }
+      }
+    })
+
+    // Start observing once the DOM is ready.
+    if (document.body) {
+      observer.observe(document.body, { childList: true, subtree: true })
+    } else {
+      document.addEventListener('DOMContentLoaded', () => {
+        observer.observe(document.body, { childList: true, subtree: true })
+      })
+    }
+  })
+
+  // Also poll via evaluate — MutationObserver via addInitScript can miss
+  // elements that appear during the Electron renderer's initial mount
+  // (before the observer is installed). A periodic poll catches those.
+  page.on('console', () => {
+    // Console messages are not errors — but we keep the listener to
+    // ensure the page context is active for our evaluate calls.
+  })
+}
+
+/**
+ * Check for error banners that appeared during the test. Called in
+ * afterEach via the custom fixture below. Also exported so specs that
+ * manage their own page lifecycle can call it directly.
+ */
+export async function collectErrorBanners(page: Page | null): Promise<string[]> {
+  if (!page) {
+    return []
+  }
+
+  try {
+    // Read errors collected by the MutationObserver in the page context.
+    const pageErrors = await page.evaluate(() => {
+      const w = window as unknown as { __ERROR_BANNER_GUARD__?: string[] }
+
+      return [...(w.__ERROR_BANNER_GUARD__ ?? [])]
+    })
+
+    // Also do a final DOM scan for any alert elements still visible.
+    const domAlerts = await page
+      .locator('[role="alert"]')
+      .allTextContents()
+      .catch(() => [] as string[])
+
+    const all = [...new Set([...pageErrors, ...domAlerts.map(t => t.trim()).filter(Boolean)])]
+    seenErrors.push(...all)
+
+    return [...new Set(seenErrors)]
+  } catch {
+    // Page might be closed — return whatever we have.
+    return [...new Set(seenErrors)]
+  }
+}
+
+// Extended test fixture: wraps the default page with the error guard.
+export const test = base.extend({
+  // Override the page fixture to auto-install the guard.
+  page: async ({ page }, use) => {
+    installErrorBannerGuard(page)
+    await use(page)
+  },
+})
+
+// afterEach: fail the test if any error banners appeared.
+// Always fires — even if the test already failed for another reason.
+// An error banner often IS the root cause (e.g. "resume failed" from a
+// backend bug), and suppressing it when the test also fails on an
+// assertion hides the real problem.
+//
+// Uses `activePage` (set by installErrorBannerGuard) instead of the
+// default `page` fixture — Electron tests create their own page via
+// app.firstWindow(), so the default `page` fixture is undefined.
+base.afterEach(async ({}, testInfo) => {
+  const wasAllowed = errorBannersAllowed
+  // Reset for the next test.
+  errorBannersAllowed = false
+
+  if (wasAllowed) {
+    // Test opted out — clear any collected errors without asserting.
+    seenErrors.length = 0
+    return
+  }
+
+  const errors = await collectErrorBanners(activePage)
+
+  if (errors.length > 0) {
+    throw new Error(
+      `Error banner(s) appeared during test "${testInfo.title}":\n` +
+        errors.map(e => `  • ${e}`).join('\n'),
+    )
+  }
+})
+
+// Reset for the next test file.
+base.afterAll(async () => {
+  seenErrors.length = 0
+  activePage = null
+})
+
+export { expect, type Page, type ElectronApplication, _electron }

hermes_state.py

@@ -4203,6 +4203,14 @@ class SessionDB:
             json.dumps(codex_message_items)
             if codex_message_items else None
         )
+        # tool_calls may arrive as a Python list (from the live agent) or
+        # as a JSON string (from import/export). Parse first to avoid
+        # double-encoding.
+        if isinstance(tool_calls, str):
+            try:
+                tool_calls = json.loads(tool_calls)
+            except (json.JSONDecodeError, TypeError):
+                tool_calls = []
         tool_calls_json = json.dumps(tool_calls) if tool_calls else None
         # Multimodal content (list of parts) must be JSON-encoded: sqlite3
         # cannot bind list/dict parameters directly.
@@ -4311,6 +4319,15 @@ class SessionDB:
             codex_message_items_json = (
                 json.dumps(codex_message_items) if codex_message_items else None
             )
+            # tool_calls may arrive as a Python list (from the live agent)
+            # or as a JSON string (from import_sessions / export_session,
+            # which store it as TEXT). json.dumps on an already-serialized
+            # string double-encodes it, so parse first.
+            if isinstance(tool_calls, str):
+                try:
+                    tool_calls = json.loads(tool_calls)
+                except (json.JSONDecodeError, TypeError):
+                    tool_calls = []
             tool_calls_json = json.dumps(tool_calls) if tool_calls else None
             # Accept either `platform_message_id` (new explicit name) or
             # `message_id` (yuanbao's existing convention on message dicts).

nix/devShell.nix

@@ -25,20 +25,24 @@
     in
     {
       devShells.default = pkgs.mkShell {
-        packages =
-          with pkgs;
-          [
-            (pkgs.runCommand "hermes" { } ''
-              mkdir -p $out/bin
-              install -Dm755 ${../hermes} $out/bin/hermes
-            '')
-            (pkgs.runCommand "dev-sandbox" { } ''
-              mkdir -p $out/bin
-              install -Dm755 ${../scripts/dev-sandbox.sh} $out/bin/sandbox
-            '')
-            uv
-          ]
-          ++ self'.packages.default.passthru.devDeps;
+        packages = with pkgs; [
+          (pkgs.runCommand "hermes" { } ''
+            mkdir -p $out/bin
+            install -Dm755 ${../hermes} $out/bin/hermes
+          '')
+          (pkgs.runCommand "dev-sandbox" { } ''
+            mkdir -p $out/bin
+            install -Dm755 ${../scripts/dev-sandbox.sh} $out/bin/sandbox
+          '')
+          uv
+          # Headless Wayland compositor for E2E tests (test:e2e:visual).
+          # cage renders a single client with no window management, so
+          # the Electron window opens at a fixed size without tiling.
+          # libglvnd provides libEGL.so.1 that cage needs on NixOS.
+          cage
+          libglvnd
+        ]
+        ++ self'.packages.default.passthru.devDeps;
         shellHook = ''
           ${combinedNonNpm}
           ${hermesNpmLib.mkNpmDevShellHook npmPackageJsonPaths}