diff --git a/README.md b/README.md index e16d17310..92a5adef9 100644 --- a/README.md +++ b/README.md @@ -584,7 +584,7 @@ JSON arguments can also be piped on stdin. Inline JSON remains accepted for back | `get_code_snippet` | Read source code for a function by qualified name. | | `get_architecture` | Codebase overview: languages, packages, routes, hotspots, clusters, ADR. | | `search_code` | Grep-like text search within indexed project files. | -| `manage_adr` | CRUD for Architecture Decision Records. | +| `manage_adr` | CRUD for Architecture Decision Records (`get` / `update` replaces / `append` adds / `sections`). | | `ingest_traces` | Ingest runtime traces to validate HTTP_CALLS edges. | ## Graph Data Model diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index fb5a750ec..b52421b90 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -640,9 +640,13 @@ static const tool_def_t TOOLS[] = { "\"required\":" "[\"project\"]}"}, - {"manage_adr", "Manage ADR", "Create or update Architecture Decision Records", + {"manage_adr", "Manage ADR", "Create, update or append to Architecture Decision Records", "{\"type\":\"object\",\"properties\":{\"project\":{\"type\":\"string\"},\"mode\":{\"type\":" - "\"string\",\"enum\":[\"get\",\"update\",\"sections\"]},\"content\":{\"type\":\"string\"}," + "\"string\",\"enum\":[\"get\",\"update\",\"append\",\"sections\"],\"description\":" + "\"get = read; update = REPLACE the whole document; append = add content to the end of " + "the stored document (use this to add an entry without re-sending the ADR); " + "sections = list markdown headers.\"},\"content\":{\"type\":\"string\",\"description\":" + "\"Full document for update, or just the chunk to add for append.\"}," "\"sections\":{\"type\":\"array\",\"items\":{\"type\":\"string\"}}},\"required\":[\"project\"]" "}"}, @@ -10141,6 +10145,45 @@ static char *adr_read_legacy_file(const char *root_path) { return buf; } +/* Join the stored ADR with an appended chunk for mode='append'. + * + * 'update' replaces the whole document, so adding one entry costs a full + * re-send of the ADR — expensive for large documents and an opportunity to + * silently drop existing text on the way back in. Appending concatenates + * server-side instead, so the stored copy is the only source of the prefix. + * + * Trailing newlines on the existing content are trimmed and exactly one blank + * line is inserted, so repeated appends can't accumulate whitespace. An empty + * or missing ADR degrades to a plain create (no leading blank line). + * Returns a heap buffer (caller frees), or NULL on bad input / OOM. */ +static char *adr_append_content(const char *existing, const char *addition) { + if (!addition) { + return NULL; + } + if (!existing || existing[0] == '\0') { + return heap_strdup(addition); + } + size_t elen = strlen(existing); + while (elen > 0 && (existing[elen - SKIP_ONE] == '\n' || existing[elen - SKIP_ONE] == '\r')) { + elen--; + } + if (elen == 0) { + return heap_strdup(addition); + } + static const char sep[] = "\n\n"; + size_t seplen = sizeof(sep) - SKIP_ONE; + size_t alen = strlen(addition); + char *out = malloc(elen + seplen + alen + SKIP_ONE); + if (!out) { + return NULL; + } + memcpy(out, existing, elen); + memcpy(out + elen, sep, seplen); + memcpy(out + elen + seplen, addition, alen); + out[elen + seplen + alen] = '\0'; + return out; +} + #define ADR_EMPTY_HINT \ "No ADR yet. Create one with manage_adr(mode='update', " \ "content='## PURPOSE\\n...\\n\\n## STACK\\n...\\n\\n## ARCHITECTURE\\n..." \ @@ -10269,6 +10312,29 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) { yyjson_mut_obj_add_str(doc, root_obj, "status", "write_error"); is_error = true; } + } else if (strcmp(mode_str, "append") == 0) { + char *merged = content ? adr_append_content(have_adr ? adr.content : NULL, content) : NULL; + if (!content) { + /* Explicit failure rather than falling through to 'get': a caller + * that meant to append must not read a success-shaped response. */ + yyjson_mut_obj_add_str(doc, root_obj, "status", "missing_content"); + yyjson_mut_obj_add_str(doc, root_obj, "error", + "mode='append' requires 'content' (the chunk to add)"); + is_error = true; + } else if (!merged) { + yyjson_mut_obj_add_str(doc, root_obj, "status", "write_error"); + is_error = true; + } else if (cbm_store_adr_store(store, project, merged) == CBM_STORE_OK) { + yyjson_mut_obj_add_str(doc, root_obj, "status", "appended"); + /* Callers verify an append landed without re-fetching the whole + * document, which for large ADRs is the expensive part. */ + yyjson_mut_obj_add_uint(doc, root_obj, "content_length", (uint64_t)strlen(merged)); + yyjson_mut_obj_add_uint(doc, root_obj, "appended_length", (uint64_t)strlen(content)); + } else { + yyjson_mut_obj_add_str(doc, root_obj, "status", "write_error"); + is_error = true; + } + free(merged); } else if (strcmp(mode_str, "sections") == 0) { adr_list_sections_from_content(doc, root_obj, have_adr ? adr.content : NULL); } else { /* get */ diff --git a/tests/test_mcp.c b/tests/test_mcp.c index f1809dccd..c69fa209f 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -4060,6 +4060,138 @@ TEST(tool_manage_adr_unified_backend_issue256) { PASS(); } +/* mode='append' adds to the stored ADR instead of replacing it, so callers can + * add one entry without re-sending the whole document (the re-send is both + * expensive and a chance to silently drop existing text). */ +TEST(tool_manage_adr_append_extends_without_replacing) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(st); + cbm_store_upsert_project(st, "adr-append", "/tmp/adr-append"); + cbm_mcp_server_set_project(srv, "adr-append"); + + char *resp = cbm_mcp_handle_tool(srv, "manage_adr", + "{\"project\":\"adr-append\",\"mode\":\"update\"," + "\"content\":\"## PURPOSE\\nFirst entry.\\n\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_NOT_NULL(strstr(resp, "updated")); + free(resp); + + resp = cbm_mcp_handle_tool(srv, "manage_adr", + "{\"project\":\"adr-append\",\"mode\":\"append\"," + "\"content\":\"## DECISIONS\\nSecond entry.\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_NOT_NULL(strstr(resp, "appended")); + ASSERT_NULL(strstr(resp, "\"isError\":true")); + free(resp); + + /* The prefix must survive verbatim, and the new chunk must land after it. */ + cbm_adr_t adr; + memset(&adr, 0, sizeof(adr)); + ASSERT_EQ(cbm_store_adr_get(st, "adr-append", &adr), CBM_STORE_OK); + ASSERT_NOT_NULL(adr.content); + const char *first = strstr(adr.content, "First entry."); + const char *second = strstr(adr.content, "Second entry."); + ASSERT_NOT_NULL(first); + ASSERT_NOT_NULL(second); + ASSERT(first < second); + /* Exactly one blank line joins them — repeated appends must not pile up + * whitespace, so the trailing newline of the stored copy is trimmed. */ + ASSERT_NOT_NULL(strstr(adr.content, "First entry.\n\n## DECISIONS")); + cbm_store_adr_free(&adr); + + /* Both sections are now discoverable via mode='sections'. */ + resp = cbm_mcp_handle_tool(srv, "manage_adr", + "{\"project\":\"adr-append\",\"mode\":\"sections\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_NOT_NULL(strstr(resp, "## PURPOSE")); + ASSERT_NOT_NULL(strstr(resp, "## DECISIONS")); + free(resp); + + cbm_mcp_server_free(srv); + PASS(); +} + +/* Appending to a project that has no ADR yet behaves as a plain create — no + * leading blank line, no error. */ +TEST(tool_manage_adr_append_creates_when_absent) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(st); + cbm_store_upsert_project(st, "adr-append-new", "/tmp/adr-append-new"); + cbm_mcp_server_set_project(srv, "adr-append-new"); + + char *resp = cbm_mcp_handle_tool(srv, "manage_adr", + "{\"project\":\"adr-append-new\",\"mode\":\"append\"," + "\"content\":\"## PURPOSE\\nOnly entry.\\n\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_NOT_NULL(strstr(resp, "appended")); + free(resp); + + cbm_adr_t adr; + memset(&adr, 0, sizeof(adr)); + ASSERT_EQ(cbm_store_adr_get(st, "adr-append-new", &adr), CBM_STORE_OK); + ASSERT_NOT_NULL(adr.content); + ASSERT_STR_EQ(adr.content, "## PURPOSE\nOnly entry.\n"); + cbm_store_adr_free(&adr); + + cbm_mcp_server_free(srv); + PASS(); +} + +/* append without content must fail loudly. Falling through to 'get' would hand + * the caller a response that looks like a successful read while nothing was + * written. */ +TEST(tool_manage_adr_append_without_content_errors) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(st); + cbm_store_upsert_project(st, "adr-append-empty", "/tmp/adr-append-empty"); + cbm_mcp_server_set_project(srv, "adr-append-empty"); + + char *resp = cbm_mcp_handle_tool(srv, "manage_adr", + "{\"project\":\"adr-append-empty\",\"mode\":\"update\"," + "\"content\":\"## PURPOSE\\nUntouched.\\n\"}"); + ASSERT_NOT_NULL(resp); + free(resp); + + resp = cbm_mcp_handle_tool(srv, "manage_adr", + "{\"project\":\"adr-append-empty\",\"mode\":\"append\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_NOT_NULL(strstr(resp, "missing_content")); + free(resp); + + /* And the stored ADR is untouched. */ + cbm_adr_t adr; + memset(&adr, 0, sizeof(adr)); + ASSERT_EQ(cbm_store_adr_get(st, "adr-append-empty", &adr), CBM_STORE_OK); + ASSERT_NOT_NULL(adr.content); + ASSERT_STR_EQ(adr.content, "## PURPOSE\nUntouched.\n"); + cbm_store_adr_free(&adr); + + cbm_mcp_server_free(srv); + PASS(); +} + +/* The mode must be advertised, otherwise callers never learn it exists and + * keep paying for whole-document rewrites. */ +TEST(tool_manage_adr_append_is_advertised) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + char *resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"tools/list\",\"params\":{}}"); + ASSERT_NOT_NULL(resp); + const char *adr_tool = strstr(resp, "manage_adr"); + ASSERT_NOT_NULL(adr_tool); + ASSERT_NOT_NULL(strstr(adr_tool, "append")); + free(resp); + cbm_mcp_server_free(srv); + PASS(); +} + TEST(tool_manage_adr_mutation_guard_balances_success) { const char *project = "guard-adr-success"; cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); @@ -9423,6 +9555,10 @@ SUITE(mcp) { RUN_TEST(tool_manage_adr_no_project); RUN_TEST(tool_manage_adr_get_with_existing_adr); RUN_TEST(tool_manage_adr_unified_backend_issue256); + RUN_TEST(tool_manage_adr_append_extends_without_replacing); + RUN_TEST(tool_manage_adr_append_creates_when_absent); + RUN_TEST(tool_manage_adr_append_without_content_errors); + RUN_TEST(tool_manage_adr_append_is_advertised); RUN_TEST(tool_index_repository_reports_store_backed_adr); RUN_TEST(tool_index_repository_dot_uses_absolute_project_key_and_preserves_adr); RUN_TEST(index_repository_relative_path_uses_explicit_session_root);