fix(sessions): stop AO hook files from making every worktree permanently dirty (#169)

* fix(sessions): stop AO hook files from making every worktree permanently dirty

Agent adapters write hook files (.codex/hooks.json, .opencode/plugins/
ao-activity.ts, .claude/settings.local.json, ...) into fresh session
worktrees as untracked files. `git worktree remove` (deliberately run
without --force) refuses on any untracked file, so Workspace.Destroy
failed for every session of the 12 workspace-writing harnesses:
POST /sessions/{id}/kill returned an unlogged 500 INTERNAL_ERROR and
`ao session cleanup` reported 'Would clean N' then '0 sessions cleaned'
with no reason, leaking workspaces forever.

Three coordinated fixes, none of which force-deletes user/agent work:

- Root cause: every adapter now writes a sentinel-guarded, self-ignoring
  .gitignore next to its hook files (hookutil.EnsureWorkspaceGitignore),
  so AO's own files no longer count as dirt while anything an agent
  drops — even in the same directory — still blocks teardown. A
  registry-wide conformance test enforces the contract for all current
  and future adapters. (Per-worktree .git/worktrees/<name>/info/exclude
  was evaluated first but git does not honor it.)
- Typed refusal: gitworktree.Destroy classifies a still-dirty refusal as
  ports.ErrWorkspaceDirty (git status probe). Kill maps it to success
  with freed=false (session terminated, worktree preserved); Cleanup
  reports it per-session as skipped-with-reason through the API
  (CleanupSessionsResponse.skipped), and the CLI prints
  'Skipped: <id> (workspace has uncommitted changes)' plus a summary.
- Observability: envelope.WriteError records the raw service error into
  a request-scoped slot and the access log attaches it to 5xx lines, so
  any remaining internal error is diagnosable server-side.

Worktrees created before this fix gain the .gitignore on restore (hook
install re-runs); their cleanup is otherwise reported as skipped instead
of erroring.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(cleanup): address Greptile P2s — surface dirty-probe failures, stop leaking raw errors

Two review findings on this PR:

- gitworktree.Destroy: when the isDirty probe itself failed, the error was
  silently discarded and the refusal looked identical to "registered but not
  dirty". The probe failure now rides the returned error (dirty probe: ...),
  so it reaches the access log via the 5xx error capture.

- Cleanup skip reasons: a non-dirty teardown failure put the raw error —
  including internal filesystem paths — into the public skipped[].reason
  field. The public reason is now the fixed string "workspace teardown
  failed"; the full cause goes to the daemon log (warn, with sessionID and
  path). The dirty-refusal reason is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(gitworktree): wrap the dirty-probe error with %w per errorlint

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

backend/internal/adapters/agent/agy/hooks.go

@@ -84,6 +84,9 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 		return fmt.Errorf("agy.GetAgentHooks: %w", err)
 	}
 
+	if err := hookutil.EnsureWorkspaceGitignore(filepath.Dir(hooksPath), agyHooksFileName); err != nil {
+		return fmt.Errorf("agy.GetAgentHooks: gitignore: %w", err)
+	}
 	return nil
 }
 

backend/internal/adapters/agent/claudecode/hooks.go

@@ -107,6 +107,9 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 	if err := writeClaudeSettings(settingsPath, topLevel, rawHooks); err != nil {
 		return fmt.Errorf("claude-code.GetAgentHooks: %w", err)
 	}
+	if err := hookutil.EnsureWorkspaceGitignore(filepath.Dir(settingsPath), claudeSettingsFileName); err != nil {
+		return fmt.Errorf("claude-code.GetAgentHooks: gitignore: %w", err)
+	}
 	return nil
 }
 

backend/internal/adapters/agent/cline/hooks.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/hookutil"
 	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
 )
 
@@ -75,6 +76,10 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 		return fmt.Errorf("cline.GetAgentHooks: create hook dir: %w", err)
 	}
 
+	// Only scripts AO actually wrote go into the workspace .gitignore: a
+	// user-authored script at one of these paths must keep counting as dirt so
+	// workspace teardown preserves it.
+	written := make([]string, 0, len(clineManagedHooks))
 	for _, spec := range clineManagedHooks {
 		scriptPath := filepath.Join(hooksDir, spec.Event)
 		// Never clobber a user-authored hook with the same event name.
@@ -85,6 +90,10 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 		if err := atomicWriteFile(scriptPath, []byte(script), 0o700); err != nil {
 			return fmt.Errorf("cline.GetAgentHooks: write %s: %w", spec.Event, err)
 		}
+		written = append(written, spec.Event)
+	}
+	if err := hookutil.EnsureWorkspaceGitignore(hooksDir, written...); err != nil {
+		return fmt.Errorf("cline.GetAgentHooks: gitignore: %w", err)
 	}
 	return nil
 }

backend/internal/adapters/agent/copilot/hooks.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/hookutil"
 	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
 )
 
@@ -121,6 +122,9 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 	if err := writeCopilotHooks(hooksPath, file); err != nil {
 		return fmt.Errorf("copilot.GetAgentHooks: %w", err)
 	}
+	if err := hookutil.EnsureWorkspaceGitignore(filepath.Dir(hooksPath), copilotHooksFileName); err != nil {
+		return fmt.Errorf("copilot.GetAgentHooks: gitignore: %w", err)
+	}
 	return nil
 }
 

backend/internal/adapters/agent/cursor/hooks.go

@@ -93,6 +93,9 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 	if err := writeCursorHooks(hooksPath, topLevel, rawHooks); err != nil {
 		return fmt.Errorf("cursor.GetAgentHooks: %w", err)
 	}
+	if err := hookutil.EnsureWorkspaceGitignore(filepath.Dir(hooksPath), cursorHooksFileName); err != nil {
+		return fmt.Errorf("cursor.GetAgentHooks: gitignore: %w", err)
+	}
 	return nil
 }
 

backend/internal/adapters/agent/droid/hooks.go

@@ -113,6 +113,9 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 	if err := writeDroidHooks(hooksPath, topLevel, rawHooks); err != nil {
 		return fmt.Errorf("droid.GetAgentHooks: %w", err)
 	}
+	if err := hookutil.EnsureWorkspaceGitignore(filepath.Dir(hooksPath), droidHooksFileName); err != nil {
+		return fmt.Errorf("droid.GetAgentHooks: gitignore: %w", err)
+	}
 	return nil
 }
 

backend/internal/adapters/agent/goose/hooks.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/hookutil"
 	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
 )
 
@@ -99,6 +100,9 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 	if err := writeGooseHooks(hooksPath, topLevel, rawHooks); err != nil {
 		return fmt.Errorf("goose.GetAgentHooks: %w", err)
 	}
+	if err := hookutil.EnsureWorkspaceGitignore(filepath.Dir(hooksPath), gooseHooksFileName); err != nil {
+		return fmt.Errorf("goose.GetAgentHooks: gitignore: %w", err)
+	}
 	return nil
 }
 

backend/internal/adapters/agent/hookutil/hookutil.go

@@ -4,10 +4,55 @@
 package hookutil
 
 import (
+	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 )
 
