acp: treat disabled mode negotiation as read-only permissions

Author: wesm

internal/agent/acp.go

@@ -869,10 +869,13 @@ func (c *acpClient) RequestPermission(ctx context.Context, params acp.RequestPer
 		}, nil
 	}
 
-	// Apply permission logic based on agent mode
+	// Apply permission logic based on effective permission mode.
+	// When session mode negotiation is disabled (Mode == ""), keep
+	// permission behavior in read-only mode by default.
+	effectiveMode := c.agent.effectivePermissionMode()
 
 	// In read-only mode, deny all destructive operations.
-	if c.agent.Mode == c.agent.ReadOnlyMode {
+	if effectiveMode == c.agent.ReadOnlyMode {
 		if isDestructive {
 			return acp.RequestPermissionResponse{
 				Outcome: selectPermissionOutcome(params.Options, false),
@@ -1410,6 +1413,13 @@ func (a *ACPAgent) mutatingOperationsAllowed() bool {
 	return a.AutoApproveMode != "" && a.Mode == a.AutoApproveMode
 }
 
+func (a *ACPAgent) effectivePermissionMode() string {
+	if strings.TrimSpace(a.Mode) != "" {
+		return a.Mode
+	}
+	return a.ReadOnlyMode
+}
+
 func defaultACPAgentConfig() *config.ACPAgentConfig {
 	return &config.ACPAgentConfig{
 		Name:            defaultACPName,

internal/agent/acp_auth_test.go

@@ -124,6 +124,50 @@ func TestACPAuthPermissionModes(t *testing.T) {
 		}
 	})
 
+	t.Run("RequestPermission: Empty mode defaults to read-only behavior for permissions", func(t *testing.T) {
+		agent := &ACPAgent{
+			agentName:       "test-acp",
+			Command:         "test-command",
+			ReadOnlyMode:    "plan",
+			AutoApproveMode: "auto-approve",
+			Mode:            "",
+			Model:           "test-model",
+			Timeout:         30 * time.Second,
+		}
+
+		client := &acpClient{
+			agent: agent,
+		}
+
+		readKind := acp.ToolKind("read")
+		readResponse, err := client.RequestPermission(context.Background(), acp.RequestPermissionRequest{
+			Options: authTestPermissionOptions(),
+			ToolCall: acp.RequestPermissionToolCall{
+				Kind: &readKind,
+			},
+		})
+		if err != nil {
+			t.Fatalf("RequestPermission failed for read kind: %v", err)
+		}
+		if readResponse.Outcome.Selected == nil || readResponse.Outcome.Selected.OptionId != authAllowAlwaysOptionID {
+			t.Errorf("Expected %q for non-destructive operation when mode is disabled, got '%v'", authAllowAlwaysOptionID, readResponse.Outcome)
+		}
+
+		editKind := acp.ToolKind("edit")
+		editResponse, err := client.RequestPermission(context.Background(), acp.RequestPermissionRequest{
+			Options: authTestPermissionOptions(),
+			ToolCall: acp.RequestPermissionToolCall{
+				Kind: &editKind,
+			},
+		})
+		if err != nil {
+			t.Fatalf("RequestPermission failed for edit kind: %v", err)
+		}
+		if editResponse.Outcome.Selected == nil || editResponse.Outcome.Selected.OptionId != authRejectAlwaysOptionID {
+			t.Errorf("Expected %q for destructive operation when mode is disabled, got '%v'", authRejectAlwaysOptionID, editResponse.Outcome)
+		}
+	})
+
 	t.Run("RequestPermission: AutoApproveMode allows all known operations", func(t *testing.T) {
 		// Create agent in auto-approve mode (agentic)
 		agent := &ACPAgent{