Skip to content

Commit e301980

Browse files
committed
docs: record the agent-runtime and verification anti-patterns
Adds three sections to CLAUDE.md per its own Documentation Updates rule: - Agent Runtime Rules: why the MCP SDK is pinned <2 (2.0.0 renamed CallToolResult.isError to is_error while hermes-agent 0.20.0 reads .isError), that hermes's circuit breaker blames a healthy server after three failures, that a running webui holds its SDK in memory, and that the hermes version was never the problem. - Verification Anti-Patterns: the checks that reported success over a dead agent — passing on an *attempted* tool, treating any long HTML as a working dashboard, testing import-presence instead of API compatibility, a mock with a hardcoded .isError that could not catch the SDK break, claiming an unreachable npm check passed, `set -e` + `read` aborting silently, and one catch making an unreachable server look like an empty account. - Data Model Rules: mcp_tokens.user_id is a non-null FK with no cascade, and why the @transactional belongs on the repository rather than a self-invoked caller.
1 parent 2cf696b commit e301980

1 file changed

Lines changed: 59 additions & 0 deletions

File tree

CLAUDE.md

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,65 @@ returns a number).
178178
- Do NOT commit automatically — wait for explicit user instruction.
179179
- Conventional commits: `feat:`, `fix:`, `refactor:`, `docs:`, `test:`, `chore:`, `perf:`, `ci:`
180180

181+
### Agent Runtime Rules (learned the hard way, 2026-08-07)
182+
183+
1. **Pin the Python MCP SDK below 2.0.** `scripts/self-host/setup-agent.sh` installs
184+
it as `mcp>=1.0,<2`. SDK **2.0.0 renamed `CallToolResult.isError` to `is_error`**
185+
and split the models into a separate `mcp-types` package, while `hermes-agent`
186+
0.20.0 still reads `result.isError` (`tools/mcp_tool.py:5222`). With the old
187+
unbounded `mcp>=1.0`, every DeepSQL tool call raised
188+
`AttributeError: 'CallToolResult' object has no attribute 'isError'` — a time bomb
189+
that detonated the day 2.0.0 shipped, with no code change on our side. Raise the
190+
ceiling only once hermes reads `is_error`.
191+
2. **Three tool failures trip hermes's circuit breaker**, after which the rest are
192+
refused as `MCP server 'deepsql' is unreachable` — blaming a healthy server for a
193+
client-side parse error. Do not trust that message; find the *first* failure in
194+
`~/.hermes/logs/errors.log`.
195+
3. **A running webui holds its SDK in memory.** After changing the SDK it must be
196+
restarted; `setup-agent.sh` now does that itself. It previously printed
197+
`✓ Hermes webui already running` and left the broken SDK loaded, so re-running the
198+
repair script gave a full column of ticks and no change.
199+
4. **The agent version was never the problem.** `agent/distribution.yaml` once pinned
200+
`hermes_requires <0.20.0` on a "verified" 401 that came from a hand-rolled
201+
`hermes serve` run rather than `hermes webui`. 0.20.0 works. Verify against the
202+
real start path before writing a version constraint.
203+
204+
### Verification Anti-Patterns (do not repeat)
205+
206+
These all reported success over broken systems — which is how the agent shipped
207+
broken. Assert the *outcome*, never the attempt:
208+
209+
- **`e2e-agent-check.py`** passed on `any("execute_sql" in t for t in tools)` — a tool
210+
being *attempted*. It printed `✓ All agent UI paths OK` and exited 0 while the
211+
agent's own reply said "I'm blocked". It now requires the answer itself.
212+
- **A dashboard that is "HTML and long"** proves nothing: with every tool failing, the
213+
agent emitted a plausible artifact full of invented numbers. A real one calls
214+
`deepsql.query()`; absence of that call means the data never came from the database.
215+
- **Presence ≠ compatibility.** The SDK check tested only that `mcp` imports, so it
216+
printed `✓ Python MCP SDK available` on an SDK whose every call failed. It now
217+
asserts `CallToolResult` still carries `isError`.
218+
- **Mocks hide SDK breaks.** `tests/tools/test_mcp_structured_content.py` uses a
219+
`_FakeCallToolResult` with a hardcoded `.isError`, so it kept passing precisely when
220+
the real SDK stopped matching. Pin the dependency; a fake cannot catch this.
221+
- **Never claim a check you did not run.** `install.sh` reported "up to date" when it
222+
could not reach npm; it now says it could not check.
223+
- **`set -e` + `read` at EOF aborts silently.** Prompts in `install.sh` use
224+
`read … || true` so the explicit emptiness checks report the problem. Without it the
225+
installer exited 1 with no message, after writing generated secrets to `.env`.
226+
- **Silent-failure rule, concretely:** the CLI rendered an unreachable server as
227+
`No databases connected yet` because one `catch` covered both the connection fetch
228+
and decorative extras. An unreachable host must never look like an empty account.
229+
230+
### Data Model Rules
231+
232+
- **`mcp_tokens.user_id` is a non-null FK with no cascade.** Deleting a user who holds
233+
a token throws `ConstraintViolationException`. `UserController` clears the user's
234+
tokens first via `McpTokenRepository.deleteByUserId`, which carries its own
235+
`@Transactional` — a derived delete needs one, and annotating a self-invoked caller
236+
does nothing (Spring proxies are bypassed by `this::`). This broke
237+
`POST /users/admin/reset` on every install that had run `setup-agent.sh`, since that
238+
mints an admin MCP token on each run.
239+
181240
### MCP & CLI Release Rules
182241

183242
**Whenever you add, rename, or remove an MCP tool or a CLI subcommand, you MUST update all of these in the same commit — they are agent-facing surfaces and drift silently breaks discoverability:**

0 commit comments

Comments
 (0)