fix(cron): schedule validation and serialized SQLite access

[adriannoes]

Summary

Consolidates three cursor high-severity cron/gateway fixes as operator-authored commits:

• fix(gateway): serialize dashboard and cron SQLite access (supersedes fix(gateway): serialize dashboard and cron SQLite access #104) — the HTTP dashboard routes and cron share one SQLite connection (SQLITE_THREADSAFE=0); access is now serialized through the agent lock, dropped before the re-offer sleep so the dashboard thread is not stalled.
• fix(cron): reject interval schedules that overflow next_run (supersedes fix(cron): reject interval schedules that overflow next_run #105) — a span that does not fit in now+seconds used to wrap next_run negative, leaving the job due forever.
• fix(cron): reject schedule numbers that stop early (supersedes fix(cron): reject schedule numbers that stop early #111) — strto* accepted a numeric prefix: at:2026-10-08 stored next_run 2026 and fired immediately; comma lists matched only the first value.

Consolidation notes:

• The partial-number rejection from fix(cron): reject schedule numbers that stop early #111 was folded into the refactored interval_next_run() introduced by fix(cron): reject interval schedules that overflow next_run #105 (the two fixes touch the same parse path).
• In-scope fixes applied during consolidation: the new test target test_gateway_db_lock now compiles with $(GATEWAY_CFLAGS) (it failed to build on macOS/homebrew where libwebsockets headers are not on the default include path), and a cppcheck variableScope finding introduced by fix(gateway): serialize dashboard and cron SQLite access #104 was resolved by scoping the loop counter inside the for statement.

Testing

• make test_cron — passed (overflow + partial-number + serialization tests)
• GATEWAY=1 make test_gateway_db_lock — passed
• CI=true make test — all 46 test binaries passed
• make static — passed

Supersedes #104, #105 and #111.

[cursor]

Context. ShellClaw is a C99 edge agent (Jetson / Raspberry Pi) with SQLite memory/cron (SQLITE_THREADSAFE=0, one shared g_db), a libwebsockets dashboard thread, and a main-loop/channel thread serialized by a non-recursive agent_lock(). This PR consolidates #104 (serialize dashboard + cron SQLite), #105 (interval overflow) and #111 (partial-number schedules) into one branch.

I built test_cron with CC=gcc and linked it with -lm -pthread -ldl (this image has no libcurl). It passes. I could not run GATEWAY=1 test_gateway_db_lock here (no libwebsockets).

Summary. The cron parsing fixes are correct and well tested. The locking fix is incomplete: it adds agent_lock around most dashboard SQLite calls but misses one, DELETE /api/sessions/{id}. That leaves the same data race the PR sets out to close, and the new test cannot see it.

Must Fix

• src/gateway/routes.c:509-517 handle_session_delete still calls session_delete(id) with no agent_lock(). It runs on the lws thread against the same g_db as agent_run and cron_poll. dispatch.c:50-55 (/reset) already locks this exact call, so the dashboard path is the only unlocked session_delete caller. agent.h now says dashboard g_db handlers must hold the lock, and the PR title says "serialized SQLite access", so this is a gap against the stated contract. Fix: wrap it like the sibling handlers (rc = session_delete(id) under lock, then build the response after unlock).
• tests/test_gateway_db_lock.c: add DELETE /api/sessions/<id> to the route list. It would fail today. The probe lives in memory.c, but session_delete has no memory_sqlite_entry_probe() call, so the hook cannot observe it either. Add the probe there, otherwise the test passes while the bug is present. Also consider probing every g_db entry point (session_load, session_save, memory_save, cron_job_update_next_run) instead of a hand-picked subset, so a future unlocked caller trips the test.

Should Fix

• CHANGELOG.md [Unreleased] / Fixed has no entry for any of the three fixes (dashboard/cron SQLite serialization, interval overflow, partial-number schedules). Recent consolidations (#119 to #121) had the same gap.
• Behavior change for already-stored jobs: parse_field and interval_next_run now reject input that used to parse (interval:60abc, 0 9 1,15 * *, at:2026-...). cron_ack_delivery (cron.c, parse failure branch) silently moves such a recurring job to now + 365d. A job someone created with a comma list before this PR will stop firing for a year with no log line. Either log the job id and schedule on that fallback, or note the migration in the CHANGELOG. Rejecting comma lists at creation is right; the point is the existing rows.
• cron.c cron_pick_due_row and the dashboard handlers now hold agent_lock while doing SQL. agent_run holds that same lock for a whole LLM round trip, so during a long agent turn GET /api/cron, /api/memory and /api/sessions block the single lws service thread, which also stalls WebSocket and /health. /asap already behaves this way, so this is consistent with existing design, but it makes the dashboard visibly freeze. Worth a doc line, or a follow-up to give SQLite its own mutex instead of reusing the agent lock.
• handle_cron_create holds the lock across cron_create_job, which for a cron expression can scan up to 3662460 minutes with localtime_r per step. Parse/validate first and take the lock only for cron_job_create. Minor on desktop, but it is measurable on a Pi.

Nice to Have

• at: still accepts ts <= 0 (e.g. at:-5, at:0), which is due immediately. Consider rejecting ts <= 0 the way interval: rejects sec <= 0.
• parse_field still uses strtol, which accepts leading whitespace and +/- signs (+5, 5). Harmless after the *end check, but a digit-only check would match the strictness the comments describe.
• test_schedule_rejects_partial_numbers covers at:, interval: and comma lists but not cron:0 9 * * 1-5x or cron:*/5 * * * * (step syntax is also rejected now, via the same path). A one-line assert for step syntax documents that it is unsupported.
• memory_sqlite_entry_set_hook_for_test is compiled into production builds, like the two existing test hooks. Fine for now, but a #ifdef SHELLCLAW_TEST would keep it out of the shipped binary.
• Makefile: test_cron now links agent.o, skill.o, config.o, toml.o and provider_common.o, and the test defines a stub provider_router_active_backend_snapshot. That is a heavy link for a cron unit test, and the stub will go stale if the agent adds more router symbols. Acceptable, but it is the first thing to break when agent.c grows.

Positive Highlights

• interval_next_run is small and correct: errno == ERANGE, end check, sec <= 0, and sec > LLONG_MAX - now as a pre-check rather than a post-hoc overflow test. Folding #111 into the #105 refactor avoided two overlapping edits of the same parse path.
• cron_pick_due_row releases the lock before the re-offer sleep and keeps the early-return paths balanced (agent_unlock() before return 1, and once on the fall-through). I found no lock leak or double unlock, and row is a heap-owned copy so using it after unlock is safe.
• Tests assert the real outcomes rather than just return codes: the overflow test checks that ack lands at now + 365d, the parse test checks next is untouched on failure, and both lock tests check the mutex is released afterwards. The wsi == NULL guard in dispatch_route that makes the route testable without a socket is a clean small change.

  

_{Sent by Cursor Automation: Code review}

[adriannoes]

Review disposition (all Must Fix / Should Fix / Nice to Have items):

Must Fix

• Unlocked session_delete() in handle_session_delete (DELETE /api/sessions/{id}), and the missing memory_sqlite_entry_probe() in session_delete: Fixed in 9347780 fix(gateway): serialize session deletion with the agent lock. The call is now wrapped in agent_lock()/agent_unlock() like the sibling handlers (response built after unlock); session_delete calls memory_sqlite_entry_probe() like the other probed entry points; DELETE /api/sessions/<id> added to tests/test_gateway_db_lock.c (seeds via session_save, asserts 200). Verified red first: probe present + lock missing → FAIL: tests/test_gateway_db_lock.c:42 g_sqlite_unlocked == 0; green after the fix.
• "Also consider probing every g_db entry point (session_load, session_save, cron_job_update_next_run)": Not changed beyond session_delete. The current hook set covers the races this PR fixes plus the new session-delete test route; expanding probes further is a follow-up as remaining entry points gain test routes.

Should Fix

• CHANGELOG.md [Unreleased] / Fixed gap: Fixed in 97a4a92 docs(changelog): record the cron schedule and SQLite serialization fixes — bullets for the dashboard+cron SQLite serialization (including session deletion), interval-overflow rejection, partial-number rejection, and the stored-jobs snooze behavior.
• Silent 365-day snooze for stored jobs with now-rejected schedules: Fixed in c91904d fix(cron): log the job snoozed on a rejected stored schedule — cron_ack_delivery now logs job id and schedule (shellclaw: cron: house style) in the parse-failure branch; fail-close behavior unchanged. The changelog entry also notes the behavior change for legacy rows.
• Dashboard freeze during long agent turns / dedicated SQLite mutex: Not changed. Dashboard g_db handlers holding agent_lock() matches the existing /asap design this PR extends; an own-mutex redesign is a scoped follow-up, not a patch on this branch.
• handle_cron_create lock scope (validate before the lock): Not changed. Lock-scope micro-optimization; hoisting validation means splitting cron_create_job's contract (parse + insert) shared with the cron tool path, so it is not a tiny clearly-correct change here. Durable fix is the dedicated SQLite mutex follow-up.

Nice to Have — Not changed, all five: at: ts <= 0 rejection, digit-only parse_field, step-syntax test assert, #ifdef SHELLCLAW_TEST for the probe hook, test_cron link weight. Incremental hardening outside this PR's smallest correctness patch — follow-ups.