fix(mcp): log the numeric auto_index_limit in autoindex.skip (#1466)

[rarepops]

Fixes #1466

Problem

When auto-indexing skips a repository because its tracked-file count exceeds auto_index_limit, the warning reports the name of the config key instead of the configured value:

level=warn msg=autoindex.skip reason=too_many_files files=2 limit=auto_index_limit

CBM_CONFIG_AUTO_INDEX_LIMIT is #defined as the string "auto_index_limit" in src/cli/cli.h, and it was being passed straight through as the value of the limit field:

cbm_log_warn("autoindex.skip", "reason",
             file_count >= 0 ? "too_many_files" : "unsafe_or_unavailable_path", "files",
             files, "limit", CBM_CONFIG_AUTO_INDEX_LIMIT);

The effective numeric limit was already in scope as file_limit, read a few lines earlier via cbm_config_get_int().

As the reporter noted, the malformed files field from v0.9.0 is already fixed on main; only the limit field remained.

Fix

Format file_limit into a buffer, mirroring exactly how the neighbouring files count is already handled:

char files[32];
char limit[32];
(void)snprintf(files, sizeof(files), "%d", file_count);
(void)snprintf(limit, sizeof(limit), "%d", file_limit);
cbm_log_warn("autoindex.skip", "reason",
             file_count >= 0 ? "too_many_files" : "unsafe_or_unavailable_path", "files",
             files, "limit", limit);

Output now matches the issue's expectation:

level=warn msg=autoindex.skip reason=too_many_files files=2 limit=1

Test

autoindex_skip_reports_numeric_limit_issue1466 in tests/test_mcp.c drives the real public entry point (initialize -> maybe_auto_index) against a fresh project holding more files than auto_index_limit, and captures the emitted warning through a log sink.

Reproduce-first, verified in both directions on the same tree:

	before the source fix	after
autoindex_skip_reports_numeric_limit_issue1466	FAIL tests/test_mcp.c:10637: strstr(warning, "limit=1") is NULL	PASS
mcp suite	203 passed, 1 failed, 6 skipped	204 passed, 0 failed, 6 skipped

The failure is not vacuous: the msg=autoindex.skip, reason=too_many_files and files=2 assertions all pass in the RED run, so the skip path is genuinely exercised and the log line is genuinely captured. Only the limit=1 assertion flips. The test also asserts the absence of limit=auto_index_limit, so a future regression that drops the value back to the key name fails loudly.

Verification

Run in a container mirroring the CI toolchain (Ubuntu 24.04 + gcc, ASan/UBSan build):

• mcp suite: 204 passed, 0 failed
• log, daemon, daemon_runtime, daemon_application (the suites covering the logging layer and the sibling auto-index admission path): 120 passed, 0 failed
• lint-format with clang-format-20: clean
• lint-cppcheck with cppcheck 2.20.0: clean

lint-no-suppress and lint-mem-ci are structurally unaffected: the diff adds no NOLINT, the only LINT_SRCS file touched is src/mcp/mcp.c (two stack buffers, no allocation), and no entry in scripts/lint-mem-whitelist.txt is pinned to maybe_auto_index, so no sha256 pin is invalidated.

Out of scope (possible follow-up)

While tracing this I noticed the daemon has a sibling warning at src/daemon/application.c that emits reason and files for the same condition but carries no limit key at all:

cbm_log_warn("daemon.autoindex.skipped", "project", project, "reason",
             tracked_files >= 0 ? "too_many_files" : "unsafe_or_unavailable_path", "files",
             files);

That is a different defect from #1466 (a missing field rather than a wrong one) and it is not what the issue reports, so I left it alone to keep this PR to one issue. Happy to open a separate issue or PR for it if you would like the two skip warnings to carry the same fields.

[github-actions]

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]

Thank you for keeping the sibling daemon warning out of this focused fix. I checked current main and your distinction is correct: the MCP warning emits the configuration key name where the value should be, while daemon.autoindex.skipped omits the limit field entirely.

Please open a separate focused issue for the daemon-log parity gap and link it here. This PR should remain limited to #1466. The current fix and binding log-sink test are grounded and have been labeled for review. Our queue is full, so detailed review may take a little time. Thank you for calling out the adjacent defect without bundling it.

[rarepops]

Opened #1818 as requested and kept it scoped to the daemon-log parity gap.

The focused daemon_application suite reproduces the current warning without a limit field, while all 49 tests pass. This remains separate from the MCP-side fix in this PR.

[DeusData]

Approved on merit — I have rebased it onto main and once CI comes back green I will merge.

I confirmed the defect is still live: src/mcp/mcp.c:12872 still passes CBM_CONFIG_AUTO_INDEX_LIMIT — the #defined key name — as the limit field's value, while file_limit sits right there holding the number. Your fix mirrors the neighbouring files handling exactly, which is the right shape: it makes the two adjacent fields obviously consistent rather than introducing a second convention.

Two things I want to name, because they are why this took one pass rather than three:

The reproduce-first evidence is stated in both directions on the same tree — RED at strstr(warning, "limit=1") is NULL, GREEN after, with the suite going 203/1 → 204/0. That is the form that actually settles whether a test earns its place.

And you pre-empted the vacuity question. Noting that msg=autoindex.skip, reason=too_many_files and files=2 all pass in the RED run proves the skip path really executes and the sink really captures — so the one failing assertion is the only thing under test. A reviewer would otherwise have to derive that, and most PRs leave them to.

Filtering the sink to keep only the too_many_files line so later output cannot displace it is a small thing that would have caused an intermittent failure otherwise.

One note on process, not on your work: this sat since 21 August and had drifted 205 commits behind main, so its green had stopped meaning anything. I have updated the branch rather than asking you to — the delay was ours.

Thank you. This is the second careful, well-evidenced PR of yours I have read today.

[DeusData]

Merged as b44e5849. Thank you — and sorry it took so long to get there; the delay was entirely ours, between the review backlog and an Actions queue that has been servicing about one job at a time all day.

Worth restating what made this quick to review once I did look:

The reproduce-first evidence was stated in both directions on the same tree — RED at strstr(warning, "limit=1") is NULL, GREEN after, with the suite moving 203/1 → 204/0.

And you pre-empted the vacuity question. Noting that msg=autoindex.skip, reason=too_many_files and files=2 all pass in the RED run proves the skip path really executes and the sink really captures it, so the single failing assertion is the only thing under test. Most PRs leave a reviewer to derive that.

Filtering the log sink to keep only the too_many_files line, so later output cannot displace it, is the small detail that would otherwise have made this test intermittently red.

I have commented the fixing SHA on #1466 for the record. Your #1773 is still open with the delta I handed back, and #1799 and #1802 are both approved and queued for merge behind this one.