feat(mcp): replace yactt mcp serve positional path with file:// project URI#54
Merged
Conversation
Designs the migration from the optional positional path on 'yactt mcp serve [path]' to a file:// URI passed in each targeting tool's args. Locks in: - file:// URI as the on-wire project reference - stateless server: resolve-and-load on every call (disk cache is the warm path) - full unification: 15 code-intel tools + 3 registry tools gain a 'project' field; list_projects and get_graph_schema remain registry-style - tree_overview's existing 'repo' field is accepted as a deprecated alias for one release, with a per-tool stderr notice - LSP client lifetime on cold paths is explicitly deferred to a future daemon / HTTP server mode Pre-implementation spec — no code changes yet.
34 tasks across 12 phases implementing the project-reference migration spec: Phase 1 - internal/project package (ParseRef, Resolve) Phase 2 - tree_overview (with deprecated repo alias) Phase 3 - 14 remaining code-intel tools (one per task) Phase 4 - get_graph_schema factory simplification Phase 5 - 3 registry tools + audit/TOFU hooks in index_repository Phase 6 - persisted_query schema + runner project injection Phase 7 - fixture helper ProjectURI() Phase 8 - cmd/yactt/main.go: drop positional path, build IndexHooks Phase 9 - junie-launcher + .mcp.json integration Phase 10 - README, docs/design.md, CHANGELOG Phase 11 - end-to-end dispatch test Phase 12 - final verification + smoke test Each task is bite-sized (2-5 min steps), TDD-driven (write failing test → implement → run passing → commit). Self-review spec coverage matrix at the bottom confirms all spec sections are mapped to plan tasks.
The new internal/project package is the seam between MCP dispatch and repo loading. ParseRef validates on-wire project references (file:// URIs only, with absolute local paths and no traversal). Resolve parses the URI, looks the decoded path up in the registry, and returns a loaded *store.Repo via the per-project disk cache. Tests cover valid input (path, trailing slash, percent-encoded space and slash), rejected input (empty, wrong scheme, authority-bearing, relative, traversal including percent-encoded ..), and Resolve behavior (registry hit, unregistered path, trailing-slash canonicalization). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…ield - Schema now requires 'project' (file:// URI); legacy 'repo' accepted as deprecated alias for one release with a stderr notice - Factory takes *registry.Registry; handler calls project.Resolve on every call and defers repo.Close() - runOverview CLI seeds an ephemeral registry so it can resolve the project URI the same way as the MCP server - registerAllTools threads both *registry.Registry and *store.Repo for the transitional period (other 15 tools still take repo) - contract test for tree_overview updated to expect the new 'project: empty reference' boundary error - wire_shape test infrastructure updated: wireShapeDefs map now takes (registry, repo); cases table for tree_overview uses the new project URI arg shape Tests passing: internal/tool, internal/contract, cmd/yactt. End-to-end tests (tests/acceptance, tests/fidelity) still on the old API — addressed in Phase 11 / Task 33.
Tasks 4-5 from the plan. Both tools migrate to the project-reference pattern: factory takes *registry.Registry, handler resolves args.Project via project.Resolve and defers repo.Close(). The cmd/yactt/main.go factory call sites and the wire_shape test infrastructure track the change. Sister tasks (node_edges, search, ...) follow in subsequent commits.
… project URI Phases 3-4 of the plan. All 14 code-intel tools (node_get, node_source, node_edges, search, edit_impact, find_symbol, get_symbols_overview, find_code, search_code, find_referencing_symbols, get_code_snippet, get_architecture, query_graph, detect_changes) now take a *registry.Registry and resolve args.Project via project.Resolve. get_graph_schema drops its unused *store.Repo factory parameter. Schemas declare 'project' (file:// URI) as a required field; args structs gain a Project string as the first field. The legacy 'repo' field on tree_overview remains as a deprecated alias for one release; the other 14 are net-new. Per-tool test files compile via the seedRegFromRepo helper (added to helpers_test.go) and the wireShapeDefs map keys now all return registry-based handlers. Several per-tool tests fail at runtime because their args JSONs don't yet include 'project' — Phase 12 final verification will fix them. wire_shape_bench_test.go, wire_shape_test.go, cmd/yactt/main.go::registerAllTools, and the contract test for tree_overview are all updated to track the migration. The wire-shape test infrastructure is the contract the agent rely on; the per-tool unit tests are exercised by tests/acceptance and tests/fidelity, which land in Phase 11.
…pository audit/TOFU hooks Phase 5 of the plan. The three targeting registry tools (index_repository, index_status, delete_project) take a 'project' (file:// URI) in args and normalise it through project.ParseRef before doing registry lookups. index_repository's factory signature gains two nil-safe hooks (emitStartup, warnTrust) called once on the first successful index per process, memoised via sync.Once. cmd/yactt/main.go passes nil for these today; the real wiring lands in Phase 8 (IndexHooks at runMCPServe). The audit startup line and TOFU check therefore move from boot-time to first-use. Contract tests update their args to include 'project' (a file:// URI the reg has an entry for) so the tool's own required-field guards fire rather than the new project guard. The empty-project boundary case is added as a separate sub-test. The registry acceptance test in tests/acceptance updates its 'path' field calls to 'project' (file:// URIs). The broader acceptance test still has per-tool unit-test build errors; those land in Phase 12 final verification.
…jects into target args Phase 6 of the plan. PersistedQueryArgs swaps the legacy 'id' field for 'op' (semantic improvement) and adds required 'project' (file:// URI). The runner now takes a project URI and a rawArgs json.RawMessage (the persisted_query caller's JSON), layers caller args on top of the op's static args (caller wins except for 'project'), and injects the project URI under the 'project' key so the wrapped code-intel tool gets it transparently. Example ops 'onboarding' and 'repo-map' drop the 'repo':'' placeholder — the runner injects project now. Registry test updates the Runner.Run call signature.
… update docs Phase 8 (CLI): runMCPServe drops the positional path entirely and removes the single-repo boot path. The startup audit line and TOFU check now move to first index_repository success per process, with emitStartup and warnTrust hooks bound in main.go and passed to the new tool.IndexRepository factory. Phase 9 (integration): junie-extension/scripts/yactt-launcher.sh drops find_project_root and the exec-time path arg. Dry-run mode emits a deprecation notice pointing agents at the file:// project URI flow. The launcher test asserts the deprecation text. .mcp.json drops the positional path. Phase 10 (docs): README updates the MCP quickstart and the registry-mode description. New CHANGELOG.md entry lists every breaking change with a before/after migration table.
Most per-tool test files now pass a real 'file://' project URI in the args JSON. The withProject helper (in helpers_test.go) prepends the project field automatically when the test passes args without one; existing tests that already had project are left as-is. The factory-signature updates land together with the args changes. A small subset of per-tool tests still need the same treatment — most notably nodeedges_handler_test.go and the bench tests. Those are addressed in a follow-up commit.
…dispatch_test The acceptance test fixture uses a package-level fixtureRegCache that's populated by loadRepo (mirroring fixtureRepoCache). Every per-tool test references the reg alias; pre-existing args JSONs are wrapped with file://<fixture-root> via the existing callJSON helper. tests/acceptance/mcp_dispatch_test.go is the new end-to-end regression test: it boots an in-process MCP server, sends initialize + index_repository + tree_overview + find_symbol JSON-RPC requests, and asserts the wire shape. The test currently times out — the stdio-draining goroutine needs investigation (likely a server shutdown race). Tracked separately.
…llDC callJSON and callAsMap in tests/acceptance/ now use withAcceptanceProject to auto-prepend a 'project' (file:// URI) field. callDC in detectchanges_test.go has the same treatment inline. Tests for empty project boundary (e.g. TestDetectChangesAcceptance_BoundaryRefRequired) now pass the project field explicitly so they exercise the ref-required guard rather than the project guard. The findcode_ts TestFindCodeAcceptance_TreeSitter_CompileError test exhibits a shared-registry state issue when run in parallel — passes alone, fails when the registry entry from a prior test (with a different path) is hit. Tracked as follow-up cleanup; not a migration correctness issue.
…test calls tests/hybrid/expand_seeds_test.go and tests/fidelity/ use a local seedRegForX helper (mirrors internal/tool/helpers_test.go's seedRegFromRepo). The hybrid tests are now green; fidelity tests still need per-test drive-site wrapping with driveWithProject — a follow-up that touches every drive() call site. Documented as a follow-up cleanup; not a migration correctness issue.
Eight items from PR-54's code-review pass, plus three
follow-ups CI surfaced.
Security hardening
- project.ParseRef: call url.PathUnescape on u.Path and re-check
traversal patterns. url.Parse already decodes one level
("%2e%2e" → ".."), but double-encoded forms like
"%252e%252e" decode once to literal "%2e%2e", which a
downstream tool that unescapes again would turn into real
"..". Add belt-and-suspenders: tests cover lower + upper-case
double-encoded patterns.
Performance
- index_repository: short-circuit on warm cache (entry exists with
same Mode, cache dir present, no source mtime newer than
IndexedAt) — return existing entry with Reloaded:false. Three
new tests pin warm-cache, stale-cache, and missing-registry-row
paths.
- Bypass the short-circuit when the caller supplies a custom
`name` that differs from the cached entry's Name — `name` is
a per-call label override, not a stored property of the cache.
Required by TestTool_IndexRepository/custom_name.
Discoverability + deprecation
- cmd/yactt mcp-serve usage string: rewrite to show the wire
shape (file:// URI in every targeting tool's args) and add a
"Project references on the wire" section so agents don't have
to spelunk the spec.
- tree_overview's `repo` deprecation shim: detect already-URI-
shaped values and pass through verbatim (don't prepend "file://"
to "file://abs/path" → "file://file:///abs/path"). Pin the
deprecation window to v0.2.0 in CHANGELOG.md.
Test ergonomics
- repofixture.ProjectURI() and repofixture.WithProject() — the
canonical "file://<root>" wire shape in one place.
- internal/registry/registrytest.Seed/SeedFromRepo — the shared
registry-seeding helper (consolidates three duplicates).
Polish
- internal/mcp.ProtocolVersion: extract "2024-11-05" to a
package-level const so the comment that explains the version
choice lives next to it. Update every call site.
…harness races
The PR-54 review surfaced eight production fixes; this commit
repairs the test files those fixes invalidated. All errors below
manifested as CI failures on the unmodified PR.
Project-field threading
- internal/tool/persisted_query_test.go: three tests used the
pre-rename {"id":"onboarding"} shape; rename to {"op":"...","project":"file:///abs/path"}.
- internal/tool/wire_shape_test.go: TestWireShape_RegistryTools
passed {"path":"/abs/path"} to index_repository/index_status/
delete_project; rename field and wrap as file:// URI. The three
TestWireShape_DetectChanges subtests get the same treatment.
- tests/acceptance/registry_test.go: TestTool_IndexRepository/custom_name
used the legacy "path" field; rename and wrap.
- internal/tool/querygraph_test.go, searchcode_test.go,
nodeedges_handler_test.go, nodeedges_lsp_test.go: per-test
withProject(root, args) wrapping so the handlers' required
project field is set on every call.
Test-harness races
- tests/acceptance/lsp_test.go::TestLSPFallback_NoGopls: rewrite
to drive store.MaterializeNode directly on the detached repo.
Going through NodeEdges → project.Resolve creates a fresh repo
with opportunistic LSP startup, defeating the test's stub
attachment. The materializer is the seam we want to pin anyway.
- internal/tool/nodeedges_lsp_test.go: same fix — drive
scanCallers directly. Two tests affected.
- tests/acceptance/mcp_dispatch_test.go: repair os.Pipe lifecycle.
The test closed stdinR synchronously after writes, racing with
the server goroutine's first read ("file already closed").
Two-stage close: stdinW after writes, stdoutW after Serve
returns, stdinR deferred to t.Cleanup.
Helpers consolidation
- tests/fidelity/fidelity_test.go, fidelity_test_part2_test.go:
drive/driveRaw now take repoRoot and apply driveWithProject.
The earlier "TODO" comment in drive is gone.
- internal/tool/helpers_test.go, tests/acceptance/acceptance_test.go:
seedRegFromRepo / seedRegForRoot / withProject / seedRegForFidelity /
seedRegForProject / driveWithProject / withAcceptanceProject
are now thin delegators around registrytest + repofixture.
Per-test fixtureRegCache / fixtureRepoCache lookups stay (loadRepo
caches the on-disk repo; only the registry was per-test corrupted).
- tests/acceptance/{findcode_ts_test.go,searchcode_test.go}: replace
the package-level fixtureRegCache (whose disk file gets cleaned up
after the first test that loaded it) with a fresh
seedRegForProject(t, repo.Root()) per test.
kellenff
force-pushed
the
mcp-cli-project-reference
branch
from
July 13, 2026 19:41
15ace80 to
1963424
Compare
kellenff
marked this pull request as ready for review
July 13, 2026 19:42
kellenff
force-pushed
the
mcp-cli-project-reference
branch
from
July 13, 2026 19:44
1963424 to
025de02
Compare
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
2 participants
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
Deprecate the optional positional path argument on
yactt mcp serveand replace it with afile://URI that every targeting tool takes in its args. The server becomes stateless across tool calls: no repo loaded at boot, every call resolves the URI to a path, looks it up in the registry, loads (or hits the disk cache for), and serves the result.After the change, the MCP server runs in what used to be called "registry mode" by default. The single-repo boot path is gone.
file://is the canonical project reference on the wire; the on-disk registry keyed by absolute path stays the source of truth.Migration
yactt mcp serve /abs/pathyactt mcp serve{"path": "/abs/path"}inindex_repository/index_status/delete_project{"project": "file:///abs/path"}projectfield on code-intel tools){"project": "file:///abs/path", ...}(required on every code-intel tool){"repo": "/abs/path"}ontree_overview(decorative){"project": "file:///abs/path"}(deprecatedrepoaccepted through v0.2.0, stderr notice emitted){"id": "onboarding"}onpersisted_query{"op": "onboarding", "project": "file:///abs/path"}What's in this PR
12 phases from the implementation plan, in 14 commits on
mainand 2 follow-up commits on this branch.On
mcp-cli-project-reference(the original PR):docs/snowball/specs/2026-07-12-mcp-cli-project-reference-design.md,docs/snowball/plans/2026-07-12-mcp-cli-project-reference.md)internal/projectpackage —ParseRef(validates file:// URIs against sentinel errors) +Resolve(registry lookup → store.Load with disk cache)tree_overviewmigrated with deprecatedrepoalias + stderr notice(reg *registry.Registry)+project.Resolveon every call +defer repo.Close()get_graph_schemafactory drops unused*store.Repoparameterindex_repository,index_status,delete_project) takeproject(file:// URI)index_repositoryaudit + TOFU hooks —emitStartup/warnTrustcallbacks, nil-safe, memoised viasync.Onceper processpersisted_queryschema updated (op+project); runner injectsprojectinto the target tool's argsIndexHooksatrunMCPServejunie-extension/scripts/yactt-launcher.shdropsfind_project_root, dry-run prints deprecation noticetests/acceptance/mcp_dispatch_test.go— end-to-end JSON-RPC plumbing testTwo follow-up commits on top (PR-54 review + CI test repairs):
fix: PR-54 review fixes — security hardening, perf, polish(547 insertions, 27 deletions across 14 files)project.ParseRef: callurl.PathUnescapeand re-check traversal patterns to catch double-encoded paths (%252e%252e)index_repository: short-circuit on warm cache (entry exists with same Mode, cache dir present, no source mtime newer than IndexedAt) → returns existing entry withReloaded:false. Bypassed when caller supplies a customnamethat differs from the cached entry's Nametree_overviewdeprecation shim: detect already-URI-shaped values and pass through verbatim (don't prepend "file://" to "file:///abs/path")repofixture.ProjectURI()+repofixture.WithProject()— the canonical "file://" wire shape in one placeinternal/registry/registrytest.Seed/SeedFromRepo— shared registry-seeding helpercmd/yactt mcp-serveusage string: rewrite to show the wire shape and documentfile://URIs explicitlyrepodeprecation window to v0.2.0mcp.ProtocolVersionconst: extract"2024-11-05"so the comment that explains the version choice lives next to itfix(ci): PR-54 test repairs — thread project through helpers, repair harness races(261 insertions, 199 deletions across 16 files)projectfield through tests that PR-54's schema migration invalidated (persisted_query_test, wire_shape_test, registry_test, querygraph_test, searchcode_test, nodeedges_handler_test, nodeedges_lsp_test, fidelity_test, findcode_ts_test, acceptance_test, mcp_dispatch_test)lsp_test::TestLSPFallback_NoGoplsandnodeedges_lsp_test: drivescanCallers/MaterializeNodedirectly so the handler'sproject.Resolve → store.Load(which creates a fresh repo) doesn't defeat the test's stub-LSP attachmentmcp_dispatch_test: repairos.Pipelifecycle — two-stage close (stdinW after writes, stdoutW after Serve returns, stdinR deferred tot.Cleanup)findcode_ts_test/searchcode_test: replace the package-levelfixtureRegCache(whose disk file gets cleaned up after the first test that loaded it) with a freshseedRegForProject(t, repo.Root())per testTest status
go build ./...,go vet ./..., and the full CI matrix (vet / test / smoke build,govulncheck + gitleaks,Analyze (go),Analyze (actions),Analyze (javascript-typescript),CodeQL) all green. PR is ready for review.Spec / plan / CHANGELOG
docs/snowball/specs/2026-07-12-mcp-cli-project-reference-design.mddocs/snowball/plans/2026-07-12-mcp-cli-project-reference.mddocs/snowball/plans/2026-07-13-pr54-fixes.mdCHANGELOG.md(the migration table at the top is the wire-shape contract)