fix(irc): close 7 long-running-adapter gaps (lock, auth/join escalation, send confirmation, channel routing, edit support)

[lambertian]

Fixes 7 independently-verified bugs in the long-running IRC adapter, each with a regression test that is red on the base and green after the change. The adapter's out-of-process cron sibling (_standalone_send) already handles most of these cases correctly; the live gateway path had drifted from it. Every fix re-verified against current plugins/platforms/irc/adapter.py before implementing.

Test file: 19 new tests added; full tests/gateway/test_irc_adapter.py passes (63 total, was 44). tests/gateway/test_config_driven_access_policy.py still passes (the access-policy change rides the existing enforces_own_access_policy contract).

1. Identity lock guard was dead code — two profiles could share one IRC nick

Symptom: two gateway profiles configured with the same server:nickname both connect, producing dueling bots, 433 nick collisions, and split/double-processed responses on one identity.

Root cause: connect() guarded with if not acquire_scoped_lock("irc", lock_key):. acquire_scoped_lock returns tuple[bool, Optional[dict]], never a bare bool. A non-empty tuple is always truthy, so not (...) is always False — including the conflict case (False, {...}). The conflict branch (log + _set_fatal_error(... retryable=False) + return False) was unreachable. The lock file was still written as a side effect, so the result looked plausible while silently letting a second instance through.

Fix: route through the inherited _acquire_platform_lock("irc", lock_key, ...), which unpacks (acquired, existing), logs the owning PID, and escalates a non-retryable irc_lock fatal error on conflict — matching Telegram/Signal/Discord. Release switched to _release_platform_lock(). Note: the base helper escalates the code <scope>_lock (i.e. irc_lock); the old inline code used a cosmetic lock_conflict string that the base never emits — corrected in passing.

2. config.yaml allowed_users was silently default-denied at the gateway

Symptom: an operator who sets allowed_users in config.yaml (with no IRC_ALLOWED_USERS env var) has every message dropped, even from a listed nick. The docstring also claimed allowed_users: [] means "allow all" — false.

Root cause: the adapter reads extra.allowed_users and filters inline in _handle_line, but the gateway's _is_user_authorized only consults the IRC_ALLOWED_USERS env var. With no env allowlist, the gateway hits its "no allowlists configured" branch and, because IRC did not override enforces_own_access_policy (default False), falls through to GATEWAY_ALLOW_ALL_USERS (default false) → deny. The two access layers were wired to different config sources.

Fix: override enforces_own_access_policy to return bool(self._allowed_users_lower). This is fail-open-safe: the adapter claims to own its access policy only when the allowlist is non-empty (so the gateway trusts the intake check it already passed); an empty allowed_users returns False and falls through to the gateway default-deny — an empty list must never silently allow every IRC user. The misleading docstring is corrected to describe the deny-by-default behavior. A regression test asserts that with allowed_users=[] and no env allowlist an arbitrary user is denied at _is_user_authorized, and that a populated list is trusted.

3. Permanent auth/ban rejections (464/465/ERROR) retried forever

Symptom: a wrong server password or a server ban causes the gateway to reconnect indefinitely on the 30s→300s backoff, burning a full 30s registration timeout every cycle and, on a ban, escalating the server's penalty.

Root cause: _handle_line dispatched only PING, 001, 433, PRIVMSG, NICK. It had no handler for 464 (ERR_PASSWDMISMATCH), 465 (ERR_YOUREBANNEDCREEP), or a bare ERROR. On these the server closes the socket; the receive loop hits EOF before _mark_connected, so its finally guard sets nothing, while connect() is still blocked on the registration wait and then escalates a retryable registration_timeout. retryable=True keeps the reconnect watcher retrying unfixable credentials.

Fix: handle 464/465 → _set_fatal_error("auth_rejected"/"banned", retryable=False), and a bare ERROR while unregistered → _set_fatal_error("server_error", retryable=False); each sets _registration_event to unblock connect(). After the wait, connect() inspects has_fatal_error and returns False, preserving the non-retryable classification so the reconnect watcher drops the platform. A post-registration ERROR is left to the receive-loop EOF path (normal connection loss). This mirrors the 464/465 handling the standalone cron path already had.

4. JOIN was never confirmed — channel-join failures black-holed every send

Symptom: when the configured channel rejects the JOIN (+i invite-only, +k key, +l full, ban, no-such-channel), the adapter still marks itself connected and logs "joined". Subsequent channel sends issue PRIVMSG to a channel the bot is not in; on default +n networks the server silently drops them, yet send() returns success.

Root cause: connect() issued JOIN then immediately _mark_connected() without reading the response, and _handle_line ignored JOIN-result numerics entirely.

Fix: add _join_event/_join_error; after JOIN, wait (10s) for the own JOIN echo / RPL_ENDOFNAMES (366) before _mark_connected(), and handle 403/405/471/473/474/475 by recording the rejection and escalating a non-retryable join_failed. A missing ack within the window logs a warning and proceeds (some servers are slow); an explicit rejection aborts. Reuses the numeric set the standalone path already rejects.

