Every Windows guard shared ONE daemon. test_daemon_stability.py said each
section "runs under its OWN cache directory: daemon coordination is
cache-scoped" -- it is not. The rendezvous key is FNV-1a over a compile-time
domain string (service.c: cbm_daemon_rendezvous_key) under a machine-global
parent (win_default_runtime_parent -> SHGetFolderPathW(CSIDL_LOCAL_APPDATA),
which ignores the LOCALAPPDATA variable). So the guard's own `daemon stop`
and kill -9 sections, the sibling guards, and anything else on the runner
all met the same daemon, and a daemon wedged anywhere failed every check
after it. That is the shape of this week's test-windows-guards red on
#1874: lifecycle's `daemon start` connected, the UI-config handshake never
answered, and stability / hook_augment / ui_drive / cli_non_ascii went red
in a row with an orphan daemon killed at cleanup -- on a diff and a base that
were each green elsewhere.
CBM_RUNTIME_DIR is the only relocation hook (bootstrap.c, resolved in
cbm_daemon_bootstrap_endpoint_new for every product endpoint). Each stability
section now gets a private runtime parent next to its cache, created before
the binary runs -- the endpoint validates the parent and never creates it --
and the daemon inherits it from the client that spawns it. Proven on macOS:
a daemon on the default rendezvous keeps its pid across the whole guard
(three stops and a kill -9 inside it).
The reporting was hiding the cause as well:
* every SETUP FAIL printed nothing at all, and every RED printed a 300-400
character HEAD slice, which is the allocator preamble, never the error.
Excerpts now show the tail (1500 chars); setup paths print the daemon's
full output.
* a subprocess timeout raised on a parallel-client worker thread was lost
and the client reported as "rc=none" with no output; it is now kept as a
failed result with its partial output. A section-level timeout is a
named RED with the command and output, not a bare traceback.
* the MCP session's stderr went to DEVNULL; it now goes to a file that is
printed when initialize never answers -- and that case is a RED against a
running daemon, not a "setup" skip.
* exit 2 was documented and never produced. Only `daemon start` failing
raises it now (test_daemon_lifecycle.py owns that surface as a
regression, so the board loses nothing); main() runs every section and
names all of the failing ones.
* test_hook_augment.py / test_cli_non_ascii_arg.py print full stdout AND
stderr on their index-setup failures instead of 300 chars of stdout.
On POSIX (local iteration only) the namespace lives under /tmp: sun_path
caps the socket at 104 bytes on macOS and TMPDIR there is ~50 bytes deep.
Out of scope, noted: test_daemon_lifecycle.py, test_non_ascii_path.py and
test_ui_drive_listing.py still use the machine-global rendezvous; a too-long
CBM_RUNTIME_DIR on POSIX is refused (ipc.c sun_path check) with no
validation detail.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Follow-up to #1824, which mapped Blazor `.razor` to the C# host and extracted
`@page` routes from it. `.cshtml` is the same Razor syntax with the same C#
host and was equally unmapped: an ASP.NET Core MVC or Razor Pages application
produced no nodes at all for its entire view tree.
`@page` matters more here than it does for components. On a Blazor component
the directive is an optional annotation; in Razor Pages it is the directive
that *defines* a page, so the routable surface of a Razor Pages app lives
exclusively in this file type.
That is why the extension mapping alone is not enough. `cbm_path_is_razor()`
hardcoded a single `.razor` suffix, so a `.cshtml` file would have been
discovered and parsed but skipped by the directive scan -- present in the
graph, with none of its routes. Widened to a suffix table covering both.
Scope is deliberately the Razor family and nothing else:
- `.vbhtml` is excluded because there is no VB.NET language in this repo,
so there is no host to map it to.
- `.aspx` / `.ascx` are excluded because Web Forms is a different templating
syntax (`<%@ %>`, `runat="server"`) with no `@page` directive, so neither
the C# error recovery nor the directive scan applies.
Best-effort on the same terms as `.razor`: the C# grammar recovers the `@{ }`
and `@functions` blocks, and the surrounding markup lands in ERROR regions
reported through parse_partial.
Tests: the extension mapping; a Razor Page asserting the extracted route path
and method; and a negative case for a layout with no `@page`, since most
`.cshtml` files are layouts and partials and a spurious Route node per view
would be worse than the missing ones this fixes.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
CI's tsan and lsan-macos runs failed the mutation-guard suite on
ASSERT(cbm_file_exists(db_path)) at the guard-balance test, while the
behavior suite passed everywhere. The assertion ran after the test's
own teardown: cleanup_project_db() unlinks the same db path before the
assertion block, and the manage_adr branch above replants and
quarantines that very path, so the late file check observed the
test's cleanup, not the query-only resolve it was meant to pin.
Whether it tripped depended on unlink/visibility timing, which is why
it surfaced only under some schedulers and never in the single-suite
local runs.
Capture cbm_file_exists() into query_db_left_in_place right after the
query branch completes, before anything replants or removes the file,
and assert on that snapshot.
Signed-off-by: 周文瑄 <zhouwx1997@126.com>
search_graph, query_graph, trace_path, get_code_snippet, get_graph_schema,
get_architecture, search_code, index_status, check_index_coverage and
detect_changes were annotated readOnlyHint=false / destructiveHint=true
because resolve_store()'s corrupt-store recovery quarantines database
files - a mutation. That conservatism locked every plan-mode client out
of the read-only analysis surface (#1100 calls the same tools "eleven
read-only analysis tools"), since spec-compliant plan gates refuse tools
that are not declared read-only.
This closes the gap the annotation comment pointed at. The query-only
resolve now classifies a failed integrity check without mutating: no
mutation lease, no quarantine. A confirmed-corrupt database is reported
(a new STORE_RECOVERY_CORRUPT status; the tool reply names the corruption
and points at index_repository instead of a misleading "project not
found") and left in place for a write-side open to quarantine and
rebuild, exactly as before. Transient verdicts stay retryable. Write-side
opens (index_repository, manage_adr updates) keep the existing blocking
recovery, quarantine, and generation re-check semantics unchanged.
The ten query tools' annotations flip to readOnlyHint=true /
destructiveHint=false, making them callable under plan mode in Claude
Code, Codex, ZCode, and every other client that gates on the hints.
Tests: the annotations expectation table is flipped; the query branch of
the guard-balance and guard-denial tests now assert zero leases taken and
byte-identical files; the ghost-db test (#704) expects the corruption
message; the four write-side recovery tests (generation re-check after
lease wait, unique backup naming, publish failure, published-WAL
completeness) are driven through manage_adr update - the write path where
that recovery now lives.
Signed-off-by: 周文瑄 <zhouwx1997@126.com>
The test built a fixture with its own HOME and PATH, then asserted that
the whole install plan contained no path with "/plugins/" in it. Agent
detection does not stay inside HOME and PATH: cbm_find_cli also looks in
/usr/local/bin and, on macOS, /opt/homebrew/bin. A developer with OpenCode
installed there has it detected inside the fixture, and OpenCode ships a
real plugin file, so the plan legitimately carried a "/plugins/" path and
the assertion failed. On a machine without OpenCode the same assertion
passed, which is why CI never saw it.
Name the two directories the check actually meant — the fixture's Qoder
and Pi directories, neither of which has a plugin directory, so a planned
path under one would be invented. The check now tests the same thing
regardless of what the developer has installed.
No production behaviour changed. Searching well-known install directories
is deliberate, so that install finds an agent whose command is not on the
current PATH.
Fixes the failure at tests/test_cli.c:6859.
Local cli suite: 293 passed, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
A dotted callee whose first segment starts upper-case names a type. That
receiver chain is evidence, and the two name-only strategies threw it away:
they matched on the final segment alone, so Foundation's
URLSession.shared.data bound to a project's own PickedFile.data — at 0.75
confidence, with nothing in the graph to show the answer was wrong.
receiver_chain_admits() now requires the candidate's own parent segment to
appear somewhere in that chain. Calendar.utcGregorian.startOfDayUTC still
resolves to AuthDTOs.Calendar.startOfDayUTC, because the project really does
extend Calendar and Calendar is in the chain.
Three shapes pass through untouched:
- a callee with no separator, which has no chain to judge;
- a lower-case root, which names a value whose declared type the chain does
not show (vm.load, http.Get, os.path.join);
- a name in capitals with underscores, which is a constant holding a value
rather than a type. Measured: without this carve-out the gate refused
ISO_4217_URL.lower -> builtins.str.lower, which is correct. JSON and URL
carry no underscore and stay guarded.
The gate applies only at the two name-only exits of resolve_name_lookup.
import_map, same_module and qualified_suffix already carry real evidence and
are left alone.
Language agnostic by design: the registry holds no language, and every
language that writes receiver chains gains the same protection.
Fixes#1893
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
strtoll answers 0 for text it cannot read, and cbm_parse_duration handed that
0 straight into its subtraction. The report of this said the result was a
false duration of zero. It is worse than that. An unreadable START time reads
as 0, so the span reports the whole END time as its duration:
FAIL tests/test_traces.c:340: info.duration_ns == 1050000000,
expected CBM_DURATION_UNKNOWN == -1
That is 1.05 seconds of measured time for a span whose start was never read.
Nothing downstream can tell that number from a real one.
The fix keeps cbm_parse_duration exactly as it is. It is public, it is
declared in traces.h, and eight tests pin its answers -- 0 for a NULL
argument, 0 for an end at or before the start. None of that moves.
A companion carries the extra answer instead:
int64_t cbm_parse_duration_checked(const char *start, const char *end, bool *ok);
Both reads go through one small reader that follows src/main.c:1104 -- an end
pointer, errno, and a check that nothing was left over -- plus a refusal of a
leading blank, which strtoll would otherwise step over.
cbm_extract_http_info now uses the companion and writes CBM_DURATION_UNKNOWN
(-1) when the timestamps do not read. It still returns true, because the
method and path on that span are still good data and dropping them would
punish them for a fault they had no part in. -1 as "not recorded" is the
sentinel this codebase already uses for the same question, in
CBM_EDGE_CONF_ABSENT. The duration_ns field comment in traces.h now says so,
so a reader who meets -1 has something to read.
Scope note: cbm_extract_http_info has no production caller on this tree. rg
finds it only in src/traces/traces.c, src/traces/traces.h and tests, and
handle_ingest_traces in src/mcp/mcp.c never touches a timestamp. So this is a
defect in a public, tested function rather than one putting bad rows in a
graph today.
Two tests come with the change. The one that pins the behaviour was seen
failing first -- the FAIL line above is from that run. After the fix,
TEST_SUITES="traces" reports 32 passed, 0 failed, exit 0.
The full suite reports 7633 passed, 2 failed. Both failures are in
tests/test_cli.c (lines 1749 and 6725), print "error: one or more agent
cleanup operations failed", and reproduce on a clean tree without this
change. make -f Makefile.cbm lint-ci passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
atoi and atol answer 0 for text they cannot read, and 0 is a real setting at
three places in this project. So a typo, a trailing unit such as "30s", or a
stray space silently chose a value nobody asked for, and nothing on screen
said the setting had been dropped.
src/mcp/index_supervisor.c CBM_INDEX_WORKER_TIMEOUT_S
src/cli/hook_augment.c CBM_HOOK_DEADLINE_MS
src/mcp/mcp.c CBM_INDEX_MAX_RESTARTS
CBM_HOOK_DEADLINE_MS was the worst of the three. atoi answered 0, 0 is below
HA_DEADLINE_MIN_MS, and the clamp then handed back 50 ms -- the SHORTEST
deadline the setting allows, for a setting whose only purpose is to give the
hook more room. The comment above that function records a hunt for hook runs
that never finished, 0 of 24 real sessions, which is the exact symptom a
silently-shortened deadline produces.
CBM_INDEX_MAX_RESTARTS lost twice. A typo kept the default of 100, and
CBM_INDEX_MAX_RESTARTS=0 -- which reads as "do not restart" to anybody who
sets it -- also kept 100. The setting did the opposite of the request.
CBM_INDEX_WORKER_TIMEOUT_S fell through to the 15-minute default, so a test
set to give up after 30 seconds hung for 15 minutes with nothing to explain
why.
The fix adds one helper rather than three copies of the same check:
bool cbm_env_long(const char *name, long *out);
It answers true only when the variable is set, is not empty, and reads
cleanly from its first character to its last. It holds no policy -- no
minimum, no maximum, no default -- because the three sites disagree on all
three, and a helper that guessed would be wrong at two of them. The shape is
the one src/main.c:1104 already uses: an end pointer, errno, and a check that
nothing was left over. It also refuses a leading blank, which strtol would
otherwise step over, so " 5" is a slip rather than the number 5.
Each site keeps its own rule:
worker timeout an unreadable value keeps the 15-minute default AND logs
the value it dropped
restart cap 0 now means no restarts; an unreadable value keeps 100
AND logs the value it dropped
hook deadline an unreadable value now yields HA_DEADLINE_DEFAULT_MS,
not the floor. This one stays silent on purpose: the file
includes no log header and writes no stderr, because its
output is hook protocol.
The restart-cap parse was lifted out of a very large function into a named
index_restart_cap(), so it can be read and reached on its own.
Four tests come with the change. The two that pin the user-visible behaviour
were seen failing before the fix:
FAIL tests/test_cli.c:402: ms == 50, expected HOOK_DEADLINE_DEFAULT == 2000
(with "unreadable value \"abc\" gave 50 ms" printed above it)
FAIL tests/test_cli.c:434: cbm_index_restart_cap_for_testing() == 100,
expected 0 == 0
After the fix, TEST_SUITES="platform cli mcp" reports 518 passed, 2 failed.
The full suite reports 7635 passed, 2 failed. Both failures are in
tests/test_cli.c (lines 1826 and 6802), print "error: one or more agent
cleanup operations failed", and reproduce on a clean tree without this
change -- they depend on the coding agents installed on the machine.
make -f Makefile.cbm lint-ci passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
execute_return_star_after_with sized its column arrays at CYP_MAX_VARS
while the parser allowed up to CBM_SZ_32 items. A WITH naming 17 to 32
aliases parsed cleanly, then answered 16 columns and dropped the rest
with no error. The caller could not tell that from a query that genuinely
had 16 columns.
Widening those arrays to 32 is the wrong fix and would make the result
worse. A binding holds exactly CYP_MAX_VARS variables, and
with_add_vbinding_var drops any alias past the 16th on the way in. The
projection would then report 20 columns of which 4 are always blank —
silent wrong data in place of a silent short answer. The 16 was not an
arbitrary undersize; it matched what the binding can carry.
So bound the WITH where the RETURN is already bounded, in
parse_return_or_with, and refuse the query instead of answering it
short. The clamp in execute_return_star_after_with stays: it can no
longer fire, and it is what keeps that function correct if the bound
ever moves.
The test asserts both halves — a 20-alias WITH is refused, and a
16-alias WITH still succeeds with 16 columns — so the guard rejects only
what the binding genuinely cannot hold.
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
RETURN * built its columns from the variables the query pattern named,
never from the bindings it was about to project. One line caused two
separate wrong answers, and neither one reported an error.
After a WITH, the pattern's variables are out of scope — the WITH
replaced them with the names it made. The old code still asked for the
old names, found none of them, and answered a full result of empty
strings. This query used to print twelve columns of nothing:
MATCH (f:Function) OPTIONAL MATCH (f)-[:CALLS]->(g)
WITH f.name AS caller, g.name AS callee RETURN *
It now prints two columns, caller and callee, holding their values. A
name the WITH made holds one value rather than a node, so it gets one
column, not the four a node variable gets.
Separately, collect_pattern_vars appended every pattern's variables with
no repeat check. A variable named in two patterns got its four columns
twice, which the OPTIONAL MATCH above does with f.
Two tests cover both faults and fail against the old code:
cypher_return_star_dedups_repeated_pattern_var col_count 12, want 8
cypher_return_star_after_with_names_aliases col_count 8, want 2
Cypher suite: 185 passed, 0 failed. clang-format clean on both files.
Reported alongside a second fault this does NOT fix: a variable the WITH
dropped is still accepted afterwards and renders empty, because nothing
checks a projected name against the live scope. See
.agents/research/2026-08-29-cypher-return-star-and-with-scope.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
handle_calls reads a call's arguments through one tree-sitter field
lookup, ts_node_child_by_field_name(node, "arguments"). The vendored
Swift grammar declares no "arguments" field at all -- its own
ts_field_names[] table has zero occurrences, against one in Go and two
in TypeScript. Swift models a call as a target expression plus a
call_suffix, and the arguments hang off the suffix as value_arguments.
So args was null for every Swift call ever parsed, first_string_arg was
never populated, and no Swift HTTP call could raise an HTTP_CALLS edge
or a Route node. Alamofire, Moya and URLSession have been in the
service-pattern table the whole time and match the callee text
correctly; the URL simply never arrived.
Three changes, all in extract_calls.c:
- swift_call_args() reaches the argument list through call_suffix, in
the same shape as the existing objectscript_call_args() fallback and
used from the same place. A trailing closure has no value_arguments,
so it returns a null node and that call behaves as before.
- extract_url_or_topic_arg() unwraps Swift's per-argument
value_argument node, stepping past a leading value_argument_label.
Without it, dataTask(with: "/api/v1/widgets") yields the label "with"
rather than the path. PHP and C# already had the same unwrap for
their own "argument" node.
- is_string_like() gains line_string_literal, which is what Swift calls
an ordinary "..." literal. The list already held raw_string_literal,
so only Swift's common case was missing.
This is the layer underneath #1892 rather than the whole of it. A
literal URL argument now arrives; a literal nested inside a
constructor, as in URLSession.shared.data(from: URL(string: "...")!),
still does not, because extract_positional_url reads a literal, a
template string, a concatenation or a named constant and that shape is
none of them.
Reproduce-first: all three tests fail without the fix, two on a null
first_string_arg and one on HTTP_CALLS being 0.
Fixes#1892
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
Expose the compiled server version through the existing UI config endpoint and render it beside the product name. Keep older or unavailable config responses compatible by hiding the label, with focused frontend and native endpoint coverage.
Fixes#1820
Signed-off-by: Rares Popa <2606875+rarepops@users.noreply.github.com>
A file whose final line lacks its newline leaves the grammar's mandatory
terminator MISSING at EOF. #1610 suppressed that phantom parse_partial on the
grounds that the node is ZERO-WIDTH: the parser consumed no source for it, so
by construction nothing was dropped.
That suppression tested `end == source_len` exactly. Trailing blanks are extras
owned by no node, so `ENTRYPOINT ["a"] ` + EOF parks the zero-width terminator
one byte short of source_len and the check missed it. This is why neither a
trailing blank nor an absent final newline flagged on its own -- only the pair
did, exactly as the reporter's byte-exact control matrix showed.
Treat "at EOF" as EOF modulo a trailing blank run. Every blank except newline
qualifies, deliberately not a hand-picked subset: a form feed is no more
content than a space, and #1610 exists precisely because whether a file got
flagged used to hinge on such incidentals. Newline stays excluded because a
terminated final line produces no MISSING terminator at all.
The rule remains ZERO-WIDTH ONLY. A width-bearing MISSING/ERROR at EOF is a
genuine loss and is still flagged (a Makefile's unterminated final recipe
really does vanish), as is any failure earlier in the file; both are pinned by
guard tests alongside the existing #1610 guards.
Verified end to end: the issue's own `cli index_repository` reproduction goes
from parse_partial_count 1 to 0 with node count unchanged, so the phantom flag
is gone without anything dropping out of the graph.
Fixes#1746
Signed-off-by: Rares Popa <2606875+rarepops@users.noreply.github.com>
Fixes#2010. Refs #1997.
Indexing a 14k-file TypeScript repository holds 13.3 GB resident at
post_extract on Linux aarch64, and about 3.1 GB of it is AST traversal
scratch that no CBMFileResult points at.
ts_nstack_init cut its stacks from ctx->arena, which is result->arena. In
the parallel path that arena is stored into ec->result_cache[file_idx]
(pass_parallel.c:963) and freed only after parallel_resolve and the infra
passes (pipeline.c:1324), so a structure written for a one-file lifetime is
in fact held for the whole index, once per file, concurrently. The header
claimed otherwise and was wrong: "freed when the arena is destroyed at end
of file extraction".
The largest instance is channels. CHAN_STACK_CAP is 4096 and sizeof(TSNode)
is 32, so scan_string_consts_js (extract_channels.c:104) and
extract_channels_js (:376) take 128 KB each, per JavaScript or TypeScript
file. Across 12,673 such files that is 3,168 MB, and it produced 23 Channel
nodes out of 164,684.
CBMExtractCtx gains a scratch arena, created and destroyed by the
cbm_extract_file_ex call that builds the context. Nothing else changes about
where anything lives: ctx->arena still owns every string a CBMFileResult
points at, and ctx->scratch owns only traversal stacks, which nothing points
at.
Nothing a CBMFileResult points at moved. A context built without a scratch,
and a file whose scratch arena fails to allocate, both fall back to
ctx->arena, which is exactly the behaviour that shipped before this change,
so no path can be handed a NULL arena and silently lose nodes.
The signature change is what makes the mistake unrepresentable.
ts_nstack_init takes the context rather than an arena, so handing it
ctx->arena is an incompatible pointer type; push and push_children read the
arena the stack recorded, so a stale argument is an arity error. Both fail
under -Werror rather than silently retaining memory.
The 55 init sites are NOT one uniform sed. 29 pass ctx->arena and 26 pass a
local alias, and that alias is shared with result allocations: in
parse_zig_imports (extract_imports.c:1093) the same `a` feeds cbm_node_text,
strip_quotes, path_last and cbm_imports_push (extract_imports.c:1101, 1106,
1108, 1109). Rebinding it would have moved import names and module paths
into memory that dies with the call while the result kept pointing at them.
Each site was edited on its own and no alias was rebound. Re-derive the set
with:
grep -rn 'ts_nstack_init' internal/cbm/extract_*.c
grep -rn 'ts_nstack_push' internal/cbm/extract_*.c
parse_lisp_imports used its alias only for the stack, so the alias goes too.
push_nested_class_nodes and push_class_body_children took an arena purely to
reach the stack, so they take the context now; the one call site passes ctx.
Two measurements, from two different builds, and they are not the same
number.
A macOS census build, which instruments every cbm_arena_alloc and sums the
blocks behind every live CBMFileResult arena, reports the retained per-file
arena set dropping from 11,226 MB mapped to 5,680 MB, and blocks from 48,045
to 34,186. Those are mapped bytes, not resident: that build logs
mem.allocator.bound_populations_only, so untouched block tails never commit.
The resident figures come from Linux aarch64, gcc 13.3, static build, where
the build logs mem.allocator.owned classes=all and mapped equals resident.
Five runs per side on the full 13,999-file tree: mem.collect post_extract
rss_mb 13293 -> 6752 (-49.2%), mem.phase peak_mb 14840 -> 8565 (-42.3%).
nodes 164684, parse_partial 280 and registry entries/defines/imports
787954/804999/100027 identical on all ten runs; edges 835869-835874 against
835867-835881, overlapping ranges, so run to run noise. Wall clock means
35651 ms against 30502 ms, but the ranges overlap and the host had other
load, so the only supportable claim is no regression. ObjectScript Studio
Export on a small fixture: nodes 18, edges 25, both sides.
Scratch block size is 512 KB, chosen from measurement. arena_grow fires on
one file in 12,000 at both 512 KB and 1 MB, and constantly at 256 KB, where
the two channel stacks alone are exactly 262144 bytes. 512 KB therefore
costs half the resident block per worker for the same growth behaviour, and
stays under MI_LARGE_MAX_OBJ_SIZE in the vendored mimalloc, which is
MI_LARGE_PAGE_SIZE/8, exactly 524288 bytes (types.h:426, with
MI_ENABLE_LARGE_PAGES defaulting to 1 at :115 and not overridden by this
build), so a 512 KB block is still bin-allocated from a 4 MiB large page.
Growth is not free at that size for the same reason: arena_grow doubles to 1
MiB, above that bound and therefore a singleton OS allocation. One file in
twelve thousand pays it, which is why it is accepted. The largest single
file's scratch high water mark on this corpus is 33.5 MB, now transient
rather than retained.
Tests. Nothing in the suite asserted on result->arena size, block count or
total_alloc after cbm_extract_file, which is why this went unnoticed for the
life of extract_node_stack.h. traversal_stack_not_in_result_arena_issue2010
pins the byte budget: extracting "export const x = 1;" as TypeScript charged
365984 bytes to the result arena before and charges 87456 after, a
difference of 278528, exactly the two 4096-entry channel walks plus the
512-entry ES import walk at 32 bytes per TSNode. Of the 87456 that remain,
7680 is the defs item array at GROW_ARRAY's starting capacity of 32 times
sizeof(CBMDefinition) 240 and the other 79776 is everything else the
extraction interns, none of it traversal scratch; the 128 KB bound is
therefore above the new figure with room and a factor of four below the old
one. extract_traversal_stacks_come_from_ctx_scratch_issue2010 in test_mem.c
pins the lifetime, since a byte budget alone would also be satisfied by
shrinking CHAN_STACK_CAP: it builds the extraction context directly over two
arenas it owns, runs cbm_extract_channels, and asserts the scratch took at
least the 262144 bytes of the two walks while the result arena did not.
extract_c_macro_hidden_call_survives_preprocessed_pass_issue2010 is not a
scratch test. pp_ctx carries ctx->scratch so every context in the file is
uniform, but nothing reads it there: pp_ctx reaches only cbm_extract_unified
and cbm_run_c_lsp, and neither extract_unified.c nor anything under
internal/cbm/lsp/ includes extract_node_stack.h, so no traversal stack is
built on that path today. The test guards the macro-expansion path itself,
which had no assertion on a call that exists only after expansion.
ObjectScript composite extraction is already covered by
pipeline_objectscript_export_preserves_calls_sequential_parallel and
iris_export_xml_multi_class, so no new test is added there; a per-call arena
is correct on that path by construction, since each call owns and destroys
its own.
Local: scripts/test.sh, 7558 passed, 0 failed, 8 skipped, 139 suites.
Alternatives considered
Thread-local scratch arena, reset per file, with a depth guard for nesting.
Rejected on three counts. This codebase never uses thread-local storage for
owned bulk memory; every existing _Thread_local here is a small cache, a
guard or a log buffer, and every per-worker resource is a field in a context
struct. It would have needed cbm_arena_reset, which has no production caller
and a documented subtlety with grown block sizes (arena.c:213-217). And
extraction threads are created per cbm_parallel_for call and joined
(worker_pool.c:71,88) with no thread-exit hook in this project's own code,
so the arena would have leaked once per thread per dispatch and turned the
leak lane red.
Save and restore mark and release on the arena. Rejected because a per-call
arena already makes nesting correct by construction, and a mark type plus
two functions would have to be added to both arena headers.
Per-walk arenas, one per traversal rather than one per file. Rejected on
cost: 55 sites would each need an explicit span, for the same bound.
Right-sizing the initial capacities alone. CHAN_STACK_CAP at 256 rather than
4096 would cut those two sites from 3,168 MB to about 198 MB, which is real,
but it tunes the constant while the lifetime defect remains at all 55 sites,
and growth is not free: 256 doubling to 4096 abandons 122,880 bytes, and one
file in this corpus took 33.5 MB of stacks. The cap and the scratch block
size are coupled, which is a further reason to change one at a time. Worth
doing separately.
Routing to a scratch inside extract_node_stack.h while keeping the arena
parameters. Rejected because a parameter that is ignored lies about the API,
and the alias hazard above would stay a review obligation instead of a
compile error.
Freeing the per-file result arena earlier. Rejected: the results are read by
parallel_resolve and the infra passes. That is the retention half #1925
addresses, and this change is independent of it.
Signed-off-by: Daniel Ochoa <daniel.ochoa@closedloop.ai>
Martin Vogel