Name the workflow files that caused an escalation (#120)

The message said only that the change touches .github/workflows/. That
leaves a reader with nothing but the agent's own account of what it
edited, and a model's prose summary is not evidence. Working out whether
a real escalation was correct cost two probe runs and a wrong hypothesis
on #119.

TouchesWorkflow becomes WorkflowChanges and returns the paths. The
rename forces every call site to be updated deliberately rather than
silently keeping a bool.

Two details the change surfaced:

git status --porcelain collapses an untracked directory to a single
'.github/workflows/' entry, so a brand new workflow directory produced a
path that named no file. -uall lists them individually, and that is
exactly the case where naming matters most.

A staged rename renders as 'R  <old> -> <new>'. The destination is the
file that would be pushed, so that is what gets named.

The push-refusal path passes no paths: git has already refused, and the
remote's message carries its own detail.

Author: charlesgreen

internal/loop/loop.go

@@ -75,9 +75,9 @@ type VCS interface {
 }
 
 type workflowToucher interface {
-	// TouchesWorkflow reports whether the working tree has changes under
+	// WorkflowChanges lists the changed paths under
 	// `.github/workflows/`.
-	TouchesWorkflow(ctx context.Context, dir string) (bool, error)
+	WorkflowChanges(ctx context.Context, dir string) ([]string, error)
 }
 
 const workflowPushRefusal = "refusing to allow a GitHub App to create or update workflow"
@@ -146,20 +146,35 @@ func (e *Engine) promptFor(role domain.Role, iss domain.Issue) string {
 	return iss.Body
 }
 
-func workflowPermissionReason(login string) string {
+func workflowPermissionReason(login string, paths []string) string {
 	who := "a GitHub App"
 	if login != "" {
 		who = "the `" + login + "` GitHub App"
 	}
-	return "the change touches `.github/workflows/`, but this run is authenticated as " + who + ", which deliberately lacks `workflows` permission. GitHub will refuse that push. Commit the workflow change as a human, or rerun the CLI under your own GitHub auth."
+	what := "the change touches `.github/workflows/`"
+	if len(paths) > 0 {
+		what = "the change touches " + quoteList(paths)
+	}
+	return what + ", but this run is authenticated as " + who + ", which deliberately lacks `workflows` permission. GitHub will refuse that push. Commit the workflow change as a human, or rerun the CLI under your own GitHub auth."
+}
+
+// quoteList renders paths for an escalation message. Naming them is the point:
+// without it a reader has only the agent's own description of what it changed,
+// which is not evidence.
+func quoteList(paths []string) string {
+	quoted := make([]string, 0, len(paths))
+	for _, p := range paths {
+		quoted = append(quoted, "`"+p+"`")
+	}
+	return strings.Join(quoted, ", ")
 }
 
 func (e *Engine) workflowPushReason(err error) string {
 	if err == nil {
 		return ""
 	}
 	if strings.Contains(err.Error(), workflowPushRefusal) {
-		return workflowPermissionReason(e.Cfg.SelfLogin)
+		return workflowPermissionReason(e.Cfg.SelfLogin, nil)
 	}
 	return ""
 }
@@ -172,14 +187,14 @@ func (e *Engine) workflowPreflight(ctx context.Context) (string, error) {
 	if !ok {
 		return "", nil
 	}
-	touched, err := vcs.TouchesWorkflow(ctx, e.Cfg.WorkDir)
+	paths, err := vcs.WorkflowChanges(ctx, e.Cfg.WorkDir)
 	if err != nil {
 		return "", err
 	}
-	if !touched {
+	if len(paths) == 0 {
 		return "", nil
 	}
-	return workflowPermissionReason(e.Cfg.SelfLogin), nil
+	return workflowPermissionReason(e.Cfg.SelfLogin, paths), nil
 }
 
 // log records an event if a ledger is configured; otherwise it is a no-op.

internal/loop/loop_vcs_test.go

@@ -12,16 +12,17 @@ import (
 )
 
 type fakeVCS struct {
-	committed    bool
-	commitDone   bool
-	commitErr    error
-	pushDone     bool
-	pushErr      error
-	pushedBranch string
-	syncedBranch string
-	workflows    bool
-	workflowErr  error
-	commitMsg    string
+	committed     bool
+	commitDone    bool
+	commitErr     error
+	pushDone      bool
+	pushErr       error
+	pushedBranch  string
+	syncedBranch  string
+	workflows     bool
+	workflowPaths []string
+	workflowErr   error
+	commitMsg     string
 }
 
 func (f *fakeVCS) Commit(_ context.Context, _, msg string) (bool, error) {
@@ -41,8 +42,17 @@ func (f *fakeVCS) Sync(_ context.Context, _, branch string) error {
 	return nil
 }
 
-func (f *fakeVCS) TouchesWorkflow(_ context.Context, _ string) (bool, error) {
-	return f.workflows, f.workflowErr
+func (f *fakeVCS) WorkflowChanges(_ context.Context, _ string) ([]string, error) {
+	if f.workflowErr != nil {
+		return nil, f.workflowErr
+	}
+	if len(f.workflowPaths) > 0 {
+		return f.workflowPaths, nil
+	}
+	if f.workflows {
+		return []string{".github/workflows/check.yml"}, nil
+	}
+	return nil, nil
 }
 
 func TestOpenPRCommitsAndPushesFirst(t *testing.T) {
@@ -221,7 +231,7 @@ func TestWorkflowEscalationNamesTheAuthenticatedIdentity(t *testing.T) {
 	})
 
 	t.Run("says a GitHub App when the identity is unknown", func(t *testing.T) {
-		if got := workflowPermissionReason(""); !strings.Contains(got, "authenticated as a GitHub App") {
+		if got := workflowPermissionReason("", nil); !strings.Contains(got, "authenticated as a GitHub App") {
 			t.Fatalf("reason = %q, expected the generic form when no login is known", got)
 		}
 	})
@@ -289,3 +299,38 @@ func TestWorkflowPreflightOnlyBlocksWhenAWorkflowActuallyChanged(t *testing.T) {
 		}
 	})
 }
+
+// An escalation that names no file asks the reader to trust the agent's own
+// account of what it changed. That is not evidence, and getting it wrong costs
+// a real investigation, so the message must name the paths git actually saw.
+func TestWorkflowEscalationNamesTheFilesThatCausedIt(t *testing.T) {
+	f := &forgefake.Forge{}
+	eng := &Engine{
+		Runner: enginefake.New(enginefake.Step{Summary: "edited a workflow", Apply: writeFixed}),
+		Gate:   gateChecksFile(),
+		Forge:  f,
+		VCS: &fakeVCS{workflowPaths: []string{
+			".github/workflows/check.yml",
+			".github/workflows/release.yml",
+		}},
+		Cfg: Config{
+			WorkDir:                t.TempDir(),
+			Branch:                 "loop/1",
+			WorkflowRestrictedPush: true,
+			SelfLogin:              "acme-code[bot]",
+		},
+	}
+
+	res, err := eng.Run(context.Background(), domain.Issue{Repo: "o/r", Number: 1})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if res.Outcome != OutcomeBlocked {
+		t.Fatalf("outcome = %s want blocked", res.Outcome)
+	}
+	for _, want := range []string{"`.github/workflows/check.yml`", "`.github/workflows/release.yml`"} {
+		if !strings.Contains(res.Reason, want) {
+			t.Fatalf("reason = %q, expected it to name %s", res.Reason, want)
+		}
+	}
+}

internal/vcs/git/git.go

@@ -115,14 +115,35 @@ func (g *Git) Push(ctx context.Context, dir, branch string) error {
 	return err
 }
 
-// TouchesWorkflow reports whether the working tree has tracked or untracked
-// changes under `.github/workflows/`.
-func (g *Git) TouchesWorkflow(ctx context.Context, dir string) (bool, error) {
-	out, err := g.run(ctx, dir, "status", "--porcelain", "--", ".github/workflows")
+// WorkflowChanges lists the tracked or untracked paths under
+// `.github/workflows/` that the working tree has changed. An empty result means
+// nothing there changed. The paths are returned rather than a bool so an
+// escalation can name the files that caused it, instead of asking the reader to
+// take the agent's word for what it edited.
+func (g *Git) WorkflowChanges(ctx context.Context, dir string) ([]string, error) {
+	// -uall lists untracked files individually. Without it git collapses a new
+	// directory to a single "?? .github/workflows/" entry, and an escalation
+	// that names a directory is barely better than one that names nothing.
+	out, err := g.run(ctx, dir, "status", "--porcelain", "-uall", "--", ".github/workflows")
 	if err != nil {
-		return false, err
+		return nil, err
+	}
+	var paths []string
+	for _, line := range strings.Split(out, "\n") {
+		// Porcelain v1 is "XY <path>", and a rename is "R  <old> -> <new>".
+		// The destination is the file that would be pushed, so report that.
+		if len(line) < 4 {
+			continue
+		}
+		path := strings.TrimSpace(line[3:])
+		if _, dst, ok := strings.Cut(path, " -> "); ok {
+			path = dst
+		}
+		if path = strings.Trim(path, `"`); path != "" {
+			paths = append(paths, path)
+		}
 	}
-	return strings.TrimSpace(out) != "", nil
+	return paths, nil
 }
 
 // Sync hard-resets dir to the remote tip of branch. It fetches the branch and

internal/vcs/git/git_test.go

@@ -152,14 +152,14 @@ func TestCommitWithoutAnIdentityStillWorksWhenGitHasOne(t *testing.T) {
 	}
 }
 
-func TestTouchesWorkflowReportsOnlyWorkflowChanges(t *testing.T) {
+func TestWorkflowChangesReportsOnlyWorkflowChanges(t *testing.T) {
 	work := repoWithBareRemote(t)
 	g := &Git{}
 	ctx := context.Background()
 
-	touched, err := g.TouchesWorkflow(ctx, work)
-	if err != nil || touched {
-		t.Fatalf("clean repo should report no workflow changes: touched=%v err=%v", touched, err)
+	paths, err := g.WorkflowChanges(ctx, work)
+	if err != nil || len(paths) != 0 {
+		t.Fatalf("clean repo should report no workflow changes: paths=%v err=%v", paths, err)
 	}
 
 	if err := os.MkdirAll(filepath.Join(work, ".github", "workflows"), 0o755); err != nil {
@@ -168,27 +168,27 @@ func TestTouchesWorkflowReportsOnlyWorkflowChanges(t *testing.T) {
 	if err := os.WriteFile(filepath.Join(work, ".github", "workflows", "check.yml"), []byte("name: check\n"), 0o644); err != nil {
 		t.Fatal(err)
 	}
-	touched, err = g.TouchesWorkflow(ctx, work)
-	if err != nil || !touched {
-		t.Fatalf("workflow edit should be reported: touched=%v err=%v", touched, err)
+	paths, err = g.WorkflowChanges(ctx, work)
+	if err != nil || len(paths) != 1 || paths[0] != ".github/workflows/check.yml" {
+		t.Fatalf("an edit must be reported by path so an escalation can name it: paths=%v err=%v", paths, err)
 	}
 
 	if _, err := g.Commit(ctx, work, "add workflow"); err != nil {
 		t.Fatalf("Commit: %v", err)
 	}
-	touched, err = g.TouchesWorkflow(ctx, work)
-	if err != nil || touched {
-		t.Fatalf("committed workflow should leave a clean tree: touched=%v err=%v", touched, err)
+	paths, err = g.WorkflowChanges(ctx, work)
+	if err != nil || len(paths) != 0 {
+		t.Fatalf("committed workflow should leave a clean tree: paths=%v err=%v", paths, err)
 	}
 
 	if err := os.WriteFile(filepath.Join(work, "plain.txt"), []byte("x"), 0o644); err != nil {
 		t.Fatal(err)
 	}
-	touched, err = g.TouchesWorkflow(ctx, work)
+	paths, err = g.WorkflowChanges(ctx, work)
 	if err != nil {
-		t.Fatalf("TouchesWorkflow: %v", err)
+		t.Fatalf("WorkflowChanges: %v", err)
 	}
-	if touched {
+	if len(paths) > 0 {
 		t.Fatal("non-workflow edits must not trip workflow detection")
 	}
 }
@@ -217,3 +217,36 @@ func TestDryRunCommitsButDoesNotPush(t *testing.T) {
 		t.Fatalf("a dry run must not push, but the remote has the branch: %s", out)
 	}
 }
+
+// Porcelain renders a staged rename as "R  <old> -> <new>". The destination is
+// the file that would be pushed, so that is the one an escalation should name.
+func TestWorkflowChangesReportsTheDestinationOfARename(t *testing.T) {
+	work := repoWithBareRemote(t)
+	g := &Git{}
+	ctx := context.Background()
+
+	dir := filepath.Join(work, ".github", "workflows")
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "old.yml"), []byte("name: old\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := g.Commit(ctx, work, "add workflow"); err != nil {
+		t.Fatalf("Commit: %v", err)
+	}
+
+	mv := exec.Command("git", "mv", ".github/workflows/old.yml", ".github/workflows/new.yml")
+	mv.Dir = work
+	if out, err := mv.CombinedOutput(); err != nil {
+		t.Fatalf("git mv: %v: %s", err, out)
+	}
+
+	paths, err := g.WorkflowChanges(ctx, work)
+	if err != nil {
+		t.Fatalf("WorkflowChanges: %v", err)
+	}
+	if len(paths) != 1 || paths[0] != ".github/workflows/new.yml" {
+		t.Fatalf("paths = %v, want the rename destination", paths)
+	}
+}