5. send() reported success for lines silently dropped mid-send

Symptom: if the TCP/TLS connection drops after send()'s initial guard — e.g. after the first line of a multi-line message, during the 0.3s rate-limit sleep — the remaining lines are discarded but send() still returns success=True with a fabricated message_id, and never sets retryable, so the base layer cannot re-deliver.

Root cause: _send_raw early-returned silently (no exception) when the writer was gone, and send() only checked connection state once at the top.

Fix: _send_raw now returns bool (False when the writer is gone/closing). send() checks the return per line and bails with SendResult(success=False, error="connection lost during send", retryable=True); exceptions in the loop are now also marked retryable=True. Lifecycle callers (PING/PONG, JOIN, NICK) ignore the return as before.

6. Inbound '+' and '!' channels misrouted as DMs, bypassing the require-mention gate

Symptom: messages in RFC 2811 + (no-modes) and ! (safe) channels were treated as DMs — replies went to the sender's nick instead of the channel, and every message in those channels was dispatched even when it did not address the bot, defeating the channel require-mention behavior.

Root cause: the inbound path used target.startswith("#") or target.startswith("&"), missing + and !. The adapter's own _is_irc_channel helper (used by the outbound JOIN decision) already covers the full "#&+!" set, so the two paths were inconsistent.

Fix: use _is_irc_channel(target) in _handle_line and _is_irc_channel(chat_id) in get_chat_info, so inbound classification matches the outbound JOIN decision and +/! channels route as channels and pass through the require-mention gate.

7. No edit API, but SUPPORTS_MESSAGE_EDITING defaulted True — broken streaming bubble

Symptom: during streaming the first PRIVMSG line is sent ending in a literal cursor character; the next frame's edit fails (IRC has no edit primitive), the consumer enters fallback and re-delivers the full answer, and the cursor-strip is itself a failing edit — so the user sees a broken first line plus a duplicate message.

Root cause: SUPPORTS_MESSAGE_EDITING resolves via getattr(adapter, "SUPPORTS_MESSAGE_EDITING", True), so the IRC adapter (no override, no edit_message) defaulted to True and the streaming path assumed in-place edits.

Fix: declare SUPPORTS_MESSAGE_EDITING = False on IRCAdapter, matching Signal. Streaming then suppresses the cursor and the edit attempts and uses the no-edit path.

Dropped findings

A couple of findings from the audit were scoped out of this PR to keep its surface focused.

Overlap

These fixes are confined to connect()/disconnect(), _handle_line, get_chat_info, send()/_send_raw, and the enforces_own_access_policy/SUPPORTS_MESSAGE_EDITING class attributes. The open IRC PRs #4676 (a separate core implementation under gateway/platforms/) and #30333 (default-port selection) touch different code and do not overlap.

[tonydwb]

Code Review Summary

Verdict: Approved ✅ — IRC platform reliability fixes.

Changes

Close 7 long-running-adapter gaps: lock, auth/join escalation, send confirmation, channel routing, edit support.

---

Reviewed by Hermes Agent

[teknium1]

Thanks for the focused IRC reliability audit. Several reported defects still reproduce on current main, including the tuple-truthiness lock guard at plugins/platforms/irc/adapter.py:170 and immediate successful connection marking after JOIN at :215-219.

Problems

• The proposed enforces_own_access_policy return at PR plugins/platforms/irc/adapter.py:185 is insufficient on current main. gateway/authz_mixin.py:491-512 trusts an own-policy adapter only when its effective _group_policy / _dm_policy is allowlist (or a group sender allowlist exists). IRC provides neither, so config-only allowed_users remains default-denied.
• PR adapter.py:210 routes locks through _acquire_platform_lock, but current gateway/platforms/base.py:2769 marks conflicts retryable. This conflicts with the PR’s non-retryable assertion and description.
• Current IRC connect accepts is_reconnect at plugins/platforms/irc/adapter.py:155; preserve that contract when salvaging.

Suggested changes

• Map a populated IRC allowed_users list into the current DM and group allowlist contract and test GatewayRunner._is_user_authorized for both routes.
• Align the lock test with the shared helper or explicitly justify an IRC-specific classification.
• Update website/docs/user-guide/messaging/irc.md:35,56 to match the finalized empty-list semantics.

Automated hermes-sweeper review.

[teknium1]

On current main this flag alone is not trusted: gateway/authz_mixin.py:491-512 additionally requires an effective _group_policy/_dm_policy of allowlist (or a group sender allowlist). IRC exposes neither, so config-only allowed_users remains denied for both group and DM routes.

[teknium1]

Current BasePlatformAdapter._acquire_platform_lock() sets retryable=True on conflict (gateway/platforms/base.py:2769). This makes the PR’s new non-retryable lock assertion and description inconsistent with the helper it now calls; please align the behavior and test.