diff --git a/internal/analysis/diffmap.go b/internal/analysis/diffmap.go index ca9b76fe7..b58f8643a 100644 --- a/internal/analysis/diffmap.go +++ b/internal/analysis/diffmap.go @@ -113,6 +113,13 @@ func MapGitDiffContext(ctx context.Context, g graph.Reader, repoRoot, repoPrefix return joinHunksToSymbolsContext(ctx, g, repoPrefix, hunks, files) } +// JoinDiffSymbolsContext maps a file-only diff (MapGitDiffContext with a nil +// reader) to the symbols g indexes, without running git again. A caller that +// needs the changed files before it can pick its reader diffs once this way. +func JoinDiffSymbolsContext(ctx context.Context, g graph.Reader, repoPrefix string, diff *DiffResult) (*DiffResult, error) { + return joinHunksToSymbolsContext(ctx, g, repoPrefix, diff.Hunks, diff.FileChanges) +} + // PathDomain names the vocabulary a file path is spelled in. The two overlap, // so the domain has to travel with the path rather than be recovered from it: // in a repo whose tree carries a top-level directory named like the repo diff --git a/internal/hooks/posttask.go b/internal/hooks/posttask.go index 268597984..b9827450b 100644 --- a/internal/hooks/posttask.go +++ b/internal/hooks/posttask.go @@ -136,25 +136,106 @@ type changeSet struct { Scope string `json:"scope"` Repo string `json:"repo"` RepoRoot string `json:"repo_root"` + // The label of an answer whose relations lag: graph_freshness on a + // primary checkout, graph_status on a worktree. + GraphFreshness string `json:"graph_freshness"` + GraphStatus string `json:"graph_status"` + PendingFileCount int `json:"pending_file_count"` } +// detectChangesBudget bounds the detect_changes round trip of a briefing. A +// daemon still publishing edits answers inside it with a labelled partial; a +// stalled one is reported rather than waited on. +const detectChangesBudget = 300 * time.Millisecond + +// detectChangesWait is how far into the budget the briefing's wait_deadline +// lets the daemon wait for pending edits before it labels them. The rest of +// the budget is for the diff walk and the transport. +const detectChangesWait = detectChangesBudget / 2 + // detectChangeSet asks the bridge for the current diff and decodes it. -// Returns nil when the bridge is unreachable or the payload is unusable. -func detectChangeSet(port int) *changeSet { - raw := callServerTool(port, "detect_changes", map[string]any{ +// answered is false when the bridge did not reply within detectChangesBudget, +// or replied with nothing once detectChangesWait had passed; changes is nil +// when it is unreachable or the payload is unusable. +func detectChangeSet(port int) (changes *changeSet, answered bool) { + started := time.Now() + raw, answered := callServerToolWithin(port, "detect_changes", map[string]any{ "scope": postTaskDiffScope, // The briefings never read by_depth; summary_only drops the // largest part of the response. - "summary_only": true, - }) + "summary_only": true, + "wait_deadline": started.Add(detectChangesWait).UTC().Format(time.RFC3339Nano), + }, detectChangesBudget) if raw == "" { - return nil + // The daemon refuses a wait_deadline that passed before it took + // the call up: a busy daemon, not an unreachable one. + return nil, answered && time.Since(started) < detectChangesWait } - var changes changeSet if err := json.Unmarshal([]byte(raw), &changes); err != nil { - return nil + return nil, true } - return &changes + return changes, true +} + +// relationsLine is the label line of a change set whose relations lag, or "" +// when the graph has published every change it reports. +func (c *changeSet) relationsLine() string { + state := c.GraphFreshness + if state == "" && c.GraphStatus != "" && c.GraphStatus != "ready" { + state = c.GraphStatus + } + if state == "" { + return "" + } + files := c.PendingFileCount + if files == 0 { + files = len(c.ChangedFiles) + } + return fmt.Sprintf("relations: %s (%d files)", state, files) +} + +// laggingChangesBriefing is the briefing for a change set the graph has not +// caught up with: the relations line and the changed files, with the +// symbol-level diagnostics left to a turn that can trust them. A stalled +// detect_changes (nil changes) is reported the same way instead of as +// silence, except to a session that recorded no writes: it has no change set +// to warn about. lagging is false when the change set is complete. +func laggingChangesBriefing(heading string, scope sessionScope, changes *changeSet) (briefing string, lagging bool) { + line := fmt.Sprintf("relations: pending (detect_changes did not answer within %v)", detectChangesBudget) + why := "" + var files []string + if changes == nil { + if scope.SessionID != "" && len(loadSessionState(scope.SessionID).WrittenPaths) == 0 { + return "", true + } + } else { + if line = changes.relationsLine(); line == "" { + return "", false + } + why = " the graph is still publishing edits, so" + files = changes.ChangedFiles + if attr := attributeChanges(scope, changes); attr.Attributed { + if len(attr.OwnedFiles) == 0 { + return "", true + } + files = attr.OwnedFiles + } + } + var sb strings.Builder + sb.WriteString(heading + "\n\n") + sb.WriteString(line + " —" + why + " symbol-level diagnostics are skipped; run detect_changes again before handoff.\n") + if len(files) > 0 { + sb.WriteString("\nChanged files:\n") + const show = 10 + for i, f := range files { + if i == show { + fmt.Fprintf(&sb, "- (+%d more)\n", len(files)-show) + break + } + fmt.Fprintf(&sb, "- `%s`\n", f) + } + } + return sb.String(), true } // briefingScope names the working tree detect_changes actually diffed. @@ -248,10 +329,13 @@ func renderUnattributedFiles(files []string, capped bool) string { // named rather than dropped. When the session cannot be identified at all the // briefing falls back to the whole tree and labels itself as such. func buildPostTaskBriefing(port int, scope sessionScope) string { - changes := detectChangeSet(port) - if changes == nil { + changes, answered := detectChangeSet(port) + if answered && changes == nil { return "" } + if briefing, lagging := laggingChangesBriefing("## Gortex Working-Tree Diagnostics", scope, changes); lagging { + return briefing + } if len(changes.ChangedSymbols) == 0 { // No indexed symbols touched — skip silently. The agent doesn't need @@ -368,8 +452,14 @@ func buildPostTaskBriefing(port int, scope sessionScope) string { // so nothing is ever recorded for it and it always resolves to the labeled // whole-tree fallback — this does not give Codex attribution coverage. func buildMutationBriefing(port int, scope sessionScope) string { - changes := detectChangeSet(port) - if changes == nil || len(changes.ChangedSymbols) == 0 { + changes, answered := detectChangeSet(port) + if answered && changes == nil { + return "" + } + if briefing, lagging := laggingChangesBriefing("## Gortex mutation follow-up", scope, changes); lagging { + return briefing + } + if len(changes.ChangedSymbols) == 0 { return "" } @@ -464,7 +554,9 @@ func renderGuardViolations(port int, idsCSV string) string { // renderDeadCodeHits filters analyze:dead_code results to the intersection // with the currently-changed symbols (i.e. "did this task leave anything -// orphaned"). Emits nothing when the intersection is empty. +// orphaned"). Emits nothing when the intersection is empty. A symbol listed +// on a "pending_unknown " line has unknown liveness while edits are pending +// and is never reported as dead. func renderDeadCodeHits(port int, ids []string) string { raw := callServerTool(port, "analyze", map[string]any{ "kind": "dead_code", @@ -475,14 +567,22 @@ func renderDeadCodeHits(port int, ids []string) string { return "" } + lines := strings.Split(raw, "\n") idSet := make(map[string]bool, len(ids)) for _, id := range ids { idSet[id] = true } + for _, line := range lines { + if rest, unknown := strings.CutPrefix(line, "pending_unknown "); unknown { + for _, field := range strings.Fields(rest) { + delete(idSet, field) + } + } + } var hits []string - for _, line := range strings.Split(raw, "\n") { - if line == "" { + for _, line := range lines { + if line == "" || strings.HasPrefix(line, "pending_unknown ") { continue } // Each compact line starts with the symbol descriptor including the diff --git a/internal/hooks/posttask_test.go b/internal/hooks/posttask_test.go index b55f14b90..2dda76944 100644 --- a/internal/hooks/posttask_test.go +++ b/internal/hooks/posttask_test.go @@ -3,9 +3,12 @@ package hooks import ( "encoding/json" "fmt" + "net/http" + "net/http/httptest" "path/filepath" "strings" "testing" + "time" "github.com/zzet/gortex/internal/daemon" "github.com/zzet/gortex/internal/localizationauth" @@ -189,6 +192,114 @@ func TestRunPostTask_DeadCodeFiltersToChanged(t *testing.T) { if strings.Contains(out, "API Contract Issues") { t.Errorf("contract section should be omitted when clean:\n%s", out) } + + // A changed symbol whose liveness is unknown while edits are pending is + // never reported as dead. + pending := newFakeServer(map[string]string{ + "analyze": "relations: pending (1 files)\npending_unknown function foo.go::Foo foo.go:3\n", + }) + defer pending.Close() + if hits := renderDeadCodeHits(portFromURL(t, pending.URL), []string{"foo.go::Foo"}); hits != "" { + t.Errorf("pending_unknown symbol reported as dead:\n%s", hits) + } +} + +// While the graph is still publishing edits, detect_changes answers with a +// labelled partial. Both briefings must say so and name the changed files +// rather than render diagnostics over lagging relations or fall silent. +func TestPostTaskBriefingPrintsPendingRelations(t *testing.T) { + for name, detect := range map[string]string{ + "primary": `{"changed_files":["a.go","b.go"],"changed_symbols":[{"id":"a.go::A","name":"A","kind":"function"}], + "risk":"LOW","complete":false,"graph_freshness":"pending","pending_file_count":2,"pending_files":["a.go","b.go"]}`, + "worktree": `{"changed_files":["a.go"],"changed_symbols":[],"risk":"UNKNOWN","complete":false,"graph_status":"pending"}`, + } { + srv := newRecordingFakeServer(map[string]string{"detect_changes": detect, "get_test_targets": "a_test.go TestA\n"}) + port := portFromURL(t, srv.URL) + started := time.Now() + for builder, briefing := range map[string]string{ + "stop": buildPostTaskBriefing(port, sessionScope{}), + "mutation": buildMutationBriefing(port, sessionScope{}), + } { + want := "relations: pending (2 files)" + if name == "worktree" { + want = "relations: pending (1 files)" + } + if !strings.Contains(briefing, want) || !strings.Contains(briefing, "`a.go`") { + t.Errorf("%s %s briefing missing %q and the changed files:\n%s", name, builder, want, briefing) + } + } + if n := srv.callCount("get_test_targets"); n != 0 { + t.Errorf("%s: diagnostics ran over lagging relations (%d get_test_targets calls)", name, n) + } + // The daemon waits for pending edits until wait_deadline and labels + // the rest. Without one it waits past the budget, and the labelled + // answer arrives after the briefing has given up on it. + for _, args := range srv.argsFor("detect_changes") { + deadline, err := time.Parse(time.RFC3339Nano, fmt.Sprint(args["wait_deadline"])) + if err != nil || !deadline.After(started) || !deadline.Before(started.Add(detectChangesBudget)) { + t.Errorf("%s: detect_changes wait_deadline %v does not end inside the %v budget", name, args["wait_deadline"], detectChangesBudget) + } + } + srv.Close() + } +} + +// A detect_changes that does not answer within its budget is reported, and +// the briefing returns well inside the half-second hook budget. So is one the +// daemon reached only after its wait_deadline had passed, which it refuses. +// A session that recorded no writes is not told about a slow daemon. +func TestPostTaskBriefingBoundedWhenDetectStalls(t *testing.T) { + t.Setenv(hookSessionDirEnvVar, t.TempDir()) + saveSessionState("writer", sessionState{WrittenPaths: []string{"a.go"}}) + release := make(chan struct{}) + stalled := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + select { + case <-release: + case <-time.After(5 * time.Second): + } + http.NotFound(w, r) + })) + defer stalled.Close() + defer close(release) + late := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var req struct { + Arguments map[string]any `json:"arguments"` + } + _ = json.NewDecoder(r.Body).Decode(&req) + if deadline, err := time.Parse(time.RFC3339Nano, fmt.Sprint(req.Arguments["wait_deadline"])); err == nil { + time.Sleep(time.Until(deadline) + 10*time.Millisecond) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"content":[{"type":"text","text":"wait_deadline is not in the future"}],"isError":true}`)) + })) + defer late.Close() + + for name, srv := range map[string]*httptest.Server{"stalled": stalled, "refused late": late} { + port := portFromURL(t, srv.URL) + for builder, build := range map[string]func(int, sessionScope) string{ + "stop": buildPostTaskBriefing, + "mutation": buildMutationBriefing, + } { + for session, report := range map[string]bool{"": true, "writer": true, "reader": false} { + if name != "stalled" && session != "" { + continue + } + started := time.Now() + briefing := build(port, sessionScope{SessionID: session}) + if elapsed := time.Since(started); elapsed > 500*time.Millisecond { + t.Errorf("%s %s briefing took %v", name, builder, elapsed) + } + switch { + case !report && briefing != "": + t.Errorf("%s %s briefing warned session %q, which wrote nothing:\n%q", name, builder, session, briefing) + case report && !strings.Contains(briefing, "relations: pending"): + t.Errorf("%s %s briefing did not report the busy detect_changes to session %q:\n%q", name, builder, session, briefing) + case report && strings.Contains(briefing, "publishing"): + t.Errorf("%s %s briefing claims edits are publishing without an answer:\n%q", name, builder, briefing) + } + } + } + } } // contractsCompact builds a realistic `contracts check --compact` payload: diff --git a/internal/hooks/precompact.go b/internal/hooks/precompact.go index a086c2393..a7cc8ddd4 100644 --- a/internal/hooks/precompact.go +++ b/internal/hooks/precompact.go @@ -232,6 +232,23 @@ func callServerTool(port int, name string, args map[string]any) string { return callServerToolHTTP(port, name, args) } +// callServerToolWithin is callServerTool bounded by timeout. answered is false +// when the call did not return in time: the bridge is reachable but busy, +// which a briefing reports instead of folding it into the silence of an +// unreachable bridge. The abandoned call ends with the hook process. +func callServerToolWithin(port int, name string, args map[string]any, timeout time.Duration) (raw string, answered bool) { + done := make(chan string, 1) + go func() { done <- callServerTool(port, name, args) }() + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case raw = <-done: + return raw, true + case <-timer.C: + return "", false + } +} + // callServerToolHTTP issues a POST /v1/tools/{name} against the server and // returns the text content from the first response block. Returns empty string // on any error (server unreachable, non-200, missing content). diff --git a/internal/indexer/go_module_extern_test.go b/internal/indexer/go_module_extern_test.go index e8546be5f..147ee509e 100644 --- a/internal/indexer/go_module_extern_test.go +++ b/internal/indexer/go_module_extern_test.go @@ -153,8 +153,17 @@ func Caller() { library.Hello(); child.Nested() } } case "parallel": errors := make(chan error, len(repos)) + // This is a module-binding test, not an identity-admission + // saturation test. The decoy case has three roots; retain + // parallel tracking within the two native observation slots. + admitted := make(chan struct{}, 2) for _, entry := range repos { - go func() { _, e := mi.TrackRepoCtx(context.Background(), entry); errors <- e }() + go func() { + admitted <- struct{}{} + defer func() { <-admitted }() + _, e := mi.TrackRepoCtx(context.Background(), entry) + errors <- e + }() } for range repos { require.NoError(t, <-errors) diff --git a/internal/indexer/prefix_observation_publication_test.go b/internal/indexer/prefix_observation_publication_test.go new file mode 100644 index 000000000..26df8a0ec --- /dev/null +++ b/internal/indexer/prefix_observation_publication_test.go @@ -0,0 +1,71 @@ +package indexer + +import ( + "context" + "os" + "testing" + "testing/synctest" + "time" + + "github.com/stretchr/testify/require" +) + +// Publication must mean the native worker has returned its allowance. A caller +// often starts another observation immediately; announcing completion before +// permit return can spuriously exhaust both slots under parallel tracking. +func TestIdentityObservationPublishesOnlyAfterPermitReturn(t *testing.T) { + root := t.TempDir() + for _, operation := range []string{"stat", "naming"} { + t.Run(operation, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + var allowance prefixStatAllowance + allowance.once.Do(func() { allowance.slots = make(chan struct{}, 2) }) + nativeDone := make(chan struct{}) + // Temporarily intercept this worker's token so its release must + // wait for the test. This makes publication ordering observable + // without scheduler timing, polling or an extra production hook. + finishNative := func() { + <-allowance.slots + close(nativeDone) + } + ctx, cancel := context.WithTimeout(t.Context(), time.Minute) + defer cancel() + result := make(chan error, 1) + if operation == "stat" { + allowance.stat = func(path string) (os.FileInfo, error) { + info, err := os.Stat(path) + finishNative() + return info, err + } + go func() { result <- allowance.observe(ctx, root).err }() + } else { + go func() { + _, err := allowance.observeNaming(ctx, root, func(context.Context, string) trackNamingFacts { + finishNative() + return trackNamingFacts{canonical: root} + }) + result <- err + }() + } + <-nativeDone + synctest.Wait() + var publishedEarly bool + var err error + select { + case err = <-result: + publishedEarly = true + default: + } + // Always let the worker finish before assertions, including RED. + allowance.slots <- struct{}{} + synctest.Wait() + if !publishedEarly { + err = <-result + } + require.NoError(t, err) + require.Zero(t, len(allowance.slots)) + require.False(t, publishedEarly, "completed identity result escaped before its permit was returned") + }) + }) + } +} diff --git a/internal/indexer/prefix_ownership.go b/internal/indexer/prefix_ownership.go index a8da85371..75beaf395 100644 --- a/internal/indexer/prefix_ownership.go +++ b/internal/indexer/prefix_ownership.go @@ -60,9 +60,14 @@ func (a *prefixStatAllowance) observe(ctx context.Context, path string) prefixSt slots := a.slots done := make(chan prefixStatResult, 1) go func() { - defer func() { <-slots }() - info, err := stat(path) - done <- prefixStatResult{info: info, err: err} + result := func() prefixStatResult { + defer func() { <-slots }() + info, err := stat(path) + return prefixStatResult{info: info, err: err} + }() + // A completed result authorizes the caller's next observation. + // Return this permit first, even when the receiver runs immediately. + done <- result }() select { case <-ctx.Done(): diff --git a/internal/indexer/track_naming.go b/internal/indexer/track_naming.go index 3505de97f..72b9dfb9b 100644 --- a/internal/indexer/track_naming.go +++ b/internal/indexer/track_naming.go @@ -86,8 +86,12 @@ func (a *prefixStatAllowance) observeNaming(ctx context.Context, root string, re workerCtx, cancel := context.WithDeadline(context.Background(), deadline) defer cancel() go func() { - defer func() { <-slots }() - done <- read(workerCtx, root) + facts := func() trackNamingFacts { + defer func() { <-slots }() + return read(workerCtx, root) + }() + // Completion includes permit return, as for root Stat observations. + done <- facts }() select { case <-ctx.Done(): diff --git a/internal/mcp/change_contract.go b/internal/mcp/change_contract.go index 81d47c99e..325397fba 100644 --- a/internal/mcp/change_contract.go +++ b/internal/mcp/change_contract.go @@ -897,6 +897,9 @@ func (s *Server) handleChangeContract(ctx context.Context, req mcp.CallToolReque if s == nil || s.graph == nil { return mcp.NewToolResultError("change_contract: server not fully initialised"), nil } + // Snapshot the unpublished edits before the walk; the changed symbols + // that scope the label are known only once it has run. + gaps := s.laggingMutationReceipts(ctx) p, err := s.lowerChange(ctx, req) if err != nil { if ctxErr := requestContextError(ctx, err); ctxErr != nil { @@ -909,5 +912,7 @@ func (s *Server) handleChangeContract(ctx context.Context, req mcp.CallToolReque } violations := s.evaluateChange(ctx, p) env := s.assembleEnvelope(ctx, p, violations) - return s.respondJSONOrTOON(ctx, req, env) + label := s.labelRelations(ctx, gaps, p.changedIDs, nil) + res, err := s.respondJSONOrTOON(ctx, req, label.labelled(env)) + return label.decorate(res), err } diff --git a/internal/mcp/gcx.go b/internal/mcp/gcx.go index ef2b16ae0..2a4caddb3 100644 --- a/internal/mcp/gcx.go +++ b/internal/mcp/gcx.go @@ -1937,8 +1937,13 @@ func encodeChangeImpact(result map[string]any) ([]byte, error) { communityNote, _ := result["community_note"].(string) contractRiskUpgrade, _ := result["contract_risk_upgrade"].(string) + var meta []string + if waited, ok := result["barrier_wait_ms"].(int64); ok { + meta = append(meta, "barrier_wait_ms", strconv.FormatInt(waited, 10)) + } sumEnc := newGCX(&buf, "explain_change_impact.summary", []string{"risk", "summary", "total_affected", "cross_repo", "processes", "communities", "test_files", "warning", "note", "risk_upgrade"}, + meta..., ) if err := sumEnc.WriteRow( string(risk), diff --git a/internal/mcp/mutation_freshness_hints_test.go b/internal/mcp/mutation_freshness_hints_test.go index eb5f96d73..9e1cd9ffb 100644 --- a/internal/mcp/mutation_freshness_hints_test.go +++ b/internal/mcp/mutation_freshness_hints_test.go @@ -105,7 +105,7 @@ func TestFreshnessRefusalAnnotationBoundsAndScope(t *testing.T) { // Exercise the public call shapes against real handlers and an owned working // tree. The existing watcher seam supplies a failed publication result after // the edit acknowledges; this does not inject a real store patch failure. -func TestFreshnessRefusalRecoveryFlow(t *testing.T) { +func TestRelationFreshnessRecoveryFlow(t *testing.T) { s, root := setupTestServer(t) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -144,14 +144,10 @@ func TestFreshnessRefusalRecoveryFlow(t *testing.T) { waitCheckoutReceipt(t, s, id) detectArgs := map[string]any{"operation": "detect", "options": map[string]any{"scope": "unstaged"}} - refused := call("change", detectArgs) - require.True(t, refused.IsError) - for _, want := range []string{ - "graph freshness unavailable", "change.receipt options.receipt=" + id, - "workspace_admin operation=reindex", "paths=[P]", `mutation_id="recovery-flow"`, - } { - require.Contains(t, resultText(refused), want) - } + partial := resultJSON(t, call("change", detectArgs)) + require.Equal(t, false, partial["complete"]) + require.Equal(t, "failed", partial["graph_freshness"]) + require.NotEmpty(t, partial["recovery_hint"]) status := resultJSON(t, call("change", map[string]any{ "operation": "receipt", "options": map[string]any{"mutation_id": id}, })) diff --git a/internal/mcp/mutation_freshness_test.go b/internal/mcp/mutation_freshness_test.go index d9dd4b21c..011c5e5fa 100644 --- a/internal/mcp/mutation_freshness_test.go +++ b/internal/mcp/mutation_freshness_test.go @@ -434,41 +434,44 @@ func TestMutationReposForSymbolIDsUnresolvedWidensBarrier(t *testing.T) { } } -func TestChangeImpactFreshnessGuardAndRetry(t *testing.T) { +func TestChangeImpactLabelsLaggingReceiptsInScope(t *testing.T) { g := graph.New() g.AddNode(&graph.Node{ ID: "repo-a::target", Kind: graph.KindFunction, Name: "target", - FilePath: "target.go", + FilePath: "/repo-a/target.go", StartLine: 1, EndLine: 1, Language: "go", RepoPrefix: "repo-a", }) s := &Server{ - graph: g, - engine: query.NewEngine(g), - session: newSessionState(), - mutationSafetyWait: time.Millisecond, + graph: g, + engine: query.NewEngine(g), + session: newSessionState(), } receipt := pendingFreshnessReceipt(s, "receipt-impact", "repo-a", "/repo-a/target.go", 11) req := mcpgo.CallToolRequest{Params: mcpgo.CallToolParams{ Name: "explain_change_impact", Arguments: map[string]any{"ids": "repo-a::target", "format": "json"}, }} - - result, err := s.handleEnhancedChangeImpact(context.Background(), req) - if err != nil { - t.Fatalf("pending impact handler error: %v", err) + impact := func() map[string]any { + t.Helper() + result, err := s.handleEnhancedChangeImpact(context.Background(), req) + if err != nil { + t.Fatalf("impact handler error: %v", err) + } + return lifecycleResultPayload(t, result) } - if result == nil || !result.IsError { - t.Fatalf("pending impact result = %#v, want MCP error", result) + + // A pending edit to the symbol's own file is labelled, never refused. + payload := impact() + if payload["graph_freshness"] != "pending" || payload["complete"] != false { + t.Fatalf("pending impact = %v, want a pending partial", payload) } - for _, want := range []string{"change impact refused a stale graph", "receipt-impact", "generation=11"} { - if !strings.Contains(freshnessToolResultText(result), want) { - t.Fatalf("pending impact response %q does not contain %q", freshnessToolResultText(result), want) - } + if files, _ := payload["pending_files"].([]any); len(files) != 1 || files[0] != "/repo-a/target.go" { + t.Fatalf("pending impact files = %v", payload["pending_files"]) } completeFreshnessReceipt(receipt, indexer.MutationResult{ @@ -476,22 +479,31 @@ func TestChangeImpactFreshnessGuardAndRetry(t *testing.T) { AppliedGeneration: 11, Reindexed: true, }) - result, err = s.handleEnhancedChangeImpact(context.Background(), req) - if err != nil { - t.Fatalf("completed impact handler error: %v", err) + if payload = impact(); payload["graph_freshness"] != nil { + t.Fatalf("published edit still labelled: %v", payload) + } + + // A terminally failed edit in scope is a labelled partial with a + // recovery hint, not a tool error. + failed := pendingFreshnessReceipt(s, "receipt-failed", "repo-a", "/repo-a/target.go", 12) + completeFreshnessReceipt(failed, indexer.MutationResult{ + RequestedGeneration: 12, + AppliedGeneration: 12, + Err: errors.New("syntax patch failed"), + }) + payload = impact() + if payload["graph_freshness"] != "failed" || payload["complete"] != false { + t.Fatalf("failed impact = %v, want a failed partial", payload) } - if result == nil || result.IsError { - t.Fatalf("completed impact remained blocked: %q", freshnessToolResultText(result)) + if hint, _ := payload["recovery_hint"].(string); !strings.Contains(hint, "reindex") { + t.Fatalf("failed impact recovery hint = %q", hint) } } -func TestDetectChangesFreshnessGuard(t *testing.T) { - s := &Server{ - graph: graph.New(), - session: newSessionState(), - mutationSafetyWait: time.Millisecond, - } - pendingFreshnessReceipt(s, "receipt-detect", "repo-a", "/repo-a/changed.go", 12) +func TestDetectChangesLabelsPendingChangedFile(t *testing.T) { + s, root := newRelationFixture(t) + pendingFreshnessReceipt(s, "receipt-detect", "", + writeUnindexed(t, root, "caller.go", "package p\n\nfunc Caller() {\n\tTarget()\n\tTarget()\n}\n"), 12) req := mcpgo.CallToolRequest{Params: mcpgo.CallToolParams{ Name: "detect_changes", Arguments: map[string]any{"format": "json"}, @@ -501,12 +513,54 @@ func TestDetectChangesFreshnessGuard(t *testing.T) { if err != nil { t.Fatalf("detect handler error: %v", err) } - if result == nil || !result.IsError { - t.Fatalf("pending detect result = %#v, want MCP error", result) + payload := lifecycleResultPayload(t, result) + for key, want := range map[string]any{ + "complete": false, + "graph_freshness": "pending", + "pending_file_count": float64(1), + } { + if payload[key] != want { + t.Fatalf("detect %s = %v, want %v: %v", key, payload[key], want, payload) + } + } + if files, _ := payload["changed_files"].([]any); len(files) != 1 || files[0] != "caller.go" { + t.Fatalf("detect changed_files = %v", payload["changed_files"]) + } + // The walk still ran: the changed symbol is mapped and risk is scored. + if symbols, _ := payload["changed_symbols"].([]any); len(symbols) == 0 || payload["risk"] == "UNKNOWN" { + t.Fatalf("labelled detect skipped the walk: %v", payload) } - for _, want := range []string{"change detection refused a stale graph", "receipt-detect", "generation=12"} { - if !strings.Contains(freshnessToolResultText(result), want) { - t.Fatalf("pending detect response %q does not contain %q", freshnessToolResultText(result), want) + + // The Stop hook bounds the wait with an earlier wait_deadline, which can + // pass between admission and the handler reading it. Either way the + // answer is labelled without sitting out the default budget. + for _, offset := range []time.Duration{50 * time.Millisecond, -time.Second} { + req.Params.Arguments = map[string]any{"format": "json", + "wait_deadline": time.Now().Add(offset).Format(time.RFC3339Nano)} + result, err = s.handleDetectChanges(context.Background(), req) + if err != nil { + t.Fatalf("detect handler error: %v", err) + } + payload = lifecycleResultPayload(t, result) + if waited, _ := payload["barrier_wait_ms"].(float64); payload["graph_freshness"] != "pending" || waited >= 200 { + t.Fatalf("detect with wait_deadline %v: graph_freshness %v after %v ms", offset, payload["graph_freshness"], waited) } } + + // A staged diff leaves the pending caller.go out of the changed files, + // but the walk still reads it as a caller of the changed Target. + writeUnindexed(t, root, "target.go", "package p\n\nfunc Target() { _ = 0 }\n") + checkoutMutationGit(t, root, "add", "target.go") + req.Params.Arguments = map[string]any{"format": "json", "scope": "staged"} + result, err = s.handleDetectChanges(context.Background(), req) + if err != nil { + t.Fatalf("detect handler error: %v", err) + } + payload = lifecycleResultPayload(t, result) + if files, _ := payload["changed_files"].([]any); len(files) != 1 || files[0] != "target.go" { + t.Fatalf("staged detect changed_files = %v", payload["changed_files"]) + } + if pending, _ := payload["pending_files"].([]any); payload["graph_freshness"] != "pending" || len(pending) != 1 || pending[0] != "caller.go" { + t.Fatalf("staged detect left the pending caller unlabelled: %v", payload) + } } diff --git a/internal/mcp/relation_freshness.go b/internal/mcp/relation_freshness.go new file mode 100644 index 000000000..b586cf68b --- /dev/null +++ b/internal/mcp/relation_freshness.go @@ -0,0 +1,376 @@ +package mcp + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "sort" + "strings" + "time" + + wire "github.com/gortexhq/gcx-go" + "github.com/mark3labs/mcp-go/mcp" + + "github.com/zzet/gortex/internal/graph" +) + +// relationFreshnessWait bounds how long a change tool waits for the edits in +// its relation scope before it answers with a label instead. +const relationFreshnessWait = 250 * time.Millisecond + +// maxLabelledPendingFiles caps the pending_files list of a label; +// pending_file_count always carries the full count. +const maxLabelledPendingFiles = 16 + +// relationFreshness labels an answer read from graph relations while edits in +// its scope are still being published. The zero value is a fresh scope. +type relationFreshness struct { + state string // "", "pending" or "failed" + files []string // graph-keyed paths of the lagging in-scope edits, sorted + waited time.Duration +} + +// relationWaitDeadline is when a change tool stops waiting for its relation +// scope: relationFreshnessWait from now, or an earlier wait_deadline. With +// require_fresh it is the request's own freshness deadline instead. The +// middleware has already refused a malformed or past wait_deadline, so one +// the handler cannot take passed after admission and ends the wait now. +func relationWaitDeadline(ctx context.Context, freshness requestFreshness) time.Time { + now := time.Now() + if freshness.err != nil { + return now + } + if freshness.requireFresh { + return freshness.effectiveDeadline(now, ctx) + } + deadline := now.Add(relationFreshnessWait) + if freshness.hasDeadline && freshness.deadline.Before(deadline) { + return freshness.deadline + } + return deadline +} + +// awaitRelationFreshness waits until deadline for the edits in the relation +// scope of ids and files, then labels whatever in that scope is still pending +// or failed. A deadline already passed labels without waiting. Edits outside +// the scope are neither waited on nor reported. +func (s *Server) awaitRelationFreshness(ctx context.Context, ids, files []string, deadline time.Time) relationFreshness { + started := time.Now() + lagging := s.laggingMutationReceipts(ctx) + if len(lagging) == 0 { + return relationFreshness{waited: time.Since(started)} + } + inScope := s.relationScope(ctx, ids, files) + if remaining := time.Until(deadline); remaining > 0 { + timer := time.NewTimer(remaining) + defer timer.Stop() + wait: + for _, receipt := range lagging { + if !inScope(receipt.path) { + continue + } + select { + case <-receipt.done: + case <-timer.C: + break wait + case <-ctx.Done(): + break wait + } + } + lagging = s.laggingMutationReceipts(ctx) + } + label := s.relationLabel(lagging, inScope) + label.waited = time.Since(started) + return label +} + +// labelRelations labels an answer that read relations without waiting. gaps +// is the laggingMutationReceipts snapshot taken before the walk; the scope of +// ids and files, known only once the walk has run, filters it afterwards. An +// edit that published during the walk still labels the answer, which may +// have read around it. +func (s *Server) labelRelations(ctx context.Context, gaps []*mutationReceipt, ids, files []string) relationFreshness { + if len(gaps) == 0 { + return relationFreshness{} + } + return s.relationLabel(gaps, s.relationScope(ctx, ids, files)) +} + +// relationLabel labels the receipts that fall in scope: failed when the edit +// failed to publish, pending otherwise. One pending receipt for a path +// outweighs an older failed one, and any failed path makes the label failed. +func (s *Server) relationLabel(receipts []*mutationReceipt, inScope func(absPath string) bool) relationFreshness { + states := make(map[string]string) + for _, receipt := range receipts { + if !inScope(receipt.path) { + continue + } + key := s.repoRelative(receipt.path) + outcome := receipt.outcome(true) + if outcome.Pending || (outcome.Err == nil && outcome.Reindexed) { + states[key] = "pending" + } else if states[key] == "" { + states[key] = "failed" + } + } + var label relationFreshness + for path, state := range states { + label.files = append(label.files, path) + if label.state != "failed" { + label.state = state + } + } + sort.Strings(label.files) + return label +} + +// laggingMutationReceipts returns the request's mutation receipts whose edit +// the graph has not published: still pending, or terminally failed. +func (s *Server) laggingMutationReceipts(ctx context.Context) []*mutationReceipt { + var lagging []*mutationReceipt + for _, receipt := range s.mutationReceiptsForRepos(ctx) { + if outcome := receipt.outcome(true); outcome.Pending || outcome.Err != nil || !outcome.Reindexed { + lagging = append(lagging, receipt) + } + } + return lagging +} + +// relationScope reports whether an edit to a file can change the relations +// of ids: the file defines one of them, holds an edge into one of them, is one +// of files (graph-keyed), or its current bytes name one of them as a whole +// identifier, which is how a call added by an edit the graph has not read yet +// shows up. An id that does not resolve contributes the file and name its +// spelling carries, so it narrows the scope instead of widening it. +func (s *Server) relationScope(ctx context.Context, ids, files []string) func(absPath string) bool { + scoped := make(map[string]bool, len(files)+len(ids)) + for _, file := range files { + scoped[file] = true + } + var names []string + reader := s.readerFor(ctx) + for _, id := range ids { + id = strings.TrimSpace(id) + if id == "" { + continue + } + node := reader.GetNode(id) + if node == nil { + if file := graph.IDFile(id); file != "" { + scoped[file] = true + } + names = append(names, identifierName(id[strings.LastIndex(id, ":")+1:])) + continue + } + scoped[node.FilePath] = true + names = append(names, identifierName(node.Name)) + for _, edge := range reader.GetInEdges(id) { + file := edge.FilePath + if file == "" { + file = graph.IDFile(edge.From) + } + if file != "" { + scoped[file] = true + } + } + } + named := make(map[string]bool) + return func(absPath string) bool { + if scoped[s.repoRelative(absPath)] { + return true + } + hit, seen := named[absPath] + if !seen && len(names) > 0 { + data, err := os.ReadFile(absPath) + hit = err == nil && namesAnyIdentifier(data, names) + named[absPath] = hit + } + return hit + } +} + +// identifierName is the last identifier of a possibly qualified name, the +// spelling a call site uses: Recv.Method is called as Method. +func identifierName(name string) string { + return name[strings.LastIndex(name, ".")+1:] +} + +// namesAnyIdentifier reports whether data holds one of names as a whole +// identifier rather than as part of a longer one. +func namesAnyIdentifier(data []byte, names []string) bool { + for _, name := range names { + if name == "" { + continue + } + needle := []byte(name) + for offset := 0; offset < len(data); { + i := bytes.Index(data[offset:], needle) + if i < 0 { + break + } + start, end := offset+i, offset+i+len(needle) + if (start == 0 || !isIdentByte(data[start-1])) && (end == len(data) || !isIdentByte(data[end])) { + return true + } + offset = start + 1 + } + } + return false +} + +// apply writes the label into an answer's fields. A fresh scope writes +// nothing, so an answer with nothing lagging keeps its shape. +func (f relationFreshness) apply(result map[string]any) { + if f.state == "" { + return + } + result["complete"] = false + result["graph_freshness"] = f.state + result["freshness_scope"] = "relations" + result["pending_file_count"] = len(f.files) + result["pending_files"] = f.files[:min(maxLabelledPendingFiles, len(f.files))] + if f.state == "failed" { + result["recovery_hint"] = mutationFreshnessRecoveryHint() + } +} + +// labelled is payload with the label's fields merged in, a struct payload +// being re-read as its JSON object first. A fresh scope returns payload +// unchanged. +func (f relationFreshness) labelled(payload any) any { + if f.state == "" { + return payload + } + data, err := json.Marshal(payload) + if err != nil { + return payload + } + var fields map[string]any + if json.Unmarshal(data, &fields) != nil || fields == nil { + return payload + } + f.apply(fields) + return fields +} + +// line is the relations line a text answer leads with. +func (f relationFreshness) line() string { + return fmt.Sprintf("relations: %s (%d files)", f.state, len(f.files)) +} + +// requireFreshError is the answer to a require_fresh call whose wait ended +// with relations in its scope still lagging. +func (f relationFreshness) requireFreshError() *mcp.CallToolResult { + message := fmt.Sprintf("require_fresh: %s after waiting %d ms: %s", + f.line(), f.waited.Milliseconds(), strings.Join(f.files[:min(maxLabelledPendingFiles, len(f.files))], ", ")) + if f.state == "failed" { + message += "; " + mutationFreshnessRecoveryHint() + } + return mcp.NewToolResultError(message) +} + +// merge folds the label of another scope the same answer read into f: the +// files of both, failed over pending, and the time spent waiting on each. +func (f relationFreshness) merge(other relationFreshness) relationFreshness { + f.waited += other.waited + if other.state == "" { + return f + } + if f.state != "failed" { + f.state = other.state + } + seen := make(map[string]bool, len(f.files)) + for _, file := range f.files { + seen[file] = true + } + for _, file := range other.files { + if !seen[file] { + f.files = append(f.files, file) + } + } + sort.Strings(f.files) + return f +} + +// decorate makes a rendered text answer lead with its label: TOON and plain +// text start with the relations line; GCX keeps GCX1 as line 1, carries the +// label as header meta and the relations line as a comment on line 2. JSON +// already carries the label in its fields and passes through unchanged. +func (f relationFreshness) decorate(res *mcp.CallToolResult) *mcp.CallToolResult { + if f.state == "" || res == nil || res.IsError || len(res.Content) == 0 { + return res + } + text, ok := res.Content[0].(mcp.TextContent) + if !ok || strings.HasPrefix(strings.TrimSpace(text.Text), "{") { + return res + } + body := text.Text + if strings.HasPrefix(body, wire.Tag) { + body, _ = injectGCXHeaderMeta(body, map[string]any{ + "graph_freshness": f.state, + "freshness_scope": "relations", + "pending_file_count": len(f.files), + }) + head, rows, _ := strings.Cut(body, wire.RowSep) + body = head + wire.RowSep + wire.CommentPrefix + " " + f.line() + wire.RowSep + rows + } else { + body = f.line() + "\n" + body + } + return rebuildTextResult(res, body) +} + +// provisionalSite is a line of a file whose edit the graph has not published +// that names a symbol as a whole identifier: a reference the graph cannot show +// yet, reported so a mutator neither misses it nor rewrites it. Line is 0 for +// the symbol's own file, which is provisional as a whole. +type provisionalSite struct { + File string `json:"file"` + Line int `json:"line"` + Text string `json:"text"` +} + +// provisionalSites scans the files of gaps for lines naming node, leaving out +// the lines where g already holds a reference to it. When the node's own file +// is among them it is one site without a line: until that edit publishes, the +// graph's span of the node may not hold, and a line that names the node there +// may be a new caller or the definition itself. +func (s *Server) provisionalSites(g graph.Reader, gaps []*mutationReceipt, node *graph.Node) []provisionalSite { + names := []string{identifierName(node.Name)} + known := make(map[string]bool) + for _, e := range g.GetInEdges(node.ID) { + if caller := g.GetNode(e.From); caller != nil && isReferencingEdgeKind(e.Kind) { + known[fmt.Sprintf("%s:%d", caller.FilePath, e.Line)] = true + } + } + seen := make(map[string]bool) + var sites []provisionalSite + for _, receipt := range gaps { + file := s.repoRelative(receipt.path) + if seen[file] { + continue + } + seen[file] = true + if file == node.FilePath { + sites = append(sites, provisionalSite{File: file}) + continue + } + data, err := os.ReadFile(receipt.path) + if err != nil || !namesAnyIdentifier(data, names) { + continue + } + for i, line := range strings.Split(string(data), "\n") { + if namesAnyIdentifier([]byte(line), names) && !known[fmt.Sprintf("%s:%d", file, i+1)] { + sites = append(sites, provisionalSite{File: file, Line: i + 1, Text: strings.TrimSpace(line)}) + } + } + } + sort.Slice(sites, func(i, j int) bool { + if sites[i].File != sites[j].File { + return sites[i].File < sites[j].File + } + return sites[i].Line < sites[j].Line + }) + return sites +} diff --git a/internal/mcp/relation_freshness_test.go b/internal/mcp/relation_freshness_test.go new file mode 100644 index 000000000..368e8b28a --- /dev/null +++ b/internal/mcp/relation_freshness_test.go @@ -0,0 +1,321 @@ +package mcp + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + mcpgo "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/indexer" + "github.com/zzet/gortex/internal/query" +) + +// newRelationFixture indexes a committed git repository in which Target +// (target.go) is called from caller.go, beside the unrelated other.go and +// namer.go. It returns the server and the indexed root receipts are keyed by. +func newRelationFixture(t *testing.T) (*Server, string) { + t.Helper() + dir := t.TempDir() + for name, src := range map[string]string{ + "target.go": "package p\n\nfunc Target() {}\n", + "caller.go": "package p\n\nfunc Caller() {\n\tTarget()\n}\n", + "other.go": "package p\n\nfunc Other() {}\n", + "namer.go": "package p\n\nfunc Namer() {}\n", + } { + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(src), 0o644)) + } + checkoutMutationGit(t, dir, "init", "-q") + checkoutMutationGit(t, dir, "add", ".") + checkoutMutationGit(t, dir, "commit", "-q", "-m", "base") + + g := graph.New() + idx := indexer.New(g, testRegistry(), config.Default().Index, zap.NewNop()) + _, err := idx.Index(dir) + require.NoError(t, err) + return NewServer(query.NewEngine(g), g, idx, nil, zap.NewNop(), nil), idx.RootPath() +} + +// writeUnindexed changes a file on disk without telling the graph, the state +// a pending edit leaves behind. +func writeUnindexed(t *testing.T, root, name, src string) string { + t.Helper() + path := filepath.Join(root, name) + require.NoError(t, os.WriteFile(path, []byte(src), 0o644)) + return path +} + +func targetID(t *testing.T, s *Server) string { + t.Helper() + for _, n := range s.graph.AllNodes() { + if n.Name == "Target" && n.Kind == graph.KindFunction { + return n.ID + } + } + t.Fatal("Target is not indexed") + return "" +} + +func callChangeTool(t *testing.T, handler func(context.Context, mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error), name string, args map[string]any) *mcpgo.CallToolResult { + t.Helper() + req := mcpgo.CallToolRequest{Params: mcpgo.CallToolParams{Name: name, Arguments: args}} + result, err := handler(context.Background(), req) + require.NoError(t, err) + require.NotNil(t, result) + return result +} + +func TestChangeToolsIgnorePendingReceiptOutsideScope(t *testing.T) { + s, root := newRelationFixture(t) + writeUnindexed(t, root, "caller.go", "package p\n\nfunc Caller() {\n\tTarget()\n\tTarget()\n}\n") + pendingFreshnessReceipt(s, "receipt-other", "", filepath.Join(root, "other.go"), 1) + + impact := lifecycleResultPayload(t, callChangeTool(t, s.handleEnhancedChangeImpact, "explain_change_impact", + map[string]any{"ids": targetID(t, s), "format": "json"})) + detect := lifecycleResultPayload(t, callChangeTool(t, s.handleDetectChanges, "detect_changes", + map[string]any{"format": "json"})) + for tool, payload := range map[string]map[string]any{"impact": impact, "detect": detect} { + require.NotContains(t, payload, "graph_freshness", "%s labelled an edit outside its scope: %v", tool, payload) + require.Less(t, payload["barrier_wait_ms"], float64(100), "%s waited on an edit outside its scope", tool) + } + require.Equal(t, []any{"caller.go"}, detect["changed_files"]) + require.NotEmpty(t, detect["changed_symbols"]) +} + +func TestChangeToolsScopeIncludesFilesNamingTheSymbol(t *testing.T) { + s, root := newRelationFixture(t) + // namer.go now calls Target, but the graph has no edge for it yet; + // other.go names only a longer identifier. + pendingFreshnessReceipt(s, "receipt-namer", "", + writeUnindexed(t, root, "namer.go", "package p\n\nfunc Namer() {\n\tTarget()\n}\n"), 1) + pendingFreshnessReceipt(s, "receipt-other", "", + writeUnindexed(t, root, "other.go", "package p\n\nfunc Other() {\n\tTargetX()\n}\n"), 2) + + payload := lifecycleResultPayload(t, callChangeTool(t, s.handleEnhancedChangeImpact, "explain_change_impact", + map[string]any{"ids": targetID(t, s), "format": "json"})) + require.Equal(t, "pending", payload["graph_freshness"]) + require.Equal(t, "relations", payload["freshness_scope"]) + require.Equal(t, false, payload["complete"]) + require.Equal(t, []any{"namer.go"}, payload["pending_files"]) + require.Equal(t, float64(1), payload["pending_file_count"]) + require.Less(t, payload["barrier_wait_ms"], float64(500)) +} + +func TestChangeToolsUnresolvedIDDoesNotWidenBarrier(t *testing.T) { + s, root := newRelationFixture(t) + pendingFreshnessReceipt(s, "receipt-other", "", filepath.Join(root, "other.go"), 1) + pendingFreshnessReceipt(s, "receipt-fresh", "", + writeUnindexed(t, root, "fresh.go", "package p\n\nfunc Fresh() {}\n"), 2) + + // Defined by a pending edit the graph has not read: labelled, and bounded + // by the budget rather than by every receipt in the repository. + payload := lifecycleResultPayload(t, callChangeTool(t, s.handleEnhancedChangeImpact, "explain_change_impact", + map[string]any{"ids": "fresh.go::Fresh", "format": "json"})) + require.Equal(t, "pending", payload["graph_freshness"]) + require.Equal(t, []any{"fresh.go"}, payload["pending_files"]) + require.Less(t, payload["barrier_wait_ms"], float64(500)) + + // No pending edit relates to it: answered at once, unlabelled. + payload = lifecycleResultPayload(t, callChangeTool(t, s.handleEnhancedChangeImpact, "explain_change_impact", + map[string]any{"ids": "gone.go::Gone", "format": "json"})) + require.NotContains(t, payload, "graph_freshness") + require.Less(t, payload["barrier_wait_ms"], float64(100)) +} + +func TestChangeToolsRequireFreshWaitsUntilWaitDeadline(t *testing.T) { + s, root := newRelationFixture(t) + target := pendingFreshnessReceipt(s, "receipt-target", "", filepath.Join(root, "target.go"), 1) + caller := pendingFreshnessReceipt(s, "receipt-caller", "", + writeUnindexed(t, root, "caller.go", "package p\n\nfunc Caller() {\n\tTarget()\n\tTarget()\n}\n"), 2) + deadline := func(d time.Duration) string { return time.Now().Add(d).Format(time.RFC3339Nano) } + + // The receipt never lands: the wait runs past the default budget to the + // caller's deadline, then refuses. + started := time.Now() + result := callChangeTool(t, s.handleEnhancedChangeImpact, "explain_change_impact", + map[string]any{"ids": targetID(t, s), "require_fresh": true, "wait_deadline": deadline(700 * time.Millisecond)}) + require.True(t, result.IsError) + require.GreaterOrEqual(t, time.Since(started), 600*time.Millisecond) + require.Contains(t, freshnessToolResultText(result), "relations: pending (2 files)") + require.Contains(t, freshnessToolResultText(result), "target.go") + + result = callChangeTool(t, s.handleDetectChanges, "detect_changes", + map[string]any{"require_fresh": true, "wait_deadline": deadline(500 * time.Millisecond)}) + require.True(t, result.IsError) + require.Contains(t, freshnessToolResultText(result), "caller.go") + + // The receipts land after the default budget but before the deadline: + // the answer waits for them and carries no label. + go func() { + time.Sleep(400 * time.Millisecond) + completeFreshnessReceipt(target, indexer.MutationResult{RequestedGeneration: 1, AppliedGeneration: 1, Reindexed: true}) + completeFreshnessReceipt(caller, indexer.MutationResult{RequestedGeneration: 2, AppliedGeneration: 2, Reindexed: true}) + }() + started = time.Now() + payload := lifecycleResultPayload(t, callChangeTool(t, s.handleEnhancedChangeImpact, "explain_change_impact", + map[string]any{"ids": targetID(t, s), "format": "json", "require_fresh": true, "wait_deadline": deadline(5 * time.Second)})) + require.GreaterOrEqual(t, time.Since(started), 350*time.Millisecond) + require.NotContains(t, payload, "graph_freshness") +} + +func TestChangeAnswersStartWithRelationsLine(t *testing.T) { + s, root := newRelationFixture(t) + pendingFreshnessReceipt(s, "receipt-caller", "", + writeUnindexed(t, root, "caller.go", "package p\n\nfunc Caller() {\n\tTarget()\n\tTarget()\n}\n"), 1) + const line = "relations: pending (1 files)" + id := targetID(t, s) + + for _, format := range []string{"toon", "gcx"} { + impact := freshnessToolResultText(callChangeTool(t, s.handleEnhancedChangeImpact, "explain_change_impact", + map[string]any{"ids": id, "format": format})) + lines := strings.SplitN(impact, "\n", 3) + require.GreaterOrEqual(t, len(lines), 2, impact) + if format == "gcx" { + require.True(t, strings.HasPrefix(lines[0], "GCX1 "), impact) + require.Contains(t, lines[0], " graph_freshness=pending") + require.Contains(t, lines[0], " pending_file_count=1") + require.Contains(t, lines[0], " barrier_wait_ms=") + require.Equal(t, "# "+line, lines[1]) + } else { + require.Equal(t, line, lines[0], impact) + } + } + + detect := freshnessToolResultText(callChangeTool(t, s.handleDetectChanges, "detect_changes", + map[string]any{"format": "toon"})) + require.Equal(t, line, strings.SplitN(detect, "\n", 2)[0], detect) +} + +func nodeID(t *testing.T, s *Server, name string) string { + t.Helper() + for _, n := range s.graph.AllNodes() { + if n.Name == name && n.Kind == graph.KindFunction { + return n.ID + } + } + t.Fatalf("%s is not indexed", name) + return "" +} + +// A call to a symbol that an edit added and the graph has not read yet must +// block safe_delete like a graph reference, in plain and propagate mode. An +// edit to the symbol's own file blocks it as a whole: it may add a caller +// there, and the graph's span of the symbol may no longer hold. +func TestSafeDeleteRejectsTargetCalledFromPendingFile(t *testing.T) { + for name, edit := range map[string]struct { + file, src string + ref map[string]any + }{ + "caller file": {"namer.go", "package p\n\nfunc Namer() {\n\tOther()\n}\n", + map[string]any{"kind": "provisional", "file_path": "namer.go", "line": float64(4)}}, + "definition file": {"other.go", "package p\n\nfunc Extra() {\n\tOther()\n}\n\nfunc Other() {}\n", + map[string]any{"kind": "provisional", "file_path": "other.go"}}, + } { + t.Run(name, func(t *testing.T) { + s, root := newRelationFixture(t) + pendingFreshnessReceipt(s, "receipt-"+edit.file, "", writeUnindexed(t, root, edit.file, edit.src), 1) + otherPath := filepath.Join(root, "other.go") + before, err := os.ReadFile(otherPath) + require.NoError(t, err) + id := nodeID(t, s, "Other") + + payload := lifecycleResultPayload(t, callChangeTool(t, s.handleSafeDeleteSymbol, "safe_delete_symbol", + map[string]any{"id": id, "dry_run": false, "format": "json"})) + require.Equal(t, "rejected_has_references", payload["status"]) + require.Equal(t, []any{edit.ref}, payload["references"]) + require.Equal(t, "pending", payload["graph_freshness"]) + + payload = lifecycleResultPayload(t, callChangeTool(t, s.handleSafeDeleteSymbol, "safe_delete_symbol", + map[string]any{"id": id, "propagate": true, "dry_run": false, "format": "json"})) + require.Equal(t, "propagation_blocked_manual", payload["status"]) + require.Equal(t, float64(1), payload["manual"]) + + after, err := os.ReadFile(otherPath) + require.NoError(t, err) + require.Equal(t, string(before), string(after), "the target was deleted with a pending caller") + }) + } + + // A call the graph already holds is listed once, as the graph's + // reference, when its file has an unrelated pending edit; propagate can + // still remove it. + t.Run("graph reference", func(t *testing.T) { + s, root := newRelationFixture(t) + pendingFreshnessReceipt(s, "receipt-caller", "", + writeUnindexed(t, root, "caller.go", "package p\n\nfunc Caller() {\n\tTarget()\n}\n\nfunc More() {}\n"), 1) + payload := lifecycleResultPayload(t, callChangeTool(t, s.handleSafeDeleteSymbol, "safe_delete_symbol", + map[string]any{"id": targetID(t, s), "format": "json"})) + require.Equal(t, "rejected_has_references", payload["status"]) + require.Equal(t, float64(1), payload["reference_count"], "%v", payload["references"]) + payload = lifecycleResultPayload(t, callChangeTool(t, s.handleSafeDeleteSymbol, "safe_delete_symbol", + map[string]any{"id": targetID(t, s), "propagate": true, "format": "json"})) + require.Equal(t, "propagation_plan", payload["status"]) + require.Equal(t, float64(0), payload["manual"], "%v", payload["caller_patches"]) + }) + + // Target is orphaned with Caller by the graph's edges, but a pending edit + // now calls it: the cascade must not delete it with Caller. + t.Run("cascade member", func(t *testing.T) { + s, root := newRelationFixture(t) + pendingFreshnessReceipt(s, "receipt-namer", "", + writeUnindexed(t, root, "namer.go", "package p\n\nfunc Namer() {\n\tTarget()\n}\n"), 1) + sources := map[string]string{} + for _, name := range []string{"caller.go", "target.go"} { + data, err := os.ReadFile(filepath.Join(root, name)) + require.NoError(t, err) + sources[name] = string(data) + } + payload := lifecycleResultPayload(t, callChangeTool(t, s.handleSafeDeleteSymbol, "safe_delete_symbol", + map[string]any{"id": nodeID(t, s, "Caller"), "cascade": "apply", "dry_run": false, "format": "json"})) + require.Equal(t, "rejected_has_references", payload["status"], "%v", payload) + require.Equal(t, []any{map[string]any{"kind": "provisional", "file_path": "namer.go", "line": float64(4)}}, payload["references"]) + for name, src := range sources { + data, err := os.ReadFile(filepath.Join(root, name)) + require.NoError(t, err) + require.Equal(t, src, string(data), "%s changed although the cascade was refused", name) + } + }) +} + +// The relation-reading mutators answer at once and label the answer while an +// edit in their scope never publishes; they never wait it out or refuse. +func TestRelationToolsLabelAndNeverRefuseWithUnresolvedReceipt(t *testing.T) { + s, root := newRelationFixture(t) + pendingFreshnessReceipt(s, "receipt-namer", "", + writeUnindexed(t, root, "namer.go", "package p\n\nfunc Namer() {\n\tTarget()\n}\n"), 1) + id := targetID(t, s) + rename := `{"changes":{"target.go":[{"range":{"start":{"line":2,"character":5},"end":{"line":2,"character":11}},"newText":"Renamed"}]}}` + comment := `{"changes":{"` + nativeFileURI(filepath.Join(root, "namer.go")) + + `":[{"range":{"start":{"line":0,"character":0},"end":{"line":0,"character":0}},"newText":"// Namer names Target.\n"}]}}` + + for tool, call := range map[string]func() *mcpgo.CallToolResult{ + "change_contract": func() *mcpgo.CallToolResult { + return callChangeTool(t, s.handleChangeContract, "change_contract", + map[string]any{"source": "symbols", "symbols": id, "format": "json"}) + }, + "preview_edit": func() *mcpgo.CallToolResult { + return callChangeTool(t, s.handlePreviewEdit, "preview_edit", + map[string]any{"workspace_edit": rename, "diagnostics": false}) + }, + // No symbol is removed or renamed: only the touched file, named by + // an absolute file URI, brings the pending edit into scope. + "preview_edit file uri": func() *mcpgo.CallToolResult { + return callChangeTool(t, s.handlePreviewEdit, "preview_edit", + map[string]any{"workspace_edit": comment, "diagnostics": false}) + }, + } { + started := time.Now() + payload := lifecycleResultPayload(t, call()) + require.Less(t, time.Since(started), 2*time.Second, "%s waited on a receipt that never publishes", tool) + require.Equal(t, "pending", payload["graph_freshness"], "%s: %v", tool, payload) + require.Equal(t, []any{"namer.go"}, payload["pending_files"], tool) + require.Equal(t, false, payload["complete"], tool) + } +} diff --git a/internal/mcp/tools_analysis.go b/internal/mcp/tools_analysis.go index feae760a1..bec7687a4 100644 --- a/internal/mcp/tools_analysis.go +++ b/internal/mcp/tools_analysis.go @@ -352,12 +352,43 @@ func (s *Server) handleDetectChanges(ctx context.Context, req mcp.CallToolReques if scopeErr := s.repoPrefixInSessionScope(ctx, repoPrefix, repoPrefix); scopeErr != nil { return mcp.NewToolResultError(scopeErr.Error()), nil } + checkoutScoped := control != nil && control.CheckoutScoped var pending bool var freshnessErr error - if control != nil && control.CheckoutScoped { + var label relationFreshness + var diff *analysis.DiffResult + var freshness requestFreshness + var keys []string + var deadline time.Time + if checkoutScoped { pending, freshnessErr = s.mutationFreshnessSummaryForRepos(ctx, repoPrefix) - } else if err := s.awaitMutationFreshnessForRepos(ctx, repoPrefix); err != nil { - return mcp.NewToolResultError("change detection refused a stale graph: " + err.Error()), nil + } else { + // Diff the files first: they are the relation scope the wait below + // is bounded to, so an edit elsewhere in the repository neither + // delays nor labels this answer. + files, err := analysis.MapGitDiffContext(ctx, nil, repoRoot, repoPrefix, scope, baseRef) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + keys = make([]string, len(files.ChangedFiles)) + for i, file := range files.ChangedFiles { + keys[i] = analysis.GraphKey(repoPrefix, file, analysis.RepoRelativePath) + } + freshness = takeRequestFreshness(&req) + deadline = relationWaitDeadline(ctx, freshness) + label = s.awaitRelationFreshness(ctx, nil, keys, deadline) + if freshness.requireFresh && label.state != "" { + return label.requireFreshError(), nil + } + diff = files + } + respond := func(result map[string]any) (*mcp.CallToolResult, error) { + if !checkoutScoped { + result["barrier_wait_ms"] = label.waited.Milliseconds() + label.apply(result) + } + res, err := s.respondJSONOrTOON(ctx, req, result) + return label.decorate(res), err } reader := s.readerFor(ctx) graphStatus, graphDetail := "ready", "" @@ -381,7 +412,12 @@ func (s *Server) handleDetectChanges(ctx context.Context, req mcp.CallToolReques reader = nil } - diff, err := analysis.MapGitDiffContext(ctx, reader, repoRoot, repoPrefix, scope, baseRef) + var err error + if diff == nil { + diff, err = analysis.MapGitDiffContext(ctx, reader, repoRoot, repoPrefix, scope, baseRef) + } else { + diff, err = analysis.JoinDiffSymbolsContext(ctx, reader, repoPrefix, diff) + } if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -398,7 +434,7 @@ func (s *Server) handleDetectChanges(ctx context.Context, req mcp.CallToolReques fileChanges = []analysis.FileChange{} } if graphStatus != "ready" { - return s.respondJSONOrTOON(ctx, req, map[string]any{ + return respond(map[string]any{ "changed_symbols": []any{}, "changed_files": changedFiles, "file_changes": fileChanges, "risk": "UNKNOWN", "complete": false, "graph_status": graphStatus, "summary": "Git file changes are available; symbol mapping and impact await an exact published graph", @@ -407,7 +443,7 @@ func (s *Server) handleDetectChanges(ctx context.Context, req mcp.CallToolReques } if len(diff.ChangedSymbols) == 0 { - return s.respondJSONOrTOON(ctx, req, map[string]any{ + return respond(map[string]any{ "changed_symbols": []any{}, "changed_files": changedFiles, "file_changes": fileChanges, @@ -432,6 +468,15 @@ func (s *Server) handleDetectChanges(ctx context.Context, req mcp.CallToolReques for i, cs := range diff.ChangedSymbols { symbolIDs[i] = cs.ID } + if !checkoutScoped { + // The walk reads the changed symbols' callers, and an edit outside + // the diff (a staged or compare scope leaves the working tree out) + // can be changing those. They share what is left of the budget. + label = label.merge(s.awaitRelationFreshness(ctx, symbolIDs, keys, deadline)) + if freshness.requireFresh && label.state != "" { + return label.requireFreshError(), nil + } + } impact, err := s.analyzeDiffImpact(ctx, symbolIDs) if err != nil { @@ -459,7 +504,7 @@ func (s *Server) handleDetectChanges(ctx context.Context, req mcp.CallToolReques req.GetBool("summary_only", false), req.GetInt("offset", 0), req.GetInt("limit", 100)) - return s.respondJSONOrTOON(ctx, req, detectResult) + return respond(detectResult) } // tryImpactAnalysisSnapshots reads optional cached enrichments without waiting @@ -487,8 +532,12 @@ func (s *Server) handleEnhancedChangeImpact(ctx context.Context, req mcp.CallToo ids[i] = strings.TrimSpace(ids[i]) } - if freshnessErr := s.awaitMutationFreshnessForRepos(ctx, s.mutationReposForSymbolIDs(ctx, ids)...); freshnessErr != nil { - return mcp.NewToolResultError("change impact refused a stale graph: " + freshnessErr.Error()), nil + // Wait briefly for the edits these symbols' relations depend on, then + // answer; whatever is still lagging is labelled, not refused. + freshness := takeRequestFreshness(&req) + label := s.awaitRelationFreshness(ctx, ids, nil, relationWaitDeadline(ctx, freshness)) + if freshness.requireFresh && label.state != "" { + return label.requireFreshError(), nil } // Keep the mandatory pre-edit safety gate well below host transport @@ -511,7 +560,9 @@ func (s *Server) handleEnhancedChangeImpact(ctx context.Context, req mcp.CallToo "test_files": impact.TestFiles, "total_affected": impact.TotalAffected, "cross_repo_impact": impact.CrossRepoImpact, + "barrier_wait_ms": label.waited.Milliseconds(), } + label.apply(result) // GNX-3: by_depth_counts is the headline; the heavy by_depth rows are // paged (offset / limit) or dropped (summary_only) so the agent gets the @@ -605,18 +656,20 @@ func (s *Server) handleEnhancedChangeImpact(ctx context.Context, req mcp.CallToo } } - if s.isGCX(ctx, req) { + var res *mcp.CallToolResult + switch { + case s.isGCX(ctx, req): // encodeChangeImpact reads the same map shape we'd return as // JSON; routing through it keeps a single source of truth for // field names and avoids divergence on the next analyzer // addition. - return s.gcxResponseWithBudget(req)(encodeChangeImpact(result)) - } - if s.isTOON(ctx, req) { - return returnTOON(result) + res, err = s.gcxResponseWithBudget(req)(encodeChangeImpact(result)) + case s.isTOON(ctx, req): + res, err = returnTOON(result) + default: + res, err = s.respondJSONOrTOON(ctx, req, result) } - - return s.respondJSONOrTOON(ctx, req, result) + return label.decorate(res), err } func impactComplete(impact *analysis.ImpactResult) bool { diff --git a/internal/mcp/tools_safe_delete.go b/internal/mcp/tools_safe_delete.go index 17b194870..08ac47ad4 100644 --- a/internal/mcp/tools_safe_delete.go +++ b/internal/mcp/tools_safe_delete.go @@ -52,10 +52,11 @@ func (s *Server) registerSafeDeleteSymbolTool() { // information for the caller to navigate to it and remove it before // retrying the delete. type safeDeleteReference struct { - FromID string `json:"from_id"` + FromID string `json:"from_id,omitempty"` Kind string `json:"kind"` FromName string `json:"from_name,omitempty"` FilePath string `json:"file_path,omitempty"` + Line int `json:"line,omitempty"` } // cascadeClosureEntry describes a symbol that the cascade pass added @@ -109,6 +110,20 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq // EdgeDefines / EdgeMemberOf are skipped (they don't represent // "someone calls this"). refs := collectReferencingEdges(g, id) + // A call that an edit added and the graph has not read yet blocks the + // delete like a graph reference does: a provisional reference here, a + // manual patch in propagate mode. The same snapshot labels the answer. + gaps := s.laggingMutationReceipts(ctx) + provisional := s.provisionalSites(g, gaps, node) + for _, site := range provisional { + refs = append(refs, safeDeleteReference{Kind: "provisional", FilePath: site.File, Line: site.Line}) + } + respond := func(result map[string]any) (*mcp.CallToolResult, error) { + label := s.labelRelations(ctx, gaps, []string{id}, nil) + label.apply(result) + res, err := s.respondJSONOrTOON(ctx, req, result) + return label.decorate(res), err + } // SRN-7: propagate-delete — patch the surviving call sites instead of // refusing. Builds a per-caller plan, applies the removable standalone @@ -116,9 +131,18 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq var appliedPatches, failedPatches []callerPatch if req.GetBool("propagate", false) && len(refs) > 0 { plan := s.buildPropagationPlan(ctx, g, node) + for _, site := range provisional { + reason := "provisional: named in a file whose edit the graph has not read yet — patch by hand" + if site.Line == 0 { + reason = "provisional: the graph has not read the latest edit to the symbol's own file — retry once it publishes" + } + plan = append(plan, callerPatch{ + File: site.File, Line: site.Line, CurrentText: site.Text, Action: "manual", Reason: reason, + }) + } manual := countManual(plan) if dryRun { - return s.respondJSONOrTOON(ctx, req, map[string]any{ + return respond(map[string]any{ "status": "propagation_plan", "symbol": id, "file": node.FilePath, @@ -131,7 +155,7 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq }) } if manual > 0 && !force { - return s.respondJSONOrTOON(ctx, req, map[string]any{ + return respond(map[string]any{ "status": "propagation_blocked_manual", "symbol": id, "caller_patches": plan, @@ -145,7 +169,7 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq } appliedPatches, failedPatches = applied, failed if len(failed) > 0 && !force { - return s.respondJSONOrTOON(ctx, req, map[string]any{ + return respond(map[string]any{ "status": "propagation_partial", "symbol": id, "patches_applied": applied, @@ -158,7 +182,7 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq force = true node = g.GetNode(id) if node == nil { - return s.respondJSONOrTOON(ctx, req, map[string]any{ + return respond(map[string]any{ "status": "deleted_by_propagation", "symbol": id, "patches_applied": appliedPatches, @@ -171,7 +195,7 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq } if len(refs) > 0 && !force { - return s.respondJSONOrTOON(ctx, req, map[string]any{ + return respond(map[string]any{ "status": "rejected_has_references", "symbol": id, "file": node.FilePath, @@ -195,6 +219,40 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq if cascadeMode != cascadeModeOff { closure, cascadeTruncated = computeCascadeClosure(g, node, cascadeIntoTests) } + if cascadeMode == cascadeModeApply { + // The closure is orphaned by the graph's edges only. A call that an + // unpublished edit added keeps a member alive, and force does not + // cover the members, so the cascade waits for that edit. + var blocked []safeDeleteReference + var members []string + for _, entry := range closure { + member := g.GetNode(entry.ID) + if member == nil { + continue + } + sites := s.provisionalSites(g, gaps, member) + if len(sites) > 0 { + members = append(members, entry.ID) + } + for _, site := range sites { + blocked = append(blocked, safeDeleteReference{Kind: "provisional", FilePath: site.File, Line: site.Line}) + } + } + if len(blocked) > 0 { + return respond(map[string]any{ + "status": "rejected_has_references", + "symbol": id, + "file": node.FilePath, + "references": blocked, + "reference_count": len(blocked), + "dry_run": dryRun, + "force": force, + "cascade_mode": cascadeMode, + "cascade_closure": closure, + "hint": "cascade member(s) " + strings.Join(members, ", ") + " are named in files whose edits the graph has not read yet; retry once those publish, or delete without cascade", + }) + } + } absPath, err := s.resolveNodePath(ctx, node) if err != nil { @@ -242,7 +300,7 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq if dryRun { result["status"] = "preview" - return s.respondJSONOrTOON(ctx, req, result) + return respond(result) } // Commit phase. cascadeModeApply expands the work to every entry @@ -282,7 +340,7 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq result["status"] = "partial_failure" result["cascade_deleted"] = deletedIDs result["error"] = err.Error() - return s.respondJSONOrTOON(ctx, req, result) + return respond(result) } // Persist session state and re-index every touched file. @@ -310,7 +368,7 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq if len(failedPatches) > 0 { result["patches_failed"] = failedPatches } - return s.respondJSONOrTOON(ctx, req, result) + return respond(result) } // normaliseCascadeMode coerces the cascade parameter to a known diff --git a/internal/mcp/tools_simulate.go b/internal/mcp/tools_simulate.go index fdfadea7c..4156dfb43 100644 --- a/internal/mcp/tools_simulate.go +++ b/internal/mcp/tools_simulate.go @@ -107,6 +107,9 @@ func (s *Server) handlePreviewEdit(ctx context.Context, req mcp.CallToolRequest) wantDiag := req.GetBool("diagnostics", true) diagTimeout := time.Duration(req.GetInt("diagnostics_timeout_ms", 1500)) * time.Millisecond + // Snapshot the unpublished edits before the simulation reads relations; + // the symbols that scope the label are known only once it has run. + gaps := s.laggingMutationReceipts(ctx) sim, simErr := s.buildSimulation(ctx, []lsp.WorkspaceEdit{edit}, inherit) if simErr != nil { if ctxErr := requestContextError(ctx, simErr); ctxErr != nil { @@ -138,6 +141,20 @@ func (s *Server) handlePreviewEdit(ctx context.Context, req mcp.CallToolRequest) result["diagnostics_summary"] = summariseDiagnostics(diags) } + // broken_callers walks the callers of the removed and renamed symbols. + seeds := append([]string{}, step.symbolsRemoved...) + for _, renamed := range step.symbolsRenamed { + seeds = append(seeds, renamed["from"]) + } + // touched_files keeps each path as the client spelled it; the scope + // takes graph keys. + var touched []string + for _, path := range step.touchedFiles { + if abs, err := s.resolveOverlayAbsPath(path); err == nil && abs != "" { + touched = append(touched, s.repoRelative(abs)) + } + } + s.labelRelations(ctx, gaps, seeds, touched).apply(result) return mcp.NewToolResultText(jsonOK(result)), nil }