feat(tui): add plan mode command and fix plan file editing

internal/agent/loop.go

@@ -1536,6 +1536,7 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
 		// the Run turn loop performs the actual provider switch. Empty for every
 		// ordinary tool result.
 		RequestedModel: result.Meta["escalate_to_model"],
+		PlanSnapshot:   result.PlanSnapshot,
 	}, nil
 }
 
@@ -1867,15 +1868,15 @@ func toolResultFromPrePermissionReject(call ToolCall, result tools.Result) ToolR
 	}
 }
 
-// hooksSuppressed reports whether advisory (non-veto) hooks must not run for
-// this run's permission mode. Plan mode promises a read-only turn, but
-// sessionStart/sessionEnd/afterTool hooks execute configured host commands
-// outside the advertised-tool and sandbox gates, so dispatching them would let
-// merely starting a plan session or finishing a read mutate the workspace.
+// hooksSuppressed reports whether lifecycle and afterTool hooks must not run
+// for this run's permission mode. Plan mode promises a read-only turn, but
+// sessionStart, sessionEnd, and afterTool hooks execute configured host
+// commands outside the advertised-tool and sandbox gates, so dispatching them
+// would let merely starting or finishing a plan session mutate the workspace
+// or spawn processes.
 //
-// beforeTool is intentionally NOT suppressed: a non-zero exit is a deny gate,
-// and skipping it fails open (operators who block secret-file reads via
-// beforeTool would lose that protection under /plan on). See dispatchBeforeTool.
+// beforeTool is intentionally not gated here: fail-closed policy vetoes must
+// still apply to read-only plan-mode calls (see dispatchBeforeTool).
 //
 // Spec-draft keeps the existing trust-gated hook model: project hooks still
 // fire when the workspace (or its worktree trust root) is trusted. That is

internal/agent/plan_mode_advertised_test.go

