Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha

## [Unreleased]

### Fixed
- Cron jobs whose channel cannot deliver (unknown name, a channel left disabled, or `cron` itself) are rejected at create time (`-2`). A row already stored with that channel is disabled on the next poll before `agent_run`. Previously `cron_send` failed after the full turn, ack was skipped, and the still-due job reran the model and its tools on every poll.

## [1.0.1] - 2026-10-18

### Fixed
Expand Down
16 changes: 16 additions & 0 deletions src/core/memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,22 @@ int cron_job_toggle(const char *id)
return ret;
}

int cron_job_set_enabled(const char *id, int enabled)
{
const char *sql = "UPDATE cron_jobs SET enabled = ?1 WHERE id = ?2 AND enabled != ?1";
sqlite3_stmt *stmt = NULL;
int ret;

if (!g_db || !id) return -1;
memory_sqlite_entry_probe();
if (sqlite3_prepare_v2(g_db, sql, -1, &stmt, NULL) != SQLITE_OK) return -1;
sqlite3_bind_int(stmt, 1, enabled ? 1 : 0);
sqlite3_bind_text(stmt, 2, id, -1, SQLITE_TRANSIENT);
ret = (sqlite3_step(stmt) == SQLITE_DONE && sqlite3_changes(g_db) > 0) ? 0 : -1;
sqlite3_finalize(stmt);
return ret;
}

