diff --git a/CLAUDE.md b/CLAUDE.md index c34fb1ef5..ffece5c13 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,6 +61,7 @@ Selected via `llm.provider` in `.gortex.yaml` or `~/.config/gortex/config.yaml`. - **`compress_bodies: true`** on `read_file` / `get_symbol_source` / `get_editing_context` elides function bodies to stubs while keeping signatures + doc-comments + structure. ~30–40% of original tokens. 14 languages. - **Overlay sessions** (`overlay_push`, `overlay_list`, `overlay_drop`, `compare_with_overlay`) let editor extensions push unsaved buffers as a per-session shadow graph — every subsequent tool call reads through it without mutating base. Bound to the MCP session lifecycle; idle TTL via `GORTEX_OVERLAY_IDLE_TTL` (default 30m). - **Speculative execution** (`preview_edit`, `simulate_chain`) takes an LSP `WorkspaceEdit` and returns the graph diff + broken callers/implementors + impact rollup + suggested tests + (optional) LSP diagnostics — disk untouched. `simulate_chain` with `keep: true` promotes the final state into a real overlay. +- **Change-contract pipeline** (`change_contract`, `symbols_for_ranges`) — one envelope every change source lowers into. `change_contract` takes a WorkspaceEdit, a git diff range (`source:diff base:…`), an explicit symbol set, or file line-ranges, runs LOWER → PREDICT → EVALUATE (guards + architecture + event-boundary rule families) → SCORE → CLASSIFY → EMIT, and returns one verdict `{allow|warn|refuse}` with reasons, risk, a `verification_command`, a checkable `stop_condition`, and an `edit_strategy`. `lens:api` focuses it on public-surface / API drift; `risk_gate:true` requires a TTL'd impact-review ack (`ack:true`, stored as a development memory) for load-bearing symbols. `symbols_for_ranges` is the standalone lowering primitive. The pre-write **parse gate** on `edit_file` / `write_file` refuses an edit that would introduce new tree-sitter parse errors (override with `allow_parse_errors`); `safe_delete_symbol propagate:true` patches surviving call sites; `analyze kind=suggest_boundaries` seeds an `architecture:` block from detected communities. - **MCP 2026 Streamable HTTP** at `POST /mcp` — `gortex server` always mounts it; `gortex daemon --http-addr ` opts the daemon in (non-localhost binds require `--http-auth-token`). - **Session memory** (`save_note`, `query_notes`, `distill_session`) persists agent-authored notes per repo, auto-linked to symbols mentioned in the body. Notes survive daemon restarts and context compactions, scoped to the session's workspace. - **Development memories** (`store_memory`, `query_memories`, `surface_memories`) — cross-session, symbol-linked durable knowledge that compounds the longer a team uses Gortex. Memories carry `kind` (invariant / constraint / convention / gotcha / decision / incident / reference), `importance` (1..5), `confidence` (0..1), and are surfaced *proactively* by `surface_memories` when their anchor symbols / files enter the agent's working set. @@ -71,7 +72,7 @@ Selected via `llm.provider` in `.gortex.yaml` or `~/.config/gortex/config.yaml`. - **Capability edges** — `reads_env` / `executes_process` / `accesses_field` are first-class traversable edges (in the `walk_graph`/`nav`/`graph_query` surface) synthesised post-resolution, so a supply-chain / least-privilege audit can ask "what reads $AWS_SECRET", "what shells out", "what writes this field" in one hop. - **PR review, end-to-end** — `gortex prs` triages open pull requests from the graph (`gortex prs ` for one PR; `--triage` / `--conflicts` / `--worktrees` / `--base` / `--format`; `gortex prs bundle`), and `gortex review [|--diff] [--audience agent|human] [--post]` reviews a diff. The MCP surface mirrors it: `pr_risk` / `list_prs` / `get_pr_impact` / `triage_prs` / `conflicts_prs` / `suggest_reviewers` score and rank PRs against the graph, while `review` / `review_pack` / `post_review` / `pr_review_context` / `suggested_review_questions` / `critique_review` / `suppress_finding` drive the review itself. - **Multimodal + broad ingest** — image files (`KindImage` assets with format/dimensions/sha256) and PDF documents (per-page searchable `KindDoc` nodes) are graph nodes; new first-class extractors cover Terraform/HCL cross-block references, Helm charts/templates, Ansible playbooks, .NET `.sln`/`.csproj`, MCP server configs, Quarto `.qmd`, Luau, COBOL paragraphs + JCL, and C/C++ `#define` macros. Grammar-less languages can register a regex fallback chunker (`index.fallback_chunkers`) or an external extractor plugin (`index.extractor_plugins`) from config — no fork. `gortex db schema --postgres ` ingests a live database's schema. -- **`analyze` is a 60-kind dispatcher** — beyond the structural kinds, it now covers `impact` (composite change-risk score), `bottlenecks` (interprocedural computation-bottleneck risk — cognitive complexity, loop depth, transitive/hidden-O(n^k) loop nesting across calls, unguarded recursion), `health_score` (per-symbol A–F grade), `sast` / `named` / `unsafe_patterns` (security), `clusters`, `connectivity_health`, `tests_as_edges`, `synthesizers` (framework-dispatch-synthesized edges, grouped by pass + provenance), `resolution_outcomes` (structured why-unresolved taxonomy), `review` (an idiomatic/correctness rulepack — NPE, thread-safety check-then-act, N+1, logic errors across Go + Python — with a graph-grounded false-positive-reduction pass), and more. +- **`analyze` is a 61-kind dispatcher** — beyond the structural kinds, it now covers `impact` (composite change-risk score), `bottlenecks` (interprocedural computation-bottleneck risk — cognitive complexity, loop depth, transitive/hidden-O(n^k) loop nesting across calls, unguarded recursion), `health_score` (per-symbol A–F grade), `sast` / `named` / `unsafe_patterns` (security), `clusters`, `suggest_boundaries` (Leiden-community-seeded architecture-layer suggestions), `connectivity_health`, `tests_as_edges`, `synthesizers` (framework-dispatch-synthesized edges, grouped by pass + provenance), `resolution_outcomes` (structured why-unresolved taxonomy), `review` (an idiomatic/correctness rulepack — NPE, thread-safety check-then-act, N+1, logic errors across Go + Python — with a graph-grounded false-positive-reduction pass), and more. ## MANDATORY: Session memory — save, recall, distill diff --git a/cmd/gortex/eval_recall.go b/cmd/gortex/eval_recall.go index 05f364b5e..a4e78d71d 100644 --- a/cmd/gortex/eval_recall.go +++ b/cmd/gortex/eval_recall.go @@ -16,9 +16,9 @@ import ( "gopkg.in/yaml.v3" "github.com/zzet/gortex/internal/config" - "github.com/zzet/gortex/internal/gitcmd" "github.com/zzet/gortex/internal/embedding" "github.com/zzet/gortex/internal/eval/recall" + "github.com/zzet/gortex/internal/gitcmd" "github.com/zzet/gortex/internal/graph" "github.com/zzet/gortex/internal/indexer" gortexmcp "github.com/zzet/gortex/internal/mcp" @@ -233,6 +233,7 @@ func runEvalRecall(_ *cobra.Command, _ []string) error { eng.SetSearch(textBackend) srv := gortexmcp.NewServer(eng, g, idx, nil, zap.NewNop(), cfg.Guards.Rules) srv.SetArchitecture(cfg.Architecture) + srv.SetEventRules(cfg.Events.Rules) srv.SetArtifacts(cfg.Artifacts) srv.SetNamedQueries(cfg.Queries) srv.RunAnalysis() diff --git a/cmd/gortex/eval_server.go b/cmd/gortex/eval_server.go index e37a1963d..804090942 100644 --- a/cmd/gortex/eval_server.go +++ b/cmd/gortex/eval_server.go @@ -60,6 +60,7 @@ func runEvalServer(cmd *cobra.Command, args []string) error { gortexmcp.Version = version srv := gortexmcp.NewServer(eng, g, idx, nil, logger, cfg.Guards.Rules) srv.SetArchitecture(cfg.Architecture) + srv.SetEventRules(cfg.Events.Rules) srv.SetArtifacts(cfg.Artifacts) srv.SetNamedQueries(cfg.Queries) diff --git a/internal/analysis/architecture.go b/internal/analysis/architecture.go index 0f2010e53..725eff654 100644 --- a/internal/analysis/architecture.go +++ b/internal/analysis/architecture.go @@ -66,6 +66,7 @@ func EvaluateArchitecture(g graph.Store, arch config.ArchitectureConfig, changed LayerFrom: fromLayer, LayerTo: toLayer, EdgeType: string(e.Kind), + Severity: ruleSeverity(arch.Severity), }) } } @@ -92,6 +93,9 @@ func evaluateArchRules(g graph.Store, arch config.ArchitectureConfig, changedSym if !ruleApplies(rule, ep, nodeLayer) { continue } + if matchesAnyGlob(ep, rule.Except) { + continue + } label := archRuleLabel(rule) if rule.MaxFanOut > 0 { if fan := distinctCallTargets(g, id); fan > rule.MaxFanOut { @@ -102,6 +106,7 @@ func evaluateArchRules(g graph.Store, arch config.ArchitectureConfig, changedSym "%s has dependency fan-out %d, exceeding the limit of %d", n.ID, fan, rule.MaxFanOut)), Violator: n.ID, + Severity: ruleSeverity(rule.Severity), }) } } @@ -127,6 +132,7 @@ func evaluateArchRules(g graph.Store, arch config.ArchitectureConfig, changedSym "%s calls into %s from outside the permitted set", caller.ID, n.ID)), Violator: caller.ID, EdgeType: string(e.Kind), + Severity: ruleSeverity(rule.Severity), }) } } diff --git a/internal/analysis/event_boundary.go b/internal/analysis/event_boundary.go new file mode 100644 index 000000000..9d020dd51 --- /dev/null +++ b/internal/analysis/event_boundary.go @@ -0,0 +1,160 @@ +package analysis + +import ( + "fmt" + + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" +) + +// EventBoundaryFamily (PYG-1) evaluates declarative event-boundary rules over +// the pub/sub graph. A changed symbol's produce edges (EdgeEmits / +// EdgeProducesTopic) and consume edges (EdgeListensOn / EdgeConsumesTopic) are +// matched against config rules: which paths may produce or consume a topic, +// whether a produced topic must have a consumer, and which paths are forbidden +// from it. It is a RuleFamily, so change_contract runs it like any other. +type EventBoundaryFamily struct { + Rules []config.EventRule +} + +func (f EventBoundaryFamily) Name() string { return "events" } + +var ( + produceEdges = []graph.EdgeKind{graph.EdgeEmits, graph.EdgeProducesTopic} + consumeEdges = []graph.EdgeKind{graph.EdgeListensOn, graph.EdgeConsumesTopic} +) + +func edgeKindIn(k graph.EdgeKind, set []graph.EdgeKind) bool { + for _, e := range set { + if e == k { + return true + } + } + return false +} + +// topicTargets returns the topic/event nodes a symbol links to via the given +// edge kinds. +func topicTargets(g graph.Store, id string, kinds []graph.EdgeKind) []*graph.Node { + var out []*graph.Node + for _, e := range g.GetOutEdges(id) { + if !edgeKindIn(e.Kind, kinds) { + continue + } + if t := g.GetNode(e.To); t != nil { + out = append(out, t) + } + } + return out +} + +// topicHasConsumer reports whether any symbol consumes the topic node. +func topicHasConsumer(g graph.Store, topicID string) bool { + for _, e := range g.GetInEdges(topicID) { + if edgeKindIn(e.Kind, consumeEdges) { + return true + } + } + return false +} + +func (f EventBoundaryFamily) Evaluate(g graph.Store, changedSet []string) []GuardViolation { + if g == nil || len(f.Rules) == 0 { + return nil + } + var violations []GuardViolation + seen := make(map[string]bool) + add := func(v GuardViolation) { + key := v.RuleName + "\x00" + v.Violator + "\x00" + v.Description + if seen[key] { + return + } + seen[key] = true + violations = append(violations, v) + } + + for _, id := range changedSet { + n := g.GetNode(id) + if n == nil { + continue + } + produced := topicTargets(g, id, produceEdges) + consumed := topicTargets(g, id, consumeEdges) + if len(produced) == 0 && len(consumed) == 0 { + continue + } + for _, rule := range f.Rules { + sev := ruleSeverity(rule.Severity) + for _, topic := range produced { + if rule.Topic != "" && !globMatch(rule.Topic, topic.Name) { + continue + } + if rule.Producer != "" && !globMatch(rule.Producer, n.FilePath) { + add(GuardViolation{ + RuleName: eventRuleLabel(rule, topic.Name), + Kind: "event_boundary", + Description: eventMessage(rule, fmt.Sprintf("%s produces topic %q from %s, outside the permitted producer path %q", n.Name, topic.Name, n.FilePath, rule.Producer)), + Violator: n.ID, + Severity: sev, + }) + } + if matchesAnyGlob(n.FilePath, rule.Forbid) { + add(GuardViolation{ + RuleName: eventRuleLabel(rule, topic.Name), + Kind: "event_boundary", + Description: eventMessage(rule, fmt.Sprintf("%s in %s is forbidden from producing topic %q", n.Name, n.FilePath, topic.Name)), + Violator: n.ID, + Severity: sev, + }) + } + if rule.RequireConsumer && !topicHasConsumer(g, topic.ID) { + add(GuardViolation{ + RuleName: eventRuleLabel(rule, topic.Name), + Kind: "event_boundary", + Description: eventMessage(rule, fmt.Sprintf("topic %q is produced by %s but has no consumer", topic.Name, n.Name)), + Violator: n.ID, + Severity: sev, + }) + } + } + for _, topic := range consumed { + if rule.Topic != "" && !globMatch(rule.Topic, topic.Name) { + continue + } + if rule.Consumer != "" && !globMatch(rule.Consumer, n.FilePath) { + add(GuardViolation{ + RuleName: eventRuleLabel(rule, topic.Name), + Kind: "event_boundary", + Description: eventMessage(rule, fmt.Sprintf("%s consumes topic %q from %s, outside the permitted consumer path %q", n.Name, topic.Name, n.FilePath, rule.Consumer)), + Violator: n.ID, + Severity: sev, + }) + } + if matchesAnyGlob(n.FilePath, rule.Forbid) { + add(GuardViolation{ + RuleName: eventRuleLabel(rule, topic.Name), + Kind: "event_boundary", + Description: eventMessage(rule, fmt.Sprintf("%s in %s is forbidden from consuming topic %q", n.Name, n.FilePath, topic.Name)), + Violator: n.ID, + Severity: sev, + }) + } + } + } + } + return violations +} + +func eventRuleLabel(rule config.EventRule, topic string) string { + if rule.Name != "" { + return rule.Name + } + return "event:" + topic +} + +func eventMessage(rule config.EventRule, fallback string) string { + if rule.Message != "" { + return rule.Message + } + return fallback +} diff --git a/internal/analysis/event_boundary_test.go b/internal/analysis/event_boundary_test.go new file mode 100644 index 000000000..46ba1ba0b --- /dev/null +++ b/internal/analysis/event_boundary_test.go @@ -0,0 +1,69 @@ +package analysis + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" +) + +// buildEventGraph wires one producer that emits topic "orders" and, optionally, +// a consumer that listens on it. +func buildEventGraph(withConsumer bool) (*graph.Graph, string) { + g := graph.New() + g.AddNode(&graph.Node{ID: "svc/pub.go::Publish", Name: "Publish", Kind: graph.KindFunction, FilePath: "svc/pub.go"}) + g.AddNode(&graph.Node{ID: "topic::orders", Name: "orders", Kind: graph.KindEvent, FilePath: "svc/pub.go"}) + g.AddEdge(&graph.Edge{From: "svc/pub.go::Publish", To: "topic::orders", Kind: graph.EdgeEmits}) + if withConsumer { + g.AddNode(&graph.Node{ID: "worker/sub.go::Consume", Name: "Consume", Kind: graph.KindFunction, FilePath: "worker/sub.go"}) + g.AddEdge(&graph.Edge{From: "worker/sub.go::Consume", To: "topic::orders", Kind: graph.EdgeListensOn}) + } + return g, "svc/pub.go::Publish" +} + +func TestEventBoundaryProducerPath(t *testing.T) { + g, pubID := buildEventGraph(true) + fam := EventBoundaryFamily{Rules: []config.EventRule{{ + Name: "orders-producer", Topic: "orders", + Producer: "ingest/**", Severity: "error", + }}} + v := fam.Evaluate(g, []string{pubID}) + require.Len(t, v, 1) + require.Equal(t, "event_boundary", v[0].Kind) + require.Equal(t, "error", v[0].Severity) +} + +func TestEventBoundaryRequireConsumer(t *testing.T) { + // No consumer present -> require_consumer fires. + g, pubID := buildEventGraph(false) + fam := EventBoundaryFamily{Rules: []config.EventRule{{ + Name: "orders-needs-consumer", Topic: "orders", RequireConsumer: true, + }}} + require.Len(t, fam.Evaluate(g, []string{pubID}), 1) + + // With a consumer present -> no violation. + g2, pubID2 := buildEventGraph(true) + require.Empty(t, fam.Evaluate(g2, []string{pubID2})) +} + +func TestEventBoundaryForbid(t *testing.T) { + g, pubID := buildEventGraph(true) + fam := EventBoundaryFamily{Rules: []config.EventRule{{ + Name: "no-pub-from-svc", Topic: "*", + Forbid: []string{"svc/**"}, + }}} + v := fam.Evaluate(g, []string{pubID}) + require.Len(t, v, 1) + require.Equal(t, "warn", v[0].Severity) // default severity +} + +func TestEventBoundaryAllowedProducerClean(t *testing.T) { + g, pubID := buildEventGraph(true) + fam := EventBoundaryFamily{Rules: []config.EventRule{{ + Name: "orders-producer", Topic: "orders", + Producer: "svc/**", // pub.go is in svc/, so it's allowed + }}} + require.Empty(t, fam.Evaluate(g, []string{pubID})) +} diff --git a/internal/analysis/guards.go b/internal/analysis/guards.go index e2180c46c..074d2d287 100644 --- a/internal/analysis/guards.go +++ b/internal/analysis/guards.go @@ -19,6 +19,30 @@ type GuardViolation struct { LayerFrom string `json:"layer_from,omitempty"` LayerTo string `json:"layer_to,omitempty"` EdgeType string `json:"edge_type,omitempty"` + // Severity tiers the violation for the change_contract verdict mapping: + // "error" → refuse, "warn" → warn, "info" → annotate. Stamped from the + // rule; empty means the consumer applies its own default. + Severity string `json:"severity,omitempty"` +} + +// ruleSeverity normalises a configured severity, defaulting to "warn" so a +// rule advises until it explicitly opts into blocking ("error"). +func ruleSeverity(s string) string { + if s == "" { + return "warn" + } + return strings.ToLower(s) +} + +// matchesAnyGlob reports whether path p matches any of the (possibly **-using) +// globs — the except-list check shared by the guard and architecture families. +func matchesAnyGlob(p string, globs []string) bool { + for _, g := range globs { + if g != "" && globMatch(g, p) { + return true + } + } + return false } // EvaluateGuards checks the given guard rules against a set of changed symbol IDs @@ -60,6 +84,9 @@ func evaluateCoChange(rule config.GuardRule, changedNodes []*graph.Node) []Guard hasTarget := false for _, n := range changedNodes { + if matchesAnyGlob(n.FilePath, rule.Except) { + continue + } if strings.HasPrefix(n.FilePath, rule.Source) { hasSource = true } @@ -80,6 +107,7 @@ func evaluateCoChange(rule config.GuardRule, changedNodes []*graph.Node) []Guard RuleName: rule.Name, Kind: "co-change", Description: msg, + Severity: ruleSeverity(rule.Severity), }} } @@ -96,6 +124,9 @@ func evaluateBoundary(g graph.Store, rule config.GuardRule, changedNodes []*grap if !strings.HasPrefix(n.FilePath, rule.Source) { continue } + if matchesAnyGlob(n.FilePath, rule.Except) { + continue + } outEdges := g.GetOutEdges(n.ID) for _, edge := range outEdges { @@ -128,6 +159,8 @@ func evaluateBoundary(g graph.Store, rule config.GuardRule, changedNodes []*grap RuleName: rule.Name, Kind: "boundary", Description: fmt.Sprintf("%s: %s %s %s", msg, n.ID, edge.Kind, target.ID), + Violator: n.ID, + Severity: ruleSeverity(rule.Severity), }) } } diff --git a/internal/analysis/rule_family.go b/internal/analysis/rule_family.go new file mode 100644 index 000000000..946d9bd5b --- /dev/null +++ b/internal/analysis/rule_family.go @@ -0,0 +1,47 @@ +package analysis + +import ( + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" +) + +// RuleFamily is one pluggable body of change-gate rules. Guards, architecture, +// event boundaries, and (in the security theme) taint all implement it, so the +// change_contract evaluator runs "every registered family over the changed +// set" instead of hard-wiring each evaluator by hand. Adding a rule family is +// then a registration, not a new branch in the pipeline. +type RuleFamily interface { + // Name identifies the family for provenance on each finding. + Name() string + // Evaluate checks a set of changed symbol IDs against the family's rules + // and returns the violations, each carrying its own severity. + Evaluate(g graph.Store, changedSet []string) []GuardViolation +} + +// GuardsFamily adapts the flat guards: list (co-change / boundary rules). +type GuardsFamily struct { + Rules []config.GuardRule +} + +func (f GuardsFamily) Name() string { return "guards" } + +func (f GuardsFamily) Evaluate(g graph.Store, changedSet []string) []GuardViolation { + if len(f.Rules) == 0 { + return nil + } + return EvaluateGuards(g, f.Rules, changedSet) +} + +// ArchitectureFamily adapts the declarative architecture: layer DSL. +type ArchitectureFamily struct { + Config config.ArchitectureConfig +} + +func (f ArchitectureFamily) Name() string { return "architecture" } + +func (f ArchitectureFamily) Evaluate(g graph.Store, changedSet []string) []GuardViolation { + if f.Config.IsEmpty() { + return nil + } + return EvaluateArchitecture(g, f.Config, changedSet) +} diff --git a/internal/config/config.go b/internal/config/config.go index b3e854745..f8db04811 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -21,12 +21,41 @@ type GuardRule struct { Source string `mapstructure:"source" yaml:"source"` // package/path prefix Target string `mapstructure:"target" yaml:"target"` // package/path prefix Message string `mapstructure:"message" yaml:"message,omitempty"` // human-readable explanation + // Severity tiers the violation: "error" → a change_contract refuse, + // "warn" → warn, "info" → annotate. Empty defaults to "warn" so a rule + // advises until it explicitly opts into blocking. + Severity string `mapstructure:"severity" yaml:"severity,omitempty"` + // Except lists path globs exempt from this rule — a source symbol whose + // file matches one is never flagged. + Except []string `mapstructure:"except" yaml:"except,omitempty"` } type GuardsConfig struct { Rules []GuardRule `mapstructure:"rules" yaml:"rules,omitempty"` } +// EventRule is one declarative event-boundary / dispatch-guard constraint over +// the pub/sub graph: which paths may produce or consume a topic, whether a +// produced topic must have a consumer, and which paths are forbidden from it. +type EventRule struct { + Name string `mapstructure:"name" yaml:"name,omitempty"` + Topic string `mapstructure:"topic" yaml:"topic,omitempty"` // glob on event/topic name; empty = any + Producer string `mapstructure:"producer" yaml:"producer,omitempty"` // path glob allowed to produce + Consumer string `mapstructure:"consumer" yaml:"consumer,omitempty"` // path glob allowed to consume + RequireConsumer bool `mapstructure:"require_consumer" yaml:"require_consumer,omitempty"` // a produced topic must have a consumer + Forbid []string `mapstructure:"forbid" yaml:"forbid,omitempty"` // path globs forbidden to produce / consume + Severity string `mapstructure:"severity" yaml:"severity,omitempty"` + Message string `mapstructure:"message" yaml:"message,omitempty"` +} + +// EventsConfig is the `events:` block of .gortex.yaml — declarative +// event-boundary rules over the pub/sub graph (EdgeEmits / EdgeProducesTopic / +// EdgeListensOn / EdgeConsumesTopic), evaluated as a change_contract rule +// family. +type EventsConfig struct { + Rules []EventRule `mapstructure:"rules" yaml:"rules,omitempty"` +} + // ArchitectureConfig is the declarative architecture-rules block of // .gortex.yaml — the top-level `architecture:` key. It promotes the // flat guards: list into named layers with directional allow/deny @@ -41,6 +70,10 @@ type ArchitectureConfig struct { // fan-out caps and caller-boundary restrictions — evaluated on // top of the layer allow/deny graph. Rules []ArchRule `mapstructure:"rules" yaml:"rules,omitempty"` + // Severity tiers the layer allow/deny violations: "error" → a + // change_contract refuse, "warn" → warn, "info" → annotate. Empty + // defaults to "warn". + Severity string `mapstructure:"severity" yaml:"severity,omitempty"` } // ArchRule is one architecture constraint scoped to a layer or a file @@ -63,6 +96,11 @@ type ArchRule struct { DenyCallersOutside []string `mapstructure:"deny_callers_outside" yaml:"deny_callers_outside,omitempty"` // Message is an optional human-readable explanation. Message string `mapstructure:"message" yaml:"message,omitempty"` + // Severity tiers the violation: "error" → a change_contract refuse, + // "warn" → warn, "info" → annotate. Empty defaults to "warn". + Severity string `mapstructure:"severity" yaml:"severity,omitempty"` + // Except lists path globs exempt from this rule. + Except []string `mapstructure:"except" yaml:"except,omitempty"` } // ArtifactEntry is one row of the `artifacts:` manifest — a non-code @@ -421,6 +459,9 @@ type Config struct { // by default; the flat Guards list above keeps working when it is // unset. Architecture ArchitectureConfig `mapstructure:"architecture" yaml:"architecture,omitempty"` + // Events is the declarative event-boundary rule family — pub/sub + // producer/consumer path constraints, evaluated by change_contract. + Events EventsConfig `mapstructure:"events" yaml:"events,omitempty"` // Artifacts is the non-code knowledge manifest — schemas, API // specs, infra configs, and ADRs surfaced as KindArtifact nodes. Artifacts []ArtifactEntry `mapstructure:"artifacts" yaml:"artifacts,omitempty"` diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 1328a1075..27a90b3d8 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -48,11 +48,13 @@ type yamlGuardsConfig struct { } type yamlGuardRule struct { - Name string `yaml:"name"` - Kind string `yaml:"kind"` - Source string `yaml:"source"` - Target string `yaml:"target"` - Message string `yaml:"message"` + Name string `yaml:"name"` + Kind string `yaml:"kind"` + Source string `yaml:"source"` + Target string `yaml:"target"` + Message string `yaml:"message"` + Severity string `yaml:"severity"` + Except []string `yaml:"except"` } func toYAMLConfig(gc GuardsConfig) yamlConfig { diff --git a/internal/mcp/change_contract.go b/internal/mcp/change_contract.go new file mode 100644 index 000000000..8a832eea9 --- /dev/null +++ b/internal/mcp/change_contract.go @@ -0,0 +1,708 @@ +package mcp + +import ( + "context" + "fmt" + "path/filepath" + "sort" + "strings" + + mcp "github.com/mark3labs/mcp-go/mcp" + + "github.com/zzet/gortex/internal/analysis" + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/semantic/lsp" +) + +// change_contract is the one envelope every change source lowers into. Rather +// than a sibling verb per question (api_drift, refuse_gate, risk_guard, …) it +// runs one pipeline — LOWER → PREDICT → EVALUATE → SCORE → CLASSIFY → EMIT — +// and returns one verdict envelope. The analysis emits data the agent reads; +// a thin enforcement layer (a pretooluse hook) is the only thing that turns a +// `refuse` verdict into a block. The graph advises; it does not wall. + +// changeVerdict is the top-level decision an agent (or hook) acts on. +type changeVerdict string + +const ( + verdictAllow changeVerdict = "allow" + verdictWarn changeVerdict = "warn" + verdictRefuse changeVerdict = "refuse" +) + +func verdictRank(v changeVerdict) int { + switch v { + case verdictRefuse: + return 2 + case verdictWarn: + return 1 + default: + return 0 + } +} + +// escalate returns the stricter of two verdicts. +func escalate(a, b changeVerdict) changeVerdict { + if verdictRank(b) > verdictRank(a) { + return b + } + return a +} + +// verdictForSeverity maps a rule severity to the verdict it implies: +// error → refuse, warn → warn, info → annotate (allow). +func verdictForSeverity(sev string) changeVerdict { + switch strings.ToLower(sev) { + case "error", "critical": + return verdictRefuse + case "warn", "warning": + return verdictWarn + default: + return verdictAllow + } +} + +// changeReason is one finding carrying its provenance (which rule family) and +// a confidence — a `warn` from a heuristic is not the same as a `refuse` from +// a compile-breaking caller, and the envelope says which. +type changeReason struct { + Family string `json:"family"` + Severity string `json:"severity"` + Message string `json:"message"` + Confidence float64 `json:"confidence"` + Symbol string `json:"symbol,omitempty"` +} + +// changeRisk is the SCORE stage output — PageRank·blast·class folded to 0..100. +type changeRisk struct { + Score int `json:"score"` + Tier string `json:"tier"` // low | medium | high + PageRank float64 `json:"pagerank,omitempty"` + BlastSize int `json:"blast_size"` + LowerBound bool `json:"lower_bound,omitempty"` +} + +// changedSymbolRef is a thin symbol descriptor carried in the envelope. +type changedSymbolRef struct { + ID string `json:"id"` + Name string `json:"name"` + Kind string `json:"kind"` + File string `json:"file"` +} + +// editStrategy is the "here is the safe path" remedy — refuse + remedy in one +// reply. Populated for symbol/range/edit sources where a single dominant +// changed symbol has a recognised refactor shape. +type editStrategy struct { + Technique string `json:"technique,omitempty"` + Steps []string `json:"steps,omitempty"` + CCImpact string `json:"cc_impact,omitempty"` + Safety []string `json:"safety_signals,omitempty"` +} + +// changeEnvelope is the packaged verdict — the output contract of the whole +// pipeline. +type changeEnvelope struct { + Verdict changeVerdict `json:"verdict"` + Source string `json:"source"` + Classification string `json:"classification"` + ChangedSymbols []changedSymbolRef `json:"changed_symbols"` + Reasons []changeReason `json:"reasons"` + Risk changeRisk `json:"risk"` + Blast map[string]any `json:"blast,omitempty"` + VerificationCommand string `json:"verification_command,omitempty"` + StopCondition string `json:"stop_condition,omitempty"` + EditStrategy *editStrategy `json:"edit_strategy,omitempty"` + APISurface []apiSurfaceEntry `json:"api_surface,omitempty"` +} + +// prediction is the normalised PREDICT-stage result. step is non-nil only for +// the workspace_edit source (a true speculative simulation); the other sources +// fill blast/impact from the change set without an edit to apply. +type prediction struct { + source string + lens string + riskGate bool + changed []changedSymbolRef + changedIDs []string + nodes []*graph.Node + step *simulationStep + impact *analysis.ImpactResult + touchedFiles []string +} + +// nodesForIDs resolves symbol IDs to graph nodes, dropping any that no longer +// exist. +func (s *Server) nodesForIDs(ids []string) []*graph.Node { + if s.graph == nil { + return nil + } + out := make([]*graph.Node, 0, len(ids)) + for _, id := range ids { + if n := s.graph.GetNode(id); n != nil { + out = append(out, n) + } + } + return out +} + +func refFromNode(n *graph.Node) changedSymbolRef { + return changedSymbolRef{ID: n.ID, Name: n.Name, Kind: string(n.Kind), File: n.FilePath} +} + +// lowerChange dispatches on the requested source and returns a normalised +// prediction. source "auto" picks the most specific input present. +func (s *Server) lowerChange(ctx context.Context, req mcp.CallToolRequest) (*prediction, error) { + source := strings.ToLower(strings.TrimSpace(req.GetString("source", "auto"))) + if source == "auto" { + switch { + case strings.TrimSpace(req.GetString("workspace_edit", "")) != "": + source = "edit" + case strings.TrimSpace(req.GetString("ranges", "")) != "" || strings.TrimSpace(req.GetString("path", "")) != "": + source = "ranges" + case strings.TrimSpace(req.GetString("symbols", "")) != "": + source = "symbols" + default: + source = "diff" + } + } + + var p *prediction + var err error + switch source { + case "edit": + p, err = s.lowerEditSource(ctx, req) + case "ranges": + p, err = s.lowerRangeSource(ctx, req) + case "symbols": + p, err = s.lowerSymbolSource(ctx, req) + case "diff": + p, err = s.lowerDiffSource(ctx, req) + default: + return nil, fmt.Errorf("unknown source %q (want auto|edit|diff|symbols|ranges)", source) + } + if err != nil { + return nil, err + } + p.lens = strings.ToLower(strings.TrimSpace(req.GetString("lens", ""))) + p.riskGate = riskGateEnabled(req) + return p, nil +} + +// lowerEditSource runs a real speculative simulation of a WorkspaceEdit. +func (s *Server) lowerEditSource(ctx context.Context, req mcp.CallToolRequest) (*prediction, error) { + raw, err := req.RequireString("workspace_edit") + if err != nil { + return nil, fmt.Errorf("source=edit requires workspace_edit") + } + edit, perr := parseWorkspaceEdit(raw) + if perr != nil { + return nil, fmt.Errorf("invalid workspace_edit: %w", perr) + } + if isEmptyEdit(edit) { + return nil, fmt.Errorf("workspace_edit contains no document changes") + } + sim, serr := s.buildSimulation(ctx, []lsp.WorkspaceEdit{edit}, false) + if serr != nil { + return nil, serr + } + step := sim.steps[0] + + ids := append([]string{}, step.symbolsAdded...) + ids = append(ids, step.symbolsRemoved...) + for _, r := range step.symbolsRenamed { + if v := r["old"]; v != "" { + ids = append(ids, v) + } + if v := r["new"]; v != "" { + ids = append(ids, v) + } + } + // Edited ranges that did not add/remove a symbol still touch their + // enclosing symbol — lower the edit's ranges so a body change counts. + for _, h := range s.lowerWorkspaceEditRanges(edit) { + ids = append(ids, h.ID) + } + ids = dedupeStrings(ids) + + nodes := s.nodesForIDs(ids) + changed := make([]changedSymbolRef, 0, len(nodes)) + for _, n := range nodes { + changed = append(changed, refFromNode(n)) + } + return &prediction{ + source: "edit", + changed: changed, + changedIDs: ids, + nodes: nodes, + step: &step, + impact: analysis.AnalyzeImpact(s.graph, ids, s.getCommunities(), s.getProcesses()), + touchedFiles: step.touchedFiles, + }, nil +} + +// lowerWorkspaceEditRanges maps each TextEdit's range to its enclosing symbols. +func (s *Server) lowerWorkspaceEditRanges(edit lsp.WorkspaceEdit) []rangeSymbolHit { + fileEdits, err := s.groupEditByFile(edit) + if err != nil { + return nil + } + var specs []rangeSpec + for _, fe := range fileEdits { + target := fe.absPath + if target == "" { + target = fe.overlayPath + } + for _, te := range fe.edits { + // LSP positions are 0-based; graph lines are 1-based. + specs = append(specs, rangeSpec{ + File: target, + StartLine: te.Range.Start.Line + 1, + EndLine: te.Range.End.Line + 1, + }) + } + } + hits, _ := s.lowerRanges(specs) + return hits +} + +func (s *Server) lowerRangeSource(ctx context.Context, req mcp.CallToolRequest) (*prediction, error) { + specs, err := parseRangeSpecs(req) + if err != nil { + return nil, err + } + hits, _ := s.lowerRanges(specs) + ids := make([]string, 0, len(hits)) + changed := make([]changedSymbolRef, 0, len(hits)) + files := make([]string, 0, len(hits)) + for _, h := range hits { + ids = append(ids, h.ID) + changed = append(changed, changedSymbolRef{ID: h.ID, Name: h.Name, Kind: h.Kind, File: h.File}) + files = append(files, h.File) + } + ids = dedupeStrings(ids) + return &prediction{ + source: "ranges", + changed: changed, + changedIDs: ids, + nodes: s.nodesForIDs(ids), + impact: analysis.AnalyzeImpact(s.graph, ids, s.getCommunities(), s.getProcesses()), + touchedFiles: dedupeStrings(files), + }, nil +} + +func (s *Server) lowerSymbolSource(ctx context.Context, req mcp.CallToolRequest) (*prediction, error) { + ids := splitCSV(req.GetString("symbols", "")) + if len(ids) == 0 { + return nil, fmt.Errorf("source=symbols requires a comma-separated `symbols` list") + } + ids = dedupeStrings(ids) + nodes := s.nodesForIDs(ids) + changed := make([]changedSymbolRef, 0, len(nodes)) + files := make([]string, 0, len(nodes)) + for _, n := range nodes { + changed = append(changed, refFromNode(n)) + files = append(files, n.FilePath) + } + return &prediction{ + source: "symbols", + changed: changed, + changedIDs: ids, + nodes: nodes, + impact: analysis.AnalyzeImpact(s.graph, ids, s.getCommunities(), s.getProcesses()), + touchedFiles: dedupeStrings(files), + }, nil +} + +func (s *Server) lowerDiffSource(ctx context.Context, req mcp.CallToolRequest) (*prediction, error) { + scope := strings.TrimSpace(req.GetString("scope", "unstaged")) + base := strings.TrimSpace(req.GetString("base", "")) + if base != "" && (scope == "" || scope == "unstaged") { + scope = "compare" + } + if scope == "" { + scope = "unstaged" + } + repoRoot, repoPrefix := s.diffRepoScope(ctx, strings.TrimSpace(req.GetString("repo", ""))) + if repoRoot == "" { + repoRoot = "." + } + diff, err := analysis.MapGitDiff(s.graph, repoRoot, repoPrefix, scope, base) + if err != nil { + return nil, err + } + ids := make([]string, 0, len(diff.ChangedSymbols)) + changed := make([]changedSymbolRef, 0, len(diff.ChangedSymbols)) + for _, cs := range diff.ChangedSymbols { + ids = append(ids, cs.ID) + changed = append(changed, changedSymbolRef{ID: cs.ID, Name: cs.Name, Kind: cs.Kind, File: cs.FilePath}) + } + ids = dedupeStrings(ids) + return &prediction{ + source: "diff", + changed: changed, + changedIDs: ids, + nodes: s.nodesForIDs(ids), + impact: analysis.AnalyzeImpact(s.graph, ids, s.getCommunities(), s.getProcesses()), + touchedFiles: diff.ChangedFiles, + }, nil +} + +// ruleFamilies returns the change-gate rule families configured for this +// server, in evaluation order. Each implements analysis.RuleFamily; adding a +// family (events, taint) is a registration here, not a new pipeline branch. +func (s *Server) ruleFamilies() []analysis.RuleFamily { + fams := []analysis.RuleFamily{ + analysis.GuardsFamily{Rules: s.guardRules}, + analysis.ArchitectureFamily{Config: s.architecture}, + } + fams = append(fams, s.extraRuleFamilies()...) + return fams +} + +// extraRuleFamilies returns rule families beyond the always-on guards + +// architecture pair — populated as families (event boundaries, taint) come +// online. +func (s *Server) extraRuleFamilies() []analysis.RuleFamily { + var fams []analysis.RuleFamily + if len(s.eventRules) > 0 { + fams = append(fams, analysis.EventBoundaryFamily{Rules: s.eventRules}) + } + return fams +} + +// evaluateChange runs every registered rule family over the changed set. +func (s *Server) evaluateChange(p *prediction) []analysis.GuardViolation { + if len(p.changedIDs) == 0 { + return nil + } + var violations []analysis.GuardViolation + for _, fam := range s.ruleFamilies() { + violations = append(violations, fam.Evaluate(s.graph, p.changedIDs)...) + } + return violations +} + +// severityForViolation prefers the severity the rule stamped; absent that it +// falls back to a conservative family default (advisory rules warn). +func severityForViolation(v analysis.GuardViolation) string { + if v.Severity != "" { + return v.Severity + } + switch v.Kind { + case "layer", "boundary", "fan_out", "caller_boundary", "co-change": + return "warn" + default: + return "info" + } +} + +func familyForViolation(v analysis.GuardViolation) string { + switch v.Kind { + case "co-change": + return "co_change" + case "layer", "boundary", "fan_out", "caller_boundary": + return "architecture" + default: + return "guards" + } +} + +// scoreChangeRisk folds blast radius and centrality into a 0..100 score. +func (s *Server) scoreChangeRisk(p *prediction) changeRisk { + blast := 0 + lowerBound := false + var impactRisk analysis.RiskLevel + if p.impact != nil { + blast = p.impact.TotalAffected + lowerBound = p.impact.LowerBound + impactRisk = p.impact.Risk + } + var maxPR float64 + if s.pageRank != nil { + for _, id := range p.changedIDs { + if pr := s.pageRank.ScoreOf(id); pr > maxPR { + maxPR = pr + } + } + } + prNorm := 0.0 + if s.pageRank != nil && s.pageRank.Max > 0 { + prNorm = maxPR / s.pageRank.Max + } + score := int(100 * (0.6*saturate(float64(blast), 30) + 0.4*prNorm)) + if score > 100 { + score = 100 + } + + tier := "low" + switch { + case score >= 67 || impactRisk == analysis.RiskHigh || impactRisk == analysis.RiskCritical: + tier = "high" + case score >= 34 || impactRisk == analysis.RiskMedium: + tier = "medium" + } + return changeRisk{Score: score, Tier: tier, PageRank: maxPR, BlastSize: blast, LowerBound: lowerBound} +} + +var configExts = map[string]bool{ + ".yaml": true, ".yml": true, ".json": true, ".toml": true, ".ini": true, + ".env": true, ".tf": true, ".hcl": true, ".conf": true, ".properties": true, +} + +var docExts = map[string]bool{ + ".md": true, ".markdown": true, ".rst": true, ".txt": true, ".adoc": true, +} + +func allMatch(files []string, pred func(string) bool) bool { + if len(files) == 0 { + return false + } + for _, f := range files { + if !pred(f) { + return false + } + } + return true +} + +func isConfigFile(f string) bool { + base := strings.ToLower(filepath.Base(f)) + if base == "dockerfile" || strings.HasPrefix(base, "dockerfile.") || base == "makefile" { + return true + } + return configExts[strings.ToLower(filepath.Ext(f))] +} + +func isDocFile(f string) bool { return docExts[strings.ToLower(filepath.Ext(f))] } + +// classifyChange tags the change behavioral / structural / runtime_drift / +// metadata_only — a pure function of the prediction, feeding both the risk +// reasoning and the verdict. +func classifyChange(p *prediction) string { + if p.step != nil { + renamedOnly := len(p.step.symbolsRenamed) > 0 && + len(p.step.symbolsAdded) == 0 && + len(p.step.symbolsRemoved) == 0 && + len(p.step.brokenCallers) == 0 + if renamedOnly { + return "structural" + } + } + if allMatch(p.touchedFiles, isConfigFile) { + return "runtime_drift" + } + if len(p.changedIDs) == 0 { + if allMatch(p.touchedFiles, isDocFile) { + return "metadata_only" + } + // No symbols resolved and not pure-docs — most likely a non-indexed + // or comment-only change; treat as metadata unless config-driven. + return "metadata_only" + } + return "behavioral" +} + +// buildVerificationCommand synthesises the command that proves the change is +// safe — drawn from the covering tests of the changed set. +func buildVerificationCommand(p *prediction) string { + testFiles := map[string]bool{} + if p.impact != nil { + for _, f := range p.impact.TestFiles { + testFiles[f] = true + } + } + if p.step != nil { + for _, t := range p.step.testTargets { + if strings.HasSuffix(t, "_test.go") { + testFiles[t] = true + } + } + } + + goChange := false + for _, f := range p.touchedFiles { + if strings.HasSuffix(f, ".go") { + goChange = true + break + } + } + + if len(testFiles) > 0 { + dirs := map[string]bool{} + for f := range testFiles { + dirs["./"+filepath.ToSlash(filepath.Dir(f))] = true + } + ds := make([]string, 0, len(dirs)) + for d := range dirs { + ds = append(ds, d) + } + sort.Strings(ds) + if goChange { + return "go test -race " + strings.Join(ds, " ") + } + return "run the covering tests in: " + strings.Join(ds, " ") + } + + if goChange { + dirs := map[string]bool{} + for _, f := range p.touchedFiles { + if strings.HasSuffix(f, ".go") { + dirs["./"+filepath.ToSlash(filepath.Dir(f))+"/..."] = true + } + } + ds := make([]string, 0, len(dirs)) + for d := range dirs { + ds = append(ds, d) + } + sort.Strings(ds) + if len(ds) > 0 { + return "go build " + strings.Join(ds, " ") + " && go test -race " + strings.Join(ds, " ") + } + return "go build ./... && go test -race ./..." + } + return "" +} + +// buildStopCondition states the checkable predicate that, once true, means the +// change is safe to land — the one field with no pre-existing source. +func buildStopCondition(p *prediction, risk changeRisk, verCmd string) string { + var parts []string + if p.step != nil && len(p.step.brokenCallers) > 0 { + parts = append(parts, fmt.Sprintf("the %d broken caller(s) are updated to the new signature", len(p.step.brokenCallers))) + } + if p.step != nil && len(p.step.brokenImplementors) > 0 { + parts = append(parts, fmt.Sprintf("the %d affected interface implementor(s) are reconciled", len(p.step.brokenImplementors))) + } + parts = append(parts, "no new tree-sitter parse errors are introduced") + if verCmd != "" { + parts = append(parts, fmt.Sprintf("`%s` exits 0", verCmd)) + } + if risk.Tier == "high" { + parts = append(parts, "the blast radius has been reviewed (re-run change_contract to confirm the verdict clears)") + } + return "Done when " + strings.Join(parts, " AND ") + "." +} + +// assembleEnvelope is the EMIT stage — fold prediction + violations + risk + +// classification into one verdict. +func (s *Server) assembleEnvelope(p *prediction, violations []analysis.GuardViolation) changeEnvelope { + risk := s.scoreChangeRisk(p) + verdict := verdictAllow + var reasons []changeReason + + for _, v := range violations { + sev := severityForViolation(v) + reasons = append(reasons, changeReason{ + Family: familyForViolation(v), + Severity: sev, + Message: v.Description, + Confidence: 0.85, + Symbol: v.Violator, + }) + verdict = escalate(verdict, verdictForSeverity(sev)) + } + + if p.step != nil && len(p.step.brokenCallers) > 0 { + reasons = append(reasons, changeReason{ + Family: "broken_callers", + Severity: "error", + Message: fmt.Sprintf("%d caller(s) would no longer compile against the changed signature", len(p.step.brokenCallers)), + Confidence: 0.95, + }) + verdict = escalate(verdict, verdictRefuse) + } + if p.step != nil && len(p.step.brokenImplementors) > 0 { + reasons = append(reasons, changeReason{ + Family: "broken_implementors", + Severity: "error", + Message: fmt.Sprintf("%d interface implementor(s) would break", len(p.step.brokenImplementors)), + Confidence: 0.9, + }) + verdict = escalate(verdict, verdictRefuse) + } + + if risk.Tier == "high" { + reasons = append(reasons, changeReason{ + Family: "risk", + Severity: "warn", + Message: fmt.Sprintf("high change risk (score %d, blast %d) — review impact before landing", risk.Score, risk.BlastSize), + Confidence: 0.7, + }) + verdict = escalate(verdict, verdictWarn) + } + + // Co-change omissions: files this set historically moves with but left out. + for _, r := range s.coChangeOmissions(p.touchedFiles) { + reasons = append(reasons, r) + verdict = escalate(verdict, verdictForSeverity(r.Severity)) + } + + // API-drift lens: focus the verdict on the public surface and its consumers. + var apiSurface []apiSurfaceEntry + if p.lens == "api" { + var apiReasons []changeReason + apiReasons, apiSurface = s.apiDriftReasons(p) + for _, r := range apiReasons { + reasons = append(reasons, r) + verdict = escalate(verdict, verdictForSeverity(r.Severity)) + } + } + + // Risk gate: load-bearing symbols need a fresh impact-review ack. + if p.riskGate { + for _, r := range s.riskGateReasons(p, risk) { + reasons = append(reasons, r) + verdict = escalate(verdict, verdictForSeverity(r.Severity)) + } + } + + verCmd := buildVerificationCommand(p) + classification := classifyChange(p) + + env := changeEnvelope{ + Verdict: verdict, + Source: p.source, + Classification: classification, + ChangedSymbols: p.changed, + Reasons: reasons, + Risk: risk, + VerificationCommand: verCmd, + StopCondition: buildStopCondition(p, risk, verCmd), + EditStrategy: s.buildEditStrategy(p), + APISurface: apiSurface, + } + if p.impact != nil { + env.Blast = map[string]any{ + "total_affected": p.impact.TotalAffected, + "risk": p.impact.Risk, + "affected_communities": p.impact.AffectedCommunities, + "affected_processes": p.impact.AffectedProcesses, + "test_files": p.impact.TestFiles, + "lower_bound": p.impact.LowerBound, + } + } + if env.ChangedSymbols == nil { + env.ChangedSymbols = []changedSymbolRef{} + } + if env.Reasons == nil { + env.Reasons = []changeReason{} + } + return env +} + +func (s *Server) handleChangeContract(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { + if s == nil || s.graph == nil { + return mcp.NewToolResultError("change_contract: server not fully initialised"), nil + } + p, err := s.lowerChange(ctx, req) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + if req.GetBool("ack", false) { + return s.handleRiskAck(ctx, req, p) + } + violations := s.evaluateChange(p) + env := s.assembleEnvelope(p, violations) + return s.respondJSONOrTOON(ctx, req, env) +} diff --git a/internal/mcp/change_contract_lenses.go b/internal/mcp/change_contract_lenses.go new file mode 100644 index 000000000..1c8b9c9cd --- /dev/null +++ b/internal/mcp/change_contract_lenses.go @@ -0,0 +1,209 @@ +package mcp + +import ( + "fmt" + "sort" + "strings" + "unicode" + "unicode/utf8" + + "github.com/zzet/gortex/internal/graph" +) + +// This file carries two change sources/lenses that ride the change_contract +// envelope rather than standing up sibling verbs: +// +// - QRT-3 co-change omissions: files the changed set historically moves with +// but that are not in the set — "you forgot to update X". +// - GGR-7 API-drift lens: between two refs (source=diff base=…) with +// lens=api, focus the verdict on the public surface and its consumers, +// composing caller analysis + contract drift + covering tests. + +const coChangeOmissionThreshold = 0.5 + +// coChangeOmissions reports files that historically change together with the +// changed set but were left out. File-level co-change edges are mined from git +// history; a high score that is absent from the touched set is a likely +// forgotten companion edit. +func (s *Server) coChangeOmissions(touchedFiles []string) []changeReason { + if len(touchedFiles) == 0 { + return nil + } + inSet := make(map[string]bool, len(touchedFiles)) + for _, f := range touchedFiles { + inSet[f] = true + } + bestScore := make(map[string]float64) + for _, f := range touchedFiles { + for partner, score := range s.coChangeScores(f) { + if inSet[partner] { + continue + } + if score > bestScore[partner] { + bestScore[partner] = score + } + } + } + + type omission struct { + file string + score float64 + } + var oms []omission + for f, sc := range bestScore { + if sc >= coChangeOmissionThreshold { + oms = append(oms, omission{f, sc}) + } + } + sort.Slice(oms, func(a, b int) bool { + if oms[a].score != oms[b].score { + return oms[a].score > oms[b].score + } + return oms[a].file < oms[b].file + }) + const maxOmissions = 5 + if len(oms) > maxOmissions { + oms = oms[:maxOmissions] + } + + var reasons []changeReason + for _, o := range oms { + reasons = append(reasons, changeReason{ + Family: "co_change_omission", + Severity: "warn", + Message: fmt.Sprintf("%s historically changes with this set (co-change %.2f) but is not included — consider updating it too", o.file, o.score), + Confidence: o.score, + Symbol: o.file, + }) + } + return reasons +} + +// apiSurfaceEntry describes one exported symbol's exposure for the API lens. +type apiSurfaceEntry struct { + ID string `json:"id"` + Name string `json:"name"` + Kind string `json:"kind"` + File string `json:"file"` + ExternalCallers int `json:"external_callers"` + Contracts []string `json:"contracts,omitempty"` +} + +// isExportedChange reports whether a node is part of the public API surface. +// Prefers the indexer-stamped visibility; falls back to the leading-rune +// convention (Go and most curly-brace languages). +func isExportedChange(n *graph.Node) bool { + if n == nil { + return false + } + if v, ok := n.Meta["visibility"].(string); ok && v != "" { + switch strings.ToLower(v) { + case "public", "exported", "open": + return true + case "private", "protected", "internal", "package", "package-private", "fileprivate", "unexported": + return false + } + } + r, _ := utf8.DecodeRuneInString(n.Name) + return unicode.IsUpper(r) +} + +// externalCallers counts call / reference sites that originate outside the +// symbol's own file — the consumers an API change would break. +func (s *Server) externalCallers(n *graph.Node) int { + if s.graph == nil { + return 0 + } + count := 0 + for _, e := range s.graph.GetInEdges(n.ID) { + switch e.Kind { + case graph.EdgeCalls, graph.EdgeReferences, graph.EdgeCrossRepoCalls: + default: + continue + } + if caller := s.graph.GetNode(e.From); caller != nil && caller.FilePath != n.FilePath { + count++ + } + } + return count +} + +// contractsTouched returns the names of API contracts (HTTP routes, topics, +// …) the symbol participates in, via implements / references edges to +// KindContract nodes. +func (s *Server) contractsTouched(n *graph.Node) []string { + if s.graph == nil { + return nil + } + seen := make(map[string]bool) + var names []string + visit := func(edges []*graph.Edge, to func(*graph.Edge) string) { + for _, e := range edges { + if e.Kind != graph.EdgeImplements && e.Kind != graph.EdgeReferences { + continue + } + if cn := s.graph.GetNode(to(e)); cn != nil && cn.Kind == graph.KindContract { + if !seen[cn.Name] { + seen[cn.Name] = true + names = append(names, cn.Name) + } + } + } + } + visit(s.graph.GetOutEdges(n.ID), func(e *graph.Edge) string { return e.To }) + visit(s.graph.GetInEdges(n.ID), func(e *graph.Edge) string { return e.From }) + sort.Strings(names) + return names +} + +// apiDriftReasons evaluates the public-surface drift of the changed set: each +// exported symbol with cross-file consumers (or a participating contract) is a +// breaking-change risk between the two refs. +func (s *Server) apiDriftReasons(p *prediction) ([]changeReason, []apiSurfaceEntry) { + var reasons []changeReason + var surface []apiSurfaceEntry + for _, n := range p.nodes { + if n.Kind != graph.KindFunction && n.Kind != graph.KindMethod && + n.Kind != graph.KindType && n.Kind != graph.KindInterface { + continue + } + if !isExportedChange(n) { + continue + } + ext := s.externalCallers(n) + contracts := s.contractsTouched(n) + surface = append(surface, apiSurfaceEntry{ + ID: n.ID, + Name: n.Name, + Kind: string(n.Kind), + File: n.FilePath, + ExternalCallers: ext, + Contracts: contracts, + }) + if ext > 0 { + reasons = append(reasons, changeReason{ + Family: "api_drift", + Severity: "warn", + Message: fmt.Sprintf("exported %s %s changed and has %d external caller(s) — a signature change here breaks consumers", n.Kind, n.Name, ext), + Confidence: 0.8, + Symbol: n.ID, + }) + } + for _, c := range contracts { + reasons = append(reasons, changeReason{ + Family: "contract_drift", + Severity: "warn", + Message: fmt.Sprintf("%s implements/serves the API contract %q — verify the contract and its mocks still hold", n.Name, c), + Confidence: 0.75, + Symbol: n.ID, + }) + } + } + sort.Slice(surface, func(a, b int) bool { + if surface[a].ExternalCallers != surface[b].ExternalCallers { + return surface[a].ExternalCallers > surface[b].ExternalCallers + } + return surface[a].Name < surface[b].Name + }) + return reasons, surface +} diff --git a/internal/mcp/change_contract_lenses_test.go b/internal/mcp/change_contract_lenses_test.go new file mode 100644 index 000000000..ddd87c342 --- /dev/null +++ b/internal/mcp/change_contract_lenses_test.go @@ -0,0 +1,118 @@ +package mcp + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + mcplib "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/parser" + "github.com/zzet/gortex/internal/parser/languages" + "github.com/zzet/gortex/internal/query" +) + +func TestIsExportedChange(t *testing.T) { + require.True(t, isExportedChange(&graph.Node{Name: "Foo"})) + require.False(t, isExportedChange(&graph.Node{Name: "foo"})) + require.True(t, isExportedChange(&graph.Node{Name: "foo", Meta: map[string]any{"visibility": "public"}})) + require.False(t, isExportedChange(&graph.Node{Name: "Foo", Meta: map[string]any{"visibility": "private"}})) +} + +func TestCoChangeOmissions(t *testing.T) { + s := &Server{} + scores := map[string]map[string]float64{ + "a.go": {"b.go": 0.8, "c.go": 0.3, "d.go": 0.6}, + } + s.storeCoChange(scores, map[string]map[string]int{}) + + // Touching a.go: b.go (0.8) and d.go (0.6) are above threshold and omitted; + // c.go (0.3) is below threshold. + reasons := s.coChangeOmissions([]string{"a.go"}) + require.Len(t, reasons, 2) + files := map[string]bool{} + for _, r := range reasons { + require.Equal(t, "co_change_omission", r.Family) + require.Equal(t, "warn", r.Severity) + files[r.Symbol] = true + } + require.True(t, files["b.go"]) + require.True(t, files["d.go"]) + require.False(t, files["c.go"]) + + // When a partner is already in the touched set it is not an omission. + reasons = s.coChangeOmissions([]string{"a.go", "b.go", "d.go"}) + require.Empty(t, reasons) +} + +func setupAPIServer(t *testing.T) (*Server, graph.Store) { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "api.go"), []byte(`package svc + +func Foo(x int) int { return x * 2 } +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "consumer.go"), []byte(`package svc + +func Bar() int { return Foo(21) } +`), 0o644)) + + g := graph.New() + reg := parser.NewRegistry() + languages.RegisterAll(reg) + cfg := config.Default() + idx := indexer.New(g, reg, cfg.Index, zap.NewNop()) + _, err := idx.Index(dir) + require.NoError(t, err) + + eng := query.NewEngine(g) + srv := NewServer(eng, g, idx, nil, zap.NewNop(), nil) + srv.RunAnalysis() + return srv, g +} + +func TestAPIDriftLens(t *testing.T) { + srv, g := setupAPIServer(t) + + var fooID string + for _, n := range g.AllNodes() { + if n.Name == "Foo" && n.Kind == graph.KindFunction { + fooID = n.ID + } + } + require.NotEmpty(t, fooID, "Foo not indexed") + + req := mcplib.CallToolRequest{} + req.Params.Arguments = map[string]any{ + "source": "symbols", + "symbols": fooID, + "lens": "api", + } + res, err := srv.handleChangeContract(context.Background(), req) + require.NoError(t, err) + require.False(t, res.IsError, "change_contract errored: %s", toolResultText(res)) + + var env changeEnvelope + require.NoError(t, json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &env)) + + require.NotEmpty(t, env.APISurface, "exported Foo should appear on the API surface") + require.Equal(t, "Foo", env.APISurface[0].Name) + require.GreaterOrEqual(t, env.APISurface[0].ExternalCallers, 1, "Bar in consumer.go is an external caller") + + // An exported symbol with external callers drives an api_drift warning. + foundDrift := false + for _, r := range env.Reasons { + if r.Family == "api_drift" { + foundDrift = true + } + } + require.True(t, foundDrift, "expected an api_drift reason, got %+v", env.Reasons) + require.Equal(t, verdictWarn, env.Verdict) +} diff --git a/internal/mcp/change_contract_riskgate.go b/internal/mcp/change_contract_riskgate.go new file mode 100644 index 000000000..9b7c9623e --- /dev/null +++ b/internal/mcp/change_contract_riskgate.go @@ -0,0 +1,188 @@ +package mcp + +import ( + "context" + "fmt" + "os" + "sort" + "time" + + mcp "github.com/mark3labs/mcp-go/mcp" + + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/persistence" +) + +// QRT-6: a risk-gated modification guard. Load-bearing symbols (high fan-in / +// high centrality) require a prior, acknowledged impact review with a TTL. +// The ack is not a side file — it is a development memory keyed on the symbol, +// so it is graph state that survives daemon restarts and surfaces for the next +// agent. The gate emits a refuse verdict; a pretooluse hook enforces it. + +const ( + riskAckTag = "risk-ack" + riskGateCallerDefault = 8 + riskGatePRThreshold = 0.66 + defaultRiskAckTTL = 4 * time.Hour + riskGateMemImportance = 4 +) + +// riskGateEnabled reports whether the per-symbol risk gate is active for this +// call — either the request opted in or GORTEX_RISK_GATE is set. +func riskGateEnabled(req mcp.CallToolRequest) bool { + if req.GetBool("risk_gate", false) { + return true + } + switch os.Getenv("GORTEX_RISK_GATE") { + case "", "0", "false", "off", "no": + return false + default: + return true + } +} + +// riskAckTTL is how long a recorded ack stays fresh. Overridable via +// GORTEX_RISK_ACK_TTL (a Go duration like "2h"). +func riskAckTTL() time.Duration { + if v := os.Getenv("GORTEX_RISK_ACK_TTL"); v != "" { + if d, err := time.ParseDuration(v); err == nil && d > 0 { + return d + } + } + return defaultRiskAckTTL +} + +func riskGateCallerThreshold() int { + if v := os.Getenv("GORTEX_RISK_GATE_MIN_CALLERS"); v != "" { + var n int + if _, err := fmt.Sscanf(v, "%d", &n); err == nil && n > 0 { + return n + } + } + return riskGateCallerDefault +} + +// isRiskGated reports whether a symbol is load-bearing enough to require an ack. +func (s *Server) isRiskGated(n *graph.Node) bool { + if n == nil { + return false + } + fanIn, _ := computeFanInOut(s.graph, []*graph.Node{n}) + if fanIn[n.ID] >= riskGateCallerThreshold() { + return true + } + if s.pageRank != nil && s.pageRank.Max > 0 { + if s.pageRank.ScoreOf(n.ID)/s.pageRank.Max >= riskGatePRThreshold { + return true + } + } + return false +} + +// riskGatedSymbols returns the changed symbols that require an ack. +func (s *Server) riskGatedSymbols(p *prediction) []*graph.Node { + var out []*graph.Node + for _, n := range p.nodes { + if s.isRiskGated(n) { + out = append(out, n) + } + } + return out +} + +// hasFreshAck reports whether a non-expired risk-ack memory exists for a symbol. +func (s *Server) hasFreshAck(id string) bool { + store := s.resolveMemoryStore("workspace") + if store == nil { + return false + } + ttl := riskAckTTL() + for _, m := range store.Query(MemoryQueryFilter{Tag: riskAckTag, SymbolID: id}) { + if time.Since(m.UpdatedAt) < ttl { + return true + } + } + return false +} + +// recordRiskAck stores (or refreshes) the ack memory for a symbol. +func (s *Server) recordRiskAck(id, author string, risk changeRisk) (string, error) { + store := s.resolveMemoryStore("workspace") + if store == nil { + return "", fmt.Errorf("no memory store available to record the ack") + } + body := fmt.Sprintf("Risk-gate ack for %s — change impact reviewed (risk score %d, blast %d). Valid for %s.", + id, risk.Score, risk.BlastSize, riskAckTTL()) + return store.Save(persistence.MemoryEntry{ + Kind: "constraint", + Source: "change_contract", + Body: body, + Tags: []string{riskAckTag}, + SymbolIDs: []string{id}, + Importance: riskGateMemImportance, + AuthorAgent: author, + }) +} + +// riskGateReasons checks each gated symbol for a fresh ack and returns refuse +// reasons for those lacking one. +func (s *Server) riskGateReasons(p *prediction, risk changeRisk) []changeReason { + var reasons []changeReason + for _, n := range s.riskGatedSymbols(p) { + if s.hasFreshAck(n.ID) { + reasons = append(reasons, changeReason{ + Family: "risk_gate", + Severity: "info", + Message: fmt.Sprintf("%s is risk-gated and has a fresh ack — cleared", n.Name), + Confidence: 1, + Symbol: n.ID, + }) + continue + } + reasons = append(reasons, changeReason{ + Family: "risk_gate", + Severity: "error", + Message: fmt.Sprintf("%s is risk-gated (load-bearing) and has no fresh impact-review ack — acknowledge with `change_contract ack:true symbols:%s` (ack valid %s)", n.Name, n.ID, riskAckTTL()), + Confidence: 0.9, + Symbol: n.ID, + }) + } + return reasons +} + +// handleRiskAck records acks for the changed set and returns a confirmation. +func (s *Server) handleRiskAck(ctx context.Context, req mcp.CallToolRequest, p *prediction) (*mcp.CallToolResult, error) { + author := s.ackAuthor(ctx) + risk := s.scoreChangeRisk(p) + + // Ack every changed symbol the caller named (an explicit acknowledgement + // of this change set), so a subsequent gated run on any of them clears. + ids := p.changedIDs + if len(ids) == 0 { + return mcp.NewToolResultError("change_contract ack: no changed symbols to acknowledge"), nil + } + acked := make([]string, 0, len(ids)) + for _, id := range ids { + if _, err := s.recordRiskAck(id, author, risk); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + acked = append(acked, id) + } + sort.Strings(acked) + return s.respondJSONOrTOON(ctx, req, map[string]any{ + "acked": acked, + "ttl": riskAckTTL().String(), + "expires_at": time.Now().Add(riskAckTTL()).UTC().Format(time.RFC3339), + "note": "Risk-gate acks recorded as development memories. A change_contract run with risk_gate:true now clears these symbols until the TTL elapses.", + }) +} + +// ackAuthor returns the MCP client name for ack provenance, or a default. +func (s *Server) ackAuthor(ctx context.Context) string { + if sess := s.sessionFor(ctx); sess != nil { + if n := sess.snapshotClientName(); n != "" { + return n + } + } + return "change_contract" +} diff --git a/internal/mcp/change_contract_riskgate_test.go b/internal/mcp/change_contract_riskgate_test.go new file mode 100644 index 000000000..9a62ba696 --- /dev/null +++ b/internal/mcp/change_contract_riskgate_test.go @@ -0,0 +1,84 @@ +package mcp + +import ( + "context" + "encoding/json" + "testing" + + mcplib "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/graph" +) + +func contractEnvelope(t *testing.T, srv *Server, args map[string]any) changeEnvelope { + t.Helper() + req := mcplib.CallToolRequest{} + req.Params.Arguments = args + res, err := srv.handleChangeContract(context.Background(), req) + require.NoError(t, err) + require.False(t, res.IsError, "change_contract errored: %s", toolResultText(res)) + var env changeEnvelope + require.NoError(t, json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &env)) + return env +} + +func hasErrorRiskGate(env changeEnvelope) bool { + for _, r := range env.Reasons { + if r.Family == "risk_gate" && r.Severity == "error" { + return true + } + } + return false +} + +func TestRiskGateAckLifecycle(t *testing.T) { + t.Setenv("GORTEX_RISK_GATE_MIN_CALLERS", "1") // Foo has 1 caller (Bar) -> gated + srv, g := setupAPIServer(t) + srv.InitMemories("", "") // in-memory ack ledger + + var fooID string + for _, n := range g.AllNodes() { + if n.Name == "Foo" && n.Kind == graph.KindFunction { + fooID = n.ID + } + } + require.NotEmpty(t, fooID) + + // 1. Gated, no ack yet -> refuse. + env := contractEnvelope(t, srv, map[string]any{"source": "symbols", "symbols": fooID, "risk_gate": true}) + require.True(t, hasErrorRiskGate(env), "ungated symbol should produce a risk_gate error, got %+v", env.Reasons) + require.Equal(t, verdictRefuse, env.Verdict) + + // 2. Record an ack. + req := mcplib.CallToolRequest{} + req.Params.Arguments = map[string]any{"source": "symbols", "symbols": fooID, "ack": true} + res, err := srv.handleChangeContract(context.Background(), req) + require.NoError(t, err) + require.False(t, res.IsError, "ack errored: %s", toolResultText(res)) + var ackResp struct { + Acked []string `json:"acked"` + } + require.NoError(t, json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &ackResp)) + require.Contains(t, ackResp.Acked, fooID) + + // 3. Gated run now clears — no risk_gate error. + env = contractEnvelope(t, srv, map[string]any{"source": "symbols", "symbols": fooID, "risk_gate": true}) + require.False(t, hasErrorRiskGate(env), "a fresh ack should clear the gate, got %+v", env.Reasons) +} + +func TestRiskGateOffByDefault(t *testing.T) { + t.Setenv("GORTEX_RISK_GATE_MIN_CALLERS", "1") + srv, g := setupAPIServer(t) + srv.InitMemories("", "") + + var fooID string + for _, n := range g.AllNodes() { + if n.Name == "Foo" && n.Kind == graph.KindFunction { + fooID = n.ID + } + } + // Without risk_gate the gate never engages. + env := contractEnvelope(t, srv, map[string]any{"source": "symbols", "symbols": fooID}) + require.False(t, hasErrorRiskGate(env), "risk gate must be opt-in") +} diff --git a/internal/mcp/change_contract_severity_test.go b/internal/mcp/change_contract_severity_test.go new file mode 100644 index 000000000..1cebe28ef --- /dev/null +++ b/internal/mcp/change_contract_severity_test.go @@ -0,0 +1,75 @@ +package mcp + +import ( + "context" + "encoding/json" + "testing" + + mcplib "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" +) + +// runContractOnBar evaluates change_contract over Bar (consumer.go), which +// calls Foo (api.go), under the given guard rules, and returns the envelope. +func runContractOnBar(t *testing.T, rules []config.GuardRule) changeEnvelope { + t.Helper() + srv, g := setupAPIServer(t) + srv.guardRules = rules + + var barID string + for _, n := range g.AllNodes() { + if n.Name == "Bar" && n.Kind == graph.KindFunction { + barID = n.ID + } + } + require.NotEmpty(t, barID, "Bar not indexed") + + req := mcplib.CallToolRequest{} + req.Params.Arguments = map[string]any{"source": "symbols", "symbols": barID} + res, err := srv.handleChangeContract(context.Background(), req) + require.NoError(t, err) + require.False(t, res.IsError, "change_contract errored: %s", toolResultText(res)) + + var env changeEnvelope + require.NoError(t, json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &env)) + return env +} + +func hasFamily(env changeEnvelope, family string) bool { + for _, r := range env.Reasons { + if r.Family == family { + return true + } + } + return false +} + +func TestChangeContractSeverityError(t *testing.T) { + env := runContractOnBar(t, []config.GuardRule{{ + Name: "no-consumer-to-api", Kind: "boundary", + Source: "consumer.go", Target: "api.go", Severity: "error", + }}) + require.True(t, hasFamily(env, "architecture"), "boundary rule should fire") + require.Equal(t, verdictRefuse, env.Verdict, "an error-severity boundary break must refuse") +} + +func TestChangeContractSeverityDefaultWarns(t *testing.T) { + env := runContractOnBar(t, []config.GuardRule{{ + Name: "no-consumer-to-api", Kind: "boundary", + Source: "consumer.go", Target: "api.go", // no severity -> default warn + }}) + require.True(t, hasFamily(env, "architecture"), "boundary rule should fire") + require.NotEqual(t, verdictRefuse, env.Verdict, "a default-severity break warns, never refuses") +} + +func TestChangeContractExceptGlobExempts(t *testing.T) { + env := runContractOnBar(t, []config.GuardRule{{ + Name: "no-consumer-to-api", Kind: "boundary", + Source: "consumer.go", Target: "api.go", Severity: "error", + Except: []string{"consumer.go"}, + }}) + require.False(t, hasFamily(env, "architecture"), "except glob should exempt consumer.go") +} diff --git a/internal/mcp/change_contract_strategy.go b/internal/mcp/change_contract_strategy.go new file mode 100644 index 000000000..0fc2d8e90 --- /dev/null +++ b/internal/mcp/change_contract_strategy.go @@ -0,0 +1,117 @@ +package mcp + +import ( + "fmt" + + "github.com/zzet/gortex/internal/graph" +) + +// QRT-4: the forward direction of the gate — when change_contract flags a +// change to a heavy symbol, it doesn't just refuse, it hands back the safe +// path. edit_strategy names a refactoring technique, ranks its complexity +// impact, and lists the graph-derived safety signals that make the refactor +// checkable. Refuse + remedy in one reply. + +// paramCount returns the number of parameters declared on a function/method +// node — params point at their owner via EdgeParamOf. +func (s *Server) paramCount(id string) int { + if s.graph == nil { + return 0 + } + n := 0 + for _, e := range s.graph.GetInEdges(id) { + if e.Kind == graph.EdgeParamOf { + n++ + } + } + return n +} + +// dominantSymbol picks the function/method in the changed set most worth a +// refactor — the widest line span, tie-broken by fan-out. +func (s *Server) dominantSymbol(nodes []*graph.Node) *graph.Node { + var best *graph.Node + bestSpan := -1 + for _, n := range nodes { + if n.Kind != graph.KindFunction && n.Kind != graph.KindMethod { + continue + } + span := n.EndLine - n.StartLine + if span > bestSpan { + best = n + bestSpan = span + } + } + return best +} + +func ccImpactTier(lineCount, fanOut int) string { + switch { + case lineCount >= 120 || fanOut >= 18: + return "High" + case lineCount >= 50 || fanOut >= 9: + return "Medium" + default: + return "Low" + } +} + +// buildEditStrategy derives a named refactoring technique for the dominant +// changed symbol, or nil when the change is small enough not to warrant one. +func (s *Server) buildEditStrategy(p *prediction) *editStrategy { + node := s.dominantSymbol(p.nodes) + if node == nil { + return nil + } + lineCount := node.EndLine - node.StartLine + 1 + params := s.paramCount(node.ID) + fanIn, fanOut := computeFanInOut(s.graph, []*graph.Node{node}) + callers := fanIn[node.ID] + callees := fanOut[node.ID] + + var technique string + var steps []string + switch { + case params >= 5: + technique = "Introduce Parameter Object" + steps = []string{ + fmt.Sprintf("Group the %d parameters of %s into a single options/request struct.", params, node.Name), + "Add the struct type next to the function and migrate the body to read its fields.", + fmt.Sprintf("Use change_contract source=edit with the call-site rewrite to confirm no caller of %s breaks before applying.", node.Name), + } + case lineCount >= 60 || callees >= 10: + technique = "Extract Method" + steps = []string{ + fmt.Sprintf("Identify the %d-line body of %s and pull cohesive blocks into named helpers.", lineCount, node.Name), + "Keep the public signature stable so callers are unaffected.", + "Re-run change_contract after each extraction to confirm the blast radius stays bounded.", + } + case callers >= 10 && lineCount >= 30: + technique = "Extract Method" + steps = []string{ + fmt.Sprintf("%s is %d lines and called from %d sites — extract the stable core into a helper to localise future edits.", node.Name, lineCount, callers), + "Keep the original as a thin wrapper so existing callers are untouched.", + } + default: + // Small, low-fan-out symbol: an in-place edit is fine, no strategy. + return nil + } + + var safety []string + if p.impact != nil && len(p.impact.TestFiles) > 0 { + safety = append(safety, fmt.Sprintf("%d covering test file(s) exercise the changed set", len(p.impact.TestFiles))) + } else { + safety = append(safety, "no covering tests found — add one before refactoring") + } + safety = append(safety, fmt.Sprintf("%d caller(s) are graph-tracked; use rename_symbol / change_contract source=edit to verify them", callers)) + if p.step != nil && len(p.step.brokenCallers) == 0 { + safety = append(safety, "current simulation shows no broken callers") + } + + return &editStrategy{ + Technique: technique, + Steps: steps, + CCImpact: ccImpactTier(lineCount, callees), + Safety: safety, + } +} diff --git a/internal/mcp/change_contract_strategy_test.go b/internal/mcp/change_contract_strategy_test.go new file mode 100644 index 000000000..9d559a4fb --- /dev/null +++ b/internal/mcp/change_contract_strategy_test.go @@ -0,0 +1,92 @@ +package mcp + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + mcplib "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/parser" + "github.com/zzet/gortex/internal/parser/languages" + "github.com/zzet/gortex/internal/query" +) + +func TestCCImpactTier(t *testing.T) { + require.Equal(t, "High", ccImpactTier(200, 0)) + require.Equal(t, "High", ccImpactTier(10, 20)) + require.Equal(t, "Medium", ccImpactTier(60, 0)) + require.Equal(t, "Medium", ccImpactTier(0, 10)) + require.Equal(t, "Low", ccImpactTier(10, 2)) +} + +func TestDominantSymbol(t *testing.T) { + s := &Server{} + nodes := []*graph.Node{ + {ID: "a", Kind: graph.KindFunction, StartLine: 1, EndLine: 5}, + {ID: "b", Kind: graph.KindMethod, StartLine: 10, EndLine: 90}, + {ID: "c", Kind: graph.KindType, StartLine: 1, EndLine: 200}, // not a func/method + } + got := s.dominantSymbol(nodes) + require.NotNil(t, got) + require.Equal(t, "b", got.ID) + + require.Nil(t, s.dominantSymbol([]*graph.Node{{ID: "t", Kind: graph.KindType}})) +} + +func setupStrategyServer(t *testing.T) (*Server, graph.Store) { + t.Helper() + dir := t.TempDir() + src := `package big + +func Wide(a, b, c, d, e, f int) int { + return a + b + c + d + e + f +} +` + require.NoError(t, os.WriteFile(filepath.Join(dir, "big.go"), []byte(src), 0o644)) + + g := graph.New() + reg := parser.NewRegistry() + languages.RegisterAll(reg) + cfg := config.Default() + idx := indexer.New(g, reg, cfg.Index, zap.NewNop()) + _, err := idx.Index(dir) + require.NoError(t, err) + + eng := query.NewEngine(g) + srv := NewServer(eng, g, idx, nil, zap.NewNop(), nil) + srv.RunAnalysis() + return srv, g +} + +func TestEditStrategyIntroduceParameterObject(t *testing.T) { + srv, g := setupStrategyServer(t) + + var wideID string + for _, n := range g.AllNodes() { + if n.Name == "Wide" && n.Kind == graph.KindFunction { + wideID = n.ID + } + } + require.NotEmpty(t, wideID, "Wide function not indexed") + + req := mcplib.CallToolRequest{} + req.Params.Arguments = map[string]any{"source": "symbols", "symbols": wideID} + res, err := srv.handleChangeContract(context.Background(), req) + require.NoError(t, err) + require.False(t, res.IsError, "change_contract errored: %s", toolResultText(res)) + + var env changeEnvelope + require.NoError(t, json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &env)) + require.NotNil(t, env.EditStrategy, "a 6-parameter function should get an edit_strategy") + require.Equal(t, "Introduce Parameter Object", env.EditStrategy.Technique) + require.NotEmpty(t, env.EditStrategy.Steps) + require.NotEmpty(t, env.EditStrategy.Safety) +} diff --git a/internal/mcp/change_contract_test.go b/internal/mcp/change_contract_test.go new file mode 100644 index 000000000..4cbcfd80b --- /dev/null +++ b/internal/mcp/change_contract_test.go @@ -0,0 +1,101 @@ +package mcp + +import ( + "context" + "encoding/json" + "testing" + + mcplib "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" +) + +func TestVerdictEscalation(t *testing.T) { + require.Equal(t, verdictRefuse, escalate(verdictWarn, verdictRefuse)) + require.Equal(t, verdictWarn, escalate(verdictAllow, verdictWarn)) + require.Equal(t, verdictWarn, escalate(verdictWarn, verdictAllow)) + require.Equal(t, verdictRefuse, escalate(verdictRefuse, verdictWarn)) + + require.Equal(t, verdictRefuse, verdictForSeverity("error")) + require.Equal(t, verdictRefuse, verdictForSeverity("critical")) + require.Equal(t, verdictWarn, verdictForSeverity("warn")) + require.Equal(t, verdictAllow, verdictForSeverity("info")) +} + +func TestClassifyChange(t *testing.T) { + // rename-only with no broken callers -> structural + structural := &prediction{step: &simulationStep{ + symbolsRenamed: []map[string]string{{"old": "a", "new": "b"}}, + }} + require.Equal(t, "structural", classifyChange(structural)) + + // config-only touched files -> runtime_drift + cfg := &prediction{touchedFiles: []string{"deploy/values.yaml", "Dockerfile"}, changedIDs: []string{"x"}} + require.Equal(t, "runtime_drift", classifyChange(cfg)) + + // docs-only, no symbols -> metadata_only + docs := &prediction{touchedFiles: []string{"README.md"}} + require.Equal(t, "metadata_only", classifyChange(docs)) + + // code change with symbols -> behavioral + code := &prediction{touchedFiles: []string{"pkg/svc.go"}, changedIDs: []string{"pkg/svc.go::Foo"}} + require.Equal(t, "behavioral", classifyChange(code)) +} + +func TestIsConfigFile(t *testing.T) { + require.True(t, isConfigFile("a/b/values.yaml")) + require.True(t, isConfigFile("Dockerfile")) + require.True(t, isConfigFile("infra/main.tf")) + require.False(t, isConfigFile("pkg/svc.go")) + require.True(t, isDocFile("README.md")) + require.False(t, isDocFile("svc.go")) +} + +func TestBuildVerificationCommand(t *testing.T) { + p := &prediction{ + touchedFiles: []string{"internal/svc/svc.go"}, + impact: nil, + } + cmd := buildVerificationCommand(p) + require.Contains(t, cmd, "go ") + + p2 := &prediction{touchedFiles: []string{"docs/guide.md"}} + require.Equal(t, "", buildVerificationCommand(p2)) +} + +func TestChangeContractSymbolSource(t *testing.T) { + srv, g := setupNavServer(t) + startID := navFindMethod(t, g, "Start") + + req := mcplib.CallToolRequest{} + req.Params.Arguments = map[string]any{ + "source": "symbols", + "symbols": startID, + } + res, err := srv.handleChangeContract(context.Background(), req) + require.NoError(t, err) + require.False(t, res.IsError, "change_contract errored: %s", toolResultText(res)) + + var env changeEnvelope + require.NoError(t, json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &env)) + + require.Equal(t, "symbols", env.Source) + require.Equal(t, "behavioral", env.Classification) + require.NotEmpty(t, env.ChangedSymbols) + require.Equal(t, startID, env.ChangedSymbols[0].ID) + // A symbol set with no signature change and no guard rules never refuses + // (refuse is reserved for correctness breakage); it may warn on risk. + require.NotEqual(t, verdictRefuse, env.Verdict) + require.NotEmpty(t, env.StopCondition) + require.NotEmpty(t, env.Risk.Tier) +} + +func TestChangeContractDiffSourceCleanTree(t *testing.T) { + srv, _ := setupNavServer(t) + // The temp repo isn't a git repo; the diff source should fail gracefully + // (an error result, not a panic). + req := mcplib.CallToolRequest{} + req.Params.Arguments = map[string]any{"source": "symbols", "symbols": ""} + res, err := srv.handleChangeContract(context.Background(), req) + require.NoError(t, err) + require.True(t, res.IsError, "empty symbol set should be a clean error") +} diff --git a/internal/mcp/edit_file_expected_test.go b/internal/mcp/edit_file_expected_test.go new file mode 100644 index 000000000..319dc7c57 --- /dev/null +++ b/internal/mcp/edit_file_expected_test.go @@ -0,0 +1,38 @@ +package mcp + +import ( + "context" + "os" + "path/filepath" + "testing" + + mcplib "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" +) + +func TestEditFileExpectedOccurrences(t *testing.T) { + srv, dir := setupTestServer(t) + p := filepath.Join(dir, "occ.txt") + require.NoError(t, os.WriteFile(p, []byte("x\nx\nx\n"), 0o644)) + + // Mismatch: 3 occurrences but the caller asserted 2 -> refuse, no write. + req := mcplib.CallToolRequest{} + req.Params.Arguments = map[string]any{ + "path": p, "old_string": "x", "new_string": "y", + "replace_all": true, "expected_occurrences": float64(2), + } + res, err := srv.handleEditFile(context.Background(), req) + require.NoError(t, err) + require.True(t, res.IsError, "a cardinality mismatch must refuse the edit") + after, _ := os.ReadFile(p) + require.Equal(t, "x\nx\nx\n", string(after), "a refused edit must not write") + + // Match: asserted 3 -> applies. + resp := callEditHandlerJSON(t, srv.handleEditFile, map[string]any{ + "path": p, "old_string": "x", "new_string": "y", + "replace_all": true, "expected_occurrences": float64(3), + }) + require.Equal(t, "applied", resp["status"]) + after, _ = os.ReadFile(p) + require.Equal(t, "y\ny\ny\n", string(after)) +} diff --git a/internal/mcp/enclosing.go b/internal/mcp/enclosing.go index 1173d2f6f..3ab1820ad 100644 --- a/internal/mcp/enclosing.go +++ b/internal/mcp/enclosing.go @@ -75,12 +75,13 @@ func (i *fileSymbolIndex) finalise() { }) } -// find returns (symbol_id, name) for the smallest enclosing symbol -// whose [StartLine, EndLine] range covers `line`. Lines are 1-based; -// graph nodes store the same convention. -func (i *fileSymbolIndex) find(line int) (string, string) { +// smallestEnclosing returns the narrowest symbol whose [StartLine, +// EndLine] range covers `line`, or nil when no symbol does. Lines are +// 1-based; graph nodes store the same convention. syms is sorted by +// StartLine ascending, so the scan can stop once StartLine passes line. +func (i *fileSymbolIndex) smallestEnclosing(line int) *graph.Node { if i == nil { - return "", "" + return nil } var best *graph.Node bestSpan := int(^uint(0) >> 1) @@ -97,12 +98,49 @@ func (i *fileSymbolIndex) find(line int) (string, string) { bestSpan = span } } + return best +} + +// find returns (symbol_id, name) for the smallest enclosing symbol +// whose [StartLine, EndLine] range covers `line`. +func (i *fileSymbolIndex) find(line int) (string, string) { + best := i.smallestEnclosing(line) if best == nil { return "", "" } return best.ID, best.Name } +// enclosingForRange returns the symbols that enclose any line in the +// inclusive [start, end] range, choosing the smallest enclosing symbol +// at each covered line — so a range inside one function yields that +// function, while a range spanning two functions yields both. Results +// are deduplicated by node ID and returned in first-seen (top-down) +// order. A degenerate range (end < start) collapses to the single +// start line. +func (i *fileSymbolIndex) enclosingForRange(start, end int) []*graph.Node { + if i == nil { + return nil + } + if end < start { + end = start + } + seen := make(map[string]struct{}) + var out []*graph.Node + for line := start; line <= end; line++ { + best := i.smallestEnclosing(line) + if best == nil { + continue + } + if _, ok := seen[best.ID]; ok { + continue + } + seen[best.ID] = struct{}{} + out = append(out, best) + } + return out +} + // enclosingName derives the enclosing owner of a node -- the symbol // the node is declared *inside* -- and returns its (id, name). // diff --git a/internal/mcp/impact_paging.go b/internal/mcp/impact_paging.go new file mode 100644 index 000000000..7c41c6f33 --- /dev/null +++ b/internal/mcp/impact_paging.go @@ -0,0 +1,72 @@ +package mcp + +import ( + "sort" + + "github.com/zzet/gortex/internal/analysis" +) + +// GNX-3: token-economy on the blast-radius output. byDepthCounts is the +// headline an agent usually wants ("47 affected, 3 at depth-1") rather than 47 +// rows; pageByDepth then serves the full rows on demand with offset / limit. + +// byDepthCounts collapses the per-depth impact entries to a depth → count map. +func byDepthCounts(byDepth map[int][]analysis.ImpactEntry) map[int]int { + out := make(map[int]int, len(byDepth)) + for d, entries := range byDepth { + out[d] = len(entries) + } + return out +} + +// pageByDepth flattens the per-depth entries in depth order, skips `offset`, +// and returns at most `limit` of them regrouped by depth, along with the count +// returned and whether the page was truncated. limit <= 0 means "no paging". +func pageByDepth(byDepth map[int][]analysis.ImpactEntry, offset, limit int) (paged map[int][]analysis.ImpactEntry, returned int, truncated bool) { + total := 0 + depths := make([]int, 0, len(byDepth)) + for d, entries := range byDepth { + depths = append(depths, d) + total += len(entries) + } + sort.Ints(depths) + + if limit <= 0 && offset <= 0 { + return byDepth, total, false + } + + out := make(map[int][]analysis.ImpactEntry) + skipped, taken := 0, 0 + for _, d := range depths { + for _, e := range byDepth[d] { + if skipped < offset { + skipped++ + continue + } + if limit > 0 && taken >= limit { + return out, taken, true + } + out[d] = append(out[d], e) + taken++ + } + } + return out, taken, offset+taken < total +} + +// applyImpactDepthPaging mutates an impact response map in place: it always +// adds by_depth_counts, and either drops the heavy by_depth rows +// (summary_only) or replaces them with a paged window (offset / limit). +func applyImpactDepthPaging(result map[string]any, byDepth map[int][]analysis.ImpactEntry, summaryOnly bool, offset, limit int) { + result["by_depth_counts"] = byDepthCounts(byDepth) + if summaryOnly { + delete(result, "by_depth") + return + } + paged, returned, truncated := pageByDepth(byDepth, offset, limit) + result["by_depth"] = paged + if truncated || offset > 0 { + result["by_depth_returned"] = returned + result["by_depth_truncated"] = truncated + result["by_depth_offset"] = offset + } +} diff --git a/internal/mcp/impact_paging_test.go b/internal/mcp/impact_paging_test.go new file mode 100644 index 000000000..f7b02eb7f --- /dev/null +++ b/internal/mcp/impact_paging_test.go @@ -0,0 +1,62 @@ +package mcp + +import ( + "context" + "encoding/json" + "testing" + + mcplib "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/analysis" +) + +func TestByDepthCounts(t *testing.T) { + byDepth := map[int][]analysis.ImpactEntry{ + 1: {{ID: "a"}, {ID: "b"}}, + 2: {{ID: "c"}}, + } + counts := byDepthCounts(byDepth) + require.Equal(t, 2, counts[1]) + require.Equal(t, 1, counts[2]) +} + +func TestPageByDepth(t *testing.T) { + byDepth := map[int][]analysis.ImpactEntry{ + 1: {{ID: "a"}, {ID: "b"}}, + 2: {{ID: "c"}, {ID: "d"}}, + } + // First page of 2 (depth order): a, b -> truncated. + paged, returned, truncated := pageByDepth(byDepth, 0, 2) + require.Equal(t, 2, returned) + require.True(t, truncated) + require.Len(t, paged[1], 2) + require.Empty(t, paged[2]) + + // Offset 2, limit 2: c, d -> not truncated. + paged, returned, truncated = pageByDepth(byDepth, 2, 2) + require.Equal(t, 2, returned) + require.False(t, truncated) + require.Len(t, paged[2], 2) + + // No paging. + _, returned, truncated = pageByDepth(byDepth, 0, 0) + require.Equal(t, 4, returned) + require.False(t, truncated) +} + +func TestExplainChangeImpactSummaryOnly(t *testing.T) { + srv, g := setupNavServer(t) + bootID := navFindMethod(t, g, "boot") + + req := mcplib.CallToolRequest{} + req.Params.Arguments = map[string]any{"ids": bootID, "summary_only": true} + res, err := srv.handleEnhancedChangeImpact(context.Background(), req) + require.NoError(t, err) + require.False(t, res.IsError, "explain_change_impact errored: %s", toolResultText(res)) + + var payload map[string]any + require.NoError(t, json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &payload)) + require.Contains(t, payload, "by_depth_counts", "summary should always carry counts") + require.NotContains(t, payload, "by_depth", "summary_only should drop the heavy rows") +} diff --git a/internal/mcp/parse_gate.go b/internal/mcp/parse_gate.go new file mode 100644 index 000000000..0fb89d135 --- /dev/null +++ b/internal/mcp/parse_gate.go @@ -0,0 +1,155 @@ +package mcp + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/zzet/gortex/internal/astquery" + "github.com/zzet/gortex/internal/parser" +) + +// The pre-write parse gate refuses an edit_file / write_file that would leave +// a file *more* syntactically broken than it already is. It parses the +// candidate content with tree-sitter before the atomic swap, so a corrupting +// edit is rejected at gate time instead of being discovered after it has +// already landed on disk (and poisoned the graph). Only a regression blocks: +// editing an already-broken file, or one whose language the gate cannot parse, +// is never refused — the gate never stands in the way of a fix. + +// parseGateLanguage maps a file path to the language name understood by +// astquery.DefaultLanguageResolver. Returns "" for files whose syntax the gate +// cannot check; those writes skip the gate entirely (a safe no-op). +func parseGateLanguage(path string) string { + switch strings.ToLower(filepath.Ext(path)) { + case ".go": + return "go" + case ".py", ".pyi": + return "python" + case ".js", ".jsx", ".mjs", ".cjs": + return "javascript" + case ".ts", ".mts", ".cts": + return "typescript" + case ".tsx": + return "tsx" + case ".rb": + return "ruby" + case ".java": + return "java" + case ".kt", ".kts": + return "kotlin" + case ".scala", ".sc": + return "scala" + case ".rs": + return "rust" + case ".ex", ".exs": + return "elixir" + case ".php": + return "php" + case ".c", ".h": + return "c" + case ".cc", ".cpp", ".cxx", ".hpp", ".hh", ".hxx": + return "cpp" + case ".cs": + return "csharp" + case ".sh", ".bash": + return "bash" + } + return "" +} + +// parseErrorCount parses content for the given language and returns the count +// of tree-sitter ERROR / MISSING nodes plus whether the language is one the +// gate can actually parse. (0, false) means "no opinion" — the gate skips. +func parseErrorCount(lang string, content []byte) (int, bool) { + if lang == "" { + return 0, false + } + sl := astquery.DefaultLanguageResolver(lang) + if sl == nil { + return 0, false + } + tree, err := parser.ParseFile(content, sl) + if err != nil || tree == nil { + // A failure here is a tree-sitter cancellation / timeout, not a + // syntax verdict — stay silent rather than block on infrastructure. + return 0, false + } + pt := parser.NewParseTree(tree, content, lang) + defer pt.Release() + return pt.CountParseErrors(), true +} + +// parseGateResult is the verdict of the pre-write syntax gate. +type parseGateResult struct { + Checked bool // the language was parseable and the gate ran + Blocked bool // newContent introduces parse errors the old content did not + OldErrors int // parse errors in the pre-edit content + NewErrors int // parse errors in the candidate content + Language string // gate language (may be set even when Checked is false) +} + +// parseGateEnabled reports whether the pre-write parse gate is active. On by +// default; set GORTEX_EDIT_PARSE_GATE=0 (false / off / no) to disable globally. +func parseGateEnabled() bool { + switch strings.ToLower(strings.TrimSpace(os.Getenv("GORTEX_EDIT_PARSE_GATE"))) { + case "0", "false", "off", "no": + return false + } + return true +} + +// checkParseGate decides whether writing newContent over oldContent would +// introduce new syntax errors. oldContent may be nil (a brand-new file), in +// which case any parse error in newContent counts as a regression. The gate +// blocks only a regression — newErrors strictly greater than oldErrors. +func checkParseGate(path string, oldContent, newContent []byte) parseGateResult { + lang := parseGateLanguage(path) + newErrs, newOK := parseErrorCount(lang, newContent) + if !newOK { + return parseGateResult{Language: lang} + } + oldErrs := 0 + if len(oldContent) > 0 { + if e, ok := parseErrorCount(lang, oldContent); ok { + oldErrs = e + } + } + return parseGateResult{ + Checked: true, + Blocked: newErrs > oldErrs, + OldErrors: oldErrs, + NewErrors: newErrs, + Language: lang, + } +} + +// parseGateError renders the agent-facing refusal message for a blocked write. +func parseGateError(relPath string, r parseGateResult) string { + return fmt.Sprintf( + "parse gate: writing %s would introduce %d new %s parse error(s) (was %d, would be %d) — the edit appears to leave the file syntactically broken and was refused. Fix the fragment, or pass allow_parse_errors=true to write anyway.", + relPath, r.NewErrors-r.OldErrors, r.Language, r.OldErrors, r.NewErrors) +} + +// parseGateInfo renders the gate verdict for inclusion in a tool response. +// Returns nil when the gate did not run (unparseable language), so a clean +// edit on a supported language stays quiet unless something is notable. +func parseGateInfo(r parseGateResult, allowed bool) map[string]any { + if !r.Checked { + return nil + } + if r.OldErrors == r.NewErrors && r.NewErrors == 0 { + return nil // nothing to report — a clean file stayed clean + } + m := map[string]any{ + "language": r.Language, + "old_errors": r.OldErrors, + "new_errors": r.NewErrors, + "blocked": r.Blocked && !allowed, + } + if r.Blocked && allowed { + m["overridden"] = true + } + return m +} diff --git a/internal/mcp/parse_gate_test.go b/internal/mcp/parse_gate_test.go new file mode 100644 index 000000000..ebfc12115 --- /dev/null +++ b/internal/mcp/parse_gate_test.go @@ -0,0 +1,92 @@ +package mcp + +import "testing" + +func TestParseGateLanguage(t *testing.T) { + cases := map[string]string{ + "foo.go": "go", + "a/b/c.py": "python", + "x.tsx": "tsx", + "x.ts": "typescript", + "x.jsx": "javascript", + "main.rs": "rust", + "App.java": "java", + "README.md": "", + "data.json": "", + "Makefile": "", + "noext": "", + "weird.UNKNOWN": "", + } + for path, want := range cases { + if got := parseGateLanguage(path); got != want { + t.Errorf("parseGateLanguage(%q) = %q, want %q", path, got, want) + } + } +} + +func TestParseErrorCountGo(t *testing.T) { + clean := []byte("package main\n\nfunc Add(a, b int) int { return a + b }\n") + if n, ok := parseErrorCount("go", clean); !ok || n != 0 { + t.Fatalf("clean Go: got (%d, %v), want (0, true)", n, ok) + } + + broken := []byte("package main\n\nfunc Add(a, b int) int { return a + \n") + if n, ok := parseErrorCount("go", broken); !ok || n == 0 { + t.Fatalf("broken Go: got (%d, %v), want (>0, true)", n, ok) + } + + // Unsupported language degrades to no-opinion. + if n, ok := parseErrorCount("cobol", clean); ok || n != 0 { + t.Fatalf("unsupported lang: got (%d, %v), want (0, false)", n, ok) + } + if n, ok := parseErrorCount("", clean); ok || n != 0 { + t.Fatalf("empty lang: got (%d, %v), want (0, false)", n, ok) + } +} + +func TestCheckParseGate(t *testing.T) { + clean := []byte("package main\n\nfunc Add(a, b int) int { return a + b }\n") + broken := []byte("package main\n\nfunc Add(a, b int) int { return a + \n") + + // clean -> broken: a regression, must block. + if r := checkParseGate("x.go", clean, broken); !r.Checked || !r.Blocked { + t.Errorf("clean->broken: got %+v, want Checked && Blocked", r) + } + // clean -> clean: no regression. + if r := checkParseGate("x.go", clean, clean); !r.Checked || r.Blocked { + t.Errorf("clean->clean: got %+v, want Checked && !Blocked", r) + } + // broken -> broken: never block an edit to an already-broken file. + if r := checkParseGate("x.go", broken, broken); !r.Checked || r.Blocked { + t.Errorf("broken->broken: got %+v, want Checked && !Blocked", r) + } + // broken -> clean: a fix, never blocked. + if r := checkParseGate("x.go", broken, clean); !r.Checked || r.Blocked { + t.Errorf("broken->clean: got %+v, want Checked && !Blocked", r) + } + // new file (nil old) -> broken: any error is a regression. + if r := checkParseGate("x.go", nil, broken); !r.Checked || !r.Blocked { + t.Errorf("new->broken: got %+v, want Checked && Blocked", r) + } + // unsupported language: gate does not run, never blocks. + if r := checkParseGate("README.md", clean, broken); r.Checked || r.Blocked { + t.Errorf("unsupported: got %+v, want !Checked && !Blocked", r) + } +} + +func TestParseGateInfo(t *testing.T) { + if parseGateInfo(parseGateResult{}, false) != nil { + t.Error("unchecked gate should produce no info") + } + if parseGateInfo(parseGateResult{Checked: true}, false) != nil { + t.Error("clean->clean gate should stay quiet") + } + info := parseGateInfo(parseGateResult{Checked: true, Blocked: true, OldErrors: 0, NewErrors: 2, Language: "go"}, false) + if info == nil || info["blocked"] != true { + t.Errorf("blocked gate info = %v, want blocked:true", info) + } + info = parseGateInfo(parseGateResult{Checked: true, Blocked: true, NewErrors: 2, Language: "go"}, true) + if info == nil || info["blocked"] != false || info["overridden"] != true { + t.Errorf("overridden gate info = %v, want blocked:false overridden:true", info) + } +} diff --git a/internal/mcp/safe_delete_propagate.go b/internal/mcp/safe_delete_propagate.go new file mode 100644 index 000000000..08673ba4c --- /dev/null +++ b/internal/mcp/safe_delete_propagate.go @@ -0,0 +1,158 @@ +package mcp + +import ( + "fmt" + "os" + "sort" + "strings" + + "github.com/zzet/gortex/internal/agents" + "github.com/zzet/gortex/internal/graph" +) + +// SRN-7: transitive propagate-delete. computeCascadeClosure already cascades +// orphaned symbols; the missing half is the surviving callers that still +// reference the deleted symbol. buildPropagationPlan reads each call site and +// classifies it — a standalone statement call can be removed outright; an +// embedded reference is flagged for manual patching — and applyRemoveLinePatches +// deletes the removable lines (parse-gate validated) so the delete leaves no +// dangling reference behind. + +type callerPatch struct { + File string `json:"file"` + CallerName string `json:"caller_name"` + Line int `json:"line"` + CurrentText string `json:"current_text"` + Action string `json:"action"` // remove_line | manual + Reason string `json:"reason"` + abs string // resolved absolute path (internal) +} + +// buildPropagationPlan walks the referencing call sites of a symbol and returns +// the per-caller patch each one needs. +func (s *Server) buildPropagationPlan(target *graph.Node) []callerPatch { + var plan []callerPatch + seen := make(map[string]bool) + for _, e := range s.graph.GetInEdges(target.ID) { + if !isReferencingEdgeKind(e.Kind) || e.Line == 0 { + continue + } + caller := s.graph.GetNode(e.From) + if caller == nil { + continue + } + key := fmt.Sprintf("%s:%d", caller.FilePath, e.Line) + if seen[key] { + continue + } + seen[key] = true + + abs, err := s.resolveNodePath(caller) + if err != nil { + continue + } + text := readSingleLineAt(abs, e.Line) + action, reason := classifyCallSite(text, target.Name) + plan = append(plan, callerPatch{ + File: caller.FilePath, + CallerName: caller.Name, + Line: e.Line, + CurrentText: strings.TrimSpace(text), + Action: action, + Reason: reason, + abs: abs, + }) + } + sort.Slice(plan, func(a, b int) bool { + if plan[a].File != plan[b].File { + return plan[a].File < plan[b].File + } + return plan[a].Line < plan[b].Line + }) + return plan +} + +// classifyCallSite decides whether a reference line is a standalone statement +// call (safe to delete whole) or an embedded reference (manual). Conservative: +// anything ambiguous is manual. +func classifyCallSite(line, target string) (action, reason string) { + t := strings.TrimSpace(line) + if t == "" || !strings.Contains(t, target) { + return "manual", "reference not found on the recorded line — patch by hand" + } + body := strings.TrimRight(t, ";,") + // An assignment or comparison means the call's value is consumed. + if strings.Contains(body, "=") && !strings.Contains(body, "==") { + return "manual", "reference feeds an assignment — removing it would drop a value" + } + for _, frag := range []string{"return ", "return\t", "if ", "for ", "while ", "switch ", "&&", "||", " ? ", "+ ", " +", "<-"} { + if strings.Contains(body, frag) { + return "manual", "reference is embedded in a larger expression" + } + } + core := strings.TrimPrefix(strings.TrimPrefix(body, "defer "), "go ") + if strings.HasSuffix(core, ")") && + (strings.HasPrefix(core, target+"(") || strings.Contains(core, "."+target+"(")) { + return "remove_line", "standalone statement call — the line can be removed" + } + return "manual", "could not confirm a standalone call — patch by hand" +} + +// applyRemoveLinePatches deletes the remove_line patches from their files, +// grouped per file and applied bottom-up so earlier line numbers stay valid. +// A patch whose removal would introduce new parse errors is not applied and is +// returned in failed. Manual patches are ignored here. +func (s *Server) applyRemoveLinePatches(plan []callerPatch) (applied, failed []callerPatch, err error) { + byFile := make(map[string][]callerPatch) + for _, p := range plan { + if p.Action == "remove_line" { + byFile[p.abs] = append(byFile[p.abs], p) + } + } + for abs, patches := range byFile { + content, readErr := os.ReadFile(abs) + if readErr != nil { + return applied, failed, fmt.Errorf("read %s: %w", abs, readErr) + } + lines := strings.Split(string(content), "\n") + // Apply bottom-up so deletions don't shift later indices. + sort.Slice(patches, func(a, b int) bool { return patches[a].Line > patches[b].Line }) + kept := patches[:0] + newLines := append([]string{}, lines...) + for _, p := range patches { + if p.Line < 1 || p.Line > len(newLines) { + failed = append(failed, p) + continue + } + newLines = append(newLines[:p.Line-1], newLines[p.Line:]...) + kept = append(kept, p) + } + newContent := []byte(strings.Join(newLines, "\n")) + // Parse gate: never let a propagated removal corrupt a file. + relPath := patches[0].File + if gate := checkParseGate(relPath, content, newContent); gate.Blocked { + failed = append(failed, patches...) + continue + } + perm := os.FileMode(0o644) + if info, statErr := os.Stat(abs); statErr == nil { + perm = info.Mode().Perm() + } + if writeErr := agents.AtomicWriteFile(abs, newContent, perm); writeErr != nil { + return applied, failed, fmt.Errorf("write %s: %w", abs, writeErr) + } + s.reindexFile(abs) + applied = append(applied, kept...) + } + return applied, failed, nil +} + +func countManual(plan []callerPatch) int { + n := 0 + for _, p := range plan { + if p.Action == "manual" { + n++ + } + } + return n +} diff --git a/internal/mcp/safe_delete_propagate_test.go b/internal/mcp/safe_delete_propagate_test.go new file mode 100644 index 000000000..b7f34059a --- /dev/null +++ b/internal/mcp/safe_delete_propagate_test.go @@ -0,0 +1,107 @@ +package mcp + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + mcplib "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/parser" + "github.com/zzet/gortex/internal/parser/languages" + "github.com/zzet/gortex/internal/query" +) + +func TestClassifyCallSite(t *testing.T) { + a, _ := classifyCallSite("\tTarget()", "Target") + require.Equal(t, "remove_line", a) + + a, _ = classifyCallSite("\tobj.Target()", "Target") + require.Equal(t, "remove_line", a) + + a, _ = classifyCallSite("\tx := Target()", "Target") + require.Equal(t, "manual", a) + + a, _ = classifyCallSite("\treturn Target()", "Target") + require.Equal(t, "manual", a) + + a, _ = classifyCallSite("\tn := a + Target()", "Target") + require.Equal(t, "manual", a) + + a, _ = classifyCallSite("\tdefer Target()", "Target") + require.Equal(t, "remove_line", a) +} + +func setupDeleteServer(t *testing.T) (*Server, graph.Store, string) { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "del.go"), []byte(`package p + +func Target() {} +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "caller.go"), []byte(`package p + +func Caller() { + Target() +} +`), 0o644)) + + g := graph.New() + reg := parser.NewRegistry() + languages.RegisterAll(reg) + cfg := config.Default() + idx := indexer.New(g, reg, cfg.Index, zap.NewNop()) + _, err := idx.Index(dir) + require.NoError(t, err) + + eng := query.NewEngine(g) + srv := NewServer(eng, g, idx, nil, zap.NewNop(), nil) + srv.RunAnalysis() + return srv, g, dir +} + +func TestPropagateDeletePlanAndApply(t *testing.T) { + srv, g, dir := setupDeleteServer(t) + + var targetID string + for _, n := range g.AllNodes() { + if n.Name == "Target" && n.Kind == graph.KindFunction { + targetID = n.ID + } + } + require.NotEmpty(t, targetID) + + callRes := func(args map[string]any) map[string]any { + req := mcplib.CallToolRequest{} + req.Params.Arguments = args + res, err := srv.handleSafeDeleteSymbol(context.Background(), req) + require.NoError(t, err) + require.False(t, res.IsError, "safe_delete errored: %s", toolResultText(res)) + var payload map[string]any + require.NoError(t, json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &payload)) + return payload + } + + // 1. Plan (dry-run): one removable caller patch. + plan := callRes(map[string]any{"id": targetID, "propagate": true, "dry_run": true}) + require.Equal(t, "propagation_plan", plan["status"]) + require.Equal(t, float64(1), plan["removable"]) + require.Equal(t, float64(0), plan["manual"]) + + // 2. Apply: patches the caller and deletes the target. + applied := callRes(map[string]any{"id": targetID, "propagate": true, "dry_run": false}) + status, _ := applied["status"].(string) + require.Contains(t, []string{"deleted", "deleted_by_propagation"}, status) + + // The caller's standalone Target() call was removed. + callerSrc, err := os.ReadFile(filepath.Join(dir, "caller.go")) + require.NoError(t, err) + require.NotContains(t, string(callerSrc), "Target()", "the call site should be removed") +} diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 11f392932..bef2d9500 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -208,6 +208,9 @@ type Server struct { guardRules []config.GuardRule architecture config.ArchitectureConfig + // eventRules is the declarative event-boundary rule family, installed via + // SetEventRules alongside SetArchitecture and evaluated by change_contract. + eventRules []config.EventRule // searchCfg carries the `.gortex.yaml::search` block — rerank // weights plus the search-behaviour knobs (keyword-soup rewrite, // equivalence-class expansion, prose indexing). Installed via @@ -1040,6 +1043,7 @@ func NewServer(engine *query.Engine, g graph.Store, idx *indexer.Indexer, watche s.registerASTTools() s.registerCloneTools() s.registerSimulationTools() + s.registerChangeContractTools() s.registerNotesTools() s.registerMemoriesTools() s.registerNotebookTools() @@ -1876,6 +1880,14 @@ func (s *Server) SetArchitecture(arch config.ArchitectureConfig) { s.architecture = arch } +// SetEventRules installs the declarative event-boundary rule family so +// change_contract evaluates pub/sub producer/consumer constraints. Called by +// the server / daemon entrypoint right after NewServer; a no-op when the +// config carries no event rules. +func (s *Server) SetEventRules(rules []config.EventRule) { + s.eventRules = rules +} + // WatchForReanalysis subscribes to hub events and re-runs analysis after // a debounce period of inactivity. It runs in a background goroutine. func (s *Server) WatchForReanalysis(h *hub.Hub, debounceMs int) { diff --git a/internal/mcp/tools_analysis.go b/internal/mcp/tools_analysis.go index 5ba39b63e..68d847d7e 100644 --- a/internal/mcp/tools_analysis.go +++ b/internal/mcp/tools_analysis.go @@ -36,6 +36,9 @@ func (s *Server) registerAnalysisTools() { mcp.WithString("scope", mcp.Description("unstaged (default), staged, all, or compare")), mcp.WithString("base_ref", mcp.Description("Branch/commit for compare scope (default: main)")), mcp.WithString("repo", mcp.Description("Repository prefix or path (multi-repo mode); defaults to the lone tracked repo or the session's cwd-bound repo")), + mcp.WithBoolean("summary_only", mcp.Description("Return only by_depth_counts and drop the per-depth row lists — the cheapest blast-radius shape.")), + mcp.WithNumber("offset", mcp.Description("Skip this many affected rows (depth order) before returning by_depth — pairs with limit to page a large blast radius.")), + mcp.WithNumber("limit", mcp.Description("Max affected rows to return in by_depth (default 100). by_depth_counts always reports the full per-depth totals.")), ), s.handleDetectChanges, ) @@ -265,7 +268,7 @@ func (s *Server) handleDetectChanges(ctx context.Context, req mcp.CallToolReques impact := analysis.AnalyzeImpact(s.graph, symbolIDs, s.getCommunities(), s.getProcesses()) - return s.respondJSONOrTOON(ctx, req, map[string]any{ + detectResult := map[string]any{ "changed_symbols": diff.ChangedSymbols, "changed_files": diff.ChangedFiles, "risk": impact.Risk, @@ -275,7 +278,12 @@ func (s *Server) handleDetectChanges(ctx context.Context, req mcp.CallToolReques "affected_communities": impact.AffectedCommunities, "test_files": impact.TestFiles, "total_affected": impact.TotalAffected, - }) + } + applyImpactDepthPaging(detectResult, impact.ByDepth, + req.GetBool("summary_only", false), + req.GetInt("offset", 0), + req.GetInt("limit", 100)) + return s.respondJSONOrTOON(ctx, req, detectResult) } // handleEnhancedChangeImpact replaces the original explain_change_impact with risk tiering @@ -304,6 +312,14 @@ func (s *Server) handleEnhancedChangeImpact(ctx context.Context, req mcp.CallToo "cross_repo_impact": impact.CrossRepoImpact, } + // 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 + // "47 affected, 3 at depth-1" summary by default and the rows on demand. + applyImpactDepthPaging(result, impact.ByDepth, + req.GetBool("summary_only", false), + req.GetInt("offset", 0), + req.GetInt("limit", 100)) + // Include per-repo grouping when cross-repo impact is detected. if impact.CrossRepoImpact { result["by_repo"] = impact.ByRepo diff --git a/internal/mcp/tools_coding.go b/internal/mcp/tools_coding.go index 811dfd0a7..e2402d8a5 100644 --- a/internal/mcp/tools_coding.go +++ b/internal/mcp/tools_coding.go @@ -49,6 +49,9 @@ func (s *Server) registerCodingTools() { mcp.NewTool("explain_change_impact", mcp.WithDescription("Given a list of symbols you plan to modify, returns risk-tiered blast radius: d=1 will break, d=2 likely affected, d=3 needs testing. Includes affected processes and communities."), mcp.WithString("ids", mcp.Required(), mcp.Description("Comma-separated list of symbol IDs to modify")), + mcp.WithBoolean("summary_only", mcp.Description("Return only by_depth_counts (e.g. {1:3, 2:12}) and drop the per-depth row lists — the cheapest blast-radius shape when you only need the headline counts.")), + mcp.WithNumber("offset", mcp.Description("Skip this many affected rows (in depth order) before returning by_depth — pairs with limit to page a large blast radius.")), + mcp.WithNumber("limit", mcp.Description("Max affected rows to return in by_depth (default 100, depth order). by_depth_counts always reports the full per-depth totals regardless.")), mcp.WithString("format", mcp.Description("Output format: json (default), gcx (GCX1 compact wire format), or toon")), mcp.WithNumber("max_bytes", mcp.Description("Cap the marshaled response at this many bytes. The longest list is trimmed; truncation metadata rides on the response. Omit for no cap.")), ), @@ -143,8 +146,10 @@ func (s *Server) registerCodingTools() { mcp.WithString("old_string", mcp.Required(), mcp.Description("Exact text to replace (must be unique unless replace_all=true). CRLF/LF line-ending differences against the file are tolerated.")), mcp.WithString("new_string", mcp.Required(), mcp.Description("Replacement text")), mcp.WithBoolean("replace_all", mcp.Description("Replace every occurrence instead of requiring uniqueness (default: false)")), + mcp.WithNumber("expected_occurrences", mcp.Description("Guard: refuse the edit unless old_string matches exactly this many locations. Pairs with replace_all to assert the cardinality of a sweep (e.g. expected_occurrences:7 replace_all:true). Omit or 0 to disable the check.")), mcp.WithBoolean("dry_run", mcp.Description("Validate the replacement and report what would change without writing (default: false)")), mcp.WithString("base_sha", mcp.Description("Optional git blob SHA-1 the caller observed at read time. When set, the call refuses to write if the on-disk file's current SHA differs (drift guard against silent clobbers).")), + mcp.WithBoolean("allow_parse_errors", mcp.Description("Bypass the pre-write parse gate. By default an edit that would introduce new tree-sitter parse errors (leaving the file more syntactically broken than before) is refused before the atomic write; set true to write anyway.")), ), s.handleEditFile, ) @@ -156,6 +161,7 @@ func (s *Server) registerCodingTools() { mcp.WithString("content", mcp.Required(), mcp.Description("Full file content")), mcp.WithBoolean("dry_run", mcp.Description("Report would_create / would_overwrite without writing (default: false)")), mcp.WithString("base_sha", mcp.Description("Optional git blob SHA-1 the caller observed at read time. When set, write_file refuses to overwrite a divergent on-disk file (or write to a path the caller expected to exist but no longer does). Drift guard against silent clobbers on existing files; leave empty when creating a new file.")), + mcp.WithBoolean("allow_parse_errors", mcp.Description("Bypass the pre-write parse gate. By default a write that would introduce new tree-sitter parse errors (relative to the prior content, or any error in a brand-new file) is refused before the atomic write; set true to write anyway.")), ), s.handleWriteFile, ) diff --git a/internal/mcp/tools_enhancements.go b/internal/mcp/tools_enhancements.go index 7377d155a..f9b0a8186 100644 --- a/internal/mcp/tools_enhancements.go +++ b/internal/mcp/tools_enhancements.go @@ -735,7 +735,7 @@ func (s *Server) handlePrefetchContext(ctx context.Context, req mcp.CallToolRequ func (s *Server) handleAnalyze(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { kind, err := req.RequireString("kind") if err != nil { - return mcp.NewToolResultError("kind is required (one of: dead_code, hotspots, cycles, would_create_cycle, todos, blame, coverage, stale_code, ownership, coverage_gaps, stale_flags, releases, cgo_users, wasm_users, orphan_tables, unreferenced_tables, coverage_summary, channel_ops, def_use, goroutine_spawns, field_writers, race_writes, unclosed_channels, unsafe_patterns, health_score, annotation_users, config_readers, event_emitters, pubsub, string_emitters, error_surface, log_events, sql_rebuild, external_calls, synthesizers, resolution_outcomes, retrieval_log, routes, models, components, k8s_resources, images, kustomize, cross_repo, impact, named, tests_as_edges, connectivity_health, pagerank, louvain, wcc, scc, kcore)"), nil + return mcp.NewToolResultError("kind is required (one of: dead_code, hotspots, cycles, would_create_cycle, todos, blame, coverage, stale_code, ownership, coverage_gaps, stale_flags, releases, cgo_users, wasm_users, orphan_tables, unreferenced_tables, coverage_summary, channel_ops, def_use, goroutine_spawns, field_writers, race_writes, unclosed_channels, unsafe_patterns, health_score, annotation_users, config_readers, event_emitters, pubsub, string_emitters, error_surface, log_events, sql_rebuild, external_calls, synthesizers, resolution_outcomes, retrieval_log, routes, models, components, k8s_resources, images, kustomize, cross_repo, impact, named, tests_as_edges, connectivity_health, pagerank, louvain, wcc, scc, kcore, suggest_boundaries)"), nil } switch kind { case "dead_code": @@ -854,6 +854,8 @@ func (s *Server) handleAnalyze(ctx context.Context, req mcp.CallToolRequest) (*m return s.handleAnalyzeConstructorsMissingFields(ctx, req) case "clusters": return s.handleAnalyzeClusters(ctx, req) + case "suggest_boundaries": + return s.handleSuggestBoundaries(ctx, req) case "concepts": return s.handleAnalyzeConcepts(ctx, req) case "impact": diff --git a/internal/mcp/tools_fileops.go b/internal/mcp/tools_fileops.go index 97b0f05ce..0f74274c4 100644 --- a/internal/mcp/tools_fileops.go +++ b/internal/mcp/tools_fileops.go @@ -537,6 +537,11 @@ func (s *Server) handleEditFile(ctx context.Context, req mcp.CallToolRequest) (* matches := findEOLMatches(fileStr, oldString) count := matches.count + if expected := req.GetInt("expected_occurrences", 0); expected > 0 && count != expected { + return mcp.NewToolResultError(fmt.Sprintf( + "expected_occurrences=%d but old_string matches %d location(s) — refusing the edit so a wrong-cardinality replacement can't slip through. Adjust the fragment or the expected count.", + expected, count)), nil + } if count == 0 { return mcp.NewToolResultError( "old_string not found in file. Use get_file_summary or get_editing_context to inspect the current content."), nil @@ -576,6 +581,15 @@ func (s *Server) handleEditFile(ctx context.Context, req mcp.CallToolRequest) (* newContentBytes := []byte(newContent) newSHA := gitBlobSHA(newContentBytes) + allowParseErrors := req.GetBool("allow_parse_errors", false) + var gate parseGateResult + if parseGateEnabled() { + gate = checkParseGate(relPath, content, newContentBytes) + if gate.Blocked && !allowParseErrors && !dryRun { + return mcp.NewToolResultError(parseGateError(relPath, gate)), nil + } + } + if dryRun { // Dry-run: validate everything but skip the write + reindex. // Returns the same shape so callers can branch on dry_run for a @@ -593,6 +607,9 @@ func (s *Server) handleEditFile(ctx context.Context, req mcp.CallToolRequest) (* if matches.normalized { preview["eol_normalized"] = true } + if info := parseGateInfo(gate, allowParseErrors); info != nil { + preview["parse_gate"] = info + } return s.respondJSONOrTOON(ctx, req, preview) } @@ -620,6 +637,9 @@ func (s *Server) handleEditFile(ctx context.Context, req mcp.CallToolRequest) (* if matches.normalized { resp["eol_normalized"] = true } + if info := parseGateInfo(gate, allowParseErrors); info != nil { + resp["parse_gate"] = info + } if health := s.fileSyntaxHealth(relPath, absPath); health != nil { resp["syntax_health"] = health } @@ -675,6 +695,19 @@ func (s *Server) handleWriteFile(ctx context.Context, req mcp.CallToolRequest) ( contentBytes := []byte(content) newSHA := gitBlobSHA(contentBytes) + allowParseErrors := req.GetBool("allow_parse_errors", false) + var gate parseGateResult + if parseGateEnabled() { + var priorContent []byte + if fileExists { + priorContent, _ = os.ReadFile(absPath) + } + gate = checkParseGate(relPath, priorContent, contentBytes) + if gate.Blocked && !allowParseErrors && !dryRun { + return mcp.NewToolResultError(parseGateError(relPath, gate)), nil + } + } + if dryRun { // Dry-run: skip the write + reindex but report what would happen, // including a unified-diff preview of the change. @@ -686,7 +719,7 @@ func (s *Server) handleWriteFile(ctx context.Context, req mcp.CallToolRequest) ( oldContent = string(existing) } } - return s.respondJSONOrTOON(ctx, req, map[string]any{ + preview := map[string]any{ "path": relPath, "status": dryStatus, "dry_run": true, @@ -694,7 +727,11 @@ func (s *Server) handleWriteFile(ctx context.Context, req mcp.CallToolRequest) ( "reindexed": false, "diff": unifiedDiff(relPath, oldContent, content), "new_sha": newSHA, - }) + } + if info := parseGateInfo(gate, allowParseErrors); info != nil { + preview["parse_gate"] = info + } + return s.respondJSONOrTOON(ctx, req, preview) } if err := agents.AtomicWriteFile(absPath, contentBytes, perm); err != nil { @@ -713,6 +750,9 @@ func (s *Server) handleWriteFile(ctx context.Context, req mcp.CallToolRequest) ( "reindexed": reindexed, "new_sha": newSHA, } + if info := parseGateInfo(gate, allowParseErrors); info != nil { + resp["parse_gate"] = info + } if health := s.fileSyntaxHealth(relPath, absPath); health != nil { resp["syntax_health"] = health } diff --git a/internal/mcp/tools_lint_test.go b/internal/mcp/tools_lint_test.go index c0f6f1c87..24a1a6b10 100644 --- a/internal/mcp/tools_lint_test.go +++ b/internal/mcp/tools_lint_test.go @@ -55,11 +55,14 @@ func TestEditFileSurfacesSyntaxHealthOnBreak(t *testing.T) { srv, dir := setupTestServer(t) mainGo := filepath.Join(dir, "main.go") - // Drop helper's closing brace — the file no longer parses. + // Drop helper's closing brace — the file no longer parses. Bypass the + // pre-write parse gate so the edit lands and the post-write syntax_health + // warning is the signal under test. resp := callEditHandlerJSON(t, srv.handleEditFile, map[string]any{ - "path": mainGo, - "old_string": "func helper() {}", - "new_string": "func helper() {", + "path": mainGo, + "old_string": "func helper() {}", + "new_string": "func helper() {", + "allow_parse_errors": true, }) require.Equal(t, "applied", resp["status"]) health, ok := resp["syntax_health"].(map[string]any) diff --git a/internal/mcp/tools_safe_delete.go b/internal/mcp/tools_safe_delete.go index 356a80121..fdc004ab6 100644 --- a/internal/mcp/tools_safe_delete.go +++ b/internal/mcp/tools_safe_delete.go @@ -40,6 +40,7 @@ func (s *Server) registerSafeDeleteSymbolTool() { mcp.WithBoolean("force", mcp.Description("Bypass the referencing-edge check. Use when you've already removed every caller in the same change set. Default false.")), mcp.WithString("cascade", mcp.Description("Orphan propagation mode: \"off\" (default — single-symbol delete only), \"preview\" (compute the transitive orphan closure and return it without deleting), or \"apply\" (compute the closure and delete every symbol in it together with the target).")), mcp.WithBoolean("cascade_into_tests", mcp.Description("When true, symbols referenced only from test files are eligible for the cascade closure. Default false — test-only references disqualify a candidate so the cascade never deletes a symbol just because production stopped using it but tests still do.")), + mcp.WithBoolean("propagate", mcp.Description("Instead of rejecting on referencing edges, build a per-caller delete-and-patch plan: standalone statement calls are removed outright (parse-gate validated), embedded references are flagged for manual patching. dry_run returns the plan; dry_run=false applies the removable patches then deletes the symbol (force=true to delete even with manual / parse-blocked sites remaining).")), mcp.WithString("format", mcp.Description("Output format: json (default), gcx, or toon")), ), s.handleSafeDeleteSymbol, @@ -105,6 +106,67 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq // EdgeDefines / EdgeMemberOf are skipped (they don't represent // "someone calls this"). refs := collectReferencingEdges(s.graph, id) + + // SRN-7: propagate-delete — patch the surviving call sites instead of + // refusing. Builds a per-caller plan, applies the removable standalone + // calls (parse-gate validated), and flags embedded references as manual. + var appliedPatches, failedPatches []callerPatch + if req.GetBool("propagate", false) && len(refs) > 0 { + plan := s.buildPropagationPlan(node) + manual := countManual(plan) + if dryRun { + return s.respondJSONOrTOON(ctx, req, map[string]any{ + "status": "propagation_plan", + "symbol": id, + "file": node.FilePath, + "caller_patches": plan, + "removable": len(plan) - manual, + "manual": manual, + "reference_count": len(refs), + "dry_run": true, + "hint": "re-run with dry_run=false to apply the remove_line patches and delete the symbol; resolve manual patches first or pass force=true", + }) + } + if manual > 0 && !force { + return s.respondJSONOrTOON(ctx, req, map[string]any{ + "status": "propagation_blocked_manual", + "symbol": id, + "caller_patches": plan, + "manual": manual, + "hint": fmt.Sprintf("%d call site(s) need manual patching; resolve them or pass force=true to delete anyway", manual), + }) + } + applied, failed, perr := s.applyRemoveLinePatches(plan) + if perr != nil { + return mcp.NewToolResultError(perr.Error()), nil + } + appliedPatches, failedPatches = applied, failed + if len(failed) > 0 && !force { + return s.respondJSONOrTOON(ctx, req, map[string]any{ + "status": "propagation_partial", + "symbol": id, + "patches_applied": applied, + "patches_failed": failed, + "hint": "some removals would break their file's syntax and were skipped; patch them by hand or pass force=true to delete the symbol anyway", + }) + } + // Callers patched — proceed with the deletion. Re-fetch the node in + // case a same-file removal shifted its line range. + force = true + node = s.graph.GetNode(id) + if node == nil { + return s.respondJSONOrTOON(ctx, req, map[string]any{ + "status": "deleted_by_propagation", + "symbol": id, + "patches_applied": appliedPatches, + "note": "the symbol's definition was removed as a side effect of patching (it no longer resolves in the graph)", + }) + } + if node.StartLine == 0 || node.EndLine == 0 { + return mcp.NewToolResultError("symbol has no line range after propagation: " + id), nil + } + } + if len(refs) > 0 && !force { return s.respondJSONOrTOON(ctx, req, map[string]any{ "status": "rejected_has_references", @@ -233,6 +295,12 @@ func (s *Server) handleSafeDeleteSymbol(ctx context.Context, req mcp.CallToolReq if cascadeMode == cascadeModeApply { result["cascade_deleted"] = deletedIDs } + if len(appliedPatches) > 0 { + result["patches_applied"] = appliedPatches + } + if len(failedPatches) > 0 { + result["patches_failed"] = failedPatches + } return s.respondJSONOrTOON(ctx, req, result) } diff --git a/internal/mcp/tools_suggest_boundaries.go b/internal/mcp/tools_suggest_boundaries.go new file mode 100644 index 000000000..1bd72c686 --- /dev/null +++ b/internal/mcp/tools_suggest_boundaries.go @@ -0,0 +1,211 @@ +package mcp + +import ( + "context" + "fmt" + "sort" + "strings" + + mcp "github.com/mark3labs/mcp-go/mcp" + + "github.com/zzet/gortex/internal/graph" +) + +// QRT-1: the graph teaches you its own architecture. Rather than hand-writing +// layer globs, suggest_boundaries seeds an architecture: block from the +// detected Leiden communities — each community becomes a candidate layer, and +// the observed cross-community call edges become its starter allow list. The +// output is a ready-to-paste .gortex.yaml block the change_contract +// architecture family then enforces. + +type suggestedLayer struct { + Name string `json:"name"` + Paths []string `json:"paths"` + Allow []string `json:"allow"` + Size int `json:"size"` +} + +func (s *Server) handleSuggestBoundaries(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { + comms := s.getCommunities() + if comms == nil || len(comms.Communities) == 0 { + return mcp.NewToolResultError("no communities detected — run `analyze kind=clusters` (or reindex) first so boundaries can be seeded"), nil + } + minSize := req.GetInt("min_size", 3) + limit := req.GetInt("limit", 12) + + // Select the largest communities that have a usable common path prefix. + type cand struct { + comm int // index into comms.Communities + prefix string + name string + size int + } + sorted := make([]int, 0, len(comms.Communities)) + for i := range comms.Communities { + if comms.Communities[i].Size >= minSize { + sorted = append(sorted, i) + } + } + sort.Slice(sorted, func(a, b int) bool { + return comms.Communities[sorted[a]].Size > comms.Communities[sorted[b]].Size + }) + + usedNames := make(map[string]bool) + commToLayer := make(map[string]string) // community ID -> layer name + var cands []cand + for _, ci := range sorted { + if len(cands) >= limit { + break + } + c := comms.Communities[ci] + prefix := commonDirPrefix(c.Files) + if prefix == "" { + continue + } + name := uniqueLayerName(prefix, c.Label, usedNames) + usedNames[name] = true + commToLayer[c.ID] = name + cands = append(cands, cand{comm: ci, prefix: prefix, name: name, size: c.Size}) + } + if len(cands) == 0 { + return mcp.NewToolResultError("detected communities have no coherent path prefix to seed layers from — boundaries cannot be suggested for this graph"), nil + } + + // Observed cross-layer dependencies become the allow lists. + allow := make(map[string]map[string]bool) + for _, cd := range cands { + c := comms.Communities[cd.comm] + for _, memberID := range c.Members { + for _, e := range s.graph.GetOutEdges(memberID) { + if e.Kind != graph.EdgeCalls && e.Kind != graph.EdgeReferences { + continue + } + targetComm, ok := comms.NodeToComm[e.To] + if !ok { + continue + } + toLayer, ok := commToLayer[targetComm] + if !ok || toLayer == cd.name { + continue + } + if allow[cd.name] == nil { + allow[cd.name] = make(map[string]bool) + } + allow[cd.name][toLayer] = true + } + } + } + + layers := make([]suggestedLayer, 0, len(cands)) + for _, cd := range cands { + deps := make([]string, 0, len(allow[cd.name])) + for d := range allow[cd.name] { + deps = append(deps, d) + } + sort.Strings(deps) + layers = append(layers, suggestedLayer{ + Name: cd.name, + Paths: []string{cd.prefix + "/**"}, + Allow: deps, + Size: cd.size, + }) + } + + return s.respondJSONOrTOON(ctx, req, map[string]any{ + "suggested_layers": layers, + "yaml": renderArchitectureYAML(layers), + "community_count": len(comms.Communities), + "modularity": comms.Modularity, + "note": "Starter architecture block seeded from detected communities. Review the layer names and allow lists, then paste into .gortex.yaml; change_contract's architecture family enforces it (set architecture.severity: error to make breaks refuse).", + }) +} + +// commonDirPrefix returns the longest shared leading directory segments across +// the files, or "" when they share none. +func commonDirPrefix(files []string) string { + if len(files) == 0 { + return "" + } + var segLists [][]string + for _, f := range files { + dir := "" + if i := strings.LastIndex(f, "/"); i >= 0 { + dir = f[:i] + } + segLists = append(segLists, strings.Split(dir, "/")) + } + common := segLists[0] + for _, segs := range segLists[1:] { + n := len(common) + if len(segs) < n { + n = len(segs) + } + k := 0 + for k < n && common[k] == segs[k] { + k++ + } + common = common[:k] + if len(common) == 0 { + return "" + } + } + if len(common) == 1 && common[0] == "" { + return "" + } + return strings.Join(common, "/") +} + +// uniqueLayerName derives a yaml-key-safe layer name from a path prefix (or +// the community label), disambiguating against names already used. +func uniqueLayerName(prefix, label string, used map[string]bool) string { + base := sanitizeLayerName(prefix) + if base == "" { + base = sanitizeLayerName(label) + } + if base == "" { + base = "layer" + } + name := base + for i := 2; used[name]; i++ { + name = fmt.Sprintf("%s_%d", base, i) + } + return name +} + +func sanitizeLayerName(s string) string { + // Keep the last two path segments for a terse but distinctive name. + segs := strings.Split(s, "/") + if len(segs) > 2 { + segs = segs[len(segs)-2:] + } + joined := strings.Join(segs, "_") + var b strings.Builder + for _, r := range joined { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '_': + b.WriteRune(r) + case r == '-' || r == '.' || r == ' ': + b.WriteRune('_') + } + } + return strings.Trim(b.String(), "_") +} + +func renderArchitectureYAML(layers []suggestedLayer) string { + var b strings.Builder + b.WriteString("architecture:\n layers:\n") + for _, l := range layers { + fmt.Fprintf(&b, " %s:\n", l.Name) + fmt.Fprintf(&b, " paths: [%q]\n", l.Paths[0]) + if len(l.Allow) == 0 { + b.WriteString(" allow: []\n") + continue + } + quoted := make([]string, len(l.Allow)) + for i, a := range l.Allow { + quoted[i] = fmt.Sprintf("%q", a) + } + fmt.Fprintf(&b, " allow: [%s]\n", strings.Join(quoted, ", ")) + } + return b.String() +} diff --git a/internal/mcp/tools_suggest_boundaries_test.go b/internal/mcp/tools_suggest_boundaries_test.go new file mode 100644 index 000000000..9669e0d63 --- /dev/null +++ b/internal/mcp/tools_suggest_boundaries_test.go @@ -0,0 +1,89 @@ +package mcp + +import ( + "context" + "encoding/json" + "testing" + + mcplib "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/analysis" + "github.com/zzet/gortex/internal/graph" +) + +func TestCommonDirPrefix(t *testing.T) { + require.Equal(t, "internal/parser", commonDirPrefix([]string{"internal/parser/a.go", "internal/parser/b.go"})) + require.Equal(t, "internal", commonDirPrefix([]string{"internal/parser/a.go", "internal/graph/g.go"})) + require.Equal(t, "", commonDirPrefix([]string{"internal/parser/a.go", "cmd/main.go"})) + require.Equal(t, "", commonDirPrefix([]string{"main.go"})) + require.Equal(t, "internal/parser", commonDirPrefix([]string{"internal/parser/a.go"})) +} + +func TestSanitizeAndUniqueLayerName(t *testing.T) { + require.Equal(t, "internal_parser", sanitizeLayerName("internal/parser")) + require.Equal(t, "pkg_auth", sanitizeLayerName("a/b/pkg/auth")) + require.Equal(t, "my_svc", sanitizeLayerName("my-svc")) + + used := map[string]bool{} + n1 := uniqueLayerName("internal/parser", "", used) + used[n1] = true + n2 := uniqueLayerName("internal/parser", "", used) + require.Equal(t, "internal_parser", n1) + require.Equal(t, "internal_parser_2", n2) +} + +func TestRenderArchitectureYAML(t *testing.T) { + out := renderArchitectureYAML([]suggestedLayer{ + {Name: "core", Paths: []string{"internal/core/**"}, Allow: []string{"util"}}, + {Name: "util", Paths: []string{"internal/util/**"}, Allow: nil}, + }) + require.Contains(t, out, "architecture:") + require.Contains(t, out, "core:") + require.Contains(t, out, `allow: ["util"]`) + require.Contains(t, out, "allow: []") +} + +func TestSuggestBoundariesHandler(t *testing.T) { + srv, g := setupNavServer(t) + + // Inject two synthetic communities with an observed cross-edge. + g.AddNode(&graph.Node{ID: "internal/parser/a.go::A", Name: "A", Kind: graph.KindFunction, FilePath: "internal/parser/a.go"}) + g.AddNode(&graph.Node{ID: "internal/graph/g.go::G", Name: "G", Kind: graph.KindFunction, FilePath: "internal/graph/g.go"}) + g.AddEdge(&graph.Edge{From: "internal/parser/a.go::A", To: "internal/graph/g.go::G", Kind: graph.EdgeCalls}) + + srv.communities = &analysis.CommunityResult{ + Communities: []analysis.Community{ + {ID: "c1", Label: "parser", Members: []string{"internal/parser/a.go::A"}, Files: []string{"internal/parser/a.go"}, Size: 4}, + {ID: "c2", Label: "graph", Members: []string{"internal/graph/g.go::G"}, Files: []string{"internal/graph/g.go"}, Size: 5}, + }, + NodeToComm: map[string]string{ + "internal/parser/a.go::A": "c1", + "internal/graph/g.go::G": "c2", + }, + Modularity: 0.5, + } + + req := mcplib.CallToolRequest{} + req.Params.Arguments = map[string]any{"kind": "suggest_boundaries"} + res, err := srv.handleSuggestBoundaries(context.Background(), req) + require.NoError(t, err) + require.False(t, res.IsError, "suggest_boundaries errored: %s", toolResultText(res)) + + var payload struct { + SuggestedLayers []suggestedLayer `json:"suggested_layers"` + YAML string `json:"yaml"` + } + require.NoError(t, json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &payload)) + require.Len(t, payload.SuggestedLayers, 2) + + byName := map[string]suggestedLayer{} + for _, l := range payload.SuggestedLayers { + byName[l.Name] = l + } + parser, ok := byName["internal_parser"] + require.True(t, ok, "expected an internal_parser layer, got %v", byName) + require.Equal(t, []string{"internal/parser/**"}, parser.Paths) + require.Contains(t, parser.Allow, "internal_graph", "observed parser->graph edge should seed an allow") + require.Contains(t, payload.YAML, "architecture:") +} diff --git a/internal/mcp/tools_symbols_for_ranges.go b/internal/mcp/tools_symbols_for_ranges.go new file mode 100644 index 000000000..a7e0d5bef --- /dev/null +++ b/internal/mcp/tools_symbols_for_ranges.go @@ -0,0 +1,216 @@ +package mcp + +import ( + "context" + "encoding/json" + "fmt" + "sort" + + mcp "github.com/mark3labs/mcp-go/mcp" +) + +// symbols_for_ranges is the lowering primitive that turns a set of +// (file, line-range) pairs into the graph symbols those ranges touch. It is +// the input adapter every range-shaped change source funnels through — an +// editor selection, an LSP code-action range, or a parsed diff hunk — so the +// rest of the change pipeline only ever has to reason about symbol IDs. + +// rangeSpec is one (file, [start,end]) request. +type rangeSpec struct { + File string + StartLine int + EndLine int +} + +// rangeSymbolHit is one symbol a range resolved to. +type rangeSymbolHit struct { + ID string `json:"id"` + Name string `json:"name"` + Kind string `json:"kind"` + File string `json:"file"` + StartLine int `json:"start_line"` + EndLine int `json:"end_line"` +} + +// rangeSpecJSON is the wire shape of one entry in the `ranges` array. Both +// `file` and `path` are accepted for the file field; end_line defaults to +// start_line when omitted (a single-line range). +type rangeSpecJSON struct { + File string `json:"file"` + Path string `json:"path"` + StartLine int `json:"start_line"` + EndLine int `json:"end_line"` +} + +// lowerRanges resolves each (file, range) spec to the symbols that enclose any +// line in the range, deduplicated across all specs by symbol ID. It returns +// the hits plus the display paths of any files that could not be resolved or +// carry no indexed symbols, so the caller can report partial coverage rather +// than silently dropping them. +func (s *Server) lowerRanges(specs []rangeSpec) ([]rangeSymbolHit, []string) { + if s == nil || s.graph == nil || len(specs) == 0 { + return nil, nil + } + byGraphPath := make(map[string][]rangeSpec) + displayOf := make(map[string]string) + var unresolved []string + for _, sp := range specs { + absPath, relPath, err := s.resolveFilePath(sp.File) + if err != nil { + unresolved = append(unresolved, sp.File) + continue + } + gp := s.resolveOverlayGraphPath(relPath, absPath) + byGraphPath[gp] = append(byGraphPath[gp], sp) + displayOf[gp] = relPath + } + if len(byGraphPath) == 0 { + return nil, unresolved + } + + want := make(map[string]struct{}, len(byGraphPath)) + for gp := range byGraphPath { + want[gp] = struct{}{} + } + indexes := s.buildFileSymbolIndexForPaths(want) + + seen := make(map[string]struct{}) + var hits []rangeSymbolHit + for gp, specsForFile := range byGraphPath { + idx := indexes[gp] + if idx == nil { + unresolved = append(unresolved, displayOf[gp]) + continue + } + for _, sp := range specsForFile { + for _, n := range idx.enclosingForRange(sp.StartLine, sp.EndLine) { + if _, ok := seen[n.ID]; ok { + continue + } + seen[n.ID] = struct{}{} + hits = append(hits, rangeSymbolHit{ + ID: n.ID, + Name: n.Name, + Kind: string(n.Kind), + File: n.FilePath, + StartLine: n.StartLine, + EndLine: n.EndLine, + }) + } + } + } + sort.Slice(hits, func(a, b int) bool { + if hits[a].File != hits[b].File { + return hits[a].File < hits[b].File + } + return hits[a].StartLine < hits[b].StartLine + }) + sort.Strings(unresolved) + return hits, dedupeStrings(unresolved) +} + +// parseRangeSpecs reads range specs from a request. Two forms are accepted: +// - ranges: a JSON array of {file|path, start_line, end_line} objects. +// - path + start_line [+ end_line]: a single-file convenience form. +func parseRangeSpecs(req mcp.CallToolRequest) ([]rangeSpec, error) { + var specs []rangeSpec + + if raw := req.GetString("ranges", ""); raw != "" { + var entries []rangeSpecJSON + if err := json.Unmarshal([]byte(raw), &entries); err != nil { + return nil, fmt.Errorf("invalid ranges JSON: %w", err) + } + for i, e := range entries { + file := e.File + if file == "" { + file = e.Path + } + if file == "" { + return nil, fmt.Errorf("ranges[%d]: file is required", i) + } + if e.StartLine <= 0 { + return nil, fmt.Errorf("ranges[%d]: start_line must be >= 1", i) + } + end := e.EndLine + if end < e.StartLine { + end = e.StartLine + } + specs = append(specs, rangeSpec{File: file, StartLine: e.StartLine, EndLine: end}) + } + return specs, nil + } + + if path := req.GetString("path", ""); path != "" { + start := req.GetInt("start_line", 0) + if start <= 0 { + return nil, fmt.Errorf("start_line must be >= 1") + } + end := req.GetInt("end_line", start) + if end < start { + end = start + } + specs = append(specs, rangeSpec{File: path, StartLine: start, EndLine: end}) + return specs, nil + } + + return nil, fmt.Errorf("provide either `ranges` (JSON array) or `path` + `start_line`") +} + +func (s *Server) handleSymbolsForRanges(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { + if s == nil || s.graph == nil { + return mcp.NewToolResultError("symbols_for_ranges: server not fully initialised"), nil + } + specs, err := parseRangeSpecs(req) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + hits, unresolved := s.lowerRanges(specs) + if hits == nil { + hits = []rangeSymbolHit{} + } + resp := map[string]any{ + "symbols": hits, + "total": len(hits), + } + if len(unresolved) > 0 { + resp["unresolved_files"] = unresolved + } + return s.respondJSONOrTOON(ctx, req, resp) +} + +// registerChangeContractTools registers the change-source lowering and +// verdict tools: symbols_for_ranges (this commit) and change_contract. +func (s *Server) registerChangeContractTools() { + s.addTool( + mcp.NewTool("symbols_for_ranges", + mcp.WithDescription("Lower a set of (file, line-range) pairs to the graph symbols those ranges touch — the input adapter for any range-shaped change source (an editor selection, an LSP code-action range, a diff hunk). Returns the enclosing symbol(s) for each range, deduplicated by ID, so downstream impact / change-contract analysis only deals in symbol IDs. Pair with `analyze kind=impact ids:` to score the blast radius of a selection."), + mcp.WithString("ranges", mcp.Description("JSON array of {file, start_line, end_line} objects (file or path; end_line defaults to start_line). Lines are 1-based. Use for multi-file or multi-range lowering.")), + mcp.WithString("path", mcp.Description("Single-file convenience form: the file whose range to lower (use with start_line / end_line instead of `ranges`).")), + mcp.WithNumber("start_line", mcp.Description("1-based start line for the single-file form.")), + mcp.WithNumber("end_line", mcp.Description("1-based end line for the single-file form (defaults to start_line).")), + mcp.WithString("format", mcp.Description("Output format: json (default), gcx, or toon")), + ), + s.handleSymbolsForRanges, + ) + + s.addTool( + mcp.NewTool("change_contract", + mcp.WithDescription("Run one change through the full pipeline — LOWER any change source to a changed-symbol set, PREDICT its blast radius, EVALUATE the guard / architecture rules, SCORE the risk, CLASSIFY the change, and EMIT one verdict envelope {verdict: allow|warn|refuse, reasons[], risk, classification, verification_command, stop_condition, edit_strategy}. The analysis advises; a pretooluse hook is the only thing that turns a `refuse` into a block. Sources: a WorkspaceEdit (true speculative simulation with broken-caller detection), a git diff range, an explicit symbol set, or file line-ranges."), + mcp.WithString("source", mcp.Description("Change source: auto (default — pick the most specific input present), edit, diff, symbols, or ranges.")), + mcp.WithString("lens", mcp.Description("Optional analysis lens. lens=api focuses the verdict on the public API surface — for each changed exported symbol it reports cross-file consumers and participating contracts (API-drift between two refs when paired with source=diff base=…).")), + mcp.WithBoolean("risk_gate", mcp.Description("Enable the risk gate: load-bearing symbols (high fan-in / centrality) require a fresh impact-review ack or the verdict refuses. Also enabled by GORTEX_RISK_GATE.")), + mcp.WithBoolean("ack", mcp.Description("Record a risk-gate acknowledgement for the changed symbols (stored as a development memory with a TTL) instead of emitting a verdict. A subsequent risk_gate run then clears those symbols until the ack expires.")), + mcp.WithString("workspace_edit", mcp.Description("source=edit: an LSP WorkspaceEdit as a JSON string. Simulated speculatively (disk untouched) for broken callers / implementors and test targets.")), + mcp.WithString("symbols", mcp.Description("source=symbols: comma-separated symbol IDs to treat as the changed set.")), + mcp.WithString("ranges", mcp.Description("source=ranges: JSON array of {file, start_line, end_line} objects, lowered to enclosing symbols.")), + mcp.WithString("path", mcp.Description("source=ranges single-file form: the file whose range to lower (with start_line / end_line).")), + mcp.WithNumber("start_line", mcp.Description("1-based start line for the single-file ranges form.")), + mcp.WithNumber("end_line", mcp.Description("1-based end line for the single-file ranges form (defaults to start_line).")), + mcp.WithString("scope", mcp.Description("source=diff: unstaged (default), staged, all, or compare.")), + mcp.WithString("base", mcp.Description("source=diff: base ref for a compare-scope diff (e.g. main); setting it implies scope=compare.")), + mcp.WithString("repo", mcp.Description("source=diff: repository selector when more than one repo is tracked.")), + mcp.WithString("format", mcp.Description("Output format: json (default), gcx, or toon")), + ), + s.handleChangeContract, + ) +} diff --git a/internal/mcp/tools_symbols_for_ranges_test.go b/internal/mcp/tools_symbols_for_ranges_test.go new file mode 100644 index 000000000..81f0b3d4d --- /dev/null +++ b/internal/mcp/tools_symbols_for_ranges_test.go @@ -0,0 +1,80 @@ +package mcp + +import ( + "context" + "encoding/json" + "testing" + + mcplib "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/graph" +) + +func TestEnclosingForRange(t *testing.T) { + // A file with two top-level functions and a closure nested in the first. + idx := &fileSymbolIndex{} + idx.add(&graph.Node{ID: "f.go::Outer", Name: "Outer", Kind: graph.KindFunction, StartLine: 1, EndLine: 20}) + idx.add(&graph.Node{ID: "f.go::Outer.closure", Name: "closure", Kind: graph.KindClosure, StartLine: 5, EndLine: 9}) + idx.add(&graph.Node{ID: "f.go::Second", Name: "Second", Kind: graph.KindFunction, StartLine: 22, EndLine: 30}) + idx.finalise() + + // A range inside the closure resolves to the closure (smallest enclosing). + got := ids(idx.enclosingForRange(6, 7)) + require.Equal(t, []string{"f.go::Outer.closure"}, got) + + // A range spanning the closure boundary picks up both the closure and Outer. + got = ids(idx.enclosingForRange(3, 7)) + require.ElementsMatch(t, []string{"f.go::Outer", "f.go::Outer.closure"}, got) + + // A range spanning two functions yields both. + got = ids(idx.enclosingForRange(18, 24)) + require.ElementsMatch(t, []string{"f.go::Outer", "f.go::Second"}, got) + + // A range covering no symbol yields nothing. + require.Empty(t, idx.enclosingForRange(40, 50)) + + // Degenerate range collapses to the start line. + got = ids(idx.enclosingForRange(25, 24)) + require.Equal(t, []string{"f.go::Second"}, got) +} + +func ids(nodes []*graph.Node) []string { + out := make([]string, 0, len(nodes)) + for _, n := range nodes { + out = append(out, n.ID) + } + return out +} + +func TestSymbolsForRangesHandler(t *testing.T) { + srv, g := setupNavServer(t) + + startNode := navFindMethod(t, g, "Start") + start := g.GetNode(startNode) + require.NotNil(t, start) + + req := mcplib.CallToolRequest{} + req.Params.Arguments = map[string]any{ + "path": "svc.go", + "start_line": float64(start.StartLine), + "end_line": float64(start.EndLine), + } + res, err := srv.handleSymbolsForRanges(context.Background(), req) + require.NoError(t, err) + require.False(t, res.IsError, "handler errored: %s", toolResultText(res)) + + var payload map[string]any + require.NoError(t, json.Unmarshal([]byte(res.Content[0].(mcplib.TextContent).Text), &payload)) + + syms, _ := payload["symbols"].([]any) + require.NotEmpty(t, syms, "Start's range should resolve to at least one symbol") + found := false + for _, s := range syms { + m := s.(map[string]any) + if m["name"] == "Start" { + found = true + } + } + require.True(t, found, "expected the Start method among resolved symbols, got %v", payload["symbols"]) +} diff --git a/internal/serverstack/shared_server.go b/internal/serverstack/shared_server.go index 6675ea856..a79e5450d 100644 --- a/internal/serverstack/shared_server.go +++ b/internal/serverstack/shared_server.go @@ -444,6 +444,7 @@ func NewSharedServer(cfg SharedServerConfig) (*SharedServer, error) { gortexmcp.Version = cfg.Version srv := gortexmcp.NewServer(eng, g, idx, nil, logger, conf.Guards.Rules, multiOpts...) srv.SetArchitecture(conf.Architecture) + srv.SetEventRules(conf.Events.Rules) srv.SetArtifacts(conf.Artifacts) srv.SetNamedQueries(conf.Queries) srv.SetSearchConfig(conf.Search)