Repository navigation
[pull] main from DeusData:main - #154
Merged
Merged
Conversation
…rings
main fails the memory-core linter:
src/mcp/mcp.c: grew by 3 (814 -> 817)
PR #1723 (merged as 92abefa) added a new early-return path to
handle_index_repository and, following the function's convention, freed
its three argument strings there with raw free(). Its CI lint was green
on 2026-09-09. On 2026-09-19 the memwaste merge cut this file's raw sites
and ratcheted the baseline down to 814, so by the time #1723 landed its
green was measured against a rule that no longer existed. The linter is
right and the merge-result check that let it through was incomplete: it
built and ran the suites and did not run the linter. It does now.
The fix is not cbm_free. The three strings come from
cbm_mcp_get_string_arg and resolved_repo_path_from_project_arg and are
raw heap memory, released raw by every other path in the function;
routing three of them through the core would free raw memory through the
accounting layer. The actual defect is that the function had 41 raw
free() calls of the same three arguments across a dozen early returns,
and #1723 added one more block in the same shape.
One helper, index_args_free(repo_path, mode_str, name_override), now
replaces twelve of those blocks. free(NULL) is a no-op, so the one path
where repo_path is still NULL passes NULL. On the cross-repo branch
mode_str used to be freed before handle_cross_repo_mode and the other
two after it; mode_str is neither passed to nor read by that call, so
releasing all three after it is not observable. Five sites on the
normal-completion path are left as they were: they free the strings at
different points as ownership is handed to the pipeline, and collapsing
them would change that order for no gain.
Raw sites in this file: 817 -> 785. The baseline follows the improvement,
814 -> 785, as the linter asks; it can only tighten.
Build: clean, -Werror, ASan/UBSan. mcp cli daemon_application:
692 passed, 4 skipped, 0 failed -- index_repository_honors_allowed_root,
which runs through a replaced early return, included.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
main fails scripts/test.sh at Step 5f on every platform:
=== Step 5f: worker request-scope regression ===
worker did not index the admitted repository
{"content":[{"type":"text","text":"missing or incomplete trusted
worker policy"}], ... "isError":true}
Second collision from the #1723 merge (92abefa), and the same kind as
the first. #1723 made an index worker refuse any request that does not
carry `_cbm_index_policy`: a worker never resolves the resource policy
from config or environment, the supervisor does and sends it along.
tests/test_worker_session_scope.sh, added on 2026-09-06 by a08b9ec,
plays the supervisor by hand -- it runs `cli --index-worker
index_repository '{"repo_path":...,"mode":"fast"}'` directly -- and
#1723's branch never contained that script, so its CI never ran the two
together. The merge was textually clean and the result was wrong. It
stayed hidden behind the lint failure the previous commit fixes, because
the test stage only runs once lint is green.
The worker is right and the script was incomplete: it now sends what the
supervisor sends, both limits off, which is the default
(`"index_max_files":"off","index_max_source_mb":"off"`, the same object
tests/test_index_policy.c uses). The fail-closed case carries it too, so
that a request without repo_path is still refused for its missing scope
("request workspace scope invalid") and not, by accident, for a missing
policy. No assertion changed.
RED, production binary built from this branch, script unmodified:
worker did not index the admitted repository
... "missing or incomplete trusted worker policy" ... rc=1
GREEN:
ok: index worker is scoped to the admitted request, not the daemon
environment rc=0
On the failed CI run of this PR, 12 of the 13 red test jobs carry exactly
that signature and nothing else. The 13th, test-windows-guards, is
test_daemon_stability.py section_cold_storm ("secure CLI coordination
could not be created (endpoint)", a racing cold start) -- a known
nondeterministic guard, unrelated to either commit here. Step 6, which
Step 5f's failure kept CI from reaching, passes locally:
security-strings allow-list 4 passed, destructive-ordering contract PASS.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
fix: restore main to green after #1723 — memory-core linter and worker-scope Step 5f
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )