FEAT: review and approve file changes with numbered diffs

README.md

@@ -1595,6 +1595,24 @@ visually marked with a vertical rail. Formatting uses validated structured
 fields rather than guessing command boundaries from the justification. Legacy
 or incomplete requests retain their original bounded text.
 
+**File-change approvals (shared app-server only):** proposed additions, deletions
+and updates are shown as scrollable patches, with file paths, rename destinations,
+old/new line numbers and green additions/red removals. An overall `+N / −N`
+summary counts added and removed source lines across all proposed files (not
+wrapped display rows or patch headers). Full detail retains the
+preceding commentary and justification. The terminal and writable web interface
+offer **APPROVE ONCE** (with confirmation), **DECLINE**, and **REJECT & STOP TURN**.
+Read-only web mode can display the patch but cannot answer it. Inline terminal
+buttons appear only when the entire proposed patch fits; otherwise open full detail.
+
+The patch must come from the matching live thread, turn and item; Codexometer
+never reconstructs approval details from files on disk. Patches are memory-only,
+bounded to 64 KiB of JSON and 64 files per item. Missing, oversized, unsafe or
+changed patches fail closed: review them in Codex (a fresh matching approval
+request can restore controls). Directory-root grants remain in Codex because
+their app-server semantics are unstable. File approvals do not offer session-wide
+or persistent permission grants.
+
 When the approval event omits the command or
 directory, Codexometer associates it with the preceding command item from the
 same thread, turn and item. A complete ordinary command request offers clickable
@@ -1692,14 +1710,14 @@ acknowledgement animations remain confined to full detail.
 Local rollout logs do **not** persist Codex's approval-request events, so a local
 preview can show only the message preceding an approval. `INPUT NEEDED` or
 `CHECK SESSION` alone never enables these controls. Requests with missing,
-truncated or sanitised-away details, network approvals, file changes, permission
+truncated or sanitised-away details, network approvals, incomplete file changes, permission
 grants and other unsupported requests remain **REPLY IN CODEX**. The complete
 eligible request is available in the scrollable detail, not just the compact
 two-line preview. Unsupported requests may have only a bounded excerpt.
 
 When controls are unavailable, the detail page explains why beside
 **REPLY IN CODEX**: for example, missing command/directory or request identity,
-additional permissions, network/file-change approval, no supported decisions,
+additional permissions, network approval, unavailable complete file changes, no supported decisions,
 truncated or sanitised text, local-only observation, or a resolved/disconnected
 request. Eligible requests on small terminals instead explain that the terminal
 must be enlarged. These diagnostics do not relax any approval safeguards.

internal/codex/file_approval.go

@@ -0,0 +1,211 @@
+package codex
+
+import (
+	"bytes"
+	"crypto/rand"
+	"encoding/json"
+	"fmt"
+	"regexp"
+	"strconv"
+	"strings"
+	"unicode"
+	"unicode/utf8"
+)
+
+// Patches stay in memory, scoped to an exact live item. Never read the local
+// filesystem to reconstruct what a remote approval might authorise.
+const fileApprovalLimit = 64 * 1024
+
+type FileChange struct {
+	Path string `json:"path"`
+	Kind struct {
+		Type string `json:"type"`
+		// PatchChangeKind's Update variant uses snake_case on the wire,
+		// unlike outer request fields. Verified against Codex 0.160.1's
+		// generated JSON schema; do not infer movePath from threadId.
+		MovePath string `json:"move_path"`
+	} `json:"kind"`
+	Diff string `json:"diff"`
+}
+
+func safePatchText(s string) bool {
+	if !utf8.ValidString(s) {
+		return false
+	}
+	for _, r := range s {
+		if (unicode.IsControl(r) && r != '\n' && r != '\t') || unicode.Is(unicode.Cf, r) {
+			return false
+		}
+	}
+	return true
+}
+
+func validatedFileChanges(raw json.RawMessage) string {
+	if len(raw) > fileApprovalLimit || !utf8.Valid(raw) {
+		return ""
+	}
+	var changes []FileChange
+	decoder := json.NewDecoder(bytes.NewReader(raw))
+	decoder.DisallowUnknownFields()
+	if decoder.Decode(&changes) != nil || len(changes) == 0 || len(changes) > 64 {
+		return ""
+	}
+	var fields []map[string]json.RawMessage
+	if json.Unmarshal(raw, &fields) != nil {
+		return ""
+	}
+	for _, f := range fields {
+		if len(f["diff"]) == 0 || string(f["diff"]) == "null" {
+			return ""
+		}
+	}
+	for _, c := range changes {
+		if strings.TrimSpace(c.Path) == "" || strings.ContainsAny(c.Path+c.Kind.MovePath, "\n\t") || !safePatchText(c.Path+c.Kind.MovePath+c.Diff) {
+			return ""
+		}
+		switch c.Kind.Type {
+		case "add", "delete", "update":
+		default:
+			return ""
+		}
+	}
+	data, _ := json.Marshal(changes)
+	return string(data)
+}
+
+func configureFileApproval(c *SessionContext, grantRoot string, state *daemonContextState) {
+	c.ApprovalToken = ""
+	c.ApprovalOptions = [8]ApprovalOption{}
+	c.ApprovalBlocked = "file-change"
+	if c.FileChanges == "" {
+		return
+	}
+	if c.TurnID == "" || c.ItemID == "" || state.ended || state.activeTurn != "" && c.TurnID != state.activeTurn {
+		c.ApprovalBlocked = "missing-identity"
+		return
+	}
+	// Root grants have unstable semantics. Keep them in Codex, rather than
+	// suggesting that approval once authorises only the displayed patch.
+	if grantRoot != "" {
+		c.ApprovalBlocked = "permissions"
+		return
+	}
+	if SanitizeSessionContext(c.Text) != c.Text {
+		c.ApprovalBlocked = "sanitised"
+		return
+	}
+	c.ApprovalOptions, c.ApprovalDecisions = commandApprovalOptions([]json.RawMessage{json.RawMessage(`"accept"`), json.RawMessage(`"decline"`), json.RawMessage(`"cancel"`)})
+	c.ApprovalBlocked, c.ApprovalToken = "", rand.Text()
+}
+
+func (s *daemonContextState) capturePatch(turn, item string, raw json.RawMessage) {
+	if turn == "" || item == "" || s.ended || s.activeTurn != "" && turn != s.activeTurn {
+		return
+	}
+	if s.patches == nil {
+		s.patches = map[string]string{}
+	}
+	key := turn + "/" + item
+	patch := validatedFileChanges(raw)
+	if _, exists := s.patches[key]; exists || len(s.patches) < 16 {
+		s.patches[key] = patch
+	}
+	// Any changed patch invalidates an already displayed/confirmed capability.
+	// A new approval request is required; never silently broaden a grant.
+	for id, c := range s.requests {
+		if c.TurnID == turn && c.ItemID == item && c.Kind == SessionContextApproval && c.FileChanges != patch {
+			c.FileChanges, c.ApprovalToken, c.ApprovalBlocked = patch, "", "file-change"
+			s.requests[id] = c
+		}
+	}
+}
+
+type FileDiffLine struct {
+	Text string `json:"text"`
+	Kind string `json:"kind"`
+	Old  int    `json:"old,omitempty"`
+	New  int    `json:"new,omitempty"`
+}
+
+// Count logical changed lines, not wrapped display rows or patch headers.
+func FileDiffTotals(lines []FileDiffLine) (added, removed int) {
+	for _, line := range lines {
+		switch line.Kind {
+		case "addition":
+			added++
+		case "removal":
+			removed++
+		}
+	}
+	return
+}
+
+var diffHunk = regexp.MustCompile(`^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@`)
+
+// FileDiffLines is shared by both front ends. Add/delete diffs are whole-file
+// content; update diffs are unified patches. Prefixes are never inferred from
+// file contents for add/delete operations.
+func FileDiffLines(encoded string) (out []FileDiffLine) {
+	var changes []FileChange
+	if json.Unmarshal([]byte(encoded), &changes) != nil {
+		return nil
+	}
+	for _, c := range changes {
+		out = append(out, FileDiffLine{Text: strings.ToUpper(c.Kind.Type) + " // " + c.Path, Kind: "heading"})
+		if c.Kind.MovePath != "" {
+			out = append(out, FileDiffLine{Text: "→ " + c.Kind.MovePath, Kind: "heading"})
+		}
+		if c.Diff == "" {
+			continue
+		}
+		old, next := 0, 0
+		if c.Kind.Type == "add" {
+			next = 1
+		}
+		if c.Kind.Type == "delete" {
+			old = 1
+		}
+		for _, text := range strings.Split(strings.TrimSuffix(c.Diff, "\n"), "\n") {
+			line := FileDiffLine{Text: strings.ReplaceAll(text, "\t", "    "), Kind: "body"}
+			switch {
+			case c.Kind.Type == "add":
+				line.Kind, line.New, line.Text = "addition", next, "+"+line.Text
+				next++
+			case c.Kind.Type == "delete":
+				line.Kind, line.Old, line.Text = "removal", old, "-"+line.Text
+				old++
+			case diffHunk.MatchString(text):
+				m := diffHunk.FindStringSubmatch(text)
+				old, _ = strconv.Atoi(m[1])
+				next, _ = strconv.Atoi(m[2])
+				line.Kind = "metadata"
+			case strings.HasPrefix(text, "+") && next > 0:
+				line.Kind, line.New = "addition", next
+				next++
+			case strings.HasPrefix(text, "-") && old > 0:
+				line.Kind, line.Old = "removal", old
+				old++
+			case strings.HasPrefix(text, " ") && (old > 0 || next > 0):
+				line.Old, line.New = old, next
+				old++
+				next++
+			}
+			out = append(out, line)
+		}
+	}
+	return out
+}
+
+func (l FileDiffLine) NumberedText() string {
+	if l.Kind == "heading" || l.Kind == "metadata" {
+		return l.Text
+	}
+	old, next := "", ""
+	if l.Old > 0 {
+		old = strconv.Itoa(l.Old)
+	}
+	if l.New > 0 {
+		next = strconv.Itoa(l.New)
+	}
+	return fmt.Sprintf("%5s %5s │ %s", old, next, l.Text)
+}

internal/codex/file_approval_test.go

@@ -0,0 +1,135 @@
+package codex
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+	"time"
+)
+
+const testFilePatch = `[{"path":"/work/a.go","kind":{"type":"update","move_path":null},"diff":"@@ -3,2 +3,2 @@\n-old\n+new\n context\n"}]`
+
+func TestFileApprovalRenameWireShape(t *testing.T) {
+	// Matches PatchChangeKind in `codex app-server generate-json-schema`
+	// from codex-cli 0.160.1: the nested variant field is move_path.
+	patch := strings.Replace(testFilePatch, `"move_path":null`, `"move_path":"/work/renamed.go"`, 1)
+	states, c := fileApprovalFixture(t, patch, nil)
+	if c.ApprovalToken == "" || c.ApprovalBlocked != "" {
+		t.Fatalf("valid rename rejected: %+v", c)
+	}
+	lines := FileDiffLines(c.FileChanges)
+	if len(lines) < 2 || lines[0].Text != "UPDATE // /work/a.go" || lines[1].Text != "→ /work/renamed.go" {
+		t.Fatalf("rename source/destination missing: %+v", lines)
+	}
+	changed := strings.Replace(patch, "/work/renamed.go", "/work/another.go", 1)
+	states["root"].capturePatch("turn", "patch", json.RawMessage(changed))
+	if got := states["root"].requests[`"req"`]; got.ApprovalToken != "" || got.FileChanges == c.FileChanges {
+		t.Fatal("destination change retained old approval", got)
+	}
+	for _, invalid := range []string{
+		strings.Replace(patch, "move_path", "movePath", 1),
+		strings.Replace(patch, "/work/renamed.go", `/work/\u202Erenamed.go`, 1),
+	} {
+		_, blocked := fileApprovalFixture(t, invalid, nil)
+		if blocked.ApprovalToken != "" || blocked.FileChanges != "" {
+			t.Fatal("unknown or unsafe rename accepted", blocked)
+		}
+	}
+}
+
+func fileApprovalFixture(t *testing.T, patch string, overrides map[string]any) (map[string]*daemonContextState, SessionContext) {
+	t.Helper()
+	states := map[string]*daemonContextState{}
+	daemonContextEvent(states, "item/started", nil, json.RawMessage(`{"threadId":"root","turnId":"turn","item":{"type":"fileChange","id":"patch","changes":`+patch+`}}`), time.Now())
+	p := map[string]any{"threadId": "root", "turnId": "turn", "itemId": "patch", "reason": "Update the implementation"}
+	for k, v := range overrides {
+		p[k] = v
+	}
+	raw, _ := json.Marshal(p)
+	daemonContextEvent(states, "item/fileChange/requestApproval", json.RawMessage(`"req"`), raw, time.Now())
+	return states, states["root"].requests[`"req"`]
+}
+
+func TestFileApprovalCorrelationAndSafety(t *testing.T) {
+	_, c := fileApprovalFixture(t, testFilePatch, nil)
+	if c.ApprovalToken == "" || c.FileChanges == "" || c.ApprovalBlocked != "" {
+		t.Fatalf("blocked complete patch: %+v", c)
+	}
+	for i, want := range []string{"accept", "decline", "cancel"} {
+		if c.ApprovalOptions[i].Wire != `"`+want+`"` {
+			t.Fatal(c.ApprovalOptions)
+		}
+	}
+	for name, p := range map[string]string{
+		"missing": "null", "empty": "[]", "invalid": "{}",
+		"no diff":       `[{"path":"a","kind":{"type":"add"}}]`,
+		"null diff":     `[{"path":"a","kind":{"type":"add"},"diff":null}]`,
+		"unknown kind":  strings.Replace(testFilePatch, `"update"`, `"future"`, 1),
+		"unknown field": strings.Replace(testFilePatch, `"path":`, `"permissions":true,"path":`, 1),
+		"escape":        strings.Replace(testFilePatch, "old", `\u001b[31mold`, 1),
+		"bidi":          strings.Replace(testFilePatch, "old", `\u202eold`, 1),
+		"large":         strings.Replace(testFilePatch, "old", strings.Repeat("x", fileApprovalLimit), 1),
+	} {
+		t.Run(name, func(t *testing.T) {
+			_, c := fileApprovalFixture(t, p, nil)
+			if c.ApprovalToken != "" || c.FileChanges != "" {
+				t.Fatal("unsafe patch actionable", c)
+			}
+		})
+	}
+	for _, fields := range []map[string]any{{"turnId": "other"}, {"itemId": "other"}, {"grantRoot": "/work"}, {"reason": "hidden\x1b[31m"}} {
+		_, c := fileApprovalFixture(t, testFilePatch, fields)
+		if c.ApprovalToken != "" {
+			t.Fatal("unsafe request actionable", c)
+		}
+	}
+}
+
+func TestFileApprovalPatchChangeAndLifecycle(t *testing.T) {
+	states, c := fileApprovalFixture(t, testFilePatch, nil)
+	changed := strings.Replace(testFilePatch, "new", "replacement", 1)
+	daemonContextEvent(states, "item/fileChange/patchUpdated", nil, json.RawMessage(`{"threadId":"root","turnId":"turn","itemId":"patch","changes":`+changed+`}`), time.Now())
+	if got := states["root"].requests[`"req"`]; got.ApprovalToken != "" || got.FileChanges == c.FileChanges {
+		t.Fatal("old grant survived patch change", got)
+	}
+	for _, method := range []string{"turn/started", "turn/completed", "turn/interrupted", "item/completed", "thread/closed"} {
+		states, _ := fileApprovalFixture(t, testFilePatch, nil)
+		daemonContextEvent(states, method, nil, json.RawMessage(`{"threadId":"root","turnId":"turn","turn":{"id":"next"},"item":{"id":"patch","type":"fileChange"}}`), time.Now())
+		if s := states["root"]; s != nil && (len(s.requests) != 0 || len(s.patches) != 0) {
+			t.Fatal(method, "retained patch")
+		}
+	}
+}
+
+func TestFileDiffNumbering(t *testing.T) {
+	lines := FileDiffLines(testFilePatch)
+	if len(lines) != 5 || lines[2].Old != 3 || lines[2].Kind != "removal" || lines[3].New != 3 || lines[3].Kind != "addition" || lines[4].Old != 4 || lines[4].New != 4 {
+		t.Fatalf("bad numbers: %+v", lines)
+	}
+	for _, kind := range []string{"add", "delete"} {
+		lines := FileDiffLines(`[{"path":"a","kind":{"type":"` + kind + `"},"diff":"+literal\n\tindented\n"}]`)
+		if len(lines) != 3 || !strings.Contains(lines[1].Text, "+literal") || !strings.Contains(lines[2].Text, "    indented") {
+			t.Fatal(lines)
+		}
+		if kind == "add" && lines[2].New != 2 || kind == "delete" && lines[2].Old != 2 {
+			t.Fatal(lines)
+		}
+	}
+}
+
+func TestFileDiffTotals(t *testing.T) {
+	patch := `[
+	{"path":"updated","kind":{"type":"update"},"diff":"--- a/updated\n+++ b/updated\n@@ -3,2 +3,2 @@\n-old\n+new\n context\n@@ -10 +10 @@\n-last\n+replacement\n\\ No newline at end of file\n"},
+	{"path":"added","kind":{"type":"add"},"diff":"first\n\n+literal\n"},
+	{"path":"deleted","kind":{"type":"delete"},"diff":"first\nsecond"},
+	{"path":"empty","kind":{"type":"add"},"diff":""},
+	{"path":"renamed","kind":{"type":"update","move_path":"destination"},"diff":""}
+	]`
+	added, removed := FileDiffTotals(FileDiffLines(patch))
+	if added != 5 || removed != 4 {
+		t.Fatalf("got +%d / -%d; want +5 / -4", added, removed)
+	}
+	if a, r := FileDiffTotals(nil); a != 0 || r != 0 {
+		t.Fatal("empty diff has changes")
+	}
+}