feat(cli): opt-in exit contract for index_repository by Blysspeak · Pull Request #2092 · DeusData/codebase-memory-mcp

Blysspeak · 2026-09-07T16:05:25Z

Opt-in exit contract for `cli index_repository`

A freshness gate cannot read a tool's account of itself, it reads the process
status. Today index_repository returns 0 for an index that parsed part of
the tree, and the same 1 whether the repository is missing or a pipeline fell
over inside one that exists. Callers cannot tell those apart.

Nothing changes unless CBM_GATE=1 is set. Without it the CLI exits
exactly as before. With it, grading happens in the CLI only; MCP responses are
untouched, so agents and scripts/soak-test.sh, which judge by JSON-RPC and
tool-level isError, see exactly what they saw before.

outcome	today	with `CBM_GATE=1`
indexed, clean	0	0
indexed, files parsed partially above threshold	0	2
indexed, a file did not parse at all	0	2
indexed, node count far below expectation (`status: degraded`)	0	2
pipeline failed inside a reachable repository	1	1
`repo_path` absent, unreadable, or not a directory	1	3

Thresholds, both optional, negative disables the check:

CBM_GATE_MAX_UNUSABLE, default 0, absolute count of unparsable files
CBM_GATE_MAX_PARTIAL_PCT, default 10, share of partially parsed files

The two defaults differ deliberately. A file that did not parse at all is a
defect and gets no tolerance. Partial parsing marks constructs a grammar does
not cover, a property of language support rather than of index quality: this
repository itself sits at 5.8% with a fully working graph. A gate its own
project cannot pass gets switched off, which protects nothing.

Additive fields

files_indexed and parse_partial_pct on the index response. The ratio had
no denominator, and a gate that can only pass or fail cannot say how
partial a parse was.
reason on failures, target_unavailable or pipeline_failed. The hint
text read identically for both.

Verification

Full suite on this branch: 8001 passed, 1 failed, 7 skipped. The one failure is
daemon_ipc_posix_single_uid_userns_real_smoke_issue1830, which fails the same
way on a clean build of main at 055fbb7 on this machine (user-namespace
setup), so it is unrelated to this change.

13 tests in the cli suite pin the contract, including that an unset, empty
or 0 CBM_GATE leaves the historical status untouched.

End to end on this repository (clean git worktree of this branch, isolated
CBM_CACHE_DIR and CBM_RUNTIME_DIR), exit status read without a pipeline so
$? is the command's own:

gate unset:
  missing path                                   exit 1   (unchanged)
  this repository                                exit 0   (unchanged)
CBM_GATE=1:
  missing path                                   exit 3   reason target_unavailable
  this repository                                exit 0   files_indexed 1065, parse_partial_pct 5.8, under the default 10
CBM_GATE=1 CBM_GATE_MAX_PARTIAL_PCT=5:
  this repository                                exit 2

The same sequence against a binary built from main at 055fbb7 gives
1 / 0 / 1 / 0 / 0: today's status, unchanged by the variable.

github-actions · 2026-09-07T16:08:23Z

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

It will not be closed for inactivity. No stale bot touches pull requests here.
It may still sit a while before a human reads it. That is on us, not on you.
Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
Get CI green, or say which failures you believe are pre-existing.
Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

DeusData · 2026-09-09T21:53:25Z

Thank you for the detailed CLI proposal and its tests. This spans exit-status semantics, environment defaults, output fields and index handling, so we need more time to review the compatibility and scope decisions before giving a direction. Please do not read the triage labels as approval of new defaults.

The review queue is currently full, so detailed feedback may take a little time. We are working through it carefully and appreciate the work you have put into supporting the project. Thank you for your patience.

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 <rahmanov.official@yandex.ru>

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 <rahmanov.official@yandex.ru>

Blysspeak · 2026-09-10T16:04:20Z

Reworked after your note on defaults, and narrowed to one claim per the contributing guide.

The contract is now opt-in: nothing changes unless CBM_GATE=1 is set, so no existing caller sees a new code. graph_generation, seed_symbols_list, absolute_path and .cbmignore are out of this PR; if any of them is wanted I will open an issue for it separately.

Rebased on main, clang-format fixed, tracking issue #2168 with measurements on a clean main build and on this branch.

DeusData · 2026-09-19T20:26:41Z

Thank you for narrowing the change to the opt-in CBM_GATE path and removing the extra features. The remaining exit-status/result contract is still a maintainer design decision. We need more time to review that boundary before accepting it; another speculative rebase is not being requested.

DeusData · 2026-09-25T20:26:08Z

Thank you, @Blysspeak, for narrowing this to the opt-in path. We've made the decision: the opt-in CBM_GATE exit contract is accepted. Gating CI on a defined failure class is a real need, and keeping it opt-in means nobody's existing scripts change behaviour.

The next steps are ours. We'll review the exit-code table for long-term stability, since once CI pipelines depend on those codes we can't renumber them. After that, it merges on a green run against current main. We'll come back with one review if anything needs adjusting.

DeusData

Here is the exit-code review we promised, @Blysspeak. The contract is clean and well tested. The "gate off keeps the base status" test and the refusal to let a typo silently widen a threshold are exactly right. There are two small things, and both matter because these codes cannot be renumbered once CI pipelines depend on them:

