diff --git a/internal/codegen/cli_go.go b/internal/codegen/cli_go.go index c4091eb..11fbbb6 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,158 @@ 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}} + entrypoint := "{{$entrypoint | esc}}" + if entrypoint == "" { + 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, 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 +910,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 +917,7 @@ var {{.GoName}}Cmd = &cobra.Command{ } {{- else}} // Execute the tool entrypoint. - c := exec.CommandContext(cmd.Context(), "echo", "running", "{{$toolName}}") + 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 ab5f10d..bec729c 100644 --- a/internal/codegen/cli_go_test.go +++ b/internal/codegen/cli_go_test.go @@ -2251,3 +2251,766 @@ 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.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) { + // 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 be assigned from the manifest — this proves the + // struct carried it through to the template. + 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) { + // 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 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) { + // 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") +} 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) + } +} diff --git a/internal/codegen/mcp_typescript.go b/internal/codegen/mcp_typescript.go index 936b0b4..7ddc09a 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,9 +681,10 @@ export { server }; const tsToolTmpl = `import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; -{{- if .HasAuth}} -import { validateRequest } from "../auth/middleware.js"; -{{- end}} +import { execFile as execFileCb } from "node:child_process"; +import { promisify } from "node:util"; + +const execFile = promisify(execFileCb); // Tool: {{.ToolName}} // Description: {{.Description}} @@ -713,15 +720,45 @@ async function handle_{{.ToolName}}(input: {{.ToolName}}Input): Promise<{ conten throw new Error("auth required: set the {{.TokenEnv | esc}} environment variable"); } {{- end}} + // Empty entrypoint guard + const entrypoint = "{{.Entrypoint | esc}}"; + if (!entrypoint) { + 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, args, { encoding: "buffer" }); const base64Data = Buffer.from(stdout).toString("base64"); {{- if .IsImageMime}} return { @@ -743,8 +780,9 @@ async function handle_{{.ToolName}}(input: {{.ToolName}}Input): Promise<{ conten }; {{- end}} {{- else}} + const { stdout } = await execFile(entrypoint, 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) + }) + } +}