Improve coverage for retry/error observability utilities (#257)

plugin/tests/ConnectionManager.test.ts

@@ -0,0 +1,38 @@
+import { describe, expect, it } from 'vitest';
+import { ConnectionManager } from '../src/ConnectionManager';
+
+describe('ConnectionManager static helpers', () => {
+  it('resolveClientId sanitizes explicit clientId overrides', () => {
+    expect(ConnectionManager.resolveClientId({ clientId: '  laptop / dev  ' })).toBe('laptop-dev');
+  });
+
+  it('resolveClientId falls back to defaultClientId when clientId is blank or omitted', () => {
+    const fromBlank = ConnectionManager.resolveClientId({ clientId: '   ' });
+    const fromEmpty = ConnectionManager.resolveClientId({ clientId: '' });
+    const fromOmitted = ConnectionManager.resolveClientId({});
+    // All three should resolve to the same defaultClientId() value
+    expect(fromBlank).toBe(fromEmpty);
+    expect(fromEmpty).toBe(fromOmitted);
+    // sanitizeClientId is applied: only safe chars, no leading/trailing dash
+    expect(fromBlank).toMatch(/^[A-Za-z0-9._-]+$/);
+    expect(fromBlank).not.toMatch(/^-|-$/);
+  });
+
+  it('formatUserLabel uses trimmed userName + resolved clientId', () => {
+    expect(ConnectionManager.formatUserLabel({
+      userName: '  alice  ',
+      clientId: 'desk 01',
+    })).toBe('alice@desk-01');
+  });
+
+  it('formatUserLabel falls back when values are blank', () => {
+    const label = ConnectionManager.formatUserLabel({ userName: '   ', clientId: '   ' });
+    const [name, id] = label.split('@');
+    // Both sides of @ must be non-empty
+    expect(name).toBeTruthy();
+    expect(id).toBeTruthy();
+    // clientId side must be sanitized (no spaces, slashes, leading/trailing dashes)
+    expect(id).toMatch(/^[A-Za-z0-9._-]+$/);
+    expect(id).not.toMatch(/^-|-$/);
+  });
+});

plugin/tests/FsChangeListener.applyChange.test.ts

@@ -0,0 +1,263 @@
+import { App } from 'obsidian';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+const hoisted = vi.hoisted(() => {
+  const interpretWatchEvent = vi.fn();
+  const logger = {
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+  };
+  const perfTracer = {
+    point: vi.fn(),
+    newCid: vi.fn(() => 'cid-1'),
+    begin: vi.fn(() => ({ t0: 1 })),
+    end: vi.fn(),
+  };
+  const builderMethods = {
+    insertOne: vi.fn(),
+    removeOne: vi.fn(),
+    modifyOne: vi.fn(),
+    renameOne: vi.fn(),
+  };
+  const disposer = vi.fn();
+  const onNotification = vi.fn(() => disposer);
+  const call = vi.fn();
+  return {
+    interpretWatchEvent,
+    logger,
+    perfTracer,
+    builderMethods,
+    disposer,
+    onNotification,
+    call,
+  };
+});
+
+vi.mock('../src/path/WatchEventFilter', () => ({
+  interpretWatchEvent: hoisted.interpretWatchEvent,
+}));
+
+vi.mock('../src/util/logger', () => ({
+  logger: hoisted.logger,
+}));
+
+vi.mock('../src/util/PerfTracer', () => ({
+  perfTracer: hoisted.perfTracer,
+}));
+
+vi.mock('../src/vault/VaultModelBuilder', () => ({
+  VaultModelBuilder: class {
+    insertOne = hoisted.builderMethods.insertOne;
+    removeOne = hoisted.builderMethods.removeOne;
+    modifyOne = hoisted.builderMethods.modifyOne;
+    renameOne = hoisted.builderMethods.renameOne;
+  },
+}));
+
+import { FsChangeListener } from '../src/vault/FsChangeListener';
+
+function makeListener() {
+  const app = new App();
+  const stat = vi.fn();
+  (app.vault.adapter as unknown as { stat: typeof stat }).stat = stat;
+  return { listener: new FsChangeListener(app), stat };
+}
+
+function makeRpcConnection() {
+  return { rpc: { onNotification: hoisted.onNotification, call: hoisted.call } };
+}
+
+describe('FsChangeListener notification + applyChange', () => {
+  beforeEach(() => {
+    hoisted.interpretWatchEvent.mockReset();
+    hoisted.logger.info.mockReset();
+    hoisted.logger.warn.mockReset();
+    hoisted.logger.error.mockReset();
+    hoisted.perfTracer.point.mockReset();
+    hoisted.perfTracer.newCid.mockReset();
+    hoisted.perfTracer.newCid.mockReturnValue('cid-1');
+    hoisted.perfTracer.begin.mockReset();
+    hoisted.perfTracer.begin.mockReturnValue({ t0: 1 });
+    hoisted.perfTracer.end.mockReset();
+    hoisted.builderMethods.insertOne.mockReset();
+    hoisted.builderMethods.removeOne.mockReset();
+    hoisted.builderMethods.modifyOne.mockReset();
+    hoisted.builderMethods.renameOne.mockReset();
+    hoisted.disposer.mockReset();
+    hoisted.onNotification.mockReset();
+    hoisted.onNotification.mockReturnValue(hoisted.disposer);
+    hoisted.call.mockReset();
+    hoisted.call.mockResolvedValue({ subscriptionId: 'sub-1' });
+  });
+
+  it('processes fs.changed renamed notifications with old/new invalidation', async () => {
+    const { listener } = makeListener();
+    const dataAdapter = { invalidateRemotePath: vi.fn() };
+    const applyChange = vi.spyOn(listener as unknown as { applyChange: (...args: unknown[]) => Promise<void> }, 'applyChange').mockResolvedValue();
+    hoisted.interpretWatchEvent.mockImplementation((p: string) => {
+      if (p === 'old.md') return { remotePath: '/remote/old.md', vaultPath: 'old.md' };
+      if (p === 'new.md') return { remotePath: '/remote/new.md', vaultPath: 'new.md' };
+      return null;
+    });
+
+    await listener.subscribe({
+      rpcConnection: makeRpcConnection() as never,
+      dataAdapter: dataAdapter as never,
+      pathMapper: {} as never,
+    });
+    const handler = hoisted.onNotification.mock.calls[0]?.[1] as (params: {
+      event: 'renamed'; path: string; newPath: string; subscriptionId: string;
+    }) => void;
+
+    handler({ event: 'renamed', path: 'old.md', newPath: 'new.md', subscriptionId: 'sub-1' });
+
+    expect(hoisted.perfTracer.point).toHaveBeenCalledTimes(1);
+    expect(dataAdapter.invalidateRemotePath).toHaveBeenCalledWith('/remote/old.md');
+    expect(dataAdapter.invalidateRemotePath).toHaveBeenCalledWith('/remote/new.md');
+    expect(applyChange).toHaveBeenCalledWith('old.md', 'new.md', 'renamed');
+  });
+
+  it('ignores notifications from different subscription ids', async () => {
+    const { listener } = makeListener();
+    const dataAdapter = { invalidateRemotePath: vi.fn() };
+    const applyChange = vi.spyOn(listener as unknown as { applyChange: (...args: unknown[]) => Promise<void> }, 'applyChange').mockResolvedValue();
+    hoisted.interpretWatchEvent.mockReturnValue({ remotePath: '/remote/a.md', vaultPath: 'a.md' });
+
+    await listener.subscribe({
+      rpcConnection: makeRpcConnection() as never,
+      dataAdapter: dataAdapter as never,
+      pathMapper: {} as never,
+    });
+    const handler = hoisted.onNotification.mock.calls[0]?.[1] as (params: {
+      event: 'modified'; path: string; subscriptionId: string;
+    }) => void;
+
+    handler({ event: 'modified', path: 'a.md', subscriptionId: 'other-sub' });
+
+    expect(hoisted.interpretWatchEvent).not.toHaveBeenCalled();
+    expect(dataAdapter.invalidateRemotePath).not.toHaveBeenCalled();
+    expect(applyChange).not.toHaveBeenCalled();
+  });
+
+  function invokeApplyChange(
+    listener: FsChangeListener,
+    oldPath: string,
+    newPath: string | undefined,
+    event: 'created' | 'modified' | 'deleted' | 'renamed',
+  ): Promise<void> {
+    return (listener as unknown as {
+      applyChange: (o: string, n: string | undefined, e: typeof event) => Promise<void>;
+    }).applyChange(oldPath, newPath, event);
+  }
+
+  it('applyChange created: inserts with stat metadata when stat succeeds', async () => {
+    const { listener, stat } = makeListener();
+    stat.mockResolvedValueOnce({ type: 'folder', ctime: 1, mtime: 2, size: 3 });
+
+    await invokeApplyChange(listener, 'dir', undefined, 'created');
+
+    expect(hoisted.builderMethods.insertOne).toHaveBeenCalledWith({
+      path: 'dir',
+      isDirectory: true,
+      ctime: 1,
+      mtime: 2,
+      size: 3,
+    }, { ensureParents: true });
+  });
+
+  it('applyChange created: warns and returns when stat returns null', async () => {
+    const { listener, stat } = makeListener();
+    stat.mockResolvedValueOnce(null);
+
+    await invokeApplyChange(listener, 'missing.md', undefined, 'created');
+
+    expect(hoisted.builderMethods.insertOne).not.toHaveBeenCalled();
+    expect(hoisted.logger.warn).toHaveBeenCalledWith('applyChange(created): stat failed for missing.md');
+  });
+
+  it('applyChange modified: calls modifyOne with stat metadata when stat succeeds', async () => {
+    const { listener, stat } = makeListener();
+    stat.mockResolvedValueOnce({ type: 'file', ctime: 10, mtime: 20, size: 100 });
+
+    await invokeApplyChange(listener, 'note.md', undefined, 'modified');
+
+    expect(hoisted.builderMethods.modifyOne).toHaveBeenCalledWith('note.md', {
+      ctime: 10,
+      mtime: 20,
+      size: 100,
+    });
+  });
+
+  it('applyChange modified: calls modifyOne without metadata when stat returns null', async () => {
+    const { listener, stat } = makeListener();
+    stat.mockResolvedValueOnce(null);
+
+    await invokeApplyChange(listener, 'note.md', undefined, 'modified');
+
+    expect(hoisted.builderMethods.modifyOne).toHaveBeenCalledWith('note.md');
+  });
+
+  it('applyChange deleted: removes the path', async () => {
+    const { listener } = makeListener();
+
+    await invokeApplyChange(listener, 'old.md', undefined, 'deleted');
+
+    expect(hoisted.builderMethods.removeOne).toHaveBeenCalledWith('old.md');
+  });
+
+  it('applyChange renamed: warns when newPath is missing', async () => {
+    const { listener } = makeListener();
+
+    await invokeApplyChange(listener, 'a.md', undefined, 'renamed');
+
+    expect(hoisted.builderMethods.renameOne).not.toHaveBeenCalled();
+    expect(hoisted.logger.warn).toHaveBeenCalledWith('applyChange(renamed): missing newPath for a.md');
+  });
+
+  it('applyChange renamed: renames the path when newPath is provided and ends the perf span', async () => {
+    const { listener } = makeListener();
+
+    await invokeApplyChange(listener, 'a.md', 'b.md', 'renamed');
+
+    expect(hoisted.builderMethods.renameOne).toHaveBeenCalledWith('a.md', 'b.md');
+    expect(hoisted.perfTracer.end).toHaveBeenCalled();
+  });
+
+  it('ignores notifications when interpretWatchEvent returns null (path filtered out)', async () => {
+    const { listener } = makeListener();
+    const dataAdapter = { invalidateRemotePath: vi.fn() };
+    const applyChange = vi.spyOn(listener as unknown as { applyChange: (...args: unknown[]) => Promise<void> }, 'applyChange').mockResolvedValue();
+    hoisted.interpretWatchEvent.mockReturnValue(null);
+
+    await listener.subscribe({
+      rpcConnection: makeRpcConnection() as never,
+      dataAdapter: dataAdapter as never,
+      pathMapper: {} as never,
+    });
+    const handler = hoisted.onNotification.mock.calls[0]?.[1] as (params: {
+      event: 'modified'; path: string; subscriptionId: string;
+    }) => void;
+
+    handler({ event: 'modified', path: 'filtered.md', subscriptionId: 'sub-1' });
+
+    // T4a stamps every push frame even before path filtering
+    expect(hoisted.perfTracer.point).toHaveBeenCalledTimes(1);
+    expect(dataAdapter.invalidateRemotePath).not.toHaveBeenCalled();
+    expect(applyChange).not.toHaveBeenCalled();
+  });
+
+  it('applyChange catches builder failures and logs warnings', async () => {
+    const { listener } = makeListener();
+    hoisted.builderMethods.removeOne.mockImplementation(() => {
+      throw new Error('boom');
+    });
+
+    await (listener as unknown as { applyChange: (...args: unknown[]) => Promise<void> })
+      .applyChange('bad.md', undefined, 'deleted');
+
+    expect(hoisted.logger.warn).toHaveBeenCalledWith(
+      expect.stringContaining('applyChange(deleted) failed for bad.md: boom'),
+    );
+  });
+});

plugin/tests/FsChangeListener.test.ts

@@ -0,0 +1,123 @@
+import { App } from 'obsidian';
+import { describe, expect, it, vi, beforeEach } from 'vitest';
+import { FsChangeListener } from '../src/vault/FsChangeListener';
+
+function makeRpcConnection() {
+  const off = vi.fn();
+  const onNotification = vi.fn(() => off);
+  const call = vi.fn();
+  return {
+    rpcConnection: { rpc: { onNotification, call } },
+    onNotification,
+    call,
+    off,
+  };
+}
+
+describe('FsChangeListener lifecycle', () => {
+  beforeEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it('subscribe registers handler and fs.watch once, then is idempotent', async () => {
+    const listener = new FsChangeListener(new App());
+    const { rpcConnection, onNotification, call } = makeRpcConnection();
+    call.mockResolvedValue({ subscriptionId: 'sub-1' });
+    const dataAdapter = { invalidateRemotePath: vi.fn() };
+    const pathMapper = { toRemotePath: vi.fn(), toVaultPath: vi.fn() };
+
+    await listener.subscribe({
+      rpcConnection: rpcConnection as never,
+      dataAdapter: dataAdapter as never,
+      pathMapper: pathMapper as never,
+    });
+    await listener.subscribe({
+      rpcConnection: rpcConnection as never,
+      dataAdapter: dataAdapter as never,
+      pathMapper: pathMapper as never,
+    });
+
+    expect(listener.hasContext()).toBe(true);
+    expect(onNotification).toHaveBeenCalledTimes(1);
+    expect(call).toHaveBeenCalledWith('fs.watch', { path: '', recursive: true });
+  });
+
+  it('prepareForReconnect disposes live handler but keeps context for resume', async () => {
+    const listener = new FsChangeListener(new App());
+    const first = makeRpcConnection();
+    first.call.mockResolvedValue({ subscriptionId: 'sub-1' });
+    const dataAdapter = { invalidateRemotePath: vi.fn() };
+    const pathMapper = { toRemotePath: vi.fn(), toVaultPath: vi.fn() };
+
+    await listener.subscribe({
+      rpcConnection: first.rpcConnection as never,
+      dataAdapter: dataAdapter as never,
+      pathMapper: pathMapper as never,
+    });
+    listener.prepareForReconnect();
+    expect(first.off).toHaveBeenCalledTimes(1);
+    expect(listener.hasContext()).toBe(true);
+
+    const second = makeRpcConnection();
+    second.call.mockResolvedValue({ subscriptionId: 'sub-2' });
+    await listener.resumeAfterReconnect({
+      rpcConnection: second.rpcConnection as never,
+      dataAdapter: dataAdapter as never,
+    });
+
+    expect(second.call).toHaveBeenCalledWith('fs.watch', { path: '', recursive: true });
+  });
+
+  it('unsubscribe sends fs.unwatch and clears local context', async () => {
+    const listener = new FsChangeListener(new App());
+    const { rpcConnection, call, off } = makeRpcConnection();
+    call.mockResolvedValue({ subscriptionId: 'sub-1' });
+    const dataAdapter = { invalidateRemotePath: vi.fn() };
+    const pathMapper = { toRemotePath: vi.fn(), toVaultPath: vi.fn() };
+
+    await listener.subscribe({
+      rpcConnection: rpcConnection as never,
+      dataAdapter: dataAdapter as never,
+      pathMapper: pathMapper as never,
+    });
+    call.mockClear();
+
+    listener.unsubscribe(rpcConnection as never);
+
+    expect(call).toHaveBeenCalledWith('fs.unwatch', { subscriptionId: 'sub-1' });
+    expect(off).toHaveBeenCalledTimes(1);
+    expect(listener.hasContext()).toBe(false);
+  });
+
+  it('subscribe disposes the notification handler when fs.watch call rejects', async () => {
+    const listener = new FsChangeListener(new App());
+    const { rpcConnection, onNotification, call, off } = makeRpcConnection();
+    // First call rejects, second succeeds — verifies subscribe is not blocked after failure
+    call.mockRejectedValueOnce(new Error('connection refused'));
+    call.mockResolvedValue({ subscriptionId: 'sub-1' });
+    const dataAdapter = { invalidateRemotePath: vi.fn() };
+    const pathMapper = { toRemotePath: vi.fn(), toVaultPath: vi.fn() };
+
+    vi.spyOn(console, 'error').mockImplementation(() => {}); // suppress logger output
+
+    await listener.subscribe({
+      rpcConnection: rpcConnection as never,
+      dataAdapter: dataAdapter as never,
+      pathMapper: pathMapper as never,
+    });
+
+    // Handler was registered then disposed to prevent leaks
+    expect(onNotification).toHaveBeenCalledTimes(1);
+    expect(off).toHaveBeenCalledTimes(1);
+    // pathMapper captured before rpc.call, so hasContext is true (resume remains possible)
+    expect(listener.hasContext()).toBe(true);
+
+    // subscriptionId was never set → a retry subscribe() goes through (not blocked)
+    await listener.subscribe({
+      rpcConnection: rpcConnection as never,
+      dataAdapter: dataAdapter as never,
+      pathMapper: pathMapper as never,
+    });
+    expect(onNotification).toHaveBeenCalledTimes(2);
+  });
+});

plugin/tests/HostKeyConfirmModal.test.ts

@@ -0,0 +1,73 @@
+import { App, findButton } from 'obsidian';
+import { describe, expect, it } from 'vitest';
+import { HostKeyConfirmModal } from '../src/ui/HostKeyConfirmModal';
+
+function makeModal(): HostKeyConfirmModal {
+  return new HostKeyConfirmModal(new App(), {
+    host: 'example.com',
+    port: 22,
+    fingerprint: 'a'.repeat(64),
+    keyType: 'ssh-ed25519',
+  });
+}
+
+describe('HostKeyConfirmModal', () => {
+  it('renders host details and key type on open', () => {
+    const modal = makeModal();
+    modal.onOpen();
+    expect(modal.titleEl.textContent).toBe('Trust remote host key?');
+    expect(modal.contentEl.textContent).toContain('example.com:22');
+    expect(modal.contentEl.textContent).toContain('ssh-ed25519');
+  });
+
+  it('does not render key type row when keyType is omitted', () => {
+    const modal = new HostKeyConfirmModal(new App(), {
+      host: 'example.com',
+      port: 22,
+      fingerprint: 'b'.repeat(64),
+    });
+    modal.onOpen();
+    expect(modal.contentEl.textContent).not.toContain('Key type:');
+  });
+
+  it('prompt resolves trust-once when that button is clicked', async () => {
+    const modal = makeModal();
+    const decisionPromise = modal.prompt();
+    await Promise.resolve();
+    findButton(modal.contentEl, 'Trust this session only')?.click();
+    await expect(decisionPromise).resolves.toBe('trust-once');
+  });
+
+  it('prompt resolves trust when Trust & remember button is clicked', async () => {
+    const modal = makeModal();
+    const decisionPromise = modal.prompt();
+    await Promise.resolve();
+    findButton(modal.contentEl, 'Trust & remember')?.click();
+    await expect(decisionPromise).resolves.toBe('trust');
+  });
+
+  it('prompt resolves reject when Reject button is clicked', async () => {
+    const modal = makeModal();
+    const decisionPromise = modal.prompt();
+    await Promise.resolve();
+    findButton(modal.contentEl, 'Reject')?.click();
+    await expect(decisionPromise).resolves.toBe('reject');
+  });
+
+  it('resolves reject when closed without explicit choice', async () => {
+    const modal = makeModal();
+    const decisionPromise = modal.prompt();
+    await Promise.resolve();
+    modal.close();
+    await expect(decisionPromise).resolves.toBe('reject');
+  });
+
+  it('resolved flag prevents onClose from overwriting an explicit choice', async () => {
+    const modal = makeModal();
+    const decisionPromise = modal.prompt();
+    await Promise.resolve();
+    findButton(modal.contentEl, 'Trust this session only')?.click(); // sets resolved = true, closes modal
+    modal.close(); // onClose fires again but should be no-op
+    await expect(decisionPromise).resolves.toBe('trust-once');
+  });
+});

plugin/tests/ObservabilityInstaller.test.ts

@@ -0,0 +1,101 @@
+import * as path from 'path';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+const {
+  loggerMock,
+  installErrorHookMock,
+  uninstallErrorHookMock,
+} = vi.hoisted(() => ({
+  loggerMock: {
+    installFileSink: vi.fn(),
+    uninstallFileSink: vi.fn().mockResolvedValue(undefined),
+    wrapConsole: vi.fn(),
+    unwrapConsole: vi.fn(),
+    warn: vi.fn(),
+    info: vi.fn(),
+  },
+  installErrorHookMock: vi.fn(),
+  uninstallErrorHookMock: vi.fn(),
+}));
+
+vi.mock('../src/util/logger', () => ({ logger: loggerMock }));
+vi.mock('../src/util/errorHook', () => ({
+  installErrorHook: installErrorHookMock,
+  uninstallErrorHook: uninstallErrorHookMock,
+}));
+
+import { ObservabilityInstaller } from '../src/util/ObservabilityInstaller';
+
+describe('ObservabilityInstaller', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    loggerMock.uninstallFileSink.mockResolvedValue(undefined);
+  });
+
+  it('installs file sink + hooks and logs plugin load when vault path exists', () => {
+    const installer = new ObservabilityInstaller(
+      { id: 'remote-ssh', version: '1.2.3' },
+      '/vault',
+      '.obsidian',
+    );
+
+    installer.install();
+
+    // Use path.join so the assertion matches the OS-native separator
+    // (Windows: backslash; POSIX: forward slash) — the source uses path.join too.
+    expect(loggerMock.installFileSink).toHaveBeenCalledWith(
+      path.join('/vault', '.obsidian', 'plugins', 'remote-ssh', 'console.log'),
+    );
+    expect(loggerMock.wrapConsole).toHaveBeenCalledTimes(1);
+    expect(installErrorHookMock).toHaveBeenCalledTimes(1);
+    expect(loggerMock.info).toHaveBeenCalledWith('Plugin remote-ssh v1.2.3 loaded');
+  });
+
+  it('warns and skips file sink when vault path is null', () => {
+    const installer = new ObservabilityInstaller(
+      { id: 'remote-ssh', version: '1.2.3' },
+      null,
+      '.obsidian',
+    );
+
+    installer.install();
+
+    expect(loggerMock.installFileSink).not.toHaveBeenCalled();
+    expect(loggerMock.warn).toHaveBeenCalledWith('vault.adapter is not FileSystemAdapter; file sink disabled');
+    expect(loggerMock.wrapConsole).toHaveBeenCalledTimes(1);
+    expect(installErrorHookMock).toHaveBeenCalledTimes(1);
+  });
+
+  it('logs installFileSink failures and still installs other observability hooks', () => {
+    loggerMock.installFileSink.mockImplementationOnce(() => {
+      throw new Error('disk full');
+    });
+    const installer = new ObservabilityInstaller(
+      { id: 'remote-ssh', version: '1.2.3' },
+      '/vault',
+      '.obsidian',
+    );
+
+    installer.install();
+
+    expect(loggerMock.warn).toHaveBeenCalledWith('installFileSink failed: disk full');
+    expect(loggerMock.wrapConsole).toHaveBeenCalledTimes(1);
+    expect(installErrorHookMock).toHaveBeenCalledTimes(1);
+    expect(loggerMock.info).toHaveBeenCalledWith('Plugin remote-ssh v1.2.3 loaded');
+  });
+
+  it('uninstalls hooks, console wrapper, and file sink in teardown path', () => {
+    const installer = new ObservabilityInstaller(
+      { id: 'remote-ssh', version: '1.2.3' },
+      '/vault',
+      '.obsidian',
+    );
+
+    installer.uninstall();
+
+    expect(loggerMock.info).toHaveBeenCalledWith('Plugin remote-ssh unloading');
+    expect(uninstallErrorHookMock).toHaveBeenCalledTimes(1);
+    expect(loggerMock.unwrapConsole).toHaveBeenCalledTimes(1);
+    expect(loggerMock.uninstallFileSink).toHaveBeenCalledTimes(1);
+  });
+});

plugin/tests/__mocks__/obsidian.ts

@@ -133,6 +133,9 @@ function patchDom(): void {
       this.appendChild(t);
     }
   };
+  proto.appendText = function (this: HTMLElement, t: string): void {
+    this.appendChild(document.createTextNode(t));
+  };
   // Real Obsidian: toggleClass(classes: string | string[], value: boolean).
   proto.toggleClass = function (this: HTMLElement, c: string | string[], on: boolean): void {
     const list = Array.isArray(c) ? c : [c];
@@ -357,10 +360,13 @@ export class Setting {
 export class Modal {
   contentEl: HTMLElement;
   modalEl: HTMLElement;
+  titleEl: HTMLElement;
 
   constructor(public readonly app: App) {
     this.modalEl = document.createElement('div');
+    this.titleEl = document.createElement('h2');
     this.contentEl = document.createElement('div');
+    this.modalEl.appendChild(this.titleEl);
     this.modalEl.appendChild(this.contentEl);
   }
 
@@ -457,6 +463,7 @@ declare global {
     addClass(...classes: string[]): void;
     removeClass(...classes: string[]): void;
     setText(t: string | DocumentFragment): void;
+    appendText(t: string): void;
     toggleClass(c: string | string[], on: boolean): void;
   }
 }

plugin/tests/errorHook.test.ts

@@ -0,0 +1,117 @@
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { installErrorHook, uninstallErrorHook } from '../src/util/errorHook';
+import { logger } from '../src/util/logger';
+
+describe('errorHook', () => {
+  afterEach(() => {
+    uninstallErrorHook();
+    vi.restoreAllMocks();
+  });
+
+  it('installs only once and unregisters both listeners on uninstall', () => {
+    const add = vi.spyOn(window, 'addEventListener');
+    const remove = vi.spyOn(window, 'removeEventListener');
+
+    installErrorHook();
+    installErrorHook();
+
+    expect(add).toHaveBeenCalledTimes(2);
+    uninstallErrorHook();
+    expect(remove).toHaveBeenCalledTimes(2);
+    expect(remove.mock.calls.map(([name]) => name)).toEqual(['unhandledrejection', 'error']);
+  });
+
+  it('logs unhandled rejection details for Error reasons', () => {
+    let onUnhandled: ((e: PromiseRejectionEvent) => void) | undefined;
+    vi.spyOn(window, 'addEventListener').mockImplementation(
+      ((name: string, cb: EventListenerOrEventListenerObject) => {
+        if (name === 'unhandledrejection') onUnhandled = cb as (e: PromiseRejectionEvent) => void;
+      }) as typeof window.addEventListener,
+    );
+    const error = vi.spyOn(logger, 'error').mockImplementation(() => {});
+
+    installErrorHook();
+    onUnhandled?.({ reason: new Error('boom') } as PromiseRejectionEvent);
+
+    expect(error).toHaveBeenCalledTimes(1);
+    expect(error.mock.calls[0]?.[0]).toContain('unhandledrejection: boom');
+  });
+
+  it('serializes plain object reason via JSON.stringify', () => {
+    let onUnhandled: ((e: PromiseRejectionEvent) => void) | undefined;
+    vi.spyOn(window, 'addEventListener').mockImplementation(
+      ((name: string, cb: EventListenerOrEventListenerObject) => {
+        if (name === 'unhandledrejection') onUnhandled = cb as (e: PromiseRejectionEvent) => void;
+      }) as typeof window.addEventListener,
+    );
+    const error = vi.spyOn(logger, 'error').mockImplementation(() => {});
+
+    installErrorHook();
+    onUnhandled?.({ reason: { code: 404 } } as PromiseRejectionEvent);
+
+    expect(error.mock.calls[0]?.[0]).toBe('unhandledrejection: {"code":404}');
+  });
+
+  it('falls back to String(reason) when JSON.stringify throws', () => {
+    let onUnhandled: ((e: PromiseRejectionEvent) => void) | undefined;
+    vi.spyOn(window, 'addEventListener').mockImplementation(
+      ((name: string, cb: EventListenerOrEventListenerObject) => {
+        if (name === 'unhandledrejection') onUnhandled = cb as (e: PromiseRejectionEvent) => void;
+      }) as typeof window.addEventListener,
+    );
+    const error = vi.spyOn(logger, 'error').mockImplementation(() => {});
+    const circular: Record<string, unknown> = {};
+    circular.self = circular;
+
+    installErrorHook();
+    onUnhandled?.({ reason: circular } as PromiseRejectionEvent);
+
+    expect(error.mock.calls[0]?.[0]).toBe('unhandledrejection: [object Object]');
+  });
+
+  it('logs window.onerror with location and stack when available', () => {
+    let onError: ((e: ErrorEvent) => void) | undefined;
+    vi.spyOn(window, 'addEventListener').mockImplementation(
+      ((name: string, cb: EventListenerOrEventListenerObject) => {
+        if (name === 'error') onError = cb as (e: ErrorEvent) => void;
+      }) as typeof window.addEventListener,
+    );
+    const error = vi.spyOn(logger, 'error').mockImplementation(() => {});
+
+    installErrorHook();
+    onError?.({
+      message: 'kaput',
+      filename: 'main.ts',
+      lineno: 12,
+      colno: 4,
+      error: new Error('kaput'),
+    } as ErrorEvent);
+
+    expect(error).toHaveBeenCalledTimes(1);
+    expect(error.mock.calls[0]?.[0]).toContain('window.onerror: kaput at main.ts:12:4');
+    expect(error.mock.calls[0]?.[0]).toContain('Error: kaput');
+  });
+
+  it('logs window.onerror without stack when error is not an Error instance', () => {
+    let onError: ((e: ErrorEvent) => void) | undefined;
+    vi.spyOn(window, 'addEventListener').mockImplementation(
+      ((name: string, cb: EventListenerOrEventListenerObject) => {
+        if (name === 'error') onError = cb as (e: ErrorEvent) => void;
+      }) as typeof window.addEventListener,
+    );
+    const error = vi.spyOn(logger, 'error').mockImplementation(() => {});
+
+    installErrorHook();
+    onError?.({
+      message: 'script error',
+      filename: 'vendor.js',
+      lineno: 1,
+      colno: 0,
+      error: null,
+    } as ErrorEvent);
+
+    expect(error).toHaveBeenCalledTimes(1);
+    expect(error.mock.calls[0]?.[0]).toBe('window.onerror: script error at vendor.js:1:0');
+    expect(error.mock.calls[0]?.[0]).not.toContain('\n');
+  });
+});

plugin/tests/pathUtils.test.ts

@@ -1,5 +1,14 @@
+import * as path from 'path';
 import { describe, it, expect } from 'vitest';
-import { normalizeRemotePath } from '../src/util/pathUtils';
+import {
+  ensureTrailingSlash,
+  expandHome,
+  normalizeRemotePath,
+  posixJoin,
+  relativeTo,
+  toLocalPath,
+  toRemotePath,
+} from '../src/util/pathUtils';
 
 describe('normalizeRemotePath', () => {
   it('strips a leading "~/" so the path becomes home-relative for SFTP', () => {
@@ -29,3 +38,66 @@ describe('normalizeRemotePath', () => {
     expect(normalizeRemotePath('  ~/work/VaultDev  ')).toBe('work/VaultDev');
   });
 });
+
+describe('path utility helpers', () => {
+  it('posixJoin joins with a single slash', () => {
+    expect(posixJoin('a', 'b', 'c')).toBe('a/b/c');
+    expect(posixJoin('/a/', '/b//', 'c')).toBe('/a/b/c');
+  });
+
+  it('relativeTo returns full path when base does not match', () => {
+    expect(relativeTo('/base', '/other/file.md')).toBe('/other/file.md');
+  });
+
+  it('relativeTo strips base with or without trailing slash', () => {
+    expect(relativeTo('/base', '/base/file.md')).toBe('file.md');
+    expect(relativeTo('/base/', '/base/file.md')).toBe('file.md');
+  });
+
+  it('ensureTrailingSlash appends only once', () => {
+    expect(ensureTrailingSlash('/tmp/work')).toBe('/tmp/work/');
+    expect(ensureTrailingSlash('/tmp/work/')).toBe('/tmp/work/');
+  });
+
+  it('toLocalPath uses platform-aware path join', () => {
+    const result = toLocalPath('/tmp/work', 'docs/a.md');
+    expect(result).toContain('docs');
+    expect(result.endsWith('docs/a.md') || result.endsWith('docs\\a.md')).toBe(true);
+  });
+
+  it('toRemotePath uses posix separator', () => {
+    expect(toRemotePath('/srv/vault', 'docs/a.md')).toBe('/srv/vault/docs/a.md');
+  });
+
+  it('expandHome expands "~/" using HOME', () => {
+    const originalHome = process.env.HOME;
+    const originalUserProfile = process.env.USERPROFILE;
+    try {
+      process.env.HOME = '/home/alice';
+      delete process.env.USERPROFILE;
+      // Use path.join for platform-agnostic comparison (Windows uses backslashes)
+      expect(expandHome('~/vault')).toBe(path.join('/home/alice', 'vault'));
+    } finally {
+      if (originalHome === undefined) delete process.env.HOME; else process.env.HOME = originalHome;
+      if (originalUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = originalUserProfile;
+    }
+  });
+
+  it('expandHome falls back to cwd-relative when neither HOME nor USERPROFILE is set', () => {
+    const originalHome = process.env.HOME;
+    const originalUserProfile = process.env.USERPROFILE;
+    try {
+      delete process.env.HOME;
+      delete process.env.USERPROFILE;
+      // home = '' → path.join('', 'vault') = 'vault'
+      expect(expandHome('~/vault')).toBe(path.join('', 'vault'));
+    } finally {
+      if (originalHome === undefined) delete process.env.HOME; else process.env.HOME = originalHome;
+      if (originalUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = originalUserProfile;
+    }
+  });
+
+  it('expandHome leaves non-tilde paths unchanged', () => {
+    expect(expandHome('/srv/vault')).toBe('/srv/vault');
+  });
+});

plugin/tests/retry.test.ts

@@ -0,0 +1,99 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { RETRY_MAX_MS } from '../src/constants';
+import { withRetry } from '../src/util/retry';
+import { logger } from '../src/util/logger';
+
+describe('withRetry', () => {
+  const aw = (globalThis as typeof globalThis & { activeWindow: typeof globalThis }).activeWindow;
+  let originalSetTimeout: typeof aw.setTimeout;
+  const delays: number[] = [];
+
+  beforeEach(() => {
+    // Capture per-test so a fake-timer left behind by another suite cannot
+    // poison our restore path.
+    originalSetTimeout = aw.setTimeout.bind(aw);
+  });
+
+  afterEach(() => {
+    delays.length = 0;
+    aw.setTimeout = originalSetTimeout;
+    vi.restoreAllMocks();
+  });
+
+  function installImmediateSleep(): void {
+    aw.setTimeout = ((cb: TimerHandler, ms?: number) => {
+      delays.push(Number(ms ?? 0));
+      if (typeof cb === 'function') cb();
+      return 0 as unknown as number;
+    }) as typeof setTimeout;
+  }
+
+  it('returns immediately on first success without logging retries', async () => {
+    installImmediateSleep();
+    const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {});
+    const fn = vi.fn(async () => 'ok');
+
+    await expect(withRetry(fn, 'rpc')).resolves.toBe('ok');
+
+    expect(fn).toHaveBeenCalledTimes(1);
+    expect(warn).not.toHaveBeenCalled();
+    expect(delays).toEqual([]);
+  });
+
+  it('retries after a failure and logs backoff details', async () => {
+    installImmediateSleep();
+    vi.spyOn(Math, 'random').mockReturnValue(0);
+    const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {});
+    const fn = vi.fn()
+      .mockRejectedValueOnce(new Error('flaky'))
+      .mockResolvedValueOnce('done');
+
+    await expect(withRetry(fn, 'fs.write', 3)).resolves.toBe('done');
+
+    expect(fn).toHaveBeenCalledTimes(2);
+    expect(delays).toEqual([1000]);
+    expect(warn).toHaveBeenCalledTimes(1);
+    expect(warn.mock.calls[0]?.[0]).toContain('fs.write: attempt 1 failed (flaky), retry in 1000ms');
+  });
+
+  it('caps retry delay at RETRY_MAX_MS and rethrows the final error', async () => {
+    installImmediateSleep();
+    vi.spyOn(Math, 'random').mockReturnValue(0);
+    const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {});
+    const boom = new Error('still failing');
+    const fn = vi.fn(async () => { throw boom; });
+
+    await expect(withRetry(fn, 'deploy', 8)).rejects.toBe(boom);
+
+    expect(fn).toHaveBeenCalledTimes(8);
+    expect(warn).toHaveBeenCalledTimes(7);
+    expect(delays.slice(0, 5)).toEqual([1000, 2000, 4000, 8000, 16000]);
+    expect(delays.slice(5)).toEqual([RETRY_MAX_MS, RETRY_MAX_MS]);
+  });
+
+  it('throws immediately when maxAttempts is 1 with no retry', async () => {
+    installImmediateSleep();
+    const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {});
+    const boom = new Error('instant');
+    const fn = vi.fn(async () => { throw boom; });
+
+    await expect(withRetry(fn, 'label', 1)).rejects.toBe(boom);
+
+    expect(fn).toHaveBeenCalledTimes(1);
+    expect(warn).not.toHaveBeenCalled();
+    expect(delays).toEqual([]);
+  });
+
+  it('uses errorMessage for non-Error thrown values', async () => {
+    installImmediateSleep();
+    vi.spyOn(Math, 'random').mockReturnValue(0);
+    const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {});
+    const fn = vi.fn()
+      .mockRejectedValueOnce('plain string error')
+      .mockResolvedValueOnce('ok');
+
+    await expect(withRetry(fn, 'label', 3)).resolves.toBe('ok');
+
+    expect(warn.mock.calls[0]?.[0]).toContain('label: attempt 1 failed (plain string error)');
+  });
+});