int cron_job_update_next_run(const char *id, long long next_run)
{
if (!g_db || !id) return -1;
Expand Down
12 changes: 12 additions & 0 deletions src/core/memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,18 @@ int cron_job_delete(const char *id);
*/
int cron_job_toggle(const char *id);

/**
* Set the enabled flag to 0 or 1. Unlike cron_job_toggle(), this does not flip
* a row that already has the requested value.
*
* @param id Job id.
* @param enabled 0 to disable, nonzero to enable.
* @return 0 when a row changes, non-zero if the job is missing or already set.
*
* Example: `cron_job_set_enabled("cron_1", 0);`
*/
int cron_job_set_enabled(const char *id, int enabled);

/**
* Update next_run for a job.
*
Expand Down
3 changes: 2 additions & 1 deletion src/gateway/routes.c
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,8 @@ static void handle_cron_create(const char *body, size_t body_len, char *buf, siz
agent_unlock();
cJSON_Delete(root);
if (ret != 0) {
json_error(buf, size, status, 400, "Failed to create job");
json_error(buf, size, status, 400,
ret == -2 ? "channel cannot deliver" : "Failed to create job");
return;
}
cJSON *resp_obj = cJSON_CreateObject();
Expand Down
60 changes: 56 additions & 4 deletions src/tools/cron.c
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,39 @@ int cron_is_one_shot(const char *schedule)
return strncmp(schedule, CRON_PREFIX_AT, strlen(CRON_PREFIX_AT)) == 0;
}

static const char *cron_delivery_channel(const char *channel)
{
if (!channel || channel[0] == '\0')
return "cli";
return channel;
}

/** 1 when a poll can hand this job to a channel that has a send callback. */
static int cron_target_can_deliver(const char *channel)
{
const char *name = cron_delivery_channel(channel);
const channel_t *dest;

if (strcmp(name, "cron") == 0)
return 0;
dest = channel_get_by_name(name);
return dest && dest->send ? 1 : 0;
}

int cron_create_job(const char *schedule, const char *message,
const char *channel, const char *recipient,
char *id_out, size_t id_size)
{
const char *ch;
const char *rec;

if (!schedule || !message || !id_out || id_size == 0) return -1;
const char *ch = channel ? channel : "cli";
const char *rec = recipient ? recipient : "default";
ch = channel ? channel : "cli";
rec = recipient ? recipient : "default";
/* An unregistered target fails in cron_send after agent_run, and the
* skipped ack leaves next_run in the past so the job reruns forever. */
if (!cron_target_can_deliver(ch))
return -2;
if (id_out[0] == '\0') {
unsigned char rnd[2];
if (crypto_read_urandom(rnd, sizeof(rnd)) != 0) return -1;
Expand Down Expand Up @@ -382,6 +408,23 @@ static int cron_pick_due_row(long long now, int timeout_ms, cron_job_row_t *row)
return 1;
}

/** Drop a stored job that can never send, before the agent runs it. */
static void cron_disable_undeliverable(const cron_job_row_t *row, int timeout_ms)
{
int rc;

fprintf(stderr,
"shellclaw: cron: job '%s' channel '%s' cannot deliver; disabling it so a failed send cannot rerun the agent\n",
row->id, cron_delivery_channel(row->channel));
agent_lock();
rc = cron_job_set_enabled(row->id, 0);
agent_unlock();
/* A missed update leaves the row due. Remember the offer so the next poll
* waits instead of spinning the scheduler on the same id. */
if (rc != 0)
cron_mark_offered(row->id, timeout_ms > 0 ? timeout_ms : 1000);
}

static int cron_poll(channel_incoming_msg_t *out, int timeout_ms)
{
cron_job_row_t row;
Expand All @@ -392,6 +435,11 @@ static int cron_poll(channel_incoming_msg_t *out, int timeout_ms)
memset(&row, 0, sizeof(row));
if (!cron_pick_due_row(now, timeout_ms, &row))
return 0;
if (!cron_target_can_deliver(row.channel)) {
cron_disable_undeliverable(&row, timeout_ms);
cron_job_row_free(&row);
return 0;
}
memset(out, 0, sizeof(*out));
snprintf(session_id, sizeof(session_id), "%s:%s",
row.channel[0] ? row.channel : "cli",
Expand Down Expand Up @@ -553,9 +601,13 @@ static int cron_tool_execute(const char *args_json, char *result_buf, size_t max
id[0] = '\0';
const char *ch = (channel && cJSON_IsString(channel)) ? channel->valuestring : NULL;
const char *rec = (recipient && cJSON_IsString(recipient)) ? recipient->valuestring : NULL;
if (cron_create_job(schedule->valuestring, message->valuestring, ch, rec, id, sizeof(id)) != 0) {
int created = cron_create_job(schedule->valuestring, message->valuestring, ch, rec, id,
sizeof(id));
if (created != 0) {
cJSON_Delete(root);
snprintf(result_buf, max_len, "{\"error\":\"failed to create job\"}");
snprintf(result_buf, max_len, "%s",
created == -2 ? "{\"error\":\"channel cannot deliver\"}"
: "{\"error\":\"failed to create job\"}");
return -1;
}
snprintf(result_buf, max_len, "{\"ok\":true,\"id\":\"%s\"}", id);
Expand Down
3 changes: 2 additions & 1 deletion src/tools/cron.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,11 @@ const tool_t *tool_cron_get(void);
* @param schedule Schedule string (interval:N, at:ts, or cron expression).
* @param message Message to inject when job fires.
* @param channel Target channel name (e.g., "cli"). NULL defaults to "cli".
* The name must be a registered channel other than "cron".
* @param recipient Recipient ID. NULL defaults to "default".
* @param id_out Buffer for job ID. If non-empty on entry, used as-is; else generated.
* @param id_size Size of id_out buffer.
* @return 0 on success, -1 on error.
* @return 0 on success, -2 if channel cannot deliver, -1 on any other error.
*/
int cron_create_job(const char *schedule, const char *message,
const char *channel, const char *recipient,
Expand Down
66 changes: 66 additions & 0 deletions tests/test_cron.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,21 @@ void provider_router_active_backend_snapshot(char *dst, size_t dst_sz)
dst[0] = '\0';
}

static int cron_test_deliver(const char *recipient, const char *text,
const channel_attachment_t *attachments, size_t att_count)
{
(void)recipient;
(void)text;
(void)attachments;
(void)att_count;
return 0;
}

static const channel_t cron_test_cli_channel = {
.name = "cli",
.send = cron_test_deliver,
};

static void cron_test_reset_offers(void)
{
const channel_t *cron_ch = channel_cron_get();
Expand Down Expand Up @@ -754,8 +769,58 @@ static int test_cron_poll_delivers_17th_after_ack(void)
return 0;
}

/**
* A due job whose channel was never registered must not be handed to the agent.
* Send would fail, ack would be skipped, and the still-due row would rerun tools
* on every poll. Direct cron_job_create bypasses the create-time check, which is
* how an already stored row reaches this path.
*/
static int test_unregistered_channel_is_not_delivered(void)
{
const char *path = "/tmp/shellclaw_test_cron_bad_channel.db";
const channel_t *cron_ch;
channel_incoming_msg_t msg;
cron_job_row_t row;
long long now;
char id[128];

cron_test_reset_offers();
remove(path);
ASSERT(memory_init(path) == 0);
now = (long long)time(NULL);
ASSERT(cron_job_create("ghost", "interval:60", "run the backup", "no-such-channel",
"default", now - 1, 1) == 0);
cron_ch = channel_cron_get();
ASSERT(cron_ch != NULL && cron_ch->poll != NULL);
memset(&msg, 0, sizeof(msg));
ASSERT(cron_ch->poll(&msg, 0) == 0);
ASSERT(msg.text == NULL);
memset(&row, 0, sizeof(row));
ASSERT(cron_job_get_by_id("ghost", &row) == 1);
ASSERT(row.enabled == 0);
cron_job_row_free(&row);
memset(&msg, 0, sizeof(msg));
ASSERT(cron_ch->poll(&msg, 0) == 0);
id[0] = '\0';
ASSERT(cron_create_job("interval:60", "hi", "no-such-channel", "user", id, sizeof(id)) != 0);
ASSERT(cron_job_create("cli_ok", "interval:60", "still delivered", "cli", "default",
now - 1, 1) == 0);
memset(&msg, 0, sizeof(msg));
ASSERT(cron_ch->poll(&msg, 0) == 1);
ASSERT(msg.text != NULL);
ASSERT(strcmp(msg.text, "still delivered") == 0);
ASSERT(msg.user_id != NULL);
ASSERT(strcmp(msg.user_id, "cli_ok") == 0);
channel_incoming_msg_clear(&msg);
cron_test_reset_offers();
memory_cleanup();
remove(path);
return 0;
}

int main(void)
{
channel_register("cli", &cron_test_cli_channel);
RUN(test_interval_next_run());
RUN(test_at_one_shot());
RUN(test_cron_expr_next_run());
Expand All @@ -781,6 +846,7 @@ int main(void)
RUN(test_cron_poll_waits_when_offer_table_is_full());
RUN(test_cron_poll_delivers_17th_after_ack());
RUN(test_cron_list_orders_ties_by_id());
RUN(test_unregistered_channel_is_not_delivered());
printf("test_cron: all tests passed\n");
return 0;
}
Loading