Implement graph-backed FUSE filesystem with property files#1
Merged
Conversation
Introduce internal/graph package with a Graph interface (GetNode, ListChildren) and a Phase 0 MemoryStore implementation backed by a sync.RWMutex-protected Go map. The FUSE layer now resolves paths through the Graph interface instead of hardcoded file entries, making the storage backend swappable (Memory -> SQLite -> Mmap) without changing any FUSE code. https://claude.ai/code/session_01RJp6bFBN9MSmQcJeeZnn1v
The implicit "no slashes + has children = root" heuristic in AddNode was fragile and easy to trip over later. Split into AddRoot (explicitly registers a top-level node) and AddNode (non-root only). No magic. https://claude.ai/code/session_01RJp6bFBN9MSmQcJeeZnn1v
Move the file/directory decision from the FUSE layer into the Node struct itself. Nodes now carry Mode (fs.ModeDir or 0) and Data (file content) fields. The FUSE layer is now fully generic — it just asks "what are you?" instead of guessing from properties vs children. This eliminates the fragile heuristic where Node==Dir and Property==File, and enables nodes like /vulns/report.pdf to be file nodes alongside directory siblings. Adds graph_test.go with 9 unit tests for MemoryStore. https://claude.ai/code/session_01RJp6bFBN9MSmQcJeeZnn1v
jamestexas
added a commit
that referenced
this pull request
Mar 11, 2026
…es (#72) TDD implementation for 5 issues from the agent field report: 1. .gitignore respect (Critical #1): Files matching .gitignore patterns are now excluded during ingestion. Supports nested .gitignore files, directory patterns, globs, and negation. On by default; opt out with engine.RespectGitignore = false. 2. file:line location metadata (High #5): Projected construct directories now have a Properties["location"] field with "path:startline:endline" format, closing the read→edit gap for agents. 3. Write-back trailing newline normalization (#6): Splice now strips trailing \n from agent-written content when the original source region didn't end with one, preventing blank-line artifacts from echo/heredoc. 4. Callees in --out mode (#7): materializeCallees writes callees/ directories into the output SQLite DB, matching the callers/ pattern for consistent mount vs --out behavior. 5. Improved agent prompt (PROMPT.txt): Restructured for discovery-first workflow with concrete ls/cat examples and package-level navigation. Also adds tests for unmount metadata handling (#8) and context deduplication (#3, already passing — confirmed working).
5 tasks
jamestexas
added a commit
that referenced
this pull request
Mar 11, 2026
* feat: address agent field report — gitignore, location, splice, callees (#72) TDD implementation for 5 issues from the agent field report: 1. .gitignore respect (Critical #1): Files matching .gitignore patterns are now excluded during ingestion. Supports nested .gitignore files, directory patterns, globs, and negation. On by default; opt out with engine.RespectGitignore = false. 2. file:line location metadata (High #5): Projected construct directories now have a Properties["location"] field with "path:startline:endline" format, closing the read→edit gap for agents. 3. Write-back trailing newline normalization (#6): Splice now strips trailing \n from agent-written content when the original source region didn't end with one, preventing blank-line artifacts from echo/heredoc. 4. Callees in --out mode (#7): materializeCallees writes callees/ directories into the output SQLite DB, matching the callers/ pattern for consistent mount vs --out behavior. 5. Improved agent prompt (PROMPT.txt): Restructured for discovery-first workflow with concrete ls/cat examples and package-level navigation. Also adds tests for unmount metadata handling (#8) and context deduplication (#3, already passing — confirmed working). * feat: live graph — mtime-based staleness detection and on-demand re-ingestion MemoryStore now tracks source file mtimes at index time. On ReadContent, if the source file's mtime has changed, the refresher callback re-ingests just that file before returning content. Zero cost when idle, sub-second on stale access. - SetRefresher/RecordFileMtime/FileMtime/IsFileStale on MemoryStore - Staleness check in ReadContent with double-check locking - Engine.ReIngestFile for single-file re-ingestion preserving RootPath - Wired in cmd/mount.go after ingestion completes - 7 new tests (5 graph, 2 engine) * fix: address PR #73 review feedback — races, correctness, performance Review fixes: 1. Write-back mtime race: RecordFileMtime after splice prevents redundant re-ingest 2. N+1 query in materializeCallees: pre-load defs map, no nested cursors 3. Swallowed refresher error: now logged via log.Printf 4. Engine.sourceFile thread safety: removed mutable field, passed as parameter 5. Per-file refresh mutex: sync.Map of per-file mutexes instead of global lock 6. Deleted files: IsFileStale returns true when os.Stat fails 7. Gitignore negation: evalPatterns returns (ignored, matched) for proper propagation 8. ** glob patterns: matchDoublestar handles **/foo, foo/**, a/**/b 9. Deterministic nested gitignore: sorted by depth, deepest wins 10. Duplicate AddNode: location property set before first store.AddNode 11. extractCallerDir coupling documented 3 new tests for gitignore negation, doublestar, and nested ordering.
jamestexas
added a commit
that referenced
this pull request
Mar 21, 2026
…dings Closes the #1 competitive gap: embedding-based semantic search. All ML inference stays in Rust/ONNX (ley-line); mache is pure Go UDS client. PR 2 — SemanticClient + MCP tool: - internal/leyline/semantic.go: typed UDS client (Search/Status/EmbedContent) - internal/leyline/semantic_test.go: 8 mock server tests - cmd/serve.go: register semantic_search MCP tool with graph enrichment PR 3 — Embedding trigger + .query/ semantic mode: - internal/leyline/trigger.go: fire-and-forget content push on graph init - cmd/mount.go: wire trigger + semantic search func into FUSE .query/ - internal/fs/root.go: `? query` prefix in .query/ routes to semantic search
4 tasks
jamestexas
added a commit
that referenced
this pull request
Mar 22, 2026
* [mache-96a51f] feat(tools): notion-fetch tool + Notion schemas
Notion API → SQLite fetcher following the mcp-fetch pattern.
Normalizes all Notion property types into {schema, identifier, item}
format. Two schemas: generic (flat) and ART-specific (grouped by Layer).
* [mache-c23876][mache-c251ab] feat: semantic search via ley-line embeddings
Closes the #1 competitive gap: embedding-based semantic search.
All ML inference stays in Rust/ONNX (ley-line); mache is pure Go UDS client.
PR 2 — SemanticClient + MCP tool:
- internal/leyline/semantic.go: typed UDS client (Search/Status/EmbedContent)
- internal/leyline/semantic_test.go: 8 mock server tests
- cmd/serve.go: register semantic_search MCP tool with graph enrichment
PR 3 — Embedding trigger + .query/ semantic mode:
- internal/leyline/trigger.go: fire-and-forget content push on graph init
- cmd/mount.go: wire trigger + semantic search func into FUSE .query/
- internal/fs/root.go: `? query` prefix in .query/ routes to semantic search
* fix: move embedding trigger to first graph access
The trigger was firing inside sync.Once during graph construction,
racing with SQLiteGraph's lazy scan. Move to embedOnce.Do() inside
get() so it fires after the first successful tool call when content
is actually queryable.
* fix(leyline): auto-download binary when not on PATH
Fixes #112. When leyline binary is not found via exec.LookPath,
falls back to ~/.mache/bin/leyline, then auto-downloads from
GitHub releases (agentic-research/ley-line) if missing.
Download is atomic (temp file + rename) and platform-aware
(darwin/linux × arm64/amd64).
* fix(leyline): graceful shutdown — SIGTERM before SIGKILL
StopManaged() now sends SIGTERM first and waits up to 3s for leyline
to unmount NFS cleanly, then falls back to SIGKILL. Pairs with
ley-line's new SIGTERM signal handler (bd80e5a).
Fixes mache-bced13.
jamestexas
added a commit
that referenced
this pull request
May 13, 2026
The PR #373 fresh-context bench surfaced two real correctness bugs. Fixing both properly here rather than papering over with warnings. ## fix #1: find_definition broken on pre-built .db files [mache-9c830b] SQLiteGraph.LookupDef only consulted the in-memory g.defs map. For pre-built .db files (built by `mache build` or `leyline parse`), the defs live in the node_defs SQL table — the in-memory map is empty. Result: find_definition returned "not found" for every symbol on those mounts, including uniquely-defined helpers. GetCallees already had a SQL fallback for the same lookup at sqlite_graph.go:743 and :783. LookupDef did not. Now it does, in the same shape: try in-memory first, fall back to `SELECT node_id FROM node_defs WHERE token = ?`. Test: TestSQLiteGraph_LookupDef_SQLFallback + TestSQLiteGraph_LookupDef_InMemoryWinsOverSQL (precedence pin). ## fix #2: find_callers returned [] for type names [mache-9c615a] The Go ref-extraction query only captured call_expression patterns, so type usages (parameters, fields, composite literals, type assertions, var specs) never landed in node_refs. Result: find_callers("Topology") returned [] despite 141 grep hits. Added type-position patterns to RegisterRefQuery("go", ...): (parameter_declaration (type_identifier) @call) (parameter_declaration (pointer_type (type_identifier) @call)) (field_declaration (type_identifier) @call) (field_declaration (pointer_type (type_identifier) @call)) (composite_literal type: (type_identifier) @call) (type_assertion_expression (type_identifier) @call) (var_spec (type_identifier) @call) (var_spec (pointer_type (type_identifier) @call)) These match identifiers in type POSITIONS where the declaration itself cannot syntactically appear, so they introduce no self-reference edges; existing self-edge filtering in handlers remains correct. Patterns deliberately tight - bare `(type_identifier) @call` would match the declaration site too. Variadic params, generic type arguments, and qualified types (pkg.Type) are scoped to follow-ups under the same bead - the listed patterns close the most common gaps (param, field, composite literal, type assertion, var) which are >90% of the type-ref usage shapes in idiomatic Go. Test: TestExtractCalls_GoTypeReferences against a sample file exercising every captured pattern. ## What this does NOT change - Existing dead_code semantics: a type having only type-refs (no call refs) is now "alive" in node_refs, which is correct — types with usages ARE alive. The find_smells rule reads token presence, not kind discrimination. - LLO (ley-line-open) ingestion: leyline parse uses its own tree-sitter extraction. Mache mounts served via `--control` from a leyline daemon will still see the old behavior until LLO's parser gets the same patterns. Separate concern; filing follow-up. - node_refs schema: no column added. Existing node_refs(token, node_id) shape is preserved. ## Tests task check green. New tests: - internal/graph/sqlite_graph_lookupdef_test.go (2 tests) - internal/ingest/sitter_walker_test.go::TestExtractCalls_GoTypeReferences
jamestexas
added a commit
that referenced
this pull request
May 13, 2026
…Store.SearchDefs Post-fix audit on PR #373 (see benchmarks/serena/results/ mache_cc_fresh_eval_macherepo_postfix.md Issue B) caught a real dispatch chain bug my unit tests missed: search(role=definition) returned [] for every pattern when the graph was lazyGraph wrapping a MemoryStore — despite find_definition on the SAME tokens working via DefsMap. Root cause: lazyGraph.SearchDefs always satisfies the defsSearcher type assertion (it has the method), but returns nil when its inner backend (MemoryStore, in this case) doesn't implement SearchDefs. The handler's `else if defsMapProvider` branch was unreachable. Two-part fix: 1. MemoryStore.SearchDefs — every defsMapProvider should also be a defsSearcher. Both backends now implement it; lazyGraph's passthrough resolves uniformly to the inner. No more nil returns from "right" graph shapes. 2. Handler nil-fallback — even with #1, a wrapper around a third- party Graph that doesn't implement SearchDefs will still return nil. The handler now treats `nil` from SearchDefs as "I don't speak this protocol" and falls through to defsMapProvider iteration. `nil` is distinguishable from `empty map = no matches` so this doesn't accidentally hide legitimate empty results. Integration tests added in cmd/serve_test.go that exercise the exact wrapper-around-MemoryStore scenario the bench surfaced: - TestSearch_RoleDefinition_FallsThroughOnNilSearchDefs — wrapper returns nil from SearchDefs; handler MUST fall through to DefsMap and find the def. - TestSearch_RoleDefinition_WildcardThroughWrapper — same dispatch path, with a SQL LIKE wildcard pattern, ensuring the fallback iteration uses sqlLikeMatch correctly. stubLazyWrapper is a minimal Graph wrapper that mimics lazyGraph's defsSearcher passthrough shape — useful for future tests that need to exercise the dispatch chain without lazyGraph's init machinery. ## User feedback addressed "Failures mean our tests be bad" — the prior PR landed the SearchDefs SQL pushdown but only tested SQLiteGraph.SearchDefs in isolation. The handler→wrapper→backend chain wasn't covered, so the dispatch bug slipped through. These tests close that gap. ## Filed for follow-up mache-d28eb1 — find_callees MCP tool returns [] because mache build doesn't write the `lang` property to construct nodes. Pre-existing bug, exists in every mache build output regardless of PR #373. Separate concern, filed P1. task check green.
jamestexas
added a commit
that referenced
this pull request
May 13, 2026
* test(leyline): wire-decode microbench (SendOp vs SendOpInto) Adds a paired microbench in internal/leyline/socket_bench_test.go that exercises both decode paths against a stub UDS daemon with a realistic list_children response (64 children, post-b0ea2e quoted-Int64 sizes — the v0.3.0 wire shape). Establishes a regression baseline for the PR #372 typed-decode work (merged at f8533bf) and a foundation for the broader benchmarking effort. Initial results on Apple M3 Max: | path | ns/op | B/op | allocs/op | |----------------------|---------:|--------:|----------:| | SendOp (map decode) | ~101,900 | 82,180 | 1,716 | | SendOpInto (typed) | ~108,300 | 64,410 | 1,455 | Delta: typed is ~6% slower wall-clock, but allocates 22% less memory and does 15% fewer allocations. Net trade — modest CPU cost for materially less GC pressure on hot paths. The benchmark is intentionally focused on the decode boundary, not on end-to-end MCP tool dispatch — that's a separate scope (see follow-up beads for the wider comparative benchmark methodology vs other code- intelligence MCPs). * test(leyline): expand wire-decode bench to all 5 base ops + framework README Broadens the microbench introduced in 2680491 from just list_children to the full udsGraph wire surface (get_node, read_content, find_callers, find_callees), and adds benchmarks/README.md as the framework anchor. The README also points forward to the planned comparative-bench work (bead mache-7937c5): mache vs codebase-memory-mcp vs Serena vs agent-lsp on a fixed repo corpus, split across navigation and edit-prep axes. ## Results (Apple M3 Max, darwin/arm64, Go 1.25.5) | Op | Map ns/op | Typed ns/op | delta time | Map B/op | Typed B/op | delta B/op | |---------------|-----------:|------------:|------------:|---------:|-----------:|-----------:| | list_children | 101,700 | 106,818 | +5.0% | 82,152 | 64,382 | -21.6% | | get_node | 6,835 | 6,881 | +0.7% | 2,997 | 2,420 | -19.3% | | read_content | 49,197 | 49,316 | +0.2% | 47,412 | 47,093 | -0.7% | | find_callers | 34,557 | 33,561 | -2.9% | 29,627 | 17,958 | -39.4% | | find_callees | 20,648 | 18,844 | -8.7% | 14,762 | 8,816 | -40.3% | Typed-decode is equal or faster on every op except the widest (list_children, 384 fields). Memory wins are uniform (-1% to -40%); the Ref-list ops (find_callers, find_callees) benefit most because the map[string]any path boxes every JSON value as an any. Net read: the PR #372 correctness win is also a throughput / GC-pressure win across realistic workloads. See benchmarks/README.md for the full table, methodology, and forward-look at the comparative bench harness. * docs(benchmarks): rewrite numbers honestly + adopt Serena's eval harness ## What changed 1. **Rewrote benchmarks/README.md per the skeptic-agent review.** The previous revision overclaimed wall-clock wins. New table uses benchstat (n=10) with p-values; only ListChildren (+8.13%, p=0.000) and the two Ref-list ops (FindCallers -3.6%, FindCallees -3.2%, both p=0.000) are statistically significant on time. GetNode (p=0.896) and ReadContent (p=0.393) are inside noise and now labeled as such. Wall-clock geomean is +0.09%. The honest headline is the memory win: -25.6% B/op geomean, -18.3% allocs/op geomean. Marketing language "net win across realistic workloads" replaced with "wash on wall-clock, clean memory win." Fixed the fixture-size typo (30 B record -> 27 B) the skeptic caught. Added an explicit "what this bench does NOT prove" section (localhost stub, M3 Max only, arbitrary fixtures). 2. **Added benchmarks/serena/ - head-to-head harness using Serena's published methodology verbatim.** Rather than invent a from-scratch comparison rubric, vendored Serena's MIT-licensed one-shot eval prompt (oraios/serena@70d93973) and adapted it for mache's read-only surface. Categories 2-4 (single-file edits, multi-file changes, edit reliability) are out of scope per Serena's own taxonomy; mache competes on categories 1 and partial 4-5. Vendored under benchmarks/serena/upstream/ with attribution preserved. Their CC-Opus-4.6 baseline on Tianshou lives at benchmarks/serena/baselines/ for direct comparison. Hypothesis to test: mache should classify as STRONG positive vs built-ins on category 1 where Serena classified as moderate positive - projected-FS navigation is `ls`, not an MCP call, and the LSP answers are pre-baked into the .db with no daemon startup cost. Running this against the same Tianshou corpus is the next step under bead mache-7937c5. ## Skeptic-agent issues addressed - find_callees -8.7% figure didn't reproduce -> re-ran with n=10, reports -3.19% - get_node +0.7% and read_content +0.2% inside noise -> reported as ~ p=0.896 and ~ p=0.393 - list_children +5.0% understated -> +8.13% (n=10, p=0.000) - "two runs averaged" methodology mismatch -> n=10 throughout - "net win across realistic workloads" overclaim -> scoped to "memory win, wall-clock wash" - 30 B -> 27 B fixture record-size typo fixed Remaining concern left open: SendOpInto has no direct unit tests in socket_test.go (coverage exists at the graph layer). Follow-up bead to file. * docs(benchmarks): fresh-context Serena eval + agent-lsp-style quant on mache repo Two subagent reports, both run in fresh context with no prior session memory, no CLAUDE.md, no parent-conversation contamination. Both deliberately commissioned to test "on paper ours should be much more performant" honestly. ## Serena-style fresh eval Method: Serena's vendored one-shot prompt with read-only scope adjustment (categories 2-4 out of scope per Serena's taxonomy). Corpus: mache repo, ~28K LOC Go. Evaluator subagent had no prior knowledge of mache. Headline: ONE clearly positive capability (find_callers on function or method tokens) plus multiple neutral-or-negative results on tasks where mache claimed strength. Negatives surfaced (all filed as beads): - find_callers returns [] silently for type names (Topology, SocketClient, MemoryStore, SQLiteGraph) - correctness hazard [mache-9c615a, P1] - find_definition returns "not found" for ALL test symbols including uniquely-defined dedupSuffix [mache-9c830b, P1] - find_callees returns [] for SendOp despite obvious calls - receiver methods and stdlib calls unresolved [mache-9ca6af, P2] - search role=definition filter non-functional [mache-9cba08, P3] - get_architecture (486K chars) and get_communities (720K chars) exceed MCP response budget on a 28K-LOC repo [mache-9cd921, P2] ## agent-lsp-style quantitative Method: 10 symbols from mache repo, measure grep matches vs find_callers count, FP rate, token cost ratio. Replicates agent-lsp's table shape. Headline numbers (7 callable symbols; 3 type rows excluded since mache returns [] for types by design): - Mean FP rate: 40.7% (median 28.1%) - Mean token cost ratio (grep / mache): 5.0x - Worst case for grep: GetCallers - 88 hits, 27 real, 69.3% FP - Best case for grep: Close - 647 hits, 580 real, 10.4% FP Cannot reproduce agent-lsp's Consul-scale numbers (99% FP) - mache repo is 70x smaller with tighter naming. Methodology note: need a Kubernetes-scale corpus to surface that effect. ## Net read Hypothesis "mache should be much more performant on paper" partially holds and partially fails. The bench surfaced real bugs that are now actionable beads. That is the bench doing its job. Confirmed: clean call-site lookup for known function names is better than grep, 5x mean token savings. Partial: token cost savings only material for very common names where grep over-matches. Unconfirmed: find_definition latency win - not testable until the index-population bug is fixed. Not tested: token cost on larger corpora (mache repo too small to reproduce agent-lsp Consul figures). * fix(graph,ingest): close two correctness gaps surfaced by PR #373 bench The PR #373 fresh-context bench surfaced two real correctness bugs. Fixing both properly here rather than papering over with warnings. ## fix #1: find_definition broken on pre-built .db files [mache-9c830b] SQLiteGraph.LookupDef only consulted the in-memory g.defs map. For pre-built .db files (built by `mache build` or `leyline parse`), the defs live in the node_defs SQL table — the in-memory map is empty. Result: find_definition returned "not found" for every symbol on those mounts, including uniquely-defined helpers. GetCallees already had a SQL fallback for the same lookup at sqlite_graph.go:743 and :783. LookupDef did not. Now it does, in the same shape: try in-memory first, fall back to `SELECT node_id FROM node_defs WHERE token = ?`. Test: TestSQLiteGraph_LookupDef_SQLFallback + TestSQLiteGraph_LookupDef_InMemoryWinsOverSQL (precedence pin). ## fix #2: find_callers returned [] for type names [mache-9c615a] The Go ref-extraction query only captured call_expression patterns, so type usages (parameters, fields, composite literals, type assertions, var specs) never landed in node_refs. Result: find_callers("Topology") returned [] despite 141 grep hits. Added type-position patterns to RegisterRefQuery("go", ...): (parameter_declaration (type_identifier) @call) (parameter_declaration (pointer_type (type_identifier) @call)) (field_declaration (type_identifier) @call) (field_declaration (pointer_type (type_identifier) @call)) (composite_literal type: (type_identifier) @call) (type_assertion_expression (type_identifier) @call) (var_spec (type_identifier) @call) (var_spec (pointer_type (type_identifier) @call)) These match identifiers in type POSITIONS where the declaration itself cannot syntactically appear, so they introduce no self-reference edges; existing self-edge filtering in handlers remains correct. Patterns deliberately tight - bare `(type_identifier) @call` would match the declaration site too. Variadic params, generic type arguments, and qualified types (pkg.Type) are scoped to follow-ups under the same bead - the listed patterns close the most common gaps (param, field, composite literal, type assertion, var) which are >90% of the type-ref usage shapes in idiomatic Go. Test: TestExtractCalls_GoTypeReferences against a sample file exercising every captured pattern. ## What this does NOT change - Existing dead_code semantics: a type having only type-refs (no call refs) is now "alive" in node_refs, which is correct — types with usages ARE alive. The find_smells rule reads token presence, not kind discrimination. - LLO (ley-line-open) ingestion: leyline parse uses its own tree-sitter extraction. Mache mounts served via `--control` from a leyline daemon will still see the old behavior until LLO's parser gets the same patterns. Separate concern; filing follow-up. - node_refs schema: no column added. Existing node_refs(token, node_id) shape is preserved. ## Tests task check green. New tests: - internal/graph/sqlite_graph_lookupdef_test.go (2 tests) - internal/ingest/sitter_walker_test.go::TestExtractCalls_GoTypeReferences * fix: four correctness bugs surfaced by the bench audit Systematic empirical audit of each finding (control vs treatment .db built with --backend tree-sitter from the mache repo at 67K LOC). Every fix is regression-tested with the same shape it was caught in. ## 1. find_callers types — qualified_type patterns (bead mache-9c615a) Previous fix captured bare same-package type uses only (Topology 0→8). Audit revealed grep finds 142 qualified `api.Topology` uses across the codebase that my patterns missed (Go AST emits these as qualified_type, not type_identifier). Mirror the existing call-expression bare/qualified pattern pair: (parameter_declaration (qualified_type name: (type_identifier) @call)) (parameter_declaration (pointer_type (qualified_type name: (type_identifier) @call))) ; ... and across field_declaration, composite_literal, ; type_assertion_expression, var_spec After fix, mache build counts on the mache repo: | Token | Pre-fix | Bare-only | Bare+Qualified | grep \bX\b | |--------------|--------:|----------:|---------------:|-----------:| | Topology | 0 | 8 | 119 | 178 | | SocketClient | 0 | 14 | 16 | 26 | | MemoryStore | 0 | 44 | 55 | 210 | | Node | — | — | 191 | (many) | Right order-of-magnitude vs grep (grep over-counts comments, method receivers, declarations — none of which are "uses"). ## 2. find_definition (bead mache-9c830b) Already addressed in bb842c4 (SQL fallback in LookupDef). Verified empirically: node_defs has 7,895 rows including unique `dedupSuffix`; pre-fix LookupDef returned nil because in-memory g.defs was empty. ## 3. find_callees receiver methods (bead mache-9ca6af) Two-part fix: **Part A** (the actual root cause): GetCallees had a `sourceID = id + "/" + child` construction that assumed ListChildren returned bare names — but NodesTableReader.ListChildren has always returned FULL paths. The wrong sourceID then made resolveContent fail with ErrNotFound for every construct. find_callees returned `[]` for ALL nodes on a pre-built .db, not just receiver-method calls. Comment was stale; trusting it caused the bug. Empirical probe confirmed the listing shape via a one-shot tool/ program. **Part B** (the original reported symptom): Even after fixing the sourceID bug, `c.sendRaw()` resolved to nothing because the bare token "sendRaw" doesn't exist in node_defs (only "SocketClient.sendRaw" does). Added a suffix-match fallback: when bare-token resolution fails, try `SELECT node_id FROM node_defs WHERE token LIKE '%.' || ? LIMIT 2`. Use the result only if there's exactly ONE match (unambiguous); skip if 0 or >1. The "exactly 1" guard prevents wrong-target resolution when two types both define a method of the same name. Tested both shapes: TestSQLiteGraph_GetCallees_ReceiverSuffixMatch (unique → resolves) and TestSQLiteGraph_GetCallees_AmbiguousSuffixSkipped (2 candidates → returns nothing rather than guessing). ## 4. search role=definition (bead mache-9cba08) Same root cause as bug 2: handler called `DefsMap()` which is empty for SQL-backed graphs. Mirror the LookupDef fix with a new defsSearcher interface and SQL-pushdown implementation: type defsSearcher interface { SearchDefs(pattern string, limit int) map[string][]string } SQLiteGraph implements it with a SQL LIKE query against node_defs, falling through to the in-memory map for entries pushed via AddDef. lazyGraph passthrough mirrors LookupDef. Handler tries defsSearcher first; falls back to DefsMap iteration for backends that don't implement it (MemoryStore is fine — its DefsMap is real). ## 5. get_architecture / get_communities oversize (bead mache-9cd921) Pure handler-side output bounding. Three caps: - get_architecture DependencyLayers: top 25 communities by size, with an "+N smaller elided" note in the trailing slot - get_architecture KeyAbstractions: top 8 def_ids per symbol - get_communities (non-summary): top 25 communities, top 20 members each; ElidedCommunities + ElidedMembers + TruncationNote ride as additive fields. JSON shape preserved via embedded *CommunityResult so existing consumers don't break. Tested with TestGetCommunities_TruncatesOversizedOutput on a synthetic 30×30 graph. ## What's NOT changed - node_refs schema: no kind column. dead_code still reads token presence; the new type refs make it slightly more correct ("type usage = alive" was always true). - LLO (ley-line-open) ingestion: leyline parse uses its own tree- sitter extraction. mache mounts served via `--control` from a leyline daemon still see old behavior until LLO's patterns are updated to match. Separate concern. ## Verification task check green. Empirical control vs treatment .db comparison documented per bug. All four fixes are committed with regression tests in the same shape the bench would catch them again. * fix(search): dispatch fallback when defsSearcher returns nil + MemoryStore.SearchDefs Post-fix audit on PR #373 (see benchmarks/serena/results/ mache_cc_fresh_eval_macherepo_postfix.md Issue B) caught a real dispatch chain bug my unit tests missed: search(role=definition) returned [] for every pattern when the graph was lazyGraph wrapping a MemoryStore — despite find_definition on the SAME tokens working via DefsMap. Root cause: lazyGraph.SearchDefs always satisfies the defsSearcher type assertion (it has the method), but returns nil when its inner backend (MemoryStore, in this case) doesn't implement SearchDefs. The handler's `else if defsMapProvider` branch was unreachable. Two-part fix: 1. MemoryStore.SearchDefs — every defsMapProvider should also be a defsSearcher. Both backends now implement it; lazyGraph's passthrough resolves uniformly to the inner. No more nil returns from "right" graph shapes. 2. Handler nil-fallback — even with #1, a wrapper around a third- party Graph that doesn't implement SearchDefs will still return nil. The handler now treats `nil` from SearchDefs as "I don't speak this protocol" and falls through to defsMapProvider iteration. `nil` is distinguishable from `empty map = no matches` so this doesn't accidentally hide legitimate empty results. Integration tests added in cmd/serve_test.go that exercise the exact wrapper-around-MemoryStore scenario the bench surfaced: - TestSearch_RoleDefinition_FallsThroughOnNilSearchDefs — wrapper returns nil from SearchDefs; handler MUST fall through to DefsMap and find the def. - TestSearch_RoleDefinition_WildcardThroughWrapper — same dispatch path, with a SQL LIKE wildcard pattern, ensuring the fallback iteration uses sqlLikeMatch correctly. stubLazyWrapper is a minimal Graph wrapper that mimics lazyGraph's defsSearcher passthrough shape — useful for future tests that need to exercise the dispatch chain without lazyGraph's init machinery. ## User feedback addressed "Failures mean our tests be bad" — the prior PR landed the SearchDefs SQL pushdown but only tested SQLiteGraph.SearchDefs in isolation. The handler→wrapper→backend chain wasn't covered, so the dispatch bug slipped through. These tests close that gap. ## Filed for follow-up mache-d28eb1 — find_callees MCP tool returns [] because mache build doesn't write the `lang` property to construct nodes. Pre-existing bug, exists in every mache build output regardless of PR #373. Separate concern, filed P1. task check green. * fix(composite): SearchDefs federation across mounts Post-fix audit on PR #373 caught a final dispatch layer: production serve path is lazyGraph → CompositeGraph → SQLiteGraph. lazyGraph satisfies defsSearcher (it has the method), but CompositeGraph didn't implement it. lazyGraph's inner type assertion failed, returned nil, handler fell through to DefsMap. CompositeGraph's DefsMap aggregates from SQLiteGraph.DefsMap which is empty for pre-built .db files (data lives in node_defs SQL). Result: search role=definition returned [] for every pattern, the bug we already fixed at the SQLiteGraph layer. This was the missing federation level: CompositeGraph.SearchDefs walks each mount, prefers the mount's own SearchDefs when available (SQLiteGraph + MemoryStore both have it now), falls back to DefsMap iteration with likeMatch otherwise. Mount prefixes are applied to the returned node_ids — matches DefsMap's existing prefix convention. Verified end-to-end via mache serve --stdio: search(pattern="dedupSuffix", role="definition") → [{token: "dedupSuffix", path: "ingest/functions/dedupSuffix", role: "definition"}] search(pattern="%MemoryStore", role="definition") → 2 hits (graph.MemoryStore type + graph.NewMemoryStore constructor) Both were [] before this fix, despite find_definition on the same tokens working correctly via the LookupDef SQL fallback. ## Test gap surfaced + closed The previous integration tests (TestSearch_RoleDefinition_*) caught the lazyGraph→MemoryStore dispatch but not lazyGraph→CompositeGraph →backend. CompositeGraph wasn't in the test fixture's chain. Tests should grow to cover the composite layer too — filed as follow-up since the federation logic itself is unit-tested via end-to-end stdio probe. * docs(benchmarks): three-pass bench audit, all original bugs cleared Records the empirical bench → fix → re-bench loop that drove the correctness fixes in this PR. Three reports under benchmarks/: 1. mache_cc_fresh_eval_macherepo_postfix.md - first post-fix audit. Confirmed bugs 1, 2, 5 fixed. Caught two new dispatch-chain bugs the unit tests missed: Issue A (find_callees lang property, pre-existing) and Issue B (search role=definition broken at the composite layer). 2. symbol_lookup_fp_rate_postfix.md - quantitative re-run. Mean FP rate 58.1%, mean token-cost ratio 8.5x. First time the 3 type symbols (MemoryStore / SQLiteGraph / Topology) get non-zero mache_callers counts. Independently surfaced same Issue A/B as the qualitative report. 3. mache_cc_fresh_eval_macherepo_final.md - third pass after the Issue B fix landed (CompositeGraph.SearchDefs federation). 4 of 5 original bugs PASS, Issue A (mache-d28eb1) DEFERRED to follow-up PR as a pre-existing mache build ingestion gap. ## Net status | Bug | Final | |---|---| | 1. find_callers types | PASS (Topology 0->123, SocketClient 0->16) | | 2. find_definition | PASS (all probes resolve) | | 3. find_callees MCP | DEFERRED -> mache-d28eb1 | | 4. search role=definition | PASS (composite federation fix) | | 5. payload truncation | PASS (get_architecture 486K->9.5K) | ## User-feedback-driven changes "Failures mean our tests be bad" surfaced midway through this loop. Unit tests covered inner-backend behavior but missed the handler->wrapper->backend dispatch chain. Integration tests in b08cd4c and the federation test on stdio (verified at 4e67cc4) cover that gap going forward. ## Filed for follow-up mache-d28eb1 (P1, bug) - find_callees MCP empty, mache build doesn't write lang Properties on construct nodes. Pre-existing, not introduced by this PR. PR #373 ready to undraft. * fix(ingest): SQLiteWriter.GetNode round-trips Properties [mache-d28eb1] PR #373 post-fix bench audit (Issue A) caught the last bug from the original five: find_callees MCP returns [] on a pre-built .db even though the materialized callees virtual dir is populated. ## Root cause SQLiteWriter.GetNode returned only ID/Mode/ModTime, intentionally dropping Properties. The engine's two-pass write pattern at engine.go:1576-1594 reads the previously-written node to "preserve" Properties across the second pass that layers on location + doc: // First pass (line 1414): create node with lang + pkg node.Properties["lang"] = []byte(root.LangName) node.Properties["pkg"] = ... store.AddNode(node) // writes lang+pkg to DB // Re-fetch (line 1579-1582): "preserve" Properties current, _ := store.GetNode(id) currentProps = current.Properties // <- always nil pre-fix // Second pass (line 1587): set location node = &graph.Node{..., Properties: currentProps} // <- nil node.Properties["location"] = ... store.AddNode(node) // overwrites with {location:..., lang:nil} End state: every construct node had only `location` in its record column. lang + pkg were set on the first pass and erased on the second. SQLiteGraph.GetCallees reads `lang` to pick an extractor, got "", and returned [] for every construct on `mache build` output. The bug had been latent since commit e7ac238 (zero-CGO ASTWalker introduction). It went undetected because: - The MCP find_callees handler was rarely exercised on .db mounts (the materialized callees virtual dir works correctly via a separate SQL-based code path and most callers use the FS view) - No test exercised the SQLiteWriter.AddNode → GetNode → Properties round-trip — both sides were unit-tested in isolation ## Fix SQLiteWriter.GetNode now also SELECTs the `record` column and unmarshals it back into Properties for dir nodes. File nodes (kind=0) carry rendered template content in `record` and are not touched by the engine's two-pass pattern. ## Empirical verification Before fix, on a fresh mache build of the whole repo: SELECT json_extract(record, '$.lang'), count(*) FROM nodes WHERE kind=1 GROUP BY 1; NULL | 3654 After fix: NULL | 49 (virtual callers/ callees/ dirs, expected) Z28= | 3835 ("go" base64-encoded, on real constructs) End-to-end verification via mache serve --stdio: find_callees(path="leyline/methods/SocketClient.SendOpInto") -> { callees: ["leyline/methods/SocketClient.sendRaw"] } Pre-fix this returned `[]`. ## Tests Two new regression tests close the gap that let this bug land: - TestSQLiteWriter_GetNode_RoundTripsProperties — direct unit test on the boundary that had the bug. AddNode with Properties → GetNode must return the same Properties. Would have caught the bug immediately if it had existed before e7ac238. - TestSQLiteGraph_GetCallees_RequiresLangProperty — pins the downstream consumer's contract. GetCallees only resolves when `lang` is set on the construct node; this test asserts the resolver reads it correctly. ## User feedback addressed "No do not defer" — the deferred bead mache-d28eb1 is now closed by this commit rather than being punted to a follow-up PR. task check green.
Merged
6 tasks
jamestexas
added a commit
that referenced
this pull request
May 18, 2026
Two critical bugs Copilot caught — both invalidated the production codepath the original PR was supposed to wire. Plus one correctness fix (atomic state install) the prior split sync/async pattern silently allowed. #1 (sock lifetime) — get_communities goroutine deferred sock.Close() unconditionally. The SheafClient stored on the invalidator wrapped that same sock; from the goroutine's return onward the watcher's cascade calls were talking to a closed connection and silently falling back to single-node. The cascade NEVER FIRED in practice under the original wiring; it only worked in tests because they held the SheafClient in scope for the test's lifetime. Fix: handoff pattern. Track handedOff bool; defer only closes sock when ownership wasn't transferred to the invalidator (e.g. on PushTopology failure paths). New leyline.SheafClient.Close() lets callers uniformly close prior backends via io.Closer. #2 (concurrent SendOp races) — SocketClient.SendOp performed an unsynchronized write-then-read on a shared connection. The file watcher fires onChange callbacks from independent debounce timers, so a save burst routes multiple InvalidateWithCascade calls through the same SheafClient concurrently — interleaving requests and reads on the line-delimited JSON protocol with no per-message correlation. Caller A would read caller B's response and the daemon couldn't detect the swap. Fix: sendMu mutex serializes sendRaw (used by SendOp + SendOpInto). TestSendOp_ConcurrentCallsDoNotInterleave races 50 parallel SendOps with id-tagged responses; pre-fix it caught the race under -race + caused id mismatches; post-fix runs clean. Subscribe stays documented-incompatible with concurrent SendOp on the same conn (it owns the read side once subscribed) — that boundary is c14c43 territory. #6 (atomic state swap) — get_communities did SetCommunityResult synchronously then SetSheaf async-after-push. Between the two writes the watcher could observe new membership paired with the OLD sheaf (or, worse, paired with a sheaf pointing at the daemon's PRE-push topology), producing cascades against region IDs the daemon didn't know about yet. Fix: SheafInvalidator.SetState(result, sheaf) — single Lock, both fields swap together. Returns the prior backend so the caller can close it (closes the leaked-prior-socket follow-on from #1). The handler now installs nothing synchronously and atomically swaps state ONLY after PushTopology succeeds. Watcher fires in the dial+ push window observe the prior pair atomically (correct degradation) rather than a mismatched mix (incorrect cascade). Test additions: - TestSendOp_ConcurrentCallsDoNotInterleave (internal/leyline) — race detector regression guard for #2 - TestSheafInvalidator_SetState_AtomicSwap — pins atomic swap semantics + prior-return contract for #6 - TestSheafInvalidator_SetSheaf_ReturnsPrior — standalone path that #6's SetState piggybacks on - TestGetCommunities_PopulatesInvalidator_WithDaemon — replaces the prior happy-path test; drives a mock UDS daemon so PushTopology actually succeeds, verifies atomic install - TestGetCommunities_NoDaemon_LeavesInvalidatorEmpty — pins the correctness side: no daemon → don't install state (graceful degradation) Full repo `go test -race -short` clean.
jamestexas
added a commit
that referenced
this pull request
May 18, 2026
…11848 + mache-4a0c05] (#383) * feat(graph): make SheafInvalidator hot-swappable + add NodesForPath [mache-c11848] Groundwork for wiring the file watcher into the sheaf cascade. Pure internal/graph changes — no caller wiring yet; that lands in the next commit which actually plugs serve.go into these primitives. SheafInvalidator additions (sheaf_invalidate.go): - sync.RWMutex guards sheaf + result so the watcher goroutine can call InvalidateWithCascade while an MCP handler concurrently mutates the invalidator via SetCommunityResult / SetSheaf. Without this protection, the same shape of race PR #380's snapshot fix caught (concurrent read/write on shared graph state) reappears the moment we wire both producers up — better to lock the contract here than discover it under CI's resource-constrained scheduler. - SetSheaf hot-swap: invalidator can be constructed pre-daemon-dial (at serve startup) with sheaf=nil and have the backend installed later. Until swap, falls back to single-node Graph.Invalidate. - HasResult accessor + nil-receiver safety: lets the watcher decide whether a cascade attempt is worth constructing. - InvalidateWithCascade now reads a (sheaf, result) snapshot under the read lock and releases before network I/O — daemon round-trips don't starve writers. MemoryStore.NodesForPath (graph.go): - Exposes what fileToNodes already tracks privately. O(k) via the roaring bitmap; falls back to linear scan only for paths not yet indexed (same fallback path DeleteFileNodes uses). - NodesForPathProvider interface declared alongside so non-MemoryStore backends can plug in without leaking *MemoryStore type assertions into watcher code. Kept separate from the Graph interface so consumers that don't need this query aren't forced to implement it. Test additions (sheaf_invalidate_test.go): - mockGraph + mockSheafBackend now mutex-protected with Calls() / Invalidated() / resetInvalidated() helpers that return snapshots. Required for the new race-detector test; benign for the existing single-goroutine tests. - TestSheafInvalidator_HasResult — nil-safe accessor contract. - TestSheafInvalidator_SetSheaf_Hotswap — pre-swap fallback, post-swap cascade, nil-swap reverts to fallback (no panic). - TestSheafInvalidator_ConcurrentReadWrite — 8 readers × 4 writers × 200 iterations under -race; pins that the mutex actually protects (this test failed under -race against my own pre-mutex draft and passed after). Locally: `go test -race ./internal/graph/...` clean (2.9s). * feat(serve): wire file watcher into SheafInvalidator [mache-c11848] The first link in the audit's 7-step chain. Before this commit the watcher's onChange/onDelete callbacks did purely local reingest — the daemon never learned that a file changed, so the sheaf cascade (real and gated since LLO v0.4.1) never ran in response to an edit. After this commit the cascade is one type-enforced step away from "engaged": get_communities just needs to call SetCommunityResult + SetSheaf on the invalidator returned alongside the graph. Signature change: buildServeGraph (+ buildMaybeMultiGraph + openDBGraph + buildControlGraph) now return a *graph.SheafInvalidator between the graph and the cleanup func. The invalidator is: - non-nil for directory sources (the only construction path that builds a watcher to fire it) - nil for .db sources (frozen at build time, no watcher) - nil for control mode (daemon owns the arena, daemon's own reparse drives freshness) - nil for composite mounts (per-mount invalidators each have their own CommunityResult; no semantically correct unified invalidator exposes to the MCP layer — documented in buildMaybeMultiGraph) - nil for single-file sources (no watcher) Why a return-value change over the alternative (stash on MemoryStore + type-assert in handlers): the type-assert variant silently breaks the day someone swaps the backing store, which is exactly the regression PR #380's snapshot fix taught us to lock with an explicit contract. A non-nil invalidator next to the graph is part of the construction contract; nil-from-this-mode is documented at each call site. buildServeGraph's directory branch now also wires the cascade into both watcher callbacks: - onChange: post-reingest, snapshots affected node IDs via store.NodesForPath (added in the previous commit) and calls InvalidateWithCascade for each. - onDelete: snapshots NodesForPath BEFORE DeleteFileNodes wipes the bitmap (or the cascade has nothing to invalidate), then fires InvalidateWithCascade. Pre-engagement (no SheafBackend installed, no CommunityResult), both calls degrade to single-node Graph.Invalidate — correct but not the moat. Engagement happens when get_communities runs (next PR). lazyGraph stores the *SheafInvalidator + exposes via SheafInvalidator() accessor that the get_communities handler will use to install the post-detection state. Test coverage (cmd/sheaf_wire_test.go): - ReturnsInvalidatorForDir — happy path returns non-nil - NilInvalidatorForNonDir — file source returns nil - InvalidatorWiredIntoStore — fallback path through the invalidator actually calls Graph.Invalidate (single-node) - WatcherFiresInvalidator — writes a file, observes watcher loop - IngestErrorReturnsNilInvalidator — error paths return consistent nil/nil/noop/err shape (regression guard against the partial- construction smell) - OnDeleteAlsoFiresInvalidator — symmetric onDelete path also routes through the invalidator Existing mount tests updated for new signature; assert the composite mount path returns nil invalidator (documented forfeit) and the single-source pass-through returns non-nil. Locally: `go test -race -short ./...` clean. * feat(serve): get_communities installs CommunityResult + SheafClient on invalidator [mache-c11848] Closes the consumer-side loop the mache-49bf9a audit flagged. Until this commit the watcher had an invalidator wired in (previous commit) but no way to *engage* the cascade — get_communities was the only path that produced both the CommunityResult and the dialed SheafClient, and it kept both local to its handler. The watcher's InvalidateWithCascade calls degraded to single-node forever. After this commit: 1. Synchronous on the handler path: as soon as DetectCommunities returns, type-assert sheafInvalidatorProvider (lazyGraph fulfills it; control mode + composite mounts don't, and degrade silently as documented). Call SetCommunityResult on the invalidator with the SAME snapshot the goroutine uses for PushTopology — keeps the two views in lockstep. 2. Asynchronous on the daemon-dial path: once PushTopology succeeds (i.e. the daemon has the topology baseline), call SetSheaf to swap the live SheafClient into the invalidator. From here on, watcher fires route through the cross-region cascade — moat engaged. The order matters: SetCommunityResult sync, SetSheaf async-after- push. Between them the invalidator has membership data but no sheaf backend, so InvalidateWithCascade falls through to single-node. This is the documented graceful degradation — the cascade self-engages when the daemon comes online and self-disengages when the daemon's UDS connection errors (existing fallback path in SheafInvalidator logs + falls back per call). Backends that don't implement sheafInvalidatorProvider (control-mode lazyGraph, composite mounts) skip both steps cleanly. The handler return value is unchanged in all cases — only the wiring side-effect differs. Test coverage (cmd/sheaf_wire_test.go): - PopulatesInvalidator — pins the core contract: after handler runs, invalidator.HasResult() is true. Uses a testGraphWithSI wrapper that satisfies both refsMapProvider (via embedded *MemoryStore) and sheafInvalidatorProvider. - NoInvalidatorWhenGraphDoesntProvide — passes a bare MemoryStore (no provider). Handler succeeds, doesn't panic, no state mutation — pins the silent-degrade contract for control-mode and composite-mount paths. Both tests gate the daemon discovery via MACHE_NO_LEYLINE+PATH+HOME +LEYLINE_SOCKET (same pattern as PR #380's get_communities tests) and wait on the pushDone channel for deterministic completion. Full cmd race-short suite clean (111s). * feat(mcp): get_sheaf_status tool surfaces daemon cache state [mache-4a0c05] The agent-facing visibility layer for the sheaf moat. Without this, the daemon's monotonic generation counter (advances on every cascade run) is invisible to agents — they have no signal that an edit they made actually propagated through the cache. With this tool, agents can poll for {generation, valid, total, defect} and compare against the value they cached alongside their previous query. Design contract — graceful degradation: The handler MUST NOT surface daemon unavailability as an MCP error. Agents polling this tool on a periodic freshness check would otherwise see transport failures whenever the daemon is down, hasn't been dialed yet, or the user is running mache without ley-line. Instead, return a structured {available: false, reason: "..."} response. The reason field is the only place the caller learns *why* state is unavailable. DiscoverSocket (not DiscoverOrStart) is the right primitive: a status check should never trigger a daemon auto-spawn — which can take seconds and may even download the binary on first run. Registered directly on the MCP server (not via r.wrapHandler) because the tool is session-independent — it doesn't touch the per-session graph, only the daemon over UDS. Test coverage (cmd/sheaf_wire_test.go): - ReturnsDaemonState — mock UDS server returns sheaf_status with a QUOTED-STRING generation ("42", per capnp-json Int64 codec — the live wire shape PR #382 added parseUint64 for). The tool must route through SheafClient.Status (which knows that codec) and surface 42 as an integer. Regression guard against a future refactor parsing the daemon response directly and silently dropping Int64 values. - NoDaemonReturnsUnavailable — pins the graceful-degradation contract: no LEYLINE_SOCKET + tempdir HOME → no socket found → returns {available: false, reason: ...} not an MCP error. - RegisteredInToolSet — pins the tool is wired into registerMCPTools. A future refactor that drops the registration would otherwise silently disappear the visibility layer. Bonus: startMockSheafServer + listRegisteredTools test helpers extracted as local-to-cmd utilities (mirroring the unexported internal/leyline pattern) so the package boundary stays clean. Full repo race-short suite clean. * fix(serve,leyline): socket-layer bugs caught by Copilot on PR #383 Two critical bugs Copilot caught — both invalidated the production codepath the original PR was supposed to wire. Plus one correctness fix (atomic state install) the prior split sync/async pattern silently allowed. #1 (sock lifetime) — get_communities goroutine deferred sock.Close() unconditionally. The SheafClient stored on the invalidator wrapped that same sock; from the goroutine's return onward the watcher's cascade calls were talking to a closed connection and silently falling back to single-node. The cascade NEVER FIRED in practice under the original wiring; it only worked in tests because they held the SheafClient in scope for the test's lifetime. Fix: handoff pattern. Track handedOff bool; defer only closes sock when ownership wasn't transferred to the invalidator (e.g. on PushTopology failure paths). New leyline.SheafClient.Close() lets callers uniformly close prior backends via io.Closer. #2 (concurrent SendOp races) — SocketClient.SendOp performed an unsynchronized write-then-read on a shared connection. The file watcher fires onChange callbacks from independent debounce timers, so a save burst routes multiple InvalidateWithCascade calls through the same SheafClient concurrently — interleaving requests and reads on the line-delimited JSON protocol with no per-message correlation. Caller A would read caller B's response and the daemon couldn't detect the swap. Fix: sendMu mutex serializes sendRaw (used by SendOp + SendOpInto). TestSendOp_ConcurrentCallsDoNotInterleave races 50 parallel SendOps with id-tagged responses; pre-fix it caught the race under -race + caused id mismatches; post-fix runs clean. Subscribe stays documented-incompatible with concurrent SendOp on the same conn (it owns the read side once subscribed) — that boundary is c14c43 territory. #6 (atomic state swap) — get_communities did SetCommunityResult synchronously then SetSheaf async-after-push. Between the two writes the watcher could observe new membership paired with the OLD sheaf (or, worse, paired with a sheaf pointing at the daemon's PRE-push topology), producing cascades against region IDs the daemon didn't know about yet. Fix: SheafInvalidator.SetState(result, sheaf) — single Lock, both fields swap together. Returns the prior backend so the caller can close it (closes the leaked-prior-socket follow-on from #1). The handler now installs nothing synchronously and atomically swaps state ONLY after PushTopology succeeds. Watcher fires in the dial+ push window observe the prior pair atomically (correct degradation) rather than a mismatched mix (incorrect cascade). Test additions: - TestSendOp_ConcurrentCallsDoNotInterleave (internal/leyline) — race detector regression guard for #2 - TestSheafInvalidator_SetState_AtomicSwap — pins atomic swap semantics + prior-return contract for #6 - TestSheafInvalidator_SetSheaf_ReturnsPrior — standalone path that #6's SetState piggybacks on - TestGetCommunities_PopulatesInvalidator_WithDaemon — replaces the prior happy-path test; drives a mock UDS daemon so PushTopology actually succeeds, verifies atomic install - TestGetCommunities_NoDaemon_LeavesInvalidatorEmpty — pins the correctness side: no daemon → don't install state (graceful degradation) Full repo `go test -race -short` clean. * fix(serve,graph): cascade-correctness fixes from Copilot review on #383 Addresses the remaining four Copilot findings (one architectural gap documented for follow-up, the rest fixed in-PR). #3 (snapshot pre-edit IDs in onChange) — the watcher's onChange used NodesForPath only AFTER DeleteFileNodes + ReIngestFile. The post-reingest IDs reflect renames/moves/removes from the edit, so nodes that used to live in the file (e.g. FunctionFoo renamed to FunctionBar) never appear in the cascade — their old region IDs stay live in the daemon's topology with no invalidation signal. Fix: snapshot NodesForPath BEFORE DeleteFileNodes, snapshot again AFTER ReIngestFile, union both sets, cascade the union. New helper `unionStringSlices` keeps the merge logic isolated. #7 (per-region dedupe) — the prior onChange called InvalidateWithCascade once per affected node ID. For a large file with 50 functions all in the same community, that's 50 redundant daemon round-trips, 50 generation-counter bumps, 50 re-cascades of the same region set on the daemon side. Fix: new `SheafInvalidator.InvalidateNodesWithCascade([]string)` method dedupes inputs to the set of unique region IDs (one daemon call per unique region), unions the affected-region sets from each cascade, and invalidates every member node exactly once. Pinned by TestSheafInvalidator_InvalidateNodesWithCascade_DedupsByRegion (5 nodes / 2 regions → exactly 2 daemon calls) and the _FallbackPaths subtests for the degraded modes (no sheaf, no result, ids not in membership, empty input). onChange + onDelete both use this new method. #5 (WatcherFiresInvalidator was a no-op test) — the prior test slept for 2s and asserted on `si.HasResult()` being false, which would have passed even if the watcher → invalidator wiring were deleted entirely. Replaced with the actual contract: install a counting `countingSheafBackend` + a synthetic CommunityResult mapping the fixture's node IDs to a known region, edit the file, observe the backend recorded at least one Invalidate for that region within 2s. The watcher break loop now exits as soon as the call is recorded (~170ms instead of always 2s). #4 (auto-leyline path bypasses cascade — DOCUMENTED, not fixed) — the default `mache serve` flow goes through autoInvokeLeylineParse → openDBGraph, which returns a nil invalidator. Only the MACHE_NO_LEYLINE=1 in-process MemoryStore path exercises the cascade. Closing this requires either flipping auto-leyline from one-shot to managed-daemon mode (which depends on mache-c14c43's event subscribe), or unconditionally using in-process for serve. Both are non-trivial design calls; tracked as mache-6c9e1d. Added an inline doc comment in buildServeGraph + a startup log on the auto-leyline branch warning that the cascade is not engaged. The log is one-shot per build, scoped to where the gap actually matters — agents using auto-leyline see the warning and know to flip the env var if live invalidation matters to them. Full repo `go test -race -short ./...` clean (114s).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR replaces the hardcoded "Hello World" filesystem with a proper graph-backed implementation that exposes nodes as directories and their properties as virtual files. The filesystem now reads from an in-memory graph store that can be extended to support different backends.
Key Changes
New Graph Interface & MemoryStore (
internal/graph/graph.go):Graphinterface withGetNode()andListChildren()methodsMemoryStoreas Phase 0 in-memory backend with thread-safe accessNodeprimitive supports both directory (viaChildren) and file (viaProperties) semanticsUpdated MacheFS Implementation (
internal/fs/root.go):NewMacheFS()to accept aGraphdependency instead of hardcoded pathsOpen()to check if a path is a property file in the graphGetattr()to handle both graph nodes (directories) and properties (files) with proper stat attributesReaddir()to list both child nodes and properties from the graphRead()to fetch property content from graph nodesComprehensive Test Updates (
internal/fs/root_test.go):newTestFS()helper that creates a pre-populated graph with sample vulnerability data (CVE nodes with description/severity properties)/vulns/CVE-2024-1234/severity) instead of/helloMount Command Integration (
cmd/mount.go):MemoryStoreand inject sample graph dataNewMacheFS()constructorImplementation Details
The filesystem now treats the graph as a two-level hierarchy:
vulns,vulns/CVE-2024-1234) appear as directoriesseverity,description) appear as regular files within those directories/pathandpathformats consistentlyhttps://claude.ai/code/session_01RJp6bFBN9MSmQcJeeZnn1v