Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions NOTICE
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ APACHE 2.0 LICENSED DEPENDENCIES

- github.com/aws/aws-sdk-go-v2/feature/s3/manager
License: Apache-2.0
URL: https://github.com/aws/aws-sdk-go-v2/blob/feature/s3/manager/v1.20.18/feature/s3/manager/LICENSE.txt
URL: https://github.com/aws/aws-sdk-go-v2/blob/feature/s3/manager/v1.20.19/feature/s3/manager/LICENSE.txt

- github.com/aws/aws-sdk-go-v2/internal/configsources
License: Apache-2.0
Expand Down Expand Up @@ -1126,7 +1126,7 @@ MIT LICENSED DEPENDENCIES

- github.com/charmbracelet/x/ansi
License: MIT
URL: https://github.com/charmbracelet/x/blob/ansi/v0.11.3/ansi/LICENSE
URL: https://github.com/charmbracelet/x/blob/ansi/v0.11.4/ansi/LICENSE

- github.com/charmbracelet/x/cellbuf
License: MIT
Expand All @@ -1146,7 +1146,7 @@ MIT LICENSED DEPENDENCIES

- github.com/clipperhouse/displaywidth
License: MIT
URL: https://github.com/clipperhouse/displaywidth/blob/v0.6.1/LICENSE
URL: https://github.com/clipperhouse/displaywidth/blob/v0.7.0/LICENSE

- github.com/clipperhouse/stringish
License: MIT
Expand Down
6 changes: 3 additions & 3 deletions go.mod

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

115 changes: 115 additions & 0 deletions internal/exec/workflow_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"os"
"path/filepath"
"slices"
"strings"
"testing"

Expand Down Expand Up @@ -1542,6 +1543,120 @@ func TestEnsureWorkflowToolchainDependencies_EmptyWorkflowDef(t *testing.T) {
assert.Empty(t, path)
}

// TestDoubleHyphenStackInsertion tests the stack insertion logic with double-hyphen separator.
// This is the fix for GitHub issue #1967 where commands with -- separator were incorrectly parsed.
func TestDoubleHyphenStackInsertion(t *testing.T) {
tests := []struct {
name string
command string
workflowStack string
expectedArgs []string
}{
{
name: "insert stack before double-hyphen",
command: "terraform plan vpc -- -consolidate-warnings=false",
workflowStack: "nonprod",
expectedArgs: []string{
"terraform", "plan", "vpc", "-s", "nonprod", "--", "-consolidate-warnings=false",
},
},
{
name: "command with stack already - workflow stack should be added",
command: "terraform plan vpc --stack dev -- -consolidate-warnings=false",
workflowStack: "nonprod",
expectedArgs: []string{
"terraform", "plan", "vpc", "--stack", "dev", "-s", "nonprod", "--", "-consolidate-warnings=false",
},
},
{
name: "no double-hyphen - append at end",
command: "terraform plan vpc",
workflowStack: "nonprod",
expectedArgs: []string{
"terraform", "plan", "vpc", "-s", "nonprod",
},
},
{
name: "multiple args after double-hyphen",
command: "terraform plan vpc -- -var=foo=bar -var=baz=qux",
workflowStack: "nonprod",
expectedArgs: []string{
"terraform", "plan", "vpc", "-s", "nonprod", "--", "-var=foo=bar", "-var=baz=qux",
},
},
{
name: "complex component name with hyphens",
command: "terraform plan 10-static-web-app-no-frontdoor -- -consolidate-warnings=false",
workflowStack: "trialintelx-eastus-sbx",
expectedArgs: []string{
"terraform", "plan", "10-static-web-app-no-frontdoor", "-s", "trialintelx-eastus-sbx", "--", "-consolidate-warnings=false",
},
},
{
name: "empty workflow stack - no insertion",
command: "terraform plan vpc -- -consolidate-warnings=false",
workflowStack: "",
expectedArgs: []string{
"terraform", "plan", "vpc", "--", "-consolidate-warnings=false",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Parse command using shell.Fields (same as workflow code).
args, err := shell.Fields(tt.command, nil)
require.NoError(t, err)

// Apply the same stack insertion logic as in workflow_utils.go line 374-383.
// Uses slices.Index to stay in sync with production code.
if tt.workflowStack != "" {
if idx := slices.Index(args, "--"); idx != -1 {
// Insert before the "--".
args = append(args[:idx], append([]string{"-s", tt.workflowStack}, args[idx:]...)...)
} else {
// Append at the end.
args = append(args, "-s", tt.workflowStack)
}
}

assert.Equal(t, tt.expectedArgs, args)
})
}
}

