[chazmaniandinkle]

What

A body channel: an agent can drive an embodied avatar (Live2D first) and get a receipt of what the model actually did.

• body.py + schemas/http/bodies.py: body registry. /ws/body/{id} (a body declares its manifest: params with ranges, named states), GET /v1/bodies, GET /v1/bodies/{id}, POST /v1/bodies/{id}/act (set/hold/release params, set state, wait for a receipt).
• dashboard/live2d-body.js + dashboard/body.html: a Live2D model as a body. Receipts are read back from the Cubism core, not echoed. Agent-held params sit on top of the procedural idle layer until released. Seeded RNG. Optional lip sync from /ws/audio/{session}. ?model= loads any model3.json.
• dashboard/rig.html: one-page dev/debug surface (health, sessions, HUD, puppet controls with receipts, speak + WebSocket probes, live stage).
• requirements.txt: websockets>=12. Without a WebSocket library the ASGI server answers every /ws/* upgrade with a plain 404 and only logs a warning, so /ws/audio, /ws/chat, /ws/acp were dead in a real deployment while every HTTP test passed. tests/test_websocket_transport.py does a real upgrade through a real server.

Provenance

live2d-body.js adapts Vroku's Live2D controller suite. Vroku released that code with no restrictions (unlicensed, no attribution required), recorded in the file header, so it ships under MIT.

Verified

• Held params: written on afterMotionUpdate and again on beforeModelUpdate (pixi-live2d-display 0.4.0 update order), so motions and physics no longer overwrite them. Live: 2–5 params held at once, every receipt exact (±0.05) immediately and after 1.5 s, 37–145 ms round trip.
• Auditioned all 8 official Cubism samples: 7 load and puppet exactly. Ren (Cubism 5.3 moc) does not load in pixi-live2d-display 0.4.0.
• Running in a real deployment since 2026-10-03: /ws/audio, /ws/chat, /ws/dashboard-chat upgrade in 1–16 ms.
• tests/test_body.py: 12 tests (validation 2, non-finite rejection 1, release feedback 2, channel 5, replaced-connection 1, registry thread-safety 1), all passing. Full suite 1080 passed; the 2 test_server_topology failures are local only (Kokoro prewarm can't read a local model cache path here, so the daemon misses its 45 s window). CI is the arbiter.
• Public release gate (cogpublic-guard.py): OK, 209 files, 8 guards.

Review rounds: round 1 found the replaced socket was never closed (fixed in 2c09685: close with 4409, the client stops reconnecting, test plus negative control). Round 3 found NaN getting through the clamp (fixed in 42b03f3: non-finite values rejected, test plus negative control), and the same-loop close task is now kept referenced. Round 4 found that hold_ms: 0 held a param forever in the JS (fixed: 0 now releases on the next tick). From the same round's notes: writes to the body socket are now serialized, and unknown release ids are reported in rejected. Round 5 found that the welcome frame bypassed the socket lock; it and the eviction close now both take the lock, and the handler's only unlocked writes are before registration. Round 6 found that the registry was iterated from the thread pool while the event loop mutated it; now every registry and pending-map access is locked (stress test: 5/5 fail without the fix, 5/5 pass with it).

[github-actions]

🤖 cog-review — request_changes (head 4f05a95)

The websockets-dependency fix is well-targeted and covers all pip-install sites that pull fastapi/uvicorn (ci.yml's two jobs; no other workflow or setup script installs that stack separately), with a real-upgrade test that would actually fail without the fix — good premise verification and test evidence. The new body channel's HTTP/WS wiring and validation (body.py, http_api.py, schemas/http/bodies.py) are internally consistent and covered by tests/test_body.py. However, BodyRegistry.register() has a confirmed resource-leak bug on reconnect: when a body_id reconnects, the old _Body's pending futures are failed, but the previous physical WebSocket connection itself is never closed (register() has no handle to the old socket — _Body stores only a send closure, not a close callback). The stale connection's ws_body coroutine keeps looping on await websocket.receive_json() forever (or until the old client disconnects on its own), leaking a server-side task and OS socket per reconnect. Everything else (clamping/validation, timeout/BodyError mapping, cross-loop future wiring via concurrent.futures.Future + asyncio.wrap_future) checked out under manual trace.

Confirmed findings (1):

• body.py:133 — BodyRegistry.register() replaces the old _Body entry and fails its pending futures but never closes/cancels the previous WebSocket connection, leaking the socket and its server-side task.
  • Failure scenario: A second client connects to /ws/body/storm (e.g. a duplicate dashboard tab, or a client reconnecting before the server notices the old TCP connection is dead) while the first connection for body_id 'storm' is still open and healthy. register() swaps self._bodies['storm'] to the new _Body and fails any pending futures on the old one, but the old websocket is never told to close — its ws_body() coroutine (http_api.py:3601) stays parked in await websocket.receive_json() indefinitely. Each such reconnect leaks one idle WebSocket + asyncio task that is only reclaimed if/when that old client happens to disconnect on its own; under routine dashboard reloads during development this accumulates unbounded orphaned connections.

Unverified notes:

• The CubismParams class (dashboard/live2d-body.js) reads private underscored fields of pixi-live2d-display's internal coreModel (_parameterIds, _parameterMinimumValues, etc.) pinned to pixi-live2d-display@0.4.0 via CDN; any upstream minor-version bump to that CDN URL could silently break parameter introspection with no test coverage (JS is untested here).
• The PR description's live performance numbers (round-trip ms, 'running in production since 2026-10-03', 7/8 Cubism samples load) are operational claims that cannot be verified from a static review of this checkout.
• act()'s cross-event-loop branch (asyncio.run_coroutine_threadsafe(body.send(frame), body.loop)) appears unexercised by any test and unreachable in the current single-loop uvicorn deployment implied by the rest of the code; not flagged as a defect since no test proves it wrong, but it is unverified.

---

This review was generated by an AI reviewer with review-only authority — it can approve or block, it cannot merge or close. Verdict basis: pr-review-rubric.md. The merge decision belongs to a human or their operator workflow.

[github-actions]

🤖 cog-review — comment (head 2c09685)

This revision of PR #154 correctly addresses the prior cog-review finding (body.py:133): BodyRegistry.register() now closes the replaced body's WebSocket with code 4409 via _Body.close, http_api.py's ws_body wires a real closer, live2d-body.js stops auto-reconnecting on 4409, and tests/test_body.py::TestReplacedConnection is a genuine regression test (the commit message documents a negative control: with the fix stashed the test hangs and is killed by its alarm, confirming it would fail without the fix). The rest of the body-channel feature (body.py registry/validation, the bodies HTTP endpoints, the websockets dependency plus tests/test_websocket_transport.py, and the Live2D JS controller) is well-scoped and the premises in the PR description check out against the diff. One narrow, low-severity gap survives in both layers of the parameter clamp: validate_command()'s min(max(v, lo), hi) in body.py and clamp() in live2d-body.js both leave a NaN value unclamped, because any comparison against NaN is false in both Python and JS min/max, so max(nan, lo) and min(nan, hi) simply return nan. BodyActRequest.params is typed dict[str, float] without allow_inf_nan=False, and pydantic v2 accepts the JSON literal NaN by default, so a POST like {"params": {"ParamAngleX": NaN}} is accepted, bypasses the range clamp the docstring promises, and is written straight into the Cubism core parameter array, producing a receipt with a NaN value instead of a clamped number. This is cosmetic (no crash, no resource leak) rather than a safety issue, so it does not block merge on its own, but it's worth tightening since the whole point of this code path is the clamp guarantee. Also noted for the author: the PR description's "tests/test_body.py 9 passed" doesn't match the 8 test functions actually present in the diff — almost certainly a stale count, not a missing test.

Confirmed findings (1):

• body.py:144 — validate_command()'s clamp min(max(v, lo), hi) does not clamp NaN because comparisons against NaN are always false in Python, so max(nan, lo) and min(nan, hi) both just return nan
  • Failure scenario: POST /v1/bodies/{id}/act with {"params": {"ParamAngleX": NaN}} (pydantic's dict[str, float] field accepts the JSON NaN literal by default since allow_inf_nan isn't disabled) passes validate_command unclamped and is forwarded to the body as an act frame; live2d-body.js's own clamp() has the identical NaN-is-a-no-op gap, so the Cubism parameter is actually set to NaN and the receipt reports NaN instead of a value inside the declared range.

Unverified notes:

• PR description claims "tests/test_body.py 9 passed" but the diff only adds 8 test functions (TestValidation: 2, TestBodyChannel: 5, TestReplacedConnection: 1) — likely a stale count rather than a missing test.
• BodyRegistry._close()'s same-loop path does here.create_task(body.close()) without keeping a reference to the returned Task. asyncio only holds a weak reference to tasks, so in general an unreferenced task can be GC'd before it completes; here the coroutine (a single awaited websocket.close()) likely either finishes within its first step or stays alive via the Future it suspends on, so this probably isn't exploitable in practice, but it isn't provably safe without tracing uvicorn's actual suspension points.

---

This review was generated by an AI reviewer with review-only authority — it can approve or block, it cannot merge or close. Verdict basis: pr-review-rubric.md. The merge decision belongs to a human or their operator workflow.

[github-actions]

🤖 cog-review — request_changes (head 42b03f3)

This PR adds a well-designed body channel (body.py registry, /ws/body/{id}, /v1/bodies/* endpoints) plus the websockets dependency fix for /ws/* transport, backed by solid Python-side tests (validation, clamping, NaN/Inf rejection, 4409 replacement semantics, timeout handling) that genuinely exercise the new code paths. The CI workflow and requirements.txt changes are consistent and complete (both pip-install stanzas updated; no other dependency manifest needed the same fix). However, there is one confirmed logic bug in the new client-side code: dashboard/live2d-body.js treats hold_ms: 0 as falsy, so a param held with hold_ms=0 never auto-releases, contradicting the documented contract in schemas/http/bodies.py. This is reachable end-to-end since body.py's validate_command explicitly forwards 0 (checking is not None, not truthiness) rather than treating it as absent.

Confirmed findings (1):

• dashboard/live2d-body.js:157 — hold_ms: 0 is falsy in JS, so the auto-release timer for held params is never scheduled when hold_ms is explicitly 0.
  • Failure scenario: POST /v1/bodies/{id}/act with {"params":{"ParamAngleX":10},"hold_ms":0} — body.py forwards hold_ms=0 (since it only checks is not None), but if (cmd.hold_ms) this.holdTimers.set(...) in live2d-body.js skips scheduling because 0 is falsy, so ParamAngleX stays held forever instead of releasing immediately as documented in schemas/http/bodies.py.

Unverified notes:

• Concurrent POST .../act calls against the same connected body_id would each call body.send() → await websocket.send_json(...) on the same WebSocket from separate asyncio tasks (one per HTTP request) with no lock serializing the writes; depending on whether Starlette/uvicorn's ASGI send is safe under concurrent callers, this could risk interleaved/corrupted WS frames. Could not inspect the installed starlette/uvicorn source in this sandbox (pip/python introspection was blocked) to confirm one way or the other, and the rest of http_api.py's /ws/* handlers follow the same no-lock pattern, so this may be a pre-existing accepted risk rather than new.
• In validate_command (body.py), unknown ids passed in release are silently dropped from the outgoing release list with no entry added to rejected, unlike unknown params ids which are reported — inconsistent feedback, not necessarily wrong behavior.
• The JS body implementation (dashboard/live2d-body.js) has no automated test coverage; only the Python-side registry/validation logic is exercised by tests/test_body.py. This project appears to have no JS test harness at all, so this may be consistent with existing conventions rather than a gap specific to this PR.
• PR description claims (production soak since 2026-10-03, specific round-trip latencies, audition results across 8 Cubism samples, and the Vroku Discord licensing conversation) are asserted but not independently verifiable from the diff or repo contents.

---

This review was generated by an AI reviewer with review-only authority — it can approve or block, it cannot merge or close. Verdict basis: pr-review-rubric.md. The merge decision belongs to a human or their operator workflow.

[github-actions]

🤖 cog-review — request_changes (head 753122a)

The body-channel feature (body.py, the /ws/body + /v1/bodies/* endpoints in http_api.py, the Live2D client, and the websockets dependency fix) is well scoped and the three prior review rounds' fixes (close replaced socket with 4409, reject non-finite params, hold_ms=0 semantics, unknown-release-id reporting) check out against the code and are covered by tests in tests/test_body.py. The websockets-missing-dependency premise is real and independently verified by tests/test_websocket_transport.py. However, this commit's own 'serialize socket writes' fix is incomplete: http_api.py's ws_body handler registers the body (making it immediately reachable by concurrent /v1/bodies/{id}/act calls) one line before sending the 'welcome' frame, and that welcome send bypasses the newly-added send_lock entirely, leaving exactly the concurrent-write race on the same socket that send_lock was introduced to close. This is a narrow window but is squarely the bug class this PR is actively fixing elsewhere in the same function, so it should be closed (e.g. send the welcome through _send(), or register only after welcome is sent) before merge.

Confirmed findings (1):

• http_api.py:3634 — The 'welcome' frame is sent via a bare websocket.send_json() that bypasses the just-added send_lock, even though the body is already registered and reachable by /act before this send completes.
  • Failure scenario: A client connects to /ws/body/{id} and sends hello; registry.register() (line 3633) makes {id} immediately visible to /v1/bodies/{id}/act. If a concurrent act() call races in during the await on line 3634, its frame is written via _send() (which takes send_lock) at the same time as the unlocked welcome write on the same socket — the same concurrent-write hazard send_lock (lines 3623-3627) was added in this very commit to prevent, so the fix's own stated guarantee ('every send path runs on that loop') does not hold for this line.

Unverified notes:

• BodyRegistry._close() (body.py) calls websocket.close() on the evicted connection without going through that connection's send_lock, so a close can run concurrently with an in-flight send_json on the same (being-evicted) socket. This looks like intentional forced eviction rather than a bug, but it was not independently verified against Starlette/uvicorn's concurrency contract for close() vs. in-flight send().

---

This review was generated by an AI reviewer with review-only authority — it can approve or block, it cannot merge or close. Verdict basis: pr-review-rubric.md. The merge decision belongs to a human or their operator workflow.

[github-actions]

🤖 cog-review — request_changes (head 8cb51a1)

The body channel implementation (body.py, http_api.py bodies_list/bodies_get/bodies_act, schemas/http/bodies.py) is well-reasoned and the four described review-round fixes (closing the replaced socket with 4409, rejecting non-finite param values, hold_ms=0 releasing instead of holding forever, serialized socket writes, and routing the welcome/4409-close frames through the socket lock) are all present and correctly implemented and covered by tests/test_body.py. The websockets>=12 dependency fix is real (uvicorn does silently 404 /ws/* upgrades without a WS library) and is applied to both CI jobs that run pytest, with tests/test_websocket_transport.py verifying a real upgrade through a real uvicorn server. However, one confirmed concurrency bug survives: BodyRegistry.list() (body.py:198) iterates self._bodies.values() with no synchronization while register()/unregister() (body.py:164,188), which run on the asyncio event loop, mutate that same dict's size; GET /v1/bodies and GET /v1/bodies/{id} are plain def routes (http_api.py:3651,3659) that Starlette dispatches to a worker thread pool, so this is a genuine cross-OS-thread race that can raise RuntimeError: dictionary changed size during iteration and surface as a 500 on a read-only endpoint, under exactly the connect/reconnect churn the PR's own description says is expected in deployment. This should be fixed before merge (e.g. snapshot with self._bodies.copy() or guard with a lock).

Confirmed findings (1):

• body.py:198 — BodyRegistry.list() iterates self._bodies.values() without synchronization while register()/unregister() mutate the same dict's size from a different OS thread (the FastAPI thread-pool thread running the sync GET routes vs. the asyncio event-loop thread running ws_body).
  • Failure scenario: A body connects or disconnects (body.py:164 self._bodies[body_id] = body or body.py:188 del self._bodies[body.body_id], both running on the event loop) at the same moment a concurrent GET /v1/bodies request (dispatched to Starlette's thread pool because bodies_list is a plain def, http_api.py:3651) is iterating self._bodies.values() in list() — CPython raises RuntimeError: dictionary changed size during iteration, which propagates as an uncaught 500 from an otherwise read-only endpoint. The PR's own description notes bodies auto-reconnect on drop, making this reconnect/list-poll overlap realistic in the deployment it describes.

Unverified notes:

• The JS-side hold_ms=0 fix (dashboard/live2d-body.js act(), 'releases on the next tick') and the Live2D visual/receipt-accuracy claims in the PR description are plausible from reading the code but are not exercised by any automated test in this repo (no browser/JS test harness exists), so they rest on the PR author's manual verification notes rather than CI-checked evidence.
• dashboard/live2d-body.js connectBody()'s onmessage handler is async but WebSocket delivers messages synchronously without backpressure, so two 'act' frames arriving back-to-back could run overlapping body.act() calls concurrently; this looked benign on inspection (receipts are correlated by id, not by arrival order) but was not exercised under real concurrency, so it is noted rather than confirmed.

---

This review was generated by an AI reviewer with review-only authority — it can approve or block, it cannot merge or close. Verdict basis: pr-review-rubric.md. The merge decision belongs to a human or their operator workflow.

[github-actions]

🤖 cog-review — approve (head 7481277)

This PR adds a new body channel (body.py, schemas/http/bodies.py, http_api.py routes, dashboard/live2d-body.js + body.html + rig.html) and fixes the real premise that uvicorn silently 404s every /ws/* upgrade without a websockets/wsproto package, backed by a real-server upgrade test. The diff under review already incorporates five rounds of prior fixes (replaced-socket 4409 close, non-finite param rejection, hold_ms=0 semantics, serialized socket writes, unknown-release-id reporting, welcome-frame/eviction-close routed through the per-socket lock, and BodyRegistry now locking _bodies/pending for the thread-pool GET routes). Independent re-reading of body.py's locking (register/unregister/get/list/on_receipt/act), the ws_body handler's lock usage (accept/hello before registration unlocked, welcome/act/close all through send_lock with no await held across the lock boundary), validate_command's clamping/rejection logic, and live2d-body.js's held-param/hold_ms/receipt handling did not surface any new confirmed defect; the claimed test coverage (tests/test_body.py, tests/test_websocket_transport.py) matches what the code actually does and exercises the specific bug classes the description calls out.

Confirmed findings (0):
none

Unverified notes:

• In ws_body (http_api.py ~3609-3618), if the client disconnects before sending the initial hello, websocket.receive_json() raises inside the asyncio.wait_for, which is caught by the broad except, and the handler then calls websocket.close(code=4400) on an already-disconnected socket; depending on Starlette/uvicorn's ASGI send behavior this could raise an uncaught exception in that connection's task (logged, not server-crashing) rather than returning cleanly — could not confirm the exact behavior without a live disconnect-before-hello repro.
• The JS-side fixes described in the PR (hold_ms=0 release-on-next-tick, afterMotionUpdate/beforeModelUpdate write ordering so held params survive physics/motion, held/holdTimers map updates under concurrent act() calls) have no automated test; verification rests entirely on the manual live-testing notes in the description. This is understandable for canvas/Live2D rendering code lacking browser-automation infra in this repo, but it means the actual write-ordering fix that rounds 1-5 centered on is not regression-covered by CI.

---

This review was generated by an AI reviewer with review-only authority — it can approve or block, it cannot merge or close. Verdict basis: pr-review-rubric.md. The merge decision belongs to a human or their operator workflow.