fix(gateway): session delete lock and partial skill body refusal

[adriannoes]

Summary

Consolidates the two remaining cursor high-severity gateway/core fixes as operator-authored commits:

• fix(gateway): hold the agent lock when deleting a session (supersedes fix(gateway): hold the agent lock when deleting a session #103) — dashboard DELETE /api/sessions/:id now goes through dispatch_delete_session(), which wraps session_delete in agent_lock() so it cannot race agent_run on the shared g_db; the inline lock from fix(cron): schedule validation and serialized SQLite access #122's review fix is replaced by this shared entry point and test_dispatch covers it.
• fix(skills): refuse a partial skill body on dashboard read (supersedes fix(skills): refuse a partial skill body on dashboard read #113) — the dashboard skill reader now refuses a truncated/partial skill body instead of serving it.

Testing

• make test_dispatch, make test_skill, GATEWAY=1 make test_gateway_http — passed
• CI=true make test — all 46 test binaries passed
• make static — passed

Supersedes #103 and #113.

[cursor]

Repo context. ShellClaw is a C99 AI agent for Jetson/Pi edge boards (core agent loop, providers, tools, channels, lws gateway with an embedded dashboard, ASAP client/server, sandbox). One test binary per module via Makefile; the shared SQLite is built SQLITE_THREADSAFE=0, so agent_lock() is the serialization point between the lws thread and everything else.

Summary. Two small fixes on top of #122. DELETE /api/sessions/:id now goes through a new dispatch_delete_session() that holds agent_lock around session_delete, and skill_get_content() now returns -2 instead of a clipped prefix, which the dashboard GET maps to HTTP 413 (also when the escaped JSON no longer fits the 64 KiB response buffer). The logic checks out: the boundary math in skill_get_content is right (sz >= out_size rejected, sz == out_size-1 accepted), and I linked skill.o by hand with gcc ... -lm and test_skill passes. I could not run test_gateway_http or test_dispatch here (no libcurl/libwebsockets in the image), so CI with GATEWAY=1 is the gate for those.

One note on the title: the base (4adbcf7) already had agent_lock() around session_delete in handle_session_delete. This PR moves that lock into a shared helper and adds a unit test for it. That is a fine refactor, but it is not a new race fix, so the description and CHANGELOG should say so.

Must Fix

• None.

Should Fix

1. No CHANGELOG.md entry. Unreleased > Fixed lists every sibling consolidation (#119 to #122); the 413 on an oversize skill GET is a user-visible API change and needs a line.
2. Skill create/update are not capped to what GET can serve (src/gateway/routes.c handle_skill_create/handle_skill_update, skill_create/skill_update). The new test itself POSTs a 40000 byte skill and gets 201, and from then on the dashboard can never open or edit it (413); only DELETE works. Refuse writes whose content length is >= SKILL_CONTENT_MAX with the same 413, so the UI cannot create a state it cannot read back. Otherwise the fix moves the data-loss trap from Save to Create.
3. See inline on skill.c: a file with an embedded NUL byte is reported as "too large".
4. -2 is a magic number shared by skill.h, skill.c, routes.c and two tests. A #define SKILL_ERR_TOO_LARGE (-2) (or enum) in skill.h keeps the contract greppable.

Nice to Have

• dispatch_delete_session blocks the lws service thread on agent_lock for the whole duration of an in-flight agent_run (provider round trips can take tens of seconds), so all HTTP/WS traffic stalls meanwhile. Pre-existing for the other dashboard handlers, so not for this PR, but worth a follow-up (try-lock with a 503, or defer the delete).
• web/js/app.js renderSkillView shows the raw {"error":"..."} text on 413. Parsing error out of the body would read better.
• test_api_skill_get_refuses_oversize: on any ASSERT failure big_skill.md stays on disk and content/json leak, which can affect later list tests. Free and delete before asserting, or use a cleanup label.
• Tests do not cover the second 413 branch in skill_get_send (raw file under 32 KiB but JSON-escaped output over 64 KiB, e.g. ~12000 \x01 bytes), nor the 32767/32768 byte edge through HTTP. test_skill covers the edge at the C level only.
• The session-delete test exercises the helper through the session_delete hook. There is no route-level DELETE /api/sessions/:id case in test_gateway_http, so the routes.c to dispatch wiring is unguarded if someone inlines the delete again.

Positive Highlights

• Tests use real tail markers (TAIL_MARKER_must_survive, SKILL_TAIL_KEEP) and check on disk that the file is untouched after the 413. That is the property that matters.
• skill_get_send catches the case where the cJSON_PrintUnformatted output would be clipped by the response buffer (a cut-off 200 parses as empty and Save wipes the file). That is a second path to the same data loss and easy to miss.
• skill.h documents the new return code with an example, the Makefile adds src/core/dispatch.h to the ROUTES_O deps so incremental builds do not use a stale header, and test_gateway_db_lock links dispatch.o through SHELLCLAW_OBJS, so the new dependency resolves.

  

_{Sent by Cursor Automation: Code review}

[cursor]

append_file_content reads with fgets and advances by strlen(buf), so a skill file that contains a NUL byte stops counting at the NUL and used < sz. That returns -2, and the dashboard then says the skill "exceeds the edit buffer" for a file of a few hundred bytes. It still fails closed (good), but the message is misleading. Consider a different code (or -1) when sz < out_size yet used != sz, and keep -2 for the real size case.

[cursor]

handle_skill_create (and handle_skill_update below) accept any content up to the 64 KiB request body cap, but GET now refuses anything >= SKILL_CONTENT_MAX (32768). A skill created at 33 KiB is unreadable in the dashboard from then on. Apply the same limit on write and return 413 with the same message shape.

[cursor]

Behaviour is unchanged here: the base already held agent_lock around session_delete. Fine as a refactor, but no route-level test hits this line. A DELETE /api/sessions/:id case in test_gateway_http (seed a session, delete, expect 200, then 404) would lock the wiring.