Windows install/daemon runtime directories created by an elevated process are
owned by the built-in Administrator ACCOUNT (RID 500 under the local machine
account-domain SID), even when that account is renamed or disabled. cbm's
directory-owner/ACL trust refused it, breaking install/activation on such
machines.
#1705: resolve THIS machine's built-in Administrator SID via LSA
(LsaOpenPolicy(POLICY_VIEW_LOCAL_INFORMATION) +
LsaQueryInformationPolicy(PolicyAccountDomainInformation) ->
CreateWellKnownSid(WinAccountAdministratorSid, accountDomainSid)), cached once
per process, and accept it as a trusted owner/grantee in the daemon
(win_sid_trusted) and the CLI activation transaction. Resolution is
deliberately NOT IsWellKnownSid(WinAccountAdministratorSid) and NOT a
trailing-RID-500 match: both accept a FOREIGN S-1-5-21-*-500 (a domain
administrator, or another machine's built-in Administrator), which would be a
cross-machine trust escalation. Any LSA or synthesis failure caches "none" and
grants no tolerance (fail closed). owner-not-current-user (the exact-owner
target rule) is unchanged.
#2023/#1686: an owner-trust refusal now names the offending owner SID (the path
was already carried), so a terse "status -3, os 0" becomes actionable.
Scope notes: on a domain controller -- which has no separate local SAM, so its
account domain IS the AD domain -- this trusts the domain Administrator, by
design (on a DC that account is the machine-equivalent top admin). An
un-sysprepped cloned Windows image shares the machine account-domain SID, so a
sibling clone's built-in Administrator would be non-foreign; this is inherent
to machine-SID identity.
Verified on a native Windows (arm64) VM: compiles under the sanitized build;
the daemon_ipc SID-trust test passes (local RID-500 trusted, a fabricated
foreign S-1-5-21-*-500 rejected, BUILTIN\Administrators trusted) at 35 passed /
0 failed with the fix, and 34 passed / 1 failed (ASSERT(local_ok)) with the
trust clause reverted -- the test binds precisely to the fix. #1856 (stock
ancestor ACEs): the Windows ancestor-ACL guards already pass; no defect
reproduced, no code change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
apt.llvm.org rotates its per-major noble snapshot channels; the
llvm-toolchain-noble-22 channel transiently emptied, breaking every lane that
installs clang from it. #2124 already moved Dockerfile.msan (clang-22 ->
clang-21) for the same reason. The two GitHub-hosted lanes that install from
apt.llvm.org were still on 22 and would break identically on the next
rotation: _test.yml test-diag (ASan+UBSan) and _lint.yml lint-mem
(clang-analyzer, clang-tidy). Repin both to clang-21 / clang-tidy-21 /
llvm-toolchain-noble-21 (the channel #2124 verified apt.llvm.org serves on
amd64+arm64). The macOS LSan lane's Homebrew llvm@22 pin is intentional and
unrelated to apt churn (left unchanged); the clang-format-20 CI-lint lane is
untouched.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The test-msan lane was red repo-wide: the Dockerfile.msan image build could
no longer install clang-22 from apt.llvm.org's noble-22 channel, failing with
"E: Unable to locate package clang-22 / libclang-rt-22-dev / llvm-22" after
the 5 apt retries, so the image never built and every PR's required test-msan
check could not go green (#2123).
apt.llvm.org keeps only the last few majors for noble and rotates newer ones
in and out; the -22 channel's package index was empty when CI fetched it (the
channel has since flapped back, confirming the transience). Rather than stay on
that volatile channel, repin to clang-21 - the highest well-established stable
noble channel apt.llvm.org currently serves (verified installable on both
amd64 and arm64):
- apt source noble-22 -> noble-21, clang-22/libclang-rt-22-dev/llvm-22 ->
clang-21/libclang-rt-21-dev/llvm-21, and the clang/clang++ symlinks
- MSan runtimes clone llvmorg-22.1.0 -> llvmorg-21.1.8 (matches the apt
clang 21.1.8, keeping the libc++/libc++abi/libunwind build at the
compiler's major, as the layer requires)
- MSAN_SYMBOLIZER_PATH /usr/lib/llvm-22 -> /usr/lib/llvm-21
Verified in an ubuntu:noble container that clang-21/libclang-rt-21-dev/llvm-21
install cleanly, the clang/clang++ symlinks resolve, libclang_rt.msan-*.a and
llvm-symbolizer are present, and the full image builds via Colima.
Infra only; the MSan lane's other files (docker-compose.yml, scripts/msan.sh)
use the version-agnostic clang/clang++ symlinks and need no change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
On Windows, install registers the install directory in the persistent
HKCU\Environment\Path, but uninstall never removed it — so every
install/uninstall cycle left its entry behind and the user PATH grew
without bound (a real test host reached ~8000 chars, emptying the
effective PATH and breaking tooling).
The reported non-ASCII "mojibake" is historical, not a current-code bug.
Both install paths already write the value through the UTF-16 registry
API: install.ps1 (shipped in each release archive) runs `cbm install`,
which calls RegSetValueExW(L"Path", ...) — git history shows no ANSI
(RegSetValueExA / SetEnvironmentVariableA) predecessor — and
setup-windows.ps1 writes via .NET [Environment]::SetEnvironmentVariable
(Unicode) with an ASCII-only install dir. The only path that handles a
non-ASCII directory is the wide-correct C path, so current code cannot
produce a garbled entry; the garbage the reporter saw was left by an
older binary. No code change is needed for the encoding itself.
The real defect is the missing uninstall-side removal. This adds
cli_remove_windows_user_path(), the counterpart to
cli_ensure_windows_user_path(): it opens the same key, walks the
';'-separated segments, and drops exactly the ones equal to the install
dir using the same case- and trailing-separator-insensitive comparison
the install `present` scan uses, so it removes precisely what install
would have de-duplicated. Every other segment is preserved
byte-for-byte, the value kind (REG_SZ / REG_EXPAND_SZ) is preserved, and
a registry error is a warning rather than a hard failure so a user is
never blocked from finishing an uninstall over a cosmetic PATH edit.
Removing the entry on uninstall also stops accumulation on reinstall: a
garbled historical entry defeats the install de-dup and gets re-added,
so taking our own entry back out keeps the value bounded.
Testing:
- tests/test_cli.c: a hermetic Windows unit test drives the append and
remove logic against a GUID-scoped scratch key via the existing
CBM_TEST_WINDOWS_USER_PATH_RUN_ID seam, never the developer's live
HKCU\Environment\Path. A non-ASCII install dir doubles as a mojibake
guard (a broken UTF-8->UTF-16 round-trip would fail the match and leak
a duplicate). It asserts append, idempotent reinstall, exact removal
from the tail and the middle, idempotent removal, and byte-for-byte
survival of unrelated entries.
- tests/test_smoke_fixture_contract.sh: a macOS-runnable source contract
that fails when the removal is absent and passes with it, proving the
wiring binds (verified RED-on-revert).
The Windows-only registry code is #ifdef _WIN32; its compile and the
behavioral unit test are verified by CI's Windows leg (the local Windows
VM is down). Pre-existing mojibaked entries from old binaries are a
separate one-time cleanup, out of scope here, and setup-windows.ps1's
own lack of an uninstall mode is noted as a possible follow-up.
Fixes#2117
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Review of the previous head reproduced a check-then-use window in the
parent-chain walk: the link was judged by name (fstatat) and then opened
by name (openat), with nothing tying the two to the same entry. An
LD_PRELOAD shim that swaps the link for one pointing elsewhere the moment
the judgement returns made both earlier heads follow the swapped link,
including one owned by a foreign uid under root; the post-follow target
check only bounds where such a swap can land.
The link is now never opened by name after it has been judged. On Linux
an O_PATH|O_NOFOLLOW descriptor pins the inode, fstat on that descriptor
makes the ownership decision and readlinkat(fd, "") reads the target text
from the same inode; on other systems the link is stat'ed by name before
and after the readlinkat and both must be the same inode with the same
owner. The walk then opens the text from the parent directory, so a
relative text resolves against the link's own directory exactly as the
kernel resolves it, and the target check runs on what that opens. The
readlinkat form is used rather than /proc/self/fd so the walk does not
depend on /proc being mounted. What is still done by path is resolving
the text itself, which the target check bounds; the comment says so.
Test: relative, ../, absolute and chained link texts resolve from the
link's directory with the process CWD moved elsewhere. The race itself
is exercised out of tree with the harness from the review thread.
Refs #1722
Signed-off-by: TheStranjer <791672+TheStranjer@users.noreply.github.com>
Review of the previous head found the global rule unsound for one caller:
cbm_mkdir_p also creates <repo_path>/.codebase-memory inside the checkout
being indexed, and git creates symlinks owned by whoever cloned, so a
repository shipping `.codebase-memory -> ~/.ssh` would have been followed
by the artifact writer. "User-owned" says nothing about "user-intended"
once the path root is a checkout.
cbm_mkdir_p keeps the strict walk (root-owned links only). The new
cbm_mkdir_p_ex(path, mode, policy) with CBM_MKDIR_FOLLOW_OWNED also
follows a symlink owned by the invoking user, and only callers whose
paths are rooted in the user's own configuration opt in: the installer's
agent roots, skills, instruction files and hook directories, the
JSON-like config editor's parent creation, the cache root, <cache>/logs,
the UI config directory and the DB parent. The artifact directory inside
the checkout and the ~/.local/bin creations stay strict; the activation
transaction re-walks the latter with its own no-follow policy anyway.
What a follow lands on is now checked too: the opened target must be a
directory owned by root or the invoking user and not world-writable
unless sticky, so a link swapped between the fstatat and the openat
cannot steer the walk into a directory an attacker controls or one where
anyone can pre-plant entries. This applies to root-owned links as well;
macOS /private/tmp is root-owned and sticky and still passes. Refusal
restores the first openat's errno.
Tests: the plain walk refuses an own link that the opted-in walk
follows; targets that are world-writable without the sticky bit are
refused; as root, a foreign-owned target and a user-owned link are both
refused, each followed by restoring ownership and walking again; the
identical `.codebase-memory -> <user-owned dir>` link is refused by the
plain walk and followed once opted in; cbm_artifact_export refuses a
checked-in artifact-dir link and leaves its target empty; the installer
test gains the euid-0 leg with home, link and target owned by a non-root
uid. Under euid 0 the tests demote their links to a non-root owner, since
root-owned links are trusted by both walks.
Refs #1722
Signed-off-by: TheStranjer <791672+TheStranjer@users.noreply.github.com>
The daemon handed every index worker aggregate/physical_job_limit (a
fixed quarter by default) regardless of how many jobs were running, so a
lone worker on a host like #1864 was capped at a quarter of the memory
the daemon itself was allowed — and with the budget now failing whole
(decision A) that fixed slice would have turned an idle host into a
guaranteed over_memory_budget. Decision 2 (#1997, #832): the slice is
decided at spawn time as aggregate / jobs active right then (the job
being spawned included), so a lone worker may use the whole aggregate
and concurrent jobs split it. The explicit CBM_MEM_BUDGET_MB override
keeps its precedence inside the worker (bf46b91f); the sub-byte-per-slot
guard is unchanged. The spawn line logs project, active_jobs,
aggregate_mb and slice_mb so a fail-whole verdict can be read against
the budget the worker really had.
Tests: sequenced admission proves first=aggregate, second=aggregate/2;
the racing 4-way admission test accepts any aggregate/1..4 slice (the
divisor is scheduling-dependent there); a scripted CLEAN worker
returning the over_memory_budget error response is passed through
verbatim on the first attempt — no recovery loop, no quarantine.
Refs #1997#832#1864
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
index_repository rendered every non-zero pipeline code as "Pipeline
failed. Check repo_path exists and contains source files", which sent
people debugging a path that was fine (#2020). Three outcomes now say
what actually happened:
- CBM_PIPELINE_ABORT_OVER_BUDGET (#1997, #832): status "error" with
reason "over_memory_budget", previous_index "preserved", the budget and
peak RSS in MB, and a hint naming CBM_MEM_BUDGET_MB / CBM_WORKERS /
excludes. The handler logs index.abort reason=over_memory_budget. The
CLI prints the same JSON and exits 1; a supervised worker still exits 0
with the response written, so the daemon passes it through verbatim
instead of entering skip-and-continue recovery.
- CBM_PIPELINE_ABORT_PRESERVE_DB: status "aborted_previous_preserved"
(retry; the previous index still serves).
- CBM_PIPELINE_PERSIST_FAILED: status "persist_failed" (disk/permissions).
The incremental test helper goes fail-closed on error responses so a
transient pre-publication abort surfaces once at the source instead of
cascading into dozens of downstream count failures.
Test: an in-process index over 64 files at a 1 MiB budget seam returns
the named error, leaves the project DB the same size on disk, and
index_status still reports the previous node count.
Refs #2020#1997#832
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The extraction memory gate was advisory: when a full back-pressure cycle
ended still over budget it latched "futile" and kept admitting files, so a
host that could not hold the graph ran on until the kernel killed the
worker (#1997, #832) and the supervisor then quarantined innocent files.
Now the worker that latches futility runs ONE confirmation cycle (collect
+ bounded naps). If resident memory drained, the gate re-arms as before.
If it is still over budget, the attempt fails whole: every worker stops
pulling, extract returns the new CBM_PIPELINE_ABORT_OVER_BUDGET, the
orchestrator discards the staging DB on that non-zero code like any other,
and the previously serving generation is left byte-identical. The verdict
is a plain return value so it travels as a clean-exit worker response,
never a kill. The abort flag is its own atomic, not the shared cancel
token, which belongs to client cancellation.
The abort/persist codes move to pipeline.h alongside the new one (callers
legitimately distinguish them); FORCE_FULL_REINDEX stays internal.
Tests: the futility test now asserts the new code (the confirmation adds
one cycle to its bound), and a new test proves fail-whole preserves the
live DB byte-for-byte, keeps its node count, and leaves no stage residue.
Refs #1997#832#1973#1864
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
After every publish the pipeline re-exports <root>/.codebase-memory/ (graph.db.zst + artifact.json) whenever an artifact already lives there: once a session indexed with persistence, or when the team committed it and every linked worktree checks it out. The watcher's dirty signature folded those files' (size, mtime) in, so each successful reindex rewrote them, the next poll saw a new dirty state, and the daemon re-triggered itself forever: index.supervisor.reap outcome=clean immediately followed by watcher.changed strategy=git, 100+ times in 15 minutes with index workers pinned (#1953). Nothing under .git moved and no source changed, which is why the reporters blamed their long hyphenated worktree branch names (the #1254 theory its own reporter retracted); the regression test builds that exact worktree and shows the branch is irrelevant.
Scope git status with the same :(exclude).codebase-memory pathspec the exporter's own clean-tree probe already uses for the same reason. Nothing under that directory is ever an index input (discovery skips it), so a change there can never require a reindex; the pathspec is CWD-relative, so a monorepo sub-package watch excludes its own artifact directory. Untracked and committed artifacts both looped before; a real edit and a real HEAD move are still detected.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Hermes `install` failed with `op=pre_llm_hook_install` whenever config.yaml
held a PLAIN scalar with a `>` or `|` anywhere after the value start —
`probe: kaomoji face >w< here` — while the identical string double-quoted,
or a mid-value `*`, installed fine. An agent-config failure aborts
activation, so the whole install exited 1 and user-PATH registration was
skipped although eleven other agents had configured cleanly. Bisected by
@ukind, key by key and persona by persona, down to two persona strings
(#1924, continuing #1631/#1656).
Root cause: yaml_sequence_line_has_unsupported — the document validator on
the mapping-sequence path that the hook upsert/remove uses — ran a second
scan after the shared yaml_range_has_unsupported_ex and returned
"unsupported" for `[`, `]`, `|` and `>` at ANY position of a plain scalar.
The mapping-body path (mcp_servers) never had that check, which is exactly
why the mcp op succeeded and only the hook op failed on the same file.
Fix: the positional rule the `*`/`&` fix (v0.10.4) and the quote fix
(#1631, v0.10.6) already use — an indicator counts only where a NODE begins
(range start, after a mapping colon, after a block-sequence dash).
Interior ones are text. `probe: >`, `probe: |`, `probe: >-` and
`probe: [a, b]` still begin a value and stay unsupported, byte-identically.
`[`/`]` get the same treatment on purpose: in block context they are
ordinary characters inside a plain scalar (`a: b[0]` parses as the string
`b[0]`), a value-start `[` still opens a flow sequence and stays refused
(the #1631 line: exact `[]` accepted, non-empty flow sequences unsupported),
and the mapping-body path already accepted them mid-value — leaving the
sequence scan stricter would keep the mcp-op-passes / hook-op-fails
asymmetry this issue is about. Known conservative residue, shared with the
`*` rule: a plain-scalar CONTINUATION line that itself starts with one of
these characters is still refused (nothing precedes it on its line).
Tests (tests/test_config_yaml_edit.c): three accept cases run both real
Hermes ops (mcp_servers + pre_llm_call) against a persona corpus with a
mid-value `>`, `|` and `[]` and require the original document to survive as
an untouched prefix; two pins keep value-start block scalars and a non-empty
flow sequence refused with the file byte-identical. RED on the unfixed
editor: 61 passed, 3 failed (`yaml_hermes_hook_upsert(&fixture) == -1`);
GREEN: 64 passed; RED again with the production change reverted.
config_yaml_edit 64 · yaml 73 · cli 306 — 443 passed, 0 failed (macOS,
ASan/UBSan test build).
Not in this change: the refusal still reaches the user as
`op=pre_llm_hook_install path=... (target: regular file, N bytes)` with no
reason. The editor collapses every one of its ~236 YAML_ERROR sites into
CBM_YAML_IDENTITY_EDIT_ERROR, so naming the refusing rule needs a detailed
variant with a typed failure out-parameter, the shape the Codex TOML hook
path already has (cbm_toml_codex_hook_failure_t +
record_agent_config_error_with_reason). Follow-up.
Fixes#1924.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
try_load_nested_gitignore() returned NULL as soon as a walk frame already
carried a local matcher, and that matcher was inherited by every child
frame. So only the SHALLOWEST nested .gitignore on a path was ever loaded;
any deeper one was silently dropped — an EMPTY storage/.gitignore was
enough to disable storage/dump/.gitignore's "*". On the standard Laravel
layout (storage/.gitignore above storage/<dir>/.gitignore) that meant
thousands of git-ignored cache/debugbar dumps were discovered and indexed,
which on the reporter's app grew to 25,638 discovered files against 6,682
git-visible ones and ended in an OOM kill (#1973, discovery half; also the
§2 root cause of #530).
Discovery now keeps a root->leaf chain of .gitignore links per directory
frame: every directory below the root loads its own .gitignore, a frame
without one shares its parent's link, and a path is judged by walking the
chain leaf->root with git's precedence — the deepest file with an opinion
wins (so a deeper negation re-includes what an ancestor ignores), and inside
one file the last matching pattern wins. The repository root's .gitignore
(merged with info/exclude) is the chain head, so the same precedence holds
between root and nested files. Cost is one match per .gitignore on the
path — O(depth) per entry, no rescans; the walk frame shrinks by the 4 KB
local-prefix buffer it no longer needs.
Tests pin the reporter's layout (empty ancestor + deeper "*"), a deeper
negation overriding an ancestor pattern, and three nesting levels with an
empty middle file. Measured on a Laravel app (koel) seeded with typical
compiled-view/cache artifacts: 2,885 discovered files with 450 git-ignored
-> 2,435 with 0; clean clones of linux, dotnet/runtime, koel and redis
discover byte-identical file sets before and after.
Fixes the discovery half of #1973.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
scripts/setup-windows.ps1 has 326 lines and its parse-coverage report read
"113-113,113-113,245-327" — the same line named twice, and an end line that
does not exist. Two separate faults, both in cbm_error_regions_push.
Past-EOF end line. A tree-sitter node that ends at column 0 stopped right
after the previous line's newline, so it holds no text on the row it points
at. Adding 1 to that row named a line past the end of the file whenever the
region ran to EOF. The node here is start=(244,2) end=(326,0). Clamp the end
to the row above when the end column is 0 and the node spans more than one
row.
Duplicate range. Line 113 carries two separate ERROR nodes, at columns 25-29
and 31-32, and each pushed its own range. A line range says nothing new the
second time. Drop a range that exactly repeats the one already open. The
drop runs BEFORE the cap check, so a repeat is never miscounted as a range
the cap threw away.
Only an EXACT repeat is dropped, never a range that merely overlaps. Each
range is judged separately afterwards by cbm_region_is_recovered, which asks
whether definitions starting inside that range cover it. Two ranges holding
the same numbers always get the same verdict, so dropping the repeat changes
nothing. Two different ranges do not. Merging 3-3 into 2-3 hands the wider
range's covering definition to an error that definition does not explain, and
a real parse failure then vanishes from the report.
That is not hypothetical. An earlier version of this commit merged on overlap
and broke perl_malformed_source_remains_partial_issue1838, the test added
with the Perl grammar refresh in 17b5a432. The malformed fixture produces two
ERROR nodes, at lines 2-3 and 3-3. Merged, the 2-3 range looks fully covered
by before_error and is removed, so parse_incomplete comes back false on a file
that plainly does not parse. That test now pins this boundary.
The real file reports "113-113,245-326".
Two tests, both proved RED first with the exact expected text:
coverage_repeated_error_line_reports_one_range_issue963 "2-2,2-2" != "2-2"
coverage_range_never_ends_past_the_last_line_issue963 "1-5" != "1-4"
Suites run on this change: parse_coverage 34, extraction 325, pipeline 264,
mcp 246, index_resilience 7 — all passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
The markerless stale-rendezvous recovery test reset the activation runtime
parent to NULL instead of restoring g_cli_suite_runtime_parent, so every
later cli test ran the activation probe against the developer's real
/tmp/cbm-daemon-<uid>. Restore the suite value per the contract at
tests/test_cli.c:61-65 and the sibling at :1310.
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
phase1_scan_functions() in pass_semantic_edges.c sorts the collected
Function/Method node pointers so the func order is deterministic. When
the graph has no Function or Method node at all -- a struct-only or
config-only project -- node_ptrs is never allocated and the call becomes
qsort(NULL, 0, ...). The func_count <= 0 early-out lives in phase 1b,
which runs only after phase 1 has returned, so it never guards this call.
A NULL qsort base is undefined behaviour. glibc declares the parameter
nonnull, so UBSan on the Linux leg reports "null pointer passed as
argument 1, which is declared to never be null" at the call. The macOS
SDK carries no such attribute, so neither the default sanitizer build nor
the Homebrew LLVM diag lane can flag it there. The existing
pipeline_docstring_go_class test already reaches the call on Linux; the
harness never gated on recoverable UBSan reports, so it went unnoticed.
Guard the sort on func_count > 0. The new test
pipeline_semantic_edges_no_functions indexes a Go struct-only fixture
through the full pipeline and pins the fixture to what it claims
(Struct >= 1, Function = Method = 0). On the Linux leg with
UBSAN_OPTIONS=halt_on_error=1 the pipeline suite aborts at the qsort
call before the fix and completes with it.
Distilled from #1810 with co-author credit.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Co-authored-by: Yusuf S. Demirci <ysf-samet@outlook.com>
The head of the first pattern is bound whether the query names it or not:
execute_single falls back to the synthetic name "_n0". The capacity check
counted only names, so an unnamed head plus 16 named nodes passed it and
binding_set then dropped the 16th name without a word — the same silence
this guard exists to remove.
Count that slot, and name the synthetic variable once instead of twice.
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
The complexity pass (transitive_loop_depth / recursive) walked Function and
Method nodes in temp-id order and each node's CALLS targets in adjacency
order. Under parallel extraction the temp ids come from one shared atomic
counter, so id order is worker-scheduling order and differs run to run. The
cycle guard flags whichever member of a mutual-recursion cycle the DFS enters
first, so `recursive` flipped between the members of a cycle across otherwise
identical multi-worker runs while the CALLS edge set stayed identical -- a
violation of the MT-byte-identical invariant.
Evidence: on a 66-file Python fixture of equal-sized mutual-recursion cycles
(24 pairs + 6 triangles, adjacent in the size-ordered work queue) a probe saw
3 distinct outputs in 5 runs at 4 workers. The new regression test is RED on
main 5/5 (a 4-worker run diverges from the single-threaded run on the first
cycle every time) and GREEN with this change.
Fix: order the traversal by content only -- seeds sorted by (qualified_name,
file_path, start_line), and each node's CALLS targets sorted the same way
before recursing, through one bump-stack scratch sized to the CALLS edge
count (a single upfront allocation that bails the way the existing calloc
path does). Semantics are unchanged: the DFS entry node of a cycle is the one
flagged, as before; which member that is now depends on the inputs alone,
identically for sequential and parallel indexing. SCC-based marking of every
cycle member is out of scope here.
Test: pipeline_complexity_props_independent_of_worker_order indexes the
fixture once single-threaded and six times with CBM_WORKERS=4 through the
same harness as the sequential/parallel parity test, collects every
Function's complexity props sorted by qualified_name, and asserts all
parallel runs are identical to the sequential run.
Distilled from #1925 with co-author credit.
Verification: build/c/test-runner pipeline complexity extraction (ASan/UBSan)
617 passed, 0 failed; revert-check RED 5/5 on main, GREEN with the fix;
make -f Makefile.cbm lint-ci clean; scripts/check-no-test-skips.sh OK.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Co-authored-by: Zhiyu <zhiyuzhang001@gmail.com>
Visual Basic 6 shares .frm with FORM and .cls with Apex/ObjectScript, so VB6
forms were parsed by the FORM grammar (0 defs) and class modules by the Apex
grammar (junk nodes, extract timeouts). Recognise the VB6 export header
(VERSION x.yy / VERSION 1.0 CLASS / Begin VB. / Attribute VB_Name) and report
such files as unsupported (CBM_LANG_COUNT) until a Visual Basic grammar exists:
new cbm_disambiguate_frm(), VB6 check added to cbm_disambiguate_cls() ahead of
its ObjectScript scan. Defaults on any doubt are unchanged (FORM / Apex).
Short-term step agreed in #721; the grammar follows separately.
Refs #721
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Tokarzewski <bartlomiej.tokarzewski@gmail.com>
The first cut on this branch (c1b9c451) respelled the session root and
the allowed root with forward slashes on the way into the session policy
(cbm_mcp_server_set_session_context) and at the three application.c sites
that canonicalize a root. CI's Windows shard then failed two tests:
tests/test_daemon_application.c:2256: ASSERT(sensitive_blocked)
tests/test_daemon_application.c:2380: ASSERT(sensitive_blocked)
Both set HOME to the cbm_canonical_path spelling of a directory - the
platform's native form, backslashes on Windows - and expect a session
rooted there to be refused for auto-index and for watch. The sensitive-root
and allowed-root containment walk (ws_is_ancestor_or_equal in
src/foundation/workspace.c) compares prefixes byte-exact, so a root that
now read C:/Users/... no longer matched a HOME of C:\Users\...: the
sensitive refusal was lost and $HOME was admitted. Respelling the policy
root was the wrong layer.
This commit keeps the policy roots in their canonical native spelling
(mcp.c and the three application.c sites are back to what main runs) and
folds the separator spelling at the one comparison the original defect
lives in: application_index_args_equal, where the running job's args are
compared with an explicit index_repository request's. The auto-index job
spells repo_path the way the session policy holds it; the handler spells
it with forward slashes. On Windows the two never matched, and the request
was refused as OPTIONS_CONFLICT instead of joining the job already running
for its root. The fold acts on the parsed copies only (yyjson_mut_obj_replace
with a cbm_normalize_path_sep'd string), so nothing the daemon stores
changes spelling and every other option stays exact.
Why this is correct on Windows without a local Windows run: the policy
spelling is untouched, so the containment path is byte-for-byte what main
runs there today; cbm_normalize_path_sep folds backslashes on every
platform, so the new fold is exercised by the new test wherever the suite
runs.
Tests: daemon_session_context_keeps_one_spelling_of_a_root asserted that
the policy respells a root with forward slashes. That was the wrong
contract: the policy's containment checks compare its spelling byte-exact
against HOME and the granted roots, so a respelled root is exactly what
stops matching them. It is replaced by
daemon_session_context_keeps_the_policy_spelling_of_a_root, which binds
the opposite: the policy stores the root as given. The comparison is exposed through
application_internal.h as cbm_daemon_application_index_args_equal_for_test
and bound by daemon_application_index_args_compare_repo_path_separator_equivalently:
a backslash and a forward-slash repo_path compare equal; a different path,
a sub-path, and a different mode still do not.
Refs #1726
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