+// GitignoreSentinel marks a workspace .gitignore as AO-managed so
+// EnsureWorkspaceGitignore can rewrite its own file idempotently while never
+// touching a user- or repo-provided .gitignore at the same path.
+const GitignoreSentinel = "# managed by agent-orchestrator: AO hook files stay out of git status"
+
+// EnsureWorkspaceGitignore writes a self-ignoring .gitignore into dir covering
+// the named AO-installed files. Hook files land in fresh session worktrees as
+// untracked files, and `git worktree remove` (without --force) refuses on ANY
+// untracked file — without this ignore, AO's own hook files would make every
+// session workspace permanently undeletable. The patterns are anchored to dir
+// and name only AO's files, so anything else an agent drops in the same
+// directory still counts as dirt and keeps blocking teardown.
+//
+// A .gitignore at the same path that lacks the sentinel is left untouched and
+// the install proceeds: the worktree then simply stays dirty and teardown
+// preserves it, which is the safe degradation.
+func EnsureWorkspaceGitignore(dir string, names ...string) error {
+	path := filepath.Join(dir, ".gitignore")
+	existing, err := os.ReadFile(path) //nolint:gosec // path built from caller-owned workspace dir
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return fmt.Errorf("read %s: %w", path, err)
+	}
+	if err == nil && !strings.Contains(string(existing), GitignoreSentinel) {
+		return nil
+	}
+	var b strings.Builder
+	b.WriteString(GitignoreSentinel)
+	b.WriteString("\n/.gitignore\n")
+	for _, name := range names {
+		b.WriteString("/")
+		b.WriteString(filepath.ToSlash(name))
+		b.WriteString("\n")
+	}
+	if err := os.MkdirAll(dir, 0o750); err != nil {
+		return fmt.Errorf("create %s: %w", dir, err)
+	}
+	if err := AtomicWriteFile(path, []byte(b.String()), 0o600); err != nil {
+		return fmt.Errorf("write %s: %w", path, err)
+	}
+	return nil
+}
+
 // AtomicWriteFile writes data to path via a temp file in the same directory
 // followed by a rename, so a crash or signal mid-write can't leave a truncated
 // or empty file that the agent then fails to parse (silently disabling hooks).

backend/internal/adapters/agent/hookutil/hookutil_test.go

@@ -0,0 +1,73 @@
+package hookutil
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestEnsureWorkspaceGitignoreWritesSelfIgnoringFile(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), ".codex")
+	if err := EnsureWorkspaceGitignore(dir, "hooks.json", "config.toml"); err != nil {
+		t.Fatalf("ensure: %v", err)
+	}
+	data, err := os.ReadFile(filepath.Join(dir, ".gitignore"))
+	if err != nil {
+		t.Fatalf("read: %v", err)
+	}
+	content := string(data)
+	if !strings.Contains(content, GitignoreSentinel) {
+		t.Fatalf("content missing sentinel: %q", content)
+	}
+	// Entries are anchored so only AO's files in THIS directory are ignored —
+	// an agent's own files (even in the same dir) must keep counting as dirt.
+	for _, want := range []string{"/.gitignore\n", "/hooks.json\n", "/config.toml\n"} {
+		if !strings.Contains(content, want) {
+			t.Errorf("content missing entry %q: %q", want, content)
+		}
+	}
+}
+
+func TestEnsureWorkspaceGitignoreIsIdempotent(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), ".codex")
+	if err := EnsureWorkspaceGitignore(dir, "hooks.json"); err != nil {
+		t.Fatalf("first ensure: %v", err)
+	}
+	first, err := os.ReadFile(filepath.Join(dir, ".gitignore"))
+	if err != nil {
+		t.Fatalf("read: %v", err)
+	}
+	if err := EnsureWorkspaceGitignore(dir, "hooks.json"); err != nil {
+		t.Fatalf("second ensure: %v", err)
+	}
+	second, err := os.ReadFile(filepath.Join(dir, ".gitignore"))
+	if err != nil {
+		t.Fatalf("read: %v", err)
+	}
+	if string(first) != string(second) {
+		t.Fatalf("rewrite changed content:\nfirst:  %q\nsecond: %q", first, second)
+	}
+}
+
+func TestEnsureWorkspaceGitignoreLeavesForeignFileUntouched(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), ".codex")
+	if err := os.MkdirAll(dir, 0o750); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	foreign := "# user rules\n*.log\n"
+	path := filepath.Join(dir, ".gitignore")
+	if err := os.WriteFile(path, []byte(foreign), 0o600); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+	if err := EnsureWorkspaceGitignore(dir, "hooks.json"); err != nil {
+		t.Fatalf("ensure: %v", err)
+	}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("read: %v", err)
+	}
+	if string(data) != foreign {
+		t.Fatalf("foreign .gitignore was modified: %q", data)
+	}
+}

backend/internal/adapters/agent/kilocode/hooks.go

@@ -10,6 +10,7 @@ import (
 
 	_ "embed"
 
+	"github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/hookutil"
 	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
 )
 
@@ -94,6 +95,9 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 	if err := atomicWriteFile(pluginPath, []byte(kilocodePluginSource), 0o600); err != nil {
 		return fmt.Errorf("kilocode.GetAgentHooks: write plugin: %w", err)
 	}
+	if err := hookutil.EnsureWorkspaceGitignore(filepath.Dir(pluginPath), kilocodePluginFileName); err != nil {
+		return fmt.Errorf("kilocode.GetAgentHooks: gitignore: %w", err)
+	}
 	return nil
 }
 

backend/internal/adapters/agent/kiro/hooks.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/hookutil"
 	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
 )
 
@@ -99,6 +100,9 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 	if err := writeKiroHooks(hooksPath, topLevel, rawHooks); err != nil {
 		return fmt.Errorf("kiro.GetAgentHooks: %w", err)
 	}
+	if err := hookutil.EnsureWorkspaceGitignore(filepath.Dir(hooksPath), kiroAgentFileName); err != nil {
+		return fmt.Errorf("kiro.GetAgentHooks: gitignore: %w", err)
+	}
 	return nil
 }
 

backend/internal/adapters/agent/opencode/hooks.go

@@ -94,6 +94,9 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 	if err := hookutil.AtomicWriteFile(pluginPath, []byte(opencodePluginSource), 0o600); err != nil {
 		return fmt.Errorf("opencode.GetAgentHooks: write plugin: %w", err)
 	}
+	if err := hookutil.EnsureWorkspaceGitignore(filepath.Dir(pluginPath), opencodePluginFileName); err != nil {
+		return fmt.Errorf("opencode.GetAgentHooks: gitignore: %w", err)
+	}
 	return nil
 }
 

backend/internal/adapters/agent/qwen/hooks.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/hookutil"
 	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
 )
 
@@ -112,6 +113,9 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi
 	if err := writeQwenSettings(settingsPath, topLevel, rawHooks); err != nil {
 		return fmt.Errorf("qwen.GetAgentHooks: %w", err)
 	}
+	if err := hookutil.EnsureWorkspaceGitignore(filepath.Dir(settingsPath), qwenSettingsFileName); err != nil {
+		return fmt.Errorf("qwen.GetAgentHooks: gitignore: %w", err)
+	}
 	return nil
 }
 

backend/internal/adapters/agent/registry/registry_test.go

@@ -0,0 +1,85 @@
+package registry
+
+import (
+	"context"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/hookutil"
+	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
+)
+
+// TestGetAgentHooksFootprintIsGitignored enforces a contract every shipped
+// (and future) adapter must hold: any file GetAgentHooks writes into a session
+// worktree must be covered by a sibling AO-managed self-ignoring .gitignore
+// (hookutil.EnsureWorkspaceGitignore). Hook files are untracked, and
+// `git worktree remove` (without --force) refuses on any untracked file — an
+// uncovered hook file makes every one of that adapter's session workspaces
+// permanently undeletable (kill/cleanup can never free them).
+func TestGetAgentHooksFootprintIsGitignored(t *testing.T) {
+	for _, ha := range Harnessed() {
+		t.Run(string(ha.Harness), func(t *testing.T) {
+			ws := t.TempDir()
+			cfg := ports.WorkspaceHookConfig{
+				SessionID:     "proj-1",
+				WorkspacePath: ws,
+				DataDir:       t.TempDir(),
+			}
+			if err := ha.Agent.GetAgentHooks(context.Background(), cfg); err != nil {
+				t.Fatalf("GetAgentHooks: %v", err)
+			}
+			files := workspaceFiles(t, ws)
+			for _, rel := range files {
+				gitignorePath := filepath.Join(ws, filepath.Dir(rel), ".gitignore")
+				data, err := os.ReadFile(gitignorePath) //nolint:gosec // test-owned temp dir
+				if err != nil {
+					t.Errorf("hook file %q has no sibling .gitignore (%v); it will keep the session worktree permanently dirty", rel, err)
+					continue
+				}
+				content := string(data)
+				if !strings.Contains(content, hookutil.GitignoreSentinel) {
+					t.Errorf(".gitignore next to %q is not AO-managed (missing sentinel)", rel)
+					continue
+				}
+				if entry := "/" + filepath.Base(rel); !hasLine(content, entry) {
+					t.Errorf(".gitignore next to %q does not list %q", rel, entry)
+				}
+			}
+		})
+	}
+}
+
+// workspaceFiles returns every regular file under root, relative to root.
+func workspaceFiles(t *testing.T, root string) []string {
+	t.Helper()
+	var files []string
+	err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.Type().IsRegular() {
+			rel, err := filepath.Rel(root, path)
+			if err != nil {
+				return err
+			}
+			files = append(files, rel)
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("walk workspace: %v", err)
+	}
+	return files
+}
+
+func hasLine(content, line string) bool {
+	for _, l := range strings.Split(content, "\n") {
+		if strings.TrimSpace(l) == line {
+			return true
+		}
+	}
+	return false
+}

