Skip to content

Commit 3def33b

Browse files
wesmwesm
committed
acp: support config mode-negotiation disable and bound line reads
1 parent 02f6b0a commit 3def33b

File tree

3 files changed

+124
-26
lines changed

3 files changed

+124
-26
lines changed

‎internal/agent/acp.go‎

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ func NewACPAgentFromConfig(config *config.ACPAgentConfig) *ACPAgent {
8989
if autoApproveMode := strings.TrimSpace(config.AutoApproveMode); autoApproveMode != "" {
9090
agent.AutoApproveMode = autoApproveMode
9191
}
92-
if mode := strings.TrimSpace(config.Mode); mode != "" {
92+
if config.DisableModeNegotiation {
93+
agent.Mode = ""
94+
} else if mode := strings.TrimSpace(config.Mode); mode != "" {
9395
agent.Mode = mode
9496
} else {
9597
agent.Mode = agent.ReadOnlyMode
@@ -139,6 +141,9 @@ func (a *ACPAgent) WithAgentic(agentic bool) Agent {
139141
if agentic && a.AutoApproveMode != "" {
140142
mode = a.AutoApproveMode
141143
}
144+
if strings.TrimSpace(a.Mode) == "" {
145+
mode = ""
146+
}
142147

143148
return &ACPAgent{
144149
agentName: a.agentName,
@@ -584,12 +589,13 @@ func readTextFileWindow(path string, startLine int, limit *int, maxBytes int) (s
584589

585590
reader := bufio.NewReader(file)
586591
var out strings.Builder
592+
var lineBuf bytes.Buffer
587593
currentLine := 0
588594
selectedLines := 0
589595
wroteLine := false
590596
endedWithNewline := false
591597

592-
appendLine := func(line string) error {
598+
appendLine := func(line []byte) error {
593599
additionalBytes := len(line)
594600
if wroteLine {
595601
additionalBytes++
@@ -600,14 +606,43 @@ func readTextFileWindow(path string, startLine int, limit *int, maxBytes int) (s
600606
if wroteLine {
601607
out.WriteByte('\n')
602608
}
603-
out.WriteString(line)
609+
if len(line) > 0 {
610+
out.Write(line)
611+
}
604612
wroteLine = true
605613
selectedLines++
606614
return nil
607615
}
608616

617+
lineBufferBudget := func() int {
618+
budget := maxBytes - out.Len()
619+
if wroteLine {
620+
budget--
621+
}
622+
return budget
623+
}
624+
625+
appendLineChunk := func(chunk []byte) error {
626+
if len(chunk) == 0 {
627+
return nil
628+
}
629+
if lineBufferBudget() < lineBuf.Len()+len(chunk) {
630+
return fmt.Errorf("file content too large: exceeds max %d bytes", maxBytes)
631+
}
632+
_, err := lineBuf.Write(chunk)
633+
return err
634+
}
635+
609636
for {
610-
chunk, readErr := reader.ReadBytes('\n')
637+
chunk, readErr := reader.ReadSlice('\n')
638+
if readErr == bufio.ErrBufferFull {
639+
if currentLine >= startLine && (limit == nil || selectedLines < *limit) {
640+
if err := appendLineChunk(chunk); err != nil {
641+
return "", err
642+
}
643+
}
644+
continue
645+
}
611646
if readErr != nil && readErr != io.EOF {
612647
return "", readErr
613648
}
@@ -616,15 +651,19 @@ func readTextFileWindow(path string, startLine int, limit *int, maxBytes int) (s
616651
}
617652

618653
hasTrailingNewline := len(chunk) > 0 && chunk[len(chunk)-1] == '\n'
619-
if hasTrailingNewline {
620-
chunk = chunk[:len(chunk)-1]
621-
}
622654
endedWithNewline = hasTrailingNewline
623655

624656
if currentLine >= startLine && (limit == nil || selectedLines < *limit) {
625-
if err := appendLine(string(chunk)); err != nil {
657+
if hasTrailingNewline {
658+
chunk = chunk[:len(chunk)-1]
659+
}
660+
if err := appendLineChunk(chunk); err != nil {
661+
return "", err
662+
}
663+
if err := appendLine(lineBuf.Bytes()); err != nil {
626664
return "", err
627665
}
666+
lineBuf.Reset()
628667
if limit != nil && selectedLines >= *limit {
629668
return out.String(), nil
630669
}
@@ -788,7 +827,7 @@ func (c *acpClient) WriteTextFile(ctx context.Context, params acp.WriteTextFileR
788827

789828
// Enforce authorization at point of operation.
790829
if !c.agent.mutatingOperationsAllowed() {
791-
if c.agent.Mode == c.agent.ReadOnlyMode {
830+
if c.agent.effectivePermissionMode() == c.agent.ReadOnlyMode {
792831
return acp.WriteTextFileResponse{}, fmt.Errorf("write operation not permitted in read-only mode")
793832
}
794833
return acp.WriteTextFileResponse{}, fmt.Errorf("write operation not permitted unless auto-approve mode is explicitly enabled")
@@ -1071,7 +1110,7 @@ func (c *acpClient) CreateTerminal(ctx context.Context, params acp.CreateTermina
10711110

10721111
// Enforce authorization at point of operation.
10731112
if !c.agent.mutatingOperationsAllowed() {
1074-
if c.agent.Mode == c.agent.ReadOnlyMode {
1113+
if c.agent.effectivePermissionMode() == c.agent.ReadOnlyMode {
10751114
return acp.CreateTerminalResponse{}, fmt.Errorf("terminal creation not permitted in read-only mode")
10761115
}
10771116
return acp.CreateTerminalResponse{}, fmt.Errorf("terminal creation not permitted unless auto-approve mode is explicitly enabled")
@@ -1410,13 +1449,16 @@ func validateConfiguredModel(configuredModel string, models *acp.SessionModelSta
14101449
}
14111450

14121451
func (a *ACPAgent) mutatingOperationsAllowed() bool {
1413-
return a.AutoApproveMode != "" && a.Mode == a.AutoApproveMode
1452+
return a.AutoApproveMode != "" && a.effectivePermissionMode() == a.AutoApproveMode
14141453
}
14151454

14161455
func (a *ACPAgent) effectivePermissionMode() string {
14171456
if strings.TrimSpace(a.Mode) != "" {
14181457
return a.Mode
14191458
}
1459+
if a.Agentic && strings.TrimSpace(a.AutoApproveMode) != "" {
1460+
return a.AutoApproveMode
1461+
}
14201462
return a.ReadOnlyMode
14211463
}
14221464

@@ -1512,7 +1554,12 @@ func applyACPAgentConfigOverride(cfg *config.ACPAgentConfig, override *config.AC
15121554
if autoApproveMode := strings.TrimSpace(override.AutoApproveMode); autoApproveMode != "" {
15131555
cfg.AutoApproveMode = autoApproveMode
15141556
}
1515-
if mode := strings.TrimSpace(override.Mode); mode != "" {
1557+
if override.DisableModeNegotiation {
1558+
cfg.DisableModeNegotiation = true
1559+
}
1560+
if cfg.DisableModeNegotiation {
1561+
cfg.Mode = ""
1562+
} else if mode := strings.TrimSpace(override.Mode); mode != "" {
15161563
cfg.Mode = mode
15171564
} else {
15181565
// If mode is omitted, default to the effective read-only mode.

‎internal/agent/acp_test.go‎

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,43 +96,57 @@ func TestACPAgentCommandLine(t *testing.T) {
9696

9797
func TestApplyACPAgentConfigOverrideModeResolution(t *testing.T) {
9898
tests := []struct {
99-
name string
100-
override *config.ACPAgentConfig
101-
wantReadOnly string
102-
wantMode string
99+
name string
100+
override *config.ACPAgentConfig
101+
wantReadOnly string
102+
wantMode string
103+
wantDisableModeNeg bool
103104
}{
104105
{
105106
name: "read_only_mode only",
106107
override: &config.ACPAgentConfig{
107108
ReadOnlyMode: "safe-plan",
108109
},
109-
wantReadOnly: "safe-plan",
110-
wantMode: "safe-plan",
110+
wantReadOnly: "safe-plan",
111+
wantMode: "safe-plan",
112+
wantDisableModeNeg: false,
111113
},
112114
{
113115
name: "mode only",
114116
override: &config.ACPAgentConfig{
115117
Mode: "custom-mode",
116118
},
117-
wantReadOnly: defaultACPReadOnlyMode,
118-
wantMode: "custom-mode",
119+
wantReadOnly: defaultACPReadOnlyMode,
120+
wantMode: "custom-mode",
121+
wantDisableModeNeg: false,
119122
},
120123
{
121124
name: "both mode and read_only_mode",
122125
override: &config.ACPAgentConfig{
123126
ReadOnlyMode: "safe-plan",
124127
Mode: "agentic-mode",
125128
},
126-
wantReadOnly: "safe-plan",
127-
wantMode: "agentic-mode",
129+
wantReadOnly: "safe-plan",
130+
wantMode: "agentic-mode",
131+
wantDisableModeNeg: false,
132+
},
133+
{
134+
name: "disable mode negotiation",
135+
override: &config.ACPAgentConfig{
136+
DisableModeNegotiation: true,
137+
},
138+
wantReadOnly: defaultACPReadOnlyMode,
139+
wantMode: "",
140+
wantDisableModeNeg: true,
128141
},
129142
{
130143
name: "neither mode nor read_only_mode",
131144
override: &config.ACPAgentConfig{
132145
Model: "devstral-2",
133146
},
134-
wantReadOnly: defaultACPReadOnlyMode,
135-
wantMode: defaultACPReadOnlyMode,
147+
wantReadOnly: defaultACPReadOnlyMode,
148+
wantMode: defaultACPReadOnlyMode,
149+
wantDisableModeNeg: false,
136150
},
137151
}
138152

@@ -147,10 +161,44 @@ func TestApplyACPAgentConfigOverrideModeResolution(t *testing.T) {
147161
if cfg.Mode != tc.wantMode {
148162
t.Fatalf("Mode = %q, want %q", cfg.Mode, tc.wantMode)
149163
}
164+
if cfg.DisableModeNegotiation != tc.wantDisableModeNeg {
165+
t.Fatalf("DisableModeNegotiation = %v, want %v", cfg.DisableModeNegotiation, tc.wantDisableModeNeg)
166+
}
150167
})
151168
}
152169
}
153170

171+
func TestNewACPAgentFromConfigDisableModeNegotiation(t *testing.T) {
172+
t.Parallel()
173+
174+
agent := NewACPAgentFromConfig(&config.ACPAgentConfig{
175+
Command: "go",
176+
Mode: "plan",
177+
ReadOnlyMode: "plan",
178+
AutoApproveMode: "auto-approve",
179+
DisableModeNegotiation: true,
180+
})
181+
if agent.Mode != "" {
182+
t.Fatalf("expected mode negotiation disabled (empty mode), got %q", agent.Mode)
183+
}
184+
185+
agentic := agent.WithAgentic(true).(*ACPAgent)
186+
if agentic.Mode != "" {
187+
t.Fatalf("expected WithAgentic(true) to preserve disabled mode negotiation, got %q", agentic.Mode)
188+
}
189+
if !agentic.mutatingOperationsAllowed() {
190+
t.Fatalf("expected mutating operations allowed in agentic mode when negotiation is disabled")
191+
}
192+
193+
nonAgentic := agent.WithAgentic(false).(*ACPAgent)
194+
if nonAgentic.Mode != "" {
195+
t.Fatalf("expected WithAgentic(false) to preserve disabled mode negotiation, got %q", nonAgentic.Mode)
196+
}
197+
if nonAgentic.mutatingOperationsAllowed() {
198+
t.Fatalf("expected mutating operations denied in non-agentic mode when negotiation is disabled")
199+
}
200+
}
201+
154202
func TestGetAvailableWithConfigResolvesACPAlias(t *testing.T) {
155203
t.Parallel()
156204

‎internal/config/config.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,11 @@ type ACPAgentConfig struct {
140140
ReadOnlyMode string `toml:"read_only_mode"` // Read-only mode. Valid values depend on the underlying agent, e.g. "plan"
141141
AutoApproveMode string `toml:"auto_approve_mode"` // Auto-approve mode. Valid values depend on the underlying agent, e.g. "auto-approve"
142142
Mode string `toml:"mode"` // Default agent mode. Use read_only_mode for review flows unless explicitly opting in.
143-
Model string `toml:"model"` // Default model to use
144-
Timeout int `toml:"timeout"` // Command timeout in seconds (default: 600)
143+
// DisableModeNegotiation skips ACP SetSessionMode while keeping
144+
// authorization behavior based on agentic/read-only mode selection.
145+
DisableModeNegotiation bool `toml:"disable_mode_negotiation"`
146+
Model string `toml:"model"` // Default model to use
147+
Timeout int `toml:"timeout"` // Command timeout in seconds (default: 600)
145148
}
146149

147150
// GitHubAppConfig holds GitHub App authentication settings.

0 commit comments

Comments
 (0)