@@ -0,0 +1,73 @@
+package agent
+
+import (
+	"context"
+	"testing"
+
+	"github.com/Gitlawb/zero/internal/tools"
+	"github.com/Gitlawb/zero/internal/zeroruntime"
+)
+
+// TestToolAdvertisedInPlanExcludesRequestPermissions guards against
+// request_permissions leaking into plan mode's read-only allowlist. It is
+// classified SideEffectNone + PermissionAllow (control-only, no filesystem or
+// network access of its own), but tools.ToolAdvertisedForPermissionMode only
+// admits SideEffectRead + PermissionAllow tools (plus no process-spawning
+// exceptions) for plan mode. SideEffectNone tools are therefore excluded,
+// including request_permissions.
+func TestToolAdvertisedInPlanExcludesRequestPermissions(t *testing.T) {
+	if tools.ToolAdvertisedForPermissionMode(tools.NewRequestPermissionsTool(), tools.PlanMode) {
+		t.Fatal("request_permissions must not be advertised in plan mode: it would let the model obtain a user-approved permission grant during a supposedly read-only planning turn, which then outlives plan mode")
+	}
+}
+
+// TestRunRejectsRequestPermissionsInPlanMode exercises the same guarantee
+// end-to-end: a model that calls request_permissions while PermissionModePlan
+// is active gets a dispatch-time rejection, never a permission prompt.
+func TestRunRejectsRequestPermissionsInPlanMode(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewRequestPermissionsTool())
+	provider := &mockProvider{
+		turns: [][]zeroruntime.StreamEvent{
+			{
+				{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "request_permissions"},
+				{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"permissions":{"network":true}}`},
+				{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"},
+				{Type: zeroruntime.StreamEventDone},
+			},
+			{
+				{Type: zeroruntime.StreamEventText, Content: "done"},
+				{Type: zeroruntime.StreamEventDone},
+			},
+		},
+	}
+	var requests []PermissionRequest
+
+	result, err := Run(context.Background(), "plan the change", provider, Options{
+		Registry:       registry,
+		PermissionMode: PermissionModePlan,
+		OnPermissionRequest: func(_ context.Context, request PermissionRequest) (PermissionDecision, error) {
+			requests = append(requests, request)
+			return PermissionDecision{Action: PermissionDecisionDeny, Reason: "unexpected permission request"}, nil
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if result.FinalAnswer != "done" {
+		t.Fatalf("final answer = %q", result.FinalAnswer)
+	}
+	if len(requests) != 0 {
+		t.Fatalf("expected no permission request while in plan mode, got %#v", requests)
+	}
+	if len(provider.requests) < 2 {
+		t.Fatalf("expected tool result to be sent back to provider, got %d requests", len(provider.requests))
+	}
+	lastMessage := provider.requests[1].Messages[len(provider.requests[1].Messages)-1]
+	if lastMessage.ToolCallID != "call-1" {
+		t.Fatalf("expected tool result message for call-1, got %#v", lastMessage)
+	}
+	if want := `Error: Tool "request_permissions" is not available in plan mode.`; lastMessage.Content != want {
+		t.Fatalf("tool result content = %q, want %q", lastMessage.Content, want)
+	}
+}

internal/agent/request_permissions_test.go

@@ -120,6 +120,37 @@ func TestRequestPermissionsTurnGrantAllowsLaterToolAndCleansUp(t *testing.T) {
 	}
 }
 
+// TestRequestPermissionsDeniedInPlanModeEvenWithoutRegistryEntry guards the
+// defense-in-depth check in executeRequestPermissions: the registry-based
+// ToolAdvertised gate in executeToolCall only fires when the tool is found in
+// whatever registry the caller passed in, but request_permissions is
+// dispatched by name regardless of registry contents. A registry that omits
+// the tool (e.g. a reduced/specialist registry) must not let a plan-mode turn
+// slip through to a real, outliving sandbox grant.
+func TestRequestPermissionsDeniedInPlanModeEvenWithoutRegistryEntry(t *testing.T) {
+	registry := tools.NewRegistry() // deliberately does not register RequestPermissionsTool
+	promptCalled := false
+	result, err := executeToolCall(context.Background(), registry, ToolCall{
+		ID:        "grant-1",
+		Name:      tools.RequestPermissionsToolName,
+		Arguments: `{"reason":"try to escape plan mode","permissions":{"file_system":{"write":["/tmp"]}}}`,
+	}, PermissionModePlan, Options{
+		OnPermissionRequest: func(_ context.Context, _ PermissionRequest) (PermissionDecision, error) {
+			promptCalled = true
+			return PermissionDecision{Action: PermissionDecisionAllow}, nil
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if promptCalled {
+		t.Fatal("request_permissions must not reach the permission prompt in plan mode, registry entry or not")
+	}
+	if result.Status != tools.StatusError || !strings.Contains(result.Output, "not available in plan mode") {
+		t.Fatalf("result = %#v, want a plan-mode denial error", result)
+	}
+}
+
 func tempDirOutsideDefaultTemp(t *testing.T) string {
 	t.Helper()
 	dir, err := os.MkdirTemp(".", ".zero-sandbox-outside-")

internal/agent/types.go

@@ -107,6 +107,9 @@ type ToolResult struct {
 	// for every normal tool result; the Run loop performs the switch when it is
 	// set and Options.ModelSwitcher is wired.
 	RequestedModel string
+	// PlanSnapshot carries the typed, immutable snapshot of the []PlanItem
+	// accepted by a successful update_plan call, untampered by transcript scrubbing.
+	PlanSnapshot []tools.PlanItem `json:"-"`
 }
 
 // ModelOutput returns the bounded provider-facing result while preserving

internal/planmode/export_test.go

@@ -0,0 +1,12 @@
+package planmode
+
+import "testing"
+
+// SetTempDirForTest overrides the temp dir func for unit tests. Kept in
+// export_test.go so the production planmode package (and therefore cmd/zero)
+// does not import testing.
+func SetTempDirForTest(t *testing.T, tempDir string) {
+	t.Helper()
+	restore := SetEffectiveTempDirForTest(tempDir)
+	t.Cleanup(restore)
+}

internal/planmode/fifo_other_test.go

@@ -0,0 +1,9 @@
+//go:build !unix
+
+package planmode
+
+import "fmt"
+
+func mkfifoForTest(path string) error {
+	return fmt.Errorf("mkfifo not available on this platform")
+}

internal/planmode/fifo_unix_test.go

@@ -0,0 +1,9 @@
+//go:build unix
+
+package planmode
+
+import "syscall"
+
+func mkfifoForTest(path string) error {
+	return syscall.Mkfifo(path, 0o600)
+}

internal/planmode/physical_other.go

@@ -0,0 +1,15 @@
+//go:build !windows
+
+package planmode
+
+import "path/filepath"
+
+// resolvePhysical returns path with every symlink component resolved. On
+// non-Windows systems filepath.EvalSymlinks resolves every link type the
+// platform has, so it is the whole implementation.
+func resolvePhysical(path string) (string, error) {
+	return filepath.EvalSymlinks(path)
+}
+
+// pathIsReparsePoint is a Windows concept; nothing here reports one.
+func pathIsReparsePoint(string) bool { return false }

internal/planmode/physical_windows.go

@@ -0,0 +1,120 @@
+//go:build windows
+
+package planmode
+
+import (
+	"errors"
+	"fmt"
+	"path/filepath"
+	"strings"
+
+	"golang.org/x/sys/windows"
+)
+
+// resolvePhysical returns path in its canonical physical spelling.
+//
+// filepath.EvalSymlinks cannot do this alone on Windows. It resolves name
+// surrogates (directory symlinks) but not junctions, which os.Lstat reports as
+// os.ModeIrregular rather than os.ModeSymlink, so EvalSymlinks hands a junction
+// straight back. A junction needs no SeCreateSymbolicLinkPrivilege, so it is
+// the reparse point an unprivileged process can actually plant, and treating
+// one as its own physical path lets a staging directory that really lands in
+// the workspace or the OS temp directory compare as though it sits outside
+// both.
+//
+// GetFinalPathNameByHandle asks the filesystem what the open handle resolved
+// to, which is the only answer that accounts for every reparse type at once.
+// VOLUME_NAME_DOS also returns long names, so it subsumes the 8.3 short-name
+// normalization (RUNNER~1) the caller needs anyway.
+func resolvePhysical(path string) (string, error) {
+	absolute, err := filepath.Abs(path)
+	if err != nil {
+		return "", err
+	}
+	pathUTF16, err := windows.UTF16PtrFromString(absolute)
+	if err != nil {
+		return "", err
+	}
+	// FILE_FLAG_BACKUP_SEMANTICS is required to open a directory handle, and
+	// no reparse flag is passed precisely so the open follows to the target
+	// this call is asking about.
+	handle, err := windows.CreateFile(
+		pathUTF16,
+		0, // Query the name only; no read or write access is needed.
+		windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE,
+		nil,
+		windows.OPEN_EXISTING,
+		windows.FILE_FLAG_BACKUP_SEMANTICS,
+		0,
+	)
+	if err != nil {
+		return "", err
+	}
+	defer windows.CloseHandle(handle)
+
+	return finalPathName(handle)
+}
+
+// fileNameNormalized|volumeNameDOS is the GetFinalPathNameByHandle flag pair
+// that asks for the normalized (long-name) path with a drive letter. Both are
+// zero, and x/sys/windows does not export either, so they are named here
+// rather than left as a bare literal.
+const (
+	fileNameNormalized = 0x0
+	volumeNameDOS      = 0x0
+)
+
+// finalPathName reads the resolved path off an open handle, growing the buffer
+// if the path is longer than MAX_PATH (a resolved path can be, which is why
+// the API reports the size it needs).
+func finalPathName(handle windows.Handle) (string, error) {
+	buf := make([]uint16, windows.MAX_PATH)
+	for range 2 {
+		// On success n excludes the terminating NUL; when the buffer is too
+		// small n is the required size INCLUDING it, so n >= len(buf) is the
+		// signal to grow rather than a result.
+		n, err := windows.GetFinalPathNameByHandle(handle, &buf[0], uint32(len(buf)), fileNameNormalized|volumeNameDOS)
+		if err != nil {
+			return "", err
+		}
+		if n < uint32(len(buf)) {
+			return trimExtendedLengthPrefix(windows.UTF16ToString(buf[:n])), nil
+		}
+		if n > windows.MAX_LONG_PATH {
+			return "", fmt.Errorf("resolved path needs %d UTF-16 units, over the %d limit", n, windows.MAX_LONG_PATH)
+		}
+		buf = make([]uint16, n)
+	}
+	return "", errors.New("resolved path length kept growing between calls")
+}
+
+// trimExtendedLengthPrefix converts the extended-length spelling
+// GetFinalPathNameByHandle returns back to the ordinary Win32 form, so the
+// result compares against paths spelled the way the rest of the process
+// spells them. `\\?\UNC\server\share` is a UNC path, not a drive path, and
+// has to become `\\server\share` rather than `UNC\server\share`.
+func trimExtendedLengthPrefix(path string) string {
+	if rest, ok := strings.CutPrefix(path, `\\?\UNC\`); ok {
+		return `\\` + rest
+	}
+	if rest, ok := strings.CutPrefix(path, `\\?\`); ok {
+		return rest
+	}
+	return path
+}
+
+// pathIsReparsePoint reports whether path itself is a reparse point of any
+// kind, junctions included. os.Lstat cannot answer this: it maps a junction to
+// os.ModeIrregular, which is indistinguishable from other irregular files, so
+// verifyPrivateDirectory's os.ModeSymlink test never fires for one.
+func pathIsReparsePoint(path string) bool {
+	pathUTF16, err := windows.UTF16PtrFromString(path)
+	if err != nil {
+		return false
+	}
+	attrs, err := windows.GetFileAttributes(pathUTF16)
+	if err != nil {
+		return false
+	}
+	return attrs&windows.FILE_ATTRIBUTE_REPARSE_POINT != 0
+}