From 3b11aad0efc189c825c65a985eae8abd06b7c626 Mon Sep 17 00:00:00 2001 From: MacAttak Date: Tue, 10 Mar 2026 17:11:34 +1100 Subject: [PATCH 1/4] feat(codegen): wire CLI generator entrypoint execution with cliArgs construction Replace the echo stub in the Go CLI template with real exec.CommandContext calls to the manifest's entrypoint path. Build a cliArgs slice following the runner convention: positional args first, flags in definition order, auth token last. Task 1 of codegen-entrypoint-wiring. Co-Authored-By: Claude Opus 4.6 --- internal/codegen/cli_go.go | 190 +++++--- internal/codegen/cli_go_test.go | 761 ++++++++++++++++++++++++++++++++ 2 files changed, 882 insertions(+), 69 deletions(-) diff --git a/internal/codegen/cli_go.go b/internal/codegen/cli_go.go index c4091eb..aa12802 100644 --- a/internal/codegen/cli_go.go +++ b/internal/codegen/cli_go.go @@ -207,7 +207,8 @@ type flagData struct { type argData struct { Name string // original arg name used in string literals - GoName string // sanitized Go identifier + GoName string // sanitized Go identifier (camelCase, lowercase first) + GoNameCap string // GoName with first letter uppercased (PascalCase) GoType string Required bool Description string @@ -224,10 +225,11 @@ type toolGoData struct { AuthType string TokenEnv string TokenFlag string - HasNonStringArgs bool // true if any arg needs strconv parsing - HasNonStringArrayFlags bool // true if any flag is a non-string array type needing strconv - HasObjectFlags bool // true if any flag is type "object" or "object[]" - IsBinaryOutput bool // true if tool output format is "binary" + HasNonStringArgs bool // true if any arg needs strconv parsing + HasNonStringArrayFlags bool // true if any flag is a non-string array type needing strconv + HasObjectFlags bool // true if any flag is type "object" or "object[]" + IsBinaryOutput bool // true if tool output format is "binary" + Entrypoint string // executable path from manifest tool.Entrypoint } type goModData struct { @@ -317,9 +319,15 @@ func toolSummaries(m manifest.Toolkit) []toolSummary { func buildToolData(m manifest.Toolkit, tool manifest.Tool, auth manifest.Auth) toolGoData { args := make([]argData, len(tool.Args)) for i, a := range tool.Args { + gn := goIdentifier(a.Name) + gnCap := gn + if len(gn) > 0 { + gnCap = strings.ToUpper(gn[:1]) + gn[1:] + } args[i] = argData{ Name: a.Name, - GoName: goIdentifier(a.Name), + GoName: gn, + GoNameCap: gnCap, GoType: goType(a.Type), Required: a.Required, Description: a.Description, @@ -413,6 +421,7 @@ func buildToolData(m manifest.Toolkit, tool manifest.Tool, auth manifest.Auth) t HasNonStringArrayFlags: hasNonStringArrayFlags, HasObjectFlags: hasObjectFlags, IsBinaryOutput: tool.Output.Format == "binary", + Entrypoint: tool.Entrypoint, } } @@ -694,6 +703,7 @@ import ( {{- $authType := .AuthType}} {{- $tokenEnv := .TokenEnv}} {{- $tokenFlag := .TokenFlag}} +{{- $entrypoint := .Entrypoint}} var ( {{- range .Flags}} {{- if .IsArray}} @@ -740,114 +750,157 @@ var {{.GoName}}Cmd = &cobra.Command{ RunE: func(cmd *cobra.Command, args []string) error { {{- range $i, $a := .Args}} {{- if eq $a.GoType "string"}} - arg{{$a.GoName}} := args[{{$i}}] // {{$a.GoType}} + arg{{$a.GoNameCap}} := args[{{$i}}] // {{$a.GoType}} {{- else if eq $a.GoType "int"}} - arg{{$a.GoName}}, err := strconv.Atoi(args[{{$i}}]) // {{$a.GoType}} + arg{{$a.GoNameCap}}, err := strconv.Atoi(args[{{$i}}]) // {{$a.GoType}} if err != nil { return fmt.Errorf("invalid value %q for argument {{$a.Name}}: %w", args[{{$i}}], err) } {{- else if eq $a.GoType "float64"}} - arg{{$a.GoName}}, err := strconv.ParseFloat(args[{{$i}}], 64) // {{$a.GoType}} + arg{{$a.GoNameCap}}, err := strconv.ParseFloat(args[{{$i}}], 64) // {{$a.GoType}} if err != nil { return fmt.Errorf("invalid value %q for argument {{$a.Name}}: %w", args[{{$i}}], err) } {{- else if eq $a.GoType "bool"}} - arg{{$a.GoName}}, err := strconv.ParseBool(args[{{$i}}]) // {{$a.GoType}} + arg{{$a.GoNameCap}}, err := strconv.ParseBool(args[{{$i}}]) // {{$a.GoType}} if err != nil { return fmt.Errorf("invalid value %q for argument {{$a.Name}}: %w", args[{{$i}}], err) } {{- end}} - _ = arg{{$a.GoName}} +{{- end}} +{{- if $hasAuth}} + // Resolve auth token: prefer flag, fall back to env var. + token := {{$goName}}Token + if token == "" { + token = os.Getenv("{{$tokenEnv | esc}}") + } + if token == "" { + return fmt.Errorf("auth required: set {{$tokenEnv | esc}} or pass --{{$tokenFlag | esc}}") + } +{{- end}} + if "{{$entrypoint | esc}}" == "" { + return fmt.Errorf("{{$toolName}}: entrypoint not configured") + } + var cliArgs []string +{{- range $i, $a := .Args}} +{{- if eq $a.GoType "string"}} + cliArgs = append(cliArgs, arg{{$a.GoNameCap}}) +{{- else if eq $a.GoType "int"}} + cliArgs = append(cliArgs, fmt.Sprintf("%d", arg{{$a.GoNameCap}})) +{{- else if eq $a.GoType "float64"}} + cliArgs = append(cliArgs, fmt.Sprintf("%g", arg{{$a.GoNameCap}})) +{{- else if eq $a.GoType "bool"}} + cliArgs = append(cliArgs, strconv.FormatBool(arg{{$a.GoNameCap}})) +{{- end}} {{- end}} {{- range .Flags}} {{- if .IsArray}} -{{- if eq .ArrayBase "int"}} - parsed{{.GoName}} := make([]int, len({{$goName}}Flag{{.GoName}})) - for i, s := range {{$goName}}Flag{{.GoName}} { - v, err := strconv.Atoi(s) - if err != nil { - return fmt.Errorf("invalid value %q for element of --{{.Name}}: not a valid int", s) +{{- if eq .ArrayBase "string"}} + for _, v := range {{$goName}}Flag{{.GoName}} { + cliArgs = append(cliArgs, "--{{.Name}}", v) + } +{{- else if eq .ArrayBase "int"}} + { + parsed{{.GoName}} := make([]int, len({{$goName}}Flag{{.GoName}})) + for i, s := range {{$goName}}Flag{{.GoName}} { + v, err := strconv.Atoi(s) + if err != nil { + return fmt.Errorf("invalid value %q for element of --{{.Name}}: not a valid int", s) + } + parsed{{.GoName}}[i] = v + } + for _, v := range parsed{{.GoName}} { + cliArgs = append(cliArgs, "--{{.Name}}", strconv.Itoa(v)) } - parsed{{.GoName}}[i] = v } - _ = parsed{{.GoName}} {{- else if eq .ArrayBase "float"}} - parsed{{.GoName}} := make([]float64, len({{$goName}}Flag{{.GoName}})) - for i, s := range {{$goName}}Flag{{.GoName}} { - v, err := strconv.ParseFloat(s, 64) - if err != nil { - return fmt.Errorf("invalid value %q for element of --{{.Name}}: not a valid float", s) + { + parsed{{.GoName}} := make([]float64, len({{$goName}}Flag{{.GoName}})) + for i, s := range {{$goName}}Flag{{.GoName}} { + v, err := strconv.ParseFloat(s, 64) + if err != nil { + return fmt.Errorf("invalid value %q for element of --{{.Name}}: not a valid float", s) + } + parsed{{.GoName}}[i] = v + } + for _, v := range parsed{{.GoName}} { + cliArgs = append(cliArgs, "--{{.Name}}", fmt.Sprintf("%g", v)) } - parsed{{.GoName}}[i] = v } - _ = parsed{{.GoName}} {{- else if eq .ArrayBase "bool"}} - parsed{{.GoName}} := make([]bool, len({{$goName}}Flag{{.GoName}})) - for i, s := range {{$goName}}Flag{{.GoName}} { - v, err := strconv.ParseBool(s) - if err != nil { - return fmt.Errorf("invalid value %q for element of --{{.Name}}: not a valid bool", s) + { + parsed{{.GoName}} := make([]bool, len({{$goName}}Flag{{.GoName}})) + for i, s := range {{$goName}}Flag{{.GoName}} { + v, err := strconv.ParseBool(s) + if err != nil { + return fmt.Errorf("invalid value %q for element of --{{.Name}}: not a valid bool", s) + } + parsed{{.GoName}}[i] = v + } + for _, v := range parsed{{.GoName}} { + cliArgs = append(cliArgs, "--{{.Name}}", strconv.FormatBool(v)) } - parsed{{.GoName}}[i] = v } - _ = parsed{{.GoName}} -{{- end}} {{- end}} -{{- end}} -{{- range .Flags}} -{{- if .IsObject}} +{{- else if .IsObject}} + if {{$goName}}Flag{{.GoName}} != "" { {{- if .IsObjectArray}} - var parsed{{.GoName}} []map[string]any - if err := json.Unmarshal([]byte({{$goName}}Flag{{.GoName}}), &parsed{{.GoName}}); err != nil { - return fmt.Errorf("invalid JSON for --{{.Name}}: %w", err) - } + var parsed{{.GoName}} []map[string]any + if err := json.Unmarshal([]byte({{$goName}}Flag{{.GoName}}), &parsed{{.GoName}}); err != nil { + return fmt.Errorf("invalid JSON for --{{.Name}}: %w", err) + } {{- if .HasItemSchema}} - for _idx, _elem := range parsed{{.GoName}} { - for _, _field := range []string{ {{joinQuoted .ItemSchemaProperties}} } { - if _, ok := _elem[_field]; !ok { - return fmt.Errorf("--{{.Name}}[%d]: required field %q missing from JSON object", _idx, _field) + for _idx, _elem := range parsed{{.GoName}} { + for _, _field := range []string{ {{joinQuoted .ItemSchemaProperties}} } { + if _, ok := _elem[_field]; !ok { + return fmt.Errorf("--{{.Name}}[%d]: required field %q missing from JSON object", _idx, _field) + } } } - } {{- end}} {{- else}} - var parsed{{.GoName}} map[string]any - if err := json.Unmarshal([]byte({{$goName}}Flag{{.GoName}}), &parsed{{.GoName}}); err != nil { - return fmt.Errorf("invalid JSON for --{{.Name}}: %w", err) - } + var parsed{{.GoName}} map[string]any + if err := json.Unmarshal([]byte({{$goName}}Flag{{.GoName}}), &parsed{{.GoName}}); err != nil { + return fmt.Errorf("invalid JSON for --{{.Name}}: %w", err) + } {{- if .HasItemSchema}} - for _, _field := range []string{ {{joinQuoted .ItemSchemaProperties}} } { - if _, ok := parsed{{.GoName}}[_field]; !ok { - return fmt.Errorf("--{{.Name}}: required field %q missing from JSON object", _field) + for _, _field := range []string{ {{joinQuoted .ItemSchemaProperties}} } { + if _, ok := parsed{{.GoName}}[_field]; !ok { + return fmt.Errorf("--{{.Name}}: required field %q missing from JSON object", _field) + } } - } {{- end}} {{- end}} - _ = parsed{{.GoName}} + cliArgs = append(cliArgs, "--{{.Name}}", {{$goName}}Flag{{.GoName}}) + } +{{- else if eq .GoType "bool"}} + if {{$goName}}Flag{{.GoName}} { + cliArgs = append(cliArgs, "--{{.Name}}") + } +{{- else if eq .GoType "string"}} + if {{$goName}}Flag{{.GoName}} != "" { + cliArgs = append(cliArgs, "--{{.Name}}", {{$goName}}Flag{{.GoName}}) + } +{{- else if eq .GoType "int"}} + cliArgs = append(cliArgs, "--{{.Name}}", fmt.Sprintf("%d", {{$goName}}Flag{{.GoName}})) +{{- else if eq .GoType "float64"}} + cliArgs = append(cliArgs, "--{{.Name}}", fmt.Sprintf("%g", {{$goName}}Flag{{.GoName}})) {{- end}} {{- end}} {{- if $hasAuth}} - // Resolve auth token: prefer flag, fall back to env var. - token := {{$goName}}Token - if token == "" { - token = os.Getenv("{{$tokenEnv | esc}}") - } - if token == "" { - return fmt.Errorf("auth required: set {{$tokenEnv | esc}} or pass --{{$tokenFlag | esc}}") - } - _ = token // passed to the entrypoint via environment + cliArgs = append(cliArgs, "--{{$tokenFlag | esc}}", token) {{- end}} + ctx := cmd.Context() {{- if .IsBinaryOutput}} - // Binary output: detect TTY and handle appropriately. + // Binary output handling fi, statErr := os.Stdout.Stat() isTTY := statErr == nil && (fi.Mode()&os.ModeCharDevice) != 0 if isTTY && {{$goName}}FlagOutput == "" { return fmt.Errorf("binary output requires --output or pipe") } - c := exec.CommandContext(cmd.Context(), "echo", "running", "{{$toolName}}") + c := exec.CommandContext(ctx, "{{$entrypoint | esc}}", cliArgs...) c.Stderr = os.Stderr if {{$goName}}FlagOutput != "" { - // --output provided: capture stdout and write to file. out, err := c.Output() if err != nil { return fmt.Errorf("{{$toolName}} failed: %w", err) @@ -856,7 +909,6 @@ var {{.GoName}}Cmd = &cobra.Command{ return fmt.Errorf("writing output file: %w", err) } } else { - // No --output: stream directly to stdout (pipe mode). c.Stdout = os.Stdout if err := c.Run(); err != nil { return fmt.Errorf("{{$toolName}} failed: %w", err) @@ -864,7 +916,7 @@ var {{.GoName}}Cmd = &cobra.Command{ } {{- else}} // Execute the tool entrypoint. - c := exec.CommandContext(cmd.Context(), "echo", "running", "{{$toolName}}") + c := exec.CommandContext(ctx, "{{$entrypoint | esc}}", cliArgs...) c.Stdout = os.Stdout c.Stderr = os.Stderr if err := c.Run(); err != nil { diff --git a/internal/codegen/cli_go_test.go b/internal/codegen/cli_go_test.go index ab5f10d..1ab0708 100644 --- a/internal/codegen/cli_go_test.go +++ b/internal/codegen/cli_go_test.go @@ -2251,3 +2251,764 @@ func TestGoCLI_Object_AC8_HasNonStringArrayFlags_NotSetForObjectArray(t *testing assert.False(t, data.HasNonStringArrayFlags, "object[] flag must not set HasNonStringArrayFlags (no strconv parsing)") } + +// --------------------------------------------------------------------------- +// Task 1 — AC1: CLI generator emits entrypoint execution +// --------------------------------------------------------------------------- + +// manifestEntrypointWiring returns a manifest exercising entrypoint + full +// arg/flag coverage for Task 1 tests. The tool has positional args, multiple +// flag types, and token auth — the kitchen sink for cliArgs construction. +func manifestEntrypointWiring() manifest.Toolkit { + return manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "wire-toolkit", + Version: "1.0.0", + Description: "Entrypoint wiring toolkit", + }, + Tools: []manifest.Tool{ + { + Name: "run-job", + Description: "Run a job on the cluster", + Entrypoint: "/usr/local/bin/run-job", + Auth: &manifest.Auth{ + Type: "token", + TokenEnv: "JOB_TOKEN", + TokenFlag: "--api-key", + }, + Args: []manifest.Arg{ + {Name: "job-name", Type: "string", Required: true, Description: "Name of the job"}, + {Name: "priority", Type: "int", Required: false, Description: "Job priority"}, + }, + Flags: []manifest.Flag{ + {Name: "verbose", Type: "bool", Required: false, Default: false, Description: "Verbose output"}, + {Name: "retries", Type: "int", Required: false, Default: 3, Description: "Number of retries"}, + {Name: "threshold", Type: "float", Required: false, Default: 0.5, Description: "Score threshold"}, + {Name: "label", Type: "string", Required: false, Description: "Job label"}, + {Name: "tags", Type: "string[]", Required: false, Description: "Tags to apply"}, + {Name: "counts", Type: "int[]", Required: false, Description: "Count list"}, + {Name: "config", Type: "object", Required: false, Description: "Config object"}, + }, + }, + }, + } +} + +func TestGoCLI_AC1_EntrypointUsed_NotEcho(t *testing.T) { + // The generated code must use the tool's manifest entrypoint as the + // executable in exec.CommandContext, NOT "echo". + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // Must contain the actual entrypoint path. + assert.Contains(t, content, "/usr/local/bin/run-job", + "generated code must use the tool's entrypoint path, not a stub") + // Must NOT use "echo" as the executable. + assert.NotContains(t, content, `"echo"`, + "generated code must not use \"echo\" as the executable — use the real entrypoint") +} + +func TestGoCLI_AC1_EntrypointInExecCommandContext(t *testing.T) { + // Verify the entrypoint appears in the specific exec.CommandContext call, + // not just anywhere in the file. This catches a sloppy implementation + // that puts the entrypoint in a comment but still calls echo. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + assert.Regexp(t, `exec\.CommandContext\([^)]*"/usr/local/bin/run-job"`, content, + "entrypoint must be the executable argument to exec.CommandContext") +} + +func TestGoCLI_AC1_EntrypointFromManifest_TableDriven(t *testing.T) { + // Table-driven (Constitution 9): verify different entrypoint values. + tests := []struct { + name string + entrypoint string + want string + }{ + { + name: "absolute path", + entrypoint: "/opt/tools/deploy", + want: "/opt/tools/deploy", + }, + { + name: "relative path", + entrypoint: "./scripts/run.sh", + want: "./scripts/run.sh", + }, + { + name: "bare command", + entrypoint: "python3", + want: "python3", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "ep-test", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "mytool", + Entrypoint: tc.entrypoint, + Auth: &manifest.Auth{Type: "none"}, + }, + }, + } + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/mytool.go") + assert.Contains(t, content, tc.want, + "entrypoint %q must appear in generated code", tc.entrypoint) + assert.NotContains(t, content, `"echo"`, + "generated code must not use echo stub with entrypoint %q", tc.entrypoint) + }) + } +} + +func TestGoCLI_AC1_BuildToolData_EntrypointPopulated(t *testing.T) { + // The toolGoData struct must include an Entrypoint field populated from + // tool.Entrypoint. We verify via the generated output: if the entrypoint + // field is missing from the struct, the template cannot interpolate it + // and the generated code will not contain the entrypoint path. + m := manifestEntrypointWiring() + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/run-job.go") + + // The entrypoint must appear in exec.CommandContext — this proves the + // struct carried it through to the template. + assert.Regexp(t, `exec\.CommandContext\([^)]*"/usr/local/bin/run-job"`, content, + "buildToolData must populate Entrypoint so the template can interpolate it into exec.CommandContext") +} + +func TestGoCLI_AC1_EmptyEntrypoint_ProducesGuard(t *testing.T) { + // A manifest with entrypoint: "" must generate a guard that returns + // an error at runtime rather than trying to exec "". + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "noep-toolkit", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "unconfigured", + Description: "Tool with empty entrypoint", + Entrypoint: "", + Auth: &manifest.Auth{Type: "none"}, + }, + }, + } + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/unconfigured.go") + + // The generated RunE must contain a guard that checks for empty entrypoint. + assert.Contains(t, content, "entrypoint not configured", + "empty entrypoint must produce a guard error containing 'entrypoint not configured'") + assert.Contains(t, content, "unconfigured", + "guard error message must include the tool name 'unconfigured'") +} + +func TestGoCLI_AC1_EmptyEntrypoint_GuardBlocksExecution(t *testing.T) { + // The guard must appear before exec.CommandContext so the tool never + // attempts to exec an empty string. Verify by checking that the guard + // string "entrypoint not configured" appears before "CommandContext" + // (or that CommandContext is absent for empty entrypoint tools). + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "noep2-toolkit", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "empty-ep", + Entrypoint: "", + Auth: &manifest.Auth{Type: "none"}, + }, + }, + } + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/empty-ep.go") + + guardIdx := strings.Index(content, "entrypoint not configured") + assert.NotEqual(t, -1, guardIdx, + "generated code must contain 'entrypoint not configured' guard") + + // Either CommandContext is absent entirely (valid: the guard returns before + // reaching it) or it appears after the guard. + cmdIdx := strings.Index(content, "CommandContext") + if cmdIdx != -1 { + assert.Greater(t, cmdIdx, guardIdx, + "entrypoint guard must appear before CommandContext call") + } +} + +func TestGoCLI_AC1_EntrypointEscaped_Quotes(t *testing.T) { + // AC6.4: Entrypoint paths with quotes must be safely escaped so the + // generated Go string literal is valid. + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "esc-toolkit", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "quotetool", + Entrypoint: `/path/with "quotes"/run`, + Auth: &manifest.Auth{Type: "none"}, + }, + }, + } + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/quotetool.go") + + // The raw double-quote must be escaped as \" in the Go string literal. + assert.Contains(t, content, `\"quotes\"`, + "double quotes in entrypoint must be escaped as \\\" in generated Go literal") + // The raw path with unescaped quotes must NOT appear verbatim. If the + // implementation forgot to escape, the template would produce something + // like: CommandContext(..., "/path/with "quotes"/run") which is broken Go. + // We check that the full raw entrypoint path does not appear unescaped + // by looking for the sequence that would only exist without escaping. + assert.NotContains(t, content, `with "quotes"`, + "raw unescaped entrypoint path with bare quotes must not appear in generated code") +} + +func TestGoCLI_AC1_EntrypointEscaped_Backslash(t *testing.T) { + // AC6.4: Entrypoint paths with backslashes (e.g., Windows paths) must + // be safely escaped. + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "bs-toolkit", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "bstool", + Entrypoint: `C:\Program Files\tool\run.exe`, + Auth: &manifest.Auth{Type: "none"}, + }, + }, + } + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/bstool.go") + + // Backslashes must be doubled in the Go string literal. + assert.Contains(t, content, `C:\\Program Files\\tool\\run.exe`, + "backslashes in entrypoint must be escaped as \\\\ in generated Go literal") +} + +// --------------------------------------------------------------------------- +// Task 1 — AC2: CLI generator builds args in runner convention order +// --------------------------------------------------------------------------- + +func TestGoCLI_AC2_CliArgsSliceExists(t *testing.T) { + // The generated code must build a cliArgs (or equivalent) slice + // containing the tool's arguments and flags for the entrypoint. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + assert.Contains(t, content, "cliArgs", + "generated code must build a cliArgs slice for entrypoint arguments") +} + +func TestGoCLI_AC2_PositionalArgsFirst(t *testing.T) { + // Positional args must appear before any flags in cliArgs. + // The generated code appends positional args first. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // The positional arg names (job-name, priority) must appear in the args + // building section before any flag names (--verbose, --retries, etc.) + // We look for the first occurrence of a positional arg variable and the + // first flag append to verify ordering. + jobNameIdx := strings.Index(content, "argJobName") + assert.NotEqual(t, -1, jobNameIdx, + "generated code must reference positional arg variable 'argJobName'") + + // The first flag in cliArgs must come after positional args. + flagAppendIdx := strings.Index(content, `"--verbose"`) + if flagAppendIdx != -1 { + assert.Greater(t, flagAppendIdx, jobNameIdx, + "flags in cliArgs must appear after positional args") + } +} + +func TestGoCLI_AC2_FlagsInDefinitionOrder(t *testing.T) { + // Flags must appear in the order defined in the manifest. + // The manifest defines: verbose, retries, threshold, label, tags, counts, config. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // All flag names must appear in the cliArgs building section. + flagNames := []string{"--verbose", "--retries", "--threshold", "--label", "--tags", "--counts", "--config"} + lastIdx := -1 + for _, flag := range flagNames { + idx := strings.Index(content, flag) + assert.NotEqual(t, -1, idx, + "generated code must reference flag %q in cliArgs construction", flag) + if lastIdx != -1 && idx != -1 { + assert.Greater(t, idx, lastIdx, + "flag %q must appear after previous flag in definition order", flag) + } + if idx != -1 { + lastIdx = idx + } + } +} + +func TestGoCLI_AC2_BoolFlag_OnlyWhenTrue(t *testing.T) { + // Bool flags emit only --flag when true; omitted when false. + // The generated code must have a conditional: if boolVar { append --flag }. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // The code must conditionally append the bool flag, not unconditionally. + // We verify that there's a conditional check around the verbose flag. + assert.Regexp(t, `if\s+.*[Vv]erbose`, content, + "bool flag 'verbose' must be conditionally appended (only when true)") + // Bool flags must NOT emit a value (--verbose true), just --verbose. + // Look for a pattern that appends --verbose without a following value. + assert.NotRegexp(t, `"--verbose".*"true"`, content, + "bool flag must emit only --verbose, not --verbose true") +} + +func TestGoCLI_AC2_StringFlag_OmittedWhenEmpty(t *testing.T) { + // Empty/zero-value string flags are omitted from cliArgs. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // There must be a conditional check for the label flag (string type). + assert.Regexp(t, `if\s+.*[Ll]abel\s*!=\s*""`, content, + "string flag 'label' must be conditionally appended (omitted when empty)") +} + +func TestGoCLI_AC2_IntFlag_Stringified(t *testing.T) { + // Int flags via fmt.Sprintf("%d", val). Zero-value int still passed. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // The retries int flag must be stringified using Sprintf("%d", ...). + assert.Regexp(t, `Sprintf\("%d".*[Rr]etries`, content, + "int flag 'retries' must be stringified via fmt.Sprintf(\"%%d\", ...)") +} + +func TestGoCLI_AC2_FloatFlag_Stringified(t *testing.T) { + // Float flags via fmt.Sprintf("%g", val). + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // The threshold float flag must be stringified using Sprintf("%g", ...). + assert.Regexp(t, `Sprintf\("%g".*[Tt]hreshold`, content, + "float flag 'threshold' must be stringified via fmt.Sprintf(\"%%g\", ...)") +} + +func TestGoCLI_AC2_ArrayFlag_RepeatedPairs(t *testing.T) { + // Array flags: each element emits a separate --flag element pair. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // The tags (string[]) flag must iterate and emit --tags for each element. + // Look for a range loop that appends --tags per element. + assert.Regexp(t, `range\s+.*[Tt]ags`, content, + "string[] flag 'tags' must iterate elements to emit repeated --tags pairs") + assert.Contains(t, content, `"--tags"`, + "string[] flag must emit '--tags' flag name for each element") +} + +func TestGoCLI_AC2_IntArrayFlag_RepeatedStringifiedPairs(t *testing.T) { + // int[] array flags: each element stringified and emitted as --flag value. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // The counts (int[]) flag must iterate and emit --counts per element. + assert.Contains(t, content, `"--counts"`, + "int[] flag must emit '--counts' flag name for each element") +} + +func TestGoCLI_AC2_ObjectFlag_JsonString(t *testing.T) { + // Object flags emit --flag json_string. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // The config (object) flag must be passed as --config . + assert.Contains(t, content, `"--config"`, + "object flag must emit '--config' in cliArgs") +} + +func TestGoCLI_AC2_TokenAppendedLast(t *testing.T) { + // Auth token last as --{tokenFlag} {token}. TokenFlag stored WITHOUT --. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // The token must be appended to cliArgs after all other args/flags. + // The token flag name (api-key) must appear in the cliArgs section. + assert.Contains(t, content, `"--api-key"`, + "token flag must appear as '--api-key' in cliArgs") + + // Verify token append comes after other flags. Find --config (last + // non-token flag) and --api-key (token flag). + configIdx := strings.Index(content, `"--config"`) + tokenIdx := strings.Index(content, `"--api-key"`) + if configIdx != -1 && tokenIdx != -1 { + assert.Greater(t, tokenIdx, configIdx, + "token flag '--api-key' must appear after last regular flag '--config' in cliArgs") + } +} + +func TestGoCLI_AC2_NoAuthTool_NoTokenInArgs(t *testing.T) { + // No token arg when HasAuth == false. + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "noauth-wiring", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "simple-run", + Entrypoint: "./simple.sh", + Auth: &manifest.Auth{Type: "none"}, + Args: []manifest.Arg{ + {Name: "input", Type: "string", Required: true, Description: "Input file"}, + }, + Flags: []manifest.Flag{ + {Name: "verbose", Type: "bool", Required: false, Description: "Verbose output"}, + }, + }, + }, + } + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/simple-run.go") + + // Must use entrypoint, not echo. + assert.Contains(t, content, "./simple.sh", + "no-auth tool must still use its entrypoint") + assert.NotContains(t, content, `"echo"`, + "no-auth tool must not use echo stub") + // Must NOT have any token-related cliArgs. + assert.NotContains(t, content, "--token", + "no-auth tool must not append --token to cliArgs") + assert.NotContains(t, content, "--api-key", + "no-auth tool must not append --api-key to cliArgs") +} + +func TestGoCLI_AC2_IntFlag_ZeroValueStillPassed(t *testing.T) { + // Zero-value int/float still passed (unlike string which is omitted). + // This means no conditional check for int == 0 or float == 0. + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "zero-val-toolkit", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "zero-tool", + Entrypoint: "./zero.sh", + Auth: &manifest.Auth{Type: "none"}, + Flags: []manifest.Flag{ + {Name: "count", Type: "int", Required: false, Default: 0, Description: "A count"}, + {Name: "weight", Type: "float", Required: false, Default: 0.0, Description: "A weight"}, + {Name: "name", Type: "string", Required: false, Description: "A name"}, + }, + }, + }, + } + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/zero-tool.go") + + // Int/float flags must NOT have an "if != 0" guard; they are always passed. + // String flags DO have an "if != ''" guard. + assert.NotRegexp(t, `if\s+.*[Cc]ount\s*!=\s*0`, content, + "int flag 'count' must NOT be conditionally omitted at zero value") + assert.NotRegexp(t, `if\s+.*[Ww]eight\s*!=\s*0`, content, + "float flag 'weight' must NOT be conditionally omitted at zero value") + // But string flag MUST have a guard. + assert.Regexp(t, `if\s+.*[Nn]ame\s*!=\s*""`, content, + "string flag 'name' must be conditionally omitted when empty") +} + +func TestGoCLI_AC2_CliArgsPassedToExecCommand(t *testing.T) { + // The cliArgs slice must actually be passed to exec.CommandContext, + // not just built and ignored. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // exec.CommandContext takes (ctx, name, arg...) — the cliArgs must be + // spread into it. + assert.Regexp(t, `CommandContext\([^,]+,\s*"[^"]*"[^)]*cliArgs`, content, + "cliArgs must be passed to exec.CommandContext (e.g., cliArgs...)") +} + +func TestGoCLI_AC2_MultiplePositionalArgs_InOrder(t *testing.T) { + // When a tool has multiple positional args, they must appear in + // definition order (job-name before priority). + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // Both arg variables must be appended to cliArgs. + jobNameIdx := strings.Index(content, "argJobName") + priorityIdx := strings.Index(content, "argPriority") + assert.NotEqual(t, -1, jobNameIdx, + "positional arg 'argJobName' must be referenced in cliArgs construction") + assert.NotEqual(t, -1, priorityIdx, + "positional arg 'argPriority' must be referenced in cliArgs construction") + // job-name before priority. + assert.Less(t, jobNameIdx, priorityIdx, + "positional args must appear in definition order: job-name before priority") +} + +func TestGoCLI_AC2_EntrypointPerTool_NotShared(t *testing.T) { + // When a manifest has multiple tools with different entrypoints, each + // tool command file must use its own entrypoint. + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "multi-ep-toolkit", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "alpha", + Entrypoint: "/bin/alpha-runner", + Auth: &manifest.Auth{Type: "none"}, + }, + { + Name: "bravo", + Entrypoint: "/bin/bravo-runner", + Auth: &manifest.Auth{Type: "none"}, + }, + }, + } + files := generateCLI(t, m) + + alphaContent := fileContent(t, files, "internal/commands/alpha.go") + bravoContent := fileContent(t, files, "internal/commands/bravo.go") + + // alpha.go must use alpha-runner and NOT bravo-runner. + assert.Contains(t, alphaContent, "/bin/alpha-runner", + "alpha.go must use alpha's entrypoint") + assert.NotContains(t, alphaContent, "/bin/bravo-runner", + "alpha.go must not use bravo's entrypoint") + + // bravo.go must use bravo-runner and NOT alpha-runner. + assert.Contains(t, bravoContent, "/bin/bravo-runner", + "bravo.go must use bravo's entrypoint") + assert.NotContains(t, bravoContent, "/bin/alpha-runner", + "bravo.go must not use alpha's entrypoint") + + // Neither should use echo. + assert.NotContains(t, alphaContent, `"echo"`, + "alpha.go must not use echo stub") + assert.NotContains(t, bravoContent, `"echo"`, + "bravo.go must not use echo stub") +} + +// --------------------------------------------------------------------------- +// Task 1 — AC6.1: No token values in string literals +// --------------------------------------------------------------------------- + +func TestGoCLI_AC6_1_NoTokenLiteralInExecArgs(t *testing.T) { + // Generated CLI code must NOT embed token values in string literals. + // The token must be passed via a variable, not hardcoded. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // The cliArgs construction must reference the token variable, not a literal. + // Looking for the pattern: token variable used as the arg value. + assert.Regexp(t, `"--api-key"[^"]*token`, content, + "token must be passed via variable (e.g., token), not as a string literal") +} + +// --------------------------------------------------------------------------- +// Task 1 — Cross-cutting: existing manifests still work with entrypoint +// --------------------------------------------------------------------------- + +func TestGoCLI_AC1_ExistingManifests_UseEntrypoint(t *testing.T) { + // All existing test manifests with non-empty entrypoints must now + // generate exec.CommandContext with that entrypoint, not "echo". + tests := []struct { + name string + manifest manifest.Toolkit + toolFile string + wantEP string + }{ + { + name: "manifestTwoToolsMixed/status", + manifest: manifestTwoToolsMixed(), + toolFile: "internal/commands/status.go", + wantEP: "./status.sh", + }, + { + name: "manifestTwoToolsMixed/deploy", + manifest: manifestTwoToolsMixed(), + toolFile: "internal/commands/deploy.go", + wantEP: "./deploy.sh", + }, + { + name: "manifestAllAuthNone/ping", + manifest: manifestAllAuthNone(), + toolFile: "internal/commands/ping.go", + wantEP: "./ping.sh", + }, + { + name: "manifestWithArgsAndFlags/upload", + manifest: manifestWithArgsAndFlags(), + toolFile: "internal/commands/upload.go", + wantEP: "./upload.sh", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + files := generateCLI(t, tc.manifest) + content := fileContent(t, files, tc.toolFile) + assert.Contains(t, content, tc.wantEP, + "generated code must contain entrypoint %q", tc.wantEP) + assert.NotContains(t, content, `"echo"`, + "generated code must not use echo stub") + }) + } +} + +func TestGoCLI_AC1_BinaryOutputTool_UsesEntrypoint(t *testing.T) { + // Binary output tools also have the echo stub — they must use entrypoint too. + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "bin-toolkit", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "image-gen", + Description: "Generate an image", + Entrypoint: "./image-gen.sh", + Auth: &manifest.Auth{Type: "none"}, + Output: manifest.Output{Format: "binary"}, + }, + }, + } + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/image-gen.go") + + // The binary output path also has exec.CommandContext — it must use the + // real entrypoint. + assert.Contains(t, content, "./image-gen.sh", + "binary output tool must use entrypoint, not echo") + assert.NotContains(t, content, `"echo"`, + "binary output tool must not use echo stub") +} + +func TestGoCLI_AC2_ObjectArrayFlag_JsonInCliArgs(t *testing.T) { + // object[] flags pass --flag json_string (same as object). + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "objarray-cli", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "batch", + Entrypoint: "./batch.sh", + Auth: &manifest.Auth{Type: "none"}, + Flags: []manifest.Flag{ + {Name: "items", Type: "object[]", Description: "Batch items"}, + }, + }, + }, + } + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/batch.go") + + // The object[] flag must appear in cliArgs as --items . + assert.Contains(t, content, `"--items"`, + "object[] flag must emit '--items' in cliArgs") + assert.NotContains(t, content, `"echo"`, + "tool must not use echo stub") +} + +func TestGoCLI_AC2_TokenFlagPrefix_NoDoubleDash(t *testing.T) { + // TokenFlag stored WITHOUT -- prefix. The cliArgs construction must add + // the -- prefix when building the arg. This tests that the TrimPrefix + // at buildToolData (line ~391) works correctly with cliArgs. + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "prefix-test", + Version: "1.0.0", + }, + Tools: []manifest.Tool{ + { + Name: "authtool", + Entrypoint: "./auth.sh", + Auth: &manifest.Auth{ + Type: "token", + TokenEnv: "AUTH_TOKEN", + TokenFlag: "--secret-key", + }, + }, + }, + } + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/authtool.go") + + // TokenFlag is stored as "secret-key" (TrimPrefix removes --). + // The cliArgs must emit "--secret-key", not "----secret-key". + assert.Contains(t, content, `"--secret-key"`, + "cliArgs must emit --secret-key (single -- prefix)") + assert.NotContains(t, content, `"----secret-key"`, + "cliArgs must not double the -- prefix on TokenFlag") +} + +func TestGoCLI_AC2_ArgsAndFlags_CompleteIntegration(t *testing.T) { + // Integration-level test: a tool with args, various flag types, and auth + // must produce generated code containing all the expected cliArgs pieces. + files := generateCLI(t, manifestEntrypointWiring()) + content := fileContent(t, files, "internal/commands/run-job.go") + + // Entrypoint, not echo. + assert.Contains(t, content, "/usr/local/bin/run-job", + "must use real entrypoint") + assert.NotContains(t, content, `"echo"`, + "must not use echo") + + // cliArgs must exist. + assert.Contains(t, content, "cliArgs", + "must build cliArgs slice") + + // Positional args referenced. + assert.Contains(t, content, "argJobName", + "must reference positional arg job-name") + + // All flags referenced. + for _, flag := range []string{"--verbose", "--retries", "--threshold", "--label", "--tags", "--counts", "--config"} { + assert.Contains(t, content, flag, + "must reference flag %q in cliArgs", flag) + } + + // Token last. + assert.Contains(t, content, `"--api-key"`, + "must include token flag in cliArgs") +} From 313b0d0f7b5dbd1ef7b23b8f9a99c28d50dfe2ba Mon Sep 17 00:00:00 2001 From: MacAttak Date: Tue, 10 Mar 2026 17:27:02 +1100 Subject: [PATCH 2/4] feat(codegen): wire MCP TypeScript generator entrypoint execution with args construction Replace the TODO stub in the MCP tool template with real execFile calls to the manifest's entrypoint path. Build an args array following the runner convention: positional args first, flags in definition order, auth token last via CLI flag (constitution rule 24). Task 2 of codegen-entrypoint-wiring. Co-Authored-By: Claude Opus 4.6 --- internal/codegen/mcp_typescript.go | 70 +- internal/codegen/mcp_typescript_test.go | 841 ++++++++++++++++++++++++ 2 files changed, 896 insertions(+), 15 deletions(-) diff --git a/internal/codegen/mcp_typescript.go b/internal/codegen/mcp_typescript.go index 936b0b4..ebd2bc1 100644 --- a/internal/codegen/mcp_typescript.go +++ b/internal/codegen/mcp_typescript.go @@ -199,11 +199,12 @@ type tsArgData struct { } type tsFlagData struct { - Name string - TSType string - ZodType string - Required bool - Description string + Name string + TSType string + ZodType string + Required bool + Description string + ManifestType string } type tsAnnotations struct { @@ -221,6 +222,8 @@ type tsToolData struct { HasAuth bool AuthType string TokenEnv string + TokenFlag string + Entrypoint string HasAnnotations bool Annotations tsAnnotations Title string @@ -478,11 +481,12 @@ func buildTSToolData(tool manifest.Tool, auth manifest.Auth) (tsToolData, error) zodExpr = zodType(f.Type) } flags[i] = tsFlagData{ - Name: f.Name, - TSType: tsType(f.Type), - ZodType: zodExpr, - Required: f.Required, - Description: f.Description, + Name: f.Name, + TSType: tsType(f.Type), + ZodType: zodExpr, + Required: f.Required, + Description: f.Description, + ManifestType: f.Type, } } hasAuth := auth.Type == "token" || auth.Type == "oauth2" @@ -535,6 +539,8 @@ func buildTSToolData(tool manifest.Tool, auth manifest.Auth) (tsToolData, error) HasAuth: hasAuth, AuthType: auth.Type, TokenEnv: auth.TokenEnv, + TokenFlag: auth.TokenFlag, + Entrypoint: tool.Entrypoint, HasAnnotations: hasAnnotations, Annotations: annot, Title: title, @@ -675,10 +681,14 @@ export { server }; const tsToolTmpl = `import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; +import { execFile as execFileCb } from "node:child_process"; +import { promisify } from "node:util"; {{- if .HasAuth}} import { validateRequest } from "../auth/middleware.js"; {{- end}} +const execFile = promisify(execFileCb); + // Tool: {{.ToolName}} // Description: {{.Description}} {{- if .SchemaPath}} @@ -713,15 +723,44 @@ async function handle_{{.ToolName}}(input: {{.ToolName}}Input): Promise<{ conten throw new Error("auth required: set the {{.TokenEnv | esc}} environment variable"); } {{- end}} + // Empty entrypoint guard + if (!"{{.Entrypoint | esc}}") { + throw new Error("{{.ToolName | esc}}: entrypoint not configured"); + } + const args: string[] = []; + // Positional args first {{- range .Args}} - const {{.Name}}: {{.TSType}} = input.{{.Name}}; + args.push(String(input.{{.Name}})); {{- end}} + // Flags in definition order {{- range .Flags}} - const {{.Name}}: {{.TSType}} | undefined = input.{{.Name}}; +{{- if eq .ManifestType "bool"}} + if (input.{{.Name}} === true) { + args.push("--{{.Name}}"); + } +{{- else if or (eq .ManifestType "object") (eq .ManifestType "object[]")}} + if (input.{{.Name}} !== undefined) { + args.push("--{{.Name}}", JSON.stringify(input.{{.Name}})); + } +{{- else if or (eq .ManifestType "string[]") (eq .ManifestType "int[]") (eq .ManifestType "float[]") (eq .ManifestType "bool[]")}} + if (input.{{.Name}} !== undefined) { + for (const v of input.{{.Name}}) { + args.push("--{{.Name}}", String(v)); + } + } +{{- else}} + if (input.{{.Name}} !== undefined) { + args.push("--{{.Name}}", String(input.{{.Name}})); + } +{{- end}} +{{- end}} +{{- if .HasAuth}} + // Auth token last (via CLI flag, constitution rule 24) + args.push("{{.TokenFlag | esc}}", envToken); {{- end}} - // TODO: implement {{.ToolName}} logic + // Execute the entrypoint {{- if .IsBinaryOutput}} - const stdout = ""; // TODO: replace with actual binary output + const { stdout } = await execFile("{{.Entrypoint | esc}}", args, { encoding: "buffer" }); const base64Data = Buffer.from(stdout).toString("base64"); {{- if .IsImageMime}} return { @@ -743,8 +782,9 @@ async function handle_{{.ToolName}}(input: {{.ToolName}}Input): Promise<{ conten }; {{- end}} {{- else}} + const { stdout } = await execFile("{{.Entrypoint | esc}}", args); return { - content: [{ type: "text", text: "{{.ToolName | esc}} executed" }], + content: [{ type: "text", text: stdout }], }; {{- end}} } diff --git a/internal/codegen/mcp_typescript_test.go b/internal/codegen/mcp_typescript_test.go index 82da76a..be05fbb 100644 --- a/internal/codegen/mcp_typescript_test.go +++ b/internal/codegen/mcp_typescript_test.go @@ -2140,3 +2140,844 @@ func TestTSMCP_Object_CrossCutting_RequiredObjectRecord_NoOptional(t *testing.T) } } } + +// --------------------------------------------------------------------------- +// AC3/AC4/AC6: Entrypoint wiring tests +// --------------------------------------------------------------------------- + +// mcpManifestEntrypointWiring returns a kitchen-sink manifest for entrypoint +// wiring tests: positional args, typed flags, array flag, object flag, +// token auth with TokenFlag. +func mcpManifestEntrypointWiring() manifest.Toolkit { + return manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "wire-mcp-server", + Version: "1.0.0", + Description: "Entrypoint wiring MCP server", + }, + Tools: []manifest.Tool{ + { + Name: "run_job", + Description: "Run a job on the cluster", + Entrypoint: "/usr/local/bin/run-job", + Auth: &manifest.Auth{ + Type: "token", + TokenEnv: "JOB_TOKEN", + TokenFlag: "--api-key", + }, + Args: []manifest.Arg{ + {Name: "job_name", Type: "string", Required: true, Description: "Name of the job"}, + {Name: "priority", Type: "int", Required: false, Description: "Job priority"}, + }, + Flags: []manifest.Flag{ + {Name: "verbose", Type: "bool", Required: false, Default: false, Description: "Verbose output"}, + {Name: "retries", Type: "int", Required: false, Default: 3, Description: "Number of retries"}, + {Name: "threshold", Type: "float", Required: false, Default: 0.5, Description: "Score threshold"}, + {Name: "label", Type: "string", Required: false, Description: "Job label"}, + {Name: "tags", Type: "string[]", Required: false, Description: "Tags to apply"}, + {Name: "config", Type: "object", Required: false, Description: "Config object"}, + }, + }, + }, + Generate: manifest.Generate{ + MCP: manifest.MCPConfig{ + Target: "typescript", + Transport: []string{"stdio"}, + }, + }, + } +} + +// mcpManifestEntrypointBinaryOutput returns a manifest with a binary-output +// tool and token auth, for testing execFile with encoding: "buffer". +func mcpManifestEntrypointBinaryOutput() manifest.Toolkit { + return manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "binary-wire-server", + Version: "1.0.0", + Description: "Binary output wiring test", + }, + Tools: []manifest.Tool{ + { + Name: "render_image", + Description: "Render an image", + Entrypoint: "/opt/render", + Output: manifest.Output{Format: "binary", MimeType: "image/png"}, + Auth: &manifest.Auth{ + Type: "token", + TokenEnv: "RENDER_TOKEN", + TokenFlag: "--auth-token", + }, + }, + }, + Generate: manifest.Generate{ + MCP: manifest.MCPConfig{ + Target: "typescript", + Transport: []string{"stdio"}, + }, + }, + } +} + +// mcpManifestEntrypointNoAuth returns a manifest with a tool that has no auth, +// used to verify no token-related code is emitted for no-auth tools. +func mcpManifestEntrypointNoAuth() manifest.Toolkit { + return manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "noauth-wire-server", + Version: "1.0.0", + Description: "No-auth wiring test", + }, + Tools: []manifest.Tool{ + { + Name: "list_files", + Description: "List directory files", + Entrypoint: "/usr/bin/ls", + Auth: &manifest.Auth{Type: "none"}, + Flags: []manifest.Flag{ + {Name: "all", Type: "bool", Required: false, Description: "Show hidden files"}, + }, + }, + }, + Generate: manifest.Generate{ + MCP: manifest.MCPConfig{ + Target: "typescript", + Transport: []string{"stdio"}, + }, + }, + } +} + +// mcpManifestEmptyEntrypoint returns a manifest where the tool has an empty +// entrypoint, used to verify the guard is emitted. +func mcpManifestEmptyEntrypoint() manifest.Toolkit { + return manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "empty-ep-server", + Version: "1.0.0", + Description: "Empty entrypoint test", + }, + Tools: []manifest.Tool{ + { + Name: "stub_tool", + Description: "Tool with empty entrypoint", + Entrypoint: "", + Auth: &manifest.Auth{Type: "none"}, + }, + }, + Generate: manifest.Generate{ + MCP: manifest.MCPConfig{ + Target: "typescript", + Transport: []string{"stdio"}, + }, + }, + } +} + +// mcpManifestMultipleToolsEntrypoint returns a manifest with two tools that +// have different entrypoints, used to verify each tool file uses its own. +func mcpManifestMultipleToolsEntrypoint() manifest.Toolkit { + return manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "multi-ep-server", + Version: "1.0.0", + Description: "Multiple entrypoints", + }, + Tools: []manifest.Tool{ + { + Name: "tool_alpha", + Description: "Alpha tool", + Entrypoint: "/bin/alpha", + Auth: &manifest.Auth{Type: "none"}, + }, + { + Name: "tool_beta", + Description: "Beta tool", + Entrypoint: "/bin/beta", + Auth: &manifest.Auth{Type: "none"}, + }, + }, + Generate: manifest.Generate{ + MCP: manifest.MCPConfig{ + Target: "typescript", + Transport: []string{"stdio"}, + }, + }, + } +} + +// --------------------------------------------------------------------------- +// AC3: MCP generator emits entrypoint execution +// --------------------------------------------------------------------------- + +func TestTSMCP_AC3_ExecFileImported(t *testing.T) { + // All generated tool files must import execFile from node:child_process. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.Contains(t, content, `from "node:child_process"`, + "tool file must import from node:child_process") + assert.Contains(t, content, "execFile", + "tool file must import execFile") +} + +func TestTSMCP_AC3_PromisifyImported(t *testing.T) { + // Generated tool files must import promisify from node:util. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.Contains(t, content, `from "node:util"`, + "tool file must import from node:util") + assert.Contains(t, content, "promisify", + "tool file must import promisify") +} + +func TestTSMCP_AC3_ExecFilePromisified(t *testing.T) { + // Generated tool files must promisify execFile for async/await usage. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + // Must contain the promisify wrapper, e.g.: + // const execFile = promisify(execFileCb); + assert.Regexp(t, `const execFile\s*=\s*promisify\(`, content, + "tool file must promisify execFile") +} + +func TestTSMCP_AC3_EntrypointUsed_NotTODO(t *testing.T) { + // The generated tool file must NOT contain a TODO comment as a placeholder + // for tool logic. It must actually call execFile. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.NotContains(t, content, "// TODO: implement", + "tool file must not contain TODO placeholder for tool logic") + assert.Contains(t, content, "await execFile(", + "tool file must call execFile with await") +} + +func TestTSMCP_AC3_EntrypointInExecFile(t *testing.T) { + // The execFile call must use the tool's manifest entrypoint path. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.Contains(t, content, `"/usr/local/bin/run-job"`, + "execFile must reference the tool's entrypoint path from the manifest") +} + +func TestTSMCP_AC3_EntrypointFromManifest_TableDriven(t *testing.T) { + // Different entrypoint values must appear in the generated execFile call. + tests := []struct { + name string + entrypoint string + wantInCode string + }{ + { + name: "absolute path", + entrypoint: "/usr/bin/mytool", + wantInCode: `"/usr/bin/mytool"`, + }, + { + name: "relative path", + entrypoint: "./tools/run.sh", + wantInCode: `"./tools/run.sh"`, + }, + { + name: "bare command", + entrypoint: "python3", + wantInCode: `"python3"`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "ep-test", Version: "1.0.0", Description: "test", + }, + Tools: []manifest.Tool{ + { + Name: "test_tool", + Description: "Test tool", + Entrypoint: tc.entrypoint, + Auth: &manifest.Auth{Type: "none"}, + }, + }, + Generate: manifest.Generate{ + MCP: manifest.MCPConfig{Target: "typescript", Transport: []string{"stdio"}}, + }, + } + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/test_tool.ts") + + assert.Contains(t, content, tc.wantInCode, + "execFile must use entrypoint %q from the manifest", tc.entrypoint) + assert.Contains(t, content, "await execFile(", + "tool file must call execFile") + }) + } +} + +func TestTSMCP_AC3_EmptyEntrypoint_ProducesGuard(t *testing.T) { + // A tool with entrypoint: "" must produce a runtime guard that throws + // an error mentioning "entrypoint not configured". + m := mcpManifestEmptyEntrypoint() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/stub_tool.ts") + + assert.Contains(t, content, "entrypoint not configured", + "empty entrypoint must produce a guard error string") + assert.Contains(t, content, "stub_tool", + "guard error must mention the tool name") +} + +func TestTSMCP_AC3_EmptyEntrypoint_GuardBlocksExecution(t *testing.T) { + // The guard for empty entrypoint must appear BEFORE any execFile call, + // so execution is blocked before trying to spawn a child process. + m := mcpManifestEmptyEntrypoint() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/stub_tool.ts") + + guardIdx := strings.Index(content, "entrypoint not configured") + require.NotEqual(t, -1, guardIdx, + "must contain entrypoint guard") + + execIdx := strings.Index(content, "await execFile(") + if execIdx != -1 { + assert.Less(t, guardIdx, execIdx, + "entrypoint guard must appear before execFile call") + } + // If no execFile is present at all, the guard still must be there + // (the guard itself blocks execution, which is valid) +} + +func TestTSMCP_AC3_EntrypointEscaped_Quotes(t *testing.T) { + // Entrypoint values with double quotes must be escaped in the generated + // TypeScript string literal (constitution 25a). + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "esc-test", Version: "1.0.0", Description: "test", + }, + Tools: []manifest.Tool{ + { + Name: "esc_tool", + Description: "Escape test tool", + Entrypoint: `/opt/my "tool"`, + Auth: &manifest.Auth{Type: "none"}, + }, + }, + Generate: manifest.Generate{ + MCP: manifest.MCPConfig{Target: "typescript", Transport: []string{"stdio"}}, + }, + } + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/esc_tool.ts") + + // The raw double quotes must be escaped + assert.NotContains(t, content, `/opt/my "tool"`, + "raw double quotes must not appear in generated string literal") + assert.Contains(t, content, `/opt/my \"tool\"`, + "entrypoint with quotes must be escaped via esc function") +} + +func TestTSMCP_AC3_EntrypointEscaped_Backslash(t *testing.T) { + // Entrypoint values with backslashes must be escaped (constitution 25a). + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "bs-test", Version: "1.0.0", Description: "test", + }, + Tools: []manifest.Tool{ + { + Name: "bs_tool", + Description: "Backslash test tool", + Entrypoint: `C:\Program Files\tool.exe`, + Auth: &manifest.Auth{Type: "none"}, + }, + }, + Generate: manifest.Generate{ + MCP: manifest.MCPConfig{Target: "typescript", Transport: []string{"stdio"}}, + }, + } + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/bs_tool.ts") + + // Raw backslash must be escaped to double-backslash in TS string literal + assert.Contains(t, content, `C:\\Program Files\\tool.exe`, + "entrypoint backslashes must be escaped") +} + +func TestTSMCP_AC3_TextOutput_StdoutAsText(t *testing.T) { + // Text output tools must return { type: "text", text: stdout } from + // the execFile result, NOT a hardcoded placeholder string. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + // Must NOT contain hardcoded placeholder text return + assert.NotContains(t, content, `text: "run_job executed"`, + "text tools must not return hardcoded placeholder text") + + // Must contain stdout from execFile used in the return + assert.Regexp(t, `type:\s*"text"`, content, + "text tool must return content with type 'text'") + // The stdout variable from execFile must be in the return + assert.Contains(t, content, "stdout", + "text tool must reference stdout from execFile result") +} + +func TestTSMCP_AC3_BinaryOutput_BufferEncoding(t *testing.T) { + // Binary output tools must call execFile with { encoding: "buffer" } + // to receive raw bytes instead of a string. + m := mcpManifestEntrypointBinaryOutput() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/render_image.ts") + + assert.Contains(t, content, `encoding: "buffer"`, + "binary tool must call execFile with encoding: buffer option") + assert.NotContains(t, content, `const stdout = ""`, + "binary tool must not use empty string placeholder for stdout") +} + +func TestTSMCP_AC3_BinaryOutput_Base64(t *testing.T) { + // Binary output tools must base64 encode the stdout Buffer from execFile. + m := mcpManifestEntrypointBinaryOutput() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/render_image.ts") + + // Must call execFile to get actual stdout, not use a hardcoded empty string + assert.Contains(t, content, "await execFile(", + "binary tool must call execFile to get stdout") + assert.NotContains(t, content, `const stdout = ""`, + "binary tool must not use empty string placeholder for stdout") + + // The stdout from execFile (as Buffer) must be base64 encoded + assert.Contains(t, content, `.toString("base64")`, + "binary tool must base64 encode stdout buffer") + // Must still contain the image return structure + assert.Contains(t, content, "base64Data", + "binary tool must use base64Data in return") +} + +func TestTSMCP_AC3_MultipleTools_EachUsesOwnEntrypoint(t *testing.T) { + // When multiple tools exist, each generated file must reference its own + // entrypoint, not share one or use the wrong one. + m := mcpManifestMultipleToolsEntrypoint() + files := generateTSMCP(t, m) + + alphaContent := fileContent(t, files, "src/tools/tool_alpha.ts") + betaContent := fileContent(t, files, "src/tools/tool_beta.ts") + + // Alpha must use /bin/alpha + assert.Contains(t, alphaContent, `"/bin/alpha"`, + "tool_alpha must use its own entrypoint /bin/alpha") + assert.NotContains(t, alphaContent, `"/bin/beta"`, + "tool_alpha must NOT reference tool_beta's entrypoint") + + // Beta must use /bin/beta + assert.Contains(t, betaContent, `"/bin/beta"`, + "tool_beta must use its own entrypoint /bin/beta") + assert.NotContains(t, betaContent, `"/bin/alpha"`, + "tool_beta must NOT reference tool_alpha's entrypoint") +} + +// --------------------------------------------------------------------------- +// AC4: MCP generator builds args in runner convention order +// --------------------------------------------------------------------------- + +func TestTSMCP_AC4_ArgsArrayExists(t *testing.T) { + // The generated handler must declare a typed args array. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.Regexp(t, `const args:\s*string\[\]\s*=\s*\[\]`, content, + "handler must declare 'const args: string[] = []'") +} + +func TestTSMCP_AC4_PositionalArgsFirst(t *testing.T) { + // Positional args must appear before any flags in the args array. + // The generated code must push positional args first, then flags. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + // Positional arg push must come before any --flag push + jobNameIdx := strings.Index(content, "job_name") + require.NotEqual(t, -1, jobNameIdx, + "must reference positional arg job_name") + + // Find the first --flag reference (e.g., --verbose) + firstFlagIdx := strings.Index(content, `"--verbose"`) + if firstFlagIdx == -1 { + firstFlagIdx = strings.Index(content, `"--retries"`) + } + require.NotEqual(t, -1, firstFlagIdx, + "must reference at least one flag with -- prefix") + + // Find the push of the positional arg — it should have String() wrapping + positionalPushIdx := strings.Index(content, "String(input.job_name)") + require.NotEqual(t, -1, positionalPushIdx, + "must push positional arg with String() conversion") + + assert.Less(t, positionalPushIdx, firstFlagIdx, + "positional args must be pushed before any flags") +} + +func TestTSMCP_AC4_PositionalArgs_Stringified(t *testing.T) { + // Positional args must be pushed as String(value) to ensure correct type. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + // String arg: String(input.job_name) + assert.Contains(t, content, "String(input.job_name)", + "string positional arg must use String(input.job_name)") + + // Int arg (priority): String(input.priority) — numbers must be stringified + assert.Contains(t, content, "String(input.priority)", + "int positional arg must use String(input.priority)") +} + +func TestTSMCP_AC4_StringFlag_NotUndefined(t *testing.T) { + // String flags must be guarded with !== undefined before pushing. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.Regexp(t, `input\.label\s*!==\s*undefined`, content, + "string flag 'label' must be guarded with !== undefined") + assert.Contains(t, content, `"--label"`, + "string flag must push --label flag name") + assert.Contains(t, content, "String(input.label)", + "string flag must push String(input.label) as value") +} + +func TestTSMCP_AC4_BoolFlag_OnlyWhenTrue(t *testing.T) { + // Boolean flags push only the flag name (no value) when true. + // Must use === true guard, not just truthiness check. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.Regexp(t, `input\.verbose\s*===\s*true`, content, + "bool flag 'verbose' must use === true guard") + assert.Contains(t, content, `"--verbose"`, + "bool flag must push --verbose flag name") + + // Boolean flags must NOT push a value after the flag name + // Find the verbose block and ensure it doesn't push a second arg + verboseIdx := strings.Index(content, `"--verbose"`) + require.NotEqual(t, -1, verboseIdx) + // Get a reasonable window after the flag push + window := content[verboseIdx:min(verboseIdx+100, len(content))] + assert.NotContains(t, window, "String(input.verbose)", + "bool flag must NOT push a value — only the flag name") +} + +func TestTSMCP_AC4_NumberFlag_Stringified(t *testing.T) { + // Number flags must be pushed with String() conversion. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.Contains(t, content, `"--retries"`, + "number flag must push --retries flag name") + assert.Contains(t, content, "String(input.retries)", + "number flag must push String(input.retries) as value") + + // Float flag too + assert.Contains(t, content, `"--threshold"`, + "float flag must push --threshold flag name") + assert.Contains(t, content, "String(input.threshold)", + "float flag must push String(input.threshold) as value") +} + +func TestTSMCP_AC4_ArrayFlag_RepeatedPairs(t *testing.T) { + // Array flags emit a separate --flag value pair for each element. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + // Must contain a for loop over the array elements + assert.Regexp(t, `for\s*\(`, content, + "array flag must use a for loop to iterate elements") + assert.Contains(t, content, "input.tags", + "array flag must reference input.tags") + assert.Contains(t, content, `"--tags"`, + "array flag must push --tags for each element") +} + +func TestTSMCP_AC4_ObjectFlag_JsonStringify(t *testing.T) { + // Object flags must be serialized with JSON.stringify. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.Contains(t, content, `"--config"`, + "object flag must push --config flag name") + assert.Contains(t, content, "JSON.stringify(input.config)", + "object flag must use JSON.stringify for value") +} + +func TestTSMCP_AC4_TokenViaCliFlag(t *testing.T) { + // Auth token must be passed to the entrypoint child process via CLI flag, + // using the TokenFlag value from the manifest (constitution rule 24). + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.Contains(t, content, `"--api-key"`, + "token must be passed using the manifest TokenFlag value") + assert.Contains(t, content, "envToken", + "token must be passed via the envToken variable, not hardcoded") + // Must push as args.push("--api-key", envToken) + assert.Regexp(t, `args\.push\([^)]*"--api-key"[^)]*envToken`, content, + "token must be pushed as args.push(\"--api-key\", envToken)") +} + +func TestTSMCP_AC4_TokenLastInArgs(t *testing.T) { + // Token flag must appear AFTER all other args and flags in the args array. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + // Find the last flag push (--config is the last manifest flag) + configIdx := strings.Index(content, `"--config"`) + require.NotEqual(t, -1, configIdx, + "must contain --config flag") + + // Find the token flag push + tokenIdx := strings.Index(content, `"--api-key"`) + require.NotEqual(t, -1, tokenIdx, + "must contain --api-key token flag") + + assert.Greater(t, tokenIdx, configIdx, + "token flag --api-key must appear after all manifest flags") +} + +func TestTSMCP_AC4_NoAuthTool_NoTokenInArgs(t *testing.T) { + // A tool with no auth (or auth:none) must not have any token-related + // args building code, but MUST still call execFile with its entrypoint. + m := mcpManifestEntrypointNoAuth() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/list_files.ts") + + // Must still call execFile (this is entrypoint wiring, not just auth) + assert.Contains(t, content, "await execFile(", + "no-auth tool must still call execFile for its entrypoint") + assert.Contains(t, content, `"/usr/bin/ls"`, + "no-auth tool must use its manifest entrypoint in execFile call") + + // Must NOT have token-related code + assert.NotContains(t, content, "envToken", + "no-auth tool must not reference envToken") + assert.NotContains(t, content, "JOB_TOKEN", + "no-auth tool must not reference any token env var") + assert.NotContains(t, content, "--api-key", + "no-auth tool must not push any token flag") +} + +func TestTSMCP_AC4_FlagsInDefinitionOrder(t *testing.T) { + // Flags must appear in the generated args-building code in the same order + // as they are defined in the manifest. This ensures predictable argument + // ordering for child processes. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + // The manifest defines flags in this order: + // verbose, retries, threshold, label, tags, config + verboseIdx := strings.Index(content, `"--verbose"`) + retriesIdx := strings.Index(content, `"--retries"`) + thresholdIdx := strings.Index(content, `"--threshold"`) + labelIdx := strings.Index(content, `"--label"`) + tagsIdx := strings.Index(content, `"--tags"`) + configIdx := strings.Index(content, `"--config"`) + + require.NotEqual(t, -1, verboseIdx, "must contain --verbose") + require.NotEqual(t, -1, retriesIdx, "must contain --retries") + require.NotEqual(t, -1, thresholdIdx, "must contain --threshold") + require.NotEqual(t, -1, labelIdx, "must contain --label") + require.NotEqual(t, -1, tagsIdx, "must contain --tags") + require.NotEqual(t, -1, configIdx, "must contain --config") + + assert.Less(t, verboseIdx, retriesIdx, + "--verbose must appear before --retries") + assert.Less(t, retriesIdx, thresholdIdx, + "--retries must appear before --threshold") + assert.Less(t, thresholdIdx, labelIdx, + "--threshold must appear before --label") + assert.Less(t, labelIdx, tagsIdx, + "--label must appear before --tags") + assert.Less(t, tagsIdx, configIdx, + "--tags must appear before --config") +} + +// --------------------------------------------------------------------------- +// AC6: Security — no token leakage +// --------------------------------------------------------------------------- + +func TestTSMCP_AC6_TokenReadFromEnv(t *testing.T) { + // The MCP server must read the token from the environment using + // process.env["TOKEN_ENV_NAME"] and then pass it to the child process + // via CLI flag args (constitution rule 24). + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + assert.Contains(t, content, `process.env["JOB_TOKEN"]`, + "token must be read from process.env with the correct env var name") + // The envToken variable must be used in args.push for the child process + assert.Regexp(t, `args\.push\([^)]*"--api-key"[^)]*envToken`, content, + "envToken from process.env must be forwarded via args.push to child process") +} + +func TestTSMCP_AC6_TokenNotInChildEnv(t *testing.T) { + // Tokens must be passed via CLI flag to child processes, NEVER as + // environment variables on the child. The execFile call must not set + // env on the options object with token values (constitution rule 24). + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + // Must have execFile to be meaningful + assert.Contains(t, content, "await execFile(", + "must call execFile to verify token is not in child env") + + // execFile options must not include env with token + assert.NotRegexp(t, `execFile\([^)]*\{[^}]*env:`, content, + "execFile must not pass env option with token to child process") +} + +func TestTSMCP_AC6_NoTokenLiteralInExecArgs(t *testing.T) { + // The token must be passed via a variable (envToken), never as a string + // literal in the execFile arguments. This ensures tokens aren't hardcoded. + m := mcpManifestEntrypointWiring() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/run_job.ts") + + // Must have execFile to be meaningful + require.Contains(t, content, "await execFile(", + "must call execFile to verify token is not literal in args") + + // The args array must reference envToken variable, not a literal + assert.Regexp(t, `args\.push\([^)]*"--api-key"[^)]*envToken`, content, + "token must be passed as envToken variable in args.push") + + // The env var name should not appear as a string literal pushed to args + execCallIdx := strings.Index(content, "await execFile(") + afterExec := content[execCallIdx:] + assert.NotContains(t, afterExec, `args.push("JOB_TOKEN"`, + "token env var name must not appear as a literal in args push") +} + +// --------------------------------------------------------------------------- +// AC3/AC4 cross-cutting: entrypoint and token flag flow through to output +// --------------------------------------------------------------------------- + +func TestTSMCP_AC3_EntrypointFlowsToOutput(t *testing.T) { + // The tool's manifest entrypoint must flow through to the generated + // TypeScript output. This is an end-to-end check that the data pipeline + // (manifest -> tsToolData -> template) preserves the entrypoint value. + // Each test case uses a distinct entrypoint to prevent a hardcoded return. + tests := []struct { + name string + entrypoint string + }{ + {name: "absolute_path", entrypoint: "/usr/local/bin/run-job"}, + {name: "relative_dotslash", entrypoint: "./my-tool.sh"}, + {name: "bare_command", entrypoint: "node"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "ep-flow-test", Version: "1.0.0", Description: "test", + }, + Tools: []manifest.Tool{ + { + Name: "flow_tool", + Description: "Flow test", + Entrypoint: tc.entrypoint, + Auth: &manifest.Auth{Type: "none"}, + }, + }, + Generate: manifest.Generate{ + MCP: manifest.MCPConfig{Target: "typescript", Transport: []string{"stdio"}}, + }, + } + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/flow_tool.ts") + + // The entrypoint must appear as a string literal in the execFile call + assert.Contains(t, content, tc.entrypoint, + "entrypoint %q must flow through to generated output", tc.entrypoint) + }) + } +} + +func TestTSMCP_AC3_TokenFlagFlowsToOutput(t *testing.T) { + // The auth TokenFlag (with -- prefix) must flow through to the generated + // TypeScript output, appearing in the args.push() call for the token. + tests := []struct { + name string + tokenFlag string + }{ + {name: "api-key", tokenFlag: "--api-key"}, + {name: "token", tokenFlag: "--token"}, + {name: "auth-header", tokenFlag: "--auth-header"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + m := manifest.Toolkit{ + APIVersion: "toolwright/v1", + Kind: "Toolkit", + Metadata: manifest.Metadata{ + Name: "tf-flow-test", Version: "1.0.0", Description: "test", + }, + Tools: []manifest.Tool{ + { + Name: "tf_tool", + Description: "Token flag flow test", + Entrypoint: "/bin/tool", + Auth: &manifest.Auth{ + Type: "token", + TokenEnv: "MY_TOKEN", + TokenFlag: tc.tokenFlag, + }, + }, + }, + Generate: manifest.Generate{ + MCP: manifest.MCPConfig{Target: "typescript", Transport: []string{"stdio"}}, + }, + } + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/tf_tool.ts") + + assert.Contains(t, content, tc.tokenFlag, + "tokenFlag %q must flow through to generated output", tc.tokenFlag) + }) + } +} From 12771ab0114f07f2b4f6202d87ee25ffc54a6351 Mon Sep 17 00:00:00 2001 From: MacAttak Date: Tue, 10 Mar 2026 17:31:53 +1100 Subject: [PATCH 3/4] test(codegen): add integration tests for entrypoint wiring across generators Verify generated CLI and MCP projects use real entrypoints instead of echo/TODO stubs. Cover per-tool entrypoint isolation, cliArgs/args construction, token passing via CLI flags, no-auth tool isolation, and TypeScript brace balance. Task 3 of codegen-entrypoint-wiring. Co-Authored-By: Claude Opus 4.6 --- internal/codegen/integration_test.go | 331 +++++++++++++++++++++++++++ 1 file changed, 331 insertions(+) diff --git a/internal/codegen/integration_test.go b/internal/codegen/integration_test.go index 9bc43fb..0947739 100644 --- a/internal/codegen/integration_test.go +++ b/internal/codegen/integration_test.go @@ -1012,3 +1012,334 @@ func TestIntegration_AllGeneratedFilesNonEmpty(t *testing.T) { "TS MCP file %q must not be empty", f.Path) } } + +// --------------------------------------------------------------------------- +// AC5.3: CLI generated code uses entrypoints, not echo stubs +// +// Every tool file must invoke exec.CommandContext with the tool's manifest +// entrypoint, not a placeholder like "echo". This catches regressions where +// the template falls back to stub behavior instead of wiring the real +// entrypoint. +// --------------------------------------------------------------------------- + +func TestIntegration_GoCLI_EntrypointNotEcho(t *testing.T) { + m := integrationManifest() + files := generateCLI(t, m) + + toolFiles := map[string]string{ + "internal/commands/check-health.go": "./health.sh", + "internal/commands/deploy-app.go": "./deploy.sh", + "internal/commands/fetch-secrets.go": "./secrets.sh", + } + + for path, entrypoint := range toolFiles { + content := fileContent(t, files, path) + + // Must contain the exact entrypoint string from the manifest + assert.Contains(t, content, entrypoint, + "%s must contain its manifest entrypoint %q", path, entrypoint) + + // Must NOT use "echo" as the executable (stub pattern) + assert.NotContains(t, content, `"echo"`, + "%s must not use \"echo\" as stub executable", path) + + // Must use exec.CommandContext (the real execution path) + assert.Contains(t, content, "exec.CommandContext", + "%s must use exec.CommandContext to invoke the entrypoint", path) + } +} + +// --------------------------------------------------------------------------- +// AC5.3: MCP generated code uses execFile, not TODO stubs +// +// Every tool file must use execFile with its manifest entrypoint. No TODO +// stubs should remain in the generated output. +// --------------------------------------------------------------------------- + +func TestIntegration_TSMCP_EntrypointNotTODO(t *testing.T) { + m := integrationManifest() + files := generateTSMCP(t, m) + + toolFiles := map[string]string{ + "src/tools/check-health.ts": "./health.sh", + "src/tools/deploy-app.ts": "./deploy.sh", + "src/tools/fetch-secrets.ts": "./secrets.sh", + } + + for path, entrypoint := range toolFiles { + content := fileContent(t, files, path) + + // Must contain the exact entrypoint string from the manifest + assert.Contains(t, content, entrypoint, + "%s must contain its manifest entrypoint %q", path, entrypoint) + + // Must NOT contain TODO stub comments + assert.NotContains(t, content, "// TODO: implement", + "%s must not contain TODO implementation stubs", path) + + // Must use execFile (the real execution mechanism) + assert.Contains(t, content, "await execFile(", + "%s must use await execFile() to invoke the entrypoint", path) + } +} + +// --------------------------------------------------------------------------- +// AC5: CLI code has cliArgs construction for entrypoint invocation +// +// All tool files must build a cliArgs slice that is passed to the entrypoint. +// This catches a lazy implementation that calls exec.Command with no arguments. +// --------------------------------------------------------------------------- + +func TestIntegration_GoCLI_CliArgsConstruction(t *testing.T) { + m := integrationManifest() + files := generateCLI(t, m) + + toolFiles := []string{ + "internal/commands/check-health.go", + "internal/commands/deploy-app.go", + "internal/commands/fetch-secrets.go", + } + + for _, path := range toolFiles { + content := fileContent(t, files, path) + + // Must declare the cliArgs variable + assert.Contains(t, content, "cliArgs", + "%s must build a cliArgs slice for entrypoint invocation", path) + + // Must use cliArgs in the exec.CommandContext call (not an empty call) + assert.Contains(t, content, "cliArgs...", + "%s must spread cliArgs into exec.CommandContext", path) + } +} + +// --------------------------------------------------------------------------- +// AC5: MCP code has args construction for entrypoint invocation +// +// All tool files must declare const args: string[] and pass it to execFile. +// --------------------------------------------------------------------------- + +func TestIntegration_TSMCP_ArgsConstruction(t *testing.T) { + m := integrationManifest() + files := generateTSMCP(t, m) + + toolFiles := []string{ + "src/tools/check-health.ts", + "src/tools/deploy-app.ts", + "src/tools/fetch-secrets.ts", + } + + for _, path := range toolFiles { + content := fileContent(t, files, path) + + // Must declare the args array + assert.Contains(t, content, "const args: string[]", + "%s must declare a typed args array for entrypoint invocation", path) + + // Must push positional args (all tools have at least one arg) + assert.Contains(t, content, "args.push(", + "%s must push arguments into the args array", path) + } +} + +// --------------------------------------------------------------------------- +// AC5: CLI token tool passes --token in cliArgs +// +// deploy-app has auth:token with TokenFlag "--token". The generated code must +// append "--token" and the resolved token value to cliArgs so the entrypoint +// receives the token via CLI flag (constitution rule 24). +// --------------------------------------------------------------------------- + +func TestIntegration_GoCLI_TokenInCliArgs(t *testing.T) { + m := integrationManifest() + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/deploy-app.go") + + // Must pass --token as a flag in cliArgs (not just declare the token var) + assert.Contains(t, content, `"--token"`, + "deploy-app must pass --token flag in cliArgs for entrypoint invocation") + + // Must also reference the token variable in the cliArgs append + assert.Contains(t, content, "token", + "deploy-app must include the resolved token value in cliArgs") + + // The token flag must be appended to cliArgs, not just used in cobra flags + assert.Regexp(t, `cliArgs\s*=\s*append\(cliArgs.*"--token"`, content, + "deploy-app must append --token to cliArgs for the entrypoint") +} + +// --------------------------------------------------------------------------- +// AC5: MCP token tool passes --token in args and reads token from env +// +// deploy-app has auth:token with TokenEnv "DEPLOY_TOKEN" and +// TokenFlag "--token". The MCP handler must read the env var and pass +// the token via --token flag to the entrypoint. +// --------------------------------------------------------------------------- + +func TestIntegration_TSMCP_TokenInArgs(t *testing.T) { + m := integrationManifest() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/deploy-app.ts") + + // Must read the token from the environment variable + assert.Contains(t, content, "DEPLOY_TOKEN", + "deploy-app.ts must reference the DEPLOY_TOKEN env var") + assert.Contains(t, content, "envToken", + "deploy-app.ts must store env token in envToken variable") + + // Must pass --token flag in args for the entrypoint + assert.Contains(t, content, `"--token"`, + "deploy-app.ts must pass --token flag in args for the entrypoint") + + // Must push the token into args (not just read it) + assert.Contains(t, content, "args.push(", + "deploy-app.ts must push --token and envToken into args") +} + +// --------------------------------------------------------------------------- +// AC6: CLI no-auth tool doesn't reference --token in cliArgs +// +// check-health has auth:none. Its generated code must not inject any +// token-related arguments into cliArgs — the entrypoint should not +// receive credentials it doesn't need. +// --------------------------------------------------------------------------- + +func TestIntegration_GoCLI_NoAuthTool_NoTokenInCliArgs(t *testing.T) { + m := integrationManifest() + files := generateCLI(t, m) + content := fileContent(t, files, "internal/commands/check-health.go") + + // Must NOT contain --token in the generated code at all + assert.NotContains(t, content, `"--token"`, + "check-health (auth:none) must not pass --token flag") + + // Must NOT reference DEPLOY_TOKEN (that belongs to deploy-app) + assert.NotContains(t, content, "DEPLOY_TOKEN", + "check-health (auth:none) must not reference DEPLOY_TOKEN") + + // Must NOT have token resolution code + assert.NotContains(t, content, "os.Getenv", + "check-health (auth:none) must not call os.Getenv for token resolution") +} + +// --------------------------------------------------------------------------- +// AC6: MCP no-auth tool doesn't reference tokens in args +// +// check-health (auth:none) must not have any token-related argument pushing. +// --------------------------------------------------------------------------- + +func TestIntegration_TSMCP_NoAuthTool_NoTokenInArgs(t *testing.T) { + m := integrationManifest() + files := generateTSMCP(t, m) + content := fileContent(t, files, "src/tools/check-health.ts") + + // Must NOT contain --token flag + assert.NotContains(t, content, `"--token"`, + "check-health.ts (auth:none) must not pass --token flag") + + // Must NOT reference DEPLOY_TOKEN + assert.NotContains(t, content, "DEPLOY_TOKEN", + "check-health.ts (auth:none) must not reference DEPLOY_TOKEN") + + // Must NOT have envToken variable + assert.NotContains(t, content, "envToken", + "check-health.ts (auth:none) must not have an envToken variable") +} + +// --------------------------------------------------------------------------- +// AC5: Entrypoint values match manifest exactly per tool +// +// This verifies that each tool gets its OWN entrypoint, not a copy of another +// tool's entrypoint. Catches a template bug where all tools share the same +// entrypoint value. +// --------------------------------------------------------------------------- + +func TestIntegration_GoCLI_EntrypointMatchesManifestPerTool(t *testing.T) { + m := integrationManifest() + files := generateCLI(t, m) + + // Each tool must contain ONLY its own entrypoint, not other tools' entrypoints. + healthContent := fileContent(t, files, "internal/commands/check-health.go") + deployContent := fileContent(t, files, "internal/commands/deploy-app.go") + secretsContent := fileContent(t, files, "internal/commands/fetch-secrets.go") + + // check-health -> ./health.sh only + assert.Contains(t, healthContent, "./health.sh") + assert.NotContains(t, healthContent, "./deploy.sh", + "check-health.go must not contain deploy-app's entrypoint") + assert.NotContains(t, healthContent, "./secrets.sh", + "check-health.go must not contain fetch-secrets' entrypoint") + + // deploy-app -> ./deploy.sh only + assert.Contains(t, deployContent, "./deploy.sh") + assert.NotContains(t, deployContent, "./health.sh", + "deploy-app.go must not contain check-health's entrypoint") + assert.NotContains(t, deployContent, "./secrets.sh", + "deploy-app.go must not contain fetch-secrets' entrypoint") + + // fetch-secrets -> ./secrets.sh only + assert.Contains(t, secretsContent, "./secrets.sh") + assert.NotContains(t, secretsContent, "./health.sh", + "fetch-secrets.go must not contain check-health's entrypoint") + assert.NotContains(t, secretsContent, "./deploy.sh", + "fetch-secrets.go must not contain deploy-app's entrypoint") +} + +// --------------------------------------------------------------------------- +// AC5: TS MCP entrypoint values match manifest exactly per tool +// --------------------------------------------------------------------------- + +func TestIntegration_TSMCP_EntrypointMatchesManifestPerTool(t *testing.T) { + m := integrationManifest() + files := generateTSMCP(t, m) + + healthContent := fileContent(t, files, "src/tools/check-health.ts") + deployContent := fileContent(t, files, "src/tools/deploy-app.ts") + secretsContent := fileContent(t, files, "src/tools/fetch-secrets.ts") + + // check-health -> ./health.sh only + assert.Contains(t, healthContent, "./health.sh") + assert.NotContains(t, healthContent, "./deploy.sh", + "check-health.ts must not contain deploy-app's entrypoint") + assert.NotContains(t, healthContent, "./secrets.sh", + "check-health.ts must not contain fetch-secrets' entrypoint") + + // deploy-app -> ./deploy.sh only + assert.Contains(t, deployContent, "./deploy.sh") + assert.NotContains(t, deployContent, "./health.sh", + "deploy-app.ts must not contain check-health's entrypoint") + assert.NotContains(t, deployContent, "./secrets.sh", + "deploy-app.ts must not contain fetch-secrets' entrypoint") + + // fetch-secrets -> ./secrets.sh only + assert.Contains(t, secretsContent, "./secrets.sh") + assert.NotContains(t, secretsContent, "./health.sh", + "fetch-secrets.ts must not contain check-health's entrypoint") + assert.NotContains(t, secretsContent, "./deploy.sh", + "fetch-secrets.ts must not contain deploy-app's entrypoint") +} + +// --------------------------------------------------------------------------- +// AC5.4: TS files have balanced braces (basic syntax validity) +// +// Catches template rendering bugs that leave unclosed { or } in generated +// TypeScript files. This is a structural check, not a full parse, but it +// catches the most common class of template brace-mismatch bugs. +// --------------------------------------------------------------------------- + +func TestIntegration_TSMCP_BalancedBraces(t *testing.T) { + m := integrationManifest() + files := generateTSMCP(t, m) + + for _, f := range files { + if !strings.HasSuffix(f.Path, ".ts") { + continue + } + content := string(f.Content) + openBraces := strings.Count(content, "{") + closeBraces := strings.Count(content, "}") + assert.Equal(t, openBraces, closeBraces, + "TS file %q has unbalanced braces: %d open vs %d close", + f.Path, openBraces, closeBraces) + } +} From 7c2a046b724ca207152c0cbf31c04fa1d74e45fd Mon Sep 17 00:00:00 2001 From: MacAttak Date: Wed, 11 Mar 2026 07:10:40 +1100 Subject: [PATCH 4/4] =?UTF-8?q?fix(codegen):=20resolve=20PR=20review=20fin?= =?UTF-8?q?dings=20=E2=80=94=20dead=20code=20guard,=20unused=20import?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use local variable for entrypoint in both CLI and MCP templates to eliminate constant-expression dead code (staticcheck SA9003 / ESLint) - Remove unused validateRequest import from MCP tool template — it's HTTP middleware not applicable in MCP tool handlers - Update 3 CLI test assertions to match variable-based pattern Co-Authored-By: Claude Opus 4.6 --- internal/codegen/cli_go.go | 7 ++++--- internal/codegen/cli_go_test.go | 18 ++++++++++-------- internal/codegen/mcp_typescript.go | 10 ++++------ 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/internal/codegen/cli_go.go b/internal/codegen/cli_go.go index aa12802..11fbbb6 100644 --- a/internal/codegen/cli_go.go +++ b/internal/codegen/cli_go.go @@ -778,7 +778,8 @@ var {{.GoName}}Cmd = &cobra.Command{ return fmt.Errorf("auth required: set {{$tokenEnv | esc}} or pass --{{$tokenFlag | esc}}") } {{- end}} - if "{{$entrypoint | esc}}" == "" { + entrypoint := "{{$entrypoint | esc}}" + if entrypoint == "" { return fmt.Errorf("{{$toolName}}: entrypoint not configured") } var cliArgs []string @@ -898,7 +899,7 @@ var {{.GoName}}Cmd = &cobra.Command{ if isTTY && {{$goName}}FlagOutput == "" { return fmt.Errorf("binary output requires --output or pipe") } - c := exec.CommandContext(ctx, "{{$entrypoint | esc}}", cliArgs...) + c := exec.CommandContext(ctx, entrypoint, cliArgs...) c.Stderr = os.Stderr if {{$goName}}FlagOutput != "" { out, err := c.Output() @@ -916,7 +917,7 @@ var {{.GoName}}Cmd = &cobra.Command{ } {{- else}} // Execute the tool entrypoint. - c := exec.CommandContext(ctx, "{{$entrypoint | esc}}", cliArgs...) + c := exec.CommandContext(ctx, entrypoint, cliArgs...) c.Stdout = os.Stdout c.Stderr = os.Stderr if err := c.Run(); err != nil { diff --git a/internal/codegen/cli_go_test.go b/internal/codegen/cli_go_test.go index 1ab0708..bec729c 100644 --- a/internal/codegen/cli_go_test.go +++ b/internal/codegen/cli_go_test.go @@ -2317,8 +2317,10 @@ func TestGoCLI_AC1_EntrypointInExecCommandContext(t *testing.T) { files := generateCLI(t, manifestEntrypointWiring()) content := fileContent(t, files, "internal/commands/run-job.go") - assert.Regexp(t, `exec\.CommandContext\([^)]*"/usr/local/bin/run-job"`, content, - "entrypoint must be the executable argument to exec.CommandContext") + assert.Contains(t, content, `entrypoint := "/usr/local/bin/run-job"`, + "entrypoint must be assigned from the manifest value") + assert.Regexp(t, `exec\.CommandContext\([^,]+,\s*entrypoint\b`, content, + "exec.CommandContext must use the entrypoint variable") } func TestGoCLI_AC1_EntrypointFromManifest_TableDriven(t *testing.T) { @@ -2381,10 +2383,10 @@ func TestGoCLI_AC1_BuildToolData_EntrypointPopulated(t *testing.T) { files := generateCLI(t, m) content := fileContent(t, files, "internal/commands/run-job.go") - // The entrypoint must appear in exec.CommandContext — this proves the + // The entrypoint must be assigned from the manifest — this proves the // struct carried it through to the template. - assert.Regexp(t, `exec\.CommandContext\([^)]*"/usr/local/bin/run-job"`, content, - "buildToolData must populate Entrypoint so the template can interpolate it into exec.CommandContext") + assert.Contains(t, content, `entrypoint := "/usr/local/bin/run-job"`, + "buildToolData must populate Entrypoint so the template can assign it to a local variable") } func TestGoCLI_AC1_EmptyEntrypoint_ProducesGuard(t *testing.T) { @@ -2751,9 +2753,9 @@ func TestGoCLI_AC2_CliArgsPassedToExecCommand(t *testing.T) { content := fileContent(t, files, "internal/commands/run-job.go") // exec.CommandContext takes (ctx, name, arg...) — the cliArgs must be - // spread into it. - assert.Regexp(t, `CommandContext\([^,]+,\s*"[^"]*"[^)]*cliArgs`, content, - "cliArgs must be passed to exec.CommandContext (e.g., cliArgs...)") + // spread via the entrypoint variable. + assert.Regexp(t, `CommandContext\([^,]+,\s*entrypoint[^)]*cliArgs`, content, + "cliArgs must be passed to exec.CommandContext via entrypoint variable (e.g., entrypoint, cliArgs...)") } func TestGoCLI_AC2_MultiplePositionalArgs_InOrder(t *testing.T) { diff --git a/internal/codegen/mcp_typescript.go b/internal/codegen/mcp_typescript.go index ebd2bc1..7ddc09a 100644 --- a/internal/codegen/mcp_typescript.go +++ b/internal/codegen/mcp_typescript.go @@ -683,9 +683,6 @@ const tsToolTmpl = `import { McpServer } from "@modelcontextprotocol/sdk/server/ import { z } from "zod"; import { execFile as execFileCb } from "node:child_process"; import { promisify } from "node:util"; -{{- if .HasAuth}} -import { validateRequest } from "../auth/middleware.js"; -{{- end}} const execFile = promisify(execFileCb); @@ -724,7 +721,8 @@ async function handle_{{.ToolName}}(input: {{.ToolName}}Input): Promise<{ conten } {{- end}} // Empty entrypoint guard - if (!"{{.Entrypoint | esc}}") { + const entrypoint = "{{.Entrypoint | esc}}"; + if (!entrypoint) { throw new Error("{{.ToolName | esc}}: entrypoint not configured"); } const args: string[] = []; @@ -760,7 +758,7 @@ async function handle_{{.ToolName}}(input: {{.ToolName}}Input): Promise<{ conten {{- end}} // Execute the entrypoint {{- if .IsBinaryOutput}} - const { stdout } = await execFile("{{.Entrypoint | esc}}", args, { encoding: "buffer" }); + const { stdout } = await execFile(entrypoint, args, { encoding: "buffer" }); const base64Data = Buffer.from(stdout).toString("base64"); {{- if .IsImageMime}} return { @@ -782,7 +780,7 @@ async function handle_{{.ToolName}}(input: {{.ToolName}}Input): Promise<{ conten }; {{- end}} {{- else}} - const { stdout } = await execFile("{{.Entrypoint | esc}}", args); + const { stdout } = await execFile(entrypoint, args); return { content: [{ type: "text", text: stdout }], };