Fix Go recovery, gate routing, evidence, and handoff defects

cli/cmd/ao/git_read.go

@@ -33,29 +33,54 @@ func gitDiscoveryEnv() []string {
 	return env
 }
 
-// gitChangedFiles lists worktree-modified paths (read-only, bounded) for
-// handoff evidence. Returns nil when git is unavailable or the tree is clean.
-func gitChangedFiles(cwd string, limit int) []string {
+// gitChangedFiles includes tracked and untracked work for handoff evidence.
+// A failed observation is distinct from a successfully observed clean tree.
+func gitChangedFiles(cwd string, limit int) ([]string, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), 1200*time.Millisecond)
 	defer cancel()
-	command := exec.CommandContext(ctx, "git", "diff", "--name-only", "HEAD")
+	command := exec.CommandContext(ctx, "git", "status", "--porcelain=v1", "-z", "--untracked-files=all")
 	command.Dir = cwd
 	command.Env = gitDiscoveryEnv()
 	out, err := command.Output()
-	if err != nil || strings.TrimSpace(string(out)) == "" {
-		return nil
+	if err != nil {
+		return nil, fmt.Errorf("observe Git status: %w", err)
+	}
+	files, err := parseGitStatus(string(out))
+	if err != nil {
+		return nil, err
+	}
+	if limit > 0 && len(files) > limit {
+		files = files[:limit]
 	}
-	lines := strings.Split(strings.TrimSpace(string(out)), "\n")
-	if limit > 0 && len(lines) > limit {
-		lines = lines[:limit]
+	return files, nil
+}
+
+func parseGitStatus(raw string) ([]string, error) {
+	if raw == "" {
+		return nil, nil
 	}
-	result := make([]string, 0, len(lines))
-	for _, line := range lines {
-		if line = strings.TrimSpace(line); line != "" {
-			result = append(result, line)
+	if !strings.HasSuffix(raw, "\x00") {
+		return nil, fmt.Errorf("unterminated Git status record")
+	}
+	records := strings.Split(strings.TrimSuffix(raw, "\x00"), "\x00")
+	var paths []string
+	for i := 0; i < len(records); i++ {
+		record := records[i]
+		if len(record) < 4 || record[2] != ' ' {
+			return nil, fmt.Errorf("invalid Git status record")
+		}
+		paths = append(paths, record[3:])
+		// Porcelain -z emits a rename/copy destination followed by the
+		// original path as a separate NUL-delimited field without a status.
+		if strings.ContainsAny(record[:2], "RC") {
+			i++
+			if i == len(records) || records[i] == "" {
+				return nil, fmt.Errorf("missing Git rename/copy source")
+			}
+			paths = append(paths, records[i])
 		}
 	}
-	return result
+	return paths, nil
 }
 
 // resolveRepoRoot is read-only discovery. AgentOps does not mutate Git state.

cli/cmd/ao/handoff.go

@@ -103,11 +103,17 @@ func runHandoff(cmd *cobra.Command, args []string) error {
 }
 
 func collectHandoffState(cwd string) *handoffState {
+	files, err := gitChangedFiles(cwd, 20)
+	if err != nil {
+		// State is optional: absence means unavailable, whereas an emitted
+		// git_dirty=false must represent an actual successful observation.
+		return nil
+	}
 	state := &handoffState{}
 	if branch, err := getCurrentBranch(cwd); err == nil {
 		state.GitBranch = branch
 	}
-	state.ModifiedFiles = gitChangedFiles(cwd, 20)
+	state.ModifiedFiles = files
 	state.GitDirty = len(state.ModifiedFiles) > 0
 	command := exec.Command("git", "log", "--oneline", "-5", "--no-decorate")
 	command.Dir = cwd

cli/cmd/ao/handoff_git_test.go

@@ -0,0 +1,82 @@
+package main
+
+import (
+	"encoding/json"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"slices"
+	"testing"
+)
+
+func TestHandoffCollectIncludesUntrackedNames(t *testing.T) {
+	dir := t.TempDir()
+	run := func(args ...string) {
+		t.Helper()
+		cmd := exec.Command("git", args...)
+		cmd.Dir, cmd.Env = dir, gitDiscoveryEnv()
+		if output, err := cmd.CombinedOutput(); err != nil {
+			t.Fatalf("git %v: %v\n%s", args, err, output)
+		}
+	}
+	run("init", "-q")
+	run("-c", "user.name=Test", "-c", "user.email=test@example.invalid", "commit", "--allow-empty", "-qm", "baseline")
+	clean := collectHandoffState(dir)
+	if clean == nil || clean.GitDirty {
+		t.Fatalf("clean repository state = %+v", clean)
+	}
+	names := []string{"new-work.go", "café.go"}
+	if runtime.GOOS != "windows" {
+		names = append(names, " spaced\n.go ")
+	}
+	for _, name := range names {
+		if err := os.WriteFile(filepath.Join(dir, name), []byte("package work\n"), 0o600); err != nil {
+			t.Fatal(err)
+		}
+	}
+	state := collectHandoffState(dir)
+	if state == nil || !state.GitDirty {
+		t.Fatalf("untracked work reported clean: %+v", state)
+	}
+	for _, name := range names {
+		if !slices.Contains(state.ModifiedFiles, name) {
+			t.Errorf("collected paths %q omit exact filename %q", state.ModifiedFiles, name)
+		}
+	}
+	data, err := json.Marshal(handoffArtifact{SchemaVersion: 1, ID: "handoff-20260909T120000Z", CreatedAt: "2026-09-09T12:00:00Z", State: state})
+	if err != nil {
+		t.Fatal(err)
+	}
+	var instance any
+	if err := json.Unmarshal(data, &instance); err != nil {
+		t.Fatal(err)
+	}
+	if err := compileHandoffSchema(t).Validate(instance); err != nil {
+		t.Fatalf("collected state violates the handoff schema: %v", err)
+	}
+	files, err := gitChangedFiles(dir, 1)
+	if err != nil || len(files) != 1 {
+		t.Fatalf("bounded collection = %q, %v", files, err)
+	}
+}
+
+func TestHandoffCollectUnavailableIsNotClean(t *testing.T) {
+	if state := collectHandoffState(t.TempDir()); state != nil {
+		t.Fatalf("non-repository produced a supposedly known Git state: %+v", state)
+	}
+}
+
+func TestParseGitStatusPreservesRenameAndCopyPaths(t *testing.T) {
+	raw := "R  new\nname\x00 old name \x00C  café.go\x00source.go\x00?? untracked.go\x00"
+	got, err := parseGitStatus(raw)
+	want := []string{"new\nname", " old name ", "café.go", "source.go", "untracked.go"}
+	if err != nil || !slices.Equal(got, want) {
+		t.Fatalf("paths = %q, %v; want %q", got, err, want)
+	}
+	for _, invalid := range []string{"?? unterminated", "x\x00", "??\x00", "R  target\x00", "C  target\x00\x00"} {
+		if _, err := parseGitStatus(invalid); err == nil {
+			t.Errorf("accepted incomplete status %q", invalid)
+		}
+	}
+}

cli/internal/doctor/doctor_test.go

@@ -74,7 +74,7 @@ func TestMutate_RoundTrip(t *testing.T) {
 		t.Fatalf("file content = %q, want %q", got, newContent)
 	}
 	// Backup exists and is byte-identical to original.
-	backup := filepath.Join(ra.BackupsDir(), ".agents", "ao", "thing.txt")
+	backup := recordedBackup(t, ra, 0)
 	bgot, err := os.ReadFile(backup)
 	if err != nil {
 		t.Fatalf("backup missing: %v", err)

cli/internal/doctor/engine.go

@@ -481,11 +481,13 @@ func applyFixers(repoRoot string, mctx *MutateContext, env *DetectEnv, findings
 		res, err := fx.Fix(mctx.WithFixer(fixerID), env, fs)
 		actions += res.ActionsTaken
 		skipped = append(skipped, res.Skipped...)
-		if err != nil || !res.Fixed {
+		if err != nil || res.Err != nil {
 			failed = true
 			continue
 		}
-		fixed += len(fs)
+		if res.Fixed {
+			fixed += len(fs)
+		}
 	}
 	return actions, fixed, failed, skipped
 }
@@ -680,8 +682,8 @@ type UndoResult struct {
 }
 
 // Undo reads a run's actions.jsonl in reverse and restores each mutated file
-// from backups/. Under strict mode (default) it fails if a backup is missing
-// or the restored hash does not match the recorded before_hash.
+// from backups/. All required backups are validated before any restoration.
+// Under strict mode (default), missing backups and rename conflicts also fail.
 func Undo(repoRoot, runID string, strict, dryRun bool) (*UndoResult, error) {
 	runDir, err := resolveRunDir(repoRoot, runID)
 	if err != nil {
@@ -692,9 +694,15 @@ func Undo(repoRoot, runID string, strict, dryRun bool) (*UndoResult, error) {
 		return &UndoResult{RunID: runID, ExitCode: ExitFixFailed}, err
 	}
 	res := &UndoResult{RunID: filepath.Base(runDir), ExitCode: ExitHealthy}
-	for i := len(records) - 1; i >= 0; i-- {
-		rec := records[i]
-		if err := undoOne(repoRoot, runDir, rec, strict, dryRun, res); err != nil {
+	prepared, err := prepareUndo(runDir, records, strict)
+	if err != nil {
+		res.ExitCode = ExitFixFailed
+		res.StrictError = err.Error()
+		return res, err
+	}
+	locks := NewLockManager(filepath.Join(repoRoot, ".doctor", "locks"))
+	for i := len(prepared) - 1; i >= 0; i-- {
+		if err := undoOne(repoRoot, prepared[i], locks, strict, dryRun, res); err != nil {
 			res.ExitCode = ExitFixFailed
 			res.StrictError = err.Error()
 			return res, err
@@ -703,59 +711,6 @@ func Undo(repoRoot, runID string, strict, dryRun bool) (*UndoResult, error) {
 	return res, nil
 }
 
-// undoOne reverses a single action record.
-func undoOne(repoRoot, runDir string, rec ActionRecord, strict, dryRun bool, res *UndoResult) error {
-	target := filepath.Join(repoRoot, rec.Path)
-	if rec.Op == "Rename" && rec.RenameTo != "" {
-		// Reverse the move: rename the quarantined file back.
-		if dryRun {
-			fmt.Fprintf(os.Stderr, "[dry-run] would restore (un-rename) %s\n", target)
-			res.Skipped++
-			return nil
-		}
-		if err := os.Rename(rec.RenameTo, target); err != nil {
-			if strict {
-				return fmt.Errorf("doctor: un-rename %s: %w", target, err)
-			}
-			res.Skipped++
-			return nil
-		}
-		res.Restored++
-		return nil
-	}
-	backup := filepath.Join(runDir, "backups", rec.Path)
-	if _, err := os.Stat(backup); err != nil {
-		if !rec.Existed {
-			// The file did not exist before; undo leaves it (created files are
-			// the user's to inspect; we never delete).
-			res.Skipped++
-			return nil
-		}
-		if strict {
-			return fmt.Errorf("doctor: missing backup for %s", rec.Path)
-		}
-		res.Skipped++
-		return nil
-	}
-	if dryRun {
-		fmt.Fprintf(os.Stderr, "[dry-run] would restore %s from backup\n", target)
-		res.Skipped++
-		return nil
-	}
-	if err := copyVerbatim(backup, target); err != nil {
-		return fmt.Errorf("doctor: restore %s: %w", target, err)
-	}
-	restored, err := os.ReadFile(target)
-	if err != nil {
-		return fmt.Errorf("doctor: read restored %s: %w", target, err)
-	}
-	if strict && sha256Hex(restored) != rec.BeforeHash {
-		return fmt.Errorf("doctor: restored hash mismatch for %s", rec.Path)
-	}
-	res.Restored++
-	return nil
-}
-
 // readActions reads and parses a run's actions.jsonl.
 func readActions(path string) ([]ActionRecord, error) {
 	f, err := os.Open(path)

cli/internal/doctor/engine_test.go

@@ -1,11 +1,37 @@
 package doctor
 
 import (
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
 )
 
+func TestFixHygieneReportsUnresolvedFindings(t *testing.T) {
+	for _, linked := range []bool{false, true} {
+		t.Run(map[bool]string{false: "manual-only", true: "mixed"}[linked], func(t *testing.T) {
+			repo, home := t.TempDir(), t.TempDir()
+			writeSkillsFile(t, filepath.Join(repo, "skills", "sample", "SKILL.md"), "---\nname: sample\ndescription: sample\n---\nBody.\n")
+			wantActions := 0
+			if linked {
+				writeSkillsFile(t, filepath.Join(repo, "skills", "sample", "references", "detail.md"), "detail")
+				wantActions = 1
+			}
+			report, err := Fix(Options{RepoRoot: repo, CWD: repo, HomeDir: home, Only: []string{"fm-skills-integrity-hygiene"}})
+			if err != nil {
+				t.Fatal(err)
+			}
+			if report.OK || report.ExitCode != ExitFixPartial || report.ActionsTaken != wantActions {
+				t.Fatalf("Fix=%+v, want unresolved partial result with %d actions", report, wantActions)
+			}
+			post, err := skillsIntegrityHygieneDetector{}.Detect(&DetectEnv{RepoRoot: repo, HomeDir: home})
+			if err != nil || len(post) != 1 {
+				t.Fatalf("Detect=%+v err=%v, want manual finding retained", post, err)
+			}
+		})
+	}
+}
+
 // TestHealthLineReportsEverySeverityBucket guards novice edge 4b: the health
 // one-liner once printed only P0 and P2, silently dropping P1 — the worst
 // severity actually present. A persisted run whose only finding is the P1

cli/internal/doctor/fix_bridges_test.go

@@ -318,7 +318,7 @@ func TestBridgesSnapshotTornLatestFix(t *testing.T) {
 	}
 
 	// Backup of the torn latest.json exists and matches the pre-fix torn bytes.
-	backup := filepath.Join(ra.RunDir, "backups", openclaw.SnapshotDirRel, "latest.json")
+	backup := recordedBackup(t, ra, 0)
 	backupBytes, err := os.ReadFile(backup)
 	if err != nil {
 		t.Fatalf("torn-latest backup missing: %v", err)

cli/internal/doctor/fix_knowledge_test.go

@@ -99,7 +99,7 @@ func TestKnowledgeCorruptIndexLines_DetectFixUndo(t *testing.T) {
 		t.Fatalf("post-fix index = %q, want %q", got, want)
 	}
 	// Backup is byte-identical to the corrupt original.
-	backup := filepath.Join(ra.BackupsDir(), ".agents", "ao", "index", "search-index.jsonl")
+	backup := recordedBackup(t, ra, 0)
 	bgot, err := os.ReadFile(backup)
 	if err != nil {
 		t.Fatalf("backup missing: %v", err)
@@ -189,7 +189,7 @@ func TestKnowledgeTornAppendLine_DetectFixUndo(t *testing.T) {
 		t.Fatalf("post-fix index = %q, want %q", got, want)
 	}
 	// Backup byte-identical to torn original.
-	backup := filepath.Join(ra.BackupsDir(), ".agents", "ao", "index", "search-index.jsonl")
+	backup := recordedBackup(t, ra, 0)
 	bgot, err := os.ReadFile(backup)
 	if err != nil {
 		t.Fatalf("backup missing: %v", err)

cli/internal/doctor/fix_skills.go

@@ -1093,7 +1093,7 @@ func (d skillsIntegrityHygieneDetector) Detect(env *DetectEnv) ([]Finding, error
 			File:  "skills",
 			Query: "hygiene: " + strings.Join(kinds, ", "),
 		},
-		Remediation: remediation(d.ID(), true, unlinked),
+		Remediation: remediation(d.ID(), unlinked > 0, unlinked),
 	}}, nil
 }
 
@@ -1166,9 +1166,8 @@ func (f skillsIntegrityHygieneFixer) Fix(ctx *MutateContext, env *DetectEnv, _ [
 		}
 	}
 	if len(bySkill) == 0 {
-		// Nothing safely fixable; report-only findings remain. A successful
-		// run with nothing to fix, not a refusal.
-		res.Fixed = true
+		// The scan succeeded, but the report-only findings remain unresolved.
+		res.Fixed = false
 		return res, nil
 	}
 	// 3/4. Append a references block to each affected SKILL.md.
@@ -1213,13 +1212,19 @@ func (f skillsIntegrityHygieneFixer) Fix(ctx *MutateContext, env *DetectEnv, _ [
 	}
 	// 5. Verify: no UNLINKED finding may remain. Report-only findings may.
 	if !ctx.DryRun {
-		post, _, _ := scanSkillHygiene(env.RepoRoot)
+		post, _, err := scanSkillHygiene(env.RepoRoot)
+		if err != nil {
+			res.Err = fmt.Errorf("doctor: %s: verify hygiene: %w", f.ID(), err)
+			return res, res.Err
+		}
 		for _, h := range post {
 			if h.Kind == "UNLINKED" {
 				res.Err = fmt.Errorf("doctor: %s: fix did not eliminate the unlinked-reference findings", f.ID())
 				return res, res.Err
 			}
 		}
+		res.Fixed = len(post) == 0
+		return res, nil
 	}
 	res.Fixed = true
 	return res, nil