backend/internal/adapters/workspace/gitworktree/commands.go

@@ -31,6 +31,13 @@ func worktreePruneArgs(repo string) []string {
 	return []string{"-C", repo, "worktree", "prune"}
 }
 
+// statusPorcelainArgs probes the worktree at path for uncommitted changes or
+// untracked files — the condition `git worktree remove` (without --force)
+// refuses on — so Destroy can classify a refusal as ports.ErrWorkspaceDirty.
+func statusPorcelainArgs(path string) []string {
+	return []string{"-C", path, "status", "--porcelain"}
+}
+
 func worktreeListPorcelainArgs(repo string) []string {
 	return []string{"-C", repo, "worktree", "list", "--porcelain"}
 }

backend/internal/adapters/workspace/gitworktree/workspace.go

@@ -157,6 +157,18 @@ func (w *Workspace) Destroy(ctx context.Context, info ports.WorkspaceInfo) error
 	}
 	if _, ok := findWorktree(records, path); ok {
 		if removeErr != nil {
+			// Distinguish the dirty-worktree refusal (uncommitted agent work)
+			// from other registration leftovers (e.g. a locked worktree) so the
+			// Session Manager can preserve the workspace without erroring.
+			dirty, statusErr := w.isDirty(ctx, path)
+			if statusErr == nil && dirty {
+				return fmt.Errorf("gitworktree: refusing to remove %q: %w (worktree remove: %w)", path, ports.ErrWorkspaceDirty, removeErr)
+			}
+			if statusErr != nil {
+				// A failed probe must stay visible: without it the caller can't
+				// tell "not dirty" from "couldn't check".
+				return fmt.Errorf("gitworktree: refusing to remove %q: path is still registered after git worktree prune (worktree remove: %w; dirty probe: %w)", path, removeErr, statusErr)
+			}
 			return fmt.Errorf("gitworktree: refusing to remove %q: path is still registered after git worktree prune (worktree remove: %w)", path, removeErr)
 		}
 		return fmt.Errorf("gitworktree: refusing to remove %q: path is still registered after git worktree prune", path)
@@ -302,6 +314,17 @@ func (w *Workspace) refExists(ctx context.Context, repo, ref string) (bool, erro
 	return false, fmt.Errorf("gitworktree: verify ref %q: %w", ref, err)
 }
 
+// isDirty reports whether the worktree at path has uncommitted changes or
+// untracked files — the same check `git worktree remove` performs before
+// refusing without --force.
+func (w *Workspace) isDirty(ctx context.Context, path string) (bool, error) {
+	out, err := w.run(ctx, w.binary, statusPorcelainArgs(path)...)
+	if err != nil {
+		return false, fmt.Errorf("gitworktree: status %q: %w", path, err)
+	}
+	return strings.TrimSpace(string(out)) != "", nil
+}
+
 func (w *Workspace) listRecords(ctx context.Context, repo string) ([]worktreeRecord, error) {
 	out, err := w.run(ctx, w.binary, worktreeListPorcelainArgs(repo)...)
 	if err != nil {

backend/internal/adapters/workspace/gitworktree/workspace_integration_test.go

@@ -92,6 +92,68 @@ func TestWorkspaceIntegrationDestroyRefusesLockedWorktree(t *testing.T) {
 	}
 }
 
+// TestWorkspaceIntegrationDestroyDirtyWorktree proves the two halves of the
+// dirty-teardown contract against real git:
+//
+//  1. A worktree whose only untracked files are covered by a self-ignoring
+//     .gitignore (the shape agent adapters install for their hook files) is
+//     clean in git's eyes, so Destroy succeeds without --force.
+//  2. Real uncommitted work makes Destroy refuse with ports.ErrWorkspaceDirty
+//     and preserves the worktree.
+func TestWorkspaceIntegrationDestroyDirtyWorktree(t *testing.T) {
+	git := requireGit(t)
+	tmp := t.TempDir()
+	repo := setupOriginClone(t, git, tmp)
+	root := filepath.Join(tmp, "managed")
+	ws, err := New(Options{Binary: git, ManagedRoot: root, RepoResolver: StaticRepoResolver{"proj": repo}})
+	if err != nil {
+		t.Fatalf("new: %v", err)
+	}
+	ctx := context.Background()
+	info, err := ws.Create(ctx, ports.WorkspaceConfig{ProjectID: "proj", SessionID: "sess", Branch: "feature/dirty"})
+	if err != nil {
+		t.Fatalf("create: %v", err)
+	}
+
+	// AO-managed hook files behind a self-ignoring .gitignore: invisible to git
+	// status, so they must not block teardown.
+	hookDir := filepath.Join(info.Path, ".codex")
+	if err := os.MkdirAll(hookDir, 0o750); err != nil {
+		t.Fatalf("mkdir hook dir: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(hookDir, "hooks.json"), []byte("{}\n"), 0o600); err != nil {
+		t.Fatalf("write hooks.json: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(hookDir, ".gitignore"), []byte(".gitignore\nhooks.json\n"), 0o600); err != nil {
+		t.Fatalf("write .gitignore: %v", err)
+	}
+
+	// Real agent work must keep blocking teardown, typed as ErrWorkspaceDirty.
+	wip := filepath.Join(info.Path, "wip.txt")
+	if err := os.WriteFile(wip, []byte("uncommitted\n"), 0o600); err != nil {
+		t.Fatalf("write wip: %v", err)
+	}
+	err = ws.Destroy(ctx, info)
+	if !errors.Is(err, ports.ErrWorkspaceDirty) {
+		t.Fatalf("destroy dirty error = %v, want ports.ErrWorkspaceDirty", err)
+	}
+	if _, statErr := os.Stat(wip); statErr != nil {
+		t.Fatalf("dirty worktree was not preserved: %v", statErr)
+	}
+
+	// With the real work gone, only the ignored AO files remain — git considers
+	// the worktree clean and Destroy succeeds without --force.
+	if err := os.Remove(wip); err != nil {
+		t.Fatalf("remove wip: %v", err)
+	}
+	if err := ws.Destroy(ctx, info); err != nil {
+		t.Fatalf("destroy with ignored-only files: %v", err)
+	}
+	if _, err := os.Stat(info.Path); !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("path after destroy stat err = %v, want not exist", err)
+	}
+}
+
 func requireGit(t *testing.T) string {
 	t.Helper()
 	git, err := exec.LookPath("git")

backend/internal/adapters/workspace/gitworktree/workspace_test.go

@@ -33,6 +33,7 @@ func TestCommandArgs(t *testing.T) {
 		{"remove", worktreeRemoveArgs(repo, path), []string{"-C", repo, "worktree", "remove", path}},
 		{"prune", worktreePruneArgs(repo), []string{"-C", repo, "worktree", "prune"}},
 		{"list", worktreeListPorcelainArgs(repo), []string{"-C", repo, "worktree", "list", "--porcelain"}},
+		{"status", statusPorcelainArgs(path), []string{"-C", path, "status", "--porcelain"}},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -216,6 +217,12 @@ func TestDestroyRefusesStillRegisteredPathAndPreservesDirectory(t *testing.T) {
 	if err == nil || !strings.Contains(err.Error(), "still registered") {
 		t.Fatalf("destroy error = %v", err)
 	}
+	// The stub reports a clean `git status`, so the refusal must NOT be typed as
+	// a dirty workspace — Kill/Cleanup would otherwise silently skip a refusal
+	// that has a different cause (e.g. a locked worktree).
+	if errors.Is(err, ports.ErrWorkspaceDirty) {
+		t.Fatalf("destroy error = %v, want non-dirty refusal for clean status", err)
+	}
 	if _, statErr := os.Stat(filepath.Join(path, "keep.txt")); statErr != nil {
 		t.Fatalf("expected directory to be preserved: %v", statErr)
 	}
@@ -229,6 +236,46 @@ func TestDestroyRefusesStillRegisteredPathAndPreservesDirectory(t *testing.T) {
 	}
 }
 
+// TestDestroyClassifiesDirtyWorktree covers the typed dirty refusal: when
+// `git worktree remove` fails, the path stays registered, and `git status`
+// reports uncommitted work, Destroy must wrap ports.ErrWorkspaceDirty so the
+// Session Manager can preserve the workspace (Kill freed=false, Cleanup
+// skipped-with-reason) instead of surfacing an opaque 500.
+func TestDestroyClassifiesDirtyWorktree(t *testing.T) {
+	root := t.TempDir()
+	repo := t.TempDir()
+	ws, err := New(Options{ManagedRoot: root, RepoResolver: StaticRepoResolver{"proj": repo}})
+	if err != nil {
+		t.Fatalf("new: %v", err)
+	}
+	path := filepath.Join(ws.managedRoot, "proj", "sess")
+	if err := mkdirFile(path, "keep.txt"); err != nil {
+		t.Fatalf("seed path: %v", err)
+	}
+	ws.run = func(_ context.Context, _ string, args ...string) ([]byte, error) {
+		joined := strings.Join(args, " ")
+		switch {
+		case strings.Contains(joined, "worktree remove"):
+			return []byte("contains modified or untracked files"), errors.New("remove failed")
+		case strings.Contains(joined, "worktree prune"):
+			return nil, nil
+		case strings.Contains(joined, "worktree list --porcelain"):
+			return []byte("worktree " + path + "\nbranch refs/heads/feature/one\n"), nil
+		case strings.Contains(joined, "status --porcelain"):
+			return []byte("?? keep.txt\n"), nil
+		default:
+			return nil, nil
+		}
+	}
+	err = ws.Destroy(context.Background(), ports.WorkspaceInfo{Path: path, ProjectID: "proj", SessionID: "sess", Branch: "feature/one"})
+	if !errors.Is(err, ports.ErrWorkspaceDirty) {
+		t.Fatalf("destroy error = %v, want ports.ErrWorkspaceDirty", err)
+	}
+	if _, statErr := os.Stat(filepath.Join(path, "keep.txt")); statErr != nil {
+		t.Fatalf("expected dirty worktree to be preserved: %v", statErr)
+	}
+}
+
 // TestAddWorktreeRefusesBranchCheckedOutElsewhere covers Bug 3 (a): if the
 // requested branch is already checked out in another worktree of the same repo,
 // Create must surface ports.ErrWorkspaceBranchCheckedOutElsewhere so the HTTP

backend/internal/cli/dto_drift_e2e_test.go

@@ -82,8 +82,8 @@ func (f *fakeSessionService) RollbackSpawn(context.Context, domain.SessionID) (s
 	return sessionsvc.RollbackOutcome{}, nil
 }
 
-func (f *fakeSessionService) Cleanup(context.Context, domain.ProjectID) ([]domain.SessionID, error) {
-	return nil, nil
+func (f *fakeSessionService) Cleanup(context.Context, domain.ProjectID) (sessionsvc.CleanupOutcome, error) {
+	return sessionsvc.CleanupOutcome{}, nil
 }
 
 func (f *fakeSessionService) Rename(context.Context, domain.SessionID, string) error {

backend/internal/cli/session.go

@@ -68,6 +68,7 @@ type sessionResponse struct {
 
 type killSessionResponse struct {
 	SessionID string `json:"sessionId"`
+	Freed     bool   `json:"freed"`
 }
 
 type restoreSessionResponse struct {
@@ -80,8 +81,14 @@ type renameSessionResponse struct {
 	DisplayName string `json:"displayName"`
 }
 
+type cleanupSkippedSession struct {
+	SessionID string `json:"sessionId"`
+	Reason    string `json:"reason"`
+}
+
 type cleanupSessionsResponse struct {
-	Cleaned []string `json:"cleaned"`
+	Cleaned []string                `json:"cleaned"`
+	Skipped []cleanupSkippedSession `json:"skipped"`
 }
 
 type claimPRRequest struct {
@@ -431,7 +438,13 @@ func (c *commandContext) killSession(ctx context.Context, cmd *cobra.Command, id
 	if err := c.postJSON(ctx, "sessions/"+url.PathEscape(id)+"/kill", struct{}{}, &res); err != nil {
 		return err
 	}
-	_, err := fmt.Fprintf(cmd.OutOrStdout(), "session %s killed\n", res.SessionID)
+	if res.Freed {
+		_, err := fmt.Fprintf(cmd.OutOrStdout(), "session %s killed\n", res.SessionID)
+		return err
+	}
+	// freed=false: the workspace was preserved (e.g. uncommitted changes) — the
+	// session is terminated either way, but the worktree is left for inspection.
+	_, err := fmt.Fprintf(cmd.OutOrStdout(), "session %s killed (workspace preserved)\n", res.SessionID)
 	return err
 }
 
@@ -530,7 +543,20 @@ func (c *commandContext) cleanupSessions(ctx context.Context, cmd *cobra.Command
 			return err
 		}
 	}
-	_, err = fmt.Fprintf(out, "\nCleanup complete. %d session%s cleaned.\n", len(cleaned), pluralS(len(cleaned)))
+	for _, skip := range res.Skipped {
+		label := skip.SessionID
+		if mapped := labelByID[skip.SessionID]; mapped != "" {
+			label = mapped
+		}
+		if _, err := fmt.Fprintf(out, "  Skipped: %s (%s)\n", label, skip.Reason); err != nil {
+			return err
+		}
+	}
+	summary := fmt.Sprintf("\nCleanup complete. %d session%s cleaned", len(cleaned), pluralS(len(cleaned)))
+	if len(res.Skipped) > 0 {
+		summary += fmt.Sprintf(", %d skipped", len(res.Skipped))
+	}
+	_, err = fmt.Fprintf(out, "%s.\n", summary)
 	return err
 }
 

backend/internal/cli/session_test.go

@@ -69,7 +69,7 @@ func sessionCommandServer(t *testing.T) (*httptest.Server, *sessionRequestLog) {
 			}
 			_, _ = io.WriteString(w, `{"ok":true,"sessionId":"demo-1","prs":[{"url":`+jsonQuote(req.PR)+`,"number":142,"state":"open","ci":"passing","review":"review_required","mergeability":"mergeable","reviewComments":false,"updatedAt":"2026-06-04T12:00:00Z"}],"branchChanged":true,"takenOverFrom":["demo-0"]}`)
 		case r.Method == http.MethodPost && r.URL.Path == "/api/v1/sessions/cleanup":
-			_, _ = io.WriteString(w, `{"ok":true,"cleaned":["demo-old","demo-orch"]}`)
+			_, _ = io.WriteString(w, `{"ok":true,"cleaned":["demo-old","demo-orch"],"skipped":[]}`)
 		case r.Method == http.MethodPost && r.URL.Path == "/api/v1/sessions/demo-1/kill":
 			_, _ = io.WriteString(w, `{"ok":true,"sessionId":"demo-1","freed":true}`)
 		case r.Method == http.MethodPost && r.URL.Path == "/api/v1/sessions/demo-1/restore":
@@ -225,6 +225,33 @@ func TestSessionKill_SuccessWithProjectScope(t *testing.T) {
 	}
 }
 
+// TestSessionKill_PreservedWorkspaceNote: freed=false means the daemon
+// terminated the session but kept the worktree (uncommitted changes are never
+// force-deleted) — the CLI must say so instead of implying a full teardown.
+func TestSessionKill_PreservedWorkspaceNote(t *testing.T) {
+	cfg := setConfigEnv(t)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		if r.Method == http.MethodPost && r.URL.Path == "/api/v1/sessions/demo-1/kill" {
+			_, _ = io.WriteString(w, `{"ok":true,"sessionId":"demo-1","freed":false}`)
+			return
+		}
+		http.NotFound(w, r)
+	}))
+	t.Cleanup(srv.Close)
+	writeRunFileFor(t, cfg, srv)
+
+	out, errOut, err := executeCLI(t, Deps{
+		ProcessAlive: func(int) bool { return true },
+	}, "session", "kill", "demo-1")
+	if err != nil {
+		t.Fatalf("session kill failed: %v\nstderr=%s", err, errOut)
+	}
+	if !strings.Contains(out, "session demo-1 killed (workspace preserved)") {
+		t.Fatalf("unexpected kill output:\n%s", out)
+	}
+}
+
 func TestSessionRestore_SuccessWithProjectScope(t *testing.T) {
 	cfg := setConfigEnv(t)
 	srv, log := sessionCommandServer(t)
@@ -274,6 +301,45 @@ func TestSessionCleanup_YesSkipsPrompt(t *testing.T) {
 	}
 }
 
+// TestSessionCleanup_ReportsSkippedWorkspaces: a session whose workspace was
+// preserved must be listed with its reason and counted in the summary —
+// previously the CLI printed "Would clean N" then "0 sessions cleaned" with no
+// explanation, leaking workspaces invisibly.
+func TestSessionCleanup_ReportsSkippedWorkspaces(t *testing.T) {
+	cfg := setConfigEnv(t)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch {
+		case r.Method == http.MethodGet && r.URL.Path == "/api/v1/sessions":
+			_, _ = io.WriteString(w, `{"sessions":[`+
+				sessionJSON("demo-old", "demo", "worker", "terminated", true)+`,`+
+				sessionJSON("demo-orch", "demo", "orchestrator", "terminated", true)+`]}`)
+		case r.Method == http.MethodPost && r.URL.Path == "/api/v1/sessions/cleanup":
+			_, _ = io.WriteString(w, `{"ok":true,"cleaned":["demo-old"],"skipped":[{"sessionId":"demo-orch","reason":"workspace has uncommitted changes"}]}`)
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	t.Cleanup(srv.Close)
+	writeRunFileFor(t, cfg, srv)
+
+	out, errOut, err := executeCLI(t, Deps{
+		ProcessAlive: func(int) bool { return true },
+	}, "session", "cleanup", "--project", "demo", "--yes")
+	if err != nil {
+		t.Fatalf("session cleanup failed: %v\nstderr=%s", err, errOut)
+	}
+	for _, want := range []string{
+		"Cleaned: demo-old",
+		"Skipped: demo-orch (workspace has uncommitted changes)",
+		"Cleanup complete. 1 session cleaned, 1 skipped.",
+	} {
+		if !strings.Contains(out, want) {
+			t.Fatalf("cleanup output missing %q:\n%s", want, out)
+		}
+	}
+}
+
 func TestSessionCleanup_PromptFailsWithoutInput(t *testing.T) {
 	cfg := setConfigEnv(t)
 	srv, log := sessionCommandServer(t)

backend/internal/httpd/apispec/openapi.yaml

@@ -1028,9 +1028,24 @@ components:
           type: array
         ok:
           type: boolean
+        skipped:
+          items:
+            $ref: '#/components/schemas/CleanupSkippedSession'
+          type: array
       required:
       - ok
       - cleaned
+      - skipped
+      type: object
+    CleanupSkippedSession:
+      properties:
+        reason:
+          type: string
+        sessionId:
+          type: string
+      required:
+      - sessionId
+      - reason
       type: object
     DegradedProject:
       properties:

backend/internal/httpd/apispec/specgen/build.go

@@ -139,6 +139,7 @@ var schemaNames = map[string]string{
 	"ControllersRenameSessionResponse":      "RenameSessionResponse",
 	"ControllersRestoreSessionResponse":     "RestoreSessionResponse",
 	"ControllersCleanupSessionsResponse":    "CleanupSessionsResponse",
+	"ControllersCleanupSkippedSession":      "CleanupSkippedSession",
 	"ControllersKillSessionResponse":        "KillSessionResponse",
 	"ControllersRollbackSessionResponse":    "RollbackSessionResponse",
 	"ControllersSendSessionMessageRequest":  "SendSessionMessageRequest",

backend/internal/httpd/controllers/dto.go

@@ -167,10 +167,19 @@ type RollbackSessionResponse struct {
 	Killed    bool             `json:"killed,omitempty"`
 }
 
+// CleanupSkippedSession is one terminal session whose workspace cleanup
+// preserved rather than reclaimed (a dirty worktree is never force-deleted),
+// with the user-facing reason.
+type CleanupSkippedSession struct {
+	SessionID domain.SessionID `json:"sessionId"`
+	Reason    string           `json:"reason"`
+}
+
 // CleanupSessionsResponse is the body of POST /api/v1/sessions/cleanup.
 type CleanupSessionsResponse struct {
-	OK      bool               `json:"ok"`
-	Cleaned []domain.SessionID `json:"cleaned"`
+	OK      bool                    `json:"ok"`
+	Cleaned []domain.SessionID      `json:"cleaned"`
+	Skipped []CleanupSkippedSession `json:"skipped"`
 }
 
 // SendSessionMessageRequest is the body of POST /api/v1/sessions/{sessionId}/send.

backend/internal/httpd/controllers/sessions.go

@@ -31,7 +31,7 @@ type SessionService interface {
 	Restore(ctx context.Context, id domain.SessionID) (domain.Session, error)
 	Kill(ctx context.Context, id domain.SessionID) (bool, error)
 	RollbackSpawn(ctx context.Context, id domain.SessionID) (sessionsvc.RollbackOutcome, error)
-	Cleanup(ctx context.Context, project domain.ProjectID) ([]domain.SessionID, error)
+	Cleanup(ctx context.Context, project domain.ProjectID) (sessionsvc.CleanupOutcome, error)
 	Rename(ctx context.Context, id domain.SessionID, displayName string) error
 	Send(ctx context.Context, id domain.SessionID, message string) error
 	ListPRs(ctx context.Context, id domain.SessionID) ([]domain.PRFacts, error)
@@ -244,12 +244,16 @@ func (c *SessionsController) cleanup(w http.ResponseWriter, r *http.Request) {
 		apispec.NotImplemented(w, r, "POST", "/api/v1/sessions/cleanup")
 		return
 	}
-	cleaned, err := c.Svc.Cleanup(r.Context(), domain.ProjectID(r.URL.Query().Get("project")))
+	out, err := c.Svc.Cleanup(r.Context(), domain.ProjectID(r.URL.Query().Get("project")))
 	if err != nil {
 		envelope.WriteError(w, r, err)
 		return
 	}
-	envelope.WriteJSON(w, http.StatusOK, CleanupSessionsResponse{OK: true, Cleaned: cleaned})
+	skipped := make([]CleanupSkippedSession, 0, len(out.Skipped))
+	for _, skip := range out.Skipped {
+		skipped = append(skipped, CleanupSkippedSession{SessionID: skip.SessionID, Reason: skip.Reason})
+	}
+	envelope.WriteJSON(w, http.StatusOK, CleanupSessionsResponse{OK: true, Cleaned: out.Cleaned, Skipped: skipped})
 }
 
 func (c *SessionsController) send(w http.ResponseWriter, r *http.Request) {

backend/internal/httpd/controllers/sessions_test.go

@@ -22,6 +22,7 @@ type fakeSessionService struct {
 	sent            string
 	cleanupProjects []domain.ProjectID
 	cleanupResult   []domain.SessionID
+	cleanupSkipped  []sessionsvc.CleanupSkipped
 	claimErr        error
 	listPRErr       error
 }
@@ -100,12 +101,13 @@ func (f *fakeSessionService) RollbackSpawn(_ context.Context, id domain.SessionI
 	return sessionsvc.RollbackOutcome{}, nil
 }
 
-func (f *fakeSessionService) Cleanup(_ context.Context, project domain.ProjectID) ([]domain.SessionID, error) {
+func (f *fakeSessionService) Cleanup(_ context.Context, project domain.ProjectID) (sessionsvc.CleanupOutcome, error) {
 	f.cleanupProjects = append(f.cleanupProjects, project)
-	if f.cleanupResult != nil {
-		return f.cleanupResult, nil
+	cleaned := f.cleanupResult
+	if cleaned == nil {
+		cleaned = []domain.SessionID{"ao-1"}
 	}
-	return []domain.SessionID{"ao-1"}, nil
+	return sessionsvc.CleanupOutcome{Cleaned: cleaned, Skipped: f.cleanupSkipped}, nil
 }
 
 func (f *fakeSessionService) Rename(_ context.Context, id domain.SessionID, displayName string) error {
@@ -318,6 +320,7 @@ func TestSessionsAPI_SendValidation(t *testing.T) {
 func TestSessionsAPI_CleanupWithProjectFilter(t *testing.T) {
 	svc := newFakeSessionService()
 	svc.cleanupResult = []domain.SessionID{"ao-1"}
+	svc.cleanupSkipped = []sessionsvc.CleanupSkipped{{SessionID: "ao-2", Reason: "workspace has uncommitted changes"}}
 	srv := newSessionTestServer(t, svc)
 
 	body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions/cleanup?project=ao", "")
@@ -327,11 +330,18 @@ func TestSessionsAPI_CleanupWithProjectFilter(t *testing.T) {
 	var got struct {
 		OK      bool     `json:"ok"`
 		Cleaned []string `json:"cleaned"`
+		Skipped []struct {
+			SessionID string `json:"sessionId"`
+			Reason    string `json:"reason"`
+		} `json:"skipped"`
 	}
 	mustJSON(t, body, &got)
 	if !got.OK || len(got.Cleaned) != 1 || got.Cleaned[0] != "ao-1" {
 		t.Fatalf("cleanup response = %#v", got)
 	}
+	if len(got.Skipped) != 1 || got.Skipped[0].SessionID != "ao-2" || got.Skipped[0].Reason != "workspace has uncommitted changes" {
+		t.Fatalf("cleanup skipped = %#v, want preserved workspace with reason", got.Skipped)
+	}
 	if len(svc.cleanupProjects) != 1 || svc.cleanupProjects[0] != "ao" {
 		t.Fatalf("cleanupProjects = %#v, want [ao]", svc.cleanupProjects)
 	}

backend/internal/httpd/envelope/envelope.go

@@ -1,6 +1,7 @@
 package envelope
 
 import (
+	"context"
 	"encoding/json"
 	"errors"
 	"net/http"
@@ -10,6 +11,30 @@ import (
 	"github.com/aoagents/agent-orchestrator/backend/internal/httpd/apierr"
 )
 
+// errCapture is a request-scoped slot WriteError records the raw service error
+// into. The wire envelope deliberately hides internals behind "Internal server
+// error", which previously meant a 500's cause was lost entirely — the access
+// log reads the captured error back so the daemon log keeps the diagnosis.
+type errCapture struct{ err error }
+
+type errCaptureKey struct{}
+
+// WithErrorCapture returns a copy of the request whose context carries an
+// error-capture slot, plus a getter for the error recorded by WriteError while
+// handling it. The request logger installs it and reads it after the handler.
+func WithErrorCapture(r *http.Request) (*http.Request, func() error) {
+	capture := &errCapture{}
+	req := r.WithContext(context.WithValue(r.Context(), errCaptureKey{}, capture))
+	return req, func() error { return capture.err }
+}
+
+// captureError records err for the request if a capture slot is present.
+func captureError(r *http.Request, err error) {
+	if c, ok := r.Context().Value(errCaptureKey{}).(*errCapture); ok {
+		c.err = err
+	}
+}
+
 // APIError is the locked wire shape for every non-2xx response.
 type APIError struct {
 	Error     string         `json:"error"`
@@ -42,6 +67,7 @@ func WriteAPIError(w http.ResponseWriter, r *http.Request, status int, kind, cod
 // back to a 500 for any other error so internal details never leak. This is the
 // only place an apierr.Kind is translated into an HTTP status and wire word.
 func WriteError(w http.ResponseWriter, r *http.Request, err error) {
+	captureError(r, err)
 	var e *apierr.Error
 	if errors.As(err, &e) {
 		status, kind := httpStatus(e.Kind)

backend/internal/httpd/log.go

@@ -6,6 +6,8 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5/middleware"
+
+	"github.com/aoagents/agent-orchestrator/backend/internal/httpd/envelope"
 )
 
 // requestLogger emits one structured access-log line per request via the
@@ -18,13 +20,18 @@ import (
 // is accurate even when the handler returns without calling WriteHeader. The
 // request id is read off the context populated by middleware.RequestID, so
 // this middleware must be mounted after it.
+//
+// A 5xx line additionally carries the raw service error recorded by
+// envelope.WriteError: the wire envelope hides internals ("Internal server
+// error"), so without this the cause of a 500 was lost entirely.
 func requestLogger(log *slog.Logger) func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor)
+			r, capturedErr := envelope.WithErrorCapture(r)
 			start := time.Now()
 			defer func() {
-				log.Info("http request",
+				attrs := []any{
 					"id", middleware.GetReqID(r.Context()),
 					"method", r.Method,
 					"path", r.URL.Path,
@@ -32,7 +39,11 @@ func requestLogger(log *slog.Logger) func(http.Handler) http.Handler {
 					"bytes", ww.BytesWritten(),
 					"duration", time.Since(start),
 					"remote", r.RemoteAddr,
-				)
+				}
+				if err := capturedErr(); err != nil && ww.Status() >= http.StatusInternalServerError {
+					attrs = append(attrs, "error", err)
+				}
+				log.Info("http request", attrs...)
 			}()
 			next.ServeHTTP(ww, r)
 		})

backend/internal/httpd/log_test.go

@@ -0,0 +1,53 @@
+package httpd
+
+import (
+	"bytes"
+	"errors"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/aoagents/agent-orchestrator/backend/internal/httpd/apierr"
+	"github.com/aoagents/agent-orchestrator/backend/internal/httpd/envelope"
+)
+
+// TestRequestLoggerRecords5xxCause: the wire envelope collapses unrecognized
+// service errors into "Internal server error", so the access log line is the
+// only place the cause can survive. A 500 must carry it; a typed 4xx (whose
+// envelope already explains itself) must not.
+func TestRequestLoggerRecords5xxCause(t *testing.T) {
+	cases := []struct {
+		name      string
+		err       error
+		wantInLog string
+		absent    bool
+	}{
+		{name: "raw error on 500 is logged", err: errors.New("gitworktree: worktree remove exploded"), wantInLog: "gitworktree: worktree remove exploded"},
+		{name: "typed 404 carries no error attr", err: apierr.NotFound("SESSION_NOT_FOUND", "Unknown session"), wantInLog: "error=", absent: true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			var buf bytes.Buffer
+			log := slog.New(slog.NewTextHandler(&buf, nil))
+			handler := requestLogger(log)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				envelope.WriteError(w, r, tc.err)
+			}))
+
+			rec := httptest.NewRecorder()
+			handler.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/api/v1/sessions/x/kill", nil))
+
+			got := buf.String()
+			if tc.absent {
+				if strings.Contains(got, tc.wantInLog) {
+					t.Fatalf("log line unexpectedly contains %q:\n%s", tc.wantInLog, got)
+				}
+				return
+			}
+			if !strings.Contains(got, tc.wantInLog) {
+				t.Fatalf("log line missing %q:\n%s", tc.wantInLog, got)
+			}
+		})
+	}
+}

backend/internal/ports/outbound.go

@@ -108,7 +108,7 @@ type Workspace interface {
 	Restore(ctx context.Context, cfg WorkspaceConfig) (WorkspaceInfo, error)
 }
 
-// Workspace-level sentinels surfaced through Create/Restore so the HTTP layer
+// Workspace-level sentinels surfaced through Create/Restore/Destroy so callers
 // can map them to typed errors rather than collapsing every adapter failure
 // into an opaque 500. Adapters wrap these via fmt.Errorf("...: %w", sentinel).
 var (
@@ -118,6 +118,10 @@ var (
 	// ErrWorkspaceBranchNotFetched reports the requested branch exists nowhere
 	// reachable (no local head, no remote-tracking branch, no tag).
 	ErrWorkspaceBranchNotFetched = errors.New("workspace: branch is not fetched")
+	// ErrWorkspaceDirty reports Destroy refused to remove a workspace because
+	// it holds uncommitted changes or untracked files. Teardown is never
+	// forced; callers treat the workspace as intentionally preserved.
+	ErrWorkspaceDirty = errors.New("workspace: uncommitted changes present")
 )
 
 // WorkspaceConfig is the spec for creating or restoring a session's workspace.

backend/internal/service/session/service.go

@@ -40,7 +40,7 @@ type commander interface {
 	Restore(ctx context.Context, id domain.SessionID) (domain.SessionRecord, error)
 	Kill(ctx context.Context, id domain.SessionID) (bool, error)
 	Send(ctx context.Context, id domain.SessionID, message string) error
-	Cleanup(ctx context.Context, project domain.ProjectID) ([]domain.SessionID, error)
+	Cleanup(ctx context.Context, project domain.ProjectID) (sessionmanager.CleanupResult, error)
 	RollbackSpawn(ctx context.Context, id domain.SessionID) (deleted, killed bool, err error)
 }
 
@@ -52,6 +52,19 @@ type RollbackOutcome struct {
 	Killed  bool `json:"killed"`
 }
 
+// CleanupOutcome reports what session cleanup reclaimed and what it preserved.
+type CleanupOutcome struct {
+	Cleaned []domain.SessionID `json:"cleaned"`
+	Skipped []CleanupSkipped   `json:"skipped"`
+}
+
+// CleanupSkipped is one terminal session whose workspace was preserved by
+// cleanup (never force-deleted), with the user-facing reason.
+type CleanupSkipped struct {
+	SessionID domain.SessionID `json:"sessionId"`
+	Reason    string           `json:"reason"`
+}
+
 type scmProvider interface {
 	ParseRepository(remote string) (ports.SCMRepo, bool)
 	FetchPullRequests(ctx context.Context, refs []ports.SCMPRRef) ([]ports.SCMObservation, error)
@@ -211,9 +224,21 @@ func (s *Service) Rename(ctx context.Context, id domain.SessionID, displayName s
 	return nil
 }
 
-// Cleanup delegates terminal workspace cleanup to the internal manager.
-func (s *Service) Cleanup(ctx context.Context, project domain.ProjectID) ([]domain.SessionID, error) {
-	return s.manager.Cleanup(ctx, project)
+// Cleanup delegates terminal workspace cleanup to the internal manager and
+// reports both reclaimed and preserved (skipped) workspaces.
+func (s *Service) Cleanup(ctx context.Context, project domain.ProjectID) (CleanupOutcome, error) {
+	res, err := s.manager.Cleanup(ctx, project)
+	if err != nil {
+		return CleanupOutcome{}, err
+	}
+	out := CleanupOutcome{Cleaned: res.Cleaned, Skipped: make([]CleanupSkipped, 0, len(res.Skipped))}
+	if out.Cleaned == nil {
+		out.Cleaned = []domain.SessionID{}
+	}
+	for _, skip := range res.Skipped {
+		out.Skipped = append(out.Skipped, CleanupSkipped{SessionID: skip.SessionID, Reason: skip.Reason})
+	}
+	return out, nil
 }
 
 // List returns sessions as enriched display models after applying API filters.

backend/internal/service/session/service_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/aoagents/agent-orchestrator/backend/internal/domain"
 	"github.com/aoagents/agent-orchestrator/backend/internal/httpd/apierr"
 	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
+	sessionmanager "github.com/aoagents/agent-orchestrator/backend/internal/session_manager"
 )
 
 type fakeStore struct {
@@ -144,13 +145,32 @@ func (f *fakeCommander) Kill(_ context.Context, id domain.SessionID) (bool, erro
 	return true, nil
 }
 func (f *fakeCommander) Send(context.Context, domain.SessionID, string) error { return nil }
-func (f *fakeCommander) Cleanup(context.Context, domain.ProjectID) ([]domain.SessionID, error) {
-	return nil, nil
+func (f *fakeCommander) Cleanup(context.Context, domain.ProjectID) (sessionmanager.CleanupResult, error) {
+	return sessionmanager.CleanupResult{
+		Cleaned: []domain.SessionID{"mer-1"},
+		Skipped: []sessionmanager.CleanupSkip{{SessionID: "mer-2", Reason: "workspace has uncommitted changes"}},
+	}, nil
 }
 func (f *fakeCommander) RollbackSpawn(context.Context, domain.SessionID) (bool, bool, error) {
 	return false, false, nil
 }
 
+// TestCleanupMapsManagerResult: the service forwards both reclaimed and
+// skipped sessions, with non-nil slices so the wire shape stays stable.
+func TestCleanupMapsManagerResult(t *testing.T) {
+	svc := &Service{manager: &fakeCommander{}}
+	out, err := svc.Cleanup(context.Background(), "mer")
+	if err != nil {
+		t.Fatalf("Cleanup: %v", err)
+	}
+	if len(out.Cleaned) != 1 || out.Cleaned[0] != "mer-1" {
+		t.Fatalf("cleaned = %#v", out.Cleaned)
+	}
+	if len(out.Skipped) != 1 || out.Skipped[0].SessionID != "mer-2" || out.Skipped[0].Reason != "workspace has uncommitted changes" {
+		t.Fatalf("skipped = %#v", out.Skipped)
+	}
+}
+
 func TestSpawnOrchestratorCleanKillsActiveOrchestratorsBeforeSpawn(t *testing.T) {
 	st := newFakeStore()
 	st.projects["mer"] = domain.ProjectRecord{ID: "mer"}

backend/internal/session_manager/manager.go

@@ -362,7 +362,8 @@ func (m *Manager) RollbackSpawn(ctx context.Context, id domain.SessionID) (delet
 
 // Kill records terminal intent with the LCM, then tears down the runtime and
 // workspace. A workspace teardown refused by the worktree-remove safety
-// (uncommitted work) surfaces as an error with freed=false and is never forced.
+// (uncommitted work) is never forced: the session still terminates and Kill
+// succeeds with freed=false, signalling the workspace was preserved.
 func (m *Manager) Kill(ctx context.Context, id domain.SessionID) (bool, error) {
 	rec, ok, err := m.store.GetSession(ctx, id)
 	if err != nil {
@@ -383,6 +384,9 @@ func (m *Manager) Kill(ctx context.Context, id domain.SessionID) (bool, error) {
 		return false, fmt.Errorf("kill %s: runtime: %w", id, err)
 	}
 	if err := m.workspace.Destroy(ctx, ws); err != nil {
+		if errors.Is(err, ports.ErrWorkspaceDirty) {
+			return false, nil
+		}
 		return false, fmt.Errorf("kill %s: workspace: %w", id, err)
 	}
 	return true, nil
@@ -471,14 +475,28 @@ func (m *Manager) Send(ctx context.Context, id domain.SessionID, message string)
 	return nil
 }
 
+// CleanupSkip reports one terminal session whose workspace was preserved
+// rather than reclaimed, and why.
+type CleanupSkip struct {
+	SessionID domain.SessionID
+	Reason    string
+}
+
+// CleanupResult reports what Cleanup reclaimed and what it preserved.
+type CleanupResult struct {
+	Cleaned []domain.SessionID
+	Skipped []CleanupSkip
+}
+
 // Cleanup reclaims the workspaces of terminal sessions in a project. A workspace
-// whose teardown is refused (uncommitted work) is skipped, never forced.
-func (m *Manager) Cleanup(ctx context.Context, project domain.ProjectID) ([]domain.SessionID, error) {
+// whose teardown is refused (uncommitted work) is never forced; it is reported
+// in Skipped with the reason so the refusal is visible instead of silent.
+func (m *Manager) Cleanup(ctx context.Context, project domain.ProjectID) (CleanupResult, error) {
 	recs, err := m.cleanupRecords(ctx, project)
 	if err != nil {
-		return nil, fmt.Errorf("cleanup %s: %w", project, err)
+		return CleanupResult{}, fmt.Errorf("cleanup %s: %w", project, err)
 	}
-	cleaned := make([]domain.SessionID, 0, len(recs))
+	result := CleanupResult{Cleaned: make([]domain.SessionID, 0, len(recs)), Skipped: []CleanupSkip{}}
 	for _, rec := range recs {
 		if !rec.IsTerminated {
 			continue
@@ -491,11 +509,28 @@ func (m *Manager) Cleanup(ctx context.Context, project domain.ProjectID) ([]doma
 			_ = m.runtime.Destroy(ctx, h) // best effort; usually already gone
 		}
 		if err := m.workspace.Destroy(ctx, ws); err != nil {
-			continue // skipped: uncommitted work
+			if !errors.Is(err, ports.ErrWorkspaceDirty) {
+				// The public reason stays a fixed string (the raw error carries
+				// internal filesystem paths); the full cause lands here.
+				m.logger.Warn("cleanup: workspace teardown failed", "sessionID", rec.ID, "path", ws.Path, "error", err)
+			}
+			result.Skipped = append(result.Skipped, CleanupSkip{SessionID: rec.ID, Reason: cleanupSkipReason(err)})
+			continue
 		}
-		cleaned = append(cleaned, rec.ID)
+		result.Cleaned = append(result.Cleaned, rec.ID)
 	}
-	return cleaned, nil
+	return result, nil
+}
+
+// cleanupSkipReason renders a workspace teardown refusal as a short
+// user-facing reason for the cleanup report. Deliberately not the raw error:
+// it flows to the API response and CLI output, and teardown errors embed
+// internal filesystem paths.
+func cleanupSkipReason(err error) string {
+	if errors.Is(err, ports.ErrWorkspaceDirty) {
+		return "workspace has uncommitted changes"
+	}
+	return "workspace teardown failed"
 }
 
 func (m *Manager) cleanupRecords(ctx context.Context, project domain.ProjectID) ([]domain.SessionRecord, error) {

backend/internal/session_manager/manager_test.go

@@ -355,6 +355,41 @@ func TestKill_RefusesIncompleteHandle(t *testing.T) {
 		t.Fatalf("want ErrIncompleteHandle, got %v", err)
 	}
 }
+
+// TestKill_DirtyWorkspaceTerminatesAndPreserves: a workspace teardown refused
+// because of uncommitted work must NOT fail the kill — the session terminates,
+// the runtime is gone, and freed=false reports the preserved worktree. This is
+// the normal path for any session with in-progress changes, so an error here
+// would turn every such kill into a 500.
+func TestKill_DirtyWorkspaceTerminatesAndPreserves(t *testing.T) {
+	m, st, rt, ws := newManager()
+	st.sessions["mer-1"] = mkLive("mer-1")
+	ws.destroyErr = fmt.Errorf("gitworktree: refusing to remove: %w", ports.ErrWorkspaceDirty)
+	freed, err := m.Kill(ctx, "mer-1")
+	if err != nil {
+		t.Fatalf("kill dirty workspace err = %v, want nil", err)
+	}
+	if freed {
+		t.Fatal("freed = true, want false for preserved workspace")
+	}
+	if rt.destroyed != 1 {
+		t.Fatal("runtime should be destroyed")
+	}
+	if !st.sessions["mer-1"].IsTerminated {
+		t.Fatal("session should be terminated")
+	}
+}
+
+// TestKill_OtherWorkspaceErrorStillFails: only the typed dirty refusal is a
+// success-with-preserved-workspace; any other teardown failure keeps erroring.
+func TestKill_OtherWorkspaceErrorStillFails(t *testing.T) {
+	m, st, _, ws := newManager()
+	st.sessions["mer-1"] = mkLive("mer-1")
+	ws.destroyErr = errors.New("disk on fire")
+	if _, err := m.Kill(ctx, "mer-1"); err == nil || !strings.Contains(err.Error(), "disk on fire") {
+		t.Fatalf("kill err = %v, want workspace error surfaced", err)
+	}
+}
 func TestRestore_ReopensTerminal(t *testing.T) {
 	m, st, rt, _ := newManager()
 	seedTerminal(st, "mer-1", domain.SessionMetadata{WorkspacePath: "/ws/mer-1", Branch: "b", AgentSessionID: "agent-x"})
@@ -396,18 +431,58 @@ func TestCleanup_ReclaimsTerminalWorkspaces(t *testing.T) {
 	m, st, _, ws := newManager()
 	seedTerminal(st, "mer-1", domain.SessionMetadata{WorkspacePath: "/ws/mer-1"})
 	st.sessions["mer-2"] = mkLive("mer-2")
-	cleaned, err := m.Cleanup(ctx, "mer")
+	res, err := m.Cleanup(ctx, "mer")
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(cleaned) != 1 || cleaned[0] != "mer-1" {
-		t.Fatalf("got %v", cleaned)
+	if len(res.Cleaned) != 1 || res.Cleaned[0] != "mer-1" {
+		t.Fatalf("got %v", res.Cleaned)
+	}
+	if len(res.Skipped) != 0 {
+		t.Fatalf("skipped = %v, want none", res.Skipped)
 	}
 	if ws.destroyed != 1 {
 		t.Fatal("live workspace must not be destroyed")
 	}
 }
 
+// TestCleanup_ReportsSkippedWorkspaces: a refused teardown must be visible in
+// the result with a reason — a silent skip leaves users staring at
+// "Would clean N … 0 sessions cleaned" with no explanation.
+func TestCleanup_ReportsSkippedWorkspaces(t *testing.T) {
+	m, st, _, ws := newManager()
+	seedTerminal(st, "mer-1", domain.SessionMetadata{WorkspacePath: "/ws/mer-1"})
+	ws.destroyErr = fmt.Errorf("gitworktree: refusing to remove: %w", ports.ErrWorkspaceDirty)
+	res, err := m.Cleanup(ctx, "mer")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(res.Cleaned) != 0 {
+		t.Fatalf("cleaned = %v, want none", res.Cleaned)
+	}
+	if len(res.Skipped) != 1 || res.Skipped[0].SessionID != "mer-1" {
+		t.Fatalf("skipped = %v, want mer-1", res.Skipped)
+	}
+	if res.Skipped[0].Reason != "workspace has uncommitted changes" {
+		t.Fatalf("reason = %q", res.Skipped[0].Reason)
+	}
+
+	// A non-dirty teardown failure is reported too — but with a fixed public
+	// reason: the raw cause carries internal filesystem paths and belongs in
+	// the server log, not the API response.
+	ws.destroyErr = errors.New("disk on fire")
+	res, err = m.Cleanup(ctx, "mer")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(res.Skipped) != 1 || res.Skipped[0].Reason != "workspace teardown failed" {
+		t.Fatalf("skipped = %v, want fixed teardown-failed reason", res.Skipped)
+	}
+	if strings.Contains(res.Skipped[0].Reason, "disk on fire") {
+		t.Fatalf("raw internal error leaked into public reason: %q", res.Skipped[0].Reason)
+	}
+}
+
 func TestSpawn_DefaultsBranchFromSessionID(t *testing.T) {
 	m, st, _, _ := newManager()
 	s, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker})

docs/agent/README.md

@@ -40,6 +40,7 @@ Implementation layout:
 
 - Agent-specific hook installation should live beside the agent adapter in `backend/internal/adapters/agent/<agent>/hooks.go`; the hook commands are defined in code, not embedded template files.
 - Launch, restore, and session-info behavior can stay in the main agent implementation unless the file grows enough to justify another split.
+- Every file an adapter writes into the session worktree must be covered by a sibling self-ignoring `.gitignore` written via `hookutil.EnsureWorkspaceGitignore`. Hook files are untracked, and `git worktree remove` (never run with `--force`) refuses on any untracked file — an uncovered hook file makes every session workspace permanently undeletable. The registry conformance test (`registry.TestGetAgentHooksFootprintIsGitignored`) enforces this for all adapters.
 
 ## Metadata Keys
 

frontend/src/api/schema.ts

@@ -353,6 +353,11 @@ export interface components {
         CleanupSessionsResponse: {
             cleaned: string[];
             ok: boolean;
+            skipped: components["schemas"]["CleanupSkippedSession"][];
+        };
+        CleanupSkippedSession: {
+            reason: string;
+            sessionId: string;
         };
         DegradedProject: {
             id: string;