Merge pull request #1552 from DeusData/fix/batch2-regressions-and-determinism

fix: two self-inflicted regressions, evidence parity in json, and the quarantine verdict tests (v0.10.3)
This commit is contained in:
Martin Vogel authored and GitHub committed 2026-08-12 16:13:46 +02:00
commit 70a9539301
14 files changed
+611 -47

No files matched your search

+21 -4
View File
@@ -38,10 +38,27 @@ if [[ "$BINARY" == *.exe ]] && command -v cygpath >/dev/null 2>&1 &&
# Qualify with the domain: a bare name from coreutils `whoami` resolves
# against the machine first, so on a host whose name equals the user's the
# grant lands on an empty principal.
ME="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
ME="${USERDOMAIN}\\${ME}"
fi
# Identify the account by SID, never by name. icacls resolves a bare name
# against the machine first, so a host whose name equals the user's grants
# to an empty principal (#1532); and a USERDOMAIN-qualified name is
# UNRESOLVABLE on a workgroup machine — "WORKGROUP\test: No mapping between
# account names and security IDs was done" — which fails the grant outright
# and silently leaves the tree writable by Authenticated Users. A SID has
# neither ambiguity, and the SYSTEM/Administrators grants below already use
# this form. Name lookup remains only as a fallback where PowerShell is
# unavailable.
ME="$(powershell.exe -NoProfile -NonInteractive -Command \
'[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value' 2>/dev/null |
tr -d '\r\n')"
case "${ME}" in
S-1-*) ME="*${ME}" ;;
*)
ME="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
ME="${USERDOMAIN}\\${ME}"
fi
;;
esac
MSYS2_ARG_CONV_EXCL='*' icacls "$WIN_ROOT_W" /reset /Q >/dev/null 2>&1 || true
if ! MSYS2_ARG_CONV_EXCL='*' icacls "$WIN_ROOT_W" /inheritance:r \
/grant:r "${ME}:(OI)(CI)F" '*S-1-5-18:(OI)(CI)F' '*S-1-5-32-544:(OI)(CI)F' \
+21 -4
View File
@@ -64,10 +64,27 @@ stamp_windows_build_dir() {
# (COMPUTERNAME=BUILD, user build) the grant lands on an empty principal
# (BUILD\) and, combined with /inheritance:r above, locks this script out
# of its own log directory.
me="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
me="${USERDOMAIN}\\${me}"
fi
# Identify the account by SID, never by name. icacls resolves a bare name
# against the machine first, so a host whose name equals the user's grants
# to an empty principal (#1532); and a USERDOMAIN-qualified name is
# UNRESOLVABLE on a workgroup machine — "WORKGROUP\test: No mapping between
# account names and security IDs was done" — which fails the grant outright
# and silently leaves the tree writable by Authenticated Users. A SID has
# neither ambiguity, and the SYSTEM/Administrators grants below already use
# this form. Name lookup remains only as a fallback where PowerShell is
# unavailable.
me="$(powershell.exe -NoProfile -NonInteractive -Command \
'[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value' 2>/dev/null |
tr -d '\r\n')"
case "${me}" in
S-1-*) me="*${me}" ;;
*)
me="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
me="${USERDOMAIN}\\${me}"
fi
;;
esac
# Normalize FIRST: some runner images stamp EXPLICIT (non-inherited)
# Authenticated-Users ACEs onto the workspace tree, which /inheritance:r
# cannot strip and /grant:r does not touch (it replaces only the granted
+21 -4
View File
@@ -80,10 +80,27 @@ soak_stamp_windows_dir() {
# Qualify with the domain: a bare name from coreutils `whoami` resolves
# against the machine first, so on a host whose name equals the user's the
# grant lands on an empty principal.
me="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
me="${USERDOMAIN}\\${me}"
fi
# Identify the account by SID, never by name. icacls resolves a bare name
# against the machine first, so a host whose name equals the user's grants
# to an empty principal (#1532); and a USERDOMAIN-qualified name is
# UNRESOLVABLE on a workgroup machine — "WORKGROUP\test: No mapping between
# account names and security IDs was done" — which fails the grant outright
# and silently leaves the tree writable by Authenticated Users. A SID has
# neither ambiguity, and the SYSTEM/Administrators grants below already use
# this form. Name lookup remains only as a fallback where PowerShell is
# unavailable.
me="$(powershell.exe -NoProfile -NonInteractive -Command \
'[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value' 2>/dev/null |
tr -d '\r\n')"
case "${me}" in
S-1-*) me="*${me}" ;;
*)
me="$(whoami | tr -d '\r')"
if [ -n "${USERDOMAIN:-}" ]; then
me="${USERDOMAIN}\\${me}"
fi
;;
esac
if ! output=$(MSYS2_ARG_CONV_EXCL='*' icacls "$dir_w" /reset /Q 2>&1); then
echo "FAIL: soak DACL normalize failed (dir=$dir_w user=$me)" >&2
printf '%s\n' "$output" >&2
+70 -14
View File
@@ -152,17 +152,31 @@ int cbm_cli_exit_status_after_maintenance(int exit_status, bool maintenance_canc
return maintenance_cancelled && exit_status == EXIT_SUCCESS ? EXIT_FAILURE : exit_status;
}
/* #1537: the refusal is correct, but "sessions could not be stopped" gives the
* reader nothing to act on — which sessions? The daemon already knows every
* client pid and `daemon status` prints them, so name that remedy here rather
* than leaving people to guess (one reporter uninstalled first and still hit
* this, with no way to see what was holding on). */
static const char CLI_ACTIVATION_REFUSED_MESSAGE[] =
/* #1537. Two very different failures reached this one message: the cohort was
* BUSY (real sessions are running — the reader can close them), or the
* reservation failed outright (lock I/O, stale coordination state, permissions
* — nothing the reader can close, because nothing is running). Reporting both
* as "active sessions" sent someone hunting processes that a reboot proved did
* not exist.
*
* The remedy line must also survive the situation it prints in: an install or
* a retry after `uninstall` has no cbm on PATH, so "run codebase-memory-mcp
* daemon status" was advice the reader could not follow at exactly the moment
* they needed it. Each message now names its own condition and stays runnable. */
static const char CLI_ACTIVATION_BUSY_MESSAGE[] =
"error: active CBM sessions and operations could not be stopped safely; "
"no activation was committed.\n"
"error: run 'codebase-memory-mcp daemon status' to list the client "
"processes still holding the daemon (MCP servers started by your editor or "
"agent are the usual holders), close them, then retry.";
"error: something is still using CBM. If an editor or agent is running an "
"MCP server, close it and retry; 'codebase-memory-mcp daemon status' lists "
"the holders when a cbm binary is still installed.";
static const char CLI_ACTIVATION_REFUSED_MESSAGE[] =
"error: activation could not reserve exclusive access; no activation was "
"committed.\n"
"error: this is NOT a running-session problem — the reservation itself "
"failed (coordination lock, leftover state, or permissions). Nothing needs "
"to be closed. Check the errors above, and report this with the output of "
"'ls -la \"${CBM_CACHE_DIR:-$HOME/.cache/codebase-memory-mcp}\"' if it "
"persists.";
static const char CLI_ACTIVATION_PARTIAL_MESSAGE[] =
"error: activation stopped after one or more agent configuration or "
"cleanup operations failed; the published/current executable was kept, "
@@ -211,7 +225,11 @@ static void cli_activation_diagnostic(const cbm_cli_activation_ops_t *ops, const
* stop/reservation failures, which record no refusal note. */
char attributed[CBM_SZ_1K];
const char *note = cbm_activation_transaction_refusal_note();
if (diagnostic == CLI_ACTIVATION_REFUSED_MESSAGE && note && note[0]) {
/* A recorded refusal is more specific than EITHER generic message, so it
* wins over both the busy and the reservation-failure wording. */
if ((diagnostic == CLI_ACTIVATION_REFUSED_MESSAGE ||
diagnostic == CLI_ACTIVATION_BUSY_MESSAGE) &&
note && note[0]) {
(void)snprintf(attributed, sizeof(attributed),
"error: activation was refused by a filesystem safety check before any "
"change was made: %s\n"
@@ -245,7 +263,12 @@ int cbm_cli_activation_guard_with_ops(const cbm_cli_activation_ops_t *ops,
if (mutation_lease) {
ops->mutation_lease_release(ops->context, mutation_lease);
}
cli_activation_diagnostic(ops, CLI_ACTIVATION_REFUSED_MESSAGE);
/* 0 means the cohort was BUSY: real sessions hold it and closing them
* is the remedy. Anything else is the reservation itself failing, and
* telling that reader to close sessions sends them after processes
* that do not exist (#1537). */
cli_activation_diagnostic(ops, reserve_status == 0 ? CLI_ACTIVATION_BUSY_MESSAGE
: CLI_ACTIVATION_REFUSED_MESSAGE);
return CLI_TRUE;
}
@@ -1232,12 +1255,17 @@ int cbm_replace_binary(const char *path, const unsigned char *data, int len, int
static const char skill_content[] =
"---\n"
"name: codebase-memory\n"
"description: Use the codebase knowledge graph for structural code queries. "
/* #1554: the value contains "Triggers on: " — a colon-space, which YAML
* reads as a nested-mapping indicator inside an unquoted scalar. Strict
* parsers (js-yaml's load, the frontmatter reader in `npx skills`) reject
* the whole file, so the skill silently fails to load rather than loading
* wrongly. Quoting the scalar is the fix; the text itself is unchanged. */
"description: \"Use the codebase knowledge graph for structural code queries. "
"Triggers on: explore the codebase, understand the architecture, what functions exist, "
"show me the structure, who calls this function, what does X call, trace the call chain, "
"find callers of, show dependencies, impact analysis, dead code, unused functions, "
"high fan-out, refactor candidates, code quality audit, graph query syntax, "
"Cypher query examples, edge types, how to use search_graph.\n"
"Cypher query examples, edge types, how to use search_graph.\"\n"
"---\n"
"\n"
"# Codebase Memory — Knowledge Graph Tools\n"
@@ -3250,8 +3278,22 @@ int cbm_upsert_codex_mcp(const char *binary_path, const char *config_path) {
return CLI_ERR;
}
char block[CLI_BUF_8K];
/* #1562: Codex sanitizes the environment of stdio MCP subprocesses, passing
* through only the names listed in env_vars. Without CBM_CACHE_DIR the
* Codex-spawned server silently falls back to the DEFAULT cache while the
* account daemon uses the configured one; the two disagree and the
* handshake closes during initialization, so Codex exposes no cbm tools at
* all.
*
* The name is listed unconditionally rather than only when the variable is
* set at install time: env_vars names variables to FORWARD IF PRESENT, so
* listing it costs nothing when unset and keeps working for someone who
* sets it after installing — which install-time detection would silently
* fail to cover. */
int written = snprintf(block, sizeof(block),
CODEX_CMM_SECTION "\ncommand = \"%s\"\nargs = []\n", escaped);
CODEX_CMM_SECTION "\ncommand = \"%s\"\nargs = []\n"
"env_vars = [\"CBM_CACHE_DIR\"]\n",
escaped);
if (written < 0 || (size_t)written >= sizeof(block) ||
cbm_remove_codex_legacy_mcp(config_path) != 0) {
return CLI_ERR;
@@ -11482,17 +11524,31 @@ int cbm_cmd_update(int argc, char **argv) {
bool dry_run = false;
bool force = false;
bool legacy_variant_flag = false;
for (int i = 0; i < argc; i++) {
if (strcmp(argv[i], "--dry-run") == 0) {
dry_run = true;
} else if (strcmp(argv[i], "--force") == 0) {
force = true;
} else if (strcmp(argv[i], "--ui") == 0 || strcmp(argv[i], "--standard") == 0) {
/* v0.10.2 deleted the ui/standard chooser and, with it, these flags —
* which turned `update --ui` from a working command into a hard
* error for everyone who had it in a script, an alias, or muscle
* memory (#1544). Removing a CHOICE is fine; removing the WORDS
* people type is a break we owe them nothing for. Accept both,
* ignore them, and say once why they no longer mean anything. */
legacy_variant_flag = true;
} else if (strcmp(argv[i], "-y") != 0 && strcmp(argv[i], "--yes") != 0 &&
strcmp(argv[i], "-n") != 0 && strcmp(argv[i], "--no") != 0) {
(void)fprintf(stderr, "error: unknown update option: %s\n", argv[i]);
return CLI_TRUE;
}
}
if (legacy_variant_flag) {
(void)fprintf(stderr, "note: --ui/--standard are accepted but no longer do anything; since "
"v0.10.0 there is one build per platform and it always includes the "
"graph UI.\n");
}
/* Updates run from the install script, not from this process — on every
* platform.
+10 -2
View File
@@ -155,8 +155,16 @@ char *cbm_client_adapter_pi(const char *binary_path) {
"}\n\n");
/* Register every tool the registry advertises — never a hand-picked subset,
* which is the drift this generator exists to prevent. */
sb_append(&sb, "export function register(pi) {\n");
* which is the drift this generator exists to prevent.
*
* DEFAULT export, and it must stay that way. Pi loads an extension via
* jiti.import(path, { default: true }) and rejects anything that is not a
* function, so a named `export function register(pi)` hands the loader the
* module namespace object instead and every install fails with "Extension
* does not export a valid factory function". A Pi extension that fails to
* load takes EVERY pi command down with it — `pi doctor` included — not
* just cbm's tools (#1550). */
sb_append(&sb, "export default function (pi) {\n");
for (int i = 0; i < count; i++) {
const char *name = cbm_mcp_tool_name(i);
if (!name || !name[0]) {
+31 -2
View File
@@ -1383,8 +1383,26 @@ static bool posix_directory_parent_secure(int directory_fd) {
!cbm_macos_extended_acl_fd_is_deny_only(directory_fd)) {
return false;
}
return (status.st_mode & 0022) == 0 ||
(status.st_uid == (uid_t)0 && (status.st_mode & S_ISVTX) != 0);
/* #1537: this ANCESTOR check refused any group-write bit, which is the same
* rule #1535 removed on the activation side — and the sibling that decision
* covers but that never got changed. A group-writable ~ or ~/.cache is
* ordinary (WSL2 ships 0775, so do several distro skeletons and any site
* with a shared primary group), and refusing it here made the daemon
* unusable with no way for the reader to see why.
*
* WORLD-writable is still refused: any local user could swap a path
* component. Group-writable is admitted for ancestors only — the private
* directory itself is chmod'd to 0700 and verified after this walk, so the
* thing that actually holds data stays owner-private either way. */
if ((status.st_mode & 0002) != 0) {
return status.st_uid == (uid_t)0 && (status.st_mode & S_ISVTX) != 0;
}
if ((status.st_mode & 0020) != 0) {
char mode_text[16];
(void)snprintf(mode_text, sizeof(mode_text), "%04o", (unsigned)(status.st_mode & 07777));
cbm_log_warn("daemon.private_dir_group_writable_ancestor", "mode", mode_text);
}
return true;
}
/* Validate a path transition only through the two already-open directory
@@ -1435,6 +1453,17 @@ static int private_directory_tree_open(const char *directory_path) {
ok = false;
} else {
ok = posix_directory_parent_secure(current_fd);
if (!ok) {
/* #1537: this branch used to leave the detail empty, so the
* caller fell back to printing errno — which NOTHING here sets.
* A reporter was handed "errno 2" (ENOENT) for a permission
* refusal and went looking for a missing file that existed.
* An unset errno is not a diagnosis; name the component. */
ipc_validation_detail_set("%s: ancestor '%s' is not a usable private-directory "
"parent (must be owned by you, not world-writable, and "
"carry no allow-ACL)",
directory_path, component);
}
bool created = ok && mkdirat(current_fd, component, 0700) == 0;
if (!created && errno != EEXIST) {
ok = false;
+38 -2
View File
@@ -882,6 +882,26 @@ static cbm_proc_poll_t cbm_subprocess_poll_win(cbm_subprocess_t *process, cbm_pr
#else /* POSIX */
/* Transient spawn-failure retry (see the EAGAIN note in cbm_posix_spawn_apple).
* Three attempts over ~30ms total: long enough to ride out a burst of process
* creation, short enough that a genuinely exhausted system still fails fast. */
enum { CBM_SPAWN_RETRY = 2, CBM_SPAWN_RETRY_ATTEMPTS = 3 };
static const struct timespec CBM_SPAWN_RETRY_DELAY = {0, 10L * 1000L * 1000L};
#define CBM_SPAWN_RETRY_DELAY_NS (&CBM_SPAWN_RETRY_DELAY)
/* fork() fails with EAGAIN under the same pressure posix_spawn does, and the
* fallback path must not be less robust than the primary one. */
static pid_t cbm_fork_with_retry(void) {
for (int attempt = 0; attempt < CBM_SPAWN_RETRY_ATTEMPTS; attempt++) {
pid_t pid = fork();
if (pid >= 0 || (errno != EAGAIN && errno != ENOMEM)) {
return pid;
}
(void)cbm_nanosleep(CBM_SPAWN_RETRY_DELAY_NS, NULL);
}
return fork();
}
/* Used by the fork+exec child. posix_spawn performs the same reset
* declaratively via SETSIGDEF + SETSIGMASK, but Apple still forks for the
* exec-failure fallback below, so this stays compiled everywhere. */
@@ -1010,6 +1030,14 @@ static int cbm_posix_spawn_apple(cbm_subprocess_t *process, int input, int outpu
rc == ELOOP || rc == ENAMETOOLONG || rc == ENOTDIR)) {
return 1;
}
/* EAGAIN/ENOMEM are the kernel saying "not right now", not "never": the
* process table or a per-user limit is momentarily full. Reporting
* spawn_failed for that turns transient load into a user-visible error —
* a git or LSP probe failing on a busy laptop for no reason the user can
* see or act on. Retry briefly. Everything else stays a hard failure. */
if (configured && (rc == EAGAIN || rc == ENOMEM)) {
return CBM_SPAWN_RETRY;
}
return -1;
}
#endif
@@ -1059,13 +1087,21 @@ static int cbm_subprocess_spawn_posix(cbm_subprocess_t *process) {
pid_t pid = -1;
#ifdef __APPLE__
int spawn_rc = cbm_posix_spawn_apple(process, input, output, &pid);
for (int attempt = 0; spawn_rc == CBM_SPAWN_RETRY && attempt < CBM_SPAWN_RETRY_ATTEMPTS;
attempt++) {
(void)cbm_nanosleep(CBM_SPAWN_RETRY_DELAY_NS, NULL);
spawn_rc = cbm_posix_spawn_apple(process, input, output, &pid);
}
if (spawn_rc == CBM_SPAWN_RETRY) {
spawn_rc = -1; /* still exhausted after backoff: a real failure */
}
if (spawn_rc < 0) {
(void)close(input);
(void)close(output);
return -1;
}
if (spawn_rc > 0) { /* exec-class failure: reproduce the fork+exec 127 */
pid = fork();
pid = cbm_fork_with_retry();
if (pid < 0) {
(void)close(input);
(void)close(output);
@@ -1076,7 +1112,7 @@ static int cbm_subprocess_spawn_posix(cbm_subprocess_t *process) {
}
}
#else
pid = fork();
pid = cbm_fork_with_retry();
if (pid < 0) {
(void)close(input);
(void)close(output);
+44 -8
View File
@@ -601,7 +601,7 @@ static const tool_def_t TOOLS[] = {
"\"description\":\"Max enriched results per call. Default 10. Response includes "
"'total_grep_matches' and 'total_results' so callers can detect truncation. No "
"offset parameter — raise limit or narrow with file_pattern / path_filter to see more."
"\",\"default\":10}},\"required\":[\"pattern\",\"project\"]}"},
"\",\"default\":10,\"minimum\":1}},\"required\":[\"pattern\",\"project\"]}"},
{"list_projects", "List projects", "List all indexed projects",
"{\"type\":\"object\",\"properties\":{}}"},
@@ -6203,7 +6203,8 @@ static int trace_watermark_index(const cbm_traverse_result_t *tr, int hop, int64
* output — {cols, groups:[{qn_prefix, rows:[[name,hop,...]]}]}. Optional
* risk/args columns mirror the flags. */
static yyjson_mut_val *bfs_to_tree_json(yyjson_mut_doc *doc, cbm_traverse_result_t *tr,
bool risk_labels, bool include_tests, bool data_flow) {
bool risk_labels, bool include_tests, bool data_flow,
bool include_evidence) {
yyjson_mut_val *leg = yyjson_mut_obj(doc);
yyjson_mut_val *cols = yyjson_mut_arr(doc);
yyjson_mut_arr_add_str(doc, cols, "name");
@@ -6211,6 +6212,14 @@ static yyjson_mut_val *bfs_to_tree_json(yyjson_mut_doc *doc, cbm_traverse_result
if (risk_labels) {
yyjson_mut_arr_add_str(doc, cols, "risk");
}
/* #1542: include_evidence was implemented on the tree path only, so
* format:"json" silently returned cols ["name","hop"] while the schema and
* --help promised two more. A structured caller — the one most likely to
* ask for json — got no error, just missing fields. */
if (include_evidence) {
yyjson_mut_arr_add_str(doc, cols, "strategy");
yyjson_mut_arr_add_str(doc, cols, "confidence");
}
if (data_flow) {
yyjson_mut_arr_add_str(doc, cols, "args");
}
@@ -6258,6 +6267,25 @@ static yyjson_mut_val *bfs_to_tree_json(yyjson_mut_doc *doc, cbm_traverse_result
yyjson_mut_arr_add_str(doc, row, "");
}
}
if (include_evidence) {
const char *ev_class = NULL;
double ev_conf = -1.0;
if (bfs_edge_evidence_for_hop(tr, tr->visited[i].node.id, &ev_class, &ev_conf)) {
yyjson_mut_arr_add_strcpy(doc, row, ev_class ? ev_class : "");
if (ev_conf >= 0.0) {
yyjson_mut_arr_add_real(doc, row, ev_conf);
} else {
yyjson_mut_arr_add_null(doc, row);
}
} else {
/* The root hop has no inbound edge and non-CALLS edges record
* no strategy. The tree path emits "-" placeholders to keep the
* column count fixed; json says null, which is the same promise
* in a form a structured caller can test. */
yyjson_mut_arr_add_null(doc, row);
yyjson_mut_arr_add_null(doc, row);
}
}
yyjson_mut_arr_add_val(cur_rows, row);
}
yyjson_mut_obj_add_val(doc, leg, "groups", groups);
@@ -6710,15 +6738,15 @@ static char *handle_trace_call_path(cbm_mcp_server_t *srv, const char *args) {
}
if (do_outbound) {
yyjson_mut_obj_add_int(doc, root, "callees_total", out_total);
yyjson_mut_obj_add_val(
doc, root, "callees",
bfs_to_tree_json(doc, &view_out, risk_labels, include_tests, data_flow));
yyjson_mut_obj_add_val(doc, root, "callees",
bfs_to_tree_json(doc, &view_out, risk_labels, include_tests,
data_flow, include_evidence));
}
if (do_inbound) {
yyjson_mut_obj_add_int(doc, root, "callers_total", in_total);
yyjson_mut_obj_add_val(
doc, root, "callers",
bfs_to_tree_json(doc, &view_in, risk_labels, include_tests, data_flow));
yyjson_mut_obj_add_val(doc, root, "callers",
bfs_to_tree_json(doc, &view_in, risk_labels, include_tests,
data_flow, include_evidence));
}
if (more_rows) {
yyjson_mut_obj_add_bool(doc, root, "truncated", true);
@@ -9606,6 +9634,14 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) {
char *path_filter = cbm_mcp_get_string_arg(args, "path_filter");
char *mode_str = cbm_mcp_get_string_arg(args, "mode");
int limit = cbm_mcp_get_int_arg(args, "limit", MCP_DEFAULT_LIMIT);
/* #1511: a negative limit flowed straight into the result cap and came back
* as the reported count ("results: -5"), which reads to an agent as a real
* answer rather than a rejected argument. The schema now declares
* minimum:1, but a schema is a request to the client, never a guarantee to
* the server — clamp here too. */
if (limit < 1) {
limit = MCP_DEFAULT_LIMIT;
}
int context_lines = cbm_mcp_get_int_arg(args, "context", 0);
bool use_regex = cbm_mcp_get_bool_arg(args, "regex");
uint64_t search_t0 = cbm_now_ms();
+19
View File
@@ -1096,6 +1096,24 @@ TEST(client_adapter_pi_registers_every_registry_tool) {
PASS();
}
/* #1550: Pi loads an extension by calling its DEFAULT export as a factory. The
* generator emitted a named `register` export instead, so the file failed to
* load — and a Pi extension that fails to load takes every pi command with it
* (`pi doctor` included), not just cbm's tools. The blast radius is why this is
* pinned rather than left to the registry-drift test above, which passed the
* whole time this was broken: it checked WHICH tools were listed, never whether
* the file Pi loads is loadable. */
TEST(client_adapter_pi_default_exports_its_factory_issue1550) {
char *js = cbm_client_adapter_pi("/usr/local/bin/codebase-memory-mcp");
ASSERT_NOT_NULL(js);
ASSERT_NOT_NULL(strstr(js, "export default function (pi)"));
/* The old shape must be gone: a bare `export function register(pi)` is the
* exact form Pi rejects. */
ASSERT_NULL(strstr(js, "export function register(pi)"));
free(js);
PASS();
}
/* #616 rejected `"` in its path template but not `\`, so a Windows home like
* C:\Users\urs\bin produced `\u` — an invalid unicode escape — and the whole
* auto-loaded plugin failed to parse. A broken plugin in an auto-load directory
@@ -1185,6 +1203,7 @@ SUITE(agent_clients) {
RUN_TEST(agent_clients_continue_uses_owned_yaml_sequence_item);
RUN_TEST(agent_clients_continue_refuses_foreign_same_name_and_nonsequence_section);
RUN_TEST(client_adapter_pi_registers_every_registry_tool);
RUN_TEST(client_adapter_pi_default_exports_its_factory_issue1550);
RUN_TEST(client_adapter_escapes_windows_paths_and_quotes);
RUN_TEST(client_adapter_opencode_sends_the_required_hook_event);
RUN_TEST(client_adapter_rejects_missing_binary_path);
+113
View File
@@ -682,6 +682,40 @@ TEST(cli_activation_refusal_note_reaches_diagnostic_issue1416) {
PASS();
}
/* #1537: a reservation that FAILED (lock I/O, leftover coordination state,
* permissions) is not a reservation that was BUSY. Reporting both as "active
* CBM sessions could not be stopped" sent a reporter hunting processes that a
* reboot proved did not exist — and the remedy we printed told them to run a
* cbm binary they had just uninstalled. Each condition now names itself. */
TEST(cli_activation_distinguishes_busy_from_reservation_failure_issue1537) {
cbm_activation_transaction_note_refusal_for_testing(NULL, 0UL);
/* BUSY (0): real sessions hold the cohort — closing something is the fix. */
cli_activation_fake_t busy = {
.participants_active = true,
.mutation_reserve_result = 0,
};
cbm_cli_activation_ops_t ops = {
.context = &busy,
.reserve_for_mutation = cli_activation_fake_reserve_mutation,
.mutation_lease_release = cli_activation_fake_release_mutation,
.visible_diagnostic = cli_activation_fake_diagnostic,
};
ASSERT_EQ(cbm_cli_activation_guard_with_ops(&ops, cli_activation_fake_mutation, &busy), 1);
ASSERT_NOT_NULL(strstr(busy.diagnostic, "could not be stopped safely"));
/* FAILURE (-1): nothing is running, so the reader must not be told to close
* sessions — and must not be handed a command that may not be installed. */
cli_activation_fake_t failed = {
.mutation_reserve_result = -1,
};
ops.context = &failed;
ASSERT_EQ(cbm_cli_activation_guard_with_ops(&ops, cli_activation_fake_mutation, &failed), 1);
ASSERT_NOT_NULL(strstr(failed.diagnostic, "NOT a running-session problem"));
ASSERT_NULL(strstr(failed.diagnostic, "could not be stopped safely"));
PASS();
}
TEST(cli_activation_refuses_unsafe_cohort_reservation) {
cli_activation_fake_t fake = {
.mutation_reserve_result = -1,
@@ -10088,6 +10122,11 @@ TEST(cli_upsert_codex_mcp_fresh) {
ASSERT_NOT_NULL(data);
ASSERT(strstr(data, "[mcp_servers.codebase-memory-mcp]") != NULL);
ASSERT(strstr(data, "/usr/local/bin/codebase-memory-mcp") != NULL);
/* #1562: Codex passes only the names listed in env_vars into a stdio MCP
* subprocess. Without CBM_CACHE_DIR the spawned server uses the DEFAULT
* cache while the daemon uses the configured one, the two disagree, and the
* handshake closes — Codex then shows no cbm tools at all. */
ASSERT(strstr(data, "env_vars = [\"CBM_CACHE_DIR\"]") != NULL);
test_rmdir_r(tmpdir);
PASS();
@@ -12061,6 +12100,77 @@ TEST(cli_sha256_file_matches_known_vector) {
PASS();
}
/* #1544: v0.10.2 deleted the ui/standard chooser AND the flags that drove it,
* so `update --ui` — a command people had in scripts and aliases — started
* failing with "unknown update option". Retiring a choice is fine; breaking the
* words people already type is not. Both flags must be accepted, do nothing,
* and say so once. A genuinely unknown flag must still be rejected, or this
* degenerates into ignoring typos. */
/* #1554: the skill's `description` contained "Triggers on: " — a colon-space,
* which YAML reads as a nested-mapping indicator inside an UNQUOTED scalar.
* Strict readers (js-yaml's load, the frontmatter parser in `npx skills`)
* reject the whole document, so the installed skill silently fails to load.
*
* The rule is checked for EVERY skill, not just the one that broke: any
* frontmatter scalar whose value contains ": " must be quoted. Pinning only
* the current text would let the next added skill reintroduce it. */
TEST(cli_skill_frontmatter_scalars_with_colons_are_quoted_issue1554) {
const cbm_skill_t *sk = cbm_get_skills();
ASSERT_NOT_NULL(sk);
for (int i = 0; i < CBM_SKILL_COUNT; i++) {
const char *body = sk[i].content;
ASSERT_NOT_NULL(body);
/* Frontmatter is the block between the first two "---" lines. */
ASSERT_TRUE(strncmp(body, "---\n", 4) == 0);
const char *end = strstr(body + 4, "\n---\n");
ASSERT_NOT_NULL(end);
const char *line = body + 4;
while (line < end) {
const char *eol = strchr(line, '\n');
if (!eol || eol > end) {
break;
}
const char *sep = NULL;
for (const char *c = line; c < eol - 1; c++) {
if (c[0] == ':' && c[1] == ' ') {
sep = c;
break;
}
}
if (sep) {
const char *value = sep + 2;
/* A value that itself contains ": " must be quoted, or a
* strict parser treats it as a nested mapping. */
bool has_inner_colon = false;
for (const char *c = value; c < eol - 1; c++) {
if (c[0] == ':' && c[1] == ' ') {
has_inner_colon = true;
break;
}
}
if (has_inner_colon) {
ASSERT_TRUE(value[0] == '"' || value[0] == '\'');
}
}
line = eol + 1;
}
}
PASS();
}
TEST(cli_update_accepts_retired_variant_flags_issue1544) {
char *ui_argv[] = {"--dry-run", "--ui", "--yes"};
char *standard_argv[] = {"--dry-run", "--standard", "--yes"};
char *bogus_argv[] = {"--dry-run", "--not-a-flag", "--yes"};
ASSERT_EQ(cbm_cmd_update(3, ui_argv), 0);
ASSERT_EQ(cbm_cmd_update(3, standard_argv), 0);
/* Unknown flags stay an error — the point is compatibility, not silence. */
ASSERT_TRUE(cbm_cmd_update(3, bogus_argv) != 0);
PASS();
}
#ifdef _WIN32
/* The Windows update contract, asserted with the activation seam OFF so this
* takes the exact dispatch a release binary ships: `update` never replaces the
@@ -12120,6 +12230,7 @@ SUITE(cli) {
RUN_TEST(cli_activation_quiesces_active_cohort_before_mutation);
RUN_TEST(cli_activation_refuses_when_cohort_does_not_drain);
RUN_TEST(cli_activation_refusal_note_reaches_diagnostic_issue1416);
RUN_TEST(cli_activation_distinguishes_busy_from_reservation_failure_issue1537);
RUN_TEST(cli_activation_refuses_unsafe_cohort_reservation);
RUN_TEST(cli_activation_releases_maintenance_lease_after_success);
RUN_TEST(cli_activation_releases_maintenance_lease_when_mutation_fails);
@@ -12136,6 +12247,8 @@ SUITE(cli) {
RUN_TEST(cli_install_config_failure_keeps_published_binary);
RUN_TEST(cli_update_download_failure_does_not_quiesce_sessions);
RUN_TEST(cli_update_already_current_does_not_quiesce_sessions);
RUN_TEST(cli_skill_frontmatter_scalars_with_colons_are_quoted_issue1554);
RUN_TEST(cli_update_accepts_retired_variant_flags_issue1544);
RUN_TEST(cli_update_agent_configs_finish_before_guard_release);
RUN_TEST(cli_uninstall_quiesces_active_cohort_before_removing_binary_and_index);
RUN_TEST(cli_uninstall_preserves_binary_and_index_when_cohort_does_not_drain);
+21 -3
View File
@@ -830,9 +830,27 @@ static bool frontend_backpressure_run_isolated(bool maintenance) {
* below that budget SIGKILLs a slow-but-legitimate startup — on
* oversubscribed CI runners this fired every round (announced=0,
* daemon_signal=9) while fast local machines never saw it. Keep the
* deadline comfortably above the daemon's own worst-case budget. */
bool announced_read = daemon > 0 && frontend_test_read_byte(ready_pipe[0], &announced_marker,
cbm_now_ms() + 30000U);
* deadline comfortably above the daemon's own worst-case budget.
*
* This wait is a LIVENESS BACKSTOP, not a race budget: the daemon either
* announces or it does not, and the test asserts the announcement, never a
* timing window. A backstop is only doing its job if it never fires on a
* legitimately slow start — so it has to scale with the build. Under
* MemorySanitizer (instrumented libc++ and origin tracking) startup runs
* several times slower than the 12s budget it must clear, which is what
* reddened test-msan while every other lane and the same suite under local
* MSan stayed green. Widening the backstop for sanitized builds costs
* nothing when the daemon is healthy: a passing run returns as soon as the
* byte arrives, whatever the ceiling is. */
#if defined(CBM_SANITIZED_BUILD) || defined(__SANITIZE_ADDRESS__) || \
defined(__SANITIZE_MEMORY__) || defined(__SANITIZE_THREAD__)
const uint64_t announce_backstop_ms = 180000U;
#else
const uint64_t announce_backstop_ms = 30000U;
#endif
bool announced_read =
daemon > 0 && frontend_test_read_byte(ready_pipe[0], &announced_marker,
cbm_now_ms() + announce_backstop_ms);
bool announced = announced_read && announced_marker == 'R';
/* This raw runtime client intentionally owns no cohort lease. It is a
+80 -2
View File
@@ -1667,8 +1667,7 @@ TEST(daemon_ipc_endpoint_is_namespaced_by_instance_key) {
if (a_startup_status == 1 && !cbm_daemon_ipc_startup_lock_prepare_handoff(a_startup)) {
a_startup_status = -1;
}
if (other_startup_status == 1 &&
!cbm_daemon_ipc_startup_lock_prepare_handoff(other_startup)) {
if (other_startup_status == 1 && !cbm_daemon_ipc_startup_lock_prepare_handoff(other_startup)) {
other_startup_status = -1;
}
cbm_daemon_ipc_startup_lock_release(&a_startup);
@@ -4712,6 +4711,83 @@ TEST(daemon_ipc_posix_rejects_non_socket_and_symlink_endpoints) {
#endif /* !_WIN32 */
#ifndef _WIN32
/* #1537: the daemon's private-directory ancestry walk refused ANY group-write
* bit on an ancestor — the same rule #1535 removed on the activation side, and
* the sibling the consolidated strictness decision covers but that never got
* changed. A group-writable ~ or ~/.cache is ordinary (WSL2 ships 0775, so do
* several distro skeletons and any shared-primary-group site), so the daemon
* was unusable there. World-writable must still be refused: any local user
* could swap a path component. And the private directory itself must still end
* up owner-private, which is what makes admitting the ancestor safe. */
TEST(daemon_ipc_posix_group_writable_ancestor_is_admitted_issue1537) {
char parent[TEST_PATH_CAP];
char ancestor[TEST_PATH_CAP];
char cache[TEST_PATH_CAP];
bool paths_ok = false;
bool ancestor_ready = false;
bool secured = false;
bool leaf_owner_private = false;
if (ipc_test_parent_new(parent, "posix-group-writable-ancestor")) {
int a = snprintf(ancestor, sizeof(ancestor), "%s/group-writable", parent);
int c = snprintf(cache, sizeof(cache), "%s/cache", ancestor);
paths_ok = a > 0 && a < (int)sizeof(ancestor) && c > 0 && c < (int)sizeof(cache);
}
if (paths_ok) {
ancestor_ready = mkdir(ancestor, 0775) == 0 && chmod(ancestor, 0775) == 0;
}
if (ancestor_ready) {
secured = cbm_daemon_ipc_private_directory_secure(cache);
struct stat leaf;
leaf_owner_private =
stat(cache, &leaf) == 0 && S_ISDIR(leaf.st_mode) && (leaf.st_mode & 07777) == 0700;
}
(void)rmdir(cache);
(void)rmdir(ancestor);
ipc_test_remove_flat_dir(parent);
ASSERT_TRUE(paths_ok);
ASSERT_TRUE(ancestor_ready);
ASSERT_TRUE(secured);
ASSERT_TRUE(leaf_owner_private);
PASS();
}
/* The other half of the same decision: world-writable stays refused. A rule
* that admitted everything would "fix" #1537 by removing the protection. */
TEST(daemon_ipc_posix_world_writable_ancestor_still_refused_issue1537) {
char parent[TEST_PATH_CAP];
char ancestor[TEST_PATH_CAP];
char cache[TEST_PATH_CAP];
bool paths_ok = false;
bool ancestor_ready = false;
bool refused = false;
if (ipc_test_parent_new(parent, "posix-world-writable-ancestor")) {
int a = snprintf(ancestor, sizeof(ancestor), "%s/world-writable", parent);
int c = snprintf(cache, sizeof(cache), "%s/cache", ancestor);
paths_ok = a > 0 && a < (int)sizeof(ancestor) && c > 0 && c < (int)sizeof(cache);
}
if (paths_ok) {
ancestor_ready = mkdir(ancestor, 0777) == 0 && chmod(ancestor, 0777) == 0;
}
if (ancestor_ready) {
refused = !cbm_daemon_ipc_private_directory_secure(cache);
}
(void)rmdir(cache);
(void)rmdir(ancestor);
ipc_test_remove_flat_dir(parent);
ASSERT_TRUE(paths_ok);
ASSERT_TRUE(ancestor_ready);
ASSERT_TRUE(refused);
PASS();
}
#endif /* !_WIN32 */
SUITE(daemon_ipc) {
RUN_TEST(daemon_ipc_pending_timeout_race_returns_completed_io);
RUN_TEST(daemon_ipc_pending_wait_failure_cancels_and_drains);
@@ -4750,6 +4826,8 @@ SUITE(daemon_ipc) {
RUN_TEST(daemon_ipc_windows_startup_release_retains_retry_authority);
RUN_TEST(daemon_ipc_frame_timeout_poisons_connection);
#ifndef _WIN32
RUN_TEST(daemon_ipc_posix_group_writable_ancestor_is_admitted_issue1537);
RUN_TEST(daemon_ipc_posix_world_writable_ancestor_still_refused_issue1537);
RUN_TEST(daemon_ipc_posix_startup_lock_is_cross_process);
RUN_TEST(daemon_ipc_posix_lifetime_reservation_rejects_fork_inheritance);
RUN_TEST(daemon_ipc_posix_child_participant_handoff_retains_legacy_bridge);
+76
View File
@@ -3161,6 +3161,26 @@ TEST(tool_trace_path_evidence_is_opt_in_and_class_mapped) {
ASSERT_NULL(strstr(ev_txt, "lsp_trait_dispatch"));
free(ev_txt);
free(ev);
/* #1542: the same request with format:"json" returned cols ["name","hop"]
* — include_evidence was implemented on the tree path only, so the callers
* most likely to ask for structured output were the ones who silently got
* nothing. The two formats must promise the same fields. */
char *ev_json = cbm_mcp_server_handle(
srv, "{\"jsonrpc\":\"2.0\",\"id\":93,\"method\":\"tools/call\","
"\"params\":{\"name\":\"trace_path\",\"arguments\":{\"function_name\":\"caller\","
"\"project\":\"ev-proj\",\"direction\":\"outbound\",\"include_evidence\":true,"
"\"format\":\"json\"}}}");
ASSERT_NOT_NULL(ev_json);
char *ev_json_txt = extract_text_content(ev_json);
ASSERT_NOT_NULL(ev_json_txt);
ASSERT_NOT_NULL(strstr(ev_json_txt, "\"strategy\""));
ASSERT_NOT_NULL(strstr(ev_json_txt, "\"confidence\""));
ASSERT_NOT_NULL(strstr(ev_json_txt, "lsp"));
ASSERT_NOT_NULL(strstr(ev_json_txt, "0.95"));
ASSERT_NULL(strstr(ev_json_txt, "lsp_trait_dispatch"));
free(ev_json_txt);
free(ev_json);
cbm_mcp_server_free(srv);
PASS();
}
@@ -3992,6 +4012,60 @@ TEST(tool_search_code_missing_pattern) {
PASS();
}
/* #1511 (distilled from @lukiod's #1512): search_code echoed a negative limit
* back as the result count — "results: -5" — which an agent reads as an answer,
* not as a rejected argument. Both halves matter: the schema declares the bound
* so well-behaved clients never send it, and the handler clamps because a
* schema is a request to the client, never a guarantee to the server. */
TEST(tool_search_code_negative_limit_is_not_echoed_issue1511) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
char *resp =
cbm_mcp_server_handle(srv, "{\"jsonrpc\":\"2.0\",\"id\":35,\"method\":\"tools/call\","
"\"params\":{\"name\":\"search_code\","
"\"arguments\":{\"pattern\":\"func main\","
"\"project\":\"nonexistent\",\"limit\":-5}}}");
ASSERT_NOT_NULL(resp);
ASSERT_NULL(strstr(resp, "results: -5"));
free(resp);
cbm_mcp_server_free(srv);
PASS();
}
TEST(tool_search_code_limit_declares_a_minimum_issue1511) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
char *resp = cbm_mcp_server_handle(
srv, "{\"jsonrpc\":\"2.0\",\"id\":36,\"method\":\"tools/list\",\"params\":{}}");
ASSERT_NOT_NULL(resp);
yyjson_doc *doc = yyjson_read(resp, strlen(resp), 0);
yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL;
yyjson_val *result = root ? yyjson_obj_get(root, "result") : NULL;
yyjson_val *tools = result ? yyjson_obj_get(result, "tools") : NULL;
yyjson_val *minimum = NULL;
if (tools && yyjson_is_arr(tools)) {
size_t index, max;
yyjson_val *tool;
yyjson_arr_foreach(tools, index, max, tool) {
yyjson_val *name = yyjson_obj_get(tool, "name");
if (!name || !yyjson_is_str(name) || strcmp(yyjson_get_str(name), "search_code") != 0) {
continue;
}
yyjson_val *schema = yyjson_obj_get(tool, "inputSchema");
yyjson_val *props = schema ? yyjson_obj_get(schema, "properties") : NULL;
yyjson_val *limit = props ? yyjson_obj_get(props, "limit") : NULL;
minimum = limit ? yyjson_obj_get(limit, "minimum") : NULL;
break;
}
}
bool declared = minimum && yyjson_is_int(minimum) && yyjson_get_int(minimum) >= 1;
yyjson_doc_free(doc);
free(resp);
cbm_mcp_server_free(srv);
ASSERT_TRUE(declared);
PASS();
}
TEST(tool_search_code_no_project) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
@@ -10571,6 +10645,8 @@ SUITE(mcp) {
RUN_TEST(tool_get_code_snippet_missing_qn);
RUN_TEST(tool_get_code_snippet_not_found);
RUN_TEST(tool_search_code_missing_pattern);
RUN_TEST(tool_search_code_negative_limit_is_not_echoed_issue1511);
RUN_TEST(tool_search_code_limit_declares_a_minimum_issue1511);
RUN_TEST(tool_search_code_no_project);
RUN_TEST(search_code_multi_word);
RUN_TEST(search_code_scoped_path_with_spaces_issue687);
+46 -2
View File
@@ -544,8 +544,7 @@ TEST(store_lsp_surface_round_trip) {
ASSERT_STR_EQ(rows[0].defs_json, "[{\"qn\":\"pkg.A\",\"sn\":\"A\",\"label\":\"Function\"}]");
ASSERT_STR_EQ(rows[0].config_ctx, "cfg-1");
ASSERT_EQ(rows[0].ref_bloom_len, (int)sizeof(bloom));
ASSERT_TRUE(rows[0].ref_bloom != NULL &&
memcmp(rows[0].ref_bloom, bloom, sizeof(bloom)) == 0);
ASSERT_TRUE(rows[0].ref_bloom != NULL && memcmp(rows[0].ref_bloom, bloom, sizeof(bloom)) == 0);
ASSERT_STR_EQ(rows[1].rel_path, "pkg/b.go");
ASSERT_STR_EQ(rows[1].defs_json, "[]");
ASSERT_EQ(rows[1].ref_bloom_len, 0);
@@ -1265,6 +1264,48 @@ TEST(store_integrity_corrupt_too_many_rows) {
PASS();
}
/* ── Quarantine verdict (#1206, #1037) ──────────────────────────────
*
* The bool check above cannot answer the only question the quarantine path
* actually asks: "is this database damaged, or did I just lose a lock race?"
* Answering "damaged" to the second question is what made concurrent instances
* rename each other's HEALTHY databases to .corrupt (#1206). These bind the
* three-way verdict so that behaviour cannot come back. */
TEST(store_integrity_verdict_healthy_is_ok) {
cbm_store_t *s = cbm_store_open_memory();
ASSERT_NOT_NULL(s);
cbm_store_upsert_project(s, "healthy-proj", "/tmp/healthy");
ASSERT_EQ(cbm_store_check_integrity_verdict(s), CBM_INTEGRITY_OK);
cbm_store_close(s);
PASS();
}
TEST(store_integrity_verdict_real_corruption_is_corrupt) {
/* Structural damage the shallow check can see: node IDs landing in
* root_path. This one MUST be quarantinable — a verdict that never says
* CORRUPT would protect broken databases instead of users. */
cbm_store_t *s = cbm_store_open_memory();
ASSERT_NOT_NULL(s);
sqlite3 *db = cbm_store_get_db(s);
sqlite3_exec(db,
"INSERT INTO projects (name, indexed_at, root_path) "
"VALUES ('broken', '2024-01-01', '826');",
NULL, NULL, NULL);
ASSERT_EQ(cbm_store_check_integrity_verdict(s), CBM_INTEGRITY_CORRUPT);
cbm_store_close(s);
PASS();
}
TEST(store_integrity_verdict_unopenable_is_transient_not_corrupt) {
/* A handle we could not open tells us NOTHING about the file's contents.
* Reporting CORRUPT here is how a database nobody could read got renamed
* and rebuilt from scratch (#1206) — the destructive answer to a question
* that was never asked. */
ASSERT_EQ(cbm_store_check_integrity_verdict(NULL), CBM_INTEGRITY_TRANSIENT);
PASS();
}
TEST(store_integrity_null_check) {
/* NULL store should return false (not crash) */
ASSERT_FALSE(cbm_store_check_integrity(NULL));
@@ -2144,6 +2185,9 @@ SUITE(store_nodes) {
RUN_TEST(store_integrity_corrupt_bad_path);
RUN_TEST(store_integrity_windows_lowercase_drive_issue367);
RUN_TEST(store_integrity_corrupt_too_many_rows);
RUN_TEST(store_integrity_verdict_healthy_is_ok);
RUN_TEST(store_integrity_verdict_real_corruption_is_corrupt);
RUN_TEST(store_integrity_verdict_unopenable_is_transient_not_corrupt);
RUN_TEST(store_integrity_null_check);
RUN_TEST(store_project_crud);
RUN_TEST(store_project_update);