doctor: fix false blockers on mixed attribution, make --redact work, quiet the noise

cmd/gortex/doctor.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 
 	"github.com/spf13/cobra"
 
@@ -81,13 +82,15 @@ var (
 	doctorJSON   bool
 	doctorDays   int
 	doctorRedact bool
+	doctorAll    bool
 )
 
 func init() {
 	for _, c := range []*cobra.Command{doctorCmd, initDoctorCmd} {
 		c.Flags().BoolVar(&doctorJSON, "json", false, "emit a structured JSON report on stdout")
 		c.Flags().IntVar(&doctorDays, "days", 7, "look-back window for hook activity, adoption, and savings")
-		c.Flags().BoolVar(&doctorRedact, "redact", false, "hash repo paths and branch names so the report can be shared")
+		c.Flags().BoolVar(&doctorRedact, "redact", false, "rewrite repo and home paths, and hash branch names, so the report can be shared")
+		c.Flags().BoolVar(&doctorAll, "all", false, "list planned files for adapters that are not installed too")
 	}
 	silenceDoctorErrors(doctorCmd, initDoctorCmd)
 	rootCmd.AddCommand(doctorCmd)
@@ -156,6 +159,13 @@ func runDoctor(cmd *cobra.Command, _ []string) error {
 		Stderr:       nil, // suppress progress lines; doctor is read-only
 	}
 
+	if doctorRedact {
+		// Install the rewriter before anything renders, so every section —
+		// static, runtime, and the paths quoted inside findings — agrees on
+		// what a given repo is called.
+		doctorPath = newDoctorPathRedactor(home, root)
+	}
+
 	doctorEnv := doctorEnvironment()
 
 	registry := buildRegistry()
@@ -310,7 +320,7 @@ func inspectAdapter(a agents.Adapter, env agents.Env) DoctorAgentReport {
 func printDoctorEnvironment(w io.Writer, env DoctorEnvironment) {
 	fmt.Fprintln(w, "Gortex doctor — environment:")
 	if env.BinaryOnPath {
-		fmt.Fprintf(w, "  %s gortex on PATH: %s\n", glyphCheck, env.BinaryPath)
+		fmt.Fprintf(w, "  %s gortex on PATH: %s\n", glyphCheck, doctorPath(env.BinaryPath))
 	} else {
 		fmt.Fprintf(w, "  %s gortex not found on PATH (%s)\n", glyphCross, env.BinaryError)
 	}
@@ -319,27 +329,49 @@ func printDoctorEnvironment(w io.Writer, env DoctorEnvironment) {
 		if ver == "" {
 			ver = "ok"
 		}
-		fmt.Fprintf(w, "  %s daemon handshake: %s (%s)\n", glyphCheck, ver, env.DaemonSocket)
+		fmt.Fprintf(w, "  %s daemon handshake: %s (%s)\n", glyphCheck, ver, doctorPath(env.DaemonSocket))
 	} else {
 		fmt.Fprintf(w, "  %s daemon handshake: %s\n", glyphCross, env.DaemonError)
 	}
 	fmt.Fprintln(w)
 }
 
-// glyphCheck / glyphCross are the doctor's status markers.
+// The doctor's status markers. The distinction between glyphAbsent and
+// glyphCross is the whole point of having both: a per-repo file that `gortex
+// init` has not written is not a fault — a machine-wide `gortex install`
+// covers the agent, and most repos never need repo-local config at all.
+// Marking it ✗ turned an optional file into an accusation and implied work
+// the user does not have to do. ✗ is reserved for a real gap: something
+// broken, outdated, or genuinely not wired up.
 const (
-	glyphCheck = "✓"
-	glyphCross = "✗"
+	glyphCheck  = "✓"
+	glyphCross  = "✗"
+	glyphWarn   = "!"
+	glyphAbsent = "·"
 )
 
 // printDoctorHuman renders the human-readable summary. One row per
 // agent, with a nested file list. Columns line up for copy-paste
 // into issue reports.
+// printDoctorHuman renders the adapter section. Adapters that are neither
+// installed nor carrying Gortex files collapse to a name list: their planned
+// paths describe writes that will never happen, and one uninstalled agent can
+// contribute twenty of them — burying the handful of lines that describe the
+// machine the user actually has. An absent agent that still holds Gortex files
+// is kept and called out instead: that is leftover config, which is a finding.
+// --all restores the full listing, and --json is always complete.
 func printDoctorHuman(w io.Writer, reports []DoctorAgentReport) {
-	fmt.Fprintln(w, "Gortex doctor — observed state of every adapter's planned files:")
+	fmt.Fprintln(w, "Gortex doctor — observed state of every detected adapter:")
+	fmt.Fprintf(w, "  %s present   %s absent (optional — repo-local config)   %s needs attention\n",
+		glyphCheck, glyphAbsent, glyphWarn)
 	fmt.Fprintln(w)
 
+	var absent []string
 	for _, r := range reports {
+		if !doctorAll && !r.Detected && !r.Configured {
+			absent = append(absent, r.Name)
+			continue
+		}
 		detMark := "–"
 		if r.Detected {
 			detMark = "✓"
@@ -348,24 +380,29 @@ func printDoctorHuman(w io.Writer, reports []DoctorAgentReport) {
 		if r.Configured {
 			cfgMark = "✓"
 		}
-		fmt.Fprintf(w, "  [%s detected] [%s any-file-present]  %s\n", detMark, cfgMark, r.Name)
+		fmt.Fprintf(w, "  [%s installed] [%s gortex files]  %s\n", detMark, cfgMark, r.Name)
 		for _, f := range r.Files {
 			statusSym := "?"
 			switch f.Status {
 			case "present":
-				statusSym = "✓"
+				statusSym = glyphCheck
 			case "missing":
-				statusSym = "✗"
+				// Absent, not wrong: repo-local config is optional on a
+				// machine configured by `gortex install`.
+				statusSym = glyphAbsent
 			case "unreadable":
-				statusSym = "!"
+				statusSym = glyphWarn
 			}
 			extra := ""
 			if f.Status == "present" && f.ByteSize > 0 {
 				extra = fmt.Sprintf(" (%d bytes)", f.ByteSize)
 			}
 			switch f.StanzaStatus {
 			case "stale":
-				extra += " [stale stanza: run `gortex install` to migrate]"
+				// Present but outdated is a real gap — Gortex wrote this and
+				// the shape has since moved on.
+				statusSym = glyphWarn
+				extra += " [outdated stanza: `gortex upgrade --run` migrates it, or `gortex install`]"
 			case "current":
 				extra += " [stanza current]"
 			}
@@ -376,13 +413,22 @@ func printDoctorHuman(w io.Writer, reports []DoctorAgentReport) {
 				if len(verb) > len("would-") && verb[:len("would-")] == "would-" {
 					verb = verb[len("would-"):]
 				}
-				extra = fmt.Sprintf(" (init would %s)", verb)
+				extra = fmt.Sprintf(" (optional — `gortex init` would %s)", verb)
 			}
-			fmt.Fprintf(w, "      %s %s%s\n", statusSym, f.Path, extra)
+			fmt.Fprintf(w, "      %s %s%s\n", statusSym, doctorPath(f.Path), extra)
+		}
+		if !r.Detected && r.Configured {
+			fmt.Fprintln(w, "      note: not installed, but Gortex files are present — leftover config.")
 		}
 		if r.DocsURL != "" {
 			fmt.Fprintf(w, "      docs: %s\n", r.DocsURL)
 		}
 		fmt.Fprintln(w)
 	}
+
+	if len(absent) > 0 {
+		fmt.Fprintf(w, "  not installed (%d): %s\n", len(absent), strings.Join(absent, ", "))
+		fmt.Fprintln(w, "  nothing is written for these; --all lists what init would write if they were.")
+		fmt.Fprintln(w)
+	}
 }

cmd/gortex/doctor_redact.go

@@ -0,0 +1,53 @@
+package main
+
+import (
+	"path/filepath"
+	"strings"
+)
+
+// doctor_redact.go makes --redact mean something. The report is built to be
+// pasted into an issue, and almost everything identifying in it is a path:
+// the repo name in every planned-file line, the account name in every home
+// path. Hashing the two roots keeps the structure legible — `<repo>/.mcp.json`
+// still reads as a config path — while removing the parts a user would
+// otherwise blur out by hand before sharing.
+
+// doctorPath rewrites one path for display. Replaced for the duration of a
+// redacted run; the identity default keeps the normal report verbatim.
+var doctorPath = func(p string) string { return p }
+
+// newDoctorPathRedactor rewrites paths under the repo root to <repo>/… and
+// paths under the home directory to ~/…. Repo first: the repo usually lives
+// under home, and its name is the more sensitive of the two.
+func newDoctorPathRedactor(home, root string) func(string) string {
+	return func(p string) string {
+		if strings.TrimSpace(p) == "" {
+			return p
+		}
+		clean := filepath.Clean(p)
+		if rel, ok := relativeTo(root, clean); ok {
+			return filepath.Join("<repo>", rel)
+		}
+		if rel, ok := relativeTo(home, clean); ok {
+			return filepath.Join("~", rel)
+		}
+		// Outside both roots: no name of ours to leak, so leave it readable.
+		return clean
+	}
+}
+
+// relativeTo reports base-relative form when path is inside base. "." maps to
+// the empty string so the caller's Join yields the bare placeholder.
+func relativeTo(base, path string) (string, bool) {
+	if strings.TrimSpace(base) == "" {
+		return "", false
+	}
+	rel, err := filepath.Rel(filepath.Clean(base), path)
+	if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
+		return "", false
+	}
+	if rel == "." {
+		return "", true
+	}
+	return rel, true
+}

cmd/gortex/doctor_runtime.go

@@ -66,6 +66,11 @@ func collectRuntime(home string, days int) doctorRuntime {
 	since := now.Add(-time.Duration(days) * 24 * time.Hour)
 
 	state := codex.Inspect(home)
+	if doctorRedact {
+		state.ConfigPath = doctorPath(state.ConfigPath)
+		state.InstructionsPath = doctorPath(state.InstructionsPath)
+		state.ShadowedBy = doctorPath(state.ShadowedBy)
+	}
 	activity, err := hooks.ReadEffectiveness(since)
 	if err != nil {
 		// A log we cannot read is not fatal; the other sections still carry
@@ -95,6 +100,7 @@ func collectRuntime(home string, days int) doctorRuntime {
 
 	if doctorRedact {
 		out.Adoption = redactAdoption(out.Adoption)
+		out.Hooks.Path = doctorPath(out.Hooks.Path)
 	}
 	return out
 }
@@ -147,20 +153,22 @@ func collectSavings(since time.Time) doctorSavings {
 // into an issue. Tool names, counts, and timings are never sensitive and are
 // always left intact — redacting them would defeat the point of sharing it.
 func redactAdoption(a doctor.Adoption) doctor.Adoption {
-	a.Root = redactPath(a.Root)
+	a.Root = doctorPath(a.Root)
 	for i := range a.Sessions {
-		a.Sessions[i].CWD = redactPath(a.Sessions[i].CWD)
-		a.Sessions[i].Branch = redactPath(a.Sessions[i].Branch)
+		a.Sessions[i].CWD = doctorPath(a.Sessions[i].CWD)
+		a.Sessions[i].Branch = redactName(a.Sessions[i].Branch)
 	}
 	return a
 }
 
-func redactPath(value string) string {
+// redactName hashes a value that is not a path — a branch name can carry a
+// ticket id or a customer name just as readily as a repo path can.
+func redactName(value string) string {
 	if value == "" {
 		return ""
 	}
 	sum := sha256.Sum256([]byte(value))
-	return "…/<" + hex.EncodeToString(sum[:])[:8] + ">"
+	return "<" + hex.EncodeToString(sum[:])[:8] + ">"
 }
 
 func printDoctorRuntime(w io.Writer, r doctorRuntime) {
@@ -186,29 +194,42 @@ func printDoctorRuntime(w io.Writer, r doctorRuntime) {
 		fmt.Fprintf(w, "    no %s — no hook has ever run on this machine\n", r.Hooks.Path)
 	} else {
 		scoped := r.Hooks.ForAgent(codex.Name)
-		fmt.Fprintf(w, "    %-18s %7s %9s %10s  %s\n", "event", "runs", "injected", "daemon-up", "last seen")
+		fmt.Fprintf(w, "    %-18s %7s %9s %10s %8s  %s\n", "event", "runs", "injected", "daemon-up", "unattrib", "last seen")
 		for _, event := range doctorEventOrder(r.Hooks) {
 			stats := scoped[event]
 			daemon := "n/a"
 			if stats.DaemonKnown > 0 {
 				daemon = fmt.Sprintf("%d/%d", stats.DaemonUp, stats.DaemonKnown)
 			}
+			// A zero in `runs` is only alarming when `unattrib` is zero too;
+			// showing them side by side is what keeps the reader from
+			// reaching the conclusion the findings deliberately withheld.
+			ambiguous := r.Hooks.AmbiguousRuns(event)
+			seen := stats.LastSeen
+			if seen.IsZero() {
+				// The event did run, we just cannot say for whom — dating it
+				// "never" would contradict the count beside it.
+				seen = r.Hooks.Unattributed[event].LastSeen
+			}
 			last := "never"
-			if !stats.LastSeen.IsZero() {
-				last = stats.LastSeen.Format(time.RFC3339)[:19]
+			if !seen.IsZero() {
+				last = seen.Format(time.RFC3339)[:19]
 			}
-			fmt.Fprintf(w, "    %-18s %7d %9d %10s  %s\n", event, stats.Runs, stats.Emitted, daemon, last)
+			fmt.Fprintf(w, "    %-18s %7d %9d %10s %8d  %s\n", event, stats.Runs, stats.Emitted, daemon, ambiguous, last)
 		}
 		fmt.Fprintf(w, "    %d row(s) in window, %d in the whole log\n", r.Hooks.WindowRows, r.Hooks.TotalRows)
 		if agents := r.Hooks.AgentNames(); len(agents) > 0 {
 			fmt.Fprintf(w, "    agents seen                %s\n", joinComma(agents))
 		}
 		if r.Hooks.UnattributedRows > 0 {
-			// Rows written before the agent field existed cannot be assigned,
-			// so say how much of the evidence is shared rather than let the
-			// table imply a precision it does not have.
-			fmt.Fprintf(w, "    note: %d row(s) predate per-agent attribution and are counted for\n", r.Hooks.UnattributedRows)
-			fmt.Fprintln(w, "          every agent; they clear as the window rolls forward.")
+			// Rows written before the agent field existed cannot be assigned
+			// to anyone. They are held out of every agent's counts rather
+			// than shared into all of them, and a `runs` of 0 beside a
+			// non-zero `unattrib` means "cannot tell", not "did not run".
+			fmt.Fprintf(w, "    note: %d row(s) predate per-agent attribution (the unattrib column).\n", r.Hooks.UnattributedRows)
+			fmt.Fprintln(w, "          They can neither confirm nor rule out an agent's hooks, so a 0 in")
+			fmt.Fprintln(w, "          runs beside a non-zero unattrib is withheld from the findings.")
+			fmt.Fprintln(w, "          They clear as the window rolls past the upgrade.")
 		}
 	}
 	fmt.Fprintln(w)