Greptile Summary

Added automatic reconnection logic for Slack socket mode with exponential backoff and bounded retry attempts. The implementation watches for disconnect, error, and unable_to_socket_mode_start events and automatically attempts to reconnect up to 12 times with delays ranging from 2-30 seconds.

• Implemented waitForSlackSocketDisconnect helper that listens for three socket events
• Added SLACK_SOCKET_RECONNECT_POLICY configuration with exponential backoff (factor 1.8) and jitter (25%)
• Integrated existing computeBackoff and sleepWithAbort utilities from src/infra/backoff.ts
• Added focused unit tests for the disconnect watcher
• Proper abort signal handling throughout the reconnect loop

Confidence Score: 3/5

• Safe to merge with one memory leak fix recommended
• The reconnect logic is well-designed with proper backoff and bounded retries, but there's a memory leak where event listeners accumulate when app.start() fails. The leak is bounded (max ~44 listeners per invocation) and only occurs in failure scenarios, so it won't affect normal operation. The fix is straightforward—move the disconnect waiter creation after successful connection.
• Focus on src/slack/monitor/provider.ts line 450 where the disconnect waiter should be created after connection success rather than before

_{Last reviewed commit: 366de2d}

Thanks for the review. I agree on the staleness-watchdog gap. I kept this PR scoped to reconnect-on-disconnect/error plus listener-leak fix for safe mergeability. I will implement lastInboundAt watchdog detection in a focused follow-up PR so behavior and rollback remain isolated.

Quick status ping on this PR.

Current intent remains the same: keep this PR focused on reconnect-on-disconnect/error and listener-leak handling for a safe, narrow merge path.

I still agree the lastInboundAt staleness watchdog is valuable and should stay as a focused follow-up so behavior and rollback remain isolated.

If maintainers prefer combining both into this PR now, I can update it accordingly.

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

1. 🟡 Potential sensitive data exposure & log forging via JSON.stringify of Slack socket errors

Description

In src/slack/monitor/provider.ts, the newly added reconnect logging formats arbitrary caught/emitted errors with formatUnknownError(). When the value is not an Error (or is an Error whose .message contains sensitive content), this can leak secrets and allow log injection:

• Secret leakage risk: formatUnknownError() will JSON.stringify(error) for non-Error objects. Errors coming from third-party libraries (Slack Bolt / Socket Mode client / Web API) can include structured fields such as request configuration, URLs, headers, or other metadata. If any of those include credentials (e.g., Slack bot/app tokens, authorization headers, webhook URLs), they will be serialized into logs.
• Log injection / multi-line log forging: Both error.message and stringified objects may contain \n/\r (or other control characters). The log statements interpolate the formatted error into a single template string, so embedded newlines can forge additional log lines or corrupt log parsing.
• Sink: runtime.error ultimately writes to console.error(...) (see src/runtime.ts), so this ends up in process/container logs where access may be broader than intended.

Vulnerable code:

function formatUnknownError(error: unknown): string {
  if (error instanceof Error) {
    return error.message;
  }
  if (typeof error === "string") {
    return error;
  }
  try {
    return JSON.stringify(error);
  } catch {
    return "unknown error";
  }
}

runtime.error?.(
  `slack socket mode failed to start ... (${formatUnknownError(err)})`,
);
...
runtime.error?.(
  `slack socket disconnected ...${disconnect.error ? ` (${formatUnknownError(disconnect.error)})` : ""}`,
);

Recommendation

Avoid logging raw/structured error objects from external libraries; log stable, redacted, single-line summaries.

Suggested approach:

• Prefer err instanceof Error and log name + message, but sanitize to single-line.
• For unknown objects, log only a minimal whitelist (e.g., code, status, type) rather than JSON.stringify.
• Apply token/URL redaction and truncation.

Example safer formatter:

function toSingleLine(s: string): string {
  return s.replace(/[\r\n\t]+/g, " ").slice(0, 500);
}

function redactSecrets(s: string): string {
  ​// Slack tokens commonly start with xoxb-, xapp-, xoxp-, etc.
  return s.replace(/xox[baprs]-[A-Za-z0-9-]+/g, "<redacted>")
          .replace(/xapp-\d-[A-Za-z0-9-]+/g, "<redacted>");
}

function formatUnknownErrorSafe(err: unknown): string {
  if (err instanceof Error) {
    return redactSecrets(toSingleLine(`${err.name}: ${err.message}`));
  }
  if (typeof err === "string") {
    return redactSecrets(toSingleLine(err));
  }
  if (err && typeof err === "object") {
    const e = err as Record<string, unknown>;
    const summary = {
      name: typeof e.name === "string" ? e.name : undefined,
      code: typeof e.code === "string" ? e.code : undefined,
      status: typeof e.status === "number" ? e.status : undefined,
      message: typeof e.message === "string" ? e.message : undefined,
    };
    return redactSecrets(toSingleLine(JSON.stringify(summary)));
  }
  return "unknown error";
}

Then use formatUnknownErrorSafe(...) in the reconnect logging.

Analyzed PR: #27232 at commit e04db96

Merged via squash in 949faff.

Thanks for the contribution. For traceability, I added the required changelog entry before merge and validated the PR with:

• pnpm check
• pnpm test:macmini

Both gate runs passed on the merged head.