diff --git a/CLAUDE.md b/CLAUDE.md index 3f29931..78d5731 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -427,15 +427,26 @@ Package local skill directories (a directory containing `SKILL.md`) and run them **`load` vs `run`:** `load` only publishes the agent (run it later with `agent run --name `); `run` starts local tool workers, launches the agent, and streams the execution. `serve` starts only the workers so the skill can be driven from elsewhere (e.g. the UI). +**Tool task types.** Each script in `scripts/` plus the built-in tools are served as the +Conductor task type `{skillName}__{tool}` — `read_skill_file`, and with a workspace enabled +`list_workspace_files`, `read_workspace_file`, `search_workspace`, `git_status`, `git_diff`. +`inputParameters.command` becomes the script's argv, stdout becomes `{"result": ""}`, +and a non-zero exit fails the task. Script language is chosen by extension +(`.py .sh .js .mjs .ts .rb .go .bat .cmd`). + +Because a tool is just a task type, a plain workflow can call one with no agent involved — +point a `SIMPLE` task at `{skillName}__{tool}` while `skill serve` is running. See +[WORKER_SKILL.md](./WORKER_SKILL.md). + ### Worker Commands Run task workers that poll Conductor and execute work locally. | Command | Description | Required Args | Optional Flags | Example | |---------|-------------|---------------|----------------|---------| -| `worker js ` | Run a JavaScript worker (EXPERIMENTAL) | JS file | `--type` (required), `--count`, `--worker-id`, `--domain`, `--timeout` | `conductor worker js worker.js --type my_task` | +| `worker js ` | Run a JavaScript worker (EXPERIMENTAL) | JS file | `--type` (required), `--count`, `--worker-id`, `--domain`, `--poll-timeout` | `conductor worker js worker.js --type my_task` | | `worker stdio [args...]` | Poll tasks and execute a command via stdin/stdout | command | `--type` (required), `--count`, `--worker-id`, `--domain`, `--poll-timeout`, `--exec-timeout`, `--verbose` | `conductor worker stdio ./handler.sh --type my_task` | -| `worker remote` | Run a worker from the job-runner registry (EXPERIMENTAL, Orkes only) | None | `--type` (required), `--count`, `--worker-id`, `--domain`, `--timeout`, `--refresh` | `conductor worker remote --type my_task` | +| `worker remote` | Run a worker from the job-runner registry (EXPERIMENTAL, Orkes only) | None | `--type` (required), `--count`, `--worker-id`, `--domain`, `--poll-timeout`, `--exec-timeout`, `--refresh` | `conductor worker remote --type my_task` | | `worker list-remote` | List workers in the job-runner registry (EXPERIMENTAL, Orkes only) | None | `--namespace` | `conductor worker list-remote` | **Flags:** @@ -443,13 +454,32 @@ Run task workers that poll Conductor and execute work locally. - `--count` - Number of tasks to poll in each batch (default: 1) - `--worker-id` - Worker ID reported to the server - `--domain` - Task domain -- `--timeout` / `--poll-timeout` - Poll timeout in milliseconds (default: 100) -- `--exec-timeout` - Execution timeout in seconds for `stdio` (default: 0 = no timeout) +- `--poll-timeout` - Server-side long-poll wait in milliseconds (default: 100) +- `--exec-timeout` - Per-task execution timeout in seconds. `stdio` and `remote` only — a + JavaScript worker runs in-process with no interrupt, so there is nothing to time out. + Default 0 (no timeout) for `stdio`, 100 for `remote`. +- `--timeout` - Deprecated hidden alias for `--poll-timeout` - `--verbose` - Print task and result JSON to stdout (`stdio` command) - `--refresh` - Force refresh the worker from the registry, ignoring cache - `--namespace` - Registry namespace to list workers from (default: `default`) -See [WORKER_JS.md](./WORKER_JS.md) and [WORKER_STDIO.md](./WORKER_STDIO.md) for the worker protocols. +All flavours share one poll loop; they differ only in how user code runs and in the result +shape it returns: + +| Flavour | Worker returns | Failure carries | +|---------|----------------|-----------------| +| `stdio` | `{"status","output","logs","reason"}` on stdout | `reasonForIncompletion` + logs | +| `js` | `{status, body}` from the script; `$.task` holds the task | `output.error` | +| skill tools | bare stdout, wrapped as `{"result": ...}` | `reasonForIncompletion` | + +Workers exit on Ctrl-C/SIGTERM once the in-flight batch finishes — a running task is left +to complete and report its real result rather than being killed, which would report a +failure the worker inflicted on itself and consume one of the task's retries. A second +signal exits immediately. Child processes receive `TASK_TYPE`, `TASK_ID`, `WORKFLOW_ID`, `EXECUTION_ID`, +`POLL_DOMAIN`, and the CLI's own `CONDUCTOR_SERVER_URL` and credentials. + +See [WORKER_JS.md](./WORKER_JS.md), [WORKER_STDIO.md](./WORKER_STDIO.md) and +[WORKER_SKILL.md](./WORKER_SKILL.md) for the worker protocols. ### Development Commands diff --git a/README.md b/README.md index 386cefd..de7606c 100644 --- a/README.md +++ b/README.md @@ -223,6 +223,7 @@ After installing, you'll get tab completion when typing `conductor `. * [Workers](#workers) * [Stdio Workers](#stdio-workers) * [JavaScript Workers (Built-in)](#javascript-workers--built-in-) + * [Skill Workers](#skill-workers) * [Remote Workers (Registry-based)](#remote-workers--registry-based-) * [Exit Codes](#exit-codes) * [Error Handling](#error-handling) @@ -548,8 +549,8 @@ conductor worker [arguments] [flags] | Command | Description | |---------|-------------| | `stdio [args...]` | Run stdio worker (`--type`, `--count`, `--worker-id`, `--domain`, `--poll-timeout`, `--exec-timeout`, `--verbose`) | -| `js ` | Run JavaScript worker (`--type`, `--count`, `--worker-id`, `--domain`, `--timeout`) | -| `remote` | Run remote worker (`--type`, `--count`, `--worker-id`, `--domain`, `--refresh`) | +| `js ` | Run JavaScript worker (`--type`, `--count`, `--worker-id`, `--domain`, `--poll-timeout`) | +| `remote` | Run remote worker (`--type`, `--count`, `--worker-id`, `--domain`, `--poll-timeout`, `--exec-timeout`, `--refresh`) | | `list-remote` | List remote workers (`--namespace`) | **Worker Options:** @@ -852,7 +853,7 @@ conductor --config /path/to/my-config.yaml workflow list ⚠️ **EXPERIMENTAL FEATURES** -The CLI supports two types of workers for processing Conductor tasks: +The CLI supports several types of workers for processing Conductor tasks: ### Stdio Workers @@ -879,14 +880,27 @@ Execute tasks using **JavaScript** scripts with built-in utilities (HTTP, crypto **Best for:** Prototyping, Lightweight tasks, quick scripts, HTTP integrations -👉 **[Complete JavaScript Worker Documentation →](WORKER_JS.md)** - **Quick example:** ```bash # Run a JavaScript worker conductor worker js --type greet_task worker.js ``` +👉 **[Complete JavaScript Worker Documentation →](WORKER_JS.md)** + +### Skill Workers + +A skill directory (`SKILL.md` plus `scripts/`) serves each of its scripts as a +Conductor task type via `conductor skill serve`. Scripts take their arguments from +`inputParameters.command` and return bare stdout, so there is no result envelope to +emit. Works with or without an agent. + +```bash +conductor skill serve ./myskill +``` + +👉 **[Complete Skill Worker Documentation →](WORKER_SKILL.md)** + ### Remote Workers (Registry-based) ⚠️ **EXPERIMENTAL** - Download and execute workers directly from your Conductor Conductor instance without managing local files. diff --git a/WORKER_JS.md b/WORKER_JS.md index ec410fe..a9ed854 100644 --- a/WORKER_JS.md +++ b/WORKER_JS.md @@ -28,7 +28,11 @@ conductor worker js --type - `--count` - Number of tasks to poll in each batch (default: 1) - `--worker-id` - Worker ID for identification - `--domain` - Domain for task polling -- `--timeout` - Timeout in milliseconds (default: 100) +- `--poll-timeout` - Poll timeout in milliseconds (default: 100) +- `--timeout` - Deprecated alias for `--poll-timeout` + +There is no execution timeout for JavaScript workers: scripts run in-process and the +interpreter has no interrupt wired, so a script that loops forever blocks its worker. ### Example @@ -874,3 +878,5 @@ if (parsed.error) { | Custom Go functions | Modify `injectUtilities()` and rebuild | The JavaScript worker is designed for lightweight task processing with HTTP integration. For heavy processing or complex dependencies, consider calling external services that have full library support. + +See also [Stdio Workers](WORKER_STDIO.md) and [Skill Workers](WORKER_SKILL.md). diff --git a/WORKER_SKILL.md b/WORKER_SKILL.md new file mode 100644 index 0000000..12f1ba2 --- /dev/null +++ b/WORKER_SKILL.md @@ -0,0 +1,152 @@ +# Skill Workers + +A **skill** is a directory containing instructions and scripts. Registered with the server +it becomes an agent, but its tools still have to run on your machine — so the server +dispatches each tool call back to the CLI as a Conductor task, and the CLI runs it locally +and returns the result. + +That local half is a worker. It shares the poll loop with +[stdio workers](WORKER_STDIO.md) and [JavaScript workers](WORKER_JS.md), and it works +with or without an agent: because a skill tool is just a Conductor task type, an ordinary +workflow can call one directly. + +## Skill layout + +``` +myskill/ + SKILL.md # required; frontmatter must set `name` + scripts/ + greet.sh # each script becomes a tool +``` + +```markdown +--- +name: greetskill +description: Says hello +--- + +# Greet skill + +Instructions the agent reads. +``` + +```bash +#!/usr/bin/env bash +echo "Hello $1" +``` + +Script language is chosen by extension: `.py`, `.sh`, `.js`, `.mjs`, `.ts`, `.rb`, `.go`, +`.bat`, `.cmd`. Anything else is run with `bash`. + +## Serving the tools + +```bash +conductor skill serve ./myskill +# Serving workers for skill greetskill. Press Ctrl-C to stop. +``` + +`skill serve` starts one worker per tool and blocks. Use it when the skill is being run +somewhere else — from the UI, or by another process. `conductor skill run ` +starts the workers *and* runs the agent, stopping the workers when the run ends. + +## Task types + +Every tool is exposed as the task type: + +``` +{skillName}__{tool} +``` + +So `greetskill` with `scripts/greet.sh` serves `greetskill__greet`. Alongside the scripts, +these built-in tools are served too: + +| Tool | Task type | Purpose | +|---|---|---| +| `read_skill_file` | `{skill}__read_skill_file` | Read a file bundled with the skill | +| `list_workspace_files` | `{skill}__list_workspace_files` | List files in the workspace | +| `read_workspace_file` | `{skill}__read_workspace_file` | Read a workspace file | +| `search_workspace` | `{skill}__search_workspace` | Search the workspace | +| `git_status` | `{skill}__git_status` | Workspace git status | +| `git_diff` | `{skill}__git_diff` | Workspace git diff | + +The workspace tools are only served when a workspace is enabled; `--no-workspace` disables +them. + +## Tool contract + +Different from stdio workers, and simpler: + +**Input** — `inputParameters.command` is passed to the script as **arguments**, not on +stdin. Only that field reaches the script. + +**Output** — the script's **stdout** becomes the task output, wrapped as +`{"result": ""}`. There is no envelope to emit. + +**Failure** — a non-zero exit fails the task, with the captured output in the failure reason. + +**stderr is merged into stdout.** `executeScript` gives the script a single buffer for both, +so anything a script logs to stderr ends up inside `{"result": …}` on success. Keep +diagnostics out of a script whose output you care about. + +**Environment** — the skill root and the configured workspace roots are exported to the +script. + +## Using a skill tool from a plain workflow + +Nothing about this requires an agent. Point a `SIMPLE` task at the tool's task type: + +```json +{ + "name": "skill_as_worker", + "version": 1, + "tasks": [ + { + "name": "greetskill__greet", + "taskReferenceName": "g", + "type": "SIMPLE", + "inputParameters": { "command": "${workflow.input.name}" } + } + ] +} +``` + +```bash +conductor skill serve ./myskill & +conductor workflow start --workflow skill_as_worker --input '{"name":"Miguel"}' --sync +# { "result": "Hello Miguel\n" } +``` + +This makes a skill the lowest-ceremony way to run a script as a Conductor worker: no +result envelope, no SDK, and no protocol to implement. + +Two constraints to know before relying on it: + +- **The task type is fixed** as `{skill}__{tool}`, so an existing workflow cannot adopt a + skill tool without renaming its task. +- **Input is a single string.** Structured `inputData` does not reach the script; only + `command` does. Use a [stdio worker](WORKER_STDIO.md) when the task needs structured + input. + +## Flags + +| Flag | Applies to | Purpose | +|---|---|---| +| `--version` | run, serve | Skill version or checksum prefix | +| `--script-timeout` | run, serve | Per-script timeout in seconds (default 300) | +| `--script-output-limit` | run, serve | Max bytes captured from a script (default 10 MiB) | +| `--workspace` | run, serve | Workspace directory (default `.`) | +| `--no-workspace` | run, serve | Do not expose a workspace | +| `--filesystem name=path` | run, serve | Extra read-only root, repeatable | +| `--model` | run | Model for the agent (required for `run`) | +| `--param` | run | Skill parameter override, repeatable | + +## Comparison with other worker types + +| | Skill tools | [Stdio](WORKER_STDIO.md) | [JavaScript](WORKER_JS.md) | +|---|---|---|---| +| Task type | `{skill}__{tool}` | any | any | +| Input | `command` → argv | full task JSON on stdin | `$.task` | +| Output | bare stdout | `{status, output, logs, reason}` | `{status, body}` | +| Boilerplate | none | result envelope | result object | +| Structured input | no | yes | yes | +| Languages | by extension | any executable | JavaScript only | diff --git a/WORKER_STDIO.md b/WORKER_STDIO.md index c74cf12..316946a 100644 --- a/WORKER_STDIO.md +++ b/WORKER_STDIO.md @@ -260,9 +260,9 @@ func main() { 7. **Use environment variables**: Access `TASK_ID`, `WORKFLOW_ID` etc. when needed 8. **Exit with code 0**: Always exit with 0 and use status field for task outcome -## Comparison with JavaScript Workers +## Comparison with other worker types -| Feature | Generic Workers (exec) | JavaScript Workers (js) | +| Feature | Stdio Workers (stdio) | JavaScript Workers (js) | |---------|----------------------|------------------------| | Languages | Any (Python, Node, Go, etc.) | JavaScript only | | Dependencies | Full access to language ecosystem | Limited (Goja ES5.1+) | @@ -271,3 +271,5 @@ func main() { | HTTP Calls | Use language's HTTP library | Built-in `http` object | | File System | Full access | No access | | Best For | Complex logic, heavy dependencies | Lightweight tasks, quick scripts | + +See also [Skill Workers](WORKER_SKILL.md), which run a script with no result envelope at all. diff --git a/cmd/skill_run.go b/cmd/skill_run.go index 9c457d9..96c27f8 100644 --- a/cmd/skill_run.go +++ b/cmd/skill_run.go @@ -17,11 +17,8 @@ import ( "context" "encoding/json" "fmt" - "os" - "os/signal" "path/filepath" "strings" - "syscall" "time" "github.com/spf13/cobra" @@ -29,6 +26,7 @@ import ( "github.com/conductor-oss/conductor-cli/internal" "github.com/conductor-oss/conductor-cli/internal/agent" "github.com/conductor-oss/conductor-cli/internal/skillworker" + "github.com/conductor-oss/conductor-cli/internal/taskworker" ) // Skill run/serve flag defaults. @@ -119,8 +117,11 @@ func runSkillRun(cmd *cobra.Command, args []string) error { // One signal-aware context governs both the workers and the stream; cancelling // it (Ctrl-C) stops everything. Workers are also cancelled when the execution - // ends normally. - ctx, stop := signal.NotifyContext(cmd.Context(), os.Interrupt, syscall.SIGTERM) + // ends normally. A second interrupt exits outright, so a stream or tool script + // that ignores cancellation cannot leave the process unkillable. + ctx, cancel := context.WithCancel(cmd.Context()) + defer cancel() + stop := interruptWithEscalation(cancel) defer stop() workerCtx, cancelWorkers := context.WithCancel(ctx) defer cancelWorkers() @@ -152,7 +153,9 @@ func runSkillServe(cmd *cobra.Command, args []string) error { return err } - ctx, stop := signal.NotifyContext(cmd.Context(), os.Interrupt, syscall.SIGTERM) + ctx, cancel := context.WithCancel(cmd.Context()) + defer cancel() + stop := interruptWithEscalation(cancel) defer stop() startSkillWorkers(ctx, buildSkillWorkerRegistry(cfg, local, ws, scriptOptions(), skillWorkspaceFileLimit)) @@ -173,9 +176,10 @@ func scriptOptions() skillworker.ScriptOptions { // They run until ctx is cancelled. func startSkillWorkers(ctx context.Context, registry map[string]skillworker.ToolHandler) { taskClient := internal.GetTaskClient() + opts := skillworker.RunnerOptions() for taskType, handler := range registry { - w := skillworker.NewWorker(skillworker.NewConductorRunner(taskClient)) - go w.Run(ctx, taskType, handler) + w := taskworker.NewWorker(taskworker.NewConductorRunner(taskClient, opts), taskworker.Config{}) + go w.Run(ctx, taskType, skillworker.AsTaskHandler(handler)) } } diff --git a/cmd/worker.go b/cmd/worker.go index 84a4252..42e2405 100644 --- a/cmd/worker.go +++ b/cmd/worker.go @@ -11,35 +11,26 @@ * specific language governing permissions and limitations under the License. */ - package cmd import ( - "bytes" "context" - "crypto/md5" - "crypto/sha1" - "crypto/sha256" - "encoding/base64" - "encoding/hex" "encoding/json" "fmt" "io" "net/http" "os" "os/exec" + "os/signal" "path/filepath" "strings" - "sync" + "syscall" "time" - "github.com/antihax/optional" + "github.com/conductor-oss/conductor-cli/internal" + "github.com/conductor-oss/conductor-cli/internal/taskworker" "github.com/conductor-sdk/conductor-go/sdk/authentication" - "github.com/conductor-sdk/conductor-go/sdk/client" - "github.com/conductor-sdk/conductor-go/sdk/model" "github.com/conductor-sdk/conductor-go/sdk/settings" - "github.com/dop251/goja" - "github.com/conductor-oss/conductor-cli/internal" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -115,9 +106,9 @@ The worker runs in continuous mode, polling for tasks and executing them in para } workerListRemoteCmd = &cobra.Command{ - Use: "list-remote", - Short: "List available workers in the job-runner registry (EXPERIMENTAL, Orkes Conductor only)", - Long: `⚠️ EXPERIMENTAL FEATURE - List all available workers in the Orkes Conductor job-runner registry. + Use: "list-remote", + Short: "List available workers in the job-runner registry (EXPERIMENTAL, Orkes Conductor only)", + Long: `⚠️ EXPERIMENTAL FEATURE - List all available workers in the Orkes Conductor job-runner registry. ⚠️ Requires Orkes Conductor. Not available in OSS Conductor.`, RunE: listRemoteWorkers, SilenceUsage: true, @@ -125,11 +116,6 @@ The worker runs in continuous mode, polling for tasks and executing them in para } ) -type TaskResult struct { - Status string `json:"status"` - Body map[string]interface{} `json:"body"` -} - // WorkerCodeResponse represents the response from the job-runner worker-code API type WorkerCodeResponse struct { Id string `json:"id"` @@ -167,298 +153,30 @@ func runJsWorker(cmd *cobra.Command, args []string) error { return fmt.Errorf("--type flag is required") } - count, _ := cmd.Flags().GetInt32("count") - workerId, _ := cmd.Flags().GetString("worker-id") - domain, _ := cmd.Flags().GetString("domain") - timeout, _ := cmd.Flags().GetInt32("timeout") - scriptContent, err := os.ReadFile(jsFile) if err != nil { return fmt.Errorf("error reading JavaScript file: %v", err) } + pollOpts, _ := workerPollFlags(cmd) + fmt.Printf("Starting worker for task type: %s\n", taskType) fmt.Printf("JavaScript file: %s\n", jsFile) - fmt.Printf("Worker ID: %s\n", workerId) - - for { - opts := &client.TaskResourceApiBatchPollOpts{} - if workerId != "" { - opts.Workerid = optional.NewString(workerId) - } - if domain != "" { - opts.Domain = optional.NewString(domain) - } - if count > 0 { - opts.Count = optional.NewInt32(count) - } - if timeout > 0 { - opts.Timeout = optional.NewInt32(timeout) - } - - taskClient := internal.GetTaskClient() - tasks, _, err := taskClient.BatchPoll(context.Background(), taskType, opts) - if err != nil { - log.Errorf("Error polling tasks: %v", err) - continue - } - - if len(tasks) == 0 { - log.Debug("No tasks available") - continue - } - - log.Infof("Polled %d task(s)", len(tasks)) - - var wg sync.WaitGroup - for _, task := range tasks { - wg.Add(1) - go func(t model.Task) { - defer wg.Done() - processTask(t, string(scriptContent), taskClient) - }(task) - } - - wg.Wait() - } -} - -func processTask(task model.Task, script string, taskClient *client.TaskResourceApiService) { - log.Infof("Processing task: %s (workflow: %s)", task.TaskId, task.WorkflowInstanceId) - - vm := goja.New() - - taskJSON, err := json.Marshal(task) - if err != nil { - log.Errorf("Error marshaling task: %v", err) - updateTaskFailed(taskClient, task, fmt.Sprintf("Error marshaling task: %v", err)) - return - } - - var taskObj interface{} - err = json.Unmarshal(taskJSON, &taskObj) - if err != nil { - log.Errorf("Error unmarshaling task: %v", err) - updateTaskFailed(taskClient, task, fmt.Sprintf("Error unmarshaling task: %v", err)) - return - } - - dollarObj := vm.NewObject() - err = dollarObj.Set("task", taskObj) - if err != nil { - log.Errorf("Error setting task in $: %v", err) - updateTaskFailed(taskClient, task, fmt.Sprintf("Error setting task: %v", err)) - return - } - err = vm.Set("$", dollarObj) - if err != nil { - log.Errorf("Error setting $ object: %v", err) - updateTaskFailed(taskClient, task, fmt.Sprintf("Error setting $ object: %v", err)) - return - } - - injectUtilities(vm) - - result, err := vm.RunString(script) - if err != nil { - log.Errorf("Error executing script for task %s: %v", task.TaskId, err) - updateTaskFailed(taskClient, task, fmt.Sprintf("Script execution error: %v", err)) - return - } - - if result != nil && !goja.IsUndefined(result) && !goja.IsNull(result) { - resultJSON := result.Export() - resultBytes, err := json.Marshal(resultJSON) - if err != nil { - log.Errorf("Error marshaling script result: %v", err) - updateTaskCompleted(taskClient, task, map[string]interface{}{}) - return - } - - var taskResult TaskResult - err = json.Unmarshal(resultBytes, &taskResult) - if err != nil { - log.Warnf("Script result not in expected format, treating as completed") - updateTaskCompleted(taskClient, task, map[string]interface{}{"result": resultJSON}) - return - } - - if taskResult.Body == nil { - taskResult.Body = make(map[string]interface{}) - } - updateTaskWithStatus(taskClient, task, taskResult.Status, taskResult.Body) - } else { - updateTaskCompleted(taskClient, task, map[string]interface{}{}) - } -} + fmt.Printf("Worker ID: %s\n", pollOpts.WorkerID) -func updateTaskCompleted(taskClient *client.TaskResourceApiService, task model.Task, output map[string]interface{}) { - updateTaskWithStatus(taskClient, task, "COMPLETED", output) -} - -func updateTaskFailed(taskClient *client.TaskResourceApiService, task model.Task, reason string) { - output := map[string]interface{}{ - "error": reason, - } - updateTaskWithStatus(taskClient, task, "FAILED", output) -} - -func updateTaskWithStatus(taskClient *client.TaskResourceApiService, task model.Task, status string, output map[string]interface{}) { - log.Infof("Updating task %s with status: %s", task.TaskId, status) - - taskResult := &model.TaskResult{ - TaskId: task.TaskId, - WorkflowInstanceId: task.WorkflowInstanceId, - WorkerId: task.WorkerId, - Status: model.TaskResultStatus(status), - OutputData: output, - } - - _, _, err := taskClient.UpdateTask(context.Background(), taskResult) + handler, err := taskworker.NewGojaHandler(string(scriptContent), jsFile) if err != nil { - log.Errorf("Error updating task %s: %v", task.TaskId, err) - return - } - - log.Infof("Task %s updated successfully with status: %s", task.TaskId, status) -} - -// injectUtilities adds utility functions to the JavaScript VM -func injectUtilities(vm *goja.Runtime) { - // HTTP utilities - httpObj := vm.NewObject() - httpObj.Set("get", func(url string, headers map[string]interface{}) map[string]interface{} { - return httpRequest("GET", url, headers, "") - }) - httpObj.Set("post", func(url string, headers map[string]interface{}, body string) map[string]interface{} { - return httpRequest("POST", url, headers, body) - }) - httpObj.Set("put", func(url string, headers map[string]interface{}, body string) map[string]interface{} { - return httpRequest("PUT", url, headers, body) - }) - httpObj.Set("delete", func(url string, headers map[string]interface{}) map[string]interface{} { - return httpRequest("DELETE", url, headers, "") - }) - vm.Set("http", httpObj) - - // Crypto utilities - cryptoObj := vm.NewObject() - cryptoObj.Set("md5", func(text string) string { - hash := md5.Sum([]byte(text)) - return hex.EncodeToString(hash[:]) - }) - cryptoObj.Set("sha1", func(text string) string { - hash := sha1.Sum([]byte(text)) - return hex.EncodeToString(hash[:]) - }) - cryptoObj.Set("sha256", func(text string) string { - hash := sha256.Sum256([]byte(text)) - return hex.EncodeToString(hash[:]) - }) - cryptoObj.Set("base64Encode", func(text string) string { - return base64.StdEncoding.EncodeToString([]byte(text)) - }) - cryptoObj.Set("base64Decode", func(text string) string { - decoded, err := base64.StdEncoding.DecodeString(text) - if err != nil { - return "" - } - return string(decoded) - }) - vm.Set("crypto", cryptoObj) - - // Utility functions - utilObj := vm.NewObject() - utilObj.Set("sleep", func(ms int) { - time.Sleep(time.Duration(ms) * time.Millisecond) - }) - utilObj.Set("uuid", func() string { - return fmt.Sprintf("%d-%d", time.Now().UnixNano(), os.Getpid()) - }) - utilObj.Set("env", func(key string) string { - return os.Getenv(key) - }) - vm.Set("util", utilObj) - - // String utilities - stringObj := vm.NewObject() - stringObj.Set("toUpper", strings.ToUpper) - stringObj.Set("toLower", strings.ToLower) - stringObj.Set("trim", strings.TrimSpace) - stringObj.Set("split", func(s, sep string) []string { - return strings.Split(s, sep) - }) - stringObj.Set("join", func(arr []string, sep string) string { - return strings.Join(arr, sep) - }) - stringObj.Set("replace", func(s, old, new string) string { - return strings.ReplaceAll(s, old, new) - }) - stringObj.Set("contains", strings.Contains) - stringObj.Set("hasPrefix", strings.HasPrefix) - stringObj.Set("hasSuffix", strings.HasSuffix) - vm.Set("str", stringObj) -} - -func httpRequest(method, url string, headers map[string]interface{}, body string) map[string]interface{} { - var bodyReader io.Reader - if body != "" { - bodyReader = strings.NewReader(body) - } - - req, err := http.NewRequest(method, url, bodyReader) - if err != nil { - return map[string]interface{}{ - "error": err.Error(), - "status": 0, - } - } - - for key, value := range headers { - if strVal, ok := value.(string); ok { - req.Header.Set(key, strVal) - } - } - - client := &http.Client{Timeout: 30 * time.Second} - resp, err := client.Do(req) - if err != nil { - return map[string]interface{}{ - "error": err.Error(), - "status": 0, - } - } - defer resp.Body.Close() - - respBody, err := io.ReadAll(resp.Body) - if err != nil { - return map[string]interface{}{ - "error": err.Error(), - "status": resp.StatusCode, - } - } - - var jsonBody interface{} - if err := json.Unmarshal(respBody, &jsonBody); err == nil { - return map[string]interface{}{ - "status": resp.StatusCode, - "body": jsonBody, - "text": string(respBody), - } + return err } - return map[string]interface{}{ - "status": resp.StatusCode, - "text": string(respBody), - } + return runWorkerLoop(cmd, taskType, handler, jsRunnerOptions(pollOpts)) } -// WorkerResult represents the expected output from a worker command -type WorkerResult struct { - Status string `json:"status"` // COMPLETED | FAILED | IN_PROGRESS - Output map[string]interface{} `json:"output,omitempty"` - Logs []string `json:"logs,omitempty"` - Reason string `json:"reason,omitempty"` +// jsRunnerOptions adjusts poll options for JavaScript workers, which report the polled +// task's own worker id on the result rather than the configured --worker-id. +func jsRunnerOptions(opts taskworker.RunnerOptions) taskworker.RunnerOptions { + opts.UseTaskWorkerID = true + return opts } func execWorker(cmd *cobra.Command, args []string) error { @@ -474,241 +192,104 @@ func execWorker(cmd *cobra.Command, args []string) error { workerCmd := args[0] workerArgs := args[1:] - workerId, _ := cmd.Flags().GetString("worker-id") - domain, _ := cmd.Flags().GetString("domain") - pollTimeout, _ := cmd.Flags().GetInt32("poll-timeout") - execTimeout, _ := cmd.Flags().GetInt32("exec-timeout") - count, _ := cmd.Flags().GetInt32("count") + pollOpts, execTimeout := workerPollFlags(cmd) verbose, _ := cmd.Flags().GetBool("verbose") - taskClient := internal.GetTaskClient() - fmt.Printf("Starting worker for task type: %s\n", taskType) fmt.Printf("Command: %s %v\n", workerCmd, workerArgs) - if workerId != "" { - fmt.Printf("Worker ID: %s\n", workerId) + if pollOpts.WorkerID != "" { + fmt.Printf("Worker ID: %s\n", pollOpts.WorkerID) } - for { - opts := &client.TaskResourceApiBatchPollOpts{} - if workerId != "" { - opts.Workerid = optional.NewString(workerId) - } - if domain != "" { - opts.Domain = optional.NewString(domain) - } - if count > 0 { - opts.Count = optional.NewInt32(count) - } - if pollTimeout > 0 { - opts.Timeout = optional.NewInt32(pollTimeout) - } + handler := taskworker.NewStdioHandler(taskworker.StdioOptions{ + Command: workerCmd, + Args: workerArgs, + Env: workerChildEnv(), + Domain: pollOpts.Domain, + ExecTimeout: execTimeout, + Verbose: verbose, + }) - tasks, _, err := taskClient.BatchPoll(context.Background(), taskType, opts) - if err != nil { - log.Errorf("Error polling tasks: %v", err) - continue - } + return runWorkerLoop(cmd, taskType, handler, pollOpts) +} - if len(tasks) == 0 { - log.Debug("No tasks available") - continue - } +// runWorkerLoop drives a handler with the shared poll loop until the user interrupts it. +// Every worker flavour funnels through here, so the loop exists once rather than per +// flavour. +func runWorkerLoop(cmd *cobra.Command, taskType string, h taskworker.Handler, opts taskworker.RunnerOptions) error { + ctx, cancel := context.WithCancel(cmd.Context()) + defer cancel() + stop := interruptWithEscalation(cancel) + defer stop() + + runner := taskworker.NewConductorRunner(internal.GetTaskClient(), opts) + taskworker.NewWorker(runner, taskworker.Config{}).Run(ctx, taskType, h) + return nil +} - log.Infof("Polled %d task(s)", len(tasks)) +// interruptWithEscalation cancels on the first interrupt and force-exits on the second. +// +// signal.NotifyContext alone is not enough here: it keeps the signal channel registered +// after firing once, so the default disposition never returns and further Ctrl-C presses +// are swallowed. A handler that cannot be interrupted — a goja script with no vm.Interrupt +// wired, or a subprocess whose grandchild is holding the captured pipes open — would then +// leave the worker unkillable by anything short of SIGQUIT. +// +// So the first signal asks the loop to drain, and a second one means the user is done +// waiting. +func interruptWithEscalation(cancel context.CancelFunc) (stop func()) { + signals := make(chan os.Signal, 2) + signal.Notify(signals, os.Interrupt, syscall.SIGTERM) + + done := make(chan struct{}) + go func() { + select { + case <-signals: + fmt.Fprintln(os.Stderr, "\nShutting down; press Ctrl-C again to exit immediately.") + cancel() + case <-done: + return + } - // Process tasks in parallel goroutines - var wg sync.WaitGroup - for _, task := range tasks { - wg.Add(1) - go func(t model.Task) { - defer wg.Done() - executeExternalWorker(t, workerCmd, workerArgs, workerId, domain, execTimeout, verbose, taskClient) - }(task) + select { + case <-signals: + os.Exit(130) + case <-done: } + }() - wg.Wait() + return func() { + signal.Stop(signals) + close(done) } } -func executeExternalWorker(task model.Task, workerCmd string, workerArgs []string, workerId, domain string, execTimeout int32, verbose bool, taskClient *client.TaskResourceApiService) { - log.Infof("Processing task: %s (workflow: %s)", task.TaskId, task.WorkflowInstanceId) - - taskJSON, err := json.Marshal(task) - if err != nil { - log.Errorf("Error marshaling task: %v", err) - updateExecTaskFailed(taskClient, task, workerId, fmt.Sprintf("error marshaling task: %v", err)) - return - } - - if verbose { - fmt.Println("=== Task Input ===") - fmt.Println(string(taskJSON)) - fmt.Println("==================") - } +// workerChildEnv builds the Conductor environment handed to worker subprocesses, so a +// worker can call back into Conductor with the same server and credentials as the CLI. +// +// Resolving viper here keeps process-global config in the cmd layer: internal/taskworker +// receives a plain []string. +func workerChildEnv() []string { + var env []string - ctx := context.Background() - if execTimeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, time.Duration(execTimeout)*time.Second) - defer cancel() - } - - execCmd := exec.CommandContext(ctx, workerCmd, workerArgs...) - execCmd.Env = append(execCmd.Environ(), - "TASK_TYPE="+task.TaskType, - "TASK_ID="+task.TaskId, - "WORKFLOW_ID="+task.WorkflowInstanceId, - "EXECUTION_ID="+task.WorkflowInstanceId, - ) - if domain != "" { - execCmd.Env = append(execCmd.Env, "POLL_DOMAIN="+domain) - } - - serverUrl := viper.GetString("server") - if serverUrl != "" { + if serverUrl := viper.GetString("server"); serverUrl != "" { serverUrl = strings.TrimSuffix(serverUrl, "/") if !strings.HasSuffix(serverUrl, "/api") { serverUrl = serverUrl + "/api" } - execCmd.Env = append(execCmd.Env, "CONDUCTOR_SERVER_URL="+serverUrl) - } - - authKey := viper.GetString("auth-key") - authSecret := viper.GetString("auth-secret") - if authKey != "" { - execCmd.Env = append(execCmd.Env, "CONDUCTOR_ACCESS_KEY_ID="+authKey) - } - if authSecret != "" { - execCmd.Env = append(execCmd.Env, "CONDUCTOR_ACCESS_KEY_SECRET="+authSecret) - } - - authToken := viper.GetString("auth-token") - if authToken != "" { - execCmd.Env = append(execCmd.Env, "CONDUCTOR_AUTH_TOKEN="+authToken) - } - - execCmd.Stdin = bytes.NewReader(taskJSON) - - var stdout, stderr bytes.Buffer - execCmd.Stdout = io.MultiWriter(&stdout, os.Stdout) - execCmd.Stderr = io.MultiWriter(&stderr, os.Stderr) - - execErr := execCmd.Run() - - var result WorkerResult - if execErr != nil { - stderrOutput := stderr.String() - log.Errorf("Worker execution failed: %v", execErr) - if stderrOutput != "" { - log.Errorf("Worker stderr:\n%s", stderrOutput) - } - - result = WorkerResult{ - Status: "FAILED", - Reason: fmt.Sprintf("worker execution failed: %v", execErr), - Logs: []string{stderrOutput}, - } - } else { - if err := json.Unmarshal(stdout.Bytes(), &result); err != nil { - stdoutOutput := stdout.String() - log.Errorf("Failed to parse worker output as JSON: %v", err) - log.Errorf("Worker stdout:\n%s", stdoutOutput) - - result = WorkerResult{ - Status: "FAILED", - Reason: fmt.Sprintf("invalid worker stdout JSON: %v", err), - Logs: []string{stdoutOutput}, - } - } - } - - if verbose { - resultJSON, _ := json.MarshalIndent(result, "", " ") - if result.Status == "FAILED" { - fmt.Println("=== Task Result (Error) ===") - fmt.Println(string(resultJSON)) - fmt.Println("===========================") - } else { - fmt.Println("=== Task Result ===") - fmt.Println(string(resultJSON)) - fmt.Println("===================") - } - } - - switch result.Status { - case "COMPLETED", "FAILED", "IN_PROGRESS": - default: - result.Status = "FAILED" - result.Reason = fmt.Sprintf("invalid status from worker: %s", result.Status) - } - - var status model.TaskResultStatus - switch result.Status { - case "COMPLETED": - status = model.CompletedTask - case "FAILED": - status = model.FailedTask - case "IN_PROGRESS": - status = model.InProgressTask - default: - status = model.FailedTask - } - - taskResult := model.TaskResult{ - TaskId: task.TaskId, - WorkflowInstanceId: task.WorkflowInstanceId, - Status: status, - } - - if result.Output != nil { - taskResult.OutputData = result.Output - } - - if len(result.Logs) > 0 { - logs := make([]model.TaskExecLog, len(result.Logs)) - for i, logLine := range result.Logs { - logs[i] = model.TaskExecLog{ - Log: logLine, - } - } - taskResult.Logs = logs + env = append(env, "CONDUCTOR_SERVER_URL="+serverUrl) } - - if result.Reason != "" { - taskResult.ReasonForIncompletion = result.Reason + if authKey := viper.GetString("auth-key"); authKey != "" { + env = append(env, "CONDUCTOR_ACCESS_KEY_ID="+authKey) } - - if workerId != "" { - taskResult.WorkerId = workerId + if authSecret := viper.GetString("auth-secret"); authSecret != "" { + env = append(env, "CONDUCTOR_ACCESS_KEY_SECRET="+authSecret) } - - _, _, err = taskClient.UpdateTask(context.Background(), &taskResult) - if err != nil { - log.Errorf("Error updating task %s: %v", task.TaskId, err) - return + if authToken := viper.GetString("auth-token"); authToken != "" { + env = append(env, "CONDUCTOR_AUTH_TOKEN="+authToken) } - log.Infof("Task %s completed with status: %s", task.TaskId, result.Status) -} - -func updateExecTaskFailed(taskClient *client.TaskResourceApiService, task model.Task, workerId, reason string) { - taskResult := model.TaskResult{ - TaskId: task.TaskId, - WorkflowInstanceId: task.WorkflowInstanceId, - Status: model.FailedTask, - ReasonForIncompletion: reason, - OutputData: map[string]interface{}{"error": reason}, - } - - if workerId != "" { - taskResult.WorkerId = workerId - } - - _, _, err := taskClient.UpdateTask(context.Background(), &taskResult) - if err != nil { - log.Errorf("Error updating task %s as failed: %v", task.TaskId, err) - } + return env } func listRemoteWorkers(cmd *cobra.Command, args []string) error { @@ -996,61 +577,24 @@ func fileExists(path string) bool { } func executeJsWorkerFromFile(cmd *cobra.Command, workerFile, taskType string) error { - count, _ := cmd.Flags().GetInt32("count") - workerId, _ := cmd.Flags().GetString("worker-id") - domain, _ := cmd.Flags().GetString("domain") - timeout, _ := cmd.Flags().GetInt32("timeout") - scriptContent, err := os.ReadFile(workerFile) if err != nil { return fmt.Errorf("error reading worker file: %v", err) } + pollOpts, _ := workerPollFlags(cmd) + log.Infof("Starting JavaScript worker for task type: %s", taskType) - if workerId != "" { - log.Infof("Worker ID: %s", workerId) + if pollOpts.WorkerID != "" { + log.Infof("Worker ID: %s", pollOpts.WorkerID) } - for { - opts := &client.TaskResourceApiBatchPollOpts{} - if workerId != "" { - opts.Workerid = optional.NewString(workerId) - } - if domain != "" { - opts.Domain = optional.NewString(domain) - } - if count > 0 { - opts.Count = optional.NewInt32(count) - } - if timeout > 0 { - opts.Timeout = optional.NewInt32(timeout) - } - - taskClient := internal.GetTaskClient() - tasks, _, err := taskClient.BatchPoll(context.Background(), taskType, opts) - if err != nil { - log.Errorf("Error polling tasks: %v", err) - continue - } - - if len(tasks) == 0 { - log.Debug("No tasks available") - continue - } - - log.Infof("Polled %d task(s)", len(tasks)) - - var wg sync.WaitGroup - for _, task := range tasks { - wg.Add(1) - go func(t model.Task) { - defer wg.Done() - processTask(t, string(scriptContent), taskClient) - }(task) - } - - wg.Wait() + handler, err := taskworker.NewGojaHandler(string(scriptContent), workerFile) + if err != nil { + return err } + + return runWorkerLoop(cmd, taskType, handler, jsRunnerOptions(pollOpts)) } func setupPythonEnvironment(cacheDir string, dependencies []string) error { @@ -1130,10 +674,7 @@ func equalStringSlices(a, b []string) bool { } func executePythonWorkerFromFile(cmd *cobra.Command, workerFile, taskType string) error { - count, _ := cmd.Flags().GetInt32("count") - workerId, _ := cmd.Flags().GetString("worker-id") - domain, _ := cmd.Flags().GetString("domain") - execTimeout, _ := cmd.Flags().GetInt32("timeout") + pollOpts, execTimeout := workerPollFlags(cmd) pythonCmd := "python3" cacheDir := filepath.Dir(workerFile) @@ -1147,52 +688,58 @@ func executePythonWorkerFromFile(cmd *cobra.Command, workerFile, taskType string } log.Infof("Starting Python worker for task type: %s", taskType) - if workerId != "" { - log.Infof("Worker ID: %s", workerId) + if pollOpts.WorkerID != "" { + log.Infof("Worker ID: %s", pollOpts.WorkerID) } - taskClient := internal.GetTaskClient() - - for { - opts := &client.TaskResourceApiBatchPollOpts{} - if workerId != "" { - opts.Workerid = optional.NewString(workerId) - } - if domain != "" { - opts.Domain = optional.NewString(domain) - } - if count > 0 { - opts.Count = optional.NewInt32(count) - } - if execTimeout > 0 { - opts.Timeout = optional.NewInt32(execTimeout) - } - - tasks, _, err := taskClient.BatchPoll(context.Background(), taskType, opts) - if err != nil { - log.Errorf("Error polling tasks: %v", err) - continue - } - - if len(tasks) == 0 { - log.Debug("No tasks available") - continue - } + handler := taskworker.NewStdioHandler(taskworker.StdioOptions{ + Command: pythonCmd, + Args: []string{workerFile}, + Env: workerChildEnv(), + Domain: pollOpts.Domain, + ExecTimeout: execTimeout, + }) - log.Infof("Polled %d task(s)", len(tasks)) + return runWorkerLoop(cmd, taskType, handler, pollOpts) +} - // Process tasks in parallel goroutines - var wg sync.WaitGroup - for _, task := range tasks { - wg.Add(1) - go func(t model.Task) { - defer wg.Done() - executeExternalWorker(t, pythonCmd, []string{workerFile}, workerId, domain, execTimeout, false, taskClient) - }(task) - } +// workerPollFlags reads the poll and execution flags shared by the worker subcommands. +// +// --poll-timeout and --exec-timeout are the canonical names. --timeout is a deprecated +// alias for --poll-timeout, kept because `worker js` and `worker remote` shipped with it. +// Previously `worker remote` fed one --timeout value to both, so the same number meant +// milliseconds of poll wait and seconds of execution budget at once (issue #91). +func workerPollFlags(cmd *cobra.Command) (taskworker.RunnerOptions, time.Duration) { + opts := taskworker.RunnerOptions{} + opts.WorkerID, _ = cmd.Flags().GetString("worker-id") + opts.Domain, _ = cmd.Flags().GetString("domain") + opts.Count, _ = cmd.Flags().GetInt32("count") + + opts.PollTimeoutMs, _ = cmd.Flags().GetInt32("poll-timeout") + if cmd.Flags().Changed("timeout") && !cmd.Flags().Changed("poll-timeout") { + legacy, _ := cmd.Flags().GetInt32("timeout") + opts.PollTimeoutMs = legacy + } + + execSeconds, _ := cmd.Flags().GetInt32("exec-timeout") + return opts, time.Duration(execSeconds) * time.Second +} - wg.Wait() - } +// addPollTimeoutFlags registers the timeout flags on a worker subcommand, plus a hidden +// deprecated --timeout alias. `worker js` and `worker remote` shipped with --timeout; +// `worker stdio` did not, but it accepts the alias too so that one spelling works +// everywhere rather than the alias being another per-command difference. +// +// The alias is hidden rather than removed so existing invocations keep working; it maps +// to --poll-timeout only. See workerPollFlags and issue #91. +func addPollTimeoutFlags(cmd *cobra.Command, execTimeout bool, execTimeoutDefault int32) { + cmd.Flags().Int32("poll-timeout", 100, "Poll timeout in milliseconds") + if execTimeout { + cmd.Flags().Int32("exec-timeout", execTimeoutDefault, "Worker execution timeout in seconds (0 = no timeout)") + } + cmd.Flags().Int32("timeout", 100, "Deprecated: use --poll-timeout") + _ = cmd.Flags().MarkHidden("timeout") + _ = cmd.Flags().MarkDeprecated("timeout", "use --poll-timeout instead") } func init() { @@ -1201,24 +748,26 @@ func init() { workerJsCmd.Flags().Int32("count", 1, "Number of tasks to poll in each batch") workerJsCmd.Flags().String("worker-id", "", "Worker ID") workerJsCmd.Flags().String("domain", "", "Domain") - workerJsCmd.Flags().Int32("timeout", 100, "Timeout in milliseconds") + addPollTimeoutFlags(workerJsCmd, false, 0) workerStdioCmd.Flags().String("type", "", "Task type to poll for (required)") workerStdioCmd.MarkFlagRequired("type") workerStdioCmd.Flags().String("worker-id", "", "Worker ID") workerStdioCmd.Flags().String("domain", "", "Domain") - workerStdioCmd.Flags().Int32("poll-timeout", 100, "Poll timeout in milliseconds") - workerStdioCmd.Flags().Int32("exec-timeout", 0, "Execution timeout in seconds (0 = no timeout)") workerStdioCmd.Flags().Int32("count", 1, "Number of tasks to poll in each batch") workerStdioCmd.Flags().Bool("verbose", false, "Print task and result JSON to stdout") + addPollTimeoutFlags(workerStdioCmd, true, 0) workerRemoteCmd.Flags().String("type", "", "Task type to poll for (required)") workerRemoteCmd.MarkFlagRequired("type") workerRemoteCmd.Flags().Int32("count", 1, "Number of tasks to poll in each batch") workerRemoteCmd.Flags().String("worker-id", "", "Worker ID") workerRemoteCmd.Flags().String("domain", "", "Domain") - workerRemoteCmd.Flags().Int32("timeout", 100, "Timeout in milliseconds") workerRemoteCmd.Flags().Bool("refresh", false, "Force refresh worker from registry (ignore cache)") + // Remote workers previously derived their execution timeout from --timeout, whose + // default was 100. Defaulting --exec-timeout to 100s keeps a hanging remote worker + // bounded as it was before the two timeouts were separated. + addPollTimeoutFlags(workerRemoteCmd, true, 100) workerListRemoteCmd.Flags().String("namespace", "default", "Namespace to list workers from") diff --git a/cmd/worker_flags_test.go b/cmd/worker_flags_test.go new file mode 100644 index 0000000..590a63f --- /dev/null +++ b/cmd/worker_flags_test.go @@ -0,0 +1,183 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package cmd + +import ( + "testing" + "time" + + "github.com/spf13/cobra" +) + +// workerFlagCmd builds a throwaway command carrying the same flags a worker subcommand +// registers, so the flag plumbing can be tested without running a worker. +func workerFlagCmd(t *testing.T, execTimeout bool, execTimeoutDefault int32, args ...string) *cobra.Command { + t.Helper() + cmd := &cobra.Command{Use: "fake"} + cmd.Flags().String("worker-id", "", "") + cmd.Flags().String("domain", "", "") + cmd.Flags().Int32("count", 1, "") + addPollTimeoutFlags(cmd, execTimeout, execTimeoutDefault) + + if err := cmd.ParseFlags(args); err != nil { + t.Fatalf("ParseFlags(%v) error = %v", args, err) + } + return cmd +} + +// TestWorkerPollFlagsTimeoutAlias is the regression test for issue #91: `worker remote` +// fed a single --timeout value to both the poll wait (milliseconds) and the execution +// budget (seconds), so one number meant two different things in two different units. +func TestWorkerPollFlagsTimeoutAlias(t *testing.T) { + tests := []struct { + name string + args []string + wantPollMs int32 + wantExecSecs time.Duration + }{ + { + name: "no flags uses defaults", + args: nil, + wantPollMs: 100, + wantExecSecs: 0, + }, + { + name: "poll-timeout alone", + args: []string{"--poll-timeout", "500"}, + wantPollMs: 500, + wantExecSecs: 0, + }, + { + name: "deprecated timeout alias maps to poll only", + args: []string{"--timeout", "4000"}, + wantPollMs: 4000, + wantExecSecs: 0, + }, + { + name: "poll-timeout wins when both are given", + args: []string{"--timeout", "4000", "--poll-timeout", "250"}, + wantPollMs: 250, + wantExecSecs: 0, + }, + { + name: "exec-timeout is independent of the poll wait", + args: []string{"--poll-timeout", "250", "--exec-timeout", "30"}, + wantPollMs: 250, + wantExecSecs: 30 * time.Second, + }, + { + // The #91 bug: one value must not become both a millisecond poll wait and a + // second-denominated execution budget. + name: "timeout alias does not also set the exec budget", + args: []string{"--timeout", "100"}, + wantPollMs: 100, + wantExecSecs: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := workerFlagCmd(t, true, 0, tt.args...) + opts, execTimeout := workerPollFlags(cmd) + + if opts.PollTimeoutMs != tt.wantPollMs { + t.Errorf("PollTimeoutMs = %d, want %d", opts.PollTimeoutMs, tt.wantPollMs) + } + if execTimeout != tt.wantExecSecs { + t.Errorf("execTimeout = %v, want %v", execTimeout, tt.wantExecSecs) + } + }) + } +} + +// TestWorkerPollFlagsRemoteExecTimeoutDefault pins the default that preserves the old +// effective behaviour: --timeout 100 previously reached the child as a 100 second kill +// timer, so defaulting remote's --exec-timeout to 0 would let a hanging worker run forever. +func TestWorkerPollFlagsRemoteExecTimeoutDefault(t *testing.T) { + cmd := workerFlagCmd(t, true, 100) + _, execTimeout := workerPollFlags(cmd) + + if execTimeout != 100*time.Second { + t.Errorf("execTimeout = %v, want 100s", execTimeout) + } +} + +// TestWorkerPollFlagsWithoutExecTimeout covers `worker js`, which registers no +// --exec-timeout because a goja script cannot be interrupted. Reading the flag must not +// panic on the command that lacks it. +func TestWorkerPollFlagsWithoutExecTimeout(t *testing.T) { + cmd := workerFlagCmd(t, false, 0, "--poll-timeout", "300") + opts, execTimeout := workerPollFlags(cmd) + + if opts.PollTimeoutMs != 300 { + t.Errorf("PollTimeoutMs = %d, want 300", opts.PollTimeoutMs) + } + if execTimeout != 0 { + t.Errorf("execTimeout = %v, want 0 when the flag is not registered", execTimeout) + } +} + +func TestWorkerPollFlagsPassesThroughIdentity(t *testing.T) { + cmd := workerFlagCmd(t, true, 0, "--worker-id", "w1", "--domain", "prod", "--count", "5") + opts, _ := workerPollFlags(cmd) + + if opts.WorkerID != "w1" { + t.Errorf("WorkerID = %q, want w1", opts.WorkerID) + } + if opts.Domain != "prod" { + t.Errorf("Domain = %q, want prod", opts.Domain) + } + if opts.Count != 5 { + t.Errorf("Count = %d, want 5", opts.Count) + } + if opts.UseTaskWorkerID { + t.Error("UseTaskWorkerID = true, want false — only JavaScript workers set it") + } +} + +// TestJsRunnerOptionsReportsTaskWorkerID pins that JavaScript workers report the polled +// task's own worker id, which is observable on the task result. +func TestJsRunnerOptionsReportsTaskWorkerID(t *testing.T) { + cmd := workerFlagCmd(t, false, 0, "--worker-id", "ignored-for-js") + opts, _ := workerPollFlags(cmd) + + if got := jsRunnerOptions(opts); !got.UseTaskWorkerID { + t.Error("jsRunnerOptions() did not set UseTaskWorkerID") + } +} + +// TestAddPollTimeoutFlagsRegistration checks the flag surface each worker command exposes: +// --timeout must be hidden everywhere, and --exec-timeout absent where it cannot be honoured. +func TestAddPollTimeoutFlagsRegistration(t *testing.T) { + withExec := workerFlagCmd(t, true, 0) + if withExec.Flags().Lookup("exec-timeout") == nil { + t.Error("--exec-timeout not registered when requested") + } + + withoutExec := workerFlagCmd(t, false, 0) + if withoutExec.Flags().Lookup("exec-timeout") != nil { + t.Error("--exec-timeout registered on a command that cannot honour it") + } + + alias := withExec.Flags().Lookup("timeout") + if alias == nil { + t.Fatal("--timeout alias not registered") + } + if !alias.Hidden { + t.Error("--timeout is not hidden; the deprecated alias should not appear in help") + } + if alias.Deprecated == "" { + t.Error("--timeout is not marked deprecated") + } +} diff --git a/cmd/worker_test.go b/cmd/worker_test.go index 1c30b2d..62fa50c 100644 --- a/cmd/worker_test.go +++ b/cmd/worker_test.go @@ -15,13 +15,9 @@ package cmd import ( "encoding/json" - "net/http" - "net/http/httptest" "os" "path/filepath" "testing" - - "github.com/dop251/goja" ) func TestGetWorkerFile(t *testing.T) { @@ -142,291 +138,6 @@ func TestLoadMetadata(t *testing.T) { }) } -func TestWorkerResultJSON(t *testing.T) { - tests := []struct { - name string - input string - status string - hasOut bool - }{ - { - name: "completed with output", - input: `{"status":"COMPLETED","output":{"key":"value"},"logs":["done"]}`, - status: "COMPLETED", - hasOut: true, - }, - { - name: "failed with reason", - input: `{"status":"FAILED","reason":"timeout"}`, - status: "FAILED", - hasOut: false, - }, - { - name: "in progress", - input: `{"status":"IN_PROGRESS"}`, - status: "IN_PROGRESS", - hasOut: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var result WorkerResult - err := json.Unmarshal([]byte(tt.input), &result) - if err != nil { - t.Fatalf("unmarshal failed: %v", err) - } - if result.Status != tt.status { - t.Errorf("Status: got %q, want %q", result.Status, tt.status) - } - if tt.hasOut && result.Output == nil { - t.Error("expected non-nil output") - } - }) - } -} - -func TestTaskResultJSON(t *testing.T) { - result := TaskResult{ - Status: "COMPLETED", - Body: map[string]interface{}{ - "message": "hello", - }, - } - - data, err := json.Marshal(result) - if err != nil { - t.Fatalf("marshal failed: %v", err) - } - - var decoded TaskResult - if err := json.Unmarshal(data, &decoded); err != nil { - t.Fatalf("unmarshal failed: %v", err) - } - - if decoded.Status != "COMPLETED" { - t.Errorf("Status: got %q, want %q", decoded.Status, "COMPLETED") - } - if decoded.Body["message"] != "hello" { - t.Errorf("Body.message: got %v, want %q", decoded.Body["message"], "hello") - } -} - -func TestHttpRequest(t *testing.T) { - t.Run("GET request", func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != "GET" { - t.Errorf("expected GET, got %s", r.Method) - } - if r.Header.Get("X-Custom") != "test" { - t.Errorf("missing custom header") - } - w.Header().Set("Content-Type", "application/json") - w.Write([]byte(`{"result":"ok"}`)) - })) - defer server.Close() - - result := httpRequest("GET", server.URL, map[string]interface{}{"X-Custom": "test"}, "") - if result["status"] != http.StatusOK { - t.Errorf("status: got %v, want %d", result["status"], http.StatusOK) - } - body, ok := result["body"].(map[string]interface{}) - if !ok { - t.Fatal("expected body to be a map") - } - if body["result"] != "ok" { - t.Errorf("body.result: got %v, want %q", body["result"], "ok") - } - }) - - t.Run("POST request with body", func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != "POST" { - t.Errorf("expected POST, got %s", r.Method) - } - w.Write([]byte(`{"created":true}`)) - })) - defer server.Close() - - result := httpRequest("POST", server.URL, nil, `{"name":"test"}`) - if result["status"] != http.StatusOK { - t.Errorf("status: got %v, want %d", result["status"], http.StatusOK) - } - }) - - t.Run("non-JSON response", func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte("plain text response")) - })) - defer server.Close() - - result := httpRequest("GET", server.URL, nil, "") - if result["text"] != "plain text response" { - t.Errorf("text: got %v, want %q", result["text"], "plain text response") - } - if result["body"] != nil { - t.Errorf("expected nil body for non-JSON, got %v", result["body"]) - } - }) - - t.Run("connection error", func(t *testing.T) { - result := httpRequest("GET", "http://localhost:1", nil, "") - if result["error"] == nil { - t.Error("expected error for connection failure") - } - if result["status"] != 0 { - t.Errorf("status: got %v, want 0", result["status"]) - } - }) -} - -func TestInjectUtilitiesCrypto(t *testing.T) { - vm := goja.New() - injectUtilities(vm) - - tests := []struct { - name string - script string - want string - }{ - {"md5", `crypto.md5("hello")`, "5d41402abc4b2a76b9719d911017c592"}, - {"sha1", `crypto.sha1("hello")`, "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d"}, - {"sha256", `crypto.sha256("hello")`, "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"}, - {"base64Encode", `crypto.base64Encode("hello world")`, "aGVsbG8gd29ybGQ="}, - {"base64Decode", `crypto.base64Decode("aGVsbG8gd29ybGQ=")`, "hello world"}, - {"base64Decode invalid", `crypto.base64Decode("!!!invalid!!!")`, ""}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - val, err := vm.RunString(tt.script) - if err != nil { - t.Fatalf("script error: %v", err) - } - if val.String() != tt.want { - t.Errorf("got %q, want %q", val.String(), tt.want) - } - }) - } -} - -func TestInjectUtilitiesString(t *testing.T) { - vm := goja.New() - injectUtilities(vm) - - tests := []struct { - name string - script string - want string - }{ - {"toUpper", `str.toUpper("hello")`, "HELLO"}, - {"toLower", `str.toLower("WORLD")`, "world"}, - {"trim", `str.trim(" spaces ")`, "spaces"}, - {"contains true", `str.contains("hello world", "world")`, "true"}, - {"contains false", `str.contains("hello", "xyz")`, "false"}, - {"hasPrefix", `str.hasPrefix("hello", "hel")`, "true"}, - {"hasSuffix", `str.hasSuffix("hello", "llo")`, "true"}, - {"replace", `str.replace("foo bar foo", "foo", "baz")`, "baz bar baz"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - val, err := vm.RunString(tt.script) - if err != nil { - t.Fatalf("script error: %v", err) - } - if val.String() != tt.want { - t.Errorf("got %q, want %q", val.String(), tt.want) - } - }) - } -} - -func TestInjectUtilitiesSplit(t *testing.T) { - vm := goja.New() - injectUtilities(vm) - - val, err := vm.RunString(`JSON.stringify(str.split("a,b,c", ","))`) - if err != nil { - t.Fatalf("script error: %v", err) - } - if val.String() != `["a","b","c"]` { - t.Errorf("got %q, want %q", val.String(), `["a","b","c"]`) - } -} - -func TestInjectUtilitiesJoin(t *testing.T) { - vm := goja.New() - injectUtilities(vm) - - val, err := vm.RunString(`str.join(["a","b","c"], "-")`) - if err != nil { - t.Fatalf("script error: %v", err) - } - if val.String() != "a-b-c" { - t.Errorf("got %q, want %q", val.String(), "a-b-c") - } -} - -func TestInjectUtilitiesEnv(t *testing.T) { - vm := goja.New() - injectUtilities(vm) - - os.Setenv("TEST_CONDUCTOR_VAR", "test_value") - defer os.Unsetenv("TEST_CONDUCTOR_VAR") - - val, err := vm.RunString(`util.env("TEST_CONDUCTOR_VAR")`) - if err != nil { - t.Fatalf("script error: %v", err) - } - if val.String() != "test_value" { - t.Errorf("got %q, want %q", val.String(), "test_value") - } - - // Non-existent env var - val, err = vm.RunString(`util.env("NONEXISTENT_VAR_12345")`) - if err != nil { - t.Fatalf("script error: %v", err) - } - if val.String() != "" { - t.Errorf("got %q, want empty string", val.String()) - } -} - -func TestInjectUtilitiesUUID(t *testing.T) { - vm := goja.New() - injectUtilities(vm) - - val, err := vm.RunString(`util.uuid()`) - if err != nil { - t.Fatalf("script error: %v", err) - } - if val.String() == "" { - t.Error("expected non-empty UUID") - } -} - -func TestInjectUtilitiesHTTP(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.Write([]byte(`{"method":"` + r.Method + `"}`)) - })) - defer server.Close() - - vm := goja.New() - injectUtilities(vm) - - // Test http.get - val, err := vm.RunString(`JSON.stringify(http.get("` + server.URL + `", {}))`) - if err != nil { - t.Fatalf("script error: %v", err) - } - result := val.String() - if result == "" { - t.Error("expected non-empty result") - } -} - func TestWorkerCodeResponseJSON(t *testing.T) { input := `{ "id": "wc-123", diff --git a/internal/skillworker/adapter.go b/internal/skillworker/adapter.go new file mode 100644 index 0000000..8cdce27 --- /dev/null +++ b/internal/skillworker/adapter.go @@ -0,0 +1,76 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package skillworker + +import ( + "context" + "encoding/json" + + "github.com/conductor-oss/conductor-cli/internal/taskworker" +) + +// WorkerID identifies the CLI in the task results it reports for skill tools. +const WorkerID = "conductor-cli" + +// PollTimeoutMs is the server-side long-poll wait used for skill tool tasks. +const PollTimeoutMs = 100 + +// RunnerOptions returns the poll settings for skill tool workers. Skill tools take one +// task at a time and always identify as the CLI, unlike the worker commands where the +// user chooses a worker id. +func RunnerOptions() taskworker.RunnerOptions { + return taskworker.RunnerOptions{ + WorkerID: WorkerID, + Count: 1, + PollTimeoutMs: PollTimeoutMs, + } +} + +// AsTaskHandler adapts a ToolHandler onto the shared worker loop. +// +// ToolHandler stays the vocabulary of this package — tool logic in, raw JSON out — and +// this adapter supplies what the loop needs around it: the task's inputData rather than +// the whole task, the {"result": …} envelope the skill agent expects, and the mapping +// from a handler error to a failed task. +func AsTaskHandler(h ToolHandler) taskworker.Handler { + return taskworker.HandlerFunc(func(ctx context.Context, t taskworker.Task) taskworker.Result { + input, err := t.InputData() + if err != nil { + return taskworker.Failure(err.Error()) + } + + output, err := h.Handle(ctx, input) + if err != nil { + return taskworker.Failure(err.Error()) + } + + return taskworker.Result{ + Status: taskworker.StatusCompleted, + Output: wrapResult(output), + } + }) +} + +// wrapResult wraps a handler's raw output under the "result" key the skill agent expects. +// Decoding to a generic value happens only here, at the seam. Output that is not valid +// JSON is carried through as a string rather than failing the task. +func wrapResult(output json.RawMessage) map[string]interface{} { + var v interface{} + if len(output) > 0 { + if err := json.Unmarshal(output, &v); err != nil { + v = string(output) + } + } + return map[string]interface{}{outputKeyResult: v} +} diff --git a/internal/skillworker/adapter_test.go b/internal/skillworker/adapter_test.go new file mode 100644 index 0000000..ddac5b5 --- /dev/null +++ b/internal/skillworker/adapter_test.go @@ -0,0 +1,166 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package skillworker + +import ( + "context" + "encoding/json" + "errors" + "testing" + + "github.com/conductor-oss/conductor-cli/internal/taskworker" +) + +// stubHandler returns a fixed output or error, standing in for a real tool. +type stubHandler struct { + output json.RawMessage + err error + gotIn json.RawMessage +} + +func (s *stubHandler) Handle(ctx context.Context, input json.RawMessage) (json.RawMessage, error) { + s.gotIn = input + return s.output, s.err +} + +func skillTask(raw string) taskworker.Task { + return taskworker.Task{ID: "t1", WorkflowID: "wf1", Raw: json.RawMessage(raw)} +} + +// TestAsTaskHandlerWrapsUnderResultKey pins the skill agent's output contract. It is the +// only contract of the three with no documentation outside the code, so this test is the +// specification. +func TestAsTaskHandlerWrapsUnderResultKey(t *testing.T) { + tests := []struct { + name string + output string + want interface{} + }{ + {name: "json string", output: `"Hello\n"`, want: "Hello\n"}, + {name: "json number", output: `42`, want: float64(42)}, + {name: "non-json carried through as a string", output: `not json at all`, want: "not json at all"}, + {name: "empty output becomes nil", output: ``, want: nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := AsTaskHandler(&stubHandler{output: json.RawMessage(tt.output)}) + got := h.Handle(context.Background(), skillTask(`{"inputData":{}}`)) + + if got.Status != taskworker.StatusCompleted { + t.Errorf("Status = %q, want COMPLETED", got.Status) + } + if len(got.Output) != 1 { + t.Fatalf("Output = %v, want exactly one key", got.Output) + } + if got.Output["result"] != tt.want { + t.Errorf("Output[result] = %#v, want %#v", got.Output["result"], tt.want) + } + }) + } +} + +// TestAsTaskHandlerNilOutputYieldsNullResult pins that the key is always present, holding +// null, rather than the output map being empty. +func TestAsTaskHandlerNilOutputYieldsNullResult(t *testing.T) { + h := AsTaskHandler(&stubHandler{output: nil}) + got := h.Handle(context.Background(), skillTask(`{"inputData":{}}`)) + + value, present := got.Output["result"] + if !present { + t.Fatal(`Output has no "result" key; it must be present even when the output is empty`) + } + if value != nil { + t.Errorf("Output[result] = %#v, want nil", value) + } +} + +// TestAsTaskHandlerObjectOutputIsDecoded checks that structured output survives as +// structure rather than being stringified. +func TestAsTaskHandlerObjectOutputIsDecoded(t *testing.T) { + h := AsTaskHandler(&stubHandler{output: json.RawMessage(`{"files":["a.go"],"count":1}`)}) + got := h.Handle(context.Background(), skillTask(`{"inputData":{}}`)) + + inner, ok := got.Output["result"].(map[string]interface{}) + if !ok { + t.Fatalf("Output[result] = %#v, want a decoded object", got.Output["result"]) + } + if inner["count"] != float64(1) { + t.Errorf("result.count = %#v, want 1", inner["count"]) + } +} + +func TestAsTaskHandlerErrorFailsTheTask(t *testing.T) { + h := AsTaskHandler(&stubHandler{err: errors.New("tool exploded")}) + got := h.Handle(context.Background(), skillTask(`{"inputData":{}}`)) + + if got.Status != taskworker.StatusFailed { + t.Errorf("Status = %q, want FAILED", got.Status) + } + if got.Reason != "tool exploded" { + t.Errorf("Reason = %q, want the handler error", got.Reason) + } + if got.Output != nil { + t.Errorf("Output = %v, want nil for a failure", got.Output) + } +} + +// TestAsTaskHandlerPassesOnlyInputData pins that a tool receives inputData rather than the +// whole task, which is how the tool handlers have always been written. +func TestAsTaskHandlerPassesOnlyInputData(t *testing.T) { + stub := &stubHandler{output: json.RawMessage(`"ok"`)} + h := AsTaskHandler(stub) + + h.Handle(context.Background(), skillTask(`{"taskId":"t1","inputData":{"command":"greet"}}`)) + + if string(stub.gotIn) != `{"command":"greet"}` { + t.Errorf("handler received %s, want only inputData", stub.gotIn) + } +} + +func TestAsTaskHandlerMalformedTaskFails(t *testing.T) { + h := AsTaskHandler(&stubHandler{output: json.RawMessage(`"ok"`)}) + got := h.Handle(context.Background(), skillTask(`not json`)) + + if got.Status != taskworker.StatusFailed { + t.Errorf("Status = %q, want FAILED", got.Status) + } +} + +// TestRunnerOptionsMatchSkillContract pins the poll settings the skill agent expects: one +// task at a time, identifying as the CLI. +func TestRunnerOptionsMatchSkillContract(t *testing.T) { + opts := RunnerOptions() + + // Asserted against the literal too: the constant is the skill agent's wire contract, + // so renaming its value is a breaking change rather than a rename. + if opts.WorkerID != "conductor-cli" { + t.Errorf("WorkerID = %q, want conductor-cli", opts.WorkerID) + } + if opts.Count != 1 { + t.Errorf("Count = %d, want 1", opts.Count) + } + if opts.PollTimeoutMs != PollTimeoutMs { + t.Errorf("PollTimeoutMs = %d, want %d", opts.PollTimeoutMs, PollTimeoutMs) + } + if opts.UseTaskWorkerID { + t.Error("UseTaskWorkerID = true, want false — skill tools always report as conductor-cli") + } +} + +func TestTaskType(t *testing.T) { + if got := TaskType("greetskill", "greet"); got != "greetskill__greet" { + t.Errorf("TaskType() = %q, want greetskill__greet", got) + } +} diff --git a/internal/skillworker/runner.go b/internal/skillworker/runner.go deleted file mode 100644 index 6ae99b8..0000000 --- a/internal/skillworker/runner.go +++ /dev/null @@ -1,105 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skillworker - -import ( - "context" - "encoding/json" - "fmt" - - "github.com/antihax/optional" - "github.com/conductor-sdk/conductor-go/sdk/client" - "github.com/conductor-sdk/conductor-go/sdk/model" -) - -// Poll tuning: one task per poll, with a short server-side long-poll wait. The -// worker loop adds its own backoff on top (see pollBackoff). -const ( - pollBatchSize = 1 // tasks requested per poll - pollTimeoutMs = 100 // server-side long-poll wait, milliseconds -) - -// conductorRunner adapts Conductor's TaskResourceApiService to TaskRunner. It is the -// ONLY place model.* and *client.TaskResourceApiService appear in this package — -// the worker loop and the handlers see only skillworker.Task and json.RawMessage. -type conductorRunner struct { - client *client.TaskResourceApiService -} - -// NewConductorRunner returns a TaskRunner backed by the Conductor task client -// (supplied by the cmd layer via internal.GetTaskClient()). -func NewConductorRunner(taskClient *client.TaskResourceApiService) TaskRunner { - return &conductorRunner{client: taskClient} -} - -func (r *conductorRunner) Poll(ctx context.Context, taskType string) (Task, bool, error) { - opts := &client.TaskResourceApiBatchPollOpts{ - Workerid: optional.NewString(workerID), - Count: optional.NewInt32(pollBatchSize), - Timeout: optional.NewInt32(pollTimeoutMs), - } - tasks, _, err := r.client.BatchPoll(ctx, taskType, opts) - if err != nil { - return Task{}, false, err - } - if len(tasks) == 0 { - return Task{}, false, nil - } - return taskFromModel(tasks[0]) -} - -func (r *conductorRunner) Complete(ctx context.Context, t Task, output json.RawMessage) error { - return r.update(ctx, t, model.CompletedTask, wrapResult(output), "") -} - -func (r *conductorRunner) Fail(ctx context.Context, t Task, reason string) error { - return r.update(ctx, t, model.FailedTask, nil, reason) -} - -func (r *conductorRunner) update(ctx context.Context, t Task, status model.TaskResultStatus, output map[string]interface{}, reason string) error { - result := &model.TaskResult{ - TaskId: t.ID, - WorkflowInstanceId: t.WorkflowID, - WorkerId: workerID, - Status: status, - OutputData: output, - } - if reason != "" { - result.ReasonForIncompletion = reason - } - _, _, err := r.client.UpdateTask(ctx, result) - return err -} - -// taskFromModel marshals the SDK task's input map to bytes at the seam, so the -// map[string]interface{} never crosses into the worker loop or the handlers. -func taskFromModel(t model.Task) (Task, bool, error) { - input, err := json.Marshal(t.InputData) - if err != nil { - return Task{}, false, fmt.Errorf("marshal task input: %w", err) - } - return Task{ID: t.TaskId, WorkflowID: t.WorkflowInstanceId, Input: input}, true, nil -} - -// wrapResult wraps a handler's raw output under the "result" key the skill agent -// expects. Decoding to a generic value happens only here, at the seam. -func wrapResult(output json.RawMessage) map[string]interface{} { - var v interface{} - if len(output) > 0 { - if err := json.Unmarshal(output, &v); err != nil { - v = string(output) - } - } - return map[string]interface{}{outputKeyResult: v} -} diff --git a/internal/skillworker/runner_test.go b/internal/skillworker/runner_test.go deleted file mode 100644 index dfeecc8..0000000 --- a/internal/skillworker/runner_test.go +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skillworker - -import ( - "encoding/json" - "testing" - - "github.com/conductor-sdk/conductor-go/sdk/model" -) - -// TestTaskFromModelMarshalsInput verifies the bridge maps the SDK task's InputData -// map to json.RawMessage at the seam (no map crosses into the loop/handlers). -func TestTaskFromModelMarshalsInput(t *testing.T) { - mt := model.Task{ - TaskId: "t1", - WorkflowInstanceId: "w1", - InputData: map[string]interface{}{"path": "notes.md"}, - } - task, ok, err := taskFromModel(mt) - if err != nil || !ok { - t.Fatalf("taskFromModel: ok=%v err=%v", ok, err) - } - if task.ID != "t1" || task.WorkflowID != "w1" { - t.Errorf("task ids = %+v", task) - } - var decoded map[string]string - if err := json.Unmarshal(task.Input, &decoded); err != nil { - t.Fatalf("input not valid JSON: %v", err) - } - if decoded["path"] != "notes.md" { - t.Errorf("input = %s", task.Input) - } -} - -// TestWrapResultWrapsUnderResultKey checks the output wrapping the skill agent -// expects, for both an object and a bare-string handler output. -func TestWrapResultWrapsUnderResultKey(t *testing.T) { - obj := wrapResult(json.RawMessage(`{"files":["a","b"]}`)) - inner, ok := obj[outputKeyResult].(map[string]interface{}) - if !ok { - t.Fatalf("result not an object: %#v", obj) - } - if _, ok := inner["files"]; !ok { - t.Errorf("wrapped object lost its fields: %#v", inner) - } - - str := wrapResult(json.RawMessage(`"hello"`)) - if str[outputKeyResult] != "hello" { - t.Errorf("string result = %#v", str[outputKeyResult]) - } - - // Empty output still produces the result key (nil value). - empty := wrapResult(nil) - if _, ok := empty[outputKeyResult]; !ok { - t.Errorf("empty output missing result key: %#v", empty) - } -} diff --git a/internal/skillworker/worker.go b/internal/skillworker/worker.go index f5da002..8b3136f 100644 --- a/internal/skillworker/worker.go +++ b/internal/skillworker/worker.go @@ -13,30 +13,26 @@ // Package skillworker is the local tool-worker runtime for skill run/serve. When a // skill agent runs on the server, it dispatches tool tasks (read_skill_file, each -// script, workspace tools) back to the CLI; this package polls for those tasks, -// runs them locally, and returns the result. It is layered: the poll→handle→update -// loop and its two interfaces live here, the Conductor SDK is confined to the -// runner bridge, and the concrete tool logic lives in the handlers (later stage). +// script, workspace tools) back to the CLI; the tools run locally and their results are +// returned. +// +// The poll→handle→update loop itself lives in internal/taskworker, shared with the +// worker commands. This package owns the tool logic (handlers.go) and the adapter that +// puts it on that loop (adapter.go). package skillworker import ( "context" "encoding/json" - "time" ) // Worker protocol constants — fixed by the skill agent/server contract, so they are // named constants, never inline literals. const ( - taskTypeSep = "__" // task type is "{skillName}__{tool}" - outputKeyResult = "result" // handler output is wrapped as {result: } - workerID = "conductor-cli" // identifies this worker in task results + taskTypeSep = "__" // task type is "{skillName}__{tool}" + outputKeyResult = "result" // handler output is wrapped as {result: } ) -// pollBackoff is the idle wait between polls that return no task or an error. The -// production runner also long-polls the server, so this is a hot-loop backstop. -const pollBackoff = 100 * time.Millisecond - // TaskType builds the "{skillName}__{tool}" task type dispatched for a skill tool. func TaskType(skillName, tool string) string { return skillName + taskTypeSep + tool @@ -47,78 +43,3 @@ func TaskType(skillName, tool string) string { type ToolHandler interface { Handle(ctx context.Context, input json.RawMessage) (json.RawMessage, error) } - -// Task is one polled tool task, decoupled from the SDK's model.Task. -type Task struct { - ID string - WorkflowID string - Input json.RawMessage -} - -// TaskRunner is the poll/complete/fail seam. The production impl (runner.go) wraps -// Conductor's TaskResourceApiService; tests inject a fake. It keeps model.Task and -// *client.TaskResourceApiService out of the worker loop and the handlers. -type TaskRunner interface { - // Poll returns the next task for taskType. ok=false means no task was available - // (poll again); a non-nil err is a real polling failure. - Poll(ctx context.Context, taskType string) (task Task, ok bool, err error) - Complete(ctx context.Context, t Task, output json.RawMessage) error - Fail(ctx context.Context, t Task, reason string) error -} - -// Worker runs the poll→handle→update loop for a single task type over a TaskRunner. -type Worker struct { - runner TaskRunner -} - -// NewWorker returns a Worker backed by the given TaskRunner. -func NewWorker(runner TaskRunner) *Worker { - return &Worker{runner: runner} -} - -// Run polls taskType and dispatches each task to h until ctx is cancelled. Transient -// poll failures back off and retry rather than stop the loop; a handler error fails -// only that task. Run returns when ctx is done. -func (w *Worker) Run(ctx context.Context, taskType string, h ToolHandler) { - for { - select { - case <-ctx.Done(): - return - default: - } - - task, ok, err := w.runner.Poll(ctx, taskType) - if err != nil { - if !sleep(ctx, pollBackoff) { - return - } - continue - } - if !ok { - if !sleep(ctx, pollBackoff) { - return - } - continue - } - - output, handleErr := h.Handle(ctx, task.Input) - if handleErr != nil { - _ = w.runner.Fail(ctx, task, handleErr.Error()) - continue - } - _ = w.runner.Complete(ctx, task, output) - } -} - -// sleep waits d or until ctx is cancelled; it returns false if ctx was cancelled, -// which keeps the poll loop responsive to Ctrl-C during idle waits. -func sleep(ctx context.Context, d time.Duration) bool { - t := time.NewTimer(d) - defer t.Stop() - select { - case <-ctx.Done(): - return false - case <-t.C: - return true - } -} diff --git a/internal/skillworker/worker_test.go b/internal/skillworker/worker_test.go deleted file mode 100644 index 6b52d8a..0000000 --- a/internal/skillworker/worker_test.go +++ /dev/null @@ -1,154 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skillworker - -import ( - "context" - "encoding/json" - "errors" - "sync" - "testing" - "time" -) - -// handlerFunc adapts a function to ToolHandler. -type handlerFunc func(ctx context.Context, input json.RawMessage) (json.RawMessage, error) - -func (f handlerFunc) Handle(ctx context.Context, input json.RawMessage) (json.RawMessage, error) { - return f(ctx, input) -} - -// fakeRunner is a TaskRunner that returns queued tasks then reports empty polls. It -// records terminal updates and can cancel the loop once the queue is drained. -type fakeRunner struct { - mu sync.Mutex - queue []Task - pollCount int - completed []completedCall - failed []failedCall - stopAfterOne context.CancelFunc // cancel the loop after the first terminal update -} - -type completedCall struct { - task Task - output json.RawMessage -} - -type failedCall struct { - task Task - reason string -} - -func (r *fakeRunner) Poll(ctx context.Context, taskType string) (Task, bool, error) { - r.mu.Lock() - defer r.mu.Unlock() - r.pollCount++ - if len(r.queue) == 0 { - return Task{}, false, nil - } - t := r.queue[0] - r.queue = r.queue[1:] - return t, true, nil -} - -func (r *fakeRunner) Complete(ctx context.Context, t Task, output json.RawMessage) error { - r.mu.Lock() - r.completed = append(r.completed, completedCall{task: t, output: output}) - r.mu.Unlock() - if r.stopAfterOne != nil { - r.stopAfterOne() - } - return nil -} - -func (r *fakeRunner) Fail(ctx context.Context, t Task, reason string) error { - r.mu.Lock() - r.failed = append(r.failed, failedCall{task: t, reason: reason}) - r.mu.Unlock() - if r.stopAfterOne != nil { - r.stopAfterOne() - } - return nil -} - -func TestWorkerRunCompletesTask(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - fr := &fakeRunner{ - queue: []Task{{ID: "t1", WorkflowID: "w1", Input: json.RawMessage(`{"path":"a"}`)}}, - stopAfterOne: cancel, - } - var gotInput json.RawMessage - h := handlerFunc(func(_ context.Context, in json.RawMessage) (json.RawMessage, error) { - gotInput = in - return json.RawMessage(`"file body"`), nil - }) - - NewWorker(fr).Run(ctx, "demo__read_skill_file", h) - - if string(gotInput) != `{"path":"a"}` { - t.Errorf("handler input = %s", gotInput) - } - if len(fr.completed) != 1 || len(fr.failed) != 0 { - t.Fatalf("completed=%d failed=%d", len(fr.completed), len(fr.failed)) - } - got := fr.completed[0] - if got.task.ID != "t1" || string(got.output) != `"file body"` { - t.Errorf("completed call = %+v", got) - } -} - -func TestWorkerRunFailsTaskOnHandlerError(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - fr := &fakeRunner{ - queue: []Task{{ID: "t2", WorkflowID: "w2"}}, - stopAfterOne: cancel, - } - h := handlerFunc(func(context.Context, json.RawMessage) (json.RawMessage, error) { - return nil, errors.New("boom") - }) - - NewWorker(fr).Run(ctx, "demo__script", h) - - if len(fr.failed) != 1 || len(fr.completed) != 0 { - t.Fatalf("completed=%d failed=%d", len(fr.completed), len(fr.failed)) - } - if fr.failed[0].reason != "boom" || fr.failed[0].task.ID != "t2" { - t.Errorf("fail call = %+v", fr.failed[0]) - } -} - -func TestWorkerRunStopsOnContextCancel(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - fr := &fakeRunner{} // always reports empty polls - done := make(chan struct{}) - go func() { - NewWorker(fr).Run(ctx, "demo__x", handlerFunc(func(context.Context, json.RawMessage) (json.RawMessage, error) { - return nil, nil - })) - close(done) - }() - - cancel() - select { - case <-done: - case <-time.After(2 * time.Second): - t.Fatal("Run did not stop after context cancel") - } -} - -func TestTaskType(t *testing.T) { - if got := TaskType("demo", "read_skill_file"); got != "demo__read_skill_file" { - t.Errorf("TaskType = %q", got) - } -} diff --git a/internal/taskworker/goja.go b/internal/taskworker/goja.go new file mode 100644 index 0000000..3e2f8c5 --- /dev/null +++ b/internal/taskworker/goja.go @@ -0,0 +1,255 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package taskworker + +import ( + "context" + "crypto/md5" + "crypto/sha1" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "strings" + "time" + + "github.com/dop251/goja" + log "github.com/sirupsen/logrus" +) + +// gojaResult is the JSON contract a JavaScript worker returns from its script. +type gojaResult struct { + Status string `json:"status"` + Body map[string]interface{} `json:"body"` +} + +// GojaHandler runs a JavaScript worker in the CLI's embedded interpreter. +// +// The program is compiled once and each task gets a fresh goja.Runtime: Runtimes are not +// safe for concurrent use, and a Handler is shared across the goroutines of a batch poll. +type GojaHandler struct { + program *goja.Program +} + +// NewGojaHandler compiles script for repeated execution. name appears in stack traces. +func NewGojaHandler(script, name string) (*GojaHandler, error) { + program, err := goja.Compile(name, script, false) + if err != nil { + return nil, fmt.Errorf("error compiling JavaScript worker: %w", err) + } + return &GojaHandler{program: program}, nil +} + +func (h *GojaHandler) Handle(ctx context.Context, t Task) Result { + log.Infof("Processing task: %s (workflow: %s)", t.ID, t.WorkflowID) + + vm := goja.New() + + var taskObj interface{} + if err := json.Unmarshal(t.Raw, &taskObj); err != nil { + log.Errorf("Error unmarshaling task: %v", err) + return gojaFailure(fmt.Sprintf("Error unmarshaling task: %v", err)) + } + + dollarObj := vm.NewObject() + if err := dollarObj.Set("task", taskObj); err != nil { + log.Errorf("Error setting task in $: %v", err) + return gojaFailure(fmt.Sprintf("Error setting task: %v", err)) + } + if err := vm.Set("$", dollarObj); err != nil { + log.Errorf("Error setting $ object: %v", err) + return gojaFailure(fmt.Sprintf("Error setting $ object: %v", err)) + } + + injectUtilities(vm) + + value, err := vm.RunProgram(h.program) + if err != nil { + log.Errorf("Error executing script for task %s: %v", t.ID, err) + return gojaFailure(fmt.Sprintf("Script execution error: %v", err)) + } + + return gojaResultToResult(value) +} + +// gojaFailure builds the failure shape JavaScript workers have always produced: the +// message lands under an "error" output key rather than in ReasonForIncompletion, which +// workflows may read as ${task.output.error}. +func gojaFailure(reason string) Result { + return Result{Status: StatusFailed, Output: map[string]interface{}{"error": reason}} +} + +// gojaResultToResult interprets whatever the script returned. +// +// A script may return {status, body}, or any other value, or nothing at all; each case +// has an established meaning that workflows depend on. Note that the status is passed +// through verbatim — unlike stdio workers, JavaScript workers may return statuses the +// CLI does not model, such as FAILED_WITH_TERMINAL_ERROR. +func gojaResultToResult(value goja.Value) Result { + if value == nil || goja.IsUndefined(value) || goja.IsNull(value) { + return Result{Status: StatusCompleted, Output: map[string]interface{}{}} + } + + exported := value.Export() + resultBytes, err := json.Marshal(exported) + if err != nil { + log.Errorf("Error marshaling script result: %v", err) + return Result{Status: StatusCompleted, Output: map[string]interface{}{}} + } + + var parsed gojaResult + if err := json.Unmarshal(resultBytes, &parsed); err != nil || parsed.Status == "" { + log.Warnf("Script result not in expected format, treating as completed") + return Result{Status: StatusCompleted, Output: map[string]interface{}{"result": exported}} + } + + body := parsed.Body + if body == nil { + body = make(map[string]interface{}) + } + return Result{Status: Status(parsed.Status), Output: body} +} + +func injectUtilities(vm *goja.Runtime) { + // HTTP utilities + httpObj := vm.NewObject() + httpObj.Set("get", func(url string, headers map[string]interface{}) map[string]interface{} { + return httpRequest("GET", url, headers, "") + }) + httpObj.Set("post", func(url string, headers map[string]interface{}, body string) map[string]interface{} { + return httpRequest("POST", url, headers, body) + }) + httpObj.Set("put", func(url string, headers map[string]interface{}, body string) map[string]interface{} { + return httpRequest("PUT", url, headers, body) + }) + httpObj.Set("delete", func(url string, headers map[string]interface{}) map[string]interface{} { + return httpRequest("DELETE", url, headers, "") + }) + vm.Set("http", httpObj) + + // Crypto utilities + cryptoObj := vm.NewObject() + cryptoObj.Set("md5", func(text string) string { + hash := md5.Sum([]byte(text)) + return hex.EncodeToString(hash[:]) + }) + cryptoObj.Set("sha1", func(text string) string { + hash := sha1.Sum([]byte(text)) + return hex.EncodeToString(hash[:]) + }) + cryptoObj.Set("sha256", func(text string) string { + hash := sha256.Sum256([]byte(text)) + return hex.EncodeToString(hash[:]) + }) + cryptoObj.Set("base64Encode", func(text string) string { + return base64.StdEncoding.EncodeToString([]byte(text)) + }) + cryptoObj.Set("base64Decode", func(text string) string { + decoded, err := base64.StdEncoding.DecodeString(text) + if err != nil { + return "" + } + return string(decoded) + }) + vm.Set("crypto", cryptoObj) + + // Utility functions + utilObj := vm.NewObject() + utilObj.Set("sleep", func(ms int) { + time.Sleep(time.Duration(ms) * time.Millisecond) + }) + utilObj.Set("uuid", func() string { + return fmt.Sprintf("%d-%d", time.Now().UnixNano(), os.Getpid()) + }) + utilObj.Set("env", func(key string) string { + return os.Getenv(key) + }) + vm.Set("util", utilObj) + + // String utilities + stringObj := vm.NewObject() + stringObj.Set("toUpper", strings.ToUpper) + stringObj.Set("toLower", strings.ToLower) + stringObj.Set("trim", strings.TrimSpace) + stringObj.Set("split", func(s, sep string) []string { + return strings.Split(s, sep) + }) + stringObj.Set("join", func(arr []string, sep string) string { + return strings.Join(arr, sep) + }) + stringObj.Set("replace", func(s, old, new string) string { + return strings.ReplaceAll(s, old, new) + }) + stringObj.Set("contains", strings.Contains) + stringObj.Set("hasPrefix", strings.HasPrefix) + stringObj.Set("hasSuffix", strings.HasSuffix) + vm.Set("str", stringObj) +} + +func httpRequest(method, url string, headers map[string]interface{}, body string) map[string]interface{} { + var bodyReader io.Reader + if body != "" { + bodyReader = strings.NewReader(body) + } + + req, err := http.NewRequest(method, url, bodyReader) + if err != nil { + return map[string]interface{}{ + "error": err.Error(), + "status": 0, + } + } + + for key, value := range headers { + if strVal, ok := value.(string); ok { + req.Header.Set(key, strVal) + } + } + + client := &http.Client{Timeout: 30 * time.Second} + resp, err := client.Do(req) + if err != nil { + return map[string]interface{}{ + "error": err.Error(), + "status": 0, + } + } + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return map[string]interface{}{ + "error": err.Error(), + "status": resp.StatusCode, + } + } + + var jsonBody interface{} + if err := json.Unmarshal(respBody, &jsonBody); err == nil { + return map[string]interface{}{ + "status": resp.StatusCode, + "body": jsonBody, + "text": string(respBody), + } + } + + return map[string]interface{}{ + "status": resp.StatusCode, + "text": string(respBody), + } +} diff --git a/internal/taskworker/goja_handler_test.go b/internal/taskworker/goja_handler_test.go new file mode 100644 index 0000000..778c66e --- /dev/null +++ b/internal/taskworker/goja_handler_test.go @@ -0,0 +1,232 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package taskworker + +import ( + "context" + "encoding/json" + "strings" + "sync" + "testing" +) + +func gojaTask() Task { + return Task{ + ID: "task-1", + WorkflowID: "wf-1", + Type: "greet", + WorkerID: "w-1", + Raw: json.RawMessage(`{"taskId":"task-1","taskType":"greet","inputData":{"name":"Miguel"},"pollCount":1}`), + } +} + +func handleScript(t *testing.T, script string) Result { + t.Helper() + h, err := NewGojaHandler(script, "test.js") + if err != nil { + t.Fatalf("NewGojaHandler() error = %v", err) + } + return h.Handle(context.Background(), gojaTask()) +} + +func TestGojaHandlerStatusAndBodyMapping(t *testing.T) { + got := handleScript(t, `(function () { + return { status: "COMPLETED", body: { message: "hi " + $.task.inputData.name } }; + })();`) + + if got.Status != StatusCompleted { + t.Errorf("Status = %q, want COMPLETED", got.Status) + } + if got.Output["message"] != "hi Miguel" { + t.Errorf("Output = %v, want message from $.task.inputData", got.Output) + } +} + +// TestGojaHandlerExposesWholeTask pins that $.task is the entire task, not just its +// input: WORKER_JS.md documents fields like pollCount and taskType on it. +func TestGojaHandlerExposesWholeTask(t *testing.T) { + got := handleScript(t, `(function () { + return { status: "COMPLETED", body: { type: $.task.taskType, polls: $.task.pollCount } }; + })();`) + + if got.Output["type"] != "greet" { + t.Errorf("$.task.taskType = %v, want greet", got.Output["type"]) + } + if got.Output["polls"] == nil { + t.Error("$.task.pollCount missing — $.task must be the whole task") + } +} + +// TestGojaHandlerPassesUnknownStatusThrough is the behaviour that forced Status to be an +// open string type: FAILED_WITH_TERMINAL_ERROR is documented, and the CLI must not +// second-guess statuses the server understands. +func TestGojaHandlerPassesUnknownStatusThrough(t *testing.T) { + for _, status := range []string{"FAILED_WITH_TERMINAL_ERROR", "IN_PROGRESS", "SOMETHING_NEW"} { + t.Run(status, func(t *testing.T) { + got := handleScript(t, `(function () { return { status: "`+status+`", body: {} }; })();`) + if string(got.Status) != status { + t.Errorf("Status = %q, want %q passed through unchanged", got.Status, status) + } + }) + } +} + +// TestGojaHandlerScriptErrorUsesErrorOutputKey pins the JavaScript failure shape. It +// differs from the stdio one, and a workflow reading ${task.output.error} depends on it. +func TestGojaHandlerScriptErrorUsesErrorOutputKey(t *testing.T) { + got := handleScript(t, `(function () { throw new Error("boom"); })();`) + + if got.Status != StatusFailed { + t.Errorf("Status = %q, want FAILED", got.Status) + } + msg, ok := got.Output["error"].(string) + if !ok { + t.Fatalf(`Output["error"] = %#v, want a string — js failures report under "error"`, got.Output["error"]) + } + if !strings.Contains(msg, "boom") { + t.Errorf(`Output["error"] = %q, want it to contain the script error`, msg) + } + if got.Reason != "" { + t.Errorf("Reason = %q, want empty — js workers do not set ReasonForIncompletion", got.Reason) + } +} + +func TestGojaHandlerCompileErrorIsReportedAtConstruction(t *testing.T) { + if _, err := NewGojaHandler(`function ( {{{ bad syntax`, "bad.js"); err == nil { + t.Error("NewGojaHandler() on invalid JavaScript returned nil error") + } +} + +func TestGojaHandlerResultShapes(t *testing.T) { + tests := []struct { + name string + script string + wantStatus Status + check func(*testing.T, Result) + }{ + { + name: "no return value completes with empty output", + script: `(function () { var x = 1; })();`, + wantStatus: StatusCompleted, + check: func(t *testing.T, got Result) { + if len(got.Output) != 0 { + t.Errorf("Output = %v, want empty", got.Output) + } + }, + }, + { + name: "null completes with empty output", + script: `null;`, + wantStatus: StatusCompleted, + check: func(t *testing.T, got Result) { + if len(got.Output) != 0 { + t.Errorf("Output = %v, want empty", got.Output) + } + }, + }, + { + name: "object without a status completes and carries the value", + script: `({ greeting: "hello" });`, + wantStatus: StatusCompleted, + check: func(t *testing.T, got Result) { + if got.Output["result"] == nil { + t.Errorf(`Output = %v, want the value under "result"`, got.Output) + } + }, + }, + { + name: "bare string completes and carries the value", + script: `"just a string";`, + wantStatus: StatusCompleted, + check: func(t *testing.T, got Result) { + if got.Output["result"] != "just a string" { + t.Errorf(`Output["result"] = %v, want the string`, got.Output["result"]) + } + }, + }, + { + name: "status with no body yields an empty output map", + script: `({ status: "COMPLETED" });`, + wantStatus: StatusCompleted, + check: func(t *testing.T, got Result) { + if got.Output == nil { + t.Error("Output = nil, want an empty map") + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := handleScript(t, tt.script) + if got.Status != tt.wantStatus { + t.Errorf("Status = %q, want %q", got.Status, tt.wantStatus) + } + tt.check(t, got) + }) + } +} + +// TestGojaHandlerConcurrentUse guards the Handler concurrency contract. The compiled +// program is shared, so each task must get its own Runtime — goja Runtimes are not safe +// for concurrent use, and a batch poll runs tasks in parallel. +func TestGojaHandlerConcurrentUse(t *testing.T) { + h, err := NewGojaHandler(`(function () { + return { status: "COMPLETED", body: { id: $.task.taskId } }; + })();`, "test.js") + if err != nil { + t.Fatalf("NewGojaHandler() error = %v", err) + } + + const n = 16 + var wg sync.WaitGroup + results := make([]Result, n) + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + id := string(rune('a' + i)) + results[i] = h.Handle(context.Background(), Task{ + ID: id, + Raw: json.RawMessage(`{"taskId":"` + id + `"}`), + }) + }(i) + } + wg.Wait() + + for i, got := range results { + want := string(rune('a' + i)) + if got.Status != StatusCompleted { + t.Errorf("result %d status = %q, want COMPLETED", i, got.Status) + } + if got.Output["id"] != want { + t.Errorf("result %d id = %v, want %q — concurrent Handle calls shared a Runtime", i, got.Output["id"], want) + } + } +} + +func TestGojaHandlerMalformedRawFails(t *testing.T) { + h, err := NewGojaHandler(`({ status: "COMPLETED" });`, "test.js") + if err != nil { + t.Fatalf("NewGojaHandler() error = %v", err) + } + + got := h.Handle(context.Background(), Task{ID: "t", Raw: json.RawMessage(`not json`)}) + if got.Status != StatusFailed { + t.Errorf("Status = %q, want FAILED", got.Status) + } + if got.Output["error"] == nil { + t.Error(`Output["error"] missing for an unparseable task`) + } +} diff --git a/internal/taskworker/goja_test.go b/internal/taskworker/goja_test.go new file mode 100644 index 0000000..e14edd0 --- /dev/null +++ b/internal/taskworker/goja_test.go @@ -0,0 +1,312 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +// Tests for the JavaScript worker runtime and the two on-the-wire result shapes. These +// moved here with injectUtilities and httpRequest when the worker flavours converged onto +// one poll loop; they previously lived in cmd/worker_test.go. +package taskworker + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/dop251/goja" +) + +func TestStdioResultJSON(t *testing.T) { + tests := []struct { + name string + input string + status string + hasOut bool + }{ + { + name: "completed with output", + input: `{"status":"COMPLETED","output":{"key":"value"},"logs":["done"]}`, + status: "COMPLETED", + hasOut: true, + }, + { + name: "failed with reason", + input: `{"status":"FAILED","reason":"timeout"}`, + status: "FAILED", + hasOut: false, + }, + { + name: "in progress", + input: `{"status":"IN_PROGRESS"}`, + status: "IN_PROGRESS", + hasOut: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var result stdioResult + err := json.Unmarshal([]byte(tt.input), &result) + if err != nil { + t.Fatalf("unmarshal failed: %v", err) + } + if result.Status != tt.status { + t.Errorf("Status: got %q, want %q", result.Status, tt.status) + } + if tt.hasOut && result.Output == nil { + t.Error("expected non-nil output") + } + }) + } +} + +func TestGojaResultJSON(t *testing.T) { + result := gojaResult{ + Status: "COMPLETED", + Body: map[string]interface{}{ + "message": "hello", + }, + } + + data, err := json.Marshal(result) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + + var decoded gojaResult + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("unmarshal failed: %v", err) + } + + if decoded.Status != "COMPLETED" { + t.Errorf("Status: got %q, want %q", decoded.Status, "COMPLETED") + } + if decoded.Body["message"] != "hello" { + t.Errorf("Body.message: got %v, want %q", decoded.Body["message"], "hello") + } +} + +func TestHttpRequest(t *testing.T) { + t.Run("GET request", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Errorf("expected GET, got %s", r.Method) + } + if r.Header.Get("X-Custom") != "test" { + t.Errorf("missing custom header") + } + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"result":"ok"}`)) + })) + defer server.Close() + + result := httpRequest("GET", server.URL, map[string]interface{}{"X-Custom": "test"}, "") + if result["status"] != http.StatusOK { + t.Errorf("status: got %v, want %d", result["status"], http.StatusOK) + } + body, ok := result["body"].(map[string]interface{}) + if !ok { + t.Fatal("expected body to be a map") + } + if body["result"] != "ok" { + t.Errorf("body.result: got %v, want %q", body["result"], "ok") + } + }) + + t.Run("POST request with body", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" { + t.Errorf("expected POST, got %s", r.Method) + } + w.Write([]byte(`{"created":true}`)) + })) + defer server.Close() + + result := httpRequest("POST", server.URL, nil, `{"name":"test"}`) + if result["status"] != http.StatusOK { + t.Errorf("status: got %v, want %d", result["status"], http.StatusOK) + } + }) + + t.Run("non-JSON response", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("plain text response")) + })) + defer server.Close() + + result := httpRequest("GET", server.URL, nil, "") + if result["text"] != "plain text response" { + t.Errorf("text: got %v, want %q", result["text"], "plain text response") + } + if result["body"] != nil { + t.Errorf("expected nil body for non-JSON, got %v", result["body"]) + } + }) + + t.Run("connection error", func(t *testing.T) { + result := httpRequest("GET", "http://localhost:1", nil, "") + if result["error"] == nil { + t.Error("expected error for connection failure") + } + if result["status"] != 0 { + t.Errorf("status: got %v, want 0", result["status"]) + } + }) +} + +func TestInjectUtilitiesCrypto(t *testing.T) { + vm := goja.New() + injectUtilities(vm) + + tests := []struct { + name string + script string + want string + }{ + {"md5", `crypto.md5("hello")`, "5d41402abc4b2a76b9719d911017c592"}, + {"sha1", `crypto.sha1("hello")`, "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d"}, + {"sha256", `crypto.sha256("hello")`, "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"}, + {"base64Encode", `crypto.base64Encode("hello world")`, "aGVsbG8gd29ybGQ="}, + {"base64Decode", `crypto.base64Decode("aGVsbG8gd29ybGQ=")`, "hello world"}, + {"base64Decode invalid", `crypto.base64Decode("!!!invalid!!!")`, ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + val, err := vm.RunString(tt.script) + if err != nil { + t.Fatalf("script error: %v", err) + } + if val.String() != tt.want { + t.Errorf("got %q, want %q", val.String(), tt.want) + } + }) + } +} + +func TestInjectUtilitiesString(t *testing.T) { + vm := goja.New() + injectUtilities(vm) + + tests := []struct { + name string + script string + want string + }{ + {"toUpper", `str.toUpper("hello")`, "HELLO"}, + {"toLower", `str.toLower("WORLD")`, "world"}, + {"trim", `str.trim(" spaces ")`, "spaces"}, + {"contains true", `str.contains("hello world", "world")`, "true"}, + {"contains false", `str.contains("hello", "xyz")`, "false"}, + {"hasPrefix", `str.hasPrefix("hello", "hel")`, "true"}, + {"hasSuffix", `str.hasSuffix("hello", "llo")`, "true"}, + {"replace", `str.replace("foo bar foo", "foo", "baz")`, "baz bar baz"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + val, err := vm.RunString(tt.script) + if err != nil { + t.Fatalf("script error: %v", err) + } + if val.String() != tt.want { + t.Errorf("got %q, want %q", val.String(), tt.want) + } + }) + } +} + +func TestInjectUtilitiesSplit(t *testing.T) { + vm := goja.New() + injectUtilities(vm) + + val, err := vm.RunString(`JSON.stringify(str.split("a,b,c", ","))`) + if err != nil { + t.Fatalf("script error: %v", err) + } + if val.String() != `["a","b","c"]` { + t.Errorf("got %q, want %q", val.String(), `["a","b","c"]`) + } +} + +func TestInjectUtilitiesJoin(t *testing.T) { + vm := goja.New() + injectUtilities(vm) + + val, err := vm.RunString(`str.join(["a","b","c"], "-")`) + if err != nil { + t.Fatalf("script error: %v", err) + } + if val.String() != "a-b-c" { + t.Errorf("got %q, want %q", val.String(), "a-b-c") + } +} + +func TestInjectUtilitiesEnv(t *testing.T) { + vm := goja.New() + injectUtilities(vm) + + os.Setenv("TEST_CONDUCTOR_VAR", "test_value") + defer os.Unsetenv("TEST_CONDUCTOR_VAR") + + val, err := vm.RunString(`util.env("TEST_CONDUCTOR_VAR")`) + if err != nil { + t.Fatalf("script error: %v", err) + } + if val.String() != "test_value" { + t.Errorf("got %q, want %q", val.String(), "test_value") + } + + // Non-existent env var + val, err = vm.RunString(`util.env("NONEXISTENT_VAR_12345")`) + if err != nil { + t.Fatalf("script error: %v", err) + } + if val.String() != "" { + t.Errorf("got %q, want empty string", val.String()) + } +} + +func TestInjectUtilitiesUUID(t *testing.T) { + vm := goja.New() + injectUtilities(vm) + + val, err := vm.RunString(`util.uuid()`) + if err != nil { + t.Fatalf("script error: %v", err) + } + if val.String() == "" { + t.Error("expected non-empty UUID") + } +} + +func TestInjectUtilitiesHTTP(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"method":"` + r.Method + `"}`)) + })) + defer server.Close() + + vm := goja.New() + injectUtilities(vm) + + // Test http.get + val, err := vm.RunString(`JSON.stringify(http.get("` + server.URL + `", {}))`) + if err != nil { + t.Fatalf("script error: %v", err) + } + result := val.String() + if result == "" { + t.Error("expected non-empty result") + } +} diff --git a/internal/taskworker/runner.go b/internal/taskworker/runner.go new file mode 100644 index 0000000..e6af50c --- /dev/null +++ b/internal/taskworker/runner.go @@ -0,0 +1,133 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package taskworker + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/antihax/optional" + "github.com/conductor-sdk/conductor-go/sdk/client" + "github.com/conductor-sdk/conductor-go/sdk/model" +) + +// RunnerOptions carries the poll and reporting settings that the worker commands expose +// as flags. Each poll field is applied only when set, because always sending a field — +// an empty Workerid, for instance — would change the outgoing request. +type RunnerOptions struct { + WorkerID string + Domain string + Count int32 + PollTimeoutMs int32 + // UseTaskWorkerID reports the polled task's own worker id on the result instead of + // WorkerID. JavaScript workers behave this way and workflows can observe it. + UseTaskWorkerID bool +} + +// conductorRunner adapts Conductor's TaskResourceApiService to Runner. It is the ONLY +// place model.* and *client.TaskResourceApiService appear in this package — the loop and +// the handlers see only taskworker.Task and taskworker.Result. +type conductorRunner struct { + client *client.TaskResourceApiService + opts RunnerOptions +} + +// NewConductorRunner returns a Runner backed by the Conductor task client, which the cmd +// layer supplies via internal.GetTaskClient(). +func NewConductorRunner(taskClient *client.TaskResourceApiService, opts RunnerOptions) Runner { + return &conductorRunner{client: taskClient, opts: opts} +} + +func (r *conductorRunner) Poll(ctx context.Context, taskType string) ([]PolledTask, error) { + opts := &client.TaskResourceApiBatchPollOpts{} + if r.opts.WorkerID != "" { + opts.Workerid = optional.NewString(r.opts.WorkerID) + } + if r.opts.Domain != "" { + opts.Domain = optional.NewString(r.opts.Domain) + } + if r.opts.Count > 0 { + opts.Count = optional.NewInt32(r.opts.Count) + } + if r.opts.PollTimeoutMs > 0 { + opts.Timeout = optional.NewInt32(r.opts.PollTimeoutMs) + } + + tasks, _, err := r.client.BatchPoll(ctx, taskType, opts) + if err != nil { + return nil, err + } + + polled := make([]PolledTask, 0, len(tasks)) + for _, t := range tasks { + polled = append(polled, taskFromModel(t)) + } + return polled, nil +} + +func (r *conductorRunner) Update(ctx context.Context, t Task, res Result) error { + _, _, err := r.client.UpdateTask(ctx, ToTaskResult(t, res, r.opts)) + return err +} + +// taskFromModel converts one SDK task, carrying any conversion failure on the task +// itself so the loop can fail it individually instead of dropping its batch peers. +func taskFromModel(t model.Task) PolledTask { + task := Task{ + ID: t.TaskId, + WorkflowID: t.WorkflowInstanceId, + Type: t.TaskType, + WorkerID: t.WorkerId, + } + + raw, err := json.Marshal(t) + if err != nil { + return PolledTask{Task: task, Err: fmt.Errorf("marshal task: %w", err)} + } + task.Raw = raw + return PolledTask{Task: task} +} + +// ToTaskResult maps a Result onto the SDK's TaskResult. It is a pure function so the +// mapping — the part most likely to drift during a refactor — can be table-tested +// without a live client. +func ToTaskResult(t Task, r Result, opts RunnerOptions) *model.TaskResult { + result := &model.TaskResult{ + TaskId: t.ID, + WorkflowInstanceId: t.WorkflowID, + Status: model.TaskResultStatus(r.Status), + OutputData: r.Output, + } + + if opts.UseTaskWorkerID { + result.WorkerId = t.WorkerID + } else if opts.WorkerID != "" { + result.WorkerId = opts.WorkerID + } + + if r.Reason != "" { + result.ReasonForIncompletion = r.Reason + } + + if len(r.Logs) > 0 { + logs := make([]model.TaskExecLog, len(r.Logs)) + for i, line := range r.Logs { + logs[i] = model.TaskExecLog{Log: line} + } + result.Logs = logs + } + + return result +} diff --git a/internal/taskworker/runner_test.go b/internal/taskworker/runner_test.go new file mode 100644 index 0000000..b0a8d64 --- /dev/null +++ b/internal/taskworker/runner_test.go @@ -0,0 +1,202 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package taskworker + +import ( + "encoding/json" + "testing" + + "github.com/conductor-sdk/conductor-go/sdk/model" +) + +// TestToTaskResultPreservesFlavourShapes pins the three result shapes that existed before +// the worker flavours were converged onto one loop. Workflows can observe all three, so a +// change here is a breaking change rather than a refactor. +func TestToTaskResultPreservesFlavourShapes(t *testing.T) { + polled := Task{ID: "t1", WorkflowID: "wf1", Type: "greet", WorkerID: "worker-from-task"} + + tests := []struct { + name string + result Result + opts RunnerOptions + want model.TaskResult + }{ + { + // worker js: reports the polled task's own worker id, and a failure carries + // the message under an "error" output key with no ReasonForIncompletion. + name: "js failure shape", + result: Result{Status: StatusFailed, Output: map[string]interface{}{"error": "script blew up"}}, + opts: RunnerOptions{UseTaskWorkerID: true}, + want: model.TaskResult{ + TaskId: "t1", + WorkflowInstanceId: "wf1", + WorkerId: "worker-from-task", + Status: model.FailedTask, + OutputData: map[string]interface{}{"error": "script blew up"}, + }, + }, + { + // worker stdio: uses the configured --worker-id, and a failure carries + // ReasonForIncompletion plus logs and no "error" key. + name: "stdio failure shape", + result: Result{Status: StatusFailed, Reason: "exit 1", Logs: []string{"stderr line"}}, + opts: RunnerOptions{WorkerID: "my-worker"}, + want: model.TaskResult{ + TaskId: "t1", + WorkflowInstanceId: "wf1", + WorkerId: "my-worker", + Status: model.FailedTask, + ReasonForIncompletion: "exit 1", + Logs: []model.TaskExecLog{{Log: "stderr line"}}, + }, + }, + { + // skill tools: constant worker id, output wrapped under "result". + name: "skill success shape", + result: Result{Status: StatusCompleted, Output: map[string]interface{}{"result": "Hello\n"}}, + opts: RunnerOptions{WorkerID: "conductor-cli"}, + want: model.TaskResult{ + TaskId: "t1", + WorkflowInstanceId: "wf1", + WorkerId: "conductor-cli", + Status: model.CompletedTask, + OutputData: map[string]interface{}{"result": "Hello\n"}, + }, + }, + { + // An unset --worker-id must leave WorkerId empty rather than sending "". + name: "no worker id configured", + result: Result{Status: StatusCompleted}, + opts: RunnerOptions{}, + want: model.TaskResult{ + TaskId: "t1", + WorkflowInstanceId: "wf1", + Status: model.CompletedTask, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ToTaskResult(polled, tt.result, tt.opts) + assertTaskResultEqual(t, got, tt.want) + }) + } +} + +// TestToTaskResultPassesStatusThrough covers the open Status type: worker js forwards +// whatever status its script returned, including values the CLI does not model. +func TestToTaskResultPassesStatusThrough(t *testing.T) { + tests := []Status{ + StatusCompleted, + StatusFailed, + StatusInProgress, + StatusFailedWithTerminalError, + Status("SOMETHING_THE_CLI_DOES_NOT_KNOW"), + } + + for _, status := range tests { + t.Run(string(status), func(t *testing.T) { + got := ToTaskResult(Task{ID: "t1"}, Result{Status: status}, RunnerOptions{}) + if string(got.Status) != string(status) { + t.Errorf("Status = %q, want %q — status must pass through unchanged", got.Status, status) + } + }) + } +} + +func TestToTaskResultOmitsEmptyLogsAndReason(t *testing.T) { + got := ToTaskResult(Task{ID: "t1"}, Result{Status: StatusCompleted}, RunnerOptions{}) + if got.Logs != nil { + t.Errorf("Logs = %v, want nil", got.Logs) + } + if got.ReasonForIncompletion != "" { + t.Errorf("ReasonForIncompletion = %q, want empty", got.ReasonForIncompletion) + } +} + +func TestTaskFromModelCarriesFullTaskAsRaw(t *testing.T) { + polled := taskFromModel(model.Task{ + TaskId: "t1", + WorkflowInstanceId: "wf1", + TaskType: "greet", + WorkerId: "w1", + InputData: map[string]interface{}{"name": "Miguel"}, + }) + + if polled.Err != nil { + t.Fatalf("conversion error = %v", polled.Err) + } + if polled.Task.ID != "t1" || polled.Task.WorkflowID != "wf1" || polled.Task.Type != "greet" { + t.Errorf("identity fields wrong: %+v", polled.Task) + } + if polled.Task.WorkerID != "w1" { + t.Errorf("WorkerID = %q, want w1 — worker js reports this back", polled.Task.WorkerID) + } + + // Raw must be the whole task, since goja exposes it as $.task and stdio writes it to + // the child's stdin. Both would break if it were only inputData. + var raw map[string]interface{} + if err := json.Unmarshal(polled.Task.Raw, &raw); err != nil { + t.Fatalf("Raw is not valid JSON: %v", err) + } + for _, key := range []string{"taskId", "workflowInstanceId", "taskType", "inputData"} { + if _, ok := raw[key]; !ok { + t.Errorf("Raw is missing %q — it must be the full task, not just inputData", key) + } + } + + input, err := polled.Task.InputData() + if err != nil { + t.Fatalf("InputData() error = %v", err) + } + if string(input) != `{"name":"Miguel"}` { + t.Errorf("InputData() = %s", input) + } +} + +func assertTaskResultEqual(t *testing.T, got *model.TaskResult, want model.TaskResult) { + t.Helper() + if got.TaskId != want.TaskId { + t.Errorf("TaskId = %q, want %q", got.TaskId, want.TaskId) + } + if got.WorkflowInstanceId != want.WorkflowInstanceId { + t.Errorf("WorkflowInstanceId = %q, want %q", got.WorkflowInstanceId, want.WorkflowInstanceId) + } + if got.WorkerId != want.WorkerId { + t.Errorf("WorkerId = %q, want %q", got.WorkerId, want.WorkerId) + } + if got.Status != want.Status { + t.Errorf("Status = %q, want %q", got.Status, want.Status) + } + if got.ReasonForIncompletion != want.ReasonForIncompletion { + t.Errorf("ReasonForIncompletion = %q, want %q", got.ReasonForIncompletion, want.ReasonForIncompletion) + } + if len(got.Logs) != len(want.Logs) { + t.Fatalf("len(Logs) = %d, want %d", len(got.Logs), len(want.Logs)) + } + for i := range want.Logs { + if got.Logs[i].Log != want.Logs[i].Log { + t.Errorf("Logs[%d] = %q, want %q", i, got.Logs[i].Log, want.Logs[i].Log) + } + } + if len(got.OutputData) != len(want.OutputData) { + t.Fatalf("len(OutputData) = %d, want %d", len(got.OutputData), len(want.OutputData)) + } + for k, v := range want.OutputData { + if got.OutputData[k] != v { + t.Errorf("OutputData[%q] = %v, want %v", k, got.OutputData[k], v) + } + } +} diff --git a/internal/taskworker/stdio.go b/internal/taskworker/stdio.go new file mode 100644 index 0000000..b635c81 --- /dev/null +++ b/internal/taskworker/stdio.go @@ -0,0 +1,204 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package taskworker + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + "time" + + log "github.com/sirupsen/logrus" +) + +// stdioResult is the JSON contract a stdio worker writes to its stdout. +type stdioResult struct { + Status string `json:"status"` + Output map[string]interface{} `json:"output,omitempty"` + Logs []string `json:"logs,omitempty"` + Reason string `json:"reason,omitempty"` +} + +// StdioOptions configures a StdioHandler. +type StdioOptions struct { + // Command and Args are the worker program to run, once per task. + Command string + Args []string + // Env is appended to the child's environment. It is passed in rather than read from + // viper so this package does not depend on process-global config, and so tests can + // assert what the child receives without mutating global state. + Env []string + // Domain, when set, is exported to the child as POLL_DOMAIN. + Domain string + // ExecTimeout bounds a single task's execution. Zero means no timeout. + ExecTimeout time.Duration + // Verbose prints the task JSON and the result JSON to stdout. + Verbose bool +} + +// StdioHandler runs an external program per task: the full task JSON goes in on stdin, +// a result JSON comes back on stdout. It is safe for concurrent use — each Handle call +// builds its own command and buffers. +type StdioHandler struct { + opts StdioOptions +} + +// NewStdioHandler returns a Handler that executes opts.Command for each task. +func NewStdioHandler(opts StdioOptions) *StdioHandler { + return &StdioHandler{opts: opts} +} + +func (h *StdioHandler) Handle(ctx context.Context, t Task) Result { + log.Infof("Processing task: %s (workflow: %s)", t.ID, t.WorkflowID) + + if h.opts.Verbose { + fmt.Println("=== Task Input ===") + fmt.Println(string(t.Raw)) + fmt.Println("==================") + } + + // The child is deliberately detached from the loop's cancellation. A task already + // running should finish and report its real result, which is what Run promises + // ("returns once the in-flight batch finishes"). Killing it on Ctrl-C instead makes + // the worker report a FAILED it inflicted on itself — and because results are + // delivered on a context that outlives cancellation, the server sees that failure + // and consumes one of the task's retries. A child that will not finish is handled by + // the second interrupt, which exits the process outright. + execCtx := context.WithoutCancel(ctx) + if h.opts.ExecTimeout > 0 { + var cancel context.CancelFunc + execCtx, cancel = context.WithTimeout(execCtx, h.opts.ExecTimeout) + defer cancel() + } + + cmd := exec.CommandContext(execCtx, h.opts.Command, h.opts.Args...) + cmd.Env = append(cmd.Environ(), + "TASK_TYPE="+t.Type, + "TASK_ID="+t.ID, + "WORKFLOW_ID="+t.WorkflowID, + "EXECUTION_ID="+t.WorkflowID, + ) + if h.opts.Domain != "" { + cmd.Env = append(cmd.Env, "POLL_DOMAIN="+h.opts.Domain) + } + cmd.Env = append(cmd.Env, h.opts.Env...) + + cmd.Stdin = bytes.NewReader(t.Raw) + + // The child's streams are both captured and echoed, so a worker's own output stays + // visible in the terminal while still being available for parsing and for logs. + var stdout, stderr bytes.Buffer + cmd.Stdout = io.MultiWriter(&stdout, os.Stdout) + cmd.Stderr = io.MultiWriter(&stderr, os.Stderr) + + result := h.runAndParse(cmd, &stdout, &stderr) + + log.Infof("Task %s handled with status: %s", t.ID, result.Status) + return result +} + +// printResultBanner reports a result under --verbose. The banner distinguishes failures +// so they stand out in a stream of task output. +func printResultBanner(result Result) { + resultJSON, _ := json.MarshalIndent(result, "", " ") + if result.Status == StatusFailed { + fmt.Println("=== Task Result (Error) ===") + fmt.Println(string(resultJSON)) + fmt.Println("===========================") + return + } + fmt.Println("=== Task Result ===") + fmt.Println(string(resultJSON)) + fmt.Println("===================") +} + +// runAndParse executes the child and turns its outcome into a Result. +func (h *StdioHandler) runAndParse(cmd *exec.Cmd, stdout, stderr *bytes.Buffer) Result { + if err := cmd.Run(); err != nil { + stderrOutput := stderr.String() + log.Errorf("Worker execution failed: %v", err) + if stderrOutput != "" { + log.Errorf("Worker stderr:\n%s", stderrOutput) + } + failure := Result{ + Status: StatusFailed, + Reason: fmt.Sprintf("worker execution failed: %v", err), + Logs: []string{stderrOutput}, + } + if h.opts.Verbose { + printResultBanner(failure) + } + return failure + } + + var parsed stdioResult + if err := json.Unmarshal(stdout.Bytes(), &parsed); err != nil { + stdoutOutput := stdout.String() + log.Errorf("Failed to parse worker output as JSON: %v", err) + log.Errorf("Worker stdout:\n%s", stdoutOutput) + failure := Result{ + Status: StatusFailed, + Reason: fmt.Sprintf("invalid worker stdout JSON: %v", err), + Logs: []string{stdoutOutput}, + } + if h.opts.Verbose { + printResultBanner(failure) + } + return failure + } + + // Reported before normalisation, so a worker that returned an unrecognised status + // sees what it actually sent rather than the rewritten failure — which is the whole + // point of asking for verbose output. + if h.opts.Verbose { + printResultBanner(Result{ + Status: Status(parsed.Status), + Output: parsed.Output, + Logs: parsed.Logs, + Reason: parsed.Reason, + }) + } + + return normalizeStdioResult(parsed) +} + +// normalizeStdioResult constrains a stdio worker's status to the three values the stdio +// contract documents. Unlike JavaScript workers — which forward any status straight +// through — an unrecognised status here fails the task. +func normalizeStdioResult(parsed stdioResult) Result { + result := Result{ + Status: Status(parsed.Status), + Output: parsed.Output, + Logs: parsed.Logs, + Reason: parsed.Reason, + } + + switch result.Status { + case StatusCompleted, StatusFailed, StatusInProgress: + return result + } + + // Carried over verbatim from the pre-convergence implementation, including its + // quirk: Status is overwritten before the reason is formatted, so the message always + // reads "invalid status from worker: FAILED" rather than naming the offending value. + // Preserved here to keep the convergence a pure refactor; see issue #92 for the + // follow-up that fixes the message. + result.Status = StatusFailed + result.Reason = fmt.Sprintf("invalid status from worker: %s", result.Status) + return result +} diff --git a/internal/taskworker/stdio_test.go b/internal/taskworker/stdio_test.go new file mode 100644 index 0000000..56bd81e --- /dev/null +++ b/internal/taskworker/stdio_test.go @@ -0,0 +1,304 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package taskworker + +import ( + "context" + "encoding/json" + "os/exec" + "strings" + "sync" + "testing" + "time" +) + +func stdioTask() Task { + return Task{ + ID: "task-1", + WorkflowID: "wf-1", + Type: "greet", + Raw: json.RawMessage(`{"taskId":"task-1","taskType":"greet","inputData":{"name":"Miguel"}}`), + } +} + +// shWorker builds options that run a shell snippet as the worker program. +func shWorker(script string) StdioOptions { + return StdioOptions{Command: "sh", Args: []string{"-c", script}} +} + +func TestStdioHandlerCompletedResult(t *testing.T) { + h := NewStdioHandler(shWorker(`echo '{"status":"COMPLETED","output":{"message":"hi"},"logs":["did it"]}'`)) + + got := h.Handle(context.Background(), stdioTask()) + + if got.Status != StatusCompleted { + t.Errorf("Status = %q, want COMPLETED", got.Status) + } + if got.Output["message"] != "hi" { + t.Errorf("Output = %v, want message=hi", got.Output) + } + if len(got.Logs) != 1 || got.Logs[0] != "did it" { + t.Errorf("Logs = %v, want [did it]", got.Logs) + } +} + +func TestStdioHandlerReceivesFullTaskOnStdin(t *testing.T) { + // The worker echoes back what it read, so the test can assert the whole task — + // not just inputData — arrived on stdin. + h := NewStdioHandler(shWorker(`payload=$(cat); echo "{\"status\":\"COMPLETED\",\"output\":{\"echoed\":$payload}}"`)) + + got := h.Handle(context.Background(), stdioTask()) + + if got.Status != StatusCompleted { + t.Fatalf("Status = %q, want COMPLETED", got.Status) + } + echoed, ok := got.Output["echoed"].(map[string]interface{}) + if !ok { + t.Fatalf("Output[echoed] = %#v, want an object", got.Output["echoed"]) + } + for _, key := range []string{"taskId", "taskType", "inputData"} { + if _, present := echoed[key]; !present { + t.Errorf("stdin payload missing %q — the whole task must be written, not just inputData", key) + } + } +} + +func TestStdioHandlerExportsTaskMetadataToChild(t *testing.T) { + h := NewStdioHandler(shWorker( + `echo "{\"status\":\"COMPLETED\",\"output\":{\"type\":\"$TASK_TYPE\",\"id\":\"$TASK_ID\",\"wf\":\"$WORKFLOW_ID\",\"exec\":\"$EXECUTION_ID\"}}"`)) + + got := h.Handle(context.Background(), stdioTask()) + + want := map[string]string{"type": "greet", "id": "task-1", "wf": "wf-1", "exec": "wf-1"} + for k, v := range want { + if got.Output[k] != v { + t.Errorf("child env produced %s=%v, want %q", k, got.Output[k], v) + } + } +} + +func TestStdioHandlerExportsDomainAndInjectedEnv(t *testing.T) { + opts := shWorker(`echo "{\"status\":\"COMPLETED\",\"output\":{\"domain\":\"$POLL_DOMAIN\",\"injected\":\"$CONDUCTOR_AUTH_TOKEN\"}}"`) + opts.Domain = "prod" + opts.Env = []string{"CONDUCTOR_AUTH_TOKEN=tok-123"} + h := NewStdioHandler(opts) + + got := h.Handle(context.Background(), stdioTask()) + + if got.Output["domain"] != "prod" { + t.Errorf("POLL_DOMAIN = %v, want prod", got.Output["domain"]) + } + if got.Output["injected"] != "tok-123" { + t.Errorf("injected env did not reach the child: %v", got.Output["injected"]) + } +} + +func TestStdioHandlerNonZeroExitFails(t *testing.T) { + h := NewStdioHandler(shWorker(`echo "something broke" >&2; exit 3`)) + + got := h.Handle(context.Background(), stdioTask()) + + if got.Status != StatusFailed { + t.Errorf("Status = %q, want FAILED", got.Status) + } + if !strings.Contains(got.Reason, "worker execution failed") { + t.Errorf("Reason = %q, want it to mention worker execution failed", got.Reason) + } + if len(got.Logs) != 1 || !strings.Contains(got.Logs[0], "something broke") { + t.Errorf("Logs = %v, want stderr captured", got.Logs) + } +} + +func TestStdioHandlerMalformedJSONFails(t *testing.T) { + h := NewStdioHandler(shWorker(`echo 'this is not json'`)) + + got := h.Handle(context.Background(), stdioTask()) + + if got.Status != StatusFailed { + t.Errorf("Status = %q, want FAILED", got.Status) + } + if !strings.Contains(got.Reason, "invalid worker stdout JSON") { + t.Errorf("Reason = %q, want it to mention invalid worker stdout JSON", got.Reason) + } + if len(got.Logs) != 1 || !strings.Contains(got.Logs[0], "this is not json") { + t.Errorf("Logs = %v, want the raw stdout captured for debugging", got.Logs) + } +} + +func TestStdioHandlerExecTimeoutKillsChild(t *testing.T) { + opts := StdioOptions{Command: "sleep", Args: []string{"30"}} + opts.ExecTimeout = 100 * time.Millisecond + h := NewStdioHandler(opts) + + start := time.Now() + got := h.Handle(context.Background(), stdioTask()) + elapsed := time.Since(start) + + if got.Status != StatusFailed { + t.Errorf("Status = %q, want FAILED", got.Status) + } + if elapsed > 5*time.Second { + t.Errorf("took %v — the exec timeout did not kill the child", elapsed) + } +} + +// TestStdioHandlerCancelledContextLetsTaskFinish pins that shutdown does not fail an +// in-flight task. Killing the child on Ctrl-C made the worker report +// "worker execution failed: signal: killed" — a failure it inflicted on itself, which the +// server then counts against the task's retries. The child is detached from the loop +// context so it finishes and reports its real result. +func TestStdioHandlerCancelledContextLetsTaskFinish(t *testing.T) { + h := NewStdioHandler(shWorker(`sleep 0.3; echo '{"status":"COMPLETED","output":{"done":true}}'`)) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // already shutting down when the task starts + + got := h.Handle(ctx, stdioTask()) + + if got.Status != StatusCompleted { + t.Errorf("Status = %q, want COMPLETED — shutdown must not fail an in-flight task", got.Status) + } + if strings.Contains(got.Reason, "killed") { + t.Errorf("Reason = %q — the child was killed by shutdown", got.Reason) + } + if got.Output["done"] != true { + t.Errorf("Output = %v, want the worker's real result", got.Output) + } +} + +// TestStdioHandlerExecTimeoutStillAppliesAfterCancel guards the other half: detaching the +// child from cancellation must not also detach it from its execution timeout. +func TestStdioHandlerExecTimeoutStillAppliesAfterCancel(t *testing.T) { + opts := StdioOptions{Command: "sleep", Args: []string{"30"}} + opts.ExecTimeout = 100 * time.Millisecond + h := NewStdioHandler(opts) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + start := time.Now() + got := h.Handle(ctx, stdioTask()) + + if got.Status != StatusFailed { + t.Errorf("Status = %q, want FAILED", got.Status) + } + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Errorf("took %v — the exec timeout stopped applying once the child was detached", elapsed) + } +} + +// TestStdioHandlerExecTimeoutDoesNotReapGrandchildren documents a limitation carried over +// from the pre-convergence implementation: the timeout signals only the direct child, and +// Run then waits for the captured stdout/stderr pipes to close. A worker that forks a +// long-running subprocess keeps those pipes open, so the effective timeout is the +// grandchild's lifetime, not ExecTimeout. +// +// Bounding a whole process tree needs process groups (setpgid plus kill on the negative +// pgid) and is platform-specific, so it is deliberately not part of the convergence work. +// The test is skipped by default because asserting it means waiting out the grandchild. +func TestStdioHandlerExecTimeoutDoesNotReapGrandchildren(t *testing.T) { + t.Skip("documents a known pre-existing limitation; unskip to observe it (takes ~3s)") + + // A compound script forks a real grandchild rather than exec-ing into it. + opts := shWorker(`sleep 3; echo '{"status":"COMPLETED"}'`) + opts.ExecTimeout = 100 * time.Millisecond + h := NewStdioHandler(opts) + + start := time.Now() + h.Handle(context.Background(), stdioTask()) + + if elapsed := time.Since(start); elapsed < time.Second { + t.Errorf("returned in %v — grandchildren are now reaped, so this limitation is fixed "+ + "and both this test and the comment on it should be removed", elapsed) + } +} + +// TestStdioHandlerUnknownStatusIsNormalized documents that stdio workers, unlike +// JavaScript workers, do not get to invent statuses. +func TestStdioHandlerUnknownStatusIsNormalized(t *testing.T) { + h := NewStdioHandler(shWorker(`echo '{"status":"BANANA"}'`)) + + got := h.Handle(context.Background(), stdioTask()) + + if got.Status != StatusFailed { + t.Errorf("Status = %q, want FAILED", got.Status) + } + if !strings.Contains(got.Reason, "invalid status from worker") { + t.Errorf("Reason = %q, want it to mention an invalid status", got.Reason) + } +} + +// TestNormalizeStdioResultPreservesStatuses covers the three statuses that pass through +// untouched, plus the pre-existing quirk in the rejection message. +func TestNormalizeStdioResultPreservesStatuses(t *testing.T) { + for _, status := range []Status{StatusCompleted, StatusFailed, StatusInProgress} { + t.Run(string(status), func(t *testing.T) { + got := normalizeStdioResult(stdioResult{Status: string(status), Reason: "as-is"}) + if got.Status != status { + t.Errorf("Status = %q, want %q", got.Status, status) + } + if got.Reason != "as-is" { + t.Errorf("Reason = %q, want it left alone", got.Reason) + } + }) + } + + t.Run("unknown status message quirk is preserved", func(t *testing.T) { + got := normalizeStdioResult(stdioResult{Status: "WEIRD"}) + // Faithful to the pre-convergence behaviour: the message names FAILED rather + // than the offending status, because Status is overwritten first. + if got.Reason != "invalid status from worker: FAILED" { + t.Errorf("Reason = %q, want the pre-existing wording preserved", got.Reason) + } + }) + + t.Run("empty status is rejected", func(t *testing.T) { + got := normalizeStdioResult(stdioResult{}) + if got.Status != StatusFailed { + t.Errorf("Status = %q, want FAILED for an empty status", got.Status) + } + }) +} + +// TestStdioHandlerConcurrentUse guards the Handler concurrency contract: one handler is +// shared across all goroutines of a batch poll. +func TestStdioHandlerConcurrentUse(t *testing.T) { + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh not available") + } + h := NewStdioHandler(shWorker(`echo "{\"status\":\"COMPLETED\",\"output\":{\"id\":\"$TASK_ID\"}}"`)) + + const n = 8 + var wg sync.WaitGroup + results := make([]Result, n) + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + task := stdioTask() + task.ID = string(rune('a' + i)) + results[i] = h.Handle(context.Background(), task) + }(i) + } + wg.Wait() + + for i, got := range results { + if got.Status != StatusCompleted { + t.Errorf("result %d status = %q, want COMPLETED", i, got.Status) + } + if want := string(rune('a' + i)); got.Output["id"] != want { + t.Errorf("result %d id = %v, want %q — concurrent Handle calls crossed state", i, got.Output["id"], want) + } + } +} diff --git a/internal/taskworker/taskworker.go b/internal/taskworker/taskworker.go new file mode 100644 index 0000000..35474a7 --- /dev/null +++ b/internal/taskworker/taskworker.go @@ -0,0 +1,263 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +// Package taskworker is the single poll→execute→update loop shared by every worker +// flavour in the CLI: stdio workers, JavaScript (goja) workers, registry workers, and +// skill tool workers. It is layered — the loop and its two interfaces live here, the +// Conductor SDK is confined to the runner bridge, and each flavour's execution logic +// lives in a Handler. +// +// Before this package existed the loop was hand-copied four times in cmd/worker.go with +// no backoff, no context cancellation, and no tests. +package taskworker + +import ( + "context" + "encoding/json" + "fmt" + "time" + + log "github.com/sirupsen/logrus" +) + +// defaultPollBackoff is the idle wait between polls that return no task or an error. +// Runners also long-poll the server, so this is a hot-loop backstop rather than the +// primary pacing mechanism. +const defaultPollBackoff = 100 * time.Millisecond + +// Status is the task status reported back to Conductor. It is deliberately an open +// string type rather than a closed enum: JavaScript workers forward whatever status +// their script returns straight through, and FAILED_WITH_TERMINAL_ERROR is documented +// and in use. Handlers that need to constrain the set normalise it themselves. +type Status string + +const ( + StatusCompleted Status = "COMPLETED" + StatusFailed Status = "FAILED" + StatusInProgress Status = "IN_PROGRESS" + StatusFailedWithTerminalError Status = "FAILED_WITH_TERMINAL_ERROR" +) + +// Task is one polled task, decoupled from the SDK's model.Task. +type Task struct { + ID string + WorkflowID string + Type string + // WorkerID is the worker id carried by the polled task. JavaScript workers report + // this value back on the result rather than the configured --worker-id. + WorkerID string + // Raw is json.Marshal(model.Task) — the whole task, not the raw HTTP body and not + // just inputData. Stdio workers write it to the child's stdin and goja workers + // expose it as $.task, so both depend on it being the SDK struct's marshalling. + Raw json.RawMessage +} + +// InputData returns just the task's inputData, for handlers that want the input rather +// than the whole task. A task with no inputData yields "null". +// +// model.Task.InputData is tagged omitempty, so an empty-but-non-nil map is absent from +// Raw and also yields "null" here, where marshalling the map directly would have given +// "{}". Handlers decode into structs, so both produce the same zero values. +func (t Task) InputData() (json.RawMessage, error) { + var envelope struct { + InputData json.RawMessage `json:"inputData"` + } + if err := json.Unmarshal(t.Raw, &envelope); err != nil { + return nil, err + } + if len(envelope.InputData) == 0 { + return json.RawMessage("null"), nil + } + return envelope.InputData, nil +} + +// Result is the outcome of executing one task. +// +// There is no error return alongside it: every outcome is a Result, so there is exactly +// one way to express failure. Handlers build their own failure Results because the +// flavours shape failures differently — JavaScript workers report the message under an +// "error" output key, stdio workers use ReasonForIncompletion plus logs — and those +// differences are observable by workflows. +// The json tags exist because --verbose prints a Result back to the user, and that +// output was previously the stdio worker's own lowercase, omitempty-tagged shape. +type Result struct { + Status Status `json:"status"` + Output map[string]interface{} `json:"output,omitempty"` + Logs []string `json:"logs,omitempty"` + Reason string `json:"reason,omitempty"` +} + +// Failure builds a Result for the common shape: FAILED with a reason and no output. +func Failure(reason string) Result { + return Result{Status: StatusFailed, Reason: reason} +} + +// Handler executes one task. +// +// A Handler MUST be safe for concurrent use: one Handler is shared across all the +// goroutines of a batch poll. +type Handler interface { + Handle(ctx context.Context, t Task) Result +} + +// HandlerFunc adapts a function to Handler. +type HandlerFunc func(ctx context.Context, t Task) Result + +func (f HandlerFunc) Handle(ctx context.Context, t Task) Result { return f(ctx, t) } + +// PolledTask is one entry from a poll. Conversion happens per task so that a single +// malformed task fails on its own instead of discarding its whole batch. +type PolledTask struct { + Task Task + // Err is set when this task could not be converted from the SDK model. The loop + // fails such a task individually and continues with the rest of the batch. + Err error +} + +// Runner is the server boundary: polling for work and reporting results. The production +// implementation wraps the Conductor SDK; tests inject a fake. +type Runner interface { + Poll(ctx context.Context, taskType string) ([]PolledTask, error) + Update(ctx context.Context, t Task, r Result) error +} + +// Config tunes the loop. +type Config struct { + // PollBackoff is the wait after an empty or failed poll. Zero uses the default. + PollBackoff time.Duration +} + +// Worker runs the poll→execute→update loop for a single task type over a Runner. +type Worker struct { + runner Runner + cfg Config +} + +// NewWorker returns a Worker backed by the given Runner. +func NewWorker(runner Runner, cfg Config) *Worker { + if cfg.PollBackoff <= 0 { + cfg.PollBackoff = defaultPollBackoff + } + return &Worker{runner: runner, cfg: cfg} +} + +// Run polls taskType and dispatches each task to h until ctx is cancelled. +// +// Cancellation is not immediate: Run returns once the in-flight batch finishes. A +// handler that blocks — a goja script has no interrupt wired, for instance — delays +// shutdown for as long as it runs. +// +// Transient poll failures back off and retry rather than stop the loop, and a failing +// task affects only itself. +func (w *Worker) Run(ctx context.Context, taskType string, h Handler) { + for { + if ctx.Err() != nil { + return + } + + polled, err := w.runner.Poll(ctx, taskType) + if err != nil { + // Logged every time rather than once: a persistent failure here (bad + // credentials, unreachable server) is the single most common reason a + // worker appears to do nothing, and the backoff keeps the volume sane. + log.Errorf("Error polling tasks: %v", err) + if !sleep(ctx, w.cfg.PollBackoff) { + return + } + continue + } + + if len(polled) == 0 { + log.Debug("No tasks available") + if !sleep(ctx, w.cfg.PollBackoff) { + return + } + continue + } + + // Debug, not Info: skill run starts one loop per tool type and streams agent output + // to the same terminal, so an Info line here buries the stream. Poll *errors* stay + // at Error — a silently idle worker is the failure this logging exists to surface. + log.Debugf("Polled %d task(s)", len(polled)) + w.runBatch(ctx, polled, h) + } +} + +// runBatch executes every task in a poll batch concurrently and waits for all of them. +// +// Waiting for the whole batch before polling again preserves the pre-existing --count +// semantics: the next poll is gated on the slowest task in the batch. Decoupling poll +// cadence from execution would be a behaviour change and is deliberately not done here. +func (w *Worker) runBatch(ctx context.Context, polled []PolledTask, h Handler) { + done := make(chan struct{}) + var pending int + + for _, p := range polled { + pending++ + go func(p PolledTask) { + defer func() { done <- struct{}{} }() + w.runOne(ctx, p, h) + }(p) + } + + for i := 0; i < pending; i++ { + <-done + } +} + +// runOne executes a single task and reports its result. A conversion error from the poll +// seam, or a panic in the handler, fails that task rather than the loop. +func (w *Worker) runOne(ctx context.Context, p PolledTask, h Handler) { + if p.Err != nil { + w.update(ctx, p.Task, Failure(p.Err.Error())) + return + } + + result := w.safeHandle(ctx, p.Task, h) + w.update(ctx, p.Task, result) +} + +// safeHandle runs the handler, converting a panic into a failed task so that one +// misbehaving handler cannot take the whole worker process down. +func (w *Worker) safeHandle(ctx context.Context, t Task, h Handler) (result Result) { + defer func() { + if r := recover(); r != nil { + result = Failure(fmt.Sprintf("worker panicked: %v", r)) + } + }() + return h.Handle(ctx, t) +} + +// update reports a result, deliberately detached from the loop's cancellation. +// +// A task that finished while the user was pressing Ctrl-C still has its result +// delivered; using the cancelled context would abandon completed work and leave the +// task in-flight until the server times it out. +func (w *Worker) update(ctx context.Context, t Task, r Result) { + if err := w.runner.Update(context.WithoutCancel(ctx), t, r); err != nil { + log.Errorf("Error updating task %s: %v", t.ID, err) + } +} + +// sleep waits d or until ctx is cancelled; it returns false if ctx was cancelled, which +// keeps the loop responsive to Ctrl-C during idle waits. +func sleep(ctx context.Context, d time.Duration) bool { + t := time.NewTimer(d) + defer t.Stop() + select { + case <-ctx.Done(): + return false + case <-t.C: + return true + } +} diff --git a/internal/taskworker/taskworker_test.go b/internal/taskworker/taskworker_test.go new file mode 100644 index 0000000..717a33d --- /dev/null +++ b/internal/taskworker/taskworker_test.go @@ -0,0 +1,338 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package taskworker + +import ( + "context" + "encoding/json" + "errors" + "sync" + "sync/atomic" + "testing" + "time" +) + +// fakeRunner is a scripted Runner. Each Poll returns the next entry from batches, then +// reports empty polls forever. +type fakeRunner struct { + mu sync.Mutex + batches [][]PolledTask + errs []error + polls atomic.Int32 + updates []update +} + +type update struct { + task Task + result Result +} + +func (f *fakeRunner) Poll(ctx context.Context, taskType string) ([]PolledTask, error) { + n := int(f.polls.Add(1)) - 1 + + f.mu.Lock() + defer f.mu.Unlock() + if n < len(f.errs) && f.errs[n] != nil { + return nil, f.errs[n] + } + if n < len(f.batches) { + return f.batches[n], nil + } + return nil, nil +} + +func (f *fakeRunner) Update(ctx context.Context, t Task, r Result) error { + f.mu.Lock() + defer f.mu.Unlock() + f.updates = append(f.updates, update{task: t, result: r}) + return nil +} + +func (f *fakeRunner) recorded() []update { + f.mu.Lock() + defer f.mu.Unlock() + out := make([]update, len(f.updates)) + copy(out, f.updates) + return out +} + +func task(id string) Task { + return Task{ID: id, WorkflowID: "wf-1", Type: "greet", Raw: json.RawMessage(`{"taskId":"` + id + `"}`)} +} + +func okHandler() Handler { + return HandlerFunc(func(ctx context.Context, t Task) Result { + return Result{Status: StatusCompleted, Output: map[string]interface{}{"id": t.ID}} + }) +} + +// runFor starts Run and cancels it once cancel() reports true, so tests never rely on +// wall-clock sleeps to decide the loop has done enough. +func runFor(t *testing.T, w *Worker, h Handler, until func() bool) { + t.Helper() + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + w.Run(ctx, "greet", h) + close(done) + }() + + deadline := time.After(2 * time.Second) + for !until() { + select { + case <-deadline: + cancel() + t.Fatal("condition not reached within 2s") + default: + time.Sleep(time.Millisecond) + } + } + cancel() + + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("Run did not return after cancellation") + } +} + +func TestRunDispatchesPolledTasks(t *testing.T) { + r := &fakeRunner{batches: [][]PolledTask{{{Task: task("t1")}}}} + w := NewWorker(r, Config{PollBackoff: time.Millisecond}) + + runFor(t, w, okHandler(), func() bool { return len(r.recorded()) >= 1 }) + + got := r.recorded() + if got[0].task.ID != "t1" { + t.Errorf("task ID = %q, want t1", got[0].task.ID) + } + if got[0].result.Status != StatusCompleted { + t.Errorf("status = %q, want COMPLETED", got[0].result.Status) + } +} + +func TestRunBacksOffOnPollErrorWithoutSpinning(t *testing.T) { + r := &fakeRunner{errs: []error{errors.New("boom"), errors.New("boom"), errors.New("boom")}} + w := NewWorker(r, Config{PollBackoff: 50 * time.Millisecond}) + + ctx, cancel := context.WithTimeout(context.Background(), 220*time.Millisecond) + defer cancel() + w.Run(ctx, "greet", okHandler()) + + // With a 50ms backoff over ~220ms, a correct loop polls a handful of times. A loop + // without backoff would poll thousands of times. + if polls := r.polls.Load(); polls > 10 { + t.Errorf("polled %d times in 220ms with a 50ms backoff — loop is not backing off", polls) + } +} + +func TestRunBacksOffOnEmptyPollWithoutSpinning(t *testing.T) { + r := &fakeRunner{} // always empty + w := NewWorker(r, Config{PollBackoff: 50 * time.Millisecond}) + + ctx, cancel := context.WithTimeout(context.Background(), 220*time.Millisecond) + defer cancel() + w.Run(ctx, "greet", okHandler()) + + if polls := r.polls.Load(); polls > 10 { + t.Errorf("polled %d times in 220ms with a 50ms backoff — loop is not backing off", polls) + } +} + +func TestRunReturnsWhenContextCancelledMidBackoff(t *testing.T) { + r := &fakeRunner{} // always empty, so the loop sits in backoff + w := NewWorker(r, Config{PollBackoff: time.Hour}) + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + w.Run(ctx, "greet", okHandler()) + close(done) + }() + + time.Sleep(20 * time.Millisecond) // let it reach the backoff sleep + cancel() + + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("Run did not return while waiting in backoff — cancellation is not interrupting the sleep") + } +} + +func TestRunFailedTaskDoesNotStopLoop(t *testing.T) { + r := &fakeRunner{batches: [][]PolledTask{ + {{Task: task("bad")}}, + {{Task: task("good")}}, + }} + w := NewWorker(r, Config{PollBackoff: time.Millisecond}) + + h := HandlerFunc(func(ctx context.Context, t Task) Result { + if t.ID == "bad" { + return Failure("handler said no") + } + return Result{Status: StatusCompleted} + }) + + runFor(t, w, h, func() bool { return len(r.recorded()) >= 2 }) + + got := r.recorded() + if got[0].result.Status != StatusFailed || got[0].result.Reason != "handler said no" { + t.Errorf("first update = %+v, want FAILED with reason", got[0].result) + } + if got[1].result.Status != StatusCompleted { + t.Errorf("second update = %+v, want COMPLETED — loop did not survive the failure", got[1].result) + } +} + +func TestRunHandlerPanicFailsOnlyThatTask(t *testing.T) { + r := &fakeRunner{batches: [][]PolledTask{ + {{Task: task("panics")}}, + {{Task: task("fine")}}, + }} + w := NewWorker(r, Config{PollBackoff: time.Millisecond}) + + h := HandlerFunc(func(ctx context.Context, t Task) Result { + if t.ID == "panics" { + panic("kaboom") + } + return Result{Status: StatusCompleted} + }) + + runFor(t, w, h, func() bool { return len(r.recorded()) >= 2 }) + + got := r.recorded() + if got[0].result.Status != StatusFailed { + t.Errorf("panicking task status = %q, want FAILED", got[0].result.Status) + } + if got[1].result.Status != StatusCompleted { + t.Errorf("loop did not survive a handler panic") + } +} + +func TestRunConversionErrorFailsOnlyThatTaskAndPeersStillRun(t *testing.T) { + r := &fakeRunner{batches: [][]PolledTask{{ + {Task: task("broken"), Err: errors.New("marshal task: nope")}, + {Task: task("peer")}, + }}} + w := NewWorker(r, Config{PollBackoff: time.Millisecond}) + + runFor(t, w, okHandler(), func() bool { return len(r.recorded()) >= 2 }) + + byID := map[string]Result{} + for _, u := range r.recorded() { + byID[u.task.ID] = u.result + } + + if got := byID["broken"]; got.Status != StatusFailed || got.Reason != "marshal task: nope" { + t.Errorf("broken task = %+v, want FAILED carrying the conversion error", got) + } + if got := byID["peer"]; got.Status != StatusCompleted { + t.Errorf("peer task = %+v, want COMPLETED — a bad task discarded its batch peers", got) + } +} + +func TestRunBatchExecutesConcurrently(t *testing.T) { + const n = 4 + batch := make([]PolledTask, 0, n) + for i := 0; i < n; i++ { + batch = append(batch, PolledTask{Task: task(string(rune('a' + i)))}) + } + r := &fakeRunner{batches: [][]PolledTask{batch}} + w := NewWorker(r, Config{PollBackoff: time.Millisecond}) + + // Every handler waits on the same barrier. If the batch ran serially the barrier + // would never be satisfied and the test would time out. + var wg sync.WaitGroup + wg.Add(n) + h := HandlerFunc(func(ctx context.Context, t Task) Result { + wg.Done() + wg.Wait() + return Result{Status: StatusCompleted} + }) + + barrierMet := make(chan struct{}) + go func() { wg.Wait(); close(barrierMet) }() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go w.Run(ctx, "greet", h) + + select { + case <-barrierMet: + case <-time.After(2 * time.Second): + t.Fatal("batch tasks did not run concurrently — all 4 never ran at once") + } +} + +func TestUpdateStillDeliversAfterContextCancelled(t *testing.T) { + r := &fakeRunner{batches: [][]PolledTask{{{Task: task("t1")}}}} + w := NewWorker(r, Config{PollBackoff: time.Millisecond}) + + // The handler cancels the loop context before returning, mimicking Ctrl-C landing + // while a task is in flight. The result must still be reported. + ctx, cancel := context.WithCancel(context.Background()) + h := HandlerFunc(func(hctx context.Context, t Task) Result { + cancel() + return Result{Status: StatusCompleted} + }) + + done := make(chan struct{}) + go func() { w.Run(ctx, "greet", h); close(done) }() + + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("Run did not return") + } + + got := r.recorded() + if len(got) != 1 { + t.Fatalf("recorded %d updates, want 1 — a result finished during shutdown was dropped", len(got)) + } + if got[0].result.Status != StatusCompleted { + t.Errorf("status = %q, want COMPLETED", got[0].result.Status) + } +} + +func TestInputData(t *testing.T) { + tests := []struct { + name string + raw string + want string + }{ + {name: "present", raw: `{"taskId":"t","inputData":{"name":"Miguel"}}`, want: `{"name":"Miguel"}`}, + {name: "absent yields null", raw: `{"taskId":"t"}`, want: `null`}, + {name: "explicit null", raw: `{"inputData":null}`, want: `null`}, + {name: "empty object", raw: `{"inputData":{}}`, want: `{}`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Task{Raw: json.RawMessage(tt.raw)}.InputData() + if err != nil { + t.Fatalf("InputData() error = %v", err) + } + if string(got) != tt.want { + t.Errorf("InputData() = %s, want %s", got, tt.want) + } + }) + } +} + +func TestInputDataMalformedRawErrors(t *testing.T) { + if _, err := (Task{Raw: json.RawMessage(`not json`)}).InputData(); err == nil { + t.Error("InputData() on malformed Raw returned nil error") + } +}