diff --git a/cli/azd/extensions/azure.ai.agents/README.md b/cli/azd/extensions/azure.ai.agents/README.md index b9c6b46b30a..4bfb3f6653e 100644 --- a/cli/azd/extensions/azure.ai.agents/README.md +++ b/cli/azd/extensions/azure.ai.agents/README.md @@ -13,6 +13,26 @@ Use `--no-inspector` to run only the local agent process: azd ai agent run --no-inspector ``` +The Agent Inspector UI binds port `8087` by default. Use `--inspector-port` to +move it, which is what you need when running two agents side by side or when a +stale process still holds the default port: + +```bash +azd ai agent run --port 9091 --inspector-port 9002 +``` + +`--inspector-port` is rejected when it cannot be honored: + +- with `--no-client` (or the deprecated `--no-inspector`), since no local client + is opened and the port would go unused; and +- for agents that use Agent Inspector, when it matches `--port`, since the agent + binds that address first and the inspector would then fail to bind it. + +azd also warns, without failing the run, when `--inspector-port` cannot take +effect: activity-protocol agents open the Microsoft 365 Agents Playground rather +than the Agent Inspector, and `--port 8087` on its own collides with the +inspector's own default UI port. + ## Migrating Legacy Agent Configuration New Foundry agent projects keep the agent definition directly on the diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go index e2d0c69dca1..51faf7fb53c 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go @@ -484,6 +484,13 @@ func (s *helpersProjectServer) Get( return &azdext.GetProjectResponse{Project: s.project}, nil } +func (s *helpersProjectServer) GetServiceConfigValue( + _ context.Context, + _ *azdext.GetServiceConfigValueRequest, +) (*azdext.GetServiceConfigValueResponse, error) { + return &azdext.GetServiceConfigValueResponse{}, nil +} + // helpersPromptServer is a fake PromptServiceServer that records Select calls // and returns a canned choice index. type helpersPromptServer struct { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go index 93184537e82..272e618a278 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go @@ -25,6 +25,7 @@ import ( "time" "azureaiagent/internal/cmd/nextstep" + "azureaiagent/internal/exterrors" "azureaiagent/internal/pkg/agents/agent_yaml" "azureaiagent/internal/project" @@ -37,15 +38,28 @@ import ( const ( agentInspectorExtensionID = "azure.ai.inspector" agentInspectorReadyPollPeriod = 250 * time.Millisecond + // defaultInspectorUIPort mirrors the default UI port of the + // azure.ai.inspector extension. The inspector extension remains the source + // of truth for the actual default: when --inspector-port is unset we do not + // forward the flag. This constant is only used to describe that default in + // help text and to warn about a likely bind conflict, never to assert it. + defaultInspectorUIPort = 8087 ) type runFlags struct { - port int - name string - startCommand string - noInspector bool - noClient bool - channel string + port int + // inspectorPort is the port the Agent Inspector UI listens on. When + // inspectorPortSet is false the flag was not supplied and + // --inspector-port is not forwarded to the inspector. + inspectorPort int + // inspectorPortSet records whether --inspector-port was explicitly + // supplied, so an explicit (and invalid) 0 is not mistaken for unset. + inspectorPortSet bool + name string + startCommand string + noInspector bool + noClient bool + channel string } func newRunCommand(extCtx *azdext.ExtensionContext) *cobra.Command { @@ -79,6 +93,9 @@ Playground for activity agents. Use --no-client to skip this.`, # Start on a custom port azd ai agent run --port 9090 + # Start a second agent with its own Agent Inspector UI port + azd ai agent run --port 9091 --inspector-port 9002 + # Start without opening a local client azd ai agent run --no-client @@ -89,12 +106,15 @@ Playground for activity agents. Use --no-client to skip this.`, if len(args) > 0 { flags.name = args[0] } + flags.inspectorPortSet = cmd.Flags().Changed("inspector-port") ctx := azdext.WithAccessToken(cmd.Context()) return runRun(ctx, flags, extCtx.NoPrompt) }, } cmd.Flags().IntVarP(&flags.port, "port", "p", DefaultPort, "Port to listen on") + cmd.Flags().IntVar(&flags.inspectorPort, "inspector-port", 0, + fmt.Sprintf("Port the Agent Inspector UI listens on (default: %d)", defaultInspectorUIPort)) cmd.Flags().StringVarP(&flags.startCommand, "start-command", "c", "", "Explicit startup command (overrides azure.yaml and auto-detection)") cmd.Flags().BoolVar(&flags.noInspector, "no-inspector", false, "Do not open the local client (Agent Inspector or Playground)") @@ -111,6 +131,10 @@ Playground for activity agents. Use --no-client to skip this.`, } func runRun(ctx context.Context, flags *runFlags, noPrompt bool) error { + if err := validateInspectorPortFlags(flags); err != nil { + return err + } + azdClient, err := azdext.NewAzdClient() if err != nil { return fmt.Errorf("failed to create azd client: %w", err) @@ -124,6 +148,14 @@ func runRun(ctx context.Context, flags *runFlags, noPrompt bool) error { } projectDir := runCtx.ProjectDir + // Resolve the activity profile before registering session cleanup. Port + // validation can fail without starting a process, and such a failure must + // not clear a session belonging to an already-running agent. + activityProfile := resolveActivityRunProfile(runCtx.Definition) + if err := validateInspectorPortForProfile(flags, activityProfile.IsActivity); err != nil { + return err + } + // Clean up stored local session when the agent process exits. localAgentKey := resolveLocalAgentKeyWithPort(ctx, azdClient, runCtx.ServiceName, noPrompt, flags.port) defer func() { @@ -136,12 +168,21 @@ func runRun(ctx context.Context, flags *runFlags, noPrompt bool) error { // environment setup (e.g., setting ASPNETCORE_URLS for .NET). pt := detectProjectType(projectDir) - // Detect whether the target service is an activity agent. - // This is the single gate that keeps all activity-specific local behavior off - // the path of non-activity (responses/invocations) agents — they are entirely - // unaffected. Detection is self-contained (reads the agent definition), so - // this command has no dependency on the deploy-side activity work. - activityProfile := resolveActivityRunProfile(runCtx.Definition) + // Resolve local-client availability before the agent starts so advisory + // port warnings can account for whether an inspector will actually launch. + // Reuse the result after proc.Start rather than issuing a second RPC. + suppressClient := flags.noInspector || flags.noClient + inspectorInstalled := false + var inspectorInstallErr error + if !activityProfile.IsActivity && !suppressClient { + inspectorInstalled, inspectorInstallErr = isInspectorExtensionInstalled(ctx, azdClient) + } + + // Surface the advisory --inspector-port problems before the agent starts. + // Once proc.Start() runs, this would land under the dependency install + // output and compete with the agent's own stdout/stderr, where it is easy + // to scroll past. + warnInspectorPortIssues(flags, activityProfile.IsActivity, inspectorInstalled, os.Stderr) // Resolve start command: --start-command flag > azure.yaml startupCommand > detect startCmd := flags.startCommand @@ -277,19 +318,14 @@ func runRun(ctx context.Context, flags *runFlags, noPrompt bool) error { // agents use the Microsoft 365 Agents Playground (the only local client that // speaks the Activity protocol); everything else uses Agent Inspector. Both // are suppressed by --no-inspector or its neutral alias --no-client. - suppressClient := flags.noInspector || flags.noClient if activityProfile.IsActivity { handlePlaygroundAutoLaunch(ctx, flags.port, flags.channel, suppressClient, os.Stderr) } else { - inspectorInstalled := false - var inspectorInstallErr error - if !suppressClient { - inspectorInstalled, inspectorInstallErr = isInspectorExtensionInstalled(ctx, azdClient) - } handleInspectorAutoLaunch( ctx, azdClient.Workflow(), flags.port, + flags.inspectorPort, suppressClient, inspectorInstalled, inspectorInstallErr, @@ -347,6 +383,7 @@ func handleInspectorAutoLaunch( ctx context.Context, workflow azdext.WorkflowServiceClient, agentPort int, + inspectorPort int, noInspector bool, inspectorInstalled bool, inspectorInstallErr error, @@ -367,6 +404,7 @@ func handleInspectorAutoLaunch( ctx, workflow, agentPort, + inspectorPort, agentInspectorReadyPollPeriod, stderr, ) @@ -376,6 +414,7 @@ func startInspectorAfterAgentReadyWithOptions( ctx context.Context, workflow azdext.WorkflowServiceClient, agentPort int, + inspectorPort int, pollPeriod time.Duration, stderr io.Writer, ) { @@ -392,7 +431,7 @@ func startInspectorAfterAgentReadyWithOptions( return } - if err := launchInspector(ctx, workflow, agentPort); err != nil && !isContextCancellation(err) { + if err := launchInspector(ctx, workflow, agentPort, inspectorPort); err != nil && !isContextCancellation(err) { fmt.Fprintln(stderr, inspectorLaunchWarning(err)) } }() @@ -422,21 +461,33 @@ func waitForLocalPort(ctx context.Context, port int, pollPeriod time.Duration) e } } -func launchInspector(ctx context.Context, workflow azdext.WorkflowServiceClient, agentPort int) error { +func launchInspector( + ctx context.Context, + workflow azdext.WorkflowServiceClient, + agentPort int, + inspectorPort int, +) error { + args := []string{ + "ai", + "inspector", + "launch", + "--port", + strconv.Itoa(agentPort), + } + // Only forward --inspector-port when the user asked for a specific UI port, + // so the inspector extension keeps applying its own default otherwise. + if inspectorPort > 0 { + args = append(args, "--inspector-port", strconv.Itoa(inspectorPort)) + } + args = append(args, "--silent") + _, err := workflow.Run(ctx, &azdext.RunWorkflowRequest{ Workflow: &azdext.Workflow{ Name: "launch-agent-inspector", Steps: []*azdext.WorkflowStep{ { Command: &azdext.WorkflowCommand{ - Args: []string{ - "ai", - "inspector", - "launch", - "--port", - strconv.Itoa(agentPort), - "--silent", - }, + Args: args, }, }, }, @@ -445,6 +496,120 @@ func launchInspector(ctx context.Context, workflow azdext.WorkflowServiceClient, return err } +// validateInspectorPort rejects out-of-range --inspector-port values. When the +// flag was not supplied (set is false) the inspector extension applies its own +// default UI port. An explicitly supplied zero is out of range and rejected. +// Validating here keeps an invalid value from being silently dropped or failing +// later inside the inspector with a less obvious message. +func validateInspectorPort(inspectorPort int, set bool) error { + if !set || (inspectorPort >= 1 && inspectorPort <= 65535) { + return nil + } + + return exterrors.Validation( + exterrors.CodeInvalidParameter, + fmt.Sprintf("--inspector-port must be between 1 and 65535, got %d", inspectorPort), + "pass a free TCP port, for example --inspector-port 9002", + ) +} + +// validateInspectorPortFlags checks --inspector-port against its own range and +// against the client-suppression flags that would silently discard it: +// +// - --no-client (or the deprecated --no-inspector) suppresses the local +// client entirely, so the inspector never launches and the port is unused. +func validateInspectorPortFlags(flags *runFlags) error { + if err := validateInspectorPort(flags.inspectorPort, flags.inspectorPortSet); err != nil { + return err + } + if !flags.inspectorPortSet { + return nil + } + + if flags.noClient || flags.noInspector { + // Name the flag the user actually passed; --no-inspector is deprecated + // but still accepted. + suppressFlag := "--no-client" + if flags.noInspector && !flags.noClient { + suppressFlag = "--no-inspector" + } + return exterrors.Validation( + exterrors.CodeConflictingArguments, + fmt.Sprintf("--inspector-port cannot be used with %s", suppressFlag), + fmt.Sprintf( + "drop %s to open the Agent Inspector on that port, or drop --inspector-port to run without a local client", + suppressFlag, + ), + ) + } + + return nil +} + +// validateInspectorPortForProfile rejects an inspector/agent port collision +// only when the resolved agent actually launches the Agent Inspector. Activity +// agents launch the Playground instead, so --inspector-port is advisory and the +// equal values do not contend for the same listener. +func validateInspectorPortForProfile(flags *runFlags, isActivity bool) error { + if !isActivity && flags.inspectorPortSet && flags.inspectorPort == flags.port { + return exterrors.Validation( + exterrors.CodeConflictingArguments, + fmt.Sprintf( + "--inspector-port must differ from --port; both are %d and cannot bind the same address", + flags.inspectorPort, + ), + "pass a different free TCP port, for example --inspector-port 9002", + ) + } + + return nil +} + +// warnInspectorPortIssues emits the --inspector-port problems that are advisory +// rather than fatal, so they can be surfaced before the agent process starts. +// The fatal combinations are rejected up front by validateInspectorPortFlags +// and validateInspectorPortForProfile. +// +// Two cases warn instead of failing: +// +// - Activity-protocol agents open the Playground, which has no inspector UI +// port. Which client a service gets is only known after the definition is +// resolved, so a user cannot reliably predict it from the command line. +// - The agent port matches the inspector's default UI port while +// --inspector-port is unset and the inspector extension is installed. The +// inspector extension owns that default, so azd flags the likely bind +// conflict rather than asserting a value it does not control. +func warnInspectorPortIssues( + flags *runFlags, + isActivity bool, + inspectorInstalled bool, + stderr io.Writer, +) { + if isActivity { + if flags.inspectorPortSet { + fmt.Fprintln(stderr, + "Warning: --inspector-port is ignored for activity-protocol agents, "+ + "which open the Microsoft 365 Agents Playground instead of the Agent Inspector.") + } + return + } + + // No inspector launches, so no port is used and nothing can collide. + // (--inspector-port with a suppressed client is already a hard error.) + if flags.noInspector || flags.noClient { + return + } + + // An explicit --inspector-port equal to --port is already a hard error; this + // covers the unset case, where the inspector falls back to its own default. + if inspectorInstalled && !flags.inspectorPortSet && flags.port == defaultInspectorUIPort { + fmt.Fprintf(stderr, + "Warning: --port %d is also the Agent Inspector UI's default port, so the inspector may fail to start.\n"+ + "Pass --inspector-port to move the Agent Inspector UI to a free port.\n", + flags.port) + } +} + func isInspectorExtensionInstalled(ctx context.Context, azdClient *azdext.AzdClient) (bool, error) { configHelper, err := azdext.NewConfigHelper(azdClient) if err != nil { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go index 014f4855f5f..f94f6504498 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go @@ -16,6 +16,7 @@ import ( "path/filepath" "runtime" "slices" + "strconv" "strings" "sync" "testing" @@ -240,19 +241,381 @@ func TestWaitForLocalPort(t *testing.T) { func TestLaunchInspectorUsesWorkflowCommand(t *testing.T) { t.Parallel() - workflow := &recordingWorkflowClient{} - if err := launchInspector(t.Context(), workflow, 9090); err != nil { - t.Fatalf("launchInspector returned error: %v", err) + tests := []struct { + name string + agentPort int + inspectorPort int + want []string + }{ + { + name: "inspector port unset is not forwarded", + agentPort: 9090, + want: []string{"ai", "inspector", "launch", "--port", "9090", "--silent"}, + }, + { + name: "inspector port is forwarded when set", + agentPort: 9091, + inspectorPort: 9002, + want: []string{ + "ai", "inspector", "launch", + "--port", "9091", + "--inspector-port", "9002", + "--silent", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + workflow := &recordingWorkflowClient{} + if err := launchInspector(t.Context(), workflow, tt.agentPort, tt.inspectorPort); err != nil { + t.Fatalf("launchInspector returned error: %v", err) + } + + if workflow.request == nil || workflow.request.Workflow == nil || + len(workflow.request.Workflow.Steps) != 1 { + t.Fatalf("unexpected workflow request: %#v", workflow.request) + } + + got := workflow.request.Workflow.Steps[0].Command.Args + if !slices.Equal(got, tt.want) { + t.Fatalf("workflow args = %v, want %v", got, tt.want) + } + }) } +} + +func TestRunCommandInspectorPortFlag(t *testing.T) { + t.Parallel() + + cmd := newRunCommand(nil) + + flag := cmd.Flags().Lookup("inspector-port") + if flag == nil { + t.Fatal("run command should expose --inspector-port") + } + // Zero means unset so the inspector extension keeps applying its own + // default UI port; the effective default is documented in the usage text. + if flag.DefValue != "0" { + t.Fatalf("--inspector-port default = %q, want %q", flag.DefValue, "0") + } + if !strings.Contains(flag.Usage, strconv.Itoa(defaultInspectorUIPort)) { + t.Fatalf("--inspector-port usage should document the %d default, got %q", + defaultInspectorUIPort, flag.Usage) + } +} - if workflow.request == nil || workflow.request.Workflow == nil || len(workflow.request.Workflow.Steps) != 1 { - t.Fatalf("unexpected workflow request: %#v", workflow.request) +func TestValidateInspectorPort(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + port int + set bool + wantErr bool + }{ + {name: "unset is allowed", port: 0}, + {name: "explicit zero is rejected", port: 0, set: true, wantErr: true}, + {name: "lower bound", port: 1, set: true}, + {name: "typical port", port: 9002, set: true}, + {name: "upper bound", port: 65535, set: true}, + {name: "negative is rejected", port: -1, set: true, wantErr: true}, + {name: "above range is rejected", port: 70000, set: true, wantErr: true}, } - got := workflow.request.Workflow.Steps[0].Command.Args - want := []string{"ai", "inspector", "launch", "--port", "9090", "--silent"} - if !slices.Equal(got, want) { - t.Fatalf("workflow args = %v, want %v", got, want) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := validateInspectorPort(tt.port, tt.set) + if tt.wantErr { + if err == nil { + t.Fatalf("validateInspectorPort(%d, %t) = nil, want error", tt.port, tt.set) + } + if !strings.Contains(err.Error(), "--inspector-port") { + t.Fatalf("error should name the flag, got %q", err.Error()) + } + return + } + if err != nil { + t.Fatalf("validateInspectorPort(%d, %t) = %v, want nil", tt.port, tt.set, err) + } + }) + } +} + +func TestValidateInspectorPortFlags(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + flags runFlags + wantErr bool + wantErrPart string + }{ + { + name: "unset flag with suppressed client is allowed", + flags: runFlags{port: DefaultPort, noClient: true}, + }, + { + name: "distinct ports are allowed", + flags: runFlags{port: 9091, inspectorPort: 9002, inspectorPortSet: true}, + }, + { + name: "out of range still rejected", + flags: runFlags{port: DefaultPort, inspectorPort: 70000, inspectorPortSet: true}, + wantErr: true, + wantErrPart: "between 1 and 65535", + }, + { + name: "conflicts with --no-client", + flags: runFlags{port: DefaultPort, inspectorPort: 9002, inspectorPortSet: true, noClient: true}, + wantErr: true, + wantErrPart: "--no-client", + }, + { + name: "conflicts with deprecated --no-inspector", + flags: runFlags{port: DefaultPort, inspectorPort: 9002, inspectorPortSet: true, noInspector: true}, + wantErr: true, + wantErrPart: "--no-inspector", + }, + { + name: "both suppress flags name the canonical one", + flags: runFlags{ + port: DefaultPort, inspectorPort: 9002, inspectorPortSet: true, + noClient: true, noInspector: true, + }, + wantErr: true, + wantErrPart: "--no-client", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + flags := tt.flags + err := validateInspectorPortFlags(&flags) + if tt.wantErr { + if err == nil { + t.Fatalf("validateInspectorPortFlags(%+v) = nil, want error", tt.flags) + } + if !strings.Contains(err.Error(), tt.wantErrPart) { + t.Fatalf("error = %q, want it to contain %q", err.Error(), tt.wantErrPart) + } + return + } + if err != nil { + t.Fatalf("validateInspectorPortFlags(%+v) = %v, want nil", tt.flags, err) + } + }) + } +} + +func TestValidateInspectorPortForProfile(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + flags runFlags + isActivity bool + wantErrPart string + }{ + { + name: "non-activity agent rejects equal ports", + flags: runFlags{port: 9091, inspectorPort: 9091, inspectorPortSet: true}, + wantErrPart: "must differ from --port", + }, + { + name: "activity agent allows equal ports because it opens the Playground", + flags: runFlags{port: 9091, inspectorPort: 9091, inspectorPortSet: true}, + isActivity: true, + }, + { + name: "non-activity agent allows distinct ports", + flags: runFlags{port: 9091, inspectorPort: 9002, inspectorPortSet: true}, + }, + { + name: "collision check ignores an unset inspector port", + flags: runFlags{port: 0}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + flags := tt.flags + err := validateInspectorPortForProfile(&flags, tt.isActivity) + if tt.wantErrPart != "" { + if err == nil { + t.Fatalf("validateInspectorPortForProfile(%+v, %t) = nil, want error", tt.flags, tt.isActivity) + } + if !strings.Contains(err.Error(), tt.wantErrPart) { + t.Fatalf("error = %q, want it to contain %q", err.Error(), tt.wantErrPart) + } + return + } + if err != nil { + t.Fatalf( + "validateInspectorPortForProfile(%+v, %t) = %v, want nil", + tt.flags, + tt.isActivity, + err, + ) + } + }) + } +} + +func TestRunRun_PortCollisionDoesNotClearStoredSession(t *testing.T) { + projectDir := t.TempDir() + projectServer := &helpersProjectServer{ + project: &azdext.ProjectConfig{ + Name: "test-project", + Path: projectDir, + Services: map[string]*azdext.ServiceConfig{ + "agent": { + Name: "agent", + Host: AiAgentHost, + RelativePath: ".", + }, + }, + }, + } + userConfigServer := newInvokeUserConfigServer() + + grpcServer := grpc.NewServer() + azdext.RegisterProjectServiceServer(grpcServer, projectServer) + azdext.RegisterUserConfigServiceServer(grpcServer, userConfigServer) + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + go func() { _ = grpcServer.Serve(listener) }() + t.Cleanup(func() { + grpcServer.Stop() + _ = listener.Close() + }) + t.Setenv("AZD_SERVER", listener.Addr().String()) + + const ( + port = 9091 + sessionID = "existing-session" + ) + agentKey := buildLocalAgentKey(port, "agent", "", projectDir) + userConfigServer.setJSON(t, configPath("sessions"), map[string]string{ + agentKey: sessionID, + }) + + err = runRun(t.Context(), &runFlags{ + name: "agent", + port: port, + inspectorPort: port, + inspectorPortSet: true, + }, true) + if err == nil || !strings.Contains(err.Error(), "must differ from --port") { + t.Fatalf("runRun() error = %v, want equal-port validation error", err) + } + + userConfigServer.mu.Lock() + stored := slices.Clone(userConfigServer.values[configPath("sessions")]) + userConfigServer.mu.Unlock() + if !bytes.Contains(stored, []byte(agentKey)) || !bytes.Contains(stored, []byte(sessionID)) { + t.Fatalf("session store = %s, want existing session %q at key %q", stored, sessionID, agentKey) + } +} + +func TestWarnInspectorPortIssues(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + flags runFlags + isActivity bool + inspectorInstalled bool + wantParts []string + wantSilent bool + }{ + { + name: "activity agent with an explicit inspector port warns", + flags: runFlags{port: 9091, inspectorPort: 9002, inspectorPortSet: true}, + isActivity: true, + wantParts: []string{"--inspector-port is ignored", "Playground"}, + }, + { + name: "activity agent without the flag is silent", + flags: runFlags{port: 9091}, + isActivity: true, + wantSilent: true, + }, + { + name: "agent port on the inspector default warns when inspector is installed", + flags: runFlags{port: defaultInspectorUIPort}, + inspectorInstalled: true, + wantParts: []string{"also the Agent Inspector UI's default port", "Pass --inspector-port"}, + }, + { + name: "missing inspector suppresses the default collision warning", + flags: runFlags{port: defaultInspectorUIPort}, + wantSilent: true, + }, + { + name: "explicit inspector port suppresses the default collision warning", + flags: runFlags{port: defaultInspectorUIPort, inspectorPort: 9002, inspectorPortSet: true}, + inspectorInstalled: true, + wantSilent: true, + }, + { + name: "suppressed client launches no inspector, so nothing can collide", + flags: runFlags{port: defaultInspectorUIPort, noClient: true}, + inspectorInstalled: true, + wantSilent: true, + }, + { + name: "deprecated suppress flag is also honored", + flags: runFlags{port: defaultInspectorUIPort, noInspector: true}, + inspectorInstalled: true, + wantSilent: true, + }, + { + name: "unrelated agent port is silent", + flags: runFlags{port: DefaultPort}, + inspectorInstalled: true, + wantSilent: true, + }, + { + name: "activity agent on the inspector default port is silent", + flags: runFlags{port: defaultInspectorUIPort}, + isActivity: true, + wantSilent: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + flags := tt.flags + var stderr bytes.Buffer + warnInspectorPortIssues(&flags, tt.isActivity, tt.inspectorInstalled, &stderr) + + got := stderr.String() + if tt.wantSilent { + if got != "" { + t.Fatalf("expected no warning, got %q", got) + } + return + } + for _, part := range tt.wantParts { + if !strings.Contains(got, part) { + t.Fatalf("warning = %q, want it to contain %q", got, part) + } + } + }) } } @@ -309,6 +672,7 @@ func TestInspectorLaunchFailureOnlyWarns(t *testing.T) { ctx, workflow, ln.Addr().(*net.TCPAddr).Port, + 0, time.Millisecond, &stderr, ) @@ -364,7 +728,7 @@ func TestNoInspectorSkipsWorkflowLaunch(t *testing.T) { t.Parallel() workflow := &recordingWorkflowClient{called: make(chan struct{})} - handleInspectorAutoLaunch(t.Context(), workflow, 8088, true, true, nil, io.Discard) + handleInspectorAutoLaunch(t.Context(), workflow, 8088, 0, true, true, nil, io.Discard) select { case <-workflow.called: