diff --git a/docs/tools/eval.md b/docs/tools/eval.md index a0d177b69..416625a06 100644 --- a/docs/tools/eval.md +++ b/docs/tools/eval.md @@ -134,11 +134,11 @@ Implemented in `packages/coding-agent/src/eval/js/worker-core.ts`, `packages/cod - `completion(prompt, opts?)` for oneshot, stateless model calls (see _Oneshot completion helper_ below) - `agent(prompt, opts?)` for a single subagent call, plus `parallel()` / `pipeline()` bounded-pool helpers (see _Subagent helper_ below) - JS helpers that touch the host/runtime boundary are async and `await`able; pure text helpers (`sort`, `uniq`, `counter`) return synchronously but may still be safely awaited. -- JS helper signatures use a trailing options object rather than Python keyword arguments: - - `await read(path, { offset?, limit? })` - - `await tree(path = ".", { maxDepth?, hidden? })` - - `sort(text, { reverse?, unique? })`, `uniq(text, { count? })`, `counter(items, { limit?, reverse? })` - - `await agent(prompt, { agentType?, model?, label?, schema? })` +- JS helper options may be passed either positionally in the Python order or as a trailing options object. `null` and `undefined` skip positional slots: + - `await read(path, offset?, limit?)` or `await read(path, { offset?, limit? })` + - `await tree(path = ".", maxDepth?, showHidden?)` or `await tree(path, { maxDepth?, showHidden? })` + - `sort(text, reverse?, unique?)`, `uniq(text, count?)`, `counter(items, limit?, reverse?)` + - `await agent(prompt, agentType?, model?, label?, schema?)` or `await agent(prompt, { agentType?, model?, label?, schema? })` - `await parallel([() => agent("a"), () => agent("b")])` - `await pipeline(items, stage1, stage2)` - `display(value)` behavior: @@ -192,7 +192,7 @@ Both runtimes expose `completion()` — a single stateless completion against a Both runtimes expose `agent()` — a single subagent invocation routed through `packages/coding-agent/src/eval/agent-bridge.ts` into the same `runSubprocess(...)` path used by the `task` tool. It uses the current eval session's spawn policy and inherits the parent eval executor id, so parent and subagent code share JS/Python runtime state. - Signatures: - - JS: `await agent(prompt, { agentType?, model?, label?, schema? })` + - JS: `await agent(prompt, agentType?, model?, label?, schema?)` or `await agent(prompt, { agentType?, model?, label?, schema? })` - Python: `agent(prompt, *, agent_type="task", model=None, label=None, schema=None)` - `agentType` / `agent_type` defaults to the bundled `task` agent and resolves through normal agent discovery, so project and user agents work. - `model` overrides the selected agent's model. Without it, normal per-agent settings and the agent frontmatter model apply. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ab843145f..27079972c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -40,6 +40,7 @@ - Fixed the `ctrl+p` model-role cycle indicator (the `default / gpt / fable / …` chip track) stacking duplicate copies in the scrollback when other chat activity landed between two cycles. The track was emitted through `showStatus`, whose back-to-back coalescing only merges when the previous status is still the last transcript child; any interleaved append broke that identity check and appended a second track. It now renders into a dedicated anchored container above the editor (cleared and rebuilt in place each cycle, like the Todos HUD) and auto-clears after a short linger, so rapid presses or concurrent activity can never duplicate it. - Fixed the `Working…` loader vanishing for the rest of a turn after an auto-compaction (context-overflow recovery) or auto-retry. Those overlays took over the shared status container with a bare `statusContainer.clear()`, which detached the working loader but left `loadingAnimation` set; the resumed turn's `agent_start` → `ensureLoadingAnimation()` is guarded by `if (!this.loadingAnimation)`, so it skipped re-attaching the loader and the spinner stayed gone while the agent kept streaming. The overlay handlers now fully tear the working loader down (stop + dereference) via `#stopWorkingLoader()`, so the next `agent_start` recreates and re-attaches it. +- Fixed JS eval helper optional arguments rejecting Python-style positional calls. `read(path, offset, limit)` now works alongside `read(path, { offset, limit })`, `null`/`undefined` skip optional positional slots, and non-local URI reads such as `artifact://...` delegate through the read tool so line slicing works on spilled artifacts. ## [15.12.6] - 2026-06-14 ### Breaking Changes diff --git a/packages/coding-agent/src/eval/js/shared/prelude.txt b/packages/coding-agent/src/eval/js/shared/prelude.txt index 095b219a2..2a28ce8a0 100644 --- a/packages/coding-agent/src/eval/js/shared/prelude.txt +++ b/packages/coding-agent/src/eval/js/shared/prelude.txt @@ -1,33 +1,85 @@ if (!globalThis.__omp_js_prelude_loaded__) { globalThis.__omp_js_prelude_loaded__ = true; + const isNil = value => value === undefined || value === null; const isPlainObject = value => value !== null && typeof value === "object" && !Array.isArray(value); - const optionsArg = (name, value, rest, example) => { - if (rest.length > 0) { + const positionalOptions = (name, args, keys, example) => { + for (let index = keys.length; index < args.length; index++) { + if (!isNil(args[index])) { + throw new TypeError( + `${name}() accepts at most ${keys.length} positional optional args; got ${args.length}. Pass ${name}(..., ${example}) for named options.`, + ); + } + } + const options = {}; + for (let index = 0; index < keys.length && index < args.length; index++) { + const value = args[index]; + if (!isNil(value)) options[keys[index]] = value; + } + return options; + }; + const optionsArg = (name, value, rest, keys, example) => { + if (isNil(value)) return positionalOptions(name, [value, ...rest], keys, example); + if (isPlainObject(value)) { + if (rest.some(arg => !isNil(arg))) { + throw new TypeError( + `${name}() takes either a single trailing options object like ${example} or positional optional args; do not mix both forms.`, + ); + } + return value; + } + if (typeof value === "object") { + const kind = Array.isArray(value) ? "an array" : value.constructor?.name ?? "object"; throw new TypeError( - `${name}() takes options as a single trailing object literal, not positional arguments (got ${rest.length + 1} extra args). Pass them as ${name}(..., ${example}).`, + `${name}() options must be a plain object like ${example}, null/undefined, or positional optional args, not ${kind}.`, ); } - if (value === undefined || value === null) return {}; - if (!isPlainObject(value)) { - const kind = Array.isArray(value) ? "an array" : typeof value; - throw new TypeError( - `${name}() options must be a trailing object literal like ${example}, not ${kind}. JS helpers never take positional options.`, - ); - } - return value; + return positionalOptions(name, [value, ...rest], keys, example); }; const callHelper = (name, ...args) => globalThis.__omp_helpers__[name](...args); + const hasScheme = path => typeof path === "string" && /^[A-Za-z][A-Za-z0-9+.-]*:\/\//.test(path); + const shouldDelegateRead = path => hasScheme(path) && !path.toLowerCase().startsWith("local://"); + const withReadLineSelector = (path, options) => { + const offset = typeof options.offset === "number" ? options.offset : 1; + const limit = typeof options.limit === "number" ? options.limit : undefined; + if (offset <= 1 && limit === undefined) return path; + if (limit !== undefined && limit <= 0) return null; + const start = Math.max(1, offset); + if (limit === undefined) return `${path}:${start}-`; + return `${path}:${start}-${start + limit - 1}`; + }; + const readToolText = async path => { + const res = await globalThis.__omp_call_tool__("read", { path }); + return res && typeof res === "object" && "text" in res ? res.text : res; + }; - const read = (path, opts, ...rest) => callHelper("read", path, optionsArg("read", opts, rest, "{ offset, limit }")); + + const read = async (path, opts, ...rest) => { + const options = optionsArg("read", opts, rest, ["offset", "limit"], "{ offset, limit }"); + if (shouldDelegateRead(path)) { + const toolPath = withReadLineSelector(path, options); + return toolPath === null ? "" : readToolText(toolPath); + } + return callHelper("read", path, options); + }; const write = async (path, data) => callHelper("writeFile", path, data); const append = (path, content) => callHelper("append", path, content); - const sort = (text, opts, ...rest) => callHelper("sortText", text, optionsArg("sort", opts, rest, "{ reverse, unique }")); - const uniq = (text, opts, ...rest) => callHelper("uniqText", text, optionsArg("uniq", opts, rest, "{ count }")); + const sort = (text, opts, ...rest) => + callHelper("sortText", text, optionsArg("sort", opts, rest, ["reverse", "unique"], "{ reverse, unique }")); + const uniq = (text, opts, ...rest) => callHelper("uniqText", text, optionsArg("uniq", opts, rest, ["count"], "{ count }")); const counter = (items, opts, ...rest) => - callHelper("counter", items, optionsArg("counter", opts, rest, "{ limit, reverse }")); + callHelper("counter", items, optionsArg("counter", opts, rest, ["limit", "reverse"], "{ limit, reverse }")); const diff = (a, b) => callHelper("diff", a, b); - const tree = (path = ".", opts, ...rest) => callHelper("tree", path, optionsArg("tree", opts, rest, "{ maxDepth, showHidden }")); + const tree = (path = ".", opts, ...rest) => { + if (isPlainObject(path) && opts === undefined && rest.length === 0) { + return callHelper("tree", ".", path); + } + return callHelper( + "tree", + isNil(path) ? "." : path, + optionsArg("tree", opts, rest, ["maxDepth", "showHidden"], "{ maxDepth, showHidden }"), + ); + }; const env = (key, value) => callHelper("env", key, value); const tool = new Proxy( @@ -58,14 +110,14 @@ if (!globalThis.__omp_js_prelude_loaded__) { const hasOwn = (object, key) => Object.prototype.hasOwnProperty.call(object, key); const completion = async (prompt, opts, ...rest) => { - const o = optionsArg("completion", opts, rest, "{ model, system, schema }"); + const o = optionsArg("completion", opts, rest, ["model", "system", "schema"], "{ model, system, schema }"); const res = await globalThis.__omp_call_tool__("__completion__", { prompt, ...o }); const text = res && typeof res === "object" ? res.text : res; return hasOwn(o, "schema") ? JSON.parse(text) : text; }; const agent = async (prompt, opts, ...rest) => { - const o = optionsArg("agent", opts, rest, "{ agentType, model, label, schema }"); + const o = optionsArg("agent", opts, rest, ["agentType", "model", "label", "schema"], "{ agentType, model, label, schema }"); const res = await globalThis.__omp_call_tool__("__agent__", { prompt, ...o }); const text = res && typeof res === "object" ? res.text : res; return hasOwn(o, "schema") ? JSON.parse(text) : text; diff --git a/packages/coding-agent/test/core/js-executor.test.ts b/packages/coding-agent/test/core/js-executor.test.ts index 053a794ad..b22bc76b6 100644 --- a/packages/coding-agent/test/core/js-executor.test.ts +++ b/packages/coding-agent/test/core/js-executor.test.ts @@ -311,8 +311,11 @@ describe("executeJs", () => { const result = await executeJs( [ "const full = await read('config.json');", - "const sliced = await read('config.json', { offset: 2, limit: 1 });", - "return { isString: typeof full === 'string', full, sliced };", + "const objectSliced = await read('config.json', { offset: 2, limit: 1 });", + "const positionalSliced = await read('config.json', 3, 1);", + "const nullOffsetLimit = await read('config.json', null, 2);", + "const undefinedOffsetLimit = await read('config.json', undefined, 1);", + "return { isString: typeof full === 'string', full, objectSliced, positionalSliced, nullOffsetLimit, undefinedOffsetLimit };", ].join("\n"), { sessionId, @@ -322,23 +325,58 @@ describe("executeJs", () => { ); expect(result.exitCode).toBe(0); - expect(getStatusEvents(result)).toHaveLength(2); + expect(getStatusEvents(result)).toHaveLength(5); expect(getJsonData(result)).toEqual({ isString: true, full: '{\n "name": "demo",\n "enabled": true\n}', - sliced: ' "name": "demo",', + objectSliced: ' "name": "demo",', + positionalSliced: ' "enabled": true', + nullOffsetLimit: '{\n "name": "demo",', + undefinedOffsetLimit: "{", }); }); - it("rejects protocol paths and directory reads from native read()", async () => { - const protocolResult = await executeJs("await read('agent://demo');", { - sessionId, - session, - sessionFile, + it("delegates URI reads through the read tool with positional slicing", async () => { + const execute = vi.fn(async (_toolCallId: string, args: unknown): Promise => { + const record = args as { path: string }; + return { content: [{ type: "text", text: record.path.endsWith(":1-1400") ? "wide" : "limited" }] }; }); - expect(protocolResult.exitCode).toBe(1); - expect(protocolResult.output).toContain("Protocol paths are not supported"); + const toolSession: ToolSession = { + ...session, + getToolByName: name => (name === "read" ? createTool("read", execute) : undefined), + }; + const result = await executeJs( + [ + "const wide = await read('artifact://15:raw', 1, 1400);", + "const limited = await read('artifact://15:raw', null, 2);", + "return { wide, limited };", + ].join("\n"), + { + sessionId, + session: toolSession, + sessionFile, + }, + ); + + expect(result.exitCode).toBe(0); + expect(getStatusEvents(result)).toHaveLength(2); + expect(getJsonData(result)).toEqual({ wide: "wide", limited: "limited" }); + expect(execute).toHaveBeenNthCalledWith( + 1, + expect.stringMatching(/^js-read-/), + { path: "artifact://15:raw:1-1400", _i: "js prelude" }, + expect.any(AbortSignal), + ); + expect(execute).toHaveBeenNthCalledWith( + 2, + expect.stringMatching(/^js-read-/), + { path: "artifact://15:raw:1-2", _i: "js prelude" }, + expect.any(AbortSignal), + ); + }); + + it("rejects directory reads from native read()", async () => { const directoryResult = await executeJs("await read('.');", { sessionId, session,