diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fd75d6..c20031b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,28 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha ## [Unreleased] +### Fixed +- Provider model ids keep their vendor prefix in config but are stripped per backend before the request: `anthropic/claude-...` no longer reaches the Anthropic API (HTTP 4xx) or OpenAI after a 5xx, so the fallback chain runs through to local. +- WebChat writes are moved off the agent thread: frames are queued and the lws service thread is woken (`lws_cancel_service`), marking writable only from `EVENT_WAIT_CANCELLED`, which stops the poll-set race against a closing socket. +- Sandboxed shell commands no longer inherit the agent environment: the command process keeps only `PATH`, locale (`LANG`, `LC_ALL`, `LC_CTYPE`), `TERM`, `TZ`, and `HOME` (set to the workspace), so a prompt-injected `printenv` cannot read provider API keys or bot tokens. `LD_*`, `CUDA_*`, and proxy variables are dropped before `exec`; see the environment model in `docs/SECURITY.md` (#109). +- The shell command timeout is enforced against a monotonic wall clock on both the sandboxed and the unsandboxed paths, so a command that keeps stdout trickling can no longer outlive `shell_timeout_sec` and hold the agent lock until the output buffer fills (#116). +- Telegram `getUpdates` requests a single update per poll and skips a message without a numeric sender id, so a backlog larger than the 256 KiB read buffer (or a channel/anonymous post) no longer pins the poll offset and stalls the inbox. +- Telegram `sendMessage` delivery is verified from the HTTP status code and the `ok` body field, so an API refusal (HTTP 400/429) is no longer reported as a successful send and acknowledged as delivered. +- Long Telegram and Discord replies are chunked to the platform limits (4096/2000 bytes, UTF-8 boundary-safe) through the shared `channel_send_chunked` helper, so a reply over the cap is delivered as several messages instead of failing the whole send; chunk posts are paced for the ~1 msg/s Telegram chat rate, and a failed mid-reply post logs its chunk index and byte offset. +- Live config reload reaches the Anthropic provider (`provider_anthropic_set_live_config`, guarded like OpenAI), so dashboard/SIGHUP reloads update model and `max_tokens` instead of staying on the config captured at init. +- Parallel tool results are grouped into one `user` message (one `tool_result` block per call, per the Messages API contract), fixing Anthropic 400s on parallel tool calls. +- The local provider re-probes on the next request after an unreachable startup, at most once every 15 seconds, so a down `llama-server` no longer costs synchronous curl probes on every agent turn. +- Dashboard and cron SQLite access is serialized with `agent_lock()` (matching `POST /asap`), including `DELETE /api/sessions/{id}` session deletion, so the lws service thread cannot race `agent_run` / `cron_poll` on the shared `g_db` (`SQLITE_THREADSAFE=0`). +- `interval:` schedules reject intervals that do not fit `now + seconds` and numbers that stop parsing early (`interval:60abc`), instead of wrapping to a past `next_run` that stays due and re-fires forever. +- Cron schedules reject partial numbers instead of silently accepting the numeric prefix: `at:2026-10-08` (would become 2026), comma lists such as `0 9 1,15 * *`, and step syntax are now parse errors. +- Stored recurring jobs whose schedule the stricter parser now rejects no longer disappear silently: on ack they are snoozed to `now + 365d` and the job id and schedule are logged (`shellclaw: cron:`), so the year-long gap is diagnosable. +- Session compaction includes the boundary turn (index `msg_count - max_context_messages`) in the summary window, so the turn at the keep-tail boundary is not deleted without being summarized. +- Session compaction keeps a message that does not fit the 64 KiB summary source, and everything after it, instead of skipping the long line and deleting it anyway. +- Session compaction no longer persists the summarized transcript before the user turn finishes: a failed follow-up provider call leaves the raw stored session unchanged, and when the kept tail already starts with a user turn the summary is folded into that turn so consecutive user roles cannot break the next Anthropic call (supersedes #126). +- Session compaction summarizes a message larger than the summary source as a truncated `[truncated]` stub, so a single oversized line can no longer abort the pass with an empty summary source or keep the session above `max_context_messages` forever. +- WebChat `ws_send_to` drops a send whose session has no live `wsi` (still returning 0) and `ws_unregister_conn` drops frames still queued for the closing session, so ~64 abandoned replies cannot pin the shared outgoing queue at `MAX_QUEUE` (64) and stall every live client until restart. +- `read_file` reads that do not fit the result buffer now fail with a JSON error instead of returning a clipped prefix, so a later `write_file` cannot persist a silently truncated file. + ## [1.0.0] - 2026-09-25 ### Fixed diff --git a/Makefile b/Makefile index f40f391..7280bcb 100644 --- a/Makefile +++ b/Makefile @@ -127,7 +127,12 @@ ASAP_INVOKE_TEST_O := $(BINDIR)/asap_invoke_test.o # Sandbox (Phase 3 §5) SANDBOX_CORE_O := src/sandbox/sandbox.o SANDBOX_LANDLOCK_O := src/sandbox/sandbox_landlock.o -SANDBOX_O := $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) +SANDBOX_ENV_O := src/sandbox/sandbox_env.o +# Sandbox unit test object: exposes drain_pipe helpers (SHELLCLAW_SANDBOX_TEST) +# so coverage does not depend on namespace privileges the CI runners revoke. +# Lives under src/sandbox/ so the coverage.sh capture (--directory src) merges it. +SANDBOX_TEST_O := src/sandbox/sandbox_test.o +SANDBOX_O := $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) ALLOWLIST_SCAN_O := src/sandbox/allowlist.o ALLOWLIST_PATH_O := src/sandbox/allowlist_path.o ALLOWLIST_O := $(ALLOWLIST_SCAN_O) $(ALLOWLIST_PATH_O) @@ -372,7 +377,7 @@ $(HTTP_O): src/gateway/http.c src/gateway/http.h src/gateway/http_lws.h src/gate $(HTTP_LWS_O): src/gateway/http_lws.c src/gateway/http_lws.h src/gateway/asap_http_body.h src/gateway/routes.h src/gateway/auth.h src/gateway/static.h src/gateway/ws.h $(CC) $(CFLAGS) $(INC) $(GATEWAY_CFLAGS) -pthread -c -o $@ src/gateway/http_lws.c -$(ROUTES_O): src/gateway/routes.c src/gateway/routes.h src/gateway/routes_hardware.h src/gateway/http.h src/gateway/http_lws.h src/gateway/auth.h src/gateway/rate_limit.h src/tools/context.h src/asap/manifest.h src/asap/envelope.h src/asap/server.h src/asap/log.h src/core/bootstrap.h src/core/agent.h src/core/config.h src/core/config_patch.h src/core/reload.h src/core/memory.h src/core/skill.h src/providers/provider.h src/channels/channel.h src/tools/cron.h src/tools/tool.h +$(ROUTES_O): src/gateway/routes.c src/gateway/routes.h src/gateway/routes_hardware.h src/gateway/http.h src/gateway/http_lws.h src/gateway/auth.h src/gateway/rate_limit.h src/tools/context.h src/asap/manifest.h src/asap/envelope.h src/asap/server.h src/asap/log.h src/core/bootstrap.h src/core/agent.h src/core/config.h src/core/config_patch.h src/core/dispatch.h src/core/reload.h src/core/memory.h src/core/skill.h src/providers/provider.h src/channels/channel.h src/tools/cron.h src/tools/tool.h $(CC) $(CFLAGS) $(INC) $(GATEWAY_CFLAGS) -pthread -c -o $@ src/gateway/routes.c $(ROUTES_HARDWARE_O): src/gateway/routes_hardware.c src/gateway/routes_hardware.h src/gateway/routes.h src/gateway/http_lws.h src/gateway/uri_match.h src/hardware/hardware.h src/hardware/hardware_gpio_snapshot.h src/hardware/hardware_tegrastats.h src/hardware/board_detect.h src/core/config.h @@ -385,9 +390,15 @@ $(SHELL_O): src/tools/shell.c src/tools/tool.h src/tools/shell.h src/core/config src/sandbox/sandbox.h src/sandbox/allowlist.h $(CC) $(CFLAGS) $(INC) -c -o $@ src/tools/shell.c -$(SANDBOX_CORE_O): src/sandbox/sandbox.c src/sandbox/sandbox.h src/sandbox/sandbox_landlock.h +$(SANDBOX_CORE_O): src/sandbox/sandbox.c src/sandbox/sandbox.h src/sandbox/sandbox_landlock.h src/sandbox/sandbox_env.h $(CC) $(CFLAGS) $(INC) -c -o $@ src/sandbox/sandbox.c +$(SANDBOX_ENV_O): src/sandbox/sandbox_env.c src/sandbox/sandbox_env.h + $(CC) $(CFLAGS) $(INC) -c -o $@ src/sandbox/sandbox_env.c + +$(SANDBOX_TEST_O): src/sandbox/sandbox.c src/sandbox/sandbox.h src/sandbox/sandbox_landlock.h src/sandbox/sandbox_env.h + $(CC) $(CFLAGS) $(INC) -DSHELLCLAW_SANDBOX_TEST -c -o $@ src/sandbox/sandbox.c + $(SANDBOX_LANDLOCK_O): src/sandbox/sandbox_landlock.c src/sandbox/sandbox_landlock.h $(CC) $(CFLAGS) $(INC) -c -o $@ src/sandbox/sandbox_landlock.c @@ -467,7 +478,7 @@ $(HARDWARE_I2C_O): src/hardware/hardware_i2c.c src/hardware/hardware_i2c.h $(HARDWARE_CAMERA_O): src/hardware/hardware_camera.c src/hardware/hardware_camera.h src/hardware/board_detect.h $(CC) $(CFLAGS) $(INC) -c -o $@ src/hardware/hardware_camera.c -$(CRON_O): src/tools/cron.c src/tools/cron.h src/crypto/crypto.h src/core/memory.h src/channels/channel.h src/core/config.h +$(CRON_O): src/tools/cron.c src/tools/cron.h src/crypto/crypto.h src/core/agent.h src/core/memory.h src/channels/channel.h src/core/config.h $(CC) $(CFLAGS) $(INC) -c -o $@ src/tools/cron.c $(ASAP_INVOKE_O): src/tools/asap_invoke.c src/tools/asap_invoke.h src/tools/tool.h src/asap/envelope.h src/asap/client.h src/asap/registry.h src/asap/ulid.h src/asap/asap_version.h src/core/config.h vendor/cJSON/cJSON.h @@ -631,9 +642,10 @@ test_web_search: tests/test_web_search.c $(WEBSEARCH_O) $(CONFIG_O) $(TOML_O) $( $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_web_search.c $(WEBSEARCH_O) $(CONFIG_O) $(TOML_O) $(CJSON_O) $(LDLIBS) $(DSYM_SCRIPT) -test_cron: tests/test_cron.c $(CRON_O) $(CRYPTO_LINK) $(MEMORY_O) $(SQLITE3_O) $(CHANNEL_COMMON_O) $(CJSON_O) +test_cron: tests/test_cron.c $(CRON_O) $(CRYPTO_LINK) $(MEMORY_O) $(SQLITE3_O) $(CHANNEL_COMMON_O) $(CJSON_O) $(AGENT_O) $(SKILL_O) $(CONFIG_O) $(TOML_O) $(PROVIDER_COMMON_O) @mkdir -p $(BINDIR) - $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_cron.c $(CRON_O) $(CRYPTO_LINK) $(MEMORY_O) $(SQLITE3_O) $(CHANNEL_COMMON_O) $(CJSON_O) $(LDLIBS) + $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_cron.c $(CRON_O) $(CRYPTO_LINK) $(MEMORY_O) $(SQLITE3_O) $(CHANNEL_COMMON_O) $(CJSON_O) $(AGENT_O) $(SKILL_O) $(CONFIG_O) $(TOML_O) $(PROVIDER_COMMON_O) $(LDLIBS) -pthread + $(DSYM_SCRIPT) test_ws: tests/test_ws.c $(WS_TEST_O) @mkdir -p $(BINDIR) @@ -735,6 +747,18 @@ test_gateway_http: shellclaw tests/test_gateway_http.c $(AUTH_O) $(CONFIG_O) $(T $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -DSHELLCLAW_GATEWAY -pthread -o $(BINDIR)/$@ tests/test_gateway_http.c $(AUTH_O) $(CRYPTO_LINK) $(CONFIG_O) $(TOML_O) $(CJSON_O) $(LDLIBS) -pthread $(DSYM_SCRIPT) +GATEWAY_DB_LOCK_OBJS := $(filter-out src/core/main.o,$(SHELLCLAW_OBJS)) + +test_gateway_db_lock: tests/test_gateway_db_lock.c $(GATEWAY_DB_LOCK_OBJS) + @mkdir -p $(BINDIR) + @if [ "$(GATEWAY)" != "1" ]; then \ + if [ "$(CI)" = "true" ]; then echo "test_gateway_db_lock: GATEWAY=0 in CI — install libwebsockets-dev"; exit 1; fi; \ + echo "test_gateway_db_lock: skipped (GATEWAY=0)"; \ + else \ + $(CC) $(CFLAGS) $(LDFLAGS) $(INC) $(GATEWAY_CFLAGS) -DSHELLCLAW_GATEWAY -pthread -o $(BINDIR)/$@ tests/test_gateway_db_lock.c $(GATEWAY_DB_LOCK_OBJS) $(LDLIBS) $(GATEWAY_LDLIBS) $(LIBGPIOD_LDLIBS) -pthread; \ + fi + $(DSYM_SCRIPT) + test_routes_hardware: tests/test_routes_hardware.c tests/test_routes_json_stub.c $(ROUTES_HARDWARE_O) $(HARDWARE_GPIO_SNAPSHOT_O) $(HARDWARE_TEGRASTATS_O) $(HARDWARE_INIT_O) $(HARDWARE_STUB_O) $(BOARD_DETECT_O) $(HARDWARE_I2C_O) $(HARDWARE_CAMERA_O) $(HARDWARE_LIBGPIOD_O) $(CONFIG_O) $(TOML_O) $(CJSON_O) @if [ "$(GATEWAY)" != "1" ]; then \ if [ "$(CI)" = "true" ]; then echo "test_routes_hardware: GATEWAY=0 in CI — install libwebsockets-dev"; exit 1; fi; \ @@ -750,9 +774,9 @@ test_static: tests/test_static.c src/gateway/ui_assets.h src/gateway/static.o $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_static.c src/gateway/static.o $(LDLIBS) $(DSYM_SCRIPT) -test_sandbox: tests/test_sandbox.c $(SANDBOX_O) +test_sandbox: tests/test_sandbox.c $(SANDBOX_TEST_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) @mkdir -p $(BINDIR) - $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_sandbox.c $(SANDBOX_O) $(LDLIBS) + $(CC) $(CFLAGS) $(LDFLAGS) $(INC) -o $(BINDIR)/$@ tests/test_sandbox.c $(SANDBOX_TEST_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) $(LDLIBS) $(DSYM_SCRIPT) test_allowlist: tests/test_allowlist.c $(ALLOWLIST_O) @@ -889,6 +913,7 @@ test: test_config test_config_patch test_memory test_skill test_provider test_an $(MAKE) test_static && $(BINDIR)/test_static @if [ "$(GATEWAY)" = "1" ]; then \ $(BINDIR)/test_routes_hardware && \ + $(MAKE) test_gateway_db_lock && $(BINDIR)/test_gateway_db_lock && \ $(MAKE) test_asap_http_body && $(BINDIR)/test_asap_http_body && \ $(MAKE) test_gateway_http && $(BINDIR)/test_gateway_http; \ elif [ "$(CI)" = "true" ]; then echo "GATEWAY=0 in CI — install libwebsockets-dev"; exit 1; fi @@ -909,9 +934,9 @@ clean-root-dsym: @rm -f shellclaw test_agent test_anthropic test_channel test_cli test_config test_file test_memory test_local_provider test_openai test_provider test_router test_shell test_skill test_telegram test_web_search test_ws clean: clean-root-dsym - rm -f $(OBJS) $(PROVIDER_COMMON_O) $(STUB_O) $(ANTHROPIC_O) $(OPENAI_COMPAT_O) $(OPENAI_O) $(LOCAL_O) $(ROUTER_O) $(CJSON_O) $(TWEETNACL_O) $(ANTHROPIC_TEST_O) $(OPENAI_TEST_O) $(LOCAL_TEST_O) $(CONTEXT_TEST_OBJS) $(HEARTBEAT_TEST_O) $(CHANNEL_TG_TEST_O) $(CHANNEL_COMMON_O) $(CHANNEL_STUB_O) $(CHANNEL_CLI_O) $(CHANNEL_TG_O) $(CHANNEL_DISCORD_O) $(DISCORD_HELPERS_O) $(CHANNEL_HEARTBEAT_O) $(CHANNEL_WEBCHAT_O) $(AUTH_O) $(STATIC_O) $(HTTP_O) $(HTTP_LWS_O) $(ASAP_HTTP_BODY_O) $(ROUTES_O) $(ROUTES_HARDWARE_O) $(WS_O) $(MANIFEST_O) $(MANIFEST_PROFILES_O) $(MANIFEST_BUILD_O) $(MANIFEST_SIGN_O) $(MANIFEST_KEYS_O) $(ENVELOPE_O) $(ULID_O) $(CLIENT_O) $(ASAP_REGISTRY_O) $(SERVER_O) $(ASAP_LOG_O) $(RATE_LIMIT_O) $(SHELL_O) $(WEBSEARCH_O) $(FILE_O) $(REGISTRY_O) $(CONTEXT_O) $(CONTEXT_CACHE_O) $(CONTEXT_HTTP_O) $(CONTEXT_GEO_O) $(CRYPTO_O) $(JCS_O) $(HARDWARE_STUB_O) $(HARDWARE_INIT_O) $(HARDWARE_GPIO_SNAPSHOT_O) $(HARDWARE_TEGRASTATS_O) $(HARDWARE_TOOLS_O) $(BOARD_DETECT_O) src/hardware/hardware_libgpiod.o $(HARDWARE_I2C_O) $(HARDWARE_CAMERA_O) $(CRON_O) $(ASAP_INVOKE_O) $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) $(ALLOWLIST_SCAN_O) $(ALLOWLIST_PATH_O) + rm -f $(OBJS) $(PROVIDER_COMMON_O) $(STUB_O) $(ANTHROPIC_O) $(OPENAI_COMPAT_O) $(OPENAI_O) $(LOCAL_O) $(ROUTER_O) $(CJSON_O) $(TWEETNACL_O) $(ANTHROPIC_TEST_O) $(OPENAI_TEST_O) $(LOCAL_TEST_O) $(CONTEXT_TEST_OBJS) $(HEARTBEAT_TEST_O) $(CHANNEL_TG_TEST_O) $(CHANNEL_COMMON_O) $(CHANNEL_STUB_O) $(CHANNEL_CLI_O) $(CHANNEL_TG_O) $(CHANNEL_DISCORD_O) $(DISCORD_HELPERS_O) $(CHANNEL_HEARTBEAT_O) $(CHANNEL_WEBCHAT_O) $(AUTH_O) $(STATIC_O) $(HTTP_O) $(HTTP_LWS_O) $(ASAP_HTTP_BODY_O) $(ROUTES_O) $(ROUTES_HARDWARE_O) $(WS_O) $(MANIFEST_O) $(MANIFEST_PROFILES_O) $(MANIFEST_BUILD_O) $(MANIFEST_SIGN_O) $(MANIFEST_KEYS_O) $(ENVELOPE_O) $(ULID_O) $(CLIENT_O) $(ASAP_REGISTRY_O) $(SERVER_O) $(ASAP_LOG_O) $(RATE_LIMIT_O) $(SHELL_O) $(WEBSEARCH_O) $(FILE_O) $(REGISTRY_O) $(CONTEXT_O) $(CONTEXT_CACHE_O) $(CONTEXT_HTTP_O) $(CONTEXT_GEO_O) $(CRYPTO_O) $(JCS_O) $(HARDWARE_STUB_O) $(HARDWARE_INIT_O) $(HARDWARE_GPIO_SNAPSHOT_O) $(HARDWARE_TEGRASTATS_O) $(HARDWARE_TOOLS_O) $(BOARD_DETECT_O) src/hardware/hardware_libgpiod.o $(HARDWARE_I2C_O) $(HARDWARE_CAMERA_O) $(CRON_O) $(ASAP_INVOKE_O) $(SANDBOX_CORE_O) $(SANDBOX_LANDLOCK_O) $(SANDBOX_ENV_O) $(ALLOWLIST_SCAN_O) $(ALLOWLIST_PATH_O) rm -f src/gateway/ui_assets.h find . -name '*.gcno' -o -name '*.gcda' -o -name '*.gcov' | xargs rm -f 2>/dev/null || true - rm -f $(WS_TEST_O) $(BINDIR)/asap_registry_test.o $(BINDIR)/asap_invoke_test.o $(CONTEXT_TEST_OBJS) $(HEARTBEAT_TEST_O) $(BINDIR)/shellclaw $(BINDIR)/test_tweetnacl_smoke $(BINDIR)/test_config $(BINDIR)/test_config_patch $(BINDIR)/test_memory $(BINDIR)/test_skill $(BINDIR)/test_provider $(BINDIR)/test_anthropic $(BINDIR)/test_openai $(BINDIR)/test_local_provider $(BINDIR)/test_router $(BINDIR)/test_heartbeat $(BINDIR)/test_crypto $(BINDIR)/test_hardware_stub $(BINDIR)/test_board_detect $(BINDIR)/test_hardware_libgpiod $(BINDIR)/test_hardware_i2c $(BINDIR)/test_hardware_camera $(BINDIR)/test_pin_tables $(BINDIR)/test_hardware_init $(BINDIR)/test_hardware_tools $(BINDIR)/test_registry $(BINDIR)/test_ws $(BINDIR)/test_agent $(BINDIR)/test_channel $(BINDIR)/test_cli $(BINDIR)/test_shell $(BINDIR)/test_file $(BINDIR)/test_telegram $(BINDIR)/test_discord_helpers $(BINDIR)/test_web_search $(BINDIR)/test_cron $(BINDIR)/test_context $(BINDIR)/test_manifest_build $(BINDIR)/test_manifest_keys $(BINDIR)/test_jcs $(BINDIR)/test_asap_envelope $(BINDIR)/test_asap_ulid $(BINDIR)/test_asap_client $(BINDIR)/test_asap_registry $(BINDIR)/test_asap_server $(BINDIR)/test_asap_invoke $(BINDIR)/test_asap_log $(BINDIR)/test_auth $(BINDIR)/test_gateway_http $(BINDIR)/test_static $(BINDIR)/test_sandbox $(BINDIR)/test_allowlist $(BINDIR)/test_rate_limit + rm -f $(WS_TEST_O) $(SANDBOX_TEST_O) $(BINDIR)/asap_registry_test.o $(BINDIR)/asap_invoke_test.o $(CONTEXT_TEST_OBJS) $(HEARTBEAT_TEST_O) $(BINDIR)/shellclaw $(BINDIR)/test_tweetnacl_smoke $(BINDIR)/test_config $(BINDIR)/test_config_patch $(BINDIR)/test_memory $(BINDIR)/test_skill $(BINDIR)/test_provider $(BINDIR)/test_anthropic $(BINDIR)/test_openai $(BINDIR)/test_local_provider $(BINDIR)/test_router $(BINDIR)/test_heartbeat $(BINDIR)/test_crypto $(BINDIR)/test_hardware_stub $(BINDIR)/test_board_detect $(BINDIR)/test_hardware_libgpiod $(BINDIR)/test_hardware_i2c $(BINDIR)/test_hardware_camera $(BINDIR)/test_pin_tables $(BINDIR)/test_hardware_init $(BINDIR)/test_hardware_tools $(BINDIR)/test_registry $(BINDIR)/test_ws $(BINDIR)/test_agent $(BINDIR)/test_channel $(BINDIR)/test_cli $(BINDIR)/test_shell $(BINDIR)/test_file $(BINDIR)/test_telegram $(BINDIR)/test_discord_helpers $(BINDIR)/test_web_search $(BINDIR)/test_cron $(BINDIR)/test_context $(BINDIR)/test_manifest_build $(BINDIR)/test_manifest_keys $(BINDIR)/test_jcs $(BINDIR)/test_asap_envelope $(BINDIR)/test_asap_ulid $(BINDIR)/test_asap_client $(BINDIR)/test_asap_registry $(BINDIR)/test_asap_server $(BINDIR)/test_asap_invoke $(BINDIR)/test_asap_log $(BINDIR)/test_auth $(BINDIR)/test_gateway_http $(BINDIR)/test_gateway_db_lock $(BINDIR)/test_static $(BINDIR)/test_sandbox $(BINDIR)/test_allowlist $(BINDIR)/test_rate_limit rm -rf $(BINDIR)/*.dSYM $(DSYMDIR) rm -f $(BOOTSTRAP_DISPATCH_STUB_O) $(TOOL_RELOAD_STUB_O) $(RELOAD_CHANNEL_STUB_O) $(HTTP_RELOAD_STUB_O) diff --git a/config.example.toml b/config.example.toml index 6ab83de..0bde621 100644 --- a/config.example.toml +++ b/config.example.toml @@ -2,6 +2,8 @@ # Copy to ~/.shellclaw/config.toml and edit [agent] +# Gateway-style vendor prefix. Anthropic receives the id after "anthropic/". +# A foreign cloud id is skipped without HTTP 4xx so local fallback still runs. model = "anthropic/claude-sonnet-4-20250514" max_tokens = 4096 temperature = 0.7 diff --git a/docs/LOCAL_INFERENCE.md b/docs/LOCAL_INFERENCE.md index 0c76275..bf3cde9 100644 --- a/docs/LOCAL_INFERENCE.md +++ b/docs/LOCAL_INFERENCE.md @@ -18,7 +18,7 @@ llama-server (127.0.0.1:8080) GGUF model on disk (/var/lib/shellclaw/models/) ``` -At startup, `local.c` probes `GET /health` and `GET /v1/models`. If unreachable, the local provider marks itself unavailable and the router falls through to the next entry in `providers.fallback_chain`. +At startup, `local.c` probes `GET /health` and `GET /v1/models`. If unreachable, the local provider marks itself unavailable and the router falls through to the next entry in `providers.fallback_chain`. After an unreachable startup the provider re-probes on the next request, at most once every 15 seconds, so a `llama-server` that comes up later can still serve. --- diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 4a2b3fd..c6ac5e1 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -39,8 +39,8 @@ The primary goals are: prevent sandboxed shell commands from escaping to host de | Surface | Mechanism | Notes | |---------|-----------|-------| -| Shell (sandbox on) | `fork()` + user ns when needed + `unshare(CLONE_NEWNS \| CLONE_NEWNET \| CLONE_NEWPID)` + fork for PID 1 + Landlock workspace bound + `prctl(PR_SET_NO_NEW_PRIVS)` | Fail-closed if namespaces or Landlock cannot apply. Isolation uses a control pipe, not `sh` exit 122/123. See [Linux sandbox (Jetson)](#linux-sandbox-jetson) | -| Shell (sandbox off) | Plain `fork()` + substring fallback blocklist | **Not** a security boundary; stderr warning | +| Shell (sandbox on) | `fork()` + user ns when needed + `unshare(CLONE_NEWNS \| CLONE_NEWNET \| CLONE_NEWPID)` + fork for PID 1 + Landlock workspace bound + `prctl(PR_SET_NO_NEW_PRIVS)` + secret-free environment | Fail-closed if namespaces or Landlock cannot apply. The command child keeps `PATH`, locale, `TERM`, `TZ`, and `HOME` set to the workspace. Host API keys and bot tokens are not inherited. Isolation uses a control pipe, not `sh` exit 122/123. See [Linux sandbox (Jetson)](#linux-sandbox-jetson) | +| Shell (sandbox off) | Plain `fork()` + substring fallback blocklist | **Not** a security boundary; the full host environment is inherited — `printenv` works there (see [Command environment model](#command-environment-model)); stderr warning | | Allowlist | Substring blocklist + workspace containment (ancestor walk, quoted/embedded `/` `~`, `$HOME`/`$PWD`, `file:` URLs, relative names) | Defense-in-depth string scan; Landlock is the kernel host-FS bound | | Landlock | Ruleset on configured `workspace_path` (RW workspace + traverse-only `/` + RO `/bin` `/usr` `/lib` and `/etc/ssl/certs` only; RW `/dev/null`, `/dev/zero`, `/dev/urandom`, `/dev/tty` without `IOCTL_DEV`). Child `fchdir`s the workspace before `restrict_self`. A failed `landlock_add_rule` fails the ruleset closed. | Primary host-FS gate; `/` is `READ_DIR` only so `/etc/passwd` stays closed. `/etc/ssl/private` is not granted. Blocks symlink and `chr(47)+` host reads | | cgroups v2 | `memory.max`, `cpu.max`; isolator writes `cgroup.procs` before the command fork | Best-effort; non-fatal if cgroup write fails | @@ -48,6 +48,19 @@ The primary goals are: prevent sandboxed shell commands from escaping to host de Implementation: [`src/sandbox/sandbox.c`](../src/sandbox/sandbox.c), [`src/sandbox/allowlist.c`](../src/sandbox/allowlist.c). +### Command environment model + +The two shell paths differ in environment exposure as much as in isolation, so the model is stated explicitly: + +| Path | Environment the command sees | `printenv` inside the command | +|------|------------------------------|------------------------------| +| Sandboxed (`sandbox.enabled = true`) | Scrubbed whitelist only: `PATH`, `LANG`, `LC_ALL`, `LC_CTYPE`, `TERM`, and `TZ` keep their host values when set, and `HOME` is set to the configured workspace (when one is set). Everything else is dropped (`clearenv` before `exec`). | Only the whitelist: provider keys, bot tokens, and loader variables are gone. | +| Unsandboxed fallback (`sandbox.enabled = false`, the default when the key is omitted) | The full agent (host) environment is inherited. | Works — it prints the agent's environment, secrets included. Intentional: this path is reduced isolation, not a security boundary; a stderr warning is printed per command. | + +Jetson-relevant variables that deliberately do **not** pass into the sandbox: `LD_LIBRARY_PATH` and all other `LD_*` (including `LD_PRELOAD`), `CUDA_HOME` and other `CUDA_*`, `TMPDIR`, `SSL_CERT_FILE`, and proxy variables (`http_proxy`, `https_proxy`, `no_proxy`, …). Sandboxed workloads that rely on them must set them per command (for example `LD_LIBRARY_PATH=… cmd`); there is no `[sandbox]` passthrough list in v1.0. Tool paths that genuinely need host setup (camera capture) run argv-only `execvp` in the agent process, outside the shell sandbox. + +Implementation: [`src/sandbox/sandbox_env.c`](../src/sandbox/sandbox_env.c). + --- ## Linux sandbox (Jetson) diff --git a/src/channels/channel.h b/src/channels/channel.h index 73b4cd7..cb64cb8 100644 --- a/src/channels/channel.h +++ b/src/channels/channel.h @@ -37,6 +37,65 @@ typedef struct channel_incoming_msg { /** Clear and free incoming message fields. Safe to call repeatedly or on zeroed. */ void channel_incoming_msg_clear(channel_incoming_msg_t *msg); +/** + * Byte length of the next outbound piece of @p text. + * The piece is at most @p max_bytes and stops before a multibyte UTF-8 + * character that would not fit, so chat APIs do not see a split code point. + * Fallback: when the first rune is wider than @p max_bytes, the piece is + * @p max_bytes bytes (that rune is split); unreachable with the 2000/4096 + * channel caps, documented so the finite cut is not mistaken for a bug. + * + * Example: `channel_outbound_chunk_len("hello", 2)` returns 2. + * + * @param text NUL-terminated text. NULL returns 0. + * @param max_bytes Maximum piece size in bytes. 0 returns 0. + * @return Byte length of the next piece, or 0 when @p text is empty. + */ +size_t channel_outbound_chunk_len(const char *text, size_t max_bytes); + +/** + * Post one chunk of a channel_send_chunked() send. NUL-terminated piece, + * never empty and never longer than the max_bytes of the driving call. + * @param piece NUL-terminated text piece (valid until the callback returns). + * @param ctx Caller context passed through unchanged. May be NULL. + * @return 0 when the platform accepted the piece, non-zero on failure. + */ +typedef int (*channel_chunk_post_fn)(const char *piece, void *ctx); + +/** Position where a chunked send stopped; zeroed by the call unless it failed. */ +typedef struct channel_chunk_fail { + size_t index; /**< Zero-based index of the chunk that failed to post. */ + size_t offset; /**< Byte offset of the failed chunk inside the sent text. */ +} channel_chunk_fail_t; + +/** + * Deliver @p text via @p post in chunks of at most @p max_bytes, cut on UTF-8 + * rune boundaries via channel_outbound_chunk_len(). Each piece is copied into + * one internal buffer and NUL-terminated, so plain C-string platform posts + * (Telegram sendMessage query, Discord JSON `content`) work unchanged. + * + * Limits are BYTES even where a platform counts characters (Telegram's 4096 + * is characters): conservative by design, so CJK/emoji-heavy replies split + * earlier than strictly required. Do not "fix" this to count code points. + * + * Example: `channel_send_chunked(text, 2000, post_fn, &ctx, &fail)` posts up + * to 2000-byte pieces until done; when `post_fn` returns non-zero the send + * stops and `fail` reports the failing chunk index and byte offset. + * + * @param text NUL-terminated text. NULL returns -1; empty posts nothing. + * @param max_bytes Maximum chunk size in bytes. 0 returns -1. + * @param post Callback that delivers one piece; non-zero fails the send. + * @param ctx Caller context passed through to @p post. May be NULL. + * @param fail Optional out: chunk index and byte offset where the send + * stopped. Zeroed when the call does not fail that way. + * @return 0 when every chunk was posted, -1 on invalid arguments, OOM, or + * a failed @p post (with @p fail, when given, pointing at the chunk + * that failed). + */ +int channel_send_chunked(const char *text, size_t max_bytes, + channel_chunk_post_fn post, void *ctx, + channel_chunk_fail_t *fail); + /** * Channel vtable: init, poll, send, cleanup. CLI and Telegram implement this. */ diff --git a/src/channels/channel_common.c b/src/channels/channel_common.c index 59b48f0..bb362a2 100644 --- a/src/channels/channel_common.c +++ b/src/channels/channel_common.c @@ -1,6 +1,6 @@ /** * @file channel_common.c - * @brief Shared channel utilities: channel_incoming_msg_clear, channel registry. + * @brief Shared channel utilities: chunked send, incoming msg clear, registry. */ #include "channels/channel.h" @@ -31,6 +31,75 @@ const channel_t *channel_get_by_name(const char *name) return NULL; } +size_t channel_outbound_chunk_len(const char *text, size_t max_bytes) +{ + size_t len; + size_t n; + + if (!text || max_bytes == 0) + return 0; + len = strlen(text); + if (len <= max_bytes) + return len; + /* Walk back over UTF-8 continuation bytes so the cut sits on a boundary. */ + n = max_bytes; + while (n > 0 && ((unsigned char)text[n] & 0xC0) == 0x80) + n--; + return n > 0 ? n : max_bytes; +} + +int channel_send_chunked(const char *text, size_t max_bytes, + channel_chunk_post_fn post, void *ctx, + channel_chunk_fail_t *fail) +{ + char *piece; + const char *cursor; + size_t index; + + if (fail) { + fail->index = 0; + fail->offset = 0; + } + if (!text || max_bytes == 0 || !post) + return -1; + /* One reused, always-NUL-terminated piece buffer: platform posts take + * plain C strings, so the chunk copy lives here instead of per caller. */ + piece = malloc(max_bytes + 1); + if (!piece) + return -1; + cursor = text; + index = 0; + while (cursor[0] != '\0') { + size_t n = channel_outbound_chunk_len(cursor, max_bytes); + + if (n == 0 || n > max_bytes) { + /* Fail-safe only: chunk_len guarantees 0 < n <= max_bytes + * for non-empty cursor, so a future bounds bug stops the + * send instead of walking past the piece buffer. */ + if (fail) { + fail->index = index; + fail->offset = (size_t)(cursor - text); + } + free(piece); + return -1; + } + memcpy(piece, cursor, n); + piece[n] = '\0'; + if (post(piece, ctx) != 0) { + if (fail) { + fail->index = index; + fail->offset = (size_t)(cursor - text); + } + free(piece); + return -1; + } + cursor += n; + index++; + } + free(piece); + return 0; +} + void channel_incoming_msg_clear(channel_incoming_msg_t *msg) { if (!msg) return; diff --git a/src/channels/discord.c b/src/channels/discord.c index 0b52b97..c3c5aa1 100644 --- a/src/channels/discord.c +++ b/src/channels/discord.c @@ -862,25 +862,13 @@ static int discord_recipient_channel_id(const char *recipient, char *out, size_t return 0; } -static int discord_send(const char *recipient, const char *text, - const channel_attachment_t *attachments, size_t att_count) +static int discord_post_text(const char *channel_id, const char *text, const char *auth_hdr) { - struct discord_ctx *dc = &g_dc; - char channel_id[DISCORD_CHANNEL_ID_CAP]; cJSON *root; char *json_body; char url[DISCORD_URL_CAP]; - char auth_hdr[DISCORD_AUTH_HDR_CAP]; int attempt; - (void)attachments; - if (att_count > 0) - return -1; - if (!dc->token || !recipient || !text || text[0] == '\0') - return -1; - if (strlen(text) > (size_t)DISCORD_MSG_MAX_LEN) - return -1; - if (discord_recipient_channel_id(recipient, channel_id, sizeof(channel_id)) != 0) - return -1; + root = cJSON_CreateObject(); if (!root) return -1; @@ -893,10 +881,6 @@ static int discord_send(const char *recipient, const char *text, free(json_body); return -1; } - if (snprintf(auth_hdr, sizeof(auth_hdr), "Authorization: Bot %s", dc->token) >= (int)sizeof(auth_hdr)) { - free(json_body); - return -1; - } for (attempt = 0; attempt < DISCORD_SEND_MAX_ATTEMPTS; attempt++) { struct discord_hdr_ud hd = { -1.0 }; struct curl_slist *hdrs = NULL; @@ -949,9 +933,54 @@ static int discord_send(const char *recipient, const char *text, free(json_body); return -1; } + free(json_body); return -1; } +struct discord_chunk_ctx { + char channel_id[DISCORD_CHANNEL_ID_CAP]; + char auth_hdr[DISCORD_AUTH_HDR_CAP]; +}; + +static int discord_post_piece(const char *piece, void *userdata) +{ + const struct discord_chunk_ctx *ctx = (const struct discord_chunk_ctx *)userdata; + + return discord_post_text(ctx->channel_id, piece, ctx->auth_hdr); +} + +static int discord_send(const char *recipient, const char *text, + const channel_attachment_t *attachments, size_t att_count) +{ + struct discord_chunk_ctx ctx; + struct discord_ctx *dc = &g_dc; + channel_chunk_fail_t fail; + + (void)attachments; + if (att_count > 0) + return -1; + if (!dc->token || !recipient || !text || text[0] == '\0') + return -1; + /* A single REST message is capped at 2000 characters, so a longer + * reply is delivered as several messages. */ + if (discord_recipient_channel_id(recipient, ctx.channel_id, sizeof(ctx.channel_id)) != 0) + return -1; + if (snprintf(ctx.auth_hdr, sizeof(ctx.auth_hdr), "Authorization: Bot %s", dc->token) >= + (int)sizeof(ctx.auth_hdr)) + return -1; + if (channel_send_chunked(text, (size_t)DISCORD_MSG_MAX_LEN, discord_post_piece, &ctx, + &fail) != 0) { + /* At-least-once delivery: dispatch skips the cron ack when send + * fails, so a re-run repeats already-delivered chunks. Log where + * the reply stopped so the duplicate boundary is traceable. */ + fprintf(stderr, + "shellclaw: discord: send failed at chunk %zu (byte offset %zu)\n", + fail.index, fail.offset); + return -1; + } + return 0; +} + static void discord_cleanup(void) { struct discord_pending *pending; diff --git a/src/channels/telegram.c b/src/channels/telegram.c index 45e3da3..4c1685a 100644 --- a/src/channels/telegram.c +++ b/src/channels/telegram.c @@ -8,10 +8,12 @@ #include "core/config.h" #include "cJSON.h" #include +#include #include #include #include #include +#include #define TG_API_BASE "https://api.telegram.org/bot" #define TG_GET_UPDATES "getUpdates" @@ -21,6 +23,12 @@ #define POLL_TIMEOUT_SEC 30 #define RESP_BUF_SIZE (256 * 1024) #define RESP_BUF_INIT 4096 +/* Bot API sendMessage text limit. Longer agent replies are sent as several messages. */ +#define TG_TEXT_MAX 4096 +/* Telegram allows about 1 message per second per chat. dispatch caps replies at + * 32 KiB, so a long reply is up to 8 back-to-back sendMessage calls; pace them + * so the chat does not hit 429 mid-reply. Full retry_after parsing is deferred. */ +#define TG_CHUNK_PAUSE_MS 500 struct telegram_ctx { char *token; @@ -148,14 +156,15 @@ static int parse_update(const char *json, channel_incoming_msg_t *out, long *upd return 0; } cJSON *from = cJSON_GetObjectItem(msg, "from"); - if (!from || !cJSON_IsObject(from)) { - cJSON_Delete(root); - return -1; - } - cJSON *from_id = cJSON_GetObjectItem(from, "id"); + cJSON *from_id = (from && cJSON_IsObject(from)) ? cJSON_GetObjectItem(from, "id") : NULL; + /* Channel and anonymous-admin posts can omit a numeric from.id. Failing + * the poll here left last_update_id unchanged, so this update stayed at + * the head of getUpdates and later messages never ran. Skip it instead. */ if (!from_id || !cJSON_IsNumber(from_id)) { + fprintf(stderr, "shellclaw: telegram: skip update %ld (message has no from id)\n", + *update_id_out); cJSON_Delete(root); - return -1; + return 0; } long user_id = (long)from_id->valuedouble; if (!is_user_allowed(g_tg.cfg, user_id)) { @@ -221,14 +230,32 @@ static int parse_update(const char *json, channel_incoming_msg_t *out, long *upd return 1; } +static int format_getupdates_url(char *url, size_t url_cap, const char *token, + int timeout_sec, long offset) +{ + int n; + if (!url || url_cap == 0 || !token || token[0] == '\0') + return -1; + /* Default getUpdates limit is 100. A backlog of long messages exceeds the + * 256 KiB read buffer, curl aborts, and the offset never advances, so + * every later poll retries the same batch. The parser only consumes + * result[0]; ask for that one update so a single message always fits. */ + n = snprintf(url, url_cap, "%s%s/%s?timeout=%d&offset=%ld&limit=1", + TG_API_BASE, token, TG_GET_UPDATES, timeout_sec, offset); + if (n < 0 || (size_t)n >= url_cap) + return -1; + return 0; +} + static int tg_poll(channel_incoming_msg_t *out, int timeout_ms) { if (!out || !g_tg.initialized || !g_tg.token) return -1; memset(out, 0, sizeof(*out)); int timeout_sec = (timeout_ms / 1000) > 0 ? (timeout_ms / 1000) : 1; char url[512]; - snprintf(url, sizeof(url), "%s%s/%s?timeout=%d&offset=%ld", - TG_API_BASE, g_tg.token, TG_GET_UPDATES, timeout_sec, g_tg.last_update_id + 1); + if (format_getupdates_url(url, sizeof(url), g_tg.token, timeout_sec, + g_tg.last_update_id + 1) != 0) + return -1; CURL *curl = curl_easy_init(); if (!curl) return -1; tg_curl_harden(curl); @@ -261,43 +288,159 @@ static int tg_poll(channel_incoming_msg_t *out, int timeout_ms) return 0; } -static int tg_send(const char *recipient, const char *text, - const channel_attachment_t *attachments, size_t att_count) +/* Curl success is not delivery. sendMessage reports refusal in HTTP status and ok. */ +static int telegram_response_ok(long http_code, const char *body) +{ + cJSON *root; + cJSON *ok_item; + + if (http_code < 200 || http_code >= 300) + return 0; + if (!body || body[0] == '\0') + return 0; + root = cJSON_Parse(body); + if (!root || !cJSON_IsObject(root)) { + if (root) + cJSON_Delete(root); + return 0; + } + ok_item = cJSON_GetObjectItemCaseSensitive(root, "ok"); + if (!cJSON_IsBool(ok_item) || !cJSON_IsTrue(ok_item)) { + cJSON_Delete(root); + return 0; + } + cJSON_Delete(root); + return 1; +} + +static int telegram_perform_post(CURL *curl, const char *url, const char *postfields) +{ + tg_buf_t resp; + long http_code = 0; + int ret = -1; + + resp.buf = malloc(RESP_BUF_INIT); + if (!resp.buf) + return -1; + resp.len = 0; + resp.cap = RESP_BUF_INIT; + resp.buf[0] = '\0'; + curl_easy_setopt(curl, CURLOPT_URL, url); + curl_easy_setopt(curl, CURLOPT_POSTFIELDS, postfields); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_cb); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &resp); + if (tg_curl_perform(curl, "sendMessage") == 0) { + curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code); + if (telegram_response_ok(http_code, resp.buf)) + ret = 0; + else + fprintf(stderr, "shellclaw: telegram: sendMessage rejected HTTP %ld\n", + http_code); + } + free(resp.buf); + return ret; +} + +static int telegram_post_chunk(const char *chat_id, const char *text) { - (void)attachments; - (void)att_count; - if (!g_tg.initialized || !g_tg.token || !recipient || !text) return -1; - const char *chat_id = strchr(recipient, ':'); - if (chat_id) chat_id++; - else chat_id = recipient; char url[512]; - snprintf(url, sizeof(url), "%s%s/%s", TG_API_BASE, g_tg.token, TG_SEND_MSG); - CURL *curl = curl_easy_init(); - if (!curl) return -1; + CURL *curl; + char *escaped; + char *postfields; + int ret; + size_t need; + + curl = curl_easy_init(); + if (!curl) + return -1; tg_curl_harden(curl); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1L); curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 10L); curl_easy_setopt(curl, CURLOPT_TIMEOUT, 30L); - char *escaped = curl_easy_escape(curl, text, (int)strlen(text)); + snprintf(url, sizeof(url), "%s%s/%s", TG_API_BASE, g_tg.token, TG_SEND_MSG); + escaped = curl_easy_escape(curl, text, (int)strlen(text)); if (!escaped) { curl_easy_cleanup(curl); return -1; } - size_t need = strlen(chat_id) + strlen(escaped) + 32; - char *postfields = malloc(need); - int ret = -1; - if (postfields) { - snprintf(postfields, need, "chat_id=%s&text=%s", chat_id, escaped); - curl_easy_setopt(curl, CURLOPT_URL, url); - curl_easy_setopt(curl, CURLOPT_POSTFIELDS, postfields); - ret = (tg_curl_perform(curl, "sendMessage") == 0) ? 0 : -1; - free(postfields); + need = strlen(chat_id) + strlen(escaped) + 32; + postfields = malloc(need); + if (!postfields) { + curl_free(escaped); + curl_easy_cleanup(curl); + return -1; } + snprintf(postfields, need, "chat_id=%s&text=%s", chat_id, escaped); + ret = telegram_perform_post(curl, url, postfields); + free(postfields); curl_free(escaped); curl_easy_cleanup(curl); return ret; } +typedef struct { + const char *chat_id; + int posted_before; +} tg_chunk_ctx_t; + +static void tg_sleep_ms(int ms) +{ + struct timespec ts; + + if (ms <= 0) + return; + ts.tv_sec = (time_t)(ms / 1000); + ts.tv_nsec = (long)(ms % 1000) * 1000000L; + while (nanosleep(&ts, &ts) == -1 && errno == EINTR) + ; +} + +static int tg_post_piece(const char *piece, void *userdata) +{ + tg_chunk_ctx_t *ctx = (tg_chunk_ctx_t *)userdata; + + /* Pause between consecutive chunks only (never before the first): + * the chat rate is ~1 msg/s, so paced posts avoid a 429 aborting the + * rest of the reply. Actually parsing parameters.retry_after from a + * 429 body and sleeping exactly that long is deferred follow-up. */ + if (ctx->posted_before) + tg_sleep_ms(TG_CHUNK_PAUSE_MS); + ctx->posted_before = 1; + return telegram_post_chunk(ctx->chat_id, piece); +} + +static int tg_send(const char *recipient, const char *text, + const channel_attachment_t *attachments, size_t att_count) +{ + tg_chunk_ctx_t ctx; + channel_chunk_fail_t fail; + const char *chat_id; + + (void)attachments; + (void)att_count; + if (!g_tg.initialized || !g_tg.token || !recipient || !text || text[0] == '\0') + return -1; + chat_id = strchr(recipient, ':'); + if (chat_id) + chat_id++; + else + chat_id = recipient; + if (chat_id[0] == '\0') + return -1; + ctx.chat_id = chat_id; + ctx.posted_before = 0; + if (channel_send_chunked(text, TG_TEXT_MAX, tg_post_piece, &ctx, &fail) != 0) { + /* At-least-once delivery: dispatch skips the cron ack when send + * fails, so a re-run repeats already-delivered chunks. Log where + * the reply stopped so the duplicate boundary is traceable. */ + fprintf(stderr, + "shellclaw: telegram: sendMessage failed at chunk %zu (byte offset %zu)\n", + fail.index, fail.offset); + return -1; + } + return 0; +} + static void tg_cleanup(void) { free(g_tg.token); @@ -331,8 +474,19 @@ int telegram_parse_update_for_test(const char *json, channel_incoming_msg_t *out return parse_update(json, out, update_id); } +int telegram_format_getupdates_url_for_test(char *url, size_t url_cap, const char *token, + int timeout_sec, long offset) +{ + return format_getupdates_url(url, url_cap, token, timeout_sec, offset); +} + void telegram_set_test_config(const config_t *cfg) { g_tg.cfg = cfg; } + +int telegram_response_ok_for_test(long http_code, const char *body) +{ + return telegram_response_ok(http_code, body); +} #endif diff --git a/src/core/agent.c b/src/core/agent.c index 92bc6f0..22d9d2e 100644 --- a/src/core/agent.c +++ b/src/core/agent.c @@ -82,7 +82,145 @@ static int copy_printed_session_json(char *dst, size_t dst_size, char *printed, static const char SUMMARIZE_SYSTEM[] = "Summarize the following conversation in one short paragraph. Output only the summary, no preamble."; -/** Summarize oldest messages when over max_ctx; replace with one summary + trailing. */ +/** Marker appended by summary_source_append_truncated when a line was cut. */ +static const char SUMMARY_TRUNC_MARKER[] = "[truncated]"; + +/** Append one "role: content" line, or roll back if the whole line does not fit. */ +static int summary_source_append(char *buf, size_t cap, size_t *used, + const char *role, const char *content) +{ + int n; + if (!buf || !used || *used >= cap - 1) + return 0; + n = snprintf(buf + *used, cap - *used, "%s: %s\n", + role ? role : "user", content ? content : ""); + if (n < 0 || (size_t)n >= cap - *used) { + buf[*used] = '\0'; + return 0; + } + *used += (size_t)n; + return 1; +} + +/** True when the line cannot fit even a fresh source, so no later pass can + * ever summarize it whole either. */ +static int summary_line_never_fits(const char *role, const char *content) +{ + const char *r = role ? role : "user"; + const char *c = content ? content : ""; + + /* summary_source_append refuses when the line plus its NUL exceeds the cap. */ + return strlen(r) + 2 + strlen(c) + 1 >= SUMMARY_SOURCE_MAX; +} + +/** Append one line with the content cut to fit, marking the cut, so a message + * that can never fit whole still reaches the summarizer. Same atomicity: + * the whole stub fits or nothing is written. */ +static int summary_source_append_truncated(char *buf, size_t cap, size_t *used, + const char *role, const char *content) +{ + const char *r = role ? role : "user"; + const char *c = content ? content : ""; + size_t remain; + size_t fixed; + size_t cut; + int n; + + if (!buf || !used || *used + 1 >= cap) + return 0; + remain = cap - *used; + fixed = strlen(r) + 2 + strlen(SUMMARY_TRUNC_MARKER) + 1 + 1; /* ": " "\n" NUL */ + if (remain <= fixed) + return 0; + cut = remain - fixed; + if (cut > strlen(c)) + cut = strlen(c); + n = snprintf(buf + *used, remain, "%s: %.*s%s\n", r, (int)cut, c, + SUMMARY_TRUNC_MARKER); + if (n < 0 || (size_t)n >= remain) { + buf[*used] = '\0'; + return 0; + } + *used += (size_t)n; + return 1; +} + +/** True when the kept tail already starts with user, so a summary user turn would not alternate. */ +static int kept_tail_starts_with_user(const cJSON *root, int start) +{ + const cJSON *item; + const cJSON *role; + + item = cJSON_GetArrayItem(root, start); + if (!item || !cJSON_IsObject(item)) + return 0; + role = cJSON_GetObjectItem(item, "role"); + return cJSON_IsString(role) && role->valuestring && strcmp(role->valuestring, "user") == 0; +} + +/** Fold the summary into an existing user turn. A second user message 400s on Anthropic. */ +static int fold_summary_into_user(cJSON *item, const char *summary) +{ + const cJSON *content; + const char *old; + char *merged; + cJSON *node; + size_t old_len; + size_t sum_len; + size_t n; + + content = cJSON_GetObjectItem(item, "content"); + old = (content && cJSON_IsString(content) && content->valuestring) ? content->valuestring : ""; + old_len = strlen(old); + sum_len = strlen(summary ? summary : ""); + if (sum_len > SIZE_MAX - old_len - 3) + return -1; + n = sum_len + 2 + old_len + 1; + merged = malloc(n); + if (!merged) + return -1; + snprintf(merged, n, "%s\n\n%s", summary ? summary : "", old); + node = cJSON_CreateString(merged); + free(merged); + if (!node) + return -1; + if (cJSON_GetObjectItem(item, "content")) { + if (!cJSON_ReplaceItemInObject(item, "content", node)) { + cJSON_Delete(node); + return -1; + } + return 0; + } + if (!cJSON_AddItemToObject(item, "content", node)) { + cJSON_Delete(node); + return -1; + } + return 0; +} + +static int append_summary_user(cJSON *arr, const char *summary) +{ + cJSON *obj; + cJSON *role; + cJSON *content; + + obj = cJSON_CreateObject(); + role = cJSON_CreateString("user"); + content = cJSON_CreateString(summary ? summary : ""); + if (!obj || !role || !content) { + cJSON_Delete(obj); + cJSON_Delete(role); + cJSON_Delete(content); + return -1; + } + cJSON_AddItemToObject(obj, "role", role); + cJSON_AddItemToObject(obj, "content", content); + cJSON_AddItemToArray(arr, obj); + return 0; +} + +/** Summarize oldest messages when over max_ctx; replace with one summary + trailing. + * Messages that do not fit in the summary source stay in the session. */ static int compact_session_via_llm(const char *session_id, char *session_buf, size_t session_buf_size, int msg_count, int max_ctx, const provider_t *provider) { @@ -92,22 +230,42 @@ static int compact_session_via_llm(const char *session_id, char *session_buf, si if (root) cJSON_Delete(root); return -1; } - int to_summarize_n = msg_count - max_ctx; + /* Tail keeps max_ctx-1 messages, so the summary must include index + * msg_count-max_ctx. Stopping one short drops that turn from SQLite. */ + int to_summarize_n = msg_count - max_ctx + 1; char *source_buf = malloc(SUMMARY_SOURCE_MAX); if (!source_buf) { cJSON_Delete(root); return -1; } source_buf[0] = '\0'; size_t used = 0; - for (int i = 0; i < to_summarize_n && used < SUMMARY_SOURCE_MAX - 1; i++) { + int next_unsummarized = to_summarize_n; + for (int i = 0; i < to_summarize_n; i++) { const cJSON *item = cJSON_GetArrayItem(root, i); - if (!item || !cJSON_IsObject(item)) continue; const char *role = "user"; const char *content = ""; - const cJSON *r = cJSON_GetObjectItem(item, "role"); - const cJSON *c = cJSON_GetObjectItem(item, "content"); + const cJSON *r; + const cJSON *c; + if (!item || !cJSON_IsObject(item)) continue; + r = cJSON_GetObjectItem(item, "role"); + c = cJSON_GetObjectItem(item, "content"); if (cJSON_IsString(r)) role = r->valuestring; if (cJSON_IsString(c)) content = c->valuestring; - int n = snprintf(source_buf + used, SUMMARY_SOURCE_MAX - used, "%s: %s\n", role, content ? content : ""); - if (n > 0 && (size_t)n < SUMMARY_SOURCE_MAX - used) used += (size_t)n; + if (!summary_source_append(source_buf, SUMMARY_SOURCE_MAX, &used, role, content)) { + /* A line that can never fit whole would abort with used==0 + * (first line) or keep the session above max_ctx forever + * (later line); stub it so this pass still consumes it. + * Lines that only lack room here stay raw for a later pass + * via next_unsummarized. */ + if (!summary_line_never_fits(role, content) || + !summary_source_append_truncated(source_buf, SUMMARY_SOURCE_MAX, &used, role, content)) { + next_unsummarized = i; + break; + } + } + } + if (used == 0) { + free(source_buf); + cJSON_Delete(root); + return -1; } provider_message_t summ_msgs[2] = { { .role = "system", .content = SUMMARIZE_SYSTEM, .tool_calls = NULL, .tool_calls_count = 0, .tool_use_id = NULL }, @@ -128,29 +286,51 @@ static int compact_session_via_llm(const char *session_id, char *session_buf, si summary_buf[sum_len] = '\0'; provider_response_clear(&resp); cJSON *new_arr = cJSON_CreateArray(); - if (!new_arr) { cJSON_Delete(root); return -1; } - cJSON *summ_obj = cJSON_CreateObject(); - if (summ_obj) { - cJSON_AddItemToObject(summ_obj, "role", cJSON_CreateString("user")); - cJSON_AddItemToObject(summ_obj, "content", cJSON_CreateString(summary_buf)); - cJSON_AddItemToArray(new_arr, summ_obj); - } int keep = max_ctx - 1; - int start = msg_count - keep; + int start = (next_unsummarized < to_summarize_n) + ? next_unsummarized : (msg_count - keep); + int merge; + int folded = 0; if (start < 0) start = 0; + /* Tail can start on a user turn (odd max_context_messages, or a cut that + * lands on user). Inserting another user role makes Anthropic reject the + * session on this turn and every later one. */ + merge = kept_tail_starts_with_user(root, start); + if (!new_arr) { cJSON_Delete(root); return -1; } + if (!merge && append_summary_user(new_arr, summary_buf) != 0) { + cJSON_Delete(new_arr); + cJSON_Delete(root); + return -1; + } for (int i = start; i < msg_count; i++) { const cJSON *item = cJSON_GetArrayItem(root, i); - if (item) { - cJSON *dup = cJSON_Duplicate(item, 1); - if (dup) cJSON_AddItemToArray(new_arr, dup); + cJSON *dup; + if (!item) continue; + dup = cJSON_Duplicate(item, 1); + if (!dup) continue; + if (merge && i == start) { + if (fold_summary_into_user(dup, summary_buf) != 0) { + cJSON_Delete(dup); + cJSON_Delete(new_arr); + cJSON_Delete(root); + return -1; + } + folded = 1; } + cJSON_AddItemToArray(new_arr, dup); + } + if (merge && !folded) { + cJSON_Delete(new_arr); + cJSON_Delete(root); + return -1; } cJSON_Delete(root); char *printed = cJSON_PrintUnformatted(new_arr); cJSON_Delete(new_arr); if (copy_printed_session_json(session_buf, session_buf_size, printed, session_id) != 0) return -1; - session_save(session_id, session_buf); + /* Do not session_save here. A failed follow-up chat must leave the stored + * transcript unchanged; agent_persist_session writes this buffer on success. */ return 0; } diff --git a/src/core/agent.h b/src/core/agent.h index c530aef..a1dcd6e 100644 --- a/src/core/agent.h +++ b/src/core/agent.h @@ -54,18 +54,22 @@ int agent_run(const config_t *cfg, const char *session_id, const char *user_mess * Every agent_run() caller (main-loop handle_message, inbound ASAP HTTP, * WebSocket dispatcher) must hold this mutex for the duration of agent_run(). * /reset in handle_message must also hold it around session_delete(). + * Dashboard DELETE /api/sessions/:id must use dispatch_delete_session(). * After a successful cron ch->send, handle_message re-takes it around * cron_ack_delivery() (SQLite amalgamation is SQLITE_THREADSAFE=0). * Inbound mcp.tool_call must hold it around tool execute (see #60). * Inbound state.query must hold it around memory_get_row_counts() (see #60). - * Release before channel I/O (ch->send). The mutex is not recursive. + * Dashboard /api/memory, /api/sessions list, and /api/cron handlers must hold + * it around g_db. cron_poll must hold it around due-job SQL, and must release + * it before the re-offer sleep. The mutex is not recursive. + * Release before channel I/O (ch->send). */ void agent_lock(void); /** * Release the global agent mutex after agent_run(), a locked session_delete(), - * inbound mcp.tool_call execute, a locked state.query memory read, or a - * locked cron_ack_delivery(). + * inbound mcp.tool_call execute, a locked state.query memory read, a locked + * cron_ack_delivery(), a dashboard g_db handler, or cron_poll's due-job SQL. */ void agent_unlock(void); diff --git a/src/core/dispatch.c b/src/core/dispatch.c index 44b1479..68d2427 100644 --- a/src/core/dispatch.c +++ b/src/core/dispatch.c @@ -43,6 +43,18 @@ static int send_then_maybe_ack_cron(const channel_t *ch, const channel_incoming_ return maybe_ack_cron(ch, msg); } +int dispatch_delete_session(const char *session_id) +{ + int rc; + if (!session_id || session_id[0] == '\0') + return -1; + /* Serialize with agent_run so session_save cannot recreate the row. */ + agent_lock(); + rc = session_delete(session_id); + agent_unlock(); + return rc; +} + int handle_message(const channel_t *ch, const channel_incoming_msg_t *msg) { const char *text = msg->text ? msg->text : ""; diff --git a/src/core/dispatch.h b/src/core/dispatch.h index 36ac069..e94bb48 100644 --- a/src/core/dispatch.h +++ b/src/core/dispatch.h @@ -18,6 +18,20 @@ extern "C" { */ int handle_message(const channel_t *ch, const channel_incoming_msg_t *msg); +/** + * Delete one session while holding the agent mutex. + * + * Dashboard `DELETE /api/sessions/:id` runs on the gateway thread. agent_run() + * loads history, talks to the model, then session_save()s. Without this lock + * the delete lands in that window and the save puts the old transcript back. + * + * Example: `if (dispatch_delete_session(id) != 0) respond 404;` + * + * @param session_id Session row id. Empty or NULL is rejected. + * @return 0 when a row was removed, non-zero otherwise. + */ +int dispatch_delete_session(const char *session_id); + #ifdef __cplusplus } #endif diff --git a/src/core/memory.c b/src/core/memory.c index 9edaa7e..8e7f0ab 100644 --- a/src/core/memory.c +++ b/src/core/memory.c @@ -17,6 +17,7 @@ static sqlite3 *g_db; static void (*g_session_delete_hook_for_test)(const char *session_id); static void (*g_memory_get_row_counts_hook_for_test)(void); +static void (*g_sqlite_entry_hook_for_test)(void); void session_delete_set_hook_for_test(void (*hook)(const char *session_id)) { @@ -28,6 +29,17 @@ void memory_get_row_counts_set_hook_for_test(void (*hook)(void)) g_memory_get_row_counts_hook_for_test = hook; } +void memory_sqlite_entry_set_hook_for_test(void (*hook)(void)) +{ + g_sqlite_entry_hook_for_test = hook; +} + +static void memory_sqlite_entry_probe(void) +{ + if (g_sqlite_entry_hook_for_test) + g_sqlite_entry_hook_for_test(); +} + static const char *SCHEMA_MEMORIES = "CREATE TABLE IF NOT EXISTS memories (" " rowid INTEGER PRIMARY KEY," @@ -202,6 +214,7 @@ int memory_save(const char *key, const char *content, const char *metadata) int memory_recall(const char *query, char *results, size_t max_len, int limit) { if (!g_db || !results || max_len == 0 || !query) return -1; + memory_sqlite_entry_probe(); results[0] = '\0'; if (limit <= 0) limit = 10; const char *sql = "SELECT content FROM memories_fts WHERE memories_fts MATCH ?1 ORDER BY rank LIMIT ?2"; @@ -279,6 +292,7 @@ int session_delete(const char *session_id) if (g_session_delete_hook_for_test) g_session_delete_hook_for_test(session_id); if (!g_db || !session_id) return -1; + memory_sqlite_entry_probe(); const char *sql = "DELETE FROM sessions WHERE id = ?1"; sqlite3_stmt *stmt = NULL; if (sqlite3_prepare_v2(g_db, sql, -1, &stmt, NULL) != SQLITE_OK) return -1; @@ -291,6 +305,7 @@ int session_delete(const char *session_id) int session_list(char **session_ids_out, int max_count) { if (!g_db || !session_ids_out || max_count <= 0) return -1; + memory_sqlite_entry_probe(); const char *sql = "SELECT id FROM sessions ORDER BY updated_at DESC"; sqlite3_stmt *stmt = NULL; if (sqlite3_prepare_v2(g_db, sql, -1, &stmt, NULL) != SQLITE_OK) return -1; @@ -398,6 +413,7 @@ int cron_job_create(const char *id, const char *schedule, const char *message, const char *channel, const char *recipient, long long next_run, int enabled) { if (!g_db || !id || !schedule || !message) return -1; + memory_sqlite_entry_probe(); /* fill_cron_job_row truncates id[128]; a longer id breaks the due cursor. */ if (strlen(id) >= sizeof(((cron_job_row_t *)0)->id)) return -1; if (strlen(schedule) > (size_t)CRON_JOB_TEXT_MAX) return -1; @@ -423,6 +439,7 @@ int cron_job_create(const char *id, const char *schedule, const char *message, int cron_job_delete(const char *id) { if (!g_db || !id) return -1; + memory_sqlite_entry_probe(); const char *sql = "DELETE FROM cron_jobs WHERE id = ?1"; sqlite3_stmt *stmt = NULL; if (sqlite3_prepare_v2(g_db, sql, -1, &stmt, NULL) != SQLITE_OK) return -1; @@ -435,6 +452,7 @@ int cron_job_delete(const char *id) int cron_job_toggle(const char *id) { if (!g_db || !id) return -1; + memory_sqlite_entry_probe(); const char *sql = "UPDATE cron_jobs SET enabled = 1 - enabled WHERE id = ?1"; sqlite3_stmt *stmt = NULL; if (sqlite3_prepare_v2(g_db, sql, -1, &stmt, NULL) != SQLITE_OK) return -1; @@ -460,6 +478,7 @@ int cron_job_update_next_run(const char *id, long long next_run) int cron_job_list(cron_job_row_t *out, int max_count) { if (!g_db || !out || max_count <= 0) return -1; + memory_sqlite_entry_probe(); const char *sql = "SELECT id, schedule, message, channel, recipient, next_run, enabled " "FROM cron_jobs ORDER BY next_run ASC, id ASC"; sqlite3_stmt *stmt = NULL; @@ -485,6 +504,7 @@ int cron_job_list_due(long long now, cron_due_key_t *out, int max_count) int count = 0; if (!g_db || !out || max_count <= 0) return -1; + memory_sqlite_entry_probe(); sql = "SELECT id, next_run FROM cron_jobs WHERE next_run <= ?1 AND enabled = 1 " "ORDER BY next_run ASC, id ASC"; if (sqlite3_prepare_v2(g_db, sql, -1, &stmt, NULL) != SQLITE_OK) return -1; @@ -551,6 +571,7 @@ int cron_job_get_by_id(const char *id, cron_job_row_t *out) int ret = 0; if (!g_db || !id || !out) return -1; + memory_sqlite_entry_probe(); sql = "SELECT id, schedule, message, channel, recipient, next_run, enabled FROM cron_jobs WHERE id = ?1"; if (sqlite3_prepare_v2(g_db, sql, -1, &stmt, NULL) != SQLITE_OK) return -1; sqlite3_bind_text(stmt, 1, id, -1, SQLITE_TRANSIENT); diff --git a/src/core/memory.h b/src/core/memory.h index b266660..e433fac 100644 --- a/src/core/memory.h +++ b/src/core/memory.h @@ -92,6 +92,15 @@ void session_delete_set_hook_for_test(void (*hook)(const char *session_id)); */ void memory_get_row_counts_set_hook_for_test(void (*hook)(void)); +/** + * Test-only: invoke @p hook when a dashboard or cron-poll SQL entry runs. + * Pass NULL to clear. Used to assert those callers hold agent_lock() + * (SQLite is SQLITE_THREADSAFE=0). + * + * Example: memory_sqlite_entry_set_hook_for_test(probe); session_list(ids, 8); + */ +void memory_sqlite_entry_set_hook_for_test(void (*hook)(void)); + /** * List session IDs from the database. * diff --git a/src/core/reload.c b/src/core/reload.c index 17c64e5..cbad861 100644 --- a/src/core/reload.c +++ b/src/core/reload.c @@ -87,6 +87,7 @@ int try_config_reload(config_t **pcfg) return -1; } provider_router_set_live_config(new_cfg); + provider_anthropic_set_live_config(new_cfg); provider_openai_set_live_config(new_cfg); provider_local_set_live_config(new_cfg); tool_set_config(new_cfg); diff --git a/src/core/skill.c b/src/core/skill.c index 7401b9a..180654f 100644 --- a/src/core/skill.c +++ b/src/core/skill.c @@ -325,15 +325,39 @@ int skill_list_names(const config_t *cfg, char **names_out, int max_count) int skill_get_content(const config_t *cfg, const char *name, char *out_buf, size_t out_size) { - if (!cfg || !name || !out_buf || out_size == 0) return -1; char path[MAX_PATH_LEN]; - if (build_skill_path(cfg, name, path, sizeof(path)) != 0) return -1; - FILE *f = fopen(path, "r"); - if (!f) return -1; + FILE *f; + long sz; + size_t used; + + if (!cfg || !name || !out_buf || out_size == 0) + return -1; + if (build_skill_path(cfg, name, path, sizeof(path)) != 0) + return -1; + f = fopen(path, "r"); + if (!f) + return -1; + if (fseek(f, 0, SEEK_END) != 0) { + fclose(f); + return -1; + } + sz = ftell(f); + /* Dashboard GET copies this buffer into the editor and PUT writes it + * back. A short read must not look like the whole file. */ + if (sz < 0 || (size_t)sz >= out_size) { + fclose(f); + return (sz < 0) ? -1 : -2; + } + if (fseek(f, 0, SEEK_SET) != 0) { + fclose(f); + return -1; + } out_buf[0] = '\0'; - size_t used = 0; + used = 0; append_file_content(f, out_buf, out_size, &used); fclose(f); + if (used != (size_t)sz) + return -2; return 0; } diff --git a/src/core/skill.h b/src/core/skill.h index 49cdae3..42d1dd9 100644 --- a/src/core/skill.h +++ b/src/core/skill.h @@ -63,8 +63,11 @@ int skill_list_names(const config_t *cfg, char **names_out, int max_count); * @param cfg Configuration (uses config_skills_dir). * @param name Skill name (file base name). * @param out_buf Buffer to receive content. - * @param out_size Size of out_buf. - * @return 0 on success, -1 if not found or error. + * @param out_size Size of out_buf. Must hold the whole file plus a NUL. + * @return 0 on success, -2 if the file does not fit in out_buf, -1 if not found or error. + * + * Example: skill_get_content(cfg, "deploy", buf, sizeof(buf)) == -2 means the + * caller must not treat buf as the skill body. */ int skill_get_content(const config_t *cfg, const char *name, char *out_buf, size_t out_size); diff --git a/src/gateway/http.c b/src/gateway/http.c index 44f3333..f806930 100644 --- a/src/gateway/http.c +++ b/src/gateway/http.c @@ -163,6 +163,9 @@ void http_stop(void) g_ctx->thread = 0; } if (g_ctx->lws_ctx) { + /* Drop the pointer before destroy so a late send cannot + * lws_cancel_service a freed context. */ + ws_set_context(NULL); lws_context_destroy(g_ctx->lws_ctx); g_ctx->lws_ctx = NULL; } diff --git a/src/gateway/http_lws.c b/src/gateway/http_lws.c index 86ecb79..9794007 100644 --- a/src/gateway/http_lws.c +++ b/src/gateway/http_lws.c @@ -488,6 +488,10 @@ int ws_callback(struct lws *wsi, enum lws_callback_reasons reason, void *user, if (conn_id > 0) ws_unregister_conn(conn_id); break; } + case LWS_CALLBACK_EVENT_WAIT_CANCELLED: + /* wsi here is not a client socket. Mark real conns from our table. */ + ws_on_service_cancel(); + break; default: break; } diff --git a/src/gateway/routes.c b/src/gateway/routes.c index 39c24a8..da4e1db 100644 --- a/src/gateway/routes.c +++ b/src/gateway/routes.c @@ -17,6 +17,7 @@ #include "asap/log.h" #include "core/agent.h" #include "core/bootstrap.h" +#include "core/dispatch.h" #include "core/config.h" #include "core/config_patch.h" #include "core/memory.h" @@ -377,28 +378,75 @@ static void handle_skills_list(const config_t *cfg, char *buf, size_t size, int #define SKILL_CONTENT_MAX 32768 #define MEMORY_RESULTS_MAX 16384 -static void handle_skill_get(const config_t *cfg, const char *name, char *buf, size_t size, int *status) +static void skill_too_large(char *buf, size_t size, int *status, const char *name, + const char *limit_desc) { - char *content = malloc(SKILL_CONTENT_MAX); - if (!content) { json_error(buf, size, status, 500, "Out of memory"); return; } - if (skill_get_content(cfg, name, content, SKILL_CONTENT_MAX) != 0) { - free(content); - json_error(buf, size, status, 404, "Skill not found"); + char msg[256]; + int n; + + n = snprintf(msg, sizeof(msg), "skill \"%s\" exceeds %s", + name ? name : "", limit_desc ? limit_desc : "edit buffer"); + if (n < 0 || (size_t)n >= sizeof(msg)) + json_error(buf, size, status, 413, "skill exceeds edit buffer"); + else + json_error(buf, size, status, 413, msg); +} + +static void skill_get_send(const char *name, const char *content, char *buf, size_t size, + int *status) +{ + cJSON *obj; + char *printed; + obj = cJSON_CreateObject(); + if (!obj) { + json_error(buf, size, status, 500, "Internal error"); return; } - cJSON *obj = cJSON_CreateObject(); - if (!obj) { free(content); json_error(buf, size, status, 500, "Internal error"); return; } cJSON_AddItemToObject(obj, "name", cJSON_CreateString(name)); cJSON_AddItemToObject(obj, "content", cJSON_CreateString(content)); - free(content); - char *s = cJSON_PrintUnformatted(obj); + printed = cJSON_PrintUnformatted(obj); cJSON_Delete(obj); - if (s) { - json_response(buf, size, status, s); - free(s); - } else { + if (!printed) { json_error(buf, size, status, 500, "Internal error"); + return; + } + /* A cut-off 200 body parses as empty in the dashboard and Save wipes the file. */ + if (strlen(printed) >= size) { + char limit_desc[64]; + free(printed); + snprintf(limit_desc, sizeof(limit_desc), "%zu byte response buffer", size); + skill_too_large(buf, size, status, name, limit_desc); + return; } + json_response(buf, size, status, printed); + free(printed); +} + +static void handle_skill_get(const config_t *cfg, const char *name, char *buf, size_t size, int *status) +{ + char *content; + int rc; + + content = malloc(SKILL_CONTENT_MAX); + if (!content) { + json_error(buf, size, status, 500, "Out of memory"); + return; + } + rc = skill_get_content(cfg, name, content, SKILL_CONTENT_MAX); + if (rc != 0) { + free(content); + if (rc == -2) { + char limit_desc[64]; + snprintf(limit_desc, sizeof(limit_desc), "%d byte edit buffer", + SKILL_CONTENT_MAX - 1); + skill_too_large(buf, size, status, name, limit_desc); + } else { + json_error(buf, size, status, 404, "Skill not found"); + } + return; + } + skill_get_send(name, content, buf, size, status); + free(content); } static void handle_skill_create(const config_t *cfg, const char *body, size_t body_len, @@ -458,8 +506,13 @@ static void handle_skill_delete(const config_t *cfg, const char *name, static void handle_memory_get(const char *query, int limit, char *buf, size_t size, int *status) { char *results = malloc(MEMORY_RESULTS_MAX); + int recall_rc; if (!results) { json_error(buf, size, status, 500, "Out of memory"); return; } - if (memory_recall(query ? query : "", results, MEMORY_RESULTS_MAX, limit > 0 ? limit : 20) != 0) { + agent_lock(); + recall_rc = memory_recall(query ? query : "", results, MEMORY_RESULTS_MAX, + limit > 0 ? limit : 20); + agent_unlock(); + if (recall_rc != 0) { free(results); json_error(buf, size, status, 500, "Memory recall failed"); return; @@ -481,7 +534,10 @@ static void handle_memory_get(const char *query, int limit, char *buf, size_t si static void handle_sessions_list(char *buf, size_t size, int *status) { char *ids[64]; - int n = session_list(ids, 64); + int n; + agent_lock(); + n = session_list(ids, 64); + agent_unlock(); cJSON *arr = cJSON_CreateArray(); if (!arr) { json_error(buf, size, status, 500, "Internal error"); return; } for (int i = 0; i < n; i++) { @@ -500,7 +556,7 @@ static void handle_sessions_list(char *buf, size_t size, int *status) static void handle_session_delete(const char *id, char *buf, size_t size, int *status) { - if (session_delete(id) != 0) { + if (dispatch_delete_session(id) != 0) { json_error(buf, size, status, 404, "Session not found"); return; } @@ -522,7 +578,9 @@ static void handle_cron_list(char *buf, size_t size, int *status) int n; int i; if (!rows) { json_error(buf, size, status, 500, "Out of memory"); return; } + agent_lock(); n = cron_job_list(rows, 64); + agent_unlock(); if (n < 0) { free(rows); json_error(buf, size, status, 500, "Internal error"); @@ -580,7 +638,10 @@ static void handle_cron_create(const char *body, size_t body_len, char *buf, siz id[0] = '\0'; const char *ch = (channel && cJSON_IsString(channel)) ? channel->valuestring : NULL; const char *rec = (recipient && cJSON_IsString(recipient)) ? recipient->valuestring : NULL; - int ret = cron_create_job(schedule->valuestring, message->valuestring, ch, rec, id, sizeof(id)); + int ret; + agent_lock(); + ret = cron_create_job(schedule->valuestring, message->valuestring, ch, rec, id, sizeof(id)); + agent_unlock(); cJSON_Delete(root); if (ret != 0) { json_error(buf, size, status, 400, "Failed to create job"); @@ -600,7 +661,11 @@ static void handle_cron_create(const char *body, size_t body_len, char *buf, siz static void handle_cron_delete(const char *id, char *buf, size_t size, int *status) { - if (cron_job_delete(id) != 0) { + int rc; + agent_lock(); + rc = cron_job_delete(id); + agent_unlock(); + if (rc != 0) { json_error(buf, size, status, 404, "Job not found"); return; } @@ -610,7 +675,11 @@ static void handle_cron_delete(const char *id, char *buf, size_t size, int *stat static void handle_cron_toggle(const char *id, char *buf, size_t size, int *status) { - if (cron_job_toggle(id) != 0) { + int rc; + agent_lock(); + rc = cron_job_toggle(id); + agent_unlock(); + if (rc != 0) { json_error(buf, size, status, 404, "Job not found"); return; } @@ -964,8 +1033,10 @@ int dispatch_route(http_server_ctx_t *ctx, struct lws *wsi, int method, if (method != HTTP_GET) { json_error(buf, size, status, 405, "Method not allowed"); return 0; } char qbuf[256] = {0}; char lbuf[32] = {0}; - lws_get_urlarg_by_name_safe(wsi, "q", qbuf, sizeof(qbuf)); - lws_get_urlarg_by_name_safe(wsi, "limit", lbuf, sizeof(lbuf)); + if (wsi) { + lws_get_urlarg_by_name_safe(wsi, "q", qbuf, sizeof(qbuf)); + lws_get_urlarg_by_name_safe(wsi, "limit", lbuf, sizeof(lbuf)); + } int limit = parse_memory_limit_arg(lbuf[0] ? lbuf : NULL); handle_memory_get(qbuf[0] ? qbuf : NULL, limit, buf, size, status); return 0; diff --git a/src/gateway/ws.c b/src/gateway/ws.c index c1848f5..3faaaf5 100644 --- a/src/gateway/ws.c +++ b/src/gateway/ws.c @@ -8,7 +8,31 @@ #ifdef SHELLCLAW_WS_TEST struct lws; struct lws_context; -static void lws_callback_on_writable(struct lws *wsi) { (void)wsi; } +static int g_ws_test_writable_count; +static int g_ws_test_cancel_count; +static void lws_callback_on_writable(struct lws *wsi) +{ + (void)wsi; + g_ws_test_writable_count++; +} +static void lws_cancel_service(struct lws_context *ctx) +{ + (void)ctx; + g_ws_test_cancel_count++; +} +int ws_test_writable_count(void) +{ + return g_ws_test_writable_count; +} +int ws_test_cancel_count(void) +{ + return g_ws_test_cancel_count; +} +void ws_test_reset_wake_counts(void) +{ + g_ws_test_writable_count = 0; + g_ws_test_cancel_count = 0; +} #else #include "gateway/lws_compat.h" #include @@ -59,7 +83,48 @@ static struct lws_context *g_lws_ctx; void ws_set_context(void *ctx) { + pthread_mutex_lock(&g_mutex); g_lws_ctx = (struct lws_context *)ctx; + pthread_mutex_unlock(&g_mutex); +} + +/* Cross-thread wake. lws_cancel_service is the call that is safe off the + * service thread; the writable mark happens in ws_on_service_cancel. */ +static void ws_wake_service(void) +{ + struct lws_context *ctx; + + pthread_mutex_lock(&g_mutex); + ctx = g_lws_ctx; + pthread_mutex_unlock(&g_mutex); + if (ctx) + lws_cancel_service(ctx); +} + +void ws_on_service_cancel(void) +{ + struct lws *wake[MAX_CONNECTIONS]; + int n = 0; + int i; + + pthread_mutex_lock(&g_mutex); + for (i = 0; i < MAX_CONNECTIONS; i++) { + ws_msg_t *msg; + int conn_id; + + if (!g_conns[i].wsi) + continue; + conn_id = g_conns[i].conn_id; + for (msg = g_outgoing_head; msg; msg = msg->next) { + if (msg->conn_id != conn_id) + continue; + wake[n++] = (struct lws *)g_conns[i].wsi; + break; + } + } + pthread_mutex_unlock(&g_mutex); + for (i = 0; i < n; i++) + lws_callback_on_writable(wake[i]); } int ws_next_conn_id(void) @@ -83,16 +148,92 @@ int ws_register_conn(int conn_id, ws_conn_t wsi) return registered ? 0 : -1; } +static void ws_free_outgoing(ws_msg_t *m) +{ + if (!m) + return; + free(m->text); + free(m); +} + +static ws_msg_t *ws_msg_new(int conn_id, const char *text) +{ + ws_msg_t *m; + + m = malloc(sizeof(*m)); + if (!m) + return NULL; + m->conn_id = conn_id; + m->text = strdup(text); + m->next = NULL; + if (!m->text) { + free(m); + return NULL; + } + return m; +} + +/** Drop every queued frame for @p conn_id. Caller holds g_mutex. */ +static void ws_drop_outgoing_locked(int conn_id) +{ + ws_msg_t *prev = NULL; + ws_msg_t *p = g_outgoing_head; + + while (p) { + ws_msg_t *next = p->next; + if (p->conn_id != conn_id) { + prev = p; + p = next; + continue; + } + if (prev) + prev->next = next; + else + g_outgoing_head = next; + if (g_outgoing_tail == p) + g_outgoing_tail = prev; + g_outgoing_count--; + ws_free_outgoing(p); + p = next; + } +} + +static ws_conn_t ws_find_wsi_locked(int conn_id) +{ + int i; + + for (i = 0; i < MAX_CONNECTIONS; i++) { + if (g_conns[i].wsi && g_conns[i].conn_id == conn_id) + return g_conns[i].wsi; + } + return NULL; +} + +static void ws_enqueue_outgoing_locked(ws_msg_t *m) +{ + if (!g_outgoing_tail) + g_outgoing_head = g_outgoing_tail = m; + else { + g_outgoing_tail->next = m; + g_outgoing_tail = m; + } + g_outgoing_count++; +} + void ws_unregister_conn(int conn_id) { + int i; + pthread_mutex_lock(&g_mutex); - for (int i = 0; i < MAX_CONNECTIONS; i++) { - if (g_conns[i].conn_id == conn_id) { + for (i = 0; i < MAX_CONNECTIONS; i++) { + if (g_conns[i].wsi && g_conns[i].conn_id == conn_id) { g_conns[i].wsi = NULL; g_conns[i].conn_id = 0; break; } } + /* Frames queued before CLOSE are never drained once wsi is gone. */ + ws_drop_outgoing_locked(conn_id); pthread_mutex_unlock(&g_mutex); } @@ -218,82 +359,58 @@ int ws_has_pending_outgoing(int conn_id) int ws_send_to(const char *session_id, const char *text) { - if (!session_id || !ws_text_len_ok(text)) return -1; - int conn_id = parse_conn_id(session_id); - if (conn_id < 0) return -1; - ws_msg_t *m = malloc(sizeof(*m)); - if (!m) return -1; - m->conn_id = conn_id; - m->text = strdup(text); - m->next = NULL; - if (!m->text) { free(m); return -1; } + int conn_id; + ws_msg_t *m; + ws_conn_t wsi; + + if (!session_id || !ws_text_len_ok(text)) + return -1; + conn_id = parse_conn_id(session_id); + if (conn_id < 0) + return -1; + m = ws_msg_new(conn_id, text); + if (!m) + return -1; pthread_mutex_lock(&g_mutex); + wsi = ws_find_wsi_locked(conn_id); + /* No live socket: enqueueing would pin the shared queue until restart. */ + if (!wsi) { + pthread_mutex_unlock(&g_mutex); + ws_free_outgoing(m); + return 0; + } if (g_outgoing_count >= MAX_QUEUE) { pthread_mutex_unlock(&g_mutex); - free(m->text); - free(m); + ws_free_outgoing(m); return -1; } - if (!g_outgoing_tail) - g_outgoing_head = g_outgoing_tail = m; - else { - g_outgoing_tail->next = m; - g_outgoing_tail = m; - } - g_outgoing_count++; - ws_conn_t wsi = NULL; - for (int i = 0; i < MAX_CONNECTIONS; i++) { - if (g_conns[i].conn_id == conn_id) { - wsi = g_conns[i].wsi; - break; - } - } + ws_enqueue_outgoing_locked(m); pthread_mutex_unlock(&g_mutex); - if (!wsi) return 0; - lws_callback_on_writable((struct lws *)wsi); + ws_wake_service(); return 0; } void ws_broadcast_text(const char *text) { int i; - int wake_count = 0; - ws_conn_t wake_list[MAX_CONNECTIONS]; + int queued = 0; if (!ws_text_len_ok(text)) return; pthread_mutex_lock(&g_mutex); for (i = 0; i < MAX_CONNECTIONS; i++) { ws_msg_t *m; - int cid; - if (!g_conns[i].wsi) continue; - cid = g_conns[i].conn_id; + if (!g_conns[i].wsi) + continue; if (g_outgoing_count >= MAX_QUEUE) break; - m = malloc(sizeof(*m)); - if (!m) break; - m->conn_id = cid; - m->text = strdup(text); - if (!m->text) { - free(m); + m = ws_msg_new(g_conns[i].conn_id, text); + if (!m) break; - } - m->next = NULL; - if (!g_outgoing_tail) - g_outgoing_head = g_outgoing_tail = m; - else { - g_outgoing_tail->next = m; - g_outgoing_tail = m; - } - g_outgoing_count++; - if (wake_count < MAX_CONNECTIONS) { - wake_list[wake_count] = g_conns[i].wsi; - wake_count++; - } +ws_enqueue_outgoing_locked(m); + queued = 1; } pthread_mutex_unlock(&g_mutex); - for (i = 0; i < wake_count; i++) { - if (wake_list[i]) - lws_callback_on_writable((struct lws *)wake_list[i]); - } + if (queued) + ws_wake_service(); } void ws_shutdown_signal(void) diff --git a/src/gateway/ws.h b/src/gateway/ws.h index 9c781e9..0d0fcba 100644 --- a/src/gateway/ws.h +++ b/src/gateway/ws.h @@ -42,6 +42,8 @@ int ws_register_conn(int conn_id, ws_conn_t wsi); /** * Unregister a WebSocket connection. Called when connection closes. + * Drops outgoing frames still queued for @p conn_id so a closed session + * cannot fill the shared outbound queue. * * @param conn_id Connection ID. */ @@ -71,6 +73,9 @@ int ws_pop_incoming(char *session_id_out, size_t session_size, /** * Send text to a WebSocket connection by session_id. * Message is queued and sent from the lws thread. Session ID format: "webchat:". + * Frames targeted at an absent, unregistered, or closed socket (no live wsi) are + * dropped instead of queued and 0 is still returned, so a dead session cannot + * pin the shared outgoing queue. * * @param session_id Recipient session ID. * @param text Text to send. @@ -97,6 +102,17 @@ int ws_dequeue_outgoing(int conn_id, char *buf, size_t buf_size, size_t *len_out */ int ws_has_pending_outgoing(int conn_id); +/** + * Ask the service thread to flush queued WebChat frames. + * Call only from the libwebsockets thread, in + * LWS_CALLBACK_EVENT_WAIT_CANCELLED. lws_callback_on_writable() is not + * safe from the agent thread: libwebsockets 4.3 updates the poll set + * without the per-thread lock. + * + * Example: `case LWS_CALLBACK_EVENT_WAIT_CANCELLED: ws_on_service_cancel(); break;` + */ +void ws_on_service_cancel(void); + /** * Get next connection ID for new connections. * diff --git a/src/providers/anthropic.c b/src/providers/anthropic.c index 2801427..561f477 100644 --- a/src/providers/anthropic.c +++ b/src/providers/anthropic.c @@ -11,6 +11,7 @@ #include "cJSON.h" #include #include +#include #include #include @@ -26,11 +27,37 @@ static const config_t *s_anth_cfg; #ifdef SHELLCLAW_TEST static int s_fail_next_reallocs; +static int s_anth_test_skip_http; +static char s_anth_test_model[160]; void anthropic_test_fail_next_reallocs(int n) { s_fail_next_reallocs = n < 0 ? 0 : n; } + +void anthropic_test_set_skip_http(int skip) +{ + s_anth_test_skip_http = skip ? 1 : 0; +} + +const char *anthropic_test_last_request_model(void) +{ + return s_anth_test_model; +} + +static void anth_test_store_model(const char *model) +{ + size_t n; + + s_anth_test_model[0] = '\0'; + if (!model) + return; + n = strlen(model); + if (n >= sizeof(s_anth_test_model)) + n = sizeof(s_anth_test_model) - 1U; + memcpy(s_anth_test_model, model, n); + s_anth_test_model[n] = '\0'; +} #endif static void *anth_realloc(void *ptr, size_t size) @@ -265,18 +292,149 @@ static int do_request(const char *body, provider_response_t *response) return ret; } +static int append_one_tool_result(cJSON *content_arr, const provider_message_t *msg) +{ + cJSON *tr = cJSON_CreateObject(); + if (!tr) + return -1; + if (!cJSON_AddStringToObject(tr, "type", "tool_result") || + !cJSON_AddStringToObject(tr, "tool_use_id", msg->tool_use_id ? msg->tool_use_id : "") || + !cJSON_AddStringToObject(tr, "content", msg->content ? msg->content : "")) { + cJSON_Delete(tr); + return -1; + } + cJSON_AddItemToArray(content_arr, tr); + return 0; +} + +/* One user message must carry every tool_result for that assistant turn. */ +static int append_tool_result_message(cJSON *msg_arr, const provider_message_t *messages, + size_t message_count, size_t *index) +{ + cJSON *msg = cJSON_CreateObject(); + cJSON *content_arr = cJSON_CreateArray(); + if (!msg || !content_arr) { + cJSON_Delete(msg); + cJSON_Delete(content_arr); + return -1; + } + cJSON_AddItemToObject(msg, "role", cJSON_CreateString("user")); + cJSON_AddItemToObject(msg, "content", content_arr); + cJSON_AddItemToArray(msg_arr, msg); + for (;;) { + if (append_one_tool_result(content_arr, &messages[*index]) != 0) + return -1; + if (*index + 1 >= message_count || !messages[*index + 1].tool_use_id) + break; + (*index)++; + } + return 0; +} +static int append_assistant_tool_content(cJSON *msg, const provider_message_t *message) +{ + cJSON *content_arr = cJSON_CreateArray(); + size_t k; + if (!content_arr) + return -1; + if (message->content && message->content[0] != '\0') { + cJSON *text_block = cJSON_CreateObject(); + if (!text_block) { + cJSON_Delete(content_arr); + return -1; + } + cJSON_AddItemToObject(text_block, "type", cJSON_CreateString("text")); + cJSON_AddItemToObject(text_block, "text", cJSON_CreateString(message->content)); + cJSON_AddItemToArray(content_arr, text_block); + } + for (k = 0; k < message->tool_calls_count; k++) { + const provider_tool_call_t *tc = &message->tool_calls[k]; + cJSON *tu = cJSON_CreateObject(); + cJSON *input; + if (!tu) { + cJSON_Delete(content_arr); + return -1; + } + cJSON_AddItemToObject(tu, "type", cJSON_CreateString("tool_use")); + cJSON_AddItemToObject(tu, "id", cJSON_CreateString(tc->id ? tc->id : "")); + cJSON_AddItemToObject(tu, "name", cJSON_CreateString(tc->name ? tc->name : "")); + input = tc->arguments && tc->arguments[0] ? cJSON_Parse(tc->arguments) : cJSON_CreateObject(); + if (input) + cJSON_AddItemToObject(tu, "input", input); + cJSON_AddItemToArray(content_arr, tu); + } + cJSON_AddItemToObject(msg, "content", content_arr); + return 0; +} + +static int append_anthropic_messages(cJSON *msg_arr, const provider_message_t *messages, + size_t message_count) +{ + size_t i; + for (i = 0; i < message_count; i++) { + const char *role = messages[i].role ? messages[i].role : "user"; + cJSON *msg; + if (strcmp(role, "system") == 0) + continue; + if (messages[i].tool_use_id) { + if (append_tool_result_message(msg_arr, messages, message_count, &i) != 0) + return -1; + continue; + } + msg = cJSON_CreateObject(); + if (!msg) + return -1; + cJSON_AddItemToArray(msg_arr, msg); + cJSON_AddItemToObject(msg, "role", cJSON_CreateString(role)); + if (messages[i].tool_calls && messages[i].tool_calls_count > 0 && + strcmp(role, "assistant") == 0) { + if (append_assistant_tool_content(msg, &messages[i]) != 0) + return -1; + } else { + cJSON_AddItemToObject(msg, "content", + cJSON_CreateString(messages[i].content ? messages[i].content : "")); + } + } + return 0; +} + static int build_and_send(const provider_message_t *messages, size_t message_count, const provider_tool_def_t *tools, size_t tool_count, provider_response_t *response) { + const char *configured; + const char *model; + int max_tokens; + char errmsg[256]; + cJSON *root; if (!s_anth_cfg) { provider_set_error(response, "Anthropic provider not initialized"); return -1; } - cJSON *root = cJSON_CreateObject(); + configured = config_agent_model(s_anth_cfg); + if (!configured || configured[0] == '\0') + configured = "claude-3-5-sonnet-20241022"; + model = provider_model_id_for_backend(configured, "anthropic"); + if (!model) { + snprintf(errmsg, sizeof(errmsg), + "Anthropic skipped: agent model \"%s\" is not an Anthropic id", + configured); + provider_set_error(response, errmsg); + return -1; + } + root = cJSON_CreateObject(); if (!root) { provider_set_error(response, "Out of memory"); return -1; } - const char *model = config_agent_model(s_anth_cfg); - int max_tokens = config_agent_max_tokens(s_anth_cfg); - if (!model) model = "claude-3-5-sonnet-20241022"; + max_tokens = config_agent_max_tokens(s_anth_cfg); if (max_tokens <= 0) max_tokens = 4096; cJSON_AddItemToObject(root, "model", cJSON_CreateString(model)); +#ifdef SHELLCLAW_TEST + if (s_anth_test_skip_http) { + anth_test_store_model(model); + cJSON_Delete(root); + response->content = provider_dup_str(""); + if (!response->content) { + provider_set_error(response, "Out of memory"); + return -1; + } + return 0; + } +#endif cJSON_AddItemToObject(root, "max_tokens", cJSON_CreateNumber(max_tokens)); for (size_t i = 0; i < message_count; i++) { if (messages[i].role && strcmp(messages[i].role, "system") == 0) { @@ -286,59 +444,12 @@ static int build_and_send(const provider_message_t *messages, size_t message_cou } cJSON *msg_arr = cJSON_CreateArray(); if (!msg_arr) { cJSON_Delete(root); provider_set_error(response, "Out of memory"); return -1; } - for (size_t i = 0; i < message_count; i++) { - const char *role = messages[i].role ? messages[i].role : "user"; - if (strcmp(role, "system") == 0) continue; - cJSON *msg = cJSON_CreateObject(); - if (!msg) { - cJSON_Delete(root); - provider_set_error(response, "Out of memory"); - return -1; - } - cJSON_AddItemToObject(msg, "role", cJSON_CreateString(role)); - if (messages[i].tool_use_id) { - cJSON *content_arr = cJSON_CreateArray(); - if (content_arr) { - cJSON *tr = cJSON_CreateObject(); - if (tr) { - cJSON_AddItemToObject(tr, "type", cJSON_CreateString("tool_result")); - cJSON_AddItemToObject(tr, "tool_use_id", cJSON_CreateString(messages[i].tool_use_id)); - cJSON_AddItemToObject(tr, "content", cJSON_CreateString(messages[i].content ? messages[i].content : "")); - cJSON_AddItemToArray(content_arr, tr); - } - cJSON_AddItemToObject(msg, "content", content_arr); - } else - cJSON_AddItemToObject(msg, "content", cJSON_CreateString("")); - } else if (messages[i].tool_calls && messages[i].tool_calls_count > 0 && strcmp(role, "assistant") == 0) { - cJSON *content_arr = cJSON_CreateArray(); - if (content_arr) { - if (messages[i].content && messages[i].content[0] != '\0') { - cJSON *text_block = cJSON_CreateObject(); - if (text_block) { - cJSON_AddItemToObject(text_block, "type", cJSON_CreateString("text")); - cJSON_AddItemToObject(text_block, "text", cJSON_CreateString(messages[i].content)); - cJSON_AddItemToArray(content_arr, text_block); - } - } - for (size_t k = 0; k < messages[i].tool_calls_count; k++) { - const provider_tool_call_t *tc = &messages[i].tool_calls[k]; - cJSON *tu = cJSON_CreateObject(); - if (!tu) break; - cJSON_AddItemToObject(tu, "type", cJSON_CreateString("tool_use")); - cJSON_AddItemToObject(tu, "id", cJSON_CreateString(tc->id ? tc->id : "")); - cJSON_AddItemToObject(tu, "name", cJSON_CreateString(tc->name ? tc->name : "")); - cJSON *input = tc->arguments && tc->arguments[0] ? cJSON_Parse(tc->arguments) : cJSON_CreateObject(); - if (input) cJSON_AddItemToObject(tu, "input", input); - cJSON_AddItemToArray(content_arr, tu); - } - cJSON_AddItemToObject(msg, "content", content_arr); - } else - cJSON_AddItemToObject(msg, "content", cJSON_CreateString(messages[i].content ? messages[i].content : "")); - } else - cJSON_AddItemToObject(msg, "content", cJSON_CreateString(messages[i].content ? messages[i].content : "")); - cJSON_AddItemToArray(msg_arr, msg); - } cJSON_AddItemToObject(root, "messages", msg_arr); + if (append_anthropic_messages(msg_arr, messages, message_count) != 0) { + cJSON_Delete(root); + provider_set_error(response, "Out of memory"); + return -1; + } if (tool_count > 0 && tools) { cJSON *tools_arr = cJSON_CreateArray(); if (tools_arr) { @@ -414,10 +525,39 @@ const provider_t *provider_anthropic_get(void) return &anthropic_provider; } +void provider_anthropic_set_live_config(const config_t *cfg) +{ + if (cfg && s_anth_api_key) + s_anth_cfg = cfg; +} + +const config_t *anthropic_live_config_for_test(void) +{ + return s_anth_cfg; +} + #ifdef SHELLCLAW_TEST int anthropic_parse_response_for_test(const char *json, provider_response_t *response) { if (!json || !response) return -1; return parse_response_body(json, response); } + +char *anthropic_messages_json_for_test(const provider_message_t *messages, size_t message_count) +{ + cJSON *arr; + char *printed; + if (!messages && message_count > 0) + return NULL; + arr = cJSON_CreateArray(); + if (!arr) + return NULL; + if (append_anthropic_messages(arr, messages, message_count) != 0) { + cJSON_Delete(arr); + return NULL; + } + printed = cJSON_PrintUnformatted(arr); + cJSON_Delete(arr); + return printed; +} #endif diff --git a/src/providers/local.c b/src/providers/local.c index 5a3f537..1bcd4aa 100644 --- a/src/providers/local.c +++ b/src/providers/local.c @@ -1,6 +1,7 @@ /** * @file local.c * @brief OpenAI-compatible Chat Completions client for llama-server; init probe marks unreachable without failing startup. + * A later chat probes again so a server that was down only at startup can still serve. */ #define _POSIX_C_SOURCE 200809L @@ -14,14 +15,17 @@ #include #include #include +#include #define REQUEST_TIMEOUT_SEC 120 #define CONNECT_TIMEOUT_SEC 30 #define LOCAL_PROBE_TIMEOUT_SEC 3L #define LOCAL_PROBE_CONNECT_SEC 2L +#define LOCAL_REPROBE_MIN_SEC 15L static const config_t *s_local_cfg; static int s_local_unavailable; +static time_t s_local_reprobe_last_wall; #ifdef SHELLCLAW_TEST static const char *s_local_test_http_body; static int s_local_test_skip_probe; @@ -184,6 +188,24 @@ static int local_chat(const provider_message_t *messages, size_t message_count, provider_set_error(response, "Local provider not initialized"); return -1; } + /* Init probes once, often before llama-server has loaded the model. + * Default fallback order puts local last, so router recovery (which + * only probes backends before the active index) never clears the flag. */ + if (s_local_unavailable) { + const char *endpoint = config_provider_local_endpoint(s_local_cfg); + time_t now = time(NULL); + /* agent_run holds agent_lock while this probe runs synchronously; + * at most one probe per LOCAL_REPROBE_MIN_SEC while unavailable, + * else a down llama-server blocks the gateway/channel every turn. */ + int reprobe_due = s_local_reprobe_last_wall == (time_t)0 || + (unsigned long)(now - s_local_reprobe_last_wall) >= + (unsigned long)LOCAL_REPROBE_MIN_SEC; + if (reprobe_due) { + s_local_reprobe_last_wall = now; + if (endpoint && endpoint[0] != '\0' && provider_local_endpoint_reachable(endpoint)) + provider_local_recovery_probe_succeeded(); + } + } if (s_local_unavailable) { provider_set_error(response, "Local inference server unreachable"); return -1; @@ -196,6 +218,7 @@ static void local_cleanup(void) { s_local_cfg = NULL; s_local_unavailable = 0; + s_local_reprobe_last_wall = (time_t)0; } #ifdef SHELLCLAW_TEST @@ -209,6 +232,7 @@ void local_provider_test_reset(void) s_local_test_http_body = NULL; s_local_test_skip_probe = 0; s_local_unavailable = 0; + s_local_reprobe_last_wall = (time_t)0; } void local_provider_test_set_http_response(const char *json_body) diff --git a/src/providers/openai.c b/src/providers/openai.c index ad2d06c..542e61a 100644 --- a/src/providers/openai.c +++ b/src/providers/openai.c @@ -10,6 +10,7 @@ #include "providers/openai_compat.h" #include "providers/provider.h" #include "cJSON.h" +#include #include #include @@ -24,6 +25,7 @@ static int build_and_send(const provider_message_t *messages, size_t message_cou provider_response_t *response) { const char *endpoint; + const char *configured; const char *model; int max_tokens; double temperature_cfg; @@ -36,11 +38,20 @@ static int build_and_send(const provider_message_t *messages, size_t message_cou endpoint = config_provider_openai_endpoint(s_oai_cfg); if (!endpoint || !endpoint[0]) endpoint = "https://api.openai.com/v1/chat/completions"; - model = config_agent_model(s_oai_cfg); + configured = config_agent_model(s_oai_cfg); + if (!configured || configured[0] == '\0') + configured = "gpt-4o-mini"; + model = provider_model_id_for_backend(configured, "openai"); + if (!model) { + char errmsg[256]; + snprintf(errmsg, sizeof(errmsg), + "OpenAI skipped: agent model \"%s\" is not an OpenAI id", + configured); + provider_set_error(response, errmsg); + return -1; + } max_tokens = config_agent_max_tokens(s_oai_cfg); temperature_cfg = config_agent_temperature(s_oai_cfg); - if (!model) - model = "gpt-4o-mini"; body = openai_compat_build_chat_body(model, max_tokens, temperature_cfg, messages, message_count, tools, tool_count, response); if (!body) diff --git a/src/providers/provider.h b/src/providers/provider.h index 6b8fa1a..d151ca8 100644 --- a/src/providers/provider.h +++ b/src/providers/provider.h @@ -81,6 +81,19 @@ int provider_parse_chat_completions_json(const char *response_body, provider_res /** Non-zero if router may try next chain entry: no `HTTP 4xx` in message, otherwise transport/5xx may retry. */ int provider_error_allows_fallback_retry(const char *provider_error_content); +/** + * Resolve @p model to the id @p backend should send. + * + * A `vendor/rest` id whose vendor matches @p backend returns the rest (a pointer + * into @p model). A known other cloud vendor, a bare Claude id on OpenAI, or a + * bare OpenAI id on Anthropic returns NULL. Callers must fail that case without + * an HTTP status: a 4xx stops the fallback chain. + * + * Example: provider_model_id_for_backend("anthropic/claude-sonnet-4-20250514", "anthropic") + * returns "claude-sonnet-4-20250514". + */ +const char *provider_model_id_for_backend(const char *model, const char *backend); + void provider_router_active_backend_snapshot(char *dst, size_t dst_sz); void provider_router_last_error_snapshot(char *dst, size_t dst_sz); void provider_router_periodic_recovery_tick(time_t now_wall); @@ -124,6 +137,10 @@ void provider_stub_b_set_chat_error_message(const char *message); /** Anthropic (Claude) provider. Messages API, tool_use. */ const provider_t *provider_anthropic_get(void); +/** After SIGHUP or dashboard reload: refresh model and max_tokens without re-init. */ +void provider_anthropic_set_live_config(const config_t *cfg); +/** Config pointer Anthropic chat reads. NULL until init. For tests. */ +const config_t *anthropic_live_config_for_test(void); /** OpenAI provider. Chat Completions, tool_calls (function calling). */ const provider_t *provider_openai_get(void); @@ -134,7 +151,7 @@ void provider_openai_set_live_config(const config_t *cfg); const provider_t *provider_local_get(void); /** After SIGHUP reload: refresh live config reads without re-init (PRD §9 Q4). */ void provider_local_set_live_config(const config_t *cfg); -/** Lightweight GET probe via `/v1/models` or `/health`; used by init and router recovery. */ +/** Lightweight GET probe via `/v1/models` or `/health`; used by init, chat retry, and router recovery. */ int provider_local_endpoint_reachable(const char *endpoint); /** After a successful recovery GET probe on local; clears init-time unreachable flag. */ void provider_local_recovery_probe_succeeded(void); diff --git a/src/providers/provider_common.c b/src/providers/provider_common.c index fb82a62..b2249ee 100644 --- a/src/providers/provider_common.c +++ b/src/providers/provider_common.c @@ -67,6 +67,65 @@ char *provider_dup_str(const char *s) return p; } +static int id_has_prefix(const char *id, const char *prefix) +{ + size_t n = strlen(prefix); + return strncmp(id, prefix, n) == 0; +} + +static int vendor_tag_is_cloud(const char *model, size_t vendor_len) +{ + if (vendor_len == 9U && strncmp(model, "anthropic", 9) == 0) + return 1; + if (vendor_len == 6U && strncmp(model, "openai", 6) == 0) + return 1; + return 0; +} + +static int bare_id_is_openai_family(const char *model) +{ + if (id_has_prefix(model, "gpt-") || id_has_prefix(model, "chatgpt-")) + return 1; + if (id_has_prefix(model, "o1") || id_has_prefix(model, "o3")) + return 1; + return id_has_prefix(model, "o4"); +} + +static const char *bare_model_for_backend(const char *model, const char *backend) +{ + if (strcmp(backend, "openai") == 0 && id_has_prefix(model, "claude")) + return NULL; + if (strcmp(backend, "anthropic") == 0 && bare_id_is_openai_family(model)) + return NULL; + return model; +} + +static int vendor_matches_backend(const char *model, size_t vendor_len, const char *backend) +{ + if (strncmp(model, backend, vendor_len) != 0) + return 0; + return backend[vendor_len] == '\0'; +} + +/* NULL means skip. Sending the foreign id would be HTTP 4xx and stop fallback. */ +const char *provider_model_id_for_backend(const char *model, const char *backend) +{ + const char *slash; + size_t vendor_len; + + if (!model || model[0] == '\0' || !backend || backend[0] == '\0') + return model; + slash = strchr(model, '/'); + if (!slash || slash == model || slash[1] == '\0') + return bare_model_for_backend(model, backend); + vendor_len = (size_t)(slash - model); + if (vendor_matches_backend(model, vendor_len, backend)) + return slash + 1; + if (vendor_tag_is_cloud(model, vendor_len)) + return NULL; + return model; +} + int provider_error_allows_fallback_retry(const char *msg) { const char *h; diff --git a/src/sandbox/sandbox.c b/src/sandbox/sandbox.c index 4aac1a7..fefddc5 100644 --- a/src/sandbox/sandbox.c +++ b/src/sandbox/sandbox.c @@ -12,7 +12,9 @@ * procfs, applies Landlock, sets PR_SET_PDEATHSIG, and closes fds >= 3 * so a timeout SIGKILL of the isolator cannot leave the command under * host init. The isolator joins its cgroup before the command fork so - * memory.max and cpu.max apply to sh -c. Limits degrade if unavailable. + * memory.max and cpu.max apply to sh -c. The command process then + * replaces the inherited environment so provider keys and bot tokens + * are not readable via printenv. Limits degrade if unavailable. * * Non-Linux path: plain fork() + execl(); a warning is emitted to stderr. */ @@ -20,6 +22,7 @@ #define _POSIX_C_SOURCE 200809L #include "sandbox/sandbox.h" +#include "sandbox/sandbox_env.h" #include "sandbox/sandbox_landlock.h" #include #include @@ -107,36 +110,90 @@ static void cgroup_remove(const char *base, const char *name) #endif /* __linux__ */ -static size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms) +/* Sandbox unit tests link drain_pipe as a global under + * SHELLCLAW_SANDBOX_TEST (CR-21 provider-test pattern). CI runners revoke + * user namespaces, so the sandbox_exec success-path tests skip there and + * the wall-clock deadline loop would otherwise stay uncovered. */ +#ifdef SHELLCLAW_SANDBOX_TEST +#define SANDBOX_UNIT +#else +#define SANDBOX_UNIT static +#endif + +/* Docs in sandbox.h. */ +long monotonic_elapsed_ms(const struct timespec *start) +{ + struct timespec now; + long ms; + + if (clock_gettime(CLOCK_MONOTONIC, &now) != 0) + return -1; + ms = (now.tv_sec - start->tv_sec) * 1000L + + (now.tv_nsec - start->tv_nsec) / 1000000L; + return ms < 0 ? 0 : ms; +} + +int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap) { + long remain; + + remain = (long)timeout_ms - elapsed_ms; + if (remain > slice_cap) + return slice_cap; + return (int)remain; +} + +/* + * Stop at timeout_ms of wall time, not idle poll time. A writer that keeps + * the pipe readable otherwise never advances the old counter, so the agent + * stays blocked until cap fills. + */ +SANDBOX_UNIT size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, int *hit_deadline) +{ + struct timespec start; size_t total = 0; - int elapsed_ms = 0; char chunk[512]; - if (cap == 0) return 0; - while (total < cap - 1 && elapsed_ms < timeout_ms) { + + if (hit_deadline) + *hit_deadline = 0; + if (cap == 0) + return 0; + /* Unchecked on purpose: CLOCK_MONOTONIC failure is not a realistic case + * (vDSO). If it ever failed, monotonic_elapsed_ms keeps returning -1, + * the deadline never fires, and poll slices stay bounded. */ + (void)clock_gettime(CLOCK_MONOTONIC, &start); + while (total < cap - 1) { struct pollfd pfd; + long elapsed_ms; int slice; int r; ssize_t n; + + elapsed_ms = monotonic_elapsed_ms(&start); + if (elapsed_ms >= (long)timeout_ms) { + if (hit_deadline) + *hit_deadline = 1; + break; + } + slice = poll_remain_ms(timeout_ms, elapsed_ms, PIPE_POLL_SLICE_MS); pfd.fd = fd; pfd.events = POLLIN; pfd.revents = 0; - slice = timeout_ms - elapsed_ms; - if (slice > PIPE_POLL_SLICE_MS) slice = PIPE_POLL_SLICE_MS; r = poll(&pfd, 1, slice); if (r < 0) { - if (errno == EINTR) continue; + if (errno == EINTR) + continue; break; } - if (r == 0) { - elapsed_ms += slice; + if (r == 0) continue; - } n = read(fd, chunk, sizeof(chunk)); - if (n <= 0) break; + if (n <= 0) + break; { size_t add = (size_t)n; - if (total + add >= cap - 1) add = cap - 1 - total; + if (total + add >= cap - 1) + add = cap - 1 - total; memcpy(buf + total, chunk, add); total += add; } @@ -446,6 +503,7 @@ int sandbox_exec(const char *cmd, char *out, size_t out_cap, #endif close(errpipe[1]); close_inherited_fds(); + sandbox_scrub_command_environment(workspace); execl("/bin/sh", "sh", "-c", cmd, (char *)NULL); _exit(127); } @@ -467,9 +525,14 @@ int sandbox_exec(const char *cmd, char *out, size_t out_cap, return report_isolation_failure(out, out_cap, iso); } } - total = drain_pipe(pipefd[0], out, out_cap, timeout_ms); - close(pipefd[0]); - timed_out = reap_child(pid, NULL); + { + int drain_deadline = 0; + total = drain_pipe(pipefd[0], out, out_cap, timeout_ms, &drain_deadline); + close(pipefd[0]); + timed_out = reap_child(pid, NULL); + if (drain_deadline) + timed_out = 1; + } if (timed_out && total < out_cap - 40) snprintf(out + total, out_cap - total, "\n[Sandbox: command timed out after %d ms]", timeout_ms); diff --git a/src/sandbox/sandbox.h b/src/sandbox/sandbox.h index 277d513..37bc7e8 100644 --- a/src/sandbox/sandbox.h +++ b/src/sandbox/sandbox.h @@ -16,6 +16,7 @@ #define SHELLCLAW_SANDBOX_H #include +#include #ifdef __cplusplus extern "C" { @@ -45,6 +46,25 @@ typedef struct sandbox_config { const char *cgroup_base; } sandbox_config_t; +/** + * Wall-clock milliseconds elapsed since @p start, or -1 when the monotonic + * clock cannot be read. Shared by the sandbox drain loop and the unsandboxed + * shell loop so steady output cannot bypass a timeout (#116). + * + * Example: long ms = monotonic_elapsed_ms(&start); + */ +long monotonic_elapsed_ms(const struct timespec *start); + +/** + * Poll slice for the remaining wall-clock deadline: the positive remainder + * of @p timeout_ms minus @p elapsed_ms, capped at @p slice_cap. Call only + * after checking @p elapsed_ms < @p timeout_ms; an expired deadline is not + * clamped here, so a post-deadline call can return a negative value. + * + * Example: poll_remain_ms(1000, 950, 100) returns 50. + */ +int poll_remain_ms(int timeout_ms, long elapsed_ms, int slice_cap); + /** * Execute @p cmd inside an isolated child process and capture output. * @@ -65,6 +85,8 @@ typedef struct sandbox_config { * @param out Buffer for captured stdout+stderr. Always NUL-terminated on success. * @param out_cap Capacity of @p out (including NUL byte). Must be > 0. * @param timeout_ms Maximum wall-clock milliseconds before SIGKILL. 0 = default (10 000 ms). + * The timeout clock starts only after the isolation handshake, + * so slow namespace/Landlock setup is not counted. * @param cfg Optional sandbox configuration. NULL = use built-in defaults. * @return 0 on success (command ran; check output for exit status text), * -1 on system error (pipe/fork/clone/isolation failure). diff --git a/src/sandbox/sandbox_env.c b/src/sandbox/sandbox_env.c new file mode 100644 index 0000000..158cedf --- /dev/null +++ b/src/sandbox/sandbox_env.c @@ -0,0 +1,77 @@ +/** + * @file sandbox_env.c + * @brief Secret-free environment for the sandboxed command process. + * + * Provider keys and channel tokens live in the agent environment. The + * shell tool's stdout is returned to the model, so sh -c must not inherit + * them. A prompt-injected printenv would otherwise leak the values. + */ +#define _GNU_SOURCE +#define _POSIX_C_SOURCE 200809L + +#include "sandbox/sandbox_env.h" + +#include +#include +#if !defined(__linux__) +#include +#endif + +static const char FALLBACK_PATH[] = + "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"; + +static char *dup_env(const char *name) +{ + const char *v; + + v = getenv(name); + if (!v || !v[0]) + return NULL; + return strdup(v); +} + +static void put_env(const char *name, const char *value) +{ + if (value && value[0]) + setenv(name, value, 1); +} + +static void free_kept(char *path, char *lang, char *lc_all, char *lc_ctype, + char *term, char *tz) +{ + free(path); + free(lang); + free(lc_all); + free(lc_ctype); + free(term); + free(tz); +} + +void sandbox_scrub_command_environment(const char *workspace) +{ + char *path = dup_env("PATH"); + char *lang = dup_env("LANG"); + char *lc_all = dup_env("LC_ALL"); + char *lc_ctype = dup_env("LC_CTYPE"); + char *term = dup_env("TERM"); + char *tz = dup_env("TZ"); +#if !defined(__linux__) + static char *empty_env[] = { NULL }; + extern char **environ; +#endif + +#if defined(__linux__) + clearenv(); +#else + environ = empty_env; +#endif + put_env("PATH", path ? path : FALLBACK_PATH); + put_env("LANG", lang); + put_env("LC_ALL", lc_all); + put_env("LC_CTYPE", lc_ctype); + put_env("TERM", term); + put_env("TZ", tz); + if (workspace && workspace[0]) + setenv("HOME", workspace, 1); + free_kept(path, lang, lc_all, lc_ctype, term, tz); +} diff --git a/src/sandbox/sandbox_env.h b/src/sandbox/sandbox_env.h new file mode 100644 index 0000000..2d4e951 --- /dev/null +++ b/src/sandbox/sandbox_env.h @@ -0,0 +1,18 @@ +/** + * @file sandbox_env.h + * @brief Drop host secrets before exec of a sandboxed shell command. + */ +#ifndef SHELLCLAW_SANDBOX_ENV_H +#define SHELLCLAW_SANDBOX_ENV_H + +/** + * Replace the process environment with PATH, locale, and an optional HOME. + * + * Call only in the child that is about to execl sh -c. API keys, bot + * tokens, and LD_* variables from the agent are not passed through. + * + * Example: sandbox_scrub_command_environment("/home/me/.shellclaw/workspace"); + */ +void sandbox_scrub_command_environment(const char *workspace); + +#endif diff --git a/src/tools/cron.c b/src/tools/cron.c index a6a753b..bf40b15 100644 --- a/src/tools/cron.c +++ b/src/tools/cron.c @@ -6,10 +6,13 @@ #include "tools/cron.h" #include "crypto/crypto.h" +#include "core/agent.h" #include "core/memory.h" #include "channels/channel.h" #include "core/config.h" #include "cJSON.h" +#include +#include #include #include #include @@ -26,14 +29,17 @@ static int parse_field(const char *s, int *out, int min_val, int max_val) { + char *end = NULL; + long n; if (!s || !out) return -1; if (strcmp(s, "*") == 0) { *out = -1; return 0; } - char *end = NULL; - long n = strtol(s, &end, 10); - if (end == s || n < min_val || n > max_val) return -1; + /* strtol stops at the first junk character. "1,15" would otherwise + * match only day 1 and still schedule successfully. */ + n = strtol(s, &end, 10); + if (!end || end == s || *end != '\0' || n < min_val || n > max_val) return -1; *out = (int)n; return 0; } @@ -125,17 +131,38 @@ static long long cron_next_from_expr(const char *cron_part, long long now) return -1; } +/* Reject a span that does not fit in now+seconds. Signed wrap stores a past + * next_run, and ack recomputes the same sum, so the job stays due forever. + * A number that stops early (interval:60abc) is rejected too: strto* would + * otherwise accept the numeric prefix. */ +static int interval_next_run(const char *num, long long now, long long *next_out) +{ + char *end; + long long sec; + + errno = 0; + sec = strtoll(num, &end, 10); + if (end == num || *end != '\0' || errno == ERANGE || sec <= 0) + return -1; + if (now < 0 || sec > LLONG_MAX - now) + return -1; + *next_out = now + sec; + return 0; +} + int cron_parse_next_run(const char *schedule, long long now, long long *next_out) { if (!schedule || !next_out) return -1; - if (strncmp(schedule, CRON_PREFIX_INTERVAL, strlen(CRON_PREFIX_INTERVAL)) == 0) { - long sec = strtol(schedule + strlen(CRON_PREFIX_INTERVAL), NULL, 10); - if (sec <= 0) return -1; - *next_out = now + sec; - return 0; - } + if (strncmp(schedule, CRON_PREFIX_INTERVAL, strlen(CRON_PREFIX_INTERVAL)) == 0) + return interval_next_run(schedule + strlen(CRON_PREFIX_INTERVAL), now, next_out); if (strncmp(schedule, CRON_PREFIX_AT, strlen(CRON_PREFIX_AT)) == 0) { - long long ts = (long long)strtoll(schedule + strlen(CRON_PREFIX_AT), NULL, 10); + const char *num = schedule + strlen(CRON_PREFIX_AT); + char *end = NULL; + long long ts; + /* A partial convert (at:2026-10-08 -> 2026) is already due, so the + * one-shot fires on the next poll and ack deletes the future reminder. */ + ts = strtoll(num, &end, 10); + if (!end || end == num || *end != '\0') return -1; *next_out = ts; return 0; } @@ -320,28 +347,38 @@ static int cron_pick_due_row(long long now, int timeout_ms, cron_job_row_t *row) { cron_due_key_t keys[CRON_DUE_SCAN_MAX]; struct timespec mono; - long age = 0; int n; - int i; + int wait_for_first = 0; if (clock_gettime(CLOCK_MONOTONIC, &mono) != 0) memset(&mono, 0, sizeof(mono)); + /* HTTP dashboard routes use this same connection (SQLITE_THREADSAFE=0). + * Drop the lock before the re-offer sleep so that thread is not stalled. */ + agent_lock(); n = cron_job_list_due(now, keys, CRON_DUE_SCAN_MAX); - if (n <= 0) - return 0; - for (i = 0; i < n; i++) { - if (!cron_due_is_returnable(keys[i].id, timeout_ms, &mono)) - continue; - if (cron_job_get_by_id(keys[i].id, row) == 1) - return 1; + if (n > 0) { + for (int i = 0; i < n; i++) { + if (!cron_due_is_returnable(keys[i].id, timeout_ms, &mono)) + continue; + if (cron_job_get_by_id(keys[i].id, row) == 1) { + agent_unlock(); + return 1; + } + } + if (cron_job_get_by_id(keys[0].id, row) == 1) + wait_for_first = 1; } - if (cron_job_get_by_id(keys[0].id, row) != 1) + agent_unlock(); + if (!wait_for_first) return 0; /* An id that never fit in the table has no age, so wait_remaining would return immediately. */ - if (cron_offer_age_ms(row->id, &mono, &age)) - cron_wait_remaining(row->id, timeout_ms); - else - cron_sleep_ms((long)timeout_ms); + { + long age = 0; + if (cron_offer_age_ms(row->id, &mono, &age)) + cron_wait_remaining(row->id, timeout_ms); + else + cron_sleep_ms((long)timeout_ms); + } return 1; } @@ -392,9 +429,16 @@ int cron_ack_delivery(const char *job_id) } now = (long long)time(NULL); rc = cron_parse_next_run(row.schedule, now, &next); - cron_job_row_free(&row); - if (rc != 0) + if (rc != 0) { + /* A stored schedule the parser now rejects (e.g. a legacy comma + * list) would otherwise snooze the job for a year with no trace; + * log the job so the silent gap is diagnosable. */ + fprintf(stderr, + "shellclaw: cron: job '%s' schedule '%s' rejected; next run snoozed +365d\n", + row.id, row.schedule); next = now + 365LL * 24 * 3600; + } + cron_job_row_free(&row); rc = cron_job_update_next_run(job_id, next); if (rc == 0) cron_offer_release(job_id); diff --git a/src/tools/file.c b/src/tools/file.c index 4e07d83..cef427a 100644 --- a/src/tools/file.c +++ b/src/tools/file.c @@ -259,7 +259,14 @@ static int file_read(const char *path, char *result_buf, size_t max_len) return -1; } size_t to_read = (size_t)sz; - if (to_read >= max_len - 1) to_read = max_len - 1; + /* A prefix with return 0 is the whole file to the model. write_file would + * then replace the original and drop the unread tail. */ + if (to_read + 1 > max_len) { + fclose(f); + snprintf(result_buf, max_len, + "{\"error\":\"file exceeds result buffer\",\"bytes\":%ld}", sz); + return -1; + } size_t n = fread(result_buf, 1, to_read, f); fclose(f); result_buf[n] = '\0'; diff --git a/src/tools/shell.c b/src/tools/shell.c index 65a2d97..bc70a2e 100644 --- a/src/tools/shell.c +++ b/src/tools/shell.c @@ -115,7 +115,8 @@ static int run_unsandboxed(const char *command, int timeout_sec, pid_t pid; size_t total = 0; int timed_out = 0; - int elapsed_ms = 0; + long timeout_ms; + struct timespec start_ts; char buf[256]; if (pipe(pipefd) != 0) { snprintf(result_buf, max_len, "{\"error\":\"pipe failed\"}"); @@ -140,30 +141,36 @@ static int run_unsandboxed(const char *command, int timeout_sec, (void)setpgid(pid, pid); close(pipefd[1]); result_buf[0] = '\0'; - while (total < max_len - 1 && elapsed_ms < timeout_sec * 1000) { + /* Unchecked on purpose: CLOCK_MONOTONIC failure is not a realistic case + * (vDSO). If it ever failed, monotonic_elapsed_ms keeps returning -1, + * the deadline never fires, and poll slices stay bounded. */ + (void)clock_gettime(CLOCK_MONOTONIC, &start_ts); + /* Wall time, not idle poll time: a steady writer never used to trip + * the timeout, so the agent lock stayed held until the buffer filled. */ + timeout_ms = (long)timeout_sec * 1000L; + while (total < max_len - 1) { struct pollfd pfd; + long elapsed_ms; int rem; int r; ssize_t n; + elapsed_ms = monotonic_elapsed_ms(&start_ts); + if (elapsed_ms >= timeout_ms) { + timed_out = 1; + kill_command_tree(pid); + break; + } pfd.fd = pipefd[0]; pfd.events = POLLIN; pfd.revents = 0; - rem = timeout_sec * 1000 - elapsed_ms; - if (rem > 5000) rem = 5000; + rem = poll_remain_ms((int)timeout_ms, elapsed_ms, 5000); r = poll(&pfd, 1, rem); if (r < 0) { if (errno == EINTR) continue; break; } - if (r == 0) { - elapsed_ms += rem; - if (elapsed_ms >= timeout_sec * 1000) { - timed_out = 1; - kill_command_tree(pid); - break; - } + if (r == 0) continue; - } n = read(pipefd[0], buf, sizeof(buf)); if (n <= 0) break; append_unsandboxed_output(result_buf, max_len, &total, buf, (size_t)n); diff --git a/tests/test_agent.c b/tests/test_agent.c index 4b3b7e6..e1e9c22 100644 --- a/tests/test_agent.c +++ b/tests/test_agent.c @@ -366,7 +366,16 @@ static int test_session_persisted_after_exchange(void) } static int compaction_call_count; -static int compaction_init(const config_t *cfg) { (void)cfg; compaction_call_count = 0; return 0; } +static int compaction_source_has_boundary; +static int compaction_source_truncated; +static int compaction_init(const config_t *cfg) +{ + (void)cfg; + compaction_call_count = 0; + compaction_source_has_boundary = 0; + compaction_source_truncated = 0; + return 0; +} static int compaction_chat(const provider_message_t *messages, size_t message_count, const provider_tool_def_t *tools, size_t tool_count, provider_response_t *response) { @@ -377,6 +386,11 @@ static int compaction_chat(const provider_message_t *messages, size_t message_co response->tool_calls_count = 0; compaction_call_count++; if (compaction_call_count == 1) { + const char *src = (message_count >= 2 && messages[1].content) ? messages[1].content : ""; + /* msg_10 is the boundary turn: the summary window must include it; + * the kept tail starts at msg_11. */ + compaction_source_has_boundary = strstr(src, "msg_10\n") != NULL; + compaction_source_truncated = strstr(src, "[truncated]") != NULL; response->content = strdup("Summary of earlier conversation."); return 0; } @@ -442,9 +456,553 @@ static int test_context_compaction_when_history_exceeds_max(void) } if (!found_summary) goto cleanup; if (!found_tail) goto cleanup; + /* Boundary message must be in the summary source; the tail starts at msg_11. */ + if (!compaction_source_has_boundary) goto cleanup; + failed = 0; +cleanup: + config_free(cfg); +cleanup_early: + remove(config_path); + remove(db_path); + memory_cleanup(); + return failed; +} + +/* A message that fits a fresh source but not the space left by earlier lines + * must stay in the session. Deleting it loses history the model never saw. */ +static int test_compaction_keeps_message_that_does_not_fit_summary(void) +{ + enum { HEAD = 30 * 1024, BIG = 5 * 1024, JSON_CAP = 96 * 1024, LOAD_CAP = 128 * 1024 }; + int failed = 1; + config_t *cfg = NULL; + char *session_json = NULL; + char *loaded = NULL; + char *head_a = NULL; + char *head_b = NULL; + char *big = NULL; + const char *db_path = "build/test_agent_compact_fit.db"; + const char *config_path = "build/test_agent_compact_fit.toml"; + size_t off; + int i; + char response_buf[4096]; + char errbuf[256]; + FILE *cf; + + memory_cleanup(); + compaction_call_count = 0; + if (memory_init(db_path) != 0) + goto cleanup_early; + session_json = malloc(JSON_CAP); + head_a = malloc((size_t)HEAD + 1); + head_b = malloc((size_t)HEAD + 1); + big = malloc((size_t)BIG + 1); + loaded = malloc(LOAD_CAP); + if (!session_json || !head_a || !head_b || !big || !loaded) + goto cleanup; + memset(head_a, 'a', (size_t)HEAD); + memcpy(head_a + HEAD - 10, "HEAD_A_END", 10); + head_a[HEAD] = '\0'; + memset(head_b, 'b', (size_t)HEAD); + memcpy(head_b + HEAD - 10, "HEAD_B_END", 10); + head_b[HEAD] = '\0'; + memset(big, 'A', (size_t)BIG); + memcpy(big + BIG - 12, "BIG_KEEP_END", 12); + big[BIG] = '\0'; + off = 0; + off += (size_t)snprintf(session_json + off, JSON_CAP - off, "["); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + "{\"role\":\"user\",\"content\":\"%s\"}", head_a); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"user\",\"content\":\"%s\"}", head_b); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"user\",\"content\":\"%s\"}", big); + for (i = 3; i < 6; i++) { + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"user\",\"content\":\"m%d\"}", i); + } + off += (size_t)snprintf(session_json + off, JSON_CAP - off, "]"); + if (off >= JSON_CAP) + goto cleanup; + if (session_save("cli:compactfit", session_json) != 0) + goto cleanup; + cf = fopen(config_path, "w"); + if (!cf) + goto cleanup; + fprintf(cf, "[agent]\nmodel = \"test\"\nmax_context_messages = 4\n[memory]\npath = \"%s\"\n", + db_path); + fclose(cf); + if (config_load(config_path, &cfg, errbuf, sizeof(errbuf)) != 0) + goto cleanup; + response_buf[0] = '\0'; + if (agent_run(cfg, "cli:compactfit", "hi", &compaction_provider, NULL, 0, + response_buf, sizeof(response_buf)) != 0) + goto cleanup; + if (session_load("cli:compactfit", loaded, LOAD_CAP) != 0) + goto cleanup; + if (strstr(loaded, "BIG_KEEP_END") == NULL) { + fprintf(stderr, + "FAIL: compacted session dropped the message that did not fit the summary\n"); + goto cleanup; + } + if (strstr(loaded, "Summary of earlier") == NULL) { + fprintf(stderr, "FAIL: expected a summary of the messages that did fit\n"); + goto cleanup; + } + if (strstr(loaded, "HEAD_A_END") != NULL || strstr(loaded, "HEAD_B_END") != NULL) { + fprintf(stderr, "FAIL: head messages that fit the summary were kept raw\n"); + goto cleanup; + } + failed = 0; +cleanup: + config_free(cfg); + free(session_json); + free(head_a); + free(head_b); + free(big); + free(loaded); +cleanup_early: + remove(config_path); + remove(db_path); + memory_cleanup(); + return failed; +} + +static int count_session_messages(const char *json) +{ + cJSON *root; + int n; + + root = cJSON_Parse(json); + if (!root || !cJSON_IsArray(root)) { + if (root) + cJSON_Delete(root); + return -1; + } + n = cJSON_GetArraySize(root); + cJSON_Delete(root); + return n; +} + +/* An oversized FIRST line used to abort compaction with an empty source, so + * the session never shrank again and every turn retried in vain. The line now + * reaches the summarizer as a truncated stub and the session keeps shrinking. */ +static int test_compaction_progresses_when_first_message_is_oversized(void) +{ + enum { OVERSIZE = 65 * 1024, JSON_CAP = 96 * 1024, LOAD_CAP = 128 * 1024 }; + int failed = 1; + config_t *cfg = NULL; + char *session_json = NULL; + char *loaded = NULL; + char *big = NULL; + const char *db_path = "build/test_agent_compact_stall_first.db"; + const char *config_path = "build/test_agent_compact_stall_first.toml"; + size_t off; + int i; + char response_buf[4096]; + char errbuf[256]; + FILE *cf; + + memory_cleanup(); + compaction_call_count = 0; + if (memory_init(db_path) != 0) + goto cleanup_early; + session_json = malloc(JSON_CAP); + big = malloc((size_t)OVERSIZE + 1); + loaded = malloc(LOAD_CAP); + if (!session_json || !big || !loaded) + goto cleanup; + memset(big, 'A', (size_t)OVERSIZE); + memcpy(big + OVERSIZE - 11, "STALL_A_END", 11); + big[OVERSIZE] = '\0'; + off = 0; + off += (size_t)snprintf(session_json + off, JSON_CAP - off, "["); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + "{\"role\":\"user\",\"content\":\"%s\"}", big); + for (i = 1; i < 6; i++) { + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"user\",\"content\":\"m%d\"}", i); + } + off += (size_t)snprintf(session_json + off, JSON_CAP - off, "]"); + if (off >= JSON_CAP) + goto cleanup; + if (session_save("cli:stallfirst", session_json) != 0) + goto cleanup; + cf = fopen(config_path, "w"); + if (!cf) + goto cleanup; + fprintf(cf, "[agent]\nmodel = \"test\"\nmax_context_messages = 4\n[memory]\npath = \"%s\"\n", + db_path); + fclose(cf); + if (config_load(config_path, &cfg, errbuf, sizeof(errbuf)) != 0) + goto cleanup; + response_buf[0] = '\0'; + if (agent_run(cfg, "cli:stallfirst", "hi", &compaction_provider, NULL, 0, + response_buf, sizeof(response_buf)) != 0) + goto cleanup; + if (compaction_call_count < 2) { + fprintf(stderr, "FAIL: compaction aborted before the summarizer call\n"); + goto cleanup; + } + if (!compaction_source_truncated) { + fprintf(stderr, + "FAIL: oversized first line did not reach the summary as a truncated stub\n"); + goto cleanup; + } + if (session_load("cli:stallfirst", loaded, LOAD_CAP) != 0) + goto cleanup; + if (strstr(loaded, "STALL_A_END") != NULL) { + fprintf(stderr, "FAIL: oversized first line is still raw instead of summarized\n"); + goto cleanup; + } + if (strstr(loaded, "Summary of earlier") == NULL) { + fprintf(stderr, "FAIL: expected a summary of the truncated oversized turn\n"); + goto cleanup; + } + /* A follow-up turn must finish shrinking the session down to max_ctx. */ + if (agent_run(cfg, "cli:stallfirst", "hi", &compaction_provider, NULL, 0, + response_buf, sizeof(response_buf)) != 0) + goto cleanup; + if (session_load("cli:stallfirst", loaded, LOAD_CAP) != 0) + goto cleanup; + if (count_session_messages(loaded) > 4 + 2) { + fprintf(stderr, "FAIL: session never shrinks back to max_ctx after the oversized turn\n"); + goto cleanup; + } + failed = 0; +cleanup: + config_free(cfg); + free(session_json); + free(big); + free(loaded); +cleanup_early: + remove(config_path); + remove(db_path); + memory_cleanup(); + return failed; +} + +/* An oversized LATER line used to be kept raw with everything after it, so + * new_count stayed above max_ctx forever. The truncated stub now consumes it + * and this pass brings the session back down to max_ctx. */ +static int test_compaction_progresses_when_later_message_is_oversized(void) +{ + enum { OVERSIZE = 65 * 1024, JSON_CAP = 96 * 1024, LOAD_CAP = 128 * 1024 }; + int failed = 1; + config_t *cfg = NULL; + char *session_json = NULL; + char *loaded = NULL; + char *big = NULL; + const char *db_path = "build/test_agent_compact_stall_later.db"; + const char *config_path = "build/test_agent_compact_stall_later.toml"; + size_t off; + int i; + char response_buf[4096]; + char errbuf[256]; + FILE *cf; + + memory_cleanup(); + compaction_call_count = 0; + if (memory_init(db_path) != 0) + goto cleanup_early; + session_json = malloc(JSON_CAP); + big = malloc((size_t)OVERSIZE + 1); + loaded = malloc(LOAD_CAP); + if (!session_json || !big || !loaded) + goto cleanup; + memset(big, 'A', (size_t)OVERSIZE); + memcpy(big + OVERSIZE - 11, "STALL_B_END", 11); + big[OVERSIZE] = '\0'; + off = 0; + off += (size_t)snprintf(session_json + off, JSON_CAP - off, "["); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + "{\"role\":\"user\",\"content\":\"alpha_small\"}"); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"user\",\"content\":\"%s\"}", big); + for (i = 2; i < 6; i++) { + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"user\",\"content\":\"m%d\"}", i); + } + off += (size_t)snprintf(session_json + off, JSON_CAP - off, "]"); + if (off >= JSON_CAP) + goto cleanup; + if (session_save("cli:stalllater", session_json) != 0) + goto cleanup; + cf = fopen(config_path, "w"); + if (!cf) + goto cleanup; + fprintf(cf, "[agent]\nmodel = \"test\"\nmax_context_messages = 4\n[memory]\npath = \"%s\"\n", + db_path); + fclose(cf); + if (config_load(config_path, &cfg, errbuf, sizeof(errbuf)) != 0) + goto cleanup; + response_buf[0] = '\0'; + if (agent_run(cfg, "cli:stalllater", "hi", &compaction_provider, NULL, 0, + response_buf, sizeof(response_buf)) != 0) + goto cleanup; + if (!compaction_source_truncated) { + fprintf(stderr, + "FAIL: oversized later line did not reach the summary as a truncated stub\n"); + goto cleanup; + } + if (session_load("cli:stalllater", loaded, LOAD_CAP) != 0) + goto cleanup; + if (strstr(loaded, "STALL_B_END") != NULL) { + fprintf(stderr, "FAIL: oversized later line is still raw instead of summarized\n"); + goto cleanup; + } + if (strstr(loaded, "Summary of earlier") == NULL) { + fprintf(stderr, "FAIL: expected a summary of the truncated oversized turn\n"); + goto cleanup; + } + if (count_session_messages(loaded) > 4 + 2) { + fprintf(stderr, "FAIL: session is not back to max_ctx after the oversized turn\n"); + goto cleanup; + } + failed = 0; +cleanup: + config_free(cfg); + free(session_json); + free(big); + free(loaded); +cleanup_early: + remove(config_path); + remove(db_path); + memory_cleanup(); + return failed; +} + +static int save_alternating_session(const char *session_id, int count) +{ + char *json; + size_t cap; + size_t off = 0; + int i; + int rc; + + cap = (size_t)count * 80u + 2u; + json = malloc(cap); + if (!json) + return -1; + off += (size_t)snprintf(json + off, cap - off, "["); + for (i = 0; i < count; i++) { + const char *role = (i % 2 == 0) ? "user" : "assistant"; + int n = snprintf(json + off, cap - off, + "%s{\"role\":\"%s\",\"content\":\"msg_%d\"}", + i ? "," : "", role, i); + if (n < 0 || (size_t)n >= cap - off) { + free(json); + return -1; + } + off += (size_t)n; + } + if (off + 1 >= cap) { + free(json); + return -1; + } + json[off++] = ']'; + json[off] = '\0'; + rc = session_save(session_id, json); + free(json); + return rc; +} + +static int json_roles_alternate_from_user(const char *json) +{ + cJSON *root; + int n; + int i; + const char *prev = NULL; + + root = cJSON_Parse(json); + if (!root || !cJSON_IsArray(root)) { + if (root) + cJSON_Delete(root); + return 0; + } + n = cJSON_GetArraySize(root); + if (n <= 0) { + cJSON_Delete(root); + return 0; + } + for (i = 0; i < n; i++) { + cJSON *item = cJSON_GetArrayItem(root, i); + cJSON *role = item ? cJSON_GetObjectItem(item, "role") : NULL; + const char *r = (role && cJSON_IsString(role) && role->valuestring) ? role->valuestring : ""; + if (i == 0 && strcmp(r, "user") != 0) { + cJSON_Delete(root); + return 0; + } + if (prev && strcmp(prev, r) == 0) { + cJSON_Delete(root); + return 0; + } + prev = r; + } + cJSON_Delete(root); + return 1; +} + +static int compact_fail_calls; +static int compact_fail_init(const config_t *cfg) +{ + (void)cfg; + compact_fail_calls = 0; + return 0; +} +static int compact_fail_chat(const provider_message_t *messages, size_t message_count, + const provider_tool_def_t *tools, size_t tool_count, provider_response_t *response) +{ + (void)messages; + (void)message_count; + (void)tools; + (void)tool_count; + response->tool_calls = NULL; + response->tool_calls_count = 0; + response->error = 0; + compact_fail_calls++; + if (compact_fail_calls == 1) { + response->content = strdup("Summary of earlier conversation."); + return 0; + } + provider_set_error(response, "mock provider error"); + return -1; +} +static void compact_fail_cleanup(void) {} +static const provider_t compact_fail_provider = { + .name = "compact_fail", + .init = compact_fail_init, + .chat = compact_fail_chat, + .cleanup = compact_fail_cleanup, +}; + +/* Summary is computed before the user turn. Saving it immediately deletes the + * raw transcript when the follow-up chat fails and the turn is not stored. */ +static int test_compaction_failure_keeps_original_session(void) +{ + int failed = 1; + config_t *cfg = NULL; + char *loaded = NULL; + const char *db_path = "build/test_agent_compact_fail.db"; + const char *config_path = "build/test_agent_compact_fail.toml"; + char response_buf[4096]; + char errbuf[256]; + FILE *cf; + + memory_cleanup(); + compact_fail_calls = 0; + if (memory_init(db_path) != 0) + goto cleanup_early; + if (save_alternating_session("cli:compactfail", 50) != 0) + goto cleanup; + cf = fopen(config_path, "w"); + if (!cf) + goto cleanup; + fprintf(cf, "[agent]\nmodel = \"test\"\nmax_context_messages = 40\n[memory]\npath = \"%s\"\n", + db_path); + fclose(cf); + if (config_load(config_path, &cfg, errbuf, sizeof(errbuf)) != 0) + goto cleanup; + loaded = malloc(128 * 1024); + if (!loaded) + goto cleanup; + response_buf[0] = '\0'; + if (agent_run(cfg, "cli:compactfail", "hi", &compact_fail_provider, NULL, 0, + response_buf, sizeof(response_buf)) == 0) + goto cleanup; + if (strstr(response_buf, "mock provider error") == NULL) + goto cleanup; + if (session_load("cli:compactfail", loaded, 128 * 1024) != 0) + goto cleanup; + if (strstr(loaded, "\"content\":\"msg_0\"") == NULL) { + fprintf(stderr, "FAIL: failed turn replaced the stored transcript\n"); + goto cleanup; + } + if (strstr(loaded, "Summary of earlier") != NULL) { + fprintf(stderr, "FAIL: summary was saved even though the turn failed\n"); + goto cleanup; + } + failed = 0; +cleanup: + config_free(cfg); + free(loaded); +cleanup_early: + remove(config_path); + remove(db_path); + memory_cleanup(); + return failed; +} + +/* Odd max_context_messages cuts the tail on a user turn. A separate summary + * user message then makes every later Anthropic call a 400. */ +static int test_compaction_merges_summary_when_tail_starts_with_user(void) +{ + int failed = 1; + config_t *cfg = NULL; + char *loaded = NULL; + const char *db_path = "build/test_agent_compact_roles.db"; + const char *config_path = "build/test_agent_compact_roles.toml"; + char response_buf[4096]; + char errbuf[256]; + FILE *cf; + size_t i; + + memory_cleanup(); + compaction_call_count = 0; + if (memory_init(db_path) != 0) + goto cleanup_early; + if (save_alternating_session("cli:compactroles", 8) != 0) + goto cleanup; + cf = fopen(config_path, "w"); + if (!cf) + goto cleanup; + fprintf(cf, "[agent]\nmodel = \"test\"\nmax_context_messages = 5\n[memory]\npath = \"%s\"\n", + db_path); + fclose(cf); + if (config_load(config_path, &cfg, errbuf, sizeof(errbuf)) != 0) + goto cleanup; + loaded = malloc(64 * 1024); + if (!loaded) + goto cleanup; + response_buf[0] = '\0'; + spy_roles_clear(); + if (agent_run(cfg, "cli:compactroles", "hi", &compaction_provider, NULL, 0, + response_buf, sizeof(response_buf)) != 0) + goto cleanup; + if (compaction_call_count < 2) + goto cleanup; + if (spy_message_count < 6 || spy_message_count > SPY_SLOTS) + goto cleanup; + if (!spy_roles[0] || strcmp(spy_roles[0], "system") != 0) + goto cleanup; + if (!spy_roles[1] || strcmp(spy_roles[1], "user") != 0) + goto cleanup; + if (strstr(spy_content[1], "Summary of earlier") == NULL || + strstr(spy_content[1], "msg_4") == NULL) { + fprintf(stderr, "FAIL: summary was not folded into the leading user turn\n"); + goto cleanup; + } + for (i = 2; i < spy_message_count; i++) { + if (!spy_roles[i] || strcmp(spy_roles[i], spy_roles[i - 1]) == 0) { + fprintf(stderr, "FAIL: compacted history has consecutive roles\n"); + goto cleanup; + } + } + if (session_load("cli:compactroles", loaded, 64 * 1024) != 0) + goto cleanup; + if (strstr(loaded, "Summary of earlier") == NULL || strstr(loaded, "msg_4") == NULL) + goto cleanup; + if (strstr(loaded, "\"content\":\"msg_0\"") != NULL) { + fprintf(stderr, "FAIL: successful turn did not persist compaction\n"); + goto cleanup; + } + if (!json_roles_alternate_from_user(loaded)) { + fprintf(stderr, "FAIL: persisted session roles do not alternate\n"); + goto cleanup; + } failed = 0; cleanup: config_free(cfg); + free(loaded); cleanup_early: remove(config_path); remove(db_path); @@ -452,6 +1010,7 @@ static int test_context_compaction_when_history_exceeds_max(void) return failed; } + static int test_local_offline_note_when_active_is_local(void) { const char *path = "build/test_agent_local_note.toml"; @@ -1186,6 +1745,11 @@ int main(void) RUN(test_react_loop_preserves_prior_tool_results()); RUN(test_session_persisted_after_exchange()); RUN(test_context_compaction_when_history_exceeds_max()); + RUN(test_compaction_keeps_message_that_does_not_fit_summary()); + RUN(test_compaction_progresses_when_first_message_is_oversized()); + RUN(test_compaction_progresses_when_later_message_is_oversized()); + RUN(test_compaction_failure_keeps_original_session()); + RUN(test_compaction_merges_summary_when_tail_starts_with_user()); RUN(test_local_offline_note_when_active_is_local()); RUN(test_local_offline_note_skipped_for_non_local()); RUN(test_session_overflow_does_not_corrupt_history()); diff --git a/tests/test_anthropic.c b/tests/test_anthropic.c index a82b28f..3af1281 100644 --- a/tests/test_anthropic.c +++ b/tests/test_anthropic.c @@ -6,6 +6,7 @@ #include "core/config.h" #include "providers/provider.h" +#include "cJSON.h" #include #include #include @@ -15,7 +16,10 @@ #ifdef SHELLCLAW_TEST extern int anthropic_parse_response_for_test(const char *json, provider_response_t *response); +extern char *anthropic_messages_json_for_test(const provider_message_t *messages, size_t message_count); extern void anthropic_test_fail_next_reallocs(int n); +extern void anthropic_test_set_skip_http(int skip); +extern const char *anthropic_test_last_request_model(void); #endif static const char *TMP_CONFIG = "/tmp/shellclaw_test_anthropic_config.toml"; @@ -233,6 +237,270 @@ static int test_parse_text_realloc_failure_is_error(void) return 0; } +static int test_parallel_tool_results_share_one_user_message(void) +{ + provider_tool_call_t calls[2] = { + {.id = "toolu_a", .name = "shell", .arguments = "{\"command\":\"pwd\"}"}, + {.id = "toolu_b", .name = "read_file", .arguments = "{\"path\":\"a.txt\"}"}, + }; + provider_message_t messages[4]; + char *json; + cJSON *arr; + cJSON *assistant; + cJSON *user; + cJSON *content; + cJSON *role; + cJSON *block; + + memset(messages, 0, sizeof(messages)); + messages[0].role = "user"; + messages[0].content = "do both"; + messages[1].role = "assistant"; + messages[1].content = ""; + messages[1].tool_calls = calls; + messages[1].tool_calls_count = 2; + messages[2].role = "user"; + messages[2].content = "out-a"; + messages[2].tool_use_id = "toolu_a"; + messages[3].role = "user"; + messages[3].content = "out-b"; + messages[3].tool_use_id = "toolu_b"; + json = anthropic_messages_json_for_test(messages, 4); + ASSERT(json != NULL); + arr = cJSON_Parse(json); + cJSON_free(json); + ASSERT(arr != NULL && cJSON_IsArray(arr)); + ASSERT(cJSON_GetArraySize(arr) == 3); + assistant = cJSON_GetArrayItem(arr, 1); + content = assistant ? cJSON_GetObjectItem(assistant, "content") : NULL; + ASSERT(cJSON_IsArray(content) && cJSON_GetArraySize(content) == 2); + user = cJSON_GetArrayItem(arr, 2); + role = user ? cJSON_GetObjectItem(user, "role") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "user") == 0); + content = user ? cJSON_GetObjectItem(user, "content") : NULL; + ASSERT(cJSON_IsArray(content) && cJSON_GetArraySize(content) == 2); + block = cJSON_GetArrayItem(content, 0); + role = block ? cJSON_GetObjectItem(block, "tool_use_id") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "toolu_a") == 0); + block = cJSON_GetArrayItem(content, 1); + role = block ? cJSON_GetObjectItem(block, "tool_use_id") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "toolu_b") == 0); + cJSON_Delete(arr); + return 0; +} + +/** Old path unregressed: one tool result still yields exactly one user message. */ +static int test_single_tool_result_one_user_message(void) +{ + provider_tool_call_t calls[1] = { + {.id = "toolu_a", .name = "shell", .arguments = "{\"command\":\"pwd\"}"}, + }; + provider_message_t messages[3]; + char *json; + cJSON *arr; + cJSON *user; + cJSON *role; + cJSON *content; + cJSON *block; + + memset(messages, 0, sizeof(messages)); + messages[0].role = "user"; + messages[0].content = "run pwd"; + messages[1].role = "assistant"; + messages[1].content = ""; + messages[1].tool_calls = calls; + messages[1].tool_calls_count = 1; + messages[2].role = "user"; + messages[2].content = "out-a"; + messages[2].tool_use_id = "toolu_a"; + json = anthropic_messages_json_for_test(messages, 3); + ASSERT(json != NULL); + arr = cJSON_Parse(json); + cJSON_free(json); + ASSERT(arr != NULL && cJSON_IsArray(arr)); + ASSERT(cJSON_GetArraySize(arr) == 3); + user = cJSON_GetArrayItem(arr, 2); + role = user ? cJSON_GetObjectItem(user, "role") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "user") == 0); + content = user ? cJSON_GetObjectItem(user, "content") : NULL; + ASSERT(cJSON_IsArray(content) && cJSON_GetArraySize(content) == 1); + block = cJSON_GetArrayItem(content, 0); + role = block ? cJSON_GetObjectItem(block, "tool_use_id") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "toolu_a") == 0); + cJSON_Delete(arr); + return 0; +} + +/** Grouping must stop at the first message without a tool_use_id. */ +static int test_tool_results_stop_at_plain_user_message(void) +{ + provider_tool_call_t calls[2] = { + {.id = "toolu_a", .name = "shell", .arguments = "{\"command\":\"pwd\"}"}, + {.id = "toolu_b", .name = "read_file", .arguments = "{\"path\":\"a.txt\"}"}, + }; + provider_message_t messages[5]; + char *json; + cJSON *arr; + cJSON *user; + cJSON *role; + cJSON *content; + cJSON *block; + + memset(messages, 0, sizeof(messages)); + messages[0].role = "user"; + messages[0].content = "do both"; + messages[1].role = "assistant"; + messages[1].content = ""; + messages[1].tool_calls = calls; + messages[1].tool_calls_count = 2; + messages[2].role = "user"; + messages[2].content = "out-a"; + messages[2].tool_use_id = "toolu_a"; + messages[3].role = "user"; + messages[3].content = "out-b"; + messages[3].tool_use_id = "toolu_b"; + messages[4].role = "user"; + messages[4].content = "what now?"; + json = anthropic_messages_json_for_test(messages, 5); + ASSERT(json != NULL); + arr = cJSON_Parse(json); + cJSON_free(json); + ASSERT(arr != NULL && cJSON_IsArray(arr)); + ASSERT(cJSON_GetArraySize(arr) == 4); + user = cJSON_GetArrayItem(arr, 2); + role = user ? cJSON_GetObjectItem(user, "role") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "user") == 0); + content = user ? cJSON_GetObjectItem(user, "content") : NULL; + ASSERT(cJSON_IsArray(content) && cJSON_GetArraySize(content) == 2); + block = cJSON_GetArrayItem(content, 0); + role = block ? cJSON_GetObjectItem(block, "tool_use_id") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "toolu_a") == 0); + block = cJSON_GetArrayItem(content, 1); + role = block ? cJSON_GetObjectItem(block, "tool_use_id") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "toolu_b") == 0); + /* The plain user message after the results stays its own message. */ + user = cJSON_GetArrayItem(arr, 3); + role = user ? cJSON_GetObjectItem(user, "role") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "user") == 0); + content = user ? cJSON_GetObjectItem(user, "content") : NULL; + ASSERT(cJSON_IsString(content) && strcmp(content->valuestring, "what now?") == 0); + cJSON_Delete(arr); + return 0; +} + +/** Results of a second round must not merge across the assistant message. */ +static int test_tool_results_do_not_merge_across_assistant_rounds(void) +{ + provider_tool_call_t calls_a[1] = { + {.id = "toolu_a", .name = "shell", .arguments = "{\"command\":\"pwd\"}"}, + }; + provider_tool_call_t calls_c[1] = { + {.id = "toolu_c", .name = "read_file", .arguments = "{\"path\":\"b.txt\"}"}, + }; + provider_message_t messages[5]; + char *json; + cJSON *arr; + cJSON *assistant; + cJSON *user; + cJSON *role; + cJSON *content; + cJSON *block; + + memset(messages, 0, sizeof(messages)); + messages[0].role = "user"; + messages[0].content = "start"; + messages[1].role = "assistant"; + messages[1].content = ""; + messages[1].tool_calls = calls_a; + messages[1].tool_calls_count = 1; + messages[2].role = "user"; + messages[2].content = "out-a"; + messages[2].tool_use_id = "toolu_a"; + messages[3].role = "assistant"; + messages[3].content = ""; + messages[3].tool_calls = calls_c; + messages[3].tool_calls_count = 1; + messages[4].role = "user"; + messages[4].content = "out-c"; + messages[4].tool_use_id = "toolu_c"; + json = anthropic_messages_json_for_test(messages, 5); + ASSERT(json != NULL); + arr = cJSON_Parse(json); + cJSON_free(json); + ASSERT(arr != NULL && cJSON_IsArray(arr)); + ASSERT(cJSON_GetArraySize(arr) == 5); + /* Round one result message: exactly its own block. */ + user = cJSON_GetArrayItem(arr, 2); + role = user ? cJSON_GetObjectItem(user, "role") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "user") == 0); + content = user ? cJSON_GetObjectItem(user, "content") : NULL; + ASSERT(cJSON_IsArray(content) && cJSON_GetArraySize(content) == 1); + /* The second assistant round keeps its own assistant message. */ + assistant = cJSON_GetArrayItem(arr, 3); + role = assistant ? cJSON_GetObjectItem(assistant, "role") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "assistant") == 0); + content = assistant ? cJSON_GetObjectItem(assistant, "content") : NULL; + ASSERT(cJSON_IsArray(content) && cJSON_GetArraySize(content) == 1); + block = cJSON_GetArrayItem(content, 0); + role = block ? cJSON_GetObjectItem(block, "id") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "toolu_c") == 0); + /* Round two result lands in its own user message. */ + user = cJSON_GetArrayItem(arr, 4); + role = user ? cJSON_GetObjectItem(user, "role") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "user") == 0); + content = user ? cJSON_GetObjectItem(user, "content") : NULL; + ASSERT(cJSON_IsArray(content) && cJSON_GetArraySize(content) == 1); + block = cJSON_GetArrayItem(content, 0); + role = block ? cJSON_GetObjectItem(block, "tool_use_id") : NULL; + ASSERT(cJSON_IsString(role) && strcmp(role->valuestring, "toolu_c") == 0); + cJSON_Delete(arr); + return 0; +} + +static int test_example_model_prefix_is_stripped(void) +{ + const char *saved = getenv("ANTHROPIC_API_KEY"); + char *saved_copy = saved ? strdup(saved) : NULL; + config_t *cfg = NULL; + char errbuf[256]; + provider_response_t response = {0}; + const provider_t *p; + provider_message_t msg = { + .role = "user", .content = "Hi", .tool_calls = NULL, + .tool_calls_count = 0, .tool_use_id = NULL + }; + FILE *f; + int chat_ret; + + f = fopen(TMP_CONFIG, "w"); + ASSERT(f != NULL); + fprintf(f, "[agent]\nmodel = \"anthropic/claude-sonnet-4-20250514\"\nmax_tokens = 256\n"); + fprintf(f, "[memory]\ndb_path = \"/tmp/shellclaw_anth_test.db\"\n"); + fprintf(f, "[providers.anthropic]\napi_key_env = \"ANTHROPIC_API_KEY\"\n"); + fclose(f); + ASSERT(setenv("ANTHROPIC_API_KEY", "sk-test-not-a-real-key", 1) == 0); + anthropic_test_set_skip_http(1); + ASSERT(config_load(TMP_CONFIG, &cfg, errbuf, sizeof(errbuf)) == 0); + p = provider_anthropic_get(); + ASSERT(p != NULL); + ASSERT(p->init(cfg) == 0); + chat_ret = p->chat(&msg, 1, NULL, 0, &response); + ASSERT(chat_ret == 0); + ASSERT(strcmp(anthropic_test_last_request_model(), "claude-sonnet-4-20250514") == 0); + p->cleanup(); + config_free(cfg); + remove(TMP_CONFIG); + anthropic_test_set_skip_http(0); + if (saved_copy) { + setenv("ANTHROPIC_API_KEY", saved_copy, 1); + free(saved_copy); + } else { + unsetenv("ANTHROPIC_API_KEY"); + } + provider_response_clear(&response); + return 0; +} + static int test_parse_tool_realloc_failure_is_error(void) { char body[1024]; @@ -268,8 +536,13 @@ int main(void) RUN(test_parse_null_json_returns_error()); RUN(test_parse_text_past_initial_cap()); RUN(test_parse_six_tool_use_blocks()); + RUN(test_parallel_tool_results_share_one_user_message()); + RUN(test_single_tool_result_one_user_message()); + RUN(test_tool_results_stop_at_plain_user_message()); + RUN(test_tool_results_do_not_merge_across_assistant_rounds()); RUN(test_parse_text_realloc_failure_is_error()); RUN(test_parse_tool_realloc_failure_is_error()); + RUN(test_example_model_prefix_is_stripped()); #endif printf("test_anthropic: all tests passed\n"); return 0; diff --git a/tests/test_channel.c b/tests/test_channel.c index a6a7493..4d656ff 100644 --- a/tests/test_channel.c +++ b/tests/test_channel.c @@ -1,10 +1,12 @@ /** * @file test_channel.c - * @brief Unit tests for channel_t vtable and stub channel. + * @brief Unit tests for channel_t vtable, stub channel, chunked send helper. */ +#define _POSIX_C_SOURCE 200809L #include "channels/channel.h" #include +#include #include static int tests_run = 0; @@ -74,6 +76,157 @@ static void test_stub_cleanup(void) MU_ASSERT(1, "stub cleanup does not crash"); } +static void test_outbound_chunk_len(void) +{ + /* U+20AC EURO SIGN is the three-byte sequence e2 82 ac. */ + const char euro[] = "x\xE2\x82\xAC" "y"; + size_t n; + + MU_ASSERT(channel_outbound_chunk_len(NULL, 10) == 0, "NULL text is empty"); + MU_ASSERT(channel_outbound_chunk_len("", 10) == 0, "empty text is empty"); + MU_ASSERT(channel_outbound_chunk_len("hi", 0) == 0, "zero cap is empty"); + MU_ASSERT(channel_outbound_chunk_len("hello", 5) == 5, "exact fit stays one piece"); + MU_ASSERT(channel_outbound_chunk_len("hello", 2) == 2, "ascii splits on the cap"); + n = channel_outbound_chunk_len(euro, 2); + MU_ASSERT(n == 1, "multibyte character is not split"); + MU_ASSERT(channel_outbound_chunk_len(euro + n, 3) == 3, "next piece is the whole euro sign"); +} + +static void test_outbound_chunk_len_4byte_rune(void) +{ + /* U+1F600 GRINNING FACE is the four-byte sequence f0 9f 98 80. */ + const char grin[] = "xx\xF0\x9F\x98\x80" "yy"; + size_t n = channel_outbound_chunk_len(grin, 3); + + MU_ASSERT(n == 2, "4-byte code point at the split edge is not split"); + n = channel_outbound_chunk_len(grin + n, 4); + MU_ASSERT(n == 4, "next piece is the whole 4-byte code point"); +} + +/* --- channel_send_chunked --- */ + +#define CHUNK_TEST_MAX_PIECES 64 + +typedef struct { + char *pieces[CHUNK_TEST_MAX_PIECES]; + size_t count; +} chunk_recorder_t; + +static int record_piece(const char *piece, void *userdata) +{ + chunk_recorder_t *rec = (chunk_recorder_t *)userdata; + + if (rec->count >= CHUNK_TEST_MAX_PIECES) + return -1; + rec->pieces[rec->count] = strdup(piece); + if (!rec->pieces[rec->count]) + return -1; + rec->count++; + return 0; +} + +static void recorder_clear(chunk_recorder_t *rec) +{ + size_t i; + + for (i = 0; i < rec->count; i++) + free(rec->pieces[i]); + rec->count = 0; +} + +static void test_send_chunked_round_trip(void) +{ + /* ASCII, a 3-byte euro sign, and a 4-byte grinning face. */ + const char *input = "hi \xE2\x82\xAC\xF0\x9F\x98\x80 tail text!"; + char joined[256]; + size_t joined_len = 0; + chunk_recorder_t rec; + channel_chunk_fail_t fail; + size_t i; + + fail.index = 99; + fail.offset = 99; + rec.count = 0; + MU_ASSERT(channel_send_chunked(input, 4, record_piece, &rec, &fail) == 0, + "chunked send over multibyte text succeeds"); + MU_ASSERT(rec.count >= 2, "text longer than max_bytes is split"); + for (i = 0; i < rec.count; i++) { + size_t plen; + + MU_ASSERT(rec.pieces[i] != NULL, "recorded piece is non-NULL"); + MU_ASSERT(rec.pieces[i][0] != '\0', "no piece is empty"); + plen = strlen(rec.pieces[i]); + MU_ASSERT(joined_len + plen < sizeof(joined), "joined buffer fits"); + memcpy(joined + joined_len, rec.pieces[i], plen); + joined_len += plen; + } + joined[joined_len] = '\0'; + MU_ASSERT(strcmp(joined, input) == 0, "pieces concatenate back to the input"); + MU_ASSERT(fail.index == 0 && fail.offset == 0, "fail stays zeroed on success"); + recorder_clear(&rec); + + /* Length within the cap is delivered as a single piece. */ + MU_ASSERT(channel_send_chunked(input, 128, record_piece, &rec, &fail) == 0, + "text within max_bytes sends as one piece"); + MU_ASSERT(rec.count == 1, "single piece for text within max_bytes"); + MU_ASSERT(rec.pieces[0] && strcmp(rec.pieces[0], input) == 0, + "single piece equals the input"); + recorder_clear(&rec); +} + +typedef struct { + size_t calls; +} chunk_counter_t; + +static int post_fail_on_third(const char *piece, void *userdata) +{ + chunk_counter_t *counter = (chunk_counter_t *)userdata; + + (void)piece; + counter->calls++; + return counter->calls == 3 ? -1 : 0; +} + +static void test_send_chunked_reports_failing_chunk(void) +{ + channel_chunk_fail_t fail; + chunk_counter_t counter; + int r; + + fail.index = 99; + fail.offset = 99; + counter.calls = 0; + /* 12 bytes / max 4: chunks "abcd" (0), "efgh" (4), "ijkl" (8). */ + r = channel_send_chunked("abcdefghijkl", 4, post_fail_on_third, &counter, &fail); + MU_ASSERT(r != 0, "send fails when a chunk post fails"); + MU_ASSERT(counter.calls == 3, "chunks before the failure are posted"); + MU_ASSERT(fail.index == 2, "failing chunk index is reported"); + MU_ASSERT(fail.offset == 8, "failing chunk byte offset is reported"); +} + +static void test_send_chunked_contract_edges(void) +{ + channel_chunk_fail_t fail; + chunk_recorder_t rec; + + fail.index = 99; + fail.offset = 99; + rec.count = 0; + MU_ASSERT(channel_send_chunked(NULL, 10, record_piece, &rec, &fail) != 0, + "NULL text fails"); + MU_ASSERT(channel_send_chunked("hi", 0, record_piece, &rec, &fail) != 0, + "zero max_bytes fails"); + MU_ASSERT(channel_send_chunked("hi", 10, NULL, &rec, &fail) != 0, + "NULL post callback fails"); + MU_ASSERT(rec.count == 0, "invalid arguments post no pieces"); + MU_ASSERT(fail.index == 0 && fail.offset == 0, + "fail stays zeroed without a post failure"); + /* Empty text is a no-op success: there is no chunk to post. */ + MU_ASSERT(channel_send_chunked("", 10, record_piece, &rec, &fail) == 0, + "empty text is a no-op success"); + MU_ASSERT(rec.count == 0, "empty text posts no pieces"); +} + int main(void) { MU_RUN(test_stub_channel_exists); @@ -82,6 +235,11 @@ int main(void) MU_RUN(test_incoming_msg_clear_safe); MU_RUN(test_stub_send_succeeds); MU_RUN(test_stub_cleanup); + MU_RUN(test_outbound_chunk_len); + MU_RUN(test_outbound_chunk_len_4byte_rune); + MU_RUN(test_send_chunked_round_trip); + MU_RUN(test_send_chunked_reports_failing_chunk); + MU_RUN(test_send_chunked_contract_edges); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; } diff --git a/tests/test_cron.c b/tests/test_cron.c index 2e8369c..5770f68 100644 --- a/tests/test_cron.c +++ b/tests/test_cron.c @@ -5,6 +5,7 @@ #define _POSIX_C_SOURCE 200809L #include "tools/cron.h" +#include "core/agent.h" #include "core/memory.h" #include "channels/channel.h" #include @@ -15,6 +16,13 @@ #define ASSERT(c) do { if (!(c)) { fprintf(stderr, "FAIL: %s:%d %s\n", __FILE__, __LINE__, #c); return 1; } } while (0) #define RUN(t) do { int r = (t); if (r) return r; } while (0) +/** Satisfies agent.o. Cron tests never enter the local-backend prompt path. */ +void provider_router_active_backend_snapshot(char *dst, size_t dst_sz) +{ + if (dst && dst_sz > 0) + dst[0] = '\0'; +} + static void cron_test_reset_offers(void) { const channel_t *cron_ch = channel_cron_get(); @@ -124,6 +132,62 @@ static int test_invalid_schedule(void) return 0; } +/* A seconds value that does not fit in now+sec used to wrap next_run negative. + * Ack recomputes that sum, so the job stays due and the agent runs it forever. */ +static int test_interval_overflow_does_not_stay_due(void) +{ + const char *path = "/tmp/shellclaw_test_cron_interval_overflow.db"; + const char *schedule = "interval:9223372036854775807"; + cron_job_row_t row; + long long now; + long long next = 0; + long long floor_next; + long long ceil_next; + + now = 1700000000; + ASSERT(cron_parse_next_run(schedule, now, &next) == -1); + ASSERT(cron_parse_next_run("interval:999999999999999999999", now, &next) == -1); + remove(path); + ASSERT(memory_init(path) == 0); + now = (long long)time(NULL); + ASSERT(cron_job_create("overflow", schedule, "msg", "cli", "default", now - 1, 1) == 0); + ASSERT(cron_ack_delivery("overflow") == 0); + memset(&row, 0, sizeof(row)); + ASSERT(cron_job_get_by_id("overflow", &row) == 1); + floor_next = now + 365LL * 24 * 3600 - 2; + ceil_next = now + 365LL * 24 * 3600 + 2; + ASSERT(row.next_run >= floor_next); + ASSERT(row.next_run <= ceil_next); + cron_job_row_free(&row); + memory_cleanup(); + remove(path); + return 0; +} + +/* strto* used to stop at the first non-digit and still return success. + * at:2026-10-08 became next_run 2026 (already due), so the one-shot fired + * on the next poll and ack deleted it. 1,3,5 matched only the first day. */ +static int test_schedule_rejects_partial_numbers(void) +{ + long long now = 1700000000; + long long next = 99; + + ASSERT(cron_parse_next_run("at:2026-10-08", now, &next) == -1); + ASSERT(next == 99); + ASSERT(cron_parse_next_run("at:tomorrow", now, &next) == -1); + ASSERT(cron_parse_next_run("at:", now, &next) == -1); + ASSERT(cron_parse_next_run("interval:60abc", now, &next) == -1); + ASSERT(cron_parse_next_run("cron:0 9 * * 1,3,5", now, &next) == -1); + ASSERT(cron_parse_next_run("cron:0 9 1,15 * *", now, &next) == -1); + ASSERT(cron_parse_next_run("0 9 * * 1-5", now, &next) == 0); + ASSERT(next > now); + ASSERT(cron_parse_next_run("at:1700000100", now, &next) == 0); + ASSERT(next == 1700000100); + ASSERT(cron_parse_next_run("interval:60", now, &next) == 0); + ASSERT(next == now + 60); + return 0; +} + static int test_cron_job_crud_and_due(void) { const char *path = "/tmp/shellclaw_test_cron.db"; @@ -321,6 +385,46 @@ static int test_cron_ack_delivery_deferred(void) return 0; } +static int g_poll_sqlite_hits; +static int g_poll_sqlite_unlocked; + +static void poll_sqlite_probe(void) +{ + g_poll_sqlite_hits++; + if (!agent_mutex_is_locked_for_test()) + g_poll_sqlite_unlocked = 1; +} + +/** Due-job SQL must run under agent_lock; the HTTP thread uses the same g_db. */ +static int test_cron_poll_holds_agent_lock(void) +{ + const channel_t *cron_ch; + channel_incoming_msg_t msg; + const char *path = "/tmp/shellclaw_test_cron_poll_lock.db"; + long long now; + + cron_test_reset_offers(); + remove(path); + ASSERT(memory_init(path) == 0); + now = (long long)time(NULL); + ASSERT(cron_job_create("poll_lock", "at:9999999999", "Due", "cli", "default", now - 1, 1) == 0); + cron_ch = channel_cron_get(); + ASSERT(cron_ch != NULL && cron_ch->poll != NULL); + g_poll_sqlite_hits = 0; + g_poll_sqlite_unlocked = 0; + memory_sqlite_entry_set_hook_for_test(poll_sqlite_probe); + memset(&msg, 0, sizeof(msg)); + ASSERT(cron_ch->poll(&msg, 0) == 1); + memory_sqlite_entry_set_hook_for_test(NULL); + ASSERT(g_poll_sqlite_hits > 0); + ASSERT(g_poll_sqlite_unlocked == 0); + ASSERT(agent_mutex_is_locked_for_test() == 0); + channel_incoming_msg_clear(&msg); + memory_cleanup(); + remove(path); + return 0; +} + static int test_cron_poll_keeps_job_until_ack(void) { cron_test_reset_offers(); @@ -659,12 +763,15 @@ int main(void) RUN(test_cron_expr_skips_current_minute()); RUN(test_cron_expr_monthly_beyond_eight_days()); RUN(test_invalid_schedule()); + RUN(test_interval_overflow_does_not_stay_due()); + RUN(test_schedule_rejects_partial_numbers()); RUN(test_cron_job_crud_and_due()); RUN(test_cron_tool_execute()); RUN(test_one_shot_detection()); RUN(test_due_job_delivers_full_message()); RUN(test_long_interval_schedule_roundtrips()); RUN(test_cron_ack_delivery_deferred()); + RUN(test_cron_poll_holds_agent_lock()); RUN(test_cron_poll_keeps_job_until_ack()); RUN(test_cron_ack_advances_recurring_past_due_minute()); RUN(test_cron_ack_fail_closed_on_parse_error()); diff --git a/tests/test_dispatch.c b/tests/test_dispatch.c index 2478f42..4286bac 100644 --- a/tests/test_dispatch.c +++ b/tests/test_dispatch.c @@ -468,8 +468,32 @@ static int test_handle_message_holds_agent_mutex(void) return 0; } +static int test_api_session_delete_holds_agent_mutex(void) +{ + const char *db_path = "build/test_dispatch_api_delete.db"; + char history[128]; + + memory_cleanup(); + remove(db_path); + ASSERT(memory_init(db_path) == 0); + ASSERT(session_save("webchat:9", "[{\"role\":\"user\",\"content\":\"secret\"}]") == 0); + g_agent_mutex_held_during_reset = 0; + session_delete_set_hook_for_test(reset_lock_probe); + ASSERT(dispatch_delete_session("webchat:9") == 0); + session_delete_set_hook_for_test(NULL); + ASSERT(g_agent_mutex_held_during_reset == 1); + ASSERT(agent_mutex_is_locked_for_test() == 0); + ASSERT(session_load("webchat:9", history, sizeof(history)) != 0); + ASSERT(dispatch_delete_session("") != 0); + ASSERT(dispatch_delete_session(NULL) != 0); + memory_cleanup(); + remove(db_path); + return 0; +} + int main(void) { + RUN(test_api_session_delete_holds_agent_mutex()); RUN(test_reset_clears_session()); RUN(test_status_returns_version()); RUN(test_agent_failure_fallback_message()); diff --git a/tests/test_file.c b/tests/test_file.c index 0907d27..490ba6f 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -563,6 +563,58 @@ static void test_write_in_workspace_symlink_alias_rejected(void) rmdir(tmpdir); } +static void test_read_rejects_file_larger_than_result_buffer(void) +{ + char tmpdir[64]; + char notes_path[PATH_MAX]; + char config_path[PATH_MAX]; + char args[PATH_MAX + 64]; + /* Large enough for the ~49-byte error JSON, still smaller than the 64-byte file. */ + char buf[56]; + char exact[8]; + FILE *f; + config_t *cfg; + const tool_t *t; + int i; + + snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_readcap_%d", (int)getpid()); + if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) + return; + snprintf(notes_path, sizeof(notes_path), "%s/notes.md", tmpdir); + f = fopen(notes_path, "w"); + MU_ASSERT(f != NULL, "create oversized notes.md"); + for (i = 0; i < 64; i++) + fputc('A', f); + fclose(f); + cfg = make_ws_config(tmpdir, config_path, sizeof(config_path)); + MU_ASSERT(cfg != NULL, "readcap: load config"); + tool_file_set_config(cfg); + t = tool_file_get(); + snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", notes_path); + MU_ASSERT(t->execute(args, buf, sizeof(buf)) == -1, "oversize read fails"); + MU_ASSERT(strstr(buf, "\"error\":\"file exceeds result buffer\"") != NULL, + "oversize read reports the refuse-to-truncate error"); + MU_ASSERT(strstr(buf, "\"bytes\":64") != NULL, "error reports the file size"); + MU_ASSERT(strstr(buf, "AAAAAAAAAAAAAAAA") == NULL, "oversize read is not a raw prefix"); + f = fopen(notes_path, "w"); + MU_ASSERT(f != NULL, "rewrite exact-fit notes.md"); + fputs("0123456", f); + fclose(f); + MU_ASSERT(t->execute(args, exact, sizeof(exact)) == 0, "exact-fit read succeeds"); + MU_ASSERT(strcmp(exact, "0123456") == 0, "exact-fit read returns the whole file"); + /* One over the boundary: size == max_len must fail, not clip (max_len - 1 covered above). */ + f = fopen(notes_path, "w"); + MU_ASSERT(f != NULL, "rewrite one-over notes.md"); + fputs("01234567", f); + fclose(f); + MU_ASSERT(t->execute(args, exact, sizeof(exact)) == -1, "one-over read fails"); + MU_ASSERT(strstr(exact, "error") != NULL, "one-over read reports an error, not a prefix"); + config_free(cfg); + unlink(config_path); + unlink(notes_path); + rmdir(tmpdir); +} + int main(void) { MU_RUN(test_file_read_write_list); @@ -577,6 +629,7 @@ int main(void) MU_RUN(test_file_write_failure_preserves_existing); MU_RUN(test_write_does_not_clobber_sibling_tmp); MU_RUN(test_write_in_workspace_symlink_alias_rejected); + MU_RUN(test_read_rejects_file_larger_than_result_buffer); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; } diff --git a/tests/test_gateway_db_lock.c b/tests/test_gateway_db_lock.c new file mode 100644 index 0000000..ed4fb88 --- /dev/null +++ b/tests/test_gateway_db_lock.c @@ -0,0 +1,90 @@ +/** + * @file test_gateway_db_lock.c + * @brief Dashboard SQLite routes must hold agent_lock (SQLITE_THREADSAFE=0). + */ +#define _POSIX_C_SOURCE 200809L + +#include "core/agent.h" +#include "core/memory.h" +#include "gateway/http_lws.h" +#include "gateway/routes.h" +#include +#include + +#define ASSERT(c) do { \ + if (!(c)) { \ + fprintf(stderr, "FAIL: %s:%d %s\n", __FILE__, __LINE__, #c); \ + return 1; \ + } \ +} while (0) + +static int g_sqlite_hits; +static int g_sqlite_unlocked; + +static void sqlite_probe(void) +{ + g_sqlite_hits++; + if (!agent_mutex_is_locked_for_test()) + g_sqlite_unlocked = 1; +} + +static int call_route(http_server_ctx_t *ctx, int method, const char *uri, + const char *body, int *status) +{ + char buf[4096]; + int hits_before = g_sqlite_hits; + + buf[0] = '\0'; + *status = 0; + ASSERT(dispatch_route(ctx, NULL, method, uri, (int)strlen(uri), + body, body ? strlen(body) : 0, buf, sizeof(buf), status) == 0); + ASSERT(g_sqlite_hits > hits_before); + ASSERT(g_sqlite_unlocked == 0); + ASSERT(agent_mutex_is_locked_for_test() == 0); + return 0; +} + +static int test_dashboard_sqlite_holds_agent_lock(void) +{ + http_server_ctx_t ctx; + const char *path = "/tmp/shellclaw_test_gateway_db_lock.db"; + const char *create_body = + "{\"id\":\"job_lock\",\"schedule\":\"interval:3600\",\"message\":\"ping\"}"; + int status = 0; + + memset(&ctx, 0, sizeof(ctx)); + remove(path); + ASSERT(memory_init(path) == 0); + g_sqlite_hits = 0; + g_sqlite_unlocked = 0; + memory_sqlite_entry_set_hook_for_test(sqlite_probe); + + ASSERT(call_route(&ctx, HTTP_GET, "/api/sessions", NULL, &status) == 0); + ASSERT(status == 200); + ASSERT(session_save("sess_lock", "[]") == 0); + ASSERT(call_route(&ctx, HTTP_DELETE, "/api/sessions/sess_lock", NULL, &status) == 0); + ASSERT(status == 200); + ASSERT(call_route(&ctx, HTTP_GET, "/api/memory", NULL, &status) == 0); + ASSERT(call_route(&ctx, HTTP_GET, "/api/cron", NULL, &status) == 0); + ASSERT(status == 200); + ASSERT(call_route(&ctx, HTTP_POST, "/api/cron", create_body, &status) == 0); + ASSERT(status == 201); + ASSERT(call_route(&ctx, HTTP_POST, "/api/cron/job_lock/toggle", NULL, &status) == 0); + ASSERT(status == 200); + ASSERT(call_route(&ctx, HTTP_DELETE, "/api/cron/job_lock", NULL, &status) == 0); + ASSERT(status == 200); + + memory_sqlite_entry_set_hook_for_test(NULL); + ASSERT(agent_mutex_is_locked_for_test() == 0); + memory_cleanup(); + remove(path); + return 0; +} + +int main(void) +{ + if (test_dashboard_sqlite_holds_agent_lock() != 0) + return 1; + printf("test_gateway_db_lock: all tests passed\n"); + return 0; +} diff --git a/tests/test_gateway_http.c b/tests/test_gateway_http.c index 58b3bfb..f2b6aab 100644 --- a/tests/test_gateway_http.c +++ b/tests/test_gateway_http.c @@ -1179,6 +1179,13 @@ static int test_api_skill_create_delete(const char *token) ASSERT(code == 200 || code == 201); free(body); body = NULL; + r = http_get_auth(gw_url("/api/skills/test_integration_skill"), token, &code, &body); + ASSERT(r == 0); + ASSERT(code == 200); + ASSERT(body != NULL); + ASSERT(strstr(body, "Test skill for integration") != NULL); + free(body); + body = NULL; r = http_delete_auth(gw_url("/api/skills/test_integration_skill"), token, &code, &body); ASSERT(r == 0); ASSERT(code == 200); @@ -1186,6 +1193,65 @@ static int test_api_skill_create_delete(const char *token) return 0; } +/* A skill larger than the edit buffer must not come back as HTTP 200. + * The dashboard saves whatever GET returned, which used to drop the tail. */ +static int test_api_skill_get_refuses_oversize(const char *token) +{ + enum { BODY_N = 40000 }; + const char *tail = "SKILL_TAIL_KEEP"; + char *content; + char *json; + char path[256]; + char disk[32]; + long code; + char *body = NULL; + FILE *f; + size_t tail_len; + size_t json_cap; + int n; + int r; + + tail_len = strlen(tail); + content = malloc((size_t)BODY_N + 1); + ASSERT(content != NULL); + memset(content, 'B', (size_t)BODY_N); + memcpy(content + (size_t)BODY_N - tail_len, tail, tail_len); + content[BODY_N] = '\0'; + json_cap = (size_t)BODY_N + 64; + json = malloc(json_cap); + ASSERT(json != NULL); + n = snprintf(json, json_cap, "{\"name\":\"big_skill\",\"content\":\"%s\"}", content); + ASSERT(n > 0 && (size_t)n < json_cap); + r = http_post_auth(gw_url("/api/skills"), token, json, &code, &body); + ASSERT(r == 0); + ASSERT(code == 200 || code == 201); + free(body); + body = NULL; + r = http_get_auth(gw_url("/api/skills/big_skill"), token, &code, &body); + ASSERT(r == 0); + ASSERT(code == 413); + ASSERT(body != NULL); + ASSERT(strstr(body, "big_skill") != NULL); + ASSERT(strstr(body, "exceeds") != NULL); + free(body); + snprintf(path, sizeof(path), "%s/.shellclaw/skills/big_skill.md", g_test_home); + f = fopen(path, "r"); + ASSERT(f != NULL); + ASSERT(fseek(f, -((long)tail_len), SEEK_END) == 0); + ASSERT(fread(disk, 1, tail_len, f) == tail_len); + disk[tail_len] = '\0'; + fclose(f); + ASSERT(strcmp(disk, tail) == 0); + free(content); + free(json); + body = NULL; + r = http_delete_auth(gw_url("/api/skills/big_skill"), token, &code, &body); + ASSERT(r == 0); + ASSERT(code == 200); + free(body); + return 0; +} + static int test_api_memory(const char *token) { long code; @@ -1728,6 +1794,10 @@ int main(int argc, char **argv) if (test_api_context_snapshot_get(token) != 0) { fprintf(stderr, "test_api_context_snapshot_get failed\n"); failed++; } if (test_api_skills_list(token) != 0) { fprintf(stderr, "test_api_skills_list failed\n"); failed++; } if (test_api_skill_create_delete(token) != 0) { fprintf(stderr, "test_api_skill_create_delete failed\n"); failed++; } + if (test_api_skill_get_refuses_oversize(token) != 0) { + fprintf(stderr, "test_api_skill_get_refuses_oversize failed\n"); + failed++; + } if (test_api_memory(token) != 0) { fprintf(stderr, "test_api_memory failed\n"); failed++; } if (test_api_cron_list(token) != 0) { fprintf(stderr, "test_api_cron_list failed\n"); failed++; } if (test_api_cron_create_delete(token) != 0) { fprintf(stderr, "test_api_cron_create_delete failed\n"); failed++; } diff --git a/tests/test_local_provider.c b/tests/test_local_provider.c index 859f679..1b4dc13 100644 --- a/tests/test_local_provider.c +++ b/tests/test_local_provider.c @@ -75,6 +75,120 @@ static int test_init_unreachable_sets_unavailable_then_chat_errors(void) return 0; } +/** + * Init probe failure must not stick for the process lifetime. llama-server + * often is not listening yet at startup; the next chat has to probe again. + */ +static int test_chat_reprobes_when_server_comes_up(void) +{ + FILE *f = fopen(TMP_CFG, "w"); + config_t *cfg = NULL; + char errbuf[256]; + const provider_t *p; + provider_message_t msg = { .role = "user", .content = "ping", .tool_calls = NULL, + .tool_calls_count = 0, .tool_use_id = NULL }; + provider_response_t response = {0}; + ASSERT(f); + local_provider_test_reset(); + unsetenv("SHELLCLAW_FALLBACK_CHAIN"); + unsetenv("SHELLCLAW_LOCAL_ENDPOINT"); + unsetenv("SHELLCLAW_LOCAL_MODEL"); + fprintf(f, "[agent]\nmodel = \"test-model\"\n"); + fprintf(f, "[providers.local]\n"); + fprintf(f, "endpoint = \"http://127.0.0.1:1/v1/chat/completions\"\nmodel = \"tiny\"\n"); + fclose(f); + ASSERT(config_load(TMP_CFG, &cfg, errbuf, sizeof(errbuf)) == 0); + p = provider_local_get(); + ASSERT(p->init(cfg) == 0); +#ifdef SHELLCLAW_TEST + ASSERT(local_provider_is_unavailable_for_test() != 0); +#endif + local_provider_test_set_skip_probe(1); + local_provider_test_set_http_response( + "{\"choices\":[{\"message\":{\"role\":\"assistant\",\"content\":\"back online\"}}]}"); + ASSERT(p->chat(&msg, 1, NULL, 0, &response) == 0); + ASSERT(response.error == 0); + ASSERT(response.content != NULL); + ASSERT(strstr(response.content, "back online") != NULL); +#ifdef SHELLCLAW_TEST + ASSERT(local_provider_is_unavailable_for_test() == 0); +#endif + p->cleanup(); + provider_response_clear(&response); + local_provider_test_reset(); + config_free(cfg); + remove(TMP_CFG); + return 0; +} + +/** + * While unavailable, chat re-probes at most once per LOCAL_REPROBE_MIN_SEC: + * agent_run holds agent_lock during chat, so a down llama-server must not + * cost synchronous curl probes on every turn. The second chat inside the + * window must not probe (skip_probe would otherwise recover it through the + * mock); after the test hook resets the throttle it must probe again. + */ +static int test_chat_reprobe_throttled_within_min_interval(void) +{ + FILE *f = fopen(TMP_CFG, "w"); + config_t *cfg = NULL; + char errbuf[256]; + const provider_t *p; + provider_message_t msg = { .role = "user", .content = "ping", .tool_calls = NULL, + .tool_calls_count = 0, .tool_use_id = NULL }; + provider_response_t response = {0}; + ASSERT(f); + local_provider_test_reset(); + unsetenv("SHELLCLAW_FALLBACK_CHAIN"); + unsetenv("SHELLCLAW_LOCAL_ENDPOINT"); + unsetenv("SHELLCLAW_LOCAL_MODEL"); + fprintf(f, "[agent]\nmodel = \"test-model\"\n"); + fprintf(f, "[providers.local]\n"); + fprintf(f, "endpoint = \"http://127.0.0.1:1/v1/chat/completions\"\nmodel = \"tiny\"\n"); + fclose(f); + ASSERT(config_load(TMP_CFG, &cfg, errbuf, sizeof(errbuf)) == 0); + p = provider_local_get(); + ASSERT(p->init(cfg) == 0); +#ifdef SHELLCLAW_TEST + ASSERT(local_provider_is_unavailable_for_test() != 0); +#endif + /* First chat after the startup miss: the probe runs against port 1 + * and fails, arming the throttle window. */ + ASSERT(p->chat(&msg, 1, NULL, 0, &response) == -1); + ASSERT(response.error != 0); + ASSERT(strstr(response.content, "unreachable") != NULL); + provider_response_clear(&response); + /* Second chat inside the window: no probe, so skip_probe cannot clear + * the flag and the mock is not served. */ + local_provider_test_set_skip_probe(1); + local_provider_test_set_http_response( + "{\"choices\":[{\"message\":{\"role\":\"assistant\",\"content\":\"back online\"}}]}"); + ASSERT(p->chat(&msg, 1, NULL, 0, &response) == -1); + ASSERT(response.error != 0); + ASSERT(strstr(response.content, "unreachable") != NULL); + provider_response_clear(&response); + /* After the hook resets the throttle and init re-marks unavailable, + * the next chat must probe again and recover through the mock. */ + local_provider_test_reset(); + ASSERT(p->init(cfg) == 0); +#ifdef SHELLCLAW_TEST + ASSERT(local_provider_is_unavailable_for_test() != 0); +#endif + local_provider_test_set_skip_probe(1); + local_provider_test_set_http_response( + "{\"choices\":[{\"message\":{\"role\":\"assistant\",\"content\":\"back online\"}}]}"); + ASSERT(p->chat(&msg, 1, NULL, 0, &response) == 0); + ASSERT(response.error == 0); + ASSERT(response.content != NULL); + ASSERT(strstr(response.content, "back online") != NULL); + p->cleanup(); + provider_response_clear(&response); + local_provider_test_reset(); + config_free(cfg); + remove(TMP_CFG); + return 0; +} + static int test_chat_mock_http_happy_path(void) { FILE *f = fopen(TMP_CFG, "w"); @@ -113,6 +227,8 @@ int main(void) RUN(test_local_provider_vtable()); RUN(test_init_requires_config()); RUN(test_init_unreachable_sets_unavailable_then_chat_errors()); + RUN(test_chat_reprobes_when_server_comes_up()); + RUN(test_chat_reprobe_throttled_within_min_interval()); RUN(test_chat_mock_http_happy_path()); curl_global_cleanup(); printf("test_local_provider: all tests passed\n"); diff --git a/tests/test_openai.c b/tests/test_openai.c index 3214712..9876987 100644 --- a/tests/test_openai.c +++ b/tests/test_openai.c @@ -103,6 +103,52 @@ static int test_init_and_chat_if_key_set(void) return 0; } +static int test_prefixed_claude_model_skips_without_http(void) +{ + const char *saved = getenv("OPENAI_API_KEY"); + char *saved_copy = saved ? strdup(saved) : NULL; + config_t *cfg = NULL; + char errbuf[256]; + provider_response_t response = {0}; + const provider_t *p; + provider_message_t msg = { + .role = "user", .content = "Hi", .tool_calls = NULL, + .tool_calls_count = 0, .tool_use_id = NULL + }; + FILE *f; + int chat_ret; + + f = fopen(TMP_CONFIG, "w"); + ASSERT(f != NULL); + fprintf(f, "[agent]\nmodel = \"anthropic/claude-sonnet-4-20250514\"\nmax_tokens = 256\n"); + fprintf(f, "[memory]\ndb_path = \"/tmp/shellclaw_openai_test.db\"\n"); + fprintf(f, "[providers.openai]\napi_key_env = \"OPENAI_API_KEY\"\n"); + fclose(f); + ASSERT(setenv("OPENAI_API_KEY", "sk-test-not-a-real-key", 1) == 0); + ASSERT(config_load(TMP_CONFIG, &cfg, errbuf, sizeof(errbuf)) == 0); + p = provider_openai_get(); + ASSERT(p != NULL); + ASSERT(p->init(cfg) == 0); + chat_ret = p->chat(&msg, 1, NULL, 0, &response); + p->cleanup(); + config_free(cfg); + remove(TMP_CONFIG); + if (saved_copy) { + setenv("OPENAI_API_KEY", saved_copy, 1); + free(saved_copy); + } else { + unsetenv("OPENAI_API_KEY"); + } + ASSERT(chat_ret == -1); + ASSERT(response.error != 0); + ASSERT(response.content != NULL); + ASSERT(strstr(response.content, "anthropic/claude-sonnet-4-20250514") != NULL); + ASSERT(strstr(response.content, "HTTP ") == NULL); + ASSERT(provider_error_allows_fallback_retry(response.content) == 1); + provider_response_clear(&response); + return 0; +} + #ifdef SHELLCLAW_TEST static int test_parse_malformed_json_sets_error(void) { @@ -219,6 +265,7 @@ int main(void) RUN(test_init_fails_without_api_key_in_env()); RUN(test_chat_fails_without_init()); RUN(test_init_and_chat_if_key_set()); + RUN(test_prefixed_claude_model_skips_without_http()); #ifdef SHELLCLAW_TEST RUN(test_parse_malformed_json_sets_error()); RUN(test_parse_api_error_object_sets_error()); diff --git a/tests/test_provider.c b/tests/test_provider.c index af4c652..e8f4c70 100644 --- a/tests/test_provider.c +++ b/tests/test_provider.c @@ -5,6 +5,7 @@ #include "providers/provider.h" #include +#include #define ASSERT(c) do { if (!(c)) { fprintf(stderr, "FAIL: %s:%d %s\n", __FILE__, __LINE__, #c); return 1; } } while (0) @@ -30,10 +31,47 @@ static int test_provider_set_error_null_response(void) return 0; } +static int test_model_id_for_backend(void) +{ + const char *example = "anthropic/claude-sonnet-4-20250514"; + const char *bare_claude = "claude-sonnet-4-5"; + const char *openai_prefixed = "openai/gpt-4o"; + const char *bare_gpt = "gpt-4o"; + const char *local_name = "Phi-3-mini"; + const char *empty = ""; + const char *stripped; + + stripped = provider_model_id_for_backend(example, "anthropic"); + ASSERT(stripped == example + strlen("anthropic/")); + ASSERT(strcmp(stripped, "claude-sonnet-4-20250514") == 0); + ASSERT(provider_model_id_for_backend(example, "openai") == NULL); + ASSERT(provider_model_id_for_backend(bare_claude, "openai") == NULL); + ASSERT(provider_model_id_for_backend(bare_claude, "anthropic") == bare_claude); + stripped = provider_model_id_for_backend(openai_prefixed, "openai"); + ASSERT(stripped == openai_prefixed + strlen("openai/")); + ASSERT(strcmp(stripped, "gpt-4o") == 0); + ASSERT(provider_model_id_for_backend(openai_prefixed, "anthropic") == NULL); + ASSERT(provider_model_id_for_backend(bare_gpt, "anthropic") == NULL); + ASSERT(provider_model_id_for_backend(bare_gpt, "openai") == bare_gpt); + ASSERT(provider_model_id_for_backend("chatgpt-4o-latest", "anthropic") == NULL); + ASSERT(provider_model_id_for_backend("o1-preview", "anthropic") == NULL); + ASSERT(provider_model_id_for_backend("o3-mini", "anthropic") == NULL); + ASSERT(provider_model_id_for_backend("o4-mini", "anthropic") == NULL); + ASSERT(provider_model_id_for_backend(local_name, "anthropic") == local_name); + ASSERT(provider_model_id_for_backend(local_name, "openai") == local_name); + ASSERT(provider_model_id_for_backend(empty, "anthropic") == empty); + ASSERT(provider_model_id_for_backend(NULL, "openai") == NULL); + ASSERT(provider_error_allows_fallback_retry( + "OpenAI skipped: agent model is not an OpenAI id") == 1); + ASSERT(provider_error_allows_fallback_retry("OpenAI API HTTP 400") == 0); + return 0; +} + int main(void) { if (test_stub_provider_vtable() != 0) return 1; if (test_provider_set_error_null_response() != 0) return 1; + if (test_model_id_for_backend() != 0) return 1; printf("test_provider: all tests passed\n"); return 0; } diff --git a/tests/test_reload.c b/tests/test_reload.c index 37e7a75..7c07693 100644 --- a/tests/test_reload.c +++ b/tests/test_reload.c @@ -2,12 +2,15 @@ * @file test_reload.c * @brief Unit tests for SIGHUP config reload and stale config queue. */ +#define _POSIX_C_SOURCE 200809L #include "test_runner.h" #include "core/reload.h" #include "core/bootstrap.h" #include "core/config.h" +#include "providers/provider.h" #include +#include #include static config_t *load_minimal_config(const char *path, const char *model) @@ -212,8 +215,49 @@ static int test_try_config_reload_ignores_stale_caller_pointer(void) return 0; } +static int test_try_config_reload_updates_anthropic_model(void) +{ + char path[128]; + config_t *cfg = NULL; + const provider_t *anth; + const config_t *live; + + ASSERT(test_runner_mkstemp_path("shellclaw_test_reload", path, sizeof(path)) == 0); + cfg = load_config_with_tokens(path, "model-before", 111); + ASSERT(cfg != NULL); + bootstrap_set_config_path(path); + bootstrap_set_cfg(cfg); + ASSERT(setenv("ANTHROPIC_API_KEY", "test-key-not-sent", 1) == 0); + anth = provider_anthropic_get(); + ASSERT(anth != NULL && anth->init != NULL && anth->cleanup != NULL); + ASSERT(anth->init(cfg) == 0); + live = anthropic_live_config_for_test(); + ASSERT(live != NULL); + ASSERT(strcmp(config_agent_model(live), "model-before") == 0); + ASSERT(config_agent_max_tokens(live) == 111); + { + FILE *f = fopen(path, "w"); + ASSERT(f); + fprintf(f, "[agent]\nmodel = \"model-after\"\nmax_tokens = 222\n"); + fclose(f); + } + ASSERT(try_config_reload(&cfg) == 0); + live = anthropic_live_config_for_test(); + ASSERT(live != NULL); + ASSERT(strcmp(config_agent_model(live), "model-after") == 0); + ASSERT(config_agent_max_tokens(live) == 222); + anth->cleanup(); + stale_free_all(); + config_free(cfg); + bootstrap_set_cfg(NULL); + unsetenv("ANTHROPIC_API_KEY"); + remove(path); + return 0; +} + int main(void) { + RUN(test_try_config_reload_updates_anthropic_model()); RUN(test_on_hup_sets_reload_flag()); RUN(test_stale_enqueue_null_is_noop()); RUN(test_stale_enqueue_preserves_old_config()); diff --git a/tests/test_sandbox.c b/tests/test_sandbox.c index 45fa2d0..e082059 100644 --- a/tests/test_sandbox.c +++ b/tests/test_sandbox.c @@ -17,13 +17,16 @@ #include "sandbox/sandbox.h" #include "sandbox/sandbox_landlock.h" +#include "sandbox/sandbox_env.h" #include #include +#include #include #include #include #include #include +#include #ifdef __linux__ #include #include @@ -31,7 +34,6 @@ #include #include #include -#include #endif #define ASSERT(c) do { \ @@ -147,6 +149,43 @@ static int test_stderr_captured(void) return 0; } +/** Host API keys must not be readable from sh -c. printenv is not blocklisted. */ +static int test_command_does_not_inherit_host_secrets(void) +{ + char out[4096]; + char ws[] = "/tmp/sc_sb_env_XXXXXX"; + sandbox_config_t cfg; + char *dir; + int rc; + const char *secret = "leaked_via_env"; + + ASSERT(setenv("SHELLCLAW_AUDIT_SECRET", secret, 1) == 0); + ASSERT(setenv("ANTHROPIC_API_KEY", secret, 1) == 0); + dir = mkdtemp(ws); + if (!dir) { + fprintf(stderr, "test_command_does_not_inherit_host_secrets: mkdtemp failed\n"); + return 1; + } + memset(&cfg, 0, sizeof cfg); + cfg.workspace_path = dir; + rc = sandbox_exec( + "printenv SHELLCLAW_AUDIT_SECRET ANTHROPIC_API_KEY; " + "tr '\\0' '\\n' < /proc/1/environ 2>/dev/null; " + "echo HOME=$(printenv HOME); echo PATH_OK", + out, sizeof out, 5000, &cfg); + rmdir(dir); + unsetenv("SHELLCLAW_AUDIT_SECRET"); + unsetenv("ANTHROPIC_API_KEY"); + if (skip_if_isolation_denied(rc, out, "test_command_does_not_inherit_host_secrets")) + return 0; + ASSERT(rc == 0); + ASSERT(strstr(out, secret) == NULL); + /* A real workspace exercises the HOME=workspace branch of the scrub. */ + ASSERT(strstr(out, dir) != NULL); + ASSERT(strstr(out, "PATH_OK") != NULL); + return 0; +} + static int test_null_cmd_returns_error(void) { char out[256]; @@ -277,6 +316,57 @@ static int test_missing_workspace_fail_closed(void) return 0; } +static void trickle_timeout_watchdog(int sig) +{ + (void)sig; + fprintf(stderr, "FAIL: sandbox timeout did not fire while stdout trickled\n"); + _exit(2); +} + +static int test_timeout_kills_trickle_output(void) +{ + char out[8192]; + struct timespec t0; + struct timespec t1; + long elapsed_ms; + int rc; + + ASSERT(clock_gettime(CLOCK_MONOTONIC, &t0) == 0); + signal(SIGALRM, trickle_timeout_watchdog); + alarm(4); + /* The background sleep is a grandchild in the command process group, + * so the deadline kill must wipe it along with the trickling shell. */ + rc = sandbox_exec("sleep 9999 & while true; do echo x; sleep 0.05; done", + out, sizeof(out), 400, NULL); + alarm(0); + signal(SIGALRM, SIG_DFL); + if (skip_if_isolation_denied(rc, out, "test_timeout_kills_trickle_output")) + return 0; + ASSERT(clock_gettime(CLOCK_MONOTONIC, &t1) == 0); + elapsed_ms = (t1.tv_sec - t0.tv_sec) * 1000L + + (t1.tv_nsec - t0.tv_nsec) / 1000000L; + ASSERT(rc == 0); + ASSERT(strstr(out, "timed out") != NULL); + ASSERT(elapsed_ms < 2500); +#ifdef __linux__ + { + int tries; + for (tries = 0; tries < 20; tries++) { + if (!proc_cmdline_has("sleep 9999")) + break; + { + struct timespec ts; + ts.tv_sec = 0; + ts.tv_nsec = 50 * 1000 * 1000; + nanosleep(&ts, NULL); + } + } + ASSERT(proc_cmdline_has("sleep 9999") == 0); + } +#endif + return 0; +} + static int test_timeout_kills_process(void) { char out[4096]; @@ -817,14 +907,152 @@ static int test_command_inherits_cgroup(void) } #endif +/* --- No-namespace units for the wall-clock deadline rewrite (#116) and the + * secret-free child environment (#109). GitHub runners reject unshare, so + * the sandbox_exec paths above skip and these units keep the helpers + * covered on every host. Linked from sandbox_test.o. --- */ + +size_t drain_pipe(int fd, char *buf, size_t cap, int timeout_ms, int *hit_deadline); + +static int test_poll_remain_ms_bounds(void) +{ + ASSERT(poll_remain_ms(1000, 0, 100) == 100); + ASSERT(poll_remain_ms(1000, 950, 100) == 50); + ASSERT(poll_remain_ms(1000, 999, 100) == 1); + ASSERT(poll_remain_ms(1000, 0, 5000) == 1000); + return 0; +} + +static int test_monotonic_elapsed_ms_never_negative(void) +{ + struct timespec start; + struct timespec future; + + ASSERT(clock_gettime(CLOCK_MONOTONIC, &start) == 0); + usleep(30000); + ASSERT(monotonic_elapsed_ms(&start) >= 20); + ASSERT(monotonic_elapsed_ms(&start) < 60000); + ASSERT(clock_gettime(CLOCK_MONOTONIC, &future) == 0); + future.tv_sec += 10; + ASSERT(monotonic_elapsed_ms(&future) == 0); + return 0; +} + +static int test_drain_pipe_reads_until_eof(void) +{ + char buf[64]; + const char msg[] = "hi"; + size_t total; + int hit = -1; + int fds[2]; + pid_t pid; + int st; + + ASSERT(pipe(fds) == 0); + pid = fork(); + if (pid == 0) { + ssize_t w; + close(fds[0]); + w = write(fds[1], msg, sizeof(msg) - 1); + (void)w; + close(fds[1]); + exit(0); + } + close(fds[1]); + total = drain_pipe(fds[0], buf, sizeof(buf), 5000, &hit); + close(fds[0]); + waitpid(pid, &st, 0); + ASSERT(total == 2); + ASSERT(hit == 0); + ASSERT(buf[0] == 'h' && buf[1] == 'i'); + return 0; +} + +static int test_drain_pipe_stops_at_wall_deadline(void) +{ + char buf[256]; + char byte = 'x'; + size_t total; + int hit = -1; + int fds[2]; + pid_t pid; + int st; + + ASSERT(pipe(fds) == 0); + pid = fork(); + if (pid == 0) { + close(fds[0]); + /* Trickle one byte every 20 ms so the pipe stays readable and the + * cap is never reached: only the wall deadline can stop the loop. */ + for (;;) { + if (write(fds[1], &byte, 1) <= 0) + exit(0); + usleep(20000); + } + } + close(fds[1]); + total = drain_pipe(fds[0], buf, sizeof(buf), 400, &hit); + close(fds[0]); + waitpid(pid, &st, 0); + ASSERT(hit == 1); + ASSERT(total >= 1); + ASSERT(total < sizeof(buf) - 1); + return 0; +} + +static int test_scrubbed_env_child_loses_secrets(void) +{ + pid_t pid; + int st; + int status; + + pid = fork(); + if (pid == 0) { + if (setenv("SHELLCLAW_AUDIT_SECRET", "leak", 1) != 0) + exit(4); + if (setenv("PATH", "/usr/bin", 1) != 0) + exit(5); + if (setenv("LD_PRELOAD", "/no/such/sc_fake_lib.so", 1) != 0) + exit(6); + sandbox_scrub_command_environment("/ws/home"); + if (getenv("SHELLCLAW_AUDIT_SECRET")) + exit(1); + if (getenv("LD_PRELOAD")) + exit(7); + if (!getenv("PATH") || strcmp(getenv("PATH"), "/usr/bin") != 0) + exit(2); + if (!getenv("HOME") || strcmp(getenv("HOME"), "/ws/home") != 0) + exit(3); + exit(0); + } + if (waitpid(pid, &st, 0) < 0) { + fprintf(stderr, "FAIL: waitpid scrub child: %s\n", strerror(errno)); + return 1; + } + if (!WIFEXITED(st)) + return 1; + status = WEXITSTATUS(st); + if (status != 0) { + fprintf(stderr, "FAIL: scrub env child exit=%d\n", status); + return 1; + } + return 0; +} + int main(void) { RUN(test_isolation_denied_helper()); RUN(test_fail_closed_reports_isolation_error()); RUN(test_output_capture()); RUN(test_stderr_captured()); + RUN(test_command_does_not_inherit_host_secrets()); RUN(test_null_cmd_returns_error()); RUN(test_zero_cap_returns_error()); + RUN(test_scrubbed_env_child_loses_secrets()); + RUN(test_poll_remain_ms_bounds()); + RUN(test_monotonic_elapsed_ms_never_negative()); + RUN(test_drain_pipe_reads_until_eof()); + RUN(test_drain_pipe_stops_at_wall_deadline()); RUN(test_exit_nonzero_runs()); RUN(test_exit_122_is_not_isolation_failure()); RUN(test_exit_123_with_workspace_is_not_isolation_failure()); @@ -832,6 +1060,7 @@ int main(void) RUN(test_dev_null_is_writable()); RUN(test_workspace_chdir()); RUN(test_missing_workspace_fail_closed()); + RUN(test_timeout_kills_trickle_output()); RUN(test_timeout_kills_process()); #ifdef __linux__ RUN(test_landlock_probe_without_restrict()); diff --git a/tests/test_shell.c b/tests/test_shell.c index a37341b..b612139 100644 --- a/tests/test_shell.c +++ b/tests/test_shell.c @@ -281,6 +281,62 @@ static void test_shell_caps_output_without_hanging(void) "sequential sleep 9999 did not leak"); } +static void trickle_timeout_watchdog(int sig) +{ + (void)sig; + fprintf(stderr, "FAIL: shell timeout did not fire while stdout trickled\n"); + _exit(2); +} + +/* Idle-only poll accounting used to ignore shell_timeout_sec while the + * child kept the pipe readable. The agent lock stayed held until the + * output buffer filled. */ +static void test_shell_trickle_output_hits_timeout(void) +{ + const tool_t *t = tool_shell_get(); + config_t *cfg = NULL; + char errbuf[256]; + char cfg_path[64]; + char buf[8192]; + FILE *f; + struct timespec t0; + struct timespec t1; + long elapsed_ms; + int before; + int r; + + snprintf(cfg_path, sizeof(cfg_path), "/tmp/sc_shell_to_%d.toml", (int)getpid()); + f = fopen(cfg_path, "w"); + MU_ASSERT(f != NULL, "write timeout config"); + fputs("[agent]\nmodel = \"test\"\n[sandbox]\nenabled = false\nshell_timeout_sec = 1\n", f); + fclose(f); + MU_ASSERT(config_load(cfg_path, &cfg, errbuf, sizeof(errbuf)) == 0, "load timeout config"); + tool_shell_set_config(cfg); + before = count_sleep_argv("9999"); + MU_ASSERT(before >= 0, "can count sleep 9999 before trickle timeout"); + memset(buf, 0, sizeof(buf)); + signal(SIGALRM, trickle_timeout_watchdog); + alarm(4); + MU_ASSERT(clock_gettime(CLOCK_MONOTONIC, &t0) == 0, "clock start"); + /* The background sleep sits in the command process group, so the + * timeout path must group-kill it together with the trickling shell. */ + r = t->execute("{\"command\":\"sleep 9999 & while true; do echo x; sleep 0.05; done\"}", + buf, sizeof(buf)); + alarm(0); + signal(SIGALRM, SIG_DFL); + MU_ASSERT(clock_gettime(CLOCK_MONOTONIC, &t1) == 0, "clock end"); + elapsed_ms = (t1.tv_sec - t0.tv_sec) * 1000L + + (t1.tv_nsec - t0.tv_nsec) / 1000000L; + tool_shell_set_config(NULL); + config_free(cfg); + unlink(cfg_path); + MU_ASSERT(r == 0, "trickle command returns"); + MU_ASSERT(strstr(buf, "timed out") != NULL, "trickle command reports timeout"); + MU_ASSERT(elapsed_ms < 3000, "trickle command stopped near the 1s timeout"); + MU_ASSERT(sleep_argv_did_not_grow("9999", before), + "timeout path killed the background sleep 9999"); +} + static void test_shell_caps_output_kills_background_sleep(void) { const tool_t *t = tool_shell_get(); @@ -317,6 +373,7 @@ int main(void) MU_RUN(test_shell_invalid_json); MU_RUN(test_shell_missing_command); MU_RUN(test_shell_caps_output_without_hanging); + MU_RUN(test_shell_trickle_output_hits_timeout); MU_RUN(test_shell_caps_output_kills_background_sleep); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; diff --git a/tests/test_skill.c b/tests/test_skill.c index dd49bf3..f00120e 100644 --- a/tests/test_skill.c +++ b/tests/test_skill.c @@ -249,6 +249,45 @@ static int test_skill_update_write_failure_preserves_existing(void) return 0; } +/* + * Dashboard GET /api/skills/:name loads this buffer into a textarea and PUT + * saves it back. A short buffer must not succeed with a prefix, or Save + * deletes the rest of the skill file. + */ +static int test_skill_get_content_refuses_short_buffer(void) +{ + char cmd[256]; + char small[32]; + char exact[64]; + char full[128]; + const char *body = "HEAD_MARKER_skill_body_TAIL_MARKER_must_survive"; + size_t body_len; + config_t *cfg = NULL; + char errbuf[256]; + + snprintf(cmd, sizeof(cmd), "rm -rf \"%s\" && mkdir -p \"%s\"", TMP_DIR, TMP_DIR); + ASSERT(system(cmd) == 0); + ASSERT(write_minimal_config(TMP_DIR) == 0); + ASSERT(config_load(TMP_CONFIG, &cfg, errbuf, sizeof(errbuf)) == 0); + ASSERT(skill_create(cfg, "longskill", body) == 0); + body_len = strlen(body); + ASSERT(body_len + 1 <= sizeof(exact)); + + memset(small, 'X', sizeof(small)); + ASSERT(skill_get_content(cfg, "longskill", small, sizeof(small)) == -2); + ASSERT(skill_get_content(cfg, "longskill", exact, body_len) == -2); + ASSERT(skill_get_content(cfg, "longskill", exact, body_len + 1) == 0); + ASSERT(strcmp(exact, body) == 0); + ASSERT(skill_get_content(cfg, "longskill", full, sizeof(full)) == 0); + ASSERT(strstr(full, "TAIL_MARKER_must_survive") != NULL); + + config_free(cfg); + remove(TMP_CONFIG); + snprintf(cmd, sizeof(cmd), "rm -rf \"%s\"", TMP_DIR); + (void)system(cmd); + return 0; +} + static int test_hot_reload_watch(void) { char cmd[256]; @@ -287,6 +326,7 @@ int main(void) RUN(test_system_prompt_base_order()); RUN(test_skill_crud()); RUN(test_skill_update_write_failure_preserves_existing()); + RUN(test_skill_get_content_refuses_short_buffer()); RUN(test_hot_reload_watch()); printf("test_skill: all tests passed\n"); return 0; diff --git a/tests/test_telegram.c b/tests/test_telegram.c index 0a4d611..c7a89d5 100644 --- a/tests/test_telegram.c +++ b/tests/test_telegram.c @@ -13,6 +13,9 @@ /* Test-only entry points from telegram.c (compiled with -DSHELLCLAW_TEST) */ extern int telegram_parse_update_for_test(const char *json, channel_incoming_msg_t *out, long *update_id); extern void telegram_set_test_config(const config_t *cfg); +extern int telegram_format_getupdates_url_for_test(char *url, size_t url_cap, const char *token, + int timeout_sec, long offset); +extern int telegram_response_ok_for_test(long http_code, const char *body); static int tests_run = 0; static int tests_failed = 0; @@ -191,6 +194,9 @@ static void test_parse_missing_from_field(void) "{\"ok\":true,\"result\":[{" "\"update_id\":400," "\"message\":{\"text\":\"no from\"}" + "},{" + "\"update_id\":401," + "\"message\":{\"from\":{\"id\":42},\"text\":\"kept\"}" "}]}"; config_t *cfg = load_test_config("/tmp/test_tg_nofrom.toml", "42"); MU_ASSERT(cfg != NULL, "load config"); @@ -200,13 +206,45 @@ static void test_parse_missing_from_field(void) memset(&msg, 0, sizeof(msg)); long uid = 0; int r = telegram_parse_update_for_test(json, &msg, &uid); - MU_ASSERT(r == -1, "missing 'from' returns -1"); + /* Skip, but keep this update_id so the next poll can fetch 401. + * Returning -1 left the offset unchanged and pinned the inbox. */ + MU_ASSERT(r == 0, "missing 'from' is skipped"); + MU_ASSERT(uid == 400, "offset stays on the skipped update"); + MU_ASSERT(msg.text == NULL, "skipped update is not delivered"); channel_incoming_msg_clear(&msg); config_free(cfg); remove("/tmp/test_tg_nofrom.toml"); } +static void test_getupdates_url_requests_one_update(void) +{ + char url[512]; + int rc = telegram_format_getupdates_url_for_test(url, sizeof(url), "tok", 1, 99); + MU_ASSERT(rc == 0, "getUpdates url built"); + MU_ASSERT(strstr(url, "/bottok/getUpdates?") != NULL, "token and method in url"); + MU_ASSERT(strstr(url, "offset=99") != NULL, "offset preserved"); + MU_ASSERT(strstr(url, "limit=1") != NULL, "batch capped at one update"); +} + +static void test_send_response_requires_ok(void) +{ + const char *too_long = "{\"ok\":false,\"error_code\":400," + "\"description\":\"Bad Request: message is too long\"}"; + const char *accepted = "{\"ok\":true,\"result\":{\"message_id\":1}}"; + + MU_ASSERT(telegram_response_ok_for_test(400, too_long) == 0, + "HTTP 400 message-too-long is not delivery"); + MU_ASSERT(telegram_response_ok_for_test(200, "{\"ok\":false}") == 0, + "HTTP 200 with ok false is not delivery"); + MU_ASSERT(telegram_response_ok_for_test(200, "") == 0, + "empty body is not delivery"); + MU_ASSERT(telegram_response_ok_for_test(429, "{\"ok\":false}") == 0, + "HTTP 429 is not delivery"); + MU_ASSERT(telegram_response_ok_for_test(200, accepted) == 1, + "HTTP 200 with ok true is delivery"); +} + static void test_vtable_fields_set(void) { const channel_t *ch = channel_telegram_get(); @@ -227,6 +265,8 @@ int main(void) MU_RUN(test_parse_photo_with_caption); MU_RUN(test_parse_no_message_key); MU_RUN(test_parse_missing_from_field); +MU_RUN(test_getupdates_url_requests_one_update); + MU_RUN(test_send_response_requires_ok); MU_RUN(test_vtable_fields_set); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; diff --git a/tests/test_ws.c b/tests/test_ws.c index af53973..a0586aa 100644 --- a/tests/test_ws.c +++ b/tests/test_ws.c @@ -268,6 +268,68 @@ static int test_pop_incoming_invalid_args(void) return 0; } +/* Counters live in the SHELLCLAW_WS_TEST build of ws.c. */ +extern int ws_test_writable_count(void); +extern int ws_test_cancel_count(void); +extern void ws_test_reset_wake_counts(void); + +/** + * Agent replies run on the main thread while lws_service runs on the gateway + * thread. The wake must be lws_cancel_service, and only the service-thread + * hook may call lws_callback_on_writable. + */ +static int test_send_wakes_service_thread(void) +{ + char buf[32]; + size_t len; + + ws_cleanup(); + ws_test_reset_wake_counts(); + ws_set_context((void *)(intptr_t)1); + ASSERT(ws_register_conn(11, (ws_conn_t)(intptr_t)11) == 0); + ASSERT(ws_send_to("webchat:11", "hello") == 0); + ASSERT(ws_test_cancel_count() == 1); + ASSERT(ws_test_writable_count() == 0); + ws_on_service_cancel(); + ASSERT(ws_test_writable_count() == 1); + ASSERT(ws_dequeue_outgoing(11, buf, sizeof(buf), &len) == 1); + ASSERT(strcmp(buf, "hello") == 0); + ws_on_service_cancel(); + ASSERT(ws_test_writable_count() == 1); + ws_set_context(NULL); + ws_cleanup(); + return 0; +} + +static int test_broadcast_wakes_once(void) +{ + ws_cleanup(); + ws_test_reset_wake_counts(); + ws_set_context((void *)(intptr_t)1); + ASSERT(ws_register_conn(12, (ws_conn_t)(intptr_t)12) == 0); + ASSERT(ws_register_conn(13, (ws_conn_t)(intptr_t)13) == 0); + ws_broadcast_text("status"); + ASSERT(ws_test_cancel_count() == 1); + ASSERT(ws_test_writable_count() == 0); + ws_on_service_cancel(); + ASSERT(ws_test_writable_count() == 2); + ws_set_context(NULL); + ws_cleanup(); + return 0; +} + +static int test_send_without_context_does_not_touch_lws(void) +{ + ws_cleanup(); + ws_test_reset_wake_counts(); + ASSERT(ws_register_conn(14, (ws_conn_t)(intptr_t)14) == 0); + ASSERT(ws_send_to("webchat:14", "queued") == 0); + ASSERT(ws_test_cancel_count() == 0); + ASSERT(ws_test_writable_count() == 0); + ws_cleanup(); + return 0; +} + static int test_shutdown_stops_pop(void) { char session[32]; @@ -280,6 +342,58 @@ static int test_shutdown_stops_pop(void) return 0; } +/** + * A frame queued before CLOSE must leave with the socket. Otherwise it + * sits on the shared outgoing list forever and, with enough disconnects, + * hits MAX_QUEUE so live clients can no longer send. + */ +static int test_unregister_drops_pending_outgoing(void) +{ + char buf[64]; + size_t len; + int i; + + ws_cleanup(); + ASSERT(ws_register_conn(6, (ws_conn_t)(intptr_t)6) == 0); + ASSERT(ws_register_conn(7, (ws_conn_t)(intptr_t)7) == 0); + ASSERT(ws_send_to("webchat:7", "first-7") == 0); + ASSERT(ws_send_to("webchat:6", "for-6") == 0); + ASSERT(ws_send_to("webchat:7", "for-7") == 0); + ws_unregister_conn(6); + ASSERT(ws_has_pending_outgoing(6) == 0); + ASSERT(ws_has_pending_outgoing(7) == 1); + for (i = 0; i < 128; i++) + ASSERT(ws_send_to("webchat:6", "orphan") == 0); + ASSERT(ws_send_to("webchat:7", "still-live") == 0); + ASSERT(ws_dequeue_outgoing(7, buf, sizeof(buf), &len) == 1); + ASSERT(strcmp(buf, "first-7") == 0); + ASSERT(ws_dequeue_outgoing(7, buf, sizeof(buf), &len) == 1); + ASSERT(strcmp(buf, "for-7") == 0); + ASSERT(ws_dequeue_outgoing(7, buf, sizeof(buf), &len) == 1); + ASSERT(strcmp(buf, "still-live") == 0); + ws_cleanup(); + return 0; +} + +/** Replies to a session that was never connected must not fill MAX_QUEUE. */ +static int test_send_to_absent_conn_does_not_fill_queue(void) +{ + char buf[64]; + size_t len; + int i; + + ws_cleanup(); + for (i = 0; i < 128; i++) + ASSERT(ws_send_to("webchat:99", "ghost") == 0); + ASSERT(ws_register_conn(8, (ws_conn_t)(intptr_t)8) == 0); + ASSERT(ws_send_to("webchat:8", "ok") == 0); + ASSERT(ws_dequeue_outgoing(8, buf, sizeof(buf), &len) == 1); + ASSERT(strcmp(buf, "ok") == 0); + ASSERT(ws_has_pending_outgoing(99) == 0); + ws_cleanup(); + return 0; +} + int main(void) { RUN(test_register_conn_full_table()); @@ -297,6 +411,11 @@ int main(void) RUN(test_push_incoming_rejects_empty()); RUN(test_pop_incoming_invalid_args()); RUN(test_shutdown_stops_pop()); +RUN(test_unregister_drops_pending_outgoing()); + RUN(test_send_to_absent_conn_does_not_fill_queue()); + RUN(test_send_wakes_service_thread()); + RUN(test_broadcast_wakes_once()); + RUN(test_send_without_context_does_not_touch_lws()); printf("test_ws: all tests passed\n"); return 0; }