Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/analysis/diffmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ func MapGitDiffContext(ctx context.Context, g graph.Reader, repoRoot, repoPrefix
return joinHunksToSymbolsContext(ctx, g, repoPrefix, hunks, files)
}

// JoinDiffSymbolsContext maps a file-only diff (MapGitDiffContext with a nil
// reader) to the symbols g indexes, without running git again. A caller that
// needs the changed files before it can pick its reader diffs once this way.
func JoinDiffSymbolsContext(ctx context.Context, g graph.Reader, repoPrefix string, diff *DiffResult) (*DiffResult, error) {
return joinHunksToSymbolsContext(ctx, g, repoPrefix, diff.Hunks, diff.FileChanges)
}

// PathDomain names the vocabulary a file path is spelled in. The two overlap,
// so the domain has to travel with the path rather than be recovered from it:
// in a repo whose tree carries a top-level directory named like the repo
Expand Down
132 changes: 116 additions & 16 deletions internal/hooks/posttask.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,25 +133,106 @@ type changeSet struct {
Scope string `json:"scope"`
Repo string `json:"repo"`
RepoRoot string `json:"repo_root"`
// The label of an answer whose relations lag: graph_freshness on a
// primary checkout, graph_status on a worktree.
GraphFreshness string `json:"graph_freshness"`
GraphStatus string `json:"graph_status"`
PendingFileCount int `json:"pending_file_count"`
}

// detectChangesBudget bounds the detect_changes round trip of a briefing. A
// daemon still publishing edits answers inside it with a labelled partial; a
// stalled one is reported rather than waited on.
const detectChangesBudget = 300 * time.Millisecond

// detectChangesWait is how far into the budget the briefing's wait_deadline
// lets the daemon wait for pending edits before it labels them. The rest of
// the budget is for the diff walk and the transport.
const detectChangesWait = detectChangesBudget / 2

