From dae51e06c73510207ba7a2f83d6f74d37c4c1d80 Mon Sep 17 00:00:00 2001 From: WHC Date: Sun, 11 Oct 2026 20:03:39 +0800 Subject: [PATCH] fix the session-list scan's line count, and pin both new paths with tests The rewritten cst_agent_store_info() counted a transcript line that ran past a whole 64 KiB read block a second time, when the newline ending its tail arrived: a session with one very long line (a tool that read a file back) reported one message too many -- exactly what the sidebar shows the person. - src/agent_store.c: the tail's newline ends a line that was already counted. - tests/test_store.c: test_info_scan() holds the scan down -- a 70 KiB line is one line, the first user message comes from the head of the file, and a user line past the window is not called the first one. - tests/test_tools_ext.c: a byte budget stops the session_search walk, says so in the result, and never reports a match it did not reach; the same query without a budget finds it. make check and make asan are green. --- src/agent_store.c | 5 ++- tests/test_store.c | 93 ++++++++++++++++++++++++++++++++++++++++++ tests/test_tools_ext.c | 44 ++++++++++++++++++++ 3 files changed, 141 insertions(+), 1 deletion(-) diff --git a/src/agent_store.c b/src/agent_store.c index 6c42f7b..96db62f 100644 --- a/src/agent_store.c +++ b/src/agent_store.c @@ -469,7 +469,10 @@ int cst_agent_store_info(const char *workspace, const char *id, if (!nl) break; const size_t len = (size_t)(nl - (buf + at)); - if (len > 0) + /* A line counts once. A line that ran past a whole block was counted + * when its tail was dropped, so the newline that ends it is not a line + * of its own. */ + if (len > 0 && !slice) info->messages++; if (!slice && len > 0 && want_first && seen < kFirstUserBytes) { *nl = 0; /* the parse reads a C string; the newline is ours */ diff --git a/tests/test_store.c b/tests/test_store.c index 1e07580..845db0d 100644 --- a/tests/test_store.c +++ b/tests/test_store.c @@ -267,6 +267,98 @@ static void test_old_store(void) { EXPECT(!exists(nested) && !exists(new_payload), "takes the payload with it"); } +/* The scan behind the session list (cst_agent_store_info): the count is a + * count of transcript lines, a line longer than one read block is still one + * line, and the first user message is only looked for in the head of the file + * -- a sidebar refresh is not allowed to read a whole transcript of tool + * output to find a greeting. */ +static void test_info_scan(void) { + char id[64] = ""; + EXPECT(cst_agent_store_new_id(id, sizeof(id)) == 0, "an id for the scan"); + char path[1200]; + EXPECT(cst_agent_store_path_for_id(g_ws, id, path, sizeof(path)) == 0, + "a session path to write into"); + cst_session_t *s = cst_session_open(path); + EXPECT(s != NULL, "its transcript opens"); + if (!s) + return; + cst_message_t m; + memset(&m, 0, sizeof(m)); + m.role = CST_ROLE_USER; + m.content = (char *)"the first user line"; + EXPECT(cst_session_append(s, &m) == 0, "the first user message goes in"); + memset(&m, 0, sizeof(m)); + m.role = CST_ROLE_ASSISTANT; + m.content = (char *)"an answer"; + EXPECT(cst_session_append(s, &m) == 0, "and an answer"); + /* One line that does not fit a read block: counted once, not once per block + * of it (which is what a wrong block count would report). */ + const size_t big = 70 * 1024; + char *huge = malloc(big + 1); + EXPECT(huge != NULL, "a buffer for one very long line"); + if (!huge) + return; + memset(huge, 'x', big); + huge[big] = 0; + memset(&m, 0, sizeof(m)); + m.role = CST_ROLE_ASSISTANT; + m.content = huge; + EXPECT(cst_session_append(s, &m) == 0, "a line longer than a block goes in"); + memset(&m, 0, sizeof(m)); + m.role = CST_ROLE_ASSISTANT; + m.content = (char *)"a short one after it"; + EXPECT(cst_session_append(s, &m) == 0, "and a short line after it"); + cst_session_close(s); + + cst_session_info_t in; + memset(&in, 0, sizeof(in)); + EXPECT(cst_agent_store_info(g_ws, id, &in) == 0, "its info reads back"); + EXPECT(in.messages == 4, "four lines counted, the long one once"); + EXPECT(strcmp(in.first_user, "the first user line") == 0, + "and the first user line is the one it found"); + + /* Now a head of tool output wider than the window the search gets, with no + * user line in it: the greeting below stays unseen (and nothing read + * megabytes of transcript to look for it). */ + char id2[64] = ""; + EXPECT(cst_agent_store_new_id(id2, sizeof(id2)) == 0, "an id for the tail"); + char path2[1200]; + EXPECT(cst_agent_store_path_for_id(g_ws, id2, path2, sizeof(path2)) == 0, + "a second session path"); + s = cst_session_open(path2); + EXPECT(s != NULL, "the second transcript opens"); + if (!s) { + free(huge); + return; + } + const size_t mid = 60 * 1024; + char *chunk = malloc(mid + 1); + if (chunk) { + memset(chunk, 'y', mid); + chunk[mid] = 0; + for (int i = 0; i < 6; i++) { /* 360 KiB of tool output, no user line */ + memset(&m, 0, sizeof(m)); + m.role = CST_ROLE_TOOL; + m.content = chunk; + if (cst_session_append(s, &m) != 0) + break; + } + free(chunk); + } + memset(&m, 0, sizeof(m)); + m.role = CST_ROLE_USER; + m.content = (char *)"a greeting far down the file"; + EXPECT(cst_session_append(s, &m) == 0, "a user line after all that"); + cst_session_close(s); + + memset(&in, 0, sizeof(in)); + EXPECT(cst_agent_store_info(g_ws, id2, &in) == 0, "its info reads too"); + EXPECT(in.messages == 7, "seven lines, one per line"); + EXPECT(in.first_user[0] == 0, + "a user line past the window is not called the first one"); + free(huge); +} + int main(void) { if (test_tmpdir(g_ws, sizeof(g_ws), "cst-store") != 0) { perror("tmpdir"); @@ -280,6 +372,7 @@ int main(void) { test_ids(); test_new_session(); test_old_store(); + test_info_scan(); test_rmrf(g_ws); if (nfail) { diff --git a/tests/test_tools_ext.c b/tests/test_tools_ext.c index 5b93b9b..cebaeea 100644 --- a/tests/test_tools_ext.c +++ b/tests/test_tools_ext.c @@ -236,6 +236,50 @@ static void test_session_search(cst_tool_ctx_t *ctx) { c = mk("session_search", NULL); EXPECT(run(ctx, c, &r) == 1, "a query is required"); cst_tool_result_free(&r); + + /* A store is not read to the end on a whim: a byte budget stops the walk + * and says so, and it never reports a match it did not get to. */ + { + char path2[512]; + EXPECT(cst_agent_store_path_for_id(g_ws, "20260101-130000-big", path2, + sizeof(path2)) == 0, + "a second session path"); + cst_session_t *s2 = cst_session_open(path2); + EXPECT(s2 != NULL, "its transcript opens"); + if (s2) { + cst_message_t m2; + memset(&m2, 0, sizeof(m2)); + m2.role = CST_ROLE_USER; + m2.content = (char *)"the start of a long one"; + EXPECT(cst_session_append(s2, &m2) == 0, "a first line goes in"); + for (int i = 0; i < 600; i++) { + memset(&m2, 0, sizeof(m2)); + m2.role = CST_ROLE_ASSISTANT; + m2.content = (char *)"a line of tool output that says nothing special"; + if (cst_session_append(s2, &m2) != 0) + break; + } + memset(&m2, 0, sizeof(m2)); + m2.role = CST_ROLE_ASSISTANT; + m2.content = (char *)"the needle is only at the very end"; + EXPECT(cst_session_append(s2, &m2) == 0, "the needle goes in last"); + cst_session_close(s2); + } + /* Enough lines that a byte budget is spent before the end is reached. */ + c = mk("session_search", "query", "needle", "max_bytes", "4096", NULL); + EXPECT(run(ctx, c, &r) == 0 && r.out != NULL, "a bounded search runs"); + EXPECT(r.out && strstr(r.out, "budget stopped the walk") != NULL, + "the byte budget stops the walk and says so"); + EXPECT(r.out == NULL || strstr(r.out, "the needle is only") == NULL, + "and it does not report a match it never got to"); + cst_tool_result_free(&r); + /* The same query without a budget reaches the end and finds it. */ + c = mk("session_search", "query", "needle", NULL); + EXPECT(run(ctx, c, &r) == 0 && r.out && + strstr(r.out, "the needle is only") != NULL, + "the unbounded search finds it"); + cst_tool_result_free(&r); + } } /* --- device_info: the environment, without a shell ---------------------- */