Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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.

120 changes: 120 additions & 0 deletions internal/exec/workflow_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,126 @@ 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.
if tt.workflowStack != "" {
idx := -1
for i, arg := range args {
if arg == "--" {
idx = i
break
}
}
if 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