The rebase resolution hand-aligned the analysis_tools array in both
src/mcp/mcp.c and tests/test_mcp.c after adding get_file_outline beside
compare_graphs, and the column widths did not match what clang-format
produces. That was the only red leg.
Whitespace only: with all whitespace stripped, both files hash identically
before and after (mcp.c d9b9bf785ac9, test_mcp.c 5a7fbf137492), so no
rebuild was required to re-validate the suites.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
An imported artifact carries the EXPORTER's mtime/size in every file_hashes
row, so the first incremental run after a bootstrap sees every file as
changed and re-parses ~the whole repository — most of the time the artifact
was supposed to save.
Reconcile the rows instead. After a successful import, ask the local git
which files are unchanged between the artifact's commit and the working
tree, and re-stamp exactly those rows with local stat() values. Everything
git cannot vouch for stays foreign and is re-parsed.
Export writes an optional "reconcile_basis":"git-clean-head" marker into the
existing artifact.json, and only when the tree is provably clean outside
.codebase-memory/ AND every hash row still matches disk. Unknown fields are
ignored by older binaries, so no schema_version bump.
Eligibility is deliberately narrow, and the polarity is skip-on-doubt:
a row is restamped only if it is tracked at the artifact commit (git
diff/ls-files are blind to ignored files — a .cbmignore negation can index
a gitignored file, #500) and absent from the changed set. Any uncertainty —
no git, missing/non-hex commit, shallow clone, truncated -z output,
allocation failure — skips reconciliation entirely and falls back to
today's behavior. Reconciliation can never fail the import.
Windows correctness: every git argument uses DOUBLE quotes, per the rule
already stated in artifact.c. cbm_popen runs cmd.exe /c on Windows, which
does not honor single quotes, so a single-quoted ":(exclude)" pathspec made
the clean-tree probe report dirty and the marker was never written there.
The commit-existence probe uses `cat-file -t <oid>` rather than
`cat-file -e <oid>^{commit}` because ^ is cmd.exe's escape character.
Fail closed on allocation failure: both cbm_ht_create() results are checked,
and every insertion is verified to have landed (cbm_ht_set discards the
underlying insert's status, so OOM drops entries silently). A dropped entry
in the changed set would make a genuinely modified file read as unchanged
and be restamped — a silently stale graph. This is the one place where the
membership-set primitive has unsafe polarity; classify_files uses it where a
dropped entry only means "re-parse".
Symlinks are refused rather than followed when stamping: git tracks a
symlink's link text, not its target's content, so "unchanged" from git says
nothing about the bytes the indexer parses. Uses the established
lstat + S_ISLNK / reparse-point idiom (discover.c, pass_pkgmap.c,
pass_envscan.c), which on Windows also keeps non-ASCII paths off the ANSI
CRT.
The trust trade-off is documented at both the import site and in
artifact.h: foreign mtimes currently force a re-parse that auto-scrubs a
poisoned artifact at an unpredictable clone time, so today's exposure is
transient and self-healing. Restamping makes it durable and gates it on a
producer-written string — acceptable only because the marker alone never
suffices, as every restamped row must additionally be proven unchanged by
the local git against a commit present in this clone.
Also skip synthetic file_hashes rows. The pipeline stores semantic-input
digests under .codebase-memory/.semantic-input/ that have no file on disk;
stat()ing them made the export-side check fail unconditionally, so the
marker was never written on ANY platform and the whole feature was inert.
find_deleted_files skips the same class. Both halves of the trust check now
ignore the .codebase-memory/ subtree, matching the clean-tree pathspec.
Distilled from #868 by moofone; #869 and #870 are not included.
Co-authored-by: moofone <35233167+moofone@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
manage_adr mode='set_sections' rebuilt the ADR from cbm_adr_parse_sections()
and cbm_adr_render(). That model is lossy, so the rebuild silently rewrote
text nobody asked to change. Measured on the previous commit:
- Everything before the first recognised heading was dropped.
- A mis-cased '## Purpose' was not a heading, so when it came first its
whole block was dropped with the preamble — an ordinary typo cost the
author a section.
- Sections stored out of canonical order were reordered.
mode='update' never had this exposure because it writes the caller's bytes
verbatim; the section merge introduced it.
The merge now locates the target heading's byte span and replaces only that
span, appending a block when the heading is absent. Bytes outside the span
are never reconstructed, so a preamble, a code fence, an unrecognised
heading and the author's section order all survive byte-for-byte. That
property is the acceptance test: every document in
adr_splice_preserves_untouched_bytes is a case the rebuild corrupted.
Arbitrary headings now work, which is what an "add an entry" mode needs.
The canonical six become a convention rather than a privilege: they are
still what ADR_EMPTY_HINT suggests, but they get no ordering priority and
none is required. Under splicing, ordering is whatever the document says,
and any privileged reordering would reintroduce the data loss above.
Names match exactly, including case, so '## Purpose' and '## PURPOSE' are
different sections. Folding them would silently merge two blocks the
author chose to keep apart — the same class of loss this commit removes.
cbm_adr_validate_section_keys() no longer enforces the canonical set. It
now rejects only names that could not round-trip through a '## NAME' line
(empty, '#'-leading, newline-bearing, edge-whitespace, over-long), because
such a name scans back as a different heading or as none, and a second
identical write would append a duplicate instead of being a no-op.
A heading inside a fenced code block is a code sample, not a section. An
unterminated fence hides every heading after it, so a write would append a
duplicate rather than replace: that is refused explicitly, and mode='sections'
reports the same ambiguity instead of answering with a partial list.
manage_adr mode='sections' now uses cbm_adr_scan_headings(), the same
classifier the write path splices with. It used to list any '#'-prefixed
line, so a '## Foo' in prose or inside a fence was reported as a section no
write could target. Two components disagreeing about what a section is was
how a section write came to be able to destroy one.
cbm_adr_parse_sections() is unchanged, so documents parse exactly as before.
TEST(adr_parse_sections_non_canonical) still passes untouched.
Co-authored-by: andis777 <24672074+andis777@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
manage_adr could only replace: mode='update' overwrites the stored
document in full, so adding one entry costs a re-send of the whole ADR.
That is a data-integrity problem before it is a cost one — the caller has
to reproduce every byte it did not intend to change, so the unchanged
prefix survives only as well as the round-trip that carried it.
mode='set_sections' rewrites only the sections named in `section_updates`
and leaves the rest of the stored document untouched, so the stored copy
stays the authority for everything the caller did not name. Unlike a
whole-document append it is idempotent: a client that loses a response
and retries re-sets the same section to the same body and the document is
byte-identical, where an append would silently duplicate the chunk.
Only the six canonical section names are writable. That is a correctness
constraint rather than a style rule: adr_try_section_header() parses ONLY
canonical headers, so a non-canonical '## FOO' written here would be read
back as body text of the section above it, and a second identical write
would append a duplicate — destroying the idempotence the mode exists for.
Three things the new mode needed that were not there:
- cbm_store_adr_update_sections() now wraps its read-modify-write in
BEGIN IMMEDIATE. Three writers replace this row wholesale — the indexing
pipeline, the UI POST /api/adr handler, and mode='update' — so the
unguarded get/merge/store lost whichever of them committed between the
read and the UPSERT. mode='update' is a single atomic UPSERT and never
had that window; a section merge introduces it.
- set_sections joins the write_request classification. A mode missing from
it takes no per-project mutation lease, resolves the store query-only,
and never reaches open_adr_store_for_write — its write would be
attempted through a read-only handle while an index runs.
- The write path reads the legacy <root>/.codebase-memory/adr.md itself.
The existing migration runs on the read path only because it must not
block on the lease; without this a section write would merge onto an
empty document and discard an ADR still present on disk.
CBM_ADR_MAX_LENGTH now applies to an MCP write path: it is enforced inside
cbm_store_adr_update_sections, which mode='update' does not go through. An
empty section body, an unknown section name and a missing section_updates
are all rejected before any store is opened, so a caller that meant to
write never receives a success-shaped read.
Section-level update was chosen over the whole-document append proposed in
PR #1243; the analysis that established the problem is from that PR.
Co-authored-by: andis777 <24672074+andis777@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Closes#518 (Section nodes don't index body text) and #519 (META.yaml /
frontmatter description values not indexed). A question asked in words
could not reach the nodes that hold the words: Section carried only its
heading, a config Module only its path, and both labels were filtered out
of BM25 results outright.
nodes_fts gains a fifth column, `body`, backfilled from each node's
EXISTING properties.docstring. No new column on `nodes`, no new table, and
CBM_INDEX_FORMAT_VERSION deliberately does NOT move -- nobody is forced to
reindex. A legacy four-column table survives untouched under CREATE VIRTUAL
TABLE IF NOT EXISTS: the backfill probes for the column before naming it,
and bm25()'s surplus fifth weight is inert on a table that has no fifth
column (FTS5 reads a weight only for a column an instance landed in). Such
a database keeps opening and searching, just without prose.
Two node kinds gain the docstring they lacked:
- Markdown section bodies (#518) -- the blocks following a heading, up to
the next heading or subsection, collapsed to single spaces and capped at
MAX_COMMENT_LEN (500 B) with a UTF-8 backoff that never splits a
multi-byte sequence.
- Top-level YAML/JSON description / summary / purpose (#519), promoted
onto the file's Module node.
Section and Module are no longer excluded from BM25 results. Both the
ranked query and the mirrored count query were changed together; removing
the filter from only one desynchronises results from counts. Column weights
are bm25(nodes_fts, 1.0, 1.0, 1.0, 1.0, 0.3): the four identifier columns
keep parity with each other and prose sits well below them, so a prose-only
hit surfaces but never outranks an identifier match.
The FTS backfill is consolidated into one writer, cbm_store_fts_rebuild(),
used by the wholesale rebuild AND the delta merge. That second site matters:
it had its own hand-written four-column INSERT, which with a fifth column
would leave body NULL for every node arriving by delta -- silently, on the
warm path most users hit, while a full reindex looked perfect. There is now
exactly one place the column list is decided, and a test that fails if the
delta path stops writing prose.
The backfill guards json_extract with json_valid: malformed properties JSON
exists in pre-fix databases and would otherwise abort the whole rebuild.
Tests: markdown section bodies (own body, stops at the next heading, UTF-8
safe cap, empty body stays NULL), YAML/JSON description promotion (block
scalars, synonym fallback, top-level only, config-only), FTS body indexing
and its column isolation, malformed-JSON tolerance, legacy four-column
degrade, incremental watermark, BM25 finding Section and Module by prose,
results/total consistency, identifier-outranks-prose, and the delta-merge
regression.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Co-authored-by: ShauryaaSharma <196896785+ShauryaaSharma@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
- Return IDXPAR_NULL_RESP if supervised run produces NULL response
- Pin exact {"status":"error","outcome":"exit_nonzero"} JSON contract in systemic test
- Update phase comments in mcp.c to document "error" alongside "crash" and "hang"
Signed-off-by: Shashwat Mishra <shashwat78.nits@gmail.com>
Add regression tests verifying index supervisor behavior when a worker exits
with a nonzero status:
- TEST(index_recovery_quarantines_exit_nonzero): proves a single file causing a
nonzero exit is quarantined with phase 'error' while remaining files are indexed.
- TEST(index_recovery_systemic_exit_nonzero_gives_up): proves systemic nonzero
exits cause the supervisor to give_up without false quarantines.
Also document the deterministic first-file progressive quarantine tradeoff in
mcp.c supervisor comment as requested during review.
Signed-off-by: Shashwat Mishra <shashwat78.nits@gmail.com>
When a worker process exits with a nonzero code (e.g. internal parse-limit
abort on a pathological file), treat it the same as CRASH/HANG: quarantine
the offending file via the existing two-consecutive-strikes mechanism and
continue indexing the rest of the chunk.
Previously, EXIT_NONZERO caused the supervisor to abort the entire chunk,
losing all indexed data for that directory. This is impactful for
large-scale repos (Android AOSP: 253GB, 60M+ files) where machine-generated
files routinely trigger internal limits.
The two-consecutive-strikes intersection mechanism already guards against
false quarantines — a systemic nonzero exit (e.g. bad CLI arg) produces no
recurring suspect, so the intersection is empty and the supervisor gives up
(same behavior as before).
Tested on Android QSSI17 (253GB, 24 chunks) + QCM6490 vendor (244 chunks).
Result: 11.35M nodes indexed, 0 data loss, 2 genuinely pathological files
quarantined (checkpatch.pl, a 140K-line generated .ts file).
Signed-off-by: Shashwat Mishra <shashwat78.nits@gmail.com>
CREATE TABLE / CREATE VIEW / CREATE MATERIALIZED VIEW now produce Table
and View nodes (previously generic Variable), CREATE PROCEDURE produces
a Function, and schema-qualified DDL names (schema.table) are named by
the table identifier instead of the schema. A view's FROM/JOIN relations
are emitted as usages and resolve into view -> table USAGE lineage
edges.
Relations join the cross-file name registry so lineage can resolve, with
two structural safeguards:
- Registry membership is defined once by cbm_label_is_registry_symbol
(helpers.c); the full, parallel and incremental seed sites all call
it, ending the KEEP-IN-SYNC copies the old label lists required.
- The default cbm_registry_resolve vetoes relation-labeled results:
common table names (users, orders, config) collide with code
identifiers in every language, and no CALLS/USAGE/READS/WRITES/THROWS/
handler/decorator consumer may bind them. The SQL lineage path opts in
through the new cbm_registry_resolve_lineage.
Table/View also join the registry-only per-file LSP surface labels so a
table rename invalidates dependent SQL files on incremental (no stale
lineage edges), rank with the type tier in BM25 search, and appear in
the architecture boundary/package/cluster queries via the pinned
CBM_SQL_RELATION_LABELS fragment.
Tests: extraction trio (labels, lineage usages, schema-qualified names),
grammar golden + probe updates, relation-label contract pin, and two
pipeline tests: cross-language isolation (binding: fails without the
veto) and incremental table-rename stale-lineage.
Closes#574.
Co-authored-by: alexisperinger-ux <alexis.peringer@iss-stoxx.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Trivial insertion-point collision in tests/test_mcp.c: both this branch and
main (#1704's UTF-8 pin) add a Windows search_code test at the same spot.
Both tests kept.
PowerShell 5.1 encodes stdout for a native-process pipe in the console
OEM codepage, so raw search content containing characters the inherited
CP cannot carry (Cyrillic under CP437/850, ...) reached
collect_grep_matches as '?' — and whether it degraded depended entirely
on which console the server happened to inherit. That surfaced as the
intermittent test_mcp raw-Русский mojibake on the windows CI leg and
means real Windows users in a default console get '?' for all
non-ASCII search_code raw content.
Every generated command now pins [Console]::OutputEncoding to UTF-8, so
the pipe is codepage-independent by construction. The read side needs
no pin: Select-String decodes BOM-less UTF-8 via .NET StreamReader
defaults. A Windows-side builder test asserts all five command variants
carry the prelude.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
json + include_evidence + data_flow labeled args as strategy because
bfs_to_tree_json declared strategy,confidence before args but emitted
args first. Tree flat_trace (risk_labels or data_flow) also dropped
include_evidence by calling bfs_to_toon_table without the flag.
Fixes#1542
Signed-off-by: rudi193-cmd <rudi193@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The generated Pi extension registered each MCP tool as { name, run }, but
Pi's ToolDefinition requires label, description, parameters, and execute.
Tools registered that way carried no parameter schema, so strict providers
such as xAI/Grok reject the request with a 422 'missing field parameters',
and the tools were uncallable because Pi invokes execute, never run.
Emit the full tool shape from the registry: label/description via new
accessors, the input_schema embedded directly as a JSON object literal, and
execute instead of run.
Signed-off-by: Alex Musichen <alex.musichen@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>
The local Windows leg caught this within hours of merging #1532: on a machine
that is not domain-joined, USERDOMAIN is the literal string WORKGROUP, and
`WORKGROUP\test` is not a resolvable principal —
WARN: build-dir DACL stamp (pre-wave) failed (user=WORKGROUP\test ...):
WORKGROUP\test: No mapping between account names and security IDs was done.
so the grant fails outright and the tree keeps its inherited
`Authenticated Users:(M)`. The guard then correctly reports that cross-account
mutation is still possible, and the whole leg stops. Since most Windows
machines are not domain-joined, that is the common case, not the exotic one.
#1532 was fixing a real bug in the other direction (a bare name resolves
against the machine first, so a host named like its user grants to an empty
principal). Both failures are name RESOLUTION failures, so this stops resolving
names: the account is identified by SID, which is unambiguous everywhere. The
SYSTEM and Administrators grants in these same commands have always used that
form — the current user was the odd one out. Name lookup survives only as a
fallback where PowerShell is unavailable.
Applied to all three scripts carrying the stamp (run-tests-parallel, soak-test,
memlab), since they share the failure and would otherwise drift apart.
Co-Authored-By: Kiborgik <Kiborgik@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Martin Vogel