Skip to content

fix(mcp): preserve coverage scope labels#1148

Open
aaiyer wants to merge 1 commit into
DeusData:mainfrom
aaiyer:fix/coverage-scope-lifetime
Open

fix(mcp): preserve coverage scope labels#1148
aaiyer wants to merge 1 commit into
DeusData:mainfrom
aaiyer:fix/coverage-scope-lifetime

Conversation

@aaiyer

@aaiyer aaiyer commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

check_index_coverage stored each normalized scope with yyjson's non-copying
string API even though the source was a loop-local buffer. With multiple
scopes, later iterations could overwrite earlier labels; AddressSanitizer also
reported a stack-use-after-scope when the response was serialized.

Use yyjson_mut_obj_add_strcpy so the mutable document owns each scope label.
Add a regression that requests three distinct, equal-length scopes and asserts
that every returned label and its order are preserved.

This PR is intentionally limited to one issue and two files:

  • src/mcp/mcp.c
  • tests/test_mcp.c

It does not change coverage semantics, tool inputs, schemas, pagination, or
status handling.

Verification

  • Reproduce-first: the focused MCP sanitizer suite aborted before the fix with
    stack-use-after-scope in handle_check_index_coverage during yyjson
    serialization.
  • make -j4 -f Makefile.cbm test-focused TEST_SUITES=mcp: 166/166 passed.
  • scripts/lint.sh --ci with CI's clang-format 20.1.8 and cppcheck 2.20.0:
    passed.
  • make -j"$(nproc)" -f Makefile.cbm security: all seven security layers
    passed, including MCP robustness 23/23.
  • Focused review: 0 High, 0 Medium, 0 Low findings.

The broad scripts/test.sh run reached the end with all test cases reporting
PASS, then LeakSanitizer exited nonzero for a 4,976-byte allocation originating
from the unchanged cli_install_plan_receipt_no_mutation_issue388 test path
(tests/test_cli.c:2398 / src/cli/cli.c:7539). This PR does not touch that
path; it is being kept out of this one-issue change and investigated separately.

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test) — all cases passed, but
    the separate unchanged CLI LeakSanitizer finding above made the process
    exit nonzero
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)

Signed-off-by: Anand Aiyer <anand@aiyer.com>
@aaiyer
aaiyer requested a review from DeusData as a code owner July 17, 2026 12:51
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.

1 participant