From 1891d448054ae8ff111d2797d91ec93bf3820907 Mon Sep 17 00:00:00 2001 From: Vladislav Rahmanov Date: Thu, 10 Sep 2026 17:50:01 +0300 Subject: [PATCH 1/4] feat(cli): grade index_repository outcomes with distinct exit codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A freshness gate cannot read a tool's account of itself — it reads the process status. index_repository returned 0 for a partial index and the same 1 for both a missing repository and a pipeline that fell over inside an existing one, so no caller could tell those apart. The CLI now grades its own outcome: 0 indexed, quality at or above the thresholds 1 hard failure inside a reachable repository (unchanged meaning) 2 indexed but below a quality threshold 3 repo_path absent, unreadable, or not a directory The contract is opt-in: it applies only with CBM_GATE set to a non-empty value other than "0". Without it the CLI exits exactly as before, so a caller that never asked for grading never sees a new code. Grading happens in the CLI only. MCP responses are untouched, so agents and the soak test, which judge by JSON-RPC and tool-level isError, see exactly what they saw before. Thresholds are separate on purpose and overridable: CBM_GATE_MAX_UNUSABLE default 0 a file that did not parse at all CBM_GATE_MAX_PARTIAL_PCT default 10 share of partially parsed files Partial parsing marks constructs a grammar does not cover — a property of language support, not of index quality; this repository itself sits at 5.8%. A gate its own project cannot pass gets switched off. Two additive fields carry what the grading needs: files_indexed, parse_partial_pct the denominator the ratio needs and the share stated outright, so a gate can quote it in a refusal reason on failures: target_unavailable vs pipeline_failed Signed-off-by: Vladislav Rahmanov --- src/cli/cli.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/cli/cli.h | 42 +++++++++++++++++++++++++++++ src/main.c | 5 ++++ src/mcp/mcp.c | 32 ++++++++++++++++++++++ 4 files changed, 154 insertions(+) diff --git a/src/cli/cli.c b/src/cli/cli.c index f47c3031ca..4ce60e9c47 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -157,6 +157,81 @@ int cbm_cli_exit_status_after_maintenance(int exit_status, bool maintenance_canc return maintenance_cancelled && exit_status == EXIT_SUCCESS ? EXIT_FAILURE : exit_status; } +/* One override, parsed strictly: a typo must not silently disable a gate, so + * anything that is not a whole number falls back to the documented default. */ +static long cli_gate_threshold(const char *name, long fallback) { + const char *raw = getenv(name); + if (!raw || !raw[0]) { + return fallback; + } + char *end = NULL; + errno = 0; + long value = strtol(raw, &end, 10); + /* Overflow clamps to LONG_MAX with the whole string consumed, which would + * read as a ceiling no run can exceed: the one typo that disables a gate. */ + if (errno == ERANGE || end == raw || *end != '\0') { + return fallback; + } + return value; +} + +/* The contract is opt-in. Unset, empty, or "0" leaves the historical 0/1 + * status untouched, so no existing caller sees a new code until it asks. */ +static bool cli_gate_enabled(void) { + const char *raw = getenv("CBM_GATE"); + return raw && raw[0] && strcmp(raw, "0") != 0; +} + +int cbm_cli_index_exit_status(const char *result, int base_status) { + if (!result || !cli_gate_enabled()) { + return base_status; + } + yyjson_doc *envelope = yyjson_read(result, strlen(result), 0); + if (!envelope) { + return base_status; + } + yyjson_val *root = yyjson_doc_get_root(envelope); + yyjson_val *content = yyjson_is_obj(root) ? yyjson_obj_get(root, "content") : NULL; + yyjson_val *first = yyjson_is_arr(content) ? yyjson_arr_get_first(content) : NULL; + const char *text = first ? yyjson_get_str(yyjson_obj_get(first, "text")) : NULL; + + int status = base_status; + yyjson_doc *payload = text ? yyjson_read(text, strlen(text), 0) : NULL; + yyjson_val *proot = payload ? yyjson_doc_get_root(payload) : NULL; + if (yyjson_is_obj(proot)) { + const char *state = yyjson_get_str(yyjson_obj_get(proot, "status")); + const char *reason = yyjson_get_str(yyjson_obj_get(proot, "reason")); + if (state && strcmp(state, "error") == 0) { + status = (reason && strcmp(reason, "target_unavailable") == 0) + ? CBM_CLI_EXIT_TARGET + : (base_status != CBM_CLI_EXIT_OK ? base_status : CBM_CLI_EXIT_FAILURE); + } else if (base_status == CBM_CLI_EXIT_OK) { + long unusable = (long)yyjson_get_int(yyjson_obj_get(proot, "parse_unusable_count")); + long partial = (long)yyjson_get_int(yyjson_obj_get(proot, "parse_partial_count")); + long files = (long)yyjson_get_int(yyjson_obj_get(proot, "files_indexed")); + long max_unusable = cli_gate_threshold("CBM_GATE_MAX_UNUSABLE", 0); + long max_partial_pct = cli_gate_threshold("CBM_GATE_MAX_PARTIAL_PCT", 10); + /* "degraded" is the pipeline's own verdict that the graph came out + * far smaller than the run expected — a quality failure by any + * reading, so it joins the two parse thresholds. */ + bool degraded = strcmp(state ? state : "", "degraded") == 0; + bool too_many_unusable = max_unusable >= 0 && unusable > max_unusable; + /* Integer arithmetic on purpose: a percentage compared through a + * double would make the threshold depend on rounding. */ + bool too_many_partial = + max_partial_pct >= 0 && files > 0 && partial * 100 > max_partial_pct * files; + if (degraded || too_many_unusable || too_many_partial) { + status = CBM_CLI_EXIT_QUALITY; + } + } + } + if (payload) { + yyjson_doc_free(payload); + } + yyjson_doc_free(envelope); + return status; +} + /* #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 diff --git a/src/cli/cli.h b/src/cli/cli.h index 139221f0a3..702dbc95b7 100644 --- a/src/cli/cli.h +++ b/src/cli/cli.h @@ -57,6 +57,48 @@ bool cbm_cli_mcp_result_is_error(const char *result); * accepted cancellation into EXIT_FAILURE. */ int cbm_cli_exit_status_after_maintenance(int exit_status, bool maintenance_cancelled); +/* ── Index exit contract ────────────────────────────────────────── + * + * A freshness gate must not have to read anyone's account of itself: it reads + * the process status. `index_repository` therefore grades its own outcome. + * + * 0 indexed, quality at or above the thresholds + * 1 hard failure (pipeline fell over inside a reachable repository) + * 2 indexed, but below a quality threshold — a graph that exists and lies + * about how much of the tree it covers is worse than no graph + * 3 target unavailable: repo_path is absent, unreadable, or not a directory + * + * 1 keeps its historical meaning so existing `|| fail` scripts are unaffected; + * 2 and 3 are new and carve out cases that used to return 0 and 1. + * + * The contract is opt-in: it applies only when CBM_GATE is set to a non-empty + * value other than "0". Without it the CLI exits exactly as before, so a + * caller that never asked for grading never sees a new code. + * + * Thresholds, both overridable by environment: + * CBM_GATE_MAX_UNUSABLE (default 0) absolute count of unparsable files + * CBM_GATE_MAX_PARTIAL_PCT (default 10) parse_partial_count / files_indexed + * A threshold set to a negative value disables that check. + * + * The two numbers differ on purpose and must not be collapsed into one: a file + * that did not parse at all is a defect and gets no tolerance, while partial + * parsing marks constructs this grammar does not cover — a property of language + * support, not of index quality, and normal at a few percent in a large tree. + * A gate its own repository cannot pass gets switched off, which protects + * nothing. */ +enum { + CBM_CLI_EXIT_OK = 0, + CBM_CLI_EXIT_FAILURE = 1, + CBM_CLI_EXIT_QUALITY = 2, + CBM_CLI_EXIT_TARGET = 3, +}; + +/* Grade an index_repository result envelope. `base_status` is what the + * ordinary isError mapping already produced; it is preserved unless the + * payload justifies a more specific code. Unparsable payloads change + * nothing — silence is never upgraded into a verdict. */ +int cbm_cli_index_exit_status(const char *result, int base_status); + /* ── Self-update: version comparison ──────────────────────────── */ /* Compare two semver strings (e.g. "0.2.1" vs "0.2.0"). diff --git a/src/main.c b/src/main.c index 53d259b1cf..de7adbc5c5 100644 --- a/src/main.c +++ b/src/main.c @@ -1000,6 +1000,11 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo } else { exit_code = cli_print_mcp_result(result); } + /* One place for both presentations: a gate reading the status must get + * the same verdict whether or not the caller asked for --json. */ + if (tool_name && strcmp(tool_name, "index_repository") == 0) { + exit_code = cbm_cli_index_exit_status(result, exit_code); + } exit_code = cbm_cli_exit_status_after_maintenance(exit_code, maintenance_cancelled); if (cbm_index_worker_active()) { /* The supervisor protocol classifies the PROCESS, not the tool diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index c8978a79d0..888c6c9d98 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -10316,6 +10316,28 @@ static bool build_index_success_response(cbm_mcp_server_t *srv, yyjson_mut_doc * add_parse_partial_summary(doc, root, file_errors, file_error_count); add_parse_unusable_summary(doc, root, file_errors, file_error_count); } + /* Denominator for quality gates. parse_partial_count on its own cannot + * say whether sixty flagged files are a rounding error or half the + * repository; file_hashes holds exactly one row per indexed file. */ + if (store) { + cbm_file_hash_t *hashes = NULL; + int hash_count = 0; + if (cbm_store_get_file_hashes(store, project_name, &hashes, &hash_count) == CBM_STORE_OK) { + yyjson_mut_obj_add_int(doc, root, "files_indexed", hash_count); + /* The share, stated outright. A gate that only passes or fails + * cannot tell anyone HOW partial the parse was, and "the graph is + * fresh, but 6.2% of files parsed partially" is a usable hint + * where a bare verdict is not. Tenths of a percent are derived by + * integer division so the number never depends on rounding. */ + yyjson_mut_val *partial_val = yyjson_mut_obj_get(root, "parse_partial_count"); + if (partial_val && hash_count > 0) { + int partial = yyjson_mut_get_int(partial_val); + long tenths = (long)partial * 1000 / hash_count; + yyjson_mut_obj_add_real(doc, root, "parse_partial_pct", (double)tenths / 10.0); + } + cbm_store_free_file_hashes(hashes, hash_count); + } + } int nodes = 0; int edges = 0; bool degraded = false; @@ -11254,6 +11276,16 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { "previous index may have been rolled back."); } else { yyjson_mut_obj_add_str(doc, root, "status", "error"); + /* A repository that is not there at all is a different failure from a + * pipeline that fell over inside one that is — the hint below reads + * identically for both, so callers could not tell them apart. The + * split is deliberately about the ROOT: an unreadable subtree is a + * pipeline failure, not a missing target. */ + cbm_path_info_t target = {0}; + bool target_reachable = + cbm_path_info_utf8(repo_path, &target) == CBM_PATH_INFO_OK && target.is_directory; + yyjson_mut_obj_add_str(doc, root, "reason", + target_reachable ? "pipeline_failed" : "target_unavailable"); yyjson_mut_obj_add_str(doc, root, "hint", "Pipeline failed. Check repo_path exists and contains source files. " "Try mode='fast' for a quicker diagnostic run."); From b037b488be4620a5e92424e7e390ee625cd7e5af Mon Sep 17 00:00:00 2001 From: Vladislav Rahmanov Date: Mon, 7 Sep 2026 19:56:21 +0300 Subject: [PATCH 2/4] test(cli): pin the index_repository exit contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contract landed measured but unpinned: six outcomes were checked by hand against the built binary and written into the pull request, and nothing in the suite would notice if a code moved. A verification that does not run again is a claim, not a test. Twelve cases over cbm_cli_index_exit_status, which grades a result envelope into a process exit code: clean run stays 0; an unusable file and a partial share above the ceiling both grade 2; "degraded" grades 2 on the status alone; an absent target grades 3 while a pipeline failure keeps 1 — the pair that shared code 1 before and gave the whole contract its reason. Three of them exist to fail if the grader stops working rather than to pass when it does: - the ceiling itself is exercised from both sides, 10 of 100 passing and 11 failing, because a test that only checks obvious excess stays green with the check switched off entirely; - the threshold overrides carry a positive control at the defaults, so "passed after raising the ceiling" cannot be confused with "no check ran"; - eight unreadable threshold values are pinned to fall back on the documented default, since a typo that silently disables a gate is the failure the strict parse was written to prevent. Silence is pinned too: a null, empty, unreadable, or content-less envelope preserves the base status in both directions. Inventing a quality verdict out of an unparsable payload would fail runs for the crime of an unexpected response shape. Envelopes are built with yyjson rather than hand-escaped, so the payload's quoting is the transport's and not the test's own. Signed-off-by: Vladislav Rahmanov --- tests/test_cli.c | 328 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 328 insertions(+) diff --git a/tests/test_cli.c b/tests/test_cli.c index 44259a63c2..7d5a8f4454 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -14886,6 +14886,319 @@ TEST(cli_update_only_names_an_installer_that_exists_issue1632) { PASS(); } +/* ── index_repository exit contract ────────────────────────────── + * + * cbm_cli_index_exit_status grades a result envelope into a process exit + * code, because a freshness gate reads process status rather than a tool's + * account of itself. The six outcomes were measured by hand against the + * built binary when the contract landed; measured once is not pinned, and + * a code that moves silently is exactly what the contract exists to stop. */ + +/* Build the envelope the CLI actually receives: the payload travels as a + * JSON string inside content[0].text. Escaping it by hand in every test + * would put the test's own escaping on trial instead of the grader. */ +static char *cli_index_envelope(const char *payload) { + yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); + if (!doc) { + return NULL; + } + yyjson_mut_val *root = yyjson_mut_obj(doc); + yyjson_mut_doc_set_root(doc, root); + yyjson_mut_val *content = yyjson_mut_arr(doc); + yyjson_mut_val *item = yyjson_mut_obj(doc); + yyjson_mut_obj_add_str(doc, item, "text", payload); + yyjson_mut_arr_append(content, item); + yyjson_mut_obj_add_val(doc, root, "content", content); + char *json = yyjson_mut_write(doc, 0, NULL); + yyjson_mut_doc_free(doc); + return json; +} + +/* Grade one payload. Returns -1 only if the envelope could not be built, a + * value no exit code uses, so a setup failure cannot pass for a verdict. */ +static int cli_index_grade(const char *payload, int base_status) { + char *envelope = cli_index_envelope(payload); + if (!envelope) { + return -1; + } + int status = cbm_cli_index_exit_status(envelope, base_status); + free(envelope); + return status; +} + +typedef struct { + char *gate; + char *unusable; + char *partial_pct; +} cli_gate_env_t; + +/* A test that means to exercise the documented defaults must not inherit + * whatever the developer happens to have exported. The contract is opt-in, + * so the gate is switched on here; the one test about it being off unsets + * CBM_GATE itself. */ +static cli_gate_env_t cli_gate_env_clear(void) { + cli_gate_env_t saved = {save_test_env("CBM_GATE"), save_test_env("CBM_GATE_MAX_UNUSABLE"), + save_test_env("CBM_GATE_MAX_PARTIAL_PCT")}; + cbm_setenv("CBM_GATE", "1", 1); + cbm_unsetenv("CBM_GATE_MAX_UNUSABLE"); + cbm_unsetenv("CBM_GATE_MAX_PARTIAL_PCT"); + return saved; +} + +static void cli_gate_env_restore(cli_gate_env_t saved) { + restore_test_env("CBM_GATE", saved.gate); + restore_test_env("CBM_GATE_MAX_UNUSABLE", saved.unusable); + restore_test_env("CBM_GATE_MAX_PARTIAL_PCT", saved.partial_pct); +} + +/* Without CBM_GATE the CLI exits exactly as it always did: a payload that + * would grade 2 or 3 with the gate on keeps the base status. "0" and the + * empty string count as off, so an exported CBM_GATE=0 is not a surprise. */ +TEST(cli_index_exit_gate_off_keeps_base_status) { + cli_gate_env_t saved = cli_gate_env_clear(); + const char *below_quality = "{\"status\":\"ok\",\"files_indexed\":10," + "\"parse_partial_count\":5,\"parse_unusable_count\":1}"; + const char *missing_target = "{\"status\":\"error\",\"reason\":\"target_unavailable\"}"; + + /* Positive control: with the gate on both payloads DO change the status, + * so the unchanged codes below are the switch working, not a dead check. */ + int quality_on = cli_index_grade(below_quality, CBM_CLI_EXIT_OK); + int target_on = cli_index_grade(missing_target, CBM_CLI_EXIT_FAILURE); + + cbm_unsetenv("CBM_GATE"); + int quality_unset = cli_index_grade(below_quality, CBM_CLI_EXIT_OK); + int target_unset = cli_index_grade(missing_target, CBM_CLI_EXIT_FAILURE); + + cbm_setenv("CBM_GATE", "0", 1); + int quality_zero = cli_index_grade(below_quality, CBM_CLI_EXIT_OK); + + cbm_setenv("CBM_GATE", "", 1); + int quality_empty = cli_index_grade(below_quality, CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + + ASSERT_EQ(quality_on, CBM_CLI_EXIT_QUALITY); + ASSERT_EQ(target_on, CBM_CLI_EXIT_TARGET); + ASSERT_EQ(quality_unset, CBM_CLI_EXIT_OK); + ASSERT_EQ(target_unset, CBM_CLI_EXIT_FAILURE); + ASSERT_EQ(quality_zero, CBM_CLI_EXIT_OK); + ASSERT_EQ(quality_empty, CBM_CLI_EXIT_OK); + PASS(); +} + +/* A clean index keeps the code it always had: the contract adds verdicts, + * it does not make previously good runs start failing. */ +TEST(cli_index_exit_clean_run_stays_zero) { + cli_gate_env_t saved = cli_gate_env_clear(); + int status = cli_index_grade("{\"status\":\"ok\",\"files_indexed\":4," + "\"parse_partial_count\":0,\"parse_unusable_count\":0}", + CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + ASSERT_EQ(status, CBM_CLI_EXIT_OK); + PASS(); +} + +/* A file that did not parse at all gets no tolerance — the default is zero, + * so one such file is already a quality failure. */ +TEST(cli_index_exit_unusable_file_is_a_quality_failure) { + cli_gate_env_t saved = cli_gate_env_clear(); + int status = cli_index_grade("{\"status\":\"ok\",\"files_indexed\":5," + "\"parse_partial_count\":0,\"parse_unusable_count\":1}", + CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + ASSERT_EQ(status, CBM_CLI_EXIT_QUALITY); + PASS(); +} + +/* Partial parsing is graded as a share, not a count: 20 of 100 files is + * twice the default ceiling. */ +TEST(cli_index_exit_partial_above_threshold_is_a_quality_failure) { + cli_gate_env_t saved = cli_gate_env_clear(); + int status = cli_index_grade("{\"status\":\"ok\",\"files_indexed\":100," + "\"parse_partial_count\":20,\"parse_unusable_count\":0}", + CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + ASSERT_EQ(status, CBM_CLI_EXIT_QUALITY); + PASS(); +} + +/* The boundary itself passes. The comparison is strict and integral on + * purpose: at exactly the documented ceiling a double could round either + * way, and a threshold whose verdict depends on rounding is not a + * threshold. 10 of 100 is the ceiling, and the ceiling is allowed. */ +TEST(cli_index_exit_partial_exactly_at_threshold_passes) { + cli_gate_env_t saved = cli_gate_env_clear(); + int at_ceiling = cli_index_grade("{\"status\":\"ok\",\"files_indexed\":100," + "\"parse_partial_count\":10,\"parse_unusable_count\":0}", + CBM_CLI_EXIT_OK); + /* One file more is over it, which proves the case above is the boundary + * and not simply a check that never fires. */ + int over_ceiling = cli_index_grade("{\"status\":\"ok\",\"files_indexed\":100," + "\"parse_partial_count\":11,\"parse_unusable_count\":0}", + CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + ASSERT_EQ(at_ceiling, CBM_CLI_EXIT_OK); + ASSERT_EQ(over_ceiling, CBM_CLI_EXIT_QUALITY); + PASS(); +} + +/* "degraded" is the pipeline's own verdict that the graph came out far + * smaller than the run expected. It carries no parse counts, so it has to + * be graded on the status alone. */ +TEST(cli_index_exit_degraded_status_is_a_quality_failure) { + cli_gate_env_t saved = cli_gate_env_clear(); + int status = cli_index_grade("{\"status\":\"degraded\",\"files_indexed\":40," + "\"parse_partial_count\":0,\"parse_unusable_count\":0}", + CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + ASSERT_EQ(status, CBM_CLI_EXIT_QUALITY); + PASS(); +} + +/* An absent target and a pipeline that fell over inside a reachable + * repository used to share code 1, which is the whole reason the contract + * was written: the caller could not tell "wrong path" from "broken run". */ +TEST(cli_index_exit_separates_missing_target_from_broken_run) { + cli_gate_env_t saved = cli_gate_env_clear(); + int target = cli_index_grade("{\"status\":\"error\",\"reason\":\"target_unavailable\"}", + CBM_CLI_EXIT_FAILURE); + int pipeline = cli_index_grade("{\"status\":\"error\",\"reason\":\"pipeline_failed\"}", + CBM_CLI_EXIT_FAILURE); + /* An error the mapping had not already flagged still has to land on a + * failure code rather than fall through as success. */ + int unflagged = cli_index_grade("{\"status\":\"error\",\"reason\":\"pipeline_failed\"}", + CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + ASSERT_EQ(target, CBM_CLI_EXIT_TARGET); + ASSERT_EQ(pipeline, CBM_CLI_EXIT_FAILURE); + ASSERT_EQ(unflagged, CBM_CLI_EXIT_FAILURE); + PASS(); +} + +/* Silence is never upgraded into a verdict. A payload the grader cannot + * read says nothing about index quality, and inventing a 2 from it would + * fail runs for the crime of an unexpected response shape. */ +TEST(cli_index_exit_never_upgrades_silence) { + cli_gate_env_t saved = cli_gate_env_clear(); + ASSERT_EQ(cbm_cli_index_exit_status(NULL, CBM_CLI_EXIT_OK), CBM_CLI_EXIT_OK); + ASSERT_EQ(cbm_cli_index_exit_status("", CBM_CLI_EXIT_OK), CBM_CLI_EXIT_OK); + ASSERT_EQ(cbm_cli_index_exit_status("not json at all", CBM_CLI_EXIT_OK), CBM_CLI_EXIT_OK); + /* A well-formed envelope carrying no content, and one whose text is not + * itself JSON — both are shapes a future response could take. */ + ASSERT_EQ(cbm_cli_index_exit_status("{\"content\":[]}", CBM_CLI_EXIT_OK), CBM_CLI_EXIT_OK); + ASSERT_EQ(cli_index_grade("plain text, not a payload", CBM_CLI_EXIT_OK), CBM_CLI_EXIT_OK); + /* And the same shapes must not erase a failure already established. */ + ASSERT_EQ(cbm_cli_index_exit_status(NULL, CBM_CLI_EXIT_FAILURE), CBM_CLI_EXIT_FAILURE); + cli_gate_env_restore(saved); + PASS(); +} + +/* Grading only ever makes a code more specific. A base failure survives a + * payload that looks perfectly healthy, because the transport already knew + * something the payload does not say. */ +TEST(cli_index_exit_does_not_downgrade_a_failing_base) { + cli_gate_env_t saved = cli_gate_env_clear(); + int status = cli_index_grade("{\"status\":\"ok\",\"files_indexed\":9," + "\"parse_partial_count\":0,\"parse_unusable_count\":0}", + CBM_CLI_EXIT_FAILURE); + cli_gate_env_restore(saved); + ASSERT_EQ(status, CBM_CLI_EXIT_FAILURE); + PASS(); +} + +/* Both thresholds are overridable, and both are read at grading time. */ +TEST(cli_index_exit_thresholds_read_the_environment) { + cli_gate_env_t saved = cli_gate_env_clear(); + const char *twenty_percent = "{\"status\":\"ok\",\"files_indexed\":100," + "\"parse_partial_count\":20,\"parse_unusable_count\":0}"; + const char *three_unusable = "{\"status\":\"ok\",\"files_indexed\":50," + "\"parse_partial_count\":0,\"parse_unusable_count\":3}"; + + /* Positive control: at the defaults both payloads fail, so a pass below + * is the override working and not the check being absent. */ + int partial_default = cli_index_grade(twenty_percent, CBM_CLI_EXIT_OK); + int unusable_default = cli_index_grade(three_unusable, CBM_CLI_EXIT_OK); + + cbm_setenv("CBM_GATE_MAX_PARTIAL_PCT", "50", 1); + cbm_setenv("CBM_GATE_MAX_UNUSABLE", "5", 1); + int partial_raised = cli_index_grade(twenty_percent, CBM_CLI_EXIT_OK); + int unusable_raised = cli_index_grade(three_unusable, CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + + ASSERT_EQ(partial_default, CBM_CLI_EXIT_QUALITY); + ASSERT_EQ(unusable_default, CBM_CLI_EXIT_QUALITY); + ASSERT_EQ(partial_raised, CBM_CLI_EXIT_OK); + ASSERT_EQ(unusable_raised, CBM_CLI_EXIT_OK); + PASS(); +} + +/* A negative ceiling switches its own check off. This is the documented + * escape hatch for a repository whose grammars are known to be thin. */ +TEST(cli_index_exit_negative_threshold_disables_the_check) { + cli_gate_env_t saved = cli_gate_env_clear(); + cbm_setenv("CBM_GATE_MAX_PARTIAL_PCT", "-1", 1); + cbm_setenv("CBM_GATE_MAX_UNUSABLE", "-1", 1); + int status = cli_index_grade("{\"status\":\"ok\",\"files_indexed\":100," + "\"parse_partial_count\":99,\"parse_unusable_count\":7}", + CBM_CLI_EXIT_OK); + /* Disabling the parse checks must not disable the pipeline's own + * verdict: "degraded" is not a threshold and has no off switch. */ + int degraded = cli_index_grade("{\"status\":\"degraded\",\"files_indexed\":10," + "\"parse_partial_count\":0,\"parse_unusable_count\":0}", + CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + ASSERT_EQ(status, CBM_CLI_EXIT_OK); + ASSERT_EQ(degraded, CBM_CLI_EXIT_QUALITY); + PASS(); +} + +/* A typo must not silently disable a gate. Anything that is not a whole + * number falls back to the documented default, so "1O" (letter O) fails the + * run it would have failed anyway instead of quietly waving it through. */ +TEST(cli_index_exit_unreadable_threshold_falls_back_to_default) { + cli_gate_env_t saved = cli_gate_env_clear(); + const char *twenty_percent = "{\"status\":\"ok\",\"files_indexed\":100," + "\"parse_partial_count\":20,\"parse_unusable_count\":0}"; + /* The last one overflows long: strtol clamps it to LONG_MAX with the + * whole string consumed, a ceiling no run could exceed. */ + const char *unreadable[] = { + "abc", "1O", "10pct", " 10", "10 ", "", "1e1", "10.0", "99999999999999999999999"}; + for (size_t i = 0; i < sizeof(unreadable) / sizeof(unreadable[0]); i++) { + cbm_setenv("CBM_GATE_MAX_PARTIAL_PCT", unreadable[i], 1); + int status = cli_index_grade(twenty_percent, CBM_CLI_EXIT_OK); + if (status != CBM_CLI_EXIT_QUALITY) { + printf(" unreadable threshold \"%s\" gave exit %d\n", unreadable[i], status); + } + ASSERT_EQ(status, CBM_CLI_EXIT_QUALITY); + } + /* A value that DOES read still takes effect, so the loop above is about + * unreadable text and not about the override being ignored outright. */ + cbm_setenv("CBM_GATE_MAX_PARTIAL_PCT", "50", 1); + int readable = cli_index_grade(twenty_percent, CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + ASSERT_EQ(readable, CBM_CLI_EXIT_OK); + PASS(); +} + +/* A share needs a denominator. Without files_indexed the percentage cannot + * be computed at all, and a run must not be failed on a number nobody + * could work out. */ +TEST(cli_index_exit_partial_without_denominator_is_not_graded) { + cli_gate_env_t saved = cli_gate_env_clear(); + int no_denominator = cli_index_grade("{\"status\":\"ok\",\"parse_partial_count\":20}", + CBM_CLI_EXIT_OK); + int zero_denominator = cli_index_grade("{\"status\":\"ok\",\"files_indexed\":0," + "\"parse_partial_count\":20}", + CBM_CLI_EXIT_OK); + /* An unusable file is an absolute count and still grades without one. */ + int unusable_still_graded = cli_index_grade("{\"status\":\"ok\",\"parse_unusable_count\":1}", + CBM_CLI_EXIT_OK); + cli_gate_env_restore(saved); + ASSERT_EQ(no_denominator, CBM_CLI_EXIT_OK); + ASSERT_EQ(zero_denominator, CBM_CLI_EXIT_OK); + ASSERT_EQ(unusable_still_graded, CBM_CLI_EXIT_QUALITY); + PASS(); +} + SUITE(cli) { if (!th_secure_runtime_parent_new(g_cli_suite_runtime_parent, sizeof(g_cli_suite_runtime_parent), "cli-suite")) { @@ -15304,6 +15617,21 @@ SUITE(cli) { /* Stdin argument gate (#1359) */ RUN_TEST(cli_zero_argument_tool_never_reads_stdin_issue1359); RUN_TEST(cli_stdin_args_gate_tracks_tool_schema_issue1359); + + /* index_repository exit contract */ + RUN_TEST(cli_index_exit_gate_off_keeps_base_status); + RUN_TEST(cli_index_exit_clean_run_stays_zero); + RUN_TEST(cli_index_exit_unusable_file_is_a_quality_failure); + RUN_TEST(cli_index_exit_partial_above_threshold_is_a_quality_failure); + RUN_TEST(cli_index_exit_partial_exactly_at_threshold_passes); + RUN_TEST(cli_index_exit_degraded_status_is_a_quality_failure); + RUN_TEST(cli_index_exit_separates_missing_target_from_broken_run); + RUN_TEST(cli_index_exit_never_upgrades_silence); + RUN_TEST(cli_index_exit_does_not_downgrade_a_failing_base); + RUN_TEST(cli_index_exit_thresholds_read_the_environment); + RUN_TEST(cli_index_exit_negative_threshold_disables_the_check); + RUN_TEST(cli_index_exit_unreadable_threshold_falls_back_to_default); + RUN_TEST(cli_index_exit_partial_without_denominator_is_not_graded); cbm_cli_set_activation_runtime_parent_for_test(NULL); test_rmdir_r(g_cli_suite_runtime_parent); g_cli_suite_runtime_parent[0] = '\0'; From 864484d3d80255d2fed801e64ee791c872a9adba Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 20 Oct 2026 19:28:10 +0200 Subject: [PATCH 3/4] fix(cli): move index verdict exit codes off the usage-error code The binary already exits 2 for a usage error (an invalid --tool-profile), and shells and CI tooling read 2 that way. With the quality verdict on 2 a gate could not tell "the index is below threshold" from "the command line was wrong". The two verdicts move to codes nothing else uses: 0 indexed, at or above the thresholds (unchanged) 1 hard failure in a reachable repository (unchanged) 10 indexed, below a quality threshold (was 2) 11 target unavailable (was 3) The header now states that these values are a stable contract, and a new test pins the four numbers so a renumbering fails in the suite rather than in a caller's pipeline. Requested in the maintainer review of #2092. Co-authored-by: Vladislav Rahmanov Signed-off-by: Martin Vogel --- src/cli/cli.h | 22 ++++++++++++++-------- tests/test_cli.c | 17 +++++++++++++++-- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/cli/cli.h b/src/cli/cli.h index cc96c591e8..bdd0c156c5 100644 --- a/src/cli/cli.h +++ b/src/cli/cli.h @@ -87,14 +87,20 @@ int cbm_cli_exit_status_after_maintenance(int exit_status, bool maintenance_canc * A freshness gate must not have to read anyone's account of itself: it reads * the process status. `index_repository` therefore grades its own outcome. * - * 0 indexed, quality at or above the thresholds - * 1 hard failure (pipeline fell over inside a reachable repository) - * 2 indexed, but below a quality threshold — a graph that exists and lies - * about how much of the tree it covers is worse than no graph - * 3 target unavailable: repo_path is absent, unreadable, or not a directory + * 0 indexed, quality at or above the thresholds + * 1 hard failure (pipeline fell over inside a reachable repository) + * 10 indexed, but below a quality threshold — a graph that exists and lies + * about how much of the tree it covers is worse than no graph + * 11 target unavailable: repo_path is absent, unreadable, or not a directory * * 1 keeps its historical meaning so existing `|| fail` scripts are unaffected; - * 2 and 3 are new and carve out cases that used to return 0 and 1. + * 10 and 11 are new and carve out cases that used to return 0 and 1. + * + * STABLE CONTRACT: these four values are public. CI pipelines branch on them, + * so they are never renumbered or reused for another outcome. 2 is + * deliberately not a verdict: the binary already returns 2 for a usage error + * (an invalid --tool-profile), and shells and CI tooling read 2 that way, so a + * verdict on 2 could not be told apart from a mistyped command line. * * The contract is opt-in: it applies only when CBM_GATE is set to a non-empty * value other than "0". Without it the CLI exits exactly as before, so a @@ -114,8 +120,8 @@ int cbm_cli_exit_status_after_maintenance(int exit_status, bool maintenance_canc enum { CBM_CLI_EXIT_OK = 0, CBM_CLI_EXIT_FAILURE = 1, - CBM_CLI_EXIT_QUALITY = 2, - CBM_CLI_EXIT_TARGET = 3, + CBM_CLI_EXIT_QUALITY = 10, + CBM_CLI_EXIT_TARGET = 11, }; /* Grade an index_repository result envelope. `base_status` is what the diff --git a/tests/test_cli.c b/tests/test_cli.c index 3f2d506c7d..54dc5ed29b 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -18263,8 +18263,20 @@ static void cli_gate_env_restore(cli_gate_env_t saved) { restore_test_env("CBM_GATE_MAX_PARTIAL_PCT", saved.partial_pct); } +/* The verdict codes are a public contract: CI pipelines branch on the + * numbers, so a renumbering must fail here rather than in someone's + * pipeline. Neither verdict may sit on 2, which the binary already returns + * for a usage error and which shells and CI tooling read that way. */ +TEST(cli_index_exit_codes_are_a_stable_contract) { + ASSERT_EQ(CBM_CLI_EXIT_OK, 0); + ASSERT_EQ(CBM_CLI_EXIT_FAILURE, 1); + ASSERT_EQ(CBM_CLI_EXIT_QUALITY, 10); + ASSERT_EQ(CBM_CLI_EXIT_TARGET, 11); + PASS(); +} + /* Without CBM_GATE the CLI exits exactly as it always did: a payload that - * would grade 2 or 3 with the gate on keeps the base status. "0" and the + * would grade 10 or 11 with the gate on keeps the base status. "0" and the * empty string count as off, so an exported CBM_GATE=0 is not a surprise. */ TEST(cli_index_exit_gate_off_keeps_base_status) { cli_gate_env_t saved = cli_gate_env_clear(); @@ -18387,7 +18399,7 @@ TEST(cli_index_exit_separates_missing_target_from_broken_run) { } /* Silence is never upgraded into a verdict. A payload the grader cannot - * read says nothing about index quality, and inventing a 2 from it would + * read says nothing about index quality, and inventing a 10 from it would * fail runs for the crime of an unexpected response shape. */ TEST(cli_index_exit_never_upgrades_silence) { cli_gate_env_t saved = cli_gate_env_clear(); @@ -18989,6 +19001,7 @@ SUITE(cli) { RUN_TEST(cli_stdin_args_gate_tracks_tool_schema_issue1359); /* index_repository exit contract */ + RUN_TEST(cli_index_exit_codes_are_a_stable_contract); RUN_TEST(cli_index_exit_gate_off_keeps_base_status); RUN_TEST(cli_index_exit_clean_run_stays_zero); RUN_TEST(cli_index_exit_unusable_file_is_a_quality_failure); From cb62f63739e934706a66422c7e14a4901af9bc1e Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 20 Oct 2026 19:28:10 +0200 Subject: [PATCH 4/4] fix(cli): read gate thresholds through cbm_env_long cli_gate_threshold re-implemented the project's strict whole-number environment reader and missed one case: strtol skips leading blanks, so CBM_GATE_MAX_PARTIAL_PCT=" 20" was read as 20, although the test lists a leading blank as unreadable. cbm_env_long (src/foundation/platform.h) refuses empty text, leading blanks, trailing junk and out-of-range values, leaves its output untouched on refusal, and reads the Windows environment the same way every other setting does. Anything unreadable now falls back to the documented default. The test's numeric-looking unreadable values move from 10 to 50. At 10 they equalled the default, so a reader that wrongly accepted one still failed the 20% payload and the test could not tell the difference. At 50 an accepted value passes the run and the test fails; against the previous reader it does, on " 50". Requested in the maintainer review of #2092. Co-authored-by: Vladislav Rahmanov Signed-off-by: Martin Vogel --- src/cli/cli.c | 21 +++++++-------------- tests/test_cli.c | 10 +++++++--- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index cbbdec0684..9a1dd25b93 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -160,21 +160,14 @@ int cbm_cli_exit_status_after_maintenance(int exit_status, bool maintenance_canc } /* One override, parsed strictly: a typo must not silently disable a gate, so - * anything that is not a whole number falls back to the documented default. */ + * anything that is not a whole number falls back to the documented default. + * cbm_env_long is the project's strict reader for exactly this case: it + * refuses empty text, blanks, trailing junk and values too large for a long + * (strtol would clamp those to LONG_MAX, a ceiling no run can exceed), and it + * reads the Windows environment the same way every other setting does. */ static long cli_gate_threshold(const char *name, long fallback) { - const char *raw = getenv(name); - if (!raw || !raw[0]) { - return fallback; - } - char *end = NULL; - errno = 0; - long value = strtol(raw, &end, 10); - /* Overflow clamps to LONG_MAX with the whole string consumed, which would - * read as a ceiling no run can exceed: the one typo that disables a gate. */ - if (errno == ERANGE || end == raw || *end != '\0') { - return fallback; - } - return value; + long value = 0; + return cbm_env_long(name, &value) ? value : fallback; } /* The contract is opt-in. Unset, empty, or "0" leaves the historical 0/1 diff --git a/tests/test_cli.c b/tests/test_cli.c index 54dc5ed29b..ec898e99d1 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -18482,10 +18482,14 @@ TEST(cli_index_exit_unreadable_threshold_falls_back_to_default) { cli_gate_env_t saved = cli_gate_env_clear(); const char *twenty_percent = "{\"status\":\"ok\",\"files_indexed\":100," "\"parse_partial_count\":20,\"parse_unusable_count\":0}"; - /* The last one overflows long: strtol clamps it to LONG_MAX with the - * whole string consumed, a ceiling no run could exceed. */ + /* The numeric-looking values carry 50, not the default 10: a reader that + * wrongly accepted one would raise the ceiling above the 20% payload and + * pass the run, so every refusal is visible. " 10" could not show that, + * and plain strtol does accept a leading blank. The last one overflows + * long: strtol clamps it to LONG_MAX with the whole string consumed, a + * ceiling no run could exceed. */ const char *unreadable[] = { - "abc", "1O", "10pct", " 10", "10 ", "", "1e1", "10.0", "99999999999999999999999"}; + "abc", "1O", "50pct", " 50", "50 ", "", "5e1", "50.0", "99999999999999999999999"}; for (size_t i = 0; i < sizeof(unreadable) / sizeof(unreadable[0]); i++) { cbm_setenv("CBM_GATE_MAX_PARTIAL_PCT", unreadable[i], 1); int status = cli_index_grade(twenty_percent, CBM_CLI_EXIT_OK);