Two contributors independently sent the same fix for #1550 while ours was
already in CI (#1553 by @musichen, who also reported the bug, and #1564 by
@henri-edh). Rather than close both as duplicates, this takes the part of
theirs that ours was missing.
@henri-edh's comment names the actual mechanism: Pi loads an extension via
jiti.import(path, { default: true }) and rejects anything that is not a
function, so a named-only export hands the loader the module namespace object
and the install fails with "Extension does not export a valid factory
function". Ours said "Pi calls the default export" — true, but it does not tell
the next reader why a named export cannot simply be added alongside, which is
exactly the change someone would try. Their wording is now ours.
The assertion is tightened the same way, from `export default function` to
`export default function (pi)`, so a default export with the wrong shape cannot
satisfy it.
Co-Authored-By: musichen <musichen@users.noreply.github.com>
Co-Authored-By: henri-edh <henri-edh@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The reporter came back with the output that named this:
exact executable identity could not be verified (cache-private) -
/Users/.../.cache/codebase-memory-mcp: ancestry component validation
failed (errno 2)
Two separate defects, and the second is why nobody could see the first.
**The refusal.** The daemon's private-directory ancestry walk rejected any
group-write bit on any ancestor — `(mode & 0022) == 0`. That is the identical
rule #1535 removed on the activation side, in the sibling gate the consolidated
strictness decision covers but that never got changed. A group-writable ~ or
~/.cache is ordinary: WSL2 ships 0775, so do several distro skeletons and any
site with a shared primary group. So the daemon refused to run, permanently,
with nothing the reader could close or restart.
World-writable is still refused (any local user could swap a path component),
and the private directory itself is still chmod'd to 0700 and verified after
the walk — which is what makes admitting the ancestor safe: the thing that
actually holds data stays owner-private either way. Group-writable ancestors
are logged, so the weaker check is never silent. Both directions are tested;
a fix that admitted everything would "fix" this by deleting the protection.
**The invented errno.** When that check failed, the branch set no detail, so
the caller fell back to printing `errno` — which nothing on the path sets. The
reporter was handed ENOENT for a permission refusal and went looking for a
missing file that was there all along; it cost a round trip to find out the
number meant nothing. The branch now names the component and the rule.
Reported by @listepo, who ran the diagnostics that turned this from a guess
into a one-line root cause.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The local Windows leg caught this within hours of merging #1532: on a machine
that is not domain-joined, USERDOMAIN is the literal string WORKGROUP, and
`WORKGROUP\test` is not a resolvable principal —
WARN: build-dir DACL stamp (pre-wave) failed (user=WORKGROUP\test ...):
WORKGROUP\test: No mapping between account names and security IDs was done.
so the grant fails outright and the tree keeps its inherited
`Authenticated Users:(M)`. The guard then correctly reports that cross-account
mutation is still possible, and the whole leg stops. Since most Windows
machines are not domain-joined, that is the common case, not the exotic one.
#1532 was fixing a real bug in the other direction (a bare name resolves
against the machine first, so a host named like its user grants to an empty
principal). Both failures are name RESOLUTION failures, so this stops resolving
names: the account is identified by SID, which is unambiguous everywhere. The
SYSTEM and Administrators grants in these same commands have always used that
form — the current user was the odd one out. Name lookup survives only as a
fallback where PowerShell is unavailable.
Applied to all three scripts carrying the stamp (run-tests-parallel, soak-test,
memlab), since they share the failure and would otherwise drift apart.
Co-Authored-By: Kiborgik <Kiborgik@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A reporter ran `uninstall`, then could not install again: every attempt said
"active CBM sessions and operations could not be stopped safely". A reboot
proved no such session existed, and the remedy line v0.10.2 added made it worse
— it told them to run `codebase-memory-mcp daemon status`, a binary they had
just uninstalled.
Both halves were wrong for their case:
The guard funneled two unrelated outcomes into one message. A cohort that is
BUSY means real sessions hold it and closing something is the fix. A reservation
that FAILED — lock I/O, leftover coordination state, permissions — means nothing
is running at all, and telling that reader to close sessions sends them after
processes that do not exist. Same misattribution class as #1416 and #1535: a
non-session failure wearing a session costume. They are now separate messages,
and the failure one says plainly that nothing needs to be closed.
The remedy also has to survive the situation it prints in. Install, and any
retry after uninstall, runs with no cbm on PATH; advice that starts with
`codebase-memory-mcp ...` is unusable exactly when it is needed. The busy
message now leads with the actionable step (close the editor/agent holding it)
and mentions the command only as an aid where a binary still exists; the
failure message asks for a cache listing, which needs no cbm at all.
A recorded transaction refusal still outranks both generic messages, so the
#1416 attribution is unchanged.
This does NOT yet explain why their reservation fails with nothing running —
that needs their environment, which is being requested on the issue. What it
does is stop sending them after the wrong thing while we find out.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`posix_spawnp` and `fork` both fail with EAGAIN when the process table or a
per-user limit is momentarily full. Both paths treated that as a hard failure,
so a burst of process creation turned into CBM_PROC_SPAWN_FAILED — a git probe,
an LSP server, or an index worker refusing to run for a reason the user can
neither see nor act on, on a machine that is merely busy.
This is the production half of a flake we have been re-running for two days:
`subprocess_run_clean` failed spawning `/bin/sh -c "exit 0"` on macOS runners in
three separate CI runs, always under a loaded release matrix, always green on a
quiet rerun. Chasing it as a test race would have been chasing the symptom — the
test is right, and a user on a loaded laptop hits the same edge.
EAGAIN/ENOMEM now retry three times over ~30ms; everything else keeps failing
immediately, and an exhausted system still fails after the backoff rather than
spinning. exec-class errors (ENOENT/EACCES/ENOEXEC/...) are untouched, so the
documented "missing binary exits 127, spawn_failed means the mechanism failed"
contract still holds. The fork fallback gets the same treatment: it fails for
the same reason under the same pressure, and the fallback must not be less
robust than the primary path.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`trace_path(include_evidence:true)` added `strategy` and `confidence` per hop in
the default tree output, and returned `cols: ["name","hop"]` under
format:"json" — no error, no warning, just two fields missing from the response
shape the schema and `--help` both promise.
include_evidence was only ever implemented in bfs_to_tree_table; bfs_to_tree_json
never took the flag. So the callers most likely to want the evidence — the ones
asking for structured output to consume programmatically — were exactly the ones
who silently got none of it. Reported by @asemraza.
The json path now emits both columns with the same semantics as the tree path,
including the placeholder case: the root hop has no inbound edge and non-CALLS
edges record no strategy, so those rows carry nulls rather than dropping fields
and leaving callers with ragged rows. Tree says "-", json says null — the same
promise in the form each consumer can test.
The existing evidence test covered only the tree path, which is why this went
unnoticed; it now asserts both formats.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Two regressions we shipped ourselves, both with the same shape: a change that
was correct in the abstract broke a concrete thing users had working.
**`update --ui` became a hard error (#1544).** v0.10.2 removed the ui/standard
chooser and deleted the flags that drove it, so a command in scripts, aliases
and muscle memory started failing with "unknown update option" where v0.10.1
had printed the install-script handoff. Retiring a CHOICE is fine; retiring the
WORDS is a break users get nothing for. Both flags are accepted again, do
nothing, and print one note explaining that every build now includes the UI.
A genuinely unknown flag is still rejected — the point is compatibility, not
silence. Reported by @ehendrix23.
**The generated Pi extension could not load (#1550).** Since #1392 the
extension has been generated from the tool registry, emitting
`export function register(pi)`. Pi loads an extension by calling its DEFAULT
export as a factory, so the file failed to load — and a Pi extension that fails
to load takes every `pi` command with it, `pi doctor` included, not just cbm's
tools. It now default-exports the factory. Reported by @musichen.
The existing generator test passed throughout: it asserted WHICH tools the file
lists, never that the file Pi loads is loadable. The new test pins the export
shape, which is the property that actually matters to the consumer.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The quarantine path in resolve_store_internal() used the shallow
cbm_store_check_integrity() to decide whether to rename a project DB
to .corrupt and rebuild it. That check has two flaws that together
cause healthy databases to be destroyed under concurrent access:
1. It treats ANY sqlite3_prepare_v2 failure as corruption. Under
concurrent MCP instances (#1206), a writer lock held by another
instance makes the schema lookup return SQLITE_BUSY/LOCKED — a
transient condition, not damage. The losing instance quarantines
a perfectly healthy DB and rebuilds a sparse replacement.
2. It never runs PRAGMA quick_check, so it cannot catch real
page-level corruption either. A DB with a torn node/edge btree
but an intact projects table passes the check and keeps being
served (#1037) — the inverse failure.
This change introduces cbm_store_check_integrity_verdict(), which
returns a tri-state (OK / CORRUPT / TRANSIENT) and runs the shallow
check plus PRAGMA quick_check, classifying BUSY/LOCKED/IO-lock codes
as TRANSIENT. The quarantine decision in mcp.c now quarantines ONLY
on a confirmed CORRUPT verdict; TRANSIENT releases the mutation lease
and retries on the next resolve, never touching the DB file.
The existing cbm_store_check_integrity() and _deep() are unchanged,
so all other call sites (hot opens, artifact import, maintenance)
keep their current behavior.
Verified: builds clean with -Wall -Wextra -Werror; cli list_projects
and index_status run correctly against the patched binary.
Signed-off-by: LynxBay <187708458+LynxBay@users.noreply.github.com>
Five field reports in the 24 hours after v0.10.0 all pointed at the same thing:
gates that were right in principle refused real, ordinary setups, and then
failed to say why. Per the consolidated strictness decision, each gate keeps the
protection that matters and drops the part that was refusing legitimate users —
and every refusal now names what it refused and how to proceed.
**Daemon image gate: npx and every ephemeral install path (#1539, #1383).**
The admission check treated "the peer's image hashes differently" and "the
peer's image cannot be examined at all" as one failure. The second is what
`npx codebase-memory-mcp` always produces (ephemeral cache path,
unfingerprintable), so every npx-invoked client was rejected — and, because the
client never reported it, agents saw a transport that closed mid-handshake with
zero bytes on stdout. Reported by @wassolles with the admission path already
read and the fix space mapped.
An unverifiable image is now admitted: the rendezvous HELLO immediately above it
has already proven semantic version, build fingerprint, and protocol/store/
feature ABI, and the image check was trading that real proof for an unavailable
one. It logs daemon.client_image_unverifiable_admitted so the weaker check is
never invisible. A fingerprint MISMATCH — the tamper case the gate exists for —
still rejects hard. Separate test seams keep the two modes testable apart.
**Client bootstrap failures are no longer silent (#1539).**
An MCP client that cannot reach the daemon now emits a JSON-RPC error on stdout
naming the reason, plus the same text on stderr. Previously the reason sat in
bootstrap_result.message and the process exited having written nothing at all.
**POSIX activation: group-writable ancestors (#1535, discussion #1526).**
activation_directory_secure required no group or other write bit on the install
directory AND every ancestor. WSL2 ships ~ and ~/.local at 0775, as do several
distro skeletons and any site using a shared primary group, so install.sh failed
for a large fraction of Linux users — reporting a policy refusal as "activation
transaction I/O failed", which sent reporters after disk errors and filesystem
types. Root-caused by @AmirF194 in a clean ubuntu container; @shochdoerfer and
@iandol confirmed independently.
World-writable ancestors are still refused (any local user could swap a path
component mid-transaction). Group-writable ancestors are now warned about and
admitted. The LEAF directory stays strictly owner-private — that is where the
binary is published, and group write there would let another account replace the
executable between validation and exec. Refusals now name the directory, its
mode, and which rule refused.
**The obsolete ui/standard chooser (#1538, from discussion #1526).**
v0.10.0 consolidated to one archive per platform with the UI always embedded,
but `update` still offered a variant choice: "ui" could only 404, and "standard"
quietly WAS the UI build. Reported by @iandol upgrading 0.9.0 -> 0.10.0. The
chooser, its --standard/--ui flags, and the ui- URL plumbing are removed, along
with the CBM_VARIANT=ui remnant in the npm installer.
Already-released 0.9.x binaries cannot be fixed retroactively, so the release
workflow now publishes byte-identical ui-*-named alias assets — their updaters
work again with no user action. The aliases are uploaded AFTER the VirusTotal
gate: they are the same bytes as archives it already cleared, and uploading them
earlier would duplicate every object in the scan set and the provenance manifest.
**macOS install noise and attribution (#1537).**
install.sh silenced the "No such xattr: com.apple.quarantine" line, which is
what happens when a curl-downloaded archive carries no quarantine attribute —
harmless, and it became the title of a bug report about an unrelated failure.
The session-stop refusal now points at `daemon status` to list the client
processes actually holding the daemon, instead of asserting sessions exist and
leaving the reader to guess. Reported by @listepo.
**Riders.** hatchling is pinned in pkg/pypi (an unpinned backend resolved fresh
inside `python -m build` is what emitted Metadata-Version 2.5 and broke the
v0.10.1 publish); SECURITY.md's supported-versions table moves to 0.10.x.
Tests: separate seams for unverifiable vs mismatched peer images with a test per
outcome; activation refusal must name directory + mode + rule; a group-writable
ancestor must stage successfully. The update tests drop the flag that no longer
exists. Verified against each reporter's environment shape.
**Open security alerts (all three, OSSF Scorecard).**
- HIGH, binary artifact: an 8.8 MB compiled Go ELF wrapper had been committed at
pkg/go/codebase-memory-mcp by accident. Removed, and both it and its .exe
sibling are gitignored so `go build` in that directory cannot repeat it.
- HIGH, GHSA-2v37-7h3g-55p8: nanoid < 3.3.17 loops forever when a custom
generator is called with size 0. It reaches us transitively (postcss -> vite),
so it is pinned through the existing graph-ui overrides block rather than
promoted to a direct dependency; the lockfile resolves 3.3.18.
- MEDIUM, unpinned pip command: the publish step installed build/twine by
version only, leaving the whole transitive graph resolved at run time.
pkg/pypi/requirements-publish.txt now hash-pins the complete toolchain (316
hashes), generated on a linux/amd64 python:3.12 image so the wheels match what
ubuntu-latest resolves, and the step runs pip with --require-hashes. Verified
by installing from it in that same image.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
win_security_init built its owner-only ACL with AddAccessAllowedAce, which
has no AceFlags parameter and can only emit an ACE with flags set to zero.
Applied to a directory together with PROTECTED_DACL_SECURITY_INFORMATION
that yields D:PAI(A;;FA;;;<user>): the protection severs the inherited ACEs
while the new ACE propagates nothing, so every child created afterwards is
born with an empty DACL and is unreadable even by its owner.
win_runtime_directory_secure is also a repair path. It continues on
ERROR_ALREADY_EXISTS and re-stamps the existing directory, so a tree
restored by hand with icacls is destroyed again on the next daemon or
client start.
Add a second ACL for containers, built with AddAccessAllowedAceEx and
CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE, and use it at the three
directory sites. Files and kernel objects are leaves and keep the flagless
ACL.
The container ACE carries FILE_ALL_ACCESS rather than GENERIC_ALL. Windows
splits an inheritable generic-rights ACE into an effective mapped ACE plus
an INHERIT_ONLY one holding the generic bits, and the owner-only DACL
validators require exactly one ACE, so the generic form fails every lock
directory with "secure CLI coordination could not be created
(project-locks)".
Fixes#1351
Signed-off-by: PETRO YAKOVYSHYN <petroyakovyshyn@gmail.com>
Three regressions shipped in 0.10.0 share one failure shape: an empty result
with a success status, indistinguishable from "nothing found" for the LLM
clients that are cbm's primary consumers.
1) structuredContent {} on the whole tree-format surface (#1522 bug 1).
#1488 replaced the duplicated payload with an EMPTY structuredContent
object while every tool still declared a blanket permissive outputSchema.
Spec-honoring clients (Claude Code among them) treat structuredContent as
THE result when a schema is declared, so search_graph, trace_path,
query_graph, get_architecture, search_code, and detect_changes all rendered
as literally "{}" on their DEFAULT format, on every platform. The corrected
contract: no tool declares an outputSchema (tool output is
format-parameter-polymorphic — no static schema is truthful), JSON-object
payloads keep their parsed structuredContent, errors keep
structuredContent.error, and text-shaped payloads carry NO structuredContent
key at all — which also preserves #1375's no-duplication win.
2) Frontend queue overflow killed the session (found by the #1522 sweep).
Any 7+ requests pipelined in one stdin burst — an agent issuing parallel
tool calls does exactly this — overflowed the 8-frame frontend queue, which
failed the whole session: rc=1 with ZERO bytes of output, every buffered
response lost. A full queue is now backpressure: the stdin reader blocks
until the worker drains (bounded by the same stop/fail flags every teardown
path already sets); only a single frame larger than the entire 12 MiB byte
budget — which could never be admitted — remains a hard failure.
3) config get printed "" with exit 0 for every unset and every unknown key
(#1522 bug 2). list printed stored-or-DEFAULT while get printed
stored-or-EMPTY, and no subcommand validated key names, so a typo was
indistinguishable from a correctly-read setting. One config-key table now
drives help, list, get, set, and reset: get prints the stored value or the
key's real default (the same fallback the runtime readers use), and unknown
keys error with exit 1 on get, set, and reset alike.
Tests — each RED on the pre-fix tree and RED again on revert:
* test_mcp.c: text results carry no structuredContent key; tools/list
declares no outputSchema; the tool-table guard now binds all three
branches (absent / parsed-object / error) for every registered tool.
* test_daemon_frontend.c: the over-capacity contract flips from
"session fails" to "backpressure without loss" — the held first request
plus all 32 over-capacity frames are answered and the run closes cleanly.
* test_cli.c: the config command contract — defaults, round-trip, reset,
and unknown-key rejection on all three subcommands.
* smoke-test.sh: Phase 3z rewritten to the corrected structuredContent
contract, new Phase 3z1 (default-format replies usable in schema-honoring
clients, no outputSchema advertised), 3z2 (24 pipelined calls all
answered), 3z3 (config defaults + unknown-key rejection) — all asserted
against the SHIPPED artifact, where #1488's smoke phase previously pinned
the empty-object behavior as correct.
Verified end-to-end on the locally built production binary: all 15 tools
declare no schema; text tools return ABSENT structuredContent, object tools
populated, error envelopes intact; 7/24/64-deep pipelined bursts all answered
with rc=0; config get prints real defaults and exits 1 on unknown keys.
hook_augment (structuredContent.projects) and index_resilience
(structuredContent.status) consume object payloads and are unaffected.
Fixes#1522.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
check_index_coverage's inputSchema declared a top-level anyOf requiring
either 'paths' or 'scopes'. Anthropic (Claude), AWS Bedrock, Google
Vertex/Gemini, and Azure OpenAI all reject custom tool input schemas
containing a top-level oneOf/allOf/anyOf, so any MCP client backed by
one of these providers fails once this tool's schema reaches the model
(e.g. Claude: 'tools.N.custom.input_schema: ... anyOf is not supported
at the top level'), breaking every tool call in the session.
handle_check_index_coverage already enforces 'at least one of paths or
scopes' at runtime independently of the JSON Schema (path_count == 0
&& scope_count == 0 rejection), so dropping the schema-level anyOf does
not weaken validation. The requirement is now stated in the tool
description and in the paths/scopes property descriptions instead.
Fixes#1524
Signed-off-by: DaveMurray-AIGuys <264998796+DaveMurray-AIGuys@users.noreply.github.com>
Both Linux soak-quick legs in the dry run failed with:
FAIL: RSS 17592186044293MB > 200MB ceiling
17592186044293 MB is 2^44-123 -- a NEGATIVE ~123MB wrapped through size_t. The
diagnostics snapshot reported mi_process_info()'s current_rss verbatim, and on
Linux mimalloc never sets that field: it keeps mimalloc's internal
committed-page counter, which this project deliberately tunes low
(purge_decommits=1, purge_delay=0) and which can transiently go negative under
purge. macOS and Windows are untouched -- mimalloc reports real RSS there --
which is exactly the observed split: both Linux legs red, both macOS legs green.
mem.c already learned this lesson in full (its comment block documents the same
trap for cbm_mem_over_budget): cbm_mem_rss() is /proc-statm-primary on Linux and
mimalloc-correct elsewhere, and cbm_mem_peak_rss() reconciles peak >= current.
The diagnostics writer now uses both instead of the raw mi fields, so the soak
leak gates measure actual process RSS on every platform.
Verified: diagnostics suite passes; compiles and formats clean.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Root cause of the Cyrillic guard failure, found via the finalize diagnostics:
seal_existing rc=-1. The primary store open normalizes the path through
cbm_path_for_file_api (UTF-8 -> wide -> UTF-8, the form SQLite's Windows VFS
needs), but four maintenance opens passed the raw UTF-8 path:
cbm_store_seal_existing_path_for_replace (the one this path hits)
cbm_store_prepare_path_for_replace
cbm_store_backup_path (both source and staging)
cbm_store_dump_to_file
So a generation was WRITTEN through the normalized path and then could not be
RE-OPENED through the raw one. On the second index into a cache the seal step
failed, finalize returned PERSIST_FAILED, and the user was told "Check repo_path
exists and contains source files" about a repo that had just indexed cleanly
(nodes=12 edges=21 in the same worker log).
On POSIX cbm_path_for_file_api is a no-op memcpy, which is why the full 3-OS
ladder and every non-Windows test passed -- the defect is Windows-only and only
manifests on a path the raw UTF-8 -> SQLite open mishandles. It surfaced on the
Cyrillic case first because the guard indexes the same repo TWICE (once for the
count, once on retry), making it the first case to re-open an existing
generation; the earlier ASCII/latin1 cases index into fresh caches.
All four now use cbm_path_for_file_api exactly as the primary open does. Also
upgrades dump_to_file from sqlite3_open to sqlite3_open_v2 with explicit flags.
The finalize.prepare_failed / rename_failed diagnostics from the previous commit
stay: a silent PERSIST_FAILED that blamed the repo cost a day of remote bisection
and must never be silent again.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Cyrillic guard failure came down to this: the worker indexes the repo
perfectly (gbuf.dump nodes=12 edges=21, DB written, sealed, sidecars staged) and
then finalize fails somewhere between stage_sidecars and prepare_live -- and
every failure edge on that path returns PERSIST_FAILED without a word. The user
then reads "Pipeline failed. Check repo_path exists and contains source files"
about a repo that indexed flawlessly.
A silent failure that misattributes itself cost a full day of remote-only
bisection: five instrumentation commits in the TEST were needed to even locate
the failing function. That is a product defect independent of the original bug.
Every edge now logs before returning -- destination-not-regular, seal rc,
sidecar-cleanup errno (both branches), and the final rename with stage and
destination paths. The next guard run names the exact edge and value instead of
requiring a sixth probe.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Ran the format gate before adding --dir rather than after, so the ternary
landed unformatted and lint/lint went red. Reformatted with the pinned Homebrew
LLVM clang-format; the diff is confined to that block and cli.c still compiles.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Windows long-path guard failed with:
error: unknown uninstall option: --dir=C:\...\install_Ωμέγα\...\bin
Restoring cli.c from before the externalization dropped --dir from uninstall
while install kept it. That is worse than a test failure: anyone who installed
outside ~/.local/bin had no supported uninstall path at all — uninstall would
look in the default location, find nothing, and leave the real install in place.
Parsing mirrors cbm_cmd_install (--dir=PATH and --dir PATH, both rejecting an
empty value), and the resolved directory now feeds the target path with an
explicit truncation check instead of an unchecked snprintf into a fixed buffer.
Confirms the earlier cascade read: with the extended-path probes fixed,
daemon_stability, hook_augment, cli_non_ascii_arg and ui_drive_listing all went
green on their own. non_ascii_path was the only real failure, and it was hiding
two distinct product regressions behind one red guard.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
test_non_ascii_path.py caught a real product regression I introduced:
[FAIL] long-path existing target was overwritten despite --no
(target existence probe is not extended-path safe)
Restoring cli.c from before the externalization also reverted three probes from
cbm_path_info_utf8() back to plain stat(). On Windows stat() resolves through
the ANSI code page, so an extended-length or non-ASCII target reports "absent" —
and a non-force `install --no` then overwrites the exact bytes the user asked to
keep. Silent data loss, not a cosmetic difference.
That fix rode along in the externalization commit but is orthogonal to it: the
wide-path helpers it depends on (win_utf8.h, compat_fs.c) were kept. Only their
call sites in cli.c were lost.
Two of main's five cbm_path_info_utf8 sites lived in asset code that is
correctly gone; the two restored here are the install target probe and the
uninstall binary probe.
Not yet explained: test_daemon_stability.py also went red in the same run
("daemon did not accept the UI configuration"). Its code path is identical to
main's apart from the intended capability predicate, and the guard that failed
before it completes an install into a long path it should have refused, so this
may be downstream state. Pushing this fix to find out rather than guessing.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Externalizing the integration templates (#1492/#1493) and the UI bundle
(#1501/#1503) was done to reduce the Microsoft `Wacatac.B!ml` surface. It did
not work: across dry runs the flagged artifact count stayed at ~3 and the
detections merely moved between artifacts.
Dissection of run 31286803592 shows there is no structural cause to fix. The
verdicts split across every axis at once — linux-amd64 (dynamic) flagged while
linux-amd64-portable (static) is clean, but linux-arm64 (dynamic) clean while
linux-arm64-portable (static) is flagged. The two macOS binaries have identical
segment structure and split clean/flagged. Siblings from one build landed in
different variant buckets (.B vs .C). Entropy is low everywhere
(code_vectors.bin 4.166, grammar tables 3.464 bits/byte, against 7.5-8.0 for
packed payloads), so the packed-payload hypothesis is excluded too.
So the complexity bought nothing, and installation goes back to being
self-contained: one binary that carries its own UI and agent integration
templates, with no adjacent data file that has to resolve before `install`
works. Only the UI-capable composition ships from now on, under the historical
unsuffixed archive name.
Removed: src/ui/asset_pack.{c,h}, asset_pack_stub.c, asset_manifest_stub.c,
scripts/pack-ui-assets.mjs, src/cli/integration_assets.{c,h},
assets/cbm-integrations.json, scripts/gen-integrations-hash.sh, the
--verify-runtime-assets probe (nothing adjacent left to verify), and the
composition gates A6/A7 whose property is now deliberately inverted.
Restored: scripts/embed-frontend.sh, src/ui/embedded_{assets.h,stub.c}, the
compiled-in hook/adapter template bodies, and the embed/EMBED_OBJS build path.
Kept from the reverted commits, re-applied by hand where a wholesale file
restore would have dropped them:
- cbm_module_path_utf8() in both self-path sites. GetModuleFileNameA renders
through the ANSI code page and mangles non-ASCII install paths.
- the /__cbm/ui-readiness HMAC proof, secure_random and cbm_hmac_sha256, so
`daemon start --open` still waits for a genuine CBM listener.
- X-Content-Type-Options: nosniff on served assets.
- the MinGW noexecstack gate, -lbcrypt, and the cppcheck/zip CI fixes.
Archives are now codebase-memory-mcp-<os>-<arch>[-portable] with exactly four
members (binary, LICENSE, installer, THIRD_PARTY_NOTICES.md). That restores the
names every static package manifest already points at — aur, chocolatey,
homebrew, scoop, winget and glama were all broken by the -ui- rename.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The product binary embedded nine complete shebang'd shell scripts, their
PowerShell/.cmd twins, and two node:child_process client-adapter modules as C
string literals, and wrote them into agent config dirs at 0755 on install. A
block of script-shaped bytes inside a large unsigned executable is exactly the
surface Microsoft Defender's ML scored Trojan:Script/Wacatac.B!ml — the
detection that has recurred across the last several release attempts, on the
Linux ELF and darwin-arm64 standard binaries while every UI and Windows variant
stayed clean.
This moves every integration template out of the binary into a single shipped
data file, assets/cbm-integrations.json (compact: one asset, because each
shipped file is itself scanned). The binary now carries only the templates'
identity — one embedded SHA-256, generated at build time from the JSON so it
can never drift — plus the code to load, verify, and render them.
Why a hash and not just a loose file: content compiled into the binary was
tamper-evident with the binary. Moving it to disk must not turn it into
unauthenticated code the installer blindly executes. install verifies the asset
against the embedded SHA-256 and fails closed on mismatch ("integration assets
missing or modified - reinstall from the release archive") before writing
anything. Verified end to end: a one-byte edit to the asset makes install
refuse.
Ownership, for safe upgrade/removal: the verified copy is published to
~/.cbm/assets/<version>/ — a sibling of the DB cache, never inside it, so
clearing the cache cannot strip a user's hooks. A deployed integration file is
ours to remove only if it matches a template rendered from that stored copy;
foreign files and user-edited files at the reserved paths are preserved. The
historical template bodies travel in the JSON's released[] arrays so files
written by older versions are still recognised and cleaned up on uninstall.
Removed from src/: cmm_gate/session/subagent script-prefix and suffix
constants, cbm_build_released_gate_script, the shell/PowerShell/.cmd bodies in
cli.c, and the JS/TS generators in client_adapter.c. Confirmed on the built
standard binary: 0 occurrences of '#!/usr/bin/env bash', '#!/bin/sh',
'node:child_process', and 'ExecutionPolicy' (was non-zero for each).
test_no_embedded_scripts_contract.sh makes the removal a build property, not a
reachability assumption: no production C source may contain a shebang or
node:child_process literal, and the built binary is scanned directly. It strips
comments first (a contract a comment can satisfy is a false guard), and is
revert-checked — reintroducing one shebang literal makes it fail naming the
file:line. check-binary-composition.sh (A6) enforces the same needles on every
release artifact post-strip.
This does not, by itself, prove the detection is cured: the clean UI binary is a
strict superset of the flagged standard one, so no single passage is a
deterministic trigger. It removes the most coherent malware-shaped composite
from every shipped binary, aligns with how gh/starship/zoxide externalise
integration material, and makes the templates maintainable as data rather than
escaped C literals. Causal confirmation still needs a VirusTotal ablation.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`cbm update` printed the Windows update command with an execution-policy
override baked in as a string literal. That put one of the most heavily
weighted malicious-script patterns there is inside every Windows artifact we
distribute — to save the user a single documented step.
The command now prints without it. Functionality is unchanged: the user still
gets a runnable command, and the failure it was pre-empting (PowerShell refusing
a file that arrived from the internet) is answered by Unblock-File, which the
message now names. The restricted-policy case is documented in the README's
Windows install section, which is the right home for that incantation — a
shipped binary is not.
Scope note, so this is not oversold: the 2026-08-07 dry-run detections are NOT
attributable to this string. It is `#ifdef _WIN32`, and every Windows artifact
scanned clean; the three flagged artifacts were Linux and macOS. This is margin
hygiene, not a fix for that regression — see the analysis in the PR.
Verification limit worth stating: the built-artifact check for this string is
vacuous on a non-Windows host, since the literal was Windows-only to begin with.
What is verified here is the source (no occurrence outside a comment) plus lint
and the cli suite; a real artifact check needs the Windows build leg in CI.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Every non-JSON tool result shipped its payload twice: content[0].text, and an
identical copy as structuredContent {"text": <entire payload>}. Measured on a
20k-node query_graph, the reply was 2.05x the payload it carried. Half of every
large answer was redundant bytes — half the 10 MiB transport budget, and double
the tokens billed to every LLM caller on every call.
Measured, same query, same fixture:
before 8,529,990 bytes content.text 4,154,932 structuredContent.text SAME
after 4,265,047 bytes content.text 4,154,932 structuredContent {}
Exactly 50.0% smaller with content.text byte-identical: the payload is fully
delivered, only the second copy is gone. The 10 MiB ceiling now also admits
roughly twice the rows before #1375's limit applies.
Nothing is lost. structuredContent exists to carry STRUCTURE, and a string
re-wrapped in a one-key object has none — a client reading
structuredContent.text learned exactly what content[0].text already told it. The
empty object still satisfies the declared outputSchema, which is
{"type":"object","additionalProperties":true} and never required a text field.
Two cases are deliberately NOT changed:
* JSON payloads. structuredContent stays the PARSED object. That is the
spec's structured+serialized pattern rather than waste, and it is what our
own hook-augment consumes (structuredContent.projects from list_projects).
* Errors. structuredContent.error is kept: bounded, small, and the only
machine-readable form of a failure a client gets.
Guarded at both levels, because the defect was invisible per-tool — each result
looked reasonable alone, and only measuring the wire showed half of it was
redundant:
* tests/test_mcp.c enumerates the TOOL TABLE itself, so a new tool is covered
the moment it is registered, with no test edit. A guard pinned to
query_graph would not have caught search_graph, and would not catch whatever
is added next. It fails if no tool produced a non-JSON payload, so it cannot
report a green it never earned — which it did on the first attempt, catching
that {} args make every tool error out and assert nothing.
* scripts/smoke-test.sh Phase 3z asserts the same property on the SHIPPED
binary. This is a wire-format contract: what a real client receives from the
real artifact is not something a from-source test can prove.
Both revert-checked: restoring the duplication reddens the unit guard at the
strcmp, the plain-text guard, the search_graph expectation, and the smoke phase
(which names search_graph and get_architecture).
tests/test_mcp.c expectations that pinned the old duplicating shape are updated
rather than relaxed — the format changed on purpose.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A single query_graph reply larger than one 10 MiB frame killed the MCP server:
exit code 1, no JSON-RPC error, nothing on stderr, no crash report. Under a host
that does not respawn the server, every tool from it was gone for the rest of
the session — the reporter lost their tooling mid-investigation to one dead-code
query and finished by driving the binary over stdio by hand.
Root cause is a conflation, not the limit itself. application_mcp_request
checked only response_length > UINT32_MAX and passed anything smaller on, so an
oversized reply reached the transport, which rejected the frame. The frontend
worker cannot tell that rejection from a dead socket, and for a dead socket it
deliberately _Exit()s the process — closing the kernel IPC handle is the only
portable way to cancel daemon session ownership from a thread blocked in stdio.
Correct for a broken transport; a fatal over-reaction to a large answer.
The substitution therefore has to happen in application_mcp_request, which is
the last layer still holding the request id. Replies that fit are returned
byte-identical; a reply that cannot be framed becomes a JSON-RPC -32603 naming
both numbers and what to do ("narrow the projection, add LIMIT, or paginate"),
because the failure it replaces was a silent exit and an opaque "internal error"
would be no improvement.
Reproduced first, on a locally generated 20k-node fixture rather than the
reporter's repo (external code stays unverified until reviewed):
before LIMIT 10000 -> ok, 8,529,990 bytes
LIMIT 20000 -> SERVER DIED exit=1 stderr=''
after LIMIT 20000 -> JSON-RPC -32603, and a follow-up query on the SAME
session succeeds — which is the property that actually matters
The decision and the substitution live in one function on purpose. The first
version of the test built the error object directly and PASSED with the size
check removed: it proved the error was well-formed while the server still died.
Testing application_framable_response instead binds the branch — verified by
removing the guard, which reddens it at ASSERT(replaced != big).
Not addressed here, deliberately: every reply still carries its payload twice
(content[0].text and structuredContent.text are verbatim duplicates, ~2.05x
measured), which is why the ceiling is reached at ~4 MB of real content. That is
a wire-format question affecting every tool and every caller's token bill, so it
belongs in its own change with a decision behind it, not smuggled into a crash
fix.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
#1448 set CREATE_NO_WINDOW on the subprocess spawn and on cbm_popen_isolated,
but cbm_exec_no_shell (compat_fs.c) still passed a bare 0 for dwCreationFlags.
That is the helper behind git, codesign and open, so the most frequently hit
path kept flashing a console — and under a stdio MCP session with auto_watch
those windows steal focus while the user is typing. Reported by @noctrex on
#1448 after verifying the merged fix.
Also adds the contract nobody had. The flag has to be set per call site, the
four sites do not resemble each other, and reviewing "does this PR add the
flag" says nothing about the sites a PR does not touch — which is exactly how
the third one survived a fix applied twice:
daemon/bootstrap.c had it from the start (DETACHED_PROCESS | ... | flag)
subprocess.c added by #1448 (flags variable)
compat_fs.c :313 added by #1448 (inline literal)
compat_fs.c :689 missed by #1448 (bare 0)
tests/test_spawn_no_window_contract.sh asserts the whole-tree property instead,
so a fifth spawn site cannot be added without it.
The contract strips C comments before matching, and that is load-bearing rather
than tidiness: the first version of it PASSED with the fix reverted, because the
explanatory comment above the fixed call contains the flag name and satisfied
the match. A contract that can be satisfied by prose about the contract is a
false guard. Verified in both directions afterwards — green with the fix, and
red naming compat_fs.c:696 with the fix reverted and that comment still in
place.
Co-Authored-By: noctrex <noctrex@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
cbm_resolve_cache_dir() returns a pointer to a static thread-local buffer. The
cache prune called it for every directory the walk visited, so it rewrote a buffer
other code was entitled to still be holding — an aliasing bug, not a cosmetic
inefficiency.
The Windows guards job caught it as two REDs: a daemon cold-storm regression and
a following "index did not run" setup failure. Both are consistent with the cache
path being rewritten underneath a live reader during indexing.
Resolve once at the discovery entry point into a thread-local copy and compare
against that, so the prune costs one resolver call per walk rather than one per
directory, and holds no pointer into shared storage.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The comment above MIMALLOC_OVERRIDE_DEFINE claimed Unix "relies on
static-link-order override". That was never true. mimalloc emits strong
malloc/free definitions only when MI_MALLOC_OVERRIDE is set — the define its
source gates alloc-override.c on — and the build set it for MinGW only.
MI_OVERRIDE, which the build does set everywhere, is this project's own
prod/test marker that mimalloc never reads. With the override body compiled
out there were no strong symbols for link order to prefer, so ordinary malloc
went to libc.
Measured, not inferred. The shipped v0.9.1-rc.1 artifacts report 0/6
allocator-owned size classes on linux-arm64 glibc AND musl-static, and the
same A/B on ubuntu-arm64 here:
main warn mem.allocator.not_owned owned_classes=0/6
with this fix info mem.allocator.owned classes=all
Every purge/reclaim option cbm_mem_init sets was therefore inert on Linux,
applying only to the bound sqlite/tree-sitter populations — the same class of
defect as #581, where committed memory ratcheted for months because nothing
asserted the wiring on a real artifact.
macOS stays off deliberately and permanently: enabling the override there
compiles alloc-override.c's forwarding definitions, and under the two-level
namespace this binary's free becomes mi_free while system libraries keep
allocating from the system allocator, so the first pointer crossing that
boundary aborts with "mi_free: invalid pointer". ELF's flat namespace has no
such split, which is why Linux can have this and macOS cannot.
Three parts:
* Makefile.cbm switches MI_MALLOC_OVERRIDE and a new CBM_MEM_GLOBAL_OVERRIDE
together in one place, for MinGW and Linux. The latter tells our own sources
what to expect and goes into CFLAGS_PROD only, because MIMALLOC_CFLAGS_TEST
builds mimalloc with -DMI_OVERRIDE=0 and no override define: a test binary
has no global override BY CONSTRUCTION and must never be told to expect one.
Deriving that expectation from the platform instead would make every Linux
test run warn about a correctly configured build.
* mem.c splits the startup audit by what the build actually asked for. Where
no override was requested, ordinary malloc reaching libc is the design, so
it reports the measured ownership at INFO and names what IS bound. A warning
that fires on every run of a correct build is not a tripwire, it is noise —
it trains readers to ignore the one line that catches #581. The genuine
warning still fires wherever an override was requested and did not take.
* smoke-test.sh Phase 1b pins the SHIPPED artifact's wiring per platform and
fails in BOTH directions: Windows/Linux must own all classes, macOS must
not. This cannot live in a unit test — a from-source test build never has
the override — which is exactly why the defect survived so long.
Verified on ubuntu-arm64: prod owns all classes; the test build reports
bound-populations-only at INFO (0/6) and the mem suite stays green; smoke
Phase 1b passes, and fails with the expected message when the same tree is
built with the override forced off. On macOS: prod reports
bound_populations_only at INFO, smoke Phase 1b passes, mem suite 51/51.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A repository may ship .cbmpathwhitelist listing outside roots it would like
indexed alongside it. The file REQUESTS; it never GRANTS. Both attackers in this
project's threat model can write a file inside a repository — a malicious indexed
repo, and an agent with write access to the project — so a repo-local file cannot
be authoritative without handing them the boundary.
Approval is recorded user-level and keyed to a SHA-256 of the manifest bytes, so
editing the file lapses approval and it must be granted again. Without the hash a
repo approved once could widen itself forever on a later pull. This is direnv's
model, and it is the only shape that keeps a checked-in file's ergonomics without
making the file a permission slip. The hash covers the raw bytes rather than the
parsed entries: a reordering or comment change is still a change the approver has
not seen.
`allow-root --approve-manifest <project>` is the human action that grants.
Approval refuses outright if any requested entry would not stand on its own as an
indexing root, so approving cannot become a route around the breadth policy, and
entries are re-classified at use time as well so a stored approval cannot outrank
a credential list that has grown since.
Entries containing control characters are rejected — the same shape as the
newline splitting that let a crafted indexed path inject an extra entry into the
scoped file list.
Deliberately NOT wired into the root boundary. A project's manifest authorizes
outside roots FOR THAT PROJECT, so the question is "may project P pull in tree T",
not "may T be indexed standalone", and the boundary check only ever sees one path
with no project context to ask that with. A first draft did wire it there by
passing the candidate as its own project root, which read a manifest that by
definition was not the one requesting it and so authorized nothing — caught by
exercising it end to end. The consuming half belongs where project context exists,
in discovery walking a project's approved extra roots;
cbm_workspace_manifest_allows is the query it will use.
Five tests, including the one the design rests on: approval lapses when the
manifest content changes.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
fork() duplicates the parent's address-space bookkeeping. Under an
ASan-instrumented parent that includes an enormous shadow mapping, and past a
footprint threshold the child is jetsam-killed BEFORE exec replaces the image.
The reaped corpse then looks like the launched tool dying, so a `git init` in a
fixture "fails" only once enough suites have run ahead of it — a spawn defect
that reads as an unrelated assertion. tests/test_daemon_runtime.c already
switched to posix_spawn for this exact reason; this moves the shared
subprocess layer over as well.
posix_spawn never copies the parent address space, so the parent's footprint
stops being a variable. Every guarantee of the fork path is carried over:
- own process group SETPGROUP + setpgroup(0) (kill-tree contract)
- default signal handling SETSIGDEF + SETSIGMASK
- std{in,out,err} wiring adddup2
- all other fds closed CLOEXEC_DEFAULT, Apple's equivalent of the
child's close-everything loop (the three dup2'd
fds survive, since dup2 clears close-on-exec)
- PATH lookup posix_spawnp keeps execvp semantics
One deliberate difference is reconciled rather than adopted: posix_spawn
reports an unusable binary to the PARENT, where fork+exec instead yields a
child that exits 127. cbm_subprocess_run's contract treats spawn_failed as
"the spawn mechanism failed", not "the tool was missing", so exec-class errors
fall back to fork+exec and macOS keeps classifying a bogus binary exactly as
Linux does. That path forks a child that exits immediately, so it does not
reintroduce the hazard.
The existing suite binds each guarantee, verified by breaking them:
dropping CLOEXEC_DEFAULT leaks the sentinel descriptor and reddens
subprocess_posix_child_closes_unrelated_descriptors; dropping SETPGROUP makes
the kill-tree turn on the runner's own group and reddens the suite wholesale.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A forced client/daemon build mismatch raises a cohort conflict locally but is
ADMITTED on every CI leg, which made the #1388 conflict regression untestable
there. Reading the code explains how that is possible: identity comparison
happens ONLY in the BUSY branch, i.e. when another process already holds the
cohort lifetime lock. When the EX acquire succeeds - no live holder - the
participant claims the cohort and NO comparison runs at all, so a mismatched
build joins.
That is correct behaviour when nothing is running. What is missing is the
ability to tell the two apart after the fact: "was the lifetime lock held?" is
exactly the question that separates the local run from the CI run, and no log
line answered it.
This is deliberately observability only - no admission behaviour changes. The
next CI run that exercises the path will say which branch it took, and that
decides whether the CI daemon simply is not holding the lease (an environment
fact) or the lease is being released early (a real bug).
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The dry run claimed EVERY hook group as installable because gate_ok /
session_ok / subagent_ok were simply set to `true` when dry_run was on. When
the on-disk hook script is not ours - user-modified, or written by a manual
install pointing at another binary - the real install refuses to rewrite it,
so the preview promised what the run could not deliver. That is the second
half of #1387: the reporter had no way to see the loss coming, and said so.
- New read-only predicate cbm_text_owned_document_status() mirrors
text_migrate_owned_document's decision WITHOUT writing: absent / already
current / an exact released document => a write would proceed; anything else
=> it would be refused.
- The dry run uses it per script, so the three 'hooks:' lines now reflect what
will actually happen, and each refused script is NAMED with the reason.
Silence was the original defect: an unmentioned group reads as 'nothing to
do', not 'this will be skipped'.
Test: cli_dry_run_predicts_refused_hook_script_issue1387 runs the real
dry-run path over an unowned gate script, capturing stdout, and asserts the
warning names the script. RED before, GREEN after, RED again on revert.
cli+agent_profiles+config_text_edit: 292 passed; lint-ci clean.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The grant store was readable but nothing could write to it, so only
CBM_ALLOWED_ROOT could declare a root. `allow-root <path>` records one,
`--list` shows what is recorded, and `--approve-sensitive` is required for a
home or credential directory. Roots that are refused outright — volume and
drive roots, paths too broad to index as a unit — cannot be granted at all.
Enrollment is a command a person types and deliberately nothing else. That is
the property the whole store exists for: neither an indexed repository nor a
tool caller can widen its own boundary. A confirmation delivered through the
MCP surface would be answered by the same agent that may have been influenced,
so it would not be a human decision.
The path is canonicalized before recording, because the policy is defined over
resolved paths and a grant stored as a symlink would not match the resolved
path the indexer later presents.
Classified stateless in the daemon bootstrap: it writes one line of user-level
config and reads nothing from a project, so enrolling a root must not depend on
daemon state. Without that classification the argument fell through to MCP
server mode and exited silently on EOF.
Verified end to end: /etc refused as too broad, $HOME refused pending the flag,
a project directory recorded, a granted root indexes, and a non-granted root is
refused naming the exact command that would allow it.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Two loose ends from the bounds work.
The decompression side already took size_t so a >2 GiB capacity could not wrap
through int, but compression still took int for both the source length and the
destination capacity, and the artifact export cast a size_t database size down
to reach it. A database past 2 GiB would have handed the encoder a negative
length. Both lengths and the bound helper are size_t now, and the function
returns int64_t like its decompressing counterpart.
The discovery walk now prunes the cache directory by absolute path. A custom
CBM_CACHE_DIR may sit inside a repository — tests do it routinely — and walking
into it pulls every other project's graph database into this project's file
list. This is the narrow form of a concern that was briefly implemented as
refusing any root that contained the cache; refusing a whole root was too blunt,
and not walking the cache is what the concern actually asks for.
The test fails without the prune: a .go file planted under the cache is
otherwise discovered, and "cache" is not in the built-in skip list, so the
assertion is not vacuous.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
confidence, strategy and via are not part of the edge dedup key
(source:target:type), and the same logical edge is legitimately minted by more
than one strategy carrying different values — pass_calls emits CALLS edges from
both LSP resolution and registry-textual matching. cbm_gbuf_insert_edge
replaced the stored blob unconditionally, and per-worker edge buffers merge in
worker-slot order, so which strategy's attributes survived was a function of
thread scheduling: the edge set stayed stable while its attributes flickered
run to run.
Replace the overwrite with a total order over the two candidates, which makes
the result commutative and associative and therefore independent of arrival
order:
1. higher "confidence" wins, which also enforces the intended precedence
that an LSP-resolved call outranks a textual match;
2. on equal confidence, the lexicographically greater blob wins, giving a
deterministic choice between otherwise equal candidates.
An empty or absent incoming blob still never displaces stored properties.
Adds three tests: order independence for the same pair of blobs inserted both
ways, the higher-confidence winner regardless of arrival, and the existing
empty-blob guard. The first two fail before this change.
Signed-off-by: Andrew Studnicky <a.j.studnicky@gmail.com>
collect_config_entries and collect_code_entries fill fixed-capacity arrays
and stop at max_out. They walk gbuf label indexes in insertion order, which
under parallel extraction is worker-merge order and varies run to run, so on
any repository with more candidates than the cap the surviving set — and
therefore the emitted CONFIGURES edges — depended on thread scheduling.
Sort candidates by a pure content key (qualified_name, file_path, start_line,
name) before the cap applies, so the surviving set is a function of the inputs
alone. Tie-breaks are content-only: node ids are handed out in merge order and
belong in no canonical comparator.
Also log configlinker.truncated when a collector fills to capacity, so the cap
reports itself instead of dropping candidates silently.
Measured on a 4191-file TypeScript corpus (12940 candidates against the 8192
code cap): before, 3-61 CONFIGURES edges differed per run against a
single-threaded reference and no two multi-threaded runs agreed; after, the
single-threaded reference and four multi-threaded runs are byte-identical.
Corpora below the cap were already stable, and all divergence was CONFIGURES.
Signed-off-by: Andrew Studnicky <a.j.studnicky@gmail.com>
