diff --git a/internal/codemode/help.go b/internal/codemode/help.go index 60ac97cf..66426585 100644 --- a/internal/codemode/help.go +++ b/internal/codemode/help.go @@ -20,7 +20,7 @@ const helpDescMax = 100 // this list a model exploring via help() would never learn they exist. var helpGlobalHelpers = []string{ "print(...values)", - "parallel([{tool, args}, ...])", + "parallel([[ns.tool, args], ...]) — concurrent, same call form as ns.tool(args)", "compact(value)", "sleep(ms)", "atob(b64) / btoa(str)", diff --git a/internal/codemode/sandbox.go b/internal/codemode/sandbox.go index 43eb6178..71ed6187 100644 --- a/internal/codemode/sandbox.go +++ b/internal/codemode/sandbox.go @@ -203,11 +203,18 @@ func (s *Sandbox) Execute(ctx context.Context, code string, tools []ToolDef) (*E inputSchema: entry.schema, examples: entry.examples, }) - if err := nsObj.Set(entry.name, fn); err != nil { + // Tag the callable with its canonical name so parallel() can accept + // the bound reference (github.list_issues) directly and recover the + // name — unifying the concurrent path with the sequential ns.tool() + // form. The tag is hidden (non-enumerable) so it never pollutes + // Object.keys() or serialization. Both the primary key and any alias + // share the SAME tagged value. + fnVal := tagToolFunc(vm, fn, fullName) + if err := nsObj.Set(entry.name, fnVal); err != nil { return nil, fmt.Errorf("set %s.%s: %w", ns, entry.name, err) } if entry.jsName != entry.name { - if err := nsObj.Set(entry.jsName, fn); err != nil { + if err := nsObj.Set(entry.jsName, fnVal); err != nil { return nil, fmt.Errorf("set %s.%s alias: %w", ns, entry.jsName, err) } } @@ -766,6 +773,59 @@ type toolSchema struct { examples []string } +// toolNameProp is the hidden own-property carrying a bound tool function's +// canonical "ns__name". parallel() reads it to accept the natural function +// reference (github.list_issues) in place of a "ns__name" string descriptor. +const toolNameProp = "__tool" + +// tagToolFunc converts a Go tool closure into a Goja callable that carries its +// canonical name as a hidden (non-enumerable, non-writable) own property. This +// lets parallel() accept the bound reference directly — parallel([[github.list_issues, +// {...}]]) — and recover the name for concurrent dispatch, so the concurrent +// path uses the same ns.tool call form as the sequential one instead of a +// separate string-descriptor convention. If tagging fails for any reason the +// untagged callable is returned unchanged: it still works as a sequential call, +// it just can't be referenced by identity inside parallel(). +func tagToolFunc(vm *goja.Runtime, fn func(goja.FunctionCall) goja.Value, fullName string) goja.Value { + fnVal := vm.ToValue(fn) + fnObj, ok := fnVal.(*goja.Object) + if !ok { + return fnVal + } + if err := fnObj.DefineDataProperty( + toolNameProp, vm.ToValue(fullName), + goja.FLAG_FALSE, goja.FLAG_FALSE, goja.FLAG_FALSE, + ); err != nil { + return fnVal + } + return fnObj +} + +// toolNameOfFunc recovers the canonical "ns__name" from a value that may be a +// tagged tool function. Returns ("", false) for non-functions and for +// functions that carry no tag (e.g. an agent's own closure). +func toolNameOfFunc(vm *goja.Runtime, v goja.Value) (string, bool) { + if v == nil || goja.IsUndefined(v) || goja.IsNull(v) { + return "", false + } + if _, isFn := goja.AssertFunction(v); !isFn { + return "", false + } + obj := v.ToObject(vm) + if obj == nil { + return "", false + } + tag := obj.Get(toolNameProp) + if tag == nil || goja.IsUndefined(tag) || goja.IsNull(tag) { + return "", false + } + name := tag.String() + if name == "" { + return "", false + } + return name, true +} + // makeToolFunc creates a Go function that Goja calls synchronously. func (s *Sandbox) makeToolFunc( ctx context.Context, diff --git a/internal/codemode/sandbox_parallel.go b/internal/codemode/sandbox_parallel.go index bbea0009..4d9cfe72 100644 --- a/internal/codemode/sandbox_parallel.go +++ b/internal/codemode/sandbox_parallel.go @@ -20,9 +20,17 @@ type parallelCallDescriptor struct { } // makeParallelFunc creates the parallel() global that runs tool calls -// concurrently. API: parallel([{tool: "ns__name", args: {...}}, ...]) => any[] -// Goja is single-threaded, so we extract descriptors, run Go goroutines, -// collect results, and return them as a JS array. +// concurrently and returns results in order (a failed call is null at its +// index; the call never throws). Items use the same call form as the +// sequential path: +// +// parallel([[ns.tool, {args}], ...]) // tuple shorthand +// parallel([{tool: ns.tool, args}, ...]) // object form, bound reference +// parallel([{tool: "ns__name", args}, ...]) // object form, string name +// +// Goja is single-threaded, so we extract descriptors (recovering each tool's +// canonical name), run the actual calls in Go goroutines, collect results, and +// return them as a JS array. func (s *Sandbox) makeParallelFunc( ctx context.Context, vm *goja.Runtime, @@ -37,13 +45,27 @@ func (s *Sandbox) makeParallelFunc( } // parseParallelArgs validates and extracts descriptors from the JS arguments. +// +// Every item may be written two ways, so the concurrent path uses the SAME +// call form as the sequential ns.tool(args) form instead of a separate string +// convention: +// +// parallel([[github.list_issues, {repo:"x"}], [linear.search, {query:"y"}]]) +// parallel([{tool: github.list_issues, args: {repo:"x"}}, ...]) +// parallel([{tool: "github__list_issues", args: {...}}, ...]) // still works +// +// A bound tool reference (github.list_issues) carries its canonical name in a +// hidden property (see tagToolFunc), which is recovered here; a plain string +// name is accepted verbatim for backward compatibility. Args classification and +// element re-reads go through the live Goja value (not Export) so the function +// tag survives — Export erases it. func parseParallelArgs(vm *goja.Runtime, call goja.FunctionCall) []parallelCallDescriptor { if len(call.Arguments) == 0 { - panic(newJSError(vm, "parallel() requires an array of {tool, args} descriptors")) + panic(newJSError(vm, "parallel() requires an array of tool calls, e.g. parallel([[ns.tool, {args}], ...]) or parallel([{tool, args}, ...])")) } - exported := call.Arguments[0].Export() - items, ok := exported.([]any) + top := call.Arguments[0] + items, ok := top.Export().([]any) if !ok { panic(newJSError(vm, "parallel() argument must be an array")) } @@ -51,24 +73,77 @@ func parseParallelArgs(vm *goja.Runtime, call goja.FunctionCall) []parallelCallD panic(newJSError(vm, fmt.Sprintf("parallel() max %d calls, got %d", maxParallelCalls, len(items)))) } + arr := top.ToObject(vm) descriptors := make([]parallelCallDescriptor, len(items)) for i, item := range items { - m, ok := item.(map[string]any) - if !ok { - panic(newJSError(vm, fmt.Sprintf("parallel() item %d must be an object with {tool, args}", i))) - } - toolName, _ := m["tool"].(string) - if toolName == "" { - panic(newJSError(vm, fmt.Sprintf("parallel() item %d missing 'tool' field", i))) - } - descriptors[i].Tool = toolName - if args, ok := m["args"].(map[string]any); ok { - descriptors[i].Args = args + // Re-read the element from the live array so a tool-function tag (erased + // by Export) is still recoverable; `item` is used only to classify shape. + el := arr.Get(strconv.Itoa(i)) + switch item.(type) { + case []any: // tuple form [toolOrFn, args] + descriptors[i] = parseParallelTuple(vm, el.ToObject(vm), i) + case map[string]any: // object form {tool, args} + descriptors[i] = parseParallelObject(vm, el.ToObject(vm), i) + default: + panic(newJSError(vm, fmt.Sprintf( + "parallel() item %d must be an object {tool, args} or a [tool, args] tuple", i))) } } return descriptors } +// parseParallelTuple resolves a [toolOrFn, args] element. +func parseParallelTuple(vm *goja.Runtime, el *goja.Object, i int) parallelCallDescriptor { + name := resolveParallelToolName(vm, el.Get("0"), i, true) + return parallelCallDescriptor{Tool: name, Args: resolveParallelArgs(vm, el.Get("1"), i)} +} + +// parseParallelObject resolves a {tool, args} element. +func parseParallelObject(vm *goja.Runtime, el *goja.Object, i int) parallelCallDescriptor { + toolVal := el.Get("tool") + if toolVal == nil || goja.IsUndefined(toolVal) || goja.IsNull(toolVal) { + panic(newJSError(vm, fmt.Sprintf("parallel() item %d missing 'tool' field", i))) + } + name := resolveParallelToolName(vm, toolVal, i, false) + return parallelCallDescriptor{Tool: name, Args: resolveParallelArgs(vm, el.Get("args"), i)} +} + +// resolveParallelToolName accepts either a bound tool function (whose hidden +// canonical name is recovered) or a plain "ns__name" string. tuple reports +// which slot the value came from so the error names the right field. +func resolveParallelToolName(vm *goja.Runtime, v goja.Value, i int, tuple bool) string { + if name, ok := toolNameOfFunc(vm, v); ok { + return name + } + if _, isFn := goja.AssertFunction(v); isFn { + panic(newJSError(vm, fmt.Sprintf( + "parallel() item %d: the function passed is not a namespace tool — pass a tool reference like github.list_issues, or its \"ns__name\" string", i))) + } + if v != nil && !goja.IsUndefined(v) && !goja.IsNull(v) { + if name, ok := v.Export().(string); ok && name != "" { + return name + } + } + slot := "'tool'" + if tuple { + slot = "element 0" + } + panic(newJSError(vm, fmt.Sprintf( + "parallel() item %d: %s must be a tool reference (e.g. github.list_issues) or an \"ns__name\" string", i, slot))) +} + +// resolveParallelArgs exports the args value to a map. Missing/undefined args +// are allowed (a no-arg tool call); a non-object args value is rejected. +func resolveParallelArgs(vm *goja.Runtime, v goja.Value, i int) map[string]any { + if v == nil || goja.IsUndefined(v) || goja.IsNull(v) { + return nil + } + if m, ok := v.Export().(map[string]any); ok { + return m + } + panic(newJSError(vm, fmt.Sprintf("parallel() item %d: 'args' must be an object", i))) +} + // parallelCallResult holds one result from a concurrent tool call. type parallelCallResult struct { index int @@ -90,13 +165,18 @@ func (s *Sandbox) executeParallel( defer wg.Done() start := time.Now() - argsJSON, marshalErr := json.Marshal(desc.Args) - if marshalErr != nil { - results[i] = parallelCallResult{index: i, err: marshalErr, dur: time.Since(start)} - return - } - if argsJSON == nil { - argsJSON = json.RawMessage("{}") + // Normalize no-arg calls to "{}" to match the sequential path + // (marshalToolArgs): a nil/empty args map marshals to "null", + // which a downstream tool expecting an object can reject. Both + // call forms must send the same shape for the same call. + argsJSON := json.RawMessage("{}") + if len(desc.Args) > 0 { + b, marshalErr := json.Marshal(desc.Args) + if marshalErr != nil { + results[i] = parallelCallResult{index: i, err: marshalErr, dur: time.Since(start)} + return + } + argsJSON = b } res, callErr := s.caller.CallTool(ctx, desc.Tool, argsJSON) @@ -132,20 +212,24 @@ func collectParallelResults( Name: toolName, Duration: cr.dur, } - argsJSON, _ := json.Marshal(descriptors[cr.index].Args) - if argsJSON != nil { - record.Args = argsJSON + // Mirror the dispatch normalization so the audit record shows the same + // "{}" shape actually sent for a no-arg call, not "null". + record.Args = json.RawMessage("{}") + if len(descriptors[cr.index].Args) > 0 { + if argsJSON, err := json.Marshal(descriptors[cr.index].Args); err == nil { + record.Args = argsJSON + } } if cr.err != nil { - record.Error = buildToolErrorMessage(toolName, argsJSON, cr.err.Error(), nil, nil, toolNames) + record.Error = buildToolErrorMessage(toolName, record.Args, cr.err.Error(), nil, nil, toolNames) _ = jsResults.Set(strconv.Itoa(cr.index), nil) } else { // Exact value, same contract as the sequential path: JS consumers // see the true downstream result, never a pruned copy. val, errText := parseToolResultValue(cr.result) if errText != "" { - record.Error = buildToolErrorMessage(toolName, argsJSON, errText, nil, nil, toolNames) + record.Error = buildToolErrorMessage(toolName, record.Args, errText, nil, nil, toolNames) record.Result = cr.result _ = jsResults.Set(strconv.Itoa(cr.index), nil) } else { diff --git a/internal/codemode/sandbox_parallel_refs_test.go b/internal/codemode/sandbox_parallel_refs_test.go new file mode 100644 index 00000000..e297b1ec --- /dev/null +++ b/internal/codemode/sandbox_parallel_refs_test.go @@ -0,0 +1,195 @@ +package codemode + +import ( + "context" + "encoding/json" + "strings" + "testing" + "time" +) + +// parallelRefTools is the tool set used by the calling-convention tests: three +// svc tools whose canned responses echo a distinguishing value so result +// ordering can be asserted. +func parallelRefTools() (*mockToolCaller, []ToolDef) { + caller := newMockCaller() + caller.responses["svc__a"] = json.RawMessage(`{"content":[{"type":"text","text":"{\"v\":\"a\"}"}]}`) + caller.responses["svc__b"] = json.RawMessage(`{"content":[{"type":"text","text":"{\"v\":\"b\"}"}]}`) + caller.responses["svc__c"] = json.RawMessage(`{"content":[{"type":"text","text":"{\"v\":\"c\"}"}]}`) + schema := json.RawMessage(`{"type":"object","properties":{"q":{"type":"string"}}}`) + tools := []ToolDef{ + {Name: "svc__a", InputSchema: schema}, + {Name: "svc__b", InputSchema: schema}, + {Name: "svc__c", InputSchema: schema}, + } + return caller, tools +} + +// argsFor returns the JSON args recorded for the first call to the named tool. +func argsFor(t *testing.T, caller *mockToolCaller, name string) string { + t.Helper() + caller.mu.Lock() + defer caller.mu.Unlock() + for _, c := range caller.calls { + if c.Name == name { + return string(c.Args) + } + } + t.Fatalf("no recorded call to %s (calls: %v)", name, caller.calls) + return "" +} + +// TestParallel_BoundReferenceForms drives parallel() through the natural +// calling forms unified with the sequential path: [fn, args] tuples, +// {tool: fn, args} objects, and mixed batches — asserting both ordering and +// that args actually reach the right downstream tool. +func TestParallel_BoundReferenceForms(t *testing.T) { + cases := []struct { + name string + code string + wantOutput string + wantArgs map[string]string // tool name -> expected recorded args JSON + }{ + { + name: "tuple form [fn, args] preserves order and passes args", + code: ` +const r = parallel([ + [svc.a, {q: "one"}], + [svc.b, {q: "two"}], + [svc.c, {q: "three"}], +]); +print(r.map(x => x.v).join(","));`, + wantOutput: "a,b,c\n", + wantArgs: map[string]string{ + "svc__a": `{"q":"one"}`, + "svc__c": `{"q":"three"}`, + }, + }, + { + name: "object form with bound function reference", + code: ` +const r = parallel([ + {tool: svc.a, args: {q: "x"}}, + {tool: svc.b, args: {q: "y"}}, +]); +print(r.map(x => x.v).join(","));`, + wantOutput: "a,b\n", + wantArgs: map[string]string{"svc__b": `{"q":"y"}`}, + }, + { + name: "mixed batch: bound tuple, bound object, and legacy string", + code: ` +const r = parallel([ + [svc.a, {q: "1"}], + {tool: svc.b, args: {q: "2"}}, + {tool: "svc__c", args: {q: "3"}}, +]); +print(r.map(x => x.v).join(","));`, + wantOutput: "a,b,c\n", + wantArgs: map[string]string{ + "svc__a": `{"q":"1"}`, + "svc__b": `{"q":"2"}`, + "svc__c": `{"q":"3"}`, + }, + }, + { + name: "tuple with string tool name (symmetry) and missing args is allowed", + code: ` +const r = parallel([ + ["svc__a"], + [svc.b, {q: "z"}], +]); +print(r.map(x => x.v).join(","));`, + wantOutput: "a,b\n", + wantArgs: map[string]string{"svc__a": `{}`}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + caller, tools := parallelRefTools() + sandbox := NewSandbox(caller, 5*time.Second) + result, err := sandbox.Execute(context.Background(), tc.code, tools) + if err != nil { + t.Fatal(err) + } + if result.Error != "" { + t.Fatalf("unexpected execution error: %s", result.Error) + } + if result.Output != tc.wantOutput { + t.Fatalf("want output %q, got %q", tc.wantOutput, result.Output) + } + for tool, wantArgs := range tc.wantArgs { + if got := argsFor(t, caller, tool); got != wantArgs { + t.Fatalf("args for %s: want %s, got %s", tool, wantArgs, got) + } + } + }) + } +} + +// TestParallel_RefValidation covers the new-form validation errors: a non-tool +// closure, a non-object args value, and a tuple whose tool slot is empty. Each +// must surface a clear, actionable message rather than a bare transport error. +func TestParallel_RefValidation(t *testing.T) { + cases := []struct { + name string + code string + wantSub string + }{ + { + name: "an agent's own closure is not a tool", + code: `parallel([[() => 1, {}]]);`, + wantSub: "not a namespace tool", + }, + { + name: "non-object args in tuple form is rejected", + code: `parallel([[svc.a, 42]]);`, + wantSub: "'args' must be an object", + }, + { + name: "tuple with empty tool slot is rejected", + code: `parallel([[undefined, {q: "x"}]]);`, + wantSub: "element 0 must be a tool reference", + }, + { + name: "scalar item is neither object nor tuple", + code: `parallel([42]);`, + wantSub: "must be an object", + }, + } + + _, tools := parallelRefTools() + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + caller := newMockCaller() + sandbox := NewSandbox(caller, 5*time.Second) + result, err := sandbox.Execute(context.Background(), tc.code, tools) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(result.Error, tc.wantSub) { + t.Fatalf("want error containing %q, got %q", tc.wantSub, result.Error) + } + }) + } +} + +// TestParallel_TaggedFuncHiddenFromKeys guards the ergonomic contract that the +// hidden __tool tag never leaks into Object.keys() of a namespace tool — an +// agent enumerating a function must not see sandbox bookkeeping. +func TestParallel_TaggedFuncHiddenFromKeys(t *testing.T) { + caller, tools := parallelRefTools() + sandbox := NewSandbox(caller, 5*time.Second) + result, err := sandbox.Execute(context.Background(), + `print(JSON.stringify(Object.keys(svc.a)));`, tools) + if err != nil { + t.Fatal(err) + } + if result.Error != "" { + t.Fatalf("unexpected error: %s", result.Error) + } + if strings.TrimSpace(result.Output) != "[]" { + t.Fatalf("tool function should expose no enumerable keys, got %q", result.Output) + } +} diff --git a/internal/gateway/handler_codemode.go b/internal/gateway/handler_codemode.go index d385477c..dcc09bfd 100644 --- a/internal/gateway/handler_codemode.go +++ b/internal/gateway/handler_codemode.go @@ -863,7 +863,8 @@ func (h *handler) buildCodeExecuteTool(ctx context.Context) (Tool, bool) { "Calling patterns:\n" + "- Sequential (default): calls return values directly, so you can " + "daisy-chain — pass the result of one call straight into the next.\n" + - "- Concurrent: use parallel([{tool,args},...]) when calls are independent (max 20 per call). " + + "- Concurrent: use parallel([[ns.tool, args], ...]) when calls are independent (max 20 per call) — " + + "same call form as the sequential path (a plain \"ns__name\" string or {tool, args} object also works). " + "Failed entries in a parallel() batch surface as `null` at their index — the call itself does NOT throw.\n\n" + "Search search_tools for function signatures before writing code, or call help() in the " + "script to list namespaces and help('namespace') for a namespace's tool signatures (no search round-trip).\n" + @@ -903,9 +904,10 @@ func (h *handler) buildCodeExecuteTool(ctx context.Context) (Tool, bool) { "- Sequential (default): calls return values directly, so you can " + "daisy-chain — pass the result of one call straight into the next.\n" + " e.g. const repo = github.get_repo({owner, repo}); const issues = github.list_issues({owner, repo: repo.name})\n" + - "- Concurrent: use parallel([{tool,args},...]) when calls are independent (max 20 per call) " + - "and don't depend on each other's results. Returns an array of results; failed entries surface " + - "as `null` at their index — parallel() itself does NOT throw.\n\n" + + "- Concurrent: use parallel([[ns.tool, args], ...]) when calls are independent (max 20 per call) " + + "and don't depend on each other's results — same call form as the sequential path (a plain " + + "\"ns__name\" string or {tool, args} object also works). Returns an array of results; failed entries " + + "surface as `null` at their index — parallel() itself does NOT throw.\n\n" + "Search search_tools for function signatures before writing code, or call help() in the " + "script to list namespaces and help('namespace') for a namespace's tool signatures (no search round-trip).\n" + "Errors throw real `Error` objects. A typo on a namespace or member yields a `ReferenceError` " +