From beeb9387b57598d859593ddd000200f65c3f7ab0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 11 Dec 2025 16:59:47 +0000 Subject: [PATCH 1/4] Initial plan From 1ca6497260a931119399a3dc6a0f284b3a9522af Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:05:04 +0000 Subject: [PATCH 2/4] Add tests to verify single expansion issue Co-authored-by: alexec <1142830+alexec@users.noreply.github.com> --- integration_test.go | 66 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/integration_test.go b/integration_test.go index abeafd7f..1a62a827 100644 --- a/integration_test.go +++ b/integration_test.go @@ -1230,3 +1230,69 @@ This rule is from a remote directory. t.Errorf("task not found in stdout. Output:\n%s", output) } } + +// TestSingleExpansion verifies that content is expanded only once in the full flow +func TestSingleExpansion(t *testing.T) { +dirs := setupTestDirs(t) + +// Create a task that uses a parameter with expansion syntax +taskFile := filepath.Join(dirs.tasksDir, "test-expand.md") +taskContent := `Task with parameter: ${param1} + +And a value that looks like expansion syntax but should not be expanded: ${"nested"}` +if err := os.WriteFile(taskFile, []byte(taskContent), 0644); err != nil { +t.Fatalf("failed to create task file: %v", err) +} + +// Run with param1 set to a value that contains expansion syntax +output := runTool(t, "-C", dirs.tmpDir, "-p", "param1=!`echo hello`", "test-expand") + +// The param1 should be replaced with the literal string "!`echo hello`" +// It should NOT be expanded again (that would execute the command) +if !strings.Contains(output, "!`echo hello`") { +t.Errorf("Expected param1 to be replaced with literal value, got: %s", output) +} + +// Verify "hello" is not in output (which would indicate the command was executed) +// Note: there may be other "hello" strings, so check for the specific context +if strings.Contains(output, "Task with parameter: hello") { +t.Errorf("Parameter value was re-expanded (command was executed), got: %s", output) +} +} + +// TestCommandExpansionOnce verifies that command files are expanded only once +func TestCommandExpansionOnce(t *testing.T) { +dirs := setupTestDirs(t) +commandsDir := filepath.Join(dirs.tmpDir, ".agents", "commands") +if err := os.MkdirAll(commandsDir, 0o755); err != nil { +t.Fatalf("failed to create commands dir: %v", err) +} + +// Create a command file with a parameter +commandFile := filepath.Join(commandsDir, "test-cmd.md") +commandContent := `Command param: ${cmd_param}` +if err := os.WriteFile(commandFile, []byte(commandContent), 0644); err != nil { +t.Fatalf("failed to create command file: %v", err) +} + +// Create a task that calls the command with a param containing expansion syntax +taskFile := filepath.Join(dirs.tasksDir, "test-cmd-task.md") +taskContent := `/test-cmd cmd_param="!` + "`echo injected`" + `"` +if err := os.WriteFile(taskFile, []byte(taskContent), 0644); err != nil { +t.Fatalf("failed to create task file: %v", err) +} + +// Run the task +output := runTool(t, "-C", dirs.tmpDir, "test-cmd-task") + +// The command parameter should be replaced with the literal string "!`echo injected`" +// It should NOT be expanded again (that would execute the command) +if !strings.Contains(output, "!`echo injected`") { +t.Errorf("Expected command param to be replaced with literal value, got: %s", output) +} + +// Verify "injected" is not in output (which would indicate the command was executed) +if strings.Contains(output, "Command param: injected") { +t.Errorf("Command parameter value was re-expanded (command was executed), got: %s", output) +} +} From 91aa33f36127ee67f71a14f54c184d64762c0375 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:09:16 +0000 Subject: [PATCH 3/4] Fix double expansion: parse task before expanding Co-authored-by: alexec <1142830+alexec@users.noreply.github.com> --- pkg/codingcontext/context.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/codingcontext/context.go b/pkg/codingcontext/context.go index f4159976..8c101b29 100644 --- a/pkg/codingcontext/context.go +++ b/pkg/codingcontext/context.go @@ -193,21 +193,24 @@ func (cc *Context) findTask(taskName string) error { cc.agent = agent } - // Expand parameters only if expand_params is not explicitly set to false - contentToProcess := md.Content - if shouldExpandParams(frontMatter.ExpandParams) { - contentToProcess = cc.expandParams(md.Content, nil) - } - - task, err := ParseTask(contentToProcess) + // Parse the task content first to separate text blocks from slash commands + task, err := ParseTask(md.Content) if err != nil { return err } + // Build the final content by processing each block + // Text blocks are expanded if expand_params is not false + // Slash command arguments are NOT expanded (they are literal parameter values) finalContent := strings.Builder{} for _, block := range task { if block.Text != nil { - finalContent.WriteString(block.Text.Content()) + textContent := block.Text.Content() + // Expand parameters in text blocks only if expand_params is not explicitly set to false + if shouldExpandParams(frontMatter.ExpandParams) { + textContent = cc.expandParams(textContent, nil) + } + finalContent.WriteString(textContent) } else if block.SlashCommand != nil { commandContent, err := cc.findCommand(block.SlashCommand.Name, block.SlashCommand.Params()) if err != nil { From 7a485388d738e633dbf4b60313dc07166e3e94a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:15:33 +0000 Subject: [PATCH 4/4] Improve comment clarity per code review feedback Co-authored-by: alexec <1142830+alexec@users.noreply.github.com> --- integration_test.go | 94 ++++++++++++++++++------------------ pkg/codingcontext/context.go | 3 +- 2 files changed, 49 insertions(+), 48 deletions(-) diff --git a/integration_test.go b/integration_test.go index 1a62a827..1036bc75 100644 --- a/integration_test.go +++ b/integration_test.go @@ -1233,66 +1233,66 @@ This rule is from a remote directory. // TestSingleExpansion verifies that content is expanded only once in the full flow func TestSingleExpansion(t *testing.T) { -dirs := setupTestDirs(t) + dirs := setupTestDirs(t) -// Create a task that uses a parameter with expansion syntax -taskFile := filepath.Join(dirs.tasksDir, "test-expand.md") -taskContent := `Task with parameter: ${param1} + // Create a task that uses a parameter with expansion syntax + taskFile := filepath.Join(dirs.tasksDir, "test-expand.md") + taskContent := `Task with parameter: ${param1} And a value that looks like expansion syntax but should not be expanded: ${"nested"}` -if err := os.WriteFile(taskFile, []byte(taskContent), 0644); err != nil { -t.Fatalf("failed to create task file: %v", err) -} + if err := os.WriteFile(taskFile, []byte(taskContent), 0644); err != nil { + t.Fatalf("failed to create task file: %v", err) + } -// Run with param1 set to a value that contains expansion syntax -output := runTool(t, "-C", dirs.tmpDir, "-p", "param1=!`echo hello`", "test-expand") + // Run with param1 set to a value that contains expansion syntax + output := runTool(t, "-C", dirs.tmpDir, "-p", "param1=!`echo hello`", "test-expand") -// The param1 should be replaced with the literal string "!`echo hello`" -// It should NOT be expanded again (that would execute the command) -if !strings.Contains(output, "!`echo hello`") { -t.Errorf("Expected param1 to be replaced with literal value, got: %s", output) -} + // The param1 should be replaced with the literal string "!`echo hello`" + // It should NOT be expanded again (that would execute the command) + if !strings.Contains(output, "!`echo hello`") { + t.Errorf("Expected param1 to be replaced with literal value, got: %s", output) + } -// Verify "hello" is not in output (which would indicate the command was executed) -// Note: there may be other "hello" strings, so check for the specific context -if strings.Contains(output, "Task with parameter: hello") { -t.Errorf("Parameter value was re-expanded (command was executed), got: %s", output) -} + // Verify "hello" is not in output (which would indicate the command was executed) + // Note: there may be other "hello" strings, so check for the specific context + if strings.Contains(output, "Task with parameter: hello") { + t.Errorf("Parameter value was re-expanded (command was executed), got: %s", output) + } } // TestCommandExpansionOnce verifies that command files are expanded only once func TestCommandExpansionOnce(t *testing.T) { -dirs := setupTestDirs(t) -commandsDir := filepath.Join(dirs.tmpDir, ".agents", "commands") -if err := os.MkdirAll(commandsDir, 0o755); err != nil { -t.Fatalf("failed to create commands dir: %v", err) -} + dirs := setupTestDirs(t) + commandsDir := filepath.Join(dirs.tmpDir, ".agents", "commands") + if err := os.MkdirAll(commandsDir, 0o755); err != nil { + t.Fatalf("failed to create commands dir: %v", err) + } -// Create a command file with a parameter -commandFile := filepath.Join(commandsDir, "test-cmd.md") -commandContent := `Command param: ${cmd_param}` -if err := os.WriteFile(commandFile, []byte(commandContent), 0644); err != nil { -t.Fatalf("failed to create command file: %v", err) -} + // Create a command file with a parameter + commandFile := filepath.Join(commandsDir, "test-cmd.md") + commandContent := `Command param: ${cmd_param}` + if err := os.WriteFile(commandFile, []byte(commandContent), 0644); err != nil { + t.Fatalf("failed to create command file: %v", err) + } -// Create a task that calls the command with a param containing expansion syntax -taskFile := filepath.Join(dirs.tasksDir, "test-cmd-task.md") -taskContent := `/test-cmd cmd_param="!` + "`echo injected`" + `"` -if err := os.WriteFile(taskFile, []byte(taskContent), 0644); err != nil { -t.Fatalf("failed to create task file: %v", err) -} + // Create a task that calls the command with a param containing expansion syntax + taskFile := filepath.Join(dirs.tasksDir, "test-cmd-task.md") + taskContent := `/test-cmd cmd_param="!` + "`echo injected`" + `"` + if err := os.WriteFile(taskFile, []byte(taskContent), 0644); err != nil { + t.Fatalf("failed to create task file: %v", err) + } -// Run the task -output := runTool(t, "-C", dirs.tmpDir, "test-cmd-task") + // Run the task + output := runTool(t, "-C", dirs.tmpDir, "test-cmd-task") -// The command parameter should be replaced with the literal string "!`echo injected`" -// It should NOT be expanded again (that would execute the command) -if !strings.Contains(output, "!`echo injected`") { -t.Errorf("Expected command param to be replaced with literal value, got: %s", output) -} + // The command parameter should be replaced with the literal string "!`echo injected`" + // It should NOT be expanded again (that would execute the command) + if !strings.Contains(output, "!`echo injected`") { + t.Errorf("Expected command param to be replaced with literal value, got: %s", output) + } -// Verify "injected" is not in output (which would indicate the command was executed) -if strings.Contains(output, "Command param: injected") { -t.Errorf("Command parameter value was re-expanded (command was executed), got: %s", output) -} + // Verify "injected" is not in output (which would indicate the command was executed) + if strings.Contains(output, "Command param: injected") { + t.Errorf("Command parameter value was re-expanded (command was executed), got: %s", output) + } } diff --git a/pkg/codingcontext/context.go b/pkg/codingcontext/context.go index 8c101b29..1b742e64 100644 --- a/pkg/codingcontext/context.go +++ b/pkg/codingcontext/context.go @@ -201,7 +201,8 @@ func (cc *Context) findTask(taskName string) error { // Build the final content by processing each block // Text blocks are expanded if expand_params is not false - // Slash command arguments are NOT expanded (they are literal parameter values) + // Slash command arguments are NOT expanded here - they are passed as literals + // to command files where they may be substituted via ${param} templates finalContent := strings.Builder{} for _, block := range task { if block.Text != nil {