sys/wait.h does not exist on the Windows toolchain, so the unguarded include
broke the Windows test build outright — the suites never ran. The test body is
already skipped on Windows, so the includes belong behind the same guard, as
test_cypher.c does for the same pair.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Windows bundle contract pinned the literal
remote_head="$(vm clangarm64 "cd /c/cbm && git rev-parse --verify HEAD")"
so parametrising the checkout path for per-run isolation failed it, even
though the property it guards - capture the remote HEAD into a local variable
and compare it HERE, never nesting quotes through cmd.exe - still holds.
Match the shape with the path as \S+ instead. Verified the contract still
binds: replacing the capture with a constant fails it for the right reason,
restoring it passes.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
"Users" was matched the way credential directory names are — against every
path component. On Windows every user's files live under C:\Users\<name>, so
that refused every ordinary project path, including a CI runner's own
workspace. The Windows smoke job on the pull request caught it; the unit
tests could not, because they only ever run compiled on macOS here.
System trees (Windows, ProgramData, Program Files) now match only as the
first component below the drive, which still refuses anything inside them at
any depth. "Users" is handled separately as the tree root only, mirroring
POSIX where "/Users" is refused but "/Users/dev/projects" is not.
Tests cover the regression directly: C:/Users/dev/projects/app and
C:/Users/runneradmin/work/repo are allowed, C:/Users is not, C:/Windows/System32
is refused at depth, and C:/dev/Windows-app — merely named after a system
tree — is allowed.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Routes the MCP index_repository handler and the graph UI's POST /api/index
through a single decision function. The UI route previously checked only that
root_path was a directory, so an operator's configured boundary held on one
entry point and not the other; it now canonicalizes first and applies the same
policy, answering 403 with the reason.
The decision is two-tier, because a bare default-deny would refuse every
first run and a bare opt-in leaves the default open:
- Breadth is always enforced, with nothing configured. Filesystem, drive and
share roots, top-level system trees, the home directory itself and
credential directories are refused as indexing roots out of the box.
- Containment in a declared root applies once CBM_ALLOWED_ROOT is set or a
grant exists, and is evaluated first so a path outside a configured root is
reported as exactly that.
Three things the tests caught, each a real defect rather than a test fix:
- On macOS /etc, /tmp and /var are firmlinked under /private, so
canonicalizing "/etc" yields "/private/etc" and counted two deep — sailing
past a minimum of two, missing the very path being guarded. Depth now
discounts a leading "private" component.
- An earlier draft refused any root containing the cache directory. That was
over-claimed: the indexer only parses recognised source files and a graph
database is binary SQLite it would never extract. Refusing a whole root is
also the wrong remedy where the concern does hold — not walking the cache
is. Removed, with the reasoning recorded at the site.
- Rewording the refusal to "outside every allowed root" broke an assertion
matching "outside the allowed root", and that test's early return skipped
its CBM_ALLOWED_ROOT cleanup, leaking the variable into every later test in
the suite. The original wording is kept and guidance appended instead.
Worth remembering: these contracts match strings, not properties.
Docs updated in the same change, since both env-var tables said "unset
imposes no restriction" and that is no longer true: CONFIGURATION.md and
README.md describe the two tiers, and CONFIGURATION.md lists the always-
refused roots along with the two limits that matter — this constrains scope
rather than sensitivity, and the credential list is a denylist that raises the
cost of a mistake rather than closing the class. SECURITY.md's supported-
versions table was still on 0.8.x and now reads 0.9.x.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Linux legs of the local 3-OS ladder build into build/linux-arm64 /
build/linux-amd64, not build/c, so Step 5c died with
'missing binary: /src/build/c/codebase-memory-mcp' and took the whole
leg down. Steps 5 and 5b already pass CBM_TEST_BINARY (derived from
$BUILD_DIR) to their scripts; 5c did not, and its test hardcoded the
path.
CI never caught this because every CI leg uses the default BUILD_DIR of
build/c - the container legs are the only ones that differ, which is
precisely what the local ladder is for.
- test_worker_error_response.sh honours ${CBM_TEST_BINARY:-build/c/...}
like its sibling watchdog tests; the default keeps bare manual runs working
- scripts/test.sh passes CBM_TEST_BINARY to Step 5c, matching 5 and 5b
- test_hook_conflict_notice.sh carried the identical hardcoding and is
fixed the same way (local-only today, but wrong is wrong)
Verified: with the fix the test passes against an out-of-tree binary;
reverting the fix reproduces the ladder's exact failure (rc=2,
'missing binary: <worktree>/build/c/codebase-memory-mcp').
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Adds cbm_workspace_classify_root, the breadth half of the workspace
boundary. It answers "is this path obviously too broad or too sensitive to
index as one root", not "is this caller allowed to index it" — that second
question needs the user-level grant store, since it is the only input a
caller cannot write. The header says so, because a reader who mistakes one
for the other would draw the wrong conclusion about what is enforced.
Three rules, each doing what the others cannot:
- Depth below the volume. Two components on POSIX refuses every top-level
tree in one rule with no list to maintain. Windows and UNC count
drive/share-relative and require one, because there the first component is
already user space ("D:/repos") — the system trees that sit at the same
depth ("C:/Windows") are covered by name instead, which is tractable
because that set is small and stable.
- The home directory itself, which is two deep on macOS and Linux and so is
invisible to depth.
- Credential directory names, matched against every component so both
"…/.ssh" and "…/.ssh/keys" are refused. This list is deliberately additive
so extending it needs no design authority.
A path that is or contains the cache directory is refused outright: indexing
such a tree would pull every other project's graph database into this
project's index.
Classification order is load-bearing and commented as such. The home
directory normally contains the cache directory, so testing the cache first
reported every home as "holds the cache" and made it non-overridable, which
contradicts the intent that a person may override it. Depth precedes the
cache test for the same reason: "/Users" satisfies both and "too broad" is
the reason that helps the reader.
Takes home and cache as parameters rather than reading the environment, so
the policy is a pure function and the tests need no filesystem. Twelve table
tests; three of them failed on the first implementation and drove the
ordering fix above.
Not yet wired to any caller — deliberately, so the policy can be reviewed on
its own.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A pass over places where a bound was either miscalculated or missing.
UI response builders (src/ui/http_server.c) mixed the clamping helper
http_appendf with raw cursor indexing. Route the per-line separators and
quotes through the helper too, so the cursor cannot leave the buffer once
output saturates it, and terminate explicitly afterwards — the helper pins
the cursor and then writes nothing, which would otherwise leave the "%s"
reply with no terminator in range. The log buffer was budgeted at
LOG_LINE_MAX + 10 per line while JSON escaping can double every byte, so
size it for the escaped worst case. Job status entries now escape their path
and error fields, which were interpolated raw.
Cypher (src/cypher/cypher.c): the WHERE grammar descends once per nested '('
and once per NOT, so parse depth followed the query text rather than
anything bounded. Track depth on parser_t and refuse past 256 levels with a
parse error. Bounding parse depth also bounds the resulting tree, so the
recursive evaluator inherits the limit.
Gitignore (src/discover/gitignore.c): '**' retries the remainder at every
position and consecutive groups multiply, so match cost is exponential in
the number of groups. Thread a step budget through the matcher and give up
past 20000 steps, reporting no-match so a pathological pattern fails to
ignore rather than ignoring the wrong files. glob_match keeps its name so
recursion_whitelist.h still describes the functions that recurse; the new
non-recursive wrapper that seeds the budget is glob_match_bounded.
Call extraction (internal/cbm/extract_calls.c): extract_fp_callee recursed
once per applied argument, so stack use followed the parse-tree depth of the
indexed file. Rewritten as a left-spine loop. Reassigning node/nk before
continuing leaves the fall-through cases where the recursive form left them,
so behaviour is unchanged and the extraction suite is untouched.
CALLS props (src/pipeline/pass_calls.c): `cap - pos - PAIR_LEN` wraps once
pos reaches cap - PAIR_LEN and stops bounding the copy below. Guard
additively, matching the closing write in the same function.
Shell-bound paths (src/foundation/str_util.*): the three git shell-out sites
each wrap a repo path in cmd.exe-compatible double quotes, where %VAR%,
!VAR! and ^ remain active. Two carried private copies of the check that
rejects those three on Windows and the third used the bare validator.
Promote the stricter form to cbm_validate_shell_path_arg and route all three
through it, so the copies cannot drift apart again.
Tests cover the buffer bounds, the parse-depth refusal, matcher termination
and the validator. The bounds tests run in a forked child so a violation
surfaces as a signal instead of silently.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The end-to-end shell regression could not run on CI (recorded in the
previous commit), which left the fix without a gating test. Move the
notice DECISION out of main.c - which is not linked into the test
runner, and is why no C test could reach it - into src/cli/
hook_augment.c, which is. main.c now calls the shared API for both the
absent-daemon and build-conflict cases instead of carrying its own
copies of the strings, so the test binds the real production path
rather than a parallel one.
cli_hook_conflict_emits_stdout_notice_issue1388 asserts what the bug
actually broke: a build conflict yields a stdout systemMessage naming a
different build and pointing at 'daemon stop'; the absent-daemon notice
stays distinct and keeps pointing at 'daemon start' (which cannot heal
a conflict); and non-Claude dialects receive no bare stdout JSON, whose
channel it would corrupt.
RED-on-revert verified: restoring the pre-fix behaviour (conflict emits
nothing on stdout) fails this test alone, for the right reason
('conflict is NULL'). cli suite 258 passed, the local end-to-end shell
test still green against a seam-bearing binary, lint-ci clean.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The regression is green locally against a seam-bearing binary but never
raises the forced conflict on ANY CI leg, so it was reddening six legs
for a reason unrelated to the fix under test. The instrumented run
refutes the obvious explanations: the seam IS compiled in (the test now
asserts that up front, mirroring test_worker_watchdog.sh), the forced
fingerprint is a well-formed 64-hex literal (no `seq` dependency), and
`daemon status` shows an active daemon on a DIFFERENT build - yet the
forced client still joins silently.
Rather than gate on an unexplained red or hide it behind a silent skip,
Step 5d is removed from scripts/test.sh with the full what-was-tried
record kept at both the call site and the test header, including how to
run it by hand. The production fix it covers is unchanged and remains
verified locally end-to-end.
Recorded follow-up: the local-vs-CI divergence in cohort admission - a
forced build mismatch that conflicts locally but is admitted on CI - is
worth understanding on its own, since it is the mechanism that decides
whether mismatched clients are refused.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The conflict regression went red on six CI legs with an unhelpful
message: every probe produced empty stderr, which reads as "the notice
is missing" when it actually means no conflict was ever raised. Three
changes, all diagnosis rather than tolerance:
- Assert the binary carries the hook-client seam up front, failing with
the rebuild hint, exactly as tests/test_worker_watchdog.sh does for
the crash-orphan probe. A seam-less binary can never raise the
conflict, and that must be said plainly rather than inferred from a
silent backstop expiry.
- Drop the `seq`-derived fingerprint for a literal 64-char constant
(length-checked), removing a dependency on a tool whose presence
varies across the MSYS2/macOS/Linux legs.
- On backstop expiry, dump stdout, the forced fingerprint and its
length, and daemon status, so the next run names the cause instead of
requiring another round trip.
Still green locally (seam-bearing binary): the conflict is observed and
the systemMessage asserted.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The daemon's cohort join completes asynchronously after 'daemon start'
returns; on slower CI hosts the single probe landed pre-cohort, never
observed the conflict, and the seam guard failed the leg. Poll for the
asserted state (a probe observing the build conflict) with a bounded
backstop, clearing the throttled notice marker before each probe so the
stdout assertion holds on whichever probe first sees the conflict.
Daemon-start failures and backstop exhaustion now dump diagnostics.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Windows users hit 'active CBM sessions and operations could not be
stopped safely; no activation was committed' from install, uninstall
and doctor with ZERO CBM processes running - they rebooted, killed
phantom handles, and deleted runtime folders chasing sessions that did
not exist. The real failure was the activation transaction's Windows
ACL safety check refusing a directory carrying cross-account mutation
grants (commonly the stock Authenticated Users:(M) inheritance from a
drive root - reproduced on the Windows VM with a plain mkdir under C:\
and with an icacls-granted parent), and the refusal detail was recorded
internally but never shown.
cli_activation_diagnostic now prefixes the recorded refusal note - the
predicate, SID and path - plus one remediation line (remove the flagged
grant or use an owner-private directory). The sessions wording remains
for genuine stop/reservation failures, which record no note.
Adds a CBM_ENABLE_TEST_SEAMS setter for the refusal note so the
attribution is testable portably; the regression test asserts the note
reaches the diagnostic (and that the no-note path keeps the sessions
text). RED before the fix, GREEN after, RED again on revert. Verified
end-to-end on the Windows VM: uninstall/doctor against an ACL-tainted
tree now name the ACL check instead of sessions.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A hook client that could not join because the active daemon runs a
DIFFERENT build reported the conflict on stderr only - and stdout is
the only hook channel Claude Code surfaces, so in-session the result
was eternal silent skips. Worse, the connect-failure path classified a
conflicted daemon as ABSENT and suggested 'daemon start', which cannot
heal a build conflict. This is the rc.1-over-0.9.0 experience in
#1388/#1335: install the new build, the old warm daemon keeps running,
and every hook goes quiet.
Both failure sites now emit a throttled stdout systemMessage naming the
real state (active daemon runs a different build) with the actionable
step ('daemon stop', next command starts a matching daemon). The
version-cohort conflict path keeps its unconditional stderr detail; the
connect-conflict path now prints the formatted conflict to stderr too.
Test infrastructure: a CBM_ENABLE_TEST_SEAMS-only env override
(CBM_TEST_HOOK_CLIENT_BUILD) lets one binary present a foreign build
fingerprint as a hook client, so the conflict is reproducible without a
second build. tests/test_hook_conflict_notice.sh (scripts/test.sh step
5d, on the step-5 TEST_SEAMS binary) asserts fail-open exit 0 + the
stderr conflict + the stdout systemMessage with restart guidance. RED
before the fix (empty stdout), GREEN after, RED again on revert.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
An image-verification rejection logged daemon.client_image_rejected and
finished the worker WITHOUT sending a hello response - the only
admission-failure path that never answered the peer. The client
reported status "pending" indefinitely, indistinguishable from a slow
cold start, with the reason visible only in the daemon log.
The rejection now sends a CONNECT_REJECTED hello response carrying the
reason (fingerprint_mismatch / image_unverifiable) and an actionable
message, matching the version-conflict and capacity paths. The
version-conflict path already responds to unverified peers, so this
discloses nothing new to a same-uid local peer; admission stays
rejected either way.
Adds a CBM_ENABLE_TEST_SEAMS-only seam forcing peer-image verification
to fail, since the in-process harness's peer pid is OS-authenticated
socket credentials and always verifies against the service's own
active image.
Test: daemon_runtime_image_rejection_reaches_client_issue1383 - the
client receives CONNECT_REJECTED with the reason in the message. RED
without the response block (client times out), GREEN with it.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A project name failing cbm_validate_project_name made project_db_path()
return an empty path, which cbm_store_open_path_query passed to SQLite -
and SQLite opens "" as an anonymous temp database. The healthy temp db
then failed the integrity check (no projects table) and
quarantine_corrupt_store rendered ".corrupt.<hex>" from the empty
prefix: a RELATIVE path, dropped as a 4 KB file into whatever directory
the daemon was started from, on every such query. The caller only ever
saw a clean 'project not found', so nothing pointed at the litter.
Two guards, per the reporter's analysis: resolve_store_internal skips
the direct open on an empty path and falls through to the existing
fallback scan (which can still resolve legacy dbs whose internal name
predates validation), and quarantine_corrupt_store refuses an empty
path outright (belt-and-braces - nothing at such a path is worth
quarantining).
Regression test: tools/call search_graph with project "bad name" from
a temp cwd asserts the clean not-found error AND that no .corrupt.*
file appears in the cwd. RED before (litter created), GREEN after,
RED again on revert.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Cypher request model stored a single order_by expression; on
ORDER BY key1 DESC, key2 ASC LIMIT n the parser consumed only key1,
left ', key2 ... LIMIT n' unparsed, and the query silently returned the
entire result set (6326 rows / 117 KB instead of 5 on the reporter's
graph - a token flood straight into agent context).
The return clause now models up to CBM_CYPHER_ORDER_KEYS_MAX (8) sort
keys with per-key direction (Cypher semantics). Both sort sites -
rb_apply_order_by for RETURN and sort_bindings for the WITH pipeline -
compare key-by-key with later keys breaking ties. More keys than the
modeled maximum is a loud parse error, never a silently dropped
remainder. The no-ORDER-BY projection fast path is preserved via
order_key_count == 0.
Tests: parse-level (2 keys, per-key direction, LIMIT consumed;
9-key over-cap rejected), exec-level RETURN (limit kept, tiebreak
ordering, per-key direction), and exec-level WITH pipeline (limit kept).
All RED before the fix (row_count 4 instead of 2), GREEN after.
Recorded, pre-existing and unchanged: an ORDER BY key that is not part
of the projection is silently skipped (single-key main behaves the
same); sorting by unprojected properties needs hidden-column projection
and is a separate issue.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Installing 0.9.1-rc.1 over an existing setup deleted the user's working
Claude Code hook entries: when the on-disk hook script is not byte-owned
(a manual install embedding another binary path, or a user-modified
reminder), cbm_write_owned_hook_script_with_legacy correctly refuses the
rewrite (TEXT_UNOWNED) - but the failure branch then called
cbm_remove_claude_hooks / cbm_remove_session_hooks /
cbm_remove_claude_subagent_hooks, destroying the registrations for
scripts that still exist and work. The reporter lost their PreToolUse
Grep|Glob entry and all four SessionStart entries.
A refused script write now records the error and leaves settings.json
untouched; entry removal belongs to uninstall only (which keeps its own
removal path, cli.c:9622).
Regression test builds the reporter's state - unowned gate script with a
foreign BIN path, user-modified session reminder, 0.9.0-form settings
entries plus a foreign Bash hook - runs the full install, and asserts
every pre-existing entry and both script bodies survive byte-identically
while the failure is still reported. RED before this fix (entries
deleted), GREEN after, RED again on revert.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Regression coverage for #1234: Java interface and enum methods were
emitted as both a Method node and a duplicate top-level Function node.
The production fix landed via 87a0e3f (language-agnostic class-body
routing); this adds the missing test coverage so it cannot silently
regress:
- java_interface_no_duplicate_function_issue1234: interface methods
produce Method nodes only, zero Function nodes
- java_enum_dedup_preserves_calls_issue1234: enum methods dedup'd while
cross-class CALLS edges survive
- cp_interface_method_no_dup_function: convergence probe asserting the
surviving Method still carries its CALLS edge end-to-end
Distilled from PR #1327.
Co-Authored-By: Harshita Joshi <j.harshitaa06@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The x86-64 leg runs this lane without exclusions on purpose, to settle
which limits are architectural. It has now run, and it disproves part of
what the previous block asserted. That block claimed all seven excluded
suites "abort with stack-overflow". Five do. Two do not, and lumping
them together hid two different problems behind one rationale.
(A) stack-overflow, five suites: grammar_regression grammar_labels
pipeline lang_contract grammar_probe_e. Confirmed on BOTH arm64 and
x86-64 (CI logged 5), so it is not the aarch64 artifact an earlier
note claimed. The recursion guards bound DEPTH while the resource
exhausted is BYTES; that follow-up stands unchanged.
(B) cli: no overflow at all. On x86-64 it runs to completion, 253
passed / 5 failed, every failure in the install or activation path,
with "agent_config agent=OpenClaw op=mcp_install" above them. Green
on every other venue. MSan reported zero use-of-uninitialized-value
in it, so the exclusion costs no uninit coverage. Recorded as
undiagnosed rather than guessed at: the local lane is arm64 where
these suites hit (A) before reaching this code, so there is no
faithful venue to iterate in and each attempt is a ~30min round
trip. That is a follow-up with an owner, not a dismissal.
(C) incremental: an RSS BUDGET failure, 3054MB against a 2304MB limit
-- not an overflow either. MSan maps shadow (and origin) memory for
every allocation, so the budget cannot separate a leak from shadow.
FIXED rather than excluded: the assertion is now skipped under
__has_feature(memory_sanitizer) only, so the guard keeps its teeth
on every other platform, where inflating the budget would have
blinded it. The suite stays IN the lane.
Verified: with (C) fixed, incremental is 163 passed / 0 failed and ZERO
stack-overflows under the local arm64 MSan container -- so it never
belonged in the overflow list on either architecture.
msan-lane.sh no longer forces MSAN_EXCLUDE empty. That override existed
to ask the architectural question; it is answered, and keeping it would
re-red the gate for causes already recorded. Both venues now read the one
authoritative list in scripts/msan.sh, which still warns loudly that the
lane is partial.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
daemon_runtime_rejects_forged_identity_extension failed on the
macos-15-intel CI leg. The suite is untouched by this branch and no
daemon production code changed here; the leg flakes on main too, so this
is a pre-existing defect the lane surfaced rather than a regression.
The test asserted that transmitting the forged frame SUCCEEDS. It cannot
be relied on to. The forged HELLO is 149 bytes against the 137-byte
first-frame envelope cap, so the worker rejects it from the header and
closes without ever reading the payload -- deliberately, so that no
attacker-controlled bytes are read. send_frame writes the header and the
payload as two separate writes, so whether the payload write lands before
that close is pure scheduling: it wins on an idle host and loses on a
loaded 4-vCPU runner. Both outcomes ARE the rejection, so the transmit
result is now recorded and not asserted.
Anti-vacuousness is preserved rather than dropped: the test now asserts
it connected at all, and the drain check waits for the state it asserts
(wait_for_clients 0) instead of sampling the count once, so the bound is
a liveness backstop and never the verdict.
Verified by mutation, not just by passing: with the envelope cap raised
to 512 and the exact-length check removed -- a daemon that accepts the
forged identity -- the repaired test goes RED on ASSERT(rejected). The
mutation was reverted and the suite is 43/43 green.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The manifest's per-file sha256 loop was single-threaded -- tens of
thousands of file reads in sequence, the second-largest block of a
kernel-scale incremental run after publication (~20s by residual). The
hash helper is pure per-file work, so files now fan out across
cbm_default_worker_count workers on a stride; ASSEMBLY stays serial and
in discovery order, so the manifest bytes are identical to the serial
build's -- the exactness doctrine is untouched, only the wall clock
moves. Repos under 64 files keep the serial path outright.
A worker that fails to spawn leaves its stride to the calling thread,
so every index is hashed exactly once regardless of thread-creation
failures.
Pinned by a threshold-crossing test: a 72-file repo must route NOOP on
unchanged bytes -- which stands entirely on two parallel builds
producing byte-identical manifests -- and still classify a single edit
into the closure route.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Path-alias configs (tsconfig/jsconfig class) stop declining the closure
route. A config delta re-routes RESOLUTION for the files it governs while
touching none of their bytes, so those files join the closure directly:
they re-extract and re-resolve under the freshly loaded alias collection,
their surfaces come out unchanged, and propagation stops -- the depth-1
argument holds exactly as it does for source edits. Governed means every
discovered file under the config's directory; over-inclusion from nested
scopes is deliberate (safe direction), and the existing budget still
bounds the total, so a root config on a large repo correctly concedes to
a full rebuild.
Classification is now explicit rather than incidental: synthetic
manifest digests (git context, extension configs) decline as
semantic_input_changed; package-control files decline as
control_file_changed (pkgmap is global -- governed repair is unsound
there); alias configs -- recognized by exact match against the loaded
collection plus a basename fallback so a REMOVED config still
classifies -- seed the governed closure, whether changed, added, or
removed.
The existing tsconfig-alias convergence test becomes the proof: no
source file changes, the route asserts CLOSURE_REPAIR, and the caller's
CALL_REFERENCE must move from target_a.ts to target_b.ts to match the
fresh-full reference -- the exact case the legacy partial route silently
corrupted and binary routing paid a full rebuild for.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The routing follow-through on the +92% warm-reindex finding: a semantic
manifest delta no longer unconditionally rebuilds the world. The planner
recomputes exactly the changed files plus the recorded consumers of any
changed SURFACE, and the executor resolves them against cross registries
rehydrated from the persisted per-file surfaces -- the same registration
code a full build feeds from fresh parses, which is what makes the output
converge instead of drift.
Routing, in order: exact manifest match stays a no-op; a delta first
offers itself to the closure planner; every uncertain case declines to
the full rebuild that was yesterday's only behaviour. Declines: virtual/
config manifest entries, new files, ADDED definition names (yesterday's
graph cannot know who would resolve to a name that did not exist -- the
write-the-caller-first flow and shadowing both live here), missing or
undecodable surface rows, dependents outside discovery, and a budget of
30% of files with an 8-file floor (a percentage alone starves small
repos: 1 changed file in 3 is 33%).
Two structural facts carry the correctness argument. Per-file extraction
is a pure function of file content, so an unchanged dependent can never
be surface-changed in turn -- the closure is depth-1 by construction, no
fixpoint. And a body edit reserializes to the identical surface bytes,
so its closure is the file itself. The dependent set comes from one
indexed query over the previous generation's edges (structural
Folder/Project containment excluded -- a container is not a consumer).
The executor is the existing partial machinery, parameterized: re-parse
list = closure; inbound-edge snapshot/re-link keeps only sources OUTSIDE
the closure (sound because every referencer of a surface-changed file is
inside it by construction); cbm_parallel_resolve now receives real cross
registries built from stored-surface defs plus this run's fresh parses;
publication merges surviving surface rows with the re-parsed files'
fresh ones inside the same generation. The legacy test-only route
publishes no surface rows at all -- a stale row that satisfies a future
closure plan with yesterday's surface would be worse than the full
rebuild an empty table forces.
Tests pin route AND convergence together (route equality matters because
a full rebuild satisfies any convergence assertion vacuously): body edit
routes CLOSURE_REPAIR with node/edge/CALL_REFERENCE counts equal to a
fresh full index; REMOVING a definition keeps the closure route and
drops the dependent's stale CALL_REFERENCE -- the assertion the legacy
QN-keyed re-link could never pass; added-name, new-file and budget cases
decline; the existing Go content-change test now routes CLOSURE_REPAIR
with its convergence assertions unchanged, making it the Go-language
proof of the same machinery.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Second piece of closure repair. At the collect_all_defs seam -- the only
moment the per-file result cache is alive -- both drivers (parallel and
sequential) now serialize each file's CBMLSPDef slice to canonical JSON,
hash it, and hand the rows to the pipeline; cbm_pipeline_publish_generation
writes them into the staging store next to the manifest, so surface data
and graph always belong to the same generation.
Canonical bytes are the point: every field is written in fixed order with
an explicit null for absent strings (NULL and "" differ in the CBMLSPDef
contract -- receiver_type NULL means "not a method"), so byte equality IS
surface equality and the sha over the bytes is the early-cutoff key.
Registry-only labels that pxc_map_label drops but the name registry serves
(Field) are folded into the hash as a separate "reg" array, or renaming
one would slip past the cutoff.
Behaviour pinned in SUITE(pipeline): a fresh full index persists a
versioned surface row per file; a BODY edit republishes the identical
surface_sha; a SIGNATURE edit changes it. That pair of properties is what
the routing layer will stand on.
cbm_pxc_collect_all_defs gains an optional per-file prefix array -- the
flat all_defs[] otherwise loses the file boundaries the serializer needs.
CORRECTION to 6c22338's scope note: it claimed cbm_pipeline_publish_
generation was reachable only behind CBM_INCREMENTAL_TEST_API. Wrong --
dump_and_persist_hashes calls it on every production full index
(pipeline.c:1863); the grep that "verified" test-only reachability had
excluded pipeline.c itself. The predictable staging name WAS in the
production publish path, which makes that fix a real production hardening,
not a test-path cleanup.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
First piece of the closure-repair incremental route (the follow-through
on the +92% warm-reindex finding): a per-file lsp_surface row holding
the file's serialized cross-file definition set -- exactly what
pass_lsp_cross registration consumes -- plus the metadata the routing
decision needs: the surface sha (early-cutoff key: a body edit leaves it
unchanged, so no dependent recomputation is owed), a referenced-name
bloom (added-symbol trigger), and a governing-config context hash.
The store treats defs_json and the bloom as opaque; the codec lives with
pass_lsp_cross, which is their only writer and reader. A project with no
rows reads back as OK/0 -- callers treat that as "no surface data" and
route to a full rebuild, which is also how databases written before this
table existed upgrade themselves.
Table appears via the CREATE IF NOT EXISTS schema on store open, so the
raw dump writer needs no change: publication opens the staging DB with
the store right after the dump, which applies the schema.
Round-trip covered in store_nodes: batch upsert, ordering, binary bloom
with embedded NUL, NULL bloom, whole-row conflict replacement including
bloom removal, project-scoped delete, and the empty-project signal.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
cbm_pipeline_publish_generation built its staging database name by hand as
"<db>.stage.<pid>.<counter>", unlinked it, then wrote it. Any local process
can compute that name in advance, so a symlink planted between the unlink
and the write redirects the write to a target of the attacker's choosing —
an arbitrary-file clobber when the database sits in a world-writable
directory.
The same file already solves this correctly elsewhere: create_staging_path()
mints the name with mkstemp, so the file is created O_EXCL and we only ever
write one we made ourselves. Publication now shares it. The unlink-first
step goes away with the predictable name — it existed to clear a leftover at
a name we might reuse, and a freshly minted name cannot collide, nor can its
sidecars pre-exist.
SCOPE, stated precisely because the PR description overstates it: the only
caller of cbm_pipeline_publish_generation sits behind
CBM_INCREMENTAL_TEST_API, which is set in CFLAGS_TEST and never in
CFLAGS_PROD. The predictable name was therefore not reachable in a shipped
binary — production publication already went through create_staging_path.
This is removing a bad pattern from a test-only path before it can be
promoted, not patching a live user-facing vulnerability.
The regression test calls the function directly, because no pipeline entry
point reaches it in a production build. It does not try to win the race — a
test that has to win a race is a coin flip, not a gate. It asserts the
property that removes the race: canaries occupy every name the old scheme
could have chosen and all must survive publication. Verified both ways
rather than green-only: against the old code it reports "survived == 31,
expected PREDICTABLE_CANARIES == 32", exactly one canary consumed; with the
fix the suite goes 18 passed/1 failed -> 19 passed.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
scripts/ci/lint-mem.sh and scripts/ci/msan-lane.sh were committed at mode
100644, so the workflow step that runs them directly died with
"Permission denied" (exit 126). scripts/lint-mem-gate.py gets the same
treatment: it is invoked through python3 today, but it carries a shebang
and should not depend on that.
The exec-bit contract already exists to catch precisely this, and it did
not, because it derives its candidate set from `git ls-files -s '*.sh'`
-- tracked files only. A brand-new script is invisible there until it is
committed, so the check passes on the run where the defect is introduced
and only starts failing on the run that ships it. The window where the
contract is most useful was the one window it could not see.
It now also considers not-yet-tracked scripts by their filesystem mode.
Verified against the real defect rather than in the abstract: with
lint-mem.sh untracked and non-executable the contract reports
"_lint.yml:76 executes scripts/ci/lint-mem.sh directly, but its committed
mode is 100644", and passes once the bit is set.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
runtime_test_spawn_blocked_executable() used fork() + execl(). fork()
duplicates the parent's address space, and under a sanitizer that address
space includes a very large shadow mapping. On macOS the duplicate trips
the per-process memory limit and jetsam SIGKILLs the child before exec
ever gets to replace the image, so the child is already dead by the time
the parent checks it.
The visible effect was daemon_runtime_process_fingerprint_never_hashes_
replacement_path failing at ASSERT(setup) under TSan while passing in
every non-sanitized and ASan build. The diagnosis is direct rather than
inferred: waitpid() reported the child as WIFSIGNALED with WTERMSIG 9,
and DYLD_INSERT_LIBRARIES was unset, ruling out library-validation
refusal of the copied system binary.
posix_spawn() never copies the parent address space, so the child is
never charged for the shadow mapping. The file actions reproduce the
child-side setup exactly (close the ready-read and input-write ends,
dup2 the input-read end onto stdin, close the original), and exec
failure is now reported by posix_spawn itself rather than over the ready
pipe. The FD_CLOEXEC-on-successful-exec EOF that the parent uses to
detect a live child is unchanged.
This keeps the test running under TSan on macOS instead of dropping the
suite from the lane. Verified: daemon_runtime 43 passed under TSan and
under the ordinary build, and the full macOS TSan set is 940 passed /
3 skipped / 0 races.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
setenv/unsetenv do not exist on Windows, so the bug-repro runner failed to
COMPILE there -- this is the 'runner did not execute' failure on the
repro-windows CI job and the local Windows board alike. The compat layer's
cbm_setenv/cbm_unsetenv (already included via repro_harness.h) are the repo
idiom; test_pipeline.c uses them for the same variable.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
c_lsp_process_file carried __attribute__((no_sanitize("address"))) over the
entire C LSP orchestration, excluding every direct memory access in it from
ASan -- placed for a stack-pointer-to-registry lifetime bug whose witness test
also had its assertion discarded ((void)find_resolved), so nothing could ever
prove the bug gone. Flagged by the memory-diagnostics standards report
(private/C_MEMORY_DIAGNOSTICS_STANDARDS_REPORT.md, priority 2) as the single
highest-value ASan coverage gap in the repository.
This branch's C LSP registry rework resolved the underlying lifetime: with the
suppression removed, the full c_lsp suite passes under ASan+UBSan (760 tests),
including the formerly-failing template field type resolution, whose test now
asserts the resolved call instead of discarding it.
Honest limit: the original bug predates the branch and its fix is the branch's
broad registry rework, not an isolatable commit -- so the revert-proof is
main-vs-branch (main: suppression + discarded assertion; branch: neither),
not a single-change revert-check.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The sequential lsp_cross pass builds its shared per-language cross registries
in ctx->seq_cross_arena, which DELIBERATELY outlives the pass -- resolved_calls
and the registries carry borrowed strings that pass_calls still reads, and the
arena is destroyed only after all passes (the earlier freeing-here bug was a
pass_calls use-after-free, says the comment at the arena's creation).
But the per-file module-QN strings (def_modules[], malloc'd in
cbm_pxc_collect_all_defs and handed to every registrar as def_module_qn) were
freed at the END OF THE PASS -- the exact mistake the arena comment warns
about, one level down. Any registry-reachable structure holding one of those
pointers read freed memory in pass_calls.
AddressSanitizer caught it as a heap-use-after-free (strcmp in
cbm_pipeline_pass_calls on a string freed by the pass-end cleanup) on the
first-ever run of the real-repo determinism tier (linux/fs/xfs, 355 files) --
a tier no CI runner can execute because the corpus is local-only, which is why
it survived: bisect shows it predates today's commits (b020748 reproduces),
and main is clean on the identical suite.
Ownership now transfers to the ctx at the end of the pass and the strings are
released beside the arena, in the pipeline teardown and in test_parallel's
direct-drive harness. The parallel path is unchanged: it already destroys its
registries and module strings together, before any later pass.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Second LSan round from the Linux leg, both verified green in the container
(532 passed, 0 failed under LSan):
- dump_and_persist_hashes' two semantic-manifest abort returns leaked BOTH of
the function's strdups (db_path and db_dir, the latter otherwise freed only
further down). Same ownership rule as the previous fix: every exit releases
what the function allocated.
- test_parallel's sequential harness drives the passes directly and never
destroyed ctx->seq_cross_arena, which the cross pass fills with the shared
per-language registries (stdlib registrations included -- ~20MB per test).
Production's run_sequential_pipeline destroys it after all passes; the
harness now does the same.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Maintainer decision (option B) closing the Kotlin property-reference repro's
platform divergence at its root instead of tie-breaking the fallback.
`holder::handler` (parsed as navigation by the vendored grammar) produced a
USAGE edge whose target was chosen by the name-only registry fallback: with a
same-named property (Holder.handler) and function (Functions.handler) in the
project, the winner was registration order -- readdir order -- so Windows
(lexicographic NTFS) borrowed the FUNCTION while macOS happened to pick the
property. The graph's answer must not depend on directory enumeration.
The occurrence is now claimed by exact, receiver-typed resolution. Five links,
each of which was missing:
1. extract side: a Kotlin navigation member read is a semantic-reference
candidate at the member occurrence, so an LSP row can claim it. With no row
the join finds nothing -- but a candidate no longer falls back to the
name-only registry guess, which is the fail-closed direction this PR is
built on.
2. kotlin cross resolution: a property READ with a proven receiver emits a
CALL_REFERENCE row against the property. The property is not a callable
target, so the join can only produce USAGE, never a fabricated
CALL_REFERENCE. Value reads only; calls stay with the invocation machinery.
3. receiver typing across files: kotlin_resolve_class_name composed
<this module>.<name> for an unimported cross-file type, which can never
name a type defined in another file. A per-call unique-short-name map
(built from the project defs, ambiguous names fail closed) resolves the
annotation to the real registered QN. Hash lookup, no scans.
4. cross registry fields: pxc_map_label dropped Variable defs entirely, so no
cross registry ever saw a property. They now flow through (every language's
registrar filters by explicit label, so only Kotlin consumes them) and the
Kotlin registrar attaches them as fields of their receiver type,
hash-bucketed -- a per-type scan would be the registry-tail-scan quadratic
pattern.
5. def side: class-body variables now record their declaring class
(parent_class) -- previously only methods did. The QN stays module-level,
so this is additive metadata: the only structural parent_class consumer is
Method-gated (DEFINES_METHOD), verified in pass_definitions/pass_parallel/
pipeline_incremental.
Emission targets each field's REAL def QN carried through the field map:
kotlin class properties are minted with module-level QNs (proj.Holder.handler,
not proj.Holder.Holder.handler), so the composed form would name a node that
does not exist and the join would silently drop the edge.
The repro now proves the property edge on every platform for the same reason,
not by racing readdir.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
write_temp_file used fopen(path, "w"): text mode on Windows rewrites \n as
\r\n on disk, so the fixture bytes stop matching the source string the test
reasons about. Three Windows-only failures follow: the two semantic-manifest
tests compare cbm_sha256_hex(<string>) against the pipeline's hash of the
file, and the quarantine test compares byte sizes (17 != 18). test_helpers.h
and repro_harness.h already write "wb" for exactly this reason; this brings
the one holdout in line (cbm_fopen + "wb").
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
pipeline_test_set_mtime used utimensat(AT_FDCWD, ...), which does not exist on
Windows, so the whole Windows test build failed to compile. Set the same
instant through SetFileTime instead: FILETIME is 100ns ticks since 1601 -- the
representation cbm_path_info_utf8 reads back -- so the round-trip loses
nothing the incremental pipeline can observe. POSIX keeps utimensat.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Maintainer decision closing the last open RED from PR item 2.
`callback = handler` contains a genuine value occurrence of handler, and its
exact callable identity is proven -- the alias binding below could not be
created otherwise. The policy of this PR is proven exact value =>
CALL_REFERENCE, so that occurrence now emits a resolved reference row instead
of remaining an ordinary USAGE. Bare identifier RHS only, and the usage-carrier
rule in extract_usages.c is extended in lockstep, Python-gated -- a candidate
the LSP never matches would change nothing, but the asymmetry would be a trap.
The strategy distinction mirrors the argument path: a source name that is
itself a local alias claims lsp_callable_alias, so the local-shadow guard
admits exactly what it admits for arguments and refuses everything else.
In the graph the two occurrences inside `argument` -- the RHS and the alias
use in accept(callback) -- surface as ONE CALL_REFERENCE edge and zero USAGE
edges, because edge dedup is by (src, tgt, type) and collapsing multiple sites
between the same pair is its existing, intended behavior. That is precisely
what the fixture always asserted, so it needed no relaxation, only the comment
recording the decision.
Also: GCC rejects repro_call_argument_matrix_b's _Static_assert comparing
enumerators of two different anonymous enums (-Werror=enum-compare); clang
accepts it, so only the Linux leg saw it. Cast both sides to int.
repro_reference_precision + call_reference_contract: 100 passed, 0 failed.
This closes all five item-2 REDs.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
PR item 2. Four of its five REDs; the fifth is a policy question left open.
THREE SHADOW REDS -- a rebound name kept looking exactly bound.
An imported `callback` that Python has already rebound must lose exact callable
proof, or the graph reports a reference to a callable the name no longer refers
to. Three binders were invisible:
(callback := 0) module-level walrus
[(callback := 0) for _ in (0,)] walrus inside a comprehension
type callback = int PEP 695 type alias
The first two because a module-level expression statement is routed to call
resolution and never reaches the binding scanner at all, so no binder inside one
was ever seen. The third because the scanner had no branch for
type_alias_statement, and its generic recursion only descends -- an identifier
alone binds nothing.
The comprehension case needed care in the other direction: the whole node was
skipped, correctly, because its iteration variables are private to it. But an
assignment expression binds in the CONTAINING scope (PEP 572), which is exactly
what separates it from every other binder a comprehension can hold. It is now
scanned for walrus targets only.
PARENTHESISED CALLABLE ARGUMENT -- `accept((handler))` produced no reference.
The semantic row was recorded on the outermost parenthesised argument while the
usage carrier stopped at the parentheses and was never marked a callable-value
candidate at all, so the occurrence-exact join had nothing to join. A bare
identifier now climbs to the same wrapper python_direct_callable_attribute_site
already returns for the bound-method form, so `accept((handler))` and
`accept((service.handler))` agree on one occurrence instead of disagreeing.
That climb uses the cursor the caller already holds and checks the parent kind
before doing anything. Walking up with ts_node_parent instead costs one slow
fallback per identifier in the file, which the linearity guard in
test_extraction.c rejects -- correctly, and it caught exactly that here.
The fixture's expected span was also wrong: "((handler))" is the argument LIST,
one byte wider on each side than either party, and per argument it would collide
for any call with more than one argument. It now names "(handler)" -- the
occurrence both sides actually use. Production was genuinely broken; the fixture
was additionally wrong about where.
LOCAL CALLABLE ALIAS -- a proven alias was refused by the shadow guard.
`callback = handler` makes callback a local, so the usage is a local shadow, and
a shadowed usage may only claim a semantic reference through the alias strategy.
Python emitted lsp_callable_value_reference for every callable argument, so the
one case the guard exists to admit could never satisfy it.
A lexical binding that does not simply name the module symbol of the same
spelling is an alias introduced in this body, and only that now claims the alias
strategy. A module function referenced by its own name keeps the strategy it has
always had, so the guard still refuses everything it refused before.
STILL OPEN: the same fixture asserts no ordinary USAGE from `argument`, and the
right-hand side of `callback = handler` is a second genuine occurrence of
handler as a value. Whether that should become a CALL_REFERENCE (this PR's own
proven-value policy, making the count 2) or emit nothing is a graph-content
decision, not a defect, and is left for the maintainer.
repro_reference_precision, call_reference_contract, extraction: 371 passed,
1 failed (that fixture).
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
cbm_agent_installed_binary_path renders the expected uninstall content
with ~/.local/bin/codebase-memory-mcp.exe on Windows, so installing the
test fixture with a suffix-less path made exact-content removal of the
migrated auditor profile miss and fail test-windows shard 2/2.
Signed-off-by: Ivan Dergachev <dergachoff@gmail.com>
Martin Vogel