[Audit] LOW: Empty catch blocks silence tool disconnection errors

[NickCalabs]

Severity: LOW

Description

Multiple locations in src/tools/registry.ts and src/daemon.ts use .catch(() => {}) to suppress errors during cleanup operations, making debugging difficult.

Locations

1. src/tools/registry.ts:38

await existing.client?.disconnect().catch(() => {});

Context: Replacing an existing MCP server registration

2. src/tools/registry.ts:134

await entry.client?.disconnect().catch(() => {});

Context: Manual server disconnection

3. src/daemon.ts:199-200

fetch(`${base}/tools`).catch(() => null),
fetch(`${base}/agents`).catch(() => null),

Context: Status check, failure is expected if server not running (this one is OK)

Issues

1. Silent Failures: Real errors (not just "process already gone") are hidden
2. Resource Leaks: If disconnect fails for an unexpected reason, resources aren't cleaned up
3. Hard to Debug: Users can't tell why cleanup failed
4. No Metrics: Can't track failure rates

Impact

• Occasional resource leaks
• Zombie processes not cleaned up
• Difficult to diagnose production issues
• No visibility into cleanup health

Recommendation

Add minimal error logging:

// src/tools/registry.ts:38
await existing.client?.disconnect().catch((err) => {
  const msg = err instanceof Error ? err.message : String(err);
  console.warn(`Warning: failed to disconnect existing server "${name}": ${msg}`);
});

// src/tools/registry.ts:134
await entry.client?.disconnect().catch((err) => {
  const msg = err instanceof Error ? err.message : String(err);
  console.warn(`Warning: failed to disconnect server "${name}": ${msg}`);
});

For disconnectAll() in line 141, track failures:

export async function disconnectAll(): Promise<void> {
  const entries = [...servers.values()];
  servers.clear();
  const results = await Promise.allSettled(entries.map((e) => e.client?.disconnect()));
  
  const failures = results.filter(r => r.status === 'rejected');
  if (failures.length > 0) {
    console.warn(`Warning: ${failures.length} MCP server(s) failed to disconnect gracefully`);
    for (const failure of failures) {
      if (failure.status === 'rejected') {
        console.warn(`  - ${failure.reason}`);
      }
    }
  }
}

Exception

The daemon.ts:199-200 usage is appropriate since fetch failure is expected when checking status of a stopped server.

---

Created by security audit