diff --git a/internal/mcp/tools_analyze_temporal.go b/internal/mcp/tools_analyze_temporal.go new file mode 100644 index 000000000..2a63ce513 --- /dev/null +++ b/internal/mcp/tools_analyze_temporal.go @@ -0,0 +1,32 @@ +package mcp + +import ( + "context" + + "github.com/mark3labs/mcp-go/mcp" + + "github.com/zzet/gortex/internal/resolver" +) + +// handleAnalyzeTemporalOrphans is the queryable face of the Temporal +// call-graph integrity check: broken dispatches (a workflow calls an +// activity/child-workflow that resolves to nothing), signals/queries with +// no handler, and registered activities/workflows nobody dispatches or +// starts. Exposed as `analyze kind=temporal_orphans`. +func (s *Server) handleAnalyzeTemporalOrphans(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { + rep := resolver.DetectTemporalOrphans(s.graph) + return s.respondJSONOrTOON(ctx, req, map[string]any{ + "broken_dispatch": rep.BrokenDispatch, + "signal_no_handler": rep.SignalNoHandler, + "query_no_handler": rep.QueryNoHandler, + "orphan_activity": rep.OrphanActivity, + "orphan_workflow": rep.OrphanWorkflow, + "totals": map[string]int{ + "broken_dispatch": len(rep.BrokenDispatch), + "signal_no_handler": len(rep.SignalNoHandler), + "query_no_handler": len(rep.QueryNoHandler), + "orphan_activity": len(rep.OrphanActivity), + "orphan_workflow": len(rep.OrphanWorkflow), + }, + }) +} diff --git a/internal/mcp/tools_analyze_temporal_test.go b/internal/mcp/tools_analyze_temporal_test.go new file mode 100644 index 000000000..e556019ef --- /dev/null +++ b/internal/mcp/tools_analyze_temporal_test.go @@ -0,0 +1,49 @@ +package mcp + +import ( + "context" + "encoding/json" + "testing" + + mcplib "github.com/mark3labs/mcp-go/mcp" + + "github.com/zzet/gortex/internal/graph" +) + +func TestAnalyzeTemporalOrphans(t *testing.T) { + srv, _ := setupTestServer(t) + // Broken dispatch: a stub that never resolved. + srv.graph.AddEdge(&graph.Edge{ + From: "wf.go::OrderWorkflow", To: "unresolved::temporal::activity::MissingActivity", + Kind: graph.EdgeCalls, FilePath: "wf.go", Line: 5, + Meta: map[string]any{"via": "temporal.stub", "temporal_kind": "activity", "temporal_name": "MissingActivity"}, + }) + // Signal sent with no handler anywhere. + srv.graph.AddEdge(&graph.Edge{ + From: "wf.go::OrderWorkflow", To: "unresolved::extern::workflow::SignalExternalWorkflow", + Kind: graph.EdgeCalls, FilePath: "wf.go", Line: 8, + Meta: map[string]any{"via": "temporal.signal-send", "temporal_kind": "signal", "temporal_name": "ghost-signal"}, + }) + + req := mcplib.CallToolRequest{} + req.Params.Name = "analyze" + req.Params.Arguments = map[string]any{"kind": "temporal_orphans"} + res, err := srv.handleAnalyze(context.Background(), req) + if err != nil { + t.Fatalf("handleAnalyze: %v", err) + } + if res.IsError { + t.Fatalf("error: %+v", res.Content) + } + var out map[string]any + if err := json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &out); err != nil { + t.Fatalf("json: %v", err) + } + totals, _ := out["totals"].(map[string]any) + if totals["broken_dispatch"].(float64) < 1 { + t.Errorf("expected a broken dispatch, got %v", totals["broken_dispatch"]) + } + if totals["signal_no_handler"].(float64) < 1 { + t.Errorf("expected a signal with no handler, got %v", totals["signal_no_handler"]) + } +} diff --git a/internal/mcp/tools_enhancements.go b/internal/mcp/tools_enhancements.go index f73a17d3c..9fc9d6096 100644 --- a/internal/mcp/tools_enhancements.go +++ b/internal/mcp/tools_enhancements.go @@ -828,6 +828,8 @@ func (s *Server) handleAnalyze(ctx context.Context, req mcp.CallToolRequest) (*m return s.handleAnalyzeExternalCalls(ctx, req) case "synthesizers": return s.handleAnalyzeSynthesizers(ctx, req) + case "temporal_orphans": + return s.handleAnalyzeTemporalOrphans(ctx, req) case "resolution_outcomes": return s.handleAnalyzeResolutionOutcomes(ctx, req) case "retrieval_log": diff --git a/internal/resolver/temporal_orphans.go b/internal/resolver/temporal_orphans.go new file mode 100644 index 000000000..a90da687c --- /dev/null +++ b/internal/resolver/temporal_orphans.go @@ -0,0 +1,136 @@ +package resolver + +import ( + "strings" + + "github.com/zzet/gortex/internal/graph" +) + +// TemporalOrphan names one side of a Temporal contract that has no +// counterpart in the graph. +type TemporalOrphan struct { + From string `json:"from"` // the dispatching / sending node + Kind string `json:"kind"` // activity / workflow / signal / query + Name string `json:"name"` // the dispatched / signalled / queried name + File string `json:"file,omitempty"` // call-site file, when known + Line int `json:"line,omitempty"` +} + +// TemporalOrphanReport is the result of DetectTemporalOrphans. Each list +// is a different integrity gap in the Temporal call graph. +type TemporalOrphanReport struct { + // BrokenDispatch: a workflow dispatches an activity / child workflow + // whose name resolves to nothing (the temporal.stub edge is still a + // placeholder). Almost always an error — a broken or renamed call. + BrokenDispatch []TemporalOrphan `json:"broken_dispatch"` + // SignalNoHandler / QueryNoHandler: a signal is sent / a query is + // called with a name no workflow handles. An error (sending into the + // void). + SignalNoHandler []TemporalOrphan `json:"signal_no_handler"` + QueryNoHandler []TemporalOrphan `json:"query_no_handler"` + // OrphanActivity / OrphanWorkflow: a registered activity / workflow + // nobody dispatches or starts. A warning — dead code or unfinished. + OrphanActivity []string `json:"orphan_activity"` + OrphanWorkflow []string `json:"orphan_workflow"` +} + +// DetectTemporalOrphans walks the resolved Temporal graph and reports the +// four integrity gaps above. Read-only. +func DetectTemporalOrphans(g graph.Store) TemporalOrphanReport { + var rep TemporalOrphanReport + if g == nil { + return rep + } + + // Signal/query handler name sets (providers). + signalHandled := map[string]bool{} + queryHandled := map[string]bool{} + // Activities/workflows that have at least one resolved inbound + // dispatch (consumed). + consumed := map[string]bool{} + + for e := range g.EdgesByKind(graph.EdgeCalls) { + if e == nil || e.Meta == nil { + continue + } + via, _ := e.Meta["via"].(string) + kind, _ := e.Meta["temporal_kind"].(string) + name, _ := e.Meta["temporal_name"].(string) + switch via { + case "temporal.handler": + switch kind { + case "signal": + signalHandled[name] = true + case "query": + queryHandled[name] = true + } + case "temporal.stub": + if strings.HasPrefix(e.To, temporalStubPrefix) { + // P0: a dispatch whose call site is a test file is almost + // always a fixture/mock (handler is a stub or lives in + // another repo); counting it as a broken_dispatch is the + // dominant false positive. Skip it — keyed on the edge's + // own FilePath (the dispatcher), so it's robust under both + // full and incremental reindex (no Node.Meta dependency). + if isTestFilePath(e.FilePath) { + continue + } + rep.BrokenDispatch = append(rep.BrokenDispatch, TemporalOrphan{ + From: e.From, Kind: kind, Name: name, File: e.FilePath, Line: e.Line, + }) + } else if e.To != "" { + consumed[e.To] = true + } + } + } + + // Second pass for senders/callers now that the handler sets are known. + for e := range g.EdgesByKind(graph.EdgeCalls) { + if e == nil || e.Meta == nil { + continue + } + via, _ := e.Meta["via"].(string) + name, _ := e.Meta["temporal_name"].(string) + switch via { + case "temporal.signal-send": + if name != "" && !signalHandled[name] { + rep.SignalNoHandler = append(rep.SignalNoHandler, TemporalOrphan{ + From: e.From, Kind: "signal", Name: name, File: e.FilePath, Line: e.Line, + }) + } + case "temporal.query-call": + if name != "" && !queryHandled[name] { + rep.QueryNoHandler = append(rep.QueryNoHandler, TemporalOrphan{ + From: e.From, Kind: "query", Name: name, File: e.FilePath, Line: e.Line, + }) + } + } + } + + // Registered-but-unconsumed activities / workflows. Only Go nodes + // carry temporal_role from a worker.Register* call; an activity with + // no resolved inbound dispatch is dead. + checkOrphanRole := func(n *graph.Node) { + if n == nil || n.Language != "go" { + return + } + role, _ := n.Meta["temporal_role"].(string) + switch role { + case "activity": + if !consumed[n.ID] { + rep.OrphanActivity = append(rep.OrphanActivity, n.ID) + } + case "workflow": + if !consumed[n.ID] { + rep.OrphanWorkflow = append(rep.OrphanWorkflow, n.ID) + } + } + } + for n := range g.NodesByKind(graph.KindFunction) { + checkOrphanRole(n) + } + for n := range g.NodesByKind(graph.KindMethod) { + checkOrphanRole(n) + } + return rep +} diff --git a/internal/resolver/temporal_orphans_test.go b/internal/resolver/temporal_orphans_test.go new file mode 100644 index 000000000..cc4d483d2 --- /dev/null +++ b/internal/resolver/temporal_orphans_test.go @@ -0,0 +1,64 @@ +package resolver + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/graph" +) + +func TestDetectTemporalOrphans(t *testing.T) { + b := newTemporalTestGraph() + // Workflow dispatches ChargeCard (resolves) and MissingActivity (broken). + b.addGoFunc("svc/wf.go::OrderWorkflow", "OrderWorkflow", "svc/wf.go", "svc") + b.addStubCall("svc/wf.go::OrderWorkflow", "activity", "ChargeCard", "svc/wf.go") + b.addStubCall("svc/wf.go::OrderWorkflow", "activity", "MissingActivity", "svc/wf.go") + // Registrations. + b.addGoFunc("svc/act.go::ChargeCard", "ChargeCard", "svc/act.go", "svc") + b.addGoFunc("svc/act.go::UnusedActivity", "UnusedActivity", "svc/act.go", "svc") + b.addGoFunc("svc/main.go::setup", "setup", "svc/main.go", "svc") + // Register edges must differ by line — the real extractor emits each + // w.Register*() at its own line (edgeKey includes File+Line); the + // addGoRegister helper pins line 5, which would dedup multiple + // same-kind registrations from one function. + reg := func(kind, name string, line int) { + b.g.AddEdge(&graph.Edge{ + From: "svc/main.go::setup", + To: "unresolved::extern::go.temporal.io/sdk/worker::Register" + capitalise(kind), + Kind: graph.EdgeCalls, FilePath: "svc/main.go", Line: line, + Meta: map[string]any{"via": "temporal.register", "temporal_kind": kind, "temporal_name": name}, + }) + } + reg("activity", "ChargeCard", 3) + reg("activity", "UnusedActivity", 4) + reg("workflow", "OrderWorkflow", 5) + // A signal sent with no handler. + b.g.AddEdge(&graph.Edge{ + From: "svc/wf.go::OrderWorkflow", To: "unresolved::extern::workflow::SignalExternalWorkflow", + Kind: graph.EdgeCalls, FilePath: "svc/wf.go", Line: 9, + Meta: map[string]any{"via": "temporal.signal-send", "temporal_kind": "signal", "temporal_name": "ghost-signal"}, + }) + + ResolveTemporalCalls(b.g) + rep := DetectTemporalOrphans(b.g) + + names := func(os []TemporalOrphan) map[string]bool { + m := map[string]bool{} + for _, o := range os { + m[o.Name] = true + } + return m + } + require.True(t, names(rep.BrokenDispatch)["MissingActivity"], "MissingActivity must be a broken dispatch") + assert.False(t, names(rep.BrokenDispatch)["ChargeCard"], "ChargeCard resolves, not broken") + assert.True(t, names(rep.SignalNoHandler)["ghost-signal"], "ghost-signal has no handler") + + orphanAct := map[string]bool{} + for _, id := range rep.OrphanActivity { + orphanAct[id] = true + } + assert.True(t, orphanAct["svc/act.go::UnusedActivity"], "UnusedActivity is registered but never dispatched") + assert.False(t, orphanAct["svc/act.go::ChargeCard"], "ChargeCard is dispatched, not orphan") +} diff --git a/internal/resolver/temporal_orphans_testfilter_test.go b/internal/resolver/temporal_orphans_testfilter_test.go new file mode 100644 index 000000000..89dd50337 --- /dev/null +++ b/internal/resolver/temporal_orphans_testfilter_test.go @@ -0,0 +1,57 @@ +package resolver + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// PURPOSE: P0 — a Temporal dispatch whose call site is a TEST file +// (`*_test.go`, `*Test.java`, files under `tests/`, …) must not be +// counted as a broken_dispatch. Test fixtures routinely contain +// `ExecuteActivity` calls whose handlers are mocks or live in another +// repo; counting them as integrity gaps is the dominant false-positive +// source the fork's measurement complained about. +// +// RATIONALE: DetectTemporalOrphans counts an unresolved temporal.stub +// edge off the edge's OWN FilePath (the dispatcher's file). Filtering +// test-file dispatchers there is robust (a pure function of the path — +// no Node.Meta dependency, survives incremental reindex) and only ever +// touches the unresolved branch, so resolved test→activity edges keep +// marking their handler consumed. +// +// KEYWORDS: temporal, broken_dispatch, test-file, false-positive, P0 + +func TestDetectTemporalOrphans_TestFileCallerExcluded(t *testing.T) { + b := newTemporalTestGraph() + b.addStubCall("wf_test", "activity", "ChargeActivity", "pkg/workflow_test.go") + rep := DetectTemporalOrphans(b.g) + assert.Empty(t, rep.BrokenDispatch, + "dispatch from a _test.go file must not count as broken_dispatch") +} + +func TestDetectTemporalOrphans_ProdCallerStillCounted(t *testing.T) { + b := newTemporalTestGraph() + b.addStubCall("wf", "activity", "ChargeActivity", "pkg/workflow.go") + rep := DetectTemporalOrphans(b.g) + require.Len(t, rep.BrokenDispatch, 1, + "unresolved dispatch from production code stays a broken_dispatch") + assert.Equal(t, "ChargeActivity", rep.BrokenDispatch[0].Name) +} + +func TestDetectTemporalOrphans_JavaTestFileCallerExcluded(t *testing.T) { + b := newTemporalTestGraph() + b.addStubCall("svc", "workflow", "OrderWorkflow", "src/main/java/OrderManagerTest.java") + rep := DetectTemporalOrphans(b.g) + assert.Empty(t, rep.BrokenDispatch, + "dispatch from a *Test.java file must not count as broken_dispatch") +} + +func TestDetectTemporalOrphans_TestDirCallerExcluded(t *testing.T) { + b := newTemporalTestGraph() + b.addStubCall("h", "activity", "HelperActivity", "pkg/tests/helper.go") + rep := DetectTemporalOrphans(b.g) + assert.Empty(t, rep.BrokenDispatch, + "dispatch from a file under a tests/ directory must not count") +} diff --git a/internal/resolver/testpath.go b/internal/resolver/testpath.go new file mode 100644 index 000000000..720211209 --- /dev/null +++ b/internal/resolver/testpath.go @@ -0,0 +1,66 @@ +package resolver + +import ( + "path/filepath" + "strings" +) + +// isTestFilePath reports whether a source path follows a recognised test +// convention. +// +// PURPOSE: let the Temporal orphan detector drop dispatches that +// originate in test fixtures (the dominant broken_dispatch false +// positive) without depending on Node.Meta test flags — which are not +// re-stamped on the incremental-reindex path. +// +// RATIONALE: a verbatim port of internal/indexer.IsTestFile. The +// resolver cannot import the indexer package (indexer → resolver is the +// established import direction; the reverse would be a cycle), so the +// predicate is duplicated here. It is intentionally identical in +// behaviour; the canonical home is internal/indexer/testpattern.go and a +// future refactor should consolidate both (plus the analysis/* copies) +// into a leaf internal/pathutil package. +// +// KEYWORDS: test-file, predicate, temporal, broken_dispatch, no-cycle +func isTestFilePath(path string) bool { + if path == "" { + return false + } + // Directory-based hints first — covers projects that don't follow + // the per-file naming convention. + dir := filepath.ToSlash(path) + for _, marker := range []string{"/__tests__/", "/tests/", "/test/", "/spec/"} { + if strings.Contains(dir, marker) { + return true + } + } + if strings.HasPrefix(dir, "tests/") || strings.HasPrefix(dir, "test/") || strings.HasPrefix(dir, "spec/") { + return true + } + + base := filepath.Base(path) + ext := strings.ToLower(filepath.Ext(base)) + stem := strings.TrimSuffix(base, ext) + + switch ext { + case ".go": + return strings.HasSuffix(stem, "_test") + case ".ts", ".tsx", ".js", ".jsx", ".mts", ".cts", ".mjs", ".cjs": + return strings.HasSuffix(stem, ".test") || strings.HasSuffix(stem, ".spec") + case ".py": + return strings.HasPrefix(stem, "test_") || strings.HasSuffix(stem, "_test") + case ".dart": + return strings.HasSuffix(stem, "_test") + case ".rb": + return strings.HasSuffix(stem, "_spec") || strings.HasSuffix(stem, "_test") + case ".java", ".kt": + return strings.HasSuffix(stem, "Test") || strings.HasSuffix(stem, "Tests") + case ".cs": + return strings.HasSuffix(stem, "Tests") || strings.HasSuffix(stem, "Test") + case ".swift": + return strings.HasSuffix(stem, "Tests") + case ".php": + return strings.HasSuffix(stem, "Test") || strings.HasSuffix(stem, "test") + } + return false +}