ソースを参照

Spell out the working rules in AGENTS.md

Purpose and chattolib/Chatto version floor, human-README rules
(unambiguous wording, current versions), behavioural contract incl.
fail-closed auth, configuration surface, typed access, override marker,
capability bookkeeping and shellcheck.
Paul Klumpp 1 週間 前
コミット
23a10d27c4
1 ファイル変更191 行追加8 行削除
  1. 191 8
      AGENTS.md

+ 191 - 8
AGENTS.md

@@ -1,19 +1,175 @@
 # Agent Guidelines for hermes-chatto-plugin
 # Agent Guidelines for hermes-chatto-plugin
 
 
-## Tests
+## 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 corresponds to upstream `chattolib[realtime]`, the 0.4.x line
+(realtime protocol v1) — Chatto servers older than v0.4.19 are not supported.
+Respect that floor when refreshing `vendor/` via `vendor_chattolib.sh`
+(see VENDORING.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.)
+
+## Behavioural contract
+
+**These behaviours are promises to users, documented in README.md — adapter
+changes must keep them, or consciously renegotiate the docs.**
+
+- **Mention gating covers channels only.** With `require_mention` enabled,
+  channel-kind rooms answer only messages @-mentioning the bot; group-kind
+  rooms and DMs answer regardless. In channels with the gate off, a message
+  aimed at someone else gets a 🫥 acknowledgement instead of an answer.
+- **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.
+- **Processing reactions.** 👀 while working, then ✅ or ❌ — driven by
+  `on_processing_start`/`on_processing_complete` and the `reactions` setting,
+  never applied to the bot's own events.
+- **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
 `adapter.py` imports from `gateway.*`, which lives in the Hermes Agent rather
 than this repo. Without its source on the `PYTHONPATH`, collection dies with
 than this repo. Without its source on the `PYTHONPATH`, collection dies with
-`ModuleNotFoundError: No module named 'gateway'` before a single test runs. So
-fetch the Hermes Agent and keep it around before running the tests:
+`ModuleNotFoundError: No module named 'gateway'` before a single test runs.
+With the layout above:
 
 
 ```bash
 ```bash
-git clone https://github.com/NousResearch/hermes-agent.git /tmp/hermes
-PYTHONPATH=/tmp/hermes uv run --with pyyaml pytest -q
+PYTHONPATH=/path/to/hermes-agent uv run --with pyyaml pytest -q
 ```
 ```
 
 
-`gateway.config` needs `pyyaml`, which is not among our dev dependencies. If a
-checkout already exists somewhere, point at that path instead of cloning again.
+`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.
+- **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.
+- **Docstrings on every public method**, following PEP 257 — plus the override
+  marker required by *Overriding* below.
+
+### Linting & Type Checking
+
+**Run Ruff on everything you touch, and leave no Pylance findings behind.**
+
+Ruff is the linter and formatter for this repo:
+
+```bash
+ruff check .          # lint
+ruff format .         # format
+```
+
+Fix what `ruff check` reports instead of adding `# noqa` — if a rule genuinely
+does not fit here, that is a change to make deliberately, not inline.
+
+Pylance (VS Code's Python language server) is the second gate. Its errors and
+warnings are expected to be clean on files you edit; treat new squiggles as
+unfinished work. Do not silence them with `cast(Any, ...)` or `# type: ignore` —
+see *Typed access* below for why that trade hides real bugs.
+
+## 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.
+
+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
 ## Typed access
 
 
@@ -47,6 +203,30 @@ Tests should exercise real library types too, not mocks shaped like them. The
 upload bug was invisible because the tests replaced `_upload_asset` wholesale
 upload bug was invisible because the tests replaced `_upload_asset` wholesale
 with an `AsyncMock`, so no test ever touched a real `AssetUpload`.
 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
 ## Capabilities
 
 
 **When you extend what the adapter can do, keep the startup banner current.**
 **When you extend what the adapter can do, keep the startup banner current.**
@@ -67,9 +247,12 @@ So when you add an overridden `BasePlatformAdapter` method:
    hint goes into the system prompt and is the only way the model learns what the
    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.
    channel can do — without it the agent falls back to shelling out.
 3. Add a test in `TestRegistration` if the capability is user-visible.
 3. Add a test in `TestRegistration` if the capability is user-visible.
+4. Update the capability matrix and feature notes in README.md — that is the
+   public version of the same list.
 
 
 The reverse holds too: drop a method and its line disappears from the log on its
 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.
+own, but its `_CAPABILITY_LABELS` entry should go with it, along with its row in
+the user docs.
 
 
 ## Shell Scripts
 ## Shell Scripts