From e61e09cc1ea017a783b05eebb3239c5cecad314a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Tue, 21 Apr 2026 19:57:22 +0100 Subject: [PATCH] fix(backend): prevent duplicate --allow-indexing in Auggie passthrough preview (#675) --- .../agent/settings/controller/agent_config.go | 13 ++- .../settings/controller/controller_test.go | 104 ++++++++++++++++++ 2 files changed, 114 insertions(+), 3 deletions(-) diff --git a/apps/backend/internal/agent/settings/controller/agent_config.go b/apps/backend/internal/agent/settings/controller/agent_config.go index 5a7e42e0b6..d2aed016d4 100644 --- a/apps/backend/internal/agent/settings/controller/agent_config.go +++ b/apps/backend/internal/agent/settings/controller/agent_config.go @@ -183,6 +183,13 @@ func (c *Controller) PreviewAgentCommand(ctx context.Context, agentName string, // Tolerate malformed entries silently — the preview is informational. cliFlagTokens, _ := cliflags.Resolve(cliFlagsFromDTO(req.CLIFlags)) + // Passthrough: BuildPassthroughCommand emits permission flags via Settings(); + // the launch path (manager_passthrough.go) does not append CLIFlagTokens for + // passthrough, so the preview must match — otherwise permission flags that + // the legacy allow_indexing backfill also pushes into CLIFlags get rendered + // twice (e.g. Auggie's --allow-indexing). + // ACP: mirror lifecycle.CommandBuilder.BuildCommand by appending CLIFlagTokens + // after the agent's BuildCommand. var cmd agents.Command if ptAgent, ok := agentConfig.(agents.PassthroughAgent); ok && req.CLIPassthrough { cmd = ptAgent.BuildPassthroughCommand(agents.PassthroughOptions{ @@ -195,9 +202,9 @@ func (c *Controller) PreviewAgentCommand(ctx context.Context, agentName string, PermissionValues: req.PermissionSettings, CLIFlagTokens: cliFlagTokens, }) - } - if len(cliFlagTokens) > 0 { - cmd = cmd.With().Flag(cliFlagTokens...).Build() + if len(cliFlagTokens) > 0 { + cmd = cmd.With().Flag(cliFlagTokens...).Build() + } } return &dto.CommandPreviewResponse{ diff --git a/apps/backend/internal/agent/settings/controller/controller_test.go b/apps/backend/internal/agent/settings/controller/controller_test.go index dcf9dd04b3..7223fef169 100644 --- a/apps/backend/internal/agent/settings/controller/controller_test.go +++ b/apps/backend/internal/agent/settings/controller/controller_test.go @@ -288,6 +288,110 @@ func TestController_PreviewAgentCommand_PassthroughDisabled(t *testing.T) { } } +// TestController_PreviewAgentCommand_PassthroughDoesNotDuplicateCLIFlag regresses +// the Auggie case where enabling CLI passthrough caused --allow-indexing to be +// emitted twice: once by BuildPassthroughCommand via Settings() and again by an +// unconditional CLIFlagTokens append in the preview. The launch path only adds +// it via Settings(), so the preview must match. +func TestController_PreviewAgentCommand_PassthroughDoesNotDuplicateCLIFlag(t *testing.T) { + permSettings := map[string]agents.PermissionSetting{ + "allow_indexing": { + Supported: true, + ApplyMethod: "cli_flag", + CLIFlag: "--allow-indexing", + }, + } + agentList := map[string]agents.Agent{ + "auggie": &testAgent{ + id: "auggie", + name: "auggie", + enabled: true, + runtime: &agents.RuntimeConfig{ + Cmd: agents.NewCommand("auggie"), + }, + StandardPassthrough: agents.StandardPassthrough{ + Cfg: agents.PassthroughConfig{ + Supported: true, + PassthroughCmd: agents.NewCommand("npx", "-y", "@augmentcode/auggie"), + }, + PermSettings: permSettings, + }, + permissionSettings: permSettings, + }, + } + + controller := newTestController(agentList) + + // Simulate the post-backfill state: allow_indexing toggle ON and the legacy + // backfill has seeded --allow-indexing into CLIFlags. Both sources would + // previously emit the flag. + req := CommandPreviewRequest{ + PermissionSettings: map[string]bool{"allow_indexing": true}, + CLIPassthrough: true, + CLIFlags: []dto.CLIFlagDTO{ + {Flag: "--allow-indexing", Enabled: true}, + }, + } + + result, err := controller.PreviewAgentCommand(context.Background(), "auggie", req) + if err != nil { + t.Fatalf("PreviewAgentCommand() error = %v", err) + } + + count := 0 + for _, part := range result.Command { + if part == "--allow-indexing" { + count++ + } + } + if count != 1 { + t.Errorf("PreviewAgentCommand() emitted --allow-indexing %d times, want 1; got %v", count, result.Command) + } +} + +// TestController_PreviewAgentCommand_ACPAppendsCLIFlagsOnce locks in that the +// non-passthrough (ACP) branch still appends user-configured CLI flags exactly +// once. Auggie's real BuildCommand intentionally does not emit --allow-indexing; +// the flag must come through CLIFlagTokens. +func TestController_PreviewAgentCommand_ACPAppendsCLIFlagsOnce(t *testing.T) { + agentList := map[string]agents.Agent{ + "auggie": &testAgent{ + id: "auggie", + name: "auggie", + enabled: true, + runtime: &agents.RuntimeConfig{ + Cmd: agents.NewCommand("auggie", "--acp"), + }, + // No permissionSettings: mirrors Auggie's real BuildCommand which + // does not emit --allow-indexing — it flows through CLIFlags only. + }, + } + + controller := newTestController(agentList) + + req := CommandPreviewRequest{ + CLIPassthrough: false, + CLIFlags: []dto.CLIFlagDTO{ + {Flag: "--allow-indexing", Enabled: true}, + }, + } + + result, err := controller.PreviewAgentCommand(context.Background(), "auggie", req) + if err != nil { + t.Fatalf("PreviewAgentCommand() error = %v", err) + } + + count := 0 + for _, part := range result.Command { + if part == "--allow-indexing" { + count++ + } + } + if count != 1 { + t.Errorf("PreviewAgentCommand() emitted --allow-indexing %d times, want 1; got %v", count, result.Command) + } +} + func TestBuildCommandString(t *testing.T) { tests := []struct { name string