// TestDoubleHyphenIssue1967 is a specific test for GitHub issue #1967.
// It reproduces the exact scenario from the bug report where the stack value was corrupted.
func TestDoubleHyphenIssue1967(t *testing.T) {
// This is the exact command from the bug report.
command := "terraform plan 10-static-web-app-no-frontdoor --stack trialintelx-eastus-sbx -- -consolidate-warnings=false"

// Parse using shell.Fields.
args, err := shell.Fields(command, nil)
require.NoError(t, err)

// Verify the parsed args are correct.
expectedParsedArgs := []string{
"terraform", "plan", "10-static-web-app-no-frontdoor",
"--stack", "trialintelx-eastus-sbx",
"--", "-consolidate-warnings=false",
}
assert.Equal(t, expectedParsedArgs, args, "shell.Fields should correctly parse the command")

// Verify each argument doesn't have corruption.
for i, arg := range args {
t.Logf("arg[%d] = %q (len=%d)", i, arg, len(arg))
}

// Specifically check the stack value is preserved.
assert.Equal(t, "--stack", args[3], "fourth arg should be --stack")
assert.Equal(t, "trialintelx-eastus-sbx", args[4], "fifth arg should be the stack value")

// Verify the terraform flag after -- is preserved.
assert.Equal(t, "--", args[5], "sixth arg should be --")
assert.Equal(t, "-consolidate-warnings=false", args[6], "seventh arg should be -consolidate-warnings=false")
}

