feat: surface stdio MCP processes and fix Godot reading gaps - #175
Merged
Conversation
A custom resource class was not a queryable symbol: parse_tres named the resource marker node from the [gd_resource type="..."] engine type, so a .tres declaring script_class="Buff" surfaced as "Resource" and a query for "Buff" missed it. Read script_class after type, each behind a non-empty guard, so a declared class wins regardless of header attribute order while the fallback chain still degrades to type and then to the default "resource" name. Golden-neutral: no fixture or golden corpus contains a .tres file.
An exact-name candidate was injected with score = max_fts_score, only TYING the best FTS hit, and the shared bonus pass then let a non-exact row overtake it on kind_bonus plus path relevance. Observed on a two-node corpus: the exact symbol ranked 2 at 65.0000020754717 against 65.00000209923664. Add exact_name_bonus, whose value is derived from the measured ceilings it must beat (80 name_match_bonus + 10 kind_bonus + 15 path relevance per query word, plus a strict-dominance margin), and lift an exact row only when it sits at or below the best non-exact score. Rows that already win keep their score, which leaves the ordered search goldens byte-stable.
The Zed-over-SSH section still presented the stdio bridge as the working workaround and carried no HTTP recipe, contradicting README.md and editors/zed/README.md, which already name HTTP the recommended remote transport. Lead with serve --http over a forwarded port, explain why forwarding beats exposing the port, and keep the SSH bridge as the documented fallback.
The exact-name ranking guarantee added in 9ab0830 was undocumented, so a user had no way to know that querying a full symbol name now returns it first. State the behavior, both of its boundaries (the query must be the name as typed, and an already-winning row keeps its score), and that ordering is deterministic.
The exact-name rescue mutated scores, which broke two ways. It lifted every needy exact row to the same value, erasing their score differences so their final order rested on the unordered SELECT in nodes_by_exact_name_nocase; and because a row already above the best non-exact score kept its original score, a weaker exact match could be lifted past a stronger one. Reproduced: a Parameter in a test path outranked a same-named Function in a matching path. Sort by exact-name status first and score second, via two stable passes, and mutate no scores. Every exact row precedes every non-exact row, exact rows keep their relative score order, no new ties are created, and the ordered search goldens stay byte-stable because the recorded scores never move. exact_name_bonus becomes the predicate it always was, is_exact_name_match; the derived dominance constants it needed are now dead and removed.
The watcher kept its own copy of the default ignore list, and that copy had drifted from the scan's: .godot and addons were absent, so live watching only skipped them when a project's .gitignore happened to list them. A Godot project that commits .godot got those trees re-indexed after any edit even though codegraph init had excluded them. Derive the shared entries from IndexingConfig::default().ignore_dirs so the two lists cannot diverge again, keep the watch-only entries in their own list, and place .godot and addons after the built-in-skip boundary so an explicit include can still opt a first-party addon back in, matching the scan. collect_watch_dirs_honors_gitignore_pruning now uses .generated/, a directory genuinely absent from the defaults, so it still proves gitignore merging rather than passing for the wrong reason.
The watcher read IndexingConfig::default().ignore_dirs and placed .godot and addons outside its built-in boundary, so include could resurface them. The scan does neither: scan_dir prunes options.ignore_dirs structurally before any include check, and ExtractOptions::for_project fills that list from the project's own config. The two disagreed in both directions — include re-included under the watcher but not the scan, and dropping addons from indexing.ignore_dirs re-included under the scan but not the watcher — either of which breaks sync output equalling index --force byte-for-byte. Thread the project's configured ignore_dirs through WatchOptions into WatchPolicy and keep every entry inside the built-in boundary. Overriding indexing.ignore_dirs is now the single re-inclusion surface on both sides, the same contract scan_reincludes_addons_when_override_drops_it already documents; gitignore-derived rules stay re-includable and exclude still beats include. with_config_include_reincludes_addons_default_ignore encoded the inverted contract and is replaced by a non-re-inclusion assertion plus a new configured-override test.
…turally is_ignored returned early when include was empty, so a configured exclude was never read in the common case: the scan dropped src/generated.ts while the watcher handled it. And gitignore rules shared one last-match-wins vector with the configured ignore_dirs, so a !addons/ line re-included a directory scan_dir prunes with a bare continue before any pattern set runs. Either way sync stopped equalling index --force. Split the single rules vector into structural_ignores and gitignore_rules so the tier boundary is a type distinction rather than an index convention, and give is_ignored the scan's three stages in the scan's order: structural prune, then a negotiable exclude-then-gitignore stream evaluated for every path, then include force-inclusion. matches_negotiable seeds its verdict from exclude before folding gitignore rules, which is what preserves both the scan's set order and each side's own glob semantics — exclude keeps the shared whole-path matcher, gitignore keeps rule_matches. gitignore_negation_reincludes_default_ignored_dir asserted that !vendor/ re-includes vendor, but vendor is in default_ignore_dirs, so the assertion was the defect. It now contrasts a negotiable Tools/ negation against structural vendor and node_modules.
The watcher had no ignore_paths tier at all, so the nine default Android res/values* style patterns applied only to the scan. XML is extractable here, so res/values/strings.xml was excluded by index --force yet watched and synced, breaking sync output equalling index --force. Add ignore_paths as the FIRST set of the negotiable stream, ahead of exclude and gitignore, mirroring the scan's pattern_sets, and thread it from project config through WatchOptions the same way ignore_dirs already travels. Match it with the shared whole-path matcher rather than rule_matches: rule_matches compares basenames for a * pattern, so res/values* could never match res/values/strings.xml, which is the same divergence the shared matcher was introduced to end. A gitignore negation and an include can still override an ignore_paths match, as they can on the scan side. Also correct default_ignore_dirs' comment: a .gitignore negation cannot re-include .godot or addons, because scan_dir prunes every ignore_dirs entry before evaluating any pattern set. Dropping the entry from a custom indexing.ignore_dirs is the only re-inclusion path.
matches_negotiable ORed the ignore_paths and exclude sets before folding gitignore, so a leading ! was honored only in gitignore and the order between the two config sets was unobservable. The scan's is_path_ignored strips ! in every set and lets the last matching pattern decide, so ignore_paths of ["gen/", "!gen/"] kept gen/helper.ts for index --force while the watcher pruned it. Fold the two config sets and gitignore through one accumulator in the scan's order, stripping ! per pattern and re-matching with that set's own matcher. The matcher split stays load-bearing and is now documented as such: config sets use the shared whole-path matcher, gitignore keeps the basename-glob rule_matches. matches_exclude keeps its plain any-match, because its remaining caller is the include stage, and the scan's IncludeSet::forces and wants_descend likewise any-match the raw exclude patterns without stripping !. A negated exclude has already cleared the verdict in the fold, so it never reaches that stage.
impact --json collected edgeCount only from the graph traversal, while the Godot reverse referrers were appended to affected through a separate path-keyed lane. A resource target therefore reported affected=13 with edgeCount=0, so an automation reading edgeCount to decide whether a dependency path exists saw none where twelve .tres files referenced the script. Take the referrer count from the already sorted and deduped set that feeds affected, add it to the traversal edges, and report it separately as resourceEdgeCount. Sharing one deduped set keeps edgeCount from ever contradicting affected, and a pure-code target keeps its previous edgeCount value with resourceEdgeCount zero. Reported by the Godot team against 0.40.4 as a JSON contract problem rather than a missing edge.
…registry A stdio serve --mcp process registered nowhere. The per-project daemon has daemon.pid and an HTTP server has an addr-keyed entry, but a foreground stdio process had no rendezvous, so neither codegraph http list nor codegraph status could name the processes holding an index. A partner report on Windows traced a failing index --force to three such invisible processes, and this machine accumulated thirteen. Key by pid, one <pid>.json per process, in a global state dir resolved like the HTTP registry but under an mcp leaf. Reads distinguish a missing directory, which means nobody has registered yet, from a directory that cannot be read, because the caller must be able to tell an empty registry from an outage. Dead-pid and unparseable entries are pruned on read. Registration covers the Direct and SpawnOrProxy modes, the foreground processes a user sees, and never BeDaemon, which already holds the daemon lock. A registry failure warns and serving continues, since MCP availability outranks observability, and no signal handler is installed because a crashed process's stale entry is what pruning exists for. The registry offers no terminate path: a pid outliving a crash may have been reused, and this crate cannot prove instance identity portably, so project, startedAt and version are recorded for a human to recognize a stale row.
The registry landed in the previous commit but nothing read it, so a user still had no way to answer the question the partner report raised: which process is holding this index. http list and status both stay silent about stdio servers. Add a mcp subcommand group mirroring http's wiring, with list printing pid, start time, version and project, plus the platform-appropriate command to stop a listed process. Guidance rather than a stop subcommand is deliberate: entries are pid-keyed, so a stale entry whose pid was reused would let us terminate an innocent process. McpAction stays an enum so stop can be added once process instance identity is provable portably. Also register the too-broad-root guard's exit, which serves tools off an existing index in the foreground exactly like Direct and was the one stdio exit left invisible. That is the IDE-launched CWD=$HOME case this command exists to surface. Every branch exits 0, including an unreadable registry, because failing a diagnostic command while the user is already debugging is hostile. The JSON keeps servers as an array in both cases and marks an outage with a separate key so a consumer never has to guess the shape.
… index index deletes codegraph.db and its sidecars before rewriting them, and on Windows that delete cannot succeed while another process has the file open. The partner report hit exactly this: three invisible serve --mcp processes held the database and the only clue was a bare remove_file error naming the path. Generate the who-holds-it guidance in a pure function so both of its branches are unit-testable, then call it from two places. Before the destructive rebuild, index warns when a registered stdio server could reach this project, listing each pid with the platform command to stop it. After a RemoveDatabase failure, the same text is appended to the CLI's presentation, leaving the store layer's statement of fact and the error chain untouched. A pinned entry only matches when the target lives at or under its project root; an entry with no project is always included, since it resolves per request and can open any database. Both sides canonicalize leniently so a symlinked ancestor does not split one project in two. The warning is strictly additive: it prints nothing when there is no holder to name, honors --quiet, and never changes the exit code, so a successful index against an empty registry emits its previous bytes exactly. An unreadable registry stays silent here and is surfaced on the failure path instead, where it is actionable. The RemoveDatabase wiring carries no integration test by design: RebuildFault exposes no database-removal hook, and a black-box attempt fails earlier, at database open.
cli.md gains two reference sections: impact --json's edgeCount now being the total with resourceEdgeCount as the resource share, and mcp list with both output shapes, the exit-0-on-outage behavior, the index pre-warning, and why there is no mcp stop. A reader who sees http stop but no mcp stop would otherwise assume it was forgotten rather than gated on portable instance-identity verification. godot.md notes the edge-count change where it matters, in the resource-impact section: a .gd referenced only from .tres files used to report zero edges while listing every referrer, because resource files have no grammar and so no graph edges of their own. AGENTS.md records the PID-keyed registry next to the addr-keyed HTTP one, since they are the same class of architectural fact and the difference in keying is what determines which one can offer a stop.
… counts A code review found five ways this round's own additions could mislead. impact counted a referrer twice when a GDScript preload gave it both a graph edge and a hit in the path-keyed resource lane: one dependency appeared twice in affected and inflated both counts. Restrict the resource lane to files the traversal did not already reach, before counting and before rendering, so disjointness is structural rather than assumed. A dangling symlink at the registry path made read_dir return NotFound, which the scan read as nobody has registered yet. Treat NotFound as missing only when symlink_metadata also fails, so a broken configuration is reported as the outage it is. live_entries trusted prune_dead's deletions to have landed, so a dead entry on a read-only registry was reported as running and made index warn about a holder that does not exist. Filter the returned entries by liveness and leave pruning as the best-effort disk self-heal it always was. The guidance asserted a listed pid was running and handed over a kill command without asking anyone to confirm what that pid is. Since we refuse to stop a process precisely because identity is unprovable, we must not imply the pid is proven either: rows now read as registered as running, and both stop hints ask for a ps or tasklist check first. The RemoveDatabase diagnostic narrowed holders by the failing artifact path, but CODEGRAPH_DIR can place the database outside the project tree, so the real holder was filtered out and the failure claimed none matched. Report every live entry on the failure path, keeping the precise narrowing only in the pre-warning, which legitimately has the project root.
The pre-rebuild warning filtered registered servers by the project each was launched for, which assumed that project bounds what a server can open. It does not. When a client passes an absolute projectPath, roots resolution accepts it on its own merits and consults the launch default only when no path was given, so a server started in one project routinely serves another. Filtering by the launch project therefore hid exactly the holder the warning exists to name, and index stayed silent before the delete that Windows cannot complete. Warn on every live registered server, as the RemoveDatabase path already did. Over-warning is the right bias here: it appears only before a destructive rebuild, only when not quiet, and only when something is registered, so a successful index against an empty registry still emits its previous bytes. HolderScope and the path predicate are removed rather than left unused, because the Project variant documented the very precondition this disproves. The project field is now recorded only for an explicit --path, since a bare launch has no pinned project and its cwd was never a scope; that also makes the <none> rendering reachable instead of dead. Wording throughout now calls it the launch project and says plainly that it does not limit what a server may hold. The test that asserted a sibling project's server needs no warning encoded the defect and is inverted. Two regressions pin the premise end to end: a real bare serve --mcp registers without a project, and a server launched for one project resolves an absolute projectPath into another.
The no-holder byte-identical guard keyed its duration normalization off
ends_with("ms"), but format_duration emits milliseconds only below a second and
switches to {:.1}s at or above. Under the parallel load of the pre-push gate the
index crossed that boundary and the guard stopped matching, so 1.6s versus 1.8s
failed a comparison that was never about timing.
Match the summary line by its own shape instead — it always reads
"{n} nodes, {m} edges in {duration}" — and truncate at its last " in ".
Relaxing the guard to ends_with('s') would have over-normalized ordinary prose,
so the test also pins that a non-summary line containing " in " survives.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #175 +/- ##
==========================================
+ Coverage 95.03% 95.07% +0.04%
==========================================
Files 135 136 +1
Lines 69512 70128 +616
==========================================
+ Hits 66059 66673 +614
- Misses 3453 3455 +2
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
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
1 participant
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
Two rounds of work, both Final-Wave approved.
Round 1 — Godot reading reinforcement (image-driven priorities)
.tresmarker nodes are now named fromscript_class, so a custom resource class is a queryable symbol (query "SkillData"used to miss it)..godot/addonsare honored without relying on.gitignore,ignore_dirscomes from project config as a structural skip,excludeapplies whether or notincludeis set, the missingignore_pathstier was added, and!negation is honored in every config set. Five commits, because each divergence brokesync == index --forcein its own way.Round 2 — Godot liaison doc rev55
impact --json'sedgeCountnow covers all impact edges including Godot static resource edges, withresourceEdgeCountas the breakdown. A.gdreferenced by 12.tresfiles reportededgeCount: 0while listing all 12 underaffected.serve --mcpprocess now registers in a PID-keyed registry,codegraph mcp listreads it back, andindexwarns before a destructive rebuild when a registered server could be holding the database. A partner report on Windows traced a failingindex --forceto three such invisible processes.The root cause of that Windows failure — the MCP engine holding the database while idle — was already fixed and released in v0.41.0; this round is the observability half.
Deliberate omissions
codegraph mcp stop. Entries are PID-keyed, so a stale entry whose PID was reused would let us terminate an innocent process, and this workspace has no portable instance-identity primitive.listprints the platform stop command and asks for aps/tasklistcheck first.codegraph-store.RemoveDatabasecannot be injected, so its guidance text lives in a pure function that IS unit-tested, and the wiring was verified by hands-on walkthrough.Verification
make cigreen: 128 suites / 3067 passed / 0 failedreference/untouched across all 18 commitsedgeCountdouble-count on apreload-ing script and a registryprojectfield being wrongly treated as a capability boundary