Please don't use exit code 2 for a verdict. main.c already returns 2 for a usage error (an invalid --tool-profile), and shells and CI tooling conventionally read 2 as "usage error". With QUALITY = 2, a gate cannot tell "the index is below threshold" apart from "the command was invoked wrongly". Could you move the two verdict codes to values nothing else uses, for example QUALITY = 10 and TARGET = 11, and keep 0 and 1 as they are? Please also state in the header comment that these values are a stable contract.
Use the shared environment reader. cli_gate_threshold re-implements cbm_env_long (src/foundation/platform.h). That is the project's strict whole-number reader, written for exactly this "a typo must not silently disable a setting" case, and it handles the Windows environment consistently. cbm_env_long(name, &value) ? value : fallback keeps your semantics.

Everything else can stay as it is. Once these two are in and CI is green against current main, this merges. Thank you.

DeusData · 2026-10-19T22:48:46Z

Thanks again for narrowing the opt-in gate. The last review asks for verdict codes that do not collide with usage errors and reuse of the shared strict environment reader. Those two requests remain the next step; the accepted opt-in direction is already recorded above.

Brings the opt-in index exit contract up to date with main a222715. Two conflicts, both additive on each side: - src/mcp/mcp.c, the error branch of handle_index_repository: main's DeusData#1665 export-error hint and this branch's "reason" field are independent. The reason (target_unavailable vs pipeline_failed) is computed first; main's hint selection follows unchanged. - tests/test_cli.c: main's DeusData#2200 update test and this branch's exit contract tests were both appended after the same test; both are kept. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>

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 DeusData#2092. Co-authored-by: Vladislav Rahmanov <rahmanov.official@yandex.ru> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>

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 DeusData#2092. Co-authored-by: Vladislav Rahmanov <rahmanov.official@yandex.ru> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>

DeusData · 2026-10-20T17:28:23Z

Thank you, @Blysspeak, for this contract and for the care that went into it. The opt-in switch, the positive control in "gate off keeps the base status", the boundary test at exactly 10 of 100, and the refusal to let a typo silently widen a threshold are exactly what a CI-facing contract needs, and they made this an easy review.

Because this branch allows maintainer edits, we made the two adjustments from the September 25 review ourselves instead of asking you for another round. Your two commits are untouched; three commits sit on top of them:

Merge of current main. The branch was far behind and conflicted in two places, both additive. In handle_index_repository, your reason field is now computed just before the export-error hint that main added in the meantime (Read-only repository cannot be indexed on 0.10.5 (0.9.0 succeeds), and the error blames repo_path #1665), and that hint logic is unchanged. In tests/test_cli.c, your tests and a newer update test had both been appended after the same test, so both are kept.
fix(cli): move index verdict exit codes off the usage-error code. The quality verdict moves from 2 to 10 and the target-unavailable verdict from 3 to 11; 0 and 1 are unchanged. The binary already exits 2 for a usage error (an invalid --tool-profile), and shells and CI tooling read 2 that way, so a gate could not have told "below threshold" apart from "invoked wrongly". The header comment now states that these values are a stable contract, and a new test pins all four numbers, so a renumbering fails in the suite instead of in someone's pipeline.
fix(cli): read gate thresholds through cbm_env_long. cli_gate_threshold now delegates to the project's shared strict reader, which refuses empty text, blanks, trailing junk and overflow, and reads the Windows environment the same way every other setting does. While wiring it in we found one case your list already intended to refuse: strtol skips leading blanks, so " 20" was read as 20. Your test could not see that, because its value " 10" equals the default 10. We moved the numeric-looking entries in that list from 10 to 50 (" 50", "50 ", "50pct", "5e1", "50.0"), so a reader that wrongly accepted one would pass the 20% payload and the test would fail. Against the previous reader it does fail on " 50"; with the shared reader all nine cases pass.

The table in the description above predates this change: with it, the two verdicts are 10 and 11.

Once CI is green on the new head, this merges, as agreed in September. Thank you again for your patience through a long queue, and for narrowing the change so cleanly.

DeusData · 2026-10-25T17:30:21Z

Thank you again for the opt-in exit contract. The October updates record the verdict-code and shared environment-reader adjustments as completed on our side, so no repeat contributor work is requested. Sorry for the delay in the remaining maintainer follow-up—I’ll follow up soon.

Brings the opt-in index exit contract up to date with main 0433142. The merge is clean; no file this branch changes needed a resolution. The previous base (fceb25d) predates the shared CI baseline (72c3604), whose language-count contract repair is why every test leg failed at Step 0w on the last run. That failure was in the base, not in this branch. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>

DeusData · 2026-10-26T00:42:28Z

Thank you again, @Blysspeak, for the opt-in exit contract and for your patience while we followed up. Here is that follow-up.

The last CI run failed on every test leg at the same step, the published language-count contract. That check was repaired on main after this branch was last updated, so the failure came from the old base and not from your change.

We pushed one commit on top of the branch: a merge of current main (aaee484). It merged without conflicts, and this PR's diff against main is unchanged: the same five files and the same 498 added lines. Your two commits and the two adjustments described in the October 20 comment are untouched.

Before merging we checked the contract again on the previous head. The cli suite passes. With the two adjustments reverted, exactly the two tests that pin them fail: the one that pins the four exit codes (0, 1, 10, 11) and the unreadable-threshold case " 50". So both adjustments stay covered by the suite.

If you push to this branch again, please pull first, since it now includes this merge. Once CI is green on the new head, this merges as agreed. Thank you again for the care you put into this contract.