mcp: label relation-reading mutators and block deletes on pending callers

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

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

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:

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).

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)