Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions gateway/platforms/signal.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,15 @@ async def connect(self) -> bool:
resp = await self.client.get(f"{self.http_url}/api/v1/check", timeout=10.0)
if resp.status_code != 200:
logger.error("Signal: health check failed (status %d)", resp.status_code)
await self.client.aclose()
self.client = None
self._release_phone_lock()
return False
except Exception as e:
logger.error("Signal: cannot reach signal-cli at %s: %s", self.http_url, e)
await self.client.aclose()
self.client = None
self._release_phone_lock()
return False

self._running = True
Expand All @@ -243,6 +249,16 @@ async def connect(self) -> bool:
logger.info("Signal: connected to %s", self.http_url)
return True

def _release_phone_lock(self) -> None:
"""Release the Signal phone scoped lock if held."""
if self._phone_lock_identity:
try:
from gateway.status import release_scoped_lock
release_scoped_lock("signal-phone", self._phone_lock_identity)
except Exception as e:
logger.warning("Signal: Error releasing phone lock: %s", e, exc_info=True)
self._phone_lock_identity = None

async def disconnect(self) -> None:
"""Stop SSE listener and clean up."""
self._running = False
Expand Down Expand Up @@ -270,13 +286,7 @@ async def disconnect(self) -> None:
await self.client.aclose()
self.client = None

if self._phone_lock_identity:
try:
from gateway.status import release_scoped_lock
release_scoped_lock("signal-phone", self._phone_lock_identity)
except Exception as e:
logger.warning("Signal: Error releasing phone lock: %s", e, exc_info=True)
self._phone_lock_identity = None
self._release_phone_lock()

logger.info("Signal: disconnected")

Expand Down
8 changes: 8 additions & 0 deletions gateway/platforms/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,14 @@ async def handle_hermes_command(ack, command):

except Exception as e: # pragma: no cover - defensive logging
logger.error("[Slack] Connection failed: %s", e, exc_info=True)
# Release the platform lock so the next gateway start isn't blocked.
if self._token_lock_identity:
try:
from gateway.status import release_scoped_lock
release_scoped_lock('slack-app-token', self._token_lock_identity)
except Exception:
pass
self._token_lock_identity = None
return False

async def disconnect(self) -> None:
Expand Down
84 changes: 84 additions & 0 deletions tests/gateway/test_platform_lock_release.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
"""Tests for platform lock release on connect() failure.

When a platform adapter acquires a scoped lock during connect() and then
fails (bad token, network error), the lock must be released so the next
gateway start isn't blocked with "already in use" errors.

Discord got this fix in PR #5302. Slack and Signal were missed.
"""

import asyncio
from unittest.mock import AsyncMock, MagicMock, patch

import pytest


# ---------------------------------------------------------------------------
# Slack
# ---------------------------------------------------------------------------

class TestSlackLockReleaseOnConnectFailure:
"""gateway/platforms/slack.py — connect() must release lock on exception."""

@staticmethod
def _read_source() -> str:
import os
base = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
with open(os.path.join(base, "gateway", "platforms", "slack.py")) as f:
return f.read()

def test_except_block_releases_lock(self):
"""The except block in connect() should call release_scoped_lock."""
src = self._read_source()
# Find the except block near the end of connect()
start = src.index("async def connect(")
end = src.index("\n async def disconnect", start)
connect_body = src[start:end]

assert "release_scoped_lock" in connect_body, (
"Slack connect() except block does not release the scoped lock — "
"next gateway start will be blocked"
)


# ---------------------------------------------------------------------------
# Signal
# ---------------------------------------------------------------------------

class TestSignalLockReleaseOnConnectFailure:
"""gateway/platforms/signal.py — connect() must release lock on health check failure."""

@staticmethod
def _read_source() -> str:
import os
base = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
with open(os.path.join(base, "gateway", "platforms", "signal.py")) as f:
return f.read()

def test_health_check_failure_releases_lock(self):
"""Both return-False paths after health check must release the phone lock."""
src = self._read_source()
start = src.index("# Health check")
end = src.index("self._running = True", start)
health_block = src[start:end]

assert health_block.count("_release_phone_lock") >= 2, (
"Signal health check failure paths do not release the phone lock — "
"both the non-200 and exception paths need _release_phone_lock()"
)

def test_release_phone_lock_method_exists(self):
"""_release_phone_lock helper should exist for reuse."""
src = self._read_source()
assert "def _release_phone_lock(self)" in src

def test_health_check_failure_closes_client(self):
"""Both return-False paths must also close the httpx client."""
src = self._read_source()
start = src.index("# Health check")
end = src.index("self._running = True", start)
health_block = src[start:end]

assert health_block.count("await self.client.aclose()") >= 2, (
"Signal health check failure paths do not close the httpx client"
)
Loading
Loading