load stack after create stack in MCP #1881
Conversation
📝 WalkthroughWalkthroughThe pull request modifies AWS and GCP stack creation tools to automatically load the stack environment immediately after stack creation, replacing manual activation guidance with automated environment setup and configuration assignment. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
KevyVo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/pkg/agent/tools/create_aws_stack.go`:
- Around line 44-49: Guard against nil StackConfig.Stack before dereferencing
it: in the functions that assign to the pointed value (the code using *sc.Stack
= newStack in create_aws_stack.go and the analogous assignments in
create_gcp_stack.go and select_stack.go) add an explicit nil-check on sc and
sc.Stack (e.g., if sc == nil || sc.Stack == nil { return error } ) and return a
clear error when nil; update callers or error messages as needed so the function
contracts fail fast instead of panicking when StackConfig.Stack is nil.
In `@src/pkg/agent/tools/create_gcp_stack.go`:
- Around line 44-49: The code dereferences sc.Stack when assigning *sc.Stack =
newStack which may panic if sc.Stack is nil; before that assignment in the
create logic, add a nil guard that ensures sc.Stack is non-nil (e.g.,
allocate/initialize sc.Stack) and only then assign the loaded newStack into
*sc.Stack; update the block around stacks.LoadStackEnv, the
nil-check/initialization for sc.Stack, and the final assignment to safely handle
an uninitialized sc.Stack pointer.
| err = stacks.LoadStackEnv(newStack, true) | ||
| if err != nil { | ||
| return "", fmt.Errorf("Unable to load stack %q: %w", params.Name, err) | ||
| } | ||
|
|
||
| *sc.Stack = newStack |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the StackConfig type definition
rg -nP --type=go 'type\s+StackConfig\b' -A 10Repository: DefangLabs/defang
Length of output: 2204
🏁 Script executed:
#!/bin/bash
# Find the function containing the problematic code
rg -nP --type=go 'HandleCreateAWSStackTool\s*\(' -A 30Repository: DefangLabs/defang
Length of output: 4433
🏁 Script executed:
#!/bin/bash
# Find all call sites of HandleCreateAWSStackTool to see how sc.Stack is passed
rg -nP --type=go 'HandleCreateAWSStackTool' -B 3 -A 1Repository: DefangLabs/defang
Length of output: 952
🏁 Script executed:
#!/bin/bash
# Find where StackConfig is instantiated or initialized
rg -nP --type=go 'StackConfig\{' -B 2 -A 5Repository: DefangLabs/defang
Length of output: 6549
🏁 Script executed:
#!/bin/bash
# Look for initialization patterns of the Stack field
rg -nP --type=go '\.Stack\s*=' -B 2 -A 2Repository: DefangLabs/defang
Length of output: 3908
🏁 Script executed:
#!/bin/bash
# Check the NewDefangMCPServer function more broadly to understand initialization
rg -nP --type=go 'NewDefangMCPServer|NewMCPServer' -B 5 -A 20Repository: DefangLabs/defang
Length of output: 5557
🏁 Script executed:
#!/bin/bash
# Check if there are any tests that pass nil Stack or verify the handling
rg -nP --type=go 'HandleCreateAWSStackTool.*Stack.*nil' -iRepository: DefangLabs/defang
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check create_gcp_stack.go for the same pattern to see if it's a systemic issue
cat -n src/pkg/agent/tools/create_gcp_stack.go | sed -n '40,55p'Repository: DefangLabs/defang
Length of output: 472
🏁 Script executed:
#!/bin/bash
# Check select_stack.go for the same pattern
cat -n src/pkg/agent/tools/select_stack.go | sed -n '15,35p'Repository: DefangLabs/defang
Length of output: 807
Guard against nil StackConfig.Stack before dereference in stack creation/selection functions.
The dereferences at line 49 of create_aws_stack.go, line 49 of create_gcp_stack.go, and line 27 of select_stack.go will panic if StackConfig.Stack is nil. While current callers always initialize it, the function signatures allow nil values without enforcement. Add explicit guards before each dereference:
🔧 Suggested guard
err = stacks.LoadStackEnv(newStack, true)
if err != nil {
return "", fmt.Errorf("Unable to load stack %q: %w", params.Name, err)
}
+ if sc.Stack == nil {
+ return "", fmt.Errorf("stack config not initialized")
+ }
*sc.Stack = newStackApply the same pattern to create_gcp_stack.go and select_stack.go.
🤖 Prompt for AI Agents
In `@src/pkg/agent/tools/create_aws_stack.go` around lines 44 - 49, Guard against
nil StackConfig.Stack before dereferencing it: in the functions that assign to
the pointed value (the code using *sc.Stack = newStack in create_aws_stack.go
and the analogous assignments in create_gcp_stack.go and select_stack.go) add an
explicit nil-check on sc and sc.Stack (e.g., if sc == nil || sc.Stack == nil {
return error } ) and return a clear error when nil; update callers or error
messages as needed so the function contracts fail fast instead of panicking when
StackConfig.Stack is nil.
| err = stacks.LoadStackEnv(newStack, true) | ||
| if err != nil { | ||
| return "", fmt.Errorf("Unable to load stack %q: %w", params.Name, err) | ||
| } | ||
|
|
||
| *sc.Stack = newStack |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the StackConfig type definition
rg -nP --type=go 'type\s+StackConfig\b' -A 10Repository: DefangLabs/defang
Length of output: 2204
🏁 Script executed:
# Find the function that contains the problematic code
rg -nP --type=go 'HandleCreateGCPStackTool|func.*CreateGCP' -B 2 -A 15Repository: DefangLabs/defang
Length of output: 3800
🏁 Script executed:
# Search for all StackConfig instantiations to understand initialization patterns
rg -nP --type=go '&StackConfig\{|StackConfig\s*{' -B 2 -A 5Repository: DefangLabs/defang
Length of output: 6549
🏁 Script executed:
# Check the specific file for context
fd 'create_gcp_stack.go' --type f | head -1 | xargs wc -lRepository: DefangLabs/defang
Length of output: 103
Add nil guard before dereferencing Stack field.
Line 49 dereferences sc.Stack without validation. Since Stack is a pointer field that may be nil, this will panic if the caller doesn't initialize it. Add a guard:
🔧 Suggested fix
err = stacks.LoadStackEnv(newStack, true)
if err != nil {
return "", fmt.Errorf("Unable to load stack %q: %w", params.Name, err)
}
+ if sc.Stack == nil {
+ return "", fmt.Errorf("stack config not initialized")
+ }
*sc.Stack = newStack📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| err = stacks.LoadStackEnv(newStack, true) | |
| if err != nil { | |
| return "", fmt.Errorf("Unable to load stack %q: %w", params.Name, err) | |
| } | |
| *sc.Stack = newStack | |
| err = stacks.LoadStackEnv(newStack, true) | |
| if err != nil { | |
| return "", fmt.Errorf("Unable to load stack %q: %w", params.Name, err) | |
| } | |
| if sc.Stack == nil { | |
| return "", fmt.Errorf("stack config not initialized") | |
| } | |
| *sc.Stack = newStack |
🤖 Prompt for AI Agents
In `@src/pkg/agent/tools/create_gcp_stack.go` around lines 44 - 49, The code
dereferences sc.Stack when assigning *sc.Stack = newStack which may panic if
sc.Stack is nil; before that assignment in the create logic, add a nil guard
that ensures sc.Stack is non-nil (e.g., allocate/initialize sc.Stack) and only
then assign the loaded newStack into *sc.Stack; update the block around
stacks.LoadStackEnv, the nil-check/initialization for sc.Stack, and the final
assignment to safely handle an uninitialized sc.Stack pointer.
Description
When using an IDE agent such as Copilot, it appears that after creating a stack, the AI does not always load the stack that was just created. This is unexpected behavior, as the most intuitive use case is to automatically select the newly created stack.
This PR is a simpler version of #1869
Linked Issues
#1853
Checklist
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.