# Agent Guidelines for hermes-chatto-plugin

## This document stays contradiction-free

**AGENTS.md is a rulebook — every rule in it must be readable without having
to guess which one wins.**

If two passages turn out to conflict, or a rule admits two readings with
different outcomes, do not quietly choose one: point out the contradiction
where you hit it, name both passages, and resolve it here in the text so the
next reader cannot stumble into the same fork.

## What this plugin is

This plugin exists to connect a Hermes Agent to a Chatto server. All Chatto
API interaction goes through `chattolib` (vendored under `vendor/`) — never
hand-roll ConnectRPC/WebSocket calls the library already covers. When behaviour
is unclear on our side, the Chatto server source is the reference:

```bash
git clone https://github.com/chattocorp/chatto.git
```

The vendored copy is upstream `chattolib[realtime]` 0.4.20.post1 (realtime
protocol v1); the matching server floor is Chatto v0.4.20. Refreshing `vendor/`
via `vendor_chattolib.sh` can move that floor (see VENDORING.md) — after each
refresh, restate the new version here and in README.md.

## README.md is the human README

**Two audiences: AGENTS.md is read by agents with this source tree open;
README.md is read by humans who have not. That makes README.md the document
where ambiguity is not allowed.**

Everything a user meets first lives there — what the plugin is, how it
behaves, how to configure it. Unambiguous in practice means:

- **Say which Chatto you mean.** Server, chattolib and plugin all answer to
  "Chatto"; write the one you mean. (A capability-matrix column labelled just
  "Chatto" recently had to be asked whether it meant the plugin.)
- **One sentence, one reading.** If a claim can be parsed two ways, it is
  false in at least one of them — rewrite until only one parse remains.
- **No shorthand that needs the source.** Config keys, env vars and RPC names
  appear spelled out, never as "see adapter.py".
- **Keep versions current.** Compatibility claims must match reality — chattolib
  tracks Chatto upstream, so every `vendor/` refresh can move the supported
  server floor, and README.md has to move with it. ("Tested against Chatto
  v0.4.19" outlived its truth the moment the vendored copy became 0.4.20.)

### after-install.md

**Hermes Agent displays `after-install.md` to the user right after plugin
installation — that is the platform-plugin convention, not our timing choice.
By then the installer has already walked through `plugin.yaml`'s
`requires_env`/`optional_env` prompts and written the answers into
`~/.hermes/.env` (`hermes_setup_fn`, `save_env_value`).**

It is the first instruction anyone reads, ahead of README.md, config files and
source. Hold it to the same no-ambiguity bar and keep it in step with reality:
every env var, command and next step printed there must work exactly as
written, so it moves whenever setup, configuration or first-run behavior
changes. Because the values were just collected, after-install.md treats
`.env`/`config.yaml` as a **review** step — "check what the installer wrote",
not "edit these files to begin".

## Behavioural contract

**These behaviours are promises to users, documented in README.md — adapter
changes must keep them, or consciously renegotiate the docs.**

- **Rooms are opt-in via two mutually exclusive lists.** Chatto has no group
  rooms like Signal — every non-DM surface is a channel, and a room
  participates only if it is listed: in `CHATTO_REQUIRE_MENTION_ROOMS` it
  answers only messages addressing the bot — by @-mention or by the
  broadcast handles `@all`/`@here` (the only virtual handles Chatto defines,
  FDR-006); in `CHATTO_OPTIONAL_MENTION_ROOMS` it answers every message,
  and one aimed at someone else gets a 🫥 acknowledgement instead of an
  answer. A room on neither list stays silent — read-only: still marked as
  read, never seeded into context, never answered, no processing reactions;
  the gate sits at the top of the dispatch paths, before any API call.
  Unknown room kinds count as channels (a server that never sets `kind`
  shows up as UNSPECIFIED), so they go through the same lists. Both gates
  share `_mentions_me`, so "addressed" means the same thing everywhere:
  case-insensitive handle matching outside code regions, per the Chatto web
  frontend's extraction. DMs answer regardless so `/join` stays reachable;
  `hermes_validate_config` rejects listing a room on both lists.
  `CHATTO_HOME_CHANNEL` is orthogonal: it is the default outbound target for
  context-less cron/notification delivery, not an answer destination.
- **Threads stay threads.** An inbound thread reply keeps its thread context;
  a fresh room reply opens a thread under the incoming message unless
  `auto_thread` is disabled or the room is a DM.
- **Edits are corrections, not new traffic.** An inbound `message_edited`
  re-runs the admission gates against the new body: a message currently being
  processed is cancelled (🚫) and redispatched with the edited text; a queued
  follow-up is rewritten in place; a never-dispatched message (a forgotten
  @mention, say) gets a fresh turn if the gates pass now; an already-answered
  message stays answered — the `_dispatched_ids` ring exists so edits cannot
  re-animate old conversations. Edits older than `CHATTO_EDIT_WINDOW` seconds
  (default 300) are dropped, as are edited DM-command texts (`/join`, `/leave`
  ran once and must not run again); `CHATTO_EDIT_DISPATCH=false` turns the
  whole behavior off. Own edit echoes (the streaming path) are filtered by
  actor before any of this.
- **Processing reactions.** 👀 while working, then ✅, ❌, or 🚫 (cancelled) —
  driven by `on_processing_start`/`on_processing_complete` and the `reactions`
  setting, never applied to the bot's own events.
- **Room membership is server-side, commands ride over DMs.** `/join` and
  `/leave` in a DM call RoomService/JoinRoom/LeaveRoom, so membership survives
  restarts; the joined list mirrors it on every `_refresh_rooms()`. Silent
  rooms are joined read-only — no history seeding (same rule as
  `_refresh_rooms`). A channel-kind `/join` reply reports the room's
  mention-list status; an unlisted one names both `CHATTO_*_MENTION_ROOMS`
  lines verbatim, because that reply is where users copy the room ID from —
  the bot never writes `~/.hermes/.env` itself. DMs and the configured home
  channel refuse `/leave`. Commands never reach the agent pipeline and are
  not intercepted outside DMs.
- **Length handling.** `send()` splits at 9900 chars against the 10000-char
  server limit; `edit_message()` refuses over-long content so callers fall
  back to `send()` rather than receiving silent truncation.

### Auth defaults to closed

Without `CHATTO_ALLOWED_USERS` and without `CHATTO_ALLOW_ALL_USERS=true`,
`_check_auth` denies everyone. Keep that fail-closed stance when touching the
auth path. `hermes_validate_config` rejects setting both options at once on
purpose, and `allow_all_users` hands every Chatto user full agent access
including tool use.

## Development

Work on this plugin happens against a real Hermes Agent checkout — set that up
first, everything else builds on it:

1. Fetch the Hermes Agent:

   ```bash
   git clone https://github.com/NousResearch/hermes-agent.git
   ```

2. Put this plugin at `hermes-agent/plugins/platforms/hermes-chatto-plugin`.
   Either clone this repo straight into that path, or link an existing checkout
   so you can keep working in your own copy:

   ```bash
   ln -s /path/to/hermes-chatto-plugin \
     hermes-agent/plugins/platforms/hermes-chatto-plugin
   ```

### Tests

`adapter.py` imports from `gateway.*`, which lives in the Hermes Agent rather
than this repo. Without its source on the `PYTHONPATH`, collection dies with
`ModuleNotFoundError: No module named 'gateway'` before a single test runs.
With the layout above:

```bash
PYTHONPATH=/path/to/hermes-agent uv run --with pyyaml pytest -q
```

`gateway.config` needs `pyyaml`, which is not among our dev dependencies.

### Readability

**Code is written for humans first and machines second — follow Clean Code
principles, with PEP 8 as the baseline.**

Everything in this repo gets read far more often than it gets written: by
maintainers revisiting it months later, and by agents working from these
guidelines. Optimising for that reader is not optional polish. Concretely:

- **PEP 8** for naming, layout and structure — Ruff enforces most of it
  mechanically, but the rules below are judgement calls no linter makes for you.
- **Small functions with one job.** If a function needs paragraphs of `#`
  commentary to explain its flow, split it until the code explains itself.
- **Names that say what they mean.** `_seed_room` beats `do_init`; a boolean
  reads as a question (`is_seen`, `retryable`). No single-letter names outside
  throwaway loop variables. Locals earn their line the same way: keep them
  when they narrow, transform or get reused across branches; drop them when
  they only re-alias a typed attribute (`asset_id = asset.id` reads inline
  just as well).
- **Comments explain why, not what.** The code already says what happens;
  the comment earns its line by recording intent, constraints, or the bug a
  guard exists to prevent (see `adapter.py` for the house style).
- **Explicit over clever.** Idiomatic Python beats micro-optimised trickery —
  comprehension over `map`/`filter` chains, dataclasses over dict-shaped blobs,
  early returns over nested conditionals.
- **`if` for states, `try` for faults — line count is not the metric.**
  A normal absence ("no client right now") reads as a guard clause;
  try/except is reserved for operations that can genuinely fail, with each
  except clause stating its own recovery. Folding a state check into an
  operation's try saves lines and costs semantics: the shared handler cannot
  tell "nothing there" from "server broke", so retry decisions and log blame
  go wrong. (The client helpers in `adapter.py` document the split locally.)
- **Docstrings on every public method**, following PEP 257 — plus the override
  marker required by *Overriding* below.

### Linting & Type Checking

**Both gates run over the repo's whole first-party code — not just the lines
you touched. Someone always forgets an older file otherwise.**

```bash
ruff check .          # lint
ruff format .         # format
uv run basedpyright   # types
```

Ruff excludes `vendor/` and `dist/` via `pyproject.toml`. basedpyright's scope
lives in `pyrightconfig.json`, which likewise skips `vendor/`, build output and
the mock-heavy test modules; its `extraPaths` resolve both Hermes layouts from
*Development* above.

Fix what the tools report instead of adding `# noqa` — if a rule genuinely
does not fit here, that is a change to make deliberately, not inline. The same
goes for silencing type findings with `cast(Any, ...)` or `# type: ignore` —
see *Typed access* below for why that trade hides real bugs.

**Sporadic dead-field check.** Neither gate catches instance state that is
written but never read: Ruff's F841 stops at locals, and pyright counts an
attribute assignment as a use. Leftovers from long-gone features therefore
survive indefinitely (`_dedup`, `_resume_cursor`, the `_ws_*` trio and the
`_token`/`_user_*` mirrors all lived here unnoticed). Run the sweep whenever
a feature is removed from or reworked in the adapter — that is when its
state gets orphaned:

```bash
git grep -hoE 'self\.[a-z_][a-z0-9_]*' -- '*.py' ':!vendor' \
  | sort | uniq -c | sort -n
```

A low count is a suspect, not a verdict — a lone mention can be dead data,
but just as well a one-shot helper method. Count assignments vs. other
mentions per name, confirm each suspect against the test fixtures and the
Hermes base class, and delete whatever nothing touches.

Two blind spots this sweep leaves, cover them by hand. First, the grep sees
only ``self.*`` state — module-level helpers go stale the same way, so check
their call sites whenever one smells redundant (a single-caller wrapper is
the same finding as an unread attribute). Second, duplication is found by
naming jobs, not by diffing text: before judging any function, ask who else
does its job (downloads? image materialisation?) — two implementations of
one job are one too many, because they drift silently until behaviour
differs (an uncapped downloader next to a capped one shipped here for
months).

## Configuration surface

**One field, three coordinated names — declared once on `ChattoConfiguration`.**

A config value lives as a `ConfigField` class attribute in `platform_config.py`.
From the attribute name it derives the `config.yaml` key under
`gateway.platforms.chatto.extra` (the *config_key*, overridable per field) and
the `CHATTO_*` environment variable (upper-cased config_key). Resolution order
per instance: env var → extra → default. Declaring a field is enough for the
adapter, the env-enablement seeding and the setup wizard to pick it up;
`plugin.yaml` additionally repeats the env names as wizard prompts, so keep
those entries in step. Every `optional_env` entry states its default right in
its description (`(default: …)`): at install time the prompt is all the user
sees — "what happens if I leave this empty?" must be answerable without
opening README.md. Required entries have no default to state; they say what
happens when left blank instead (see `CHATTO_BASE_URL`).

When adding or renaming a field, also update the environment-variable table in
README.md — and document each option's default next to it, for env vars
(*Default* column) and `config.yaml` keys alike. A user reading only the
configuration docs must be able to predict what happens when they set nothing.
Secrets (`CHATTO_TOKEN`, `CHATTO_PASSWORD`) belong in `~/.hermes/.env`, never
in `config.yaml` — docs and setup wizard rely on that split.

## Typed access

**Work against the declared types. Do not reach for values dynamically when a
typed attribute exists.**

`chattolib` ships dataclasses with `py.typed` for everything it returns —
`Message`, `Asset`, `AssetUpload`, `Room`, `User`. Read their fields directly, so
a wrong name is a loud `AttributeError` at the first call and a type checker can
see it before that:

```python
upload_id = upload.upload_id  # yes
upload_id = str(getattr(cast(Any, upload), "id", ""))  # no
```

The second form cost us every single file upload. `AssetUpload` has no `id`
field — it is called `upload_id` — but `cast(Any, ...)` disables the check and
the `""` default turns the mismatch into an empty string. The result was an
error message blaming the server (`CreateUpload returned no upload ID`) for a
name we got wrong ourselves, and it survived in production because it never
raised.

So: no `cast(Any, ...)` on a value whose type is known, and no `getattr` with a
fallback default to paper over a field you are not sure about — look the field up
in `vendor/common/chattolib/types.py` instead. `getattr` is fine for genuinely
optional or duck-typed things (a handler that may not be set, a payload from an
untyped source), not for dodging a type.

Tests should exercise real library types too, not mocks shaped like them. The
upload bug was invisible because the tests replaced `_upload_asset` wholesale
with an `AsyncMock`, so no test ever touched a real `AssetUpload`.

## Overriding

**Every method that overrides a base-class method must say so in its
docstring, closing with the marker line `BasePlatformAdapter override` — the
pattern already used throughout `adapter.py`.**

This plugin lives by subclassing (`ChattoAdapter` overrides `BasePlatformAdapter`
methods throughout), so an override is not an implementation detail — it is the
contract between plugin and platform. Follow the existing form: first line says
what the method does, the body spells out what changes about the inherited
behaviour, and the last line names the parent class:

```python
async def edit_message(self, chat_id, message_id, content, *, finalize=False):
    """Edit a message we previously sent, via MessageService/UpdateMessage.

    Without this override the base class reports "Not supported" and every
    incremental streaming update arrives as a *new* message. Content beyond
    the per-message limit is refused so callers fall back to ``send()``.

    BasePlatformAdapter override
    """
```

## Capabilities

**When you extend what the adapter can do, keep the startup banner current.**

`_capabilities()` in `adapter.py` logs what the plugin supports at registration,
so starting it up tells you what is available instead of making you read the
source. Whether a capability counts is derived from real overrides: it only
counts when `ChattoAdapter` replaces the `BasePlatformAdapter` method. The label
for it, however, lives in `_CAPABILITY_LABELS` and is *not* discovered
automatically.

So when you add an overridden `BasePlatformAdapter` method:

1. Add an entry to `_CAPABILITY_LABELS` (method name → wording that means
   something to someone who has not read the code). Skip this and the plugin can
   do the thing but says so nowhere.
2. Check whether the `platform_hint` in `register()` needs to mention it. That
   hint goes into the system prompt and is the only way the model learns what the
   channel can do — without it the agent falls back to shelling out.
3. Add a test in `TestRegistration` if the capability is user-visible.
4. Update the capability matrix and feature notes in README.md — the
   user-facing counterpart of the startup list. It may say more than the
   banner (setup-level features such as markdown or reconnect belong there),
   but nothing less.

The reverse holds too: drop a method and its line disappears from the log on its
own, but its `_CAPABILITY_LABELS` entry should go with it, along with its row in
the user docs.

## Shell Scripts

**Every shell script in this repo must pass `shellcheck` before its change
lands.**

Right now that is exactly one script: `vendor_chattolib.sh`. After editing
it — or adding any other script — run:

```bash
shellcheck vendor_chattolib.sh
```

Fix what it reports instead of suppressing it, same stance as *Linting &
Type Checking* above: an exemption is a deliberate decision recorded here in
this document, never an inline disable. If a future script ever needs
project-specific options, put them in a `.shellcheckrc`, commit that file,
and name it here.

Why this earns a rule of its own: the script deletes things (`rm -rf
"$VENDOR_DIR"`, the build-dir trap), so a quoting or word-splitting bug is
destructive rather than merely wrong output — and both are exactly the
mistakes review tends to skim past.
