Merge remote-tracking branch 'origin/message-readback-tools' into hot-dog

skills/chatto-platform-plugin/SKILL.md

@@ -11,6 +11,16 @@ metadata:
 
 # Hermes Chatto Platform Plugin
 
+## Where knowledge lives — what goes where
+
+Three homes, one rule each. Pick the destination by asking "does this travel with the plugin?":
+
+- **This skill** — plugin behavior that holds on ANY installation: what the tools do, debugging procedures tied to plugin code (gates, realtime stream, tool contract), lessons a fresh checkout would rediscover the hard way. If it references `~/.hermes` paths of a particular machine, a particular server, or a local Hermes checkout's quirks, it does NOT belong here.
+- **The repo root's `AGENTS.md`** — the binding rulebook for WORK ON THE REPO: editing, gates, vendoring, structure rules (e.g. how tools register through two doors). This skill does not repeat those rules; when a lesson belongs there, fix `AGENTS.md` instead.
+- **The operator's local skill (`chatto-local-preferences`)** — installation facts and personal workflow: server URL, room IDs, cron jobs, how to run the suite against THIS machine's Hermes checkout, baseline quirks of THIS dev environment (pre-existing failures, checker noise), the user's git/merge preferences.
+
+When in doubt: a lesson written because of one bug is upstream-worthy only if another installation could hit the same bug from the plugin's own code — not from a stale local checkout.
+
 ## When to Use
 
 Using or developing/debugging the Chatto gateway plugin: its plugin tools (`chatto_send_dm`, `chatto_send_room`, `chatto_list_rooms`, `chatto_get_message`, `chatto_get_history`) or adding new tools to the plugin. Repo-internal rules (editing, linting, vendoring) live in the plugin root's `AGENTS.md` — see Development workflow.
@@ -84,9 +94,19 @@ Pitfalls:
 - **Fail-open gates degrade silently.** An unresolved handle reads as "not a mention", so the bot barges in on every @colleague message with no log line louder than DEBUG. The fix is a fuller local cache, never a per-handle lookup: an admin-directory call (`admin_get_member`) needs permissions a bot account must not hold, and its `permission_denied` failed *every* lookup in the field, which fail-open turned into "answer every @colleague message". Prefer what the roster projection already holds, and keep it fed — the OPEN admission path fetches it precisely so this check has something to read.
 - **Mentions are extracted from body text** (`ChattoConstants.MENTION_RE`) outside code fences and inline code spans, matching the upstream web frontend — a handle quoted in code does not gate, and case-insensitivity applies throughout.
 
+## Realtime stream health — a message the bot never saw
+
+A message that produced no dispatch, no reaction and no log line usually was not swallowed by the gates — it arrived while the realtime stream was in a reconnect gap. `_chattolib_event_loop` answers ANY exception escaping `_handle_realtime_event` with `unexpected realtime error: <name>, reconnecting in Ns` and exponential backoff (up to 30s); inbound messages inside the gap are lost silently.
+
+1. `grep -n "unexpected realtime error" ~/.hermes/logs/agent.log` — the exception's name names a handler bug, not a transport failure. An `AttributeError`/`KeyError` on an event payload means the handler reads a field that event kind does not carry.
+2. Read the wire shape before reading a payload field: the envelope's `actor_id` is the identified user of an event; payload shapes live in the DESCRIPTOR blob of `vendor/common/chattolib/_pb/chatto/realtime/v1/events_pb2.py`, and chattolib's own `bot.py` dispatch shows the sanctioned field per event kind. Payload shapes drift with chattolib versions.
+3. Fix the handler, never the loop — the loop cannot tell handler bugs from wire damage, so both cost the same reconnect outage; no handler path may be able to raise.
+
 ## Development workflow
 
-1. **Read the repo's `AGENTS.md` first** — it is the binding rulebook for all work on this repo: editing, linting/type-checking gates, shell-script checks, typed access, overriding, capabilities, README discipline. This skill does not repeat those rules.
-2. **Gateway restart is REQUIRED after plugin code changes** — the running Hermes session keeps the old module in memory. Ask the user to restart, then verify via `tool_call`.
+1. **Read the repo's `AGENTS.md` first** — the binding rulebook for all work on this repo: editing, linting/type-checking gates, shell-script checks, typed access, overriding, capabilities, README discipline. This skill does not repeat those rules.
+2. **Gateway restart is REQUIRED after plugin code changes** — the running Hermes session keeps the old module in memory. The user's `/restart` chat command restarts in-band (gateway exits with code 75, the service manager relaunches); a restart issued from a tool terminal inside the gateway is blocked, because the gateway kills its own child processes on shutdown. Verify via `tool_call` afterwards.
 3. **Before requesting a restart, verify config-dependent fixes in an isolated subprocess** simulating the cron scope: pop the relevant env vars (`CHATTO_TOKEN`, `CHATTO_BASE_URL`), `sys.path.insert` the Hermes kernel and plugin dir, set `HERMES_HOME`, call the handler directly. A green isolated test proves the cron path before burning a restart cycle.
 4. **Bundled skills:** add a `skills/<name>/SKILL.md` and register it via `_register_bundled_skills(ctx)`, which passes the SKILL.md frontmatter `description` through — `register_skill` stores it verbatim and does not read the frontmatter itself, so an empty description leaves the skill listed with nothing explaining when to load it.
+5. **Run the suite against a Hermes checkout:** the Hermes kernel is not a plugin dependency and `yaml` is not in the dev group, so a plain run dies at import — the kernel path and the exact invocation are installation-specific (see the operator's local skill).
+6. **Mock event payloads to the wire shape, not to the attributes the code happens to read:** a bare `MagicMock()` payload answers any attribute, so tests pass on fields the real event never carries — use `MagicMock(spec=[...])` with exactly the proto's fields. Same family: a MagicMock stub client auto-creates plain (non-async) Mocks for unassigned methods, so assign `AsyncMock` explicitly before any `assert_not_awaited`.