diff --git a/CHANGELOG.md b/CHANGELOG.md index 0252119..443c2f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha ## [Unreleased] +### Fixed +- Session compaction that folds a summary into an assistant line now retags that line as `user` and merges a following user turn, so an odd `max_context_messages` cannot send an assistant-first transcript. Anthropic rejects that shape with HTTP 400, the router stops, and the unsaved turn is rebuilt on every later message. + ## [1.0.1] - 2026-10-18 ### Fixed diff --git a/src/core/agent.c b/src/core/agent.c index a3df259..77e7294 100644 --- a/src/core/agent.c +++ b/src/core/agent.c @@ -228,6 +228,109 @@ static int append_summary_user(cJSON *arr, const char *summary) return 0; } +static int message_role_is(const cJSON *item, const char *role) +{ + const cJSON *r; + + r = item ? cJSON_GetObjectItem(item, "role") : NULL; + return cJSON_IsString(r) && r->valuestring && role && + strcmp(r->valuestring, role) == 0; +} + +static const char *message_text(const cJSON *item) +{ + const cJSON *c; + + c = item ? cJSON_GetObjectItem(item, "content") : NULL; + if (c && cJSON_IsString(c) && c->valuestring) + return c->valuestring; + return ""; +} + +static int set_message_role(cJSON *item, const char *role) +{ + cJSON *node; + + node = cJSON_CreateString(role ? role : ""); + if (!node) + return -1; + if (cJSON_GetObjectItem(item, "role")) { + if (!cJSON_ReplaceItemInObject(item, "role", node)) { + cJSON_Delete(node); + return -1; + } + return 0; + } + if (!cJSON_AddItemToObject(item, "role", node)) { + cJSON_Delete(node); + return -1; + } + return 0; +} + +/** Append @p extra after the message body. The summary fold prepends; this + * joins the next user turn so two user roles are not sent in a row. */ +static int append_message_text(cJSON *item, const char *extra) +{ + const char *old; + char *merged; + cJSON *node; + size_t old_len; + size_t extra_len; + size_t n; + + old = message_text(item); + extra = extra ? extra : ""; + old_len = strlen(old); + extra_len = strlen(extra); + if (extra_len > SIZE_MAX - old_len - 3) + return -1; + n = old_len + 2 + extra_len + 1; + merged = malloc(n); + if (!merged) + return -1; + snprintf(merged, n, "%s\n\n%s", old, extra); + 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; +} + +/** Anthropic rejects a transcript whose first message is not user, and rejects + * two user turns in a row. HTTP 4xx stops the router, the turn is not saved, + * and the next turn rebuilds the same chat. A fold into a non-user head used + * to leave that role in place. Retag it and merge one following user turn. */ +static int normalize_folded_non_user_head(cJSON *arr) +{ + cJSON *head; + cJSON *next; + + head = cJSON_GetArrayItem(arr, 0); + if (!head || set_message_role(head, "user") != 0) + return -1; + cJSON_DeleteItemFromObject(head, "tool_use_id"); + next = cJSON_GetArrayItem(arr, 1); + if (!next || !message_role_is(next, "user") || + cJSON_GetObjectItem(next, "tool_use_id")) + return 0; + if (append_message_text(head, message_text(next)) != 0) + return -1; + cJSON_DeleteItemFromArray(arr, 1); + 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, @@ -318,10 +421,10 @@ static int compact_session_via_llm(const char *session_id, char *session_buf, si 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. fold_into_oversized breaks on - * that rule for the line that stopped the pass: it is not a user turn, so - * the summary folds into it instead of a prepended wrapper (see the loop). */ + * lands on user). A second user role makes Anthropic reject the session + * on this turn and every later one. A non-user line that stops the pass + * folds the summary into itself; normalize_folded_non_user_head retags + * that head to user so an odd max_ctx does not send assistant first. */ merge = kept_tail_starts_with_user(root, start); if (!new_arr) { cJSON_Delete(root); return -1; } if (!merge && !fold_into_oversized && append_summary_user(new_arr, summary_buf) != 0) { @@ -351,6 +454,12 @@ static int compact_session_via_llm(const char *session_id, char *session_buf, si cJSON_Delete(root); return -1; } + if (fold_into_oversized && !merge && + normalize_folded_non_user_head(new_arr) != 0) { + cJSON_Delete(new_arr); + cJSON_Delete(root); + return -1; + } cJSON_Delete(root); char *printed = cJSON_PrintUnformatted(new_arr); cJSON_Delete(new_arr); diff --git a/tests/test_agent.c b/tests/test_agent.c index 8be45d8..0db9d07 100644 --- a/tests/test_agent.c +++ b/tests/test_agent.c @@ -1073,7 +1073,9 @@ static int test_compaction_gives_oversized_line_full_buffer_on_next_pass(void) fprintf(stderr, "FAIL: pass 1 destroyed the oversized message instead of keeping it whole\n"); goto cleanup; } - if (count_session_messages(loaded) != 5 + 2) { + /* Fold drops the consumed head. The following user turn is merged into + * the retagged head, so the count shrinks by two before the new exchange. */ + if (count_session_messages(loaded) >= 6 + 2) { fprintf(stderr, "FAIL: partial pass did not fold its summary into the oversized line\n"); goto cleanup; } @@ -2431,6 +2433,120 @@ static int test_near_full_system_prompt_keeps_one_recall_byte(void) return failed; } +/* A user line that fills most of the 64 KiB summary source, then an assistant + * line that does not fit the remainder, folds at index 1. With an odd + * max_context_messages the stored count is max+1, so the folded head is inside + * the sent window (skip == 0). Leaving that head as assistant makes Anthropic + * return 400. The router does not fall through, the turn is not saved, and + * every later turn rebuilds the same transcript. The head must be retagged + * user and the following user turn merged so the roles still alternate. */ +static int test_compaction_fold_keeps_alternating_user_head(void) +{ + enum { USER_LEN = 60 * 1024, ASST_LEN = 8 * 1024, JSON_CAP = 120 * 1024, + LOAD_CAP = 128 * 1024 }; + int failed = 1; + config_t *cfg = NULL; + char *session_json = NULL; + char *loaded = NULL; + char *user_body = NULL; + char *asst_body = NULL; + const char *db_path = "build/test_agent_compact_fold_roles.db"; + const char *config_path = "build/test_agent_compact_fold_roles.toml"; + size_t off; + size_t 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); + user_body = malloc((size_t)USER_LEN + 1); + asst_body = malloc((size_t)ASST_LEN + 1); + loaded = malloc(LOAD_CAP); + if (!session_json || !user_body || !asst_body || !loaded) + goto cleanup; + memset(user_body, 'U', (size_t)USER_LEN); + user_body[USER_LEN] = '\0'; + memset(asst_body, 'A', (size_t)ASST_LEN); + memcpy(asst_body + ASST_LEN - 8, "ASST_END", 8); + asst_body[ASST_LEN] = '\0'; + off = 0; + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + "[{\"role\":\"user\",\"content\":\"%s\"}", user_body); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"assistant\",\"content\":\"%s\"}", asst_body); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"user\",\"content\":\"m2\"}"); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"assistant\",\"content\":\"m3\"}"); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"user\",\"content\":\"m4\"}"); + off += (size_t)snprintf(session_json + off, JSON_CAP - off, + ",{\"role\":\"assistant\",\"content\":\"m5\"}]"); + if (off >= JSON_CAP) + goto cleanup; + if (session_save("cli:foldroles", session_json) != 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; + response_buf[0] = '\0'; + spy_roles_clear(); + if (agent_run(cfg, "cli:foldroles", "hi", &compaction_provider, NULL, 0, + response_buf, sizeof(response_buf)) != 0) + goto cleanup; + if (spy_message_count < 2 || 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) { + fprintf(stderr, "FAIL: folded head role is %s, expected user\n", + spy_roles[1] ? spy_roles[1] : "(null)"); + goto cleanup; + } + if (strstr(spy_content[1], "Summary of earlier") == NULL) { + fprintf(stderr, "FAIL: summary was not folded into the leading user turn\n"); + goto cleanup; + } + for (i = 2; i < spy_message_count && i < SPY_SLOTS; i++) { + if (!spy_roles[i] || strcmp(spy_roles[i], spy_roles[i - 1]) == 0) { + fprintf(stderr, "FAIL: folded chat has consecutive %s roles\n", + spy_roles[i] ? spy_roles[i] : "(null)"); + goto cleanup; + } + } + if (session_load("cli:foldroles", loaded, LOAD_CAP) != 0) + goto cleanup; + if (strstr(loaded, "ASST_END") == NULL) { + fprintf(stderr, "FAIL: folded assistant body was dropped\n"); + goto cleanup; + } + if (!json_roles_alternate_from_user(loaded)) { + fprintf(stderr, "FAIL: persisted folded session does not alternate from user\n"); + goto cleanup; + } + failed = 0; +cleanup: + config_free(cfg); + free(session_json); + free(loaded); + free(user_body); + free(asst_body); +cleanup_early: + remove(config_path); + remove(db_path); + memory_cleanup(); + return failed; +} + int main(void) { RUN(test_agent_run_with_stub_and_no_tools()); @@ -2451,6 +2567,7 @@ int main(void) RUN(test_compaction_progresses_when_assistant_message_is_oversized()); RUN(test_compaction_failure_keeps_original_session()); RUN(test_compaction_merges_summary_when_tail_starts_with_user()); + RUN(test_compaction_fold_keeps_alternating_user_head()); 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());