Skip to content

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

Open
rarepops wants to merge 2 commits into
DeusData:mainfrom
rarepops:fix/autoindex-skip-limit-value-1466
Open

fix(mcp): log the numeric auto_index_limit in autoindex.skip (#1466)#1779
rarepops wants to merge 2 commits into
DeusData:mainfrom
rarepops:fix/autoindex-skip-limit-value-1466

Conversation

@rarepops

Copy link
Copy Markdown
Contributor

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.

The too_many_files warning passed the CBM_CONFIG_AUTO_INDEX_LIMIT key
constant as the value of the `limit` field, so the warning read
`limit=auto_index_limit` instead of the configured number. Format the
effective file_limit into a buffer the same way the neighbouring `files`
count already is.

Fixes DeusData#1466

Signed-off-by: Rares Popa <2606875+rarepops@users.noreply.github.com>
@rarepops
rarepops requested a review from DeusData as a code owner August 21, 2026 07:40
@github-actions

Copy link
Copy Markdown

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.

@rarepops rarepops changed the title fix(mcp): log the numeric auto_index_limit in autoindex.skip fix(mcp): log the numeric auto_index_limit in autoindex.skip (#1466) Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

autoindex.skip logs the config key instead of the numeric auto_index_limit

1 participant