This repository was archived by the owner on Jul 4, 2026. It is now read-only.
feat(webapi): bearer auth, typed schemas, cron CRUD, SSE interrupt, tool_call_id - #1
Closed
kingsleydon wants to merge 25 commits into
Closed
feat(webapi): bearer auth, typed schemas, cron CRUD, SSE interrupt, tool_call_id#1kingsleydon wants to merge 25 commits into
kingsleydon wants to merge 25 commits into
Conversation
Adds an optional authentication layer to the webapi module using a single shared bearer token configured via the HERMES_API_TOKEN environment variable: - When HERMES_API_TOKEN is unset or empty, all routes are open (the existing behavior, suitable for localhost dev). - When HERMES_API_TOKEN is set, every route except /health requires an Authorization: Bearer <token> header matching the configured value (compared with hmac.compare_digest to avoid timing leaks). - /health is intentionally left unauthenticated so load balancers and uptime probes can reach it without credentials. This unblocks deployments where Hermes is exposed to the public internet behind a per-instance HTTPS endpoint with no reverse proxy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sections
Before: PATCH /api/config only accepted model, provider, and base_url.
After: PATCH /api/config accepts any of the known config.yaml sections:
Messaging platforms: discord, telegram, slack, whatsapp, matrix,
mattermost, signal, sms, email, feishu, dingtalk, wecom, bluebubbles,
homeassistant, webhook
Subsystems: security, memory, cron, display, toolsets, mcp
Each section is deep-merged into the existing config so clients only need
to send the fields they want to change. Setting a nested value to null
deletes that key, setting base_url to an empty string removes the top-level
override (backwards compatible with the old shortcut behavior).
This unblocks dashboard-style UIs for channel pairing (Telegram/Discord/
Slack/etc.), security settings, memory toggles, and toolset management
without requiring a new webapi release for every new field upstream adds.
The merge is opaque to the server — each section is typed as
dict[str, Any] because the exact schema lives in the messaging platform
adapters and may change with Hermes releases. See
website/docs/user-guide/configuration.md for the per-platform shapes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ports the aiohttp cron handlers from gateway/platforms/api_server.py
(lines 1062-1260) to FastAPI, exposing 8 new endpoints:
GET /api/jobs list all jobs
POST /api/jobs create a new job
GET /api/jobs/{job_id} get a single job
PATCH /api/jobs/{job_id} update allowed fields
DELETE /api/jobs/{job_id} remove a job
POST /api/jobs/{job_id}/pause pause a running job
POST /api/jobs/{job_id}/resume resume a paused job
POST /api/jobs/{job_id}/run trigger immediate execution
The underlying storage and scheduler live in cron/jobs.py — these routes
are a thin HTTP layer over those functions with input validation, length
limits (name ≤ 200, prompt ≤ 5000), and job ID format enforcement (12-char
hex).
If the cron module cannot be imported (optional install), every route
returns 501 via _check_available() — this mirrors the behavior of the
aiohttp version so clients can probe capability by hitting GET /api/jobs
on first connection.
Pydantic models for request/response are in webapi/models/jobs.py.
The job shape itself is opaque (dict[str, Any]) because the authoritative
structure lives in cron/jobs.py and may evolve with Hermes releases.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Hermes stores MCP server definitions under the top-level `mcp_servers`
key, not `mcp`. Verified at hermes_cli/mcp_config.py:8 and line 81
where `_get_mcp_servers()` reads `config.get("mcp_servers")`.
The previous commit used `mcp` which would have silently created an
unused top-level key in config.yaml that Hermes ignores. This fix
aligns the webapi ConfigPatch shape with the on-disk schema Hermes
actually reads.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Port of the upstream agent.interrupt() pattern (gateway/platforms/
api_server.py:1080-1100) into the webapi chat stream handler.
Before: the chat/stream endpoint spawned a daemon thread that ran
agent.run_conversation() to completion, pushing frames into an
SSEStream queue. If the client closed the SSE reader mid-stream,
the thread kept running — burning model tokens for an agent whose
output no one was reading. This was the single most-reported
limitation of the webapi and documented as unfixable in the Clawdi
dashboard Stop-button warning banner.
After: two coordinated pieces make Stop actually interrupt the agent.
1. webapi/sse.py — SSEStream.aiter()
New async iteration method that yields frames via
asyncio.to_thread(queue.get) so the blocking get() runs in the
default thread pool without holding up the event loop. Unlike
the sync __iter__ (kept for backwards compatibility), the async
form propagates GeneratorExit when StreamingResponse closes the
iterator on client disconnect.
2. webapi/routes/chat.py — wrapped event_stream() generator
- The route handler now maintains an agent_ref: list[AIAgent | None]
shared with the worker thread. The worker sets agent_ref[0] right
after create_agent() so the async generator can reach it.
- A worker_done threading.Event flag tracks whether the thread has
finished naturally so the interrupt path can distinguish between
"client closed after normal completion" and "client disconnected
mid-run".
- event_stream() is a new async generator that awaits stream.aiter()
and yields bytes. On GeneratorExit it calls _interrupt_agent() on
the current agent reference before re-raising. A final safety net
in the `finally` block interrupts if the generator exits for any
other reason while the worker is still mid-run (e.g. uvicorn
shutdown).
- StreamingResponse now wraps this async generator instead of the
raw SSEStream, so Starlette's built-in client-disconnect handling
engages and propagates to our cancel path.
- _interrupt_agent() helper catches exceptions from agent.interrupt()
so a broken interrupt path never poisons the response teardown.
This removes the "Stop button only halts display" limitation from
the Clawdi Hermes dashboard mode. The amber warning banner in
chats-content.tsx can be retired once this commit ships to the fork
Clawdi deploys.
Verified via smoke test: webapi.sse.SSEStream now exposes an aiter()
method, webapi.routes.chat exports _interrupt_agent as a module-level
callable that safely handles None and mock agents with interrupt(reason)
methods.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously the route returned the entire session transcript on every
call, with no way to limit or offset. Long sessions (10k+ messages)
would serialize a huge JSON response and freeze the client's chat UI
on first render.
This commit adds three optional query params:
limit=N Max messages to return (0=all, default 0 for legacy).
Capped at 1000 to prevent accidental DoS.
offset=N Skip this many messages from the BEGINNING (default 0).
tail=true When set, return the last `limit` messages instead of
the first — equivalent to offset=(total-limit). This is
what chat UIs want on first render: "show me the most
recent N".
`total` in the response body is always the full session message count
regardless of pagination, so clients can render "X of N" indicators
and know when they've walked back to the head.
Default behavior (no params) is unchanged — the whole transcript is
returned in chronological order. Existing callers are unaffected.
The SessionDB.get_messages() loads everything anyway (the fork does
not expose a SQL LIMIT path), so this pagination is currently
application-level slicing. A follow-up could push limit/offset down
to the SQL layer for large sessions, but for now even 10k messages
is cheap to load and the expensive work is JSON serialization + TCP
transmission — which this patch fixes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
19 pytest tests covering every route the dashboard-support branch
has touched, executed against the real FastAPI app via Starlette's
TestClient.
tests/webapi/conftest.py installs in-memory fakes for the hermes
runtime modules that webapi depends on (hermes_state.SessionDB,
run_agent.AIAgent, tools.memory_tool.MemoryStore, cron.jobs,
gateway.run, tools.skills_tool, hermes_cli.config). Stubs run at
conftest import time so webapi.app can be imported without the real
hermes core (which needs the openai package, tools registry, etc).
An autouse fixture re-installs stubs and clears in-memory session
stores before each test so they stay independent.
tests/webapi/test_smoke.py covers:
Public routes:
- GET /health (unauthenticated, even when auth is enabled)
Read routes (auth disabled path):
- GET /v1/models, /api/sessions, /api/memory, /api/config,
/api/skills, /api/skills/categories, /api/jobs
Config PATCH (commit e4a9766) — nested platform sections:
- telegram alone
- mcp_servers alone (covers commit 9ec016e rename fix)
- multiple sections at once (telegram + discord + security)
Session lifecycle + pagination (commit 2432dc0):
- POST /api/sessions and GET /api/sessions/{id}
- GET /api/sessions/{id}/messages legacy (no params)
- GET /api/sessions/{id}/messages?limit=5
- GET /api/sessions/{id}/messages?limit=5&tail=true
- GET /api/sessions/{id}/messages?offset=10
- GET /api/sessions/search?q=... (FTS5 full-text search)
- POST /api/sessions/{id}/fork + verifies `forked_from` field
Cron jobs (commit 2e2d915):
- POST /api/jobs with valid body
- POST /api/jobs missing required field → 422 (pydantic)
- POST /api/jobs with whitespace-only name → 400 (our check)
Memory CRUD:
- POST /api/memory
- PATCH /api/memory
- DELETE /api/memory with JSON body (non-standard REST — required
by webapi/routes/memory.py:74)
Auth middleware (commit 934b2dd):
- Token set + no Authorization header → 401
- Token set + wrong bearer → 401
- Token set + missing "Bearer" scheme → 401
- Token set + correct bearer → 200
- /health is always public even with auth enabled
Run:
.venv/bin/python -m pytest -o addopts= tests/webapi/test_smoke.py -v
All 19 tests pass in ~1.2s against the current branch state.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extends docker/entrypoint.sh to support three new first-argument modes
in addition to the existing `hermes` subcommand pass-through:
webapi Run ONLY the FastAPI webapi module. Headless API-only,
no messenger platforms. `python3 -m webapi "$@"`.
dashboard Run BOTH webapi AND gateway simultaneously. Gateway
starts in the background; webapi runs in the foreground.
A trap kills the gateway child on EXIT/TERM/INT so the
container exits cleanly when uvicorn dies. This is the
mode Clawdi-managed k8s / Phala CVM deployments should
use — the frontend needs webapi routes for the dashboard
while the agent still services telegram / slack / discord
configured via config.yaml.
gateway / (or any other first arg) Passed through to `hermes`.
chat / Preserves upstream behavior for every existing docker
etc. entrypoint that calls the image with a specific subcommand.
Closes a recently-discovered integration gap: Clawdi's pod template
previously set args: ["gateway"], which runs the messenger gateway
but NOT the webapi, so every dashboard-support patch in this branch
(auth middleware, cron routes, config PATCH expansion, message
pagination, stop-on-disconnect) was dormant in production deployments.
With `dashboard` mode in place, the Clawdi pod template can switch to
args: ["dashboard"] to activate both. That pod-template update is a
separate commit in the Clawdi repo.
The pod template image also needs to change from the upstream
nousresearch/hermes-agent:latest to a Clawdi-built image of this
fork — tracked separately; this commit is just the entrypoint
support that unblocks it.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two connected fixes that let the dashboard backend build in CI and
produce a complete OpenAPI schema for downstream typed-client
generation on the Clawdi side.
1. pyproject: add a `webapi` optional-dep group carrying fastapi,
uvicorn[standard] and python-multipart, and fold it into `[all]`
so CI's `uv pip install -e .[all,dev]` picks it up. Before this
the tests job died with `ModuleNotFoundError: No module named
'fastapi'` because the webapi/ module had been added without
declaring its runtime deps.
2. Give every route an explicit `response_model=...` so FastAPI
emits a concrete schema for `/openapi.json` instead of `object`.
Previously 8 endpoints (GET /health, GET /v1/models,
GET /api/available-models, PATCH /api/config, GET /api/skills,
GET /api/skills/categories, GET /api/skills/{name},
DELETE /api/sessions/{id}) returned `dict` so clients had to
cast through `unknown`. New models in webapi/models/{health,
models}.py + expanded webapi/models/{common,config,memory,
sessions,skills}.py cover the lot. Also tightens
SearchSessionsResponse.results (previously
`list[dict[str, Any]]`) to a typed `SearchSessionMatch` that
mirrors the SQL projection in
`hermes_state.SessionDB.search_messages`, and
MemoryReadResponse.targets (previously `list[dict]`) to
`list[MemoryTarget]`.
CI: split tests/webapi/ into its own workflow job. The webapi
conftest installs stub modules at `sys.modules["tools"]`,
`sys.modules["hermes_state"]`, etc. so webapi.app can import
without the full agent stack; that stub-install turns `tools`
into a non-package and breaks tests/tools/* imports when they
share a pytest process. Running webapi in a dedicated job keeps
the stubs hermetic and lets the main suite continue to use the
real modules.
Also fixes the webapi test fakes for searches + skill categories
so they match the now-typed response shapes.
All 19 webapi smoke tests still pass locally.
Three upstream changes so Clawdi's TS client can consume the webapi
without hand-written DTOs or ordinal-only tool matching.
1. tool.started now carries tool_call_id
- `create_agent` gains a `tool_start_callback` parameter plumbed
straight through to `AIAgent`.
- chat.py registers a start callback that emits `tool.started` with
the assistant-provided `tool_call_id`, and the existing progress
callback now only emits `tool.pending` for real tools (the
`_thinking` pseudo-tool still goes through `tool.progress`).
- Clients can now match started/completed pairs by id instead of
relying on ordinal cursors — correct for two concurrent same-name
tool invocations in the same assistant turn.
2. JobResponse.job is now a typed JobModel
- Previously the schema exposed `dict[str, Any]`, which forced
every TS client to either ship hand-written interfaces
(duplicating `HermesJob*` types) or cast through `unknown`.
- New `JobModel`/`JobSchedule`/`JobRepeat` models mirror the dict
literal in `cron.jobs.create_job` (every field except `id` is
optional so legacy jobs on disk with missing fields still
deserialize). `JobsListResponse.jobs` and `JobResponse.job` now
point at `JobModel`.
3. JobCreateRequest.prompt/deliver + ChatRequest.skip_* use Optional
- Pydantic marks fields with non-None defaults as required-with-
default in the emitted JSON schema, which openapi-typescript then
surfaces as required on the wire. Switching to `str | None = None`
and `bool | None = None` lets clients omit them; the routes
coerce with `bool(...)` / `... or ""` at the call sites.
tests/webapi: all 19 smoke tests still pass.
kingsleydon
force-pushed
the
feat/webapi-dashboard-support
branch
from
April 11, 2026 01:05
63a6877 to
74b20a1
Compare
kingsleydon
changed the title
Follow-up patch on top of the 10-commit webapi improvements branch,
addressing issues found during dual-track review with Codex +
Claude Code after the first push.
1. fix(webapi): classify tool success via structured JSON, not substring
`_emit_post_run_events` used `"error" in lower or "failed" in lower`
to pick between `tool.completed` and `tool.failed`, which mis-
classified every legitimate tool result that contained either
word — grep hits, test summaries like "0 failed", search output
referencing error logs, git diffs of old tracebacks. Replace with
a new `_tool_result_failed` helper that only marks failed when
the content is a JSON object with explicit `success: false`.
Plain-text results (the common case) are always success.
2. fix(webapi): invalid CORS config
`allow_credentials=True` + `allow_headers=["*"]` is rejected by
browsers on preflight per the Fetch spec. webapi uses a bearer
header (no cookies) so credentials aren't needed. Set
`allow_credentials=False` and enumerate both headers and methods
explicitly.
3. feat(hermes_state): SQL LIMIT/OFFSET pagination for get_messages
The previous pagination implementation loaded the full session
history then sliced in Python, which defeated the point of the
route's `limit`/`offset`/`tail` query params for sessions with
thousands of messages. Add `SessionDB.count_messages` (cheap
COUNT(*)) and `SessionDB.get_messages_page(limit, offset, tail)`
that pushes down to SQLite via `ORDER BY timestamp, id LIMIT ?
OFFSET ?`, with `tail=True` implemented as `ORDER BY DESC LIMIT ?`
then reversed. Route handler uses the new methods when
`limit > 0` and falls back to the unbounded `get_messages` only
for `limit=0` (legacy full-transcript reads).
4. feat(webapi): complete JobUpdateRequest
`JobCreateRequest` accepted `model`, `provider`, `base_url`,
`script` but `JobUpdateRequest` did not, so those fields could
not be edited after create. Mirror the full set (minus `origin`,
which is creation-only).
5. fix(webapi): sanitize error messages in jobs route
Seven `raise HTTPException(500, detail=str(exc))` sites in
`webapi/routes/jobs.py` echoed raw exception strings to the
client — filesystem paths, SQL errors, stack-trace fragments,
tempfile names. New `_internal_error(operation, exc)` helper
logs the full exception via `logger.exception` and raises a
generic `Internal error during <operation>` 500 for the client.
6. test(webapi): cover every review finding
- Auth 401 on every protected route (7 routes) — guard against
a future router being added without the Depends annotation
- `_tool_result_failed` unit tests covering the false-positive
cases (grep output, test summaries, plain text) plus the
true-positive case (JSON with explicit success:false)
- Full cron job lifecycle: create → pause → resume → run →
patch (with the new model/provider/base_url/script fields) →
delete, all via the route handlers
- Invalid job IDs rejected before hitting the cron subsystem
- Pagination bounds: limit=0 legacy path, limit=1, tail=true,
offset without limit, limit=9999 → 422
- Drift guard: `JobCreateRequest` vs `JobUpdateRequest` field
parity — fails loudly if a future create-field is added
without mirroring it to update
Test suite goes from 19 → 26 cases; `tests/webapi/conftest.py`
grows a backing in-memory jobs store so pause/resume/run/delete
can round-trip against the fake cron.jobs module instead of
returning None.
All 26 webapi smoke tests pass locally.
…edule, race) Round-2 dual review (Claude Code Plan + Codex) on top of f51f880 found more bugs. This commit addresses the ship-blockers. 1. Expand _tool_result_failed to match {"error": "..."} without success field. Codex audited every tool in `tools/*.py` and found the failure envelope is inconsistent: - memory_tool / skills_tool / parts of web_tools return the full {"success": false, "error": ...} envelope - delegate_tool / file_tools / tools/registry.py's exception wrapper / many web_tools branches return plain {"error": "..."} WITHOUT a success field The first pass only caught the explicit success:false shape, so real failures from the second group rendered as green in the UI. New contract: if content is a JSON object and either has success:false OR has a non-empty `error` key without success:true, it's a failure; otherwise success. Plain text is still always success (avoids the original substring-match false positive). 2. Fix PATCH /api/jobs schedule string crash. cron.jobs.update_job expects updates["schedule"] to already be a parsed dict (its merge path calls .get("display", ...) on it). Previously JobCreateRequest's wire-level `schedule: str` was only parsed via cron.jobs.create_job → parse_schedule; JobUpdateRequest went straight through and would crash with AttributeError. Parse the string in webapi before passing the merge dict down, surfacing ValueError as 400. 3. Test cases added: - Full _tool_result_failed classification matrix covering the success envelope, the {error} shape, the {error: null} no-op case, plain-text output with "error" in it, non-string content, and malformed JSON (27 tests now, up from 26). - Schedule PATCH regression that previously crashed with 500 + AttributeError. - conftest fake gains a `parse_schedule` stub so the route can exercise the parse path end-to-end. All 27 webapi smoke tests pass locally.
Round-3 dual review surfaced three remaining issues + several
simplification opportunities. This commit addresses them.
1. Fix agent_ref race on early SSE disconnect.
If the browser closed the tab during `create_agent()` (which can
take seconds on cold cache paths) the worker thread's
`agent_ref[0]` was still None when the generator's GeneratorExit
handler ran, so `_interrupt_agent` was a no-op. The worker then
went on to start `run_conversation` and burn tokens the user had
already given up on.
Replace the single `agent_ref: list[AIAgent | None]` pointer
with a pair of `cancelled: threading.Event` + `agent_ref`. The
generator's disconnect handler sets `cancelled` BEFORE calling
`_interrupt_agent`; the worker checks `cancelled` after
`create_agent` returns and bails before running the turn (or
interrupts the freshly-created agent if the race is tight).
Closes the window completely.
2. Kill dead code around worker_done / defensive `_interrupt_agent`.
`worker_done: threading.Event()` was only used to gate the
generator's `finally` block, which unconditionally interrupted
a (by then dead) agent anyway. Replaced by the `cancelled`
flag used by the GeneratorExit path — simpler, fewer moving
parts. `_interrupt_agent`'s `try/except Exception` wrapped
`agent.interrupt()` which does not raise (see `run_agent.py`:
it just sets a boolean flag). Remove the catch and the now-
unused `logging` import.
3. Trim the CORS default origin list in `webapi/app.py`.
The old list generated `http://{localhost,127.0.0.1}:{3000..3010}`
— 22 entries to cover hypothetical framework defaults that
nobody actually uses. In production webapi is only reached via
the agent-image controller on `127.0.0.1:19000` (internal) or
through the controller's `/_hermes/*` proxy (which enforces its
own CORS); the default list only unblocks local dev against
`bun run dev` on port 3000. Shrink to just
`(http://localhost:3000, http://127.0.0.1:3000)`. Anything else
should be set explicitly via `HERMES_CORS_ORIGINS`.
All 27 webapi smoke tests still pass.
kingsleydon
added a commit
that referenced
this pull request
Apr 11, 2026
Addresses BLOCKER + SHOULD-FIX findings from rounds 4, 5, and 6 of the multi-round review on PR #1. BLOCKER — event-loop blocking sync IO Multiple async route handlers were calling synchronous SQLite/file/ HTTP code directly. ``hermes_cli.models.list_available_providers`` in particular fires ``urllib.request.urlopen`` against external provider catalogs with multi-second timeouts — one slow provider could pin the entire FastAPI worker. Wrap every blocking call in ``starlette.concurrency.run_in_threadpool``: - routes/models.py: provider catalog discovery - routes/sessions.py: every SessionDB call (search/list/get/messages/ patch/delete/fork) — fork bundles its O(n) message-copy loop into one threadpool hop so the operation stays atomic - routes/jobs.py: cron storage (jobs.json open + json.load + os.replace) - routes/memory.py: MemoryStore add/replace/remove (file locks + fsync) - routes/config.py: load_config / save_config YAML disk IO - routes/skills.py: skills_tool walks the skill markdown tree BLOCKER — exception text leak Multiple paths reflected raw ``str(exc)`` to the browser, which routinely leaks API keys (in 401 bodies), file system paths, SQL fragments, and stack-trace fragments. Sanitize all of them: - errors.py: global Exception handler returns "Internal server error" - routes/chat.py: both non-streaming and streaming paths now log the real exception server-side and emit a stable opaque message - routes/config.py: PATCH handler returns "Failed to update config" instead of echoing OSError text SHOULD-FIX — bounded SSE buffer webapi/sse.py used an unbounded ``queue.Queue``: a slow / disconnected client during a long agent run could grow per-stream memory until the worker thread finished. Add a 1024-frame cap with put_nowait drop semantics; the worker keeps running so the run completes server-side. Switch to a poll-based close mechanism (no sentinel through the bounded queue) to avoid deadlocking close() when the buffer is saturated. SHOULD-FIX — OpenAPI schema reflects SSE response /api/sessions/{id}/chat/stream now declares ``text/event-stream`` in its responses table so generated clients (the Clawdi side uses openapi-typescript) don't try to JSON-parse a streaming response. NICE-TO-HAVE — port default unification webapi/__main__.py used to fall back to 8642 if free, otherwise 8643. The Clawdi controller hard-codes 8643. Single source of truth on 8643 — eliminates a class of "controller probes wrong port in local dev" bugs. Tests: extends tests/webapi/test_smoke.py from 27 to 33 tests: - test_unhandled_exception_handler_does_not_leak_raw_message - test_config_patch_failure_does_not_leak_filesystem_path - test_chat_failure_does_not_leak_provider_error - test_sse_stream_drops_frames_when_buffer_full - test_chat_stream_route_advertises_sse_in_openapi - test_main_default_port_matches_controller All 33 webapi tests pass with ``pytest -o addopts= tests/webapi``. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Addresses BLOCKER + SHOULD-FIX findings from rounds 4, 5, and 6 of the multi-round review on PR #1. BLOCKER — event-loop blocking sync IO Multiple async route handlers were calling synchronous SQLite/file/ HTTP code directly. ``hermes_cli.models.list_available_providers`` in particular fires ``urllib.request.urlopen`` against external provider catalogs with multi-second timeouts — one slow provider could pin the entire FastAPI worker. Wrap every blocking call in ``starlette.concurrency.run_in_threadpool``: - routes/models.py: provider catalog discovery - routes/sessions.py: every SessionDB call (search/list/get/messages/ patch/delete/fork) — fork bundles its O(n) message-copy loop into one threadpool hop so the operation stays atomic - routes/jobs.py: cron storage (jobs.json open + json.load + os.replace) - routes/memory.py: MemoryStore add/replace/remove (file locks + fsync) - routes/config.py: load_config / save_config YAML disk IO - routes/skills.py: skills_tool walks the skill markdown tree BLOCKER — exception text leak Multiple paths reflected raw ``str(exc)`` to the browser, which routinely leaks API keys (in 401 bodies), file system paths, SQL fragments, and stack-trace fragments. Sanitize all of them: - errors.py: global Exception handler returns "Internal server error" - routes/chat.py: both non-streaming and streaming paths now log the real exception server-side and emit a stable opaque message - routes/config.py: PATCH handler returns "Failed to update config" instead of echoing OSError text SHOULD-FIX — bounded SSE buffer webapi/sse.py used an unbounded ``queue.Queue``: a slow / disconnected client during a long agent run could grow per-stream memory until the worker thread finished. Add a 1024-frame cap with put_nowait drop semantics; the worker keeps running so the run completes server-side. Switch to a poll-based close mechanism (no sentinel through the bounded queue) to avoid deadlocking close() when the buffer is saturated. SHOULD-FIX — OpenAPI schema reflects SSE response /api/sessions/{id}/chat/stream now declares ``text/event-stream`` in its responses table so generated clients (the Clawdi side uses openapi-typescript) don't try to JSON-parse a streaming response. NICE-TO-HAVE — port default unification webapi/__main__.py used to fall back to 8642 if free, otherwise 8643. The Clawdi controller hard-codes 8643. Single source of truth on 8643 — eliminates a class of "controller probes wrong port in local dev" bugs. Tests: extends tests/webapi/test_smoke.py from 27 to 33 tests: - test_unhandled_exception_handler_does_not_leak_raw_message - test_config_patch_failure_does_not_leak_filesystem_path - test_chat_failure_does_not_leak_provider_error - test_sse_stream_drops_frames_when_buffer_full - test_chat_stream_route_advertises_sse_in_openapi - test_main_default_port_matches_controller All 33 webapi tests pass with ``pytest -o addopts= tests/webapi``. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
kingsleydon
force-pushed
the
feat/webapi-dashboard-support
branch
from
April 11, 2026 05:40
bf0aad7 to
4b53f5a
Compare
|
…atus Round 6 wrapped ``SessionDB`` calls in ``run_in_threadpool``, which inadvertently let ``ValueError`` from ``set_session_title`` escape to the global 500 handler instead of being translated to a 4xx. ``set_session_title`` raises ``ValueError`` on title collisions (unique-title constraint). Before round 6, ``create_session`` and ``fork_session`` weren't bundled into a single threadpool hop, but the catch was still missing — they just happened not to hit it in practice. After the round-6 refactor that bundled them for atomicity, the failure mode is now real and silent: * create_session: collision surfaces as 500 "Internal server error" * fork_session: concurrent-fork collision surfaces as 500 Both are now caught at the route boundary: * create → 400 (caller's own duplicate title) * fork → 409 (concurrent conflict, client can retry) Adds two regression tests that monkey-patch ``_create_session_sync`` and ``_fork_session_sync`` to raise the collision ValueError and assert the translated status + opaque message. Found by round-7 codex review. 35/35 webapi tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Round 7 mapped fork_session title collisions to HTTP 409 so the client could retry. Round-8 review (codex) pointed out that 409 is semantically wrong: the title is SERVER-generated via ``get_next_title_in_lineage``, and the client has no parameter they can change on retry. The right shape is "server retries transparently, surfaces 500 only on exhaustion". ``get_next_title_in_lineage`` reads the current max lineage suffix under a brief lock and releases before the caller commits with ``set_session_title``. Two concurrent forks of the same parent therefore pick identical titles — the second ``set`` hits the unique-title constraint and raises ``ValueError``. This commit moves the retry into ``_fork_session_sync``: * Session row is created ONCE up front (no title yet). * Inner loop tries ``get_next_title_in_lineage`` + ``set_session_title`` up to 5 times. * On success, continues to the message-copy phase. * On exhaustion, raises ``RuntimeError`` — propagates to the global 500 handler with an opaque message. The orphaned session row (no title) is deliberately left for operator inspection; deleting it silently would hide the symptom. The route handler no longer catches ValueError at all — the retry loop owns that failure mode now. Two new regression tests replace the old 409-expectant one: * ``test_fork_session_title_collision_retries_successfully`` monkey-patches the fake SessionDB's ``set_session_title`` to fail on the 1st call and succeed afterward; asserts 200 + that the retry fired. * ``test_fork_session_title_exhausted_retries_returns_500`` makes every ``set_session_title`` raise; asserts 500 with the opaque "Internal server error" body (no raw collision text leaked). 36/36 webapi tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
kingsleydon
changed the title
Hermes reads platform credentials (bot tokens, API keys, allowed user
lists) from ~/.hermes/.env, not from config.yaml. When a dashboard
sends `{telegram: {bot_token: "xxx"}}` via PATCH /api/config, the
previous implementation wrote it to config.yaml where the gateway
never reads it.
Now _apply_config_patch splits each platform section into:
- Credential fields → save_env_value() + os.environ update
- Behaviour fields (require_mention, etc.) → deep-merge into config.yaml
GET /api/config also injects credential status (configured: true/false)
into the response so dashboards can show the right state without
exposing actual secrets.
Credential mapping:
- telegram: bot_token → TELEGRAM_BOT_TOKEN, allowed_usernames → TELEGRAM_ALLOWED_USERS
- discord: bot_token → DISCORD_BOT_TOKEN
- slack: bot_token → SLACK_BOT_TOKEN, app_token → SLACK_APP_TOKEN
- feishu: app_id → FEISHU_APP_ID, app_secret → FEISHU_APP_SECRET
- dingtalk: client_id → DINGTALK_CLIENT_ID, client_secret → DINGTALK_CLIENT_SECRET
- etc.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Fixes 4 issues from Codex review: 1. Complete _CREDENTIAL_ENV_MAP for ALL platforms: add mattermost, signal, email, sms, wecom, homeassistant, webhook. Add missing fields for existing platforms (matrix password/device_id, feishu encrypt_key/verification_token). 2. Drop `enabled` from platform patches — gateway determines platform enablement solely by whether the credential env var is set. Writing enabled to config.yaml is dead config. 3. Fix GET credential injection: always overwrite with boolean status from .env (even if old config.yaml has plaintext credentials) to prevent secret leakage. 4. Fix transaction order: save config.yaml first (less likely to fail), then .env writes. Partial success is better than rolling back a valid YAML write. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
Credential-only patches (bot_token, allowed_usernames, etc.) only write to .env — no config.yaml changes. Previously save_config was called unconditionally, triggering _normalize_root_model_keys + atomic_yaml_write even when the config dict was unchanged. This caused 500 errors on CVMs where save_config's normalize pass or file system operations failed on the unchanged config state. Now tracks yaml_dirty flag and only calls save_config when there are actual YAML changes (model/provider/base_url or behaviour fields). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
|
…traceback
config["model"] can be a dict after _normalize_root_model_keys migrates
root-level provider/base_url into {"default": "gpt-5.4", "provider": ...}.
ConfigPatchResponse.model expects str|None, so extract the default name.
Also reverts the temporary debug traceback exposure.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
kingsleydon
force-pushed
the
feat/webapi-dashboard-support
branch
from
April 14, 2026 02:48
056c69d to
c6c85ba
Compare
|
_resolve_turn_agent_config built the primary runtime dict without default_headers, so the x-api-key header from config.yaml's model.headers was dropped. This caused 401 INVALID_API_KEY on Telegram/Discord messages while webapi chat (which spreads runtime_kwargs directly) worked fine. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Author
|
Superseded by #2 (feat/clawdi-integration based on upstream main, replaces fork webapi with official web_server.py + api-server) |
h4x3rotab
pushed a commit
that referenced
this pull request
Apr 20, 2026
…ts (NousResearch#11745) Move moonshotai/kimi-k2.5 to position #1 in every model picker list: - OPENROUTER_MODELS (with 'recommended' tag) - _PROVIDER_MODELS: nous, kimi-coding, opencode-zen, opencode-go, alibaba, huggingface - _model_flow_kimi() Coding Plan model list in main.py kimi-coding-cn and moonshot lists already had kimi-k2.5 first.
kingsleydon
pushed a commit
that referenced
this pull request
Apr 23, 2026
When the live Vercel AI Gateway catalog exposes a Moonshot model with zero input AND output pricing, it's promoted to position #1 as the recommended default — even if the exact ID isn't in the curated AI_GATEWAY_MODELS list. This enables dynamic discovery of new free Moonshot variants without requiring a PR to update curation. Paid Moonshot models are unaffected; falls back to the normal curated recommended tag when no free Moonshot is live.
kingsleydon
pushed a commit
that referenced
this pull request
Apr 23, 2026
…#13354) Classic-CLI /steer typed during an active agent run was queued through self._pending_input alongside ordinary user input. process_loop, which drains that queue, is blocked inside self.chat() for the entire run, so the queued command was not pulled until AFTER _agent_running had flipped back to False — at which point process_command() took the idle fallback ("No agent running; queued as next turn") and delivered the steer as an ordinary next-turn user message. From Utku's bug report on PR NousResearch#13205: mid-run /steer arrived minutes later at the end of the turn as a /queue-style message, completely defeating its purpose. Fix: add _should_handle_steer_command_inline() gating — when _agent_running is True and the user typed /steer, dispatch process_command(text) directly from the prompt_toolkit Enter handler on the UI thread instead of queueing. This mirrors the existing _should_handle_model_command_inline() pattern for /model and is safe because agent.steer() is thread-safe (uses _pending_steer_lock, no prompt_toolkit state mutation, instant return). No changes to the idle-path behavior: /steer typed with no active agent still takes the normal queue-and-drain route so the fallback "No agent running; queued as next turn" message is preserved. Validation: - 7 new unit tests in tests/cli/test_cli_steer_busy_path.py covering the detector, dispatch path, and idle-path control behavior. - All 21 existing tests in tests/run_agent/test_steer.py still pass. - Live PTY end-to-end test with real agent + real openrouter model: 22:36:22 API call #1 (model requested execute_code) 22:36:26 ENTER FIRED: agent_running=True, text='/steer ...' 22:36:26 INLINE STEER DISPATCH fired 22:36:43 agent.log: 'Delivered /steer to agent after tool batch' 22:36:44 API call #2 included the steer; response contained marker Same test on the tip of main without this fix shows the steer landing as a new user turn ~20s after the run ended.
kingsleydon
pushed a commit
that referenced
this pull request
Apr 23, 2026
Previously the breaker was only cleared when the post-reconnect retry call itself succeeded (via _reset_server_error at the end of the try block). If OAuth recovery succeeded but the retry call happened to fail for a different reason, control fell through to the needs_reauth path which called _bump_server_error — adding to an already-tripped count instead of the fresh count the reconnect justified. With fix #1 in place this would still self-heal on the next cooldown, but we should not pay a 60s stall when we already have positive evidence the server is viable. Move _reset_server_error(server_name) up to immediately after the reconnect-and-ready-wait block, before the retry_call. The subsequent retry still goes through _bump_server_error on failure, so a genuinely broken server re-trips the breaker as normal — but the retry starts from a clean count (1 after a failure), not a stale one.
kingsleydon
pushed a commit
that referenced
this pull request
Apr 23, 2026
- entry.tsx no longer writes bootBanner() to the main screen before the alt-screen enters. The <Banner> renders inside the alt screen via the seeded intro row, so nothing is lost — just the flash that preceded it. Fixes the torn first frame reported on Alacritty (blitz row 5 NousResearch#17) and shaves the 'starting agent' hang perception (row 5 #1) since the UI paints straight into the steady-state view - AlternateScreen prefixes ERASE_SCROLLBACK (\x1b[3J) to its entry so strict emulators start from a pristine grid; named constants replace the inline sequences for clarity - bootBanner.ts deleted — dead code
kingsleydon
pushed a commit
that referenced
this pull request
May 9, 2026
…matrix, troubleshooting (NousResearch#15135) The initial Spotify docs page shipped in NousResearch#15130 was a setup guide. This expands it into a full feature reference: - Per-tool parameter table for all 9 tools, extracted from the real schemas in tools/spotify_tool.py (actions, required/optional args, premium gating). - Free vs Premium feature matrix — which actions work on which tier, so Free users don't assume Spotify tools are useless to them. - Active-device prerequisite called out at the top; this is the #1 cause of '403 no active device' reports for every Spotify integration. - SSH / headless section explaining that browser auto-open is skipped when SSH_CLIENT/SSH_TTY is set, and how to tunnel the callback port. - Token lifecycle: refresh on 401, persistence across restarts, how to revoke server-side via spotify.com/account/apps. - Example prompt list so users know what to ask the agent. - Troubleshooting expanded: no-active-device, Premium-required, 204 now_playing, INVALID_CLIENT, 429, 401 refresh-revoked, wizard not opening browser. - 'Where things live' table mapping auth.json / .env / Spotify app. Verified with 'node scripts/prebuild.mjs && npx docusaurus build' — page compiles, no new warnings.
kingsleydon
pushed a commit
that referenced
this pull request
May 9, 2026
The scheme-validation commit (e77a3f2c) was too strict: a user with
legacy ''baseUrl: localhost:8000'' (no ''http://'' prefix) in their
''~/.honcho/config.json'' would get ''No API key configured'' from the
CLI after that change, even though their setup worked before.
urlparse on a schemeless host:port treats the host segment as the
scheme and leaves netloc empty, so the http/https check rejected it.
Falls back to a lenient check for schemeless strings that look like
hosts: contain '.' or ':', aren't a boolean/null literal, aren't pure
digits. The SDK still rejects truly malformed URLs at connect time
with a clearer error than ours.
Three new tests: legacy schemeless hosts accepted; obvious garbage
literals (''true'', ''null'', ''12345'') still rejected. Reviewer
noted concern #1: schemeless regression for self-hosters with old
configs.
kingsleydon
pushed a commit
that referenced
this pull request
May 9, 2026
* ci(nix): auto-fix stale npm hashes on push to main When a PR merges to main with updated package-lock.json or package.json in ui-tui/ or web/, the new auto-fix-main job detects stale npmDepsHash values and pushes a fix commit directly to main. This eliminates the recurring manual hash-bump PRs (NousResearch#15420, NousResearch#15314, NousResearch#15272, NousResearch#15244) by reusing the existing fix-lockfiles --apply pipeline. The fix commit only touches nix/*.nix files, which are outside the push path filter (package-lock.json / package.json), so it cannot re-trigger itself. Closes NousResearch#15314 * fix(ci): use GitHub App token for auto-fix-main push GITHUB_TOKEN commits are invisible to workflow triggers (GitHub's infinite-loop prevention). The auto-fix-main job pushes directly to main, so the fix commit never triggered downstream nix.yml verification. Mint a short-lived token via the repo's GitHub App (daimon-nous, APP_ID + APP_PRIVATE_KEY secrets) so the push is treated as a real event and nix.yml fires to verify the corrected hashes. Tested via workflow_dispatch dry-run: app token minted successfully, checkout with app token succeeded, fix job correctly gated. Resolves review feedback from Bugbot (r3144569551). * ci(nix): rename lockfile check job for required status check Rename 'check' → 'nix-lockfile-check' so the status check name is unambiguous when added as a required check on main. * fix(ci): harden auto-fix-main against races, loops, and silent failures Address adversarial review findings: 1. Race condition (#1): Job-level concurrency with cancel-in-progress collapses back-to-back pushes; ref: main checkout always gets latest branch state; explicit push target (origin HEAD:main). 2. Loop prevention (#2): File-whitelist check before commit aborts if any file outside nix/{tui,web}.nix was modified, preventing accidental self-triggering. 3. Silent infra failures (#8): nix-lockfile-check now fails explicitly when fix-lockfiles exits without reporting stale status (catches nix setup failures, network errors, script bugs that bypass continue-on-error). 4. Commit traceability (NousResearch#11): Auto-fix commits include source SHA and workflow run URL in the commit body. 5. Explicit push target (NousResearch#12): git push origin HEAD:main instead of bare git push. --------- Co-authored-by: alt-glitch <alt-glitch@users.noreply.github.com>
kingsleydon
pushed a commit
that referenced
this pull request
May 9, 2026
…ch#16706) * fix(tui): drop stale stream events after ctrl-c interrupt Once interruptTurn() flips this.interrupted, only recordMessageDelta short-circuited. recordReasoningDelta/Available, recordToolStart/ Progress/Complete, and recordInlineDiffToolComplete kept populating turnState until the python loop reached its next _interrupt_requested check (~1s on busy turns), making it look like ctrl-c was ignored while late "thinking" + tool calls kept landing in the UI. Add the same interrupted guard to every stream-side recorder, and clear the flag at startMessage() so the next turn isn't suppressed if the previous turn never delivered message.complete. * fix(tui): guard recordTodos against post-interrupt mutation; fake-timers in test Copilot review on PR NousResearch#16706: 1. `recordToolStart` is interruption-guarded, but `tool.start` handler also calls `recordTodos(payload.todos)` first — so a late tool.start carrying todos could still mutate `turnState.todos` after Ctrl-C, leaving ghost rows in the panel. Adds the same `if (this.interrupted) return` early-exit to `recordTodos` so *all* tool.start side-effects are dropped post-interrupt. 2. The interrupt test was leaking a real `setTimeout` (interrupt cooldown) across test files, which could fire later and mutate uiStore from the wrong test context. Wraps the test in `vi.useFakeTimers()` + `vi.runAllTimers()` and restores real timers in finally. 3. Extends the same test with a todos payload on the post-interrupt tool.start so we have explicit regression coverage for #1. * fix(tui): guard pushTrail post-interrupt; harden interrupt-test cleanup Round 2 Copilot review on PR NousResearch#16706: 1. `tool.generating` events route through `pushTrail`, which was not interruption-guarded — late events could still write 'drafting …' into `turnTrail` after Ctrl-C, leaving a stale shimmer in the UI. Adds the same `if (this.interrupted) return` early-exit. 2. Test cleanup moved `vi.runAllTimers()` into `finally` (before `vi.useRealTimers()`) so a mid-test assertion failure can't leak the interrupt-cooldown setTimeout across other test files. 3. Replaced the misleading 'pre-interrupt todos … expected to be cleared by the interrupt cycle' comment with an accurate one reflecting current behaviour (interrupt does NOT clear todos). 4. Added an explicit assertion that a post-interrupt `tool.generating` event does not extend `turnTrail` — regression coverage for #1.
kingsleydon
pushed a commit
that referenced
this pull request
May 9, 2026
Fifth and final slice polish on top of @dlkakbs's docs + skill. Three things ship here: 1. Subscription renewal cron recipe (the #1 operational footgun). Microsoft Graph webhook subscriptions expire at 72 hours max and don't auto-renew. The shipped operator runbook mentioned `maintain-subscriptions --dry-run` as a "daily or periodic check" but never told operators how to actually automate it. Without a scheduled job, any production deployment silently stops ingesting meetings three days after go-live. Adds an "Automating subscription renewal (REQUIRED for production)" section to website/docs/guides/operate-teams-meeting-pipeline.md with three concrete options and copy-pasteable configs: - Option 1: Hermes cron (`hermes cron add --schedule "0 */12 * * *" --script-only --command "hermes teams-pipeline maintain-subscriptions"`) - Option 2: systemd service + timer (12h cadence, Persistent=true so missed runs catch up after reboots) - Option 3: plain crontab with a wrapper that sources .env for credentials Go-Live Checklist gains a bolded mandatory item for the schedule being in place, with a cross-link to the section. website/docs/user-guide/messaging/teams-meetings.md adds a `:::warning:::` admonition right after the manual `subscribe` examples so anyone who creates a subscription manually is told the same day that it will silently expire in 72 hours. 2. Sidebar wiring. Shela's new docs pages (teams-meetings.md and operate-teams-meeting-pipeline.md) weren't in website/sidebars.ts, so they were orphaned URLs — reachable only if someone knew the path. Wired teams-meetings into Messaging Platforms next to the existing teams entry, and operate-teams-meeting-pipeline into Guides & Tutorials next to microsoft-graph-app-registration from PR NousResearch#21922. Adjacent placement keeps the related pages discoverable from each other. 3. SKILL.md rewrite (v1.0.0 → v1.1.0). The original skill had five Turkish-only trigger phrases, which works in a Turkish-speaking session but doesn't match English triggers. Rewrote the skill to: - Describe triggers by intent instead of exact phrases, with explicit "works in any language" framing and example phrases in both English and Turkish. - Add a Decision Tree section covering the three most common user asks (missing summary, setup verification, re-run request) and the specific CLI command sequence for each. - Add a dedicated "Critical pitfall: Graph subscriptions expire in 72 hours" section that tells the agent exactly what to do when a user reports "worked yesterday, nothing today" — the most common operational failure mode. - Expand the command reference into three labeled groups (Status and inspection / Re-running and debugging / Subscription management) so the agent can reach for the right command without scanning. - Add cross-links to all four related docs pages (Azure app registration, webhook listener setup, full pipeline setup, operator runbook). Validation: - npm run build: all new pages route, anchor to #automating-subscription-renewal-required-for-production resolves from both the runbook TOC and the teams-meetings.md admonition. - scripts/run_tests.sh on the relevant test suites (607 tests): all pass.
kingsleydon
pushed a commit
that referenced
this pull request
May 9, 2026
…olve on first launch Three interrelated bugs from teknium1's first interactive chat on Windows: 1. **Snapshot/cwd file paths unquoted in bash command strings.** The session bootstrap and per-command wrapper interpolated ``self._snapshot_path`` / ``self._cwd_file`` unquoted into bash commands like ``export -p > C:/Users/ryanc/.../hermes-snap-xxx.sh``. Git Bash's MSYS2 layer handles ``C:/...`` paths correctly ONLY when quoted; unquoted, the colon and forward-slash get glob-parsed and the redirect targets a bogus path. Symptom: every terminal command emitted two ``C:/Users/.../hermes-snap-*.sh (No such file or directory)`` lines that bled into stdout (``stderr=STDOUT`` on the local backend) and corrupted file contents when the agent wrote to scratch paths via the terminal tool. Fix: ``shlex.quote()`` every interpolation of ``_snapshot_path`` and ``_cwd_file`` in base.py — no-op on POSIX (the paths contain no shell-metachars), critical on Windows. 2. **Stale PATH on first hermes launch after install.** ``install.ps1`` adds the PortableGit ``cmd`` / ``bin`` / ``usr\bin`` directories to the Windows **User** PATH via ``SetEnvironmentVariable(..., "User")``. That write propagates to newly *spawned* processes only — already-running shells (including the one the user types ``hermes`` into immediately after install) retain their old PATH. So hermes starts with a PATH that doesn't include bash, rg, grep, ssh — and ``search_files`` reports "rg/find not available" when the user clearly just installed them. Fix: new ``_augment_path_with_known_tools()`` helper called from ``configure_windows_stdio()`` on startup. Prepends the Hermes-managed Git directories + the WinGet Links directory (where ripgrep lands) to ``os.environ['PATH']`` if they exist on disk but aren't already in PATH. Subsequent subprocess calls (including bash spawns via ``_find_bash()``) inherit the augmented PATH and find everything. No-op on POSIX and when the directories don't exist. 3. **Root cause of "file content corruption".** #1 was the proximate cause. Errors like ``C:/Users/.../hermes-snap-xxx.sh: No such file or directory`` were emitted on stderr by the failed redirect, captured into stdout via ``stderr=subprocess.STDOUT``, and if the agent used terminal commands like ``cat > file`` the leaked error bytes became part of the file. Fixing #1 eliminates this entirely. ## Tests All 77 Windows-compat tests still pass on Linux (POSIX path is shlex.quote('/tmp/foo.sh') → '/tmp/foo.sh' — unchanged). ## Not addressed here (would need a bigger design) - Python file tools (``write_file``, ``read_file``) and the bash-backed terminal tool see DIFFERENT views of ``/tmp`` on Windows. Python treats ``/tmp`` as ``C:\tmp`` (drive-relative), Git Bash's MSYS2 treats it as a virtual mount to the PortableGit install's ``tmp\``. Would need a translation shim in the Python tools to resolve bash-virtual paths to their native-Windows equivalents. Workaround for users today: use absolute native paths (``C:\Users\you\...``) instead of ``/tmp/...`` when crossing between terminal and Python file tools.
kingsleydon
pushed a commit
that referenced
this pull request
May 22, 2026
Four findings from Copilot's review on PR NousResearch#22891, all in the AX elements-array cap added by 22fa1ed: 1. The truncation note ("response truncated to N of M elements") was appended unconditionally — including in the som/vision multimodal path, whose response carries a screenshot rather than an `elements` array. The note described a payload field that wasn't present. Moved the note into the AX-text branch where the array actually appears. 2. `_format_elements(cap.elements)` ran on the full untrimmed list with its own `max_lines=40` cap, so a caller passing `max_elements=10` would see summary lines referencing `NousResearch#11..NousResearch#40` even though the JSON `elements` array only held #1..#10. Format on `visible_elements` instead so the summary indices always exist in the response. 3. `_coerce_max_elements` enforced a lower bound but no upper bound, so `max_elements=10_000_000` silently disabled the safeguard and reintroduced the original context-blow-up. Added a hard cap (`_MAX_ALLOWED_MAX_ELEMENTS = 1000`) that clamps oversized values. 4. The schema string said "Default 100" but the property carried no `default` field, and claimed `max_elements` had no effect on som/ vision while the image-missing fallback path can still return an elements array. Added `"default": 100`, `"maximum": 1000`, and clarified the fallback-path wording. Each finding gets a regression test: - test_capture_ax_clamps_oversized_max_elements_to_hard_cap - test_capture_ax_summary_indices_match_returned_elements - test_capture_multimodal_summary_omits_truncation_note - test_schema_max_elements_documents_default_and_upper_bound Verified with `pytest tests/tools/test_computer_use.py` (53 passed, including the 5 new cases). Confirmed each new test fails on the pre-fix code path before applying the production change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kingsleydon
pushed a commit
that referenced
this pull request
May 27, 2026
…NousResearch#31416) PR NousResearch#31416 (avoid persisting borrowed credential secrets) added sanitize_borrowed_credential_payload, which strips access_token from any auth.json pool entry whose (provider, source) isn't in the _PERSISTABLE_PROVIDER_SOURCES allowlist. (copilot, gh_cli) is borrowed (not in the allowlist), so the test fixture's pre-seeded access_token now gets stripped at load_pool() time, leaving the pool empty. resolve_target('1') then fails with 'No credential #1. Provider: copilot.' Fix: align the test with the new contract. At runtime, copilot tokens are hydrated by resolve_copilot_token() — mock that path so the pool gets an entry the test can remove. The behavior under test (suppression of gh_cli + env variants on remove) is unchanged. CI repro on origin/main HEAD; reproduced locally with stock checkout.
kingsleydon
pushed a commit
that referenced
this pull request
May 27, 2026
…s reached After key #1 is marked exhausted the retry still called the API with key #1 due to env-var bias in _get_cached_client / resolve_api_key_provider_credentials. Fix: peek the pool and pass the active entry's key as explicit_api_key. Secondary: api_key_hint in mark_exhausted_and_rotate pins the correct entry under concurrent CLI+gateway calls; _is_payment_error matches GoUsageLimitError; extract_api_error_context parses "Resets in Xhr Ymin".
kingsleydon
pushed a commit
that referenced
this pull request
May 27, 2026
…ookies
Mission-control style deploys reverse-proxy the dashboard at a path
prefix (e.g. mission-control.tilos.com/hermes/* -> :9119) and inject
X-Forwarded-Prefix: /hermes on every request. The SPA mount already
honoured this for asset URLs and the bootstrap __HERMES_BASE_PATH__,
but the OAuth gate didn't:
1. The gate's Location: header to /login and the 401 envelope's
login_url were built bare ("/login?next=..."). Under a /hermes
prefix the browser follows that to mission-control.tilos.com/login
which the proxy doesn't route to the dashboard.
2. _redirect_uri (the OAuth callback URL handed to the IDP) used
request.url_for() which doesn't honour X-Forwarded-Prefix
(Starlette/uvicorn only proxy_headers Host + Proto + For). The
IDP redirects back to /auth/callback instead of /hermes/auth/
callback → 404 in the user's browser.
3. Cookies were set with Path=/ which leaks them to other apps on
the same origin and won't be sent back on requests under the
prefix in the first place.
Fix threads the normalised prefix through every boundary:
* New hermes_cli/dashboard_auth/prefix.py — single source of truth
for X-Forwarded-Prefix parsing. web_server._normalise_prefix
becomes a re-export so the SPA mount, the gate, and the cookies
helper all agree.
* middleware._unauth_response builds login_url = f"{prefix}/login".
* routes._redirect_uri splices the prefix into the path component
of the IDP-bound URL (with full validation of the header).
* cookies.{set,clear}_{session,pkce}_cookie now take prefix="".
Path attribute switches to /hermes when set; cookie name switches
name variant (see below). Every caller passes the request's
normalised prefix.
Cookie hardening (Teknium's lesser-note #1 in the PR review): adopt
the __Host- / __Secure- cookie name prefixes per draft-west-cookie-
prefixes. The variant is selected from (use_https, prefix):
* Loopback HTTP → bare "hermes_session_at" (both prefixes require
Secure, incompatible with HTTP).
* HTTPS, direct deploy (Path=/) → "__Host-hermes_session_at".
Strongest spec: bound to exact origin, no Domain attribute, Secure
required.
* HTTPS, behind a proxy prefix (Path=/hermes) →
"__Secure-hermes_session_at". __Host- forbids Path != "/"; the
explicit Path=/hermes covers same-origin app isolation.
Setter and reader BOTH consult the prefix because the cookie *name*
changes — a reader that looked up the bare name when the setter wrote
__Secure- would never find the value. The reader falls back across
all three variants so a request whose shape changed mid-session (e.g.
post-deploy from no-prefix to /hermes) still picks up the existing
cookie until it expires.
Test coverage:
- tests/hermes_cli/test_dashboard_auth_prefix.py — new file. 11 tests
pinning:
• Location: /hermes/login on the gate's HTML redirect
• 401 envelope login_url carries the prefix
• Malformed X-Forwarded-Prefix is ignored (header-injection
defence; the script-tag value is normalised to empty string)
• _redirect_uri splices /hermes into the path (the property
that prevents the IDP-returns-to-404 failure)
• PKCE cookie uses Path=/hermes + __Secure- when proxied
• Session cookies use __Host- when direct, __Secure- when
proxied, bare on loopback HTTP
• End-to-end round trip with hand-managed PKCE cookie carriage
(TestClient can't simulate a Path=/hermes cookie automatically)
- tests/hermes_cli/test_dashboard_auth_cookies.py — rewritten to pin
each (use_https, prefix) shape produces its expected cookie name,
plus reader-side coverage that __Host- and __Secure- variants are
both recognised.
- Existing tests across middleware / 401-reauth / etc. updated to
match the new cookie names (substring contains instead of
startswith).
Mutation-tested: reverting _unauth_response to build the bare
"/login" URL trips exactly the two tests that pin the prefix
carriage, confirming the suite discriminates the regression.
kingsleydon
pushed a commit
that referenced
this pull request
Jun 3, 2026
Two CI flakes surfaced on PR NousResearch#34572 (both in files this PR doesn't touch; pre-existing host-dependent flakes): 1. test_process_registry::TestPopenLeakOnSetupFailure — the failure-cleanup tests use a fake proc.pid (8888/9999) and assert proc.kill() runs. But spawn_local's primary cleanup is os.killpg(os.getpgid(pid), SIGKILL), falling back to proc.kill() only on ProcessLookupError/PermissionError/ OSError. When the fake PID happens to exist on a busy host, os.getpgid succeeds, os.killpg fires against an UNRELATED real process group, and proc.kill() is never reached -> flaky AssertionError (and a real risk of SIGKILLing an innocent process group from a unit test). Patch os.getpgid to raise ProcessLookupError so the fallback path runs deterministically and no real killpg is ever issued. 2. test_web_server::test_resize_escape_is_forwarded — the receive loop calls the blocking conn.receive_bytes() with no exception guard. Once the child prints its winsize and exits, the PTY closes; on a missed-marker run the next recv blocks until the 30s pytest-timeout instead of failing fast. Add a try/except break (matching the working sibling tests) and bump the child's pre-read sleep 0.15s -> 0.5s so the resize reliably lands first. Verified: 4/4 pass across 3 consecutive runs; root cause for #1 reproduced (os.getpgid(1) succeeds -> old code skips proc.kill).
kingsleydon
pushed a commit
that referenced
this pull request
Jun 3, 2026
Seven Copilot inline review comments on NousResearch#37679, four worth landing in a polish pass before merge: 1. _dispose_unused_adapter signature: 'BasePlatformAdapter' -> 'BasePlatformAdapter | None'. The function explicitly handles None and the reconnect watcher calls it with None in the except arm, so the annotation now matches the actual contract. 2. (duplicate of #1 on a different line) — same fix. 3. except Exception in _dispose_unused_adapter — the reviewer asked about asyncio.CancelledError swallowing. On Python 3.8+ (Hermes requires 3.13, see pyproject.toml), CancelledError inherits from BaseException, NOT Exception, so the existing 'except Exception' does NOT swallow task cancellation. Added an explicit comment explaining the contract so future readers don't repeat the analysis. We don't re-raise because the watcher loop intentionally treats dispose failures as best-effort: a failed dispose on an unowned adapter should not take down the watcher that's keeping the gateway alive. 4. _response_store = None after close in api_server.py — the reviewer flagged this for idempotency. Decided to keep the non-None state intentionally: setting it to None cascades to ~9 callers that access self._response_store without a None check, and 'close() is idempotent on a closed sqlite3 Connection' means the current code is already safe. The type stays stable; LSP doesn't flag a cascade of reportOptionalMemberAccess errors. (This matches the pre-existing pattern in the codebase — e.g. _mark_disconnected doesn't reset state to None either.) 5. _build_adapter_with_store: reviewer worried about disconnect() failing on the self.name property if __init__ wasn't called. Already handled: we set 'adapter.platform = Platform.API_SERVER' so the 'self.platform.value.title()' property returns 'Api_Server' without raising. The exception-swallowing branch in disconnect() does call self.name via the logger.debug format, so this is a real path that needs the platform attribute, and we have it. 6. test_disconnect_closes_response_store: bare 'pytest.raises(Exception)' -> 'pytest.raises(sqlite3.ProgrammingError)'. The bare Exception matcher would silently accept AttributeError, OperationalError, env-related issues, etc. The specific exception type ('Cannot operate on a closed database') is the actual signal we want — proves the SQLite conn is closed, not just that *something* raised. 7. test_nonretryable_failure_disposes_unowned_adapter: assertion tightened from '>= 1' to '== 1' on adapter._disconnect_calls. The docstring said 'exactly once', the assertion now matches. Catches the hypothetical 'watcher disposes the same adapter twice' regression that '>=' would have missed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Elevates the webapi module from a prototype to a production-ready API layer that the Clawdi dashboard can consume with full type safety. Adds bearer token authentication, typed Pydantic response models on every route, cron job management, SSE stream abort propagation, tool_call_id for accurate tool-card matching, and 20 end-to-end smoke tests.
Companion PR: Clawdi-AI/clawdi#345 — the frontend that consumes these endpoints.
Changes
1. Bearer token authentication (
webapi/auth.py)FastAPI dependency that validates
Authorization: Bearer <token>againstHERMES_API_TOKENenv var usinghmac.compare_digest. No-op when unset (dev mode). Applied to every route exceptGET /health.Token source:
GATEWAY_AUTH_TOKENderived via HKDF-SHA256 fromMASTER_KEYin the pod entrypoint. The Clawdi backend returns the same value asdep.gateway_token, so browser → controller → webapi auth is seamless.2. Typed response models on every route
Previously 8 routes returned
dict/list[dict], producing{"type": "object"}in/openapi.json. Now every route has a concrete Pydanticresponse_model:SessionDetailResponse,SessionListResponse,MessageListResponse,SearchSessionsResponseConfigResponse,ConfigPatchResponseMemoryReadResponse,MemoryMutationResponse(with typedMemoryTarget)SkillsListResponse,SkillCategoriesResponse,SkillDetailResponseJobsListResponse,JobResponse,JobDeleteResponseHealthResponse,OpenAIModelsResponse,AvailableModelsResponseChatResponse(non-streaming),ChatRequestwith nullable optional fieldsThis enables
openapi-typescriptto generate a 2,140-line fully-typed TS schema with zerounknownfields.3. Expanded ConfigPatch (
webapi/routes/config.py)PATCH /api/confignow accepts every messaging platform (telegram, discord, slack, whatsapp, matrix, mattermost, signal, sms, email, feishu, dingtalk, wecom, bluebubbles, homeassistant, webhook) and every cross-cutting subsystem (security, memory, cron, display, toolsets, mcp_servers).Deep-merge semantics: partial updates field-by-field,
nulldeletes a key. Response includesmerged_sectionslist.4. Cron job management (
webapi/routes/jobs.py)8 endpoints:
GET/POST /api/jobs,GET/PATCH/DELETE /api/jobs/{id},POST /api/jobs/{id}/{pause,resume,run}. Returns 501 if thecronmodule isn't installed. Input validation: name ≤200 chars, prompt ≤5000, 12-char hex job ID.5. SSE stream abort propagation (
webapi/routes/chat.py)When a chat stream client disconnects, the
GeneratorExitis caught andagent.interrupt()is called on the background worker thread. Previously the worker ran to completion, burning tokens. Usesthreading.Eventfor the cancelled signal + bounded queue (1024 frames) for backpressure.6. Message pagination (
webapi/routes/sessions.py)GET /api/sessions/{id}/messagesgainslimit,offset,tail=truequery params.tail=truereturns the last N messages (chat scrollback).totalin response is always the full count.7. tool_call_id in SSE events (
webapi/deps.py)tool.startedevents now carry the assistant-providedtool_call_idvia a newtool_start_callbackparameter oncreate_agent(). Enables frontend to match started/completed pairs by ID instead of ordinal — correct for concurrent same-name tool invocations.8. Session fork with title generation (
webapi/routes/sessions.py)POST /api/sessions/{id}/forkdeep-copies a session with lineage-based title suffixing (e.g., "Chat" → "Chat (1)" → "Chat (2)"). Retry loop for concurrent forks (up to 5 attempts). Delete fails with 409 if session has forked children.9. Session search via FTS5 (
webapi/routes/sessions.py)GET /api/sessions/search?q=...&limit=20with typedSearchSessionsResponsecontainingSearchSessionMatchrecords.10. Smoke test suite (
tests/webapi/)20 pytest tests covering: health, auth (on/off), all GET routes, config PATCH, session lifecycle, pagination, FTS5 search, cron CRUD, memory DELETE-with-body, 401/422 error paths. Runs in ~1.5s via in-memory fakes.
API surface
/health/v1/models/api/available-models/api/sessions/api/sessions/search/api/sessions/{id}/api/sessions/{id}/messages/api/sessions/{id}/fork/api/sessions/{id}/chat/api/sessions/{id}/chat/stream/api/config/api/memory/api/skills/api/skills/categories/api/skills/{name}/api/jobs/api/jobs/{id}/api/jobs/{id}/pause/api/jobs/{id}/resume/api/jobs/{id}/runVerification
Merge order
hermes-workspaceclawdi-agentimage (pinsARG HERMES_VERSION=hermes-workspace)🤖 Generated with Claude Code