#1954's config-symlink follow support stored its opted-in roots in a
_Thread_local char[4][4096] -- 16 KiB in the binary's static TLS block.
On glibc/x86-64 that block is carved out of every thread's stack, including
the parent-death watchdog thread, created with a deliberately tiny 64 KiB
stack (PARENT_WATCHDOG_STACK_SIZE, main.c). The extra 16 KiB pushed that
thread's pthread_create past the static-TLS + TCB + guard budget, so it
failed with EINVAL: client_start_parent_watchdog() returned false and the
stdio server exited before answering initialize. The parent-death watchdog
regression test (#406/#407, scripts/test.sh Step 5) went red on every Linux
x86-64 leg while macOS/arm64 stayed green -- its TLS is not stack-carved.
Move the buffer off the static TLS block: keep a _Thread_local POINTER
(8 bytes) and calloc the 16 KiB lazily on first add, freeing it in
follow_clear(). Per-thread isolation and the exact API/semantics are
unchanged; the watchdog thread's static-TLS budget is restored. main.c and
the watchdog path are untouched.
Evidence (local, Linux x86-64 gcc under QEMU + Linux arm64):
- PT_TLS MemSiz 59,889 B -> 43,505 B (-16,384 B, exactly the array).
- parent-watchdog test 10/10 red -> 10/10 green; red again on revert.
- origin/main 10/10 green on the same env (regression is PR-specific).
- config editor + cli suites: 522/522 ASan/UBSan (macOS),
523/523 ASan/LeakSanitizer (Linux arm64), no leak.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
cli_fd_capture_begin/end use POSIX fd redirection (dup/dup2/tmpfile) and
are called only by the uninstall tests, which are themselves #ifndef
_WIN32. Defined at top level, the helpers were compiled but unused on
Windows, tripping -Werror,-Wunused-function on the test-windows CLANG64
leg. Wrap the struct and both helpers in #ifndef _WIN32 to match their
callers. Test-only; no production change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
config_edit_path.c's edit_path_note is defined before the '#ifndef _WIN32'
follow-resolver block but only called inside it, so the Windows build compiled
it dead and failed under -Werror,-Wunused-function (pr-smoke windows-latest).
Windows never follows a symlinked config, so guard the recorder with the same
condition as its callers.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Every config editor (JSON-like, TOML, YAML, text) opens with O_NOFOLLOW
and refuses symlinks, so a link planted at ~/.cursor/mcp.json cannot
point a privileged writer at another file. Users keep exactly those
files as symlinks into a dotfiles checkout; the blanket refusal made
install and uninstall fail on them and, before the previous commit,
turned uninstall into data loss (#1954).
Decision C: a symlink the INVOKING user owns, resolving to a regular
file the same user owns, is followed — as an explicit opt-in of the
agent-configuration writers only. cbm_install_agent_configs and the
uninstall activation register the user's configuration roots (HOME,
XDG_CONFIG_HOME) for the duration of their work; a link whose own
directory is outside every registered root, or any link seen by another
editor caller, stays refused, so repository paths never follow.
One shared helper (src/cli/config_edit_path.c) runs the race-safe
sequence: lstat(link) → owner == invoking uid (a root process follows
root-owned links only) → realpath → the target's parent directory
opened as a dirfd (O_DIRECTORY|O_NOFOLLOW; owned by the invoking user
or root, not writable by others unless sticky) → openat(dirfd, base,
O_NOFOLLOW) → fstat: regular file, same owner, link count 1, and the
same (dev, ino) as stat(realpath). Each editor reads through that
validated descriptor, stages its temp file with openat(dirfd, …,
O_CREAT|O_EXCL|O_NOFOLLOW), runs its pre-publish snapshot comparison
against the resolved path, and commits with renameat on the pinned
dirfd, so the link itself is never replaced. Every other link is refused
with an observable reason (surfaced in the uninstall diagnostics):
another owner, root over a non-root link, dangling, a directory or
special target, a hard-linked or swapped target, a foreign or
world-writable parent. Whether the target must also stay under a root
is a pending user decision behind CBM_CONFIG_EDIT_TARGET_UNDER_ROOT
(default: anywhere the user owns). Windows never follows reparse points.
Tests: each editor suite gains a same-owner link edited in place under
the opt-in (link preserved and still pointing at the same target, temp
file staged beside the target, bytes identical to the same edit on a
plain file, no stray temp files) and keeps its refusal cases; the
JSON-like suite adds no-opt-in, outside-root, dangling, directory,
identity-skew and world-writable-parent refusals. The foreign-owner and
root-run refusals move the observer through a test seam (a test cannot
create a foreign-owned link without root; a real root run demotes the
link first). test_cli.c drives `uninstall -y` end to end through a
symlinked ~/.cursor/mcp.json.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A single agent configuration the editors refuse to touch (the reporter's
~/.cursor/mcp.json was a symlink into a dotfiles checkout) aborted the
uninstall activation BEFORE index and binary removal. `uninstall -y`
exited 1 and left a 300 MB executable plus the whole cache behind — the
data-loss half of #1954 — while the transcript still read "removed".
Agent-config failures are now collected instead of gating: every cleanup
runs, the indexes and the executable go, and the command ends with one
list of the files that could not be cleaned (agent, operation, path,
observed target) and a non-zero exit. The existing per-error stderr line
is unchanged.
Two tests encoded the old hostage contract ("binary must survive failed
cleanup") and move to the new one. The new test drives `uninstall -y`
against a sandbox HOME with a dangling ~/.cursor/mcp.json link and
asserts binary + index gone, exit non-zero, and the file named on
stderr; on main it fails at ASSERT(binary_gone). (#1954)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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>
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>
CI caught the macOS run exiting 1 AFTER the watchdog assertion passed
('ok: child exited after parent death' followed by exit code 1): the
kill_hard helper resolved the Windows pid with 'ps -W', an MSYS-only
flag. Under 'set -euo pipefail' the failing ps aborts the helper, the
'[[ -n ]] && kill_hard' statement inherits that status, and the EXIT
trap turns a passing test into a red job. Linux ps would fail the same
way. Guard the pipeline (empty winpid on POSIX, where the flag does not
exist) and restore the bare '|| true' tail on every cleanup kill, which
the original script had and my refactor dropped.
Signed-off-by: 周文瑄 <zhouwx1997@126.com>
The POSIX parent-death watchdog (#407) was excluded on Windows with the
comment "job objects handle this". They do not: the KILL_ON_JOB_CLOSE
job in subprocess.c only wraps processes CBM spawns itself. An MCP stdio
server is spawned BY the client as its child, and Windows never
propagates a parent's termination to its children, so a force-killed
client (editor crash, task manager, CI timeout) leaves the server
lingering forever blocked on stdin. The orphan pins SQLite WAL read
locks, blocking checkpoints (#1083) and feeding the delete_project
permission-denied chain from #914.
Windows has no reparenting to poll for, so instead of getppid polling
the watchdog opens a SYNCHRONIZE handle to the parent at startup and
waits on it: the kernel signals a process handle exactly once on
termination, and the held handle pins the process object, so PID reuse
cannot fool the wait. The 500 ms loop timeout exists only to re-check
g_shutdown, mirroring the POSIX poll cadence. On a signaled parent the
thread takes the same deliberate _exit(0) as POSIX: after the owning
client is gone, kernel handle reclamation is the only trustworthy
release for the daemon connection, file locks and the WAL read lock.
When no trustworthy parent signal exists at startup (snapshot failed,
reserved PID, parent already exited, or the handle cannot be opened),
the client keeps running and leans on the stdin EOF path - the same
fail-open choice as the POSIX initial_ppid <= 1 bail-out - while a
watchdog thread creation failure stays fail-closed. Workers are
unchanged: they are spawned inside CBM's own kill-on-close job, which
is the containment the old comment assumed everyone had.
The parent-watchdog shell test now runs on MSYS2 instead of skipping:
an MSYS bash wrapper launches the native server over an anonymous pipe
(an MSYS FIFO is not readable by native binaries), a writer helper
holds the write end open so the exit can only come from the watchdog,
and the kill resolves the child's Windows-physical parent (the pipeline
subshell, not the wrapper script process - the watchdog watches the
Toolhelp ParentProcessId) and TerminateProcesses exactly that one,
because MSYS kill -9 does not reliably terminate the Windows process
behind an MSYS pid.
Verified against the pre-fix binary: same test, same tree, the old
build leaves the server running after the parent dies; this build exits
within one watchdog tick.
Signed-off-by: 周文瑄 <zhouwx1997@126.com>
cli_uninstall_quiesces_active_cohort_before_removing_binary_and_index moved
HOME and CBM_CACHE_DIR into a temporary directory but left PATH alone. Agent
detection reads PATH first, so a real agent binary on the developer's machine
was found even though HOME pointed at the empty fixture. Uninstall then tried
to edit that agent's config file inside the fixture, failed because the fixture
never created one, and stopped before removing the binary and the index. The
test measures exactly that removal, so it failed.
On this machine the agent is Goose, at /Users/<me>/go/bin/goose. Same tree and
same built runner, only PATH differs:
goose on PATH 291 passed, 1 failed
goose off PATH 292 passed, 0 failed
With this change the suite reports 292 passed, 0 failed while goose is still on
PATH, so the fix is the redirect and not a quieter environment.
This only stops the suite from depending on what the developer has installed.
The abort it exposed is a real defect and is reported in #1954: one agent config
that cannot be edited stops teardown for every other agent and for the
executable and indexes.
Eleven other tests that call cli_activation_save_env leave PATH open the same
way. They pass today, so I left them alone rather than change what they
exercise without evidence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
check_projection_scope models declared names in a fixed 32-entry array
and skipped the check entirely when a query declared more. Nothing
bounded how many variables a pattern could declare, so the out-of-scope
refusal added by #1922 switched itself off on a wide query: the same
undeclared name was refused at 10 declared names and quietly accepted at
35, answering a column of empty strings.
Skipping was deliberate — a wrong refusal costs the caller a working
query, which is worse than the silence. The premise was wrong. A binding
holds CYP_MAX_VARS node variables and CYP_MAX_EDGE_VARS edge variables in
plain arrays, and binding_set and binding_set_edge drop anything past
those without a word, so a query naming more cannot be answered at all.
Its extra names bind to nothing and project as blanks, which reads as
"the graph holds no such data". The choice was never refuse-or-stay-quiet;
it was refuse, or answer wrong.
So bound the input. check_pattern_var_capacity counts distinct node and
edge variables across every pattern — they share one binding, and an
OPTIONAL MATCH pattern sits in the same array — and refuses beyond what a
binding holds, naming the limit and how to get under it. Leaving the name
off a node frees its slot, because an unnamed node takes none.
Bounding the input rather than each consumer also makes three other silent
drops unreachable: binding_set past 16, binding_set_edge past 8, and the
column collection in execute_default_projection. Those would otherwise
need an error path threaded up through every caller, since both binding
setters return void.
The two `< 0` branches in check_projection_scope stay, with comments
saying they can no longer fire. They keep the guard standing if either
bound ever moves.
Fixes#1995
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
