fix(whatsapp): skip sibling-recorded bridge ports; name a corrupt port record
A secondary allocated its bridge port by bind probe alone. A sibling whose bridge is down (gateway stopped, profile disabled) leaves its recorded port unbound, so a newly paired profile took it and both records claimed one port; the second to start hit the non-retryable whatsapp_bridge_conflict on every restart. Allocation now skips ports another profile's record claims. A non-numeric record surfaced int()'s "invalid literal" as the fatal text; it now names the record file and says to delete it. The secondary connect tests stub find_node_executable: connect() now ensures Node before anything else, so without it they exited before the secondary path. Co-authored-by: Victor Kyriazakos <victor@rocketfueldev.com> Co-authored-by: Enough1122 <Enough1122@users.noreply.github.com>
This commit is contained in:
committed by
Mark S.
parent
5827154a32
commit
01071fdbbb
@@ -1,5 +1,6 @@
|
||||
"""Non-destructive bridge allocation for multiplexed secondary profiles."""
|
||||
import socket
|
||||
from contextlib import suppress
|
||||
from pathlib import Path
|
||||
|
||||
# 3000 stays the default profile's historical port; secondaries allocate above it. 999 ports
|
||||
@@ -17,14 +18,34 @@ def port_is_free(port: int) -> bool:
|
||||
return True
|
||||
|
||||
|
||||
def _read_port_record(record: Path) -> int:
|
||||
try:
|
||||
return int(record.read_text(encoding="utf-8-sig").strip())
|
||||
except ValueError:
|
||||
raise ValueError(f"Bridge port record {record} is not a port number; delete it to allocate a new port") from None
|
||||
|
||||
|
||||
def _sibling_claims(home: Path) -> set:
|
||||
"""Ports other profiles' records claim. A sibling whose bridge is down leaves its port unbound, so a
|
||||
bind probe alone would hand it to this profile and the sibling would hit a conflict on its next start."""
|
||||
claims = set()
|
||||
for record in home.parent.glob("*/platforms/whatsapp/bridge_port"):
|
||||
if record.parents[2] != home:
|
||||
with suppress(OSError, ValueError):
|
||||
claims.add(_read_port_record(record))
|
||||
return claims
|
||||
|
||||
|
||||
def secondary_bridge_port(home: Path, explicit) -> int:
|
||||
record = home / "platforms/whatsapp/bridge_port"
|
||||
if explicit is not None:
|
||||
port = int(explicit)
|
||||
elif record.exists():
|
||||
port = int(record.read_text().strip())
|
||||
port = _read_port_record(record)
|
||||
else:
|
||||
port = next((p for p in range(SECONDARY_PORT_FIRST, SECONDARY_PORT_LAST + 1) if port_is_free(p)), None)
|
||||
claimed = _sibling_claims(home)
|
||||
port = next((p for p in range(SECONDARY_PORT_FIRST, SECONDARY_PORT_LAST + 1)
|
||||
if p not in claimed and port_is_free(p)), None)
|
||||
if port is None:
|
||||
raise ValueError(f"No free bridge port in {SECONDARY_PORT_FIRST}..{SECONDARY_PORT_LAST}")
|
||||
record.parent.mkdir(parents=True, exist_ok=True)
|
||||
@@ -47,7 +68,7 @@ def standalone_bridge_port(home: Path, explicit) -> int:
|
||||
record = home / "platforms/whatsapp/bridge_port"
|
||||
if not record.exists():
|
||||
return 3000
|
||||
port = int(record.read_text(encoding="utf-8").strip())
|
||||
port = _read_port_record(record)
|
||||
if not 1 <= port <= 65535:
|
||||
raise ValueError(f"Invalid bridge port {port}")
|
||||
return port
|
||||
|
||||
@@ -35,6 +35,7 @@ async def test_secondary_ports_and_foreign_listener(tmp_path, monkeypatch):
|
||||
import plugins.platforms.whatsapp.adapter as module
|
||||
adapters = []
|
||||
listener = socket.socket()
|
||||
monkeypatch.setattr(module, "find_node_executable", lambda name: f"/usr/bin/{name}")
|
||||
monkeypatch.setattr(module, "_write_bridge_pidfile", lambda *a: None)
|
||||
for name in ("a", "b"):
|
||||
home = tmp_path / name
|
||||
@@ -90,6 +91,23 @@ def _hold_port():
|
||||
return sock, sock.getsockname()[1]
|
||||
|
||||
|
||||
def test_allocation_skips_ports_recorded_by_siblings(tmp_path):
|
||||
"""A sibling whose bridge is down leaves its recorded port unbound; a new profile must not take it,
|
||||
or the sibling hits a non-retryable port conflict on its next start. A corrupt record names itself."""
|
||||
from plugins.platforms.whatsapp.bridge_ownership import port_is_free, secondary_bridge_port
|
||||
profiles = tmp_path / "profiles"
|
||||
sibling, fresh = profiles / "a", profiles / "b"
|
||||
recorded = next(p for p in range(3001, 4000) if port_is_free(p))
|
||||
(sibling / "platforms/whatsapp").mkdir(parents=True)
|
||||
(sibling / "platforms/whatsapp/bridge_port").write_text(str(recorded))
|
||||
fresh.mkdir()
|
||||
assert secondary_bridge_port(fresh, None) != recorded
|
||||
assert secondary_bridge_port(sibling, None) == recorded
|
||||
(fresh / "platforms/whatsapp/bridge_port").write_text("30o1")
|
||||
with pytest.raises(ValueError, match="delete it"):
|
||||
secondary_bridge_port(fresh, None)
|
||||
|
||||
|
||||
def test_ownership_verdicts(tmp_path):
|
||||
"""free: unbound port (a stale pidfile changes nothing, and nothing is signalled). ours: bound port
|
||||
and this profile's pidfile names a live process by pid + kernel start time. Anything else bound
|
||||
@@ -119,6 +137,7 @@ async def test_secondary_adopts_its_own_orphaned_bridge(tmp_path, monkeypatch):
|
||||
instead of dying with a port conflict. Only the identity-checked pidfile path may reap it."""
|
||||
import plugins.platforms.whatsapp.adapter as module
|
||||
from plugins.platforms.whatsapp.adapter import _write_bridge_pidfile
|
||||
monkeypatch.setattr(module, "find_node_executable", lambda name: f"/usr/bin/{name}")
|
||||
home = tmp_path / "c"
|
||||
home.mkdir()
|
||||
with _profile_runtime_scope(home, hydrate_secrets=False):
|
||||
|
||||
@@ -39,9 +39,10 @@ for that profile. An enabled profile without `creds.json` is skipped with the
|
||||
`whatsapp_unpaired` status and its pairing command.
|
||||
|
||||
An explicit `platforms.whatsapp.extra.bridge_port` takes precedence. Otherwise,
|
||||
a secondary selects the first free port in 3001 to 3999 and saves it in its own
|
||||
`platforms/whatsapp/bridge_port` file for subsequent starts. Operators can
|
||||
pre-create that file with a port number. The launch profile keeps port 3000.
|
||||
a secondary selects the first free port in 3001 to 3999 that no other profile's
|
||||
record claims, and saves it in its own `platforms/whatsapp/bridge_port` file for
|
||||
subsequent starts. Operators can pre-create that file with a port number; delete
|
||||
it to have a new port allocated. The launch profile keeps port 3000.
|
||||
|
||||
A secondary adopts a bridge already running on its port only when its own
|
||||
session pidfile identifies that process (pid and kernel start time), which is
|
||||
|
||||
Reference in New Issue
Block a user