All green. Report: **Implemented / verified (phase 01 final pass)** - `.agents/validate.sh` re-verified as the gate: ①`tests/run_tests.sh` ②`SANITIZE=1` run ③`uv … platformio run -e bridge -e console` ④page byte count ⑤hygiene, then the informational `tools/hw_probe.sh`. Added an explicit `== repo hygiene ==` header + `repo hygiene: OK (57 tracked files…)` verdict line so every check prints one (previously hygiene passed in silence). - `.agents/HW_VERIFICATION.md`: baseline (bridge 26.7 %/87 592 B, 36.1 %/1 135 293 B; console 16.8 %/33.6 %; 3406 checks; page 48 598 B) re-confirmed; served-page row extended with this pass's one-attempt measurements. **Tests / gate (100 % green)** - `bash .agents/validate.sh` → **exit 0** (also from `/tmp`): `RESULT: all suites passed` twice (3406 = 3162 C++ + 160 relayctl + 84 page), `2 succeeded`, `page bytes: 48598`, `repo hygiene: OK`, `validation OK`. - Faults re-demonstrated then reverted: (a) inverted `test_keystate` assert → cold-copy gate `RESULT: FAILED`, **GATE_EXIT=1**; (b) markup id `pArmed`→`pArmedZ` → page suite rc 1; (c) type error in `src/main.cpp` → `bridge FAILED / console FAILED`, rc 1; (d) hygiene regex matches `certs/test.pem` (git index commands are forbidden in this pass — task 01's report holds the original live `git add` demo). - Lint: no linter is configured for this repo; compiler is the linter — `-Wall -Wextra` host suites + both firmware builds: **0 warnings**; `bash -n` clean; `py_compile` clean. `ruff check` (this machine's broad ruleset) reports 21 pre-existing *preview-rule* style nits in untouched `tools/*.py` + `tests/test_relayctl.py` (SIM/UP/ISC/EXE/RUF/FURB), zero correctness findings — not churned: `watch_target.py`/`relayctl.py` are hardware-verified artifacts outside this phase. `shellcheck` is absent here and not in the local container image (fetching it would violate the gate's no-network rule). Coverage: n/a (shell + docs; no new Python). **Completion criteria** — all met: exit 0 with per-check output ✓; SANITIZE in-gate ✓; no `-t upload`/device curl/network fetch (grep: only in comments; `run_tests.sh` checks `podman image exists` first, so never pulls) ✓; faults bite ✓; HW_VERIFICATION.md names command+expected observation for every 02–05 device claim + one-attempt rule ✓; full suite green, `git status` = phase artifacts + mandated doc edits, nothing stray ✓. **Documentation** — README inconsistencies found & fixed: `test_proto` 193→**2327** (table now sums to the headline 3406); duplicate stale `test_page 47` row deleted (element-id clause merged into the 84 row); "last six checks"/"fails 3 checks" claims replaced by measurement (5 fail-safe source pins; `linked()` gate → 2, accept-stamp → 1, both → 3); console line formats corrected to what the code prints (`mouse interface=present target=report protocol buttons=0x00 …`, `queue 0 keys pending…`, `target=BOOT protocol (refused)`); status-JSON sample gained `typed/typing/mouse/mbtn/mframes`; footprint 87 400/35.5 %→87 592/36.1 %; page delivery `Content-Length: 35164`→**48 598** (re-measured live, one capped curl: 200/48 598, 404 on `/nope`, 401 on `/status`); stale Backlog item "Mouse." removed and renumbered; "~35 KB"→"~48 KB"; page-button names matched to the real markup; `validate.sh`/`HW_VERIFICATION.md`/`hw_probe.sh` added to Architecture, Tests and End-to-end. AGENTS.md: only stale statements fixed — "one commit, `Initial import`… predates the mouse and `output.h`" and the pure-module list missing `mouse.h`/`output.h`; not expanded. **Deviations** — Did not run the keystroke/mouse device rows: the board's target is this very machine, so injecting input during an unattended pass is the side effect the README's "hazard nobody warns you about" forbids; those rows stay blank per the contract (read-only GETs only). No live infra changed. **Next pending phase:** `02_tls_control_channel` (tasks `01_tls_identity_module` … `06_tls_docs_rollback`).
180 lines
12 KiB
Markdown
180 lines
12 KiB
Markdown
# AGENTS.md — working on keyrelay
|
||
|
||
A WiFi-controlled USB HID keyboard: laptop browser → ESP32-S3 board → USB into the target.
|
||
The target runs no software and sees a Logitech G815. Everything the board does is gated on
|
||
one idea: **the difference between a remote keyboard and an unattended injection device is a
|
||
human deciding, live, with the arming light on.** Changes that quietly move that line are the
|
||
ones to catch here.
|
||
|
||
The README is the single source of truth for behaviour, protocol, safety and measurement.
|
||
This file is the operating manual: what must not break, how to build and verify, and where
|
||
the traps are. **Do not fork the protocol or the safety model into `docs/`** — a second copy
|
||
drifts, and two paths that can disagree about arming is the exact failure this project keeps
|
||
having to fix (see README, "Ten bugs").
|
||
|
||
## Phased execution
|
||
|
||
This repository runs work through `.agents/phases/`:
|
||
|
||
1. Always read `.agents/PLAN.md` first to understand the project context and goals.
|
||
2. Follow the phased execution protocol in `.agents/phases/`.
|
||
3. Never modify `.agents/PLAN.md` or any files in `.agents/phases/complete/`.
|
||
4. If you need to update any file in `.agents/phases/todo/`, you must ask the user for permission first.
|
||
5. Strictly adhere to the **LOCKED DECISIONS** listed in `.agents/PLAN.md`.
|
||
|
||
## The invariants
|
||
|
||
1. **State, not events.** `S=<mods>:<usages>` restates the *entire* desired key state. Applying
|
||
the same state twice is a no-op; any one message resynchronises the target completely. Never
|
||
introduce key-up/key-down events, sequence numbers, or a resynchronisation timer. Every
|
||
stuck-key horror story in HID injection tooling is an event-protocol bug.
|
||
2. **Disarmed is the only default.** Boot, reload, reconnect, focus loss, socket loss, target
|
||
detach, bus suspend, queue overflow and 8 s of controller silence all disarm, and disarming
|
||
releases every key **and every mouse button**. Arming needs the passphrase, the board
|
||
button, or the serial console — nothing else may gain the ability. The mouse is captured,
|
||
never grabbed: Pointer Lock, so `Esc` (the browser's own, never forwarded) always frees
|
||
it. `mouse off` over serial removes the pointer interface at the descriptor level, which
|
||
needs a reboot and is the rollback.
|
||
3. **Every fail-safe cancels work in flight; nothing "pauses"** — and **no fail-safe may be
|
||
gated on the absence of the thing it detects.** `ControlServer::linked()` is false when a
|
||
controller is gone, so gating the silence-disarm on it made the backstop for a vanished
|
||
controller uncallable exactly when a controller vanished. Same shape: `closeLink()`
|
||
releasing only `if (linked())` skipped the release for every path that cleared `_authed`
|
||
first. If a rule fires when X stops existing, `X still exists` cannot be a precondition
|
||
of it. See README bug 10; `test_relayctl.py` pins the three sentences.
|
||
|
||
`releaseEverything()` in
|
||
`main.cpp` is the single funnel: it clears the report queue, raises the keys, ends the chord
|
||
and discards a pasted run. Any new state that can outlive a disconnect (a queue, a timer, a
|
||
run) must be cancelled there too, or it will keep acting after the operator stopped asking.
|
||
4. **Nothing is stored.** Counters only. No keystrokes, characters, key names or text in NVS,
|
||
logs, `Serial.printf`, error messages or status frames — `src/main.cpp` prints *lengths* for
|
||
text runs, never content. No macro store, no schedule, no replay, no auto-arm.
|
||
5. **Reject, never guess.** A malformed frame leaves the previous state in force. A character
|
||
the map cannot express is `E9` and *none* of that frame is typed — a paste that quietly loses
|
||
a character is worse than one that refuses. Silent sanitising is never the default.
|
||
6. **`Ctrl+Alt+Del` is fixed on the device.** It is never composed from wire bytes; on a Linux
|
||
VT it reboots the target. Same reasoning for the VT-switch chords.
|
||
7. **One frame is one command.** ASCII, no JSON inbound, ≤ `kWsMessageCap`. Long payloads are
|
||
the *controller's* problem to split, with the board reporting how much it still holds.
|
||
8. **The keyboard is exclusive; the board is not.** A second `/ws` upgrade is refused (`E3`);
|
||
read-only HTTP is served even while a controller holds the bridge. Keep both routes going
|
||
through `serveReadonly()` so they cannot drift apart.
|
||
|
||
## Three tables that must agree
|
||
|
||
| table | where | names | pinned by |
|
||
|---|---|---|---|
|
||
| `CODE_TO_USAGE` | `src/web_page.h` | physical position → usage | `tests/test_relayctl.py` |
|
||
| `CODE_TO_USAGE` | `tools/relayctl.py` | same, for the terminal | `tests/test_relayctl.py` |
|
||
| `strokeFor()` | `src/text.h` | character → modifier + usage | `tests/test_text.cpp` (whole printable range walked) + tool/board agreement on all 128 ASCII codes |
|
||
| mouse button bits | `src/mouse.h`, `src/web_page.h` (`MOUSE_BTN`), `tools/relayctl.py` (`MOUSE_BUTTONS`) | HID bit → DOM `MouseEvent.button` **and** name | `test_proto.cpp` (page↔config), `test_relayctl.py` (tool↔page↔config) |
|
||
| evdev axis codes | `tools/watch_target.py` (`REL_*`) | kernel code → label, for the *verifier* | `test_relayctl.py` — pinned to `<linux/input-event-codes.h>`, **not** to the report's byte order |
|
||
|
||
The last row exists because it was guessed from the HID report's field order and was wrong
|
||
(`REL_WHEEL` is 8, not 2): a working wheel looked dead and the summary said no scroll
|
||
happened. Codes the verifier has no name for are now counted and printed rather than
|
||
dropped — the same fix as giving F13–F24 names. **When you add a table that decodes
|
||
something somebody else defined, test the table, not your confidence in it.**
|
||
|
||
Chord buttons carry their **wire bytes** in `data-chord`, not a name — which is why
|
||
`tests/test_proto.cpp` asserts every labelled chord against the combination its label promises.
|
||
A mislabelled button parses cleanly, is correctly gated, arrives intact and is the wrong key;
|
||
Alt+Tab was `05:04` (Ctrl+Alt+A) until that test existed. `TEXT_FRAME_MAX` / `TEXT_JOB_MAX` in
|
||
the page and `TEXT_FRAME_MAX` / `MAX_PASTE_CHARS` in `relayctl.py` are hand-copied from
|
||
`config.h` and are asserted equal in tests — change one, the suite fails, that is the point.
|
||
|
||
## Build, test, flash
|
||
|
||
```bash
|
||
tests/run_tests.sh # 3406 checks, no hardware
|
||
SANITIZE=1 tests/run_tests.sh # + ASan/UBSan — use it for anything touching buffers
|
||
# PlatformIO. The venv below runs Python 3.15, which esptool REFUSES ("Python version must
|
||
# be 3.10..3.14") -- builds fine, dies at upload. Use uv's 3.14 for anything with -t upload:
|
||
uv run --python 3.14 --with platformio python -m platformio run -e bridge -e console
|
||
python3 tools/to_bootloader.py # 1200-baud touch → ROM download mode
|
||
uv run --python 3.14 --with platformio python -m platformio run -e bridge -t upload
|
||
```
|
||
|
||
Environment facts that will otherwise cost you a cycle:
|
||
|
||
- **`pio` on PATH is broken** here (`ModuleNotFoundError: platformio`), and
|
||
`/tmp/piovenv` is Python 3.15 so it can *build* but not *upload*. Use the uv invocation
|
||
above for both. If it is unavailable, find another interpreter with the package:
|
||
`find / -maxdepth 6 -name platformio -type d -path '*site-packages*'`.
|
||
- **`pio run -t upload` reports SUCCESS on a stale binary if the build before it failed.**
|
||
This happened: a diagnostic build failed to compile, the upload line right after it said
|
||
SUCCESS, and the board was running the previous firmware. Never chain build and upload
|
||
with `;` — check the build's own exit status first, or the verification you run next is
|
||
verifying something that is not in the tree.
|
||
- **No `g++` on the host.** `run_tests.sh` falls back to a local podman image listed in its
|
||
`images[]`. The page suite deliberately runs on the *host* so it works without node in there.
|
||
- **Build `console` too.** `ARDUINO_USB_MODE=1` compiles `KR_HAVE_HID == 0`, which is the only
|
||
thing that checks the no-HID `#else` branches.
|
||
- **The page is served from flash.** A change to `src/web_page.h` is invisible on the network
|
||
until the board is reflashed — and a tab that is already open keeps the old page in memory
|
||
until it is reloaded. Confirm with `curl -s http://keyrelay.local/ | wc -c` against the size
|
||
of the raw string in `src/web_page.h`, and grep the served bytes for the element you added.
|
||
- **This directory is a git repository** with a short history that predates most of the
|
||
current tree. Use it: `git diff` before and after any sweeping edit, and remember that
|
||
`git checkout <file>` reverts far more than you asked for — restore a *region*, not a
|
||
file.
|
||
- **Never do a multi-line `.*?` regex rewrite across a source file.** One did
|
||
`#if KR_DEBUG_LINK.*?#endif` with `DOTALL` to remove a diagnostic block and deleted ~500
|
||
lines of `main.cpp` — `onMessage()`, `setup()`, the console, the USB handlers — because
|
||
the earliest `#if` in the file was elsewhere. The compiler was the only thing that
|
||
noticed. Remove instrumentation with exact-string edits, or per-function slices, and
|
||
compile immediately after. When it happens anyway: the last good `firmware.elf` still
|
||
has the `.rodata` strings (console text, `printf` formats) and the symbol table, which is
|
||
enough to rebuild faithfully against the surviving headers.
|
||
|
||
## Verification standard
|
||
|
||
A feature that lives on the device is not done when the source builds — it is done when the
|
||
device is running it and something observed it. Concretely:
|
||
|
||
- Protocol/page claims → the host suites, plus `curl` against the served page.
|
||
- Keystroke delivery → `tools/watch_target.py` on the target (evdev, focus-independent); it is
|
||
the only thing that says what the target actually received, including *which* keys a chord
|
||
really was.
|
||
- Text runs → check the `typed:` summary and press count, not the scrolling stream.
|
||
|
||
When you write a check, name the mechanism it exercises. "Silence disarms" sat in the README's
|
||
safety list while nothing compared `lastInboundMs()` to `kNoClientTimeoutMs`; the documented
|
||
verification passed because killing the controller process closes the socket, and the
|
||
socket-close path disarms. The check proved a different property from the one it was named after
|
||
and nobody noticed, until a run that keeps typing on its own made the difference matter. It is
|
||
wired now — but ask, of every green check, *what would still pass if this were broken?*
|
||
|
||
## Editing rules
|
||
|
||
- **Pure modules** (`keystate.h`, `mouse.h`, `output.h`, `proto.h`, `text.h`, `ws_codec.h`,
|
||
`sha1.h`) include no Arduino or ESP-IDF headers. That is what lets the identical code run
|
||
on a host under sanitisers. Keep it.
|
||
- **`web_page.h`** is a C++ raw string: no compiler looks at the HTML or JS. Run
|
||
`tests/test_page.js` after touching it (it executes the page against a DOM shim and checks
|
||
that every element id the script reaches for exists in the markup).
|
||
- **A protocol change is four edits, minimum**: `proto.h` (parser + `format*` + its header
|
||
comment), `web_page.h`, `relayctl.py`, and the README Wire protocol table + error-code list.
|
||
Tests that pin the wire format are `test_proto.cpp` and `test_relayctl.py`.
|
||
- **Docs move with the code.** Update in the same change: README intro bullets and check count,
|
||
Safety model, Security model trust-boundary table, Controller page, Wire protocol,
|
||
Architecture file map, Tests table, Backlog, and the docstring at the top of whichever module
|
||
you changed. This README documents *why* decisions were made — keep the reasoning, don't just
|
||
restate the behaviour.
|
||
- Secrets: never in a file, a URL, a log or a shell history. The serial `token` command does not
|
||
echo it; keep it that way. `/status` takes the token as a query parameter because `curl` is
|
||
the point of that endpoint — do not add the token to any other URL.
|
||
- `TODO.md` holds **open items only**; clear it when they are done. The durable record is the
|
||
README: a fixed bug belongs in "Ten bugs this verification caught" with its root cause and
|
||
how it was proven, a new capability in the section that describes it, and a rule worth
|
||
repeating here. A TODO that has accumulated resolved items tells the next reader nothing about
|
||
what is left to do.
|
||
|
||
## Current defaults worth knowing
|
||
|
||
Default passphrase `changeme` (the page and `status` shout about it via `dflt:1`); AP
|
||
`keyrelay-ctl` / `keyrelay-ctl-pass`; `http://keyrelay.local` or `192.168.4.1`; paste pace
|
||
12 ms/character, 120 characters per frame, 2048 per run; silence disarm 8 s; NVS holds the
|
||
station credentials and passphrase and survives reflashing.
|