Skip to content

Commit 3cb1a75

Browse files
wbrezaCopilot
andcommitted
fix: rebuild cobra command tree on each workflow re-execution
The workflowCmdAdapter previously cached a single rootCmd cobra command tree as a singleton. When the ErrorMiddleware retry loop or gRPC WorkflowService re-executed commands (e.g. after Copilot agent fixes a provision failure), the stale tree with cancelled contexts caused 'context cancelled' exceptions. Changed workflowCmdAdapter to use a command factory that calls NewRootCmd() on each ExecuteContext() call, producing a fresh cobra command tree with no stale state. This follows the same proven pattern used in auto_install.go. Fixes Azure#6530 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 890d937 commit 3cb1a75

2 files changed

Lines changed: 96 additions & 45 deletions

File tree

cli/azd/cmd/container.go

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -891,13 +891,12 @@ func registerCommonDependencies(container *ioc.NestedContainer) {
891891
container.MustRegisterNamedSingleton(platformName, constructor)
892892
}
893893

894-
container.MustRegisterSingleton(func(s ioc.ServiceLocator) (workflow.AzdCommandRunner, error) {
895-
var rootCmd *cobra.Command
896-
if err := s.ResolveNamed("root-cmd", &rootCmd); err != nil {
897-
return nil, err
894+
container.MustRegisterSingleton(func() workflow.AzdCommandRunner {
895+
return &workflowCmdAdapter{
896+
newCommand: func() *cobra.Command {
897+
return NewRootCmd(false, nil, container)
898+
},
898899
}
899-
return &workflowCmdAdapter{cmd: rootCmd}, nil
900-
901900
})
902901
container.MustRegisterSingleton(workflow.NewRunner)
903902

@@ -948,19 +947,29 @@ func registerCommonDependencies(container *ioc.NestedContainer) {
948947
registerAction[*configShowAction](container, "azd-config-show-action")
949948
}
950949

951-
// workflowCmdAdapter adapts a cobra command to the workflow.AzdCommandRunner interface
950+
// workflowCmdAdapter adapts a cobra command to the workflow.AzdCommandRunner interface.
951+
// On each ExecuteContext call, a fresh cobra command tree is built via the newCommand factory
952+
// to avoid stale context or command state from previous executions.
953+
// See: https://github.com/Azure/azure-dev/issues/6530
952954
type workflowCmdAdapter struct {
953-
cmd *cobra.Command
955+
newCommand func() *cobra.Command
956+
args []string
954957
}
955958

956959
func (w *workflowCmdAdapter) SetArgs(args []string) {
957-
w.cmd.SetArgs(args)
960+
w.args = args
958961
}
959962

960-
// ExecuteContext implements workflow.AzdCommandRunner
963+
// ExecuteContext implements workflow.AzdCommandRunner.
964+
// It rebuilds the cobra command tree on each call to ensure a clean slate,
965+
// preventing "context cancelled" errors from stale command state during retries.
961966
func (w *workflowCmdAdapter) ExecuteContext(ctx context.Context) error {
962967
childCtx := middleware.WithChildAction(ctx)
963-
return w.cmd.ExecuteContext(childCtx)
968+
rootCmd := w.newCommand()
969+
if w.args != nil {
970+
rootCmd.SetArgs(w.args)
971+
}
972+
return rootCmd.ExecuteContext(childCtx)
964973
}
965974

966975
// ArmClientInitializer is a function definition for all Azure SDK ARM Client

cli/azd/cmd/container_test.go

Lines changed: 76 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -170,24 +170,27 @@ func Test_WorkflowCmdAdapter_ContextPropagation(t *testing.T) {
170170
// Track which contexts were seen by the subcommand
171171
var receivedContexts []context.Context
172172

173-
// Create a root command with a subcommand
174-
rootCmd := &cobra.Command{
175-
Use: "root",
176-
}
173+
// Create a command factory that builds a fresh tree on each call
174+
newCommand := func() *cobra.Command {
175+
rootCmd := &cobra.Command{
176+
Use: "root",
177+
}
177178

178-
subCmd := &cobra.Command{
179-
Use: "sub",
180-
RunE: func(cmd *cobra.Command, args []string) error {
181-
// Capture the context that the subcommand receives
182-
receivedContexts = append(receivedContexts, cmd.Context())
183-
return nil
184-
},
185-
}
179+
subCmd := &cobra.Command{
180+
Use: "sub",
181+
RunE: func(cmd *cobra.Command, args []string) error {
182+
// Capture the context that the subcommand receives
183+
receivedContexts = append(receivedContexts, cmd.Context())
184+
return nil
185+
},
186+
}
186187

187-
rootCmd.AddCommand(subCmd)
188+
rootCmd.AddCommand(subCmd)
189+
return rootCmd
190+
}
188191

189-
// Create the adapter
190-
adapter := &workflowCmdAdapter{cmd: rootCmd}
192+
// Create the adapter with a factory
193+
adapter := &workflowCmdAdapter{newCommand: newCommand}
191194

192195
// In production, main.go wraps with context.WithoutCancel.
193196
// Simulate this by using a non-cancellable context.
@@ -209,7 +212,7 @@ func Test_WorkflowCmdAdapter_ContextPropagation(t *testing.T) {
209212
// Expected: context is still valid
210213
}
211214

212-
// Execute again - should still work
215+
// Execute again - should still work (fresh command tree each time)
213216
adapter.SetArgs([]string{"sub"})
214217
err = adapter.ExecuteContext(ctx)
215218
require.NoError(t, err)
@@ -224,27 +227,30 @@ func Test_WorkflowCmdAdapter_ContextPropagation(t *testing.T) {
224227
// Track which contexts were seen
225228
var receivedContexts []context.Context
226229

227-
// Create a root command with nested subcommands
228-
rootCmd := &cobra.Command{
229-
Use: "root",
230-
}
230+
// Create a command factory that builds a fresh tree on each call
231+
newCommand := func() *cobra.Command {
232+
rootCmd := &cobra.Command{
233+
Use: "root",
234+
}
231235

232-
parentCmd := &cobra.Command{
233-
Use: "parent",
234-
}
236+
parentCmd := &cobra.Command{
237+
Use: "parent",
238+
}
235239

236-
childCmd := &cobra.Command{
237-
Use: "child",
238-
RunE: func(cmd *cobra.Command, args []string) error {
239-
receivedContexts = append(receivedContexts, cmd.Context())
240-
return nil
241-
},
242-
}
240+
childCmd := &cobra.Command{
241+
Use: "child",
242+
RunE: func(cmd *cobra.Command, args []string) error {
243+
receivedContexts = append(receivedContexts, cmd.Context())
244+
return nil
245+
},
246+
}
243247

244-
parentCmd.AddCommand(childCmd)
245-
rootCmd.AddCommand(parentCmd)
248+
parentCmd.AddCommand(childCmd)
249+
rootCmd.AddCommand(parentCmd)
250+
return rootCmd
251+
}
246252

247-
adapter := &workflowCmdAdapter{cmd: rootCmd}
253+
adapter := &workflowCmdAdapter{newCommand: newCommand}
248254

249255
// In production, main.go wraps with context.WithoutCancel.
250256
ctx := context.WithoutCancel(context.Background())
@@ -257,7 +263,7 @@ func Test_WorkflowCmdAdapter_ContextPropagation(t *testing.T) {
257263
require.True(t, middleware.IsChildAction(receivedContexts[0]),
258264
"Nested context should be marked as child action")
259265

260-
// Second execution should also work
266+
// Second execution should also work (fresh command tree)
261267
adapter.SetArgs([]string{"parent", "child"})
262268
err = adapter.ExecuteContext(ctx)
263269
require.NoError(t, err)
@@ -274,4 +280,40 @@ func Test_WorkflowCmdAdapter_ContextPropagation(t *testing.T) {
274280
require.True(t, middleware.IsChildAction(receivedContexts[1]),
275281
"Second nested context should also be marked as child action")
276282
})
283+
284+
t.Run("FreshCommandTreeOnEachExecution", func(t *testing.T) {
285+
// Verify that each ExecuteContext call creates a new command tree,
286+
// ensuring no stale state from previous executions.
287+
var commandTreeInstances []*cobra.Command
288+
289+
newCommand := func() *cobra.Command {
290+
rootCmd := &cobra.Command{
291+
Use: "root",
292+
}
293+
rootCmd.AddCommand(&cobra.Command{
294+
Use: "test",
295+
RunE: func(cmd *cobra.Command, args []string) error {
296+
return nil
297+
},
298+
})
299+
commandTreeInstances = append(commandTreeInstances, rootCmd)
300+
return rootCmd
301+
}
302+
303+
adapter := &workflowCmdAdapter{newCommand: newCommand}
304+
ctx := context.WithoutCancel(context.Background())
305+
306+
adapter.SetArgs([]string{"test"})
307+
err := adapter.ExecuteContext(ctx)
308+
require.NoError(t, err)
309+
310+
adapter.SetArgs([]string{"test"})
311+
err = adapter.ExecuteContext(ctx)
312+
require.NoError(t, err)
313+
314+
// Each execution should have created a distinct command tree
315+
require.Len(t, commandTreeInstances, 2, "Factory should have been called twice")
316+
require.NotSame(t, commandTreeInstances[0], commandTreeInstances[1],
317+
"Each execution should use a distinct command tree instance")
318+
})
277319
}

0 commit comments

Comments
 (0)