diff --git a/internal/cli/declarative/inspector/inspector.go b/internal/cli/declarative/inspector/inspector.go new file mode 100644 index 00000000..9e79b35f --- /dev/null +++ b/internal/cli/declarative/inspector/inspector.go @@ -0,0 +1,40 @@ +// Package inspector launches MCP Inspector as a subprocess against a given +// MCP server URL. The implementation is a thin wrapper around +// `npx -y @modelcontextprotocol/inspector --server-url ` — it has no +// independent protocol handling. The caller owns the returned process's +// lifecycle and must call Process.Kill() on shutdown. +package inspector + +import ( + "context" + "fmt" + "os" + "os/exec" +) + +// commandFactory matches the signature of exec.CommandContext so tests can +// inject a fake without invoking npx. +type commandFactory func(ctx context.Context, name string, args ...string) *exec.Cmd + +// starter matches the signature of (*exec.Cmd).Start so tests can avoid +// spawning processes during unit tests. +type starter func(cmd *exec.Cmd) error + +// Launch starts MCP Inspector as a subprocess pointed at serverURL. +// The subprocess's stdout/stderr are wired to the current process's streams. +// Returns the *exec.Cmd so the caller can call Process.Kill() on shutdown. +// Returns an error only if the subprocess fails to start (typically because +// npx is not on PATH). +func Launch(ctx context.Context, serverURL string) (*exec.Cmd, error) { + return launchWith(ctx, serverURL, exec.CommandContext, func(c *exec.Cmd) error { return c.Start() }) +} + +func launchWith(ctx context.Context, serverURL string, makeCmd commandFactory, start starter) (*exec.Cmd, error) { + cmd := makeCmd(ctx, "npx", "-y", "@modelcontextprotocol/inspector", "--server-url", serverURL) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if err := start(cmd); err != nil { + return nil, fmt.Errorf("starting MCP Inspector subprocess: %w", err) + } + return cmd, nil +} diff --git a/internal/cli/declarative/inspector/inspector_test.go b/internal/cli/declarative/inspector/inspector_test.go new file mode 100644 index 00000000..37e989c2 --- /dev/null +++ b/internal/cli/declarative/inspector/inspector_test.go @@ -0,0 +1,55 @@ +package inspector + +import ( + "context" + "os/exec" + "testing" +) + +func TestLaunch_BuildsExpectedArgv(t *testing.T) { + var gotName string + var gotArgs []string + fakeFactory := func(ctx context.Context, name string, args ...string) *exec.Cmd { + gotName = name + gotArgs = args + // Return a cmd we never actually start; the fake starter swallows it. + return exec.CommandContext(ctx, "echo") + } + + _, err := launchWith(context.Background(), "http://localhost:3000/mcp", fakeFactory, fakeStarter) + if err != nil { + t.Fatalf("launchWith returned error: %v", err) + } + if gotName != "npx" { + t.Errorf("expected npx, got %q", gotName) + } + want := []string{"-y", "@modelcontextprotocol/inspector", "--server-url", "http://localhost:3000/mcp"} + if len(gotArgs) != len(want) { + t.Fatalf("argv length: got %d want %d (%v)", len(gotArgs), len(want), gotArgs) + } + for i := range want { + if gotArgs[i] != want[i] { + t.Errorf("argv[%d]: got %q want %q", i, gotArgs[i], want[i]) + } + } +} + +func TestLaunch_ReturnsErrorWhenStartFails(t *testing.T) { + fakeFactory := func(ctx context.Context, name string, args ...string) *exec.Cmd { + return exec.CommandContext(ctx, "/nonexistent/path") + } + failingStarter := func(_ *exec.Cmd) error { + return exec.ErrNotFound + } + + cmd, err := launchWith(context.Background(), "http://localhost:3000/mcp", fakeFactory, failingStarter) + if err == nil { + t.Fatalf("expected error, got cmd=%v err=nil", cmd) + } + if cmd != nil { + t.Errorf("expected nil cmd on error, got %v", cmd) + } +} + +// fakeStarter returns nil so Launch treats it as a successful start. +func fakeStarter(_ *exec.Cmd) error { return nil } diff --git a/internal/cli/declarative/run.go b/internal/cli/declarative/run.go index 80e2de63..e6f03115 100644 --- a/internal/cli/declarative/run.go +++ b/internal/cli/declarative/run.go @@ -31,10 +31,11 @@ func NewRunCmd() *cobra.Command { func newRunCmd() *cobra.Command { var ( - extraEnv []string - dryRun bool - watch bool - noChat bool + extraEnv []string + dryRun bool + watch bool + noChat bool + inspector bool ) cmd := &cobra.Command{ Use: "run [DIRECTORY]", @@ -48,7 +49,9 @@ A2A chat. When chat exits the runtime is torn down. Use --no-chat to keep the old foreground-only behavior. For MCPServer kinds chat does not apply; the framework's run command runs -in the foreground until interrupted. +in the foreground until interrupted. Pass --inspector to launch the MCP +Inspector subprocess (requires 'npx' on PATH) alongside the server; the +Inspector retries until the server is reachable. Reads arctl.yaml to look up the matching framework by (framework, language) and dispatches to its run command. Loads .env (if present) and validates @@ -56,8 +59,9 @@ that the framework's required env vars are set.`, Example: ` arctl run arctl run ./myagent arctl run -e FOO=bar -e BAZ=qux - arctl run --no-chat - arctl run --watch`, + arctl run --no-chat # agent without chat + arctl run --watch # iterative dev loop + arctl run mymcp --inspector # MCP with MCP Inspector launched`, SilenceUsage: true, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -65,13 +69,14 @@ that the framework's required env vars are set.`, if err != nil { return err } - return runProject(cmd.OutOrStdout(), dir, extraEnv, dryRun, watch, noChat) + return runProject(cmd.Context(), cmd.OutOrStdout(), dir, extraEnv, dryRun, watch, noChat, inspector) }, } cmd.Flags().StringArrayVarP(&extraEnv, "env", "e", nil, "KEY=VALUE env override") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Skip actual exec; useful for tests") - cmd.Flags().BoolVar(&watch, "watch", false, "Rebuild and restart on file change") - cmd.Flags().BoolVar(&noChat, "no-chat", false, "Skip chat for Agents; run the framework command in the foreground") + cmd.Flags().BoolVar(&watch, "watch", false, "Rebuild and restart on file change (skips chat for agents; for chat open a second terminal)") + cmd.Flags().BoolVar(&noChat, "no-chat", false, "Skip chat for Agents; run the framework command in the foreground (agent projects only; errors on MCP projects)") + cmd.Flags().BoolVar(&inspector, "inspector", false, "Launch MCP Inspector alongside the server; it connects when ready (MCP projects only; errors on agent projects)") return cmd } @@ -96,7 +101,7 @@ func resolveProjectDir(args []string) (string, error) { return abs, nil } -func runProject(out io.Writer, projectDir string, extraEnv []string, dryRun, watch, noChat bool) error { +func runProject(ctx context.Context, out io.Writer, projectDir string, extraEnv []string, dryRun, watch, noChat, inspector bool) error { cfg, err := buildconfig.Read(projectDir) if err != nil { return err @@ -125,6 +130,17 @@ func runProject(out io.Writer, projectDir string, extraEnv []string, dryRun, wat return fmt.Errorf("no framework for framework=%s language=%s", cfg.Framework, cfg.Language) } + // Strict flag-vs-kind validation. Symmetric: --inspector errors on + // agent projects, --no-chat errors on MCP projects. Fail fast before + // any exec or dry-run narration so a typo'd flag gives clear feedback + // instead of being silently ignored. + if inspector && frameworkType == "agent" { + return fmt.Errorf("--inspector is only valid for MCP projects; this is an agent project (agents are inspected via chat, the default behavior of arctl run)") + } + if noChat && frameworkType == "mcp" { + return fmt.Errorf("--no-chat is only valid for agent projects; this is an MCP project (MCPs do not open a chat)") + } + name := filepath.Base(projectDir) dotEnv, err := LoadDotEnv(projectDir) @@ -185,7 +201,17 @@ func runProject(out io.Writer, projectDir string, extraEnv []string, dryRun, wat // surface ("Watching for changes…", "Change detected") without // shelling out to a long-running runtime. if watch { - return runWithWatch(out, projectDir, p, envv, dryRun) + // Agent + --watch is the no-chat foreground rebuild loop. Print a + // signpost so users know (a) where the agent is reachable and + // (b) that chat lives in another terminal. Suppress the chat hint + // when the user has explicitly opted out via --no-chat. + if frameworkType == "agent" { + fmt.Fprintf(out, "→ Agent at %s\n", agentReadinessURL) + if !noChat { + fmt.Fprintf(out, "→ For chat, open another terminal: arctl run %s\n", name) + } + } + return runWithWatch(ctx, out, projectDir, p, image, port, envv, dryRun, inspector) } // Chat default applies only to Agents (not MCPServers) and when the @@ -198,9 +224,22 @@ func runProject(out io.Writer, projectDir string, extraEnv []string, dryRun, wat if dryRun { fmt.Fprintf(out, "→ %s: %s\n", p.Name, strings.Join(rendered, " ")) + if inspector { + fmt.Fprintf(out, "→ would launch MCP Inspector against http://localhost:%d/mcp\n", port) + } fmt.Fprintln(out, "(dry-run; skipping exec)") return nil } + + // Inspector retries connecting on its own until the MCP is up, so launch + // it BEFORE the foreground docker run — the race window is invisible. + // Not blocking the MCP on a missing npx is intentional: debug tools + // should degrade gracefully, not gate the dev loop. + if inspector { + stop := launchInspector(out, port) + defer stop() + } + fmt.Fprintf(out, "→ %s: %s\n", p.Name, strings.Join(rendered, " ")) return frameworks.ExecForeground(p.Run, projectDir, vars, envv) } diff --git a/internal/cli/declarative/run_test.go b/internal/cli/declarative/run_test.go index cd6add00..82df6254 100644 --- a/internal/cli/declarative/run_test.go +++ b/internal/cli/declarative/run_test.go @@ -2,6 +2,7 @@ package declarative_test import ( "bytes" + "context" "os" "path/filepath" "testing" @@ -73,6 +74,79 @@ func TestRun_ChatDefault_DryRunNarratesFullLifecycle(t *testing.T) { require.Contains(t, out, "(dry-run; skipping exec)") } +// TestRun_InspectorOnAgentErrors verifies the strict-symmetric flag +// validation: --inspector is MCP-only and fails fast on agent projects +// before any exec or dry-run narration, with a message pointing the user +// at chat (the agent's equivalent inspection surface). +func TestRun_InspectorOnAgentErrors(t *testing.T) { + t.Setenv("GOOGLE_API_KEY", "fake") + tmp := t.TempDir() + cwd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(cwd) }) + + require.NoError(t, os.Chdir(tmp)) + initCmd := declarative.NewInitCmd() + initCmd.SetArgs([]string{"agent", "agentproj", "--framework", "adk", "--language", "python"}) + require.NoError(t, initCmd.Execute()) + + require.NoError(t, os.Chdir(filepath.Join(tmp, "agentproj"))) + cmd := declarative.NewRunCmd() + cmd.SetArgs([]string{"--inspector", "--dry-run"}) + err = cmd.Execute() + require.Error(t, err) + require.Contains(t, err.Error(), "--inspector is only valid for MCP projects") +} + +// TestRun_InspectorDryRunNarratesURL verifies that --inspector on an MCP +// project under --dry-run prints the inspector URL the user would see at +// runtime, so docs + CI can assert on the narration without spawning npx. +func TestRun_InspectorDryRunNarratesURL(t *testing.T) { + tmp := t.TempDir() + cwd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(cwd) }) + + require.NoError(t, os.Chdir(tmp)) + initCmd := declarative.NewInitCmd() + initCmd.SetArgs([]string{"mcp", "acme/inspmcp", "--framework", "fastmcp", "--language", "python"}) + require.NoError(t, initCmd.Execute()) + + require.NoError(t, os.Chdir(filepath.Join(tmp, "inspmcp"))) + cmd := declarative.NewRunCmd() + var buf bytes.Buffer + cmd.SetOut(&buf) + cmd.SetErr(&buf) + cmd.SetArgs([]string{"--inspector", "--dry-run"}) + require.NoError(t, cmd.Execute()) + + out := buf.String() + require.Contains(t, out, "would launch MCP Inspector") + require.Contains(t, out, "http://localhost:3000/mcp") + require.Contains(t, out, "(dry-run; skipping exec)") +} + +// TestRun_NoChatOnMCPErrors mirrors the above for the other direction: +// --no-chat is agent-only and fails fast on MCP projects. +func TestRun_NoChatOnMCPErrors(t *testing.T) { + tmp := t.TempDir() + cwd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(cwd) }) + + require.NoError(t, os.Chdir(tmp)) + initCmd := declarative.NewInitCmd() + initCmd.SetArgs([]string{"mcp", "acme/mcpproj", "--framework", "fastmcp", "--language", "python"}) + require.NoError(t, initCmd.Execute()) + + require.NoError(t, os.Chdir(filepath.Join(tmp, "mcpproj"))) + cmd := declarative.NewRunCmd() + cmd.SetArgs([]string{"--no-chat", "--dry-run"}) + err = cmd.Execute() + require.Error(t, err) + require.Contains(t, err.Error(), "--no-chat is only valid for agent projects") +} + // TestRun_DoesNotRequireAgentYAML proves the structural decoupling: run // reads arctl.yaml only. Removing agent.yaml from a freshly inited project // must not break run. @@ -96,3 +170,70 @@ func TestRun_DoesNotRequireAgentYAML(t *testing.T) { cmd.SetArgs([]string{"--dry-run"}) require.NoError(t, cmd.Execute()) } + +// TestRun_AgentWatch_DryRunNarratesSignpost verifies that the agent + --watch +// path emits the "where is my agent" + "open chat in another terminal" +// signpost so users know watch is the no-chat foreground iterate mode. +func TestRun_AgentWatch_DryRunNarratesSignpost(t *testing.T) { + t.Setenv("GOOGLE_API_KEY", "fake") + tmp := t.TempDir() + cwd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(cwd) }) + + require.NoError(t, os.Chdir(tmp)) + initCmd := declarative.NewInitCmd() + initCmd.SetArgs([]string{"agent", "watchagent", "--framework", "adk", "--language", "python"}) + require.NoError(t, initCmd.Execute()) + + require.NoError(t, os.Chdir(filepath.Join(tmp, "watchagent"))) + cmd := declarative.NewRunCmd() + var buf bytes.Buffer + cmd.SetOut(&buf) + cmd.SetErr(&buf) + cmd.SetArgs([]string{"--watch", "--dry-run"}) + // Pre-cancel so runWithWatch's fsnotify loop exits immediately after + // printing the signpost + watcher banner. Without this, --watch under + // dry-run blocks forever waiting on file events. + ctx, cancel := context.WithCancel(context.Background()) + cancel() + cmd.SetContext(ctx) + require.NoError(t, cmd.Execute()) + + out := buf.String() + require.Contains(t, out, "→ Agent at http://localhost:8080") + require.Contains(t, out, "→ For chat, open another terminal: arctl run watchagent") + require.Contains(t, out, "Watching for changes") +} + +// TestRun_AgentWatchNoChat_DryRunSuppressesChatHint verifies that when the +// user explicitly opts out of chat with --no-chat alongside --watch, the +// signpost drops the "open another terminal for chat" line — the user +// already said they don't want chat, no need to nag. +func TestRun_AgentWatchNoChat_DryRunSuppressesChatHint(t *testing.T) { + t.Setenv("GOOGLE_API_KEY", "fake") + tmp := t.TempDir() + cwd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(cwd) }) + + require.NoError(t, os.Chdir(tmp)) + initCmd := declarative.NewInitCmd() + initCmd.SetArgs([]string{"agent", "watchquiet", "--framework", "adk", "--language", "python"}) + require.NoError(t, initCmd.Execute()) + + require.NoError(t, os.Chdir(filepath.Join(tmp, "watchquiet"))) + cmd := declarative.NewRunCmd() + var buf bytes.Buffer + cmd.SetOut(&buf) + cmd.SetErr(&buf) + cmd.SetArgs([]string{"--watch", "--no-chat", "--dry-run"}) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + cmd.SetContext(ctx) + require.NoError(t, cmd.Execute()) + + out := buf.String() + require.Contains(t, out, "→ Agent at http://localhost:8080") + require.NotContains(t, out, "For chat, open another terminal") +} diff --git a/internal/cli/declarative/watch.go b/internal/cli/declarative/watch.go index ad367033..b924317e 100644 --- a/internal/cli/declarative/watch.go +++ b/internal/cli/declarative/watch.go @@ -2,19 +2,28 @@ package declarative import ( "context" + "crypto/rand" + "encoding/hex" "fmt" "io" "os" "os/exec" "path/filepath" "strings" + "syscall" "time" "github.com/fsnotify/fsnotify" + inspectorpkg "github.com/agentregistry-dev/agentregistry/internal/cli/declarative/inspector" "github.com/agentregistry-dev/agentregistry/internal/cli/frameworks" ) +// stopGracePeriod is how long the non-docker fallback in stopChild waits after +// SIGINT before escalating to SIGKILL. The docker path doesn't use this — it +// removes the container daemon-side, which doesn't depend on signal forwarding. +const stopGracePeriod = 2 * time.Second + // runWithWatch runs the project under fsnotify; on file changes it restarts // the child process after a short debounce. Ignores .git/, .gitignore, .env. // @@ -22,20 +31,29 @@ import ( // and the "Change detected" line still print, but the underlying child // process is never started. This is what `arctl run --watch --dry-run` // surfaces to tests. -func runWithWatch(out io.Writer, projectDir string, p *frameworks.Framework, env []string, dryRun bool) error { +func runWithWatch(ctx context.Context, out io.Writer, projectDir string, p *frameworks.Framework, image string, port int, env []string, dryRun, inspector bool) error { + // Per-watch-session container name. The random suffix keeps two terminals + // running `arctl run --watch` on the same project from stomping each + // other's containers — `docker rm -f` on restart would otherwise kill the + // sibling session's container silently. Within one session, every restart + // reuses this name so `docker rm -f` is well-defined. + containerName := fmt.Sprintf("arctl-watch-%s-%s", filepath.Base(projectDir), randSuffix(6)) + var current *exec.Cmd startCmd := func() error { if current != nil { - _ = current.Process.Kill() - _ = current.Wait() + stopChild(current, containerName) } argv, err := frameworks.RenderArgs(p.Run.Command, map[string]any{ "ProjectDir": projectDir, "FrameworkDir": p.SourceDir, + "Image": image, + "Port": port, }) if err != nil { return err } + argv = injectDockerName(argv, containerName) fmt.Fprintf(out, "→ %s: %s\n", p.Name, strings.Join(argv, " ")) if dryRun { fmt.Fprintln(out, "(dry-run; skipping exec)") @@ -52,6 +70,19 @@ func runWithWatch(out io.Writer, projectDir string, p *frameworks.Framework, env return err } + // Inspector launches ONCE for the whole watch session. The MCP container + // restarts repeatedly behind a stable URL (same port, same path), and + // the Inspector reconnects on its own across restarts — so we don't + // relaunch per cycle. dryRun narrates without launching. + if inspector { + if dryRun { + fmt.Fprintf(out, "→ would launch MCP Inspector against http://localhost:%d/mcp\n", port) + } else { + stop := launchInspector(out, port) + defer stop() + } + } + w, err := fsnotify.NewWatcher() if err != nil { return err @@ -65,8 +96,13 @@ func runWithWatch(out io.Writer, projectDir string, p *frameworks.Framework, env debounce := time.NewTimer(time.Hour) debounce.Stop() - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + + // Editors typically write a file via several inode operations (atomic + // rename, chmod, etc.), and macOS fsevents fans these out further. Print + // "Change detected" only on the first event in a debounce window so a + // single save doesn't spam the log; reset on restart so the next save + // still gets a line. + pending := false for { select { @@ -74,9 +110,13 @@ func runWithWatch(out io.Writer, projectDir string, p *frameworks.Framework, env if shouldIgnore(e.Name) { continue } - fmt.Fprintf(out, "→ Change detected: %s\n", filepath.Base(e.Name)) + if !pending { + fmt.Fprintf(out, "→ Change detected: %s\n", filepath.Base(e.Name)) + pending = true + } debounce.Reset(200 * time.Millisecond) case <-debounce.C: + pending = false if err := startCmd(); err != nil { return err } @@ -84,11 +124,94 @@ func runWithWatch(out io.Writer, projectDir string, p *frameworks.Framework, env case err := <-w.Errors: return err case <-ctx.Done(): + if current != nil { + stopChild(current, containerName) + } return nil } } } +// launchInspector starts the MCP Inspector subprocess pointed at the local +// MCP and returns a cleanup function the caller defers. On launch failure +// (npx missing, etc.) the helper prints a warning and returns a no-op +// cleanup — Inspector is a debug tool and must not gate the dev loop. +func launchInspector(out io.Writer, port int) func() { + inspectorURL := fmt.Sprintf("http://localhost:%d/mcp", port) + insCmd, err := inspectorpkg.Launch(context.Background(), inspectorURL) + if err != nil { + fmt.Fprintf(out, "Warning: --inspector skipped: %v\n", err) + fmt.Fprintf(out, " MCP will still start on %s.\n", inspectorURL) + return func() {} + } + fmt.Fprintf(out, "→ MCP Inspector launching (will connect to %s when ready)\n", inspectorURL) + return func() { + if insCmd.Process != nil { + _ = insCmd.Process.Kill() + _ = insCmd.Wait() + } + } +} + +// injectDockerName inserts `--name ` after `docker run` so we can +// `docker rm -f ` on restart. If argv isn't `docker run …`, returns it +// unchanged — non-docker frameworks fall back to signal-based stop in +// stopChild, which is correct because they don't need the daemon-side +// cleanup. +func injectDockerName(argv []string, name string) []string { + if len(argv) < 2 || argv[0] != "docker" || argv[1] != "run" { + return argv + } + out := make([]string, 0, len(argv)+2) + out = append(out, argv[0], argv[1], "--name", name) + out = append(out, argv[2:]...) + return out +} + +// stopChild stops a running child process. For `docker run` children, signal +// propagation to the daemon-managed container is unreliable (the daemon runs +// its own 10s stop timer on SIGTERM, independent of the CLI process), so we +// rely on the daemon-side `docker rm -f ` to actually kill the +// container; the CLI subprocess exits on its own once the container is gone. +// For non-docker children we keep the SIGINT → wait → SIGKILL escalation as +// a generic graceful stop. +func stopChild(cmd *exec.Cmd, containerName string) { + if cmd.Process == nil { + return + } + isDocker := len(cmd.Args) > 1 && cmd.Args[0] == "docker" && cmd.Args[1] == "run" + if isDocker && containerName != "" { + // Blocking on `docker rm -f` is intentional: it returns only after + // the container is gone AND the userland port mapping is released, + // so the next `docker run` won't race the daemon's teardown. + rm := exec.Command("docker", "rm", "-f", containerName) + _ = rm.Run() + _ = cmd.Wait() + return + } + _ = cmd.Process.Signal(syscall.SIGINT) + done := make(chan struct{}) + go func() { + _ = cmd.Wait() + close(done) + }() + select { + case <-done: + case <-time.After(stopGracePeriod): + _ = cmd.Process.Kill() + <-done + } +} + +// randSuffix returns a lowercase hex string of n characters. Used to keep +// concurrent `arctl run --watch` sessions on the same project from sharing +// a container name. +func randSuffix(n int) string { + b := make([]byte, (n+1)/2) + _, _ = rand.Read(b) + return hex.EncodeToString(b)[:n] +} + // addWatches recursively adds every directory and file under root to the // watcher, skipping ignored paths (.git, .gitignore, .env). func addWatches(w *fsnotify.Watcher, root string) error { diff --git a/internal/cli/declarative/watch_test.go b/internal/cli/declarative/watch_test.go index 5ecb01ba..e8480c9d 100644 --- a/internal/cli/declarative/watch_test.go +++ b/internal/cli/declarative/watch_test.go @@ -1,9 +1,14 @@ package declarative import ( + "bytes" + "context" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/agentregistry-dev/agentregistry/internal/cli/frameworks" ) func TestShouldIgnore_GitDir(t *testing.T) { @@ -11,3 +16,91 @@ func TestShouldIgnore_GitDir(t *testing.T) { assert.True(t, shouldIgnore("/proj/.gitignore")) assert.False(t, shouldIgnore("/proj/agent.py")) } + +// runWithWatch must render {{.Image}} and {{.Port}} for MCP frameworks whose +// run command references them directly (e.g. fastmcp-python: `docker run -p +// {{.Port}}:{{.Port}} {{.Image}}`). Regression for a bug where the watch +// path's vars map omitted Image/Port and the template rendered "". +func TestRunWithWatch_RendersImageAndPort(t *testing.T) { + p := &frameworks.Framework{ + Name: "fastmcp-python", + Type: "mcp", + SourceDir: t.TempDir(), + Run: frameworks.Command{ + Command: []string{"docker", "run", "--rm", "-p", "{{.Port}}:{{.Port}}", "{{.Image}}"}, + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + var buf bytes.Buffer + err := runWithWatch(ctx, &buf, t.TempDir(), p, "localhost:5001/test:latest", 3000, nil, true /*dryRun*/, false /*inspector*/) + require.NoError(t, err) + + out := buf.String() + assert.NotContains(t, out, "", "watch path must populate Image and Port template vars") + assert.Contains(t, out, "3000:3000") + assert.Contains(t, out, "localhost:5001/test:latest") + assert.Contains(t, out, "--name arctl-watch-", "docker run argv must be named so we can docker rm -f on restart") +} + +// --watch + --inspector should compose: today the watch path returns before +// the inspector code in runProject, so without explicit plumbing the +// --inspector flag was silently dropped. This locks in the narrative path +// — actual subprocess launch happens only outside dryRun. +func TestRunWithWatch_InspectorComposes(t *testing.T) { + p := &frameworks.Framework{ + Name: "fastmcp-python", + Type: "mcp", + SourceDir: t.TempDir(), + Run: frameworks.Command{ + Command: []string{"docker", "run", "--rm", "-p", "{{.Port}}:{{.Port}}", "{{.Image}}"}, + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + var buf bytes.Buffer + err := runWithWatch(ctx, &buf, t.TempDir(), p, "img:latest", 3000, nil, true /*dryRun*/, true /*inspector*/) + require.NoError(t, err) + + out := buf.String() + assert.Contains(t, out, "MCP Inspector", "inspector narration must appear when --watch --inspector is set") + assert.Contains(t, out, "http://localhost:3000/mcp") +} + +func TestInjectDockerName(t *testing.T) { + tests := []struct { + name string + in []string + want []string + }{ + { + name: "docker run gets name injected after 'run'", + in: []string{"docker", "run", "--rm", "-p", "3000:3000", "img:latest"}, + want: []string{"docker", "run", "--name", "arctl-watch-x", "--rm", "-p", "3000:3000", "img:latest"}, + }, + { + name: "docker compose unchanged (not docker run)", + in: []string{"docker", "compose", "up", "--build"}, + want: []string{"docker", "compose", "up", "--build"}, + }, + { + name: "non-docker unchanged", + in: []string{"podman", "run", "--rm", "img:latest"}, + want: []string{"podman", "run", "--rm", "img:latest"}, + }, + { + name: "too short unchanged", + in: []string{"docker"}, + want: []string{"docker"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, injectDockerName(tc.in, "arctl-watch-x")) + }) + } +}