Re-stamp the runtime DACL only when it is actually wrong, and repair cache
children left unusable by the pre-v0.10.3 DACL regime.
The unconditional per-start re-stamp rewrote an already-correct security
descriptor and propagated it to children (#1601 counted eleven no-op
"Security change" USN records against one _config.db in a day), and every
rewrite is a window in which a concurrent atomic publish can be refused
DELETE on the destination (#1620). The old regime's PROTECTED,
non-inheritable ACE also left every child born unusable — the 0-byte
worker-log class behind #1416's diagnosis — so the secured directory now
walks its regular children and repairs any with an empty DACL or a foreign
owner.
The fast path is gated on the ADOPTION-level predicate, not the general
secure() check: lock-directory adoption (private_win_owner_only_dacl)
demands the exact protected owner-only single-ACE descriptor the stamp
writes, while secure() also admits SYSTEM/Administrators ACEs. A fresh
directory with an inherited DACL passed secure(), skipped the stamp, and
stranded every subsequent lock adoption — 59/77 daemon-suite failures on
the real Windows VM. With the ported predicate (SE_DACL_PROTECTED,
single non-inherited owner ACE, FILE_ALL_ACCESS/GENERIC_ALL) the same VM
runs 77/77 and the full suite 7346/0.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Codex sanitizes stdio MCP subprocess environments to the names listed in
env_vars. Since #1645 CBM_RUNTIME_DIR relocates the daemon rendezvous, so a
Codex subprocess that does not receive it looks for the daemon in the DEFAULT
location and never finds it — the same silent client/daemon split
CBM_CACHE_DIR caused in #1562. Both names decide WHICH daemon a process talks
to and are now forwarded unconditionally (forward-if-present semantics);
behavioural knobs (log level, workers, budgets) deliberately stay
unforwarded — that broader list remains #1664's open enhancement question.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
gotspatel's live opencode.json stores our entry with backslashes
(`C:\...\codebase-memory-mcp.exe`) while the installer compares its own path
with forward slashes — the same file on disk, refused over the separator
spelling, so op=mcp_install failed on a correctly-installed machine (and on
Windows the dead-path probe rightly reported the binary PRESENT, which turned
the mismatch into a hard refusal).
Ownership comparison now treats `\` and `/` as equal everywhere and folds
case on Windows only, where the filesystem is case-insensitive; POSIX
byte-exactness otherwise holds. The annotated entry that names this binary is
recognised as already satisfied and preserved byte-for-byte.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
PowerShell 5.1's `Set-Content -Encoding UTF8` writes a BOM, so real
Windows-authored Hermes configs start with EF BB BF — and both edit ops
failed content-independently (the reporter's 26-byte reproduction is their
23-byte file plus exactly this BOM; reproduced RED on macOS with the same
bytes, so the platform was never the variable).
The document read now validates past a leading BOM and yaml_doc_init treats
it as a prologue: the first line's structure starts after it, the key lookup
still sees our own section when the BOM immediately precedes it (guarded by a
dedicated no-duplicate-section test), and every edit splices interior ranges,
so the BOM survives writes byte-for-byte. Non-document inputs — keys, entry
blocks, identity scalars — keep the strict no-BOM rule.
Also makes the moved-entry cli test fixture platform-correct: the Windows
dead-path probe can only prove a fixed-drive path absent, so the Windows
branch uses one; a POSIX-shaped path is refused there by design.
Fixes#1656.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Two install failures with the same root theme — the entry we write is a
compatibility contract with the agent's parser, and both sides of that
contract needed repair:
goose (#1675): ExtensionConfig::Stdio declares `name` as a required serde
field with no default, and goose's loader silently drops entries that fail to
deserialize — install reported success and the extension was invisible. The
goose block now carries `name: codebase-memory-mcp`; the non-goose YAML
schema stays name-free. A CBM_CLI_ENABLE_TEST_API seam asserts the exact
block bytes per schema.
annotated MCP entries (#1630, the deferred field-merge): dbd20eaa recognised
an entry the client annotated ("enabled": true beside our command/type) but
could only leave it untouched, because replacing the whole entry would drop
the client's keys. config_json_like gains
cbm_json_like_replace_field_raw_if_unchanged — splice ONE member's value,
preserving every other byte (comments, ordering, client keys) — and the
upsert flow uses it on the two AUTHORIZED repair channels only:
- a relocating update (the entry names the previous managed binary), and
- the existing Windows dead-path probe, which previously fell back to a
wholesale rewrite and lost the annotations.
POSIX keeps its doctrine unchanged: a config-supplied path is never trusted,
so a moved-looking entry without that authority is preserved byte-for-byte
and install fails loudly (cli_editor_mcp_preserves_unrecorded_posix_absolute_
entries_without_probe holds). All repair/refusal paths are covered by tests
proven RED on the unfixed flow.
Fixes#1675.
Fixes#1630.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Four legal-YAML constructs from the reporters' real Hermes configs made
`install` fail permanently (any one of them aborted mcp_install and/or
pre_llm_hook_install):
- exact empty flow collections as values (`plugins: []`, `tool_choice: {}`)
— now validated key-only in both the mapping-body and sequence document
scans, mirroring the #1673 empty-mapping exception; non-empty flow
collections stay rejected.
- block sequences at the same indent as their mapping key (column-0 `- item`)
— item lines directly after a value-less key are structure, not malformed
keys, in the root walker, the key matcher, and the sequence mapping-range
walker.
- double-quoted scalars continued across lines with a trailing `\` — the doc
loader now precomputes per-line continuation flags; continuation lines are
value bytes every structural walker skips, and a document ending inside an
open continuation stays an error.
- mid-word quote characters in plain scalars (`LET'S`) — quotes are scalar
indicators only at a node start (range start, after `:`, after `-`),
exactly like the #1639 anchor/alias rule; real quoted values keep their
protection.
Byte-identity alone also froze users on canonicals older releases wrote:
galaxy's entry had `command:` unquoted, and the goose block gained `name:`
(#1675), so the existing entry was declared FOREIGN forever. An entry under
our key now repairs when it parses as a known prior shape (single command
line, or the pre-name goose block) with a codebase-memory-mcp[.exe] command
basename; anything else stays FOREIGN and the file untouched.
End-to-end: both reporters' full configs (iandol 15.6 KB, galaxy 15.2 KB) now
install with zero agent_config errors, every original line byte-preserved,
and the goose upgrade path rewrites the old block in place. Each construct
carries a distilled regression test proven RED on the unfixed editor.
Fixes#1631.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Two measured changes on the TypeScript corpus (81,397 files; v0.9.0 17.9 s,
before 44.1 s, after 37.5 s):
- cov_rebuild_shadow_graph upserted every directory segment for every
failure row — 13,243 parse-partial baseline files under one tests/
subtree meant ~80k redundant node/edge round-trips, 9.1 s of a 9.2 s
coverage_replace. An in-rebuild path->id map creates each directory once;
identical graph (edges deduped by unique key before, absent now).
Coverage block 9,162 -> 2,920 ms. Sub-block timings
(publish.timing.coverage: del/rows/prune/meta/commit + row_count +
detail_bytes) are kept — the caller-level number could not name the
culprit.
- JS/TS export_statement is an import CONTEXT only in its re-export forms
(source field, or a bare specifier list without a declaration). The old
is_export_of_declaration blacklist missed TS-only forms
(ambient_declaration, function_signature, module_declaration), running
declare-heavy subtrees (.d.ts, export namespace) behind inside_import:
suppressed usages + per-identifier ancestor walks. Positive detection
replaces the blacklist; +3,954 restored usage edges on the corpus,
nodes identical. (Measured perf-neutral here — kept for correctness.)
Suites green incl. store_nodes/edges/search, mcp, extraction, ts_lsp,
complexity, and the 53-language calls-breadth contract.
Refs #1669.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Four measured changes on the dotnet/runtime corpus (58,656 files; every
step's numbers below are from full-corpus runs on the same host, baselines
captured this session; v0.9.0 = 444.3 s):
1. Two-phase cs registry build (types -> finalize -> funcs), the same
pre-finalize linear-lookup disease fixed for Java: prepare 280 -> 140 s.
2. C# import-context carve-out: cs_import_types lists namespace_declaration
(for namespace-name mapping) and using_statement (C#'s RAII block — a
grammar-name collision), so EVERY namespaced C# file's whole body ran
with inside_import=true. That both suppressed ordinary usage extraction
under namespaces and sent every identifier through the ancestor-walking
import-binding check (tree-sitter's ts_node_parent re-descends from the
root, so wide files went quadratic: one 147 KB JIT torture file cost
490 s; 6.8 s after). Only using_directive / namespace_use_declaration
open an import scope now. Restores the suppressed usages:
edges 4,291,387 -> 5,869,093 (+37%), nodes identical.
3. The usages walker maintains call/import ancestry as enter/exit counters
on its explicit stack instead of per-node ancestor re-walks
(extract_usages.c had grown from 6 to 100 ts_node_parent calls since
v0.9.0; the two per-node gates are now O(1) with semantics preserved —
strict ancestors only, emit before self-count).
4. Registry short-name indexes replace the two remaining full scans:
cs_lookup_extension walked all 963k funcs per unresolved invocation
(now the existing free-func short-name iterator, first-match order
preserved via min-index selection), and cs_resolve_type_name's step-9
fallback scanned every type per unresolved name IN BOTH the builder and
per-file resolution (new type_short index in finalize, same
reverse-insertion ascending-order pattern, best-score ties keep the
first-in-registration-order winner). Builder 140,095 -> 500 ms; resolve
cross-LSP CPU 5.5M -> 317k ms (us_per_file_per_kdef 191 -> 11).
End state: 449.9 s wall (1.01x of v0.9.0) with +48.6% edges vs v0.9.0 —
per-edge cost 32% BETTER than v0.9.0. Extract's remaining 359 s is the
24 MB hugeexpr1.cs parse floor both versions pay.
Guarded by the complexity suite; cs_lsp/extraction/edge/lang-contract
suites green including the 53-language calls-breadth contract.
Refs #1669.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Cross-file LSP for Java rebuilt a type registry for EVERY file from a def
set the module filter reduces by a constant factor, not to a constant
size: the JVM filter branch includes every def sharing the file's
namespace, so per-file work tracks the corpus (measured defs_per_file
1,292 -> 5,031 in lockstep as defs_total went 179k -> 689k). That makes
the pass O(files x corpus_defs) — 87% of a Java index and the bulk of
the v0.9.0 -> v0.10.x slowdown.
Three changes, one architecture (the pattern Go/Python/C/C#/TS already
use):
1. cbm_java_build_cross_registry — the JVM def universe (Java + Kotlin,
for mixed source roots) built ONCE per run, sealed read-only, shared
across resolve workers. Wired into both pipeline.c and
pipeline_incremental.c.
2. Two-phase registration inside that build: all TYPES first, finalize
(hash buckets exist), then FUNCS. Func registration parses signatures,
and type-name qualification via cbm_registry_lookup_type is a LINEAR
scan until finalize — a single mixed pass measured 0.44 ms/def at
689k defs, a ~300 s sequential build that erased the sharing win.
Two-phase: 306 s -> 2.8 s. Stable partition order because overload
ties resolve to the first registered QN match.
3. cbm_run_java_lsp_cross_with_registry resolves each file against the
shared base through an overlay holding exactly THIS FILE's defs
(register_local_func_or_type_from_file). Own-file scope is the load-
bearing choice: patch_one_method refines signatures from the AST and
must write a private copy (its types live in the per-file arena, the
base is sealed), and any wider scope re-imports the quadratic — a
module/namespace-scoped overlay measured ratio 4.00 on the growing-
package corpus, own-file measures 2.00.
Elasticsearch corpus (46,477 files), same host, CBM_PROFILE=1:
v0.10.5 this change v0.9.0
wall 419.9 s 85.6 s (4.9x) 61.5 s
cross-LSP CPU 6,195,780ms 52,513ms (118x) 110,344 ms
us_per_file 207,448 1,770 3,722
us_per_file_per_kdef 300 2 —
nodes 693,100 693,100 —
edges 5,646,235 5,649,949 —
Cross-LSP CPU now beats v0.9.0. Nodes are identical; edges +0.066%,
consistent with the shared base resolving cross-package targets the old
per-file namespace/import filter could not see, plus source-order
independence from the two-phase build.
Guarded by the complexity suite's shared-package gate (RED at ratio 4.00
on the pre-change tree, 2.00 after — see the suite commit).
Refs #1669.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The per-file cross-LSP path rebuilds a type registry for every file. Whether
that is cheap or quadratic depends entirely on how many defs the module
filter leaves, and nothing reported it — so a filter that reduces by a
constant FACTOR rather than to a constant SIZE looked identical to one that
worked.
parallel.resolve.perfile_registry reports defs_per_file next to defs_total,
plus how often the filter failed. Measured on one Java tree:
1/4 corpus: defs_total=179033 defs_per_file=1292
full corpus: defs_total=689216 defs_per_file=5031
defs_per_file grew 3.89x while the corpus grew 3.85x — lockstep, so per-file
work is proportional to the whole corpus and cross-file LSP is
O(files x corpus_defs). filter_failed=0 throughout, which is why this was
invisible: the filter always 'succeeds', it just does not bound the set.
Diagnostics only; no behaviour change.
Refs #1669.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Formatting only, on lines added by the previous commit. Verified with the
repo's pinned Homebrew LLVM clang-format so this is the CI-canonical
result, and confirmed no unrelated file needed reformatting.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Finding the 0.10.x indexing regression took an 11-corpus A/B across two
release binaries, a subset-scaling series, and a per-pass exponent fit.
None of that should have been necessary: the pass that carried it,
cross-file LSP, was already timed and already logged its def count. The
numbers were there, just never normalised into something a reader could
judge.
Three additions, all shipped without a flag except the detailed curve:
- parallel.resolve.cross_lsp_cost — cross-LSP cost NORMALISED per file,
next to defs_total. Wall time cannot separate "big repo" from
"superlinear pass"; us_per_file can. Measured on one Java tree:
files=3710 defs=102845 us_per_file=35129 us_per_file_per_kdef=341
files=14833 defs=339866 us_per_file=106722 us_per_file_per_kdef=314
Per-file cost tripled while per-kdef stayed flat — the fingerprint of
work proportional to the whole corpus (files x defs). One grep on two
differently sized repos now answers what previously took a two-binary
bench.
- parallel.resolve.scan_cost — candidates visited per tail-match lookup,
plus fallback_rows, which the code has counted since #1085 but exposed
only to a test. On the same tree the tail scan reached 242M candidate
visits (n^2.04), worth seeing even though it proved cheap in wall time.
- cbm_scale_probe (foundation/profile.h) — samples cumulative elapsed at
1/8, 1/4, 1/2 and 1 of a pass's items and fits k in T ~ n^k, warning
once k reaches 1.35 in shipped builds. Wired into parallel_extract and
parallel_resolve.
The probe's limits are documented rather than oversold: it catches growth
WITHIN a run, and would NOT have caught this bug, whose per-item cost is
constant-but-large within any single run (it reported 1.26 while the
cross-corpus exponent was 1.86). That is exactly why us_per_item and
us_per_file are emitted alongside it.
Tests are deterministic by construction: the exponent fit is a pure
function fed synthetic points, and the checkpoint bookkeeping is asserted
directly. A test that proved the detector by generating a real quadratic
workload would be asserting on the scheduler.
No product behaviour changes; diagnostics only.
Refs #1669.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Since #1360 routed ordinary malloc/new through mimalloc on Linux, the
arena policy governs every allocation in the process rather than just the
bound sqlite/tree_sitter populations. cbm sets arena_eager_commit=0, so
mimalloc commits sub-ranges with mprotect(PROT_READ|PROT_WRITE) over a
PROT_NONE reservation, and each partial commit SPLITS the reserved VMA.
Measured on the Go corpus, Linux arm64, shipped binaries:
v0.9.0 10 mappings, at ANY worker count
v0.10.5 ~22k mappings, peak; the count tracks CONCURRENCY
(999 at 1 worker, 8460 at 4, 11965 at 18)
Two consequences, both of which #1654 reported from a 96-CPU/376 GB host:
the mmap/mprotect churn serialises on the kernel's per-process mmap_lock,
and the VMA count climbs toward vm.max_map_count, after which mmap fails
for ANY size -- so mimalloc reported it could not allocate 10 KB while
`free -g` still showed 246 GB available.
mimalloc's own default for this option is 2, meaning "eager-commit arenas
only on an OS that overcommits (i.e. linux)", precisely because commit is
free there until pages are touched. Overriding it to 0 opted Linux out of
the default written for Linux. Restore it on Linux only; every other
platform keeps the lazy setting, where commit is NOT free and the
upfront-memory reason still holds (Windows especially, #581).
Measured effect, same corpus and host, baseline build vs this build:
mappings 22450 -> 17312 (-23%)
wall 92.4s -> 92.6s (unchanged)
peak RSS 19.14 -> 19.22 GB (unchanged)
This is a partial mitigation, not a cure: the remaining ~17k mappings are
individual 64 KB-3 MB extraction buffers, each taking its own mmap (the
worker reserves ~40 GB of address space for ~19 GB of RSS). Pooling those
is the durable fix and is deliberately left out of this change.
Guard: mem_arena_eager_commit_follows_platform_commit_cost pins the
platform split so the Linux default cannot be silently opted out again.
Reproduction and controlled 2x2 (only vm.max_map_count varied) are
recorded on #1654.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`update` hands off to install.sh (install.ps1 on Windows) and prints the command
to run. It built that command from cbm_detect_self_path - the BINARY's directory
- and treated "I resolved my own location" as "the installer is beside me".
Those are different questions. install.sh is placed beside the binary by
install.sh itself, but a binary that was moved, packaged by a distro, or built
from source has no installer next to it. We printed the path anyway:
bash "/home/<user>/.local/bin/install.sh"
/usr/bin/bash: /home/<user>/.local/bin/install.sh: No such file or directory
Reported on discussion #1560 (#1632) by a user who was already three releases
deep in install trouble and had just been told, by us, to run a file that does
not exist.
`update` exists to tell someone how to proceed. Ending the interaction on a
command that cannot run is the one outcome it must not produce - and the
fallback text was already there and already correct, naming install.sh as
shipping in the release archive without asserting a path.
The probe goes through cbm_path_info_utf8 so a non-ASCII install directory
resolves on Windows, and rejects a DIRECTORY of that name, because `bash <dir>`
is not a command either. A symlink still counts: it is reported rather than
followed, and the shell runs it perfectly well.
The Windows branch gets the same treatment; it had the identical assumption
about install.ps1.
Reproduce-first and revert-checked: with the probe forced to return true - the
old behaviour - the new test fails with "a directory with no installer must not
be named as one", and passes once it is restored. cli: 276 passed.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Indexing writes an internal "<name>::missed" miss-graph row into the SAME db as
the primary project whenever a file parses partially. cr_store_has_exact_project
required `count == 1` over ALL rows returned by cbm_store_list_projects, which
does not filter those rows - so any project that had ever recorded a parse miss
failed validation, as SOURCE and as TARGET, and the whole feature reported:
project is not indexed
for a project that plainly was. There is no user-level workaround: a partial
parse is not something the operator controls, and re-indexing reproduces the
shadow row.
This is the same defect mcp.c fixed for list_projects in #1044 ("requiring
n == 1 over ALL rows made every project with a miss graph vanish"); the
cross-repo site never learned it. The fix ports that primary-row filter.
The single-primary requirement itself is deliberately kept: it is what proves
the db belongs to the project we were asked about rather than being a shared or
mislabelled store. Only "::" shadow rows stop counting toward it.
Reported by vitaliy-shatskiy in #1609, whose diagnosis named the exact function
and the exact reason.
Reproduce-first, and revert-checked both ways:
- the new test fails on origin/main with `ASSERT(!(result.failed))`
(tests/test_cross_repo.c), for the behaviour under test rather than a setup
error;
- it passes with the fix;
- reverting ONLY src/pipeline/pass_cross_repo.c and keeping the test brings
the identical RED back, so the test binds to the production change.
The existing pair without shadow rows is the control: those tests already prove
that path returns edges, so this cannot pass vacuously on a fixture that never
matched.
cross_repo 8 passed; pipeline 249 passed; store_edges 25 and store_nodes 67
passed - no collateral change.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`codebase-memory-mcp uninstall --help` removed the binary and every agent
configuration (#1038). It did the destructive thing to someone asking what the
command does.
The top-level dispatcher matches the subcommand at argv[1] and forwards the rest,
so its own --help check at src/main.c:1047 never sees argv[2]. Nothing downstream
looked either, and cbm_cmd_uninstall went straight to parse_auto_answer.
The guard is checked FIRST, before parse_auto_answer, so a `-y` elsewhere on the
line cannot auto-confirm the destruction we are trying to prevent. It prints real
usage, states plainly that the command is destructive, and points at --dry-run.
--help is the flag a person types precisely BECAUSE they are unsure what a
command does. It must never be the thing that destroys their install.
cli suite: 271 passed.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Two same-named symbols in different languages were collapsed by
suffix_match onto one winner, so get_architecture hotspots inherited
the other language's in-degree. unique_name (candidates==1) is #1572
and is left unchanged. JS/TS/TSX stay one family.
Fixes#725
Signed-off-by: rudi193-cmd <rudi193@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
posix_directory_parent_secure(current_fd) validates the directory we are ALREADY
IN, but the refusal message printed `component` - the child about to be entered.
Every reporter has therefore been sent to inspect the wrong directory:
#1537 read "ancestor '.cache'" when /Users/<user> was refusing
#1621 read "ancestor 'cbm-daemon-501'" when /private/tmp was refusing
Both inspected the named directory, found it clean, and said so. They were
right. #1537 has been open for weeks with the reporter repeatedly confirming a
correct `.cache` - including on v0.10.4 today - because we kept pointing at it.
The message now names the CONTAINING directory and says explicitly not to check
the component itself.
This fixes no permission logic. It changes weeks of talking past each other into
a report someone can act on in a minute, which for this class of bug is the
whole game: the refusal is invisible from outside, so the message IS the
diagnosis.
Found while reviewing the CBM_RUNTIME_DIR work; the original detail was added in
the earlier half of #1537 and named the wrong variable from the start.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The daemon/CLI rendezvous directory is created under %LOCALAPPDATA% (Windows) or
/tmp -- /private/tmp on macOS -- and every ancestor of it must pass the
private-directory walk. That ancestry is not always acceptable, and when it is
not, EVERY invocation fails, `config list` included, so the settings surface
cannot be reached either:
codebase-memory-mcp: secure daemon endpoint could not be created
#1623 narrowed the Windows side of this by admitting AppContainer package and
capability SIDs on ancestors, and named the remainder explicitly: a live local
group, Authenticated Users inherited from a secondary volume root, and orphaned
unresolvable SIDs still refuse, and "those need CBM_RUNTIME_DIR or a separate
change". #1621 is the POSIX shape of the same dead end -- /private/tmp/cbm-daemon-<uid>
refused with no way to move it.
There was no way to move it in a shipped build. The only relocation hook,
CBM_TEST_DAEMON_RUNTIME_PARENT, is compiled out unless CBM_ENABLE_TEST_SEAMS is
defined, so a test build started while the shipped build did not; CBM_CACHE_DIR
is no help either, because it moves the cache and never the rendezvous.
CBM_RUNTIME_DIR names the parent directory the rendezvous is created under. It
does NOT relax the check: the directory it names goes through exactly the same
validation as the default -- ancestors owned by you or root, not world-writable,
no allow-ACL; the rendezvous directory itself still forced to owner-only -- and a
value that fails is refused rather than silently replaced by the default. The
operator only chooses an ancestry that passes. cbm_safe_getenv never truncates,
so no half of an over-long value can become a runtime parent.
The override is resolved in cbm_daemon_bootstrap_endpoint_new(), the one function
every product endpoint goes through: the daemon, the MCP client, the local CLI,
the index worker, and the install/update/uninstall activation path in cli.c. No
call site can silently keep the default, and the detached daemon inherits the
value with the rest of its environment. An explicit parent still wins, so the
compile-time test seam and the lifecycle guards' isolated namespace behave
exactly as before.
Approach and variable name from #1576 by Leonardo trindade miranda, resolved one
layer lower so the activation path is covered too.
Refs #1574
Refs #1621
Co-Authored-By: Leonardo trindade miranda <tmonestudio@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Six reports describe the same dead end: the indexing worker dies, the
supervisor points at `logs/.worker-<pid>.log`, and the file is 0 bytes.
Nothing was ever flushed, so not one of #1070, #1130, #1132, #1133,
#1145 or #1450 is reproducible or attributable -- the hint says "crashed
on a file" and the file is never named.
Root cause is buffering, not the crash. The worker's stderr is redirected
to that log file by the supervisor, and every diagnostic goes through
`fprintf(stderr, ...)` with no flush. The C standard only promises stderr
is "not fully buffered"; the Windows CRT gives a redirected stderr FULL
buffering, which is why five of the six reports are Windows. A worker
that aborts, is SIGKILLed by the OOM killer, or is terminated after a
hang takes its whole buffer with it.
- cbm_log_set_crash_durable(): setvbuf(_IONBF) plus a per-line flush in
emit_line. Both, deliberately -- setvbuf covers every writer to the
stream including the plain fprintf startup errors that explain a
worker which never got as far as logging, and the flush covers the
case where setvbuf is refused because the stream was already written
to. Enabled for the worker role only, claimed in main() before the
process writes anything.
- cbm_index_worker_log_begin(): a startup header written and flushed as
the worker's first act -- version, build fingerprint, pid, repo path,
and the worker's own arguments. A control record rather than an info
line, so CBM_LOG_LEVEL cannot restore the 0-byte log.
- Under a crash-durable log the parallel extract pass logs every file
it starts, not just the first two rounds of workers, so the log ends
with the files that were in flight when the worker died. One line per
file, never per node; unchanged for every non-worker caller.
This fixes no crash. It converts six unreproducible reports into reports
we can act on, and every future one arrives with the run identified.
Test: a worker started with a fully buffered stderr -- the state the
Windows CRT hands it, forced on POSIX so the contract binds on all three
legs -- writes diagnostics and is then SIGKILLed. The retained log must
be non-empty, carry the header with the version, pid, repo path and args,
and still hold the line written after it. Reverting the production change
leaves that log at 0 bytes.
The probe kills rather than aborts because Darwin's abort() runs the
stdio cleanup handler and flushes the very buffer the repro depends on
stranding: under abort the reverted build still produced a populated log
and the test was quietly toothless. SIGKILL runs no cleanup, and is
literally #1070's death (signal=9).
Refs #1070
Refs #1130
Refs #1132
Refs #1133
Refs #1145
Refs #1450
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`cli <tool>` resolved its JSON arguments from stdin whenever stdin was not a
terminal and no --args-file, raw-JSON positional or --flag form was supplied,
and cli_slurp_stream reads to EOF. An ordinary automation caller never sends
that EOF: Node's child_process.spawn defaults to stdio ['pipe','pipe','pipe']
and the parent must call child.stdin.end() explicitly, which almost nobody does
for a command it is not writing to. fd 0 then stays open with no writer and the
read never returns. The reporter's shell script was still parked in fread(0)
fourteen minutes later.
`list_projects` advertises "properties":{} — stdin could never have carried
anything it accepts, so the read was pure deadlock with nothing to gain. Gate
the stdin path on the tool's input_schema actually declaring properties, in a
seam (cbm_cli_args_from_stdin_allowed) that main.c's resolution chain calls.
An unknown tool also stops blocking: dispatch rejects it by name and no stdin
content can change that verdict. Tools that do declare properties keep the
documented `echo '<json>' | cli <tool>` channel untouched, and interactive runs
are unchanged because isatty(0) already short-circuited them.
Measured on the pre-fix binary, stdin held open by a writer that never writes,
15-second watchdog: `cli list_projects` was killed at 15s with 0 bytes of
output, while `cli list_projects < /dev/null` returned in 3s with 90 bytes.
After the fix the same open-pipe invocation returns in 3s with 90 bytes.
Regression tests (tests/test_cli.c): the reported hang, the piped-argument
channel that must survive, the interactive path, unknown/NULL tool names, and a
sweep asserting the gate tracks the advertised schema across the whole tool
table so a future zero-argument tool cannot reintroduce this silently.
Revert-check: with the schema gate reverted to the pre-fix `return
!stdin_is_tty`, both tests fail on the list_projects assertion and the suite
reports 2 failed; with the gate applied, 272 passed / 0 failed.
Known limit, not addressed here: a tool that DOES declare properties, invoked
with its required flag omitted (`cli index_status` without --project), still
blocks on an open pipe. Distinguishing "arguments are coming" from "nobody will
ever write" needs a bounded wait, which is a public-surface decision (deadline
value, and the cross-platform pipe-readiness probe Windows would need) rather
than a mechanical fix.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Martin Vogel