// detectChangeSet asks the bridge for the current diff and decodes it.
// Returns nil when the bridge is unreachable or the payload is unusable.
func detectChangeSet(port int) *changeSet {
raw := callServerTool(port, "detect_changes", map[string]any{
// answered is false when the bridge did not reply within detectChangesBudget,
// or replied with nothing once detectChangesWait had passed; changes is nil
// when it is unreachable or the payload is unusable.
func detectChangeSet(port int) (changes *changeSet, answered bool) {
started := time.Now()
raw, answered := callServerToolWithin(port, "detect_changes", map[string]any{
"scope": postTaskDiffScope,
// The briefings never read by_depth; summary_only drops the
// largest part of the response.
"summary_only": true,
})
"summary_only": true,
"wait_deadline": started.Add(detectChangesWait).UTC().Format(time.RFC3339Nano),
}, detectChangesBudget)
if raw == "" {
return nil
// The daemon refuses a wait_deadline that passed before it took
// the call up: a busy daemon, not an unreachable one.
return nil, answered && time.Since(started) < detectChangesWait
}
var changes changeSet
if err := json.Unmarshal([]byte(raw), &changes); err != nil {
return nil
return nil, true
}
return &changes
return changes, true
}

// relationsLine is the label line of a change set whose relations lag, or ""
// when the graph has published every change it reports.
func (c *changeSet) relationsLine() string {
state := c.GraphFreshness
if state == "" && c.GraphStatus != "" && c.GraphStatus != "ready" {
state = c.GraphStatus
}
if state == "" {
return ""
}
files := c.PendingFileCount
if files == 0 {
files = len(c.ChangedFiles)
}
return fmt.Sprintf("relations: %s (%d files)", state, files)
}

// laggingChangesBriefing is the briefing for a change set the graph has not
// caught up with: the relations line and the changed files, with the
// symbol-level diagnostics left to a turn that can trust them. A stalled
// detect_changes (nil changes) is reported the same way instead of as
// silence, except to a session that recorded no writes: it has no change set
// to warn about. lagging is false when the change set is complete.
func laggingChangesBriefing(heading string, scope sessionScope, changes *changeSet) (briefing string, lagging bool) {
line := fmt.Sprintf("relations: pending (detect_changes did not answer within %v)", detectChangesBudget)
why := ""
var files []string
if changes == nil {
if scope.SessionID != "" && len(loadSessionState(scope.SessionID).WrittenPaths) == 0 {
return "", true
}
} else {
if line = changes.relationsLine(); line == "" {
return "", false
}
why = " the graph is still publishing edits, so"
files = changes.ChangedFiles
if attr := attributeChanges(scope, changes); attr.Attributed {
if len(attr.OwnedFiles) == 0 {
return "", true
}
files = attr.OwnedFiles
}
}
var sb strings.Builder
sb.WriteString(heading + "\n\n")
sb.WriteString(line + " —" + why + " symbol-level diagnostics are skipped; run detect_changes again before handoff.\n")
if len(files) > 0 {
sb.WriteString("\nChanged files:\n")
const show = 10
for i, f := range files {
if i == show {
fmt.Fprintf(&sb, "- (+%d more)\n", len(files)-show)
break
}
fmt.Fprintf(&sb, "- `%s`\n", f)
}
}
return sb.String(), true
}

// briefingScope names the working tree detect_changes actually diffed.
Expand Down Expand Up @@ -245,10 +326,13 @@ func renderUnattributedFiles(files []string, capped bool) string {
// named rather than dropped. When the session cannot be identified at all the
// briefing falls back to the whole tree and labels itself as such.
func buildPostTaskBriefing(port int, scope sessionScope) string {
changes := detectChangeSet(port)
if changes == nil {
changes, answered := detectChangeSet(port)
if answered && changes == nil {
return ""
}
if briefing, lagging := laggingChangesBriefing("## Gortex Working-Tree Diagnostics", scope, changes); lagging {
return briefing
}

if len(changes.ChangedSymbols) == 0 {
// No indexed symbols touched — skip silently. The agent doesn't need
Expand Down Expand Up @@ -365,8 +449,14 @@ func buildPostTaskBriefing(port int, scope sessionScope) string {
// so nothing is ever recorded for it and it always resolves to the labeled
// whole-tree fallback — this does not give Codex attribution coverage.
func buildMutationBriefing(port int, scope sessionScope) string {
changes := detectChangeSet(port)
if changes == nil || len(changes.ChangedSymbols) == 0 {
changes, answered := detectChangeSet(port)
if answered && changes == nil {
return ""
}
if briefing, lagging := laggingChangesBriefing("## Gortex mutation follow-up", scope, changes); lagging {
return briefing
}
if len(changes.ChangedSymbols) == 0 {
return ""
}

Expand Down Expand Up @@ -461,7 +551,9 @@ func renderGuardViolations(port int, idsCSV string) string {

// renderDeadCodeHits filters analyze:dead_code results to the intersection
// with the currently-changed symbols (i.e. "did this task leave anything
// orphaned"). Emits nothing when the intersection is empty.
// orphaned"). Emits nothing when the intersection is empty. A symbol listed
// on a "pending_unknown " line has unknown liveness while edits are pending
// and is never reported as dead.
func renderDeadCodeHits(port int, ids []string) string {
raw := callServerTool(port, "analyze", map[string]any{
"kind": "dead_code",
Expand All @@ -472,14 +564,22 @@ func renderDeadCodeHits(port int, ids []string) string {
return ""
}

lines := strings.Split(raw, "\n")
idSet := make(map[string]bool, len(ids))
for _, id := range ids {
idSet[id] = true
}
for _, line := range lines {
if rest, unknown := strings.CutPrefix(line, "pending_unknown "); unknown {
for _, field := range strings.Fields(rest) {
delete(idSet, field)
}
}
}

var hits []string
for _, line := range strings.Split(raw, "\n") {
if line == "" {
for _, line := range lines {
if line == "" || strings.HasPrefix(line, "pending_unknown ") {
continue
}
// Each compact line starts with the symbol descriptor including the
Expand Down
111 changes: 111 additions & 0 deletions internal/hooks/posttask_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ package hooks
import (
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"path/filepath"
"strings"
"testing"
"time"

"github.com/zzet/gortex/internal/daemon"
"github.com/zzet/gortex/internal/localizationauth"
Expand Down Expand Up @@ -189,6 +192,114 @@ func TestRunPostTask_DeadCodeFiltersToChanged(t *testing.T) {
if strings.Contains(out, "API Contract Issues") {
t.Errorf("contract section should be omitted when clean:\n%s", out)
}

// A changed symbol whose liveness is unknown while edits are pending is
// never reported as dead.
pending := newFakeServer(map[string]string{
"analyze": "relations: pending (1 files)\npending_unknown function foo.go::Foo foo.go:3\n",
})
defer pending.Close()
if hits := renderDeadCodeHits(portFromURL(t, pending.URL), []string{"foo.go::Foo"}); hits != "" {
t.Errorf("pending_unknown symbol reported as dead:\n%s", hits)
}
}

// While the graph is still publishing edits, detect_changes answers with a
// labelled partial. Both briefings must say so and name the changed files
// rather than render diagnostics over lagging relations or fall silent.
func TestPostTaskBriefingPrintsPendingRelations(t *testing.T) {
for name, detect := range map[string]string{
"primary": `{"changed_files":["a.go","b.go"],"changed_symbols":[{"id":"a.go::A","name":"A","kind":"function"}],
"risk":"LOW","complete":false,"graph_freshness":"pending","pending_file_count":2,"pending_files":["a.go","b.go"]}`,
"worktree": `{"changed_files":["a.go"],"changed_symbols":[],"risk":"UNKNOWN","complete":false,"graph_status":"pending"}`,
} {
srv := newRecordingFakeServer(map[string]string{"detect_changes": detect, "get_test_targets": "a_test.go TestA\n"})
port := portFromURL(t, srv.URL)
started := time.Now()
for builder, briefing := range map[string]string{
"stop": buildPostTaskBriefing(port, sessionScope{}),
"mutation": buildMutationBriefing(port, sessionScope{}),
} {
want := "relations: pending (2 files)"
if name == "worktree" {
want = "relations: pending (1 files)"
}
if !strings.Contains(briefing, want) || !strings.Contains(briefing, "`a.go`") {
t.Errorf("%s %s briefing missing %q and the changed files:\n%s", name, builder, want, briefing)
}
}
if n := srv.callCount("get_test_targets"); n != 0 {
t.Errorf("%s: diagnostics ran over lagging relations (%d get_test_targets calls)", name, n)
}
// The daemon waits for pending edits until wait_deadline and labels
// the rest. Without one it waits past the budget, and the labelled
// answer arrives after the briefing has given up on it.
for _, args := range srv.argsFor("detect_changes") {
deadline, err := time.Parse(time.RFC3339Nano, fmt.Sprint(args["wait_deadline"]))
if err != nil || !deadline.After(started) || !deadline.Before(started.Add(detectChangesBudget)) {
t.Errorf("%s: detect_changes wait_deadline %v does not end inside the %v budget", name, args["wait_deadline"], detectChangesBudget)
}
}
srv.Close()
}
}

// A detect_changes that does not answer within its budget is reported, and
// the briefing returns well inside the half-second hook budget. So is one the
// daemon reached only after its wait_deadline had passed, which it refuses.
// A session that recorded no writes is not told about a slow daemon.
func TestPostTaskBriefingBoundedWhenDetectStalls(t *testing.T) {
t.Setenv(hookSessionDirEnvVar, t.TempDir())
saveSessionState("writer", sessionState{WrittenPaths: []string{"a.go"}})
release := make(chan struct{})
stalled := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
select {
case <-release:
case <-time.After(5 * time.Second):
}
http.NotFound(w, r)
}))
defer stalled.Close()
defer close(release)
late := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req struct {
Arguments map[string]any `json:"arguments"`
}
_ = json.NewDecoder(r.Body).Decode(&req)
if deadline, err := time.Parse(time.RFC3339Nano, fmt.Sprint(req.Arguments["wait_deadline"])); err == nil {
time.Sleep(time.Until(deadline) + 10*time.Millisecond)
}
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"content":[{"type":"text","text":"wait_deadline is not in the future"}],"isError":true}`))
}))
defer late.Close()

for name, srv := range map[string]*httptest.Server{"stalled": stalled, "refused late": late} {
port := portFromURL(t, srv.URL)
for builder, build := range map[string]func(int, sessionScope) string{
"stop": buildPostTaskBriefing,
"mutation": buildMutationBriefing,
} {
for session, report := range map[string]bool{"": true, "writer": true, "reader": false} {
if name != "stalled" && session != "" {
continue
}
started := time.Now()
briefing := build(port, sessionScope{SessionID: session})
if elapsed := time.Since(started); elapsed > 500*time.Millisecond {
t.Errorf("%s %s briefing took %v", name, builder, elapsed)
}
switch {
case !report && briefing != "":
t.Errorf("%s %s briefing warned session %q, which wrote nothing:\n%q", name, builder, session, briefing)
case report && !strings.Contains(briefing, "relations: pending"):
t.Errorf("%s %s briefing did not report the busy detect_changes to session %q:\n%q", name, builder, session, briefing)
case report && strings.Contains(briefing, "publishing"):
t.Errorf("%s %s briefing claims edits are publishing without an answer:\n%q", name, builder, briefing)
}
}
}
}
}

// contractsCompact builds a realistic `contracts check --compact` payload:
Expand Down
17 changes: 17 additions & 0 deletions internal/hooks/precompact.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,23 @@ func callServerTool(port int, name string, args map[string]any) string {
return callServerToolHTTP(port, name, args)
}

// callServerToolWithin is callServerTool bounded by timeout. answered is false
// when the call did not return in time: the bridge is reachable but busy,
// which a briefing reports instead of folding it into the silence of an
// unreachable bridge. The abandoned call ends with the hook process.
func callServerToolWithin(port int, name string, args map[string]any, timeout time.Duration) (raw string, answered bool) {
done := make(chan string, 1)
go func() { done <- callServerTool(port, name, args) }()
timer := time.NewTimer(timeout)
defer timer.Stop()
select {
case raw = <-done:
return raw, true
case <-timer.C:
return "", false
}
}

// callServerToolHTTP issues a POST /v1/tools/{name} against the server and
// returns the text content from the first response block. Returns empty string
// on any error (server unreachable, non-200, missing content).
Expand Down
5 changes: 5 additions & 0 deletions internal/mcp/gcx.go
Original file line number Diff line number Diff line change
Expand Up @@ -1937,8 +1937,13 @@ func encodeChangeImpact(result map[string]any) ([]byte, error) {
communityNote, _ := result["community_note"].(string)
contractRiskUpgrade, _ := result["contract_risk_upgrade"].(string)

var meta []string
if waited, ok := result["barrier_wait_ms"].(int64); ok {
meta = append(meta, "barrier_wait_ms", strconv.FormatInt(waited, 10))
}
sumEnc := newGCX(&buf, "explain_change_impact.summary",
[]string{"risk", "summary", "total_affected", "cross_repo", "processes", "communities", "test_files", "warning", "note", "risk_upgrade"},
meta...,
)
if err := sumEnc.WriteRow(
string(risk),
Expand Down
Loading
Loading