// TestEnsureWorkflowToolchainDependencies_WithToolVersionsFile tests with .tool-versions file present.
func TestEnsureWorkflowToolchainDependencies_WithToolVersionsFile(t *testing.T) {
tempDir := t.TempDir()
Expand Down
32 changes: 24 additions & 8 deletions pkg/flags/compat/compatibility_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const (
const (
// FlagPrefix is the single dash prefix for flags.
flagPrefix = "-"
// EndOfOptionsMarker is the POSIX standard "--" that separates options from positional arguments.
endOfOptionsMarker = "--"
)

// CompatibilityFlag defines how a single compatibility flag should be handled.
Expand Down Expand Up @@ -87,7 +89,7 @@ func (t *CompatibilityFlagTranslator) ValidateNoConflicts(cmd *cobra.Command) er

for compatFlag := range t.flagMap {
// Only check single-dash flags (potential shorthands).
if !strings.HasPrefix(compatFlag, flagPrefix) || strings.HasPrefix(compatFlag, "--") {
if !strings.HasPrefix(compatFlag, flagPrefix) || strings.HasPrefix(compatFlag, endOfOptionsMarker) {
continue
}

Expand Down Expand Up @@ -125,7 +127,7 @@ func (t *CompatibilityFlagTranslator) ValidateTargetsInArgs(cmd *cobra.Command,
}

// Skip already-modern flags (--).
if strings.HasPrefix(arg, "--") {
if strings.HasPrefix(arg, endOfOptionsMarker) {
continue
}

Expand Down Expand Up @@ -177,26 +179,40 @@ func (t *CompatibilityFlagTranslator) Translate(args []string) (atmosArgs []stri
for i := 0; i < len(args); i++ {
arg := args[i]

// Not a flag - it's a positional arg
// Not a flag - it's a positional arg.
if !strings.HasPrefix(arg, flagPrefix) {
atmosArgs = append(atmosArgs, arg)
continue
}

// Already modern format (--flag) - pass to Atmos
if strings.HasPrefix(arg, "--") {
// Handle "--" end-of-options marker per POSIX convention.
// Everything after "--" is passed through to the subprocess, not parsed by Cobra.
// This fixes issue #1967 where args like "-consolidate-warnings=false" after "--"
// were incorrectly parsed by Cobra/pflag.
// We include the "--" in atmosArgs so Cobra knows to stop parsing flags,
// but everything after it goes to separatedArgs to bypass Cobra completely.
if arg == endOfOptionsMarker {
atmosArgs = append(atmosArgs, endOfOptionsMarker)
if i+1 < len(args) {
separatedArgs = append(separatedArgs, args[i+1:]...)
}
break
}

// Already modern format (--flag) - pass to Atmos.
if strings.HasPrefix(arg, endOfOptionsMarker) {
atmosArgs = append(atmosArgs, arg)
continue
}

// Single-dash flag - check for compatibility flag
// Single-dash flag - check for compatibility flag.
translated, consumed := t.translateSingleDashFlag(args, i)

// Add translated args to appropriate destination
// Add translated args to appropriate destination.
atmosArgs = append(atmosArgs, translated.atmosArgs...)
separatedArgs = append(separatedArgs, translated.separatedArgs...)

// Skip consumed args
// Skip consumed args.
i += consumed
}

Expand Down
101 changes: 101 additions & 0 deletions pkg/flags/compat/compatibility_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,107 @@ func TestCompatibilityFlagTranslator_EdgeCases(t *testing.T) {
}
}

// TestCompatibilityFlagTranslator_DoubleHyphenSeparator tests the "--" end-of-options handling.
// This is the fix for GitHub issue #1967 where args after "--" were incorrectly parsed.
func TestCompatibilityFlagTranslator_DoubleHyphenSeparator(t *testing.T) {
tests := []struct {
name string
input []string
expected translationResult
}{
{
name: "double-hyphen with terraform flags after",
input: []string{"terraform", "plan", "vpc", "--stack", "nonprod", "--", "-consolidate-warnings=false"},
expected: translationResult{
atmosArgs: []string{"terraform", "plan", "vpc", "--stack", "nonprod", "--"},
separatedArgs: []string{"-consolidate-warnings=false"},
},
},
{
name: "double-hyphen with multiple args after",
input: []string{"terraform", "plan", "vpc", "-s", "nonprod", "--", "-var=foo=bar", "-var=baz=qux"},
expected: translationResult{
atmosArgs: []string{"terraform", "plan", "vpc", "-s", "nonprod", "--"},
separatedArgs: []string{"-var=foo=bar", "-var=baz=qux"},
},
},
{
name: "double-hyphen at end (no args after)",
input: []string{"terraform", "plan", "vpc", "--stack", "nonprod", "--"},
expected: translationResult{
atmosArgs: []string{"terraform", "plan", "vpc", "--stack", "nonprod", "--"},
separatedArgs: []string{},
},
},
{
name: "no double-hyphen",
input: []string{"terraform", "plan", "vpc", "--stack", "nonprod"},
expected: translationResult{
atmosArgs: []string{"terraform", "plan", "vpc", "--stack", "nonprod"},
separatedArgs: []string{},
},
},
{
name: "double-hyphen with compat flag after (should go to separated)",
input: []string{"terraform", "plan", "vpc", "--", "-var", "foo=bar"},
expected: translationResult{
atmosArgs: []string{"terraform", "plan", "vpc", "--"},
separatedArgs: []string{"-var", "foo=bar"},
},
},
{
name: "complex component name with hyphens (issue 1967 exact scenario)",
input: []string{"terraform", "plan", "10-static-web-app-no-frontdoor", "--stack", "trialintelx-eastus-sbx", "--", "-consolidate-warnings=false"},
expected: translationResult{
atmosArgs: []string{"terraform", "plan", "10-static-web-app-no-frontdoor", "--stack", "trialintelx-eastus-sbx", "--"},
separatedArgs: []string{"-consolidate-warnings=false"},
},
},
{
name: "double-hyphen with -s flag after (should NOT be parsed as stack)",
input: []string{"terraform", "plan", "vpc", "--stack", "prod", "--", "-s", "wrongstack"},
expected: translationResult{
atmosArgs: []string{"terraform", "plan", "vpc", "--stack", "prod", "--"},
separatedArgs: []string{"-s", "wrongstack"},
},
},
{
name: "multiple double-hyphens (only first matters)",
input: []string{"terraform", "plan", "vpc", "--", "-var=a", "--", "-var=b"},
expected: translationResult{
atmosArgs: []string{"terraform", "plan", "vpc", "--"},
separatedArgs: []string{"-var=a", "--", "-var=b"},
},
},
{
name: "double-hyphen as only arg after command",
input: []string{"terraform", "plan", "--"},
expected: translationResult{
atmosArgs: []string{"terraform", "plan", "--"},
separatedArgs: []string{},
},
},
{
name: "double-hyphen first in args",
input: []string{"--", "-var=foo"},
expected: translationResult{
atmosArgs: []string{"--"},
separatedArgs: []string{"-var=foo"},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
translator := buildTestCompatibilityTranslator()
atmosArgs, separatedArgs := translator.Translate(tt.input)

assert.Equal(t, tt.expected.atmosArgs, atmosArgs, "atmosArgs mismatch")
assert.Equal(t, tt.expected.separatedArgs, separatedArgs, "separatedArgs mismatch")
})
}
}

func TestCompatibilityFlagTranslator_ValidateTargets(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading
Loading